[serve] Production defaults for HAProxy connect timeout and dead-replica detection

python/ray/serve/_private/constants.py

@@ -850,12 +850,16 @@
     else None
 )
 
-RAY_SERVE_HAPROXY_TIMEOUT_CONNECT_S = (
-    # Guarded by the truthiness check below; the two get() calls can't be
-    # narrowed by mypy.
-    int(os.environ.get("RAY_SERVE_HAPROXY_TIMEOUT_CONNECT_S"))  # type: ignore[arg-type]
-    if os.environ.get("RAY_SERVE_HAPROXY_TIMEOUT_CONNECT_S")
-    else None
+# Connection timeout to a replica, in seconds. Replicas are in-cluster, so a
+# connect that takes seconds means the node is gone rather than busy; bounding
+# it lets `retry-on conn-failure` + `option redispatch` reach another replica
+# while the request still has budget.
+#
+# Set to 0 to disable. HAProxy stores an unset timeout as 0, so `timeout
+# connect 0s` is indistinguishable from omitting the directive: an infinite
+# connect timeout, plus HAProxy's "missing timeouts" startup warning.
+RAY_SERVE_HAPROXY_TIMEOUT_CONNECT_S = get_env_int_non_negative(
+    "RAY_SERVE_HAPROXY_TIMEOUT_CONNECT_S", 5
 )
 
 # When enabled, adds 'option http-no-delay' to the HAProxy config defaults,
@@ -908,7 +912,7 @@
 # redispatch + the `backup` fallback take over. Health checks revive a false
 # positive in ~0.5s. Backup/fallback servers are never observed.
 RAY_SERVE_HAPROXY_OBSERVE_MARK_DOWN_ENABLED = get_env_bool(
-    "RAY_SERVE_HAPROXY_OBSERVE_MARK_DOWN_ENABLED", "0"
+    "RAY_SERVE_HAPROXY_OBSERVE_MARK_DOWN_ENABLED", "1"
 )
 
 # Consecutive observed layer4 errors before a server is marked DOWN. Only

python/ray/serve/tests/test_haproxy_api.py

@@ -397,8 +397,8 @@ def test_generate_config_file_internal(haproxy_api_cleanup):
     http-check expect status 200
     default-server fastinter 250ms downinter 250ms fall 2 rise 3 inter 5s check
     # Servers in this backend
-    server api_server1 127.0.0.1:8001 check
-    server api_server2 127.0.0.1:8002 check
+    server api_server1 127.0.0.1:8001 check observe layer4 error-limit 3 on-error mark-down
+    server api_server2 127.0.0.1:8002 check observe layer4 error-limit 3 on-error mark-down
     # Fallback to head node's Serve proxy when no ingress replicas are available
     server api_fallback_server 127.0.0.1:8500 check backup
 backend web_backend
@@ -417,7 +417,7 @@ def test_generate_config_file_internal(haproxy_api_cleanup):
     http-check expect status 200
     default-server fastinter 250ms downinter 250ms fall 3 rise 2 inter 2s check
     # Servers in this backend
-    server web_server1 127.0.0.1:8003 check
+    server web_server1 127.0.0.1:8003 check observe layer4 error-limit 3 on-error mark-down
 listen stats
   bind *:8080
   stats enable
@@ -941,6 +941,66 @@ def render(overrides):
     assert "\n    retries 2\n" in overridden
 
 
+# Overriding any of these is supported, and makes the shipped-default
+# assertions below meaningless rather than false, so the test opts out instead.
+_DEFAULT_TUNING_ENV_VARS = (
+    "RAY_SERVE_HAPROXY_OBSERVE_MARK_DOWN_ENABLED",
+    "RAY_SERVE_HAPROXY_OBSERVE_ERROR_LIMIT",
+    "RAY_SERVE_HAPROXY_TIMEOUT_CONNECT_S",
+    "RAY_SERVE_HAPROXY_TIMEOUT_SERVER_S",
+)
+
+
+@pytest.mark.skipif(
+    any(v in os.environ for v in _DEFAULT_TUNING_ENV_VARS),
+    reason=f"environment overrides one of {_DEFAULT_TUNING_ENV_VARS}",
+)
+def test_default_data_plane_tuning_renders(haproxy_api_cleanup):
+    """The shipped defaults bound connect time, mark dead replicas down from
+    live traffic, and leave `timeout server` unset.
+
+    Every other test in this file passes explicit overrides, so nothing else
+    would catch a silent change to these; each one changes how every Serve
+    cluster routes."""
+    with tempfile.TemporaryDirectory() as temp_dir:
+        config_file_path = os.path.join(temp_dir, "haproxy.cfg")
+        api = HAProxyApi(
+            cfg=HAProxyConfig(socket_path=os.path.join(temp_dir, "admin.sock")),
+            backend_configs={
+                "api": BackendConfig(
+                    name="api",
+                    path_prefix="/api",
+                    app_name="api",
+                    servers=[
+                        ServerConfig(
+                            name="s1", host="127.0.0.1", port=8001, replica_id="rid_1"
+                        )
+                    ],
+                )
+            },
+            config_file_path=config_file_path,
+        )
+        api._generate_config_file_internal()
+        with open(config_file_path) as f:
+            cfg = f.read()
+
+    # Replicas are in-cluster: a connect that takes seconds means the node is
+    # gone, and failing fast lets redispatch reach another replica in time.
+    assert "\n    timeout connect 5s\n" in cfg
+
+    # No `timeout server`. It is a server-side inactivity limit applied to every
+    # connection on the live process, so any value caps request duration -- and
+    # Serve's own request_timeout_s defaults to no timeout. hard-stop-after
+    # bounds draining workers on a separate clock.
+    assert "\n    timeout server " not in cfg
+
+    # Live traffic marks a dead replica DOWN without waiting for a health check.
+    assert (
+        "server s1 127.0.0.1:8001 check observe layer4 "
+        "error-limit 3 on-error mark-down"
+    ) in cfg
+
+
 @pytest.mark.parametrize("forward_body", [True, False])
 def test_ingress_request_router_forward_body_gate_renders(
     haproxy_api_cleanup, monkeypatch, forward_body