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.
So we can leave `getRegionServers` at HBaseReplicationEndpoint, emphasize it
should never return null. I'm not sure a UT for this would be effective,
though. Custom implementations of HBaseReplicationEndpoint returning null would
still blow it at runtime.
> 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]
