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



##########
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:
       Thanks for the review @Apache9 
   
   I need help understanding here why returning an array with empty slots is 
the way to go instead? What you thinking? Is it that you are afraid that 
elsewhere we are using replicaid as index?
   
   When you say find the max replicaid, are you suggesting the max replicaid 
present in the list metaReplicaZNodes (doable)?  Or are you thinking the array 
should be same as the configured replica count (hard -- hard to pass an active 
table schema reference down to ConnectionRegistry)?
   
   Thanks for the fast review. 
   




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to