mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-13 16:46:55 +00:00
fix: make ecstore trash cleanup idempotent (#3205)
* fix(http): reduce header timeout log noise Log peer_addr for HTTP connection errors so idle or probe traffic can be traced back to the source. Downgrade HeaderTimeout connection logs from warn to info because these timeouts are often caused by incomplete probe traffic rather than a service-side fault. * fix(ecstore): suppress spurious warn logs for missing trash cleanup sources Extract `reliable_rename_inner` with a `warn_on_failure` flag so that `rename_all_ignore_missing_source` can skip the warn log when the source is absent. This completes the idempotent cleanup fix — the previous implementation still emitted a `reliable_rename failed` warning before the NotFound was caught and silenced. Also restore the `recursive` branching in `move_to_trash` so that non-recursive deletions use `fs::rename` directly (preserving original semantics), while recursive deletions use the idempotent helper.
This commit is contained in:
@@ -26,7 +26,7 @@ use crate::disk::{
|
||||
format::FormatV3,
|
||||
fs::{O_APPEND, O_CREATE, O_RDONLY, O_TRUNC, O_WRONLY, access, lstat, lstat_std, remove, remove_all_std, remove_std, rename},
|
||||
os,
|
||||
os::{check_path_length, is_empty_dir, is_root_disk, rename_all},
|
||||
os::{check_path_length, is_empty_dir, is_root_disk, rename_all, rename_all_ignore_missing_source},
|
||||
};
|
||||
use crate::erasure_coding::bitrot_verify;
|
||||
use crate::global::{GLOBAL_IsErasureSD, GLOBAL_RootDiskThreshold};
|
||||
@@ -929,19 +929,20 @@ impl LocalDisk {
|
||||
// }
|
||||
|
||||
let err = if recursive {
|
||||
rename_all(delete_path, trash_path, self.get_bucket_path(RUSTFS_META_TMP_DELETED_BUCKET)?)
|
||||
rename_all_ignore_missing_source(delete_path, trash_path, self.get_bucket_path(RUSTFS_META_TMP_DELETED_BUCKET)?)
|
||||
.await
|
||||
.err()
|
||||
} else {
|
||||
rename(&delete_path, &trash_path)
|
||||
.await
|
||||
.map_err(|e| to_file_error(e).into())
|
||||
.err()
|
||||
match rename(&delete_path, &trash_path).await {
|
||||
Ok(()) => None,
|
||||
Err(err) if err.kind() == ErrorKind::NotFound => None,
|
||||
Err(err) => Some(to_file_error(err).into()),
|
||||
}
|
||||
};
|
||||
|
||||
if immediate_purge || delete_path.to_string_lossy().ends_with(SLASH_SEPARATOR) {
|
||||
let trash_path2 = self.get_object_path(RUSTFS_META_TMP_DELETED_BUCKET, Uuid::new_v4().to_string().as_str())?;
|
||||
let _ = rename_all(
|
||||
let _ = rename_all_ignore_missing_source(
|
||||
encode_dir_object(delete_path.to_string_lossy().as_ref()),
|
||||
trash_path2,
|
||||
self.get_bucket_path(RUSTFS_META_TMP_DELETED_BUCKET)?,
|
||||
|
||||
@@ -140,10 +140,32 @@ pub async fn rename_all(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tracing::instrument(level = "debug", skip_all)]
|
||||
pub async fn rename_all_ignore_missing_source(
|
||||
src_file_path: impl AsRef<Path>,
|
||||
dst_file_path: impl AsRef<Path>,
|
||||
base_dir: impl AsRef<Path>,
|
||||
) -> Result<()> {
|
||||
match reliable_rename_inner(src_file_path, dst_file_path.as_ref(), base_dir, false).await {
|
||||
Ok(()) => Ok(()),
|
||||
Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(()),
|
||||
Err(err) => Err(to_file_error(err).into()),
|
||||
}
|
||||
}
|
||||
|
||||
async fn reliable_rename(
|
||||
src_file_path: impl AsRef<Path>,
|
||||
dst_file_path: impl AsRef<Path>,
|
||||
base_dir: impl AsRef<Path>,
|
||||
) -> io::Result<()> {
|
||||
reliable_rename_inner(src_file_path, dst_file_path, base_dir, true).await
|
||||
}
|
||||
|
||||
async fn reliable_rename_inner(
|
||||
src_file_path: impl AsRef<Path>,
|
||||
dst_file_path: impl AsRef<Path>,
|
||||
base_dir: impl AsRef<Path>,
|
||||
warn_on_failure: bool,
|
||||
) -> io::Result<()> {
|
||||
if let Some(parent) = dst_file_path.as_ref().parent()
|
||||
&& !file_exists(parent)
|
||||
@@ -158,13 +180,15 @@ async fn reliable_rename(
|
||||
i += 1;
|
||||
continue;
|
||||
}
|
||||
warn!(
|
||||
"reliable_rename failed. src_file_path: {:?}, dst_file_path: {:?}, base_dir: {:?}, err: {:?}",
|
||||
src_file_path.as_ref(),
|
||||
dst_file_path.as_ref(),
|
||||
base_dir.as_ref(),
|
||||
e
|
||||
);
|
||||
if warn_on_failure {
|
||||
warn!(
|
||||
"reliable_rename failed. src_file_path: {:?}, dst_file_path: {:?}, base_dir: {:?}, err: {:?}",
|
||||
src_file_path.as_ref(),
|
||||
dst_file_path.as_ref(),
|
||||
base_dir.as_ref(),
|
||||
e
|
||||
);
|
||||
}
|
||||
return Err(e);
|
||||
}
|
||||
|
||||
@@ -263,4 +287,17 @@ mod tests {
|
||||
assert!(matches!(err, DiskError::FileNotFound));
|
||||
assert!(!dst.exists());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn rename_all_ignore_missing_source_returns_ok() {
|
||||
let temp_dir = tempdir().expect("create temp dir");
|
||||
let src = temp_dir.path().join("missing");
|
||||
let dst = temp_dir.path().join("dst");
|
||||
|
||||
rename_all_ignore_missing_source(&src, &dst, temp_dir.path())
|
||||
.await
|
||||
.expect("missing cleanup source must be ignored");
|
||||
|
||||
assert!(!dst.exists());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user