Skip to content

Add CreateChained to RateLimiter #107230

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 7, 2025

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 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