fix(platform): claim message slots until a deadline, not a fixed count - #3267
Merged
Conversation
`appendMessageRow` and `saveMessage` claim `max(order)+1` with `INSERT … ON CONFLICT DO NOTHING` and re-claim after a lost race, but the loop gave up after MESSAGE_SLOT_ATTEMPTS = 8 rounds. Under READ COMMITTED every round has exactly one winner among the appenders racing for a thread, so a burst of N appenders needs up to N rounds for its unluckiest member: any burst larger than eight failed a legitimate append with "no free slot after 8 attempts". The integration lane fires twelve and went red on roughly one proof in eight — read as host-load noise until now. One shared claim helper now runs the claim until it lands a row, bounded by a wall-clock deadline (10 s) instead of a count, with a jittered exponential pause between rounds so the losers of one round do not re-collide in lockstep. Clock and sleep are injectable, so the unit tests exhaust the budget deterministically and prove a 40-loss burst still lands; the integration lane fires 32 appends, deliberately above the old cap.
Review follow-ups on the deadline-bounded slot claim: the backoff now caps at 20 ms instead of 50 ms, so the last member of a 32-way burst waits well under a second in total while the lockstep is still broken; the options parameter is named for what it holds (`slot`, a SlotClaimOptions); the comment states the bound as "at least one winner per round, exactly one in lockstep"; and both appenders' suites lock the contract the SERIALIZABLE callers rely on — an error thrown by the claim (a 40001) propagates unchanged after exactly one insert, never retried or swallowed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Defect
appendMessageRow(domains/chat/store.ts) andsaveMessage(domains/threads/store.ts) claimmax("order") + 1withINSERT … ON CONFLICT (thread_id, "order", step_order) DO NOTHINGand re-claim after a lost race — but onlyMESSAGE_SLOT_ATTEMPTS = 8times (#3197). Under READ COMMITTED every round has exactly one winner among the appenders racing for a thread, so a burst of N appenders needs up to N rounds for its unluckiest member. Any burst larger than eight therefore failed a legitimate append withmessage insert failed: no free slot after 8 attempts— automation fan-out or several agents replying on one thread hits that in production.The integration lane
messages: concurrent appends each take their own slotfires twelve concurrent appends and went red on roughly one proof of main in eight (appends=12, failed=1), most recently on c70872f. It had been read as host-load noise.Fix
One shared helper,
claimMessageSlotindomains/threads/store.ts, runs the claim until it lands a row, bounded by a wall-clock deadline (MESSAGE_SLOT_CLAIM_DEADLINE_MS = 10 s) instead of a count — a count is defeated by any burst larger than itself. Rounds pause with a jittered exponential backoff (1 ms → 50 ms cap) so the losers of one round do not re-collide in lockstep. Clock and sleep are injectable (SlotClaimOptions, an optional trailing parameter on both appenders; production callers are untouched). Under SERIALIZABLE the conflict still surfaces as a 40001 andtransactSerializablereruns the transaction, as before. Nothing else is written while a claim is pending.Tests
chat/append-message.test.ts,threads/store.test.ts: the first-try and re-claim shapes stay; the exhaustion test now spends the budget through an injected clock (deterministic, names… within 10000 ms (3 attempts)); a new case proves a 40-loss burst still lands (41 inserts) — larger than any fixed count.Proof
appends=32, failed=0, rows=32, distinctOrders=32, contiguous=true.vitest --project server: 589 files / 6504 tests passed.tsc --noEmit,oxlint --type-aware,knip:check,oxfmtclean.appends=32, failed=0, rows=32, distinctOrders=32, contiguous=true. (The first run had started before a last cosmetic edit to the backoff base — 2 ms → 1 ms first pause, matching its comment.)Review follow-ups (adversarial review: approve, advisory only) — e344a02
transactSerializablekeeps its rerun.claim→slot(it holdsSlotClaimOptions); the constant's comment now says "at least one winner per round, exactly one in lockstep"; the test header says "while the deadline remains".tasks/comments.tsviatransactSerializable) still carrieswithRetry's 5-attempt bound for its 40001s; if a human comment burst on one task ever exhausts it,markRetryQueueKey(error, threadId)queues the retries behind the per-thread advisory lock — a separate change with its own proof.appends=32, failed=0, rows=32, distinctOrders=32, contiguous=true— the third green proof of this branch.Merged origin/main (4df487e) — e450a1a
Plain merge commit, no conflicts, so the E2E workflow runs under the Bun 1.4.2 pin that #3265 landed (the earlier docs-job reds on this branch were that pin's exit stall, twice, both green on retry). Platform
tsc --noEmitand both appender suites (10/10) green on the merged tree.