librbd: only cancel replay of journal op events upon error

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

diff --git a/src/librbd/Journal.cc b/src/librbd/Journal.cc
index 58a76d1ab04c8b37bb1000453aac04bbbecd78cf..c07ba02b9f7d1d239cd854d044ce9aca2fdf2310 100644 (file)
--- a/src/librbd/Journal.cc
+++ b/src/librbd/Journal.cc
@@ -687,13 +687,13 @@ void Journal<I>::handle_replay_complete(int r) {
     transition_state(STATE_FLUSHING_RESTART, r);
     m_lock.Unlock();
 
-    m_journal_replay->shut_down(create_context_callback<
+    m_journal_replay->shut_down(true, create_context_callback<
       Journal<I>, &Journal<I>::handle_flushing_restart>(this));
   } else {
     transition_state(STATE_FLUSHING_REPLAY, 0);
     m_lock.Unlock();
 
-    m_journal_replay->shut_down(create_context_callback<
+    m_journal_replay->shut_down(false, create_context_callback<
       Journal<I>, &Journal<I>::handle_flushing_replay>(this));
   }
 }
@@ -723,7 +723,7 @@ void Journal<I>::handle_replay_process_safe(ReplayEntry replay_entry, int r) {
       m_journaler->stop_replay();
       transition_state(STATE_FLUSHING_RESTART, r);
 
-      m_journal_replay->shut_down(create_context_callback<
+      m_journal_replay->shut_down(true, create_context_callback<
         Journal<I>, &Journal<I>::handle_flushing_restart>(this));
       return;
     } else if (m_state == STATE_FLUSHING_REPLAY) {

diff --git a/src/librbd/journal/Replay.cc b/src/librbd/journal/Replay.cc
index 4b6daf19ab280558d46c02876f61da85d42bb2ae..fa53a86e3e56541fd9ff3f7f35e68fd371d4ff24 100644 (file)
--- a/src/librbd/journal/Replay.cc
+++ b/src/librbd/journal/Replay.cc
@@ -94,7 +94,7 @@ void Replay<I>::process(bufferlist::iterator *it, Context *on_ready,
 }
 
 template <typename I>
-void Replay<I>::shut_down(Context *on_finish) {
+void Replay<I>::shut_down(bool cancel_ops, Context *on_finish) {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 20) << this << " " << __func__ << dendl;
 
@@ -112,10 +112,12 @@ void Replay<I>::shut_down(Context *on_finish) {
     }
 
     // cancel ops that are waiting to start
-    for (auto &op_event_pair : m_op_events) {
-      const OpEvent &op_event = op_event_pair.second;
-      if (op_event.on_start_ready == nullptr) {
-        cancel_op_tids.push_back(op_event_pair.first);
+    if (cancel_ops) {
+      for (auto &op_event_pair : m_op_events) {
+        const OpEvent &op_event = op_event_pair.second;
+        if (op_event.on_start_ready == nullptr) {
+          cancel_op_tids.push_back(op_event_pair.first);
+        }
       }
     }
 

diff --git a/src/librbd/journal/Replay.h b/src/librbd/journal/Replay.h
index d461090092e3738b8114c6ded8ffeb7713cd7c4c..d842a03ff5b41d4eaff4abf79b1f526fe5c9665f 100644 (file)
--- a/src/librbd/journal/Replay.h
+++ b/src/librbd/journal/Replay.h
@@ -35,7 +35,7 @@ public:
 
   void process(bufferlist::iterator *it, Context *on_ready, Context *on_safe);
 
-  void shut_down(Context *on_finish);
+  void shut_down(bool cancel_ops, Context *on_finish);
   void flush(Context *on_finish);
 
   void replay_op_ready(uint64_t op_tid, Context *on_resume);

diff --git a/src/test/librbd/journal/test_mock_Replay.cc b/src/test/librbd/journal/test_mock_Replay.cc
index 9ec20ffc72878d37f0344a1d3dd4c34c582b20b9..033ef3c5f9793d3279a8713d2c26b918d2c19890 100644 (file)
--- a/src/test/librbd/journal/test_mock_Replay.cc
+++ b/src/test/librbd/journal/test_mock_Replay.cc
@@ -208,9 +208,9 @@ public:
     return ctx.wait();
   }
 
-  int when_shut_down(MockJournalReplay &mock_journal_replay) {
+  int when_shut_down(MockJournalReplay &mock_journal_replay, bool cancel_ops) {
     C_SaferCond ctx;
-    mock_journal_replay.shut_down(&ctx);
+    mock_journal_replay.shut_down(cancel_ops, &ctx);
     return ctx.wait();
   }
 
@@ -263,7 +263,7 @@ TEST_F(TestMockJournalReplay, AioDiscard) {
   ASSERT_EQ(0, on_ready.wait());
 
   expect_aio_flush(mock_image_ctx, mock_aio_image_request, 0);
-  ASSERT_EQ(0, when_shut_down(mock_journal_replay));
+  ASSERT_EQ(0, when_shut_down(mock_journal_replay, false));
   ASSERT_EQ(0, on_safe.wait());
 }
 
@@ -291,7 +291,7 @@ TEST_F(TestMockJournalReplay, AioWrite) {
   ASSERT_EQ(0, on_ready.wait());
 
   expect_aio_flush(mock_image_ctx, mock_aio_image_request, 0);
-  ASSERT_EQ(0, when_shut_down(mock_journal_replay));
+  ASSERT_EQ(0, when_shut_down(mock_journal_replay, false));
   ASSERT_EQ(0, on_safe.wait());
 }
 
@@ -317,7 +317,7 @@ TEST_F(TestMockJournalReplay, AioFlush) {
   when_complete(mock_image_ctx, aio_comp, 0);
   ASSERT_EQ(0, on_safe.wait());
 
-  ASSERT_EQ(0, when_shut_down(mock_journal_replay));
+  ASSERT_EQ(0, when_shut_down(mock_journal_replay, false));
   ASSERT_EQ(0, on_ready.wait());
 }
 
@@ -345,7 +345,7 @@ TEST_F(TestMockJournalReplay, IOError) {
   ASSERT_EQ(-EINVAL, on_safe.wait());
 
   expect_aio_flush(mock_image_ctx, mock_aio_image_request, 0);
-  ASSERT_EQ(0, when_shut_down(mock_journal_replay));
+  ASSERT_EQ(0, when_shut_down(mock_journal_replay, false));
   ASSERT_EQ(0, on_ready.wait());
 }
 
@@ -385,7 +385,7 @@ TEST_F(TestMockJournalReplay, SoftFlushIO) {
     ASSERT_EQ(0, on_safe.wait());
   }
 
-  ASSERT_EQ(0, when_shut_down(mock_journal_replay));
+  ASSERT_EQ(0, when_shut_down(mock_journal_replay, false));
 }
 
 TEST_F(TestMockJournalReplay, PauseIO) {
@@ -428,7 +428,7 @@ TEST_F(TestMockJournalReplay, PauseIO) {
     ASSERT_EQ(0, on_safe.wait());
   }
 
-  ASSERT_EQ(0, when_shut_down(mock_journal_replay));
+  ASSERT_EQ(0, when_shut_down(mock_journal_replay, false));
 }
 
 TEST_F(TestMockJournalReplay, Flush) {
@@ -536,7 +536,7 @@ TEST_F(TestMockJournalReplay, MissingOpFinishEvent) {
 
   ASSERT_EQ(0, on_ready.wait());
 
-  ASSERT_EQ(0, when_shut_down(mock_journal_replay));
+  ASSERT_EQ(0, when_shut_down(mock_journal_replay, true));
   ASSERT_EQ(-ERESTART, on_safe.wait());
 }
 

diff --git a/src/test/librbd/test_mock_Journal.cc b/src/test/librbd/test_mock_Journal.cc
index b797ac838d5f4bc128912969a47c2798f3040bef..a4ded00675fd829cd09d909c87e2762f64e48f87 100644 (file)
--- a/src/test/librbd/test_mock_Journal.cc
+++ b/src/test/librbd/test_mock_Journal.cc
@@ -203,7 +203,7 @@ struct MockReplay {
     s_instance = this;
   }
 
-  MOCK_METHOD1(shut_down, void(Context *));
+  MOCK_METHOD2(shut_down, void(bool cancel_ops, Context *));
   MOCK_METHOD3(process, void(bufferlist::iterator*, Context *, Context *));
   MOCK_METHOD2(replay_op_ready, void(uint64_t, Context *));
 };
@@ -215,8 +215,8 @@ public:
     return new Replay();
   }
 
-  void shut_down(Context *on_finish) {
-    MockReplay::get_instance().shut_down(on_finish);
+  void shut_down(bool cancel_ops, Context *on_finish) {
+    MockReplay::get_instance().shut_down(cancel_ops, on_finish);
   }
 
   void process(bufferlist::iterator *it, Context *on_ready,
@@ -314,10 +314,11 @@ public:
   }
 
   void expect_shut_down_replay(MockImageCtx &mock_image_ctx,
-                               MockJournalReplay &mock_journal_replay, int r) {
-    EXPECT_CALL(mock_journal_replay, shut_down(_))
-                  .WillOnce(Invoke([this, &mock_image_ctx, r](Context *on_flush) {
-                    this->commit_replay(mock_image_ctx, on_flush, r);}));
+                               MockJournalReplay &mock_journal_replay, int r,
+                               bool cancel_ops = false) {
+    EXPECT_CALL(mock_journal_replay, shut_down(cancel_ops, _))
+                  .WillOnce(WithArg<1>(Invoke([this, &mock_image_ctx, r](Context *on_flush) {
+                    this->commit_replay(mock_image_ctx, on_flush, r);})));
   }
 
   void expect_get_data(::journal::MockReplayEntry &mock_replay_entry) {
@@ -542,7 +543,7 @@ TEST_F(TestMockJournal, ReplayCompleteError) {
 
   MockJournalReplay mock_journal_replay;
   expect_stop_replay(mock_journaler);
-  expect_shut_down_replay(mock_image_ctx, mock_journal_replay, 0);
+  expect_shut_down_replay(mock_image_ctx, mock_journal_replay, 0, true);
 
   // replay failure should result in replay-restart
   expect_construct_journaler(mock_journaler);
@@ -670,7 +671,7 @@ TEST_F(TestMockJournal, ReplayOnDiskPreFlushError) {
 
   expect_try_pop_front(mock_journaler, false, mock_replay_entry);
   expect_stop_replay(mock_journaler);
-  expect_shut_down_replay(mock_image_ctx, mock_journal_replay, 0);
+  expect_shut_down_replay(mock_image_ctx, mock_journal_replay, 0, true);
 
   // replay write-to-disk failure should result in replay-restart
   expect_construct_journaler(mock_journaler);
@@ -739,8 +740,8 @@ TEST_F(TestMockJournal, ReplayOnDiskPostFlushError) {
   expect_stop_replay(mock_journaler);
 
   Context *on_flush = nullptr;
-  EXPECT_CALL(mock_journal_replay, shut_down(_))
-    .WillOnce(DoAll(SaveArg<0>(&on_flush),
+  EXPECT_CALL(mock_journal_replay, shut_down(false, _))
+    .WillOnce(DoAll(SaveArg<1>(&on_flush),
                     InvokeWithoutArgs(this, &TestMockJournal::wake_up)));
 
   // replay write-to-disk failure should result in replay-restart

diff --git a/src/tools/rbd_mirror/ImageReplayer.cc b/src/tools/rbd_mirror/ImageReplayer.cc
index 255fae2c9e078c652286a0c18b3419d66bbe6a03..04e135f098c6b29224efebc42cae58c3458489ab 100644 (file)
--- a/src/tools/rbd_mirror/ImageReplayer.cc
+++ b/src/tools/rbd_mirror/ImageReplayer.cc
@@ -323,7 +323,7 @@ fail:
 
   if (m_local_replay) {
     Mutex::Locker locker(m_lock);
-    shut_down_journal_replay();
+    shut_down_journal_replay(true);
     m_local_image_ctx->journal->stop_external_replay();
     m_local_replay = nullptr;
   }
@@ -367,7 +367,7 @@ void ImageReplayer::stop()
     m_state = STATE_STOPPING;
   }
 
-  shut_down_journal_replay();
+  shut_down_journal_replay(false);
 
   m_local_image_ctx->journal->stop_external_replay();
   m_local_replay = nullptr;
@@ -822,10 +822,10 @@ cleanup:
   return r;
 }
 
-void ImageReplayer::shut_down_journal_replay()
+void ImageReplayer::shut_down_journal_replay(bool cancel_ops)
 {
   C_SaferCond cond;
-  m_local_replay->shut_down(&cond);
+  m_local_replay->shut_down(cancel_ops, &cond);
   int r = cond.wait();
   if (r < 0) {
     derr << "error flushing journal replay: " << cpp_strerror(r) << dendl;

diff --git a/src/tools/rbd_mirror/ImageReplayer.h b/src/tools/rbd_mirror/ImageReplayer.h
index c37c5f03cdcecc3dff9a0bfeb38d67b3686c9b15..305a1762100cdb492325f14267f76857b825ccfd 100644 (file)
--- a/src/tools/rbd_mirror/ImageReplayer.h
+++ b/src/tools/rbd_mirror/ImageReplayer.h
@@ -93,7 +93,7 @@ private:
                   std::string *image_id);
   int copy();
 
-  void shut_down_journal_replay();
+  void shut_down_journal_replay(bool cancel_ops);
 
   friend std::ostream &operator<<(std::ostream &os,
                                  const ImageReplayer &replayer);