The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

There are some sqlite tests failing here - likely related.

@ephraimbuddy Glad to report that the scheduler and system is still responsive +24h after the deployment.
I've ran our full pipeline yesterday and now today without any hiccups.

Looks like... all good... knock on wood...

Thanks @kristoffern. Please can you update with the recent changes in #21556

Thanks @kristoffern. Please can you update with the recent changes in #21556

Any update?

@ephraimbuddy Sorry, but I'm on limited amount of time currently for this and won't have any bandwidth to test it.
Again, my apologies.

@Chris7 can you help and test the 3 patches here?
1: Patch #19769
2: Patch #21335
3: Patch #21556

Otherwise, we may have to revert this change

Is there an issue you can link with respect to why it needs a reversion? I do recall having some issue w/ tasks being submitted to celery during a deployment and getting left in queued limbo, but unsure what about the current code is problematic.

Since you are no longer having this issue, we should revert. I called your attention because of this issue: #21225 which is related to this

@pingzh, Since you also experience this, could you help in testing it? We have these patches
1: Patch #19769
2: Patch #21335
3: Patch #21556
and on the verge of reverting all if no one tests them

Thanks for the work on this @ephraimbuddy. I am not able to test right now. Our previous usage pattern was farming out a ton of celery tasks that were monitoring work being done by a remote service (Batch). We've since moved these to use the deferred pattern to monitor these tasks (this was a great thing to gain in the most recent upgrade).

So while I think the scenario still exists from my bug report, the chance of us hitting it are drastically lower (and we haven't hit it again yet) as the majority of our tasks are now in deferred.

@ephraimbuddy since we switched to SQS as our message broker from redis (it only happened when under high load) 2 years, we haven't experienced this issue.

@ephraimbuddy we can reproduce this reliably in an isolated test environment, and would love to help test this fix to get it merged up. We're running with a redis broker and pgsql results db, on k8s with KEDA auto scaling - it happens when under load during a scale-in event (shutting down a worker)

We've spent a bit of time tracking it down and (at least in our case) it looks to be a problem in celery (possibly celery/celery#7266). Airflow throws the task at celery and it just never executes, never makes it into the celery_taskmeta table. Airflow gets a celery task_id back and assumes its running, polling celery for updated status returns celery_states.PENDING indefinitely, as it has vanished and that's celery's default response for an unknown task. It seems like celery's warm shutdown on SIGTERM somehow loses tasks sometimes. We've run some tests with this patch and it seems to recover from this state nicely - I believe it's (almost exactly) what you had implemented in pull 21556.

I do have some concerns around self.tasks - in your implementation when setting the task back to SCHEDULED it is never cleared out of self.tasks, so if the re-scheduled task happens to be picked up by a different scheduler instance it will just stay in there forever (and in self.running come to think of it - they'd potentially accumulate over time and eat up all the open slots). I think it would be desirable to add a filter so each executor will only "re-schedule" tasks which it "owns" - this way the executor can clean up its own internal structures at the same time....and any tasks which are not owned by a running executor will get picked up by the adoption code and then "re-scheduled" by the new owner. I tried taking a stab at that change but it didn't quite work and I haven't figured out why yet....so maybe there's something going on I don't fully understand, I'm going to keep poking around at it though. Let me know if I should open a new PR, or if you'd like to resurrect your existing one...

Hi @repl-chris , thanks for the detailed description.
I think it'd be best if you work on this since you can reproduce it. That way we won't go back and forth on testing.
So please go ahead with a new PR.