-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Bugfix: Revoked tasks now immediately update backend status to REVOKED #9869
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9869 +/- ##
=======================================
Coverage 88.42% 88.43%
=======================================
Files 153 153
Lines 19368 19373 +5
Branches 2227 2228 +1
=======================================
+ Hits 17127 17133 +6
Misses 1941 1941
+ Partials 300 299 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
6008d8a to
557cda3
Compare
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 backend status updates for task revocation operations in Celery. The purpose is to ensure that when tasks are revoked, the backend is properly notified to mark these tasks as revoked, which helps maintain consistency between worker state and backend state.
Key changes:
- Added backend status update calls in the
_revokefunction to mark tasks as revoked in the backend - Added defensive error handling to prevent backend failures from breaking the revocation process
- Added comprehensive test coverage for the new backend integration functionality
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| celery/worker/control.py | Added backend.mark_as_revoked calls with error handling in the _revoke function |
| t/unit/worker/test_control.py | Added test coverage for backend status updates during revocation, including error handling scenarios |
|
Testing the new GitHub Copilot instructions on this PR @auvipy FYI |
|
I think we can start with celery 5.6b1 release process |
Done: |
|
thanks! |
|
the tests are passing |
auvipy
left a 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.
as per a comment from discussion, the solution seems working for the reporter
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
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
cf44795 to
c2c8137
Compare
84a4c63 to
e360e3f
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Fixes #9844
When a task is revoked via
app.control.revoke(), the task status in the result backend was not being updated toREVOKED. The status would remainPENDINGuntil a worker actually picked up the task and discovered it was revoked.This was particularly problematic for tasks with ETA/countdown - if the worker was shut down before the ETA arrived, the task status would stay
PENDINGforever.Changes
backend.mark_as_revoked()in the_revoke()function to immediately update the backend status when a task is revokedBehavior Change
The result backend is now immediately updated when a task is revoked. Previously, the backend was only updated when a worker attempted to process the revoked task.
This is the expected/correct behavior - users calling
AsyncResult(task_id).statusafter revocation will now correctly seeREVOKEDinstead ofPENDING.