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/12/16 20:43:36 UTC

[GitHub] [ozone] prashantpogde opened a new pull request #1720: Implement Datanode Finalization.

prashantpogde opened a new pull request #1720:
URL: https://github.com/apache/ozone/pull/1720


   ## What changes were proposed in this pull request?
   
   Implement Datanode Finalization
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/HDDS-4175
   
   ## How was this patch tested?
   
   Build/UT. I will address CI failures if any.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] prashantpogde commented on a change in pull request #1720: HDDS-4175. Implement Datanode Finalization.

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1720:
URL: https://github.com/apache/ozone/pull/1720#discussion_r553763593



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/upgrade/HDDSLayoutFeatureCatalog.java
##########
@@ -39,7 +40,11 @@
 
     private int layoutVersion;
     private String description;
-    private Optional< ? extends HDDSUpgradeAction> hddsUpgradeAction =
+
+    private Optional<? extends HDDSUpgradeAction> scmUpgradeAction =
+        Optional.empty();
+
+    private Optional<? extends HDDSUpgradeAction> datanodeUpgradeAction =

Review comment:
       Yes. I have updated the PR to add datanode action and scm action. There is an integration test for this, thats working but thats in a separate PR for integration test.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] avijayanhwx commented on a change in pull request #1720: HDDS-4175. Implement Datanode Finalization.

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



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/upgrade/HDDSLayoutFeatureCatalog.java
##########
@@ -39,7 +40,11 @@
 
     private int layoutVersion;
     private String description;
-    private Optional< ? extends HDDSUpgradeAction> hddsUpgradeAction =
+
+    private Optional<? extends HDDSUpgradeAction> scmUpgradeAction =
+        Optional.empty();
+
+    private Optional<? extends HDDSUpgradeAction> datanodeUpgradeAction =

