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: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org