Skip to content

fix(hnsw): search the index while it lags instead of falling back to a full table scan - #30989

Merged
XuPeng-SH merged 9 commits into
matrixorigin:mainfrom
cpegeric:issue_30987
Oct 27, 2026
Merged

XuPeng-SH merged 9 commits into
matrixorigin:mainfrom
cpegeric:issue_30987

Conversation

@cpegeric

@cpegeric cpegeric commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #30987

Related:

What this PR does / why we need it:

#30117 (fix for #29889) made HNSW fall back to a brute-force full table scan whenever it could not prove the async index covers the read. That happens:

  • after every committed write, until ISCP applies it;
  • for the rest of any transaction that has written the source table;
  • when build_ts isn't strictly before the read snapshot;
  • on any lookup error.

On a large table (for example 88M × 768-D vectors) that makes a scan of every vector the common plan. See #30987.

This PR removes #30117's HNSW-only parts, so HNSW behaves as it did before #30117 (the #30127 baseline):

  • the plan-time coverage gate (hnswIndexCoversSnapshot);
  • the runtime searched-generation validation and its brute-force retry;
  • the HNSW coverage hook, and HNSW's prepared-plan coverage dependency (prepared HNSW plans are reusable again);
  • the compile and frontend retry plumbing (HnswSnapshotFallbackKey).

ErrVectorIndexSnapshotChanged stays registered, so a peer still on the older version reports it clearly during a rolling upgrade. #30117's non-HNSW parts (fulltext2 JSON coverage, GC and logtail tests) are unchanged. No storage-engine code is changed.

Behaviour

Without mode=force, the plan is a Vector Index Scan. It searches the usearch index for a fixed number of candidates (set by LIMIT, OFFSET and the filter mode) and joins them to the source table by primary key. Distances and ordering come from the vectors stored in the index. Only BY RANK WITH OPTION 'mode=force' scans the table.

Until ISCP applies a write to the index and the CN's index cache loads that version:

  • a row not in the index (a new insert) is not returned;
  • a row deleted from the source but still in the index is dropped by the join, so a page can have fewer than LIMIT rows;
  • a row whose vector changed is ranked and returned with the distance of its indexed vector.

This holds for the reading transaction's own uncommitted writes and for committed writes ISCP has not applied. mode=force computes distances from the current vectors.

This is the decision recorded as WARNING: TOXIC at the mode=force branch in pkg/sql/plan/apply_indices_hnsw.go, and documented in docs/cn/vector_search.md ("HNSW:近似检索与异步可见性").

Tests

BVT vector/issue_29889_hnsw_committed_read:

  • Plan locks: EXPLAIN must show Vector Index Scan and no Sort Key: l2_distance in every state fix: prevent stale HNSW top-k results #30117 fell back in:
    • after CREATE INDEX;
    • after committed writes;
    • inside a transaction after an insert, an update and a delete;
    • with a filter.
    • mode=force must plan the brute-force sort. Against the base with fix: prevent stale HNSW top-k results #30117, 7 of the 9 index locks fail.
  • Each documented statement, checked from SQL: each index query is paired with mode=force as the oracle, and the transaction's uncommitted writes hold the index behind deterministically.
    • an inserted row is not returned (force returns it);
    • with 15 rows deleted, LIMIT 2 returns 0 rows while 5 remain (force returns 16, 17);
    • an updated row is returned with its indexed distance (force uses the current vector).
    • When a committed row becomes visible through the index is not asserted. That depends on ISCP and on each CN's index cache, which is eventually consistent.

Unit tests: pkg/vectorindex/hnsw/... pass.

…a full table scan

matrixorigin#30117 made HNSW fall back to a brute-force scan of the source table whenever
the async index could not be proven to cover the read: after every committed
write until ISCP applied it, for the rest of any transaction that wrote the
table, and on any lookup error. On a large table (88M vectors) that turns the
common plan into a full scan.

HNSW now always searches the index, as before matrixorigin#30117 and as IVF-PQ does; only
BY RANK mode=force scans the table. Removed: the plan-time coverage gate, the
runtime searched-generation validation and its brute-force retry, the coverage
hook and prepared-plan coverage dependency, and the compile/frontend retry
plumbing that only served that fallback. The error code stays registered so a
peer on the older version still reports it clearly during a rolling upgrade.

The transaction's own deletes and updates are exact: the HNSW candidate budget
is at least LIMIT plus the rows the transaction deleted or updated in the
source table, counted from its workspace through the new optional
engine.TransactionLocalDeletesProvider. Commits by other transactions that ISCP
has not applied yet are eventually consistent; a query in that window can
return fewer than LIMIT rows by design.

BVT: the matrixorigin#29889 case waits for the committed row to become visible, asserts
the index plan, and checks mode=force; 15 nearest rows deleted in the
transaction under LIMIT 1 return row 16.

Fixes matrixorigin#30987

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@cpegeric

Copy link
Copy Markdown
Contributor Author

Remaining BVT results on 99316162c8 (fresh instance, -g -n): vector 100%, pessimistic_transaction/vector 100%, ddl/alter_copy_add_index 100%.

