gjacoby126 commented on a change in pull request #4094:
URL: https://github.com/apache/hbase/pull/4094#discussion_r799716639



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -2699,11 +2701,11 @@ public void checkTableModifiable(final TableName 
tableName)
     }
   }
 
-  public ClusterMetrics getClusterMetricsWithoutCoprocessor() throws 
InterruptedIOException {
-    return getClusterMetricsWithoutCoprocessor(EnumSet.allOf(Option.class));
+  public ClusterMetrics getClusterMetricsInternal() throws 
InterruptedIOException {
+    return getClusterMetricsInternal(EnumSet.allOf(Option.class));
   }
 
-  public ClusterMetrics getClusterMetricsWithoutCoprocessor(EnumSet<Option> 
options)
+  public ClusterMetrics getClusterMetricsInternal(EnumSet<Option> options)

Review comment:
       This changes the public method name on an LP(TOOLS) class in a minor 
release. Are there any tools not in the immediate project that depend on this?

##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -2712,16 +2714,52 @@ public ClusterMetrics 
getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
       options = EnumSet.allOf(Option.class);
     }
 
+    // TASKS and/or LIVE_SERVERS will populate this map, which will be given 
to the builder if
+    // not null after option processing completes.
+    Map<ServerName, ServerMetrics> serverMetricsMap = null;
+    boolean processedLiveServers = false;
+
     for (Option opt : options) {
       switch (opt) {
         case HBASE_VERSION: builder.setHBaseVersion(VersionInfo.getVersion()); 
break;
         case CLUSTER_ID: builder.setClusterId(getClusterId()); break;
         case MASTER: builder.setMasterName(getServerName()); break;
         case BACKUP_MASTERS: builder.setBackerMasterNames(getBackupMasters()); 
break;
+        case TASKS: {
+          // Master tasks
+          builder.setMasterTasks(TaskMonitor.get().getTasks().stream()
+            .map(task -> ServerTaskBuilder.newBuilder()
+              .setDescription(task.getDescription())
+              .setStatus(task.getStatus())
+              .setState(ServerTask.State.valueOf(task.getState().name()))
+              .setStartTime(task.getStartTime())
+              .setCompletionTime(task.getCompletionTimestamp())
+              .build())
+            .collect(Collectors.toList()));
+          // TASKS is also synonymous with LIVE_SERVERS for now because task 
information for
+          // regionservers is carried in ServerLoad.
+          // Add entries to serverMetricsMap for all live servers
+          if (serverManager != null && !processedLiveServers) {
+            if (serverMetricsMap == null) {
+              serverMetricsMap = new HashMap<>();
+            }
+            final Map<ServerName, ServerMetrics> map = serverMetricsMap;
+            serverManager.getOnlineServers().entrySet()

Review comment:
       nit: could we factor out the common code into a helper rather than 
duplicating?




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


Reply via email to