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 <mgolub@mirantis.com>

diff --git a/src/test/rbd_mirror/test_ImageSync.cc b/src/test/rbd_mirror/test_ImageSync.cc
index 6615cb834c9027d1d0dda92cbf611dfa37496638..51305755b80c53a22bf26533e3f20a0dcf5f1e78 100644 (file)
--- 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<int64_t>(
+    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<int64_t>(
+    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<std::string> snap_names;
 

diff --git a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
index fed48194c265f200f186a12ac060c46d8f58228a..749b14a576d8ca369186184e7ab0042008f40d68 100644 (file)
--- a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
+++ b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
@@ -174,23 +174,48 @@ void ObjectCopyRequest<I>::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<I>::compute_diffs() {
                                                          bufferlist(),
                                                          std::map<uint64_t, uint64_t>());
       }
+      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 2e353383afad73a2dced7d856a2e963e20d5bfe8..fb9126e39b3bc2d9e77d11f7fc879082770e9035 100644 (file)
--- 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<SyncOp> SyncOps;
   typedef std::map<librados::snap_t, SyncOps> SnapSyncOps;
   typedef std::map<librados::snap_t, uint8_t> SnapObjectStates;
+  typedef std::map<librados::snap_t, uint64_t> 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);