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

Use a custom Task Router to route tasks dynamically #6849

Merged
merged 11 commits into from
Apr 16, 2020

Conversation

humitos
Copy link
Member

@humitos humitos commented Apr 2, 2020

This PR is an example using a Celery Task Router (under a Feature flag for now) to route tasks based on some project properties and queue's length. I think this could help us to get the most of the build:default queue without loosing performance on projects that don't really fit there and "auto-scale" projects to a bigger server when building time is getting closer to the limit.

The logic behind this, is that it will route the task to build:large in any of these cases:

  1. the project is using conda
  2. new project with less than N successful builds
  3. last N successful builds have a high time average

(it ignores projects that have already set build_queue attribute)

These are just three examples that I thought could be useful, but we can discuss them and add others.

I tested it with our current Docker Compose setup and I does the routing properly.

@humitos humitos requested a review from a team April 2, 2020 08:29
readthedocs/builds/tasks.py Show resolved Hide resolved
readthedocs/builds/tasks.py Outdated Show resolved Hide resolved
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

We should standardize on one way to return the deafult queue. I prefer returning None, and letting Celery handle it, since it reduces our logic.

readthedocs/builds/tasks.py Outdated Show resolved Hide resolved
readthedocs/builds/tasks.py Outdated Show resolved Hide resolved
@ericholscher
Copy link
Member

I'm 👍 on merging & shipping this once tests are fixed.

@ericholscher ericholscher merged commit 8ec4071 into master Apr 16, 2020
@ericholscher ericholscher deleted the humitos/task-routing branch April 16, 2020 20:22
@humitos humitos mentioned this pull request Apr 21, 2020
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.

3 participants