joshelser commented on a change in pull request #2118:
URL: https://github.com/apache/hbase/pull/2118#discussion_r458916606
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/HBaseInterClusterReplicationEndpoint.java
##########
@@ -127,6 +127,7 @@
private boolean dropOnDeletedTables;
private boolean dropOnDeletedColumnFamilies;
private boolean isSerial = false;
+ private long lastSinkFetchTime;
Review comment:
JVM will initialize this to zero which is of consequence to the log
message (will make sure that you get the `LOG.warn` the first time).
Explicitly initialize it here so with a comment so we know that?
##########
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?
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.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/HBaseInterClusterReplicationEndpoint.java
##########
@@ -513,8 +514,14 @@ public boolean replicate(ReplicateContext
replicateContext) {
int numSinks = replicationSinkMgr.getNumSinks();
if (numSinks == 0) {
- LOG.warn("{} No replication sinks found, returning without replicating. "
- + "The source should retry with the same set of edits.", logPeerId());
+ if((System.currentTimeMillis() - lastSinkFetchTime) >=
(maxRetriesMultiplier*1000)) {
+ LOG.warn(
+ "No replication sinks found, returning without replicating. "
+ + "The source should retry with the same set of edits. Not logging
this again for "
+ + "the next " + maxRetriesMultiplier + " seconds.");
Review comment:
nit, parameterized logging
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/HBaseInterClusterReplicationEndpoint.java
##########
@@ -513,8 +514,14 @@ public boolean replicate(ReplicateContext
replicateContext) {
int numSinks = replicationSinkMgr.getNumSinks();
if (numSinks == 0) {
- LOG.warn("{} No replication sinks found, returning without replicating. "
- + "The source should retry with the same set of edits.", logPeerId());
+ if((System.currentTimeMillis() - lastSinkFetchTime) >=
(maxRetriesMultiplier*1000)) {
+ LOG.warn(
+ "No replication sinks found, returning without replicating. "
+ + "The source should retry with the same set of edits. Not logging
this again for "
+ + "the next " + maxRetriesMultiplier + " seconds.");
+ lastSinkFetchTime = System.currentTimeMillis();
+ }
+ sleepForRetries("No sinks available at peer", sleepMultiplier);
Review comment:
Might it be helpful to include which peer (in the case that we have
multiple) here?
----------------------------------------------------------------
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]
