`StateSnapshot::remaining_burst_capacity()` never "replenishes"

hgmich commented 2 years ago

StateSnapshot::remaining_burst_capacity() does not seem to replenish correctly, despite correct rate limiting behaviour being observed; once the initial burst quota is exceeded, it is permanently set at zero, even after the replenishment time has passed (and the rate limiter is not rejecting requests). Some tracing indicates that the calculation uses tat as if it were relative to the last "replenishment window", but it is always relative to the RateLimiter's start time, leading to the calculation being as if no time had passed since the first construction of the RateLimiter.

I will update this issue shortly with a reproducible test case.

hgmich commented 2 years ago

Test case reproducing/demonstrating issue:

#[test]
fn state_snapshot_tracks_quota_accurately() {
    use crate::clock::FakeRelativeClock;
    use crate::RateLimiter;
    use nonzero_ext::*;

    let clock = FakeRelativeClock::default();

    let lim = RateLimiter::direct_with_clock(Quota::per_minute(nonzero!(5_u32)), &clock)
        .with_middleware::<StateInformationMiddleware>();

    let state = lim.check_n(nonzero!(4_u32)).expect("Should not rate limit");
    assert_eq!(state.remaining_burst_capacity(), 1);

    let state = lim.check().expect("Should not rate limit");
    assert_eq!(state.remaining_burst_capacity(), 0);

    lim.check().expect_err("Should rate limit");

    clock.advance(Duration::from_secs(120));
    let state = lim.check().expect("Should not rate limit");
    assert_eq!(state.remaining_burst_capacity(), 4);
}

The expected behaviour should be that this test passes.

Instead, it fails at the last line as state.remaining_burst_capacity() is 0:

thread 'middleware::test::state_snapshot_tracks_quota_accurately' panicked at 'assertion failed: `(left == right)`
  left: `0`,
 right: `4`'[...]