Skip to content

Commit

Permalink
Merge pull request #11153 from dillaman/wip-17317
Browse files Browse the repository at this point in the history 
test/rbd: fix possible mock journal race conditions

Reviewed-by: Mykola Golub <mgolub@mirantis.com>
  • Loading branch information
Mykola Golub committed Sep 23, 2016
2 parents 6b52071 + 4718983 commit 70d6b55
Showing 1 changed file with 71 additions and 77 deletions.
148 changes: 71 additions & 77 deletions src/test/librbd/test_mock_Journal.cc
View file
Expand Up @@ -174,7 +174,6 @@ using ::testing::WithArg;
using namespace std::placeholders;

ACTION_P2(StartReplay, wq, ctx) {
ctx->replay_handler = arg0;
wq->queue(ctx, 0);
}

Expand All @@ -186,7 +185,6 @@ class TestMockJournal : public TestMockFixture {
typedef Journal<MockJournalImageCtx> MockJournal;

typedef std::function<void(::journal::ReplayHandler*)> ReplayAction;
typedef std::list<ReplayAction> ReplayActions;
typedef std::list<Context *> Contexts;

TestMockJournal() : m_lock("lock") {
Expand All @@ -200,16 +198,17 @@ class TestMockJournal : public TestMockFixture {
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);
}
}
};
Expand Down Expand Up @@ -268,10 +267,12 @@ class TestMockJournal : public TestMockFixture {

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) {
Expand All @@ -292,11 +293,16 @@ class TestMockJournal : public TestMockFixture {
.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);
Expand Down Expand Up @@ -426,9 +432,8 @@ class TestMockJournal : public TestMockFixture {
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);
Expand All @@ -451,6 +456,8 @@ class TestMockJournal : public TestMockFixture {
static void invoke_replay_complete(::journal::ReplayHandler *handler, int r) {
handler->handle_complete(r);
}

::journal::ReplayHandler *m_replay_handler = nullptr;
};

TEST_F(TestMockJournal, StateTransitions) {
Expand All @@ -472,22 +479,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);
Expand Down Expand Up @@ -583,9 +589,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);
Expand All @@ -599,9 +604,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);
Expand Down Expand Up @@ -632,16 +636,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);
Expand All @@ -653,9 +656,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);
Expand Down Expand Up @@ -686,14 +688,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);
Expand All @@ -706,9 +706,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);
Expand Down Expand Up @@ -738,9 +737,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);
Expand Down Expand Up @@ -771,16 +769,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));
Expand All @@ -789,7 +784,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);
Expand Down Expand Up @@ -827,7 +823,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());

Expand Down Expand Up @@ -855,16 +851,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;
Expand All @@ -880,9 +875,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);
Expand All @@ -891,24 +885,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());
Expand Down

0 comments on commit 70d6b55

Please sign in to comment.