From fa0d516ccb71c09881ef4e87fd87b396853b1a00 Mon Sep 17 00:00:00 2001 From: Daniel Carl Jones Date: Wed, 25 Oct 2023 15:20:02 +0100 Subject: [PATCH] Add request count tests for FS operations with metadata caching enabled (#567) * Add counters to MockClient for testing request counts Signed-off-by: Daniel Carl Jones * Add tests verifying request counts after readdir,open,lookup,unlink with caching enabled Signed-off-by: Daniel Carl Jones * Appease rustc warnings Signed-off-by: Daniel Carl Jones * Appease clippy warnings Signed-off-by: Daniel Carl Jones * Update OperationCounter::count rustdoc Signed-off-by: Daniel Carl Jones * Add test verifying request counts on open when cache is off Signed-off-by: Daniel Carl Jones --------- Signed-off-by: Daniel Carl Jones --- mountpoint-s3-client/src/mock_client.rs | 86 +++++++- mountpoint-s3/tests/fs.rs | 257 +++++++++++++++++++++++- 2 files changed, 340 insertions(+), 3 deletions(-) diff --git a/mountpoint-s3-client/src/mock_client.rs b/mountpoint-s3-client/src/mock_client.rs index d80c1eea2..320b30ae9 100644 --- a/mountpoint-s3-client/src/mock_client.rs +++ b/mountpoint-s3-client/src/mock_client.rs @@ -1,7 +1,7 @@ //! A mock implementation of an object client for use in tests. use std::borrow::Cow; -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap}; use std::ops::Range; use std::pin::Pin; use std::sync::{Arc, RwLock}; @@ -60,6 +60,7 @@ pub struct MockClient { config: MockClientConfig, objects: Arc>>, in_progress_uploads: Arc>>, + operation_counts: Arc>>, } fn add_object(objects: &Arc>>, key: &str, value: MockObject) { @@ -73,6 +74,7 @@ impl MockClient { config, objects: Default::default(), in_progress_uploads: Default::default(), + operation_counts: Default::default(), } } @@ -134,6 +136,55 @@ impl MockClient { Err(MockClientError("object not found".into())) } } + + /// Create a new counter for the given operation, starting at 0. + pub fn new_counter(&self, operation: Operation) -> OperationCounter<'_> { + let op_counts = self.operation_counts.read().unwrap(); + let initial_count = op_counts.get(&operation).copied().unwrap_or_default(); + + OperationCounter { + client: self, + initial_count, + operation, + } + } + + /// Track number of operations for verifying API calls made by the client in testing. + fn inc_op_count(&self, operation: Operation) { + let mut op_counts = self.operation_counts.write().unwrap(); + op_counts.entry(operation).and_modify(|count| *count += 1).or_insert(1); + } +} + +/// Operations for use in operation counters. +#[derive(Debug, Eq, Hash, PartialEq)] +pub enum Operation { + DeleteObject, + HeadObject, + GetObject, + GetObjectAttributes, + ListObjectsV2, + PutObject, +} + +/// Counter for a specific client [Operation]. +/// +/// Obtainable via `new_counter(&Operation)` method on [MockClient] +/// Its lifetime is bounded by the client which created it. +pub struct OperationCounter<'a> { + client: &'a MockClient, + initial_count: u64, + operation: Operation, +} + +impl<'a> OperationCounter<'a> { + /// Return number of requests since the counter was created. + /// The counter is **not** reset when read. + pub fn count(&self) -> u64 { + let op_counts = self.client.operation_counts.read().unwrap(); + let total_count = op_counts.get(&self.operation).copied().unwrap_or_default(); + total_count - self.initial_count + } } #[derive(Clone)] @@ -309,6 +360,7 @@ impl ObjectClient for MockClient { key: &str, ) -> ObjectClientResult { trace!(bucket, key, "DeleteObject"); + self.inc_op_count(Operation::DeleteObject); if bucket != self.config.bucket { return Err(ObjectClientError::ServiceError(DeleteObjectError::NoSuchBucket)); @@ -327,6 +379,7 @@ impl ObjectClient for MockClient { if_match: Option, ) -> ObjectClientResult { trace!(bucket, key, ?range, ?if_match, "GetObject"); + self.inc_op_count(Operation::GetObject); if bucket != self.config.bucket { return Err(ObjectClientError::ServiceError(GetObjectError::NoSuchBucket)); @@ -367,6 +420,7 @@ impl ObjectClient for MockClient { key: &str, ) -> ObjectClientResult { trace!(bucket, key, "HeadObject"); + self.inc_op_count(Operation::HeadObject); if bucket != self.config.bucket { return Err(ObjectClientError::ServiceError(HeadObjectError::NotFound)); @@ -399,6 +453,7 @@ impl ObjectClient for MockClient { prefix: &str, ) -> ObjectClientResult { trace!(bucket, ?continuation_token, delimiter, max_keys, prefix, "ListObjects"); + self.inc_op_count(Operation::ListObjectsV2); if bucket != self.config.bucket { return Err(ObjectClientError::ServiceError(ListObjectsError::NoSuchBucket)); @@ -499,6 +554,7 @@ impl ObjectClient for MockClient { params: &PutObjectParams, ) -> ObjectClientResult { trace!(bucket, key, "PutObject"); + self.inc_op_count(Operation::PutObject); if bucket != self.config.bucket { return Err(ObjectClientError::ServiceError(PutObjectError::NoSuchBucket)); @@ -523,6 +579,7 @@ impl ObjectClient for MockClient { object_attributes: &[ObjectAttribute], ) -> ObjectClientResult { trace!(bucket, key, "GetObjectAttributes"); + self.inc_op_count(Operation::GetObjectAttributes); if bucket != self.config.bucket { return Err(ObjectClientError::ServiceError(GetObjectAttributesError::NoSuchBucket)); @@ -960,4 +1017,31 @@ mod tests { matches!(&list_result.objects[..], [object] if object.key == key && object.storage_class.as_deref() == storage_class ) ); } + + #[tokio::test] + async fn counter_test() { + let bucket = "test_bucket"; + let client = MockClient::new(MockClientConfig { + bucket: bucket.to_owned(), + part_size: 1024, + }); + + let head_counter_1 = client.new_counter(Operation::HeadObject); + let delete_counter_1 = client.new_counter(Operation::DeleteObject); + + let _result = client.head_object(bucket, "key").await; + assert_eq!(1, head_counter_1.count()); + assert_eq!(0, delete_counter_1.count()); + + let head_counter_2 = client.new_counter(Operation::HeadObject); + assert_eq!(0, head_counter_2.count()); + + let _result = client.head_object(bucket, "key").await; + let _result = client.delete_object(bucket, "key").await; + let _result = client.delete_object(bucket, "key").await; + let _result = client.delete_object(bucket, "key").await; + assert_eq!(2, head_counter_1.count()); + assert_eq!(3, delete_counter_1.count()); + assert_eq!(1, head_counter_2.count()); + } } diff --git a/mountpoint-s3/tests/fs.rs b/mountpoint-s3/tests/fs.rs index 59d86cf56..307157dbf 100644 --- a/mountpoint-s3/tests/fs.rs +++ b/mountpoint-s3/tests/fs.rs @@ -1,10 +1,12 @@ //! Manually implemented tests executing the FUSE protocol against [S3Filesystem] use fuser::FileType; -use mountpoint_s3::fs::{ToErrno, FUSE_ROOT_INODE}; +use libc::S_IFREG; +use mountpoint_s3::fs::{CacheConfig, ToErrno, FUSE_ROOT_INODE}; use mountpoint_s3::prefix::Prefix; +use mountpoint_s3::S3FilesystemConfig; use mountpoint_s3_client::failure_client::countdown_failure_client; -use mountpoint_s3_client::mock_client::{MockClient, MockClientConfig, MockClientError, MockObject}; +use mountpoint_s3_client::mock_client::{MockClient, MockClientConfig, MockClientError, MockObject, Operation}; use mountpoint_s3_client::types::{ETag, RestoreStatus}; use mountpoint_s3_client::ObjectClient; use nix::unistd::{getgid, getuid}; @@ -164,6 +166,257 @@ async fn test_read_dir_nested(prefix: &str) { fs.releasedir(dir_ino, dir_handle, 0).await.unwrap(); } +#[tokio::test] +async fn test_lookup_negative_cached() { + let fs_config = S3FilesystemConfig { + cache_config: CacheConfig { + serve_lookup_from_cache: true, + dir_ttl: Duration::from_secs(600), + file_ttl: Duration::from_secs(600), + }, + ..Default::default() + }; + let (client, fs) = make_test_filesystem("test_lookup_negative_cached", &Default::default(), fs_config); + + let head_counter = client.new_counter(Operation::HeadObject); + let list_counter = client.new_counter(Operation::ListObjectsV2); + + let _ = fs + .lookup(FUSE_ROOT_INODE, "file1.txt".as_ref()) + .await + .expect_err("should fail as no object exists"); + assert_eq!(head_counter.count(), 1); + assert_eq!(list_counter.count(), 1); + + // Check no negative caching + let _ = fs + .lookup(FUSE_ROOT_INODE, "file1.txt".as_ref()) + .await + .expect_err("should fail as no object exists"); + assert_eq!(head_counter.count(), 2); + assert_eq!(list_counter.count(), 2); + + client.add_object("file1.txt", MockObject::constant(0xa1, 15, ETag::for_tests())); + + let _ = fs + .lookup(FUSE_ROOT_INODE, "file1.txt".as_ref()) + .await + .expect("should succeed as object exists and no negative cache"); + assert_eq!(head_counter.count(), 3); + assert_eq!(list_counter.count(), 3); + + let _ = fs + .lookup(FUSE_ROOT_INODE, "file1.txt".as_ref()) + .await + .expect("should succeed as object is cached and exists"); + assert_eq!(head_counter.count(), 3); + assert_eq!(list_counter.count(), 3); +} + +#[tokio::test] +async fn test_lookup_then_open_cached() { + let fs_config = S3FilesystemConfig { + cache_config: CacheConfig { + serve_lookup_from_cache: true, + dir_ttl: Duration::from_secs(600), + file_ttl: Duration::from_secs(600), + }, + ..Default::default() + }; + let (client, fs) = make_test_filesystem("test_lookup_then_open_cached", &Default::default(), fs_config); + + client.add_object("file1.txt", MockObject::constant(0xa1, 15, ETag::for_tests())); + + let head_counter = client.new_counter(Operation::HeadObject); + let list_counter = client.new_counter(Operation::ListObjectsV2); + + let entry = fs.lookup(FUSE_ROOT_INODE, "file1.txt".as_ref()).await.unwrap(); + let ino = entry.attr.ino; + assert_eq!(head_counter.count(), 1); + assert_eq!(list_counter.count(), 1); + + let fh = fs.open(ino, S_IFREG as i32, 0).await.unwrap().fh; + fs.release(ino, fh, 0, None, true).await.unwrap(); + assert_eq!(head_counter.count(), 1); + assert_eq!(list_counter.count(), 1); + + let fh = fs.open(entry.attr.ino, S_IFREG as i32, 0).await.unwrap().fh; + fs.release(ino, fh, 0, None, true).await.unwrap(); + assert_eq!(head_counter.count(), 1); + assert_eq!(list_counter.count(), 1); +} + +#[tokio::test] +async fn test_lookup_then_open_no_cache() { + let fs_config = S3FilesystemConfig { + cache_config: CacheConfig { + serve_lookup_from_cache: false, + ..Default::default() + }, + ..Default::default() + }; + let (client, fs) = make_test_filesystem("test_lookup_then_open_no_cache", &Default::default(), fs_config); + + client.add_object("file1.txt", MockObject::constant(0xa1, 15, ETag::for_tests())); + + let head_counter = client.new_counter(Operation::HeadObject); + let list_counter = client.new_counter(Operation::ListObjectsV2); + + let entry = fs.lookup(FUSE_ROOT_INODE, "file1.txt".as_ref()).await.unwrap(); + let ino = entry.attr.ino; + assert_eq!(head_counter.count(), 1); + assert_eq!(list_counter.count(), 1); + + let fh = fs.open(ino, S_IFREG as i32, 0).await.unwrap().fh; + fs.release(ino, fh, 0, None, true).await.unwrap(); + assert_eq!(head_counter.count(), 2); + assert_eq!(list_counter.count(), 2); + + let fh = fs.open(entry.attr.ino, S_IFREG as i32, 0).await.unwrap().fh; + fs.release(ino, fh, 0, None, true).await.unwrap(); + assert_eq!(head_counter.count(), 3); + assert_eq!(list_counter.count(), 3); +} + +#[tokio::test] +async fn test_readdir_then_open_cached() { + let fs_config = S3FilesystemConfig { + cache_config: CacheConfig { + serve_lookup_from_cache: true, + dir_ttl: Duration::from_secs(600), + file_ttl: Duration::from_secs(600), + }, + ..Default::default() + }; + let (client, fs) = make_test_filesystem("test_readdir_then_open_cached", &Default::default(), fs_config); + + client.add_object("file1.txt", MockObject::constant(0xa1, 15, ETag::for_tests())); + + // Repeat to check readdir is not currently served from cache + for _ in 0..2 { + let head_counter = client.new_counter(Operation::HeadObject); + let list_counter = client.new_counter(Operation::ListObjectsV2); + + let dir_ino = FUSE_ROOT_INODE; + let dir_handle = fs.opendir(dir_ino, 0).await.unwrap().fh; + let mut reply = Default::default(); + let _reply = fs.readdirplus(dir_ino, dir_handle, 0, &mut reply).await.unwrap(); + + assert_eq!(reply.entries.len(), 2 + 1); + + let mut entries = reply.entries.iter(); + let _ = entries.next().expect("should have current directory"); + let _ = entries.next().expect("should have parent directory"); + + let entry = entries.next().expect("should have file1.txt in entries"); + let expected = OsString::from("file1.txt"); + assert_eq!(entry.name, expected); + assert_eq!(entry.attr.kind, FileType::RegularFile); + + assert_eq!(head_counter.count(), 0); + assert_eq!(list_counter.count(), 1); + + let fh = fs.open(entry.ino, S_IFREG as i32, 0).await.unwrap().fh; + + assert_eq!(head_counter.count(), 0); + assert_eq!(list_counter.count(), 1); + fs.release(entry.ino, fh, 0, None, true).await.unwrap(); + fs.releasedir(dir_ino, dir_handle, 0).await.unwrap(); + + assert_eq!(head_counter.count(), 0); + assert_eq!(list_counter.count(), 1); + } +} + +#[tokio::test] +async fn test_unlink_cached() { + let fs_config = S3FilesystemConfig { + cache_config: CacheConfig { + serve_lookup_from_cache: true, + dir_ttl: Duration::from_secs(600), + file_ttl: Duration::from_secs(600), + }, + allow_delete: true, + ..Default::default() + }; + let (client, fs) = make_test_filesystem("test_lookup_then_open_cached", &Default::default(), fs_config); + + client.add_object("file1.txt", MockObject::constant(0xa1, 15, ETag::for_tests())); + + let parent_ino = FUSE_ROOT_INODE; + let head_counter = client.new_counter(Operation::HeadObject); + let list_counter = client.new_counter(Operation::ListObjectsV2); + + let _entry = fs + .lookup(parent_ino, "file1.txt".as_ref()) + .await + .expect("should find file as object exists"); + assert_eq!(head_counter.count(), 1); + assert_eq!(list_counter.count(), 1); + + client.remove_object("file1.txt"); + let _entry = fs + .lookup(parent_ino, "file1.txt".as_ref()) + .await + .expect("lookup should still see obj"); + assert_eq!(head_counter.count(), 1); + assert_eq!(list_counter.count(), 1); + + fs.unlink(parent_ino, "file1.txt".as_ref()) + .await + .expect("unlink should unlink cached object"); + assert_eq!(head_counter.count(), 1); + assert_eq!(list_counter.count(), 1); + + let _entry = fs + .lookup(parent_ino, "file1.txt".as_ref()) + .await + .expect_err("cached entry should now be gone"); + assert_eq!(head_counter.count(), 2); + assert_eq!(list_counter.count(), 2); +} + +#[tokio::test] +async fn test_mknod_cached() { + const BUCKET_NAME: &str = "test_mknod_cached"; + let fs_config = S3FilesystemConfig { + cache_config: CacheConfig { + serve_lookup_from_cache: true, + dir_ttl: Duration::from_secs(600), + file_ttl: Duration::from_secs(600), + }, + ..Default::default() + }; + let (client, fs) = make_test_filesystem(BUCKET_NAME, &Default::default(), fs_config); + + let parent = FUSE_ROOT_INODE; + let head_counter = client.new_counter(Operation::HeadObject); + let list_counter = client.new_counter(Operation::ListObjectsV2); + + client.add_object("file1.txt", MockObject::constant(0xa1, 15, ETag::for_tests())); + + let mode = libc::S_IFREG | libc::S_IRWXU; // regular file + 0700 permissions + let err_no = fs + .mknod(parent, "file1.txt".as_ref(), mode, 0, 0) + .await + .expect_err("file already exists") + .to_errno(); + assert_eq!(err_no, libc::EEXIST, "expected EEXIST but got {:?}", err_no); + assert_eq!(head_counter.count(), 1); + assert_eq!(list_counter.count(), 1); + + client.remove_object("file1.txt"); + + let err_no = fs + .mknod(parent, "file1.txt".as_ref(), mode, 0, 0) + .await + .expect_err("should fail as directory entry still cached") + .to_errno(); + assert_eq!(err_no, libc::EEXIST, "expected EEXIST but got {:?}", err_no); + assert_eq!(head_counter.count(), 1); + assert_eq!(list_counter.count(), 1); +} + #[test_case(1024 * 1024; "small")] #[test_case(50 * 1024 * 1024; "large")] #[tokio::test]