fix(replication): close IAM snapshot, marker purge and broadcast gaps (#7195)

crates/replication/src/delete.rs

@@ -76,6 +76,21 @@ impl ReplicationWorkerOperation for DeletedObjectReplicationInfo {
                 .delete_object
                 .delete_marker_mtime
                 .and_then(|t| i64::try_from(t.unix_timestamp_nanos()).ok()),
+            // Carry the target-assigned marker version ids (and the fail-closed corrupt
+            // flag) into the journal so a purge intent replayed after a restart addresses
+            // the same version the live path did (backlog#2290). Only delete-marker state
+            // ever records these; other deletes serialize an empty map.
+            target_delete_marker_version_ids: self
+                .delete_object
+                .replication_state
+                .as_ref()
+                .map(|state| state.target_delete_marker_version_ids.clone())
+                .unwrap_or_default(),
+            target_delete_marker_version_ids_corrupt: self
+                .delete_object
+                .replication_state
+                .as_ref()
+                .is_some_and(|state| state.target_delete_marker_version_ids_corrupt),
             target_arns: self.admitted_target_arns(),
             force_delete_id: self.delete_object.force_delete_id,
             force_delete_generation: self.delete_object.force_delete_generation,
@@ -238,6 +253,28 @@ pub fn delete_marker_purge_version_id(
     })
 }
 
+/// The version a delete replication addresses on `arn`, or `None` to refuse.
+///
+/// A version purge whose purged version is a delete marker must address the
+/// marker version the TARGET assigned — the recorded mapping, exactly as the
+/// delayed-purge watcher does. The source-side `DELETE ?versionId=<marker>`
+/// replicates as such a purge, and a generic S3 target answers a DELETE of an
+/// unknown versionId with 204 while keeping its marker, so addressing it by
+/// the source id reported success and left the marker behind (backlog#2290,
+/// R6.1 on the VMs). Nothing recorded falls back to the source-derived id
+/// (id-mirroring peers); a corrupt record refuses, as the watcher does.
+pub fn delete_replication_target_version_id(dobj: &DeletedObject, arn: &str) -> Option<Option<String>> {
+    let is_version_purge = is_version_delete_replication(dobj);
+    if is_version_purge
+        && !dobj.delete_marker
+        && let Some(marker) = dobj.delete_marker_version_id
+    {
+        return delete_marker_purge_version_id(dobj.replication_state.as_ref(), arn, marker);
+    }
+    let source_version = dobj.delete_marker_version_id.or(dobj.version_id).unwrap_or_default();
+    Some(target_delete_version_id(source_version, is_version_purge))
+}
+
 /// Shape an exhausted purge intent as a marker-creation delete entry. Replay
 /// reconstructs it with `delete_marker: true`, finds the source marker gone,
 /// and funnels into the stale-marker branch of `replicate_delete_with_outcome`
