Skip to content

[MOD-10654] [MOD-10694] Enhance SVS Debug Info Iterator #736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Aug 7, 2025

Conversation

meiravgri
Copy link
Collaborator

@meiravgri meiravgri commented Aug 5, 2025

Implements comprehensive debug info iterator for SVS indexes and fixes background indexing status for tiered indexes.

Key Changes

  • Expanded SVS debug info: Added missing fields (quantization bits, alpha, graph params, search params, etc.)
  • Fixed tiered background indexing: Properly reports indexing status based on frontend size and scheduled updates
  • Unified string constants: Renamed HNSW-specific strings to shared constants (EPSILON, NUM_MARKED_DELETED)

Technical Details
Increased SVS debug iterator
Changed backgroundIndexing from bool to VecSimBool enum
Added utility functions: VecSimQuantBits_ToString()
Added tiered SVS-specific parameters (training/update thresholds, timeout)

To make them more general remove "HNSW" indicator from:
- HNSW_NUM_MARKED_DELETED
- HNSW_EPSILON_STRING

elaborate vec_utils with the new SVS strings, and *_ToString functions

update svs and multi svs tests
Copy link

codecov bot commented Aug 5, 2025

Codecov Report

❌ Patch coverage is 98.43750% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96.85%. Comparing base (cd82a1a) to head (103a9d1).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/VecSim/utils/vec_utils.cpp 94.73% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #736      +/-   ##
==========================================
+ Coverage   96.82%   96.85%   +0.02%     
==========================================
  Files         122      122              
  Lines        7443     7498      +55     
==========================================
+ Hits         7207     7262      +55     
  Misses        236      236              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- Add missing SVS-specific fields to compareSVSIndexInfoToIterator function
- Create scoped field count constants for all index types (FLAT, HNSW, SVS, TIERED)
- Unify tiered index testing with generic compareTieredIndexInfoToIterator
- Fix VecSimBool enum definition and backgroundIndexing field type consistency
- Add SVS tiered-specific debug fields (training threshold, update threshold, timeout)
@meiravgri meiravgri changed the title expose svs info via iterator in debugInfoIterator [MOD-10654] [MOD-10694] expose svs info via iterator in debugInfoIterator Aug 5, 2025
@meiravgri meiravgri changed the title [MOD-10654] [MOD-10694] expose svs info via iterator in debugInfoIterator [MOD-10654] [MOD-10694] Enhance SVS Debug Info Iterator Aug 5, 2025
@meiravgri meiravgri requested a review from lerman25 August 5, 2025 16:51
Copy link
Collaborator

@lerman25 lerman25 left a comment

Choose a reason for hiding this comment

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

Great work

return info;
}

VecSimDebugInfoIterator *debugInfoIterator() const override {
VecSimIndexDebugInfo info = this->debugInfo();
// For readability. Update this number when needed.
size_t numberOfInfoFields = 10;
size_t numberOfInfoFields = 24;
Copy link
Collaborator

Choose a reason for hiding this comment

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

There must be a better way
Maybe not for this PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

its just an optiomisation to avoid vector reallocation. if the number doesn't exactly match the number of fields the vector will increase its capacity.

Comment on lines 367 to 371
.searchWindowSize = this->search_window_size,
.searchBufferCapacity = this->search_buffer_capacity,
.leanvecDim = this->leanvec_dim,
.epsilon = this->epsilon};
.epsilon = this->epsilon,
.changes_num = this->changes_num};
Copy link
Collaborator

Choose a reason for hiding this comment

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

number of marked deleted and changes num are the same?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good catch :)
they are

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  • currently
    "// Count severe index changes (currently deletions only) and consolidate index if needed"

lerman25
lerman25 previously approved these changes Aug 6, 2025
@meiravgri meiravgri enabled auto-merge August 6, 2025 15:57
@meiravgri meiravgri added this pull request to the merge queue Aug 6, 2025
@meiravgri meiravgri removed this pull request from the merge queue due to a manual request Aug 6, 2025
@meiravgri meiravgri enabled auto-merge August 7, 2025 07:25
@meiravgri meiravgri added this pull request to the merge queue Aug 7, 2025
Merged via the queue into main with commit d545769 Aug 7, 2025
19 checks passed
@meiravgri meiravgri deleted the meiravg_svs_info_iter branch August 7, 2025 12:42
github-actions bot pushed a commit that referenced this pull request Aug 7, 2025
* expose svs info via iterator in debugInfoIterator

To make them more general remove "HNSW" indicator from:
- HNSW_NUM_MARKED_DELETED
- HNSW_EPSILON_STRING

elaborate vec_utils with the new SVS strings, and *_ToString functions

update svs and multi svs tests

* Enhance SVS debug info iterator with complete field validation

- Add missing SVS-specific fields to compareSVSIndexInfoToIterator function
- Create scoped field count constants for all index types (FLAT, HNSW, SVS, TIERED)
- Unify tiered index testing with generic compareTieredIndexInfoToIterator
- Fix VecSimBool enum definition and backgroundIndexing field type consistency
- Add SVS tiered-specific debug fields (training threshold, update threshold, timeout)

* info.tieredInfo.backgroundIndexing =
fix tierred  in svs

* fix hnsw tiered backgroudhnindxxing

* remove VecSimBool_ToString, not needed

* remove num_threads

* add coverage

* fi field count

* fix wuant bits

* fix tests

(cherry picked from commit d545769)
Copy link

github-actions bot commented Aug 7, 2025

Successfully created backport PR for 8.2:

github-merge-queue bot pushed a commit that referenced this pull request Aug 7, 2025
[MOD-10654] [MOD-10694] Enhance SVS Debug Info Iterator (#736)

* expose svs info via iterator in debugInfoIterator

To make them more general remove "HNSW" indicator from:
- HNSW_NUM_MARKED_DELETED
- HNSW_EPSILON_STRING

elaborate vec_utils with the new SVS strings, and *_ToString functions

update svs and multi svs tests

* Enhance SVS debug info iterator with complete field validation

- Add missing SVS-specific fields to compareSVSIndexInfoToIterator function
- Create scoped field count constants for all index types (FLAT, HNSW, SVS, TIERED)
- Unify tiered index testing with generic compareTieredIndexInfoToIterator
- Fix VecSimBool enum definition and backgroundIndexing field type consistency
- Add SVS tiered-specific debug fields (training threshold, update threshold, timeout)

* info.tieredInfo.backgroundIndexing =
fix tierred  in svs

* fix hnsw tiered backgroudhnindxxing

* remove VecSimBool_ToString, not needed

* remove num_threads

* add coverage

* fi field count

* fix wuant bits

* fix tests

(cherry picked from commit d545769)

Co-authored-by: meiravgri <109056284+meiravgri@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants