Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

HBASE-24774 Space Quota violation policy Disable does not work at nam… #2142

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+82 −1
Open

HBASE-24774 Space Quota violation policy Disable does not work at nam… #2142

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
938a526
HBASE-24774 Space Quota violation policy Disable does not work at nam…
sanjeetnishad95 Jul 25, 2020
ef20731
HBASE-24774 Space Quota violation policy Disable does not work at nam…
sanjeetnishad95 Jul 28, 2020
a277d24
HBASE-24774 Space Quota violation policy Disable does not work at nam…
sanjeetnishad95 Jul 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaObserverChore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that makes sense. I have updated the PR.

&& targetStatus.getPolicy().get() == SpaceViolationPolicy.DISABLE) {
QuotaUtil.disableTableIfNotDisabled(conn, tableInNS);
}
}
}
}
Expand Down
44 changes: 44 additions & 0 deletions ...e-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotaBasicFunctioning.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HBaseTestingUtility;
import org.apache.hadoop.hbase.MetaTableAccessor;
import org.apache.hadoop.hbase.NamespaceDescriptor;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.Waiter;
import org.apache.hadoop.hbase.client.Admin;
Expand Down Expand Up @@ -257,4 +258,47 @@ public boolean evaluate() throws Exception {
.filter(k -> k.getTable().equals(tableName)).count();
Assert.assertTrue(regionSizes > 0);
}

@Test
public void testDisableAtNamespaceLevel() throws Exception {
NamespaceDescriptor nd = helper.createNamespace();
TableName table1 = helper.createTableInNamespace(nd);
TableName table2 = helper.createTableInNamespace(nd);
helper.setQuotaLimit(nd.getName(), SpaceViolationPolicy.DISABLE, 2L);
helper.writeData(table1, 1L * SpaceQuotaHelperForTests.ONE_MEGABYTE);
helper.writeData(table2, 1L * SpaceQuotaHelperForTests.ONE_MEGABYTE);
TEST_UTIL.waitTableDisabled(table2, 20000);
TEST_UTIL.waitTableDisabled(table1, 20000);
}

@Test
public void testTableQuotaOverridesNamespaceQuotaDisableViolation() throws Exception {
NamespaceDescriptor nd = helper.createNamespace();
TableName table1 = helper.createTableInNamespace(nd);
TableName table2 = helper.createTableInNamespace(nd);
helper.setQuotaLimit(nd.getName(), SpaceViolationPolicy.DISABLE, 3L);
helper.setQuotaLimit(table1, SpaceViolationPolicy.NO_INSERTS, 2L);
helper.writeData(table1, 2L * SpaceQuotaHelperForTests.ONE_MEGABYTE);

Put put = new Put(Bytes.toBytes("reject"));
put.addColumn(Bytes.toBytes(SpaceQuotaHelperForTests.F1), Bytes.toBytes("to"),
Bytes.toBytes("reject"));
helper.verifyViolation(SpaceViolationPolicy.NO_INSERTS, table1, put);
helper.writeData(table2, 2L * SpaceQuotaHelperForTests.ONE_MEGABYTE);
TEST_UTIL.waitTableDisabled(table2, 20000);
TEST_UTIL.waitTableEnabled(table1);
}

@Test
public void testTableQuotaOverridesNamespaceQuotaDisableObservance() throws Exception {
NamespaceDescriptor nd = helper.createNamespace();
TableName table1 = helper.createTableInNamespace(nd);
TableName table2 = helper.createTableInNamespace(nd);
helper.setQuotaLimit(nd.getName(), SpaceViolationPolicy.DISABLE, 3L);
helper.setQuotaLimit(table1, SpaceViolationPolicy.NO_INSERTS, 2L);
helper.writeData(table1, 1L * SpaceQuotaHelperForTests.ONE_MEGABYTE);
helper.writeData(table2, 2L * SpaceQuotaHelperForTests.ONE_MEGABYTE);
TEST_UTIL.waitTableDisabled(table2, 20000);
TEST_UTIL.waitTableDisabled(table1, 20000);
}
}