Skip to content

v0.6.72: billing pool contention fix#4496

Merged
TheodoreSpeaks merged 1 commit intomainfrom
staging
May 7, 2026
Merged

v0.6.72: billing pool contention fix#4496
TheodoreSpeaks merged 1 commit intomainfrom
staging

Conversation

@TheodoreSpeaks
Copy link
Copy Markdown
Collaborator

… contention (#4494)

* fix(billing): drop transaction wrapper in recordUsage to relieve pool contention

* fix(billing): always warn on userStats drift in recordUsage
@vercel
Copy link
Copy Markdown

vercel Bot commented May 7, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 7, 2026 6:50pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 7, 2026

PR Summary

Medium Risk
Changes billing write semantics by removing the atomic transaction between usage_log inserts and user_stats counter updates, allowing counters to drift when the update fails. This reduces pgbouncer connection pinning under high concurrency but risks temporary mismatches in reported totals/period costs.

Overview
Updates recordUsage to stop wrapping usage_log inserts and user_stats counter increments in a single DB transaction to alleviate pgbouncer pool contention under concurrent usage.

usage_log inserts remain required (errors propagate), while the user_stats update is now best-effort: missing-row and update failures no longer throw/rollback and instead log warnings (including whether entries existed and which additionalStats keys were requested).

Reviewed by Cursor Bugbot for commit ec5793f. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 7, 2026

Greptile Summary

This PR removes the db.transaction() wrapper from recordUsage to fix pgbouncer pool exhaustion caused by long-lived row locks on user_stats under high per-user concurrency. The usage_log INSERT now commits immediately and propagates errors to callers, while the userStats UPDATE is demoted to best-effort with all failures logged and swallowed.

  • Behavioral change: previously, a missing user_stats row caused the usage_log INSERT to roll back; it now commits the INSERT regardless, which is a strict improvement for data durability.
  • Accepted trade-off: any user_stats UPDATE failure leaves currentPeriodCost / totalCost counters under-counted until reconciled from usage_log; billing-limit enforcement may allow slightly more spend than configured during drift windows.
  • Catch-all error handling: the catch block treats transient and permanent failures identically, permanently dropping the counter increment rather than retrying."

Confidence Score: 4/5

Safe to merge — the fix correctly addresses pool contention and the data durability trade-offs are clearly documented and intentional.

The change is well-reasoned and the comments document the trade-offs clearly. The one gap is that all userStats UPDATE failures — including transient ones — are permanently dropped rather than retried, which could widen counter drift more than necessary.

apps/sim/lib/billing/core/usage-log.ts — specifically the catch block in recordUsage and any downstream callers that read currentPeriodCost / totalCost from userStats to enforce billing limits.

Important Files Changed

Filename Overview
apps/sim/lib/billing/core/usage-log.ts Removes the db.transaction() wrapper from recordUsage, making usage_log INSERT the committed source of truth and demoting userStats UPDATE to best-effort; any UPDATE failure (including transient errors) is now permanently swallowed and logged as a warning rather than retried.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant recordUsage
    participant DB_usageLog as DB: usage_log
    participant DB_userStats as DB: user_stats

    Caller->>recordUsage: recordUsage(params)

    alt "validEntries.length > 0"
        recordUsage->>DB_usageLog: INSERT rows (propagates errors)
        DB_usageLog-->>recordUsage: OK / throws
    end

    recordUsage->>DB_userStats: UPDATE userStats (lastActive, totalCost, currentPeriodCost)
    alt UPDATE succeeds, row found
        DB_userStats-->>recordUsage: "[{userId}]"
        recordUsage-->>Caller: void
    else UPDATE succeeds, row NOT found
        DB_userStats-->>recordUsage: []
        recordUsage->>recordUsage: logger.warn (counter dropped)
        recordUsage-->>Caller: void
    else UPDATE throws
        DB_userStats-->>recordUsage: error
        recordUsage->>recordUsage: logger.warn (counter dropped)
        recordUsage-->>Caller: void
    end
Loading

Reviews (1): Last reviewed commit: "fix(billing): drop transaction wrapper i..." | Re-trigger Greptile

Comment thread apps/sim/lib/billing/core/usage-log.ts
@TheodoreSpeaks TheodoreSpeaks merged commit 273e608 into main May 7, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant