chungen0126 commented on code in PR #11086:
URL: https://github.com/apache/ozone/pull/11086#discussion_r4017004764
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -4328,20 +4329,16 @@ 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.");
Review Comment:
Should we keep the error message?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -4464,6 +4464,18 @@ TermIndex installCheckpoint(String leaderId, Path
checkpointLocation,
return newTermIndex;
}
+ private static IOException newInstallCheckpointException(TransactionInfo
checkpointTrxnInfo,
+ String operation, Exception cause) {
+ return new IOException("Failed to install checkpoint " + checkpointTrxnInfo
+ + ": Cannot " + operation + '.', cause);
+ }
+
+ private static void throwIfInstallCheckpointFailed(IOException
installFailure) throws IOException {
+ if (installFailure != null) {
+ throw installFailure;
+ }
+ }
Review Comment:
Do we really need these methods?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
Review Comment:
I think we can remove this check. It's redundant because a failed snapshot
installation now throws an exception instead.
--
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]
