[
https://issues.apache.org/jira/browse/HBASE-13888?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14584599#comment-14584599
]
Guanghao Zhang commented on HBASE-13888:
----------------------------------------
bq. I am not very clear with your bug scenario, can you explain me in short
what you want to achieve which is failing ?
In my test, the client's qps is about 500, when quota set is bigger than 35,
the quota will not work. As I output the log and found that after 30 ms, client
have 15 request, then avail will be 35 - 15 = 20. then the refill will return
Math.min(35, 20 + 1), because delta > 0. refillAmount is 21, then avail =
Math.max(0, Math.min(avail + refillAmount, limit)); so avail will be 35, again.
Every 30 ms, this is a cycle, so the limiter can not work.
bq. here you are not consuming any resource and updating the refill time is not
at all correct which means without consuming any resource by user refill time
is getting updated how is this possible in real world scenario !!?
It is for the test. In your code, refill() will return how many resources need
to refill. So I just test if it is correct. Because how many resources need to
refill is only related to how long since last refill.
bq. If we add code to consume all 60 resources before we update the next refill
time then the test will pass without your source code modification also.
Can you add limiter.consume(30) to test the code?
bq. Regarding testCanExecuteOfAverageIntervalRateLimiter I did not understand
what are you testing here, can you explain ?
As the above test case, the limiter can not work when limit is bigger than 35
per sec. So I want test if it work correctly when limit is 100, 200, 500 per
sec. The key point of test if it work is to test how many times of the
canExecute() can return true.
> refill bug from HBASE-13686
> ---------------------------
>
> Key: HBASE-13888
> URL: https://issues.apache.org/jira/browse/HBASE-13888
> Project: HBase
> Issue Type: Bug
> Affects Versions: 2.0.0
> Reporter: Guanghao Zhang
> Assignee: Guanghao Zhang
> Attachments: HBASE-13888-v1.patch, HBASE-13888-v2.patch
>
>
> As I report the RateLimiter fail to limit in HBASE-13686, then [~ashish
> singhi] fix that problem by support two kinds of RateLimiter:
> AverageIntervalRateLimiter and FixedIntervalRateLimiter. But in my use of the
> code, I found a new bug about refill() in AverageIntervalRateLimiter.
> {code}
> long delta = (limit * (now - nextRefillTime)) /
> super.getTimeUnitInMillis();
> if (delta > 0) {
> this.nextRefillTime = now;
> return Math.min(limit, available + delta);
> }
> {code}
> When delta > 0, refill maybe return available + delta. Then in the
> canExecute(), avail will add refillAmount again. So the new avail maybe 2 *
> avail + delta.
> {code}
> long refillAmount = refill(limit, avail);
> if (refillAmount == 0 && avail < amount) {
> return false;
> }
> // check for positive overflow
> if (avail <= Long.MAX_VALUE - refillAmount) {
> avail = Math.max(0, Math.min(avail + refillAmount, limit));
> } else {
> avail = Math.max(0, limit);
> }
> {code}
> I will add more unit tests for RateLimiter in the next days.
> Review Board: https://reviews.apache.org/r/35384/
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
