Repository navigation
HDDS-16442. Fix snapshot diff integrity failure recovery after upgrade - #11808
jojochuang wants to merge 3 commits into
Conversation
Change-Id: I27661c03858cc1e82677ef502d26b6bc44f7d0d2
jojochuang
added
snapshot
There was a problem hiding this comment.
🟡 Changes recommended
A stale integrity check can mark a healthy replacement generation FAILED because the transition does not verify the job ID.
1 open issue
What changed in this PR
This PR fixes snapshot diff integrity-failure recovery in OM and preserves readability of completed jobs created before largestEntryKey was stored.
Changes:
- Passes the snapshot-pair job key through pagination to integrity-failure status updates.
- Skips last-entry comparisons for legacy jobs and guards updates when a job is missing.
- Adds regression tests for legacy reports and genuine integrity failures.
| File | Description |
|---|---|
| hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java | Adds regression coverage and updates pagination calls. |
| hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java | Corrects recovery lookups and handles legacy and missing jobs. |
Review effort: Balanced
Change-Id: I9a067647517f2ae511f6d5aa191d6ae552340593
There was a problem hiding this comment.
🟡 Changes recommended
Cleanup can delete an expired job between two lookups without the manager lock, causing recovery to throw an NPE instead of the intended IOException; shared synchronization or an atomic operation and a regression test are needed.
1 open issue
1 resolved since last review
Review effort: Balanced
Change-Id: If27454033da53cc3134539bdef0d145aa00ff707
SaketaChalamchala
left a comment
SaketaChalamchala
left a comment
There was a problem hiding this comment.
@jojochuang Thanks for the patch. Created a PR jojochuang#635 for the following and updated the tests as well.
The current change bypasses the integrity check for legacy diff jobs with null largestEntrykey. I created above PR retaining the integrity check for the legacy jobs Pre - #9399. PTAL
What changes were proposed in this pull request?
Snapshot diff report integrity failures currently update the job using its generation ID instead of the snapshot-pair key used by the job table. The lookup returns null and throws an NPE, leaving the job DONE instead of marking it FAILED for cleanup. Older completed jobs without
largestEntryKeyalso enter this failure path after an OM upgrade.Pass the job-table key through report pagination to the integrity check, skip the last-entry comparison when the stored key is absent, and atomically mark only the matching DONE generation FAILED. Stale readers leave replacement, already-failed, and removed jobs untouched. Job cleanup and the integrity-failure transition share the DB monitor, preventing cleanup from deleting the row between the status check and update. Regression tests cover reading a completed job with a null stored last-entry key and persisting FAILED on a genuine mismatch with distinct job ID and job-table key.
Adapted from HDDS-16442.001.patch, attached by Siyao Meng. The patch is adapted to current master. The integrity-failure transition also checks the current job ID and status under the manager lock to protect replacement generations.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16442
How was this patch tested?
Using JDK 21:
mvn -B -ntp -pl :ozone-manager -am test -Dtest=TestSnapshotDiffManager,TestSnapshotDiffCleanupService -Dsurefire.failIfNoSpecifiedTests=false -DskipShade -DskipRecon -DskipDocs: 75 tests passed (73 manager cases and 2 cleanup cases), zero failures or errors../hadoop-ozone/dev-support/checks/checkstyle.sh: passed, zero violations.-pl :ozone-manager -am -Perrorprone test-compile -DskipTests -DskipShade -DskipRecon -DskipDocs: passed.git diff --check: passed.Generated-by: Specula (Claude Opus 4.8), as disclosed in the Jira; Codex (GPT-6) for adapting and validating the patch.