Skip to content

Commit 0d214ff

Browse files
authored
HBASE-26433 Rollback from ZK-less to ZK-based assignment could produce inconsistent state - doubly assigned regions (#3826)
Signed-off-by: Andrew Purtell <apurtell@apache.org> Signed-off-by: Geoffrey Jacoby <gjacoby@apache.org> Signed-off-by: David Manning <david.manning@salesforce.com>
1 parent 40b4cb1 commit 0d214ff

File tree

5 files changed

+173
-10
lines changed

5 files changed

+173
-10
lines changed

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

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import org.apache.hadoop.conf.Configuration;
5151
import org.apache.hadoop.fs.FileSystem;
5252
import org.apache.hadoop.fs.Path;
53+
import org.apache.hadoop.hbase.Cell;
5354
import org.apache.hadoop.hbase.CoordinatedStateException;
5455
import org.apache.hadoop.hbase.HBaseIOException;
5556
import org.apache.hadoop.hbase.HConstants;
@@ -68,6 +69,7 @@
6869
import org.apache.hadoop.hbase.classification.InterfaceAudience;
6970
import org.apache.hadoop.hbase.client.Admin;
7071
import org.apache.hadoop.hbase.client.Admin.MasterSwitchType;
72+
import org.apache.hadoop.hbase.client.Delete;
7173
import org.apache.hadoop.hbase.client.RegionReplicaUtil;
7274
import org.apache.hadoop.hbase.client.Result;
7375
import org.apache.hadoop.hbase.coordination.BaseCoordinatedStateManager;
@@ -3293,7 +3295,8 @@ Set<ServerName> rebuildUserRegions() throws
32933295
HRegionInfo regionInfo = hrl.getRegionInfo();
32943296
if (regionInfo == null) continue;
32953297
int replicaId = regionInfo.getReplicaId();
3296-
State state = RegionStateStore.getRegionState(result, replicaId);
3298+
State state = RegionStateStore.getRegionState(result, replicaId,
3299+
ConfigUtil.isZKAssignmentInUse(server.getConfiguration()));
32973300
// keep a track of replicas to close. These were the replicas of the split parents
32983301
// from the previous life of the master. The master should have closed them before
32993302
// but it couldn't maybe because it crashed
@@ -3303,7 +3306,8 @@ Set<ServerName> rebuildUserRegions() throws
33033306
}
33043307
}
33053308
ServerName lastHost = hrl.getServerName();
3306-
ServerName regionLocation = RegionStateStore.getRegionServer(result, replicaId);
3309+
ServerName regionLocation = RegionStateStore.getRegionServer(result, replicaId,
3310+
ConfigUtil.isZKAssignmentInUse(server.getConfiguration()));
33073311
if (tableStateManager.isTableState(regionInfo.getTable(),
33083312
ZooKeeperProtos.Table.State.DISABLED)) {
33093313
// force region to forget it hosts for disabled/disabling tables.
@@ -3343,6 +3347,61 @@ Set<ServerName> rebuildUserRegions() throws
33433347
return offlineServers;
33443348
}
33453349

