Skip to content

Commit b7d11a7

Browse files
charlesconnellcharlesconnell
authored
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>
1 parent aeeb855 commit b7d11a7

File tree

7 files changed

+56
-72
lines changed

7 files changed

+56
-72
lines changed

‎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
@@ -120,13 +120,8 @@ static void updateConfigForQuotas(Configuration conf) {
120120
* Returns the number of quotas defined in the HBase quota table.
121121
*/
122122
long listNumDefinedQuotas(Connection conn) throws IOException {
123-
QuotaRetriever scanner = QuotaRetriever.open(conn.getConfiguration());
124-
try {
123+
try (QuotaRetriever scanner = new QuotaRetriever(conn)) {
125124
return Iterables.size(scanner);
126-
} finally {
127-
if (scanner != null) {
128-
scanner.close();
129-
}
130125
}
131126
}
132127

@@ -353,8 +348,7 @@ void removeAllQuotas(Connection conn) throws IOException {
353348
waitForQuotaTable(conn);
354349
} else {
355350
// Or, clean up any quotas from previous test runs.
356-
QuotaRetriever scanner = QuotaRetriever.open(conn.getConfiguration());
357-
try {
351+
try (QuotaRetriever scanner = new QuotaRetriever(conn);) {
358352
for (QuotaSettings quotaSettings : scanner) {
359353
final String namespace = quotaSettings.getNamespace();
360354
final TableName tableName = quotaSettings.getTableName();
@@ -370,17 +364,13 @@ void removeAllQuotas(Connection conn) throws IOException {
370364
QuotaUtil.deleteUserQuota(conn, userName);
371365
}
372366
}
373-
} finally {
374-
if (scanner != null) {
375-
scanner.close();
376-
}
377367
}
378368
}
379369
}
380370

381371
QuotaSettings getTableSpaceQuota(Connection conn, TableName tn) throws IOException {
382-
try (QuotaRetriever scanner = QuotaRetriever.open(conn.getConfiguration(),
383-
new QuotaFilter().setTableFilter(tn.getNameAsString()))) {
372+
try (QuotaRetriever scanner =
373+
new QuotaRetriever(conn, new QuotaFilter().setTableFilter(tn.getNameAsString()))) {
384374
for (QuotaSettings setting : scanner) {
385375
if (setting.getTableName().equals(tn) && setting.getQuotaType() == QuotaType.SPACE) {
386376
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
@@ -327,25 +327,27 @@ public boolean namespaceExists(String ns) throws IOException {
327327
}
328328

329329
public int getNumSpaceQuotas() throws Exception {
330-
QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration());
331-
int numSpaceQuotas = 0;
332-
for (QuotaSettings quotaSettings : scanner) {
333-
if (quotaSettings.getQuotaType() == QuotaType.SPACE) {
334-
numSpaceQuotas++;
330+
try (QuotaRetriever scanner = new QuotaRetriever(TEST_UTIL.getConnection())) {
331+
int numSpaceQuotas = 0;
332+
for (QuotaSettings quotaSettings : scanner) {
333+
if (quotaSettings.getQuotaType() == QuotaType.SPACE) {
334+
numSpaceQuotas++;
335+
}
335336
}
337+
return numSpaceQuotas;
336338
}
337-
return numSpaceQuotas;
338339
}
339340

340341
public int getThrottleQuotas() throws Exception {
341-
QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration());
342-
int throttleQuotas = 0;
343-
for (QuotaSettings quotaSettings : scanner) {
344-
if (quotaSettings.getQuotaType() == QuotaType.THROTTLE) {
345-
throttleQuotas++;
342+
try (QuotaRetriever scanner = new QuotaRetriever(TEST_UTIL.getConnection())) {
343+
int throttleQuotas = 0;
344+
for (QuotaSettings quotaSettings : scanner) {
345+
if (quotaSettings.getQuotaType() == QuotaType.THROTTLE) {
346+
throttleQuotas++;
347+
}
346348
}
349+
return throttleQuotas;
347350
}
348-
return throttleQuotas;
349351
}
350352

351353
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)