From c1bf75bd27ff271038ebcdae7b4e6ab96cc60bb3 Mon Sep 17 00:00:00 2001
From: Abhijat Malviya <abhijat@redpanda.com>
Date: Fri, 25 Aug 2023 19:09:10 +0530
Subject: [PATCH] cloud_storage: Force exhaustive trim when fast trim fails

In the case where the cache dir consists solely of index+tx files, the
current code path in fast trim does not remove anything. This should be
followed up by an exhaustive trim to free up slots in cache, but due to
an adjustment of the objects to delete counter, this ends up not
happening.

The change here makes sure that if we have a certain count of objects to
delete, and we were not able to delete the count, and there is a set of
filtered out files, we proceed to do an exhaustive trim.
---
 src/v/cloud_storage/cache_service.cc | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/v/cloud_storage/cache_service.cc b/src/v/cloud_storage/cache_service.cc
index f8e5464d09020..f3f477532ebd9 100644
--- a/src/v/cloud_storage/cache_service.cc
+++ b/src/v/cloud_storage/cache_service.cc
@@ -351,7 +351,8 @@ ss::future<> cache::trim(
     vlog(
       cst_log.debug,
       "trim: set target_size {}/{}, size {}/{}, walked size {} (max {}/{}), "
-      " reserved {}/{}, pending {}/{})",
+      " reserved {}/{}, pending {}/{}), candidates for deletion: {}, filtered "
+      "out: {}",
       target_size,
       target_objects,
       _current_cache_size,
@@ -362,7 +363,9 @@ ss::future<> cache::trim(
       _reserved_cache_size,
       _reserved_cache_objects,
       _reservations_pending,
-      _reservations_pending_objects);
+      _reservations_pending_objects,
+      candidates_for_deletion.size(),
+      filtered_out_files);
 
     if (
       _current_cache_size + _reserved_cache_size < target_size
@@ -453,9 +456,23 @@ ss::future<> cache::trim(
     // cache.
     size_to_delete = std::min(
       walked_cache_size - fast_result.deleted_size, size_to_delete);
-    objects_to_delete = std::min(
-      candidates_for_deletion.size() - fast_result.deleted_count,
-      objects_to_delete);
+
+    // If we were not able to delete enough files and there are some filtered
+    // out files, force an exhaustive trim. This ensures that if the cache is
+    // dominated by filtered out files, we do not skip trimming them by reducing
+    // the objects_to_delete counter next.
+    bool force_exhaustive_trim = fast_result.deleted_count < objects_to_delete
+                                 && filtered_out_files > 0;
+
+    // In the situation where all files in cache are filtered out,
+    // candidates_for_deletion is one (accesstime tracker) and the following
+    // adjustment ends up setting objects_to_delete to 1, skipping the
+    // exhaustive trim. The check force_exhaustive_trim prevents this.
+    if (!force_exhaustive_trim) {
+        objects_to_delete = std::min(
+          candidates_for_deletion.size() - fast_result.deleted_count,
+          objects_to_delete);
+    }
 
     if (
       size_to_delete > undeletable_bytes