sodonnel commented on code in PR #3329:
URL: https://github.com/apache/ozone/pull/3329#discussion_r861616728


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java:
##########
@@ -272,6 +282,57 @@ public void setNodeStatus(NodeStatus newNodeStatus) {
     }
   }
 
+  /**
+   * Set the current command counts for this datanode, as reported in the last
+   * heartbeat.
+   * @param cmds Proto message containing a list of command count pairs.
+   */
+  public void setCommandCounts(CommandQueueReportProto cmds) {
+    try {
+      int count = cmds.getCommandCount();
+      lock.writeLock().lock();
+      for (int i = 0; i < count; i++) {
+        SCMCommandProto.Type command = cmds.getCommand(i);
+        if (command == SCMCommandProto.Type.unknownScmCommand) {
+          LOG.warn("Unknown SCM Command received from {} in the "
+              + "heartbeat. SCM and the DN may not be at the same version.",
+              this);
+          continue;
+        }
+        int cmdCount = cmds.getCount(i);
+        if (cmdCount < 0) {
+          LOG.warn("Command count of {} from {} should be greater than zero. " 
+
+              "Setting it to zero", cmdCount, this);
+          cmdCount = 0;
+        }
+        commandCounts.put(command, cmdCount);
+      }
+    } finally {
+      lock.writeLock().unlock();
+    }
+  }
+
+  /**
+   * Retrieve the number of queued commands of the given type, as reported by
+   * the datanode at the last heartbeat.
+   * @param cmd The command for which to receive the queued command count

Review Comment:
   -1 means we have not received any data yet. In the case of an upgrade adding 
a new command (eg SCM upgraded with a new command, but some DNs not upgraded) 
those DNs will always show a -1 for the new command.
   
   I am not sure how we should handle this - possibly we need a fallback 
position in any code that uses these counts. If it is "-1" then we need to use 
some other way of limiting the commands sent. The upgrade scenario should be 
short lived, and then DNs should only have -1 until their first heartbeat.
   
   I just thought it was a good idea to include -1 as a different state than 
zero, so we can tell the difference between the two.



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to