[GitHub] [ozone] errose28 commented on a change in pull request #2475: HDDS-5516. Duplicate metrics registered while running checkScmHA upon scm startup.

[GitBox <gi...@apache.org>]

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



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/ScmHAUnfinalizedStateValidationAction.java
##########
@@ -45,21 +45,20 @@
 
   @Override
   public void execute(StorageContainerManager scm) throws IOException {
-    checkScmHA(scm.getConfiguration(), scm.getScmStorageConfig());
+    checkScmHA(scm.getConfiguration(), scm.getScmStorageConfig(),
+        scm.getLayoutVersionManager());
   }
 
   /**
    * Allows checking that SCM HA is not enabled while pre-finalized in both
    * scm init and the upgrade action run on start.
    */
   public static void checkScmHA(OzoneConfiguration conf,
-      SCMStorageConfig storageConf) throws IOException {
+      SCMStorageConfig storageConf, LayoutVersionManager versionManager)
+      throws IOException {
 
     // Since this action may need to be called outside the upgrade framework
     // during init, it needs to check for pre-finalized state.
-    HDDSLayoutVersionManager versionManager =
-        new HDDSLayoutVersionManager(storageConf.getLayoutVersion());
-
     if (versionManager.needsFinalization() &&

Review comment:
       This was a minor bug in the original code I believe. `versionManager.needsFinalization()` should be `versionManager.isAllowed(SCM_HA)`. Per the comment above, this action which also needs to run on init is not guaranteed to run only when SCM HA is pre-finalized, so the original check could incorrectly fail SCM HA usage for later layout features. Although it is unlikely another SCM layout feature will require init of an existing SCM like SCM HA, I think we should update this here just to be safe.




-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org