smengcl opened a new pull request, #10904:
URL: https://github.com/apache/ozone/pull/10904

   ## What changes were proposed in this pull request?
   
   `OzoneManager.replaceOMDBWithCheckpoint` moves the existing 
metadata-directory contents into `om.db.backup.<index>_<ts>` before moving the 
checkpoint into place. The [backup 
loop](https://github.com/apache/ozone/blob/8acc0413a9548462df8a699133d009a230dff2c5/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java#L4485-L4509)
 has no `catch`, so an `IOException` after the [first 
move](https://github.com/apache/ozone/blob/8acc0413a9548462df8a699133d009a230dff2c5/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java#L4505)
 leaves the DB directory partially emptied. The rollback for this exists only 
in [`moveCheckpointFiles`'s catch 
block](https://github.com/apache/ozone/blob/8acc0413a9548462df8a699133d009a230dff2c5/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java#L4566-L4602),
 which runs [after the loop that 
threw](https://github.com/apache/ozone/blob/8acc0413a9548462df8a699133d009a230df
 
f2c5/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java#L4513).
   
   Nothing then fails: the `dbInconsistentMarker` guard is written inside 
`moveCheckpointFiles`, so a backup-loop failure never sets it, and 
`reloadOMState()` succeeds by re-creating `om.db` empty. Full impact analysis 
is on the Jira.
   
   This PR extracts that rollback into `restoreFromBackup(dbDir, dbBackupDir, 
backedUpItems)` and calls it from a new `catch (IOException)` on the backup 
loop before rethrowing. The extracted body is unchanged apart from indentation, 
and no currently-succeeding path changes behaviour.
   
   The second commit adds a nullable `checkpointBackupInjector` and a 
`@VisibleForTesting` setter on `OzoneManager` so the test can fail a chosen 
backup move, following the existing [`FaultInjector` 
idiom](https://github.com/apache/ozone/blob/8acc0413a9548462df8a699133d009a230dff2c5/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/RDBSnapshotProvider.java#L225-L248).
 It is a no-op in production. A filesystem-permission injection was tried 
first, but it depended on `Files.list` ordering and made the test flaky.
   
   Found by a TLA+ model of the follower snapshot-install path, confirmed by 
code reading, and reproduced by the test below.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-16026
   
   ## How was this patch tested?
   
   New `TestOMRatisSnapshots#testInstallSnapshotFailedBackupRestoresDbDir` 
drives the real install path on a 3-OM `MiniOzoneHACluster`, fails the second 
backup move, and asserts the metadata directory is unchanged. Reverting the 
first commit makes it fail with a different inode at `metaDir/om.db`:
   
   ```
   om.db was replaced rather than restored
     expected: <(dev=1000012,ino=4003784)> but was: <(dev=1000012,ino=4004376)>
   ```
   
   That is the original DB relocated and a fresh empty one created in its 
place. Names alone are not enough, since RocksDB re-creates `om.db`, so the 
test compares inodes; its name-set assertion covers the opposite `Files.list` 
ordering, where `db.snapshots` is lost instead.
   
   * `mvn -pl :ozone-integration-test test -Dtest=TestOMRatisSnapshots` — 6 
tests, 0 failures (176.7s), including `testInstallCorruptedCheckpointFailure`, 
which exercises the extracted rollback, and both happy-path installs.
   * `checkstyle.sh` — 0 violations.
   
   Not tested: whether a separate volume or bind mount under the OM metadata 
directory makes the failure deterministic rather than sporadic. That needs a 
mount; it is recorded as unverified on the Jira.
   
   Generated-by: Claude Code (Opus 5)
   


-- 
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]

Reply via email to