From 6fae4df02e70d8d9ed7ac19a3b0e0a0fc8505f4c Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 14 Mar 2018 12:54:53 -0400 Subject: [PATCH] rbd-mirror: simplify the interface between image mapper and policy The context callbacks between the image map and its policy required too many hooks between the two classes. Additionally, the original implementation didn't support re-sending of acquire messages after initializing (to close a potential race condition). Signed-off-by: Jason Dillaman --- src/test/rbd_mirror/CMakeLists.txt | 2 +- .../test_Policy.cc} | 281 +++------ src/test/rbd_mirror/test_mock_ImageMap.cc | 147 +++-- src/tools/rbd_mirror/CMakeLists.txt | 1 - src/tools/rbd_mirror/ImageMap.cc | 370 +++++------- src/tools/rbd_mirror/ImageMap.h | 92 +-- src/tools/rbd_mirror/image_map/Action.cc | 89 --- src/tools/rbd_mirror/image_map/Action.h | 42 -- src/tools/rbd_mirror/image_map/Policy.cc | 533 ++++++++---------- src/tools/rbd_mirror/image_map/Policy.h | 163 ++---- .../rbd_mirror/image_map/SimplePolicy.cc | 49 +- src/tools/rbd_mirror/image_map/SimplePolicy.h | 16 +- .../rbd_mirror/image_map/StateTransition.cc | 108 ++-- .../rbd_mirror/image_map/StateTransition.h | 77 +-- src/tools/rbd_mirror/image_map/Types.cc | 25 + src/tools/rbd_mirror/image_map/Types.h | 17 +- 16 files changed, 816 insertions(+), 1196 deletions(-) rename src/test/rbd_mirror/{test_ImagePolicy.cc => image_map/test_Policy.cc} (54%) delete mode 100644 src/tools/rbd_mirror/image_map/Action.cc delete mode 100644 src/tools/rbd_mirror/image_map/Action.h diff --git a/src/test/rbd_mirror/CMakeLists.txt b/src/test/rbd_mirror/CMakeLists.txt index 4744f946ba36a..a86dca523efec 100644 --- a/src/test/rbd_mirror/CMakeLists.txt +++ b/src/test/rbd_mirror/CMakeLists.txt @@ -2,13 +2,13 @@ set(rbd_mirror_test_srcs test_ClusterWatcher.cc test_PoolWatcher.cc test_ImageDeleter.cc - test_ImagePolicy.cc test_ImageReplayer.cc test_ImageSync.cc test_InstanceWatcher.cc test_Instances.cc test_LeaderWatcher.cc test_fixture.cc + image_map/test_Policy.cc ) add_library(rbd_mirror_test STATIC ${rbd_mirror_test_srcs}) set_target_properties(rbd_mirror_test PROPERTIES COMPILE_FLAGS diff --git a/src/test/rbd_mirror/test_ImagePolicy.cc b/src/test/rbd_mirror/image_map/test_Policy.cc similarity index 54% rename from src/test/rbd_mirror/test_ImagePolicy.cc rename to src/test/rbd_mirror/image_map/test_Policy.cc index a0320534137e6..fe5692d9c9d59 100644 --- a/src/test/rbd_mirror/test_ImagePolicy.cc +++ b/src/test/rbd_mirror/image_map/test_Policy.cc @@ -15,11 +15,14 @@ namespace rbd { namespace mirror { namespace image_map { -class TestImagePolicy : public TestFixture { +class TestImageMapPolicy : public TestFixture { public: void SetUp() override { TestFixture::SetUp(); + EXPECT_EQ(0, _rados->conf_set("rbd_mirror_image_policy_migration_throttle", + "0")); + CephContext *cct = reinterpret_cast(m_local_io_ctx.cct()); std::string policy_type = cct->_conf->get_val("rbd_mirror_image_policy_type"); @@ -37,135 +40,57 @@ public: delete m_policy; } - struct C_UpdateMap : Context { - TestImagePolicy *test; - std::string global_image_id; - - C_UpdateMap(TestImagePolicy *test, const std::string &global_image_id) - : test(test), - global_image_id(global_image_id) { - } - - void finish(int r) override { - test->m_updated = true; - } - - void complete(int r) override { - finish(r); - } - }; - struct C_RemoveMap : Context { - TestImagePolicy *test; - std::string global_image_id; - - C_RemoveMap(TestImagePolicy *test, const std::string &global_image_id) - : test(test), - global_image_id(global_image_id) { - } - - void finish(int r) override { - test->m_removed = true; - } - - void complete(int r) override { - finish(r); - } - }; - - struct C_AcquireImage : Context { - TestImagePolicy *test; - std::string global_image_id; - - C_AcquireImage(TestImagePolicy *test, const std::string &global_image_id) - : test(test), - global_image_id(global_image_id) { - } - - void finish(int r) override { - test->m_acquired = true; - } - - void complete(int r) override { - finish(r); - } - }; - - void reset_flags() { - m_updated = false; - m_removed = false; - m_acquired = false; - m_released = false; - } - void map_image(const std::string &global_image_id) { - Context *on_update = new C_UpdateMap(this, global_image_id); - Context *on_acquire = new C_AcquireImage(this, global_image_id); - - ASSERT_TRUE(m_policy->add_image(global_image_id, on_update, on_acquire, nullptr)); + ASSERT_TRUE(m_policy->add_image(global_image_id)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_MAP_UPDATE, m_policy->start_action(global_image_id)); ASSERT_TRUE(m_policy->finish_action(global_image_id, 0)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_ACQUIRE, m_policy->start_action(global_image_id)); ASSERT_FALSE(m_policy->finish_action(global_image_id, 0)); - - ASSERT_TRUE(m_updated && m_acquired); } void unmap_image(const std::string &global_image_id) { - Context *on_release = new FunctionContext([this, global_image_id](int r) { - m_released = true; - }); - Context *on_remove = new C_RemoveMap(this, global_image_id); + ASSERT_TRUE(m_policy->remove_image(global_image_id)); - ASSERT_TRUE(m_policy->remove_image(global_image_id, on_release, on_remove, nullptr)); - - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_RELEASE, m_policy->start_action(global_image_id)); ASSERT_TRUE(m_policy->finish_action(global_image_id, 0)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_MAP_REMOVE, m_policy->start_action(global_image_id)); ASSERT_FALSE(m_policy->finish_action(global_image_id, 0)); - - ASSERT_TRUE(m_released && m_removed); } void shuffle_image(const std::string &global_image_id) { - Context *on_release = new FunctionContext([this, global_image_id](int r) { - m_released = true; - }); - Context *on_update = new C_UpdateMap(this, global_image_id); - Context *on_acquire = new C_AcquireImage(this, global_image_id); - - ASSERT_TRUE(m_policy->shuffle_image(global_image_id, on_release, - on_update, on_acquire, nullptr)); - - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_RELEASE, m_policy->start_action(global_image_id)); ASSERT_TRUE(m_policy->finish_action(global_image_id, 0)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_MAP_UPDATE, m_policy->start_action(global_image_id)); ASSERT_TRUE(m_policy->finish_action(global_image_id, 0)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_ACQUIRE, m_policy->start_action(global_image_id)); ASSERT_FALSE(m_policy->finish_action(global_image_id, 0)); - - ASSERT_TRUE(m_released && m_updated && m_acquired); } Policy *m_policy; - bool m_updated = false; - bool m_removed = false; - bool m_acquired = false; - bool m_released = false; }; -TEST_F(TestImagePolicy, NegativeLookup) { +TEST_F(TestImageMapPolicy, NegativeLookup) { const std::string global_image_id = "global id 1"; LookupInfo info = m_policy->lookup(global_image_id); ASSERT_TRUE(info.instance_id == UNMAPPED_INSTANCE_ID); } -TEST_F(TestImagePolicy, MapImage) { +TEST_F(TestImageMapPolicy, Init) { + const std::string global_image_id = "global id 1"; + + m_policy->init({{global_image_id, {"9876", {}, {}}}}); + + ASSERT_EQ(ACTION_TYPE_ACQUIRE, m_policy->start_action(global_image_id)); + ASSERT_FALSE(m_policy->finish_action(global_image_id, 0)); +} + +TEST_F(TestImageMapPolicy, MapImage) { const std::string global_image_id = "global id 1"; map_image(global_image_id); @@ -174,7 +99,7 @@ TEST_F(TestImagePolicy, MapImage) { ASSERT_TRUE(info.instance_id != UNMAPPED_INSTANCE_ID); } -TEST_F(TestImagePolicy, UnmapImage) { +TEST_F(TestImageMapPolicy, UnmapImage) { const std::string global_image_id = "global id 1"; // map image @@ -183,8 +108,6 @@ TEST_F(TestImagePolicy, UnmapImage) { LookupInfo info = m_policy->lookup(global_image_id); ASSERT_TRUE(info.instance_id != UNMAPPED_INSTANCE_ID); - reset_flags(); - // unmap image unmap_image(global_image_id); @@ -192,7 +115,7 @@ TEST_F(TestImagePolicy, UnmapImage) { ASSERT_TRUE(info.instance_id == UNMAPPED_INSTANCE_ID); } -TEST_F(TestImagePolicy, ShuffleImageAddInstance) { +TEST_F(TestImageMapPolicy, ShuffleImageAddInstance) { std::set global_image_ids { "global id 1", "global id 2", "global id 3", "global id 4", "global id 5", "global id 6" }; @@ -205,8 +128,6 @@ TEST_F(TestImagePolicy, ShuffleImageAddInstance) { ASSERT_TRUE(info.instance_id != UNMAPPED_INSTANCE_ID); } - reset_flags(); - std::set shuffle_global_image_ids; m_policy->add_instances({"9876"}, &shuffle_global_image_ids); @@ -218,7 +139,7 @@ TEST_F(TestImagePolicy, ShuffleImageAddInstance) { } } -TEST_F(TestImagePolicy, ShuffleImageRemoveInstance) { +TEST_F(TestImageMapPolicy, ShuffleImageRemoveInstance) { std::set global_image_ids { "global id 1", "global id 2", "global id 3", "global id 4", "global id 5" }; @@ -231,8 +152,6 @@ TEST_F(TestImagePolicy, ShuffleImageRemoveInstance) { ASSERT_TRUE(info.instance_id != UNMAPPED_INSTANCE_ID); } - reset_flags(); - std::set shuffle_global_image_ids; m_policy->add_instances({"9876"}, &shuffle_global_image_ids); @@ -252,8 +171,6 @@ TEST_F(TestImagePolicy, ShuffleImageRemoveInstance) { } } - reset_flags(); - shuffle_global_image_ids.clear(); m_policy->remove_instances({"9876"}, &shuffle_global_image_ids); @@ -267,72 +184,67 @@ TEST_F(TestImagePolicy, ShuffleImageRemoveInstance) { } } -TEST_F(TestImagePolicy, RetryMapUpdate) { +TEST_F(TestImageMapPolicy, RetryMapUpdate) { const std::string global_image_id = "global id 1"; - Context *on_update = new C_UpdateMap(this, global_image_id); - Context *on_acquire = new C_AcquireImage(this, global_image_id); - - ASSERT_TRUE(m_policy->add_image(global_image_id, on_update, on_acquire, nullptr)); + ASSERT_TRUE(m_policy->add_image(global_image_id)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_MAP_UPDATE, m_policy->start_action(global_image_id)); // on-disk map update failed ASSERT_TRUE(m_policy->finish_action(global_image_id, -EIO)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_MAP_UPDATE, m_policy->start_action(global_image_id)); ASSERT_TRUE(m_policy->finish_action(global_image_id, 0)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_ACQUIRE, m_policy->start_action(global_image_id)); ASSERT_FALSE(m_policy->finish_action(global_image_id, 0)); - ASSERT_TRUE(m_updated && m_acquired); - LookupInfo info = m_policy->lookup(global_image_id); ASSERT_TRUE(info.instance_id != UNMAPPED_INSTANCE_ID); } -TEST_F(TestImagePolicy, MapFailureAndUnmap) { +TEST_F(TestImageMapPolicy, MapFailureAndUnmap) { const std::string global_image_id = "global id 1"; - Context *on_update = new C_UpdateMap(this, global_image_id); - Context *on_acquire = new C_AcquireImage(this, global_image_id); - - ASSERT_TRUE(m_policy->add_image(global_image_id, on_update, on_acquire, nullptr)); + ASSERT_TRUE(m_policy->add_image(global_image_id)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_MAP_UPDATE, m_policy->start_action(global_image_id)); ASSERT_TRUE(m_policy->finish_action(global_image_id, 0)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_ACQUIRE, m_policy->start_action(global_image_id)); + + std::set shuffle_global_image_ids; + m_policy->add_instances({"9876"}, &shuffle_global_image_ids); + ASSERT_TRUE(shuffle_global_image_ids.empty()); + + m_policy->remove_instances({stringify(m_local_io_ctx.get_instance_id())}, + &shuffle_global_image_ids); + ASSERT_TRUE(shuffle_global_image_ids.empty()); + ASSERT_TRUE(m_policy->finish_action(global_image_id, -EBLACKLISTED)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_RELEASE, m_policy->start_action(global_image_id)); + ASSERT_TRUE(m_policy->finish_action(global_image_id, -ENOENT)); + + ASSERT_EQ(ACTION_TYPE_MAP_UPDATE, m_policy->start_action(global_image_id)); ASSERT_TRUE(m_policy->finish_action(global_image_id, 0)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_ACQUIRE, m_policy->start_action(global_image_id)); ASSERT_FALSE(m_policy->finish_action(global_image_id, 0)); - ASSERT_TRUE(m_updated && m_acquired); + ASSERT_TRUE(m_policy->remove_image(global_image_id)); - reset_flags(); - - Context *on_release = new FunctionContext([this, global_image_id](int r) { - m_released = true; - }); - Context *on_remove = new C_RemoveMap(this, global_image_id); - ASSERT_TRUE(m_policy->remove_image(global_image_id, on_release, on_remove, nullptr)); - - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_RELEASE, m_policy->start_action(global_image_id)); ASSERT_TRUE(m_policy->finish_action(global_image_id, 0)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_MAP_REMOVE, m_policy->start_action(global_image_id)); ASSERT_FALSE(m_policy->finish_action(global_image_id, 0)); - - ASSERT_TRUE(m_removed && m_released); } -TEST_F(TestImagePolicy, ReshuffleWithMapFailure) { +TEST_F(TestImageMapPolicy, ReshuffleWithMapFailure) { std::set global_image_ids { - "global id 1", "global id 2", "global id 3", "global id 4", "global id 5", "global id 6" + "global id 1", "global id 2", "global id 3", "global id 4", "global id 5", + "global id 6" }; for (auto const &global_image_id : global_image_ids) { @@ -345,32 +257,18 @@ TEST_F(TestImagePolicy, ReshuffleWithMapFailure) { std::set shuffle_global_image_ids; m_policy->add_instances({"9876"}, &shuffle_global_image_ids); - - if (shuffle_global_image_ids.empty()) { - return; - } + ASSERT_FALSE(shuffle_global_image_ids.empty()); const std::string global_image_id = *(shuffle_global_image_ids.begin()); shuffle_global_image_ids.clear(); - reset_flags(); - - Context *on_release = new FunctionContext([this, global_image_id](int r) { - m_released = true; - }); - Context *on_update = new C_UpdateMap(this, global_image_id); - Context *on_acquire = new C_AcquireImage(this, global_image_id); - - ASSERT_TRUE(m_policy->shuffle_image(global_image_id, on_release, - on_update, on_acquire, nullptr)); - - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_RELEASE, m_policy->start_action(global_image_id)); ASSERT_TRUE(m_policy->finish_action(global_image_id, 0)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_MAP_UPDATE, m_policy->start_action(global_image_id)); ASSERT_TRUE(m_policy->finish_action(global_image_id, 0)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_ACQUIRE, m_policy->start_action(global_image_id)); // peer unavailable m_policy->remove_instances({"9876"}, &shuffle_global_image_ids); @@ -378,18 +276,20 @@ TEST_F(TestImagePolicy, ReshuffleWithMapFailure) { ASSERT_TRUE(m_policy->finish_action(global_image_id, -EBLACKLISTED)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_RELEASE, m_policy->start_action(global_image_id)); ASSERT_TRUE(m_policy->finish_action(global_image_id, 0)); - m_policy->start_next_action(global_image_id); - ASSERT_FALSE(m_policy->finish_action(global_image_id, 0)); + ASSERT_EQ(ACTION_TYPE_MAP_UPDATE, m_policy->start_action(global_image_id)); + ASSERT_TRUE(m_policy->finish_action(global_image_id, 0)); - ASSERT_TRUE(m_released && m_updated && m_acquired); + ASSERT_EQ(ACTION_TYPE_ACQUIRE, m_policy->start_action(global_image_id)); + ASSERT_FALSE(m_policy->finish_action(global_image_id, 0)); } -TEST_F(TestImagePolicy, ShuffleFailureAndRemove) { +TEST_F(TestImageMapPolicy, ShuffleFailureAndRemove) { std::set global_image_ids { - "global id 1", "global id 2", "global id 3", "global id 4", "global id 5", "global id 6" + "global id 1", "global id 2", "global id 3", "global id 4", "global id 5", + "global id 6" }; for (auto const &global_image_id : global_image_ids) { @@ -402,31 +302,18 @@ TEST_F(TestImagePolicy, ShuffleFailureAndRemove) { std::set shuffle_global_image_ids; m_policy->add_instances({"9876"}, &shuffle_global_image_ids); - if (shuffle_global_image_ids.empty()) { - return; - } + ASSERT_FALSE(shuffle_global_image_ids.empty()); std::string global_image_id = *(shuffle_global_image_ids.begin()); shuffle_global_image_ids.clear(); - reset_flags(); - - Context *on_release = new FunctionContext([this, global_image_id](int r) { - m_released = true; - }); - Context *on_update = new C_UpdateMap(this, global_image_id); - Context *on_acquire = new C_AcquireImage(this, global_image_id); - - ASSERT_TRUE(m_policy->shuffle_image(global_image_id, on_release, - on_update, on_acquire, nullptr)); - - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_RELEASE, m_policy->start_action(global_image_id)); ASSERT_TRUE(m_policy->finish_action(global_image_id, 0)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_MAP_UPDATE, m_policy->start_action(global_image_id)); ASSERT_TRUE(m_policy->finish_action(global_image_id, 0)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_ACQUIRE, m_policy->start_action(global_image_id)); // peer unavailable m_policy->remove_instances({"9876"}, &shuffle_global_image_ids); @@ -434,31 +321,23 @@ TEST_F(TestImagePolicy, ShuffleFailureAndRemove) { ASSERT_TRUE(m_policy->finish_action(global_image_id, -EBLACKLISTED)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_RELEASE, m_policy->start_action(global_image_id)); ASSERT_TRUE(m_policy->finish_action(global_image_id, 0)); - m_policy->start_next_action(global_image_id); - ASSERT_FALSE(m_policy->finish_action(global_image_id, 0)); - - ASSERT_TRUE(m_released && m_updated && m_acquired); - - reset_flags(); + ASSERT_EQ(ACTION_TYPE_MAP_UPDATE, m_policy->start_action(global_image_id)); + ASSERT_TRUE(m_policy->finish_action(global_image_id, 0)); - on_release = new FunctionContext([this, global_image_id](int r) { - m_released = true; - }); - Context *on_remove = new C_RemoveMap(this, global_image_id); + ASSERT_EQ(ACTION_TYPE_ACQUIRE, m_policy->start_action(global_image_id)); + ASSERT_FALSE(m_policy->finish_action(global_image_id, 0)); - ASSERT_TRUE(m_policy->remove_image(global_image_id, on_release, on_remove, nullptr)); + ASSERT_TRUE(m_policy->remove_image(global_image_id)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_RELEASE, m_policy->start_action(global_image_id)); ASSERT_TRUE(m_policy->finish_action(global_image_id, 0)); - m_policy->start_next_action(global_image_id); + ASSERT_EQ(ACTION_TYPE_MAP_REMOVE, m_policy->start_action(global_image_id)); ASSERT_FALSE(m_policy->finish_action(global_image_id, 0)); - ASSERT_TRUE(m_released && m_removed); - LookupInfo info = m_policy->lookup(global_image_id); ASSERT_TRUE(info.instance_id == UNMAPPED_INSTANCE_ID); } diff --git a/src/test/rbd_mirror/test_mock_ImageMap.cc b/src/test/rbd_mirror/test_mock_ImageMap.cc index f9aa231ed247f..12586bdcac385 100644 --- a/src/test/rbd_mirror/test_mock_ImageMap.cc +++ b/src/test/rbd_mirror/test_mock_ImageMap.cc @@ -53,12 +53,15 @@ namespace image_map { template <> struct LoadRequest { + std::map *image_map; Context *on_finish = nullptr; + static LoadRequest *s_instance; static LoadRequest *create(librados::IoCtx &ioctx, std::map *image_map, Context *on_finish) { assert(s_instance != nullptr); + s_instance->image_map = image_map; s_instance->on_finish = on_finish; return s_instance; } @@ -163,6 +166,13 @@ public: m_map_update_count(0) { } + void SetUp() override { + TestFixture::SetUp(); + + EXPECT_EQ(0, _rados->conf_set("rbd_mirror_image_policy_migration_throttle", + "0")); + } + void expect_work_queue(MockThreads &mock_threads) { EXPECT_CALL(*mock_threads.work_queue, queue(_, _)) .WillRepeatedly(Invoke([this](Context *ctx, int r) { @@ -238,11 +248,11 @@ public: }))); } - void expect_listener_images_unmapped(MockListener &mock_listener, + void expect_listener_images_unmapped(MockListener &mock_listener, size_t count, std::set *global_image_ids, std::map *peer_ack_ctxs) { EXPECT_CALL(mock_listener, mock_release_image(_, _)) - .Times(AtLeast(0)) + .Times(count) .WillRepeatedly(Invoke([this, global_image_ids, peer_ack_ctxs](std::string global_image_id, Context* ctx) { Mutex::Locker locker(m_lock); global_image_ids->emplace(global_image_id); @@ -266,14 +276,16 @@ public: void remote_peer_ack_wait(MockImageMap *image_map, const std::set &global_image_ids, - int ret, + int ret, bool expect_map_update, std::map *peer_ack_ctxs) { for (auto& global_image_id : global_image_ids) { auto it = peer_ack_ctxs->find(global_image_id); ASSERT_TRUE(it != peer_ack_ctxs->end()); it->second->complete(ret); peer_ack_ctxs->erase(it); - ASSERT_TRUE(wait_for_map_update(1)); + if (expect_map_update) { + ASSERT_TRUE(wait_for_map_update(1)); + } } } @@ -495,7 +507,7 @@ TEST_F(TestMockImageMap, AddRemoveLocalImage) { ASSERT_TRUE(wait_for_listener_notify(remove_global_image_ids_ack.size())); remote_peer_ack_wait(mock_image_map.get(), remove_global_image_ids_ack, 0, - &peer_ack_ctxs); + true, &peer_ack_ctxs); wait_for_scheduled_task(); ASSERT_EQ(0, when_shut_down(mock_image_map.get())); @@ -550,8 +562,9 @@ TEST_F(TestMockImageMap, AddRemoveRemoteImage) { &peer_ack_ctxs); // RELEASE+REMOVE_MAPPING + std::map peer_remove_ack_ctxs; listener_remove_images(mock_listener, "uuid1", remove_global_image_ids, - &peer_ack_ctxs); + &peer_remove_ack_ctxs); expect_add_event(mock_threads); listener_release_images(mock_listener, remove_global_image_ids, &peer_ack_ctxs); @@ -563,7 +576,9 @@ TEST_F(TestMockImageMap, AddRemoveRemoteImage) { ASSERT_TRUE(wait_for_listener_notify(remove_global_image_ids_ack.size() * 2)); remote_peer_ack_wait(mock_image_map.get(), remove_global_image_ids_ack, 0, - &peer_ack_ctxs); + false, &peer_remove_ack_ctxs); + remote_peer_ack_wait(mock_image_map.get(), remove_global_image_ids_ack, 0, + true, &peer_ack_ctxs); wait_for_scheduled_task(); ASSERT_EQ(0, when_shut_down(mock_image_map.get())); @@ -623,8 +638,9 @@ TEST_F(TestMockImageMap, AddRemoveRemoteImageDuplicateNotification) { &peer_ack_ctxs); // RELEASE+REMOVE_MAPPING + std::map peer_remove_ack_ctxs; listener_remove_images(mock_listener, "uuid1", remove_global_image_ids, - &peer_ack_ctxs); + &peer_remove_ack_ctxs); expect_add_event(mock_threads); listener_release_images(mock_listener, remove_global_image_ids, &peer_ack_ctxs); @@ -635,7 +651,9 @@ TEST_F(TestMockImageMap, AddRemoveRemoteImageDuplicateNotification) { ASSERT_TRUE(wait_for_listener_notify(remove_global_image_ids_ack.size() * 2)); remote_peer_ack_wait(mock_image_map.get(), remove_global_image_ids_ack, 0, - &peer_ack_ctxs); + false, &peer_remove_ack_ctxs); + remote_peer_ack_wait(mock_image_map.get(), remove_global_image_ids_ack, 0, + true, &peer_ack_ctxs); // trigger duplicate "remove" notification mock_image_map->update_images("uuid1", {}, std::move(remove_global_image_ids_dup)); @@ -759,8 +777,9 @@ TEST_F(TestMockImageMap, RemoveRemoteAndLocalImage) { // remove remote images -- this should be a no-op from policy pov // except the listener notification + std::map peer_ack_remove_ctxs; listener_remove_images(mock_listener, "uuid1", remote_remove_global_image_ids, - &peer_ack_ctxs); + &peer_ack_remove_ctxs); mock_image_map->update_images("uuid1", {}, std::move(remote_remove_global_image_ids)); ASSERT_TRUE(wait_for_listener_notify(remote_remove_global_image_ids_ack.size())); @@ -776,7 +795,9 @@ TEST_F(TestMockImageMap, RemoveRemoteAndLocalImage) { ASSERT_TRUE(wait_for_listener_notify(local_remove_global_image_ids_ack.size())); remote_peer_ack_wait(mock_image_map.get(), local_remove_global_image_ids_ack, - 0, &peer_ack_ctxs); + 0, false, &peer_ack_remove_ctxs); + remote_peer_ack_wait(mock_image_map.get(), local_remove_global_image_ids_ack, + 0, true, &peer_ack_ctxs); wait_for_scheduled_task(); ASSERT_EQ(0, when_shut_down(mock_image_map.get())); @@ -823,12 +844,13 @@ TEST_F(TestMockImageMap, AddInstance) { // remote peer ACKs image acquire request remote_peer_ack_nowait(mock_image_map.get(), global_image_ids_ack, 0, &peer_ack_ctxs); + wait_for_scheduled_task(); std::set shuffled_global_image_ids; // RELEASE+UPDATE_MAPPING+ACQUIRE expect_add_event(mock_threads); - expect_listener_images_unmapped(mock_listener, &shuffled_global_image_ids, + expect_listener_images_unmapped(mock_listener, 3, &shuffled_global_image_ids, &peer_ack_ctxs); mock_image_map->update_instances_added({"9876"}); @@ -892,12 +914,13 @@ TEST_F(TestMockImageMap, RemoveInstance) { // remote peer ACKs image acquire request -- completing action remote_peer_ack_nowait(mock_image_map.get(), global_image_ids_ack, 0, &peer_ack_ctxs); + wait_for_scheduled_task(); std::set shuffled_global_image_ids; // RELEASE+UPDATE_MAPPING+ACQUIRE expect_add_event(mock_threads); - expect_listener_images_unmapped(mock_listener, &shuffled_global_image_ids, + expect_listener_images_unmapped(mock_listener, 3, &shuffled_global_image_ids, &peer_ack_ctxs); mock_image_map->update_instances_added({"9876"}); @@ -914,12 +937,13 @@ TEST_F(TestMockImageMap, RemoveInstance) { // completion shuffle action for now (re)mapped images remote_peer_ack_nowait(mock_image_map.get(), shuffled_global_image_ids, 0, &peer_ack_ctxs); + wait_for_scheduled_task(); shuffled_global_image_ids.clear(); // remove added instance expect_add_event(mock_threads); - expect_listener_images_unmapped(mock_listener, &shuffled_global_image_ids, + expect_listener_images_unmapped(mock_listener, 2, &shuffled_global_image_ids, &peer_ack_ctxs); mock_image_map->update_instances_removed({"9876"}); @@ -949,10 +973,31 @@ TEST_F(TestMockImageMap, AddInstancePingPongImageTest) { InSequence seq; + std::set global_image_ids{ + "global id 1", "global id 2", "global id 3", "global id 4", "global id 5", + "global id 6", "global id 7", "global id 8", "global id 9", "global id 10", + "global id 11", "global id 12", "global id 13", "global id 14" + }; + + auto local_instance_id = stringify(m_local_io_ctx.get_instance_id()); + std::map image_mapping; + for (auto& global_image_id : global_image_ids) { + image_mapping[global_image_id] = {local_instance_id, {}, {}}; + } + + // ACQUIRE MockLoadRequest mock_load_request; - expect_load_request(mock_load_request, 0); + EXPECT_CALL(mock_load_request, send()).WillOnce( + Invoke([&mock_load_request, &image_mapping]() { + *mock_load_request.image_map = image_mapping; + mock_load_request.on_finish->complete(0); + })); + expect_add_event(mock_threads); MockListener mock_listener(this); + std::map peer_ack_ctxs; + listener_acquire_images(mock_listener, global_image_ids, + &peer_ack_ctxs); std::unique_ptr mock_image_map{ MockImageMap::create(m_local_io_ctx, &mock_threads, mock_listener)}; @@ -961,19 +1006,19 @@ TEST_F(TestMockImageMap, AddInstancePingPongImageTest) { mock_image_map->init(&cond); ASSERT_EQ(0, cond.wait()); - std::set global_image_ids{ - "global id 1", "global id 2", "global id 3", "global id 4", "global id 5", - "global id 6", "global id 7", "global id 8", "global id 9", "global id 10", - "global id 11", "global id 12", "global id 13", "global id 14" - }; std::set global_image_ids_ack(global_image_ids); - // UPDATE_MAPPING+ACQUIRE + // remote peer ACKs image acquire request -- completing action + ASSERT_TRUE(wait_for_listener_notify(global_image_ids_ack.size())); + remote_peer_ack_nowait(mock_image_map.get(), global_image_ids_ack, 0, + &peer_ack_ctxs); + wait_for_scheduled_task(); + + // RELEASE+UPDATE_MAPPING+ACQUIRE expect_add_event(mock_threads); MockUpdateRequest mock_update_request; expect_update_request(mock_update_request, 0); expect_add_event(mock_threads); - std::map peer_ack_ctxs; listener_acquire_images(mock_listener, global_image_ids, &peer_ack_ctxs); @@ -986,12 +1031,13 @@ TEST_F(TestMockImageMap, AddInstancePingPongImageTest) { // remote peer ACKs image acquire request -- completing action remote_peer_ack_nowait(mock_image_map.get(), global_image_ids_ack, 0, &peer_ack_ctxs); + wait_for_scheduled_task(); std::set shuffled_global_image_ids; // RELEASE+UPDATE_MAPPING+ACQUIRE expect_add_event(mock_threads); - expect_listener_images_unmapped(mock_listener, &shuffled_global_image_ids, + expect_listener_images_unmapped(mock_listener, 7, &shuffled_global_image_ids, &peer_ack_ctxs); mock_image_map->update_instances_added({"9876"}); @@ -1008,13 +1054,14 @@ TEST_F(TestMockImageMap, AddInstancePingPongImageTest) { // completion shuffle action for now (re)mapped images remote_peer_ack_nowait(mock_image_map.get(), shuffled_global_image_ids, 0, &peer_ack_ctxs); + wait_for_scheduled_task(); std::set migrated_global_image_ids(shuffled_global_image_ids); shuffled_global_image_ids.clear(); // RELEASE+UPDATE_MAPPING+ACQUIRE expect_add_event(mock_threads); - expect_listener_images_unmapped(mock_listener, &shuffled_global_image_ids, + expect_listener_images_unmapped(mock_listener, 3, &shuffled_global_image_ids, &peer_ack_ctxs); // add another instance @@ -1089,12 +1136,13 @@ TEST_F(TestMockImageMap, RemoveInstanceWithRemoveImage) { remote_peer_ack_nowait(mock_image_map.get(), global_image_ids_ack, 0, &peer_ack_ctxs); + wait_for_scheduled_task(); std::set shuffled_global_image_ids; // RELEASE+UPDATE_MAPPING+ACQUIRE expect_add_event(mock_threads); - expect_listener_images_unmapped(mock_listener, &shuffled_global_image_ids, + expect_listener_images_unmapped(mock_listener, 2, &shuffled_global_image_ids, &peer_ack_ctxs); mock_image_map->update_instances_added({"9876"}); @@ -1111,15 +1159,20 @@ TEST_F(TestMockImageMap, RemoveInstanceWithRemoveImage) { // completion shuffle action for now (re)mapped images remote_peer_ack_nowait(mock_image_map.get(), shuffled_global_image_ids, 0, &peer_ack_ctxs); + wait_for_scheduled_task(); std::set shuffled_global_image_ids_ack(shuffled_global_image_ids); // RELEASE + + std::map peer_ack_remove_ctxs; listener_remove_images(mock_listener, "uuid1", shuffled_global_image_ids, - &peer_ack_ctxs); + &peer_ack_remove_ctxs); expect_add_event(mock_threads); listener_release_images(mock_listener, shuffled_global_image_ids, &peer_ack_ctxs); + expect_add_event(mock_threads); + expect_update_request(mock_update_request, 0); mock_image_map->update_images("uuid1", {}, std::move(shuffled_global_image_ids)); ASSERT_TRUE(wait_for_listener_notify(shuffled_global_image_ids_ack.size() * 2)); @@ -1127,6 +1180,8 @@ TEST_F(TestMockImageMap, RemoveInstanceWithRemoveImage) { // instance failed -- update policy for instance removal mock_image_map->update_instances_removed({"9876"}); + remote_peer_ack_nowait(mock_image_map.get(), shuffled_global_image_ids, + -ENOENT, &peer_ack_remove_ctxs); remote_peer_ack_nowait(mock_image_map.get(), shuffled_global_image_ids, -EBLACKLISTED, &peer_ack_ctxs); @@ -1175,12 +1230,13 @@ TEST_F(TestMockImageMap, AddErrorAndRemoveImage) { // remote peer ACKs image acquire request remote_peer_ack_nowait(mock_image_map.get(), global_image_ids_ack, 0, &peer_ack_ctxs); + wait_for_scheduled_task(); std::set shuffled_global_image_ids; // RELEASE+UPDATE_MAPPING+ACQUIRE expect_add_event(mock_threads); - expect_listener_images_unmapped(mock_listener, &shuffled_global_image_ids, + expect_listener_images_unmapped(mock_listener, 2, &shuffled_global_image_ids, &peer_ack_ctxs); mock_image_map->update_instances_added({"9876"}); @@ -1193,26 +1249,40 @@ TEST_F(TestMockImageMap, AddErrorAndRemoveImage) { &peer_ack_ctxs); remote_peer_ack_listener_wait(mock_image_map.get(), shuffled_global_image_ids, 0, &peer_ack_ctxs); + wait_for_scheduled_task(); mock_image_map->update_instances_removed({"9876"}); - // instance blacklisted -- ACQUIRE request fails + expect_add_event(mock_threads); + std::map release_peer_ack_ctxs; + expect_listener_images_unmapped(mock_listener, 2, &shuffled_global_image_ids, + &release_peer_ack_ctxs); + + std::map remap_peer_ack_ctxs; update_map_and_acquire(mock_threads, mock_update_request, mock_listener, shuffled_global_image_ids, 0, - &peer_ack_ctxs); + &remap_peer_ack_ctxs); + + // instance blacklisted -- ACQUIRE and RELEASE request fails + remote_peer_ack_nowait(mock_image_map.get(), shuffled_global_image_ids, + -EBLACKLISTED, &peer_ack_ctxs); + + ASSERT_TRUE(wait_for_listener_notify(shuffled_global_image_ids.size())); remote_peer_ack_listener_wait(mock_image_map.get(), shuffled_global_image_ids, - -EBLACKLISTED, &peer_ack_ctxs); + -ENOENT, &release_peer_ack_ctxs); + wait_for_scheduled_task(); // new peer acks acquire request remote_peer_ack_nowait(mock_image_map.get(), shuffled_global_image_ids, 0, - &peer_ack_ctxs); + &remap_peer_ack_ctxs); wait_for_scheduled_task(); std::set shuffled_global_image_ids_ack(shuffled_global_image_ids); // remove image + std::map peer_ack_remove_ctxs; listener_remove_images(mock_listener, "uuid1", shuffled_global_image_ids, - &peer_ack_ctxs); + &peer_ack_remove_ctxs); expect_add_event(mock_threads); listener_release_images(mock_listener, shuffled_global_image_ids, &peer_ack_ctxs); @@ -1222,7 +1292,9 @@ TEST_F(TestMockImageMap, AddErrorAndRemoveImage) { ASSERT_TRUE(wait_for_listener_notify(shuffled_global_image_ids_ack.size() * 2)); remote_peer_ack_wait(mock_image_map.get(), shuffled_global_image_ids_ack, 0, - &peer_ack_ctxs); + false, &peer_ack_remove_ctxs); + remote_peer_ack_wait(mock_image_map.get(), shuffled_global_image_ids_ack, 0, + true, &peer_ack_ctxs); wait_for_scheduled_task(); ASSERT_EQ(0, when_shut_down(mock_image_map.get())); @@ -1282,10 +1354,12 @@ TEST_F(TestMockImageMap, MirrorUUIDUpdated) { remote_peer_ack_nowait(mock_image_map.get(), initial_remote_global_image_ids_ack, 0, &peer_ack_ctxs); + wait_for_scheduled_task(); // RELEASE+REMOVE_MAPPING + std::map peer_remove_ack_ctxs; listener_remove_images(mock_listener, "uuid1", remote_removed_global_image_ids, - &peer_ack_ctxs); + &peer_remove_ack_ctxs); expect_add_event(mock_threads); listener_release_images(mock_listener, remote_removed_global_image_ids, &peer_ack_ctxs); @@ -1296,7 +1370,10 @@ TEST_F(TestMockImageMap, MirrorUUIDUpdated) { remote_peer_ack_wait(mock_image_map.get(), remote_removed_global_image_ids_ack, 0, - &peer_ack_ctxs); + false, &peer_remove_ack_ctxs); + remote_peer_ack_wait(mock_image_map.get(), + remote_removed_global_image_ids_ack, 0, + true, &peer_ack_ctxs); // UPDATE_MAPPING+ACQUIRE expect_add_event(mock_threads); diff --git a/src/tools/rbd_mirror/CMakeLists.txt b/src/tools/rbd_mirror/CMakeLists.txt index 9a3b8e18bcc16..b9ecdcfbbb0c6 100644 --- a/src/tools/rbd_mirror/CMakeLists.txt +++ b/src/tools/rbd_mirror/CMakeLists.txt @@ -25,7 +25,6 @@ set(rbd_mirror_internal image_deleter/SnapshotPurgeRequest.cc image_deleter/TrashMoveRequest.cc image_deleter/TrashWatcher.cc - image_map/Action.cc image_map/LoadRequest.cc image_map/Policy.cc image_map/SimplePolicy.cc diff --git a/src/tools/rbd_mirror/ImageMap.cc b/src/tools/rbd_mirror/ImageMap.cc index b945e2c9ac876..bc226c1c5ac43 100644 --- a/src/tools/rbd_mirror/ImageMap.cc +++ b/src/tools/rbd_mirror/ImageMap.cc @@ -17,11 +17,13 @@ #define dout_context g_ceph_context #define dout_subsys ceph_subsys_rbd_mirror #undef dout_prefix -#define dout_prefix *_dout << "rbd::mirror::ImageMap: " << this << " " << __func__ +#define dout_prefix *_dout << "rbd::mirror::ImageMap: " << this << " " \ + << __func__ << ": " namespace rbd { namespace mirror { +using ::operator<<; using image_map::Policy; using librbd::util::unique_lock_name; @@ -31,14 +33,21 @@ template struct ImageMap::C_NotifyInstance : public Context { ImageMap* image_map; std::string global_image_id; + bool acquire_release; - C_NotifyInstance(ImageMap* image_map, const std::string& global_image_id) - : image_map(image_map), global_image_id(global_image_id) { + C_NotifyInstance(ImageMap* image_map, const std::string& global_image_id, + bool acquire_release) + : image_map(image_map), global_image_id(global_image_id), + acquire_release(acquire_release) { image_map->start_async_op(); } void finish(int r) override { - image_map->handle_peer_ack(global_image_id, r); + if (acquire_release) { + image_map->handle_peer_ack(global_image_id, r); + } else { + image_map->handle_peer_ack_remove(global_image_id, r); + } image_map->finish_async_op(); } }; @@ -57,74 +66,9 @@ ImageMap::~ImageMap() { assert(m_timer_task == nullptr); } -template -bool ImageMap::add_peer(const std::string &global_image_id, const std::string &peer_uuid) { - assert(m_lock.is_locked()); - - dout(20) << ": global_image_id=" << global_image_id << ", peer_uuid=" - << peer_uuid << dendl; - - auto ins = m_peer_map[global_image_id].insert(peer_uuid); - return ins.second && m_peer_map[global_image_id].size() == 1; -} - -template -bool ImageMap::remove_peer(const std::string &global_image_id, const std::string &peer_uuid) { - assert(m_lock.is_locked()); - - dout(20) << ": global_image_id=" << global_image_id << ", peer_uuid=" - << peer_uuid << dendl; - - auto rm = m_peer_map[global_image_id].erase(peer_uuid); - return rm && m_peer_map[global_image_id].empty(); -} - -template -void ImageMap::queue_update_map(const std::string &global_image_id) { - assert(m_lock.is_locked()); - - dout(20) << ": global_image_id=" << global_image_id << dendl; - - image_map::LookupInfo info = m_policy->lookup(global_image_id); - assert(info.instance_id != image_map::UNMAPPED_INSTANCE_ID); - - m_updates.emplace_back(global_image_id, info.instance_id, info.mapped_time); -} - -template -void ImageMap::queue_remove_map(const std::string &global_image_id) { - assert(m_lock.is_locked()); - - dout(20) << ": global_image_id=" << global_image_id << dendl; - m_remove_global_image_ids.emplace(global_image_id); -} - template -void ImageMap::queue_acquire_image(const std::string &global_image_id) { - assert(m_lock.is_locked()); - - dout(20) << ": global_image_id=" << global_image_id << dendl; - - image_map::LookupInfo info = m_policy->lookup(global_image_id); - assert(info.instance_id != image_map::UNMAPPED_INSTANCE_ID); - - m_acquire_updates.emplace_back(global_image_id, info.instance_id); -} - -template -void ImageMap::queue_release_image(const std::string &global_image_id) { - assert(m_lock.is_locked()); - - dout(20) << ": global_image_id=" << global_image_id << dendl; - - image_map::LookupInfo info = m_policy->lookup(global_image_id); - assert(info.instance_id != image_map::UNMAPPED_INSTANCE_ID); - - m_release_updates.emplace_back(global_image_id, info.instance_id); -} - -template -void ImageMap::continue_action(const std::set &global_image_ids, int r) { +void ImageMap::continue_action(const std::set &global_image_ids, + int r) { dout(20) << dendl; { @@ -145,13 +89,15 @@ void ImageMap::continue_action(const std::set &global_image_ids, } template -void ImageMap::handle_update_request(const Updates &updates, - const std::set &remove_global_image_ids, int r) { - dout(20) << ": r=" << r << dendl; +void ImageMap::handle_update_request( + const Updates &updates, + const std::set &remove_global_image_ids, int r) { + dout(20) << "r=" << r << dendl; std::set global_image_ids; - global_image_ids.insert(remove_global_image_ids.begin(), remove_global_image_ids.end()); + global_image_ids.insert(remove_global_image_ids.begin(), + remove_global_image_ids.end()); for (auto const &update : updates) { global_image_ids.insert(update.global_image_id); } @@ -160,20 +106,18 @@ void ImageMap::handle_update_request(const Updates &updates, } template -void ImageMap::update_image_mapping() { - dout(20) << ": update_count=" << m_updates.size() << ", remove_count=" - << m_remove_global_image_ids.size() << dendl; - - if (m_updates.empty() && m_remove_global_image_ids.empty()) { +void ImageMap::update_image_mapping(Updates&& map_updates, + std::set&& map_removals) { + if (map_updates.empty() && map_removals.empty()) { return; } - Updates updates(m_updates); - std::set remove_global_image_ids(m_remove_global_image_ids); + dout(5) << "updates=[" << map_updates << "], " + << "removes=[" << map_removals << "]" << dendl; Context *on_finish = new FunctionContext( - [this, updates, remove_global_image_ids](int r) { - handle_update_request(updates, remove_global_image_ids, r); + [this, map_updates, map_removals](int r) { + handle_update_request(map_updates, map_removals, r); finish_async_op(); }); on_finish = create_async_context_callback(m_threads->work_queue, on_finish); @@ -186,14 +130,15 @@ void ImageMap::update_image_mapping() { // prepare update map std::map update_mapping; - for (auto const &update : updates) { + for (auto const &update : map_updates) { update_mapping.emplace( - update.global_image_id, cls::rbd::MirrorImageMap(update.instance_id, update.mapped_time, bl)); + update.global_image_id, cls::rbd::MirrorImageMap(update.instance_id, + update.mapped_time, bl)); } start_async_op(); image_map::UpdateRequest *req = image_map::UpdateRequest::create( - m_ioctx, std::move(update_mapping), std::move(remove_global_image_ids), on_finish); + m_ioctx, std::move(update_mapping), std::move(map_removals), on_finish); req->send(); } @@ -204,33 +149,49 @@ void ImageMap::process_updates() { assert(m_threads->timer_lock.is_locked()); assert(m_timer_task == nullptr); - { - Mutex::Locker locker(m_lock); + Updates map_updates; + std::set map_removals; + Updates acquire_updates; + Updates release_updates; - // gather updates by advancing the state machine - for (auto const &global_image_id : m_global_image_ids) { - m_policy->start_next_action(global_image_id); + // gather updates by advancing the state machine + m_lock.Lock(); + for (auto const &global_image_id : m_global_image_ids) { + image_map::ActionType action_type = + m_policy->start_action(global_image_id); + image_map::LookupInfo info = m_policy->lookup(global_image_id); + assert(info.instance_id != image_map::UNMAPPED_INSTANCE_ID); + + switch (action_type) { + case image_map::ACTION_TYPE_NONE: + continue; + case image_map::ACTION_TYPE_MAP_UPDATE: + map_updates.emplace_back(global_image_id, info.instance_id, + info.mapped_time); + break; + case image_map::ACTION_TYPE_MAP_REMOVE: + map_removals.emplace(global_image_id); + break; + case image_map::ACTION_TYPE_ACQUIRE: + acquire_updates.emplace_back(global_image_id, info.instance_id); + break; + case image_map::ACTION_TYPE_RELEASE: + release_updates.emplace_back(global_image_id, info.instance_id); + break; } - - m_global_image_ids.clear(); } + m_global_image_ids.clear(); + m_lock.Unlock(); // notify listener (acquire, release) and update on-disk map. note // that its safe to process this outside m_lock as we still hold // timer lock. - notify_listener_acquire_release_images(m_acquire_updates, m_release_updates); - update_image_mapping(); - - m_updates.clear(); - m_remove_global_image_ids.clear(); - m_acquire_updates.clear(); - m_release_updates.clear(); + notify_listener_acquire_release_images(acquire_updates, release_updates); + update_image_mapping(std::move(map_updates), std::move(map_removals)); } template void ImageMap::schedule_update_task() { - dout(20) << dendl; - Mutex::Locker timer_lock(m_threads->timer_lock); if (m_timer_task != nullptr) { return; @@ -253,14 +214,14 @@ void ImageMap::schedule_update_task() { CephContext *cct = reinterpret_cast(m_ioctx.cct()); double after = cct->_conf->get_val("rbd_mirror_image_policy_update_throttle_interval"); - dout(20) << ": scheduling image check update (" << m_timer_task << ")" + dout(20) << "scheduling image check update (" << m_timer_task << ")" << " after " << after << " second(s)" << dendl; m_threads->timer->add_event_after(after, m_timer_task); } template void ImageMap::schedule_action(const std::string &global_image_id) { - dout(20) << ": global_image_id=" << global_image_id << dendl; + dout(20) << "global_image_id=" << global_image_id << dendl; assert(m_lock.is_locked()); m_global_image_ids.emplace(global_image_id); @@ -269,15 +230,19 @@ void ImageMap::schedule_action(const std::string &global_image_id) { template void ImageMap::notify_listener_acquire_release_images( const Updates &acquire, const Updates &release) { - dout(20) << ": acquire_count: " << acquire.size() << ", release_count=" - << release.size() << dendl; + if (acquire.empty() && release.empty()) { + return; + } + + dout(5) << "acquire=[" << acquire << "], " + << "release=[" << release << "]" << dendl; for (auto const &update : acquire) { m_listener.acquire_image( update.global_image_id, update.instance_id, create_async_context_callback( m_threads->work_queue, - new C_NotifyInstance(this, update.global_image_id))); + new C_NotifyInstance(this, update.global_image_id, true))); } for (auto const &update : release) { @@ -285,164 +250,122 @@ void ImageMap::notify_listener_acquire_release_images( update.global_image_id, update.instance_id, create_async_context_callback( m_threads->work_queue, - new C_NotifyInstance(this, update.global_image_id))); + new C_NotifyInstance(this, update.global_image_id, true))); } } template void ImageMap::notify_listener_remove_images(const std::string &peer_uuid, const Updates &remove) { - dout(20) << ": peer_uuid=" << peer_uuid << ", remove_count=" << remove.size() - << dendl; + dout(5) << "peer_uuid=" << peer_uuid << ", " + << "remove=[" << remove << "]" << dendl; for (auto const &update : remove) { m_listener.remove_image( peer_uuid, update.global_image_id, update.instance_id, create_async_context_callback( m_threads->work_queue, - new C_NotifyInstance(this, update.global_image_id))); + new C_NotifyInstance(this, update.global_image_id, false))); } } template -void ImageMap::handle_load(const std::map &image_mapping) { +void ImageMap::handle_load(const std::map &image_mapping) { dout(20) << dendl; - Mutex::Locker locker(m_lock); - m_policy->init(image_mapping); -} - -template -void ImageMap::handle_add_action(const std::string &global_image_id, int r) { - assert(m_lock.is_locked()); - dout(20) << ": global_image_id=" << global_image_id << dendl; - - if (r < 0) { - derr << ": failed to add global_image_id=" << global_image_id << dendl; - } -} - -template -void ImageMap::handle_remove_action(const std::string &global_image_id, int r) { - assert(m_lock.is_locked()); - dout(20) << ": global_image_id=" << global_image_id << dendl; - - if (r < 0) { - derr << ": failed to remove global_image_id=" << global_image_id << dendl; - } + { + Mutex::Locker locker(m_lock); + m_policy->init(image_mapping); - if (m_peer_map[global_image_id].empty()) { - m_peer_map.erase(global_image_id); + for (auto& pair : image_mapping) { + schedule_action(pair.first); + } } + schedule_update_task(); } template -void ImageMap::handle_shuffle_action(const std::string &global_image_id, int r) { - assert(m_lock.is_locked()); - dout(20) << ": global_image_id=" << global_image_id << dendl; +void ImageMap::handle_peer_ack_remove(const std::string &global_image_id, + int r) { + Mutex::Locker locker(m_lock); + dout(5) << "global_image_id=" << global_image_id << dendl; if (r < 0) { - derr << ": failed to shuffle global_image_id=" << global_image_id << dendl; - } -} - -template -void ImageMap::schedule_add_action(const std::string &global_image_id) { - dout(20) << ": global_image_id=" << global_image_id << dendl; - - // in order of state-machine execution, so its easier to follow - Context *on_update = new C_UpdateMap(this, global_image_id); - Context *on_acquire = new C_AcquireImage(this, global_image_id); - Context *on_finish = new FunctionContext([this, global_image_id](int r) { - handle_add_action(global_image_id, r); - }); - - if (m_policy->add_image(global_image_id, on_update, on_acquire, on_finish)) { - schedule_action(global_image_id); + derr << "failed to remove global_image_id=" << global_image_id << dendl; } -} - -template -void ImageMap::schedule_remove_action(const std::string &global_image_id) { - dout(20) << ": global_image_id=" << global_image_id << dendl; - // in order of state-machine execution, so its easier to follow - Context *on_release = new FunctionContext([this, global_image_id](int r) { - queue_release_image(global_image_id); - }); - Context *on_remove = new C_RemoveMap(this, global_image_id); - Context *on_finish = new FunctionContext([this, global_image_id](int r) { - handle_remove_action(global_image_id, r); - }); - - if (m_policy->remove_image(global_image_id, on_release, on_remove, on_finish)) { - schedule_action(global_image_id); + auto peer_it = m_peer_map.find(global_image_id); + if (peer_it == m_peer_map.end()) { + return; } -} -template -void ImageMap::schedule_shuffle_action(const std::string &global_image_id) { - assert(m_lock.is_locked()); - - dout(20) << ": global_image_id=" << global_image_id << dendl; - - // in order of state-machine execution, so its easier to follow - Context *on_release = new FunctionContext([this, global_image_id](int r) { - queue_release_image(global_image_id); - }); - Context *on_update = new C_UpdateMap(this, global_image_id); - Context *on_acquire = new C_AcquireImage(this, global_image_id); - Context *on_finish = new FunctionContext([this, global_image_id](int r) { - handle_shuffle_action(global_image_id, r); - }); - - if (m_policy->shuffle_image(global_image_id, on_release, on_update, on_acquire, on_finish)) { - schedule_action(global_image_id); - } + m_peer_map.erase(peer_it); } template -void ImageMap::update_images_added(const std::string &peer_uuid, - const std::set &global_image_ids) { - dout(20) << dendl; +void ImageMap::update_images_added( + const std::string &peer_uuid, + const std::set &global_image_ids) { + dout(5) << "peer_uuid=" << peer_uuid << ", " + << "global_image_ids=[" << global_image_ids << "]" << dendl; assert(m_lock.is_locked()); for (auto const &global_image_id : global_image_ids) { - bool schedule_update = add_peer(global_image_id, peer_uuid); - if (schedule_update) { - schedule_add_action(global_image_id); + auto result = m_peer_map[global_image_id].insert(peer_uuid); + if (result.second && m_peer_map[global_image_id].size() == 1) { + if (m_policy->add_image(global_image_id)) { + schedule_action(global_image_id); + } } } } template -void ImageMap::update_images_removed(const std::string &peer_uuid, - const std::set &global_image_ids) { - dout(20) << dendl; +void ImageMap::update_images_removed( + const std::string &peer_uuid, + const std::set &global_image_ids) { + dout(5) << "peer_uuid=" << peer_uuid << ", " + << "global_image_ids=[" << global_image_ids << "]" << dendl; assert(m_lock.is_locked()); Updates to_remove; for (auto const &global_image_id : global_image_ids) { - bool schedule_update = remove_peer(global_image_id, peer_uuid); - if (schedule_update) { - schedule_remove_action(global_image_id); + image_map::LookupInfo info = m_policy->lookup(global_image_id); + bool image_mapped = (info.instance_id != image_map::UNMAPPED_INSTANCE_ID); + + bool image_removed = image_mapped; + bool peer_removed = false; + auto peer_it = m_peer_map.find(global_image_id); + if (peer_it != m_peer_map.end()) { + auto& peer_set = peer_it->second; + peer_removed = peer_set.erase(peer_uuid); + image_removed = peer_removed && peer_set.empty(); } - image_map::LookupInfo info = m_policy->lookup(global_image_id); - if (info.instance_id != image_map::UNMAPPED_INSTANCE_ID) { + if (image_mapped && peer_removed && !peer_uuid.empty()) { + // peer image has been deleted to_remove.emplace_back(global_image_id, info.instance_id); } + + if (image_mapped && image_removed) { + // local and peer images have been deleted + if (m_policy->remove_image(global_image_id)) { + schedule_action(global_image_id); + } + } } - // removal notification will be notified instantly. this is safe - // even after scheduling action for images as we still hold m_lock - if (!peer_uuid.empty()) { + if (!to_remove.empty()) { + // removal notification will be notified instantly. this is safe + // even after scheduling action for images as we still hold m_lock notify_listener_remove_images(peer_uuid, to_remove); } } template -void ImageMap::update_instances_added(const std::vector &instance_ids) { +void ImageMap::update_instances_added( + const std::vector &instance_ids) { dout(20) << dendl; { @@ -455,7 +378,7 @@ void ImageMap::update_instances_added(const std::vector &instanc m_policy->add_instances(instance_ids, &remap_global_image_ids); for (auto const &global_image_id : remap_global_image_ids) { - schedule_shuffle_action(global_image_id); + schedule_action(global_image_id); } } @@ -463,7 +386,8 @@ void ImageMap::update_instances_added(const std::vector &instanc } template -void ImageMap::update_instances_removed(const std::vector &instance_ids) { +void ImageMap::update_instances_removed( + const std::vector &instance_ids) { dout(20) << dendl; { @@ -476,7 +400,7 @@ void ImageMap::update_instances_removed(const std::vector &insta m_policy->remove_instances(instance_ids, &remap_global_image_ids); for (auto const &global_image_id : remap_global_image_ids) { - schedule_shuffle_action(global_image_id); + schedule_action(global_image_id); } } @@ -487,9 +411,9 @@ template void ImageMap::update_images(const std::string &peer_uuid, std::set &&added_global_image_ids, std::set &&removed_global_image_ids) { - dout(20) << ": peer_uuid=" << peer_uuid << ", " << "added_count=" - << added_global_image_ids.size() << ", " << "removed_count=" - << removed_global_image_ids.size() << dendl; + dout(5) << "peer_uuid=" << peer_uuid << ", " << "added_count=" + << added_global_image_ids.size() << ", " << "removed_count=" + << removed_global_image_ids.size() << dendl; { Mutex::Locker locker(m_lock); @@ -497,8 +421,12 @@ void ImageMap::update_images(const std::string &peer_uuid, return; } - update_images_removed(peer_uuid, removed_global_image_ids); - update_images_added(peer_uuid, added_global_image_ids); + if (!removed_global_image_ids.empty()) { + update_images_removed(peer_uuid, removed_global_image_ids); + } + if (!added_global_image_ids.empty()) { + update_images_added(peer_uuid, added_global_image_ids); + } } schedule_update_task(); @@ -506,7 +434,7 @@ void ImageMap::update_images(const std::string &peer_uuid, template void ImageMap::handle_peer_ack(const std::string &global_image_id, int r) { - dout (20) << ": global_image_id=" << global_image_id << ", r=" << r + dout (20) << "global_image_id=" << global_image_id << ", r=" << r << dendl; continue_action({global_image_id}, r); @@ -525,7 +453,7 @@ void ImageMap::init(Context *on_finish) { assert(false); // not really needed as such, but catch it. } - dout(20) << ": mapping policy=" << policy_type << dendl; + dout(20) << "mapping policy=" << policy_type << dendl; start_async_op(); C_LoadMap *ctx = new C_LoadMap(this, on_finish); diff --git a/src/tools/rbd_mirror/ImageMap.h b/src/tools/rbd_mirror/ImageMap.h index ffe91742bb31a..aa86fcbbfa275 100644 --- a/src/tools/rbd_mirror/ImageMap.h +++ b/src/tools/rbd_mirror/ImageMap.h @@ -67,6 +67,14 @@ private: Update(const std::string &global_image_id, const std::string &instance_id) : Update(global_image_id, instance_id, ceph_clock_now()) { } + + friend std::ostream& operator<<(std::ostream& os, + const Update& update) { + os << "{global_image_id=" << update.global_image_id << ", " + << "instance_id=" << update.instance_id << "}"; + return os; + } + }; typedef std::list Updates; @@ -86,11 +94,6 @@ private: // global_image_id -> registered peers ("" == local, remote otherwise) std::map > m_peer_map; - Updates m_updates; - std::set m_remove_global_image_ids; - Updates m_acquire_updates; - Updates m_release_updates; - std::set m_global_image_ids; struct C_LoadMap : Context { @@ -114,65 +117,6 @@ private: } }; - // context callbacks which are retry-able get deleted after - // transiting to the next state. - struct C_UpdateMap : Context { - ImageMap *image_map; - std::string global_image_id; - - C_UpdateMap(ImageMap *image_map, const std::string &global_image_id) - : image_map(image_map), - global_image_id(global_image_id) { - } - - void finish(int r) override { - image_map->queue_update_map(global_image_id); - } - - // maybe called more than once - void complete(int r) override { - finish(r); - } - }; - - struct C_RemoveMap : Context { - ImageMap *image_map; - std::string global_image_id; - - C_RemoveMap(ImageMap *image_map, const std::string &global_image_id) - : image_map(image_map), - global_image_id(global_image_id) { - } - - void finish(int r) override { - image_map->queue_remove_map(global_image_id); - } - - // maybe called more than once - void complete(int r) override { - finish(r); - } - }; - - struct C_AcquireImage : Context { - ImageMap *image_map; - std::string global_image_id; - - C_AcquireImage(ImageMap *image_map, const std::string &global_image_id) - : image_map(image_map), - global_image_id(global_image_id) { - } - - void finish(int r) override { - image_map->queue_acquire_image(global_image_id); - } - - // maybe called more than once - void complete(int r) override { - finish(r); - } - }; - // async op-tracker helper routines void start_async_op() { m_async_op_tracker.start_op(); @@ -184,23 +128,12 @@ private: m_async_op_tracker.wait_for_ops(on_finish); } - bool add_peer(const std::string &global_image_id, const std::string &peer_uuid); - bool remove_peer(const std::string &global_image_id, const std::string &peer_uuid); - void handle_peer_ack(const std::string &global_image_id, int r); - - // queue on-disk,acquire,remove updates in appropriate list - void queue_update_map(const std::string &global_image_id); - void queue_remove_map(const std::string &global_image_id); - void queue_acquire_image(const std::string &global_image_id); - void queue_release_image(const std::string &global_image_id); + void handle_peer_ack_remove(const std::string &global_image_id, int r); void handle_load(const std::map &image_mapping); void handle_update_request(const Updates &updates, const std::set &remove_global_image_ids, int r); - void handle_add_action(const std::string &global_image_id, int r); - void handle_remove_action(const std::string &global_image_id, int r); - void handle_shuffle_action(const std::string &global_image_id, int r); // continue (retry or resume depending on state machine) processing // current action. @@ -211,15 +144,12 @@ private: void schedule_update_task(); void process_updates(); - void update_image_mapping(); + void update_image_mapping(Updates&& map_updates, + std::set&& map_removals); void notify_listener_acquire_release_images(const Updates &acquire, const Updates &release); void notify_listener_remove_images(const std::string &peer_uuid, const Updates &remove); - void schedule_add_action(const std::string &global_image_id); - void schedule_remove_action(const std::string &global_image_id); - void schedule_shuffle_action(const std::string &global_image_id); - void update_images_added(const std::string &peer_uuid, const std::set &global_image_ids); void update_images_removed(const std::string &peer_uuid, diff --git a/src/tools/rbd_mirror/image_map/Action.cc b/src/tools/rbd_mirror/image_map/Action.cc deleted file mode 100644 index 12055d06198b1..0000000000000 --- a/src/tools/rbd_mirror/image_map/Action.cc +++ /dev/null @@ -1,89 +0,0 @@ -// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- -// vim: ts=8 sw=2 smarttab - -#include -#include "include/Context.h" -#include "Action.h" - -namespace rbd { -namespace mirror { -namespace image_map { - -std::ostream &operator<<(std::ostream &os, const Action &action) { - os << "[action_type=" << action.get_action_type() << "]"; - return os; -} - -Action::Action(StateTransition::ActionType action_type) - : action_type(action_type) { -} - -Action Action::create_add_action(Context *on_update, Context *on_acquire, Context *on_finish) { - Action action(StateTransition::ACTION_TYPE_ADD); - action.context_map.emplace(StateTransition::STATE_UPDATE_MAPPING, on_update); - action.context_map.emplace(StateTransition::STATE_ASSOCIATED, on_acquire); - action.context_map.emplace(StateTransition::STATE_COMPLETE, on_finish); - - return action; -} - -Action Action::create_remove_action(Context *on_release, Context *on_remove, Context *on_finish) { - Action action(StateTransition::ACTION_TYPE_REMOVE); - action.context_map.emplace(StateTransition::STATE_DISASSOCIATED, on_release); - action.context_map.emplace(StateTransition::STATE_REMOVE_MAPPING, on_remove); - action.context_map.emplace(StateTransition::STATE_COMPLETE, on_finish); - - return action; -} - -Action Action::create_shuffle_action(Context *on_release, Context *on_update, Context *on_acquire, - Context *on_finish) { - Action action(StateTransition::ACTION_TYPE_SHUFFLE); - action.context_map.emplace(StateTransition::STATE_DISASSOCIATED, on_release); - action.context_map.emplace(StateTransition::STATE_UPDATE_MAPPING, on_update); - action.context_map.emplace(StateTransition::STATE_ASSOCIATED, on_acquire); - action.context_map.emplace(StateTransition::STATE_COMPLETE, on_finish); - - return action; -} - -StateTransition::ActionType Action::get_action_type() const { - return action_type; -} - -void Action::execute_state_callback(StateTransition::State state) { - auto it = context_map.find(state); - if (it != context_map.end() && it->second != nullptr) { - it->second->complete(0); - } -} - -void Action::state_callback_complete(StateTransition::State state) { - auto it = context_map.find(state); - if (it != context_map.end()) { - it->second = nullptr; - } -} - -void Action::execute_completion_callback(int r) { - Context *on_finish = nullptr; - - for (auto &ctx : context_map) { - Context *on_state = nullptr; - std::swap(ctx.second, on_state); - - if (ctx.first == StateTransition::STATE_COMPLETE) { - on_finish = on_state; - } else if (on_state != nullptr) { - delete on_state; - } - } - - if (on_finish != nullptr) { - on_finish->complete(r); - } -} - -} // namespace image_map -} // namespace mirror -} // namespace rbd diff --git a/src/tools/rbd_mirror/image_map/Action.h b/src/tools/rbd_mirror/image_map/Action.h deleted file mode 100644 index 6414f66517a2b..0000000000000 --- a/src/tools/rbd_mirror/image_map/Action.h +++ /dev/null @@ -1,42 +0,0 @@ -// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- -// vim: ts=8 sw=2 smarttab - -#ifndef CEPH_RBD_MIRROR_IMAGE_MAP_ACTION_H -#define CEPH_RBD_MIRROR_IMAGE_MAP_ACTION_H - -#include -#include "StateTransition.h" - -class Context; - -namespace rbd { -namespace mirror { -namespace image_map { - -struct Action { -public: - static Action create_add_action(Context *on_update, Context *on_acquire, Context *on_finish); - static Action create_remove_action(Context *on_release, Context *on_remove, Context *on_finish); - static Action create_shuffle_action(Context *on_release, Context *on_update, Context *on_acquire, - Context *on_finish); - - void execute_state_callback(StateTransition::State state); - void state_callback_complete(StateTransition::State state); - void execute_completion_callback(int r); - - StateTransition::ActionType get_action_type() const; - -private: - Action(StateTransition::ActionType action_type); - - StateTransition::ActionType action_type; // action type for this action - std::map context_map; // map sub action type to context callback -}; - -std::ostream &operator<<(std::ostream &os, const Action &action); - -} // namespace image_map -} // namespace mirror -} // namespace rbd - -#endif // CEPH_RBD_MIRROR_IMAGE_MAP_ACTION_H diff --git a/src/tools/rbd_mirror/image_map/Policy.cc b/src/tools/rbd_mirror/image_map/Policy.cc index 16580800f6e73..ca7517e896891 100644 --- a/src/tools/rbd_mirror/image_map/Policy.cc +++ b/src/tools/rbd_mirror/image_map/Policy.cc @@ -11,412 +11,351 @@ #define dout_subsys ceph_subsys_rbd_mirror #undef dout_prefix #define dout_prefix *_dout << "rbd::mirror::image_map::Policy: " << this \ - << " " << __func__ + << " " << __func__ << ": " namespace rbd { namespace mirror { namespace image_map { +namespace { + +bool is_instance_action(ActionType action_type) { + switch (action_type) { + case ACTION_TYPE_ACQUIRE: + case ACTION_TYPE_RELEASE: + return true; + case ACTION_TYPE_NONE: + case ACTION_TYPE_MAP_UPDATE: + case ACTION_TYPE_MAP_REMOVE: + break; + } + return false; +} + +} // anonymous namespace + +using ::operator<<; using librbd::util::unique_lock_name; Policy::Policy(librados::IoCtx &ioctx) : m_ioctx(ioctx), - m_map_lock(unique_lock_name("rbd::mirror::image_map::Policy::m_map_lock", this)) { + m_map_lock(unique_lock_name("rbd::mirror::image_map::Policy::m_map_lock", + this)) { // map should at least have once instance std::string instance_id = stringify(ioctx.get_instance_id()); - add_instances({instance_id}, nullptr); + m_map.emplace(instance_id, std::set{}); } -void Policy::init(const std::map &image_mapping) { +void Policy::init( + const std::map &image_mapping) { dout(20) << dendl; RWLock::WLocker map_lock(m_map_lock); - - for (auto const &it : image_mapping) { - map(it.first, it.second.instance_id, it.second.mapped_time, m_map_lock); + for (auto& it : image_mapping) { + auto map_result = m_map[it.second.instance_id].emplace(it.first); + assert(map_result.second); + + auto image_state_result = m_image_states.emplace( + it.first, ImageState{it.second.instance_id, it.second.mapped_time}); + assert(image_state_result.second); + + // ensure we (re)send image acquire actions to the instance + auto& image_state = image_state_result.first->second; + auto start_action = set_state(&image_state, + StateTransition::STATE_INITIALIZING, false); + assert(start_action); } } LookupInfo Policy::lookup(const std::string &global_image_id) { - dout(20) << ": global_image_id=" << global_image_id << dendl; + dout(20) << "global_image_id=" << global_image_id << dendl; RWLock::RLocker map_lock(m_map_lock); - return lookup(global_image_id, m_map_lock); -} - -bool Policy::add_image(const std::string &global_image_id, - Context *on_update, Context *on_acquire, Context *on_finish) { - dout(20) << ": global_image_id=" << global_image_id << dendl; - - RWLock::WLocker map_lock(m_map_lock); + LookupInfo info; - auto it = m_actions.find(global_image_id); - if (it == m_actions.end()) { - m_actions.emplace(global_image_id, ActionState()); + auto it = m_image_states.find(global_image_id); + if (it != m_image_states.end()) { + info.instance_id = it->second.instance_id; + info.mapped_time = it->second.mapped_time; } - - Action action = Action::create_add_action(on_update, on_acquire, on_finish); - return queue_action(global_image_id, action); + return info; } -bool Policy::remove_image(const std::string &global_image_id, - Context *on_release, Context *on_remove, Context *on_finish) { - dout(20) << ": global_image_id=" << global_image_id << dendl; +bool Policy::add_image(const std::string &global_image_id) { + dout(5) << "global_image_id=" << global_image_id << dendl; RWLock::WLocker map_lock(m_map_lock); - - on_finish = new FunctionContext([this, global_image_id, on_finish](int r) { - { - RWLock::WLocker map_lock(m_map_lock); - if (!actions_pending(global_image_id, m_map_lock)) { - m_actions.erase(global_image_id); - } - } - - if (on_finish != nullptr) { - on_finish->complete(r); - } - }); - - Action action = Action::create_remove_action(on_release, on_remove, on_finish); - return queue_action(global_image_id, action); + auto image_state_result = m_image_states.emplace(global_image_id, + ImageState{}); + auto& image_state = image_state_result.first->second; + return set_state(&image_state, StateTransition::STATE_ASSOCIATING, false); } -bool Policy::shuffle_image(const std::string &global_image_id, - Context *on_release, Context *on_update, - Context *on_acquire, Context *on_finish) { - dout(20) << ": global_image_id=" << global_image_id << dendl; +bool Policy::remove_image(const std::string &global_image_id) { + dout(5) << "global_image_id=" << global_image_id << dendl; RWLock::WLocker map_lock(m_map_lock); + auto it = m_image_states.find(global_image_id); + if (it == m_image_states.end()) { + return false; + } - Action action = Action::create_shuffle_action(on_release, on_update, on_acquire, on_finish); - return queue_action(global_image_id, action); + auto& image_state = it->second; + return set_state(&image_state, StateTransition::STATE_DISSOCIATING, false); } -void Policy::add_instances(const std::vector &instance_ids, - std::set *remap_global_image_ids) { - dout(20) << ": adding " << instance_ids.size() << " instance(s)" << dendl; +void Policy::add_instances(const InstanceIds &instance_ids, + GlobalImageIds* global_image_ids) { + dout(5) << "instance_ids=" << instance_ids << dendl; RWLock::WLocker map_lock(m_map_lock); - - for (auto const &instance : instance_ids) { - dout(10) << ": adding instance_id=" << instance << dendl; + for (auto& instance : instance_ids) { m_map.emplace(instance, std::set{}); } - if (remap_global_image_ids != nullptr) { - do_shuffle_add_instances(instance_ids, remap_global_image_ids); + GlobalImageIds shuffle_global_image_ids; + do_shuffle_add_instances(m_map, m_image_states.size(), instance_ids, + &shuffle_global_image_ids); + dout(5) << "shuffling global_image_ids=[" << shuffle_global_image_ids + << "]" << dendl; + for (auto& global_image_id : shuffle_global_image_ids) { + auto it = m_image_states.find(global_image_id); + assert(it != m_image_states.end()); + + auto& image_state = it->second; + if (set_state(&image_state, StateTransition::STATE_SHUFFLING, false)) { + global_image_ids->emplace(global_image_id); + } } } -void Policy::remove_instances(const std::vector &instance_ids, - std::set *remap_global_image_ids) { - dout(20) << ": removing " << instance_ids.size() << " instance(s)" << dendl; +void Policy::remove_instances(const InstanceIds &instance_ids, + GlobalImageIds* global_image_ids) { + dout(5) << "instance_ids=" << instance_ids << dendl; RWLock::WLocker map_lock(m_map_lock); + for (auto& instance_id : instance_ids) { + auto map_it = m_map.find(instance_id); + if (map_it != m_map.end()) { + m_dead_instances.insert(instance_id); + dout(5) << "force shuffling global_image_ids=[" << map_it->second + << "]" << dendl; + for (auto& global_image_id : map_it->second) { + auto it = m_image_states.find(global_image_id); + assert(it != m_image_states.end()); + + auto& image_state = it->second; + if (is_state_scheduled(image_state, + StateTransition::STATE_DISSOCIATING)) { + // don't shuffle images that no longer exist + continue; + } - for (auto const &instance : instance_ids) { - dout(10) << ": removing instance_id=" << instance << dendl; - for (auto const &global_image_id : m_map[instance]) { - if (!actions_pending(global_image_id, m_map_lock)) { - remap_global_image_ids->emplace(global_image_id); + if (set_state(&image_state, StateTransition::STATE_SHUFFLING, true)) { + global_image_ids->emplace(global_image_id); + } } } } - - m_dead_instances.insert(instance_ids.begin(), instance_ids.end()); } -// new actions are always started from a stable (idle) state since -// actions either complete successfully ending up in an idle state -// or get aborted due to peer being blacklisted. -void Policy::start_next_action(const std::string &global_image_id) { +ActionType Policy::start_action(const std::string &global_image_id) { RWLock::WLocker map_lock(m_map_lock); - auto it = m_actions.find(global_image_id); - assert(it != m_actions.end()); - assert(!it->second.actions.empty()); - - ActionState &action_state = it->second; - Action &action = action_state.actions.front(); - - StateTransition::ActionType action_type = action.get_action_type(); - action_state.transition = StateTransition::transit(action_type, action_state.current_state); + auto it = m_image_states.find(global_image_id); + assert(it != m_image_states.end()); - StateTransition::State next_state = action_state.transition.next_state; + auto& image_state = it->second; + auto& transition = image_state.transition; + assert(transition.action_type != ACTION_TYPE_NONE); - dout(10) << ": global_image_id=" << global_image_id << ", action=" << action - << ", current_state=" << action_state.current_state << ", next_state=" - << next_state << dendl; - - // invoke state context callback - pre_execute_state_callback(global_image_id, action_type, next_state); - m_map_lock.put_write(); - action.execute_state_callback(next_state); - m_map_lock.get_write(); + dout(5) << "global_image_id=" << global_image_id << ", " + << "state=" << image_state.state << ", " + << "action_type=" << transition.action_type << dendl; + if (transition.start_policy_action) { + execute_policy_action(global_image_id, &image_state, + *transition.start_policy_action); + transition.start_policy_action = boost::none; + } + return transition.action_type; } bool Policy::finish_action(const std::string &global_image_id, int r) { RWLock::WLocker map_lock(m_map_lock); - dout(10) << ": global_image_id=" << global_image_id << ", r=" << r - << dendl; - - auto it = m_actions.find(global_image_id); - assert(it != m_actions.end()); - assert(!it->second.actions.empty()); - - ActionState &action_state = it->second; - Action &action = action_state.actions.front(); - - bool complete; - if (can_transit(action_state, r)) { - complete = perform_transition(global_image_id, &action_state, &action, r != 0); - } else { - complete = abort_or_retry(&action_state, &action); - } - - if (complete) { - dout(10) << ": completing action=" << action << dendl; - - m_map_lock.put_write(); - action.execute_completion_callback(r); - m_map_lock.get_write(); - - action_state.last_idle_state.reset(); - action_state.actions.pop_front(); + auto it = m_image_states.find(global_image_id); + assert(it != m_image_states.end()); + + auto& image_state = it->second; + auto& transition = image_state.transition; + dout(5) << "global_image_id=" << global_image_id << ", " + << "state=" << image_state.state << ", " + << "action_type=" << transition.action_type << ", " + << "r=" << r << dendl; + + // retry on failure unless it's an RPC message to an instance that is dead + if (r < 0 && + (!is_instance_action(image_state.transition.action_type) || + image_state.instance_id == UNMAPPED_INSTANCE_ID || + m_dead_instances.find(image_state.instance_id) == + m_dead_instances.end())) { + return true; } - return !action_state.actions.empty(); -} - -bool Policy::queue_action(const std::string &global_image_id, const Action &action) { - dout(20) << ": global_image_id=" << global_image_id << ", action=" << action - << dendl; - assert(m_map_lock.is_wlocked()); - - auto it = m_actions.find(global_image_id); - assert(it != m_actions.end()); - - it->second.actions.push_back(action); - return it->second.actions.size() == 1; -} - -void Policy::rollback(ActionState *action_state) { - dout(20) << dendl; - assert(m_map_lock.is_wlocked()); - - assert(action_state->transition.error_state); - StateTransition::State state = action_state->transition.error_state.get(); - - dout(10) << ": rolling back state=" << action_state->current_state << " -> " - << state << dendl; - action_state->current_state = state; -} - -bool Policy::advance(const std::string &global_image_id, - ActionState *action_state, Action *action) { - dout(20) << dendl; - assert(m_map_lock.is_wlocked()); - - StateTransition::State state = action_state->transition.next_state; - if (!is_state_retriable(state)) { - action->state_callback_complete(state); - } - - post_execute_state_callback(global_image_id, state); - - bool reached_final_state = false; - if (action_state->transition.final_state) { - reached_final_state = true; - state = action_state->transition.final_state.get(); - assert(is_idle_state(state)); - } - - dout(10) << ": advancing state=" << action_state->current_state << " -> " - << state << dendl; - action_state->current_state = state; - - return reached_final_state; -} - -bool Policy::perform_transition(const std::string &global_image_id, - ActionState *action_state, Action *action, bool transition_error) { - dout(20) << dendl; - assert(m_map_lock.is_wlocked()); - - bool complete = false; - if (transition_error) { - rollback(action_state); - } else { - complete = advance(global_image_id, action_state, action); + auto finish_policy_action = transition.finish_policy_action; + StateTransition::transit(image_state.state, &image_state.transition); + if (transition.finish_state) { + // in-progress state machine complete + assert(StateTransition::is_idle(*transition.finish_state)); + image_state.state = *transition.finish_state; + image_state.transition = {}; } - if (is_idle_state(action_state->current_state)) { - action_state->last_idle_state = action_state->current_state; - dout(10) << ": transition reached idle state=" << action_state->current_state - << dendl; + if (StateTransition::is_idle(image_state.state) && image_state.next_state) { + // advance to pending state machine + bool start_action = set_state(&image_state, *image_state.next_state, false); + assert(start_action); } - return complete; -} - -bool Policy::abort_or_retry(ActionState *action_state, Action *action) { - dout(20) << dendl; - assert(m_map_lock.is_wlocked()); - - StateTransition::State state = action_state->transition.next_state; - bool complete = !is_state_retriable(state); - - if (complete) { - // we aborted, so the context need not be freed later - action->state_callback_complete(state); - - if (action_state->last_idle_state) { - dout(10) << ": using last idle state=" << action_state->last_idle_state.get() - << " as current state" << dendl; - action_state->current_state = action_state->last_idle_state.get(); - } + if (finish_policy_action) { + execute_policy_action(global_image_id, &image_state, *finish_policy_action); } - return complete; + return (image_state.transition.action_type != ACTION_TYPE_NONE); } -void Policy::pre_execute_state_callback(const std::string &global_image_id, - StateTransition::ActionType action_type, - StateTransition::State state) { - assert(m_map_lock.is_wlocked()); - - dout(10) << ": global_image_id=" << global_image_id << ", action_type=" - << action_type << ", state=" << state << dendl; +void Policy::execute_policy_action( + const std::string& global_image_id, ImageState* image_state, + StateTransition::PolicyAction policy_action) { + dout(5) << "global_image_id=" << global_image_id << ", " + << "policy_action=" << policy_action << dendl; - utime_t map_time = generate_image_map_timestamp(action_type); - switch (state) { - case StateTransition::STATE_UPDATE_MAPPING: - map(global_image_id, map_time); + switch (policy_action) { + case StateTransition::POLICY_ACTION_MAP: + map(global_image_id, image_state); break; - case StateTransition::STATE_ASSOCIATED: - case StateTransition::STATE_DISASSOCIATED: - case StateTransition::STATE_REMOVE_MAPPING: + case StateTransition::POLICY_ACTION_UNMAP: + unmap(global_image_id, image_state); + break; + case StateTransition::POLICY_ACTION_REMOVE: + if (image_state->state == StateTransition::STATE_UNASSOCIATED) { + assert(image_state->instance_id == UNMAPPED_INSTANCE_ID); + assert(!image_state->next_state); + m_image_states.erase(global_image_id); + } break; - case StateTransition::STATE_UNASSIGNED: - default: - assert(false); } } -void Policy::post_execute_state_callback(const std::string &global_image_id, StateTransition::State state) { +void Policy::map(const std::string& global_image_id, ImageState* image_state) { assert(m_map_lock.is_wlocked()); - dout(10) << ": global_image_id=" << global_image_id << ", state=" << state << dendl; - - switch (state) { - case StateTransition::STATE_DISASSOCIATED: - unmap(global_image_id); - break; - case StateTransition::STATE_ASSOCIATED: - case StateTransition::STATE_UPDATE_MAPPING: - case StateTransition::STATE_REMOVE_MAPPING: - break; - default: - case StateTransition::STATE_UNASSIGNED: - assert(false); + std::string instance_id = image_state->instance_id; + if (instance_id != UNMAPPED_INSTANCE_ID && !is_dead_instance(instance_id)) { + return; } -} - -bool Policy::actions_pending(const std::string &global_image_id, const RWLock &lock) { - dout(20) << ": global_image_id=" << global_image_id << dendl; - assert(m_map_lock.is_locked()); - - auto it = m_actions.find(global_image_id); - assert(it != m_actions.end()); - - return !it->second.actions.empty(); -} - -LookupInfo Policy::lookup(const std::string &global_image_id, const RWLock &lock) { - assert(m_map_lock.is_locked()); - - LookupInfo info; - - for (auto it = m_map.begin(); it != m_map.end(); ++it) { - if (it->second.find(global_image_id) != it->second.end()) { - info.instance_id = it->first; - info.mapped_time = get_image_mapped_timestamp(global_image_id); - } + if (is_dead_instance(instance_id)) { + unmap(global_image_id, image_state); } - return info; -} + instance_id = do_map(m_map, global_image_id); + dout(5) << "global_image_id=" << global_image_id << ", " + << "instance_id=" << instance_id << dendl; -void Policy::map(const std::string &global_image_id, const std::string &instance_id, - utime_t map_time, const RWLock &lock) { - assert(m_map_lock.is_wlocked()); + image_state->instance_id = instance_id; + image_state->mapped_time = ceph_clock_now(); auto ins = m_map[instance_id].emplace(global_image_id); assert(ins.second); - - set_image_mapped_timestamp(global_image_id, map_time); } -void Policy::unmap(const std::string &global_image_id, const std::string &instance_id, - const RWLock &lock) { +void Policy::unmap(const std::string &global_image_id, + ImageState* image_state) { assert(m_map_lock.is_wlocked()); + std::string instance_id = image_state->instance_id; + if (instance_id == UNMAPPED_INSTANCE_ID) { + return; + } + + dout(5) << "global_image_id=" << global_image_id << ", " + << "instance_id=" << instance_id << dendl; + m_map[instance_id].erase(global_image_id); + image_state->instance_id = UNMAPPED_INSTANCE_ID; + image_state->mapped_time = {}; if (is_dead_instance(instance_id) && m_map[instance_id].empty()) { - dout(10) << ": removing dead instance_id=" << instance_id << dendl; + dout(5) << "removing dead instance_id=" << instance_id << dendl; m_map.erase(instance_id); m_dead_instances.erase(instance_id); } } -void Policy::map(const std::string &global_image_id, utime_t map_time) { - dout(20) << ": global_image_id=" << global_image_id << dendl; - assert(m_map_lock.is_wlocked()); - - LookupInfo info = lookup(global_image_id, m_map_lock); - std::string instance_id = info.instance_id; - - if (instance_id != UNMAPPED_INSTANCE_ID && !is_dead_instance(instance_id)) { - return; - } - if (is_dead_instance(instance_id)) { - unmap(global_image_id, instance_id, m_map_lock); - } - - instance_id = do_map(global_image_id); - map(global_image_id, instance_id, map_time, m_map_lock); -} - -void Policy::unmap(const std::string &global_image_id) { - dout(20) << ": global_image_id=" << global_image_id << dendl; - assert(m_map_lock.is_wlocked()); +bool Policy::is_image_shuffling(const std::string &global_image_id) { + assert(m_map_lock.is_locked()); - LookupInfo info = lookup(global_image_id, m_map_lock); - if (info.instance_id == UNMAPPED_INSTANCE_ID) { - return; - } + auto it = m_image_states.find(global_image_id); + assert(it != m_image_states.end()); + auto& image_state = it->second; - unmap(global_image_id, info.instance_id, m_map_lock); + // avoid attempting to re-shuffle a pending shuffle + auto result = is_state_scheduled(image_state, + StateTransition::STATE_SHUFFLING); + dout(20) << "global_image_id=" << global_image_id << ", " + << "result=" << result << dendl; + return result; } bool Policy::can_shuffle_image(const std::string &global_image_id) { - dout(20) << ": global_image_id=" << global_image_id << dendl; assert(m_map_lock.is_locked()); CephContext *cct = reinterpret_cast(m_ioctx.cct()); - int migration_throttle = cct->_conf->get_val("rbd_mirror_image_policy_migration_throttle"); + int migration_throttle = cct->_conf->get_val( + "rbd_mirror_image_policy_migration_throttle"); - utime_t last_shuffled_time = get_image_mapped_timestamp(global_image_id); - dout(10) << ": migration_throttle=" << migration_throttle << ", last_shuffled_time=" - << last_shuffled_time << dendl; + auto it = m_image_states.find(global_image_id); + assert(it != m_image_states.end()); + auto& image_state = it->second; + utime_t last_shuffled_time = image_state.mapped_time; + + // idle images that haven't been recently remapped can shuffle utime_t now = ceph_clock_now(); - return !actions_pending(global_image_id, m_map_lock) && - !(migration_throttle > 0 && (now - last_shuffled_time < migration_throttle)); + auto result = (StateTransition::is_idle(image_state.state) && + ((migration_throttle <= 0) || + (now - last_shuffled_time >= migration_throttle))); + dout(10) << "global_image_id=" << global_image_id << ", " + << "migration_throttle=" << migration_throttle << ", " + << "last_shuffled_time=" << last_shuffled_time << ", " + << "result=" << result << dendl; + return result; +} + +bool Policy::set_state(ImageState* image_state, StateTransition::State state, + bool ignore_current_state) { + if (!ignore_current_state && image_state->state == state) { + return false; + } else if (StateTransition::is_idle(image_state->state)) { + image_state->state = state; + image_state->next_state = boost::none; + + StateTransition::transit(image_state->state, &image_state->transition); + assert(image_state->transition.action_type != ACTION_TYPE_NONE); + assert(!image_state->transition.finish_state); + return true; + } + + image_state->next_state = state; + return false; +} + +bool Policy::is_state_scheduled(const ImageState& image_state, + StateTransition::State state) const { + return (image_state.state == StateTransition::STATE_DISSOCIATING || + (image_state.next_state && + *image_state.next_state == StateTransition::STATE_DISSOCIATING)); } } // namespace image_map diff --git a/src/tools/rbd_mirror/image_map/Policy.h b/src/tools/rbd_mirror/image_map/Policy.h index 547bce49fba60..5b1ec322bdc10 100644 --- a/src/tools/rbd_mirror/image_map/Policy.h +++ b/src/tools/rbd_mirror/image_map/Policy.h @@ -11,7 +11,7 @@ #include "common/RWLock.h" #include "cls/rbd/cls_rbd_types.h" #include "include/rados/librados.hpp" -#include "Action.h" +#include "tools/rbd_mirror/image_map/StateTransition.h" #include "tools/rbd_mirror/image_map/Types.h" class Context; @@ -28,144 +28,87 @@ public: } // init -- called during initialization - void init(const std::map &image_mapping); + void init( + const std::map &image_mapping); // lookup an image from the map LookupInfo lookup(const std::string &global_image_id); - // add, remove, shuffle - bool add_image(const std::string &global_image_id, - Context *on_update, Context *on_acquire, Context *on_finish); - bool remove_image(const std::string &global_image_id, - Context *on_release, Context *on_remove, Context *on_finish); - bool shuffle_image(const std::string &global_image_id, - Context *on_release, Context *on_update, - Context *on_acquire, Context *on_finish); + // add, remove + bool add_image(const std::string &global_image_id); + bool remove_image(const std::string &global_image_id); // shuffle images when instances are added/removed - void add_instances(const std::vector &instance_ids, - std::set *remap_global_image_ids); - void remove_instances(const std::vector &instance_ids, - std::set *remap_global_image_ids); + void add_instances(const InstanceIds &instance_ids, + GlobalImageIds* global_image_ids); + void remove_instances(const InstanceIds &instance_ids, + GlobalImageIds* global_image_ids); - void start_next_action(const std::string &global_image_id); + ActionType start_action(const std::string &global_image_id); bool finish_action(const std::string &global_image_id, int r); -private: - typedef std::list Actions; - - struct ActionState { - Actions actions; // list of pending actions - - StateTransition::State current_state = StateTransition::STATE_UNASSIGNED; // current state - boost::optional last_idle_state; // last successfull idle - // state transition - - StateTransition::Transition transition; // (cached) next transition - - utime_t map_time; // (re)mapped time - }; +protected: + typedef std::map > InstanceToImageMap; - // for the lack of a better function name - bool is_state_retriable(StateTransition::State state) { - return state == StateTransition::STATE_UPDATE_MAPPING || - state == StateTransition::STATE_REMOVE_MAPPING || - state == StateTransition::STATE_ASSOCIATED; - } - // can the state machine transit advance (on success) or rollback - // (on failure). - bool can_transit(const ActionState &action_state, int r) { + bool is_dead_instance(const std::string instance_id) { assert(m_map_lock.is_locked()); - return r == 0 || action_state.transition.error_state; - } - - void set_image_mapped_timestamp(const std::string &global_image_id, utime_t time) { - assert(m_map_lock.is_wlocked()); - - auto it = m_actions.find(global_image_id); - assert(it != m_actions.end()); - it->second.map_time = time; + return m_dead_instances.find(instance_id) != m_dead_instances.end(); } - utime_t get_image_mapped_timestamp(const std::string &global_image_id) { - assert(m_map_lock.is_locked()); - auto it = m_actions.find(global_image_id); - assert(it != m_actions.end()); - return it->second.map_time; - } + bool is_image_shuffling(const std::string &global_image_id); + bool can_shuffle_image(const std::string &global_image_id); - librados::IoCtx &m_ioctx; - std::map m_actions; - std::set m_dead_instances; + // map an image (global image id) to an instance + virtual std::string do_map(const InstanceToImageMap& map, + const std::string &global_image_id) = 0; - bool is_idle_state(StateTransition::State state) { - if (state == StateTransition::STATE_UNASSIGNED || - state == StateTransition::STATE_ASSOCIATED || - state == StateTransition::STATE_DISASSOCIATED) { - return true; - } + // shuffle images when instances are added/removed + virtual void do_shuffle_add_instances( + const InstanceToImageMap& map, size_t image_count, + const std::vector &instance_ids, + std::set *remap_global_image_ids) = 0; - return false; - } +private: + struct ImageState { + std::string instance_id = UNMAPPED_INSTANCE_ID; + utime_t mapped_time; - // generate image map time based on action type - utime_t generate_image_map_timestamp(StateTransition::ActionType action_type) { - // for a shuffle action (image remap) use current time as - // map time, historical time otherwise. - utime_t time; - if (action_type == StateTransition::ACTION_TYPE_SHUFFLE) { - time = ceph_clock_now(); - } else { - time = utime_t(0, 0); + ImageState() {} + ImageState(const std::string& instance_id, const utime_t& mapped_time) + : instance_id(instance_id), mapped_time(mapped_time) { } - return time; - } - - bool queue_action(const std::string &global_image_id, const Action &action); - bool actions_pending(const std::string &global_image_id, const RWLock &lock); + // active state and action + StateTransition::State state = StateTransition::STATE_UNASSOCIATED; + StateTransition::Transition transition; - LookupInfo lookup(const std::string &global_image_id, const RWLock &lock); - void map(const std::string &global_image_id, - const std::string &instance_id, utime_t map_time, const RWLock &lock); - void unmap(const std::string &global_image_id, const std::string &instance_id, const RWLock &lock); - - // map an image - void map(const std::string &global_image_id, utime_t map_time); - // unmap (remove) an image from the map - void unmap(const std::string &global_image_id); + // next scheduled state + boost::optional next_state = boost::none; + }; - // state transition related.. - void pre_execute_state_callback(const std::string &global_image_id, - StateTransition::ActionType action_type, StateTransition::State state); - void post_execute_state_callback(const std::string &global_image_id, StateTransition::State state); + typedef std::map ImageStates; - void rollback(ActionState *action_state); - bool advance(const std::string &global_image_id, ActionState *action_state, Action *action); + librados::IoCtx &m_ioctx; - bool perform_transition(const std::string &global_image_id, ActionState *action_state, - Action *action, bool transition_error); - bool abort_or_retry(ActionState *action_state, Action *action); + RWLock m_map_lock; // protects m_map + InstanceToImageMap m_map; // instance_id -> global_id map -protected: - typedef std::map > InstanceToImageMap; + ImageStates m_image_states; + std::set m_dead_instances; - RWLock m_map_lock; // protects m_map, m_shuffled_timestamp - InstanceToImageMap m_map; // instance_id -> global_id map + bool set_state(ImageState* image_state, StateTransition::State state, + bool ignore_current_state); - bool is_dead_instance(const std::string &instance_id) { - assert(m_map_lock.is_locked()); - return m_dead_instances.find(instance_id) != m_dead_instances.end(); - } + void execute_policy_action(const std::string& global_image_id, + ImageState* image_state, + StateTransition::PolicyAction policy_action); - bool can_shuffle_image(const std::string &global_image_id); + void map(const std::string& global_image_id, ImageState* image_state); + void unmap(const std::string &global_image_id, ImageState* image_state); - // map an image (global image id) to an instance - virtual std::string do_map(const std::string &global_image_id) = 0; + bool is_state_scheduled(const ImageState& image_state, + StateTransition::State state) const; - // shuffle images when instances are added/removed - virtual void do_shuffle_add_instances(const std::vector &instance_ids, - std::set *remap_global_image_ids) = 0; }; } // namespace image_map diff --git a/src/tools/rbd_mirror/image_map/SimplePolicy.cc b/src/tools/rbd_mirror/image_map/SimplePolicy.cc index dc2ec5a347cbf..3c817e2eaf221 100644 --- a/src/tools/rbd_mirror/image_map/SimplePolicy.cc +++ b/src/tools/rbd_mirror/image_map/SimplePolicy.cc @@ -10,7 +10,7 @@ #define dout_subsys ceph_subsys_rbd_mirror #undef dout_prefix #define dout_prefix *_dout << "rbd::mirror::image_map::SimplePolicy: " << this \ - << " " << __func__ + << " " << __func__ << ": " namespace rbd { namespace mirror { namespace image_map { @@ -19,17 +19,17 @@ SimplePolicy::SimplePolicy(librados::IoCtx &ioctx) : Policy(ioctx) { } -uint64_t SimplePolicy::calc_images_per_instance(int nr_instances) { - assert(nr_instances > 0); - - uint64_t nr_images = 0; - for (auto const &it : m_map) { +size_t SimplePolicy::calc_images_per_instance(const InstanceToImageMap& map, + size_t image_count) { + size_t nr_instances = 0; + for (auto const &it : map) { if (!Policy::is_dead_instance(it.first)) { - nr_images += it.second.size(); + ++nr_instances; } } + assert(nr_instances > 0); - uint64_t images_per_instance = nr_images / nr_instances; + size_t images_per_instance = image_count / nr_instances; if (images_per_instance == 0) { ++images_per_instance; } @@ -37,14 +37,14 @@ uint64_t SimplePolicy::calc_images_per_instance(int nr_instances) { return images_per_instance; } -void SimplePolicy::do_shuffle_add_instances(const std::vector &instance_ids, - std::set *remap_global_image_ids) { - assert(m_map_lock.is_wlocked()); - - uint64_t images_per_instance = calc_images_per_instance(m_map.size()); - dout(5) << ": images per instance=" << images_per_instance << dendl; +void SimplePolicy::do_shuffle_add_instances( + const InstanceToImageMap& map, size_t image_count, + const std::vector &instance_ids, + std::set *remap_global_image_ids) { + uint64_t images_per_instance = calc_images_per_instance(map, image_count); + dout(5) << "images per instance=" << images_per_instance << dendl; - for (auto const &instance : m_map) { + for (auto const &instance : map) { if (instance.second.size() <= images_per_instance) { continue; } @@ -53,7 +53,9 @@ void SimplePolicy::do_shuffle_add_instances(const std::vector &inst uint64_t cut_off = instance.second.size() - images_per_instance; while (it != instance.second.end() && cut_off > 0) { - if (Policy::can_shuffle_image(*it)) { + if (Policy::is_image_shuffling(*it)) { + --cut_off; + } else if (Policy::can_shuffle_image(*it)) { --cut_off; remap_global_image_ids->emplace(*it); } @@ -63,19 +65,18 @@ void SimplePolicy::do_shuffle_add_instances(const std::vector &inst } } -std::string SimplePolicy::do_map(const std::string &global_image_id) { - assert(m_map_lock.is_wlocked()); - - auto min_it = m_map.begin(); - - for (auto it = min_it; it != m_map.end(); ++it) { +std::string SimplePolicy::do_map(const InstanceToImageMap& map, + const std::string &global_image_id) { + auto min_it = map.begin(); + for (auto it = min_it; it != map.end(); ++it) { assert(it->second.find(global_image_id) == it->second.end()); - if (it->second.size() < min_it->second.size() && !Policy::is_dead_instance(it->first)) { + if (it->second.size() < min_it->second.size() && + !Policy::is_dead_instance(it->first)) { min_it = it; } } - dout(20) << ": global_image_id=" << global_image_id << " maps to instance_id=" + dout(20) << "global_image_id=" << global_image_id << " maps to instance_id=" << min_it->first << dendl; return min_it->first; } diff --git a/src/tools/rbd_mirror/image_map/SimplePolicy.h b/src/tools/rbd_mirror/image_map/SimplePolicy.h index 53511943b86c2..6a73cb624a6d9 100644 --- a/src/tools/rbd_mirror/image_map/SimplePolicy.h +++ b/src/tools/rbd_mirror/image_map/SimplePolicy.h @@ -17,18 +17,20 @@ public: } protected: - using Policy::m_map_lock; - using Policy::m_map; - SimplePolicy(librados::IoCtx &ioctx); - std::string do_map(const std::string &global_image_id) override; + std::string do_map(const InstanceToImageMap& map, + const std::string &global_image_id) override; - void do_shuffle_add_instances(const std::vector &instance_ids, - std::set *remap_global_image_ids) override; + void do_shuffle_add_instances( + const InstanceToImageMap& map, size_t image_count, + const std::vector &instance_ids, + std::set *remap_global_image_ids) override; private: - uint64_t calc_images_per_instance(int nr_instances); + size_t calc_images_per_instance(const InstanceToImageMap& map, + size_t image_count); + }; } // namespace image_map diff --git a/src/tools/rbd_mirror/image_map/StateTransition.cc b/src/tools/rbd_mirror/image_map/StateTransition.cc index b075e7f9a469d..1a3f521fe5c74 100644 --- a/src/tools/rbd_mirror/image_map/StateTransition.cc +++ b/src/tools/rbd_mirror/image_map/StateTransition.cc @@ -9,73 +9,85 @@ namespace rbd { namespace mirror { namespace image_map { -std::ostream &operator<<(std::ostream &os, const StateTransition::ActionType &action_type) { - switch (action_type) { - case StateTransition::ACTION_TYPE_ADD: - os << "ADD_IMAGE"; +std::ostream &operator<<(std::ostream &os, + const StateTransition::State &state) { + switch(state) { + case StateTransition::STATE_INITIALIZING: + os << "INITIALIZING"; + break; + case StateTransition::STATE_ASSOCIATING: + os << "ASSOCIATING"; + break; + case StateTransition::STATE_ASSOCIATED: + os << "ASSOCIATED"; + break; + case StateTransition::STATE_SHUFFLING: + os << "SHUFFLING"; break; - case StateTransition::ACTION_TYPE_REMOVE: - os << "REMOVE_IMAGE"; + case StateTransition::STATE_DISSOCIATING: + os << "DISSOCIATING"; break; - case StateTransition::ACTION_TYPE_SHUFFLE: - os << "SHUFFLE_IMAGE"; + case StateTransition::STATE_UNASSOCIATED: + os << "UNASSOCIATED"; break; - default: - os << "UNKNOWN (" << static_cast(action_type) << ")"; } - return os; } -std::ostream &operator<<(std::ostream &os, const StateTransition::State &state) { - switch(state) { - case StateTransition::STATE_UNASSIGNED: - os << "UNASSIGNED"; +std::ostream &operator<<(std::ostream &os, + const StateTransition::PolicyAction &policy_action) { + switch(policy_action) { + case StateTransition::POLICY_ACTION_MAP: + os << "MAP"; break; - case StateTransition::STATE_ASSOCIATED: - os << "ASSOCIATED"; - break; - case StateTransition::STATE_DISASSOCIATED: - os << "DISASSOCIATED"; - break; - case StateTransition::STATE_UPDATE_MAPPING: - os << "UPDATE_MAPPING"; + case StateTransition::POLICY_ACTION_UNMAP: + os << "UNMAP"; break; - case StateTransition::STATE_REMOVE_MAPPING: - os << "REMOVE_MAPPING"; - break; - case StateTransition::STATE_COMPLETE: - os << "COMPLETE"; + case StateTransition::POLICY_ACTION_REMOVE: + os << "REMOVE"; break; } - return os; } -const StateTransition::TransitionTable StateTransition::transition_table[] = { - // action_type current_state Transition - // ------------------------------------------------------------------------------- - ACTION_TYPE_ADD, STATE_UNASSIGNED, Transition(STATE_UPDATE_MAPPING), - ACTION_TYPE_ADD, STATE_UPDATE_MAPPING, Transition(STATE_ASSOCIATED, STATE_ASSOCIATED, - STATE_UNASSIGNED), +const StateTransition::TransitionTable StateTransition::s_transition_table { + // state current_action Transition + // --------------------------------------------------------------------------- + {{STATE_INITIALIZING, ACTION_TYPE_NONE}, {ACTION_TYPE_ACQUIRE, {}, {}, + {}}}, + {{STATE_INITIALIZING, ACTION_TYPE_ACQUIRE}, {ACTION_TYPE_NONE, {}, {}, + {STATE_ASSOCIATED}}}, - ACTION_TYPE_REMOVE, STATE_ASSOCIATED, Transition(STATE_DISASSOCIATED), - ACTION_TYPE_REMOVE, STATE_DISASSOCIATED, Transition(STATE_REMOVE_MAPPING, STATE_UNASSIGNED), + {{STATE_ASSOCIATING, ACTION_TYPE_NONE}, {ACTION_TYPE_MAP_UPDATE, + {POLICY_ACTION_MAP}, {}, {}}}, + {{STATE_ASSOCIATING, ACTION_TYPE_MAP_UPDATE}, {ACTION_TYPE_ACQUIRE, {}, {}, + {}}}, + {{STATE_ASSOCIATING, ACTION_TYPE_ACQUIRE}, {ACTION_TYPE_NONE, {}, {}, + {STATE_ASSOCIATED}}}, - ACTION_TYPE_SHUFFLE, STATE_ASSOCIATED, Transition(STATE_DISASSOCIATED), - ACTION_TYPE_SHUFFLE, STATE_DISASSOCIATED, Transition(STATE_UPDATE_MAPPING), - ACTION_TYPE_SHUFFLE, STATE_UPDATE_MAPPING, Transition(STATE_ASSOCIATED, STATE_ASSOCIATED, - STATE_DISASSOCIATED), + {{STATE_DISSOCIATING, ACTION_TYPE_NONE}, {ACTION_TYPE_RELEASE, {}, {}, + {}}}, + {{STATE_DISSOCIATING, ACTION_TYPE_RELEASE}, {ACTION_TYPE_MAP_REMOVE, {}, + {POLICY_ACTION_UNMAP}, {}}}, + {{STATE_DISSOCIATING, ACTION_TYPE_MAP_REMOVE}, {ACTION_TYPE_NONE, {}, + {POLICY_ACTION_REMOVE}, + {STATE_UNASSOCIATED}}}, + + {{STATE_SHUFFLING, ACTION_TYPE_NONE}, {ACTION_TYPE_RELEASE, {}, + {POLICY_ACTION_UNMAP}, {}}}, + {{STATE_SHUFFLING, ACTION_TYPE_RELEASE}, {ACTION_TYPE_MAP_UPDATE, + {POLICY_ACTION_MAP}, {}, {}}}, + {{STATE_SHUFFLING, ACTION_TYPE_MAP_UPDATE}, {ACTION_TYPE_ACQUIRE, {}, {}, + {}}}, + {{STATE_SHUFFLING, ACTION_TYPE_ACQUIRE}, {ACTION_TYPE_NONE, {}, {}, + {STATE_ASSOCIATED}}} }; -const StateTransition::Transition &StateTransition::transit(ActionType action_type, State state) { - for (auto const &entry : transition_table) { - if (entry.action_type == action_type && entry.current_state == state) { - return entry.transition; - } - } +void StateTransition::transit(State state, Transition* transition) { + auto it = s_transition_table.find({state, transition->action_type}); + assert(it != s_transition_table.end()); - assert(false); + *transition = it->second; } } // namespace image_map diff --git a/src/tools/rbd_mirror/image_map/StateTransition.h b/src/tools/rbd_mirror/image_map/StateTransition.h index 7cc9db051ea9a..02a5ce4e9c256 100644 --- a/src/tools/rbd_mirror/image_map/StateTransition.h +++ b/src/tools/rbd_mirror/image_map/StateTransition.h @@ -4,7 +4,9 @@ #ifndef CEPH_RBD_MIRROR_IMAGE_MAP_STATE_TRANSITION_H #define CEPH_RBD_MIRROR_IMAGE_MAP_STATE_TRANSITION_H +#include "tools/rbd_mirror/image_map/Types.h" #include +#include namespace rbd { namespace mirror { @@ -12,61 +14,60 @@ namespace image_map { class StateTransition { public: - enum ActionType { - ACTION_TYPE_ADD = 0, - ACTION_TYPE_REMOVE, - ACTION_TYPE_SHUFFLE, + enum State { + STATE_UNASSOCIATED, + STATE_INITIALIZING, + STATE_ASSOCIATING, + STATE_ASSOCIATED, + STATE_SHUFFLING, + STATE_DISSOCIATING }; - enum State { - STATE_UNASSIGNED = 0, // starting (initial) state - STATE_ASSOCIATED, // acquire image - STATE_DISASSOCIATED, // release image - STATE_UPDATE_MAPPING, // update on-disk map - STATE_REMOVE_MAPPING, // remove on-disk map - STATE_COMPLETE, // special state to invoke completion callback + enum PolicyAction { + POLICY_ACTION_MAP, + POLICY_ACTION_UNMAP, + POLICY_ACTION_REMOVE }; struct Transition { - State next_state; - boost::optional final_state; - boost::optional error_state; + // image map action + ActionType action_type = ACTION_TYPE_NONE; - Transition() - : Transition(STATE_UNASSIGNED, boost::none, boost::none) { - } - Transition(State next_state) - : Transition(next_state, boost::none, boost::none) { - } - Transition(State next_state, State final_state) - : Transition(next_state, final_state, boost::none) { + // policy internal action + boost::optional start_policy_action; + boost::optional finish_policy_action; + + // state machine complete + boost::optional finish_state; + + Transition() { } - Transition(State next_state, boost::optional final_state, - boost::optional error_state) - : next_state(next_state), - final_state(final_state), - error_state(error_state) { + Transition(ActionType action_type, + const boost::optional& start_policy_action, + const boost::optional& finish_policy_action, + const boost::optional& finish_state) + : action_type(action_type), start_policy_action(start_policy_action), + finish_policy_action(finish_policy_action), finish_state(finish_state) { } }; - static const Transition &transit(ActionType action_type, State state); + static bool is_idle(State state) { + return (state == STATE_UNASSOCIATED || state == STATE_ASSOCIATED); + } -private: - struct TransitionTable { - // in: action + current_state - ActionType action_type; - State current_state; + static void transit(State state, Transition* transition); - // out: Transition - Transition transition; - }; +private: + typedef std::pair TransitionKey; + typedef std::map TransitionTable; // image transition table - static const TransitionTable transition_table[]; + static const TransitionTable s_transition_table; }; -std::ostream &operator<<(std::ostream &os, const StateTransition::ActionType &action_type); std::ostream &operator<<(std::ostream &os, const StateTransition::State &state); +std::ostream &operator<<(std::ostream &os, + const StateTransition::PolicyAction &policy_action); } // namespace image_map } // namespace mirror diff --git a/src/tools/rbd_mirror/image_map/Types.cc b/src/tools/rbd_mirror/image_map/Types.cc index 525ee2ac12324..e9041a5b805d4 100644 --- a/src/tools/rbd_mirror/image_map/Types.cc +++ b/src/tools/rbd_mirror/image_map/Types.cc @@ -5,6 +5,7 @@ #include "include/assert.h" #include "include/stringify.h" #include "common/Formatter.h" +#include namespace rbd { namespace mirror { @@ -108,6 +109,30 @@ void PolicyData::generate_test_instances(std::list &o) { o.push_back(new PolicyData(PolicyMetaNone())); } +std::ostream &operator<<(std::ostream &os, const ActionType& action_type) { + switch (action_type) { + case ACTION_TYPE_NONE: + os << "NONE"; + break; + case ACTION_TYPE_MAP_UPDATE: + os << "MAP_UPDATE"; + break; + case ACTION_TYPE_MAP_REMOVE: + os << "MAP_REMOVE"; + break; + case ACTION_TYPE_ACQUIRE: + os << "ACQUIRE"; + break; + case ACTION_TYPE_RELEASE: + os << "RELEASE"; + break; + default: + os << "UNKNOWN (" << static_cast(action_type) << ")"; + break; + } + return os; +} + } // namespace image_map } // namespace mirror } // namespace rbd diff --git a/src/tools/rbd_mirror/image_map/Types.h b/src/tools/rbd_mirror/image_map/Types.h index 56433fa1b1cd2..bc23622e8efbe 100644 --- a/src/tools/rbd_mirror/image_map/Types.h +++ b/src/tools/rbd_mirror/image_map/Types.h @@ -4,7 +4,9 @@ #ifndef CEPH_RBD_MIRROR_IMAGE_MAP_TYPES_H #define CEPH_RBD_MIRROR_IMAGE_MAP_TYPES_H +#include #include +#include #include #include @@ -46,6 +48,18 @@ struct LookupInfo { utime_t mapped_time; }; +enum ActionType { + ACTION_TYPE_NONE, + ACTION_TYPE_MAP_UPDATE, + ACTION_TYPE_MAP_REMOVE, + ACTION_TYPE_ACQUIRE, + ACTION_TYPE_RELEASE +}; + +typedef std::vector InstanceIds; +typedef std::set GlobalImageIds; +typedef std::map ImageActionTypes; + enum PolicyMetaType { POLICY_META_TYPE_NONE = 0, }; @@ -107,9 +121,10 @@ struct PolicyData { WRITE_CLASS_ENCODER(PolicyData); +std::ostream &operator<<(std::ostream &os, const ActionType &action_type); + } // namespace image_map } // namespace mirror } // namespace rbd - #endif // CEPH_RBD_MIRROR_IMAGE_MAP_TYPES_H -- 2.39.5