Skip to content

Enforce rate limits across replicas with a shared Redis backend - #801

Merged
fejilaup-cloud merged 2 commits into
AtomicIP:mainfrom
zaraahmedev-glitch:fix/787-shared-rate-limit-backend
Aug 25, 2026
Merged

Enforce rate limits across replicas with a shared Redis backend#801
fejilaup-cloud merged 2 commits into
AtomicIP:mainfrom
zaraahmedev-glitch:fix/787-shared-rate-limit-backend

Conversation

@zaraahmedev-glitch

Copy link
Copy Markdown
Contributor

Summary

RateLimitMiddleware kept token-bucket counters in an Arc<Mutex<Store>> owned per instance. Behind a load balancer running N replicas, a client whose requests were distributed round-robin effectively received N times the documented per-IP/per-user quota, since each replica enforced its own bucket independently with no shared counter.

  • Extracted a RateLimitStore trait behind RateLimitMiddleware, with two implementations selected via RateLimitConfig::backend:
    • RateLimitBackend::InProcess (default) — the previous in-memory behavior. Kept for tests and single-instance deployments; documented as not safe for multiple replicas.
    • RateLimitBackend::Redis(url) — refills and consumes tokens for every bucket in a request (global, IP, user) in a single atomic Lua script, so a request can't pass the IP bucket on one replica and the user bucket on another. Time comes from Redis's own TIME command rather than the caller's clock, so refill accounting can't be skewed by clock drift between replicas. Fails open (allows the request, logs a warning) if Redis is unreachable, so a Redis outage degrades to unlimited-but-available rather than taking the API down with false 429s.
  • Violation backoff, tier assignment (set_user_tier), and tracked-identity cardinality bounding stay process-local under both backends — they're memory-protection heuristics for that replica, not part of the documented quota, so they don't need to be distributed for the quota fix to be correct.
  • set_user_tier and the rest of the public API are unchanged.
  • docs/api-reference.md now has a "Deployment topology" section stating explicitly that multi-instance correctness requires the Redis backend.

New tests:

  • redis_backend_enforces_one_combined_quota_across_instances — spins up a real Redis container (via testcontainers) and proves two independent RateLimitMiddleware instances pointed at it enforce one combined quota for the same client, not two independent ones.
  • in_process_backend_does_not_share_quota_across_instances — documents, as a test, that the in-process backend does not share quota (matching the new docs caveat).
  • All five pre-existing tests (burst limiting, tier isolation, exponential backoff, concurrent single-instance thread-safety, bounded cardinality overflow, and the middleware header/429-body test) are preserved with identical assertions, only adapted to async since the store trait's refill_and_consume is async.

Verification

Ran from api-server/:

$ cargo test --lib rate_limit::
running 8 tests
test rate_limit::tests::burst_is_limited_and_tokens_recover ... ok
test rate_limit::tests::in_process_backend_does_not_share_quota_across_instances ... ok
test rate_limit::tests::middleware_returns_client_headers_and_429_body ... ok
test rate_limit::tests::concurrent_requests_cannot_overspend_bucket ... ok
test rate_limit::tests::repeated_violations_back_off_exponentially ... ok
test rate_limit::tests::tiers_and_users_are_isolated ... ok
test rate_limit::tests::zero_tracking_capacity_uses_bounded_overflow_bucket ... ok
test rate_limit::tests::redis_backend_enforces_one_combined_quota_across_instances ... ok

test result: ok. 8 passed; 0 failed
  • rustfmt --check src/rate_limit.rs — clean.
  • cargo clippy --lib — no warnings in rate_limit.rs (added #[derive(Default)]/#[default] for RateLimitBackend to satisfy derivable_impls).
  • The Redis cross-instance test actually starts a Redis container via Docker/testcontainers and exercises the real Lua script end-to-end (not mocked).

Pre-existing, unrelated breakage found while verifying (not touched by this PR): api-server currently does not build as a whole on main — independent of this change:

  • src/audit.rs doesn't compile (tokio::sync::RwLock::read()/write() treated as fallible; it isn't).
  • src/distributed_tracing.rs doesn't compile (opentelemetry::trace::BoxedSpan — the type actually lives at opentelemetry::global::BoxedSpan).
  • The api-server binary target fails with "multiple different versions of crate axum in the dependency graph."
  • deduplication::tests::test_concurrent_deduplicator_duplicate_waits hangs indefinitely.
  • circuit_breaker::tests::* (5 tests) and validation::tests::test_validate_stellar_address_valid / test_validation_rule_address fail with assertion panics.

This is consistent with api-server being commented out of the root Cargo workspace ("temporarily excluded: async-graphql version conflict") and therefore never actually built by .github/workflows/ci.yml's cargo build/test --workspace. None of these are touched by this diff; I verified this PR's change by temporarily patching those two unrelated compile errors locally (never committed), confirmed the full cargo test --lib run passes aside from the pre-existing failures/hang listed above (which are unrelated to rate limiting by file and by inspection), then reverted those files before committing so this diff stays scoped to #787. Recommend a separate issue for the pre-existing api-server build breakage.

Closes #787

zaraahmedev-glitch and others added 2 commits August 25, 2026 03:00
RateLimitMiddleware previously kept token-bucket counters in an
Arc<Mutex<Store>> owned per instance. Behind a load balancer running N
replicas, a client distributed round-robin effectively got N times the
documented per-IP/per-user quota, since each replica enforced its own
bucket independently.

RateLimitStore is now a trait with two implementations:
- InProcessStore: the previous in-memory behavior, kept as the default
  for tests and single-instance deployments (not safe for multiple
  replicas).
- RedisStore: refills and consumes tokens for all buckets (global, IP,
  user) in one atomic Lua script per request, so a client can't pass
  the ip bucket on one replica and the user bucket on another. Time
  comes from Redis's own TIME command rather than the caller's clock,
  avoiding clock-skew between replicas. Fails open (with a warning
  log) if Redis is unreachable, rather than taking the API down.

Violation backoff, tier assignment, and cardinality bounding stay
process-local under both backends, since they're memory-protection
heuristics for that replica rather than part of the enforced quota.

set_user_tier and the rest of the public API are unchanged.
docs/api-reference.md now documents the deployment-topology
requirement explicitly.

Closes AtomicIP#787
@fejilaup-cloud
fejilaup-cloud merged commit d9a9638 into AtomicIP:main Aug 25, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rate limiting is enforced per-instance, multiplying the documented quota by replica count

2 participants