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

Reply via email to