Skip to content
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

feat: add gevent as an openedx requirement #15

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

Ian2012
Copy link
Contributor

@Ian2012 Ian2012 commented Dec 12, 2024

This PR adds gevent as an openedx requirement, allowing celery workers to run with a pool optimized for I/O bound tasks.

It also adds an examples for the recommended settings on a production system and for running a specific (and optimized) celery queue for Aspects.

@@ -0,0 +1,2 @@
RUN --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \
pip install "gevent==24.11.1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works as is? Don't you have to add a flag to celery to enable gevent?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you need to set pool=gevent and a high concurrency value such as 100 or 400. It depends on the workload. The default is prefork, which works well for CPU-bound tasks.

I've tested Aspects with a dedicated deployment and routing I/O bound tasks there, but it wasn't worth running Aspects like. However, there are advantages when running a dedicated I/O bound deployment but it would require a good mapping of tasks and that's a large effort this PR doesn't try to solve

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if aspects is not a good use case for that then what would be?

I'm thinking that you add the io_bound deployment with the correct celery and put that and the python requirements behind a configuration setting, by default false. We can later enable it when needed and use CELERY_LMS_EXPLICIT_QUEUES to route the tasks that make sense to that deployment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants