xBis7 commented on code in PR #69336:
URL: https://github.com/apache/airflow/pull/69336#discussion_r3989549728
##########
task-sdk/src/airflow/sdk/execution_time/supervisor.py:
##########
@@ -2631,6 +2631,15 @@ def supervise_task(
final_state=result.final_state,
)
return result.exit_code
+ except TaskAlreadyRunningError:
+ # Another worker is already running this task, so the server told
us to back off. This is
+ # normal -- it just means we were a duplicate that lost the race.
Our task never started any
+ # real work, so exit quietly instead of reporting a failure that
would look like a crash.
+ log.info(
+ "Task instance already running on another worker; standing
down without failing it",
+ workload_id=str(ti.id),
+ )
+ return 0
Review Comment:
I've been running tests and I think that the changes in the supervisor
should be reverted entirely.
Without the heartbeat check in the scheduler, this doesn't add any value and
makes things worse for celery because it overrides the existing behavior.
For a local executor, this returns 0 and then the worker sends a success
event back to the scheduler. The scheduler compares the reported event from the
executor (state SUCCESS) with the value in the DB (state RUNNING) and because
there is a mismatch, it updates the DB so that the running task is marked as
FAILED. The worker picks up the update during the next heartbeat and kills the
task.
For local and the other executors, this isn't any different than the
existing behavior where the executor sends a state FAILED event back to the
scheduler. In both cases (success/failure), there is a mismatch between the
executor reported state and the state in the DB.
For celery, the existing executor event is ignored and therefore the
scheduler doesn't have something to react on. But with the change, it gets back
a SUCCESS event and goes back to killing the running task. With the heartbeat
check, there isn't an issue but without it, this is a regression.
--
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]
