Skip to content

Commit 43995bc

Browse files
devin-ai-integration[bot]yassinkortamyassin-berriai
authored
fix(db): apply the configured connection params to the read replica URL (#37691)
The read replica never received the operator's DB pool settings, so its Prisma pool fell back to `num_physical_cpus * 2 + 1` and the configured cap was not enforced. Both startup paths now pass the same params to the reader: the CLI, and the componentized entrypoints that go through `DatabaseURLSettings.apply_to_env`. Only pool and timeout params are inherited, through a single allowlist both paths share. Anything that decides which tables a query resolves against stays on the writer, including entries smuggled in through `database_extra_connection_params`, so a writer `search_path` cannot repoint reader queries. Params the operator pinned on the replica URL still win. Co-authored-by: Yassin Kortam <yassin.kortam@gmail.com> Co-authored-by: yassin <yassin@berri.ai> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent fb3dd0f commit 43995bc

4 files changed

Lines changed: 369 additions & 44 deletions

File tree

litellm/proxy/db/db_url_settings.py

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,16 @@
2929
token auth is gated on the same global toggle as the writer: the chart only
3030
emits the reader token env vars when the writer also uses token auth.
3131
Reader-side fields fall back to the writer's user / name / schema / port /
32-
password when their ``*_READ_REPLICA`` counterpart is unset.
32+
password when their ``*_READ_REPLICA`` counterpart is unset, and to the
33+
writer's connection params (pool size, timeouts, pgbouncer mode) for the
34+
ones the reader URL does not pin itself.
3335
"""
3436

3537
import os
3638
import urllib.parse
39+
from collections.abc import Mapping
3740
from functools import partial
41+
from types import MappingProxyType
3842
from typing import Annotated, Final, cast
3943

4044
from pydantic import AliasChoices, BeforeValidator, Field
@@ -62,6 +66,51 @@
6266
_MISSING_SCHEME: Final = "<missing scheme>"
6367

6468

69+
# An allowlist, deliberately not a denylist: only these pool and timeout params
70+
# follow the writer to the read replica, so nothing that decides which tables a
71+
# query resolves against (``schema``, or a ``search_path`` inside ``options``)
72+
# can ever repoint the reader. Without them the reader pool silently falls back
73+
# to Prisma's default size.
74+
CONNECTION_PARAM_KEYS: Final[frozenset[str]] = frozenset(
75+
{
76+
"connection_limit",
77+
"pool_timeout",
78+
"connect_timeout",
79+
"socket_timeout",
80+
"pgbouncer",
81+
}
82+
)
83+
84+
85+
def add_missing_query_params(url: str, params: Mapping[str, str | int | float]) -> str:
86+
"""Return ``url`` with the ``params`` it does not already carry appended.
87+
88+
Params the operator pinned on the URL win, so a hand-tuned replica URL keeps
89+
its values. Returns the URL untouched when there is nothing to add, leaving
90+
its existing encoding alone.
91+
"""
92+
parsed: Final = urllib.parse.urlsplit(url)
93+
existing: Final = tuple(urllib.parse.parse_qsl(parsed.query, keep_blank_values=True))
94+
pinned: Final = frozenset(key for key, _ in existing)
95+
additions: Final = tuple((key, str(value)) for key, value in params.items() if key not in pinned)
96+
if not additions:
97+
return url
98+
query: Final = urllib.parse.urlencode(existing + additions)
99+
return urllib.parse.urlunsplit(parsed._replace(query=query))
100+
101+
102+
def reader_shareable_params(params: Mapping[str, str | int | float]) -> Mapping[str, str | int | float]:
103+
"""Return the subset of ``params`` the read replica is allowed to inherit."""
104+
return MappingProxyType({key: value for key, value in params.items() if key in CONNECTION_PARAM_KEYS})
105+
106+
107+
def connection_params_from_url(url: str) -> Mapping[str, str | int | float]:
108+
"""Return the connection params on ``url`` that the read replica shares."""
109+
return reader_shareable_params(
110+
MappingProxyType({key: value for key, value in urllib.parse.parse_qsl(urllib.parse.urlsplit(url).query)})
111+
)
112+
113+
65114
def unsupported_db_scheme(database_url: str) -> str | None:
66115
"""Return the connection URL scheme when it is not PostgreSQL, else None.
67116
@@ -326,8 +375,14 @@ def apply_to_env(self) -> bool:
326375
self._raise_for_unsupported_scheme()
327376
wrote_writer: Final = self.apply_writer_url_to_env()
328377

329-
reader_url: Final = self.build_reader_url()
378+
# The reader inherits the writer's connection params (pool size, timeouts,
379+
# pgbouncer mode). Without this the reader pool ignores the configured cap
380+
# and falls back to Prisma's `num_physical_cpus * 2 + 1` default.
381+
reader_url: Final = self.build_reader_url() or self.database_url_read_replica
330382
if reader_url is not None:
331-
os.environ["DATABASE_URL_READ_REPLICA"] = reader_url
383+
os.environ["DATABASE_URL_READ_REPLICA"] = add_missing_query_params(
384+
reader_url,
385+
connection_params_from_url(os.environ.get("DATABASE_URL", "")),
386+
)
332387

333388
return wrote_writer

litellm/proxy/proxy_cli.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,6 +1224,8 @@ def run_server(
12241224

12251225
if os.getenv("DATABASE_URL", None) is not None or os.getenv("DIRECT_URL", None) is not None:
12261226
from litellm.proxy.db.db_url_settings import (
1227+
add_missing_query_params,
1228+
reader_shareable_params,
12271229
unsupported_db_scheme,
12281230
unsupported_db_scheme_message,
12291231
)
@@ -1273,6 +1275,24 @@ def run_server(
12731275
database_url = os.getenv("DIRECT_URL")
12741276
modified_url = append_query_params(database_url, connection_url_params)
12751277
os.environ["DIRECT_URL"] = modified_url
1278+
# The reader pool is a real pool against the same configured cap, so it
1279+
# gets the allowlisted pool params. Schema-affecting ones, including any
1280+
# the operator smuggled in through database_extra_connection_params, stay
1281+
# on the writer. Anything pinned on the replica URL wins, unlike the
1282+
# writer where the config is applied on top.
1283+
read_replica_url: Final[str | None] = os.getenv("DATABASE_URL_READ_REPLICA")
1284+
if read_replica_url:
1285+
reader_options: Final[str] = _pg_options_with_timeouts(
1286+
_url_query_value(read_replica_url, "options"),
1287+
db_statement_timeout,
1288+
db_lock_timeout,
1289+
)
1290+
os.environ["DATABASE_URL_READ_REPLICA"] = add_missing_query_params(
1291+
_with_query_value(read_replica_url, "options", reader_options)
1292+
if reader_options
1293+
else read_replica_url,
1294+
reader_shareable_params(connection_url_params),
1295+
)
12761296
subprocess.run(["prisma"], capture_output=True)
12771297
is_prisma_runnable = True
12781298
except FileNotFoundError:

tests/test_litellm/proxy/db/test_db_url_settings.py

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
"""
1313

1414
import os
15+
import urllib.parse
1516
from unittest.mock import patch
1617

1718
import pytest
@@ -524,6 +525,137 @@ def test_apply_to_env_accepts_pinned_postgres(monkeypatch):
524525
assert _apply() is False
525526

526527

528+
# ---------------------------------------------------------------------------
529+
# Connection params on the read replica
530+
# ---------------------------------------------------------------------------
531+
532+
533+
def test_reader_inherits_writer_connection_params(monkeypatch):
534+
"""The reader is a second pool: without the writer's params it sizes itself
535+
from Prisma's default and the operator's cap is not enforced."""
536+
monkeypatch.setenv(
537+
"DATABASE_URL",
538+
"postgresql://u:p@writer.example.com:5432/db?connection_limit=3&pool_timeout=20&pgbouncer=true",
539+
)
540+
monkeypatch.setenv(
541+
"DATABASE_URL_READ_REPLICA", "postgresql://u:p@reader.example.com:5432/db"
542+
)
543+
544+
_apply()
545+
546+
query = urllib.parse.parse_qs(
547+
urllib.parse.urlsplit(os.environ["DATABASE_URL_READ_REPLICA"]).query
548+
)
549+
assert query["connection_limit"] == ["3"]
550+
assert query["pool_timeout"] == ["20"]
551+
assert query["pgbouncer"] == ["true"]
552+
553+
554+
def test_reader_keeps_its_own_pinned_connection_params(monkeypatch):
555+
monkeypatch.setenv(
556+
"DATABASE_URL",
557+
"postgresql://u:p@writer.example.com:5432/db?connection_limit=3&pool_timeout=20",
558+
)
559+
monkeypatch.setenv(
560+
"DATABASE_URL_READ_REPLICA",
561+
"postgresql://u:p@reader.example.com:5432/db?connection_limit=50",
562+
)
563+
564+
_apply()
565+
566+
query = urllib.parse.parse_qs(
567+
urllib.parse.urlsplit(os.environ["DATABASE_URL_READ_REPLICA"]).query
568+
)
569+
assert query["connection_limit"] == ["50"]
570+
assert query["pool_timeout"] == ["20"]
571+
572+
573+
def test_assembled_reader_url_inherits_writer_connection_params(monkeypatch):
574+
"""A reader assembled from the discrete DATABASE_*_READ_REPLICA vars must
575+
carry the params too, and must not inherit the writer's schema."""
576+
monkeypatch.setenv(
577+
"DATABASE_URL", "postgresql://u:p@writer.example.com:5432/db?connection_limit=3&schema=writer_schema"
578+
)
579+
monkeypatch.setenv("DATABASE_USER", "litellm")
580+
monkeypatch.setenv("DATABASE_NAME", "litellm_db")
581+
monkeypatch.setenv("DATABASE_PASSWORD", "s3cr3t")
582+
monkeypatch.setenv("DATABASE_HOST_READ_REPLICA", "reader.example.com")
583+
584+
_apply()
585+
586+
reader_url = os.environ["DATABASE_URL_READ_REPLICA"]
587+
assert reader_url.startswith("postgresql://litellm:s3cr3t@reader.example.com:5432/litellm_db?")
588+
query = urllib.parse.parse_qs(urllib.parse.urlsplit(reader_url).query)
589+
assert query["connection_limit"] == ["3"]
590+
assert "schema" not in query
591+
592+
593+
def test_reader_does_not_inherit_writer_options(monkeypatch):
594+
"""A writer search_path must not follow the reader, or reader queries resolve
595+
against the wrong schema."""
596+
monkeypatch.setenv(
597+
"DATABASE_URL",
598+
"postgresql://u:p@writer.example.com:5432/db?connection_limit=3&options=-c%20search_path%3Dwriter_schema",
599+
)
600+
monkeypatch.setenv("DATABASE_URL_READ_REPLICA", "postgresql://u:p@reader.example.com:5432/db")
601+
602+
_apply()
603+
604+
query = urllib.parse.parse_qs(urllib.parse.urlsplit(os.environ["DATABASE_URL_READ_REPLICA"]).query)
605+
assert query["connection_limit"] == ["3"]
606+
assert "options" not in query
607+
608+
609+
def test_reader_does_not_inherit_an_unvetted_writer_param(monkeypatch):
610+
"""Inheritance is an allowlist, so a param nobody vetted for the reader stays
611+
on the writer. Flipping this to a denylist would let the next schema-affecting
612+
param leak through by default."""
613+
monkeypatch.setenv(
614+
"DATABASE_URL",
615+
"postgresql://u:p@writer.example.com:5432/db?connection_limit=3&application_name=writer&novel_param=x",
616+
)
617+
monkeypatch.setenv("DATABASE_URL_READ_REPLICA", "postgresql://u:p@reader.example.com:5432/db")
618+
619+
_apply()
620+
621+
query = urllib.parse.parse_qs(urllib.parse.urlsplit(os.environ["DATABASE_URL_READ_REPLICA"]).query)
622+
assert query["connection_limit"] == ["3"]
623+
assert "application_name" not in query
624+
assert "novel_param" not in query
625+
626+
627+
def test_reader_keeps_its_own_options_when_writer_params_are_appended(monkeypatch):
628+
"""Appending the writer's pool params must leave the reader's own search_path
629+
intact, since that is what decides which tables its queries resolve against."""
630+
monkeypatch.setenv("DATABASE_URL", "postgresql://u:p@writer.example.com:5432/db?connection_limit=3")
631+
monkeypatch.setenv(
632+
"DATABASE_URL_READ_REPLICA",
633+
"postgresql://u:p@reader.example.com:5432/db?options=-c%20search_path%3Dreader_schema",
634+
)
635+
636+
_apply()
637+
638+
query = urllib.parse.parse_qs(urllib.parse.urlsplit(os.environ["DATABASE_URL_READ_REPLICA"]).query)
639+
assert query["options"] == ["-c search_path=reader_schema"]
640+
assert query["connection_limit"] == ["3"]
641+
642+
643+
def test_reader_url_left_alone_when_writer_has_no_params(monkeypatch):
644+
"""No params to inherit must mean the reader URL is not rewritten at all."""
645+
monkeypatch.setenv("DATABASE_URL", "postgresql://u:p@writer.example.com:5432/db")
646+
monkeypatch.setenv(
647+
"DATABASE_URL_READ_REPLICA",
648+
"postgresql://u:p@reader.example.com:5432/db?options=-c%20search_path%3Dapp",
649+
)
650+
651+
_apply()
652+
653+
assert (
654+
os.environ["DATABASE_URL_READ_REPLICA"]
655+
== "postgresql://u:p@reader.example.com:5432/db?options=-c%20search_path%3Dapp"
656+
)
657+
658+
527659
def test_unsupported_db_scheme_message_names_var_and_scheme():
528660
msg = unsupported_db_scheme_message("DIRECT_URL", "sqlite")
529661
assert "DIRECT_URL" in msg

0 commit comments

Comments
 (0)