Skip to content

fix(result): preserve the shared cluster client across operations - #146

Draft
cw-jlu wants to merge 2 commits into
taskiq-python:mainfrom
cw-jlu:fix/cluster-backend-client-lifecycle
Draft

cw-jlu wants to merge 2 commits into
taskiq-python:mainfrom
cw-jlu:fix/cluster-backend-client-lifecycle

Conversation

@cw-jlu

@cw-jlu cw-jlu commented Nov 12, 2026 •

Copy link
Copy Markdown

Finishing a result/progress write could disconnect another task's pending result, progress or readiness read. Both write methods entered the backend's shared RedisCluster as a per-operation context, whose exit closes its node connections. The read methods use the same client outside that context. Keep the shared client open between operations, preserve the explicit initialization previously performed by context entry, and let the existing backend shutdown() close it. TTL selection and serialization are unchanged.

Fixes #145. Assignment has been requested; the issue is not assigned. This is separate from the dedicated PubSub listener connection in #144.

The eight regressions cover both write methods overlapping GET, consuming GETDEL, progress GET, and readiness EXISTS. Both write methods are also exercised as the first operation on a cold client, with address remapping checked before preparing the read. A private loopback TCP relay holds the real server's reply before delivery to the tested client. It forwards bytes unchanged, uses events rather than sleeps, does not pause the shared CI cluster, and does not mock the backend or Redis client. Each case checks the pending read, subsequent operations, and named server-side connections disappearing on backend shutdown. Test keys, relay ports and client names are unique.

Actual local verification on Windows 11, Python 3.14.7, redis-py 8.0.0 and taskiq 0.13.0:

  • Final eight regressions copied to untouched main 8f70bc85dd6a469cae7d3b467af7dea720c92434, same interpreter/dependencies and real three-primary Redis 8.2.11 Cluster: 8 failed, all because the pending read loses its connection after the write. The source import was checked against the baseline worktree.
  • python -m pytest -q -n 2 --tb=short: 73 passed, with isolated Redis 8.2.11 standalone, three-primary Cluster and Sentinel services available, including the eight new cases running in parallel.
  • python -m pytest tests/test_result_backend.py tests/test_cluster_client_lifecycle.py -q --tb=short: 24 passed on real Redis 7.4.12 services as well.
  • uv run --frozen prek run --files taskiq_redis/redis_backend.py tests/test_cluster_client_lifecycle.py: all applicable hooks passed, including repository-wide Ruff/mypy (16 files), Black, whitespace and EOF checks. git diff --check passed.

The services were local Docker containers with all 16384 Cluster slots covered; the Python client/tests ran on Windows. This is not a full Taskiq worker-process end-to-end run, replica failover test, or OS/Python matrix run. Those remain CI/review work. The standalone CLIENT PAUSE reproduction in the issue was run only on a disposable owned cluster; the committed regressions do not use it.

Additional local composition check: #144, #146 and #148 were cherry-picked onto unchanged main 8f70bc85dd6a469cae7d3b467af7dea720c92434 in an isolated worktree. python -m pytest -q -n 2 --tb=short returned 81 passed with the same real Redis 8.2.11 services. This local integration branch was not pushed; the public PR branches remain independent. This is not a GitHub CI result.

Prepared with OpenAI Codex assistance. Please keep this PR in Draft pending independent and account-owner review. No account-owner human review is claimed.

Prepared with OpenAI Codex assistance; independent and account-owner review are pending.
Preserve the initialization previously provided by context entry while retaining the shared client. Exercise both write methods cold with address remapping before the overlap checks.
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.

Cluster result writes disconnect concurrent reads by closing the shared client

1 participant