From 7dce6d22b2cbff88ba455fbfa3352eec07f32714 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Sun, 8 Apr 2018 23:19:48 -0400 Subject: [PATCH] rbd-mirror: initial policy instance update should detect dead instances At initial promotion to the leader position, it can be nearly always assumed that at least one instance has died and needs to have its images shuffled to another instance (i.e. the old leader). Signed-off-by: Jason Dillaman --- src/test/rbd_mirror/image_map/test_Policy.cc | 24 +++++++++++++++++ src/test/rbd_mirror/test_mock_ImageMap.cc | 14 ++++++++++ src/tools/rbd_mirror/image_map/Policy.cc | 28 +++++++++++++++++++- src/tools/rbd_mirror/image_map/Policy.h | 5 ++++ 4 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/test/rbd_mirror/image_map/test_Policy.cc b/src/test/rbd_mirror/image_map/test_Policy.cc index fe5692d9c9d59..f22bae0f14ab2 100644 --- a/src/test/rbd_mirror/image_map/test_Policy.cc +++ b/src/test/rbd_mirror/image_map/test_Policy.cc @@ -342,6 +342,30 @@ TEST_F(TestImageMapPolicy, ShuffleFailureAndRemove) { ASSERT_TRUE(info.instance_id == UNMAPPED_INSTANCE_ID); } +TEST_F(TestImageMapPolicy, InitialInstanceUpdate) { + 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)); + + auto instance_id = stringify(m_local_io_ctx.get_instance_id()); + std::set shuffle_global_image_ids; + m_policy->add_instances({instance_id}, &shuffle_global_image_ids); + + ASSERT_EQ(0U, shuffle_global_image_ids.size()); + ASSERT_TRUE(m_policy->finish_action(global_image_id, -ENOENT)); + + ASSERT_EQ(ACTION_TYPE_RELEASE, m_policy->start_action(global_image_id)); + ASSERT_TRUE(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_EQ(ACTION_TYPE_ACQUIRE, m_policy->start_action(global_image_id)); + ASSERT_FALSE(m_policy->finish_action(global_image_id, 0)); +} + } // namespace image_map } // namespace mirror } // namespace rbd diff --git a/src/test/rbd_mirror/test_mock_ImageMap.cc b/src/test/rbd_mirror/test_mock_ImageMap.cc index 12586bdcac385..3edfd1517ae89 100644 --- a/src/test/rbd_mirror/test_mock_ImageMap.cc +++ b/src/test/rbd_mirror/test_mock_ImageMap.cc @@ -846,6 +846,9 @@ TEST_F(TestMockImageMap, AddInstance) { &peer_ack_ctxs); wait_for_scheduled_task(); + auto local_instance_id = stringify(m_local_io_ctx.get_instance_id()); + mock_image_map->update_instances_added({local_instance_id}); + std::set shuffled_global_image_ids; // RELEASE+UPDATE_MAPPING+ACQUIRE @@ -916,6 +919,9 @@ TEST_F(TestMockImageMap, RemoveInstance) { &peer_ack_ctxs); wait_for_scheduled_task(); + auto local_instance_id = stringify(m_local_io_ctx.get_instance_id()); + mock_image_map->update_instances_added({local_instance_id}); + std::set shuffled_global_image_ids; // RELEASE+UPDATE_MAPPING+ACQUIRE @@ -1006,6 +1012,8 @@ TEST_F(TestMockImageMap, AddInstancePingPongImageTest) { mock_image_map->init(&cond); ASSERT_EQ(0, cond.wait()); + mock_image_map->update_instances_added({local_instance_id}); + std::set global_image_ids_ack(global_image_ids); // remote peer ACKs image acquire request -- completing action @@ -1138,6 +1146,9 @@ TEST_F(TestMockImageMap, RemoveInstanceWithRemoveImage) { &peer_ack_ctxs); wait_for_scheduled_task(); + auto local_instance_id = stringify(m_local_io_ctx.get_instance_id()); + mock_image_map->update_instances_added({local_instance_id}); + std::set shuffled_global_image_ids; // RELEASE+UPDATE_MAPPING+ACQUIRE @@ -1207,6 +1218,9 @@ TEST_F(TestMockImageMap, AddErrorAndRemoveImage) { mock_image_map->init(&cond); ASSERT_EQ(0, cond.wait()); + auto local_instance_id = stringify(m_local_io_ctx.get_instance_id()); + mock_image_map->update_instances_added({local_instance_id}); + std::set global_image_ids{ "global id 1", "global id 2", "global id 3", "remote id 4", }; diff --git a/src/tools/rbd_mirror/image_map/Policy.cc b/src/tools/rbd_mirror/image_map/Policy.cc index ca7517e896891..fe7d66931c33a 100644 --- a/src/tools/rbd_mirror/image_map/Policy.cc +++ b/src/tools/rbd_mirror/image_map/Policy.cc @@ -114,6 +114,25 @@ void Policy::add_instances(const InstanceIds &instance_ids, m_map.emplace(instance, std::set{}); } + // post-failover, remove any dead instances and re-shuffle their images + if (m_initial_update) { + dout(5) << "initial instance update" << dendl; + m_initial_update = false; + + std::set alive_instances(instance_ids.begin(), + instance_ids.end()); + InstanceIds dead_instances; + for (auto& map_pair : m_map) { + if (alive_instances.find(map_pair.first) == alive_instances.end()) { + dead_instances.push_back(map_pair.first); + } + } + + if (!dead_instances.empty()) { + remove_instances(m_map_lock, dead_instances, global_image_ids); + } + } + GlobalImageIds shuffle_global_image_ids; do_shuffle_add_instances(m_map, m_image_states.size(), instance_ids, &shuffle_global_image_ids); @@ -132,9 +151,16 @@ void Policy::add_instances(const InstanceIds &instance_ids, void Policy::remove_instances(const InstanceIds &instance_ids, GlobalImageIds* global_image_ids) { + RWLock::WLocker map_lock(m_map_lock); + remove_instances(m_map_lock, instance_ids, global_image_ids); +} + +void Policy::remove_instances(const RWLock& lock, + const InstanceIds &instance_ids, + GlobalImageIds* global_image_ids) { + assert(m_map_lock.is_wlocked()); 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()) { diff --git a/src/tools/rbd_mirror/image_map/Policy.h b/src/tools/rbd_mirror/image_map/Policy.h index 5b1ec322bdc10..b5acab215fbe2 100644 --- a/src/tools/rbd_mirror/image_map/Policy.h +++ b/src/tools/rbd_mirror/image_map/Policy.h @@ -96,6 +96,11 @@ private: ImageStates m_image_states; std::set m_dead_instances; + bool m_initial_update = true; + + void remove_instances(const RWLock& lock, const InstanceIds &instance_ids, + GlobalImageIds* global_image_ids); + bool set_state(ImageState* image_state, StateTransition::State state, bool ignore_current_state); -- 2.39.5