RuntimeError "Detected recursive loop when walking DAG directory"

[tatiana]

Apache Airflow version

2.9.3

If "Other Airflow 2 version" selected, which one?

No response

What happened?

Disclaimer: This may be considered a bug or a change of behaviour in Airflow.

Airflow is very strict with symbolic links in the DAGs folder that lead to recursive loops:

airflow/airflow/utils/file.py

Line 243 in 965d752

"Detected recursive loop when walking DAG directory "

However, let's say someone accidentally deploys a "bad" folder (e.g. by using astro deploy --dags). To have Airflow raise a RuntimeError and stop parsing all DAGs, which may be critical to a business, could be considered an overreaction. To raise this exception can have a very high cost for business and is very disruptive to Airflow users, as recently experienced by at least one Astronomer customer.

What you think should happen instead?

I believe by default Airflow should raise warnings if it faces symbolic links that lead to recursive links and ignore them. We could expose a configuration for erroring, if users want to keep the current behaviour.

How to reproduce

In an existing working Airflow project, create a symbolic link that leads to a recursive loop, e.g.:

ln -s `pwd`/my-airflow-project/ `pwd`/my-airflow-project/dags/bla

Try to run a DAG that previously worked:

airflow dags test example_dag  `date -Iseconds`

See the exception RuntimeError: Detected recursive loop for being raised.

Operating System

N/A

Versions of Apache Airflow Providers

N/A

Deployment

Astronomer

Deployment details

ZD 57833

Anything else?

No response

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[potiuk]

This is adeliberate decision. We need to make certain assumptions about healthiness of DAGs and such errors are precisely meant to tell "it's wrong". You might think of other ways of surfacing the information (for example in the same way Import Errors are raised in the UI) if not there already but yes. Catastrofic errors and stoppig DAG processing in such case is the way how you can get attention of the users. This is really "consistency" check. When you have a recursive error, the problem is that many things might be broken, so "good" behaviour in this case is to "stop-the world" basically. Because you do not know what else can be broken.

Converting it to a discussion, but I am quite sure we won't change the "stop-the world" in this case.

[potiuk]

But of course maybe it should be more visible immediately....

[tatiana]

@potiuk I fully agree that DAGs' healthiness issues should be surfaced. I understand the rationale for not trying to hide errors and avoid making troubleshooting harder than necessary.

What about the scenario when an odd folder is unintentionally deployed by mistake in the DAGs folder, and no DAGs actually use it? It's sort of the "wrong folder" in the "wrong place" at the "wrong time." Doesn't it feel too harsh not to be able to parse any Airflow DAGs in these cases and fail to schedule everything?

[potiuk]

Also to give one more example from the past I had - generally when you see something totally unexpected (like this case) it's better to crash hard, because you never know what harm is done. Various things can happen if such recursive loop is found and many things can go wrong.

Happened to me when running a system which was "half working" because such catastriphic unknown situatiuon occured and problem was undetected for 2 days. It took me and my collegue 2 weeks to recover from wrongly charged data in billing system of telecom operator - we had to track down a lot of "bad" transactions. That tought me - "when in doubts, CRASH". Then you will get attention.

[tatiana]

This sounds awful. I wonder if there is a way of splitting use cases and treating them differently. But I also understand your perspective on `when in doubts, CRASH. '

I checked for Astronomer customers' ZenDesk tickets reporting the issue "RuntimeError: Detected recursive loop"; only four customers reported this problem over the last three years. If this was more common, then it would make sense to revisit - but since, as you suggested, this seems to be an edge case, we would be more likely to introduce more risks to Airflow by trying to change it in the core project.

A more suitable place, in our case, to address this is likely the Astro CLI: astronomer/astro-cli#1682

[potiuk]

I think also maybe a better fix would be to simply give a very explicit message and instructoins what to do in such case (and explain why it is dangerous to keep scheduling things when something like that happens) . It's always good to give more explanatory reasons and make it apparent that this is a deliberated decision to crash i this case and makes it really clear that the problem is on the user side.

This will be a clear indication to users that openinig a ticket is frutless and they should fix their broken DAG folder instead.

Recursive symbolic links to directories is a huge pain in general and generally NEVER a good thing.

[electrical]

I'm getting same kind of error, but I'm not able to find the root cause for this loop
Version 3.1.5

Error:

RuntimeError: Detected recursive loop when walking DAG directory /opt/airflow/dags: /opt/airflow/dags/.worktrees/292a564de556e743b6f268bb59325b10ec154121 has appeared more than once

File structure in the git-sync container:

git-sync@airflow-imports-dag-processor-59b468986d-lflg5:/git$ ls -lash
total 16K
4.0K drwxrwsrwx 4 root     root 4.0K Jan 12 14:54 .
4.0K drwxr-xr-x 1 root     root 4.0K Jan 12 14:54 ..
4.0K drwxr-sr-x 8 git-sync root 4.0K Jan 12 15:15 .git
4.0K drwxr-sr-x 3 git-sync root 4.0K Jan 12 14:54 .worktrees
   0 lrwxrwxrwx 1 git-sync root   51 Jan 12 14:54 repo -> .worktrees/292a564de556e743b6f268bb59325b10ec154121

I would assume it would only run through the repo directory, which should make it impossible to get the duplicate.
Unless it runs through everything in this directory including the .worktrees directory.

Anything i can check?

[electrical]

We solved it by setting dags_folder to '/opt/airflow/dags/repo/dags'.
Would have been good if this was standard