From 9a97e1b454e6a826765105a43f6117718f81268d Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 13 Jul 2016 14:49:06 -0400 Subject: [PATCH] rbd-mirror: use pool id + global image id as deletion primary key Fixes: http://tracker.ceph.com/issues/16538 Signed-off-by: Jason Dillaman (cherry picked from commit 25203a8a9d59ff025d223ec1afaeb14946d54993) --- src/test/rbd_mirror/test_ImageDeleter.cc | 51 +++++++++++++++-------- src/test/rbd_mirror/test_ImageReplayer.cc | 6 +-- src/tools/rbd_mirror/ImageDeleter.cc | 31 +++++++------- src/tools/rbd_mirror/ImageDeleter.h | 21 ++++++---- src/tools/rbd_mirror/Replayer.cc | 8 +++- 5 files changed, 73 insertions(+), 44 deletions(-) diff --git a/src/test/rbd_mirror/test_ImageDeleter.cc b/src/test/rbd_mirror/test_ImageDeleter.cc index d96b6249078cb..ef2200e4c5f34 100644 --- a/src/test/rbd_mirror/test_ImageDeleter.cc +++ b/src/test/rbd_mirror/test_ImageDeleter.cc @@ -216,7 +216,8 @@ TEST_F(TestImageDeleter, Delete_NonPrimary_Image) { m_image_name, GLOBAL_IMAGE_ID); C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx); + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, + &ctx); EXPECT_EQ(0, ctx.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); @@ -232,7 +233,8 @@ TEST_F(TestImageDeleter, Fail_Delete_Primary_Image) { m_image_name, GLOBAL_IMAGE_ID); C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx); + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, + &ctx); EXPECT_EQ(-rbd::mirror::ImageDeleter::EISPRM, ctx.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); @@ -247,7 +249,8 @@ TEST_F(TestImageDeleter, Fail_Delete_Diff_GlobalId) { m_image_name, "diff global id"); C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx); + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, "diff global id", + &ctx); EXPECT_EQ(-EINVAL, ctx.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); @@ -261,7 +264,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_Child) { m_image_name, GLOBAL_IMAGE_ID); C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx); + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, + &ctx); EXPECT_EQ(0, ctx.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); @@ -276,7 +280,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_Children) { m_image_name, GLOBAL_IMAGE_ID); C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx); + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, + &ctx); EXPECT_EQ(0, ctx.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); @@ -290,7 +295,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChild) { m_image_name, GLOBAL_IMAGE_ID); C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx); + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, + &ctx); EXPECT_EQ(0, ctx.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); @@ -305,7 +311,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChildren) { m_image_name, GLOBAL_IMAGE_ID); C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx); + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, + &ctx); EXPECT_EQ(0, ctx.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); @@ -319,7 +326,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_Clone) { m_image_name, GLOBAL_IMAGE_ID); C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx); + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, + &ctx); EXPECT_EQ(-EBUSY, ctx.wait()); ASSERT_EQ(1u, m_deleter->get_delete_queue_items().size()); @@ -329,11 +337,13 @@ TEST_F(TestImageDeleter, Delete_Image_With_Clone) { "clone1", GLOBAL_CLONE_IMAGE_ID); C_SaferCond ctx2; - m_deleter->wait_for_scheduled_deletion("clone1", &ctx2); + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_CLONE_IMAGE_ID, + &ctx2); EXPECT_EQ(0, ctx2.wait()); C_SaferCond ctx3; - m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx3); + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, + &ctx3); EXPECT_EQ(0, ctx3.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); @@ -352,7 +362,8 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image) { m_image_name, GLOBAL_IMAGE_ID); C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx); + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, + &ctx); EXPECT_EQ(0, ctx.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); @@ -376,7 +387,8 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image_With_MirroringState) { m_image_name, GLOBAL_IMAGE_ID); C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx); + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, + &ctx); EXPECT_EQ(0, ctx.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); @@ -392,7 +404,8 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image_Without_MirroringState) { m_image_name, GLOBAL_IMAGE_ID); C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx); + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, + &ctx); EXPECT_EQ(-ENOENT, ctx.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); @@ -410,7 +423,8 @@ TEST_F(TestImageDeleter, Fail_Delete_NonPrimary_Image) { m_image_name, GLOBAL_IMAGE_ID); C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx); + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, + &ctx); EXPECT_EQ(-EBUSY, ctx.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); @@ -430,13 +444,15 @@ TEST_F(TestImageDeleter, Retry_Failed_Deletes) { m_image_name, GLOBAL_IMAGE_ID); C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx); + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, + &ctx); EXPECT_EQ(-EBUSY, ctx.wait()); EXPECT_EQ(0, ictx->state->close()); C_SaferCond ctx2; - m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx2); + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, + &ctx2); EXPECT_EQ(0, ctx2.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); @@ -454,7 +470,8 @@ TEST_F(TestImageDeleter, Delete_Is_Idempotent) { m_image_name, GLOBAL_IMAGE_ID); C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx); + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, + &ctx); EXPECT_EQ(-EBUSY, ctx.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); diff --git a/src/test/rbd_mirror/test_ImageReplayer.cc b/src/test/rbd_mirror/test_ImageReplayer.cc index 4e85be9b7dc5e..f36cb2a8be792 100644 --- a/src/test/rbd_mirror/test_ImageReplayer.cc +++ b/src/test/rbd_mirror/test_ImageReplayer.cc @@ -567,7 +567,7 @@ TEST_F(TestImageReplayer, Resync) C_SaferCond delete_ctx; m_image_deleter->wait_for_scheduled_deletion( - m_replayer->get_local_image_name(), &delete_ctx); + m_local_ioctx.get_id(), m_replayer->get_global_image_id(), &delete_ctx); EXPECT_EQ(0, delete_ctx.wait()); C_SaferCond cond; @@ -632,7 +632,7 @@ TEST_F(TestImageReplayer, Resync_While_Stop) C_SaferCond delete_ctx; m_image_deleter->wait_for_scheduled_deletion( - m_replayer->get_local_image_name(), &delete_ctx); + m_local_ioctx.get_id(), m_replayer->get_global_image_id(), &delete_ctx); EXPECT_EQ(0, delete_ctx.wait()); C_SaferCond cond3; @@ -673,7 +673,7 @@ TEST_F(TestImageReplayer, Resync_StartInterrupted) C_SaferCond delete_ctx; m_image_deleter->wait_for_scheduled_deletion( - m_replayer->get_local_image_name(), &delete_ctx); + m_local_ioctx.get_id(), m_replayer->get_global_image_id(), &delete_ctx); EXPECT_EQ(0, delete_ctx.wait()); C_SaferCond cond2; diff --git a/src/tools/rbd_mirror/ImageDeleter.cc b/src/tools/rbd_mirror/ImageDeleter.cc index e4ad55ae60f3e..d69458ce88762 100644 --- a/src/tools/rbd_mirror/ImageDeleter.cc +++ b/src/tools/rbd_mirror/ImageDeleter.cc @@ -192,18 +192,18 @@ void ImageDeleter::run() { } void ImageDeleter::schedule_image_delete(RadosRef local_rados, - uint64_t local_pool_id, + int64_t local_pool_id, const std::string& local_image_id, const std::string& local_image_name, const std::string& global_image_id) { dout(20) << "enter" << dendl; - Mutex::Locker l(m_delete_lock); + Mutex::Locker locker(m_delete_lock); - auto del_info = find_delete_info(local_image_name); + auto del_info = find_delete_info(local_pool_id, global_image_id); if (del_info != nullptr) { - dout(20) << "image " << local_image_name << " was already scheduled for " - << "deletion" << dendl; + dout(20) << "image " << local_image_name << " (" << global_image_id << ") " + << "was already scheduled for deletion" << dendl; return; } @@ -213,7 +213,8 @@ void ImageDeleter::schedule_image_delete(RadosRef local_rados, m_delete_queue_cond.Signal(); } -void ImageDeleter::wait_for_scheduled_deletion(const std::string& image_name, +void ImageDeleter::wait_for_scheduled_deletion(int64_t local_pool_id, + const std::string &global_image_id, Context *ctx, bool notify_on_failed_retry) { @@ -221,8 +222,8 @@ void ImageDeleter::wait_for_scheduled_deletion(const std::string& image_name, m_work_queue->queue(ctx, r); }); - Mutex::Locker l(m_delete_lock); - auto del_info = find_delete_info(image_name); + Mutex::Locker locker(m_delete_lock); + auto del_info = find_delete_info(local_pool_id, global_image_id); if (!del_info) { // image not scheduled for deletion ctx->complete(0); @@ -236,9 +237,10 @@ void ImageDeleter::wait_for_scheduled_deletion(const std::string& image_name, (*del_info)->notify_on_failed_retry = notify_on_failed_retry; } -void ImageDeleter::cancel_waiter(const std::string& image_name) { +void ImageDeleter::cancel_waiter(int64_t local_pool_id, + const std::string &global_image_id) { Mutex::Locker locker(m_delete_lock); - auto del_info = find_delete_info(image_name); + auto del_info = find_delete_info(local_pool_id, global_image_id); if (!del_info) { return; } @@ -506,21 +508,22 @@ void ImageDeleter::retry_failed_deletions() { } unique_ptr const* ImageDeleter::find_delete_info( - const std::string& image_name) { + int64_t local_pool_id, const std::string &global_image_id) { assert(m_delete_lock.is_locked()); - if (m_active_delete && m_active_delete->match(image_name)) { + if (m_active_delete && m_active_delete->match(local_pool_id, + global_image_id)) { return &m_active_delete; } for (const auto& del_info : m_delete_queue) { - if (del_info->match(image_name)) { + if (del_info->match(local_pool_id, global_image_id)) { return &del_info; } } for (const auto& del_info : m_failed_queue) { - if (del_info->match(image_name)) { + if (del_info->match(local_pool_id, global_image_id)) { return &del_info; } } diff --git a/src/tools/rbd_mirror/ImageDeleter.h b/src/tools/rbd_mirror/ImageDeleter.h index 3d994b1bbc94b..d9b0f2e743405 100644 --- a/src/tools/rbd_mirror/ImageDeleter.h +++ b/src/tools/rbd_mirror/ImageDeleter.h @@ -44,14 +44,16 @@ class ImageDeleter { ImageDeleter& operator=(const ImageDeleter&) = delete; void schedule_image_delete(RadosRef local_rados, - uint64_t local_pool_id, + int64_t local_pool_id, const std::string& local_image_id, const std::string& local_image_name, const std::string& global_image_id); - void wait_for_scheduled_deletion(const std::string& image_name, + void wait_for_scheduled_deletion(int64_t local_pool_id, + const std::string &global_image_id, Context *ctx, bool notify_on_failed_retry=true); - void cancel_waiter(const std::string& image_name); + void cancel_waiter(int64_t local_pool_id, + const std::string &global_image_id); void print_status(Formatter *f, std::stringstream *ss); @@ -75,7 +77,7 @@ class ImageDeleter { struct DeleteInfo { RadosRef local_rados; - uint64_t local_pool_id; + int64_t local_pool_id; std::string local_image_id; std::string local_image_name; std::string global_image_id; @@ -84,7 +86,7 @@ class ImageDeleter { bool notify_on_failed_retry; Context *on_delete; - DeleteInfo(RadosRef local_rados, uint64_t local_pool_id, + DeleteInfo(RadosRef local_rados, int64_t local_pool_id, const std::string& local_image_id, const std::string& local_image_name, const std::string& global_image_id) : @@ -94,8 +96,9 @@ class ImageDeleter { notify_on_failed_retry(true), on_delete(nullptr) { } - bool match(const std::string& image_name) { - return local_image_name == image_name; + bool match(int64_t local_pool_id, const std::string &global_image_id) { + return (this->local_pool_id == local_pool_id && + this->global_image_id == global_image_id); } void notify(int r); void to_string(std::stringstream& ss); @@ -133,7 +136,9 @@ class ImageDeleter { void enqueue_failed_delete(int error_code); void retry_failed_deletions(); - unique_ptr const* find_delete_info(const std::string& image_name); + unique_ptr const* + find_delete_info(int64_t local_pool_id, const std::string &global_image_id); + }; } // namespace mirror diff --git a/src/tools/rbd_mirror/Replayer.cc b/src/tools/rbd_mirror/Replayer.cc index fd13a8dec61b6..f3ccca9b35b2a 100644 --- a/src/tools/rbd_mirror/Replayer.cc +++ b/src/tools/rbd_mirror/Replayer.cc @@ -740,7 +740,9 @@ void Replayer::start_image_replayer(unique_ptr > &image_replayer } } ); - m_image_deleter->wait_for_scheduled_deletion(image_name.get(), ctx, false); + + m_image_deleter->wait_for_scheduled_deletion( + m_local_pool_id, image_replayer->get_global_image_id(), ctx, false); } } @@ -752,7 +754,9 @@ bool Replayer::stop_image_replayer(unique_ptr > &image_replayer) // TODO: check how long it is stopping and alert if it is too long. if (image_replayer->is_stopped()) { - m_image_deleter->cancel_waiter(image_replayer->get_local_image_name()); + m_image_deleter->cancel_waiter(m_local_pool_id, + image_replayer->get_global_image_id()); + if (!m_stopping.read()) { dout(20) << "scheduling delete" << dendl; m_image_deleter->schedule_image_delete(