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