mirror of
https://github.com/rustfs/rustfs.git
synced 2026-07-26 08:18:18 +00:00
fix(replication): persist delete marker mtime in MRF entries (#4331)
fix(replication): persist original mtime in MRF entries (backlog#867) MRF delete entries did not persist the original delete-marker mtime, so after a restart the recovery replay path reconstructed the delete without a source timestamp. Downstream the replica delete-marker was stamped with the replay time (now()) instead of the source mtime, causing delete-marker timestamp divergence across clusters. Extend the MrfReplicateEntry disk format with an optional deleteMarkerMtime field (persisted as Unix nanoseconds) in both duplicate struct definitions (rustfs-replication and rustfs-filemeta). DeletedObjectReplicationInfo now persists delete_marker_mtime, and start_mrf_processor restores it onto the reconstructed delete so the replica keeps the source timestamp. Backward compatibility: the new key uses skip_serializing_if + serde default, so historical MRF files without it decode to None and replay falls back to the current time (pre-#867 behaviour). No panic or entry loss on old files. Closes rustfs/backlog#867
This commit is contained in:
@@ -651,12 +651,22 @@ impl<S: ReplicationStorage> ReplicationPool<S> {
|
||||
let mut rstate = oi.replication_state();
|
||||
rstate.replicate_decision_str = dsc.to_string();
|
||||
|
||||
// Restore the original delete-marker mtime persisted with the entry so
|
||||
// the replica keeps the source timestamp. Old MRF files lack this field
|
||||
// (delete_marker_mtime = None) — fall back to None so the replica is
|
||||
// stamped with the current time, preserving pre-#867 behaviour
|
||||
// (backlog#867).
|
||||
let delete_marker_mtime = entry
|
||||
.delete_marker_mtime
|
||||
.and_then(|nanos| OffsetDateTime::from_unix_timestamp_nanos(nanos as i128).ok());
|
||||
|
||||
let dv = DeletedObjectReplicationInfo {
|
||||
delete_object: ReplicationDeletedObject {
|
||||
object_name: entry.object.clone(),
|
||||
version_id: entry.version_id,
|
||||
delete_marker_version_id: entry.delete_marker_version_id,
|
||||
delete_marker: entry.delete_marker,
|
||||
delete_marker_mtime,
|
||||
replication_state: Some(rstate),
|
||||
..Default::default()
|
||||
},
|
||||
@@ -1509,6 +1519,7 @@ mod tests {
|
||||
op: MrfOpKind::Object,
|
||||
delete_marker_version_id: None,
|
||||
delete_marker: false,
|
||||
delete_marker_mtime: None,
|
||||
};
|
||||
let second = MrfReplicateEntry {
|
||||
object: "second".to_string(),
|
||||
@@ -1564,6 +1575,7 @@ mod tests {
|
||||
op: MrfOpKind::Object,
|
||||
delete_marker_version_id: None,
|
||||
delete_marker: false,
|
||||
delete_marker_mtime: None,
|
||||
};
|
||||
|
||||
let encoded = encode_mrf_file(std::slice::from_ref(&entry)).expect("encode");
|
||||
@@ -1584,6 +1596,9 @@ mod tests {
|
||||
#[test]
|
||||
fn mrf_entry_delete_marker_roundtrip() {
|
||||
let dm_vid = Uuid::new_v4();
|
||||
// A specific, non-now() nanosecond timestamp: replay must preserve this exact value
|
||||
// instead of stamping the replica with the current time (backlog#867).
|
||||
let mtime_nanos = 1_705_312_200_123_456_789i64;
|
||||
let entry = MrfReplicateEntry {
|
||||
bucket: "del-bucket".to_string(),
|
||||
object: "key".to_string(),
|
||||
@@ -1593,6 +1608,7 @@ mod tests {
|
||||
op: MrfOpKind::Delete,
|
||||
delete_marker_version_id: Some(dm_vid),
|
||||
delete_marker: true,
|
||||
delete_marker_mtime: Some(mtime_nanos),
|
||||
};
|
||||
|
||||
let encoded = encode_mrf_file(std::slice::from_ref(&entry)).expect("encode");
|
||||
@@ -1606,6 +1622,11 @@ mod tests {
|
||||
assert_eq!(got.op, MrfOpKind::Delete);
|
||||
assert_eq!(got.delete_marker_version_id, Some(dm_vid));
|
||||
assert!(got.delete_marker);
|
||||
assert_eq!(
|
||||
got.delete_marker_mtime,
|
||||
Some(mtime_nanos),
|
||||
"delete-marker mtime must survive the MRF disk round-trip"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1620,6 +1641,7 @@ mod tests {
|
||||
op: MrfOpKind::Delete,
|
||||
delete_marker_version_id: None,
|
||||
delete_marker: false,
|
||||
delete_marker_mtime: None,
|
||||
};
|
||||
|
||||
let encoded = encode_mrf_file(&[entry]).expect("encode");
|
||||
@@ -1647,6 +1669,7 @@ mod tests {
|
||||
op: MrfOpKind::Object,
|
||||
delete_marker_version_id: None,
|
||||
delete_marker: false,
|
||||
delete_marker_mtime: None,
|
||||
},
|
||||
MrfReplicateEntry {
|
||||
bucket: "b".to_string(),
|
||||
@@ -1657,6 +1680,7 @@ mod tests {
|
||||
op: MrfOpKind::Delete,
|
||||
delete_marker_version_id: Some(del_dm_vid),
|
||||
delete_marker: true,
|
||||
delete_marker_mtime: None,
|
||||
},
|
||||
];
|
||||
|
||||
@@ -1685,6 +1709,7 @@ mod tests {
|
||||
op: MrfOpKind::Object,
|
||||
delete_marker_version_id: None,
|
||||
delete_marker: false,
|
||||
delete_marker_mtime: None,
|
||||
};
|
||||
assert_eq!(obj_entry.op, MrfOpKind::Object);
|
||||
|
||||
@@ -1698,6 +1723,7 @@ mod tests {
|
||||
op: MrfOpKind::Delete,
|
||||
delete_marker_version_id: Some(Uuid::new_v4()),
|
||||
delete_marker: true,
|
||||
delete_marker_mtime: None,
|
||||
};
|
||||
assert_eq!(del_entry.op, MrfOpKind::Delete);
|
||||
|
||||
@@ -1712,6 +1738,7 @@ mod tests {
|
||||
op: MrfOpKind::default(),
|
||||
delete_marker_version_id: None,
|
||||
delete_marker: false,
|
||||
delete_marker_mtime: None,
|
||||
};
|
||||
assert_eq!(legacy_entry.op, MrfOpKind::Object, "legacy default must be Object");
|
||||
}
|
||||
@@ -1758,5 +1785,8 @@ mod tests {
|
||||
assert_eq!(entry.op, MrfOpKind::Object, "missing op key must default to Object");
|
||||
assert!(!entry.delete_marker);
|
||||
assert_eq!(entry.delete_marker_version_id, None);
|
||||
// The "deleteMarkerMtime" key was absent in old files — #[serde(default)] must fill in
|
||||
// None so replay falls back to the current time (backlog#867 backward compatibility).
|
||||
assert_eq!(entry.delete_marker_mtime, None, "missing deleteMarkerMtime key must default to None");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -586,6 +586,13 @@ pub struct MrfReplicateEntry {
|
||||
// Old files lack this; default=false is correct.
|
||||
#[serde(rename = "deleteMarker", default)]
|
||||
pub delete_marker: bool,
|
||||
|
||||
// For delete entries: the original delete-marker mtime, persisted as Unix nanoseconds so
|
||||
// replay stamps replicas with the source timestamp instead of the replay time. Old files
|
||||
// lack this key; default=None means "unknown", and replay falls back to the current time
|
||||
// to preserve pre-existing behaviour (backlog#867).
|
||||
#[serde(rename = "deleteMarkerMtime", skip_serializing_if = "Option::is_none", default)]
|
||||
pub delete_marker_mtime: Option<i64>,
|
||||
}
|
||||
|
||||
pub trait ReplicationWorkerOperation: Any + Send + Sync {
|
||||
@@ -787,6 +794,7 @@ impl ReplicationWorkerOperation for ReplicateObjectInfo {
|
||||
op: MrfOpKind::Object,
|
||||
delete_marker_version_id: None,
|
||||
delete_marker: false,
|
||||
delete_marker_mtime: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -837,6 +845,7 @@ impl ReplicateObjectInfo {
|
||||
op: MrfOpKind::Object,
|
||||
delete_marker_version_id: None,
|
||||
delete_marker: false,
|
||||
delete_marker_mtime: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -42,6 +42,13 @@ impl ReplicationWorkerOperation for DeletedObjectReplicationInfo {
|
||||
op: MrfOpKind::Delete,
|
||||
delete_marker_version_id: self.delete_object.delete_marker_version_id,
|
||||
delete_marker: self.delete_object.delete_marker,
|
||||
// Persist the original delete-marker mtime as Unix nanoseconds so replay after a
|
||||
// restart stamps the replica with the source timestamp rather than the replay time
|
||||
// (backlog#867). None when unknown; replay then falls back to the current time.
|
||||
delete_marker_mtime: self
|
||||
.delete_object
|
||||
.delete_marker_mtime
|
||||
.and_then(|t| i64::try_from(t.unix_timestamp_nanos()).ok()),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -92,6 +99,7 @@ mod tests {
|
||||
fn deleted_object_replication_info_encodes_delete_mrf_entry() {
|
||||
let version_id = Uuid::new_v4();
|
||||
let delete_marker_version_id = Uuid::new_v4();
|
||||
let mtime = time::OffsetDateTime::from_unix_timestamp_nanos(1_705_312_200_123_456_789).expect("valid mtime");
|
||||
let info = DeletedObjectReplicationInfo {
|
||||
bucket: "bucket".to_string(),
|
||||
op_type: ReplicationType::Delete,
|
||||
@@ -100,6 +108,7 @@ mod tests {
|
||||
version_id: Some(version_id),
|
||||
delete_marker_version_id: Some(delete_marker_version_id),
|
||||
delete_marker: true,
|
||||
delete_marker_mtime: Some(mtime),
|
||||
..Default::default()
|
||||
},
|
||||
..Default::default()
|
||||
@@ -113,9 +122,34 @@ mod tests {
|
||||
assert_eq!(entry.delete_marker_version_id, Some(delete_marker_version_id));
|
||||
assert_eq!(entry.op, MrfOpKind::Delete);
|
||||
assert!(entry.delete_marker);
|
||||
// The original mtime must be persisted (as Unix nanos) so replay keeps the source
|
||||
// timestamp instead of stamping the replica with the replay time (backlog#867).
|
||||
assert_eq!(
|
||||
entry.delete_marker_mtime,
|
||||
Some(mtime.unix_timestamp_nanos() as i64),
|
||||
"delete-marker mtime must be persisted in the MRF entry"
|
||||
);
|
||||
assert_eq!(info.get_object(), "object");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deleted_object_replication_info_without_mtime_yields_none() {
|
||||
// Absent source mtime must persist as None so replay falls back to the current time,
|
||||
// preserving pre-#867 behaviour.
|
||||
let info = DeletedObjectReplicationInfo {
|
||||
bucket: "bucket".to_string(),
|
||||
delete_object: DeletedObject {
|
||||
object_name: "object".to_string(),
|
||||
delete_marker: true,
|
||||
delete_marker_mtime: None,
|
||||
..Default::default()
|
||||
},
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert_eq!(info.to_mrf_entry().delete_marker_mtime, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn version_delete_replication_tracks_delete_marker_version_purge() {
|
||||
let dobj = DeletedObject {
|
||||
|
||||
@@ -586,6 +586,13 @@ pub struct MrfReplicateEntry {
|
||||
// Old files lack this; default=false is correct.
|
||||
#[serde(rename = "deleteMarker", default)]
|
||||
pub delete_marker: bool,
|
||||
|
||||
// For delete entries: the original delete-marker mtime, persisted as Unix nanoseconds so
|
||||
// replay stamps replicas with the source timestamp instead of the replay time. Old files
|
||||
// lack this key; default=None means "unknown", and replay falls back to the current time
|
||||
// to preserve pre-existing behaviour (backlog#867).
|
||||
#[serde(rename = "deleteMarkerMtime", skip_serializing_if = "Option::is_none", default)]
|
||||
pub delete_marker_mtime: Option<i64>,
|
||||
}
|
||||
|
||||
fn retry_count_to_mrf(retry_count: u32) -> i32 {
|
||||
@@ -791,6 +798,7 @@ impl ReplicationWorkerOperation for ReplicateObjectInfo {
|
||||
op: MrfOpKind::Object,
|
||||
delete_marker_version_id: None,
|
||||
delete_marker: false,
|
||||
delete_marker_mtime: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -844,6 +852,7 @@ impl ReplicateObjectInfo {
|
||||
op: MrfOpKind::Object,
|
||||
delete_marker_version_id: None,
|
||||
delete_marker: false,
|
||||
delete_marker_mtime: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -70,6 +70,7 @@ mod tests {
|
||||
op: MrfOpKind::Object,
|
||||
delete_marker_version_id: None,
|
||||
delete_marker: false,
|
||||
delete_marker_mtime: None,
|
||||
},
|
||||
MrfReplicateEntry {
|
||||
bucket: "bucket-a".to_string(),
|
||||
@@ -80,6 +81,7 @@ mod tests {
|
||||
op: MrfOpKind::Delete,
|
||||
delete_marker_version_id: Some(del_vid),
|
||||
delete_marker: true,
|
||||
delete_marker_mtime: Some(1_705_312_200_123_456_789),
|
||||
},
|
||||
];
|
||||
|
||||
@@ -89,9 +91,15 @@ mod tests {
|
||||
assert_eq!(decoded.len(), 2);
|
||||
assert_eq!(decoded[0].version_id, Some(obj_vid));
|
||||
assert_eq!(decoded[0].op, MrfOpKind::Object);
|
||||
assert_eq!(decoded[0].delete_marker_mtime, None);
|
||||
assert_eq!(decoded[1].delete_marker_version_id, Some(del_vid));
|
||||
assert_eq!(decoded[1].op, MrfOpKind::Delete);
|
||||
assert!(decoded[1].delete_marker);
|
||||
assert_eq!(
|
||||
decoded[1].delete_marker_mtime,
|
||||
Some(1_705_312_200_123_456_789),
|
||||
"delete-marker mtime must survive the MRF disk round-trip"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -121,6 +129,9 @@ mod tests {
|
||||
assert_eq!(decoded[0].retry_count, 2);
|
||||
assert_eq!(decoded[0].size, 100);
|
||||
assert_eq!(decoded[0].op, MrfOpKind::Object);
|
||||
// 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);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user