Skip to content

Commit 43debd4

Browse files
committed
Run loop gates and advance iterations atomically
The scheduler, the API and the UI must never see every task of a loop finished while its next pass does not exist yet. If we recorded a continuing gate as successful and created the next pass afterwards, a crash between the two would leave recovery unable to tell an unfinished advancement from a completed decision. Gate completion and next-pass creation therefore happen in one transaction under the DagRun lock, which also stops a duplicate completion from advancing the loop twice. The continue-or-stop decision is a reserved XCom stored under the gate's attempt UUID. A retried gate starts without a decision, and a retired attempt cannot advance the loop. An invalid decision moves the gate attempt to retry or failed within the same request; rejecting the request instead would leave the attempt running with nobody to finish it. Gates use normal task dependencies and an explicit loop context. A barrier that waited for every body task would override trigger rules that allow early progress. Workers and dag.test supply the same coordinates, so decisions and previous-iteration reads reach the intended gate and data. Skipping downstream of a task from outside a loop skips every live pass. Skipping only one would let later passes run work the author meant to skip.
1 parent d25a7c8 commit 43debd4

41 files changed

Lines changed: 2622 additions & 220 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎airflow-core/src/airflow/api_fastapi/execution_api/datamodels/taskinstance.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,12 +421,24 @@ def safe_extract_from_orm(cls, data: Any) -> Any:
421421
return values
422422

423423

424+
class LoopContext(BaseModel):
425+
"""Pinned loop definition and enclosing iteration for a task execution."""
426+
427+
node_id: str
428+
index: int = Field(ge=0)
429+
max_iterations: int = Field(gt=0)
430+
terminal_task_id: str
431+
terminal_is_mapped: bool
432+
433+
424434
class TIRunContext(BaseModel):
425435
"""Response schema for TaskInstance run context."""
426436

427437
dag_run: DagRun
428438
"""DAG run information for the task instance."""
429439

440+
loop: LoopContext | None = None
441+
430442
task_reschedule_count: int = 0
431443
"""How many times the task has been rescheduled."""
432444

‎airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py‎

