]> git.apps.os.sepia.ceph.com Git - ceph-ci.git/commitdiff
rbd-mirror: ensure remote demotion is replayed locally
authorJason Dillaman <dillaman@redhat.com>
Fri, 4 May 2018 15:14:34 +0000 (11:14 -0400)
committerJason Dillaman <dillaman@redhat.com>
Wed, 9 May 2018 13:42:40 +0000 (09:42 -0400)
The bootstrap process cannot immediately quit if it notices the remote
image is not primary. Instead, it needs to continue if the local image is
still chained to the remote.

Fixes: http://tracker.ceph.com/issues/24009
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc
src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc
src/tools/rbd_mirror/image_replayer/BootstrapRequest.h
src/tools/rbd_mirror/image_replayer/IsPrimaryRequest.cc
src/tools/rbd_mirror/image_replayer/OpenLocalImageRequest.cc

index bd8fb6893c4e7d998883ba49764487ae2b0b25be..16ce1ad0ac1432c56ed7b217103f177f9a1a8e12 100644 (file)
@@ -523,6 +523,63 @@ TEST_F(TestMockImageReplayerBootstrapRequest, NonPrimaryRemoteSyncingState) {
   ASSERT_EQ(-EREMOTEIO, ctx.wait());
 }
 
+TEST_F(TestMockImageReplayerBootstrapRequest, NonPrimaryRemoteNotTagOwner) {
+  create_local_image();
+
+  InSequence seq;
+
+  // lookup remote image tag class
+  cls::journal::Client client;
+  librbd::journal::ClientData client_data{
+    librbd::journal::ImageClientMeta{123}};
+  encode(client_data, client.data);
+  ::journal::MockJournaler mock_journaler;
+  expect_journaler_get_client(mock_journaler,
+                              librbd::Journal<>::IMAGE_CLIENT_ID,
+                              client, 0);
+
+  // open the remote image
+  librbd::MockJournal mock_journal;
+  librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx);
+  MockOpenImageRequest mock_open_image_request;
+  expect_open_image(mock_open_image_request, m_remote_io_ctx,
+                    mock_remote_image_ctx.id, mock_remote_image_ctx, 0);
+
+  // test if remote image is primary
+  MockIsPrimaryRequest mock_is_primary_request;
+  expect_is_primary(mock_is_primary_request, false, 0);
+
+  // open the local image
+  librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx);
+  mock_local_image_ctx.journal = &mock_journal;
+  MockOpenLocalImageRequest mock_open_local_image_request;
+  expect_open_local_image(mock_open_local_image_request, m_local_io_ctx,
+                          mock_local_image_ctx.id, &mock_local_image_ctx, 0);
+  expect_is_resync_requested(mock_journal, false, 0);
+
+  expect_journal_get_tag_tid(mock_journal, 345);
+  expect_journal_get_tag_data(mock_journal, {librbd::Journal<>::LOCAL_MIRROR_UUID,
+                                             librbd::Journal<>::ORPHAN_MIRROR_UUID,
+                                             true, 344, 0});
+
+  MockCloseImageRequest mock_close_image_request;
+  expect_close_image(mock_close_image_request, mock_local_image_ctx, 0);
+  expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0);
+
+  C_SaferCond ctx;
+  MockInstanceWatcher mock_instance_watcher;
+  cls::journal::ClientState client_state = cls::journal::CLIENT_STATE_CONNECTED;
+  librbd::journal::MirrorPeerClientMeta mirror_peer_client_meta{
+    mock_local_image_ctx.id};
+  mirror_peer_client_meta.state = librbd::journal::MIRROR_PEER_STATE_REPLAYING;
+  MockBootstrapRequest *request = create_request(
+    &mock_instance_watcher, mock_journaler, mock_local_image_ctx.id,
+    mock_remote_image_ctx.id, "global image id", "local mirror uuid",
+    "remote mirror uuid", &client_state, &mirror_peer_client_meta, &ctx);
+  request->send();
+  ASSERT_EQ(-EREMOTEIO, ctx.wait());
+}
+
 TEST_F(TestMockImageReplayerBootstrapRequest, RemoteDemotePromote) {
   create_local_image();
 
@@ -547,7 +604,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, RemoteDemotePromote) {
 
   // test if remote image is primary
   MockIsPrimaryRequest mock_is_primary_request;
-  expect_is_primary(mock_is_primary_request, true, 0);
+  expect_is_primary(mock_is_primary_request, false, 0);
 
   // open the local image
   librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx);
