From ab84c3f5cf9dfd2a0a659b8af048e351da06a7d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=94=90=E5=B0=8F=E9=B8=AD?= Date: Sat, 29 Aug 2026 15:49:50 +0800 Subject: [PATCH] fix(replication): keep versionId on version-purge delete replication (#6841) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix(replication): never mint delete markers when replicating a version purge Heal/resync/MRF rebuilds of a delete-marker version purge carry delete_marker: true together with a purge-shaped entry. Passing that flag straight into replication_delete_remove_options made the target DELETE omit the versionId (marker-creation semantics), so a generic S3 target that ignores the internal source-version headers minted a fresh delete marker on every retry instead of purging one — the marker count on the target grew monotonically (rustfs#6823). - Gate marker-creation semantics on the new pure helper delete_replication_creates_marker (delete_marker && !version purge) so a purge always addresses the exact version. - Stop falling through to the marker-creation send when the pre-send source delete-marker verification fails with a transient error; fail the entry instead so the MRF replay / heal scanner retries without minting a marker on the target. - Pin the purge-shape contract with unit tests in crates/replication/src/delete.rs. --- .../replication_object_decision_boundary.rs | 6 +- .../replication/replication_resyncer.rs | 39 ++++++-- crates/replication/src/delete.rs | 97 ++++++++++++++++++- crates/replication/src/lib.rs | 5 +- 4 files changed, 132 insertions(+), 15 deletions(-) diff --git a/crates/ecstore/src/bucket/replication/replication_object_decision_boundary.rs b/crates/ecstore/src/bucket/replication/replication_object_decision_boundary.rs index 4360ed4ef..c84099a30 100644 --- a/crates/ecstore/src/bucket/replication/replication_object_decision_boundary.rs +++ b/crates/ecstore/src/bucket/replication/replication_object_decision_boundary.rs @@ -19,9 +19,9 @@ pub use rustfs_replication::{ }; pub(crate) use rustfs_replication::{ ReplicationDeleteSource, ReplicationMultipartPartInput, ReplicationResyncTargetObject, delete_marker_purge_mrf_entry, - delete_marker_purge_version_id, delete_replication_missing_source_decision, delete_replication_object_opts, - heal_uses_delete_replication_path, is_retryable_delete_replication_head_error, is_version_delete_replication, - replicate_delete_outcome, replication_etags_match, replication_multipart_complete_actual_size, + delete_marker_purge_version_id, delete_replication_creates_marker, delete_replication_missing_source_decision, + delete_replication_object_opts, heal_uses_delete_replication_path, is_retryable_delete_replication_head_error, + is_version_delete_replication, replicate_delete_outcome, replication_etags_match, replication_multipart_complete_actual_size, replication_multipart_part_plan, resync_existing_delete_replication_info, resync_target_for_object, should_retry_delete_marker_purge, target_delete_version_id, }; diff --git a/crates/ecstore/src/bucket/replication/replication_resyncer.rs b/crates/ecstore/src/bucket/replication/replication_resyncer.rs index 2fb6145d3..18fdb4290 100644 --- a/crates/ecstore/src/bucket/replication/replication_resyncer.rs +++ b/crates/ecstore/src/bucket/replication/replication_resyncer.rs @@ -30,8 +30,8 @@ use super::replication_msgp_boundary::ReplicationMsgpCodec; use super::replication_object_config::{ReplicationConfig, get_replication_config, must_replicate}; use super::replication_object_decision_boundary::{ MustReplicateOptions, ReplicationMultipartPartInput, delete_marker_purge_mrf_entry, delete_marker_purge_version_id, - heal_uses_delete_replication_path, is_retryable_delete_replication_head_error, is_version_delete_replication, - replicate_delete_outcome, replication_etags_match, replication_multipart_complete_actual_size, + delete_replication_creates_marker, heal_uses_delete_replication_path, is_retryable_delete_replication_head_error, + is_version_delete_replication, replicate_delete_outcome, replication_etags_match, replication_multipart_complete_actual_size, replication_multipart_part_plan, resync_existing_delete_replication_info, should_retry_delete_marker_purge, target_delete_version_id, }; @@ -1617,7 +1617,6 @@ pub(crate) async fn replicate_delete_with_outcome( } let bucket = dobj.bucket.clone(); - let mut source_state_verified = true; let version_id = if let Some(version_id) = &dobj.delete_object.delete_marker_version_id { Some(version_id.to_owned()) } else { @@ -1675,7 +1674,12 @@ pub(crate) async fn replicate_delete_with_outcome( return purge_stale_delete_marker_targets(&bucket, &dobj).await; } Err(err) => { - source_state_verified = false; + // A transient source error (lock timeout, IO error) must not + // fall through to the marker-creation send below: that DELETE + // omits the versionId, so every such retry lets a generic S3 + // target mint one more delete marker (rustfs#6823). Fail the + // entry without touching the target; the MRF replay / heal + // scanner retries once the source is readable again. debug!( event = EVENT_REPLICATION_DELETE_SKIPPED, component = LOG_COMPONENT_ECSTORE, @@ -1687,6 +1691,20 @@ pub(crate) async fn replicate_delete_with_outcome( reason = "source_state_verification_failed", "Failed to verify source delete-marker state before replication" ); + send_local_event(EventArgs { + event_name: EventName::ObjectReplicationNotTracked.to_string(), + bucket_name: bucket.clone(), + object: ObjectInfo { + bucket: bucket.clone(), + name: dobj.delete_object.object_name.clone(), + version_id, + delete_marker: dobj.delete_object.delete_marker, + ..Default::default() + }, + user_agent: "Internal: [Replication]".to_string(), + ..Default::default() + }); + return false; } } } @@ -2008,7 +2026,9 @@ pub(crate) async fn replicate_delete_with_outcome( expected_targets, rinfos.targets.len(), state_persisted, - source_state_verified, + // Source state is verified by construction here: a verification + // error returns early above instead of replicating unverified. + true, &replication_status, ) } @@ -2637,7 +2657,14 @@ async fn replicate_delete_to_target(dobj: &DeletedObjectReplicationInfo, tgt_cli &tgt_client.bucket, &dobj.delete_object.object_name, version_id.clone(), - replication_delete_remove_options(dobj.delete_object.delete_marker, dobj.delete_object.delete_marker_mtime), + // A version purge must keep the versionId on the DELETE even when + // the purged version is a delete marker: marker-creation semantics + // would drop it and a generic S3 target would mint a fresh marker + // on every retry (rustfs#6823). + replication_delete_remove_options( + delete_replication_creates_marker(&dobj.delete_object), + dobj.delete_object.delete_marker_mtime, + ), ) .await { diff --git a/crates/replication/src/delete.rs b/crates/replication/src/delete.rs index 3b6fbb016..49d305b40 100644 --- a/crates/replication/src/delete.rs +++ b/crates/replication/src/delete.rs @@ -108,6 +108,20 @@ pub fn is_version_delete_replication(dobj: &DeletedObject) -> bool { dobj.version_id.is_some() || (dobj.delete_marker_version_id.is_some() && !dobj.delete_marker) } +/// Whether the target DELETE for this delete replication may use +/// marker-CREATION semantics (`replication_delete_marker`, which omits the +/// `versionId` query so the target mints its own marker). +/// +/// A version purge must never do so: the DELETE has to address the exact +/// version, otherwise a generic S3 target — which ignores the internal +/// source-version headers — mints a fresh delete marker on every retry +/// instead of removing one (rustfs#6823). The `delete_marker` flag alone is +/// not enough because heal/resync rebuilds carry `delete_marker: true` +/// together with a purge-shaped entry. +pub fn delete_replication_creates_marker(dobj: &DeletedObject) -> bool { + dobj.delete_marker && !is_version_delete_replication(dobj) +} + pub fn should_retry_delete_marker_purge(dobj: &DeletedObject) -> bool { dobj.delete_marker_version_id.is_some() } @@ -223,13 +237,14 @@ mod tests { use super::{ DeletedObjectReplicationInfo, delete_marker_purge_mrf_entry, delete_marker_purge_version_id, - is_retryable_delete_replication_head_error, is_version_delete_replication, replicate_delete_outcome, - should_retry_delete_marker_purge, target_delete_version_id, + delete_replication_creates_marker, is_retryable_delete_replication_head_error, is_version_delete_replication, + replicate_delete_outcome, resync_existing_delete_replication_info, should_retry_delete_marker_purge, + target_delete_version_id, }; use crate::storage_api::DeletedObject; use crate::{ - MrfOpKind, NULL_VERSION_ID, ReplicationState, ReplicationStatusType, ReplicationType, ReplicationWorkerOperation, - VersionPurgeStatusType, + MrfOpKind, NULL_VERSION_ID, ReplicateObjectInfo, ReplicationState, ReplicationStatusType, ReplicationType, + ReplicationWorkerOperation, VersionPurgeStatusType, }; use uuid::Uuid; @@ -400,6 +415,80 @@ mod tests { assert!(!is_version_delete_replication(&dobj)); } + /// rustfs#6823 regression guard: a purge-shaped entry must never take + /// marker-creation semantics, which would drop the `versionId` from the + /// target DELETE and let a generic S3 target mint a fresh delete marker + /// on every heal/resync/MRF retry. + #[test] + fn purge_shapes_never_take_marker_creation_semantics() { + // Heal/resync rebuild of a delete-marker version purge: the entry + // carries `delete_marker: true` together with the purged version id. + let heal_purge = DeletedObject { + delete_marker: true, + version_id: Some(Uuid::new_v4()), + ..Default::default() + }; + assert!( + !delete_replication_creates_marker(&heal_purge), + "a version purge must address the version, not mint a marker" + ); + + // Live delete-marker version purge addressed via the marker id. + let marker_purge = DeletedObject { + delete_marker: false, + delete_marker_version_id: Some(Uuid::new_v4()), + ..Default::default() + }; + assert!(!delete_replication_creates_marker(&marker_purge)); + + // Only a plain delete-marker creation may let the target mint one. + let marker_creation = DeletedObject { + delete_marker: true, + delete_marker_version_id: Some(Uuid::new_v4()), + ..Default::default() + }; + assert!(delete_replication_creates_marker(&marker_creation)); + + let versionless = DeletedObject { + delete_marker: false, + ..Default::default() + }; + assert!(!delete_replication_creates_marker(&versionless)); + } + + /// The resync scan rebuilds purge work items with `delete_marker: true` + /// (crates/replication/src/delete.rs `resync_existing_delete_replication_info`); + /// pin that this shape flows into purge — not marker-creation — semantics. + #[test] + fn resync_rebuilt_purge_entry_keeps_versioned_delete_semantics() { + let roi = ReplicateObjectInfo { + bucket: "bucket".to_string(), + name: "object".to_string(), + version_id: Some(Uuid::new_v4()), + delete_marker: true, + version_purge_status: VersionPurgeStatusType::Pending, + ..Default::default() + }; + + let info = resync_existing_delete_replication_info(&roi, "arn:target-a"); + + assert!(is_version_delete_replication(&info.delete_object)); + assert!( + !delete_replication_creates_marker(&info.delete_object), + "a rebuilt purge must not re-mint delete markers on the target" + ); + + // Without a pending purge the rebuild is a marker creation again. + let roi = ReplicateObjectInfo { + delete_marker: true, + version_id: Some(Uuid::new_v4()), + version_purge_status: VersionPurgeStatusType::Empty, + ..roi + }; + let info = resync_existing_delete_replication_info(&roi, "arn:target-a"); + assert!(delete_replication_creates_marker(&info.delete_object)); + } + #[test] fn delete_marker_purge_retry_covers_version_purge_and_marker_creation() { let version_purge = DeletedObject { diff --git a/crates/replication/src/lib.rs b/crates/replication/src/lib.rs index e35280749..4e32b8a3e 100644 --- a/crates/replication/src/lib.rs +++ b/crates/replication/src/lib.rs @@ -40,8 +40,9 @@ pub use config::{ }; pub use delete::{ DeletedObjectReplicationInfo, delete_marker_purge_mrf_entry, delete_marker_purge_version_id, - is_retryable_delete_replication_head_error, is_version_delete_replication, replicate_delete_outcome, - resync_existing_delete_replication_info, should_retry_delete_marker_purge, target_delete_version_id, + delete_replication_creates_marker, is_retryable_delete_replication_head_error, is_version_delete_replication, + replicate_delete_outcome, resync_existing_delete_replication_info, should_retry_delete_marker_purge, + target_delete_version_id, }; pub use filemeta::{ NULL_VERSION_ID, REPLICATE_EXISTING, REPLICATE_EXISTING_DELETE, REPLICATE_HEAL, REPLICATE_HEAL_DELETE, REPLICATE_INCOMING,