Re: [PR] HDDS-9303. Ozone Manager : Display leader information in table format and highlight current node. [ozone]

["dombizita (via GitHub)" <gi...@apache.org>]

dombizita commented on code in PR #5311:
URL: https://github.com/apache/ozone/pull/5311#discussion_r1379806700


##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java:
##########
@@ -824,26 +823,18 @@ public static String format(List<ServiceInfo> nodes, int port,
     for (ServiceInfo info : omNodes) {
       // Printing only the OM's running
       if (info.getNodeType() == HddsProtos.NodeType.OM) {
-        String role =
-            info.getOmRoleInfo().getNodeId().equals(leaderId) ? "LEADER" :
-                "FOLLOWER";
-        sb.append(
-            String.format(
-                " { HostName: %s | Node-Id: %s | Ratis-Port : %d | Role: %s} ",
-                info.getHostname(),
-                info.getOmRoleInfo().getNodeId(),
-                port,
-                role
-            ));
+        String role = info.getOmRoleInfo().getNodeId().equals(leaderId) ? "LEADER" : "FOLLOWER";
+        List<String> omInfo = new ArrayList<>();
+        omInfo.add(info.getHostname());
+        omInfo.add(info.getOmRoleInfo().getNodeId());
+        omInfo.add(String.valueOf(port));
+        omInfo.add(role);
+        omInfoList.add(omInfo);
         count++;
       }
     }
-    // Print Stand-alone if only one OM exists
-    if (count == 1) {
-      return "STANDALONE";
-    } else {
-      return sb.toString();
-    }
+    // Return omInfoList if count ==1 or count >=1

Review Comment:
   I don't think this comment is necessary. 
   ```suggestion
   ```



##########
hadoop-ozone/ozone-manager/src/main/resources/webapps/ozoneManager/om-overview.html:
##########
@@ -49,6 +45,37 @@ <h2>Status</h2>
     </tbody>
 </table>
 
+<h4 ng-show="$ctrl.overview.jmx.RatisRoles[0] == 'Exception: Not a Leader' || $ctrl.overview.jmx.RatisRoles[0] == 'IO-Exception Occurred' || $ctrl.overview.jmx.RatisRoles[0] == 'Ratis Disabled'">Exception: {{$ctrl.overview.jmx.RatisRoles[0]}}</h4>
+<div ng-hide="$ctrl.overview.jmx.RatisRoles[0] == 'Exception: Not a Leader' || $ctrl.overview.jmx.RatisRoles[0] == 'IO-Exception Occurred' || $ctrl.overview.jmx.RatisRoles[0] == 'Ratis Disabled'">

Review Comment:
   I think it would be better if we could check the size of `RatisRoles` and do the visualisation based on that:
   
   - If we have one element in `RatisRoles` and inside that nested list only one more (we can check `$ctrl.overview.jmx.RatisRoles[0].length`), we can assume that we have an exception, that we can show as one line and no table.
   - If we have one element in `RatisRoles`, but we have more inside that, we can say that we have only one OM. In that case we don't want to show anything (not even the title of the table), as all the information is already there in the _Status_ above this. We only want to show information if we have OM HA.
   - If we have multiple elements in `RatisRoles`, then we can think the we have multiple OMs, so we can show that in the table as it is already happening. 



-- 
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: issues-unsubscribe@ozone.apache.org

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