-
Notifications
You must be signed in to change notification settings - Fork 11
Increase the length a job can run in RabbitMQ #1060
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for antenna-preview canceled.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds RabbitMQ/Celery environment variables: local env receives a new Erlang consumer timeout arg; production example gains a RabbitMQ block (broker URL, results backend set to rpc://, default user/pass placeholders, and the same Erlang consumer timeout). Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.envs/.production/.django-example (1)
27-32: Consider adding a comment to document the timeout value.The RabbitMQ configuration block is well-structured, and the empty credential placeholders are appropriate for an example file. The timeout value matches the local environment configuration, ensuring consistency.
Consider adding a brief comment to explain the timeout value:
# RabbitMQ CELERY_BROKER_URL= CELERY_RESULT_BACKEND=rpc:// # Use RabbitMQ for results backend RABBITMQ_DEFAULT_USER= RABBITMQ_DEFAULT_PASS= +# Consumer timeout: 60480000 ms = 16.8 hours (for long-running tasks) RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS=-rabbit consumer_timeout 60480000This helps operators understand the timeout duration when configuring production environments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.envs/.local/.django(1 hunks).envs/.production/.django-example(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Agent
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
- GitHub Check: test
🔇 Additional comments (1)
.envs/.local/.django (1)
24-24: All verification checks passed—no issues found.The shell script output confirms that the RabbitMQ container in docker-compose loads environment variables from
./.envs/.local/.djangovia theenv_filedirective, ensuring thatRABBITMQ_SERVER_ADDITIONAL_ERL_ARGSis properly consumed. The web search validates that the Erlang argument syntax-rabbit consumer_timeout 60480000is correct for RabbitMQ 3.x+ and is the recommended approach for Docker deployments. The configuration is appropriately implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds RabbitMQ consumer_timeout configuration to address an issue where long-running Celery tasks were being terminated after 30 minutes (RabbitMQ's default timeout). The configuration is added to both local development and production example environment files to allow jobs to run longer without being prematurely killed by RabbitMQ.
Key changes:
- Added
RABBITMQ_SERVER_ADDITIONAL_ERL_ARGSenvironment variable withconsumer_timeoutset to 60480000 ms (~16.8 hours) - Added comprehensive RabbitMQ configuration section to production example file including broker URL, result backend, and authentication variables
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.envs/.production/.django-example |
Added new RabbitMQ configuration section with broker URL, result backend, credentials, and consumer timeout settings |
.envs/.local/.django |
Added consumer timeout configuration to existing RabbitMQ settings |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Important note: I realized that I had previously increased the consumer timeout above the default on the server, and then recently reduced the timeout back to the default. Here are my notes. We need to start committing this to the devops repo. |
Summary
Long running tasks continue to go missing periodically (the parent job status never changes, and no additional progress is made). @carlosgjs discovered a behavior specific to RabbitMQ that may be killing our long-running processing tasks prematurely and the setting that seems to alleviate it after initial testing:
consumer_timeout. The variable must be set on the server where RabbitMQ is running, but in this PR we add it to the example production .env file for documentation purposes.Here @carlosgjs notes
The change was initially added in this PR but I decided to make an explicit one.
d2711a7#diff-f9a9a1ff76dacf233aaeb2ff844646b76955e06f958a597b14ebff11b50297b3R24
Related Issues
Part of the fixes for #721 and overall robustness for the current synchronous processing service API
How to Test the Changes
Start a job that runs over 30 minutes (the previous default?)
Screenshots
If applicable, add screenshots to help explain this PR (ex. Before and after for UI changes).
Deployment Notes
Set
consumer_timeoutto60480000inrabbitmq.confon the RabbitMQ serverChecklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.
✏️ Tip: You can customize this high-level summary in your review settings.