fix(storage): unblock Postgres boot — disable FK creation during AutoMigrate (RAN-49)#32
Merged
Conversation
…res boot (RAN-49)
After RAN-21 made trace identity tenant-scoped — replacing the single-column
unique on traces.trace_id with a composite (tenant_id, trace_id) — Postgres
rejected the auto-generated spans.trace_id FK at CREATE TABLE time with
SQLSTATE 42830 ("there is no unique constraint matching given keys for
referenced table"), aborting boot on every fresh Postgres deployment.
The model already declared `constraint:false`, but gorm v2's relationship
parser does not honor that tag — which is why the MySQL migrate path was
explicitly dropping fk_traces_spans / fk_traces_logs post-AutoMigrate. The
reliable cross-driver suppression is the gorm.Config flag, so set
DisableForeignKeyConstraintWhenMigrating=true on the global session.
This matches the existing async-ingest design (spans/logs may land before
their parent trace) and removes the silent SQLite-hiding-Postgres regression
class: SQLite did not validate FK targets at CREATE TABLE time, so the
SQLite-only migrate tests passed while Postgres failed.
- factory.go: set DisableForeignKeyConstraintWhenMigrating; reword the MySQL
drop comments to reflect they are now legacy-only.
- pg_integration_test.go: add TestPG_AutoMigrate_FromEmpty_NoSpansTracesFK
to lock in the regression class — boots an empty Postgres 16 container,
asserts AutoMigrateModels succeeds, then verifies no FK exists from
spans/logs back to traces.
Verified: full default test suite green; 8/8 PG integration tests pass
against a real Postgres 16 container (the unrelated pg_trgm planner-stats
flake is pre-existing and reproduces on the prior tree).
Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
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
1 participant
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.
Summary
DisableForeignKeyConstraintWhenMigrating=trueon the global GORM config so AutoMigrate never emitsspans.trace_id → traces.trace_id(or the equivalent onlogs). After RAN-21 made trace identity tenant-scoped (composite(tenant_id, trace_id)unique), Postgres rejected the auto-emitted FK at CREATE TABLE time with SQLSTATE 42830 and the binary aborted boot on every fresh Postgres deployment.constraint:false, but gorm v2's relationship parser does not honor that tag — which is exactly why the MySQL migrate path was already droppingfk_traces_spans/fk_traces_logspost-AutoMigrate. This change replaces the cross-driver hack with the documented config flag.tenant_id) and at the composite unique ontraces.TestPG_AutoMigrate_FromEmpty_NoSpansTracesFK(integration tag) — boots a fresh Postgres 16 container, runsAutoMigrateModels, and asserts no FK exists fromspans/logsback totraces. Locks in the regression class so SQLite can no longer hide a Postgres DDL failure (SQLite does not validate FK targets at CREATE TABLE time, which is why the regression slipped past CI).Verification
go vet ./...— cleango build ./...— cleango test -race ./...— all default suites passgo test -race -tags=integration -timeout=10m ./internal/storage/...against Docker-backed Postgres 16:TestPG_AutoMigrate_FromEmpty_NoSpansTracesFK— PASSTestPG_ILIKE_CaseInsensitiveSearch/TestPG_Bytea_CompressedTextRoundTrip/TestPG_PgTrgm_IndexCreated/TestPG_VacuumAnalyze_OutsideTx/TestPG_PurgeLogsBatched_LargeVolume/TestPG_PurgeTracesBatched_OrphanSpanSweep_NOT_IN/TestPG_AutoMigrate_BlobTypesBecomeBytea— all PASSTestPG_PgTrgm_SubstringQueryUsesGIN— failing both before and after this change (planner picks Seq Scan on tiny seed; pre-existing flake, unrelated to RAN-49)failed to migrate database: ERROR: there is no unique constraint matching given keys for referenced table "traces" (SQLSTATE 42830)with the offending DDLCONSTRAINT "fk_traces_spans" FOREIGN KEY ("trace_id") REFERENCES "traces"("trace_id"). With the fix applied, the migrator completes cleanly.Acceptance criteria mapping
./otelcontextboots cleanly against an empty Postgres 16 DB — verified via integration test (which exercisesAutoMigrateModelsagainst a fresh Postgres 16 container).spans.trace_idFK references match an actual unique constraint after migrate — by removing the FK entirely, the schema is internally consistent on every driver. Async ingest semantics already required no FK.internal/storage/...exercises a full migrate from empty —TestPG_AutoMigrate_FromEmpty_NoSpansTracesFK.Test plan
go test -race -tags=integration -run TestPG_AutoMigrate_FromEmpty -timeout=10m ./internal/storage/...against a Docker-equipped host and confirms PASS.Resolves RAN-49.