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



##########
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:
       Do we need the ReconNewNodeHandler after this change?

##########
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:
       Any reason why we have to always show thse mostly non changing data 
fields? (I believe they were left as isVisible = false and specific users can 
select them if needed)

##########
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 see that the in memory DN details are not seeded with the DB DN 
details on startup. This means that dead datanode information will never show 
up in Recon UI. While SCM is supposed to make use of only 'Live' nodes, Recon 
has to keep track of **all** nodes in the system.




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