Skip to content

Commit 0025b29

Browse files
committed
fix: broken test and new test
1 parent fea8f41 commit 0025b29

6 files changed

Lines changed: 382 additions & 8 deletions

File tree

internal/datastore/memdb/memdb.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,10 @@ func NewMemdbDatastore(
7171
{
7272
revision: nowRevision(),
7373
schemaHash: "",
74-
db: db,
74+
// A snapshot of the empty database rather than the database
75+
// itself: reads at this revision must see the datastore as it
76+
// was when it was created, not as it is now.
77+
db: db.Snapshot(),
7578
},
7679
},
7780

@@ -122,7 +125,7 @@ func (mdb *memdbDatastore) UniqueID(_ context.Context) (string, error) {
122125
}
123126

124127
// SnapshotReader returns a reader for the snapshot visible at the given
125-
// revision: the first entry in mdb.revisions at or after it, located by
128+
// revision: the most recent entry in mdb.revisions at or before it, located by
126129
// binary search.
127130
func (mdb *memdbDatastore) SnapshotReader(dr datastore.Revision) datastore.Reader {
128131
mdb.RLock()
@@ -140,13 +143,17 @@ func (mdb *memdbDatastore) SnapshotReader(dr datastore.Revision) datastore.Reade
140143
return &memdbReader{nil, nil, err, time.Now()}
141144
}
142145

146+
// sort.Search finds the first snapshot newer than the requested revision,
147+
// so the one visible at it is the entry before that.
143148
revIndex := sort.Search(len(mdb.revisions), func(i int) bool {
144-
return mdb.revisions[i].revision.GreaterThan(dr) || mdb.revisions[i].revision.Equal(dr)
149+
return mdb.revisions[i].revision.GreaterThan(dr)
145150
})
146151

147-
// handle the case when there is no revision snapshot newer than the requested revision
148-
if revIndex == len(mdb.revisions) {
149-
revIndex = len(mdb.revisions) - 1
152+
// Handle the case where every snapshot is newer than the requested
153+
// revision, i.e. it predates the datastore itself: the oldest snapshot is
154+
// the closest thing to the state at that revision.
155+
if revIndex > 0 {
156+
revIndex--
150157
}
151158

152159
rev := mdb.revisions[revIndex]

internal/datastore/memdb/revisions.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,16 @@ func (mdb *memdbDatastore) OptimizedRevision(_ context.Context) (datastore.Revis
8282
optimized = now
8383
}
8484

85+
// Quantization rounds downward, so a datastore created moments ago would
86+
// otherwise advertise a boundary from before it held any snapshot at all.
87+
// No snapshot can serve that revision, so advertise the head revision
88+
// instead: the same fallback Postgres makes when the bucket it rounded back
89+
// to contains no transaction. This can only happen within one quantization
90+
// window of the datastore being created.
91+
if optimized.LessThan(mdb.revisions[0].revision) {
92+
optimized = mdb.headRevisionNoLock()
93+
}
94+
8595
// Find the schema hash visible at the optimized revision: walk the
8696
// revisions list backward for the most recent snapshot whose revision
8797
// is at or before `optimized`.

pkg/datastore/datastore.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,29 @@ type ReadOnlyDatastore interface {
727727

728728
// SnapshotReader creates a read-only handle that reads the datastore at the specified revision.
729729
// Any errors establishing the reader will be returned by subsequent calls.
730+
//
731+
// Reads through the handle behave like a snapshot: they are repeatable, and
732+
// they never observe a write committed after the given revision.
733+
// Each engine reaches that guarantee differently:
734+
//
735+
// - Postgres and MySQL treat the revision as a transaction snapshot and
736+
// filter each row against it, keeping only rows created at or before
737+
// the revision and not yet deleted as of it. Postgres in strict read
738+
// mode additionally asserts that the revision is present on the
739+
// instance being read, so a lagging replica fails the query with a
740+
// RevisionUnavailableError instead of answering from older data.
741+
// - CockroachDB and Spanner treat the revision as an MVCC timestamp and
742+
// leave the filtering to the database, in both cases pinned to exactly
743+
// that timestamp rather than to the nearest one at or after it:
744+
// CockroachDB appends AS OF SYSTEM TIME <revision> to each query, and
745+
// Spanner bounds a read-only transaction with ReadTimestamp. A
746+
// revision the database no longer retains, or one in the future, fails
747+
// at query time rather than when the handle is created.
748+
// - memdb keeps one in-memory snapshot per write and reads from the most
749+
// recent snapshot at or before the revision, falling back to its
750+
// oldest snapshot for a revision that predates the datastore itself.
751+
// It is also the only implementation that validates the revision when
752+
// the handle is created, rather than when it is first read.
730753
SnapshotReader(Revision) Reader
731754

732755
// OptimizedRevision gets a revision that will likely already be replicated

0 commit comments

Comments
 (0)