rbd-mirror: prevent image health status from ping-ponging during re-attempt

Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc b/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc
index 19580620eccac1046a3e1a1fa81b8cd49815a740..c7b995387e1d3b8e8f0dcf9d3064c028fb7ba008 100644 (file)
--- a/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc
+++ b/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc
@@ -245,7 +245,6 @@ OpenLocalImageRequest<librbd::MockTestImageCtx>*
 
 // template definitions
 #include "tools/rbd_mirror/image_replayer/BootstrapRequest.cc"
-template class rbd::mirror::image_replayer::BootstrapRequest<librbd::MockTestImageCtx>;
 
 namespace rbd {
 namespace mirror {

diff --git a/src/test/rbd_mirror/test_mock_ImageReplayer.cc b/src/test/rbd_mirror/test_mock_ImageReplayer.cc
index 7cff043ba7ad323bd3e0b4d86d174cef30eb9603..aa514db923abffee94680075754eb1d213512755 100644 (file)
--- a/src/test/rbd_mirror/test_mock_ImageReplayer.cc
+++ b/src/test/rbd_mirror/test_mock_ImageReplayer.cc
@@ -152,6 +152,10 @@ struct BootstrapRequest<librbd::MockTestImageCtx> {
   void get() {
   }
 
+  inline bool is_syncing() const {
+    return false;
+  }
+
   MOCK_METHOD0(send, void());
   MOCK_METHOD0(cancel, void());
 };
@@ -500,6 +504,8 @@ TEST_F(TestMockImageReplayer, StartStop) {
   C_SaferCond start_ctx;
   m_image_replayer->start(&start_ctx);
   ASSERT_EQ(0, start_ctx.wait());
+  ASSERT_EQ(image_replayer::HEALTH_STATE_OK,
+            m_image_replayer->get_health_state());
 
   // STOP
 
@@ -517,6 +523,8 @@ TEST_F(TestMockImageReplayer, StartStop) {
   C_SaferCond stop_ctx;
   m_image_replayer->stop(&stop_ctx);
   ASSERT_EQ(0, stop_ctx.wait());
+  ASSERT_EQ(image_replayer::HEALTH_STATE_OK,
+            m_image_replayer->get_health_state());
 }
 
 TEST_F(TestMockImageReplayer, LocalImagePrimary) {
@@ -663,6 +671,8 @@ TEST_F(TestMockImageReplayer, StartExternalReplayError) {
   C_SaferCond start_ctx;
   m_image_replayer->start(&start_ctx);
   ASSERT_EQ(-EINVAL, start_ctx.wait());
+  ASSERT_EQ(image_replayer::HEALTH_STATE_ERROR,
+            m_image_replayer->get_health_state());
 }
 
 TEST_F(TestMockImageReplayer, StopError) {

diff --git a/src/tools/rbd_mirror/ImageReplayer.cc b/src/tools/rbd_mirror/ImageReplayer.cc
index fb3cb89a83ac1b81d69842ceffa8d3b2e806dbab..37bc528cefe35bc6fed764dca79f9d82fd11ef9c 100644 (file)
--- a/src/tools/rbd_mirror/ImageReplayer.cc
+++ b/src/tools/rbd_mirror/ImageReplayer.cc
@@ -326,6 +326,19 @@ ImageReplayer<I>::~ImageReplayer()
   delete m_asok_hook;
 }
 
+template <typename I>
+image_replayer::HealthState ImageReplayer<I>::get_health_state() const {
+  Mutex::Locker locker(m_lock);
+
+  if (!m_mirror_image_status_state) {
+    return image_replayer::HEALTH_STATE_OK;
+  } else if (*m_mirror_image_status_state ==
+      cls::rbd::MIRROR_IMAGE_STATUS_STATE_SYNCING) {
+    return image_replayer::HEALTH_STATE_WARNING;
+  }
+  return image_replayer::HEALTH_STATE_ERROR;
+}
+
 template <typename I>
 void ImageReplayer<I>::add_remote_image(const std::string &mirror_uuid,
                                         const std::string &image_id,
@@ -1302,15 +1315,30 @@ void ImageReplayer<I>::send_mirror_status_update(const OptionalState &opt_state)
   State state;
   std::string state_desc;
   int last_r;
-  bool bootstrapping;
   bool stopping_replay;
+
+  OptionalMirrorImageStatusState mirror_image_status_state{
+    boost::make_optional(false, cls::rbd::MirrorImageStatusState{})};
+  image_replayer::BootstrapRequest<I>* bootstrap_request = nullptr;
   {
     Mutex::Locker locker(m_lock);
     state = m_state;
     state_desc = m_state_desc;
+    mirror_image_status_state = m_mirror_image_status_state;
     last_r = m_last_r;
-    bootstrapping = (m_bootstrap_request != nullptr);
     stopping_replay = (m_local_image_ctx != nullptr);
+
+    if (m_bootstrap_request != nullptr) {
+      bootstrap_request = m_bootstrap_request;
+      bootstrap_request->get();
+    }
+  }
+
+  bool syncing = false;
+  if (bootstrap_request != nullptr) {
+    syncing = bootstrap_request->is_syncing();
+    bootstrap_request->put();
+    bootstrap_request = nullptr;
   }
 
   if (opt_state) {
@@ -1321,9 +1349,10 @@ void ImageReplayer<I>::send_mirror_status_update(const OptionalState &opt_state)
   status.up = true;
   switch (state) {
   case STATE_STARTING:
-    if (bootstrapping) {
+    if (syncing) {
       status.state = cls::rbd::MIRROR_IMAGE_STATUS_STATE_SYNCING;
       status.description = state_desc.empty() ? "syncing" : state_desc;
+      mirror_image_status_state = status.state;
     } else {
       status.state = cls::rbd::MIRROR_IMAGE_STATUS_STATE_STARTING_REPLAY;
       status.description = "starting replay";
@@ -1351,6 +1380,7 @@ void ImageReplayer<I>::send_mirror_status_update(const OptionalState &opt_state)
         return;
       }
       status.description = "replaying, " + desc;
+      mirror_image_status_state = boost::none;
     }
     break;
   case STATE_STOPPING:
@@ -1364,15 +1394,28 @@ void ImageReplayer<I>::send_mirror_status_update(const OptionalState &opt_state)
     if (last_r < 0) {
       status.state = cls::rbd::MIRROR_IMAGE_STATUS_STATE_ERROR;
       status.description = state_desc;
+      mirror_image_status_state = status.state;
     } else {
       status.state = cls::rbd::MIRROR_IMAGE_STATUS_STATE_STOPPED;
       status.description = state_desc.empty() ? "stopped" : state_desc;
+      mirror_image_status_state = boost::none;
     }
     break;
   default:
     assert(!"invalid state");
   }
 
+  {
+    Mutex::Locker locker(m_lock);
+    m_mirror_image_status_state = mirror_image_status_state;
+  }
+
+  // prevent the status from ping-ponging when failed replays are restarted
+  if (mirror_image_status_state &&
+      *mirror_image_status_state == cls::rbd::MIRROR_IMAGE_STATUS_STATE_ERROR) {
+    status.state = *mirror_image_status_state;
+  }
+
   dout(20) << "status=" << status << dendl;
   librados::ObjectWriteOperation op;
   librbd::cls_client::mirror_image_status_set(&op, m_global_image_id, status);

diff --git a/src/tools/rbd_mirror/ImageReplayer.h b/src/tools/rbd_mirror/ImageReplayer.h
index 1e3eca583954b00fc5c8cad3f8b2c5d776196cc0..3ea41dc5378f50abc88d9c7b26272b27ac974811 100644 (file)
--- a/src/tools/rbd_mirror/ImageReplayer.h
+++ b/src/tools/rbd_mirror/ImageReplayer.h
@@ -18,6 +18,7 @@
 #include "ImageDeleter.h"
 #include "ProgressContext.h"
 #include "types.h"
+#include "tools/rbd_mirror/image_replayer/Types.h"
 
 #include <boost/noncopyable.hpp>
 #include <boost/optional.hpp>
@@ -60,17 +61,6 @@ namespace image_replayer { template <typename> class ReplayStatusFormatter; }
 template <typename ImageCtxT = librbd::ImageCtx>
 class ImageReplayer {
 public:
-  typedef typename librbd::journal::TypeTraits<ImageCtxT>::ReplayEntry ReplayEntry;
-
-  enum State {
-    STATE_UNKNOWN,
-    STATE_STARTING,
-    STATE_REPLAYING,
-    STATE_REPLAY_FLUSHING,
-    STATE_STOPPING,
-    STATE_STOPPED,
-  };
-
   static ImageReplayer *create(
     Threads<librbd::ImageCtx> *threads, ImageDeleter<ImageCtxT>* image_deleter,
     InstanceWatcher<ImageCtxT> *instance_watcher,
@@ -93,7 +83,6 @@ public:
   ImageReplayer(const ImageReplayer&) = delete;
   ImageReplayer& operator=(const ImageReplayer&) = delete;
 
-  State get_state() { Mutex::Locker l(m_lock); return get_state_(); }
   bool is_stopped() { Mutex::Locker l(m_lock); return is_stopped_(); }
   bool is_running() { Mutex::Locker l(m_lock); return is_running_(); }
   bool is_replaying() { Mutex::Locker l(m_lock); return is_replaying_(); }
@@ -106,6 +95,8 @@ public:
     return (m_last_r == -EBLACKLISTED);
   }
 
+  image_replayer::HealthState get_health_state() const;
+
   void add_remote_image(const std::string &remote_mirror_uuid,
                         const std::string &remote_image_id,
                         librados::IoCtx &remote_io_ctx);
@@ -215,6 +206,17 @@ protected:
   bool on_replay_interrupted();
 
 private:
+  typedef typename librbd::journal::TypeTraits<ImageCtxT>::ReplayEntry ReplayEntry;
+
+  enum State {
+    STATE_UNKNOWN,
+    STATE_STARTING,
+    STATE_REPLAYING,
+    STATE_REPLAY_FLUSHING,
+    STATE_STOPPING,
+    STATE_STOPPED,
+  };
+
   struct RemoteImage {
     std::string mirror_uuid;
     std::string image_id;
@@ -248,6 +250,8 @@ private:
 
   typedef typename librbd::journal::TypeTraits<ImageCtxT>::Journaler Journaler;
   typedef boost::optional<State> OptionalState;
+  typedef boost::optional<cls::rbd::MirrorImageStatusState>
+      OptionalMirrorImageStatusState;
 
   struct JournalListener : public librbd::journal::Listener {
     ImageReplayer *img_replayer;
@@ -296,8 +300,11 @@ private:
   std::string m_name;
   mutable Mutex m_lock;
   State m_state = STATE_STOPPED;
-  int m_last_r = 0;
   std::string m_state_desc;
+
+  OptionalMirrorImageStatusState m_mirror_image_status_state = boost::none;
+  int m_last_r = 0;
+
   BootstrapProgressContext m_progress_cxt;
   bool m_do_resync{false};
   image_replayer::EventPreprocessor<ImageCtxT> *m_event_preprocessor = nullptr;
@@ -364,9 +371,6 @@ private:
 
   static std::string to_string(const State state);
 
-  State get_state_() const {
-    return m_state;
-  }
   bool is_stopped_() const {
     return m_state == STATE_STOPPED;
   }

diff --git a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc
index 4dadc744f6c9e6ef777a9805cad862cb76fb0131..434aa783c05055bf32c6773ef527b976a97d8abc 100644 (file)
--- a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc
+++ b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc
@@ -74,6 +74,12 @@ BootstrapRequest<I>::~BootstrapRequest() {
   assert(m_remote_image_ctx == nullptr);
 }
 
+template <typename I>
+bool BootstrapRequest<I>::is_syncing() const {
+  Mutex::Locker locker(m_lock);
+  return (m_image_sync != nullptr);
+}
+
 template <typename I>
 void BootstrapRequest<I>::send() {
   *m_do_resync = false;
@@ -637,24 +643,26 @@ void BootstrapRequest<I>::image_sync() {
   }
 
   dout(20) << dendl;
-  update_progress("IMAGE_SYNC");
-
-  Context *ctx = create_context_callback<
-    BootstrapRequest<I>, &BootstrapRequest<I>::handle_image_sync>(
-      this);
-
   {
     Mutex::Locker locker(m_lock);
     if (m_canceled) {
       m_ret_val = -ECANCELED;
     } else {
       assert(m_image_sync == nullptr);
+
+      Context *ctx = create_context_callback<
+        BootstrapRequest<I>, &BootstrapRequest<I>::handle_image_sync>(this);
       m_image_sync = ImageSync<I>::create(
           *m_local_image_ctx, m_remote_image_ctx, m_timer, m_timer_lock,
           m_local_mirror_uuid, m_journaler, m_client_meta, m_work_queue,
           m_instance_watcher, ctx, m_progress_ctx);
 
       m_image_sync->get();
+
+      m_lock.Unlock();
+      update_progress("IMAGE_SYNC");
+      m_lock.Lock();
+
       m_image_sync->send();
       return;
     }
@@ -670,7 +678,6 @@ void BootstrapRequest<I>::handle_image_sync(int r) {
 
   {
     Mutex::Locker locker(m_lock);
-
     m_image_sync->put();
     m_image_sync = nullptr;
 

diff --git a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h
index 5696cce61fdd1e5de517f8b196a15777f9a94b7f..cebd568972ba88efaf9c21ed248d30646d54e6d8 100644 (file)
--- a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h
+++ b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h
@@ -80,6 +80,8 @@ public:
                    bool *do_resync, ProgressContext *progress_ctx = nullptr);
   ~BootstrapRequest() override;
 
+  bool is_syncing() const;
+
   void send() override;
   void cancel() override;
 
@@ -163,7 +165,7 @@ private:
   ProgressContext *m_progress_ctx;
   bool *m_do_resync;
 
-  Mutex m_lock;
+  mutable Mutex m_lock;
   bool m_canceled = false;
 
   Tags m_remote_tags;

diff --git a/src/tools/rbd_mirror/image_replayer/Types.h b/src/tools/rbd_mirror/image_replayer/Types.h
new file mode 100644 (file)
index 0000000..6ab988a
--- /dev/null
+++ b/src/tools/rbd_mirror/image_replayer/Types.h
@@ -0,0 +1,21 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+
+#ifndef CEPH_RBD_MIRROR_IMAGE_REPLAYER_TYPES_H
+#define CEPH_RBD_MIRROR_IMAGE_REPLAYER_TYPES_H
+
+namespace rbd {
+namespace mirror {
+namespace image_replayer {
+
+enum HealthState {
+  HEALTH_STATE_OK,
+  HEALTH_STATE_WARNING,
+  HEALTH_STATE_ERROR
+};
+
+} // namespace image_replayer
+} // namespace mirror
+} // namespace rbd
+
+#endif // CEPH_RBD_MIRROR_IMAGE_REPLAYER_TYPES_H