huaxiangsun commented on a change in pull request #2652:
URL: https://github.com/apache/hbase/pull/2652#discussion_r522623604
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKConnectionRegistry.java
##########
@@ -133,13 +133,17 @@ private static void tryComplete(MutableInt remaining,
HRegionLocation[] locs,
ServerName.valueOf(snProto.getHostName(), snProto.getPort(),
snProto.getStartCode()));
}
- private void getMetaRegionLocation(CompletableFuture<RegionLocations> future,
+ @VisibleForTesting
+ void getMetaRegionLocation(CompletableFuture<RegionLocations> future,
List<String> metaReplicaZNodes) {
if (metaReplicaZNodes.isEmpty()) {
future.completeExceptionally(new IOException("No meta znode available"));
}
HRegionLocation[] locs = new HRegionLocation[metaReplicaZNodes.size()];
MutableInt remaining = new MutableInt(locs.length);
+ // Do NOT use replicaid as index into locations array. The location set
may not be complete
Review comment:
Based on Duo and Stack's comments, I spend quite some time trying to
understand how `locs` is being used.
It is being used in
https://github.com/apache/hbase/blob/master/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKConnectionRegistry.java#L162
It is used for initialize RegionLocations() object.
Within RegionLocations(), all logic is there trying to allocate a location
array with replica id as index.
https://github.com/apache/hbase/blob/master/hbase-client/src/main/java/org/apache/hadoop/hbase/RegionLocations.java#L74
So it seems that at least in this place, it is fine to do in this way.
Otherwise, the logic is duplicated twice?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