@@ -557,6 +614,9 @@ TEST_F(TestMockImageReplayerBootstrapRequest, RemoteDemotePromote) {
                           mock_local_image_ctx.id, &mock_local_image_ctx, 0);
   expect_is_resync_requested(mock_journal, false, 0);
 
+  expect_journal_get_tag_tid(mock_journal, 345);
+  expect_journal_get_tag_data(mock_journal, {"remote mirror uuid"});
+
   // remote demotion / promotion event
   Tags tags = {
     {2, 123, encode_tag_data({librbd::Journal<>::LOCAL_MIRROR_UUID,
@@ -573,8 +633,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, RemoteDemotePromote) {
                               true, 4, 369})}
   };
   expect_journaler_get_tags(mock_journaler, 123, tags, 0);
-  expect_journal_get_tag_tid(mock_journal, 345);
-  expect_journal_get_tag_data(mock_journal, {"remote mirror uuid"});
 
   MockCloseImageRequest mock_close_image_request;
   expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0);
@@ -627,6 +685,10 @@ TEST_F(TestMockImageReplayerBootstrapRequest, MultipleRemoteDemotePromotes) {
                           mock_local_image_ctx.id, &mock_local_image_ctx, 0);
   expect_is_resync_requested(mock_journal, false, 0);
 
+  expect_journal_get_tag_tid(mock_journal, 345);
+  expect_journal_get_tag_data(mock_journal, {librbd::Journal<>::ORPHAN_MIRROR_UUID,
+                                             "remote mirror uuid", true, 4, 1});
+
   // remote demotion / promotion event
   Tags tags = {
     {2, 123, encode_tag_data({librbd::Journal<>::LOCAL_MIRROR_UUID,
@@ -652,9 +714,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, MultipleRemoteDemotePromotes) {
                               true, 7, 1})}
   };
   expect_journaler_get_tags(mock_journaler, 123, tags, 0);
-  expect_journal_get_tag_tid(mock_journal, 345);
-  expect_journal_get_tag_data(mock_journal, {librbd::Journal<>::ORPHAN_MIRROR_UUID,
-                                             "remote mirror uuid", true, 4, 1});
 
   MockCloseImageRequest mock_close_image_request;
   expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0);
@@ -707,6 +766,12 @@ TEST_F(TestMockImageReplayerBootstrapRequest, LocalDemoteRemotePromote) {
                           mock_local_image_ctx.id, &mock_local_image_ctx, 0);
   expect_is_resync_requested(mock_journal, false, 0);
 
+  expect_journal_get_tag_tid(mock_journal, 346);
+  expect_journal_get_tag_data(mock_journal,
+                              {librbd::Journal<>::ORPHAN_MIRROR_UUID,
+                               librbd::Journal<>::LOCAL_MIRROR_UUID,
+                               true, 345, 1});
+
   // remote demotion / promotion event
   Tags tags = {
     {2, 123, encode_tag_data({"local mirror uuid", "local mirror uuid",
@@ -718,11 +783,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, LocalDemoteRemotePromote) {
                               true, 3, 1})}
   };
   expect_journaler_get_tags(mock_journaler, 123, tags, 0);
-  expect_journal_get_tag_tid(mock_journal, 346);
-  expect_journal_get_tag_data(mock_journal,
-                              {librbd::Journal<>::ORPHAN_MIRROR_UUID,
-                               librbd::Journal<>::LOCAL_MIRROR_UUID,
-                               true, 345, 1});
 
   MockCloseImageRequest mock_close_image_request;
   expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0);
