From a2cedd676934fc73e405c0d9aee57ff1d736b845 Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Fri, 24 Jul 2026 22:26:57 +0200 Subject: [PATCH] feat: make the DB connection pool env-configurable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add DATABASE_POOL_SIZE / DATABASE_MAX_OVERFLOW / DATABASE_POOL_TIMEOUT, consumed like every other typed env var through the pydantic Settings (constrained Fields, defaults 5/10/30 matching SQLAlchemy's own baseline so leaving them unset is behaviour-neutral). An out-of-range or non-integer value fails validation and refuses to boot — the same fail-loud behaviour a malformed DATABASE_URL already has — rather than silently starting up misconfigured. create_db_engine sizes the pool from these and logs the effective values at startup so they can be confirmed from the boot output during an incident. In-memory SQLite (StaticPool, which rejects the pool kwargs) is detected and built without them. pool_pre_ping is deliberately not exposed: the default backend is a local SQLite file with no network peer to drop idle connections, so it would add a SELECT 1 per checkout for no benefit — and it detects dead connections, not the live-but-wedged ones behind the exhaustion this series addresses. These knobs are infrastructure the node needs before it can open a DB session, so they can never be sourced from the DB (chicken-and-egg). A new ENV_ONLY_FIELDS set keeps them out of the persisted settings blob and stops a DB value from shadowing env in both SettingsService.initialize and .update, so env stays authoritative. Co-Authored-By: Claude Opus 4.8 --- .env.example | 32 ++++++++ routstr/core/db.py | 45 ++++++++++- routstr/core/settings.py | 42 +++++++++- tests/unit/test_db_pool_config.py | 129 ++++++++++++++++++++++++++++++ tests/unit/test_settings.py | 62 ++++++++++++++ 5 files changed, 305 insertions(+), 5 deletions(-) create mode 100644 tests/unit/test_db_pool_config.py diff --git a/.env.example b/.env.example index 8ff04b35..fba99b56 100644 --- a/.env.example +++ b/.env.example @@ -23,6 +23,38 @@ ROUTSTR_SECRET_KEY= # Database # DATABASE_URL=sqlite+aiosqlite:///keys.db +# Database tuning (advanced — you almost certainly do not need these) +# --------------------------------------------------------------------------- +# These size the SQLAlchemy connection pool. The defaults below match +# SQLAlchemy's own baseline, so leaving them unset changes nothing. Touch them +# ONLY if the logs show the pool running out of connections: +# +# QueuePool limit of size 5 overflow 10 reached, connection timed out, timeout 30.00 +# +# The effective values are printed at startup ("Database pool configured: ...") +# so you can confirm what is actually in effect while diagnosing an incident. +# An invalid value (non-integer, or a pool size below 1) stops the node at boot +# with a clear error rather than starting up misconfigured. +# +# DATABASE_POOL_SIZE Connections kept open in the pool. (default 5) +# DATABASE_MAX_OVERFLOW Extra connections opened under load, (default 10) +# beyond pool_size, then discarded. +# DATABASE_POOL_TIMEOUT Seconds a request waits for a free (default 30) +# connection before failing with the error above. +# +# Note for the default SQLite backend: SQLite serialises writes behind a single +# database-level lock, so a bigger pool does NOT buy more write throughput — it +# only lets more readers run at once (WAL mode) and trades pool-timeout errors +# for "database is locked" errors. If you exhaust the pool on SQLite, the cause +# is almost always connections being held too long, not the pool being too +# small. These knobs come into their own if you point DATABASE_URL at a +# networked database (e.g. Postgres), where connections genuinely run in +# parallel; there, size the pool against the server's max_connections. +# +# DATABASE_POOL_SIZE=5 +# DATABASE_MAX_OVERFLOW=10 +# DATABASE_POOL_TIMEOUT=30 + # Node Information # NAME=My Routstr Node # DESCRIPTION=Fast AI API access with Bitcoin payments diff --git a/routstr/core/db.py b/routstr/core/db.py index ab263be4..2c9b5438 100644 --- a/routstr/core/db.py +++ b/routstr/core/db.py @@ -13,7 +13,9 @@ from alembic import command from alembic.config import Config from alembic.util.exc import CommandError from sqlalchemy import Index, UniqueConstraint, case, delete, or_ +from sqlalchemy.engine import make_url from sqlalchemy.exc import IntegrityError, OperationalError +from sqlalchemy.ext.asyncio import AsyncEngine from sqlalchemy.ext.asyncio.engine import create_async_engine from sqlalchemy.orm import aliased from sqlmodel import Field, Relationship, SQLModel, col, func, select, update @@ -26,7 +28,48 @@ logger = get_logger(__name__) DATABASE_URL = os.environ.get("DATABASE_URL", "sqlite+aiosqlite:///keys.db") -engine = create_async_engine(DATABASE_URL, echo=False) # echo=True for debugging SQL +def create_db_engine(database_url: str = DATABASE_URL) -> AsyncEngine: + """Build the async engine, sizing the connection pool from ``settings``. + + Pool sizing comes from the pydantic ``Settings`` (validated at boot like + every other typed env var), so an out-of-range value refuses to start rather + than running misconfigured. Defaults match SQLAlchemy's own baseline + (5 / 10 / 30s); operators only need to tune them when logs show + ``QueuePool limit ... reached, connection timed out`` — see the "Database + tuning" section of ``.env.example``. The effective config is logged so it can + be confirmed from the boot output during an incident. + + In-memory SQLite is a special case: it uses ``StaticPool``, which rejects + the pool-sizing kwargs (passing them raises ``TypeError``), and there is no + pool to size anyway — so those URLs are built without them. + """ + from .settings import settings + + url = make_url(database_url) + is_memory_sqlite = url.get_backend_name() == "sqlite" and url.database in ( + None, + "", + ":memory:", + ) + if is_memory_sqlite: + logger.info("Database pool: in-memory SQLite, using default StaticPool") + return create_async_engine(database_url, echo=False) + + logger.info( + f"Database pool configured: pool_size={settings.database_pool_size} " + f"max_overflow={settings.database_max_overflow} " + f"pool_timeout={settings.database_pool_timeout}s", + ) + return create_async_engine( # echo=True for debugging SQL + database_url, + echo=False, + pool_size=settings.database_pool_size, + max_overflow=settings.database_max_overflow, + pool_timeout=settings.database_pool_timeout, + ) + + +engine = create_db_engine() class ApiKey(SQLModel, table=True): # type: ignore diff --git a/routstr/core/settings.py b/routstr/core/settings.py index a0b5d05c..404ce8d1 100644 --- a/routstr/core/settings.py +++ b/routstr/core/settings.py @@ -100,6 +100,14 @@ class Settings(BaseSettings): refund_cache_ttl_seconds: int = Field(default=3600, env="REFUND_CACHE_TTL_SECONDS") refund_sweep_ttl_seconds: int = Field(default=604800, env="REFUND_SWEEP_TTL_SECONDS") + # Database connection-pool sizing (advanced). Defaults match SQLAlchemy's own + # baseline, so unset is behaviour-neutral — see the "Database tuning" section + # of .env.example. A pool_size of 0 would mean an unbounded pool in + # SQLAlchemy, so it is rejected (ge=1); max_overflow=0 (no overflow) is valid. + database_pool_size: int = Field(default=5, ge=1, env="DATABASE_POOL_SIZE") + database_max_overflow: int = Field(default=10, ge=0, env="DATABASE_MAX_OVERFLOW") + database_pool_timeout: int = Field(default=30, ge=1, env="DATABASE_POOL_TIMEOUT") + # Logging log_level: str = Field(default="INFO", env="LOG_LEVEL") enable_console_logging: bool = Field(default=True, env="ENABLE_CONSOLE_LOGGING") @@ -144,10 +152,25 @@ def _normalize_settings_data(data: dict[str, Any]) -> dict[str, Any]: # ``routstr.core.vault``. SECRET_FIELDS = frozenset({"admin_password", "nsec"}) +# Infrastructure the node needs *before* it can open a DB session — so it can +# never be configured from the DB (chicken-and-egg) and stays env-only. Unlike +# secrets (owned by bootstrap), these are excluded so the DB settings blob can +# neither store nor shadow them; env is always authoritative. +ENV_ONLY_FIELDS = frozenset( + {"database_pool_size", "database_max_overflow", "database_pool_timeout"} +) + +_NON_PERSISTED_FIELDS = SECRET_FIELDS | ENV_ONLY_FIELDS + def _strip_secret_fields(data: dict[str, Any]) -> dict[str, Any]: - """Return a copy of ``data`` without any secret fields (for persistence).""" - return {k: v for k, v in data.items() if k not in SECRET_FIELDS} + """Return a copy of ``data`` without secret or env-only fields. + + Both are kept out of the persisted settings blob: secrets for confidentiality, + env-only fields (e.g. DB pool sizing) because they must never be sourced from + the database. + """ + return {k: v for k, v in data.items() if k not in _NON_PERSISTED_FIELDS} def _apply_to_live_settings(data: dict[str, Any]) -> None: @@ -327,7 +350,13 @@ class SettingsService: valid_fields = set(env_resolved.dict().keys()) merged_dict: dict[str, Any] = dict(env_resolved.dict()) merged_dict.update( - {k: v for k, v in db_json.items() if v not in (None, "", [], {}) and k in valid_fields} + { + k: v + for k, v in db_json.items() + if v not in (None, "", [], {}) + and k in valid_fields + and k not in ENV_ONLY_FIELDS + } ) merged_dict = Settings(**merged_dict).dict() @@ -394,8 +423,13 @@ class SettingsService: ) ) await db_session.commit() - # Update in-place + # Update in-place. Env-only fields (e.g. DB pool sizing) are never + # applied here: the engine pool is already built at boot from env, + # so letting an update mutate the live value would only make it + # diverge from the running pool. for k, v in candidate.dict().items(): + if k in ENV_ONLY_FIELDS: + continue setattr(settings, k, v) cls._current = settings return settings diff --git a/tests/unit/test_db_pool_config.py b/tests/unit/test_db_pool_config.py new file mode 100644 index 00000000..03473024 --- /dev/null +++ b/tests/unit/test_db_pool_config.py @@ -0,0 +1,129 @@ +"""Coverage for env-configurable DB connection-pool sizing. + +Pool sizing comes from the pydantic ``Settings`` (DATABASE_POOL_SIZE / +DATABASE_MAX_OVERFLOW / DATABASE_POOL_TIMEOUT), matching how every other typed +env var is consumed. Defaults equal SQLAlchemy's own baseline so unset is +behaviour-neutral; a bad value fails validation and refuses to boot (like a +malformed DATABASE_URL); the effective config is logged at engine creation; and +pool_pre_ping is never enabled (the default DB is a local SQLite file with no +network peer to drop idle connections). In-memory SQLite uses StaticPool, which +rejects the pool kwargs, so those URLs are built without them. +""" + +import logging + +import pytest +from pydantic.v1 import ValidationError +from sqlalchemy.ext.asyncio import AsyncEngine +from sqlalchemy.pool import QueuePool, StaticPool + +from routstr.core.db import create_db_engine +from routstr.core.settings import Settings, settings + +# A file URL keeps the pool an AsyncAdaptedQueuePool (which honours pool_size); +# engine creation is lazy, so no connection is opened and no file is created. +_URL = "sqlite+aiosqlite:///./_pool_config_test.db" + + +def _pool(engine: AsyncEngine) -> QueuePool: + pool = engine.sync_engine.pool + assert isinstance(pool, QueuePool) + return pool + + +def test_pool_defaults_match_sqlalchemy_baseline() -> None: + """With defaults, the pool keeps SQLAlchemy's 5 / 10 / 30 baseline.""" + pool = _pool(create_db_engine(_URL)) + assert pool.size() == 5 + assert pool._max_overflow == 10 + assert pool._timeout == 30 + + +def test_pool_config_reads_settings_overrides() -> None: + """create_db_engine sizes the pool from the Settings values.""" + with pytest.MonkeyPatch.context() as mp: + mp.setattr(settings, "database_pool_size", 3) + mp.setattr(settings, "database_max_overflow", 7) + mp.setattr(settings, "database_pool_timeout", 12) + + pool = _pool(create_db_engine(_URL)) + + assert pool.size() == 3 + assert pool._max_overflow == 7 + assert pool._timeout == 12 + + +@pytest.mark.parametrize("url", ["sqlite+aiosqlite://", "sqlite+aiosqlite:///:memory:"]) +def test_in_memory_sqlite_skips_pool_sizing(url: str) -> None: + """In-memory SQLite uses StaticPool, which rejects pool kwargs. + + Passing pool_size/max_overflow/pool_timeout to such a URL raises TypeError, + so create_db_engine must not send them — the engine must build cleanly. + """ + engine = create_db_engine(url) + assert isinstance(engine.sync_engine.pool, StaticPool) + + +def test_pool_does_not_enable_pre_ping() -> None: + """pre_ping stays off: the local SQLite file has no peer to drop connections.""" + assert _pool(create_db_engine(_URL))._pre_ping is False + + +def test_engine_creation_logs_effective_pool_config( + caplog: pytest.LogCaptureFixture, +) -> None: + """The effective pool config is logged so operators can confirm it at boot.""" + # The routstr loggers set propagate=False, so caplog's root handler never + # sees the record — attach the capture handler to this logger directly. + db_logger = logging.getLogger("routstr.core.db") + db_logger.addHandler(caplog.handler) + db_logger.setLevel(logging.INFO) + try: + with pytest.MonkeyPatch.context() as mp: + mp.setattr(settings, "database_pool_size", 4) + mp.setattr(settings, "database_max_overflow", 9) + mp.setattr(settings, "database_pool_timeout", 15) + create_db_engine(_URL) + finally: + db_logger.removeHandler(caplog.handler) + + logged = " ".join(record.getMessage() for record in caplog.records) + assert "pool_size" in logged + assert "4" in logged and "9" in logged and "15" in logged + + +# --- Validation happens in Settings (pydantic), like every other typed env var. +# A bad value raises ValidationError when Settings is built at boot, refusing to +# start rather than running misconfigured. + + +def test_non_integer_pool_size_rejected_at_boot( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("DATABASE_POOL_SIZE", "twenty") + with pytest.raises(ValidationError): + Settings() + + +@pytest.mark.parametrize("value", ["0", "-3"]) +def test_out_of_range_pool_size_rejected_at_boot( + monkeypatch: pytest.MonkeyPatch, value: str +) -> None: + """pool_size below 1 is rejected: 0 would mean an unbounded pool.""" + monkeypatch.setenv("DATABASE_POOL_SIZE", value) + with pytest.raises(ValidationError): + Settings() + + +def test_negative_pool_timeout_rejected_at_boot( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("DATABASE_POOL_TIMEOUT", "0") + with pytest.raises(ValidationError): + Settings() + + +def test_zero_max_overflow_is_allowed(monkeypatch: pytest.MonkeyPatch) -> None: + """max_overflow=0 (no overflow) is a valid choice, not a rejection trigger.""" + monkeypatch.setenv("DATABASE_MAX_OVERFLOW", "0") + assert Settings().database_max_overflow == 0 diff --git a/tests/unit/test_settings.py b/tests/unit/test_settings.py index a553b6d4..d85b52f7 100644 --- a/tests/unit/test_settings.py +++ b/tests/unit/test_settings.py @@ -62,6 +62,68 @@ def test_payout_settings_have_sensible_defaults() -> None: assert s.payout_interval_seconds == 900 +@pytest.mark.asyncio +async def test_database_pool_fields_are_env_only_not_persisted( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """DB pool sizing is infrastructure the node needs *before* it can read the + DB, so it can never be configured from the DB — it must never be written to + the settings blob, and a stale/injected DB value must never shadow env. + """ + monkeypatch.setenv("DATABASE_POOL_SIZE", "7") + + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + async with AsyncSession(engine, expire_on_commit=False) as session: + s = await SettingsService.initialize(session) + + # The env value is live for runtime consumers... + assert s.database_pool_size == 7 + # ...but pool sizing is never written to the settings blob. + blob = await _read_settings_blob(session) + assert "database_pool_size" not in blob + assert "database_max_overflow" not in blob + assert "database_pool_timeout" not in blob + + # Even a stale blob that somehow carries a pool value must not win: env + # stays authoritative on the next initialize. + await session.exec( # type: ignore + text("UPDATE settings SET data = :d WHERE id = 1").bindparams( + d=json.dumps({"database_pool_size": 99}) + ) + ) + await session.commit() + again = await SettingsService.initialize(session) + assert again.database_pool_size == 7 + + +@pytest.mark.asyncio +async def test_update_does_not_apply_env_only_fields_to_live_settings( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """DB pool sizing is env-only: a settings update must neither persist it nor + mutate the live value. The engine pool is already built at boot from env, so + a UI/API update carrying a pool value must not make the live setting diverge + from the running pool. + """ + monkeypatch.delenv("DATABASE_POOL_SIZE", raising=False) + monkeypatch.setattr(settings, "database_pool_size", 5) + + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + async with AsyncSession(engine, expire_on_commit=False) as session: + await SettingsService.initialize(session) + await SettingsService.update( + {"database_pool_size": 99, "name": "PoolTweaker"}, session + ) + + # A non-env-only field still updates normally... + assert settings.name == "PoolTweaker" + # ...but the env-only pool size stays at the boot value. + assert settings.database_pool_size == 5 + # ...and it is never written to the settings blob. + blob = await _read_settings_blob(session) + assert "database_pool_size" not in blob + + @pytest.mark.parametrize( "field,bad_value", [