imbajin commented on code in PR #3102: URL: https://github.com/apache/hugegraph/pull/3102#discussion_r3625127178
########## hugegraph-server/hugegraph-dist/src/assembly/travis/run-rocksdb-jni-smoke-test.sh: ########## @@ -0,0 +1,119 @@ +#!/bin/bash +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +set -euo pipefail + +if [[ $# -ne 1 ]]; then + echo "USAGE: $0 SERVER_DIR" >&2 + exit 1 +fi + +TRAVIS_DIR=$(cd "$(dirname "$0")" && pwd) +SERVER_DIR=$(cd "$1" && pwd) +EXPECTED_ARCH=${EXPECTED_ARCH:-} +EXPECTED_JAVA_MAJOR=${EXPECTED_JAVA_MAJOR:-11} +ACTUAL_ARCH=$(uname -m) + +if [[ -n "$EXPECTED_ARCH" && "$ACTUAL_ARCH" != "$EXPECTED_ARCH" ]]; then + echo "Expected architecture $EXPECTED_ARCH, got $ACTUAL_ARCH" >&2 + exit 1 +fi + +JAVA_VERSION=$(java -version 2>&1 | awk -F '"' '/version/ {print $2; exit}') Review Comment: ⚠️ **Important: the native smoke test ignores the selected `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. ########## hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/TaskCoreTest.java: ########## @@ -112,6 +114,29 @@ private static void waitUntilTaskRunning(TaskScheduler scheduler) { Assert.fail("Timed out waiting for task to start running"); } + private static void waitUntilTaskProgress(TaskScheduler scheduler, Id id, + int expectedProgress, + long timeoutSeconds) { + long deadline = System.nanoTime() + + TimeUnit.SECONDS.toNanos(timeoutSeconds); + do { + HugeTask<Object> task = scheduler.task(id); + if (task.progress() >= expectedProgress) { Review Comment: ⚠️ **Important: progress polling can accept an already-completed task** `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. ########## .github/workflows/docker-build-ci.yml: ########## @@ -50,3 +61,43 @@ jobs: HC=$(docker inspect --format='{{json .Config.Healthcheck}}' "$IMAGE_ID") echo "Healthcheck: $HC" [[ "$HC" != "null" ]] || { echo "ERROR: HEALTHCHECK missing in ${{ matrix.dockerfile }}"; exit 1; } + + server-runtime-smoke: + runs-on: ubuntu-24.04 + timeout-minutes: 45 + strategy: + fail-fast: false + matrix: + arch: [amd64, arm64, riscv64] + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up QEMU + if: matrix.arch != 'amd64' + uses: docker/setup-qemu-action@v3 Review Comment: ‼️ **Critical: the new multi-architecture validation cannot start** 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. ########## .github/workflows/docker-build-ci.yml: ########## @@ -50,3 +61,43 @@ jobs: HC=$(docker inspect --format='{{json .Config.Healthcheck}}' "$IMAGE_ID") echo "Healthcheck: $HC" [[ "$HC" != "null" ]] || { echo "ERROR: HEALTHCHECK missing in ${{ matrix.dockerfile }}"; exit 1; } + + server-runtime-smoke: + runs-on: ubuntu-24.04 + timeout-minutes: 45 + strategy: + fail-fast: false + matrix: + arch: [amd64, arm64, riscv64] + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up QEMU + if: matrix.arch != 'amd64' + uses: docker/setup-qemu-action@v3 + with: + platforms: ${{ matrix.arch }} + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v3 + + - name: Build HugeGraph Server for linux/${{ matrix.arch }} Review Comment: ⚠️ **Important: this matrix does not exercise the native RISC-V Maven profiles** 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. ########## hugegraph-server/hugegraph-dist/src/assembly/travis/run-server-e2e-smoke-test.sh: ########## @@ -0,0 +1,183 @@ +#!/bin/bash +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +set -euo pipefail + +if [[ $# -ne 3 || ( "$2" != "create" && "$2" != "verify" ) ]]; then + echo "USAGE: $0 SERVER_URL create|verify RUN_ID" >&2 + exit 1 +fi + +SERVER_URL=${1%/} +MODE=$2 +RUN_ID=$3 + +if [[ ! "$RUN_ID" =~ ^[a-zA-Z0-9_]+$ ]]; then + echo "RUN_ID must contain only letters, numbers, and underscores" >&2 + exit 1 +fi + +GRAPH_PATH=/graphspaces/DEFAULT/graphs/hugegraph +WORK_DIR=$(mktemp -d "${TMPDIR:-/tmp}/hugegraph-server-smoke.XXXXXX") +RESPONSE_FILE="$WORK_DIR/response.json" +PROPERTY_KEY="riscv_smoke_name_$RUN_ID" +VERTEX_LABEL="riscv_smoke_node_$RUN_ID" +EDGE_LABEL="riscv_smoke_link_$RUN_ID" +VERTEX_ONE="riscv-smoke-v1-$RUN_ID" +VERTEX_TWO="riscv-smoke-v2-$RUN_ID" + +cleanup() { + rm -rf "$WORK_DIR" +} +trap cleanup EXIT + +for command in curl jq; do + if ! command -v "$command" >/dev/null 2>&1; then + echo "Required command is unavailable: $command" >&2 + exit 1 + fi +done + +if [[ -n "${HUGEGRAPH_USERNAME:-}" || -n "${HUGEGRAPH_PASSWORD:-}" ]]; then + if [[ -z "${HUGEGRAPH_USERNAME:-}" || -z "${HUGEGRAPH_PASSWORD:-}" ]]; then + echo "Set both HUGEGRAPH_USERNAME and HUGEGRAPH_PASSWORD" >&2 + exit 1 + fi +fi + +curl_request() { + if [[ -n "${HUGEGRAPH_USERNAME:-}" ]]; then + curl --compressed --user "$HUGEGRAPH_USERNAME:$HUGEGRAPH_PASSWORD" "$@" + else + curl --compressed "$@" + fi +} + +wait_for_server() { + local attempt + for attempt in $(seq 1 240); do + if curl_request --silent --show-error --fail \ Review Comment: ⚠️ **Important: one stalled request can defeat the bounded readiness loop** `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. -- 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]
