diff --git a/crates/ecstore/src/bucket/metadata.rs b/crates/ecstore/src/bucket/metadata.rs index e2e7e8945..2407a7e5b 100644 --- a/crates/ecstore/src/bucket/metadata.rs +++ b/crates/ecstore/src/bucket/metadata.rs @@ -1464,9 +1464,13 @@ pub(crate) async fn load_bucket_metadata_parse_with_presence( "bucket incarnation sidecar is missing for new-format metadata: {bucket}" ))); } - } else if incarnation.is_some() { - return Err(Error::other("bucket incarnation sidecar exists without bucket metadata")); } + // A sidecar without `.metadata.bin` is the window between the two legacy + // migration writes (rustfs/rustfs#8003): the sidecar lands first, and a + // crash or lost namespace lease before the metadata write leaves the + // bucket in this state on every node. Report it as not persisted so the + // migration runs again; the writers adopt the stored incarnation instead + // of minting a new one, so the sidecar keeps its authority. bm.default_timestamps(); diff --git a/crates/ecstore/src/bucket/metadata_sys.rs b/crates/ecstore/src/bucket/metadata_sys.rs index 06bda6f19..9782ad6e4 100644 --- a/crates/ecstore/src/bucket/metadata_sys.rs +++ b/crates/ecstore/src/bucket/metadata_sys.rs @@ -2504,6 +2504,16 @@ impl BucketMetadataSys { if !persisted { metadata = BucketMetadata::new(bucket); metadata.created = bucket_info.created.unwrap_or(OffsetDateTime::UNIX_EPOCH); + // An interrupted migration may already have published this + // bucket's incarnation; re-read it under the transaction lock so + // the retry never replaces an identity other nodes fenced on. A + // retired incarnation is residue of a deleted bucket and must not + // come back, or heal would reclaim the bucket's new objects. + if let Some(stored) = load_bucket_incarnation(self.object_store(), bucket).await? + && !crate::bucket::retirement::is_retired(self.object_store(), bucket, stored).await? + { + metadata.bucket_incarnation_id = stored; + } } else if metadata.bucket_incarnation_id.is_nil() { metadata.bucket_incarnation_id = Uuid::new_v4(); } @@ -3556,6 +3566,98 @@ mod tests { assert!(err.to_string().contains("sidecar is missing")); } + /// rustfs/rustfs#8003: the legacy migration writes the incarnation sidecar + /// before `.metadata.bin`. A crash or lost namespace lease between the two + /// left every pre-existing bucket answering 500 on every request, with no + /// path back. The sidecar-only state must load as a legacy bucket, and the + /// retried migration must keep the stored incarnation. + #[tokio::test] + async fn issue_8003_sidecar_without_metadata_loads_as_legacy_and_migrates_with_its_incarnation() { + let (dirs, ecstore) = isolated_store_over_temp_disks().await; + let sys = BucketMetadataSys::new(ecstore.clone()); + let bucket = "issue-8003-sidecar-only"; + for dir in &dirs { + std::fs::create_dir_all(dir.path().join(bucket)).unwrap(); + } + let stored = Uuid::new_v4(); + save_bucket_incarnation(ecstore.clone(), bucket, stored) + .await + .expect("simulate the interrupted migration's first write"); + + let (fabricated, persisted) = load_bucket_metadata_parse_with_presence(ecstore.clone(), bucket, true) + .await + .expect("a sidecar without metadata must read as a legacy bucket, not fail closed"); + assert!(!persisted); + assert!(!fabricated.bucket_incarnation_sidecar); + + let (_, fabricated_read) = sys + .get_config(bucket) + .await + .expect("request-path metadata reads must not fail closed on the sidecar-only state"); + assert!(fabricated_read); + + let migrated = sys + .get_authoritative_metadata(bucket) + .await + .expect("the retried legacy migration must complete"); + assert!(migrated.bucket_incarnation_sidecar); + assert_eq!( + migrated.bucket_incarnation_id, stored, + "the retry must adopt the published incarnation instead of minting a new one" + ); + assert_eq!(sys.get_bucket_incarnation_id(bucket).await.unwrap(), stored); + + let on_disk = sys.get_config_from_disk(bucket).await.expect("metadata is now persisted"); + assert!(on_disk.bucket_incarnation_sidecar); + assert_eq!(on_disk.bucket_incarnation_id, stored); + assert_eq!(load_bucket_incarnation(ecstore, bucket).await.unwrap(), Some(stored)); + } + + /// A sidecar left behind by a deleted bucket names a retired incarnation. + /// The migration must mint a new identity for a same-name volume instead + /// of re-publishing the retired one, or heal would treat the bucket's new + /// objects as reclaimable residue. + #[tokio::test] + async fn issue_8003_sidecar_only_migration_does_not_adopt_a_retired_incarnation() { + let (dirs, ecstore) = isolated_store_over_temp_disks().await; + let sys = BucketMetadataSys::new(ecstore.clone()); + let bucket = "issue-8003-retired-sidecar"; + for dir in &dirs { + std::fs::create_dir_all(dir.path().join(bucket)).unwrap(); + } + let retired = Uuid::new_v4(); + save_bucket_incarnation(ecstore.clone(), bucket, retired) + .await + .expect("residual sidecar"); + crate::bucket::retirement::commit_retirement( + ecstore.clone(), + bucket, + retired, + &ObjectOptions { + max_parity: true, + ..Default::default() + }, + ) + .await + .expect("retirement record"); + + let migrated = sys + .get_authoritative_metadata(bucket) + .await + .expect("the migration must still complete for the same-name volume"); + assert!(migrated.bucket_incarnation_sidecar); + assert_ne!( + migrated.bucket_incarnation_id, retired, + "a retired incarnation must never be re-published" + ); + assert!(!migrated.bucket_incarnation_id.is_nil()); + assert_eq!( + load_bucket_incarnation(ecstore, bucket).await.unwrap(), + Some(migrated.bucket_incarnation_id), + "the sidecar must be rewritten to the new incarnation" + ); + } + /// Concurrent cache misses for one bucket must collapse into a single disk /// load. /// diff --git a/crates/ecstore/src/store/bucket.rs b/crates/ecstore/src/store/bucket.rs index d501b255a..72553dc43 100644 --- a/crates/ecstore/src/store/bucket.rs +++ b/crates/ecstore/src/store/bucket.rs @@ -645,6 +645,17 @@ impl ECStore { .as_ref() .and_then(|info| info.created) .unwrap_or(OffsetDateTime::UNIX_EPOCH); + // A sidecar left by an interrupted legacy migration is the + // bucket's published incarnation; keep it rather than minting + // one that would invalidate fences taken on the stored value. + // A retired incarnation is residue of a deleted bucket and + // must not come back. + let store = metadata_sys::object_store_in(&self.ctx).await?; + if let Some(stored) = crate::bucket::metadata::load_bucket_incarnation(store.clone(), bucket).await? + && !crate::bucket::retirement::is_retired(store, bucket, stored).await? + { + metadata.bucket_incarnation_id = stored; + } } else if !metadata.bucket_incarnation_sidecar && !metadata.bucket_incarnation_id.is_nil() { return Err(Error::other(format!( "bucket incarnation sidecar is missing for new-format metadata: {bucket}" @@ -2881,6 +2892,57 @@ mod tests { ); } + /// rustfs/rustfs#8003: a force-create (site replication replay, admin + /// import) against a bucket whose legacy migration stopped after the + /// sidecar write must persist `.metadata.bin` under the stored incarnation + /// rather than fail closed or replace the published identity. + #[tokio::test(flavor = "multi_thread")] + #[serial] + async fn issue_8003_force_create_adopts_existing_incarnation_sidecar() { + let (disk_paths, ecstore) = setup_bucket_delete_test_env().await; + let bucket = format!("bucket-8003-sidecar-{}", Uuid::new_v4().simple()); + + ecstore + .make_bucket(&bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let stored = metadata_sys::get_in(&ecstore.ctx, &bucket) + .await + .expect("metadata should load") + .bucket_incarnation_id; + assert!(!stored.is_nil()); + + let metadata_path = format!("{BUCKET_META_PREFIX}/{bucket}/{}", crate::bucket::metadata::BUCKET_METADATA_FILE); + crate::config::com::delete_config(ecstore.clone(), &metadata_path) + .await + .expect("simulate the interrupted migration by removing only .metadata.bin"); + assert!(!any_disk_path_exists(&disk_paths, format!("{RUSTFS_META_BUCKET}/{metadata_path}")).await); + metadata_sys::remove_bucket_metadata_in(&ecstore.ctx, &bucket) + .await + .expect("drop the cached copy so the next read hits disk"); + + ecstore + .make_bucket( + &bucket, + &MakeBucketOptions { + force_create: true, + ..Default::default() + }, + ) + .await + .expect("force create must repair the sidecar-only bucket"); + + let (repaired, persisted) = metadata_sys::get_config_from_disk_with_presence_in(&ecstore.ctx, &bucket) + .await + .expect("repaired metadata should load"); + assert!(persisted, "force create must persist .metadata.bin again"); + assert!(repaired.bucket_incarnation_sidecar); + assert_eq!( + repaired.bucket_incarnation_id, stored, + "the repair must keep the incarnation the sidecar already published" + ); + } + #[tokio::test(flavor = "multi_thread")] #[serial] async fn make_bucket_seeds_new_bucket_durability_override() {