Skip to content

fix: report remaining, not elapsed, time from LinearRateLimiter - #3521

Draft
csviri wants to merge 1 commit into
operator-framework:mainfrom
csviri:fix/rate-limiter-remaining-duration
Draft

fix: report remaining, not elapsed, time from LinearRateLimiter#3521
csviri wants to merge 1 commit into
operator-framework:mainfrom
csviri:fix/rate-limiter-remaining-duration

Conversation

@csviri

@csviri csviri commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

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.

Part of #3517

`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.
Copilot AI review requested due to automatic review settings July 30, 2026 09:04
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 30, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes LinearRateLimiter#isLimited to report the remaining time until a permission can be acquired (as documented), preventing overly-early rescheduling of rate-limited resources.

Changes:

  • Update LinearRateLimiter.isLimited to compute remaining time in the current refresh period (clamped at zero).
  • Strengthen returnsMinimalDurationToAcquirePermission to assert the reported wait is close to the full refresh period.
  • Add a regression test ensuring the reported duration decreases as the refresh period elapses.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/LinearRateLimiter.java Switches isLimited from reporting elapsed time to reporting remaining time in the refresh period.
operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/rate/LinearRateLimiterTest.java Updates and adds tests to distinguish “remaining” vs “elapsed” duration behavior.

Comment on lines +66 to +71
var justAfterLimit = rl.isLimited(state).orElseThrow();
Thread.sleep(REFRESH_PERIOD.toMillis() / 2);
var halfWayThroughPeriod = rl.isLimited(state).orElseThrow();

// as the refresh period elapses, less time remains until a permission is available again
assertThat(halfWayThroughPeriod).isLessThan(justAfterLimit);
Comment on lines +67 to +70
var remaining =
Duration.between(
LocalDateTime.now(), actualState.getLastRefreshTime().plus(refreshPeriod));
return Optional.of(remaining.isNegative() ? Duration.ZERO : remaining);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants