diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKAsyncRegistry.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKAsyncRegistry.java index c02643ff3f9f..34069d1a0670 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKAsyncRegistry.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKAsyncRegistry.java @@ -26,7 +26,9 @@ import static org.apache.hadoop.hbase.zookeeper.ZKMetadata.removeMetaData; import java.io.IOException; +import java.util.List; import java.util.concurrent.CompletableFuture; +import java.util.stream.Collectors; import org.apache.commons.lang3.mutable.MutableInt; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.ClusterId; @@ -134,12 +136,13 @@ private Pair getStateAndServerName( ServerName.valueOf(snProto.getHostName(), snProto.getPort(), snProto.getStartCode())); } - @Override - public CompletableFuture getMetaRegionLocation() { - CompletableFuture future = new CompletableFuture<>(); - HRegionLocation[] locs = new HRegionLocation[znodePaths.metaReplicaZNodes.size()]; + private void getMetaRegionLocation(CompletableFuture future, + List metaReplicaZNodes) { + HRegionLocation[] locs = new HRegionLocation[metaReplicaZNodes.size()]; MutableInt remaining = new MutableInt(locs.length); - znodePaths.metaReplicaZNodes.forEach((replicaId, path) -> { + for (String metaReplicaZNode : metaReplicaZNodes) { + int replicaId = znodePaths.getMetaReplicaIdFromZnode(metaReplicaZNode); + String path = ZNodePaths.joinZNode(znodePaths.baseZNode, metaReplicaZNode); if (replicaId == DEFAULT_REPLICA_ID) { addListener(getAndConvert(path, ZKAsyncRegistry::getMetaProto), (proto, error) -> { if (error != null) { @@ -186,7 +189,23 @@ public CompletableFuture getMetaRegionLocation() { tryComplete(remaining, locs, future); }); } - }); + } + } + + @Override + public CompletableFuture getMetaRegionLocation() { + CompletableFuture future = new CompletableFuture<>(); + addListener( + zk.list(znodePaths.baseZNode) + .thenApply(children -> children.stream() + .filter(c -> c.startsWith(znodePaths.metaZNodePrefix)).collect(Collectors.toList())), + (metaReplicaZNodes, error) -> { + if (error != null) { + future.completeExceptionally(error); + return; + } + getMetaRegionLocation(future, metaReplicaZNodes); + }); return future; } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java index 9873e831a6ab..b4f3ccba9cd0 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java @@ -24,6 +24,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.EnumSet; +import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.DelayQueue; import java.util.concurrent.Delayed; @@ -284,6 +285,22 @@ protected void doExec(ZooKeeper zk) { return future; } + public CompletableFuture> list(String path) { + if (closed.get()) { + return FutureUtils.failedFuture(new DoNotRetryIOException("Client already closed")); + } + CompletableFuture> future = new CompletableFuture<>(); + tasks.add(new ZKTask>(path, future, "list") { + + @Override + protected void doExec(ZooKeeper zk) { + zk.getChildren(path, false, (rc, path, ctx, children) -> onComplete(zk, rc, children, true), + null); + } + }); + return future; + } + private void closeZk() { if (zookeeper != null) { try { diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZNodePaths.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZNodePaths.java index 32792f64e645..b9a34120fdd8 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZNodePaths.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZNodePaths.java @@ -166,7 +166,7 @@ public String getZNodeForReplica(int replicaId) { /** * Parse the meta replicaId from the passed znode - * @param znode + * @param znode the name of the znode, does not include baseZNode * @return replicaId */ public int getMetaReplicaIdFromZnode(String znode) { @@ -178,7 +178,7 @@ public int getMetaReplicaIdFromZnode(String znode) { /** * Is it the default meta replica's znode - * @param znode + * @param znode the name of the znode, does not include baseZNode * @return true or false */ public boolean isDefaultMetaReplicaZnode(String znode) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestZKAsyncRegistry.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestZKAsyncRegistry.java index 5a2d738386b9..a4441b81d9de 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestZKAsyncRegistry.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestZKAsyncRegistry.java @@ -61,7 +61,11 @@ public class TestZKAsyncRegistry { public static void setUp() throws Exception { TEST_UTIL.getConfiguration().setInt(META_REPLICAS_NUM, 3); TEST_UTIL.startMiniCluster(3); - REGISTRY = new ZKAsyncRegistry(TEST_UTIL.getConfiguration()); + Configuration conf = new Configuration(TEST_UTIL.getConfiguration()); + // make sure that we do not depend on this config when getting locations for meta replicas, see + // HBASE-21658. + conf.setInt(META_REPLICAS_NUM, 1); + REGISTRY = new ZKAsyncRegistry(conf); } @AfterClass diff --git a/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestReadOnlyZKClient.java b/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestReadOnlyZKClient.java index a97a7c617464..1da73e994f89 100644 --- a/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestReadOnlyZKClient.java +++ b/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestReadOnlyZKClient.java @@ -38,6 +38,8 @@ import static org.mockito.Mockito.when; import java.io.IOException; +import java.util.Collections; +import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Exchanger; import java.util.concurrent.ExecutionException; @@ -126,9 +128,15 @@ public String explainFailure() throws Exception { } @Test - public void testGetAndExists() throws Exception { + public void testRead() throws Exception { assertArrayEquals(DATA, RO_ZK.get(PATH).get()); assertEquals(CHILDREN, RO_ZK.exists(PATH).get().getNumChildren()); + List children = RO_ZK.list(PATH).get(); + assertEquals(CHILDREN, children.size()); + Collections.sort(children); + for (int i = 0; i < CHILDREN; i++) { + assertEquals("c" + i, children.get(i)); + } assertNotNull(RO_ZK.zookeeper); waitForIdleConnectionClosed(); } @@ -145,6 +153,15 @@ public void testNoNode() throws InterruptedException, ExecutionException { assertEquals(Code.NONODE, ke.code()); assertEquals(pathNotExists, ke.getPath()); } + try { + RO_ZK.list(pathNotExists).get(); + fail("should fail because of " + pathNotExists + " does not exist"); + } catch (ExecutionException e) { + assertThat(e.getCause(), instanceOf(KeeperException.class)); + KeeperException ke = (KeeperException) e.getCause(); + assertEquals(Code.NONODE, ke.code()); + assertEquals(pathNotExists, ke.getPath()); + } // exists will not throw exception. assertNull(RO_ZK.exists(pathNotExists).get()); }