A search does not hold a runtime worker, and its two channels run side by side - #535
Merged
Merged
Conversation
…e by side Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: WaylandYang <wayland0916@gmail.com>
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.
Closes #515.
What changed
retrieval::hybridcalledSearchIndex::searchstraight from the async function. That call is synchronous Tantivy work (tokenise, walk the mmap'd segments, fetch each hit), so it held a runtime worker for its whole duration; the other six Tantivy call sites in the server already wrap theirs inspawn_blocking, and this was the hottest one left. The two channels also ran in series: BM25, then the settings row, then the embedding round-trip to the model, then the vector query, so a question paid the sum of both legs.Now:
spawn_blocking, the same shape as the reindex and delete sites.tokio::join!. Latency becomesmax(bm25, embed + vector), and since the embed leg is a network hop, the BM25 leg is close to free.Option. No embedding model, a failed embedding request, or an empty answer all yieldNoneand the search continues on BM25, which is the degradation the module header promises.join!rather thantry_join!so a model outage stays a degradation and does not become a failed search. A failure of the vector query itself is still an error: that is the database, not the model.rrf_fuseis pinned: BM25 first, vector second, whichever finished first. RRF ignores order today; it would not if the channels were ever weighted.search_docsin the chat tools had the same shape on the Charter index and gets the same wrap.The record-axis asymmetry the header documents (
as_ofcomplete on the vector and fetch paths, not on Tantivy) is untouched.Tests
rrf_fusehad no tests. Four pure ones inutopia-search: one list keeps its order, two lists follow the reciprocal-rank arithmetic, an empty list contributes nothing, the limit cuts after fusion.channel_listsis the pure seam for the ordering rule: the channels keep their places, and a missing vector channel leaves BM25 alone.retrieval_tests.rs, with a temp Tantivy index and wiremock standing in for the embedding endpoint: a search without an embedding model still answers; a 500 from the endpoint degrades to BM25 rather than failing; with both channels the chunk both found ranks first and the vector-only chunk still comes back. They usetest_db::url()and ran green withUTOPIA_TEST_REQUIRE_DB=1against a database at dev's migrations.Not tested: "a search never occupies a runtime worker". On a small index the search takes microseconds and any assertion on it would flake;
spawn_blockingis the established pattern here and the change is visible in the code.Acceptance
Not done in this PR. What it buys shows under load: run twenty concurrent
/searchrequests against a base of a few thousand chunks while polling/healthevery 100 ms. Before, the health latency follows the searches; after, it does not. Latency p50/p95 needs a base large enough for the BM25 leg to be measurable; the local bases are hundreds of chunks.🤖 Generated with Claude Code