Skip to content

Commit e213ad7

Browse files
kaxildominikhei
authored andcommitted
Fix DepContext mutation leak and restore reschedule-mode guard (apache#62089)
Two related fixes for issues introduced in apache#59604: 1. `are_dependencies_met()` mutated the caller's dep_context.deps set in-place when adding ReadyToRescheduleDep for UP_FOR_RESCHEDULE TIs. Since the scheduler shares one DepContext across all TIs in a loop, this permanently leaked the dep into unrelated TIs. Fix: use `attrs.evolve` to create a new DepContext instead. 2. `ReadyToRescheduleDep` lost its fast-exit guard for non-reschedule tasks in NONE state. Without it, every task hit the `task_reschedule` table on each scheduling loop. Fix: restore the guard that short-circuits for non-reschedule, non-mapped tasks in NONE state while still honoring reschedule_date for tasks explicitly in UP_FOR_RESCHEDULE state.
1 parent f7daf7b commit e213ad7

4 files changed

Lines changed: 46 additions & 8 deletions

File tree

airflow-core/src/airflow/models/taskinstance.py

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -942,16 +942,15 @@ def are_dependencies_met(
942942
"""
943943
dep_context = dep_context or DepContext()
944944
if self.state == TaskInstanceState.UP_FOR_RESCHEDULE:
945-
# This DepContext is used when a task instance is in UP_FOR_RESCHEDULE state.
946-
#
947945
# Tasks can be put into UP_FOR_RESCHEDULE by the task runner itself (e.g. when
948-
# the worker cannot load the Dag or task). In this case, the scheduler must respect
949-
# the task instance's reschedule_date before scheduling it again.
946+
# the worker cannot load the DAG or task). The scheduler must respect the
947+
# reschedule_date before scheduling it again.
950948
#
951-
# ReadyToRescheduleDep is the only dependency that enforces this time-based gating.
952-
# We therefore extend the normal scheduling dependency set with it, instead of
953-
# modifying the global scheduler dependencies.
954-
dep_context.deps.add(ReadyToRescheduleDep())
949+
# We use attrs.evolve to create a *new* DepContext with ReadyToRescheduleDep added,
950+
# instead of mutating the caller's dep_context.deps set in-place. The same
951+
# dep_context is shared across all TIs in a scheduler loop, so mutating it would
952+
# permanently leak the dep into subsequent, unrelated TIs.
953+
dep_context = attrs.evolve(dep_context, deps=dep_context.deps | {ReadyToRescheduleDep()})
955954
failed = False
956955
verbose_aware_logger = self.log.info if verbose else self.log.debug
957956
for dep_status in self.get_failed_dep_statuses(dep_context=dep_context, session=session):

airflow-core/src/airflow/ti_deps/deps/ready_to_reschedule.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,14 @@ def _get_dep_statuses(
5858
This dependency fails if the latest reschedule request's reschedule date is still
5959
in the future.
6060
"""
61+
# Fast-exit for non-reschedule tasks in NONE state. When the task is explicitly
62+
# UP_FOR_RESCHEDULE we *always* check TaskReschedule regardless of operator type
63+
# (e.g. startup/DAG-load rescheduling). For NONE-state tasks, only reschedule-mode
64+
# sensors (and mapped tasks whose reschedule attr is unknown) need the DB query.
65+
if ti.state is None and ti.map_index < 0 and not getattr(ti.task, "reschedule", False):
66+
yield self._passing_status(reason="Task is not in reschedule mode.")
67+
return
68+
6169
if dep_context.ignore_in_reschedule_period:
6270
yield self._passing_status(
6371
reason="The context specified that being in a reschedule period was permitted."

airflow-core/tests/unit/models/test_taskinstance.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1337,6 +1337,29 @@ def test_respects_prev_dagrun_dep(self, dag_maker, session):
13371337
):
13381338
assert ti.are_dependencies_met()
13391339

1340+
def test_are_dependencies_met_does_not_mutate_shared_dep_context(self, dag_maker, session):
1341+
"""Verify that calling are_dependencies_met on an UP_FOR_RESCHEDULE TI does not
1342+
mutate the caller's DepContext.deps set. The scheduler shares one DepContext across
1343+
all TIs in a loop, so mutation would leak ReadyToRescheduleDep into unrelated TIs."""
1344+
with dag_maker("test_depctx_no_mutation", serialized=True):
1345+
EmptyOperator(task_id="t")
1346+
1347+
dr = dag_maker.create_dagrun(session=session)
1348+
ti = dr.get_task_instance(task_id="t", session=session)
1349+
ti.state = TaskInstanceState.UP_FOR_RESCHEDULE
1350+
session.merge(ti)
1351+
session.flush()
1352+
1353+
dep_context = DepContext(deps=RUNNING_DEPS)
1354+
original_deps = dep_context.deps.copy()
1355+
1356+
ti.task = dr.dag.task_dict[ti.task_id]
1357+
ti.are_dependencies_met(dep_context=dep_context, session=session)
1358+
1359+
assert dep_context.deps == original_deps, (
1360+
"DepContext.deps was mutated — ReadyToRescheduleDep leaked into the shared set"
1361+
)
1362+
13401363
@pytest.mark.parametrize(
13411364
("downstream_ti_state", "expected_are_dependents_done"),
13421365
[

airflow-core/tests/unit/ti_deps/deps/test_ready_to_reschedule_dep.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,14 @@ def test_should_pass_if_not_in_none_state(self, not_expected_tr_db_call):
107107
ti = self._get_task_instance(State.UP_FOR_RETRY)
108108
assert ReadyToRescheduleDep().is_met(ti=ti)
109109

110+
def test_should_pass_without_db_query_for_non_reschedule_task_in_none_state(
111+
self, not_expected_tr_db_call
112+
):
113+
"""Non-reschedule, non-mapped tasks in NONE state should short-circuit without a DB query."""
114+
ti = self._get_task_instance(State.NONE)
115+
ti.task.reschedule = False
116+
assert ReadyToRescheduleDep().is_met(ti=ti)
117+
110118
def test_should_pass_if_no_reschedule_record_exists(self):
111119
ti = self._get_task_instance(State.NONE)
112120
assert ReadyToRescheduleDep().is_met(ti=ti)

0 commit comments

Comments
 (0)