[SLOP(claude-opus-4-8-high)] feat(universaldb): postgres leader-resolver driver overhaul

[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 5329
Push local edits: forklift submit
Merge when ready: forklift merge 5329

_{change xrrmomwz}

[claude]

PR Review: Postgres Leader-Resolver Driver Overhaul (updated for latest commit)

The core architecture is a solid improvement: replacing the GiST exclusion-constraint / advisory-lock approach with a leader-elected drain queue (udb_commit_requests), a shared in-memory TransactionConflictTracker (now factored out and reused from the rocksdb driver), and NOTIFY-driven wakeups with polling backstops. Nice catches along the way too: moving global_version_seq from UNLOGGED to a durable udb_version_seq fixes a real crash-durability bug in the old code, and the new commit-queue payload correctly uses versioned BARE (OwnedVersionedData) per the repo's vbare convention.

The PR was rebased since the last review pass, but the four previously-flagged issues are still present at the same locations in the current diff. Re-verified against the latest commit (3063bd1):

1. Cold-start recovery floor doesn't account for is_called, stalling every write for 5s on a fresh database

engine/packages/universaldb/src/driver/postgres/resolver/mod.rs:141

.query_one("SELECT last_value FROM udb_version_seq", &[])

On a brand-new sequence, Postgres reports last_value = 1 (the start value) with is_called = false, even though nextval has never actually been called. recovery_floor therefore returns recovery_version = max(durable=0, seq_high=1) = 1. Every first-boot follower opens its snapshot with read_version = 0, so start_version = 0 < 1 and gets unconditionally rejected as a conflict for the full TXN_TIMEOUT cold window. Fix: SELECT CASE WHEN is_called THEN last_value ELSE 0 END FROM udb_version_seq.

2. PgListener reconnect race can permanently miss a LISTEN for a channel registered mid-reconnect

engine/packages/universaldb/src/driver/postgres/listener.rs:150-176

connect_and_run snapshots registered channels, issues LISTEN for each, and only then does *client.lock().await = Some(new_client) (line 176). If listen() for a new channel races in between the snapshot and that assignment, the channel is inserted into the scc::HashMap but self.client is still None, so the "best-effort immediate LISTEN" in listen() is skipped, and the re-LISTEN loop that would have caught it already ran. That channel gets no LISTEN until the next reconnect, which may not happen for a long time on a stable connection. Impact is bounded by the polling backstops elsewhere, but it silently defeats the "re-LISTENs on reconnect" comment's intent. Fix: set *client = Some(new_client) before the re-LISTEN loop so concurrent listen() calls land on the active client.

3. wait_for_leader busy-polls instead of using the watch::Receiver that already exists for this purpose

engine/packages/universaldb/src/driver/postgres/commit.rs:88-99

async fn wait_for_leader(shared: &Arc<PostgresShared>) -> Result<LeaseInfo> {
    let deadline = Instant::now() + LEADER_WAIT_TIMEOUT;
    loop {
        if let Some(lease) = shared.current_lease() { return Ok(lease); }
        if Instant::now() >= deadline { return Err(DatabaseError::NotCommitted.into()); }
        tokio::time::sleep(RESULT_POLL_INTERVAL).await;
    }
}

This is exactly the loop { check; sleep } pattern CLAUDE.md's Performance section prohibits. PostgresShared already carries lease_tx/lease_rx (a watch::channel) that's updated by set_lease; the event-driven primitive is sitting right there unused. This fires on every commit submitted during a leader-election gap (routine during failover, not just cold start), so it's not a purely theoretical path. Fix: expose a subscribe_lease() on PostgresShared and drive this with lease_rx.changed() inside a tokio::select! against the deadline instead of a fixed 250ms sleep.

4. Commit-queue GC deletes pending rows too, contradicting its own documented invariant

engine/packages/universaldb/src/driver/postgres/database.rs:217-224

"DELETE FROM udb_commit_requests
 WHERE created_at < now() - ($1::bigint * interval '1 second')"

COMMIT_ROW_MAX_AGE_SECS's doc comment says "Terminal and orphaned commit-request rows older than this are garbage collected," but the query has no status filter; it deletes pending rows too. A commit request stuck pending for >60s (leader stall, pool exhaustion) gets silently GC'd out from under a follower still awaiting its result in await_result. The follower does handle Ok(None) from read_status as NotCommitted and retries, so this doesn't appear to be a correctness break today, but it directly contradicts the stated invariant and would silently corrupt any future logic/metric that assumes live pending rows survive. Fix: add AND status != 'pending', or update the comment to reflect actual behavior.

---

New observations on this pass

5. commit::submit can silently skip leader registration for a write-only conflict range with no operations
engine/packages/universaldb/src/driver/postgres/commit.rs:29-35

if operations.is_empty()
    && conflict_ranges.iter().all(|(_, _, kind)| matches!(kind, ConflictRangeType::Write))
{
    return Ok(());
}

The public Transaction::add_conflict_range API allows a caller to manually register a Write-type conflict range without any accompanying operation (used to signal "this range should conflict with concurrent readers" without a literal write). If a transaction does only that (no SetValue/Clear/atomic ops, and its only conflict ranges are Write-type), this shortcut skips the leader entirely, so the write-conflict range never reaches TransactionConflictTracker. A concurrent transaction with an overlapping Read conflict range would then commit without ever detecting the conflict. None of the current call sites (gasoline/db/kv/{mod,debug}.rs, tests/integration.rs) hit this exact combination since they all pair Write-type manual ranges with a real operation, or include a Read range too, so this is currently latent rather than triggered, but it's a trap in a documented public API. Worth either a comment explaining the assumption, or tightening the skip condition to also require conflict_ranges.is_empty().

6. Unused new dependencies on universaldb

base64 and tokio-util are both added to engine/packages/universaldb/Cargo.toml in this PR, but neither appears referenced anywhere in engine/packages/universaldb/src/ (checked via grep across the whole crate, not just the diff). Worth double-checking these aren't leftovers from an earlier iteration before merging.

7. Test coverage gap

This PR rewrites the entire postgres commit path (leader election, epoch fencing, group-commit batching, cold-window recovery, GC) but adds no new tests exercising any of it; the only test file touched (tests/integration.rs) is an unrelated API rename (set_option → txn_retry_limit). Given this is a correctness-critical rewrite (conflict resolution + versionstamp assignment for the entire postgres driver), at minimum a failover/epoch-fencing test and a cold-window-rejection test would catch regressions like #1 above.

Minor

• LeaseInfo::leader_addr (shared.rs) actually holds a per-process node_id (UUID), not a network address; the field name reads as if it's a host:port and may confuse future readers of the commit/reply channel naming.
• The postgres:17 → postgres:18 bump touches compose files, k8s manifests, and docs across the repo. Nothing in the new schema looks PG18-specific; worth confirming this is an intentional, separately-validated upgrade rather than incidental scope creep in this PR.