yuqian90 commented on a change in pull request #16681:
URL: https://github.com/apache/airflow/pull/16681#discussion_r660500931
##########
File path: airflow/models/taskinstance.py
##########
@@ -1503,6 +1503,10 @@ def handle_failure_with_callback(
def is_eligible_to_retry(self):
"""Is task instance is eligible for retry"""
+ if self.state == State.CLEARED_WHEN_RUNNING:
+ # If a task was cleared when running, it is always eligible for
retry
Review comment:
Hi @ashb , I think we should run it again. For example, if a task is
configured with `max_retries=0`, and it's now running with current `try_number`
0. Then the user clears it (e.g. to pick up a code change), should we kill the
job? Yes. Should it retry? I think the answer is Yes too. Intuitively, the
Clear operation in Airflow is always associated with clearing the current state
of the taskinstance and get it ready to run again once its dependencies are
met. The only exception is if the task is currently running. If a running task
is cleared, it currently fails, not because of the clearing, but because it
gets a SIGTERM after its state is set to SHUTDOWN. So the failure and the
`on_failure_callback` received when a running task is cleared is
counter-intuitive to many users.
If the user insists he wants to see a failure, he can still use Mark Failed
which does exactly that. So in order to make Clear consistently clear the state
of the task, I think it makes more sense to quietly kill the task and let
`is_eligible_to_retry` return True here. Let me know what you think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