HNSW is an approximate index whose top-k can miss true nearest neighbours
even when it is up to date, and it is maintained asynchronously by ISCP: until
a commit is applied, new rows and new vectors are not searchable and rows other
transactions deleted or updated can shorten the result below LIMIT. Document
that contract, the transaction's own deletes and updates being accounted for,
and mode=force as the exact full-table scan with its cost.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cpegeric

Copy link
Copy Markdown
Contributor Author

@XuPeng-SH, documented in 22686b1bfe: docs/cn/vector_search.md, new section "HNSW:近似检索与异步可见性". It states:

  • HNSW is approximate: its top-k can miss true nearest neighbours even when the index is fully up to date, and recall depends on build and search parameters.
  • It is maintained asynchronously by ISCP: until a commit is applied, new rows and new vectors are not searchable, and rows other transactions deleted or updated can shorten the result below LIMIT. This is the expected eventually consistent behaviour, and queries reflect those commits once ISCP catches up.
  • The transaction's own deletes and updates are accounted for. Its own inserts are not searchable until they are committed and applied.
  • BY RANK WITH OPTION 'mode=force' is the exact answer: it scans every row of the table, so its cost grows with rows × dimension. The planner never switches to it on its own.

On "mistaking an underfilled ANN result for a complete top-k": an HNSW result was never a guaranteed-complete top-k. Approximation is the trade the index makes for speed, so a result that matches brute force exactly was never part of its contract, with or without lag. A rule that a top-k must equal the brute-force answer would reject HNSW itself, not just its lag window. The exact contract is mode=force, and it is now documented as such.

@fengttt fengttt 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.

Is this PR only releasted to HNSW? What about IVFFLAT? What about fulltext2?

@cpegeric

Copy link
Copy Markdown
Contributor Author

Is this PR only releasted to HNSW? What about IVFFLAT? What about fulltext2?

@fengttt regression because one of the PR fallback to brute force search with HNSW. Other index not affected. Revert is for HNSW only.

@XuPeng-SH

XuPeng-SH commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

mo-bot review · mo-bot-local · done

Commit: 92ae83da942cf262d02f7d10a81ab34ab3677f0e

Review completed. Read the assessment.

@XuPeng-SH XuPeng-SH 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.

mo-bot review · mo-bot-local · review

Commit: 92ae83da942cf262d02f7d10a81ab34ab3677f0e

Assessment: request_changes

Reviewed all 24 changed files and supplied discussion. Removing lag-triggered full scans reduces coverage I/O and keeps responsibility in existing owners. However, stale index scores still replace scalar expressions evaluated against current rows. The earlier scalar-correctness concern remains; the candidate-widening concern is resolved.

Model: gpt-6-astra / xhigh

Session: 01a13dcf-1586-7a01-9000-d45e20995cf8 · slot 1 · generation 1

  • P1 pkg/sql/plan/apply_indices_hnsw.go:75: Build an HNSW index containing (1,'[0,0]'), then execute BEGIN; UPDATE t SET v='[100,100]' WHERE id=1; SELECT id,v,l2_distance(v,'[0,0]') AS d FROM t ORDER BY d LIMIT 3.
    Impact: Removing the coverage gate permits the PK join to return the updated vector while the planner substitutes its old indexed distance into the projection and ordering. This produces incorrect scalar values independently of approximate recall. Retain ANN candidate selection, but recompute distances from the joined rows and order those candidates consistently; update the regression expectations accordingly.
    Evidence: The current-head frontend probe returned id=1, v=[100,100], d=0; mode=force returned d=141.42135620117188. Both queries returned all three rows. Restoring all changed Go inputs to the merge base made both return the correct distance. See hnsw-scalar-probe.log and checks/1793019733914074322-2.json. The committed SQL fixture also explicitly expects a stale scalar at issue_29889_hnsw_committed_read.result:194.

Validation:

  • Isolation preflight passed. Verified 329 native build inputs and 52 retained outputs against the current source and repository-owned build receipt.

  • Current-head SQL reproduction completed with the demonstrated assertion failure. The base-overlay runtime control passed; its overall command failed because auxiliary go vet could not enter an overlay-only deleted directory.

  • HNSW plugin compile, idxcron, plan, runtime and search package tests passed. Focused planner coverage and compiler retry/lifecycle tests passed.

  • git diff --check passed; source remains unchanged.

  • review-evidence.json reconciles eight command/source/exit receipts, retained logs, overlay hashes and native provenance. The final audit passed with receipt 1793020092914321451-2.json.

Limitations:

  • Artifacts are under /home/xupeng/.local/state/mo-bot/jobs/732cef1a7f6ec066afbb94ad/attempt-1.

  • The broader HNSW suite timed out after 120 seconds during the unchanged TestBuildSingleThreadF64 recall workload; the whole suite is not counted as passing.

  • The runtime reproduction used one CN and CPU execution. Full BVT, race and multi-CN suites were not rerun. Frozen CI reports success but supplies no per-case logs; local_validation is empty.

  • No comparative performance benchmark was run. Expected savings follow from removing source-watermark checks and whole-table distance evaluation.

@cpegeric

