HBASE-28313 StorefileRefresherChore should not refresh readonly table (#5641)
Co-authored-by: sunhao5 <sunhao5@kingsoft.com>
Signed-off-by: Duo Zhang <zhangduo@apache.org>
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 40108e3..1111e72 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.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 @@
@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 5f900e7..93b2a87 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 @@
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 @@
// 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);
+ }
}
