sadanand48 commented on code in PR #10904:
URL: https://github.com/apache/ozone/pull/10904#discussion_r3688372567


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMRatisSnapshots.java:
##########
@@ -557,6 +560,88 @@ public void testInstallCorruptedCheckpointFailure() throws 
Exception {
     assertLogCapture(logCapture, msg);
   }
 
+  /**
+   * When the pre-install backup loop in replaceOMDBWithCheckpoint fails part 
way
+   * through, every item it already relocated into om.db.backup.* must be put 
back.
+   * Today the loop has no catch, so the items stay in the backup directory: 
the
+   * one that was moved first is lost, and if that item is om.db then 
reloadOMState
+   * silently re-creates an empty one.
+   */
+  @Test
+  public void testInstallSnapshotFailedBackupRestoresDbDir() throws Exception {
+    final String leaderOMNodeId = 
OmTestUtil.getCurrentOmProxyNodeId(objectStore);
+    OzoneManager leaderOM = cluster.getOzoneManager(leaderOMNodeId);
+    OzoneManagerRatisServer leaderRatisServer = leaderOM.getOmRatisServer();
+
+    // Find the inactive OM, so the checkpoint index is ahead of its applied 
index
+    // and canProceed lets the replacement start.
+    String followerNodeId = leaderOM.getPeerNodes().get(0).getNodeId();
+    if (cluster.isOMActive(followerNodeId)) {
+      followerNodeId = leaderOM.getPeerNodes().get(1).getNodeId();
+    }
+    OzoneManager followerOM = cluster.getOzoneManager(followerNodeId);
+
+    writeKeysToIncreaseLogIndex(leaderRatisServer, 100);
+
+    // Build a checkpoint whose top level holds two entries, so the backup loop
+    // performs two moves and can fail on the second.
+    DBCheckpoint leaderDbCheckpoint =
+        leaderOM.getMetadataManager().getStore().getCheckpoint(false);
+    Path leaderCheckpointLocation = leaderDbCheckpoint.getCheckpointLocation();
+    assertNotNull(leaderCheckpointLocation);
+    Path omDbDir = leaderCheckpointLocation.resolve(OM_DB_NAME);
+    assertTrue(omDbDir.toFile().mkdir());
+    moveCheckpointContentsToOmDbDir(leaderCheckpointLocation, omDbDir);
+    Files.createDirectory(leaderCheckpointLocation.resolve(OM_SNAPSHOT_DIR));
+
+    TransactionInfo leaderCheckpointTrxnInfo =
+        OzoneManagerRatisUtils.getTrxnInfoFromCheckpoint(conf, omDbDir);
+
+    // Give the follower a matching second entry with a sentinel inside it, so 
the
+    // loop finds two items in the follower's metadata dir to relocate.
+    File followerMetaDir = OMStorage.getOmDbDir(followerOM.getConfiguration());
+    Path followerDbDir = Paths.get(followerMetaDir.toString(), OM_DB_NAME);
+    Path followerSnapshotDir = Paths.get(followerMetaDir.toString(), 
OM_SNAPSHOT_DIR);
+    Files.createDirectories(followerSnapshotDir);
+    Path sentinel = followerSnapshotDir.resolve("sentinel");
+    Files.write(sentinel, "keep-me".getBytes(UTF_8));
+
+    Set<String> namesBefore = topLevelNames(followerMetaDir);
+    assertThat(namesBefore).contains(OM_DB_NAME, OM_SNAPSHOT_DIR);
+    Object dbInodeBefore = getINode(followerDbDir);
+
+    // Fail the second of the two backup moves. The first has already 
succeeded,
+    // so the DB directory is now missing whichever item was relocated first.
+    followerOM.setCheckpointBackupInjector(new ThrowOnNthPauseFaultInjector(2,
+        "Simulated backup move failure for test"));
+    followerOM.setExitManagerForTesting(new DummyExitManager());
+    try {
+      TermIndex termIndex = followerOM.installCheckpoint(
+          leaderOMNodeId, leaderCheckpointLocation, leaderCheckpointTrxnInfo);
+      assertNull(termIndex, "Install should have been reported as failed");
+
+      // Everything present before the aborted install must still be present.
+      assertThat(topLevelNames(followerMetaDir)).containsAll(namesBefore);
+      assertTrue(Files.exists(sentinel),
+          "Sentinel under " + OM_SNAPSHOT_DIR + " was relocated and never 
restored");
+      assertEquals("keep-me", new String(Files.readAllBytes(sentinel), UTF_8));
+      // The original om.db must be the one still in place, not a fresh empty 
DB.
+      assertEquals(dbInodeBefore, getINode(followerDbDir),
+          OM_DB_NAME + " was replaced rather than restored");
+    } finally {
+      followerOM.setCheckpointBackupInjector(null);
+    }

Review Comment:
   +1



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