Skip to content

HBASE-22142 Space quota: If table inside namespace having space quota is dropped, data size usage is still considered for the drop table. #598

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

Closed
wants to merge 2 commits into from
Closed

HBASE-22142 Space quota: If table inside namespace having space quota is dropped, data size usage is still considered for the drop table. #598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6208055
HBASE-22142
shardul-cr7 Sep 9, 2019
b1dbe87
review comments handled
shardul-cr7 Sep 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotaManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -740,5 +740,14 @@ public void processFileArchivals(FileArchiveNotificationRequest request, Connect
notifier.addArchivedFiles(filesWithSize);
}
}

/**
* Removes each region size entry where the RegionInfo references the provided TableName.
*
* @param tableName tableName.
*/
public void removeRegionSizesForTable(TableName tableName) {
regionSizes.keySet().removeIf(regionInfo -> regionInfo.getTable().equals(tableName));
}
}

47 changes: 34 additions & 13 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotasObserver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,22 @@
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.Admin;
import org.apache.hadoop.hbase.client.Connection;
import org.apache.hadoop.hbase.coprocessor.CoprocessorException;
import org.apache.hadoop.hbase.coprocessor.CoreCoprocessor;
import org.apache.hadoop.hbase.coprocessor.HasMasterServices;
import org.apache.hadoop.hbase.coprocessor.MasterCoprocessor;
import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment;
import org.apache.hadoop.hbase.coprocessor.MasterObserver;
import org.apache.hadoop.hbase.coprocessor.ObserverContext;
import org.apache.hadoop.hbase.master.MasterServices;
import org.apache.yetus.audience.InterfaceAudience;
import org.apache.hadoop.hbase.shaded.protobuf.generated.QuotaProtos.Quotas;

/**
* An observer to automatically delete quotas when a table/namespace
* is deleted.
*/
@CoreCoprocessor
@InterfaceAudience.Private
public class MasterQuotasObserver implements MasterCoprocessor, MasterObserver {
public static final String REMOVE_QUOTA_ON_TABLE_DELETE = "hbase.quota.remove.on.table.delete";
Expand All @@ -43,6 +48,7 @@ public class MasterQuotasObserver implements MasterCoprocessor, MasterObserver {
private CoprocessorEnvironment cpEnv;
private Configuration conf;
private boolean quotasEnabled = false;
private MasterServices masterServices;

@Override
public Optional<MasterObserver> getMasterObserver() {
Expand All @@ -51,9 +57,19 @@ public Optional<MasterObserver> getMasterObserver() {

@Override
public void start(CoprocessorEnvironment ctx) throws IOException {
this.cpEnv = ctx;
this.conf = cpEnv.getConfiguration();
this.conf = ctx.getConfiguration();
this.quotasEnabled = QuotaUtil.isQuotaEnabled(conf);

if (!(ctx instanceof MasterCoprocessorEnvironment)) {
throw new CoprocessorException("Must be loaded on master.");
}
// if running on master
MasterCoprocessorEnvironment mEnv = (MasterCoprocessorEnvironment) ctx;
if (mEnv instanceof HasMasterServices) {
this.masterServices = ((HasMasterServices) mEnv).getMasterServices();
} else {
throw new CoprocessorException("Must be loaded on a master having master services.");
}
}

@Override
Expand All @@ -64,18 +80,23 @@ public void postDeleteTable(
return;
}
final Connection conn = ctx.getEnvironment().getConnection();
Quotas quotas = QuotaUtil.getTableQuota(conn, tableName);
if (quotas != null){
if (quotas.hasSpace()){
QuotaSettings settings = QuotaSettingsFactory.removeTableSpaceLimit(tableName);
try (Admin admin = conn.getAdmin()) {
admin.setQuota(settings);
Quotas tableQuotas = QuotaUtil.getTableQuota(conn, tableName);
Quotas namespaceQuotas = QuotaUtil.getNamespaceQuota(conn, tableName.getNamespaceAsString());
if (tableQuotas != null || namespaceQuotas != null) {
// Remove regions of table from space quota map.
this.masterServices.getMasterQuotaManager().removeRegionSizesForTable(tableName);
if (tableQuotas != null) {
if (tableQuotas.hasSpace()) {
QuotaSettings settings = QuotaSettingsFactory.removeTableSpaceLimit(tableName);
try (Admin admin = conn.getAdmin()) {
admin.setQuota(settings);
}
}
}
if (quotas.hasThrottle()){
QuotaSettings settings = QuotaSettingsFactory.unthrottleTable(tableName);
try (Admin admin = conn.getAdmin()) {
admin.setQuota(settings);
if (tableQuotas.hasThrottle()) {
QuotaSettings settings = QuotaSettingsFactory.unthrottleTable(tableName);
try (Admin admin = conn.getAdmin()) {
admin.setQuota(settings);
}
}
}
}
Expand Down
52 changes: 52 additions & 0 deletions hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotaDropTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,23 @@
*/
package org.apache.hadoop.hbase.quotas;

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.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.Put;
import org.apache.hadoop.hbase.client.RegionInfo;
import org.apache.hadoop.hbase.master.HMaster;
import org.apache.hadoop.hbase.testclassification.LargeTests;
import org.apache.hadoop.hbase.util.Bytes;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
Expand Down Expand Up @@ -87,6 +94,51 @@ public void testSetQuotaAndThenDropTableWithDisable() throws Exception {
setQuotaAndThenDropTable(SpaceViolationPolicy.DISABLE);
}

@Test
public void testSetQuotaAndThenDropTableWithRegionReport() throws Exception {
Copy link
Member

@joshelser joshelser Sep 18, 2019

Choose a reason for hiding this comment

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

Great job at capturing this in a concise test case.

Copy link
Contributor Author

@shardul-cr7 shardul-cr7 Sep 19, 2019

Choose a reason for hiding this comment

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

Thanks for the review @joshelser . Did all the changes and pushed. :)

final TableName tn = helper.createTable();
helper.setQuotaLimit(tn, SpaceViolationPolicy.NO_INSERTS, 1L);
helper.writeData(tn, 2L);

final HMaster master = TEST_UTIL.getMiniHBaseCluster().getMaster();
final 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(), tn);
return regionSizes.containsKey(tableRegions.get(0));
}
});

boolean hasRegionSize = false;

// region report should be present before dropping the table.
for (Map.Entry<RegionInfo, Long> entry : quotaManager.snapshotRegionSizes().entrySet()) {
if (entry.getKey().getTable().equals(tn)) {
hasRegionSize = true;
break;
}
}

// regionSize report for the given table should be present before dropping the table.
Assert.assertTrue(hasRegionSize);

// drop the table
TEST_UTIL.getAdmin().disableTable(tn);
TEST_UTIL.getAdmin().deleteTable(tn);

// check if deleted table region report still present in the map.
for (Map.Entry<RegionInfo, Long> entry : quotaManager.snapshotRegionSizes().entrySet()) {
if (entry.getKey().getTable().equals(tn)) {
Assert.fail("Dropped table regionSizes were not deleted during the drop command");
}
}
}

private void setQuotaAndThenDropTable(SpaceViolationPolicy policy) throws Exception {
Put put = new Put(Bytes.toBytes("to_reject"));
put.addColumn(Bytes.toBytes(SpaceQuotaHelperForTests.F1), Bytes.toBytes("to"),
Expand Down