You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/09/30 02:21:52 UTC

[GitHub] [hadoop-ozone] fapifta opened a new pull request #1456: Upgrade

fapifta opened a new pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456


   ## What changes were proposed in this pull request?
   
   Server side implementation of the finalization logic is implemented in this pull request.
   The initial idea in the client side implementation is to issue an initiating request, then monitor the process with client side polling.
   This is reflected in the server side code, but due to complications, in the server side, the background finalization of layoutfeatures one by one, is postponed and tracked in HDDS-4286.
   The problem with the backround finalization is that it has to be synced between the OMs, and we need to ensure that once requests are in the state machine for finalizing a layout feature, the requests processed by the master before the finalization requests, and after the finalization requests are in sync and in order.
   This means that we need to post separate requests into the state machine internally on the leader OM, to have a flow, where the a specific request type or change to a request handling, once its activated, it is on all OMs activated at the same transaction. We do not really have such a logic for now, and it requires some further review. I will post a design doc on this one into HDDS-4286 in the near future about possible solutions.
   
   With that, in this patch, the finalization of the features is happening inside the initiating UpgradeFinalizeRequest RPC call handling, and it has to finish on at least two OMs. So we process the finalization inside the statemachine in one batch.
   After this, the client gets a STARTING_FINALIZATION status back, and after the 500ms delay on the client side, the client will grab the results.
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/HDDS-4172
   
   ## How was this patch tested?
   
   Quick manual test for the very basic workflow so far.
   JUnit tests to be added to the PR, but wanted to share to have reviews on the approach while I am working on the tests.
   


----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] fapifta commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
fapifta commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r498761138



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/StorageInfo.java
##########
@@ -150,6 +179,16 @@ private void verifyClusterId()
     }
   }
 
