Repository navigation
Description
On Postgres, when two tenants insert into the same autonumber-bearing object for the first time concurrently, the whole batch fails with 25P02 current transaction is aborted. The counters are advanced anyway, so the numbers the failed attempt reserved are lost — a permanent gap at the start of both tenants' sequences.
Single-tenant concurrency is fine at every level I tried. The tenant boundary is what breaks it, which makes this specific to multi-org deployments — and the EE stack runs Postgres.
Measured
Driver-level probe, SqlDriver on postgres:16, object { organization_id, code: autonumber 'TK-{0000}', name }, one Promise.all burst per case, each case on a fresh driver and a fresh table:
| case | total inserts | result |
|---|---|---|
| 1 tenant × 2 / 4 / 6 / 8 / 12 | up to 12 | OK — TK-0001…TK-0012, contiguous, no duplicates |
| 2 tenants × 2 | 4 | FAILED 25P02 |
| 2 tenants × 4 | 8 | FAILED 25P02 |
| 2 tenants × 6 | 12 | FAILED 25P02 |
2 tenants × 6, pool max: 20 |
12 | FAILED 25P02 — not pool exhaustion |
| 2 tenants, strictly serial | 8 | OK — A:TK-0001 B:TK-0001 A:TK-0002 B:TK-0002 … |
The failing statement is the sequence reservation itself:
select * from "_objectstack_sequences" where "key_hash" = $1 limit $2 for update
- current transaction is aborted, commands ignored until end of transaction block
Cold vs warm is the discriminator. Seed one serial row per tenant first (so both counter rows exist), then run the exact burst that failed cold:
[W] warm-up => orgA:TK-0001 orgB:TK-0001
[W] counter rows now => [{"tenant_id":"orgA","last_value":"1"},{"tenant_id":"orgB","last_value":"1"}]
[W] warm concurrent => OK orgA=TK-0002..TK-0005 orgB=TK-0002..TK-0005
[W] cold concurrent => FAILED code=25P02
Retry "recovers" but loses numbers. Three successive bursts on a cold object:
[R] attempt 1 => FAILED code=25P02
[R] counters after 1 => [{"tenant_id":"orgA","last_value":"3"},{"tenant_id":"orgB","last_value":"1"}]
[R] attempt 2 => OK orgA:TK-0004 TK-0005 TK-0006 orgB:TK-0002 TK-0003 TK-0004
[R] attempt 3 => OK orgA:TK-0007 TK-0008 TK-0009 orgB:TK-0005 TK-0006 TK-0007
TK-0001…TK-0003 (orgA) and TK-0001 (orgB) exist in no row — the counters were advanced by the attempt whose transaction then aborted. So the user-visible story is: "creating the first records failed; I retried, it worked, and my numbering starts at 0004."
SQLite is unaffected — the identical cold cross-tenant burst succeeds:
[R] sqlite cold burst => OK orgA:TK-0001 TK-0002 TK-0003 orgB:TK-0001 TK-0002 TK-0003
which is why the existing autonumber suite (sqlite-backed) does not catch it.
Cause
reserveSequenceValue handles the first-insert race by catching the unique violation and continuing inside the same transaction (packages/drivers/driver-sql/src/sql-driver.ts, ~line 4209):
try {
await trx(SEQUENCES_TABLE).insert({ ...insertRow, last_value: initial });
return initial;
} catch (err) {
// Another writer raced us to the first INSERT. Fall through to
// the UPDATE path with the now-present row.
existing = await trx(SEQUENCES_TABLE).where(key).forUpdate().first(); // ← 25P02 here
if (!existing) throw err;
}In Postgres any statement error poisons the whole transaction: every subsequent statement returns 25P02 until rollback. So this recovery path can never run on Postgres — the SELECT … FOR UPDATE in the catch is itself the statement that raises the error we observe. The pattern is only valid on SQLite/MySQL, where a statement error does not abort the transaction. The comment a few lines above (Postgres/MySQL behave normally here) is the assumption that does not hold.
Suggested fix
Make the first-insert race dialect-safe, e.g.
- wrap the speculative INSERT in a
SAVEPOINTand roll back to it before falling through, or - do the insert as
INSERT … ON CONFLICT DO NOTHING(the driver already usesonConflictatsql-driver.ts:4596) and then re-select, so no statement ever errors.
A regression test needs to be Postgres-backed and cross-tenant + cold + concurrent — the three conditions together. Single-tenant or warm variants pass regardless.
Environment
Reproduced against postgres:16 in Docker, via SqlDriver directly (branch claude/multi-org-service-testing-a75937, deps built from the current worktree). Same shape as the objectos-ee-deploy stack serving http://localhost:8080, which is single-database with organization_id partitioning. The probe files were temporary and are not part of this report; the tables above are the full output.
Activity
baozhoutao commentedon Aug 13, 2026
Found while testing autonumber behaviour on the same multi-org deployment as #8090–#8094, #8095, #8096.
os-zhuang commentedon Aug 13, 2026
Claim: PM loop, drivers lane
Session:
session_01VoxQqG5FiUHZKCST7KDoZCBranch:
claude/issue-8269-autonumber-cold-race-25p02Worktree:
objectstack-issue-8269Domain:
domain:driversFile surface:
packages/drivers/driver-sql/src/sql-driver.ts+ a new Postgres-backed test inpackages/drivers/driver-sql/src/+.changeset/(stop on breach; explain in the report)Container & model:
M,mode:subagent,model: opusSerial constraints cleared:
sql-driver.tslast moved by #8198 (a5dcb74) and #8234 (7e5af5c), both MERGED; no open drivers card touches this file — #8138 is a measurement card onsql-driver-overlay-index-drift.test.ts, disjoint.pm:*state label present — this card carriedbug+priority:p1+domain:driversand no pm-state. I treated thedomain:*label as the routing authority and did not add or alterdomain:*,priority:*or the type (triage's fields — I added onlypm:dispatched, the executing seat's own state marker). A p1 data-integrity defect with a located cause and a live customer surface should not wait a sweep cycle for a queue label. If triage wanted this held, say so and I yield.Premise verified on
origin/main@021d8ddbefore dispatch — live, with three correctionsThe defect reproduces in the code as described, but the report's landmarks have drifted (
mainmoved after the reporting branch):getNextSequenceValue(sql-driver.ts:4305), notreserveSequenceValue— that name does not exist in this file. Line ~4209 is now ~4373.Postgres/MySQL behave normally herecomment is on a different try/catch than the one that breaks. It sits at:4353, on theforUpdate()fallback; the first-insert race the report measured is at:4373.:4353doestry { …forUpdate().first() } catch { …first() }— on Postgres that fallbackSELECTruns inside the same poisoned transaction and would raise25P02for exactly the same reason. It is a rarer trigger (it needsforUpdate()itself to throw), so it is not what the report captured, but a fix that only patches:4373leaves the same bug behind a narrower door.The report's analysis is otherwise correct and its measurement is the good kind — the cold/warm discriminator and the sqlite-passes/postgres-fails split are exactly what explain why the existing autonumber suite never caught this.
Generated by Claude Code
hotlong commentedon Aug 13, 2026
Triage: lands in
packages/drivers/driver-sql(sql-driver.tsfirst-insert race handler — anchors re-verified onorigin/main, the speculative-INSERT catch +forUpdate().first()fall-through at lines 4354/4377–4379) ⇒domain:driversconfirmed; not in the driver-memory/mongodb investment freeze.pm:queue, type Bug.target:v17per the release-board binary test, class ① (published-surface defect users hit today): measured on the EE multi-org deploy — the first concurrent cross-tenant autonumber insert on Postgres fails25P02and permanently burns reserved sequence numbers, and Postgres + multi-org is exactly the EE stack's shape.Removed
priority:p1: the p1–p5 ladder is retired (onlypriority:p0has a reader — the jump-the-queue rule; a p1 label has none). Urgency is carried by the release board plus the Bug-first tie-break; nothing about this card's severity is lost by the removal.For the drivers seat at claim time: serial-check vs in-flight #8138 (same lane; expected file-disjoint — verify, don't assume). The regression test must be Postgres-backed AND cross-tenant AND cold AND concurrent — the card measures that dropping any one of the three conditions goes green regardless. Size/model suggestion: M,
mode:subagent, opus.Generated by Claude Code
os-zhuang commentedon Aug 13, 2026
{ "issue": 8269, "status": "done", "branch": "claude/issue-8269-autonumber-cold-race-25p02", "pr": "https://github.com/objectstack-ai/objectstack/pull/8279", "premise_still_valid": true, "summary": "Reproduced the defect on a live postgres:16 (started locally from the system PG16 binaries — no docker daemon in this container — at TZ Asia/Shanghai to mirror the CI job): the exact 25P02 on the exact statement the card names. Both catch-inside-transaction sites in getNextSequenceValue now run their speculative statement under a SAVEPOINT via a new protected helper attemptWithoutPoisoning (a knex nested transaction), so a failed attempt leaves the surrounding transaction usable on every dialect and the first-insert race handler that was written for this case actually runs. Chose SAVEPOINT over ON CONFLICT DO NOTHING on measured evidence (below), not on ordering. ISSUE BODY IS UNDAMAGED — all five code fences balanced, no empty inline-code spans, no '<'+letter truncation; the only body defects are the landmark drift the PM already corrected. ONE REFINEMENT TO THE CARD: single-tenant cold concurrent is NOT safe, only flakier — measured 5 rounds each before the fix, 1 tenant x2 cold 0/5 failed, x4 1/5, x6 3/5, x12 2/5, all 25P02. The tenant boundary is an amplifier (two cold counter rows make the window near-certain), not a precondition, so single-org deployments were exposed too and the card's 'specific to multi-org deployments' framing understates the blast radius. The guard is still written cross-tenant because that shape fails deterministically.", "tests": "NEW packages/drivers/driver-sql/src/sql-driver-autonumber-cold-race.test.ts — Postgres-backed AND cross-tenant AND cold AND concurrent, all four, reusing the existing harness (live-dialect-matrix.testkit.ts + OS_TEST_POSTGRES_URL, the Temporal Conformance job's mechanism); no second PG mechanism invented. An unprovisioned pg cell is a reported skip that becomes a hard failure under OS_EXPECT_LIVE_DIALECT_MATRIX=1. A sqlite cell runs the same burst as an explicitly-labelled CONTROL, not as coverage. || FULL SUITE vs live PG16: 'Test Files 91 passed | 1 skipped (92) / Tests 1629 passed | 31 skipped (1660)', 0 failed; 'pnpm --filter @objectstack/driver-sql typecheck' -> tsc --noEmit clean. Test-file count 91 on origin/main -> 92 on branch, so the new file is inside that count. || REVERSE VERIFICATION, direction predicted BEFORE running (fix committed first as 92cd69a68, reverted only sql-driver.ts via git checkout origin/main -- <path>, restored via git checkout <branch> -- <path> to a byte-identical tree, empty git diff HEAD). Predicted: 2 pg tests red with 25P02; the mechanism test red DIFFERENTLY (TypeError, helper absent); 2 sqlite controls STAY GREEN. Observed exactly that — 'Tests 3 failed | 2 passed (5)': 'error: select * from \"_objectstack_sequences\" where \"key_hash\" = $1 limit $2 for update - current transaction is aborted, commands ignored until end of transaction block' and 'TypeError: driver.attemptWithoutPoisoning is not a function', with both sqlite rows green. The sqlite pair staying green is the load-bearing half: it proves the control is a control and not a second copy of the guard. || REPRO BEFORE/AFTER: before — {single:false, cross2:false, cross4:false, cross6:false, serial:true, warm:true}; after — all six true with contiguous TK-0001..TK-000N per tenant and counters exactly N. || GATES, derived at implementation time from my ACTUAL changed paths (node scripts/pm/dispatch-gates.mjs on the 3 files): check:changeset-gate-self-tests OK (117 assertions), check:driver-conformance OK (45 covered cells, 0 debt), check:objectui-changeset OK, check:test-source-alias OK (72 pkgs), check:type-source-resolution OK (76 pkgs), check-changeset-no-major OK ('introduces no major bump'), check:nul-bytes OK (7507 files, plus a manual grep -naP control-byte self-scan of both new files, clean), plus the two convention-triggered families the script named for adding a test file: check:query-options-erasure OK ('baseline key set verified against 021d8dd: no files added') and check:type-check-coverage OK. TWO REDS I DID NOT CAUSE: (1) check:objectui-pin-fresh — 'objectui pin is stale, refresh .objectui-sha' — my diff is 3 files and does not touch .objectui-sha (git diff origin/main -- .objectui-sha is empty), it only path-matched via .changeset/; pre-existing on main. (2) check-dev-prereqs.mjs exit 1 — '60 of 67 workspace packages declare an entry point under dist/ that is not on disk', an artifact of my deliberately scoped build (driver-sql + its closure only, per resource discipline), not a code defect; CI builds the workspace. Also: with OS_EXPECT_LIVE_DIALECT_MATRIX=1 set locally, 6 PRE-EXISTING files fail solely on 'OS_TEST_MYSQL_URL is unset' — no MySQL server is reachable in this container (no docker daemon); my new file is not among them and declares no mysql cell.", "open_questions": [], "out_of_scope_findings": [ "NOT filed as a new card — the reserved-number gap on a failed attempt is already described in #8269's own body, and the dispatch explicitly scoped it out; filing it separately would duplicate the card I was working. Recording it here instead: the counters advance in their own committed transaction (runner.transaction over parentTrx ?? this.knex) and are never rolled back with the caller's insert. I did not change that, and after this fix the cold-race path no longer produces failed attempts, so the reported symptom ('my numbering starts at 0004') disappears in practice. If the PM wants non-rollback treated as a defect in its own right rather than ordinary sequence semantics, that is a fresh card and I can file it on request.", "Recorded, not filed (needs a maintainer's read first, and it is a comment change in the file I just touched): sql-driver.ts's forUpdate() fallback carried the comment 'Some dialects/versions reject .forUpdate() on a missing row in weird ways... Postgres/MySQL behave normally here'. Measured on PG16: forUpdate() on a missing row does NOT throw, it returns zero rows — so that catch was unreachable for its documented reason on Postgres. It IS reachable for lock-level failures (40P01 deadlock, 55P03/57014 timeouts), where the fallback read was masking a real error as an uninformative 25P02. I corrected the comment in-place as part of this fix rather than filing it, since leaving a comment I had just proven false next to the code I was changing would have been worse." ] }Mechanism assumptions — all three measured on live PG16
:4353is the same bug classforUpdate()on a missing row returns zero rows, no throw — the catch is dead for the reason its comment gives. It is live for40P01/55P03/57014, and the fallback read then produced25P02, masking the real error. Same consequence, different door — worth fixing, not just commenting.ON CONFLICT DO NOTHINGis available herekey_hashPK and on the legacy 3-column PK, butON CONFLICT (object, tenant_id, field)raises42P10 there is no unique or exclusion constraint matching the ON CONFLICT specificationagainst an interim table whose PK is the four columns(object, tenant_id, field, scope)— exactly what a failedensureSequencesKeyHashShapemigration leaves behind, which is whensequencesHasKeyHashis false and that 3-column key is used.42P10: the second site is aSELECT … FOR UPDATEand there is noON CONFLICTfor a read, soON CONFLICTcannot cover both sites.ON CONFLICT DO NOTHINGalso affects zero rows silently, losing the ability to tell "another writer raced me" from any other rejection — the helper returns a discriminated result so the original error survives. Savepoint verified on PG16 and on better-sqlite3 at poolmax: 1, where it rides the parent's connection and never asks for a second one (the #4250 deadlock).Changeset:
patch, on the discriminator supplied. This change refuses no shape and removes no information a caller legitimately had — it makes an operation that should have worked start working, adds no API surface and changes no contract, so it sits below both rungs (theminorrung is about removal).scripts/check-changeset-no-major.mjsconfirmsmajoris barred in the launch window andcheck-changeset-no-major --base origin/mainreports "This diff introduces nomajorbump".Generated by Claude Code
Generated by Claude Code
os-zhuang commentedon Aug 13, 2026
ACCEPT the implementation — holding the ready-flip until CI converges
Reviewed against the branch and
origin/main, ⛔ not against the report's self-description. PR #8279 @92cd69a, 3 files, +340/−15.Verified
attemptWithoutPoisoninghelper (a knex nested transaction =SAVEPOINT, rolled back to on failure). That was the part I flagged as mandatory, and it holds.ON CONFLICT DO NOTHINGalternative was rejected on evidence, not on ordering — and the reason is sharper than my suggestion deserved:ON CONFLICT (object, tenant_id, field)(the legacy key shape whensequencesHasKeyHashis false) raises42P10against an interim table whose primary key is the four columns includingscope— which is exactly what a failedensureSequencesKeyHashShapemigration leaves behind. That would have swapped this bug for a worse one on the deployments least able to absorb it. It also notesON CONFLICThas no form for the second site, which is aSELECT … FOR UPDATE.beforeEachdrops the data table and deletes the counter rows (cold is real, not assumed); two tenants;Promise.allinterleaved; the Postgres cell runs via the existinglive-dialect-matrix.testkit.ts+OS_TEST_POSTGRES_URLharness — the Temporal Conformance mechanism, reused rather than duplicated — withdeclareUnprovisionedCellwhen absent. The assertion is a contiguousTK-0001…TK-0006band per tenant, so it catches both the25P02failure and the burned-number gap.sql-driver.tsalone reverted: 2 pg rows red with the25P02text, the mechanism test red differently (TypeError: attemptWithoutPoisoning is not a function), and both sqlite rows green. The sqlite pair staying green is the load-bearing half.patchis correct. I checked whether my fix(driver-sql,driver-turso): a cross-field$fieldrefusal stops naming the two columns it compared (#7929, #7988) #8198 argument transfers and it does not:index.tsis untouched andattemptWithoutPoisoningis a protected class member, so there is no new public export, and the behaviour change is "a failure stops happening" rather than information removed from every caller.I told the dev that
:4353was the same bug class because "the fallback SELECT would also hit 25P02", flagged as an unverified inference. Measured on PG16:.forUpdate()on a missing row does not throw at all — it returns zero rows — so that catch was unreachable for the reason its comment gave. It is reachable for lock-level failures (40P01deadlock,55P03/57014timeouts), and there the fallback read was converting a real error into an uninformative25P02. Same conclusion (the site needed fixing), different and better-evidenced reason. The dev corrected the false comment in place rather than leaving it next to code it had just disproved — right call.This card states "Single-tenant concurrency is fine at every level I tried" and scopes the defect to multi-org deployments. Measured, 5 rounds per level before the fix: 1 tenant × 2 → 0/5 failed, × 4 → 1/5, × 6 → 3/5, × 12 → 2/5, all
25P02. The tenant boundary is an amplifier, not a precondition — two cold counter rows make the window near-certain, but a single-org deployment inserting concurrently into a cold autonumber object was exposed too, just flakily. Single-org deployments on Postgres were hitting this as an intermittent first-insert failure.⛔ I am not touching
priority:*— that is triage's field — but flagging it explicitly: the p1 grade was set on a multi-org-only reading that measurement has now widened. The regression guard is still written cross-tenant, correctly, because that shape fails deterministically.Accepted deviation
I instructed the dev to file a card for the reserved-number gap on a failed attempt. It recorded it in the report instead, reasoning that #8269's own body already describes it, that the dispatch scoped it out, and that after this fix the cold-race path stops producing failed attempts so the reported symptom disappears in practice. That reasoning is sound and a duplicate card would have been noise — accepted. The underlying behaviour (counters commit in their own transaction and never roll back with the caller's insert) is unchanged and remains ordinary sequence semantics unless someone rules otherwise.
Next: CI is still converging (23 checks, several
in_progressat time of writing). ⛔ No ready-flip on anin_progressread. Once every gate job readscompleted: success→ ready → merge queue → follow to MERGED. Two reds the dev reported locally are expected to be absent in CI:check:objectui-pin-freshis ambient (it path-matches via.changeset/and fires for any PR carrying one — confirmed ambient on #8198 earlier today), andcheck-dev-prereqs.mjswas an artifact of a deliberately scoped local build.Generated by Claude Code
os-zhuang commentedon Aug 13, 2026
PR #8279 → ready, queued. Head
92cd69a, all 25 check runscompleted+success/skipped(job-level conclusions), path surface clean (.changeset/+ twodriver-sqlsource files),mergeable_state: clean,Fixes #8269intact.Temporal Conformance (live PG + MySQL)went green at 02:22:58Z — that is the job the new Postgres cell rides on, so the regression guard actually executed against a live PG rather than being skipped into a false pass. The two reds the dev reported from its local run (check:objectui-pin-fresh,check-dev-prereqs.mjs) have no corresponding CI job here, confirming both were local artifacts as it said.The card's stated scope — "Single-tenant concurrency is fine at every level I tried", hence multi-org-specific — is refuted by measurement. Single tenant, cold, concurrent, 5 rounds per level before the fix: ×2 → 0/5 failed, ×4 → 1/5, ×6 → 3/5, ×12 → 2/5, every failure the same
25P02. The tenant boundary makes the window near-certain but is not a precondition, so single-org Postgres deployments were also hitting this, as an intermittent first-insert failure with a permanent gap in the numbering afterwards.⛔ I have not touched
priority:*— that is triage's field. Flagging it because thep1grade was assigned against the narrower reading, and the fix has now landed regardless; the point is the record, not the queue position.Generated by Claude Code
baozhoutao commentedon Aug 13, 2026
The everyday version of the same mechanism is recorded separately in the finding on burned numbers.
fix(driver-sql): run speculative sequence statements under a savepoint (
fix(driver-sql): stamp `updated_at` when the driver never ran DDL (#1…
fix(driver-sql): stamp `updated_at` when the driver never ran DDL (ob…