SnowJena icon indicating copy to clipboard operation
SnowJena copied to clipboard

[Bug] should decrease the batch but limit on the token server side

Open wzdxhcschg opened this issue 3 years ago • 1 comments

Regarding line 42 and 45 in TokenServiceImpl.java. I feel like we should use rateLimiterRule.getBatch() but rateLimiterRule.getLimit()

TokenServiceImpl: //加锁 redisLock.acquire(RuleService.getLockKey(rateLimiterRule)); l = Long.parseLong(valueOperations.get(RuleService.getBucketKey(rateLimiterRule))); long result; if (l<=0){ result = 0L; }else if (l>= rateLimiterRule.getLimit()){ 42: valueOperations.decrement(RuleService.getBucketKey(rateLimiterRule), rateLimiterRule.getLimit()); result = rateLimiterRule.getLimit(); }else { 45: valueOperations.decrement(RuleService.getBucketKey(rateLimiterRule),l); result = l; } //解锁 redisLock.release(RuleService.getLockKey(rateLimiterRule)); return result; }

wzdxhcschg avatar Jun 24 '21 03:06 wzdxhcschg

Normally the batch value should less than the limit value. For example, the limit is 100 and the batch is 20. For the whole cluster, the limit will be 100 which means all the clients will request the downstream in less than 100 QPS. And for each client, when there's not enough token in the client side, it will request to fetch 20(the batch value) tokens from the token server. The reason it request 20 but 100 is that it would be better to not let other clients stop service(because all the tokens was token away by this client).

wzdxhcschg avatar Jun 24 '21 03:06 wzdxhcschg

这个地方确实是个bug,ticket-server 应该关注的是 rateLimiterRule.getBatch() 而不是 rateLimiterRule.getLimit()。

onblog avatar Sep 23 '22 03:09 onblog