From 15412351001d6eb284462d3990f52b3c44b6c73a Mon Sep 17 00:00:00 2001 From: ayanx Date: Thu, 29 Oct 2026 11:50:57 +0530 Subject: [PATCH] fix(store): survive bad cgroup paths, honor a command-line budget CgroupMemoryLimit.read resolved mount points outside any catch, so a mountinfo path the JVM can't represent, such as a NUL from a \000 escape, threw InvalidPathException and failed the Store's start. Treat that line's limits as hidden and flag the view incomplete instead. PdConfigureListener now also drops PD's rocksdb.total_memory_size when a --rocksdb.total_memory_size argument sets it. Environment variables stay out: Spring binds ROCKSDB_TOTAL_MEMORY_SIZE to total.memory.size, so it never sets the budget and must not drop PD's value. --- hugegraph-store/docs/deployment-guide.md | 4 +- .../store/options/CgroupMemoryLimit.java | 72 ++++++++++--------- .../node/listener/PdConfigureListener.java | 10 +-- ...PdConfigureListenerMemoryOverrideTest.java | 39 +++++++++- .../store/options/CgroupMemoryLimitTest.java | 22 ++++++ 5 files changed, 107 insertions(+), 40 deletions(-) diff --git a/hugegraph-store/docs/deployment-guide.md b/hugegraph-store/docs/deployment-guide.md index 1e31c92137..2fad0d33a2 100644 --- a/hugegraph-store/docs/deployment-guide.md +++ b/hugegraph-store/docs/deployment-guide.md @@ -452,7 +452,9 @@ local file only when it has no local file or the file is no newer than PD's `S:T upload sets no timestamp and a missing one counts as 0, so changing `S:FS` alone changes nothing. The simplest upgrade is to set `HG_STORE_ROCKSDB_TOTAL_MEMORY_SIZE=auto` (or `-Drocksdb.total_memory_size=auto`) on every Store and restart it; the Store keeps this value -above both its local file and PD's copy. To change the shared copy instead, set +above both its local file and PD's copy, as it does a `--rocksdb.total_memory_size` argument to +the Store jar. An environment variable such as `ROCKSDB_TOTAL_MEMORY_SIZE` doesn't set the +budget, because Spring binds it to a different key. To change the shared copy instead, set `rocksdb.total_memory_size` to `0` or remove it in `S:FS`, then put the current time in milliseconds into `S:Timestamp` through PD's KV service (gRPC `KvService.put`). Use `auto` there only once every Store runs this release: an earlier release can't parse `auto` and fails to start, diff --git a/hugegraph-store/hg-store-core/src/main/java/org/apache/hugegraph/store/options/CgroupMemoryLimit.java b/hugegraph-store/hg-store-core/src/main/java/org/apache/hugegraph/store/options/CgroupMemoryLimit.java index d17917feef..4c7c8ff2a8 100644 --- a/hugegraph-store/hg-store-core/src/main/java/org/apache/hugegraph/store/options/CgroupMemoryLimit.java +++ b/hugegraph-store/hg-store-core/src/main/java/org/apache/hugegraph/store/options/CgroupMemoryLimit.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.InvalidPathException; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; @@ -81,43 +82,50 @@ static CgroupMemoryLimit read(Path procSelfCgroup, Path mountinfo, Path defaultR if (!v2 && !("," + parts[1] + ",").contains(",memory,")) { continue; } - List mounts = v2 ? v2Mounts : v1Mounts; - boolean matched = false; - boolean fullView = false; long lineLimit = -1L; - for (String[] mount : mounts) { - if (!contains(mount[0], parts[2])) { - // A mount of another subtree says nothing about this cgroup - continue; - } - matched = true; - Path point = Paths.get(mount[1]); - String relative = rebase(parts[2], mount[0]); - // A full-hierarchy mount only covers the ancestors if this process's own - // directory is reachable through it - fullView |= mount[0].equals("/") && visible(point, relative); - lineLimit = lowest(lineLimit, v2 ? v2Limit(point, relative) : - v1Limit(point, relative)); - } - if (!matched && !mounts.isEmpty()) { - // In a cgroup namespace /proc/self/cgroup says "/" while a mount made outside - // it has a root such as "/..", so the process's own cgroup can't be located. - // The limits at the mount roots still bound it - incomplete = true; + try { + List mounts = v2 ? v2Mounts : v1Mounts; + boolean matched = false; + boolean fullView = false; for (String[] mount : mounts) { + if (!contains(mount[0], parts[2])) { + // A mount of another subtree says nothing about this cgroup + continue; + } + matched = true; Path point = Paths.get(mount[1]); - lineLimit = lowest(lineLimit, v2 ? readLimit(point.resolve("memory.max")) : - v1Limit(point, "")); + String relative = rebase(parts[2], mount[0]); + // A full-hierarchy mount only covers the ancestors if this process's own + // directory is reachable through it + fullView |= mount[0].equals("/") && visible(point, relative); + lineLimit = lowest(lineLimit, v2 ? v2Limit(point, relative) : + v1Limit(point, relative)); } - } else if (!matched) { - String relative = rebase(parts[2], "/"); - lineLimit = v2 ? v2Limit(defaultRoot, relative) : - v1Limit(defaultRoot.resolve("memory"), relative); - } else if (v2 && (!fullView || parts[2].equals("/"))) { - // Only subtree mounts such as /kubepods, or the process sits at the root it - // can see, which inside a private cgroup namespace is the namespace root: a - // memory.max above is hidden. v1's hierarchical limit has no such gap + if (!matched && !mounts.isEmpty()) { + // In a cgroup namespace /proc/self/cgroup says "/" while a mount made outside + // it has a root such as "/..", so the process's own cgroup can't be located. + // The limits at the mount roots still bound it + incomplete = true; + for (String[] mount : mounts) { + Path point = Paths.get(mount[1]); + lineLimit = lowest(lineLimit, v2 ? readLimit(point.resolve("memory.max")) : + v1Limit(point, "")); + } + } else if (!matched) { + String relative = rebase(parts[2], "/"); + lineLimit = v2 ? v2Limit(defaultRoot, relative) : + v1Limit(defaultRoot.resolve("memory"), relative); + } else if (v2 && (!fullView || parts[2].equals("/"))) { + // Only subtree mounts such as /kubepods, or the process sits at the root it + // can see, which inside a private cgroup namespace is the namespace root: a + // memory.max above is hidden. v1's hierarchical limit has no such gap + incomplete = true; + } + } catch (InvalidPathException e) { + // A path the JVM can't represent, such as a NUL from a \000 escape, hides + // this line's limits; say so rather than fail the Store's start incomplete = true; + continue; } if (lineLimit > 0 && (limit <= 0 || lineLimit < limit)) { limit = lineLimit; diff --git a/hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/listener/PdConfigureListener.java b/hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/listener/PdConfigureListener.java index 9ca6d0e043..8c76876d6b 100644 --- a/hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/listener/PdConfigureListener.java +++ b/hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/listener/PdConfigureListener.java @@ -57,10 +57,12 @@ public class PdConfigureListener implements private static final String TIMESTAMP_KEY = "S:Timestamp"; private static final String PD_CONFIG_FILE_NAME = "application-pd.yml"; static final String MEMORY_KEY = "rocksdb.total_memory_size"; - // Where an operator sets one Store's RocksDB budget: the Docker entrypoint's - // SPRING_APPLICATION_JSON (HG_STORE_ROCKSDB_TOTAL_MEMORY_SIZE) or - // -Drocksdb.total_memory_size - private static final String[] MEMORY_OVERRIDE_SOURCES = {"spring.application.json", + // Where an operator sets one Store's RocksDB budget: --rocksdb.total_memory_size, the + // Docker entrypoint's SPRING_APPLICATION_JSON (HG_STORE_ROCKSDB_TOTAL_MEMORY_SIZE) or + // -Drocksdb.total_memory_size. Not systemEnvironment: ROCKSDB_TOTAL_MEMORY_SIZE binds to + // rocksdb's total.memory.size key, so it never sets the budget and must not drop PD's + private static final String[] MEMORY_OVERRIDE_SOURCES = {"commandLineArgs", + "spring.application.json", "systemProperties"}; private final String workDir = System.getProperty("user.dir"); private final String fileSeparator = System.getProperty("file.separator"); diff --git a/hugegraph-store/hg-store-test/src/main/java/org/apache/hugegraph/store/node/listener/PdConfigureListenerMemoryOverrideTest.java b/hugegraph-store/hg-store-test/src/main/java/org/apache/hugegraph/store/node/listener/PdConfigureListenerMemoryOverrideTest.java index d6c37a91fc..901359e811 100644 --- a/hugegraph-store/hg-store-test/src/main/java/org/apache/hugegraph/store/node/listener/PdConfigureListenerMemoryOverrideTest.java +++ b/hugegraph-store/hg-store-test/src/main/java/org/apache/hugegraph/store/node/listener/PdConfigureListenerMemoryOverrideTest.java @@ -30,13 +30,15 @@ import org.springframework.core.env.MapPropertySource; import org.springframework.core.env.MutablePropertySources; import org.springframework.core.env.PropertiesPropertySource; +import org.springframework.core.env.SimpleCommandLinePropertySource; import org.springframework.core.env.StandardEnvironment; +import org.springframework.core.env.SystemEnvironmentPropertySource; /** * PdConfigureListener puts the PD-managed application-pd.yml in front of every other - * property source. An explicit per-Store rocksdb.total_memory_size, from the Docker - * entrypoint's SPRING_APPLICATION_JSON or -D, must still win when RocksdbConfig binds, - * while the rest of the PD configuration keeps its precedence. + * property source. An explicit per-Store rocksdb.total_memory_size, from a command-line + * argument, the Docker entrypoint's SPRING_APPLICATION_JSON or -D, must still win when + * RocksdbConfig binds, while the rest of the PD configuration keeps its precedence. */ public class PdConfigureListenerMemoryOverrideTest { @@ -65,6 +67,37 @@ public void testSystemPropertyOverrideWinsOverPdConfig() { .get("total_memory_size")); } + @Test + public void testCommandLineOverrideWinsOverPdConfig() { + assertEquals("1073741824", + bind(withCommandLine(environment(null, null))).get("total_memory_size")); + // Spring puts command-line arguments ahead of SPRING_APPLICATION_JSON + assertEquals("1073741824", bind(withCommandLine(environment(json("268435456"), null))) + .get("total_memory_size")); + } + + private static StandardEnvironment withCommandLine(StandardEnvironment env) { + env.getPropertySources().addFirst(new SimpleCommandLinePropertySource( + "--" + PdConfigureListener.MEMORY_KEY + "=1073741824")); + return env; + } + + @Test + public void testEnvironmentVariableKeepsPdConfig() { + StandardEnvironment env = environment(null, null); + Map variables = new HashMap<>(); + variables.put("ROCKSDB_TOTAL_MEMORY_SIZE", "268435456"); + env.getPropertySources().addBefore("applicationConfig: [application-pd.yml]", + new SystemEnvironmentPropertySource( + StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, + variables)); + // The variable binds to the total.memory.size key, not the budget, so it must not + // drop PD's value and leave the Store on its local file + Map rocksdb = bind(env); + assertEquals(PD_BUDGET, rocksdb.get("total_memory_size")); + assertEquals("268435456", rocksdb.get("total.memory.size")); + } + @Test public void testWithoutOverridePdConfigStillApplies() { Map rocksdb = bind(environment(null, null)); diff --git a/hugegraph-store/hg-store-test/src/main/java/org/apache/hugegraph/store/options/CgroupMemoryLimitTest.java b/hugegraph-store/hg-store-test/src/main/java/org/apache/hugegraph/store/options/CgroupMemoryLimitTest.java index 79f508a737..84ad2280d7 100644 --- a/hugegraph-store/hg-store-test/src/main/java/org/apache/hugegraph/store/options/CgroupMemoryLimitTest.java +++ b/hugegraph-store/hg-store-test/src/main/java/org/apache/hugegraph/store/options/CgroupMemoryLimitTest.java @@ -196,6 +196,28 @@ public void testCgroupV1SubtreeMountAndEscapedMountPoint() throws IOException { } } + @Test + public void testUnrepresentableMountPointIsFlaggedNotFatal() throws IOException { + Path root = Files.createTempDirectory("cgroup-bad-path"); + try { + Path v1 = root.resolve("memory"); + write(v1, "memory.limit_in_bytes", String.valueOf(4 * GIB)); + Path proc = write(root, "proc-self-cgroup", "0::/kubepods/pod1\n4:memory:/\n"); + // The kernel never writes \000, but a NUL in the v2 mount point must only hide + // that line's limits, not stop the Store or the v1 line's lookup + Path mountinfo = write(root, "mountinfo", + "30 25 0:26 / " + root.resolve("v2") + + "\\000x rw - cgroup2 cgroup2 rw\n" + + "40 25 0:30 / " + v1 + " rw - cgroup cgroup rw,memory\n"); + CgroupMemoryLimit limit = CgroupMemoryLimit.read(proc, mountinfo, root); + assertTrue(limit.incomplete); + assertEquals(4 * GIB, limit.bytes); + assertEquals(1, limit.version); + } finally { + FileUtils.deleteQuietly(root.toFile()); + } + } + @Test public void testCgroupV2MountOrderDoesNotHideAncestorLimit() throws IOException { Path root = Files.createTempDirectory("cgroup-v2-mounts");