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

Build: explore these Celery's features #8941

Closed
humitos opened this issue Feb 17, 2022 · 4 comments
Closed

Build: explore these Celery's features #8941

humitos opened this issue Feb 17, 2022 · 4 comments
Assignees
Labels
Improvement Minor improvement to code Needed: design decision A core team decision is required Sprintable Small enough to sprint on

Comments

@humitos
Copy link
Member

humitos commented Feb 17, 2022

I haven't tested them yet, but look promising and more accurate than our current approach.

  1. raise Reject(requeue=False) on duplicated builds
  2. use a global task_cls (https://docs.celeryproject.org/en/latest/userguide/tasks.html#app-wide-usage) to logs actions
  3. use CELERY_IMPORTS to register the tasks (https://docs.celeryproject.org/en/latest/userguide/configuration.html#std-setting-imports)
  4. use CELERY_TASK_IGNORE_RESULT=True since we are not using the result at all
  5. review time_limit from DOCKER_LIMITS and for Celery task so we can handle them nicely at on_timeout. In the last case, we will be able to do a nice cleanup (related to Builds: check celery and redis before marking a build as stale #8269)
@humitos humitos added the Needed: design decision A core team decision is required label Feb 17, 2022
@humitos
Copy link
Member Author

humitos commented Feb 23, 2022

This setting would be useful as well to avoid zombie builds when the worker is killed by ASG, https://docs.celeryproject.org/en/stable/userguide/configuration.html#task-reject-on-worker-lost

@humitos humitos added the Sprintable Small enough to sprint on label Mar 2, 2022
@humitos humitos added the Improvement Minor improvement to code label Apr 12, 2022
@humitos
Copy link
Member Author

humitos commented Apr 12, 2022

I'm not sure why, but I cross-linked this issue in this PR #9096 and it's not shown here for some reason 🤷🏼. So, I'm commenting here.

@ericholscher
Copy link
Member

I'm fine with this PR. Actually, I opened #8941 and wrote that we should use CELERY_TASK_IGNORE_RESULT=True globally so we don't store the results since we are not using them and they could generate these kinds of problems.

This seems like a good thing to do, if we aren't using the result 👍

@humitos humitos moved this to Planned in 📍Roadmap Apr 13, 2022
@humitos humitos self-assigned this May 31, 2022
@humitos
Copy link
Member Author

humitos commented Jan 17, 2024

We have been happy with Celery lately and I don't think we need to "improve" it with these features. I'm closing it for now.

@humitos humitos closed this as completed Jan 17, 2024
@github-project-automation github-project-automation bot moved this from Planned to Done in 📍Roadmap Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Minor improvement to code Needed: design decision A core team decision is required Sprintable Small enough to sprint on
Projects
Archived in project
Development

No branches or pull requests

2 participants