You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/08/16 16:03:07 UTC

[GitHub] [ozone] smengcl commented on a diff in pull request #3677: HDDS-7045. Election info is out of date in Recon

smengcl commented on code in PR #3677:
URL: https://github.com/apache/ozone/pull/3677#discussion_r946529412


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/PipelineEndpoint.java:
##########
@@ -130,25 +130,55 @@ public Response getPipelines() {
     return Response.ok(pipelinesResponse).build();
   }
 
-  private Optional<Long> getMetricValue(String metricName, String groupId) {
+  private Long getElectionCountMetricValue(String groupId) {
+    Long electionCount = 0L;
     String metricsQuery = String.format(
-        "query=%s{group=\"%s\"}", metricName, groupId);
+            "query=ratis_leader_election_electionCount{group=\"%s\"}", groupId);
     try {
       List<Metric> metrics = metricsServiceProvider.getMetricsInstant(
-          metricsQuery);
+              metricsQuery);
+      if (!metrics.isEmpty()) {
+        for (Metric m : metrics) {
+          TreeMap<Double, Double> values = (TreeMap<Double, Double>)
+                  m.getValues();
+          if (!values.isEmpty()) {
+            electionCount += Optional.of(values.firstEntry().getValue()
+                    .longValue()).orElse(0L);
+          }
+        }
+      }
+    } catch (Exception ex) {
+      if (LOG.isErrorEnabled()) {
+        LOG.error(String.format("Unable to get metrics value for " +
+                "ratis_leader_election_electionCount"), ex);
+      }
+    }
+    return electionCount;
+  }
+
+  private Long getLastLeaderElectionElapsedTimeMetricValue(String groupId,
+                                                           UUID uuid) {
+    String metricsQuery = String.format(
+            "query=ratis_leader_election_lastLeaderElectionElapsedTime{group=" +
+                    "\"%s\",exported_instance=\"%s\"}", groupId,
+            uuid.toString());
+    try {
+      List<Metric> metrics = metricsServiceProvider.getMetricsInstant(
+              metricsQuery);
       if (!metrics.isEmpty()) {
         TreeMap<Double, Double> values = (TreeMap<Double, Double>)
-            metrics.get(0).getValues();
+                metrics.get(0).getValues();
         if (!values.isEmpty()) {
-          return Optional.of(values.firstEntry().getValue().longValue());
+          return Optional.of(values.firstEntry().getValue().longValue())
+                  .orElse(0L);

Review Comment:
   How about returning -1 to indicate error?
   
   ```suggestion
                     .orElse(-1L);
   ```



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/PipelineEndpoint.java:
##########
@@ -130,25 +130,55 @@ public Response getPipelines() {
     return Response.ok(pipelinesResponse).build();
   }
 
-  private Optional<Long> getMetricValue(String metricName, String groupId) {
+  private Long getElectionCountMetricValue(String groupId) {
+    Long electionCount = 0L;
     String metricsQuery = String.format(
-        "query=%s{group=\"%s\"}", metricName, groupId);
+            "query=ratis_leader_election_electionCount{group=\"%s\"}", groupId);
     try {
       List<Metric> metrics = metricsServiceProvider.getMetricsInstant(
-          metricsQuery);
+              metricsQuery);
+      if (!metrics.isEmpty()) {
+        for (Metric m : metrics) {
+          TreeMap<Double, Double> values = (TreeMap<Double, Double>)
+                  m.getValues();
+          if (!values.isEmpty()) {
+            electionCount += Optional.of(values.firstEntry().getValue()
+                    .longValue()).orElse(0L);

Review Comment:
   In which cases will this optional value become absent?



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/PipelineEndpoint.java:
##########
@@ -130,25 +130,55 @@ public Response getPipelines() {
     return Response.ok(pipelinesResponse).build();
   }
 
-  private Optional<Long> getMetricValue(String metricName, String groupId) {
+  private Long getElectionCountMetricValue(String groupId) {
+    Long electionCount = 0L;
     String metricsQuery = String.format(
-        "query=%s{group=\"%s\"}", metricName, groupId);
+            "query=ratis_leader_election_electionCount{group=\"%s\"}", groupId);
     try {
       List<Metric> metrics = metricsServiceProvider.getMetricsInstant(
-          metricsQuery);
+              metricsQuery);
+      if (!metrics.isEmpty()) {
+        for (Metric m : metrics) {
+          TreeMap<Double, Double> values = (TreeMap<Double, Double>)
+                  m.getValues();
+          if (!values.isEmpty()) {
+            electionCount += Optional.of(values.firstEntry().getValue()
+                    .longValue()).orElse(0L);
+          }
+        }
+      }
+    } catch (Exception ex) {

Review Comment:
   I wanted to say we preferred catching a more detailed exception than such a generic one, but I figured it is not easy to do so at the moment because `getMetricsInstant()` throws generic exception atm.



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