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.
|
|
||
| - name: Set up QEMU | ||
| if: matrix.arch != 'amd64' | ||
| uses: docker/setup-qemu-action@v3 |
There was a problem hiding this comment.
At this exact head, Docker Build CI run 29841863669 ended with startup_failure and zero jobs; its annotation reports that docker/setup-qemu-action@v3 and docker/setup-buildx-action@v3 are forbidden by the repository's Actions policy. Consequently none of the added amd64/arm64/riscv64 build and runtime smoke jobs execute. Please replace both setup actions with organization-allowed, commit-pinned actions or an allowed shell-based setup, or obtain explicit allowlisting, then require the complete matrix to pass.
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v3 | ||
|
|
||
| - name: Build HugeGraph Server for linux/${{ matrix.arch }} |
There was a problem hiding this comment.
The Dockerfile runs Maven in a $BUILDPLATFORM stage, so on the amd64 runner the new arch=riscv64 profiles in hugegraph-core and hg-pd-grpc never activate. The runtime smoke can validate the packaged JNI library while the documented native path through /usr/bin/protoc, grpc_java_plugin, and the profile dependency override remains broken. Please add a native/QEMU RISC-V Maven gate, or install the system protobuf tools and explicitly build with -P riscv64-protobuf-tools -Drocksdb-only so these profiles are covered.
| TimeUnit.SECONDS.toNanos(timeoutSeconds); | ||
| do { | ||
| HugeTask<Object> task = scheduler.task(id); | ||
| if (task.progress() >= expectedProgress) { |
There was a problem hiding this comment.
TaskAndResultScheduler.task() reloads persisted state, while TaskCallable.updateProgress() saves only after its 30-second default interval; this script normally finishes in about 2.2 seconds. Completion persists progress 10, and because this condition is checked before task.completed(), the helper can return and the test then tries to cancel a completed task, contradicting the expected CANCELLING/CANCELLED state. Please make intermediate progress observable (for example, set a short save interval in the job or use explicit synchronization) and reject completion before accepting the threshold.
| exit 1 | ||
| fi | ||
|
|
||
| JAVA_VERSION=$(java -version 2>&1 | awk -F '"' '/version/ {print $2; exit}') |
There was a problem hiding this comment.
JAVA_HOME
The native instructions select Dragonwell via JAVA_HOME, and the server launcher honors that setting, but this script invokes bare java here, again for properties, and for the RocksDB JNI process. If another JVM precedes Dragonwell in PATH, the smoke test either fails spuriously or validates a different runtime from the server. Please resolve a single Java executable up front, preferring $JAVA_HOME/bin/java when set, verify it is executable, and use it for all three invocations.
| wait_for_server() { | ||
| local attempt | ||
| for attempt in $(seq 1 240); do | ||
| if curl_request --silent --show-error --fail \ |
There was a problem hiding this comment.
curl_request supplies neither --connect-timeout nor --max-time. Curl's total request time is unlimited by default, so one accepted-but-stalled connection can prevent this 240-iteration loop from advancing; the normal API request helper has the same issue. This can consume the full 45-minute job timeout and delay cleanup and diagnostics. Please add explicit connection and total request timeouts to readiness probes and normal requests, sized separately if mutations need a longer bound.
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## master #3102 +/- ##
============================================
- Coverage 37.43% 30.49% -6.94%
+ Complexity 520 374 -146
============================================
Files 808 808
Lines 69581 69581
Branches 9165 9165
============================================
- Hits 26048 21221 -4827
- Misses 40750 45936 +5186
+ Partials 2783 2424 -359 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Purpose of the PR
Main Changes
eclipse-temurin:11-jre-jammy.libatomic.so.1before RocksDB JNI is used on RISC-V.protoc/grpc_java_pluginin automatically activated RISC-V Maven profiles.-Drocksdb-onlyreactor, distribution registry, and backend-boundary check. Docker and test helpers select it only for RISC-V; existing architectures retain the default full build.hugegraph-server/README.md.Verifying these changes
git diff --checkpassed.Does this PR potentially affect the following parts?
Documentation Status
Doc - TODODoc - DoneDoc - No NeedResidual Risks