Skip to content

Conversation

@manandre
Copy link
Contributor

@ghost
Copy link

ghost commented Aug 31, 2024

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Aug 31, 2024

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@stephentoub stephentoub requested a review from Copilot March 6, 2025 03:28
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR introduces chained rate limiter support by adding a new CreateChained API and corresponding implementations that acquire and aggregate leases from multiple rate limiters. Key changes include:

  • Adding a CombinedRateLimitLease class that aggregates metadata and disposes inner leases in reverse order.
  • Creating a ChainedRateLimiter to coordinate acquisition from a sequence of limiters using common acquisition logic.
  • Exposing a new static CreateChained method on RateLimiter and updating related tests and partitioned limiter implementations.

Reviewed Changes

File Description
src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/CombinedRateLimitLease.cs New combined lease implementation aggregating multiple leases and their metadata.
src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/ChainedRateLimiter.cs New chained rate limiter with shared acquisition logic for synchronous and asynchronous operations.
src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/RateLimiter.cs Added static CreateChained method with necessary validations.
src/libraries/System.Threading.RateLimiting/tests/Infrastructure/Utils.cs Added ThrowDisposeLease for testing failure scenarios in Dispose.
src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/ChainedPartitionedRateLimiter.cs Updated to clone the limiters array and reference ChainedRateLimiter.CommonAcquireLogic for consistency.
src/libraries/System.Threading.RateLimiting/ref/System.Threading.RateLimiting.cs Updated public API reference to include CreateChained.

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more

@BrennanConroy BrennanConroy merged commit 81dff04 into dotnet:main Mar 7, 2025
81 of 84 checks passed
@BrennanConroy
Copy link
Member

Thanks! Sorry it took so long

@BrennanConroy BrennanConroy added this to the 10.0.0 milestone Mar 7, 2025
@manandre manandre deleted the chained-rate-limiter branch March 7, 2025 07:22
@github-actions github-actions bot locked and limited conversation to collaborators Apr 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Threading community-contribution Indicates that the PR has been added by a community member new-api-needs-documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[API Proposal]: AggregateRateLimiter

4 participants