errose28 commented on a change in pull request #2257:
URL: https://github.com/apache/ozone/pull/2257#discussion_r636945091



##########
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -962,6 +963,11 @@ public static boolean scmInit(OzoneConfiguration conf,
         return false;
       }
     } else {
+      // If SCM HA was not being used before pre-finalize, and is being used
+      // when the cluster is pre-finalized for the SCM HA feature, init
+      // should fail.
+      ScmHAUnfinalizedStateValidationAction.checkScmHA(conf, scmStorageConfig);
+
       clusterId = scmStorageConfig.getClusterID();
       final boolean isSCMHAEnabled = scmStorageConfig.isSCMHAEnabled();
       if (SCMHAUtils.isSCMHAEnabled(conf) && !isSCMHAEnabled) {

Review comment:
       If SCM HA is used pre-finalized and it was not already being used, this 
action will fail with an exception and the code after the `checkScmHA` call 
will not run because the whole SCM will exit. However, if SCM HA is used 
pre-finalized when it was being used before the upgrade framework, or the 
cluster is not pre-finalized, the action will not throw an exception, and the 
code below the call will run. So it is not totally dead. This is the same 
behavior added to the SCM start method. Raising the exception on failure is 
preferable to an if-else check because the `UpgradeException` with 
`PREFINALIZE_ACTION_VALIDATION_FAILED` result code and message allows us to 
give more information to the user, rather than returning `false` from init.




-- 
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to