From e6444ecc058e909893bb0a2974e37ff45738da0b Mon Sep 17 00:00:00 2001 From: "lancedb-gatefixer[bot]" <313497061+lancedb-gatefixer[bot]@users.noreply.github.com> Date: Thu, 6 Aug 2026 16:49:49 +0800 Subject: [PATCH] fix(rust): handle missing mirrored copy sources (#3843) ## Summary - treat `NotFound` from the mirrored secondary copy as a cache miss while preserving every other secondary error - perform the durable primary copy after either a successful secondary copy or a secondary cache miss - cover both an initially missing secondary manifest and eviction immediately before the secondary copy ## Root cause Readers can use process-local secondary stores that do not contain a staging manifest written by another process, or that evict it before finalization. `MirroringObjectStore::copy_opts` propagated that secondary `NotFound`, so older object_store versions could loop indefinitely and the locked version aborted before performing the durable primary copy. ## Validation - `cargo fmt --all -- --check` - `cargo test --quiet --features remote -p lancedb io::object_store::test::test_copy_when -- --nocapture` - `cargo check --quiet --features remote --tests --examples` - `cargo clippy --quiet --features remote --tests --examples` - `cargo test --quiet --features remote --tests` Fixes #1176 --------- Co-authored-by: Gatefixer <313497061+lancedb-gatefixer[bot]@users.noreply.github.com> --- rust/lancedb/src/io/object_store.rs | 147 +++++++++++++++++++++++++++- 1 file changed, 143 insertions(+), 4 deletions(-) diff --git a/rust/lancedb/src/io/object_store.rs b/rust/lancedb/src/io/object_store.rs index d27357b82..d594bd857 100644 --- a/rust/lancedb/src/io/object_store.rs +++ b/rust/lancedb/src/io/object_store.rs @@ -132,9 +132,14 @@ impl ObjectStore for MirroringObjectStore { if to.primary_only() { self.primary.copy_opts(from, to, options).await } else { - self.secondary.copy_opts(from, to, options.clone()).await?; - self.primary.copy_opts(from, to, options).await?; - Ok(()) + // The secondary store can be process-local and less durable than the + // primary, so a source written by another process may not exist here + // or may be evicted before the copy begins. + match self.secondary.copy_opts(from, to, options.clone()).await { + Ok(()) | Err(Error::NotFound { .. }) => {} + Err(err) => return Err(err), + } + self.primary.copy_opts(from, to, options).await } } } @@ -192,7 +197,8 @@ mod test { use futures::TryStreamExt; use lance::{dataset::WriteParams, io::ObjectStoreParams}; use lance_testing::datagen::{BatchGenerator, IncrementingInt32, RandomVector}; - use object_store::local::LocalFileSystem; + use object_store::{local::LocalFileSystem, memory::InMemory}; + use std::time::Duration; use tempfile; use crate::{ @@ -201,6 +207,139 @@ mod test { table::WriteOptions, }; + #[derive(Debug)] + struct EvictBeforeCopyStore { + inner: Arc, + } + + impl std::fmt::Display for EvictBeforeCopyStore { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "EvictBeforeCopyStore") + } + } + + #[async_trait] + impl ObjectStore for EvictBeforeCopyStore { + async fn put_opts( + &self, + location: &Path, + payload: PutPayload, + options: PutOptions, + ) -> Result { + self.inner.put_opts(location, payload, options).await + } + + async fn put_multipart_opts( + &self, + location: &Path, + options: PutMultipartOptions, + ) -> Result> { + self.inner.put_multipart_opts(location, options).await + } + + async fn get_opts(&self, location: &Path, options: GetOptions) -> Result { + self.inner.get_opts(location, options).await + } + + fn delete_stream( + &self, + locations: BoxStream<'static, Result>, + ) -> BoxStream<'static, Result> { + self.inner.delete_stream(locations) + } + + fn list(&self, prefix: Option<&Path>) -> BoxStream<'static, Result> { + self.inner.list(prefix) + } + + async fn list_with_delimiter(&self, prefix: Option<&Path>) -> Result { + self.inner.list_with_delimiter(prefix).await + } + + async fn copy_opts(&self, from: &Path, to: &Path, options: CopyOptions) -> Result<()> { + self.inner.delete(from).await?; + self.inner.copy_opts(from, to, options).await + } + } + + #[tokio::test] + async fn test_copy_when_source_is_missing_from_secondary() { + let primary_dir = tempfile::tempdir().unwrap(); + let secondary_dir = tempfile::tempdir().unwrap(); + let primary: Arc = + Arc::new(LocalFileSystem::new_with_prefix(primary_dir.path()).unwrap()); + let secondary: Arc = + Arc::new(LocalFileSystem::new_with_prefix(secondary_dir.path()).unwrap()); + let store = MirroringObjectStore { + primary: primary.clone(), + secondary: secondary.clone(), + }; + let staging = Path::from("_versions/1.manifest-staging"); + let finalized = Path::from("_versions/1.manifest"); + + primary + .put(&staging, "manifest contents".into()) + .await + .unwrap(); + + tokio::time::timeout(Duration::from_secs(5), store.copy(&staging, &finalized)) + .await + .expect("copy should not hang when the secondary source is missing") + .unwrap(); + + let copied = primary + .get(&finalized) + .await + .unwrap() + .bytes() + .await + .unwrap(); + assert_eq!(copied, "manifest contents"); + assert!(matches!( + secondary.head(&finalized).await, + Err(Error::NotFound { .. }) + )); + } + + #[tokio::test] + async fn test_copy_when_secondary_source_disappears_after_head() { + let primary: Arc = Arc::new(InMemory::new()); + let secondary_inner: Arc = Arc::new(InMemory::new()); + let secondary: Arc = Arc::new(EvictBeforeCopyStore { + inner: secondary_inner.clone(), + }); + let store = MirroringObjectStore { + primary: primary.clone(), + secondary, + }; + let staging = Path::from("_versions/1.manifest-staging"); + let finalized = Path::from("_versions/1.manifest"); + + primary + .put(&staging, "manifest contents".into()) + .await + .unwrap(); + secondary_inner + .put(&staging, "manifest contents".into()) + .await + .unwrap(); + + store.copy(&staging, &finalized).await.unwrap(); + + let copied = primary + .get(&finalized) + .await + .unwrap() + .bytes() + .await + .unwrap(); + assert_eq!(copied, "manifest contents"); + assert!(matches!( + secondary_inner.head(&finalized).await, + Err(Error::NotFound { .. }) + )); + } + // This test is ignored because lance 3.0 introduced LocalWriter optimization // that bypasses the object store wrapper for local writes. The mirroring feature // still works for remote/cloud storage, but can't be tested with local storage.