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/06/20 00:06:14 UTC

[GitHub] [hbase] skochhar opened a new pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

skochhar opened a new pull request #1935:
URL: https://github.com/apache/hbase/pull/1935


   HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Namespace level 
   
   When a namespace space quota is deleted, the namespace quota (q:s) row from hbase:quota table is removed, however the related u:p rows from tables in the namespace still stay. Removing the u:p rows for tables in the namespace to fix this issue.


----------------------------------------------------------------
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 #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   2m 37s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 21s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 43s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 37s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 20s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 26s |  hbase-client in master failed.  |
   | -0 :warning: |  javadoc  |   0m 42s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 33s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 38s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 38s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 21s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 25s |  hbase-client in the patch failed.  |
   | -0 :warning: |  javadoc  |   0m 41s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 25s |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  | 199m 59s |  hbase-server in the patch passed.  |
   |  |   | 234m  0s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1935 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 9de714d8fa26 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 / 16f306b4a1 |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/5/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-client.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/5/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/5/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-client.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/5/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/5/testReport/ |
   | Max. process+thread count | 3449 (vs. ulimit of 12500) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/5/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



[GitHub] [hbase] Apache-HBase commented on pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 31s |  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  |   5m  9s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 18s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 37s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 47s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 46s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 14s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 14s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 46s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 50s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 221m 57s |  hbase-server in the patch failed.  |
   |  |   | 253m  7s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.11 Server=19.03.11 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1935 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 6033a2470a26 4.15.0-91-generic #92-Ubuntu SMP Fri Feb 28 11:09:48 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 3c31981179 |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/1/testReport/ |
   | Max. process+thread count | 3121 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/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



[GitHub] [hbase] Apache-HBase commented on pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  1s |  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 58s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 26s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 32s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 18s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   1m 23s |  hbase-server: The patch generated 5 new + 6 unchanged - 0 fixed = 11 total (was 6)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  14m 52s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m 54s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 16s |  The patch does not generate ASF License warnings.  |
   |  |   |  41m 43s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1935 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 1df75ec5df14 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 / 84e246f9b1 |
   | checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/2/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
   | Max. process+thread count | 95 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/2/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] skochhar commented on pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   
   > > My change removes these QUOTA_FAMILY_USAGE column family for the tables when a namespace quota is deleted and allows the user to insert into the tables in the namespace when the violated space quota is removed.
   > 
   > Got it. But I thought you should fix the problem not in QuotaUtil class, maybe in MasterQuotaManager? Keep QuotaUtil simple and fix this in upper layer. Thanks.
   
   Thanks Guanghao, I had looked at MasterQuotaManager, it in turn calls QuotaUtil.deleteNamespaceQuota(), and there are other classes that also directly call QuotaUtil.deleteNamespaceQuota(). If I make a change only in MasterQuotaManager, other classes that call QuotaUtil.deleteNamespaceQuota() will not get the fix.


----------------------------------------------------------------
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] infraio edited a comment on pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   > My change removes these QUOTA_FAMILY_USAGE column family for the tables when a namespace quota is deleted and allows the user to insert into the tables in the namespace when the violated space quota is removed.
   
   Got it. But I thought you should fix the problem not in QuotaUtil class, maybe in MasterQuotaManager? Keep QuotaUtil simple and fix this in upper layer. 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 #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 27s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 23s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 13s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 20s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m  5s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  0s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 44s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 22s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 22s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   7m 20s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m 13s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 27s |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  | 202m 55s |  hbase-server in the patch passed.  |
   |  |   | 234m 45s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1935 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux ec9de4fad3cc 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 / 16f306b4a1 |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/5/testReport/ |
   | Max. process+thread count | 3575 (vs. ulimit of 12500) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/5/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



[GitHub] [hbase] infraio commented on pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   > My change removes these QUOTA_FAMILY_USAGE column family for the tables when a namespace quota is deleted and allows the user to insert into the tables in the namespace when the violated space quota is removed.
   
   Got it. But I thought you should fix the problem not in QuotaUtil class, maybe in MasterQuotaManager?


