Skip to content

Commit 90bb0b4

Browse files
Fix default data cache limit validation during eviction (#1779)
Fixes `is_limit_exceeded()` function used by the data cache when in default mode (which should keep 5% of space in the cache file system). Before the fix, it was incorrectly considering space reserved by the file system as available space leading to 100% space usage. Now, reserved space is considered and the 5% available space is respected. A regression test was also added. ### Does this change impact existing behavior? No ### Does this change need a changelog entry? Does it require a version change? Yes --- 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: Renan Magagnin <renanmag@amazon.co.uk>
1 parent 27ca3a0 commit 90bb0b4

6 files changed

Lines changed: 253 additions & 28 deletions

File tree

mountpoint-s3-fs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
## Unreleased (v0.9.1)
22

33
* Upgrade cargo dependencies.
4+
* Fix incorrect validation of default data cache limit which would cause Mountpoint to preserve less than 5% of available space ([#1779](https://github.com/awslabs/mountpoint-s3/pull/1779))
45

56
## v0.9.0 (January 22, 2026)
67

mountpoint-s3-fs/src/data_cache.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ use thiserror::Error;
1515

1616
pub use crate::checksums::ChecksummedBytes;
1717
pub use crate::data_cache::cache_directory::ManagedCacheDir;
18-
pub use crate::data_cache::disk_data_cache::{CacheLimit, DiskDataCache, DiskDataCacheConfig};
18+
pub use crate::data_cache::disk_data_cache::{
19+
CacheLimit, DEFAULT_CACHE_MIN_AVAILABLE_RATIO, DiskDataCache, DiskDataCacheConfig,
20+
};
1921
pub use crate::data_cache::express_data_cache::{ExpressDataCache, ExpressDataCacheConfig, build_prefix, get_s3_key};
2022
pub use crate::data_cache::in_memory_data_cache::InMemoryDataCache;
2123
pub use crate::data_cache::multilevel_cache::MultilevelDataCache;

mountpoint-s3-fs/src/data_cache/disk_data_cache.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,15 @@ pub enum CacheLimit {
6262
AvailableSpace { min_ratio: f64 },
6363
}
6464

65+
/// Default minimum ratio of available space to preserve when using AvailableSpace cache limit.
66+
/// This preserves 5% of the filesystem's total space as available space.
67+
pub const DEFAULT_CACHE_MIN_AVAILABLE_RATIO: f64 = 0.05;
68+
6569
impl Default for CacheLimit {
6670
fn default() -> Self {
67-
CacheLimit::AvailableSpace { min_ratio: 0.05 } // Preserve 5% available space
71+
CacheLimit::AvailableSpace {
72+
min_ratio: DEFAULT_CACHE_MIN_AVAILABLE_RATIO,
73+
}
6874
}
6975
}
7076

@@ -407,7 +413,7 @@ impl DiskDataCache {
407413
return false;
408414
}
409415
};
410-
(stats.blocks_free() as f64) < min_ratio * (stats.blocks() as f64)
416+
(stats.blocks_available() as f64) < min_ratio * (stats.blocks() as f64)
411417
}
412418
}
413419
}

mountpoint-s3-fs/tests/fuse_tests/cache_test.rs

