[Airflow-434] Move check for max_runs to only affect creation of new DagRun

[jbhsieh]

Dear Airflow Maintainers,

Please accept this PR that addresses the following issues:

• https://issues.apache.org/jira/browse/AIRFLOW-434

Testing Done:

• Tested locally, but could not figure out how to setup a working unit test.

Thanks.

[r39132]

Hi! I understand this problem this fix is meant to address. What you are suggesting is in fact the way things worked a few weeks ago.. if merged, we will be back to an issue that occurs when a "clear" operation is called - which results in a lot of dag runs being scheduled at one time and not honoring max_dag_runs. I need to spend some time looking at this today so that we can support both. The issue seems to be the way dags are currently prioritized... in a cross-dag run order over a in-dag run order.

[codecov-io]

Current coverage is 65.51% (diff: 0.00%)

Merging #1763 into master will increase coverage by 0.05%

@@             master      #1763   diff @@
==========================================
  Files           142        142          
  Lines          9883       9882     -1   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           6469       6474     +5   
+ Misses         3414       3408     -6   
  Partials          0          0          

Powered by Codecov. Last update f360414...d4e58d1

[jbhsieh]

@r39132 I don't believe this undoes the fix in #1716. It continues to call dag.reached_max_runs but allows existing DAGs to be processed.

It may not solve the cross-dag vs in-dag run ordering problem, and I don't know that it will prevent a backfill from exceeding the number of concurrent DAG runs. Still, it's progress from stalling ALL processing for a DAG once its max_runs is reached.

[jbhsieh]

Part of the problem is that DAGs are created as RUNNING.

The create_dagrun function (not to be confused with create_dag_run) does NOT gate on whether max_runs has been reached. The dagrun_operator, backfill, trigger_dag, and the scheduler all call create_dagrun directly and expect it to succeed.

Is it possible to add a QUEUED state to DagRun like there exists for TASKS? Just a thought.

[r39132]

@jbhsieh Sorry for the long delay in getting to this. I like your approach and we should be able to add a concept of queued to dag runs as well. Can you update your PR with your final solution and ping me to review/test/merge? It should handle the case case of a large number of cleared dags that need to be re-run but also adhere to the max_run limit. Please add a test for this.

[r39132]

Also, since this is part of cold case PR clean-ups, I need an ETA for a fix? I need a response within the next 5 days.

[jbhsieh]

@r39132 I've moved on to a different project, and don't know that I will have much time to work on this for the remainder of this calendar year. sorry.

[r39132]

No worries - reopen when you have time! We will be in a better position in the new year to quickly respond PRs.

[r39132]

Closing for now