Skip to content

Commit e2a9f11

Browse files
bharathvndimiduk
authored andcommitted
HBASE-23604: Clarify AsyncRegistry usage in the code. (#957)
* HBASE-23604: Cleanup AsyncRegistry interface - Cleans up the method names to make more sense and adds a little more javadocs for context. In future patches we can revisit the name of the actual class to make it more self explanatory. - Does AsyncRegistry -> ConnectionRegistry rename. "async" ness of the registry is kind of implicit based on the interface contents and need not be reflected in the name. Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: stack <stack@apache.org> Signed-off-by: Viraj Jasani <vjasani@apache.org>
1 parent bc891a8 commit e2a9f11

29 files changed

+124
-111
lines changed

hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ class AsyncConnectionImpl implements AsyncConnection {
8585

8686
private final User user;
8787

88-
final AsyncRegistry registry;
88+
final ConnectionRegistry registry;
8989

9090
private final int rpcTimeout;
9191

@@ -122,7 +122,7 @@ class AsyncConnectionImpl implements AsyncConnection {
122122

123123
private volatile ConnectionOverAsyncConnection conn;
124124

125-
public AsyncConnectionImpl(Configuration conf, AsyncRegistry registry, String clusterId,
125+
public AsyncConnectionImpl(Configuration conf, ConnectionRegistry registry, String clusterId,
126126
SocketAddress localAddress, User user) {
127127
this.conf = conf;
128128
this.user = user;
@@ -136,7 +136,8 @@ public AsyncConnectionImpl(Configuration conf, AsyncRegistry registry, String cl
136136
} else {
137137
this.metrics = Optional.empty();
138138
}
139-
this.rpcClient = RpcClientFactory.createClient(conf, clusterId, localAddress, metrics.orElse(null));
139+
this.rpcClient = RpcClientFactory.createClient(
140+
conf, clusterId, localAddress, metrics.orElse(null));
140141
this.rpcControllerFactory = RpcControllerFactory.instantiate(conf);
141142
this.hostnameCanChange = conf.getBoolean(RESOLVE_HOSTNAME_ON_FAIL_KEY, true);
142143
this.rpcTimeout =
@@ -257,7 +258,7 @@ AdminService.Interface getAdminStub(ServerName serverName) throws IOException {
257258
CompletableFuture<MasterService.Interface> getMasterStub() {
258259
return ConnectionUtils.getOrFetch(masterStub, masterStubMakeFuture, false, () -> {
259260
CompletableFuture<MasterService.Interface> future = new CompletableFuture<>();
260-
addListener(registry.getMasterAddress(), (addr, error) -> {
261+
addListener(registry.getActiveMaster(), (addr, error) -> {
261262
if (error != null) {
262263
future.completeExceptionally(error);
263264
} else if (addr == null) {
@@ -368,7 +369,7 @@ public Connection toConnection() {
368369
@Override
369370
public CompletableFuture<Hbck> getHbck() {
370371
CompletableFuture<Hbck> future = new CompletableFuture<>();
371-
addListener(registry.getMasterAddress(), (sn, error) -> {
372+
addListener(registry.getActiveMaster(), (sn, error) -> {
372373
if (error != null) {
373374
future.completeExceptionally(error);
374375
} else {

hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncMetaRegionLocator.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,14 @@
3838
@InterfaceAudience.Private
3939
class AsyncMetaRegionLocator {
4040

41-
private final AsyncRegistry registry;
41+
private final ConnectionRegistry registry;
4242

4343
private final AtomicReference<RegionLocations> metaRegionLocations = new AtomicReference<>();
4444

4545
private final AtomicReference<CompletableFuture<RegionLocations>> metaRelocateFuture =
4646
new AtomicReference<>();
4747

48-
AsyncMetaRegionLocator(AsyncRegistry registry) {
48+
AsyncMetaRegionLocator(ConnectionRegistry registry) {
4949
this.registry = registry;
5050
}
5151

@@ -60,7 +60,7 @@ class AsyncMetaRegionLocator {
6060
*/
6161
CompletableFuture<RegionLocations> getRegionLocations(int replicaId, boolean reload) {
6262
return ConnectionUtils.getOrFetch(metaRegionLocations, metaRelocateFuture, reload,
63-
registry::getMetaRegionLocation, locs -> isGood(locs, replicaId), "meta region location");
63+
registry::getMetaRegionLocations, locs -> isGood(locs, replicaId), "meta region location");
6464
}
6565

6666
private HRegionLocation getCacheLocation(HRegionLocation loc) {

hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTableRegionLocatorImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public CompletableFuture<HRegionLocation> getRegionLocation(byte[] row, int repl
5555
@Override
5656
public CompletableFuture<List<HRegionLocation>> getAllRegionLocations() {
5757
if (TableName.isMetaTableName(tableName)) {
58-
return conn.registry.getMetaRegionLocation()
58+
return conn.registry.getMetaRegionLocations()
5959
.thenApply(locs -> Arrays.asList(locs.getRegionLocations()));
6060
}
6161
return AsyncMetaTableAccessor.getTableHRegionLocations(conn.getTable(TableName.META_TABLE_NAME),

hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ public static CompletableFuture<AsyncConnection> createAsyncConnection(Configura
279279
public static CompletableFuture<AsyncConnection> createAsyncConnection(Configuration conf,
280280
final User user) {
281281
CompletableFuture<AsyncConnection> future = new CompletableFuture<>();
282-
AsyncRegistry registry = AsyncRegistryFactory.getRegistry(conf);
282+
ConnectionRegistry registry = ConnectionRegistryFactory.getRegistry(conf);
283283
addListener(registry.getClusterId(), (clusterId, error) -> {
284284
if (error != null) {
285285
registry.close();

hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegistry.java renamed to hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionRegistry.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,17 @@
2424
import org.apache.yetus.audience.InterfaceAudience;
2525

2626
/**
27-
* Implementations hold cluster information such as this cluster's id, location of hbase:meta, etc..
27+
* Registry for meta information needed for connection setup to a HBase cluster. Implementations
28+
* hold cluster information such as this cluster's id, location of hbase:meta, etc..
2829
* Internal use only.
2930
*/
3031
@InterfaceAudience.Private
31-
interface AsyncRegistry extends Closeable {
32+
interface ConnectionRegistry extends Closeable {
3233

3334
/**
34-
* Get the location of meta region.
35+
* Get the location of meta region(s).
3536
*/
36-
CompletableFuture<RegionLocations> getMetaRegionLocation();
37+
CompletableFuture<RegionLocations> getMetaRegionLocations();
3738

3839
/**
3940
* Should only be called once.
@@ -43,9 +44,9 @@ interface AsyncRegistry extends Closeable {
4344
CompletableFuture<String> getClusterId();
4445

4546
/**
46-
* Get the address of HMaster.
47+
* Get the address of active HMaster.
4748
*/
48-
CompletableFuture<ServerName> getMasterAddress();
49+
CompletableFuture<ServerName> getActiveMaster();
4950

5051
/**
5152
* Closes this instance and releases any system resources associated with it

hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegistryFactory.java renamed to hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionRegistryFactory.java

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/**
1+
/*
22
* Licensed to the Apache Software Foundation (ASF) under one
33
* or more contributor license agreements. See the NOTICE file
44
* distributed with this work for additional information
@@ -18,26 +18,28 @@
1818
package org.apache.hadoop.hbase.client;
1919

2020
import org.apache.hadoop.conf.Configuration;
21-
import org.apache.yetus.audience.InterfaceAudience;
2221
import org.apache.hadoop.hbase.util.ReflectionUtils;
22+
import org.apache.yetus.audience.InterfaceAudience;
2323

2424
/**
25-
* Get instance of configured Registry.
25+
* Factory class to get the instance of configured connection registry.
2626
*/
2727
@InterfaceAudience.Private
28-
final class AsyncRegistryFactory {
28+
final class ConnectionRegistryFactory {
2929

30-
static final String REGISTRY_IMPL_CONF_KEY = "hbase.client.registry.impl";
30+
static final String CLIENT_CONNECTION_REGISTRY_IMPL_CONF_KEY =
31+
"hbase.client.connection.registry.impl";
3132

32-
private AsyncRegistryFactory() {
33+
private ConnectionRegistryFactory() {
3334
}
3435

3536
/**
36-
* @return The cluster registry implementation to use.
37+
* @return The connection registry implementation to use.
3738
*/
38-
static AsyncRegistry getRegistry(Configuration conf) {
39-
Class<? extends AsyncRegistry> clazz =
40-
conf.getClass(REGISTRY_IMPL_CONF_KEY, ZKAsyncRegistry.class, AsyncRegistry.class);
39+
static ConnectionRegistry getRegistry(Configuration conf) {
40+
Class<? extends ConnectionRegistry> clazz = conf.getClass(
41+
CLIENT_CONNECTION_REGISTRY_IMPL_CONF_KEY, ZKConnectionRegistry.class,
42+
ConnectionRegistry.class);
4143
return ReflectionUtils.newInstance(clazz, conf);
4244
}
4345
}

hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -709,7 +709,7 @@ public CompletableFuture<Boolean> isTableDisabled(TableName tableName) {
709709
@Override
710710
public CompletableFuture<Boolean> isTableAvailable(TableName tableName) {
711711
if (TableName.isMetaTableName(tableName)) {
712-
return connection.registry.getMetaRegionLocation().thenApply(locs -> Stream
712+
return connection.registry.getMetaRegionLocations().thenApply(locs -> Stream
713713
.of(locs.getRegionLocations()).allMatch(loc -> loc != null && loc.getServerName() != null));
714714
}
715715
CompletableFuture<Boolean> future = new CompletableFuture<>();
@@ -847,7 +847,7 @@ public CompletableFuture<List<RegionInfo>> getRegions(ServerName serverName) {
847847
@Override
848848
public CompletableFuture<List<RegionInfo>> getRegions(TableName tableName) {
849849
if (tableName.equals(META_TABLE_NAME)) {
850-
return connection.registry.getMetaRegionLocation()
850+
return connection.registry.getMetaRegionLocations()
851851
.thenApply(locs -> Stream.of(locs.getRegionLocations()).map(HRegionLocation::getRegion)
852852
.collect(Collectors.toList()));
853853
} else {
@@ -1075,8 +1075,9 @@ private CompletableFuture<List<HRegionLocation>> getTableHRegionLocations(TableN
10751075
if (TableName.META_TABLE_NAME.equals(tableName)) {
10761076
CompletableFuture<List<HRegionLocation>> future = new CompletableFuture<>();
10771077
// For meta table, we use zk to fetch all locations.
1078-
AsyncRegistry registry = AsyncRegistryFactory.getRegistry(connection.getConfiguration());
1079-
addListener(registry.getMetaRegionLocation(), (metaRegions, err) -> {
1078+
ConnectionRegistry registry = ConnectionRegistryFactory.getRegistry(
1079+
connection.getConfiguration());
1080+
addListener(registry.getMetaRegionLocations(), (metaRegions, err) -> {
10801081
if (err != null) {
10811082
future.completeExceptionally(err);
10821083
} else if (metaRegions == null || metaRegions.isEmpty() ||
@@ -1104,7 +1105,7 @@ private CompletableFuture<Void> compact(TableName tableName, byte[] columnFamily
11041105

11051106
switch (compactType) {
11061107
case MOB:
1107-
addListener(connection.registry.getMasterAddress(), (serverName, err) -> {
1108+
addListener(connection.registry.getActiveMaster(), (serverName, err) -> {
11081109
if (err != null) {
11091110
future.completeExceptionally(err);
11101111
return;
@@ -2343,7 +2344,7 @@ CompletableFuture<HRegionLocation> getRegionLocation(byte[] regionNameOrEncodedR
23432344
String encodedName = Bytes.toString(regionNameOrEncodedRegionName);
23442345
if (encodedName.length() < RegionInfo.MD5_HEX_LENGTH) {
23452346
// old format encodedName, should be meta region
2346-
future = connection.registry.getMetaRegionLocation()
2347+
future = connection.registry.getMetaRegionLocations()
23472348
.thenApply(locs -> Stream.of(locs.getRegionLocations())
23482349
.filter(loc -> loc.getRegion().getEncodedName().equals(encodedName)).findFirst());
23492350
} else {
@@ -2354,7 +2355,7 @@ CompletableFuture<HRegionLocation> getRegionLocation(byte[] regionNameOrEncodedR
23542355
RegionInfo regionInfo =
23552356
MetaTableAccessor.parseRegionInfoFromRegionName(regionNameOrEncodedRegionName);
23562357
if (regionInfo.isMetaRegion()) {
2357-
future = connection.registry.getMetaRegionLocation()
2358+
future = connection.registry.getMetaRegionLocations()
23582359
.thenApply(locs -> Stream.of(locs.getRegionLocations())
23592360
.filter(loc -> loc.getRegion().getReplicaId() == regionInfo.getReplicaId())
23602361
.findFirst());
@@ -2927,7 +2928,7 @@ public CompletableFuture<CompactionState> getCompactionState(TableName tableName
29272928

29282929
switch (compactType) {
29292930
case MOB:
2930-
addListener(connection.registry.getMasterAddress(), (serverName, err) -> {
2931+
addListener(connection.registry.getActiveMaster(), (serverName, err) -> {
29312932
if (err != null) {
29322933
future.completeExceptionally(err);
29332934
return;

hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKAsyncRegistry.java renamed to hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKConnectionRegistry.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,15 @@
5050
* Zookeeper based registry implementation.
5151
*/
5252
@InterfaceAudience.Private
53-
class ZKAsyncRegistry implements AsyncRegistry {
53+
class ZKConnectionRegistry implements ConnectionRegistry {
5454

55-
private static final Logger LOG = LoggerFactory.getLogger(ZKAsyncRegistry.class);
55+
private static final Logger LOG = LoggerFactory.getLogger(ZKConnectionRegistry.class);
5656

5757
private final ReadOnlyZKClient zk;
5858

5959
private final ZNodePaths znodePaths;
6060

61-
ZKAsyncRegistry(Configuration conf) {
61+
ZKConnectionRegistry(Configuration conf) {
6262
this.znodePaths = new ZNodePaths(conf);
6363
this.zk = new ReadOnlyZKClient(conf);
6464
}
@@ -93,7 +93,7 @@ private static String getClusterId(byte[] data) throws DeserializationException
9393

9494
@Override
9595
public CompletableFuture<String> getClusterId() {
96-
return getAndConvert(znodePaths.clusterIdZNode, ZKAsyncRegistry::getClusterId);
96+
return getAndConvert(znodePaths.clusterIdZNode, ZKConnectionRegistry::getClusterId);
9797
}
9898

9999
@VisibleForTesting
@@ -144,7 +144,7 @@ private void getMetaRegionLocation(CompletableFuture<RegionLocations> future,
144144
int replicaId = znodePaths.getMetaReplicaIdFromZnode(metaReplicaZNode);
145145
String path = ZNodePaths.joinZNode(znodePaths.baseZNode, metaReplicaZNode);
146146
if (replicaId == DEFAULT_REPLICA_ID) {
147-
addListener(getAndConvert(path, ZKAsyncRegistry::getMetaProto), (proto, error) -> {
147+
addListener(getAndConvert(path, ZKConnectionRegistry::getMetaProto), (proto, error) -> {
148148
if (error != null) {
149149
future.completeExceptionally(error);
150150
return;
@@ -162,7 +162,7 @@ private void getMetaRegionLocation(CompletableFuture<RegionLocations> future,
162162
tryComplete(remaining, locs, future);
163163
});
164164
} else {
165-
addListener(getAndConvert(path, ZKAsyncRegistry::getMetaProto), (proto, error) -> {
165+
addListener(getAndConvert(path, ZKConnectionRegistry::getMetaProto), (proto, error) -> {
166166
if (future.isDone()) {
167167
return;
168168
}
@@ -191,7 +191,7 @@ private void getMetaRegionLocation(CompletableFuture<RegionLocations> future,
191191
}
192192

193193
@Override
194-
public CompletableFuture<RegionLocations> getMetaRegionLocation() {
194+
public CompletableFuture<RegionLocations> getMetaRegionLocations() {
195195
CompletableFuture<RegionLocations> future = new CompletableFuture<>();
196196
addListener(
197197
zk.list(znodePaths.baseZNode)
@@ -217,8 +217,8 @@ private static ZooKeeperProtos.Master getMasterProto(byte[] data) throws IOExcep
217217
}
218218

219219
@Override
220-
public CompletableFuture<ServerName> getMasterAddress() {
221-
return getAndConvert(znodePaths.masterAddressZNode, ZKAsyncRegistry::getMasterProto)
220+
public CompletableFuture<ServerName> getActiveMaster() {
221+
return getAndConvert(znodePaths.masterAddressZNode, ZKConnectionRegistry::getMasterProto)
222222
.thenApply(proto -> {
223223
if (proto == null) {
224224
return null;

hbase-client/src/test/java/org/apache/hadoop/hbase/client/DoNothingAsyncRegistry.java renamed to hbase-client/src/test/java/org/apache/hadoop/hbase/client/DoNothingConnectionRegistry.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,13 @@
2727
* Registry that does nothing. Otherwise, default Registry wants zookeeper up and running.
2828
*/
2929
@InterfaceAudience.Private
30-
class DoNothingAsyncRegistry implements AsyncRegistry {
30+
class DoNothingConnectionRegistry implements ConnectionRegistry {
3131

32-
public DoNothingAsyncRegistry(Configuration conf) {
32+
public DoNothingConnectionRegistry(Configuration conf) {
3333
}
3434

3535
@Override
36-
public CompletableFuture<RegionLocations> getMetaRegionLocation() {
36+
public CompletableFuture<RegionLocations> getMetaRegionLocations() {
3737
return CompletableFuture.completedFuture(null);
3838
}
3939

@@ -43,7 +43,7 @@ public CompletableFuture<String> getClusterId() {
4343
}
4444

4545
@Override
46-
public CompletableFuture<ServerName> getMasterAddress() {
46+
public CompletableFuture<ServerName> getActiveMaster() {
4747
return CompletableFuture.completedFuture(null);
4848
}
4949

hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncAdminRpcPriority.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ public Void answer(InvocationOnMock invocation) throws Throwable {
142142
}).when(adminStub).stopServer(any(HBaseRpcController.class), any(StopServerRequest.class),
143143
any());
144144

145-
conn = new AsyncConnectionImpl(CONF, new DoNothingAsyncRegistry(CONF), "test", null,
145+
conn = new AsyncConnectionImpl(CONF, new DoNothingConnectionRegistry(CONF), "test", null,
146146
UserProvider.instantiate(CONF).getCurrent()) {
147147

148148
@Override

hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncMetaRegionLocatorFailFast.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/**
1+
/*
22
* Licensed to the Apache Software Foundation (ASF) under one
33
* or more contributor license agreements. See the NOTICE file
44
* distributed with this work for additional information
@@ -43,21 +43,21 @@ public class TestAsyncMetaRegionLocatorFailFast {
4343

4444
private static AsyncMetaRegionLocator LOCATOR;
4545

46-
private static final class FaultyAsyncRegistry extends DoNothingAsyncRegistry {
46+
private static final class FaultyConnectionRegistry extends DoNothingConnectionRegistry {
4747

48-
public FaultyAsyncRegistry(Configuration conf) {
48+
public FaultyConnectionRegistry(Configuration conf) {
4949
super(conf);
5050
}
5151

5252
@Override
53-
public CompletableFuture<RegionLocations> getMetaRegionLocation() {
53+
public CompletableFuture<RegionLocations> getMetaRegionLocations() {
5454
return FutureUtils.failedFuture(new DoNotRetryRegionException("inject error"));
5555
}
5656
}
5757

5858
@BeforeClass
5959
public static void setUp() {
60-
LOCATOR = new AsyncMetaRegionLocator(new FaultyAsyncRegistry(CONF));
60+
LOCATOR = new AsyncMetaRegionLocator(new FaultyConnectionRegistry(CONF));
6161
}
6262

6363
@Test(expected = DoNotRetryIOException.class)

hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableRpcPriority.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ public Void answer(InvocationOnMock invocation) throws Throwable {
175175
return null;
176176
}
177177
}).when(stub).get(any(HBaseRpcController.class), any(GetRequest.class), any());
178-
conn = new AsyncConnectionImpl(CONF, new DoNothingAsyncRegistry(CONF), "test", null,
178+
conn = new AsyncConnectionImpl(CONF, new DoNothingConnectionRegistry(CONF), "test", null,
179179
UserProvider.instantiate(CONF).getCurrent()) {
180180

181181
@Override

0 commit comments

Comments
 (0)