[GitHub] [hbase] virajjasani commented on a change in pull request #2005: HBASE-24662 Update DumpClusterStatusAction to notice changes in regio…

[GitBox <gi...@apache.org>]

virajjasani commented on a change in pull request #2005:
URL: https://github.com/apache/hbase/pull/2005#discussion_r452940628



##########
File path: hbase-it/src/test/java/org/apache/hadoop/hbase/chaos/actions/DumpClusterStatusAction.java
##########
@@ -28,18 +35,69 @@
 public class DumpClusterStatusAction extends Action {
   private static final Logger LOG = LoggerFactory.getLogger(DumpClusterStatusAction.class);
 
-  @Override protected Logger getLogger() {
+  private Set<Address> initialRegionServers;
+
+  @Override
+  protected Logger getLogger() {
     return LOG;
   }
 
   @Override
   public void init(ActionContext context) throws IOException {
     super.init(context);
+    initialRegionServers = collectKnownRegionServers(initialStatus);
   }
 
   @Override
   public void perform() throws Exception {
     getLogger().debug("Performing action: Dump cluster status");
-    getLogger().info("Cluster status\n" + cluster.getClusterMetrics());
+    final ClusterMetrics currentMetrics = cluster.getClusterMetrics();
+    getLogger().info("Cluster status\n{}", currentMetrics);
+    reportMissingRegionServers(currentMetrics);
+    reportNewRegionServers(currentMetrics);
+  }
+
+  /**
+   * Build a set of all the host:port pairs of region servers known to this cluster.
+   */
+  private static Set<Address> collectKnownRegionServers(final ClusterMetrics clusterMetrics) {
+    final Set<Address> regionServers = clusterMetrics.getLiveServerMetrics()
+      .keySet()
+      .stream()
+      .map(ServerName::getAddress)
+      .collect(Collectors.toSet());
+    clusterMetrics.getDeadServerNames()
+      .stream()
+      .map(ServerName::getAddress)
+      .forEach(regionServers::add);
+    return Collections.unmodifiableSet(regionServers);
+  }
+
+  private void reportMissingRegionServers(final ClusterMetrics clusterMetrics) {
+    final Set<Address> regionServers = collectKnownRegionServers(clusterMetrics);
+    final Set<Address> missingRegionServers = new HashSet<>(initialRegionServers);
+    missingRegionServers.removeAll(regionServers);
+    if (!missingRegionServers.isEmpty()) {
+      final StringBuilder stringBuilder = new StringBuilder()
+        .append("region server(s) are missing from this cluster report");
+      missingRegionServers.forEach(address -> {
+        stringBuilder.append("\n  ").append(address);
+      });

Review comment:
       nit: lambda expression `address -> stringBuilder.append("\n  ").append(address)`

##########
File path: hbase-it/src/test/java/org/apache/hadoop/hbase/chaos/actions/DumpClusterStatusAction.java
##########
@@ -28,18 +35,69 @@
 public class DumpClusterStatusAction extends Action {
   private static final Logger LOG = LoggerFactory.getLogger(DumpClusterStatusAction.class);
 
-  @Override protected Logger getLogger() {
+  private Set<Address> initialRegionServers;
+
+  @Override
+  protected Logger getLogger() {
     return LOG;
   }
 
   @Override
   public void init(ActionContext context) throws IOException {
     super.init(context);
+    initialRegionServers = collectKnownRegionServers(initialStatus);
   }
 
   @Override
   public void perform() throws Exception {
     getLogger().debug("Performing action: Dump cluster status");
-    getLogger().info("Cluster status\n" + cluster.getClusterMetrics());
+    final ClusterMetrics currentMetrics = cluster.getClusterMetrics();
+    getLogger().info("Cluster status\n{}", currentMetrics);
+    reportMissingRegionServers(currentMetrics);
+    reportNewRegionServers(currentMetrics);
+  }
+
+  /**
+   * Build a set of all the host:port pairs of region servers known to this cluster.
+   */
+  private static Set<Address> collectKnownRegionServers(final ClusterMetrics clusterMetrics) {
+    final Set<Address> regionServers = clusterMetrics.getLiveServerMetrics()
+      .keySet()
+      .stream()
+      .map(ServerName::getAddress)
+      .collect(Collectors.toSet());
+    clusterMetrics.getDeadServerNames()
+      .stream()
+      .map(ServerName::getAddress)
+      .forEach(regionServers::add);
+    return Collections.unmodifiableSet(regionServers);
+  }
+
+  private void reportMissingRegionServers(final ClusterMetrics clusterMetrics) {
+    final Set<Address> regionServers = collectKnownRegionServers(clusterMetrics);
+    final Set<Address> missingRegionServers = new HashSet<>(initialRegionServers);
+    missingRegionServers.removeAll(regionServers);
+    if (!missingRegionServers.isEmpty()) {
+      final StringBuilder stringBuilder = new StringBuilder()
+        .append("region server(s) are missing from this cluster report");
+      missingRegionServers.forEach(address -> {
+        stringBuilder.append("\n  ").append(address);
+      });
+      getLogger().warn(stringBuilder.toString());
+    }
+  }
+
+  private void reportNewRegionServers(final ClusterMetrics clusterMetrics) {
+    final Set<Address> regionServers = collectKnownRegionServers(clusterMetrics);
+    final Set<Address> newRegionServers = new HashSet<>(regionServers);
+    newRegionServers.removeAll(initialRegionServers);
+    if (!newRegionServers.isEmpty()) {
+      final StringBuilder stringBuilder = new StringBuilder()
+        .append("region server(s) are new for this cluster report");
+      newRegionServers.forEach(address -> {
+        stringBuilder.append("\n  ").append(address);

Review comment:
       same 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org