Skip to content

Commit 5e580a8

Browse files
notoriagapassaro
notoriaga
and
passaro
authored
Add negative metadata cache ttl (#1246)
Adds a new CLI argument `--negative-cache-ttl` that lets you set the TTL for negative metadata entries separately from `--metadata-ttl`. My use case is a write once read many bucket. Objects do not get deleted from this bucket, and new objects are added every few minutes. I'd like to be able to set `--metadata-ttl indefinite` and `--negative-cache-ttl 60` to effectively utilize the caching while still being able to pick up new objects. There is an open issue for this here - #831 ### Does this change impact existing behavior? No, if `--negative-cache-ttl` is omitted the existing behavior is maintained (use `--metadata-ttl` or the default file_ttl). ### Does this change need a changelog entry? Does it require a version change? Because this is a new feature I believe it would require both. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the [Developer Certificate of Origin (DCO)](https://developercertificate.org/). --------- Signed-off-by: notoriaga <[email protected]> Signed-off-by: Steven Meyer <[email protected]> Co-authored-by: Alessandro Passaro <[email protected]>
1 parent 6410b8c commit 5e580a8

File tree

6 files changed

+100
-57
lines changed

6 files changed

+100
-57
lines changed

‎mountpoint-s3/src/cli.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,14 @@ Learn more in Mountpoint's configuration documentation (CONFIGURATION.md).\
315315
)]
316316
pub metadata_ttl: Option<TimeToLive>,
317317

318+
#[clap(
319+
long,
320+
help = "Time-to-live (TTL) for cached negative entries in seconds [default: same as metadata TTL]",
321+
value_name = "SECONDS|indefinite|minimal",
322+
help_heading = CACHING_OPTIONS_HEADER,
323+
)]
324+
pub negative_metadata_ttl: Option<TimeToLive>,
325+
318326
#[clap(
319327
long,
320328
help = "Maximum size of the cache directory in MiB [default: preserve 5% of available space]",
@@ -932,6 +940,11 @@ where
932940
}
933941
tracing::trace!("using metadata TTL setting {metadata_cache_ttl:?}");
934942
filesystem_config.cache_config = CacheConfig::new(metadata_cache_ttl);
943+
if let Some(negative_cache_ttl) = args.negative_metadata_ttl {
944+
filesystem_config.cache_config = filesystem_config
945+
.cache_config
946+
.with_negative_metadata_ttl(negative_cache_ttl);
947+
}
935948

936949
match (args.disk_data_cache_config(), args.express_data_cache_config()) {
937950
(None, Some((config, bucket_name, cache_bucket_name))) => {

‎mountpoint-s3/src/fs/config.rs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,10 @@ pub struct CacheConfig {
8787
pub file_ttl: Duration,
8888
/// How long the kernel will cache metadata for directories
8989
pub dir_ttl: Duration,
90+
/// Should the file system cache negative lookups?
91+
pub use_negative_cache: bool,
92+
/// How long the file system will cache negative entries
93+
pub negative_cache_ttl: Duration,
9094
/// Maximum number of negative entries to cache.
9195
pub negative_cache_size: usize,
9296
}
@@ -116,6 +120,8 @@ impl Default for CacheConfig {
116120
serve_lookup_from_cache: false,
117121
file_ttl,
118122
dir_ttl,
123+
use_negative_cache: false,
124+
negative_cache_ttl: file_ttl,
119125
negative_cache_size,
120126
}
121127
}
@@ -130,14 +136,38 @@ impl CacheConfig {
130136
serve_lookup_from_cache: true,
131137
file_ttl: TimeToLive::INDEFINITE_DURATION,
132138
dir_ttl: TimeToLive::INDEFINITE_DURATION,
139+
use_negative_cache: true,
140+
negative_cache_ttl: TimeToLive::INDEFINITE_DURATION,
133141
..Default::default()
134142
},
135143
TimeToLive::Duration(ttl) => Self {
136144
serve_lookup_from_cache: true,
137145
file_ttl: ttl,
138146
dir_ttl: ttl,
147+
use_negative_cache: true,
148+
negative_cache_ttl: ttl,
139149
..Default::default()
140150
},
141151
}
142152
}
153+
154+
pub fn with_negative_metadata_ttl(self, negative_metadata_ttl: TimeToLive) -> Self {
155+
match negative_metadata_ttl {
156+
TimeToLive::Minimal => Self {
157+
use_negative_cache: false,
158+
negative_cache_ttl: Self::default().negative_cache_ttl,
159+
..self
160+
},
161+
TimeToLive::Indefinite => Self {
162+
use_negative_cache: true,
163+
negative_cache_ttl: TimeToLive::INDEFINITE_DURATION,
164+
..self
165+
},
166+
TimeToLive::Duration(ttl) => Self {
167+
use_negative_cache: true,
168+
negative_cache_ttl: ttl,
169+
..self
170+
},
171+
}
172+
}
143173
}

