Problem
RateLimitRegistry is instantiated once at module scope (src/taskq/ratelimit/registry.py) and consumed by importing that module-level object. It is an invisible, process-wide dependency: callers do not receive it, cannot substitute it, and cannot scope it.
To be clear about what this issue is not: the registry is not an unbounded leak. Keyed materialisation is capped (settings.max_keyed_reservations, raising ReservationUnavailable rather than growing without bound), idle-eviction sweeps exist, and the actual limiter state (tokens, windows) lives in Redis/Postgres with TTLs — the in-process structure is bookkeeping only. That is the architecturally correct arrangement, and it matches how Sidekiq Enterprise and Dramatiq externalise limiter state rather than accumulating it in process memory. No change is needed on that axis.
The remaining problem is narrower: ownership and substitutability.
Why it matters
-
Test isolation is not structural. Two tests that register primitives with the same name, or that materialise keyed entries, share one process-wide registry. Isolation therefore depends on every test remembering to clean up, or on monkeypatching a module attribute. That is the failure mode where a fixture silently stops doing its job and the suite keeps passing — the tests only look isolated because names happen not to collide. Registry-name collisions across test modules also produce order-dependent failures that are painful to diagnose.
-
One registry per process is assumed, not chosen. Any consumer wanting two independently-configured clients in one process (a common shape in tests, in multi-tenant hosts, and in embedded/library usage) cannot have them.
-
The dependency is invisible. Nothing in a Worker or client signature reveals that rate limiting reaches into module state, which makes the coupling easy to miss when reasoning about lifecycle and shutdown.
Proposed fix
Make the registry an owned, injectable dependency — constructed by (or passed into) the worker/client that uses it — with the module-level instance retained as a default so existing callers are unaffected.
This is the same move Flask made from an import-time global app to the application-factory pattern, and the shape svcs formalises: one long-lived, explicitly app-scoped registry rather than an implicit module global. The benefit is that test isolation becomes a property of construction rather than of cleanup discipline.
Suggested scope:
- Thread an optional registry through the worker/client construction path, defaulting to the existing module-level instance.
- Provide a documented way for tests to obtain a fresh, isolated registry.
- Migrate internal call sites off the direct module import.
Notes
- Deliberately non-blocking. Filed as a follow-up rather than folded into a feature PR: it touches every call site that imports the module-level registry, which is disproportionate to bundle into a feature change and would make that diff unreviewable.
- Related deployment-topology question (whether idle-eviction sweeps fire when leader election never elects anyone, e.g. single-worker deployments or a worker tier scaled to zero) is being handled separately, since it is a reliability concern rather than an ownership one.
Problem
RateLimitRegistryis instantiated once at module scope (src/taskq/ratelimit/registry.py) and consumed by importing that module-level object. It is an invisible, process-wide dependency: callers do not receive it, cannot substitute it, and cannot scope it.To be clear about what this issue is not: the registry is not an unbounded leak. Keyed materialisation is capped (
settings.max_keyed_reservations, raisingReservationUnavailablerather than growing without bound), idle-eviction sweeps exist, and the actual limiter state (tokens, windows) lives in Redis/Postgres with TTLs — the in-process structure is bookkeeping only. That is the architecturally correct arrangement, and it matches how Sidekiq Enterprise and Dramatiq externalise limiter state rather than accumulating it in process memory. No change is needed on that axis.The remaining problem is narrower: ownership and substitutability.
Why it matters
Test isolation is not structural. Two tests that register primitives with the same name, or that materialise keyed entries, share one process-wide registry. Isolation therefore depends on every test remembering to clean up, or on monkeypatching a module attribute. That is the failure mode where a fixture silently stops doing its job and the suite keeps passing — the tests only look isolated because names happen not to collide. Registry-name collisions across test modules also produce order-dependent failures that are painful to diagnose.
One registry per process is assumed, not chosen. Any consumer wanting two independently-configured clients in one process (a common shape in tests, in multi-tenant hosts, and in embedded/library usage) cannot have them.
The dependency is invisible. Nothing in a
Workeror client signature reveals that rate limiting reaches into module state, which makes the coupling easy to miss when reasoning about lifecycle and shutdown.Proposed fix
Make the registry an owned, injectable dependency — constructed by (or passed into) the worker/client that uses it — with the module-level instance retained as a default so existing callers are unaffected.
This is the same move Flask made from an import-time global app to the application-factory pattern, and the shape
svcsformalises: one long-lived, explicitly app-scoped registry rather than an implicit module global. The benefit is that test isolation becomes a property of construction rather than of cleanup discipline.Suggested scope:
Notes