This is an automated email from the ASF dual-hosted git repository.
elserj pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/master by this push:
new e44fe49 HBASE-22054: Space Quota: Compaction is not working for super
user in case of NO_WRITES_COMPACTIONS
e44fe49 is described below
commit e44fe4964a25644de2912a08a55784f2268f5e9c
Author: Sakthi <sakthivel.azh...@gmail.com>
AuthorDate: Wed Apr 3 15:52:51 2019 -0700
HBASE-22054: Space Quota: Compaction is not working for super user in case
of NO_WRITES_COMPACTIONS
Signed-off-by: Josh Elser <els...@apache.org>
---
.../apache/hadoop/hbase/security/Superusers.java | 3 ++
.../hadoop/hbase/regionserver/CompactSplit.java | 8 ++-
.../hadoop/hbase/regionserver/HRegionServer.java | 5 ++
.../quotas/TestSuperUserQuotaPermissions.java | 57 ++++++++++++++++------
.../TestCompactionLifeCycleTracker.java | 11 ++---
src/main/asciidoc/_chapters/ops_mgt.adoc | 1 +
6 files changed, 62 insertions(+), 23 deletions(-)
diff --git
a/hbase-common/src/main/java/org/apache/hadoop/hbase/security/Superusers.java
b/hbase-common/src/main/java/org/apache/hadoop/hbase/security/Superusers.java
index b5566e6..a7b2782 100644
---
a/hbase-common/src/main/java/org/apache/hadoop/hbase/security/Superusers.java
+++
b/hbase-common/src/main/java/org/apache/hadoop/hbase/security/Superusers.java
@@ -91,6 +91,9 @@ public final class Superusers {
throw new IllegalStateException("Super users/super groups lists"
+ " have not been initialized properly.");
}
+ if (user == null){
+ throw new IllegalArgumentException("Null user passed for super user
check");
+ }
if (superUsers.contains(user.getShortName())) {
return true;
}
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
index ae3cc15..265b8d5 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
@@ -47,6 +47,7 @@ import
org.apache.hadoop.hbase.regionserver.compactions.CompactionRequestImpl;
import org.apache.hadoop.hbase.regionserver.compactions.CompactionRequester;
import
org.apache.hadoop.hbase.regionserver.throttle.CompactionThroughputControllerFactory;
import org.apache.hadoop.hbase.regionserver.throttle.ThroughputController;
+import org.apache.hadoop.hbase.security.Superusers;
import org.apache.hadoop.hbase.security.User;
import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
import org.apache.hadoop.hbase.util.StealJobQueue;
@@ -341,8 +342,11 @@ public class CompactSplit implements CompactionRequester,
PropagatingConfigurati
}
RegionServerSpaceQuotaManager spaceQuotaManager =
this.server.getRegionServerSpaceQuotaManager();
- if (spaceQuotaManager != null &&
-
spaceQuotaManager.areCompactionsDisabled(region.getTableDescriptor().getTableName()))
{
+
+ if (user != null && !Superusers.isSuperUser(user) && spaceQuotaManager !=
null
+ &&
spaceQuotaManager.areCompactionsDisabled(region.getTableDescriptor().getTableName()))
{
+ // Enter here only when:
+ // It's a user generated req, the user is super user, quotas enabled,
compactions disabled.
String reason = "Ignoring compaction request for " + region +
" as an active space quota violation " + " policy disallows
compactions.";
tracker.notExecuted(store, reason);
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
index b78110f..bcbc78b 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
@@ -3764,6 +3764,11 @@ public class HRegionServer extends HasThread implements
old.stop("configuration change");
}
this.flushThroughputController =
FlushThroughputControllerFactory.create(this, newConf);
+ try {
+ Superusers.initialize(newConf);
+ } catch (IOException e) {
+ LOG.warn("Failed to initialize SuperUsers on reloading of the
configuration");
+ }
}
@Override
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSuperUserQuotaPermissions.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSuperUserQuotaPermissions.java
index 01f8a2f..6785339 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSuperUserQuotaPermissions.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSuperUserQuotaPermissions.java
@@ -42,6 +42,7 @@ import
org.apache.hadoop.hbase.security.access.AccessControlClient;
import org.apache.hadoop.hbase.security.access.AccessController;
import org.apache.hadoop.hbase.security.access.Permission.Action;
import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.security.UserGroupInformation;
import org.junit.AfterClass;
import org.junit.Before;
@@ -125,10 +126,6 @@ public class TestSuperUserQuotaPermissions {
try (Connection conn = getConnection()) {
Admin admin = conn.getAdmin();
final TableName tn = helper.createTableWithRegions(admin, 5);
- final long sizeLimit = 2L * SpaceQuotaHelperForTests.ONE_MEGABYTE;
- QuotaSettings settings = QuotaSettingsFactory.limitTableSpace(
- tn, sizeLimit, SpaceViolationPolicy.NO_WRITES_COMPACTIONS);
- admin.setQuota(settings);
// Grant the normal user permissions
try {
AccessControlClient.grant(
@@ -149,12 +146,23 @@ public class TestSuperUserQuotaPermissions {
@Override
public Void call() throws Exception {
try (Connection conn = getConnection()) {
- helper.writeData(tn, 3L * SpaceQuotaHelperForTests.ONE_MEGABYTE);
+ // Write way more data so that we have HFiles > numRegions after
flushes
+ // helper.writeData flushes itself, so no need to flush explicitly
+ helper.writeData(tn, 2L * SpaceQuotaHelperForTests.ONE_MEGABYTE);
+ helper.writeData(tn, 2L * SpaceQuotaHelperForTests.ONE_MEGABYTE);
return null;
}
}
});
+ final long sizeLimit = 2L * SpaceQuotaHelperForTests.ONE_MEGABYTE;
+ QuotaSettings settings = QuotaSettingsFactory.limitTableSpace(
+ tn, sizeLimit, SpaceViolationPolicy.NO_WRITES_COMPACTIONS);
+
+ try (Connection conn = getConnection()) {
+ conn.getAdmin().setQuota(settings);
+ }
+
waitForTableToEnterQuotaViolation(tn);
// Should throw an exception, unprivileged users cannot compact due to the
quota
@@ -170,19 +178,29 @@ public class TestSuperUserQuotaPermissions {
});
fail("Expected an exception trying to compact a table with a quota
violation");
} catch (DoNotRetryIOException e) {
- // Expected
+ // Expected Exception.
+ LOG.debug("message", e);
}
- // Should not throw an exception (superuser can do anything)
- doAsSuperUser(new Callable<Void>() {
- @Override
- public Void call() throws Exception {
- try (Connection conn = getConnection()) {
- conn.getAdmin().majorCompact(tn);
- return null;
+ try{
+ // Should not throw an exception (superuser can do anything)
+ doAsSuperUser(new Callable<Void>() {
+ @Override
+ public Void call() throws Exception {
+ try (Connection conn = getConnection()) {
+ conn.getAdmin().majorCompact(tn);
+ return null;
+ }
}
- }
- });
+ });
+ } catch (Exception e) {
+ // Unexpected Exception.
+ LOG.debug("message", e);
+ fail("Did not expect an exception to be thrown while a super user tries "
+ + "to compact a table with a quota violation");
+ }
+ int numberOfRegions = TEST_UTIL.getAdmin().getRegions(tn).size();
+ waitForHFilesCountLessorEqual(tn, Bytes.toBytes("f1"), numberOfRegions);
}
@Test
@@ -299,4 +317,13 @@ public class TestSuperUserQuotaPermissions {
}
});
}
+
+ private void waitForHFilesCountLessorEqual(TableName tn, byte[] cf, int
count) throws Exception {
+ Waiter.waitFor(TEST_UTIL.getConfiguration(), 30 * 1000, 1000, new
Predicate<Exception>() {
+ @Override
+ public boolean evaluate() throws Exception {
+ return TEST_UTIL.getNumHFiles(tn, cf) <= count;
+ }
+ });
+ }
}
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactionLifeCycleTracker.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactionLifeCycleTracker.java
index e680e86..6cd91a7 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactionLifeCycleTracker.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactionLifeCycleTracker.java
@@ -58,6 +58,7 @@ import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
+import org.junit.Ignore;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -273,7 +274,10 @@ public class TestCompactionLifeCycleTracker {
assertTrue(tracker.afterExecuteStores.isEmpty());
}
- @Test
+ // This test assumes that compaction wouldn't happen with null user.
+ // But null user means system generated compaction so compaction should
happen
+ // even if the space quota is violated. So this test should be
removed/ignored.
+ @Ignore @Test
public void testSpaceQuotaViolation() throws IOException,
InterruptedException {
region.getRegionServerServices().getRegionServerSpaceQuotaManager().enforceViolationPolicy(NAME,
new SpaceQuotaSnapshot(new
SpaceQuotaStatus(SpaceViolationPolicy.NO_WRITES_COMPACTIONS), 10L,
@@ -286,11 +290,6 @@ public class TestCompactionLifeCycleTracker {
tracker.notExecutedStores.sort((p1, p2) ->
p1.getFirst().getColumnFamilyName()
.compareTo(p2.getFirst().getColumnFamilyName()));
- assertEquals(Bytes.toString(CF1),
- tracker.notExecutedStores.get(0).getFirst().getColumnFamilyName());
- assertThat(tracker.notExecutedStores.get(0).getSecond(),
- containsString("space quota violation"));
-
assertEquals(Bytes.toString(CF2),
tracker.notExecutedStores.get(1).getFirst().getColumnFamilyName());
assertThat(tracker.notExecutedStores.get(1).getSecond(),
diff --git a/src/main/asciidoc/_chapters/ops_mgt.adoc
b/src/main/asciidoc/_chapters/ops_mgt.adoc
index 1924011..4bec817 100644
--- a/src/main/asciidoc/_chapters/ops_mgt.adoc
+++ b/src/main/asciidoc/_chapters/ops_mgt.adoc
@@ -2522,6 +2522,7 @@ take when the quota subject's usage exceeds the `LIMIT`.
The following are valid
* `NO_INSERTS` - No new data may be written (e.g. `Put`, `Increment`,
`Append`).
* `NO_WRITES` - Same as `NO_INSERTS` but `Deletes` are also disallowed.
* `NO_WRITES_COMPACTIONS` - Same as `NO_WRITES` but compactions are also
disallowed.
+** This policy only prevents user-submitted compactions. System can still run
compactions.
* `DISABLE` - The table(s) are disabled, preventing all read/write access.
.Setting simple space quotas
