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.

(cherry picked from commit 56089f5)
  • Loading branch information
abhijat authored and vbotbuildovich committed Sep 1, 2023
1 parent 2f0de10 commit a8640f7
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/v/cloud_storage/cache_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <seastar/util/defer.hh>

#include <cloud_storage/cache_service.h>
#include <re2/re2.h>

#include <algorithm>
#include <exception>
Expand All @@ -36,6 +37,11 @@

using namespace std::chrono_literals;

namespace {
// Matches log segments optionally containing a numeric term suffix
const re2::RE2 segment_expr{R"#(.*\.log(\.\d+)?)#"};
} // namespace

namespace cloud_storage {

static constexpr auto access_timer_period = 60s;
Expand Down Expand Up @@ -568,7 +574,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 (RE2::FullMatch(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 a8640f7

Please sign in to comment.