Review comment:
       @prashantpogde I am ok with that. If we are going to add these No-Op actions for the first upgrade release, I would suggest renaming the action to something more intuitive rather than "DatanodeAction1" and "SCMAction1". I am OK with doing these changes in a follow up JIRA.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] avijayanhwx commented on a change in pull request #1720: HDDS-4175. Implement Datanode Finalization.

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



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/Storage.java
##########
@@ -145,7 +145,7 @@ protected StorageInfo getStorageInfo() {
   abstract protected Properties getNodeProperties();
 
   /**
-   * Sets the Node properties specific to OM/SCM.
+   * Sets the Node properties specific to OM/SCM/DataNode.

Review comment:
       Can we add a unit test for the Datanode layout version startup validation like in TestScmStartupSlvLessThanMlv, TestOmStartupSlvLessThanMlv? 

##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/BasicUpgradeFinalizer.java
##########
@@ -71,6 +69,11 @@ public synchronized StatusAndMessages preFinalize(String upgradeClientID,
       return FINALIZATION_IN_PROGRESS_MSG;
     case FINALIZATION_DONE:
     case ALREADY_FINALIZED:
+      if (versionManager.needsFinalization()) {

Review comment:
       Can you please explain how this can happen, and how can a user recover from this? 

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/DataNodeStorageConfig.java
##########
@@ -0,0 +1,77 @@
+/**
+ * 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.container.common;
+
+import static org.apache.hadoop.ozone.OzoneConsts.DATANODE_STORAGE_DIR;
+import static org.apache.hadoop.ozone.OzoneConsts.DATANODE_UUID;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Properties;
+import java.util.UUID;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeType;
+import org.apache.hadoop.hdds.server.ServerUtils;
+import org.apache.hadoop.ozone.common.Storage;
+
+/**
+ * DataNodeStorageConfig is responsible for management of the
+ * StorageDirectories used by the DataNode.
+ */
+public class DataNodeStorageConfig extends Storage {

Review comment:
       The StorageInfo class generates a new "clusterId" for every Version file that it writes down. There is no clusterId with respect to DN now. Can we confirm the implications of adding a clusterId for the DN persistent state even when we need only the layout version? 

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeUpgradeFinalizer.java
##########
@@ -0,0 +1,99 @@
+/**
+ * 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.container.upgrade;
+
+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;
+
+import java.io.IOException;
+import java.util.concurrent.Callable;
+
+import org.apache.hadoop.hdds.upgrade.HDDSLayoutFeatureCatalog.HDDSLayoutFeature;
+import org.apache.hadoop.ozone.container.common.statemachine.DatanodeStateMachine;
+import org.apache.hadoop.ozone.upgrade.BasicUpgradeFinalizer;
+
+/**
+ * UpgradeFinalizer for the DataNode.
+ */
+public class DataNodeUpgradeFinalizer extends
+    BasicUpgradeFinalizer<DatanodeStateMachine, DataNodeLayoutVersionManager> {
+
+  public DataNodeUpgradeFinalizer(DataNodeLayoutVersionManager versionManager) {
+    super(versionManager);
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID,
+                                    DatanodeStateMachine dsm)
+      throws IOException {
+    StatusAndMessages response = preFinalize(upgradeClientID, dsm);
+    if (response.status() != FINALIZATION_REQUIRED) {
+      return response;
+    }
+    new Worker(dsm).call();
+    return STARTING_MSG;
+  }
+
+  private class Worker implements Callable<Void> {
+    private DatanodeStateMachine datanodeStateMachine;
+
+    /**
+     * Initiates the Worker, for the specified DataNode instance.
+     * @param dsm the DataNodeStateMachine instance on which to finalize the
+     *           new LayoutFeatures.
+     */
+    Worker(DatanodeStateMachine dsm) {
+      datanodeStateMachine = dsm;
+    }
+
+    @Override
+    public Void call() throws IOException {
+      if(!datanodeStateMachine.preFinalizeUpgrade()) {
+      // datanode is not yet ready to finalize.
+      // Reset the Finalization state.
+        versionManager.setUpgradeState(FINALIZATION_REQUIRED);
+        return null;
+      }
+      try {
+        emitStartingMsg();
+        versionManager.setUpgradeState(FINALIZATION_IN_PROGRESS);
+        /*
+         * Before we can call finalize the feature, we need to make sure that
+         * all existing pipelines are closed and pipeline Manger would freeze
+         * all new pipeline creation.
+         */
+
+        for (HDDSLayoutFeature f : versionManager.unfinalizedFeatures()) {
+          finalizeFeature(f, datanodeStateMachine.getDataNodeStorageConfig());

Review comment:
       We currently have a common layout feature hierarchy for SCM and Datanode. It is likely that a feature can either be part of the SCM or the DN. If we call on the onFinalize action in both the SCM and DN for every unfinalized HDDS layout feature, then we may be left with unintended consequences of trying to call on finalize action on an SCM feature on the DN. We should either have a marker on every layout feature to say if it belongs to SCM or DN, or not support finalization actions for DN for the V1 upgrade implementation. 

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeLayoutVersionManager.java
##########
@@ -69,41 +58,15 @@ public static synchronized LayoutVersionManager getInstance() {
   /**
    * Initialize DataNode version manager from version file stored on the
    * DataNode.
-   * @param conf - Ozone Configuration
+   * @param dataNodeStorage - DataNode storage config
    * @return version manager instance.
    */
+
   public static synchronized DataNodeLayoutVersionManager initialize(
-      ConfigurationSource conf)
-      throws IOException {
+      Storage dataNodeStorage) throws IOException {
     if (dataNodeLayoutVersionManager == null) {
       dataNodeLayoutVersionManager = new DataNodeLayoutVersionManager();
-      int layoutVersion = 0;
-      Collection<String> rawLocations = getDatanodeStorageDirs(conf);
-      for (String locationString : rawLocations) {
-        StorageLocation location = StorageLocation.parse(locationString);
-        File hddsRootDir = new File(location.getUri().getPath(),
-            HDDS_VOLUME_DIR);
-        // Read the version from VersionFile Stored on the data node.
-        File versionFile = HddsVolumeUtil.getVersionFile(hddsRootDir);
-        if (!versionFile.exists()) {
-          // Volume Root is non empty but VERSION file does not exist.
-          LOG.warn("VERSION file does not exist in volume {},"
-                  + " current volume state: {}.",
-              hddsRootDir.getPath(), HddsVolume.VolumeState.INCONSISTENT);
-          continue;
-        } else {
-          LOG.debug("Reading version file {} from disk.", versionFile);
-        }
-        Properties props = DatanodeVersionFile.readFrom(versionFile);
-        if (props.isEmpty()) {
-          continue;
-        }
-        int storedVersion = HddsVolumeUtil.getLayOutVersion(props, versionFile);
-        if (storedVersion > layoutVersion) {
-          layoutVersion = storedVersion;
-        }
-      }
-      dataNodeLayoutVersionManager.init(layoutVersion,
+      dataNodeLayoutVersionManager.init(dataNodeStorage.getLayoutVersion(),

Review comment:
       With these changes, I believe that _HDDSLayoutFeature#FIRST_UPGRADE_VERSION_ can be removed.

##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
##########
@@ -42,6 +42,7 @@
 
   public static final String STORAGE_ID = "storageID";
   public static final String DATANODE_UUID = "datanodeUuid";
+  public static final String DATANODE_STORAGE_DIR = "datanodeStorageConfig";

Review comment:
       There is also a "datanode.id" file being created in the Datanode that looks like a metadata yaml file. Can we look into whether we can add our layout version into that file? Reference -> _org.apache.hadoop.hdds.utils.HddsServerUtil#getDatanodeIdFilePath_




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] avijayanhwx commented on a change in pull request #1720: HDDS-4175. Implement Datanode Finalization.

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



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/upgrade/HDDSLayoutFeatureCatalog.java
##########
@@ -39,7 +40,11 @@
 
     private int layoutVersion;
     private String description;
-    private Optional< ? extends HDDSUpgradeAction> hddsUpgradeAction =
+
+    private Optional<? extends HDDSUpgradeAction> scmUpgradeAction =
+        Optional.empty();
+
+    private Optional<? extends HDDSUpgradeAction> datanodeUpgradeAction =

Review comment:
       I am wondering whether the organization of modules will cause a problem here. The DatanodeStateMachine class is in container-service module, while this class (HDDSLayoutFeatureCatalog) is in hdds-common. The common module is usually shared across the the more specific modules. Where does one create an upgrade action for the Datanode? If the action is in container-service, then it cannot be accessed in hdds-common, if the action is in common, then it cannot access the DatanodeStateMachine class.

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeUpgradeAction.java
##########
@@ -0,0 +1,31 @@
+/**
+ * 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.container.upgrade;
+
+import org.apache.hadoop.hdds.upgrade.HDDSUpgradeAction;
+import org.apache.hadoop.ozone.container.common.statemachine.DatanodeStateMachine;
+
+/**
+ * Upgrade Action for DataNode which takes in a 'DataNodeStateMachine' instance.
+ */
+public class DataNodeUpgradeAction extends

Review comment:
       Can we have DataNodeUpgradeAction and SCMUpgradeAction as an interface or abstract class?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] prashantpogde commented on a change in pull request #1720: HDDS-4175. Implement Datanode Finalization.

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1720:
URL: https://github.com/apache/ozone/pull/1720#discussion_r546886234



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/BasicUpgradeFinalizer.java
##########
@@ -71,6 +69,11 @@ public synchronized StatusAndMessages preFinalize(String upgradeClientID,
       return FINALIZATION_IN_PROGRESS_MSG;
     case FINALIZATION_DONE:
     case ALREADY_FINALIZED:
+      if (versionManager.needsFinalization()) {

Review comment:
       it cant happen. just trying to catch if any future modification messes this up.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] avijayanhwx commented on a change in pull request #1720: HDDS-4175. Implement Datanode Finalization.

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



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/upgrade/HDDSLayoutFeatureCatalog.java
##########
@@ -39,7 +40,11 @@
 
     private int layoutVersion;
     private String description;
-    private Optional< ? extends HDDSUpgradeAction> hddsUpgradeAction =
+
+    private Optional<? extends HDDSUpgradeAction> scmUpgradeAction =
+        Optional.empty();
+
+    private Optional<? extends HDDSUpgradeAction> datanodeUpgradeAction =

Review comment:
       New approach looks good to me. Can we fix the CI issues? Also, do we need to include the sample SCM and DN action that you have added in the latest commit? Maybe we could use a different implementation that accepts an empty list (enum does not). 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] prashantpogde commented on pull request #1720: HDDS-4175. Implement Datanode Finalization.

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on pull request #1720:
URL: https://github.com/apache/ozone/pull/1720#issuecomment-757360804


   these CI failures are unrelated.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] avijayanhwx commented on a change in pull request #1720: HDDS-4175. Implement Datanode Finalization.

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



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/upgrade/HDDSLayoutFeatureCatalog.java
##########
@@ -39,7 +40,11 @@
 
     private int layoutVersion;
     private String description;
-    private Optional< ? extends HDDSUpgradeAction> hddsUpgradeAction =
+
+    private Optional<? extends HDDSUpgradeAction> scmUpgradeAction =
+        Optional.empty();
+
+    private Optional<? extends HDDSUpgradeAction> datanodeUpgradeAction =

Review comment:
       New approach looks good to me. Can we fix the CI issues? Also, do we need to include the sample SCM and DN action that you have added in the latest commit?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] prashantpogde commented on a change in pull request #1720: HDDS-4175. Implement Datanode Finalization.

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1720:
URL: https://github.com/apache/ozone/pull/1720#discussion_r546885782



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/DataNodeStorageConfig.java
##########
@@ -0,0 +1,77 @@
+/**
+ * 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.container.common;
+
+import static org.apache.hadoop.ozone.OzoneConsts.DATANODE_STORAGE_DIR;
+import static org.apache.hadoop.ozone.OzoneConsts.DATANODE_UUID;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Properties;
+import java.util.UUID;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeType;
+import org.apache.hadoop.hdds.server.ServerUtils;
+import org.apache.hadoop.ozone.common.Storage;
+
+/**
+ * DataNodeStorageConfig is responsible for management of the
+ * StorageDirectories used by the DataNode.
+ */
+public class DataNodeStorageConfig extends Storage {

Review comment:
       It will remain a "don't care bit". storage config is used only for layout information.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] prashantpogde commented on a change in pull request #1720: HDDS-4175. Implement Datanode Finalization.

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1720:
URL: https://github.com/apache/ozone/pull/1720#discussion_r554289867



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/upgrade/HDDSLayoutFeatureCatalog.java
##########
@@ -39,7 +40,11 @@
 
     private int layoutVersion;
     private String description;
-    private Optional< ? extends HDDSUpgradeAction> hddsUpgradeAction =
+
+    private Optional<? extends HDDSUpgradeAction> scmUpgradeAction =
+        Optional.empty();
+
+    private Optional<? extends HDDSUpgradeAction> datanodeUpgradeAction =

Review comment:
       Yes, uploaded the new changes to address CI failures. We can leave sample SCM and DN actions to be used as an example in future upgrades. This also makes it a self validating release.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] prashantpogde commented on a change in pull request #1720: HDDS-4175. Implement Datanode Finalization.

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1720:
URL: https://github.com/apache/ozone/pull/1720#discussion_r554289867



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/upgrade/HDDSLayoutFeatureCatalog.java
##########
@@ -39,7 +40,11 @@
 
     private int layoutVersion;
     private String description;
-    private Optional< ? extends HDDSUpgradeAction> hddsUpgradeAction =
+
+    private Optional<? extends HDDSUpgradeAction> scmUpgradeAction =
+        Optional.empty();
+
+    private Optional<? extends HDDSUpgradeAction> datanodeUpgradeAction =

Review comment:
       Yes, uploaded the new changes to address CI failures. We can leave sample SCM and DN actions to be used as an example in future upgrades. This also makes it a self validating release.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] prashantpogde commented on pull request #1720: HDDS-4175. Implement Datanode Finalization.

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on pull request #1720:
URL: https://github.com/apache/ozone/pull/1720#issuecomment-757360804


   these CI failures are unrelated.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] prashantpogde commented on a change in pull request #1720: HDDS-4175. Implement Datanode Finalization.

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1720:
URL: https://github.com/apache/ozone/pull/1720#discussion_r551475478



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
##########
@@ -42,6 +42,7 @@
 
   public static final String STORAGE_ID = "storageID";
   public static final String DATANODE_UUID = "datanodeUuid";
+  public static final String DATANODE_STORAGE_DIR = "datanodeStorageConfig";

Review comment:
       yup, storage config is used just for the upgrade and nothing else. We will continue to use datanode.id file as it is.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] avijayanhwx commented on a change in pull request #1720: HDDS-4175. Implement Datanode Finalization.

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



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/upgrade/HDDSLayoutFeatureCatalog.java
##########
@@ -39,7 +40,11 @@
 
     private int layoutVersion;
     private String description;
-    private Optional< ? extends HDDSUpgradeAction> hddsUpgradeAction =
+
+    private Optional<? extends HDDSUpgradeAction> scmUpgradeAction =
+        Optional.empty();
+
+    private Optional<? extends HDDSUpgradeAction> datanodeUpgradeAction =

Review comment:
       I am wondering whether the organization of modules will cause a problem here. The DatanodeStateMachine class is in container-service module, while this class (HDDSLayoutFeatureCatalog) is in hdds-common. The common module is usually shared across the more specific modules. Where does one create an upgrade action for the Datanode? If the action is in container-service, then it cannot be accessed in hdds-common, if the action is in common, then it cannot access the DatanodeStateMachine class.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] prashantpogde commented on a change in pull request #1720: HDDS-4175. Implement Datanode Finalization.

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1720:
URL: https://github.com/apache/ozone/pull/1720#discussion_r552407409



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeUpgradeFinalizer.java
##########
@@ -0,0 +1,99 @@
+/**
+ * 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.container.upgrade;
+
+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;
+
+import java.io.IOException;
+import java.util.concurrent.Callable;
+
+import org.apache.hadoop.hdds.upgrade.HDDSLayoutFeatureCatalog.HDDSLayoutFeature;
+import org.apache.hadoop.ozone.container.common.statemachine.DatanodeStateMachine;
+import org.apache.hadoop.ozone.upgrade.BasicUpgradeFinalizer;
+
+/**
+ * UpgradeFinalizer for the DataNode.
+ */
+public class DataNodeUpgradeFinalizer extends
+    BasicUpgradeFinalizer<DatanodeStateMachine, DataNodeLayoutVersionManager> {
+
+  public DataNodeUpgradeFinalizer(DataNodeLayoutVersionManager versionManager) {
+    super(versionManager);
+  }
+
+  @Override
+  public StatusAndMessages finalize(String upgradeClientID,
+                                    DatanodeStateMachine dsm)
+      throws IOException {
+    StatusAndMessages response = preFinalize(upgradeClientID, dsm);
+    if (response.status() != FINALIZATION_REQUIRED) {
+      return response;
+    }
+    new Worker(dsm).call();
+    return STARTING_MSG;
+  }
+
+  private class Worker implements Callable<Void> {
+    private DatanodeStateMachine datanodeStateMachine;
+
+    /**
+     * Initiates the Worker, for the specified DataNode instance.
+     * @param dsm the DataNodeStateMachine instance on which to finalize the
+     *           new LayoutFeatures.
+     */
+    Worker(DatanodeStateMachine dsm) {
+      datanodeStateMachine = dsm;
+    }
+
+    @Override
+    public Void call() throws IOException {
+      if(!datanodeStateMachine.preFinalizeUpgrade()) {
+      // datanode is not yet ready to finalize.
+      // Reset the Finalization state.
+        versionManager.setUpgradeState(FINALIZATION_REQUIRED);
+        return null;
+      }
+      try {
+        emitStartingMsg();
+        versionManager.setUpgradeState(FINALIZATION_IN_PROGRESS);
+        /*
+         * Before we can call finalize the feature, we need to make sure that
+         * all existing pipelines are closed and pipeline Manger would freeze
+         * all new pipeline creation.
+         */
+
+        for (HDDSLayoutFeature f : versionManager.unfinalizedFeatures()) {
+          finalizeFeature(f, datanodeStateMachine.getDataNodeStorageConfig());

Review comment:
       yes. I added separate SCM and datanode actions. Please take a look.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] prashantpogde commented on a change in pull request #1720: HDDS-4175. Implement Datanode Finalization.

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1720:
URL: https://github.com/apache/ozone/pull/1720#discussion_r551474954



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeLayoutVersionManager.java
##########
@@ -69,41 +58,15 @@ public static synchronized LayoutVersionManager getInstance() {
   /**
    * Initialize DataNode version manager from version file stored on the
    * DataNode.
-   * @param conf - Ozone Configuration
+   * @param dataNodeStorage - DataNode storage config
    * @return version manager instance.
    */
+
   public static synchronized DataNodeLayoutVersionManager initialize(
-      ConfigurationSource conf)
-      throws IOException {
+      Storage dataNodeStorage) throws IOException {
     if (dataNodeLayoutVersionManager == null) {
       dataNodeLayoutVersionManager = new DataNodeLayoutVersionManager();
-      int layoutVersion = 0;
-      Collection<String> rawLocations = getDatanodeStorageDirs(conf);
-      for (String locationString : rawLocations) {
-        StorageLocation location = StorageLocation.parse(locationString);
-        File hddsRootDir = new File(location.getUri().getPath(),
-            HDDS_VOLUME_DIR);
-        // Read the version from VersionFile Stored on the data node.
-        File versionFile = HddsVolumeUtil.getVersionFile(hddsRootDir);
-        if (!versionFile.exists()) {
-          // Volume Root is non empty but VERSION file does not exist.
-          LOG.warn("VERSION file does not exist in volume {},"
-                  + " current volume state: {}.",
-              hddsRootDir.getPath(), HddsVolume.VolumeState.INCONSISTENT);
-          continue;
-        } else {
-          LOG.debug("Reading version file {} from disk.", versionFile);
-        }
-        Properties props = DatanodeVersionFile.readFrom(versionFile);
-        if (props.isEmpty()) {
-          continue;
-        }
-        int storedVersion = HddsVolumeUtil.getLayOutVersion(props, versionFile);
-        if (storedVersion > layoutVersion) {
-          layoutVersion = storedVersion;
-        }
-      }
-      dataNodeLayoutVersionManager.init(layoutVersion,
+      dataNodeLayoutVersionManager.init(dataNodeStorage.getLayoutVersion(),

Review comment:
       As discussed in the last call. Let us keep it as it can be used to test the upgrade feature itself when we role out this feature. It would be a self validating upgrade.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] prashantpogde commented on a change in pull request #1720: HDDS-4175. Implement Datanode Finalization.

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1720:
URL: https://github.com/apache/ozone/pull/1720#discussion_r551475969



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/Storage.java
##########
@@ -145,7 +145,7 @@ protected StorageInfo getStorageInfo() {
   abstract protected Properties getNodeProperties();
 
   /**
-   * Sets the Node properties specific to OM/SCM.
+   * Sets the Node properties specific to OM/SCM/DataNode.

Review comment:
       yes




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] avijayanhwx merged pull request #1720: HDDS-4175. Implement Datanode Finalization.

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


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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


[GitHub] [ozone] prashantpogde commented on a change in pull request #1720: HDDS-4175. Implement Datanode Finalization.

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1720:
URL: https://github.com/apache/ozone/pull/1720#discussion_r553763321



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeUpgradeAction.java
##########
@@ -0,0 +1,31 @@
+/**
+ * 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.container.upgrade;
+
+import org.apache.hadoop.hdds.upgrade.HDDSUpgradeAction;
+import org.apache.hadoop.ozone.container.common.statemachine.DatanodeStateMachine;
+
+/**
+ * Upgrade Action for DataNode which takes in a 'DataNodeStateMachine' instance.
+ */
+public class DataNodeUpgradeAction extends

Review comment:
       Yes, will do




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



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