From f8bfbad43589c44b455001c99222454a5d08b0be Mon Sep 17 00:00:00 2001 From: Yucheng Zhu Date: Wed, 12 Aug 2026 00:25:06 -0700 Subject: [PATCH] perf(observability): skip the pool read when no collector is configured A proxy without prometheus was still taking the throttled query-engine read every interval and discarding the result. The client resolver now checks for a collector first, so that deployment does no extra work while the interval is still consumed, which keeps the throttle from retrying on every database call. Also condenses the rationale comments this change added down to what the code cannot say for itself, per the repo's comment policy. --- litellm/integrations/prometheus.py | 7 +- litellm/proxy/db/db_pool_metrics.py | 78 +++++-------------- litellm/proxy/db/log_db_metrics.py | 41 ++++------ litellm/proxy/db/prisma_client.py | 11 +-- .../proxy/db/test_log_db_metrics.py | 26 +++++++ 5 files changed, 67 insertions(+), 96 deletions(-) diff --git a/litellm/integrations/prometheus.py b/litellm/integrations/prometheus.py index d5ac694fa24..bd0079cabfb 100644 --- a/litellm/integrations/prometheus.py +++ b/litellm/integrations/prometheus.py @@ -161,11 +161,8 @@ class PrometheusLogger(CustomLogger): def get_instance() -> PrometheusLogger | None: """The registered PrometheusLogger, however it was registered. - Searching ``litellm.callbacks`` alone misses the equally supported - ``litellm_settings.success_callback: ["prometheus"]``, which lands the - logger on the success-callback lists instead. Callers of this method - publish metrics from outside the request hooks, so a miss shows up as a - metric that registers and then never leaves zero. + ``litellm.callbacks`` alone misses ``success_callback: ["prometheus"]``, + which shows up as a metric that registers and never leaves zero. """ import litellm diff --git a/litellm/proxy/db/db_pool_metrics.py b/litellm/proxy/db/db_pool_metrics.py index 570b875ba89..0b9fd232337 100644 --- a/litellm/proxy/db/db_pool_metrics.py +++ b/litellm/proxy/db/db_pool_metrics.py @@ -1,15 +1,7 @@ """Bridges the Prisma query engine's connection-pool counters into Prometheus. -The query engine tracks pool occupancy and, separately, how long a query spent -waiting for a pool slot versus executing against the database. That split is the -only way to tell "the database is slow" apart from "we ran out of connections", -so it is sampled here and re-published under ``litellm_db_pool_*`` names. - -Sampling is driven from the DB call path rather than from a scheduled job: an -exporter that runs on a timer stops reporting exactly when the event loop is -saturated, which is the window an operator most needs. Requests are what stall -during pool exhaustion, and they keep arriving, so a throttled sample taken -alongside real DB work stays alive through the incident. +Sampled from the DB call path, not a timer: an exporter on a timer stops +reporting exactly when the event loop is saturated. """ from __future__ import annotations @@ -40,26 +32,17 @@ _QUERY_DURATION_HISTOGRAM_KEY: Final = "prisma_datasource_queries_duration_histo class SupportsPoolSample(Protocol): - """The one method this module needs from a Prisma client wrapper. - - Deliberately not ``runtime_checkable``: ``PrismaWrapper`` resolves most of - its surface through an instance-level ``__getattr__``, which an - ``isinstance`` check against a protocol does not see, so a runtime check - here would reject the very object it exists to accept. - """ + """Not ``runtime_checkable``: ``PrismaWrapper`` resolves through an + instance-level ``__getattr__``, which ``isinstance`` does not see.""" async def get_pool_sample(self) -> DBPoolSample: ... @dataclass(frozen=True, slots=True) class DBPoolSample: - """One reading of the engine's pool counters. - - ``max_connections`` is derived as ``busy + idle`` rather than parsed out of - ``DATABASE_URL``. The engine reports ``idle`` as remaining capacity, so the - sum is the configured ``connection_limit``, and deriving it here keeps the - database credentials out of this path entirely. - """ + """One reading of the engine's pool counters. ``max_connections`` is + ``busy + idle`` because the engine reports idle as remaining capacity, which + also keeps ``DATABASE_URL`` out of this path.""" busy_connections: float idle_connections: float @@ -77,13 +60,8 @@ class DBPoolSample: @dataclass(frozen=True, slots=True) class DBPoolMetricsUpdate: - """A sample plus the cumulative movement since the previous sample. - - The engine reports totals; Prometheus counters take increments. The deltas - are the difference between consecutive samples, and are omitted entirely - when the engine's totals move backwards, which happens when the query engine - restarts and its counters reset. - """ + """A sample plus the movement since the previous one. Deltas are zeroed when + the engine's totals move backwards, which means it restarted.""" sample: DBPoolSample pending_acquirers: float @@ -123,12 +101,8 @@ def parse_pool_sample(metrics: Metrics) -> DBPoolSample: class DBPoolMetricsSampler: - """Throttled reader of the engine's pool counters. - - One instance per process. ``maybe_sample`` is safe to call on every DB - operation: it returns ``None`` without touching the engine until - ``min_interval_seconds`` has elapsed since the last reading. - """ + """Throttled reader of the engine's pool counters. Safe to call on every DB + operation: it touches nothing until ``min_interval_seconds`` has elapsed.""" def __init__( self, @@ -149,19 +123,11 @@ class DBPoolMetricsSampler: return (self._monotonic() - self._last_sampled_at) >= self._min_interval_seconds async def maybe_sample(self, resolve_client: Callable[[], SupportsPoolSample | None]) -> DBPoolMetricsUpdate | None: - """Read the engine's counters, or return ``None`` if not yet due. + """Read the engine's counters, or ``None`` if not yet due. - The interval is consumed as soon as an attempt starts, before the client - is resolved, so a deployment that has no client to sample throttles the - same as one that does rather than retrying on every database call. - - Never raises, and never blocks for long: a wedged query engine costs a - bounded wait. A pool sample is diagnostic, and an engine that cannot - answer one is already the subject of a louder alarm. - - The client is resolved through a callable rather than passed in so that - resolution, which reads proxy module state, happens under the same - throttle and the same exception guard as the read itself. + The interval is consumed before the client is resolved, so a deployment + with nothing to sample throttles the same as one that does. Never raises + and never blocks for long; a pool sample is diagnostic. """ if not self.is_due(): return None @@ -185,16 +151,10 @@ class DBPoolMetricsSampler: def _corrected_pending_acquirers(self, sample: DBPoolSample) -> float: """The engine's waiter gauge, corrected for waiters that timed out. - ``prisma_client_queries_wait`` is decremented when a waiter acquires a - connection but not when it gives up, so every pool timeout latches the - gauge one higher for the life of the process. Observed directly against - a live engine at ``connection_limit=2``: after two bursts that produced - seven P2024s, a fully drained pool still reported seven waiters. - - Free capacity is proof that nobody is queued, so any reading taken while - a connection is idle is exactly the accumulated latch, and subtracting it - recovers the real depth. The baseline re-arms on every idle sample, so - repeated exhaustion stays accurate rather than drifting further each time. + ``prisma_client_queries_wait`` decrements on acquisition but not on + timeout, so each P2024 latches it one higher for the life of the process. + Free capacity proves nobody is queued, so an idle reading is exactly the + accumulated latch; subtracting it recovers the real depth. """ if sample.idle_connections > 0: self._pending_baseline = sample.pending_acquirers diff --git a/litellm/proxy/db/log_db_metrics.py b/litellm/proxy/db/log_db_metrics.py index 1536af4cb0d..3074f435d43 100644 --- a/litellm/proxy/db/log_db_metrics.py +++ b/litellm/proxy/db/log_db_metrics.py @@ -30,33 +30,31 @@ _pool_metrics_sampler: Final = DBPoolMetricsSampler() def _prometheus_logger() -> "PrometheusLogger | None": - """The active PrometheusLogger, or None when prometheus is not configured. - - Imports lazily. Bare ``import litellm`` does not load the prometheus - integration, so hoisting this would make every proxy pay for a module that - only prometheus deployments use. - """ + """The active PrometheusLogger, or None. Imported lazily so a proxy without + prometheus never loads the integration.""" from litellm.integrations.prometheus import PrometheusLogger return PrometheusLogger.get_instance() def _resolve_pool_client() -> "SupportsPoolSample | None": + """The client to sample, or None when there is nothing to sample for. + + Checks for a metrics consumer before returning a client, so a proxy without + prometheus never pays for the engine read. + """ from litellm.proxy.proxy_server import prisma_client - return None if prisma_client is None else prisma_client.db + if prisma_client is None or _prometheus_logger() is None: + return None + return prisma_client.db async def _sample_db_pool_metrics() -> None: - """Publish a throttled reading of the connection pool, if one is due. + """Publish a throttled pool reading, if one is due. - Runs alongside real database work rather than on a timer: a scheduled - exporter goes quiet exactly when the event loop is saturated, which is the - window this metric exists to cover. - - Every step is inside the guard, including resolving the client and locating - the logger. This runs off a database call that has already succeeded, so - nothing here may turn a working query into a failed one. + Every step is inside the guard: this runs off a database call that already + succeeded, so nothing here may turn a working query into a failed one. """ try: update: Final = await _pool_metrics_sampler.maybe_sample(_resolve_pool_client) @@ -70,16 +68,11 @@ async def _sample_db_pool_metrics() -> None: def _record_db_pool_timeout_if_exhausted(e: Exception) -> None: - """Count a pool exhaustion once, without ever displacing the error itself. + """Count a pool exhaustion once, without displacing the error itself. - Decorated database helpers nest: ``get_object_permission`` is called from - inside ``get_key_object``, and both carry this decorator. One P2024 therefore - passes through several ``except`` blocks on its way up, so the exception is - marked the first time it is counted and skipped afterwards. - - The caller re-raises after this returns. Anything escaping here would replace - a P2024 with a metrics error, during the incident this counter exists to - record. + Decorated helpers nest (``get_object_permission`` inside ``get_key_object``), + so one P2024 passes through several ``except`` blocks; the exception is + marked the first time it is counted. """ try: if not PrismaDBExceptionHandler.is_connection_pool_timeout_error(e): diff --git a/litellm/proxy/db/prisma_client.py b/litellm/proxy/db/prisma_client.py index 00f92ddd94d..8d5876c3046 100644 --- a/litellm/proxy/db/prisma_client.py +++ b/litellm/proxy/db/prisma_client.py @@ -800,14 +800,9 @@ class PrismaWrapper: return seconds_left > self.TOKEN_REFRESH_BUFFER_SECONDS async def get_pool_sample(self) -> DBPoolSample: - """This connection pool's current occupancy, as the query engine sees it. - - Declared rather than left to ``__getattr__``, which resolves through an - untyped delegation and so cannot satisfy a typed caller, and which also - makes the underlying client's identity easy to lose track of. Keeping - the engine's raw counter names behind this boundary means the metrics - layer never has to know them. - """ + """This pool's occupancy, as the query engine sees it. Declared rather + than left to ``__getattr__`` so a typed caller can reach it, and so the + engine's counter names stay out of the metrics layer.""" return parse_pool_sample(await self._original_prisma.get_metrics()) def __getattr__(self, name: str): diff --git a/tests/test_litellm/proxy/db/test_log_db_metrics.py b/tests/test_litellm/proxy/db/test_log_db_metrics.py index 84cdcf1ced6..2fcf6815e9c 100644 --- a/tests/test_litellm/proxy/db/test_log_db_metrics.py +++ b/tests/test_litellm/proxy/db/test_log_db_metrics.py @@ -160,3 +160,29 @@ async def test_two_separate_pool_timeouts_are_counted_separately(): await failing(table_name="x") assert logger.record_db_pool_timeout.call_count == 2 + + +@pytest.mark.asyncio +async def test_a_proxy_without_prometheus_never_reads_the_query_engine(): + """The sample is worthless with no collector configured, so the engine read + must be skipped rather than taken and discarded every interval.""" + from litellm.proxy.db.log_db_metrics import _pool_metrics_sampler, _sample_db_pool_metrics + + reads = [] + prisma_client = MagicMock() + + async def _sample(): + reads.append(1) + raise AssertionError("the engine must not be read when prometheus is off") + + prisma_client.db.get_pool_sample = _sample + _pool_metrics_sampler._last_sampled_at = None + + with ( + patch("litellm.proxy.db.log_db_metrics._prometheus_logger", return_value=None), + patch("litellm.proxy.proxy_server.prisma_client", prisma_client), + ): + await _sample_db_pool_metrics() + + assert reads == [] + assert _pool_metrics_sampler.is_due() is False, "the interval must still be consumed"