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: v12.2.6~59^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=2de4e30240fc1e7e141623272e7408c2b49facc8;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 (cherry picked from commit a27269ab95e4cadcade2131367e40ccf1011316a) Conflicts: src/test/rbd_mirror/test_ImageReplayer.cc: removed "md_config_t::get_val" usage --- diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc index 5b277314f2a34..3f64a8003b05a 100644 --- a/src/librbd/ImageCtx.cc +++ b/src/librbd/ImageCtx.cc @@ -1147,6 +1147,10 @@ struct C_InvalidateCache : public Context { 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 e73ca7cc82b5b..dc77af6d48d01 100644 --- a/src/librbd/ImageCtx.h +++ b/src/librbd/ImageCtx.h @@ -331,6 +331,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 21ee18c98d11e..5c0404de1fb95 100644 --- a/src/librbd/journal/Replay.cc +++ b/src/librbd/journal/Replay.cc @@ -858,7 +858,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 << ", " @@ -877,8 +878,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 @@ -1057,13 +1073,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" @@ -1086,7 +1107,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 4a4260cb9f00d..50ccfa96551e9 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 e318cc3916b59..a20828a81e4f5 100644 --- a/src/test/librbd/journal/test_mock_Replay.cc +++ b/src/test/librbd/journal/test_mock_Replay.cc @@ -288,6 +288,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) { @@ -367,6 +373,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; @@ -400,6 +407,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; @@ -464,6 +472,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; @@ -500,6 +509,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; @@ -560,6 +570,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; @@ -593,6 +604,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; @@ -637,6 +649,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; @@ -672,6 +685,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)); @@ -683,6 +698,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; @@ -785,6 +801,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)); @@ -844,6 +862,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)); @@ -896,6 +916,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)); @@ -2008,5 +2030,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 223ea537d5556..4bb723dc766e4 100644 --- a/src/test/librbd/mock/MockImageCtx.h +++ b/src/test/librbd/mock/MockImageCtx.h @@ -218,6 +218,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 d8e159f5f3d4e..9baebf58a4b04 100644 --- a/src/test/rbd_mirror/test_ImageReplayer.cc +++ b/src/test/rbd_mirror/test_ImageReplayer.cc @@ -83,6 +83,9 @@ public: TestImageReplayer() : m_local_cluster(new librados::Rados()), m_watch_handle(0) { + 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")); @@ -142,6 +145,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", + "5")); } template > @@ -301,10 +306,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;