diff --git a/sstables/storage.cc b/sstables/storage.cc index ee95236f56..4f7f49c4bf 100644 --- a/sstables/storage.cc +++ b/sstables/storage.cc @@ -543,11 +543,16 @@ future<> filesystem_storage::wipe(const sstable& sst, sync_dir sync) noexcept { // during SSTable writing and removed before sealing. If the write // failed before sealing, the file may still be on disk and must be // cleaned up explicitly. + // The component is only defined for the `ms` sstable format; for + // older formats it is absent from the component map and looking up + // its filename would throw std::out_of_range. // Use file_exists() to avoid a C++ exception on the common path // where the file was already removed before sealing. - auto temp_hashes = filename(sst, dir_name.native(), sst._generation, component_type::TemporaryHashes); - if (co_await file_exists(temp_hashes)) { - co_await sst.sstable_write_io_check(remove_file, std::move(temp_hashes)); + if (sstable_version_constants::get_component_map(sst.get_version()).contains(component_type::TemporaryHashes)) { + auto temp_hashes = filename(sst, dir_name.native(), sst._generation, component_type::TemporaryHashes); + if (co_await file_exists(temp_hashes)) { + co_await sst.sstable_write_io_check(remove_file, std::move(temp_hashes)); + } } if (sync) { co_await sst.sstable_write_io_check(sync_directory, dir_name.native()); diff --git a/test/boost/sstable_directory_test.cc b/test/boost/sstable_directory_test.cc index b8e85c1d9c..98faeb68b1 100644 --- a/test/boost/sstable_directory_test.cc +++ b/test/boost/sstable_directory_test.cc @@ -246,6 +246,33 @@ SEASTAR_TEST_CASE(sstable_directory_test_table_extra_temporary_toc) { }); } +// Reproducer for SCYLLADB-1697 +SEASTAR_TEST_CASE(sstable_directory_test_unlink_sstable_leaves_no_orphans) { + return sstables::test_env::do_with_async([] (test_env& env) { + for (const auto version : {sstable_version_types::me, sstable_version_types::ms}) { + testlog.info("Testing sstable version: {}", version); + auto sst = make_sstable_for_this_shard([&env, version] { + return env.make_sstable(test_table_schema(), version); + }); + + // Sanity: the TOC was written, otherwise the assertion below would be vacuous. + BOOST_REQUIRE(file_exists(test(sst).filename(sstables::component_type::TOC).native()).get()); + + sst->unlink().get(); + + std::vector remaining; + lister::scan_dir(env.tempdir().path(), lister::dir_entry_types::of(), + [&remaining] (fs::path, directory_entry de) { + remaining.push_back(de.name); + return make_ready_future<>(); + }).get(); + + BOOST_REQUIRE_MESSAGE(remaining.empty(), + fmt::format("Expected empty sstable dir after unlink for version {}, found: {}", version, remaining)); + } + }); +} + // Test the absence of TOC. Behavior is controllable by a flag SEASTAR_TEST_CASE(sstable_directory_test_table_missing_toc) { return sstables::test_env::do_with_async([] (test_env& env) {