Switch migrator to orchestrator and manage downstream migrations#2094
Open
iplay88keys wants to merge 6 commits into
Open
Switch migrator to orchestrator and manage downstream migrations#2094iplay88keys wants to merge 6 commits into
iplay88keys wants to merge 6 commits into
Conversation
Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
…s-improvements-pt-3 Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the database migration system from a hardcoded “core/vector” runner into a general multi-source orchestrator ([]migrations.Source), enabling downstream consumers to register additional migration tracks while keeping ordering, prechecks, and compensating rollback centralized.
Changes:
- Introduces
migrations.Source+migrations.RunUp(ctx, url, []Source)orchestration with upfront prechecks, schema scoping, collision detection, and multi-source compensating rollback. - Updates app startup and test helpers to pass built-in sources + optional downstream-registered sources instead of injecting a custom runner.
- Expands/updates migration tests to cover multi-source ordering, rollback across N sources, schema scoping, and validation behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| go/core/test/upgrade/roundtrip_test.go | Updates upgrade test to call the new context-aware RunUp with BuiltinSources. |
| go/core/pkg/migrations/runner.go | Replaces the old runner with a Source-based orchestrator, schema scoping, collision validation, and generalized rollback/error handling. |
| go/core/pkg/migrations/runner_test.go | Adapts existing tests and adds new coverage for multi-source orchestration, schema scoping, prechecks, and validations. |
| go/core/pkg/app/app.go | Changes app.Start to accept extraSources []migrations.Source and runs built-ins + extras via the orchestrator. |
| go/core/internal/dbtest/dbtest.go | Updates DB test helper to use the new RunUp(ctx, url, BuiltinSources(...)) signature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
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.
Description
Restructures the DB migration machinery from two hardcoded tracks (
core,vector) into a single orchestrator that applies an ordered list ofSourcedescriptors. Downstream consumers register their ownSources instead of shipping a duplicate runner, so track ordering and failure handling stay centralized in one place.This is the first step toward broader migration rollback tooling.
What changed
Sourcedescriptor + orchestrator: newSource{Name, Schema, TrackingTable, FS, Dir, PreCheck}andRunUp(ctx, url, []Source).BuiltinSources(vectorEnabled)builds the built-in set:corealways,vector(with the pgvectorPreCheck) when enabled.Sourceis registered rather than a flag inside the runner, sovectorEnabledno longer leaks into the orchestrator contract.PreChecks run before any source is applied, preserving the existing fail-fast pgvector behavior (verified beforecoreruns).prevVersion == 0guard (don't roll a freshly-installed source back to 0). This generalizes today's "roll core back if vector fails". Failed compensating rollbacks are surfaced viaerrors.Joinrather than log-only.Source.Schema != ""): a non-empty schema scopes the track. The orchestrator creates the schema (CREATE SCHEMA IF NOT EXISTS), pinssearch_pathon the DSN, and scopes the tracking table viamigratepgx.Config.SchemaName. Schema names are validated (lowercase identifier, so the quotedCREATE SCHEMAand the case-foldedsearch_pathcan't diverge) and quoted. Built-in tracks useSchema == "", which resolves to the connection default (public), unchanged from today.(schema, tracking table), including the case whereSchema == ""resolves to a schema another source names explicitly (checkResolvedSchemaCollisions, run with a live connection).app.Start'sMigrationRunnerDI callback is removed in favor ofextraSources []migrations.Source; built-in sources run first, then downstream extras in registration order.Breaking change
app.Start(getExtensionConfig, migrationRunner)becomesapp.Start(getExtensionConfig, extraSources []migrations.Source). TheMigrationRunnertype is removed; downstream consumers register sources instead, and migrate to that in the same release.cmd/controller/main.gopassesnil(a valid empty source list).Behavior preserved
Same tracking tables (
schema_migrations,vector_schema_migrations), same ahead-of-binary compatibility mode, same dirty-and-ahead hard error, sameErrNoChangeno-op, sameprevVersion == 0rollback-skip guard, same fail-fast pgvector check.golang-migrate's pgx driver still checks out a single dedicated connection for the session-scoped advisory lock; the pool is not capped because the driver pins lock and migration work to that connection.Testing
prev == 0guard), schema-scoped source, precheck-runs-before-any-apply, source-collision validation, schema-name validation,withSearchPath, and empty/nil no-op.core/vectorscenarios adapted to the new signature; all prior behaviors retained.go build,go vet, and the container-free tests pass; the testcontainers suite requires Docker.