fix(postgres): guard strict reads against replica revision gaps - #3243
Open
josephschorr wants to merge 1 commit into
Open
fix(postgres): guard strict reads against replica revision gaps#3243josephschorr wants to merge 1 commit into
josephschorr wants to merge 1 commit into
Conversation
josephschorr
force-pushed
the
fix-strict-read-replica-revision-gap
branch
from
July 20, 2026 19:16
9265578 to
a1baa48
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
josephschorr
marked this pull request as ready for review
July 20, 2026 19:27
|
Hey, great work, can it be merged now? |
The strict read-replica guard asserted only that transaction `xmin - 1` had resolved on the replica. A revision snapshot makes visible any committed transaction with an xid below xmax that is not in its in-progress list, so visible data can come from transactions with an xid at or above xmin. When a concurrent, still-open transaction holds xmin below a freshly committed write, a lagging replica that had applied xmin-1 but not that write passed the guard while the row was absent, returning "object definition <ns> not found" instead of falling back to the primary. Replace the single-transaction check with an inline snapshot-domination check: the replica's current snapshot must have reached the revision's xmax and must not still consider in-progress any transaction the revision treats as committed. This is the inline equivalent of the comparison CheckRevision performs, evaluated on the replica's own connection so it stays valid behind a load balancer. A single visible xid is not sufficient because commit order does not follow xid order: a lower-xid transaction the revision sees as committed can still be replaying on the replica after higher xids have been applied. Add datastore tests that stand up a real Postgres primary and streaming replica and drive replication lag deterministically via pg_wal_replay_pause(), covering both the fallback and the replica-serves-a-caught-up-revision paths. Fixes authzed#2525
josephschorr
force-pushed
the
fix-strict-read-replica-revision-gap
branch
from
July 27, 2026 23:00
a1baa48 to
cb23809
Compare
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.
Problem
Fixes #2525.
With Postgres read replicas enabled, SpiceDB intermittently returned
object definition <ns> not found(surfaced asUnknown/FailedPrecondition) even when the configured follower delay exceeded the replication latency. It reproduced only under load and went away when replicas were removed.Root cause
Reads filter rows with
pg_visible_in_snapshot(created_xid, revision.snapshot)— a pure function of the revision's snapshot. A snapshot makes visible any committed transaction with an xid< xmaxthat is not in its in-progress (xip) list, so visible data can come from transactions with an xid at or abovexmin.The strict read-replica guard, however, only asserted that transaction
xmin - 1had resolved on the replica. When a concurrent, still-open transaction holdsxminbelow a freshly committed write, a lagging replica that has appliedxmin-1but not that write passes the guard while the row is absent → zero rows, no error, no fallback → not-found. At production write QPS there is almost always an in-progress write with a lower xid than the most recently committed ones, which is why it only showed up under load.Fix
Replace the single-transaction check with an inline snapshot-domination check, evaluated on the replica's own connection so it stays valid behind a load balancer (which is why the check must be inline rather than a separate
CheckRevisionround-trip):xmaxmust have reached the revision'sxmax; andxmaxthat the revision treats as committed.This is the inline equivalent of the comparison
CheckRevisionperforms. A single visible xid is not sufficient, because commit order does not follow xid order: a lower-xid transaction the revision sees as committed can still be replaying on the replica after higher xids have been applied.Testing
New datastore tests (
//go:build datastore && postgres) stand up a real primary + streaming hot-standby replica in containers and drive replication lag deterministically withpg_wal_replay_pause():TestStrictReadGuardDetectsVisibleDataGapOnReplica— drives the production guard SQL directly; it raisesreplica missing revisionwhile behind and succeeds after catch-up.TestReplicaLagStrictReadFallsBackForNamespace— end-to-end through the strict replicated datastore; the read now falls back to the primary and finds the namespace instead of reporting it not-found.TestReplicaServesCaughtUpRevisionWithoutFallback— verifies a caught-up (min-latency) revision is still served by the replica with no fallback, asserted via the fallback metric.Verified against Postgres 14, 17, and 18;
mage testds:postgresis green.Routing
FullyConsistentreads to the primary is a separate optimization and is intentionally out of scope here.