From 2e47d352cfa18609a09554bf174cc59a21ee6338 Mon Sep 17 00:00:00 2001 From: Sakthi Date: Wed, 3 Apr 2019 14:20:46 -0700 Subject: [PATCH] HBASE-22086: Space Quota issue: Deleting snapshot doesn't update the usage of table --- .../hadoop/hbase/quotas/QuotaTableUtil.java | 55 +++++++++++++ .../quotas/SnapshotQuotaObserverChore.java | 23 ++++++ .../hbase/quotas/TestQuotaTableUtil.java | 34 ++++++++ .../TestSnapshotQuotaObserverChore.java | 80 +++++++++++++++++++ 4 files changed, 192 insertions(+) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaTableUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaTableUtil.java index 2b5cb02b2cad81f9a285b72e40eb4f1b35256d71..e7eb8ece0379d28fef488262c3f18962c6905256 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaTableUtil.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaTableUtil.java @@ -24,6 +24,7 @@ import java.io.IOException; import java.util.Collection; import java.util.HashMap; import java.util.List; +import java.util.ArrayList; import java.util.Map; import java.util.Objects; import java.util.regex.Pattern; @@ -40,6 +41,7 @@ import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.ResultScanner; import org.apache.hadoop.hbase.client.Scan; import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.client.Delete; import org.apache.hadoop.hbase.filter.ColumnPrefixFilter; import org.apache.hadoop.hbase.filter.Filter; import org.apache.hadoop.hbase.filter.FilterList; @@ -540,6 +542,38 @@ public class QuotaTableUtil { return p; } + static List createDeletesForExistingTableSnapshotSizes(Connection connection) + throws IOException { + return createDeletesForExistingSnapshotsFromScan(connection, createScanForSpaceSnapshotSizes()); + } + + static List createDeletesForExistingNamespaceSnapshotSizes(Connection connection) + throws IOException { + return createDeletesForExistingSnapshotsFromScan(connection, + createScanForNamespaceSnapshotSizes()); + } + + static List createDeletesForExistingSnapshotsFromScan(Connection connection, Scan scan) + throws IOException { + List deletes = new ArrayList<>(); + try (Table quotaTable = connection.getTable(QUOTA_TABLE_NAME); + ResultScanner rs = quotaTable.getScanner(scan)) { + for (Result r : rs) { + CellScanner cs = r.cellScanner(); + while (cs.advance()) { + Cell c = cs.current(); + byte[] family = Bytes.copy(c.getFamilyArray(), c.getFamilyOffset(), c.getFamilyLength()); + byte[] qual = + Bytes.copy(c.getQualifierArray(), c.getQualifierOffset(), c.getQualifierLength()); + Delete d = new Delete(r.getRow()); + d.addColumns(family, qual); + deletes.add(d); + } + } + return deletes; + } + } + /** * Fetches the computed size of all snapshots against tables in a namespace for space quotas. */ @@ -575,6 +609,27 @@ public class QuotaTableUtil { return QuotaProtos.SpaceQuotaSnapshot.parseFrom(bs).getQuotaUsage(); } + static Scan createScanForNamespaceSnapshotSizes() { + return createScanForNamespaceSnapshotSizes(null); + } + + static Scan createScanForNamespaceSnapshotSizes(String namespace) { + Scan s = new Scan(); + if (namespace == null || namespace.isEmpty()) { + // Read all namespaces, just look at the row prefix + s.setRowPrefixFilter(QUOTA_NAMESPACE_ROW_KEY_PREFIX); + } else { + // Fetch the exact row for the table + byte[] rowkey = getNamespaceRowKey(namespace); + // Fetch just this one row + s.withStartRow(rowkey).withStopRow(rowkey, true); + } + + // Just the usage family and only the snapshot size qualifiers + return s.addFamily(QUOTA_FAMILY_USAGE) + .setFilter(new ColumnPrefixFilter(QUOTA_SNAPSHOT_SIZE_QUALIFIER)); + } + static Scan createScanForSpaceSnapshotSizes() { return createScanForSpaceSnapshotSizes(null); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SnapshotQuotaObserverChore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SnapshotQuotaObserverChore.java index f74bae0c512b6c77b7da3d158b132395892b0f7a..d2c52f64ed0f2757348efba82e0aa0ad77a118f3 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SnapshotQuotaObserverChore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SnapshotQuotaObserverChore.java @@ -23,6 +23,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Map.Entry; +import java.util.List; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; @@ -32,6 +33,7 @@ import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.hbase.ScheduledChore; import org.apache.hadoop.hbase.Stoppable; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Delete; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -110,9 +112,15 @@ public class SnapshotQuotaObserverChore extends ScheduledChore { metrics.incrementSnapshotFetchTime((System.nanoTime() - start) / 1_000_000); } + // Remove old table snapshots data + removeExistingTableSnapshotSizes(); + // For each table, compute the size of each snapshot Map namespaceSnapshotSizes = computeSnapshotSizes(snapshotsToComputeSize); + // Remove old namespace snapshots data + removeExistingNamespaceSnapshotSizes(); + // Write the size data by namespaces to the quota table. // We need to do this "globally" since each FileArchiverNotifier is limited to its own Table. persistSnapshotSizesForNamespaces(namespaceSnapshotSizes); @@ -220,6 +228,21 @@ public class SnapshotQuotaObserverChore extends ScheduledChore { } } + void removeExistingTableSnapshotSizes() throws IOException { + removeExistingSnapshotSizes(QuotaTableUtil.createDeletesForExistingTableSnapshotSizes(conn)); + } + + void removeExistingNamespaceSnapshotSizes() throws IOException { + removeExistingSnapshotSizes( + QuotaTableUtil.createDeletesForExistingNamespaceSnapshotSizes(conn)); + } + + void removeExistingSnapshotSizes(List deletes) throws IOException { + try (Table quotaTable = conn.getTable(QuotaUtil.QUOTA_TABLE_NAME)) { + quotaTable.delete(deletes); + } + } + /** * Extracts the period for the chore from the configuration. * diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaTableUtil.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaTableUtil.java index 6aac054ba8f7a613ea9614756ba143e4414c4201..a103c4aadeff61cfa885c4c76ed66370d00a8cfc 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaTableUtil.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaTableUtil.java @@ -36,6 +36,7 @@ import org.apache.hadoop.hbase.NamespaceDescriptor; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.ConnectionFactory; +import org.apache.hadoop.hbase.client.Delete; import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.ResultScanner; @@ -109,6 +110,39 @@ public class TestQuotaTableUtil { this.connection.close(); } + @Test + public void testDeleteSnapshots() throws Exception { + TableName tn = TableName.valueOf(name.getMethodName()); + try (Table t = connection.getTable(QuotaTableUtil.QUOTA_TABLE_NAME)) { + Quotas quota = Quotas.newBuilder().setSpace( + QuotaProtos.SpaceQuota.newBuilder().setSoftLimit(7l) + .setViolationPolicy(QuotaProtos.SpaceViolationPolicy.NO_WRITES).build()).build(); + QuotaUtil.addTableQuota(connection, tn, quota); + + String snapshotName = name.getMethodName() + "_snapshot"; + t.put(QuotaTableUtil.createPutForSnapshotSize(tn, snapshotName, 3l)); + t.put(QuotaTableUtil.createPutForSnapshotSize(tn, snapshotName, 5l)); + assertEquals(1, QuotaTableUtil.getObservedSnapshotSizes(connection).size()); + + List deletes = QuotaTableUtil.createDeletesForExistingTableSnapshotSizes(connection); + assertEquals(1, deletes.size()); + + t.delete(deletes); + assertEquals(0, QuotaTableUtil.getObservedSnapshotSizes(connection).size()); + + String ns = name.getMethodName(); + t.put(QuotaTableUtil.createPutForNamespaceSnapshotSize(ns, 3l)); + t.put(QuotaTableUtil.createPutForNamespaceSnapshotSize(ns, 3l)); + assertEquals(3l, QuotaTableUtil.getNamespaceSnapshotSize(connection, ns)); + + deletes = QuotaTableUtil.createDeletesForExistingNamespaceSnapshotSizes(connection); + assertEquals(1, deletes.size()); + + t.delete(deletes); + assertEquals(0l, QuotaTableUtil.getNamespaceSnapshotSize(connection, ns)); + } + } + @Test public void testTableQuotaUtil() throws Exception { final TableName tableName = TableName.valueOf(name.getMethodName()); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSnapshotQuotaObserverChore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSnapshotQuotaObserverChore.java index bddfe261a1e068e654afe6a949d790bef1a1a209..b9bd8a38dac938c70e9ee06dd167452193e6b9c0 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSnapshotQuotaObserverChore.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSnapshotQuotaObserverChore.java @@ -323,6 +323,86 @@ public class TestSnapshotQuotaObserverChore { } } + @Test + public void testRemovedSnapshots() throws Exception { + // Create a table and set a quota + TableName tn1 = helper.createTableWithRegions(1); + admin.setQuota(QuotaSettingsFactory.limitTableSpace(tn1, SpaceQuotaHelperForTests.ONE_GIGABYTE, + SpaceViolationPolicy.NO_INSERTS)); + + // Write some data and flush it + helper.writeData(tn1, 256L * SpaceQuotaHelperForTests.ONE_KILOBYTE); // 256 KB + + final AtomicReference lastSeenSize = new AtomicReference<>(); + // Wait for the Master chore to run to see the usage (with a fudge factor) + TEST_UTIL.waitFor(30_000, new SpaceQuotaSnapshotPredicate(conn, tn1) { + @Override boolean evaluate(SpaceQuotaSnapshot snapshot) throws Exception { + lastSeenSize.set(snapshot.getUsage()); + return snapshot.getUsage() > 230L * SpaceQuotaHelperForTests.ONE_KILOBYTE; + } + }); + + // Create a snapshot on the table + final String snapshotName1 = tn1 + "snapshot1"; + admin.snapshot(new SnapshotDescription(snapshotName1, tn1, SnapshotType.SKIPFLUSH)); + + // Snapshot size has to be 0 as the snapshot shares the data with the table + final Table quotaTable = conn.getTable(QuotaUtil.QUOTA_TABLE_NAME); + TEST_UTIL.waitFor(30_000, new Predicate() { + @Override public boolean evaluate() throws Exception { + Get g = QuotaTableUtil.makeGetForSnapshotSize(tn1, snapshotName1); + Result r = quotaTable.get(g); + if (r == null || r.isEmpty()) { + return false; + } + r.advance(); + Cell c = r.current(); + return QuotaTableUtil.parseSnapshotSize(c) == 0; + } + }); + // Total usage has to remain same as what we saw before taking a snapshot + TEST_UTIL.waitFor(30_000, new SpaceQuotaSnapshotPredicate(conn, tn1) { + @Override boolean evaluate(SpaceQuotaSnapshot snapshot) throws Exception { + return snapshot.getUsage() == lastSeenSize.get(); + } + }); + + // Major compact the table to force a rewrite + TEST_UTIL.compact(tn1, true); + // Now the snapshot size has to prev total size + TEST_UTIL.waitFor(30_000, new Predicate() { + @Override public boolean evaluate() throws Exception { + Get g = QuotaTableUtil.makeGetForSnapshotSize(tn1, snapshotName1); + Result r = quotaTable.get(g); + if (r == null || r.isEmpty()) { + return false; + } + r.advance(); + Cell c = r.current(); + // The compaction result file has an additional compaction event tracker + return lastSeenSize.get() == QuotaTableUtil.parseSnapshotSize(c); + } + }); + // The total size now has to be equal/more than double of prev total size + // as double the number of store files exist now. + final AtomicReference sizeAfterCompaction = new AtomicReference<>(); + TEST_UTIL.waitFor(30_000, new SpaceQuotaSnapshotPredicate(conn, tn1) { + @Override boolean evaluate(SpaceQuotaSnapshot snapshot) throws Exception { + sizeAfterCompaction.set(snapshot.getUsage()); + return snapshot.getUsage() >= 2 * lastSeenSize.get(); + } + }); + + // Delete the snapshot + admin.deleteSnapshot(snapshotName1); + // Total size has to come down to prev totalsize - snapshot size(which was removed) + TEST_UTIL.waitFor(30_000, new SpaceQuotaSnapshotPredicate(conn, tn1) { + @Override boolean evaluate(SpaceQuotaSnapshot snapshot) throws Exception { + return snapshot.getUsage() == (sizeAfterCompaction.get() - lastSeenSize.get()); + } + }); + } + @Test public void testBucketingFilesToSnapshots() throws Exception { // Create a table and set a quota -- 2.17.2 (Apple Git-113)