[GitHub] [ozone] errose28 commented on a change in pull request #2017: HDDS-4908. Layout version should be available in DB for an un-finalized OM to be finalized through a Ratis snapshot.

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

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -550,6 +551,23 @@ private void instantiateServices(boolean withNewSnapshot) throws IOException {
     keyManager = new KeyManagerImpl(this, scmClient, configuration,
         omStorage.getOmId());
 
+    if (withNewSnapshot) {
+      Integer layoutVersionInDB = getLayoutVersionInDB();
+      if (layoutVersionInDB != null &&
+          versionManager.getMetadataLayoutVersion() < layoutVersionInDB) {
+        LOG.info("New OM snapshot received with higher layout version {}. " +
+            "Attempting to finalize current OM to that version.",
+            layoutVersionInDB);
+        upgradeFinalizer.finalizeAndWaitForCompletion(
+            "om-ratis-snapshot", this,
+            TimeUnit.SECONDS.toSeconds(10));

Review comment:
       Any reason for choosing this value? Should it be configurable?

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -100,6 +104,7 @@ public Void call() throws IOException {
         for (OMLayoutFeature f : versionManager.unfinalizedFeatures()) {
           finalizeFeature(f);
           updateLayoutVersionInVersionFile(f, ozoneManager.getOmStorage());
+          updateLayoutVersionInDB(f, ozoneManager);

Review comment:
       This will update the layout version in the DB in the finalize request, not the finalize response. Say the OM goes down while processing a finalize upgrade request but before it gets to this step. Will the DB still get the layout version info when the OM is restarted?

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##########
@@ -143,6 +143,7 @@
   public static final String PREFIX_TABLE = "prefixTable";
   public static final String TRANSACTION_INFO_TABLE =
       "transactionInfoTable";
+  public static final String META_TABLE = "metaTable";

Review comment:
       Can we come up with a better name than `metaTable`? Maybe `versionTable`?




----------------------------------------------------------------
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:
users@infra.apache.org



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