imbajin commented on code in PR #3164:
URL: https://github.com/apache/hugegraph/pull/3164#discussion_r3805553518
##########
hugegraph-store/hg-store-core/src/main/java/org/apache/hugegraph/store/snapshot/SnapshotHandler.java:
##########
@@ -172,8 +174,14 @@ public void onSnapshotLoad(final SnapshotReader reader,
long committedIndex) thr
// No need to load locally saved snapshots
if (shouldNotLoad(reader)) {
- log.info("skip to load snapshot because of should_not_load flag");
- return;
+ final String dataDir = snapshotDir + File.separator +
SNAPSHOT_DATA_PATH;
+ if (new File(dataDir).exists()) {
Review Comment:
⚠️ Blocking: yes. `File.exists()` is also true when `data` is a regular
file. A snapshot containing `should_not_load` plus a file named `data` still
takes this early return and hides the corruption instead of reaching
`loadSnapshot`; use `isDirectory()` (and validate the expected contents) before
skipping. Evidence: `SnapshotHandler.java:176-185` at head
`7ee5d420ca7301d8038f1fe981dd82623c8961c9`.
##########
docker/test/test-snapshot-corruption.sh:
##########
@@ -0,0 +1,314 @@
+
+#!/usr/bin/env bash
Review Comment:
🧹 Blocking: no. The file starts with a blank byte before `#!/usr/bin/env
bash`, so direct execution of this 100755 script can fail with `Exec format
error`; move the shebang to byte 0. Evidence: the exact-head first bytes are
`0a 23 21 2f`.
##########
docker/test/test-snapshot-corruption.sh:
##########
@@ -0,0 +1,314 @@
+
+#!/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"
Review Comment:
⚠️ Blocking: no. With this script under `docker/test`,
`$SCRIPT_DIR/../../docker-compose-3pd-3store-3server.yml` resolves to
`<repo>/docker-compose-3pd-3store-3server.yml`, but the exact-head tree
contains the compose file at `docker/docker-compose-3pd-3store-3server.yml`;
the first `docker compose` command therefore fails in both modes. The same path
block sets `REPO_ROOT` with `../../../..` above the repository, so `--fixed`
also cannot find `pom.xml`. Please correct the relative paths and the stale
`docker/hbase/test` examples. Evidence: exact-head tree listing and path
resolution.
##########
docker/test/test-snapshot-corruption.sh:
##########
@@ -0,0 +1,314 @@
+
+#!/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; }
+
+# In --fixed mode use the locally-built patched image.
+# Build it from source if it doesn't exist yet so the caller only needs
--fixed.
+PATCHED_IMAGE="hugegraph/store:patched"
+DOCKERFILE="$SCRIPT_DIR/../../Dockerfile.store-patched"
Review Comment:
‼️ Blocking: no. A clean `--fixed` run defaults to
`hugegraph/store:patched`, then unconditionally builds with
`.../Dockerfile.store-patched` when that image is absent, but the exact-head
tree contains no `Dockerfile.store-patched` under `docker/` or the repository
root. This makes the documented fixed mode fail before the reproducer starts;
add the Dockerfile or point to an existing build path. Evidence: `gh api
repos/apache/hugegraph/contents/docker?ref=7ee5d420ca7301d8038f1fe981dd82623c8961c9`
lists no such file.
--
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]
