ndimiduk commented on code in PR #6451:
URL: https://github.com/apache/hbase/pull/6451#discussion_r1834691267
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaCache.java:
##########
@@ -40,7 +40,7 @@ public class TestQuotaCache {
HBaseClassTestRule.forClass(TestQuotaCache.class);
private static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
- private static final int REFRESH_TIME = 30_000;
+ private static final int REFRESH_TIME = 1000;
Review Comment:
nit: `REFRESH_TIME_MS`
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaCache.java:
##########
@@ -233,8 +242,33 @@ Map<String, UserQuotaState> getUserQuotaCache() {
private class QuotaRefresherChore extends ScheduledChore {
private long lastUpdate = 0;
- public QuotaRefresherChore(final int period, final Stoppable stoppable) {
+ // Querying cluster metrics so often, per-RegionServer, limits horizontal
scalability.
+ // So we cache the results to reduce that load.
+ private final RefreshableExpiringValueCache<ClusterMetrics>
tableRegionStatesClusterMetrics;
+ private final RefreshableExpiringValueCache<Integer> regionServersSize;
+
+ public QuotaRefresherChore(Configuration conf, final int period, final
Stoppable stoppable) {
super("QuotaRefresherChore", stoppable, period);
+
+ Duration tableRegionStatesCacheTtl =
+ Duration.ofMillis(conf.getLong(TABLE_REGION_STATES_CACHE_TTL_MS,
period));
+ this.tableRegionStatesClusterMetrics =
+ new RefreshableExpiringValueCache<>("tableRegionStatesClusterMetrics",
Review Comment:
As a follow-up JIRA, should these configuration values be hot-reloadable ?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaCache.java:
##########
@@ -61,6 +67,10 @@ public class QuotaCache implements Stoppable {
private static final Logger LOG = LoggerFactory.getLogger(QuotaCache.class);
public static final String REFRESH_CONF_KEY = "hbase.quota.refresh.period";
+ public static final String TABLE_REGION_STATES_CACHE_TTL_MS =
+ "hbase.quota.cache.ttl.region.states.ms";
Review Comment:
Thank you for including a unit indicator in the config key! 🙇
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaCache.java:
##########
@@ -436,6 +489,52 @@ private void updateQuotaFactors() {
}
}
}
+
+ private void updateMachineQuotaFactors(int rsSize) {
+ if (rsSize != 0) {
+ // TODO if use rs group, the cluster limit should be shared by the rs
group
+ machineQuotaFactor = 1.0 / rsSize;
+ }
+ }
+ }
+
+ static class RefreshableExpiringValueCache<T> {
+ private static final String DUMMY_KEY = "dummyKey";
Review Comment:
Can you explain what `DUMMY_KEY` is doing here? If I'm following, you have a
`LoadingCache` (which essentially is a `Map` interface) that is only every
populated with a single key, the value passed into the constructor as `String
name`. The `CacheLoader` interface ignores the provided `key`, but, so what's
with `DUMMY_KEY` ? Can't you use the provided `name` instead of `DUMMY_KEY` ?
Or is there something else I'm missing here?
--
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
