Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pre-mature evaluation of tasks in mapped task group #40460

Merged

Conversation

shahar1
Copy link
Contributor

@shahar1 shahar1 commented Jun 27, 2024

closes: #34023
previous attempts: #34337 (and subsequent #36462 which has been stale and closed)
related issues: #35541

ports: #44937


This PR deals with handling pre-mature evaluation of tasks in mapped task group, which happens when using one of the "fast" trigger rules, and specifically ONE_FAILED [1]. This PR adds the exact logic presented by @uranusjr and @ephraimbuddy in the previous attempt (#34337), with a slight modification to be executed only for the appropriate trigger rules (the previous attempt has been reverted due to a bug that it caused). The slight modification is rather adding a simple if statement, which seems to do the job.
For a more robust and performent solution, I think that eventually we'll have to completely refactor the trigger rule dependencies (as their code is quite messy atm).

[1] Theoretically it might also happen in the other "fast" trigger rules (ONE_SUCCESS, ONE_DONE), but I couldn't come up with good test case for that.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@shahar1 shahar1 force-pushed the fix-premature-evaluation-in-mapped-task-group branch 3 times, most recently from 2399576 to a69699e Compare July 13, 2024 11:40
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Aug 28, 2024
@eladkal eladkal removed the stale Stale PRs per the .github/workflows/stale.yml policy file label Aug 31, 2024
@eladkal eladkal force-pushed the fix-premature-evaluation-in-mapped-task-group branch from a69699e to 708a89c Compare August 31, 2024 07:21
@eladkal eladkal force-pushed the fix-premature-evaluation-in-mapped-task-group branch from 708a89c to 2eba0ae Compare October 4, 2024 01:56
@shahar1 shahar1 force-pushed the fix-premature-evaluation-in-mapped-task-group branch from 2eba0ae to 4d40d46 Compare October 12, 2024 13:37
@shahar1 shahar1 added area:dynamic-task-mapping AIP-42 type:bug-fix Changelog: Bug Fixes labels Oct 12, 2024
@shahar1 shahar1 added this to the Airflow 2.10.3 milestone Oct 12, 2024
@shahar1 shahar1 force-pushed the fix-premature-evaluation-in-mapped-task-group branch from 4d40d46 to caf4087 Compare October 13, 2024 20:28
@eladkal eladkal force-pushed the fix-premature-evaluation-in-mapped-task-group branch from caf4087 to 9808276 Compare December 2, 2024 15:59
@shahar1 shahar1 force-pushed the fix-premature-evaluation-in-mapped-task-group branch from 9808276 to 9e1ea99 Compare December 14, 2024 06:29
@shahar1 shahar1 force-pushed the fix-premature-evaluation-in-mapped-task-group branch from 9e1ea99 to 33dee9c Compare December 14, 2024 18:38
@shahar1 shahar1 marked this pull request as ready for review December 14, 2024 18:38
@shahar1 shahar1 force-pushed the fix-premature-evaluation-in-mapped-task-group branch 2 times, most recently from a2025d1 to ae05cab Compare December 14, 2024 19:19
@shahar1
Copy link
Contributor Author

shahar1 commented Dec 14, 2024

This PR is blocked by #44360, as MappedOperator is not implemented yet in the tasks_sdk :)
It means that its counterpart backport to v2-10-test, #44937, will actually precede this one.
@kaxil FYI

@shahar1 shahar1 marked this pull request as draft December 14, 2024 20:10
@kaxil
Copy link
Member

kaxil commented Dec 16, 2024

This PR is blocked by #44360, as MappedOperator is not implemented yet in the tasks_sdk :) It means that its counterpart backport to v2-10-test, #44937, will actually precede this one. @kaxil FYI

@amoghrajesh is planning to take that on in the next coming days

@shahar1 shahar1 force-pushed the fix-premature-evaluation-in-mapped-task-group branch from ae05cab to ab1294f Compare February 1, 2025 13:12
@shahar1 shahar1 marked this pull request as ready for review February 1, 2025 13:13
Co-authored-by: Tzu-ping Chung <[email protected]>
Co-authored-by: Ephraim Anierobi <[email protected]>
@shahar1 shahar1 force-pushed the fix-premature-evaluation-in-mapped-task-group branch from ab1294f to 63f9fed Compare February 7, 2025 10:00
@shahar1
Copy link
Contributor Author

shahar1 commented Feb 7, 2025

This PR is blocked by #44360, as MappedOperator is not implemented yet in the tasks_sdk :) It means that its counterpart backport to v2-10-test, #44937, will actually precede this one. @kaxil FYI

Not blocked anymore as mapped operators were handled in main.
Rebased and made last adjustments - we're good to go :)

@shahar1 shahar1 requested a review from ashb February 7, 2025 11:52
@shahar1 shahar1 merged commit c73783b into apache:main Feb 7, 2025
45 checks passed
@shahar1 shahar1 deleted the fix-premature-evaluation-in-mapped-task-group branch February 7, 2025 15:22
niklasr22 pushed a commit to niklasr22/airflow that referenced this pull request Feb 8, 2025
* Fix pre-mature evaluation of tasks in mapped task group

Co-authored-by: Tzu-ping Chung <[email protected]>
Co-authored-by: Ephraim Anierobi <[email protected]>

* Update airflow/ti_deps/deps/trigger_rule_dep.py

Co-authored-by: Ash Berlin-Taylor <[email protected]>

---------

Co-authored-by: Tzu-ping Chung <[email protected]>
Co-authored-by: Ephraim Anierobi <[email protected]>
Co-authored-by: Ash Berlin-Taylor <[email protected]>
ambika-garg pushed a commit to ambika-garg/airflow that referenced this pull request Feb 17, 2025
* Fix pre-mature evaluation of tasks in mapped task group

Co-authored-by: Tzu-ping Chung <[email protected]>
Co-authored-by: Ephraim Anierobi <[email protected]>

* Update airflow/ti_deps/deps/trigger_rule_dep.py

Co-authored-by: Ash Berlin-Taylor <[email protected]>

---------

Co-authored-by: Tzu-ping Chung <[email protected]>
Co-authored-by: Ephraim Anierobi <[email protected]>
Co-authored-by: Ash Berlin-Taylor <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trigger Rule ONE_FAILED does not work in task group with mapped tasks
5 participants