[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1129: HDDS-3741. Reload old OM state if Install Snapshot from Leader fails
arp7 commented on a change in pull request #1129:
URL: https://github.com/apache/hadoop-ozone/pull/1129#discussion_r455950095
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -3041,58 +3047,74 @@ public TermIndex installSnapshot(String leaderId) {
omRatisServer.getOmStateMachine().pause();
} catch (Exception e) {
LOG.error("Failed to stop/ pause the services. Cannot proceed with " +
- "installing the new checkpoint.", e);
- return null;
-}
-
-//TODO: un-pause SM if any failures and retry?
-
-long lastAppliedIndex = omRatisServer.getLastAppliedTermIndex().getIndex();
-
-boolean canProceed =
-OzoneManagerRatisUtils.verifyTransactionInfo(omTransactionInfo,
-lastAppliedIndex, leaderId, newDBlocation);
-
-// If downloaded DB has transaction info less than current one, return.
-if (!canProceed) {
- return null;
+ "installing the new checkpoint.");
+ // During stopServices, if KeyManager was stopped successfully and
+ // OMMetadataManager stop failed, we should restart the KeyManager.
+ keyManager.start(configuration);
+ throw e;
}
-long leaderIndex = omTransactionInfo.getTransactionIndex();
-long leaderTerm = omTransactionInfo.getCurrentTerm();
+File dbBackup = null;
+TermIndex termIndex = omRatisServer.getLastAppliedTermIndex();
+long term = termIndex.getTerm();
+long lastAppliedIndex = termIndex.getIndex();
+// Check if current applied log index is smaller than the downloaded
+// checkpoint transaction index. If yes, proceed by stopping the ratis
+// server so that the OM state can be re-initialized. If no then do not
+// proceed with installSnapshot.
+boolean canProceed = OzoneManagerRatisUtils.verifyTransactionInfo(
+checkpointTrxnInfo, lastAppliedIndex, leaderId, checkpointLocation);
-File dbBackup;
-try {
- dbBackup = replaceOMDBWithCheckpoint(lastAppliedIndex, oldDBLocation,
- newDBlocation);
-} catch (Exception e) {
- LOG.error("OM DB checkpoint replacement with new downloaded checkpoint "
+
- "failed.", e);
- return null;
+if (canProceed) {
+ try {
+dbBackup = replaceOMDBWithCheckpoint(lastAppliedIndex, oldDBLocation,
+checkpointLocation);
+term = checkpointTrxnInfo.getTerm();
+lastAppliedIndex = checkpointTrxnInfo.getTransactionIndex();
+LOG.info("Replaced DB with checkpoint from OM: {}, term: {}, index:
{}",
+leaderId, term, lastAppliedIndex);
+ } catch (Exception e) {
+LOG.error("Failed to install Snapshot from {} as OM failed to replace"
+
+" DB with downloaded checkpoint. Reloading old OM state.", e);
+ }
+} else {
+ LOG.warn("Cannot proceed with InstallSnapshot as OM is at TermIndex {} "
+
+ "and checkpoint has lower TermIndex {}. Reloading old state of OM.",
+ termIndex, checkpointTrxnInfo.getTermIndex());
}
// Reload the OM DB store with the new checkpoint.
// Restart (unpause) the state machine and update its last applied index
// to the installed checkpoint's snapshot index.
try {
- reloadOMState(leaderIndex, leaderTerm);
- omRatisServer.getOmStateMachine().unpause(leaderIndex, leaderTerm);
-} catch (IOException e) {
- LOG.error("Failed to reload OM state with new DB checkpoint.", e);
- return null;
+ reloadOMState(lastAppliedIndex, term);
+ omRatisServer.getOmStateMachine().unpause(lastAppliedIndex, term);
+ LOG.info("Reloaded OM state with Term: {} and Index: {}", term,
+ lastAppliedIndex);
+} catch (IOException ex) {
+ String errorMsg = "Failed to reload OM state and instantiate services.";
+ exitManager.exitSystem(1, errorMsg, ex, LOG);
}
// Delete the backup DB
try {
- FileUtils.deleteFully(dbBackup);
+ if (dbBackup != null) {
+FileUtils.deleteFully(dbBackup);
+ }
Review comment:
Let's do this in a followup jira. Ok to commit this one.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1129: HDDS-3741. Reload old OM state if Install Snapshot from Leader fails
arp7 commented on a change in pull request #1129:
URL: https://github.com/apache/hadoop-ozone/pull/1129#discussion_r455442847
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -3041,58 +3047,74 @@ public TermIndex installSnapshot(String leaderId) {
omRatisServer.getOmStateMachine().pause();
} catch (Exception e) {
LOG.error("Failed to stop/ pause the services. Cannot proceed with " +
- "installing the new checkpoint.", e);
- return null;
-}
-
-//TODO: un-pause SM if any failures and retry?
-
-long lastAppliedIndex = omRatisServer.getLastAppliedTermIndex().getIndex();
-
-boolean canProceed =
-OzoneManagerRatisUtils.verifyTransactionInfo(omTransactionInfo,
-lastAppliedIndex, leaderId, newDBlocation);
-
-// If downloaded DB has transaction info less than current one, return.
-if (!canProceed) {
- return null;
+ "installing the new checkpoint.");
+ // During stopServices, if KeyManager was stopped successfully and
+ // OMMetadataManager stop failed, we should restart the KeyManager.
+ keyManager.start(configuration);
+ throw e;
}
-long leaderIndex = omTransactionInfo.getTransactionIndex();
-long leaderTerm = omTransactionInfo.getCurrentTerm();
+File dbBackup = null;
+TermIndex termIndex = omRatisServer.getLastAppliedTermIndex();
+long term = termIndex.getTerm();
+long lastAppliedIndex = termIndex.getIndex();
+// Check if current applied log index is smaller than the downloaded
+// checkpoint transaction index. If yes, proceed by stopping the ratis
+// server so that the OM state can be re-initialized. If no then do not
+// proceed with installSnapshot.
+boolean canProceed = OzoneManagerRatisUtils.verifyTransactionInfo(
+checkpointTrxnInfo, lastAppliedIndex, leaderId, checkpointLocation);
-File dbBackup;
-try {
- dbBackup = replaceOMDBWithCheckpoint(lastAppliedIndex, oldDBLocation,
- newDBlocation);
-} catch (Exception e) {
- LOG.error("OM DB checkpoint replacement with new downloaded checkpoint "
+
- "failed.", e);
- return null;
+if (canProceed) {
+ try {
+dbBackup = replaceOMDBWithCheckpoint(lastAppliedIndex, oldDBLocation,
+checkpointLocation);
+term = checkpointTrxnInfo.getTerm();
+lastAppliedIndex = checkpointTrxnInfo.getTransactionIndex();
+LOG.info("Replaced DB with checkpoint from OM: {}, term: {}, index:
{}",
+leaderId, term, lastAppliedIndex);
+ } catch (Exception e) {
+LOG.error("Failed to install Snapshot from {} as OM failed to replace"
+
+" DB with downloaded checkpoint. Reloading old OM state.", e);
+ }
+} else {
+ LOG.warn("Cannot proceed with InstallSnapshot as OM is at TermIndex {} "
+
+ "and checkpoint has lower TermIndex {}. Reloading old state of OM.",
+ termIndex, checkpointTrxnInfo.getTermIndex());
}
// Reload the OM DB store with the new checkpoint.
// Restart (unpause) the state machine and update its last applied index
// to the installed checkpoint's snapshot index.
try {
- reloadOMState(leaderIndex, leaderTerm);
- omRatisServer.getOmStateMachine().unpause(leaderIndex, leaderTerm);
-} catch (IOException e) {
- LOG.error("Failed to reload OM state with new DB checkpoint.", e);
- return null;
+ reloadOMState(lastAppliedIndex, term);
+ omRatisServer.getOmStateMachine().unpause(lastAppliedIndex, term);
+ LOG.info("Reloaded OM state with Term: {} and Index: {}", term,
+ lastAppliedIndex);
+} catch (IOException ex) {
+ String errorMsg = "Failed to reload OM state and instantiate services.";
+ exitManager.exitSystem(1, errorMsg, ex, LOG);
}
// Delete the backup DB
try {
- FileUtils.deleteFully(dbBackup);
+ if (dbBackup != null) {
+FileUtils.deleteFully(dbBackup);
+ }
Review comment:
On successful restart we should clear any old backups.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1129: HDDS-3741. Reload old OM state if Install Snapshot from Leader fails
arp7 commented on a change in pull request #1129:
URL: https://github.com/apache/hadoop-ozone/pull/1129#discussion_r455433582
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##
@@ -249,6 +252,20 @@ public void start(OzoneConfiguration configuration) throws
IOException {
if (store == null) {
File metaDir = OMStorage.getOmDbDir(configuration);
+ // Check if there is a DB Inconsistent Marker in the metaDir. This
+ // marker indicates that the DB is in an inconsistent state and hence
+ // the OM process should be terminated.
+ File markerFile = new File(metaDir, DB_TRANSIENT_MARKER);
+ if (Files.exists(markerFile.toPath())) {
Review comment:
Nitpick: why not just `markerFile.exists()`?
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OzoneManagerSnapshotProvider.java
##
@@ -113,8 +113,10 @@ public
OzoneManagerSnapshotProvider(MutableConfigurationSource conf,
public DBCheckpoint getOzoneManagerDBSnapshot(String leaderOMNodeID)
throws IOException {
String snapshotTime = Long.toString(System.currentTimeMillis());
-String snapshotFileName = Paths.get(omSnapshotDir.getAbsolutePath(),
-snapshotTime, OM_DB_NAME).toFile().getAbsolutePath();
+String snapshotFileName = OM_DB_NAME + "-" + leaderOMNodeID
Review comment:
Did we change the snapshot file name convention in this jira?
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##
@@ -228,15 +228,34 @@ public static Status
exceptionToResponseStatus(IOException exception) {
}
/**
- * Obtain Transaction info from downloaded snapshot DB.
+ * Obtain OMTransactionInfo from Checkpoint.
+ */
+ public static OMTransactionInfo getTrxnInfoFromCheckpoint(
+ OzoneConfiguration conf, Path dbPath) throws Exception {
+Path dbDir = dbPath.getParent();
+String dbName = dbPath.getFileName().toString();
+if (dbDir == null) {
+ throw new IOException("Checkpoint {} does not have proper DB location");
Review comment:
Print the `dbPath` in the exception message to make debugging easier in
case this happens.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1129: HDDS-3741. Reload old OM state if Install Snapshot from Leader fails
arp7 commented on a change in pull request #1129:
URL: https://github.com/apache/hadoop-ozone/pull/1129#discussion_r453138655
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##
@@ -249,6 +252,20 @@ public void start(OzoneConfiguration configuration) throws
IOException {
if (store == null) {
File metaDir = OMStorage.getOmDbDir(configuration);
+ // Check if there is a DB Inconsistent Marker in the metaDir. This
+ // marker indicates that the DB is in an inconsistent state and hence
+ // the OM process should be terminated.
+ File markerFile = new File(metaDir, DB_TRANSIENT_MARKER);
+ if (Files.exists(markerFile.toPath())) {
+LOG.error("File {} marks that OM DB is in an inconsistent state.");
+// Note - The marker file should be deleted only after fixing the DB.
+// In an HA setup, this can be done by replacing this DB with a
+// checkpoint from another OM.
+String errorMsg = "Cannot load OM DB as it is in an inconsistent " +
+"state.";
+ExitUtils.terminate(1, errorMsg, LOG);
Review comment:
Should we use `ExitManager` here?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1129: HDDS-3741. Reload old OM state if Install Snapshot from Leader fails
arp7 commented on a change in pull request #1129:
URL: https://github.com/apache/hadoop-ozone/pull/1129#discussion_r453138266
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -3314,4 +3352,8 @@ private void startJVMPauseMonitor() {
jvmPauseMonitor.init(configuration);
jvmPauseMonitor.start();
}
+
+ void setExitManagerForTesting(ExitManager exitManagerForTesting) {
Review comment:
Can you add a `@VisibleForTesting` annotation here?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1129: HDDS-3741. Reload old OM state if Install Snapshot from Leader fails
arp7 commented on a change in pull request #1129:
URL: https://github.com/apache/hadoop-ozone/pull/1129#discussion_r447284712
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -3040,45 +3024,61 @@ public TermIndex installSnapshot(String leaderId) {
} catch (Exception e) {
LOG.error("Failed to stop/ pause the services. Cannot proceed with " +
"installing the new checkpoint.", e);
+
+ // During stopServices, if KeyManager was stopped successfully and
+ // OMMetadataManager stop failed, we should restart the KeyManager.
+ keyManager.start(configuration);
+
return null;
}
-//TODO: un-pause SM if any failures and retry?
+File dbBackup;
+TermIndex termIndex = omRatisServer.getLastAppliedTermIndex();
+long currentTerm = termIndex.getTerm();
+long lastAppliedIndex = termIndex.getIndex();
+boolean loadSuccess = false;
-long lastAppliedIndex = omRatisServer.getLastAppliedTermIndex().getIndex();
+try {
+ // Check if current applied log index is smaller than the downloaded
+ // checkpoint transaction index. If yes, proceed by stopping the ratis
+ // server so that the OM state can be re-initialized. If no then do not
+ // proceed with installSnapshot.
+ boolean canProceed = OzoneManagerRatisUtils.verifyTransactionInfo(
+ omTransactionInfo, lastAppliedIndex, leaderId, newDBLocation);
+ if (!canProceed) {
+return null;
+ }
-boolean canProceed =
-OzoneManagerRatisUtils.verifyTransactionInfo(omTransactionInfo,
-lastAppliedIndex, leaderId, newDBlocation);
+ try {
+dbBackup = replaceOMDBWithCheckpoint(lastAppliedIndex, oldDBLocation,
Review comment:
Also the marker file should be created before starting the move
operations, and deleted on success.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1129: HDDS-3741. Reload old OM state if Install Snapshot from Leader fails
arp7 commented on a change in pull request #1129:
URL: https://github.com/apache/hadoop-ozone/pull/1129#discussion_r447280689
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -3040,45 +3024,61 @@ public TermIndex installSnapshot(String leaderId) {
} catch (Exception e) {
LOG.error("Failed to stop/ pause the services. Cannot proceed with " +
"installing the new checkpoint.", e);
+
+ // During stopServices, if KeyManager was stopped successfully and
+ // OMMetadataManager stop failed, we should restart the KeyManager.
+ keyManager.start(configuration);
+
return null;
}
-//TODO: un-pause SM if any failures and retry?
+File dbBackup;
+TermIndex termIndex = omRatisServer.getLastAppliedTermIndex();
+long currentTerm = termIndex.getTerm();
+long lastAppliedIndex = termIndex.getIndex();
+boolean loadSuccess = false;
-long lastAppliedIndex = omRatisServer.getLastAppliedTermIndex().getIndex();
+try {
+ // Check if current applied log index is smaller than the downloaded
+ // checkpoint transaction index. If yes, proceed by stopping the ratis
+ // server so that the OM state can be re-initialized. If no then do not
+ // proceed with installSnapshot.
+ boolean canProceed = OzoneManagerRatisUtils.verifyTransactionInfo(
+ omTransactionInfo, lastAppliedIndex, leaderId, newDBLocation);
+ if (!canProceed) {
+return null;
+ }
-boolean canProceed =
-OzoneManagerRatisUtils.verifyTransactionInfo(omTransactionInfo,
-lastAppliedIndex, leaderId, newDBlocation);
+ try {
+dbBackup = replaceOMDBWithCheckpoint(lastAppliedIndex, oldDBLocation,
+newDBLocation);
+ } catch (Exception e) {
+LOG.error("OM DB checkpoint replacement with new downloaded " +
+"checkpoint failed.", e);
+return null;
+ }
-// If downloaded DB has transaction info less than current one, return.
-if (!canProceed) {
- return null;
+ loadSuccess = true;
+} finally {
+ if (!loadSuccess) {
Review comment:
Remove this unpause, we already do a reload and unpause below.
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -3040,45 +3024,61 @@ public TermIndex installSnapshot(String leaderId) {
} catch (Exception e) {
LOG.error("Failed to stop/ pause the services. Cannot proceed with " +
"installing the new checkpoint.", e);
+
+ // During stopServices, if KeyManager was stopped successfully and
+ // OMMetadataManager stop failed, we should restart the KeyManager.
+ keyManager.start(configuration);
+
return null;
}
-//TODO: un-pause SM if any failures and retry?
+File dbBackup;
+TermIndex termIndex = omRatisServer.getLastAppliedTermIndex();
+long currentTerm = termIndex.getTerm();
+long lastAppliedIndex = termIndex.getIndex();
+boolean loadSuccess = false;
-long lastAppliedIndex = omRatisServer.getLastAppliedTermIndex().getIndex();
+try {
+ // Check if current applied log index is smaller than the downloaded
+ // checkpoint transaction index. If yes, proceed by stopping the ratis
+ // server so that the OM state can be re-initialized. If no then do not
+ // proceed with installSnapshot.
+ boolean canProceed = OzoneManagerRatisUtils.verifyTransactionInfo(
+ omTransactionInfo, lastAppliedIndex, leaderId, newDBLocation);
+ if (!canProceed) {
+return null;
+ }
-boolean canProceed =
-OzoneManagerRatisUtils.verifyTransactionInfo(omTransactionInfo,
-lastAppliedIndex, leaderId, newDBlocation);
+ try {
+dbBackup = replaceOMDBWithCheckpoint(lastAppliedIndex, oldDBLocation,
+newDBLocation);
+ } catch (Exception e) {
+LOG.error("OM DB checkpoint replacement with new downloaded " +
+"checkpoint failed.", e);
+return null;
+ }
-// If downloaded DB has transaction info less than current one, return.
-if (!canProceed) {
- return null;
+ loadSuccess = true;
Review comment:
Don't need `loadSuccess` any more.
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -3040,45 +3024,61 @@ public TermIndex installSnapshot(String leaderId) {
} catch (Exception e) {
LOG.error("Failed to stop/ pause the services. Cannot proceed with " +
"installing the new checkpoint.", e);
+
+ // During stopServices, if KeyManager was stopped successfully and
+ // OMMetadataManager stop failed, we should restart the KeyManager.
+ keyManager.start(configuration);
+
return null;
}
-//TODO: un-pause SM if any failures and retry?
+File dbBackup;
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1129: HDDS-3741. Reload old OM state if Install Snapshot from Leader fails
arp7 commented on a change in pull request #1129:
URL: https://github.com/apache/hadoop-ozone/pull/1129#discussion_r447272528
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -2999,32 +3000,15 @@ public TermIndex installSnapshot(String leaderId) {
}
DBCheckpoint omDBcheckpoint = getDBCheckpointFromLeader(leaderId);
-Path newDBlocation = omDBcheckpoint.getCheckpointLocation();
+Path newDBLocation = omDBcheckpoint.getCheckpointLocation();
LOG.info("Downloaded checkpoint from Leader {}, in to the location {}",
-leaderId, newDBlocation);
+leaderId, newDBLocation);
-// Check if current ratis log index is smaller than the downloaded
-// checkpoint transaction index. If yes, proceed by stopping the ratis
-// server so that the OM state can be re-initialized. If no, then do not
-// proceed with installSnapshot.
+OMTransactionInfo omTransactionInfo = getTransactionInfoFromDB(
+newDBLocation);
-OMTransactionInfo omTransactionInfo = null;
-
-Path dbDir = newDBlocation.getParent();
-if (dbDir == null) {
- LOG.error("Incorrect DB location path {} received from checkpoint.",
- newDBlocation);
- return null;
-}
-
-try {
- omTransactionInfo =
- OzoneManagerRatisUtils.getTransactionInfoFromDownloadedSnapshot(
- configuration, dbDir);
-} catch (Exception ex) {
- LOG.error("Failed during opening downloaded snapshot from " +
- "{} to obtain transaction index", newDBlocation, ex);
+if (omTransactionInfo == null) {
Review comment:
Let's log an error here.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
