potiuk commented on code in PR #40468:
URL: https://github.com/apache/airflow/pull/40468#discussion_r1663802176
##########
airflow/executors/base_executor.py:
##########
@@ -284,8 +289,11 @@ def trigger_tasks(self, open_slots: int) -> None:
self.log.info("queued but still running; attempt=%s
task=%s", attempt.total_tries, key)
continue
# Otherwise, we give up and remove the task from the queue.
- self.log.error(
- "could not queue task %s (still running after %d
attempts)", key, attempt.total_tries
+ self.task_context_logger.error(
+ "could not queue task %s (still running after %d
attempts)",
Review Comment:
I think the fact that we are trying few times in order to avoid race
condition is something that should be in the info:
```
self.log.info("The task is queued but still running. Trying for %s time to
avoid deferred task race condition. Task=%s ", attempt.total_tries, key)
```
This is pure information, that might help with diagnostics. No action from
user is expected.
But then when we reach the time to avoid the race conditions and we are
reasonably sure this is externally killed task we should do this (when you look
at `can_try_again()` it allows you to run multiple (any number of) attempts
unless 10 seconds pass:
```
self.task_context_logger.error(
"Could not queue task %s as it is seen as still running
after %d attempts (tried for 10 seconds). It looks like it was killed
externally. Look for extermal reasons why it has been killed (likely a bug or
deployment issue).",
```
Likely the 10 seconds should be somewhat extracted as a constant to pass to
can_try_again and used in the message rather than hardcoding the 10 seconds.
@malthe @dstandish - I think you were implementing the changes to handle
those cases - is my understanding and proposal reasonable (if you still
remember anything about it, that is :) )
--
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]
