SemaphoreBasedRateLimiter: lowering limitForPeriod below available permits stops the scheduled refresh task
Summary
SemaphoreBasedRateLimiter.refreshLimit() computes permissionsToRelease = limitForPeriod - semaphore.availablePermits() and calls semaphore.release(permissionsToRelease). After changeLimitForPeriod(newLimit) lowers the limit below the currently available permits, permissionsToRelease becomes negative on the next refresh. Semaphore.release(int) throws IllegalArgumentException for negative values, and because refreshLimit runs via scheduleAtFixedRate, that uncaught exception suppresses all subsequent executions (JDK contract). The refresh task stops executing and the limiter no longer replenishes permits for the lifetime of that instance.
Verified against the current upstream master: the implementation of refreshLimit() still contains the same code path.
void refreshLimit() {
int permissionsToRelease =
this.rateLimiterConfig.get().getLimitForPeriod() - semaphore.availablePermits();
semaphore.release(permissionsToRelease);
}Steps to reproduce
RateLimiterConfig cfg = RateLimiterConfig.custom()
.limitForPeriod(5)
.limitRefreshPeriod(Duration.ofMillis(5))
.timeoutDuration(Duration.ofSeconds(1))
.build();
SemaphoreBasedRateLimiter rl = new SemaphoreBasedRateLimiter("demo", cfg);
rl.changeLimitForPeriod(1); // availablePermits(5) > new limit(1)
// next scheduled refreshLimit(): permissionsToRelease = 1 - 5 = -4
// → semaphore.release(-4) throws IllegalArgumentException
// → scheduleAtFixedRate suppresses all future executions
// limiter never refreshes again for the lifetime of this instanceExpected
The JavaDoc of RateLimiter#changeLimitForPeriod states:
"New limit won't affect current period permissions and will apply only from next one."
There is no documented restriction that the new limit must be greater than or equal to the current number of available permits, nor is lowering the limit documented as unsupported.
Actual
The first refreshLimit after a downward changeLimitForPeriod throws IllegalArgumentException; the periodic refresh task terminates and no longer replenishes permits.
Why this appears to be a bug
The two RateLimiter implementations expose the same public API but currently behave differently: AtomicRateLimiter supports decreasing the limit without terminating its internal state-update mechanism (it copies activePermissions unchanged and absorbs the excess), while SemaphoreBasedRateLimiter causes its scheduled refresh task to terminate.
Impact
Any runtime decrease of limitForPeriod (e.g. autoscaling a rate limit down) on a SemaphoreBasedRateLimiter silently breaks permit replenishment until the instance is recreated.
Possible directions (maintainer's call)
- Guard
releaseagainst negative permits (minimal). - Adjust the semaphore inside
changeLimitForPeriod(e.g. drain down to the new limit) so the new limit takes effect immediately. - Make the scheduled refresh task failure-safe regardless of the cause.
A regression test is available that reproduces the failure. If this behavior is considered a bug, I'd be happy to prepare a PR rebased onto the current master together with a JUnit 5 regression test.
Source: resilience4j/resilience4j