szetszwo commented on code in PR #11086:
URL: https://github.com/apache/ozone/pull/11086#discussion_r3839065895
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java:
##########
@@ -634,7 +634,12 @@ public CompletableFuture<TermIndex>
notifyInstallSnapshotFromLeader(
return CompletableFuture.supplyAsync(
() -> {
try {
- return ozoneManager.installSnapshotFromLeader(leaderNodeId);
+ TermIndex termIndex =
ozoneManager.installSnapshotFromLeader(leaderNodeId);
+ if (termIndex == null) {
+ throw new CompletionException(
+ new IOException("Failed to install snapshot from OM leader "
+ leaderNodeId));
+ }
+ return termIndex;
Review Comment:
@andyhuangdev , I agree your approach and have tried it a little bit as
below (TODO: we should change all LOG.error to throw an excepiton)
```diff
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
index 4254a634b3..1806ec3d42 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
@@ -4260,7 +4260,7 @@ public synchronized TermIndex
installSnapshotFromLeader(String leaderId) throws
}
termIndex = installCheckpoint(leaderId, checkpointLocation);
} catch (Exception ex) {
- LOG.error("Failed to install snapshot from Leader OM.", ex);
+ throw new IOException("Failed to install snapshot from Leader " +
leaderId, ex);
} finally {
cleanupCheckpoint(omDBCheckpoint);
}
@@ -4317,6 +4317,7 @@ TermIndex installCheckpoint(String leaderId, Path
checkpointLocation,
long startTime = Time.monotonicNow();
File oldDBLocation = metadataManager.getStore().getDbLocation();
Path omDbPath = Paths.get(checkpointLocation.toString(), OM_DB_NAME);
+ Exception exception = null;
try {
// Stop Background services
keyManager.stop();
@@ -4328,13 +4329,11 @@ TermIndex installCheckpoint(String leaderId, Path
checkpointLocation,
// pending transactions in the buffer, they are discarded.
omRatisServer.getOmStateMachine().pause();
} catch (Exception e) {
- LOG.error("Failed to stop/ pause the services. Cannot proceed with " +
- "installing the new checkpoint.");
// Stop the checkpoint install process and restart the services.
keyManager.start(configuration);
startSecretManagerIfNecessary();
startTrashEmptier(configuration);
- throw e;
+ throw new IOException("Failed to installCheckpoint " +
checkpointTrxnInfo + ": Cannot stop/pause services.");
}
File dbBackup = null;
@@ -4380,9 +4379,8 @@ TermIndex installCheckpoint(String leaderId, Path
checkpointLocation,
"index: {}, time: {} ms", leaderId, term, lastAppliedIndex,
Time.monotonicNow() - time);
} catch (Exception e) {
- LOG.error("Failed to install Snapshot from {} as OM failed to
replace" +
- " DB with downloaded checkpoint. Reloading old OM state.",
- leaderId, e);
+ exception = new IOException("Failed to installCheckpoint " +
checkpointTrxnInfo
+ + ": Cannot replace DB.");
}
} else {
LOG.warn("Cannot proceed with InstallSnapshot as OM is at TermIndex
{} " +
@@ -4449,6 +4447,9 @@ TermIndex installCheckpoint(String leaderId, Path
checkpointLocation,
dbBackup, e);
}
+ if (exception != null) {
+ throw exception;
+ }
if (lastAppliedIndex != checkpointTrxnInfo.getTransactionIndex()) {
// Install Snapshot failed and old state was reloaded. Return null to
// Ratis to indicate that installation failed.
```
--
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]
