Skip to content

Commit e83deee

Browse files
fix(billing): bound cumulative usage lock holders (#7655)
* fix(billing): bound cumulative usage lock holders * fix(billing): retain older PostgreSQL timeout compatibility
1 parent 23afa2a commit e83deee

9 files changed

Lines changed: 537 additions & 76 deletions

File tree

.github/workflows/test-build.yml

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,19 @@ jobs:
3030
--health-interval 5s
3131
--health-timeout 5s
3232
--health-retries 10
33+
postgres-legacy:
34+
image: postgres:16-alpine
35+
env:
36+
POSTGRES_USER: postgres
37+
POSTGRES_PASSWORD: postgres
38+
POSTGRES_DB: sim_billing_test
39+
ports:
40+
- 5433:5432
41+
options: >-
42+
--health-cmd "pg_isready -U postgres -d sim_billing_test"
43+
--health-interval 5s
44+
--health-timeout 5s
45+
--health-retries 10
3346
env:
3447
DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5432/sim_auth_scim
3548
OAUTH_TOKEN_FAMILY_TEST_DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5432/sim_auth_scim
@@ -83,6 +96,19 @@ jobs:
8396
ee/scim/lib/managed-membership.postgres.test.ts
8497
lib/auth/sso/application/admit-sso-user.postgres.test.ts
8598
99+
- name: Verify cumulative billing timeout recovery in PostgreSQL
100+
working-directory: apps/sim
101+
env:
102+
BILLING_USAGE_TEST_DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5432/sim_auth_scim
103+
run: bunx vitest run lib/billing/core/usage-log.postgres.test.ts
104+
105+
- name: Verify cumulative billing timeout recovery on PostgreSQL 16
106+
if: matrix.provision == 'push'
107+
working-directory: apps/sim
108+
env:
109+
BILLING_USAGE_TEST_DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5433/sim_billing_test
110+
run: bunx vitest run lib/billing/core/usage-log.postgres.test.ts
111+
86112
- name: Verify SCIM and administration over real HTTP
87113
working-directory: apps/sim
88114
env:

apps/sim/app/api/billing/update-cost/route.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
312312
* Every accepted callback has a stable key, so the maximum cumulative cost
313313
* converges on one ledger event without underbilling or double-billing.
314314
*/
315+
const usageStartedAt = Date.now()
315316
const result = await recordCumulativeUsage({
316317
userId,
317318
workspaceId: resolvedWorkspaceId,
@@ -330,6 +331,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
330331
billedDelta: result.delta,
331332
newTotal: result.total,
332333
billed: result.billed,
334+
durationMs: Date.now() - usageStartedAt,
333335
})
334336

335337
// Reconcile the payer's ledger-backed threshold after every cumulative
Lines changed: 271 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,271 @@
1+
/**
2+
* @vitest-environment node
3+
*
4+
* Uses a disposable schema in local PostgreSQL 15+. The paused callback models
5+
* a client that stops progressing after writing usage but before COMMIT; the
6+
* database must release its locks without waiting for that client to resume.
7+
*/
8+
import { createRequire } from 'node:module'
9+
import { getPostgresErrorCode } from '@sim/utils/errors'
10+
import { generateId } from '@sim/utils/id'
11+
import { sql } from 'drizzle-orm'
12+
import { drizzle } from 'drizzle-orm/postgres-js'
13+
import postgres from 'postgres'
14+
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
15+
16+
const { databaseUrl, transaction } = vi.hoisted(() => {
17+
const databaseUrl = process.env.BILLING_USAGE_TEST_DATABASE_URL
18+
if (databaseUrl && !['localhost', '127.0.0.1', '[::1]'].includes(new URL(databaseUrl).hostname)) {
19+
throw new Error('Billing usage integration tests require a disposable local database')
20+
}
21+
return { databaseUrl, transaction: vi.fn() }
22+
})
23+
24+
vi.unmock('@sim/db/schema')
25+
vi.unmock('drizzle-orm')
26+
vi.mock('@sim/db', () => ({ db: { transaction }, dbReplica: {} }))
27+
vi.mock('@/lib/billing/core/plan', () => ({ getHighestPrioritySubscription: vi.fn() }))
28+
vi.mock('@/lib/billing/subscriptions/utils', () => ({ isOrgScopedSubscription: vi.fn() }))
29+
30+
import {
31+
CumulativeUsageContextMismatchError,
32+
type RecordCumulativeUsageParams,
33+
recordCumulativeUsage,
34+
} from '@/lib/billing/core/usage-log'
35+
36+
const require = createRequire(import.meta.url)
37+
const commonJsPostgres = require('postgres') as typeof postgres
38+
39+
const schemaName = `billing_usage_${generateId().replaceAll('-', '')}`
40+
const connection = databaseUrl
41+
? postgres(databaseUrl, {
42+
max: 8,
43+
prepare: false,
44+
fetch_types: false,
45+
connection: { search_path: schemaName },
46+
onnotice: () => undefined,
47+
})
48+
: undefined
49+
const database = connection ? drizzle(connection) : undefined
50+
51+
type Transaction = Parameters<Parameters<NonNullable<typeof database>['transaction']>[0]>[0]
52+
53+
function deferred() {
54+
let resolve: () => void = () => undefined
55+
const promise = new Promise<void>((done) => {
56+
resolve = done
57+
})
58+
return { promise, resolve }
59+
}
60+
61+
interface PausedTransaction {
62+
reached: ReturnType<typeof deferred>
63+
release: ReturnType<typeof deferred>
64+
lockOnly: boolean
65+
}
66+
67+
let nextPause: PausedTransaction | undefined
68+
let holderTimeoutSetting = 'transaction_timeout'
69+
70+
function pauseNextTransaction(lockOnly = false): PausedTransaction {
71+
const pause = { reached: deferred(), release: deferred(), lockOnly }
72+
nextPause = pause
73+
return pause
74+
}
75+
76+
function usage(cost: number, eventKey = 'update-cost:shared-request'): RecordCumulativeUsageParams {
77+
return {
78+
userId: 'actor',
79+
workspaceId: 'workspace',
80+
billingEntity: { type: 'organization', id: 'payer' },
81+
billingPeriod: {
82+
start: new Date('2026-09-01T00:00:00.000Z'),
83+
end: new Date('2026-10-01T00:00:00.000Z'),
84+
},
85+
source: 'workspace-chat',
86+
model: 'test-model',
87+
eventKey,
88+
cost,
89+
metadata: { inputTokens: 10, outputTokens: 5 },
90+
}
91+
}
92+
93+
async function ledgerRows() {
94+
if (!connection) throw new Error('PostgreSQL fixture is unavailable')
95+
return connection<{ event_key: string; cost: string }[]>`
96+
select event_key, cost from usage_log order by event_key
97+
`
98+
}
99+
100+
afterAll(async () => {
101+
nextPause?.release.resolve()
102+
if (connection) {
103+
await connection.unsafe(`DROP SCHEMA IF EXISTS "${schemaName}" CASCADE`)
104+
await connection.end()
105+
}
106+
})
107+
108+
describe.skipIf(!databaseUrl)('Cumulative billing with PostgreSQL', () => {
109+
beforeAll(async () => {
110+
if (!connection || !database) throw new Error('PostgreSQL fixture is unavailable')
111+
const [version] = await connection`
112+
select current_setting('server_version_num')::integer as version,
113+
current_setting('transaction_timeout', true) is not null as has_transaction_timeout
114+
`
115+
expect(version.version).toBeGreaterThanOrEqual(150000)
116+
holderTimeoutSetting = version.has_transaction_timeout
117+
? 'transaction_timeout'
118+
: 'idle_in_transaction_session_timeout'
119+
await connection.unsafe(`CREATE SCHEMA "${schemaName}"`)
120+
await connection.unsafe(`
121+
CREATE TABLE usage_log (
122+
id text PRIMARY KEY, user_id text NOT NULL, category text NOT NULL,
123+
source text NOT NULL, description text NOT NULL, metadata jsonb,
124+
cost numeric NOT NULL, event_key text, billing_entity_type text,
125+
billing_entity_id text, billing_period_start timestamp, billing_period_end timestamp,
126+
workspace_id text, workflow_id text, execution_id text,
127+
created_at timestamp NOT NULL DEFAULT now()
128+
);
129+
CREATE UNIQUE INDEX usage_log_event_key_unique ON usage_log(event_key)
130+
WHERE event_key IS NOT NULL;
131+
CREATE TABLE driver_probe (id text PRIMARY KEY)
132+
`)
133+
transaction.mockImplementation(async (callback: (tx: Transaction) => Promise<unknown>) => {
134+
const pause = nextPause
135+
nextPause = undefined
136+
return database.transaction(async (tx) => {
137+
const result = await callback(tx)
138+
if (pause) {
139+
if (pause.lockOnly) {
140+
/** Reproduce the old policy: lock_timeout does not expire an idle holder. */
141+
await tx.execute(sql`select set_config(${holderTimeoutSetting}, '0', true)`)
142+
}
143+
pause.reached.resolve()
144+
await pause.release.promise
145+
}
146+
return result
147+
})
148+
})
149+
})
150+
151+
beforeEach(async () => {
152+
nextPause = undefined
153+
if (!connection) throw new Error('PostgreSQL fixture is unavailable')
154+
await connection`truncate usage_log`
155+
})
156+
157+
it.each([
158+
{ name: 'ESM', create: postgres },
159+
{ name: 'CommonJS', create: commonJsPostgres },
160+
])(
161+
'rejects resumed $name transaction queries after its connection is reused',
162+
async ({ create }) => {
163+
const pool = create(databaseUrl!, {
164+
max: 1,
165+
prepare: false,
166+
fetch_types: false,
167+
connection: { search_path: schemaName },
168+
})
169+
const release = deferred()
170+
const resumed = deferred()
171+
let resumedError: unknown
172+
const holder = pool.begin(async (tx) => {
173+
await tx`select set_config(${holderTimeoutSetting}, '150ms', true)`
174+
await tx`select 1`
175+
await release.promise
176+
try {
177+
await tx`insert into driver_probe (id) values (${generateId()})`
178+
} catch (error) {
179+
resumedError = error
180+
} finally {
181+
resumed.resolve()
182+
}
183+
})
184+
try {
185+
await expect(holder).rejects.toMatchObject({ code: 'CONNECTION_CLOSED' })
186+
/** max: 1 forces the underlying connection object to serve a new session. */
187+
await pool`select 1`
188+
release.resolve()
189+
await resumed.promise
190+
expect(getPostgresErrorCode(resumedError)).toBe('CONNECTION_CLOSED')
191+
const [row] = await pool`select count(*)::integer as count from driver_probe`
192+
expect(row.count).toBe(0)
193+
} finally {
194+
release.resolve()
195+
await pool.end({ timeout: 0 })
196+
}
197+
}
198+
)
199+
200+
it('reproduces a retry timing out behind an idle holder when only lock waits are bounded', async () => {
201+
const pause = pauseNextTransaction(true)
202+
const holder = recordCumulativeUsage(usage(0.4))
203+
try {
204+
await pause.reached.promise
205+
const failure = await recordCumulativeUsage(usage(0.8)).catch((error: unknown) => error)
206+
expect(getPostgresErrorCode(failure)).toBe('55P03')
207+
expect(await ledgerRows()).toEqual([])
208+
} finally {
209+
pause.release.resolve()
210+
await holder
211+
}
212+
expect(await ledgerRows()).toEqual([{ event_key: usage(0).eventKey, cost: '0.4' }])
213+
}, 10_000)
214+
215+
it.each([0, 0.2])(
216+
'expires the idle holder, rolls back its write, and recovers exactly once (initial %s)',
217+
async (initial) => {
218+
if (initial > 0) await recordCumulativeUsage(usage(initial))
219+
const pause = pauseNextTransaction()
220+
const holder = recordCumulativeUsage(usage(0.4)).catch((error: unknown) => error)
221+
try {
222+
await pause.reached.promise
223+
const failure = await recordCumulativeUsage(usage(0.8)).catch((error: unknown) => error)
224+
expect(getPostgresErrorCode(failure)).toBe('55P03')
225+
const recovered = await recordCumulativeUsage(usage(0.8))
226+
expect(recovered.billed).toBe(true)
227+
expect(recovered.delta).toBeCloseTo(0.8 - initial, 9)
228+
expect(recovered.total).toBe(0.8)
229+
expect(await recordCumulativeUsage(usage(0.8))).toEqual({
230+
billed: false,
231+
delta: 0,
232+
total: 0.8,
233+
})
234+
expect(await recordCumulativeUsage(usage(0.3))).toEqual({
235+
billed: false,
236+
delta: 0,
237+
total: 0.8,
238+
})
239+
expect(await ledgerRows()).toEqual([{ event_key: usage(0).eventKey, cost: '0.8' }])
240+
} finally {
241+
pause.release.resolve()
242+
}
243+
expect(getPostgresErrorCode(await holder)).toBe('CONNECTION_CLOSED')
244+
},
245+
12_000
246+
)
247+
248+
it('converges concurrent out-of-order callbacks and independent events to their exact totals', async () => {
249+
const costs = [0.4, 0.1, 0.8, 0.3, 0.8, 0.6]
250+
const results = await Promise.all(costs.map((cost) => recordCumulativeUsage(usage(cost))))
251+
expect(results.reduce((total, result) => total + result.delta, 0)).toBeCloseTo(0.8, 9)
252+
await Promise.all(
253+
Array.from({ length: 32 }, (_, index) =>
254+
recordCumulativeUsage(usage(0.25, `independent:${index}`))
255+
)
256+
)
257+
expect(await ledgerRows()).toHaveLength(33)
258+
expect(await recordCumulativeUsage(usage(0.8))).toEqual({ billed: false, delta: 0, total: 0.8 })
259+
})
260+
261+
it.each([0.2, 0.8])(
262+
'rejects an actor mismatch even for a non-increasing callback (%s)',
263+
async (cost) => {
264+
await recordCumulativeUsage(usage(0.8))
265+
await expect(
266+
recordCumulativeUsage({ ...usage(cost), userId: 'another-actor' })
267+
).rejects.toBeInstanceOf(CumulativeUsageContextMismatchError)
268+
expect(await ledgerRows()).toEqual([{ event_key: usage(0).eventKey, cost: '0.8' }])
269+
}
270+
)
271+
})

apps/sim/lib/billing/core/usage-log.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ describe('recordCumulativeUsage', () => {
429429
})
430430
})
431431

