From a2cfc17d25ca8b61a54afc88a05e70df415d6da7 Mon Sep 17 00:00:00 2001 From: Mykola Golub Date: Wed, 7 Dec 2016 18:18:47 +0200 Subject: [PATCH] rbd-mirror: fix sparse read optimization in image sync Issue truncate or zero ops for the subtracted extents between the diff and the sparse read. Fixes: http://tracker.ceph.com/issues/18146 Signed-off-by: Mykola Golub --- src/test/rbd_mirror/test_ImageSync.cc | 82 +++++++++++++++++++ .../image_sync/ObjectCopyRequest.cc | 42 ++++++++-- .../rbd_mirror/image_sync/ObjectCopyRequest.h | 2 + 3 files changed, 118 insertions(+), 8 deletions(-) diff --git a/src/test/rbd_mirror/test_ImageSync.cc b/src/test/rbd_mirror/test_ImageSync.cc index 6615cb834c902..51305755b80c5 100644 --- a/src/test/rbd_mirror/test_ImageSync.cc +++ b/src/test/rbd_mirror/test_ImageSync.cc @@ -136,6 +136,88 @@ TEST_F(TestImageSync, Simple) { } } +TEST_F(TestImageSync, Resize) { + int64_t object_size = std::min( + m_remote_image_ctx->size, 1 << m_remote_image_ctx->order); + + uint64_t off = 0; + uint64_t len = object_size / 10; + + std::string str(len, '1'); + ASSERT_EQ((int)len, m_remote_image_ctx->aio_work_queue->write(off, len, + str.c_str(), 0)); + { + RWLock::RLocker owner_locker(m_remote_image_ctx->owner_lock); + ASSERT_EQ(0, m_remote_image_ctx->flush()); + } + + ASSERT_EQ(0, create_snap(m_remote_image_ctx, "snap", nullptr)); + + uint64_t size = object_size - 1; + librbd::NoOpProgressContext no_op_progress_ctx; + ASSERT_EQ(0, m_remote_image_ctx->operations->resize(size, true, + no_op_progress_ctx)); + + C_SaferCond ctx; + ImageSync<> *request = create_request(&ctx); + request->send(); + ASSERT_EQ(0, ctx.wait()); + + bufferlist read_remote_bl; + read_remote_bl.append(std::string(len, '\0')); + bufferlist read_local_bl; + read_local_bl.append(std::string(len, '\0')); + + ASSERT_LE(0, m_remote_image_ctx->aio_work_queue->read( + off, len, read_remote_bl.c_str(), 0)); + ASSERT_LE(0, m_local_image_ctx->aio_work_queue->read( + off, len, read_local_bl.c_str(), 0)); + + ASSERT_TRUE(read_remote_bl.contents_equal(read_local_bl)); +} + +TEST_F(TestImageSync, Discard) { + int64_t object_size = std::min( + m_remote_image_ctx->size, 1 << m_remote_image_ctx->order); + + uint64_t off = 0; + uint64_t len = object_size / 10; + + std::string str(len, '1'); + ASSERT_EQ((int)len, m_remote_image_ctx->aio_work_queue->write(off, len, + str.c_str(), 0)); + { + RWLock::RLocker owner_locker(m_remote_image_ctx->owner_lock); + ASSERT_EQ(0, m_remote_image_ctx->flush()); + } + + ASSERT_EQ(0, create_snap(m_remote_image_ctx, "snap", nullptr)); + + ASSERT_EQ((int)len - 2, m_remote_image_ctx->aio_work_queue->discard(off + 1, + len - 2)); + { + RWLock::RLocker owner_locker(m_remote_image_ctx->owner_lock); + ASSERT_EQ(0, m_remote_image_ctx->flush()); + } + + C_SaferCond ctx; + ImageSync<> *request = create_request(&ctx); + request->send(); + ASSERT_EQ(0, ctx.wait()); + + bufferlist read_remote_bl; + read_remote_bl.append(std::string(object_size, '\0')); + bufferlist read_local_bl; + read_local_bl.append(std::string(object_size, '\0')); + + ASSERT_LE(0, m_remote_image_ctx->aio_work_queue->read( + off, len, read_remote_bl.c_str(), 0)); + ASSERT_LE(0, m_local_image_ctx->aio_work_queue->read( + off, len, read_local_bl.c_str(), 0)); + + ASSERT_TRUE(read_remote_bl.contents_equal(read_local_bl)); +} + TEST_F(TestImageSync, SnapshotStress) { std::list snap_names; diff --git a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc index fed48194c265f..749b14a576d8c 100644 --- a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc +++ b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc @@ -174,23 +174,48 @@ void ObjectCopyRequest::send_write_object() { auto &sync_ops = m_snap_sync_ops.begin()->second; assert(!sync_ops.empty()); + uint64_t object_offset; uint64_t buffer_offset; librados::ObjectWriteOperation op; for (auto &sync_op : sync_ops) { switch (std::get<0>(sync_op)) { case SYNC_OP_TYPE_WRITE: + object_offset = std::get<1>(sync_op); buffer_offset = 0; - for(auto it : std::get<4>(sync_op)){ - bufferlist tmpbl; - tmpbl.substr_of(std::get<3>(sync_op), buffer_offset, it.second); - op.write(it.first, tmpbl); - op.set_op_flags2(LIBRADOS_OP_FLAG_FADVISE_SEQUENTIAL | - LIBRADOS_OP_FLAG_FADVISE_NOCACHE); - buffer_offset += it.second; - dout(20) << ": write op: " << it.first<< "~" << it.second << dendl; + for (auto it : std::get<4>(sync_op)) { + if (object_offset < it.first) { + dout(20) << ": zero op: " << object_offset << "~" + << it.first - object_offset << dendl; + op.zero(object_offset, it.first - object_offset); + } + dout(20) << ": write op: " << it.first << "~" << it.second << dendl; + bufferlist tmpbl; + tmpbl.substr_of(std::get<3>(sync_op), buffer_offset, it.second); + op.write(it.first, tmpbl); + op.set_op_flags2(LIBRADOS_OP_FLAG_FADVISE_SEQUENTIAL | + LIBRADOS_OP_FLAG_FADVISE_NOCACHE); + buffer_offset += it.second; + object_offset = it.first + it.second; + } + if (object_offset < std::get<1>(sync_op) + std::get<2>(sync_op)) { + uint64_t sync_op_end = std::get<1>(sync_op) + std::get<2>(sync_op); + assert(sync_op_end <= m_snap_object_sizes[remote_snap_seq]); + if (sync_op_end == m_snap_object_sizes[remote_snap_seq]) { + dout(20) << ": trunc op: " << object_offset << dendl; + op.truncate(object_offset); + m_snap_object_sizes[remote_snap_seq] = object_offset; + } else { + dout(20) << ": zero op: " << object_offset << "~" + << sync_op_end - object_offset << dendl; + op.zero(object_offset, sync_op_end - object_offset); + } } break; case SYNC_OP_TYPE_TRUNC: + if (std::get<1>(sync_op) > m_snap_object_sizes[remote_snap_seq]) { + // skip (must have been updated in WRITE op case issuing trunc op) + break; + } dout(20) << ": trunc op: " << std::get<1>(sync_op) << dendl; op.truncate(std::get<1>(sync_op)); break; @@ -353,6 +378,7 @@ void ObjectCopyRequest::compute_diffs() { bufferlist(), std::map()); } + m_snap_object_sizes[end_remote_snap_id] = end_size; } else { if (prev_exists) { // object remove diff --git a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.h b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.h index 2e353383afad7..fb9126e39b3bc 100644 --- a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.h +++ b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.h @@ -85,6 +85,7 @@ private: typedef std::list SyncOps; typedef std::map SnapSyncOps; typedef std::map SnapObjectStates; + typedef std::map SnapObjectSizes; ImageCtxT *m_local_image_ctx; ImageCtxT *m_remote_image_ctx; @@ -102,6 +103,7 @@ private: SnapSyncOps m_snap_sync_ops; SnapObjectStates m_snap_object_states; + SnapObjectSizes m_snap_object_sizes; void send_list_snaps(); void handle_list_snaps(int r); -- 2.39.5