Skip to content

chore: refactor migration code and tests#3222

Open
miparnisari wants to merge 2 commits into
mainfrom
refactor-migrations
Open

chore: refactor migration code and tests#3222
miparnisari wants to merge 2 commits into
mainfrom
refactor-migrations

Conversation

@miparnisari

@miparnisari miparnisari commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Two changes individually reviewable:

  • Refactor: Datastore engines now register their builders themselves. Each engine's new enginebuilder.go registers with the new leaf package pkg/cmd/datastore/dsconfig . The server-construction path no longer statically imports any engine.
  • The important change: Migratable engines can now list their migrations in order, and declare the earliest migration the current code can operate against; the datastore test suite's rewritten TestMigration uses this to apply every migration one at a time from an empty database, standing up a real SpiceDB server at that earliest supported revision and verifying via the riteSchema/ReadSchema API that each remaining migration preserves the schema written before it.

@miparnisari miparnisari changed the title Refactor migrations chore: refactor migration code and tests Jul 10, 2026
@github-actions github-actions Bot added area/cli Affects the command line area/datastore Affects the storage system area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) labels Jul 10, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 31135a8 Previous: cbdf06c Ratio
BenchmarkDatastoreDriver/cockroachdb-overlap-static/TestTuple/SnapshotReverseRead (github.com/authzed/spicedb/internal/datastore/benchmark) 34635145 ns/op 172560 B/op 20195 allocs/op 6805173 ns/op 172708 B/op 20195 allocs/op 5.09
BenchmarkDatastoreDriver/cockroachdb-overlap-static/TestTuple/SnapshotReverseRead (github.com/authzed/spicedb/internal/datastore/benchmark) - ns/op 34635145 ns/op 6805173 ns/op 5.09
BenchmarkDatastoreDriver/cockroachdb-overlap-insecure/TestTuple/SnapshotReverseRead (github.com/authzed/spicedb/internal/datastore/benchmark) 34055916 ns/op 172667 B/op 20196 allocs/op 6853237 ns/op 172793 B/op 20195 allocs/op 4.97
BenchmarkDatastoreDriver/cockroachdb-overlap-insecure/TestTuple/SnapshotReverseRead (github.com/authzed/spicedb/internal/datastore/benchmark) - ns/op 34055916 ns/op 6853237 ns/op 4.97
BenchmarkDatastoreDriver/cockroachdb-overlap-insecure/TestTuple/Create (github.com/authzed/spicedb/internal/datastore/benchmark) 5176124 ns/op 19541 B/op 281 allocs/op 2004369 ns/op 19414 B/op 281 allocs/op 2.58
BenchmarkDatastoreDriver/cockroachdb-overlap-insecure/TestTuple/Create (github.com/authzed/spicedb/internal/datastore/benchmark) - ns/op 5176124 ns/op 2004369 ns/op 2.58

This comment was automatically generated by workflow using github-action-benchmark.

Engine builders move from pkg/cmd/datastore into their engine packages,
registering themselves via the new leaf package pkg/cmd/datastore/dsconfig,
which also takes over the Config type and its options; pkg/cmd/datastore
re-exports everything, so callers are unaffected. Binaries link all in-repo
engines through a blank import of internal/datastore/engines.

This removes the static engine imports from the server-construction path,
so that shared test code can stand up a SpiceDB server without creating an
import cycle with the engines' own tests. Adding a new engine no longer
requires editing pkg/cmd/datastore.
Migratable engines can now list every migration they support, in order, and
declare the earliest migration the current code can operate against. The
datastore test suite's TestMigration uses this to apply every migration one
at a time from an empty database, standing up a SpiceDB server at the
earliest supported revision and verifying schema writes and reads through
its API across each remaining migration.
@miparnisari
miparnisari force-pushed the refactor-migrations branch from 31135a8 to 916899e Compare July 16, 2026 20:09
@miparnisari
miparnisari marked this pull request as ready for review July 16, 2026 20:11
@miparnisari
miparnisari requested a review from a team as a code owner July 16, 2026 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cli Affects the command line area/datastore Affects the storage system area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) Skip-Changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants