fix(db): raise SQLAlchemy connection pool size to match real concurrency - #77
Open
mateusbellozupko wants to merge 2 commits into
Open
mateusbellozupko wants to merge 2 commits into
mateusbellozupko wants to merge 2 commits into
Conversation
engine = create_engine(POSTGRES_CONNECTION_STRING, ...) never set pool_size/max_overflow, so it ran on SQLAlchemy's bare defaults (5 + 10 overflow = 15 total connections). Under normal load — many stage-inactivity AI calls held open for 30-90s each, plus live chat traffic — that pool exhausts and stays exhausted: every new request times out after 30s with "QueuePool limit of size 5 overflow 10 reached", with no recovery until the process is restarted. Observed in production for 14+ hours, blocking every AI-generated automated message account-wide. Make pool_size/max_overflow configurable via DB_POOL_SIZE/ DB_MAX_OVERFLOW, defaulting higher (20/40) than the library defaults that exhausted. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Reviewer's GuideThe PR addresses production pool exhaustion by configuring SQLAlchemy with larger, environment-controlled pool and overflow limits, defaulting to 20 and 40 instead of 5 and 10, and adds tests covering both overrides and safer defaults. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/config/database.py" line_range="50-51" />
<code_context>
POSTGRES_CONNECTION_STRING,
pool_pre_ping=True,
pool_recycle=1800,
+ pool_size=settings.DB_POOL_SIZE,
+ max_overflow=settings.DB_MAX_OVERFLOW,
connect_args={
"keepalives": 1,
</code_context>
<issue_to_address>
**issue (broader_impact):** The default pool configuration permits up to 60 PostgreSQL connections per process (20 pooled connections plus 40 overflow connections). With multiple Uvicorn workers or service replicas, the aggregate limit exceeds PostgreSQL's default `max_connections` and causes connection attempts to fail once the database-wide limit is reached.
**Triggers:** When the service runs with multiple workers/replicas or shares PostgreSQL with other services.
**Suggested fix:** Set a deployment-wide connection budget and size each process's pool accordingly, or explicitly raise/configure PostgreSQL's connection limit alongside these settings.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and the new defaults change how many database connections the service can open, potentially overloading Postgres or causing connection exhaustion and an outage under real concurrency. Reverting restores the previous limits, but any outage or failed requests that already occurred cannot be undone.
Blocking findings: src/config/database.py:51
…ployments (Sourcery review) Sourcery's review on the public PR flagged that DB_POOL_SIZE/ DB_MAX_OVERFLOW are per-process: with multiple uvicorn workers or service replicas, the aggregate connection count can exceed Postgres's own max_connections shared across every service on that instance. The defaults (20/40) are correct for this deployment's single-process topology (no --workers flag) and were chosen from a real production incident, so they aren't being lowered defensively without evidence -- documenting the tradeoff so it's sized correctly before anyone scales workers/replicas. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
create_engine(POSTGRES_CONNECTION_STRING, ...)never setpool_size/max_overflow, so it ran on SQLAlchemy's bare defaults (5 + 10 overflow = 15 total connections).QueuePool limit of size 5 overflow 10 reached, with no recovery until the process is restarted. Observed in production for 14+ hours, blocking every AI-generated automated message account-wide.pool_size/max_overflowconfigurable viaDB_POOL_SIZE/DB_MAX_OVERFLOW, defaulting higher (20/40) than the library defaults that exhausted.Test plan
tests/unit/test_database_pool_config.py: the engine's pool size/overflow are configurable via env vars, and default higher than SQLAlchemy's bare defaults.🤖 Generated with Claude Code
Summary by Sourcery
Configure a larger, environment-controlled database connection pool to sustain concurrent AI and chat workloads without exhausting available connections.
Bug Fixes:
Enhancements:
Tests: