Apache9 commented on a change in pull request #2652:
URL: https://github.com/apache/hbase/pull/2652#discussion_r522665883



##########
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:
       The problem is for RegionLocations. As Huaxiang pointed out that in 
RegionLocations we will handle the out of order replicas, I think it is fine to 
do something like this here. Maybe we could use a List instead of an array? And 
add a comment here to say that "we do not care about the order of the replicas 
or if there are holes, the constructor of RegionLocations will handle this". 
And we could add a Constructor for RegionLoations to accept a List. And for the 
old constructor which accepts an array, just do this(Arrays.asList(locations));




----------------------------------------------------------------
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]


Reply via email to