‎mountpoint-s3/src/superblock.rs‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,10 @@ impl Superblock {
9090
let mut inodes = InodeMap::default();
9191
inodes.insert(root.ino(), root);
9292

93-
let negative_cache = NegativeCache::new(config.cache_config.negative_cache_size, config.cache_config.file_ttl);
93+
let negative_cache = NegativeCache::new(
94+
config.cache_config.negative_cache_size,
95+
config.cache_config.negative_cache_ttl,
96+
);
9497

9598
let inner = SuperblockInner {
9699
bucket: bucket.to_owned(),
@@ -789,7 +792,7 @@ impl SuperblockInner {
789792
return Err(InodeError::NotADirectory(parent.err()));
790793
}
791794

792-
if self.config.cache_config.serve_lookup_from_cache {
795+
if self.config.cache_config.use_negative_cache {
793796
match &remote {
794797
// Remove negative cache entry.
795798
Some(_) => self.negative_cache.remove(parent_ino, name),
@@ -1164,7 +1167,7 @@ mod tests {
11641167
use test_case::test_case;
11651168
use time::{Duration, OffsetDateTime};
11661169

1167-
use crate::fs::{ToErrno, FUSE_ROOT_INODE};
1170+
use crate::fs::{TimeToLive, ToErrno, FUSE_ROOT_INODE};
11681171

11691172
use super::*;
11701173

@@ -1327,12 +1330,7 @@ mod tests {
13271330
bucket,
13281331
&prefix,
13291332
SuperblockConfig {
1330-
cache_config: CacheConfig {
1331-
serve_lookup_from_cache: true,
1332-
dir_ttl: ttl,
1333-
file_ttl: ttl,
1334-
..Default::default()
1335-
},
1333+
cache_config: CacheConfig::new(TimeToLive::Duration(ttl)),
13361334
s3_personality: S3Personality::Standard,
13371335
},
13381336
);
@@ -1382,12 +1380,7 @@ mod tests {
13821380
bucket,
13831381
&prefix,
13841382
SuperblockConfig {
1385-
cache_config: CacheConfig {
1386-
serve_lookup_from_cache: true,
1387-
dir_ttl: ttl,
1388-
file_ttl: ttl,
1389-
..Default::default()
1390-
},
1383+
cache_config: CacheConfig::new(TimeToLive::Duration(ttl)),
13911384
s3_personality: S3Personality::Standard,
13921385
},
13931386
);

‎mountpoint-s3/tests/fs.rs‎

Lines changed: 7 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use fuser::FileType;
55
use mountpoint_s3::fs::error_metadata::MOUNTPOINT_ERROR_LOOKUP_NONEXISTENT;
66
#[cfg(all(feature = "s3_tests", not(feature = "s3express_tests")))]
77
use mountpoint_s3::fs::error_metadata::{ErrorMetadata, MOUNTPOINT_ERROR_CLIENT};
8-
use mountpoint_s3::fs::{CacheConfig, OpenFlags, ToErrno, FUSE_ROOT_INODE};
8+
use mountpoint_s3::fs::{CacheConfig, OpenFlags, TimeToLive, ToErrno, FUSE_ROOT_INODE};
99
use mountpoint_s3::prefix::Prefix;
1010
use mountpoint_s3::s3::S3Personality;
1111
use mountpoint_s3::S3FilesystemConfig;
@@ -192,12 +192,7 @@ async fn test_read_dir_nested(prefix: &str) {
192192
#[tokio::test]
193193
async fn test_lookup_negative_cached() {
194194
let fs_config = S3FilesystemConfig {
195-
cache_config: CacheConfig {
196-
serve_lookup_from_cache: true,
197-
dir_ttl: Duration::from_secs(600),
198-
file_ttl: Duration::from_secs(600),
199-
..Default::default()
200-
},
195+
cache_config: CacheConfig::new(TimeToLive::Duration(Duration::from_secs(600))),
201196
..Default::default()
202197
};
203198
let (client, fs) = make_test_filesystem("test_lookup_negative_cached", &Default::default(), fs_config);
@@ -263,12 +258,7 @@ async fn test_lookup_negative_cached() {
263258
#[tokio::test]
264259
async fn test_lookup_then_open_cached() {
265260
let fs_config = S3FilesystemConfig {
266-
cache_config: CacheConfig {
267-
serve_lookup_from_cache: true,
268-
dir_ttl: Duration::from_secs(600),
269-
file_ttl: Duration::from_secs(600),
270-
..Default::default()
271-
},
261+
cache_config: CacheConfig::new(TimeToLive::Duration(Duration::from_secs(600))),
272262
..Default::default()
273263
};
274264
let (client, fs) = make_test_filesystem("test_lookup_then_open_cached", &Default::default(), fs_config);
@@ -297,10 +287,7 @@ async fn test_lookup_then_open_cached() {
297287
#[tokio::test]
298288
async fn test_lookup_then_open_no_cache() {
299289
let fs_config = S3FilesystemConfig {
300-
cache_config: CacheConfig {
301-
serve_lookup_from_cache: false,
302-
..Default::default()
303-
},
290+
cache_config: CacheConfig::new(TimeToLive::Minimal),
304291
..Default::default()
305292
};
306293
let (client, fs) = make_test_filesystem("test_lookup_then_open_no_cache", &Default::default(), fs_config);
@@ -329,12 +316,7 @@ async fn test_lookup_then_open_no_cache() {
329316
#[tokio::test]
330317
async fn test_readdir_then_open_cached() {
331318
let fs_config = S3FilesystemConfig {
332-
cache_config: CacheConfig {
333-
serve_lookup_from_cache: true,
334-
dir_ttl: Duration::from_secs(600),
335-
file_ttl: Duration::from_secs(600),
336-
..Default::default()
337-
},
319+
cache_config: CacheConfig::new(TimeToLive::Duration(Duration::from_secs(600))),
338320
..Default::default()
339321
};
340322
let (client, fs) = make_test_filesystem("test_readdir_then_open_cached", &Default::default(), fs_config);
@@ -380,12 +362,7 @@ async fn test_readdir_then_open_cached() {
380362
#[tokio::test]
381363
async fn test_unlink_cached() {
382364
let fs_config = S3FilesystemConfig {
383-
cache_config: CacheConfig {
384-
serve_lookup_from_cache: true,
385-
dir_ttl: Duration::from_secs(600),
386-
file_ttl: Duration::from_secs(600),
387-
..Default::default()
388-
},
365+
cache_config: CacheConfig::new(TimeToLive::Duration(Duration::from_secs(600))),
389366
allow_delete: true,
390367
..Default::default()
391368
};
@@ -430,12 +407,7 @@ async fn test_unlink_cached() {
430407
async fn test_mknod_cached() {
431408
const BUCKET_NAME: &str = "test_mknod_cached";
432409
let fs_config = S3FilesystemConfig {
433-
cache_config: CacheConfig {
434-
serve_lookup_from_cache: true,
435-
dir_ttl: Duration::from_secs(600),
436-
file_ttl: Duration::from_secs(600),
437-
..Default::default()
438-
},
410+
cache_config: CacheConfig::new(TimeToLive::Duration(Duration::from_secs(600))),
439411
..Default::default()
440412
};
441413
let (client, fs) = make_test_filesystem(BUCKET_NAME, &Default::default(), fs_config);

‎mountpoint-s3/tests/fuse_tests/lookup_test.rs‎

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ use std::{
55
time::Duration,
66
};
77

8-
use mountpoint_s3::{fs::CacheConfig, S3FilesystemConfig};
8+
use mountpoint_s3::{
9+
fs::{CacheConfig, TimeToLive},
10+
S3FilesystemConfig,
11+
};
912
use test_case::test_case;
1013

1114
use crate::common::fuse::{self, read_dir_to_entry_names, TestSessionConfig, TestSessionCreator};
@@ -106,6 +109,7 @@ fn lookup_previously_shadowed_file_test(creator_fn: impl TestSessionCreator) {
106109
serve_lookup_from_cache: false,
107110
file_ttl: Duration::ZERO,
108111
dir_ttl: Duration::ZERO,
112+
negative_cache_ttl: Duration::ZERO,
109113
..Default::default()
110114
},
111115
..Default::default()
@@ -194,12 +198,7 @@ fn lookup_with_negative_cache(creator_fn: impl TestSessionCreator) {
194198
const FILE_NAME: &str = "hello.txt";
195199
let config = TestSessionConfig {
196200
filesystem_config: S3FilesystemConfig {
197-
cache_config: CacheConfig {
198-
serve_lookup_from_cache: true,
199-
dir_ttl: Duration::from_secs(600),
200-
file_ttl: Duration::from_secs(600),
201-
..Default::default()
202-
},
201+
cache_config: CacheConfig::new(TimeToLive::Duration(Duration::from_secs(600))),
203202
..Default::default()
204203
},
205204
..Default::default()
@@ -232,3 +231,38 @@ fn lookup_with_negative_cache_s3() {
232231
fn lookup_with_negative_cache_mock() {
233232
lookup_with_negative_cache(fuse::mock_session::new);
234233
}
234+
235+
fn lookup_with_negative_cache_ttl(creator_fn: impl TestSessionCreator, ttl: Duration) {
236+
const FILE_NAME: &str = "hello.txt";
237+
let config = TestSessionConfig {
238+
filesystem_config: S3FilesystemConfig {
239+
cache_config: CacheConfig::new(TimeToLive::Duration(Duration::from_secs(600)))
240+
.with_negative_metadata_ttl(TimeToLive::Duration(ttl)),
241+
..Default::default()
242+
},
243+
..Default::default()
244+
};
245+
let test_session = creator_fn("lookup_with_negative_cache_ttl", config);
246+
247+
let file_path = test_session.mount_path().join(FILE_NAME);
248+
metadata(&file_path).expect_err("should fail as no object exists");
249+
250+
test_session.client().put_object(FILE_NAME, b"hello").unwrap();
251+
metadata(&file_path).expect_err("should fail as mountpoint should use negative cache");
252+
253+
std::thread::sleep(ttl);
254+
255+
let m = metadata(&file_path).expect("should succeed as the ttl has expired");
256+
assert!(m.file_type().is_file());
257+
}
258+
259+
#[cfg(feature = "s3_tests")]
260+
#[test]
261+
fn lookup_with_negative_cache_ttl_s3() {
262+
lookup_with_negative_cache_ttl(fuse::s3_session::new, Duration::from_secs(5));
263+
}
264+
265+
#[test]
266+
fn lookup_with_negative_cache_ttl_mock() {
267+
lookup_with_negative_cache_ttl(fuse::mock_session::new, Duration::from_secs(1));
268+
}

‎mountpoint-s3/tests/reftests/harness.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -950,6 +950,7 @@ mod mutations {
950950
serve_lookup_from_cache: false,
951951
dir_ttl: Duration::ZERO,
952952
file_ttl: Duration::ZERO,
953+
negative_cache_ttl: Duration::ZERO,
953954
..Default::default()
954955
},
955956
..Default::default()

0 commit comments

Comments
 (0)