byteayan opened a new pull request, #3355:
URL: https://github.com/apache/hugegraph/pull/3355
## Purpose of the PR
- close #3254
Both shipped `application-pd.yml` files set `rocksdb.total_memory_size:
32000000000`, and the `pd` profile is always included, so the
`Runtime.maxMemory()` fallback in `AppConfig.init()` never ran.
`RaftRocksdbOptions` splits the value into the `WriteBufferManager` cache (66%)
and the block cache. Both are native `LRUCache`s, so neither `-Xmx` nor 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.sh` builds
`SPRING_APPLICATION_JSON` from five `HG_STORE_*` keys, and none of them is a
`rocksdb.*` key. Bind-mounting a config file or passing
`-Drocksdb.total_memory_size=<bytes>` in `JAVA_OPTS` both 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
- New `RocksdbMemoryBudget` in `hg-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 from
`OperatingSystemMXBean.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; those get host memory, as before this change. An
unset `MaxDirectMemorySize` is counted as the heap size, since that is what the
JVM allows. If physical memory can't be read, the previous heap-size default
applies.
- The issue's setup (4 GiB limit, `-Xmx1g`,
`-XX:MaxDirectMemorySize=512m`) gets 512 MiB, and an 8 GiB limit gets 2.5 GiB.
I checked the 4 GiB case with `resolve()` inside `eclipse-temurin:17-jre-jammy`
under a 4 GiB Docker limit: 536870912 bytes. With `-Xmx2g` and no direct-memory
cap it lands on the 256 MiB floor. The 32 GB cap means no host gets a bigger
budget than before.
- A positive value is used unchanged, with one exception: values under 1
MiB are refused. The write buffer manager gets 2/3 of the budget, so `1` would
give a 0-byte manager, which RocksDB treats as no memtable limit at all. `4GB`
and other non-numeric values now fail at startup with "must be a number of
bytes", where before they failed with a bare `NumberFormatException`. Nothing
shipped (configs, compose files, Helm, CI) sets a value under 1 MiB.
- Startup logs the chosen value and its inputs. It warns when the 256 MiB
floor applies (heap plus direct memory already fill the limit), when an
explicit value is under 256 MiB, and when an explicit value exceeds the memory
left after the heap, direct memory and the reserve. That last case is what an
upgraded install with an old mounted `application-pd.yml` will hit.
- `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 to `RocksdbMemoryBudget.disruptorBufferSize()` so it has
a test. The shipped `application.yml` pins `disruptorBufferSize: 1024`, so this
only matters when that key is absent.
- Both `application-pd.yml` files ship `total_memory_size: 0`, which also
removes the #3254 TODO from #3260. The test resources keep their explicit
values.
- `docker-entrypoint.sh` accepts `HG_STORE_ROCKSDB_TOTAL_MEMORY_SIZE`.
Values must be digits only; anything else exits with code 2. It is added to
`SPRING_APPLICATION_JSON` only 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 with `store.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
- [x] Need tests and can be verified as follows:
- New `RocksdbMemoryBudgetTest` (8 tests, added to `CoreSuiteTest`,
which the `store-core-test` profile runs). It covers:
- explicit values kept unchanged, and values under 1 MiB refused;
- unset, blank, `0` and `00` going to auto;
- refusal of `4GB`, `-1`, `1.5e9` and a value past `Long.MAX_VALUE`;
- the 4 GiB and 8 GiB examples above, both clamp bounds, and the
unknown-physical fallback;
- the disruptor size for 512 MiB, 1 GB and 32 GB.
- New `test-store-docker-entrypoint.sh` (9 cases), added to the store
job in `pd-store-ci.yml`. It checks that the `rocksdb` key is absent when the
variable is unset or empty, that bytes and `0` pass through, that
`app.data-path` is still set, and that `4GB`, `-1`, `1.5e9`, `1 2` and `1"}`
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-test` on JDK
17: 113 tests, 0 failures. `helm unittest helm/hugegraph`: 294 tests pass. `mvn
editorconfig:check` and `git diff --check` are clean.
- Not tested: a full Store started in a memory-limited container, and
the load from the issue.
- CI selection gap, left as is: a PR that changes only
`test-store-docker-entrypoint.sh` doesn't select the store job, because the
script isn't in `CONTRACT_SUITES` in `.github/scripts/ci-policy.py`. Changing
the entrypoint itself does select it. The PD entrypoint test has the same gap,
so I'd rather fix both in one place.
## Does this PR potentially affect the following parts?
- [ ] Dependencies ([add/update
license](https://hugegraph.apache.org/docs/contribution-guidelines/contribute/#321-check-licenses)
info &
[regenerate_known_dependencies.sh](../install-dist/scripts/dependency/regenerate_known_dependencies.sh))
- [x] Modify configurations
- [ ] The public API
- [ ] Other affects (typed here)
- [ ] Nope
## Documentation Status
- [x] `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
nonexistent `block_cache_size` key replaced with `write_buffer_ratio`).
- `hugegraph-store/README.md`: the env var in the `docker run` section.
- Paired apache/hugegraph-doc PR, link to follow:
`content/{en,cn}/docs/quickstart/hugegraph/hugegraph-hstore.md` (the default,
the auto-sizing rule and the env var) and
`content/{en,cn}/docs/guides/hugegraph-docker-cluster.md` (the new `HG_STORE_*`
row). The hstore page's "checked against Server master commit" note should move
to this PR's merge commit once both land.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