Lines changed: 85 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
from opentelemetry.trace import StatusCode
3636
from opentelemetry.trace.propagation.tracecontext import TraceContextTextMapPropagator
3737
from pydantic import JsonValue, ValidationError
38-
from sqlalchemy import and_, func, or_, tuple_, update
38+
from sqlalchemy import and_, func, or_, tuple_, union, update
3939
from sqlalchemy.engine import CursorResult
4040
from sqlalchemy.exc import DataError, NoResultFound, SQLAlchemyError
4141
from sqlalchemy.orm import contains_eager, joinedload
@@ -57,6 +57,7 @@
5757
from airflow.api_fastapi.execution_api.datamodels.taskinstance import (
5858
DagRunNoteUpdatePayload,
5959
InactiveAssetsResponse,
60+
LoopContext,
6061
PreviousTIResponse,
6162
PrevSuccessfulDagRunResponse,
6263
TaskBreadcrumbsResponse,
@@ -94,7 +95,7 @@
9495
from airflow.models.asset import AssetActive
9596
from airflow.models.base import ID_LEN
9697
from airflow.models.dag import DagModel
97-
from airflow.models.dagrun import DagRun as DR
98+
from airflow.models.dagrun import DagRun as DR, InvalidLoopDecision
9899
from airflow.models.dynamic_region import SENTINEL_REGION_ID, AmbiguousProducerError
99100
from airflow.models.hitl import HITLDetail
100101
from airflow.models.log import Log
@@ -107,7 +108,7 @@
107108
from airflow.state import get_state_backend
108109
from airflow.triggers.base import TriggerEvent
109110
from airflow.utils.sqlalchemy import get_dialect_name
110-
from airflow.utils.state import DagRunState, TaskInstanceState, TerminalTIState
111+
from airflow.utils.state import DagRunState, IntermediateTIState, TaskInstanceState, TerminalTIState
111112

112113
router = VersionedAPIRouter()
113114

@@ -331,6 +332,19 @@ def ti_run(
331332
should_retry=_is_eligible_to_retry(previous_state, ti.try_number, ti.max_tries),
332333
multi_team=conf.getboolean("core", "multi_team"),
333334
)
335+
resolver = TaskCoordinateResolver(dag_bag, session)
336+
if loop_context := resolver.loop_context(ti):
337+
group, index = loop_context
338+
terminal = resolver.get_task(
339+
ti.dag_id, ti.run_id, group.terminal_task_id, dag_version_id=ti.dag_version_id
340+
)
341+
context.loop = LoopContext(
342+
node_id=group.node_id,
343+
index=index,
344+
max_iterations=group.max_iterations,
345+
terminal_task_id=group.terminal_task_id,
346+
terminal_is_mapped=terminal.get_needs_expansion(),
347+
)
334348

335349
# Only set for lang-SDK (foreign-runtime) tasks with a captured TaskFlow arg
336350
# spec; the route excludes unset fields, keeping regular responses lean.
@@ -430,6 +444,32 @@ def ti_update_state(
430444
raise HTTPException(status_code=409, detail={"reason": "invalid_state"})
431445
return Response(status_code=status.HTTP_204_NO_CONTENT)
432446

447+
loop_group = None
448+
loop_gate = None
449+
if isinstance(ti_patch_payload, (TISuccessStatePayload, TITerminalStatePayload)):
450+
gate_run = session.execute(
451+
select(TI.dag_id, TI.run_id).where(
452+
TI.id == task_instance_id,
453+
TI.working_set.is_(True),
454+
TI.operator == "LoopGateOperator",
455+
)
456+
).one_or_none()
457+
if gate_run is not None:
458+
session.execute(
459+
select(DR).where(DR.dag_id == gate_run.dag_id, DR.run_id == gate_run.run_id).with_for_update()
460+
).scalar_one()
461+
loop_gate = session.scalar(
462+
select(TI)
463+
.where(TI.id == task_instance_id, TI.working_set.is_(True))
464+
.with_for_update(of=TI)
465+
.execution_options(populate_existing=True)
466+
)
467+
if loop_gate is not None:
468+
loop_context = TaskCoordinateResolver(dag_bag, session).loop_context(loop_gate)
469+
if loop_context is None or loop_context[0].gate_task_id != loop_gate.task_id:
470+
raise HTTPException(status_code=409, detail={"reason": "invalid_loop_gate"})
471+
loop_group = loop_context[0]
472+
433473
old = (
434474
select(
435475
TI.state,
@@ -517,6 +557,25 @@ def ti_update_state(
517557
detail={"reason": "invalid_partition_key", "message": str(e)},
518558
) from e
519559

560+
gate_completed = False
561+
if (
562+
loop_gate is not None
563+
and loop_group is not None
564+
and isinstance(ti_patch_payload, TISuccessStatePayload)
565+
):
566+
try:
567+
loop_gate.dag_run.complete_loop_gate(
568+
loop_gate, loop_group, TaskInstanceState.SUCCESS, session=session
569+
)
570+
gate_completed = True
571+
except InvalidLoopDecision as error:
572+
log.warning("Loop gate success rejected", error=str(error))
573+
ti_patch_payload = _build_rejected_gate_payload(
574+
ti_patch_payload,
575+
reason=f"Loop gate success rejected: {error}",
576+
retry=_is_eligible_to_retry(previous_state, try_number, max_tries),
577+
)
578+
520579
# We exclude_unset to avoid updating fields that are not set in the payload
521580
data = ti_patch_payload.model_dump(
522581
exclude={"task_outlets", "outlet_events", "retry_delay_seconds", "retry_reason"},
@@ -540,6 +599,8 @@ def ti_update_state(
540599
# Let DataErrorHandler return a 422 instead of silently marking the TI FAILED below.
541600
raise
542601
except Exception:
602+
if loop_gate is not None:
603+
raise
543604
# Set a task to failed in case any unexpected exception happened during task state update
544605
log.exception(
545606
"Error updating Task Instance state. Setting the task to failed.",
@@ -590,6 +651,9 @@ def ti_update_state(
590651
# Defer to app-level SQLAlchemyError handler (returns HTTP 500).
591652
raise
592653

654+
if loop_gate is not None and loop_group is not None and not gate_completed:
655+
loop_gate.dag_run.complete_loop_gate(loop_gate, loop_group, updated_state, session=session)
656+
593657
if updated_state == TaskInstanceState.SUCCESS:
594658
if conf.getboolean("state_store", "clear_on_success"):
595659
scope = TaskScope(
@@ -623,6 +687,15 @@ def ti_update_state(
623687
callback()
624688

625689

690+
def _build_rejected_gate_payload(
691+
payload: TISuccessStatePayload, *, reason: str, retry: bool
692+
) -> TIRetryStatePayload | TITerminalStatePayload:
693+
carried = payload.model_dump(include={"end_date", "rendered_map_index"}, exclude_unset=True)
694+
if retry:
695+
return TIRetryStatePayload(state=IntermediateTIState.UP_FOR_RETRY, retry_reason=reason, **carried)
696+
return TITerminalStatePayload(state=TerminalStateNonSuccess.FAILED, retry_reason=reason, **carried)
697+
698+
626699
def _emit_task_span(ti, state, *, resolver: TaskCoordinateResolver):
627700
# just to be safe
628701
if not ti.dag_run:
@@ -926,21 +999,21 @@ def ti_skip_downstream(
926999
try:
9271000
# A bare task_id skips every TI of that task, so an already expanded mapped task
9281001
# (e.g. one mapped over a literal list) is skipped too, not only map_index -1.
929-
selected_ids = {
930-
ti.id
931-
for task in tasks
932-
for ti in resolver.resolve(
933-
dag_id=dag_id,
934-
run_id=run_id,
935-
task_id=task if isinstance(task, str) else task[0],
1002+
selects = [
1003+
resolver.select_skip_target_ids(
9361004
caller=caller,
1005+
task_id=task if isinstance(task, str) else task[0],
9371006
map_indexes=None if isinstance(task, str) else task[1],
9381007
)
939-
}
1008+
for task in tasks
1009+
]
9401010
except AmbiguousProducerError as error:
9411011
raise HTTPException(status.HTTP_409_CONFLICT, str(error)) from error
9421012
except ValueError as error:
9431013
raise HTTPException(status.HTTP_400_BAD_REQUEST, str(error)) from error
1014+
if not selects:
1015+
return
1016+
selected_ids = set(session.scalars(union(*selects)))
9441017

9451018
# Don't overwrite tasks that are already executing or finished.
9461019
# See: https://github.com/apache/airflow/issues/59378
@@ -958,6 +1031,7 @@ def ti_skip_downstream(
9581031
query = (
9591032
update(TI)
9601033
.where(
1034+
TI.working_set.is_(True),
9611035
TI.id.in_(selected_ids),
9621036
skippable_state_clause,
9631037
)

0 commit comments

Comments
 (0)