From 828c6a5ae3c32bc7ecf44782dd9133d6cc3494a2 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 23 Sep 2026 08:52:56 -0400 Subject: [PATCH] fix: each lane needs its own celery node name, or three of four vanish (4295) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by the all-role smoke on its very first execution (run 7319), which is the whole argument for having added it one commit ago. Celery's default node name is `celery@`. In the single-container layout all four lanes share one hostname, so all four registered as the SAME node. Celery says so itself: DuplicateNodenameWarning: Received multiple replies from node name: celery@72adc5b706a7 `inspect` collapses four replies into one dict key and the last one wins, so three lanes read as absent — and WHICH three varies between calls: lanes not answering: maintenance_long, ml, worker lanes not answering: maintenance_long, scheduler, worker Fatal twice over: * The composite healthcheck can never pass. In Swarm that is a container that never goes healthy — restart loop, then an automatic rollback of a deploy whose image was fine. * `pool_grow`/`pool_shrink` take a `destination` of node names. The UI dial and the autoscaler would have resized whichever lane happened to answer rather than the one asked for — silently, and differently each time. Every celery role now starts with `-n "${CELERY_NODENAME:-celery}@%h"`, and the generated supervisord config sets that per lane. The lanes become worker@, scheduler@, maintenance_long@, ml@ — distinct, so inspect keeps four entries and `destination` addresses what it names. `inspect_lanes_sync` maps hostname to lane by QUEUES, so nothing there changes; it just stops having three of its four entries overwritten. Unset, it falls back to `celery` — exactly celery's own default — so every service in the multi-service stack is byte-identical to before, including the `celery@$HOSTNAME` healthcheck in docker-compose.yml and in the operator's Swarm stack. The test asserts DISTINCTNESS across the whole lane table rather than a fixed string per lane. The property that broke is that no two collide, and stating it that way keeps holding when a lane is added. This is the bug I said a live deploy was needed to find, found in CI instead for the price of one `docker run` — and it would have met the operator as a rollback loop on their first consolidated deploy. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR --- backend/app/scripts/gen_supervisord.py | 9 ++++- entrypoint.sh | 28 +++++++++++++++ tests/test_gen_supervisord.py | 47 ++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/backend/app/scripts/gen_supervisord.py b/backend/app/scripts/gen_supervisord.py index cd76171..059853e 100644 --- a/backend/app/scripts/gen_supervisord.py +++ b/backend/app/scripts/gen_supervisord.py @@ -98,7 +98,14 @@ def _program(lane: Lane, *, slots: int) -> str: # the lane would consume only its first queue. Silent: the worker # starts, reports healthy, and simply never picks up `import`. f'environment=CELERY_QUEUES="{",".join(lane.queues)}",' - f"CELERY_CONCURRENCY={slots}", + f"CELERY_CONCURRENCY={slots}," + # A UNIQUE celery node name per lane, and the reason is not cosmetic. + # These processes share one hostname, so celery's default + # `celery@` made all four the SAME node: inspect collapsed + # their replies, three lanes read as absent, and which three varied + # per call (run 7319). The healthcheck could never pass, and + # pool_grow's `destination` would have addressed an arbitrary lane. + f"CELERY_NODENAME={lane.name}", "autostart=true", "autorestart=true", # A lane that dies instantly and repeatedly is a broken image, not a diff --git a/entrypoint.sh b/entrypoint.sh index d7a99b2..4e679c0 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -6,6 +6,31 @@ set -euo pipefail # disagreement between them would only show up as `docker run --entrypoint` # behaving differently from `docker run`. ROLE="${1:-all}" + +# CELERY NODE NAME. Every celery role below starts with `-n $CELERY_NODENAME@%h`. +# +# Celery's default node name is `celery@`, and in the single- +# container layout all four lanes share one hostname — so all four registered +# as the SAME node. celery's own words for it, observed on run 7319: +# +# DuplicateNodenameWarning: Received multiple replies from node name: +# celery@72adc5b706a7 +# +# `inspect` then collapses four replies into one dict key and the last one +# wins, so three lanes read as absent and WHICH three varies per call: +# +# lanes not answering: maintenance_long, ml, worker +# lanes not answering: maintenance_long, scheduler, worker +# +# That is fatal twice over. The composite healthcheck can never pass, so the +# container is permanently unhealthy; and `pool_grow(destination=[hostname])` +# addresses a lane BY that name, so the UI dial and the autoscaler would have +# resized whichever lane happened to answer rather than the one asked for. +# +# The generated supervisord config sets this per lane. Unset — which is every +# service in the multi-service stack — it falls back to `celery`, exactly +# celery's own default, so `celery@$HOSTNAME` healthchecks there still work. +: "${CELERY_NODENAME:=celery}" shift || true case "$ROLE" in @@ -32,6 +57,7 @@ case "$ROLE" in CONCURRENCY="${CELERY_CONCURRENCY:-2}" echo "[entrypoint] Starting Celery worker queues=$QUEUES concurrency=$CONCURRENCY" exec celery -A backend.app.celery_app:celery worker \ + -n "${CELERY_NODENAME:-celery}@%h" \ --loglevel=info \ -Q "$QUEUES" \ --concurrency="$CONCURRENCY" @@ -47,6 +73,7 @@ case "$ROLE" in CONCURRENCY="${CELERY_CONCURRENCY:-1}" echo "[entrypoint] Starting Celery beat+worker queues=$QUEUES concurrency=$CONCURRENCY" exec celery -A backend.app.celery_app:celery worker \ + -n "${CELERY_NODENAME:-celery}@%h" \ --beat \ --loglevel=info \ -Q "$QUEUES" \ @@ -68,6 +95,7 @@ case "$ROLE" in CONCURRENCY="${CELERY_CONCURRENCY:-1}" echo "[entrypoint] Starting ML Celery worker queues=$QUEUES concurrency=$CONCURRENCY" exec celery -A backend.app.celery_app:celery worker \ + -n "${CELERY_NODENAME:-celery}@%h" \ --loglevel=info \ -Q "$QUEUES" \ --concurrency="$CONCURRENCY" diff --git a/tests/test_gen_supervisord.py b/tests/test_gen_supervisord.py index 7f27745..6e6d5fa 100644 --- a/tests/test_gen_supervisord.py +++ b/tests/test_gen_supervisord.py @@ -184,3 +184,50 @@ def test_every_program_writes_to_the_container_stdout_with_its_lane_named(): assert cp.getint(section, "stdout_logfile_maxbytes") == 0, section assert cp.get(section, "redirect_stderr") == "true", section assert f"[{name}] " in cp.get(section, "command"), section + + +def test_every_lane_gets_its_own_celery_node_name(): + """The bug that made the single-container layout unusable, found the first + time CI actually booted it (run 7319). + + Celery's default node name is `celery@`, and these processes + share one hostname — so all four lanes registered as the SAME node: + + DuplicateNodenameWarning: Received multiple replies from node name: + celery@72adc5b706a7 + + `inspect` then collapses four replies into one dict key, last one wins. + Three lanes read as absent, and WHICH three varied between calls: + + lanes not answering: maintenance_long, ml, worker + lanes not answering: maintenance_long, scheduler, worker + + Fatal twice: the composite healthcheck can never pass, so the container is + permanently unhealthy and Swarm restart-loops it; and `pool_grow` takes a + `destination` of node names, so the UI dial and the autoscaler would have + resized whichever lane happened to answer rather than the one asked for. + + Asserted as DISTINCTNESS across the whole table rather than as a fixed + string per lane — the property is that no two lanes collide, which is what + actually broke, and it keeps holding when a lane is added. + """ + cp = _parse() + names = {} + for section in cp.sections(): + if not section.startswith("program:"): + continue + lane = section.split(":", 1)[1] + if lane == "web": + continue # hypercorn, not a celery node + env = cp.get(section, "environment") + pairs = dict( + p.split("=", 1) for p in env.split(",") if "=" in p and not p.startswith('"') + ) + assert "CELERY_NODENAME" in pairs, f"{lane} has no CELERY_NODENAME: {env}" + names[lane] = pairs["CELERY_NODENAME"] + + assert len(set(names.values())) == len(names), ( + f"two lanes share a celery node name, which is the collapse itself: {names}" + ) + for lane, node in names.items(): + assert node == lane, f"{lane} announces itself as {node!r}"