danielsinai commented on a change in pull request #11446:
URL: https://github.com/apache/pulsar/pull/11446#discussion_r676026166
##########
File path:
pulsar-common/src/main/java/org/apache/pulsar/common/util/RateLimiter.java
##########
@@ -257,14 +267,14 @@ public synchronized TimeUnit getRateTimeUnit() {
}
synchronized void renew() {
- acquiredPermits = isDispatchRateLimiter ? Math.max(0, acquiredPermits
- permits) : 0;
+ acquiredPermits = isDispatchOrPrecisePublishRateLimiter ? Math.max(0,
acquiredPermits - permits) : 0;
if (permitUpdater != null) {
long newPermitRate = permitUpdater.get();
if (newPermitRate > 0) {
setRate(newPermitRate);
}
}
- if (rateLimitFunction != null) {
+ if (rateLimitFunction != null && this.getAvailablePermits() > 0) {
Review comment:
Yea Ill add a comment.
And actually i didn't give it much thought but when I'm thinking about it
the rateLimitFunction is a callback that lets the outer scope access to the
renew function, I don't think that we should use this property without knowing
exactly what expected to be happening.
I believe that checking whether there are available permits is the
reasonable condition here because we would want to let the outer scope a way to
run something when there are any available permits, and it doesn't really
depend on the class property.
If it sets to false we can assume the user of the rateLimiter wants the
state to be reset every time window otherwise he probably want back-pressuring
something.
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