Lines changed: 240 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,46 @@
11
use crate::common::cache::CacheTestWrapper;
2-
use crate::common::fuse::create_fuse_session;
3-
use crate::common::fuse::s3_session::create_crt_client;
4-
use crate::common::s3::{get_test_prefix, get_test_s3_path};
5-
6-
use mountpoint_s3_client::S3CrtClient;
7-
use mountpoint_s3_fs::Runtime;
8-
use mountpoint_s3_fs::data_cache::{DataCache, DiskDataCache, DiskDataCacheConfig};
9-
use mountpoint_s3_fs::fuse::session::FuseSession;
10-
use mountpoint_s3_fs::memory::PagedPool;
11-
use mountpoint_s3_fs::object::ObjectId;
12-
use mountpoint_s3_fs::prefetch::Prefetcher;
13-
use mountpoint_s3_fs::s3::S3Path;
14-
2+
use crate::common::fuse::{self, TestSessionConfig};
3+
use mountpoint_s3_fs::data_cache::{CacheLimit, DEFAULT_CACHE_MIN_AVAILABLE_RATIO, DiskDataCache, DiskDataCacheConfig};
154
use rand::rngs::SmallRng;
16-
use rand::{Rng, RngExt, SeedableRng};
17-
use std::fs;
18-
use std::time::Duration;
19-
use tempfile::TempDir;
20-
use test_case::test_case;
5+
use rand::{Rng, SeedableRng};
6+
use std::fs::{self, File};
7+
use std::io::Read;
8+
use std::path::{Path, PathBuf};
9+
use std::process::Command;
10+
use tracing::{debug, info, warn};
11+
12+
#[cfg(feature = "s3_tests")]
13+
use {
14+
crate::common::fuse::create_fuse_session,
15+
crate::common::fuse::s3_session::create_crt_client,
16+
crate::common::s3::{get_test_prefix, get_test_s3_path},
17+
mountpoint_s3_client::S3CrtClient,
18+
mountpoint_s3_fs::Runtime,
19+
mountpoint_s3_fs::data_cache::DataCache,
20+
mountpoint_s3_fs::fuse::session::FuseSession,
21+
mountpoint_s3_fs::memory::PagedPool,
22+
mountpoint_s3_fs::object::ObjectId,
23+
mountpoint_s3_fs::prefetch::Prefetcher,
24+
mountpoint_s3_fs::s3::S3Path,
25+
rand::RngExt,
26+
std::time::Duration,
27+
tempfile::TempDir,
28+
test_case::test_case,
29+
};
30+
31+
#[cfg(feature = "s3express_tests")]
32+
use {
33+
crate::common::s3::{get_express_bucket, get_express_sse_kms_bucket, get_standard_bucket, get_test_kms_key_id},
34+
mountpoint_s3_client::ObjectClient,
35+
mountpoint_s3_fs::data_cache::{BlockIndex, ExpressDataCache, ExpressDataCacheConfig, build_prefix, get_s3_key},
36+
};
2137

2238
#[cfg(all(feature = "s3express_tests", feature = "second_account_tests"))]
2339
use crate::common::s3::{get_bucket_owner, get_external_express_bucket, get_test_endpoint_config};
24-
#[cfg(feature = "s3express_tests")]
25-
use crate::common::s3::{get_express_bucket, get_express_sse_kms_bucket, get_standard_bucket, get_test_kms_key_id};
26-
#[cfg(feature = "s3express_tests")]
27-
use mountpoint_s3_client::ObjectClient;
28-
#[cfg(feature = "s3express_tests")]
29-
use mountpoint_s3_fs::data_cache::{BlockIndex, ExpressDataCache, ExpressDataCacheConfig, build_prefix, get_s3_key};
3040

41+
#[cfg(feature = "s3_tests")]
3142
const CACHE_BLOCK_SIZE: u64 = 1024 * 1024;
43+
#[cfg(feature = "s3_tests")]
3244
const CLIENT_PART_SIZE: usize = 8 * 1024 * 1024;
3345

