Skip to content

fix: keep S3 session parts ordered across writers - #451

Open
cmun2 wants to merge 1 commit into
anthropics:mainfrom
cmun2:fix/s3-session-store-cross-writer-order
Open

cmun2 wants to merge 1 commit into
anthropics:mainfrom
cmun2:fix/s3-session-store-cross-writer-order

Conversation

@cmun2

@cmun2 cmun2 commented Sep 5, 2026

Copy link
Copy Markdown

nextPartName() separated same-millisecond appends by advancing the timestamp past the clock (Math.max(now, this.lastMs + 1)). Those borrowed milliseconds then belong to whoever writes next, so a second instance sharing the bucket stamps a part with a millisecond the first instance had already taken, and its part sorts inside the earlier burst.

const a = new S3SessionStore({ bucket, client })   // two processes mirroring
const b = new S3SessionStore({ bucket, client })   // the same session

// t = T
for (const type of ['a1','a2','a3','a4','a5']) await a.append(KEY, [{ type }])
// t = T+1
await b.append(KEY, [{ type: 'b1' }])
// t = T+2
await a.append(KEY, [{ type: 'a6' }])

await a.load(KEY)
// written: a1 a2 a3 a4 a5 b1 a6
// loaded:  a1 b1 a2 a3 a4 a5 a6

a's burst is named T … T+4, so b's part — stamped with the real T+1 — lands in position two. The rand suffix only breaks ties inside one millisecond; it cannot undo a name that claims a millisecond that has not arrived.

This matters because the transcript is what resume replays. Across 300 appends from two instances, 1747 of 44460 pairs written at different wall-clock times came back inverted:

instances inversions before after
1 0 / 44460 0 / 44460
2 1747 / 44460 0 / 44460
3 383 / 44460 0 / 44460

A single instance was always correct, which is why the conformance suite did not catch it — every case builds one store, and one writer is the case that works.

The fix keeps the timestamp honest and puts the burst counter in its own field:

part-{epochMs13}-{rand6}.jsonl  →  part-{epochMs13}-{seq6}-{rand6}.jsonl

The millisecond orders parts across writers, the sequence orders one instance's appends inside a millisecond, and the rand suffix breaks what is left. A clock that steps backwards keeps the last millisecond and advances the sequence, so an instance's own parts stay in write order — the property Math.max(now, lastMs + 1) was also providing.

listSessions() matches both name shapes, so a bucket written by an earlier version keeps its timestamps and still loads in order. Parts of the two shapes are only unordered relative to each other inside a single shared millisecond, where two writers have no ordering to preserve anyway.

Three tests added: a burst against a second writer on one bucket, a backwards clock, and listSessions() reading a part written before the sequence field. The first fails on main. The existing same-millisecond ordering test and the 13 conformance checks pass unchanged; 25 tests in s3/, and the Redis and Postgres adapters are untouched.

nextPartName() separated same-millisecond appends by advancing the
timestamp past the clock. The borrowed milliseconds then belong to
whoever writes next, so a second instance sharing the bucket stamps a
part with a millisecond the first instance had already taken, and its
part sorts inside the earlier burst.

From [a, b] on one instance and a third entry from another a millisecond
later, load() returned the second instance's entry in position two
instead of last. Across 300 appends from two instances, 1747 of 44460
pairs written at different wall-clock times came back inverted. One
instance stayed correct, which is why the conformance suite -- a single
store per case -- never saw it.

A per-millisecond sequence in the name separates a burst without moving
the timestamp: part-{epochMs13}-{seq6}-{rand6}.jsonl. Cross-writer
inversions go to zero at one, two and three instances, and an instance's
own parts stay in write order even if the clock steps backwards, which
the old max(now, lastMs + 1) was also protecting against.

listSessions() reads both name shapes, so a bucket written by an earlier
version keeps its timestamps and still loads in order.
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