From 7af5b30eca89591e6c03319d356c690e72f531ce Mon Sep 17 00:00:00 2001 From: shardul-cr7 Date: Wed, 21 Aug 2019 00:41:11 -0700 Subject: [PATCH] HBASE-22013 SpaceQuotas - getNumRegions() returning wrong number of regions due to region replicas Signed-off-by: Sakthi --- .../hbase/quotas/QuotaObserverChore.java | 4 ++ .../quotas/SpaceQuotaHelperForTests.java | 19 +++++--- .../hadoop/hbase/quotas/TestSpaceQuotas.java | 47 +++++++++++++++++++ 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaObserverChore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaObserverChore.java index 92a149c070ea..2b76cfdc236d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaObserverChore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaObserverChore.java @@ -32,6 +32,7 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.client.RegionReplicaUtil; import org.apache.hadoop.hbase.client.Scan; import org.apache.hadoop.hbase.master.HMaster; import org.apache.hadoop.hbase.master.MetricsMaster; @@ -764,6 +765,9 @@ int getNumRegions(TableName table) throws IOException { List regions = this.conn.getAdmin().getRegions(table); if (regions == null) { return 0; + } else { + // Filter the region replicas if any and return the original number of regions for a table. + RegionReplicaUtil.removeNonDefaultRegions(regions); } return regions.size(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/SpaceQuotaHelperForTests.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/SpaceQuotaHelperForTests.java index 84463b3bfeed..4c8b7db8e3b3 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/SpaceQuotaHelperForTests.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/SpaceQuotaHelperForTests.java @@ -306,15 +306,16 @@ TableName createTableWithRegions(int numRegions) throws Exception { } TableName createTableWithRegions(Admin admin, int numRegions) throws Exception { - return createTableWithRegions( - testUtil.getAdmin(), NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, numRegions); + return createTableWithRegions(admin, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, numRegions, + 0); } TableName createTableWithRegions(String namespace, int numRegions) throws Exception { - return createTableWithRegions(testUtil.getAdmin(), namespace, numRegions); + return createTableWithRegions(testUtil.getAdmin(), namespace, numRegions, 0); } - TableName createTableWithRegions(Admin admin, String namespace, int numRegions) throws Exception { + TableName createTableWithRegions(Admin admin, String namespace, int numRegions, + int numberOfReplicas) throws Exception { final TableName tn = getNextTableName(namespace); // Delete the old table @@ -324,8 +325,14 @@ TableName createTableWithRegions(Admin admin, String namespace, int numRegions) } // Create the table - TableDescriptor tableDesc = TableDescriptorBuilder.newBuilder(tn) - .setColumnFamily(ColumnFamilyDescriptorBuilder.of(F1)).build(); + TableDescriptor tableDesc; + if (numberOfReplicas > 0) { + tableDesc = TableDescriptorBuilder.newBuilder(tn).setRegionReplication(numberOfReplicas) + .setColumnFamily(ColumnFamilyDescriptorBuilder.of(F1)).build(); + } else { + tableDesc = TableDescriptorBuilder.newBuilder(tn) + .setColumnFamily(ColumnFamilyDescriptorBuilder.of(F1)).build(); + } if (numRegions == 1) { admin.createTable(tableDesc); } else { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotas.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotas.java index fca54535a279..5e96d763cab8 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotas.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotas.java @@ -38,6 +38,7 @@ import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.NamespaceDescriptor; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.TableNotEnabledException; import org.apache.hadoop.hbase.client.Admin; @@ -485,6 +486,52 @@ public void testSetQuotaOnNonExistingTableWithDisable() throws Exception { setQuotaLimit(NON_EXISTENT_TABLE, SpaceViolationPolicy.DISABLE, 2L); } + @Test + public void testSetQuotaWithRegionReplicaSingleRegion() throws Exception { + setQuotaAndVerfiyForRegionReplication(1, 2, SpaceViolationPolicy.NO_INSERTS); + setQuotaAndVerfiyForRegionReplication(1, 2, SpaceViolationPolicy.NO_WRITES); + setQuotaAndVerfiyForRegionReplication(1, 2, SpaceViolationPolicy.NO_WRITES_COMPACTIONS); + setQuotaAndVerfiyForRegionReplication(1, 2, SpaceViolationPolicy.DISABLE); + } + + @Test + public void testSetQuotaWithRegionReplicaMultipleRegion() throws Exception { + setQuotaAndVerfiyForRegionReplication(5, 3, SpaceViolationPolicy.NO_INSERTS); + setQuotaAndVerfiyForRegionReplication(6, 3, SpaceViolationPolicy.NO_WRITES); + setQuotaAndVerfiyForRegionReplication(6, 3, SpaceViolationPolicy.NO_WRITES_COMPACTIONS); + setQuotaAndVerfiyForRegionReplication(6, 3, SpaceViolationPolicy.DISABLE); + } + + @Test + public void testSetQuotaWithSingleRegionZeroRegionReplica() throws Exception { + setQuotaAndVerfiyForRegionReplication(1, 0, SpaceViolationPolicy.NO_INSERTS); + setQuotaAndVerfiyForRegionReplication(1, 0, SpaceViolationPolicy.NO_WRITES); + setQuotaAndVerfiyForRegionReplication(1, 0, SpaceViolationPolicy.NO_WRITES_COMPACTIONS); + setQuotaAndVerfiyForRegionReplication(1, 0, SpaceViolationPolicy.DISABLE); + } + + @Test + public void testSetQuotaWithMultipleRegionZeroRegionReplicas() throws Exception { + setQuotaAndVerfiyForRegionReplication(5, 0, SpaceViolationPolicy.NO_INSERTS); + setQuotaAndVerfiyForRegionReplication(6, 0, SpaceViolationPolicy.NO_WRITES); + setQuotaAndVerfiyForRegionReplication(6, 0, SpaceViolationPolicy.NO_WRITES_COMPACTIONS); + setQuotaAndVerfiyForRegionReplication(6, 0, SpaceViolationPolicy.DISABLE); + } + + private void setQuotaAndVerfiyForRegionReplication(int region, int replicatedRegion, + SpaceViolationPolicy policy) throws Exception { + TableName tn = helper.createTableWithRegions(TEST_UTIL.getAdmin(), + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, region, replicatedRegion); + setQuotaLimit(tn, policy, 5L); + helper.writeData(tn, 5L * SpaceQuotaHelperForTests.ONE_MEGABYTE); + Put p = new Put(Bytes.toBytes("to_reject")); + p.addColumn(Bytes.toBytes(SpaceQuotaHelperForTests.F1), Bytes.toBytes("to"), + Bytes.toBytes("reject")); + // Adding a sleep for 5 sec, so all the chores run and to void flakiness of the test. + Thread.sleep(5000); + verifyViolation(policy, tn, p); + } + public void setQuotaAndViolateNextSwitchPoliciesAndValidate(SpaceViolationPolicy policy1, SpaceViolationPolicy policy2) throws Exception { Put put = new Put(Bytes.toBytes("to_reject"));