kaxil commented on code in PR #70307:
URL: https://github.com/apache/airflow/pull/70307#discussion_r4282738273
##########
airflow-core/src/airflow/jobs/scheduler_job_runner.py:
##########
@@ -968,21 +980,37 @@ def _select_task_instances_to_queue(
pool_stats["open"] = open_slots
- is_done = executable_tis or len(task_instances_to_examine) <
max_tis
+ if len(executable_tis) >= max_tis:
+ break
+
# Check this to avoid accidental infinite loops
found_new_filters = (
len(starved_pools) > num_starved_pools
- or len(starved_dags) > num_starved_dags
+ or len(starved_dag_runs) > num_starved_dag_runs
or len(starved_tasks) > num_starved_tasks
or len(starved_tasks_task_dagrun_concurrency) >
num_starved_tasks_task_dagrun_concurrency
)
- if is_done or not found_new_filters:
+ if len(executable_tis) >= max_tis or not found_new_filters:
break
+ if executable_tis:
Review Comment:
Nothing tests this gate. If I change `if executable_tis:` to `if True:` so
queries before the first selection also count toward the cap, the whole of
`test_scheduler_job.py` still passes, because the cap test's fixture already
selects `runnable_0` on the first page. The docstring on the constant names
this gate as what stops runnable TIs starving behind blocked ones, so could we
pin it with a test? For example: five `max_active_tis_per_dag=1` tasks running
in one run and scheduled in the next, plus one lower-priority runnable task,
then call with `max_tis=1` and expect only `runnable` to be queued. With the
gate removed, that returns an empty batch on every loop.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
