Uh oh!
There was an error while loading. Please reload this page.
feat(ratelimit): cluster-level rate limiting via shared Redis - #607
Merged
Conversation
Rate-limit counters lived in per-process memory, so an N-replica DP
cluster enforced N× every configured limit (a key capped at rpm:1 got
one request per replica per minute). Add a Redis-backed shared store so
the whole cluster enforces one global window.
- Introduce a `RateStore` backend behind `Limiter`: `LocalStore`
(unchanged in-memory default) and `RedisStore` (Lua check-and-increment
over wall-clock-aligned fixed windows, `redis.call('TIME')` for
cross-replica window consistency, hash-tagged keys for Cluster slot
co-location). All dimensions are shared — rps/rpm/rph/rpd/tpm/tpd plus
concurrency, tracked as a crash-safe ZSET semaphore reclaimed after
`concurrency_ttl_secs`. On a Redis outage the store fails open to
per-replica counting.
- Make the enforcement path async (`pre_commit`/`commit_tokens`/`peek`);
concurrency release stays a sync `Drop` (Redis detaches a ZREM).
- New `ratelimit` config block (`backend: memory|redis`, `redis`,
`concurrency_ttl_secs`), enabled via env on managed deployments.
Tests: Rust integration (gated on RATELIMIT_TEST_REDIS_URL) for shared
rpm/rps/tpm/concurrency + TTL reclaim; DP e2e spins two real binaries on
one Redis (A→200, B→429) plus a memory-backend regression (both 200).
Fixesapi7/AISIX-Cloud#798No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds a pluggable ChangesCluster-wide rate limiting
Sequence Diagram(s)sequenceDiagram
participant Client
participant ProxyHandler as chat.rs / embeddings.rs
participant Quota as quota.rs enforce_rate_limit
participant Limiter
participant RateStore as LocalStore or RedisStore
participant Redis
Client->>ProxyHandler: POST /v1/chat/completions
ProxyHandler->>Quota: enforce_rate_limit(state, auth, model_rl).await
Quota->>Limiter: pre_commit(key, limits).await
Limiter->>RateStore: acquire(key, limits, member).await
alt Redis backend
RateStore->>Redis: EVALSHA ACQUIRE_LUA
Redis-->>RateStore: ok / rate_limit_error
end
RateStore-->>Limiter: Ok(()) or RateLimitError
Limiter-->>Quota: Reservation
Quota-->>ProxyHandler: MultiReservation
alt streaming
ProxyHandler->>ProxyHandler: into_stream_hold() → StreamConcurrencyGuard
Note over ProxyHandler: concurrency held for stream lifetime
ProxyHandler->>RateStore: add_tokens_post_stream(key, tokens)
Note over ProxyHandler: guard dropped → release(key, member)
else non-streaming / cache-hit
ProxyHandler->>ProxyHandler: reservation.commit_tokens(tokens).await
ProxyHandler->>RateStore: commit(key, tokens, member).await
alt Redis backend
RateStore->>Redis: EVALSHA COMMIT_LUA
end
end
ProxyHandler-->>Client: 200 OK or 429 Too Many Requests
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@crates/aisix-core/src/config.rs`:
- Around line 704-708: Add a validation check in the same config validation
block that checks the Redis backend to also enforce that concurrency_ttl_secs
must be a positive value (greater than 0) when the Redis backend is selected.
When Redis semaphore reclamation is enabled, a concurrency_ttl_secs value of 0
would immediately reclaim active slots and disable concurrency limiting, so add
a condition to return a BootstrapError::Config with a clear message if
concurrency_ttl_secs is 0 or negative while using the Redis backend, similar to
the existing Redis backend validation pattern.
In `@crates/aisix-ratelimit/tests/redis_integration.rs`:
- Around line 161-167: Replace the hardcoded 200ms sleep after a.release(&key,
"a-1") with bounded polling that repeatedly attempts the b.acquire(&key,
&limits, "b-2") operation until it succeeds or a reasonable timeout is reached.
Instead of assuming a fixed propagation delay, use a loop with
tokio::time::timeout or a similar mechanism to poll for the actual condition
(slot availability) rather than sleeping, which makes the test robust to varying
CI executor speeds.
In `@crates/aisix-server/src/main.rs`:
- Around line 379-401: The rate limiter selection logic does not respect the
`ratelimit.backend` configuration setting. Currently, the match expression at
line 385 only checks if `cfg.ratelimit.redis` is present, which means the redis
backend can be used even when `backend: memory` is configured. Modify the
branching logic to check both the `cfg.ratelimit.backend` value AND the presence
of `cfg.ratelimit.redis`. Only instantiate the redis-backed limiter when
`backend` is explicitly set to redis and the `cfg.ratelimit.redis` block is
present; otherwise, use the memory backend with `Limiter::new()`. This ensures
the user's backend configuration choice is honored regardless of whether a redis
block exists in the config.
In `@tests/e2e/src/cases/ratelimit-cluster-e2e.test.ts`:
- Around line 140-147: The afterAll hook in the ratelimit-cluster-e2e.test.ts
file calls deletePrefix without checking whether the etcd infrastructure is
available, which causes test suite failures when etcd is unavailable even though
tests correctly skip via ctx.skip() when infra is down. Guard the deletePrefix
call (at line 146 in the afterAll hook and also at line 195 in another afterAll
hook) behind a readiness check to ensure cleanup only runs if etcd is actually
available, preventing teardown failures from failing the entire suite when
infrastructure is unavailable.
🪄 Autofix (Beta)
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
Run ID: 47eb2a68-4b09-4fc1-8b62-5de06116e6e4
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
.github/workflows/ci.ymlconfig.example.yamlconfig.managed.yamlcrates/aisix-core/src/config.rscrates/aisix-core/src/lib.rscrates/aisix-proxy/src/chat.rscrates/aisix-proxy/src/embeddings.rscrates/aisix-proxy/src/quota.rscrates/aisix-ratelimit/Cargo.tomlcrates/aisix-ratelimit/src/lib.rscrates/aisix-ratelimit/src/limiter.rscrates/aisix-ratelimit/src/store/local.rscrates/aisix-ratelimit/src/store/mod.rscrates/aisix-ratelimit/src/store/redis.rscrates/aisix-ratelimit/tests/redis_integration.rscrates/aisix-server/src/main.rsdocs/configuration/rate-limits.mdtests/e2e/src/cases/ratelimit-cluster-e2e.test.ts
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
… robustness - main.rs: select the rate-limit store on `ratelimit.backend`, not on `ratelimit.redis` presence, so a stray redis block under `backend: memory` no longer silently activates Redis. - config: reject `concurrency_ttl_secs: 0` for the redis backend (a zero TTL prunes a slot in the same second it is taken, disabling concurrency limiting). + unit test. - redis integration test: poll (bounded) for the detached ZREM instead of a fixed 200ms sleep. - cluster e2e: guard the afterAll deletePrefix behind the readiness flag so teardown doesn't fail when infra is unavailable.
Uh oh!
There was an error while loading. Please reload this page.
moonming added a commit
that referenced
this pull request
Jun 15, 2026
) Unbreaks main: #606 merged onto a main with #607's MultiReservation API change (semantic conflict, no textual conflict). Ports the streaming-ensemble reservation code to the new API and fixes the latent un-awaited commit_tokens (panel tokens were never billed on streaming error exits). Verified green locally + CI.
This was referenced Jun 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Rate-limit counters live in per-process memory (
FixedWindowCounterin aDashMap), so each DP replica counts only the traffic it personally served. A cluster of N replicas behind a load balancer therefore enforces N× every configured limit — a key capped atrpm: 1gets one request per replica per minute. The reporter saw exactly this: instance:3000returns 429 while:3001still serves the same key.Approach
Introduce a pluggable
RateStorebackend behindLimiter:LocalStore— the historical per-process fixed-window counters, unchanged. Stays the default, so single-node and dev deployments behave exactly as before (all prior limiter unit tests pass against it verbatim).RedisStore— shares the counters across every replica through one Redis, so the whole cluster enforces one global window. Counter math mirrorsLocalStore/FixedWindowCounter(wall-clock-aligned windowsnow - now % window) so swappingmemory ↔ redisdoesn't change observable limits, only whether the count is shared.RedisStore details:
redis.call('TIME')is used fornowso window boundaries are identical across replicas regardless of host clock skew.aisix:rl:and hash-tagged{<bucket>}so all of a bucket's keys co-locate on one Redis Cluster slot (the per-bucket Lua stays atomic). The Redis may be the same instance used for the response cache.concurrency: it's tracked as a ZSET semaphore (member → score=now) where acquire prunes entries older thanconcurrency_ttl_secsbefore counting, so a slot held by a crashed/hung replica is reclaimed within the TTL. (A window-TTL counter would mishandle long streaming responses — the same reasonStreamConcurrencyGuardexists.)The enforcement path (
pre_commit/commit_tokens/peek) is nowasync; concurrency release stays a synchronousDrop(the Redis backend detaches aZREM, bounded by the TTL prune). All LLM endpoints share thequota::enforce/enforce_rate_limithelpers, so every endpoint inherits the fix uniformly.Configuration
New
ratelimitblock, defaulting tomemory(current behaviour):Reachable by env on managed/containerized deployments:
AISIX_RATELIMIT__BACKEND=redis,AISIX_RATELIMIT__REDIS__URL=....backend: rediswithout aredisblock is rejected at boot.Behaviour changes
backend: memory).backend: redisnow enforce limits cluster-wide instead of per replica.Tests
crates/aisix-ratelimit/tests/redis_integration.rs, gated onRATELIMIT_TEST_REDIS_URL, CI provisionsredis:7-alpine): twoRedisStoreinstances share rpm/rps/tpm/concurrency counters; rps window rollover; stale concurrency slot reclaimed after TTL.tests/e2e/src/cases/ratelimit-cluster-e2e.test.ts): spins up two realaisixbinaries on one shared etcd + one shared Redis with anrpm: 1key — request to replica A → 200, request to replica B → 429 + Retry-After (the exact issue repro). A contrast suite with the defaultmemorybackend shows both replicas serve the request (the per-replica bug).LocalStore; new config-validation unit tests cover theratelimit.redisrules.Fixes api7/AISIX-Cloud#798
Summary by CodeRabbit
New Features
ratelimitconfiguration block with selectablebackend(memorydefault orredis) for cluster-wide rate limiting across multiple replicas.concurrency_ttl_secs, enabling cross-replica in-flight slot reclamation.Documentation
Tests
Fixes api7/AISIX-Cloud#788