Repository navigation
Conversation
The shipped application-pd.yml set rocksdb.total_memory_size to 32 GB. RaftRocksdbOptions splits that into the write buffer manager's cache and the block cache, both off-heap, so -Xmx and the container limit do not bound them. A Store under a 4 GiB limit was OOM-killed once it held about 1 GB of data (apache#3254). The heap-size fallback in AppConfig never ran, because the key was always set. total_memory_size now ships as 0. Unset or 0 sizes the budget as half of what is left after the heap, MaxDirectMemorySize (heap size when unset) and a 1.5 GiB reserve for the raft log cache and other native memory, clamped to 256 MiB..32 GB. Physical memory comes from the OperatingSystemMXBean, which on JDK 17 reports the cgroup v1/v2 limit in a container; older JDK 17 updates may not see a limit set only on a parent cgroup. A positive value is used as given, but values under 1 MiB are refused, since the write buffer manager's 2/3 share would round to 0 bytes and lift the memtable limit. A non-numeric value such as "4GB" fails with a clear message instead of a bare NumberFormatException. Startup warns when the floor applies, when an explicit value is under 256 MiB, or when it exceeds what is left after the heap, direct memory and the reserve. The Docker entrypoint, which builds SPRING_APPLICATION_JSON itself, had no HG_STORE_* variable for any rocksdb.* key. It now accepts HG_STORE_ROCKSDB_TOTAL_MEMORY_SIZE (bytes) and adds it only when set; the value outranks the config file and -Drocksdb.total_memory_size. AppConfig also stops computing a zero raft disruptor buffer for budgets under 1 GB, where log(0) rounded to a size of 0; the formula moved to RocksdbMemoryBudget.disruptorBufferSize() so it can be tested. The shipped application.yml pins disruptorBufferSize, so this only applies when that key is unset. The comment in helm/hugegraph/values-cluster.yaml that said container limits do not configure the RocksDB budget now describes the auto-sizing and HG_STORE_ROCKSDB_TOTAL_MEMORY_SIZE. Fixes apache#3254
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3355 +/- ##
============================================
+ Coverage 54.31% 54.62% +0.31%
- Complexity 2250 2350 +100
============================================
Files 770 775 +5
Lines 67952 68655 +703
Branches 9213 9599 +386
============================================
+ Hits 36905 37502 +597
- Misses 27470 27605 +135
+ Partials 3577 3548 -29
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
With write_buffer_size unset or 0, AppConfig derives it as a thousandth of the RocksDB budget. An auto-sized budget can now be under 1 GiB: a 4 GiB container with -Xmx1g and -XX:MaxDirectMemorySize=512m gets 512 MiB, which derived 536870 bytes. rocksdb.write_buffer_size requires at least 1 MiB (RocksDBOptions.MEMTABLE_SIZE), so Store startup would have rejected the config in BusinessHandlerImpl.initRocksdb(). RocksdbMemoryBudget.writeBufferSize() clamps the derived value to 1 MiB, and a test runs it through the same OptionSpace/HugeConfig validation. The shipped write_buffer_size of 32000000 never took this path.
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: Score 7/10. The container case from #3254 is handled, and the earlier write_buffer_size thread is fixed by writeBufferSize() at 1927fbe. One blocking problem: auto-sizing counts an unset -XX:MaxDirectMemorySize as a second full heap, and the tarball launcher never sets it, so bare-metal installs with a large heap drop from the shipped 32000000000 bytes to the 256 MiB floor on upgrade. Evidence: the head's RocksdbMemoryBudget.java compiled standalone and resolve(null) run on a 16 GiB host with JDK 17.0.20 returned 268435456 for -Xmx8g and 3221225472 for -Xmx8g -XX:MaxDirectMemorySize=512m; start-hugegraph-store.sh, util.sh calc_xmx, the Store Dockerfile, RaftRocksdbOptions and the Helm store template were read at e691489. Only HugeGraph-Server CI and check-license are listed for this head, both queued, so the new Store tests were not seen passing in CI.
Auto-sizing counted an unset -XX:MaxDirectMemorySize as a second full heap, because that is what the JVM allows. bin/start-hugegraph-store.sh never sets the flag, so a bare-metal Store with a raised heap fell to the 256 MiB floor on upgrade: -Xmx8g on a 16 GiB host, or -Xmx36g on a 64 GiB host, went from the old fixed 32000000000 bytes to 268435456. The Store's direct buffers (gRPC/Netty) do not grow with the heap, so an unset cap now counts as the heap size capped at 1 GiB. Those two hosts get 2.75 GiB and 12.75 GiB. An explicit -XX:MaxDirectMemorySize is still counted as given, so the container case from apache#3254 is unchanged. The deployment guide now says what bare-metal hosts get.
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: Score 9/10. The earlier threads are fixed at this head: the derived write_buffer_size is clamped to 1 MiB, and an unset -XX:MaxDirectMemorySize now counts as at most 1 GiB, so a large-heap bare-metal Store no longer drops to the 256 MiB floor. The auto budget fits the #3254 container setup, and an explicit value still overrides it. Evidence: read RocksdbMemoryBudget, AppConfig.init(), RaftRocksdbOptions, RocksDBOptions, the Store launcher and calc_xmx, the Dockerfile, the entrypoint and both Helm values files at 84e8e27; worked through the arithmetic for 4 GiB/-Xmx1g/512m (512 MiB), the Helm cluster preset (2.5 GiB) and 16 GiB/-Xmx8g (2.75 GiB); ran test-store-docker-entrypoint.sh locally (9 passed); CI at this head ran CoreSuiteTest with the new tests (52 tests, 0 failures) and the entrypoint step, and cluster, docker and helm jobs pass. Only codecov/project fails, because the base is 19 commits behind.
imbajin
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: Auto-sizing can still use host memory on supported older JDK 17 builds that miss parent-only cgroup limits. Evidence: exact-head source and cgroup behavior review; see inline comments for the mounted-config upgrade and Store test-selection gaps.
test-store-docker-entrypoint.sh runs in the store job of
pd-store-ci.yml, but the selector didn't map it, so a PR that changed
only that script selected server, docker and macOS paths and skipped
the test. Map it to {"store"} in CONTRACT_SUITES and extend the selector
test so it checks the store job's condition and step as well.
Older JDK 17 updates can miss a cgroup memory limit set only on a parent cgroup and report host memory, so auto-sizing could still give RocksDB most of host RAM inside a smaller pod or systemd slice. RocksdbMemoryBudget now also reads the cgroup itself: on v2 it walks memory.max from the process's cgroup up to the mount root, on v1 it reads hierarchical_memory_limit, and it uses the lower of that and the JVM value. A limit above a private cgroup namespace stays invisible, which the deployment guide now says. An existing deployment with a mounted or kept application-pd.yml still has total_memory_size: 32000000000, which is explicit and bypasses auto-sizing. The Store now warns when it sees that exact value, and the deployment guide has an upgrade step to set it to 0, remove it, or set HG_STORE_ROCKSDB_TOTAL_MEMORY_SIZE.
/proc/self/cgroup gives the membership path from the hierarchy root, but the cgroupfs can be mounted from a subtree: mountinfo root /kubepods at /sys/fs/cgroup puts the pod's memory.max at /sys/fs/cgroup/pod1, not /sys/fs/cgroup/kubepods/pod1. The resolver looked for the latter, fell back to the unlimited mount root and missed the visible 4 GiB pod limit. Read the cgroup2 mount, or the cgroup v1 mount carrying memory, from /proc/self/mountinfo and strip its root from the membership path before walking ancestors. Without a mountinfo entry the default /sys/fs/cgroup layout is used as before.
imbajin
left a comment
There was a problem hiding this comment.
[P2] Consider compatible readable cgroup mount views instead of returning the first match.
The original mount-root translation finding is fixed at this revision: independent Java 21 synthetic-file checks using the exact resolver, with only Lombok logging stubbed, now find the 4 GiB limit and produce the 512 MiB budget. Fourteen bounded checks pass, but three multi-mount discrepancies remain, confirmed by two independent reviews.
The resolver returns the first matching cgroup mount without considering later readable views. With membership /kubepods/pod1/ctr, an unrelated first mount or a leaf-only first mount followed by a full readable hierarchy can hide a visible 4 GiB ancestor. With a simulated 64 GiB host, 1 GiB heap and 512 MiB direct cap, the budget is then 32,000,000,000 bytes instead of 536,870,912. Reversing mount order succeeds; this reproduces for both v2 and v1.
Please consider compatible readable mount views and their ancestor limits, with order-invariance regressions. Selecting only the most-specific mount can still omit visible ancestors. Multiple mounts are a plausible layout: see the OpenJDK duplicate-mount fix and runc issue #2404.
Evidence boundary: synthetic file fixtures, not real mounts, full Store startup, performance or load measurement. A correct JVM container limit can mask the overbudget path; this is conditional, not a claim that every container is affected. No new numerical score is assigned.
mountinfo can list several mounts of the same controller, and the resolver used the first one. An unrelated subtree or a leaf-only view listed before the full hierarchy hid the pod's visible limit, so the result depended on mount order. Read every cgroup2 mount, or every v1 mount carrying memory, skip those whose root doesn't contain the process's cgroup, walk each remaining one and keep the lowest limit. With no matching mount the default /sys/fs/cgroup layout is used as before.
|
@imbajin thanks, fixed in 535c2d0. The resolver now reads every cgroup2 mount, or every v1 mount carrying |
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Score: 10/10.
Since 33e53ee, 535c2d0 reads every cgroup2 mount, or every v1 mount carrying memory, from mountinfo. It skips mounts whose root does not contain the process cgroup and keeps the lowest limit across the rest. I reran my earlier synthetic case against this head. A leaf-only cgroup2 mount listed before the full hierarchy now gives 4294967296 instead of -1, and the hybrid v2 plus v1 layout still resolves to the v1 limit. I also tried a private namespace with 0::/, a 0::/../sib path, a /.. mount root, the v1 9223372036854771712 sentinel, a nested child cgroup and an escaped mount point. Each gave the expected limit or fell back to the mount root.
Evidence: on a cgroup v2 host (JDK 17.0.20) I ran resolve() inside a transient slice with MemoryMax=4G and an unlimited service, using -Xmx1g -XX:MaxDirectMemorySize=512m. The JVM reported 16473444352, resolve(null) and resolve("0") returned 536870912, and resolve("32000000000") was kept. Locally at 535c2d0, RocksdbMemoryBudgetTest passed 21 tests and CoreSuiteTest passed 62 with 0 failures. CI at this head is still running.
Earlier threads:
- @imbajin, cgroup mount first match: fixed in 535c2d0.
- @imbajin, subtree mount root not rebased: fixed in 33e53ee, still holds.
- @imbajin, entrypoint test not picked by CI policy: fixed in 3fb0da0.
- @imbajin, parent-only limit missed by older JDK 17, and the old application-pd.yml keeping 32 GB: fixed and documented in 03a4d43, with a WARN on 32000000000.
- @imbajin, derived write_buffer_size below 1 MiB: fixed in 1927fbe.
- Mine, unset MaxDirectMemorySize counted as a full heap: fixed in 84e8e27, still holds.
Not covered: a full Store under write load in a memory-limited container.
imbajin
left a comment
There was a problem hiding this comment.
Rechecked the multi-mount finding against the updated source, reviewed 2026-10-16. The prior finding is fixed: all applicable mount views are considered, unrelated roots are excluded, and the minimum visible limit wins rather than the first mount. Two independent source reviews found no new blocker in this repair.
I independently replayed the three earlier counterexamples and added reordered v1/v2 mount and containment controls: 40 bounded Java 21 synthetic-file assertions passed. The executable copy retained the exact resolver/sizing code and replaced only Lombok logging with a no-op logger. This was not a full Store/container load test or a performance measurement.
Provisional assessment: 8.5/10, based on the earlier full review plus this verified repair, not a fresh full Store acceptance. Current-head CI and the pending paired documentation still need completion; the author's 126 Store tests remain author-reported rather than my own execution.
|
@imbajin thanks for the thorough rounds on this one. Everything you raised is now addressed at 535c2d0: the ancestor and subtree cgroup limits, the multi-mount order, the upgrade step for an old |
apache/hugegraph#3355 fills an absent or 0 rocksdb.write_buffer_size with total_memory_size / 1000, but never below the 1 MiB that RocksDBOptions accepts. Any budget under about 1 GB, including the 256 MiB auto-sizing floor, therefore gets 1048576. Say so in the EN and CN rows.
The Docker guide said an unset HG_STORE_ROCKSDB_TOTAL_MEMORY_SIZE sizes the budget from the container memory limit, but the Compose files it documents set no limit, so each Store sizes from the whole Docker host. Say so, and tell readers to set a limit or pin the variable when several Stores share a host. The HStore row now says the cgroup limit applies when one is set, otherwise the host's memory. The HStore page also said its tables were checked at a master commit from before apache/hugegraph#3355; note that the three RocksDB and disruptor rows describe that PR until its merge commit replaces it.
imbajin
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: Automatic sizing can miss the process cgroup limit in a supported cgroup-namespace mount layout and can reserve too little for uncapped direct memory. Either condition can leave the Store with a RocksDB budget above its available container memory. Evidence: exact-head source review of RocksdbMemoryBudget, the default Store launcher and gRPC configuration; see inline comments for the two cases.
| limit = lowest(limit, v2 ? cgroupV2Limit(point, relative) : | ||
| cgroupV1Limit(point, relative)); | ||
| } | ||
| if (!matched) { |
There was a problem hiding this comment.
| * as given, otherwise (0, unset) the heap size capped at UNSET_DIRECT. | ||
| */ | ||
| static long directMemory(long configured, long heap) { | ||
| return configured > 0 ? configured : Math.min(heap, UNSET_DIRECT); |
There was a problem hiding this comment.
imbajin
left a comment
There was a problem hiding this comment.
Updated independent cross-review: 8.0/10, pending focused handling of the namespace mapping case noted in #3355 (comment). I independently reproduced the missing-child-limit case using synthetic proc/mountinfo/cgroup files and the exact resolver source, compiled with only Lombok logging replaced by a no-op logger. Eight new bounded Java 21 assertions passed, and all 40 earlier controls still pass. An inherited mount rooted at /.. with process membership / falls back to the parent mount root and misses the limited child. If the JVM also reports host memory, auto-sizing can overestimate the budget; an accurate JVM-reported limit masks the failure. This is a conditional gap in the new protection, not a demonstrated production OOM or a claim that it regresses the previous fixed default. Please distinguish ambiguous mapping from an unlimited hierarchy and provide narrow safe handling or an explicit-budget diagnostic with a regression fixture. Normalizing /.. alone cannot recover the hidden child path.
The direct-memory finding #3355 (comment) also identifies a real accounting assumption: an independent Java 21 probe with -Xmx2g and unset MaxDirectMemorySize reported a 2 GiB allowance, while this code reserves 1 GiB. This heuristic is explicit in the source. No buffers or Store workload were allocated, so no direct-buffer workload failure or OOM was demonstrated. Counting the default heap-sized allowance is the conservative option; imposing a new runtime direct-memory cap would need separate validation.
Evidence boundary: two independent source reviews, synthetic resolver tests and a lightweight JVM allowance probe; no live namespace, JDK 17, full Store, OOM or performance acceptance. Current review source:
CI status at publication (2026-10-17 01:36 UTC): 19 exact-head checks passed, 2 were skipped, and 0 remain running/queued. Both Codecov checks passed. CI source: https://github.com/apache/hugegraph/actions/runs/38938886440. These reported CI results do not establish coverage of the synthetic namespace-mapping case and are separate from our bounded independent controls.
Purpose of the PR
Both shipped
application-pd.ymlfiles setrocksdb.total_memory_size: 32000000000, and thepdprofile is always included, so theRuntime.maxMemory()fallback inAppConfig.init()never ran.RaftRocksdbOptionssplits the value into theWriteBufferManagercache (66%) and the block cache. Both are nativeLRUCaches, so neither-Xmxnor the container limit bounds them, and a separate 1 GiB raft log cache sits on top. In the run in the issue all three Stores were OOM-killed at a 4 GiB limit once each held about 1 GB of RocksDB data.Docker users had no clean way to change it.
docker-entrypoint.shbuildsSPRING_APPLICATION_JSONfrom fiveHG_STORE_*keys, and none of them is arocksdb.*key. Bind-mounting a config file or passing-Drocksdb.total_memory_size=<bytes>inJAVA_OPTSboth work, but neither is documented.As the issue points out, dropping the line from the yml alone would hand RocksDB a budget equal to the heap, which is a second heap-sized allocation on top of the first.
Main Changes
RocksdbMemoryBudgetinhg-store-core/.../options. An unset, blank or 0 value means auto: half of (physical memory − max heap −MaxDirectMemorySize− 1.5 GiB reserve), clamped to 256 MiB to 32 GB. The reserve covers the raft log cache plus stacks, metaspace and other native memory. Physical memory comes fromOperatingSystemMXBean.getTotalMemorySize(), which on JDK 17 reports the cgroup v1/v2 limit inside a container. Older JDK 17 updates may not see a limit set only on a parent cgroup, so the budget also reads the cgroup itself (v2:memory.maxfrom the process's cgroup up to the mount root; v1:hierarchical_memory_limit) and uses the lower value. The membership path is rebased onto the mount's root from/proc/self/mountinfo, so a cgroupfs mounted from a subtree such as/kubepodsis walked from the right directory. Every mount that contains the process's cgroup is checked and the lowest limit wins, so mount order doesn't matter. Inside a private cgroup namespace a limit above the container can't be seen, and the deployment guide says to pin the value there. An unsetMaxDirectMemorySizeis counted as the heap size capped at 1 GiB. The JVM would allow a full heap of direct memory, but the Store's gRPC/Netty buffers don't grow with the heap, and counting a raised heap twice left large bare-metal hosts on the floor. If physical memory can't be read, the previous heap-size default applies.-Xmx1g,-XX:MaxDirectMemorySize=512m) gets 512 MiB, and an 8 GiB limit gets 2.5 GiB. I checked the 4 GiB case withresolve()insideeclipse-temurin:17-jre-jammyunder a 4 GiB Docker limit: 536870912 bytes. With-Xmx2gand no direct-memory cap it lands on the 256 MiB floor. The 32 GB cap means no host gets a bigger budget than before. On bare metal the default launcher (2 GiB heap) on a 16 GiB host gets 5.75 GiB; a raised heap gets less (2.75 GiB with-Xmx8gon 16 GiB, 12.75 GiB with-Xmx36gon 64 GiB), so a large node that relied on the fixed 32 GB should pin the value after upgrading.1would give a 0-byte manager, which RocksDB treats as no memtable limit at all.4GBand other non-numeric values now fail at startup with "must be a number of bytes", where before they failed with a bareNumberFormatException. Nothing shipped (configs, compose files, Helm, CI) sets a value under 1 MiB.application-pd.ymlwill hit; the Store also warns when it sees exactly32000000000, the old shipped value, and the deployment guide has an upgrade step (set it to 0, remove it, or setHG_STORE_ROCKSDB_TOTAL_MEMORY_SIZE).AppConfig.init()uses it. While there I fixed the raft disruptor sizing for budgets under 1 GB:log(0)rounded to a buffer size of 0. It now gives 32. The formula moved toRocksdbMemoryBudget.disruptorBufferSize()so it has a test. The shippedapplication.ymlpinsdisruptorBufferSize: 1024, so this only matters when that key is absent.application-pd.ymlfiles shiptotal_memory_size: 0, which also removes the [Bug] Store ships rocksdb.total_memory_size of 32 GB, ignores the container memory limit, and Docker users cannot override it #3254 TODO from chore: add TODO breadcrumbs for the Helm chart's server workarounds #3260. The test resources keep their explicit values.docker-entrypoint.shacceptsHG_STORE_ROCKSDB_TOTAL_MEMORY_SIZE. Values must be digits only; anything else exits with code 2. It is added toSPRING_APPLICATION_JSONonly when set, so an unset variable leaves the config file or auto-sizing in charge. When set it outranks both the config file and-Drocksdb.total_memory_size(checked against Spring Boot 2.5's property order).helm/hugegraph/values-cluster.yaml: the store comment said container limits don't configure the RocksDB budget. It now says how the budget is derived (2.5 GiB for that preset) and how to pin it withstore.extraEnv.In a small container the write buffer manager now flushes memtables much earlier than before, which means more small L0 files and lower write throughput in exchange for staying inside the limit. Both caps are soft, so RocksDB can still go over briefly under heavy writes. I have not measured either effect. Setting the value explicitly restores the old behaviour.
Verifying these changes
RocksdbMemoryBudgetTest(21 tests, added toCoreSuiteTest, which thestore-core-testprofile runs). It covers:0and00going to auto;4GB,-1,1.5e9and a value pastLong.MAX_VALUE;write_buffer_sizestaying at or above RocksDB's 1 MiB minimum, checked through the sameHugeConfigvalidation as startup;/proc/self/cgroup.test-store-docker-entrypoint.sh(9 cases), added to the store job inpd-store-ci.yml. It checks that therocksdbkey is absent when the variable is unset or empty, that bytes and0pass through, thatapp.data-pathis still set, and that4GB,-1,1.5e9,1 2and1"}are refused. It clears the variable from the caller's environment first, so "unset" really is unset.mvn test -pl hugegraph-store/hg-store-test -P store-core-teston JDK 17: 126 tests, 0 failures.python3 -m unittest discover -s .github/scripts -p 'test_*.py'passes.helm unittest helm/hugegraph: 294 tests pass.mvn editorconfig:checkandgit diff --checkare clean.test-store-docker-entrypoint.shis mapped to thestoresuite in.github/scripts/ci-policy.py, so a PR that changes only that script still runs it; the selector test covers the mapping.Does this PR potentially affect the following parts?
Documentation Status
Doc - TODO: required documentation is pending; complete it before merging.Doc - Done: documentation is included here or linked below.Doc - No Need: no user-visible documentation is affected.Documentation files in this PR or paired hugegraph-doc PR:
hugegraph-store/docs/deployment-guide.md: the auto-sizing rule and its inputs, the 1 MiB minimum, the new env var and its precedence in the compose and Kubernetes examples, and the RocksDB block (total_memory_size: 0; the nonexistentblock_cache_sizekey replaced withwrite_buffer_ratio).hugegraph-store/README.md: the env var in thedocker runsection.content/{en,cn}/docs/quickstart/hugegraph/hugegraph-hstore.md(the default, the auto-sizing rule and the env var) andcontent/{en,cn}/docs/guides/hugegraph-docker-cluster.md(the newHG_STORE_*row). The hstore page's "checked against Server master commit" note should move to this PR's merge commit once both land.Conceptual overview
Concept illustration: derive the RocksDB budget from memory remaining after heap, direct memory and reserve, with clamp bounds and a 1 MiB derived write-buffer floor. Cache budgets remain soft.