fix(store): throw on compaction-busy snapshot save; validate data/ on load (#3162)

[vaijosh]

Purpose of the PR

• close [Bug] A race between RocksDB compaction and periodic Raft snapshot save can commit an invalid snapshot directory that contains __raft_snapshot_meta but no data/ #3162

This PR addresses a race condition that occurs during snapshot saves when compaction is busy, which could previously lead to corrupted snapshots or stuck partitions.

Main Changes

• Throw on compaction-busy: Modified the snapshot save behavior to throw an exception rather than returning early/silently failing when compaction is busy.
• Validate data/ on load: Added validation during the snapshot load process to verify the presence of the data/ directory, preventing the system from loading incomplete snapshots.
• Reproduction Script: Added test-snapshot-corruption.sh to deterministically reproduce the bug and validate the fix across different storage states.
• Unit Tests: Added UTs to cover the new validation logic and race condition handling.

Verifying these changes

• Trivial rework / code cleanup without any test coverage. (No Need)
• Already covered by existing tests, such as (please modify tests here).
• Need tests and can be verified as follows:
  • Execute the newly added unit tests.
  • Run the test-snapshot-corruption.sh script to verify the corrupted snapshot detection and prevention.

Does this PR potentially affect the following parts?

• Dependencies
• Modify configurations
• The public API
• Other affects (typed here)
• Nope

Documentation Status

• Doc - TODO
• Doc - Done
• Doc - No Need

[imbajin]

Blocking: yes. Summary: Snapshot loading still silently accepts a non-directory data path, and the added reproducer cannot resolve its compose/root paths on a clean checkout; fixed mode also references a missing Dockerfile. Evidence: exact head 7ee5d42; all 17 exact-head check runs completed successfully.

[vaijosh]

Thanks @imbajin for review. I have addressed the review comments.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.76%. Comparing base (431f6e6) to head (46ce0c4).
⚠️ Report is 13 commits behind head on master.

Files with missing lines	Patch %	Lines
...ache/hugegraph/store/snapshot/SnapshotHandler.java	0.00%	8 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3164      +/-   ##
============================================
- Coverage     41.06%   37.76%   -3.30%     
- Complexity      519     6551    +6032     
============================================
  Files           771      800      +29     
  Lines         65962    68934    +2972     
  Branches       8766     9158     +392     
============================================
- Hits          27088    26035    -1053     
- Misses        36008    39841    +3833     
- Partials       2866     3058     +192     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[imbajin]

Blocking: no. Summary: The snapshot handling change is covered by green exact-head checks, but the new reproducer still cannot validate the compaction-busy save fix. Evidence: exact-head CI and Codecov checks completed successfully; see the inline finding.

[imbajin]

⚠️ 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.

[vaijosh]

Hi @imbajin
You're correct. The save-side throw (SnapshotHandler.java:98–101) only fires if compaction is actually in flight when snapshot() is called, and we have no reliable way to hold that state long enough. Because /test/compact submits a background task and returns immediately, the race window is too narrow to hit deterministically. This would make the test flaky.

Currently, the script simulates the outcome of that race (a missing data/ dir) by removing it manually, which lets us at least verify the load-side behavior. I have added a comment in the script to make this clear.

If there is a way to deterministically reproduce the save-side throw, I would be happy to incorporate it into the script.

[vaijosh]

@imbajin
I could not found deterministic steps to repro this issue ( because its timing issue). I had encountered this while testing changes related to Cloud storage support feature.

But this PR addresses critical edge case. For users running distributed HugeGraph clusters, this enhancement prevents silent failures or data inconsistency during high-throughput operations when snapshots and compactions overlap.

[bitflicker64]

Blocking: no. Summary: The load-side validation is correct (isDirectory() rather than exists(), and deliberately not requiring a non-empty data/, which keeps empty partitions working) and the fix lands in the handler that PartitionEngine actually wires up; the save-side change is broader than the defect needs, and the new tests and reproducer have a few rough edges. Evidence: read of SnapshotHandler.java, HgSnapshotHandlerTest.java and docker/test/test-snapshot-corruption.sh at 8e121d4; PartitionEngine.java:176-177, PartitionStateMachine.java:192-206 and BusinessHandlerImpl.dbCompaction read for the surrounding lifecycle; gh -R apache/hugegraph pr checks 3164 (all 17 pass).