Repository navigation
fix(rate-limit): honor zero token refill rate - #2053
Conversation
Signed-off-by: lixiang5 <lixiang5@sensetime.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe rate-limit configuration now preserves an explicit zero token refill rate from Helm output through model gateway initialization. An asynchronous test verifies that zero refill prevents replenishment while initial and returned tokens remain usable. ChangesRate-limit configuration and runtime behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@model_gateway/src/app_context.rs`:
- Around line 718-736: Strengthen the regression test
explicit_zero_rate_limit_disables_refill so the old fallback rate is guaranteed
to replenish at least one token during the wait: adjust the bucket capacity and
sleep duration accordingly while preserving the zero-rate assertions. Then run
the pr-test-analyzer agent to verify the test adequately detects fallback
behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a41039b5-3493-4736-9368-c7e4acb5db99
📒 Files selected for processing (2)
deploy/helm/smg/templates/_helpers.tplmodel_gateway/src/app_context.rs
Signed-off-by: lixiang5 <lixiang5@sensetime.com>
Description
Problem
rate_limit_tokens_per_second = 0passes configuration validation and is supported byTokenBucketas a zero-refill concurrency limiter. However, startup filtered out zero and fell back tomax_concurrent_requestsas the refill rate. The Helm chart also omitted the CLI flag when the value was zero.Solution
Preserve explicit non-negative refill rates when constructing the token bucket, and render the Helm CLI argument for zero. Keep negative values filtered so unchecked configurations cannot be cast to an invalid
usize.Changes
AppContextBuilder.auth.rateLimitTokensPerSecond: 0through the Helm chart.Test Plan
cargo +nightly fmt --all -- --checkcargo clippy --all-targets --all-features -- -D warningscargo testhelm lint deploy/helm/smgauth.rateLimitTokensPerSecond=0and verify that--rate-limit-tokens-per-second 0is present.-1and verify that the rate-limit argument is absent.Checklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspasses