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 108919740c4..72dd36efbb2 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 @@ -90,6 +90,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 98df9b167ec..2f5c9d94731 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; @@ -309,8 +310,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 cc83eb27c24..7fb2c89bf45 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 @@ -3696,6 +3696,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 01f8a2f4bbf..678533923ca 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() { - @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() { + @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() { + @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 e680e861648..6cd91a71140 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 3a43134a368..5a7c62eccdd 100644 --- a/src/main/asciidoc/_chapters/ops_mgt.adoc +++ b/src/main/asciidoc/_chapters/ops_mgt.adoc @@ -2309,6 +2309,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