Repository navigation
Move rate-limit state to a shared store when the backend scales past 1 replica #38
Description
Context
Issue #25 implements rate limiting (token bucket per identity: user_id for chat, IP for auth) using in-process state — a plain dict guarded by a lock, living inside the single uvicorn worker process. This was a deliberate v1 decision, not an oversight: every deployment path this repo ships today runs the backend at replicas: 1 (infra/helm/charts/backend/values.yaml, asserted by infra/tests/test_helm_charts.py) and a single uvicorn process with no --workers flag (docker-compose.yml, backend/docker-entrypoint.sh).
This differs from the retention CronJob precedent (docs/SECURITY.md's "Enforcement: K8s CronJob" section), which had to avoid in-process state outright because it would cause destructive double-execution across replicas. Rate limiting's failure mode is different in kind: if a fork scales the backend Deployment to N replicas, each replica keeps its own independent bucket per identity, so the effective limit becomes N × the configured value — silent under-enforcement, not data loss or duplicate side effects. That's judged an acceptable v1 trade-off, but it's a real one, and it needs to be closed before any deployment intentionally scales replicas past 1.
Scope
- Replace (or make pluggable) the in-process
dict-backed bucket store inbackend/app/services/rate_limiter_service.pywith a shared store — Redis is the natural choice (the token-bucket state is two numbers per identity, i.e. trivially a Redis hash), though a Postgres-backed counter approach should also be considered given this repo currently has zero Redis dependency anywhere and adding one is a real new piece of air-gapped infrastructure to vendor, document, and operate. - Decide and document the mechanism the same way issue Add rate limiting to chat and auth endpoints #25 documented the in-process choice — write the rationale into
docs/SECURITY.md's rate-limiting section, replacing (not just appending to) the current "single-replica assumption" write-up. - The pure
token_bucket.pyalgorithm module should not need to change; only where its resulting state is persisted between requests.
Trigger
Not scheduled speculatively — pick this up when a fork's infra/helm/charts/backend/values.yaml is actually intended to run with replicas > 1, or air-gapped Redis vendoring is otherwise already planned for another reason.
Standards
AGENTS.md — TDD as usual; the store swap should be covered by the same test_rate_limiter_service.py suite issue #25 introduces, run against whichever backing store is chosen.
Activity