You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2021/03/03 11:45:36 UTC

[GitHub] [hbase] wchevreuil commented on a change in pull request #3009: HBASE-25627: HBase replication should have a metric to represent if it cannot talk to peer's zk

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:
users@infra.apache.org