Skip to content

Commit 7ffbda7

Browse files
committed
fix(billing): retain older PostgreSQL timeout compatibility
1 parent b0dc327 commit 7ffbda7

5 files changed

Lines changed: 56 additions & 17 deletions

File tree

.github/workflows/test-build.yml

Lines changed: 20 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
@@ -89,6 +102,13 @@ jobs:
89102
BILLING_USAGE_TEST_DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5432/sim_auth_scim
90103
run: bunx vitest run lib/billing/core/usage-log.postgres.test.ts
91104

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+
92112
- name: Verify SCIM and administration over real HTTP
93113
working-directory: apps/sim
94114
env:

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* @vitest-environment node
33
*
4-
* Uses a disposable schema in local PostgreSQL 17+. The paused callback models
4+
* Uses a disposable schema in local PostgreSQL 15+. The paused callback models
55
* a client that stops progressing after writing usage but before COMMIT; the
66
* database must release its locks without waiting for that client to resume.
77
*/
@@ -65,6 +65,7 @@ interface PausedTransaction {
6565
}
6666

6767
let nextPause: PausedTransaction | undefined
68+
let holderTimeoutSetting = 'transaction_timeout'
6869

6970
function pauseNextTransaction(lockOnly = false): PausedTransaction {
7071
const pause = { reached: deferred(), release: deferred(), lockOnly }
@@ -107,9 +108,14 @@ afterAll(async () => {
107108
describe.skipIf(!databaseUrl)('Cumulative billing with PostgreSQL', () => {
108109
beforeAll(async () => {
109110
if (!connection || !database) throw new Error('PostgreSQL fixture is unavailable')
110-
const [version] =
111-
await connection`select current_setting('server_version_num')::integer as version`
112-
expect(version.version).toBeGreaterThanOrEqual(170000)
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'
113119
await connection.unsafe(`CREATE SCHEMA "${schemaName}"`)
114120
await connection.unsafe(`
115121
CREATE TABLE usage_log (
@@ -132,7 +138,7 @@ describe.skipIf(!databaseUrl)('Cumulative billing with PostgreSQL', () => {
132138
if (pause) {
133139
if (pause.lockOnly) {
134140
/** Reproduce the old policy: lock_timeout does not expire an idle holder. */
135-
await tx.execute(sql`select set_config('transaction_timeout', '0', true)`)
141+
await tx.execute(sql`select set_config(${holderTimeoutSetting}, '0', true)`)
136142
}
137143
pause.reached.resolve()
138144
await pause.release.promise
@@ -164,7 +170,7 @@ describe.skipIf(!databaseUrl)('Cumulative billing with PostgreSQL', () => {
164170
const resumed = deferred()
165171
let resumedError: unknown
166172
const holder = pool.begin(async (tx) => {
167-
await tx`select set_config('transaction_timeout', '150ms', true)`
173+
await tx`select set_config(${holderTimeoutSetting}, '150ms', true)`
168174
await tx`select 1`
169175
await release.promise
170176
try {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,7 @@ describe('recordCumulativeUsage', () => {
439439
eventKey: 'update-cost:msg-1-billing',
440440
})
441441
expect(executedSqlContaining(tx, 'transaction_timeout')).toBe(true)
442+
expect(executedSqlContaining(tx, 'idle_in_transaction_session_timeout')).toBe(true)
442443
expect(executedSqlContaining(tx, 'statement_timeout')).toBe(true)
443444
expect(executedSqlContaining(tx, 'lock_timeout')).toBe(true)
444445
expect(tx.execute.mock.calls[0][0]).toMatchObject({ values: ['4000ms', '3500ms', '3000ms'] })

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

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -635,10 +635,11 @@ function assertCumulativeUsageLedgerBinding(
635635
}
636636

637637
/**
638-
* PostgreSQL 17 bounds the holder's entire transaction, including time spent
639-
* waiting for the application between statements. A lock timeout alone only
640-
* bounds other callers waiting behind that holder. Keep the transaction below
641-
* the billing callback's five-second client deadline so retries can recover.
638+
* PostgreSQL 17+ bounds the entire transaction below the callback's five-second
639+
* deadline. Older supported servers instead bound each idle interval between
640+
* statements, alongside the per-statement budget. Both policies release an idle
641+
* lock holder without waiting for its application process to resume; only the
642+
* newer policy also limits total elapsed transaction time.
642643
*/
643644
const CUMULATIVE_FLUSH_TRANSACTION_TIMEOUT_MS = 4_000
644645
const CUMULATIVE_FLUSH_STATEMENT_TIMEOUT_MS = 3_500
@@ -658,8 +659,8 @@ type CumulativeUsageStage = 'pool' | 'configure' | 'lock' | 'read' | 'write' | '
658659
* An existing row must match the incoming actor, workspace, payer, and billing
659660
* period before either a duplicate no-op or a top-up is accepted.
660661
* The billing context is resolved BEFORE the transaction and the lock wait is
661-
* bounded by `lock_timeout`. A server-enforced `transaction_timeout` releases
662-
* the holder even when its application process stops making progress. The
662+
* bounded by `lock_timeout`. A server-enforced transaction deadline, or idle
663+
* transaction deadline on older PostgreSQL, releases a stalled holder. The
663664
* critical section uses one SELECT plus one INSERT/UPDATE on a single connection.
664665
*
665666
* Because every leg flushes its cumulative and this converges to the max,
@@ -706,7 +707,14 @@ export async function recordCumulativeUsage(
706707
enterStage('configure')
707708
await tx.execute(sql`
708709
select
709-
set_config('transaction_timeout', ${`${CUMULATIVE_FLUSH_TRANSACTION_TIMEOUT_MS}ms`}, true),
710+
set_config(
711+
case when current_setting('transaction_timeout', true) is null
712+
then 'idle_in_transaction_session_timeout'
713+
else 'transaction_timeout'
714+
end,
715+
${`${CUMULATIVE_FLUSH_TRANSACTION_TIMEOUT_MS}ms`},
716+
true
717+
),
710718
set_config('statement_timeout', ${`${CUMULATIVE_FLUSH_STATEMENT_TIMEOUT_MS}ms`}, true),
711719
set_config('lock_timeout', ${`${CUMULATIVE_FLUSH_LOCK_TIMEOUT_MS}ms`}, true)
712720
`)

patches/README.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,16 @@ including its implicit COMMIT/ROLLBACK, before they reach the connection or pool
3232
The guard is applied to all published ESM, CommonJS, and Cloudflare entry points.
3333
It does not change connection establishment, retries, or healthy transactions.
3434

35-
This is required for cumulative billing's PostgreSQL 17 `transaction_timeout`:
36-
the database can release an idle lock holder without waiting for the application.
35+
This is required for cumulative billing's server-enforced holder deadline:
36+
PostgreSQL 17+ uses `transaction_timeout`; older supported servers use
37+
`idle_in_transaction_session_timeout` alongside the statement timeout. Both release
38+
a stalled idle holder; the older fallback limits each idle interval and statement,
39+
not the total elapsed transaction time.
3740
`apps/sim/lib/billing/core/usage-log.postgres.test.ts` tests real ESM/CommonJS driver
3841
closure and reconnection, rollback after billing INSERT/UPDATE, and exact retry
39-
accounting. CI runs it against PostgreSQL 17. Set `BILLING_USAGE_TEST_DATABASE_URL`
40-
to a disposable local PostgreSQL 17+ database to run it manually.
42+
accounting. CI runs it against PostgreSQL 17 and 16. Set
43+
`BILLING_USAGE_TEST_DATABASE_URL` to a disposable local PostgreSQL 15+ database to
44+
run it manually.
4145

4246
Remove this patch when the pinned driver includes equivalent transaction-scope
4347
closure handling. Keep the reconnect regression tests when upgrading.

0 commit comments

Comments
 (0)