]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
rbd-mirror: synchronize with in-flight stop in ImageReplayer::stop()
authorIlya Dryomov <idryomov@gmail.com>
Sun, 20 Feb 2022 16:33:08 +0000 (17:33 +0100)
committerPonnuvel Palaniyappan <pponnuvel@gmail.com>
Wed, 11 May 2022 06:18:12 +0000 (07:18 +0100)
Complete on_finish right away only if the replayer is stopped (meaning
that it is legible to be restarted immediately, possibly from on_finish
itself).  This is the behaviour pretty much anyone would assume and
also what ImageReplayer::restart() relies on.

Fixes: https://tracker.ceph.com/issues/54344
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit 8965a0f2a6f7bdbe732be94b1ee269cab5be0a2a)

src/test/rbd_mirror/test_mock_ImageReplayer.cc
src/tools/rbd_mirror/ImageReplayer.cc

index 9f40797dd4f4c95cde09723b2d9f05abc6fb3e0a..c19d4923ab9626b5b5ebe705df67e91ce043ed68 100644 (file)
@@ -840,5 +840,109 @@ TEST_F(TestMockImageReplayer, ReplayerRenamed) {
   ASSERT_EQ(image_spec, m_image_replayer->get_name());
 }
 
+TEST_F(TestMockImageReplayer, StopJoinInterruptedReplayer) {
+  // START
+  create_local_image();
+  librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx);
+
+  MockThreads mock_threads(m_threads);
+  expect_work_queue_repeatedly(mock_threads);
+  expect_add_event_after_repeatedly(mock_threads);
+
+  MockReplayer mock_replayer;
+  expect_get_replay_status(mock_replayer);
+  expect_set_mirror_image_status_repeatedly();
+
+  InSequence seq;
+  MockBootstrapRequest mock_bootstrap_request;
+  MockStateBuilder mock_state_builder;
+  expect_send(mock_bootstrap_request, mock_state_builder, mock_local_image_ctx,
+              false, false, 0);
+
+  expect_create_replayer(mock_state_builder, mock_replayer);
+  expect_init(mock_replayer, 0);
+
+  create_image_replayer(mock_threads);
+
+  C_SaferCond start_ctx;
+  m_image_replayer->start(&start_ctx);
+  ASSERT_EQ(0, start_ctx.wait());
+
+  // NOTIFY
+  EXPECT_CALL(mock_replayer, is_resync_requested())
+    .WillOnce(Return(false));
+  EXPECT_CALL(mock_replayer, is_replaying())
+    .WillOnce(Return(false));
+  EXPECT_CALL(mock_replayer, get_error_code())
+    .WillOnce(Return(-EINVAL));
+  EXPECT_CALL(mock_replayer, get_error_description())
+    .WillOnce(Return("INVALID"));
+  const double DELAY = 10;
+  EXPECT_CALL(mock_replayer, shut_down(_))
+    .WillOnce(Invoke([this, DELAY](Context* ctx) {
+               m_threads->timer->add_event_after(DELAY, ctx);
+              }));
+  EXPECT_CALL(mock_replayer, destroy());
+  expect_close(mock_state_builder, 0);
+  expect_mirror_image_status_exists(false);
+
+  mock_replayer.replayer_listener->handle_notification();
+  ASSERT_FALSE(m_image_replayer->is_running());
+
+  C_SaferCond stop_ctx;
+  m_image_replayer->stop(&stop_ctx);
+  ASSERT_EQ(ETIMEDOUT, stop_ctx.wait_for(DELAY * 3 / 4));
+  ASSERT_EQ(0, stop_ctx.wait_for(DELAY));
+}
+
+TEST_F(TestMockImageReplayer, StopJoinRequestedStop) {
+  // START
+  create_local_image();
+  librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx);
+
+  MockThreads mock_threads(m_threads);
+  expect_work_queue_repeatedly(mock_threads);
+  expect_add_event_after_repeatedly(mock_threads);
+
+  MockReplayer mock_replayer;
+  expect_get_replay_status(mock_replayer);
+  expect_set_mirror_image_status_repeatedly();
+
+  InSequence seq;
+  MockBootstrapRequest mock_bootstrap_request;
+  MockStateBuilder mock_state_builder;
+  expect_send(mock_bootstrap_request, mock_state_builder, mock_local_image_ctx,
+              false, false, 0);
+
+  expect_create_replayer(mock_state_builder, mock_replayer);
+  expect_init(mock_replayer, 0);
+
+  create_image_replayer(mock_threads);
+
+  C_SaferCond start_ctx;
+  m_image_replayer->start(&start_ctx);
+  ASSERT_EQ(0, start_ctx.wait());
+
+  // STOP
+  const double DELAY = 10;
+  EXPECT_CALL(mock_replayer, shut_down(_))
+    .WillOnce(Invoke([this, DELAY](Context* ctx) {
+               m_threads->timer->add_event_after(DELAY, ctx);
+              }));
+  EXPECT_CALL(mock_replayer, destroy());
+  expect_close(mock_state_builder, 0);
+  expect_mirror_image_status_exists(false);
+
+  C_SaferCond stop_ctx1;
+  m_image_replayer->stop(&stop_ctx1);
+
+  C_SaferCond stop_ctx2;
+  m_image_replayer->stop(&stop_ctx2);
+  ASSERT_EQ(ETIMEDOUT, stop_ctx2.wait_for(DELAY * 3 / 4));
+  ASSERT_EQ(0, stop_ctx2.wait_for(DELAY));
+
+  ASSERT_EQ(0, stop_ctx1.wait_for(0));
+}
+
 } // namespace mirror
 } // namespace rbd
