Skip to content

Commit

Permalink
cst/cache: skip trim if free disk info is not available
Browse files Browse the repository at this point in the history
Otherwise it ends up removing everything from the cache when disk info is not yet available.
  • Loading branch information
nvartolomei committed Dec 23, 2024
1 parent 30a15ed commit cecaf85
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 3 deletions.
13 changes: 11 additions & 2 deletions src/v/cloud_storage/cache_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,10 @@ ss::future<> cache::trim(
target_size *= _cache_size_low_watermark;
}

if (!_free_space.has_value()) {
throw std::runtime_error("Free space information is not available.");
}

// In the extreme case where even trimming to the low watermark wouldn't
// free enough space to enable writing to the cache, go even further.
if (_free_space < config::shard_local_cfg().storage_min_free_bytes()) {
Expand Down Expand Up @@ -1533,7 +1537,7 @@ bool cache::may_exceed_limits(uint64_t bytes, size_t objects) {

auto would_fit_in_cache = _current_cache_size + bytes <= _max_bytes;

return !_block_puts && _free_space > bytes * 10
return !_block_puts && _free_space.value_or(0) > bytes * 10
&& _current_cache_objects + _reserved_cache_objects + objects
< _max_objects()
&& !would_fit_in_cache;
Expand Down Expand Up @@ -1871,7 +1875,12 @@ ss::future<> cache::do_reserve_space(uint64_t bytes, size_t objects) {
// all skipping the cache limit based on the same apparent
// free bytes. This counter will get reset to ground
// truth the next time we get a disk status notification.
_free_space -= bytes;
if (unlikely(!_free_space.has_value())) {
throw std::runtime_error(
"Free space information must be available by the "
"time we execute this code path");
}
*_free_space -= bytes;
break;
} else {
// No allowance, and the disk does not have a lot of
Expand Down
2 changes: 1 addition & 1 deletion src/v/cloud_storage/cache_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ class cache
/// and have to decide whether to block writes, or exceed our configured
/// limit.
/// (shard 0 only)
uint64_t _free_space{0};
std::optional<uint64_t> _free_space;

ssx::semaphore _cleanup_sm{1, "cloud/cache"};
std::set<std::filesystem::path> _files_in_progress;
Expand Down

0 comments on commit cecaf85

Please sign in to comment.