Don't fail task on timeout during cold shutdown #9678

Nusnus · 2025-04-29T00:34:24Z

After the soft shutdown is over, the cold shutdown cannot honor the error-handling flow, as the worker must terminate as fast as possible so we may skip the timeout failure in that specific edge case.

Summary by CodeRabbit

Bug Fixes
- Improved worker shutdown handling to prevent redundant task failure reporting and backend updates during cold shutdown.
- Enhanced cancellation logic to ensure successful but unacknowledged tasks are not canceled during shutdown or connection loss.
Tests
- Added tests to verify tasks complete successfully during soft shutdown, including scenarios with task time limits.
- Introduced unit tests to confirm correct suppression of failure and timeout handling when the worker is terminating.
- Added tests to ensure successful tasks are preserved during cancellation of unacknowledged requests.

codecov · 2025-04-29T00:39:06Z

Codecov Report

❌ Patch coverage is 74.19355% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.68%. Comparing base (30649db) to head (97a8ba5).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
celery/apps/worker.py	0.00%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9678      +/-   ##
==========================================
- Coverage   78.68%   78.68%   -0.01%     
==========================================
  Files         153      153              
  Lines       19314    19328      +14     
  Branches     2214     2221       +7     
==========================================
+ Hits        15198    15208      +10     
- Misses       3817     3822       +5     
+ Partials      299      298       -1

