From 9efeb6bc2df559f8d91c480c396b4dd97b981d34 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 4 May 2017 17:56:22 -0400 Subject: [PATCH] librbd: add no-op event when promoting an image The rbd-mirror process needs an event in the journal to properly detect the transition between primary and non-primary state between peers. Fixes: http://tracker.ceph.com/issues/19858 Signed-off-by: Jason Dillaman (cherry picked from commit 4031555dda7597d24e9eb04b9ff29173909586f7) Conflicts: src/librbd/journal/DemoteRequest.cc: logic exists in Journal.cc --- qa/workunits/rbd/rbd_mirror.sh | 19 +++ src/librbd/Journal.cc | 2 +- src/librbd/journal/PromoteRequest.cc | 87 ++++++++++++ src/librbd/journal/PromoteRequest.h | 35 ++++- src/librbd/journal/Replay.cc | 4 +- src/librbd/journal/Replay.h | 2 +- src/librbd/journal/Types.cc | 16 +-- src/librbd/journal/Types.h | 9 +- .../journal/test_mock_PromoteRequest.cc | 125 ++++++++++++++++++ 9 files changed, 276 insertions(+), 23 deletions(-) diff --git a/qa/workunits/rbd/rbd_mirror.sh b/qa/workunits/rbd/rbd_mirror.sh index 114f14127d5e5..645811c5f4620 100755 --- a/qa/workunits/rbd/rbd_mirror.sh +++ b/qa/workunits/rbd/rbd_mirror.sh @@ -127,6 +127,25 @@ wait_for_status_in_pool_dir ${CLUSTER2} ${POOL} ${image} 'up+stopped' wait_for_status_in_pool_dir ${CLUSTER1} ${POOL} ${image} 'up+replaying' 'master_position' compare_images ${POOL} ${image} +# failover (unmodified) +demote_image ${CLUSTER2} ${POOL} ${image} +wait_for_image_replay_stopped ${CLUSTER1} ${POOL} ${image} +wait_for_status_in_pool_dir ${CLUSTER1} ${POOL} ${image} 'up+stopped' +wait_for_status_in_pool_dir ${CLUSTER2} ${POOL} ${image} 'up+stopped' +promote_image ${CLUSTER1} ${POOL} ${image} +wait_for_image_replay_started ${CLUSTER2} ${POOL} ${image} + +# failback (unmodified) +demote_image ${CLUSTER1} ${POOL} ${image} +wait_for_image_replay_stopped ${CLUSTER2} ${POOL} ${image} +wait_for_status_in_pool_dir ${CLUSTER2} ${POOL} ${image} 'up+stopped' +promote_image ${CLUSTER2} ${POOL} ${image} +wait_for_image_replay_started ${CLUSTER1} ${POOL} ${image} +wait_for_replay_complete ${CLUSTER1} ${CLUSTER2} ${POOL} ${image} +wait_for_status_in_pool_dir ${CLUSTER1} ${POOL} ${image} 'up+replaying' 'master_position' +wait_for_status_in_pool_dir ${CLUSTER2} ${POOL} ${image} 'up+stopped' +compare_images ${POOL} ${image} + # failover demote_image ${CLUSTER2} ${POOL} ${image} wait_for_image_replay_stopped ${CLUSTER1} ${POOL} ${image} diff --git a/src/librbd/Journal.cc b/src/librbd/Journal.cc index f799f6b8d1424..a54c8204b5cb2 100644 --- a/src/librbd/Journal.cc +++ b/src/librbd/Journal.cc @@ -723,7 +723,7 @@ int Journal::demote() { return r; } - journal::EventEntry event_entry{journal::DemoteEvent{}}; + journal::EventEntry event_entry{journal::DemotePromoteEvent{}}; bufferlist event_entry_bl; ::encode(event_entry, event_entry_bl); diff --git a/src/librbd/journal/PromoteRequest.cc b/src/librbd/journal/PromoteRequest.cc index b541e48197eb3..99e06519234bd 100644 --- a/src/librbd/journal/PromoteRequest.cc +++ b/src/librbd/journal/PromoteRequest.cc @@ -102,6 +102,93 @@ void PromoteRequest::handle_allocate_tag(int r) { if (r < 0) { m_ret_val = r; lderr(cct) << "failed to allocate tag: " << cpp_strerror(r) << dendl; + shut_down(); + return; + } + + m_tag_tid = m_tag.tid; + append_event(); +} + +template +void PromoteRequest::append_event() { + CephContext *cct = m_image_ctx->cct; + ldout(cct, 20) << dendl; + + EventEntry event_entry{DemotePromoteEvent{}}; + bufferlist event_entry_bl; + ::encode(event_entry, event_entry_bl); + + m_journaler->start_append(0, 0, 0); + m_future = m_journaler->append(m_tag_tid, event_entry_bl); + + auto ctx = create_context_callback< + PromoteRequest, &PromoteRequest::handle_append_event>(this); + m_future.flush(ctx); +} + +template +void PromoteRequest::handle_append_event(int r) { + CephContext *cct = m_image_ctx->cct; + ldout(cct, 20) << "r=" << r << dendl; + + if (r < 0) { + m_ret_val = r; + lderr(cct) << "failed to append promotion journal event: " + << cpp_strerror(r) << dendl; + stop_append(); + return; + } + + commit_event(); +} + +template +void PromoteRequest::commit_event() { + CephContext *cct = m_image_ctx->cct; + ldout(cct, 20) << dendl; + + m_journaler->committed(m_future); + + auto ctx = create_context_callback< + PromoteRequest, &PromoteRequest::handle_commit_event>(this); + m_journaler->flush_commit_position(ctx); +} + +template +void PromoteRequest::handle_commit_event(int r) { + CephContext *cct = m_image_ctx->cct; + ldout(cct, 20) << "r=" << r << dendl; + + if (r < 0) { + m_ret_val = r; + lderr(cct) << "failed to flush promote commit position: " + << cpp_strerror(r) << dendl; + } + + stop_append(); +} + +template +void PromoteRequest::stop_append() { + CephContext *cct = m_image_ctx->cct; + ldout(cct, 20) << dendl; + + auto ctx = create_context_callback< + PromoteRequest, &PromoteRequest::handle_stop_append>(this); + m_journaler->stop_append(ctx); +} + +template +void PromoteRequest::handle_stop_append(int r) { + CephContext *cct = m_image_ctx->cct; + ldout(cct, 20) << "r=" << r << dendl; + + if (r < 0) { + if (m_ret_val == 0) { + m_ret_val = r; + } + lderr(cct) << "failed to stop journal append: " << cpp_strerror(r) << dendl; } shut_down(); diff --git a/src/librbd/journal/PromoteRequest.h b/src/librbd/journal/PromoteRequest.h index 4ef1d116c5833..0d01f596108d8 100644 --- a/src/librbd/journal/PromoteRequest.h +++ b/src/librbd/journal/PromoteRequest.h @@ -7,6 +7,7 @@ #include "include/int_types.h" #include "common/Mutex.h" #include "cls/journal/cls_journal_types.h" +#include "journal/Future.h" #include "librbd/journal/Types.h" #include "librbd/journal/TypeTraits.h" @@ -37,13 +38,22 @@ private: * * | * v - * OPEN - * | - * v - * ALLOCATE_TAG - * | - * v - * SHUT_DOWN + * OPEN * * * * * * * * * * + * | * + * v * + * ALLOCATE_TAG * * * * * * + * | * + * v * + * APPEND_EVENT * * * * + * | * * + * v * * + * COMMIT_EVENT * * + * | * * + * v * * + * STOP_APPEND <* * * * + * | * + * v * + * SHUT_DOWN <* * * * * * * * | * v * @@ -52,6 +62,7 @@ private: */ typedef typename TypeTraits::Journaler Journaler; + typedef typename TypeTraits::Future Future; ImageCtxT *m_image_ctx; bool m_force; @@ -66,6 +77,7 @@ private: TagData m_tag_data; cls::journal::Tag m_tag; + Future m_future; void send_open(); void handle_open(int r); @@ -73,6 +85,15 @@ private: void allocate_tag(); void handle_allocate_tag(int r); + void append_event(); + void handle_append_event(int r); + + void commit_event(); + void handle_commit_event(int r); + + void stop_append(); + void handle_stop_append(int r); + void shut_down(); void handle_shut_down(int r); diff --git a/src/librbd/journal/Replay.cc b/src/librbd/journal/Replay.cc index ff52c50d0468d..c3fe99a6ab30f 100644 --- a/src/librbd/journal/Replay.cc +++ b/src/librbd/journal/Replay.cc @@ -643,10 +643,10 @@ void Replay::handle_event(const journal::FlattenEvent &event, } template -void Replay::handle_event(const journal::DemoteEvent &event, +void Replay::handle_event(const journal::DemotePromoteEvent &event, Context *on_ready, Context *on_safe) { CephContext *cct = m_image_ctx.cct; - ldout(cct, 20) << ": Demote event" << dendl; + ldout(cct, 20) << ": Demote/Promote event" << dendl; on_ready->complete(0); on_safe->complete(0); } diff --git a/src/librbd/journal/Replay.h b/src/librbd/journal/Replay.h index 5b7b7e07925dc..e19cfc763a0da 100644 --- a/src/librbd/journal/Replay.h +++ b/src/librbd/journal/Replay.h @@ -155,7 +155,7 @@ private: Context *on_safe); void handle_event(const FlattenEvent &event, Context *on_ready, Context *on_safe); - void handle_event(const DemoteEvent &event, Context *on_ready, + void handle_event(const DemotePromoteEvent &event, Context *on_ready, Context *on_safe); void handle_event(const SnapLimitEvent &event, Context *on_ready, Context *on_safe); diff --git a/src/librbd/journal/Types.cc b/src/librbd/journal/Types.cc index 8242cffbec871..633f361bd4fda 100644 --- a/src/librbd/journal/Types.cc +++ b/src/librbd/journal/Types.cc @@ -239,13 +239,13 @@ void ResizeEvent::dump(Formatter *f) const { f->dump_unsigned("size", size); } -void DemoteEvent::encode(bufferlist& bl) const { +void DemotePromoteEvent::encode(bufferlist& bl) const { } -void DemoteEvent::decode(__u8 version, bufferlist::iterator& it) { +void DemotePromoteEvent::decode(__u8 version, bufferlist::iterator& it) { } -void DemoteEvent::dump(Formatter *f) const { +void DemotePromoteEvent::dump(Formatter *f) const { } void UpdateFeaturesEvent::encode(bufferlist& bl) const { @@ -366,8 +366,8 @@ void EventEntry::decode(bufferlist::iterator& it) { case EVENT_TYPE_FLATTEN: event = FlattenEvent(); break; - case EVENT_TYPE_DEMOTE: - event = DemoteEvent(); + case EVENT_TYPE_DEMOTE_PROMOTE: + event = DemotePromoteEvent(); break; case EVENT_TYPE_UPDATE_FEATURES: event = UpdateFeaturesEvent(); @@ -431,7 +431,7 @@ void EventEntry::generate_test_instances(std::list &o) { o.push_back(new EventEntry(FlattenEvent(123))); - o.push_back(new EventEntry(DemoteEvent())); + o.push_back(new EventEntry(DemotePromoteEvent())); o.push_back(new EventEntry(UpdateFeaturesEvent())); o.push_back(new EventEntry(UpdateFeaturesEvent(123, 127, true))); @@ -689,8 +689,8 @@ std::ostream &operator<<(std::ostream &out, const EventType &type) { case EVENT_TYPE_FLATTEN: out << "Flatten"; break; - case EVENT_TYPE_DEMOTE: - out << "Demote"; + case EVENT_TYPE_DEMOTE_PROMOTE: + out << "Demote/Promote"; break; case EVENT_TYPE_UPDATE_FEATURES: out << "UpdateFeatures"; diff --git a/src/librbd/journal/Types.h b/src/librbd/journal/Types.h index 505822d9ffc2a..2bc7157716b47 100644 --- a/src/librbd/journal/Types.h +++ b/src/librbd/journal/Types.h @@ -36,7 +36,7 @@ enum EventType { EVENT_TYPE_RENAME = 10, EVENT_TYPE_RESIZE = 11, EVENT_TYPE_FLATTEN = 12, - EVENT_TYPE_DEMOTE = 13, + EVENT_TYPE_DEMOTE_PROMOTE = 13, EVENT_TYPE_SNAP_LIMIT = 14, EVENT_TYPE_UPDATE_FEATURES = 15, EVENT_TYPE_METADATA_SET = 16, @@ -285,8 +285,9 @@ struct FlattenEvent : public OpEventBase { using OpEventBase::dump; }; -struct DemoteEvent { - static const EventType TYPE = static_cast(EVENT_TYPE_DEMOTE); +struct DemotePromoteEvent { + static const EventType TYPE = static_cast( + EVENT_TYPE_DEMOTE_PROMOTE); void encode(bufferlist& bl) const; void decode(__u8 version, bufferlist::iterator& it); @@ -364,7 +365,7 @@ typedef boost::variant struct TypeTraits { typedef ::journal::MockJournalerProxy Journaler; + typedef ::journal::MockFutureProxy Future; }; template <> @@ -63,7 +64,9 @@ namespace librbd { namespace journal { using ::testing::_; +using ::testing::A; using ::testing::InSequence; +using ::testing::Return; using ::testing::WithArg; class TestMockJournalPromoteRequest : public TestMockFixture { @@ -95,6 +98,35 @@ public: .WillOnce(WithArg<3>(CompleteContext(r, NULL))); } + void expect_append_journaler(::journal::MockJournaler &mock_journaler) { + EXPECT_CALL(mock_journaler, append(_, _)) + .WillOnce(Return(::journal::MockFutureProxy())); + } + + void expect_future_flush(::journal::MockFuture &mock_future, int r) { + EXPECT_CALL(mock_future, flush(_)) + .WillOnce(CompleteContext(r, static_cast(NULL))); + } + + void expect_future_committed(::journal::MockJournaler &mock_journaler) { + EXPECT_CALL(mock_journaler, committed(A())); + } + + void expect_flush_commit_position(::journal::MockJournaler &mock_journaler, + int r) { + EXPECT_CALL(mock_journaler, flush_commit_position(_)) + .WillOnce(CompleteContext(r, static_cast(NULL))); + } + + void expect_start_append(::journal::MockJournaler &mock_journaler) { + EXPECT_CALL(mock_journaler, start_append(_, _, _)); + } + + void expect_stop_append(::journal::MockJournaler &mock_journaler, int r) { + EXPECT_CALL(mock_journaler, stop_append(_)) + .WillOnce(CompleteContext(r, static_cast(NULL))); + } + void expect_shut_down_journaler(::journal::MockJournaler &mock_journaler, int r) { EXPECT_CALL(mock_journaler, shut_down(_)) @@ -120,6 +152,15 @@ TEST_F(TestMockJournalPromoteRequest, SuccessOrderly) { expect_open_journaler(mock_image_ctx, mock_open_request, 0); expect_allocate_tag(mock_journaler, {Journal<>::ORPHAN_MIRROR_UUID, true, 567, 1}, 0); + + ::journal::MockFuture mock_future; + expect_start_append(mock_journaler); + expect_append_journaler(mock_journaler); + expect_future_flush(mock_future, 0); + expect_future_committed(mock_journaler); + expect_flush_commit_position(mock_journaler, 0); + expect_stop_append(mock_journaler, 0); + expect_shut_down_journaler(mock_journaler, 0); C_SaferCond ctx; @@ -145,6 +186,15 @@ TEST_F(TestMockJournalPromoteRequest, SuccessForced) { expect_open_journaler(mock_image_ctx, mock_open_request, 0); expect_allocate_tag(mock_journaler, {Journal<>::LOCAL_MIRROR_UUID, true, 567, 0}, 0); + + ::journal::MockFuture mock_future; + expect_start_append(mock_journaler); + expect_append_journaler(mock_journaler); + expect_future_flush(mock_future, 0); + expect_future_committed(mock_journaler); + expect_flush_commit_position(mock_journaler, 0); + expect_stop_append(mock_journaler, 0); + expect_shut_down_journaler(mock_journaler, 0); C_SaferCond ctx; @@ -201,6 +251,72 @@ TEST_F(TestMockJournalPromoteRequest, AllocateTagError) { ASSERT_EQ(-EBADMSG, ctx.wait()); } +TEST_F(TestMockJournalPromoteRequest, AppendEventError) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + ::journal::MockJournaler mock_journaler; + MockOpenRequest mock_open_request; + + expect_op_work_queue(mock_image_ctx); + + InSequence seq; + expect_construct_journaler(mock_journaler); + expect_open_journaler(mock_image_ctx, mock_open_request, 0); + expect_allocate_tag(mock_journaler, + {Journal<>::ORPHAN_MIRROR_UUID, true, 567, 1}, 0); + + ::journal::MockFuture mock_future; + expect_start_append(mock_journaler); + expect_append_journaler(mock_journaler); + expect_future_flush(mock_future, -EPERM); + expect_stop_append(mock_journaler, 0); + + expect_shut_down_journaler(mock_journaler, 0); + + C_SaferCond ctx; + auto req = MockPromoteRequest::create(&mock_image_ctx, false, &ctx); + req->send(); + ASSERT_EQ(-EPERM, ctx.wait()); +} + +TEST_F(TestMockJournalPromoteRequest, CommitEventError) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + ::journal::MockJournaler mock_journaler; + MockOpenRequest mock_open_request; + + expect_op_work_queue(mock_image_ctx); + + InSequence seq; + expect_construct_journaler(mock_journaler); + expect_open_journaler(mock_image_ctx, mock_open_request, 0); + expect_allocate_tag(mock_journaler, + {Journal<>::ORPHAN_MIRROR_UUID, true, 567, 1}, 0); + + ::journal::MockFuture mock_future; + expect_start_append(mock_journaler); + expect_append_journaler(mock_journaler); + expect_future_flush(mock_future, 0); + expect_future_committed(mock_journaler); + expect_flush_commit_position(mock_journaler, -EINVAL); + expect_stop_append(mock_journaler, 0); + + expect_shut_down_journaler(mock_journaler, 0); + + C_SaferCond ctx; + auto req = MockPromoteRequest::create(&mock_image_ctx, false, &ctx); + req->send(); + ASSERT_EQ(-EINVAL, ctx.wait()); +} + TEST_F(TestMockJournalPromoteRequest, ShutDownError) { REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); @@ -218,6 +334,15 @@ TEST_F(TestMockJournalPromoteRequest, ShutDownError) { expect_open_journaler(mock_image_ctx, mock_open_request, 0); expect_allocate_tag(mock_journaler, {Journal<>::LOCAL_MIRROR_UUID, true, 567, 0}, 0); + + ::journal::MockFuture mock_future; + expect_start_append(mock_journaler); + expect_append_journaler(mock_journaler); + expect_future_flush(mock_future, 0); + expect_future_committed(mock_journaler); + expect_flush_commit_position(mock_journaler, 0); + expect_stop_append(mock_journaler, 0); + expect_shut_down_journaler(mock_journaler, -EINVAL); C_SaferCond ctx; -- 2.39.5