@@ -775,6 +835,11 @@ TEST_F(TestMockImageReplayerBootstrapRequest, SplitBrainForcePromote) {
                           mock_local_image_ctx.id, &mock_local_image_ctx, 0);
   expect_is_resync_requested(mock_journal, false, 0);
 
+  expect_journal_get_tag_tid(mock_journal, 345);
+  expect_journal_get_tag_data(mock_journal, {librbd::Journal<>::LOCAL_MIRROR_UUID,
+                                             librbd::Journal<>::ORPHAN_MIRROR_UUID,
+                                             true, 344, 0});
+
   // remote demotion / promotion event
   Tags tags = {
     {2, 123, encode_tag_data({librbd::Journal<>::LOCAL_MIRROR_UUID,
@@ -785,10 +850,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, SplitBrainForcePromote) {
                               true, 2, 1})}
   };
   expect_journaler_get_tags(mock_journaler, 123, tags, 0);
-  expect_journal_get_tag_tid(mock_journal, 345);
-  expect_journal_get_tag_data(mock_journal, {librbd::Journal<>::LOCAL_MIRROR_UUID,
-                                             librbd::Journal<>::ORPHAN_MIRROR_UUID,
-                                             true, 344, 0});
 
   MockCloseImageRequest mock_close_image_request;
   expect_close_image(mock_close_image_request, mock_local_image_ctx, 0);
@@ -845,6 +906,8 @@ TEST_F(TestMockImageReplayerBootstrapRequest, ResyncRequested) {
   // resync is requested
   expect_is_resync_requested(mock_journal, true, 0);
 
+  expect_journal_get_tag_tid(mock_journal, 345);
+  expect_journal_get_tag_data(mock_journal, {"remote mirror uuid"});
 
   MockCloseImageRequest mock_close_image_request;
   expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0);
@@ -914,6 +977,9 @@ TEST_F(TestMockImageReplayerBootstrapRequest, PrimaryRemote) {
                           mock_local_image_ctx.id, &mock_local_image_ctx, 0);
   expect_is_resync_requested(mock_journal, false, 0);
 
+  expect_journal_get_tag_tid(mock_journal, 345);
+  expect_journal_get_tag_data(mock_journal, {"remote mirror uuid"});
+
   // sync the remote image to the local image
   MockImageSync mock_image_sync;
   expect_image_sync(mock_image_sync, 0);
@@ -997,6 +1063,9 @@ TEST_F(TestMockImageReplayerBootstrapRequest, PrimaryRemoteLocalDeleted) {
                           mock_local_image_ctx.id, &mock_local_image_ctx, 0);
   expect_is_resync_requested(mock_journal, false, 0);
 
+  expect_journal_get_tag_tid(mock_journal, 345);
+  expect_journal_get_tag_data(mock_journal, {"remote mirror uuid"});
+
   // sync the remote image to the local image
   MockImageSync mock_image_sync;
   expect_image_sync(mock_image_sync, 0);
