apurtell commented on a change in pull request #4094:
URL: https://github.com/apache/hbase/pull/4094#discussion_r803942597
##########
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:
You mean here in this compilation unit? It's a little different in each
location and pretty well handled, I think. We use a map to collect the results
from the processing of either Option alternative and a boolean to avoid double
processing during the processing of the Option alternatives. Does it improve
readability to make a two or three line private method that is called twice?
I suppose it is possible to collapse this into a single code block, would
that be better? E.g.
boolean doTasks = false;
....
case TASKS:
doTasks = true;
// fall through
case LIVE_SERVERS: {
...
// do stuff
if (doTasks) {
// do tasks stuff too, like the master monitoredtask collection
...
}
--
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]