3446
/// A test that checks that an invalid block may not be served from the cache
@@ -124,6 +136,7 @@ fn express_cache_write_read(key_suffix: &str, key_size: usize, object_size: usiz
124136
#[test_case("£", 100, 1024; "non-ascii key")]
125137
#[test_case("key", 1024, 1024; "long key")]
126138
#[test_case("key", 100, 1024 * 1024; "big file")]
139+
#[cfg(feature = "s3_tests")]
127140
fn disk_cache_write_read(key_suffix: &str, key_size: usize, object_size: usize) {
128141
let cache_dir = tempfile::tempdir().unwrap();
129142
let cache_config = DiskDataCacheConfig {
@@ -177,6 +190,7 @@ async fn express_cache_read_empty() {
177190
}
178191

179192
#[tokio::test]
193+
#[cfg(feature = "s3_tests")]
180194
async fn disk_cache_read_empty() {
181195
let cache_dir = tempfile::tempdir().unwrap();
182196
let cache_config = DiskDataCacheConfig {
@@ -267,6 +281,7 @@ async fn express_cache_verify_fail_forbidden() {
267281
}
268282

269283
#[allow(clippy::too_many_arguments)]
284+
#[cfg(feature = "s3_tests")]
270285
fn cache_write_read_base<Cache>(
271286
client: S3CrtClient,
272287
s3_path: S3Path,
@@ -307,6 +322,7 @@ fn cache_write_read_base<Cache>(
307322
);
308323
}
309324

325+
#[cfg(feature = "s3_tests")]
310326
async fn cache_read_empty<Cache>(cache: Cache, test_name: &str)
311327
where
312328
Cache: DataCache + Send + Sync + 'static,
@@ -389,6 +405,7 @@ fn express_cache_expected_bucket_owner(cache_bucket: String, owner_checked: bool
389405
}
390406

391407
/// Generates random data of the specified size
408+
#[cfg(feature = "s3_tests")]
392409
fn random_binary_data(size_in_bytes: usize) -> Vec<u8> {
393410
let seed = rand::rng().random();
394411
let mut rng = SmallRng::seed_from_u64(seed);
@@ -399,6 +416,7 @@ fn random_binary_data(size_in_bytes: usize) -> Vec<u8> {
399416

400417
/// Creates a random key which has a size of at least `min_size_in_bytes`
401418
/// The `key_prefix` is not included in the return value.
419+
#[cfg(feature = "s3_tests")]
402420
fn get_random_key(key_prefix: &str, key_suffix: &str, min_size_in_bytes: usize) -> String {
403421
let random_suffix: u64 = rand::rng().random();
404422
let last_key_part = format!("{key_suffix}{random_suffix}"); // part of the key after all the "/"
@@ -409,6 +427,7 @@ fn get_random_key(key_prefix: &str, key_suffix: &str, min_size_in_bytes: usize)
409427
format!("{last_key_part}{padding}")
410428
}
411429

430+
#[cfg(feature = "s3_tests")]
412431
fn mount_bucket<Cache>(client: S3CrtClient, cache: Cache, pool: PagedPool, s3_path: S3Path) -> (TempDir, FuseSession)
413432
where
414433
Cache: DataCache + Send + Sync + 'static,
@@ -428,6 +447,7 @@ where
428447
(mount_point, session)
429448
}
430449

450+
#[cfg(feature = "s3_tests")]
431451
fn get_object_id(prefix: &str, key: &str, etag: &str) -> ObjectId {
432452
ObjectId::new(format!("{prefix}{key}"), etag.into())
433453
}
@@ -437,3 +457,199 @@ fn get_express_cache_block_key(bucket: &str, cache_key: &ObjectId, block_idx: Bl
437457
let block_key_prefix = build_prefix(bucket, CACHE_BLOCK_SIZE);
438458
get_s3_key(&block_key_prefix, cache_key, block_idx)
439459
}
460+
461+
/// Get filesystem statistics for a given path
462+
fn get_filesystem_stats(path: &Path) -> (u64, u64) {
463+
let stat = nix::sys::statvfs::statvfs(path).expect("Failed to get filesystem stats");
464+
let block_size = stat.block_size();
465+
(stat.blocks() * block_size, stat.blocks_available() * block_size)
466+
}
467+
468+
/// Test that the cache respects the available space limit (default 5% free) during sequential reads.
469+
///
470+
/// An isolated loop device filesystem is used for the cache, ensuring Mountpoint calculates the 95% limit based on the isolated filesystem, not the entire host.
471+
///
472+
/// Note: requires `sudo` for loop device mount/umount operations.
473+
#[test]
474+
fn available_space_cache_limit_test_mock() {
475+
const FILE_SIZE: usize = 4 * 1024 * 1024; // 4 MiB per file
476+
const NUM_FILES: usize = 50; // 50 files = 200 MiB total data
477+
const TOLERANCE_RATIO: f64 = 0.02; // 2% tolerance for filesystem metadata overhead
478+
const LOOP_DEVICE_SIZE_MIB: u64 = 128; // 128 MiB loop device - total data (200 MiB) intentionally exceeds this
479+
480+
let loop_fs = LoopDeviceFilesystem::new(LOOP_DEVICE_SIZE_MIB).expect("Failed to create loop device filesystem");
481+
let cache_path = loop_fs.mount_path().to_path_buf();
482+
483+
let test_session = fuse::mock_session::new_with_cache(|block_size, pool| {
484+
let cache_config = DiskDataCacheConfig {
485+
cache_directory: cache_path.clone(),
486+
block_size,
487+
limit: CacheLimit::AvailableSpace {
488+
min_ratio: DEFAULT_CACHE_MIN_AVAILABLE_RATIO,
489+
},
490+
};
491+
CacheTestWrapper::new(DiskDataCache::new(cache_config, pool))
492+
})("available_space_cache_limit_test", TestSessionConfig::default());
493+
494+
// Create test files
495+
let mut rng = SmallRng::seed_from_u64(0x12345678);
496+
let mut file_data = vec![0u8; FILE_SIZE];
497+
498+
for i in 0..NUM_FILES {
499+
rng.fill_bytes(&mut file_data);
500+
let key = format!("file-{}.bin", i + 1);
501+
test_session.client().put_object(&key, &file_data).unwrap();
502+
}
503+
504+
let (total_space, initial_available) = get_filesystem_stats(&cache_path);
505+
let mut min_available_space = initial_available;
506+
let mut has_violation = false;
507+
508+
// Sequential read pattern - read each file once
509+
for i in 0..NUM_FILES {
510+
let key = format!("file-{}.bin", i + 1);
511+
let path = test_session.mount_path().join(&key);
512+
513+
let mut file = File::open(&path).unwrap();
514+
let mut buffer = Vec::new();
515+
file.read_to_end(&mut buffer).unwrap();
516+
517+
assert_eq!(buffer.len(), FILE_SIZE, "File {} has incorrect size", key);
518+
519+
// Check filesystem available space
520+
let (_, current_available) = get_filesystem_stats(&cache_path);
521+
min_available_space = min_available_space.min(current_available);
522+
523+
let available_ratio = current_available as f64 / total_space as f64;
524+
let used_percent = ((total_space - current_available) as f64 / total_space as f64) * 100.0;
525+
526+
// Check if we're preserving at least 5% available space of total filesystem with 2% tolerance.
527+
// This margin is necessary because Mountpoint currently slightly exceeds the limit occasionally.
528+
let tolerance = (total_space as f64 * TOLERANCE_RATIO) as u64;
529+
let min_required_available = (total_space as f64 * DEFAULT_CACHE_MIN_AVAILABLE_RATIO) as u64;
530+
531+
if current_available < min_required_available.saturating_sub(tolerance) {
532+
has_violation = true;
533+
let shortage = min_required_available - current_available;
534+
let shortage_pct = (shortage as f64 / total_space as f64) * 100.0;
535+
warn!(
536+
"File {}: Available space {} bytes ({:.1}% used) - BELOW minimum {} bytes (shortage: {} bytes, {:.2}%)",
537+
i + 1,
538+
current_available,
539+
used_percent,
540+
min_required_available,
541+
shortage,
542+
shortage_pct
543+
);
544+
} else if (i + 1) % 10 == 0 {
545+
debug!(
546+
"File {}: Used space {} bytes, Available {} bytes ({:.1}% used, {:.1}% free)",
547+
i + 1,
548+
total_space - current_available,
549+
current_available,
550+
used_percent,
551+
available_ratio * 100.0
552+
);
553+
}
554+
}
555+
556+
let min_available_space_percent = (min_available_space as f64 / total_space as f64) * 100.0;
557+
let (_, final_available) = get_filesystem_stats(&cache_path);
558+
let final_used_percent = ((total_space - final_available) as f64 / total_space as f64) * 100.0;
559+
info!(
560+
"Filesystem Total: {} MiB, Initial available: {} MiB, Min available: {} MiB ({:.1}%), Final usage: {:.1}%",
561+
total_space / (1024 * 1024),
562+
initial_available / (1024 * 1024),
563+
min_available_space / (1024 * 1024),
564+
min_available_space_percent,
565+
final_used_percent,
566+
);
567+
568+
// Assert that eviction actually triggered - the cache should have gotten close to the limit.
569+
let max_expected_available = (total_space as f64 * (DEFAULT_CACHE_MIN_AVAILABLE_RATIO + TOLERANCE_RATIO)) as u64;
570+
assert!(
571+
min_available_space <= max_expected_available,
572+
"Cache eviction may not have triggered - available space never got close to the limit. \
573+
Min available: {} bytes ({:.1}%), expected to reach within {:.1}% above the {:.1}% minimum",
574+
min_available_space,
575+
min_available_space_percent,
576+
TOLERANCE_RATIO * 100.0,
577+
DEFAULT_CACHE_MIN_AVAILABLE_RATIO * 100.0,
578+
);
579+
580+
// Assert no violations (with the tolerance)
581+
assert!(
582+
!has_violation,
583+
"Cache violated available space limit. Min available: {} bytes ({:.1}%), Required: {:.1}% (tolerance: {:.1}%)",
584+
min_available_space,
585+
min_available_space_percent,
586+
DEFAULT_CACHE_MIN_AVAILABLE_RATIO * 100.0,
587+
TOLERANCE_RATIO * 100.0
588+
);
589+
}
590+
591+
/// Represents an isolated loop device filesystem for cache testing
592+
struct LoopDeviceFilesystem {
593+
mount_path: PathBuf,
594+
/// Kept alive to ensure the temp directory is not deleted before `Drop` unmounts the loop device.
595+
_temp_dir: tempfile::TempDir,
596+
}
597+
598+
impl LoopDeviceFilesystem {
599+
/// Create a new loop device filesystem with the specified size in MiB
600+
fn new(size_mib: u64) -> std::io::Result<Self> {
601+
let temp_dir = tempfile::tempdir()?;
602+
let image_path = temp_dir.path().join("cache-device.img");
603+
let mount_path = temp_dir.path().join("cache-mount");
604+
605+
fs::create_dir_all(&mount_path)?;
606+
607+
// Create a sparse file for the loop device
608+
let file = File::create(&image_path)?;
609+
file.set_len(size_mib * 1024 * 1024)?;
610+
drop(file);
611+
612+
// Create ext4 filesystem on the image
613+
let output = Command::new("mkfs.ext4").arg("-F").arg(&image_path).output()?;
614+
if !output.status.success() {
615+
return Err(std::io::Error::other(format!(
616+
"mkfs.ext4 failed: {}",
617+
String::from_utf8_lossy(&output.stderr)
618+
)));
619+
}
620+
621+
let output = Command::new("sudo")
622+
.args(["mount", "-o", "loop"])
623+
.arg(&image_path)
624+
.arg(&mount_path)
625+
.output()?;
626+
if !output.status.success() {
627+
return Err(std::io::Error::other(format!(
628+
"mount failed: {}",
629+
String::from_utf8_lossy(&output.stderr)
630+
)));
631+
}
632+
633+
// Make the mount point writable
634+
let output = Command::new("sudo").args(["chmod", "777"]).arg(&mount_path).output()?;
635+
if !output.status.success() {
636+
warn!("chmod failed: {}", String::from_utf8_lossy(&output.stderr));
637+
}
638+
639+
Ok(Self {
640+
mount_path,
641+
_temp_dir: temp_dir,
642+
})
643+
}
644+
645+
/// Get the mount path for the loop device
646+
fn mount_path(&self) -> &Path {
647+
&self.mount_path
648+
}
649+
}
650+
651+
impl Drop for LoopDeviceFilesystem {
652+
fn drop(&mut self) {
653+
let _ = Command::new("sudo").arg("umount").arg(&self.mount_path).output();
654+
}
655+
}

mountpoint-s3-fs/tests/fuse_tests/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
#[cfg(feature = "s3_tests")]
21
mod cache_test;
32
mod consistency_test;
43
#[cfg(all(feature = "manifest", feature = "event_log"))]

mountpoint-s3/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
## Unreleased (v1.22.1)
22

33
* Upgrade cargo dependencies.
4+
* Fix incorrect validation of default data cache limit which would cause Mountpoint to preserve less than 5% of available space ([#1779](https://github.com/awslabs/mountpoint-s3/pull/1779))
45

56
## v1.22.0 (January 22, 2026)
67

0 commit comments

Comments
 (0)