mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-12 08:42:18 +00:00
* fix(caching): key cached async clients by a per-loop token instead of id(loop) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(caching): drop deferred closes queued on a collected event loop Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(caching): pin that a close queued on a live loop waits for that loop * test(caching): run loop-reuse tests until a loop address repeats The collected-loop tests ran a fixed 50 loops and passed with the fix reverted whenever no loop reused an earlier address. They now stop at the first reuse and skip if none happens, through a shared helper. * test(caching): cover the cache key fallback for loops without weak references Also corrects the evicted client closer docstring: a dropped bucket's clients are left to the garbage collector. * fix(http): free short-lived event loops whose handler finalizer close finished as the loop stopped * fix(caching): share sync clients across event loops and catch a failed finalizer close * fix(agentops): stop closing the shared sync http client after the auth call --------- Co-authored-by: mateo <mateo@berri.ai> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com>
543 lines
16 KiB
Python
543 lines
16 KiB
Python
"""
|
|
Tests for EvictedClientCloser.
|
|
|
|
An evicted client must stay open long enough for a request that already holds it
|
|
to finish, and must then actually be closed, otherwise its connection pool is
|
|
retained until a generational collection runs. A client the caller supplied is
|
|
never closed, because litellm does not own its lifecycle.
|
|
"""
|
|
|
|
import asyncio
|
|
import gc
|
|
import weakref
|
|
from typing import Final
|
|
from unittest.mock import AsyncMock
|
|
|
|
import httpx
|
|
import pytest
|
|
from redis.asyncio import ConnectionPool, Redis
|
|
|
|
from litellm.caching.evicted_client_closer import EvictedClientCloser
|
|
from litellm.llms.custom_httpx.http_handler import AsyncHTTPHandler
|
|
from tests.unit.caching._event_loop_reuse import runs_until_a_loop_address_repeats
|
|
|
|
|
|
class FakeClock:
|
|
"""Hand-advanced monotonic clock, so grace windows need no real waiting."""
|
|
|
|
def __init__(self) -> None:
|
|
self.now = 1000.0
|
|
|
|
def __call__(self) -> float:
|
|
return self.now
|
|
|
|
def advance(self, seconds: float) -> None:
|
|
self.now += seconds
|
|
|
|
|
|
class AsyncClient:
|
|
def __init__(self) -> None:
|
|
self.closed = False
|
|
|
|
async def close(self) -> None:
|
|
self.closed = True
|
|
|
|
|
|
class RecordingAsyncClient:
|
|
def __init__(self) -> None:
|
|
self.close_calls = 0
|
|
|
|
async def aclose(self) -> None:
|
|
self.close_calls += 1
|
|
|
|
|
|
class SyncClient:
|
|
def __init__(self) -> None:
|
|
self.closed = False
|
|
|
|
def close(self) -> None:
|
|
self.closed = True
|
|
|
|
|
|
class CountingDeadline(float):
|
|
"""A clock reading that tallies every deadline comparison made against it.
|
|
|
|
Deadline comparisons are the work a reap does, so counting them says whether
|
|
that work tracks the entries that are due or the size of the whole queue.
|
|
"""
|
|
|
|
comparisons = 0
|
|
|
|
def __add__(self, other: float) -> "CountingDeadline":
|
|
return CountingDeadline(float(self) + other)
|
|
|
|
def __le__(self, other: float) -> bool:
|
|
CountingDeadline.comparisons += 1
|
|
return float(self) <= float(other)
|
|
|
|
def __gt__(self, other: float) -> bool:
|
|
CountingDeadline.comparisons += 1
|
|
return float(self) > float(other)
|
|
|
|
|
|
def make_closer(clock: FakeClock, grace_seconds: float = 60.0) -> EvictedClientCloser:
|
|
return EvictedClientCloser(grace_seconds=grace_seconds, clock=clock)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_redis_client_is_closed_only_after_its_subscription_releases_the_connection():
|
|
closer = EvictedClientCloser(grace_seconds=0)
|
|
pool = ConnectionPool()
|
|
client = Redis.from_pool(pool)
|
|
closed = asyncio.Event()
|
|
connection = AsyncMock()
|
|
connection.disconnect.side_effect = closed.set
|
|
pool._available_connections.append(connection)
|
|
borrowed = pool.get_available_connection()
|
|
closer.mark_owned(client)
|
|
closer.schedule(client)
|
|
|
|
closer.reap()
|
|
await asyncio.sleep(0.05)
|
|
|
|
connection.disconnect.assert_not_awaited()
|
|
assert closer.pending_count == 1
|
|
|
|
await pool.release(borrowed)
|
|
closer.reap()
|
|
await asyncio.wait_for(closed.wait(), timeout=1)
|
|
|
|
connection.disconnect.assert_awaited_once()
|
|
assert closer.pending_count == 0
|
|
|
|
|
|
async def _trickling_upstream(reader: asyncio.StreamReader, writer: asyncio.StreamWriter) -> None:
|
|
"""Serves a chunked body slowly, so a request stays on the wire long enough to observe."""
|
|
await reader.read(4096)
|
|
writer.write(b"HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n\r\n")
|
|
await writer.drain()
|
|
for _ in range(6):
|
|
writer.write(b"5\r\nhello\r\n")
|
|
await writer.drain()
|
|
await asyncio.sleep(0.1)
|
|
writer.write(b"0\r\n\r\n")
|
|
await writer.drain()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_owned_client_is_closed_once_the_grace_window_elapses():
|
|
clock = FakeClock()
|
|
closer = make_closer(clock)
|
|
client = AsyncClient()
|
|
|
|
closer.mark_owned(client)
|
|
closer.schedule(client)
|
|
clock.advance(61.0)
|
|
closer.reap()
|
|
await asyncio.sleep(0.05)
|
|
|
|
assert client.closed is True
|
|
assert closer.pending_count == 0
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_owned_client_stays_open_inside_the_grace_window():
|
|
"""A request handed the client just before eviction is still using it."""
|
|
clock = FakeClock()
|
|
closer = make_closer(clock)
|
|
client = AsyncClient()
|
|
|
|
closer.mark_owned(client)
|
|
closer.schedule(client)
|
|
clock.advance(59.0)
|
|
closer.reap()
|
|
await asyncio.sleep(0.05)
|
|
|
|
assert client.closed is False
|
|
assert closer.pending_count == 1
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_caller_supplied_client_is_never_closed():
|
|
clock = FakeClock()
|
|
closer = make_closer(clock)
|
|
client = AsyncClient()
|
|
|
|
closer.schedule(client)
|
|
clock.advance(3600.0)
|
|
closer.reap()
|
|
await asyncio.sleep(0.05)
|
|
|
|
assert client.closed is False
|
|
assert closer.pending_count == 0
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_sync_client_is_closed_once_the_grace_window_elapses():
|
|
clock = FakeClock()
|
|
closer = make_closer(clock)
|
|
client = SyncClient()
|
|
|
|
closer.mark_owned(client)
|
|
closer.schedule(client)
|
|
clock.advance(61.0)
|
|
closer.reap()
|
|
|
|
assert client.closed is True
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_failing_close_does_not_propagate_or_block_the_others():
|
|
class ExplodingClient:
|
|
async def close(self) -> None:
|
|
raise RuntimeError("connection already gone")
|
|
|
|
clock = FakeClock()
|
|
closer = make_closer(clock)
|
|
exploding, healthy = ExplodingClient(), AsyncClient()
|
|
|
|
for client in (exploding, healthy):
|
|
closer.mark_owned(client)
|
|
closer.schedule(client)
|
|
clock.advance(61.0)
|
|
closer.reap()
|
|
await asyncio.sleep(0.05)
|
|
|
|
assert healthy.closed is True
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_an_unhashable_cached_value_does_not_break_eviction():
|
|
"""The cache holds arbitrary values; an ownership test must never raise on one."""
|
|
|
|
class Unhashable:
|
|
__hash__ = None # pyright: ignore[reportAssignmentType] # unhashable by construction
|
|
|
|
clock = FakeClock()
|
|
closer = make_closer(clock)
|
|
|
|
closer.mark_owned(Unhashable())
|
|
closer.schedule(Unhashable())
|
|
|
|
assert closer.pending_count == 0
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_values_with_nothing_to_close_are_never_queued():
|
|
"""The cache holds plain values too; those have nothing to reclaim."""
|
|
|
|
class NotAClient:
|
|
pass
|
|
|
|
clock = FakeClock()
|
|
closer = make_closer(clock)
|
|
value = NotAClient()
|
|
|
|
closer.mark_owned(value)
|
|
closer.schedule(value)
|
|
|
|
assert closer.pending_count == 0
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_queued_client_is_not_kept_alive_by_the_queue():
|
|
"""Waiting out a grace window must not retain what the collector would free first."""
|
|
clock = FakeClock()
|
|
closer = make_closer(clock)
|
|
client = AsyncClient()
|
|
gone = weakref.ref(client)
|
|
|
|
closer.mark_owned(client)
|
|
closer.schedule(client)
|
|
del client
|
|
gc.collect()
|
|
|
|
assert gone() is None, "the pending queue is holding the client alive"
|
|
|
|
clock.advance(61.0)
|
|
closer.reap()
|
|
assert closer.pending_count == 0
|
|
|
|
|
|
def test_sync_client_evicted_outside_an_event_loop_is_still_closed():
|
|
"""The sync httpx handler is cached and evicted from call sites with no loop."""
|
|
clock = FakeClock()
|
|
closer = make_closer(clock)
|
|
client = SyncClient()
|
|
|
|
closer.mark_owned(client)
|
|
closer.schedule(client)
|
|
assert closer.pending_count == 1
|
|
|
|
clock.advance(61.0)
|
|
closer.reap()
|
|
|
|
assert client.closed is True
|
|
assert closer.pending_count == 0
|
|
|
|
|
|
def test_async_client_from_collected_loop_is_not_closed_by_fresh_loops() -> None:
|
|
clock: Final = FakeClock()
|
|
closer: Final = EvictedClientCloser(grace_seconds=0, clock=clock)
|
|
|
|
async def reap_then_schedule(client: RecordingAsyncClient) -> None:
|
|
closer.reap()
|
|
await asyncio.sleep(0)
|
|
closer.schedule(client)
|
|
|
|
def run_on_new_loop() -> tuple[int, RecordingAsyncClient]:
|
|
client: Final = RecordingAsyncClient()
|
|
closer.mark_owned(client)
|
|
loop: Final = asyncio.new_event_loop()
|
|
loop_id: Final = id(loop)
|
|
try:
|
|
loop.run_until_complete(reap_then_schedule(client))
|
|
finally:
|
|
loop.close()
|
|
del loop
|
|
gc.collect()
|
|
return loop_id, client
|
|
|
|
runs: Final = runs_until_a_loop_address_repeats(run_on_new_loop)
|
|
|
|
assert all(client.close_calls == 0 for _, client in runs)
|
|
|
|
|
|
def test_async_client_queued_on_a_live_loop_is_closed_only_by_that_loop() -> None:
|
|
clock: Final = FakeClock()
|
|
closer: Final = EvictedClientCloser(grace_seconds=0, clock=clock)
|
|
client: Final = RecordingAsyncClient()
|
|
closer.mark_owned(client)
|
|
owner_loop: Final = asyncio.new_event_loop()
|
|
other_loop: Final = asyncio.new_event_loop()
|
|
|
|
async def schedule() -> None:
|
|
closer.schedule(client)
|
|
|
|
async def reap() -> None:
|
|
closer.reap()
|
|
await asyncio.sleep(0)
|
|
|
|
try:
|
|
owner_loop.run_until_complete(schedule())
|
|
other_loop.run_until_complete(reap())
|
|
closes_after_other_loop_reaped: Final = client.close_calls
|
|
pending_after_other_loop_reaped: Final = closer.pending_count
|
|
owner_loop.run_until_complete(reap())
|
|
finally:
|
|
other_loop.close()
|
|
owner_loop.close()
|
|
|
|
assert closes_after_other_loop_reaped == 0
|
|
assert pending_after_other_loop_reaped == 1
|
|
assert client.close_calls == 1
|
|
assert closer.pending_count == 0
|
|
|
|
|
|
def test_closes_queued_on_a_collected_loop_free_capacity() -> None:
|
|
clock: Final = FakeClock()
|
|
closer: Final = EvictedClientCloser(grace_seconds=0, max_pending=1, clock=clock)
|
|
first: Final = RecordingAsyncClient()
|
|
closer.mark_owned(first)
|
|
loop_a: Final = asyncio.new_event_loop()
|
|
|
|
async def schedule_first() -> None:
|
|
closer.schedule(first)
|
|
|
|
try:
|
|
loop_a.run_until_complete(schedule_first())
|
|
finally:
|
|
loop_a.close()
|
|
del loop_a
|
|
gc.collect()
|
|
|
|
second: Final = RecordingAsyncClient()
|
|
closer.mark_owned(second)
|
|
loop_b: Final = asyncio.new_event_loop()
|
|
|
|
async def schedule_and_reap_second() -> None:
|
|
closer.schedule(second)
|
|
closer.reap()
|
|
await asyncio.sleep(0)
|
|
|
|
try:
|
|
loop_b.run_until_complete(schedule_and_reap_second())
|
|
finally:
|
|
loop_b.close()
|
|
|
|
assert first.close_calls == 0
|
|
assert second.close_calls == 1
|
|
assert closer.pending_count == 0
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_an_async_client_waits_for_a_loop_rather_than_being_dropped():
|
|
clock = FakeClock()
|
|
closer = make_closer(clock)
|
|
client = AsyncClient()
|
|
closer.mark_owned(client)
|
|
|
|
def schedule_outside_a_loop() -> None:
|
|
closer.schedule(client)
|
|
clock.advance(61.0)
|
|
closer.reap()
|
|
|
|
await asyncio.to_thread(schedule_outside_a_loop)
|
|
assert client.closed is False, "no loop was running, so it could not have been closed"
|
|
assert closer.pending_count == 1
|
|
|
|
closer.reap()
|
|
await asyncio.sleep(0.05)
|
|
|
|
assert client.closed is True
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_client_evicted_on_a_collected_loop_is_dropped():
|
|
clock = FakeClock()
|
|
closer = make_closer(clock)
|
|
client = AsyncClient()
|
|
closer.mark_owned(client)
|
|
|
|
def schedule_on_its_own_loop() -> None:
|
|
asyncio.run(_schedule())
|
|
|
|
async def _schedule() -> None:
|
|
closer.schedule(client)
|
|
|
|
await asyncio.to_thread(schedule_on_its_own_loop)
|
|
assert closer.pending_count == 1
|
|
|
|
clock.advance(61.0)
|
|
closer.reap()
|
|
await asyncio.sleep(0.05)
|
|
|
|
assert client.closed is False
|
|
assert closer.pending_count == 0
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_client_serving_a_request_is_not_closed_when_its_grace_window_ends():
|
|
"""The grace window on its own cannot promise that a request has finished.
|
|
|
|
``litellm.request_timeout`` defaults to 6000 seconds and a streaming response
|
|
is bounded only by how long the upstream keeps sending, so a client past its
|
|
deadline is closed only once its own pool reports nothing in flight.
|
|
"""
|
|
server = await asyncio.start_server(_trickling_upstream, "127.0.0.1", 0)
|
|
port = server.sockets[0].getsockname()[1]
|
|
clock = FakeClock()
|
|
closer = make_closer(clock)
|
|
client = httpx.AsyncClient()
|
|
|
|
closer.mark_owned(client)
|
|
closer.schedule(client)
|
|
|
|
async def read_the_stream() -> int:
|
|
received = 0
|
|
async with client.stream("GET", f"http://127.0.0.1:{port}/") as response:
|
|
async for chunk in response.aiter_bytes():
|
|
received += len(chunk)
|
|
return received
|
|
|
|
streaming = asyncio.create_task(read_the_stream())
|
|
await asyncio.sleep(0.25) # the request is on the wire
|
|
clock.advance(3600.0) # and its grace window is long gone
|
|
closer.reap()
|
|
await asyncio.sleep(0.05)
|
|
|
|
assert client.is_closed is False, "closed a client that was serving a request"
|
|
assert await streaming > 0, "the in-flight request did not survive the reap"
|
|
|
|
clock.advance(3600.0)
|
|
closer.reap()
|
|
await asyncio.sleep(0.05)
|
|
|
|
assert client.is_closed is True, "an idle client past its grace window must be closed"
|
|
assert closer.pending_count == 0
|
|
server.close()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_the_aiohttp_backed_handler_is_not_closed_mid_request():
|
|
"""The default async path is aiohttp-backed, whose pool accounts for its own leases."""
|
|
server = await asyncio.start_server(_trickling_upstream, "127.0.0.1", 0)
|
|
port = server.sockets[0].getsockname()[1]
|
|
clock = FakeClock()
|
|
closer = make_closer(clock)
|
|
handler = AsyncHTTPHandler()
|
|
held_client = handler.client
|
|
|
|
closer.mark_owned(handler)
|
|
closer.schedule(handler)
|
|
|
|
request = asyncio.create_task(handler.get(f"http://127.0.0.1:{port}/"))
|
|
await asyncio.sleep(0.25)
|
|
clock.advance(3600.0)
|
|
closer.reap()
|
|
await asyncio.sleep(0.05)
|
|
|
|
assert held_client.is_closed is False, "closed a handler that was serving a request"
|
|
assert (await request).status_code == 200
|
|
|
|
clock.advance(3600.0)
|
|
closer.reap()
|
|
await asyncio.sleep(0.05)
|
|
|
|
assert held_client.is_closed is True
|
|
assert handler.client.is_closed is False, "a held handler must self-heal after its evicted client is closed"
|
|
server.close()
|
|
|
|
|
|
def test_the_pending_queue_cannot_grow_past_its_bound():
|
|
"""A caller that churns the client cache must not be able to grow this queue."""
|
|
clock = FakeClock()
|
|
closer = EvictedClientCloser(grace_seconds=60.0, max_pending=8, clock=clock)
|
|
clients = tuple(SyncClient() for _ in range(50))
|
|
|
|
for client in clients:
|
|
closer.mark_owned(client)
|
|
closer.schedule(client)
|
|
|
|
assert closer.pending_count == 8, "the queue grew past max_pending"
|
|
|
|
clock.advance(61.0)
|
|
closer.reap()
|
|
|
|
assert closer.pending_count == 0
|
|
assert sum(client.closed for client in clients) == 8, "everything queued should have been closed"
|
|
|
|
|
|
def test_a_reap_looks_at_what_is_due_rather_than_at_the_whole_queue():
|
|
"""Sustained churn evicts a client per request, and every read of the cache reaps.
|
|
|
|
So the cost of a reap has to track the entries that are due, not the length of
|
|
the queue; a reap that filters the whole queue makes the pair quadratic. Each
|
|
bucket is ordered by deadline, so an up-to-date reap compares one entry per
|
|
bucket and stops. Counting the comparisons measures that directly, where a
|
|
wall-clock budget would only measure the machine.
|
|
"""
|
|
evictions = 1_000
|
|
clock = FakeClock()
|
|
closer = EvictedClientCloser(
|
|
grace_seconds=60.0,
|
|
max_pending=evictions,
|
|
clock=lambda: CountingDeadline(clock.now),
|
|
)
|
|
clients = tuple(SyncClient() for _ in range(evictions))
|
|
for client in clients:
|
|
closer.mark_owned(client)
|
|
|
|
CountingDeadline.comparisons = 0
|
|
for client in clients:
|
|
closer.schedule(client)
|
|
closer.reap() # nothing is due yet, which is the hot path
|
|
clock.advance(61.0)
|
|
closer.reap()
|
|
|
|
assert closer.pending_count == 0
|
|
assert all(client.closed for client in clients)
|
|
assert CountingDeadline.comparisons < 10 * evictions, (
|
|
f"{CountingDeadline.comparisons} deadline comparisons for {evictions} evictions; "
|
|
"a reap is walking the whole queue"
|
|
)
|