Skip to content

Check for query cancellation during vector similarity index building#102010

Merged
alexey-milovidov merged 2 commits intomasterfrom
fix-vector-similarity-hung-check
Apr 9, 2026
Merged

Check for query cancellation during vector similarity index building#102010
alexey-milovidov merged 2 commits intomasterfrom
fix-vector-similarity-hung-check

Conversation

@alexey-milovidov
Copy link
Copy Markdown
Member

@alexey-milovidov alexey-milovidov commented Apr 8, 2026

The stress test (arm_tsan) hits a hung check failure because USearch's HNSW graph construction does not respect query cancellation. When KILL QUERY is issued, threads stuck inside USearch add keep running indefinitely, especially under TSan where each operation is very slow.

Add a cancellation check before calling USearch add. This lets KILL QUERY interrupt index building.

CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=102000&sha=9190a5549402d12fff136f61209c5405f2aa896c&name_0=PR&name_1=Stress%20test%20%28arm_tsan%29

Changelog category (leave one):

  • CI Fix or Improvement (changelog entry is not required)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

...

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

The stress test (arm_tsan) hits a hung check failure because USearch's
HNSW graph construction does not respect query cancellation. When
`KILL QUERY` is issued, threads stuck inside USearch `add` keep running
indefinitely, especially under TSan where each operation is very slow.

Add a periodic cancellation check (every 128 rows) before calling
USearch `add`. This lets `KILL QUERY` interrupt index building without
adding measurable overhead to every single vector insertion.

https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=102000&sha=9190a5549402d12fff136f61209c5405f2aa896c&name_0=PR&name_1=Stress%20test%20%28arm_tsan%29

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Apr 8, 2026

Workflow [PR], commit [384235f]

Summary:

job_name test_name status info comment
Stateless tests (arm_asan_ubsan, flaky check) failure
03172_error_log_table_not_empty FAIL cidb
Stateless tests (amd_asan_ubsan, flaky check) failure
03172_error_log_table_not_empty FAIL cidb
Stateless tests (amd_tsan, flaky check) failure
03172_error_log_table_not_empty FAIL cidb
03172_error_log_table_not_empty FAIL cidb
03172_error_log_table_not_empty FAIL cidb
03634_autopr_output_bytes_estimation FAIL cidb
03634_autopr_output_bytes_estimation FAIL cidb
Stateless tests (amd_msan, flaky check) failure
03172_error_log_table_not_empty FAIL cidb
03172_error_log_table_not_empty FAIL cidb
03172_error_log_table_not_empty FAIL cidb
03172_error_log_table_not_empty FAIL cidb
03172_error_log_table_not_empty FAIL cidb
Stateless tests (amd_debug, flaky check) failure
03172_error_log_table_not_empty FAIL cidb

AI Review

Summary

This PR adds a query-cancellation check before each vector insertion in MergeTreeIndexAggregatorVectorSimilarity::updateImpl, so long-running USearch index builds can stop earlier when cancellation is requested. I found no correctness, safety, or performance issues in the current patch, and no additional blocking concerns beyond already-discussed review threads.

ClickHouse Rules
Item Status Notes
Deletion logging
Serialization versioning
Core-area scrutiny
No test removal
Experimental gate
No magic constants
Backward compatibility
SettingsChangesHistory.cpp
PR metadata quality
Safe rollout
Compilation time
Final Verdict
  • Status: ✅ Approve

@clickhouse-gh clickhouse-gh Bot added the pr-ci label Apr 8, 2026
Comment thread src/Storages/MergeTree/MergeTreeIndexVectorSimilarity.cpp Outdated
Each row is submitted as a separate task to the thread pool, so the
previous `row % 128 == 0` guard meant only ~1 in 128 tasks ever checked
for cancellation. Remove the modulo guard and check before every `add`
call — the check just reads an atomic flag and is cheap.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@murphy-4o murphy-4o self-assigned this Apr 8, 2026
Comment thread src/Storages/MergeTree/MergeTreeIndexVectorSimilarity.cpp
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Apr 8, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 83.90% 84.00% +0.10%
Functions 90.90% 90.90% +0.00%
Branches 76.40% 76.50% +0.10%

Changed lines: 100.00% (7/7) · Uncovered code

Full report · Diff report

@alexey-milovidov
Copy link
Copy Markdown
Member Author

Flaky check - unfortunately, needs #102148

Onyx2406 added a commit to Onyx2406/ClickHouse that referenced this pull request Apr 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-ci pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants