diff --git a/mountpoint-s3/src/data_cache.rs b/mountpoint-s3/src/data_cache.rs index f3d031d54..7d17afbf2 100644 --- a/mountpoint-s3/src/data_cache.rs +++ b/mountpoint-s3/src/data_cache.rs @@ -32,8 +32,8 @@ pub enum DataCacheError { IoFailure(#[source] anyhow::Error), #[error("Block header was not valid: {0}")] InvalidBlockHeader(String), - #[error("Block checksum was not present")] - BlockChecksumMissing, + #[error("Block checksum was not valid")] + InvalidBlockChecksum, #[error("Block content was not valid/readable")] InvalidBlockContent, #[error("Block offset does not match block index")] @@ -42,6 +42,19 @@ pub enum DataCacheError { EvictionFailure, } +impl DataCacheError { + fn get_reason(&self) -> &'static str { + match self { + DataCacheError::IoFailure(_) => "io_failure", + DataCacheError::InvalidBlockHeader(_) => "invalid_block_header", + DataCacheError::InvalidBlockChecksum => "invalid_block_checksum", + DataCacheError::InvalidBlockContent => "invalid_block_content", + DataCacheError::InvalidBlockOffset => "invalid_block_offset", + DataCacheError::EvictionFailure => "eviction_failure", + } + } +} + pub type DataCacheResult = Result; /// Data cache for fixed-size checksummed buffers. diff --git a/mountpoint-s3/src/data_cache/express_data_cache.rs b/mountpoint-s3/src/data_cache/express_data_cache.rs index 948aaf894..4514ba363 100644 --- a/mountpoint-s3/src/data_cache/express_data_cache.rs +++ b/mountpoint-s3/src/data_cache/express_data_cache.rs @@ -104,14 +104,14 @@ where block_idx: BlockIndex, block_offset: u64, object_size: usize, - ) -> Result, (DataCacheError, &'static str)> { + ) -> Result, DataCacheError> { if object_size > self.config.max_object_size { metrics::counter!("express_data_cache.over_max_object_size", "type" => "read").increment(1); return Ok(None); } if block_offset != block_idx * self.config.block_size { - return Err((DataCacheError::InvalidBlockOffset, "invalid_block_offset")); + return Err(DataCacheError::InvalidBlockOffset); } let object_key = get_s3_key(&self.prefix, cache_key, block_idx); @@ -129,7 +129,7 @@ where return Ok(None); } Err(e) => { - return Err((DataCacheError::IoFailure(e.into()), "get_object_failure")); + return Err(DataCacheError::IoFailure(e.into())); } }; @@ -142,7 +142,7 @@ where match chunk { Ok((offset, body)) => { if offset != buffer.len() as u64 { - return Err((DataCacheError::InvalidBlockOffset, "invalid_block_offset")); + return Err(DataCacheError::InvalidBlockOffset); } buffer = if buffer.is_empty() { @@ -161,7 +161,7 @@ where return Ok(None); } Err(e) => { - return Err((DataCacheError::IoFailure(e.into()), "get_object_failure")); + return Err(DataCacheError::IoFailure(e.into())); } } } @@ -170,17 +170,16 @@ where let checksum = result .get_object_checksum() - .map_err(|err| (DataCacheError::IoFailure(err.into()), "invalid_block_checksum"))?; + .map_err(|_| DataCacheError::InvalidBlockChecksum)?; let crc32c_b64 = checksum .checksum_crc32c - .ok_or_else(|| (DataCacheError::BlockChecksumMissing, "missing_block_checksum"))?; + .ok_or_else(|| DataCacheError::InvalidBlockChecksum)?; let crc32c = crc32c_from_base64(&crc32c_b64) - .map_err(|err| (DataCacheError::IoFailure(err.into()), "unparsable_block_checksum"))?; + .map_err(|_| DataCacheError::InvalidBlockChecksum)?; let block_metadata = BlockMetadata::new(block_idx, block_offset, cache_key, &self.bucket_name, crc32c); block_metadata - .validate_object_metadata(&object_metadata) - .map_err(|err| (err, "invalid_block_metadata"))?; + .validate_object_metadata(&object_metadata)?; Ok(Some(ChecksummedBytes::new_from_inner_data(buffer, crc32c))) } @@ -192,21 +191,21 @@ where block_offset: u64, bytes: ChecksummedBytes, object_size: usize, - ) -> Result<(), (DataCacheError, &'static str)> { + ) -> Result<(), DataCacheError> { if object_size > self.config.max_object_size { metrics::counter!("express_data_cache.over_max_object_size", "type" => "write").increment(1); return Ok(()); } if block_offset != block_idx * self.config.block_size { - return Err((DataCacheError::InvalidBlockOffset, "invalid_block_offset")); + return Err(DataCacheError::InvalidBlockOffset); } let object_key = get_s3_key(&self.prefix, &cache_key, block_idx); let (data, checksum) = bytes .into_inner() - .map_err(|_| (DataCacheError::InvalidBlockContent, "invalid_block_content"))?; + .map_err(|_| DataCacheError::InvalidBlockContent)?; let block_metadata = BlockMetadata::new(block_idx, block_offset, &cache_key, &self.bucket_name, checksum); self.client @@ -218,7 +217,7 @@ where ) .in_current_span() .await - .map_err(|err| (err.into(), "put_object_failure"))?; + .map_err(|err| DataCacheError::IoFailure(err.into()))?; Ok(()) } @@ -241,15 +240,15 @@ where Ok(Some(data)) => { metrics::counter!("express_data_cache.block_hit").increment(1); metrics::counter!("express_data_cache.total_bytes", "type" => "read").increment(data.len() as u64); - (Ok(Some(data)), "hit") + (Ok(Some(data)), "ok") } Ok(None) => { metrics::counter!("express_data_cache.block_hit").increment(0); (Ok(None), "miss") } - Err((err, reason)) => { + Err(err) => { metrics::counter!("express_data_cache.block_hit").increment(0); - metrics::counter!("express_data_cache.block_err", "reason" => reason, "type" => "read").increment(1); + metrics::counter!("express_data_cache.block_err", "reason" => err.get_reason(), "type" => "read").increment(1); (Err(err), "error") } }; @@ -273,10 +272,10 @@ where { Ok(()) => { metrics::counter!("express_data_cache.total_bytes", "type" => "write").increment(object_size as u64); - (Ok(()), "hit") + (Ok(()), "ok") } - Err((err, reason)) => { - metrics::counter!("express_data_cache.block_err", "reason" => reason, "type" => "write").increment(1); + Err(err) => { + metrics::counter!("express_data_cache.block_err", "reason" => err.get_reason(), "type" => "write").increment(1); (Err(err), "error") } }; @@ -611,7 +610,7 @@ mod tests { .get_block(&cache_key, 0, 0, data.len()) .await .expect_err("cache should return error if checksum isn't present"); - assert!(matches!(err, DataCacheError::BlockChecksumMissing)); + assert!(matches!(err, DataCacheError::InvalidBlockChecksum)); // Remove the object metadata when writing. client