@@ -258,9 +295,9 @@ mod tests {
 
     use super::{
         DeletedObjectReplicationInfo, delete_marker_purge_mrf_entry, delete_marker_purge_version_id,
-        delete_replication_creates_marker, is_object_lock_denied_delete, 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, delete_replication_target_version_id, is_object_lock_denied_delete,
+        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::{
@@ -595,6 +632,76 @@ mod tests {
         assert_eq!(entry.retry_count, 0);
         assert_eq!(entry.bucket, "bucket-a");
         assert_eq!(entry.object, "doc.txt");
+        assert!(
+            entry.target_delete_marker_version_ids.is_empty(),
+            "no recorded target marker ids means the journal carries none"
+        );
+        assert!(!entry.target_delete_marker_version_ids_corrupt);
+    }
+
+    /// backlog#2290: a purge intent journaled to MRF must carry the marker
+    /// version ids the targets assigned, plus the fail-closed corrupt flag,
+    /// so a replay after restart addresses the same version the live path did.
+    #[test]
+    fn delete_marker_purge_mrf_entry_carries_target_assigned_marker_versions() {
+        let delete_marker_version_id = Uuid::new_v4();
+        let mut state = ReplicationState::default();
+        state
+            .target_delete_marker_version_ids
+            .insert("arn:a".to_string(), "remote-marker-a".to_string());
+        state
+            .target_delete_marker_version_ids
+            .insert("arn:b".to_string(), "remote-marker-b".to_string());
+        let mut dobj = DeletedObjectReplicationInfo {
+            delete_object: DeletedObject {
+                object_name: "doc.txt".to_string(),
+                delete_marker: false,
+                version_id: Some(Uuid::new_v4()),
+                delete_marker_version_id: Some(delete_marker_version_id),
+                replication_state: Some(state),
+                ..Default::default()
+            },
+            bucket: "bucket-a".to_string(),
+            ..Default::default()
+        };
+
+        let entry = delete_marker_purge_mrf_entry(&dobj, vec!["arn:a".to_string()]);
+        assert_eq!(
+            entry.target_delete_marker_version_ids,
+            HashMap::from([
+                ("arn:a".to_string(), "remote-marker-a".to_string()),
+                ("arn:b".to_string(), "remote-marker-b".to_string()),
+            ]),
+            "every recorded target marker id survives the journal, regardless of the retried ARN subset"
+        );
+        assert!(!entry.target_delete_marker_version_ids_corrupt);
+        assert_eq!(
+            delete_marker_purge_version_id(
+                Some(&ReplicationState {
+                    target_delete_marker_version_ids: entry.target_delete_marker_version_ids,
+                    ..Default::default()
+                }),
+                "arn:a",
+                delete_marker_version_id
+            ),
+            Some(Some("remote-marker-a".to_string()))
+        );
+
+        // The live path refuses to purge on inconsistent metadata and reports the target
+        // as failed; the journaled intent must keep refusing after a restart.
+        dobj.delete_object
+            .replication_state
+            .as_mut()
+            .expect("state was set above")
+            .target_delete_marker_version_ids_corrupt = true;
+        let entry = delete_marker_purge_mrf_entry(&dobj, vec!["arn:a".to_string()]);
+        assert!(entry.target_delete_marker_version_ids_corrupt);
+
+        // A delete without replication state journals an empty map.
+        dobj.delete_object.replication_state = None;
+        let entry = dobj.to_mrf_entry();
+        assert!(entry.target_delete_marker_version_ids.is_empty());
+        assert!(!entry.target_delete_marker_version_ids_corrupt);
     }
 
     #[test]
@@ -656,4 +763,57 @@ mod tests {
         assert!(!is_object_lock_denied_delete(Some("InternalError"), Some("retention lookup failed")));
         assert!(!is_object_lock_denied_delete(None, Some("legal hold")));
     }
+
+    fn purge_of_marker(marker: Uuid, state: Option<ReplicationState>) -> DeletedObject {
+        DeletedObject {
+            object_name: "obj".to_string(),
+            delete_marker: false,
+            delete_marker_version_id: Some(marker),
+            version_id: None,
+            replication_state: state,
+            ..Default::default()
+        }
+    }
+
+    #[test]
+    fn delete_replication_target_version_id_addresses_recorded_marker_for_purges() {
+        let arn = "arn:minio:replication::generic:photos";
+        let marker = Uuid::new_v4();
+        let mut state = ReplicationState::default();
+        state
+            .target_delete_marker_version_ids
+            .insert(arn.to_string(), "remote-marker".to_string());
+
+        // purge of a replicated marker: the target's own version
+        assert_eq!(
+            delete_replication_target_version_id(&purge_of_marker(marker, Some(state.clone())), arn),
+            Some(Some("remote-marker".to_string()))
+        );
+        // nothing recorded for this arn: the source-derived id (id-mirroring peers)
+        assert_eq!(
+            delete_replication_target_version_id(&purge_of_marker(marker, None), arn),
+            Some(Some(marker.to_string()))
+        );
+        // corrupt record: refuse instead of guessing
+        state.target_delete_marker_version_ids_corrupt = true;
+        assert_eq!(delete_replication_target_version_id(&purge_of_marker(marker, Some(state)), arn), None);
+
+        // marker creation keeps the source id (the target mints its own on a
+        // versionless DELETE; the id only travels in the source header)
+        let creation = DeletedObject {
+            object_name: "obj".to_string(),
+            delete_marker: true,
+            delete_marker_version_id: Some(marker),
+            ..Default::default()
+        };
+        assert_eq!(delete_replication_target_version_id(&creation, arn), Some(Some(marker.to_string())));
+        // plain version purge: the source version id
+        let version = Uuid::new_v4();
+        let purge = DeletedObject {
+            object_name: "obj".to_string(),
+            version_id: Some(version),
+            ..Default::default()
+        };
+        assert_eq!(delete_replication_target_version_id(&purge, arn), Some(Some(version.to_string())));
+    }
 }

crates/replication/src/filemeta.rs

@@ -641,6 +641,26 @@ pub struct MrfReplicateEntry {
     #[serde(rename = "deleteMarkerMtime", skip_serializing_if = "Option::is_none", default)]
     pub delete_marker_mtime: Option<i64>,
 
+    // For delete-marker purge intents: the exact version id each target assigned to the
+    // replicated marker, keyed by target ARN. A generic S3 target mints its own version ids
+    // and answers a DELETE of an unknown id with 204, so a replay that fell back to the source
+    // marker id would be acknowledged while the real marker stayed behind (backlog#2290).
+    // Old files lack this key; default=empty means "unknown" and replay keeps the source-id
+    // fallback it always had.
+    #[serde(rename = "targetDeleteMarkerVersionIDs", skip_serializing_if = "HashMap::is_empty", default)]
+    pub target_delete_marker_version_ids: HashMap<String, String>,
+
+    // Companion to the map above: the source metadata disagreed about the recorded ids when
+    // the intent was journaled, so the live path refused to guess and reported the target as
+    // failed. Replay must keep refusing instead of falling back to the source id. Old files
+    // lack this key; default=false.
+    #[serde(
+        rename = "targetDeleteMarkerVersionIDsCorrupt",
+        skip_serializing_if = "std::ops::Not::not",
+        default
+    )]
+    pub target_delete_marker_version_ids_corrupt: bool,
+
     #[serde(rename = "targetARNs", skip_serializing_if = "Vec::is_empty", default)]
     pub target_arns: Vec<String>,
 

crates/replication/src/lib.rs

@@ -41,9 +41,9 @@ pub use config::{
 };
 pub use delete::{
     DeletedObjectReplicationInfo, delete_marker_purge_mrf_entry, delete_marker_purge_version_id,
-    delete_replication_creates_marker, is_object_lock_denied_delete, 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, delete_replication_target_version_id, is_object_lock_denied_delete,
+    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,

crates/replication/src/mrf.rs

@@ -31,8 +31,13 @@ const CAPABILITY_OPERATION_KIND: u64 = 1 << 0;
 const CAPABILITY_TARGET_ARNS: u64 = 1 << 1;
 const CAPABILITY_FORCE_DELETE: u64 = 1 << 2;
 const CAPABILITY_DELETE_MARKER_MTIME: u64 = 1 << 3;
-const MRF_KNOWN_CAPABILITIES: u64 =
-    CAPABILITY_OPERATION_KIND | CAPABILITY_TARGET_ARNS | CAPABILITY_FORCE_DELETE | CAPABILITY_DELETE_MARKER_MTIME;
+// Per-ARN target-assigned delete-marker version ids on purge intents (backlog#2290).
+const CAPABILITY_TARGET_DELETE_MARKER_VERSION_IDS: u64 = 1 << 4;
+const MRF_KNOWN_CAPABILITIES: u64 = CAPABILITY_OPERATION_KIND
+    | CAPABILITY_TARGET_ARNS
+    | CAPABILITY_FORCE_DELETE
+    | CAPABILITY_DELETE_MARKER_MTIME
+    | CAPABILITY_TARGET_DELETE_MARKER_VERSION_IDS;
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum MrfCapability {
@@ -40,6 +45,7 @@ pub enum MrfCapability {
     TargetArns,
     ForceDelete,
     DeleteMarkerMtime,
+    TargetDeleteMarkerVersionIds,
 }
 
 impl MrfCapability {
@@ -49,6 +55,7 @@ impl MrfCapability {
             Self::TargetArns => CAPABILITY_TARGET_ARNS,
             Self::ForceDelete => CAPABILITY_FORCE_DELETE,
             Self::DeleteMarkerMtime => CAPABILITY_DELETE_MARKER_MTIME,
+            Self::TargetDeleteMarkerVersionIds => CAPABILITY_TARGET_DELETE_MARKER_VERSION_IDS,
         }
     }
 }
@@ -601,9 +608,17 @@ pub fn decode_mrf_file(data: &[u8]) -> Result<Vec<MrfReplicateEntry>> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use std::collections::HashMap;
     use uuid::Uuid;
 
+    // Capability word 31 = OperationKind | TargetArns | ForceDelete | DeleteMarkerMtime |
+    // TargetDeleteMarkerVersionIds (backlog#2290).
     const ENVELOPE_FIXTURE: &[u8] = &[
+        b'M', b'R', b'F', b'E', 1, 0, 1, 0, 1, 0, 0, 0, 31, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 1, 2, 3,
+    ];
+
+    // The envelope a binary from before backlog#2290 writes: same header, capability word 15.
+    const PRE_TARGET_MARKER_IDS_ENVELOPE_FIXTURE: &[u8] = &[
         b'M', b'R', b'F', b'E', 1, 0, 1, 0, 1, 0, 0, 0, 15, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 1, 2, 3,
     ];
 
@@ -626,6 +641,8 @@ mod tests {
                 delete_marker_version_id: None,
                 delete_marker: false,
                 delete_marker_mtime: None,
+                target_delete_marker_version_ids: HashMap::new(),
+                target_delete_marker_version_ids_corrupt: false,
                 target_arns: vec!["arn:target-a".to_string()],
             },
             MrfReplicateEntry {
@@ -642,6 +659,8 @@ mod tests {
                 delete_marker_version_id: None,
                 delete_marker: false,
                 delete_marker_mtime: None,
+                target_delete_marker_version_ids: HashMap::new(),
+                target_delete_marker_version_ids_corrupt: false,
                 target_arns: vec!["arn:target-a".to_string(), "arn:target-b".to_string()],
             },
             MrfReplicateEntry {
@@ -658,6 +677,11 @@ mod tests {
                 delete_marker_version_id: Some(del_vid),
                 delete_marker: true,
                 delete_marker_mtime: Some(1_705_312_200_123_456_789),
+                target_delete_marker_version_ids: HashMap::from([
+                    ("arn:target-a".to_string(), "remote-marker-a".to_string()),
+                    ("arn:target-b".to_string(), "remote-marker-b".to_string()),
+                ]),
+                target_delete_marker_version_ids_corrupt: false,
                 target_arns: vec!["arn:target-a".to_string()],
             },
         ];
@@ -685,6 +709,54 @@ mod tests {
             Some(1_705_312_200_123_456_789),
             "delete-marker mtime must survive the MRF disk round-trip"
         );
+        assert!(decoded[0].target_delete_marker_version_ids.is_empty());
+        assert!(decoded[1].target_delete_marker_version_ids.is_empty());
+        assert_eq!(
+            decoded[2].target_delete_marker_version_ids,
+            HashMap::from([
+                ("arn:target-a".to_string(), "remote-marker-a".to_string()),
+                ("arn:target-b".to_string(), "remote-marker-b".to_string()),
+            ]),
+            "target-assigned marker version ids must survive the MRF disk round-trip (backlog#2290)"
+        );
+        assert!(!decoded[2].target_delete_marker_version_ids_corrupt);
+    }
+
+    /// backlog#2290: the corrupt flag rides the same journal round trip, and an
+    /// entry that carries neither field encodes exactly as it did before the
+    /// field existed (both keys are skipped when empty/false).
+    #[test]
+    fn mrf_file_round_trips_target_marker_ids_corrupt_flag_and_skips_empty_keys() {
+        let corrupt = MrfReplicateEntry {
+            bucket: "bucket-a".to_string(),
+            object: "delete-a".to_string(),
+            op: MrfOpKind::Delete,
+            delete_marker: true,
+            delete_marker_version_id: Some(Uuid::new_v4()),
+            target_delete_marker_version_ids_corrupt: true,
+            target_arns: vec!["arn:target-a".to_string()],
+            ..Default::default()
+        };
+        let decoded = decode_mrf_file(&encode_mrf_file(std::slice::from_ref(&corrupt)).expect("mrf file should encode"))
+            .expect("mrf file should decode");
+        assert_eq!(decoded, vec![corrupt]);
+        assert!(decoded[0].target_delete_marker_version_ids_corrupt);
+
+        let plain = MrfReplicateEntry {
+            bucket: "bucket-a".to_string(),
+            object: "delete-a".to_string(),
+            op: MrfOpKind::Delete,
+            delete_marker: true,
+            target_arns: vec!["arn:target-a".to_string()],
+            ..Default::default()
+        };
+        let encoded = encode_mrf_file(std::slice::from_ref(&plain)).expect("mrf file should encode");
+        let payload = String::from_utf8_lossy(&encoded);
+        assert!(
+            !payload.contains("targetDeleteMarkerVersionIDs"),
+            "an entry without recorded ids must not grow the new keys: {payload}"
+        );
+        assert_eq!(decode_mrf_file(&encoded).expect("mrf file should decode"), vec![plain]);
     }
 
     #[test]
@@ -719,6 +791,99 @@ mod tests {
         // Old files lack the deleteMarkerMtime key; it must default to None so replay keeps the
         // pre-#867 fallback to the current time.
         assert_eq!(decoded[0].delete_marker_mtime, None);
+        // Old files also lack the target marker id keys; they must default to an empty map
+        // and a clear corrupt flag so replay keeps the pre-#2290 source-id fallback.
+        assert!(decoded[0].target_delete_marker_version_ids.is_empty());
+        assert!(!decoded[0].target_delete_marker_version_ids_corrupt);
+    }
+
+    /// backlog#2290: a delete-marker entry written by a binary that predates the
+    /// `targetDeleteMarkerVersionIDs` key decodes with an empty map and a clear
+    /// corrupt flag — the exact shape replay handled before the field existed.
+    #[test]
+    fn mrf_pre_target_marker_ids_delete_entry_decodes_with_empty_map() {
+        let marker_version_id = Uuid::new_v4();
+        let mut payload = Vec::new();
+        rmp::encode::write_array_len(&mut payload, 1).expect("array len should encode");
+        rmp::encode::write_map_len(&mut payload, 9).expect("map len should encode");
+        rmp::encode::write_str(&mut payload, "bucket").expect("bucket key should encode");
+        rmp::encode::write_str(&mut payload, "old-bucket").expect("bucket value should encode");
+        rmp::encode::write_str(&mut payload, "object").expect("object key should encode");
+        rmp::encode::write_str(&mut payload, "old-key").expect("object value should encode");
+        rmp::encode::write_str(&mut payload, "retryCount").expect("retry key should encode");
+        rmp::encode::write_i32(&mut payload, 0).expect("retry value should encode");
+        rmp::encode::write_str(&mut payload, "size").expect("size key should encode");
+        rmp::encode::write_i64(&mut payload, 0).expect("size value should encode");
+        rmp::encode::write_str(&mut payload, "op").expect("op key should encode");
+        rmp::encode::write_str(&mut payload, "delete").expect("op value should encode");
+        rmp::encode::write_str(&mut payload, "forceDelete").expect("forceDelete key should encode");
+        rmp::encode::write_bool(&mut payload, false).expect("forceDelete value should encode");
+        rmp::encode::write_str(&mut payload, "deleteMarkerVersionID").expect("marker id key should encode");
+        // Uuid serializes as a 16-byte bin in the MessagePack journal.
+        rmp::encode::write_bin(&mut payload, marker_version_id.as_bytes()).expect("marker id value should encode");
+        rmp::encode::write_str(&mut payload, "deleteMarker").expect("deleteMarker key should encode");
+        rmp::encode::write_bool(&mut payload, true).expect("deleteMarker value should encode");
+        rmp::encode::write_str(&mut payload, "targetARNs").expect("targetARNs key should encode");
+        rmp::encode::write_array_len(&mut payload, 1).expect("targetARNs len should encode");
+        rmp::encode::write_str(&mut payload, "arn:target-a").expect("targetARNs value should encode");
+
+        let mut data = Vec::with_capacity(4 + payload.len());
+        data.extend_from_slice(&MRF_META_FORMAT.to_le_bytes());
+        data.extend_from_slice(&MRF_META_VERSION.to_le_bytes());
+        data.extend_from_slice(&payload);
+
+        let decoded = decode_mrf_file(&data).expect("pre-#2290 delete-marker entry should decode");
+
+        assert_eq!(decoded.len(), 1);
+        assert_eq!(decoded[0].op, MrfOpKind::Delete);
+        assert!(decoded[0].delete_marker);
+        assert_eq!(decoded[0].delete_marker_version_id, Some(marker_version_id));
+        assert_eq!(decoded[0].target_arns, vec!["arn:target-a".to_string()]);
+        assert!(decoded[0].target_delete_marker_version_ids.is_empty());
+        assert!(!decoded[0].target_delete_marker_version_ids_corrupt);
+    }
+
+    /// backlog#2290: the new field is fenced by its own capability bit exactly
+    /// like the earlier optional fields — a reader without the bit refuses an
+    /// envelope that advertises it, while the current reader still accepts the
+    /// pre-#2290 envelope.
+    #[test]
+    fn envelope_target_marker_ids_capability_is_fenced_and_backward_compatible() {
+        assert!(MrfCapabilities::current().contains(MrfCapability::TargetDeleteMarkerVersionIds));
+        assert_eq!(MrfCapabilities::with(MrfCapability::TargetDeleteMarkerVersionIds).bits(), 1 << 4);
+
+        // Old envelope, current reader: accepted, and the negotiated set lacks the new bit.
+        let legacy = MrfEnvelope::decode(PRE_TARGET_MARKER_IDS_ENVELOPE_FIXTURE, MrfProtocolCapabilities::current())
+            .expect("pre-#2290 envelope should decode");
+        assert_eq!(legacy.protocol().capabilities().bits(), 15);
+        assert!(
+            !legacy
+                .protocol()
+                .capabilities()
+                .contains(MrfCapability::TargetDeleteMarkerVersionIds)
+        );
+        assert_eq!(legacy.payload(), &[1, 2, 3]);
+
+        // Current envelope, reader that only knows the pre-#2290 bits: refused.
+        let pre_2290_reader = MrfProtocolCapabilities::new(1, 1, MrfCapabilities::from_bits(15).expect("known bits"));
+        assert_eq!(
+            MrfEnvelope::decode(ENVELOPE_FIXTURE, pre_2290_reader),
+            Err(MrfEnvelopeError::MissingCapabilities {
+                required: 31,
+                available: 15,
+            })
+        );
+
+        // Negotiation with such a peer drops the bit instead of failing.
+        let negotiated = MrfProtocolCapabilities::current()
+            .negotiate(pre_2290_reader)
+            .expect("negotiation with a pre-#2290 peer should succeed");
+        assert!(
+            !negotiated
+                .capabilities()
+                .contains(MrfCapability::TargetDeleteMarkerVersionIds)
+        );
+        assert!(negotiated.capabilities().contains(MrfCapability::DeleteMarkerMtime));
     }
 
     #[test]