Skip to content

Commit

Permalink
HBASE-22012 Prevent DISABLE quota violation policy from disabling vio…
Browse files Browse the repository at this point in the history
…lation policy

Space quotas has a feature which intends to avoid enacting a space quota
violation policy when only a subset of the Regions for that Table have
reported their space usage (under the assumption that we cannot make an
informed decision if we do not include all regions in our calculations).

This had the unintended side-effect, when a table is disabled as a part
of a violation policy, of causing the regions for that table to not be
reported which disables the violation policy and enables the table.

Need to make sure that when a table is disabled because of a violation
policy that the code does not automatically move that table out of
violation because region sizes are not being reported (because those
regions are not open).

Closes #572

Signed-off-by: Josh Elser <[email protected]>
  • Loading branch information
shardul-cr7 authored and joshelser committed Sep 26, 2019
1 parent 277c1cc commit 3250a80
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.util.Iterator;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

Expand Down Expand Up @@ -703,23 +704,66 @@ public Map<RegionInfo, Long> snapshotRegionSizes() {
return copy;
}

int pruneEntriesOlderThan(long timeToPruneBefore) {
int pruneEntriesOlderThan(long timeToPruneBefore, QuotaObserverChore quotaObserverChore) {
if (regionSizes == null) {
return 0;
}
int numEntriesRemoved = 0;
Iterator<Entry<RegionInfo,SizeSnapshotWithTimestamp>> iterator =
Iterator<Entry<RegionInfo, SizeSnapshotWithTimestamp>> iterator =
regionSizes.entrySet().iterator();
while (iterator.hasNext()) {
long currentEntryTime = iterator.next().getValue().getTime();
if (currentEntryTime < timeToPruneBefore) {
RegionInfo regionInfo = iterator.next().getKey();
long currentEntryTime = regionSizes.get(regionInfo).getTime();
// do not prune the entries if table is in violation and
// violation policy is disable to avoid cycle of enable/disable.
// Please refer HBASE-22012 for more details.
// prune entries older than time.
if (currentEntryTime < timeToPruneBefore && !isInViolationAndPolicyDisable(
regionInfo.getTable(), quotaObserverChore)) {
iterator.remove();
numEntriesRemoved++;
}
}
return numEntriesRemoved;
}

/**
* Method to check if a table is in violation and policy set on table is DISABLE.
*
* @param tableName tableName to check.
* @param quotaObserverChore QuotaObserverChore instance
* @return returns true if table is in violation and policy is disable else false.
*/
private boolean isInViolationAndPolicyDisable(TableName tableName,
QuotaObserverChore quotaObserverChore) {
boolean isInViolationAtTable = false;
boolean isInViolationAtNamespace = false;
SpaceViolationPolicy tablePolicy = null;
SpaceViolationPolicy namespacePolicy = null;
// Get Current Snapshot for the given table
SpaceQuotaSnapshot tableQuotaSnapshot = quotaObserverChore.getTableQuotaSnapshot(tableName);
SpaceQuotaSnapshot namespaceQuotaSnapshot =
quotaObserverChore.getNamespaceQuotaSnapshot(tableName.getNamespaceAsString());
if (tableQuotaSnapshot != null) {
// check if table in violation
isInViolationAtTable = tableQuotaSnapshot.getQuotaStatus().isInViolation();
Optional<SpaceViolationPolicy> policy = tableQuotaSnapshot.getQuotaStatus().getPolicy();
if (policy.isPresent()) {
tablePolicy = policy.get();
}
}
if (namespaceQuotaSnapshot != null) {
// check namespace in violation
isInViolationAtNamespace = namespaceQuotaSnapshot.getQuotaStatus().isInViolation();
Optional<SpaceViolationPolicy> policy = namespaceQuotaSnapshot.getQuotaStatus().getPolicy();
if (policy.isPresent()) {
namespacePolicy = policy.get();
}
}
return (tablePolicy == SpaceViolationPolicy.DISABLE && isInViolationAtTable) || (
namespacePolicy == SpaceViolationPolicy.DISABLE && isInViolationAtNamespace);
}

public void processFileArchivals(FileArchiveNotificationRequest request, Connection conn,
Configuration conf, FileSystem fs) throws IOException {
final HashMultimap<TableName,Entry<String,Long>> archivedFilesByTable = HashMultimap.create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ void updateNamespaceQuota(
void pruneOldRegionReports() {
final long now = EnvironmentEdgeManager.currentTime();
final long pruneTime = now - regionReportLifetimeMillis;
final int numRemoved = quotaManager.pruneEntriesOlderThan(pruneTime);
final int numRemoved = quotaManager.pruneEntriesOlderThan(pruneTime,this);
if (LOG.isTraceEnabled()) {
LOG.trace("Removed " + numRemoved + " old region size reports that were older than "
+ pruneTime + ".");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,18 +73,19 @@ public void testOldEntriesRemoved() {

assertEquals(5, manager.snapshotRegionSizes().size());

QuotaObserverChore chore = mock(QuotaObserverChore.class);
// Prune nothing
assertEquals(0, manager.pruneEntriesOlderThan(0));
assertEquals(0, manager.pruneEntriesOlderThan(0, chore));
assertEquals(5, manager.snapshotRegionSizes().size());
assertEquals(0, manager.pruneEntriesOlderThan(10));
assertEquals(0, manager.pruneEntriesOlderThan(10, chore));
assertEquals(5, manager.snapshotRegionSizes().size());

// Prune the elements at time1
assertEquals(2, manager.pruneEntriesOlderThan(15));
assertEquals(2, manager.pruneEntriesOlderThan(15, chore));
assertEquals(3, manager.snapshotRegionSizes().size());

// Prune the elements at time2
assertEquals(2, manager.pruneEntriesOlderThan(30));
assertEquals(2, manager.pruneEntriesOlderThan(30, chore));
assertEquals(1, manager.snapshotRegionSizes().size());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,31 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicLong;

import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.DoNotRetryIOException;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HBaseTestingUtility;
import org.apache.hadoop.hbase.MetaTableAccessor;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.Waiter;
import org.apache.hadoop.hbase.client.Admin;
import org.apache.hadoop.hbase.client.Append;
import org.apache.hadoop.hbase.client.Delete;
import org.apache.hadoop.hbase.client.Increment;
import org.apache.hadoop.hbase.client.Put;
import org.apache.hadoop.hbase.client.RegionInfo;
import org.apache.hadoop.hbase.client.Table;
import org.apache.hadoop.hbase.master.HMaster;
import org.apache.hadoop.hbase.security.AccessDeniedException;
import org.apache.hadoop.hbase.testclassification.LargeTests;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.util.StringUtils;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
Expand Down Expand Up @@ -221,4 +228,33 @@ public void testTableQuotaOverridesNamespaceQuota() throws Exception {
Bytes.toBytes("reject"));
helper.verifyViolation(policy, tn, p);
}

@Test
public void testDisablePolicyQuotaAndViolate() throws Exception {
TableName tableName = helper.createTable();
helper.setQuotaLimit(tableName, SpaceViolationPolicy.DISABLE, 1L);
helper.writeData(tableName, SpaceQuotaHelperForTests.ONE_MEGABYTE * 2L);
TEST_UTIL.getConfiguration()
.setLong("hbase.master.quotas.region.report.retention.millis", 100);

HMaster master = TEST_UTIL.getMiniHBaseCluster().getMaster();
MasterQuotaManager quotaManager = master.getMasterQuotaManager();

// Make sure the master has report for the table.
Waiter.waitFor(TEST_UTIL.getConfiguration(), 30 * 1000, new Waiter.Predicate<Exception>() {
@Override
public boolean evaluate() throws Exception {
Map<RegionInfo, Long> regionSizes = quotaManager.snapshotRegionSizes();
List<RegionInfo> tableRegions =
MetaTableAccessor.getTableRegions(TEST_UTIL.getConnection(), tableName);
return regionSizes.containsKey(tableRegions.get(0));
}
});

// Check if disabled table region report present in the map after retention period expired.
// It should be present after retention period expired.
final long regionSizes = quotaManager.snapshotRegionSizes().keySet().stream()
.filter(k -> k.getTable().equals(tableName)).count();
Assert.assertTrue(regionSizes > 0);
}
}

0 comments on commit 3250a80

Please sign in to comment.