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

Ensure task state doesn't change when marked as failed/success/skipped #19095

Merged
merged 2 commits into from
Oct 20, 2021

Conversation

ephraimbuddy
Copy link
Contributor

Closes: #16078

Currently, when a dagrun is marked as success in the UI, the expected
sigterm is sent but the task continues running, changing the dagrun to running
before eventually failing. Same as well when marked as failed.

Also, queued tasks continue running even when the dagrun was failed

The same thing happens when a dagrun times out. The tasks that are marked
skipped, starts again, and then fails.

This PR fixes these issues


^ 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.

@boring-cyborg boring-cyborg bot added the area:Scheduler Scheduler or dag parsing Issues label Oct 20, 2021
@ephraimbuddy
Copy link
Contributor Author

I'll add tests later...

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Oct 20, 2021
@github-actions
Copy link

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.

Currently, when a dagrun is marked as success in the UI, the expected
sigterm is sent but the task continues running, changing the dagrun to running
before eventually failing. Same as well when marked as failed.

Also, queued tasks continue running even when the dagrun was failed

The same thing happens when a dagrun times out. The tasks that are marked
skipped, starts again, and then fails.

This PR fixes these issues
@ephraimbuddy ephraimbuddy added this to the Airflow 2.2.1 milestone Oct 20, 2021
@ephraimbuddy ephraimbuddy merged commit 8d703ae into apache:main Oct 20, 2021
@ephraimbuddy ephraimbuddy deleted the properly-set-state branch October 20, 2021 14:10
jedcunningham pushed a commit that referenced this pull request Oct 20, 2021
#19095)

Currently, when a dagrun is marked as success in the UI, the expected
sigterm is sent but the task continues running, changing the dagrun to running
before eventually failing. Same as well when marked as failed.

Also, queued tasks continue running even when the dagrun was failed

The same thing happens when a dagrun times out. The tasks that are marked
skipped, starts again, and then fails.

This PR fixes these issues

(cherry picked from commit 8d703ae)
ephraimbuddy added a commit to astronomer/airflow that referenced this pull request Oct 26, 2021
When changing the state of ti to None if the dagrun is in a finished state, to avoid
queueing the task, scheduler raises unexpected commit.

The reason is that I didn't pass a session to ti.set_state in apache#19095

This happens when you mark a dagrun successful/failed and there are
some taskinstances that are not yet queued in the dagrun.
jedcunningham pushed a commit to astronomer/airflow that referenced this pull request Oct 26, 2021
apache#19095)

Currently, when a dagrun is marked as success in the UI, the expected
sigterm is sent but the task continues running, changing the dagrun to running
before eventually failing. Same as well when marked as failed.

Also, queued tasks continue running even when the dagrun was failed

The same thing happens when a dagrun times out. The tasks that are marked
skipped, starts again, and then fails.

This PR fixes these issues

(cherry picked from commit 8d703ae)
sharon2719 pushed a commit to sharon2719/airflow that referenced this pull request Oct 27, 2021
apache#19095)

Currently, when a dagrun is marked as success in the UI, the expected
sigterm is sent but the task continues running, changing the dagrun to running
before eventually failing. Same as well when marked as failed.

Also, queued tasks continue running even when the dagrun was failed

The same thing happens when a dagrun times out. The tasks that are marked
skipped, starts again, and then fails.

This PR fixes these issues
jedcunningham pushed a commit to astronomer/airflow that referenced this pull request Oct 27, 2021
apache#19095)

Currently, when a dagrun is marked as success in the UI, the expected
sigterm is sent but the task continues running, changing the dagrun to running
before eventually failing. Same as well when marked as failed.

Also, queued tasks continue running even when the dagrun was failed

The same thing happens when a dagrun times out. The tasks that are marked
skipped, starts again, and then fails.

This PR fixes these issues

(cherry picked from commit 8d703ae)
@jedcunningham jedcunningham added the type:bug-fix Changelog: Bug Fixes label Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Scheduler Scheduler or dag parsing Issues full tests needed We need to run full set of tests for this PR to merge type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Queued tasks become running after dagrun is marked failed
3 participants