(paas) place imports and reindex worker queues in dedicated containers#643
Conversation
📝 WalkthroughWalkthroughProcfile updated to split the single Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Procfile`:
- Around line 2-3: Procfile currently starts two Celery schedulers because both
Procfile entries "worker" and "workerhuge" omit the scheduler-disable flag;
update the Procfile so only one process runs the scheduler by adding the
--disable-scheduler flag to all worker entries except the one you want to run
beat (e.g., add --disable-scheduler to "workerhuge"), or alternatively create a
dedicated "beat" process and leave other entries "worker" and "workerhuge" with
--disable-scheduler; reference the --disable-scheduler flag and the Procfile
entries "worker" and "workerhuge" when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
2c22daf to
12bb31a
Compare
12bb31a to
dff40d3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Procfile (1)
2-5:⚠️ Potential issue | 🔴 CriticalCritical: multiple Celery beat schedulers will run, causing duplicate periodic tasks.
Per
src/backend/worker.pylines 143-145,worker.pyadds--beatunless--disable-scheduleris passed. Bothworker_all(Line 2) andworker_rest(Line 5) omit that flag, so each will start its own beat scheduler and every periodic/scheduled task will be enqueued at least twice (duplicate emails, reindex jobs, etc.). Only one process should run beat.🛠️ Suggested fix — pick a single beat owner
-worker_all: python worker.py -worker_imports: python worker.py --concurrency=1 --queues=imports --disable-scheduler -worker_reindex: python worker.py --concurrency=2 --queues=reindex --disable-scheduler -worker_rest: python worker.py --concurrency=4 --exclude=imports,reindex +worker_all: python worker.py --disable-scheduler +worker_imports: python worker.py --concurrency=1 --queues=imports --disable-scheduler +worker_reindex: python worker.py --concurrency=2 --queues=reindex --disable-scheduler +worker_rest: python worker.py --concurrency=4 --exclude=imports,reindexOr, preferably, run beat as its own dedicated process and disable the scheduler on every worker.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Procfile` around lines 2 - 5, Procfile currently starts multiple Celery beat schedulers because worker_all and worker_rest omit the --disable-scheduler flag; update Procfile so only one process runs beat: either (A) add --disable-scheduler to worker_all and worker_rest and create a single dedicated beat entry (e.g., "beat") that runs worker.py without --disable-scheduler, or (B) choose one existing entry to be the sole beat owner and add --disable-scheduler to all other entries; key symbols to modify are the Procfile entries worker_all and worker_rest and the worker.py behavior that injects --beat unless --disable-scheduler is present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Procfile`:
- Around line 2-5: Procfile currently defines worker_all which overlaps with
worker_imports and worker_reindex and breaks the intended concurrency isolation;
pick one of the two fixes and implement it: either remove the worker_all entry
entirely so only the dedicated process types (worker_imports, worker_reindex,
worker_rest) run, or change worker_all to exclude those dedicated queues by
adding the same exclusions as worker_rest (i.e., add --exclude=imports,reindex)
so it no longer pulls from imports/reindex; update the Procfile process line for
worker_all (or delete that line) and ensure the change aligns with the
documented deployment topology.
---
Duplicate comments:
In `@Procfile`:
- Around line 2-5: Procfile currently starts multiple Celery beat schedulers
because worker_all and worker_rest omit the --disable-scheduler flag; update
Procfile so only one process runs beat: either (A) add --disable-scheduler to
worker_all and worker_rest and create a single dedicated beat entry (e.g.,
"beat") that runs worker.py without --disable-scheduler, or (B) choose one
existing entry to be the sole beat owner and add --disable-scheduler to all
other entries; key symbols to modify are the Procfile entries worker_all and
worker_rest and the worker.py behavior that injects --beat unless
--disable-scheduler is present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Purpose
Description...
Proposal
Description...
Summary by CodeRabbit