[GitHub] [hbase] tedyu commented on a change in pull request #2142: HBASE-24774 Space Quota violation policy Disable does not work at nam…

[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