diff --git a/Cargo.lock b/Cargo.lock index f41936bea..e35c80380 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1601,15 +1601,6 @@ dependencies = [ "libc", ] -[[package]] -name = "num_threads" -version = "0.1.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2819ce041d2ee131036f4fc9d6ae7ae125a3a40e97ba64d04fe799ad9dabbb44" -dependencies = [ - "libc", -] - [[package]] name = "object" version = "0.29.0" @@ -2170,6 +2161,7 @@ dependencies = [ "tempfile", "test-case", "thiserror", + "time", "tokio", "tracing", "tracing-subscriber", @@ -2509,12 +2501,10 @@ dependencies = [ [[package]] name = "time" -version = "0.3.16" +version = "0.3.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0fab5c8b9980850e06d92ddbe3ab839c062c801f3927c0fb8abd6fc8e918fbca" +checksum = "a561bf4617eebd33bca6434b988f39ed798e527f51a1e797d0ee4f61c0a38376" dependencies = [ - "libc", - "num_threads", "serde", "time-core", "time-macros", @@ -2528,9 +2518,9 @@ checksum = "2e153e1f1acaef8acc537e68b44906d2db6436e2b35ac2c6b42640fff91f00fd" [[package]] name = "time-macros" -version = "0.2.5" +version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "65bb801831d812c562ae7d2bfb531f26e66e4e1f6b17307ba4149c5064710e5b" +checksum = "d967f99f534ca7e495c575c62638eebc2898a8c84c119b89e250477bc4ba16b2" dependencies = [ "time-core", ] diff --git a/s3-client/src/mock_client.rs b/s3-client/src/mock_client.rs index 88ce2e734..d6b0b9a73 100644 --- a/s3-client/src/mock_client.rs +++ b/s3-client/src/mock_client.rs @@ -71,6 +71,7 @@ impl MockClient { pub struct MockObject { generator: Box Box<[u8]> + Send + Sync>, size: usize, + pub last_modified: OffsetDateTime, } impl MockObject { @@ -84,6 +85,7 @@ impl MockObject { Self { size: bytes.len(), generator: Box::new(move |offset, size| bytes[offset as usize..offset as usize + size].into()), + last_modified: OffsetDateTime::now_utc(), } } @@ -91,6 +93,7 @@ impl MockObject { Self { generator: Box::new(move |_offset, size| vec![v; size].into_boxed_slice()), size, + last_modified: OffsetDateTime::now_utc(), } } @@ -108,9 +111,14 @@ impl MockObject { vec.into_boxed_slice() }), size, + last_modified: OffsetDateTime::now_utc(), } } + pub fn set_last_modified(&mut self, last_modified: OffsetDateTime) { + self.last_modified = last_modified; + } + pub fn len(&self) -> usize { self.size } @@ -244,8 +252,7 @@ impl ObjectClient for MockClient { object: ObjectInfo { key: key.to_string(), size: object.size as u64, - // TODO real mtimes - last_modified: OffsetDateTime::from_unix_timestamp(0).unwrap(), + last_modified: object.last_modified, etag: "TODO".to_string(), storage_class: None, }, @@ -283,7 +290,7 @@ impl ObjectClient for MockClient { // start at the beginning of the bucket. let object_iterator = objects.range(continuation_token.unwrap_or("").to_string()..); - for (key, value) in object_iterator { + for (key, object) in object_iterator { // If the prefix is `n` characters long, and we encounter a key whose first `n` // characters are lexicographically larger than the prefix, then we can stop iterating. // Note that we cannot just do a direct comparison between the full key and prefix. For @@ -340,9 +347,8 @@ impl ObjectClient for MockClient { } else { object_vec.push(ObjectInfo { key: key.to_string(), - size: value.len() as u64, - // TODO real mtimes - last_modified: OffsetDateTime::from_unix_timestamp(0).unwrap(), + size: object.len() as u64, + last_modified: object.last_modified, etag: "TODO".to_string(), storage_class: None, }); diff --git a/s3-file-connector/Cargo.toml b/s3-file-connector/Cargo.toml index 7e1d6c147..6ce043d9e 100644 --- a/s3-file-connector/Cargo.toml +++ b/s3-file-connector/Cargo.toml @@ -25,6 +25,7 @@ thiserror = "1.0.34" tracing = { version = "0.1.35", default-features = false, features = ["std", "log"] } tracing-subscriber = { version = "0.3.14", features = ["fmt", "env-filter"] } nix = { version = "0.26.1", default-features = false, features = ["user"] } +time = "0.3.17" [dev-dependencies] assert_cmd = "2.0.6" diff --git a/s3-file-connector/src/fs.rs b/s3-file-connector/src/fs.rs index 36490424e..bddb2250a 100644 --- a/s3-file-connector/src/fs.rs +++ b/s3-file-connector/src/fs.rs @@ -199,9 +199,9 @@ where ino, size: stat.size as u64, blocks: stat.size as u64 / BLOCK_SIZE, - atime: UNIX_EPOCH, - mtime: UNIX_EPOCH, // TODO - ctime: UNIX_EPOCH, + atime: stat.atime.into(), + mtime: stat.mtime.into(), + ctime: stat.ctime.into(), crtime: UNIX_EPOCH, kind: (&stat.kind).into(), perm, diff --git a/s3-file-connector/src/inode.rs b/s3-file-connector/src/inode.rs index a48cfb4e7..0e4107786 100644 --- a/s3-file-connector/src/inode.rs +++ b/s3-file-connector/src/inode.rs @@ -28,6 +28,7 @@ use fuser::FileType; use futures::{select_biased, FutureExt}; use s3_client::ObjectClient; use thiserror::Error; +use time::OffsetDateTime; use tracing::{error, trace, warn}; use crate::sync::atomic::{AtomicU64, Ordering}; @@ -80,10 +81,7 @@ impl Superblock { // We stash the prefix in the root inode's name so that path resolution "just works" // with prefixes name: stripped_prefix, - stat_cache: RwLock::new(InodeStat { - kind: InodeStatKind::Directory {}, - size: 0, - }), + stat_cache: RwLock::new(InodeStat::for_directory(OffsetDateTime::UNIX_EPOCH)), stat_cache_expiry: Instant::now(), data: InodeData::Directory { children: Default::default(), @@ -174,10 +172,7 @@ impl Superblock { { trace!(?parent, ?name, "unsuffixed lookup found a directory"); - let stat = InodeStat { - kind: InodeStatKind::Directory {}, - size: 0, - }; + let stat = InodeStat::for_directory(OffsetDateTime::UNIX_EPOCH); let ino = self.inner .update_or_insert(parent, name, InodeKind::Directory, stat.clone(), Instant::now())?; @@ -194,10 +189,8 @@ impl Superblock { .map(|object| object.key == full_path.to_str().unwrap()) .unwrap_or(false) { - let stat = InodeStat { - kind: InodeStatKind::File {}, - size: result.objects[0].size as usize, - }; + let last_modified = result.objects[0].last_modified; + let stat = InodeStat::for_file(result.objects[0].size as usize, last_modified); file_state = Some(stat); } } @@ -241,10 +234,7 @@ impl Superblock { if found_directory { trace!(?parent, ?name, kind=?InodeKind::Directory, "suffixed lookup found a directory"); - let stat = InodeStat { - kind: InodeStatKind::Directory {}, - size: 0, - }; + let stat = InodeStat::for_directory(OffsetDateTime::UNIX_EPOCH); let ino = self.inner .update_or_insert(parent, name, InodeKind::Directory, stat.clone(), Instant::now())?; @@ -529,10 +519,7 @@ impl ReaddirHandle { .map(|prefix| OsString::from(&prefix[self.full_path.len()..prefix.len() - 1])) .filter(|name| valid_inode_name(name)) .map(|name| { - let stat = InodeStat { - kind: InodeStatKind::Directory {}, - size: 0, - }; + let stat = InodeStat::for_directory(OffsetDateTime::UNIX_EPOCH); let stat_clone = stat.clone(); self.inner @@ -556,10 +543,8 @@ impl ReaddirHandle { // Hide keys that end with '/', since they can be confused with directories .filter(|(name, _object)| valid_inode_name(name)) .flat_map(|(name, object)| { - let stat = InodeStat { - kind: InodeStatKind::File {}, - size: object.size as usize, - }; + let last_modified = object.last_modified; + let stat = InodeStat::for_file(object.size as usize, last_modified); let stat_clone = stat.clone(); let result = self @@ -674,13 +659,44 @@ enum InodeData { /// Public inode stat data that can expire #[derive(Debug, Clone)] pub struct InodeStat { - // common metadata: mtime, ctime, ... + /// Size in bytes pub size: usize, + /// Time of last file content modification + pub mtime: OffsetDateTime, + /// Time of last file metadata (or content) change + pub ctime: OffsetDateTime, + /// Time of last access + pub atime: OffsetDateTime, + /// Per-kind metadata pub kind: InodeStatKind, } +impl InodeStat { + /// Initialize an [InodeStat] for a file, given some metadata. + pub fn for_file(size: usize, datetime: OffsetDateTime) -> InodeStat { + InodeStat { + size, + atime: datetime, + ctime: datetime, + mtime: datetime, + kind: InodeStatKind::File {}, + } + } + + /// Initialize an [InodeStat] for a directory, given some metadata. + pub fn for_directory(datetime: OffsetDateTime) -> InodeStat { + InodeStat { + size: 0, + atime: datetime, + ctime: datetime, + mtime: datetime, + kind: InodeStatKind::Directory {}, + } + } +} + /// Public inode stat data that can expire and differs by kind #[derive(Debug, PartialEq, Eq, Clone)] pub enum InodeStatKind { @@ -730,17 +746,30 @@ pub struct DirEntryPlus { mod tests { use s3_client::mock_client::{MockClient, MockClientConfig, MockObject}; use test_case::test_case; + use time::{Duration, OffsetDateTime}; use crate::fs::FUSE_ROOT_INODE; use super::*; + /// Check an [InodeStat] matches a series of fields. + macro_rules! assert_inode_stat { + ($stat:expr, $type:expr, $datetime:expr, $size:expr) => { + assert_eq!($stat.kind, $type); + assert_eq!($stat.atime, $datetime); + assert_eq!($stat.ctime, $datetime); + assert_eq!($stat.mtime, $datetime); + assert_eq!($stat.size, $size); + }; + } + #[test_case(""; "unprefixed")] #[test_case("test_prefix/"; "prefixed")] #[tokio::test] async fn test_lookup(prefix: &str) { + let bucket = "test_bucket"; let client_config = MockClientConfig { - bucket: "test_bucket".to_string(), + bucket: bucket.to_string(), part_size: 1024 * 1024, }; let client = Arc::new(MockClient::new(client_config)); @@ -759,11 +788,16 @@ mod tests { format!("{}dir1/sdir3/file1.txt", prefix), ]; + let object_size = 30; + let mut last_modified = OffsetDateTime::UNIX_EPOCH; for key in keys { - client.add_object(key, MockObject::constant(0xaa, 30)); + let mut obj = MockObject::constant(0xaa, object_size); + last_modified += Duration::days(1); + obj.set_last_modified(last_modified); + client.add_object(key, obj); } - let superblock = Superblock::new("test_bucket".to_string(), OsString::from(prefix)); + let superblock = Superblock::new(bucket.to_string(), OsString::from(prefix)); // Try it twice to test the inode reuse path too for _ in 0..2 { @@ -771,46 +805,44 @@ mod tests { .lookup(&client, FUSE_ROOT_INODE, &OsString::from("dir0")) .await .expect("should exist"); - assert_eq!(dir0.stat.kind, InodeStatKind::Directory {}); + assert_inode_stat!(dir0.stat, InodeStatKind::Directory {}, OffsetDateTime::UNIX_EPOCH, 0); assert_eq!(dir0.full_key, OsString::from(format!("{}dir0", prefix))); + let dir1 = superblock .lookup(&client, FUSE_ROOT_INODE, &OsString::from("dir1")) .await .expect("should exist"); - assert_eq!(dir1.stat.kind, InodeStatKind::Directory {}); + assert_inode_stat!(dir1.stat, InodeStatKind::Directory {}, OffsetDateTime::UNIX_EPOCH, 0); assert_eq!(dir1.full_key, OsString::from(format!("{}dir1", prefix))); + let sdir0 = superblock .lookup(&client, dir0.ino, &OsString::from("sdir0")) .await .expect("should exist"); - assert_eq!(sdir0.stat.kind, InodeStatKind::Directory {}); + assert_inode_stat!(sdir0.stat, InodeStatKind::Directory {}, OffsetDateTime::UNIX_EPOCH, 0); assert_eq!(sdir0.full_key, OsString::from(format!("{}dir0/sdir0", prefix))); + let sdir1 = superblock .lookup(&client, dir0.ino, &OsString::from("sdir1")) .await .expect("should exist"); - assert_eq!(sdir1.stat.kind, InodeStatKind::Directory {}); + assert_inode_stat!(sdir1.stat, InodeStatKind::Directory {}, OffsetDateTime::UNIX_EPOCH, 0); assert_eq!(sdir1.full_key, OsString::from(format!("{}dir0/sdir1", prefix))); + let sdir2 = superblock .lookup(&client, dir1.ino, &OsString::from("sdir2")) .await .expect("should exist"); - assert_eq!(sdir2.stat.kind, InodeStatKind::Directory {}); + assert_inode_stat!(sdir2.stat, InodeStatKind::Directory {}, OffsetDateTime::UNIX_EPOCH, 0); assert_eq!(sdir2.full_key, OsString::from(format!("{}dir1/sdir2", prefix))); + let sdir3 = superblock .lookup(&client, dir1.ino, &OsString::from("sdir3")) .await .expect("should exist"); - assert_eq!(sdir3.stat.kind, InodeStatKind::Directory {}); + assert_inode_stat!(sdir3.stat, InodeStatKind::Directory {}, OffsetDateTime::UNIX_EPOCH, 0); assert_eq!(sdir3.full_key, OsString::from(format!("{}dir1/sdir3", prefix))); - let file0 = superblock - .lookup(&client, dir0.ino, &OsString::from("file0.txt")) - .await - .expect("should exist"); - assert_eq!(file0.stat.kind, InodeStatKind::File {}); - assert_eq!(file0.full_key, OsString::from(format!("{}dir0/file0.txt", prefix))); - for (dir, sdir, ino, n) in &[ (0, 0, sdir0.ino, 3), (0, 1, sdir1.ino, 2), @@ -821,7 +853,15 @@ mod tests { let file = superblock .lookup(&client, *ino, &OsString::from(format!("file{}.txt", i))) .await - .expect("should exist"); + .expect("inode should exist"); + // Grab last modified time according to mock S3 + let modified_time = client + .head_object(bucket, file.full_key.to_str().unwrap()) + .await + .expect("object should exist") + .object + .last_modified; + assert_inode_stat!(file.stat, InodeStatKind::File {}, modified_time, object_size); assert_eq!( file.full_key, OsString::from(format!("{}dir{}/sdir{}/file{}.txt", prefix, dir, sdir, i)) @@ -989,4 +1029,23 @@ mod tests { assert!(matches!(lookup, Err(InodeError::InvalidFileName(_)))); } } + + #[test] + fn test_inodestat_constructors() { + let ts = OffsetDateTime::UNIX_EPOCH + Duration::days(90); + let file_inodestat = InodeStat::for_file(128, ts); + assert_eq!(file_inodestat.size, 128); + assert_eq!(file_inodestat.atime, ts); + assert_eq!(file_inodestat.ctime, ts); + assert_eq!(file_inodestat.mtime, ts); + assert_eq!(file_inodestat.kind, InodeStatKind::File {}); + + let ts = OffsetDateTime::UNIX_EPOCH + Duration::days(180); + let file_inodestat = InodeStat::for_directory(ts); + assert_eq!(file_inodestat.size, 0); + assert_eq!(file_inodestat.atime, ts); + assert_eq!(file_inodestat.ctime, ts); + assert_eq!(file_inodestat.mtime, ts); + assert_eq!(file_inodestat.kind, InodeStatKind::Directory {}); + } } diff --git a/s3-file-connector/src/main.rs b/s3-file-connector/src/main.rs index 4f692a8a4..6bc57e973 100644 --- a/s3-file-connector/src/main.rs +++ b/s3-file-connector/src/main.rs @@ -99,6 +99,7 @@ fn main() -> anyhow::Result<()> { MountOption::RO, MountOption::DefaultPermissions, MountOption::FSName("fuse_sync".to_string()), + MountOption::NoAtime, ]; if args.auto_unmount { options.push(MountOption::AutoUnmount);