mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-16 18:08:21 +00:00
fix(ecstore): defer delete cleanup for snapshot reads (#5408)
* feat(ecstore): add local snapshot leases * feat(ecstore): add remote snapshot lease RPCs * feat(ecstore): protect streaming GETs with snapshot leases * fix(ecstore): defer version cleanup for snapshot reads * fix(ecstore): cover batch snapshot cleanup safely * fix(e2e): stub snapshot lease RPCs in lock mock * fix(e2e): stub snapshot lease RPCs in lock mock * fix(ecstore): bind deferred delete cleanup intents * fix(rpc): keep snapshot lease checks CI-compatible
This commit is contained in:
cxymds
@@ -46,6 +46,7 @@ use crate::diagnostics::get::{
|
||||
GetObjectFailureReason, classify_disk_error, get_stage_timer_if_enabled, record_get_object_pipeline_failure,
|
||||
record_get_object_pipeline_failure_for_path, record_get_stage_duration_if_enabled,
|
||||
};
|
||||
use crate::disk::local::DELETE_DATA_DIR_MARKER_PREFIX;
|
||||
use crate::disk::{
|
||||
DataDirDeleteStatus, OldCurrentSize, PART_TRANSACTION_NEW_META, PART_TRANSACTION_OLD_META, PART_TRANSACTION_ROLLBACK,
|
||||
PartTransactionAction, part_transaction_path,
|
||||
@@ -3952,9 +3953,9 @@ impl SetDisks {
|
||||
/// * The set of referenced data dirs is the UNION of `get_data_dirs()` across
|
||||
/// every online disk's `xl.meta`, so a dir named by *any* replica is kept.
|
||||
/// * If a disk holds the object directory but its `xl.meta` is missing or
|
||||
/// unparsable, the object is treated as degraded and NOTHING is removed —
|
||||
/// the unreadable copy could be the only one naming a live data dir, and a
|
||||
/// heal must run first.
|
||||
/// unparsable, the object is treated as degraded and unmarked data dirs are
|
||||
/// never removed. A data dir carrying a committed delete-transaction marker
|
||||
/// remains reclaimable after a downgrade/re-upgrade cleanup interruption.
|
||||
/// * Only subdirectories whose names parse as a UUID are ever considered;
|
||||
/// removal is non-recursive-safe via a recursive delete of the full stray
|
||||
/// data-dir path only.
|
||||
@@ -3969,7 +3970,7 @@ impl SetDisks {
|
||||
// physical UUID subdirectories present on each disk. Abort on any degraded
|
||||
// copy so a healable object is never stripped of a referenced data dir.
|
||||
let mut referenced: HashSet<Uuid> = HashSet::new();
|
||||
let mut per_disk_dirs: Vec<(usize, Vec<Uuid>)> = Vec::new();
|
||||
let mut per_disk_dirs: Vec<(usize, Vec<(Uuid, bool)>)> = Vec::new();
|
||||
let mut healthy_metas = 0usize;
|
||||
|
||||
for (i, disk) in disks.iter().enumerate() {
|
||||
@@ -4005,6 +4006,22 @@ impl SetDisks {
|
||||
// to the orphan-dir / dangling-object heal paths.
|
||||
continue;
|
||||
}
|
||||
let mut committed = Vec::with_capacity(physical.len());
|
||||
for dir in physical {
|
||||
let data_dir = format!("{object}/{dir}");
|
||||
let committed_delete = disk.list_dir("", bucket, &data_dir, 0).await.is_ok_and(|entries| {
|
||||
entries.iter().any(|entry| {
|
||||
entry
|
||||
.strip_prefix(DELETE_DATA_DIR_MARKER_PREFIX)
|
||||
.is_some_and(|transaction| Uuid::parse_str(transaction).is_ok())
|
||||
})
|
||||
});
|
||||
committed.push((dir, committed_delete));
|
||||
}
|
||||
if committed.iter().all(|(_, committed_delete)| *committed_delete) {
|
||||
per_disk_dirs.push((i, committed));
|
||||
continue;
|
||||
}
|
||||
warn!(
|
||||
target: "rustfs_ecstore::set_disk",
|
||||
bucket, object,
|
||||
@@ -4041,22 +4058,16 @@ impl SetDisks {
|
||||
|
||||
healthy_metas += 1;
|
||||
if !physical.is_empty() {
|
||||
per_disk_dirs.push((i, physical));
|
||||
per_disk_dirs.push((i, physical.into_iter().map(|dir| (dir, false)).collect()));
|
||||
}
|
||||
}
|
||||
|
||||
// No healthy metadata anywhere: this is not a live object, so surplus dirs
|
||||
// (if any) belong to the dangling-object heal path, not here.
|
||||
if healthy_metas == 0 {
|
||||
return Ok(0);
|
||||
}
|
||||
|
||||
// Phase 2: delete every physical data dir not referenced by the union.
|
||||
let mut removed = 0usize;
|
||||
for (i, physical) in per_disk_dirs {
|
||||
let Some(disk) = disks[i].as_ref() else { continue };
|
||||
for dir in physical {
|
||||
if referenced.contains(&dir) {
|
||||
for (dir, committed_delete) in physical {
|
||||
if referenced.contains(&dir) || (healthy_metas == 0 && !committed_delete) {
|
||||
continue;
|
||||
}
|
||||
let stray = format!("{object}/{dir}");
|
||||
|
||||
@@ -4698,6 +4698,7 @@ mod tests {
|
||||
use crate::bucket::replication::{replication_statuses_map, version_purge_statuses_map};
|
||||
use crate::disk::CHECK_PART_UNKNOWN;
|
||||
use crate::disk::CHECK_PART_VOLUME_NOT_FOUND;
|
||||
use crate::disk::DataDirDeleteStatus;
|
||||
use crate::disk::DiskOption;
|
||||
use crate::disk::RUSTFS_META_BUCKET;
|
||||
use crate::disk::RUSTFS_META_TMP_BUCKET;
|
||||
@@ -6403,6 +6404,62 @@ mod tests {
|
||||
assert!(object_dir.join(STORAGE_FORMAT_FILE).exists(), "metadata must be preserved");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reclaim_orphan_data_dirs_recovers_deferred_cleanup_after_restart() {
|
||||
let (dir, disk) = make_single_local_disk().await;
|
||||
let live = Uuid::new_v4();
|
||||
let orphan = Uuid::new_v4();
|
||||
let object_dir = dir.path().join("bucket").join("obj");
|
||||
write_object_meta_with_data_dirs(&object_dir, "bucket", "obj", &[live]).await;
|
||||
fs::create_dir_all(object_dir.join(live.to_string()))
|
||||
.await
|
||||
.expect("live data dir should be created");
|
||||
let orphan_path = format!("obj/{orphan}");
|
||||
disk.write_all("bucket", &format!("{orphan_path}/part.1"), Bytes::from_static(b"stale"))
|
||||
.await
|
||||
.expect("orphan part should be written");
|
||||
|
||||
let _token = disk
|
||||
.acquire_snapshot_lease("bucket", &orphan_path)
|
||||
.await
|
||||
.expect("snapshot lease should be acquired");
|
||||
assert_eq!(
|
||||
disk.delete_data_dir(
|
||||
"bucket",
|
||||
&orphan_path,
|
||||
DeleteOptions {
|
||||
recursive: true,
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("cleanup should be deferred"),
|
||||
DataDirDeleteStatus::Deferred
|
||||
);
|
||||
drop(disk);
|
||||
|
||||
let endpoint =
|
||||
Endpoint::try_from(dir.path().to_str().expect("tempdir path should be utf8")).expect("endpoint should parse");
|
||||
let restarted = new_disk(
|
||||
&endpoint,
|
||||
&DiskOption {
|
||||
cleanup: false,
|
||||
health_check: false,
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("disk should restart");
|
||||
let set = make_set_disks_with(vec![Some(restarted)]).await;
|
||||
let removed = set
|
||||
.reclaim_orphan_data_dirs("bucket", "obj")
|
||||
.await
|
||||
.expect("restart reclaim should succeed");
|
||||
|
||||
assert_eq!(removed, 1, "the deferred orphan should be reclaimed after restart");
|
||||
assert!(object_dir.join(live.to_string()).exists(), "referenced data dir must be preserved");
|
||||
assert!(!object_dir.join(orphan.to_string()).exists(), "deferred orphan must be removed");
|
||||
}
|
||||
|
||||
// Nothing to reclaim when every physical data dir is still referenced.
|
||||
#[tokio::test]
|
||||
async fn reclaim_orphan_data_dirs_keeps_referenced_dir() {
|
||||
@@ -6441,6 +6498,16 @@ mod tests {
|
||||
fs::write(object_dir.join(stray.to_string()).join("part.1"), b"data")
|
||||
.await
|
||||
.expect("part should be written");
|
||||
fs::write(
|
||||
object_dir.join(stray.to_string()).join(format!(
|
||||
"{}{}",
|
||||
crate::disk::local::RESERVED_DELETE_DATA_DIR_MARKER_PREFIX,
|
||||
Uuid::new_v4()
|
||||
)),
|
||||
[],
|
||||
)
|
||||
.await
|
||||
.expect("pre-commit delete reservation should be written");
|
||||
|
||||
let set = make_set_disks_with(vec![Some(disk)]).await;
|
||||
let removed = set
|
||||
@@ -6455,6 +6522,36 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reclaim_orphan_data_dirs_recovers_committed_delete_marker_without_meta() {
|
||||
let (dir, disk) = make_single_local_disk().await;
|
||||
let stale = Uuid::new_v4();
|
||||
let transaction = Uuid::new_v4();
|
||||
let object_dir = dir.path().join("bucket").join("obj");
|
||||
let stale_dir = object_dir.join(stale.to_string());
|
||||
fs::create_dir_all(&stale_dir)
|
||||
.await
|
||||
.expect("committed stale data dir should be created");
|
||||
fs::write(stale_dir.join("part.1"), b"stale")
|
||||
.await
|
||||
.expect("stale part should be written");
|
||||
fs::write(
|
||||
stale_dir.join(format!("{}{}", crate::disk::local::DELETE_DATA_DIR_MARKER_PREFIX, transaction)),
|
||||
[],
|
||||
)
|
||||
.await
|
||||
.expect("committed delete marker should be written");
|
||||
|
||||
let set = make_set_disks_with(vec![Some(disk)]).await;
|
||||
let removed = set
|
||||
.reclaim_orphan_data_dirs("bucket", "obj")
|
||||
.await
|
||||
.expect("upgrade reclaim should succeed");
|
||||
|
||||
assert_eq!(removed, 1, "the committed delete residue should be reclaimed");
|
||||
assert!(!stale_dir.exists(), "the committed stale data dir should be removed");
|
||||
}
|
||||
|
||||
// Cross-replica union: a data dir referenced by ANOTHER disk's xl.meta must be
|
||||
// kept even where the local replica does not name it.
|
||||
#[tokio::test]
|
||||
@@ -9774,6 +9871,113 @@ mod tests {
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
#[serial]
|
||||
async fn streaming_get_snapshot_survives_concurrent_delete() {
|
||||
temp_env::async_with_vars([(rustfs_config::ENV_OBJECT_LOCK_OPTIMIZATION_ENABLE, Some("true"))], async {
|
||||
let set_disks = make_local_bucket_test_set_disks().await;
|
||||
let bucket = "snapshot-streaming-delete";
|
||||
let object = "object";
|
||||
let body = vec![0x41; 2 * 1024 * 1024];
|
||||
let opts = ObjectOptions::default();
|
||||
|
||||
set_disks
|
||||
.make_bucket(bucket, &MakeBucketOptions::default())
|
||||
.await
|
||||
.expect("bucket should be created");
|
||||
let mut reader = PutObjReader::from_vec(body.clone());
|
||||
set_disks
|
||||
.put_object(bucket, object, &mut reader, &opts)
|
||||
.await
|
||||
.expect("object should be written");
|
||||
|
||||
let mut snapshot = set_disks
|
||||
.get_object_reader(bucket, object, None, HeaderMap::new(), &opts)
|
||||
.await
|
||||
.expect("snapshot reader should open");
|
||||
let delete_set = Arc::clone(&set_disks);
|
||||
let delete_opts = opts.clone();
|
||||
let delete = tokio::spawn(async move { delete_set.delete_object(bucket, object, delete_opts).await });
|
||||
tokio::time::timeout(Duration::from_secs(30), delete)
|
||||
.await
|
||||
.expect("delete should not wait for the response body")
|
||||
.expect("delete task should join")
|
||||
.expect("delete should succeed");
|
||||
|
||||
let mut restored = Vec::new();
|
||||
snapshot
|
||||
.stream
|
||||
.read_to_end(&mut restored)
|
||||
.await
|
||||
.expect("leased snapshot should remain readable after delete");
|
||||
assert_eq!(restored, body);
|
||||
let err = match set_disks
|
||||
.get_object_reader(bucket, object, None, HeaderMap::new(), &opts)
|
||||
.await
|
||||
{
|
||||
Ok(_) => panic!("a new read must not observe the deleted object"),
|
||||
Err(err) => err,
|
||||
};
|
||||
assert!(is_err_object_not_found(&err));
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
#[serial]
|
||||
async fn streaming_get_snapshot_survives_concurrent_delete_objects() {
|
||||
temp_env::async_with_vars([(rustfs_config::ENV_OBJECT_LOCK_OPTIMIZATION_ENABLE, Some("true"))], async {
|
||||
let set_disks = make_local_bucket_test_set_disks().await;
|
||||
let bucket = "snapshot-streaming-delete-objects";
|
||||
let object = "object";
|
||||
let body = vec![0x41; 2 * 1024 * 1024];
|
||||
let opts = ObjectOptions::default();
|
||||
|
||||
set_disks
|
||||
.make_bucket(bucket, &MakeBucketOptions::default())
|
||||
.await
|
||||
.expect("bucket should be created");
|
||||
let mut reader = PutObjReader::from_vec(body.clone());
|
||||
set_disks
|
||||
.put_object(bucket, object, &mut reader, &opts)
|
||||
.await
|
||||
.expect("object should be written");
|
||||
|
||||
let mut snapshot = set_disks
|
||||
.get_object_reader(bucket, object, None, HeaderMap::new(), &opts)
|
||||
.await
|
||||
.expect("snapshot reader should open");
|
||||
let delete_set = Arc::clone(&set_disks);
|
||||
let delete_opts = opts.clone();
|
||||
let delete = tokio::spawn(async move {
|
||||
delete_set
|
||||
.delete_objects(
|
||||
bucket,
|
||||
vec![ObjectToDelete {
|
||||
object_name: object.to_string(),
|
||||
..Default::default()
|
||||
}],
|
||||
delete_opts,
|
||||
)
|
||||
.await
|
||||
});
|
||||
let (_, errors) = tokio::time::timeout(Duration::from_secs(30), delete)
|
||||
.await
|
||||
.expect("batch delete should not wait for the response body")
|
||||
.expect("batch delete task should join");
|
||||
assert!(errors.iter().all(Option::is_none));
|
||||
|
||||
let mut restored = Vec::new();
|
||||
snapshot
|
||||
.stream
|
||||
.read_to_end(&mut restored)
|
||||
.await
|
||||
.expect("leased snapshot should remain readable after batch delete");
|
||||
assert_eq!(restored, body);
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn set_level_batched_large_put_get_restores_body() {
|
||||
const BATCHED_LARGE_SIZE: usize = 64 * 1024 * 1024;
|
||||
|
||||
Reference in New Issue
Block a user