[cpegeric]

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:

• [Bug]: ISCP sync iteration is an unbounded work unit: no per-iteration events/bytes/time cap, whole-interval atomic watermark, full-interval replay #26973: ISCP latency. [Bug]: Committed rows not visible via HNSW index >10s after commit at 100k rows (single CN), with repeated ISCP timeouts #29889's delay comes from ISCP iteration and flush timeouts in its log, and is tracked there.
• [Bug]: Committed rows not visible via HNSW index >10s after commit at 100k rows (single CN), with repeated ISCP timeouts #29889
• [Bug]: FULLTEXT2 index lags the base table (ISCP latency): deleted, stale or too few rows until it syncs #30853

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.

[qodo-code-review]

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]

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

[cpegeric]

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

[XuPeng-SH]

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

Commit: 030d171ab8656d03c367dcb1bac3a8b38b861bd8

Review completed. Read the assessment.

[XuPeng-SH]

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

Commit: 030d171ab8656d03c367dcb1bac3a8b38b861bd8

Assessment: request_changes

Reviewed the complete 25-file delta and supplied comments. Removing lag-triggered full scans and consolidating workspace counting under the engine are reasonable design choices, but candidate widening does not preserve the promised semantics for transaction-local vector updates. A real SQL probe confirmed incorrect distance values.

Model: gpt-6-astra / xhigh

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

