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 07:31:35 UTC

[GitHub] [hbase] sandeepvinayak opened a new pull request #3009: HBASE-25627: HBase replication should have a metric to represent if it cannot talk to peer's zk

sandeepvinayak opened a new pull request #3009:
URL: https://github.com/apache/hbase/pull/3009


   


----------------------------------------------------------------
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



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

Posted by GitBox <gi...@apache.org>.
apurtell commented on pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#issuecomment-790087318


   I also agree with @wchevreuil that starting with branch-1 for an issue that affects all branches is upside down. We should have a master patch and merge it before merging this. Sure, a separate PR may be needed for branch-1 because of code difference, that is fine, but patch application should proceed in the normal order, which is master -> branch-2 -> releasing branch-2s -> branch-1. 


----------------------------------------------------------------
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



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

Posted by GitBox <gi...@apache.org>.
sandeepvinayak edited a comment on pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#issuecomment-791065280


   @bharathv @apurtell @wchevreuil I believe @bharathv  suggestion to have a metric at source level instead makes sense to me. That metric will eventually catch the situation of peer connection failure as well. 
   I have raised a PR for `master` branch [here](https://github.com/apache/hbase/pull/3018/files). Can you please review it? 
   
   Once that is committed, I can change this one on `branch-1`.


----------------------------------------------------------------
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



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

Posted by GitBox <gi...@apache.org>.
bharathv commented on pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#issuecomment-790090315


   @apurtell I believe our reviews overlapped. I [proposed](https://github.com/apache/hbase/pull/3009#discussion_r586815267) an alternative metric to track. It helps to track the number of uninitialized sources (stuck during initialization) that we can flag via monitoring right away (along with additional logging from back port of HBASE-22731).


----------------------------------------------------------------
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



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

Posted by GitBox <gi...@apache.org>.
sandeepvinayak commented on pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#issuecomment-791065280


   @bharathv @apurtell @wchevreuil I believe @bharathv  suggestion to have a metric at source level instead makes sense to me. That metric will eventually catch the situation of peer connection failure as well. 
   I have raised a PR for `master` branch [here](https://github.com/apache/hbase/pull/3018/files). Can you please review it? 


----------------------------------------------------------------
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



[GitHub] [hbase] Apache-HBase commented on pull request #3009: HBASE-25627: HBase replication should have a metric to represent if the source is stuck getting initialized

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#issuecomment-803270483


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  7s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ branch-1 Compile Tests _ |
   | +0 :ok: |  mvndep  |   2m 29s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   8m  7s |  branch-1 passed  |
   | +1 :green_heart: |  compile  |   1m 11s |  branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  compile  |   1m 22s |  branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +1 :green_heart: |  checkstyle  |   2m  5s |  branch-1 passed  |
   | +1 :green_heart: |  shadedjars  |   2m 59s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  9s |  branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  javadoc  |   1m 15s |  branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +0 :ok: |  spotbugs  |   2m 46s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   4m  8s |  branch-1 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m 53s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 11s |  the patch passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  javac  |   1m 11s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 18s |  the patch passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +1 :green_heart: |  javac  |   1m 18s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 14s |  The patch passed checkstyle in hbase-hadoop-compat  |
   | +1 :green_heart: |  checkstyle  |   0m 15s |  The patch passed checkstyle in hbase-hadoop2-compat  |
   | +1 :green_heart: |  checkstyle  |   1m 28s |  hbase-server: The patch generated 0 new + 15 unchanged - 2 fixed = 15 total (was 17)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedjars  |   2m 48s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  hadoopcheck  |   4m 38s |  Patch does not cause any errors with Hadoop 2.8.5 2.9.2.  |
   | +1 :green_heart: |  javadoc  |   1m  1s |  the patch passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  javadoc  |   1m 17s |  the patch passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +1 :green_heart: |  findbugs  |   4m 24s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 28s |  hbase-hadoop-compat in the patch passed.  |
   | +1 :green_heart: |  unit  |   0m 40s |  hbase-hadoop2-compat in the patch passed.  |
   | +1 :green_heart: |  unit  | 104m  0s |  hbase-server in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m 19s |  The patch does not generate ASF License warnings.  |
   |  |   | 157m 27s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/5/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3009 |
   | JIRA Issue | HBASE-25627 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux 386c8a914803 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3009/out/precommit/personality/provided.sh |
   | git revision | branch-1 / f807800 |
   | Default Java | Azul Systems, Inc.-1.7.0_272-b10 |
   | Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:Azul Systems, Inc.-1.8.0_262-b19 /usr/lib/jvm/zulu-7-amd64:Azul Systems, Inc.-1.7.0_272-b10 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/5/testReport/ |
   | Max. process+thread count | 3703 (vs. ulimit of 10000) |
   | modules | C: hbase-hadoop-compat hbase-hadoop2-compat hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/5/console |
   | versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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



[GitHub] [hbase] sandeepvinayak 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

Posted by GitBox <gi...@apache.org>.
sandeepvinayak commented on a change in pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#discussion_r586847746



##########
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:
       yeah, I have added a metric as well as a log. 




----------------------------------------------------------------
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



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

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#issuecomment-789532154


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   6m 49s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ branch-1 Compile Tests _ |
   | +0 :ok: |  mvndep  |   2m 26s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   8m  1s |  branch-1 passed  |
   | +1 :green_heart: |  compile  |   1m 13s |  branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  compile  |   1m 20s |  branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +1 :green_heart: |  checkstyle  |   2m 10s |  branch-1 passed  |
   | +1 :green_heart: |  shadedjars  |   3m  2s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m 10s |  branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  javadoc  |   1m 15s |  branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +0 :ok: |  spotbugs  |   2m 45s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   4m  7s |  branch-1 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for patch  |
   | -1 :x: |  mvninstall  |   1m 13s |  root in the patch failed.  |
   | -1 :x: |  compile  |   0m 35s |  hbase-server in the patch failed with JDK Azul Systems, Inc.-1.8.0_262-b19.  |
   | -1 :x: |  javac  |   0m 35s |  hbase-server in the patch failed with JDK Azul Systems, Inc.-1.8.0_262-b19.  |
   | -1 :x: |  compile  |   0m 42s |  hbase-server in the patch failed with JDK Azul Systems, Inc.-1.7.0_272-b10.  |
   | -1 :x: |  javac  |   0m 42s |  hbase-server in the patch failed with JDK Azul Systems, Inc.-1.7.0_272-b10.  |
   | +1 :green_heart: |  checkstyle  |   0m 13s |  The patch passed checkstyle in hbase-hadoop-compat  |
   | +1 :green_heart: |  checkstyle  |   0m 15s |  hbase-hadoop2-compat: The patch generated 0 new + 1 unchanged - 8 fixed = 1 total (was 9)  |
   | +1 :green_heart: |  checkstyle  |   1m 27s |  hbase-server: The patch generated 0 new + 1 unchanged - 1 fixed = 1 total (was 2)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | -1 :x: |  shadedjars  |   1m 54s |  patch has 14 errors when building our shaded downstream artifacts.  |
   | -1 :x: |  hadoopcheck  |   1m 12s |  The patch causes 14 errors with Hadoop v2.8.5.  |
   | -1 :x: |  hadoopcheck  |   3m  7s |  The patch causes 14 errors with Hadoop v2.9.2.  |
   | +1 :green_heart: |  javadoc  |   0m 59s |  the patch passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  javadoc  |   1m 13s |  the patch passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | -1 :x: |  findbugs  |   0m 46s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 28s |  hbase-hadoop-compat in the patch passed.  |
   | +1 :green_heart: |  unit  |   0m 40s |  hbase-hadoop2-compat in the patch passed.  |
   | -1 :x: |  unit  |   0m 46s |  hbase-server in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m 37s |  The patch does not generate ASF License warnings.  |
   |  |   |  51m 33s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/1/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3009 |
   | JIRA Issue | HBASE-25627 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux 4e33ab635cbe 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3009/out/precommit/personality/provided.sh |
   | git revision | branch-1 / 4cfbf19 |
   | Default Java | Azul Systems, Inc.-1.7.0_272-b10 |
   | Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:Azul Systems, Inc.-1.8.0_262-b19 /usr/lib/jvm/zulu-7-amd64:Azul Systems, Inc.-1.7.0_272-b10 |
   | mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/1/artifact/out/patch-mvninstall-root.txt |
   | compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/1/artifact/out/patch-compile-hbase-server-jdkAzulSystems,Inc.-1.8.0_262-b19.txt |
   | javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/1/artifact/out/patch-compile-hbase-server-jdkAzulSystems,Inc.-1.8.0_262-b19.txt |
   | compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/1/artifact/out/patch-compile-hbase-server-jdkAzulSystems,Inc.-1.7.0_272-b10.txt |
   | javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/1/artifact/out/patch-compile-hbase-server-jdkAzulSystems,Inc.-1.7.0_272-b10.txt |
   | shadedjars | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/1/artifact/out/patch-shadedjars.txt |
   | hadoopcheck | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/1/artifact/out/patch-javac-2.8.5.txt |
   | hadoopcheck | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/1/artifact/out/patch-javac-2.9.2.txt |
   | findbugs | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/1/artifact/out/patch-findbugs-hbase-server.txt |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/1/artifact/out/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/1/testReport/ |
   | Max. process+thread count | 181 (vs. ulimit of 10000) |
   | modules | C: hbase-hadoop-compat hbase-hadoop2-compat hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/1/console |
   | versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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



[GitHub] [hbase] sandeepvinayak commented on pull request #3009: HBASE-25627: [Backport]HBase replication should have a metric to represent if the source is stuck getting initialized

Posted by GitBox <gi...@apache.org>.
sandeepvinayak commented on pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#issuecomment-803646356


   fyi @bharathv 


-- 
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



[GitHub] [hbase] sandeepvinayak 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

Posted by GitBox <gi...@apache.org>.
sandeepvinayak commented on a change in pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#discussion_r586729721



##########
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:
       Yes, I agree that WARN should be there as well but the metric helps to monitor and alert in this case. Since connection to peer's ZK blocks the whole replication, it should be good to monitor this as a metric. 




----------------------------------------------------------------
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



[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

Posted by GitBox <gi...@apache.org>.
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



[GitHub] [hbase] Apache-HBase commented on pull request #3009: HBASE-25627: HBase replication should have a metric to represent if the source is stuck getting initialized

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#issuecomment-803240325


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 12s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ branch-1 Compile Tests _ |
   | +0 :ok: |  mvndep  |   2m 27s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   8m  0s |  branch-1 passed  |
   | +1 :green_heart: |  compile  |   1m  9s |  branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  compile  |   1m 21s |  branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +1 :green_heart: |  checkstyle  |   2m  6s |  branch-1 passed  |
   | +1 :green_heart: |  shadedjars  |   3m  2s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  9s |  branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  javadoc  |   1m 16s |  branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +0 :ok: |  spotbugs  |   2m 41s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   4m  5s |  branch-1 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 18s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m 55s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 11s |  the patch passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  javac  |   1m 11s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 18s |  the patch passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +1 :green_heart: |  javac  |   1m 18s |  the patch passed  |
   | -1 :x: |  checkstyle  |   1m 31s |  hbase-server: The patch generated 1 new + 15 unchanged - 2 fixed = 16 total (was 17)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedjars  |   2m 47s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  hadoopcheck  |   4m 34s |  Patch does not cause any errors with Hadoop 2.8.5 2.9.2.  |
   | +1 :green_heart: |  javadoc  |   1m  0s |  the patch passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  javadoc  |   1m 16s |  the patch passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +1 :green_heart: |  findbugs  |   4m 24s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 29s |  hbase-hadoop-compat in the patch passed.  |
   | +1 :green_heart: |  unit  |   0m 40s |  hbase-hadoop2-compat in the patch passed.  |
   | -1 :x: |  unit  | 100m 13s |  hbase-server in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   1m 14s |  The patch does not generate ASF License warnings.  |
   |  |   | 153m 29s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | hadoop.hbase.TestCachedClusterId |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/4/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3009 |
   | JIRA Issue | HBASE-25627 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux b439bb2a9230 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3009/out/precommit/personality/provided.sh |
   | git revision | branch-1 / bea87b3 |
   | Default Java | Azul Systems, Inc.-1.7.0_272-b10 |
   | Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:Azul Systems, Inc.-1.8.0_262-b19 /usr/lib/jvm/zulu-7-amd64:Azul Systems, Inc.-1.7.0_272-b10 |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/4/artifact/out/diff-checkstyle-hbase-server.txt |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/4/artifact/out/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/4/testReport/ |
   | Max. process+thread count | 3533 (vs. ulimit of 10000) |
   | modules | C: hbase-hadoop-compat hbase-hadoop2-compat hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/4/console |
   | versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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



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

Posted by GitBox <gi...@apache.org>.
bharathv commented on pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#issuecomment-801215108


   @sandeepvinayak Mind refreshing this PR with the latest patch? Thanks.


----------------------------------------------------------------
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



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

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#issuecomment-790205731


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |  11m 49s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ branch-1 Compile Tests _ |
   | +0 :ok: |  mvndep  |   2m 27s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   8m  7s |  branch-1 passed  |
   | +1 :green_heart: |  compile  |   1m  8s |  branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  compile  |   1m 18s |  branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +1 :green_heart: |  checkstyle  |   2m  7s |  branch-1 passed  |
   | +1 :green_heart: |  shadedjars  |   3m  0s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  8s |  branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  javadoc  |   1m 17s |  branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +0 :ok: |  spotbugs  |   2m 43s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   4m  7s |  branch-1 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m 58s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 11s |  the patch passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  javac  |   1m 11s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 20s |  the patch passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +1 :green_heart: |  javac  |   1m 20s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 14s |  The patch passed checkstyle in hbase-hadoop-compat  |
   | +1 :green_heart: |  checkstyle  |   0m 16s |  hbase-hadoop2-compat: The patch generated 0 new + 1 unchanged - 8 fixed = 1 total (was 9)  |
   | +1 :green_heart: |  checkstyle  |   1m 30s |  hbase-server: The patch generated 0 new + 1 unchanged - 1 fixed = 1 total (was 2)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedjars  |   2m 50s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  hadoopcheck  |   4m 39s |  Patch does not cause any errors with Hadoop 2.8.5 2.9.2.  |
   | +1 :green_heart: |  javadoc  |   1m  0s |  the patch passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  javadoc  |   1m 16s |  the patch passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +1 :green_heart: |  findbugs  |   4m 23s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 28s |  hbase-hadoop-compat in the patch passed.  |
   | +1 :green_heart: |  unit  |   0m 40s |  hbase-hadoop2-compat in the patch passed.  |
   | +1 :green_heart: |  unit  | 120m  9s |  hbase-server in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m 19s |  The patch does not generate ASF License warnings.  |
   |  |   | 184m 10s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/3/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3009 |
   | JIRA Issue | HBASE-25627 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux efad72cacb2b 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3009/out/precommit/personality/provided.sh |
   | git revision | branch-1 / 4cfbf19 |
   | Default Java | Azul Systems, Inc.-1.7.0_272-b10 |
   | Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:Azul Systems, Inc.-1.8.0_262-b19 /usr/lib/jvm/zulu-7-amd64:Azul Systems, Inc.-1.7.0_272-b10 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/3/testReport/ |
   | Max. process+thread count | 4208 (vs. ulimit of 10000) |
   | modules | C: hbase-hadoop-compat hbase-hadoop2-compat hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/3/console |
   | versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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



[GitHub] [hbase] apurtell 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

Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#discussion_r586818343



##########
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:
       Yes, add a separate method for set. Set and Increment is fine. Set(0) to reset. 

##########
File path: hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/replication/regionserver/MetricsReplicationSourceSource.java
##########
@@ -78,4 +77,6 @@
   void incrFailedRecoveryQueue();
   void setOldestWalAge(long age);
   long getOldestWalAge();
+  void setPeerZkConnectionFailures(boolean success);
+  long getPeerZkConnectionFailures();

Review comment:
       Getters are not needed by convention here, remove

##########
File path: hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/replication/regionserver/MetricsReplicationSourceSource.java
##########
@@ -78,4 +77,6 @@
   void incrFailedRecoveryQueue();
   void setOldestWalAge(long age);
   long getOldestWalAge();
+  void setPeerZkConnectionFailures(boolean success);

Review comment:
       This is weird. It should be an increment function. See above incrFailedRecoveryQueue as example. 

##########
File path: hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/replication/regionserver/MetricsReplicationSourceSource.java
##########
@@ -50,8 +50,7 @@
   public static final String SOURCE_COMPLETED_LOGS = "source.completedLogs";
   public static final String SOURCE_COMPLETED_RECOVERY_QUEUES = "source.completedRecoverQueues";
   public static final String SOURCE_FAILED_RECOVERY_QUEUES = "source.failedRecoverQueues";
-  /* Used to track the age of oldest wal in ms since its creation time */
-  String OLDEST_WAL_AGE = "source.oldestWalAge";
+  public static final String SOURCE_PEER_ZK_CONNECTION_FAILURE = "source.peerZkConnectionFailure";

Review comment:
       Can we just call this peerConnectionFailure? ZK may or may not be the reason, if not now, then in the future. What we want to count is connection failures, let the naming reflect that. (We need to care about metric names because it becomes part of operational compat.)

##########
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:
       Agree with @wchevreuil , metrics and logs are consumed differently by operators and a reasonable request to add a log is not solved by emitting a metric




----------------------------------------------------------------
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



[GitHub] [hbase] bharathv merged pull request #3009: HBASE-25627: [Backport]HBase replication should have a metric to represent if the source is stuck getting initialized

Posted by GitBox <gi...@apache.org>.
bharathv merged pull request #3009:
URL: https://github.com/apache/hbase/pull/3009


   


-- 
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



[GitHub] [hbase] sandeepvinayak 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

Posted by GitBox <gi...@apache.org>.
sandeepvinayak commented on a change in pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#discussion_r586701468



##########
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:
       @wchevreuil This was intentional since we want to reset it to zero once we get the success connection. 
   Do you suggest having a separate method for a reset? 




----------------------------------------------------------------
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



[GitHub] [hbase] bharathv 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

Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#discussion_r586812579



##########
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:
       This logic in a global source doesn't give the intended result here because `peerZkConnectionFailures` is reset to 0 if _any_ source succeeds. That doesn't seem right.

##########
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:
       Had a brief discussion offline with @sandeepvinayak on how to generalize this. I think the intent here is capture and flag any sources that are stuck during initialization and ZK connection failure is just a symptom. So capturing those ZK failure count may not add much value, instead we track _number of uninitialized sources_, (gauge) that'd be much more helpful.
   
   So one way forward is to track number of such uninitialized sources at a global scope (that the monitoring tooling can flag if its > 0 for a time window) and then backport https://issues.apache.org/jira/browse/HBASE-22731 to branch-1. These two together should help us narrow down it to the right RS and root cause.
   
   Thoughts?




----------------------------------------------------------------
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



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

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#issuecomment-790109273


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |  12m 29s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ branch-1 Compile Tests _ |
   | +0 :ok: |  mvndep  |   2m 25s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   8m  4s |  branch-1 passed  |
   | +1 :green_heart: |  compile  |   1m  8s |  branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  compile  |   1m 20s |  branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +1 :green_heart: |  checkstyle  |   2m 16s |  branch-1 passed  |
   | +1 :green_heart: |  shadedjars  |   3m 16s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  5s |  branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  javadoc  |   1m 14s |  branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +0 :ok: |  spotbugs  |   2m 54s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   4m 17s |  branch-1 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   2m  4s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  8s |  the patch passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  javac  |   1m  8s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 20s |  the patch passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +1 :green_heart: |  javac  |   1m 20s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 13s |  The patch passed checkstyle in hbase-hadoop-compat  |
   | +1 :green_heart: |  checkstyle  |   0m 14s |  hbase-hadoop2-compat: The patch generated 0 new + 1 unchanged - 8 fixed = 1 total (was 9)  |
   | +1 :green_heart: |  checkstyle  |   1m 44s |  hbase-server: The patch generated 0 new + 1 unchanged - 1 fixed = 1 total (was 2)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedjars  |   3m  5s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  hadoopcheck  |   4m 50s |  Patch does not cause any errors with Hadoop 2.8.5 2.9.2.  |
   | +1 :green_heart: |  javadoc  |   0m 57s |  the patch passed with JDK Azul Systems, Inc.-1.8.0_262-b19  |
   | +1 :green_heart: |  javadoc  |   1m 15s |  the patch passed with JDK Azul Systems, Inc.-1.7.0_272-b10  |
   | +1 :green_heart: |  findbugs  |   4m 40s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 27s |  hbase-hadoop-compat in the patch passed.  |
   | +1 :green_heart: |  unit  |   0m 37s |  hbase-hadoop2-compat in the patch passed.  |
   | +1 :green_heart: |  unit  | 118m 40s |  hbase-server in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 59s |  The patch does not generate ASF License warnings.  |
   |  |   | 184m 21s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/2/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3009 |
   | JIRA Issue | HBASE-25627 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux 988d48e1a065 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3009/out/precommit/personality/provided.sh |
   | git revision | branch-1 / 4cfbf19 |
   | Default Java | Azul Systems, Inc.-1.7.0_272-b10 |
   | Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:Azul Systems, Inc.-1.8.0_262-b19 /usr/lib/jvm/zulu-7-amd64:Azul Systems, Inc.-1.7.0_272-b10 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/2/testReport/ |
   | Max. process+thread count | 3694 (vs. ulimit of 10000) |
   | modules | C: hbase-hadoop-compat hbase-hadoop2-compat hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3009/2/console |
   | versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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



[GitHub] [hbase] apurtell 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

Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #3009:
URL: https://github.com/apache/hbase/pull/3009#discussion_r586818343



##########
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:
       Yes, add a separate method for set, but set methods on counter metrics accept the integer value as the new value for the metric, not a weird boolean. Set and Increment is fine. Set(0) to reset. 




----------------------------------------------------------------
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