3350+
void deleteNonZkBasedQualifiersForZkBasedAssignment() throws IOException {
3351+
boolean isZKAssignmentInUse = ConfigUtil.isZKAssignmentInUse(server.getConfiguration());
3352+
if (isZKAssignmentInUse) {
3353+
List<Result> results = MetaTableAccessor.fullScanOfMeta(server.getConnection());
3354+
List<Delete> redundantCQDeletes = new ArrayList<>();
3355+
for (Result result : results) {
3356+
RegionLocations rl = MetaTableAccessor.getRegionLocations(result);
3357+
if (rl == null) {
3358+
LOG.error("No location found for " + result);
3359+
continue;
3360+
}
3361+
HRegionLocation[] locations = rl.getRegionLocations();
3362+
if (locations == null) {
3363+
LOG.error("No location found for " + rl);
3364+
continue;
3365+
}
3366+
for (HRegionLocation hrl : locations) {
3367+
if (hrl == null) {
3368+
continue;
3369+
}
3370+
HRegionInfo regionInfo = hrl.getRegionInfo();
3371+
if (regionInfo == null) {
3372+
LOG.error("No region info found " + hrl);
3373+
continue;
3374+
}
3375+
int replicaId = regionInfo.getReplicaId();
3376+
Cell cell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY,
3377+
RegionStateStore.getServerNameColumn(replicaId));
3378+
if (cell != null && cell.getValueLength() > 0) {
3379+
Delete delete =
3380+
new Delete(cell.getRowArray(), cell.getRowOffset(), cell.getRowLength());
3381+
delete.addColumns(HConstants.CATALOG_FAMILY,
3382+
RegionStateStore.getServerNameColumn(replicaId));
3383+
redundantCQDeletes.add(delete);
3384+
}
3385+
cell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY,
3386+
RegionStateStore.getStateColumn(replicaId));
3387+
if (cell != null && cell.getValueLength() > 0) {
3388+
Delete delete =
3389+
new Delete(cell.getRowArray(), cell.getRowOffset(), cell.getRowLength());
3390+
delete
3391+
.addColumns(HConstants.CATALOG_FAMILY, RegionStateStore.getStateColumn(replicaId));
3392+
redundantCQDeletes.add(delete);
3393+
}
3394+
}
3395+
}
3396+
if (!redundantCQDeletes.isEmpty()) {
3397+
LOG.info("Meta contains multiple info:sn and/or info:state values that are not required "
3398+
+ "for ZK based region assignment workflows. Preparing to delete these CQs. Number of"
3399+
+ " Deletes: " + redundantCQDeletes.size());
3400+
MetaTableAccessor.deleteFromMetaTable(server.getConnection(), redundantCQDeletes);
3401+
}
3402+
}
3403+
}
3404+
33463405
/**
33473406
* Recover the tables that were not fully moved to DISABLED state. These
33483407
* tables are in DISABLING state when the master restarted/switched.

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -938,6 +938,8 @@ private void finishActiveMasterInitialization(MonitoredTask status)
938938
// Set master as 'initialized'.
939939
setInitialized(true);
940940

941+
this.assignmentManager.deleteNonZkBasedQualifiersForZkBasedAssignment();
942+
941943
assignmentManager.checkIfShouldMoveSystemRegionAsync();
942944

943945
status.setStatus("Starting quota manager");

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

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,9 @@ public class RegionStateStore {
7373
* @return A ServerName instance or {@link HRegionInfo#getServerName(Result)}
7474
* if necessary fields not found or empty.
7575
*/
76-
static ServerName getRegionServer(final Result r, int replicaId) {
76+
static ServerName getRegionServer(final Result r, int replicaId, boolean isZKAssignmentInUse) {
7777
Cell cell = r.getColumnLatestCell(HConstants.CATALOG_FAMILY, getServerNameColumn(replicaId));
78-
if (cell == null || cell.getValueLength() == 0) {
78+
if (cell == null || cell.getValueLength() == 0 || isZKAssignmentInUse) {
7979
RegionLocations locations = MetaTableAccessor.getRegionLocations(r);
8080
if (locations != null) {
8181
HRegionLocation location = locations.getRegionLocation(replicaId);
@@ -89,7 +89,7 @@ static ServerName getRegionServer(final Result r, int replicaId) {
8989
cell.getValueOffset(), cell.getValueLength()));
9090
}
9191

92-
private static byte[] getServerNameColumn(int replicaId) {
92+
static byte[] getServerNameColumn(int replicaId) {
9393
return replicaId == 0
9494
? HConstants.SERVERNAME_QUALIFIER
9595
: Bytes.toBytes(HConstants.SERVERNAME_QUALIFIER_STR + META_REPLICA_ID_DELIMITER
@@ -101,14 +101,16 @@ private static byte[] getServerNameColumn(int replicaId) {
101101
* @param r Result to pull the region state from
102102
* @return the region state, or OPEN if there's no value written.
103103
*/
104-
static State getRegionState(final Result r, int replicaId) {
104+
static State getRegionState(final Result r, int replicaId, boolean isZKAssignmentInUse) {
105105
Cell cell = r.getColumnLatestCell(HConstants.CATALOG_FAMILY, getStateColumn(replicaId));
106-
if (cell == null || cell.getValueLength() == 0) return State.OPEN;
107-
return State.valueOf(Bytes.toString(cell.getValueArray(),
108-
cell.getValueOffset(), cell.getValueLength()));
106+
if (cell == null || cell.getValueLength() == 0 || isZKAssignmentInUse) {
107+
return State.OPEN;
108+
}
109+
return State
110+
.valueOf(Bytes.toString(cell.getValueArray(), cell.getValueOffset(), cell.getValueLength()));
109111
}
110112

111-
private static byte[] getStateColumn(int replicaId) {
113+
static byte[] getStateColumn(int replicaId) {
112114
return replicaId == 0
113115
? HConstants.STATE_QUALIFIER
114116
: Bytes.toBytes(HConstants.STATE_QUALIFIER_STR + META_REPLICA_ID_DELIMITER

hbase-server/src/main/java/org/apache/hadoop/hbase/util/ConfigUtil.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,12 @@ public static boolean useZKForAssignment(Configuration conf) {
3030
// To change the default, please also update ZooKeeperWatcher.java
3131
return conf.getBoolean("hbase.assignment.usezk", true);
3232
}
33+
34+
public static boolean isZKAssignmentInUse(Configuration conf) {
35+
// ZK based region assignment is in use only if "hbase.assignment.usezk" is true
36+
// and "hbase.assignment.usezk.migrating" is false.
37+
return ConfigUtil.useZKForAssignment(conf) && !conf
38+
.getBoolean("hbase.assignment.usezk.migrating", false);
39+
}
40+
3341
}
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
/*
2+
*
3+
* Licensed to the Apache Software Foundation (ASF) under one
4+
* or more contributor license agreements. See the NOTICE file
5+
* distributed with this work for additional information
6+
* regarding copyright ownership. The ASF licenses this file
7+
* to you under the Apache License, Version 2.0 (the
8+
* "License"); you may not use this file except in compliance
9+
* with the License. You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
*/
19+
20+
package org.apache.hadoop.hbase.client;
21+
22+
import java.util.List;
23+
import org.junit.Assert;
24+
import org.junit.Test;
25+
import org.junit.experimental.categories.Category;
26+
import org.apache.hadoop.conf.Configuration;
27+
import org.apache.hadoop.hbase.Cell;
28+
import org.apache.hadoop.hbase.HBaseTestingUtility;
29+
import org.apache.hadoop.hbase.HConstants;
30+
import org.apache.hadoop.hbase.MetaTableAccessor;
31+
import org.apache.hadoop.hbase.TableName;
32+
import org.apache.hadoop.hbase.testclassification.LargeTests;
33+
import org.apache.hadoop.hbase.util.Bytes;
34+
35+
@Category({ LargeTests.class})
36+
public class TestZKLessAssignment {
37+
38+
private final static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
39+
40+
@Test
41+
public void testZkLessAssignmentRollbackAndRollForward() throws Exception {
42+
Configuration config = TEST_UTIL.getConfiguration();
43+
config.setBoolean("hbase.assignment.usezk.migrating", true);
44+
TEST_UTIL.startMiniCluster(2);
45+
TableName tableName = TableName.valueOf("testZkLessAssignmentRollbackAndRollForward");
46+
TEST_UTIL.createTable(tableName.getName(), new byte[][] { Bytes.toBytes("cf1") }, config)
47+
.close();
48+
49+
try (Connection connection = ConnectionFactory.createConnection(config)) {
50+
List<Result> results = MetaTableAccessor.fullScanOfMeta(connection);
51+
boolean isSNQualifierExist = false;
52+
boolean isStateQualifierExist = false;
53+
for (Result result : results) {
54+
Cell cell =
55+
result.getColumnLatestCell(HConstants.CATALOG_FAMILY, HConstants.SERVERNAME_QUALIFIER);
56+
if (cell != null && cell.getValueLength() > 0) {
57+
isSNQualifierExist = true;
58+
}
59+
cell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY, HConstants.STATE_QUALIFIER);
60+
if (cell != null && cell.getValueLength() > 0) {
61+
isStateQualifierExist = true;
62+
}
63+
}
64+
Assert.assertTrue(isSNQualifierExist);
65+
Assert.assertTrue(isStateQualifierExist);
66+
}
67+
68+
TEST_UTIL.shutdownMiniHBaseCluster();
69+
config.unset("hbase.assignment.usezk.migrating");
70+
TEST_UTIL.restartHBaseCluster(2);
71+
72+
try (Connection connection = ConnectionFactory.createConnection(config)) {
73+
List<Result> results = MetaTableAccessor.fullScanOfMeta(connection);
74+
boolean isSNQualifierExist = false;
75+
boolean isStateQualifierExist = false;
76+
for (Result result : results) {
77+
Cell cell =
78+
result.getColumnLatestCell(HConstants.CATALOG_FAMILY, HConstants.SERVERNAME_QUALIFIER);
79+
if (cell != null && cell.getValueLength() > 0) {
80+
isSNQualifierExist = true;
81+
}
82+
cell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY, HConstants.STATE_QUALIFIER);
83+
if (cell != null && cell.getValueLength() > 0) {
84+
isStateQualifierExist = true;
85+
}
86+
}
87+
Assert.assertFalse(isSNQualifierExist);
88+
Assert.assertFalse(isStateQualifierExist);
89+
}
90+
}
91+
92+
}

0 commit comments

Comments
 (0)