Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add negative metadata cache ttl #1246

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Add negative metadata cache ttl #1246

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
28838e4
add negative metadata cache ttl
notoriaga Jan 24, 2025
e3b600a
add negative_cache_ttl to other tests
notoriaga Jan 27, 2025
52774f1
Update mountpoint-s3/src/fs/config.rs
notoriaga Jan 31, 2025
3bd3836
Update mountpoint-s3/tests/fuse_tests/lookup_test.rs
notoriaga Jan 31, 2025
26e8c88
rename negative_cache_ttl to negative_metadata_ttl
notoriaga Jan 31, 2025
9accfa1
add fuse lookup test
notoriaga Feb 1, 2025
73016b8
update config struct to make cli flag name change
notoriaga Feb 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions mountpoint-s3/src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,14 @@ Learn more in Mountpoint's configuration documentation (CONFIGURATION.md).\
)]
pub metadata_ttl: Option<TimeToLive>,

#[clap(
long,
help = "Time-to-live (TTL) for cached negative entries in seconds [default: same as metadata TTL]",
value_name = "SECONDS|indefinite|minimal",
help_heading = CACHING_OPTIONS_HEADER,
)]
pub negative_cache_ttl: Option<TimeToLive>,
notoriaga marked this conversation as resolved.
Show resolved Hide resolved

#[clap(
long,
help = "Maximum size of the cache directory in MiB [default: preserve 5% of available space]",
Expand Down Expand Up @@ -934,6 +942,11 @@ where
}
tracing::trace!("using metadata TTL setting {metadata_cache_ttl:?}");
filesystem_config.cache_config = CacheConfig::new(metadata_cache_ttl);
if let Some(negative_cache_ttl) = args.negative_cache_ttl {
filesystem_config.cache_config = filesystem_config
.cache_config
.with_negative_cache_ttl(negative_cache_ttl);
}

match (args.disk_data_cache_config(), args.express_data_cache_config()) {
(None, Some((config, bucket_name, cache_bucket_name))) => {
Expand Down
22 changes: 22 additions & 0 deletions mountpoint-s3/src/fs/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ pub struct CacheConfig {
pub file_ttl: Duration,
/// How long the kernel will cache metadata for directories
pub dir_ttl: Duration,
/// How long the kernel will cache negative entries
notoriaga marked this conversation as resolved.
Show resolved Hide resolved
pub negative_cache_ttl: Duration,
/// Maximum number of negative entries to cache.
pub negative_cache_size: usize,
}
Expand Down Expand Up @@ -116,6 +118,7 @@ impl Default for CacheConfig {
serve_lookup_from_cache: false,
file_ttl,
dir_ttl,
negative_cache_ttl: file_ttl,
negative_cache_size,
}
}
Expand All @@ -130,14 +133,33 @@ impl CacheConfig {
serve_lookup_from_cache: true,
file_ttl: TimeToLive::INDEFINITE_DURATION,
dir_ttl: TimeToLive::INDEFINITE_DURATION,
negative_cache_ttl: TimeToLive::INDEFINITE_DURATION,
..Default::default()
},
TimeToLive::Duration(ttl) => Self {
serve_lookup_from_cache: true,
file_ttl: ttl,
dir_ttl: ttl,
negative_cache_ttl: ttl,
..Default::default()
},
}
}

