Skip to content

Commit aaf985a

Browse files
committed
fix: report remaining, not elapsed, time from LinearRateLimiter
`RateLimiter.isLimited` is documented to return the "minimal duration until a permission could be acquired again", but `LinearRateLimiter` returned the time *elapsed* since the current period started: Duration.between(actualState.getLastRefreshTime(), LocalDateTime.now()) The two are inverted. Measured with a 1000ms refresh period: moment reported correct right after limit hit 19ms ~1000ms 800ms into the period 804ms ~200ms `EventProcessor.handleRateLimitedSubmission` feeds this value straight into `TimerEventSource.scheduleOnce`, so a rate-limited resource is rescheduled almost immediately after the limit is reached (floored at MINIMAL_RATE_LIMIT_RESCHEDULE_DURATION), gets rate-limited again, and repeats — producing a burst of pointless timer events. Conversely, a resource limited near the end of a period waits roughly a full extra period after a permission was already available. Now returns the time until the current period ends, clamped at zero. `returnsMinimalDurationToAcquirePermission` only asserted `isLessThan(REFRESH_PERIOD)`, which held for both the correct and the inverted value; it now also asserts the reported wait is close to the full period. A second test asserts the reported duration shrinks as the period elapses, which is what distinguishes remaining from elapsed. Both fail without this change.
1 parent 67c47c7 commit aaf985a

2 files changed

Lines changed: 20 additions & 2 deletions

File tree

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/LinearRateLimiter.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,10 @@ public Optional<Duration> isLimited(RateLimitState rateLimitState) {
6464
actualState.increaseCount();
6565
return Optional.empty();
6666
} else {
67-
return Optional.of(Duration.between(actualState.getLastRefreshTime(), LocalDateTime.now()));
67+
var remaining =
68+
Duration.between(
69+
LocalDateTime.now(), actualState.getLastRefreshTime().plus(refreshPeriod));
70+
return Optional.of(remaining.isNegative() ? Duration.ZERO : remaining);
6871
}
6972
}
7073

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/rate/LinearRateLimiterTest.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,22 @@ void returnsMinimalDurationToAcquirePermission() {
5353
res = rl.isLimited(state);
5454

5555
assertThat(res).isPresent();
56-
assertThat(res.get()).isLessThan(REFRESH_PERIOD);
56+
assertThat(res.get()).isLessThanOrEqualTo(REFRESH_PERIOD);
57+
// the whole period is still ahead of us, so the reported wait must be close to it
58+
assertThat(res.get()).isGreaterThan(REFRESH_PERIOD.dividedBy(2));
59+
}
60+
61+
@Test
62+
void reportedDurationIsTheTimeRemainingNotTheTimeElapsed() throws InterruptedException {
63+
var rl = new LinearRateLimiter(REFRESH_PERIOD, 1);
64+
assertThat(rl.isLimited(state)).isEmpty();
65+
66+
var justAfterLimit = rl.isLimited(state).orElseThrow();
67+
Thread.sleep(REFRESH_PERIOD.toMillis() / 2);
68+
var halfWayThroughPeriod = rl.isLimited(state).orElseThrow();
69+
70+
// as the refresh period elapses, less time remains until a permission is available again
71+
assertThat(halfWayThroughPeriod).isLessThan(justAfterLimit);
5772
}
5873

5974
@Test

0 commit comments

Comments
 (0)