(hbase) branch branch-3 updated: HBASE-28313 StorefileRefresherChore should not refresh readonly table (#5641)

[zh...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

zhangduo pushed a commit to branch branch-3
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-3 by this push:
     new bc8cb2d4a7b HBASE-28313 StorefileRefresherChore should not refresh readonly table (#5641)
bc8cb2d4a7b is described below

commit bc8cb2d4a7b0cfead9a6f56f1888cf6324fd010c
Author: xxishu <71...@users.noreply.github.com>
AuthorDate: Thu Feb 29 14:18:58 2024 +0800

    HBASE-28313 StorefileRefresherChore should not refresh readonly table (#5641)
    
    Co-authored-by: sunhao5 <su...@kingsoft.com>
    Signed-off-by: Duo Zhang <zh...@apache.org>
    (cherry picked from commit a997301d39f525f97bce82adf67ce7ebcda36006)
---
 .../regionserver/StorefileRefresherChore.java      | 10 ++++-
 .../regionserver/TestStoreFileRefresherChore.java  | 52 +++++++++++++++++++++-
 2 files changed, 58 insertions(+), 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
index 40108e346d1..1111e72bcf7 100644
--- 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.Iterator;
 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 class StorefileRefresherChore extends ScheduledChore {
   @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/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileRefresherChore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileRefresherChore.java
index 5f900e707d3..93b2a87545b 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileRefresherChore.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileRefresherChore.java
@@ -81,8 +81,13 @@ public class TestStoreFileRefresherChore {
 
   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 class TestStoreFileRefresherChore {
       // 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);
+  }
 }