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) ?
I guess we can leave it without the extra logging here, as this would
already cause the extra logging we are suppressing in
HBaseInterClusterReplicationEndpoint #518.
----------------------------------------------------------------
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
