execution_timeout seems to be ignored in EcsRunTaskOperator when using deferable

[tim-x-y-z]

Apache Airflow version

2.6.3

What happened

It seems that with the new aws provider package, when using the deferable keyword in the EcsRunTaskOperator - the execution_timeout is ignored and the task is killed from another timeout, the trigger timeout seems to be timeout=timedelta(seconds=self.waiter_max_attempts * self.waiter_delay + 60).

Also, it seems when the trigger fires that timeout - it seems the task return "success" even though it hasn't finished.
It seems this doesn't kill the task either.

What you think should happen instead

The execution_timeout should be used in the trigger timeout, or at least a warning if that timeout is overriden or is smaller

How to reproduce

Run an EcsRunTaskOperator task with deferable mode, put a large execution_timeout and a small number of waiter_retries. The task should terminates based on the trigger timing out before the execution_timeout is up.

Operating System

linux ubuntu

Versions of Apache Airflow Providers

No response

Deployment

Other Docker-based deployment

Deployment details

No response

Anything else

No response

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[nathadfield]

#31881 was the PR that introduced this. @vandonr-amz are you able to determine whether this is indeed a bug?

[vandonr-amz]

Hmm that's interesting because execution_timeout is a property of the base operator (i.e. of all operators), which is usually handled "automatically", but I don't think that plays well with deferrable...
Some operators have included it manually in the defer() call, with varying logic, some others don't consider it.

I'm also confused by what's the expected behavior in this report. Setting a large execution_timeout is not going to make the trigger run longer than waiter_retries.

I actually see 3 bugs being reported:

• tasks are successful when triggers time out (probably not what we want ?)
• we hit the trigger timeout even though it should end on max_attempts by itself 60 seconds before ?
• execution_timeout is not handled properly on defer

[vandonr-amz]

After testing it, I can confirm that there was a bug where success was returned after max_attempts had been reached

however, setting a short execution_timeout works well and returns an error even when the task is in the triggerer

and setting a long execution timeout works well too, but it doesn't "override" the max attempts.
And I don't think it should be the case, except maybe if max_attempts is not specified but execution timeout is ? But it's a complex behavior, and I'm not sure it'd be a good idea.

Maybe another way to see this is that the defaults that are set for the ecs operator are too short, and we should increase those values so that less people would run into timeouts "out of the box".
We currently allow 100 pings with 6 sec delay by default, so any task taking more than 10 minutes needs an override of those params.

[github-actions]

This issue has been automatically marked as stale because it has been open for 30 days with no response from the author. It will be closed in next 7 days if no further activity occurs from the issue author.

[tim-x-y-z]

It seems this #32589 pr has fixed the fact that the task doesn't return success anymore, however - it doesn't seem to have solved the "killing of the task part".
If the execution_timeout exceeds the task doesn't seem to be killed. I should expect that if the task fail, the ecs task gets killed.

[vandonr-amz]

ah ok, I missed the It seems this doesn't kill the task either. line. Several issues in one here !
It is a known issue yeah that the on_kill of operators doesn't get called in deferrable mode :/
Maybe it'd be worth opening a new more general issue about this. We can try to fix it in every operator by piecing code together to replicate the expected behavior, but it's a suboptimal solution.

[vandonr-amz]

using the search, it seems like it was reported a looong time ago in #19929 (though at the same time as an other issue too)

[thesuperzapper]

Here is the generic issue for the fact that on_kill is not supported when using deferable operators:

• Deferrable operator tasks do not call on_kill method when fail or restarted #36090

There are also some workarounds described in that issue for provider/operator developers before its fully fixed.