Skip to content

[GMLP-10778] Keep busy pool workers and in-flight reservations across a broker reconnect (5.6.3+gumloop_0.2.1) - #5

Merged
rbehal merged 7 commits into
mainfrom
rahul-gmlp-10778
Sep 14, 2026
Merged

rbehal merged 7 commits into
mainfrom
rahul-gmlp-10778

Conversation

@rbehal

@rbehal rbehal commented Sep 14, 2026 •

Copy link
Copy Markdown

Linear: GMLP-10778

Backport of upstream celery#10346 and celery#10345 (merged June 2026, not in any 5.6.x release) onto our 5.6.3+gumloop_0.2.0 base (3e1bcb66), per the spec in the Linear comments.

Root cause

On a broker reconnect Consumer.on_close() calls AsynPool.flush(), whose finally block cleared _busy_workers even for pool children still executing an accepted task. The fair scheduler then wrote the next received task into a still-busy child's pipe, where it waited for that task to finish while other children sat idle (a 643.9s "Getting started…" stall in prod). on_close() also wiped reserved_requests, which broke our worker_disable_prefetch gate.

Changes

  • celery/concurrency/asynpool.py — flush() trims _busy_workers to the inqueue fds of alive children still holding an accepted job instead of clear().
  • celery/worker/consumer/consumer.py — on_close() keeps active requests in reserved_requests and only drops never-accepted bookkeeping.
  • Upstream regression tests in t/unit/concurrency/test_prefork.py (+5) and t/unit/worker/test_consumer.py (+2). The upstream RabbitMQ quorum-queue smoke test change is excluded (file doesn't exist in the fork).
  • Version bump celery/__init__.py → 5.6.3+gumloop_0.2.1.

Fork customizations preserved

The only fork-customized files touched are celery/__init__.py (version string only; author/homepage kept) and t/unit/worker/test_consumer.py (additions to test_Consumer only; the custom test_Tasks.test_ready_worker_limit_* tests are untouched). spawn.py, tasks.py (ready_worker_limit / disable-prefetch gate), setup.py, and the spawn docs/tests have zero diff vs main. Both ported hunks are byte-identical to upstream main.

Verification

  • git apply --check of the spec's two patches against a clean 3e1bcb66: clean; post-image blob hashes match the spec's index lines.
  • pytest t/unit/concurrency/test_prefork.py t/unit/worker/test_consumer.py t/unit/concurrency/test_spawn.py t/unit/concurrency/test_concurrency.py → 168 passed, 37 subtests passed.
  • The new tests: 21 pass on this branch; with only the two source files reverted to main, 5 fail (test_flush_preserves_busy_worker_for_accepted_job, …_via_scheduled_for_fallback, …_preserves_accepted_and_releases_unaccepted_together, test_on_close_purges_orphan_reservations_from_requests_dict, test_disable_prefetch_after_connection_loss_keeps_gate_closed).
  • t/unit/worker + t/unit/concurrency: 662 passed; the one failure (test_gevent.py::test_make_killable_target, No module named 'greenlet') is environmental and fails identically on main.
  • uv build produces gumloop_celery-5.6.3+gumloop.0.2.1.tar.gz and .whl.

Not in this PR (follow-ups from the spec)

  1. Publish sdist + wheel to Artifact Registry (python -m build && twine upload … --skip-existing, README.MD procedure). uv.lock pins the sdist, so both must be uploaded.
  2. gumloop_backend: bump backend/pyproject.toml to gumloop-celery==5.6.3+gumloop.0.2.1, uv lock --upgrade-package gumloop-celery, roll agent-execution-service first.

CI (added after the initial push)

The fork's CI had been red since it was created, independent of this change: every Ubuntu job targeted upstream's blacksmith-* runner labels (no such runners in this org, so PR #4 and main sat queued until the 24h timeout), and Unit (3.14, windows-latest) failed on a click 8.5 message change. Fixed here so this PR could actually be verified:

  • t/unit/app/test_preload_cli.py: assert "No such option" (upstream's fix for click ≥8.5).
  • Workflows: blacksmith-4vcpu-ubuntu-* → ubuntu-24.04/ubuntu-22.04, useblacksmith/setup-python → actions/setup-python@v6, useblacksmith/setup-docker-builder → docker/setup-buildx-action@v3; Codecov upload no longer fails the job (fork has no CODECOV_TOKEN).
  • Integration/Smoke test fan-out (~370 jobs, and they run against pytest-celery's PyPI celery image) only on workflow_dispatch.
  • Removed codeql-analysis.yml: the repo has default CodeQL setup enabled (it runs and passes on this PR); advanced-workflow uploads are rejected while default setup is on.
  • Dropped Python 3.9 from the Unit matrix and the dev docker/Dockerfile: current kombu/isort releases require ≥3.10 (upstream has dropped 3.9 too; the backend runs 3.10).
  • libev-dev in the apt step and dev image: cassandra-driver has no PyPy wheel and needs ev.h to build.

Result: Unit 3.10–3.14 + pypy3.11 on Ubuntu, Unit 3.14 on Windows, linter, docker-build, smoke image builds, CodeQL all green.

rbehal and others added 7 commits September 14, 2026 19:42
…onnect

Backport of upstream celery#10346 and celery#10345 onto the
5.6.3+gumloop_0.2.0 base.

On a broker connection loss Consumer.start() calls on_close(), which wipes
reserved_requests and calls AsynPool.flush(). flush() ended by clearing
_busy_workers unconditionally, even for pool children still executing an
accepted job. _busy_workers is the only state the fair scheduler consults
(on_poll_start / schedule_writes), so after a reconnect the next received
task could be written into the pipe of a child that was still mid-task and
sit there until that task finished, while other children stayed idle.
Clearing reserved_requests likewise let the worker_disable_prefetch gate
under-count the slots in use.

- AsynPool.flush(): trim _busy_workers to the inqueue fds of children that
  still hold an accepted job and are alive, instead of clearing the set.
- Consumer.on_close(): keep active requests in reserved_requests and only
  drop the bookkeeping of requests that were never accepted.

Regression tests from the upstream commits are included; the upstream
change to the RabbitMQ quorum-queue smoke test is not (not used here).
Co-authored-by: Cursor <cursoragent@cursor.com>
Backport of the upstream assertion; click 8.5 changed the message from
"No such option: --ini" to "No such option '--ini'".

Co-authored-by: Cursor <cursoragent@cursor.com>
The fork inherited upstream's Blacksmith runner labels, which this org
has no runners for, so every Ubuntu job sat queued until the 24h timeout.

- blacksmith-4vcpu-ubuntu-* -> ubuntu-24.04 / ubuntu-22.04
- useblacksmith/setup-python -> actions/setup-python
- useblacksmith/setup-docker-builder -> docker/setup-buildx-action
- Codecov upload no longer fails the job (no CODECOV_TOKEN on the fork)
- Integration/Smoke fan-out only on workflow_dispatch
- Drop codeql-analysis.yml: the repo uses default CodeQL setup, which
  rejects uploads from an advanced workflow

Co-authored-by: Cursor <cursoragent@cursor.com>
kombu's current release line requires Python >=3.10, so the 3.9 unit env
can no longer resolve its dependencies. Upstream celery has dropped 3.9
as well; the backend runs 3.10.

Co-authored-by: Cursor <cursoragent@cursor.com>
cassandra-driver has no PyPy wheel and needs ev.h to build from source
(upstream's apt step already lists libev-dev). The dev Dockerfile still
installed the dev requirements into a Python 3.9 interpreter, which can
no longer resolve isort==7.0.0.

Co-authored-by: Cursor <cursoragent@cursor.com>
@rbehal
rbehal merged commit f615167 into main Sep 14, 2026
17 checks passed
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