From: Jason Dillaman Date: Tue, 20 Sep 2016 17:31:36 +0000 (-0400) Subject: test/rbd: fix possible mock journal race conditions X-Git-Tag: v10.2.4~61^2~26 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=5d1d898e1132325cae7045dc764a533878d56e00;p=ceph.git test/rbd: fix possible mock journal race conditions Fixes: http://tracker.ceph.com/issues/17317 Signed-off-by: Jason Dillaman (cherry picked from commit 471898392372ba4c404376410fb56f3af5287c80) --- diff --git a/src/test/librbd/test_mock_Journal.cc b/src/test/librbd/test_mock_Journal.cc index 3785f2ce783..3864cdbfec2 100644 --- a/src/test/librbd/test_mock_Journal.cc +++ b/src/test/librbd/test_mock_Journal.cc @@ -109,7 +109,6 @@ using ::testing::WithArg; using namespace std::placeholders; ACTION_P2(StartReplay, wq, ctx) { - ctx->replay_handler = arg0; wq->queue(ctx, 0); } @@ -121,7 +120,6 @@ public: typedef Journal MockJournal; typedef std::function ReplayAction; - typedef std::list ReplayActions; typedef std::list Contexts; TestMockJournal() : m_lock("lock") { @@ -135,16 +133,17 @@ public: Cond m_cond; Contexts m_commit_contexts; - struct C_StartReplay : public Context { - ReplayActions replay_actions; - ::journal::ReplayHandler *replay_handler; + struct C_ReplayAction : public Context { + ::journal::ReplayHandler **replay_handler; + ReplayAction replay_action; - C_StartReplay(const ReplayActions &replay_actions) - : replay_actions(replay_actions), replay_handler(nullptr) { + C_ReplayAction(::journal::ReplayHandler **replay_handler, + const ReplayAction &replay_action) + : replay_handler(replay_handler), replay_action(replay_action) { } virtual void finish(int r) { - for (auto &action : replay_actions) { - action(replay_handler); + if (replay_action) { + replay_action(*replay_handler); } } }; @@ -203,10 +202,12 @@ public: void expect_start_replay(MockJournalImageCtx &mock_image_ctx, ::journal::MockJournaler &mock_journaler, - const ReplayActions &actions) { + const ReplayAction &action) { EXPECT_CALL(mock_journaler, start_replay(_)) - .WillOnce(StartReplay(mock_image_ctx.image_ctx->op_work_queue, - new C_StartReplay(actions))); + .WillOnce(DoAll(SaveArg<0>(&m_replay_handler), + StartReplay(mock_image_ctx.image_ctx->op_work_queue, + new C_ReplayAction(&m_replay_handler, + action)))); } void expect_stop_replay(::journal::MockJournaler &mock_journaler) { @@ -227,11 +228,16 @@ public: .WillOnce(Return(bufferlist())); } - void expect_try_pop_front(::journal::MockJournaler &mock_journaler, + void expect_try_pop_front(MockJournalImageCtx &mock_image_ctx, + ::journal::MockJournaler &mock_journaler, bool entries_available, - ::journal::MockReplayEntry &mock_replay_entry) { + ::journal::MockReplayEntry &mock_replay_entry, + const ReplayAction &action = {}) { EXPECT_CALL(mock_journaler, try_pop_front(_)) .WillOnce(DoAll(SetArgPointee<0>(::journal::MockReplayEntryProxy()), + StartReplay(mock_image_ctx.image_ctx->op_work_queue, + new C_ReplayAction(&m_replay_handler, + action)), Return(entries_available))); if (entries_available) { expect_get_data(mock_replay_entry); @@ -361,9 +367,8 @@ public: expect_get_journaler_cached_client(mock_journaler, 0); expect_get_journaler_tags(mock_image_ctx, mock_journaler, 0); expect_start_replay( - mock_image_ctx, mock_journaler, { - std::bind(&invoke_replay_complete, _1, 0) - }); + mock_image_ctx, mock_journaler, + std::bind(&invoke_replay_complete, _1, 0)); MockJournalReplay mock_journal_replay; expect_stop_replay(mock_journaler); @@ -386,6 +391,8 @@ public: static void invoke_replay_complete(::journal::ReplayHandler *handler, int r) { handler->handle_complete(r); } + + ::journal::ReplayHandler *m_replay_handler = nullptr; }; TEST_F(TestMockJournal, StateTransitions) { @@ -407,22 +414,21 @@ TEST_F(TestMockJournal, StateTransitions) { expect_get_journaler_cached_client(mock_journaler, 0); expect_get_journaler_tags(mock_image_ctx, mock_journaler, 0); expect_start_replay( - mock_image_ctx, mock_journaler, { - std::bind(&invoke_replay_ready, _1), - std::bind(&invoke_replay_ready, _1), - std::bind(&invoke_replay_complete, _1, 0) - }); + mock_image_ctx, mock_journaler, + std::bind(&invoke_replay_ready, _1)); ::journal::MockReplayEntry mock_replay_entry; MockJournalReplay mock_journal_replay; - expect_try_pop_front(mock_journaler, true, mock_replay_entry); + expect_try_pop_front(mock_image_ctx, mock_journaler, true, mock_replay_entry); expect_replay_process(mock_journal_replay); - expect_try_pop_front(mock_journaler, true, mock_replay_entry); + expect_try_pop_front(mock_image_ctx, mock_journaler, true, mock_replay_entry); expect_replay_process(mock_journal_replay); - expect_try_pop_front(mock_journaler, false, mock_replay_entry); - expect_try_pop_front(mock_journaler, true, mock_replay_entry); + expect_try_pop_front(mock_image_ctx, mock_journaler, false, mock_replay_entry, + std::bind(&invoke_replay_ready, _1)); + expect_try_pop_front(mock_image_ctx, mock_journaler, true, mock_replay_entry); expect_replay_process(mock_journal_replay); - expect_try_pop_front(mock_journaler, false, mock_replay_entry); + expect_try_pop_front(mock_image_ctx, mock_journaler, false, mock_replay_entry, + std::bind(&invoke_replay_complete, _1, 0)); expect_stop_replay(mock_journaler); expect_shut_down_replay(mock_image_ctx, mock_journal_replay, 0); @@ -518,9 +524,8 @@ TEST_F(TestMockJournal, ReplayCompleteError) { expect_get_journaler_cached_client(mock_journaler, 0); expect_get_journaler_tags(mock_image_ctx, mock_journaler, 0); expect_start_replay( - mock_image_ctx, mock_journaler, { - std::bind(&invoke_replay_complete, _1, -EINVAL) - }); + mock_image_ctx, mock_journaler, + std::bind(&invoke_replay_complete, _1, -EINVAL)); MockJournalReplay mock_journal_replay; expect_stop_replay(mock_journaler); @@ -534,9 +539,8 @@ TEST_F(TestMockJournal, ReplayCompleteError) { expect_get_journaler_cached_client(mock_journaler, 0); expect_get_journaler_tags(mock_image_ctx, mock_journaler, 0); expect_start_replay( - mock_image_ctx, mock_journaler, { - std::bind(&invoke_replay_complete, _1, 0) - }); + mock_image_ctx, mock_journaler, + std::bind(&invoke_replay_complete, _1, 0)); expect_stop_replay(mock_journaler); expect_shut_down_replay(mock_image_ctx, mock_journal_replay, 0); @@ -567,16 +571,15 @@ TEST_F(TestMockJournal, FlushReplayError) { expect_get_journaler_cached_client(mock_journaler, 0); expect_get_journaler_tags(mock_image_ctx, mock_journaler, 0); expect_start_replay( - mock_image_ctx, mock_journaler, { - std::bind(&invoke_replay_ready, _1), - std::bind(&invoke_replay_complete, _1, 0) - }); + mock_image_ctx, mock_journaler, + std::bind(&invoke_replay_ready, _1)); ::journal::MockReplayEntry mock_replay_entry; MockJournalReplay mock_journal_replay; - expect_try_pop_front(mock_journaler, true, mock_replay_entry); + expect_try_pop_front(mock_image_ctx, mock_journaler, true, mock_replay_entry); expect_replay_process(mock_journal_replay); - expect_try_pop_front(mock_journaler, false, mock_replay_entry); + expect_try_pop_front(mock_image_ctx, mock_journaler, false, mock_replay_entry, + std::bind(&invoke_replay_complete, _1, 0)); expect_stop_replay(mock_journaler); expect_shut_down_replay(mock_image_ctx, mock_journal_replay, -EINVAL); expect_shut_down_journaler(mock_journaler); @@ -588,9 +591,8 @@ TEST_F(TestMockJournal, FlushReplayError) { expect_get_journaler_cached_client(mock_journaler, 0); expect_get_journaler_tags(mock_image_ctx, mock_journaler, 0); expect_start_replay( - mock_image_ctx, mock_journaler, { - std::bind(&invoke_replay_complete, _1, 0) - }); + mock_image_ctx, mock_journaler, + std::bind(&invoke_replay_complete, _1, 0)); expect_stop_replay(mock_journaler); expect_shut_down_replay(mock_image_ctx, mock_journal_replay, 0); @@ -621,14 +623,12 @@ TEST_F(TestMockJournal, CorruptEntry) { expect_get_journaler_cached_client(mock_journaler, 0); expect_get_journaler_tags(mock_image_ctx, mock_journaler, 0); expect_start_replay( - mock_image_ctx, mock_journaler, { - std::bind(&invoke_replay_ready, _1), - std::bind(&invoke_replay_complete, _1, 0) - }); + mock_image_ctx, mock_journaler, + std::bind(&invoke_replay_ready, _1)); ::journal::MockReplayEntry mock_replay_entry; MockJournalReplay mock_journal_replay; - expect_try_pop_front(mock_journaler, true, mock_replay_entry); + expect_try_pop_front(mock_image_ctx, mock_journaler, true, mock_replay_entry); EXPECT_CALL(mock_journal_replay, decode(_, _)).WillOnce(Return(-EBADMSG)); expect_stop_replay(mock_journaler); expect_shut_down_replay(mock_image_ctx, mock_journal_replay, 0, true); @@ -641,9 +641,8 @@ TEST_F(TestMockJournal, CorruptEntry) { expect_get_journaler_cached_client(mock_journaler, 0); expect_get_journaler_tags(mock_image_ctx, mock_journaler, 0); expect_start_replay( - mock_image_ctx, mock_journaler, { - std::bind(&invoke_replay_complete, _1, 0) - }); + mock_image_ctx, mock_journaler, + std::bind(&invoke_replay_complete, _1, 0)); expect_stop_replay(mock_journaler); expect_shut_down_replay(mock_image_ctx, mock_journal_replay, 0); expect_start_append(mock_journaler); @@ -673,9 +672,8 @@ TEST_F(TestMockJournal, StopError) { expect_get_journaler_cached_client(mock_journaler, 0); expect_get_journaler_tags(mock_image_ctx, mock_journaler, 0); expect_start_replay( - mock_image_ctx, mock_journaler, { - std::bind(&invoke_replay_complete, _1, 0) - }); + mock_image_ctx, mock_journaler, + std::bind(&invoke_replay_complete, _1, 0)); MockJournalReplay mock_journal_replay; expect_stop_replay(mock_journaler); @@ -706,16 +704,13 @@ TEST_F(TestMockJournal, ReplayOnDiskPreFlushError) { expect_get_journaler_cached_client(mock_journaler, 0); expect_get_journaler_tags(mock_image_ctx, mock_journaler, 0); - ::journal::ReplayHandler *replay_handler = nullptr; expect_start_replay( - mock_image_ctx, mock_journaler, { - std::bind(&invoke_replay_ready, _1), - [&replay_handler] (::journal::ReplayHandler *handler) {replay_handler = handler;}, - }); + mock_image_ctx, mock_journaler, + std::bind(&invoke_replay_ready, _1)); ::journal::MockReplayEntry mock_replay_entry; MockJournalReplay mock_journal_replay; - expect_try_pop_front(mock_journaler, true, mock_replay_entry); + expect_try_pop_front(mock_image_ctx, mock_journaler, true, mock_replay_entry); EXPECT_CALL(mock_journal_replay, decode(_, _)) .WillOnce(Return(0)); @@ -724,7 +719,8 @@ TEST_F(TestMockJournal, ReplayOnDiskPreFlushError) { .WillOnce(DoAll(SaveArg<1>(&on_ready), WithArg<2>(Invoke(this, &TestMockJournal::save_commit_context)))); - expect_try_pop_front(mock_journaler, false, mock_replay_entry); + expect_try_pop_front(mock_image_ctx, mock_journaler, false, + mock_replay_entry); expect_stop_replay(mock_journaler); expect_shut_down_replay(mock_image_ctx, mock_journal_replay, 0, true); expect_shut_down_journaler(mock_journaler); @@ -762,7 +758,7 @@ TEST_F(TestMockJournal, ReplayOnDiskPreFlushError) { on_safe->complete(-EINVAL); // flag the replay as complete - replay_handler->handle_complete(0); + m_replay_handler->handle_complete(0); ASSERT_EQ(0, ctx.wait()); @@ -790,16 +786,15 @@ TEST_F(TestMockJournal, ReplayOnDiskPostFlushError) { expect_get_journaler_cached_client(mock_journaler, 0); expect_get_journaler_tags(mock_image_ctx, mock_journaler, 0); expect_start_replay( - mock_image_ctx, mock_journaler, { - std::bind(&invoke_replay_ready, _1), - std::bind(&invoke_replay_complete, _1, 0) - }); + mock_image_ctx, mock_journaler, + std::bind(&invoke_replay_ready, _1)); ::journal::MockReplayEntry mock_replay_entry; MockJournalReplay mock_journal_replay; - expect_try_pop_front(mock_journaler, true, mock_replay_entry); + expect_try_pop_front(mock_image_ctx, mock_journaler, true, mock_replay_entry); expect_replay_process(mock_journal_replay); - expect_try_pop_front(mock_journaler, false, mock_replay_entry); + expect_try_pop_front(mock_image_ctx, mock_journaler, false, mock_replay_entry, + std::bind(&invoke_replay_complete, _1, 0)); expect_stop_replay(mock_journaler); Context *on_flush = nullptr; @@ -815,9 +810,8 @@ TEST_F(TestMockJournal, ReplayOnDiskPostFlushError) { expect_get_journaler_cached_client(mock_journaler, 0); expect_get_journaler_tags(mock_image_ctx, mock_journaler, 0); expect_start_replay( - mock_image_ctx, mock_journaler, { - std::bind(&invoke_replay_complete, _1, 0) - }); + mock_image_ctx, mock_journaler, + std::bind(&invoke_replay_complete, _1, 0)); expect_stop_replay(mock_journaler); expect_shut_down_replay(mock_image_ctx, mock_journal_replay, 0); @@ -826,24 +820,24 @@ TEST_F(TestMockJournal, ReplayOnDiskPostFlushError) { C_SaferCond ctx; mock_journal.open(&ctx); + // proceed with the flush { - // wait for the on_safe process callback + // wait for on_flush callback Mutex::Locker locker(m_lock); - while (m_commit_contexts.empty()) { + while (on_flush == nullptr) { m_cond.Wait(m_lock); } } - m_commit_contexts.front()->complete(-EINVAL); - m_commit_contexts.clear(); - // proceed with the flush { - // wait for on_flush callback + // wait for the on_safe process callback Mutex::Locker locker(m_lock); - while (on_flush == nullptr) { + while (m_commit_contexts.empty()) { m_cond.Wait(m_lock); } } + m_commit_contexts.front()->complete(-EINVAL); + m_commit_contexts.clear(); on_flush->complete(0); ASSERT_EQ(0, ctx.wait());