Skip to content

Commit 3e31ed2

Browse files
liuxiaocs7liuxiaocs7
authored
HBASE-29809 Unify region reopening methods to use throttled implement… (#7818)
Signed-off-by: Charles Connell <cconnell@apache.org> Signed-off-by: Duo Zhang <zhangduo@apache.org>
1 parent 1b6aee0 commit 3e31ed2

4 files changed

Lines changed: 81 additions & 17 deletions

File tree

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4268,7 +4268,12 @@ public void remoteProcedureFailed(long procId, RemoteProcedureException error) {
42684268
* unique id).
42694269
* @return procedure Id
42704270
* @throws IOException if reopening region fails while running procedure
4271+
* @deprecated since 3.0.0 and will be removed in 4.0.0. Use
4272+
* {@link #reopenRegionsThrottled(TableName, List, long, long)} instead so region
4273+
* reopening honors the configured throttling.
4274+
* @see <a href="https://issues.apache.org/jira/browse/HBASE-29809">HBASE-29809</a>
42714275
*/
4276+
@Deprecated
42724277
long reopenRegions(final TableName tableName, final List<byte[]> regionNames,
42734278
final long nonceGroup, final long nonce) throws IOException {
42744279

@@ -4311,17 +4316,21 @@ long reopenRegionsThrottled(final TableName tableName, final List<byte[]> region
43114316
throw new TableNotFoundException(tableName);
43124317
}
43134318

4319+
TableDescriptor tableDescriptor = getTableDescriptors().get(tableName);
4320+
if (tableDescriptor == null) {
4321+
throw new TableNotFoundException(tableName);
4322+
}
4323+
43144324
return MasterProcedureUtil
43154325
.submitProcedure(new MasterProcedureUtil.NonceProcedureRunnable(this, nonceGroup, nonce) {
43164326
@Override
43174327
protected void run() throws IOException {
43184328
ReopenTableRegionsProcedure proc;
43194329
if (regionNames.isEmpty()) {
4320-
proc = ReopenTableRegionsProcedure.throttled(getConfiguration(),
4321-
getTableDescriptors().get(tableName));
4330+
proc = ReopenTableRegionsProcedure.throttled(getConfiguration(), tableDescriptor);
43224331
} else {
4323-
proc = ReopenTableRegionsProcedure.throttled(getConfiguration(),
4324-
getTableDescriptors().get(tableName), regionNames);
4332+
proc = ReopenTableRegionsProcedure.throttled(getConfiguration(), tableDescriptor,
4333+
regionNames);
43254334
}
43264335

43274336
LOG.info("{} throttled reopening {} regions for table {}", getClientIdAuditPrefix(),

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

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ public class RegionsRecoveryChore extends ScheduledChore {
5050

5151
private static final String REGIONS_RECOVERY_CHORE_NAME = "RegionsRecoveryChore";
5252

53-
private static final String ERROR_REOPEN_REIONS_MSG =
54-
"Error reopening regions with high storeRefCount. ";
53+
private static final String ERROR_REOPEN_REGIONS_MSG =
54+
"Error reopening regions with high storeRefCount.";
5555

5656
private final HMaster hMaster;
5757
private final int storeFileRefCountThreshold;
@@ -73,7 +73,6 @@ public class RegionsRecoveryChore extends ScheduledChore {
7373
this.hMaster = hMaster;
7474
this.storeFileRefCountThreshold = configuration.getInt(
7575
HConstants.STORE_FILE_REF_COUNT_THRESHOLD, HConstants.DEFAULT_STORE_FILE_REF_COUNT_THRESHOLD);
76-
7776
}
7877

7978
@Override
@@ -83,6 +82,12 @@ protected void chore() {
8382
"Starting up Regions Recovery chore for reopening regions based on storeFileRefCount...");
8483
}
8584
try {
85+
if (!hMaster.isInitialized()) {
86+
if (LOG.isDebugEnabled()) {
87+
LOG.debug("Skipping regions recovery chore because master is not initialized yet");
88+
}
89+
return;
90+
}
8691
// only if storeFileRefCountThreshold > 0, consider the feature turned on
8792
if (storeFileRefCountThreshold > 0) {
8893
final ClusterMetrics clusterMetrics = hMaster.getClusterMetrics();
@@ -94,11 +99,11 @@ protected void chore() {
9499
tableToReopenRegionsMap.forEach((tableName, regionNames) -> {
95100
try {
96101
LOG.warn("Reopening regions due to high storeFileRefCount. "
97-
+ "TableName: {} , noOfRegions: {}", tableName, regionNames.size());
98-
hMaster.reopenRegions(tableName, regionNames, NONCE_GENERATOR.getNonceGroup(),
99-
NONCE_GENERATOR.newNonce());
102+
+ "TableName: {}, numOfRegions: {}", tableName, regionNames.size());
103+
hMaster.reopenRegionsThrottled(tableName, regionNames,
104+
NONCE_GENERATOR.getNonceGroup(), NONCE_GENERATOR.newNonce());
100105
} catch (IOException e) {
101-
LOG.error("{} tableName: {}, regionNames: {}", ERROR_REOPEN_REIONS_MSG, tableName,
106+
LOG.error("{} tableName: {}, regionNames: {}", ERROR_REOPEN_REGIONS_MSG, tableName,
102107
regionNames, e);
103108
}
104109
});

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

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ private Configuration getCustomConf() {
9595
public void setUp() throws Exception {
9696
this.hMaster = Mockito.mock(HMaster.class);
9797
this.assignmentManager = Mockito.mock(AssignmentManager.class);
98+
Mockito.when(hMaster.isInitialized()).thenReturn(true);
9899
}
99100

100101
@AfterEach
@@ -128,9 +129,11 @@ public void testRegionReopensWithStoreRefConfig() throws Exception {
128129
regionsRecoveryChore = new RegionsRecoveryChore(stoppable, configuration, hMaster);
129130
regionsRecoveryChore.chore();
130131

132+
Mockito.verify(hMaster, Mockito.times(1)).isInitialized();
133+
131134
// Verify that we need to reopen regions of 2 tables
132-
Mockito.verify(hMaster, Mockito.times(2)).reopenRegions(Mockito.any(), Mockito.anyList(),
133-
Mockito.anyLong(), Mockito.anyLong());
135+
Mockito.verify(hMaster, Mockito.times(2)).reopenRegionsThrottled(Mockito.any(),
136+
Mockito.anyList(), Mockito.anyLong(), Mockito.anyLong());
134137
Mockito.verify(hMaster, Mockito.times(1)).getClusterMetrics();
135138

136139
// Verify that we need to reopen total 3 regions that have refCount > 300
@@ -163,9 +166,11 @@ public void testRegionReopensWithLessThreshold() throws Exception {
163166
regionsRecoveryChore = new RegionsRecoveryChore(stoppable, configuration, hMaster);
164167
regionsRecoveryChore.chore();
165168

169+
Mockito.verify(hMaster, Mockito.times(1)).isInitialized();
170+
166171
// Verify that we need to reopen regions of only 1 table
167-
Mockito.verify(hMaster, Mockito.times(1)).reopenRegions(Mockito.any(), Mockito.anyList(),
168-
Mockito.anyLong(), Mockito.anyLong());
172+
Mockito.verify(hMaster, Mockito.times(1)).reopenRegionsThrottled(Mockito.any(),
173+
Mockito.anyList(), Mockito.anyLong(), Mockito.anyLong());
169174
Mockito.verify(hMaster, Mockito.times(1)).getClusterMetrics();
170175

171176
// Verify that we need to reopen only 1 region with refCount > 400
@@ -198,16 +203,36 @@ public void testRegionReopensWithoutStoreRefConfig() throws Exception {
198203
regionsRecoveryChore = new RegionsRecoveryChore(stoppable, configuration, hMaster);
199204
regionsRecoveryChore.chore();
200205

206+
Mockito.verify(hMaster, Mockito.times(1)).isInitialized();
207+
201208
// Verify that by default the feature is turned off so no regions
202209
// should be reopened
203-
Mockito.verify(hMaster, Mockito.times(0)).reopenRegions(Mockito.any(), Mockito.anyList(),
204-
Mockito.anyLong(), Mockito.anyLong());
210+
Mockito.verify(hMaster, Mockito.times(0)).reopenRegionsThrottled(Mockito.any(),
211+
Mockito.anyList(), Mockito.anyLong(), Mockito.anyLong());
205212

206213
// default maxCompactedStoreFileRefCount is -1 (no regions to be reopened using AM)
207214
Mockito.verify(hMaster, Mockito.times(0)).getAssignmentManager();
208215
Mockito.verify(assignmentManager, Mockito.times(0)).getRegionInfo(Mockito.any(byte[].class));
209216
}
210217

218+
@Test
219+
public void testRegionReopensSkippedWhenMasterNotInitialized() throws Exception {
220+
Mockito.when(hMaster.isInitialized()).thenReturn(false);
221+
222+
Stoppable stoppable = new StoppableImplementation();
223+
Configuration configuration = getCustomConf();
224+
configuration.setInt("hbase.regions.recovery.store.file.ref.count", 300);
225+
regionsRecoveryChore = new RegionsRecoveryChore(stoppable, configuration, hMaster);
226+
regionsRecoveryChore.chore();
227+
228+
Mockito.verify(hMaster, Mockito.times(1)).isInitialized();
229+
Mockito.verify(hMaster, Mockito.times(0)).getClusterMetrics();
230+
Mockito.verify(hMaster, Mockito.times(0)).reopenRegionsThrottled(Mockito.any(),
231+
Mockito.anyList(), Mockito.anyLong(), Mockito.anyLong());
232+
Mockito.verify(hMaster, Mockito.times(0)).getAssignmentManager();
233+
Mockito.verify(assignmentManager, Mockito.times(0)).getRegionInfo(Mockito.any(byte[].class));
234+
}
235+
211236
private static ClusterMetrics getClusterMetrics(int noOfLiveServer) {
212237
ClusterMetrics clusterMetrics = new ClusterMetrics() {
213238

‎hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestReopenTableRegionsIntegration.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import static org.junit.jupiter.api.Assertions.assertEquals;
2121
import static org.junit.jupiter.api.Assertions.assertNotEquals;
22+
import static org.junit.jupiter.api.Assertions.assertThrows;
2223

2324
import java.util.ArrayList;
2425
import java.util.HashMap;
@@ -29,11 +30,13 @@
2930
import org.apache.hadoop.conf.Configuration;
3031
import org.apache.hadoop.hbase.HBaseTestingUtil;
3132
import org.apache.hadoop.hbase.TableName;
33+
import org.apache.hadoop.hbase.TableNotFoundException;
3234
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
3335
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
3436
import org.apache.hadoop.hbase.client.RegionInfo;
3537
import org.apache.hadoop.hbase.client.TableDescriptor;
3638
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
39+
import org.apache.hadoop.hbase.master.HMaster;
3740
import org.apache.hadoop.hbase.regionserver.HRegion;
3841
import org.apache.hadoop.hbase.regionserver.MetricsRegionWrapperImpl;
3942
import org.apache.hadoop.hbase.testclassification.MasterTests;
@@ -327,4 +330,26 @@ public void testLazyUpdateThenReopenSpecificRegions() throws Exception {
327330
UTIL.deleteTable(tableName);
328331
}
329332
}
333+
334+
@Test
335+
public void testReopenTableRegionsFailsCleanlyWhenDescriptorMissing() throws Exception {
336+
TableName tableName = TableName.valueOf("testReopenTableRegionsFailsWhenDescriptorMissing");
337+
TableDescriptor td = TableDescriptorBuilder.newBuilder(tableName)
338+
.setColumnFamily(ColumnFamilyDescriptorBuilder.of(CF)).build();
339+
UTIL.getAdmin().createTable(td, Bytes.toBytes("a"), Bytes.toBytes("z"), 3);
340+
UTIL.waitTableAvailable(tableName);
341+
342+
HMaster master = UTIL.getMiniHBaseCluster().getMaster();
343+
TableDescriptor removedDescriptor = null;
344+
try {
345+
removedDescriptor = master.getTableDescriptors().remove(tableName);
346+
assertThrows(TableNotFoundException.class,
347+
() -> UTIL.getAdmin().reopenTableRegions(tableName));
348+
} finally {
349+
if (removedDescriptor != null && master.getTableDescriptors().get(tableName) == null) {
350+
master.getTableDescriptors().update(removedDescriptor);
351+
}
352+
UTIL.deleteTable(tableName);
353+
}
354+
}
330355
}

0 commit comments

Comments
 (0)