From: Jason Dillaman Date: Thu, 31 May 2018 13:29:00 +0000 (-0400) Subject: librbd: commit IO as safe when complete if writeback cache is disabled X-Git-Tag: v14.0.1~1209^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=a27269ab95e4cadcade2131367e40ccf1011316a;p=ceph.git librbd: commit IO as safe when complete if writeback cache is disabled We do not need to flush IO to ensure its safe if the writeback cache is disabled when performing a journal replay. Instead, immediately mark the IO as safe and let the journal's periodic commit throttle handle updating the position. Fixes: http://tracker.ceph.com/issues/23516 Signed-off-by: Jason Dillaman --- diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc index 254f357323096..83a92ea94642e 100644 --- a/src/librbd/ImageCtx.cc +++ b/src/librbd/ImageCtx.cc @@ -904,6 +904,10 @@ public: journal_policy = policy; } + bool ImageCtx::is_writeback_cache_enabled() const { + return (cache && cache_max_dirty > 0); + } + void ImageCtx::get_thread_pool_instance(CephContext *cct, ThreadPool **thread_pool, ContextWQ **op_work_queue) { diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h index e0e4ec556aa56..1b2953b4bba0a 100644 --- a/src/librbd/ImageCtx.h +++ b/src/librbd/ImageCtx.h @@ -316,6 +316,8 @@ namespace librbd { journal::Policy *get_journal_policy() const; void set_journal_policy(journal::Policy *policy); + bool is_writeback_cache_enabled() const; + static void get_thread_pool_instance(CephContext *cct, ThreadPool **thread_pool, ContextWQ **op_work_queue); diff --git a/src/librbd/journal/Replay.cc b/src/librbd/journal/Replay.cc index 4af7158880ea2..89c2bcd858efb 100644 --- a/src/librbd/journal/Replay.cc +++ b/src/librbd/journal/Replay.cc @@ -882,7 +882,8 @@ void Replay::handle_event(const journal::UnknownEvent &event, template void Replay::handle_aio_modify_complete(Context *on_ready, Context *on_safe, - int r, std::set &filters) { + int r, std::set &filters, + bool writeback_cache_enabled) { Mutex::Locker locker(m_lock); CephContext *cct = m_image_ctx.cct; ldout(cct, 20) << ": on_ready=" << on_ready << ", " @@ -901,8 +902,23 @@ void Replay::handle_aio_modify_complete(Context *on_ready, Context *on_safe, return; } - // will be completed after next flush operation completes - m_aio_modify_safe_contexts.insert(on_safe); + if (writeback_cache_enabled) { + // will be completed after next flush operation completes + m_aio_modify_safe_contexts.insert(on_safe); + } else { + // IO is safely stored on disk + assert(m_in_flight_aio_modify > 0); + --m_in_flight_aio_modify; + + if (m_on_aio_ready != nullptr) { + ldout(cct, 10) << ": resuming paused AIO" << dendl; + m_on_aio_ready->complete(0); + m_on_aio_ready = nullptr; + } + + ldout(cct, 20) << ": completing safe context: " << on_safe << dendl; + m_image_ctx.op_work_queue->queue(on_safe, 0); + } } template @@ -1081,13 +1097,18 @@ Replay::create_aio_modify_completion(Context *on_ready, } ++m_in_flight_aio_modify; - m_aio_modify_unsafe_contexts.push_back(on_safe); + + bool writeback_cache_enabled = m_image_ctx.is_writeback_cache_enabled(); + if (writeback_cache_enabled) { + m_aio_modify_unsafe_contexts.push_back(on_safe); + } // FLUSH if we hit the low-water mark -- on_safe contexts are // completed by flushes-only so that we don't move the journal // commit position until safely on-disk - *flush_required = (m_aio_modify_unsafe_contexts.size() == + *flush_required = (writeback_cache_enabled && + m_aio_modify_unsafe_contexts.size() == IN_FLIGHT_IO_LOW_WATER_MARK); if (*flush_required) { ldout(cct, 10) << ": hit AIO replay low-water mark: scheduling flush" @@ -1110,7 +1131,8 @@ Replay::create_aio_modify_completion(Context *on_ready, // event. when flushed, the completion of the next flush will fire the // on_safe callback auto aio_comp = io::AioCompletion::create_and_start( - new C_AioModifyComplete(this, on_ready, on_safe, std::move(filters)), + new C_AioModifyComplete(this, on_ready, on_safe, std::move(filters), + writeback_cache_enabled), util::get_image_ctx(&m_image_ctx), aio_type); return aio_comp; } diff --git a/src/librbd/journal/Replay.h b/src/librbd/journal/Replay.h index 6e058ddb35b7d..206d7db46bbd9 100644 --- a/src/librbd/journal/Replay.h +++ b/src/librbd/journal/Replay.h @@ -78,13 +78,17 @@ private: Context *on_ready; Context *on_safe; std::set filters; + bool writeback_cache_enabled; C_AioModifyComplete(Replay *replay, Context *on_ready, - Context *on_safe, std::set &&filters) + Context *on_safe, std::set &&filters, + bool writeback_cache_enabled) : replay(replay), on_ready(on_ready), on_safe(on_safe), - filters(std::move(filters)) { + filters(std::move(filters)), + writeback_cache_enabled(writeback_cache_enabled) { } void finish(int r) override { - replay->handle_aio_modify_complete(on_ready, on_safe, r, filters); + replay->handle_aio_modify_complete(on_ready, on_safe, r, filters, + writeback_cache_enabled); } }; @@ -177,7 +181,8 @@ private: Context *on_safe); void handle_aio_modify_complete(Context *on_ready, Context *on_safe, - int r, std::set &filters); + int r, std::set &filters, + bool writeback_cache_enabled); void handle_aio_flush_complete(Context *on_flush_safe, Contexts &on_safe_ctxs, int r); diff --git a/src/test/librbd/journal/test_mock_Replay.cc b/src/test/librbd/journal/test_mock_Replay.cc index 95bd5c50ac833..5c82b552eb179 100644 --- a/src/test/librbd/journal/test_mock_Replay.cc +++ b/src/test/librbd/journal/test_mock_Replay.cc @@ -293,6 +293,12 @@ public: } } + void expect_writeback_cache_enabled(MockReplayImageCtx &mock_image_ctx, + bool enabled) { + EXPECT_CALL(mock_image_ctx, is_writeback_cache_enabled()) + .WillRepeatedly(Return(enabled)); + } + void when_process(MockJournalReplay &mock_journal_replay, EventEntry &&event_entry, Context *on_ready, Context *on_safe) { @@ -372,6 +378,7 @@ TEST_F(TestMockJournalReplay, AioDiscard) { MockJournalReplay mock_journal_replay(mock_image_ctx); MockIoImageRequest mock_io_image_request; + expect_writeback_cache_enabled(mock_image_ctx, true); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -405,6 +412,7 @@ TEST_F(TestMockJournalReplay, AioWrite) { MockJournalReplay mock_journal_replay(mock_image_ctx); MockIoImageRequest mock_io_image_request; + expect_writeback_cache_enabled(mock_image_ctx, true); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -469,6 +477,7 @@ TEST_F(TestMockJournalReplay, AioWriteSame) { MockJournalReplay mock_journal_replay(mock_image_ctx); MockIoImageRequest mock_io_image_request; + expect_writeback_cache_enabled(mock_image_ctx, true); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -505,6 +514,7 @@ TEST_F(TestMockJournalReplay, AioCompareAndWrite) { MockJournalReplay mock_compare_and_write_journal_replay(mock_image_ctx); MockJournalReplay mock_mis_compare_and_write_journal_replay(mock_image_ctx); MockIoImageRequest mock_io_image_request; + expect_writeback_cache_enabled(mock_image_ctx, true); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -565,6 +575,7 @@ TEST_F(TestMockJournalReplay, IOError) { MockJournalReplay mock_journal_replay(mock_image_ctx); MockIoImageRequest mock_io_image_request; + expect_writeback_cache_enabled(mock_image_ctx, true); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -598,6 +609,7 @@ TEST_F(TestMockJournalReplay, SoftFlushIO) { MockJournalReplay mock_journal_replay(mock_image_ctx); MockIoImageRequest mock_io_image_request; + expect_writeback_cache_enabled(mock_image_ctx, true); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -642,6 +654,7 @@ TEST_F(TestMockJournalReplay, PauseIO) { MockJournalReplay mock_journal_replay(mock_image_ctx); MockIoImageRequest mock_io_image_request; + expect_writeback_cache_enabled(mock_image_ctx, true); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -677,6 +690,8 @@ TEST_F(TestMockJournalReplay, PauseIO) { } TEST_F(TestMockJournalReplay, Flush) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); @@ -688,6 +703,7 @@ TEST_F(TestMockJournalReplay, Flush) { MockJournalReplay mock_journal_replay(mock_image_ctx); MockIoImageRequest mock_io_image_request; + expect_writeback_cache_enabled(mock_image_ctx, true); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -790,6 +806,8 @@ TEST_F(TestMockJournalReplay, BlockedOpFinishError) { } TEST_F(TestMockJournalReplay, MissingOpFinishEvent) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); @@ -849,6 +867,8 @@ TEST_F(TestMockJournalReplay, MissingOpFinishEvent) { } TEST_F(TestMockJournalReplay, MissingOpFinishEventCancelOps) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); @@ -901,6 +921,8 @@ TEST_F(TestMockJournalReplay, MissingOpFinishEventCancelOps) { } TEST_F(TestMockJournalReplay, UnknownOpFinishEvent) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); @@ -2013,5 +2035,37 @@ TEST_F(TestMockJournalReplay, LockLostBeforeExecuteOp) { ASSERT_EQ(-ECANCELED, on_finish_safe.wait()); } +TEST_F(TestMockJournalReplay, WritebackCacheDisabled) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockReplayImageCtx mock_image_ctx(*ictx); + + MockExclusiveLock mock_exclusive_lock; + mock_image_ctx.exclusive_lock = &mock_exclusive_lock; + expect_accept_ops(mock_exclusive_lock, true); + + MockJournalReplay mock_journal_replay(mock_image_ctx); + MockIoImageRequest mock_io_image_request; + expect_writeback_cache_enabled(mock_image_ctx, false); + expect_op_work_queue(mock_image_ctx); + + InSequence seq; + io::AioCompletion *aio_comp; + C_SaferCond on_ready; + C_SaferCond on_safe; + expect_aio_discard(mock_io_image_request, &aio_comp, 123, 456, false); + when_process(mock_journal_replay, + EventEntry{AioDiscardEvent(123, 456, false)}, + &on_ready, &on_safe); + + when_complete(mock_image_ctx, aio_comp, 0); + ASSERT_EQ(0, on_ready.wait()); + ASSERT_EQ(0, on_safe.wait()); + ASSERT_EQ(0, when_shut_down(mock_journal_replay, false)); +} + } // namespace journal } // namespace librbd diff --git a/src/test/librbd/mock/MockImageCtx.h b/src/test/librbd/mock/MockImageCtx.h index 8c77ca16ffb84..44ade806c755c 100644 --- a/src/test/librbd/mock/MockImageCtx.h +++ b/src/test/librbd/mock/MockImageCtx.h @@ -214,6 +214,8 @@ struct MockImageCtx { MOCK_CONST_METHOD0(get_stripe_count, uint64_t()); MOCK_CONST_METHOD0(get_stripe_period, uint64_t()); + MOCK_CONST_METHOD0(is_writeback_cache_enabled, bool()); + ImageCtx *image_ctx; CephContext *cct; PerfCounters *perfcounter; diff --git a/src/test/rbd_mirror/test_ImageReplayer.cc b/src/test/rbd_mirror/test_ImageReplayer.cc index 5ea29c6d5d1a0..c3ea3246b2f1b 100644 --- a/src/test/rbd_mirror/test_ImageReplayer.cc +++ b/src/test/rbd_mirror/test_ImageReplayer.cc @@ -82,6 +82,11 @@ public: TestImageReplayer() : m_local_cluster(new librados::Rados()), m_watch_handle(0) { + EXPECT_EQ(0, g_ceph_context->_conf->get_val("rbd_mirror_journal_commit_age", + &m_journal_commit_age)); + EXPECT_EQ(0, g_ceph_context->_conf->set_val("rbd_mirror_journal_commit_age", + "0.1")); + EXPECT_EQ("", connect_cluster_pp(*m_local_cluster.get())); EXPECT_EQ(0, m_local_cluster->conf_set("rbd_cache", "false")); EXPECT_EQ(0, m_local_cluster->conf_set("rbd_mirror_journal_poll_age", "1")); @@ -139,6 +144,8 @@ public: EXPECT_EQ(0, m_remote_cluster.pool_delete(m_remote_pool_name.c_str())); EXPECT_EQ(0, m_local_cluster->pool_delete(m_local_pool_name.c_str())); + EXPECT_EQ(0, g_ceph_context->_conf->set_val("rbd_mirror_journal_commit_age", + m_journal_commit_age)); } template > @@ -298,10 +305,6 @@ public: cls::journal::ObjectPosition mirror_position; for (int i = 0; i < 100; i++) { - printf("m_replayer->flush()\n"); - C_SaferCond cond; - m_replayer->flush(&cond); - ASSERT_EQ(0, cond.wait()); get_commit_positions(&master_position, &mirror_position); if (master_position == mirror_position) { break; @@ -394,6 +397,7 @@ public: C_WatchCtx *m_watch_ctx; uint64_t m_watch_handle; char m_test_data[TEST_IO_SIZE + 1]; + std::string m_journal_commit_age; }; int TestImageReplayer::_image_number;