[GitHub] [airflow] yuqian90 commented on a change in pull request #16681: Introduce CLEARED_WHEN_RUNNING state

[GitBox <gi...@apache.org>]

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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org