fix(redis): do not remove on_poll_start from on_tick on disconnect by antonio-mello-ai · Pull Request #2492 · celery/kombu

antonio-mello-ai · 2026-03-15T14:15:28Z

Summary

Fix race condition in Redis transport reconnection that causes "catatonic worker" — a worker that is alive (responds to ping) but silently stops consuming tasks after a broker restart.

Root cause

_on_disconnect in register_with_event_loop removes the on_poll_start callback from loop.on_tick when a Redis connection drops. During reconnection, a new channel re-registers its own on_poll_start. But if _on_disconnect from the stale channel fires after the new registration, it removes the new callback, leaving the event loop without any poll trigger. The worker stays alive but _register_BRPOP is never called — tasks pile up in Redis and are never consumed.

Fix

Remove the loop.on_tick.remove(on_poll_start) call from _on_disconnect. on_poll_start is idempotent — when there are no active file descriptors, cycle.fds is empty and it simply does nothing. Removing it from on_tick was a premature optimization that created a window for the race condition.

Companion PR

Celery: fix(worker): add hub.reset() to synloop on connection error celery#10204 — adds hub.reset() to synloop (gevent/eventlet path), which was missing the cleanup that asynloop already had.

Related issues

Worker stops consuming tasks after redis reconnection on celery 5.2.3 celery#8030 — original report (closed but not fully fixed)
Worker stops consuming tasks after redis reconnection (gevent) celery#9191 — gevent-specific variant (open)
Celery consumers losing connection behind redis haproxy celery#9631 — Redis + HAProxy variant (open)
worker stops consuming tasks after rabbitmq reconnection on celery 5 celery#9095 — Discussion with reports through Nov 2025 on Celery 5.5.3
Partial fix: Fix Redis connections after reconnect - consumer starts consuming the tasks after crash. #2007 (addressed GC of partially-initialized channels, but not this reconnection race)

Test plan

Updated existing test_register_with_event_loop__on_disconnect__loop_cleanup to verify on_poll_start stays in on_tick after disconnect
All 135 Redis transport tests pass

auvipy · 2026-03-15T18:51:21Z

@ChickenBenny please have a look

Copilot

Pull request overview

Fixes a Redis transport reconnection race where a stale disconnect handler could remove the newly-registered poll callback from the event loop, leaving workers alive but no longer consuming after a broker restart.

Changes:

Stop removing on_poll_start from hub.on_tick during Redis disconnect handling to avoid the reconnection race.
Update the Redis transport unit test to assert on_poll_start remains registered after disconnect.
Expand inline commentary/test docstrings to document the “catatonic worker” failure mode and why on_poll_start must remain.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
`kombu/transport/redis.py`	Keeps `on_poll_start` registered on disconnect to prevent the on-tick callback race that breaks consumption after reconnect.
`t/unit/transport/test_redis.py`	Adjusts the disconnect cleanup test to assert `on_poll_start` stays in `on_tick` regardless of `fds` state.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

codecov · 2026-03-16T05:36:01Z

Codecov Report

❌ Patch coverage is 92.30769% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.44%. Comparing base (c3db62b) to head (a318027).
⚠️ Report is 48 commits behind head on main.

Files with missing lines	Patch %	Lines
kombu/transport/redis.py	92.30%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2492      +/-   ##
==========================================
+ Coverage   82.37%   82.44%   +0.06%     
==========================================
  Files          79       79              
  Lines       10119    10142      +23     
  Branches     1160     1165       +5     
==========================================
+ Hits         8336     8362      +26     
+ Misses       1580     1579       -1     
+ Partials      203      201       -2

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

themavik

Keeping on_poll_start on loop.on_tick and pruning cycle._fd_to_chan in _on_disconnect targets the celery#8030 catatonic-worker race; the fileno() >= 0 guard and raw-fd branch cover the stale-map cases your tests assert.

ChickenBenny · 2026-03-24T15:21:49Z

LGTM!

