Hi all,

I'd like to start a discussion about a regression introduced in Airflow
3.3.1. The fix we choose will be a breaking change for some users either
way.
I'm affected by this on my own Dags too, so I'd like us to agree on a
direction before anything lands.

Tracking issue: https://github.com/apache/airflow/issues/73678

WHAT HAPPENED

Since 3.3.1, the Grid and Graph views return HTTP 500 for Dags whose
TaskGroup dependencies form a cycle when each group is treated as a single
unit.
At the task level these Dags are acyclic, and they parse, schedule and run
normally.

A minimal example:

    with TaskGroup("group1"):
        a1 = EmptyOperator(task_id="a1")
        a2 = EmptyOperator(task_id="a2")
    with TaskGroup("group2"):
        b1 = EmptyOperator(task_id="b1")
        b2 = EmptyOperator(task_id="b2")

    a1 >> b1  # group1 -> group2
    b2 >> a2  # group2 -> group1

A simpler variant also triggers it: two tasks in the same group connected
through a task outside the group (a >> ext >> b).

This is common when TaskGroups are used for logical or visual grouping (for
example one group per database schema). Users have hit it when upgrading
from 2.x to 3.3.1.

The cause is a bug fix. Grid/Graph used to ignore group-to-group and
cross-group edges when ordering TaskGroups, and silently rendered an
arbitrary order. The fix made the sort account for those edges, so
group-level cycles now raise an error:
  - https://github.com/apache/airflow/pull/69933
  - https://github.com/apache/airflow/pull/70591 (backport, released in
3.3.1)

APPROACHES SO FAR

  - https://github.com/apache/airflow/pull/72822 (closed): keep these Dags
valid and make Grid/Graph fall back to a strongly-connected-component
ordering.
  - https://github.com/apache/airflow/pull/73087 (open): reject these Dags
at parse time with an import error that names the TaskGroups involved.

The arguments on each side, from the PR discussions:

  - For treating them as invalid: planned TaskGroup features such as task
loops, dynamic TaskGroups, retrying or clearing a TaskGroup, and waiting
for a TaskGroup to complete need an unambiguous group ordering. They would
have undefined behaviour on these Dags.
  - For allowing them: the task graph itself is valid. Many users use
TaskGroups only for grouping. Rejecting these Dags in a patch release would
turn a UI-only problem into Dags failing to load, which conflicts with our
deprecation policy (
https://airflow.apache.org/docs/apache-airflow/stable/release-process.html#deprecation-policy
).

QUESTIONS

1. Should Dags with group-level cycles be allowed at all, long term?

2. If we decide to disallow them, how should we roll that out? Some options:
   a) Fix the Grid/Graph 500 in 3.4.x and emit a deprecation / Dag warning
at parse time. Reject in a later minor or major release.
   b) Deprecate it starting 3.4.0

3. If we decide to allow them, is a fallback ordering in Grid/Graph (as in
#72822) acceptable? How should the planned TaskGroup features handle these
Dags?

4. Communication: should we add a known-issue note to the 3.3.1/3.3.2
release notes, and publish upgrade guidance on how to restructure affected
Dags?


Thanks,
Dheeraj

Reply via email to