From: Jason Dillaman Date: Tue, 22 Nov 2016 15:57:57 +0000 (-0500) Subject: rbd-mirror: utilize global image id as internal unique key X-Git-Tag: v12.0.1~375^2~5 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=ced71875671e277b192d19ac09452ce47b7fa5d7;p=ceph.git rbd-mirror: utilize global image id as internal unique key Signed-off-by: Jason Dillaman --- diff --git a/src/test/rbd_mirror/test_ImageDeleter.cc b/src/test/rbd_mirror/test_ImageDeleter.cc index 88ca29cebc5b..6e44e5279e5d 100644 --- a/src/test/rbd_mirror/test_ImageDeleter.cc +++ b/src/test/rbd_mirror/test_ImageDeleter.cc @@ -216,7 +216,7 @@ int64_t TestImageDeleter::m_local_pool_id; TEST_F(TestImageDeleter, Delete_NonPrimary_Image) { m_deleter->schedule_image_delete(_rados, m_local_pool_id, m_local_image_id, - m_image_name, GLOBAL_IMAGE_ID); + GLOBAL_IMAGE_ID); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -233,7 +233,7 @@ TEST_F(TestImageDeleter, Fail_Delete_Primary_Image) { promote_image(); m_deleter->schedule_image_delete(_rados, m_local_pool_id, m_local_image_id, - m_image_name, GLOBAL_IMAGE_ID); + GLOBAL_IMAGE_ID); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -249,7 +249,7 @@ TEST_F(TestImageDeleter, Fail_Delete_Diff_GlobalId) { // there is bug in the implementation m_deleter->schedule_image_delete(_rados, m_local_pool_id, m_local_image_id, - m_image_name, "diff global id"); + "diff global id"); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, "diff global id", @@ -264,7 +264,7 @@ TEST_F(TestImageDeleter, Delete_Image_With_Child) { create_snapshot(); m_deleter->schedule_image_delete(_rados, m_local_pool_id, m_local_image_id, - m_image_name, GLOBAL_IMAGE_ID); + GLOBAL_IMAGE_ID); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -280,7 +280,7 @@ TEST_F(TestImageDeleter, Delete_Image_With_Children) { create_snapshot("snap2"); m_deleter->schedule_image_delete(_rados, m_local_pool_id, m_local_image_id, - m_image_name, GLOBAL_IMAGE_ID); + GLOBAL_IMAGE_ID); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -295,7 +295,7 @@ TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChild) { create_snapshot("snap1", true); m_deleter->schedule_image_delete(_rados, m_local_pool_id, m_local_image_id, - m_image_name, GLOBAL_IMAGE_ID); + GLOBAL_IMAGE_ID); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -311,7 +311,7 @@ TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChildren) { create_snapshot("snap2", true); m_deleter->schedule_image_delete(_rados, m_local_pool_id, m_local_image_id, - m_image_name, GLOBAL_IMAGE_ID); + GLOBAL_IMAGE_ID); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -326,7 +326,7 @@ TEST_F(TestImageDeleter, Delete_Image_With_Clone) { std::string clone_id = create_clone(); m_deleter->schedule_image_delete(_rados, m_local_pool_id, m_local_image_id, - m_image_name, GLOBAL_IMAGE_ID); + GLOBAL_IMAGE_ID); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -337,7 +337,7 @@ TEST_F(TestImageDeleter, Delete_Image_With_Clone) { ASSERT_EQ(0u, m_deleter->get_failed_queue_items().size()); m_deleter->schedule_image_delete(_rados, m_local_pool_id, clone_id, - "clone1", GLOBAL_CLONE_IMAGE_ID); + GLOBAL_CLONE_IMAGE_ID); C_SaferCond ctx2; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_CLONE_IMAGE_ID, @@ -362,7 +362,7 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image) { mirror_image)); m_deleter->schedule_image_delete(_rados, m_local_pool_id, m_local_image_id, - m_image_name, GLOBAL_IMAGE_ID); + GLOBAL_IMAGE_ID); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -387,7 +387,7 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image_With_MirroringState) { mirror_image)); m_deleter->schedule_image_delete(_rados, m_local_pool_id, m_local_image_id, - m_image_name, GLOBAL_IMAGE_ID); + GLOBAL_IMAGE_ID); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -404,7 +404,7 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image_Without_MirroringState) { remove_image(); m_deleter->schedule_image_delete(_rados, m_local_pool_id, m_local_image_id, - m_image_name, GLOBAL_IMAGE_ID); + GLOBAL_IMAGE_ID); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -423,7 +423,7 @@ TEST_F(TestImageDeleter, Fail_Delete_NonPrimary_Image) { EXPECT_EQ(0, ictx->state->open(false)); m_deleter->schedule_image_delete(_rados, m_local_pool_id, m_local_image_id, - m_image_name, GLOBAL_IMAGE_ID); + GLOBAL_IMAGE_ID); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -444,7 +444,7 @@ TEST_F(TestImageDeleter, Retry_Failed_Deletes) { m_deleter->set_failed_timer_interval(2); m_deleter->schedule_image_delete(_rados, m_local_pool_id, m_local_image_id, - m_image_name, GLOBAL_IMAGE_ID); + GLOBAL_IMAGE_ID); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -470,7 +470,7 @@ TEST_F(TestImageDeleter, Delete_Is_Idempotent) { EXPECT_EQ(0, ictx->state->open(false)); m_deleter->schedule_image_delete(_rados, m_local_pool_id, m_local_image_id, - m_image_name, GLOBAL_IMAGE_ID); + GLOBAL_IMAGE_ID); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -481,7 +481,7 @@ TEST_F(TestImageDeleter, Delete_Is_Idempotent) { ASSERT_EQ(1u, m_deleter->get_failed_queue_items().size()); m_deleter->schedule_image_delete(_rados, m_local_pool_id, m_local_image_id, - m_image_name, GLOBAL_IMAGE_ID); + GLOBAL_IMAGE_ID); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); ASSERT_EQ(1u, m_deleter->get_failed_queue_items().size()); diff --git a/src/test/rbd_mirror/test_PoolWatcher.cc b/src/test/rbd_mirror/test_PoolWatcher.cc index e177abaf441c..beb95d1db3a5 100644 --- a/src/test/rbd_mirror/test_PoolWatcher.cc +++ b/src/test/rbd_mirror/test_PoolWatcher.cc @@ -109,7 +109,7 @@ TestPoolWatcher() : m_lock("TestPoolWatcherLock"), image.close(); m_mirrored_images.insert(PoolWatcher::ImageId( - get_image_id(&ioctx, name), name, mirror_image_info.global_id)); + mirror_image_info.global_id, get_image_id(&ioctx, name), name)); } if (image_name != nullptr) *image_name = name; @@ -155,7 +155,7 @@ TestPoolWatcher() : m_lock("TestPoolWatcherLock"), image.close(); m_mirrored_images.insert(PoolWatcher::ImageId( - get_image_id(&cioctx, name), name, mirror_image_info.global_id)); + mirror_image_info.global_id, get_image_id(&cioctx, name), name)); } if (image_name != nullptr) *image_name = name; diff --git a/src/tools/rbd_mirror/ImageDeleter.cc b/src/tools/rbd_mirror/ImageDeleter.cc index 39bf57dd813f..70b5baab1c3f 100644 --- a/src/tools/rbd_mirror/ImageDeleter.cc +++ b/src/tools/rbd_mirror/ImageDeleter.cc @@ -199,7 +199,6 @@ void ImageDeleter::run() { void ImageDeleter::schedule_image_delete(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) { dout(20) << "enter" << dendl; @@ -207,14 +206,14 @@ void ImageDeleter::schedule_image_delete(RadosRef local_rados, auto del_info = find_delete_info(local_pool_id, global_image_id); if (del_info != nullptr) { - dout(20) << "image " << local_image_name << " (" << global_image_id << ") " + dout(20) << "image " << local_image_id << " (" << global_image_id << ") " << "was already scheduled for deletion" << dendl; return; } m_delete_queue.push_front(unique_ptr( new DeleteInfo(local_rados, local_pool_id, local_image_id, - local_image_name, global_image_id))); + global_image_id))); m_delete_queue_cond.Signal(); } @@ -235,6 +234,9 @@ void ImageDeleter::wait_for_scheduled_deletion(int64_t local_pool_id, return; } + dout(20) << "local_pool_id=" << local_pool_id << ", " + << "global_image_id=" << global_image_id << dendl; + if ((*del_info)->on_delete != nullptr) { (*del_info)->on_delete->complete(-ESTALE); } @@ -414,8 +416,9 @@ bool ImageDeleter::process_image_delete() { librbd::NoOpProgressContext ctx; r = librbd::remove(ioctx, "", m_active_delete->local_image_id, ctx, true); if (r < 0 && r != -ENOENT) { - derr << "error removing image " << m_active_delete->local_image_name - << " from local pool: " << cpp_strerror(r) << dendl; + derr << "error removing image " << m_active_delete->local_image_id << " " + << "(" << m_active_delete->global_image_id << ") from local pool: " + << cpp_strerror(r) << dendl; enqueue_failed_delete(r); return true; } @@ -435,8 +438,9 @@ bool ImageDeleter::process_image_delete() { return true; } - dout(10) << "Successfully deleted image: " - << m_active_delete->local_image_name << dendl; + dout(10) << "Successfully deleted image " + << m_active_delete->local_image_id << " " + << "(" << m_active_delete->global_image_id << ")" << dendl; complete_active_delete(0); return true; @@ -603,7 +607,7 @@ vector ImageDeleter::get_delete_queue_items() { Mutex::Locker l(m_delete_lock); for (const auto& del_info : m_delete_queue) { - items.push_back(del_info->local_image_name); + items.push_back(del_info->local_image_id); } return items; @@ -614,7 +618,7 @@ vector > ImageDeleter::get_failed_queue_items() { Mutex::Locker l(m_delete_lock); for (const auto& del_info : m_failed_queue) { - items.push_back(make_pair(del_info->local_image_name, + items.push_back(make_pair(del_info->local_image_id, del_info->error_code)); } diff --git a/src/tools/rbd_mirror/ImageDeleter.h b/src/tools/rbd_mirror/ImageDeleter.h index 32d1c66345b9..64d0671dfb25 100644 --- a/src/tools/rbd_mirror/ImageDeleter.h +++ b/src/tools/rbd_mirror/ImageDeleter.h @@ -46,7 +46,6 @@ public: void schedule_image_delete(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); void wait_for_scheduled_deletion(int64_t local_pool_id, const std::string &global_image_id, @@ -79,7 +78,6 @@ private: RadosRef local_rados; int64_t local_pool_id; std::string local_image_id; - std::string local_image_name; std::string global_image_id; int error_code; int retries; @@ -88,12 +86,11 @@ private: 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) : local_rados(local_rados), local_pool_id(local_pool_id), - local_image_id(local_image_id), local_image_name(local_image_name), - global_image_id(global_image_id), error_code(0), retries(0), - notify_on_failed_retry(true), on_delete(nullptr) { + local_image_id(local_image_id), global_image_id(global_image_id), + error_code(0), retries(0), notify_on_failed_retry(true), + on_delete(nullptr) { } bool match(int64_t local_pool_id, const std::string &global_image_id) { diff --git a/src/tools/rbd_mirror/ImageReplayer.cc b/src/tools/rbd_mirror/ImageReplayer.cc index 132db547da95..13a78741d51c 100644 --- a/src/tools/rbd_mirror/ImageReplayer.cc +++ b/src/tools/rbd_mirror/ImageReplayer.cc @@ -1495,7 +1495,6 @@ void ImageReplayer::handle_shut_down(int r) { m_image_deleter->schedule_image_delete(m_local, m_local_pool_id, m_local_image_id, - m_local_image_name, m_global_image_id); m_stopping_for_resync = false; } diff --git a/src/tools/rbd_mirror/PoolWatcher.cc b/src/tools/rbd_mirror/PoolWatcher.cc index 360e00dd48a9..23b2cca17bdd 100644 --- a/src/tools/rbd_mirror/PoolWatcher.cc +++ b/src/tools/rbd_mirror/PoolWatcher.cc @@ -129,7 +129,7 @@ int PoolWatcher::refresh(ImageIds *image_ids) { if (it2 != image_id_to_name.end()) { image_name = it2->second; } - image_ids->insert(ImageId(it->first, image_name, it->second)); + image_ids->insert(ImageId(it->second, it->first, image_name)); } if (!mirror_images.empty()) { last_read = mirror_images.rbegin()->first; diff --git a/src/tools/rbd_mirror/PoolWatcher.h b/src/tools/rbd_mirror/PoolWatcher.h index 4aeca3dc23cd..9e1e75b64af3 100644 --- a/src/tools/rbd_mirror/PoolWatcher.h +++ b/src/tools/rbd_mirror/PoolWatcher.h @@ -14,6 +14,7 @@ #include "common/Timer.h" #include "include/rados/librados.hpp" #include "types.h" +#include namespace rbd { namespace mirror { @@ -25,21 +26,20 @@ namespace mirror { class PoolWatcher { public: struct ImageId { + std::string global_id; std::string id; boost::optional name; - std::string global_id; - ImageId(const std::string &id, - const boost::optional &name = boost::none, - const std::string &global_id = "") - : id(id), name(name), global_id(global_id) { + ImageId(const std::string &global_id, const std::string &id = "", + const boost::optional &name = boost::none) + : global_id(global_id), id(id), name(name) { } inline bool operator==(const ImageId &rhs) const { - return (id == rhs.id && name == rhs.name && global_id == rhs.global_id); + return (global_id == rhs.global_id && id == rhs.id && name == rhs.name); } inline bool operator<(const ImageId &rhs) const { - return id < rhs.id; + return global_id < rhs.global_id; } }; typedef std::set ImageIds; diff --git a/src/tools/rbd_mirror/Replayer.cc b/src/tools/rbd_mirror/Replayer.cc index 2da5d86bfe6f..095d24970cd7 100644 --- a/src/tools/rbd_mirror/Replayer.cc +++ b/src/tools/rbd_mirror/Replayer.cc @@ -390,7 +390,7 @@ void Replayer::init_local_mirroring_images() { return; } - std::set images; + ImageIds image_ids; std::string last_read = ""; int max_read = 1024; @@ -411,7 +411,7 @@ void Replayer::init_local_mirroring_images() { << dendl; continue; } - images.insert(InitImageInfo(it->second, it->first, image_name)); + image_ids.insert(ImageId(it->second, it->first, image_name)); } if (!mirror_images.empty()) { last_read = mirror_images.rbegin()->first; @@ -419,7 +419,7 @@ void Replayer::init_local_mirroring_images() { r = mirror_images.size(); } while (r == max_read); - m_init_images = std::move(images); + m_init_image_ids = std::move(image_ids); } void Replayer::run() @@ -582,33 +582,34 @@ void Replayer::set_sources(const ImageIds &image_ids) assert(m_lock.is_locked()); - if (!m_init_images.empty() && !m_stopping.read() && + if (!m_init_image_ids.empty() && !m_stopping.read() && m_leader_watcher->is_leader()) { dout(20) << "scanning initial local image set" << dendl; for (auto &remote_image : image_ids) { - auto it = m_init_images.find(InitImageInfo(remote_image.global_id)); - if (it != m_init_images.end()) { - m_init_images.erase(it); + auto it = m_init_image_ids.find(ImageId(remote_image.global_id)); + if (it != m_init_image_ids.end()) { + m_init_image_ids.erase(it); } } - // the remaining images in m_init_images must be deleted - for (auto &image : m_init_images) { + // the remaining images in m_init_image_ids must be deleted + for (auto &image_id : m_init_image_ids) { dout(20) << "scheduling the deletion of init image: " - << image.name << dendl; + << image_id.global_id << " (" << image_id.id << ")" << dendl; m_image_deleter->schedule_image_delete(m_local_rados, m_local_pool_id, - image.id, image.name, - image.global_id); + image_id.id, image_id.global_id); } - m_init_images.clear(); + m_init_image_ids.clear(); } // shut down replayers for non-mirrored images for (auto image_it = m_image_replayers.begin(); image_it != m_image_replayers.end();) { - if (image_ids.find(ImageId(image_it->first)) == image_ids.end()) { + auto image_id_it = image_ids.find(image_it->first); + if (image_id_it == image_ids.end() || + image_id_it->id != image_it->second->get_remote_image_id()) { if (image_it->second->is_running()) { - dout(20) << "stop image replayer for " + dout(20) << "stop image replayer for remote image " << image_it->second->get_global_image_id() << dendl; } if (stop_image_replayer(image_it->second)) { @@ -642,30 +643,32 @@ void Replayer::set_sources(const ImageIds &image_ids) } for (auto &image_id : image_ids) { - auto it = m_image_replayers.find(image_id.id); + auto it = m_image_replayers.find(image_id.global_id); if (it == m_image_replayers.end()) { unique_ptr > image_replayer(new ImageReplayer<>( m_threads, m_image_deleter, m_image_sync_throttler, m_local_rados, m_remote_rados, local_mirror_uuid, remote_mirror_uuid, m_local_pool_id, m_remote_pool_id, image_id.id, image_id.global_id)); it = m_image_replayers.insert( - std::make_pair(image_id.id, std::move(image_replayer))).first; + std::make_pair(image_id.global_id, std::move(image_replayer))).first; + } else if (image_id.id != it->second->get_remote_image_id()) { + // mismatched replayer in progress of stopping + continue; } if (!it->second->is_running()) { - dout(20) << "starting image replayer for " - << it->second->get_global_image_id() << dendl; + dout(20) << "starting image replayer for remote image " + << image_id.global_id << dendl; } - start_image_replayer(it->second, image_id.id, image_id.name); + start_image_replayer(it->second); } } -void Replayer::start_image_replayer(unique_ptr > &image_replayer, - const std::string &image_id, - const boost::optional& image_name) +void Replayer::start_image_replayer(unique_ptr > &image_replayer) { assert(m_lock.is_locked()); - dout(20) << "global_image_id=" << image_replayer->get_global_image_id() - << dendl; + + std::string global_image_id = image_replayer->get_global_image_id(); + dout(20) << "global_image_id=" << global_image_id << dendl; if (!image_replayer->is_stopped()) { return; @@ -676,31 +679,31 @@ void Replayer::start_image_replayer(unique_ptr > &image_replayer return; } - if (image_name) { - FunctionContext *ctx = new FunctionContext( - [this, image_id, image_name] (int r) { - if (r == -ESTALE || r == -ECANCELED) { - return; - } + FunctionContext *ctx = new FunctionContext( + [this, global_image_id] (int r) { + dout(20) << "image deleter result: r=" << r << ", " + << "global_image_id=" << global_image_id << dendl; + if (r == -ESTALE || r == -ECANCELED) { + return; + } - Mutex::Locker locker(m_lock); - auto it = m_image_replayers.find(image_id); - if (it == m_image_replayers.end()) { - return; - } + Mutex::Locker locker(m_lock); + auto it = m_image_replayers.find(global_image_id); + if (it == m_image_replayers.end()) { + return; + } - auto &image_replayer = it->second; - if (r >= 0) { - image_replayer->start(); - } else { - start_image_replayer(image_replayer, image_id, image_name); - } - } - ); + auto &image_replayer = it->second; + if (r >= 0) { + image_replayer->start(); + } else { + start_image_replayer(image_replayer); + } + } + ); - m_image_deleter->wait_for_scheduled_deletion( - m_local_pool_id, image_replayer->get_global_image_id(), ctx, false); - } + m_image_deleter->wait_for_scheduled_deletion( + m_local_pool_id, image_replayer->get_global_image_id(), ctx, false); } bool Replayer::stop_image_replayer(unique_ptr > &image_replayer) @@ -720,7 +723,6 @@ bool Replayer::stop_image_replayer(unique_ptr > &image_replayer) m_local_rados, image_replayer->get_local_pool_id(), image_replayer->get_local_image_id(), - image_replayer->get_local_image_name(), image_replayer->get_global_image_id()); } return true; @@ -735,7 +737,6 @@ bool Replayer::stop_image_replayer(unique_ptr > &image_replayer) m_local_rados, image_replayer->get_local_pool_id(), image_replayer->get_local_image_id(), - image_replayer->get_local_image_name(), image_replayer->get_global_image_id()); } } diff --git a/src/tools/rbd_mirror/Replayer.h b/src/tools/rbd_mirror/Replayer.h index ddf99d662452..c60c86c88341 100644 --- a/src/tools/rbd_mirror/Replayer.h +++ b/src/tools/rbd_mirror/Replayer.h @@ -61,9 +61,7 @@ private: void init_local_mirroring_images(); void set_sources(const ImageIds &image_ids); - void start_image_replayer(unique_ptr > &image_replayer, - const std::string &image_id, - const boost::optional& image_name); + void start_image_replayer(unique_ptr > &image_replayer); bool stop_image_replayer(unique_ptr > &image_replayer); int init_rados(const std::string &cluster_name, const std::string &client_name, @@ -98,25 +96,7 @@ private: std::string m_asok_hook_name; ReplayerAdminSocketHook *m_asok_hook; - struct InitImageInfo { - std::string global_id; - std::string id; - std::string name; - - InitImageInfo(const std::string& global_id, const std::string &id = "", - const std::string &name = "") - : global_id(global_id), id(id), name(name) { - } - - inline bool operator==(const InitImageInfo &rhs) const { - return (global_id == rhs.global_id && id == rhs.id && name == rhs.name); - } - inline bool operator<(const InitImageInfo &rhs) const { - return global_id < rhs.global_id; - } - }; - - std::set m_init_images; + std::set m_init_image_ids; class ReplayerThread : public Thread { Replayer *m_replayer;