Skip to content

Move rate-limit state to a shared store when the backend scales past 1 replica #38

Open

Description

@prometheusgr

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 in backend/app/services/rate_limiter_service.py with 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.py algorithm 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

  1. added
    enhancementNew feature or request
    type:improvementMakes an existing capability better/safer/more complete
    priority:P3Deferred/speculative — explicitly not scheduled yet
    effort:MMedium: one layer (backend or frontend), a few days
    area:rate-limitingToken-bucket rate limiting for chat/auth endpoints
    and removed
    enhancementNew feature or request
    on Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:rate-limitingToken-bucket rate limiting for chat/auth endpointseffort:MMedium: one layer (backend or frontend), a few dayspriority:P3Deferred/speculative — explicitly not scheduled yettype:improvementMakes an existing capability better/safer/more complete

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions