Skip to content

perf(migrations): build the trgm and scope indexes concurrently - #623

Merged
zkrebbekx merged 4 commits into
mainfrom
fix/611-concurrent-index-splits
Aug 10, 2026
Merged

perf(migrations): build the trgm and scope indexes concurrently#623
zkrebbekx merged 4 commits into
mainfrom
fix/611-concurrent-index-splits

Conversation

@zkrebbekx

Copy link
Copy Markdown
Owner

Closes #611.

The defect

Three released migrations build indexes on flexitype_attribute_value — always the largest table — with a plain CREATE INDEX, which takes a lock conflicting with every write and holds it for the whole build:

file index
000004_outbox_search idx_flexitype_attribute_value_trgm, ..._trgm_lower
000014_scoped_values idx_flexitype_attribute_value_scope
000021_plan_indexes the same trigram pair, rebuilt after 000018 dropped them

#595 established why "grandfathered" was the wrong answer: a deployment upgrading across the version still pays the stall, and bookkeeping records the version with no checksum, so correcting an applied migration is a no-op for those who have it and a fix for those who have not.

The tracker listed 000004 and 000014. 000021 belongs with them — it rebuilds the same pair, so it is the copy that survives a full upgrade chain.

Why none of them could be fixed in place

  • 000004 creates tables and 000014 alters two. A no-transaction file may not do either, because it is replayed in full after an interruption.
  • 000004 and 000021 must make the trigram build conditional: gin_trgm_ops does not exist where pg_trgm could not be installed. The only conditional form in SQL is a DO block, a DO block is a transaction, and CREATE INDEX CONCURRENTLY refuses to run inside one. 000021 says exactly this, at length, and takes the plain build as the lesser evil.

That last one is the real blocker, and it is not a SQL problem — it is a runner problem.

The fix

New file directive. -- +flexitype:requires-extension <name>. The runner checks pg_extension before applying, and where the extension is absent it skips the file without recording it. A database that installs the extension a week later applies the file on its next start; recording it as applied would mean the index is never built and nothing says so. The skip is announced on stderr, because a silently absent index reads as a planner that has simply got worse.

The name must be one bare identifier. A directive line carrying anything else would read as an extension nobody has, and skip the file for ever.

Two new migrations. 000041 builds both trigram indexes CONCURRENTLY, gated on pg_trgm. 000042 builds the scoped-value index CONCURRENTLY. Both are IF NOT EXISTS, so a database that already holds the index from 000004, 000014 or 000021 sees a no-op. The three originals keep their table DDL and their extension-creation attempt, and hand the builds over with a comment saying why.

000021's bespoke invalid-namesake reaper went with the builds it guarded. It existed because the runner's own reap covers CREATE INDEX CONCURRENTLY statements only, and those builds were not concurrent. Now they are, so the runner covers them — including the schema-qualification the in-file version had to hand-roll.

grandfatheredPlainIndexes drops from seven files to five.

Tests

TestExtensionGatedMigration (goconvey, Postgres):

  • The directive parses alongside no-transaction.
  • An installed extension (plpgsql, present everywhere) applies; an absent one skips rather than fails — failing would wedge every later migration on a managed provider.
  • Three malformed directive lines are refused.
  • On a fully migrated database, version 41 is recorded exactly when pg_trgm is installed, and both trigram indexes plus the scope index exist and are indisvalid.
  • The upgrade shape: delete the 41/42 rows while the indexes exist, re-run — clean, indexes untouched.
  • Down to 40 and back up: the revert drops all three, the re-apply rebuilds them. This is what catches a down file that still names an index its up file no longer creates.

TestEmbeddedMigrationDirectives now strips comments before looking for CONCURRENTLY — a file that merely names the migration its index moved to declares nothing to the runner.

Mutation-verified: recording a skipped file, accepting any directive text, and gutting the 000042 down file each fail 5 assertions.

Docs: docs/upgrades.md gains the directive, the reason it exists, and rule 7 — an index needing an optional extension goes in its own gated file rather than inside a DO block.

Green: full Go suite, golangci-lint clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DaPmWmgaGYqLsjW8DCU3P1

zkrebbekx and others added 4 commits August 10, 2026 18:22
Three released migrations built indexes on flexitype_attribute_value, the
largest table in the database, with a plain CREATE INDEX. That takes a
lock conflicting with every write and holds it for the whole build, so a
deployment upgrading across those versions stalled fleet-wide.

None of the three could be corrected in place. 000004 creates tables and
000014 alters two, so neither may be a no-transaction file. 000004 and
000021 also had to make the pg_trgm build conditional on the extension,
and the only conditional form in SQL is a DO block — which is a
transaction, and CREATE INDEX CONCURRENTLY refuses to run inside one.

So the condition moved out of SQL. A migration file may now declare
`-- +flexitype:requires-extension <name>`. The runner skips a file whose
extension is absent and does NOT record it, so a database that installs
the extension later applies the file on its next start, and the skip is
announced on stderr. Recording it instead would mean the index is never
built and nothing says so.

000041 builds both trigram indexes CONCURRENTLY, gated on pg_trgm. 000042
builds the scoped-value index CONCURRENTLY. Both are IF NOT EXISTS, so a
database that already has the index from 000004, 000014 or 000021 is
unaffected — bookkeeping records the version and no checksum, so
correcting an applied migration is a no-op for those who have it and a
fix for those who have not.

000021's bespoke invalid-namesake reaper went with the builds it guarded.
It existed because the runner's reap covers CREATE INDEX CONCURRENTLY
statements only, and these were not; now they are.

The directive test stripped comments before looking for CONCURRENTLY. A
file that merely names the migration its index moved to declares nothing
to the runner, and reading prose as a statement would force a directive
onto a purely transactional file.

Closes #611

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DaPmWmgaGYqLsjW8DCU3P1
Moving the scoped-value index into 000042 changes WHEN it is created on
a fresh database, which inverts an arbitrary planner tie. Three indexes
carry (entity, attribute) — entity_attr, scope and definition_entity —
and at this fixture's size they cost the same, so the winner is decided
by creation order rather than by anything the test cares about.

What must hold is the property the index exists for: the attribute is an
index CONDITION, never a per-row filter. Asserting a name made the test
a hostage to migration ordering.

Verified live: turning the attribute predicate into a non-indexable
expression fails it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DaPmWmgaGYqLsjW8DCU3P1
strings.Index can return -1, and the substring check for "Index Scan"
refused the BEST plan available: when the chosen index carries every
column the query touches, Postgres reports an Index Only Scan. The
assertion now keys on "Index Cond", which appears on both and is the
thing being asserted — the attribute is an index condition, not a
per-row filter.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DaPmWmgaGYqLsjW8DCU3P1
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DaPmWmgaGYqLsjW8DCU3P1
@zkrebbekx
zkrebbekx force-pushed the fix/611-concurrent-index-splits branch from 5932788 to 2202658 Compare August 10, 2026 08:52
@zkrebbekx
zkrebbekx merged commit 6a1dea6 into main Aug 10, 2026
10 checks passed
@zkrebbekx
zkrebbekx deleted the fix/611-concurrent-index-splits branch August 10, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Split 000004 and 000014 so their index builds can be concurrent

1 participant