fix(replication): persist target delete-marker version ids in MRF purge intents (backlog#2290)

A delete-marker purge intent that outlived its watch window was journaled
without the version ids the targets assigned to the replicated markers.
Replay rebuilt the replication state from a blank ObjectInfo, so
`delete_marker_purge_version_id` fell back to the source marker id; a
generic S3 target that mints its own ids answers that DELETE with 204,
the entry was acknowledged and the real marker stayed on the target.

- `MrfReplicateEntry` gains `targetDeleteMarkerVersionIDs` (per-ARN map)
  and `targetDeleteMarkerVersionIDsCorrupt`; both default and are skipped
  when empty/false, so old journals decode to the pre-existing shape.
- `DeletedObjectReplicationInfo::to_mrf_entry` copies both from the
  source replication state; `reconstructed_heal_delete_info` restores
  them into the replayed state so the purge addresses the recorded id
  and a fail-closed refusal stays a refusal after restart.
- MRF envelope capability bit `TargetDeleteMarkerVersionIds` (1 << 4)
  fences the field like `DeleteMarkerMtime`; readers without the bit
  refuse envelopes that advertise it, current readers accept old ones.

(cherry picked from commit ddacaaa185fda7a5f426138ba5b179f986b862d9)
This commit is contained in:
唐小鸭
2026-09-05 17:10:34 +08:00
parent c5e6b7259e
commit 18670b269b
5 changed files with 371 additions and 2 deletions
@@ -882,6 +882,20 @@ fn reconstructed_heal_delete_info(
) -> DeletedObjectReplicationInfo {
let mut rstate = oi.replication_state();
rstate.replicate_decision_str = dsc.to_string();
// The caller hands us a blank ObjectInfo (the source marker may already be
// gone), so the state above carries no target-assigned marker version ids.
// Restore them from the journal: `delete_marker_purge_version_id` must hit
// the id the target reported, not fall back to the source marker id, which
// a target that mints its own ids answers with an idempotent 204 that would
// acknowledge the intent while the real marker stays behind (backlog#2290).
// The corrupt flag rides along so a refusal stays a refusal after restart.
for (arn, version_id) in &entry.target_delete_marker_version_ids {
rstate
.target_delete_marker_version_ids
.entry(arn.clone())
.or_insert_with(|| version_id.clone());
}
rstate.target_delete_marker_version_ids_corrupt |= entry.target_delete_marker_version_ids_corrupt;
let delete_marker_mtime = entry
.delete_marker_mtime
@@ -6601,4 +6615,87 @@ mod tests {
replacement_data
);
}
/// backlog#2290: a delete-marker purge intent that survives a restart
/// through the MRF journal addresses the marker version the TARGET
/// assigned, exactly as the live watcher does (see the
/// `requires_delayed_purge` spawn). The journal carries the per-ARN ids
/// (`targetDeleteMarkerVersionIDs`) and replay restores them into the
/// reconstructed replication state; without that the replay would fall
/// back to the source marker id, which a target that mints its own ids
/// answers with an idempotent 204 — the entry would be acknowledged while
/// the real marker stayed behind.
#[test]
fn mrf_delete_marker_purge_replay_preserves_target_assigned_marker_version() {
use super::super::replication_object_decision_boundary::{delete_marker_purge_mrf_entry, delete_marker_purge_version_id};
let arn = "arn:minio:replication::generic-target:photos".to_string();
let source_marker = uuid::Uuid::new_v4();
let remote_marker = "remote-assigned-marker-version".to_string();
let live_oi = ObjectInfo {
bucket: "photos".to_string(),
name: "obj".to_string(),
version_id: Some(source_marker),
delete_marker: true,
..Default::default()
};
let mut live_state = live_oi.replication_state();
live_state.replicate_decision_str = replicate_decision_for_admitted_targets(std::slice::from_ref(&arn)).to_string();
live_state
.target_delete_marker_version_ids
.insert(arn.clone(), remote_marker.clone());
let live = DeletedObjectReplicationInfo {
delete_object: ReplicationDeletedObject {
object_name: "obj".to_string(),
delete_marker: true,
delete_marker_version_id: Some(source_marker),
replication_state: Some(live_state),
..Default::default()
},
bucket: "photos".to_string(),
..Default::default()
};
assert_eq!(
delete_marker_purge_version_id(live.delete_object.replication_state.as_ref(), &arn, source_marker),
Some(Some(remote_marker.clone())),
"the live purge addresses the recorded target version"
);
// Watch window exhausted: persist the intent, restart, replay it.
let entry = delete_marker_purge_mrf_entry(&live, vec![arn.clone()]);
let replay_oi = ObjectInfo {
bucket: entry.bucket.clone(),
name: entry.object.clone(),
version_id: entry.version_id,
delete_marker: entry.delete_marker,
..Default::default()
};
let dsc = replicate_decision_for_admitted_targets(&entry.target_arns);
let replayed = reconstructed_heal_delete_info(&entry, &replay_oi, &dsc);
assert_eq!(
delete_marker_purge_version_id(replayed.delete_object.replication_state.as_ref(), &arn, source_marker),
Some(Some(remote_marker)),
"the MRF replay must address the target-assigned marker version, not source marker {source_marker}"
);
// A refusal (inconsistent recorded ids) must stay a refusal across the
// journal round trip instead of degrading into the source-id fallback.
let mut refused = live;
refused
.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(&refused, vec![arn.clone()]);
assert!(entry.target_delete_marker_version_ids_corrupt);
let replayed = reconstructed_heal_delete_info(&entry, &replay_oi, &dsc);
assert_eq!(
delete_marker_purge_version_id(replayed.delete_object.replication_state.as_ref(), &arn, source_marker),
None,
"the MRF replay must keep refusing to guess when the recorded ids were inconsistent"
);
}
}
+85
View File
@@ -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,
@@ -595,6 +610,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]
+20
View File
@@ -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>,
+167 -2
View File
@@ -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]
+2
View File
@@ -726,6 +726,8 @@ pub(crate) mod bucket {
delete_marker_version_id: None,
delete_marker: false,
delete_marker_mtime: None,
target_delete_marker_version_ids: Default::default(),
target_delete_marker_version_ids_corrupt: false,
target_arns,
force_delete_id: Some(operation_id),
force_delete_generation: Some(i64::try_from(generation.unix_timestamp_nanos()).unwrap_or(i64::MAX)),