fix: count stored rows, not sealed row_count, in search and stats (#534) - #745
Open
DrTrippel wants to merge 1 commit into
Open
fix: count stored rows, not sealed row_count, in search and stats (#534)#745DrTrippel wants to merge 1 commit into
DrTrippel wants to merge 1 commit into
Conversation
…lliztech#534) get_collection_stats()["row_count"] only counts sealed/flushed segments. On Milvus Server, freshly upserted rows stay in a growing segment until the DataNode syncs them (syncPeriod, 600s by default), so row_count reads 0. MilvusStore.search() returned [] on row_count == 0 (the zilliztech#306 empty-collection guard) and count() reported the same number, so a new collection looked empty to `memsearch search` and `memsearch stats` for ~10 minutes after `memsearch index` succeeded. count() now uses a strong-consistency count(*) query, and search() guards on count() == 0, which keeps the zilliztech#306 protection for truly empty collections. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ng6tyJJrBWCrSZNKGUbBdc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #534
Problem
On Milvus Server,
memsearch searchandmemsearch statsreport a collection as empty for about 10 minutes aftermemsearch indexsucceeds. The data is not lost. memsearch just doesn't see it.MilvusStore.search()guards against the #306 BM25 crash by returning[]whenget_collection_stats()["row_count"] == 0, andcount()returns the same value.row_countonly counts sealed/flushed segments. Freshly upserted rows sit in a growing segment until the DataNode syncs them (dataNode.segment.syncPeriod, 600s by default). Until then:row_countis 0, so search returns nothing without querying;statsprintsTotal indexed chunks: 0.Collections that already have an older flushed segment are not affected: the guard passes, and
hybrid_searchsees the growing rows immediately. That is why the bug only shows up on new or freshly re-created collections.Milvus Lite flushes eagerly, which masks the problem, so #534 looked like a missing flush.
Repro (Milvus v3.0.1 standalone in Docker, memsearch 0.4.20, pymilvus 3.0.1)
memsearch index <dir> --collection ms_probeIndexed 1 chunks.get_collection_stats("ms_probe")["row_count"]0query("ms_probe", filter="", output_fields=["count(*)"], consistency_level="Strong")1memsearch search <canary> --collection ms_probeNo results found.until 608s after indexing, then foundstats/search0/ nothing; afterMilvusClient.flush(...),23and a score-1.0 hitFix
count()counts stored rows withquery(filter="", output_fields=["count(*)"], consistency_level="Strong").search()guards onself.count() == 0, so the Milvus crash on hybrid_search: Assert "std::isfinite(element.val)" failed → "Invalid sparse row: NaN or Inf value" #306 protection for truly empty collections stays. It also stops counting deleted-but-not-compacted rows.Tests
New tests
test_unsealed_rows_are_counted_and_searchablesimulates Milvus Server by patchingget_collection_statsto reportrow_count: 0after an upsert. It fails onmain(assert 0 == 1) and passes with this change.test_search_empty_collection_returns_emptykeeps the Milvus crash on hybrid_search: Assert "std::isfinite(element.val)" failed → "Invalid sparse row: NaN or Inf value" #306 guard covered.Local runs
uv run python -m pytest tests/test_store.py: 19 passed.uv run python -m pytest: 423 passed, 7 skipped, 6 failed. The same 6 fail on unmodifiedmain(test_claude_stop_indexes_only_server[*],test_session_start_and_data_commands_are_scoped_to_host_project[*]), so they are unrelated to this change.ruff checkandruff format --check: clean.🤖 Generated with Claude Code
https://claude.ai/code/session_01Ng6tyJJrBWCrSZNKGUbBdc