[GitHub] [airflow] uranusjr commented on a change in pull request #18897: Fix catchup by limiting queued dagrun creation using max_active_runs

[GitBox <gi...@apache.org>]

uranusjr commented on a change in pull request #18897:
URL: https://github.com/apache/airflow/pull/18897#discussion_r730893370



##########
File path: airflow/models/dag.py
##########
@@ -2425,6 +2425,14 @@ def bulk_write_to_db(cls, dags: Collection["DAG"], session=None):
         )
         most_recent_runs = {run.dag_id: run for run in most_recent_runs_iter}
 
+        # Get number of active dagruns for all dags we are processing as a single query.
+        num_active_runs = dict(
+            session.query(DagRun.dag_id, func.count('*'))
+            .filter(DagRun.dag_id.in_(existing_dag_ids), DagRun.state.in_([State.RUNNING, State.QUEUED]))
+            .group_by(DagRun.dag_id)
+            .all()
+        )

Review comment:
       This filter pattern occurs four times in this PR that I feel we might want some abstraction over it. I don't have a very good suggestion since SQLAlchemy's query API isn't that easy to work with, however; maybe one class method `DagRun.count_all_active_runs() -> Dict[DagId, int]` and an instance method `count_active_runs() -> int`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org