This is an automated email from the ASF dual-hosted git repository.
liuxiaocs7 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 56e86102460 HBASE-29809 Unify region reopening methods to use
throttled implement… (#7818)
56e86102460 is described below
commit 56e861024606fecbf898111b1e6ac3e198d5d803
Author: Xiao Liu <[email protected]>
AuthorDate: Tue Jun 16 19:07:25 2026 +0800
HBASE-29809 Unify region reopening methods to use throttled implement…
(#7818)
Signed-off-by: Charles Connell <[email protected]>
Signed-off-by: Duo Zhang <[email protected]>
---
.../org/apache/hadoop/hbase/master/HMaster.java | 17 +++++++---
.../hadoop/hbase/master/RegionsRecoveryChore.java | 19 +++++++----
.../hbase/master/TestRegionsRecoveryChore.java | 37 ++++++++++++++++++----
.../TestReopenTableRegionsIntegration.java | 25 +++++++++++++++
4 files changed, 81 insertions(+), 17 deletions(-)
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
index 4939d08a435..4128b48f791 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
@@ -4242,7 +4242,12 @@ public class HMaster extends
HBaseServerBase<MasterRpcServices> implements Maste
* unique id).
* @return procedure Id
* @throws IOException if reopening region fails while running procedure
+ * @deprecated since 3.0.0 and will be removed in 4.0.0. Use
+ * {@link #reopenRegionsThrottled(TableName, List, long, long)}
instead so region
+ * reopening honors the configured throttling.
+ * @see <a
href="https://issues.apache.org/jira/browse/HBASE-29809";>HBASE-29809</a>
*/
+ @Deprecated
long reopenRegions(final TableName tableName, final List<byte[]> regionNames,
final long nonceGroup, final long nonce) throws IOException {
@@ -4285,17 +4290,21 @@ public class HMaster extends
HBaseServerBase<MasterRpcServices> implements Maste
throw new TableNotFoundException(tableName);
}
+ TableDescriptor tableDescriptor = getTableDescriptors().get(tableName);
+ if (tableDescriptor == null) {
+ throw new TableNotFoundException(tableName);
+ }
+
return MasterProcedureUtil
.submitProcedure(new MasterProcedureUtil.NonceProcedureRunnable(this,
nonceGroup, nonce) {
@Override
protected void run() throws IOException {
ReopenTableRegionsProcedure proc;
if (regionNames.isEmpty()) {
- proc = ReopenTableRegionsProcedure.throttled(getConfiguration(),
- getTableDescriptors().get(tableName));
+ proc = ReopenTableRegionsProcedure.throttled(getConfiguration(),
tableDescriptor);
} else {
- proc = ReopenTableRegionsProcedure.throttled(getConfiguration(),
- getTableDescriptors().get(tableName), regionNames);
+ proc = ReopenTableRegionsProcedure.throttled(getConfiguration(),
tableDescriptor,
+ regionNames);
}
LOG.info("{} throttled reopening {} regions for table {}",
getClientIdAuditPrefix(),
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionsRecoveryChore.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionsRecoveryChore.java
index c88516f2ffe..deac23958eb 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionsRecoveryChore.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionsRecoveryChore.java
@@ -50,8 +50,8 @@ public class RegionsRecoveryChore extends ScheduledChore {
private static final String REGIONS_RECOVERY_CHORE_NAME =
"RegionsRecoveryChore";
- private static final String ERROR_REOPEN_REIONS_MSG =
- "Error reopening regions with high storeRefCount. ";
+ private static final String ERROR_REOPEN_REGIONS_MSG =
+ "Error reopening regions with high storeRefCount.";
private final HMaster hMaster;
private final int storeFileRefCountThreshold;
@@ -73,7 +73,6 @@ public class RegionsRecoveryChore extends ScheduledChore {
this.hMaster = hMaster;
this.storeFileRefCountThreshold = configuration.getInt(
HConstants.STORE_FILE_REF_COUNT_THRESHOLD,
HConstants.DEFAULT_STORE_FILE_REF_COUNT_THRESHOLD);
-
}
@Override
@@ -83,6 +82,12 @@ public class RegionsRecoveryChore extends ScheduledChore {
"Starting up Regions Recovery chore for reopening regions based on
storeFileRefCount...");
}
try {
+ if (!hMaster.isInitialized()) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Skipping regions recovery chore because master is not
initialized yet");
+ }
+ return;
+ }
// only if storeFileRefCountThreshold > 0, consider the feature turned on
if (storeFileRefCountThreshold > 0) {
final ClusterMetrics clusterMetrics = hMaster.getClusterMetrics();
@@ -94,11 +99,11 @@ public class RegionsRecoveryChore extends ScheduledChore {
tableToReopenRegionsMap.forEach((tableName, regionNames) -> {
try {
LOG.warn("Reopening regions due to high storeFileRefCount. "
- + "TableName: {} , noOfRegions: {}", tableName,
regionNames.size());
- hMaster.reopenRegions(tableName, regionNames,
NONCE_GENERATOR.getNonceGroup(),
- NONCE_GENERATOR.newNonce());
+ + "TableName: {}, numOfRegions: {}", tableName,
regionNames.size());
+ hMaster.reopenRegionsThrottled(tableName, regionNames,
+ NONCE_GENERATOR.getNonceGroup(), NONCE_GENERATOR.newNonce());
} catch (IOException e) {
- LOG.error("{} tableName: {}, regionNames: {}",
ERROR_REOPEN_REIONS_MSG, tableName,
+ LOG.error("{} tableName: {}, regionNames: {}",
ERROR_REOPEN_REGIONS_MSG, tableName,
regionNames, e);
}
});
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRegionsRecoveryChore.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRegionsRecoveryChore.java
index 2c38f56cfdd..e36a67dfb34 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRegionsRecoveryChore.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRegionsRecoveryChore.java
@@ -95,6 +95,7 @@ public class TestRegionsRecoveryChore {
public void setUp() throws Exception {
this.hMaster = Mockito.mock(HMaster.class);
this.assignmentManager = Mockito.mock(AssignmentManager.class);
+ Mockito.when(hMaster.isInitialized()).thenReturn(true);
}
@AfterEach
@@ -128,9 +129,11 @@ public class TestRegionsRecoveryChore {
regionsRecoveryChore = new RegionsRecoveryChore(stoppable, configuration,
hMaster);
regionsRecoveryChore.chore();
+ Mockito.verify(hMaster, Mockito.times(1)).isInitialized();
+
// Verify that we need to reopen regions of 2 tables
- Mockito.verify(hMaster, Mockito.times(2)).reopenRegions(Mockito.any(),
Mockito.anyList(),
- Mockito.anyLong(), Mockito.anyLong());
+ Mockito.verify(hMaster,
Mockito.times(2)).reopenRegionsThrottled(Mockito.any(),
+ Mockito.anyList(), Mockito.anyLong(), Mockito.anyLong());
Mockito.verify(hMaster, Mockito.times(1)).getClusterMetrics();
// Verify that we need to reopen total 3 regions that have refCount > 300
@@ -163,9 +166,11 @@ public class TestRegionsRecoveryChore {
regionsRecoveryChore = new RegionsRecoveryChore(stoppable, configuration,
hMaster);
regionsRecoveryChore.chore();
+ Mockito.verify(hMaster, Mockito.times(1)).isInitialized();
+
// Verify that we need to reopen regions of only 1 table
- Mockito.verify(hMaster, Mockito.times(1)).reopenRegions(Mockito.any(),
Mockito.anyList(),
- Mockito.anyLong(), Mockito.anyLong());
+ Mockito.verify(hMaster,
Mockito.times(1)).reopenRegionsThrottled(Mockito.any(),
+ Mockito.anyList(), Mockito.anyLong(), Mockito.anyLong());
Mockito.verify(hMaster, Mockito.times(1)).getClusterMetrics();
// Verify that we need to reopen only 1 region with refCount > 400
@@ -198,16 +203,36 @@ public class TestRegionsRecoveryChore {
regionsRecoveryChore = new RegionsRecoveryChore(stoppable, configuration,
hMaster);
regionsRecoveryChore.chore();
+ Mockito.verify(hMaster, Mockito.times(1)).isInitialized();
+
// Verify that by default the feature is turned off so no regions
// should be reopened
- Mockito.verify(hMaster, Mockito.times(0)).reopenRegions(Mockito.any(),
Mockito.anyList(),
- Mockito.anyLong(), Mockito.anyLong());
+ Mockito.verify(hMaster,
Mockito.times(0)).reopenRegionsThrottled(Mockito.any(),
+ Mockito.anyList(), Mockito.anyLong(), Mockito.anyLong());
// default maxCompactedStoreFileRefCount is -1 (no regions to be reopened
using AM)
Mockito.verify(hMaster, Mockito.times(0)).getAssignmentManager();
Mockito.verify(assignmentManager,
Mockito.times(0)).getRegionInfo(Mockito.any(byte[].class));
}
+ @Test
+ public void testRegionReopensSkippedWhenMasterNotInitialized() throws
Exception {
+ Mockito.when(hMaster.isInitialized()).thenReturn(false);
+
+ Stoppable stoppable = new StoppableImplementation();
+ Configuration configuration = getCustomConf();
+ configuration.setInt("hbase.regions.recovery.store.file.ref.count", 300);
+ regionsRecoveryChore = new RegionsRecoveryChore(stoppable, configuration,
hMaster);
+ regionsRecoveryChore.chore();
+
+ Mockito.verify(hMaster, Mockito.times(1)).isInitialized();
+ Mockito.verify(hMaster, Mockito.times(0)).getClusterMetrics();
+ Mockito.verify(hMaster,
Mockito.times(0)).reopenRegionsThrottled(Mockito.any(),
+ Mockito.anyList(), Mockito.anyLong(), Mockito.anyLong());
+ Mockito.verify(hMaster, Mockito.times(0)).getAssignmentManager();
+ Mockito.verify(assignmentManager,
Mockito.times(0)).getRegionInfo(Mockito.any(byte[].class));
+ }
+
private static ClusterMetrics getClusterMetrics(int noOfLiveServer) {
ClusterMetrics clusterMetrics = new ClusterMetrics() {
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestReopenTableRegionsIntegration.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestReopenTableRegionsIntegration.java
index 0684166fae2..0bb51d45193 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestReopenTableRegionsIntegration.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestReopenTableRegionsIntegration.java
@@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.master.procedure;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import java.util.ArrayList;
import java.util.HashMap;
@@ -29,11 +30,13 @@ import java.util.Set;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HBaseTestingUtil;
import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.TableNotFoundException;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
import org.apache.hadoop.hbase.client.RegionInfo;
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
+import org.apache.hadoop.hbase.master.HMaster;
import org.apache.hadoop.hbase.regionserver.HRegion;
import org.apache.hadoop.hbase.regionserver.MetricsRegionWrapperImpl;
import org.apache.hadoop.hbase.testclassification.MasterTests;
@@ -327,4 +330,26 @@ public class TestReopenTableRegionsIntegration {
UTIL.deleteTable(tableName);
}
}
+
+ @Test
+ public void testReopenTableRegionsFailsCleanlyWhenDescriptorMissing() throws
Exception {
+ TableName tableName =
TableName.valueOf("testReopenTableRegionsFailsWhenDescriptorMissing");
+ TableDescriptor td = TableDescriptorBuilder.newBuilder(tableName)
+ .setColumnFamily(ColumnFamilyDescriptorBuilder.of(CF)).build();
+ UTIL.getAdmin().createTable(td, Bytes.toBytes("a"), Bytes.toBytes("z"), 3);
+ UTIL.waitTableAvailable(tableName);
+
+ HMaster master = UTIL.getMiniHBaseCluster().getMaster();
+ TableDescriptor removedDescriptor = null;
+ try {
+ removedDescriptor = master.getTableDescriptors().remove(tableName);
+ assertThrows(TableNotFoundException.class,
+ () -> UTIL.getAdmin().reopenTableRegions(tableName));
+ } finally {
+ if (removedDescriptor != null &&
master.getTableDescriptors().get(tableName) == null) {
+ master.getTableDescriptors().update(removedDescriptor);
+ }
+ UTIL.deleteTable(tableName);
+ }
+ }
}
