Clarify behaviour of test_backfill_depends_on_past #19862
Conversation
It used to be that the backfill job deadlocked when a task had "depends_on_past" set. This was because it depended on past, non-existing and not succesful task. You needed to set `ignore_first_depends_on_past' set in order to avoid the deadlock. This test was quarantined but I think the behaviour has changed. It looks like currently Airflow behaves differently: when checking dependencies, it actually retrieves the previous dag-run and if there is none available it does not deadlock no matter if depends_on_past is set. That makes much more sense and likely was fixed during the recent changes involved in timetable implementation. This PR updates the tests to show that backfill will behave properly, regardless from `ignore_first_depends_on_past`. Fixes: apache#14755
boring-cyborg
bot
added
the
area:Scheduler
potiuk
requested review from
ephraimbuddy,
jmcarp,
kaxil,
milton0825 and
uranusjr
and removed request for
ephraimbuddy
|
I am not 100% - but from what I debugged, the behaviour of backfill in case From what I see the backfil job will not deadlock any more but it will just work even if "ignore_first_depends_on_past" is set to true, simply because in prev_dagrun_dep, the prev_dagrun_dep is None (as it should be IMHO): in No matter if catchup is True or false, previous DagRun is None (as I think it should be for backfill job). However I'd love to hear other's comment if understand it right. I am not sure if there is other case where we actually still need |
Co-authored-by: Tzu-ping Chung <[email protected]>
potiuk
force-pushed
the
remove-test-backfill-depends-on-past
branch
from
8630adb to
cfeaff6
Compare
|
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. |
github-actions
bot
added
the
full tests needed
* Clarify behaviour of test_backfill_depends_on_past It used to be that the backfill job deadlocked when a task had "depends_on_past" set. This was because it depended on past, non-existing and not succesful task. You needed to set `ignore_first_depends_on_past' set in order to avoid the deadlock. This test was quarantined but I think the behaviour has changed. It looks like currently Airflow behaves differently: when checking dependencies, it actually retrieves the previous dag-run and if there is none available it does not deadlock no matter if depends_on_past is set. That makes much more sense and likely was fixed during the recent changes involved in timetable implementation. This PR updates the tests to show that backfill will behave properly, regardless from `ignore_first_depends_on_past`. Fixes: apache#14755
2 participants
It used to be that the backfill job deadlocked when a task had
"depends_on_past" set. This was because it depended on past,
non-existing and not succesful task.
You needed to set `ignore_first_depends_on_past' set in order to
avoid the deadlock. This test was quarantined but I think
the behaviour has changed.
It looks like currently Airflow behaves differently: when checking
dependencies, it actually retrieves the previous dag-run and
if there is none available it does not deadlock no matter if
depends_on_past is set. That makes much more sense and likely
was fixed during the recent changes involved in timetable
implementation.
This PR updates the tests to show that backfill will behave
properly, regardless from
ignore_first_depends_on_past.Fixes: #14755
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.