Repository navigation
Fix use_job_schedule=False blocking asset-triggered Dag runs - #73952
luc-pimentel wants to merge 3 commits into
Conversation
kaxil
left a comment
kaxil
left a comment
There was a problem hiding this comment.
LGTM, thanks. Please update the use_job_schedule description in config.yml before merge so the config reference matches the new behaviour; the other two comments are optional.
Thanks for the review! All three points were addressed |
The option is documented as turning off the scheduler's use of cron intervals, with manually triggered runs still running. Asset events are not cron intervals, so a queued event should still produce a run when the option is off. Today it does not: the event stays queued forever.
The option is documented as turning off the scheduler's use of cron intervals, but its check wrapped all automatic run creation, so asset events and partitioned-asset events stopped producing runs too and stayed queued. With the option off, a Dag that is due by its schedule never gets a run, so it stays due. Were such Dags still selected, they would take the per-loop slots and keep their row locks on every loop, and on PostgreSQL, which sorts the asset-triggered Dags' empty next_dagrun_create_after last, they could keep those Dags out of every batch. They are left out of the query instead. The best-practices page offered the option as a way to stop all new Dag runs during maintenance, which is no longer true for asset-triggered runs.
The config.yml description still said the option turns off cron intervals and that only manually triggered Dags keep running. It now says the scheduler stops creating runs from timetables while manual and asset-triggered runs are still created. The include_scheduled predicate is the only enforcement of the option, so build it before the query and say so in the comment. The test now limits the batch to one Dag so a filter applied after the query would fail it on Postgres, where the asset Dag's NULL sorts last.
ca94ed1 to
dda0f9b
Compare
|
Had to rebase on main to resolve the conflict with #58543 |
kaxil
left a comment
kaxil
left a comment
There was a problem hiding this comment.
@jedcunningham @uranusjr Looking for second opinion on this one from the behaviour we want from use_job_schedule
|
I'm -0.1 on the surface to this. I would have expected |
|
Yeah, I am unsure of which behaviour we want as the default or whether we need another knob to provide more controls. The use case for the issue is:
|
|
I talked this through with @jedcunningham too apart from Ash's feedback above. We think this should be considered together with the draining work.
It also matches the draining state from #72407: no scheduled or asset-triggered runs, while manual triggers, backfills and materializations still go through. use_job_schedule = False is the instance-wide version of that rule. The use case in #62929 (timetables off, assets on in a dev environment) is still valid imo. It may need finer, runtime-settable control over which triggers the scheduler acts on, rather than a new meaning for this flag. @dheerajturaga, this overlaps with your drain work and your team's use cases. What do you think? @luc-pimentel, your view too, please. If we keep the current behaviour, the fix would be updating the |
|
I agree with keeping the current behaviour. I read the config description literally and didn't weigh the maintenance use, and the draining state in #72407 already follows the same rule... When I tested on 3.3.2, the asset event stayed queued while the option was off. I can rework this PR to be docs only: revert the scheduler change, update the |
This branch has not been deployed
4 participants
[scheduler] use_job_schedule = Falseis documented as turning off cron scheduling, but it also stopped asset-triggered Dag runs. The asset event stayed queued and no run was created.With this change, asset-triggered runs are created even when the option is off. In that case the scheduler only picks Dags with queued asset events, so Dags that are due on a schedule don't use up
max_dagruns_to_create_per_loop. I also updated the tip inbest-practices.rstthat said the option stops all new runs.Tested on 3.3.2 and main with
airflow standaloneand the Dag from the issue, posting an asset event through the REST API:use_job_scheduleTrueFalseWith the option off, cron Dags still get no runs and manual runs still work. Three of the four new tests fail without this change.
#62931 and #67764 tried a similar fix and were closed before merging.
closes: #62929
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 5.5) following the guidelines