Skip to content

Commit 5913f89

Browse files
richardantalRichard Antal
authored and
Richard Antal
committed
HBASE-28962 Meta replication is inconsistent after startup when reusing hbase storage location (#6448)
Signed-off-by: Andor Molnár <[email protected]> Signed-off-by: Wellington Chevreuil <[email protected]> Reviewed-by: Aman Poonia <[email protected]>
1 parent 524ba01 commit 5913f89

File tree

3 files changed

+129
-23
lines changed

3 files changed

+129
-23
lines changed

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

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1143,30 +1143,33 @@ private void finishActiveMasterInitialization() throws IOException, InterruptedE
11431143
int replicasNumInConf =
11441144
conf.getInt(HConstants.META_REPLICAS_NUM, HConstants.DEFAULT_META_REPLICA_NUM);
11451145
TableDescriptor metaDesc = tableDescriptors.get(TableName.META_TABLE_NAME);
1146+
int existingReplicasCount =
1147+
assignmentManager.getRegionStates().getRegionsOfTable(TableName.META_TABLE_NAME).size();
1148+
11461149
if (metaDesc.getRegionReplication() != replicasNumInConf) {
11471150
// it is possible that we already have some replicas before upgrading, so we must set the
11481151
// region replication number in meta TableDescriptor directly first, without creating a
11491152
// ModifyTableProcedure, otherwise it may cause a double assign for the meta replicas.
1150-
int existingReplicasCount =
1151-
assignmentManager.getRegionStates().getRegionsOfTable(TableName.META_TABLE_NAME).size();
1152-
if (existingReplicasCount > metaDesc.getRegionReplication()) {
1153-
LOG.info("Update replica count of hbase:meta from {}(in TableDescriptor)"
1154-
+ " to {}(existing ZNodes)", metaDesc.getRegionReplication(), existingReplicasCount);
1155-
metaDesc = TableDescriptorBuilder.newBuilder(metaDesc)
1156-
.setRegionReplication(existingReplicasCount).build();
1157-
tableDescriptors.update(metaDesc);
1158-
}
1159-
// check again, and issue a ModifyTableProcedure if needed
1160-
if (metaDesc.getRegionReplication() != replicasNumInConf) {
1161-
LOG.info(
1162-
"The {} config is {} while the replica count in TableDescriptor is {}"
1163-
+ " for hbase:meta, altering...",
1164-
HConstants.META_REPLICAS_NUM, replicasNumInConf, metaDesc.getRegionReplication());
1165-
procedureExecutor.submitProcedure(new ModifyTableProcedure(
1166-
procedureExecutor.getEnvironment(), TableDescriptorBuilder.newBuilder(metaDesc)
1167-
.setRegionReplication(replicasNumInConf).build(),
1168-
null, metaDesc, false, true));
1169-
}
1153+
LOG.info("Update replica count of hbase:meta from {}(in TableDescriptor)"
1154+
+ " to {}(existing ZNodes)", metaDesc.getRegionReplication(), existingReplicasCount);
1155+
metaDesc = TableDescriptorBuilder.newBuilder(metaDesc)
1156+
.setRegionReplication(existingReplicasCount).build();
1157+
tableDescriptors.update(metaDesc);
1158+
}
1159+
// check again, and issue a ModifyTableProcedure if needed
1160+
if (
1161+
metaDesc.getRegionReplication() != replicasNumInConf
1162+
|| existingReplicasCount != metaDesc.getRegionReplication()
1163+
) {
1164+
LOG.info(
1165+
"The {} config is {} while the replica count in TableDescriptor is {},"
1166+
+ " The number of replicas seen on ZK {} for hbase:meta, altering...",
1167+
HConstants.META_REPLICAS_NUM, replicasNumInConf, metaDesc.getRegionReplication(),
1168+
existingReplicasCount);
1169+
procedureExecutor.submitProcedure(new ModifyTableProcedure(
1170+
procedureExecutor.getEnvironment(), TableDescriptorBuilder.newBuilder(metaDesc)
1171+
.setRegionReplication(replicasNumInConf).build(),
1172+
null, metaDesc, false, true));
11701173
}
11711174
}
11721175
// Initialize after meta is up as below scans meta

hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -498,14 +498,17 @@ private void removeReplicaColumnsIfNeeded(MasterProcedureEnv env) throws IOExcep
498498
private void assignNewReplicasIfNeeded(MasterProcedureEnv env) throws IOException {
499499
final int oldReplicaCount = unmodifiedTableDescriptor.getRegionReplication();
500500
final int newReplicaCount = modifiedTableDescriptor.getRegionReplication();
501-
if (newReplicaCount <= oldReplicaCount) {
501+
int existingReplicasCount = env.getAssignmentManager().getRegionStates()
502+
.getRegionsOfTable(modifiedTableDescriptor.getTableName()).size();
503+
if (newReplicaCount <= Math.min(oldReplicaCount, existingReplicasCount)) {
502504
return;
503505
}
504506
if (isTableEnabled(env)) {
505507
List<RegionInfo> newReplicas = env.getAssignmentManager().getRegionStates()
506508
.getRegionsOfTable(getTableName()).stream().filter(RegionReplicaUtil::isDefaultReplica)
507-
.flatMap(primaryRegion -> IntStream.range(oldReplicaCount, newReplicaCount).mapToObj(
508-
replicaId -> RegionReplicaUtil.getRegionInfoForReplica(primaryRegion, replicaId)))
509+
.flatMap(primaryRegion -> IntStream
510+
.range(Math.min(oldReplicaCount, existingReplicasCount), newReplicaCount).mapToObj(
511+
replicaId -> RegionReplicaUtil.getRegionInfoForReplica(primaryRegion, replicaId)))
509512
.collect(Collectors.toList());
510513
addChildProcedure(env.getAssignmentManager().createAssignProcedures(newReplicas));
511514
}
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.apache.hadoop.hbase.client;
19+
20+
import static org.junit.Assert.assertEquals;
21+
22+
import org.apache.hadoop.hbase.HBaseClassTestRule;
23+
import org.apache.hadoop.hbase.HBaseTestingUtil;
24+
import org.apache.hadoop.hbase.HConstants;
25+
import org.apache.hadoop.hbase.StartTestingClusterOption;
26+
import org.apache.hadoop.hbase.TableDescriptors;
27+
import org.apache.hadoop.hbase.TableName;
28+
import org.apache.hadoop.hbase.TableNameTestRule;
29+
import org.apache.hadoop.hbase.master.HMaster;
30+
import org.apache.hadoop.hbase.regionserver.StorefileRefresherChore;
31+
import org.apache.hadoop.hbase.testclassification.MediumTests;
32+
import org.apache.hadoop.hbase.testclassification.MiscTests;
33+
import org.junit.AfterClass;
34+
import org.junit.BeforeClass;
35+
import org.junit.ClassRule;
36+
import org.junit.Rule;
37+
import org.junit.Test;
38+
import org.junit.experimental.categories.Category;
39+
40+
@Category({ MiscTests.class, MediumTests.class })
41+
public class TestMetaReplicaAssignment {
42+
protected static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
43+
44+
protected static final int REGIONSERVERS_COUNT = 3;
45+
46+
@Rule
47+
public TableNameTestRule name = new TableNameTestRule();
48+
49+
@ClassRule
50+
public static final HBaseClassTestRule CLASS_RULE =
51+
HBaseClassTestRule.forClass(TestMetaReplicaAssignment.class);
52+
53+
@AfterClass
54+
public static void tearDown() throws Exception {
55+
TEST_UTIL.shutdownMiniCluster();
56+
}
57+
58+
@BeforeClass
59+
public static void setUp() throws Exception {
60+
TEST_UTIL.getConfiguration().setInt("zookeeper.session.timeout", 30000);
61+
TEST_UTIL.getConfiguration()
62+
.setInt(StorefileRefresherChore.REGIONSERVER_STOREFILE_REFRESH_PERIOD, 1000);
63+
StartTestingClusterOption option = StartTestingClusterOption.builder()
64+
.numAlwaysStandByMasters(1).numMasters(1).numRegionServers(REGIONSERVERS_COUNT).build();
65+
TEST_UTIL.startMiniCluster(option);
66+
Admin admin = TEST_UTIL.getAdmin();
67+
TEST_UTIL.waitFor(30000, () -> TEST_UTIL.getMiniHBaseCluster().getMaster() != null);
68+
HBaseTestingUtil.setReplicas(admin, TableName.META_TABLE_NAME, 1);
69+
}
70+
71+
@Test
72+
public void testUpgradeSameReplicaCount() throws Exception {
73+
HMaster oldMaster = TEST_UTIL.getMiniHBaseCluster().getMaster();
74+
TableDescriptors oldTds = oldMaster.getTableDescriptors();
75+
TableDescriptor oldMetaTd = oldTds.get(TableName.META_TABLE_NAME);
76+
77+
assertEquals(1, oldMaster.getAssignmentManager().getRegionStates()
78+
.getRegionsOfTable(TableName.META_TABLE_NAME).size());
79+
assertEquals(1, oldMetaTd.getRegionReplication());
80+
// force update the replica count to 3 and then kill the master, to simulate that
81+
// HBase storage location is reused, resulting a sane-looking meta and only the
82+
// RegionInfoBuilder.FIRST_META_REGIONINFO gets assigned in InitMetaProcedure.
83+
// Meta region replicas are not assigned, but we have replication in meta table descriptor.
84+
85+
oldTds.update(TableDescriptorBuilder.newBuilder(oldMetaTd).setRegionReplication(3).build());
86+
oldMaster.stop("Restarting");
87+
TEST_UTIL.waitFor(30000, () -> oldMaster.isStopped());
88+
89+
// increase replica count to 3 through Configuration
90+
TEST_UTIL.getMiniHBaseCluster().getConfiguration().setInt(HConstants.META_REPLICAS_NUM, 3);
91+
TEST_UTIL.getMiniHBaseCluster().startMaster();
92+
TEST_UTIL.waitFor(30000,
93+
() -> TEST_UTIL.getZooKeeperWatcher().getMetaReplicaNodes().size() == 3);
94+
HMaster newMaster = TEST_UTIL.getMiniHBaseCluster().getMaster();
95+
assertEquals(3, newMaster.getAssignmentManager().getRegionStates()
96+
.getRegionsOfTable(TableName.META_TABLE_NAME).size());
97+
assertEquals(3,
98+
newMaster.getTableDescriptors().get(TableName.META_TABLE_NAME).getRegionReplication());
99+
}
100+
}

0 commit comments

Comments
 (0)