dotnet / dotnet/runtime

Revisit RetryAfter behavior in TokenBucketRateLimiter and SlidingWindowRateLimiter

Open
#129,036 3 comments 0 reactions 0 assignees View on GitHub
area-System.Threading
Dominant language
C#
Stars
18.3k
Forks
5.6k
PR merge metrics
PR metrics pending

Description

Issue #92557 identified misleading RetryAfter metadata in FixedWindowRateLimiter.

PR #124478 addressed this by changing FixedWindowRateLimiter to return the time until the next replenishment/window boundary, regardless of whether the limiter is using a queue.

TokenBucketRateLimiter and SlidingWindowRateLimiter also expose RetryAfter metadata, but their behavior appears inconsistent with the updated FixedWindowRateLimiter implementation:

SlidingWindowRateLimiter returns a failed lease where the RetryAfter value is null.
TokenBucketRateLimiter appears to follow the same approach that FixedWindowRateLimiter used prior to #124478, by calling CreateFailedTokenLease and returning the full ReplenishmentPeriod rather than the time remaining until the next replenishment.

In #92557, @BrennanConroy mentioned that the other rate limiters exposing RetryAfter should be reviewed as well.

The question is whether the semantics of RetryAfter across the built-in rate limiters should be aligned, and if so, whether TokenBucketRateLimiter and SlidingWindowRateLimiter should adopt behavior similar to the updated FixedWindowRateLimiter.

The simplest approach would be to mirror the changes made in FixedWindowRateLimiter, but it would be useful to confirm the intended behavior before making that change.

Related issues:

#77991
#92557

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.