Copy link
Copy Markdown
Contributor Author

@XuPeng-SH (mo-bot P1, apply_indices_hnsw.go:75): this is the same finding answered on 2026-10-24 (comment), with no new evidence. It is a recorded design decision and will not be changed in this PR.

  • The reported value is not new behaviour. An HNSW plan returns the distance of the vector stored in the index. It behaves the same way after a committed UPDATE that ISCP has not yet applied, and it did so before fix: prevent stale HNSW top-k results #30117. The in-transaction case is just one instance of index lag.
  • The fixture is intentional. issue_29889_hnsw_committed_read.result:194 records that documented behaviour next to the exact mode=force answer.
  • Restoring the merge base "fixes" it only by bringing back the regression. The base contains fix: prevent stale HNSW top-k results #30117's coverage gate, which makes HNSW fall back to a full table scan. Removing that fallback is the whole purpose of this PR.
  • Recomputing distances on the joined rows would not make the result correct:
    • a row whose new vector moved close to the query is not among the index candidates, so it is still missing;
    • re-sorting a stale candidate set produces a result that is neither the index answer nor the exact answer;
    • it adds a per-row distance evaluation that the index exists to avoid.
  • The contract is documented in docs/cn/vector_search.md under "HNSW:近似检索与异步可见性":
    • HNSW is approximate and maintained asynchronously;
    • BY RANK WITH OPTION 'mode=force' is the exact path;
    • the planner never switches to it on its own.

Under the mo-self-review developer-authority rule, please treat this item as closed rather than blocking.

@XuPeng-SH

XuPeng-SH commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

mo-bot review · mo-bot-local · done

Commit: 92ae83da942cf262d02f7d10a81ab34ab3677f0e

Review completed. Read the assessment.

@XuPeng-SH XuPeng-SH 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.

mo-bot review · mo-bot-local · review

Commit: 92ae83da942cf262d02f7d10a81ab34ab3677f0e

Assessment: approve

Reviewed all 24 changed files, surrounding callers and all 13 discussion items. Removing HNSW freshness-triggered fallback is coherent with the documented asynchronous ANN contract. The earlier stale-score observation is real, but current documentation explicitly accepts indexed scores for both local and committed updates; mode=force provides current-vector results. The base control restores the deliberately removed fallback and does not establish a separate contract violation. Uncapped candidate widening is removed; fulltext2 coverage and ordinary retry behavior remain intact.

Model: gpt-6-astra / xhigh

Session: 01a13dcf-1517-7f90-b3f9-c5f9f9e7031b · slot 0 · generation 1

Validation:

  • Saved discussion-assessment.json and review-map.json under /home/xupeng/.local/state/mo-bot/jobs/e30b32e5c3e0808a0e78614b/attempt-1. Every nonempty discussion key is accounted for exactly once.

  • Independently reconciled eight same-head execution receipts, retained logs, overlay inputs, 329 native build inputs and 52 outputs. retained-audit.json records provenance; current audit receipt 1793023233985603328-4.json exited 0.

  • Reused passing HNSW plugin compile, idxcron, plan, runtime and search tests, plus focused planner and compiler retry/lifecycle tests from job 732cef1a7f6ec066afbb94ad at this exact head.

  • Verified the retained frontend probe observed indexed distance 0 versus mode=force distance 141.42135620117188 after updating the vector. Its exit 1 reflects an assertion contrary to the documented indexed-score contract; it is not counted as a passing test.

  • Reconciled same-head CI success metadata for build, UT, SCA and multi-CN BVT. Final artifact/discussion audit passed with receipt 1793023479935581445-5.json; source remains unchanged.

Limitations:

  • The retained broader HNSW run timed out in unchanged TestBuildSingleThreadF64; only its individually passing plugin packages are credited.

  • The base-overlay runtime control passed, but its overall command exited 1 because auxiliary vet could not enter an overlay-only directory.

  • CI lacks per-case BVT logs; separate linked-issue bodies were not supplied. No fresh product, race, multi-CN or GPU tests were run.

  • Performance gains are structural: fewer coverage lookups and lag-triggered full scans, without transaction-delete-driven widening. No comparative speedup measurement is claimed.

@mergify

mergify Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • ✅ Entered queue — 2026-10-27 01:42 UTC · Rule: main · triggered by rule Automatic queue on approval for main
  • 🚫 Left the queue — 2026-10-27 01:42 UTC · at 92ae83da942cf262d02f7d10a81ab34ab3677f0e

This pull request spent 9 seconds in the queue, with no time running CI.

Reason

The pull request can't be updated

GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/ci-scope-check.yml without workflows permission
GitHub only lets Mergify write to .github/workflows/ with the workflows permission. An organization owner can accept Mergify's pending permissions at https://dashboard.mergify.com/orgs/matrixorigin/repositories.

Hint

You should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue, you can requeue the pull request, without updating it, by posting a @mergifyio queue comment.

Tick the box to put this pull request back in the merge queue (same as @mergifyio queue).

  • Requeue this pull request

This branch was previously deployed

1 inactive deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/enhancement size/XL Denotes a PR that changes [1000, 1999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants