diff --git a/Cargo.lock b/Cargo.lock index a43820d53..d132573cc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2538,6 +2538,7 @@ dependencies = [ "humansize", "libc", "linked-hash-map", + "log", "metrics", "mountpoint-s3-client", "mountpoint-s3-crt", diff --git a/mountpoint-s3/Cargo.toml b/mountpoint-s3/Cargo.toml index 499c1a50b..16f630437 100644 --- a/mountpoint-s3/Cargo.toml +++ b/mountpoint-s3/Cargo.toml @@ -47,6 +47,7 @@ time = { version = "0.3.37", features = ["macros", "formatting"] } tracing = { version = "0.1.41", features = ["log"] } tracing-log = "0.2.0" tracing-subscriber = { version = "0.3.19", features = ["env-filter"] } +log = "0.4.22" [target.'cfg(target_os = "linux")'.dependencies] procfs = { version = "0.17.0", default-features = false } diff --git a/mountpoint-s3/tests/reftests/harness.rs b/mountpoint-s3/tests/reftests/harness.rs index 045362024..4c42aaff5 100644 --- a/mountpoint-s3/tests/reftests/harness.rs +++ b/mountpoint-s3/tests/reftests/harness.rs @@ -4,6 +4,9 @@ use std::path::{Component, Path, PathBuf}; use std::sync::Arc; use std::time::Duration; +use crate::common::{make_test_filesystem, DirectoryReply, TestS3Filesystem}; +use crate::reftests::generators::{flatten_tree, gen_tree, FileContent, FileSize, Name, TreeNode, ValidName}; +use crate::reftests::reference::{File, Node, Reference}; use fuser::FileType; use futures::future::{BoxFuture, FutureExt}; use mountpoint_s3::fs::{CacheConfig, InodeNo, OpenFlags, ToErrno, FUSE_ROOT_INODE}; @@ -15,10 +18,6 @@ use proptest::prelude::*; use proptest_derive::Arbitrary; use tracing::{debug, debug_span, trace}; -use crate::common::{make_test_filesystem, DirectoryReply, TestS3Filesystem}; -use crate::reftests::generators::{flatten_tree, gen_tree, FileContent, FileSize, Name, TreeNode, ValidName}; -use crate::reftests::reference::{File, Node, Reference}; - /// Operations that the mutating proptests can perform on the file system. // TODO: "reboot" (forget all the local inodes and re-bootstrap) #[derive(Debug, Arbitrary)] @@ -157,6 +156,17 @@ impl InflightWrites { let index = self.index(index).unwrap(); self.writes.remove(index) } + + // Invalidates all inflight writes that are shadowed / affected by a PutObject request. + fn invalidate(&mut self, path: PathBuf) { + debug!("writes has {} entries ", self.writes.len()); + debug!("path: {:?}", path); + for write in &mut self.writes { + debug!("write with path {}", write.path.display()); + } + self.writes.retain(|write| write.path.starts_with(&path)); + debug!("writes has {} entries", self.writes.len()); + } } #[derive(Debug)] @@ -576,7 +586,12 @@ impl Harness { self.client.add_object(&key, object.clone()); self.reference.add_remote_key(&key, object); // Any local directories along the path are made remote by adding this object - self.reference.remove_local_parents(key_as_path); + self.reference.remove_local_parents(key_as_path.clone()); + // If we overwrite a file that is currently part of an inflight write + // we have to remove that inflight write, as otherwise we fail when opening the + // ino to continue writing. + debug!("invalidating"); + self.inflight_writes.invalidate(key_as_path); } /// Perform a DeleteObject on the bucket, to simulate concurrent access to the bucket by a @@ -1330,4 +1345,25 @@ mod mutations { 0, ) } + + #[test] + fn regression_stale_ino() { + run_test( + TreeNode::Directory(BTreeMap::from([])), + vec![ + Op::CreateFile( + ValidName("a".into()), + DirectoryIndex(0), + FileContent(0, FileSize::Small(0)), + ), + Op::PutObject( + DirectoryIndex(0), + Name("a/a".into()), + FileContent(0, FileSize::Small(0)), + ), + Op::FinishWrite(InflightWriteIndex(0)), + ], + 0, + ); + } }