librbd: deep-copy image copy state machine skips clean objects

If fast-diff is enabled (and valid), the image-copy state machine will
now attempt to skip any objects that are flagged as clean (DIFF_STATE_NONE).
For larger images, this should avoid the need to iterate over potentially
tens or hundreds of thousands of objects.

Fixes: https://tracker.ceph.com/issues/43590
Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/deep_copy/ImageCopyRequest.cc b/src/librbd/deep_copy/ImageCopyRequest.cc
index 64be5fee908153a5405b3cc1c389b427b56f1561..09d51169f1a3b409233537d5c7d1e0b13c0962ef 100644 (file)
--- a/src/librbd/deep_copy/ImageCopyRequest.cc
+++ b/src/librbd/deep_copy/ImageCopyRequest.cc
@@ -8,6 +8,7 @@
 #include "librbd/deep_copy/Utils.h"
 #include "librbd/image/CloseRequest.h"
 #include "librbd/image/OpenRequest.h"
+#include "librbd/object_map/DiffRequest.h"
 #include "osdc/Striper.h"
 
 #define dout_subsys ceph_subsys_rbd
@@ -53,7 +54,7 @@ void ImageCopyRequest<I>::send() {
     return;
   }
 
-  send_object_copies();
+  compute_diff();
 }
 
 template <typename I>
@@ -64,6 +65,30 @@ void ImageCopyRequest<I>::cancel() {
   m_canceled = true;
 }
 
