wchevreuil commented on a change in pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#discussion_r586346587
##########
File path:
hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/replication/regionserver/MetricsReplicationGlobalSourceSource.java
##########
@@ -197,6 +205,20 @@ public void incrFailedRecoveryQueue() {
failedRecoveryQueue.incr(1L);
}
+ @Override
+ public void setPeerZkConnectionFailures(boolean success) {
+ if (success) {
+ peerZkConnectionFailures.set(0L);
+ } else {
+ peerZkConnectionFailures.incr(0L);
Review comment:
Shouldn't we pass _1L_ to this _incr_ call?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -293,9 +293,12 @@ public void run() {
while (this.isSourceActive() && this.peerClusterId == null) {
this.peerClusterId = replicationEndpoint.getPeerUUID();
if (this.isSourceActive() && this.peerClusterId == null) {
+ metrics.setPeerZkConnectionFailures(false);
Review comment:
One of the motivations described in the jira is that there's no evidence
of this condition in the logs. Since log files are normally the first source of
info operators normally look after, is it possible to add a WARN reporting the
peerId info is missing, possibly due to ZK connectivity issues?
##########
File path:
hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/replication/regionserver/MetricsReplicationGlobalSourceSource.java
##########
@@ -197,6 +205,20 @@ public void incrFailedRecoveryQueue() {
failedRecoveryQueue.incr(1L);
}
+ @Override
+ public void setPeerZkConnectionFailures(boolean success) {
Review comment:
Since this is a numeric metric, we should comply with the other numeric
metrics and provide an _incrPeerZkConnectionFailures_ method, rather than a
_setter_.
----------------------------------------------------------------
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]