auvipy · 2026-03-25T13:38:50Z

LGTM!

are you handling supporting PR for this or yours are different ones beside this?

Copilot

Pull request overview

Fixes a Redis transport reconnection race where a stale disconnect callback can remove the event-loop poll trigger (on_poll_start), leaving workers alive but no longer consuming after broker restarts.

Changes:

Stop removing on_poll_start from loop.on_tick during Redis disconnect handling.
Add disconnect-time pruning of poller fd tracking (cycle._fd_to_chan) to prevent re-registering stale sockets.
Expand Redis transport unit tests to cover new disconnect cleanup behavior and edge cases.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`kombu/transport/redis.py`	Adjusts disconnect cleanup to keep `on_poll_start` registered and prunes `_fd_to_chan` entries.
`t/unit/transport/test_redis.py`	Updates and adds tests validating `on_tick` behavior and fd pruning across multiple disconnect scenarios.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

auvipy

please cross check the review comments, and failing tests

The _on_disconnect handler in register_with_event_loop removed the on_poll_start callback from loop.on_tick when the Redis connection dropped. This caused a race condition during reconnection: if a stale channel's _on_disconnect fired after a new channel had already re-registered its on_poll_start, the new callback was removed, leaving the worker alive but unable to consume any tasks ("catatonic worker"). on_poll_start is idempotent — when there are no active file descriptors it simply does nothing. Removing it from on_tick was a premature optimization that created a window for the race condition. This fix removes the on_tick.remove() call from _on_disconnect, keeping on_poll_start registered through the full reconnection cycle. Fixes celery/celery#8030 Related: celery/celery#9191, celery/celery#9631 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

Co-authored-by: Copilot Autofix powered by AI <[email protected]>

Add four tests covering the _on_disconnect callback registered by register_with_event_loop: - socket with fileno(): loop.remove() called and fd pruned from cycle._fd_to_chan - socket without fileno() (raw int fd): fd itself used as key - fileno() raises OSError (closed socket): error swallowed gracefully - fd not in _fd_to_chan: KeyError swallowed gracefully All branches introduced in the Copilot patch (aaf75bd) are now covered, resolving the codecov/patch failure on PR #2492. Co-Authored-By: Claude Opus 4.6 <[email protected]>

When a socket is closed but not yet garbage-collected, fileno() returns -1 rather than raising OSError. The previous pruning code would attempt to delete key -1 from cycle._fd_to_chan (silently failing via KeyError), leaving the original positive fd — which maps to a stale socket — still registered. The next on_poll_start tick would then re-register that stale fd. Fix by checking `raw_fd >= 0` before treating the return value as a valid key. Also simplify the pruning path by accessing cycle._fd_to_chan directly (the attribute is always present on MultiChannelPoller) instead of going through a `hasattr` guard. Add test_register_with_event_loop__on_disconnect__fileno_negative to cover this path, bringing patch coverage to 100% on all new _on_disconnect branches. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

In async Redis mode, Connection.disconnect() clears _sock to None before invoking _on_connection_disconnect. Without this fix, stale entries remain in cycle._fd_to_chan and get re-registered by the next on_poll_start tick. Adds conservative scan of _fd_to_chan matching channels by their client/subclient connection identity. Uses tuple unpacking to match the real _fd_to_chan data structure (channel, type). Tests cover both client.connection and subclient.connection match paths, plus verifies unrelated channels are left intact. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

antonio-mello-ai · 2026-03-26T10:39:26Z

@auvipy Done — addressed the Copilot review feedback in commit c393447:

Code change: Added an else branch in _on_disconnect for the case where connection._sock is None (async Redis disconnect path). When the socket reference is already cleared, we conservatively scan cycle._fd_to_chan for channels whose client.connection or subclient.connection matches the disconnected connection, and prune those stale entries.

Note: _fd_to_chan stores (channel, type) tuples, so the scan uses tuple unpacking to access the channel correctly.

Tests: Two new tests covering:

