Skip to content

Commit 8fc0e0c

Browse files
sezrubyclaude
andcommitted
fix(index): tighten batch IVF gate, move ranking to CPU runtime, record partitions_searched
Three follow-ups on the shared-scan batch IVF path: - Gate eligibility on whether the *selected* index_segments cover every requested fragment, not whether the whole logical index does. A subset selected via with_index_segments could otherwise let the batch node search only the selected segments and silently drop a fragment covered solely by an unselected segment. Extracted the coverage check into fragments_missing_from_index_segments, shared by the gate and knn_combined so eligibility and fallback stay in lockstep. - Run the per-query centroid ranking on the dedicated CPU runtime (find_partitions_batch_on_cpu) instead of the async worker: the ranking is pure CPU and batch width multiplies it, so a wide batch over a large centroid set could monopolize a Tokio worker. - Record partitions_searched on ANNIvfBatchExec. It built the metric but never incremented it, so EXPLAIN ANALYZE reported 0 for every batch query. Report the distinct partitions read -- the shared I/O this node exists to save -- mirroring the single-query ANNIvfSubIndexExec. Tests: partial-segment fallback, CPU-runtime ranking, and a partitions_searched=2 assertion (distinct union, not the per-query sum of 4). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 8533cb6 commit 8fc0e0c

2 files changed

Lines changed: 242 additions & 43 deletions

File tree

rust/lance/src/dataset/scanner.rs

