test: add revision tests (#3257)

* test: add revision test

* fix: making Spanner honor the configured GC window instead of hardcoding it

* test: modify SnapshotReadStabilityTest so it fails for memdb ONLY

* fix: broken test and new test

* chore: changelog

* chore: simplify godocs

* chore: autovacuum=off for postgres tests

Author: miparnisari

CHANGELOG.md

@@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
+### Fixed
+- MemDB: a read at a given revision, such as a `Check` at an `at_exact_snapshot` ZedToken, could include relationships written *after* that revision, so the same revision returned different results as later writes arrived instead of a stable point-in-time view. Reads at a revision now return only the data committed as of it. (https://github.com/authzed/spicedb/pull/3257)
+- Spanner: the configured `--datastore-gc-window` was ignored, and revisions were always treated as valid for 24 hours (the change stream retention). Spanner now honors the configured window, capped at that retention since older revisions cannot be read regardless. (https://github.com/authzed/spicedb/pull/3257)
 
 ## [1.56.1] - 2026-08-26
 ### Changed

internal/datastore/memdb/memdb.go

@@ -71,7 +71,12 @@ func NewMemdbDatastore(
 			{
 				revision:   nowRevision(),
 				schemaHash: "",
-				db:         db,
+				// A snapshot of the still-empty database rather than the
+				// database itself, so that a read at the creation revision
+				// above sees the datastore as it was created rather than as it
+				// is now. Every later entry likewise holds a snapshot frozen
+				// at its own revision.
+				db: db.Snapshot(),
 			},
 		},
 
@@ -122,7 +127,7 @@ func (mdb *memdbDatastore) UniqueID(_ context.Context) (string, error) {
 }
 
 // SnapshotReader returns a reader for the snapshot visible at the given
-// revision: the first entry in mdb.revisions at or after it, located by
+// revision: the most recent entry in mdb.revisions at or before it, located by
 // binary search.
 func (mdb *memdbDatastore) SnapshotReader(dr datastore.Revision) datastore.Reader {
 	mdb.RLock()
@@ -140,13 +145,17 @@ func (mdb *memdbDatastore) SnapshotReader(dr datastore.Revision) datastore.Reade
 		return &memdbReader{nil, nil, err, time.Now()}
 	}
 
+	// sort.Search finds the first snapshot newer than the requested revision,
+	// so the one visible at it is the entry before that.
 	revIndex := sort.Search(len(mdb.revisions), func(i int) bool {
-		return mdb.revisions[i].revision.GreaterThan(dr) || mdb.revisions[i].revision.Equal(dr)
+		return mdb.revisions[i].revision.GreaterThan(dr)
 	})
 
-	// handle the case when there is no revision snapshot newer than the requested revision
-	if revIndex == len(mdb.revisions) {
-		revIndex = len(mdb.revisions) - 1
+	// Handle the case where every snapshot is newer than the requested
+	// revision, i.e. it predates the datastore itself: the oldest snapshot is
+	// the closest thing to the state at that revision.
+	if revIndex > 0 {
+		revIndex--
 	}
 
 	rev := mdb.revisions[revIndex]

internal/datastore/memdb/revisions.go

@@ -82,6 +82,12 @@ func (mdb *memdbDatastore) OptimizedRevision(_ context.Context) (datastore.Revis
 		optimized = now
 	}
 
+	// Rounding down can land before the oldest snapshot, which no read can be
+	// served at. Advertise head instead, as Postgres does for an empty bucket.
+	if optimized.LessThan(mdb.revisions[0].revision) {
+		optimized = mdb.headRevisionNoLock()
+	}
+
 	// Find the schema hash visible at the optimized revision: walk the
 	// revisions list backward for the most recent snapshot whose revision
 	// is at or before `optimized`.

internal/datastore/postgres/postgres_shared_test.go

@@ -1155,15 +1155,27 @@ func HeadRevisionDoesNotConsumeXIDTest(t *testing.T, ds datastore.Datastore) {
 		return x
 	}
 
-	const iterations = 10
+	// The xid counter is cluster-wide, so activity that has nothing to do with
+	// this test can advance it while the test runs: autoanalyze burns two xids
+	// each time it fires, and the test instance is shared by every database the
+	// suite creates. Tolerate that noise rather than requiring an exact match,
+	// but keep the allowance far below the one-per-call a HeadRevision that
+	// opened a write transaction would produce.
+	const (
+		iterations        = 100
+		allowedBackground = 10
+	)
+
 	before := nextXID()
 	for i := 0; i < iterations; i++ {
 		_, err := ds.HeadRevision(ctx)
 		require.NoError(t, err)
 	}
 	after := nextXID()
 
-	require.Equal(t, before, after, "HeadRevision burned %d xid(s) over %d calls; expected 0", after-before, iterations)
+	require.LessOrEqual(t, after-before, uint64(allowedBackground),
+		"HeadRevision burned %d xid(s) over %d calls; expected at most %d from unrelated background activity",
+		after-before, iterations, allowedBackground)
 }
 
 // ConcurrentRevisionWatchTest uses goroutines and channels to intentionally set up a pair of

internal/datastore/spanner/enginebuilder.go

@@ -40,6 +40,7 @@ func newDatastoreFromConfig(ctx context.Context, opts datastorecfg.Config) (data
 		opts.URI,
 		FollowerReadDelay(opts.FollowerReadDelay),
 		RevisionQuantization(opts.RevisionQuantization),
+		GCWindow(opts.GCWindow),
 		MaxRevisionStalenessPercent(opts.MaxRevisionStalenessPercent),
 		//nolint:staticcheck // the deprecated credentials options remain supported until removal
 		CredentialsFile(opts.SpannerCredentialsFile),

internal/datastore/spanner/options.go

@@ -36,6 +36,7 @@ type spannerOptions struct {
 	watchChangeBufferMaximumSize uint64
 	watchBufferWriteTimeout      time.Duration
 	revisionQuantization         time.Duration
+	gcWindow                     time.Duration
 	followerReadDelay            time.Duration
 	maxRevisionStalenessPercent  float64
 	credentialsFilePath          string
@@ -66,12 +67,12 @@ const (
 	errQuantizationTooLarge = "revision quantization (%s) must be less than (%s)"
 
 	defaultRevisionQuantization        = 5 * time.Second
+	defaultGCWindow                    = defaultChangeStreamRetention
 	defaultFollowerReadDelay           = 0 * time.Second
 	defaultMaxRevisionStalenessPercent = 0.1
 	defaultWatchBufferLength           = 128
 	defaultWatchBufferWriteTimeout     = 1 * time.Second
 	defaultDisableStats                = false
-	maxRevisionQuantization            = 24 * time.Hour
 	defaultFilterMaximumIDCount        = 100
 	defaultColumnOptimizationOption    = common.ColumnOptimizationOptionStaticValues
 	defaultWatchDisabled               = false
@@ -89,6 +90,7 @@ func generateConfig(options []Option) (spannerOptions, error) {
 		watchBufferLength:           defaultWatchBufferLength,
 		watchBufferWriteTimeout:     defaultWatchBufferWriteTimeout,
 		revisionQuantization:        defaultRevisionQuantization,
+		gcWindow:                    defaultGCWindow,
 		followerReadDelay:           defaultFollowerReadDelay,
 		maxRevisionStalenessPercent: defaultMaxRevisionStalenessPercent,
 		disableStats:                defaultDisableStats,
@@ -104,12 +106,22 @@ func generateConfig(options []Option) (spannerOptions, error) {
 		option(&computed)
 	}
 
-	// Run any checks on the config that need to be done
-	if computed.revisionQuantization >= maxRevisionQuantization {
+	// Run any checks on the config that need to be done.
+	// Revisions older than the change stream retention are not readable no
+	// matter what is configured, so a larger window cannot be honored.
+	if computed.gcWindow > defaultChangeStreamRetention {
+		log.Warn().
+			Dur("changeStreamRetention", defaultChangeStreamRetention).
+			Dur("gcWindow", computed.gcWindow).
+			Msg("configured gc window exceeds the Spanner change stream retention, so capping to the retention")
+		computed.gcWindow = defaultChangeStreamRetention
+	}
+
+	if computed.revisionQuantization >= computed.gcWindow {
 		return computed, fmt.Errorf(
 			errQuantizationTooLarge,
 			computed.revisionQuantization,
-			maxRevisionQuantization,
+			computed.gcWindow,
 		)
 	}
 
@@ -157,6 +169,12 @@ func RevisionQuantization(bucketSize time.Duration) Option {
 	}
 }
 
+func GCWindow(window time.Duration) Option {
+	return func(so *spannerOptions) {
+		so.gcWindow = window
+	}
+}
+
 // FollowerReadDelay is the time delay to apply to enable historical reads.
 //
 // This value defaults to 0 seconds.

internal/datastore/spanner/options_test.go

@@ -0,0 +1,50 @@
+package spanner
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestGCWindowOption(t *testing.T) {
+	tcs := []struct {
+		name             string
+		options          []Option
+		expectedGCWindow time.Duration
+		expectedError    string
+	}{
+		{
+			name:             "defaults to the change stream retention",
+			expectedGCWindow: defaultChangeStreamRetention,
+		},
+		{
+			name:             "a window within the change stream retention is honored",
+			options:          []Option{GCWindow(1 * time.Hour)},
+			expectedGCWindow: 1 * time.Hour,
+		},
+		{
+			name:             "a window beyond the change stream retention is capped",
+			options:          []Option{GCWindow(48 * time.Hour)},
+			expectedGCWindow: defaultChangeStreamRetention,
+		},
+		{
+			name:          "quantization at least as large as the window is rejected",
+			options:       []Option{GCWindow(time.Second), RevisionQuantization(time.Second)},
+			expectedError: "revision quantization (1s) must be less than (1s)",
+		},
+	}
+
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			config, err := generateConfig(tc.options)
+			if tc.expectedError != "" {
+				require.ErrorContains(t, err, tc.expectedError)
+				return
+			}
+
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedGCWindow, config.gcWindow)
+		})
+	}
+}

internal/datastore/spanner/spanner.go

@@ -209,7 +209,7 @@ func NewSpannerDatastore(ctx context.Context, database string, opts ...Option) (
 
 	ds := &spannerDatastore{
 		RemoteClockRevisions: revisions.NewRemoteClockRevisions(
-			defaultChangeStreamRetention,
+			config.gcWindow,
 			maxRevisionStaleness,
 			config.followerReadDelay,
 			config.revisionQuantization,

internal/datastore/spanner/spanner_test.go

@@ -33,6 +33,7 @@ func TestSpannerDatastore(t *testing.T) {
 		ds := b.NewDatastore(t, func(engine, uri string) datastore.Datastore {
 			ds, err := NewSpannerDatastore(ctx, uri,
 				RevisionQuantization(revisionParameters.Quantization),
+				GCWindow(time.Duration(revisionParameters.GCRetentionWindow)),
 				WatchBufferLength(watchBufferLength),
 				WithDatastoreMetricsOption(DatastoreMetricsOptionOpenTelemetry),
 			)

internal/testserver/datastore/config/postgres-with-timestamps.conf

@@ -1,3 +1,14 @@
 listen_addresses = '*'
 max_connections = 3000
 track_commit_timestamp = 1
+# Autovacuum is left on so autoanalyze keeps table statistics fresh: without
+# them the planner cannot push batched `object_id IN (...)` lookups into the
+# relation_tuple index, which roughly halves check throughput in the larger
+# tests. Vacuuming itself buys nothing on the timescale of a test, so the
+# thresholds below put it out of reach and -1 turns off insert-triggered
+# vacuums entirely. Autovacuum's own transactions still burn xids, so
+# HeadRevisionDoesNotConsumeXIDTest tolerates a few rather than requiring none.
+autovacuum = on
+autovacuum_vacuum_threshold = 2000000000
+autovacuum_vacuum_insert_threshold = -1
+autovacuum_freeze_max_age = 2000000000