feat(server): support RocksDB on RISC-V#3102
Conversation
b5da4f4 to
25a16ff
Compare
25a16ff to
b9d9d8c
Compare
imbajin
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: The new multi-architecture gate cannot start under the repository policy, and the native/runtime validation paths retain correctness and liveness gaps. Evidence: exact-head workflow run 29841863669 ended startup_failure with zero jobs; static inspection of the changed workflow, test, and smoke scripts.
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## master #3102 +/- ##
============================================
- Coverage 37.43% 30.89% -6.54%
+ Complexity 520 329 -191
============================================
Files 808 817 +9
Lines 69581 69848 +267
Branches 9165 9187 +22
============================================
- Hits 26048 21582 -4466
- Misses 40750 45802 +5052
+ Partials 2783 2464 -319 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
imbajin
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The current head fixes the earlier RISC-V validation gaps and all visible checks pass, but the new cleanup path cannot detect a timed-out server shutdown. Evidence: exact-head shell control-flow inspection and Bash exit-status reproduction.
| trap - EXIT | ||
|
|
||
| if [[ "$SERVER_START_ATTEMPTED" == "true" ]]; then | ||
| if ! "$TRAVIS_DIR"/stop-server.sh "$SERVER_DIR"; then |
There was a problem hiding this comment.
This check cannot currently detect a shutdown timeout. stop-server.sh delegates to stop-hugegraph.sh, whose last command is an if kill_process_and_wait ...; then rm ...; fi; when that condition is false, Bash returns status 0 for the if because there is no else. The cleanup can therefore report success while the server and stale PID file remain. Please make stop-hugegraph.sh explicitly return nonzero when kill_process_and_wait fails, and remove the PID file only on success, so this new EXIT cleanup can enforce the result it checks.
There was a problem hiding this comment.
Fixed in 3e6f4d8d. stop-hugegraph.sh now exits 1 when kill_process_and_wait fails and removes the PID file only after a successful shutdown, so stop-server.sh and the EXIT cleanup receive the real status. A real timeout check with a SIGTERM-ignoring process returned 1 and retained the PID file; the normal path returned 0, stopped the process, and removed the PID file. On the current head, Linux RocksDB/memory API jobs, both macOS jobs, and the full runtime matrix have passed in org CI.
There was a problem hiding this comment.
Follow-up on the exact current head bd69efb: the org RISC-V job initially failed precisely because stop-hugegraph.sh propagated the 10-second shutdown failure. Logs showed the Java shutdown hooks completed but the sleep-as-PID-1 container retained the exited daemon as a zombie. The CI container now uses Docker --init to reap it; the product timeout and failure propagation remain unchanged. Exact-head jobs 89152468612 and 89155333088 both then passed both shutdowns and cleanup. This confirms the failure is not swallowed.
|
All six review findings have now been addressed in Current-head validation in hugegraph/hugegraph PR #164 has passed the RISC-V QEMU runtime and native Maven gate, amd64/arm64 runtime, Linux RocksDB/memory, both macOS RocksDB jobs, PD/Store/HStore, CodeQL, licensing, dependency, and Docker checks. Cluster passed on the repository's automatic second attempt. HBase is still in its pre-existing backend environment preparation step, so I am not reporting it as passed yet. The seven Apache workflows for head |
|
Update: current-head validation in hugegraph/hugegraph#164 is now complete. All 24 checks passed, including HBase compile/unit/core/API, the RISC-V QEMU runtime and native Maven gate, amd64/arm64 runtime, Linux RocksDB/memory, both macOS jobs, PD/Store/HStore, and Cluster after the repository's automatic retry. The Apache workflows for the same head remain |
imbajin
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: The RISC-V path is substantially improved, but the default Maven invocation now silently drops the existing Core test and memory-backend profiles; the architecture smoke trigger also misses RocksDB source-only changes. Evidence: exact-head Maven active-profile output, workflow path inspection, shell syntax checks, and rocksdb-only reactor validation.
| <id>all-backends</id> | ||
| <activation> | ||
| <property> | ||
| <name>!rocksdb-only</name> |
There was a problem hiding this comment.
This property-activated all-backends profile is active whenever rocksdb-only is absent. Maven deactivates an activeByDefault profile when another profile in the same POM activates, so the normal invocation no longer activates core-test; the analogous new profile in hugegraph-test/pom.xml also disables the default memory profile. On this exact head, mvn help:active-profiles -pl hugegraph-server/hugegraph-test lists both all-backends profiles but neither core-test nor memory, while -Drocksdb-only restores them. Ordinary default builds can therefore skip the established Core Surefire execution and lose the filtered memory backend configuration. Please restructure the full-backend selection so default builds retain core-test and memory, and add a regression check for the default active-profile/test set.
There was a problem hiding this comment.
Fixed on the current head. The new profiles in hugegraph-server/pom.xml and hugegraph-test/pom.xml were removed, so the existing activeByDefault core-test and memory profiles are no longer displaced. The default invocation was rechecked with Maven and still activates memory, core-test, and the complete module set; current-head org memory, RocksDB, and both macOS Server jobs have also passed.
| - 'hugegraph-pd/hg-pd-grpc/pom.xml' | ||
| - 'hugegraph-server/pom.xml' | ||
| - 'hugegraph-server/hugegraph-core/pom.xml' | ||
| - 'hugegraph-server/hugegraph-rocksdb/pom.xml' |
There was a problem hiding this comment.
The new runtime matrix builds and executes the RocksDB/JNI module, but this path filter matches only its pom.xml. A PR that changes hugegraph-server/hugegraph-rocksdb/src/** without touching one of the listed infrastructure files will not schedule the amd64/arm64/riscv64 smoke, leaving native-loading and architecture regressions untested. Please cover the module path (for example, hugegraph-server/hugegraph-rocksdb/**) or otherwise add an equivalent trigger for the runtime code exercised by this job.
There was a problem hiding this comment.
Superseded by the maintainer-requested scope reduction. The Docker runtime matrix and its path filter were removed. RISC-V validation now runs as an independent reusable job invoked from the existing server-ci.yml, so every Server CI pull_request event, including hugegraph-rocksdb source-only changes, schedules the RISC-V build and runtime gate. Exact-head org job 89155333088 passed the clean build, JNI, REST/Gremlin, restart persistence, shutdown, and cleanup flow.
Restore the existing Docker and default test paths unchanged. Run the native RocksDB-only build and runtime smoke in a dedicated QEMU job.
7f38296 to
6227b2b
Compare
|
Current head Exact-head validation after the final reduction:
Two org checks expose existing failures outside this PR's diff:
The six Apache workflow suites for exact head |
imbajin
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: The main RISC-V server path is covered, but the distribution's store-dump JVM still opens RocksDB without the required libatomic preload. Evidence: exact-head call-site inspection of util.sh, dump-store.sh, and StoreDumper.java; bash/action workflow static validation.
| fi | ||
| } | ||
|
|
||
| function configure_riscv64_libatomic() { |
There was a problem hiding this comment.
This helper is invoked by hugegraph-server.sh and init-store.sh, but dump-store.sh still launches StoreDumper directly without sourcing util.sh. StoreDumper constructs the graph with HugeFactory.open(conf), so a RocksDB-backed dump on RISC-V starts a separate JVM without the LD_PRELOAD required by this change and can fail at native library loading even though server startup succeeds. Please invoke this helper before the dumper JVM (and audit any other standalone RocksDB-opening launchers), then cover the store-dump path in the native RISC-V smoke test.
Purpose of the PR
linux/riscv64build and minimum runtimesupport tracked by [Task] Support HugeGraph with RocksDB on RISC-V #3099.
build, and test paths used by other architectures.
Main Changes
-Drocksdb-onlydistribution selection. CI uses Maven's existing-pl ... -amproject selection so unrelated storage implementations are not built orpackaged; the default reactor and distribution remain unchanged.
protocandgrpc_java_pluginexecutables only in the automatically activated RISC-V profiles.libatomic.so.1before HugeGraph initializes orstarts on
linux/riscv64, fixing the unresolved RocksDB JNI atomic symbol.and graph data, Gremlin, shutdown, restart, and persistence.
a clean native RISC-V Maven build and the runtime smoke under QEMU with a
checksum-pinned Alibaba Dragonwell 11 Extended Server VM.
the official HugeGraph Dockerfile and published image are not changed by this PR.
Scope Boundaries
Validation
remained in the normal distribution.
from clean source; protobuf and gRPC Java sources were regenerated with the documented
system tools.
put/get/reopen, REST schema/vertices/edge, Gremlin, both shutdowns, restart
persistence, and final process/port cleanup.
0 failures and 0 errors (41 skipped), and the API suite passed 161 tests with
0 failures and 0 errors (14 skipped) with a valid JaCoCo XML report. These suites are
supporting evidence, not represented as exact-head hosted CI.
distribution-boundary, and
git diff --checkvalidation passed.89169639441
passed the 16-project clean build, distribution boundary, JNI, REST/Gremlin, restart
persistence, shutdown, and container cleanup under QEMU in 23m17s.
Compatibility
11 jobs unchanged; the RISC-V workflow is an additional isolated job.
memoryandcore-testprofiles and retains the complete module set.
installed
libatomic.so.1.Residual Risks
packaged RocksDB JNI ELF requirements.
load or performance.