You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by bh...@apache.org on 2020/09/19 22:05:02 UTC
[hbase] 06/09: HBASE-23604: Clarify Registry usage in the code
This is an automated email from the ASF dual-hosted git repository.
bharathv pushed a commit to branch branch-1
in repository https://gitbox.apache.org/repos/asf/hbase.git
commit d86699463505f48426beacfb93fe174fbcc30b55
Author: Bharath Vissapragada <bh...@apache.org>
AuthorDate: Wed Sep 2 14:59:18 2020 -0700
HBASE-23604: Clarify Registry usage in the code
Signed-off-by: Andrew Purtell <ap...@apache.org>
---
.../hadoop/hbase/client/ConnectionManager.java | 71 +++++++++-------------
.../{Registry.java => ConnectionRegistry.java} | 12 +++-
...Factory.java => ConnectionRegistryFactory.java} | 12 ++--
...istry.java => ZooKeeperConnectionRegistry.java} | 38 ++++++------
.../hadoop/hbase/client/TestAsyncProcess.java | 11 +++-
.../hadoop/hbase/client/TestClientNoCluster.java | 13 ++--
.../hbase/client/TestMetaRegionLocationCache.java | 8 +--
7 files changed, 83 insertions(+), 82 deletions(-)
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionManager.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionManager.java
index 961ee3a..9798e72 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionManager.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionManager.java
@@ -83,7 +83,7 @@ import org.apache.hadoop.hbase.protobuf.generated.AdminProtos.AdminService;
import org.apache.hadoop.hbase.protobuf.generated.ClientProtos.ClientService;
import org.apache.hadoop.hbase.protobuf.generated.ClientProtos.CoprocessorServiceRequest;
import org.apache.hadoop.hbase.protobuf.generated.ClientProtos.CoprocessorServiceResponse;
-import org.apache.hadoop.hbase.protobuf.generated.*;
+import org.apache.hadoop.hbase.protobuf.generated.MasterProtos;
import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.AddColumnRequest;
import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.AddColumnResponse;
import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.AssignRegionRequest;
@@ -635,7 +635,7 @@ class ConnectionManager {
/**
* Cluster registry of basic info such as clusterid and meta region location.
*/
- Registry registry;
+ ConnectionRegistry registry;
private final ClientBackoffPolicy backoffPolicy;
@@ -920,8 +920,8 @@ class ConnectionManager {
* @return The cluster registry implementation to use.
* @throws IOException
*/
- private Registry setupRegistry() throws IOException {
- return RegistryFactory.getRegistry(this);
+ private ConnectionRegistry setupRegistry() throws IOException {
+ return ConnectionRegistryFactory.getRegistry(this);
}
/**
@@ -1259,7 +1259,7 @@ class ConnectionManager {
}
}
// Look up from zookeeper
- metaLocations = this.registry.getMetaRegionLocation();
+ metaLocations = this.registry.getMetaRegionLocations();
lastMetaLookupTime = EnvironmentEdgeManager.currentTime();
if (metaLocations != null &&
metaLocations.getRegionLocation(replicaId) != null) {
@@ -1589,43 +1589,31 @@ class ConnectionManager {
* @throws KeeperException
* @throws ServiceException
*/
- private Object makeStubNoRetries() throws IOException, KeeperException, ServiceException {
- ZooKeeperKeepAliveConnection zkw;
- try {
- zkw = getKeepAliveZooKeeperWatcher();
- } catch (IOException e) {
- ExceptionUtil.rethrowIfInterrupt(e);
- throw new ZooKeeperConnectionException("Can't connect to ZooKeeper", e);
- }
- try {
- checkIfBaseNodeAvailable(zkw);
- ServerName sn = MasterAddressTracker.getMasterAddress(zkw);
- if (sn == null) {
- String msg = "ZooKeeper available but no active master location found";
- LOG.info(msg);
- throw new MasterNotRunningException(msg);
- }
- if (isDeadServer(sn)) {
- throw new MasterNotRunningException(sn + " is dead.");
+ private Object makeStubNoRetries() throws IOException, ServiceException {
+ ServerName sn = registry.getActiveMaster();
+ if (sn == null) {
+ String msg = "ZooKeeper available but no active master location found";
+ LOG.info(msg);
+ throw new MasterNotRunningException(msg);
+ }
+ if (isDeadServer(sn)) {
+ throw new MasterNotRunningException(sn + " is dead.");
+ }
+ // Use the security info interface name as our stub key
+ String key = getStubKey(getServiceName(),
+ sn.getHostname(), sn.getPort(), hostnamesCanChange);
+ connectionLock.putIfAbsent(key, key);
+ Object stub = null;
+ synchronized (connectionLock.get(key)) {
+ stub = stubs.get(key);
+ if (stub == null) {
+ BlockingRpcChannel channel = rpcClient.createBlockingRpcChannel(sn, user, rpcTimeout);
+ stub = makeStub(channel);
+ isMasterRunning();
+ stubs.put(key, stub);
}
- // Use the security info interface name as our stub key
- String key = getStubKey(getServiceName(),
- sn.getHostname(), sn.getPort(), hostnamesCanChange);
- connectionLock.putIfAbsent(key, key);
- Object stub = null;
- synchronized (connectionLock.get(key)) {
- stub = stubs.get(key);
- if (stub == null) {
- BlockingRpcChannel channel = rpcClient.createBlockingRpcChannel(sn, user, rpcTimeout);
- stub = makeStub(channel);
- isMasterRunning();
- stubs.put(key, stub);
- }
- }
- return stub;
- } finally {
- zkw.close();
}
+ return stub;
}
/**
@@ -1643,12 +1631,9 @@ class ConnectionManager {
return makeStubNoRetries();
} catch (IOException e) {
exceptionCaught = e;
- } catch (KeeperException e) {
- exceptionCaught = e;
} catch (ServiceException e) {
exceptionCaught = e;
}
-
throw new MasterNotRunningException(exceptionCaught);
} else {
throw new DoNotRetryIOException("Connection was closed while trying to get master");
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Registry.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionRegistry.java
similarity index 83%
rename from hbase-client/src/main/java/org/apache/hadoop/hbase/client/Registry.java
rename to hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionRegistry.java
index 9debd63..9c4f22a 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Registry.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionRegistry.java
@@ -19,26 +19,32 @@ package org.apache.hadoop.hbase.client;
import java.io.IOException;
+import org.apache.hadoop.hbase.ServerName;
import org.apache.hadoop.hbase.classification.InterfaceAudience;
import org.apache.hadoop.hbase.RegionLocations;
/**
- * Cluster registry.
* Implementations hold cluster information such as this cluster's id, location of hbase:meta, etc.
+ * needed by cluster connections.
* Internal use only.
*/
@InterfaceAudience.Private
-interface Registry {
+interface ConnectionRegistry {
/**
* @param connection
*/
void init(Connection connection);
/**
+ * @return the currently active master, null if none exists.
+ */
+ ServerName getActiveMaster() throws IOException;
+
+ /**
* @return Meta region location
* @throws IOException
*/
- RegionLocations getMetaRegionLocation() throws IOException;
+ RegionLocations getMetaRegionLocations() throws IOException;
/**
* @return Cluster id.
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegistryFactory.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionRegistryFactory.java
similarity index 79%
rename from hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegistryFactory.java
rename to hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionRegistryFactory.java
index 789e2e1..c166e21 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegistryFactory.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionRegistryFactory.java
@@ -22,23 +22,23 @@ import java.io.IOException;
import org.apache.hadoop.hbase.classification.InterfaceAudience;
/**
- * Get instance of configured Registry.
+ * Get instance of configured Connection Registry.
*/
@InterfaceAudience.Private
-class RegistryFactory {
+class ConnectionRegistryFactory {
static final String REGISTRY_IMPL_CONF_KEY = "hbase.client.registry.impl";
/**
* @return The cluster registry implementation to use.
* @throws IOException
*/
- static Registry getRegistry(final Connection connection)
+ static ConnectionRegistry getRegistry(final Connection connection)
throws IOException {
String registryClass = connection.getConfiguration().get(REGISTRY_IMPL_CONF_KEY,
- ZooKeeperRegistry.class.getName());
- Registry registry = null;
+ ZooKeeperConnectionRegistry.class.getName());
+ ConnectionRegistry registry = null;
try {
- registry = (Registry)Class.forName(registryClass).getDeclaredConstructor().newInstance();
+ registry = (ConnectionRegistry)Class.forName(registryClass).getDeclaredConstructor().newInstance();
} catch (Throwable t) {
throw new IOException(t);
}
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZooKeeperRegistry.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZooKeeperConnectionRegistry.java
similarity index 81%
rename from hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZooKeeperRegistry.java
rename to hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZooKeeperConnectionRegistry.java
index 8f7257e..0401aee 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZooKeeperRegistry.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZooKeeperConnectionRegistry.java
@@ -22,10 +22,12 @@ import java.util.List;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.hbase.HBaseIOException;
import org.apache.hadoop.hbase.HRegionInfo;
import org.apache.hadoop.hbase.HRegionLocation;
import org.apache.hadoop.hbase.RegionLocations;
import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.zookeeper.MasterAddressTracker;
import org.apache.hadoop.hbase.zookeeper.MetaTableLocator;
import org.apache.hadoop.hbase.zookeeper.ZKClusterId;
import org.apache.hadoop.hbase.zookeeper.ZKUtil;
@@ -34,8 +36,8 @@ import org.apache.zookeeper.KeeperException;
/**
* A cluster registry that stores to zookeeper.
*/
-class ZooKeeperRegistry implements Registry {
- private static final Log LOG = LogFactory.getLog(ZooKeeperRegistry.class);
+class ZooKeeperConnectionRegistry implements ConnectionRegistry {
+ private static final Log LOG = LogFactory.getLog(ZooKeeperConnectionRegistry.class);
// Needs an instance of hci to function. Set after construct this instance.
ConnectionManager.HConnectionImplementation hci;
@@ -48,10 +50,19 @@ class ZooKeeperRegistry implements Registry {
}
@Override
- public RegionLocations getMetaRegionLocation() throws IOException {
- ZooKeeperKeepAliveConnection zkw = hci.getKeepAliveZooKeeperWatcher();
+ public ServerName getActiveMaster() throws IOException {
+ ServerName sn;
+ try (ZooKeeperKeepAliveConnection zkw = hci.getKeepAliveZooKeeperWatcher()) {
+ sn = MasterAddressTracker.getMasterAddress(zkw);
+ } catch (KeeperException e) {
+ throw new HBaseIOException(e);
+ }
+ return sn;
+ }
- try {
+ @Override
+ public RegionLocations getMetaRegionLocations() throws IOException {
+ try (ZooKeeperKeepAliveConnection zkw = hci.getKeepAliveZooKeeperWatcher();) {
if (LOG.isTraceEnabled()) {
LOG.trace("Looking up meta region location in ZK," + " connection=" + this);
}
@@ -84,8 +95,6 @@ class ZooKeeperRegistry implements Registry {
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
return null;
- } finally {
- zkw.close();
}
}
@@ -96,34 +105,25 @@ class ZooKeeperRegistry implements Registry {
if (this.clusterId != null) return this.clusterId;
// No synchronized here, worse case we will retrieve it twice, that's
// not an issue.
- ZooKeeperKeepAliveConnection zkw = null;
- try {
- zkw = hci.getKeepAliveZooKeeperWatcher();
+ try (ZooKeeperKeepAliveConnection zkw = hci.getKeepAliveZooKeeperWatcher()) {
this.clusterId = ZKClusterId.readClusterIdZNode(zkw);
if (this.clusterId == null) {
LOG.info("ClusterId read in ZooKeeper is null");
}
- } catch (KeeperException e) {
- LOG.warn("Can't retrieve clusterId from Zookeeper", e);
- } catch (IOException e) {
+ } catch (KeeperException | IOException e) {
LOG.warn("Can't retrieve clusterId from Zookeeper", e);
- } finally {
- if (zkw != null) zkw.close();
}
return this.clusterId;
}
@Override
public int getCurrentNrHRS() throws IOException {
- ZooKeeperKeepAliveConnection zkw = hci.getKeepAliveZooKeeperWatcher();
- try {
+ try (ZooKeeperKeepAliveConnection zkw = hci.getKeepAliveZooKeeperWatcher()) {
// We go to zk rather than to master to get count of regions to avoid
// HTable having a Master dependency. See HBase-2828
return ZKUtil.getNumberOfChildren(zkw, zkw.rsZNode);
} catch (KeeperException ke) {
throw new IOException("Unexpected ZooKeeper exception", ke);
- } finally {
- zkw.close();
}
}
}
diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncProcess.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncProcess.java
index 21e3d85..d10cc2f 100644
--- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncProcess.java
+++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncProcess.java
@@ -460,12 +460,17 @@ public class TestAsyncProcess {
* Returns our async process.
*/
static class MyConnectionImpl extends ConnectionManager.HConnectionImplementation {
- public static class TestRegistry implements Registry {
+ public static class TestConnectionRegistry implements ConnectionRegistry {
@Override
public void init(Connection connection) {}
@Override
- public RegionLocations getMetaRegionLocation() throws IOException {
+ public ServerName getActiveMaster() {
+ return null;
+ }
+
+ @Override
+ public RegionLocations getMetaRegionLocations() throws IOException {
return null;
}
@@ -487,7 +492,7 @@ public class TestAsyncProcess {
}
private static Configuration setupConf(Configuration conf) {
- conf.setClass(RegistryFactory.REGISTRY_IMPL_CONF_KEY, TestRegistry.class, Registry.class);
+ conf.setClass(ConnectionRegistryFactory.REGISTRY_IMPL_CONF_KEY, TestConnectionRegistry.class, ConnectionRegistry.class);
return conf;
}
diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestClientNoCluster.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestClientNoCluster.java
index 06647ca..d2e4f0f 100644
--- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestClientNoCluster.java
+++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestClientNoCluster.java
@@ -105,13 +105,13 @@ public class TestClientNoCluster extends Configured implements Tool {
// Run my HConnection overrides. Use my little HConnectionImplementation below which
// allows me insert mocks and also use my Registry below rather than the default zk based
// one so tests run faster and don't have zk dependency.
- this.conf.set("hbase.client.registry.impl", SimpleRegistry.class.getName());
+ this.conf.set("hbase.client.registry.impl", SimpleConnectionRegistry.class.getName());
}
/**
* Simple cluster registry inserted in place of our usual zookeeper based one.
*/
- static class SimpleRegistry implements Registry {
+ static class SimpleConnectionRegistry implements ConnectionRegistry {
final ServerName META_HOST = META_SERVERNAME;
@Override
@@ -119,7 +119,12 @@ public class TestClientNoCluster extends Configured implements Tool {
}
@Override
- public RegionLocations getMetaRegionLocation() throws IOException {
+ public ServerName getActiveMaster() {
+ return null;
+ }
+
+ @Override
+ public RegionLocations getMetaRegionLocations() throws IOException {
return new RegionLocations(
new HRegionLocation(HRegionInfo.FIRST_META_REGIONINFO, META_HOST));
}
@@ -796,7 +801,7 @@ public class TestClientNoCluster extends Configured implements Tool {
getConf().set("hbase.client.connection.impl",
ManyServersManyRegionsConnection.class.getName());
// Use simple kv registry rather than zk
- getConf().set("hbase.client.registry.impl", SimpleRegistry.class.getName());
+ getConf().set("hbase.client.registry.impl", SimpleConnectionRegistry.class.getName());
// When to report fails. Default is we report the 10th. This means we'll see log everytime
// an exception is thrown -- usually RegionTooBusyException when we have more than
// hbase.test.multi.too.many requests outstanding at any time.
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaRegionLocationCache.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaRegionLocationCache.java
index e9fa26d..f5ba56d 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaRegionLocationCache.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaRegionLocationCache.java
@@ -55,11 +55,11 @@ public class TestMetaRegionLocationCache {
private static final Log LOG = LogFactory.getLog(TestMetaRegionLocationCache.class.getName());
private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
- private static Registry REGISTRY;
+ private static ConnectionRegistry REGISTRY;
// waits for all replicas to have region location
static void waitUntilAllMetaReplicasHavingRegionLocation(Configuration conf,
- final Registry registry, final int regionReplication) throws IOException {
+ final ConnectionRegistry registry, final int regionReplication) throws IOException {
Waiter.waitFor(conf, conf.getLong(
"hbase.client.sync.wait.timeout.msec", 60000), 200, true,
new Waiter.ExplainingPredicate<IOException>() {
@@ -71,7 +71,7 @@ public class TestMetaRegionLocationCache {
@Override
public boolean evaluate() throws IOException {
try {
- RegionLocations locs = registry.getMetaRegionLocation();
+ RegionLocations locs = registry.getMetaRegionLocations();
if (locs == null || locs.size() < regionReplication) {
return false;
}
@@ -93,7 +93,7 @@ public class TestMetaRegionLocationCache {
public static void setUp() throws Exception {
TEST_UTIL.getConfiguration().setInt(HConstants.META_REPLICAS_NUM, 3);
TEST_UTIL.startMiniCluster(3);
- REGISTRY = RegistryFactory.getRegistry(TEST_UTIL.getConnection());
+ REGISTRY = ConnectionRegistryFactory.getRegistry(TEST_UTIL.getConnection());
waitUntilAllMetaReplicasHavingRegionLocation(
TEST_UTIL.getConfiguration(), REGISTRY, 3);
TEST_UTIL.getConnection().getAdmin().setBalancerRunning(false, true);