From fcc4e014f6cd1037257b3bdffe5170e49a437bc4 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Wed, 29 Jun 2022 15:26:47 -0700 Subject: [PATCH 1/8] [RateLimiting] TryReplenish handles multiple replenish periods at a time --- .../RateLimiting/FixedWindowRateLimiter.cs | 18 +- .../RateLimiting/SlidingWindowRateLimiter.cs | 38 +++- .../RateLimiting/TokenBucketRateLimiter.cs | 18 +- .../tests/FixedWindowRateLimiterTests.cs | 146 +++++++----- .../tests/SlidingWindowRateLimiterTests.cs | 214 +++++++++++------- .../tests/TokenBucketRateLimiterTests.cs | 207 +++++++++-------- 6 files changed, 377 insertions(+), 264 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs index fe4b0c29c3a62..f04e5ec8fc533 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs @@ -287,29 +287,25 @@ private void ReplenishInternal(long nowTicks) return; } - if ((long)((nowTicks - _lastReplenishmentTick) * TickFrequency) < _options.Window.Ticks) + long periods = (long)((nowTicks - _lastReplenishmentTick) * TickFrequency) / _options.Window.Ticks; + if (periods == 0) { return; } - _lastReplenishmentTick = nowTicks; + // increment last tick by the number of replenish periods that occurred since the last replenish + // this way if replenish isn't being called every ReplenishmentPeriod we correctly track it so we know when replenishes should be occurring + _lastReplenishmentTick += (long)(periods * ReplenishmentPeriod.Ticks / TickFrequency); int availableRequestCounters = _requestCount; - int maxPermits = _options.PermitLimit; - int resourcesToAdd; - if (availableRequestCounters < maxPermits) - { - resourcesToAdd = maxPermits - availableRequestCounters; - } - else + if (availableRequestCounters >= _options.PermitLimit) { // All counters available, nothing to do return; } - _requestCount += resourcesToAdd; - Debug.Assert(_requestCount == _options.PermitLimit); + _requestCount = _options.PermitLimit; // Process queued requests while (_queue.Count > 0) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs index 1ccf40775e2d8..11d4649ec3bdc 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs @@ -26,6 +26,7 @@ public sealed class SlidingWindowRateLimiter : ReplenishingRateLimiter private readonly Timer? _renewTimer; private readonly SlidingWindowRateLimiterOptions _options; + private readonly TimeSpan _replenishmentPeriod; private readonly Deque _queue = new Deque(); // Use the queue as the lock field so we don't need to allocate another object for a lock and have another field in the object @@ -42,7 +43,7 @@ public sealed class SlidingWindowRateLimiter : ReplenishingRateLimiter public override bool IsAutoReplenishing => _options.AutoReplenishment; /// - public override TimeSpan ReplenishmentPeriod => new TimeSpan(_options.Window.Ticks / _options.SegmentsPerWindow); + public override TimeSpan ReplenishmentPeriod => _replenishmentPeriod; /// /// Initializes the . @@ -78,6 +79,7 @@ public SlidingWindowRateLimiter(SlidingWindowRateLimiterOptions options) }; _requestCount = options.PermitLimit; + _replenishmentPeriod = new TimeSpan(_options.Window.Ticks / _options.SegmentsPerWindow); // _requestsPerSegment holds the no. of acquired requests in each window segment _requestsPerSegment = new int[options.SegmentsPerWindow]; @@ -287,27 +289,39 @@ private void ReplenishInternal(long nowTicks) return; } - if ((long)((nowTicks - _lastReplenishmentTick) * TickFrequency) < ReplenishmentPeriod.Ticks) + long periods = (long)((nowTicks - _lastReplenishmentTick) * TickFrequency) / ReplenishmentPeriod.Ticks; + if (periods == 0) { return; } - _lastReplenishmentTick = nowTicks; + // increment last tick by the number of replenish periods that occurred since the last replenish + // this way if replenish isn't being called every ReplenishmentPeriod we correctly track it so we know when replenishes should be occurring + _lastReplenishmentTick += (long)(periods * ReplenishmentPeriod.Ticks / TickFrequency); - // Increment the current segment index while move the window - // We need to know the no. of requests that were acquired in a segment previously to ensure that we don't acquire more than the permit limit. - _currentSegmentIndex = (_currentSegmentIndex + 1) % _options.SegmentsPerWindow; - int oldSegmentRequestCount = _requestsPerSegment[_currentSegmentIndex]; - _requestsPerSegment[_currentSegmentIndex] = 0; + int initialRequestCount = _requestCount; + do + { + // Increment the current segment index while move the window + // We need to know the no. of requests that were acquired in a segment previously to ensure that we don't acquire more than the permit limit. + _currentSegmentIndex = (_currentSegmentIndex + 1) % _options.SegmentsPerWindow; + int oldSegmentRequestCount = _requestsPerSegment[_currentSegmentIndex]; + _requestsPerSegment[_currentSegmentIndex] = 0; - if (oldSegmentRequestCount == 0) + if (oldSegmentRequestCount != 0) + { + _requestCount += oldSegmentRequestCount; + Debug.Assert(_requestCount <= _options.PermitLimit); + } + periods--; + } while (periods > 0); + + if (initialRequestCount == _requestCount) { + // no requests added, queued items don't need updating return; } - _requestCount += oldSegmentRequestCount; - Debug.Assert(_requestCount <= _options.PermitLimit); - // Process queued requests while (_queue.Count > 0) { diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs index 7baf91ea59080..d27fc7c9e551d 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs @@ -278,7 +278,7 @@ private static void Replenish(object? state) limiter!.ReplenishInternal(nowTicks); } - // Used in tests that test behavior with specific time intervals + // Used in tests to avoid dealing with real time private void ReplenishInternal(long nowTicks) { // method is re-entrant (from Timer), lock to avoid multiple simultaneous replenishes @@ -289,21 +289,23 @@ private void ReplenishInternal(long nowTicks) return; } - if ((long)((nowTicks - _lastReplenishmentTick) * TickFrequency) < _options.ReplenishmentPeriod.Ticks) + long periods = (long)((nowTicks - _lastReplenishmentTick) * TickFrequency) / _options.ReplenishmentPeriod.Ticks; + if (periods == 0) { return; } - _lastReplenishmentTick = nowTicks; + // increment last tick by the number of replenish periods that occurred since the last replenish + // this way if replenish isn't being called every ReplenishmentPeriod we correctly track it so we know when replenishes should be occurring + _lastReplenishmentTick += (long)(periods * _options.ReplenishmentPeriod.Ticks / TickFrequency); int availablePermits = _tokenCount; - TokenBucketRateLimiterOptions options = _options; - int maxPermits = options.TokenLimit; + int maxPermits = _options.TokenLimit; int resourcesToAdd; if (availablePermits < maxPermits) { - resourcesToAdd = Math.Min(options.TokensPerPeriod, maxPermits - availablePermits); + resourcesToAdd = (int)Math.Min(_options.TokensPerPeriod * periods, maxPermits - availablePermits); } else { @@ -319,7 +321,7 @@ private void ReplenishInternal(long nowTicks) while (queue.Count > 0) { RequestRegistration nextPendingRequest = - options.QueueProcessingOrder == QueueProcessingOrder.OldestFirst + _options.QueueProcessingOrder == QueueProcessingOrder.OldestFirst ? queue.PeekHead() : queue.PeekTail(); @@ -327,7 +329,7 @@ private void ReplenishInternal(long nowTicks) { // Request can be fulfilled nextPendingRequest = - options.QueueProcessingOrder == QueueProcessingOrder.OldestFirst + _options.QueueProcessingOrder == QueueProcessingOrder.OldestFirst ? queue.DequeueHead() : queue.DequeueTail(); diff --git a/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs index 6830a1ce74281..243613c17f5ed 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs @@ -17,7 +17,7 @@ public override void CanAcquireResource() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); var lease = limiter.AttemptAcquire(); @@ -27,7 +27,7 @@ public override void CanAcquireResource() lease.Dispose(); Assert.False(limiter.AttemptAcquire().IsAcquired); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.True(limiter.AttemptAcquire().IsAcquired); } @@ -62,6 +62,10 @@ public override void InvalidOptionsThrows() Window = TimeSpan.MinValue, AutoReplenishment = false })); + Assert.Throws( + () => new FixedWindowRateLimiterOptions(1, QueueProcessingOrder.NewestFirst, 1, TimeSpan.FromMinutes(-2), autoReplenishment: false)); + Assert.Throws( + () => new FixedWindowRateLimiterOptions(1, QueueProcessingOrder.NewestFirst, 1, TimeSpan.Zero, autoReplenishment: false)); } [Fact] @@ -72,7 +76,7 @@ public override async Task CanAcquireResourceAsync() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); @@ -82,7 +86,7 @@ public override async Task CanAcquireResourceAsync() var wait = limiter.AcquireAsync(); Assert.False(wait.IsCompleted); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.True((await wait).IsAcquired); } @@ -95,7 +99,7 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsOldest() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 2, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); var lease = await limiter.AcquireAsync(); @@ -107,7 +111,7 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsOldest() Assert.False(wait2.IsCompleted); lease.Dispose(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); lease = await wait1; Assert.True(lease.IsAcquired); @@ -115,7 +119,7 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsOldest() lease.Dispose(); Assert.Equal(0, limiter.GetStatistics().CurrentAvailablePermits); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); lease = await wait2; Assert.True(lease.IsAcquired); @@ -129,7 +133,7 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsNewest() PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 3, - Window = TimeSpan.FromMinutes(0), + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); @@ -142,7 +146,7 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsNewest() Assert.False(wait2.IsCompleted); lease.Dispose(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); // second queued item completes first with NewestFirst lease = await wait2; @@ -151,7 +155,7 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsNewest() lease.Dispose(); Assert.Equal(1, limiter.GetStatistics().CurrentAvailablePermits); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); lease = await wait1; Assert.True(lease.IsAcquired); @@ -165,7 +169,7 @@ public override async Task FailsWhenQueuingMoreThanLimit_OldestFirst() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); using var lease = limiter.AttemptAcquire(1); @@ -174,7 +178,7 @@ public override async Task FailsWhenQueuingMoreThanLimit_OldestFirst() var failedLease = await limiter.AcquireAsync(1); Assert.False(failedLease.IsAcquired); Assert.True(failedLease.TryGetMetadata(MetadataName.RetryAfter, out var timeSpan)); - Assert.Equal(TimeSpan.Zero, timeSpan); + Assert.Equal(TimeSpan.FromMilliseconds(2), timeSpan); } [Fact] @@ -185,7 +189,7 @@ public override async Task DropsOldestWhenQueuingMoreThanLimit_NewestFirst() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); var lease = limiter.AttemptAcquire(1); @@ -197,7 +201,7 @@ public override async Task DropsOldestWhenQueuingMoreThanLimit_NewestFirst() Assert.False(lease1.IsAcquired); Assert.False(wait2.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait2; Assert.True(lease.IsAcquired); @@ -211,7 +215,7 @@ public override async Task DropsMultipleOldestWhenQueuingMoreThanLimit_NewestFir PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 2, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); var lease = limiter.AttemptAcquire(2); @@ -229,7 +233,7 @@ public override async Task DropsMultipleOldestWhenQueuingMoreThanLimit_NewestFir Assert.False(lease2.IsAcquired); Assert.False(wait3.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait3; Assert.True(lease.IsAcquired); @@ -243,7 +247,7 @@ public override async Task DropsRequestedLeaseIfPermitCountGreaterThanQueueLimit PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); var lease = limiter.AttemptAcquire(2); @@ -256,7 +260,7 @@ public override async Task DropsRequestedLeaseIfPermitCountGreaterThanQueueLimit var lease1 = await limiter.AcquireAsync(2); Assert.False(lease1.IsAcquired); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); @@ -270,7 +274,7 @@ public override async Task QueueAvailableAfterQueueLimitHitAndResources_BecomeAv PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); var lease = limiter.AttemptAcquire(1); @@ -279,14 +283,14 @@ public override async Task QueueAvailableAfterQueueLimitHitAndResources_BecomeAv var failedLease = await limiter.AcquireAsync(1); Assert.False(failedLease.IsAcquired); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); wait = limiter.AcquireAsync(1); Assert.False(wait.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); } @@ -299,7 +303,7 @@ public override async Task LargeAcquiresAndQueuesDoNotIntegerOverflow() PermitLimit = int.MaxValue, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = int.MaxValue, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); var lease = limiter.AttemptAcquire(int.MaxValue); @@ -315,7 +319,7 @@ public override async Task LargeAcquiresAndQueuesDoNotIntegerOverflow() var lease1 = await wait; Assert.False(lease1.IsAcquired); - limiter.TryReplenish(); + Replenish(limiter, 1L); var lease2 = await wait2; Assert.True(lease2.IsAcquired); } @@ -328,7 +332,7 @@ public override void ThrowsWhenAcquiringMoreThanLimit() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); Assert.Throws(() => limiter.AttemptAcquire(2)); @@ -342,7 +346,7 @@ public override async Task ThrowsWhenWaitingForMoreThanLimit() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); await Assert.ThrowsAsync(async () => await limiter.AcquireAsync(2)); @@ -356,7 +360,7 @@ public override void ThrowsWhenAcquiringLessThanZero() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); Assert.Throws(() => limiter.AttemptAcquire(-1)); @@ -370,7 +374,7 @@ public override async Task ThrowsWhenWaitingForLessThanZero() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); await Assert.ThrowsAsync(async () => await limiter.AcquireAsync(-1)); @@ -384,7 +388,7 @@ public override void AcquireZero_WithAvailability() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); @@ -400,7 +404,7 @@ public override void AcquireZero_WithoutAvailability() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); using var lease = limiter.AttemptAcquire(1); @@ -419,7 +423,7 @@ public override async Task AcquireAsyncZero_WithAvailability() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); @@ -435,7 +439,7 @@ public override async Task AcquireAsyncZero_WithoutAvailabilityWaitsForAvailabil PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); var lease = await limiter.AcquireAsync(1); @@ -445,7 +449,7 @@ public override async Task AcquireAsyncZero_WithoutAvailabilityWaitsForAvailabil Assert.False(wait.IsCompleted); lease.Dispose(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); using var lease2 = await wait; Assert.True(lease2.IsAcquired); } @@ -458,7 +462,7 @@ public override async Task CanDequeueMultipleResourcesAtOnce() PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 2, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); using var lease = await limiter.AcquireAsync(2); @@ -470,7 +474,7 @@ public override async Task CanDequeueMultipleResourcesAtOnce() Assert.False(wait2.IsCompleted); lease.Dispose(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); var lease1 = await wait1; var lease2 = await wait2; @@ -486,7 +490,7 @@ public override async Task CanCancelAcquireAsyncAfterQueuing() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); var lease = limiter.AttemptAcquire(1); @@ -500,7 +504,7 @@ public override async Task CanCancelAcquireAsyncAfterQueuing() Assert.Equal(cts.Token, ex.CancellationToken); lease.Dispose(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.Equal(1, limiter.GetStatistics().CurrentAvailablePermits); } @@ -513,7 +517,7 @@ public override async Task CanCancelAcquireAsyncBeforeQueuing() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); var lease = limiter.AttemptAcquire(1); @@ -526,7 +530,7 @@ public override async Task CanCancelAcquireAsyncBeforeQueuing() Assert.Equal(cts.Token, ex.CancellationToken); lease.Dispose(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.Equal(1, limiter.GetStatistics().CurrentAvailablePermits); } @@ -539,7 +543,7 @@ public override async Task CancelUpdatesQueueLimit() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); var lease = limiter.AttemptAcquire(1); @@ -555,7 +559,7 @@ public override async Task CancelUpdatesQueueLimit() wait = limiter.AcquireAsync(1); Assert.False(wait.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); } @@ -568,7 +572,7 @@ public override void NoMetadataOnAcquiredLease() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); using var lease = limiter.AttemptAcquire(1); @@ -583,7 +587,7 @@ public override void MetadataNamesContainsAllMetadata() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); using var lease = limiter.AttemptAcquire(1); @@ -598,7 +602,7 @@ public override async Task DisposeReleasesQueuedAcquires() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 3, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); var lease = limiter.AttemptAcquire(1); @@ -631,7 +635,7 @@ public override async Task DisposeAsyncReleasesQueuedAcquires() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 3, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); var lease = limiter.AttemptAcquire(1); @@ -762,6 +766,22 @@ public async Task AutoReplenish_ReplenishesCounters() Assert.True(lease.IsAcquired); } + public void ReplenishAfterMultiplePeriodsIncreaseTokensBasedOnNumberOfPeriods() + { + var limiter = new FixedWindowRateLimiter(new FixedWindowRateLimiterOptions(70, QueueProcessingOrder.OldestFirst, 1, + TimeSpan.FromMilliseconds(1), autoReplenishment: false)); + Assert.True(limiter.Acquire(50).IsAcquired); + Assert.False(limiter.Acquire(30).IsAcquired); + + Assert.Equal(20, limiter.GetAvailablePermits()); + Replenish(limiter, 2L); + Assert.Equal(70, limiter.GetAvailablePermits()); + + Assert.True(limiter.Acquire(50).IsAcquired); + Replenish(limiter, 5L); + Assert.Equal(70, limiter.GetAvailablePermits()); + } + [Fact] public override async Task CanAcquireResourcesWithAcquireAsyncWithQueuedItemsIfNewestFirst() { @@ -770,7 +790,7 @@ public override async Task CanAcquireResourcesWithAcquireAsyncWithQueuedItemsIfN PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 2, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); @@ -785,7 +805,7 @@ public override async Task CanAcquireResourcesWithAcquireAsyncWithQueuedItemsIfN Assert.True(lease.IsAcquired); Assert.False(wait.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); @@ -799,7 +819,7 @@ public override async Task CannotAcquireResourcesWithAcquireAsyncWithQueuedItems PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 3, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); @@ -811,13 +831,13 @@ public override async Task CannotAcquireResourcesWithAcquireAsyncWithQueuedItems Assert.False(wait.IsCompleted); Assert.False(wait2.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); Assert.False(wait2.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait2; Assert.True(lease.IsAcquired); @@ -831,7 +851,7 @@ public override async Task CanAcquireResourcesWithAcquireWithQueuedItemsIfNewest PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 3, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); @@ -845,7 +865,7 @@ public override async Task CanAcquireResourcesWithAcquireWithQueuedItemsIfNewest Assert.True(lease.IsAcquired); Assert.False(wait.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); @@ -859,7 +879,7 @@ public override async Task CannotAcquireResourcesWithAcquireWithQueuedItemsIfOld PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 3, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); @@ -872,7 +892,7 @@ public override async Task CannotAcquireResourcesWithAcquireWithQueuedItemsIfOld lease = limiter.AttemptAcquire(1); Assert.False(lease.IsAcquired); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); @@ -918,11 +938,11 @@ public override void IdleDurationUpdatesWhenChangingFromActive() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 2, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); limiter.AttemptAcquire(1); - limiter.TryReplenish(); + Replenish(limiter, 1L); Assert.NotNull(limiter.IdleDuration); } @@ -962,7 +982,7 @@ public override async Task CanFillQueueWithNewestFirstAfterCancelingQueuedReques PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 2, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); var lease = limiter.AttemptAcquire(2); @@ -988,7 +1008,7 @@ public override async Task CanFillQueueWithNewestFirstAfterCancelingQueuedReques lease = await wait2; Assert.False(lease.IsAcquired); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait3; Assert.True(lease.IsAcquired); } @@ -1001,7 +1021,7 @@ public override async Task CanDisposeAfterCancelingQueuedRequest() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); var lease = limiter.AttemptAcquire(1); @@ -1151,5 +1171,15 @@ public override void GetStatisticsThrowsAfterDispose() limiter.Dispose(); Assert.Throws(limiter.GetStatistics); } + + private static readonly double TickFrequency = (double)TimeSpan.TicksPerSecond / Stopwatch.Frequency; + + static internal void Replenish(FixedWindowRateLimiter limiter, long addMilliseconds) + { + var replenishInternalMethod = typeof(FixedWindowRateLimiter).GetMethod("ReplenishInternal", Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance)!; + var internalTick = typeof(FixedWindowRateLimiter).GetField("_lastReplenishmentTick", Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance)!; + var currentTick = (long)internalTick.GetValue(limiter); + replenishInternalMethod.Invoke(limiter, new object[] { currentTick + addMilliseconds * (long)(TimeSpan.TicksPerMillisecond / TickFrequency) }); + } } } diff --git a/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs index 7241a39e0f1f4..41f4dc066021b 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs @@ -17,7 +17,7 @@ public override void CanAcquireResource() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -28,8 +28,8 @@ public override void CanAcquireResource() lease.Dispose(); Assert.False(limiter.AttemptAcquire().IsAcquired); - Assert.True(limiter.TryReplenish()); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); + Replenish(limiter, 1L); Assert.True(limiter.AttemptAcquire().IsAcquired); } @@ -77,6 +77,10 @@ public override void InvalidOptionsThrows() SegmentsPerWindow = 1, AutoReplenishment = false })); + Assert.Throws( + () => new SlidingWindowRateLimiterOptions(1, QueueProcessingOrder.NewestFirst, 1, TimeSpan.Zero, 1, autoReplenishment: false)); + Assert.Throws( + () => new SlidingWindowRateLimiterOptions(1, QueueProcessingOrder.NewestFirst, 1, TimeSpan.FromMinutes(-2), 1, autoReplenishment: false)); } [Fact] @@ -87,7 +91,7 @@ public override async Task CanAcquireResourceAsync() PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 4, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -98,14 +102,14 @@ public override async Task CanAcquireResourceAsync() var wait = limiter.AcquireAsync(2); Assert.False(wait.IsCompleted); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.False(wait.IsCompleted); var wait2 = limiter.AcquireAsync(2); Assert.False(wait2.IsCompleted); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.True((await wait2).IsAcquired); } @@ -121,7 +125,7 @@ public async Task CanAcquireMultipleRequestsAsync() PermitLimit = 4, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 4, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 3, AutoReplenishment = false }); @@ -132,19 +136,19 @@ public async Task CanAcquireMultipleRequestsAsync() var wait = limiter.AcquireAsync(3); Assert.False(wait.IsCompleted); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.False(wait.IsCompleted); var wait2 = limiter.AcquireAsync(2); Assert.True(wait2.IsCompleted); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); var wait3 = limiter.AcquireAsync(2); Assert.False(wait3.IsCompleted); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.True((await wait3).IsAcquired); Assert.False((await wait).IsAcquired); @@ -159,7 +163,7 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsOldest() PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 3, - Window = TimeSpan.FromMinutes(0), + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -172,10 +176,10 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsOldest() Assert.False(wait2.IsCompleted); lease.Dispose(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.False(wait1.IsCompleted); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); lease = await wait1; Assert.True(lease.IsAcquired); @@ -183,8 +187,8 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsOldest() lease.Dispose(); Assert.Equal(1, limiter.GetStatistics().CurrentAvailablePermits); - Assert.True(limiter.TryReplenish()); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); + Replenish(limiter, 1L); lease = await wait2; Assert.True(lease.IsAcquired); @@ -198,7 +202,7 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsNewest() PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 3, - Window = TimeSpan.FromMinutes(0), + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -212,10 +216,10 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsNewest() Assert.False(wait2.IsCompleted); lease.Dispose(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.False(wait2.IsCompleted); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); // second queued item completes first with NewestFirst lease = await wait2; Assert.True(lease.IsAcquired); @@ -223,8 +227,8 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsNewest() lease.Dispose(); Assert.Equal(1, limiter.GetStatistics().CurrentAvailablePermits); - Assert.True(limiter.TryReplenish()); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); + Replenish(limiter, 1L); lease = await wait1; Assert.True(lease.IsAcquired); @@ -238,7 +242,7 @@ public override async Task FailsWhenQueuingMoreThanLimit_OldestFirst() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -257,7 +261,7 @@ public override async Task DropsOldestWhenQueuingMoreThanLimit_NewestFirst() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -270,8 +274,8 @@ public override async Task DropsOldestWhenQueuingMoreThanLimit_NewestFirst() Assert.False(lease1.IsAcquired); Assert.False(wait2.IsCompleted); - limiter.TryReplenish(); - limiter.TryReplenish(); + Replenish(limiter, 1L); + Replenish(limiter, 1L); lease = await wait2; Assert.True(lease.IsAcquired); @@ -285,7 +289,7 @@ public override async Task DropsMultipleOldestWhenQueuingMoreThanLimit_NewestFir PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 2, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -304,8 +308,8 @@ public override async Task DropsMultipleOldestWhenQueuingMoreThanLimit_NewestFir Assert.False(lease2.IsAcquired); Assert.False(wait3.IsCompleted); - limiter.TryReplenish(); - limiter.TryReplenish(); + Replenish(limiter, 1L); + Replenish(limiter, 1L); lease = await wait3; Assert.True(lease.IsAcquired); @@ -319,7 +323,7 @@ public override async Task DropsRequestedLeaseIfPermitCountGreaterThanQueueLimit PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -333,8 +337,8 @@ public override async Task DropsRequestedLeaseIfPermitCountGreaterThanQueueLimit var lease1 = await limiter.AcquireAsync(2); Assert.False(lease1.IsAcquired); - limiter.TryReplenish(); - limiter.TryReplenish(); + Replenish(limiter, 1L); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); @@ -348,7 +352,7 @@ public override async Task QueueAvailableAfterQueueLimitHitAndResources_BecomeAv PermitLimit = 3, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 2, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 3, AutoReplenishment = false }); @@ -358,20 +362,20 @@ public override async Task QueueAvailableAfterQueueLimitHitAndResources_BecomeAv var failedLease = await limiter.AcquireAsync(2); Assert.False(failedLease.IsAcquired); - limiter.TryReplenish(); - limiter.TryReplenish(); + Replenish(limiter, 1L); + Replenish(limiter, 1L); Assert.False(wait.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); wait = limiter.AcquireAsync(2); Assert.False(wait.IsCompleted); - limiter.TryReplenish(); - limiter.TryReplenish(); - limiter.TryReplenish(); + Replenish(limiter, 1L); + Replenish(limiter, 1L); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); @@ -385,7 +389,7 @@ public override async Task LargeAcquiresAndQueuesDoNotIntegerOverflow() PermitLimit = int.MaxValue, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = int.MaxValue, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -402,8 +406,8 @@ public override async Task LargeAcquiresAndQueuesDoNotIntegerOverflow() var lease1 = await wait; Assert.False(lease1.IsAcquired); - limiter.TryReplenish(); - limiter.TryReplenish(); + Replenish(limiter, 1L); + Replenish(limiter, 1L); var lease2 = await wait2; Assert.True(lease2.IsAcquired); } @@ -416,7 +420,7 @@ public override void ThrowsWhenAcquiringMoreThanLimit() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 1, AutoReplenishment = false }); @@ -431,7 +435,7 @@ public override async Task ThrowsWhenWaitingForMoreThanLimit() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 1, AutoReplenishment = false }); @@ -446,7 +450,7 @@ public override void ThrowsWhenAcquiringLessThanZero() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 1, AutoReplenishment = false }); @@ -461,7 +465,7 @@ public override async Task ThrowsWhenWaitingForLessThanZero() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 1, AutoReplenishment = false }); @@ -476,7 +480,7 @@ public override void AcquireZero_WithAvailability() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 1, AutoReplenishment = false }); @@ -493,7 +497,7 @@ public override void AcquireZero_WithoutAvailability() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 1, AutoReplenishment = false }); @@ -513,7 +517,7 @@ public override async Task AcquireAsyncZero_WithAvailability() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 1, AutoReplenishment = false }); @@ -530,7 +534,7 @@ public override async Task AcquireAsyncZero_WithoutAvailabilityWaitsForAvailabil PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -541,8 +545,8 @@ public override async Task AcquireAsyncZero_WithoutAvailabilityWaitsForAvailabil Assert.False(wait.IsCompleted); lease.Dispose(); - Assert.True(limiter.TryReplenish()); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); + Replenish(limiter, 1L); using var lease2 = await wait; Assert.True(lease2.IsAcquired); } @@ -555,7 +559,7 @@ public override async Task CanDequeueMultipleResourcesAtOnce() PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 4, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -568,8 +572,8 @@ public override async Task CanDequeueMultipleResourcesAtOnce() Assert.False(wait2.IsCompleted); lease.Dispose(); - Assert.True(limiter.TryReplenish()); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); + Replenish(limiter, 1L); var lease1 = await wait1; var lease2 = await wait2; @@ -585,7 +589,7 @@ public override async Task CanCancelAcquireAsyncAfterQueuing() PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -600,7 +604,7 @@ public override async Task CanCancelAcquireAsyncAfterQueuing() Assert.Equal(cts.Token, ex.CancellationToken); lease.Dispose(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.Equal(0, limiter.GetStatistics().CurrentAvailablePermits); } @@ -613,7 +617,7 @@ public override async Task CanCancelAcquireAsyncBeforeQueuing() PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -627,7 +631,7 @@ public override async Task CanCancelAcquireAsyncBeforeQueuing() Assert.Equal(cts.Token, ex.CancellationToken); lease.Dispose(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.Equal(0, limiter.GetStatistics().CurrentAvailablePermits); } @@ -640,7 +644,7 @@ public override async Task CancelUpdatesQueueLimit() PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -657,8 +661,8 @@ public override async Task CancelUpdatesQueueLimit() wait = limiter.AcquireAsync(1); Assert.False(wait.IsCompleted); - limiter.TryReplenish(); - limiter.TryReplenish(); + Replenish(limiter, 1L); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); @@ -673,7 +677,7 @@ public override void NoMetadataOnAcquiredLease() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -689,7 +693,7 @@ public override void MetadataNamesContainsAllMetadata() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 1, AutoReplenishment = false }); @@ -705,7 +709,7 @@ public override async Task DisposeReleasesQueuedAcquires() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 3, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 1, AutoReplenishment = false }); @@ -739,7 +743,7 @@ public override async Task DisposeAsyncReleasesQueuedAcquires() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 3, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -801,6 +805,34 @@ public async Task AutoReplenish_ReplenishesCounters() Assert.True(lease.IsAcquired); } + [Fact[ + public void ReplenishAfterMultiplePeriodsIncreaseTokensBasedOnNumberOfPeriods() + { + var limiter = new SlidingWindowRateLimiter(new SlidingWindowRateLimiterOptions(70, QueueProcessingOrder.OldestFirst, 1, + TimeSpan.FromMilliseconds(7), 7, autoReplenishment: false)); + Assert.True(limiter.Acquire(50).IsAcquired); + Assert.False(limiter.Acquire(30).IsAcquired); + + Assert.Equal(20, limiter.GetAvailablePermits()); + Replenish(limiter, 2L); + Assert.Equal(20, limiter.GetAvailablePermits()); + Replenish(limiter, 5L); + Assert.Equal(70, limiter.GetAvailablePermits()); + + Assert.True(limiter.Acquire(50).IsAcquired); + Replenish(limiter, 5L); + Assert.Equal(20, limiter.GetAvailablePermits()); + Replenish(limiter, 1L); + Assert.Equal(20, limiter.GetAvailablePermits()); + Replenish(limiter, 1L); + Assert.Equal(70, limiter.GetAvailablePermits()); + + Assert.True(limiter.Acquire(50).IsAcquired); + Assert.Equal(20, limiter.GetAvailablePermits()); + Replenish(limiter, 9L); + Assert.Equal(70, limiter.GetAvailablePermits()); + } + [Fact] public override async Task CanAcquireResourcesWithAcquireAsyncWithQueuedItemsIfNewestFirst() { @@ -809,7 +841,7 @@ public override async Task CanAcquireResourcesWithAcquireAsyncWithQueuedItemsIfN PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 2, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 3, AutoReplenishment = false }); @@ -825,12 +857,12 @@ public override async Task CanAcquireResourcesWithAcquireAsyncWithQueuedItemsIfN Assert.True(lease.IsAcquired); Assert.False(wait.IsCompleted); - limiter.TryReplenish(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); + Replenish(limiter, 1L); Assert.False(wait.IsCompleted); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); } @@ -843,7 +875,7 @@ public override async Task CannotAcquireResourcesWithAcquireAsyncWithQueuedItems PermitLimit = 3, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 5, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -856,18 +888,18 @@ public override async Task CannotAcquireResourcesWithAcquireAsyncWithQueuedItems Assert.False(wait.IsCompleted); Assert.False(wait2.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); Assert.False(wait.IsCompleted); Assert.False(wait2.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); - limiter.TryReplenish(); - limiter.TryReplenish(); + Replenish(limiter, 1L); + Replenish(limiter, 1L); lease = await wait2; Assert.True(lease.IsAcquired); @@ -881,7 +913,7 @@ public override async Task CanAcquireResourcesWithAcquireWithQueuedItemsIfNewest PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 3, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -896,8 +928,8 @@ public override async Task CanAcquireResourcesWithAcquireWithQueuedItemsIfNewest Assert.True(lease.IsAcquired); Assert.False(wait.IsCompleted); - limiter.TryReplenish(); - limiter.TryReplenish(); + Replenish(limiter, 1L); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); @@ -911,7 +943,7 @@ public override async Task CannotAcquireResourcesWithAcquireWithQueuedItemsIfOld PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 3, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -925,8 +957,8 @@ public override async Task CannotAcquireResourcesWithAcquireWithQueuedItemsIfOld lease = limiter.AttemptAcquire(1); Assert.False(lease.IsAcquired); - limiter.TryReplenish(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); @@ -974,13 +1006,13 @@ public override void IdleDurationUpdatesWhenChangingFromActive() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 2, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); limiter.AttemptAcquire(1); - limiter.TryReplenish(); - limiter.TryReplenish(); + Replenish(limiter, 1L); + Replenish(limiter, 1L); Assert.NotNull(limiter.IdleDuration); } @@ -1022,7 +1054,7 @@ public override async Task CanFillQueueWithNewestFirstAfterCancelingQueuedReques PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 2, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -1041,7 +1073,7 @@ public override async Task CanFillQueueWithNewestFirstAfterCancelingQueuedReques Assert.Equal(cts.Token, ex.CancellationToken); lease.Dispose(); - limiter.TryReplenish(); + Replenish(limiter, 1L); var wait3 = limiter.AcquireAsync(2); Assert.False(wait3.IsCompleted); @@ -1050,7 +1082,7 @@ public override async Task CanFillQueueWithNewestFirstAfterCancelingQueuedReques lease = await wait2; Assert.False(lease.IsAcquired); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait3; Assert.True(lease.IsAcquired); } @@ -1063,7 +1095,7 @@ public override async Task CanDisposeAfterCancelingQueuedRequest() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -1215,5 +1247,15 @@ public override void GetStatisticsThrowsAfterDispose() limiter.Dispose(); Assert.Throws(limiter.GetStatistics); } + + private static readonly double TickFrequency = (double)TimeSpan.TicksPerSecond / Stopwatch.Frequency; + + static internal void Replenish(SlidingWindowRateLimiter limiter, long addMilliseconds) + { + var replenishInternalMethod = typeof(SlidingWindowRateLimiter).GetMethod("ReplenishInternal", Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance)!; + var internalTick = typeof(SlidingWindowRateLimiter).GetField("_lastReplenishmentTick", Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance)!; + var currentTick = (long)internalTick.GetValue(limiter); + replenishInternalMethod.Invoke(limiter, new object[] { currentTick + addMilliseconds * (long)(TimeSpan.TicksPerMillisecond / TickFrequency) }); + } } } diff --git a/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs index 272c294a09b34..c55138bf1d082 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs @@ -17,7 +17,7 @@ public override void CanAcquireResource() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -26,9 +26,10 @@ public override void CanAcquireResource() Assert.True(lease.IsAcquired); Assert.False(limiter.AttemptAcquire().IsAcquired); + // Dispose doesn't change token count lease.Dispose(); Assert.False(limiter.AttemptAcquire().IsAcquired); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.True(limiter.AttemptAcquire().IsAcquired); } @@ -76,6 +77,10 @@ public override void InvalidOptionsThrows() TokensPerPeriod = 1, AutoReplenishment = false })); + Assert.Throws( + () => new TokenBucketRateLimiterOptions(1, QueueProcessingOrder.NewestFirst, 1, TimeSpan.Zero, 1, autoReplenishment: false)); + Assert.Throws( + () => new TokenBucketRateLimiterOptions(1, QueueProcessingOrder.NewestFirst, 1, TimeSpan.FromMilliseconds(-1), 1, autoReplenishment: false)); } [Fact] @@ -86,7 +91,7 @@ public override async Task CanAcquireResourceAsync() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -97,7 +102,7 @@ public override async Task CanAcquireResourceAsync() var wait = limiter.AcquireAsync(); Assert.False(wait.IsCompleted); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.True((await wait).IsAcquired); } @@ -110,7 +115,7 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsOldest() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 2, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -123,7 +128,7 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsOldest() Assert.False(wait2.IsCompleted); lease.Dispose(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); lease = await wait1; Assert.True(lease.IsAcquired); @@ -131,7 +136,7 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsOldest() lease.Dispose(); Assert.Equal(0, limiter.GetStatistics().CurrentAvailablePermits); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); lease = await wait2; Assert.True(lease.IsAcquired); @@ -145,7 +150,7 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsNewest() TokenLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 3, - ReplenishmentPeriod = TimeSpan.FromMinutes(0), + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -159,7 +164,7 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsNewest() Assert.False(wait2.IsCompleted); lease.Dispose(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); // second queued item completes first with NewestFirst lease = await wait2; @@ -168,8 +173,9 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsNewest() lease.Dispose(); Assert.Equal(0, limiter.GetStatistics().CurrentAvailablePermits); - Assert.True(limiter.TryReplenish()); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); + Assert.Equal(1, limiter.GetAvailablePermits()); + Replenish(limiter, 1L); lease = await wait1; Assert.True(lease.IsAcquired); @@ -183,7 +189,7 @@ public override async Task FailsWhenQueuingMoreThanLimit_OldestFirst() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -193,7 +199,7 @@ public override async Task FailsWhenQueuingMoreThanLimit_OldestFirst() var failedLease = await limiter.AcquireAsync(1); Assert.False(failedLease.IsAcquired); Assert.True(failedLease.TryGetMetadata(MetadataName.RetryAfter, out var timeSpan)); - Assert.Equal(TimeSpan.Zero, timeSpan); + Assert.Equal(TimeSpan.FromMilliseconds(2), timeSpan); } [Fact] @@ -204,7 +210,7 @@ public override async Task DropsOldestWhenQueuingMoreThanLimit_NewestFirst() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -217,7 +223,7 @@ public override async Task DropsOldestWhenQueuingMoreThanLimit_NewestFirst() Assert.False(lease1.IsAcquired); Assert.False(wait2.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait2; Assert.True(lease.IsAcquired); @@ -231,7 +237,7 @@ public override async Task DropsMultipleOldestWhenQueuingMoreThanLimit_NewestFir TokenLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 2, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -250,8 +256,8 @@ public override async Task DropsMultipleOldestWhenQueuingMoreThanLimit_NewestFir Assert.False(lease2.IsAcquired); Assert.False(wait3.IsCompleted); - limiter.TryReplenish(); - limiter.TryReplenish(); + Replenish(limiter, 1L); + Replenish(limiter, 1L); lease = await wait3; Assert.True(lease.IsAcquired); @@ -265,7 +271,7 @@ public override async Task DropsRequestedLeaseIfPermitCountGreaterThanQueueLimit TokenLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -279,7 +285,7 @@ public override async Task DropsRequestedLeaseIfPermitCountGreaterThanQueueLimit var lease1 = await limiter.AcquireAsync(2); Assert.False(lease1.IsAcquired); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); @@ -293,7 +299,7 @@ public override async Task QueueAvailableAfterQueueLimitHitAndResources_BecomeAv TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -303,14 +309,14 @@ public override async Task QueueAvailableAfterQueueLimitHitAndResources_BecomeAv var failedLease = await limiter.AcquireAsync(1); Assert.False(failedLease.IsAcquired); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); wait = limiter.AcquireAsync(1); Assert.False(wait.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); } @@ -323,7 +329,7 @@ public override async Task LargeAcquiresAndQueuesDoNotIntegerOverflow() TokenLimit = int.MaxValue, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = int.MaxValue, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = int.MaxValue, AutoReplenishment = false }); @@ -340,7 +346,7 @@ public override async Task LargeAcquiresAndQueuesDoNotIntegerOverflow() var lease1 = await wait; Assert.False(lease1.IsAcquired); - limiter.TryReplenish(); + Replenish(limiter, 1L); var lease2 = await wait2; Assert.True(lease2.IsAcquired); } @@ -353,7 +359,7 @@ public override void ThrowsWhenAcquiringMoreThanLimit() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -368,7 +374,7 @@ public override async Task ThrowsWhenWaitingForMoreThanLimit() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -383,7 +389,7 @@ public override void ThrowsWhenAcquiringLessThanZero() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -398,7 +404,7 @@ public override async Task ThrowsWhenWaitingForLessThanZero() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -413,7 +419,7 @@ public override void AcquireZero_WithAvailability() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -430,7 +436,7 @@ public override void AcquireZero_WithoutAvailability() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -450,7 +456,7 @@ public override async Task AcquireAsyncZero_WithAvailability() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -467,7 +473,7 @@ public override async Task AcquireAsyncZero_WithoutAvailabilityWaitsForAvailabil TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -478,7 +484,7 @@ public override async Task AcquireAsyncZero_WithoutAvailabilityWaitsForAvailabil Assert.False(wait.IsCompleted); lease.Dispose(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); using var lease2 = await wait; Assert.True(lease2.IsAcquired); } @@ -491,7 +497,7 @@ public override async Task CanDequeueMultipleResourcesAtOnce() TokenLimit = 2, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 2, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 2, AutoReplenishment = false }); @@ -504,7 +510,7 @@ public override async Task CanDequeueMultipleResourcesAtOnce() Assert.False(wait2.IsCompleted); lease.Dispose(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); var lease1 = await wait1; var lease2 = await wait2; @@ -520,7 +526,7 @@ public override async Task CanCancelAcquireAsyncAfterQueuing() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -535,7 +541,7 @@ public override async Task CanCancelAcquireAsyncAfterQueuing() Assert.Equal(cts.Token, ex.CancellationToken); lease.Dispose(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.Equal(1, limiter.GetStatistics().CurrentAvailablePermits); } @@ -548,7 +554,7 @@ public override async Task CanFillQueueWithNewestFirstAfterCancelingQueuedReques TokenLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 2, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 2, AutoReplenishment = false }); @@ -575,7 +581,7 @@ public override async Task CanFillQueueWithNewestFirstAfterCancelingQueuedReques lease = await wait2; Assert.False(lease.IsAcquired); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait3; Assert.True(lease.IsAcquired); } @@ -588,7 +594,7 @@ public override async Task CanDisposeAfterCancelingQueuedRequest() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -614,7 +620,7 @@ public override async Task CanCancelAcquireAsyncBeforeQueuing() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -628,7 +634,7 @@ public override async Task CanCancelAcquireAsyncBeforeQueuing() Assert.Equal(cts.Token, ex.CancellationToken); lease.Dispose(); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.Equal(1, limiter.GetStatistics().CurrentAvailablePermits); } @@ -641,7 +647,7 @@ public override async Task CancelUpdatesQueueLimit() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -658,7 +664,7 @@ public override async Task CancelUpdatesQueueLimit() wait = limiter.AcquireAsync(1); Assert.False(wait.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); } @@ -671,7 +677,7 @@ public override void NoMetadataOnAcquiredLease() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -687,7 +693,7 @@ public override void MetadataNamesContainsAllMetadata() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -703,7 +709,7 @@ public override async Task DisposeReleasesQueuedAcquires() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 3, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -737,7 +743,7 @@ public override async Task DisposeAsyncReleasesQueuedAcquires() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 3, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); @@ -888,14 +894,14 @@ public async Task CorrectRetryMetadataWithNonZeroAvailableItems() } [Fact] - public void TryReplenishHonorsTokensPerPeriod() + public void ReplenishHonorsTokensPerPeriod() { var limiter = new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions { TokenLimit = 7, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 3, AutoReplenishment = false }); @@ -903,27 +909,42 @@ public void TryReplenishHonorsTokensPerPeriod() Assert.False(limiter.AttemptAcquire(3).IsAcquired); Assert.Equal(2, limiter.GetStatistics().CurrentAvailablePermits); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.Equal(5, limiter.GetStatistics().CurrentAvailablePermits); - Assert.True(limiter.TryReplenish()); + Replenish(limiter, 1L); Assert.Equal(7, limiter.GetStatistics().CurrentAvailablePermits); } [Fact] - public void TryReplenishWithAllTokensAvailable_Noops() + public void ReplenishAfterMultiplePeriodsIncreaseTokensBasedOnNumberOfPeriods() { - var limiter = new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions - { - TokenLimit = 2, - QueueProcessingOrder = QueueProcessingOrder.OldestFirst, - QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, - TokensPerPeriod = 1, - AutoReplenishment = false - }); - Assert.Equal(2, limiter.GetStatistics().CurrentAvailablePermits); - Assert.True(limiter.TryReplenish()); + var limiter = new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions(70, QueueProcessingOrder.OldestFirst, 1, + TimeSpan.FromMilliseconds(1), 3, autoReplenishment: false)); + Assert.True(limiter.Acquire(50).IsAcquired); + Assert.False(limiter.Acquire(30).IsAcquired); + + Assert.Equal(20, limiter.GetAvailablePermits()); + Replenish(limiter, 2L); + Assert.Equal(26, limiter.GetAvailablePermits()); + + Replenish(limiter, 1L); + Assert.Equal(29, limiter.GetAvailablePermits()); + Replenish(limiter, 5L); + Assert.Equal(44, limiter.GetAvailablePermits()); + + Replenish(limiter, 10L); + Assert.Equal(70, limiter.GetAvailablePermits()); + } + + [Fact] + public async void TryReplenishWithAllTokensAvailable_Noops() + { + var limiter = new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions(2, QueueProcessingOrder.OldestFirst, 1, + TimeSpan.FromMilliseconds(30), 1, autoReplenishment: false)); + Assert.Equal(2, limiter.GetAvailablePermits()); + await Task.Delay(100); + limiter.TryReplenish(); Assert.Equal(2, limiter.GetStatistics().CurrentAvailablePermits); } @@ -971,7 +992,7 @@ public override async Task CanAcquireResourcesWithAcquireAsyncWithQueuedItemsIfN TokenLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 2, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 2, AutoReplenishment = false }); @@ -987,7 +1008,7 @@ public override async Task CanAcquireResourcesWithAcquireAsyncWithQueuedItemsIfN Assert.True(lease.IsAcquired); Assert.False(wait.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); @@ -1001,7 +1022,7 @@ public override async Task CannotAcquireResourcesWithAcquireAsyncWithQueuedItems TokenLimit = 2, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 3, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 2, AutoReplenishment = false }); @@ -1014,13 +1035,13 @@ public override async Task CannotAcquireResourcesWithAcquireAsyncWithQueuedItems Assert.False(wait.IsCompleted); Assert.False(wait2.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); Assert.False(wait2.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait2; Assert.True(lease.IsAcquired); @@ -1034,7 +1055,7 @@ public override async Task CanAcquireResourcesWithAcquireWithQueuedItemsIfNewest TokenLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 3, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 2, AutoReplenishment = false }); @@ -1049,7 +1070,7 @@ public override async Task CanAcquireResourcesWithAcquireWithQueuedItemsIfNewest Assert.True(lease.IsAcquired); Assert.False(wait.IsCompleted); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); @@ -1063,7 +1084,7 @@ public override async Task CannotAcquireResourcesWithAcquireWithQueuedItemsIfOld TokenLimit = 2, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 3, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 2, AutoReplenishment = false }); @@ -1077,14 +1098,12 @@ public override async Task CannotAcquireResourcesWithAcquireWithQueuedItemsIfOld lease = limiter.AttemptAcquire(1); Assert.False(lease.IsAcquired); - limiter.TryReplenish(); + Replenish(limiter, 1L); lease = await wait; Assert.True(lease.IsAcquired); } - private static readonly double TickFrequency = (double)TimeSpan.TicksPerSecond / Stopwatch.Frequency; - [Fact] public async Task ReplenishWorksWithTicksOverInt32Max() { @@ -1098,16 +1117,16 @@ public async Task ReplenishWorksWithTicksOverInt32Max() AutoReplenishment = false }); - var lease = limiter.AttemptAcquire(10); + // Ensure next tick is over uint.MaxValue + Replenish(limiter, uint.MaxValue); + + var lease = limiter.AttemtpAcquire(10); Assert.True(lease.IsAcquired); var wait = limiter.AcquireAsync(1); Assert.False(wait.IsCompleted); - var replenishInternalMethod = typeof(TokenBucketRateLimiter).GetMethod("ReplenishInternal", Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance)!; - // Ensure next tick is over uint.MaxValue - var tick = Stopwatch.GetTimestamp() + uint.MaxValue; - replenishInternalMethod.Invoke(limiter, new object[] { tick }); + Replenish(limiter, 2L); lease = await wait; Assert.True(lease.IsAcquired); @@ -1116,11 +1135,11 @@ public async Task ReplenishWorksWithTicksOverInt32Max() Assert.False(wait.IsCompleted); // Tick 1 millisecond too soon and verify that the queued item wasn't completed - replenishInternalMethod.Invoke(limiter, new object[] { tick + 1L * (long)(TimeSpan.TicksPerMillisecond / TickFrequency) }); + Replenish(limiter, 1L); Assert.False(wait.IsCompleted); // ticks would wrap if using uint - replenishInternalMethod.Invoke(limiter, new object[] { tick + 2L * (long)(TimeSpan.TicksPerMillisecond / TickFrequency) }); + Replenish(limiter, 2L); lease = await wait; Assert.True(lease.IsAcquired); } @@ -1167,12 +1186,12 @@ public override void IdleDurationUpdatesWhenChangingFromActive() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 2, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 1, AutoReplenishment = false }); limiter.AttemptAcquire(1); - limiter.TryReplenish(); + Replenish(limiter, 1L); Assert.NotNull(limiter.IdleDuration); } @@ -1214,7 +1233,7 @@ public override void GetStatisticsReturnsNewInstances() TokenLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 2, AutoReplenishment = false }); @@ -1238,7 +1257,7 @@ public override async Task GetStatisticsHasCorrectValues() TokenLimit = 100, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 50, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 30, AutoReplenishment = false }); @@ -1297,7 +1316,7 @@ public override async Task GetStatisticsWithZeroPermitCount() TokenLimit = 100, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 50, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 30, AutoReplenishment = false }); @@ -1331,12 +1350,22 @@ public override void GetStatisticsThrowsAfterDispose() TokenLimit = 100, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 50, - ReplenishmentPeriod = TimeSpan.Zero, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(1), TokensPerPeriod = 30, AutoReplenishment = false }); limiter.Dispose(); Assert.Throws(limiter.GetStatistics); } + + private static readonly double TickFrequency = (double)TimeSpan.TicksPerSecond / Stopwatch.Frequency; + + static internal void Replenish(TokenBucketRateLimiter limiter, long addMilliseconds) + { + var replenishInternalMethod = typeof(TokenBucketRateLimiter).GetMethod("ReplenishInternal", Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance)!; + var internalTick = typeof(TokenBucketRateLimiter).GetField("_lastReplenishmentTick", Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance)!; + var currentTick = (long)internalTick.GetValue(limiter); + replenishInternalMethod.Invoke(limiter, new object[] { currentTick + addMilliseconds * (long)(TimeSpan.TicksPerMillisecond / TickFrequency) }); + } } } From 68a01ca4932372ce60f981fcd3c93e8c146b13df Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Fri, 19 Aug 2022 09:30:21 -0700 Subject: [PATCH 2/8] ignore tick on auto --- .../RateLimiting/FixedWindowRateLimiter.cs | 9 +- .../FixedWindowRateLimiterOptions.cs | 2 +- .../RateLimiting/SlidingWindowRateLimiter.cs | 33 ++-- .../SlidingWindowRateLimiterOptions.cs | 2 +- .../RateLimiting/TokenBucketRateLimiter.cs | 13 +- .../TokenBucketRateLimiterOptions.cs | 2 +- .../tests/FixedWindowRateLimiterTests.cs | 72 +++++--- .../tests/SlidingWindowRateLimiterTests.cs | 121 ++++++++------ .../tests/TokenBucketRateLimiterTests.cs | 158 +++++++++++------- 9 files changed, 240 insertions(+), 172 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs index f04e5ec8fc533..2f273f64b5a73 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs @@ -59,9 +59,9 @@ public FixedWindowRateLimiter(FixedWindowRateLimiterOptions options) { throw new ArgumentException($"{nameof(options.QueueLimit)} must be set to a value greater than or equal to 0.", nameof(options)); } - if (options.Window < TimeSpan.Zero) + if (options.Window <= TimeSpan.Zero) { - throw new ArgumentException($"{nameof(options.Window)} must be set to a value greater than or equal to TimeSpan.Zero.", nameof(options)); + throw new ArgumentException($"{nameof(options.Window)} must be set to a value greater than TimeSpan.Zero.", nameof(options)); } _options = new FixedWindowRateLimiterOptions @@ -287,15 +287,14 @@ private void ReplenishInternal(long nowTicks) return; } - long periods = (long)((nowTicks - _lastReplenishmentTick) * TickFrequency) / _options.Window.Ticks; - if (periods == 0) + if (((nowTicks - _lastReplenishmentTick) * TickFrequency) < _options.Window.Ticks && !_options.AutoReplenishment) { return; } // increment last tick by the number of replenish periods that occurred since the last replenish // this way if replenish isn't being called every ReplenishmentPeriod we correctly track it so we know when replenishes should be occurring - _lastReplenishmentTick += (long)(periods * ReplenishmentPeriod.Ticks / TickFrequency); + _lastReplenishmentTick = nowTicks; int availableRequestCounters = _requestCount; diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiterOptions.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiterOptions.cs index 92cac84012c06..3ebab40439f6b 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiterOptions.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiterOptions.cs @@ -10,7 +10,7 @@ public sealed class FixedWindowRateLimiterOptions { /// /// Specifies the time window that takes in the requests. - /// Must be set to a value >= by the time these options are passed to the constructor of . + /// Must be set to a value > by the time these options are passed to the constructor of . /// public TimeSpan Window { get; set; } = TimeSpan.Zero; diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs index 11d4649ec3bdc..c609ce4d1455e 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs @@ -63,9 +63,9 @@ public SlidingWindowRateLimiter(SlidingWindowRateLimiterOptions options) { throw new ArgumentException($"{nameof(options.QueueLimit)} must be set to a value greater than or equal to 0.", nameof(options)); } - if (options.Window < TimeSpan.Zero) + if (options.Window <= TimeSpan.Zero) { - throw new ArgumentException($"{nameof(options.Window)} must be set to a value greater than or equal to TimeSpan.Zero.", nameof(options)); + throw new ArgumentException($"{nameof(options.Window)} must be set to a value greater than TimeSpan.Zero.", nameof(options)); } _options = new SlidingWindowRateLimiterOptions @@ -289,32 +289,27 @@ private void ReplenishInternal(long nowTicks) return; } - long periods = (long)((nowTicks - _lastReplenishmentTick) * TickFrequency) / ReplenishmentPeriod.Ticks; - if (periods == 0) + if (((nowTicks - _lastReplenishmentTick) * TickFrequency) < ReplenishmentPeriod.Ticks && !_options.AutoReplenishment) { return; } // increment last tick by the number of replenish periods that occurred since the last replenish // this way if replenish isn't being called every ReplenishmentPeriod we correctly track it so we know when replenishes should be occurring - _lastReplenishmentTick += (long)(periods * ReplenishmentPeriod.Ticks / TickFrequency); + _lastReplenishmentTick = nowTicks; int initialRequestCount = _requestCount; - do - { - // Increment the current segment index while move the window - // We need to know the no. of requests that were acquired in a segment previously to ensure that we don't acquire more than the permit limit. - _currentSegmentIndex = (_currentSegmentIndex + 1) % _options.SegmentsPerWindow; - int oldSegmentRequestCount = _requestsPerSegment[_currentSegmentIndex]; - _requestsPerSegment[_currentSegmentIndex] = 0; + // Increment the current segment index while move the window + // We need to know the no. of requests that were acquired in a segment previously to ensure that we don't acquire more than the permit limit. + _currentSegmentIndex = (_currentSegmentIndex + 1) % _options.SegmentsPerWindow; + int oldSegmentRequestCount = _requestsPerSegment[_currentSegmentIndex]; + _requestsPerSegment[_currentSegmentIndex] = 0; - if (oldSegmentRequestCount != 0) - { - _requestCount += oldSegmentRequestCount; - Debug.Assert(_requestCount <= _options.PermitLimit); - } - periods--; - } while (periods > 0); + if (oldSegmentRequestCount != 0) + { + _requestCount += oldSegmentRequestCount; + Debug.Assert(_requestCount <= _options.PermitLimit); + } if (initialRequestCount == _requestCount) { diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs index 8e1d397a57f11..459c44b4d81c6 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs @@ -10,7 +10,7 @@ public sealed class SlidingWindowRateLimiterOptions { /// /// Specifies the minimum period between replenishments. - /// Must be set to a value >= by the time these options are passed to the constructor of . + /// Must be set to a value > by the time these options are passed to the constructor of . /// public TimeSpan Window { get; set; } = TimeSpan.Zero; diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs index d27fc7c9e551d..3266b135a428d 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs @@ -60,9 +60,9 @@ public TokenBucketRateLimiter(TokenBucketRateLimiterOptions options) { throw new ArgumentException($"{nameof(options.QueueLimit)} must be set to a value greater than or equal to 0.", nameof(options)); } - if (options.ReplenishmentPeriod < TimeSpan.Zero) + if (options.ReplenishmentPeriod <= TimeSpan.Zero) { - throw new ArgumentException($"{nameof(options.ReplenishmentPeriod)} must be set to a value greater than or equal to TimeSpan.Zero.", nameof(options)); + throw new ArgumentException($"{nameof(options.ReplenishmentPeriod)} must be set to a value greater than TimeSpan.Zero.", nameof(options)); } _options = new TokenBucketRateLimiterOptions @@ -289,15 +289,12 @@ private void ReplenishInternal(long nowTicks) return; } - long periods = (long)((nowTicks - _lastReplenishmentTick) * TickFrequency) / _options.ReplenishmentPeriod.Ticks; - if (periods == 0) + if (((nowTicks - _lastReplenishmentTick) * TickFrequency) < _options.ReplenishmentPeriod.Ticks && !_options.AutoReplenishment) { return; } - // increment last tick by the number of replenish periods that occurred since the last replenish - // this way if replenish isn't being called every ReplenishmentPeriod we correctly track it so we know when replenishes should be occurring - _lastReplenishmentTick += (long)(periods * _options.ReplenishmentPeriod.Ticks / TickFrequency); + _lastReplenishmentTick = nowTicks; int availablePermits = _tokenCount; int maxPermits = _options.TokenLimit; @@ -305,7 +302,7 @@ private void ReplenishInternal(long nowTicks) if (availablePermits < maxPermits) { - resourcesToAdd = (int)Math.Min(_options.TokensPerPeriod * periods, maxPermits - availablePermits); + resourcesToAdd = Math.Min(maxPermits - availablePermits, _options.TokensPerPeriod); } else { diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiterOptions.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiterOptions.cs index 55b63f65d36bc..451228c0683fa 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiterOptions.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiterOptions.cs @@ -10,7 +10,7 @@ public sealed class TokenBucketRateLimiterOptions { /// /// Specifies the minimum period between replenishments. - /// Must be set to a value >= by the time these options are passed to the constructor of . + /// Must be set to a value > by the time these options are passed to the constructor of . /// public TimeSpan ReplenishmentPeriod { get; set; } = TimeSpan.Zero; diff --git a/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs index 243613c17f5ed..a74b1001ea8d3 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs @@ -62,10 +62,24 @@ public override void InvalidOptionsThrows() Window = TimeSpan.MinValue, AutoReplenishment = false })); - Assert.Throws( - () => new FixedWindowRateLimiterOptions(1, QueueProcessingOrder.NewestFirst, 1, TimeSpan.FromMinutes(-2), autoReplenishment: false)); - Assert.Throws( - () => new FixedWindowRateLimiterOptions(1, QueueProcessingOrder.NewestFirst, 1, TimeSpan.Zero, autoReplenishment: false)); + Assert.Throws( + () => new FixedWindowRateLimiter(new FixedWindowRateLimiterOptions + { + PermitLimit = 1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = 1, + Window = TimeSpan.FromMinutes(-2), + AutoReplenishment = false, + })); + Assert.Throws( + () => new FixedWindowRateLimiter(new FixedWindowRateLimiterOptions + { + PermitLimit = 1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = 1, + Window = TimeSpan.Zero, + AutoReplenishment = false + })); } [Fact] @@ -766,22 +780,6 @@ public async Task AutoReplenish_ReplenishesCounters() Assert.True(lease.IsAcquired); } - public void ReplenishAfterMultiplePeriodsIncreaseTokensBasedOnNumberOfPeriods() - { - var limiter = new FixedWindowRateLimiter(new FixedWindowRateLimiterOptions(70, QueueProcessingOrder.OldestFirst, 1, - TimeSpan.FromMilliseconds(1), autoReplenishment: false)); - Assert.True(limiter.Acquire(50).IsAcquired); - Assert.False(limiter.Acquire(30).IsAcquired); - - Assert.Equal(20, limiter.GetAvailablePermits()); - Replenish(limiter, 2L); - Assert.Equal(70, limiter.GetAvailablePermits()); - - Assert.True(limiter.Acquire(50).IsAcquired); - Replenish(limiter, 5L); - Assert.Equal(70, limiter.GetAvailablePermits()); - } - [Fact] public override async Task CanAcquireResourcesWithAcquireAsyncWithQueuedItemsIfNewestFirst() { @@ -1046,7 +1044,7 @@ public override void GetStatisticsReturnsNewInstances() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); @@ -1069,7 +1067,7 @@ public override async Task GetStatisticsHasCorrectValues() PermitLimit = 100, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 50, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); @@ -1113,7 +1111,7 @@ public override async Task GetStatisticsHasCorrectValues() Assert.Equal(2, stats.TotalFailedLeases); Assert.Equal(1, stats.TotalSuccessfulLeases); - limiter.TryReplenish(); + Replenish(limiter, 1); await lease2Task; // success from wait + available + queue @@ -1132,7 +1130,7 @@ public override async Task GetStatisticsWithZeroPermitCount() PermitLimit = 100, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 50, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); var lease = limiter.AttemptAcquire(0); @@ -1165,13 +1163,37 @@ public override void GetStatisticsThrowsAfterDispose() PermitLimit = 100, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 50, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(1), AutoReplenishment = false }); limiter.Dispose(); Assert.Throws(limiter.GetStatistics); } + [Fact] + public void AutoReplenishIgnoresTimerJitter() + { + var replenishmentPeriod = TimeSpan.FromMinutes(10); + using var limiter = new FixedWindowRateLimiter(new FixedWindowRateLimiterOptions + { + PermitLimit = 10, + QueueProcessingOrder = QueueProcessingOrder.OldestFirst, + QueueLimit = 1, + Window = replenishmentPeriod, + AutoReplenishment = true, + }); + + var lease = limiter.AttemptAcquire(permitCount: 3); + Assert.True(lease.IsAcquired); + + Assert.Equal(7, limiter.GetStatistics().CurrentAvailablePermits); + + // Replenish 1 millisecond less than ReplenishmentPeriod while AutoReplenishment is enabled + Replenish(limiter, (long)replenishmentPeriod.TotalMilliseconds - 1); + + Assert.Equal(10, limiter.GetStatistics().CurrentAvailablePermits); + } + private static readonly double TickFrequency = (double)TimeSpan.TicksPerSecond / Stopwatch.Frequency; static internal void Replenish(FixedWindowRateLimiter limiter, long addMilliseconds) diff --git a/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs index 41f4dc066021b..ce42825446bac 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs @@ -77,10 +77,26 @@ public override void InvalidOptionsThrows() SegmentsPerWindow = 1, AutoReplenishment = false })); - Assert.Throws( - () => new SlidingWindowRateLimiterOptions(1, QueueProcessingOrder.NewestFirst, 1, TimeSpan.Zero, 1, autoReplenishment: false)); - Assert.Throws( - () => new SlidingWindowRateLimiterOptions(1, QueueProcessingOrder.NewestFirst, 1, TimeSpan.FromMinutes(-2), 1, autoReplenishment: false)); + Assert.Throws( + () => new SlidingWindowRateLimiter(new SlidingWindowRateLimiterOptions + { + PermitLimit = 1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = 1, + Window = TimeSpan.Zero, + SegmentsPerWindow = 1, + AutoReplenishment = false, + })); + Assert.Throws( + () => new SlidingWindowRateLimiter(new SlidingWindowRateLimiterOptions + { + PermitLimit = 1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = 1, + Window = TimeSpan.FromMinutes(-2), + SegmentsPerWindow = 1, + AutoReplenishment = false + })); } [Fact] @@ -91,7 +107,7 @@ public override async Task CanAcquireResourceAsync() PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 4, - Window = TimeSpan.FromMilliseconds(1), + Window = TimeSpan.FromMilliseconds(2), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -125,7 +141,7 @@ public async Task CanAcquireMultipleRequestsAsync() PermitLimit = 4, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 4, - Window = TimeSpan.FromMilliseconds(1), + Window = TimeSpan.FromMilliseconds(3), SegmentsPerWindow = 3, AutoReplenishment = false }); @@ -163,7 +179,7 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsOldest() PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 3, - Window = TimeSpan.FromMilliseconds(1), + Window = TimeSpan.FromMilliseconds(2), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -202,7 +218,7 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsNewest() PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 3, - Window = TimeSpan.FromMilliseconds(1), + Window = TimeSpan.FromMilliseconds(2), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -352,7 +368,7 @@ public override async Task QueueAvailableAfterQueueLimitHitAndResources_BecomeAv PermitLimit = 3, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 2, - Window = TimeSpan.FromMilliseconds(1), + Window = TimeSpan.FromMilliseconds(3), SegmentsPerWindow = 3, AutoReplenishment = false }); @@ -589,7 +605,7 @@ public override async Task CanCancelAcquireAsyncAfterQueuing() PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.FromMilliseconds(1), + Window = TimeSpan.FromMilliseconds(2), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -617,7 +633,7 @@ public override async Task CanCancelAcquireAsyncBeforeQueuing() PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.FromMilliseconds(1), + Window = TimeSpan.FromMilliseconds(2), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -644,7 +660,7 @@ public override async Task CancelUpdatesQueueLimit() PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.FromMilliseconds(1), + Window = TimeSpan.FromMilliseconds(2), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -805,34 +821,6 @@ public async Task AutoReplenish_ReplenishesCounters() Assert.True(lease.IsAcquired); } - [Fact[ - public void ReplenishAfterMultiplePeriodsIncreaseTokensBasedOnNumberOfPeriods() - { - var limiter = new SlidingWindowRateLimiter(new SlidingWindowRateLimiterOptions(70, QueueProcessingOrder.OldestFirst, 1, - TimeSpan.FromMilliseconds(7), 7, autoReplenishment: false)); - Assert.True(limiter.Acquire(50).IsAcquired); - Assert.False(limiter.Acquire(30).IsAcquired); - - Assert.Equal(20, limiter.GetAvailablePermits()); - Replenish(limiter, 2L); - Assert.Equal(20, limiter.GetAvailablePermits()); - Replenish(limiter, 5L); - Assert.Equal(70, limiter.GetAvailablePermits()); - - Assert.True(limiter.Acquire(50).IsAcquired); - Replenish(limiter, 5L); - Assert.Equal(20, limiter.GetAvailablePermits()); - Replenish(limiter, 1L); - Assert.Equal(20, limiter.GetAvailablePermits()); - Replenish(limiter, 1L); - Assert.Equal(70, limiter.GetAvailablePermits()); - - Assert.True(limiter.Acquire(50).IsAcquired); - Assert.Equal(20, limiter.GetAvailablePermits()); - Replenish(limiter, 9L); - Assert.Equal(70, limiter.GetAvailablePermits()); - } - [Fact] public override async Task CanAcquireResourcesWithAcquireAsyncWithQueuedItemsIfNewestFirst() { @@ -841,7 +829,7 @@ public override async Task CanAcquireResourcesWithAcquireAsyncWithQueuedItemsIfN PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 2, - Window = TimeSpan.FromMilliseconds(1), + Window = TimeSpan.FromMilliseconds(3), SegmentsPerWindow = 3, AutoReplenishment = false }); @@ -875,7 +863,7 @@ public override async Task CannotAcquireResourcesWithAcquireAsyncWithQueuedItems PermitLimit = 3, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 5, - Window = TimeSpan.FromMilliseconds(1), + Window = TimeSpan.FromMilliseconds(2), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -1054,7 +1042,7 @@ public override async Task CanFillQueueWithNewestFirstAfterCancelingQueuedReques PermitLimit = 2, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 2, - Window = TimeSpan.FromMilliseconds(1), + Window = TimeSpan.FromMilliseconds(2), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -1121,7 +1109,7 @@ public override void GetStatisticsReturnsNewInstances() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(2), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -1145,7 +1133,7 @@ public override async Task GetStatisticsHasCorrectValues() PermitLimit = 100, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 50, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(2), SegmentsPerWindow = 2, AutoReplenishment = false }); @@ -1170,7 +1158,7 @@ public override async Task GetStatisticsHasCorrectValues() Assert.Equal(0, stats.TotalFailedLeases); Assert.Equal(1, stats.TotalSuccessfulLeases); - limiter.TryReplenish(); + Replenish(limiter, 1); var lease3 = await limiter.AcquireAsync(1); Assert.False(lease3.IsAcquired); @@ -1188,7 +1176,7 @@ public override async Task GetStatisticsHasCorrectValues() Assert.Equal(2, stats.TotalFailedLeases); Assert.Equal(1, stats.TotalSuccessfulLeases); - limiter.TryReplenish(); + Replenish(limiter, 1); await lease2Task; stats = limiter.GetStatistics(); @@ -1206,7 +1194,7 @@ public override async Task GetStatisticsWithZeroPermitCount() PermitLimit = 100, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 50, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(3), SegmentsPerWindow = 3, AutoReplenishment = false }); @@ -1240,7 +1228,7 @@ public override void GetStatisticsThrowsAfterDispose() PermitLimit = 100, QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 50, - Window = TimeSpan.Zero, + Window = TimeSpan.FromMilliseconds(3), SegmentsPerWindow = 3, AutoReplenishment = false }); @@ -1248,6 +1236,41 @@ public override void GetStatisticsThrowsAfterDispose() Assert.Throws(limiter.GetStatistics); } + [Fact] + public void AutoReplenishIgnoresTimerJitter() + { + var replenishmentPeriod = TimeSpan.FromMinutes(10); + using var limiter = new SlidingWindowRateLimiter(new SlidingWindowRateLimiterOptions + { + PermitLimit = 10, + QueueProcessingOrder = QueueProcessingOrder.OldestFirst, + QueueLimit = 1, + Window = replenishmentPeriod, + SegmentsPerWindow = 2, + AutoReplenishment = true, + }); + + var lease = limiter.AttemptAcquire(permitCount: 3); + Assert.True(lease.IsAcquired); + + Assert.Equal(7, limiter.GetStatistics().CurrentAvailablePermits); + + // Replenish 1 millisecond less than ReplenishmentPeriod while AutoReplenishment is enabled + Replenish(limiter, (long)replenishmentPeriod.TotalMilliseconds / 2 - 1); + + Assert.Equal(7, limiter.GetStatistics().CurrentAvailablePermits); + + lease = limiter.AttemptAcquire(permitCount: 3); + Assert.True(lease.IsAcquired); + + Assert.Equal(4, limiter.GetStatistics().CurrentAvailablePermits); + + // Replenish 1 millisecond longer than ReplenishmentPeriod while AutoReplenishment is enabled + Replenish(limiter, (long)replenishmentPeriod.TotalMilliseconds / 2 + 1); + + Assert.Equal(7, limiter.GetStatistics().CurrentAvailablePermits); + } + private static readonly double TickFrequency = (double)TimeSpan.TicksPerSecond / Stopwatch.Frequency; static internal void Replenish(SlidingWindowRateLimiter limiter, long addMilliseconds) diff --git a/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs index c55138bf1d082..596b157078932 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs @@ -39,48 +39,64 @@ public override void InvalidOptionsThrows() { Assert.Throws( () => new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions - { - TokenLimit = -1, - QueueProcessingOrder = QueueProcessingOrder.NewestFirst, - QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.FromMinutes(2), - TokensPerPeriod = 1, - AutoReplenishment = false - })); + { + TokenLimit = -1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = 1, + ReplenishmentPeriod = TimeSpan.FromMinutes(2), + TokensPerPeriod = 1, + AutoReplenishment = false + })); Assert.Throws( () => new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions - { - TokenLimit = 1, - QueueProcessingOrder = QueueProcessingOrder.NewestFirst, - QueueLimit = -1, - ReplenishmentPeriod = TimeSpan.FromMinutes(2), - TokensPerPeriod = 1, - AutoReplenishment = false - })); + { + TokenLimit = 1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = -1, + ReplenishmentPeriod = TimeSpan.FromMinutes(2), + TokensPerPeriod = 1, + AutoReplenishment = false + })); Assert.Throws( () => new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions - { - TokenLimit = 1, - QueueProcessingOrder = QueueProcessingOrder.NewestFirst, - QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.FromMinutes(2), - TokensPerPeriod = -1, - AutoReplenishment = false - })); + { + TokenLimit = 1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = 1, + ReplenishmentPeriod = TimeSpan.FromMinutes(2), + TokensPerPeriod = -1, + AutoReplenishment = false + })); Assert.Throws( () => new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions - { - TokenLimit = 1, - QueueProcessingOrder = QueueProcessingOrder.NewestFirst, - QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.MinValue, - TokensPerPeriod = 1, - AutoReplenishment = false - })); - Assert.Throws( - () => new TokenBucketRateLimiterOptions(1, QueueProcessingOrder.NewestFirst, 1, TimeSpan.Zero, 1, autoReplenishment: false)); - Assert.Throws( - () => new TokenBucketRateLimiterOptions(1, QueueProcessingOrder.NewestFirst, 1, TimeSpan.FromMilliseconds(-1), 1, autoReplenishment: false)); + { + TokenLimit = 1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = 1, + ReplenishmentPeriod = TimeSpan.MinValue, + TokensPerPeriod = 1, + AutoReplenishment = false + })); + Assert.Throws( + () => new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions + { + TokenLimit = 1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = 1, + ReplenishmentPeriod = TimeSpan.Zero, + TokensPerPeriod = 1, + AutoReplenishment = false + })); + Assert.Throws( + () => new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions + { + TokenLimit = 1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = 1, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(-1), + TokensPerPeriod = 1, + AutoReplenishment = false + })); } [Fact] @@ -174,7 +190,7 @@ public override async Task CanAcquireResourceAsync_QueuesAndGrabsNewest() lease.Dispose(); Assert.Equal(0, limiter.GetStatistics().CurrentAvailablePermits); Replenish(limiter, 1L); - Assert.Equal(1, limiter.GetAvailablePermits()); + Assert.Equal(1, limiter.GetStatistics().CurrentAvailablePermits); Replenish(limiter, 1L); lease = await wait1; @@ -916,33 +932,19 @@ public void ReplenishHonorsTokensPerPeriod() Assert.Equal(7, limiter.GetStatistics().CurrentAvailablePermits); } - [Fact] - public void ReplenishAfterMultiplePeriodsIncreaseTokensBasedOnNumberOfPeriods() - { - var limiter = new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions(70, QueueProcessingOrder.OldestFirst, 1, - TimeSpan.FromMilliseconds(1), 3, autoReplenishment: false)); - Assert.True(limiter.Acquire(50).IsAcquired); - Assert.False(limiter.Acquire(30).IsAcquired); - - Assert.Equal(20, limiter.GetAvailablePermits()); - Replenish(limiter, 2L); - Assert.Equal(26, limiter.GetAvailablePermits()); - - Replenish(limiter, 1L); - Assert.Equal(29, limiter.GetAvailablePermits()); - Replenish(limiter, 5L); - Assert.Equal(44, limiter.GetAvailablePermits()); - - Replenish(limiter, 10L); - Assert.Equal(70, limiter.GetAvailablePermits()); - } - [Fact] public async void TryReplenishWithAllTokensAvailable_Noops() { - var limiter = new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions(2, QueueProcessingOrder.OldestFirst, 1, - TimeSpan.FromMilliseconds(30), 1, autoReplenishment: false)); - Assert.Equal(2, limiter.GetAvailablePermits()); + var limiter = new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions + { + TokenLimit = 2, + QueueProcessingOrder = QueueProcessingOrder.OldestFirst, + QueueLimit = 1, + ReplenishmentPeriod = TimeSpan.FromMilliseconds(30), + TokensPerPeriod = 1, + AutoReplenishment = false + }); + Assert.Equal(2, limiter.GetStatistics().CurrentAvailablePermits); await Task.Delay(100); limiter.TryReplenish(); Assert.Equal(2, limiter.GetStatistics().CurrentAvailablePermits); @@ -1120,7 +1122,7 @@ public async Task ReplenishWorksWithTicksOverInt32Max() // Ensure next tick is over uint.MaxValue Replenish(limiter, uint.MaxValue); - var lease = limiter.AttemtpAcquire(10); + var lease = limiter.AttemptAcquire(10); Assert.True(lease.IsAcquired); var wait = limiter.AcquireAsync(1); @@ -1298,7 +1300,7 @@ public override async Task GetStatisticsHasCorrectValues() Assert.Equal(2, stats.TotalFailedLeases); Assert.Equal(1, stats.TotalSuccessfulLeases); - limiter.TryReplenish(); + Replenish(limiter, 1); await lease2Task; stats = limiter.GetStatistics(); @@ -1358,6 +1360,36 @@ public override void GetStatisticsThrowsAfterDispose() Assert.Throws(limiter.GetStatistics); } + [Fact] + public void AutoReplenishIgnoresTimerJitter() + { + var replenishmentPeriod = TimeSpan.FromMinutes(10); + using var limiter = new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions + { + TokenLimit = 10, + QueueProcessingOrder = QueueProcessingOrder.OldestFirst, + QueueLimit = 1, + ReplenishmentPeriod = replenishmentPeriod, + AutoReplenishment = true, + TokensPerPeriod = 1, + }); + + var lease = limiter.AttemptAcquire(permitCount: 3); + Assert.True(lease.IsAcquired); + + Assert.Equal(7, limiter.GetStatistics().CurrentAvailablePermits); + + // Replenish 1 millisecond less than ReplenishmentPeriod while AutoReplenishment is enabled + Replenish(limiter, (long)replenishmentPeriod.TotalMilliseconds - 1); + + Assert.Equal(8, limiter.GetStatistics().CurrentAvailablePermits); + + // Replenish 1 millisecond longer than ReplenishmentPeriod while AutoReplenishment is enabled + Replenish(limiter, (long)replenishmentPeriod.TotalMilliseconds + 1); + + Assert.Equal(9, limiter.GetStatistics().CurrentAvailablePermits); + } + private static readonly double TickFrequency = (double)TimeSpan.TicksPerSecond / Stopwatch.Frequency; static internal void Replenish(TokenBucketRateLimiter limiter, long addMilliseconds) From 3a169d413559e948c8d99840b86419cbd9f52a5f Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Fri, 19 Aug 2022 09:34:37 -0700 Subject: [PATCH 3/8] fixup --- .../RateLimiting/FixedWindowRateLimiter.cs | 2 -- .../RateLimiting/SlidingWindowRateLimiter.cs | 15 ++++----------- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs index 2f273f64b5a73..780eddd2cee2e 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs @@ -292,8 +292,6 @@ private void ReplenishInternal(long nowTicks) return; } - // increment last tick by the number of replenish periods that occurred since the last replenish - // this way if replenish isn't being called every ReplenishmentPeriod we correctly track it so we know when replenishes should be occurring _lastReplenishmentTick = nowTicks; int availableRequestCounters = _requestCount; diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs index c609ce4d1455e..5dfc36914487e 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs @@ -294,29 +294,22 @@ private void ReplenishInternal(long nowTicks) return; } - // increment last tick by the number of replenish periods that occurred since the last replenish - // this way if replenish isn't being called every ReplenishmentPeriod we correctly track it so we know when replenishes should be occurring _lastReplenishmentTick = nowTicks; - int initialRequestCount = _requestCount; // Increment the current segment index while move the window // We need to know the no. of requests that were acquired in a segment previously to ensure that we don't acquire more than the permit limit. _currentSegmentIndex = (_currentSegmentIndex + 1) % _options.SegmentsPerWindow; int oldSegmentRequestCount = _requestsPerSegment[_currentSegmentIndex]; _requestsPerSegment[_currentSegmentIndex] = 0; - if (oldSegmentRequestCount != 0) + if (oldSegmentRequestCount == 0) { - _requestCount += oldSegmentRequestCount; - Debug.Assert(_requestCount <= _options.PermitLimit); - } - - if (initialRequestCount == _requestCount) - { - // no requests added, queued items don't need updating return; } + _requestCount += oldSegmentRequestCount; + Debug.Assert(_requestCount <= _options.PermitLimit); + // Process queued requests while (_queue.Count > 0) { From af637b0906f87b791e0916e62a897acac6630ee6 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Mon, 22 Aug 2022 09:50:39 -0700 Subject: [PATCH 4/8] partial --- .../RateLimiting/TokenBucketRateLimiter.cs | 32 +++++++++---------- .../tests/TokenBucketRateLimiterTests.cs | 6 ++-- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs index 3266b135a428d..bca282a9ce866 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs @@ -13,7 +13,7 @@ namespace System.Threading.RateLimiting /// public sealed class TokenBucketRateLimiter : ReplenishingRateLimiter { - private int _tokenCount; + private double _tokenCount; private int _queueCount; private long _lastReplenishmentTick; private long? _idleSince; @@ -22,6 +22,7 @@ public sealed class TokenBucketRateLimiter : ReplenishingRateLimiter private long _failedLeasesCount; private long _successfulLeasesCount; + private readonly double _fillRate; private readonly Timer? _renewTimer; private readonly TokenBucketRateLimiterOptions _options; private readonly Deque _queue = new Deque(); @@ -76,6 +77,7 @@ public TokenBucketRateLimiter(TokenBucketRateLimiterOptions options) }; _tokenCount = options.TokenLimit; + _fillRate = (double)options.TokensPerPeriod / options.ReplenishmentPeriod.Ticks; _idleSince = _lastReplenishmentTick = Stopwatch.GetTimestamp(); @@ -91,7 +93,7 @@ public TokenBucketRateLimiter(TokenBucketRateLimiterOptions options) ThrowIfDisposed(); return new RateLimiterStatistics() { - CurrentAvailablePermits = _tokenCount, + CurrentAvailablePermits = (long)_tokenCount, CurrentQueuedCount = _queueCount, TotalFailedLeases = Interlocked.Read(ref _failedLeasesCount), TotalSuccessfulLeases = Interlocked.Read(ref _successfulLeasesCount), @@ -210,7 +212,7 @@ protected override ValueTask AcquireAsyncCore(int tokenCount, Ca private RateLimitLease CreateFailedTokenLease(int tokenCount) { - int replenishAmount = tokenCount - _tokenCount + _queueCount; + int replenishAmount = tokenCount - (int)_tokenCount + _queueCount; // can't have 0 replenish periods, that would mean it should be a successful lease // if TokensPerPeriod is larger than the replenishAmount needed then it would be 0 Debug.Assert(_options.TokensPerPeriod > 0); @@ -289,31 +291,27 @@ private void ReplenishInternal(long nowTicks) return; } - if (((nowTicks - _lastReplenishmentTick) * TickFrequency) < _options.ReplenishmentPeriod.Ticks && !_options.AutoReplenishment) + if (_tokenCount == _options.TokenLimit) { return; } - _lastReplenishmentTick = nowTicks; - - int availablePermits = _tokenCount; - int maxPermits = _options.TokenLimit; - int resourcesToAdd; + var add = _fillRate * (nowTicks - _lastReplenishmentTick) * TickFrequency; - if (availablePermits < maxPermits) + // special case the scenario when TokenLimit is 1 so that it doesn't potentially take two timer calls to fully update the token by 1 + // other limits are hit by this, but they are a lot smoother in practice so it isn't as bad that two timer calls may be needed + if (_options.TokenLimit == 1 && _options.AutoReplenishment) { - resourcesToAdd = Math.Min(maxPermits - availablePermits, _options.TokensPerPeriod); - } - else - { - // All tokens available, nothing to do - return; + add = 1; } + _tokenCount = Math.Min(_options.TokenLimit, _tokenCount + add); + + _lastReplenishmentTick = nowTicks; + // Process queued requests Deque queue = _queue; - _tokenCount += resourcesToAdd; Debug.Assert(_tokenCount <= _options.TokenLimit); while (queue.Count > 0) { diff --git a/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs index 596b157078932..55e13feff4ab1 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs @@ -1361,7 +1361,7 @@ public override void GetStatisticsThrowsAfterDispose() } [Fact] - public void AutoReplenishIgnoresTimerJitter() + public void AutoReplenishPreservesTimeWithTimerJitter() { var replenishmentPeriod = TimeSpan.FromMinutes(10); using var limiter = new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions @@ -1382,7 +1382,9 @@ public void AutoReplenishIgnoresTimerJitter() // Replenish 1 millisecond less than ReplenishmentPeriod while AutoReplenishment is enabled Replenish(limiter, (long)replenishmentPeriod.TotalMilliseconds - 1); - Assert.Equal(8, limiter.GetStatistics().CurrentAvailablePermits); + // Timer ran faster than ReplenishmentPeriod so a full token wasn't added. + // Internally the limiter is tracking that so the next timer call will add to the previous partial token + Assert.Equal(7, limiter.GetStatistics().CurrentAvailablePermits); // Replenish 1 millisecond longer than ReplenishmentPeriod while AutoReplenishment is enabled Replenish(limiter, (long)replenishmentPeriod.TotalMilliseconds + 1); From 353679089a0a55045ea1a8fb29b0fbb83e966099 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Tue, 23 Aug 2022 09:02:56 -0700 Subject: [PATCH 5/8] allow TimeSpan.Zero --- .../RateLimiting/FixedWindowRateLimiter.cs | 4 +- .../FixedWindowRateLimiterOptions.cs | 3 +- .../RateLimiting/SlidingWindowRateLimiter.cs | 4 +- .../SlidingWindowRateLimiterOptions.cs | 3 +- .../RateLimiting/TokenBucketRateLimiter.cs | 4 +- .../TokenBucketRateLimiterOptions.cs | 3 +- .../tests/FixedWindowRateLimiterTests.cs | 45 ++++++-------- .../tests/SlidingWindowRateLimiterTests.cs | 62 ++++++++----------- .../tests/TokenBucketRateLimiterTests.cs | 10 --- 9 files changed, 56 insertions(+), 82 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs index 780eddd2cee2e..774a6876c245e 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs @@ -59,9 +59,9 @@ public FixedWindowRateLimiter(FixedWindowRateLimiterOptions options) { throw new ArgumentException($"{nameof(options.QueueLimit)} must be set to a value greater than or equal to 0.", nameof(options)); } - if (options.Window <= TimeSpan.Zero) + if (options.Window < TimeSpan.Zero) { - throw new ArgumentException($"{nameof(options.Window)} must be set to a value greater than TimeSpan.Zero.", nameof(options)); + throw new ArgumentException($"{nameof(options.Window)} must be set to a value greater than or equal to TimeSpan.Zero.", nameof(options)); } _options = new FixedWindowRateLimiterOptions diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiterOptions.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiterOptions.cs index 3ebab40439f6b..a6d2b16475552 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiterOptions.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiterOptions.cs @@ -10,8 +10,9 @@ public sealed class FixedWindowRateLimiterOptions { /// /// Specifies the time window that takes in the requests. - /// Must be set to a value > by the time these options are passed to the constructor of . + /// Must be set to a value >= by the time these options are passed to the constructor of . /// + /// means the limiter will never replenish. public TimeSpan Window { get; set; } = TimeSpan.Zero; /// diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs index 5dfc36914487e..a1fe3e2839ed2 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs @@ -63,9 +63,9 @@ public SlidingWindowRateLimiter(SlidingWindowRateLimiterOptions options) { throw new ArgumentException($"{nameof(options.QueueLimit)} must be set to a value greater than or equal to 0.", nameof(options)); } - if (options.Window <= TimeSpan.Zero) + if (options.Window < TimeSpan.Zero) { - throw new ArgumentException($"{nameof(options.Window)} must be set to a value greater than TimeSpan.Zero.", nameof(options)); + throw new ArgumentException($"{nameof(options.Window)} must be set to a value greater than or equal to TimeSpan.Zero.", nameof(options)); } _options = new SlidingWindowRateLimiterOptions diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs index 459c44b4d81c6..f7f399e175b1f 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs @@ -10,8 +10,9 @@ public sealed class SlidingWindowRateLimiterOptions { /// /// Specifies the minimum period between replenishments. - /// Must be set to a value > by the time these options are passed to the constructor of . + /// Must be set to a value >= by the time these options are passed to the constructor of . /// + /// means the limiter will never replenish. public TimeSpan Window { get; set; } = TimeSpan.Zero; /// diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs index bca282a9ce866..5faa59c7731ea 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs @@ -61,9 +61,9 @@ public TokenBucketRateLimiter(TokenBucketRateLimiterOptions options) { throw new ArgumentException($"{nameof(options.QueueLimit)} must be set to a value greater than or equal to 0.", nameof(options)); } - if (options.ReplenishmentPeriod <= TimeSpan.Zero) + if (options.ReplenishmentPeriod < TimeSpan.Zero) { - throw new ArgumentException($"{nameof(options.ReplenishmentPeriod)} must be set to a value greater than TimeSpan.Zero.", nameof(options)); + throw new ArgumentException($"{nameof(options.ReplenishmentPeriod)} must be set to a value greater than or equal to TimeSpan.Zero.", nameof(options)); } _options = new TokenBucketRateLimiterOptions diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiterOptions.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiterOptions.cs index 451228c0683fa..b0371119ce3c0 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiterOptions.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiterOptions.cs @@ -10,8 +10,9 @@ public sealed class TokenBucketRateLimiterOptions { /// /// Specifies the minimum period between replenishments. - /// Must be set to a value > by the time these options are passed to the constructor of . + /// Must be set to a value >= by the time these options are passed to the constructor of . /// + /// means the limiter will never replenish. public TimeSpan ReplenishmentPeriod { get; set; } = TimeSpan.Zero; /// diff --git a/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs index a74b1001ea8d3..f5c75a0308ab2 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs @@ -37,39 +37,30 @@ public override void InvalidOptionsThrows() { Assert.Throws( () => new FixedWindowRateLimiter(new FixedWindowRateLimiterOptions - { - PermitLimit = -1, - QueueProcessingOrder = QueueProcessingOrder.NewestFirst, - QueueLimit = 1, - Window = TimeSpan.FromMinutes(2), - AutoReplenishment = false - })); - Assert.Throws( - () => new FixedWindowRateLimiter(new FixedWindowRateLimiterOptions - { - PermitLimit = 1, - QueueProcessingOrder = QueueProcessingOrder.NewestFirst, - QueueLimit = -1, - Window = TimeSpan.FromMinutes(2), - AutoReplenishment = false - })); + { + PermitLimit = -1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = 1, + Window = TimeSpan.FromMinutes(2), + AutoReplenishment = false + })); Assert.Throws( () => new FixedWindowRateLimiter(new FixedWindowRateLimiterOptions - { - PermitLimit = 1, - QueueProcessingOrder = QueueProcessingOrder.NewestFirst, - QueueLimit = 1, - Window = TimeSpan.MinValue, - AutoReplenishment = false - })); + { + PermitLimit = 1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = -1, + Window = TimeSpan.FromMinutes(2), + AutoReplenishment = false + })); Assert.Throws( () => new FixedWindowRateLimiter(new FixedWindowRateLimiterOptions { PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.FromMinutes(-2), - AutoReplenishment = false, + Window = TimeSpan.MinValue, + AutoReplenishment = false })); Assert.Throws( () => new FixedWindowRateLimiter(new FixedWindowRateLimiterOptions @@ -77,8 +68,8 @@ public override void InvalidOptionsThrows() PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, - AutoReplenishment = false + Window = TimeSpan.FromMinutes(-2), + AutoReplenishment = false, })); } diff --git a/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs index ce42825446bac..bd0d0d298d5ad 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs @@ -39,53 +39,43 @@ public override void InvalidOptionsThrows() { Assert.Throws( () => new SlidingWindowRateLimiter(new SlidingWindowRateLimiterOptions - { - PermitLimit = -1, - QueueProcessingOrder = QueueProcessingOrder.NewestFirst, - QueueLimit = 1, - Window = TimeSpan.FromMinutes(2), - SegmentsPerWindow = 1, - AutoReplenishment = false - })); - Assert.Throws( - () => new SlidingWindowRateLimiter(new SlidingWindowRateLimiterOptions - { - PermitLimit = 1, - QueueProcessingOrder = QueueProcessingOrder.NewestFirst, - QueueLimit = -1, - Window = TimeSpan.FromMinutes(2), - SegmentsPerWindow = 1, - AutoReplenishment = false - })); + { + PermitLimit = -1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = 1, + Window = TimeSpan.FromMinutes(2), + SegmentsPerWindow = 1, + AutoReplenishment = false + })); Assert.Throws( () => new SlidingWindowRateLimiter(new SlidingWindowRateLimiterOptions - { - PermitLimit = 1, - QueueProcessingOrder = QueueProcessingOrder.NewestFirst, - QueueLimit = 1, - Window = TimeSpan.FromMinutes(2), - SegmentsPerWindow = -1, - AutoReplenishment = false - })); + { + PermitLimit = 1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = -1, + Window = TimeSpan.FromMinutes(2), + SegmentsPerWindow = 1, + AutoReplenishment = false + })); Assert.Throws( () => new SlidingWindowRateLimiter(new SlidingWindowRateLimiterOptions - { - PermitLimit = 1, - QueueProcessingOrder = QueueProcessingOrder.NewestFirst, - QueueLimit = 1, - Window = TimeSpan.MinValue, - SegmentsPerWindow = 1, - AutoReplenishment = false - })); + { + PermitLimit = 1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = 1, + Window = TimeSpan.FromMinutes(2), + SegmentsPerWindow = -1, + AutoReplenishment = false + })); Assert.Throws( () => new SlidingWindowRateLimiter(new SlidingWindowRateLimiterOptions { PermitLimit = 1, QueueProcessingOrder = QueueProcessingOrder.NewestFirst, QueueLimit = 1, - Window = TimeSpan.Zero, + Window = TimeSpan.MinValue, SegmentsPerWindow = 1, - AutoReplenishment = false, + AutoReplenishment = false })); Assert.Throws( () => new SlidingWindowRateLimiter(new SlidingWindowRateLimiterOptions diff --git a/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs index 55e13feff4ab1..5e8cd4135a400 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs @@ -77,16 +77,6 @@ public override void InvalidOptionsThrows() TokensPerPeriod = 1, AutoReplenishment = false })); - Assert.Throws( - () => new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions - { - TokenLimit = 1, - QueueProcessingOrder = QueueProcessingOrder.NewestFirst, - QueueLimit = 1, - ReplenishmentPeriod = TimeSpan.Zero, - TokensPerPeriod = 1, - AutoReplenishment = false - })); Assert.Throws( () => new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions { From 8c07cb1efeef95d8bef408fd89081ab8f5aeb58c Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Thu, 1 Sep 2022 16:03:20 -0700 Subject: [PATCH 6/8] no special case --- .../RateLimiting/TokenBucketRateLimiter.cs | 13 ++++--- .../tests/TokenBucketRateLimiterTests.cs | 34 +++++++++++++++++-- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs index 5faa59c7731ea..9238a62a0c03e 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs @@ -296,13 +296,16 @@ private void ReplenishInternal(long nowTicks) return; } - var add = _fillRate * (nowTicks - _lastReplenishmentTick) * TickFrequency; + double add; - // special case the scenario when TokenLimit is 1 so that it doesn't potentially take two timer calls to fully update the token by 1 - // other limits are hit by this, but they are a lot smoother in practice so it isn't as bad that two timer calls may be needed - if (_options.TokenLimit == 1 && _options.AutoReplenishment) + // Trust the timer to be close enough to when we want to replenish, this avoids issues with Timer jitter where it might be .99 seconds instead of 1, and 1.1 seconds the next time etc. + if (_options.AutoReplenishment) { - add = 1; + add = _options.TokensPerPeriod; + } + else + { + add = _fillRate * (nowTicks - _lastReplenishmentTick) * TickFrequency; } _tokenCount = Math.Min(_options.TokenLimit, _tokenCount + add); diff --git a/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs index 5e8cd4135a400..69876938fdfba 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs @@ -1351,7 +1351,7 @@ public override void GetStatisticsThrowsAfterDispose() } [Fact] - public void AutoReplenishPreservesTimeWithTimerJitter() + public void AutoReplenishIgnoresTimerJitter() { var replenishmentPeriod = TimeSpan.FromMinutes(10); using var limiter = new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions @@ -1372,8 +1372,36 @@ public void AutoReplenishPreservesTimeWithTimerJitter() // Replenish 1 millisecond less than ReplenishmentPeriod while AutoReplenishment is enabled Replenish(limiter, (long)replenishmentPeriod.TotalMilliseconds - 1); - // Timer ran faster than ReplenishmentPeriod so a full token wasn't added. - // Internally the limiter is tracking that so the next timer call will add to the previous partial token + Assert.Equal(8, limiter.GetStatistics().CurrentAvailablePermits); + + // Replenish 1 millisecond longer than ReplenishmentPeriod while AutoReplenishment is enabled + Replenish(limiter, (long)replenishmentPeriod.TotalMilliseconds + 1); + + Assert.Equal(9, limiter.GetStatistics().CurrentAvailablePermits); + } + + [Fact] + public void ManualReplenishPreservesTimeWithTimerJitter() + { + var replenishmentPeriod = TimeSpan.FromMinutes(10); + using var limiter = new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions + { + TokenLimit = 10, + QueueProcessingOrder = QueueProcessingOrder.OldestFirst, + QueueLimit = 1, + ReplenishmentPeriod = replenishmentPeriod, + AutoReplenishment = false, + TokensPerPeriod = 1, + }); + + var lease = limiter.AttemptAcquire(permitCount: 3); + Assert.True(lease.IsAcquired); + + Assert.Equal(7, limiter.GetStatistics().CurrentAvailablePermits); + + // Replenish 1 millisecond less than ReplenishmentPeriod while AutoReplenishment is enabled + Replenish(limiter, (long)replenishmentPeriod.TotalMilliseconds - 1); + Assert.Equal(7, limiter.GetStatistics().CurrentAvailablePermits); // Replenish 1 millisecond longer than ReplenishmentPeriod while AutoReplenishment is enabled From 7513e71c368b16c9b78468fdf8cf65acde211233 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Fri, 2 Sep 2022 11:39:28 -0700 Subject: [PATCH 7/8] TimeSpan.Zero --- .../Threading/RateLimiting/FixedWindowRateLimiter.cs | 4 ++-- .../RateLimiting/FixedWindowRateLimiterOptions.cs | 3 +-- .../Threading/RateLimiting/SlidingWindowRateLimiter.cs | 4 ++-- .../RateLimiting/SlidingWindowRateLimiterOptions.cs | 3 +-- .../Threading/RateLimiting/TokenBucketRateLimiter.cs | 4 ++-- .../RateLimiting/TokenBucketRateLimiterOptions.cs | 3 +-- .../tests/FixedWindowRateLimiterTests.cs | 9 +++++++++ .../tests/SlidingWindowRateLimiterTests.cs | 10 ++++++++++ .../tests/TokenBucketRateLimiterTests.cs | 10 ++++++++++ 9 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs index 774a6876c245e..780eddd2cee2e 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiter.cs @@ -59,9 +59,9 @@ public FixedWindowRateLimiter(FixedWindowRateLimiterOptions options) { throw new ArgumentException($"{nameof(options.QueueLimit)} must be set to a value greater than or equal to 0.", nameof(options)); } - if (options.Window < TimeSpan.Zero) + if (options.Window <= TimeSpan.Zero) { - throw new ArgumentException($"{nameof(options.Window)} must be set to a value greater than or equal to TimeSpan.Zero.", nameof(options)); + throw new ArgumentException($"{nameof(options.Window)} must be set to a value greater than TimeSpan.Zero.", nameof(options)); } _options = new FixedWindowRateLimiterOptions diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiterOptions.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiterOptions.cs index a6d2b16475552..3ebab40439f6b 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiterOptions.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiterOptions.cs @@ -10,9 +10,8 @@ public sealed class FixedWindowRateLimiterOptions { /// /// Specifies the time window that takes in the requests. - /// Must be set to a value >= by the time these options are passed to the constructor of . + /// Must be set to a value > by the time these options are passed to the constructor of . /// - /// means the limiter will never replenish. public TimeSpan Window { get; set; } = TimeSpan.Zero; /// diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs index a1fe3e2839ed2..5dfc36914487e 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiter.cs @@ -63,9 +63,9 @@ public SlidingWindowRateLimiter(SlidingWindowRateLimiterOptions options) { throw new ArgumentException($"{nameof(options.QueueLimit)} must be set to a value greater than or equal to 0.", nameof(options)); } - if (options.Window < TimeSpan.Zero) + if (options.Window <= TimeSpan.Zero) { - throw new ArgumentException($"{nameof(options.Window)} must be set to a value greater than or equal to TimeSpan.Zero.", nameof(options)); + throw new ArgumentException($"{nameof(options.Window)} must be set to a value greater than TimeSpan.Zero.", nameof(options)); } _options = new SlidingWindowRateLimiterOptions diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs index f7f399e175b1f..459c44b4d81c6 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs @@ -10,9 +10,8 @@ public sealed class SlidingWindowRateLimiterOptions { /// /// Specifies the minimum period between replenishments. - /// Must be set to a value >= by the time these options are passed to the constructor of . + /// Must be set to a value > by the time these options are passed to the constructor of . /// - /// means the limiter will never replenish. public TimeSpan Window { get; set; } = TimeSpan.Zero; /// diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs index 9238a62a0c03e..f1fbcb4433c4d 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiter.cs @@ -61,9 +61,9 @@ public TokenBucketRateLimiter(TokenBucketRateLimiterOptions options) { throw new ArgumentException($"{nameof(options.QueueLimit)} must be set to a value greater than or equal to 0.", nameof(options)); } - if (options.ReplenishmentPeriod < TimeSpan.Zero) + if (options.ReplenishmentPeriod <= TimeSpan.Zero) { - throw new ArgumentException($"{nameof(options.ReplenishmentPeriod)} must be set to a value greater than or equal to TimeSpan.Zero.", nameof(options)); + throw new ArgumentException($"{nameof(options.ReplenishmentPeriod)} must be set to a value greater than TimeSpan.Zero.", nameof(options)); } _options = new TokenBucketRateLimiterOptions diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiterOptions.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiterOptions.cs index b0371119ce3c0..451228c0683fa 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiterOptions.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiterOptions.cs @@ -10,9 +10,8 @@ public sealed class TokenBucketRateLimiterOptions { /// /// Specifies the minimum period between replenishments. - /// Must be set to a value >= by the time these options are passed to the constructor of . + /// Must be set to a value > by the time these options are passed to the constructor of . /// - /// means the limiter will never replenish. public TimeSpan ReplenishmentPeriod { get; set; } = TimeSpan.Zero; /// diff --git a/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs index f5c75a0308ab2..1f597748d67f3 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/FixedWindowRateLimiterTests.cs @@ -71,6 +71,15 @@ public override void InvalidOptionsThrows() Window = TimeSpan.FromMinutes(-2), AutoReplenishment = false, })); + Assert.Throws( + () => new FixedWindowRateLimiter(new FixedWindowRateLimiterOptions + { + PermitLimit = 1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = 1, + Window = TimeSpan.Zero, + AutoReplenishment = false, + })); } [Fact] diff --git a/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs index bd0d0d298d5ad..66e6cb2d5f228 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/SlidingWindowRateLimiterTests.cs @@ -87,6 +87,16 @@ public override void InvalidOptionsThrows() SegmentsPerWindow = 1, AutoReplenishment = false })); + Assert.Throws( + () => new SlidingWindowRateLimiter(new SlidingWindowRateLimiterOptions + { + PermitLimit = 1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = 1, + Window = TimeSpan.Zero, + SegmentsPerWindow = 1, + AutoReplenishment = false + })); } [Fact] diff --git a/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs b/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs index 69876938fdfba..79c368e2b6d34 100644 --- a/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs +++ b/src/libraries/System.Threading.RateLimiting/tests/TokenBucketRateLimiterTests.cs @@ -87,6 +87,16 @@ public override void InvalidOptionsThrows() TokensPerPeriod = 1, AutoReplenishment = false })); + Assert.Throws( + () => new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions + { + TokenLimit = 1, + QueueProcessingOrder = QueueProcessingOrder.NewestFirst, + QueueLimit = 1, + ReplenishmentPeriod = TimeSpan.Zero, + TokensPerPeriod = 1, + AutoReplenishment = false + })); } [Fact] From c5754dcec77e3f0fef791016019369c9350c31f8 Mon Sep 17 00:00:00 2001 From: Brennan Date: Fri, 2 Sep 2022 13:04:35 -0700 Subject: [PATCH 8/8] Apply suggestions from code review Co-authored-by: Stephen Halter --- .../Threading/RateLimiting/FixedWindowRateLimiterOptions.cs | 2 +- .../Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs | 2 +- .../Threading/RateLimiting/TokenBucketRateLimiterOptions.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiterOptions.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiterOptions.cs index 3ebab40439f6b..8f7dbaa344beb 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiterOptions.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/FixedWindowRateLimiterOptions.cs @@ -10,7 +10,7 @@ public sealed class FixedWindowRateLimiterOptions { /// /// Specifies the time window that takes in the requests. - /// Must be set to a value > by the time these options are passed to the constructor of . + /// Must be set to a value greater than by the time these options are passed to the constructor of . /// public TimeSpan Window { get; set; } = TimeSpan.Zero; diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs index 459c44b4d81c6..93f7ba933b464 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs @@ -10,7 +10,7 @@ public sealed class SlidingWindowRateLimiterOptions { /// /// Specifies the minimum period between replenishments. - /// Must be set to a value > by the time these options are passed to the constructor of . + /// Must be set to a value greater than by the time these options are passed to the constructor of . /// public TimeSpan Window { get; set; } = TimeSpan.Zero; diff --git a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiterOptions.cs b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiterOptions.cs index 451228c0683fa..2c065d9432e67 100644 --- a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiterOptions.cs +++ b/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/TokenBucketRateLimiterOptions.cs @@ -10,7 +10,7 @@ public sealed class TokenBucketRateLimiterOptions { /// /// Specifies the minimum period between replenishments. - /// Must be set to a value > by the time these options are passed to the constructor of . + /// Must be set to a value greater than by the time these options are passed to the constructor of . /// public TimeSpan ReplenishmentPeriod { get; set; } = TimeSpan.Zero;