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.

Pull request overview

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

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

• isLimited calls LocalDateTime.now() multiple times and can therefore return Optional.of(Duration.ZERO) (or clamp a negative value to zero) even though the refresh period has effectively just expired between the two now() calls. In that case the caller treats the resource as still rate-limited and reschedules, even though a permission could already be acquired. Consider capturing now once and deciding expiration based on the same timestamp (and treating expiry-at-boundary as not limited).

      var remaining =
          Duration.between(
              LocalDateTime.now(), actualState.getLastRefreshTime().plus(refreshPeriod));
      return Optional.of(remaining.isNegative() ? Duration.ZERO : remaining);

LGTM

Pull request overview

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

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

• isLimited returns Optional.of(Duration.ZERO) when lastRefreshTime is exactly now.minus(refreshPeriod) (period boundary) because the reset branch uses a strict isBefore check. Per the RateLimiter contract, when the minimal wait is 0 a permission can be acquired now, so this should return Optional.empty() (and increment the new period count) rather than being treated as still rate-limited (which in EventProcessor triggers a timer reschedule with a non-zero floor).

    var now = LocalDateTime.now();
    if (actualState.getCount() < limitForPeriod) {
      actualState.increaseCount();
      return Optional.empty();
    } else if (actualState.getLastRefreshTime().isBefore(now.minus(refreshPeriod))) {
      actualState.reset();
      actualState.increaseCount();
      return Optional.empty();
    } else {
      var remaining = Duration.between(now, actualState.getLastRefreshTime().plus(refreshPeriod));
      return Optional.of(remaining.isNegative() ? Duration.ZERO : remaining);

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

• The new test relies on Thread.sleep(REFRESH_PERIOD / 2) with REFRESH_PERIOD set to 300ms and then assumes the second isLimited call is still within the same period (orElseThrow()). On slow/loaded CI runners, oversleep/jitter can exceed the remaining ~150ms and the limiter can reset, making isLimited return empty and causing a sporadic failure. Consider using a longer refresh period just for this test to reduce flakiness.

  void reportedDurationIsTheTimeRemainingNotTheTimeElapsed() throws InterruptedException {
    var rl = new LinearRateLimiter(REFRESH_PERIOD, 1);
    assertThat(rl.isLimited(state)).isEmpty();

    var justAfterLimit = rl.isLimited(state).orElseThrow();
    Thread.sleep(REFRESH_PERIOD.toMillis() / 2);
    var halfWayThroughPeriod = rl.isLimited(state).orElseThrow();

Pull request overview

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

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

• The refresh-period reset check excludes the exact boundary case (when lastRefreshTime equals now.minus(refreshPeriod)), which causes isLimited to return a 0-duration wait instead of allowing a permission immediately at the period boundary.

    } else if (actualState.getLastRefreshTime().isBefore(now.minus(refreshPeriod))) {

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

• This test uses Thread.sleep with a relatively short refresh period, which can be flaky on loaded CI runners (oversleeping past the period makes the second isLimited call return empty and the test fail). Consider making it deterministic by constructing RateState instances with controlled lastRefreshTime values instead of sleeping.

    assertThat(rl.isLimited(state)).isEmpty();

    var justAfterLimit = rl.isLimited(state).orElseThrow();
    Thread.sleep(REFRESH_PERIOD.toMillis() / 2);
    var halfWayThroughPeriod = rl.isLimited(state).orElseThrow();