From 1f54ec8242091b1518c8b0421f59ab5908547770 Mon Sep 17 00:00:00 2001 From: Alessandro Passaro Date: Tue, 1 Oct 2024 13:34:06 +0100 Subject: [PATCH 1/2] Improve error handling when removing cache blocks Signed-off-by: Alessandro Passaro --- .../src/data_cache/disk_data_cache.rs | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/mountpoint-s3/src/data_cache/disk_data_cache.rs b/mountpoint-s3/src/data_cache/disk_data_cache.rs index c136c7aff..ab41ad52b 100644 --- a/mountpoint-s3/src/data_cache/disk_data_cache.rs +++ b/mountpoint-s3/src/data_cache/disk_data_cache.rs @@ -335,11 +335,19 @@ impl DiskDataCache { let path_to_remove = self.get_path_for_block_key(&to_remove); trace!("evicting block at {}", path_to_remove.display()); if let Err(remove_err) = fs::remove_file(&path_to_remove) { - warn!("unable to remove invalid block: {:?}", remove_err); + if remove_err.kind() != ErrorKind::NotFound { + warn!("unable to evict block: {:?}", remove_err); + } } } Ok(()) } + + fn remove_block_from_usage(&self, block_key: &DiskBlockKey) { + if let Some(usage) = &self.usage { + usage.lock().unwrap().remove(block_key); + } + } } /// Hash the cache key using its fields as well as the [CACHE_VERSION]. @@ -389,12 +397,15 @@ impl DataCache for DiskDataCache { metrics::counter!("disk_data_cache.block_hit").increment(0); metrics::counter!("disk_data_cache.block_err").increment(1); match fs::remove_file(&path) { - Ok(()) => { - if let Some(usage) = &self.usage { - usage.lock().unwrap().remove(&block_key); + Ok(()) => self.remove_block_from_usage(&block_key), + Err(remove_err) => { + // We failed to delete the block. + if remove_err.kind() == ErrorKind::NotFound { + // No need to report or try again. + self.remove_block_from_usage(&block_key); } + warn!("unable to remove invalid block: {:?}", remove_err); } - Err(remove_err) => warn!("unable to remove invalid block: {:?}", remove_err), } Err(err) } From 8c9e7b2f1ae93661df2b63dac5d2ed5ccd6b8681 Mon Sep 17 00:00:00 2001 From: Alessandro Passaro Date: Tue, 1 Oct 2024 13:53:12 +0100 Subject: [PATCH 2/2] Clean up ObjectId Debug implementation Signed-off-by: Alessandro Passaro --- mountpoint-s3/src/object.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/mountpoint-s3/src/object.rs b/mountpoint-s3/src/object.rs index 6e8087ff6..cd733b215 100644 --- a/mountpoint-s3/src/object.rs +++ b/mountpoint-s3/src/object.rs @@ -1,14 +1,25 @@ +use std::fmt::Debug; + use mountpoint_s3_client::types::ETag; use crate::sync::Arc; /// Identifier for a specific version of an S3 object. /// Formed by the object key and etag. Holds its components in an [Arc], so it can be cheaply cloned. -#[derive(Clone, Debug, Hash, PartialEq, Eq)] +#[derive(Clone, Hash, PartialEq, Eq)] pub struct ObjectId { inner: Arc, } +impl Debug for ObjectId { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ObjectId") + .field("key", &self.inner.key) + .field("etag", &self.inner.etag) + .finish() + } +} + #[derive(Debug, Hash, PartialEq, Eq)] struct InnerObjectId { key: String,