]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph.git/commitdiff
rbd-mirror: ensure remote demotion is replayed locally 22142/head
authorJason Dillaman <dillaman@redhat.com>
Fri, 4 May 2018 15:14:34 +0000 (11:14 -0400)
committerPrashant D <pdhange@redhat.com>
Tue, 22 May 2018 01:10:41 +0000 (21:10 -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>
(cherry picked from commit d35bc94319c6fa267e973e533f4591568289ee96)

Conflicts:
src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc : Resolved in
handle_get_remote_tags

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 3d4c16a820d55e43d74d3952032976a87a394b4a..cde2df07bdbabfaf4b7ea761ffab79c0e0f05f40 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 2c24ef1f7d5e07e27831088f44dc3147de1a7940..cd2d9b4ab2709fdf5e5f177f79103f7f1b5f0d5c 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();
@@ -513,24 +545,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(20) << ": 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 =
@@ -539,10 +553,10 @@ 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) {
-      dout(20) << ": skipping processed predecessor remote tag "
+    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;
     }
@@ -562,7 +576,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(20) << ": skipping non-primary remote tag" << dendl;
@@ -573,17 +587,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(20) << ": found matching local demotion tag" << dendl;
@@ -591,7 +605,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
@@ -617,8 +631,8 @@ void BootstrapRequest<I>::handle_get_remote_tags(int r) {
   }
 
   if (remote_tag_data_valid &&
-      local_tag_data.mirror_uuid == m_remote_mirror_uuid) {
-    dout(20) << ": local image is in clean replay state" << dendl;
+      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(20) << ": remote image was demoted/promoted" << dendl;
   } else {
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 f2faa9f20f21f16112a8eefa029396d788b2199c..4c768b965594fd6d53c6db56843ab1f06426c1d7 100644 (file)
@@ -63,7 +63,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;
@@ -72,6 +72,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 a54216a8328cab410591fa983bb9111babb7dd57..9a2d6683c9d5eea9259d75aeee93e92c402cd9da 100644 (file)
@@ -152,7 +152,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);