+template <typename I>
+void ImageCopyRequest<I>::compute_diff() {
+  ldout(m_cct, 10) << dendl;
+
+  auto ctx = create_context_callback<
+    ImageCopyRequest<I>, &ImageCopyRequest<I>::handle_compute_diff>(this);
+  auto req = object_map::DiffRequest<I>::create(m_src_image_ctx, m_src_snap_id_start,
+                                                m_src_snap_id_end, &m_object_diff_state,
+                                                ctx);
+  req->send();
+}
+
+template <typename I>
+void ImageCopyRequest<I>::handle_compute_diff(int r) {
+  ldout(m_cct, 10) << "r=" << r << dendl;
+
+  if (r < 0) {
+    ldout(m_cct, 10) << "fast-diff optimization disabled" << dendl;
+    m_object_diff_state.resize(0);
+  }
+
+  send_object_copies();
+}
+
 template <typename I>
 void ImageCopyRequest<I>::send_object_copies() {
   m_object_no = 0;
@@ -87,14 +112,18 @@ void ImageCopyRequest<I>::send_object_copies() {
   bool complete;
   {
     std::lock_guard locker{m_lock};
-    for (uint64_t i = 0;
-         i < m_src_image_ctx->config.template get_val<uint64_t>("rbd_concurrent_management_ops");
-         ++i) {
-      send_next_object_copy();
-      if (m_ret_val < 0 && m_current_ops == 0) {
+    auto max_ops = m_src_image_ctx->config.template get_val<uint64_t>(
+      "rbd_concurrent_management_ops");
+
+    // attempt to schedule at least 'max_ops' initial requests where
+    // some objects might be skipped if fast-diff notes no change
+    while (m_current_ops < max_ops) {
+      int r = send_next_object_copy();
+      if (r < 0) {
         break;
       }
     }
+
     complete = (m_current_ops == 0) && !m_updating_progress;
   }
 
@@ -104,7 +133,7 @@ void ImageCopyRequest<I>::send_object_copies() {
 }
 
 template <typename I>
-void ImageCopyRequest<I>::send_next_object_copy() {
+int ImageCopyRequest<I>::send_next_object_copy() {
   ceph_assert(ceph_mutex_is_locked(m_lock));
 
   if (m_canceled && m_ret_val == 0) {
@@ -112,14 +141,20 @@ void ImageCopyRequest<I>::send_next_object_copy() {
     m_ret_val = -ECANCELED;
   }
 
-  if (m_ret_val < 0 || m_object_no >= m_end_object_no) {
-    return;
+  if (m_ret_val < 0) {
+    return m_ret_val;
+  } else if (m_object_no >= m_end_object_no) {
+    return -ENODATA;
   }
 
   uint64_t ono = m_object_no++;
+  if (ono < m_object_diff_state.size() &&
+      m_object_diff_state[ono] == object_map::DIFF_STATE_NONE) {
+    ldout(m_cct, 20) << "skipping clean object " << ono << dendl;
+    return 1;
+  }
 
   ldout(m_cct, 20) << "object_num=" << ono << dendl;
-
   ++m_current_ops;
 
   Context *ctx = new LambdaContext(
@@ -130,6 +165,7 @@ void ImageCopyRequest<I>::send_next_object_copy() {
     m_src_image_ctx, m_dst_image_ctx, m_src_snap_id_start, m_dst_snap_id_start,
     m_snap_map, ono, m_flatten, ctx);
   req->send();
+  return 0;
 }
 
 template <typename I>
@@ -164,7 +200,13 @@ void ImageCopyRequest<I>::handle_object_copy(uint64_t object_no, int r) {
       }
     }
 
-    send_next_object_copy();
+    while (true) {
+      r = send_next_object_copy();
+      if (r != 1) {
+        break;
+      }
+    }
+
     complete = (m_current_ops == 0) && !m_updating_progress;
   }
 

diff --git a/src/librbd/deep_copy/ImageCopyRequest.h b/src/librbd/deep_copy/ImageCopyRequest.h
index 903952254427a7d18e2613a761b60642a54df719..45967fdaf2a548378b2b3c2617de00b1e74f6666 100644 (file)
--- a/src/librbd/deep_copy/ImageCopyRequest.h
+++ b/src/librbd/deep_copy/ImageCopyRequest.h
@@ -6,6 +6,7 @@
 
 #include "include/int_types.h"
 #include "include/rados/librados.hpp"
+#include "common/bit_vector.hpp"
 #include "common/ceph_mutex.h"
 #include "common/RefCountedObj.h"
 #include "librbd/Types.h"
@@ -59,6 +60,10 @@ private:
    * @verbatim
    *
    * <start>
+   *    |
+   *    v
+   * COMPUTE_DIFF
+   *    |
    *    |      . . . . .
    *    |      .       .  (parallel execution of
    *    v      v       .   multiple objects at once)
@@ -94,8 +99,13 @@ private:
   SnapMap m_snap_map;
   int m_ret_val = 0;
 
+  BitVector<2> m_object_diff_state;
+
+  void compute_diff();
+  void handle_compute_diff(int r);
+
   void send_object_copies();
-  void send_next_object_copy();
+  int send_next_object_copy();
   void handle_object_copy(uint64_t object_no, int r);
 
   void finish(int r);

diff --git a/src/test/librbd/deep_copy/test_mock_ImageCopyRequest.cc b/src/test/librbd/deep_copy/test_mock_ImageCopyRequest.cc
index 8becf673b07826285e42f9df86eb75f3b974df6c..a0a0c18b47b60c3145c518e4be8537e089b18744 100644 (file)
--- a/src/test/librbd/deep_copy/test_mock_ImageCopyRequest.cc
+++ b/src/test/librbd/deep_copy/test_mock_ImageCopyRequest.cc
@@ -5,12 +5,13 @@
 #include "include/rbd/librbd.hpp"
 #include "librbd/ImageCtx.h"
 #include "librbd/ImageState.h"
+#include "librbd/internal.h"
 #include "librbd/Operations.h"
 #include "librbd/deep_copy/ImageCopyRequest.h"
 #include "librbd/deep_copy/ObjectCopyRequest.h"
 #include "librbd/image/CloseRequest.h"
 #include "librbd/image/OpenRequest.h"
-#include "librbd/internal.h"
+#include "librbd/object_map/DiffRequest.h"
 #include "test/librados_test_stub/MockTestMemIoCtxImpl.h"
 #include "test/librbd/mock/MockImageCtx.h"
 #include "test/librbd/test_support.h"
@@ -122,6 +123,33 @@ OpenRequest<MockTestImageCtx>* OpenRequest<MockTestImageCtx>::s_instance = nullp
 
 } // namespace image
 
+namespace object_map {
+
+template <>
+struct DiffRequest<MockTestImageCtx> {
+  BitVector<2>* object_diff_state = nullptr;
+  Context* on_finish = nullptr;
+  static DiffRequest* s_instance;
+  static DiffRequest* create(MockTestImageCtx *image_ctx,
+                             uint64_t snap_id_start, uint64_t snap_id_end,
+                             BitVector<2>* object_diff_state,
+                             Context* on_finish) {
+    ceph_assert(s_instance != nullptr);
+    s_instance->object_diff_state = object_diff_state;
+    s_instance->on_finish = on_finish;
+    return s_instance;
+  }
+
+  DiffRequest() {
+    s_instance = this;
+  }
+
+  MOCK_METHOD0(send, void());
+};
+
+DiffRequest<MockTestImageCtx>* DiffRequest<MockTestImageCtx>::s_instance = nullptr;
+
+} // namespace object_map
 } // namespace librbd
 
 // template definitions
@@ -133,12 +161,14 @@ namespace deep_copy {
 
 using ::testing::_;
 using ::testing::InSequence;
+using ::testing::Invoke;
 using ::testing::Return;
 
 class TestMockDeepCopyImageCopyRequest : public TestMockFixture {
 public:
   typedef ImageCopyRequest<librbd::MockTestImageCtx> MockImageCopyRequest;
   typedef ObjectCopyRequest<librbd::MockTestImageCtx> MockObjectCopyRequest;
+  typedef object_map::DiffRequest<librbd::MockTestImageCtx> MockDiffRequest;
 
   librbd::ImageCtx *m_src_image_ctx;
   librbd::ImageCtx *m_dst_image_ctx;
@@ -167,6 +197,17 @@ public:
       .WillOnce(Return(size)).RetiresOnSaturation();
   }
 
+  void expect_diff_send(MockDiffRequest& mock_request,
+                        const BitVector<2>& diff_state, int r) {
+    EXPECT_CALL(mock_request, send())
+      .WillOnce(Invoke([this, &mock_request, diff_state, r]() {
+                  if (r >= 0) {
+                    *mock_request.object_diff_state = diff_state;
+                  }
+                  m_work_queue->queue(mock_request.on_finish, r);
+                }));
+  }
+
   void expect_object_copy_send(MockObjectCopyRequest &mock_object_copy_request) {
     EXPECT_CALL(mock_object_copy_request, send());
   }
@@ -266,6 +307,8 @@ TEST_F(TestMockDeepCopyImageCopyRequest, SimpleImage) {
   MockObjectCopyRequest mock_object_copy_request;
 
   InSequence seq;
+  MockDiffRequest mock_diff_request;
+  expect_diff_send(mock_diff_request, {}, -EINVAL);
   expect_get_image_size(mock_src_image_ctx, 1 << m_src_image_ctx->order);
   expect_get_image_size(mock_src_image_ctx, 0);
   expect_object_copy_send(mock_object_copy_request);
@@ -283,6 +326,34 @@ TEST_F(TestMockDeepCopyImageCopyRequest, SimpleImage) {
   ASSERT_EQ(0, ctx.wait());
 }
 
+TEST_F(TestMockDeepCopyImageCopyRequest, FastDiff) {
+  librados::snap_t snap_id_end;
+  ASSERT_EQ(0, create_snap("copy", &snap_id_end));
+
+  librbd::MockTestImageCtx mock_src_image_ctx(*m_src_image_ctx);
+  librbd::MockTestImageCtx mock_dst_image_ctx(*m_dst_image_ctx);
+
+  InSequence seq;
+
+  MockDiffRequest mock_diff_request;
+  BitVector<2> diff_state;
+  diff_state.resize(1);
+  expect_diff_send(mock_diff_request, diff_state, 0);
+
+  expect_get_image_size(mock_src_image_ctx, 1 << m_src_image_ctx->order);
+  expect_get_image_size(mock_src_image_ctx, 0);
+
+  librbd::NoOpProgressContext no_op;
+  C_SaferCond ctx;
+  auto request = new MockImageCopyRequest(&mock_src_image_ctx,
+                                          &mock_dst_image_ctx,
+                                          0, snap_id_end, 0, false, boost::none,
+                                          m_snap_seqs, &no_op, &ctx);
+  request->send();
+
+  ASSERT_EQ(0, ctx.wait());
+}
+
 TEST_F(TestMockDeepCopyImageCopyRequest, OutOfOrder) {
   std::string max_ops_str;
   ASSERT_EQ(0, _rados.conf_get("rbd_concurrent_management_ops", max_ops_str));
@@ -301,6 +372,8 @@ TEST_F(TestMockDeepCopyImageCopyRequest, OutOfOrder) {
   librbd::MockTestImageCtx mock_dst_image_ctx(*m_dst_image_ctx);
   MockObjectCopyRequest mock_object_copy_request;
 
+  MockDiffRequest mock_diff_request;
+  expect_diff_send(mock_diff_request, {}, -EINVAL);
   expect_get_image_size(mock_src_image_ctx,
                         object_count * (1 << m_src_image_ctx->order));
   expect_get_image_size(mock_src_image_ctx, 0);
@@ -368,6 +441,8 @@ TEST_F(TestMockDeepCopyImageCopyRequest, SnapshotSubset) {
   MockObjectCopyRequest mock_object_copy_request;
 
   InSequence seq;
+  MockDiffRequest mock_diff_request;
+  expect_diff_send(mock_diff_request, {}, -EINVAL);
   expect_get_image_size(mock_src_image_ctx, 1 << m_src_image_ctx->order);
   expect_get_image_size(mock_src_image_ctx, 0);
   expect_get_image_size(mock_src_image_ctx, 0);
@@ -400,6 +475,8 @@ TEST_F(TestMockDeepCopyImageCopyRequest, RestartPartialSync) {
   MockObjectCopyRequest mock_object_copy_request;
 
   InSequence seq;
+  MockDiffRequest mock_diff_request;
+  expect_diff_send(mock_diff_request, {}, -EINVAL);
   expect_get_image_size(mock_src_image_ctx, 2 * (1 << m_src_image_ctx->order));
   expect_get_image_size(mock_src_image_ctx, 0);
   expect_object_copy_send(mock_object_copy_request);
@@ -434,6 +511,8 @@ TEST_F(TestMockDeepCopyImageCopyRequest, Cancel) {
   MockObjectCopyRequest mock_object_copy_request;
 
   InSequence seq;
+  MockDiffRequest mock_diff_request;
+  expect_diff_send(mock_diff_request, {}, -EINVAL);
   expect_get_image_size(mock_src_image_ctx, 1 << m_src_image_ctx->order);
   expect_get_image_size(mock_src_image_ctx, 0);
   expect_object_copy_send(mock_object_copy_request);
@@ -470,6 +549,8 @@ TEST_F(TestMockDeepCopyImageCopyRequest, Cancel_Inflight_Sync) {
   MockObjectCopyRequest mock_object_copy_request;
 
   InSequence seq;
+  MockDiffRequest mock_diff_request;
+  expect_diff_send(mock_diff_request, {}, -EINVAL);
   expect_get_image_size(mock_src_image_ctx, 6 * (1 << m_src_image_ctx->order));
   expect_get_image_size(mock_src_image_ctx, m_image_size);