Skip to content

Fix search in empty HNSW segments#7620

Merged
monoid merged 3 commits intodevfrom
fix/top-zero
Nov 27, 2025
Merged

Fix search in empty HNSW segments#7620
monoid merged 3 commits intodevfrom
fix/top-zero

Conversation

@monoid
Copy link
Contributor

@monoid monoid commented Nov 26, 2025 •
edited
Loading

For an empty HNSW segment, sampling top becomes 0, and BatchFilteredSearcher doesn't handle it properly, panicing.

All Submissions:

  • Contributions should target the dev branch. Did you create your branch from dev?
  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

@monoid monoid self-assigned this Nov 26, 2025
@monoid monoid marked this pull request as ready for review November 26, 2025 16:03
coderabbitai[bot]

This comment was marked as resolved.

Sign in to view

@qdrant qdrant deleted a comment from coderabbitai bot Nov 26, 2025
@timvisee
Copy link
Member

timvisee commented Nov 26, 2025

Am I right that this only resolves a panic in our recent release if top becomes 0?

Out of curiosity; does that ever happen in practice, because we validate limit >= 1 (ref).

@monoid
Copy link
Contributor Author

monoid commented Nov 26, 2025 •
edited
Loading

Am I right that this only resolves a panic in our recent release if top becomes 0?

Out of curiosity; does that ever happen in practice, because we validate limit >= 1 (ref).

Yes, it resolves panic if top becomes 0.

I observed it when I was experimented with parallel read and write. Somehow an empty(!) HNSW segment got created, and here top becomes 0. I presume it indexed a segment where all the points got deleted during the write. Unfortunately, I cannot confirm it because the relevant piece of log was evicted by stacktraces.

Upd: but I think it should be a rare event.

@timvisee
Copy link
Member

timvisee commented Nov 26, 2025

It's good to protect against this anyway. Thank you for including tests!

IvanPleshkov
IvanPleshkov approved these changes Nov 27, 2025
View reviewed changes
Copy link
Contributor

@IvanPleshkov IvanPleshkov left a comment

Choose a reason for hiding this comment

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

Thanks for tests

@monoid
Move top==0 check upper in the call stack
fcc34c3 
And add test for HNSW index search too.
coderabbitai[bot]

This comment was marked as resolved.

Sign in to view

@monoid monoid merged commit 2a7fc71 into dev Nov 27, 2025
15 checks passed
@monoid monoid deleted the fix/top-zero branch November 27, 2025 11:56
@agourlay agourlay mentioned this pull request Nov 28, 2025
Closed
@qdrant qdrant deleted a comment from coderabbitai bot Nov 28, 2025
timvisee pushed a commit that referenced this pull request Dec 3, 2025
@monoid @timvisee
Fix search in empty HNSW segments (#7620)
01d9d6f 
* Add top==0 tests

* Fix HNSW scan top==0 error
@timvisee timvisee mentioned this pull request Dec 3, 2025
Merged
1 task
@claude claude bot mentioned this pull request Dec 4, 2025
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@IvanPleshkov IvanPleshkov IvanPleshkov approved these changes

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@timvisee timvisee timvisee approved these changes

@generall generall Awaiting requested review from generall

@xzfc xzfc Awaiting requested review from xzfc

Assignees

@monoid monoid

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@monoid @timvisee @IvanPleshkov