Skip to content

pubsub: stop leaking Redis connections from abandoned subscriptions - #724

Merged
nuclearcat merged 1 commit into
kernelci:mainfrom
nuclearcat:fix/pubsub-redis-pool-leak
Oct 28, 2026
Merged

nuclearcat merged 1 commit into
kernelci:mainfrom
nuclearcat:fix/pubsub-redis-pool-leak

Conversation

@nuclearcat

Copy link
Copy Markdown
Member

Every pub/sub subscription pins one dedicated Redis connection in the API process. redis-py 8 lowered the default asyncio pool size to 100, and once 100 subscriptions are held every other Redis command (INCR on subscribe, PUBLISH on node updates) fails with MaxConnectionsError, which surfaced as HTTP 500 for all writes and took the whole pipeline down.

The subscriptions piled up because a client that subscribes and dies before its first listen() leaves last_poll unset, and cleanup_stale_subscriptions() only ever looked at last_poll, so such entries were never removed. A crash-looping external client exhausted the pool in about an hour.

Fixes:

  • cleanup_stale_subscriptions() ages never-polled subscriptions from their creation time.
  • On every subscribe, evict that user's never-polled subscriptions older than abandoned_subscription_seconds (60s) and then enforce a cap of max_subscriptions_per_user (20) active subscriptions, returning HTTP 429 when exceeded.
  • Pass an explicit redis_max_connections (1000) to the Redis pool.
  • Close the previous PubSub when rebuilding after a connection error, and close the half-initialised PubSub if SUBSCRIBE fails, so neither path leaks a pooled connection.
  • Keep the keep-alive loop alive across transient Redis errors.
  • Map Redis MaxConnectionsError / ConnectionError to a 503 with Retry-After instead of a generic 500.

All new settings are overridable through the environment via PubSubSettings. Unit tests cover the cleanup, eviction, limit, rebuild and keep-alive paths.

Every pub/sub subscription pins one dedicated Redis connection in the
API process.  redis-py 8 lowered the default asyncio pool size to 100,
and once 100 subscriptions are held every other Redis command (INCR on
subscribe, PUBLISH on node updates) fails with MaxConnectionsError,
which surfaced as HTTP 500 for all writes and took the whole pipeline
down.

The subscriptions piled up because a client that subscribes and dies
before its first listen() leaves last_poll unset, and
cleanup_stale_subscriptions() only ever looked at last_poll, so such
entries were never removed.  A crash-looping external client exhausted
the pool in about an hour.

Fixes:

- cleanup_stale_subscriptions() ages never-polled subscriptions from
  their creation time.
- On every subscribe, evict that user's never-polled subscriptions
  older than abandoned_subscription_seconds (60s) and then enforce a cap
  of max_subscriptions_per_user (20) active subscriptions, returning
  HTTP 429 when exceeded.
- Pass an explicit redis_max_connections (1000) to the Redis pool.
- Close the previous PubSub when rebuilding after a connection error,
  and close the half-initialised PubSub if SUBSCRIBE fails, so neither
  path leaks a pooled connection.
- Keep the keep-alive loop alive across transient Redis errors.
- Map Redis MaxConnectionsError / ConnectionError to a 503 with
  Retry-After instead of a generic 500.

All new settings are overridable through the environment via
PubSubSettings.  Unit tests cover the cleanup, eviction, limit, rebuild
and keep-alive paths.

Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
@nuclearcat
nuclearcat added this pull request to the merge queue Oct 28, 2026
Merged via the queue into kernelci:main with commit da44dd4 Oct 28, 2026
3 checks passed
@nuclearcat
nuclearcat deleted the fix/pubsub-redis-pool-leak branch October 28, 2026 07:03
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.

1 participant