Flag	Coverage Δ
unittests	`78.66% <74.19%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

celery/worker/request.py

auvipy

should we consider improving test coverage and if possible some integration tests? what do you think?

t/unit/worker/test_request.py

Nusnus · 2025-05-04T11:31:55Z

@daveisfera do you want to give this PR a test to confirm it works for you as well?

celery/celery#9678

daveisfera · 2025-05-05T16:38:37Z

@daveisfera do you want to give this PR a test to confirm it works for you as well?

I tested this fix and it appears to be handling it correctly now, but the status is still incorrectly set to RETRY even though the task finished.

Here's the output:

2025-05-05T16:32:25.134218717Z [2025-05-05 16:32:25,133: INFO/MainProcess] Task mysite.celery.long_task[562e669b-41f4-4e7b-a188-e36f71560b9f] received
2025-05-05T16:32:25.135509561Z [2025-05-05 16:32:25,135: WARNING/ForkPoolWorker-1] Value: 8
2025-05-05T16:32:25.322559812Z 
2025-05-05T16:32:25.322610489Z worker: Hitting Ctrl+C again will terminate all running tasks!
2025-05-05T16:32:25.322829567Z [2025-05-05 16:32:25,322: WARNING/MainProcess] Initiating Soft Shutdown, terminating in 16 seconds
2025-05-05T16:32:33.145373138Z [2025-05-05 16:32:33,144: WARNING/ForkPoolWorker-1] Done: 8
2025-05-05T16:32:33.162053417Z [2025-05-05 16:32:33,161: INFO/ForkPoolWorker-1] Task mysite.celery.long_task[562e669b-41f4-4e7b-a188-e36f71560b9f] succeeded in 8.02601714399998s: None
2025-05-05T16:32:41.381467890Z 
2025-05-05T16:32:41.381502893Z worker: Cold shutdown (MainProcess)

And here's the result saved in the database:

sqlite> SELECT * FROM django_celery_results_taskresult ORDER BY id DESC LIMIT 10;
1|562e669b-41f4-4e7b-a188-e36f71560b9f|RETRY|application/json|utf-8|{"exc_type": "Retry", "exc_message": ["Retry(Retry(...), None, None)", null, null], "exc_module": "celery.exceptions"}|2025-05-05 16:32:41.348411||{"children": []}|"(8,)"|"{}"|mysite.celery.long_task|2025-05-05 16:32:33.155238|celery@celery|

Nusnus · 2025-05-08T14:40:10Z

@daveisfera do you want to give this PR a test to confirm it works for you as well?

I tested this fix and it appears to be handling it correctly now, but the status is still incorrectly set to RETRY even though the task finished.

Here's the output:

2025-05-05T16:32:25.134218717Z [2025-05-05 16:32:25,133: INFO/MainProcess] Task mysite.celery.long_task[562e669b-41f4-4e7b-a188-e36f71560b9f] received
2025-05-05T16:32:25.135509561Z [2025-05-05 16:32:25,135: WARNING/ForkPoolWorker-1] Value: 8
2025-05-05T16:32:25.322559812Z 
2025-05-05T16:32:25.322610489Z worker: Hitting Ctrl+C again will terminate all running tasks!
2025-05-05T16:32:25.322829567Z [2025-05-05 16:32:25,322: WARNING/MainProcess] Initiating Soft Shutdown, terminating in 16 seconds
2025-05-05T16:32:33.145373138Z [2025-05-05 16:32:33,144: WARNING/ForkPoolWorker-1] Done: 8
2025-05-05T16:32:33.162053417Z [2025-05-05 16:32:33,161: INFO/ForkPoolWorker-1] Task mysite.celery.long_task[562e669b-41f4-4e7b-a188-e36f71560b9f] succeeded in 8.02601714399998s: None
2025-05-05T16:32:41.381467890Z 
2025-05-05T16:32:41.381502893Z worker: Cold shutdown (MainProcess)

And here's the result saved in the database:

sqlite> SELECT * FROM django_celery_results_taskresult ORDER BY id DESC LIMIT 10;
1|562e669b-41f4-4e7b-a188-e36f71560b9f|RETRY|application/json|utf-8|{"exc_type": "Retry", "exc_message": ["Retry(Retry(...), None, None)", null, null], "exc_module": "celery.exceptions"}|2025-05-05 16:32:41.348411||{"children": []}|"(8,)"|"{}"|mysite.celery.long_task|2025-05-05 16:32:33.155238|celery@celery|

Thank you, I’ll check it out

daveisfera · 2025-05-19T16:39:46Z

For the sake of documentation, I tried this latest version (06dd9b3) and still had the issue of recording the status as RETRY

Nusnus · 2025-05-31T20:06:47Z

@daveisfera

For the sake of documentation, I tried this latest version (06dd9b3) and still had the issue of recording the status as RETRY

I’ve added a potential fix in 5a62a26

The idea is that canceling a successful task changes the status to RETRY, so avoiding it should fix this in theory.
Can you please verify the latest version of this PR works as expected?

Thank you!

auvipy

looks good to me

daveisfera · 2025-06-03T03:58:48Z

I’ve added a potential fix in 5a62a26

The idea is that canceling a successful task changes the status to RETRY, so avoiding it should fix this in theory. Can you please verify the latest version of this PR works as expected?

The status is still set to RETRY even though the task completes successfully. That doesn't reflect what actually happened and seems "wrong", so why not set it to SUCCESS to match the actual result?

celery/celery#9678

Nusnus · 2025-11-16T08:33:00Z

Could be related: Ensure that task results are delivered during pool shutdown #435

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

… to RETRY

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull Request Overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

celery/apps/worker.py

Nusnus · 2025-11-16T14:08:50Z

This issue is much more challenging than what any AI can handle right now; avoid trusting it here @auvipy

I'm giving it another shot right now, please so not make any change to the PR/branch 🙏

auvipy · 2025-11-16T14:10:33Z

This issue is much more challenging than what any AI can handle right now; avoid trusting it here @auvipy

I'm giving it another shot right now, please so not make any change to the PR/branch 🙏

no worries, not going to merge anything here from it, as already got fooled once by him! :p

Nusnus · 2025-11-16T14:10:50Z

Could be related: Ensure that task results are delivered during pool shutdown #435

Just for the record: this doesn't solve it.
Tested on the provided bug reproduction env.

… avoid setting the task to RETRY

Nusnus · 2025-11-16T18:36:48Z

@daveisfera

Ran my test with these latest changes and it's still failing. I noticed that it correctly sets the task to SUCCESS when it completes, but then when the cold shutdown happens, it sets it to RETRY and there's a delay in the shutdown happening even though the task is complete, so I think that's related to the incorrectly setting of status

Potential fix pushed, ready for QA!

daveisfera · 2025-11-17T15:36:53Z

Potential fix pushed, ready for QA!

Saaaaweeeet!!!! Yes, I can confirm that my test repo now works correctly!

Nusnus · 2025-11-17T21:39:43Z

Potential fix pushed, ready for QA!

Saaaaweeeet!!!! Yes, I can confirm that my test repo now works correctly!

Woohoo 🎉

Thank you for the quick confirmation!

Nusnus self-assigned this Apr 29, 2025

Nusnus force-pushed the hotfix branch from 7b6c3b0 to 5e9d6f4 Compare April 29, 2025 00:35

Nusnus mentioned this pull request Apr 29, 2025

Tasks completed during a cold shutdown incorrectly timeout with 5.5 #9505

Closed

19 tasks

Nusnus marked this pull request as ready for review April 29, 2025 00:51

Nusnus added this to the 5.5.3 milestone Apr 29, 2025

auvipy requested a review from Copilot April 29, 2025 05:11

This comment was marked as resolved.

Sign in to view

auvipy requested changes Apr 29, 2025

View reviewed changes

celery/worker/request.py Outdated Show resolved Hide resolved

celery/worker/request.py Outdated Show resolved Hide resolved

auvipy reviewed Apr 29, 2025

View reviewed changes

t/unit/worker/test_request.py Show resolved Hide resolved

daveisfera added a commit to daveisfera/celery_cold_shutdown that referenced this pull request May 5, 2025

Test fix (not reporting an error but status of result is still RETRY)

d09ec0f

celery/celery#9678

Nusnus force-pushed the hotfix branch from aa567f9 to 4668881 Compare May 8, 2025 14:38

Nusnus force-pushed the hotfix branch 2 times, most recently from c034f3a to 06dd9b3 Compare May 18, 2025 19:40

Nusnus force-pushed the hotfix branch 2 times, most recently from 6eecfaf to 5a62a26 Compare May 31, 2025 19:45

Nusnus marked this pull request as draft May 31, 2025 19:54

Nusnus force-pushed the hotfix branch from 5a62a26 to ab72311 Compare June 1, 2025 20:00

Nusnus marked this pull request as ready for review June 1, 2025 20:02

auvipy previously approved these changes Jun 2, 2025

View reviewed changes

daveisfera added a commit to daveisfera/celery_cold_shutdown that referenced this pull request Jun 3, 2025

Test fix (not reporting an error but status of result is still RETRY)

4f6b98d

celery/celery#9678

auvipy modified the milestones: 5.5.3, 5.5.4 Jun 10, 2025

Nusnus force-pushed the hotfix branch from 0811cbf to 2bfec64 Compare November 9, 2025 13:29

Nusnus and others added 9 commits November 16, 2025 10:49

Don't fail task on timeout during cold shutdown

61471f7

Update celery/worker/request.py

118ea6f

Update celery/worker/request.py

dd01623

Update t/unit/worker/test_request.py

efde2ed

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update t/unit/worker/test_request.py

d451425

Fixed bug where tasks that finished during the soft shutdown were set…

afba80b

… to RETRY

Update t/unit/worker/test_request.py

8fd4d58

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update t/unit/worker/test_request.py

ca1507d

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Fixed lint error

99c6d84

Nusnus force-pushed the hotfix branch from 2bfec64 to 99c6d84 Compare November 16, 2025 08:59

auvipy requested review from auvipy and Copilot November 16, 2025 13:51

Copilot started reviewing on behalf of auvipy November 16, 2025 13:51 View session

Copilot finished reviewing on behalf of auvipy November 16, 2025 13:55

Copilot AI reviewed Nov 16, 2025

View reviewed changes

celery/apps/worker.py Show resolved Hide resolved

Nusnus added 3 commits November 16, 2025 19:44

Bugfix: Successful tasks that were canceled during soft shutdown will…

4870111

… avoid setting the task to RETRY

Renamed cancel_all_unacked_requests -> cancel_active_requests

54a5ef2

Added unit tests

97a8ba5

Nusnus merged commit 63c1910 into celery:main Nov 17, 2025
106 of 107 checks passed

Nusnus deleted the hotfix branch November 17, 2025 21:40

Nusnus modified the milestones: 5.6.x, 5.6.0 Nov 18, 2025

Uh oh!

Don't fail task on timeout during cold shutdown #9678

Don't fail task on timeout during cold shutdown #9678

Uh oh!

Conversation

Nusnus commented Apr 29, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

codecov bot commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Uh oh!

auvipy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Nusnus commented May 4, 2025

Uh oh!

daveisfera commented May 5, 2025

Uh oh!

Nusnus commented May 8, 2025

Uh oh!

daveisfera commented May 19, 2025

Uh oh!

Nusnus commented May 31, 2025

Uh oh!

auvipy left a comment

Choose a reason for hiding this comment

Uh oh!

daveisfera commented Jun 3, 2025

Uh oh!

Nusnus commented Nov 16, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Nusnus commented Nov 16, 2025

Uh oh!

auvipy commented Nov 16, 2025

Uh oh!

Nusnus commented Nov 16, 2025

Uh oh!

Nusnus commented Nov 16, 2025

Uh oh!

daveisfera commented Nov 17, 2025

Uh oh!

Nusnus commented Nov 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Nusnus commented Apr 29, 2025 •

edited by coderabbitai bot

Loading

codecov bot commented Apr 29, 2025 •

edited

Loading