_sock=None with stale fd pruned via client.connection match (verifies unrelated channels are left intact)
_sock=None with match via subclient.connection

Existing test fix: Updated test_register_with_event_loop__on_disconnect__loop_cleanup to provide a real _fd_to_chan dict since the else branch now iterates it.

Regarding test_connparams_health_check_interval_supported — that test fails on main as well (not related to this PR).

All checks pass: 149 tests green, pre-commit clean.

Celery 5.7.0 includes upstream fixes for the catatonic worker bug where workers reconnect after a Redis broker restart but fail to re-register their queue consumers (celery/celery#10204, celery/kombu#2492). This replaces the previous worker-health-check CLI approach with a version bump, as recommended by maintainers.

This was referenced Mar 15, 2026

fix(worker): add hub.reset() to synloop on connection error celery/celery#10204

Merged

feat(providers/celery): bump minimum celery to 5.7.0 for catatonic worker fix apache/airflow#63583

Closed

auvipy requested review from auvipy and Copilot March 15, 2026 18:51

Copilot started reviewing on behalf of auvipy March 15, 2026 18:51 View session

Copilot AI reviewed Mar 15, 2026

View reviewed changes

Comment thread kombu/transport/redis.py

antonio-mello-ai force-pushed the fix/redis-reconnect-on-tick-race branch 3 times, most recently from fd9ee39 to 7145b6e Compare March 23, 2026 00:20

themavik reviewed Mar 23, 2026

View reviewed changes

ChickenBenny mentioned this pull request Mar 25, 2026

fix: clear the timer while catch the exception celery/celery#10218

Merged

auvipy added this to the 5.7.0 milestone Mar 25, 2026

auvipy requested a review from Copilot March 25, 2026 13:35

Copilot started reviewing on behalf of auvipy March 25, 2026 13:36 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

Comment thread kombu/transport/redis.py

Comment thread t/unit/transport/test_redis.py

auvipy requested changes Mar 26, 2026

View reviewed changes

antonio-mello-ai and others added 5 commits March 26, 2026 07:25

Potential fix for pull request finding

5d78de1

Co-authored-by: Copilot Autofix powered by AI <[email protected]>

antonio-mello-ai force-pushed the fix/redis-reconnect-on-tick-race branch from 0cafd57 to c393447 Compare March 26, 2026 10:39

Merge branch 'main' into fix/redis-reconnect-on-tick-race

a318027

auvipy approved these changes Mar 26, 2026

View reviewed changes

auvipy added the Component: Redis Broker label Mar 26, 2026

auvipy added the py-redis label Mar 26, 2026

auvipy merged commit 6a37089 into celery:main Mar 26, 2026
36 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(redis): do not remove on_poll_start from on_tick on disconnect#2492

fix(redis): do not remove on_poll_start from on_tick on disconnect#2492
auvipy merged 6 commits into
celery:mainfrom
antonio-mello-ai:fix/redis-reconnect-on-tick-race

antonio-mello-ai commented Mar 15, 2026 •

edited

Loading

Uh oh!

auvipy commented Mar 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

codecov Bot commented Mar 16, 2026 •

edited

Loading

Uh oh!

themavik left a comment

Uh oh!

ChickenBenny commented Mar 24, 2026

Uh oh!

auvipy commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

auvipy left a comment

Uh oh!

antonio-mello-ai commented Mar 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Uh oh!

Conversation

antonio-mello-ai commented Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Fix

Companion PR

Related issues

Test plan

Uh oh!

auvipy commented Mar 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

codecov Bot commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

themavik left a comment

Choose a reason for hiding this comment

Uh oh!

ChickenBenny commented Mar 24, 2026

Uh oh!

auvipy commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

auvipy left a comment

Choose a reason for hiding this comment

Uh oh!

antonio-mello-ai commented Mar 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

antonio-mello-ai commented Mar 15, 2026 •

edited

Loading

codecov Bot commented Mar 16, 2026 •

edited

Loading