[slopfix] fix(universaldb): match fdb conflict semantics in rocksdb + postgres drivers

[MasterPtato]

No description provided.

[MasterPtato]

Stack for rivet-dev/rivet

• #5552 [slopfix] fix(universaldb): match fdb conflict semantics in … 👈
• #5551 [slopfix] fix(universaldb,util,pegboard-envoy): index dedup …
• #5398 feat(kitchen-sink): add churnDb actor and seed driver for co …
• #5397 chore: Clean up
• #5382 [slopfix] docs(self-hosting): correct postgres config format …
• #5356 [slopfix] fix(pegboard): decrement kv_storage_used metric on …
• #5354 [slopfix] refactor(universaldb,ups): single/multi-node udb o …
• #5353 [slopfix] perf(universaldb): fold postgres drain_batch into …
• #5342 [SLOP(claude-opus-4-8-high)] perf(universaldb): batch leader …
• #5341 [slopfix] docs(self-hosting): promote postgres from experime …
• #5340 [slopfix] fix(self-host): repair dev compose generation, kit …
• #5337 [SLOP(claude-opus-4-8-high)] feat(universaldb): graceful pos …
• #5336 [slopfix] test(universaldb): postgres leader failover + abor …
• #5335 [SLOP(claude-opus-4-8-high)] feat(ups): table-backed postgre …
• #5329 [SLOP(claude-opus-4-8-high)] feat(universaldb): postgres lea …
• #5327 [SLOP(claude-opus-4-8)] feat(pegboard-envoy): rate-limit env …
• #5326 [SLOP(claude-opus-4-8)] feat(util): add rate limiter primiti …
• #5322 [SLOP(claude-opus-4-8)] feat(rivetkit-core): add serde durat …
• #5321 Add observation for serde_bare
• #5320 Add observation and ser/de metrics for serde calls
• #5314 [SLOP(claude-opus-4-8-medium)] chore: add profiling cargo pr …
• #5306 [SLOP(claude-opus-4-8)] fix(kitchen-sink): serve built front …
• main

Get stack: forklift get 5552
Push local edits: forklift submit
Merge when ready: forklift merge 5552

_{change kuuxrnqs}

[claude]

Review

Solid, well-scoped correctness fix. The core change matches real FDB conflict semantics: conflicts are directional (a committing txn aborts only if something it read was written by a transaction inside its version window), so the resolver now only needs to retain write ranges and only needs to check a committing txn's reads against them. The accompanying doc comments clearly explain the "why," which is appreciated.

Correctness

• conflict_tracker.rs: The old symmetric check (cr1_type != cr2_type over the full cross product) allowed a read-only previously-committed transaction's read range to abort a later transaction's blind write, which isn't how FDB behaves. The new directional check (only txn1 reads vs txn2 writes) fixes this, and dropping read-only entries from the retained map (if !write_ranges.is_empty()) is a nice memory win since they could never have aborted anyone anyway.
• tx_ops::is_read_only: This is actually a behavior change, not just a refactor, and I think it's the most important fix in the PR. The old check in postgres/commit.rs treated a transaction as skippable when operations.is_empty() && conflict_ranges.iter().all(|k| kind == Write) — which is vacuously true only when no ranges are present, but also (bug) true when a transaction added an explicit Write-type conflict range via add_conflict_range with no actual mutation. That would silently skip submitting the write-range-only txn to the leader/tracker, meaning it could never cause a real writer-vs-reader conflict for anyone else. The new check correctly treats any explicit Write range as "not read-only." gasoline's kv/mod.rs calls add_conflict_range directly in a couple of places, so this had real blast radius on postgres/rocksdb backends (FDB itself was never affected since it has native conflict ranges).
• rocksdb per-task snapshot: Previously every Get/GetKey/GetRange spun up a brand-new rocksdb transaction (hence a new snapshot) per call, so reads within the same logical UDB transaction were not actually repeatable — a commit landing between two reads in one transaction could be partially visible. Pinning one snapshot per TransactionTask at first read and releasing it on Commit fixes this and is also a minor perf win (no per-read transaction object). I traced the driver/task lifecycle (RocksDbDatabaseDriver::run creates a fresh create_txn() → fresh TransactionTask per retry attempt) and confirmed the snapshot can't leak stale state across retries — each attempt gets its own task/snapshot regardless of the explicit reset. Nice catch, validated by the new reads_are_repeatable_within_a_txn test.

Test coverage

• conflict_parity.rs is a good addition and does a nice job covering the directional semantics end-to-end against both drivers (read-only-does-not-abort-writer, read-only-never-conflicts, repeatable reads, blind write/write non-conflict, and the one case that must still abort).
• One gap: there's no regression test for the is_read_only fix described above — a transaction that calls add_conflict_range(_, _, Write) with no actual set/clear/atomic_op and no reads, verifying it's still recorded in the tracker and can conflict with a later reader of that range. The existing integration.rs::test_conflict_ranges cases that use add_conflict_range(Write) all pair it with a real tx.set, so operations.is_empty() was already false under both old and new logic — they wouldn't have caught this bug. Given gasoline relies on exactly this pattern, a targeted test here seems worth adding.
• Minor: Cargo.lock bumps rivet-universaldb-commit from 2.3.2 to 2.3.7 with no corresponding source changes to that crate in the diff — likely just workspace-version drift from local tooling, but worth double-checking it's intentional before merge.

Style

• Comments follow the repo's complete-sentence convention and explain why (FDB parity, directionality) rather than what, which matches CLAUDE.md guidance well.
• No concerns on logging, metrics, or locking conventions — this PR doesn't touch those areas.

Overall this looks like a correct and valuable fix for two real bugs (symmetric conflict checking, and read-only misclassification of write-range-only transactions) plus a repeatable-reads bug in the rocksdb driver, with good new test coverage. The one suggested addition is a direct regression test for the is_read_only write-conflict-range-only scenario.