feat(evaluation): Introduce dedicated evaluations queue - #1123
feat(evaluation): Introduce dedicated evaluations queue#1123AkhileshNegi wants to merge 2 commits into
Conversation
OpenAPI changes ⚪ No API surface changesNote This PR does not modify the API contract.
|
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Issue
Closes #1019
Summary
Fast evaluations and high-priority LLM jobs shared one Celery
defaultqueue and one worker pool, with no cap on concurrent fast-eval chunk tasks. A 500-item run fans out to 10 chunk tasks (chunk size 50), which can occupy every worker slot —x-max-priorityonly reorders queued-not-yet-claimed messages, it can't preempt a task a worker already started, so a high-priority LLM job (priority 9) waits behind eval chunks (priority 6) until a slot frees.This adds a second RabbitMQ queue (
evaluations) for the actual burst source —run_evaluation_fast_chunk+run_evaluation_fast_aggregate— consumed by a dedicated worker pool, so the isolation is physical instead of priority-advisory:backend/app/celery/celery_app.py:DEFAULT_QUEUE/EVALUATIONS_QUEUEconstants, secondQueue(...)entry (x-max-priority=10, distinct routing key).backend/app/celery/tasks/job_execution.py: only the two fast-eval tasks move toqueue=EVALUATIONS_QUEUE; every other task stays ondefault. Module docstring rewritten for the two-queue topology.docker-compose.yml/.dev.yml/.staging.yml: existingcelery_workerpinned to-Q default, newcelery_worker_evalservice added with-Q evaluations --concurrency=${CELERY_EVAL_WORKER_CONCURRENCY:-2}..env.example: newCELERY_EVAL_WORKER_CONCURRENCYknob..claude/conventions/celery.md,docs/wiki/services.md: docs updated for the two-queue setup.Note: a Celery worker with no
-Qflag auto-subscribes to every declared queue, so declaring the queue alone (without a worker pinned to it) achieves nothing — this is why an earlier attempt at multiple queues (#964) was reverted, and why this PR pairs the queue with dedicated compose workers.Production (ECS) still needs a manual companion change, not covered by this PR — ECS task defs aren't managed in this repo. The existing ECS worker(s) currently consume
defaultwith no-Qflag; until they're repointed (-Q defaulton the existing task def(s), plus a new task def running-Q evaluations), a queue-less worker there will silently also drainevaluations, and the split won't take effect in prod. See PR description history / ask reviewer for the ECS runbook if not tracked elsewhere yet.Checklist
docker compose uplocally and verified queue isolation (celery -A app.celery.celery_app inspect active_queueson each worker container reports exactly one, distinct queue name).Notes
Reviewer: production rollout needs the ECS task-def change (see Summary) applied in the same deploy window, or the split has no effect in prod. No GitHub issue tracks that yet — happy to file one if wanted.