Uh oh!
There was an error while loading. Please reload this page.
improvement(db): opt-in read-replica client + migration runner hardening - #4955
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Migration runner is hardened: optional Replica pagination safety: data drains apply a 5-minute UI: table column rename returns Reviewed by Cursor Bugbot for commit f078233. Configure here. |
Uh oh!
There was an error while loading. Please reload this page.
Greptile SummaryThis PR introduces two independent but related improvements: migration runner hardening and an opt-in read-replica client.
Confidence Score: 5/5Safe to merge. Billing enforcement, auth, and authz-boundary reads deliberately stay on the primary; the replica is only used for display and export paths where bounded staleness is acceptable. The enforcement/replica boundary is carefully drawn and consistently applied across all 15+ touched call sites. Migration hardening is additive and backward-compatible. The one minor inconsistency ( apps/sim/lib/billing/core/usage.ts — Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[API Request] --> B{Read type?}
B -->|Enforcement: usage gates, thresholds| C[primary db]
B -->|Auth / workflow state| C
B -->|Billing display: summary, usage-limits| D{DATABASE_REPLICA_URL set?}
B -->|Log listing / stats / export| D
B -->|Data drain page scans| D
B -->|Audit-log API page queries| D
D -->|Yes| E[dbReplica pool - max: 10 connections]
D -->|No| C
C --> F[(Primary Postgres)]
E --> G[(Read Replica)]
G -.->|async replication| F
subgraph boundary[Boundary reads - always primary]
H[org workspace scope IDs - getOrgWorkspaceIds]
I[cursor resolution - usageLog cursor row]
J[auth / session]
end
H --> F
I --> F
J --> F
Reviews (5): Last reviewed commit: "fix(billing): usage-limits returns cost ..." | Re-trigger Greptile |
… migration DSN support
…coping, shared error helper
…d backend pid across retries
…er pids legitimately vary)
…ad replica via executor threading
waleedlatif1
commented
Jun 11, 2026
waleedlatif1
commented
Jun 11, 2026
@cursor review |
Uh oh!
There was an error while loading. Please reload this page.
…e rows are never skipped
waleedlatif1
commented
Jun 11, 2026
waleedlatif1
commented
Jun 11, 2026
@cursor review |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
waleedlatif1
commented
Jun 11, 2026
waleedlatif1
commented
Jun 11, 2026
@cursor review |
Uh oh!
There was an error while loading. Please reload this page.
waleedlatif1
commented
Jun 11, 2026
waleedlatif1
commented
Jun 11, 2026
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f078233. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
Summary
Migration runner hardening (
packages/db/scripts/migrate.ts)lock_timeout = '5s'asserted at the top of every migration attempt — DDL that can't get its table lock fails fast (SQLSTATE 55P03) instead of queueing all table traffic behind a pending AccessExclusiveLock. Applied viaclient.unsafe(Postgres rejects parameterizedSET)backoffWithJitter), 55P03 detected viagetPostgresErrorCode(walks drizzle's wrappedcausechains)pg_try_advisory_lockloop (30-min deadline) — a wedged runner fails its peers visibly instead of silently hanging concurrent migration runnerswarnOnInvalidIndexes()after every run — surfaces INVALID indexes from failed CONCURRENTLY builds thatIF NOT EXISTSwould silently skip foreverCOMMITfiles must clear AND restorelock_timeout, with corrected batch-transaction semantics (drizzle wraps all pending files in ONE transaction) and idempotency requirementsmax_lifetime: null(postgres-js otherwise recycles the connection after a randomized 30–60 min, silently dropping the advisory lock and sessionSETs) and a backend-pid guard before every attempt (an externally dropped/replaced connection aborts loudly instead of running unlocked)MIGRATION_DATABASE_URL: migrations prefer a direct (non-pooled) DSN — session advisory locks and sessionSETs are unsupported through PgBouncer transaction pooling. Falls back toDATABASE_URL(direct-connection setups need no change)Read replica (opt-in, not
withReplicas)dbReplicaexport in@sim/db: separate pool from optionalDATABASE_REPLICA_URL, falls back to the primary when unset (self-hosted setups unaffected), fail-fast DSN validation at bootlist-logs.ts), logs stats + export routes, v1 audit-log page queries, the display-only reads ingetUserUsageLogs, and data-drain source page scansexecutor: DbOrTx = dbparam ongetBillingPeriodUsageCost/getBillingPeriodUsageCostByUser/computeDailyRefreshConsumed/getUserUsageData/getEffectiveCurrentPeriodCost/getSimplifiedBillingSummary/getOrganizationBillingData/getOrgMemberLedgerByUser. Only display routes (billing summary, usage-limits, org member ledgers, admin billing views) pass the replica; every enforcement caller (usage gates, overage/invoice calculation, threshold billing) keeps the primary default@sim/testing+ local factories) exportdbReplicaas the same instance asdbMisc
table-gridcolumn rename returnsmutateAsyncperuseInlineRename's contract (isSavingnow spans the request)Deliberate trade (reviewed)
The per-row
permissionsjoin inside the logs list/stats/export queries now evaluates on the replica: a user whose workspace access was just revoked can still list/export that workspace's logs for up to the replication-lag window (typically sub-second). Accepted because the exposure is transient and bounded, there is no monotonic cursor to cause permanent damage, and pinning the join to the primary would defeat the offload. Tenant-boundary scoping reads that gate external API access (v1 audit logs org scope, data-drain workspace scope) deliberately stay on the primary.Type of Change
Testing
check:api-validationpassesMIGRATION_DATABASE_URLoverride verifiedChecklist