index e13652280fc28cf08571c6c5e3dce0459d1dbade..8417a005bf7dac1482b3031ea7b2a1e0ba6af3f0 100644 (file)
@@ -194,7 +194,12 @@ template <typename I>
 void BootstrapRequest<I>::handle_is_primary(int r) {
   dout(20) << ": r=" << r << dendl;
 
-  if (r < 0) {
+  if (r == -ENOENT) {
+    dout(5) << ": remote image is not mirrored" << dendl;
+    m_ret_val = -EREMOTEIO;
+    close_remote_image();
+    return;
+  } else if (r < 0) {
     derr << ": error querying remote image primary status: " << cpp_strerror(r)
          << dendl;
     m_ret_val = r;
@@ -203,11 +208,22 @@ void BootstrapRequest<I>::handle_is_primary(int r) {
   }
 
   if (!m_primary) {
-    dout(5) << ": remote image is not primary -- skipping image replay"
-            << dendl;
-    m_ret_val = -EREMOTEIO;
-    update_client_state();
-    return;
+    if (m_local_image_id.empty()) {
+      // no local image and remote isn't primary -- don't sync it
+      dout(5) << ": remote image is not primary -- not syncing"
+              << dendl;
+      m_ret_val = -EREMOTEIO;
+      close_remote_image();
+      return;
+    } else if (m_client_meta->state !=
+                 librbd::journal::MIRROR_PEER_STATE_REPLAYING) {
+      // ensure we attempt to re-sync to remote if it's re-promoted
+      dout(5) << ": remote image is not primary -- sync interrupted"
+              << dendl;
+      m_ret_val = -EREMOTEIO;
+      update_client_state();
+      return;
+    }
   }
 
   if (!m_client_meta->image_id.empty()) {
@@ -217,6 +233,7 @@ void BootstrapRequest<I>::handle_is_primary(int r) {
   }
 
   if (m_local_image_id.empty()) {
+    // prepare to create local image
     update_client_image();
     return;
   }
@@ -226,12 +243,6 @@ void BootstrapRequest<I>::handle_is_primary(int r) {
 
 template <typename I>
 void BootstrapRequest<I>::update_client_state() {
-  if (m_client_meta->state == librbd::journal::MIRROR_PEER_STATE_REPLAYING) {
-    // state already set for replaying upon failover
-    close_remote_image();
-    return;
-  }
-
   dout(20) << dendl;
   update_progress("UPDATE_CLIENT_STATE");
 
@@ -300,8 +311,10 @@ void BootstrapRequest<I>::handle_open_local_image(int r) {
 
   I *local_image_ctx = (*m_local_image_ctx);
   {
-    RWLock::RLocker snap_locker(local_image_ctx->snap_lock);
+    local_image_ctx->snap_lock.get_read();
     if (local_image_ctx->journal == nullptr) {
+      local_image_ctx->snap_lock.put_read();
+
       derr << ": local image does not support journaling" << dendl;
       m_ret_val = -EINVAL;
       close_local_image();
@@ -310,11 +323,30 @@ void BootstrapRequest<I>::handle_open_local_image(int r) {
 
     r = (*m_local_image_ctx)->journal->is_resync_requested(m_do_resync);
     if (r < 0) {
+      local_image_ctx->snap_lock.put_read();
+
       derr << ": failed to check if a resync was requested" << dendl;
       m_ret_val = r;
       close_local_image();
       return;
     }
+
+    m_local_tag_tid = local_image_ctx->journal->get_tag_tid();
+    m_local_tag_data = local_image_ctx->journal->get_tag_data();
+    dout(10) << ": local tag=" << m_local_tag_tid << ", "
+             << "local tag data=" << m_local_tag_data << dendl;
+    local_image_ctx->snap_lock.put_read();
+  }
+
+  if (m_local_tag_data.mirror_uuid != m_remote_mirror_uuid && !m_primary) {
+    // if the local mirror is not linked to the (now) non-primary image,
+    // stop the replay. Otherwise, we ignore that the remote is non-primary
+    // so that we can replay the demotion
+    dout(5) << ": remote image is not primary -- skipping image replay"
+            << dendl;
+    m_ret_val = -EREMOTEIO;
+    close_local_image();
+    return;
   }
 
   if (*m_do_resync) {
@@ -366,8 +398,8 @@ void BootstrapRequest<I>::register_client() {
 
   update_progress("REGISTER_CLIENT");
 
-  librbd::journal::MirrorPeerClientMeta mirror_peer_client_meta{
-    m_local_image_id};
+  assert(m_local_image_id.empty());
+  librbd::journal::MirrorPeerClientMeta mirror_peer_client_meta;
   mirror_peer_client_meta.state = librbd::journal::MIRROR_PEER_STATE_REPLAYING;
 
   librbd::journal::ClientData client_data{mirror_peer_client_meta};
@@ -393,7 +425,7 @@ void BootstrapRequest<I>::handle_register_client(int r) {
   }
 
   *m_client_state = cls::journal::CLIENT_STATE_CONNECTED;
-  *m_client_meta = librbd::journal::MirrorPeerClientMeta(m_local_image_id);
+  *m_client_meta = librbd::journal::MirrorPeerClientMeta();
   m_client_meta->state = librbd::journal::MIRROR_PEER_STATE_REPLAYING;
 
   is_primary();
@@ -517,24 +549,6 @@ void BootstrapRequest<I>::handle_get_remote_tags(int r) {
   // At this point, the local image was existing, non-primary, and replaying;
   // and the remote image is primary.  Attempt to link the local image's most
   // recent tag to the remote image's tag chain.
-  uint64_t local_tag_tid;
-  librbd::journal::TagData local_tag_data;
-  I *local_image_ctx = (*m_local_image_ctx);
-  {
-    RWLock::RLocker snap_locker(local_image_ctx->snap_lock);
-    if (local_image_ctx->journal == nullptr) {
-      derr << ": local image does not support journaling" << dendl;
-      m_ret_val = -EINVAL;
-      close_local_image();
-      return;
-    }
-
-    local_tag_tid = local_image_ctx->journal->get_tag_tid();
-    local_tag_data = local_image_ctx->journal->get_tag_data();
-    dout(10) << ": local tag " << local_tag_tid << ": "
-             << local_tag_data << dendl;
-  }
-
   bool remote_tag_data_valid = false;
   librbd::journal::TagData remote_tag_data;
   boost::optional<uint64_t> remote_orphan_tag_tid =
@@ -543,9 +557,9 @@ void BootstrapRequest<I>::handle_get_remote_tags(int r) {
 
   // decode the remote tags
   for (auto &remote_tag : m_remote_tags) {
-    if (local_tag_data.predecessor.commit_valid &&
-        local_tag_data.predecessor.mirror_uuid == m_remote_mirror_uuid &&
-        local_tag_data.predecessor.tag_tid > remote_tag.tid) {
+    if (m_local_tag_data.predecessor.commit_valid &&
+        m_local_tag_data.predecessor.mirror_uuid == m_remote_mirror_uuid &&
+        m_local_tag_data.predecessor.tag_tid > remote_tag.tid) {
       dout(15) << ": skipping processed predecessor remote tag "
                << remote_tag.tid << dendl;
       continue;
@@ -566,7 +580,7 @@ void BootstrapRequest<I>::handle_get_remote_tags(int r) {
     dout(10) << ": decoded remote tag " << remote_tag.tid << ": "
              << remote_tag_data << dendl;
 
-    if (!local_tag_data.predecessor.commit_valid) {
+    if (!m_local_tag_data.predecessor.commit_valid) {
       // newly synced local image (no predecessor) replays from the first tag
       if (remote_tag_data.mirror_uuid != librbd::Journal<>::LOCAL_MIRROR_UUID) {
         dout(15) << ": skipping non-primary remote tag" << dendl;
@@ -577,17 +591,17 @@ void BootstrapRequest<I>::handle_get_remote_tags(int r) {
       break;
     }
 
-    if (local_tag_data.mirror_uuid == librbd::Journal<>::ORPHAN_MIRROR_UUID) {
+    if (m_local_tag_data.mirror_uuid == librbd::Journal<>::ORPHAN_MIRROR_UUID) {
       // demotion last available local epoch
 
-      if (remote_tag_data.mirror_uuid == local_tag_data.mirror_uuid &&
+      if (remote_tag_data.mirror_uuid == m_local_tag_data.mirror_uuid &&
           remote_tag_data.predecessor.commit_valid &&
           remote_tag_data.predecessor.tag_tid ==
-            local_tag_data.predecessor.tag_tid) {
+            m_local_tag_data.predecessor.tag_tid) {
         // demotion matches remote epoch
 
         if (remote_tag_data.predecessor.mirror_uuid == m_local_mirror_uuid &&
-            local_tag_data.predecessor.mirror_uuid ==
+            m_local_tag_data.predecessor.mirror_uuid ==
               librbd::Journal<>::LOCAL_MIRROR_UUID) {
           // local demoted and remote has matching event
           dout(15) << ": found matching local demotion tag" << dendl;
@@ -595,7 +609,7 @@ void BootstrapRequest<I>::handle_get_remote_tags(int r) {
           continue;
         }
 
-        if (local_tag_data.predecessor.mirror_uuid == m_remote_mirror_uuid &&
+        if (m_local_tag_data.predecessor.mirror_uuid == m_remote_mirror_uuid &&
             remote_tag_data.predecessor.mirror_uuid ==
               librbd::Journal<>::LOCAL_MIRROR_UUID) {
           // remote demoted and local has matching event
@@ -621,7 +635,7 @@ void BootstrapRequest<I>::handle_get_remote_tags(int r) {
   }
 
   if (remote_tag_data_valid &&
-      local_tag_data.mirror_uuid == m_remote_mirror_uuid) {
+      m_local_tag_data.mirror_uuid == m_remote_mirror_uuid) {
     dout(10) << ": local image is in clean replay state" << dendl;
   } else if (reconnect_orphan) {
     dout(10) << ": remote image was demoted/promoted" << dendl;
index 8a46e7038d6d043b53146ccd3dcfd73f36a524c1..a28789b9f8fb8c5a6b2e0837747f78ff32c39cf7 100644 (file)
@@ -8,6 +8,7 @@
 #include "include/rados/librados.hpp"
 #include "common/Mutex.h"
 #include "cls/journal/cls_journal_types.h"
+#include "librbd/journal/Types.h"
 #include "librbd/journal/TypeTraits.h"
 #include "tools/rbd_mirror/BaseRequest.h"
 #include "tools/rbd_mirror/types.h"
@@ -174,6 +175,9 @@ private:
   int m_ret_val = 0;
   ImageSync<ImageCtxT> *m_image_sync = nullptr;
 
+  uint64_t m_local_tag_tid = 0;
+  librbd::journal::TagData m_local_tag_data;
+
   bufferlist m_out_bl;
 
   void get_remote_tag_class();
index ef2008b99947e8f6b481f5fb211e89576fc420ce..e728e24617171137ca2f1c12ccc352dec037e1ca 100644 (file)
@@ -64,7 +64,7 @@ void IsPrimaryRequest<I>::handle_get_mirror_state(int r) {
         return;
       } else if (mirror_image.state == cls::rbd::MIRROR_IMAGE_STATE_DISABLING) {
         dout(5) << ": image mirroring is being disabled" << dendl;
-        *m_primary = false;
+        r = -ENOENT;
       } else {
         derr << ": image mirroring is disabled" << dendl;
         r = -EINVAL;
@@ -73,6 +73,8 @@ void IsPrimaryRequest<I>::handle_get_mirror_state(int r) {
       derr << ": failed to decode image mirror state: " << cpp_strerror(r)
            << dendl;
     }
+  } else if (r == -ENOENT) {
+    dout(5) << ": image is not mirrored" << dendl;
   } else {
     derr << ": failed to retrieve image mirror state: " << cpp_strerror(r)
          << dendl;
index 1894905fca131af22c2ecb400ed666cfb9edb20a..aca3f04b8757dfc18c3404d866b1a9900dc6b1f6 100644 (file)
@@ -157,7 +157,11 @@ template <typename I>
 void OpenLocalImageRequest<I>::handle_is_primary(int r) {
   dout(20) << ": r=" << r << dendl;
 
-  if (r < 0) {
+  if (r == -ENOENT) {
+    dout(5) << ": local image is not mirrored" << dendl;
+    send_close_image(r);
+    return;
+  } else if (r < 0) {
     derr << ": error querying local image primary status: " << cpp_strerror(r)
          << dendl;
     send_close_image(r);