----------------------------------------------------------------
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] joshelser commented on pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   > My change removes these QUOTA_FAMILY_USAGE column family for the tables when a namespace quota is deleted and allows the user to insert into the tables in the namespace when the violated space quota is removed.
   
   > But I thought you should fix the problem not in QuotaUtil class, maybe in MasterQuotaManager? Keep QuotaUtil simple and fix this in upper layer.
   
   Thanks for the explanation in https://github.com/apache/hbase/pull/1935#pullrequestreview-439900394, Surbhi. This helps tremendously.
   
   I'm not sure that removing the `u:p` cell is the correct solution. These columns are just tracking the _size_ of the table. In themselves, they aren't controlling anything (if memory serves). Instead, we should know that when we delete the NamespaceQuota, MasterQuotaManager needs to become aware of this (somehow) and remove any SpaceQuotaViolationPolicies that are in effect.
   
   I suspect your fix does work, but for the wrong reason. If you remove the sizes for these tables, the Master will simply think that the table is no longer in validation. If you have a table-level quota also defined (like I commented in-code), I think this change will incorrectly cause that table-level to not be applied.


----------------------------------------------------------------
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 #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 10s |  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  7s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 14s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m  8s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 45s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   1m 11s |  hbase-server: The patch generated 2 new + 6 unchanged - 0 fixed = 8 total (was 6)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  13m 18s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m 24s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 12s |  The patch does not generate ASF License warnings.  |
   |  |   |  37m 23s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.11 Server=19.03.11 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1935 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 04b748f7652d 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 / 3c31981179 |
   | checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
   | 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-1935/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] skochhar commented on pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   > > My change removes these QUOTA_FAMILY_USAGE column family for the tables when a namespace quota is deleted and allows the user to insert into the tables in the namespace when the violated space quota is removed.
   > 
   > > But I thought you should fix the problem not in QuotaUtil class, maybe in MasterQuotaManager? Keep QuotaUtil simple and fix this in upper layer.
   > 
   > Thanks for the explanation in [#1935 (review)](https://github.com/apache/hbase/pull/1935#pullrequestreview-439900394), Surbhi. This helps tremendously.
   > 
   > I'm not sure that removing the `u:p` cell is the correct solution. These columns are just tracking the _size_ of the table. In themselves, they aren't controlling anything (if memory serves). Instead, we should know that when we delete the NamespaceQuota, MasterQuotaManager needs to become aware of this (somehow) and remove any SpaceQuotaViolationPolicies that are in effect.
   > 
   > I suspect your fix does work, but for the wrong reason. If you remove the sizes for these tables, the Master will simply think that the table is no longer in validation. If you have a table-level quota also defined (like I commented in-code), I think this change will incorrectly cause that table-level to not be applied.
   
   Thanks Josh, I tested that my fix does not remove any table-level quotas. 
   
   If a namespace "test" has 2 tables "test:table1" and "test:table2".
   And namespace "test" has a space quota on it and the "test.table1" also has a table-quota on it so you see 4 rows in quota table, the row for the namespace quota, the row for the table quota and the usage rows for the tables under the namespace.
   Showing you below scan of hbase:quota table with a namespace "test" and table "test:table1" that has the space quota
   
   
   hbase(main):015:0> scan 'hbase:quota'
   ROW                                                          COLUMN+CELL                                                                                                                                                                      
    n.test                                                      column=q:s, timestamp=1591306396362, value=PBUF\x1A\x07\x08\..                                                                                             
    t.test:table1                                               column=q:s, timestamp=1591274418036, value=PBUF\x1A\x07\x08\..                                                                                             
    t.test:table1                                               column=u:p, timestamp=1591306447760, value=\x0A\x04\x08\..                                                                     
    t.test:table2                                               column=u:p, timestamp=1591306447758, value=\x0A\x04\x08..
   
   hbase(main):018:0> list_quotas
   OWNER                                                        QUOTAS                                                                                                                                                                           
    NAMESPACE => test                                           TYPE => SPACE, NAMESPACE => test, LIMIT => 2.00M, VIOLATION_POLICY => NO_INSERTS                                                                                                 
    TABLE => test:table1                                        TYPE => SPACE, TABLE => test:table1, LIMIT => 30.00M, VIOLATION_POLICY => NO_INSERTS                                                                                             
   2 row(s)
   
   My fix only removes the column family QUOTA_FAMILY_USAGE(u:p here) for the table and the QUOTA_FAMILY_INFO(q:s here) stay with my fix, so the table-quotas still get applied once the namespace quota is removed.                   
   
   


----------------------------------------------------------------
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 #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 15s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 31s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 53s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 40s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   3m  7s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 46s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 28s |  hbase-client: The patch generated 1 new + 2 unchanged - 0 fixed = 3 total (was 2)  |
   | -0 :warning: |  checkstyle  |   1m 10s |  hbase-server: The patch generated 3 new + 6 unchanged - 0 fixed = 9 total (was 6)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  12m 20s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   3m 29s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 22s |  The patch does not generate ASF License warnings.  |
   |  |   |  40m 10s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/5/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1935 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 09cb01171fe1 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 16f306b4a1 |
   | checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/5/artifact/yetus-general-check/output/diff-checkstyle-hbase-client.txt |
   | checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/5/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
   | Max. process+thread count | 84 (vs. ulimit of 12500) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/5/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] joshelser commented on a change in pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaTableUtil.java
