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 <dillaman@redhat.com>

diff --git a/src/test/rbd_mirror/image_map/test_Policy.cc b/src/test/rbd_mirror/image_map/test_Policy.cc
index fe5692d9c9d59e2907ca07772da3785cbfc199a2..f22bae0f14ab2e47ec0ee6ea4f9236c4510cf3df 100644 (file)
--- 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<std::string> 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 12586bdcac3858397b3c81f86d9e5065cd9396ad..3edfd1517ae891eb569131fefa60d30a566c20f2 100644 (file)
--- 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<std::string> 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<std::string> 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<std::string> 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<std::string> 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<std::string> 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 ca7517e896891ff5d2d320cbc272e619ce735655..fe7d66931c33af8bba1154d8a74ce214005d3bcc 100644 (file)
--- 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<std::string>{});
   }
 
+  // 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<std::string> 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 5b1ec322bdc101140557ff725d46359d796c2646..b5acab215fbe2d843e5ad4c01460baba4cc79a40 100644 (file)
--- 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<std::string> 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);