mirror of
https://github.com/rustfs/rustfs.git
synced 2026-09-01 09:48:20 +00:00
fix(heal): skip dangling delete grace failures (#6799)
Co-authored-by: heihutu <heihutu@gmail.com>
This commit is contained in:
@@ -16,6 +16,8 @@ use thiserror::Error;
|
||||
|
||||
use super::heal::{DiskError, EcstoreError};
|
||||
|
||||
const HEAL_DANGLING_DELETE_GRACE_MESSAGE: &str = "dangling object deletion deferred by heal grace window";
|
||||
|
||||
/// Custom error type for heal operations
|
||||
/// This enum defines various error variants that can occur during
|
||||
/// the execution of heal-related tasks, such as I/O errors, storage errors,
|
||||
@@ -98,6 +100,9 @@ impl Error {
|
||||
// them.
|
||||
Error::Storage(EcstoreError::Lock(lock_err)) => !lock_err.is_fatal(),
|
||||
Error::Storage(err) => {
|
||||
if err.is_dangling_delete_grace() {
|
||||
return true;
|
||||
}
|
||||
err.is_quorum_error()
|
||||
|| matches!(
|
||||
err,
|
||||
@@ -110,6 +115,9 @@ impl Error {
|
||||
|| is_recoverable_heal_error_message(&err.to_string())
|
||||
}
|
||||
Error::Disk(err) => {
|
||||
if err.is_dangling_delete_grace() {
|
||||
return true;
|
||||
}
|
||||
matches!(
|
||||
err,
|
||||
DiskError::DiskNotFound
|
||||
@@ -127,6 +135,18 @@ impl Error {
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn is_dangling_delete_grace(&self) -> bool {
|
||||
match self {
|
||||
Error::Storage(err) => err.is_dangling_delete_grace(),
|
||||
Error::Disk(err) => err.is_dangling_delete_grace(),
|
||||
Error::Io(err) => DiskError::io_error_is_dangling_delete_grace(err),
|
||||
Error::TaskExecutionFailed { message } | Error::Other(message) => {
|
||||
message.contains(HEAL_DANGLING_DELETE_GRACE_MESSAGE)
|
||||
}
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Documented substring fallback for errors that reach heal with their typed
|
||||
|
||||
@@ -159,9 +159,13 @@ impl ErasureSetHealer {
|
||||
|
||||
/// Classify an error returned by [`HealStorageAPI::heal_object`].
|
||||
///
|
||||
/// Both the inner `Ok((_, Some(err)))` and the outer `Err(err)` produced by
|
||||
/// `heal_object` wrap `Error::Storage(StorageError)`, so match on that.
|
||||
/// Most heal object failures wrap `Error::Storage(StorageError)`, while
|
||||
/// compatibility markers can also arrive through Disk/Io/task wrappers.
|
||||
fn classify_heal_object_error(err: &Error) -> HealObjectOutcome {
|
||||
if err.is_dangling_delete_grace() {
|
||||
return HealObjectOutcome::Transient;
|
||||
}
|
||||
|
||||
let Error::Storage(se) = err else {
|
||||
return HealObjectOutcome::Failed;
|
||||
};
|
||||
@@ -1459,6 +1463,7 @@ mod tests {
|
||||
// genuine object absence, or transient failures get recorded as "healed" and
|
||||
// permanently skipped.
|
||||
use super::{EcstoreError, Error, HealObjectOutcome};
|
||||
use crate::heal::DiskError;
|
||||
|
||||
fn classify(err: EcstoreError) -> HealObjectOutcome {
|
||||
ErasureSetHealer::classify_heal_object_error(&Error::Storage(err))
|
||||
@@ -1479,6 +1484,16 @@ mod tests {
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dangling_delete_grace_is_transient() {
|
||||
assert!(matches!(
|
||||
ErasureSetHealer::classify_heal_object_error(&Error::Disk(DiskError::other(
|
||||
"dangling object deletion deferred by heal grace window; retry_after_secs=3599; grace_secs=3600"
|
||||
))),
|
||||
HealObjectOutcome::Transient
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn genuine_object_absence_is_absent() {
|
||||
assert!(matches!(classify(EcstoreError::FileNotFound), HealObjectOutcome::Absent));
|
||||
|
||||
@@ -682,6 +682,10 @@ impl HealTask {
|
||||
Self::is_data_usage_cache_object(bucket, object) && Self::is_transient_lock_or_timeout_error(err)
|
||||
}
|
||||
|
||||
fn is_dangling_delete_grace_error(err: &Error) -> bool {
|
||||
err.is_dangling_delete_grace()
|
||||
}
|
||||
|
||||
fn is_no_heal_required_error(err: &Error) -> bool {
|
||||
match err {
|
||||
Error::Storage(EcstoreError::NoHealRequired) | Error::Disk(DiskError::NoHealRequired) => true,
|
||||
@@ -746,6 +750,30 @@ impl HealTask {
|
||||
true
|
||||
}
|
||||
|
||||
async fn skip_dangling_delete_grace_error(&self, bucket: &str, object: &str, err: &Error) -> bool {
|
||||
if !Self::is_dangling_delete_grace_error(err) {
|
||||
return false;
|
||||
}
|
||||
|
||||
warn!(
|
||||
target: "rustfs::heal::task",
|
||||
event = EVENT_HEAL_OBJECT_RESULT,
|
||||
component = LOG_COMPONENT_HEAL,
|
||||
subsystem = LOG_SUBSYSTEM_OBJECT,
|
||||
task_id = %self.id,
|
||||
bucket,
|
||||
object,
|
||||
result = "dangling_delete_grace_skip",
|
||||
error = %err,
|
||||
"Heal object dangling cleanup deferred by grace window"
|
||||
);
|
||||
let mut progress = self.progress.write().await;
|
||||
progress.set_current_object(Some(format!("skipped: {bucket}/{object}")));
|
||||
progress.update_object_progress(1, 0, 0, 1, 0);
|
||||
progress.update_stage(3, 3);
|
||||
true
|
||||
}
|
||||
|
||||
async fn skip_scanner_synthetic_object_dir_missing(&self, bucket: &str, object: &str, err: &Error) -> bool {
|
||||
if self.source != HealRequestSource::Scanner || !is_missing_object_dir_heal_result(object, err) {
|
||||
return false;
|
||||
|
||||
@@ -359,7 +359,21 @@ impl HealTask {
|
||||
};
|
||||
|
||||
if let Some(err) = error {
|
||||
if Self::should_skip_data_usage_cache_heal_error(bucket, object, &err) {
|
||||
if Self::is_dangling_delete_grace_error(&err) {
|
||||
telemetry_unknown |= !increment_counter(&mut skipped);
|
||||
warn!(
|
||||
target: "rustfs::heal::task",
|
||||
event = EVENT_HEAL_BUCKET_RESULT,
|
||||
component = LOG_COMPONENT_HEAL,
|
||||
subsystem = LOG_SUBSYSTEM_TASK,
|
||||
task_id = %self.id,
|
||||
bucket,
|
||||
object,
|
||||
result = "dangling_delete_grace_skip",
|
||||
error = %err,
|
||||
"Heal bucket object dangling cleanup deferred by grace window"
|
||||
);
|
||||
} else if Self::should_skip_data_usage_cache_heal_error(bucket, object, &err) {
|
||||
telemetry_unknown |= !increment_counter(&mut skipped);
|
||||
warn!(
|
||||
target: "rustfs::heal::task",
|
||||
|
||||
@@ -169,6 +169,10 @@ impl HealTask {
|
||||
match heal_result {
|
||||
Ok((result, error)) => {
|
||||
if let Some(e) = error {
|
||||
if self.skip_dangling_delete_grace_error(bucket, object, &e).await {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
if self.skip_data_usage_cache_heal_error(bucket, object, &e).await {
|
||||
return Ok(());
|
||||
}
|
||||
@@ -257,6 +261,10 @@ impl HealTask {
|
||||
Err(Error::TaskCancelled) => Err(Error::TaskCancelled),
|
||||
Err(Error::TaskTimeout) => Err(Error::TaskTimeout),
|
||||
Err(e) => {
|
||||
if self.skip_dangling_delete_grace_error(bucket, object, &e).await {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
if self.skip_data_usage_cache_heal_error(bucket, object, &e).await {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
@@ -705,6 +705,7 @@ fn replacement_identity(
|
||||
enum MockHealObjectOutcome {
|
||||
OkWithOtherError(&'static str),
|
||||
ErrOther(&'static str),
|
||||
DanglingGraceDeferred,
|
||||
RetryableReadQuorum,
|
||||
RetryableSlowDown,
|
||||
PermanentOther(&'static str),
|
||||
@@ -806,6 +807,12 @@ impl HealStorageAPI for MockStorage {
|
||||
.and_then(VecDeque::pop_front)
|
||||
{
|
||||
return match outcome {
|
||||
MockHealObjectOutcome::DanglingGraceDeferred => Ok((
|
||||
HealResultItem::default(),
|
||||
Some(Error::Disk(DiskError::other(
|
||||
"dangling object deletion deferred by heal grace window; retry_after_secs=3599; grace_secs=3600",
|
||||
))),
|
||||
)),
|
||||
MockHealObjectOutcome::RetryableReadQuorum => Err(Error::Storage(EcstoreError::InsufficientReadQuorum(
|
||||
bucket.to_string(),
|
||||
object.to_string(),
|
||||
@@ -820,6 +827,12 @@ impl HealStorageAPI for MockStorage {
|
||||
}
|
||||
if let Some(outcome) = self.heal_object_outcome.lock().unwrap().take() {
|
||||
return match outcome {
|
||||
MockHealObjectOutcome::DanglingGraceDeferred => Ok((
|
||||
HealResultItem::default(),
|
||||
Some(Error::Disk(DiskError::other(
|
||||
"dangling object deletion deferred by heal grace window; retry_after_secs=3599; grace_secs=3600",
|
||||
))),
|
||||
)),
|
||||
MockHealObjectOutcome::OkWithOtherError(message) => Ok((HealResultItem::default(), Some(Error::other(message)))),
|
||||
MockHealObjectOutcome::ErrOther(message) | MockHealObjectOutcome::PermanentOther(message) => {
|
||||
Err(Error::other(message))
|
||||
@@ -1310,6 +1323,31 @@ async fn test_cluster_heal_visits_bucket_objects() {
|
||||
assert!(matches!(task.get_status().await, HealTaskStatus::Completed));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn object_heal_skips_dangling_delete_grace_without_failing_task() {
|
||||
let storage = Arc::new(MockStorage {
|
||||
heal_object_outcome: Mutex::new(Some(MockHealObjectOutcome::DanglingGraceDeferred)),
|
||||
..Default::default()
|
||||
});
|
||||
let task = HealTask::from_request(
|
||||
HealRequest::object("bucket-a".to_string(), "recent.txt".to_string(), None),
|
||||
storage.clone(),
|
||||
);
|
||||
|
||||
task.execute()
|
||||
.await
|
||||
.expect("grace-protected dangling cleanup should be reported as a skipped object");
|
||||
|
||||
assert!(matches!(task.get_status().await, HealTaskStatus::Completed));
|
||||
assert!(storage.healed_objects.lock().unwrap().is_empty());
|
||||
let progress = task.get_progress().await;
|
||||
assert_eq!(progress.current_object.as_deref(), Some("skipped: bucket-a/recent.txt"));
|
||||
assert_eq!(progress.objects_scanned, 1);
|
||||
assert_eq!(progress.objects_healed, 0);
|
||||
assert_eq!(progress.objects_failed, 0);
|
||||
assert_eq!(progress.skipped_objects, 1);
|
||||
}
|
||||
|
||||
#[tokio::test(start_paused = true)]
|
||||
async fn test_recursive_bucket_heal_retries_only_retryable_objects() {
|
||||
let storage = Arc::new(MockStorage::default());
|
||||
@@ -1345,6 +1383,43 @@ async fn test_recursive_bucket_heal_retries_only_retryable_objects() {
|
||||
assert_eq!(progress.objects_failed, 0);
|
||||
}
|
||||
|
||||
#[tokio::test(start_paused = true)]
|
||||
async fn recursive_bucket_heal_skips_dangling_delete_grace_without_batch_failure() {
|
||||
let storage = Arc::new(MockStorage::default());
|
||||
storage
|
||||
.heal_object_outcomes
|
||||
.lock()
|
||||
.unwrap()
|
||||
.insert("object-a".to_string(), VecDeque::from([MockHealObjectOutcome::DanglingGraceDeferred]));
|
||||
let request = HealRequest::new(
|
||||
HealType::Bucket {
|
||||
bucket: "bucket-a".to_string(),
|
||||
},
|
||||
HealOptions {
|
||||
recursive: true,
|
||||
timeout: None,
|
||||
..Default::default()
|
||||
},
|
||||
HealPriority::Normal,
|
||||
);
|
||||
let task = HealTask::from_request(request, storage.clone());
|
||||
|
||||
task.heal_bucket("bucket-a")
|
||||
.await
|
||||
.expect("grace-protected dangling cleanup should not fail the bucket heal batch");
|
||||
|
||||
assert_eq!(
|
||||
storage.heal_object_calls.lock().unwrap().as_slice(),
|
||||
["object-a".to_string(), "object-b".to_string()]
|
||||
);
|
||||
assert_eq!(storage.healed_objects.lock().unwrap().as_slice(), ["object-b".to_string()]);
|
||||
let progress = task.get_progress().await;
|
||||
assert_eq!(progress.objects_scanned, 2);
|
||||
assert_eq!(progress.objects_healed, 1);
|
||||
assert_eq!(progress.objects_failed, 0);
|
||||
assert_eq!(progress.skipped_objects, 1);
|
||||
}
|
||||
|
||||
#[tokio::test(start_paused = true)]
|
||||
async fn test_recursive_bucket_heal_reports_typed_exhausted_and_permanent_failures() {
|
||||
let storage = Arc::new(MockStorage::default());
|
||||
|
||||
Reference in New Issue
Block a user