Apache
/
hbase
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

HBASE-25280 [meta replicas] ArrayIndexOutOfBoundsException in ZKConnectionRegistry (#2652) 

Signed-off-by: Duo Zhang <zhangduo@apache.org>
Signed-off-by: Huaxiang Sun <huaxiangsun@apache.com>
This commit is contained in:
Michael Stack 2020-11-16 08:58:05 -08:00 committed by GitHub
parent 1c85c14994
commit 6210dafc47
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 79 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKConnectionRegistry.java
View File

@ -25,7 +25,10 @@ import static org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil.lengthOfPBMag
import static org.apache.hadoop.hbase.util.FutureUtils.addListener;
import static org.apache.hadoop.hbase.zookeeper.ZKMetadata.removeMetaData;
import java.io.IOException;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import java.util.concurrent.CompletableFuture;
import java.util.stream.Collectors;
import org.apache.commons.lang3.mutable.MutableInt;
@ -111,7 +114,7 @@ class ZKConnectionRegistry implements ConnectionRegistry {
data.length - prefixLen);
}
private static void tryComplete(MutableInt remaining, HRegionLocation[] locs,
private static void tryComplete(MutableInt remaining, Collection<HRegionLocation> locs,
CompletableFuture<RegionLocations> future) {
remaining.decrement();
if (remaining.intValue() > 0) {
@ -138,8 +141,15 @@ class ZKConnectionRegistry implements ConnectionRegistry {
if (metaReplicaZNodes.isEmpty()) {
future.completeExceptionally(new IOException("No meta znode available"));
}
HRegionLocation[] locs = new HRegionLocation[metaReplicaZNodes.size()];
MutableInt remaining = new MutableInt(locs.length);
// Note, the list of metaReplicaZNodes may be discontiguous regards replicaId; i.e. we may have
// a znode for the default -- replicaId=0 -- and perhaps replicaId '2' but be could be missing
// znode for replicaId '1'. This is a transient condition. Because of this we are careful
// accumulating locations. We use a Map so retries overwrite rather than aggregate and the
// Map sorts just to be kind to further processing. The Map will retain the discontinuity on
// replicaIds but on completion (of the future), the Map values are passed to the
// RegionLocations constructor which knows how to deal with discontinuities.
final Map<Integer, HRegionLocation> locs = new TreeMap<>();
MutableInt remaining = new MutableInt(metaReplicaZNodes.size());
for (String metaReplicaZNode : metaReplicaZNodes) {
int replicaId = znodePaths.getMetaReplicaIdFromZNode(metaReplicaZNode);
String path = ZNodePaths.joinZNode(znodePaths.baseZNode, metaReplicaZNode);
@ -157,9 +167,9 @@ class ZKConnectionRegistry implements ConnectionRegistry {
if (stateAndServerName.getFirst() != RegionState.State.OPEN) {
LOG.warn("Meta region is in state " + stateAndServerName.getFirst());
}
locs[DEFAULT_REPLICA_ID] = new HRegionLocation(
getRegionInfoForDefaultReplica(FIRST_META_REGIONINFO), stateAndServerName.getSecond());
tryComplete(remaining, locs, future);
locs.put(replicaId, new HRegionLocation(
getRegionInfoForDefaultReplica(FIRST_META_REGIONINFO), stateAndServerName.getSecond()));
tryComplete(remaining, locs.values(), future);
});
} else {
addListener(getAndConvert(path, ZKConnectionRegistry::getMetaProto), (proto, error) -> {
@ -168,23 +178,23 @@ class ZKConnectionRegistry implements ConnectionRegistry {
}
if (error != null) {
LOG.warn("Failed to fetch " + path, error);
locs[replicaId] = null;
locs.put(replicaId, null);
} else if (proto == null) {
LOG.warn("Meta znode for replica " + replicaId + " is null");
locs[replicaId] = null;
locs.put(replicaId, null);
} else {
Pair<RegionState.State, ServerName> stateAndServerName = getStateAndServerName(proto);
if (stateAndServerName.getFirst() != RegionState.State.OPEN) {
LOG.warn("Meta region for replica " + replicaId + " is in state " +
stateAndServerName.getFirst());
locs[replicaId] = null;
locs.put(replicaId, null);
} else {
locs[replicaId] =
new HRegionLocation(getRegionInfoForReplica(FIRST_META_REGIONINFO, replicaId),
stateAndServerName.getSecond());
locs.put(replicaId, new HRegionLocation(
getRegionInfoForReplica(FIRST_META_REGIONINFO, replicaId),
stateAndServerName.getSecond()));
}
}
tryComplete(remaining, locs, future);
tryComplete(remaining, locs.values(), future);
});
}
}

61
hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestZKConnectionRegistry.java
View File

@ -24,27 +24,40 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
import java.io.IOException;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.stream.IntStream;
import org.apache.commons.io.IOUtils;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.Abortable;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HBaseTestingUtility;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.HRegionLocation;
import org.apache.hadoop.hbase.RegionLocations;
import org.apache.hadoop.hbase.ServerName;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.master.RegionState;
import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
import org.apache.hadoop.hbase.shaded.protobuf.generated.ZooKeeperProtos;
import org.apache.hadoop.hbase.testclassification.ClientTests;
import org.apache.hadoop.hbase.testclassification.MediumTests;
import org.apache.hadoop.hbase.zookeeper.MiniZooKeeperCluster;
import org.apache.hadoop.hbase.zookeeper.ReadOnlyZKClient;
import org.apache.hadoop.hbase.zookeeper.ZKUtil;
import org.apache.hadoop.hbase.zookeeper.ZKWatcher;
import org.apache.zookeeper.KeeperException;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.rules.TestName;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -55,6 +68,9 @@ public class TestZKConnectionRegistry {
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestZKConnectionRegistry.class);
@Rule
public final TestName name = new TestName();
static final Logger LOG = LoggerFactory.getLogger(TestZKConnectionRegistry.class);
static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
@ -82,8 +98,7 @@ public class TestZKConnectionRegistry {
clusterId);
assertEquals(TEST_UTIL.getHBaseCluster().getMaster().getServerName(),
REGISTRY.getActiveMaster().get());
RegionReplicaTestHelper
.waitUntilAllMetaReplicasAreReady(TEST_UTIL, REGISTRY);
RegionReplicaTestHelper.waitUntilAllMetaReplicasAreReady(TEST_UTIL, REGISTRY);
RegionLocations locs = REGISTRY.getMetaRegionLocations().get();
assertEquals(3, locs.getRegionLocations().length);
IntStream.range(0, 3).forEach(i -> {
@ -101,8 +116,8 @@ public class TestZKConnectionRegistry {
otherConf.set(HConstants.ZOOKEEPER_QUORUM, MiniZooKeeperCluster.HOST);
try (ZKConnectionRegistry otherRegistry = new ZKConnectionRegistry(otherConf)) {
ReadOnlyZKClient zk2 = otherRegistry.getZKClient();
assertNotSame("Using a different configuration / quorum should result in different " +
"backing zk connection.", zk1, zk2);
assertNotSame("Using a different configuration / quorum should result in " +
"different backing zk connection.", zk1, zk2);
assertNotEquals(
"Using a different configrution / quorum should be reflected in the zk connection.",
zk1.getConnectString(), zk2.getConnectString());
@ -125,4 +140,40 @@ public class TestZKConnectionRegistry {
}
}
}
/**
* Pass discontinuous list of znodes to registry getMetaRegionLocation. Should work fine.
* It used to throw ArrayOutOfBoundsException. See HBASE-25280.
*/
@Test
public void testDiscontinuousLocations()
throws ExecutionException, InterruptedException, IOException, KeeperException,
TimeoutException {
// Write discontinuous meta replica locations to a zk namespace particular to this test to
// avoid polluting other tests.
Configuration conf = new Configuration(TEST_UTIL.getConfiguration());
conf.set(HConstants.ZOOKEEPER_ZNODE_PARENT, "/" + this.name.getMethodName());
ZooKeeperProtos.MetaRegionServer pbrsr = ZooKeeperProtos.MetaRegionServer.newBuilder()
.setServer(ProtobufUtil.toServerName(ServerName.valueOf("example.org,1,1")))
.setRpcVersion(HConstants.RPC_CURRENT_VERSION)
.setState(RegionState.State.OPEN.convert()).build();
byte[] data = ProtobufUtil.prependPBMagic(pbrsr.toByteArray());
try (ZKWatcher zkw = new ZKWatcher(conf, this.name.getMethodName(), new Abortable() {
@Override public void abort(String why, Throwable e) {}
@Override public boolean isAborted() {
return false;
}
})) {
// Write default replica and then a replica for replicaId #3.
ZKUtil.createSetData(zkw, zkw.getZNodePaths().getZNodeForReplica(0), data);
ZKUtil.createSetData(zkw, zkw.getZNodePaths().getZNodeForReplica(3), data);
List<String> znodes = zkw.getMetaReplicaNodes();
assertEquals(2, znodes.size());
try (ZKConnectionRegistry registry = new ZKConnectionRegistry(conf)) {
CompletableFuture<RegionLocations> cf = registry.getMetaRegionLocations();
RegionLocations locations = cf.get(60, TimeUnit.SECONDS);
assertEquals(2, locations.numNonNullElements());
}
}
}
}