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/04/07 17:58:54 UTC

[GitHub] [hbase] bharathv opened a new pull request #3131: HBASE-25743: Retry REQUESTTIMEOUT based KeeperExceptions in ZK client.

bharathv opened a new pull request #3131:
URL: https://github.com/apache/hbase/pull/3131


   Starting ZOOKEEPER-2251, client requests exceeding a timeout can throw
   a KeeperException with REQUESTTIMEOUT opcode set. RecoverableZookeeper
   doesn't transparently retry in such cases.


-- 
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 #3131: HBASE-25743: Retry REQUESTTIMEOUT based KeeperExceptions in ZK client.

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


   


-- 
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 #3131: HBASE-25743: Retry REQUESTTIMEOUT based KeeperExceptions in ZK client.

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 28s |  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.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 58s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 32s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 15s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   0m 35s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 36s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 30s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 30s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 13s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  17m 59s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   0m 45s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 13s |  The patch does not generate ASF License warnings.  |
   |  |   |  36m 56s |   |
   
   
   | 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-3131/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3131 |
   | JIRA Issue | HBASE-25743 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux 3e24a8d1df7b 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / d9f4f41f76 |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | Max. process+thread count | 95 (vs. ulimit of 30000) |
   | modules | C: hbase-zookeeper U: hbase-zookeeper |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3131/1/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
   | 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] shahrs87 commented on a change in pull request #3131: HBASE-25743: Retry REQUESTTIMEOUT based KeeperExceptions in ZK client.

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



##########
File path: hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/RecoverableZooKeeper.java
##########
@@ -182,9 +182,8 @@ public void delete(String path, int version) throws InterruptedException, Keeper
               throw e;
 
             case CONNECTIONLOSS:
-              retryOrThrow(retryCounter, e, "delete");
-              break;
             case OPERATIONTIMEOUT:
+            case REQUESTTIMEOUT:

Review comment:
       can we add a test case for this change ?




-- 
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] shahrs87 commented on pull request #3131: HBASE-25743: Retry REQUESTTIMEOUT based KeeperExceptions in ZK client.

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


   >  We would have to manufacture zk behaviors in our context. Do you have a suggestion?
   
   @saintstack  Is it possible to mock RecoverableZookeeper for first time to throw REQUESTTIMEOUT exception and second time it succeeds and verify the client call succeeded ?


-- 
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 #3131: HBASE-25743: Retry REQUESTTIMEOUT based KeeperExceptions in ZK client.

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


   It looks like a lot of code refactoring is needed to subclass and test and mocking seems to be complex since there are no setters for internal class state that doesn't seem worth refactoring. Skipping the test for now as discussed. Thanks for the reviews.


-- 
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 #3131: HBASE-25743: Retry REQUESTTIMEOUT based KeeperExceptions in ZK client.

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  9s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 16s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 17s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m 20s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 23s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   9m 13s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 17s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 51s |  hbase-zookeeper in the patch passed.  |
   |  |   |  30m 37s |   |
   
   
   | 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-3131/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3131 |
   | JIRA Issue | HBASE-25743 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 7ff0cbc95091 4.15.0-128-generic #131-Ubuntu SMP Wed Dec 9 06:57:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / d9f4f41f76 |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3131/1/testReport/ |
   | Max. process+thread count | 354 (vs. ulimit of 30000) |
   | modules | C: hbase-zookeeper U: hbase-zookeeper |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3131/1/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | 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] saintstack commented on pull request #3131: HBASE-25743: Retry REQUESTTIMEOUT based KeeperExceptions in ZK client.

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


   Or subclass so could override the creation of the zk instance.... But we'd just be testing that retry 'works'? That the case catches the new enum, REQUESTTIMEOUT (which is from zk). I love tests. Just not sure it would add much in this case. Thanks @shahrs87 


-- 
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] shahrs87 commented on pull request #3131: HBASE-25743: Retry REQUESTTIMEOUT based KeeperExceptions in ZK client.

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


   @saintstack I don't have any strong opinion in this case. If you feel this patch is good, then I am +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] Apache-HBase commented on pull request #3131: HBASE-25743: Retry REQUESTTIMEOUT based KeeperExceptions in ZK client.

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 32s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 35s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m  6s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 19s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 43s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 11s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 17s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 47s |  hbase-zookeeper in the patch passed.  |
   |  |   |  29m 23s |   |
   
   
   | 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-3131/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3131 |
   | JIRA Issue | HBASE-25743 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 5e7299028ed6 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 | dev-support/hbase-personality.sh |
   | git revision | master / d9f4f41f76 |
   | Default Java | AdoptOpenJDK-11.0.10+9 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3131/1/testReport/ |
   | Max. process+thread count | 426 (vs. ulimit of 30000) |
   | modules | C: hbase-zookeeper U: hbase-zookeeper |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3131/1/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | 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