symious commented on a change in pull request #2392:
URL: https://github.com/apache/ozone/pull/2392#discussion_r673625149



##########
File path: 
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconNodeManager.java
##########
@@ -173,6 +252,25 @@ protected void updateDatanodeOpState(DatanodeDetails 
reportedDn)
         reportedDn.getPersistedOpStateExpiryEpochSec());
   }
 
+  @Override
+  public RegisteredCommand register(
+      DatanodeDetails datanodeDetails, NodeReportProto nodeReport,
+      PipelineReportsProto pipelineReportsProto,
+      LayoutVersionProto layoutInfo) {
+    inMemDatanodeDetails.put(datanodeDetails.getUuid(), datanodeDetails);
+    if (isNodeRegistered(datanodeDetails)) {
+      try {
+        nodeDB.put(datanodeDetails.getUuid(), datanodeDetails);

Review comment:
       I think it's still needed. For new nodes, it will still need 
ReconNewNodeHandler to add to NodeDB.
   For existing nodes, the `register` here is to keep nodeDB updated.

##########
File path: 
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NodeEndpoint.java
##########
@@ -139,18 +139,18 @@ public Response getDatanodes() {
       }
 
       DatanodeInfo dnInfo = (DatanodeInfo) datanode;
-      datanodes.add(builder.withHostname(hostname)
+      datanodes.add(builder.withHostname(nodeManager.getHostName(datanode))
           .withDatanodeStorageReport(storageReport)
           .withLastHeartbeat(nodeManager.getLastHeartbeat(datanode))
           .withState(nodeState)
           .withOperationalState(nodeOpState)
           .withPipelines(pipelines)
           .withLeaderCount(leaderCount.get())
           .withUUid(datanode.getUuidString())
-          .withVersion(datanode.getVersion())
-          .withSetupTime(datanode.getSetupTime())
-          .withRevision(datanode.getRevision())
-          .withBuildDate(datanode.getBuildDate())
+          .withVersion(nodeManager.getVersion(datanode))

Review comment:
       I saw the original `loadingExistingDB` is calling the method of 
`register` for all existing datanodes, so it will be added to in-mem DB on 
startup.

##########
File path: 
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/datanodes/datanodes.tsx
##########
@@ -240,7 +240,7 @@ const COLUMNS = [
     title: 'Version',
     dataIndex: 'version',
     key: 'version',
-    isVisible: false,

Review comment:
       In fact, I didn't know these values can be selected from the UI, I was 
wondering if it's better to show the information to users directly.
   I think the "setupTime" and "version" are quite good when the user is doing 
an upgrade or maintenance.




-- 
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