bbeaudreault commented on PR #5099: URL: https://github.com/apache/hbase/pull/5099#issuecomment-1479423790
@Apache9 Thanks for taking a look. I agree the API is weird. I think the main reason is, unlike a normal RateLimiter, in HBase we can't simply do `rateLimiter.acquire(permits)`. When a request comes in (Get/Scan), we don't know how many bytes it will consume. We also have a bunch of RateLimiters of various types to check. So at a higher level, we do `quota.checkQuota` which does two loops of all rate limiters with an estimated read size (100 bytes for get, 1000 bytes for scan): - The first loop checks `canExecute` of the estimate bytes, which refills and confirms whether after refill there are enough. - The second loop does `consume` of the estimate, which actually acquires them from the rate limiter. Once the request finishes, we know how much bytes it consumed. At that point we call `consume` once more, with the real byte value minus the estimate. This can overflow the rate limiter. I was not around for the initial implementation of this. I tried looking up reviews from back then, but we weren't on GitHub yet so it's only in reviewboard where the review was minimal. My guess on why they went with 2 loops is to minimize the chance that the 5th rate limiter (or whatever) fails to consume, and then you have to rollback all the ones that succeeded in consuming. > Why we do a 'Long.MAX_VALUE + amount'? It will always overflow? In that code snippet, `amount` is negative. So despite being confusing to read, it won't overflow. I agree its confusing though. -- 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]