##########
@@ -628,6 +628,34 @@ static Put createPutForNamespaceSnapshotSize(String namespace, long size) {
     }
   }
 
+  /**
+   * Remove table usage snapshots (u:p columns) for the namespace passed
+   * @param connection connection to re-use
+   * @param namespace the namespace to fetch the list of table usage snapshots
+   */
+  static void deleteTableUsageSnapshotsForNamespace(Connection connection, String namespace)
+    throws IOException {
+    Scan s = new Scan();
+    //Get rows for all tables in namespace
+    s.setRowPrefixFilter(Bytes.toBytes("t." + namespace));

Review comment:
       need the namespace delimiter at the end of your prefix filter, otherwise you'll over-match
   
   e.g. deleting usage for the namespace 'foo' would also end up matching and deleting for the namespace 'foobar'. Making it 't.foo:' should be good enough.

##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaTableUtil.java
##########
@@ -628,6 +628,34 @@ static Put createPutForNamespaceSnapshotSize(String namespace, long size) {
     }
   }
 
+  /**
+   * Remove table usage snapshots (u:p columns) for the namespace passed
+   * @param connection connection to re-use
+   * @param namespace the namespace to fetch the list of table usage snapshots
+   */
+  static void deleteTableUsageSnapshotsForNamespace(Connection connection, String namespace)
+    throws IOException {
+    Scan s = new Scan();
+    //Get rows for all tables in namespace
+    s.setRowPrefixFilter(Bytes.toBytes("t." + namespace));

Review comment:
       Bonus points if you write a test which demonstrates the bug before you fix it :)

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaUtil.java
##########
@@ -266,6 +266,13 @@ private static void deleteQuotas(final Connection connection, final byte[] rowKe
     if (qualifier != null) {
       delete.addColumns(QUOTA_FAMILY_INFO, qualifier);
     }
+    if (isNamespaceRowKey(rowKey)) {
+      String ns = getNamespaceFromRowKey(rowKey);
+      Quotas namespaceQuota = getNamespaceQuota(connection,ns);
+      if (namespaceQuota != null && namespaceQuota.hasSpace()) {
+        deleteTableUsageSnapshotsForNamespace(connection, ns);

Review comment:
       Please add a comment here as to why we need to delete the table usage rows (to prevent the next person from having to come back and do the same investigation we did :)




----------------------------------------------------------------
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] infraio commented on pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   > If I make a change only in MasterQuotaManager, other classes that call QuotaUtil.deleteNamespaceQuota() will not get the fix.
   
   Which class will call this? Clear all related table's quota is not the duty of QuotaUtil.deleteNamespaceQuota(). The upper layer should do this job, call deleteNamespaceQuota first then call deleteTableQuota. 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 #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 28s |  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 45s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  8s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 18s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 44s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 31s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  9s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  9s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 16s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 197m 19s |  hbase-server in the patch passed.  |
   |  |   | 225m  6s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1935 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 98b826bb5e33 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 / 84e246f9b1 |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/2/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/2/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/2/testReport/ |
   | Max. process+thread count | 3076 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/2/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



[GitHub] [hbase] Apache-HBase commented on pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 31s |  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 32s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 39s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 26s |  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 37s |  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  | 135m 45s |  hbase-server in the patch passed.  |
   |  |   | 159m 52s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1935 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 85f5c21e3ae1 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 / 84e246f9b1 |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/2/testReport/ |
   | Max. process+thread count | 4643 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/2/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



[GitHub] [hbase] skochhar commented on a change in pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaTableUtil.java
##########
@@ -628,6 +628,34 @@ static Put createPutForNamespaceSnapshotSize(String namespace, long size) {
     }
   }
 
+  /**
+   * Remove table usage snapshots (u:p columns) for the namespace passed
+   * @param connection connection to re-use
+   * @param namespace the namespace to fetch the list of table usage snapshots
+   */
+  static void deleteTableUsageSnapshotsForNamespace(Connection connection, String namespace)
+    throws IOException {
+    Scan s = new Scan();
+    //Get rows for all tables in namespace
+    s.setRowPrefixFilter(Bytes.toBytes("t." + namespace));

Review comment:
       Thanks Josh for the good catch!




----------------------------------------------------------------
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] asfgit closed pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1935:
URL: https://github.com/apache/hbase/pull/1935


   


----------------------------------------------------------------
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] skochhar commented on a change in pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaUtil.java
##########
@@ -266,6 +266,13 @@ private static void deleteQuotas(final Connection connection, final byte[] rowKe
     if (qualifier != null) {
       delete.addColumns(QUOTA_FAMILY_INFO, qualifier);
     }
+    if (isNamespaceRowKey(rowKey)) {
+      String ns = getNamespaceFromRowKey(rowKey);
+      Quotas namespaceQuota = getNamespaceQuota(connection,ns);
+      if (namespaceQuota != null && namespaceQuota.hasSpace()) {
+        deleteTableUsageSnapshotsForNamespace(connection, ns);

Review comment:
       Will do, 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 #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 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  |   4m 20s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  3s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m  7s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 38s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  1s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  0s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  0s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m  7s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 36s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 235m 23s |  hbase-server in the patch failed.  |
   |  |   | 262m 50s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.11 Server=19.03.11 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1935 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux afcdceb63442 4.15.0-91-generic #92-Ubuntu SMP Fri Feb 28 11:09:48 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 3c31981179 |
   | Default Java | 1.8.0_232 |
   | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/1/testReport/ |
   | Max. process+thread count | 3480 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/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



[GitHub] [hbase] Apache-HBase commented on pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 30s |  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 43s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m  4s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m  3s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 22s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   1m  4s |  hbase-server: The patch generated 4 new + 6 unchanged - 0 fixed = 10 total (was 6)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  11m 14s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m  6s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 15s |  The patch does not generate ASF License warnings.  |
   |  |   |  32m 38s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/3/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1935 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 9b058ce96e41 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 / afe2eac7b6 |
   | checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/3/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/3/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] Apache-HBase commented on pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  8s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 21s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 44s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 37s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 25s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 27s |  hbase-client in master failed.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 32s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 38s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 38s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 31s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 26s |  hbase-client in the patch failed.  |
   | -0 :warning: |  javadoc  |   0m 41s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 20s |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  | 220m 39s |  hbase-server in the patch passed.  |
   |  |   | 253m 17s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1935 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 8bb9f794a9cf 4.15.0-91-generic #92-Ubuntu SMP Fri Feb 28 11:09:48 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 2505c7760d |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/4/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-client.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/4/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/4/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-client.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/4/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/4/testReport/ |
   | Max. process+thread count | 3197 (vs. ulimit of 12500) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/4/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



[GitHub] [hbase] Apache-HBase commented on pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 19s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 21s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m  3s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 38s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   3m  9s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 45s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   1m 11s |  hbase-server: The patch generated 1 new + 6 unchanged - 0 fixed = 7 total (was 6)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  12m 28s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   3m 37s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 25s |  The patch does not generate ASF License warnings.  |
   |  |   |  40m 18s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/4/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1935 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 13c674ccd6f9 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 2505c7760d |
   | checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/4/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
   | Max. process+thread count | 84 (vs. ulimit of 12500) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/4/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] Apache-HBase commented on pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |  10m 51s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 24s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m  5s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 21s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m  3s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 58s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 47s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 21s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 21s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m  3s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  4s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m  8s |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  | 219m 55s |  hbase-server in the patch passed.  |
   |  |   | 259m 23s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1935 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 8818ecb214cb 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 / 2505c7760d |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/4/testReport/ |
   | Max. process+thread count | 2903 (vs. ulimit of 12500) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/4/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



[GitHub] [hbase] joshelser commented on pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   I just had a chat with Surbhi and we walked through the code, end to end, talking about her solution here.
   
   tl;dr I'm with her on the solution, barring one exception where we remove the serialized `SpaceQuotaSnapshot` (column `u:p`) for a table which had both a namespace quota (now removed) and a table quota. Removing the `u:p` record when the table quota still applies would result in a RegionServer potentially move a table out of quota violation when it should still remain in violation.
   
   A sketch of how space quotas work again, because I had to remind myself:
   1. RegionServers report Region space utilization to the master
   2. Master holds this in memory, eventually computing table-level and namespace-level utilization.
   3. Master computes whether each table is or is not in violation, and persists that (with usage and limit) into a SpaceQuotaSnapshot (`u:p` column) for each table.
   4. RegionServers will read these `SpaceQuotaSnapshots` from `hbase:quota` and use this to determine whether or not to reject the new Mutations in `multi()`.
   
   That is, when we drop a quota for a namespace, we should clean up all SpaceQuotaSnapshot records in hbase:quota that do not otherwise have a table-level space quota defined on them. for example,
   ```
   hbase(main):015:0> scan 'hbase:quota'
   ROW COLUMN+CELL
   n.test column=q:s, timestamp=1591306396362, value=PBUF\x1A\x07\x08..
   t.test:table1 column=q:s, timestamp=1591274418036, value=PBUF\x1A\x07\x08..
   t.test:table1 column=u:p, timestamp=1591306447760, value=\x0A\x04\x08..
   t.test:table2 column=u:p, timestamp=1591306447758, value=\x0A\x04\x08..
   ```
   
   when the space quota on namespace `test` is deleted, we would want to delete `t.test:table2 u:p`, but we would not want to delete `t.test:table1 u:p` because there is a quota defined on this table (`t.test:table1 q:s`).
   
   Normally, `QuotaObserverChore` (in the master) is pushing new `SpaceQuotaSnapshot` records into `hbase:quota`, but this class doesn't handle the cleanup. The method that Surbhi has modified is what MasterQuotaManager is ultimately calling (after a couple of levels of indirection), so making this change inside `QuotaUtil` appears to be the right thing to me, hiding the complexity of what it means to delete a namespace quota :)


----------------------------------------------------------------
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] infraio commented on pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   > HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Namespace level
   > 
   > When a namespace space quota is deleted, the namespace quota (q:s) row from hbase:quota table is removed, however the related u:p rows from tables in the namespace still stay. Removing the u:p rows for tables in the namespace to fix this issue.
   
   How this fix related to the issue description?


