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.
> So maybe we can safely log warning for if(slaveAddresses.size()==0) ?
Yeah, can just leave that.
----------------------------------------------------------------
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]