• P1 pkg/vectorindex/hnsw/plugin/search/search.go:172: Build an HNSW index containing row (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: Increasing the candidate count cannot discard a vector update that preserves its primary key. The PK-only base-table join matches the new row, while the planner substitutes the old index score into the projected distance and ordering. Handle locally updated candidates before reusing their scores, and add a result-level update regression test.
  Evidence: TestReviewHNSWLocalUpdate exercised the real frontend with three indexed rows. EXPLAIN confirmed HNSW execution; the query returned id=1, v=[100,100], d=0. The same query with mode=force returned d=141.42135620117188. This is incorrect scalar evaluation, not approximate recall. Retained hnsw-update-probe.log and checks/1792801618090101549-3.json record the assertion failure at the assigned clean HEAD.

Validation:

• Isolation preflight passed: checks/1792801346884596974-2.json.

• Verified retained native artifacts through the repository provenance owner, with matching native source inputs: native-input-ledger.json, native-provenance.log, and checks/1792801447411351636-3.json.

• Ran the test-only overlay probe with -count=1, -timeout=180s and the supplied test-exec. It completed with the demonstrated result mismatch; SQL output, control query and fixture shutdown are retained.

• Reconciled receipts, source fingerprints, overlay hashes and logs in evidence-reconciliation.log. Source remains unchanged; review coverage is recorded in review-audit.json.

• The frozen evidence reports successful build, UT, SCA and BVT checks at this HEAD; local_validation contains no retained worker gates.

Limitations:

• Diagnostic paths above are relative to /home/xupeng/.local/state/mo-bot/jobs/75994f03945b8004d97c8a62/attempt-1.

• The local reproduction used one CN and CPU execution. Broad suites, race tests and multi-CN tests were not rerun; supplied CI lacks per-case logs.

• No comparative performance benchmark was run. Source analysis shows fewer coverage lookups/full scans, offset by workspace scanning and wider candidate sets for writable transactions.

• Separate linked-issue bodies were not supplied; their requirements were assessed through the frozen PR evidence.

[gouhongshen]

Codex automated review

This PR cleanly reverts the HNSW-only snapshot-coverage machinery from #30117: HNSW always searches the index (with mode=force as the only brute-force path), and a transaction's own deletes are compensated by widening the candidate limit through the new optional engine capability engine.TransactionLocalDeletesProvider. I verified the removals leave no dangling references (HnswSnapshotFallbackKey, hnswIndexCoversSnapshot, the HNSW coverage hook, validateSearchedGeneration, the compile/frontend retry plumbing, and the deleted tests are all consistently excised, and ErrVectorIndexSnapshotChanged intentionally stays registered with no producer for rolling-upgrade peers), that fulltext2's separate JSON coverage path is untouched, that TransactionLocalDeletedRows correctly counts row-level and S3-tombstone DELETE entries for the source table, fails closed on SOFT_DELETE_OBJECT and ALTER, sums partition members, and skips read-only workspaces and historical snapshot reads, and that a writable workspace keeps the query on the ingress CN via executionRequiresCurrentCN so the workspace count is observable. Widened limits flow correctly into both iterative and non-iterative searches, and the new BVT locks the index plan in every state that used to fall back, in addition to unit tests for the budget, the engine counting, and prepared-plan reusability. The design tradeoff while the index lags (fewer than LIMIT rows and missed not-yet-indexed rows) is documented in docs/cn/vector_search.md and in code comments, so I treat it as accepted design intent rather than a defect. One blocking defect is already on the record from the prior mo-bot review and remains unaddressed at this head: an in-transaction vector update preserves the primary key, so the PK-only base-table join pairs the updated row with the stale index score (reproduced as d=0 for v='[100,100]'); per review policy I do not duplicate it here, but it still gates the PR and is not waived by the eventual-consistency documentation because the PR explicitly claims the transaction's own writes are exact. My remaining findings are non-blocking: the transaction-local widening has no cap, so a transaction that deletes a large fraction of the table turns every subsequent LIMIT-k HNSW query inside it into a near-whole-index search with a proportional result heap. Local verification was limited: building or running the HNSW, plan, compile, frontend, or disttae tests requires third-party native headers (usearch.h, xxhash.h, roaring.h, jemalloc.h) that are unavailable in this environment and whose installation is out of scope, so only pkg/vectorindex/overfetch tests ran (passing); no test failure here is attributable to the PR. Static review of the full 25-file diff plus green UT, BVT multi-CN, and SCA checks at this head fill that gap, but runtime multi-CN behavior was validated only by CI BVT runs, not locally.

P2 - Transaction-local candidate widening is uncapped and can turn in-transaction HNSW queries into near-whole-index searches (pkg/vectorindex/hnsw/plugin/search/search.go:223)

transactionLocalBudget returns resultLimit plus every uncommitted deleted or updated row with no ceiling, and newSearcher passes that straight through as limit: max(req.CandidateBudget, local, 1) (search.go:172). For non-iterative searches nothing caps this widened limit: searchOnce clamps it only to the total indexed rows via boundedHnswSearchLimits (hnsw/search.go:232) and allocates a results heap of that size. Iterative mode does not bound it either: hnsw/iterative.go:72 re-raises the round cap with max(capITopK, req.CandidateBudget, ...), where CandidateBudget is the widened rt.Limit, so DefaultMaxInternalTopK (20000) and a user-set max_itopk are both lifted by the same widening. A transaction that deletes or updates a large share of the source table (for example a bulk cleanup of tens of millions of rows on an 88M-row index) therefore makes every subsequent LIMIT-k HNSW query inside that transaction search k+deleted candidates and hold min(k+deleted, index size) heap entries, approaching the whole-index cost this PR set out to avoid, with a large memory footprint on top. Results stay correct, so this is not a correctness regression, but the widening deserves a bound: cap it at max_itopk or a dedicated configurable budget and return fewer-than-LIMIT rows when the cap is hit, or document the sharp cost of in-transaction mass deletes so users are steered toward committing first.

[cpegeric]

@XuPeng-SH (P1) and @gouhongshen (P2): as of 9af535d6c3, the code both findings were about is gone. HNSW no longer widens its candidate count by the transaction's own deletes, and engine.TransactionLocalDeletesProvider is removed, so this PR changes no storage-engine code. The search returns the candidates found in the usearch index, joined to the source table by primary key, as before #30117.

P1 (in-transaction vector update shows the indexed distance): this is the documented behaviour, not something this PR claims to make exact. The same result appears after a committed update that ISCP has not applied:

Query	Row 1 (v changed to [100,100])
in-transaction UPDATE, index plan	d = 0, ranked first
committed UPDATE, before ISCP applies it, index plan	d = 0, ranked first
mode=force	d = 141.42, ranked last

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

• a row not in the index is not returned;
• a row deleted from the source is dropped by the join, so a page can have fewer than LIMIT rows;
• a row whose vector changed is ranked and returned with its indexed vector's distance.

This holds for the reading transaction's own writes and for committed writes alike. mode=force computes distances from current vectors. Recomputing distances on the base row would not make the result exact, because a row whose new vector moved close to the query is not among the index's candidates. Making it exact needs the full scan this PR refuses (WARNING: TOXIC in pkg/sql/plan/apply_indices_hnsw.go).

P2 (uncapped widening): removed with the count.

Docs and tests:

• docs/cn/vector_search.md states exactly the behaviour above.
• BVT vector/issue_29889_hnsw_committed_read checks each statement from SQL. Each index query is paired with mode=force, and the transaction's uncommitted writes hold the index behind deterministically:
  • an inserted row is not returned;
  • with 15 rows deleted, LIMIT 2 returns 0 rows while 5 remain;
  • an updated row is returned with its indexed distance.
• The same BVT locks the index plan in every state fix: prevent stale HNSW top-k results #30117 fell back in.
• Results: vector, pessimistic_transaction/vector and ddl/alter_copy_add_index at 100%; pkg/vectorindex/hnsw/... unit tests pass.