From d7f0b6a23d6e9d6bd5c760bbf94b509d50e80f7e Mon Sep 17 00:00:00 2001 From: Adam Kupczyk Date: Mon, 31 Mar 2025 20:49:26 +0000 Subject: [PATCH] test/store_test: Expose race in BlueFS truncate / remove Created test that exposes race between BlueFS::truncate and BlueFS::unlink. Test requires injection of 1ms sleep to BlueFS::truncate. Therefore, in this form, it is unsuitable for merge. Signed-off-by: Adam Kupczyk --- src/os/bluestore/BlueFS.cc | 2 +- src/os/bluestore/BlueFS.h | 2 ++ src/test/objectstore/store_test.cc | 47 ++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc index da6dea89315..51f609b0d6c 100644 --- a/src/os/bluestore/BlueFS.cc +++ b/src/os/bluestore/BlueFS.cc @@ -3861,7 +3861,7 @@ int BlueFS::truncate(FileWriter *h, uint64_t offset)/*_WF_L*/ if (offset > fnode.size) { ceph_abort_msg("truncate up not supported"); } - + unittest_inject_delay(); _flush_bdev(h); { std::lock_guard ll(log.lock); diff --git a/src/os/bluestore/BlueFS.h b/src/os/bluestore/BlueFS.h index f21c20869fb..592424be650 100644 --- a/src/os/bluestore/BlueFS.h +++ b/src/os/bluestore/BlueFS.h @@ -237,6 +237,7 @@ public: : m_func(func) {} template void operator()(Arg... arg) { if (m_func) m_func(std::forward(arg...)); } + void operator()() { if (m_func) m_func(); } void operator=(T&& func) { m_func = std::move(func);} void operator=(T& func) { m_func = func;} private: @@ -813,6 +814,7 @@ public: bool debug_get_is_dev_dirty(FileWriter *h, uint8_t dev); debug_point_t> tracepoint_async_compact; void trim_free_space(const std::string& type, std::ostream& outss); + debug_point_t> unittest_inject_delay; private: // Wrappers for BlockDevice::read(...) and BlockDevice::read_random(...) diff --git a/src/test/objectstore/store_test.cc b/src/test/objectstore/store_test.cc index 07c824ffbf4..52ef9e05c0c 100644 --- a/src/test/objectstore/store_test.cc +++ b/src/test/objectstore/store_test.cc @@ -11996,6 +11996,53 @@ TEST_P(StoreTestSpecificAUSize, BlueFSReservedTest) { g_conf()->bluefs_alloc_size + wal_extra); } + +TEST_P(StoreTest, BlueFS_truncate_remove_race) { + if (string(GetParam()) != "bluestore") + GTEST_SKIP(); + + BlueStore* bstore = dynamic_cast (store.get()); + ceph_assert(bstore); + BlueFS& fs = *bstore->get_bluefs(); + fs.unittest_inject_delay = []() { usleep(1000); }; + static constexpr uint32_t batch_size = 20; + std::binary_semaphore go_remove(0); + std::binary_semaphore done_remove(0); + + auto files_remover = [&]() { + for (uint32_t cnt = 0; cnt < batch_size; cnt++) { + go_remove.acquire(); + std::string name = "test-file-" + std::to_string(cnt); + fs.unlink("dir", name); + fs.sync_metadata(false); + done_remove.release(); + } + }; + + ASSERT_EQ(0, fs.mkdir("dir")); + std::thread remover_thread(files_remover); + + for (uint32_t cnt = 0; cnt < batch_size; cnt++) { + std::string name = "test-file-" + std::to_string(cnt); + BlueFS::FileWriter *f = nullptr; + ASSERT_EQ(0, fs.open_for_write("dir", name, &f, false)); + fs.preallocate(f->file, 0, 100000); + for (uint32_t i = 0; i < 10; i++) { + fs.append_try_flush(f, "x", 1); + fs.fsync(f); + } + go_remove.release(); + fs.truncate(f, 10); + fs.close_writer(f); + done_remove.acquire(); + } + + remover_thread.join(); + fs.unittest_inject_delay = nullptr; + EXPECT_EQ(store->umount(), 0); + EXPECT_EQ(store->mount(), 0); +} + #endif // WITH_BLUESTORE int main(int argc, char **argv) { -- 2.39.5