index 3d643f8aa69c8161cfe7bfb45137d876342a9c09..191e714bde6e5714dc923b8c6a2f8a0d79a072b3 100644 (file)
@@ -522,12 +522,11 @@ void ImageReplayer<I>::stop(Context *on_finish, bool manual, bool restart)
 
   image_replayer::BootstrapRequest<I> *bootstrap_request = nullptr;
   bool shut_down_replay = false;
-  bool running = true;
+  bool is_stopped = false;
   {
     std::lock_guard locker{m_lock};
 
     if (!is_running_()) {
-      running = false;
       if (manual && !m_manual_stop) {
         dout(10) << "marking manual" << dendl;
         m_manual_stop = true;
@@ -536,6 +535,15 @@ void ImageReplayer<I>::stop(Context *on_finish, bool manual, bool restart)
         dout(10) << "canceling restart" << dendl;
         m_restart_requested = false;
       }
+      if (is_stopped_()) {
+        dout(10) << "already stopped" << dendl;
+        is_stopped = true;
+      } else {
+        dout(10) << "joining in-flight stop" << dendl;
+        if (on_finish != nullptr) {
+          m_on_stop_contexts.push_back(on_finish);
+        }
+      }
     } else {
       if (m_state == STATE_STARTING) {
         dout(10) << "canceling start" << dendl;
@@ -557,6 +565,13 @@ void ImageReplayer<I>::stop(Context *on_finish, bool manual, bool restart)
     }
   }
 
+  if (is_stopped) {
+    if (on_finish) {
+      on_finish->complete(-EINVAL);
+    }
+    return;
+  }
+
   // avoid holding lock since bootstrap request will update status
   if (bootstrap_request != nullptr) {
     dout(10) << "canceling bootstrap" << dendl;
@@ -564,14 +579,6 @@ void ImageReplayer<I>::stop(Context *on_finish, bool manual, bool restart)
     bootstrap_request->put();
   }
 
-  if (!running) {
-    dout(20) << "not running" << dendl;
-    if (on_finish) {
-      on_finish->complete(-EINVAL);
-    }
-    return;
-  }
-
   if (shut_down_replay) {
     on_stop_journal_replay();
   }