byteayan opened a new pull request, #3432:
URL: https://github.com/apache/hugegraph/pull/3432
## Purpose of the PR
- related to #3254, follow-up to #3355
This fixes two gaps in the memory budget code from #3355 that bitflicker64
listed as unverified in their review.
- `CgroupMemoryLimit.read` calls `Paths.get` and `resolve` on mount points
outside any catch. If a mountinfo path unescapes to something the JVM can't
represent, such as a NUL from a `\000` escape, the resulting
`InvalidPathException` reaches `AppConfig.init` and the Store fails to start.
The kernel doesn't write `\000`, so only a broken or hand-made mountinfo can
trigger it, but one bad line shouldn't stop the Store.
- `PdConfigureListener` drops PD's `rocksdb.total_memory_size` only when
`SPRING_APPLICATION_JSON` or a `-D` system property sets it. Spring ranks a
`--rocksdb.total_memory_size=...` argument to the Store jar above both, but
PD's copy still overrode it.
## Main Changes
- `CgroupMemoryLimit.read`: the lookup for each `/proc/self/cgroup` line now
catches `InvalidPathException`. That line's limits are treated as hidden and
the view is flagged incomplete, so the Store logs the existing "can't see the
whole hierarchy" warning instead of failing. Other lines still count.
- `PdConfigureListener`: `commandLineArgs` joins `spring.application.json`
and `systemProperties` as a source that drops PD's budget key.
- `systemEnvironment` stays out on purpose. Spring binds
`ROCKSDB_TOTAL_MEMORY_SIZE` to the `total.memory.size` key of the `rocksdb`
map, not `total_memory_size`, so it never sets the budget. Relaxed lookup still
makes `systemEnvironment.getProperty("rocksdb.total_memory_size")` return it,
so adding that source would drop PD's value while the variable did nothing, and
the Store would fall back to its local file.
`testEnvironmentVariableKeepsPdConfig` covers this.
- `hugegraph-store/docs/deployment-guide.md`: the upgrade section says a
command-line argument works like `-D`, and that an environment variable such as
`ROCKSDB_TOTAL_MEMORY_SIZE` doesn't set the budget.
## Verifying these changes
- [ ] Trivial rework / code cleanup without any test coverage. (No Need)
- [ ] Already covered by existing tests, such as *(please modify tests
here)*.
- [x] Need tests and can be verified as follows:
-
`CgroupMemoryLimitTest.testUnrepresentableMountPointIsFlaggedNotFatal`: a
hybrid layout where the v2 mount point holds a `\000` and a v1 memory line has
a 4 GiB limit. It reads 4 GiB from v1 and flags the view incomplete. On master
it throws `InvalidPathException`.
-
`PdConfigureListenerMemoryOverrideTest.testCommandLineOverrideWinsOverPdConfig`:
a command-line budget beats PD's copy, and also `SPRING_APPLICATION_JSON`. On
master PD's `32000000000` wins.
-
`PdConfigureListenerMemoryOverrideTest.testEnvironmentVariableKeepsPdConfig`:
`ROCKSDB_TOTAL_MEMORY_SIZE` leaves PD's value in place and binds as
`total.memory.size`.
- `mvn test -pl hugegraph-store/hg-store-test -P store-core-test`: 221
tests, 0 failures, 1 skipped. `mvn editorconfig:check` passes.
## 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))
- [ ] Modify configurations
- [ ] The public API
- [x] Other affects (typed here): a `--rocksdb.total_memory_size` argument
now outranks PD's shared `application-pd.yml`, as `-D` already does
- [ ] Nope
## Documentation Status
- [ ] `Doc - TODO`: required documentation is pending; complete it before
merging.
- [x] `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`
--
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]
