From e90eafbbfb348f140e10cb0e183edbf6bb270f7d Mon Sep 17 00:00:00 2001
From: Nicolae Vartolomei <nv@redpanda.com>
Date: Mon, 4 Nov 2024 16:52:06 +0000
Subject: [PATCH] cst/cache: Remove tmp file on put failure

---
 src/v/cloud_storage/cache_service.cc    | 17 +++++++++++++++++
 src/v/cloud_storage/tests/cache_test.cc |  4 +---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/v/cloud_storage/cache_service.cc b/src/v/cloud_storage/cache_service.cc
index c87e6229a04c9..dc6b437a45780 100644
--- a/src/v/cloud_storage/cache_service.cc
+++ b/src/v/cloud_storage/cache_service.cc
@@ -27,6 +27,7 @@
 #include <seastar/core/shard_id.hh>
 #include <seastar/core/smp.hh>
 #include <seastar/core/sstring.hh>
+#include <seastar/coroutine/as_future.hh>
 #include <seastar/util/defer.hh>
 
 #include <cloud_storage/cache_service.h>
@@ -1304,7 +1305,23 @@ ss::future<> cache::put(
         eptr = std::current_exception();
     }
 
+    // If we failed to write to the tmp file, we should delete it, maybe do an
+    // eager trim, and rethrow the exception.
     if (eptr) {
+        if (!_gate.is_closed()) {
+            auto delete_tmp_fut = co_await ss::coroutine::as_future(
+              delete_file_and_empty_parents(tmp_filepath.native()));
+            if (
+              delete_tmp_fut.failed()
+              && !ssx::is_shutdown_exception(delete_tmp_fut.get_exception())) {
+                vlog(
+                  cst_log.error,
+                  "Failed to delete tmp file {}: {}",
+                  tmp_filepath.native(),
+                  delete_tmp_fut.get_exception());
+            }
+        }
+
         if (no_space_on_device) {
             vlog(cst_log.error, "Out of space while writing to cache");
 
diff --git a/src/v/cloud_storage/tests/cache_test.cc b/src/v/cloud_storage/tests/cache_test.cc
index 5794703ec4cc6..2b4f13cce0343 100644
--- a/src/v/cloud_storage/tests/cache_test.cc
+++ b/src/v/cloud_storage/tests/cache_test.cc
@@ -496,10 +496,8 @@ FIXTURE_TEST(test_clean_up_on_stream_exception, cache_test_fixture) {
     BOOST_CHECK_EQUAL(sharded_cache.local().get_usage_bytes(), 0);
     BOOST_CHECK_EQUAL(sharded_cache.local().get_usage_objects(), 0);
 
-    // TODO: This is not expected behavior. The temporary file should be cleaned
-    // up on exception.
     vlog(test_log.info, "Counting files in cache directory");
-    BOOST_CHECK_EQUAL(count_files(CACHE_DIR.native()).get(), 1);
+    BOOST_CHECK_EQUAL(count_files(CACHE_DIR.native()).get(), 0);
 
     vlog(test_log.info, "Test passed");
 }