Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cloud_storage: Extend fast trim cleanup to new segment name #13158

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
}));
}
Loading