wchevreuil commented on a change in pull request #2118:
URL: https://github.com/apache/hbase/pull/2118#discussion_r459572083



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSinkManager.java
##########
@@ -150,9 +151,14 @@ public synchronized void reportSinkSuccess(SinkPeer 
sinkPeer) {
    */
   public synchronized void chooseSinks() {
     List<ServerName> slaveAddresses = endpoint.getRegionServers();
-    Collections.shuffle(slaveAddresses, ThreadLocalRandom.current());
-    int numSinks = (int) Math.ceil(slaveAddresses.size() * ratio);
-    sinks = slaveAddresses.subList(0, numSinks);
+    if(slaveAddresses==null){

Review comment:
       > Doesn't look like HBaseReplicationEndpoint ever returns null. Guarding 
against custom endpoint implementations?
   
   Yeah, that's indeed the case. 
   
   >We should expose getRegionServers on a base-class or interface and 
explicitly say that we expect a non-null answer. Follow-on..
   
   If easy, this would be good to write a quick unit test to cover this method.
   
   Sounds good, actually. So we can leave `getRegionServers` at 
HBaseReplicationEndpoint, emphasize it should never return null and add UT.




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