Skip to content

Commit

Permalink
cloud_storage: Extend fast trim cleanup to new segment name
Browse files Browse the repository at this point in the history
When a log segment is removed in fast trim, the associated index and tx
files are also removed. This commit extends this feature to new log
segment name formats, which may contain a term after the name.
  • Loading branch information
abhijat committed Aug 31, 2023
1 parent f5f6d08 commit 20417b1
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/v/cloud_storage/cache_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,8 @@ ss::future<cache::trim_result> cache::trim_fast(
trim_result result;

size_t candidate_i = 0;
const std::regex segment_expr{
R"#(.*\.log(\.\d+)?)#", std::regex_constants::optimize};
while (
candidate_i < candidates.size()
&& (result.deleted_size < size_to_delete || result.deleted_count < objects_to_delete)) {
Expand Down Expand Up @@ -568,7 +570,7 @@ ss::future<cache::trim_result> cache::trim_fast(
std::optional<std::string> tx_file;
std::optional<std::string> index_file;

if (std::string_view(file_stat.path).ends_with(".log")) {
if (std::regex_match(file_stat.path.data(), segment_expr)) {
// If this was a legacy whole-segment item, delete the index
// and tx file along with the segment
tx_file = fmt::format("{}.tx", file_stat.path);
Expand Down
38 changes: 38 additions & 0 deletions src/v/cloud_storage/tests/cache_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -500,3 +500,41 @@ FIXTURE_TEST(test_exhaustive_trim_runs_after_fast_trim, cache_test_fixture) {
return !std::filesystem::exists(path);
}));
}

FIXTURE_TEST(test_log_segment_cleanup, cache_test_fixture) {
std::vector<std::filesystem::path> objects{
CACHE_DIR / "test.log.1",
CACHE_DIR / "test.log.1.index",
CACHE_DIR / "test.log.1.tx",
CACHE_DIR / "test.log",
CACHE_DIR / "test.log.index",
CACHE_DIR / "test.log.tx"};
for (const auto& obj : objects) {
std::ofstream f{obj};
f.flush();
}

// An un-removable file makes sure the fast trim will have to remove the two
// segment files.
{
std::ofstream f{CACHE_DIR / "accesstime"};
f.flush();
}

BOOST_REQUIRE(
std::all_of(objects.cbegin(), objects.cend(), [](const auto& path) {
return std::filesystem::exists(path);
}));

clean_up_at_start().get();

// With this limit all of the index+tx files should not have to be removed,
// but fast trim will remove all of these files after removing the two
// segments.
trim_cache(std::nullopt, 4);

BOOST_REQUIRE(
std::all_of(objects.cbegin(), objects.cend(), [](const auto& path) {
return !std::filesystem::exists(path);
}));
}

0 comments on commit 20417b1

Please sign in to comment.