Description
Session::new builds a spill store eagerly, and its constructor panics rather than returning an error. rust/lance/src/session.rs:130 and :150 both do Arc::new(LocalSpillStore::default()), and the Default impl is Self::new().expect("failed to create temp directory for LocalSpillStore") (rust/lance-io/src/spill.rs:233). So every session creates a temp directory up front, whether or not anything ever spills, and a machine where that directory cannot be created takes down any code path that opens a dataset.
On Windows CI runners this happens intermittently, and it lands on tests that have nothing to do with spilling. From windows-build, in io::commit::conflict_resolver::tests::test_data_overlay_finish_conflicts_with_row_moving_update::case_2_coverage_disjoint_from_moved_row:
failed to create temp directory for LocalSpillStore: IO { source: Custom { kind: PermissionDenied,
error: PathError { path: "C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\.tmp9uNyQx",
err: Os { code: 5, kind: PermissionDenied, message: "Access is denied." } } }
Why it matters
The panic comes out of a Default impl, so no caller can handle it: a library user gets a process abort where an error would do. It also fires for sessions that never spill, since nothing about Session::new knows whether a query will need one.
Expected behavior
Either create the spill store on first use, so a session that never spills never touches the temp directory, or keep the eager construction and propagate the failure as an error from the session constructor.
Environment
Windows runners in GitHub Actions, intermittently. Not observed on Linux or macOS.
Description
Session::newbuilds a spill store eagerly, and its constructor panics rather than returning an error.rust/lance/src/session.rs:130and:150both doArc::new(LocalSpillStore::default()), and theDefaultimpl isSelf::new().expect("failed to create temp directory for LocalSpillStore")(rust/lance-io/src/spill.rs:233). So every session creates a temp directory up front, whether or not anything ever spills, and a machine where that directory cannot be created takes down any code path that opens a dataset.On Windows CI runners this happens intermittently, and it lands on tests that have nothing to do with spilling. From
windows-build, inio::commit::conflict_resolver::tests::test_data_overlay_finish_conflicts_with_row_moving_update::case_2_coverage_disjoint_from_moved_row:Why it matters
The panic comes out of a
Defaultimpl, so no caller can handle it: a library user gets a process abort where an error would do. It also fires for sessions that never spill, since nothing aboutSession::newknows whether a query will need one.Expected behavior
Either create the spill store on first use, so a session that never spills never touches the temp directory, or keep the eager construction and propagate the failure as an error from the session constructor.
Environment
Windows runners in GitHub Actions, intermittently. Not observed on Linux or macOS.