charlesconnell commented on code in PR #7188:
URL: https://github.com/apache/hbase/pull/7188#discussion_r2304750563
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaCache.java:
##########
@@ -100,6 +97,57 @@ public class QuotaCache implements Stoppable {
private QuotaRefresherChore refreshChore;
private boolean stopped = true;
+ private final Fetcher<String, UserQuotaState> userQuotaStateFetcher = new
Fetcher<>() {
Review Comment:
These Fetcher objects have the same logic as before, but are lifted into
top-level-class fields so I can access them in new places.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaCache.java:
##########
@@ -153,8 +201,13 @@ public QuotaLimiter getUserLimiter(final
UserGroupInformation ugi, final TableNa
* @return the quota info associated to specified user
*/
public UserQuotaState getUserQuotaState(final UserGroupInformation ugi) {
- return computeIfAbsent(userQuotaCache, getQuotaUserName(ugi),
- () ->
QuotaUtil.buildDefaultUserQuotaState(rsServices.getConfiguration(), 0L));
+ String user = getQuotaUserName(ugi);
+ if (!userQuotaCache.containsKey(user)) {
+ userQuotaCache.put(user,
+ QuotaUtil.buildDefaultUserQuotaState(rsServices.getConfiguration(),
0L));
+ fetch("user", userQuotaCache, userQuotaStateFetcher);
+ }
+ return userQuotaCache.get(user);
Review Comment:
This is the meat of the change. If a caller asks for a quota for a given
user, check if the quota is already saved in the cache. If so, return it from
the cache. If not, do a lookup from the `hbase:quota` table, cache the result,
and then return it. This means that users of QuotaCache don't need to wait for
its chore to run in order to get correct results.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestDefaultAtomicQuota.java:
##########
@@ -81,10 +81,6 @@ public static void setUpBeforeClass() throws Exception {
@Test
public void testDefaultAtomicReadLimits() throws Exception {
- // No write throttling
- configureLenientThrottle(ThrottleType.ATOMIC_WRITE_SIZE);
- refreshQuotas();
-
Review Comment:
These lines shouldn't have been here. Seting a per-user quota of any type
shadows the default quotas of all types, making the following assertion fail,
if QuotaCache is working properly. The updates to QuotaCache in this PR exposed
this test as incorrect.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaCache.java:
##########
@@ -543,8 +555,8 @@ static interface ThrowingSupplier<T> {
T get() throws Exception;
}
- static interface Fetcher<Key, Value> {
- Get makeGet(Map.Entry<Key, Value> entry);
Review Comment:
None of the Fetcher implementations used the entry value, so I've stopped
passing the a fully Entry
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]