Skip to content

Commit db386cf

Browse files
ndimidukcharlesconnell
ndimiduk
and
charlesconnell
committed
HBASE-28716 Users of QuotaRetriever should pass an existing connection (#6065)
Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Pankaj Kumar <pankajkumar@apache.org> Co-authored-by: Charles Connell <cconnell@hubspot.com>
1 parent 2d83a8a commit db386cf

File tree

8 files changed

+60
-78
lines changed

8 files changed

+60
-78
lines changed

‎hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import java.util.Collections;
3030
import java.util.EnumSet;
3131
import java.util.HashMap;
32-
import java.util.Iterator;
3332
import java.util.List;
3433
import java.util.Map;
3534
import java.util.Set;
@@ -3153,16 +3152,15 @@ protected Void rpcCall() throws Exception {
31533152

31543153
@Override
31553154
public QuotaRetriever getQuotaRetriever(final QuotaFilter filter) throws IOException {
3156-
return QuotaRetriever.open(conf, filter);
3155+
return new QuotaRetriever(connection, filter);
31573156
}
31583157

31593158
@Override
31603159
public List<QuotaSettings> getQuota(QuotaFilter filter) throws IOException {
31613160
List<QuotaSettings> quotas = new ArrayList<>();
3162-
try (QuotaRetriever retriever = QuotaRetriever.open(conf, filter)) {
3163-
Iterator<QuotaSettings> iterator = retriever.iterator();
3164-
while (iterator.hasNext()) {
3165-
quotas.add(iterator.next());
3161+
try (QuotaRetriever retriever = new QuotaRetriever(connection, filter)) {
3162+
for (QuotaSettings quotaSettings : retriever) {
3163+
quotas.add(quotaSettings);
31663164
}
31673165
}
31683166
return quotas;

‎hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaRetriever.java

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,19 +55,29 @@ public class QuotaRetriever implements Closeable, Iterable<QuotaSettings> {
5555
/**
5656
* Should QutoaRetriever manage the state of the connection, or leave it be.
5757
*/
58-
private boolean isManagedConnection = false;
58+
private final boolean isManagedConnection;
5959

60-
QuotaRetriever() {
60+
public QuotaRetriever(final Connection conn) throws IOException {
61+
this(conn, (QuotaFilter) null);
6162
}
6263

63-
void init(final Configuration conf, final Scan scan) throws IOException {
64+
public QuotaRetriever(final Connection conn, final QuotaFilter filter) throws IOException {
65+
this(conn, QuotaTableUtil.makeScan(filter));
66+
}
67+
68+
public QuotaRetriever(final Connection conn, final Scan scan) throws IOException {
69+
isManagedConnection = false;
70+
init(conn, scan);
71+
}
72+
73+
QuotaRetriever(final Configuration conf, final Scan scan) throws IOException {
6474
// Set this before creating the connection and passing it down to make sure
6575
// it's cleaned up if we fail to construct the Scanner.
66-
this.isManagedConnection = true;
76+
isManagedConnection = true;
6777
init(ConnectionFactory.createConnection(conf), scan);
6878
}
6979

70-
void init(final Connection conn, final Scan scan) throws IOException {
80+
private void init(final Connection conn, final Scan scan) throws IOException {
7181
this.connection = Objects.requireNonNull(conn);
7282
this.table = this.connection.getTable(QuotaTableUtil.QUOTA_TABLE_NAME);
7383
try {
@@ -159,7 +169,10 @@ public void remove() {
159169
* @param conf Configuration object to use.
160170
* @return the QuotaRetriever
161171
* @throws IOException if a remote or network exception occurs
172+
* @deprecated Since 3.0.0, will be removed in 4.0.0. Use
173+
* {@link #QuotaRetriever(Configuration, Scan)} instead.
162174
*/
175+
@Deprecated
163176
public static QuotaRetriever open(final Configuration conf) throws IOException {
164177
return open(conf, null);
165178
}
@@ -170,12 +183,14 @@ public static QuotaRetriever open(final Configuration conf) throws IOException {
170183
* @param filter the QuotaFilter
171184
* @return the QuotaRetriever
172185
* @throws IOException if a remote or network exception occurs
186+
* @deprecated Since 3.0.0, will be removed in 4.0.0. Use
187+
* {@link #QuotaRetriever(Configuration, Scan)} instead.
173188
*/
189+
@Deprecated
174190
public static QuotaRetriever open(final Configuration conf, final QuotaFilter filter)
175191
throws IOException {
176192
Scan scan = QuotaTableUtil.makeScan(filter);
177-
QuotaRetriever scanner = new QuotaRetriever();
178-
scanner.init(conf, scan);
179-
return scanner;
193+
return new QuotaRetriever(conf, scan);
180194
}
195+
181196
}

‎hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaObserverChore.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -475,8 +475,7 @@ void pruneOldRegionReports() {
475475
TablesWithQuotas fetchAllTablesWithQuotasDefined() throws IOException {
476476
final Scan scan = QuotaTableUtil.makeScan(null);
477477
final TablesWithQuotas tablesWithQuotas = new TablesWithQuotas(conn, conf);
478-
try (final QuotaRetriever scanner = new QuotaRetriever()) {
479-
scanner.init(conn, scan);
478+
try (final QuotaRetriever scanner = new QuotaRetriever(conn, scan)) {
480479
for (QuotaSettings quotaSettings : scanner) {
481480
// Only one of namespace and tablename should be 'null'
482481
final String namespace = quotaSettings.getNamespace();

‎hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SnapshotQuotaObserverChore.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,9 @@ Multimap<TableName, String> getSnapshotsToComputeSize() throws IOException {
166166
Set<TableName> tablesToFetchSnapshotsFrom = new HashSet<>();
167167
QuotaFilter filter = new QuotaFilter();
168168
filter.addTypeFilter(QuotaType.SPACE);
169-
try (Admin admin = conn.getAdmin()) {
169+
try (Admin admin = conn.getAdmin(); QuotaRetriever qr = new QuotaRetriever(conn, filter)) {
170170
// Pull all of the tables that have quotas (direct, or from namespace)
171-
for (QuotaSettings qs : QuotaRetriever.open(conf, filter)) {
171+
for (QuotaSettings qs : qr) {
172172
if (qs.getQuotaType() == QuotaType.SPACE) {
173173
String ns = qs.getNamespace();
174174
TableName tn = qs.getTableName();

‎hbase-server/src/main/resources/hbase-webapps/master/quotas.jsp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
%>
3131
<%
3232
HMaster master = (HMaster) getServletContext().getAttribute(HMaster.MASTER);
33-
Configuration conf = master.getConfiguration();
3433
pageContext.setAttribute("pageTitle", "HBase Master Quotas: " + master.getServerName());
3534
List<ThrottleSettings> regionServerThrottles = new ArrayList<>();
3635
List<ThrottleSettings> namespaceThrottles = new ArrayList<>();
@@ -39,7 +38,7 @@
3938
boolean exceedThrottleQuotaEnabled = false;
4039
if (quotaManager != null) {
4140
exceedThrottleQuotaEnabled = quotaManager.isExceedThrottleQuotaEnabled();
42-
try (QuotaRetriever scanner = QuotaRetriever.open(conf, null)) {
41+
try (QuotaRetriever scanner = new QuotaRetriever(master.getConnection())) {
4342
for (QuotaSettings quota : scanner) {
4443
if (quota instanceof ThrottleSettings) {
4544
ThrottleSettings throttle = (ThrottleSettings) quota;

‎hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/SpaceQuotaHelperForTests.java

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -124,13 +124,8 @@ static void updateConfigForQuotas(Configuration conf) {
124124
* Returns the number of quotas defined in the HBase quota table.
125125
*/
126126
long listNumDefinedQuotas(Connection conn) throws IOException {
127-
QuotaRetriever scanner = QuotaRetriever.open(conn.getConfiguration());
128-
try {
127+
try (QuotaRetriever scanner = new QuotaRetriever(conn)) {
129128
return Iterables.size(scanner);
130-
} finally {
131-
if (scanner != null) {
132-
scanner.close();
133-
}
134129
}
135130
}
136131

@@ -436,8 +431,7 @@ void removeAllQuotas(Connection conn) throws IOException {
436431
waitForQuotaTable(conn);
437432
} else {
438433
// Or, clean up any quotas from previous test runs.
439-
QuotaRetriever scanner = QuotaRetriever.open(conn.getConfiguration());
440-
try {
434+
try (QuotaRetriever scanner = new QuotaRetriever(conn);) {
441435
for (QuotaSettings quotaSettings : scanner) {
442436
final String namespace = quotaSettings.getNamespace();
443437
final TableName tableName = quotaSettings.getTableName();
@@ -453,17 +447,13 @@ void removeAllQuotas(Connection conn) throws IOException {
453447
QuotaUtil.deleteUserQuota(conn, userName);
454448
}
455449
}
456-
} finally {
457-
if (scanner != null) {
458-
scanner.close();
459-
}
460450
}
461451
}
462452
}
463453

464454
QuotaSettings getTableSpaceQuota(Connection conn, TableName tn) throws IOException {
465-
try (QuotaRetriever scanner = QuotaRetriever.open(conn.getConfiguration(),
466-
new QuotaFilter().setTableFilter(tn.getNameAsString()))) {
455+
try (QuotaRetriever scanner =
456+
new QuotaRetriever(conn, new QuotaFilter().setTableFilter(tn.getNameAsString()))) {
467457
for (QuotaSettings setting : scanner) {
468458
if (setting.getTableName().equals(tn) && setting.getQuotaType() == QuotaType.SPACE) {
469459
return setting;

‎hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterQuotasObserver.java

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -325,25 +325,27 @@ public boolean namespaceExists(String ns) throws IOException {
325325
}
326326

327327
public int getNumSpaceQuotas() throws Exception {
328-
QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration());
329-
int numSpaceQuotas = 0;
330-
for (QuotaSettings quotaSettings : scanner) {
331-
if (quotaSettings.getQuotaType() == QuotaType.SPACE) {
332-
numSpaceQuotas++;
328+
try (QuotaRetriever scanner = new QuotaRetriever(TEST_UTIL.getConnection())) {
329+
int numSpaceQuotas = 0;
330+
for (QuotaSettings quotaSettings : scanner) {
331+
if (quotaSettings.getQuotaType() == QuotaType.SPACE) {
332+
numSpaceQuotas++;
333+
}
333334
}
335+
return numSpaceQuotas;
334336
}
335-
return numSpaceQuotas;
336337
}
337338

338339
public int getThrottleQuotas() throws Exception {
339-
QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration());
340-
int throttleQuotas = 0;
341-
for (QuotaSettings quotaSettings : scanner) {
342-
if (quotaSettings.getQuotaType() == QuotaType.THROTTLE) {
343-
throttleQuotas++;
340+
try (QuotaRetriever scanner = new QuotaRetriever(TEST_UTIL.getConnection())) {
341+
int throttleQuotas = 0;
342+
for (QuotaSettings quotaSettings : scanner) {
343+
if (quotaSettings.getQuotaType() == QuotaType.THROTTLE) {
344+
throttleQuotas++;
345+
}
344346
}
347+
return throttleQuotas;
345348
}
346-
return throttleQuotas;
347349
}
348350

349351
private void createTable(Admin admin, TableName tn) throws Exception {

‎hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaAdmin.java

Lines changed: 11 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ public void testThrottleType() throws Exception {
123123
QuotaSettingsFactory.throttleUser(userName, ThrottleType.WRITE_NUMBER, 12, TimeUnit.MINUTES));
124124
admin.setQuota(QuotaSettingsFactory.bypassGlobals(userName, true));
125125

126-
try (QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration())) {
126+
try (QuotaRetriever scanner = new QuotaRetriever(TEST_UTIL.getConnection())) {
127127
int countThrottle = 0;
128128
int countGlobalBypass = 0;
129129
for (QuotaSettings settings : scanner) {
@@ -169,7 +169,7 @@ public void testSimpleScan() throws Exception {
169169
TimeUnit.MINUTES));
170170
admin.setQuota(QuotaSettingsFactory.bypassGlobals(userName, true));
171171

172-
try (QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration())) {
172+
try (QuotaRetriever scanner = new QuotaRetriever(TEST_UTIL.getConnection())) {
173173
int countThrottle = 0;
174174
int countGlobalBypass = 0;
175175
for (QuotaSettings settings : scanner) {
@@ -345,11 +345,8 @@ public void testSetGetRemoveSpaceQuota() throws Exception {
345345
}
346346

347347
// Verify we can retrieve it via the QuotaRetriever API
348-
QuotaRetriever scanner = QuotaRetriever.open(admin.getConfiguration());
349-
try {
348+
try (QuotaRetriever scanner = new QuotaRetriever(admin.getConnection())) {
350349
assertSpaceQuota(sizeLimit, violationPolicy, Iterables.getOnlyElement(scanner));
351-
} finally {
352-
scanner.close();
353350
}
354351

355352
// Now, remove the quota
@@ -367,11 +364,8 @@ public void testSetGetRemoveSpaceQuota() throws Exception {
367364
}
368365

369366
// Verify that we can also not fetch it via the API
370-
scanner = QuotaRetriever.open(admin.getConfiguration());
371-
try {
367+
try (QuotaRetriever scanner = new QuotaRetriever(admin.getConnection())) {
372368
assertNull("Did not expect to find a quota entry", scanner.next());
373-
} finally {
374-
scanner.close();
375369
}
376370
}
377371

@@ -399,11 +393,8 @@ public void testSetModifyRemoveSpaceQuota() throws Exception {
399393
}
400394

401395
// Verify we can retrieve it via the QuotaRetriever API
402-
QuotaRetriever quotaScanner = QuotaRetriever.open(admin.getConfiguration());
403-
try {
396+
try (QuotaRetriever quotaScanner = new QuotaRetriever(admin.getConnection())) {
404397
assertSpaceQuota(originalSizeLimit, violationPolicy, Iterables.getOnlyElement(quotaScanner));
405-
} finally {
406-
quotaScanner.close();
407398
}
408399

409400
// Setting a new size and policy should be reflected
@@ -427,11 +418,8 @@ public void testSetModifyRemoveSpaceQuota() throws Exception {
427418
}
428419

429420
// Verify we can retrieve the new quota via the QuotaRetriever API
430-
quotaScanner = QuotaRetriever.open(admin.getConfiguration());
431-
try {
421+
try (QuotaRetriever quotaScanner = new QuotaRetriever(admin.getConnection())) {
432422
assertSpaceQuota(newSizeLimit, newViolationPolicy, Iterables.getOnlyElement(quotaScanner));
433-
} finally {
434-
quotaScanner.close();
435423
}
436424

437425
// Now, remove the quota
@@ -449,11 +437,8 @@ public void testSetModifyRemoveSpaceQuota() throws Exception {
449437
}
450438

451439
// Verify that we can also not fetch it via the API
452-
quotaScanner = QuotaRetriever.open(admin.getConfiguration());
453-
try {
440+
try (QuotaRetriever quotaScanner = new QuotaRetriever(admin.getConnection())) {
454441
assertNull("Did not expect to find a quota entry", quotaScanner.next());
455-
} finally {
456-
quotaScanner.close();
457442
}
458443
}
459444

@@ -549,8 +534,7 @@ public void testSetAndRemoveRegionServerQuota() throws Exception {
549534
admin.setQuota(QuotaSettingsFactory.throttleRegionServer(regionServer, ThrottleType.READ_NUMBER,
550535
30, TimeUnit.SECONDS));
551536
int count = 0;
552-
QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration(), rsFilter);
553-
try {
537+
try (QuotaRetriever scanner = new QuotaRetriever(TEST_UTIL.getConnection(), rsFilter)) {
554538
for (QuotaSettings settings : scanner) {
555539
assertTrue(settings.getQuotaType() == QuotaType.THROTTLE);
556540
ThrottleSettings throttleSettings = (ThrottleSettings) settings;
@@ -564,8 +548,6 @@ public void testSetAndRemoveRegionServerQuota() throws Exception {
564548
assertEquals(TimeUnit.SECONDS, throttleSettings.getTimeUnit());
565549
}
566550
}
567-
} finally {
568-
scanner.close();
569551
}
570552
assertEquals(2, count);
571553

@@ -733,14 +715,14 @@ private void verifyRecordNotPresentInQuotaTable() throws Exception {
733715
private void verifyFetchableViaAPI(Admin admin, ThrottleType type, long limit, TimeUnit tu)
734716
throws Exception {
735717
// Verify we can retrieve the new quota via the QuotaRetriever API
736-
try (QuotaRetriever quotaScanner = QuotaRetriever.open(admin.getConfiguration())) {
718+
try (QuotaRetriever quotaScanner = new QuotaRetriever(admin.getConnection())) {
737719
assertRPCQuota(type, limit, tu, Iterables.getOnlyElement(quotaScanner));
738720
}
739721
}
740722

741723
private void verifyNotFetchableViaAPI(Admin admin) throws Exception {
742724
// Verify that we can also not fetch it via the API
743-
try (QuotaRetriever quotaScanner = QuotaRetriever.open(admin.getConfiguration())) {
725+
try (QuotaRetriever quotaScanner = new QuotaRetriever(admin.getConnection())) {
744726
assertNull("Did not expect to find a quota entry", quotaScanner.next());
745727
}
746728
}
@@ -830,16 +812,13 @@ private void assertSpaceQuota(long sizeLimit, SpaceViolationPolicy violationPoli
830812
}
831813

832814
private int countResults(final QuotaFilter filter) throws Exception {
833-
QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration(), filter);
834-
try {
815+
try (QuotaRetriever scanner = new QuotaRetriever(TEST_UTIL.getConnection(), filter)) {
835816
int count = 0;
836817
for (QuotaSettings settings : scanner) {
837818
LOG.debug(Objects.toString(settings));
838819
count++;
839820
}
840821
return count;
841-
} finally {
842-
scanner.close();
843822
}
844823
}
845824

0 commit comments

Comments
 (0)