HBASE-24967 The table.jsp cost long time to load if the table include… (#2326)

* HBASE-24967 The table.jsp cost long time to load if the table include closed regions

* fix it  by another way

* fix review issue

* fix checkstyle warnings

* fix checkstyle warning

hbase-client/src/main/java/org/apache/hadoop/hbase/RegionMetrics.java

@@ -21,6 +21,7 @@
 package org.apache.hadoop.hbase;
 
 import java.util.Map;
+import org.apache.hadoop.hbase.client.CompactionState;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.yetus.audience.InterfaceAudience;
 
@@ -182,4 +183,9 @@ public interface RegionMetrics {
    * @return the block total weight of this region
    */
   long getBlocksTotalWeight();
+
+  /**
+   * @return the compaction state of this region
+   */
+  CompactionState getCompactionState();
 }

hbase-client/src/main/java/org/apache/hadoop/hbase/RegionMetricsBuilder.java

@@ -24,12 +24,14 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
+import org.apache.hadoop.hbase.client.CompactionState;
 import org.apache.hadoop.hbase.util.Strings;
 import org.apache.yetus.audience.InterfaceAudience;
 
 import org.apache.hbase.thirdparty.com.google.common.base.Preconditions;
 import org.apache.hbase.thirdparty.com.google.protobuf.UnsafeByteOperations;
 
+import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.ClusterStatusProtos;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos;
@@ -58,6 +60,8 @@ public final class RegionMetricsBuilder {
         .setBlocksLocalWithSsdWeight(regionLoadPB.hasBlocksLocalWithSsdWeight() ?
           regionLoadPB.getBlocksLocalWithSsdWeight() : 0)
         .setBlocksTotalWeight(regionLoadPB.getBlocksTotalWeight())
+        .setCompactionState(ProtobufUtil.createCompactionStateForRegionLoad(
+          regionLoadPB.getCompactionState()))
         .setFilteredReadRequestCount(regionLoadPB.getFilteredReadRequestsCount())
         .setStoreFileUncompressedDataIndexSize(new Size(regionLoadPB.getTotalStaticIndexSizeKB(),
           Size.Unit.KILOBYTE))
@@ -159,6 +163,7 @@ public final class RegionMetricsBuilder {
   private long blocksLocalWeight;
   private long blocksLocalWithSsdWeight;
   private long blocksTotalWeight;
+  private CompactionState compactionState;
   private RegionMetricsBuilder(byte[] name) {
     this.name = name;
   }
@@ -263,6 +268,11 @@ public final class RegionMetricsBuilder {
     this.blocksTotalWeight = value;
     return this;
   }
+  public RegionMetricsBuilder setCompactionState(CompactionState compactionState) {
+    this.compactionState = compactionState;
+    return this;
+  }
+
   public RegionMetrics build() {
     return new RegionMetricsImpl(name,
         storeCount,
@@ -289,7 +299,8 @@ public final class RegionMetricsBuilder {
         dataLocalityForSsd,
         blocksLocalWeight,
         blocksLocalWithSsdWeight,
-        blocksTotalWeight);
+        blocksTotalWeight,
+        compactionState);
   }
 
   private static class RegionMetricsImpl implements RegionMetrics {
@@ -319,6 +330,7 @@ public final class RegionMetricsBuilder {
     private final long blocksLocalWeight;
     private final long blocksLocalWithSsdWeight;
     private final long blocksTotalWeight;
+    private final CompactionState compactionState;
     RegionMetricsImpl(byte[] name,
         int storeCount,
         int storeFileCount,
@@ -344,7 +356,8 @@ public final class RegionMetricsBuilder {
         float dataLocalityForSsd,
         long blocksLocalWeight,
         long blocksLocalWithSsdWeight,
-        long blocksTotalWeight) {
+        long blocksTotalWeight,
+        CompactionState compactionState) {
       this.name = Preconditions.checkNotNull(name);
       this.storeCount = storeCount;
       this.storeFileCount = storeFileCount;
@@ -371,6 +384,7 @@ public final class RegionMetricsBuilder {
       this.blocksLocalWeight = blocksLocalWeight;
       this.blocksLocalWithSsdWeight = blocksLocalWithSsdWeight;
       this.blocksTotalWeight = blocksTotalWeight;
+      this.compactionState = compactionState;
     }
 
     @Override
@@ -503,6 +517,11 @@ public final class RegionMetricsBuilder {
       return blocksTotalWeight;
     }
 
+    @Override
+    public CompactionState getCompactionState() {
+      return compactionState;
+    }
+
     @Override
     public String toString() {
       StringBuilder sb = Strings.appendKeyValue(new StringBuilder(), "storeCount",
@@ -562,6 +581,8 @@ public final class RegionMetricsBuilder {
         blocksLocalWithSsdWeight);
       Strings.appendKeyValue(sb, "blocksTotalWeight",
         blocksTotalWeight);
+      Strings.appendKeyValue(sb, "compactionState",
+        compactionState);
       return sb.toString();
     }
   }

hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java

@@ -2960,6 +2960,23 @@ public final class ProtobufUtil {
     return GetRegionInfoResponse.CompactionState.valueOf(state.toString());
   }
 
+  /**
+   * Creates {@link CompactionState} from
+   * {@link org.apache.hadoop.hbase.shaded.protobuf.generated.ClusterStatusProtos
+   * .RegionLoad.CompactionState} state
+   * @param state the protobuf CompactionState
+   * @return CompactionState
+   */
+  public static CompactionState createCompactionStateForRegionLoad(
+      RegionLoad.CompactionState state) {
+    return CompactionState.valueOf(state.toString());
+  }
+
+  public static RegionLoad.CompactionState createCompactionStateForRegionLoad(
+      CompactionState state) {
+    return RegionLoad.CompactionState.valueOf(state.toString());
+  }
+
   public static Optional<Long> toOptionalTimestamp(MajorCompactionTimestampResponse resp) {
     long timestamp = resp.getCompactionTimestamp();
     return timestamp == 0 ? Optional.empty() : Optional.of(timestamp);

hbase-protocol-shaded/src/main/protobuf/server/ClusterStatus.proto

@@ -167,6 +167,16 @@ message RegionLoad {
 
   /** The current blocks total weight for region in the regionserver */
   optional uint64 blocks_total_weight = 26;
+
+  /** The compaction state for region */
+  optional CompactionState compaction_state = 27;
+
+  enum CompactionState {
+    NONE = 0;
+    MINOR = 1;
+    MAJOR = 2;
+    MAJOR_AND_MINOR = 3;
+  }
 }
 
 message UserLoad {

hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java

@@ -75,13 +75,16 @@ import org.apache.hadoop.hbase.MasterNotRunningException;
 import org.apache.hadoop.hbase.MetaTableAccessor;
 import org.apache.hadoop.hbase.NamespaceDescriptor;
 import org.apache.hadoop.hbase.PleaseHoldException;
+import org.apache.hadoop.hbase.RegionMetrics;
 import org.apache.hadoop.hbase.ReplicationPeerNotFoundException;
+import org.apache.hadoop.hbase.ServerMetrics;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.TableNotDisabledException;
 import org.apache.hadoop.hbase.TableNotFoundException;
 import org.apache.hadoop.hbase.UnknownRegionException;
 import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.client.CompactionState;
 import org.apache.hadoop.hbase.client.MasterSwitchType;
 import org.apache.hadoop.hbase.client.NormalizeTableFilterParams;
 import org.apache.hadoop.hbase.client.RegionInfo;
@@ -237,7 +240,7 @@ import org.apache.hbase.thirdparty.org.eclipse.jetty.servlet.ServletHolder;
 import org.apache.hbase.thirdparty.org.eclipse.jetty.webapp.WebAppContext;
 
 import org.apache.hadoop.hbase.shaded.protobuf.RequestConverter;
-import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.GetRegionInfoResponse.CompactionState;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.GetRegionInfoResponse;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.SnapshotProtos.SnapshotDescription;
 
 /**
@@ -3445,12 +3448,12 @@ public class HMaster extends HRegionServer implements MasterServices {
    * @param tableName The current table name.
    * @return If a given table is in mob file compaction now.
    */
-  public CompactionState getMobCompactionState(TableName tableName) {
+  public GetRegionInfoResponse.CompactionState getMobCompactionState(TableName tableName) {
     AtomicInteger compactionsCount = mobCompactionStates.get(tableName);
     if (compactionsCount != null && compactionsCount.get() != 0) {
-      return CompactionState.MAJOR_AND_MINOR;
+      return GetRegionInfoResponse.CompactionState.MAJOR_AND_MINOR;
     }
-    return CompactionState.NONE;
+    return GetRegionInfoResponse.CompactionState.NONE;
   }
 
   public void reportMobCompactionStart(TableName tableName) throws IOException {
@@ -3900,4 +3903,47 @@ public class HMaster extends HRegionServer implements MasterServices {
   public RSGroupInfoManager getRSGroupInfoManager() {
     return rsGroupInfoManager;
   }
+
+  /**
+   * Get the compaction state of the table
+   *
+   * @param tableName The table name
+   * @return CompactionState Compaction state of the table
+   */
+  public CompactionState getCompactionState(final TableName tableName) {
+    CompactionState compactionState = CompactionState.NONE;
+    try {
+      List<RegionInfo> regions =
+        assignmentManager.getRegionStates().getRegionsOfTable(tableName, false);
+      for (RegionInfo regionInfo : regions) {
+        ServerName serverName =
+          assignmentManager.getRegionStates().getRegionServerOfRegion(regionInfo);
+        if (serverName == null) {
+          continue;
+        }
+        ServerMetrics sl = serverManager.getLoad(serverName);
+        if (sl == null) {
+          continue;
+        }
+        RegionMetrics regionMetrics = sl.getRegionMetrics().get(regionInfo.getRegionName());
+        if (regionMetrics.getCompactionState() == CompactionState.MAJOR) {
+          if (compactionState == CompactionState.MINOR) {
+            compactionState = CompactionState.MAJOR_AND_MINOR;
+          } else {
+            compactionState = CompactionState.MAJOR;
+          }
+        } else if (regionMetrics.getCompactionState() == CompactionState.MINOR) {
+          if (compactionState == CompactionState.MAJOR) {
+            compactionState = CompactionState.MAJOR_AND_MINOR;
+          } else {
+            compactionState = CompactionState.MINOR;
+          }
+        }
+      }
+    } catch (Exception e) {
+      compactionState = null;
+      LOG.error("Exception when get compaction state for " + tableName.getNameAsString(), e);
+    }
+    return compactionState;
+  }
 }

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java

@@ -1758,9 +1758,9 @@ public class HRegionServer extends Thread implements
       .setBlocksLocalWeight(blocksLocalWeight)
       .setBlocksLocalWithSsdWeight(blocksLocalWithSsdWeight)
       .setBlocksTotalWeight(blocksTotalWeight)
+      .setCompactionState(ProtobufUtil.createCompactionStateForRegionLoad(r.getCompactionState()))
       .setLastMajorCompactionTs(r.getOldestHfileTs(true));
     r.setCompleteSequenceId(regionLoadBldr);
-
     return regionLoadBldr.build();
   }
 

hbase-server/src/main/resources/hbase-webapps/master/table.jsp

@@ -30,6 +30,7 @@
   import="java.util.HashSet"
   import="java.util.Optional"
   import="java.util.TreeMap"
+  import="java.util.concurrent.TimeoutException"
   import="java.util.concurrent.TimeUnit"
   import="org.apache.commons.lang3.StringEscapeUtils"
   import="org.apache.hadoop.conf.Configuration"
@@ -654,21 +655,8 @@
       <td>
 <%
   if (master.getAssignmentManager().isTableEnabled(table.getName())) {
-    try {
+    CompactionState compactionState = master.getCompactionState(table.getName());
-      CompactionState compactionState = admin.getCompactionState(table.getName()).get();
+    %><%= compactionState==null?"UNKNOWN":compactionState %><%
-      %><%= compactionState %><%
-    } catch (Exception e) {
-
-      if(e.getCause() != null && e.getCause().getCause() instanceof NotServingRegionException) {
-        %><%= CompactionState.NONE %><%
-      } else {
-        // Nothing really to do here
-        for(StackTraceElement element : e.getStackTrace()) {
-           %><%= StringEscapeUtils.escapeHtml4(element.toString()) %><%
-        }
-       %> Unknown <%
-      }
-    }
   } else {
     %><%= CompactionState.NONE %><%
   }

hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRegionsRecoveryChore.java

@@ -37,6 +37,7 @@ import org.apache.hadoop.hbase.Size;
 import org.apache.hadoop.hbase.Stoppable;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.UserMetrics;
+import org.apache.hadoop.hbase.client.CompactionState;
 import org.apache.hadoop.hbase.client.RegionInfo;
 import org.apache.hadoop.hbase.client.RegionStatesCount;
 import org.apache.hadoop.hbase.master.assignment.AssignmentManager;
@@ -512,6 +513,11 @@ public class TestRegionsRecoveryChore {
       public long getBlocksTotalWeight() {
         return 0;
       }
+
+      @Override
+      public CompactionState getCompactionState() {
+        return null;
+      }
     };
     return regionMetrics;
   }

hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactionState.java

@@ -31,6 +31,7 @@ import org.apache.hadoop.hbase.client.Admin;
 import org.apache.hadoop.hbase.client.CompactionState;
 import org.apache.hadoop.hbase.client.Put;
 import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.master.HMaster;
 import org.apache.hadoop.hbase.testclassification.LargeTests;
 import org.apache.hadoop.hbase.testclassification.VerySlowRegionServerTests;
 import org.apache.hadoop.hbase.util.Bytes;
@@ -69,24 +70,50 @@ public class TestCompactionState {
     TEST_UTIL.shutdownMiniCluster();
   }
 
-  @Test
+  enum StateSource {
-  public void testMajorCompaction() throws IOException, InterruptedException {
+    ADMIN, MASTER
-    compaction(name.getMethodName(), 8, CompactionState.MAJOR, false);
   }
 
   @Test
-  public void testMinorCompaction() throws IOException, InterruptedException {
+  public void testMajorCompactionStateFromAdmin() throws IOException, InterruptedException {
-    compaction(name.getMethodName(), 15, CompactionState.MINOR, false);
+    compaction(name.getMethodName(), 8, CompactionState.MAJOR, false, StateSource.ADMIN);
   }
 
   @Test
-  public void testMajorCompactionOnFamily() throws IOException, InterruptedException {
+  public void testMinorCompactionStateFromAdmin() throws IOException, InterruptedException {
-    compaction(name.getMethodName(), 8, CompactionState.MAJOR, true);
+    compaction(name.getMethodName(), 15, CompactionState.MINOR, false, StateSource.ADMIN);
   }
 
   @Test
-  public void testMinorCompactionOnFamily() throws IOException, InterruptedException {
+  public void testMajorCompactionOnFamilyStateFromAdmin() throws IOException, InterruptedException {
-    compaction(name.getMethodName(), 15, CompactionState.MINOR, true);
+    compaction(name.getMethodName(), 8, CompactionState.MAJOR, true, StateSource.ADMIN);
+  }
+
+  @Test
+  public void testMinorCompactionOnFamilyStateFromAdmin() throws IOException, InterruptedException {
+    compaction(name.getMethodName(), 15, CompactionState.MINOR, true, StateSource.ADMIN);
+  }
+
+  @Test
+  public void testMajorCompactionStateFromMaster() throws IOException, InterruptedException {
+    compaction(name.getMethodName(), 8, CompactionState.MAJOR, false, StateSource.MASTER);
+  }
+
+  @Test
+  public void testMinorCompactionStateFromMaster() throws IOException, InterruptedException {
+    compaction(name.getMethodName(), 15, CompactionState.MINOR, false, StateSource.MASTER);
+  }
+
+  @Test
+  public void testMajorCompactionOnFamilyStateFromMaster()
+      throws IOException, InterruptedException {
+    compaction(name.getMethodName(), 8, CompactionState.MAJOR, true, StateSource.MASTER);
+  }
+
+  @Test
+  public void testMinorCompactionOnFamilyStateFromMaster()
+      throws IOException, InterruptedException {
+    compaction(name.getMethodName(), 15, CompactionState.MINOR, true, StateSource.MASTER);
   }
 
   @Test
@@ -127,11 +154,12 @@ public class TestCompactionState {
    * @param flushes
    * @param expectedState
    * @param singleFamily otherwise, run compaction on all cfs
+   * @param stateSource get the state by Admin or Master
    * @throws IOException
    * @throws InterruptedException
    */
   private void compaction(final String tableName, final int flushes,
-      final CompactionState expectedState, boolean singleFamily)
+      final CompactionState expectedState, boolean singleFamily, StateSource stateSource)
       throws IOException, InterruptedException {
     // Create a table with regions
     TableName table = TableName.valueOf(tableName);
@@ -143,6 +171,7 @@ public class TestCompactionState {
       ht = TEST_UTIL.createTable(table, families);
       loadData(ht, families, 3000, flushes);
       HRegionServer rs = TEST_UTIL.getMiniHBaseCluster().getRegionServer(0);
+      HMaster master = TEST_UTIL.getMiniHBaseCluster().getMaster();
       List<HRegion> regions = rs.getRegions(table);
       int countBefore = countStoreFilesInFamilies(regions, families);
       int countBeforeSingleFamily = countStoreFilesInFamily(regions, family);
@@ -164,10 +193,10 @@ public class TestCompactionState {
       long curt = System.currentTimeMillis();
       long waitTime = 5000;
       long endt = curt + waitTime;
-      CompactionState state = admin.getCompactionState(table);
+      CompactionState state = getCompactionState(stateSource, master, admin, table);
       while (state == CompactionState.NONE && curt < endt) {
         Thread.sleep(10);
-        state = admin.getCompactionState(table);
+        state = getCompactionState(stateSource, master, admin, table);
         curt = System.currentTimeMillis();
       }
       // Now, should have the right compaction state,
@@ -179,10 +208,10 @@ public class TestCompactionState {
         }
       } else {
         // Wait until the compaction is done
-        state = admin.getCompactionState(table);
+        state = getCompactionState(stateSource, master, admin, table);
         while (state != CompactionState.NONE && curt < endt) {
           Thread.sleep(10);
-          state = admin.getCompactionState(table);
+          state = getCompactionState(stateSource, master, admin, table);
         }
         // Now, compaction should be done.
         assertEquals(CompactionState.NONE, state);
@@ -210,6 +239,14 @@ public class TestCompactionState {
     }
   }
 
+  private static CompactionState getCompactionState(StateSource stateSource, HMaster master,
+      Admin admin, TableName table) throws IOException {
+    CompactionState state = stateSource == StateSource.ADMIN ?
+      admin.getCompactionState(table) :
+      master.getCompactionState(table);
+    return state;
+  }
+
   private static int countStoreFilesInFamily(
       List<HRegion> regions, final byte[] family) {
     return countStoreFilesInFamilies(regions, new byte[][]{family});