bbeaudreault commented on code in PR #5426:
URL: https://github.com/apache/hbase/pull/5426#discussion_r1334522954
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaCache.java:
##########
@@ -371,19 +371,21 @@ private void updateQuotaFactors() {
// Update table machine quota factors
for (TableName tableName : tableQuotaCache.keySet()) {
- double factor = 1;
- try {
- long regionSize =
tableRegionStatesCount.get(tableName).getOpenRegions();
- if (regionSize == 0) {
- factor = 0;
- } else {
- int localRegionSize = rsServices.getRegions(tableName).size();
- factor = 1.0 * localRegionSize / regionSize;
+ if (tableRegionStatesCount.containsKey(tableName)) {
Review Comment:
The default quota scope is MACHINE, where the quota is applied individually
on each regionserver. This method here is for updating "factors" related to
CLUSTER scoped quotas. So you can define `1000 req/s` CLUSTER quota, and it
will limit you to that in aggregate across all regionservers.
At my company, we only use the default MACHINE quota. So beyond that, I also
don't have a ton of familiarity with how it works in practice.
Just skimming the code, I believe it works by converting your CLUSTER quota
to a local MACHINE quota based on the portion of the cluster this machine
occupies. So for a normal USER quota of 1000 req/s, and 10 regionservers, each
regionserver will be allowed 100 req/s. For a TABLE quota, it needs to further
divide the quota by how many regions the regionserver hosts for each table. So
for a 1000 req/s TABLE quota, if regionserver A hosts 20/100 regions it can do
200 req/s while regionserver B which hosts 10/100 regions can only do 100 req/s.
This method seems to be handling calculation of the factors used in that
division. So for regionserver A, it'd put 0.2 into the map. But that's the
limit of my understanding at this point. Obviously it seems very imperfect,
just an approximation. I also dont fully understand other portions of the
implementation, like why it assumes a factor of `1` if a table doesnt exist on
a server. To me 0 makes sense, since the quota should not apply to that server
in that case.
Anyway, in terms of the original implementation here I think we don't need
to worry about thread safety since the map is initialized 2-3 lines up. And I
do think we should trim out dropped tables from the tableRegionStatesCount as i
mentioned in my other comment.
--
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]