pub fn with_negative_cache_ttl(self, negative_cache_ttl: TimeToLive) -> Self {
match negative_cache_ttl {
TimeToLive::Minimal => Self {
negative_cache_ttl: Self::default().negative_cache_ttl,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just setting the TTL here may not be enough. We use the same serve_lookup_from_cache flag both for the "positive" cache and the negative cache (here, in particular). I suspect we need to introduce a separate flag and ensure it is set to false on Minimal here.

..self
},
TimeToLive::Indefinite => Self {
negative_cache_ttl: TimeToLive::INDEFINITE_DURATION,
..self
},
TimeToLive::Duration(ttl) => Self {
negative_cache_ttl: ttl,
..self
},
}
}
}
6 changes: 5 additions & 1 deletion mountpoint-s3/src/superblock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,10 @@ impl Superblock {
let mut inodes = InodeMap::default();
inodes.insert(root.ino(), root);

let negative_cache = NegativeCache::new(config.cache_config.negative_cache_size, config.cache_config.file_ttl);
let negative_cache = NegativeCache::new(
config.cache_config.negative_cache_size,
config.cache_config.negative_cache_ttl,
);

let inner = SuperblockInner {
bucket: bucket.to_owned(),
Expand Down Expand Up @@ -1386,6 +1389,7 @@ mod tests {
serve_lookup_from_cache: true,
dir_ttl: ttl,
file_ttl: ttl,
negative_cache_ttl: ttl,
..Default::default()
},
s3_personality: S3Personality::Standard,
Expand Down
5 changes: 5 additions & 0 deletions mountpoint-s3/tests/fs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ async fn test_lookup_negative_cached() {
serve_lookup_from_cache: true,
dir_ttl: Duration::from_secs(600),
file_ttl: Duration::from_secs(600),
negative_cache_ttl: Duration::from_secs(600),
..Default::default()
},
..Default::default()
Expand Down Expand Up @@ -267,6 +268,7 @@ async fn test_lookup_then_open_cached() {
serve_lookup_from_cache: true,
dir_ttl: Duration::from_secs(600),
file_ttl: Duration::from_secs(600),
negative_cache_ttl: Duration::from_secs(600),
..Default::default()
},
..Default::default()
Expand Down Expand Up @@ -333,6 +335,7 @@ async fn test_readdir_then_open_cached() {
serve_lookup_from_cache: true,
dir_ttl: Duration::from_secs(600),
file_ttl: Duration::from_secs(600),
negative_cache_ttl: Duration::from_secs(600),
..Default::default()
},
..Default::default()
Expand Down Expand Up @@ -384,6 +387,7 @@ async fn test_unlink_cached() {
serve_lookup_from_cache: true,
dir_ttl: Duration::from_secs(600),
file_ttl: Duration::from_secs(600),
negative_cache_ttl: Duration::from_secs(600),
..Default::default()
},
allow_delete: true,
Expand Down Expand Up @@ -434,6 +438,7 @@ async fn test_mknod_cached() {
serve_lookup_from_cache: true,
dir_ttl: Duration::from_secs(600),
file_ttl: Duration::from_secs(600),
negative_cache_ttl: Duration::from_secs(600),
..Default::default()
},
..Default::default()
Expand Down
2 changes: 2 additions & 0 deletions mountpoint-s3/tests/fuse_tests/lookup_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
serve_lookup_from_cache: false,
file_ttl: Duration::ZERO,
dir_ttl: Duration::ZERO,
negative_cache_ttl: Duration::ZERO,
..Default::default()
},
..Default::default()
Expand Down Expand Up @@ -198,6 +199,7 @@
serve_lookup_from_cache: true,
dir_ttl: Duration::from_secs(600),
file_ttl: Duration::from_secs(600),
negative_ttl: Duration::from_secs(600),

Check failure on line 202 in mountpoint-s3/tests/fuse_tests/lookup_test.rs

View workflow job for this annotation

GitHub Actions / Integration / S3 Express One Zone tests (Ubuntu x86, FUSE 2) 

struct `mountpoint_s3::fs::CacheConfig` has no field named `negative_ttl`

Check failure on line 202 in mountpoint-s3/tests/fuse_tests/lookup_test.rs

View workflow job for this annotation

GitHub Actions / Integration / S3 Express One Zone tests (Amazon Linux arm, FUSE 2) 

struct `mountpoint_s3::fs::CacheConfig` has no field named `negative_ttl`

Check failure on line 202 in mountpoint-s3/tests/fuse_tests/lookup_test.rs

View workflow job for this annotation

GitHub Actions / Integration / S3 Express One Zone tests (Ubuntu x86, FUSE 3) 

struct `mountpoint_s3::fs::CacheConfig` has no field named `negative_ttl`

Check failure on line 202 in mountpoint-s3/tests/fuse_tests/lookup_test.rs

View workflow job for this annotation

GitHub Actions / Integration / Tests (Ubuntu x86, FUSE 2) 

struct `mountpoint_s3::fs::CacheConfig` has no field named `negative_ttl`

Check failure on line 202 in mountpoint-s3/tests/fuse_tests/lookup_test.rs

View workflow job for this annotation

GitHub Actions / Integration / Tests (Amazon Linux arm, FUSE 2) 

struct `mountpoint_s3::fs::CacheConfig` has no field named `negative_ttl`

Check failure on line 202 in mountpoint-s3/tests/fuse_tests/lookup_test.rs

View workflow job for this annotation

GitHub Actions / Integration / Tests (Ubuntu x86, FUSE 3) 

struct `mountpoint_s3::fs::CacheConfig` has no field named `negative_ttl`
notoriaga marked this conversation as resolved.
Show resolved Hide resolved
..Default::default()
},
..Default::default()
Expand Down
1 change: 1 addition & 0 deletions mountpoint-s3/tests/reftests/harness.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -936,6 +936,7 @@ mod mutations {
serve_lookup_from_cache: false,
dir_ttl: Duration::ZERO,
file_ttl: Duration::ZERO,
negative_cache_ttl: Duration::ZERO,
..Default::default()
},
..Default::default()
Expand Down
Loading