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

chore: add gevent as bundle dependency #35591

Closed
wants to merge 1 commit into from

Conversation

Ian2012
Copy link
Contributor

@Ian2012 Ian2012 commented Oct 3, 2024

Description

Adds gevent as a bundle dependency to optimize celery for I/O bound tasks when running multiple queues.

By itself this change will not affect Celery, to work you need to change the pool option from process to gevent and increase the concurrency parameter. This parameter is currently not set by tutor and shouldn't reflect any change on their installations unless it's specifically changed.

For more information on the Celery's pool option: https://celery.school/celery-worker-pools

The technical details can be found here: overhangio/tutor#1130

Supporting information

Link to other information about the change, such as Jira issues, GitHub issues, or Discourse discussions.
Be sure to check they are publicly readable, or if not, repeat the information here.

Testing instructions

Please provide detailed step-by-step instructions for testing this change.

Deadline

None

Other information

See openedx/openedx-aspects#116 (comment) for more information

@openedx-webhooks

This comment was marked as outdated.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 3, 2024
@Ian2012 Ian2012 requested a review from bmtcril October 3, 2024 18:17
@felipemontoya
Copy link
Member

@Ian2012 it took me a while to get to this as I did not understand the product requirement. Now, when reviewing the context links I found:

we have developed a tutor Celery plugin to manage multiple queues for Celery. With it, we have tested switching to a gevent pool which uses lightweight threads on the default lms worker deployment with concurrency set to 100 events. It improved a lot the performance of the tasks.

This raises so many questions for me:

  • How much is a lot of performance gains?
  • how many events/second (or any other meassure of traffic that you have studied) can regular celery handle before this 'gevent' package is needed?
  • is the eduNEXT/tutor-contrib-celery plugin obligatory for aspects?
  • Is technically possible to add this dependency from the tutor-celery-plugin?
  • will this only affect installs that are running aspects?

I think adding a new dependency, specially one that is fixed in code, but will be using in operations only is weird. The natural place for the dependency would be the actual plugin that uses the code. Even if this needs to be present in the edxapp image perhaps we should move it to openedx/platform-plugin-aspects.

@Ian2012 Ian2012 closed this Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open-source-contribution PR author is not from Axim or 2U
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants