Skip to content

Commit d8542b5

Browse files
westonpaceclaude
andauthored
perf!: run scheduler initialize eagerly in async read_tasks (#6710)
BREAKING CHANGE: the rust file reader's read methods are now async. This is to allow the caller control over when the scheduler initialization and inline scheduling occurs so that they can parallelize this work across fragments, if appropriate. ## Summary Fix the v6→v7 inline-scheduling regression by running the decode scheduler's `initialize` I/O eagerly on the awaiting task instead of smuggling it into the returned stream's first poll. This is offered as an alternative to #6709 (which reverted #6637 entirely): we keep the inline-scheduling optimization for small reads, but make the work explicit and properly parallelized across fragments. ## Background #6637 introduced an "inline scheduling" path that, for small reads, attached the scheduler future to the front of the returned stream via `flatten_stream` and only ran it on first poll. Combined with the per-fragment `try_flatten` in `FilteredReadExec` (`rust/lance/src/io/exec/filtered_read.rs:455`) and `LanceScanExec` — both of which poll one inner stream at a time — this serialized the scheduler's `initialize` I/O across fragments. `StructuralPrimitiveFieldScheduler::initialize` (`rust/lance-encoding/src/encodings/logical/primitive.rs:3422`) does a real `io.submit_request(...).await` for chunk metadata. The cache is per-file (the `FieldDataCacheKey` is column-scoped within a file's metadata cache), so every fragment open misses. With 800 small fragments × tens of ms of S3 latency, the inline path was catastrophic. ## Repro [gist](https://gist.github.com/wkalt/e080fc9ddff6edd8eaee5ab50a069fbe) — 400k rows / 800 fragments × 500 rows, KNN brute force, no index: | | before fix | after fix | |-----------------------|---------------|-----------------| | default | ~60–66 ms | **~48–52 ms** | | `LANCE_INLINE_SCHEDULING_THRESHOLD=0` (spawn) | ~46 ms | ~50–52 ms | Default and spawn are now matched. Cross-fragment-count ablation shows no regression at any scale (default tracks spawn ±2 ms across rpf=500/2000/8000/50000). ## Approach The goal was to make the scheduling work explicit, not "smuggled into the poll of the first batch." 1. **`schedule_and_decode` is now `async`** (`rust/lance-encoding/src/decoder.rs`). It awaits `DecodeBatchScheduler::try_new` (which runs `initialize`) before returning. For the inline branch, it then runs the synchronous `schedule_ranges` / `schedule_take` work in line, leaving a fully primed decode stream. The non-inline branch still spawns the scheduling task so it can overlap with decoding. 2. **Cascade async through the file-reader surface.** All of `FileReader::read_tasks`, `read_range`, `read_ranges`, `read_stream`, and `read_stream_projected` are now `async`. Each got a "Why is this async?" doc paragraph explaining that the decode scheduler's metadata I/O happens on the awaiting task rather than on the consumer that polls the stream. 3. **`GenericFileReader` trait methods return `BoxFuture<'_, Result<ReadBatchTaskStream>>`.** V1Reader, the v2 adapter `Reader`, and `NullReader` updated. `FragmentReader::{read_range, read_all, read_ranges, take_range}` and `new_read_impl` are now async; `new_read_impl` uses `try_join_all` so per-data-file `initialize` I/Os run concurrently within a fragment. 4. **Callers updated** in `scan.rs` (v1 + v2 paths), `filtered_read.rs`, `dataset/updater.rs` (`Updater::try_new` made async), `lance-index` (shufflers, distributed index merger, scalar lance_format, vector storage), benches, and `python/src/file.rs`. The fix relies on the existing `SpawnedTask::spawn` of `read_fragment` in `FilteredReadExec` and the `tokio::spawn` of the open task in `LanceScanExec`: the per-fragment task now also drives `initialize`, so all fragments' scheduling I/Os run in parallel up to `fragment_readahead`. ## Behavior change Errors from `initialize` (e.g. corrupted metadata, transient I/O) now surface from the `read_*` await instead of from the first stream item. Existing callers that match on the result of `read_*` keep working; callers that previously assumed the construction was infallible and only the stream could error will now see the error one step earlier. ## Test plan - [x] `cargo check --workspace --tests --benches` — clean - [x] Repro gist — regression resolved (numbers above) - [x] Cross-fragment-count ablation — no regression at any scale - [x] Python tests: 526 passed across `test_dataset`, `test_scalar_index`, `test_blob`, `test_filter`, `test_file`, `test_fragment`, `test_vector_index` (minus the timing-fragile test below) - [ ] Cloud / S3 verification — would expect a much larger improvement than local ## One known test failure to flag `test_create_index_progress_callback_error_before_completion_propagates` fails after this change. It is a **pre-existing timing race exposed by the speedup**, not a correctness break: - The test registers `fail_on_tag="start:train_ivf"` and expects `create_index` to raise. - Mechanism: Rust calls `progress.stage_start("train_ivf").await`, which only does a sync channel send — the callback's error surfaces later, when Python's `block_on_pumping` (`python/src/executor.rs:200-247`) calls `pump()` between 100 ms `tokio::time::sleep`s. - After this change the default-mode operation completes inside the first 100 ms slice often enough that pump doesn't get a chance to surface the error mid-flight. It hits the post-completion branch at `executor.rs:238-244`, which logs and ignores errors from the final pump (`"Ignoring progress callback error after operation completed successfully"`). Running with `LANCE_INLINE_SCHEDULING_THRESHOLD=0` (spawn) makes the test pass. - Earlier perf commits on main may already have exposed variants of this on some platforms; commit 87ef5e2 fixed a related case. The clean fix is in `block_on_pumping` (propagate the final pump's error rather than ignoring it), but that's outside this refactor's scope and changes the contract of "operation succeeded but a callback later errored". Happy to land that as a separate PR if reviewers want. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b7ff253 commit d8542b5

17 files changed

Lines changed: 476 additions & 278 deletions

File tree

python/src/file.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -734,8 +734,6 @@ impl LanceFileReader {
734734
batch_size: u32,
735735
batch_readahead: u32,
736736
) -> PyResult<PyArrowType<Box<dyn RecordBatchReader + Send>>> {
737-
// read_stream is a synchronous method but it launches tasks and needs to be
738-
// run in the context of a tokio runtime
739737
let inner = self.inner.clone();
740738
let stream = rt().block_on(None, async move {
741739
inner
@@ -745,6 +743,7 @@ impl LanceFileReader {
745743
batch_readahead,
746744
FilterExpression::no_filter(),
747745
)
746+
.await
748747
.infer_error()
749748
})??;
750749
Ok(PyArrowType(Box::new(LanceReaderAdapter(stream))))

rust/lance-encoding/src/decoder.rs

Lines changed: 104 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -263,13 +263,28 @@ const BATCH_SIZE_BYTES_WARNING: u64 = 10 * 1024 * 1024;
263263
const ENV_LANCE_STRUCTURAL_BATCH_DECODE_SPAWN_MODE: &str =
264264
"LANCE_STRUCTURAL_BATCH_DECODE_SPAWN_MODE";
265265
const ENV_LANCE_READ_CACHE_REPETITION_INDEX: &str = "LANCE_READ_CACHE_REPETITION_INDEX";
266+
const ENV_LANCE_INLINE_SCHEDULING_THRESHOLD: &str = "LANCE_INLINE_SCHEDULING_THRESHOLD";
267+
268+
// If a request is for at most this many rows we skip the scheduler-task spawn
269+
// and run scheduling inline as part of the `schedule_and_decode` await.
270+
const DEFAULT_INLINE_SCHEDULING_THRESHOLD: u64 = 16 * 1024;
266271

267272
fn default_cache_repetition_index() -> bool {
268273
static DEFAULT_CACHE_REPETITION_INDEX: OnceLock<bool> = OnceLock::new();
269274
*DEFAULT_CACHE_REPETITION_INDEX
270275
.get_or_init(|| parse_env_as_bool(ENV_LANCE_READ_CACHE_REPETITION_INDEX, true))
271276
}
272277

278+
fn inline_scheduling_threshold() -> u64 {
279+
static THRESHOLD: OnceLock<u64> = OnceLock::new();
280+
*THRESHOLD.get_or_init(|| {
281+
std::env::var(ENV_LANCE_INLINE_SCHEDULING_THRESHOLD)
282+
.ok()
283+
.and_then(|v| v.trim().parse::<u64>().ok())
284+
.unwrap_or(DEFAULT_INLINE_SCHEDULING_THRESHOLD)
285+
})
286+
}
287+
273288
/// Top-level encoding message for a page. Wraps both the
274289
/// legacy pb::ArrayEncoding and the newer pb::PageLayout
275290
///
@@ -1956,13 +1971,32 @@ pub struct DecoderConfig {
19561971
pub cache_repetition_index: bool,
19571972
/// Whether to validate decoded data
19581973
pub validate_on_decode: bool,
1974+
/// Override the strategy used to dispatch the scheduling work in
1975+
/// [`schedule_and_decode`].
1976+
///
1977+
/// `schedule_and_decode` always awaits the scheduler's `initialize` (which
1978+
/// performs metadata I/O) before returning. This flag controls what
1979+
/// happens with the subsequent (synchronous) work of pushing decoder
1980+
/// messages into the channel that feeds the decode stream.
1981+
///
1982+
/// * `None` - default behavior: the scheduling work runs inline (as part
1983+
/// of the `schedule_and_decode` await) when the request is small
1984+
/// (controlled by the `LANCE_INLINE_SCHEDULING_THRESHOLD` env var) and
1985+
/// is dispatched onto a spawned task otherwise.
1986+
/// * `Some(true)` - always run scheduling inline. The await of
1987+
/// `schedule_and_decode` does not return until every decoder message
1988+
/// has been queued.
1989+
/// * `Some(false)` - always spawn a task for scheduling so that it can
1990+
/// overlap with consumption of the decode stream.
1991+
pub inline_scheduling: Option<bool>,
19591992
}
19601993

19611994
impl Default for DecoderConfig {
19621995
fn default() -> Self {
19631996
Self {
19641997
cache_repetition_index: default_cache_repetition_index(),
19651998
validate_on_decode: false,
1999+
inline_scheduling: None,
19662000
}
19672001
}
19682002
}
@@ -2089,7 +2123,7 @@ pub fn create_decode_iterator(
20892123
}
20902124
}
20912125

2092-
fn create_scheduler_decoder(
2126+
async fn create_scheduler_decoder(
20932127
column_infos: Vec<Arc<ColumnInfo>>,
20942128
requested_rows: RequestedRows,
20952129
filter: FilterExpression,
@@ -2120,28 +2154,35 @@ fn create_scheduler_decoder(
21202154
config.batch_size_bytes,
21212155
)?;
21222156

2123-
let scheduler_handle = tokio::task::spawn(async move {
2124-
let mut decode_scheduler = match DecodeBatchScheduler::try_new(
2125-
target_schema.as_ref(),
2126-
&column_indices,
2127-
&column_infos,
2128-
&vec![],
2129-
num_rows,
2130-
config.decoder_plugins,
2131-
config.io.clone(),
2132-
config.cache,
2133-
&filter,
2134-
&config.decoder_config,
2135-
)
2136-
.await
2137-
{
2138-
Ok(scheduler) => scheduler,
2139-
Err(e) => {
2140-
let _ = tx.send(Err(e));
2141-
return;
2142-
}
2143-
};
2157+
// The scheduler's `initialize` may perform I/O to load column metadata
2158+
// unless that metadata is already in the cache. This metadata loading
2159+
// happens as part of this call and should be parallelized if reading
2160+
// multiple files.
2161+
let mut decode_scheduler = DecodeBatchScheduler::try_new(
2162+
target_schema.as_ref(),
2163+
&column_indices,
2164+
&column_infos,
2165+
&vec![],
2166+
num_rows,
2167+
config.decoder_plugins,
2168+
config.io.clone(),
2169+
config.cache,
2170+
&filter,
2171+
&config.decoder_config,
2172+
)
2173+
.await?;
21442174

2175+
// For small requests the scheduling cost is dwarfed by the overhead of
2176+
// spawning a task, so we run scheduling inline (still as part of this
2177+
// await) before returning. The threshold is configurable via
2178+
// `LANCE_INLINE_SCHEDULING_THRESHOLD`, and callers can force either
2179+
// strategy via `DecoderConfig::inline_scheduling`.
2180+
let inline_scheduling = config
2181+
.decoder_config
2182+
.inline_scheduling
2183+
.unwrap_or_else(|| num_rows <= inline_scheduling_threshold());
2184+
2185+
if inline_scheduling {
21452186
match requested_rows {
21462187
RequestedRows::Ranges(ranges) => {
21472188
decode_scheduler.schedule_ranges(&ranges, &filter, tx, config.io)
@@ -2150,26 +2191,50 @@ fn create_scheduler_decoder(
21502191
decode_scheduler.schedule_take(&indices, &filter, tx, config.io)
21512192
}
21522193
}
2153-
});
2154-
2155-
Ok(check_scheduler_on_drop(decode_stream, scheduler_handle))
2194+
Ok(decode_stream)
2195+
} else {
2196+
// Spawn the (still synchronous) scheduling work so that decoder
2197+
// messages can stream into the channel while the consumer is
2198+
// already pulling from the decode stream.
2199+
let scheduling = async move {
2200+
match requested_rows {
2201+
RequestedRows::Ranges(ranges) => {
2202+
decode_scheduler.schedule_ranges(&ranges, &filter, tx, config.io)
2203+
}
2204+
RequestedRows::Indices(indices) => {
2205+
decode_scheduler.schedule_take(&indices, &filter, tx, config.io)
2206+
}
2207+
}
2208+
};
2209+
let scheduler_handle = tokio::task::spawn(scheduling);
2210+
Ok(check_scheduler_on_drop(decode_stream, scheduler_handle))
2211+
}
21562212
}
21572213

2158-
/// Launches a scheduler on a dedicated (spawned) task and creates a decoder to
2159-
/// decode the scheduled data and returns the decoder as a stream of record batches.
2214+
/// Initializes the scheduler, schedules the requested rows, and returns a
2215+
/// stream of decode tasks for the resulting batches.
2216+
///
2217+
/// This is a convenience function that creates both the scheduler and the
2218+
/// decoder, which can be a little tricky to get right.
2219+
///
2220+
/// # Why is this async?
21602221
///
2161-
/// This is a convenience function that creates both the scheduler and the decoder
2162-
/// which can be a little tricky to get right.
2163-
pub fn schedule_and_decode(
2222+
/// Constructing the scheduler runs `initialize` which will perform I/O
2223+
/// unless the data required is already in the file metadata cache.
2224+
///
2225+
/// When `DecoderConfig::inline_scheduling` resolves to `true`, the
2226+
/// subsequent (synchronous) scheduling work also runs before this function
2227+
/// returns, leaving a fully primed decode stream.
2228+
pub async fn schedule_and_decode(
21642229
column_infos: Vec<Arc<ColumnInfo>>,
21652230
requested_rows: RequestedRows,
21662231
filter: FilterExpression,
21672232
column_indices: Vec<u32>,
21682233
target_schema: Arc<Schema>,
21692234
config: SchedulerDecoderConfig,
2170-
) -> BoxStream<'static, ReadBatchTask> {
2235+
) -> Result<BoxStream<'static, ReadBatchTask>> {
21712236
if requested_rows.num_rows() == 0 {
2172-
return stream::empty().boxed();
2237+
return Ok(stream::empty().boxed());
21732238
}
21742239

21752240
// If the user requested any ranges that are empty, ignore them. They are pointless and
@@ -2178,27 +2243,19 @@ pub fn schedule_and_decode(
21782243

21792244
let io = config.io.clone();
21802245

2181-
// For convenience we really want this method to be a snchronous method where all
2182-
// errors happen on the stream. There is some async initialization that must happen
2183-
// when creating a scheduler. We wrap that all up in the very first task.
2184-
match create_scheduler_decoder(
2246+
let stream = create_scheduler_decoder(
21852247
column_infos,
21862248
requested_rows,
21872249
filter,
21882250
column_indices,
21892251
target_schema,
21902252
config,
2191-
) {
2192-
// Keep the io alive until the stream is dropped or finishes. Otherwise the
2193-
// I/O drops as soon as the scheduling is finished and the I/O loop terminates.
2194-
Ok(stream) => stream.finally(move || drop(io)).boxed(),
2195-
// If the initialization failed make it look like a failed task
2196-
Err(e) => stream::once(std::future::ready(ReadBatchTask {
2197-
num_rows: 0,
2198-
task: std::future::ready(Err(e)).boxed(),
2199-
}))
2200-
.boxed(),
2201-
}
2253+
)
2254+
.await?;
2255+
2256+
// Keep the io alive until the stream is dropped or finishes. Otherwise the
2257+
// I/O drops as soon as the scheduling is finished and the I/O loop terminates.
2258+
Ok(stream.finally(move || drop(io)).boxed())
22022259
}
22032260

22042261
pub static WAITER_RT: LazyLock<tokio::runtime::Runtime> = LazyLock::new(|| {

rust/lance-file/benches/reader.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ fn bench_reader(c: &mut Criterion) {
8989
None,
9090
FilterExpression::no_filter(),
9191
)
92+
.await
9293
.unwrap();
9394
let stats = Arc::new(Mutex::new((0, 0)));
9495
let mut stream = stream
@@ -305,6 +306,7 @@ fn read_task(
305306
None,
306307
FilterExpression::no_filter(),
307308
)
309+
.await
308310
.unwrap();
309311
let stats = Arc::new(Mutex::new((0, 0)));
310312
let mut stream = stream.then(|batch_task| {

0 commit comments

Comments
 (0)