Skip to content

Commit

Permalink
Add failing proptest
Browse files Browse the repository at this point in the history
Adds a failing proptest, which occured as we did not remove inflight writes to a file, if it was overriden by a PutObject. Fixes this by invalidating such writes.
Signed-off-by: Christian Hagemeier <[email protected]>
  • Loading branch information
c-hagem committed Jan 15, 2025
1 parent 456c7de commit b654a0d
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 5 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions mountpoint-s3/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
46 changes: 41 additions & 5 deletions mountpoint-s3/tests/reftests/harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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)]
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
);
}
}

0 comments on commit b654a0d

Please sign in to comment.