From c7a9b045808ba741f6aadd2c2e65bd84135347c4 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Tue, 3 Oct 2017 15:42:50 -0400 Subject: [PATCH] librbd: journal should ignore -EILSEQ errors from compare-and-write Fixes: http://tracker.ceph.com/issues/21628 Signed-off-by: Jason Dillaman --- src/librbd/Journal.cc | 14 +++++--- src/librbd/Journal.h | 11 +++--- src/librbd/io/ImageRequest.cc | 8 ++--- src/test/librbd/journal/test_Replay.cc | 2 +- src/test/librbd/mock/MockJournal.h | 8 ++--- src/test/librbd/test_mock_Journal.cc | 47 ++++++++++++++++++++++---- 6 files changed, 65 insertions(+), 25 deletions(-) diff --git a/src/librbd/Journal.cc b/src/librbd/Journal.cc index 30ada69b0a81f..8203390808d15 100644 --- a/src/librbd/Journal.cc +++ b/src/librbd/Journal.cc @@ -766,19 +766,19 @@ uint64_t Journal::append_write_event(uint64_t offset, size_t length, } while (bytes_remaining > 0); return append_io_events(journal::EVENT_TYPE_AIO_WRITE, bufferlists, requests, - offset, length, flush_entry); + offset, length, flush_entry, 0); } template uint64_t Journal::append_io_event(journal::EventEntry &&event_entry, const IOObjectRequests &requests, uint64_t offset, size_t length, - bool flush_entry) { + bool flush_entry, int filter_ret_val) { bufferlist bl; event_entry.timestamp = ceph_clock_now(); ::encode(event_entry, bl); return append_io_events(event_entry.get_event_type(), {bl}, requests, offset, - length, flush_entry); + length, flush_entry, filter_ret_val); } template @@ -786,7 +786,7 @@ uint64_t Journal::append_io_events(journal::EventType event_type, const Bufferlists &bufferlists, const IOObjectRequests &requests, uint64_t offset, size_t length, - bool flush_entry) { + bool flush_entry, int filter_ret_val) { assert(!bufferlists.empty()); uint64_t tid; @@ -806,7 +806,7 @@ uint64_t Journal::append_io_events(journal::EventType event_type, { Mutex::Locker event_locker(m_event_lock); - m_events[tid] = Event(futures, requests, offset, length); + m_events[tid] = Event(futures, requests, offset, length, filter_ret_val); } CephContext *cct = m_image_ctx.cct; @@ -1165,6 +1165,10 @@ void Journal::complete_event(typename Events::iterator it, int r) { << "r=" << r << dendl; Event &event = it->second; + if (r < 0 && r == event.filter_ret_val) { + // ignore allowed error codes + r = 0; + } if (r < 0) { // event recorded to journal but failed to update disk, we cannot // commit this IO event. this event must be replayed. diff --git a/src/librbd/Journal.h b/src/librbd/Journal.h index 3e3cf4928ddb6..abe3723381fb3 100644 --- a/src/librbd/Journal.h +++ b/src/librbd/Journal.h @@ -142,7 +142,7 @@ public: uint64_t append_io_event(journal::EventEntry &&event_entry, const IOObjectRequests &requests, uint64_t offset, size_t length, - bool flush_entry); + bool flush_entry, int filter_ret_val); void commit_io_event(uint64_t tid, int r); void commit_io_event_extent(uint64_t tid, uint64_t offset, uint64_t length, int r); @@ -193,6 +193,7 @@ private: IOObjectRequests aio_object_requests; Contexts on_safe_contexts; ExtentInterval pending_extents; + int filter_ret_val = 0; bool committed_io = false; bool safe = false; int ret_val = 0; @@ -200,8 +201,9 @@ private: Event() { } Event(const Futures &_futures, const IOObjectRequests &_requests, - uint64_t offset, size_t length) - : futures(_futures), aio_object_requests(_requests) { + uint64_t offset, size_t length, int filter_ret_val) + : futures(_futures), aio_object_requests(_requests), + filter_ret_val(filter_ret_val) { if (length > 0) { pending_extents.insert(offset, length); } @@ -334,7 +336,8 @@ private: uint64_t append_io_events(journal::EventType event_type, const Bufferlists &bufferlists, const IOObjectRequests &requests, - uint64_t offset, size_t length, bool flush_entry); + uint64_t offset, size_t length, bool flush_entry, + int filter_ret_val); Future wait_event(Mutex &lock, uint64_t tid, Context *on_safe); void create_journaler(); diff --git a/src/librbd/io/ImageRequest.cc b/src/librbd/io/ImageRequest.cc index 434c208eefd1b..cd87d92e07479 100644 --- a/src/librbd/io/ImageRequest.cc +++ b/src/librbd/io/ImageRequest.cc @@ -605,7 +605,7 @@ uint64_t ImageDiscardRequest::append_journal_event( this->m_skip_partial_discard)); tid = image_ctx.journal->append_io_event(std::move(event_entry), requests, extent.first, - extent.second, synchronous); + extent.second, synchronous, 0); } AioCompletion *aio_comp = this->m_aio_comp; @@ -723,7 +723,7 @@ void ImageFlushRequest::send_request() { // in-flight ops are flushed prior to closing the journal uint64_t journal_tid = image_ctx.journal->append_io_event( journal::EventEntry(journal::AioFlushEvent()), - ObjectRequests(), 0, 0, false); + ObjectRequests(), 0, 0, false, 0); aio_comp->set_request_count(1); aio_comp->associate_journal_event(journal_tid); @@ -822,7 +822,7 @@ uint64_t ImageWriteSameRequest::append_journal_event( m_data_bl)); tid = image_ctx.journal->append_io_event(std::move(event_entry), requests, extent.first, - extent.second, synchronous); + extent.second, synchronous, 0); } if (image_ctx.object_cacher == NULL) { @@ -923,7 +923,7 @@ uint64_t ImageCompareAndWriteRequest::append_journal_event( m_cmp_bl, m_bl)); tid = image_ctx.journal->append_io_event(std::move(event_entry), requests, extent.first, - extent.second, synchronous); + extent.second, synchronous, -EILSEQ); AioCompletion *aio_comp = this->m_aio_comp; aio_comp->associate_journal_event(tid); diff --git a/src/test/librbd/journal/test_Replay.cc b/src/test/librbd/journal/test_Replay.cc index d8c651858a822..0b7180e87cd62 100644 --- a/src/test/librbd/journal/test_Replay.cc +++ b/src/test/librbd/journal/test_Replay.cc @@ -51,7 +51,7 @@ public: { RWLock::RLocker owner_locker(ictx->owner_lock); uint64_t tid = ictx->journal->append_io_event(std::move(event_entry), - requests, 0, 0, true); + requests, 0, 0, true, 0); ictx->journal->wait_event(tid, &ctx); } ASSERT_EQ(0, ctx.wait()); diff --git a/src/test/librbd/mock/MockJournal.h b/src/test/librbd/mock/MockJournal.h index 9f0a985cb7bf3..a0909e34c1d4f 100644 --- a/src/test/librbd/mock/MockJournal.h +++ b/src/test/librbd/mock/MockJournal.h @@ -71,16 +71,16 @@ struct MockJournal { MOCK_METHOD5(append_write_event, uint64_t(uint64_t, size_t, const bufferlist &, const ObjectRequests &, bool)); - MOCK_METHOD5(append_io_event_mock, uint64_t(const journal::EventEntry&, + MOCK_METHOD6(append_io_event_mock, uint64_t(const journal::EventEntry&, const ObjectRequests &, - uint64_t, size_t, bool)); + uint64_t, size_t, bool, int)); uint64_t append_io_event(journal::EventEntry &&event_entry, const ObjectRequests &requests, uint64_t offset, size_t length, - bool flush_entry) { + bool flush_entry, int filter_ret_val) { // googlemock doesn't support move semantics return append_io_event_mock(event_entry, requests, offset, length, - flush_entry); + flush_entry, filter_ret_val); } MOCK_METHOD3(append_op_event_mock, void(uint64_t, const journal::EventEntry&, diff --git a/src/test/librbd/test_mock_Journal.cc b/src/test/librbd/test_mock_Journal.cc index 50b59aac645e8..0e5478a092548 100644 --- a/src/test/librbd/test_mock_Journal.cc +++ b/src/test/librbd/test_mock_Journal.cc @@ -426,7 +426,8 @@ public: uint64_t when_append_io_event(MockJournalImageCtx &mock_image_ctx, MockJournal &mock_journal, - io::ObjectRequest<> *object_request = nullptr) { + io::ObjectRequest<> *object_request, + int filter_ret_val) { RWLock::RLocker owner_locker(mock_image_ctx.owner_lock); MockJournal::IOObjectRequests object_requests; if (object_request != nullptr) { @@ -434,7 +435,7 @@ public: } return mock_journal.append_io_event( journal::EventEntry{journal::AioFlushEvent{}}, object_requests, 0, 0, - false); + false, filter_ret_val); } void save_commit_context(Context *ctx) { @@ -934,13 +935,13 @@ TEST_F(TestMockJournal, EventAndIOCommitOrder) { Context *on_journal_safe1; expect_append_journaler(mock_journaler); expect_wait_future(mock_future, &on_journal_safe1); - ASSERT_EQ(1U, when_append_io_event(mock_image_ctx, mock_journal)); + ASSERT_EQ(1U, when_append_io_event(mock_image_ctx, mock_journal, nullptr, 0)); mock_journal.get_work_queue()->drain(); Context *on_journal_safe2; expect_append_journaler(mock_journaler); expect_wait_future(mock_future, &on_journal_safe2); - ASSERT_EQ(2U, when_append_io_event(mock_image_ctx, mock_journal)); + ASSERT_EQ(2U, when_append_io_event(mock_image_ctx, mock_journal, nullptr, 0)); mock_journal.get_work_queue()->drain(); // commit journal event followed by IO event (standard) @@ -1026,7 +1027,7 @@ TEST_F(TestMockJournal, EventCommitError) { expect_append_journaler(mock_journaler); expect_wait_future(mock_future, &on_journal_safe); ASSERT_EQ(1U, when_append_io_event(mock_image_ctx, mock_journal, - object_request)); + object_request, 0)); mock_journal.get_work_queue()->drain(); // commit the event in the journal w/o waiting writeback @@ -1067,7 +1068,7 @@ TEST_F(TestMockJournal, EventCommitErrorWithPendingWriteback) { expect_append_journaler(mock_journaler); expect_wait_future(mock_future, &on_journal_safe); ASSERT_EQ(1U, when_append_io_event(mock_image_ctx, mock_journal, - object_request)); + object_request, 0)); mock_journal.get_work_queue()->drain(); expect_future_is_valid(mock_future); @@ -1104,7 +1105,7 @@ TEST_F(TestMockJournal, IOCommitError) { Context *on_journal_safe; expect_append_journaler(mock_journaler); expect_wait_future(mock_future, &on_journal_safe); - ASSERT_EQ(1U, when_append_io_event(mock_image_ctx, mock_journal)); + ASSERT_EQ(1U, when_append_io_event(mock_image_ctx, mock_journal, nullptr, 0)); mock_journal.get_work_queue()->drain(); // failed IO remains uncommitted in journal @@ -1115,6 +1116,38 @@ TEST_F(TestMockJournal, IOCommitError) { expect_shut_down_journaler(mock_journaler); } +TEST_F(TestMockJournal, IOCommitErrorFiltered) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockJournalImageCtx mock_image_ctx(*ictx); + MockJournal mock_journal(mock_image_ctx); + ::journal::MockJournaler mock_journaler; + MockJournalOpenRequest mock_open_request; + open_journal(mock_image_ctx, mock_journal, mock_journaler, mock_open_request); + BOOST_SCOPE_EXIT_ALL(&) { + close_journal(mock_journal, mock_journaler); + }; + + ::journal::MockFuture mock_future; + Context *on_journal_safe; + expect_append_journaler(mock_journaler); + expect_wait_future(mock_future, &on_journal_safe); + ASSERT_EQ(1U, when_append_io_event(mock_image_ctx, mock_journal, nullptr, + -EILSEQ)); + mock_journal.get_work_queue()->drain(); + + // filter failed IO committed in journal + on_journal_safe->complete(0); + ictx->op_work_queue->drain(); + expect_future_committed(mock_journaler); + mock_journal.commit_io_event(1U, -EILSEQ); + + expect_shut_down_journaler(mock_journaler); +} + TEST_F(TestMockJournal, FlushCommitPosition) { REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); -- 2.39.5