kaxil commented on code in PR #73244:
URL: https://github.com/apache/airflow/pull/73244#discussion_r4031972343


##########
airflow-core/newsfragments/73244.significant.rst:
##########
@@ -0,0 +1,21 @@
+Task heartbeats are no longer retried by ``[workers] execution_api_retries``
+
+Heartbeats had two overlapping retry mechanisms: the Task SDK API client 
retried
+every request, including heartbeats, and the supervisor separately counted
+consecutive heartbeat failures against ``[workers] max_failed_heartbeats``. The
+client retries also had the unwanted side-effect of blocking the supervisor's
+monitor loop while it backed off, so during an API server outage the supervisor

Review Comment:
   Apart from just heartbeats, the `_handle_request` func in supervisor.py runs 
on that same loop and its API calls still retry, so a `GetConnection` during an 
outage can hold the loop for up to ~40s with defaults (5 attempts at a 5s 
timeout, plus up to 15s of backoff).
   
   While it is parked there the loop never reaches `_send_heartbeat_if_needed`, 
so `failed_heartbeats` does not tick and the `max_failed_heartbeats` budget 
does not engage at all.
   
   I am not sure this belongs here since we are talking specifically about 
heartbeats, but I also don't want readers coming away thinking the loop can 
only stall on heartbeats. wdyt?



-- 
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]

Reply via email to