feat(Database): Support for reader DB instance for all read operations - #956
feat(Database): Support for reader DB instance for all read operations#956MonishJuspay wants to merge 1 commit into
Conversation
- Ensures reducing load on the primary DB instance to avoid downtime due to heavy analytic queries - Faster retrievable of data for read operations as reader DB has no overhead of writing ,it just replicates from the primary DB - Fallback prevents even if the reader_db_instance connection fails, fallsback to the primary instance at the start
WalkthroughAdds PostgreSQL read-replica configuration, separate write/read asyncpg pools with lifecycle handling, and a read-query execution entry point while preserving the existing write-pool alias and query behavior. ChangesDatabase read-replica support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Application
participant run_read_query
participant get_read_db_connection
participant read_pool
Application->>run_read_query: submit SELECT query and values
run_read_query->>get_read_db_connection: request read connection
get_read_db_connection->>read_pool: acquire connection
run_read_query->>read_pool: fetch query results
read_pool-->>Application: return first result set
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds infrastructure to support routing read-only database queries to a PostgreSQL read replica (when configured), while preserving a safe fallback to the primary instance when the replica is not configured.
Changes:
- Introduces separate
write_pool(primary) andread_pool(replica/fallback) and addsget_read_db_connection()to acquire read connections. - Adds
run_read_query()as an explicit read-path query helper alongside the existing write-path helper. - Extends static configuration with
POSTGRES_READER_HOST/POSTGRES_READER_PORTenvironment variables.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| app/database/queries/init.py | Adds run_read_query() and documents read vs write query entry points. |
| app/database/init.py | Implements read/write pools, connection generators, and coordinated shutdown behavior. |
| app/core/config/static.py | Adds env vars for read-replica host/port configuration. |
Comments suppressed due to low confidence (1)
app/database/init.py:135
- This comment suggests that callers who imported
pooldirectly will get a live pool object. Reassigningpoolat runtime does not update previously imported bindings, so the comment should be corrected to avoid implying stronger backward compatibility than Python provides.
# Keep the legacy ``pool`` alias in sync so callers that imported
# ``pool`` directly still get a live pool object.
pool = write_pool
| run_read_query(query, values) | ||
| Executes on the READ pool (replica when POSTGRES_READER_HOST is set, | ||
| otherwise the primary via the write_pool alias — safe fallback with no | ||
| config change required). | ||
| Use for plain SELECT queries where a small replication lag is acceptable. |
| # ``pool`` is kept as a public alias for ``write_pool`` so that any code | ||
| # outside this module that imported ``pool`` directly continues to work. | ||
| # --------------------------------------------------------------------------- |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/database/__init__.py`:
- Around line 141-149: Update the replica initialization branch in init_db_pool
around _create_pool so failures creating the configured read pool are caught and
logged, then set read_pool to write_pool as the fallback. Preserve the existing
replica creation path when successful and ensure the fallback aliases the
already initialized primary pool.
- Around line 165-180: The connection generator functions get_db_connection()
and get_read_db_connection() both need explicit async-generator return
annotations. Update each signature in app/database/__init__.py at lines 165-180
and 183-199 to use the appropriate AsyncGenerator type for the yielded database
connection, preserving their existing pool acquisition behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cd90b72a-3f1c-435d-8f64-46f7babe61f8
📒 Files selected for processing (3)
app/core/config/static.pyapp/database/__init__.pyapp/database/queries/__init__.py
| if POSTGRES_READER_HOST: | ||
| read_pool = await _create_pool( | ||
| host=POSTGRES_READER_HOST, | ||
| port=POSTGRES_READER_PORT or POSTGRES_PORT, | ||
| min_size=resolved_min, | ||
| max_size=resolved_max, | ||
| decrypted_password=decrypted_password, | ||
| label="Read", | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Fall back to the primary when replica initialization fails.
_create_pool() re-raises its failure, so an unavailable configured replica aborts init_db_pool() rather than setting read_pool = write_pool. This defeats the promised primary fallback and makes read access unavailable. Catch replica-pool creation failures here, log them, and alias the write pool instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/database/__init__.py` around lines 141 - 149, Update the replica
initialization branch in init_db_pool around _create_pool so failures creating
the configured read pool are caught and logged, then set read_pool to write_pool
as the fallback. Preserve the existing replica creation path when successful and
ensure the fallback aliases the already initialized primary pool.
| async def get_db_connection(): | ||
| """ | ||
| Get a database connection from the pool. | ||
| Yield a connection from the WRITE pool (primary). | ||
|
|
||
| Use for INSERT / UPDATE / DELETE and any transaction that mixes reads | ||
| with writes. | ||
| """ | ||
| global write_pool | ||
| if write_pool is None: | ||
| await init_db_pool() | ||
|
|
||
| if write_pool is None: | ||
| raise RuntimeError("Write pool is not initialised.") | ||
|
|
||
| async with write_pool.acquire() as connection: | ||
| yield connection |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## git status/stat"
git status --short
git diff --stat || true
echo "## relevant file outline/lines"
wc -l app/database/__init__.py
sed -n '1,240p' app/database/__init__.py | nl -ba
echo "## type annotation/search indicators"
python3 - <<'PY'
import ast
from pathlib import Path
p = Path("app/database/__init__.py")
tree = ast.parse(p.read_text())
for node in ast.walk(tree):
if isinstance(node, (ast.AsyncFunctionDef, ast.FunctionDef)):
if node.name in {"get_db_connection", "get_read_db_connection"}:
print(f"{node.name}:{node.lineno} returns={ast.unparse(node.returns) if node.returns else None}")
PYRepository: juspay/clairvoyance
Length of output: 277
🏁 Script executed:
#!/bin/bash
set -u
echo "## relevant file lines"
wc -l app/database/__init__.py
awk '{printf "%-5s%s\n", NR, $0}' app/database/__init__.py | sed -n '1,245p'
echo "## AST annotations"
python3 - <<'PY'
import ast
from pathlib import Path
p = Path("app/database/__init__.py")
tree = ast.parse(p.read_text())
for node in ast.walk(tree):
if isinstance(node, (ast.AsyncFunctionDef, ast.FunctionDef)):
if node.name in {"get_db_connection", "get_read_db_connection"}:
returns = ast.unparse(node.returns) if node.returns else None
print(f"{node.name}:{node.lineno} returns={returns}")
PYRepository: juspay/clairvoyance
Length of output: 9580
Add return annotations to both connection generators.
get_db_connection()currently lacks any return annotation.get_read_db_connection()also lacks any return annotation.
Annotate both with the appropriate async-generator return type so the required signature hints are complete.
📍 Affects 1 file
app/database/__init__.py#L165-L180(this comment)app/database/__init__.py#L183-L199
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/database/__init__.py` around lines 165 - 180, The connection generator
functions get_db_connection() and get_read_db_connection() both need explicit
async-generator return annotations. Update each signature in
app/database/__init__.py at lines 165-180 and 183-199 to use the appropriate
AsyncGenerator type for the yielded database connection, preserving their
existing pool acquisition behavior.
Source: Coding guidelines
|
check build |
| return [] | ||
|
|
||
|
|
||
| async def run_read_query(query_text: str, values: List[Any]) -> List[asyncpg.Record]: |
There was a problem hiding this comment.
🟥 [MAJOR — the reader is never used; PR delivers zero read-offloading
run_read_query / get_read_db_connection (defined here) have zero callers anywhere in app/ (verified by grep). Every accessor still routes through run_parameterized_query → get_db_connection → the primary. So with POSTGRES_READER_HOST set, the read pool opens connections and is then never queried — all reads still hit the primary. The PR title ("read operations routed to reader") is not realized.
Fix: migrate read-only accessors to run_read_query (at least one) before this is useful — otherwise this is dead plumbing.
PR #956 —
|
|
Reviewed at head 1. Confirming narsimhaReddyJuspay's open MAJOR — the reader is currently a no-op. Independently re-verified via a repo-wide GitHub code search: 2. Confirming coderabbitai's open Major — replica-init failure isn't actually a fallback. 3. New — if read_pool is not None and read_pool is not write_pool:
try:
await read_pool.close()
...
except Exception as e:
logger.error(f"Failed to close read pool: {e}")
raise
finally:
read_pool = None
if write_pool is not None:
try:
await write_pool.close()If Not raising any test-coverage/design nits beyond these — this is backend-only Python, no UI surface. |
Summary by CodeRabbit
New Features
Bug Fixes