VersionAware optimistic concurrency: replaceIf(key, value) CAS on a store-stamped TSID version (lib-data-store-state-redis 1.2.0) [release]

[pditommaso]

Problem

replaceIf(key, expected, value) (1.1.0, #104) performed the compare-and-swap on a re-serialization of the expected value, silently requiring the encoding strategy to be byte-deterministic across processes. That assumption fails in practice: Jackson orders a class's extra (non-component) accessors by Class.getDeclaredMethods() reflection, which differs between JVM instances. Observed in the sched-dev rollout of seqeralabs/sched#913: two pods of the same image serialized TaskSpec's four derived accessors in different orders, so a CAS attempted by any replica other than the one that wrote the entry refused persistently — causing duplicate command executions, poison-message retry loops, and a user pipeline terminated by the fallout. Invisible in single-JVM tests, since each process is internally consistent. It also shipped the full expected payload to the server on every attempt (~270 KB twice over for sched's batch states).

Design

Values carry their own optimistic-concurrency version, and the whole capability lives in dedicated types — AbstractStateStore and StateProvider keep their 1.0 contracts, byte-for-byte:

interface VersionAware<T> {          // the value contract
    long version();
    T withVersion(long version);
}

// consumer contract — new class, versioning enforced at compile time
abstract class VersionedStateStore<V extends VersionAware<V>> extends AbstractStateStore<V> {
    boolean replaceIf(String key, V value);
    boolean replaceIf(String key, V value, Duration ttl);
}

// implementation contract — standalone capability, deliberately unrelated to StateProvider
interface VersionProvider<K,V> {
    boolean replaceIf(K key, long expected, V value, Duration ttl);
}

Both providers implement StateProvider and VersionProvider; VersionedStateStore accepts a plain StateProvider — a store switches between plain and versioned by changing only its extends clause — and verifies the capability at construction, failing fast otherwise. A non-VersionAware value type is now a compile error, not a runtime check.

The version is a unique time-sorted identifier (TSID), stamped by the store on every write path — put/putIfAbsent included, never assigned by callers, who only carry forward the version of the value they read. Because any write moves the version, every outstanding witness is invalidated by every write: there is no put-is-creation-only invariant and no ABA across a remove-and-recreate or TTL-expiry-and-recreate — the caveats an incremental counter would carry (an increment chain restarts at small, highly collision-prone values; and computing "previous + 1" on a blind put would force every write through a server-side read-modify script, where the TSID keeps put a plain SET).

Caller flow:

var state = store.get(id);            // version as stored
var next  = state.completed(result);  // transitions carry the version through
if (!store.replaceIf(id, next))       // lands only if the entry is unchanged since the read
    // re-read, re-decide

Single atomic server-side call. The store frames every versioned write with a leading {"@v":N} JSON property — written by the store at a fixed position, never by the encoder — and the swap is one Lua script that peeks the stored version with GETRANGE (28 bytes, covering 19-digit TSIDs) and conditionally SETs the new value:

• one round-trip per CAS, no read-back
• no expected payload on the wire — only the new value ships
• no payload parsing server-side — cost independent of the value size
• no re-serialization anywhere — the byte-determinism assumption is gone, not worked around

The frame is transparent to the encoding strategy, symmetrically. VersionedStateStore adds it on write and strips it on read, injecting its version into the decoded value through withVersion — the decoder always receives exactly the payload the encoder produced, so no unknown-property tolerance is required of any decoder, the value type does not need to serialize its version field, and the frame is the single source of truth for the version (whatever the payload itself may carry). Versioned values must serialize to a JSON object (enforced fail-fast with IllegalStateException), and their leading @v property is reserved for the store.

Reads are resilient to foreign data that merely resembles a frame: version digits that do not fit a long — which no store-written frame can carry — fall back to decoding the stored form as-is at version 0, never a read failure; and a plain AbstractStateStore never frames nor strips, so a genuine leading @v property of a non-versioned value is always preserved.

Migration

None for storage. Entries stay plain strings; the frame appears as values are written by new code. An entry written before versioning carries no frame, counts as version 0, and is adopted transparently by its first successful replace; a rolling window against older code degrades to the pre-existing behavior, never worse, and converges once the fleet is uniform.

API note: the 1.1.0 raw byte-compare replaceIf is removed — a breaking change for its one consumer (sched), which migrates in lockstep (see Follow-up). StateStore, AbstractStateStore and StateProvider are all back to their pre-1.1.0 contracts, so plain-store consumers (lib-cmd-queue-redis, lib-pairing) are untouched.

Testing

TDD throughout. Store level: the regression drives a store through a deliberately non-deterministic JSON encoder (property order and spacing rotate on every encode() — the single-process equivalent of the cross-JVM divergence observed in dev) and passes because nothing compares serialized bytes; frame transparency is pinned by an encoder that does not serialize the version at all and throws if the frame ever reaches its decoder (round-trip, witness validity, putIfAbsentAndCount read-back); framing and fresh-stamp asserted on creation, replace, and legacy adoption; frame-authoritative reads (a spurious payload ver loses to the absent frame); the empty-object frame {"@v":N} round-trips; foreign frame-lookalikes preserved on plain stores and overflow digits degrade gracefully; stale version refused with the concurrent write preserved; missing key refused; non-JSON-object encoders and non-VersionProvider providers rejected fail-fast; RequestIdAware mapping refreshed. Provider level, in both Local and Redis (testcontainers) suites: version match/mismatch, unframed-counts-as-zero adoption, null/empty stored values refused without throwing, TTL reset semantics, and a 16-thread barrier test proving exactly one concurrent replace wins per round — validating the actual Lua. 54 tests across the module, all green; lib-cmd-queue-redis and lib-pairing compile and pass unaffected.

Follow-up

seqeralabs/sched PR: bump to 1.2.0, have CommandStateStoreImpl extend VersionedStateStore, add a version component to CommandState (carried through its transition methods) implementing VersionAware<CommandState>, and rebase CommandStateStoreImpl.update() on the new replaceIf.

🤖 Generated with Claude Code

[pditommaso]

Claude Code review summary

Verdict: approve. No correctness issue on any store-written path. Verified: window math (max store-written frame is 6 + 19 digits + terminator = 26 bytes, inside the 28-byte peek — a TSID is a positive long), Lua atomicity and its local mirror (CAS on entry identity), frame transparency through every write path (put, putIfAbsent, putIfAbsentAndCount), witness captured before the re-stamp, legacy adoption at version 0. Module tests green; lib-cmd-queue-redis and lib-pairing unaffected.

Findings, all addressed on the branch:

1. Foreign frame-lookalike guarantee overstated (low, docs) — "never matches and is left untouched" holds only while the digit run fits the 28-byte head peek; a ≥22-digit run (impossible for a store-written frame) degrades to version 0 like an unframed entry. Claim scoped in the changelog, VersionProvider javadoc and the Lua comment → a13b9fd
2. Unbalanced {@code {"@v":N} doc tags (cosmetic) — the unmatched inner brace made the tag swallow the prose after it; 6 occurrences balanced → a13b9fd
3. Nits — duplicate log on overflow reads collapsed to the single parseVersion warn; Pattern imported instead of the inline FQN; PR body sketch showed a VersionProvider overload that doesn't exist (body corrected) → a13b9fd
4. From review discussion (swampie: can value be null?) — a raw put(key, null) on the local provider could NPE versionOf during the CAS; a null/empty stored value now refuses the match outright, mirroring the Lua empty-head refusal, with tests on both providers → 9e9cf39

🤖 Generated with Claude Code

[jonmarti]

Four things:

1. No write returns the stamped version, including a successful replaceIf.

read                       → version 100
replaceIf #1               → true    (entry now version 200, caller's object still 100)
replaceIf #2 chained       → false
replaceIf #2 after re-read → true

Compounding it: version 0 means two different things — a value you constructed and never read carries 0, and a pre-versioning entry has no frame so it also reads back as 0. The CAS just compares numbers, so those two
collide:

never-read value (0) vs a framed entry (200) → false, safe no-op
never-read value (0) vs a legacy entry  (0)  → true, blind overwrite

Both verified. So during the rolling window the same slip overwrites instead of no-op'ing — and false is indistinguishable from contention, so a retry loop spins rather than surfacing it. sched's flow always
re-reads, so this is latent rather than active.

→ The README comment is wrong either way: false covers three causes (another writer won, key missing, caller didn't re-read) and names only the first.

→ Then a choice: add V replaceIfAndGet(key, value) returning the stamped copy, or leave the API as-is and document the re-read rule for all writes.

2. AbstractStateStoreTest's helper flipped to extends VersionedStateStore, so its ~10 tests now cover the versioned path only. put/get survive indirectly via CommandServiceTest, but TTL expiry, custom
TTL, both putIfAbsent forms, putIfAbsentAndCount + counter script, and findByRequestId now have no direct test on the plain store — which is still published contract.

→ Restore the extends, add the versioned copies as VersionedStateStoreTest.

3. serialize/deserialize should be final. A subclass override silently drops the frame, and every CAS from then on is a blind write at version 0. Nothing in the repo overrides either, so it's non-breaking
here.

4. An @v head of 22+ digits gets overwritten. Swept 17–26 digits on both providers:

digits | bytes to terminator | Local | Redis
    21 |                  28 | false | false
    22 |                  29 |  true |  true

The terminator falls outside the 28-byte peek, ver falls through to unframed, and any version-0 witness clobbers it. Same input shape as the 20-digit case you protect and test.

→ In the Lua and in versionOf: if the pattern doesn't match but the head starts {"@v":%d, refuse rather than falling through.

[pditommaso]

Addressed in #108 (releases 1.2.1):

• 4 — the window-overrun fallthrough is now refused outright in the Lua and in versionOf; your 22-digit shape is covered on both providers and at store level.
• 2 — AbstractStateStoreTest is back on the plain store; versioned features moved to a new VersionedStateStoreTest.
• 3 — serialize/deserialize are final on VersionedStateStore.
• 1 — README comment reworded to cover every cause of false, and the re-read rule is documented. No replaceIfAndGet for now: no caller needs it (sched re-reads), so the documented rule covers the hazard — worth adding when a real chained-CAS caller shows up.