airflow icon indicating copy to clipboard operation
airflow copied to clipboard

Published 20 hours ago • verified publisher icon apache

Increment try_number while clearing deferred tasks.

Open tirkarthi opened this issue 1 year ago • 4 comments

closes: #38735 related: #38735

This is already done while marking tasks as success/failure. Should this be done for up_for_reschedule tasks too? See also https://github.com/apache/airflow/pull/30669

https://github.com/apache/airflow/blob/d03ba594b3158c127c1c1b3f1d0c31fb93104367/airflow/api/common/mark_tasks.py#L161-L164

tirkarthi avatar Apr 13 '24 09:04 tirkarthi

cc: @dstandish

bbovenzi avatar May 02 '24 16:05 bbovenzi

this should be obsoleted by this PR https://github.com/apache/airflow/pull/39336

i'm just working through bunch of test fixes cus we make a lot of assertions about try_number because it's always been so problematic

i would suggest we close this one

dstandish avatar May 02 '24 18:05 dstandish

@dstandish I agree the linked PR is a good long term solution but this PR is good enough for a bugfix in 2.9.x series with 2.10 release few months away.

tirkarthi avatar May 03 '24 00:05 tirkarthi

@dstandish I agree the linked PR is a good long term solution but this PR is good enough for a bugfix in 2.9.x series with 2.10 release few months away.

Sure no problem

dstandish avatar May 03 '24 00:05 dstandish

@tirkarthi @dstandish do we want to merge this PR for 2.9.2?

eladkal avatar Jun 03 '24 06:06 eladkal

@tirkarthi @dstandish do we want to merge this PR for 2.9.2?

@eladkal I think we cannot merge this anymore now that https://github.com/apache/airflow/pull/39336 is merged. At least not into main. I think the only way this could be released is if it's patched on a 2.9.2 branch specifically. WDYT?

dstandish avatar Jun 03 '24 20:06 dstandish

Then its up to @ephraimbuddy I think the real question here is if do we think there is value? If so then @tirkarthi will need to change the base branch of this PR from main to v2.9 stable

eladkal avatar Jun 03 '24 21:06 eladkal

Then its up to @ephraimbuddy I think the real question here is if do we think there is value? If so then @tirkarthi will need to change the base branch of this PR from main to v2.9 stable

We can only PR it to v2-9-test but not stable. However, we don't usually do that. We only PR against the test branch if we have issues during the release process and it's mostly CI changes. cc @potiuk

ephraimbuddy avatar Jun 03 '24 21:06 ephraimbuddy

I am okay with closing this since main has a long term fix and 2.10 will be released in few months hopefully. We had this patch since we found this during our upgrade to 2.7.0 using deferrable operators and the patch has been working well.

tirkarthi avatar Jun 04 '24 00:06 tirkarthi

Yeah. 2.10 is not that far away - and until then @tirkarthi can simply run their forked Airlfow until they upgrade to 2.10.

potiuk avatar Jun 04 '24 05:06 potiuk