From c4387301c15da7ff84e03561099233c24769feeb Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 24 Feb 2016 17:00:13 -0500 Subject: [PATCH] librbd: only cancel replay of journal op events upon error Signed-off-by: Jason Dillaman --- src/librbd/Journal.cc | 6 +++--- src/librbd/journal/Replay.cc | 12 ++++++----- src/librbd/journal/Replay.h | 2 +- src/test/librbd/journal/test_mock_Replay.cc | 18 ++++++++-------- src/test/librbd/test_mock_Journal.cc | 23 +++++++++++---------- src/tools/rbd_mirror/ImageReplayer.cc | 8 +++---- src/tools/rbd_mirror/ImageReplayer.h | 2 +- 7 files changed, 37 insertions(+), 34 deletions(-) diff --git a/src/librbd/Journal.cc b/src/librbd/Journal.cc index 58a76d1ab04c8..c07ba02b9f7d1 100644 --- a/src/librbd/Journal.cc +++ b/src/librbd/Journal.cc @@ -687,13 +687,13 @@ void Journal::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, &Journal::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, &Journal::handle_flushing_replay>(this)); } } @@ -723,7 +723,7 @@ void Journal::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, &Journal::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 4b6daf19ab280..fa53a86e3e565 100644 --- a/src/librbd/journal/Replay.cc +++ b/src/librbd/journal/Replay.cc @@ -94,7 +94,7 @@ void Replay::process(bufferlist::iterator *it, Context *on_ready, } template -void Replay::shut_down(Context *on_finish) { +void Replay::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::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 d461090092e37..d842a03ff5b41 100644 --- 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 9ec20ffc72878..033ef3c5f9793 100644 --- 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 b797ac838d5f4..a4ded00675fd8 100644 --- 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 255fae2c9e078..04e135f098c6b 100644 --- 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 c37c5f03cdcec..305a1762100cd 100644 --- 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); -- 2.39.5