From bde67362133681d14ee4fdf0ba9f5dd0698bbaa7 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Wed, 19 Aug 2026 10:25:56 +0800 Subject: [PATCH] fix(ecstore): classify a missing data-usage cache by the error that arrives (#6233) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `is_data_usage_cache_absent` matched `FileNotFound | VolumeNotFound`, but `SetDisks::get_object_reader` runs its failures through `to_object_err`, which rewrites those to `ObjectNotFound` and `BucketNotFound` before they reach the caller. The classifier therefore never matched in production: a cache object that simply does not exist was treated as a transient failure, retried five times with backoff, and then reported as an error instead of an empty cache. Admin server-info resolves one cache per erasure set, so that is roughly 1.5s of pointless backoff per set on any cluster whose scanner has not written a cache yet. The same rewrite is why the pre-existing `FileNotFound | VolumeNotFound` arm in the old loop never fired either, which left the legacy-key fallback beside it unreachable — it only ever returned an empty cache through the catch-all break. The classifier now covers the rewritten variants as well as the raw pair, the test store reports absence the way `to_object_err` does, and a new test pins which variants actually arrive. Refs backlog#1828 --- crates/ecstore/src/data_usage/mod.rs | 33 ++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/crates/ecstore/src/data_usage/mod.rs b/crates/ecstore/src/data_usage/mod.rs index 75a1eb970..917edc649 100644 --- a/crates/ecstore/src/data_usage/mod.rs +++ b/crates/ecstore/src/data_usage/mod.rs @@ -2026,8 +2026,17 @@ enum DataUsageCacheRead { /// True when the error means the cache object does not exist, as opposed to a /// transient failure that is worth another attempt. +/// +/// `SetDisks::get_object_reader` runs its failures through `to_object_err`, +/// which rewrites `FileNotFound` to `ObjectNotFound` and `VolumeNotFound` to +/// `BucketNotFound`, so those are the variants that actually arrive here. The +/// raw pair is matched too because callers reading through a different layer +/// can still surface it. fn is_data_usage_cache_absent(err: &Error) -> bool { - matches!(err, Error::FileNotFound | Error::VolumeNotFound) + matches!( + err, + Error::FileNotFound | Error::VolumeNotFound | Error::ObjectNotFound(..) | Error::BucketNotFound(..) + ) } async fn read_data_usage_cache_object(store: &S, key: &str) -> crate::error::Result @@ -2511,7 +2520,10 @@ mod tests { *remaining -= 1; return Err(Error::other("transient read failure")); } - Err(Error::FileNotFound) + // `SetDisks::get_object_reader` reports a missing object through + // `to_object_err`, so the absence that reaches the caller is + // `ObjectNotFound`, not the raw `FileNotFound`. + Err(Error::ObjectNotFound(RUSTFS_META_BUCKET.to_string(), object.to_string())) } async fn put_object( @@ -2533,6 +2545,23 @@ mod tests { .to_string() } + #[test] + fn data_usage_cache_absence_covers_the_variants_that_actually_arrive() { + // `to_object_err` rewrites the raw storage variants before they reach + // `load_data_usage_cache`; classifying only the raw pair would treat a + // missing cache as a transient failure and retry it. + assert!(is_data_usage_cache_absent(&Error::ObjectNotFound( + "bucket".to_string(), + "object".to_string() + ))); + assert!(is_data_usage_cache_absent(&Error::BucketNotFound("bucket".to_string()))); + assert!(is_data_usage_cache_absent(&Error::FileNotFound)); + assert!(is_data_usage_cache_absent(&Error::VolumeNotFound)); + + assert!(!is_data_usage_cache_absent(&Error::other("transient read failure"))); + assert!(!is_data_usage_cache_absent(&Error::DiskNotFound)); + } + #[tokio::test] async fn load_data_usage_cache_treats_absence_as_an_empty_cache_without_retrying() { let name = "usage-cache";