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 2020/08/27 20:07:15 UTC
[GitHub] [hbase] shahrs87 opened a new pull request #2323: [HBASE-24957] ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not.
shahrs87 opened a new pull request #2323:
URL: https://github.com/apache/hbase/pull/2323
----------------------------------------------------------------
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 #2323: [HBASE-24957] ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not.
Posted by GitBox <gi...@apache.org>.
shahrs87 commented on pull request #2323:
URL: https://github.com/apache/hbase/pull/2323#issuecomment-683846784
> +1, please take care of checkstyle warnings, it's related to import order.
The checkstyle errors in hbase-client were due to my changes, fixed that but the errors in hbase-server are not due to my changes so kept as is.
Thank you @virajjasani for the review.
----------------------------------------------------------------
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 #2323: [HBASE-24957] ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not.
Posted by GitBox <gi...@apache.org>.
shahrs87 commented on a change in pull request #2323:
URL: https://github.com/apache/hbase/pull/2323#discussion_r480226831
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java
##########
@@ -1492,4 +1487,16 @@ public void testMergeRegions() throws Exception {
this.admin.deleteTable(tableName);
}
}
+
+ @Test (timeout=30000)
+ public void testTableNotFoundException() throws Exception {
+ ZooKeeperWatcher zkw = HBaseTestingUtility.getZooKeeperWatcher(TEST_UTIL);
Review comment:
Thank you for the feedback. Addressed in latest commit. Please review again.
----------------------------------------------------------------
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] virajjasani closed pull request #2323: [HBASE-24957] ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not.
Posted by GitBox <gi...@apache.org>.
virajjasani closed pull request #2323:
URL: https://github.com/apache/hbase/pull/2323
----------------------------------------------------------------
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 #2323: [HBASE-24957] ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not.
Posted by GitBox <gi...@apache.org>.
shahrs87 commented on a change in pull request #2323:
URL: https://github.com/apache/hbase/pull/2323#discussion_r479815790
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
##########
@@ -4222,7 +4222,14 @@ public String explainFailure() throws IOException {
@Override
public boolean evaluate() throws IOException {
- return getHBaseAdmin().tableExists(tableName) && getHBaseAdmin().isTableEnabled(tableName);
+ boolean tableEnabled = false;
+ try {
+ tableEnabled = getHBaseAdmin().tableExists(tableName)
+ && getHBaseAdmin().isTableEnabled(tableName);
+ } catch (TableNotFoundException tnfe) {
+ // Ignore TNFE
+ }
+ return tableEnabled;
Review comment:
Thank you for the feedback. Done in latest commit. Please review again.
----------------------------------------------------------------
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 #2323: [HBASE-24957] ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not.
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #2323:
URL: https://github.com/apache/hbase/pull/2323#discussion_r480220638
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java
##########
@@ -1492,4 +1487,16 @@ public void testMergeRegions() throws Exception {
this.admin.deleteTable(tableName);
}
}
+
+ @Test (timeout=30000)
+ public void testTableNotFoundException() throws Exception {
+ ZooKeeperWatcher zkw = HBaseTestingUtility.getZooKeeperWatcher(TEST_UTIL);
Review comment:
connection leak, needs to be closed.
Instead use the non-static version TEST_UTIL.getZookeeperWatcher()... that is automatically closed with HTU teardown.
----------------------------------------------------------------
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 #2323: [HBASE-24957] ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not.
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2323:
URL: https://github.com/apache/hbase/pull/2323#issuecomment-683968431
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 5s | 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 3 new or modified test files. |
||| _ branch-1 Compile Tests _ |
| +0 :ok: | mvndep | 2m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 8m 3s | branch-1 passed |
| +1 :green_heart: | compile | 0m 59s | branch-1 passed with JDK v1.8.0_262 |
| +1 :green_heart: | compile | 1m 11s | branch-1 passed with JDK v1.7.0_272 |
| +1 :green_heart: | checkstyle | 2m 35s | branch-1 passed |
| +1 :green_heart: | shadedjars | 3m 18s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 59s | branch-1 passed with JDK v1.8.0_262 |
| +1 :green_heart: | javadoc | 1m 7s | branch-1 passed with JDK v1.7.0_272 |
| +0 :ok: | spotbugs | 2m 57s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 37s | branch-1 passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 12s | the patch passed |
| +1 :green_heart: | compile | 1m 3s | the patch passed with JDK v1.8.0_262 |
| +1 :green_heart: | javac | 1m 3s | the patch passed |
| +1 :green_heart: | compile | 1m 10s | the patch passed with JDK v1.7.0_272 |
| +1 :green_heart: | javac | 1m 10s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 36s | hbase-client: The patch generated 0 new + 18 unchanged - 1 fixed = 18 total (was 19) |
| -1 :x: | checkstyle | 1m 55s | hbase-server: The patch generated 2 new + 374 unchanged - 7 fixed = 376 total (was 381) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 3m 14s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 5m 15s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2. |
| +1 :green_heart: | javadoc | 0m 53s | the patch passed with JDK v1.8.0_262 |
| +1 :green_heart: | javadoc | 1m 7s | the patch passed with JDK v1.7.0_272 |
| +1 :green_heart: | findbugs | 4m 58s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 57s | hbase-client in the patch passed. |
| -1 :x: | unit | 163m 31s | hbase-server in the patch failed. |
| +1 :green_heart: | asflicense | 0m 47s | The patch does not generate ASF License warnings. |
| | | 220m 13s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hbase.master.TestMasterMetrics |
| | hadoop.hbase.mapreduce.TestSecureLoadIncrementalHFiles |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2323 |
| JIRA Issue | HBASE-24957 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux a175fc8a599a 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-agent/workspace/Base-PreCommit-GitHub-PR_PR-2323/out/precommit/personality/provided.sh |
| git revision | branch-1 / 041debd |
| Default Java | 1.7.0_272 |
| Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:1.8.0_262 /usr/lib/jvm/zulu-7-amd64:1.7.0_272 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/4/artifact/out/diff-checkstyle-hbase-server.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/4/artifact/out/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/4/testReport/ |
| Max. process+thread count | 3935 (vs. ulimit of 10000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/4/console |
| versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
| Powered by | Apache Yetus 0.11.1 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] Apache-HBase commented on pull request #2323: [HBASE-24957] ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not.
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2323:
URL: https://github.com/apache/hbase/pull/2323#issuecomment-683489174
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | 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 3 new or modified test files. |
||| _ branch-1 Compile Tests _ |
| +0 :ok: | mvndep | 2m 26s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 7m 57s | branch-1 passed |
| +1 :green_heart: | compile | 0m 59s | branch-1 passed with JDK v1.8.0_262 |
| +1 :green_heart: | compile | 1m 9s | branch-1 passed with JDK v1.7.0_272 |
| +1 :green_heart: | checkstyle | 2m 24s | branch-1 passed |
| +1 :green_heart: | shadedjars | 3m 0s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 3s | branch-1 passed with JDK v1.8.0_262 |
| +1 :green_heart: | javadoc | 1m 6s | branch-1 passed with JDK v1.7.0_272 |
| +0 :ok: | spotbugs | 2m 38s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 13s | branch-1 passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 56s | the patch passed |
| +1 :green_heart: | compile | 1m 1s | the patch passed with JDK v1.8.0_262 |
| +1 :green_heart: | javac | 1m 1s | the patch passed |
| +1 :green_heart: | compile | 1m 8s | the patch passed with JDK v1.7.0_272 |
| +1 :green_heart: | javac | 1m 8s | the patch passed |
| -1 :x: | checkstyle | 0m 36s | hbase-client: The patch generated 1 new + 18 unchanged - 1 fixed = 19 total (was 19) |
| -1 :x: | checkstyle | 1m 39s | hbase-server: The patch generated 2 new + 374 unchanged - 7 fixed = 376 total (was 381) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 2m 46s | 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 | 0m 52s | the patch passed with JDK v1.8.0_262 |
| +1 :green_heart: | javadoc | 1m 5s | the patch passed with JDK v1.7.0_272 |
| +1 :green_heart: | findbugs | 4m 22s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 42s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 122m 51s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 57s | The patch does not generate ASF License warnings. |
| | | 175m 39s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2323 |
| JIRA Issue | HBASE-24957 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 938cddacfdba 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-agent/workspace/Base-PreCommit-GitHub-PR_PR-2323/out/precommit/personality/provided.sh |
| git revision | branch-1 / 041debd |
| Default Java | 1.7.0_272 |
| Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:1.8.0_262 /usr/lib/jvm/zulu-7-amd64:1.7.0_272 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/3/artifact/out/diff-checkstyle-hbase-client.txt |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/3/artifact/out/diff-checkstyle-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/3/testReport/ |
| Max. process+thread count | 4137 (vs. ulimit of 10000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/3/console |
| versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
| Powered by | Apache Yetus 0.11.1 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] Apache-HBase commented on pull request #2323: [HBASE-24957] ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not.
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2323:
URL: https://github.com/apache/hbase/pull/2323#issuecomment-684073809
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 38s | 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 3 new or modified test files. |
||| _ branch-1 Compile Tests _ |
| +0 :ok: | mvndep | 2m 31s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 8m 40s | branch-1 passed |
| +1 :green_heart: | compile | 1m 11s | branch-1 passed with JDK v1.8.0_262 |
| +1 :green_heart: | compile | 1m 21s | branch-1 passed with JDK v1.7.0_272 |
| +1 :green_heart: | checkstyle | 3m 5s | branch-1 passed |
| +1 :green_heart: | shadedjars | 3m 40s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 7s | branch-1 passed with JDK v1.8.0_262 |
| +1 :green_heart: | javadoc | 1m 12s | branch-1 passed with JDK v1.7.0_272 |
| +0 :ok: | spotbugs | 3m 21s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 5m 12s | branch-1 passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 18s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 29s | the patch passed |
| +1 :green_heart: | compile | 1m 23s | the patch passed with JDK v1.8.0_262 |
| +1 :green_heart: | javac | 1m 23s | the patch passed |
| +1 :green_heart: | compile | 1m 27s | the patch passed with JDK v1.7.0_272 |
| +1 :green_heart: | javac | 1m 27s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 41s | hbase-client: The patch generated 0 new + 18 unchanged - 1 fixed = 18 total (was 19) |
| -1 :x: | checkstyle | 2m 2s | hbase-server: The patch generated 2 new + 374 unchanged - 7 fixed = 376 total (was 381) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 3m 32s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 5m 14s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2. |
| +1 :green_heart: | javadoc | 0m 50s | the patch passed with JDK v1.8.0_262 |
| +1 :green_heart: | javadoc | 1m 8s | the patch passed with JDK v1.7.0_272 |
| +1 :green_heart: | findbugs | 4m 45s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 42s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 141m 19s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 45s | The patch does not generate ASF License warnings. |
| | | 202m 21s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2323 |
| JIRA Issue | HBASE-24957 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux e1fe9af057a7 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-agent/workspace/Base-PreCommit-GitHub-PR_PR-2323/out/precommit/personality/provided.sh |
| git revision | branch-1 / 041debd |
| Default Java | 1.7.0_272 |
| Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:1.8.0_262 /usr/lib/jvm/zulu-7-amd64:1.7.0_272 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/5/artifact/out/diff-checkstyle-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/5/testReport/ |
| Max. process+thread count | 3830 (vs. ulimit of 10000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/5/console |
| versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
| Powered by | Apache Yetus 0.11.1 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] virajjasani commented on a change in pull request #2323: [HBASE-24957] ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not.
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #2323:
URL: https://github.com/apache/hbase/pull/2323#discussion_r479732610
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKTableStateClientSideReader.java
##########
@@ -175,15 +176,18 @@ static boolean isTableState(final ZooKeeperProtos.Table.State expectedState,
/**
* @param zkw ZooKeeperWatcher instance to use
* @param tableName table we're checking
- * @return Null or {@link ZooKeeperProtos.Table.State} found in znode.
+ * @return {@link ZooKeeperProtos.Table.State} found in znode.
* @throws KeeperException
+ * @throws TableNotFoundException if tableName doesn't exist
*/
static ZooKeeperProtos.Table.State getTableState(final ZooKeeperWatcher zkw,
final TableName tableName)
- throws KeeperException, InterruptedException {
+ throws KeeperException, InterruptedException, TableNotFoundException {
String znode = ZKUtil.joinZNode(zkw.tableZNode, tableName.getNameAsString());
byte [] data = ZKUtil.getData(zkw, znode);
- if (data == null || data.length <= 0) return null;
+ if (data == null || data.length <= 0) {
+ throw new TableNotFoundException(tableName);
Review comment:
I think this is good move but since so many methods of this class have started throwing TNFE, all callers of all these methods have handled change accordingly? Like HBaseFsck etc?
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
##########
@@ -4222,7 +4222,14 @@ public String explainFailure() throws IOException {
@Override
public boolean evaluate() throws IOException {
- return getHBaseAdmin().tableExists(tableName) && getHBaseAdmin().isTableEnabled(tableName);
+ boolean tableEnabled = false;
+ try {
+ tableEnabled = getHBaseAdmin().tableExists(tableName)
+ && getHBaseAdmin().isTableEnabled(tableName);
+ } catch (TableNotFoundException tnfe) {
+ // Ignore TNFE
+ }
+ return tableEnabled;
Review comment:
This could be
```
public boolean evaluate() throws IOException {
try {
return getHBaseAdmin().tableExists(tableName)
&& getHBaseAdmin().isTableEnabled(tableName);
} catch (TableNotFoundException tnfe) {
return false;
}
}
```
----------------------------------------------------------------
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 #2323: [HBASE-24957] ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not.
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2323:
URL: https://github.com/apache/hbase/pull/2323#issuecomment-683383581
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 6m 43s | 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 3 new or modified test files. |
||| _ branch-1 Compile Tests _ |
| +0 :ok: | mvndep | 2m 24s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 7m 56s | branch-1 passed |
| +1 :green_heart: | compile | 0m 59s | branch-1 passed with JDK v1.8.0_262 |
| +1 :green_heart: | compile | 1m 8s | branch-1 passed with JDK v1.7.0_272 |
| +1 :green_heart: | checkstyle | 2m 23s | branch-1 passed |
| +1 :green_heart: | shadedjars | 3m 2s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 2s | branch-1 passed with JDK v1.8.0_262 |
| +1 :green_heart: | javadoc | 1m 8s | branch-1 passed with JDK v1.7.0_272 |
| +0 :ok: | spotbugs | 2m 39s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 13s | 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 | 0m 59s | the patch passed with JDK v1.8.0_262 |
| +1 :green_heart: | javac | 0m 59s | the patch passed |
| +1 :green_heart: | compile | 1m 7s | the patch passed with JDK v1.7.0_272 |
| +1 :green_heart: | javac | 1m 7s | the patch passed |
| -1 :x: | checkstyle | 0m 35s | hbase-client: The patch generated 1 new + 18 unchanged - 1 fixed = 19 total (was 19) |
| -1 :x: | checkstyle | 1m 40s | hbase-server: The patch generated 2 new + 374 unchanged - 7 fixed = 376 total (was 381) |
| +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 35s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2. |
| +1 :green_heart: | javadoc | 0m 52s | the patch passed with JDK v1.8.0_262 |
| +1 :green_heart: | javadoc | 1m 7s | the patch passed with JDK v1.7.0_272 |
| +1 :green_heart: | findbugs | 4m 21s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 42s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 122m 18s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 58s | The patch does not generate ASF License warnings. |
| | | 181m 18s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2323 |
| JIRA Issue | HBASE-24957 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 7eba5eed2429 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-agent/workspace/Base-PreCommit-GitHub-PR_PR-2323/out/precommit/personality/provided.sh |
| git revision | branch-1 / 31e47af |
| Default Java | 1.7.0_272 |
| Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:1.8.0_262 /usr/lib/jvm/zulu-7-amd64:1.7.0_272 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/2/artifact/out/diff-checkstyle-hbase-client.txt |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/2/artifact/out/diff-checkstyle-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/2/testReport/ |
| Max. process+thread count | 4138 (vs. ulimit of 10000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/2/console |
| versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
| Powered by | Apache Yetus 0.11.1 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 pull request #2323: [HBASE-24957] ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not.
Posted by GitBox <gi...@apache.org>.
shahrs87 commented on pull request #2323:
URL: https://github.com/apache/hbase/pull/2323#issuecomment-683840914
@virajjasani Could you please take a final look ? Thank you !
----------------------------------------------------------------
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 #2323: [HBASE-24957] ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not.
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2323:
URL: https://github.com/apache/hbase/pull/2323#issuecomment-682259838
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 15m 47s | 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 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 7m 57s | branch-1 passed |
| +1 :green_heart: | compile | 1m 1s | branch-1 passed with JDK v1.8.0_262 |
| +1 :green_heart: | compile | 1m 9s | branch-1 passed with JDK v1.7.0_272 |
| +1 :green_heart: | checkstyle | 2m 14s | branch-1 passed |
| +1 :green_heart: | shadedjars | 3m 1s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 2s | branch-1 passed with JDK v1.8.0_262 |
| +1 :green_heart: | javadoc | 1m 9s | branch-1 passed with JDK v1.7.0_272 |
| +0 :ok: | spotbugs | 2m 38s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 14s | branch-1 passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 56s | the patch passed |
| +1 :green_heart: | compile | 1m 1s | the patch passed with JDK v1.8.0_262 |
| +1 :green_heart: | javac | 1m 1s | the patch passed |
| +1 :green_heart: | compile | 1m 9s | the patch passed with JDK v1.7.0_272 |
| +1 :green_heart: | javac | 1m 9s | the patch passed |
| -1 :x: | checkstyle | 0m 35s | hbase-client: The patch generated 1 new + 18 unchanged - 1 fixed = 19 total (was 19) |
| -1 :x: | checkstyle | 1m 32s | hbase-server: The patch generated 2 new + 43 unchanged - 7 fixed = 45 total (was 50) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 2m 49s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 4m 42s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2. |
| +1 :green_heart: | javadoc | 0m 51s | the patch passed with JDK v1.8.0_262 |
| +1 :green_heart: | javadoc | 1m 7s | the patch passed with JDK v1.7.0_272 |
| +1 :green_heart: | findbugs | 4m 23s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 44s | hbase-client in the patch passed. |
| -1 :x: | unit | 149m 20s | hbase-server in the patch failed. |
| +1 :green_heart: | asflicense | 0m 57s | The patch does not generate ASF License warnings. |
| | | 217m 9s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hbase.security.access.TestTablePermissions |
| | hadoop.hbase.security.access.TestCellACLs |
| | hadoop.hbase.security.visibility.TestVisibilityLabelReplicationWithExpAsString |
| | hadoop.hbase.security.visibility.TestVisibilityLabelsWithACL |
| | hadoop.hbase.security.visibility.TestEnforcingScanLabelGenerator |
| | hadoop.hbase.security.visibility.TestVisibilityLabelsWithCustomVisLabService |
| | hadoop.hbase.security.visibility.TestDefaultScanLabelGeneratorStack |
| | hadoop.hbase.security.visibility.TestVisibilityLablesWithGroups |
| | hadoop.hbase.security.access.TestAccessControlFilter |
| | hadoop.hbase.security.visibility.TestVisibilityLabelsWithDeletes |
| | hadoop.hbase.mapreduce.TestSecureLoadIncrementalHFilesSplitRecovery |
| | hadoop.hbase.security.visibility.TestVisibilityLabelsReplication |
| | hadoop.hbase.mapreduce.TestImportTSVWithVisibilityLabels |
| | hadoop.hbase.security.visibility.TestVisibilityLabelsWithDefaultVisLabelService |
| | hadoop.hbase.security.visibility.TestVisibilityLabelsWithSLGStack |
| | hadoop.hbase.security.access.TestScanEarlyTermination |
| | hadoop.hbase.security.access.TestCellACLWithMultipleVersions |
| | hadoop.hbase.security.visibility.TestVisibilityLabelsOpWithDifferentUsersNoACL |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2323 |
| JIRA Issue | HBASE-24957 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 45627959cabe 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2323/out/precommit/personality/provided.sh |
| git revision | branch-1 / 8f946be |
| Default Java | 1.7.0_272 |
| Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:1.8.0_262 /usr/lib/jvm/zulu-7-amd64:1.7.0_272 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/1/artifact/out/diff-checkstyle-hbase-client.txt |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/1/artifact/out/diff-checkstyle-hbase-server.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/1/artifact/out/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/1/testReport/ |
| Max. process+thread count | 4649 (vs. ulimit of 10000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2323/1/console |
| versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
| Powered by | Apache Yetus 0.11.1 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 #2323: [HBASE-24957] ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not.
Posted by GitBox <gi...@apache.org>.
shahrs87 commented on a change in pull request #2323:
URL: https://github.com/apache/hbase/pull/2323#discussion_r479813867
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKTableStateClientSideReader.java
##########
@@ -175,15 +176,18 @@ static boolean isTableState(final ZooKeeperProtos.Table.State expectedState,
/**
* @param zkw ZooKeeperWatcher instance to use
* @param tableName table we're checking
- * @return Null or {@link ZooKeeperProtos.Table.State} found in znode.
+ * @return {@link ZooKeeperProtos.Table.State} found in znode.
* @throws KeeperException
+ * @throws TableNotFoundException if tableName doesn't exist
*/
static ZooKeeperProtos.Table.State getTableState(final ZooKeeperWatcher zkw,
final TableName tableName)
- throws KeeperException, InterruptedException {
+ throws KeeperException, InterruptedException, TableNotFoundException {
String znode = ZKUtil.joinZNode(zkw.tableZNode, tableName.getNameAsString());
byte [] data = ZKUtil.getData(zkw, znode);
- if (data == null || data.length <= 0) return null;
+ if (data == null || data.length <= 0) {
+ throw new TableNotFoundException(tableName);
Review comment:
> all callers of all these methods have handled change accordingly?
All the callers throws IOException and TNFE is a sub class of IOException. So no need to handle it explicitly.
----------------------------------------------------------------
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] virajjasani commented on pull request #2323: [HBASE-24957] ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not.
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #2323:
URL: https://github.com/apache/hbase/pull/2323#issuecomment-684556920
Will take care of the checkstyle while committing.
----------------------------------------------------------------
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 #2323: [HBASE-24957] ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not.
Posted by GitBox <gi...@apache.org>.
shahrs87 commented on pull request #2323:
URL: https://github.com/apache/hbase/pull/2323#issuecomment-683372226
@virajjasani @bharathv Could you please help review this change ? Thank you !
----------------------------------------------------------------
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