bitflicker64 commented on PR #3355: URL: https://github.com/apache/hugegraph/pull/3355#issuecomment-6465942718
### Review summary for head `7fdd54ab` — score 10/10, no confirmed defects **Coverage:** this round (R2) is a delta review of the merge commit `7fdd54ab` (parents `1dcabb74` and master `97ecd5e2`). It is not a new full review. The baseline is a full R1 review at base `1216e914` / head `1dcabb74`, which covered all 24 files and found no defects. The delta work checked: - **The merge itself.** `git merge-tree 1dcabb74 97ecd5e2` reproduces the head tree, so there was no manual conflict resolution. The PR's added and removed lines are the same as at `1dcabb74`. - **Files the new base also changed:** `AppConfig.java`, `CoreSuiteTest.java`, `values-cluster.yaml` and the Store docs. Base and PR changes don't overlap, and the 2.5 GiB budget comment in `values-cluster.yaml` still matches the numbers. - **Non-root images (#3381).** The budget code reads only world-readable `/proc/self/*` and cgroup files. #3381 doesn't touch the entrypoint or the launcher. - **The segment log storage from #3420.** `RaftSegmentLogStorage` extends `RocksDBSegmentLogStorage`, and JRaft still looks up options by `RocksDBLogStorage.class`, so the shared write-buffer manager still applies. **Reviewers:** two independent reviewers covered both rounds, Codex (GPT-6) and Claude (`claude-opus-5-5`). Copilot's R1 report is supporting evidence only and doesn't count toward the score. An AGY reviewer is still pending. If a later report changes this result, I'll edit this comment. **Checks run at `7fdd54ab`:** - The 5 new Store test classes plus `PartitionLogStorageTest`: 45 tests, 0 failures - `test-store-docker-entrypoint.sh`: 11 passed - `test-start-hugegraph-store.sh --gc-only`: passed - CI policy tests: 66 passed **Not verified, so not scored:** - Raft-log memtables, previously uncapped per partition, now share one 256 MiB `WriteBufferManager`. A Store with many partitions may flush the raft log more often. This has not been benchmarked. - `PdConfigureListener.MEMORY_OVERRIDE_SOURCES` doesn't check `systemEnvironment` or `commandLineArgs`. The shipped launcher passes no program arguments. - `CgroupMemoryLimit` doesn't catch `InvalidPathException` from a malformed `/proc/self/mountinfo` escape. **Limits:** no full CoreSuite run, no live cgroup or container run, no image build, and no Helm install. -- 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]
