From a247c98231a577faf3106cc28f2d8b872d504abf Mon Sep 17 00:00:00 2001 From: Charles Connell Date: Tue, 9 Jul 2024 13:20:22 -0400 Subject: [PATCH 1/4] Use existing Connection in all cases with QuotaRetriever --- .../hadoop/hbase/quotas/QuotaRetriever.java | 35 +++++-------------- .../quotas/SnapshotQuotaObserverChore.java | 2 +- .../resources/hbase-webapps/master/quotas.jsp | 3 +- .../quotas/SpaceQuotaHelperForTests.java | 8 ++--- .../quotas/TestMasterQuotasObserver.java | 4 +-- .../hadoop/hbase/quotas/TestQuotaAdmin.java | 22 ++++++------ 6 files changed, 27 insertions(+), 47 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaRetriever.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaRetriever.java index 1dd5bf275bba..9bea6bf2f77a 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaRetriever.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaRetriever.java @@ -23,9 +23,7 @@ import java.util.Iterator; import java.util.Objects; import java.util.Queue; -import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.client.Connection; -import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.ResultScanner; import org.apache.hadoop.hbase.client.Scan; @@ -52,21 +50,9 @@ public class QuotaRetriever implements Closeable, Iterable { private Connection connection; private Table table; - /** - * Should QutoaRetriever manage the state of the connection, or leave it be. - */ - private boolean isManagedConnection = false; - QuotaRetriever() { } - void init(final Configuration conf, final Scan scan) throws IOException { - // Set this before creating the connection and passing it down to make sure - // it's cleaned up if we fail to construct the Scanner. - this.isManagedConnection = true; - init(ConnectionFactory.createConnection(conf), scan); - } - void init(final Connection conn, final Scan scan) throws IOException { this.connection = Objects.requireNonNull(conn); this.table = this.connection.getTable(QuotaTableUtil.QUOTA_TABLE_NAME); @@ -88,13 +74,8 @@ public void close() throws IOException { this.table.close(); this.table = null; } - // Null out the connection on close() even if we didn't explicitly close it + // Null out the connection on close() even though we don't explicitly close it // to maintain typical semantics. - if (isManagedConnection) { - if (this.connection != null) { - this.connection.close(); - } - } this.connection = null; } @@ -156,26 +137,26 @@ public void remove() { /** * Open a QuotaRetriever with no filter, all the quota settings will be returned. - * @param conf Configuration object to use. + * @param conn Connection object to use. * @return the QuotaRetriever * @throws IOException if a remote or network exception occurs */ - public static QuotaRetriever open(final Configuration conf) throws IOException { - return open(conf, null); + public static QuotaRetriever open(final Connection conn) throws IOException { + return open(conn, null); } /** - * Open a QuotaRetriever with the specified filter. - * @param conf Configuration object to use. + * Open a QuotaRetriever with the specified filter using an existing Connection + * @param conn Connection object to use. * @param filter the QuotaFilter * @return the QuotaRetriever * @throws IOException if a remote or network exception occurs */ - public static QuotaRetriever open(final Configuration conf, final QuotaFilter filter) + public static QuotaRetriever open(final Connection conn, final QuotaFilter filter) throws IOException { Scan scan = QuotaTableUtil.makeScan(filter); QuotaRetriever scanner = new QuotaRetriever(); - scanner.init(conf, scan); + scanner.init(conn, scan); return scanner; } } 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 cfdcb52db71c..3c9d1d55fa0d 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 @@ -168,7 +168,7 @@ Multimap getSnapshotsToComputeSize() throws IOException { filter.addTypeFilter(QuotaType.SPACE); try (Admin admin = conn.getAdmin()) { // Pull all of the tables that have quotas (direct, or from namespace) - for (QuotaSettings qs : QuotaRetriever.open(conf, filter)) { + for (QuotaSettings qs : QuotaRetriever.open(conn, filter)) { if (qs.getQuotaType() == QuotaType.SPACE) { String ns = qs.getNamespace(); TableName tn = qs.getTableName(); diff --git a/hbase-server/src/main/resources/hbase-webapps/master/quotas.jsp b/hbase-server/src/main/resources/hbase-webapps/master/quotas.jsp index 780a8d4b3605..8a92eab1edc5 100644 --- a/hbase-server/src/main/resources/hbase-webapps/master/quotas.jsp +++ b/hbase-server/src/main/resources/hbase-webapps/master/quotas.jsp @@ -30,7 +30,6 @@ %> <% HMaster master = (HMaster) getServletContext().getAttribute(HMaster.MASTER); - Configuration conf = master.getConfiguration(); pageContext.setAttribute("pageTitle", "HBase Master Quotas: " + master.getServerName()); List regionServerThrottles = new ArrayList<>(); List namespaceThrottles = new ArrayList<>(); @@ -39,7 +38,7 @@ boolean exceedThrottleQuotaEnabled = false; if (quotaManager != null) { exceedThrottleQuotaEnabled = quotaManager.isExceedThrottleQuotaEnabled(); - try (QuotaRetriever scanner = QuotaRetriever.open(conf, null)) { + try (QuotaRetriever scanner = QuotaRetriever.open(master.getConnection(), null)) { for (QuotaSettings quota : scanner) { if (quota instanceof ThrottleSettings) { ThrottleSettings throttle = (ThrottleSettings) quota; 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 0dda78d26d34..bb9008ce7558 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 @@ -120,7 +120,7 @@ static void updateConfigForQuotas(Configuration conf) { * Returns the number of quotas defined in the HBase quota table. */ long listNumDefinedQuotas(Connection conn) throws IOException { - QuotaRetriever scanner = QuotaRetriever.open(conn.getConfiguration()); + QuotaRetriever scanner = QuotaRetriever.open(conn); try { return Iterables.size(scanner); } finally { @@ -353,7 +353,7 @@ void removeAllQuotas(Connection conn) throws IOException { waitForQuotaTable(conn); } else { // Or, clean up any quotas from previous test runs. - QuotaRetriever scanner = QuotaRetriever.open(conn.getConfiguration()); + QuotaRetriever scanner = QuotaRetriever.open(conn); try { for (QuotaSettings quotaSettings : scanner) { final String namespace = quotaSettings.getNamespace(); @@ -379,8 +379,8 @@ void removeAllQuotas(Connection conn) throws IOException { } QuotaSettings getTableSpaceQuota(Connection conn, TableName tn) throws IOException { - try (QuotaRetriever scanner = QuotaRetriever.open(conn.getConfiguration(), - new QuotaFilter().setTableFilter(tn.getNameAsString()))) { + try (QuotaRetriever scanner = + QuotaRetriever.open(conn, new QuotaFilter().setTableFilter(tn.getNameAsString()))) { for (QuotaSettings setting : scanner) { if (setting.getTableName().equals(tn) && setting.getQuotaType() == QuotaType.SPACE) { return setting; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterQuotasObserver.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterQuotasObserver.java index 0f01a43355b0..57dac7d60da7 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterQuotasObserver.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterQuotasObserver.java @@ -327,7 +327,7 @@ public boolean namespaceExists(String ns) throws IOException { } public int getNumSpaceQuotas() throws Exception { - QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration()); + QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConnection()); int numSpaceQuotas = 0; for (QuotaSettings quotaSettings : scanner) { if (quotaSettings.getQuotaType() == QuotaType.SPACE) { @@ -338,7 +338,7 @@ public int getNumSpaceQuotas() throws Exception { } public int getThrottleQuotas() throws Exception { - QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration()); + QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConnection()); int throttleQuotas = 0; for (QuotaSettings quotaSettings : scanner) { if (quotaSettings.getQuotaType() == QuotaType.THROTTLE) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaAdmin.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaAdmin.java index 817f135f0c95..2f340bd09e5b 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaAdmin.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaAdmin.java @@ -123,7 +123,7 @@ public void testThrottleType() throws Exception { QuotaSettingsFactory.throttleUser(userName, ThrottleType.WRITE_NUMBER, 12, TimeUnit.MINUTES)); admin.setQuota(QuotaSettingsFactory.bypassGlobals(userName, true)); - try (QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration())) { + try (QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConnection())) { int countThrottle = 0; int countGlobalBypass = 0; for (QuotaSettings settings : scanner) { @@ -169,7 +169,7 @@ public void testSimpleScan() throws Exception { TimeUnit.MINUTES)); admin.setQuota(QuotaSettingsFactory.bypassGlobals(userName, true)); - try (QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration())) { + try (QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConnection())) { int countThrottle = 0; int countGlobalBypass = 0; for (QuotaSettings settings : scanner) { @@ -345,7 +345,7 @@ public void testSetGetRemoveSpaceQuota() throws Exception { } // Verify we can retrieve it via the QuotaRetriever API - QuotaRetriever scanner = QuotaRetriever.open(admin.getConfiguration()); + QuotaRetriever scanner = QuotaRetriever.open(admin.getConnection()); try { assertSpaceQuota(sizeLimit, violationPolicy, Iterables.getOnlyElement(scanner)); } finally { @@ -367,7 +367,7 @@ public void testSetGetRemoveSpaceQuota() throws Exception { } // Verify that we can also not fetch it via the API - scanner = QuotaRetriever.open(admin.getConfiguration()); + scanner = QuotaRetriever.open(admin.getConnection()); try { assertNull("Did not expect to find a quota entry", scanner.next()); } finally { @@ -399,7 +399,7 @@ public void testSetModifyRemoveSpaceQuota() throws Exception { } // Verify we can retrieve it via the QuotaRetriever API - QuotaRetriever quotaScanner = QuotaRetriever.open(admin.getConfiguration()); + QuotaRetriever quotaScanner = QuotaRetriever.open(admin.getConnection()); try { assertSpaceQuota(originalSizeLimit, violationPolicy, Iterables.getOnlyElement(quotaScanner)); } finally { @@ -427,7 +427,7 @@ public void testSetModifyRemoveSpaceQuota() throws Exception { } // Verify we can retrieve the new quota via the QuotaRetriever API - quotaScanner = QuotaRetriever.open(admin.getConfiguration()); + quotaScanner = QuotaRetriever.open(admin.getConnection()); try { assertSpaceQuota(newSizeLimit, newViolationPolicy, Iterables.getOnlyElement(quotaScanner)); } finally { @@ -449,7 +449,7 @@ public void testSetModifyRemoveSpaceQuota() throws Exception { } // Verify that we can also not fetch it via the API - quotaScanner = QuotaRetriever.open(admin.getConfiguration()); + quotaScanner = QuotaRetriever.open(admin.getConnection()); try { assertNull("Did not expect to find a quota entry", quotaScanner.next()); } finally { @@ -549,7 +549,7 @@ public void testSetAndRemoveRegionServerQuota() throws Exception { admin.setQuota(QuotaSettingsFactory.throttleRegionServer(regionServer, ThrottleType.READ_NUMBER, 30, TimeUnit.SECONDS)); int count = 0; - QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration(), rsFilter); + QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConnection(), rsFilter); try { for (QuotaSettings settings : scanner) { assertTrue(settings.getQuotaType() == QuotaType.THROTTLE); @@ -733,14 +733,14 @@ private void verifyRecordNotPresentInQuotaTable() throws Exception { private void verifyFetchableViaAPI(Admin admin, ThrottleType type, long limit, TimeUnit tu) throws Exception { // Verify we can retrieve the new quota via the QuotaRetriever API - try (QuotaRetriever quotaScanner = QuotaRetriever.open(admin.getConfiguration())) { + try (QuotaRetriever quotaScanner = QuotaRetriever.open(admin.getConnection())) { assertRPCQuota(type, limit, tu, Iterables.getOnlyElement(quotaScanner)); } } private void verifyNotFetchableViaAPI(Admin admin) throws Exception { // Verify that we can also not fetch it via the API - try (QuotaRetriever quotaScanner = QuotaRetriever.open(admin.getConfiguration())) { + try (QuotaRetriever quotaScanner = QuotaRetriever.open(admin.getConnection())) { assertNull("Did not expect to find a quota entry", quotaScanner.next()); } } @@ -830,7 +830,7 @@ private void assertSpaceQuota(long sizeLimit, SpaceViolationPolicy violationPoli } private int countResults(final QuotaFilter filter) throws Exception { - QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration(), filter); + QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConnection(), filter); try { int count = 0; for (QuotaSettings settings : scanner) { From dd42a14f81665840b7a85d7da7684569178c109f Mon Sep 17 00:00:00 2001 From: Charles Connell Date: Wed, 17 Jul 2024 10:20:54 -0400 Subject: [PATCH 2/4] Bring back public methods --- .../hadoop/hbase/quotas/QuotaRetriever.java | 61 ++++++++++++++++--- .../hbase/quotas/QuotaObserverChore.java | 3 +- 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaRetriever.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaRetriever.java index 9bea6bf2f77a..4f8074622f14 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaRetriever.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaRetriever.java @@ -23,7 +23,9 @@ import java.util.Iterator; import java.util.Objects; import java.util.Queue; +import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.client.Connection; +import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.ResultScanner; import org.apache.hadoop.hbase.client.Scan; @@ -50,10 +52,24 @@ public class QuotaRetriever implements Closeable, Iterable { private Connection connection; private Table table; - QuotaRetriever() { + /** + * Should QutoaRetriever manage the state of the connection, or leave it be. + */ + private final boolean isManagedConnection; + + QuotaRetriever(final Connection conn, final Scan scan) throws IOException { + isManagedConnection = false; + init(conn, scan); } - void init(final Connection conn, final Scan scan) throws IOException { + QuotaRetriever(final Configuration conf, final Scan scan) throws IOException { + // Set this before creating the connection and passing it down to make sure + // it's cleaned up if we fail to construct the Scanner. + isManagedConnection = true; + init(ConnectionFactory.createConnection(conf), scan); + } + + private void init(final Connection conn, final Scan scan) throws IOException { this.connection = Objects.requireNonNull(conn); this.table = this.connection.getTable(QuotaTableUtil.QUOTA_TABLE_NAME); try { @@ -74,8 +90,13 @@ public void close() throws IOException { this.table.close(); this.table = null; } - // Null out the connection on close() even though we don't explicitly close it + // Null out the connection on close() even if we didn't explicitly close it // to maintain typical semantics. + if (isManagedConnection) { + if (this.connection != null) { + this.connection.close(); + } + } this.connection = null; } @@ -135,6 +156,18 @@ public void remove() { } } + /** + * Open a QuotaRetriever with no filter, all the quota settings will be returned. + * @param conf Configuration object to use. + * @return the QuotaRetriever + * @throws IOException if a remote or network exception occurs + * @deprecated Since 3.0.0, will be removed in 4.0.0. Use {@link #open(Connection)} instead. + */ + @Deprecated + public static QuotaRetriever open(final Configuration conf) throws IOException { + return open(conf, null); + } + /** * Open a QuotaRetriever with no filter, all the quota settings will be returned. * @param conn Connection object to use. @@ -146,7 +179,23 @@ public static QuotaRetriever open(final Connection conn) throws IOException { } /** - * Open a QuotaRetriever with the specified filter using an existing Connection + * Open a QuotaRetriever with the specified filter. + * @param conf Configuration object to use. + * @param filter the QuotaFilter + * @return the QuotaRetriever + * @throws IOException if a remote or network exception occurs + * @deprecated Since 3.0.0, will be removed in 4.0.0. Use {@link #open(Connection, QuotaFilter)} + * instead. + */ + @Deprecated + public static QuotaRetriever open(final Configuration conf, final QuotaFilter filter) + throws IOException { + Scan scan = QuotaTableUtil.makeScan(filter); + return new QuotaRetriever(conf, scan); + } + + /** + * Open a QuotaRetriever with the specified filter. * @param conn Connection object to use. * @param filter the QuotaFilter * @return the QuotaRetriever @@ -155,8 +204,6 @@ public static QuotaRetriever open(final Connection conn) throws IOException { public static QuotaRetriever open(final Connection conn, final QuotaFilter filter) throws IOException { Scan scan = QuotaTableUtil.makeScan(filter); - QuotaRetriever scanner = new QuotaRetriever(); - scanner.init(conn, scan); - return scanner; + return new QuotaRetriever(conn, scan); } } 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 32bfcebcb9e4..a89db895cb4c 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 @@ -475,8 +475,7 @@ void pruneOldRegionReports() { TablesWithQuotas fetchAllTablesWithQuotasDefined() throws IOException { final Scan scan = QuotaTableUtil.makeScan(null); final TablesWithQuotas tablesWithQuotas = new TablesWithQuotas(conn, conf); - try (final QuotaRetriever scanner = new QuotaRetriever()) { - scanner.init(conn, scan); + try (final QuotaRetriever scanner = new QuotaRetriever(conn, scan)) { for (QuotaSettings quotaSettings : scanner) { // Only one of namespace and tablename should be 'null' final String namespace = quotaSettings.getNamespace(); From 80dd227a1319ecd8412ab0e546f5370b2951dabb Mon Sep 17 00:00:00 2001 From: Charles Connell Date: Thu, 18 Jul 2024 08:05:15 -0400 Subject: [PATCH 3/4] Expose QuotaRetriever constructor, use it via try-with-resources --- .../hadoop/hbase/quotas/QuotaRetriever.java | 39 ++++++----------- .../quotas/SnapshotQuotaObserverChore.java | 28 ++++++------ .../resources/hbase-webapps/master/quotas.jsp | 2 +- .../quotas/SpaceQuotaHelperForTests.java | 16 ++----- .../quotas/TestMasterQuotasObserver.java | 26 +++++------ .../hadoop/hbase/quotas/TestQuotaAdmin.java | 43 +++++-------------- 6 files changed, 57 insertions(+), 97 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaRetriever.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaRetriever.java index 4f8074622f14..1d902ff9b718 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaRetriever.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaRetriever.java @@ -57,7 +57,15 @@ public class QuotaRetriever implements Closeable, Iterable { */ private final boolean isManagedConnection; - QuotaRetriever(final Connection conn, final Scan scan) throws IOException { + public QuotaRetriever(final Connection conn) throws IOException { + this(conn, (QuotaFilter) null); + } + + public QuotaRetriever(final Connection conn, final QuotaFilter filter) throws IOException { + this(conn, QuotaTableUtil.makeScan(filter)); + } + + public QuotaRetriever(final Connection conn, final Scan scan) throws IOException { isManagedConnection = false; init(conn, scan); } @@ -161,31 +169,22 @@ public void remove() { * @param conf Configuration object to use. * @return the QuotaRetriever * @throws IOException if a remote or network exception occurs - * @deprecated Since 3.0.0, will be removed in 4.0.0. Use {@link #open(Connection)} instead. + * @deprecated Since 3.0.0, will be removed in 4.0.0. Use + * {@link #QuotaRetriever(Configuration, Scan)} instead. */ @Deprecated public static QuotaRetriever open(final Configuration conf) throws IOException { return open(conf, null); } - /** - * Open a QuotaRetriever with no filter, all the quota settings will be returned. - * @param conn Connection object to use. - * @return the QuotaRetriever - * @throws IOException if a remote or network exception occurs - */ - public static QuotaRetriever open(final Connection conn) throws IOException { - return open(conn, null); - } - /** * Open a QuotaRetriever with the specified filter. * @param conf Configuration object to use. * @param filter the QuotaFilter * @return the QuotaRetriever * @throws IOException if a remote or network exception occurs - * @deprecated Since 3.0.0, will be removed in 4.0.0. Use {@link #open(Connection, QuotaFilter)} - * instead. + * @deprecated Since 3.0.0, will be removed in 4.0.0. Use + * {@link #QuotaRetriever(Configuration, Scan)} instead. */ @Deprecated public static QuotaRetriever open(final Configuration conf, final QuotaFilter filter) @@ -194,16 +193,4 @@ public static QuotaRetriever open(final Configuration conf, final QuotaFilter fi return new QuotaRetriever(conf, scan); } - /** - * Open a QuotaRetriever with the specified filter. - * @param conn Connection object to use. - * @param filter the QuotaFilter - * @return the QuotaRetriever - * @throws IOException if a remote or network exception occurs - */ - public static QuotaRetriever open(final Connection conn, final QuotaFilter filter) - throws IOException { - Scan scan = QuotaTableUtil.makeScan(filter); - return new QuotaRetriever(conn, scan); - } } 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 3c9d1d55fa0d..2365f26fc375 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 @@ -168,19 +168,21 @@ Multimap getSnapshotsToComputeSize() throws IOException { filter.addTypeFilter(QuotaType.SPACE); try (Admin admin = conn.getAdmin()) { // Pull all of the tables that have quotas (direct, or from namespace) - for (QuotaSettings qs : QuotaRetriever.open(conn, filter)) { - if (qs.getQuotaType() == QuotaType.SPACE) { - String ns = qs.getNamespace(); - TableName tn = qs.getTableName(); - if ((null == ns && null == tn) || (null != ns && null != tn)) { - throw new IllegalStateException( - "Expected either one of namespace and tablename to be null but not both"); - } - // Collect either the table name itself, or all of the tables in the namespace - if (null != ns) { - tablesToFetchSnapshotsFrom.addAll(Arrays.asList(admin.listTableNamesByNamespace(ns))); - } else { - tablesToFetchSnapshotsFrom.add(tn); + try (QuotaRetriever qr = new QuotaRetriever(conn, filter)) { + for (QuotaSettings qs : qr) { + if (qs.getQuotaType() == QuotaType.SPACE) { + String ns = qs.getNamespace(); + TableName tn = qs.getTableName(); + if ((null == ns && null == tn) || (null != ns && null != tn)) { + throw new IllegalStateException( + "Expected either one of namespace and tablename to be null but not both"); + } + // Collect either the table name itself, or all of the tables in the namespace + if (null != ns) { + tablesToFetchSnapshotsFrom.addAll(Arrays.asList(admin.listTableNamesByNamespace(ns))); + } else { + tablesToFetchSnapshotsFrom.add(tn); + } } } } diff --git a/hbase-server/src/main/resources/hbase-webapps/master/quotas.jsp b/hbase-server/src/main/resources/hbase-webapps/master/quotas.jsp index 8a92eab1edc5..a52085b529f9 100644 --- a/hbase-server/src/main/resources/hbase-webapps/master/quotas.jsp +++ b/hbase-server/src/main/resources/hbase-webapps/master/quotas.jsp @@ -38,7 +38,7 @@ boolean exceedThrottleQuotaEnabled = false; if (quotaManager != null) { exceedThrottleQuotaEnabled = quotaManager.isExceedThrottleQuotaEnabled(); - try (QuotaRetriever scanner = QuotaRetriever.open(master.getConnection(), null)) { + try (QuotaRetriever scanner = new QuotaRetriever(master.getConnection())) { for (QuotaSettings quota : scanner) { if (quota instanceof ThrottleSettings) { ThrottleSettings throttle = (ThrottleSettings) quota; 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 bb9008ce7558..5c29748bf143 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 @@ -120,13 +120,8 @@ static void updateConfigForQuotas(Configuration conf) { * Returns the number of quotas defined in the HBase quota table. */ long listNumDefinedQuotas(Connection conn) throws IOException { - QuotaRetriever scanner = QuotaRetriever.open(conn); - try { + try (QuotaRetriever scanner = new QuotaRetriever(conn)) { return Iterables.size(scanner); - } finally { - if (scanner != null) { - scanner.close(); - } } } @@ -353,8 +348,7 @@ void removeAllQuotas(Connection conn) throws IOException { waitForQuotaTable(conn); } else { // Or, clean up any quotas from previous test runs. - QuotaRetriever scanner = QuotaRetriever.open(conn); - try { + try (QuotaRetriever scanner = new QuotaRetriever(conn);) { for (QuotaSettings quotaSettings : scanner) { final String namespace = quotaSettings.getNamespace(); final TableName tableName = quotaSettings.getTableName(); @@ -370,17 +364,13 @@ void removeAllQuotas(Connection conn) throws IOException { QuotaUtil.deleteUserQuota(conn, userName); } } - } finally { - if (scanner != null) { - scanner.close(); - } } } } QuotaSettings getTableSpaceQuota(Connection conn, TableName tn) throws IOException { try (QuotaRetriever scanner = - QuotaRetriever.open(conn, new QuotaFilter().setTableFilter(tn.getNameAsString()))) { + new QuotaRetriever(conn, new QuotaFilter().setTableFilter(tn.getNameAsString()))) { for (QuotaSettings setting : scanner) { if (setting.getTableName().equals(tn) && setting.getQuotaType() == QuotaType.SPACE) { return setting; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterQuotasObserver.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterQuotasObserver.java index 57dac7d60da7..a5d879ce77e6 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterQuotasObserver.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterQuotasObserver.java @@ -327,25 +327,27 @@ public boolean namespaceExists(String ns) throws IOException { } public int getNumSpaceQuotas() throws Exception { - QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConnection()); - int numSpaceQuotas = 0; - for (QuotaSettings quotaSettings : scanner) { - if (quotaSettings.getQuotaType() == QuotaType.SPACE) { - numSpaceQuotas++; + try (QuotaRetriever scanner = new QuotaRetriever(TEST_UTIL.getConnection())) { + int numSpaceQuotas = 0; + for (QuotaSettings quotaSettings : scanner) { + if (quotaSettings.getQuotaType() == QuotaType.SPACE) { + numSpaceQuotas++; + } } + return numSpaceQuotas; } - return numSpaceQuotas; } public int getThrottleQuotas() throws Exception { - QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConnection()); - int throttleQuotas = 0; - for (QuotaSettings quotaSettings : scanner) { - if (quotaSettings.getQuotaType() == QuotaType.THROTTLE) { - throttleQuotas++; + try (QuotaRetriever scanner = new QuotaRetriever(TEST_UTIL.getConnection())) { + int throttleQuotas = 0; + for (QuotaSettings quotaSettings : scanner) { + if (quotaSettings.getQuotaType() == QuotaType.THROTTLE) { + throttleQuotas++; + } } + return throttleQuotas; } - return throttleQuotas; } private void createTable(Admin admin, TableName tn) throws Exception { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaAdmin.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaAdmin.java index 2f340bd09e5b..cd266fa0baac 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaAdmin.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaAdmin.java @@ -123,7 +123,7 @@ public void testThrottleType() throws Exception { QuotaSettingsFactory.throttleUser(userName, ThrottleType.WRITE_NUMBER, 12, TimeUnit.MINUTES)); admin.setQuota(QuotaSettingsFactory.bypassGlobals(userName, true)); - try (QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConnection())) { + try (QuotaRetriever scanner = new QuotaRetriever(TEST_UTIL.getConnection())) { int countThrottle = 0; int countGlobalBypass = 0; for (QuotaSettings settings : scanner) { @@ -169,7 +169,7 @@ public void testSimpleScan() throws Exception { TimeUnit.MINUTES)); admin.setQuota(QuotaSettingsFactory.bypassGlobals(userName, true)); - try (QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConnection())) { + try (QuotaRetriever scanner = new QuotaRetriever(TEST_UTIL.getConnection())) { int countThrottle = 0; int countGlobalBypass = 0; for (QuotaSettings settings : scanner) { @@ -345,11 +345,8 @@ public void testSetGetRemoveSpaceQuota() throws Exception { } // Verify we can retrieve it via the QuotaRetriever API - QuotaRetriever scanner = QuotaRetriever.open(admin.getConnection()); - try { + try (QuotaRetriever scanner = new QuotaRetriever(admin.getConnection())) { assertSpaceQuota(sizeLimit, violationPolicy, Iterables.getOnlyElement(scanner)); - } finally { - scanner.close(); } // Now, remove the quota @@ -367,11 +364,8 @@ public void testSetGetRemoveSpaceQuota() throws Exception { } // Verify that we can also not fetch it via the API - scanner = QuotaRetriever.open(admin.getConnection()); - try { + try (QuotaRetriever scanner = new QuotaRetriever(admin.getConnection())) { assertNull("Did not expect to find a quota entry", scanner.next()); - } finally { - scanner.close(); } } @@ -399,11 +393,8 @@ public void testSetModifyRemoveSpaceQuota() throws Exception { } // Verify we can retrieve it via the QuotaRetriever API - QuotaRetriever quotaScanner = QuotaRetriever.open(admin.getConnection()); - try { + try (QuotaRetriever quotaScanner = new QuotaRetriever(admin.getConnection())) { assertSpaceQuota(originalSizeLimit, violationPolicy, Iterables.getOnlyElement(quotaScanner)); - } finally { - quotaScanner.close(); } // Setting a new size and policy should be reflected @@ -427,11 +418,8 @@ public void testSetModifyRemoveSpaceQuota() throws Exception { } // Verify we can retrieve the new quota via the QuotaRetriever API - quotaScanner = QuotaRetriever.open(admin.getConnection()); - try { + try (QuotaRetriever quotaScanner = new QuotaRetriever(admin.getConnection())) { assertSpaceQuota(newSizeLimit, newViolationPolicy, Iterables.getOnlyElement(quotaScanner)); - } finally { - quotaScanner.close(); } // Now, remove the quota @@ -449,11 +437,8 @@ public void testSetModifyRemoveSpaceQuota() throws Exception { } // Verify that we can also not fetch it via the API - quotaScanner = QuotaRetriever.open(admin.getConnection()); - try { + try (QuotaRetriever quotaScanner = new QuotaRetriever(admin.getConnection())) { assertNull("Did not expect to find a quota entry", quotaScanner.next()); - } finally { - quotaScanner.close(); } } @@ -549,8 +534,7 @@ public void testSetAndRemoveRegionServerQuota() throws Exception { admin.setQuota(QuotaSettingsFactory.throttleRegionServer(regionServer, ThrottleType.READ_NUMBER, 30, TimeUnit.SECONDS)); int count = 0; - QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConnection(), rsFilter); - try { + try (QuotaRetriever scanner = new QuotaRetriever(TEST_UTIL.getConnection(), rsFilter)) { for (QuotaSettings settings : scanner) { assertTrue(settings.getQuotaType() == QuotaType.THROTTLE); ThrottleSettings throttleSettings = (ThrottleSettings) settings; @@ -564,8 +548,6 @@ public void testSetAndRemoveRegionServerQuota() throws Exception { assertEquals(TimeUnit.SECONDS, throttleSettings.getTimeUnit()); } } - } finally { - scanner.close(); } assertEquals(2, count); @@ -733,14 +715,14 @@ private void verifyRecordNotPresentInQuotaTable() throws Exception { private void verifyFetchableViaAPI(Admin admin, ThrottleType type, long limit, TimeUnit tu) throws Exception { // Verify we can retrieve the new quota via the QuotaRetriever API - try (QuotaRetriever quotaScanner = QuotaRetriever.open(admin.getConnection())) { + try (QuotaRetriever quotaScanner = new QuotaRetriever(admin.getConnection())) { assertRPCQuota(type, limit, tu, Iterables.getOnlyElement(quotaScanner)); } } private void verifyNotFetchableViaAPI(Admin admin) throws Exception { // Verify that we can also not fetch it via the API - try (QuotaRetriever quotaScanner = QuotaRetriever.open(admin.getConnection())) { + try (QuotaRetriever quotaScanner = new QuotaRetriever(admin.getConnection())) { assertNull("Did not expect to find a quota entry", quotaScanner.next()); } } @@ -830,16 +812,13 @@ private void assertSpaceQuota(long sizeLimit, SpaceViolationPolicy violationPoli } private int countResults(final QuotaFilter filter) throws Exception { - QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConnection(), filter); - try { + try (QuotaRetriever scanner = new QuotaRetriever(TEST_UTIL.getConnection(), filter)) { int count = 0; for (QuotaSettings settings : scanner) { LOG.debug(Objects.toString(settings)); count++; } return count; - } finally { - scanner.close(); } } From 6caf25e488254fa2a8899e034c3b72ef6d0ac8e5 Mon Sep 17 00:00:00 2001 From: Charles Connell Date: Thu, 18 Jul 2024 10:14:40 -0400 Subject: [PATCH 4/4] silly mistake --- .../quotas/SnapshotQuotaObserverChore.java | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) 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 2365f26fc375..c198f3d9d322 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 @@ -166,23 +166,21 @@ Multimap getSnapshotsToComputeSize() throws IOException { Set tablesToFetchSnapshotsFrom = new HashSet<>(); QuotaFilter filter = new QuotaFilter(); filter.addTypeFilter(QuotaType.SPACE); - try (Admin admin = conn.getAdmin()) { + try (Admin admin = conn.getAdmin(); QuotaRetriever qr = new QuotaRetriever(conn, filter)) { // Pull all of the tables that have quotas (direct, or from namespace) - try (QuotaRetriever qr = new QuotaRetriever(conn, filter)) { - for (QuotaSettings qs : qr) { - if (qs.getQuotaType() == QuotaType.SPACE) { - String ns = qs.getNamespace(); - TableName tn = qs.getTableName(); - if ((null == ns && null == tn) || (null != ns && null != tn)) { - throw new IllegalStateException( - "Expected either one of namespace and tablename to be null but not both"); - } - // Collect either the table name itself, or all of the tables in the namespace - if (null != ns) { - tablesToFetchSnapshotsFrom.addAll(Arrays.asList(admin.listTableNamesByNamespace(ns))); - } else { - tablesToFetchSnapshotsFrom.add(tn); - } + for (QuotaSettings qs : qr) { + if (qs.getQuotaType() == QuotaType.SPACE) { + String ns = qs.getNamespace(); + TableName tn = qs.getTableName(); + if ((null == ns && null == tn) || (null != ns && null != tn)) { + throw new IllegalStateException( + "Expected either one of namespace and tablename to be null but not both"); + } + // Collect either the table name itself, or all of the tables in the namespace + if (null != ns) { + tablesToFetchSnapshotsFrom.addAll(Arrays.asList(admin.listTableNamesByNamespace(ns))); + } else { + tablesToFetchSnapshotsFrom.add(tn); } } }