dombizita commented on code in PR #5311:
URL: https://github.com/apache/ozone/pull/5311#discussion_r1370148231
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java:
##########
@@ -812,38 +812,30 @@ public static boolean isBucketSnapshotIndicator(String
key) {
return key.startsWith(OM_SNAPSHOT_INDICATOR) && key.split("/").length == 2;
}
- public static String format(List<ServiceInfo> nodes, int port,
- String leaderId) {
- StringBuilder sb = new StringBuilder();
+ public static List<Map<String, String>> format(List<ServiceInfo> nodes, int
port, String leaderId) {
+ List<Map<String, String>> omInfoList = new ArrayList<>();
// Ensuring OM's are printed in correct order
List<ServiceInfo> omNodes = nodes.stream()
.filter(node -> node.getNodeType() == HddsProtos.NodeType.OM)
.sorted(Comparator.comparing(ServiceInfo::getHostname))
.collect(Collectors.toList());
int count = 0;
- 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
- ));
- count++;
- }
- }
- // Print Stand-alone if only one OM exists
- if (count == 1) {
- return "STANDALONE";
- } else {
- return sb.toString();
- }
+ 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";
+ Map<String, String> omInfo = new HashMap<>();
Review Comment:
HashMap does not maintains insertion order in java and later when we are
showing this on the UI we are expecting it that it will be in insertion order.
We should use LinkedHashMap.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -3007,25 +3007,33 @@ public String getRpcPort() {
}
@Override
- public String getRatisRoles() {
+ public List<Map<String, String>> getRatisRoles() {
List<ServiceInfo> serviceList;
+ List<Map<String, String>> resultList = new ArrayList<>();
+ Map<String, String> messageException = new HashMap<>();
int port = omNodeDetails.getRatisPort();
RaftPeer leaderId;
if (isRatisEnabled) {
try {
leaderId = omRatisServer.getLeader();
if (leaderId == null) {
LOG.error("No leader found");
- return "Exception: Not a leader";
+ messageException.put("Message", "Exception: Not a leader");
+ resultList.add(messageException);
+ return resultList;
Review Comment:
This solution could work, but currently if we have an exception, we will
return with this list of 1 map, which only has one element. But in the
`om-overview.html` file we are expecting the 4 attributes of the OM role and we
are explicitly requesting the 2nd and 3rd element's values from the map, so
this won't work this way. Maybe if we only have one element in the map (or if
we switch it to list, one element there) we should handle that on the UI and
show only the message without the table.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java:
##########
@@ -812,38 +812,30 @@ public static boolean isBucketSnapshotIndicator(String
key) {
return key.startsWith(OM_SNAPSHOT_INDICATOR) && key.split("/").length == 2;
}
- public static String format(List<ServiceInfo> nodes, int port,
- String leaderId) {
- StringBuilder sb = new StringBuilder();
+ public static List<Map<String, String>> format(List<ServiceInfo> nodes, int
port, String leaderId) {
+ List<Map<String, String>> omInfoList = new ArrayList<>();
// Ensuring OM's are printed in correct order
List<ServiceInfo> omNodes = nodes.stream()
.filter(node -> node.getNodeType() == HddsProtos.NodeType.OM)
.sorted(Comparator.comparing(ServiceInfo::getHostname))
.collect(Collectors.toList());
int count = 0;
- 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
- ));
- count++;
- }
- }
- // Print Stand-alone if only one OM exists
- if (count == 1) {
- return "STANDALONE";
- } else {
- return sb.toString();
- }
+ 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";
+ Map<String, String> omInfo = new HashMap<>();
Review Comment:
Also I just realised that we won't even use the "keys", we are only using
the "values" from this map, so maybe we could switch it to a list.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java:
##########
@@ -812,38 +812,30 @@ public static boolean isBucketSnapshotIndicator(String
key) {
return key.startsWith(OM_SNAPSHOT_INDICATOR) && key.split("/").length == 2;
}
- public static String format(List<ServiceInfo> nodes, int port,
- String leaderId) {
- StringBuilder sb = new StringBuilder();
+ public static List<Map<String, String>> format(List<ServiceInfo> nodes, int
port, String leaderId) {
+ List<Map<String, String>> omInfoList = new ArrayList<>();
// Ensuring OM's are printed in correct order
List<ServiceInfo> omNodes = nodes.stream()
.filter(node -> node.getNodeType() == HddsProtos.NodeType.OM)
.sorted(Comparator.comparing(ServiceInfo::getHostname))
.collect(Collectors.toList());
int count = 0;
- 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
- ));
- count++;
- }
- }
- // Print Stand-alone if only one OM exists
- if (count == 1) {
- return "STANDALONE";
- } else {
- return sb.toString();
- }
+ 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";
+ Map<String, String> omInfo = new HashMap<>();
+ omInfo.put("hostName", info.getHostname());
+ omInfo.put("nodeId", info.getOmRoleInfo().getNodeId());
+ omInfo.put("ratisPort", String.valueOf(port));
+ omInfo.put("role", role);
+
+ omInfoList.add(omInfo);
+ count++;
+ }
+ }
+ // Return omInfoList if count ==1 or count >=1
+ return omInfoList;
Review Comment:
please fix the indentation here
--
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]
