Skip to content

[csharp][generichost] Revert to DropWrite#24080

Merged
wing328 merged 1 commit into
OpenAPITools:masterfrom
devhl-labs:devhl/revert-to-DropWrite
Jun 21, 2026
Merged

[csharp][generichost] Revert to DropWrite#24080
wing328 merged 1 commit into
OpenAPITools:masterfrom
devhl-labs:devhl/revert-to-DropWrite

Conversation

@devhl-labs

@devhl-labs devhl-labs commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Partial revert from #22233 In most simple cases we are only going to be concerned about rate limiting. For other concerns, we can give new default limiters for each token type. Users can also provide their own rate limiter. We can consider other changes as well to address some of what EraYaN said feels off about the approach currently used. The following is copilot's view on it.

C# GenericHost: revert BoundedChannelFullMode in RateLimitProvider from DropOldest to DropWrite

PR #22233 changed BoundedChannelFullMode in RateLimitProvider from DropWrite to DropOldest. This reverts that change.

Why DropWrite is correct

The bounded channel in RateLimitProvider has a capacity equal to container.Tokens.Count. Each token writes itself to the channel when its rate-limit timer fires. A full channel simply means every token is already queued and available — any additional write for that token is a no-op duplicate. DropWrite handles this correctly by silently discarding the redundant write.

DropOldest evicts the longest-waiting token from the front of the queue to make room for the incoming write. This is actively harmful for rate limiting:

  1. FIFO ordering is intentional. The token that cooled down longest ago should be served first to maximize throughput while respecting per-token rate limits.
  2. A full channel is not an error condition here. The capacity is bounded by design — there is no scenario where a genuinely new, distinct token needs to displace an existing one.
  3. DropOldest can cause tokens to "disappear." If the evicted token's TokenBecameAvailable event doesn't fire again immediately, the channel will have one fewer token than expected until the next timer tick.

Note on JIT / OAuth tokens

The concern raised in #22233 about token freshness and expired claims is valid but belongs in a different TokenProvider subclass designed for JIT/OAuth flows, not in RateLimitProvider which is designed solely for fixed-key rate limiting.

Changes

  • RateLimitProvider\1.mustache: BoundedChannelFullMode.DropOldestBoundedChannelFullMode.DropWrite`
  • Regenerated all affected C# GenericHost samples

PR checklist

  • Read the contribution guidelines.
  • Run the following to build the project and update samples:
    ./mvnw clean package || exit
    ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
    ./bin/utils/export_docs_generators.sh || exit
    
    (For Windows users, please run the script in WSL)
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 50 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

@wing328 wing328 merged commit c7d1e10 into OpenAPITools:master Jun 21, 2026
76 checks passed
@wing328 wing328 added this to the 7.24.0 milestone Jun 21, 2026
@devhl-labs devhl-labs deleted the devhl/revert-to-DropWrite branch June 21, 2026 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants