Skip to content

Fix Grid and Graph 500 errors for cyclic TaskGroup dependencies - #73724

Merged
dheerajturaga merged 2 commits into
apache:mainfrom
dheerajturaga:fix-grid-graph-taskgroup-cycles
Oct 8, 2026
Merged

dheerajturaga merged 2 commits into
apache:mainfrom
dheerajturaga:fix-grid-graph-taskgroup-cycles

Conversation

@dheerajturaga

@dheerajturaga dheerajturaga commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Since 3.3.1, the Grid and Graph views return HTTP 500 for Dags whose TaskGroups depend on each other when each group is treated as a single unit. The task graph itself is acyclic, and these Dags parse, schedule and run normally. #69933 made the group-level sort account for group-to-group and cross-group edges, so these group-level cycles now raise instead of rendering in an arbitrary order.

Two shapes trigger it:

  • Sibling groups with task-level edges in both directions (a1 >> b1 and b2 >> a2 across group1 and group2).
  • Two tasks in one group bridged by a task outside it (a >> ext >> b). A task with no upstream inside its own group counts as a root of that group, so ext and the group depend on each other.

Filtering can also create the cycle in a Dag that has none. With root=g.first&include_downstream=true, partial_subset() can drop a task's only in-group upstream and turn it into a group root. So Grid and Graph need to tolerate group-level cycles whatever is decided about rejecting these Dags at parse time, which is being discussed in #73678.

The serialized TaskGroup sort now orders the siblings on a cycle as one unit, placed where its first child is declared, with its members in declaration order. Everything around the cycle keeps the existing pass-numbering order, and Dags without a group-level cycle are unaffected. The serialized and Task SDK sorts share this fallback through TaskGroupMixin in the shared dagnode library, so the deprecated DAG.topological_sort(), which has raised AirflowDagCycleException for these Dags since 3.3.1, orders them the same way. Detecting these Dags at parse time is left to the deprecation check in #73746.

This replaces #72822 with a narrower change: it drops the task-level cycle re-check (task-level cycles are already rejected at parse time).

related: #73678

Screenshots

Graph view with this change, all TaskGroups expanded. Without it, the Graph view returns HTTP 500 for each of these.

Sibling groups (a1 >> b1 and b2 >> a2)

Graph view of two sibling TaskGroups that depend on each other

A group bridged by an outside task (g.a >> bridge >> g.b)

Graph view of a TaskGroup whose two tasks are bridged by an outside task

One group per database schema

Graph view of two schema TaskGroups that depend on each other

Filtered view (root g.first, Downstream). The full Dag has no group-level cycle; the filter creates one.

Graph view filtered downstream of g.first

More: a ring of three groups, a cycle inside a nested group, and the filtered example Dag without the filter

Three groups in a ring (takes the pass-numbering path)

Graph view of three TaskGroups in a ring

Cycle inside a nested group (outer.inner and outer.bridge)

Graph view of a cycle between a nested TaskGroup and a task in its parent group

The filtered example Dag without the filter (rendered before this change too; included for comparison)

Graph view of the filtered example Dag with no filter applied

Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 5.5)

Generated-by: Claude Code (Opus 5.5) following the guidelines

@JH0917

JH0917 commented Sep 30, 2026

Copy link
Copy Markdown

Hi @dheerajturaga, thanks for driving this. We're hitting the same 500 in production: dbt models
collapsed into a TaskGroup, with a Spark task wired between two of them, so the Dag has exactly
the g.a >> ext >> g.b shape from #73678.

While looking into it, I tried a slightly different approach and opened it as #73933. Instead of
collapsing the SCC, it releases one child on the cycle inside the existing pass-numbering loop,
and acyclic Dags are sorted exactly as before. Details and a benchmark are in the PR description.


Drafted-by: Claude Code (Opus 5.5); reviewed by @JH0917 before posting

@dheerajturaga

Copy link
Copy Markdown
Member Author

@ashb your thoughts on this? Would like this to go in 3.4 as agreed in the Lazy Consensus https://lists.apache.org/thread/5fpwlxg4y6jhyrw7n2m3jz3o7t6xvhlk

Comment thread airflow-core/tests/unit/utils/test_task_group.py
Comment thread airflow-core/src/airflow/serialization/definitions/taskgroup.py Outdated
@dheerajturaga
dheerajturaga force-pushed the fix-grid-graph-taskgroup-cycles branch from fd6f270 to 3c54e11 Compare October 7, 2026 19:31
Since 3.3.1 the Grid and Graph views fail with HTTP 500 for Dags whose
TaskGroups depend on each other when each group is treated as a single
unit, even though the task graph is acyclic and the Dag parses and runs
normally. Filtering a view by a root task can create the same group-level
cycle in a Dag that has none, so the views have to tolerate these cycles
whether or not such Dags are later rejected at parse time.
Since 3.3.1 the deprecated DAG.topological_sort() raises for Dags that
pass check_cycle, for the same group-level cycles that broke Grid and
Graph. Parse-time detection of these Dags now has its own check, so
nothing relies on the raise, and the two sorts should order these
siblings the same way.

Deserialization orders children by label, which had made the expected
orders in the tests match the input order, so they could not tell
whether the fallback reorders anything.
@dheerajturaga
dheerajturaga force-pushed the fix-grid-graph-taskgroup-cycles branch from 3c54e11 to cc356fd Compare October 8, 2026 01:47
@dheerajturaga
dheerajturaga requested a review from kaxil October 8, 2026 02:07
@dheerajturaga
dheerajturaga merged commit ef9feef into apache:main Oct 8, 2026
108 checks passed
@dheerajturaga
dheerajturaga deleted the fix-grid-graph-taskgroup-cycles branch October 8, 2026 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API area:DAG-processing ready for maintainer review Set after triaging when all criteria pass.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants