Add readonly async pool settings for pre-warming and optimized pool recycling

[johnewart]

This change adds some new configuration flags and changes the way the readonly async connection pool behaves to achieve higher read throughput on async endpoints. These include:

• Pool pre-warming (checks out all available connections to force the pool to open them to the database to avoid TCP connection overhead later on)
• Disabling rollback on returning the connection to the pool
• Turning on isolation_level=AUTOCOMMIT for the pool
• Adds a specific connection pool size and overflow for the pool

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Review	Updated (UTC)
fides-plus-nightly	Ignored	Preview	Jan 13, 2026 0:04am
fides-privacy-center	Ignored		Jan 13, 2026 0:04am

[johnewart]

@greptileai

[greptile-apps]

Greptile Overview

Greptile Summary

Overview

This PR adds configuration options to optimize the async read-only database connection pool for higher throughput. It introduces 6 new settings: pool size/overflow, pre-ping, skip rollback, autocommit, and pre-warming.

Key Changes

• Pool Configuration: Adds dedicated pool size (default 5) and max overflow (default 10) settings for the readonly async pool, separate from the main async pool settings
• Performance Optimizations: Enables pool_reset_on_return=None (skips rollback) and isolation_level="AUTOCOMMIT" by default for readonly connections
• Pool Pre-warming: Implements a warm_async_pool() function that pre-establishes all pool connections on startup to avoid TCP handshake overhead on first request
• Session Management: Refactors session creation to use context managers (prewarmed_async_readonly_session and non_warmed_async_readonly_session)

Critical Issues Found

1. Prewarm Configuration Never Used (CRITICAL)

The async_readonly_database_prewarm config setting is defined but never checked. The code always warms the pool when a readonly URI exists, regardless of this setting. Line 143-147 should check CONFIG.database.async_readonly_database_prewarm to honor the user's preference.

2. Incorrect Type Annotation

Line 149-151 assigns async_session_factory (a sessionmaker) to async_session but annotates it as returning _AsyncGeneratorContextManager. The type should be Callable[[], AsyncSession].

Design Concerns

Aggressive Performance Defaults

The PR enables both skip_rollback=True and autocommit=True by default. While these improve performance, they assume:

• The database enforces read-only at the user/role level (not just connection string)
• Application code doesn't rely on transaction rollback for cleanup
• Sessions can be returned to the pool in any state

Consider whether these should be opt-in rather than default, or add explicit documentation about the required database configuration.

Prewarm Default Value

The prewarm feature defaults to False despite being highlighted in the PR description as a throughput improvement. Combined with the bug where it's always enabled anyway, this creates confusion about the intended behavior.

Confidence Score: 2/5

• This PR has a critical bug where the prewarm configuration is never used, plus type annotation issues and aggressive performance defaults that need careful review
• Score of 2 reflects a critical logic bug where async_readonly_database_prewarm is defined but never checked, causing the pool to always warm regardless of configuration. Additionally, the type annotation for async_session is incorrect. The aggressive defaults (autocommit=True, skip_rollback=True) could cause issues if database permissions aren't properly configured. While the changes are conceptually sound for performance optimization, the implementation has bugs that must be fixed before merging.
• src/fides/api/db/ctl_session.py requires immediate attention - the prewarm config bug on lines 143-147 must be fixed, and the type annotation on lines 149-151 should be corrected

Important Files Changed

File Analysis

Filename	Score	Overview
src/fides/config/database_settings.py	4/5	Adds 6 new configuration fields for async readonly pool settings - pool_size, max_overflow, pre_ping, pool_skip_rollback, autocommit, and prewarm flags
src/fides/api/db/ctl_session.py	1/5	Implements pool warming and optimized settings, but the prewarm config flag is never used and always warms when readonly URI is configured

[greptile-apps]

_{6 files reviewed, 6 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

There's an inconsistency: the session is created after acquiring and releasing the lock, but the session commit/rollback operations happen outside the lock. This means if warming fails or is still in progress when multiple requests arrive, some sessions could be created before the pool is fully warmed.

Additionally, for a read-only database with AUTOCOMMIT isolation level and pool_reset_on_return=None, calling await session.commit() on line 121 is redundant. The commit is effectively a no-op since autocommit is enabled and there's no transaction to commit.

Consider whether the commit/rollback logic should be simplified for read-only sessions, especially when autocommit is enabled.

[johnewart]

I believe the lock should be acquired at the beginning of the generator by whomever calls it and given to whoever requests the lock first; if anyone else is currently warming the pool, block until the pool is warm, at which point it will discover that the pool is warmed and then the lock will be held only briefly by anyone from then onwards; the session should not be inside the lock because then the others will block until the transaction is done.

[greptile-apps]

The combination of async_readonly_database_pool_skip_rollback=True (line 202-205) and async_readonly_database_autocommit=True (line 206-209) as defaults is aggressive and could cause issues if the database isn't truly read-only or if application code assumes standard transaction semantics.

Concerns:

1. AUTOCOMMIT with read-only: While the comment says "this should effectively be a no-op because it's a readonly database," this assumes database-level enforcement. If the readonly restriction is only enforced at the user/role level and there's a misconfiguration, this could cause unintended writes to auto-commit.

2. Skipping rollback: Combined with AUTOCOMMIT, this means sessions won't be cleaned up properly if they encounter errors. While this improves performance, it could leave connections in unexpected states.

Consider adding more documentation about the assumptions these settings make (e.g., database must enforce read-only at the user/role level) and potentially making these opt-in rather than default.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

[erosselli]

@johnewart do we want set defaults as false for now and just change the settings to true in nightly?

[johnewart]

Updated defaults to false

[greptile-apps]

The PR description states that pool pre-warming is one of the features being added "to achieve higher read throughput on async endpoints." However, this setting defaults to False, which means users won't get this benefit unless they explicitly enable it.

Additionally, due to the bug in ctl_session.py lines 143-147, this setting is currently not being used at all - the pool is always warmed when a readonly URI is configured.

Consider whether:

1. The default should be True to match the PR's intent of improving throughput
2. The setting should be better documented about when to enable/disable it
3. There should be a warning logged when warming is disabled but a readonly URI is configured

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

[johnewart]

Defaults are false to avoid introducing default behavior unintentionally

[erosselli]

Approving with my open comments