+  private void verifyUpgradingToLayoutVersion()
+      throws InconsistentStorageStateException {
+    int upgradeMark = getUpgradingToLayoutVersion();
+    if (upgradeMark != INVALID_LAYOUT_VERSION) {
+      throw new InconsistentStorageStateException("Ozone Manager died during"
+          + "a LayoutFeature upgrade.");

Review comment:
       fixed




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r499129787



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+    if (versionManager.needsFinalization()) {
+      status = FINALIZATION_REQUIRED;
+    }
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;
+    }
+    assertClientId(upgradeClientID);
+    List<String> returningMsgs = new ArrayList<>(msgs.size()+10);
+    status = isDone ? FINALIZATION_DONE : FINALIZATION_IN_PROGRESS;
+    while (msgs.size() > 0) {
+      returningMsgs.add(msgs.poll());
+    }
+    return new StatusAndMessages(status, returningMsgs);
+  }
+
+  private void assertClientId(String id) throws OMException {
+    if (!this.clientID.equals(id)) {
+      throw new OMException("Unknown client tries to get finalization status.\n"
+          + "The requestor is not the initiating client of the finalization,"
+          + " if you want to take over, and get unsent status messages, check"
+          + " -takeover option.", INVALID_REQUEST);
+    }
+  }
+
+
+
+
+  private class Worker implements Callable<Void> {
+    private OzoneManager ozoneManager;
+
+    Worker(OzoneManager om) {
+      ozoneManager = om;
+    }
+
+    @Override
+    public Void call() throws OMException {
+      try {
+        emitStartingMsg();
+
+        for (OMLayoutFeature f : versionManager.unfinalizedFeatures()) {
+          finalizeFeature(f);
+          updateLayoutVersionInVersionFile(f);
+          versionManager.finalized(f);

Review comment:
        Okay, let's do the future track on HDDS-4303 




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] fapifta commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
fapifta commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r503552977



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,328 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.*;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private  static final OmUpgradeAction NOOP = a -> {};
+
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;

Review comment:
       yes, I did not wanted to change the overall flow of messages and statusas that I came up with, but technically at this point we already done in the current implementation.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r498312712



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+    if (versionManager.needsFinalization()) {
+      status = FINALIZATION_REQUIRED;
+    }
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;

Review comment:
       Here clientID should be thread-safe to update, can we make clientID as a volatile variable?




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r498056289



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+    if (versionManager.needsFinalization()) {
+      status = FINALIZATION_REQUIRED;
+    }
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;
+    }
+    assertClientId(upgradeClientID);
+    List<String> returningMsgs = new ArrayList<>(msgs.size()+10);
+    status = isDone ? FINALIZATION_DONE : FINALIZATION_IN_PROGRESS;
+    while (msgs.size() > 0) {
+      returningMsgs.add(msgs.poll());
+    }
+    return new StatusAndMessages(status, returningMsgs);
+  }
+
+  private void assertClientId(String id) throws OMException {
+    if (!this.clientID.equals(id)) {
+      throw new OMException("Unknown client tries to get finalization status.\n"
+          + "The requestor is not the initiating client of the finalization,"
+          + " if you want to take over, and get unsent status messages, check"
+          + " -takeover option.", INVALID_REQUEST);
+    }
+  }
+
+
+
+
+  private class Worker implements Callable<Void> {
+    private OzoneManager ozoneManager;
+
+    Worker(OzoneManager om) {
+      ozoneManager = om;
+    }
+
+    @Override
+    public Void call() throws OMException {
+      try {
+        emitStartingMsg();
+
+        for (OMLayoutFeature f : versionManager.unfinalizedFeatures()) {
+          finalizeFeature(f);
+          updateLayoutVersionInVersionFile(f);
+          versionManager.finalized(f);

Review comment:
       Should we do the safe layout check here? We should ensure the feature layout is increasing between OMLayoutFeature, otherwise versionManager#finalized will throw IllegalArgumentException error. 

##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/StorageInfo.java
##########
@@ -150,6 +179,16 @@ private void verifyClusterId()
     }
   }
 
+  private void verifyUpgradingToLayoutVersion()
+      throws InconsistentStorageStateException {
+    int upgradeMark = getUpgradingToLayoutVersion();
+    if (upgradeMark != INVALID_LAYOUT_VERSION) {
+      throw new InconsistentStorageStateException("Ozone Manager died during"
+          + "a LayoutFeature upgrade.");

Review comment:
       Nit: Missing a whitespace before 'a LayoutFeature upgrade'.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+    if (versionManager.needsFinalization()) {
+      status = FINALIZATION_REQUIRED;
+    }
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;
+    }
+    assertClientId(upgradeClientID);
+    List<String> returningMsgs = new ArrayList<>(msgs.size()+10);
+    status = isDone ? FINALIZATION_DONE : FINALIZATION_IN_PROGRESS;
+    while (msgs.size() > 0) {
+      returningMsgs.add(msgs.poll());
+    }
+    return new StatusAndMessages(status, returningMsgs);
+  }
+
+  private void assertClientId(String id) throws OMException {
+    if (!this.clientID.equals(id)) {
+      throw new OMException("Unknown client tries to get finalization status.\n"
+          + "The requestor is not the initiating client of the finalization,"
+          + " if you want to take over, and get unsent status messages, check"
+          + " -takeover option.", INVALID_REQUEST);
+    }
+  }
+
+
+
+
+  private class Worker implements Callable<Void> {
+    private OzoneManager ozoneManager;
+
+    Worker(OzoneManager om) {
+      ozoneManager = om;
+    }
+
+    @Override
+    public Void call() throws OMException {
+      try {
+        emitStartingMsg();
+
+        for (OMLayoutFeature f : versionManager.unfinalizedFeatures()) {
+          finalizeFeature(f);
+          updateLayoutVersionInVersionFile(f);
+          versionManager.finalized(f);
+        }
+
+        emitFinishedMsg();
+      } finally {
+        isDone = true;
+      }
+      return null;
+    }
+
+    private void finalizeFeature(OMLayoutFeature feature)
+        throws OMException {
+      OmUpgradeAction action = feature.onFinalizeAction().orElse(NOOP);
+
+      if (action == NOOP) {
+        emitNOOPMsg(feature.name());
+        return;
+      }
+
+      putFinalizationMarkIntoVersionFile(feature);
+
+      emitStartingFinalizationActionMsg(feature.name());
+      action.executeAction(ozoneManager);
+      emitFinishFinalizationActionMsg(feature.name());
+
+      removeFinalizationMarkFromVersionFile(feature);
+    }
+
+    private void updateLayoutVersionInVersionFile(OMLayoutFeature feature)
+        throws OMException {
+      int prevLayoutVersion = currentStoredLayoutVersion();
+
+      updateStorageLayoutVersion(feature.layoutVersion());
+      try {
+        persistStorage();
+      } catch (IOException e) {
+        updateStorageLayoutVersion(prevLayoutVersion);
+        logLayoutVersionUpdateFailureAndThrow(e);
+      }
+    }
+
+    private void putFinalizationMarkIntoVersionFile(OMLayoutFeature feature)
+        throws OMException {
+      try {
+        emitUpgradeToLayoutVersionPersistingMsg(feature.name());
+
+        setUpgradeToLayoutVersionInStorage(feature.layoutVersion());
+        persistStorage();

Review comment:
       Same comment for persistStorage like above.

##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/AbstractLayoutVersionManager.java
##########
@@ -65,53 +64,66 @@ protected void initializeFeatures(LayoutFeature[] lfs) {
     });
   }
 
+  protected void reset() {
+    metadataLayoutVersion = 0;
+    softwareLayoutVersion = 0;
+    featureMap.clear();
+    features.clear();
+    isInitialized = false;
+  }
+
+  public void finalized(T layoutFeature) {
+    if (layoutFeature.layoutVersion() > metadataLayoutVersion) {
+      metadataLayoutVersion = layoutFeature.layoutVersion();
+    } else {
+      throw new IllegalArgumentException(
+          "Finalize attempt on a layoutFeature which has already been"
+              + "finalized. Software Layout version: " + softwareLayoutVersion

Review comment:
       Nit: Missing a whitespace before 'finalized'.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+    if (versionManager.needsFinalization()) {
+      status = FINALIZATION_REQUIRED;
+    }
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;
+    }
+    assertClientId(upgradeClientID);
+    List<String> returningMsgs = new ArrayList<>(msgs.size()+10);
+    status = isDone ? FINALIZATION_DONE : FINALIZATION_IN_PROGRESS;
+    while (msgs.size() > 0) {
+      returningMsgs.add(msgs.poll());
+    }
+    return new StatusAndMessages(status, returningMsgs);
+  }
+
+  private void assertClientId(String id) throws OMException {
+    if (!this.clientID.equals(id)) {
+      throw new OMException("Unknown client tries to get finalization status.\n"
+          + "The requestor is not the initiating client of the finalization,"
+          + " if you want to take over, and get unsent status messages, check"
+          + " -takeover option.", INVALID_REQUEST);
+    }
+  }
+
+
+
+
+  private class Worker implements Callable<Void> {
+    private OzoneManager ozoneManager;
+
+    Worker(OzoneManager om) {
+      ozoneManager = om;
+    }
+
+    @Override
+    public Void call() throws OMException {
+      try {
+        emitStartingMsg();
+
+        for (OMLayoutFeature f : versionManager.unfinalizedFeatures()) {
+          finalizeFeature(f);
+          updateLayoutVersionInVersionFile(f);
+          versionManager.finalized(f);
+        }
+
+        emitFinishedMsg();
+      } finally {
+        isDone = true;
+      }
+      return null;
+    }
+
+    private void finalizeFeature(OMLayoutFeature feature)
+        throws OMException {
+      OmUpgradeAction action = feature.onFinalizeAction().orElse(NOOP);
+
+      if (action == NOOP) {
+        emitNOOPMsg(feature.name());
+        return;
+      }
+
+      putFinalizationMarkIntoVersionFile(feature);
+
+      emitStartingFinalizationActionMsg(feature.name());
+      action.executeAction(ozoneManager);
+      emitFinishFinalizationActionMsg(feature.name());
+
+      removeFinalizationMarkFromVersionFile(feature);
+    }
+
+    private void updateLayoutVersionInVersionFile(OMLayoutFeature feature)
+        throws OMException {
+      int prevLayoutVersion = currentStoredLayoutVersion();
+
+      updateStorageLayoutVersion(feature.layoutVersion());
+      try {
+        persistStorage();
+      } catch (IOException e) {

Review comment:
       Do we really need to persist version to storage every time? Why not persist storage only once for the final finalization version? We already update current layout version on memory value (StorageInfo#LAYOUT_VERSION) each time.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] fapifta commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
fapifta commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r498766375



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+    if (versionManager.needsFinalization()) {
+      status = FINALIZATION_REQUIRED;
+    }
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;
+    }
+    assertClientId(upgradeClientID);
+    List<String> returningMsgs = new ArrayList<>(msgs.size()+10);
+    status = isDone ? FINALIZATION_DONE : FINALIZATION_IN_PROGRESS;
+    while (msgs.size() > 0) {
+      returningMsgs.add(msgs.poll());
+    }
+    return new StatusAndMessages(status, returningMsgs);
+  }
+
+  private void assertClientId(String id) throws OMException {
+    if (!this.clientID.equals(id)) {
+      throw new OMException("Unknown client tries to get finalization status.\n"
+          + "The requestor is not the initiating client of the finalization,"
+          + " if you want to take over, and get unsent status messages, check"
+          + " -takeover option.", INVALID_REQUEST);
+    }
+  }
+
+
+
+
+  private class Worker implements Callable<Void> {
+    private OzoneManager ozoneManager;
+
+    Worker(OzoneManager om) {
+      ozoneManager = om;
+    }
+
+    @Override
+    public Void call() throws OMException {
+      try {
+        emitStartingMsg();
+
+        for (OMLayoutFeature f : versionManager.unfinalizedFeatures()) {
+          finalizeFeature(f);
+          updateLayoutVersionInVersionFile(f);
+          versionManager.finalized(f);
+        }
+
+        emitFinishedMsg();
+      } finally {
+        isDone = true;
+      }
+      return null;
+    }
+
+    private void finalizeFeature(OMLayoutFeature feature)
+        throws OMException {
+      OmUpgradeAction action = feature.onFinalizeAction().orElse(NOOP);
+
+      if (action == NOOP) {
+        emitNOOPMsg(feature.name());
+        return;
+      }
+
+      putFinalizationMarkIntoVersionFile(feature);
+
+      emitStartingFinalizationActionMsg(feature.name());
+      action.executeAction(ozoneManager);
+      emitFinishFinalizationActionMsg(feature.name());
+
+      removeFinalizationMarkFromVersionFile(feature);
+    }
+
+    private void updateLayoutVersionInVersionFile(OMLayoutFeature feature)
+        throws OMException {
+      int prevLayoutVersion = currentStoredLayoutVersion();
+
+      updateStorageLayoutVersion(feature.layoutVersion());
+      try {
+        persistStorage();
+      } catch (IOException e) {

Review comment:
       The main things we want to ensure here:
   1. at any point in time there can be a failure point, so we would better persist where we are at before and after every major step, that is what we can not loose, as all the in-memory states can be lost at any point in time.
   2. The finalization action that we do for LayoutFeatures does the modifications required by the LayoutFeature inside our internal structures, so we need to know when we have started one such a step but we have not finished it due to a catastrophic failure, as in that case we are potentially in an inconsistent state, and OM should not proceed with anything until the state is examined and we are free to move further.
   3. If the finalization action is a NOOP, we skip this part, and just update the LayoutVersion and persist the update
   4. After a LayoutFeature is finalized we need to update and persist the LayoutVersion, as if the next finalize action ends in a catastrophic failure, we need to know where to catch up from.
   
   So In this particular case, we persisting the LayoutFeature's LayoutVersion to Storage to ensure point 4, so we never redo the finalization of a LayoutFeature, as redoing it might lead to an issue as the start state of a second finalization would be unexpected after it finished once.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+    if (versionManager.needsFinalization()) {
+      status = FINALIZATION_REQUIRED;
+    }
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;
+    }
+    assertClientId(upgradeClientID);
+    List<String> returningMsgs = new ArrayList<>(msgs.size()+10);
+    status = isDone ? FINALIZATION_DONE : FINALIZATION_IN_PROGRESS;
+    while (msgs.size() > 0) {
+      returningMsgs.add(msgs.poll());
+    }
+    return new StatusAndMessages(status, returningMsgs);
+  }
+
+  private void assertClientId(String id) throws OMException {
+    if (!this.clientID.equals(id)) {
+      throw new OMException("Unknown client tries to get finalization status.\n"
+          + "The requestor is not the initiating client of the finalization,"
+          + " if you want to take over, and get unsent status messages, check"
+          + " -takeover option.", INVALID_REQUEST);
+    }
+  }
+
+
+
+
+  private class Worker implements Callable<Void> {
+    private OzoneManager ozoneManager;
+
+    Worker(OzoneManager om) {
+      ozoneManager = om;
+    }
+
+    @Override
+    public Void call() throws OMException {
+      try {
+        emitStartingMsg();
+
+        for (OMLayoutFeature f : versionManager.unfinalizedFeatures()) {
+          finalizeFeature(f);
+          updateLayoutVersionInVersionFile(f);
+          versionManager.finalized(f);
+        }
+
+        emitFinishedMsg();
+      } finally {
+        isDone = true;
+      }
+      return null;
+    }
+
+    private void finalizeFeature(OMLayoutFeature feature)
+        throws OMException {
+      OmUpgradeAction action = feature.onFinalizeAction().orElse(NOOP);
+
+      if (action == NOOP) {
+        emitNOOPMsg(feature.name());
+        return;
+      }
+
+      putFinalizationMarkIntoVersionFile(feature);
+
+      emitStartingFinalizationActionMsg(feature.name());
+      action.executeAction(ozoneManager);
+      emitFinishFinalizationActionMsg(feature.name());
+
+      removeFinalizationMarkFromVersionFile(feature);
+    }
+
+    private void updateLayoutVersionInVersionFile(OMLayoutFeature feature)
+        throws OMException {
+      int prevLayoutVersion = currentStoredLayoutVersion();
+
+      updateStorageLayoutVersion(feature.layoutVersion());
+      try {
+        persistStorage();
+      } catch (IOException e) {
+        updateStorageLayoutVersion(prevLayoutVersion);
+        logLayoutVersionUpdateFailureAndThrow(e);
+      }
+    }
+
+    private void putFinalizationMarkIntoVersionFile(OMLayoutFeature feature)
+        throws OMException {
+      try {
+        emitUpgradeToLayoutVersionPersistingMsg(feature.name());
+
+        setUpgradeToLayoutVersionInStorage(feature.layoutVersion());
+        persistStorage();

Review comment:
       From the comment for the previous persisting related comment point 2 is relevant here, we need to know if we have started the finalization of a LayoutFeature but never finished it, and we need to know it even if there is a catastrophic failure, and OM has been restarted due to that failure, and persisting this information to disk seems to be the way to fullfil this requirement.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r499129787



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+    if (versionManager.needsFinalization()) {
+      status = FINALIZATION_REQUIRED;
+    }
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;
+    }
+    assertClientId(upgradeClientID);
+    List<String> returningMsgs = new ArrayList<>(msgs.size()+10);
+    status = isDone ? FINALIZATION_DONE : FINALIZATION_IN_PROGRESS;
+    while (msgs.size() > 0) {
+      returningMsgs.add(msgs.poll());
+    }
+    return new StatusAndMessages(status, returningMsgs);
+  }
+
+  private void assertClientId(String id) throws OMException {
+    if (!this.clientID.equals(id)) {
+      throw new OMException("Unknown client tries to get finalization status.\n"
+          + "The requestor is not the initiating client of the finalization,"
+          + " if you want to take over, and get unsent status messages, check"
+          + " -takeover option.", INVALID_REQUEST);
+    }
+  }
+
+
+
+
+  private class Worker implements Callable<Void> {
+    private OzoneManager ozoneManager;
+
+    Worker(OzoneManager om) {
+      ozoneManager = om;
+    }
+
+    @Override
+    public Void call() throws OMException {
+      try {
+        emitStartingMsg();
+
+        for (OMLayoutFeature f : versionManager.unfinalizedFeatures()) {
+          finalizeFeature(f);
+          updateLayoutVersionInVersionFile(f);
+          versionManager.finalized(f);

Review comment:
        Okay, let's do the further track on HDDS-4303 




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] fapifta commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
fapifta commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r498861508



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};

Review comment:
       Ok, it turned out that if OMUpgradeAction has just one constants, that qualifies as a bad practice, as if an interface does contain just constants, that can have some disadvantages according to seemingly more clever ppl than me. :)
   
   I moved it back to OMUpgradeFinalizer, as due to the logic we need to preserve the OMUpgradeAction-ness of the NOOP to work well there, as we want to push OzoneManager instances to the executeAction, and the type system is against having a simple UpgradeAction as a NOOP. We can thin about adding this to OMLayoutFeature instead, but as we do not use it elsewhere at the moment, and because we use Optional and we provide just a null in case the finalize action is a noop, we do not need it elsewhere as it seems...
   
   An other approach can be an orElse(null), but with that we can even leave out the Optional in the OMLayoutFeature and LayoutFeature implementations/definitions, and instead checking if it is a NOOP, we can check for null.
   
   What do you think?




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] fapifta commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
fapifta commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r498802254



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+    if (versionManager.needsFinalization()) {
+      status = FINALIZATION_REQUIRED;
+    }
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;

Review comment:
       Good point, though making clientID volatile, does not really help to solve a race condition here I believe, so I propose to synchronize the whole reportStatus method.
   The reasons is that, if there is the original client that polls together with a client that does a takeover, then both can step on each others toes, as if for the original client we are already processing the message queue when an other client takes over, then returning status might change as well, and also there might be missing messages in the responses sent to the clients.
   
   Fortunately we don't have to worry about isDone and messages at least, as even if isDone is being updated late, and the final message is polled from the msgs queue with an IN_PROGRES state, the client in the next cycle gets a DONE state without messages, and that is acceptable.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] fapifta commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
fapifta commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r503556816



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,328 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.*;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private  static final OmUpgradeAction NOOP = a -> {};
+
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public synchronized StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;
+    }
+    assertClientId(upgradeClientID);
+    List<String> returningMsgs = new ArrayList<>(msgs.size()+10);
+    Status status = isDone ? FINALIZATION_DONE : FINALIZATION_IN_PROGRESS;
+    while (msgs.size() > 0) {
+      returningMsgs.add(msgs.poll());
+    }
+    return new StatusAndMessages(status, returningMsgs);
+  }
+
+  private void assertClientId(String id) throws OMException {
+    if (!this.clientID.equals(id)) {
+      throw new OMException("Unknown client tries to get finalization status.\n"
+          + "The requestor is not the initiating client of the finalization,"
+          + " if you want to take over, and get unsent status messages, check"
+          + " -takeover option.", INVALID_REQUEST);
+    }
+  }
+
+  /**
+   * This class implements the finalization logic applied to every
+   * LayoutFeature that needs to be finalized.
+   *
+   * For the first approach this happens synchronously within the state machine
+   * during the FinalizeUpgrade request, but ideally this has to be moved to
+   * individual calls that are going into the StateMaching one by one.
+   * The prerequisits for this to happen in the background are the following:
+   * - more fine grained control for LayoutFeatures to prepare the
+   *    finalization outside the state machine, do the switch from old to new
+   *    logic inside the statemachine and apply the finalization, and then do
+   *    any cleanup necessary outside the state machine
+   * - a way to post a request to the state machine that is not part of the
+   *    client API, so basically not an OMRequest, but preferably an internal
+   *    request, which is posted from the leader OM to the follower OMs only.
+   * - ensure that there is a possibility to implement a rollback logic if
+   *    something goes wrong inside the state machine, to avoid OM stuck in an
+   *    intermediate state due to an error.
+   */
+  private class Worker implements Callable<Void> {
+    private OzoneManager ozoneManager;
+
+    /**
+     * Initiates the Worker, for the specified OM instance.
+     * @param om the OzoneManager instance on which to finalize the new
+     *           LayoutFeatures.
+     */
+    Worker(OzoneManager om) {
+      ozoneManager = om;
+    }
+
+    @Override
+    public Void call() throws OMException {
+      try {
+        emitStartingMsg();
+
+        for (OMLayoutFeature f : versionManager.unfinalizedFeatures()) {
+          finalizeFeature(f);
+          updateLayoutVersionInVersionFile(f);
+          versionManager.finalized(f);
+        }
+
+        emitFinishedMsg();
+      } finally {
+        isDone = true;
+      }
+      return null;
+    }
+
+    private void finalizeFeature(OMLayoutFeature feature)
+        throws OMException {
+      OmUpgradeAction action = feature.onFinalizeAction().orElse(NOOP);
+
+      if (action == NOOP) {
+        emitNOOPMsg(feature.name());
+        return;
+      }
+
+      putFinalizationMarkIntoVersionFile(feature);
+
+      emitStartingFinalizationActionMsg(feature.name());
+      try {
+        action.executeAction(ozoneManager);
+      } catch (Exception e) {
+        logFinalizationFailureAndThrow(e, feature.name());
+      }
+      emitFinishFinalizationActionMsg(feature.name());
+
+      removeFinalizationMarkFromVersionFile(feature);
+    }
+
+    private void updateLayoutVersionInVersionFile(OMLayoutFeature feature)
+        throws OMException {
+      int prevLayoutVersion = currentStoredLayoutVersion();
+
+      updateStorageLayoutVersion(feature.layoutVersion());
+      try {
+        persistStorage();
+      } catch (IOException e) {
+        updateStorageLayoutVersion(prevLayoutVersion);
+        logLayoutVersionUpdateFailureAndThrow(e);
+      }
+    }
+
+    private void putFinalizationMarkIntoVersionFile(OMLayoutFeature feature)
+        throws OMException {
+      try {
+        emitUpgradeToLayoutVersionPersistingMsg(feature.name());
+
+        setUpgradeToLayoutVersionInStorage(feature.layoutVersion());
+        persistStorage();
+
+        emitUpgradeToLayoutVersionPersistedMsg();
+      } catch (IOException e) {
+        logUpgradeToLayoutVersionPersistingFailureAndThrow(feature.name(), e);
+      }
+    }
+
+    private void removeFinalizationMarkFromVersionFile(OMLayoutFeature feature)
+        throws OMException {
+      try {
+        emitRemovingUpgradeToLayoutVersionMsg(feature.name());
+
+        unsetUpgradeToLayoutVersionInStorage();
+        persistStorage();
+
+        emitRemovedUpgradeToLayoutVersionMsg();
+      } catch (IOException e) {
+        logUpgradeToLayoutVersionRemovalFailureAndThrow(feature.name(), e);
+      }
+    }
+
+
+
+
+
+    private void setUpgradeToLayoutVersionInStorage(int version) {
+      ozoneManager.getOmStorage().setUpgradeToLayoutVersion(version);
+    }
+
+    private void unsetUpgradeToLayoutVersionInStorage() {
+      ozoneManager.getOmStorage().unsetUpgradeToLayoutVersion();
+    }
+
+    private int currentStoredLayoutVersion() {
+      return ozoneManager.getOmStorage().getLayoutVersion();
+    }
+
+    private void updateStorageLayoutVersion(int version) {
+      ozoneManager.getOmStorage().setLayoutVersion(version);
+    }
+
+    private void persistStorage() throws IOException {

Review comment:
       Storage and OMStorage chose to have an implementation for setters where the setter does not persist the change to the disk, these are the two that I have checked, and decided to remain consistent with this behaviour. This is the sole reason for controlling this from outside the Storage class implementation. Here it is extracted into a method to make it easier to follow the flow, and to hide the implementation detail of how to persist the storage from the reader when he reads the backbone of the internal logic of the methods with a higher layer in the abstractions.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] avijayanhwx merged pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
avijayanhwx merged pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456


   


----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r499129873



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+    if (versionManager.needsFinalization()) {
+      status = FINALIZATION_REQUIRED;
+    }
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;
+    }
+    assertClientId(upgradeClientID);
+    List<String> returningMsgs = new ArrayList<>(msgs.size()+10);
+    status = isDone ? FINALIZATION_DONE : FINALIZATION_IN_PROGRESS;
+    while (msgs.size() > 0) {
+      returningMsgs.add(msgs.poll());
+    }
+    return new StatusAndMessages(status, returningMsgs);
+  }
+
+  private void assertClientId(String id) throws OMException {
+    if (!this.clientID.equals(id)) {
+      throw new OMException("Unknown client tries to get finalization status.\n"
+          + "The requestor is not the initiating client of the finalization,"
+          + " if you want to take over, and get unsent status messages, check"
+          + " -takeover option.", INVALID_REQUEST);
+    }
+  }
+
+
+
+
+  private class Worker implements Callable<Void> {
+    private OzoneManager ozoneManager;
+
+    Worker(OzoneManager om) {
+      ozoneManager = om;
+    }
+
+    @Override
+    public Void call() throws OMException {
+      try {
+        emitStartingMsg();
+
+        for (OMLayoutFeature f : versionManager.unfinalizedFeatures()) {
+          finalizeFeature(f);
+          updateLayoutVersionInVersionFile(f);
+          versionManager.finalized(f);
+        }
+
+        emitFinishedMsg();
+      } finally {
+        isDone = true;
+      }
+      return null;
+    }
+
+    private void finalizeFeature(OMLayoutFeature feature)
+        throws OMException {
+      OmUpgradeAction action = feature.onFinalizeAction().orElse(NOOP);
+
+      if (action == NOOP) {
+        emitNOOPMsg(feature.name());
+        return;
+      }
+
+      putFinalizationMarkIntoVersionFile(feature);
+
+      emitStartingFinalizationActionMsg(feature.name());
+      action.executeAction(ozoneManager);
+      emitFinishFinalizationActionMsg(feature.name());
+
+      removeFinalizationMarkFromVersionFile(feature);
+    }
+
+    private void updateLayoutVersionInVersionFile(OMLayoutFeature feature)
+        throws OMException {
+      int prevLayoutVersion = currentStoredLayoutVersion();
+
+      updateStorageLayoutVersion(feature.layoutVersion());
+      try {
+        persistStorage();
+      } catch (IOException e) {
+        updateStorageLayoutVersion(prevLayoutVersion);
+        logLayoutVersionUpdateFailureAndThrow(e);
+      }
+    }
+
+    private void putFinalizationMarkIntoVersionFile(OMLayoutFeature feature)
+        throws OMException {
+      try {
+        emitUpgradeToLayoutVersionPersistingMsg(feature.name());
+
+        setUpgradeToLayoutVersionInStorage(feature.layoutVersion());
+        persistStorage();

Review comment:
       Comment makes sense to me.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] avijayanhwx commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r503538733



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,328 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.*;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private  static final OmUpgradeAction NOOP = a -> {};
+
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public synchronized StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;
+    }
+    assertClientId(upgradeClientID);
+    List<String> returningMsgs = new ArrayList<>(msgs.size()+10);
+    Status status = isDone ? FINALIZATION_DONE : FINALIZATION_IN_PROGRESS;
+    while (msgs.size() > 0) {
+      returningMsgs.add(msgs.poll());
+    }
+    return new StatusAndMessages(status, returningMsgs);
+  }
+
+  private void assertClientId(String id) throws OMException {
+    if (!this.clientID.equals(id)) {
+      throw new OMException("Unknown client tries to get finalization status.\n"
+          + "The requestor is not the initiating client of the finalization,"
+          + " if you want to take over, and get unsent status messages, check"
+          + " -takeover option.", INVALID_REQUEST);
+    }
+  }
+
+  /**
+   * This class implements the finalization logic applied to every
+   * LayoutFeature that needs to be finalized.
+   *
+   * For the first approach this happens synchronously within the state machine
+   * during the FinalizeUpgrade request, but ideally this has to be moved to
+   * individual calls that are going into the StateMaching one by one.
+   * The prerequisits for this to happen in the background are the following:
+   * - more fine grained control for LayoutFeatures to prepare the
+   *    finalization outside the state machine, do the switch from old to new
+   *    logic inside the statemachine and apply the finalization, and then do
+   *    any cleanup necessary outside the state machine
+   * - a way to post a request to the state machine that is not part of the
+   *    client API, so basically not an OMRequest, but preferably an internal
+   *    request, which is posted from the leader OM to the follower OMs only.
+   * - ensure that there is a possibility to implement a rollback logic if
+   *    something goes wrong inside the state machine, to avoid OM stuck in an
+   *    intermediate state due to an error.
+   */
+  private class Worker implements Callable<Void> {
+    private OzoneManager ozoneManager;
+
+    /**
+     * Initiates the Worker, for the specified OM instance.
+     * @param om the OzoneManager instance on which to finalize the new
+     *           LayoutFeatures.
+     */
+    Worker(OzoneManager om) {
+      ozoneManager = om;
+    }
+
+    @Override
+    public Void call() throws OMException {
+      try {
+        emitStartingMsg();
+
+        for (OMLayoutFeature f : versionManager.unfinalizedFeatures()) {
+          finalizeFeature(f);
+          updateLayoutVersionInVersionFile(f);
+          versionManager.finalized(f);
+        }
+
+        emitFinishedMsg();
+      } finally {
+        isDone = true;
+      }
+      return null;
+    }
+
+    private void finalizeFeature(OMLayoutFeature feature)
+        throws OMException {
+      OmUpgradeAction action = feature.onFinalizeAction().orElse(NOOP);
+
+      if (action == NOOP) {
+        emitNOOPMsg(feature.name());
+        return;
+      }
+
+      putFinalizationMarkIntoVersionFile(feature);
+
+      emitStartingFinalizationActionMsg(feature.name());
+      try {
+        action.executeAction(ozoneManager);
+      } catch (Exception e) {
+        logFinalizationFailureAndThrow(e, feature.name());
+      }
+      emitFinishFinalizationActionMsg(feature.name());
+
+      removeFinalizationMarkFromVersionFile(feature);
+    }
+
+    private void updateLayoutVersionInVersionFile(OMLayoutFeature feature)
+        throws OMException {
+      int prevLayoutVersion = currentStoredLayoutVersion();
+
+      updateStorageLayoutVersion(feature.layoutVersion());
+      try {
+        persistStorage();
+      } catch (IOException e) {
+        updateStorageLayoutVersion(prevLayoutVersion);
+        logLayoutVersionUpdateFailureAndThrow(e);
+      }
+    }
+
+    private void putFinalizationMarkIntoVersionFile(OMLayoutFeature feature)
+        throws OMException {
+      try {
+        emitUpgradeToLayoutVersionPersistingMsg(feature.name());
+
+        setUpgradeToLayoutVersionInStorage(feature.layoutVersion());
+        persistStorage();
+
+        emitUpgradeToLayoutVersionPersistedMsg();
+      } catch (IOException e) {
+        logUpgradeToLayoutVersionPersistingFailureAndThrow(feature.name(), e);
+      }
+    }
+
+    private void removeFinalizationMarkFromVersionFile(OMLayoutFeature feature)
+        throws OMException {
+      try {
+        emitRemovingUpgradeToLayoutVersionMsg(feature.name());
+
+        unsetUpgradeToLayoutVersionInStorage();
+        persistStorage();
+
+        emitRemovedUpgradeToLayoutVersionMsg();
+      } catch (IOException e) {
+        logUpgradeToLayoutVersionRemovalFailureAndThrow(feature.name(), e);
+      }
+    }
+
+
+
+
+
+    private void setUpgradeToLayoutVersionInStorage(int version) {
+      ozoneManager.getOmStorage().setUpgradeToLayoutVersion(version);
+    }
+
+    private void unsetUpgradeToLayoutVersionInStorage() {
+      ozoneManager.getOmStorage().unsetUpgradeToLayoutVersion();
+    }
+
+    private int currentStoredLayoutVersion() {
+      return ozoneManager.getOmStorage().getLayoutVersion();
+    }
+
+    private void updateStorageLayoutVersion(int version) {
+      ozoneManager.getOmStorage().setLayoutVersion(version);
+    }
+
+    private void persistStorage() throws IOException {

Review comment:
       Can this be moved to the individual layout version update methods above, so that the caller does not need to invoke the persist explicitly every time? As a caller, if I do **storage**.updateStorageLayoutVersion(), I can expect it to be persisted, right? I don't have a strong opinion about this, just a suggestion.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/upgrade/OMFinalizeUpgradeRequest.java
##########
@@ -63,11 +64,20 @@ public OMClientResponse validateAndUpdateCache(
 
       String upgradeClientID = request.getUpgradeClientId();
 
-      UpgradeFinalizationStatus status =
+      StatusAndMessages omStatus =

Review comment:
       Line **54** has a debug log line left.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,328 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.*;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private  static final OmUpgradeAction NOOP = a -> {};
+
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;

Review comment:
       Just an observation. By the time we return 'STARTING_MSG' at 77, we are already done with Finalization right? Is this return message left as it is for a future refactoring through HDDS-3286 where async finalization will be done?




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r499129830



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+    if (versionManager.needsFinalization()) {
+      status = FINALIZATION_REQUIRED;
+    }
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;
+    }
+    assertClientId(upgradeClientID);
+    List<String> returningMsgs = new ArrayList<>(msgs.size()+10);
+    status = isDone ? FINALIZATION_DONE : FINALIZATION_IN_PROGRESS;
+    while (msgs.size() > 0) {
+      returningMsgs.add(msgs.poll());
+    }
+    return new StatusAndMessages(status, returningMsgs);
+  }
+
+  private void assertClientId(String id) throws OMException {
+    if (!this.clientID.equals(id)) {
+      throw new OMException("Unknown client tries to get finalization status.\n"
+          + "The requestor is not the initiating client of the finalization,"
+          + " if you want to take over, and get unsent status messages, check"
+          + " -takeover option.", INVALID_REQUEST);
+    }
+  }
+
+
+
+
+  private class Worker implements Callable<Void> {
+    private OzoneManager ozoneManager;
+
+    Worker(OzoneManager om) {
+      ozoneManager = om;
+    }
+
+    @Override
+    public Void call() throws OMException {
+      try {
+        emitStartingMsg();
+
+        for (OMLayoutFeature f : versionManager.unfinalizedFeatures()) {
+          finalizeFeature(f);
+          updateLayoutVersionInVersionFile(f);
+          versionManager.finalized(f);
+        }
+
+        emitFinishedMsg();
+      } finally {
+        isDone = true;
+      }
+      return null;
+    }
+
+    private void finalizeFeature(OMLayoutFeature feature)
+        throws OMException {
+      OmUpgradeAction action = feature.onFinalizeAction().orElse(NOOP);
+
+      if (action == NOOP) {
+        emitNOOPMsg(feature.name());
+        return;
+      }
+
+      putFinalizationMarkIntoVersionFile(feature);
+
+      emitStartingFinalizationActionMsg(feature.name());
+      action.executeAction(ozoneManager);
+      emitFinishFinalizationActionMsg(feature.name());
+
+      removeFinalizationMarkFromVersionFile(feature);
+    }
+
+    private void updateLayoutVersionInVersionFile(OMLayoutFeature feature)
+        throws OMException {
+      int prevLayoutVersion = currentStoredLayoutVersion();
+
+      updateStorageLayoutVersion(feature.layoutVersion());
+      try {
+        persistStorage();
+      } catch (IOException e) {

Review comment:
       Comment makes sense to me.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r498051832



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+    if (versionManager.needsFinalization()) {
+      status = FINALIZATION_REQUIRED;
+    }
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;
+    }
+    assertClientId(upgradeClientID);
+    List<String> returningMsgs = new ArrayList<>(msgs.size()+10);
+    status = isDone ? FINALIZATION_DONE : FINALIZATION_IN_PROGRESS;
+    while (msgs.size() > 0) {
+      returningMsgs.add(msgs.poll());
+    }
+    return new StatusAndMessages(status, returningMsgs);
+  }
+
+  private void assertClientId(String id) throws OMException {
+    if (!this.clientID.equals(id)) {
+      throw new OMException("Unknown client tries to get finalization status.\n"
+          + "The requestor is not the initiating client of the finalization,"
+          + " if you want to take over, and get unsent status messages, check"
+          + " -takeover option.", INVALID_REQUEST);
+    }
+  }
+
+
+
+
+  private class Worker implements Callable<Void> {
+    private OzoneManager ozoneManager;
+
+    Worker(OzoneManager om) {
+      ozoneManager = om;
+    }
+
+    @Override
+    public Void call() throws OMException {
+      try {
+        emitStartingMsg();
+
+        for (OMLayoutFeature f : versionManager.unfinalizedFeatures()) {
+          finalizeFeature(f);
+          updateLayoutVersionInVersionFile(f);
+          versionManager.finalized(f);
+        }
+
+        emitFinishedMsg();
+      } finally {
+        isDone = true;
+      }
+      return null;
+    }
+
+    private void finalizeFeature(OMLayoutFeature feature)
+        throws OMException {
+      OmUpgradeAction action = feature.onFinalizeAction().orElse(NOOP);
+
+      if (action == NOOP) {
+        emitNOOPMsg(feature.name());
+        return;
+      }
+
+      putFinalizationMarkIntoVersionFile(feature);
+
+      emitStartingFinalizationActionMsg(feature.name());
+      action.executeAction(ozoneManager);
+      emitFinishFinalizationActionMsg(feature.name());
+
+      removeFinalizationMarkFromVersionFile(feature);
+    }
+
+    private void updateLayoutVersionInVersionFile(OMLayoutFeature feature)
+        throws OMException {
+      int prevLayoutVersion = currentStoredLayoutVersion();
+
+      updateStorageLayoutVersion(feature.layoutVersion());
+      try {
+        persistStorage();
+      } catch (IOException e) {

Review comment:
       Do we really need to persist version to storage every time? Why not persist storage only once for the final finalization version? We already update current layout version on memory value (StorageInfo#LAYOUT_VERSION) each time Other place can get this value to know which feature is now on finalizing.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r499129654



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+    if (versionManager.needsFinalization()) {
+      status = FINALIZATION_REQUIRED;
+    }
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;

Review comment:
       To synchronize the whole reportStatus method looks good to me.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] fapifta commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
fapifta commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r498774350



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+    if (versionManager.needsFinalization()) {
+      status = FINALIZATION_REQUIRED;
+    }
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;
+    }
+    assertClientId(upgradeClientID);
+    List<String> returningMsgs = new ArrayList<>(msgs.size()+10);
+    status = isDone ? FINALIZATION_DONE : FINALIZATION_IN_PROGRESS;
+    while (msgs.size() > 0) {
+      returningMsgs.add(msgs.poll());
+    }
+    return new StatusAndMessages(status, returningMsgs);
+  }
+
+  private void assertClientId(String id) throws OMException {
+    if (!this.clientID.equals(id)) {
+      throw new OMException("Unknown client tries to get finalization status.\n"
+          + "The requestor is not the initiating client of the finalization,"
+          + " if you want to take over, and get unsent status messages, check"
+          + " -takeover option.", INVALID_REQUEST);
+    }
+  }
+
+
+
+
+  private class Worker implements Callable<Void> {
+    private OzoneManager ozoneManager;
+
+    Worker(OzoneManager om) {
+      ozoneManager = om;
+    }
+
+    @Override
+    public Void call() throws OMException {
+      try {
+        emitStartingMsg();
+
+        for (OMLayoutFeature f : versionManager.unfinalizedFeatures()) {
+          finalizeFeature(f);
+          updateLayoutVersionInVersionFile(f);
+          versionManager.finalized(f);

Review comment:
       AbstractLayoutVersionManager gives the insurance for us here, as that has a map for the features with the feature's LayoutVersion as a key, and unfinalizedFeatures() works based off of that map.
   So the internal structure, and the overall design should ensure that there can be only one LayoutFeature associated with one LayoutVersion.
   It would be beneficial to work based on the ordinal of the OMLayoutFeature enum, but this is not the case for now, I think that is an improvement we can add there, so that a wrongly defined enum member can not hide an other LayoutFeature, or we can ensure that with a test. I filed HDDS-4303 to track this.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] fapifta commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
fapifta commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r498760977



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+    if (versionManager.needsFinalization()) {
+      status = FINALIZATION_REQUIRED;
+    }
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;
+    }
+    assertClientId(upgradeClientID);
+    List<String> returningMsgs = new ArrayList<>(msgs.size()+10);
+    status = isDone ? FINALIZATION_DONE : FINALIZATION_IN_PROGRESS;
+    while (msgs.size() > 0) {
+      returningMsgs.add(msgs.poll());
+    }
+    return new StatusAndMessages(status, returningMsgs);
+  }
+
+  private void assertClientId(String id) throws OMException {
+    if (!this.clientID.equals(id)) {
+      throw new OMException("Unknown client tries to get finalization status.\n"
+          + "The requestor is not the initiating client of the finalization,"
+          + " if you want to take over, and get unsent status messages, check"
+          + " -takeover option.", INVALID_REQUEST);
+    }
+  }
+
+
+
+
+  private class Worker implements Callable<Void> {

Review comment:
       Added doc to the Worker class as well.

##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/AbstractLayoutVersionManager.java
##########
@@ -65,53 +64,66 @@ protected void initializeFeatures(LayoutFeature[] lfs) {
     });
   }
 
+  protected void reset() {
+    metadataLayoutVersion = 0;
+    softwareLayoutVersion = 0;
+    featureMap.clear();
+    features.clear();
+    isInitialized = false;
+  }
+
+  public void finalized(T layoutFeature) {
+    if (layoutFeature.layoutVersion() > metadataLayoutVersion) {
+      metadataLayoutVersion = layoutFeature.layoutVersion();
+    } else {
+      throw new IllegalArgumentException(
+          "Finalize attempt on a layoutFeature which has already been"
+              + "finalized. Software Layout version: " + softwareLayoutVersion

Review comment:
       fixed




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] avijayanhwx commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r499305029



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+    if (versionManager.needsFinalization()) {
+      status = FINALIZATION_REQUIRED;
+    }
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;
+    }
+    assertClientId(upgradeClientID);
+    List<String> returningMsgs = new ArrayList<>(msgs.size()+10);
+    status = isDone ? FINALIZATION_DONE : FINALIZATION_IN_PROGRESS;
+    while (msgs.size() > 0) {
+      returningMsgs.add(msgs.poll());
+    }
+    return new StatusAndMessages(status, returningMsgs);
+  }
+
+  private void assertClientId(String id) throws OMException {
+    if (!this.clientID.equals(id)) {
+      throw new OMException("Unknown client tries to get finalization status.\n"
+          + "The requestor is not the initiating client of the finalization,"
+          + " if you want to take over, and get unsent status messages, check"
+          + " -takeover option.", INVALID_REQUEST);
+    }
+  }
+
+
+
+
+  private class Worker implements Callable<Void> {
+    private OzoneManager ozoneManager;
+
+    Worker(OzoneManager om) {
+      ozoneManager = om;
+    }
+
+    @Override
+    public Void call() throws OMException {
+      try {
+        emitStartingMsg();
+
+        for (OMLayoutFeature f : versionManager.unfinalizedFeatures()) {
+          finalizeFeature(f);
+          updateLayoutVersionInVersionFile(f);
+          versionManager.finalized(f);

Review comment:
       There is a test that covers this check (in a way) --> TestOMVersionManager#testOMLayoutFeatureCatalog. Maybe, we can build on that.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] fapifta commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
fapifta commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r503559123



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/upgrade/OMFinalizeUpgradeRequest.java
##########
@@ -63,11 +64,20 @@ public OMClientResponse validateAndUpdateCache(
 
       String upgradeClientID = request.getUpgradeClientId();
 
-      UpgradeFinalizationStatus status =
+      StatusAndMessages omStatus =

Review comment:
       Nice catch, it seems I forgot to remove this one. I am pushing the deletion of this line.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] fapifta commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
fapifta commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r498760785



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};

Review comment:
       I moved it to OmUpgradeAction, I believe that is the best place. Thank you for catching this one.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] avijayanhwx commented on a change in pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#discussion_r497516613



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};
+
+  public OMUpgradeFinalizer(OMLayoutVersionManagerImpl versionManager) {
+    this.versionManager = versionManager;
+    if (versionManager.needsFinalization()) {
+      status = FINALIZATION_REQUIRED;
+    }
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID, OzoneManager om)
+      throws IOException {
+    if (!versionManager.needsFinalization()) {
+      return FINALIZED_MSG;
+    }
+    clientID = upgradeClientID;
+
+// This requires some more investigation on how to do it properly while
+// requests are on the fly, and post finalize features one by one.
+// Until that is done, monitoring is not really doing anything meaningful
+// but this is a tradoff we can take for the first iteration either if needed,
+// as the finalization of the first few features should not take that long.
+// Follow up JIRA is in HDDS-4286
+//    String threadName = "OzoneManager-Upgrade-Finalizer";
+//    ExecutorService executor =
+//        Executors.newSingleThreadExecutor(r -> new Thread(threadName));
+//    executor.submit(new Worker(om));
+    new Worker(om).call();
+    return STARTING_MSG;
+  }
+
+  @Override
+  public StatusAndMessages reportStatus(
+      String upgradeClientID, boolean takeover
+  ) throws IOException {
+    if (takeover) {
+      clientID = upgradeClientID;
+    }
+    assertClientId(upgradeClientID);
+    List<String> returningMsgs = new ArrayList<>(msgs.size()+10);
+    status = isDone ? FINALIZATION_DONE : FINALIZATION_IN_PROGRESS;
+    while (msgs.size() > 0) {
+      returningMsgs.add(msgs.poll());
+    }
+    return new StatusAndMessages(status, returningMsgs);
+  }
+
+  private void assertClientId(String id) throws OMException {
+    if (!this.clientID.equals(id)) {
+      throw new OMException("Unknown client tries to get finalization status.\n"
+          + "The requestor is not the initiating client of the finalization,"
+          + " if you want to take over, and get unsent status messages, check"
+          + " -takeover option.", INVALID_REQUEST);
+    }
+  }
+
+
+
+
+  private class Worker implements Callable<Void> {

Review comment:
       Nit. Class needs a Javadoc.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java
##########
@@ -0,0 +1,303 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.upgrade;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS;
+import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED;
+
+/**
+ * UpgradeFinalizer implementation for the Ozone Manager service.
+ */
+public class OMUpgradeFinalizer implements UpgradeFinalizer<OzoneManager> {
+
+  private Status status = ALREADY_FINALIZED;
+  private OMLayoutVersionManagerImpl versionManager;
+  private String clientID;
+
+  private Queue<String> msgs = new ConcurrentLinkedQueue<>();
+  private boolean isDone = false;
+
+  private static final OmUpgradeAction NOOP = a -> {};

Review comment:
       Nit. Can this be moved to the interface for reuse in other implementations? 




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] fapifta commented on pull request #1456: HDDS-4172. Implement Finalize command in Ozone Manager server.

Posted by GitBox <gi...@apache.org>.
fapifta commented on pull request #1456:
URL: https://github.com/apache/hadoop-ozone/pull/1456#issuecomment-702771988


   Guys thank you for the reviews, hopefully I was able to address all the concerns mentioned so far, let me know if you see anything more, and please share your opinion about the syncronization, and about the NOOP, as that seems to be remaining open questions.
   
   I will add unit level tests hopefully by Monday and then will move out the PR from draft.


----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org