You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "smitajoshi12 (via GitHub)" <gi...@apache.org> on 2023/09/18 13:43:39 UTC
[GitHub] [ozone] smitajoshi12 opened a new pull request, #5311: HDDS-9303. Ozone Manager : Add Ozone leader information on the Ozone manager UI
smitajoshi12 opened a new pull request, #5311:
URL: https://github.com/apache/ozone/pull/5311
## What changes were proposed in this pull request?
SCM web UI displays the leader information on the webUI and it makes debugging easier. Need to do similar in Ozone Manager.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9303
## How was this patch tested?
Manually
New Changes
--
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
[GitHub] [ozone] devmadhuu commented on pull request #5311: HDDS-9303. Ozone Manager : Add Ozone leader information on the Ozone manager UI
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on PR #5311:
URL: https://github.com/apache/ozone/pull/5311#issuecomment-1724888481
@smitajoshi12 thanks for working on this patch, PR description says need to show leader information in ozone manager webUI, however I think we already show LEADER and FOLLOWER info in OM Web UI, but in JSON format, so pls correct the PR description and PR title.
--
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
Re: [PR] HDDS-9303. Ozone Manager : Display leader information in table format and highlight current node. [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #5311:
URL: https://github.com/apache/ozone/pull/5311#discussion_r1384802988
##########
hadoop-ozone/ozone-manager/src/main/resources/webapps/ozoneManager/om-overview.html:
##########
@@ -49,6 +45,36 @@ <h2>Status</h2>
</tbody>
</table>
+<h4 ng-show="$ctrl.overview.jmx.RatisRoles.length == 1 && $ctrl.overview.jmx.RatisRoles[0].length == 1">Exception: {{$ctrl.overview.jmx.RatisRoles[0]}}</h4>
+<div ng-show="$ctrl.overview.jmx.RatisRoles.length > 1">
+ <h2>OM Roles (HA)</h2>
+ <table class="table table-striped table-bordered" class="col-md-6">
+ <thead>
+ <tr>
+ <th>Host Name</th>
+ <th>Node ID</th>
+ <th>Ratis Port</th>
+ <th>Role</th>
+ </tr>
+ </thead>
+ <tbody ng-repeat="roles in $ctrl.overview.jmx.RatisRoles">
+ <tr class="om-roles-background" ng-if="$ctrl.role.Id == roles[1]">
+ <td>{{roles[0]}}</td>
Review Comment:
Could you help answering this question @smitajoshi12?
--
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
Re: [PR] HDDS-9303. Ozone Manager : Display leader information in table format and highlight current node. [ozone]
Posted by "smitajoshi12 (via GitHub)" <gi...@apache.org>.
smitajoshi12 commented on PR #5311:
URL: https://github.com/apache/ozone/pull/5311#issuecomment-1827063253
/ready
@adoroszlai
I have attached screenshot for Exception Handling.
There is no issue over UI in terms of Exception Handling.
--
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
[GitHub] [ozone] devmadhuu commented on a diff in pull request #5311: HDDS-9303. Ozone Manager : Add Ozone leader information on the Ozone manager UI
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #5311:
URL: https://github.com/apache/ozone/pull/5311#discussion_r1329623311
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -2956,16 +2956,17 @@ public String getRatisRoles() {
leaderId = omRatisServer.getLeader();
if (leaderId == null) {
LOG.error("No leader found");
- return "Exception: Not a leader";
+ return Collections.emptyList();
}
serviceList = getServiceList();
} catch (IOException e) {
LOG.error("IO-Exception Occurred", e);
- return "Exception: " + e;
+ return Collections.emptyList();
}
return OmUtils.format(serviceList, port, leaderId.getId().toString());
- } else {
- return "Ratis-Disabled";
+ }
Review Comment:
Correct the formatting and indentation.
--
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
Re: [PR] HDDS-9303. Display leader in table and highlight current node in OM web UI [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #5311:
URL: https://github.com/apache/ozone/pull/5311#discussion_r1413548487
##########
hadoop-ozone/ozone-manager/src/main/resources/webapps/ozoneManager/om-overview.html:
##########
@@ -49,6 +45,36 @@ <h2>Status</h2>
</tbody>
</table>
+<h4 ng-show="$ctrl.overview.jmx.RatisRoles.length == 1 && $ctrl.overview.jmx.RatisRoles[0].length == 1">Exception: {{$ctrl.overview.jmx.RatisRoles[0]}}</h4>
+<div ng-show="$ctrl.overview.jmx.RatisRoles.length > 1">
+ <h2>OM Roles (HA)</h2>
+ <table class="table table-striped table-bordered" class="col-md-6">
+ <thead>
+ <tr>
+ <th>Host Name</th>
+ <th>Node ID</th>
+ <th>Ratis Port</th>
+ <th>Role</th>
+ </tr>
+ </thead>
+ <tbody ng-repeat="roles in $ctrl.overview.jmx.RatisRoles">
+ <tr class="om-roles-background" ng-if="$ctrl.role.Id == roles[1]">
+ <td>{{roles[0]}}</td>
Review Comment:
ok, just display exception message below "OM Roles (HA)" heading
--
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
Re: [PR] HDDS-9303. Ozone Manager : Display leader information in table format and highlight current node. [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #5311:
URL: https://github.com/apache/ozone/pull/5311#issuecomment-1824575493
/pending If there is exception, how the output looks? do still accessing roles[1],... can have issue over UI in exception case?
--
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
Re: [PR] HDDS-9303. Display leader in table and highlight current node in OM web UI [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on PR #5311:
URL: https://github.com/apache/ozone/pull/5311#issuecomment-1842732778
/ready
--
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
Re: [PR] HDDS-9303. Ozone Manager : Display leader information in table format and highlight current node. [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #5311:
URL: https://github.com/apache/ozone/pull/5311#discussion_r1405857963
##########
hadoop-ozone/ozone-manager/src/main/resources/webapps/ozoneManager/om-overview.html:
##########
@@ -49,6 +45,36 @@ <h2>Status</h2>
</tbody>
</table>
+<h4 ng-show="$ctrl.overview.jmx.RatisRoles.length == 1 && $ctrl.overview.jmx.RatisRoles[0].length == 1">Exception: {{$ctrl.overview.jmx.RatisRoles[0]}}</h4>
Review Comment:
Can we show the `OM Roles (HA)` heading even when we have an exception? We could just move the 50th line before this.
--
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
Re: [PR] HDDS-9303. Display leader in table and highlight current node in OM web UI [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #5311:
URL: https://github.com/apache/ozone/pull/5311#discussion_r1407549628
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java:
##########
@@ -825,26 +825,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++;
Review Comment:
I just realised that this `count` variable is not used anymore, we can remove it, right? If yes, please also remove it from line 824 too.
##########
hadoop-ozone/ozone-manager/src/main/resources/webapps/ozoneManager/om-overview.html:
##########
@@ -49,6 +45,36 @@ <h2>Status</h2>
</tbody>
</table>
+<h2>OM Roles (HA)</h2>
+<h4 ng-show="$ctrl.overview.jmx.RatisRoles.length == 1 && $ctrl.overview.jmx.RatisRoles[0].length == 1">Exception: {{$ctrl.overview.jmx.RatisRoles[0]}}</h4>
Review Comment:
I don't think we need the `Expection:` here, we have the necessary information from the `getRatisRoles()` method.
Also if we use `{{$ctrl.overview.jmx.RatisRoles[0][0]}}` we won't have the exception showed like this: `["Exception: Not a Leader"]`, but like this: `Exception: Not a Leader`, right? I think it would be better this way.
```suggestion
<h4 ng-show="$ctrl.overview.jmx.RatisRoles.length == 1 && $ctrl.overview.jmx.RatisRoles[0].length == 1">{{$ctrl.overview.jmx.RatisRoles[0][0]}}</h4>
```
--
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
Re: [PR] HDDS-9303. Ozone Manager : Display leader information in table format and highlight current node. [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on PR #5311:
URL: https://github.com/apache/ozone/pull/5311#issuecomment-1753013778
> ep same as in SCM webUI (in Json format)
Giving as JSON format, and showing in tablular format is good. I think its just render the page with data and may not have much impact for table.
Already given few comments for STANDALONE, non-ratis or exception cases, this is hidden now. That may need resolve for showing.
--
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
[GitHub] [ozone] devmadhuu commented on pull request #5311: HDDS-9303. Ozone Manager : Display leader information in tanle format and highlight current node.
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on PR #5311:
URL: https://github.com/apache/ozone/pull/5311#issuecomment-1728992892
> > thanks for working on this @smitajoshi12!
> > > @smitajoshi12 thanks for working on this patch, PR description says need to show leader information in ozone manager webUI, however I think we already show LEADER and FOLLOWER info in OM Web UI, but in JSON format, so pls correct the PR description and PR title.
> >
> >
> > I agree with Devesh, the OM roles are already present in the OM UI, it was added by @ArafatKhan2198 in [HDDS-6863](https://issues.apache.org/jira/browse/HDDS-6863). For me it looks good already as it is, all the necessary information is present there. This new table that is nested in another table looks a bit weird for me, but maybe that's just for me :) If we want to have a change like this, maybe it should be moved to a new table, but I'm not sure if that would improve the OM UI that much. In the SCM UI it also presents this information in JSON format. Let me know what you think!
>
> @dombizita Hi ZIta, Done Changes as suggested by you. Attached new Screenshot.
@smitajoshi12 what is "`tanle`" format as you have mentioned in PR title. Is it by mistake ? Also PR description is not in line with what is being done in this patch. Also I agree with @dombizita , if this patch really needed as JSON format itself look fine and we want to keep any formatting or processing over OM UI as minimal as possible unless it is very much necessary. Can we keep same as in SCM webUI (in Json format) ? @dombizita @sumitagrawl pls suggest.
--
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
Re: [PR] HDDS-9303. Display leader in table and highlight current node in OM web UI [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on PR #5311:
URL: https://github.com/apache/ozone/pull/5311#issuecomment-1842737204
Thanks for working on this @smitajoshi12! Thank you for the review @devmadhuu, @ArafatKhan2198 and @sumitagrawl!
--
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
Re: [PR] HDDS-9303. Ozone Manager : Display leader information in table format and highlight current node. [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
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: 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
Re: [PR] HDDS-9303. Ozone Manager : Display leader information in table format and highlight current node. [ozone]
Posted by "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
[GitHub] [ozone] dombizita commented on pull request #5311: HDDS-9303. Ozone Manager : Add Ozone leader information on the Ozone manager UI
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on PR #5311:
URL: https://github.com/apache/ozone/pull/5311#issuecomment-1725104204
thanks for working on this @smitajoshi12!
> @smitajoshi12 thanks for working on this patch, PR description says need to show leader information in ozone manager webUI, however I think we already show LEADER and FOLLOWER info in OM Web UI, but in JSON format, so pls correct the PR description and PR title.
I agree with Devesh, the OM roles are already present in the OM UI, it was added by @ArafatKhan2198 in [HDDS-6863](https://issues.apache.org/jira/browse/HDDS-6863). For me it looks good already as it is, all the necessary information is present there.
This new table that is nested in another table looks a bit weird for me, but maybe that's just for me :) If we want to have a change like this, maybe it should be moved to a new table, but I'm not sure if that improve the OM UI that much. In the SCM UI it also presents this information in JSON format. Let me know what you think!
--
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
Re: [PR] HDDS-9303. Ozone Manager : Display leader information in table format and highlight current node. [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #5311:
URL: https://github.com/apache/ozone/pull/5311#discussion_r1350287502
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -2956,16 +2956,16 @@ public String getRatisRoles() {
leaderId = omRatisServer.getLeader();
if (leaderId == null) {
LOG.error("No leader found");
- return "Exception: Not a leader";
+ return Collections.emptyList();
}
serviceList = getServiceList();
} catch (IOException e) {
LOG.error("IO-Exception Occurred", e);
- return "Exception: " + e;
+ return Collections.emptyList();
}
return OmUtils.format(serviceList, port, leaderId.getId().toString());
} else {
- return "Ratis-Disabled";
+ return Collections.emptyList();
Review Comment:
In case of standalone, i.e. ratis disabled, how the info will be shown? as list is empty
can see node info showing "CANDIDATE", do that will be enough?
##########
hadoop-ozone/ozone-manager/src/main/resources/webapps/ozoneManager/om-overview.html:
##########
@@ -49,6 +45,35 @@ <h2>Status</h2>
</tbody>
</table>
+<div ng-show="$ctrl.overview.jmx.RatisRoles.length > 0">
+ <h2>OM Roles (HA)</h2>
+ <table class="table table-striped table-bordered" class="col-md-6">
+ <thead>
+ <tr>
+ <th>Host Name</th>
+ <th>Node ID</th>
+ <th>Ratis Port</th>
+ <th>Role</th>
+ </tr>
+ </thead>
+ <tbody>
+ <tr class="om-roles-background" ng-repeat="roles in $ctrl.overview.jmx.RatisRoles"
+ ng-if="$ctrl.role.Id == roles[2].value">
+ <td>{{roles[3].value}}</td>
Review Comment:
there is assumption that index as in order represent hostname, nodeid, port, role. Do this is valid for all scenario? or need get based on string in json map ?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -2956,16 +2956,16 @@ public String getRatisRoles() {
leaderId = omRatisServer.getLeader();
if (leaderId == null) {
LOG.error("No leader found");
- return "Exception: Not a leader";
+ return Collections.emptyList();
Review Comment:
How to show "Not a leader" or any exception occured in system? if exeption is happening, instead of empty list, need throw exception.
--
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
[GitHub] [ozone] smitajoshi12 commented on pull request #5311: HDDS-9303. Ozone Manager : Display leader information in table format and highlight current node.
Posted by "smitajoshi12 (via GitHub)" <gi...@apache.org>.
smitajoshi12 commented on PR #5311:
URL: https://github.com/apache/ozone/pull/5311#issuecomment-1729285944
> Thanks for the changes @smitajoshi12 for the OM-HA the table looks fine but what about when we have a standalone cluster will we want to display a table there as well, could you please attach a screenshot for that too.
@ArafatKhan2198
Attached New Screenshot with Standalone cluster with no Data.
--
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
Re: [PR] HDDS-9303. Display leader in table and highlight current node in OM web UI [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita merged PR #5311:
URL: https://github.com/apache/ozone/pull/5311
--
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
[GitHub] [ozone] smitajoshi12 commented on pull request #5311: HDDS-9303. Ozone Manager : Display leader information in tanle format and highlight current node.
Posted by "smitajoshi12 (via GitHub)" <gi...@apache.org>.
smitajoshi12 commented on PR #5311:
URL: https://github.com/apache/ozone/pull/5311#issuecomment-1728829527
> thanks for working on this @smitajoshi12!
>
> > @smitajoshi12 thanks for working on this patch, PR description says need to show leader information in ozone manager webUI, however I think we already show LEADER and FOLLOWER info in OM Web UI, but in JSON format, so pls correct the PR description and PR title.
>
> I agree with Devesh, the OM roles are already present in the OM UI, it was added by @ArafatKhan2198 in [HDDS-6863](https://issues.apache.org/jira/browse/HDDS-6863). For me it looks good already as it is, all the necessary information is present there. This new table that is nested in another table looks a bit weird for me, but maybe that's just for me :) If we want to have a change like this, maybe it should be moved to a new table, but I'm not sure if that would improve the OM UI that much. In the SCM UI it also presents this information in JSON format. Let me know what you think!
@dombizita
Hi ZIta,
Done Changes as suggested by you. Attached new Screenshot.
--
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
[GitHub] [ozone] smitajoshi12 commented on a diff in pull request #5311: HDDS-9303. Ozone Manager : Display leader information in tanle format and highlight current node.
Posted by "smitajoshi12 (via GitHub)" <gi...@apache.org>.
smitajoshi12 commented on code in PR #5311:
URL: https://github.com/apache/ozone/pull/5311#discussion_r1332459594
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -2956,16 +2956,17 @@ public String getRatisRoles() {
leaderId = omRatisServer.getLeader();
if (leaderId == null) {
LOG.error("No leader found");
- return "Exception: Not a leader";
+ return Collections.emptyList();
}
serviceList = getServiceList();
} catch (IOException e) {
LOG.error("IO-Exception Occurred", e);
- return "Exception: " + e;
+ return Collections.emptyList();
}
return OmUtils.format(serviceList, port, leaderId.getId().toString());
- } else {
- return "Ratis-Disabled";
+ }
Review Comment:
Done Formatting.
--
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
Re: [PR] HDDS-9303. Ozone Manager : Display leader information in table format and highlight current node. [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #5311:
URL: https://github.com/apache/ozone/pull/5311#discussion_r1382574344
##########
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<List<String>> getRatisRoles() {
List<ServiceInfo> serviceList;
+ List<List<String>> resultList = new ArrayList<>();
+ List<String> messageException = new ArrayList<>();
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.add("Exception: Not a Leader");
+ resultList.add(messageException);
+ return resultList;
}
serviceList = getServiceList();
} catch (IOException e) {
LOG.error("IO-Exception Occurred", e);
- return "Exception: " + e;
+ messageException.add("IO-Exception Occurred");
Review Comment:
we can add e.getMessage() for this, `messageException.add("IO-Exception Occurred, " + e.getMessaage());`
##########
hadoop-ozone/ozone-manager/src/main/resources/webapps/ozoneManager/om-overview.html:
##########
@@ -49,6 +45,36 @@ <h2>Status</h2>
</tbody>
</table>
+<h4 ng-show="$ctrl.overview.jmx.RatisRoles.length == 1 && $ctrl.overview.jmx.RatisRoles[0].length == 1">Exception: {{$ctrl.overview.jmx.RatisRoles[0]}}</h4>
+<div ng-show="$ctrl.overview.jmx.RatisRoles.length > 1">
+ <h2>OM Roles (HA)</h2>
+ <table class="table table-striped table-bordered" class="col-md-6">
+ <thead>
+ <tr>
+ <th>Host Name</th>
+ <th>Node ID</th>
+ <th>Ratis Port</th>
+ <th>Role</th>
+ </tr>
+ </thead>
+ <tbody ng-repeat="roles in $ctrl.overview.jmx.RatisRoles">
+ <tr class="om-roles-background" ng-if="$ctrl.role.Id == roles[1]">
+ <td>{{roles[0]}}</td>
Review Comment:
If there is exception, how the output looks? do still accessing roles[1],... can have issue over UI in exception case?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMXBean.java:
##########
@@ -20,6 +20,8 @@
import org.apache.hadoop.hdds.annotation.InterfaceAudience;
import org.apache.hadoop.hdds.server.ServiceRuntimeInfo;
+import java.util.List;
+import java.util.Map;
Review Comment:
Map is not more used, plz remove import, this may report findbug
--
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