----------------------------------------------------------------
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] skochhar commented on a change in pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaTableUtil.java
##########
@@ -628,6 +628,34 @@ static Put createPutForNamespaceSnapshotSize(String namespace, long size) {
     }
   }
 
+  /**
+   * Remove table usage snapshots (u:p columns) for the namespace passed
+   * @param connection connection to re-use
+   * @param namespace the namespace to fetch the list of table usage snapshots
+   */
+  static void deleteTableUsageSnapshotsForNamespace(Connection connection, String namespace)
+    throws IOException {
+    Scan s = new Scan();
+    //Get rows for all tables in namespace
+    s.setRowPrefixFilter(Bytes.toBytes("t." + namespace));

Review comment:
       I added a test to check for the bug, in my test I create 2 namespaces with similar names and tested that the code only deletes the usage columns for the tables in the deleted namespace. 




----------------------------------------------------------------
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 #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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


   :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 49s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 54s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 38s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 37s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 25s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 53s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 53s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 32s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 35s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 139m 10s |  hbase-server in the patch passed.  |
   |  |   | 163m 12s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1935 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux e1a0d6d7c6a3 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 / afe2eac7b6 |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/3/testReport/ |
   | Max. process+thread count | 4857 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1935/3/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



[GitHub] [hbase] esteban commented on a change in pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaUtil.java
##########
@@ -264,17 +265,33 @@ private static void deleteQuotas(final Connection connection, final byte[] rowKe
       final byte[] qualifier) throws IOException {
     Delete delete = new Delete(rowKey);
     if (qualifier != null) {
-      delete.addColumns(QUOTA_FAMILY_INFO, qualifier);
+      if (Arrays.equals(qualifier,QUOTA_QUALIFIER_POLICY)) {
+        delete.addColumns(QUOTA_FAMILY_USAGE, qualifier);
+      } else
+        delete.addColumns(QUOTA_FAMILY_INFO, qualifier);
     }
-    doDelete(connection, delete);
     if (isNamespaceRowKey(rowKey)) {
-      TableName[] tableArray = connection.getAdmin().listTableNamesByNamespace(getNamespaceFromRowKey(rowKey));
-      for (TableName tableName: tableArray) {
-        if (QuotaUtil.getTableQuota(connection, tableName) == null) {
-          deleteTableQuota(connection,tableName);
+      //Check namespace is not deleted before you get info about quota and list of tables in namespace
+      NamespaceDescriptor[] descs = connection.getAdmin().listNamespaceDescriptors();
+      String ns = getNamespaceFromRowKey(rowKey);
+      int index = 0;
+      while (index < descs.length) {
+        if (ns.equals(descs[index].getName())) {
+          Quotas namespaceQuota = getNamespaceQuota(connection,ns);
+          if (namespaceQuota != null && namespaceQuota.hasSpace()) {
+            TableName[] tableArray = connection.getAdmin().listTableNamesByNamespace(ns);
+            for (TableName tableName : tableArray) {
+              deleteQuotas(connection, getTableRowKey(tableName), QUOTA_QUALIFIER_POLICY);
+            }
+          }
+          //Exit the while loop by moving to last index
+          index = descs.length;
+        } else {
+          index++;
         }
       }
     }
+    doDelete(connection, delete);

Review comment:
       What would happen if we get an exception trying to delete a quota? Would it be necessary to retry the operation 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] joshelser commented on a change in pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaUtil.java
##########
@@ -264,7 +265,32 @@ private static void deleteQuotas(final Connection connection, final byte[] rowKe
       final byte[] qualifier) throws IOException {
     Delete delete = new Delete(rowKey);
     if (qualifier != null) {
-      delete.addColumns(QUOTA_FAMILY_INFO, qualifier);
+      //Check if delete qualifier is for persisted space quota snapshot usage column family
+      if (Arrays.equals(qualifier,QUOTA_QUALIFIER_POLICY)) {
+        delete.addColumns(QUOTA_FAMILY_USAGE, qualifier);
+      } else
+        delete.addColumns(QUOTA_FAMILY_INFO, qualifier);
+    }
+    if (isNamespaceRowKey(rowKey)) {
+      //Check namespace is not deleted before you get info about quota and list of tables in namespace
+      NamespaceDescriptor[] descs = connection.getAdmin().listNamespaceDescriptors();

Review comment:
       Beware that this requires ADMIN to list descriptors. This might be increasing the permissions required to modify quotas.

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaUtil.java
##########
@@ -264,7 +265,32 @@ private static void deleteQuotas(final Connection connection, final byte[] rowKe
       final byte[] qualifier) throws IOException {
     Delete delete = new Delete(rowKey);
     if (qualifier != null) {
-      delete.addColumns(QUOTA_FAMILY_INFO, qualifier);
+      //Check if delete qualifier is for persisted space quota snapshot usage column family
+      if (Arrays.equals(qualifier,QUOTA_QUALIFIER_POLICY)) {
+        delete.addColumns(QUOTA_FAMILY_USAGE, qualifier);
+      } else

Review comment:
       Missing curly-brackets on else-branch

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaUtil.java
##########
@@ -264,7 +265,32 @@ private static void deleteQuotas(final Connection connection, final byte[] rowKe
       final byte[] qualifier) throws IOException {
     Delete delete = new Delete(rowKey);
     if (qualifier != null) {
-      delete.addColumns(QUOTA_FAMILY_INFO, qualifier);
+      //Check if delete qualifier is for persisted space quota snapshot usage column family
+      if (Arrays.equals(qualifier,QUOTA_QUALIFIER_POLICY)) {
+        delete.addColumns(QUOTA_FAMILY_USAGE, qualifier);
+      } else
+        delete.addColumns(QUOTA_FAMILY_INFO, qualifier);
+    }
+    if (isNamespaceRowKey(rowKey)) {
+      //Check namespace is not deleted before you get info about quota and list of tables in namespace
+      NamespaceDescriptor[] descs = connection.getAdmin().listNamespaceDescriptors();
+      String ns = getNamespaceFromRowKey(rowKey);
+      int index = 0;
+      while (index < descs.length) {
+        if (ns.equals(descs[index].getName())) {
+          Quotas namespaceQuota = getNamespaceQuota(connection,ns);
+          if (namespaceQuota != null && namespaceQuota.hasSpace()) {
+            TableName[] tableArray = connection.getAdmin().listTableNamesByNamespace(ns);
+            for (TableName tableName : tableArray) {
+              deleteQuotas(connection, getTableRowKey(tableName), QUOTA_QUALIFIER_POLICY);

Review comment:
       If I override the namespace quota with a table-level quota, will this remove my table-level quota too?
   
   e.g. say there is a 10G quota on a namespace, but I further restrict a specific table in that namespace to be 1G. I would expect that my table can only use 1G at most.




----------------------------------------------------------------
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] skochhar commented on a change in pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…

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



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/namespace/NamespaceAuditor.java
##########
@@ -18,6 +18,7 @@
 package org.apache.hadoop.hbase.namespace;
 
 import java.io.IOException;
+import java.util.Set;

Review comment:
       Good catch Sakthi, I will remove the import

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaUtil.java
##########
@@ -264,17 +265,33 @@ private static void deleteQuotas(final Connection connection, final byte[] rowKe
       final byte[] qualifier) throws IOException {
     Delete delete = new Delete(rowKey);
     if (qualifier != null) {
-      delete.addColumns(QUOTA_FAMILY_INFO, qualifier);
+      if (Arrays.equals(qualifier,QUOTA_QUALIFIER_POLICY)) {
+        delete.addColumns(QUOTA_FAMILY_USAGE, qualifier);
+      } else
+        delete.addColumns(QUOTA_FAMILY_INFO, qualifier);
     }
-    doDelete(connection, delete);
     if (isNamespaceRowKey(rowKey)) {
-      TableName[] tableArray = connection.getAdmin().listTableNamesByNamespace(getNamespaceFromRowKey(rowKey));
-      for (TableName tableName: tableArray) {
-        if (QuotaUtil.getTableQuota(connection, tableName) == null) {
-          deleteTableQuota(connection,tableName);
+      //Check namespace is not deleted before you get info about quota and list of tables in namespace
+      NamespaceDescriptor[] descs = connection.getAdmin().listNamespaceDescriptors();
+      String ns = getNamespaceFromRowKey(rowKey);
+      int index = 0;
+      while (index < descs.length) {
+        if (ns.equals(descs[index].getName())) {
+          Quotas namespaceQuota = getNamespaceQuota(connection,ns);
+          if (namespaceQuota != null && namespaceQuota.hasSpace()) {
+            TableName[] tableArray = connection.getAdmin().listTableNamesByNamespace(ns);
+            for (TableName tableName : tableArray) {
+              deleteQuotas(connection, getTableRowKey(tableName), QUOTA_QUALIFIER_POLICY);
+            }
+          }
+          //Exit the while loop by moving to last index
+          index = descs.length;
+        } else {
+          index++;
         }
       }
     }
+    doDelete(connection, delete);

Review comment:
       
   > What would happen if we get an exception trying to delete a quota? Would it be necessary to retry the operation again?
   
   Thanks for reviewing Esteban. 
   
   I am doing null checks everywhere to avoid NPEs. I tested this code with a namespace quota on a namespace both without any tables, and with tables and the code worked fine.
   
   I see that in line 282 connection.getAdmin().listTableNamesByNamespace(ns) can throw IOException. I can add try-catch around my new code to avoid the final doDelete() for the namespace getting impacted from any exception in the new code.

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaUtil.java
##########
@@ -264,17 +265,33 @@ private static void deleteQuotas(final Connection connection, final byte[] rowKe
       final byte[] qualifier) throws IOException {
     Delete delete = new Delete(rowKey);
     if (qualifier != null) {
-      delete.addColumns(QUOTA_FAMILY_INFO, qualifier);
+      if (Arrays.equals(qualifier,QUOTA_QUALIFIER_POLICY)) {
+        delete.addColumns(QUOTA_FAMILY_USAGE, qualifier);
+      } else
+        delete.addColumns(QUOTA_FAMILY_INFO, qualifier);
     }
-    doDelete(connection, delete);
     if (isNamespaceRowKey(rowKey)) {
-      TableName[] tableArray = connection.getAdmin().listTableNamesByNamespace(getNamespaceFromRowKey(rowKey));
-      for (TableName tableName: tableArray) {
-        if (QuotaUtil.getTableQuota(connection, tableName) == null) {
-          deleteTableQuota(connection,tableName);
+      //Check namespace is not deleted before you get info about quota and list of tables in namespace
+      NamespaceDescriptor[] descs = connection.getAdmin().listNamespaceDescriptors();
+      String ns = getNamespaceFromRowKey(rowKey);
+      int index = 0;
+      while (index < descs.length) {
+        if (ns.equals(descs[index].getName())) {
+          Quotas namespaceQuota = getNamespaceQuota(connection,ns);
+          if (namespaceQuota != null && namespaceQuota.hasSpace()) {
+            TableName[] tableArray = connection.getAdmin().listTableNamesByNamespace(ns);
+            for (TableName tableName : tableArray) {
+              deleteQuotas(connection, getTableRowKey(tableName), QUOTA_QUALIFIER_POLICY);
+            }
+          }
+          //Exit the while loop by moving to last index
+          index = descs.length;
+        } else {
+          index++;
         }
       }
     }
+    doDelete(connection, delete);

Review comment:
       I checked more usages of this method and other files where getAdmin().listTableNamesByNamespace(ns) is used and in case of error they let the calling method handle it and allow the user to decide to retry the operation. 
   
   The call for deleteQuotas() also mentions about throwing IOException like other methods:
   private static void deleteQuotas(final Connection connection, final byte[] rowKey,
         final byte[] qualifier) throws IOException

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/namespace/NamespaceAuditor.java
##########
@@ -18,6 +18,7 @@
 package org.apache.hadoop.hbase.namespace;
 
 import java.io.IOException;
+import java.util.Set;

Review comment:
       Added a new PR where I removed the import




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