HBASE-28313 StorefileRefresherChore should not refresh readonly table (…

…#5641) Co-authored-by: sunhao5 <[email protected]> Signed-off-by: Duo Zhang <[email protected]> (cherry picked from commit a997301)
apache · Feb 29, 2024 · cf1bded · cf1bded
1 parent 7636a2d
commit cf1bded
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 4 deletions.
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StorefileRefresherChore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StorefileRefresherChore.java
@@ -23,6 +23,7 @@
 import java.util.Map;
 import org.apache.hadoop.hbase.ScheduledChore;
 import org.apache.hadoop.hbase.Stoppable;
+import org.apache.hadoop.hbase.client.RegionInfo;
 import org.apache.hadoop.hbase.master.cleaner.TimeToLiveHFileCleaner;
 import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
 import org.apache.hadoop.util.StringUtils;
@@ -81,8 +82,13 @@ public StorefileRefresherChore(int period, boolean onlyMetaRefresh, HRegionServe
   @Override
   protected void chore() {
     for (Region r : regionServer.getOnlineRegionsLocalContext()) {
-      if (!r.isReadOnly()) {
-        // skip checking for this region if it can accept writes
+      if (
+        !r.isReadOnly() || r.getRegionInfo().getReplicaId() == RegionInfo.DEFAULT_REPLICA_ID
+          || r.getTableDescriptor().isReadOnly()
+      ) {
+        // Skip checking for this region if it can accept writes.
+        // The refresher is only for refreshing secondary replicas. And if the table is readonly,
+        // meaning no writes to the primary replica, skip checking the secondary replicas as well.
         continue;
       }
       // don't refresh unless enabled for all files, or it the meta region

diff --git a/...erver/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileRefresherChore.java b/...erver/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileRefresherChore.java
@@ -81,8 +81,13 @@ public void setUp() throws IOException {
 
   private TableDescriptor getTableDesc(TableName tableName, int regionReplication,
     byte[]... families) {
-    TableDescriptorBuilder builder =
-      TableDescriptorBuilder.newBuilder(tableName).setRegionReplication(regionReplication);
+    return getTableDesc(tableName, regionReplication, false, families);
+  }
+
+  private TableDescriptor getTableDesc(TableName tableName, int regionReplication, boolean readOnly,
+    byte[]... families) {
+    TableDescriptorBuilder builder = TableDescriptorBuilder.newBuilder(tableName)
+      .setRegionReplication(regionReplication).setReadOnly(readOnly);
     Arrays.stream(families).map(family -> ColumnFamilyDescriptorBuilder.newBuilder(family)
       .setMaxVersions(Integer.MAX_VALUE).build()).forEachOrdered(builder::setColumnFamily);
     return builder.build();
@@ -235,4 +240,47 @@ public void testIsStale() throws IOException {
       // expected
     }
   }
+
+  @Test
+  public void testRefreshReadOnlyTable() throws IOException {
+    int period = 0;
+    byte[][] families = new byte[][] { Bytes.toBytes("cf") };
+    byte[] qf = Bytes.toBytes("cq");
+
+    HRegionServer regionServer = mock(HRegionServer.class);
+    List<HRegion> regions = new ArrayList<>();
+    when(regionServer.getOnlineRegionsLocalContext()).thenReturn(regions);
+    when(regionServer.getConfiguration()).thenReturn(TEST_UTIL.getConfiguration());
+
+    TableDescriptor htd = getTableDesc(TableName.valueOf(name.getMethodName()), 2, families);
+    HRegion primary = initHRegion(htd, HConstants.EMPTY_START_ROW, HConstants.EMPTY_END_ROW, 0);
+    HRegion replica1 = initHRegion(htd, HConstants.EMPTY_START_ROW, HConstants.EMPTY_END_ROW, 1);
+    regions.add(primary);
+    regions.add(replica1);
+
+    StorefileRefresherChore chore =
+      new StorefileRefresherChore(period, false, regionServer, new StoppableImplementation());
+
+    // write some data to primary and flush
+    putData(primary, 0, 100, qf, families);
+    primary.flush(true);
+    verifyData(primary, 0, 100, qf, families);
+
+    verifyDataExpectFail(replica1, 0, 100, qf, families);
+    chore.chore();
+    verifyData(replica1, 0, 100, qf, families);
+
+    // write some data to primary and flush before refresh the store files for the replica
+    putData(primary, 100, 100, qf, families);
+    primary.flush(true);
+    verifyData(primary, 0, 200, qf, families);
+
+    // then the table is set to readonly
+    htd = getTableDesc(TableName.valueOf(name.getMethodName()), 2, true, families);
+    primary.setTableDescriptor(htd);
+    replica1.setTableDescriptor(htd);
+
+    chore.chore(); // we cannot refresh the store files
+    verifyDataExpectFail(replica1, 100, 100, qf, families);
+  }
 }