diff --git a/crates/ecstore/src/api/mod.rs b/crates/ecstore/src/api/mod.rs index 72c8669f5..758fae75a 100644 --- a/crates/ecstore/src/api/mod.rs +++ b/crates/ecstore/src/api/mod.rs @@ -99,7 +99,7 @@ pub mod bucket { get_lifecycle_config, get_logging_config, get_notification_config, get_object_lock_config, get_public_access_block_config, get_quota_config, get_replication_config, get_request_payment_config, get_sse_config, get_tagging_config, get_versioning_config, get_website_config, init_bucket_metadata_sys, list_bucket_targets, - set_bucket_metadata, update, + remove_bucket_metadata, set_bucket_metadata, update, }; } diff --git a/crates/ecstore/src/bucket/metadata_sys.rs b/crates/ecstore/src/bucket/metadata_sys.rs index f6e7df9d2..aba5028f3 100644 --- a/crates/ecstore/src/bucket/metadata_sys.rs +++ b/crates/ecstore/src/bucket/metadata_sys.rs @@ -74,6 +74,17 @@ pub async fn set_bucket_metadata(bucket: String, bm: BucketMetadata) -> Result<( Ok(()) } +/// Drop a bucket's cached metadata from the in-memory map. +/// +/// This is the counterpart to [`set_bucket_metadata`] and is invoked when a +/// bucket is deleted so peers stop serving stale cached configuration for it. +/// Returns `true` if an entry was present. +pub async fn remove_bucket_metadata(bucket: &str) -> Result { + let sys = get_bucket_metadata_sys()?; + let lock = sys.read().await; + Ok(lock.remove(bucket).await) +} + pub async fn get(bucket: &str) -> Result> { let sys = get_bucket_metadata_sys()?; let lock = sys.read().await; @@ -360,6 +371,17 @@ impl BucketMetadataSys { } } + /// Remove a bucket's cached metadata from the in-memory map. + /// + /// Returns `true` if an entry was present. Reserved meta buckets are ignored. + pub async fn remove(&self, bucket: &str) -> bool { + if is_meta_bucketname(bucket) { + return false; + } + let mut map = self.metadata_map.write().await; + map.remove(bucket).is_some() + } + async fn _reset(&mut self) { let mut map = self.metadata_map.write().await; map.clear(); diff --git a/rustfs/src/admin/handlers/heal.rs b/rustfs/src/admin/handlers/heal.rs index cfefbe0dc..e1013ad0a 100644 --- a/rustfs/src/admin/handlers/heal.rs +++ b/rustfs/src/admin/handlers/heal.rs @@ -791,7 +791,6 @@ mod tests { use serde_json::json; use time::{OffsetDateTime, format_description::well_known::Rfc3339}; use tokio::sync::mpsc; - use tracing::debug; #[test] fn test_heal_opts_serialization() { diff --git a/rustfs/src/app/bucket_usecase.rs b/rustfs/src/app/bucket_usecase.rs index 9ead490bb..a4fa3b78e 100644 --- a/rustfs/src/app/bucket_usecase.rs +++ b/rustfs/src/app/bucket_usecase.rs @@ -452,6 +452,29 @@ fn notify_bucket_metadata_reload( }); } +/// Notify peers to drop their cached metadata for a bucket that was just deleted, +/// so they stop serving stale bucket configuration. Runs in the background to +/// avoid blocking the delete response. +fn notify_bucket_metadata_delete(bucket: String, request_context: Option) { + spawn_background_with_context(request_context, async move { + if let Some(notification_sys) = current_notification_system() { + for peer_err in notification_sys + .delete_bucket_metadata(&bucket) + .await + .into_iter() + .filter(|e| e.err.is_some()) + { + warn!( + bucket = %bucket, + host = %peer_err.host, + error = ?peer_err.err, + "failed to notify peer to delete bucket metadata" + ); + } + } + }); +} + fn validate_replication_config_targets(targets: &BucketTargets, config: &ReplicationConfiguration) -> S3Result<()> { let configured_arns = targets .targets @@ -1133,6 +1156,10 @@ impl DefaultBucketUsecase { warn!(bucket = %input.bucket, error = ?err, "site replication delete bucket hook failed"); } + // Notify peers to drop their cached metadata for the now-deleted bucket. + let request_context = req.extensions.get::().cloned(); + notify_bucket_metadata_delete(input.bucket.clone(), request_context); + let result = Ok(S3Response::new(DeleteBucketOutput {})); let _ = helper.complete(&result); result diff --git a/rustfs/src/storage/rpc/node_service.rs b/rustfs/src/storage/rpc/node_service.rs index 7c8309583..d7ed95fff 100644 --- a/rustfs/src/storage/rpc/node_service.rs +++ b/rustfs/src/storage/rpc/node_service.rs @@ -2601,18 +2601,18 @@ mod tests { } #[tokio::test] - async fn test_delete_bucket_metadata() { + async fn test_delete_bucket_metadata_empty_bucket() { let service = create_test_node_service(); - let request = Request::new(DeleteBucketMetadataRequest { - bucket: "test-bucket".to_string(), - }); + let request = Request::new(DeleteBucketMetadataRequest { bucket: String::new() }); let response = service.delete_bucket_metadata(request).await; assert!(response.is_ok()); + // An empty bucket name is rejected before touching the metadata system. let delete_response = response.unwrap().into_inner(); - assert!(delete_response.success); // Currently returns success (todo implementation) + assert!(!delete_response.success); + assert!(delete_response.error_info.unwrap().contains("bucket name is missing")); } #[tokio::test] diff --git a/rustfs/src/storage/rpc/node_service/bucket.rs b/rustfs/src/storage/rpc/node_service/bucket.rs index d0161c9ca..68549eec7 100644 --- a/rustfs/src/storage/rpc/node_service/bucket.rs +++ b/rustfs/src/storage/rpc/node_service/bucket.rs @@ -17,7 +17,7 @@ use crate::storage::storage_api::rpc_consumer::node_service::contract::bucket::{ BucketOptions, DeleteBucketOptions, MakeBucketOptions, }; use crate::storage::storage_api::rpc_consumer::node_service::{ - DiskError, StoragePeerS3ClientExt as _, load_bucket_metadata, set_bucket_metadata, + DiskError, StoragePeerS3ClientExt as _, load_bucket_metadata, remove_bucket_metadata, set_bucket_metadata, }; use rustfs_common::heal_channel::HealOpts; use rustfs_protos::proto_gen::node_service::*; @@ -30,13 +30,26 @@ impl NodeService { request: Request, ) -> Result, Status> { let request = request.into_inner(); - let _bucket = request.bucket; + let bucket = request.bucket; + if bucket.is_empty() { + return Ok(Response::new(DeleteBucketMetadataResponse { + success: false, + error_info: Some("bucket name is missing".to_string()), + })); + } - //todo - Ok(Response::new(DeleteBucketMetadataResponse { - success: true, - error_info: None, - })) + // Drop the bucket's cached metadata on this peer so it stops serving + // stale configuration after the bucket has been deleted. + match remove_bucket_metadata(&bucket).await { + Ok(_) => Ok(Response::new(DeleteBucketMetadataResponse { + success: true, + error_info: None, + })), + Err(err) => Ok(Response::new(DeleteBucketMetadataResponse { + success: false, + error_info: Some(err.to_string()), + })), + } } pub(super) async fn handle_load_bucket_metadata( diff --git a/rustfs/src/storage/storage_api.rs b/rustfs/src/storage/storage_api.rs index 57cccd7e5..a4f1ce103 100644 --- a/rustfs/src/storage/storage_api.rs +++ b/rustfs/src/storage/storage_api.rs @@ -197,8 +197,8 @@ pub(crate) mod rpc_consumer { ECStore, Error, FileInfoVersions, LocalPeerS3Client, MetricType, PEER_RESTSIGNAL, PEER_RESTSUB_SYS, ReadMultipleReq, ReadMultipleResp, ReadOptions, SERVICE_SIGNAL_REFRESH_CONFIG, SERVICE_SIGNAL_RELOAD_DYNAMIC, StorageDiskRpcExt, StoragePeerS3ClientExt, UpdateMetadataOpts, all_local_disk_path, collect_local_metrics, find_local_disk_by_ref, - get_local_server_property, load_bucket_metadata, reload_transition_tier_config, set_bucket_metadata, - validate_batch_read_version_item_count, + get_local_server_property, load_bucket_metadata, reload_transition_tier_config, remove_bucket_metadata, + set_bucket_metadata, validate_batch_read_version_item_count, }; pub(crate) type StorageResult = super::super::Result; @@ -1170,6 +1170,10 @@ pub(crate) async fn set_bucket_metadata(bucket: String, bm: BucketMetadata) -> R ecstore_bucket::metadata_sys::set_bucket_metadata(bucket, bm).await } +pub(crate) async fn remove_bucket_metadata(bucket: &str) -> Result { + ecstore_bucket::metadata_sys::remove_bucket_metadata(bucket).await +} + pub(crate) async fn update_bucket_metadata_config( bucket: &str, config_file: &str,