Lines changed: 137 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5545,8 +5545,8 @@ impl Scanner {
55455545
/// - no refine step (the batch path does not yet rerank);
55465546
/// - fixed nprobes (`minimum_nprobes == maximum_nprobes`) — see below;
55475547
/// - every segment an IVF index with a flat-style sub-index (i.e. not HNSW);
5548-
/// - all target fragments indexed (or `fast_search`, which ignores
5549-
/// unindexed fragments).
5548+
/// - every target fragment covered by the *selected* `index_segments` (or
5549+
/// `fast_search`, which searches only the selected segments anyway).
55505550
///
55515551
/// The fixed-nprobes requirement is a *correctness* gate, not just an
55525552
/// optimization. The shared-scan path searches exactly `minimum_nprobes`
@@ -5625,11 +5625,47 @@ impl Scanner {
56255625
if self.fast_search {
56265626
return Ok(true);
56275627
}
5628-
// The batch node only searches indexed partitions, so any unindexed
5629-
// target fragment would silently drop rows; fall back in that case.
5630-
let unindexed_fragments =
5631-
self.retain_target_fragments(self.dataset.unindexed_fragments(index_name).await?);
5632-
Ok(unindexed_fragments.is_empty())
5628+
// The batch node only searches the selected `index_segments`, so any
5629+
// target fragment those segments do not cover would silently drop rows
5630+
// (the single-query path re-scores such fragments on a flat fallback in
5631+
// `knn_combined`). Measure coverage against the selected segments -- not
5632+
// the whole logical index -- so a subset selected via
5633+
// `with_index_segments` cannot hide a fragment that an unselected
5634+
// segment happens to cover; fall back whenever any remain.
5635+
let uncovered_fragments = self
5636+
.fragments_missing_from_index_segments(index_name, index_segments)
5637+
.await?;
5638+
Ok(uncovered_fragments.is_empty())
5639+
}
5640+
5641+
/// Target fragments the given `index_segments` do not cover.
5642+
///
5643+
/// The ANN scan reads only the selected segments' partitions, so these are
5644+
/// exactly the fragments the single-query path re-scores on a flat fallback
5645+
/// in [`Self::knn_combined`]. Coverage is measured against the *selected*
5646+
/// segments rather than every segment of the logical index (which
5647+
/// `Dataset::unindexed_fragments` would do): a caller may select a subset
5648+
/// via [`with_index_segments`](Self::with_index_segments) while another,
5649+
/// unselected segment covers one of the requested fragments.
5650+
async fn fragments_missing_from_index_segments(
5651+
&self,
5652+
index_name: &str,
5653+
index_segments: &[IndexMetadata],
5654+
) -> Result<Vec<Fragment>> {
5655+
if let Some(target_fragments) = &self.fragments {
5656+
let indexed_fragments = self.get_indexed_frags(index_segments);
5657+
Ok(target_fragments
5658+
.iter()
5659+
.filter(|fragment| !indexed_fragments.contains(fragment.id as u32))
5660+
.cloned()
5661+
.collect())
5662+
} else if self.index_segments.is_some() {
5663+
// An explicit segment selection with no fragment restriction searches
5664+
// exactly those segments; there is nothing to fall back for.
5665+
Ok(Vec::new())
5666+
} else {
5667+
self.dataset.unindexed_fragments(index_name).await
5668+
}
56335669
}
56345670

56355671
async fn batch_indexed_vector_search(
@@ -5748,18 +5784,9 @@ impl Scanner {
57485784
mut knn_node: Arc<dyn ExecutionPlan>,
57495785
filter_plan: &ExprFilterPlan,
57505786
) -> Result<Arc<dyn ExecutionPlan>> {
5751-
let fallback_fragments = if let Some(target_fragments) = &self.fragments {
5752-
let indexed_fragments = self.get_indexed_frags(indexed_segments);
5753-
target_fragments
5754-
.iter()
5755-
.filter(|fragment| !indexed_fragments.contains(fragment.id as u32))
5756-
.cloned()
5757-
.collect::<Vec<_>>()
5758-
} else if self.index_segments.is_some() {
5759-
Vec::new()
5760-
} else {
5761-
self.dataset.unindexed_fragments(index_name).await?
5762-
};
5787+
let fallback_fragments = self
5788+
.fragments_missing_from_index_segments(index_name, indexed_segments)
5789+
.await?;
57635790

57645791
let has_fallback = !fallback_fragments.is_empty();
57655792
let has_stale = !stale_rows.is_empty();
@@ -9773,6 +9800,23 @@ mod test {
97739800
plan
97749801
);
97759802

9803+
// The batch node loads each probed partition once and scores every query
9804+
// that probes it, so it must report the *distinct* partitions read: with
9805+
// 2 partitions and nprobes(2), both queries probe both partitions, so the
9806+
// union is 2 -- not the per-query sum (2 queries x 2 = 4), and never 0
9807+
// (which is what a dropped metric would show). This guards the observed
9808+
// `partitions_searched` against silently regressing to either.
9809+
let analyzed = scan.analyze_plan().await.unwrap();
9810+
let batch_line = analyzed
9811+
.lines()
9812+
.find(|line| line.contains("ANNIvfBatch"))
9813+
.expect("analyzed plan should contain the ANNIvfBatch node");
9814+
assert!(
9815+
batch_line.contains("partitions_searched=2"),
9816+
"batch node must report the distinct partitions searched, got:\n{}",
9817+
batch_line
9818+
);
9819+
97769820
let batch = scan.try_into_batch().await.unwrap();
97779821
assert_query_index_field(&batch);
97789822
assert_eq!(
@@ -10056,6 +10100,80 @@ mod test {
1005610100
}
1005710101
}
1005810102

10103+
/// The shared-scan fast path is only equivalent to repeated single-query
10104+
/// search when the selected `index_segments` cover every requested fragment.
10105+
/// With one segment per fragment, requesting both fragments but selecting
10106+
/// only the first segment leaves fragment 1 covered solely by the
10107+
/// *unselected* segment: the batch node would search just the selected
10108+
/// segment and silently drop it. Eligibility must be computed from the
10109+
/// selected segments' coverage (as `knn_combined` does), not the whole
10110+
/// logical index, so the scanner falls back to the per-query loop, which
10111+
/// re-scores the uncovered fragment on the flat path and returns every row.
10112+
#[tokio::test]
10113+
async fn test_batch_knn_indexed_partial_segment_selection_falls_back() {
10114+
let mut test_ds = TestVectorDataset::new(LanceFileVersion::Stable, false)
10115+
.await
10116+
.unwrap();
10117+
// One segment per fragment: segment_ids[0] covers fragment 0 (i=0..200),
10118+
// segment_ids[1] covers fragment 1 (i=200..400).
10119+
let segment_ids = test_ds.make_segmented_vector_index().await.unwrap();
10120+
let dataset = &test_ds.dataset;
10121+
let fragments = dataset.fragments();
10122+
assert_eq!(fragments.len(), 2, "base dataset should have two fragments");
10123+
10124+
let (queries, _query_values) = batch_knn_two_queries();
10125+
// k covers every row in both requested fragments (200 each), so a
10126+
// complete search returns 400 rows per query.
10127+
let k = 400;
10128+
10129+
let mut scan = dataset.scan();
10130+
scan.nearest("vec", &queries, k).unwrap();
10131+
scan.nprobes(2);
10132+
// Request both indexed fragments but select only the segment covering
10133+
// fragment 0; fragment 1 is covered only by the unselected segment.
10134+
scan.with_fragments(vec![fragments[0].clone(), fragments[1].clone()]);
10135+
scan.with_index_segments(vec![segment_ids[0]]).unwrap();
10136+
scan.project(&["i"]).unwrap();
10137+
10138+
let plan = scan.explain_plan(false).await.unwrap();
10139+
assert!(
10140+
!plan.contains("ANNIvfBatch"),
10141+
"a requested fragment outside the selected segments must force a fallback, \
10142+
not a shared scan that drops it, got:\n{plan}"
10143+
);
10144+
10145+
let batch = scan.try_into_batch().await.unwrap();
10146+
assert_query_index_field(&batch);
10147+
assert_eq!(
10148+
batch.num_rows(),
10149+
2 * k,
10150+
"each query must return all 400 rows across both requested fragments"
10151+
);
10152+
let query_indices = batch[QUERY_INDEX_COL].as_primitive::<Int32Type>();
10153+
for query_index in 0..2 {
10154+
let rows_for_query = query_indices
10155+
.iter()
10156+
.filter(|value| *value == Some(query_index))
10157+
.count();
10158+
assert_eq!(
10159+
rows_for_query, k,
10160+
"query_index {query_index} must cover both fragments (got {rows_for_query})"
10161+
);
10162+
}
10163+
// Fragment 0 (i in 0..200) comes from the selected segment; fragment 1
10164+
// (i in 200..400) must appear via the flat fallback.
10165+
let i_array = batch["i"].as_primitive::<Int32Type>();
10166+
assert!(
10167+
i_array
10168+
.iter()
10169+
.any(|v| v.is_some_and(|val| (0..200).contains(&val)))
10170+
&& i_array
10171+
.iter()
10172+
.any(|v| v.is_some_and(|val| (200..400).contains(&val))),
10173+
"results must include rows from both the selected segment and the flat-fallback fragment"
10174+
);
10175+
}
10176+
1005910177
/// A wide batch probes more distinct partitions than one streaming chunk holds
1006010178
/// (`STREAMING_SEARCH_BATCH_SIZE` = 16), so `search_partitions_batch` scores
1006110179
/// them in several `spawn_cpu` dispatches, threading the per-query top-k heaps

rust/lance/src/io/exec/knn.rs

Lines changed: 105 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,58 @@ async fn find_partitions_on_cpu(
126126
.map_err(|e| DataFusionError::Execution(format!("Failed to find partitions: {}", e)))
127127
}
128128

129+
/// Per-query IVF partition rankings: for each query, its probed-partition ids
130+
/// and the corresponding centroid distances, in query order.
131+
type BatchPartitionRankings = (Vec<Arc<UInt32Array>>, Vec<Arc<Float32Array>>);
132+
133+
/// Rank every query vector in a batch against the IVF centroids on the CPU
134+
/// runtime.
135+
///
136+
/// [`VectorIndex::find_partitions`] is pure CPU work, and a wide batch over a
137+
/// large centroid set multiplies it enough to monopolize a Tokio worker and
138+
/// stall unrelated async progress. Dispatch the whole ranking loop as a single
139+
/// `spawn_cpu` job -- mirroring the single-query [`find_partitions_on_cpu`] --
140+
/// so it stays off the async executor threads.
141+
///
142+
/// `query.key` holds all `query_count` vectors concatenated and `dim` is the
143+
/// per-vector width. Returns each query's probed-partition list and the
144+
/// corresponding centroid distances, in query order.
145+
async fn find_partitions_batch_on_cpu(
146+
index: Arc<dyn VectorIndex>,
147+
query: Query,
148+
query_count: usize,
149+
dim: usize,
150+
) -> DataFusionResult<BatchPartitionRankings> {
151+
spawn_cpu(move || -> Result<BatchPartitionRankings> {
152+
let mut partitions_per_query = Vec::with_capacity(query_count);
153+
let mut dists_per_query = Vec::with_capacity(query_count);
154+
for query_index in 0..query_count {
155+
let mut single_query = query.clone();
156+
single_query.key = query.key.slice(query_index * dim, dim);
157+
// Probe a fixed number of partitions per query. The scanner only
158+
// routes here when `minimum_nprobes == maximum_nprobes` and
159+
// `minimum_nprobes > 0` (see `Scanner::batch_index_search_supported`),
160+
// so this is exactly what the single-query path would search -- no
161+
// adaptive `early_pruning` floor or late-search expansion applies,
162+
// making the batch result identical to repeated single-query search.
163+
// No clamp is needed: the gate rejects `nprobes(0)` (which the
164+
// single-query path treats as "probe nothing") rather than silently
165+
// searching one partition here.
166+
debug_assert!(
167+
single_query.minimum_nprobes > 0,
168+
"batch node reached with nprobes(0); the scanner gate should have fallen back"
169+
);
170+
single_query.maximum_nprobes = Some(single_query.minimum_nprobes);
171+
let (partitions, q_c_dists) = index.find_partitions(&single_query)?;
172+
partitions_per_query.push(Arc::new(partitions));
173+
dists_per_query.push(Arc::new(q_c_dists));
174+
}
175+
Ok((partitions_per_query, dists_per_query))
176+
})
177+
.await
178+
.map_err(|e| DataFusionError::Execution(format!("Failed to find partitions: {e}")))
179+
}
180+
129181
fn normalize_query_for_index(index: &dyn VectorIndex, query: Query) -> DataFusionResult<Query> {
130182
if index.metric_type() != DistanceType::Cosine {
131183
return Ok(query);
@@ -2620,30 +2672,32 @@ impl ExecutionPlan for ANNIvfBatchExec {
26202672
dim,
26212673
)?;
26222674

2623-
let mut partitions_per_query = Vec::with_capacity(query_count);
2624-
let mut dists_per_query = Vec::with_capacity(query_count);
2625-
for query_index in 0..query_count {
2626-
let mut single_query = normalized.clone();
2627-
single_query.key = normalized.key.slice(query_index * dim, dim);
2628-
// Probe a fixed number of partitions per query. The scanner
2629-
// only routes here when `minimum_nprobes == maximum_nprobes`
2630-
// and `minimum_nprobes > 0` (see
2631-
// `Scanner::batch_index_search_supported`), so this is exactly
2632-
// what the single-query path would search — no adaptive
2633-
// `early_pruning` floor or late-search expansion applies,
2634-
// making the batch result identical to repeated single-query
2635-
// search. No clamp is needed: the gate rejects `nprobes(0)`
2636-
// (which the single-query path treats as "probe nothing")
2637-
// rather than silently searching one partition here.
2638-
debug_assert!(
2639-
single_query.minimum_nprobes > 0,
2640-
"batch node reached with nprobes(0); the scanner gate should have fallen back"
2641-
);
2642-
single_query.maximum_nprobes = Some(single_query.minimum_nprobes);
2643-
let (partitions, q_c_dists) = index.find_partitions(&single_query)?;
2644-
partitions_per_query.push(Arc::new(partitions));
2645-
dists_per_query.push(Arc::new(q_c_dists));
2646-
}
2675+
// Rank every query vector against the IVF centroids on the CPU
2676+
// runtime rather than inside this async future: the ranking is
2677+
// pure CPU and the batch width multiplies it, so a wide batch
2678+
// over a large centroid set could otherwise monopolize a Tokio
2679+
// worker. See `find_partitions_batch_on_cpu`.
2680+
let (partitions_per_query, dists_per_query) = find_partitions_batch_on_cpu(
2681+
index.clone(),
2682+
normalized.clone(),
2683+
query_count,
2684+
dim,
2685+
)
2686+
.await?;
2687+
2688+
// Record the partitions this delta actually reads. The batch
2689+
// node loads each probed partition once and scores every query
2690+
// that probes it, so the honest "partitions searched" count is
2691+
// the union across queries -- the shared I/O this node exists to
2692+
// save -- not the per-query sum. Mirrors the single-query
2693+
// ANNIvfSubIndexExec, which also records PARTITIONS_SEARCHED.
2694+
let distinct_partitions: RoaringBitmap = partitions_per_query
2695+
.iter()
2696+
.flat_map(|parts| parts.values().iter().copied())
2697+
.collect();
2698+
metrics
2699+
.partitions_searched
2700+
.add(distinct_partitions.len() as usize);
26472701

26482702
let index_metrics: Arc<dyn MetricsCollector> =
26492703
Arc::new(metrics.index_metrics.clone());
@@ -3692,6 +3746,33 @@ mod tests {
36923746
);
36933747
}
36943748

3749+
// Batch analogue of `test_find_partitions_runs_on_cpu_runtime`: the batch
3750+
// node's per-query centroid ranking multiplies the CPU cost, so it must also
3751+
// run on the dedicated cpu runtime rather than a Tokio async worker.
3752+
#[tokio::test]
3753+
async fn test_find_partitions_batch_runs_on_cpu_runtime() {
3754+
let thread_name = Arc::new(Mutex::new(None));
3755+
let index: Arc<dyn VectorIndex> = Arc::new(ThreadCapturingIndex {
3756+
thread_name: thread_name.clone(),
3757+
row_ids: Vec::new(),
3758+
});
3759+
3760+
// Two query vectors of dim 1 concatenated into one key.
3761+
let mut query = base_query();
3762+
query.key = Arc::new(Float32Array::from(vec![0.0f32, 1.0f32]));
3763+
let (partitions, dists) = find_partitions_batch_on_cpu(index, query, 2, 1)
3764+
.await
3765+
.unwrap();
3766+
assert_eq!(partitions.len(), 2, "one partition list per query");
3767+
assert_eq!(dists.len(), 2, "one distance list per query");
3768+
3769+
let thread_name = thread_name.lock().unwrap().clone().unwrap();
3770+
assert!(
3771+
thread_name.contains("lance-cpu"),
3772+
"expected batch find_partitions to run on the dedicated cpu runtime, got thread {thread_name}",
3773+
);
3774+
}
3775+
36953776
// All partitions fit in a single search batch, so they are searched in one
36963777
// `spawn_cpu` dispatch and therefore share one cpu thread. The partition count
36973778
// adapts to the configured batch size so the single-batch property holds under

0 commit comments

Comments
 (0)