imbajin commented on code in PR #3164: URL: https://github.com/apache/hugegraph/pull/3164#discussion_r3816134782
########## docker/test/test-snapshot-corruption.sh: ########## @@ -0,0 +1,311 @@ +#!/usr/bin/env 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. + + +# test-snapshot-corruption.sh — deterministic reproducer for the HStore snapshot corruption bug +# +# Requires: Docker Desktop >= 20.10, >= 12 GB allocated to Docker, Docker Compose v2 +# Run from the repo root: +# bash docker/hbase/test/test-snapshot-corruption.sh # confirm bug is present (buggy image) +# bash docker/hbase/test/test-snapshot-corruption.sh --fixed # confirm bug is absent (fixed image) + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" +COMPOSE_FILE="$SCRIPT_DIR/../docker-compose-3pd-3store-3server.yml" +HUGEGRAPH_VERSION="${HUGEGRAPH_VERSION:-1.7.0}" +VOLUME_PREFIX="hugegraph-3x3" +STORE_LOG="hugegraph-store.log" +FIXED_MODE=false +[[ "${1:-}" == "--fixed" ]] && FIXED_MODE=true + +RED='\033[0;31m'; GREEN='\033[0;32m'; YELLOW='\033[1;33m'; NC='\033[0m' +log() { echo -e "${GREEN}[repro]${NC} $*"; } +warn() { echo -e "${YELLOW}[repro]${NC} $*"; } +fail() { echo -e "${RED}[repro] FAIL${NC} $*" >&2; exit 1; } + +JAR_SOURCE="$REPO_ROOT/hugegraph-store/hg-store-node/target/hg-store-node-${HUGEGRAPH_VERSION}.jar" + +if $FIXED_MODE; then + STORE_IMAGE="${STORE_IMAGE:-hugegraph/store:patched}" + if [[ "$STORE_IMAGE" == "hugegraph/store:patched" ]] && \ + ! docker image inspect "hugegraph/store:patched" >/dev/null 2>&1; then + log "Patched image not found — building from source..." + if [[ ! -f "$JAR_SOURCE" ]]; then + log " Compiling hugegraph-store (this takes a minute)..." + mvn package -pl hugegraph-store/hg-store-node -am -DskipTests -q \ + -f "$REPO_ROOT/pom.xml" + fi + BUILD_CTX="$(mktemp -d)" + cp "$JAR_SOURCE" "$BUILD_CTX/hg-store-node-${HUGEGRAPH_VERSION}.jar" + cat > "$BUILD_CTX/Dockerfile" <<EOF +FROM hugegraph/store:${HUGEGRAPH_VERSION} +COPY hg-store-node-${HUGEGRAPH_VERSION}.jar /hugegraph-store/lib/hg-store-node-${HUGEGRAPH_VERSION}.jar +EOF + docker build -t "hugegraph/store:patched" "$BUILD_CTX" >/dev/null + log " Built hugegraph/store:patched." + fi +else + STORE_IMAGE="${STORE_IMAGE:-hugegraph/store:${HUGEGRAPH_VERSION}}" +fi + +log "Store image: $STORE_IMAGE (fixed-mode: $FIXED_MODE)" +log "Compose File: $COMPOSE_FILE" + +# If a non-default store image is requested, write a temporary compose override that +# replaces the store image — without modifying the committed compose file. +OVERRIDE_FILE="" +if [ "$STORE_IMAGE" != "hugegraph/store:${HUGEGRAPH_VERSION}" ]; then + OVERRIDE_FILE="$(mktemp /tmp/snapshot-test-override-XXXXXX.yml)" + cat > "$OVERRIDE_FILE" <<EOF +services: + store0: + image: ${STORE_IMAGE} + store1: + image: ${STORE_IMAGE} + store2: + image: ${STORE_IMAGE} +EOF + log " Using override file $OVERRIDE_FILE for image $STORE_IMAGE" +fi +# Build the compose -f arguments (base file always first; override appended when set) +COMPOSE_ARGS="-f $COMPOSE_FILE" +[ -n "$OVERRIDE_FILE" ] && COMPOSE_ARGS="$COMPOSE_ARGS -f $OVERRIDE_FILE" +# Clean up temp files on exit (BUILD_CTX trap already set in --fixed mode above) +trap '[ -n "$OVERRIDE_FILE" ] && rm -f "$OVERRIDE_FILE"; [ -n "${BUILD_CTX:-}" ] && rm -rf "$BUILD_CTX"' EXIT + +wait_http() { + local url=$1 label=$2 tries=${3:-60} + log "Waiting for $label..." + for i in $(seq 1 "$tries"); do + if curl -fsS "$url" >/dev/null 2>&1; then log "$label up."; return 0; fi + sleep 3 + done + fail "$label not healthy after $((tries * 3))s" +} + +# ── Step 1: Start cluster ───────────────────────────────────────────────────── +log "Step 1: Tearing down any previous run and starting a clean cluster..." +HUGEGRAPH_VERSION="$HUGEGRAPH_VERSION" \ + docker compose $COMPOSE_ARGS down -v --remove-orphans 2>&1 | tail -3 || true +HUGEGRAPH_VERSION="$HUGEGRAPH_VERSION" \ + docker compose $COMPOSE_ARGS up -d \ + --scale server0=0 --scale server1=0 --scale server2=0 \ + 2>&1 | grep -E "Started|Healthy|healthy" | tail -5 || true + +wait_http "http://localhost:8620/v1/health"; "pd0" 60 +wait_http "http://localhost:8520/v1/health"; "store0" 60 +wait_http "http://localhost:8521/v1/health"; "store1" 60 +wait_http "http://localhost:8522/v1/health"; "store2" 60 + +# Raft partition dirs are created lazily when the server first registers a graph. +# Start server0 just long enough for init-store to run, then stop it. +# We only need the init_complete flag to be written — we do NOT wait for /versions +# because start-hugegraph.sh has a 120s JVM-ready timeout that can expire on a cold +# distributed cluster, causing the entrypoint to exit and Docker to restart the +# container, resetting the timer indefinitely. +if ! docker exec hg-store0 sh -c 'ls /hugegraph-store/storage/raft/ 2>/dev/null | grep -qE "^[0-9]{5}$"' 2>/dev/null; then + log " Fresh cluster: starting server0 briefly to initialise partitions..." + HUGEGRAPH_VERSION="$HUGEGRAPH_VERSION" \ + HUGEGRAPH_STORE_IMAGE="$STORE_IMAGE" \ + docker compose $COMPOSE_ARGS up -d server0 2>&1 | tail -2 || true + + log " Waiting for init-store to complete (up to 120s)..." + for i in $(seq 1 40); do + if docker exec hg-server0 test -f /hugegraph-server/docker/init_complete 2>/dev/null; then + log " init_complete flag found after ~$((i * 3))s." + break + fi + sleep 3 + done + docker exec hg-server0 test -f /hugegraph-server/docker/init_complete 2>/dev/null \ + || fail "init-store did not complete within 120s" + + # Give Raft groups a moment to create their partition dirs + sleep 5 + docker compose $COMPOSE_ARGS stop server0 2>/dev/null || true + log " server0 stopped — partitions initialised." +fi + +# ── Step 2: Ensure committed snapshots exist on store0 ─────────────────────── +log "Step 2: Flushing + snapshotting all store nodes..." +for port in 8520 8521 8522; do + curl -fsS "http://localhost:${port}/test/flush"; >/dev/null && log " :${port} flush OK" + curl -fsS "http://localhost:${port}/test/snapshot"; >/dev/null && log " :${port} snapshot triggered" +done +log "Waiting 20s for Raft snapshot commits..." +sleep 20 + +SNAP_COUNT=$(docker run --rm \ + -v "${VOLUME_PREFIX}_hg-store0-data:/hugegraph-store/storage" \ + busybox sh -c 'find /hugegraph-store/storage/raft -name "data" -type d | wc -l') +log "store0 has $SNAP_COUNT committed snapshot data/ directories." +[ "$SNAP_COUNT" -ge 1 ] || fail "No committed snapshots on store0. Retry." + +# ── Step 3: Stop all stores ─────────────────────────────────────────────────── +log "Step 3: Stopping all store nodes..." +docker stop hg-store0 hg-store1 hg-store2 >/dev/null +log "All stores stopped." + +# ── Step 4: Corrupt one partition snapshot on store0 ───────────────────────── +# Two sub-cases of the bug: +# +# Sub-case A (race: state==doing at snapshot-save time) — tested in default mode: +# onSnapshotSave returns early → neither data/ nor should_not_load written. +# Snapshot dir has only __raft_snapshot_meta. +# On load: goes straight to loadSnapshot(missingDir) → "not exists" → stuck. +# Fix 1 (throw instead of return) prevents this snapshot from ever being committed. +# +# Sub-case B (JVM killed after should_not_load but before data/ completes) — tested in --fixed mode: +# Snapshot dir has __raft_snapshot_meta + should_not_load, but no data/. +# On load (buggy): shouldNotLoad() == true → silent return → partition silently has no data. +# On load (fixed): Fix 2 detects data/ is missing → logs warning → falls through to +# loadSnapshot → throws "not exists" → JRaft signals error → leader rescues. +# +log "Step 4: Corrupting one snapshot on store0 (sub-case $( $FIXED_MODE && echo B || echo A ))..." +TARGET=$(docker run --rm \ + -v "${VOLUME_PREFIX}_hg-store0-data:/hugegraph-store/storage" \ + busybox sh -c ' + for meta in $(find /hugegraph-store/storage/raft -name "__raft_snapshot_meta" | sort); do + snap=$(dirname "$meta") + if [ -d "$snap/data" ] && [ -f "$snap/should_not_load" ]; then + echo "$snap"; break + fi + done + ') + +[ -n "$TARGET" ] || fail "No suitable snapshot found (need data/ + should_not_load + __raft_snapshot_meta)" + +PARTITION_ID=$(echo "$TARGET" | grep -oE '/[0-9]{5}/' | head -1 | tr -d '/') +SNAP_NAME=$(basename "$TARGET") + +if $FIXED_MODE; then + # Sub-case B: remove only data/, keep should_not_load. + # Buggy image: shouldNotLoad() fires, silently returns — no error logged. + # Fixed image (Fix 2): detects data/ missing, logs warning, falls through. + log " Target: partition $PARTITION_ID / $SNAP_NAME" + log " Removing data/ only — keeping should_not_load (sub-case B)" + docker run --rm \ + -v "${VOLUME_PREFIX}_hg-store0-data:/hugegraph-store/storage" \ + busybox sh -c "rm -rf '${TARGET}/data' + echo 'Contents after corruption:'; ls '${TARGET}'" +else + # Sub-case A: remove both data/ and should_not_load — exactly what the race produces. + log " Target: partition $PARTITION_ID / $SNAP_NAME" + log " Removing data/ and should_not_load — leaving only __raft_snapshot_meta (sub-case A)" + docker run --rm \ Review Comment: ⚠️ Blocking: no. The default mode does not exercise the compaction-busy save path: it first creates a completed snapshot, then deletes `data/` and `should_not_load` at lines 211-214 and labels the resulting load error as `BUG REPRODUCED`. The fixed implementation would hit the same injected corruption, while `--fixed` only checks the load-side warning. Please add a run that triggers snapshot while compaction is actually active and asserts the save failure/retry, or narrow the script's claim. Evidence: lines 168-178 and 211-214; `/test/snapshot` only invokes `PartitionEngine.snapshot()` and the script never invokes `/test/compact`. -- 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]
