Skip to content

fix(engine): drop per-bank vector indexes before bank delete data (#3485) - #3493

Open
xiaoxiaoHe-E wants to merge 1 commit into
vectorize-io:mainfrom
xiaoxiaoHe-E:fix/delete-bank-drop-indexes-first
Open

fix(engine): drop per-bank vector indexes before bank delete data (#3485)#3493
xiaoxiaoHe-E wants to merge 1 commit into
vectorize-io:mainfrom
xiaoxiaoHe-E:fix/delete-bank-drop-indexes-first

Conversation

@xiaoxiaoHe-E

@xiaoxiaoHe-E xiaoxiaoHe-E commented Aug 14, 2026

Copy link
Copy Markdown

Summary

Short-term fix for #3485 (per-bank partial vector indexes exhaust the Postgres lock table at ~2,100 banks — recall AND bank deletion both fail cluster-wide).

delete_bank previously dropped a bank's 3 per-bank vector indexes only after its delete transaction committed. With per-bank backends (pgvector/pgvectorscale/vchord), every bank adds 3 partial indexes to the shared memory_units table, and Postgres plans any statement against that table by taking a lock on all of its indexes. Past a few thousand banks the lock-manager shared-memory pool is exhausted, so every statement against memory_units fails — including the delete DML. Bank deletion was therefore part of the failure, not the recovery: the API could not delete banks to escape the wall.

This PR changes the order: the bank's per-bank vector indexes are dropped before the delete transaction starts.

Why this works

  • DROP INDEX (CONCURRENTLY) is a utility statement that only locks its own index plus the table — it still succeeds when the lock pool is exhausted, while the delete DML cannot plan.
  • Dropping first keeps bank deletion usable at any index count, and a deletion attempt whose data delete cannot plan yet still removes 3 indexes on every call, restoring an API-driven recovery path.
  • Still CONCURRENTLY on an autocommit connection (never inside a transaction), still wrapped in retry_with_backoff for the transient deadlock with concurrent index builds.
  • Behavior preserved: fact_type-scoped deletes and delete_bank_profile=False keep the indexes (only the full-delete path drops them).

Changes

  • hindsight_api/engine/memory_engine.py: fetch internal_id, drop the 3 per-bank vector indexes on an autocommit connection, then run the delete transaction; removed the post-commit drop.
  • hindsight_api/engine/db/ops_postgresql.py / hindsight_api/engine/retain/bank_utils.py: comments/docstrings updated to the new order.
  • tests/test_hnsw_indexes.py: new regression test test_delete_bank_drops_indexes_before_data asserting the indexes are dropped while the bank's rows and profile still exist.
  • tests/test_bank_lifecycle_deadlock_retry.py: docstring updated.

Notes

  • This is the short-term fix; the durable direction (global/partitioned indexes so index count is independent of bank count, plus a catalog-only recovery path) is discussed in the issue.
  • Verified: 38 tests green (HNSW lifecycle incl. the new regression test, bank-lifecycle deadlock retry, non-SQL store deletion, extension bank tables, bank stats cache), ruff + ./scripts/hooks/lint.sh clean.

Refs #3485

Bank deletion previously dropped a bank's per-bank vector indexes only
after its delete transaction committed. With per-bank backends, every
bank adds 3 partial indexes to the shared memory_units table, and
Postgres plans any statement against it by locking ALL of its indexes;
past a few thousand banks the lock-manager pool is exhausted and every
statement against memory_units fails — including the delete DML, so the
API could no longer delete banks to recover.

Drop the indexes first (still CONCURRENTLY, on an autocommit connection,
still retried with backoff): DROP INDEX only locks its own index plus the
table, so it keeps working when the pool is exhausted, restoring an
API-driven recovery path. Fact-type-scoped deletes and
delete_bank_profile=False keep the indexes, as before.

Refs vectorize-io#3485

@koriyoshi2041 koriyoshi2041 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested the exact head 9680107 locally on macOS/Python 3.11 with embedded PostgreSQL: both HNSW delete-order tests passed, including the new before-data regression, and both bank lifecycle deadlock retry tests passed. The full-delete-only guard also preserves the scoped-delete behavior. The recovery ordering looks sound to me.

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.

2 participants