Skip to content

fix(core): create vector storage at initialization, never at runtime - #1656

Merged
phernandez merged 4 commits into
mainfrom
vector-schema-precheck
Oct 6, 2026
Merged

phernandez merged 4 commits into
mainfrom
vector-schema-precheck

Conversation

@phernandez

@phernandez phernandez commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Problem

Every embedding job builds a fresh PostgresSearchRepository, and its first vector call ran CREATE EXTENSION / CREATE TABLE IF NOT EXISTS / CREATE INDEX IF NOT EXISTS for the chunk and embedding tables. CREATE INDEX IF NOT EXISTS takes a SHARE lock on the table before it sees the index exists, so under concurrent writers every chunk/embedding write queued behind it. In production one tenant had ~28k index_embeddings* jobs time out (asyncpg 30s) in two days; pg_stat_activity showed 9 of these CREATE INDEX statements and ~50 writes waiting on relation locks.

Fix: no DDL at runtime

  • Runtime (_ensure_vector_tables, used by search and vector sync) only binds the vector adapter. PgVectorIndex.upsert/search/delete* no longer call initialize(); PgVectorIndex.initialize() is now a runtime no-op. A database without vector storage fails on its first query.
  • Initialization (init_search_index() → _create_vector_storage() → PgVectorIndex.create_storage()) creates the chunk and embedding tables. Local startup already calls this from db.run_migrations; cloud will call it from its once-per-tenant-per-process schema hook (companion cloud PR).
  • create_storage() reads the catalog first (pg_class/pg_attribute/pg_extension, scoped to current_schema()), so initializing an already-initialized database — every new worker — takes no locks on our tables. Only a fresh or outdated database runs DDL.

Tests

  • Real Postgres: with another connection holding ROW EXCLUSIVE on both vector tables, runtime binding and re-initialization both finish within 5s. On main this times out, the production failure.
  • Real Postgres: a layered search_path (tenant, public) with tables only in public still creates the tenant schema's own tables (Codex P1).
  • Unit: runtime upsert/delete issue no DDL; current storage needs exactly one catalog read.
  • Tests that relied on lazy runtime creation now call init_search_index().
  • Postgres repository/services: 1750 passed; SQLite: 1773 passed; just lint, just typecheck clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF

Every embedding job builds a fresh search repository, so its per-instance
"tables ready" flags start false and setup re-ran CREATE TABLE/INDEX IF NOT
EXISTS on every job. CREATE INDEX IF NOT EXISTS takes a SHARE lock before it
sees the index exists, so under concurrent writers it queued every chunk and
embedding write behind it until queries hit the 30s timeout (one cloud
tenant: ~28k embedding job timeouts in two days).

Setup now reads the catalog first (to_regclass/pg_attribute, no table locks)
and only runs DDL when the chunk table, the embeddings table, or their
indexes are missing or out of date.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF
Signed-off-by: phernandez <paul@basicmachines.co>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-06T01:52:02.140264Z 1910011 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7d27cbafa9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +334 to +336
"SELECT to_regclass('search_vector_chunks') IS NOT NULL "
"AND to_regclass('idx_search_vector_chunks_project_entity') "
"IS NOT NULL"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep readiness probes in the current schema

When a deployment uses a layered search path such as tenant,public and the tenant schema is being bootstrapped while public already contains these objects, to_regclass searches the entire path, so this probe evaluates true for the public table and index. Unlike the previous unconditional CREATE TABLE/CREATE INDEX, which targets the first writable schema, setup now skips tenant-local creation and later unqualified vector writes fall through to the public tables, mixing search state across schema boundaries. Verify that both OIDs belong to current_schema() before taking the fast path.

AGENTS.md reference: AGENTS.md:L392-L396

Useful? React with 👍 / 👎.

@phernandez phernandez changed the title Skip vector DDL when the storage already exists fix(core): skip vector DDL when the storage already exists Oct 6, 2026
phernandez and others added 2 commits October 5, 2026 20:07
to_regclass searches the whole search_path, so with a layered path like
`tenant, public` the vector tables in public satisfied the probe and setup
skipped creating the tenant schema's own. The probes now read pg_class for
current_schema() only, matching where unqualified DDL and writes land.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF
Signed-off-by: phernandez <paul@basicmachines.co>
Runtime code no longer issues DDL. Search and vector-sync paths only bind
the vector adapter; PgVectorIndex upsert/search/delete no longer call
initialize(). Storage is created by init_search_index() -> create_storage(),
which local startup already calls and cloud will call once per tenant per
process alongside migrations. A database missing vector storage now fails
on its first query instead of creating tables mid-job.

create_storage() still reads the catalog first, so initializing an already
initialized database (every new worker) takes no locks on the tables.

Tests that relied on runtime creation now initialize the database first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF
Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez phernandez changed the title fix(core): skip vector DDL when the storage already exists fix(core): create vector storage at initialization, never at runtime Oct 6, 2026

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2b89294ab5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +129 to +132
existing_dimensions = await self._existing_dimensions(session)
storage_missing = existing_dimensions is None
source_hash_missing = (
existing_dimensions is not None and not await self._has_source_hash_column(session)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep mismatch checks in the target schema

With a layered search path such as tenant,public, when the tenant embedding table is absent but public.search_vector_embeddings has different dimensions or lacks source_hash, the new local _read_storage probe correctly takes the creation path, but _existing_dimensions() and _has_source_hash_column() still resolve the public table. The resulting mismatch reaches the unqualified DROP TABLE IF EXISTS search_vector_embeddings, which resolves to and deletes the public table rather than creating isolated tenant storage. Fresh evidence beyond the earlier comment is that the fast probe is now schema-scoped while these follow-up probes remain search-path-scoped; scope them to current_schema() as well.

Useful? React with 👍 / 👎.

create_storage() already reads pg_class/pg_attribute for current_schema();
the dimension and source_hash checks used separate lookups that searched the
whole search_path, so a mismatched same-named table in another schema could
be treated as ours and dropped. Reuse the schema-local read and delete the
two path-wide helpers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF
Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez
phernandez merged commit dcbf55d into main Oct 6, 2026
51 of 52 checks passed
@phernandez
phernandez deleted the vector-schema-precheck branch October 6, 2026 02:35
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.

1 participant