vaijosh commented on code in PR #3164: URL: https://github.com/apache/hugegraph/pull/3164#discussion_r3896231298
########## docker/test/test-snapshot-corruption.sh: ########## @@ -0,0 +1,322 @@ +#!/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 — load-path reproducer for the HStore snapshot corruption bug +# +# This script covers the LOAD side of the fix (SnapshotHandler.onSnapshotLoad): +# default mode — simulates Sub-case A outcome (missing data/ dir) to verify the load error +# --fixed mode — simulates Sub-case B (should_not_load present, data/ missing) to verify Fix 2 +# +# The SAVE side of the fix (SnapshotHandler.onSnapshotSave throwing when compaction state==doing) +# cannot be reproduced deterministically here: /test/compact submits a background job and returns +# immediately, so the race window is too narrow to hit reliably from a shell script. +# Save-side coverage lives in the unit test: HgSnapshotHandlerTest. +# +# 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 load-path bug is present (buggy image) +# bash docker/hbase/test/test-snapshot-corruption.sh --fixed # confirm load-path fix is active (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) — load-path simulated in default mode: +# The actual race cannot be triggered deterministically from a shell script (see header). +# We instead simulate the outcome: manually remove data/ and should_not_load, leaving only +# __raft_snapshot_meta, which is what the race would produce. +# On load: goes straight to loadSnapshot(missingDir) → "not exists" → stuck. +# Fix 1 (throw instead of return in onSnapshotSave) prevents this snapshot from ever being +# committed; this script validates only the resulting load-path error, not the throw itself. +# +# 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 '/') Review Comment: I have removed `test-snapshot-corruption.sh` because I was unable to reproduce the original issue (the `saveSnapshot` race condition) using it. Instead, I have covered those scenarios with unit tests in a more deterministic way. -- 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]
