Repository navigation
fix(memory-pgvector): serialize schema init under an advisory lock - #727
Merged
Merged
Conversation
initSchema created the vector extension, the schema, two tables and six indexes with no lock and no transaction, under a comment claiming IF NOT EXISTS made it safe to call repeatedly. It does make each statement idempotent, but not concurrency-safe: two sessions running the same statement race on the catalog and the loser raises 23505 rather than a no-op — on pg_extension for the extension, pg_namespace for the schema, pg_type for a table and pg_class for an index. That is a real deployment shape, not a theoretical one. A serverless deploy scaling 0 to N cold-starts N isolates that all initialize the same database, and the store's memoized ready() covers a single process. This is the defect @b4run/postgres-storage carried as #709. The whole pass now runs in one transaction holding pg_advisory_xact_lock keyed on the schema and prefix it builds, so concurrent callers queue and every one after the first no-ops through work already done. A failure rolls back rather than handing the caller a connection stuck in an aborted transaction, and the rollback cannot mask the original error. Proven against real Postgres: six concurrent initSchema calls on a virgin schema failed with 23505 on pg_extension_name_index before the change and pass after it, leaving exactly the two expected tables. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
blove
enabled auto-merge (squash)
September 18, 2026 02:31
blove
temporarily deployed
to
vercel-preview
September 18, 2026 02:31 — with
GitHub Actions
Inactive
This branch was successfully deployed
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.
Follow-up to #723, which fixed the same defect in
@b4run/postgres-storageas issue #709. I found this while fixing that one.The bug
initSchemacreated thevectorextension, the schema, two tables and six indexes with no lock and no transaction, under this comment:IF NOT EXISTSdoes make each statement idempotent. It does not make it concurrency-safe. Two sessions running the same statement race on the catalog and the loser raises23505rather than a benign no-op:pg_extensionfor the extension,pg_namespacefor the schema,pg_typefor a table,pg_classfor an index.That is a real deployment shape. A serverless deploy scaling from zero cold-starts several isolates that all initialize the same database, and the memoized
ready()inpgvector-store.tsonly covers one process.The fix
The whole pass runs in one transaction holding
pg_advisory_xact_lock, keyed on the schema and table prefix it builds. Concurrent callers queue, and each one after the first finds the work done and no-ops through it.A failure now rolls back rather than leaving the caller a connection stuck in an aborted transaction, and a failing rollback cannot mask the original error.
Unlike #723 there is no catalog fast path here. That one needed it because a cold start pays one lock per store component and a test pins that cost; this runs once per store instance behind an existing memo, so an unconditional lock costs one acquisition per process.
Tests
Gated on
B4_TEST_PGVECTOR=1, against realpgvector/pgvector:pg16: six concurrentinitSchemacalls on a virgin schema. Before the change it failed with23505onpg_extension_name_index; after it, all six resolve and the schema holds exactly the two expected tables, proving the winner did not leave the losers half-built.Validation
@b4run/memory-pgvectorfull suite with the gated lane: 5 files, 95 passed, 1 skipped@b4run/memory-pgvectortypecheck and lintpnpm lint,node scripts/check-docs.mjs,node scripts/check-changesets.mjsapi-reference-inventorybehavior-contract fingerprints: 252 passed🤖 Generated with Claude Code