Subham-KRLX commented on code in PR #64203:
URL: https://github.com/apache/airflow/pull/64203#discussion_r2988558134
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py:
##########
@@ -730,15 +730,12 @@ def ti_heartbeat(
)
if previous_state != TaskInstanceState.RUNNING:
- log.warning("Task not in running state", current_state=previous_state)
- raise HTTPException(
- status_code=status.HTTP_409_CONFLICT,
- detail={
- "reason": "not_running",
- "message": "TI is no longer in the running state and task
should terminate",
- "current_state": previous_state,
- },
Review Comment:
@kaxil thanks for the quick review and sorry for the wrong approachI
understand now the ti_heartbeat
endpoint should remain strict the actual fix belongs in the supervisor:
instead of immediately force killing the process when it receives a 409
not_running we should first check if the subprocess has already exited (i.e.,
_exit_code is not None) If it has the conflict is a benign race condition (the
task finished and updated its state just before the last heartbeat) and we
should let it clean up normally.
I am reworking the PR with this corrected approach fix only in the
supervisor's _send_heartbeat_if_needed Would it be OK to open a new PR with
this revised fix?
--
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]
