test(e2e): assert a cooldown reaches a sibling replica within the 1s Redis read interval (#42422)

* test(e2e): assert a cooldown reaches a sibling replica within the 1s Redis read interval

* test(e2e): skip the sibling replica cooldown cell when one gateway URL is named

* test(e2e): collapse repeated gateway addresses so the sibling cooldown cell skips instead of erroring

---------

Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com>

tests/e2e/coverage_registry/reliability.yaml

@@ -9,6 +9,7 @@
 - {id: reliability.retry.auth.succeeds_within_retries, module: reliability, tier: P1, behavior: retry, variant: auth, assertions: [succeeds_within_retries], exercised_on: [chat_completions], source: "get_retry_from_policy.py:42", rationale: "Transient auth glitch retry"}
 - {id: reliability.retry.context_window.succeeds_within_retries, module: reliability, tier: P1, behavior: retry, variant: context_window, assertions: [succeeds_within_retries], exercised_on: [chat_completions], source: "get_retry_from_policy.py:51", fail_before_fix: proven, rationale: "A context-window 400 under BadRequestErrorRetries retries onto a sibling deployment in the same model group, instead of coming straight back as the 400 the deployment that just refused it returned"}
 - {id: reliability.cooldown.5xx.trips_then_recovers, module: reliability, tier: P0, behavior: cooldown, variant: "5xx", assertions: [trips_then_recovers], exercised_on: [chat_completions], source: "cooldown_handlers.py:40", rationale: "Deployment cools after repeated 5xx, recovers after cooldown_time"}
+- {id: reliability.cooldown.sibling_replica.serves_backup_within_read_interval, module: reliability, tier: P1, behavior: cooldown, variant: sibling_replica, assertions: [serves_backup_within_read_interval], exercised_on: [chat_completions], source: "cooldown_cache.py:44", fail_before_fix: proven, rationale: "A bench taken on one gateway reaches a sibling that already holds the key's read timer within the 1s Redis read interval plus margin, so its next call lands on the backup"}
 - {id: reliability.cooldown.429.trips_then_recovers, module: reliability, tier: P0, behavior: cooldown, variant: "429", assertions: [trips_then_recovers], exercised_on: [chat_completions], source: "cooldown_handlers.py:69", rationale: "Cools on 429, avoids hammering exhausted provider"}
 - {id: reliability.cooldown.auth.trips_then_recovers, module: reliability, tier: P1, behavior: cooldown, variant: auth, assertions: [trips_then_recovers], exercised_on: [chat_completions], source: "cooldown_handlers.py:74", rationale: "Cools on 401 auth error"}
 - {id: reliability.cooldown.timeout.trips_then_recovers, module: reliability, tier: P1, behavior: cooldown, variant: timeout, assertions: [trips_then_recovers], exercised_on: [chat_completions], source: "cooldown_handlers.py:77", rationale: "Cools on 408 timeout"}

tests/e2e/e2e_config.py

@@ -34,7 +34,7 @@ CONTROL_PLANE_BASE_URL = os.environ.get(
 
 
 def parse_replica_urls(raw: str, fallback: str) -> tuple[str, ...]:
-    urls: Final = tuple(url.strip().rstrip("/") for url in raw.split(",") if url.strip())
+    urls: Final = tuple(dict.fromkeys(url.strip().rstrip("/") for url in raw.split(",") if url.strip()))
     return urls or (fallback,)
 
 

tests/e2e/router/reliability_support.py

@@ -22,6 +22,7 @@ from pydantic import ValidationError
 from proxy_client import ProxyClient
 from e2e_config import CHEAP_OPENAI_MODEL, PROXY_BASE_URL, unique_marker
 from e2e_http import NetworkError, StreamHead, StreamingResponse
+from transport import Transport
 from models import (
     CacheControl,
     ChatMessage,
@@ -274,9 +275,36 @@ def chat_turns_override(
 ) -> StreamingResponse:
     """POST /chat/completions with an optional per-request router_settings_override,
     returning the raw outcome so tests read status, body, and reliability headers."""
-    return proxy.transport.send(
+    return chat_turns_override_via(
+        proxy.transport, key, model, turns, override=override, stream=stream, cache=cache, max_tokens=max_tokens
+    )
+
+
+def chat_override_via(
+    transport: Transport,
+    key: str,
+    model: str,
+    content: str,
+    override: RouterSettingsOverride | None = None,
+) -> StreamingResponse:
+    """`chat_override` aimed at one replica's transport (from `proxy.replicas`) instead of
+    the client's default, for cells that must know which gateway took the call."""
+    return chat_turns_override_via(transport, key, model, [ChatMessage(role="user", content=content)], override=override)
+
+
+def chat_turns_override_via(
+    transport: Transport,
+    key: str,
+    model: str,
+    turns: Sequence[ChatMessage],
+    override: RouterSettingsOverride | None = None,
+    stream: bool = False,
+    cache: dict[str, bool] | None = {"no-cache": True},
+    max_tokens: int = 512,
+) -> StreamingResponse:
+    return transport.send(
         "/chat/completions",
-        headers=proxy.transport.bearer(key),
+        headers=transport.bearer(key),
         json=ReliabilityChatBody(
             model=model,
             messages=turns,

tests/e2e/router/test_reliability_cooldowns_e2e.py

@@ -6,21 +6,33 @@ way (a 500, a 429, a 401, or a timeout) holding all of the group's shuffle weigh
 with an `allowed_fails_policy` of zero for that error class and a short
 `cooldown_time`, plus a healthy backup at weight 0. The first call, retries off,
 surfaces the failure to the customer as-is and benches the deployment. The proxy
-records the bench off the request path, and a sibling replica only sees it on
-its next read of the cooldown keys from Redis, which the cooldown cache does at
-most every 1s (DEFAULT_COOLDOWN_REDIS_READ_INTERVAL_SECONDS). So for
+writes the bench to Redis before it answers that failure, and a sibling replica
+only sees it on its next read of the cooldown keys from Redis, which the
+cooldown cache does at most once per COOLDOWN_REDIS_READ_INTERVAL_SECONDS per
+key (DEFAULT_COOLDOWN_REDIS_READ_INTERVAL_SECONDS, 1s). So for
 REPLICA_PROPAGATION_SECONDS after the trip, a window kept far wider than that
-so this cell asserts the trip and the recovery rather than how fast siblings
-catch up, every answer has to be either the deployment's own failure or a 200
-from the backup, which the proxy names in x-litellm-model-id, and at least one
-replica has to have served from the backup by then. From then until shortly
-before the cooldown can lapse, every call has to land on the backup whichever
-replica takes it. Then the test polls until the weighted shuffle opens on the
-failing deployment again and the same failure comes back (or, for the 429 pair,
-its own 200 once the key's rpm window has reset): that is the recovery, since a
-benched deployment is one the router will try again, not one it forgot. Its
-deadline counts from the last failure a stale replica caused, because every
-failure re-arms the cooldown.
+so the trip-then-recover cells assert the trip and the recovery rather than how
+fast siblings catch up, every answer has to be either the deployment's own
+failure or a 200 from the backup, which the proxy names in x-litellm-model-id,
+and at least one replica has to have served from the backup by then. From then
+until shortly before the cooldown can lapse, every call has to land on the
+backup whichever replica takes it. Then the test polls until the weighted
+shuffle opens on the failing deployment again and the same failure comes back
+(or, for the 429 pair, its own 200 once the key's rpm window has reset): that
+is the recovery, since a benched deployment is one the router will try again,
+not one it forgot. Its deadline counts from the last failure a stale replica
+caused, because every failure re-arms the cooldown.
+
+The sibling cell is the one that asserts the speed. It addresses two gateways
+from PROXY_REPLICA_URLS by name, warms the second with a healthy call so its
+router has already read the failing deployment's cooldown key from Redis and
+started the read interval on it, trips the deployment through the first, waits
+the interval plus a margin, and then sends the second replica exactly one call,
+which has to come back from the backup. One call, because a poll that reached
+the failing deployment through the second replica would bench it there too and
+hide whether the first replica's bench ever travelled. A stack addressed only
+through its load balancer cannot pin which replica takes a call, so the cell is
+skipped at collection unless LITELLM_PROXY_REPLICA_URLS names at least two.
 
 The failures are the same real ones the retry tests use: a 1ms deadline and a
 bogus key on the real backend, and this proxy standing in as the upstream for
@@ -37,7 +49,7 @@ from dataclasses import dataclass
 
 import pytest
 from complexity_router_client import ComplexityRouterClient
-from e2e_config import CHEAP_OPENAI_MODEL, unique_marker
+from e2e_config import CHEAP_OPENAI_MODEL, PROXY_REPLICA_URLS, unique_marker
 from e2e_http import StreamingResponse
 from lifecycle import ResourceManager
 from models import KeyGenerateBody, RouterSettingsOverride
@@ -45,6 +57,7 @@ from reliability_support import (
     COOLDOWN_SECONDS,
     REPLICA_PROPAGATION_SECONDS,
     chat_override,
+    chat_override_via,
     create_always_5xx_deployment,
     create_always_rate_limited_deployment,
     create_always_timing_out_deployment,
@@ -54,12 +67,15 @@ from reliability_support import (
     model_id_of,
     spend_only_request_of,
 )
+from transport import Transport
 
 pytestmark = pytest.mark.e2e
 
 RECOVERY_GRACE_SECONDS = 10
 PROPAGATION_POLL_SECONDS = 0.25
 BENCH_MARGIN_SECONDS = 4.0
+COOLDOWN_REDIS_READ_INTERVAL_SECONDS = 1.0
+SIBLING_READ_MARGIN_SECONDS = 1.0
 
 
 def _call_without_retries(client: ComplexityRouterClient, key: str, group: str) -> StreamingResponse:
@@ -68,6 +84,31 @@ def _call_without_retries(client: ComplexityRouterClient, key: str, group: str)
     )
 
 
+def _call_replica_without_retries(transport: Transport, key: str, group: str) -> StreamingResponse:
+    return chat_override_via(
+        transport, key, group, f"say hi {unique_marker()}", override=RouterSettingsOverride(num_retries=0)
+    )
+
+
+@dataclass(frozen=True, slots=True)
+class _Replica:
+    url: str
+    transport: Transport
+
+
+def _two_replicas(client: ComplexityRouterClient) -> tuple[_Replica, _Replica]:
+    first, second, *_ = (_Replica(url, transport) for url, transport in client.proxy.replicas.items())
+    return first, second
+
+
+def _warm_cooldown_reads(replica: _Replica, key: str) -> None:
+    warmed = chat_override_via(replica.transport, key, CHEAP_OPENAI_MODEL, f"say hi {unique_marker()}")
+    assert warmed.status_code == 200, (
+        f"{replica.url} should have answered a healthy {CHEAP_OPENAI_MODEL} call before the trip, got "
+        f"{warmed.status_code}: {warmed.body[:300]}"
+    )
+
+
 def _assert_served_by_backup(resp: StreamingResponse, backup: str, when: str) -> None:
     assert resp.status_code == 200, (
         f"{when} the group should have served from the backup, got {resp.status_code}: {resp.body[:300]}"
@@ -176,6 +217,47 @@ class TestReliabilityCooldowns:
 
         _assert_trips_then_recovers(client, scoped_key, group, failing, backup, failure_status=500)
 
+    @pytest.mark.covers("reliability.cooldown.sibling_replica.serves_backup_within_read_interval")
+    @pytest.mark.skipif(
+        len(PROXY_REPLICA_URLS) < 2,
+        reason=(
+            "this cell trips a deployment through one gateway and reads the bench from another, so "
+            f"LITELLM_PROXY_REPLICA_URLS has to name at least two distinct gateways, got {PROXY_REPLICA_URLS}"
+        ),
+    )
+    def test_sibling_replica_serves_backup_within_redis_read_interval(
+        self, client: ComplexityRouterClient, resources: ResourceManager, scoped_key: str
+    ) -> None:
+        tripping, sibling = _two_replicas(client)
+
+        upstream = f"reliability-cooldown-sibling-upstream-{unique_marker()}"
+        upstream_id = create_bad_base_deployment(client.proxy, upstream)
+        resources.defer(lambda: client.proxy.delete_model(upstream_id))
+
+        group = f"reliability-cooldown-sibling-{unique_marker()}"
+        failing = create_always_5xx_deployment(
+            client.proxy, group, upstream, scoped_key, cooldown_time=COOLDOWN_SECONDS
+        )
+        resources.defer(lambda: client.proxy.delete_model(failing))
+        backup = create_zero_weight_backup_deployment(client.proxy, group)
+        resources.defer(lambda: client.proxy.delete_model(backup))
+
+        _warm_cooldown_reads(sibling, scoped_key)
+
+        tripped = _call_replica_without_retries(tripping.transport, scoped_key, group)
+        assert tripped.status_code == 500, (
+            f"the first call through {tripping.url} should have surfaced the deployment's own 500, got "
+            f"{tripped.status_code}: {tripped.body[:300]}"
+        )
+        tripped_at = time.monotonic()
+
+        time.sleep(COOLDOWN_REDIS_READ_INTERVAL_SECONDS + SIBLING_READ_MARGIN_SECONDS)
+        _assert_served_by_backup(
+            _call_replica_without_retries(sibling.transport, scoped_key, group),
+            backup,
+            f"{time.monotonic() - tripped_at:.1f}s after {tripping.url} benched {failing}, on {sibling.url}",
+        )
+
     @pytest.mark.covers("reliability.cooldown.429.trips_then_recovers")
     def test_429_trips_cooldown_then_recovers(
         self, client: ComplexityRouterClient, resources: ResourceManager, scoped_key: str

tests/e2e/test_proxy_client.py

@@ -338,6 +338,10 @@ class TestParseReplicaUrls:
     def test_falls_back_to_the_data_plane_address_when_unset(self) -> None:
         assert parse_replica_urls("", "http://lb") == ("http://lb",)
 
+    def test_collapses_repeated_gateway_addresses_to_one_replica(self) -> None:
+        raw: Final = "http://127.0.0.1:4010,http://127.0.0.1:4010/,http://127.0.0.1:4011,http://127.0.0.1:4010"
+        assert parse_replica_urls(raw, "http://lb") == ("http://127.0.0.1:4010", "http://127.0.0.1:4011")
+
 
 def _answers(answers: Iterable[str]) -> ReplicaRead[str]:
     it: Final = iter(answers)