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/07/25 23:47:19 UTC

[GitHub] [hbase] sanjeetnishad95 opened a new pull request #2142: HBASE-24774 Space Quota violation policy Disable does not work at nam…

sanjeetnishad95 opened a new pull request #2142:
URL: https://github.com/apache/hbase/pull/2142


   …espace level


----------------------------------------------------------------
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 #2142: HBASE-24774 Space Quota violation policy Disable does not work at nam…

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  0s |  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  |   4m  1s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 15s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m  7s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 43s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 14s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  12m  9s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m 13s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 13s |  The patch does not generate ASF License warnings.  |
   |  |   |  35m 33s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2142/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2142 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 5b4a3abf05ad 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 975cdf7b88 |
   | Max. process+thread count | 84 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2142/1/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
   | 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] tedyu commented on a change in pull request #2142: HBASE-24774 Space Quota violation policy Disable does not work at nam…

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



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaObserverChore.java
##########
@@ -433,6 +433,11 @@ void updateNamespaceQuota(
             LOG.info(tableInNS + " moving into violation of namespace space quota with policy "
                 + targetStatus.getPolicy());
             this.snapshotNotifier.transitionTable(tableInNS, targetSnapshot);
+            // when the Namespace is in violation due to Disable Policy, Disable the table
+            if (targetStatus.isInViolation()

Review comment:
       snapshotNotifier.transitionTable may throw IOE.
   Do you want to enclose the call to snapshotNotifier.transitionTable in try / catch block so that the new code would always run ?

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaObserverChore.java
##########
@@ -433,6 +433,11 @@ void updateNamespaceQuota(
             LOG.info(tableInNS + " moving into violation of namespace space quota with policy "
                 + targetStatus.getPolicy());
             this.snapshotNotifier.transitionTable(tableInNS, targetSnapshot);
+            // when the Namespace is in violation due to Disable Policy, Disable the table
+            if (targetStatus.isInViolation()

Review comment:
       The else block starting at line 418 is for targetStatus.isInViolation()
   So if SpaceViolationPolicy.DISABLE is in effect, I think the table should be disabled.
   
   We'd better keep quota table consistent with regard to the violation.




----------------------------------------------------------------
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] sanjeetnishad95 commented on a change in pull request #2142: HBASE-24774 Space Quota violation policy Disable does not work at nam…

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



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaObserverChore.java
##########
@@ -433,6 +433,11 @@ void updateNamespaceQuota(
             LOG.info(tableInNS + " moving into violation of namespace space quota with policy "
                 + targetStatus.getPolicy());
             this.snapshotNotifier.transitionTable(tableInNS, targetSnapshot);
+            // when the Namespace is in violation due to Disable Policy, Disable the table
+            if (targetStatus.isInViolation()

Review comment:
       hi @tedyu yes I can enclose it in a try/catch but I have a small question. If the `snapshotNotifier.transitionTable` failed with an exception and if it is caught and the table is disabled, wouldn't there be inconsistency as the quota table will still show the table as non-violated(wrong usage and limit) but at the same time the table will be disabled because of Quota violation?

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaObserverChore.java
##########
@@ -433,6 +433,11 @@ void updateNamespaceQuota(
             LOG.info(tableInNS + " moving into violation of namespace space quota with policy "
                 + targetStatus.getPolicy());
             this.snapshotNotifier.transitionTable(tableInNS, targetSnapshot);
+            // when the Namespace is in violation due to Disable Policy, Disable the table
+            if (targetStatus.isInViolation()

Review comment:
       Yes, that makes sense. I have updated the PR.




----------------------------------------------------------------
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 #2142: HBASE-24774 Space Quota violation policy Disable does not work at nam…

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






----------------------------------------------------------------
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 #2142: HBASE-24774 Space Quota violation policy Disable does not work at nam…

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 30s |  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  |   3m 47s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 53s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 41s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 45s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 34s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 54s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 54s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 36s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 37s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 159m 24s |  hbase-server in the patch passed.  |
   |  |   | 183m 39s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2142/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2142 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux b53a46d34392 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 / 975cdf7b88 |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2142/1/testReport/ |
   | Max. process+thread count | 3839 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2142/1/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | 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