432-
it('bounds the advisory-lock wait and locks on the 64-bit event-key hash', async () => {
432+
it('bounds the holder lifetime and lock wait before acquiring the event-key lock', async () => {
433433
const { tx } = setupTx({ id: 'row-1', cost: '0.3474447' })
434434
await recordCumulativeUsage({
435435
userId: 'user-1',
@@ -438,7 +438,11 @@ describe('recordCumulativeUsage', () => {
438438
cost: 0.4662453,
439439
eventKey: 'update-cost:msg-1-billing',
440440
})
441+
expect(executedSqlContaining(tx, 'transaction_timeout')).toBe(true)
442+
expect(executedSqlContaining(tx, 'idle_in_transaction_session_timeout')).toBe(true)
443+
expect(executedSqlContaining(tx, 'statement_timeout')).toBe(true)
441444
expect(executedSqlContaining(tx, 'lock_timeout')).toBe(true)
445+
expect(tx.execute.mock.calls[0][0]).toMatchObject({ values: ['4000ms', '3500ms', '3000ms'] })
442446
expect(executedSqlContaining(tx, 'pg_advisory_xact_lock')).toBe(true)
443447
expect(executedSqlContaining(tx, 'hashtextextended')).toBe(true)
444448
})

0 commit comments

Comments
 (0)