perf(search): multi-index fan-out executes concurrently (#875) - #877
Conversation
The wildcard/multi-index search loop spawned one task per index and awaited it before spawning the next — serial by construction, so `ax-*` over N datasets cost ~N x single-index latency. Measured on the 15-repo autoindex corpus (41 datasets): 701 ms p50 for the identical query that answers in 11.7 ms against one index. The reference-coding workflow queries `ax-*` on every retrieval, so the flagship use case paid the worst case of this loop. Two-phase now: phase 1 spawns every per-index search up front (per-index request prep is sync map lookups and stays serial); phase 2 awaits and merges in the SAME deterministic index order, so hit ordering and agg-merge order are unchanged. Concurrency is bounded by the existing global search permit (`Index::search` -> `governor.acquire_search`, sized by `limits.max_concurrent_searches`); excess searches queue deadline-aware on that semaphore exactly as they did when issued one at a time. An early error return drops the remaining AbortOnDrop handles, aborting in-flight searches. Observable shift: `queries_by_index` now increments for every resolvable index even when a later index errors the request — those searches really do run now. Proof, not timing: the pending-search test injection extends to a Vec of signal pairs and prefix-matched index names, and the new `wildcard_fanout_runs_per_index_searches_concurrently` test asserts BOTH per-index searches START while NEITHER can complete — impossible under the serial loop (the second never spawned until the first resolved), immediate under the fan-out. Abort propagation to all spawned searches is asserted in the same test. Gates: ES-YAML conformance on this build 1366 passed / 0 failed / 3 skipped; all 5 search-task lifetime unit tests green; fmt + clippy clean. The scroll snapshot path (search_with_scroll) still fans out serially — it fetches whole snapshots and is not latency-critical; noted on #875 as a follow-up rather than widening this diff.
|
Same-state A/B, and an honest correction to the headline expectation. Measured today on the identical corpus state (41 autoindex datasets, settled and merged), same 21-rep p50 method, released rc.71 (serial loop) as the control:
Result parity: identical totals and top-hit scores across both arms. The issue's 701 ms baseline does NOT reproduce on a settled corpus — that number was compound: the serial loop multiplying a per-index cost that was itself pathological (~17 ms/index while 100k-doc datasets sat memtable-resident during post-ingest merge churn — the #873/#876 state). What this PR is worth, stated precisely: it bounds |
ab-fanout.sh was a throwaway measurement script I used to compare the serial and concurrent fan-out; review caught that it was committed at the repo root and would ship in rc.72. It is not product: its header documents $1/$2 while the body requires a third argument, and it assigns SP without using it. The measurement it produced is recorded in the PR discussion, which is where that belongs.
Brings the branch onto rc.72's main after the eight-PR sweep (#877 concurrent multi-index fan-out, #881 event-driven merge scheduling, #882 match_phrase slop transpositions, #883 lock-free memtable byte accounting, #888 date epoch scale from the mapping, #880/#884/#885/#886). One textual conflict, in CHANGELOG.md: this branch and #882 both inserted a new bullet as the first entry under `## [Unreleased] / ### Fixed`. Both kept, in order — the two #781 entries (declared numeric/boolean type enforcement and the type-mixed doc-values column) followed by #882's `match_phrase` slop entry. Verified line-for-line that nothing main carried was dropped. The two shared source files merged without conflict, and the resolution was checked rather than assumed: the merge's diff against origin/main is byte-identical in shape to this branch's diff against its old base (77 lines in es_compat.rs, 207 in index.rs, 129 in bulk.rs), i.e. the merge added this branch's changes and subtracted none of main's. Region-by-region, the two sides do not touch: * es_compat.rs — main edited `search_impl` (11110-11286, #877), the search-task lifetime tests and `es_properties_to_fields` / `es_type_to_native` (16557-16853, #888's `date_precision`); this branch edits `index_doc_auto` / `index_doc` / `create_doc` (2831-3099) and appends `enforce_field_types` beside `apply_ignore_malformed`. * index.rs — main edited the merge-scheduling and sort/shadow sites (`request_merge_check`, `build_sort_shadow`, `shadow_range_bounds`, `sort_epoch_memo`, `normalize_search_after_value`, `compute_sort_values`, `phrase_walk` / `phrase_positions_in_tokens`); this branch edits `build_doc_value_columns`, `rewrite_query_aliases` and `json_values_equal`, none of which main touched. * bulk.rs, xerj-common/src/field_coercion.rs, xerj-common/src/lib.rs — not touched by main at all. Each sibling's distinctive identifiers were then grepped back out of the merged tree: `request_merge_check` (11), `DateScale` (31), `phrase_positions_match` (4), and the memtable byte-delta counters. The only surviving `spawn_merge_task` mentions are the two doc-comments #881 itself left behind.
Brings rc.72's eight merged siblings under the xerj-org#825 knn-beside-query work: xerj-org#877 concurrent multi-index fan-out, xerj-org#881 event-driven merge scheduling, xerj-org#882 match_phrase slop transpositions, xerj-org#883 lock-free memtable byte accounting, xerj-org#888 date epoch scale from the mapping, xerj-org#885/xerj-org#886 autoindex and xerj-org#880/xerj-org#884 docs. One textual conflict, in CHANGELOG.md: both sides inserted a new bullet at the head of `### Fixed` — the xerj-org#825 union entry here, the xerj-org#830 sloppy-phrase entry on main. They describe unrelated fixes, so both are kept, xerj-org#825 first. The two files the siblings and this branch share — engine/crates/ xerj-engine/src/index.rs and engine/crates/xerj-api/src/es_compat.rs — merged without conflict, and that was verified rather than assumed: the merge result diffed against origin/main is byte-identical to this branch's own diff against the merge base (5f3b6ea), so main's side is carried through intact and no sibling hunk was dropped. The two edits that share a function are non-overlapping by construction: xerj-org#877 rewrites the per-index fan-out loop near the end of `search_impl`, while xerj-org#825 rewrites the `knn`-beside-`query` fold ~1500 lines earlier, before any index is resolved — the fold still runs once per request, and the pinned tree is what each concurrently spawned per-index search receives. Gates on the merge result: `cargo build --release -p xerj-api` clean; `cargo fmt --all --check` clean; `cargo clippy --release -p xerj-engine -p xerj-api -- -D warnings` clean.
The wildcard/multi-index search loop spawned one task per index and awaited it before spawning the next — serial by construction, so
ax-*over N datasets cost the SUM of per-index latencies instead of the maximum.Two-phase now: phase 1 spawns every per-index search up front (per-index request prep is sync map lookups and stays serial); phase 2 awaits and merges in the SAME deterministic index order, so hit ordering and agg-merge order are unchanged. Concurrency is bounded by the existing global search permit (
Index::search→governor.acquire_search, sized bylimits.max_concurrent_searches); excess searches queue deadline-aware on that semaphore.What this is worth, measured honestly
The issue opened with a 701 ms
ax-*p50 against 11.7 ms for the same query on one index. That baseline does not reproduce on a settled corpus, and the PR should not claim it. Re-measured on the identical corpus state, released rc.71 (serial) as the control, p50 of 21:ax-*ax-*ax-*Result parity: identical totals and top-hit scores.
The original 701 ms was compound — the serial loop multiplying a per-index cost that was itself pathological (~17 ms/index while 100k-doc datasets sat memtable-resident during post-ingest merge churn, the #873/#876 state). So this change bounds
ax-*bymax(per-index)instead ofΣ(per-index): ~1.2× when indices are cheap, and the ~N× the issue measured when they are not. It is the insurance half; #878 and #876 remove the expensive state itself.The merge gate is the deterministic concurrency test, not the latency table.
Proof
wildcard_fanout_runs_per_index_searches_concurrentlyasserts BOTH per-index searches START while NEITHER can complete — impossible under the serial loop (the second never spawned until the first resolved), immediate under the fan-out. Abort propagation to all spawned searches is asserted in the same test. The pending-search test injection was extended to a Vec of signal pairs with prefix-matched index names to make that expressible.Behaviour changes, disclosed
queries_by_indexnow increments for every resolvable index even when a later index errors the request — those searches really do run now.timed_out: truewith partial results where the serial loop would have returned complete results slowly. Only reachable whenceil(N/permits) × per-index latencyexceeds the timeout.Review follow-ups
Raised and deliberately not taken in this PR, recorded for #875: the fan-out is unbounded per request (up to N tasks, capped process-wide by the 64-permit pool rather than per-request). Bounding it to
min(N, cpus)would cost nothing and remove a fairness/peak-memory question on single-user deployments; it is a small follow-up rather than a change to make under a release gate.Closes #875.
🤖 Generated with Claude Code