HBASE-22054: Space Quota: Compaction is not working for super user in case of NO_WRITES_COMPACTIONS

Signed-off-by: Josh Elser <elserj@apache.org>
This commit is contained in:
Sakthi 2019-04-03 15:52:51 -07:00 committed by Josh Elser
parent 3f40df8085
commit e44fe4964a
6 changed files with 62 additions and 23 deletions

View File

@ -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;
}

View File

@ -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);

View File

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

View File

@ -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;
}
});
}
}

View File

@ -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(),

View File

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