Skip to content

Commit ac39596

Browse files
fix(scheduler): ignore stale executor success after defer reschedule (#66431)
* fix(scheduler): ignore stale executor success after defer reschedule When a trigger moves a deferred task back to scheduled before the scheduler processes the executor success from the worker defer exit, treat it as benign (same try_number, next_method set) instead of state mismatch failure. Closes #66374 Co-authored-by: Cursor <cursoragent@cursor.com> * Remove newsfragment for bugfix (per review) --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent fe13eeb commit ac39596

2 files changed

Lines changed: 73 additions & 3 deletions

File tree

airflow-core/src/airflow/jobs/scheduler_job_runner.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,7 +1211,9 @@ def process_executor_events(
12111211
The method handles several key scenarios:
12121212
1. **Normal task completion**: Updates task states for successful/failed tasks
12131213
2. **External termination**: Detects tasks killed outside Airflow and marks them as failed
1214-
3. **Task requeuing**: Handles tasks that were requeued by other schedulers or executors
1214+
3. **Task requeuing**: Handles tasks that were requeued by other schedulers or executors,
1215+
and tasks moved to ``scheduled`` after a trigger fired so a stale executor success from the
1216+
pre-deferral worker exit does not fail the task instance
12151217
4. **Callback processing**: Sends task callback requests to DAG Processor for execution
12161218
5. **Email notifications**: Sends email notification requests to DAG Processor
12171219
@@ -1361,12 +1363,14 @@ def process_executor_events(
13611363
ti.pid,
13621364
)
13631365

1364-
# There are two scenarios why the same TI with the same try_number is queued
1365-
# after executor is finished with it:
1366+
# There are multiple scenarios why the same TI with the same try_number looks queued or
1367+
# waiting after the executor is finished with it:
13661368
# 1) the TI was killed externally and it had no time to mark itself failed
13671369
# - in this case we should mark it as failed here.
13681370
# 2) the TI has been requeued after getting deferred - in this case either our executor has it
13691371
# or the TI is queued by another job. Either ways we should not fail it.
1372+
# 3) the trigger already put the TI back to scheduled (resume after defer) but the executor success
1373+
# from the worker exit after defer() has not been processed yet - should not fail it.
13701374

13711375
# All of this could also happen if the state is "running",
13721376
# but that is handled by the scheduler detecting task instances without heartbeats.
@@ -1380,6 +1384,13 @@ def process_executor_events(
13801384
ti_requeued = (
13811385
ti.queued_by_job_id != job_id # Another scheduler has queued this task again
13821386
or executor.has_task(ti) # This scheduler has this task already
1387+
or (
1388+
# Resume-after-defer: trigger moved TI to scheduled (next_method set) before we saw the
1389+
# executor success from the defer exit for the same try_number.
1390+
ti.state == TaskInstanceState.SCHEDULED
1391+
and state == TaskInstanceState.SUCCESS
1392+
and ti.next_method is not None
1393+
)
13831394
)
13841395

13851396
if ti_queued and not ti_requeued:

airflow-core/tests/unit/jobs/test_scheduler_job.py

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,65 @@ def test_process_executor_events_ti_requeued(
817817
self.job_runner.executor.callback_sink.send.assert_not_called()
818818
mock_stats.incr.assert_not_called()
819819

820+
@mock.patch("airflow.jobs.scheduler_job_runner.TaskCallbackRequest")
821+
@mock.patch("airflow._shared.observability.metrics.stats._get_backend")
822+
def test_process_executor_events_stale_success_when_scheduled_after_defer(
823+
self, mock_get_backend, mock_task_callback, dag_maker
824+
):
825+
"""
826+
Trigger moved TI to scheduled (resume after defer) before executor success from defer exit arrived.
827+
828+
Regression for https://github.com/apache/airflow/issues/66374 — must not treat as state mismatch.
829+
"""
830+
mock_stats = mock.MagicMock(spec=StatsLogger)
831+
mock_get_backend.return_value = mock_stats
832+
dag_id = "test_process_executor_events_stale_success_scheduled_after_defer"
833+
task_id_1 = "dummy_task"
834+
835+
session = settings.Session()
836+
with dag_maker(dag_id=dag_id, fileloc="/test_path1/"):
837+
task1 = EmptyOperator(task_id=task_id_1)
838+
ti1 = dag_maker.create_dagrun().get_task_instance(task1.task_id)
839+
840+
executor = MockExecutor(do_update=False)
841+
task_callback = mock.MagicMock()
842+
mock_task_callback.return_value = task_callback
843+
scheduler_job = Job()
844+
session.add(scheduler_job)
845+
session.flush()
846+
self.job_runner = SchedulerJobRunner(scheduler_job, executors=[executor])
847+
848+
ti1.state = State.SCHEDULED
849+
ti1.next_method = "execute_callback"
850+
ti1.queued_by_job_id = scheduler_job.id
851+
ti1.try_number = 1
852+
session.merge(ti1)
853+
session.commit()
854+
855+
executor.event_buffer[ti1.key] = State.SUCCESS, None
856+
executor.has_task = mock.MagicMock(return_value=False)
857+
mock_stats.incr.reset_mock()
858+
859+
self.job_runner._process_executor_events(executor=executor, session=session)
860+
ti1.refresh_from_db(session=session)
861+
assert ti1.state == State.SCHEDULED
862+
self.job_runner.executor.callback_sink.send.assert_not_called()
863+
mock_stats.incr.assert_not_called()
864+
865+
# Without next_method, scheduled + stale success is still a mismatch (e.g. external kill).
866+
ti1.next_method = None
867+
session.merge(ti1)
868+
session.commit()
869+
870+
executor.event_buffer[ti1.key] = State.SUCCESS, None
871+
mock_stats.incr.reset_mock()
872+
873+
self.job_runner._process_executor_events(executor=executor, session=session)
874+
mock_stats.incr.assert_any_call(
875+
"scheduler.tasks.killed_externally",
876+
tags={"dag_id": dag_id, "task_id": ti1.task_id},
877+
)
878+
820879
@mock.patch("airflow.jobs.scheduler_job_runner.TaskCallbackRequest")
821880
@mock.patch("airflow._shared.observability.metrics.stats._get_backend")
822881
def test_process_executor_events_multiple_try_numbers_warns(

0 commit comments

Comments
 (0)