Skip to content

feat: object store decides scheduler type#6373

Merged
westonpace merged 3 commits intomainfrom
feat-lite-scheduler-uring-fs
Apr 1, 2026
Merged

feat: object store decides scheduler type#6373
westonpace merged 3 commits intomainfrom
feat-lite-scheduler-uring-fs

Conversation

@westonpace
Copy link
Copy Markdown
Member

Summary

  • Add ObjectStore::prefers_lite_scheduler() that returns true for file+uring:// stores, so the lite scheduler is used automatically without needing the env var
  • Change SchedulerConfig::use_lite_scheduler from bool to Option<bool>Some(true/false) overrides, None defers to the object store's preference
  • LANCE_USE_LITE_SCHEDULER env var still works as an override when explicitly set

Test plan

  • cargo check -p lance-io --tests --benches compiles cleanly
  • cargo test -p lance-io — all 148 tests pass
  • cargo clippy -p lance-io --tests --benches -- -D warnings — no warnings

🤖 Generated with Claude Code

The lite scheduler was designed for io_uring (lazy polling, no dedicated
I/O loop thread), so a file+uring:// object store should automatically
use it. This changes SchedulerConfig::use_lite_scheduler from bool to
Option<bool> and resolves the final value in ScanScheduler::new() using
the object store's preference as the default, with the env var serving
as an override.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot added the enhancement New feature or request label Apr 1, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 1, 2026

Review

Clean, focused PR. Two items worth addressing:

P1: LANCE_USE_LITE_SCHEDULER env var doesn't respect falsy values

use_lite_scheduler: if std::env::var("LANCE_USE_LITE_SCHEDULER").is_ok() {
    Some(true)
} else {
    None
},

LANCE_USE_LITE_SCHEDULER=0 or LANCE_USE_LITE_SCHEDULER=false will still enable the lite scheduler since .is_ok() only checks presence, not value. Now that the field is Option<bool>, this is a good opportunity to parse the value properly (e.g., treat "0", "false", "" as Some(false) and "1", "true" as Some(true)), so users can also force-disable the lite scheduler via env var when the object store would otherwise prefer it.

P1: No test coverage for the new fallback logic

The tri-state resolution (Option<bool> → object store preference) and prefers_lite_scheduler() are untested. A unit test that constructs a file+uring:// object store and verifies the scheduler selection falls through to prefers_lite_scheduler() when use_lite_scheduler is None would prevent regressions.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 1, 2026

Codecov Report

❌ Patch coverage is 98.18182% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance-io/src/scheduler.rs 98.00% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

… test

Address review feedback:
- Use str_is_truthy to parse the env var value so that "0"/"false" map
  to Some(false) (force-disable) instead of always enabling on presence.
- Add test_object_store_selects_scheduler covering all four cases: None
  with memory (standard), None with file+uring (lite), Some(false)
  override, and Some(true) override.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
file+uring:// is a local filesystem backend (io_uring), but is_local()
only matched "file" and is_cloud() excluded only "file" and "memory",
so file+uring was incorrectly classified as cloud. This affected
heuristics like scanner byte-width thresholds and skipped local-only
code paths (copy, remove_dir_all). Define is_cloud() in terms of
is_local() to keep the logic in one place.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@westonpace westonpace merged commit 1a1094d into main Apr 1, 2026
28 checks passed
@westonpace westonpace deleted the feat-lite-scheduler-uring-fs branch April 1, 2026 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants