From 562e03670ec569b0514a8a1182bd8b16a71e8b90 Mon Sep 17 00:00:00 2001 From: Chris Date: Sat, 3 Oct 2026 20:02:39 -0700 Subject: [PATCH] fix(storage): retain typed owners across borrowed writes (#8329) * fix(storage): retain typed owners across borrowed writes * fix(storage): preserve Copy write completion policy * refactor(storage): remove redundant write owner references --- crates/ecstore/src/api/mod.rs | 2 +- .../bucket/lifecycle/recovery_disposition.rs | 16 +- crates/ecstore/src/bucket/metadata_sys.rs | 312 ++++++++++++++++-- crates/ecstore/src/bucket/migration.rs | 8 +- .../ecstore/src/bucket/quota/reservation.rs | 15 +- .../replication/replication_config_store.rs | 32 +- .../bucket/replication/replication_pool.rs | 22 +- .../replication_storage_boundary.rs | 3 +- crates/ecstore/src/config/com.rs | 106 +++--- crates/ecstore/src/core/pools.rs | 308 +++++++++++++---- .../src/ecstore_validation_blackbox.rs | 30 +- crates/ecstore/src/object_api/mod.rs | 2 + crates/ecstore/src/object_api/types.rs | 3 + .../src/object_api/write_commit_context.rs | 186 +++++++++++ .../ecstore/src/services/rebalance/control.rs | 5 +- crates/ecstore/src/set_disk/mod.rs | 3 +- crates/ecstore/src/set_disk/ops/multipart.rs | 66 +++- crates/ecstore/src/set_disk/ops/object.rs | 262 +++++++++++++-- crates/ecstore/src/store/object.rs | 189 +++++++++-- .../heal/tests/shard_identity_receipt_test.rs | 25 +- crates/heal/tests/storage_api.rs | 4 +- crates/lock/src/namespace/mod.rs | 5 + crates/scanner/src/scanner/tests.rs | 56 ++-- crates/scanner/src/scanner_io/tests.rs | 23 +- .../src/scanner_io/tests/service_cohort.rs | 24 +- crates/scanner/src/storage_api.rs | 2 + rustfs/src/admin/handlers/site_replication.rs | 54 +-- .../src/admin/handlers/table_catalog/mod.rs | 12 +- .../src/admin/handlers/table_catalog/tests.rs | 74 +++++ rustfs/src/admin/service/config.rs | 16 +- rustfs/src/admin/service/site_replication.rs | 4 +- rustfs/src/admin/storage_api.rs | 13 +- rustfs/src/app/object/copy.rs | 13 +- rustfs/src/app/storage_api.rs | 1 + rustfs/src/server/module_switch.rs | 8 +- rustfs/src/site_replication/hooks.rs | 20 +- rustfs/src/site_replication/repair.rs | 15 +- rustfs/src/site_replication/retry.rs | 82 ++--- rustfs/src/site_replication/state.rs | 16 +- rustfs/src/site_replication/state_lock.rs | 4 +- rustfs/src/storage/rpc/node_service.rs | 2 +- rustfs/src/storage/storage_api.rs | 17 +- rustfs/src/storage_api.rs | 2 + rustfs/src/table_catalog/store/mod.rs | 43 ++- rustfs/src/table_catalog/store/object.rs | 133 +++++++- 45 files changed, 1774 insertions(+), 464 deletions(-) create mode 100644 crates/ecstore/src/object_api/write_commit_context.rs diff --git a/crates/ecstore/src/api/mod.rs b/crates/ecstore/src/api/mod.rs index d06831afd..121a81878 100644 --- a/crates/ecstore/src/api/mod.rs +++ b/crates/ecstore/src/api/mod.rs @@ -519,7 +519,7 @@ pub mod object { ObjectInfo, ObjectLockConfigSnapshot, ObjectMutationHook, ObjectOptions, PutObjReader, QuotaAdmission, RangedDecompressReader, ReadEncryptionMaterial, ReadEncryptionMode, ReadEncryptionRequest, SCANNER_PUBLICATION_LEASE_FENCE_METADATA_KEY, ScannerPublicationCommitScope, ScannerPublicationCommitStartError, - ScannerPublicationCommitState, ShardIntegrityWriteMode, StreamConsumer, WriteCompletion, + ScannerPublicationCommitState, ShardIntegrityWriteMode, StreamConsumer, WriteCommitGuard, WriteCompletion, get_object_body_cache_plaintext_len, lookup_get_object_body_cache_hook, register_get_object_body_cache_hook, register_object_mutation_hook, unregister_get_object_body_cache_hook, unregister_object_mutation_hook, }; diff --git a/crates/ecstore/src/bucket/lifecycle/recovery_disposition.rs b/crates/ecstore/src/bucket/lifecycle/recovery_disposition.rs index 229ec3eb3..418c1764d 100644 --- a/crates/ecstore/src/bucket/lifecycle/recovery_disposition.rs +++ b/crates/ecstore/src/bucket/lifecycle/recovery_disposition.rs @@ -1122,9 +1122,11 @@ pub(crate) async fn resume_recovery_disposition( // Lock order is control record, then legacy source. Disposition revisions // use ETag CAS and never acquire either namespace lock internally. let control_lock = api.new_ns_lock(RUSTFS_META_BUCKET, &control_object).await?; - let control_guard = control_lock - .get_read_lock(crate::set_disk::get_lock_acquire_timeout()) - .await?; + let control_guard = Arc::new( + control_lock + .get_read_lock(crate::set_disk::get_lock_acquire_timeout()) + .await?, + ); let source_lock = api .new_ns_lock(RUSTFS_META_BUCKET, &initial.disposition.identity.canonical_source_path) .await?; @@ -1504,7 +1506,7 @@ fn control_is_abandoned_successor(disposition: &IlmRecoveryDisposition, control: async fn abandon_recovery_control_fenced( api: Arc, disposition: &IlmRecoveryDisposition, - control_guard: &rustfs_lock::NamespaceLockGuard, + control_guard: &Arc, source_guard: &rustfs_lock::NamespaceLockGuard, proof: &IlmRecoveryExportFleetProofToken, ) -> EcstoreResult<()> { @@ -1557,7 +1559,7 @@ async fn abandon_recovery_control_fenced( }), ..Default::default() }; - options.add_namespace_lock_guard(control_guard); + options.add_owned_write_lock(Arc::clone(control_guard), RUSTFS_META_BUCKET, &object); options.add_namespace_lock_guard(source_guard); let write_result = config_boundary::save_config_with_opts(api.clone(), &object, encoded, &options).await; let observed = match load_recovery_control_no_lock(api.clone(), disposition).await { @@ -1630,7 +1632,7 @@ async fn delete_exact_local_recovery_copy( api: Arc, copy: &IlmRecoverySourceCopy, source_schema: &str, - control_guard: &rustfs_lock::NamespaceLockGuard, + control_guard: &Arc, source_guard: &rustfs_lock::NamespaceLockGuard, ) -> EcstoreResult<()> { let set = recovery_copy_set(&api, ©.authority)?; @@ -1704,7 +1706,7 @@ async fn save_recovery_disposition_step_fenced( api: Arc, current: &ObservedIlmRecoveryDisposition, next: &IlmRecoveryDisposition, - control_guard: &rustfs_lock::NamespaceLockGuard, + control_guard: &Arc, source_guard: &rustfs_lock::NamespaceLockGuard, proof: &IlmRecoveryExportFleetProofToken, ) -> EcstoreResult { diff --git a/crates/ecstore/src/bucket/metadata_sys.rs b/crates/ecstore/src/bucket/metadata_sys.rs index a509c87d4..e04e5fce6 100644 --- a/crates/ecstore/src/bucket/metadata_sys.rs +++ b/crates/ecstore/src/bucket/metadata_sys.rs @@ -1331,10 +1331,54 @@ pub(crate) async fn get_quota_config_and_incarnation_from_disk_in( let bucket_meta_sys_lock = bucket_metadata_sys_of(ctx)?; let bucket_meta_sys = bucket_meta_sys_lock.read().await.clone(); - match bucket_meta_sys - .read_authoritative_metadata_from_disk_under_transaction_lock(bucket) - .await? + quota_config_and_incarnation_from_authority( + bucket, + bucket_meta_sys + .read_authoritative_metadata_from_disk_under_transaction_lock(bucket) + .await?, + ) +} + +pub(crate) async fn get_quota_config_and_incarnation_from_disk_for_options_in( + ctx: &crate::runtime::instance::InstanceContext, + bucket: &str, + opts: &ObjectOptions, + transaction_guard: &Arc, +) -> Result<(Option, Uuid, OffsetDateTime)> { + let Some(snapshot) = opts.object_lock_config_snapshot.as_ref() else { + return get_quota_config_and_incarnation_from_disk_in(ctx, bucket).await; + }; + let sys = bucket_metadata_sys_of(ctx)?.read().await.clone(); + let store = sys.object_store(); + let snapshot_guard = snapshot + .metadata_transaction_guard_for(store.id, bucket, opts.expected_bucket_incarnation_id) + .filter(|guard| Arc::ptr_eq(guard, transaction_guard)) + .ok_or_else(|| Error::other("quota snapshot does not hold this bucket's metadata transaction fence"))?; + if snapshot_guard.is_released() { + return Err(Error::other("quota snapshot metadata transaction fence was released")); + } + + // The snapshot carries the caller's lifecycle fence, excluding deletion. + // Its transaction guard excludes config writes, so a same-name object + // owner need not recursively acquire the bucket namespace here. + let authority = sys + .read_authoritative_metadata_from_disk_under_guard(bucket, transaction_guard) + .await?; + if snapshot_guard.is_lock_lost() + || snapshot_guard.is_released() + || matches!(&authority, BucketMetadataAuthority::Authoritative(metadata) + if !snapshot.is_valid_for_destructive_put(store.id, bucket, metadata.bucket_incarnation_id)) { + return Err(Error::other("quota snapshot fence or bucket incarnation changed during the disk read")); + } + quota_config_and_incarnation_from_authority(bucket, authority) +} + +fn quota_config_and_incarnation_from_authority( + bucket: &str, + authority: BucketMetadataAuthority, +) -> Result<(Option, Uuid, OffsetDateTime)> { + match authority { BucketMetadataAuthority::Authoritative(metadata) if metadata.bucket_incarnation_sidecar && !metadata.bucket_incarnation_id.is_nil() => { @@ -2703,29 +2747,30 @@ impl BucketMetadataSys { async fn read_authoritative_metadata_from_disk_under_transaction_lock( &self, bucket: &str, + ) -> Result { + let namespace_lock = self.object_store().new_ns_lock(bucket, bucket).await?; + let namespace_guard = namespace_lock + .get_read_lock(crate::set_disk::get_lock_acquire_timeout()) + .await?; + self.read_authoritative_metadata_from_disk_under_guard(bucket, &namespace_guard) + .await + } + + async fn read_authoritative_metadata_from_disk_under_guard( + &self, + bucket: &str, + guard: &rustfs_lock::NamespaceLockGuard, ) -> Result { #[cfg(test)] if self.object_lock_disk_read_errors.write().await.remove(bucket) { return Err(Error::other(format!("injected Object Lock metadata disk read failure: {bucket}"))); } - let namespace_lock = self.object_store().new_ns_lock(bucket, bucket).await?; - let namespace_guard = namespace_lock - .get_read_lock(crate::set_disk::get_lock_acquire_timeout()) - .await?; - match await_bucket_namespace_operation( - Some(&namespace_guard), - bucket, - "bucket metadata snapshot existence check", - async { - self.object_store() - .get_bucket_info_from_sets_at_read_quorum( - bucket, - &crate::storage_api_contracts::bucket::BucketOptions::default(), - ) - .await - }, - ) + match await_bucket_namespace_operation(Some(guard), bucket, "bucket metadata snapshot existence check", async { + self.object_store() + .get_bucket_info_from_sets_at_read_quorum(bucket, &crate::storage_api_contracts::bucket::BucketOptions::default()) + .await + }) .await { Ok(_) => {} @@ -2734,7 +2779,7 @@ impl BucketMetadataSys { } let (metadata, persisted) = await_bucket_namespace_operation( - Some(&namespace_guard), + Some(guard), bucket, "bucket metadata authoritative snapshot", load_bucket_metadata_parse_with_presence(self.object_store(), bucket, true), @@ -4736,6 +4781,231 @@ mod tests { .expect("DeleteBucket should succeed"); } + #[tokio::test] + #[serial] + async fn quota_snapshot_reuses_same_name_owner_and_rereads_disk_authority() { + let (_dirs, store) = isolated_store_over_temp_disks().await; + init_bucket_metadata_sys(store.clone(), Vec::new()).await; + let bucket = "quota-same-name-owner"; + store + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("quota fixture bucket"); + let snapshot = store + .object_lock_config_snapshot(bucket) + .await + .expect("guarded Object Lock snapshot"); + let incarnation = store.bucket_incarnation_id(bucket).await.expect("fixture incarnation"); + let transaction_guard = snapshot + .metadata_transaction_guard_for(store.id, bucket, Some(incarnation)) + .expect("snapshot's actual transaction guard"); + let expected = get_quota_config_and_incarnation_from_disk_for_options_in( + &store.ctx, + bucket, + &ObjectOptions::default(), + &transaction_guard, + ) + .await + .expect("a reader without a snapshot must retain the ordinary namespace path"); + + let sys = bucket_metadata_sys_of(&store.ctx).expect("fixture metadata system"); + let sys = sys.read().await.clone(); + let mut stale = (*sys.get(bucket).await.expect("cached fixture metadata")).clone(); + let metadata_path = stale.save_file_path(); + stale.quota_config = Some(BucketQuota { + quota: Some(123), + ..Default::default() + }); + sys.set(bucket.to_string(), Arc::new(stale)).await; + let namespace = store.new_ns_lock(bucket, bucket).await.expect("same-name object namespace"); + let _owner = namespace + .get_write_lock(crate::set_disk::get_lock_acquire_timeout()) + .await + .expect("same-name object write owner"); + let opts = ObjectOptions { + object_lock_config_snapshot: Some(snapshot), + expected_bucket_incarnation_id: Some(incarnation), + ..Default::default() + }; + assert_eq!( + get_quota_config_and_incarnation_from_disk_for_options_in(&store.ctx, bucket, &opts, &transaction_guard) + .await + .expect("quota disk read must reuse the held lifecycle and transaction fences"), + expected, + "the cached quota must not replace disk authority" + ); + + crate::config::com::save_config_with_opts( + store.clone(), + &metadata_path, + b"corrupt bucket metadata".to_vec(), + &ObjectOptions { + write_completion: crate::object_api::WriteCompletion::TailDrained, + ..Default::default() + }, + ) + .await + .expect("inject committed metadata corruption"); + assert!( + get_quota_config_and_incarnation_from_disk_for_options_in(&store.ctx, bucket, &opts, &transaction_guard) + .await + .is_err(), + "a valid snapshot must not hide corrupt disk metadata" + ); + crate::config::com::delete_config(store.clone(), &metadata_path) + .await + .expect("remove the persisted metadata while retaining its sidecar"); + assert!( + get_quota_config_and_incarnation_from_disk_for_options_in(&store.ctx, bucket, &opts, &transaction_guard) + .await + .is_err(), + "a valid snapshot must not promote fabricated metadata to authority" + ); + } + + #[tokio::test] + #[serial] + async fn quota_snapshot_rejects_wrong_scope_incarnation_and_transaction_guard() { + let (_dirs, store) = isolated_store_over_temp_disks().await; + init_bucket_metadata_sys(store.clone(), Vec::new()).await; + let bucket = "quota-snapshot-scope"; + store + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("quota fixture bucket"); + let incarnation = store.bucket_incarnation_id(bucket).await.expect("fixture incarnation"); + let snapshot = store + .object_lock_config_snapshot(bucket) + .await + .expect("guarded Object Lock snapshot"); + let transaction_guard = snapshot + .metadata_transaction_guard_for(store.id, bucket, Some(incarnation)) + .expect("snapshot's actual transaction guard"); + let mut opts = ObjectOptions { + object_lock_config_snapshot: Some(snapshot), + expected_bucket_incarnation_id: Some(incarnation), + ..Default::default() + }; + assert!( + get_quota_config_and_incarnation_from_disk_for_options_in(&store.ctx, "another-bucket", &opts, &transaction_guard) + .await + .is_err() + ); + opts.expected_bucket_incarnation_id = Some(Uuid::new_v4()); + assert!( + get_quota_config_and_incarnation_from_disk_for_options_in(&store.ctx, bucket, &opts, &transaction_guard) + .await + .is_err() + ); + opts.expected_bucket_incarnation_id = Some(incarnation); + let other_guard = Arc::new( + acquire_bucket_metadata_transaction_read_lock_in(&store.ctx, bucket) + .await + .expect("distinct real transaction read guard"), + ); + assert!( + get_quota_config_and_incarnation_from_disk_for_options_in(&store.ctx, bucket, &opts, &other_guard) + .await + .is_err(), + "the same namespace key does not prove that this is the snapshot's actual guard" + ); + let (_other_dirs, other_store) = isolated_store_over_temp_disks().await; + init_bucket_metadata_sys(other_store.clone(), Vec::new()).await; + assert!( + get_quota_config_and_incarnation_from_disk_for_options_in(&other_store.ctx, bucket, &opts, &transaction_guard) + .await + .is_err() + ); + opts.object_lock_config_snapshot = Some(Arc::new(crate::object_api::ObjectLockConfigSnapshot::new( + ObjectLockConfigState::ConfirmedAbsent, + ))); + assert!( + get_quota_config_and_incarnation_from_disk_for_options_in(&store.ctx, bucket, &opts, &transaction_guard) + .await + .is_err(), + "an unguarded snapshot must not authorize namespace reuse" + ); + } + + #[tokio::test] + #[serial] + async fn quota_snapshot_rejects_lifecycle_loss_during_read_and_changed_disk_incarnation() { + let (_dirs, store) = isolated_store_over_temp_disks().await; + init_bucket_metadata_sys(store.clone(), Vec::new()).await; + let bucket = "quota-snapshot-loss"; + store + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("quota fixture bucket"); + let sys = bucket_metadata_sys_of(&store.ctx).expect("fixture metadata system"); + let sys = sys.read().await.clone(); + let mut metadata = (*sys.get(bucket).await.expect("fixture metadata")).clone(); + let incarnation = metadata.bucket_incarnation_id; + let _lifecycle_guard = store + .acquire_bucket_lifecycle_read_lock(bucket) + .await + .expect("real lifecycle owner"); + let transaction_guard = Arc::new( + acquire_bucket_metadata_transaction_read_lock_in(&store.ctx, bucket) + .await + .expect("real metadata transaction owner"), + ); + let (lifecycle_fence, lost) = crate::object_api::NamespaceLockFence::loss_handle_for_test(); + let snapshot = crate::object_api::ObjectLockConfigSnapshot::for_store_bucket_under_lifecycle_fence( + store.id, + bucket, + incarnation, + metadata.object_lock_config_updated_at, + ObjectLockConfigState::ConfirmedAbsent, + lifecycle_fence, + Arc::clone(&transaction_guard), + ); + let opts = ObjectOptions { + object_lock_config_snapshot: Some(Arc::new(snapshot)), + expected_bucket_incarnation_id: Some(incarnation), + ..Default::default() + }; + let mut read = Box::pin(get_quota_config_and_incarnation_from_disk_for_options_in( + &store.ctx, + bucket, + &opts, + &transaction_guard, + )); + assert!( + futures::poll!(read.as_mut()).is_pending(), + "the authoritative disk read must be in flight" + ); + lost.store(true, std::sync::atomic::Ordering::Release); + assert!(read.await.is_err(), "a lifecycle fence lost during the disk read must reject its result"); + + let snapshot = store + .object_lock_config_snapshot(bucket) + .await + .expect("fresh guarded snapshot"); + let transaction_guard = snapshot + .metadata_transaction_guard_for(store.id, bucket, Some(incarnation)) + .expect("snapshot's actual transaction guard"); + let opts = ObjectOptions { + object_lock_config_snapshot: Some(snapshot), + expected_bucket_incarnation_id: Some(incarnation), + ..Default::default() + }; + metadata.bucket_incarnation_id = Uuid::new_v4(); + save_bucket_incarnation(store.clone(), bucket, metadata.bucket_incarnation_id) + .await + .expect("inject a new disk incarnation"); + metadata + .save_with_store_committed(store.clone()) + .await + .expect("persist its matching metadata blob"); + assert!( + get_quota_config_and_incarnation_from_disk_for_options_in(&store.ctx, bucket, &opts, &transaction_guard) + .await + .is_err(), + "a matching sidecar and blob from another incarnation must not reuse the old snapshot" + ); + } + /// Writers on different nodes updating *different* config files of one /// bucket must both survive. Each rewrites the whole metadata blob, so /// without a lock spanning the read-modify-write the later save carries diff --git a/crates/ecstore/src/bucket/migration.rs b/crates/ecstore/src/bucket/migration.rs index 6724a408a..723b75c69 100644 --- a/crates/ecstore/src/bucket/migration.rs +++ b/crates/ecstore/src/bucket/migration.rs @@ -280,11 +280,11 @@ where debug!("Found {} migrating bucket metadata, migrating...", buckets.len()); - let opts = ObjectOptions { + let mut opts = ObjectOptions { max_parity: true, - no_lock: true, ..Default::default() }; + opts.use_offline_recovery_write(); let h = HeaderMap::new(); for bucket in buckets { @@ -397,11 +397,11 @@ where return Ok(()); } - let opts = ObjectOptions { + let mut opts = ObjectOptions { max_parity: true, - no_lock: true, ..Default::default() }; + opts.use_offline_recovery_write(); let h = HeaderMap::new(); let prefix = format!("{IAM_CONFIG_PREFIX}/"); let mut continuation: Option = None; diff --git a/crates/ecstore/src/bucket/quota/reservation.rs b/crates/ecstore/src/bucket/quota/reservation.rs index 7157d3262..0a10b9200 100644 --- a/crates/ecstore/src/bucket/quota/reservation.rs +++ b/crates/ecstore/src/bucket/quota/reservation.rs @@ -273,7 +273,7 @@ impl QuotaContext { reap_stale_reservations(Arc::clone(&store), &ledger_data.bucket, &ledger_data.ledger_object).await?; let ledger_lock = store.new_ns_lock(RUSTFS_META_BUCKET, &ledger_data.ledger_object).await?; - let ledger_guard = ledger_lock.get_write_lock(get_lock_acquire_timeout()).await?; + let ledger_guard = Arc::new(ledger_lock.get_write_lock(get_lock_acquire_timeout()).await?); fence_namespace_mutations(&store, RUSTFS_META_BUCKET, &ledger_data.ledger_object, None).await?; let mut ledger = load_current_ledger_locked( Arc::clone(&store), @@ -503,7 +503,7 @@ pub(crate) async fn begin( let metadata_guard = metadata_sys::acquire_bucket_metadata_transaction_read_lock_for_options_in(ctx, bucket, opts).await?; let (quota, bucket_incarnation, quota_revision) = - metadata_sys::get_quota_config_and_incarnation_from_disk_in(ctx, bucket).await?; + metadata_sys::get_quota_config_and_incarnation_from_disk_for_options_in(ctx, bucket, opts, &metadata_guard).await?; if metadata_guard.is_lock_lost() { return Err(StorageError::NamespaceLockQuorumUnavailable { mode: "quota_config", @@ -623,7 +623,7 @@ async fn mark_commit_started(data: &LedgerReservationData) -> Result<()> { let data = data.clone(); tokio::spawn(async move { let ledger_lock = data.store.new_ns_lock(RUSTFS_META_BUCKET, &data.ledger_object).await?; - let ledger_guard = ledger_lock.get_write_lock(get_lock_acquire_timeout()).await?; + let ledger_guard = Arc::new(ledger_lock.get_write_lock(get_lock_acquire_timeout()).await?); fence_namespace_mutations(&data.store, RUSTFS_META_BUCKET, &data.ledger_object, None).await?; let mut ledger = load_ledger_locked(Arc::clone(&data.store), &data.ledger_object).await?; ledger.mark_commit_started(data.operation_id, &data.reservation)?; @@ -644,7 +644,7 @@ async fn settle(data: &LedgerReservationData, committed: bool) -> Result<()> { // object lock is released and would otherwise revoke a later write's // newly acquired fence for the same object. let ledger_lock = store.new_ns_lock(RUSTFS_META_BUCKET, &ledger_object).await?; - let ledger_guard = ledger_lock.get_write_lock(get_lock_acquire_timeout()).await?; + let ledger_guard = Arc::new(ledger_lock.get_write_lock(get_lock_acquire_timeout()).await?); fence_namespace_mutations(&store, RUSTFS_META_BUCKET, &ledger_object, None).await?; let mut ledger = load_ledger_locked(Arc::clone(&store), &ledger_object).await?; if committed { @@ -727,7 +727,7 @@ async fn reap_stale_reservations(store: Arc, bucket: &str, ledger_objec } let ledger_lock = store.new_ns_lock(RUSTFS_META_BUCKET, ledger_object).await?; - let ledger_guard = ledger_lock.get_write_lock(get_lock_acquire_timeout()).await?; + let ledger_guard = Arc::new(ledger_lock.get_write_lock(get_lock_acquire_timeout()).await?); fence_namespace_mutations(&store, RUSTFS_META_BUCKET, ledger_object, None).await?; let mut ledger = load_ledger_locked(Arc::clone(&store), ledger_object).await?; let cursor_changed = next_cursor.is_some() && ledger.reap_cursor != next_cursor; @@ -887,7 +887,7 @@ async fn save_ledger_locked( store: Arc, ledger_object: &str, ledger: &QuotaLedger, - ledger_guard: &NamespaceLockGuard, + ledger_guard: &Arc, ) -> Result<()> { if ledger_guard.is_lock_lost() { return Err(StorageError::NamespaceLockQuorumUnavailable { @@ -908,7 +908,8 @@ async fn save_ledger_locked( ..Default::default() }; let _ = opts.set_quota_admission(0, u64::MAX); - opts.add_namespace_lock_guard(ledger_guard); + opts.add_owned_write_lock(Arc::clone(ledger_guard), RUSTFS_META_BUCKET, ledger_object); + opts.write_completion = crate::object_api::WriteCompletion::TailDrained; save_config_with_opts(store, ledger_object, serde_json::to_vec(ledger)?, &opts).await } diff --git a/crates/ecstore/src/bucket/replication/replication_config_store.rs b/crates/ecstore/src/bucket/replication/replication_config_store.rs index 5ac6a48af..9f30eb6f3 100644 --- a/crates/ecstore/src/bucket/replication/replication_config_store.rs +++ b/crates/ecstore/src/bucket/replication/replication_config_store.rs @@ -13,7 +13,9 @@ // limitations under the License. use super::replication_error_boundary::Result; -use super::replication_storage_boundary::{HTTPPreconditions, ObjectInfo, ObjectOptions, ReplicationObjectIO}; +use super::replication_storage_boundary::{ + HTTPPreconditions, ObjectInfo, ObjectOptions, ReplicationObjectIO, WriteCommitGuard, WriteCompletion, +}; use crate::config::{com, storageclass}; use std::sync::Arc; @@ -77,13 +79,6 @@ impl ReplicationConfigStore { com::save_config(api, file, data).await } - pub(crate) async fn save_no_lock(api: Arc, file: &str, data: Vec) -> Result<()> - where - S: ReplicationObjectIO, - { - com::save_config_no_lock(api, file, data).await - } - pub(crate) async fn save_conditional( api: Arc, file: &str, @@ -111,21 +106,18 @@ impl ReplicationConfigStore { file: &str, data: Vec, http_preconditions: HTTPPreconditions, + guard: &WriteCommitGuard, ) -> Result<()> where S: ReplicationObjectIO, { - com::save_config_with_opts_quiet( - api, - file, - data, - &ObjectOptions { - max_parity: true, - no_lock: true, - http_preconditions: Some(http_preconditions), - ..Default::default() - }, - ) - .await + let mut opts = ObjectOptions { + max_parity: true, + http_preconditions: Some(http_preconditions), + write_completion: WriteCompletion::TailDrained, + ..Default::default() + }; + opts.add_write_commit_guard(guard); + com::save_config_with_opts_quiet(api, file, data, &opts).await } } diff --git a/crates/ecstore/src/bucket/replication/replication_pool.rs b/crates/ecstore/src/bucket/replication/replication_pool.rs index c1f5ae916..f0eb97e2d 100644 --- a/crates/ecstore/src/bucket/replication/replication_pool.rs +++ b/crates/ecstore/src/bucket/replication/replication_pool.rs @@ -44,7 +44,7 @@ use super::replication_resyncer::{ use super::replication_state::ReplicationStats; use super::replication_storage_boundary::{ HTTPPreconditions, ObjectInfo, ObjectOptions, ObjectToDelete, ReplicationDeletedObject, ReplicationObjectIO, - ReplicationStorage, + ReplicationStorage, WriteCommitGuard, }; use super::replication_target_boundary::{BucketTargetError, ReplicationTargetStore, replication_object_is_ssec_encrypted}; use super::replication_versioning_boundary::ReplicationVersioningStore; @@ -513,7 +513,7 @@ where // Lock order is transaction namespace lock -> force-delete journal object lock. // Keep the transaction guard alive through the conditional write so legacy // writers cannot interleave a read-modify-write transition within this process. - let guard = lock.get_write_lock(ReplicationLockTiming::acquire_timeout()).await?; + let guard = WriteCommitGuard::acquire(&lock, ReplicationLockTiming::acquire_timeout()).await?; let (mut entries, preconditions, exists) = read_force_delete_intents(storage.clone(), file).await?; if !update(&mut entries, exists)? { return Ok(()); @@ -565,7 +565,7 @@ async fn read_force_delete_intents( async fn save_force_delete_intents( storage: Arc, file: &str, - guard: &rustfs_lock::NamespaceLockGuard, + guard: &WriteCommitGuard, entries: Vec, preconditions: HTTPPreconditions, ) -> Result<(), EcstoreError> { @@ -628,7 +628,7 @@ async fn acknowledge_mrf_recovery( let lock = storage .new_ns_lock(ReplicationMetadataStore::rustfs_meta_bucket(), file) .await?; - let guard = lock.get_write_lock(ReplicationLockTiming::acquire_timeout()).await?; + let guard = WriteCommitGuard::acquire(&lock, ReplicationLockTiming::acquire_timeout()).await?; let current = ReplicationConfigStore::read_no_lock_with_metadata_preserve_empty(storage.clone(), file).await; let (current_data, current_etag, current_exists) = match current { Ok((data, object_info)) => (data, object_info.etag, true), @@ -658,7 +658,7 @@ async fn acknowledge_mrf_recovery( } else { encode_mrf_file(&retained)? }; - match ReplicationConfigStore::save_conditional_no_lock(storage.clone(), file, data, preconditions).await { + match ReplicationConfigStore::save_conditional_no_lock(storage.clone(), file, data, preconditions, &guard).await { Ok(()) => return Ok(retained), Err(EcstoreError::PreconditionFailed) => continue, Err(error) => return Err(error), @@ -2522,7 +2522,7 @@ async fn quarantine_mrf_file(storage: &Arc, data: &[u8 continue; } }; - let guard = match lock.get_write_lock(ReplicationLockTiming::acquire_timeout()).await { + let guard = match WriteCommitGuard::acquire(&lock, ReplicationLockTiming::acquire_timeout()).await { Ok(guard) => guard, Err(error) => { warn!( @@ -2573,6 +2573,7 @@ async fn quarantine_mrf_file(storage: &Arc, data: &[u8 ReplicationMetadataStore::MRF_REPLICATION_FILE, Vec::new(), preconditions, + &guard, ) .await { @@ -2627,13 +2628,12 @@ async fn recover_corrupt_mrf_generation( entries: &[MrfReplicateEntry], preconditions: Option, storage: &Arc, - guard: &rustfs_lock::NamespaceLockGuard, + guard: &WriteCommitGuard, pending_payload: &mut Option, started: Instant, ) -> Option { let quarantine_file = format!("{MRF_CORRUPT_FILE_PREFIX}.{}.bin", OffsetDateTime::now_utc().unix_timestamp_nanos()); - if let Err(error) = ReplicationConfigStore::save_no_lock(storage.clone(), &quarantine_file, corrupt_generation.to_vec()).await - { + if let Err(error) = ReplicationConfigStore::save(storage.clone(), &quarantine_file, corrupt_generation.to_vec()).await { warn!( component = LOG_COMPONENT_ECSTORE, subsystem = LOG_SUBSYSTEM_REPLICATION, @@ -2688,6 +2688,7 @@ async fn recover_corrupt_mrf_generation( ReplicationMetadataStore::MRF_REPLICATION_FILE, data, preconditions, + guard, ) .await { @@ -2735,7 +2736,7 @@ async fn append_mrf_entries_to_disk( return None; } }; - let guard = match lock.get_write_lock(ReplicationLockTiming::acquire_timeout()).await { + let guard = match WriteCommitGuard::acquire(&lock, ReplicationLockTiming::acquire_timeout()).await { Ok(guard) => guard, Err(error) => { warn!( @@ -2862,6 +2863,7 @@ async fn append_mrf_entries_to_disk( ReplicationMetadataStore::MRF_REPLICATION_FILE, data, preconditions, + &guard, ) .await { diff --git a/crates/ecstore/src/bucket/replication/replication_storage_boundary.rs b/crates/ecstore/src/bucket/replication/replication_storage_boundary.rs index 1e39819bf..0a5cbd756 100644 --- a/crates/ecstore/src/bucket/replication/replication_storage_boundary.rs +++ b/crates/ecstore/src/bucket/replication/replication_storage_boundary.rs @@ -19,7 +19,8 @@ use super::replication_error_boundary::Error; use super::replication_filemeta_boundary::{replication_state_from_filemeta, version_purge_status_from_filemeta}; pub(crate) type ReplicationObjectStore = crate::store::ECStore; pub(crate) use crate::object_api::{ - GetObjectReader, ObjectInfo, ObjectOptions, PutObjReader, ReplicationStatusWritebackCondition, ReplicationStatusWritebackMode, + GetObjectReader, ObjectInfo, ObjectOptions, PutObjReader, ReplicationStatusWritebackCondition, + ReplicationStatusWritebackMode, WriteCommitGuard, WriteCompletion, }; #[cfg(test)] pub(crate) use crate::object_api::{NamespaceLockFence, NamespaceLockSignalTestFence}; diff --git a/crates/ecstore/src/config/com.rs b/crates/ecstore/src/config/com.rs index 2521729a3..b35e776da 100644 --- a/crates/ecstore/src/config/com.rs +++ b/crates/ecstore/src/config/com.rs @@ -74,7 +74,7 @@ fn config_task_join_error(operation: &'static str, error: tokio::task::JoinError /// The operation must use the corresponding no-lock read/save functions. pub async fn with_server_config_write_lock(store: Arc, operation: F) -> Result where - F: FnOnce() -> Fut + Send + 'static, + F: FnOnce(crate::object_api::WriteCommitGuard) -> Fut + Send + 'static, Fut: std::future::Future + Send + 'static, T: Send + 'static, { @@ -85,8 +85,8 @@ where let transaction_lock = store.new_ns_lock(RUSTFS_META_BUCKET, &transaction_lock).await?; let _transaction_guard = transaction_lock.get_write_lock(get_lock_acquire_timeout()).await?; let namespace_lock = store.new_ns_lock(RUSTFS_META_BUCKET, SERVER_CONFIG_OBJECT).await?; - let _write_guard = namespace_lock.get_write_lock(get_lock_acquire_timeout()).await?; - Ok(operation().await) + let write_guard = crate::object_api::WriteCommitGuard::acquire(&namespace_lock, get_lock_acquire_timeout()).await?; + Ok(operation(write_guard.clone()).await) }) .await .map_err(|error| config_task_join_error("server config write", error))? @@ -119,14 +119,14 @@ where /// one metadata config object. The operation must use no-lock object I/O. pub async fn with_config_object_write_lock(store: Arc, object: String, operation: F) -> Result where - F: FnOnce() -> Fut + Send + 'static, + F: FnOnce(crate::object_api::WriteCommitGuard) -> Fut + Send + 'static, Fut: std::future::Future + Send + 'static, T: Send + 'static, { tokio::spawn(async move { let namespace_lock = store.new_ns_lock(RUSTFS_META_BUCKET, &object).await?; - let _write_guard = namespace_lock.get_write_lock(get_lock_acquire_timeout()).await?; - Ok(operation().await) + let write_guard = crate::object_api::WriteCommitGuard::acquire(&namespace_lock, get_lock_acquire_timeout()).await?; + Ok(operation(write_guard.clone()).await) }) .await .map_err(|error| config_task_join_error("config object write", error))? @@ -638,7 +638,12 @@ where .await } -pub async fn save_config_no_lock(api: Arc, file: &str, data: Vec) -> Result<()> +pub async fn save_config_no_lock( + api: Arc, + file: &str, + data: Vec, + guard: &crate::object_api::WriteCommitGuard, +) -> Result<()> where S: ObjectIO< Error = Error, @@ -650,17 +655,13 @@ where PutObjectReader = PutObjReader, >, { - save_config_with_opts( - api, - file, - data, - &ObjectOptions { - max_parity: true, - no_lock: true, - ..Default::default() - }, - ) - .await + let mut opts = ObjectOptions { + max_parity: true, + write_completion: crate::object_api::WriteCompletion::TailDrained, + ..Default::default() + }; + opts.add_write_commit_guard(guard); + save_config_with_opts(api, file, data, &opts).await } /// `delete_config` with `no_lock` set — for callers already holding the @@ -846,12 +847,12 @@ where Ok(cfg) } -async fn new_and_save_server_config_no_lock(api: Arc) -> Result +async fn new_and_save_server_config_no_lock(api: Arc, guard: &crate::object_api::WriteCommitGuard) -> Result where S: EcstoreObjectIO + StorageAdminApi, { let cfg = new_server_config(); - save_server_config_no_lock(api, &cfg).await?; + save_server_config_no_lock(api, &cfg, guard).await?; Ok(cfg) } @@ -2138,14 +2139,18 @@ where } /// Handle the situation where the configuration file does not exist, create and save a new configuration -async fn handle_missing_config(api: Arc, context: &str, namespace_lock_held: bool) -> Result +async fn handle_missing_config( + api: Arc, + context: &str, + write_guard: Option<&crate::object_api::WriteCommitGuard>, +) -> Result where S: EcstoreObjectIO + StorageAdminApi + NamespaceLocking, { warn!("Configuration not found ({}): Start initializing new configuration", context); let cfg = if runtime_sources::first_cluster_node_is_local().await { - if namespace_lock_held { - new_and_save_server_config_no_lock(api.clone()).await? + if let Some(guard) = write_guard { + new_and_save_server_config_no_lock(api.clone(), guard).await? } else { new_and_save_server_config(api.clone()).await? } @@ -2175,13 +2180,13 @@ where > + StorageAdminApi, S: NamespaceLocking, { - read_config_without_migrate_inner(api, false).await + read_config_without_migrate_inner(api, None).await } /// Reads the server config while an upper layer holds the namespace write lock /// for [`SERVER_CONFIG_OBJECT`]. Missing-config initialization uses the matching /// no-lock save path and therefore cannot recursively acquire the same lock. -pub async fn read_config_without_migrate_no_lock(api: Arc) -> Result +pub async fn read_config_without_migrate_no_lock(api: Arc, guard: &crate::object_api::WriteCommitGuard) -> Result where S: ObjectIO< Error = Error, @@ -2194,7 +2199,7 @@ where > + StorageAdminApi + NamespaceLocking, { - read_config_without_migrate_inner(api, true).await + read_config_without_migrate_inner(api, Some(guard)).await } /// Reads an already-initialized server config while a caller owns a namespace @@ -2204,7 +2209,10 @@ pub async fn read_existing_server_config_no_lock(api: Arc) -> Result(api: Arc, namespace_lock_held: bool) -> Result +async fn read_config_without_migrate_inner( + api: Arc, + write_guard: Option<&crate::object_api::WriteCommitGuard>, +) -> Result where S: ObjectIO< Error = Error, @@ -2220,19 +2228,23 @@ where let config_file = server_config_path(); // Try to read the configuration file. - let data = if namespace_lock_held { + let data = if write_guard.is_some() { read_config_no_lock(api.clone(), &config_file).await } else { read_config(api.clone(), &config_file).await }; match data { - Ok(data) => read_server_config(api, &data, namespace_lock_held).await, - Err(Error::ConfigNotFound) => handle_missing_config(api, "Read the main configuration", namespace_lock_held).await, + Ok(data) => read_server_config(api, &data, write_guard).await, + Err(Error::ConfigNotFound) => handle_missing_config(api, "Read the main configuration", write_guard).await, Err(err) => handle_config_read_error(err, &config_file), } } -async fn read_server_config(api: Arc, data: &[u8], namespace_lock_held: bool) -> Result +async fn read_server_config( + api: Arc, + data: &[u8], + write_guard: Option<&crate::object_api::WriteCommitGuard>, +) -> Result where S: EcstoreObjectIO + StorageAdminApi + NamespaceLocking, { @@ -2242,7 +2254,7 @@ where warn!("Received empty configuration data, try to reread from '{}'", config_file); // Try to read the configuration again - let data = if namespace_lock_held { + let data = if write_guard.is_some() { read_config_no_lock(api.clone(), &config_file).await } else { read_config(api.clone(), &config_file).await @@ -2253,7 +2265,7 @@ where return Ok(cfg.merge()); } Err(Error::ConfigNotFound) => { - return handle_missing_config(api, "Read alternate configuration", namespace_lock_held).await; + return handle_missing_config(api, "Read alternate configuration", write_guard).await; } Err(err) => return handle_config_read_error(err, &config_file), } @@ -2687,7 +2699,7 @@ where /// Saves the server config while an upper layer holds the namespace write /// lock for [`SERVER_CONFIG_OBJECT`]. -pub async fn save_server_config_no_lock(api: Arc, cfg: &Config) -> Result<()> +pub async fn save_server_config_no_lock(api: Arc, cfg: &Config, guard: &crate::object_api::WriteCommitGuard) -> Result<()> where S: ObjectIO< Error = Error, @@ -2699,10 +2711,10 @@ where PutObjectReader = PutObjReader, >, { - save_server_config_inner(api, cfg, true).await + save_server_config_inner(api, cfg, guard).await } -async fn save_server_config_inner(api: Arc, cfg: &Config, no_lock: bool) -> Result<()> +async fn save_server_config_inner(api: Arc, cfg: &Config, guard: &crate::object_api::WriteCommitGuard) -> Result<()> where S: ObjectIO< Error = Error, @@ -2715,11 +2727,7 @@ where >, { let config_file = get_config_file(); - let existing = match if no_lock { - read_config_no_lock(api.clone(), &config_file).await - } else { - read_config(api.clone(), &config_file).await - } { + let existing = match read_config_no_lock(api.clone(), &config_file).await { Ok(v) => Some(v), Err(Error::ConfigNotFound) => None, Err(err) => { @@ -2743,21 +2751,7 @@ where return Ok(()); } - if no_lock { - save_config_with_opts( - api, - &config_file, - data, - &ObjectOptions { - max_parity: true, - no_lock: true, - ..Default::default() - }, - ) - .await - } else { - save_config(api, &config_file, data).await - } + save_config_no_lock(api, &config_file, data, guard).await } pub async fn lookup_configs(cfg: &mut Config, api: Arc) -> Result<()> diff --git a/crates/ecstore/src/core/pools.rs b/crates/ecstore/src/core/pools.rs index 795fe950f..f807eb603 100644 --- a/crates/ecstore/src/core/pools.rs +++ b/crates/ecstore/src/core/pools.rs @@ -3692,8 +3692,8 @@ fn activation_pool_meta_lock_error(err: rustfs_lock::LockError) -> Error { } pub(crate) struct PoolRebalanceActivationFence { - pool_meta_guard: rustfs_lock::NamespaceLockGuard, - rebalance_meta_guard: rustfs_lock::NamespaceLockGuard, + pool_meta_guard: Arc, + rebalance_meta_guard: Arc, fleet_proof: Option, decommission_target_fence_proof: Option, #[cfg(test)] @@ -3720,7 +3720,12 @@ impl PoolRebalanceActivationFence { let forced_lost = self.forced_lost.load(Ordering::Acquire); #[cfg(not(test))] let forced_lost = false; - if forced_lost || self.pool_meta_guard.is_lock_lost() || self.rebalance_meta_guard.is_lock_lost() { + if forced_lost + || self.pool_meta_guard.is_lock_lost() + || self.pool_meta_guard.is_released() + || self.rebalance_meta_guard.is_lock_lost() + || self.rebalance_meta_guard.is_released() + { return Err(Error::other("activation lock lost before metadata commit or worker admission")); } if self @@ -3742,8 +3747,8 @@ impl PoolRebalanceActivationFence { } pub(crate) fn add_namespace_lock_fence(&self, opts: &mut ObjectOptions) { - opts.add_namespace_lock_guard(&self.pool_meta_guard); - opts.add_namespace_lock_guard(&self.rebalance_meta_guard); + opts.add_owned_write_lock(Arc::clone(&self.pool_meta_guard), RUSTFS_META_BUCKET, POOL_META_NAME); + opts.add_owned_write_lock(Arc::clone(&self.rebalance_meta_guard), RUSTFS_META_BUCKET, REBAL_META_NAME); } #[cfg(test)] @@ -3784,8 +3789,8 @@ where .map_err(activation_pool_meta_lock_error)?; Ok(PoolRebalanceActivationFence { - pool_meta_guard, - rebalance_meta_guard, + pool_meta_guard: Arc::new(pool_meta_guard), + rebalance_meta_guard: Arc::new(rebalance_meta_guard), fleet_proof, decommission_target_fence_proof: None, #[cfg(test)] @@ -4067,7 +4072,7 @@ fn rollback_start_decommission_pool_meta(pool_meta: &mut PoolMeta, previous_pool } fn ensure_pool_meta_write_fence(guard: &rustfs_lock::NamespaceLockGuard, operation: &str) -> Result<()> { - if guard.is_lock_lost() { + if guard.is_lock_lost() || guard.is_released() { return Err(Error::other(format!("{operation}: pool metadata distributed fence was lost"))); } Ok(()) @@ -6339,6 +6344,9 @@ fn encode_pool_meta_v3_envelope( } enum PoolMetaPersistenceFence<'a> { + Owned(Arc), + OfflineRecovery, + #[cfg(test)] Distributed(Option>), Activation(&'a PoolRebalanceActivationFence), } @@ -6346,6 +6354,10 @@ enum PoolMetaPersistenceFence<'a> { impl PoolMetaPersistenceFence<'_> { fn ensure_held(&self) -> Result<()> { match self { + Self::Owned(guard) if guard.is_lock_lost() || guard.is_released() => { + Err(Error::other("pool metadata distributed fence was lost before a replica write")) + } + #[cfg(test)] Self::Distributed(Some(signal)) if signal.is_lost() => { Err(Error::other("pool metadata distributed fence was lost before a replica write")) } @@ -6354,14 +6366,30 @@ impl PoolMetaPersistenceFence<'_> { } } - fn add_to_options(&self, opts: &mut ObjectOptions) { + fn add_to_options(&self, opts: &mut ObjectOptions, object: &str) { match self { + Self::Owned(guard) => opts.add_owned_write_lock(Arc::clone(guard), RUSTFS_META_BUCKET, object), + Self::OfflineRecovery => opts.use_offline_recovery_write(), + #[cfg(test)] Self::Distributed(Some(signal)) => opts.add_namespace_lock_lost_signal(Arc::clone(signal)), - Self::Activation(fence) => fence.add_namespace_lock_fence(opts), + Self::Activation(fence) => { + fence.add_namespace_lock_fence(opts); + opts.add_owned_write_lock(Arc::clone(&fence.pool_meta_guard), RUSTFS_META_BUCKET, object); + } + #[cfg(test)] Self::Distributed(None) => {} } } + fn allows_concurrent_phases(&self) -> bool { + match self { + Self::Owned(guard) => guard.lock_lost_signal().is_some(), + #[cfg(test)] + Self::Distributed(signal) => signal.is_some(), + _ => false, + } + } + fn is_activation(&self) -> bool { matches!(self, Self::Activation(_)) } @@ -6662,7 +6690,7 @@ fn pool_meta_cas_options(object: &str, token: &PoolMetaCasToken, fence: &PoolMet http_preconditions: Some(pool_meta_cas_preconditions(token, object)?), ..Default::default() }; - fence.add_to_options(&mut opts); + fence.add_to_options(&mut opts, object); Ok(opts) } @@ -7049,7 +7077,7 @@ where write_state, initialized, PoolMetaIdentityWriteScope::All, - &PoolMetaPersistenceFence::Distributed(None), + &PoolMetaPersistenceFence::OfflineRecovery, &mut transaction_arm, ) .await?; @@ -7076,7 +7104,7 @@ where write_state, false, PoolMetaIdentityWriteScope::Pools(pool_indices), - &PoolMetaPersistenceFence::Distributed(None), + &PoolMetaPersistenceFence::OfflineRecovery, &mut transaction_arm, ) .await?; @@ -8154,11 +8182,16 @@ impl PoolMeta { .cloned() .ok_or_else(|| Error::other("pool metadata save failed: no storage pools available"))?; let pool_meta_lock = pool.new_ns_lock(RUSTFS_META_BUCKET, POOL_META_NAME).await?; - let pool_meta_guard = pool_meta_lock.get_write_lock(get_lock_acquire_timeout()).await?; + let pool_meta_guard = Arc::new(pool_meta_lock.get_write_lock(get_lock_acquire_timeout()).await?); let mut write_state = PoolMetaWriteState::default(); let indices = (0..self.pools.len()).collect::>(); let outcome = self - .save_no_lock_armed_scoped(pools, &mut write_state, pool_meta_guard.lock_lost_signal(), Some(&indices)) + .save_no_lock_armed_scoped( + pools, + &mut write_state, + PoolMetaPersistenceFence::Owned(Arc::clone(&pool_meta_guard)), + Some(&indices), + ) .await?; outcome.disarm(); Ok(()) @@ -8182,14 +8215,16 @@ impl PoolMeta { where S: EcstoreObjectIO, { - let outcome = self.save_no_lock_armed_scoped(pools, write_state, None, None).await?; + let outcome = self + .save_no_lock_armed_scoped(pools, write_state, PoolMetaPersistenceFence::OfflineRecovery, None) + .await?; Ok(outcome.into_committed()) } async fn save_no_lock_with_fence( &self, pools: Vec>, - lock_lost: Option>, + fence: PoolMetaPersistenceFence<'_>, indices: &[usize], ) -> Result<()> where @@ -8197,7 +8232,7 @@ impl PoolMeta { { let mut write_state = PoolMetaWriteState::default(); let outcome = self - .save_no_lock_armed_scoped(pools, &mut write_state, lock_lost, Some(indices)) + .save_no_lock_armed_scoped(pools, &mut write_state, fence, Some(indices)) .await?; outcome.disarm(); Ok(()) @@ -8233,21 +8268,20 @@ impl PoolMeta { &self, pools: Vec>, write_state: &mut PoolMetaWriteState, - lock_lost: Option>, + fence: PoolMetaPersistenceFence<'_>, indices: &[usize], ) -> Result where S: EcstoreObjectIO, { - self.save_no_lock_armed_scoped(pools, write_state, lock_lost, Some(indices)) - .await + self.save_no_lock_armed_scoped(pools, write_state, fence, Some(indices)).await } async fn save_no_lock_armed_scoped( &self, pools: Vec>, write_state: &mut PoolMetaWriteState, - lock_lost: Option>, + fence: PoolMetaPersistenceFence<'_>, indices: Option<&[usize]>, ) -> Result where @@ -8258,7 +8292,6 @@ impl PoolMeta { // returned outcome is dropped before publication, Drop latches the // sticky recovery gate. let mut transaction_arm = write_state.arm_transaction(); - let fence = PoolMetaPersistenceFence::Distributed(lock_lost); let committed = self .save_no_lock_transaction(pools, write_state, &fence, indices, &mut transaction_arm) .await @@ -8489,7 +8522,7 @@ impl PoolMeta { && selection.revision.is_generation_protocol() && !selection.replica_state.needs_repair && !bootstrap_generation_required - && matches!(fence, PoolMetaPersistenceFence::Distributed(Some(_))); + && fence.allows_concurrent_phases(); #[cfg(feature = "e2e-test-hooks")] let phase_barrier = if concurrent_phases && pools.len() == 4 { PoolMetaPhaseBarrier::bind(&previous, &committed, revision, &durable).await? @@ -8635,7 +8668,9 @@ impl PoolMeta { S: EcstoreObjectIO, { let indices = (0..self.pools.len()).collect::>(); - let outcome = self.save_no_lock_armed(pools, write_state, None, &indices).await?; + let outcome = self + .save_no_lock_armed(pools, write_state, PoolMetaPersistenceFence::OfflineRecovery, &indices) + .await?; outcome.disarm(); Ok(()) } @@ -10831,7 +10866,7 @@ impl ECStore { &self, write_state: &mut PoolMetaWriteState, operation: &str, - ) -> Result<(rustfs_lock::NamespaceLockGuard, PoolMeta)> { + ) -> Result<(Arc, PoolMeta)> { self.acquire_pool_meta_write_guard_with_lock_error(write_state, operation, activation_pool_meta_lock_error) .await } @@ -10841,7 +10876,7 @@ impl ECStore { write_state: &mut PoolMetaWriteState, operation: &str, map_lock_error: F, - ) -> Result<(rustfs_lock::NamespaceLockGuard, PoolMeta)> + ) -> Result<(Arc, PoolMeta)> where F: FnOnce(rustfs_lock::LockError) -> Error, { @@ -10862,7 +10897,7 @@ impl ECStore { let selection = load_pool_meta_replicas_observing(self.pools.clone(), true, write_state).await?; write_state.observe_replicas(selection.replica_state); write_state.ensure_write_safe(operation)?; - Ok((pool_meta_guard, selection.meta)) + Ok((Arc::new(pool_meta_guard), selection.meta)) } async fn acquire_pool_meta_read_guard( @@ -11132,7 +11167,7 @@ impl ECStore { pub(crate) async fn acquire_pool_meta_object_heal_fence( &self, target_pool_indices: &[usize], - ) -> Result<(rustfs_lock::NamespaceLockGuard, Vec>)> { + ) -> Result<(Arc, Vec>)> { let mut save_guard = self.pool_meta_save_gate.lock().await; let (pool_meta_guard, snapshot) = self .acquire_pool_meta_write_guard_with_lock_error(&mut save_guard, "pool metadata heal admission failed", Error::from) @@ -11361,7 +11396,12 @@ impl ECStore { ensure_exact_delete_capacity_namespace_fences(opts, bucket, object)?; let outcome = snapshot - .save_no_lock_armed(self.pools.clone(), &mut save_guard, write_guard.lock_lost_signal(), &source_pool_indices) + .save_no_lock_armed( + self.pools.clone(), + &mut save_guard, + PoolMetaPersistenceFence::Owned(Arc::clone(&write_guard)), + &source_pool_indices, + ) .await?; ensure_pool_meta_write_fence(&write_guard, "exact delete capacity reconciliation save failed")?; for (target_pool_index, target_guard) in &target_guards { @@ -11470,7 +11510,7 @@ impl ECStore { .save_no_lock_armed( self.pools.clone(), &mut save_guard, - pool_meta_guard.lock_lost_signal(), + PoolMetaPersistenceFence::Owned(Arc::clone(&pool_meta_guard)), &[source_pool_index], ) .await?; @@ -11659,7 +11699,7 @@ impl ECStore { .save_no_lock_armed( self.pools.clone(), &mut save_guard, - pool_meta_guard.lock_lost_signal(), + PoolMetaPersistenceFence::Owned(Arc::clone(&pool_meta_guard)), &[source_pool_index], ) .await?; @@ -11965,7 +12005,12 @@ impl ECStore { }; if reservation_grew || pending_added > 0 { let outcome = snapshot - .save_no_lock_armed(self.pools.clone(), &mut save_guard, write_guard.lock_lost_signal(), &[source_pool_index]) + .save_no_lock_armed( + self.pools.clone(), + &mut save_guard, + PoolMetaPersistenceFence::Owned(Arc::clone(&write_guard)), + &[source_pool_index], + ) .await?; ensure_pool_meta_write_fence(&write_guard, "decommission target capacity intent save failed")?; snapshot = outcome.committed.clone(); @@ -12137,7 +12182,12 @@ impl ECStore { return result; } let outcome = snapshot - .save_no_lock_armed(self.pools.clone(), save_guard, write_guard.lock_lost_signal(), &[source_pool_index]) + .save_no_lock_armed( + self.pools.clone(), + save_guard, + PoolMetaPersistenceFence::Owned(Arc::clone(write_guard)), + &[source_pool_index], + ) .await?; ensure_pool_meta_write_fence(write_guard, "decommission target capacity progress save failed")?; if let Some(target_guard) = target_guard.as_ref() { @@ -12272,8 +12322,8 @@ impl ECStore { .first() .ok_or_else(|| Error::other("pool metadata recovery has no storage pools"))?; let lock = pool.new_ns_lock(RUSTFS_META_BUCKET, POOL_META_NAME).await?; - let guard = lock.get_write_lock(get_lock_acquire_timeout()).await?; - let fence = PoolMetaPersistenceFence::Distributed(guard.lock_lost_signal()); + let guard = Arc::new(lock.get_write_lock(get_lock_acquire_timeout()).await?); + let fence = PoolMetaPersistenceFence::Owned(Arc::clone(&guard)); let mut candidate = PoolMetaWriteState { aborted_transaction: Arc::new(AtomicBool::new(false)), transaction_failure: Arc::default(), @@ -12449,7 +12499,12 @@ impl ECStore { merge_pool_meta_updates_for_save(&mut snapshot, &pool_meta, indices, "pool metadata save failed")?; } let outcome = snapshot - .save_no_lock_armed(self.pools.clone(), &mut save_guard, pool_meta_guard.lock_lost_signal(), indices) + .save_no_lock_armed( + self.pools.clone(), + &mut save_guard, + PoolMetaPersistenceFence::Owned(Arc::clone(&pool_meta_guard)), + indices, + ) .await?; let mut pool_meta = self.pool_meta.write().await; ensure_pool_meta_write_fence(&pool_meta_guard, "pool metadata save failed")?; @@ -12544,7 +12599,12 @@ impl ECStore { }; let outcome = match snapshot - .save_no_lock_armed(self.pools.clone(), &mut save_guard, pool_meta_guard.lock_lost_signal(), &[idx]) + .save_no_lock_armed( + self.pools.clone(), + &mut save_guard, + PoolMetaPersistenceFence::Owned(Arc::clone(&pool_meta_guard)), + &[idx], + ) .await { Ok(outcome) => outcome, @@ -12610,7 +12670,12 @@ impl ECStore { #[cfg(test)] decommission_test_wrap_result("bucket_completion_before_save", &bucket.name, &bucket.prefix, 0, Ok(()))?; let outcome = snapshot - .save_no_lock_armed(self.pools.clone(), &mut save_guard, pool_meta_guard.lock_lost_signal(), &[idx]) + .save_no_lock_armed( + self.pools.clone(), + &mut save_guard, + PoolMetaPersistenceFence::Owned(Arc::clone(&pool_meta_guard)), + &[idx], + ) .await?; #[cfg(test)] decommission_test_wrap_result("bucket_completion_after_save", &bucket.name, &bucket.prefix, 0, Ok(()))?; @@ -12744,7 +12809,12 @@ impl ECStore { .await?; rollback_start_decommission_pool_meta(&mut snapshot, previous_pool_meta, indices); let outcome = snapshot - .save_no_lock_armed(self.pools.clone(), &mut save_guard, pool_meta_guard.lock_lost_signal(), indices) + .save_no_lock_armed( + self.pools.clone(), + &mut save_guard, + PoolMetaPersistenceFence::Owned(Arc::clone(&pool_meta_guard)), + indices, + ) .await?; let mut pool_meta = self.pool_meta.write().await; ensure_pool_meta_write_fence(&pool_meta_guard, "decommission start rollback failed")?; @@ -13044,7 +13114,7 @@ impl ECStore { save_pool_meta: Save, ) -> Result<()> where - Save: FnOnce(PoolMeta, Option>) -> SaveFuture + Send + 'static, + Save: FnOnce(PoolMeta, PoolMetaPersistenceFence<'static>) -> SaveFuture + Send + 'static, SaveFuture: Future> + Send + 'static, { let store = self.clone(); @@ -13064,7 +13134,7 @@ impl ECStore { save_pool_meta: Save, ) -> Result<()> where - Save: FnOnce(PoolMeta, Option>) -> SaveFuture, + Save: FnOnce(PoolMeta, PoolMetaPersistenceFence<'static>) -> SaveFuture, SaveFuture: Future>, { let owner = owner.as_ref(); @@ -13113,9 +13183,17 @@ impl ECStore { )); } } - let pool_meta_fence = _pool_meta_guard - .as_ref() - .and_then(rustfs_lock::NamespaceLockGuard::lock_lost_signal); + let pool_meta_fence = match _pool_meta_guard.as_ref() { + Some(guard) => PoolMetaPersistenceFence::Owned(Arc::clone(guard)), + None => { + #[cfg(test)] + { + PoolMetaPersistenceFence::OfflineRecovery + } + #[cfg(not(test))] + return Err(Error::other("runtime pool metadata cancellation requires its write-lock owner")); + } + }; // Lock order: start gate, target gates, save gate, distributed pool // metadata fence, rebalance_meta, decommission_cancelers, then @@ -13554,7 +13632,12 @@ impl ECStore { let (save_outcome, save_error) = if changed { ensure_decommission_target_fence_fleet_proof(target_fence_proof.as_ref(), target_fence_proof_required)?; match snapshot - .save_no_lock_armed(self.pools.clone(), &mut save_guard, pool_meta_guard.lock_lost_signal(), &changed_indices) + .save_no_lock_armed( + self.pools.clone(), + &mut save_guard, + PoolMetaPersistenceFence::Owned(Arc::clone(&pool_meta_guard)), + &changed_indices, + ) .await { Ok(outcome) => (Some(outcome), None), @@ -13646,7 +13729,12 @@ impl ECStore { .await?; snapshot.mark_decommission_capacity_blocked(idx, err.to_string(), OffsetDateTime::now_utc())?; let outcome = snapshot - .save_no_lock_armed(self.pools.clone(), &mut save_guard, pool_meta_guard.lock_lost_signal(), &[idx]) + .save_no_lock_armed( + self.pools.clone(), + &mut save_guard, + PoolMetaPersistenceFence::Owned(Arc::clone(&pool_meta_guard)), + &[idx], + ) .await?; ensure_pool_meta_write_fence(&pool_meta_guard, "decommission capacity pause failed")?; { @@ -22023,7 +22111,7 @@ mod tests { let mut state = store.pool_meta_save_gate.lock().await; let (fence, _) = store.acquire_pool_meta_write_guard(&mut state, "test save").await.unwrap(); let outcome = requested - .save_no_lock_armed(store.pools.clone(), &mut state, fence.lock_lost_signal(), &[0]) + .save_no_lock_armed(store.pools.clone(), &mut state, PoolMetaPersistenceFence::Owned(Arc::clone(&fence)), &[0]) .await .unwrap(); assert_eq!(outcome.committed.pools[0].last_update, requested.pools[0].last_update); @@ -24825,11 +24913,12 @@ mod pools_tests { config[3][0].gate = Some(gate.clone()); let fixture = Fixture::new(config); let mut state = fixture.state(); - let mut save = Box::pin( - fixture - .desired - .save_no_lock_armed(fixture.pools.clone(), &mut state, signal(), &[0]), - ); + let mut save = Box::pin(fixture.desired.save_no_lock_armed( + fixture.pools.clone(), + &mut state, + PoolMetaPersistenceFence::Distributed(signal()), + &[0], + )); tokio::select! { _ = fixture.trace.wait_for(4, 3) => {}, _ = &mut save => panic!("commit crossed the last prepare") } assert_eq!(fixture.trace.commit_count(), 0); gate.add_permits(1); @@ -24849,7 +24938,7 @@ mod pools_tests { let mut state = fixture.state(); let err = fixture .desired - .save_no_lock_armed(fixture.pools.clone(), &mut state, signal(), &[0]) + .save_no_lock_armed(fixture.pools.clone(), &mut state, PoolMetaPersistenceFence::Distributed(signal()), &[0]) .await .expect_err("a missing conditional-write revision must fail"); assert!(err.to_string().contains("without a conditional-write revision")); @@ -24866,11 +24955,12 @@ mod pools_tests { config[3][0].gate = Some(gate.clone()); let fixture = Fixture::new(config); let mut state = fixture.state(); - let mut save = Box::pin( - fixture - .desired - .save_no_lock_armed(fixture.pools.clone(), &mut state, signal(), &[0]), - ); + let mut save = Box::pin(fixture.desired.save_no_lock_armed( + fixture.pools.clone(), + &mut state, + PoolMetaPersistenceFence::Distributed(signal()), + &[0], + )); tokio::select! { _ = fixture.trace.wait_for(4, 3) => {}, _ = &mut save => panic!("phase returned with a live replica") } assert_eq!(fixture.trace.commit_count(), 0); gate.add_permits(1); @@ -25045,11 +25135,12 @@ mod pools_tests { } let fixture = Fixture::new(config); let mut state = fixture.state(); - let mut save = Box::pin( - fixture - .desired - .save_no_lock_armed(fixture.pools.clone(), &mut state, signal(), &[0]), - ); + let mut save = Box::pin(fixture.desired.save_no_lock_armed( + fixture.pools.clone(), + &mut state, + PoolMetaPersistenceFence::Distributed(signal()), + &[0], + )); tokio::select! { _ = fixture.trace.wait_for(4 * (phase + 1), 4 * phase) => {}, _ = &mut save => panic!("blocked phase finished") } drop(save); assert!(state.ensure_write_safe("cancelled phase").is_err()); @@ -25060,7 +25151,7 @@ mod pools_tests { let mut state = fixture.state(); let outcome = fixture .desired - .save_no_lock_armed(fixture.pools.clone(), &mut state, signal(), &[0]) + .save_no_lock_armed(fixture.pools.clone(), &mut state, PoolMetaPersistenceFence::Distributed(signal()), &[0]) .await .expect("durable save before publication"); drop(outcome); @@ -25088,6 +25179,7 @@ mod pools_tests { .await .expect("lease request") .expect("lease acquired"); + let guard = Arc::new(guard); let gate = Arc::new(tokio::sync::Semaphore::new(0)); let mut config = steps(6); for step in &mut config { @@ -25096,7 +25188,7 @@ mod pools_tests { let fixture = Fixture::new(config); let state = fixture.state(); let mut arm = state.arm_transaction(); - let fence = PoolMetaPersistenceFence::Distributed(guard.lock_lost_signal()); + let fence = PoolMetaPersistenceFence::Owned(Arc::clone(&guard)); let mut pending = Box::pin(prepare(&fixture, &fence, &mut arm)); tokio::select! { _ = fixture.trace.wait_for(4, 0) => {}, _ = &mut pending => panic!("gate ignored") } tokio::time::timeout(StdDuration::from_secs(3), guard.lock_lost_notified()) @@ -25122,7 +25214,7 @@ mod pools_tests { let mut state = fixture.state(); let result = fixture .desired - .save_no_lock_armed(fixture.pools.clone(), &mut state, signal(), &[0]) + .save_no_lock_armed(fixture.pools.clone(), &mut state, PoolMetaPersistenceFence::Distributed(signal()), &[0]) .await; assert_eq!(fixture.trace.commit_count(), 4); if matches!(fault, WriteFault::TimeoutAfterWrite) { @@ -25172,7 +25264,7 @@ mod pools_tests { let mut save = Box::pin(fixture.desired.save_no_lock_armed( fixture.pools.clone(), &mut state, - if legacy || needs_repair { signal() } else { None }, + PoolMetaPersistenceFence::Distributed(if legacy || needs_repair { signal() } else { None }), &[0], )); tokio::select! { _ = fixture.trace.wait_for(1, 0) => {}, _ = &mut save => panic!("serial write gate ignored") } @@ -25732,7 +25824,7 @@ mod pools_tests { let save_task = tokio::spawn(async move { let mut save_guard = save_store.pool_meta_save_gate.lock().await; let outcome = snapshot - .save_no_lock_armed(vec![save_committed], &mut save_guard, None, &[0]) + .save_no_lock_armed(vec![save_committed], &mut save_guard, PoolMetaPersistenceFence::OfflineRecovery, &[0]) .await?; task_publish_started.notify_one(); task_publish_release.notified().await; @@ -25759,6 +25851,73 @@ mod pools_tests { .expect_err("cancellation after replica save but before publication must keep writes blocked"); } + #[tokio::test] + async fn pool_meta_cas_context_retains_actual_owner_for_exact_target() { + use crate::object_api::write_commit_context::WriteCommitContext; + + let lock = NamespaceLock::with_local_manager("pool-meta-cas-owner".to_string(), Arc::new(GlobalLockManager::new())); + let request = LockRequest::new( + ObjectKey::new(super::RUSTFS_META_BUCKET, POOL_META_NAME), + LockType::Exclusive, + "cas-writer", + ) + .with_acquire_timeout(StdDuration::from_secs(1)); + let owner = Arc::new( + lock.acquire_guard(&request) + .await + .expect("pool metadata lock acquisition should succeed") + .expect("pool metadata lock should be available"), + ); + let weak_owner = Arc::downgrade(&owner); + let fence = PoolMetaPersistenceFence::Owned(Arc::clone(&owner)); + let mut contexts = Vec::new(); + for object in [POOL_META_NAME, POOL_META_IDENTITY_NAME] { + let options = super::pool_meta_cas_options(object, &PoolMetaCasToken::Missing, &fence) + .expect("a held pool metadata lock should authorize its CAS target"); + let cloned_options = options.clone(); + let context = WriteCommitContext::from_options(&cloned_options, super::RUSTFS_META_BUCKET, object, false) + .expect("the actual CAS target should retain its borrowed namespace owner"); + assert!(context.has_borrowed_owner(), "CAS must borrow the real protocol lock owner"); + assert!( + WriteCommitContext::from_options(&cloned_options, super::RUSTFS_META_BUCKET, "unrelated-object", false).is_err(), + "a pool metadata CAS capability must not authorize an unrelated object" + ); + drop(options); + drop(cloned_options); + contexts.push(context); + } + drop(fence); + drop(owner); + assert!( + weak_owner.upgrade().is_some(), + "queued commit contexts must keep the actual lock guard alive" + ); + drop(contexts); + assert!(weak_owner.upgrade().is_none(), "the last commit context must release its lock owner"); + } + + #[tokio::test] + async fn pool_meta_cas_rejects_released_owner() { + let lock = NamespaceLock::with_local_manager("pool-meta-released-owner".to_string(), Arc::new(GlobalLockManager::new())); + let request = LockRequest::new( + ObjectKey::new(super::RUSTFS_META_BUCKET, POOL_META_NAME), + LockType::Exclusive, + "released-cas-writer", + ) + .with_acquire_timeout(StdDuration::from_secs(1)); + let mut owner = lock + .acquire_guard(&request) + .await + .expect("pool metadata lock acquisition should succeed") + .expect("pool metadata lock should be available"); + assert!(owner.release(), "the test must release a real namespace owner"); + let fence = PoolMetaPersistenceFence::Owned(Arc::new(owner)); + assert!( + super::pool_meta_cas_options(POOL_META_NAME, &PoolMetaCasToken::Missing, &fence).is_err(), + "a released namespace owner must not dispatch another CAS" + ); + } + #[tokio::test] async fn test_lost_pool_meta_fence_rejects_replica_write() { let client = Arc::new(LocalClient::with_manager(Arc::new(GlobalLockManager::new()))); @@ -25776,6 +25935,7 @@ mod pools_tests { .await .expect("pool metadata fence acquisition should not error") .expect("the stale writer should acquire the pool metadata fence"); + let guard = Arc::new(guard); tokio::time::timeout(StdDuration::from_secs(2), guard.lock_lost_notified()) .await .expect("the stale writer lease should expire"); @@ -25797,7 +25957,12 @@ mod pools_tests { }; let mut write_state = super::PoolMetaWriteState::default(); let err = snapshot - .save_no_lock_armed(vec![storage.clone()], &mut write_state, guard.lock_lost_signal(), &[0]) + .save_no_lock_armed( + vec![storage.clone()], + &mut write_state, + PoolMetaPersistenceFence::Owned(Arc::clone(&guard)), + &[0], + ) .await .expect_err("a writer must not persist after losing the distributed pool metadata fence"); @@ -25825,6 +25990,7 @@ mod pools_tests { .await .expect("pool metadata fence acquisition should not error") .expect("the publishing writer should acquire the pool metadata fence"); + let guard = Arc::new(guard); let storage = Arc::new(PartialPoolMetaWriteStorage { fail_write: false, fail_after_first_write: false, @@ -25845,7 +26011,7 @@ mod pools_tests { current.pools[0].last_update = OffsetDateTime::UNIX_EPOCH; let mut write_state = super::PoolMetaWriteState::for_test_bootstrap(); let outcome = saved - .save_no_lock_armed(vec![storage], &mut write_state, guard.lock_lost_signal(), &[0]) + .save_no_lock_armed(vec![storage], &mut write_state, PoolMetaPersistenceFence::Owned(Arc::clone(&guard)), &[0]) .await .expect("the replica save should finish while the fence is valid"); diff --git a/crates/ecstore/src/ecstore_validation_blackbox.rs b/crates/ecstore/src/ecstore_validation_blackbox.rs index 7806fe098..6fccc53b5 100644 --- a/crates/ecstore/src/ecstore_validation_blackbox.rs +++ b/crates/ecstore/src/ecstore_validation_blackbox.rs @@ -259,7 +259,7 @@ async fn blackbox_put_unknown_actual_size_restores_body_and_records_written_size .map(|idx| ((idx * 17) % 251) as u8) .collect::>(); let opts = ObjectOptions { - no_lock: true, + write_completion: crate::object_api::WriteCompletion::TailDrained, ..Default::default() }; @@ -300,7 +300,7 @@ async fn blackbox_get_restores_body_after_one_shard_file_is_removed() { .map(|idx| ((idx * 19) % 251) as u8) .collect::>(); let opts = ObjectOptions { - no_lock: true, + write_completion: crate::object_api::WriteCompletion::TailDrained, ..Default::default() }; @@ -406,7 +406,7 @@ async fn blackbox_heal_requests_preserve_repair_scope() { put_object, &mut put_reader, &ObjectOptions { - no_lock: true, + write_completion: crate::object_api::WriteCompletion::TailDrained, versioned: true, expected_bucket_incarnation_id: Some(put_incarnation), ..Default::default() @@ -469,7 +469,7 @@ async fn blackbox_heal_requests_preserve_repair_scope() { put_object, &mut healthy_reader, &ObjectOptions { - no_lock: true, + write_completion: crate::object_api::WriteCompletion::TailDrained, ..Default::default() }, ) @@ -509,7 +509,7 @@ async fn blackbox_heal_requests_preserve_repair_scope() { .map(|idx| ((idx * 29) % 251) as u8) .collect::>(); let opts = ObjectOptions { - no_lock: true, + write_completion: crate::object_api::WriteCompletion::TailDrained, ..Default::default() }; @@ -567,7 +567,7 @@ async fn blackbox_heal_requests_preserve_repair_scope() { let suspended_object = "suspended.bin"; let payload = vec![0x5a; 1 << 20]; let mpu_opts = ObjectOptions { - no_lock: true, + write_completion: crate::object_api::WriteCompletion::TailDrained, versioned: true, ..Default::default() }; @@ -675,7 +675,7 @@ async fn blackbox_heal_requests_preserve_repair_scope() { .expect("fourth disk should be online before suspended completion") }; let suspended_opts = ObjectOptions { - no_lock: true, + write_completion: crate::object_api::WriteCompletion::TailDrained, version_suspended: true, ..Default::default() }; @@ -712,7 +712,7 @@ async fn blackbox_range_read_restores_exact_slice_with_one_offline_disk() { let range_start = 513usize; let range_len = 8192usize; let opts = ObjectOptions { - no_lock: true, + write_completion: crate::object_api::WriteCompletion::TailDrained, ..Default::default() }; @@ -756,7 +756,7 @@ async fn blackbox_delete_marker_hides_object_body_without_erasing_prior_version_ let bucket = "bb-delete-marker-read-negative"; let object = "object.bin"; let opts = ObjectOptions { - no_lock: true, + write_completion: crate::object_api::WriteCompletion::TailDrained, version_suspended: true, object_lock_config_snapshot: Some(Arc::new(ObjectLockConfigSnapshot::new(ObjectLockConfigState::ConfirmedAbsent))), ..Default::default() @@ -853,7 +853,7 @@ async fn blackbox_issue3031_diag_covers_put_success_cleanup_and_error_summary() let first_payload = b"first diagnostic body".to_vec(); let second_payload = b"second diagnostic body that replaces the first".to_vec(); let opts = ObjectOptions { - no_lock: true, + write_completion: crate::object_api::WriteCompletion::TailDrained, ..Default::default() }; @@ -959,7 +959,7 @@ mod old_current_size_backfill { .await .expect("bucket should be created"); let opts = ObjectOptions { - no_lock: true, + write_completion: crate::object_api::WriteCompletion::TailDrained, ..Default::default() }; @@ -993,7 +993,7 @@ mod old_current_size_backfill { // latest's size, not the incoming version's. let versioned = "versioned.bin"; let first_version_opts = ObjectOptions { - no_lock: true, + write_completion: crate::object_api::WriteCompletion::TailDrained, versioned: true, version_id: Some(uuid::Uuid::new_v4().to_string()), ..Default::default() @@ -1005,7 +1005,7 @@ mod old_current_size_backfill { let expected = prelookup_expectation(&set_disks, bucket, versioned).await; assert_eq!(expected, OldCurrentSize::Present(111)); let second_version_opts = ObjectOptions { - no_lock: true, + write_completion: crate::object_api::WriteCompletion::TailDrained, versioned: true, version_id: Some(uuid::Uuid::new_v4().to_string()), ..Default::default() @@ -1037,7 +1037,7 @@ mod old_current_size_backfill { let expected = prelookup_expectation(&set_disks, bucket, versioned).await; assert_eq!(expected, OldCurrentSize::Present(0)); let third_version_opts = ObjectOptions { - no_lock: true, + write_completion: crate::object_api::WriteCompletion::TailDrained, versioned: true, version_id: Some(uuid::Uuid::new_v4().to_string()), ..Default::default() @@ -1061,7 +1061,7 @@ mod old_current_size_backfill { .await .expect("bucket should be created"); let opts = ObjectOptions { - no_lock: true, + write_completion: crate::object_api::WriteCompletion::TailDrained, ..Default::default() }; diff --git a/crates/ecstore/src/object_api/mod.rs b/crates/ecstore/src/object_api/mod.rs index 60f7e6776..008005547 100644 --- a/crates/ecstore/src/object_api/mod.rs +++ b/crates/ecstore/src/object_api/mod.rs @@ -104,6 +104,7 @@ mod hook_slot; mod object_mutation_hook; mod readers; mod types; +pub(crate) mod write_commit_context; #[cfg(test)] mod persisted_metadata_keys_tests; @@ -125,3 +126,4 @@ pub(crate) use object_mutation_hook::notify_object_mutation; pub use object_mutation_hook::{ObjectMutationHook, register_object_mutation_hook, unregister_object_mutation_hook}; pub use readers::*; pub use types::*; +pub use write_commit_context::{WriteCommitGuard, WriteLockContext}; diff --git a/crates/ecstore/src/object_api/types.rs b/crates/ecstore/src/object_api/types.rs index 0fda80ac5..7666ef2b7 100644 --- a/crates/ecstore/src/object_api/types.rs +++ b/crates/ecstore/src/object_api/types.rs @@ -935,6 +935,9 @@ pub struct ObjectOptions { /// Persisted bucket incarnation observed before authorization. pub expected_bucket_incarnation_id: Option, pub no_lock: bool, + /// A genuine target write guard; loss fences alone do not authorize a borrowed write. + #[doc(hidden)] + pub write_lock_context: Option, /// Internal read-only inspection must not enqueue metadata or payload repairs. pub suppress_read_repair: bool, /// Control-plane writers that immediately read or CAS the same namespace diff --git a/crates/ecstore/src/object_api/write_commit_context.rs b/crates/ecstore/src/object_api/write_commit_context.rs new file mode 100644 index 000000000..571664265 --- /dev/null +++ b/crates/ecstore/src/object_api/write_commit_context.rs @@ -0,0 +1,186 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use super::{ObjectOptions, WriteCompletion}; +use crate::error::{Error, Result}; +use std::sync::Arc; +use std::time::Duration; + +/// A real write guard shared with an inner commit, never a lock-loss signal alone. +/// +/// Callers cannot fabricate a guard without acquiring its namespace lock. +/// +/// ```compile_fail +/// use rustfs_ecstore::api::object::WriteCommitGuard; +/// use std::sync::Arc; +/// +/// let guard = WriteCommitGuard { +/// bucket: "bucket".into(), +/// object: "object".into(), +/// guards: Arc::new(Vec::new()), +/// }; +/// ``` +#[doc(hidden)] +#[derive(Clone, Debug)] +pub struct WriteCommitGuard { + bucket: Arc, + object: Arc, + guards: Arc>>, +} + +impl WriteCommitGuard { + /// Acquire the key recorded by the namespace wrapper and retain its owner. + pub async fn acquire( + lock: &rustfs_lock::NamespaceLockWrapper, + timeout: Duration, + ) -> std::result::Result { + let guard = Arc::new(lock.get_write_lock(timeout).await?); + let key = lock.resource(); + Ok(Self::from_guard(guard, &key.bucket, &key.object)) + } + + pub(crate) fn from_guard(guard: Arc, bucket: &str, object: &str) -> Self { + Self { + bucket: bucket.into(), + object: object.into(), + guards: Arc::new(vec![guard]), + } + } + + pub fn is_lock_lost(&self) -> bool { + self.guards.iter().any(|guard| guard.is_lock_lost() || guard.is_released()) + } + + fn matches(&self, bucket: &str, object: &str) -> bool { + self.bucket.as_ref() == bucket && self.object.as_ref() == object + } +} + +/// Opaque boundary capability; only real acquisitions or offline startup construct it. +/// +/// ```compile_fail +/// use rustfs_ecstore::object_api::WriteLockContext; +/// +/// let context = WriteLockContext {}; +/// ``` +#[doc(hidden)] +#[derive(Clone, Debug)] +pub struct WriteLockContext { + mode: WriteLockMode, +} + +#[derive(Clone, Debug)] +enum WriteLockMode { + Borrowed(Vec), + OfflineRecovery, +} + +pub(crate) enum WriteCommitContext { + Owned, + Borrowed(Vec), + Publication, + OfflineRecovery, + #[cfg(test)] + UnlockedFixture, +} + +impl WriteCommitContext { + pub(crate) fn from_options(opts: &ObjectOptions, bucket: &str, object: &str, publication: bool) -> Result { + if publication { + if let Some(WriteLockContext { + mode: WriteLockMode::Borrowed(guards), + }) = opts.write_lock_context.as_ref() + && !guards.iter().any(|guard| guard.matches(bucket, object)) + { + return Err(Error::InvalidArgument( + bucket.to_owned(), + object.to_owned(), + "publication carries a different borrowed namespace".to_owned(), + )); + } + return Ok(Self::Publication); + } + match (opts.no_lock, opts.write_lock_context.as_ref().map(|context| &context.mode)) { + (false, None | Some(WriteLockMode::Borrowed(_))) => Ok(Self::Owned), + (true, Some(WriteLockMode::Borrowed(guards))) if guards.iter().any(|guard| guard.matches(bucket, object)) => { + Ok(Self::Borrowed(guards.clone())) + } + (true, Some(WriteLockMode::OfflineRecovery)) => Ok(Self::OfflineRecovery), + #[cfg(test)] + (true, None) if !opts.metadata_cache_safe => Ok(Self::UnlockedFixture), + _ => Err(Error::InvalidArgument( + bucket.to_owned(), + object.to_owned(), + "write lock context does not prove the target namespace".to_owned(), + )), + } + } + + pub(crate) fn acquires_namespace(&self) -> bool { + matches!(self, Self::Owned) + } + + pub(crate) fn has_borrowed_owner(&self) -> bool { + matches!(self, Self::Borrowed(_)) + } + + pub(crate) fn is_lock_lost(&self) -> bool { + matches!(self, Self::Borrowed(guards) if guards.iter().any(WriteCommitGuard::is_lock_lost)) + } + + pub(crate) fn allows_early_ack(&self, completion: WriteCompletion) -> bool { + completion == WriteCompletion::Quorum && matches!(self, Self::Owned | Self::Borrowed(_) | Self::Publication) + } +} + +impl ObjectOptions { + pub fn add_write_commit_guard(&mut self, guard: &WriteCommitGuard) { + for owner in guard.guards.iter() { + self.add_namespace_lock_guard(owner); + } + let mut guards = match self.write_lock_context.as_ref() { + Some(WriteLockContext { + mode: WriteLockMode::Borrowed(existing), + }) => existing.clone(), + _ => Vec::new(), + }; + if let Some(existing) = guards.iter_mut().find(|held| held.matches(&guard.bucket, &guard.object)) { + let owners = Arc::make_mut(&mut existing.guards); + for owner in guard.guards.iter() { + if !owners.iter().any(|held| Arc::ptr_eq(held, owner)) { + owners.push(Arc::clone(owner)); + } + } + } else { + guards.push(guard.clone()); + } + self.no_lock = true; + self.write_lock_context = Some(WriteLockContext { + mode: WriteLockMode::Borrowed(guards), + }); + } + + pub(crate) fn add_owned_write_lock(&mut self, guard: Arc, bucket: &str, object: &str) { + self.add_write_commit_guard(&WriteCommitGuard::from_guard(guard, bucket, object)); + } + + /// Only startup migration may publish without a running namespace owner. + pub(crate) fn use_offline_recovery_write(&mut self) { + self.no_lock = true; + self.write_lock_context = Some(WriteLockContext { + mode: WriteLockMode::OfflineRecovery, + }); + self.write_completion = WriteCompletion::TailDrained; + } +} diff --git a/crates/ecstore/src/services/rebalance/control.rs b/crates/ecstore/src/services/rebalance/control.rs index a0b2a2cb9..a8fef5fd8 100644 --- a/crates/ecstore/src/services/rebalance/control.rs +++ b/crates/ecstore/src/services/rebalance/control.rs @@ -521,11 +521,12 @@ impl ECStore { .map_err(|err| rebalance_meta_lock_error(err, "write")) }) .await?; + let guard = Arc::new(guard); let mut opts = ObjectOptions { - no_lock: true, + write_completion: crate::object_api::WriteCompletion::TailDrained, ..Default::default() }; - opts.add_namespace_lock_guard(&guard); + opts.add_owned_write_lock(Arc::clone(&guard), crate::disk::RUSTFS_META_BUCKET, REBAL_META_NAME); merge_and_save_rebalance_meta_no_lock(pool, local_snapshot, stage, opts, None, expected_id).await?; if guard.is_lock_lost() { diff --git a/crates/ecstore/src/set_disk/mod.rs b/crates/ecstore/src/set_disk/mod.rs index e12e42feb..ba79bb254 100644 --- a/crates/ecstore/src/set_disk/mod.rs +++ b/crates/ecstore/src/set_disk/mod.rs @@ -868,7 +868,7 @@ static OBJECT_LOCK_DIAG_ENABLED: OnceLock = OnceLock::new(); mod core; #[cfg(test)] pub(crate) use core::io_primitives::disk_call_counters; -#[cfg(all(test, feature = "test-util"))] +#[cfg(test)] pub(crate) use core::io_primitives::{ENV_RUSTFS_PUT_RENAME_EARLY_ACK_ENABLE, rename_fanout_barrier}; mod ctx; mod metadata; @@ -3964,7 +3964,6 @@ impl SetDisks { b"rustfs-marker-purge-receipt-v1".to_vec(), &ObjectOptions { max_parity: true, - no_lock: true, ..Default::default() }, ) diff --git a/crates/ecstore/src/set_disk/ops/multipart.rs b/crates/ecstore/src/set_disk/ops/multipart.rs index b84458619..007970aa1 100644 --- a/crates/ecstore/src/set_disk/ops/multipart.rs +++ b/crates/ecstore/src/set_disk/ops/multipart.rs @@ -21,6 +21,7 @@ //! unchanged; method bodies are moved verbatim and runtime behavior is the same. use crate::core::pools::DecommissionCapacityAdmission; +use crate::object_api::write_commit_context::WriteCommitContext; #[cfg(test)] use super::super::GetObjectMetadataCacheKey; @@ -2446,17 +2447,19 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for SetDisks { opts: &ObjectOptions, ) -> Result { crate::hp_guard!("SetDisks::complete_multipart_upload"); + let write_context = WriteCommitContext::from_options(opts, bucket, object, false)?; self.invalidate_get_object_metadata_cache(bucket, object).await; let upload_id_path = Self::get_multipart_upload_dir(bucket, object, upload_id, opts.data_movement); - let range_seek_rollout_enabled = crate::object_api::legacy_encrypted_range_seek_enabled() && !opts.no_lock; + let range_seek_rollout_enabled = + crate::object_api::legacy_encrypted_range_seek_enabled() && write_context.acquires_namespace(); let mut object_lock_guard = None; let mut decommission_object_lock_guard = None; let mut decommission_target_lock_covered = false; let mut decommission_capacity_guard = None; if opts.http_preconditions.is_some() { - if !opts.no_lock { + if write_context.acquires_namespace() { object_lock_guard = Some( self.acquire_write_lock_diag("complete_multipart_upload_precondition", bucket, object) .await?, @@ -2479,7 +2482,7 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for SetDisks { self.pool_index, bucket, object, - opts.no_lock || object_lock_guard.is_some(), + !write_context.acquires_namespace() || object_lock_guard.is_some(), DecommissionCapacityAdmission::ExistingMultipart, ) .await?; @@ -2487,7 +2490,7 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for SetDisks { decommission_target_lock_covered = target_lock_covered; decommission_capacity_guard = capacity_guard; } - if !opts.no_lock && object_lock_guard.is_none() && !decommission_target_lock_covered { + if write_context.acquires_namespace() && object_lock_guard.is_none() && !decommission_target_lock_covered { object_lock_guard = Some( self.acquire_write_lock_diag("complete_multipart_upload_commit", bucket, object) .await?, @@ -3354,8 +3357,10 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for SetDisks { // usable as a CopyObject source. Do not return on rename quorum while // a tail owner may still hold the object guard and finish shard moves. let commit_allows_early_ack = false; - let detach_commit_owner = commit_allows_early_ack || upload_guard.is_some() || quota_mutation_fence; + let detach_commit_owner = + write_context.has_borrowed_owner() || commit_allows_early_ack || upload_guard.is_some() || quota_mutation_fence; let commit = async move { + let write_context = write_context; let mut _object_lock_guard = commit_object_lock_guard; let mut _decommission_object_lock_guard = commit_decommission_object_lock_guard; let mut _upload_guard = upload_guard; @@ -3377,6 +3382,7 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for SetDisks { pause_multipart_commit(&commit_bucket, &commit_object, MultipartCommitPause::BeforeQuotaRename).await; if quota_reservation.is_lock_lost() || !quota_reservation.capability_proof_matches() + || write_context.is_lock_lost() || _object_lock_guard.as_ref().is_some_and(|guard| guard.is_lock_lost()) || _decommission_object_lock_guard .as_ref() @@ -3431,6 +3437,7 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for SetDisks { } if quota_reservation.is_lock_lost() || !quota_reservation.capability_proof_matches() + || write_context.is_lock_lost() || _object_lock_guard.as_ref().is_some_and(|guard| guard.is_lock_lost()) || _decommission_object_lock_guard .as_ref() @@ -3761,6 +3768,7 @@ mod tests { use crate::layout::endpoints::SetupType; use crate::services::notification_sys::install_remote_version_state_fleet_proof_for_test; use crate::set_disk::core::io_primitives::{ENV_RUSTFS_PUT_RENAME_EARLY_ACK_ENABLE, rename_fanout_barrier}; + use crate::set_disk::get_lock_acquire_timeout; // No-locker helpers resolve to the isolated-context variants (see // `hermetic_set_disks_isolated`); the guard-based tests build through // `hermetic_set_disks_with_lockers`, which stays on the bootstrap context @@ -7122,6 +7130,54 @@ mod tests { .await; } + #[tokio::test(flavor = "multi_thread")] + #[serial] + async fn borrowed_complete_keeps_namespace_owner_after_caller_cancel() { + let (_dirs, disks, set) = hermetic_set_disks(4).await; + let bucket = "multipart-borrowed-cancel"; + let object = "object"; + make_bucket_on_all(&disks, bucket).await; + let (upload_id, parts) = + stage_upload_with_create_opts(&set, bucket, object, &[0x53; 4096], &ObjectOptions::default()).await; + let barrier = MultipartCommitBarrier::install(bucket, object, MultipartCommitPause::AfterRename); + let writer = Arc::clone(&set); + let complete = tokio::spawn(async move { + let outer_guard = crate::object_api::WriteCommitGuard::acquire( + &writer.new_ns_lock(bucket, object).await.expect("namespace wrapper"), + get_lock_acquire_timeout(), + ) + .await + .expect("outer MPU namespace guard"); + let mut opts = ObjectOptions::default(); + opts.add_write_commit_guard(&outer_guard); + writer + .complete_multipart_upload(bucket, object, &upload_id, parts, &opts) + .await + }); + tokio::time::timeout(Duration::from_secs(30), barrier.wait_until_paused()) + .await + .expect("borrowed completion reaches committed cleanup"); + complete.abort(); + assert!(complete.await.expect_err("completion caller canceled").is_cancelled()); + let mut probe = Box::pin(set.acquire_write_lock_diag("borrowed_complete_probe", bucket, object)); + assert!( + futures::poll!(probe.as_mut()).is_pending(), + "borrowed completion retains the real outer namespace owner" + ); + barrier.release(); + drop( + tokio::time::timeout(Duration::from_secs(30), probe) + .await + .expect("completion owner finishes cleanup") + .expect("next writer obtains namespace"), + ); + let info = set + .get_object_info(bucket, object, &ObjectOptions::default()) + .await + .expect("borrowed MPU result remains readable"); + assert_eq!(info.size, 4096); + } + #[tokio::test(flavor = "multi_thread")] #[serial] async fn cancelled_complete_keeps_upload_lock_through_tail_cleanup() { diff --git a/crates/ecstore/src/set_disk/ops/object.rs b/crates/ecstore/src/set_disk/ops/object.rs index 81ab6cb48..64987f12d 100644 --- a/crates/ecstore/src/set_disk/ops/object.rs +++ b/crates/ecstore/src/set_disk/ops/object.rs @@ -281,7 +281,7 @@ use super::super::GetObjectMetadataCacheEntry; use super::super::GetObjectMetadataCacheKey; #[cfg(test)] use super::super::capacity_scope_from_disks; -#[cfg(all(test, feature = "test-util"))] +#[cfg(test)] use super::super::get_lock_acquire_timeout; use crate::bucket::lifecycle::{ tier_delete_journal::{TierDeleteDispatchAuthorization, record_tier_delete_journal_backend_identity}, @@ -320,10 +320,13 @@ use crate::disk::format::FormatV3; use crate::disk::new_disk; use crate::disk::{DataDirDeleteStatus, OldCurrentSize}; use crate::error::is_err_invalid_upload_id; +#[cfg(test)] +use crate::object_api::WriteCompletion; +use crate::object_api::write_commit_context::WriteCommitContext; use crate::object_api::{GetObjectBodySource, get_object_body_cache_hook_suppressed}; use crate::object_api::{ NamespaceLockFence, ReplicationStatusWritebackCondition, ReplicationStatusWritebackMode, - SCANNER_PUBLICATION_LEASE_FENCE_METADATA_KEY, WriteCompletion, + SCANNER_PUBLICATION_LEASE_FENCE_METADATA_KEY, }; use crate::services::notification_sys::RemoteVersionStateFleetProofToken; use crate::services::tier::tier::{TierConfigMgr, TierDestinationId, TierOperationLease, tier_destination_id_from_metadata}; @@ -3499,6 +3502,7 @@ impl SetDisks { opts: &ObjectOptions, mut publication_fence: Option, ) -> Result<(ObjectInfo, Option)> { + let write_context = WriteCommitContext::from_options(opts, bucket, object, publication_fence.is_some())?; let protect_write = opts.shard_integrity_write_enabled(); let source_bucket_incarnation_id = match opts.expected_bucket_incarnation_id { Some(incarnation_id) => Some(incarnation_id), @@ -4026,7 +4030,7 @@ impl SetDisks { self.pool_index, bucket, object, - opts.no_lock || object_lock_guard.is_some(), + !write_context.acquires_namespace() || object_lock_guard.is_some(), DecommissionCapacityAdmission::Mutation, ) .await?; @@ -4034,7 +4038,8 @@ impl SetDisks { decommission_target_lock_covered = target_lock_covered; decommission_capacity_guard = capacity_guard; } - if publication_fence.is_some() || (!opts.no_lock && object_lock_guard.is_none() && !decommission_target_lock_covered) + if publication_fence.is_some() + || (write_context.acquires_namespace() && object_lock_guard.is_none() && !decommission_target_lock_covered) { #[cfg(any(test, feature = "test-util"))] pause_put_object_commit(bucket, object, PutObjectCommitPause::BeforeNamespace).await; @@ -4382,10 +4387,11 @@ impl SetDisks { // complete rename fan-out drains. Keep this path synchronous so // its terminal state is known before the coordinator releases // remote leases. - let commit_owns_namespace_guard = commit_object_lock_guard.is_some() + let commit_owns_namespace_guard = write_context.has_borrowed_owner() + || commit_object_lock_guard.is_some() || commit_decommission_object_lock_guard.is_some() || commit_publication_guard.is_some(); - let commit_allows_early_ack = opts.write_completion == WriteCompletion::Quorum + let commit_allows_early_ack = write_context.allows_early_ack(opts.write_completion) && !(opts.data_movement && opts.has_decommission_capacity_reservation()) && commit_owns_namespace_guard && commit_scanner_publication_scope.is_none(); @@ -4412,6 +4418,7 @@ impl SetDisks { tmp_cleanup_owned = true; let commit = move |cancellation: Option| async move { + let write_context = write_context; let mut _object_lock_guard = commit_object_lock_guard; let mut _decommission_object_lock_guard = commit_decommission_object_lock_guard; let mut _publication_guard = commit_publication_guard; @@ -4427,6 +4434,7 @@ impl SetDisks { pause_put_object_commit(&commit_bucket, &commit_object, PutObjectCommitPause::BeforeQuotaRename).await; if quota_reservation.is_lock_lost() || !quota_reservation.capability_proof_matches() + || write_context.is_lock_lost() || _object_lock_guard.as_ref().is_some_and(|guard| guard.is_lock_lost()) || _decommission_object_lock_guard .as_ref() @@ -4486,6 +4494,7 @@ impl SetDisks { } if quota_reservation.is_lock_lost() || !quota_reservation.capability_proof_matches() + || write_context.is_lock_lost() || _object_lock_guard.as_ref().is_some_and(|guard| guard.is_lock_lost()) || _decommission_object_lock_guard .as_ref() @@ -4671,6 +4680,7 @@ impl SetDisks { guard_release_rx, ( object_lock_guard, + write_context, publication_guard, bucket_lifecycle_guard, decommission_object_lock_guard, @@ -4691,6 +4701,7 @@ impl SetDisks { }, move |( object_lock_guard, + write_context, publication_guard, bucket_lifecycle_guard, decommission_object_lock_guard, @@ -4698,6 +4709,7 @@ impl SetDisks { ), targets| async move { drop(object_lock_guard); + drop(write_context); drop(publication_guard); drop(bucket_lifecycle_guard); cleanup_set @@ -19817,6 +19829,16 @@ mod put_object_tmp_cleanup_tests { .await .expect("committed object must remain readable despite the failed tail"); assert_eq!(info.size, TEST_OBJECT_SIZE as i64); + let mut read = set + .get_object_reader(bucket, object, None, HeaderMap::new(), &ObjectOptions::default()) + .await + .expect("committed body must remain readable despite the failed tail"); + let mut body = Vec::new(); + read.stream + .read_to_end(&mut body) + .await + .expect("committed body must stream completely after the failed tail"); + assert_eq!(body, vec![b'1'; TEST_OBJECT_SIZE]); } #[tokio::test] @@ -20002,6 +20024,93 @@ mod put_object_tmp_cleanup_tests { wait_for_tmp_workspace_to_drain(&dirs, "cancelled full-tail commit should release staging ownership").await; } + #[tokio::test] + #[serial_test::serial(capacity_dirty_scope)] + async fn borrowed_write_context_rejects_wrong_namespace_and_cache_flag() { + let (_dirs, _disks, set) = hermetic_set_disks(4).await; + let bucket = "borrowed-write-target"; + let object = "owned-object"; + let guard = crate::object_api::WriteCommitGuard::acquire( + &set.new_ns_lock(bucket, object).await.expect("namespace wrapper"), + get_lock_acquire_timeout(), + ) + .await + .expect("outer write lock"); + let mut opts = ObjectOptions::default(); + opts.add_write_commit_guard(&guard); + assert!(matches!( + WriteCommitContext::from_options(&opts, bucket, "other-object", false), + Err(Error::InvalidArgument(..)) + )); + assert!(matches!( + WriteCommitContext::from_options(&opts, bucket, "other-object", true), + Err(Error::InvalidArgument(..)) + )); + let fake = ObjectOptions { + no_lock: true, + metadata_cache_safe: true, + ..Default::default() + }; + assert!(matches!( + WriteCommitContext::from_options(&fake, bucket, object, false), + Err(Error::InvalidArgument(..)) + )); + } + + #[tokio::test] + #[serial_test::serial(capacity_dirty_scope)] + async fn borrowed_tail_drained_put_keeps_owner_after_caller_cancel() { + let (_dirs, disks, set) = hermetic_set_disks(4).await; + let bucket = "borrowed-put-cancel"; + let object = "borrowed-put-cancel-object"; + for disk in &disks { + disk.make_volume(bucket).await.expect("bucket volume"); + } + let barrier = rename_fanout_barrier::arm(object, 0, rename_fanout_barrier::PHASE_RENAME); + let tasks = rename_fanout_barrier::observe_tasks(object); + let writer = Arc::clone(&set); + let put = tokio::spawn(async move { + let guard = crate::object_api::WriteCommitGuard::acquire( + &writer.new_ns_lock(bucket, object).await.expect("namespace wrapper"), + get_lock_acquire_timeout(), + ) + .await + .expect("outer write guard"); + let mut opts = ObjectOptions { + write_completion: WriteCompletion::TailDrained, + ..Default::default() + }; + opts.add_write_commit_guard(&guard); + let mut reader = PutObjReader::from_vec(vec![b'b'; TEST_OBJECT_SIZE]); + writer.put_object(bucket, object, &mut reader, &opts).await + }); + tokio::time::timeout(Duration::from_secs(30), barrier.wait_until_paused()) + .await + .expect("borrowed commit reaches rename phase"); + wait_for_paused_tail_metadata_quorum(&disks, bucket, object).await; + assert!(!put.is_finished(), "full-tail borrowed write remains pending"); + put.abort(); + assert!(put.await.expect_err("caller was canceled").is_cancelled()); + let mut probe = Box::pin(set.acquire_write_lock_diag("borrowed_cancel_probe", bucket, object)); + assert!( + futures::poll!(probe.as_mut()).is_pending(), + "commit must retain the real borrowed namespace owner" + ); + barrier.release(); + drop( + tokio::time::timeout(Duration::from_secs(30), probe) + .await + .expect("borrowed owner drains") + .expect("next writer acquires"), + ); + assert_eq!(tasks.running(), 0, "all borrowed rename tasks are reaped before releasing the owner"); + let info = set + .get_object_info(bucket, object, &ObjectOptions::default()) + .await + .expect("committed borrowed write remains readable"); + assert_eq!(info.size, TEST_OBJECT_SIZE as i64); + } + #[tokio::test] #[serial_test::serial(capacity_dirty_scope)] async fn no_lock_put_waits_for_rename_tail_under_outer_guard() { @@ -20013,27 +20122,23 @@ mod put_object_tmp_cleanup_tests { disk.make_volume(bucket).await.expect("bucket volume should be created"); } - let outer_guard = set_disks - .acquire_write_lock_diag("outer_no_lock_put", bucket, object) - .await - .expect("the outer caller should hold the namespace guard"); + let outer_guard = crate::object_api::WriteCommitGuard::acquire( + &set_disks.new_ns_lock(bucket, object).await.expect("outer namespace wrapper"), + get_lock_acquire_timeout(), + ) + .await + .expect("the outer caller should hold the namespace guard"); let rename_tasks = rename_fanout_barrier::observe_tasks(object); let rename_barrier = rename_fanout_barrier::arm(object, 0, rename_fanout_barrier::PHASE_RENAME); + let mut opts = ObjectOptions { + write_completion: WriteCompletion::TailDrained, + ..Default::default() + }; + opts.add_write_commit_guard(&outer_guard); let put_store = Arc::clone(&set_disks); let put = tokio::spawn(async move { let mut reader = PutObjReader::from_vec(vec![b'1'; TEST_OBJECT_SIZE]); - put_store - .put_object( - bucket, - object, - &mut reader, - &ObjectOptions { - no_lock: true, - write_completion: WriteCompletion::TailDrained, - ..Default::default() - }, - ) - .await + put_store.put_object(bucket, object, &mut reader, &opts).await }); tokio::time::timeout(Duration::from_secs(30), rename_barrier.wait_until_paused()) @@ -22427,6 +22532,119 @@ mod body_cache_hook_e2e_tests { } } +#[cfg(test)] +mod marker_purge_receipt_tests { + use super::hermetic_set_disks_support::hermetic_set_disks_isolated; + use super::*; + use crate::disk::ReadOptions; + + #[tokio::test] + #[serial_test::serial(capacity_dirty_scope)] + async fn quorum_marker_purge_receipt_survives_reopen_with_stale_member() { + let (_dirs, disks, set) = hermetic_set_disks_isolated(4).await; + let bucket = "marker-purge-receipt"; + let object = "explicit-marker-version"; + let version = Uuid::new_v4(); + let incarnation = Uuid::new_v4(); + for disk in &disks { + disk.make_volume(bucket).await.expect("fixture bucket"); + } + let mut marker = FileInfo { + name: object.to_string(), + version_id: Some(version), + deleted: true, + mark_deleted: true, + mod_time: Some(OffsetDateTime::now_utc()), + ..Default::default() + }; + marker.set_delete_marker_incarnation(incarnation); + let owner = set + .acquire_write_lock_diag("marker_purge_receipt_test", bucket, object) + .await + .expect("own the deleted object namespace"); + set.delete_object_version_with_purge(bucket, object, &marker, true, None, Some(incarnation)) + .await + .expect("seed the real marker on every disk"); + let marker = disks[0] + .read_version("", bucket, object, &version.to_string(), &ReadOptions::default()) + .await + .expect("read the persisted marker identity"); + let purge = delete_marker_purge_candidate(&marker, Some(incarnation)) + .expect("the canonical marker must authorize its exact purge"); + let request = FileInfo { + name: object.to_string(), + version_id: Some(version), + ..Default::default() + }; + + // A member that misses the explicit DELETE retains its original + // marker. The other three disks still satisfy configured write quorum. + set.disks.write().await[0] = None; + tokio::time::timeout( + Duration::from_secs(10), + set.delete_object_version_with_purge(bucket, object, &request, false, Some(purge.clone()), Some(incarnation)), + ) + .await + .expect("the receipt must not reacquire the deleted object's owner") + .expect("the explicit marker purge must reach write quorum"); + drop(owner); + for disk in &disks[1..] { + assert!( + matches!( + disk.read_version("", bucket, object, &version.to_string(), &ReadOptions::default()) + .await, + Err(DiskError::FileNotFound | DiskError::FileVersionNotFound) + ), + "acknowledging members must remove the exact marker" + ); + } + let stale = disks[0] + .read_version("", bucket, object, &version.to_string(), &ReadOptions::default()) + .await + .expect("the offline member must retain its marker"); + assert!(stale.deleted); + assert_eq!(stale.version_id, Some(version)); + + let ctx = Arc::new(InstanceContext::new()); + ctx.update_erasure_type(crate::layout::endpoints::SetupType::Erasure).await; + let reopened = SetDisks::new_with_instance_ctx( + set.locker_owner.clone(), + Arc::new(tokio::sync::RwLock::new(disks.iter().cloned().map(Some).collect())), + set.set_drive_count, + set.default_parity_count, + set.set_index, + set.pool_index, + set.set_endpoints.clone(), + set.format.clone(), + Vec::new(), + ctx, + ) + .await; + assert!( + reopened.has_marker_purge_receipt(bucket, object, version, &purge).await, + "a successful explicit DELETE must leave durable proof for stale-marker cleanup after reopen" + ); + let receipt = crate::set_disk::marker_purge_receipt_path(bucket, object, version, &purge); + assert_eq!( + crate::config::com::read_config_limited_preserve_empty(reopened.clone(), &receipt, 128) + .await + .expect("read the physical quorum receipt"), + b"rustfs-marker-purge-receipt-v1" + ); + assert!( + !reopened + .has_marker_purge_receipt(bucket, object, Uuid::new_v4(), &purge) + .await, + "the receipt must not authorize another marker version" + ); + reopened.consume_marker_purge_receipt(bucket, object, version, &purge).await; + assert!( + !reopened.has_marker_purge_receipt(bucket, object, version, &purge).await, + "cleanup must be able to consume the persisted receipt" + ); + } +} + #[cfg(test)] mod single_delete_namespace_owner_tests { use super::hermetic_set_disks_support::hermetic_set_disks_isolated; diff --git a/crates/ecstore/src/store/object.rs b/crates/ecstore/src/store/object.rs index 52fdad5ce..444d00ab8 100644 --- a/crates/ecstore/src/store/object.rs +++ b/crates/ecstore/src/store/object.rs @@ -1038,8 +1038,23 @@ impl fmt::Display for ObjectLockDiagMode { } } +enum ObjectLockOwner { + Read(rustfs_lock::NamespaceLockGuard), + Write(Arc), +} + +impl ObjectLockOwner { + fn as_ref(&self) -> &rustfs_lock::NamespaceLockGuard { + match self { + Self::Read(guard) => guard, + Self::Write(guard) => guard, + } + } +} + pub(crate) struct ObjectLockDiagGuard { - guard: rustfs_lock::NamespaceLockGuard, + guard: ObjectLockOwner, + write_key: Option<(String, String)>, #[cfg(test)] test_namespace_lock_fence: Option, enabled: bool, @@ -1063,7 +1078,11 @@ impl ObjectLockDiagGuard { mode: ObjectLockDiagMode, ) -> Self { Self { - guard, + guard: match mode { + ObjectLockDiagMode::Read => ObjectLockOwner::Read(guard), + ObjectLockDiagMode::Write => ObjectLockOwner::Write(Arc::new(guard)), + }, + write_key: None, #[cfg(test)] test_namespace_lock_fence: None, enabled, @@ -1082,15 +1101,20 @@ impl ObjectLockDiagGuard { self } + fn with_write_key(mut self, bucket: &str, object: &str) -> Self { + self.write_key = Some((bucket.to_owned(), object.to_owned())); + self + } + pub(crate) fn lock_lost_signal(&self) -> Option> { - match &self.guard { + match self.guard.as_ref() { rustfs_lock::NamespaceLockGuard::Standard(guard) => Some(guard.lock_lost()), rustfs_lock::NamespaceLockGuard::Fast(_) => None, } } pub(crate) fn is_lock_lost(&self) -> bool { - self.guard.is_lock_lost() || { + self.guard.as_ref().is_lock_lost() || { #[cfg(test)] { self.test_namespace_lock_fence @@ -1105,6 +1129,13 @@ impl ObjectLockDiagGuard { } pub(crate) fn add_namespace_lock_fence(&self, opts: &mut ObjectOptions) { + if let Some((bucket, object)) = self.write_key.as_ref() + && let ObjectLockOwner::Write(guard) = &self.guard + { + let no_lock = opts.no_lock; + opts.add_owned_write_lock(Arc::clone(guard), bucket, object); + opts.no_lock = no_lock; + } opts.ensure_namespace_lock_fence(); if let Some(signal) = self.lock_lost_signal() { opts.add_namespace_lock_lost_signal(signal); @@ -1301,7 +1332,7 @@ impl RestoreAcceptGuard { /// refresh lost quorum). Callers must check this before committing the /// restore-status write the guard exists to serialize. pub fn is_lock_lost(&self) -> bool { - self.0.guard.is_lock_lost() + self.0.guard.as_ref().is_lock_lost() } pub fn add_namespace_lock_fence(&self, opts: &mut ObjectOptions) { @@ -1322,7 +1353,7 @@ impl RestoreWorkerGuard { impl Drop for ObjectLockDiagGuard { fn drop(&mut self) { - if !self.enabled || self.guard.is_released() { + if !self.enabled || self.guard.as_ref().is_released() { return; } @@ -3029,7 +3060,8 @@ impl ECStore { owner, ObjectLockDiagMode::Write, ) - .with_attempt(attempt)) + .with_attempt(attempt) + .with_write_key(bucket, object)) } async fn acquire_object_write_lock_if_needed( @@ -3044,11 +3076,9 @@ impl ECStore { } let guard = self.acquire_object_write_lock(op, bucket, object).await?; - if let Some(signal) = guard.lock_lost_signal() { - opts.add_namespace_lock_lost_signal(signal); - } - opts.ensure_namespace_lock_fence(); + guard.add_namespace_lock_fence(opts); opts.no_lock = true; + opts.write_completion = crate::object_api::WriteCompletion::TailDrained; Ok(Some(guard)) } @@ -3187,7 +3217,10 @@ impl ECStore { let store = Arc::clone(self); let write = async move { let object = "buckets/.scanner-pause-backlog.json"; - let mut opts = ObjectOptions::default(); + let mut opts = ObjectOptions { + write_completion: crate::object_api::WriteCompletion::TailDrained, + ..Default::default() + }; // Match migration: fixed object namespace -> durable pool metadata -> // actual replica namespace. The replica need not be the hash-routed set. let object_guard = if store.single_pool() { @@ -3727,15 +3760,18 @@ impl ECStore { acquire_start.elapsed(), diag_enabled, ); - guards.push(ObjectLockDiagGuard::new( - guard, - diag_enabled, - op, - diag_enabled.then(|| bucket.to_string()), - diag_enabled.then(|| object.to_string()), - owner, - ObjectLockDiagMode::Write, - )); + guards.push( + ObjectLockDiagGuard::new( + guard, + diag_enabled, + op, + diag_enabled.then(|| bucket.to_string()), + diag_enabled.then(|| object.to_string()), + owner, + ObjectLockDiagMode::Write, + ) + .with_write_key(bucket, object), + ); locked_sets.push(Arc::clone(set)); } } @@ -3826,7 +3862,8 @@ impl ECStore { diag_enabled.then(|| object.to_string()), owner, ObjectLockDiagMode::Write, - ); + ) + .with_write_key(bucket, object); #[cfg(test)] let guard = { let mut guard = guard; @@ -4571,11 +4608,13 @@ impl ECStore { shard_integrity_write_mode: dst_opts.shard_integrity_write_mode, version_id: dst_opts.version_id.clone(), no_lock: dst_opts.no_lock, + write_completion: dst_opts.write_completion, mod_time: dst_opts.mod_time, http_preconditions: dst_opts.http_preconditions.clone(), expected_current_version_id: dst_opts.expected_current_version_id.clone(), expected_bucket_incarnation_id: dst_opts.expected_bucket_incarnation_id, namespace_lock_fence: dst_opts.namespace_lock_fence.clone(), + write_lock_context: dst_opts.write_lock_context.clone(), bucket_lifecycle_lock_fence: dst_opts.bucket_lifecycle_lock_fence.clone(), object_lock_config_snapshot: dst_opts.object_lock_config_snapshot.clone(), quota_admission: dst_opts.quota_admission, @@ -4614,11 +4653,13 @@ impl ECStore { shard_integrity_write_mode: dst_opts.shard_integrity_write_mode, version_id: dst_opts.version_id.clone(), no_lock: dst_opts.no_lock, + write_completion: dst_opts.write_completion, mod_time: dst_opts.mod_time, http_preconditions: dst_opts.http_preconditions.clone(), expected_current_version_id: dst_opts.expected_current_version_id.clone(), expected_bucket_incarnation_id: dst_opts.expected_bucket_incarnation_id, namespace_lock_fence: dst_opts.namespace_lock_fence.clone(), + write_lock_context: dst_opts.write_lock_context.clone(), bucket_lifecycle_lock_fence: dst_opts.bucket_lifecycle_lock_fence.clone(), object_lock_config_snapshot: dst_opts.object_lock_config_snapshot.clone(), quota_admission: dst_opts.quota_admission, @@ -4667,11 +4708,13 @@ impl ECStore { shard_integrity_write_mode: dst_opts.shard_integrity_write_mode, version_id: dst_opts.version_id.clone(), no_lock: dst_opts.no_lock, + write_completion: dst_opts.write_completion, mod_time: dst_opts.mod_time, http_preconditions: dst_opts.http_preconditions.clone(), expected_current_version_id: dst_opts.expected_current_version_id.clone(), expected_bucket_incarnation_id: dst_opts.expected_bucket_incarnation_id, namespace_lock_fence: dst_opts.namespace_lock_fence.clone(), + write_lock_context: dst_opts.write_lock_context.clone(), bucket_lifecycle_lock_fence: dst_opts.bucket_lifecycle_lock_fence.clone(), object_lock_config_snapshot: dst_opts.object_lock_config_snapshot.clone(), quota_admission: dst_opts.quota_admission, @@ -8721,6 +8764,108 @@ mod tests { } } + #[tokio::test] + #[serial_test::serial] + async fn borrowed_copy_tail_drained_waits_for_rename_tail() { + use crate::object_api::{WriteCommitGuard, WriteCompletion}; + use crate::set_disk::{ENV_RUSTFS_PUT_RENAME_EARLY_ACK_ENABLE, rename_fanout_barrier}; + + temp_env::async_with_vars([(ENV_RUSTFS_PUT_RENAME_EARLY_ACK_ENABLE, Some("true"))], async { + let ctx = Arc::new(crate::runtime::instance::InstanceContext::new()); + let (_dirs, original) = make_local_set_disks_with_ctx(4, 2, Arc::clone(&ctx)).await; + let store = Arc::new(new_prepared_reader_test_store_with_ctx(&[original], ctx).await); + let bucket = RUSTFS_META_BUCKET; + let source = "copy-tail-drained-source"; + let target = "copy-tail-drained-target"; + let payload = vec![0x64; 4097]; + let set = store.pools[0].get_disks_by_key(target); + let mut source_info = set + .put_object( + bucket, + source, + &mut PutObjReader::from_vec(payload.clone()), + &ObjectOptions { + write_completion: WriteCompletion::TailDrained, + ..Default::default() + }, + ) + .await + .expect("seed copy source"); + source_info.put_object_reader = Some(PutObjReader::from_vec(payload.clone())); + + let barrier = rename_fanout_barrier::arm(target, 0, rename_fanout_barrier::PHASE_RENAME); + let rename_tasks = rename_fanout_barrier::observe_tasks(target); + let copy_store = Arc::clone(&store); + let mut copy = tokio::spawn(async move { + let lock = copy_store + .new_ns_lock(bucket, target) + .await + .expect("target namespace wrapper"); + let guard = WriteCommitGuard::acquire(&lock, get_lock_acquire_timeout()) + .await + .expect("real Copy target namespace owner"); + let mut options = ObjectOptions { + write_completion: WriteCompletion::TailDrained, + ..Default::default() + }; + options.add_write_commit_guard(&guard); + copy_store + .handle_copy_object(bucket, source, bucket, target, &mut source_info, &ObjectOptions::default(), &options) + .await + }); + let deadline = tokio::time::Instant::now() + Duration::from_secs(30); + tokio::time::timeout_at(deadline, barrier.wait_until_paused()) + .await + .expect("copy reaches the paused rename checkpoint"); + tokio::time::timeout_at(deadline, async { + loop { + if let Ok(info) = set + .get_object_info( + bucket, + target, + &ObjectOptions { + no_lock: true, + ..Default::default() + }, + ) + .await + && info.size == 4097 + && rename_tasks.running() == 1 + { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .expect("copy commits a metadata quorum while one rename remains paused"); + assert!( + tokio::time::timeout(Duration::from_secs(1), &mut copy).await.is_err(), + "Copy TailDrained must remain pending after metadata quorum" + ); + barrier.release(); + let result = tokio::time::timeout_at(deadline, copy) + .await + .expect("copy drains its rename tail") + .expect("copy task completes") + .expect("copy succeeds"); + assert_eq!(result.size, 4097); + assert_eq!(rename_tasks.running(), 0); + let mut reader = store + .get_object_reader(bucket, target, None, HeaderMap::new(), &ObjectOptions::default()) + .await + .expect("read copied target"); + let mut actual = Vec::new(); + reader + .stream + .read_to_end(&mut actual) + .await + .expect("read complete copied bytes"); + assert_eq!(actual, payload); + }) + .await; + } + #[tokio::test] #[serial_test::serial] async fn object_lock_diagnostics_report_historical_get_blocked_by_put_commit() { diff --git a/crates/heal/tests/shard_identity_receipt_test.rs b/crates/heal/tests/shard_identity_receipt_test.rs index 4811c0d81..aa0f59a8d 100644 --- a/crates/heal/tests/shard_identity_receipt_test.rs +++ b/crates/heal/tests/shard_identity_receipt_test.rs @@ -21,11 +21,13 @@ use rustfs_heal::heal::{ use rustfs_heal_contracts::heal_channel::{HealOpts, HealScanMode}; use rustfs_test_utils::TestECStoreEnv; use serial_test::serial; +use std::time::Duration; use tokio::io::AsyncReadExt as _; mod storage_api; use storage_api::integration::{ - DiskAPI, ObjectIO, ObjectOptions, PutObjReader, ReadOptions, ShardIntegrityWriteMode, WriteCompletion, + DiskAPI, NamespaceLocking as _, ObjectIO, ObjectOptions, PutObjReader, ReadOptions, ShardIntegrityWriteMode, + WriteCommitGuard, WriteCompletion, }; #[tokio::test] @@ -132,17 +134,18 @@ async fn receipt_requires_independent_deep_verification() { let object = format!("target-{corrupt_count}"); let donor = format!("donor-{corrupt_count}"); for (name, byte) in [(&object, 0x3c), (&donor, 0xa9)] { + let lock = env.ecstore.new_ns_lock(bucket, name).await.expect("fixture namespace lock"); + let guard = WriteCommitGuard::acquire(&lock, Duration::from_secs(30)) + .await + .expect("fixture write owner"); + let mut opts = ObjectOptions { + shard_integrity_write_mode: Some(ShardIntegrityWriteMode::Protected), + write_completion: WriteCompletion::TailDrained, + ..Default::default() + }; + opts.add_write_commit_guard(&guard); env.ecstore - .put_object( - bucket, - name, - &mut PutObjReader::from_vec(vec![byte; 1024 * 1024 + 123]), - &ObjectOptions { - shard_integrity_write_mode: Some(ShardIntegrityWriteMode::Protected), - no_lock: true, - ..Default::default() - }, - ) + .put_object(bucket, name, &mut PutObjReader::from_vec(vec![byte; 1024 * 1024 + 123]), &opts) .await .expect("commit all fixture shards"); } diff --git a/crates/heal/tests/storage_api.rs b/crates/heal/tests/storage_api.rs index 4edf01007..b47737203 100644 --- a/crates/heal/tests/storage_api.rs +++ b/crates/heal/tests/storage_api.rs @@ -24,7 +24,9 @@ pub(crate) mod integration { DiskAPI, DiskError, DiskOption, DiskStore, Endpoint, RUSTFS_META_BUCKET, ReadOptions, new_disk, }; pub(crate) use rustfs_ecstore::api::error::StorageError as EcstoreStorageError; - pub(crate) use rustfs_ecstore::api::object::{ObjectOptions, PutObjReader, ShardIntegrityWriteMode, WriteCompletion}; + pub(crate) use rustfs_ecstore::api::object::{ + ObjectOptions, PutObjReader, ShardIntegrityWriteMode, WriteCommitGuard, WriteCompletion, + }; pub(crate) use rustfs_ecstore::api::storage::ECStore; pub(crate) use rustfs_storage_api::BucketOperations; pub(crate) use rustfs_storage_api::MakeBucketOptions; diff --git a/crates/lock/src/namespace/mod.rs b/crates/lock/src/namespace/mod.rs index ec90c5ccc..64238295b 100644 --- a/crates/lock/src/namespace/mod.rs +++ b/crates/lock/src/namespace/mod.rs @@ -57,6 +57,11 @@ impl NamespaceLockWrapper { &self.owner } + /// The exact namespace key used by this acquisition wrapper. + pub fn resource(&self) -> &ObjectKey { + &self.resource + } + /// Logical namespace label for diagnostics; local namespaces may share a manager. pub fn namespace(&self) -> &str { self.lock.namespace() diff --git a/crates/scanner/src/scanner/tests.rs b/crates/scanner/src/scanner/tests.rs index c52a890c8..f94f1c960 100644 --- a/crates/scanner/src/scanner/tests.rs +++ b/crates/scanner/src/scanner/tests.rs @@ -15,7 +15,7 @@ use super::heal_info::{classify_background_heal_read_error, decode_background_heal_info}; use super::*; use crate::EcstoreResult; -use crate::storage_api::owner::ecstore_hold_namespace_commit; +use crate::storage_api::owner::{WriteCommitGuard, WriteCompletion, ecstore_hold_namespace_commit}; use crate::storage_api::scan::{BucketOperations as _, ObjectIO as _}; use crate::{ DATA_USAGE_BLOOM_RECOVERY_PATH, DATA_USAGE_CACHE_KEY_FORMAT, DATA_USAGE_CACHE_NAME, DATA_USAGE_ROOT, @@ -1437,18 +1437,21 @@ async fn coordinator_walks_during_pending_put_without_persisting_or_acknowledgin .await .expect("fixture bucket should be created"); let mut reader = PutObjReader::from_vec(b"first".to_vec()); - store.pools[0].disk_set[0] - .put_object( - &bucket, - "object", - &mut reader, - &ObjectOptions { - no_lock: true, - ..Default::default() - }, - ) - .await - .expect("fixture object should finish its rename fanout"); + { + let set = &store.pools[0].disk_set[0]; + let lock = set.new_ns_lock(&bucket, "object").await.expect("fixture namespace lock"); + let guard = WriteCommitGuard::acquire(&lock, Duration::from_secs(30)) + .await + .expect("fixture write owner"); + let mut opts = ObjectOptions { + write_completion: WriteCompletion::TailDrained, + ..Default::default() + }; + opts.add_write_commit_guard(&guard); + set.put_object(&bucket, "object", &mut reader, &opts) + .await + .expect("fixture object should finish its rename fanout"); + } crate::scanner_io::record_dirty_usage_bucket(&bucket); let dirty_before = crate::scanner_io::dirty_usage_buckets_for_tests(); let baseline = read_config(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str()) @@ -1509,18 +1512,21 @@ async fn coordinator_walks_during_pending_put_without_persisting_or_acknowledgin let committed_body = b"committed-after-walk"; let mut reader = PutObjReader::from_vec(committed_body.to_vec()); - store.pools[0].disk_set[0] - .put_object( - &bucket, - "object", - &mut reader, - &ObjectOptions { - no_lock: true, - ..Default::default() - }, - ) - .await - .expect("the pending tail must change the physical object before it drains"); + { + let set = &store.pools[0].disk_set[0]; + let lock = set.new_ns_lock(&bucket, "object").await.expect("fixture namespace lock"); + let guard = WriteCommitGuard::acquire(&lock, Duration::from_secs(30)) + .await + .expect("fixture write owner"); + let mut opts = ObjectOptions { + write_completion: WriteCompletion::TailDrained, + ..Default::default() + }; + opts.add_write_commit_guard(&guard); + set.put_object(&bucket, "object", &mut reader, &opts) + .await + .expect("the pending tail must change the physical object before it drains"); + } assert_eq!(crate::scanner_io::dirty_usage_buckets_for_tests(), dirty_before); drop(pending); let retry_budget = ScannerCycleBudget::new_with_progress_tracking(&ctx, ScannerCycleBudgetConfig::default()); diff --git a/crates/scanner/src/scanner_io/tests.rs b/crates/scanner/src/scanner_io/tests.rs index 552957fd8..05f51fbe3 100644 --- a/crates/scanner/src/scanner_io/tests.rs +++ b/crates/scanner/src/scanner_io/tests.rs @@ -23,7 +23,7 @@ use crate::scanner_folder::ScannerItem; use crate::storage_api::EcstoreScannerPeerDirtyUsageSnapshot; use crate::storage_api::owner::{ EcstorePoolDecommissionInfo, EcstoreRebalStatus, EcstoreRebalanceInfo, EcstoreRebalanceMeta, EcstoreRebalanceStats, - ecstore_hold_namespace_commit, + WriteCommitGuard, WriteCompletion, ecstore_hold_namespace_commit, }; use crate::storage_api::scan::{BucketOperations as _, DeleteBucketOptions, MakeBucketOptions, ObjectIO as _}; use crate::{ @@ -1424,16 +1424,17 @@ async fn pending_put_commit_keeps_scanner_walk_live_without_authoritative_usage( .enumerate() { let mut reader = ScannerPutObjReader::from_vec(body.to_vec()); - store.pools[pool_index].disk_set[0] - .put_object( - &bucket, - object, - &mut reader, - &ScannerObjectOptions { - no_lock: true, - ..Default::default() - }, - ) + let set = &store.pools[pool_index].disk_set[0]; + let lock = set.new_ns_lock(&bucket, object).await.expect("fixture namespace lock"); + let guard = WriteCommitGuard::acquire(&lock, Duration::from_secs(30)) + .await + .expect("fixture write owner"); + let mut opts = ScannerObjectOptions { + write_completion: WriteCompletion::TailDrained, + ..Default::default() + }; + opts.add_write_commit_guard(&guard); + set.put_object(&bucket, object, &mut reader, &opts) .await .expect("fixture objects must finish their rename fanouts before scanning"); } diff --git a/crates/scanner/src/scanner_io/tests/service_cohort.rs b/crates/scanner/src/scanner_io/tests/service_cohort.rs index 00ea5324c..deb1da547 100644 --- a/crates/scanner/src/scanner_io/tests/service_cohort.rs +++ b/crates/scanner/src/scanner_io/tests/service_cohort.rs @@ -26,18 +26,20 @@ async fn create_cohort_bucket_objects(store: &ECStore, bucket: &str, objects: us .expect("fixture bucket"); for set in store.all_set_disks() { for index in 0..objects { + let object = format!("object-{index:04}"); let mut reader = ScannerPutObjReader::from_vec(b"cohort".to_vec()); - set.put_object( - bucket, - &format!("object-{index:04}"), - &mut reader, - &ScannerObjectOptions { - no_lock: true, - ..Default::default() - }, - ) - .await - .expect("fixture object and all rename tails should persist"); + let lock = set.new_ns_lock(bucket, &object).await.expect("fixture namespace lock"); + let guard = WriteCommitGuard::acquire(&lock, Duration::from_secs(30)) + .await + .expect("fixture write owner"); + let mut opts = ScannerObjectOptions { + write_completion: WriteCompletion::TailDrained, + ..Default::default() + }; + opts.add_write_commit_guard(&guard); + set.put_object(bucket, &object, &mut reader, &opts) + .await + .expect("fixture object and all rename tails should persist"); } } } diff --git a/crates/scanner/src/storage_api.rs b/crates/scanner/src/storage_api.rs index d1be792b7..cf715f089 100644 --- a/crates/scanner/src/storage_api.rs +++ b/crates/scanner/src/storage_api.rs @@ -149,6 +149,8 @@ pub(crate) mod owner { #[cfg(test)] pub(crate) use super::{NativeScannerPauseBacklogWriteFault, SourceCleanupDeleteBarrier}; + #[cfg(test)] + pub(crate) use rustfs_ecstore::api::object::{WriteCommitGuard, WriteCompletion}; #[cfg(test)] pub(crate) use rustfs_ecstore::api::set_disk::get_lock_acquire_timeout as ecstore_get_lock_acquire_timeout; #[cfg(test)] diff --git a/rustfs/src/admin/handlers/site_replication.rs b/rustfs/src/admin/handlers/site_replication.rs index 38f2828fa..09e668fa7 100644 --- a/rustfs/src/admin/handlers/site_replication.rs +++ b/rustfs/src/admin/handlers/site_replication.rs @@ -7358,32 +7358,36 @@ where { let store = current_object_store_handle().ok_or_else(|| S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string()))?; - with_config_object_write_lock(store, SITE_REPLICATION_JOIN_ADMISSION_LOCK_PATH.to_string(), move || async move { - let fresh = load_site_replication_state().await?; - let fresh_local_peer = local_peer_at_endpoint(local_endpoint.clone(), &fresh); - if join_request_is_superseded(&fresh, join_req.updated_at) { - let peer = fresh - .peers - .get(&fresh_local_peer.deployment_id) - .cloned() - .unwrap_or(fresh_local_peer); - return Ok(PeerJoinOutcome::Superseded(peer)); - } - - apply_iam(join_req.clone()).await?; - - let incoming_updated_at = join_req.updated_at; - update_site_replication_state_when_changed(move |state| { - let local_peer = local_peer_at_endpoint(local_endpoint, state); - if join_request_is_superseded(state, incoming_updated_at) { - let peer = state.peers.get(&local_peer.deployment_id).cloned().unwrap_or(local_peer); - return Ok(StateCommit::Unchanged(PeerJoinOutcome::Superseded(peer))); + with_config_object_write_lock( + store, + SITE_REPLICATION_JOIN_ADMISSION_LOCK_PATH.to_string(), + move |_write_guard| async move { + let fresh = load_site_replication_state().await?; + let fresh_local_peer = local_peer_at_endpoint(local_endpoint.clone(), &fresh); + if join_request_is_superseded(&fresh, join_req.updated_at) { + let peer = fresh + .peers + .get(&fresh_local_peer.deployment_id) + .cloned() + .unwrap_or(fresh_local_peer); + return Ok(PeerJoinOutcome::Superseded(peer)); } - apply_peer_join(state, &local_peer, join_req, defer_sync_state_enable); - Ok(StateCommit::Changed(PeerJoinOutcome::Applied(Box::new(state.clone()), local_peer))) - }) - .await - }) + + apply_iam(join_req.clone()).await?; + + let incoming_updated_at = join_req.updated_at; + update_site_replication_state_when_changed(move |state| { + let local_peer = local_peer_at_endpoint(local_endpoint, state); + if join_request_is_superseded(state, incoming_updated_at) { + let peer = state.peers.get(&local_peer.deployment_id).cloned().unwrap_or(local_peer); + return Ok(StateCommit::Unchanged(PeerJoinOutcome::Superseded(peer))); + } + apply_peer_join(state, &local_peer, join_req, defer_sync_state_enable); + Ok(StateCommit::Changed(PeerJoinOutcome::Applied(Box::new(state.clone()), local_peer))) + }) + .await + }, + ) .await .map_err(|e| S3Error::with_message(S3ErrorCode::InternalError, format!("lock site replication join admission failed: {e}")))? } diff --git a/rustfs/src/admin/handlers/table_catalog/mod.rs b/rustfs/src/admin/handlers/table_catalog/mod.rs index 144319a1c..37c06d295 100644 --- a/rustfs/src/admin/handlers/table_catalog/mod.rs +++ b/rustfs/src/admin/handlers/table_catalog/mod.rs @@ -1726,8 +1726,18 @@ where object: &str, data: Vec, precondition: crate::table_catalog::TableCatalogPutPrecondition, + write_guards: Vec, ) -> crate::table_catalog::TableCatalogStoreResult<()> { - self.put_object(bucket, object, data, precondition).await + self.authorize(bucket, object, S3Action::PutObjectAction).await?; + self.ensure_observation_allowed(bucket, object)?; + let observation = TableCommitObservedObject { + identity: TableCommitObjectIdentity::ContentSha256(hex_sha256(&data, str::to_string)), + max_size: None, + }; + self.backend + .put_object_unlocked(bucket, object, data, precondition, write_guards) + .await?; + self.record_observation(bucket, object, observation) } async fn delete_object(&self, bucket: &str, object: &str) -> crate::table_catalog::TableCatalogStoreResult<()> { diff --git a/rustfs/src/admin/handlers/table_catalog/tests.rs b/rustfs/src/admin/handlers/table_catalog/tests.rs index 00509c58f..2cab5a7b2 100644 --- a/rustfs/src/admin/handlers/table_catalog/tests.rs +++ b/rustfs/src/admin/handlers/table_catalog/tests.rs @@ -87,6 +87,80 @@ async fn call_load_table_handler(request: S3Request) -> S3Result(()) }) @@ -1154,9 +1154,9 @@ mod tests { let transaction_order = transaction_order.clone(); tokio::spawn(async move { let transaction_store = store.clone(); - let transaction = with_admin_server_config_write_lock(store, move || async move { + let transaction = with_admin_server_config_write_lock(store, move |write_guard| async move { let _ = oidc_entered_tx.send(()); - let mut config = read_admin_config_without_migrate_no_lock(transaction_store.clone()).await?; + let mut config = read_admin_config_without_migrate_no_lock(transaction_store.clone(), &write_guard).await?; transaction_order.lock().expect("transaction order lock").push("oidc-read"); let mut provider = KVS::new(); @@ -1170,7 +1170,7 @@ mod tests { .entry(IDENTITY_OPENID_SUB_SYS.to_string()) .or_default() .insert("concurrent-oidc".to_string(), provider); - save_admin_server_config_no_lock(transaction_store, &config).await?; + save_admin_server_config_no_lock(transaction_store, &config, &write_guard).await?; transaction_order.lock().expect("transaction order lock").push("oidc-save"); Ok::<(), StorageError>(()) }); @@ -1449,9 +1449,9 @@ mod tests { let (writer_entered_tx, mut writer_entered_rx) = tokio::sync::oneshot::channel(); let writer = tokio::spawn(async move { let transaction_store = writer_store.clone(); - let transaction = with_admin_server_config_write_lock(writer_store, move || async move { + let transaction = with_admin_server_config_write_lock(writer_store, move |write_guard| async move { writer_entered_tx.send(()).expect("signal writer entry"); - save_admin_server_config_no_lock(transaction_store, &latest).await + save_admin_server_config_no_lock(transaction_store, &latest, &write_guard).await }); tokio::pin!(transaction); let mut writer_polled_tx = Some(writer_polled_tx); diff --git a/rustfs/src/admin/service/site_replication.rs b/rustfs/src/admin/service/site_replication.rs index 3f47b584e..f8504b134 100644 --- a/rustfs/src/admin/service/site_replication.rs +++ b/rustfs/src/admin/service/site_replication.rs @@ -118,13 +118,13 @@ pub async fn reload_site_replication_runtime_state_for_context(context: Option<& // fan-out cannot overwrite a concurrent state writer. IO must be the // no-lock variants — the boundary already holds the object lock. let lock_store = store.clone(); - with_site_replication_state_lock_on(lock_store, move || async move { + with_site_replication_state_lock_on(lock_store, move |write_guard| async move { match read_config_no_lock(store.clone(), SITE_REPLICATION_STATE_PATH).await { Ok(data) => { if let Some(normalized) = normalize_site_replication_state_json(&data) .map_err(|e| S3Error::with_message(S3ErrorCode::InternalError, e))? { - save_config_no_lock(store, SITE_REPLICATION_STATE_PATH, normalized) + save_config_no_lock(store, SITE_REPLICATION_STATE_PATH, normalized, &write_guard) .await .map_err(|e| { S3Error::with_message( diff --git a/rustfs/src/admin/storage_api.rs b/rustfs/src/admin/storage_api.rs index beade7fc2..3f56bcb2d 100644 --- a/rustfs/src/admin/storage_api.rs +++ b/rustfs/src/admin/storage_api.rs @@ -826,8 +826,11 @@ pub(crate) async fn read_existing_admin_server_config_no_lock(api: Arc) } #[cfg(test)] -pub(crate) async fn read_admin_config_without_migrate_no_lock(api: Arc) -> Result { - ecstore_config::com::read_config_without_migrate_no_lock(api).await +pub(crate) async fn read_admin_config_without_migrate_no_lock( + api: Arc, + guard: &crate::storage::storage_api::ecstore_object::WriteCommitGuard, +) -> Result { + ecstore_config::com::read_config_without_migrate_no_lock(api, guard).await } pub(crate) type AdminServerConfigSnapshot = ecstore_config::com::ServerConfigSnapshot; @@ -850,14 +853,15 @@ pub(crate) async fn save_admin_server_config(api: Arc, cfg: &rustfs_con pub(crate) async fn save_admin_server_config_no_lock( api: Arc, cfg: &rustfs_config::server_config::Config, + guard: &crate::storage::storage_api::ecstore_object::WriteCommitGuard, ) -> Result<()> { - ecstore_config::com::save_server_config_no_lock(api, cfg).await + ecstore_config::com::save_server_config_no_lock(api, cfg, guard).await } #[cfg(test)] pub(crate) async fn with_admin_server_config_write_lock(api: Arc, operation: F) -> Result where - F: FnOnce() -> Fut + Send + 'static, + F: FnOnce(crate::storage::storage_api::ecstore_object::WriteCommitGuard) -> Fut + Send + 'static, Fut: std::future::Future + Send + 'static, T: Send + 'static, { @@ -1033,6 +1037,7 @@ pub(crate) mod metrics { pub(crate) mod object { pub(crate) use crate::storage::storage_api::StorageObjectOptions; + pub(crate) use crate::storage::storage_api::ecstore_object::WriteCommitGuard; } pub(crate) mod rebalance { diff --git a/rustfs/src/app/object/copy.rs b/rustfs/src/app/object/copy.rs index 4ce0774ff..e674375cd 100644 --- a/rustfs/src/app/object/copy.rs +++ b/rustfs/src/app/object/copy.rs @@ -96,13 +96,17 @@ fn copy_namespace_lock_error(bucket: &str, object: &str, mode: &'static str, err } } -async fn acquire_self_copy_namespace_lock(store: &S, bucket: &str, object: &str) -> S3Result +async fn acquire_self_copy_namespace_lock( + store: &S, + bucket: &str, + object: &str, +) -> S3Result where S: NamespaceLocking + ?Sized, { let object = encode_dir_object(object); let lock = store.new_ns_lock(bucket, &object).await.map_err(ApiError::from)?; - lock.get_write_lock(get_lock_acquire_timeout()) + crate::app::storage_api::object_usecase::WriteCommitGuard::acquire(&lock, get_lock_acquire_timeout()) .await .map_err(|err| ApiError::from(copy_namespace_lock_error(bucket, &object, "write", err)).into()) } @@ -420,7 +424,8 @@ impl DefaultObjectUsecase { None }; if let Some(guard) = _self_copy_lock_guard.as_ref() { - dst_opts.add_namespace_lock_guard(guard); + dst_opts.add_write_commit_guard(guard); + dst_opts.write_completion = crate::app::storage_api::object_usecase::WriteCompletion::TailDrained; } dst_opts.expected_current_version_id = expected_current_version_id.clone(); @@ -1098,7 +1103,7 @@ mod tests { let payload = b"object whose key equals its bucket".to_vec(); let mut reader = PutObjReader::from_vec(payload.clone()); let setup_opts = ObjectOptions { - no_lock: true, + write_completion: crate::app::storage_api::object_usecase::WriteCompletion::TailDrained, ..Default::default() }; store diff --git a/rustfs/src/app/storage_api.rs b/rustfs/src/app/storage_api.rs index 23e63777b..4e15fc1e6 100644 --- a/rustfs/src/app/storage_api.rs +++ b/rustfs/src/app/storage_api.rs @@ -1181,6 +1181,7 @@ pub(crate) mod bucket_usecase { pub(crate) mod object_usecase { pub(crate) use super::storage_contracts::BUCKET_LIFECYCLE_LOCK_OBJECT; + pub(crate) use crate::storage::storage_api::ecstore_object::{WriteCommitGuard, WriteCompletion}; pub(crate) mod object_cache { #[cfg(test)] diff --git a/rustfs/src/server/module_switch.rs b/rustfs/src/server/module_switch.rs index fdca6fe6e..4143f842e 100644 --- a/rustfs/src/server/module_switch.rs +++ b/rustfs/src/server/module_switch.rs @@ -275,11 +275,11 @@ where // waiting for the local guard. serialize_module_switch_rmw(|| async move { let save_store = store.clone(); - with_config_object_write_lock(store, MODULE_SWITCH_CONFIG_PATH.to_string(), move || async move { + with_config_object_write_lock(store, MODULE_SWITCH_CONFIG_PATH.to_string(), move |write_guard| async move { save_persisted_module_switches_inner( config, move |data| async move { - save_config_no_lock(save_store, MODULE_SWITCH_CONFIG_PATH, data) + save_config_no_lock(save_store, MODULE_SWITCH_CONFIG_PATH, data, &write_guard) .await .map_err(|e| format!("failed to save module switch config: {e}")) }, @@ -631,14 +631,14 @@ mod tests { async move { writer_started_tx.send(()).expect("signal remote writer start"); let save_store = store.clone(); - with_config_object_write_lock(store, MODULE_SWITCH_CONFIG_PATH.to_string(), move || async move { + with_config_object_write_lock(store, MODULE_SWITCH_CONFIG_PATH.to_string(), move |write_guard| async move { writer_entered.store(true, AtomicOrdering::SeqCst); let data = serde_json::to_vec(&PersistedModuleSwitches { notify_enabled: false, audit_enabled: false, }) .expect("serialize disabled module state"); - save_config_no_lock(save_store, MODULE_SWITCH_CONFIG_PATH, data) + save_config_no_lock(save_store, MODULE_SWITCH_CONFIG_PATH, data, &write_guard) .await .map_err(|err| err.to_string())?; publish_order.lock().expect("module publish order lock").push("new-false"); diff --git a/rustfs/src/site_replication/hooks.rs b/rustfs/src/site_replication/hooks.rs index a705985e0..79a03f5b0 100644 --- a/rustfs/src/site_replication/hooks.rs +++ b/rustfs/src/site_replication/hooks.rs @@ -46,7 +46,7 @@ where store, SITE_REPLICATION_BUCKET_MUTATION_ADMISSION_LOCK_PATH.to_string(), move || async move { - with_config_object_write_lock(mutation_store, mutation_path, operation) + with_config_object_write_lock(mutation_store, mutation_path, move |_write_guard| operation()) .await .map_err(|err| S3Error::from(ApiError::from(err))) }, @@ -68,9 +68,13 @@ where Fut: std::future::Future> + Send + 'static, T: Send + 'static, { - with_config_object_write_lock(store, SITE_REPLICATION_BUCKET_MUTATION_ADMISSION_LOCK_PATH.to_string(), operation) - .await - .map_err(|err| S3Error::from(ApiError::from(err)))? + with_config_object_write_lock( + store, + SITE_REPLICATION_BUCKET_MUTATION_ADMISSION_LOCK_PATH.to_string(), + move |_write_guard| operation(), + ) + .await + .map_err(|err| S3Error::from(ApiError::from(err)))? } #[derive(Debug, Default)] @@ -978,9 +982,11 @@ pub(crate) async fn commit_site_replication_delete_bucket(intent: &SiteReplicati reservations: intent.reservations.clone(), displaced: Vec::new(), }; - match with_config_object_write_lock(store, SITE_REPLICATION_REPAIR_EXECUTION_LOCK_PATH.to_string(), move || async move { - broadcast_site_replication_delete_bucket(&delivery_intent).await - }) + match with_config_object_write_lock( + store, + SITE_REPLICATION_REPAIR_EXECUTION_LOCK_PATH.to_string(), + move |_write_guard| async move { broadcast_site_replication_delete_bucket(&delivery_intent).await }, + ) .await { Ok(result) => result, diff --git a/rustfs/src/site_replication/repair.rs b/rustfs/src/site_replication/repair.rs index ec341d70f..ca9523263 100644 --- a/rustfs/src/site_replication/repair.rs +++ b/rustfs/src/site_replication/repair.rs @@ -169,10 +169,11 @@ pub(crate) async fn load_site_replication_repair_state_from_store(store: Arc, state: &SiteReplicationRepairState, + guard: &crate::storage_api::site_replication::WriteCommitGuard, ) -> S3Result<()> { let data = serde_json::to_vec(state) .map_err(|e| S3Error::with_message(S3ErrorCode::InternalError, format!("serialize repair state failed: {e}")))?; - save_config_no_lock(store, SITE_REPLICATION_REPAIR_STATE_PATH, data) + save_config_no_lock(store, SITE_REPLICATION_REPAIR_STATE_PATH, data, guard) .await .map_err(|e| S3Error::with_message(S3ErrorCode::InternalError, format!("save repair state failed: {e}"))) } @@ -197,10 +198,10 @@ where current_object_store_handle().ok_or_else(|| S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string()))?; let read_store = store.clone(); let save_store = store.clone(); - with_config_object_write_lock(store, SITE_REPLICATION_REPAIR_STATE_PATH.to_string(), move || async move { + with_config_object_write_lock(store, SITE_REPLICATION_REPAIR_STATE_PATH.to_string(), move |write_guard| async move { let mut state = load_site_replication_repair_state_from_store(read_store).await?; let result = update(&mut state)?; - save_site_replication_repair_state_to_store(save_store, &state).await?; + save_site_replication_repair_state_to_store(save_store, &state, &write_guard).await?; Ok(result) }) .await @@ -709,9 +710,11 @@ pub(crate) async fn execute_site_replication_repair( ) -> S3Result> { let store = current_object_store_handle().ok_or_else(|| S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string()))?; - with_config_object_write_lock(store, SITE_REPLICATION_REPAIR_EXECUTION_LOCK_PATH.to_string(), move || async move { - execute_site_replication_repair_locked(request).await - }) + with_config_object_write_lock( + store, + SITE_REPLICATION_REPAIR_EXECUTION_LOCK_PATH.to_string(), + move |_write_guard| async move { execute_site_replication_repair_locked(request).await }, + ) .await .map_err(|_| { S3Error::with_message(S3ErrorCode::ClientTokenConflict, "another site replication repair is active".to_string()) diff --git a/rustfs/src/site_replication/retry.rs b/rustfs/src/site_replication/retry.rs index 383cb15f7..e1a3b9599 100644 --- a/rustfs/src/site_replication/retry.rs +++ b/rustfs/src/site_replication/retry.rs @@ -1700,23 +1700,27 @@ async fn drain_site_replication_retry_queue_lightweight_inner() -> S3Result<()> // timestamp, and the locked reload below decides what may actually send. promote_reachable_deferred_retry_events(&runtime, &actionable, deferred).await?; - with_config_object_write_lock(store, SITE_REPLICATION_REPAIR_EXECUTION_LOCK_PATH.to_string(), move || async move { - let Some(runtime) = runtime_site_replication_targets().await? else { - return Ok(()); - }; - if runtime.state.pending_endpoint_refresh.is_some() - || runtime.state.pending_remove.is_some() - || runtime.state.pending_rotation.is_some() - { - return Ok(()); - } - let now = OffsetDateTime::now_utc(); - let (actionable, _) = lightweight_retry_drain_partition(&runtime.state, now); - if actionable.is_empty() { - return Ok(()); - } - drain_site_replication_retry_queue_lightweight_locked(Arc::new(runtime), actionable, now).await - }) + with_config_object_write_lock( + store, + SITE_REPLICATION_REPAIR_EXECUTION_LOCK_PATH.to_string(), + move |_write_guard| async move { + let Some(runtime) = runtime_site_replication_targets().await? else { + return Ok(()); + }; + if runtime.state.pending_endpoint_refresh.is_some() + || runtime.state.pending_remove.is_some() + || runtime.state.pending_rotation.is_some() + { + return Ok(()); + } + let now = OffsetDateTime::now_utc(); + let (actionable, _) = lightweight_retry_drain_partition(&runtime.state, now); + if actionable.is_empty() { + return Ok(()); + } + drain_site_replication_retry_queue_lightweight_locked(Arc::new(runtime), actionable, now).await + }, + ) .await .map_err(ApiError::from)? } @@ -1762,26 +1766,30 @@ pub(crate) async fn drain_site_replication_retry_queue_inner() -> S3Result<()> { // re-runs the dry-run. The lock elects one server to replay the queue; // after acquiring it, reload state so a settled event or deleted bucket // cannot be replayed from this admission snapshot. - with_config_object_write_lock(store, SITE_REPLICATION_REPAIR_EXECUTION_LOCK_PATH.to_string(), move || async move { - // Runtime and queue snapshots captured before this distributed lock - // are only admission hints. Another node may have settled the event, - // or a local bucket may have been deleted, while this node waited. - let Some(runtime) = runtime_site_replication_targets().await? else { - return Ok(()); - }; - if runtime.state.pending_endpoint_refresh.is_some() - || runtime.state.pending_remove.is_some() - || runtime.state.pending_rotation.is_some() - { - return Ok(()); - } - let horizon = heavyweight_retry_drain_horizon(OffsetDateTime::now_utc()); - let actionable = actionable_site_replication_retry_events(&runtime.state, horizon); - if actionable.is_empty() { - return Ok(()); - } - drain_site_replication_retry_queue_locked(runtime, actionable).await - }) + with_config_object_write_lock( + store, + SITE_REPLICATION_REPAIR_EXECUTION_LOCK_PATH.to_string(), + move |_write_guard| async move { + // Runtime and queue snapshots captured before this distributed lock + // are only admission hints. Another node may have settled the event, + // or a local bucket may have been deleted, while this node waited. + let Some(runtime) = runtime_site_replication_targets().await? else { + return Ok(()); + }; + if runtime.state.pending_endpoint_refresh.is_some() + || runtime.state.pending_remove.is_some() + || runtime.state.pending_rotation.is_some() + { + return Ok(()); + } + let horizon = heavyweight_retry_drain_horizon(OffsetDateTime::now_utc()); + let actionable = actionable_site_replication_retry_events(&runtime.state, horizon); + if actionable.is_empty() { + return Ok(()); + } + drain_site_replication_retry_queue_locked(runtime, actionable).await + }, + ) .await .map_err(ApiError::from)? } diff --git a/rustfs/src/site_replication/state.rs b/rustfs/src/site_replication/state.rs index 731dd5496..beb7e94e8 100644 --- a/rustfs/src/site_replication/state.rs +++ b/rustfs/src/site_replication/state.rs @@ -382,7 +382,11 @@ pub(crate) async fn load_site_replication_state_no_lock(store: Arc) -> /// Persist-or-clear under an already-held state object lock. Normalizes the /// peer map exactly once (the historical persist path normalized twice with /// two full clones — P2-22). -pub(crate) async fn persist_site_replication_state_no_lock(store: Arc, mut state: SiteReplicationState) -> S3Result<()> { +pub(crate) async fn persist_site_replication_state_no_lock( + store: Arc, + mut state: SiteReplicationState, + guard: &crate::storage_api::site_replication::WriteCommitGuard, +) -> S3Result<()> { state.peers = normalize_peer_map_by_identity(state.peers); if state.peers.len() <= 1 && state.pending_rotation.is_none() && state.pending_remove.is_none() { match delete_config_no_lock(store, SITE_REPLICATION_STATE_PATH).await { @@ -392,7 +396,7 @@ pub(crate) async fn persist_site_replication_state_no_lock(store: Arc, } else { let data = serde_json::to_vec(&state) .map_err(|e| S3Error::with_message(S3ErrorCode::InternalError, format!("serialize state failed: {e}")))?; - save_config_no_lock(store, SITE_REPLICATION_STATE_PATH, data) + save_config_no_lock(store, SITE_REPLICATION_STATE_PATH, data, guard) .await .map_err(|e| S3Error::with_message(S3ErrorCode::InternalError, format!("save state failed: {e}"))) } @@ -437,13 +441,13 @@ where F: FnOnce(SiteReplicationState) -> Fut + Send + 'static, Fut: std::future::Future)>> + Send + 'static, { - with_site_replication_state_lock(move || async move { + with_site_replication_state_lock(move |write_guard| async move { let store = current_object_store_handle() .ok_or_else(|| S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string()))?; let state = load_site_replication_state_no_lock(store.clone()).await?; let (result, changed) = transaction(state).await?; if let Some(state) = changed { - persist_site_replication_state_no_lock(store, state).await?; + persist_site_replication_state_no_lock(store, state, &write_guard).await?; } Ok(result) }) @@ -457,13 +461,13 @@ where T: Send + 'static, F: FnOnce(&mut SiteReplicationState) -> S3Result> + Send + 'static, { - with_site_replication_state_lock(move || async move { + with_site_replication_state_lock(move |write_guard| async move { let store = current_object_store_handle() .ok_or_else(|| S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string()))?; let mut state = load_site_replication_state_no_lock(store.clone()).await?; match update(&mut state)? { StateCommit::Changed(result) => { - persist_site_replication_state_no_lock(store, state).await?; + persist_site_replication_state_no_lock(store, state, &write_guard).await?; Ok(result) } StateCommit::Unchanged(result) => Ok(result), diff --git a/rustfs/src/site_replication/state_lock.rs b/rustfs/src/site_replication/state_lock.rs index 9eb228882..5ca14153c 100644 --- a/rustfs/src/site_replication/state_lock.rs +++ b/rustfs/src/site_replication/state_lock.rs @@ -54,7 +54,7 @@ pub(crate) const SITE_REPLICATION_STATE_PATH: &str = "config/site-replication/st pub(crate) async fn with_site_replication_state_lock(operation: F) -> S3Result where T: Send + 'static, - F: FnOnce() -> Fut + Send + 'static, + F: FnOnce(crate::storage_api::site_replication::WriteCommitGuard) -> Fut + Send + 'static, Fut: std::future::Future> + Send + 'static, { let store = current_object_store_handle().ok_or_else(|| S3Error::with_message(S3ErrorCode::InternalError, "Not init"))?; @@ -91,7 +91,7 @@ where pub(crate) async fn with_site_replication_state_lock_on(store: Arc, operation: F) -> S3Result where T: Send + 'static, - F: FnOnce() -> Fut + Send + 'static, + F: FnOnce(crate::storage_api::site_replication::WriteCommitGuard) -> Fut + Send + 'static, Fut: std::future::Future> + Send + 'static, { with_config_object_write_lock(store, SITE_REPLICATION_STATE_PATH.to_string(), operation) diff --git a/rustfs/src/storage/rpc/node_service.rs b/rustfs/src/storage/rpc/node_service.rs index 45e11792a..b4c2d9912 100644 --- a/rustfs/src/storage/rpc/node_service.rs +++ b/rustfs/src/storage/rpc/node_service.rs @@ -9060,7 +9060,7 @@ mod tests { "object", &mut reader, &ObjectOptions { - no_lock: true, + write_completion: crate::storage::storage_api::ecstore_object::WriteCompletion::TailDrained, ..Default::default() }, ), diff --git a/rustfs/src/storage/storage_api.rs b/rustfs/src/storage/storage_api.rs index 8321bad8b..ba09225b8 100644 --- a/rustfs/src/storage/storage_api.rs +++ b/rustfs/src/storage/storage_api.rs @@ -577,9 +577,9 @@ pub(crate) mod ecstore_object { pub(crate) use rustfs_ecstore::api::object::{ EncryptionResolutionError, EncryptionResolutionErrorKind, GetObjectBodyCacheHook, GetObjectBodyCacheHookLookup, ObjectEncryptionResolver, ObjectMutationHook, PrepareSelectObjectSnapshotError, ReadEncryptionMaterial, - ReadEncryptionMode, ReadEncryptionRequest, SelectObjectSnapshot, WriteCompletion, get_object_body_cache_plaintext_len, - lookup_get_object_body_cache_hook, register_get_object_body_cache_hook, register_object_mutation_hook, - unregister_get_object_body_cache_hook, unregister_object_mutation_hook, + ReadEncryptionMode, ReadEncryptionRequest, SelectObjectSnapshot, WriteCommitGuard, WriteCompletion, + get_object_body_cache_plaintext_len, lookup_get_object_body_cache_hook, register_get_object_body_cache_hook, + register_object_mutation_hook, unregister_get_object_body_cache_hook, unregister_object_mutation_hook, }; } @@ -1296,8 +1296,13 @@ pub(crate) fn replication_queue_current_count() -> Option { get_global_replication_stats().and_then(|stats| stats.queue_current_count()) } -pub(crate) async fn save_config_no_lock(api: Arc, file: &str, data: Vec) -> Result<()> { - ecstore_config::com::save_config_no_lock(api, file, data).await +pub(crate) async fn save_config_no_lock( + api: Arc, + file: &str, + data: Vec, + guard: &ecstore_object::WriteCommitGuard, +) -> Result<()> { + ecstore_config::com::save_config_no_lock(api, file, data, guard).await } pub(crate) async fn delete_config_no_lock(api: Arc, file: &str) -> Result<()> { @@ -1306,7 +1311,7 @@ pub(crate) async fn delete_config_no_lock(api: Arc, file: &str) -> Resu pub(crate) async fn with_config_object_write_lock(api: Arc, object: String, operation: F) -> Result where - F: FnOnce() -> Fut + Send + 'static, + F: FnOnce(ecstore_object::WriteCommitGuard) -> Fut + Send + 'static, Fut: std::future::Future + Send + 'static, T: Send + 'static, { diff --git a/rustfs/src/storage_api.rs b/rustfs/src/storage_api.rs index ea42490df..9aabb1826 100644 --- a/rustfs/src/storage_api.rs +++ b/rustfs/src/storage_api.rs @@ -289,6 +289,7 @@ pub(crate) mod site_replication { pub(crate) use crate::storage::storage_api::ecstore_bucket::metadata::{ BUCKET_REPLICATION_CONFIG, BUCKET_TARGETS_FILE, BUCKET_VERSIONING_CONFIG, BucketMetadata, }; + pub(crate) use crate::storage::storage_api::ecstore_object::WriteCommitGuard; #[cfg(test)] pub(crate) use crate::storage::storage_api::ecstore_bucket::replication::merge_incoming_replication_config; @@ -443,6 +444,7 @@ pub(crate) mod workload { } pub(crate) mod table { + pub(crate) use crate::storage::storage_api::ecstore_object::{WriteCommitGuard, WriteCompletion}; pub(crate) mod contract { pub(crate) mod http { pub(crate) use super::super::super::storage_contracts::HTTPPreconditions; diff --git a/rustfs/src/table_catalog/store/mod.rs b/rustfs/src/table_catalog/store/mod.rs index a8c9c3eef..8f8bba826 100644 --- a/rustfs/src/table_catalog/store/mod.rs +++ b/rustfs/src/table_catalog/store/mod.rs @@ -14,6 +14,7 @@ use super::identifier::is_valid_table_metadata_file_name; use super::*; +use crate::storage_api::table::{WriteCommitGuard, WriteCompletion}; mod migration; mod object; @@ -573,6 +574,7 @@ pub(crate) struct TableCatalogObjectMetadata { pub(crate) struct TableCatalogLockGuard { _guard: Box, lock_lost: Option>, + write_commit_guard: Option, } impl TableCatalogLockGuard { @@ -584,6 +586,7 @@ impl TableCatalogLockGuard { Self { _guard: Box::new(guard), lock_lost: None, + write_commit_guard: None, } } @@ -592,11 +595,25 @@ impl TableCatalogLockGuard { Self { _guard: Box::new(guard), lock_lost, + write_commit_guard: None, } } + fn write_namespace(guard: WriteCommitGuard) -> Self { + Self { + _guard: Box::new(()), + lock_lost: None, + write_commit_guard: Some(guard), + } + } + + fn write_commit_guards(&self) -> Vec { + self.write_commit_guard.iter().cloned().collect() + } + pub(crate) fn is_lock_lost(&self) -> bool { self.lock_lost.as_ref().is_some_and(|signal| signal.is_lost()) + || self.write_commit_guard.as_ref().is_some_and(WriteCommitGuard::is_lock_lost) } } @@ -722,6 +739,7 @@ pub(crate) trait TableCatalogObjectBackend: Clone + Send + Sync + 'static { object: &str, data: Vec, precondition: TableCatalogPutPrecondition, + _write_guards: Vec, ) -> TableCatalogStoreResult<()> { self.put_object(bucket, object, data, precondition).await } @@ -1946,7 +1964,7 @@ where data: Vec, precondition: TableCatalogPutPrecondition, ) -> TableCatalogStoreResult<()> { - self.put_object_with_options(bucket, object, data, precondition, false).await + self.put_object_with_options(bucket, object, data, precondition, None).await } async fn put_object_unlocked( @@ -1955,8 +1973,10 @@ where object: &str, data: Vec, precondition: TableCatalogPutPrecondition, + write_guards: Vec, ) -> TableCatalogStoreResult<()> { - self.put_object_with_options(bucket, object, data, precondition, true).await + self.put_object_with_options(bucket, object, data, precondition, Some(write_guards)) + .await } async fn delete_object(&self, bucket: &str, object: &str) -> TableCatalogStoreResult<()> { @@ -2043,11 +2063,10 @@ where .new_ns_lock(bucket, object) .await .map_err(|err| storage_error_to_catalog("create catalog table lock", err))?; - let guard = lock - .get_write_lock(get_lock_acquire_timeout()) + let guard = WriteCommitGuard::acquire(&lock, get_lock_acquire_timeout()) .await .map_err(|err| catalog_lock_acquisition_error("acquire catalog table lock", err))?; - Ok(TableCatalogLockGuard::namespace(guard)) + Ok(TableCatalogLockGuard::write_namespace(guard)) } async fn acquire_read_lock(&self, bucket: &str, object: &str) -> TableCatalogStoreResult { @@ -2120,14 +2139,22 @@ where object: &str, data: Vec, precondition: TableCatalogPutPrecondition, - no_lock: bool, + write_guards: Option>, ) -> TableCatalogStoreResult<()> { let mut reader = PutObjReader::from_vec(data); - let opts = ObjectOptions { + let mut opts = ObjectOptions { http_preconditions: http_preconditions_for_catalog_put(precondition), - no_lock, ..Default::default() }; + if let Some(write_guards) = write_guards { + if write_guards.is_empty() { + return Err(TableCatalogStoreError::Internal("catalog write has no namespace owner".to_string())); + } + for guard in &write_guards { + opts.add_write_commit_guard(guard); + } + opts.write_completion = WriteCompletion::TailDrained; + } self.store .put_object(bucket, object, &mut reader, &opts) .await diff --git a/rustfs/src/table_catalog/store/object.rs b/rustfs/src/table_catalog/store/object.rs index 347d096af..daf11ee60 100644 --- a/rustfs/src/table_catalog/store/object.rs +++ b/rustfs/src/table_catalog/store/object.rs @@ -747,13 +747,16 @@ where object: &str, entry: &T, precondition: TableCatalogPutPrecondition, + write_guards: Vec, ) -> TableCatalogStoreResult<()> where T: Serialize, { let data = serde_json::to_vec(entry) .map_err(|err| TableCatalogStoreError::Internal(format!("failed to serialize catalog entry {object}: {err}")))?; - self.backend.put_object_unlocked(bucket, object, data, precondition).await + self.backend + .put_object_unlocked(bucket, object, data, precondition, write_guards) + .await } async fn write_exact_entry_unlocked( @@ -762,11 +765,14 @@ where object: &str, entry: &T, precondition: TableCatalogPutPrecondition, + write_guards: Vec, ) -> TableCatalogStoreResult where T: DeserializeOwned + PartialEq + Serialize, { - let write_result = self.write_entry_unlocked(bucket, object, entry, precondition).await; + let write_result = self + .write_entry_unlocked(bucket, object, entry, precondition, write_guards) + .await; let current = self.read_entry_unlocked::(bucket, object).await?; match current { Some((current, Some(etag))) if current == *entry => Ok(etag), @@ -878,14 +884,21 @@ where intent: &mut TableRenameIntent, etag: String, state: TableRenameIntentState, + write_guards: Vec, ) -> TableCatalogStoreResult { if intent.state >= state { return Ok(etag); } intent.state = state; intent.updated_at = OffsetDateTime::now_utc().to_string(); - self.write_exact_entry_unlocked(self.catalog_bucket(), object, intent, TableCatalogPutPrecondition::IfMatch(etag)) - .await + self.write_exact_entry_unlocked( + self.catalog_bucket(), + object, + intent, + TableCatalogPutPrecondition::IfMatch(etag), + write_guards, + ) + .await } async fn recover_active_table_rename( @@ -1027,6 +1040,10 @@ where &source_object, &source_fence, TableCatalogPutPrecondition::IfMatch(current_etag), + _catalog_guards + .iter() + .flat_map(TableCatalogLockGuard::write_commit_guards) + .collect(), ) .await?; } @@ -1039,7 +1056,13 @@ where } } intent_etag = self - .advance_table_rename_intent_unlocked(&intent_object, &mut intent, intent_etag, TableRenameIntentState::SourceFenced) + .advance_table_rename_intent_unlocked( + &intent_object, + &mut intent, + intent_etag, + TableRenameIntentState::SourceFenced, + _intent_guard.write_commit_guards(), + ) .await?; let mut destination_fence = intent.destination.clone(); @@ -1055,6 +1078,10 @@ where &destination_object, &destination_fence, TableCatalogPutPrecondition::IfAbsent, + _catalog_guards + .iter() + .flat_map(TableCatalogLockGuard::write_commit_guards) + .collect(), ) .await?; } @@ -1067,6 +1094,10 @@ where &destination_object, &destination_fence, TableCatalogPutPrecondition::IfMatch(current_etag), + _catalog_guards + .iter() + .flat_map(TableCatalogLockGuard::write_commit_guards) + .collect(), ) .await?; } @@ -1084,6 +1115,7 @@ where &mut intent, intent_etag, TableRenameIntentState::DestinationWritten, + _intent_guard.write_commit_guards(), ) .await?; @@ -1098,6 +1130,10 @@ where &source_object, &source_tombstone, TableCatalogPutPrecondition::IfMatch(current_etag), + _catalog_guards + .iter() + .flat_map(TableCatalogLockGuard::write_commit_guards) + .collect(), ) .await?; } @@ -1115,6 +1151,7 @@ where &mut intent, intent_etag, TableRenameIntentState::SourceTombstoned, + _intent_guard.write_commit_guards(), ) .await?; @@ -1139,6 +1176,10 @@ where &index_object, &destination_index, TableCatalogPutPrecondition::IfMatch(intent.warehouse_index_etag.clone()), + _catalog_guards + .iter() + .flat_map(TableCatalogLockGuard::write_commit_guards) + .collect(), ) .await?; } @@ -1148,6 +1189,7 @@ where &mut intent, intent_etag, TableRenameIntentState::IndexPublished, + _intent_guard.write_commit_guards(), ) .await?; match self @@ -1161,6 +1203,10 @@ where &destination_object, &intent.destination, TableCatalogPutPrecondition::IfMatch(current_etag), + _catalog_guards + .iter() + .flat_map(TableCatalogLockGuard::write_commit_guards) + .collect(), ) .await?; } @@ -1178,10 +1224,17 @@ where &mut intent, intent_etag, TableRenameIntentState::DestinationPublished, + _intent_guard.write_commit_guards(), ) .await?; - self.advance_table_rename_intent_unlocked(&intent_object, &mut intent, intent_etag, TableRenameIntentState::Completed) - .await?; + self.advance_table_rename_intent_unlocked( + &intent_object, + &mut intent, + intent_etag, + TableRenameIntentState::Completed, + _intent_guard.write_commit_guards(), + ) + .await?; let Some((mut bucket_entry, bucket_etag)) = self.read_table_bucket_with_etag_unlocked(table_bucket).await? else { return Err(TableCatalogStoreError::NotFound(format!("table bucket {table_bucket}"))); @@ -1196,12 +1249,17 @@ where &bucket_object, &bucket_entry, TableCatalogPutPrecondition::IfMatch(bucket_etag), + _bucket_guard.write_commit_guards(), ) .await?; Ok(()) } - async fn write_warehouse_index_state_unlocked(&self, table_bucket: &str) -> TableCatalogStoreResult<()> { + async fn write_warehouse_index_state_unlocked( + &self, + table_bucket: &str, + write_guards: Vec, + ) -> TableCatalogStoreResult<()> { let state = TableWarehouseIndexStateEntry { version: TABLE_WAREHOUSE_INDEX_STATE_VERSION, table_bucket: table_bucket.to_string(), @@ -1212,6 +1270,7 @@ where &self.paths.warehouse_index_state_path(table_bucket), &state, TableCatalogPutPrecondition::Any, + write_guards, ) .await } @@ -1324,6 +1383,7 @@ where object, replacement, TableCatalogPutPrecondition::IfMatch(current_etag), + _guard.write_commit_guards(), ) .await?; Ok(true) @@ -1606,7 +1666,13 @@ where .await? else { return self - .write_entry_unlocked(self.catalog_bucket(), &object, &tombstone, TableCatalogPutPrecondition::IfAbsent) + .write_entry_unlocked( + self.catalog_bucket(), + &object, + &tombstone, + TableCatalogPutPrecondition::IfAbsent, + _guard.write_commit_guards(), + ) .await; }; validate_table_warehouse_index_entry_object(&self.paths, &object, ¤t)?; @@ -1626,6 +1692,7 @@ where &object, &tombstone, TableCatalogPutPrecondition::IfMatch(current_etag), + _guard.write_commit_guards(), ) .await } @@ -1927,7 +1994,8 @@ where ) .await?; } - self.write_warehouse_index_state_unlocked(table_bucket).await + self.write_warehouse_index_state_unlocked(table_bucket, _guard.write_commit_guards()) + .await } async fn require_table_bucket(&self, table_bucket: &str) -> TableCatalogStoreResult<()> { @@ -2178,7 +2246,13 @@ where )); } let result = self - .write_entry_unlocked(self.catalog_bucket(), &table_path, &entry, precondition) + .write_entry_unlocked( + self.catalog_bucket(), + &table_path, + &entry, + precondition, + _table_guard.write_commit_guards(), + ) .await; if result.is_err() { self.rollback_table_warehouse_index_reservation(&entry, reservation, "table entry write failed") @@ -2244,7 +2318,7 @@ where "view creation publication fence was lost before catalog update".to_string(), )); } - self.write_entry_unlocked(self.catalog_bucket(), &view_path, &entry, precondition) + self.write_entry_unlocked(self.catalog_bucket(), &view_path, &entry, precondition, _view_guard.write_commit_guards()) .await } @@ -4799,8 +4873,14 @@ where } entry.updated_at = Some(next_table_catalog_update_time(current.updated_at.as_deref())); } - self.write_entry_unlocked(self.catalog_bucket(), &object, &entry, TableCatalogPutPrecondition::Any) - .await + self.write_entry_unlocked( + self.catalog_bucket(), + &object, + &entry, + TableCatalogPutPrecondition::Any, + _guard.write_commit_guards(), + ) + .await } async fn create_namespace(&self, entry: NamespaceEntry) -> TableCatalogStoreResult<()> { @@ -4849,8 +4929,14 @@ where TableCatalogPutPrecondition::IfAbsent } }; - self.write_entry_unlocked(self.catalog_bucket(), &object, &entry, precondition) - .await + self.write_entry_unlocked( + self.catalog_bucket(), + &object, + &entry, + precondition, + _namespace_guard.write_commit_guards(), + ) + .await } async fn list_namespaces(&self, table_bucket: &str) -> TableCatalogStoreResult> { @@ -4954,8 +5040,14 @@ where if before == next { return Ok(result); } - self.write_entry_unlocked(self.catalog_bucket(), &namespace_path, &next, precondition) - .await?; + self.write_entry_unlocked( + self.catalog_bucket(), + &namespace_path, + &next, + precondition, + _namespace_guard.write_commit_guards(), + ) + .await?; Ok(result) } @@ -5313,6 +5405,7 @@ where &index_object, &source_index, TableCatalogPutPrecondition::IfAbsent, + _index_guard.write_commit_guards(), ) .await? } @@ -5338,6 +5431,7 @@ where &intent_object, &intent, TableCatalogPutPrecondition::IfAbsent, + _intent_guard.write_commit_guards(), ) .await?; bucket_entry.active_rename_id = Some(rename_id); @@ -5347,6 +5441,7 @@ where &bucket_object, &bucket_entry, TableCatalogPutPrecondition::IfMatch(bucket_etag), + _bucket_guard.write_commit_guards(), ) .await?; } @@ -5796,6 +5891,7 @@ where &table_path, &next, TableCatalogPutPrecondition::IfMatch(current_etag), + _guard.write_commit_guards(), ) .await; record_table_commit_cas_result(&request.operation, cas_started, &cas_result); @@ -6091,6 +6187,7 @@ where &view_path, &next, TableCatalogPutPrecondition::IfMatch(current_etag), + _guard.write_commit_guards(), ) .await; if let Err(err) = write_result {