Add circ-celery container image to replace circ-scripts

[dbernstein]

Description

Introduces a single circ-celery Docker image that runs one Celery process per container, with the role selected by the container command (beat or worker). One image backs the beat scheduler and every worker pool.

Unlike circ-scripts — which supervises the beat scheduler and every worker in a single container with runit — circ-celery runs exactly one process, so each role can be deployed and (for workers) autoscaled independently. Running one process per container is what lets the worker pools be scaled horizontally per queue and the beat scheduler be pinned to a single instance.

Key points:

• Boots via my_init (like the circ-webapp / circ-scripts images), so the shared /etc/my_init.d startup scripts run before Celery — see "Database migration on startup" below. --skip-runit is used because this image has no runit services; the single Celery process is my_init's main command.
• Role dispatch via the container command (beat | worker), which my_init appends to the entrypoint (docker/celery-entrypoint.sh).
• Env-driven workers — queues and concurrency are supplied per-deployment via PALACE_CELERY_QUEUES / PALACE_CELERY_CONCURRENCY, so a single image covers every worker configuration. Concurrency is a single pool shared across all queues a worker consumes; per-queue concurrency = a separate deployment.
• Logs to stdout/stderr (as JSON, via the app's logging config) instead of files, so nothing is lost when an autoscaled worker is scaled away.
• circ-scripts is left in place; deployments can be cut over role-by-role and it can be dropped in a later change.

Database migration on startup

Like circ-webapp and circ-scripts, circ-celery boots through my_init, so the shared /etc/my_init.d startup scripts run before the Celery role starts — most importantly the advisory-locked initialize_instance (alembic upgrade head). Every celery container therefore brings the schema to head on start, exactly like the other images, so the migration system stays intact and no separate, bespoke migration step is needed on the deployment side — a new worker/beat never starts against an un-migrated schema.

The celery --uid palace drop keeps HOME=/home/palace so that libpq (psycopg2) finds its ~/.postgresql TLS material and negotiates SSL correctly against an SSL-required Postgres, rather than inheriting root's HOME and falling back to a rejected plaintext connection.

Queue-depth metrics without an always-on process

The queue-depth metrics that drive worker autoscaling (QueueWaiting / QueueOldestAge, Celery CloudWatch namespace) are just a periodic Redis LLEN/LINDEX read plus put_metric_data — they don't need the always-on celery events camera. This PR publishes them from a new publish_queue_stats task on the beat schedule (every minute, routed to the high queue), so there's no standalone metrics container and no new artifact/deploy pipeline.

• The metric logic is extracted into a reusable QueueStatsReporter; the existing Cloudwatch(Polaroid) camera now composes it and still works for the circ-scripts celery events deployment.
• Namespace, Manager/QueueName dimensions, and values are unchanged, so the existing hosting-playbook CloudWatch alarms keep matching.
• Routing to high (the always-staffed, time-sensitive queue) keeps the metric flowing while the default/apply pools are saturated and scaling — exactly when the autoscaler needs it.

Draft: opened for design review of the image/entrypoint structure and the metrics approach before the hosting-playbook side (deployments + autoscaling policy) is wired up.

Motivation and Context

Replace the monolithic circ-scripts container with independently operable containers: a beat container (pinned to one instance) that kicks off scheduled tasks, and worker containers that autoscale on queue depth.

Because the image self-migrates on startup (via my_init, as above), the deployment can roll a new celery task definition the same way it rolls the web service and let the container migrate-then-start — no gated, out-of-band migration step. On the hosting-playbook side this is what lets the ECS task-service follow the same rollout pattern as the web service.

Autoscaling scale-down is safe with the existing Celery config: task_acks_late, task_reject_on_worker_lost, worker_prefetch_multiplier=1, and a 30-minute task_time_limit under the 1-hour broker visibility timeout together ensure a worker killed mid-task has its work redelivered rather than lost.

Deployment topology, replica counts, IAM (cloudwatch:PutMetricData moves from the scripts host to the worker pool), and the autoscaling policy live in hosting-playbook and are out of scope here.

Alternatives Considered

Image packaging — one configurable image vs. one image per role. We considered building separate circ-beat / circ-worker (and originally circ-cloudwatch) images. Rejected: the image contents are identical across roles — only the launch command and a few env vars differ, which are runtime concerns. Separate images would mean N Dockerfile targets to build, scan, and keep version-locked for no isolation benefit. Chosen: a single circ-celery image, role selected by the container command.

Database migration — self-migrate via my_init vs. a bespoke gated step. An earlier iteration ran the Celery process directly (no my_init) and left migration to a separate, gated deployment step so new code never ran against an old schema. Rejected in favor of booting through my_init like the other images: the container self-migrates on start (advisory-locked, idempotent), which keeps the migration system intact, needs no new machinery, and lets the deployment roll celery exactly like it rolls the web service.

Queue-depth metrics — how to publish QueueWaiting / QueueOldestAge without the always-on celery events camera. The key realization is that these metrics are just a periodic Redis LLEN/LINDEX read plus put_metric_data — the camera's event-stream machinery is only being used as a 60s timer, so any scheduler can do the job. Three options:

1. Keep the always-on camera as its own container. Simplest, but it's a third always-on singleton. On today's EC2 + docker-compose hosts it's effectively free (co-located), so this is only worth eliminating if/when workers move to Fargate (where each always-on task has a cost floor). Rejected as the default because it doesn't advance the "avoid an always-on metrics process" goal.

2. Scheduled Lambda (EventBridge, every minute). Near-zero run cost and fully decoupled from the worker fleet — the strongest option for an autoscaling signal. Rejected for now because the delivery plumbing doesn't exist: circulation's CI publishes only to GHCR (images) and PyPI, and has no AWS credentials at all. A Lambda would require either granting circulation CI AWS access to push code, or publishing the zip as a versioned artifact that hosting-playbook fetches — plus VPC-attaching the function to reach the broker Redis. That's a non-trivial, net-new pipeline for a metric we already collect. Kept on the table as the natural choice if we later move to Fargate or otherwise want fleet-independent publishing.

3. Custom beat Scheduler subclass that polls in the beat process. Would give the camera's fleet-independence (beat is an unavoidable always-on singleton) with zero extra containers, but couples us to beat internals and is more code. Held as a future upgrade if the residual coupling below proves to matter.

Chosen: a beat-scheduled task (publish_queue_stats, every minute on the high queue). It ships inside the already-published circ-celery image, needs no new artifact pipeline, no AWS creds in CI, and no Lambda/terraform, and reuses the exact metric logic (QueueStatsReporter). Its one tradeoff is that the metric now rides on a worker running the task, rather than an independent process; routing to high (the always-staffed, time-sensitive queue) largely defuses this, since autoscaling the default/apply pools doesn't affect high, and a total fleet outage is a separate condition the depth metric can't act on anyway (the existing alarms use treat_missing_data).

How Has This Been Tested?

• pytest tests/manager/celery/test_monitoring.py tests/manager/celery/tasks/test_monitoring.py — 18 passed (existing camera tests unchanged + new QueueStatsReporter and publish_queue_stats tests); tests/manager/service/celery/ — 7 passed.
• mypy clean on all changed modules and tests.
• docker buildx build --check --target celery passes.
• docker compose config resolves the beat / worker services and confirms the worker receives both the shared environment and its PALACE_CELERY_QUEUES / PALACE_CELERY_CONCURRENCY launch vars.
• CI integration smoke test (docker/ci/test_celery.sh) brings up Postgres/OpenSearch and verifies the worker answers celery inspect ping over the broker and that beat starts and stays running — so the my_init startup migration runs before the role process, exercising the self-migration path.
• All pre-commit hooks pass (shell/YAML/workflow validation, black/isort/autoflake, markdown lint).

Checklist

• I have updated the documentation accordingly.
• All new and existing tests passed.

[claude]

Claude finished @dbernstein's task in 7m 13s —— View job

---

Code Review — PR #3529

• Read the full PR diff
• Review the Dockerfile & entrypoint
• Review the monitoring refactor (QueueStatsReporter, publish_queue_stats)
• Review beat schedule / celery config wiring
• Review docker-compose & CI changes
• Review tests

Summary

Solid, thoroughly documented change. The QueueStatsReporter extraction is behavior-preserving for the existing Cloudwatch camera — I traced the MRO (Cloudwatch(QueueStatsReporter, Polaroid)): Polaroid.__init__ sets self.app, then QueueStatsReporter.__init__ reads it and overwrites self.logger exactly as the old code did, so the circ-scripts celery events path is unchanged. The beat-scheduled publish_queue_stats routes to high via the decorator queue= (matching the established pattern used by search_indexing and every other beat task, which rely on the task's default queue with no options in the schedule entry). The entrypoint role dispatch, the HOME=/home/palace SSL fix, the venv path (/var/www/circulation/env, per UV_PROJECT_ENVIRONMENT in the base image), and the docker-compose anchor refactor all check out. Only two minor documentation/test nits below.

Details

Minor: docker/README.md:110-113

This sentence directs operators to PALACE_CELERY_CLOUDWATCH_FLUSH_INTERVAL as a "cloudwatch flush interval" documented in the top-level README.md, but that variable does not exist anywhere in the codebase — there is no corresponding setting in service/celery/configuration.py (which defines cloudwatch_statistics_dryrun/namespace/region/upload_size but no flush interval), and it is not present in README.md. It is also not relevant to circ-celery, whose metric cadence comes from the every-minute beat schedule rather than a flush interval. Drop the clause so it only references the variables that actually exist:

The `worker` launch variables (`PALACE_CELERY_QUEUES`, `PALACE_CELERY_CONCURRENCY`,
`PALACE_CELERY_WORKER_HOSTNAME`) are documented in the top-level [`README.md`](../README.md).

circulation/docker/README.md

Lines 110 to 113 in 408094d

	The `worker` launch variables (`PALACE_CELERY_QUEUES`, `PALACE_CELERY_CONCURRENCY`,
	`PALACE_CELERY_WORKER_HOSTNAME`) and the `cloudwatch` flush interval
	(`PALACE_CELERY_CLOUDWATCH_FLUSH_INTERVAL`) are documented in the top-level
	[`README.md`](../README.md). Logs are written to stdout/stderr rather than to files, so nothing is

Nit: src/palace/manager/celery/tasks/monitoring.py:14

The reuse branch of the _reporter singleton (if _reporter is None evaluating false) is never exercised: TestPublishQueueStats resets _reporter to None and invokes the task once, so the per-process caching that is the entire purpose of the singleton goes untested (codecov flags this as the one partial line). Calling publish_queue_stats.delay().wait() twice inside the patch.object(monitoring, "QueueStatsReporter") block and keeping the mock_reporter_cls.assert_called_once() assertion would cover the branch and prove the reporter is built once and reused.

circulation/src/palace/manager/celery/tasks/monitoring.py

Line 14 in 408094d

global _reporter

---

Note on the three prior review threads (ephemeral beat schedule, point-in-time assert_running, missing HEALTHCHECK): those are already tracked in existing threads, so I haven't duplicated them here.

[codecov]

Codecov Report

❌ Patch coverage is 96.77419% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 93.45%. Comparing base (2d879be) to head (9ded01e).

Files with missing lines	Patch %	Lines
src/palace/manager/celery/tasks/monitoring.py	91.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3529      +/-   ##
==========================================
- Coverage   93.45%   93.45%   -0.01%     
==========================================
  Files         511      512       +1     
  Lines       46494    46511      +17     
  Branches     6346     6347       +1     
==========================================
+ Hits        43450    43465      +15     
- Misses       1968     1969       +1     
- Partials     1076     1077       +1     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[greptile-apps]

Greptile Summary

This PR introduces a circ-celery Docker image that runs a single Celery process per container (either beat or worker role), replacing the monolithic circ-scripts pattern of runit-supervised processes. It also extracts QueueStatsReporter from the existing Cloudwatch Polaroid camera so queue-depth metrics can be published from a beat-scheduled task (publish_queue_stats) without requiring an always-on celery events process.

• New circ-celery image: docker/celery-entrypoint.sh dispatches by role (beat/worker), inheriting the my_init startup chain (including advisory-locked Alembic migration) from the existing image base.
• QueueStatsReporter extraction: The metric-publishing logic is pulled into a standalone class that the Cloudwatch camera now composes; a new publish_queue_stats task drives it from the beat schedule, routing to the high queue so metrics flow even when default/apply pools are saturated.
• circ-scripts left in place to allow role-by-role cutover; both images share the same beat_schedule(), which now includes publish_queue_stats.

Confidence Score: 4/5

Safe to merge after resolving the double CloudWatch publishing concern for existing circ-scripts deployments.

Adding publish_queue_stats to the shared beat_schedule() means any circ-scripts deployment that picks up this code will have both the celery events Cloudwatch camera and the new scheduled task writing to the same CloudWatch namespace simultaneously. Whether this breaks existing alarms depends on the aggregation function in hosting-playbook, but it is a real, observable behavioral change for any deployment that has not yet completed the cutover. Everything else is well-implemented.

src/palace/manager/service/celery/celery.py (beat schedule change affects all deployments sharing the codebase) and src/palace/manager/celery/monitoring.py (dual publishing paths now active simultaneously during transition).

Important Files Changed

Filename	Overview
docker/celery-entrypoint.sh	New entrypoint dispatching beat/worker roles with env-driven queue/concurrency config; well-structured with clear error messages for missing required vars
docker/Dockerfile	New celery stage with correct HOME env override for psycopg2 SSL and --skip-runit flag; no HEALTHCHECK instruction already noted in review thread
src/palace/manager/celery/monitoring.py	QueueStatsReporter extracted from Cloudwatch; MRO and dual init calls are correct; introduces double-publishing risk for circ-scripts deployments that retain the celery events camera
src/palace/manager/celery/tasks/monitoring.py	New publish_queue_stats task with per-process reporter singleton; module-level global is safe with prefork workers; test correctly resets state with monkeypatch
src/palace/manager/service/celery/celery.py	publish_queue_stats added to shared beat_schedule(); monitoring module imported alongside other task modules; no routing issues since task decorator sets default queue
docker/ci/test_celery.sh	Smoke test uses inspect ping for worker liveness and log-grep for beat scheduler loop; point-in-time assert_running already flagged in review thread
.github/workflows/build.yml	circ-celery build/tag/push steps follow the identical pattern used for circ-exec/circ-scripts; CELERY_IMAGE set and test step wired correctly
tests/manager/celery/tasks/test_monitoring.py	New test mocks QueueStatsReporter to avoid Redis dependency; correctly verifies cls and run are called once; monkeypatches _reporter state for isolation
tests/manager/celery/test_monitoring.py	TestQueueStatsReporter added alongside existing TestCloudwatch; run integration path tested end-to-end with frozen time and mock redis/boto3
docker-compose.yml	Environment block extracted into a YAML anchor so celery-worker can merge it and add PALACE_CELERY_QUEUES/CONCURRENCY; clean refactor with no lost keys

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant myinit as my_init
    participant entrypoint as celery-entrypoint.sh
    participant beat as celery beat
    participant worker as celery worker (high queue)
    participant redis as Redis broker
    participant cw as CloudWatch

    myinit->>myinit: run /etc/my_init.d scripts (alembic upgrade head)
    myinit->>entrypoint: "exec with role arg (beat|worker)"

    alt "role = beat"
        entrypoint->>beat: exec celery beat --uid palace
        beat->>redis: schedule publish_queue_stats every minute
    else "role = worker"
        entrypoint->>worker: exec celery worker --queues PALACE_CELERY_QUEUES
    end

    loop every minute
        beat->>redis: enqueue publish_queue_stats to high queue
        redis->>worker: deliver task
        worker->>redis: LLEN / LINDEX per queue via QueueStatsReporter
        worker->>cw: put_metric_data QueueWaiting / QueueOldestAge
    end

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant myinit as my_init
    participant entrypoint as celery-entrypoint.sh
    participant beat as celery beat
    participant worker as celery worker (high queue)
    participant redis as Redis broker
    participant cw as CloudWatch

    myinit->>myinit: run /etc/my_init.d scripts (alembic upgrade head)
    myinit->>entrypoint: "exec with role arg (beat|worker)"

    alt "role = beat"
        entrypoint->>beat: exec celery beat --uid palace
        beat->>redis: schedule publish_queue_stats every minute
    else "role = worker"
        entrypoint->>worker: exec celery worker --queues PALACE_CELERY_QUEUES
    end

    loop every minute
        beat->>redis: enqueue publish_queue_stats to high queue
        redis->>worker: deliver task
        worker->>redis: LLEN / LINDEX per queue via QueueStatsReporter
        worker->>cw: put_metric_data QueueWaiting / QueueOldestAge
    end

Loading

_{Reviews (5): Last reviewed commit: "Self-migrate the circ-celery image via m..." | Re-trigger Greptile}

[greptile-apps]

Beat schedule file lost on every container recreation

The Celery beat schedule database is written to /var/run/celery/beat-schedule, which lives in the container's ephemeral writable layer. Any time the container is replaced — rolling deployment, crash recovery, ECS task substitution, Kubernetes pod eviction — a fresh container starts with no schedule file. Celery beat then treats every scheduled task as never having run and fires all of them immediately, causing a burst of task executions at each restart. For tasks with side effects or that are expensive, this burst can be disruptive.

Consider either: (a) mounting a persistent volume at /var/run/celery so the schedule file survives container replacement, (b) using a broker-backed scheduler like celery-redbeat that stores schedule state in Redis rather than a local file, or (c) explicitly documenting the ephemeral-burst behaviour so operators know to expect it and can make scheduled tasks idempotent.

[greptile-apps]

assert_running is a point-in-time snapshot, not a stability check

docker inspect -f '{{.State.Running}}' returns true only at the instant the check runs. A service that is crash-looping (starts, fails, restarts) can appear healthy here if the check happens to land during a brief "running" window. For celery-beat and celery-cloudwatch this is the only liveness check in the suite, so a restart-looping container will silently pass the smoke test. Consider adding a short sleep (e.g. 5–10 s) followed by a second assert_running call for those two services, or inspecting the restart count (RestartCount in docker inspect) to detect instability.

[greptile-apps]

No HEALTHCHECK in the celery stage

The other images (scripts, webapp) rely on runit or nginx/uwsgi to signal health, but the celery stage provides no HEALTHCHECK instruction. Without one, ECS, EKS, or any orchestrator that uses Docker health status will treat the container as always healthy — even if the worker has stalled and is no longer consuming tasks. For the worker role, celery inspect ping (running against the same broker) is a natural healthcheck. For beat and cloudwatch, checking that the process is still alive (CMD-SHELL pidof celery || exit 1) would at least catch a silent crash.