linyiqun commented on a change in pull request #1486:
URL: https://github.com/apache/hadoop-ozone/pull/1486#discussion_r502802281



##########
File path: 
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/AbstractLayoutVersionManager.java
##########
@@ -107,6 +121,10 @@ public void doFinalize(Object param) {
     }
   }
 
+  public void completeFinalize(Object param) {
+    currentUpgradeState = Finalized;

Review comment:
       param not used here.

##########
File path: 
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/OzoneUpgradeState.java
##########
@@ -0,0 +1,39 @@
+/**
+ * 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.upgrade;
+
+/**
+ * Generic UpgradeState or Ozone.
+ */
+public enum OzoneUpgradeState {
+
+  InValid(0, "Invalid State"),

Review comment:
       Can we use uppercase letter here, like INVALID, PRE_FINALIZE.. FINALIZED.

##########
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
##########
@@ -400,6 +405,66 @@ public void processNodeReport(DatanodeDetails 
datanodeDetails,
     }
   }
 
+  /**
+   * Process Layout Version report.
+   *
+   * @param datanodeDetails
+   * @param layoutVersionReport
+   */
+  @Override
+  public void processLayoutVersionReport(DatanodeDetails datanodeDetails,
+                                LayoutVersionProto layoutVersionReport) {
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("Processing Layout Version report from [datanode={}]",
+          datanodeDetails.getHostName());
+    }
+    if (LOG.isTraceEnabled()) {
+      LOG.trace("HB is received from [datanode={}]: <json>{}</json>",
+          datanodeDetails.getHostName(),
+          layoutVersionReport.toString().replaceAll("\n", "\\\\n"));
+    }
+
+    if (layoutVersionReport != null) {
+      int scmSlv = scmLayoutVersionManager.getSoftwareLayoutVersion();
+      int scmMlv = scmLayoutVersionManager.getMetadataLayoutVersion();
+      OzoneUpgradeState scmUpgradeState =
+          scmLayoutVersionManager.getUpgradeState();
+      int dnSlv = layoutVersionReport.getSoftwareLayoutVersion();
+      int dnMlv = layoutVersionReport.getMetadataLayoutVersion();
+
+      // If the data node slv is > scm slv => log error condition
+      if (dnSlv > scmSlv) {
+        LOG.error("Rogue data node {} in the cluster. " +
+                "DataNode SoftwareLayoutVersion = {}, SCM " +
+                "SoftwareLayoutVersion = {}",
+            datanodeDetails.getHostName(), dnSlv, scmSlv);

Review comment:
       Can we put this node into exclude list temporarily? I mean SCM should 
take some action when it finds some unexpected version DNs.

##########
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
##########
@@ -400,6 +405,66 @@ public void processNodeReport(DatanodeDetails 
datanodeDetails,
     }
   }
 
+  /**
+   * Process Layout Version report.
+   *
+   * @param datanodeDetails
+   * @param layoutVersionReport
+   */
+  @Override
+  public void processLayoutVersionReport(DatanodeDetails datanodeDetails,
+                                LayoutVersionProto layoutVersionReport) {
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("Processing Layout Version report from [datanode={}]",
+          datanodeDetails.getHostName());
+    }
+    if (LOG.isTraceEnabled()) {
+      LOG.trace("HB is received from [datanode={}]: <json>{}</json>",
+          datanodeDetails.getHostName(),
+          layoutVersionReport.toString().replaceAll("\n", "\\\\n"));
+    }
+
+    if (layoutVersionReport != null) {
+      int scmSlv = scmLayoutVersionManager.getSoftwareLayoutVersion();
+      int scmMlv = scmLayoutVersionManager.getMetadataLayoutVersion();
+      OzoneUpgradeState scmUpgradeState =
+          scmLayoutVersionManager.getUpgradeState();
+      int dnSlv = layoutVersionReport.getSoftwareLayoutVersion();
+      int dnMlv = layoutVersionReport.getMetadataLayoutVersion();
+
+      // If the data node slv is > scm slv => log error condition
+      if (dnSlv > scmSlv) {
+        LOG.error("Rogue data node {} in the cluster. " +
+                "DataNode SoftwareLayoutVersion = {}, SCM " +
+                "SoftwareLayoutVersion = {}",
+            datanodeDetails.getHostName(), dnSlv, scmSlv);
+      }
+
+      // If the datanode slv < scm slv, it can not be allowed to be part of
+      // any pipeline. However it can be allowed to join the cluster
+      if (dnMlv < scmMlv) {
+        LOG.warn("Data node {} can not be used in any pipeline in the " +
+                "cluster. " + "DataNode MetadataLayoutVersion = {}, SCM " +
+                "MetadataLayoutVersion = {}",
+            datanodeDetails.getHostName(), dnMlv, scmMlv);
+
+        // TBD: Add NEED_UPGRADE state and fill out state transitions
+        // around this state. Fire event to move this data node to
+        // NEED_UPGRADE state. The DataNode will be considered HEALTHY in
+        // this state but it can not be made part of any Pipeline.
+
+        // Also send Finalize command to the data node. Its OK to
+        // send Finalize command multiple times.
+        scmNodeEventPublisher.fireEvent(SCMEvents.DATANODE_COMMAND,
+            new CommandForDatanode<>(datanodeDetails.getUuid(),
+                new FinalizeNewLayoutVersionCommand(true,
+                    LayoutVersionProto.newBuilder()
+                        .setSoftwareLayoutVersion(dnSlv)
+                        .setMetadataLayoutVersion(dnSlv).build())));

Review comment:
       Here the dnSlv is returned in finalize command, is that right? should be 
scmSlv/scmMlv?

##########
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/LayoutVersionReportHandler.java
##########
@@ -0,0 +1,54 @@
+/**
+ * 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.hdds.scm.node;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import 
org.apache.hadoop.hdds.scm.server.SCMDatanodeHeartbeatDispatcher.LayoutReportFromDatanode;
+import org.apache.hadoop.hdds.server.events.EventHandler;
+import org.apache.hadoop.hdds.server.events.EventPublisher;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Handles Node Layout Version Reports from DataNode.
+ */
+public class LayoutVersionReportHandler implements
+    EventHandler<LayoutReportFromDatanode> {
+
+  private static final Logger LOGGER = LoggerFactory
+      .getLogger(NodeReportHandler.class);

Review comment:
       Wrong log instance name used.




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



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

Reply via email to