librbd: fix snap create/rm may taking long time

fix snap create/rm may taking long time http://tracker.ceph.com/issues/22716 Signed-off-by: Song Shun <[email protected]>
mcronce · Jan 25, 2018 · d04ed34 · d04ed34
1 parent d079916
commit d04ed34
Show file tree

Hide file tree

Showing 8 changed files with 78 additions and 24 deletions.
diff --git a/src/cls/rbd/cls_rbd.cc b/src/cls/rbd/cls_rbd.cc
@@ -2669,9 +2669,11 @@ int object_map_snap_add(cls_method_context_t hctx, bufferlist *in,
   }
 
   bool updated = false;
-  for (uint64_t i = 0; i < object_map.size(); ++i) {
-    if (object_map[i] == OBJECT_EXISTS) {
-      object_map[i] = OBJECT_EXISTS_CLEAN;
+  auto it = object_map.begin();
+  auto end_it = object_map.end();
+  for (; it != end_it; ++it) {
+    if (*it == OBJECT_EXISTS) {
+      *it = OBJECT_EXISTS_CLEAN;
       updated = true;
     }
   }
@@ -2712,12 +2714,19 @@ int object_map_snap_remove(cls_method_context_t hctx, bufferlist *in,
   }
 
   bool updated = false;
-  for (uint64_t i = 0; i < dst_object_map.size(); ++i) {
-    if (dst_object_map[i] == OBJECT_EXISTS_CLEAN &&
-        (i >= src_object_map.size() || src_object_map[i] == OBJECT_EXISTS)) {
-      dst_object_map[i] = OBJECT_EXISTS;
+  auto src_it = src_object_map.begin();
+  auto dst_it = dst_object_map.begin();
+  auto dst_it_end = dst_object_map.end();
+  uint64_t i = 0;
+  for (; dst_it != dst_it_end; ++dst_it) {
+    if (*dst_it == OBJECT_EXISTS_CLEAN &&
+        (i >= src_object_map.size() || *src_it == OBJECT_EXISTS)) {
+      *dst_it = OBJECT_EXISTS;
       updated = true;
     }
+    if (i < src_object_map.size())
+      ++src_it;
+    ++i;
   }
 
   if (updated) {

diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc
@@ -141,6 +141,17 @@ void ObjectMap<I>::close(Context *on_finish) {
   req->send();
 }
 
+template <typename I>
+bool ObjectMap<I>::set_object_map(ceph::BitVector<2> &target_object_map) {
+  assert(m_image_ctx.owner_lock.is_locked());
+  assert(m_image_ctx.snap_lock.is_locked());
+  assert(m_image_ctx.test_features(RBD_FEATURE_OBJECT_MAP,
+                                   m_image_ctx.snap_lock));
+  RWLock::RLocker object_map_locker(m_image_ctx.object_map_lock);
+  m_object_map = target_object_map;
+  return true;
+}
+
 template <typename I>
 void ObjectMap<I>::rollback(uint64_t snap_id, Context *on_finish) {
   assert(m_image_ctx.snap_lock.is_locked());

diff --git a/src/librbd/ObjectMap.h b/src/librbd/ObjectMap.h
@@ -46,7 +46,7 @@ class ObjectMap {
 
   void open(Context *on_finish);
   void close(Context *on_finish);
-
+  bool set_object_map(ceph::BitVector<2> &target_object_map);
   bool object_may_exist(uint64_t object_no) const;
 
   void aio_save(Context *on_finish);

diff --git a/src/librbd/object_map/SnapshotCreateRequest.cc b/src/librbd/object_map/SnapshotCreateRequest.cc
@@ -135,10 +135,12 @@ bool SnapshotCreateRequest::send_add_snapshot() {
 void SnapshotCreateRequest::update_object_map() {
   RWLock::WLocker snap_locker(m_image_ctx.snap_lock);
   RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock);
-
-  for (uint64_t i = 0; i < m_object_map.size(); ++i) {
-    if (m_object_map[i] == OBJECT_EXISTS) {
-      m_object_map[i] = OBJECT_EXISTS_CLEAN;
+
+  auto it = m_object_map.begin();
+  auto end_it = m_object_map.end();
+  for (; it != end_it; ++it) {
+    if (*it == OBJECT_EXISTS) {
+      *it = OBJECT_EXISTS_CLEAN;
     }
   }
 }

diff --git a/src/librbd/object_map/SnapshotRemoveRequest.cc b/src/librbd/object_map/SnapshotRemoveRequest.cc
@@ -194,13 +194,21 @@ void SnapshotRemoveRequest::update_object_map() {
   if (m_next_snap_id == m_image_ctx.snap_id && m_next_snap_id == CEPH_NOSNAP) {
     CephContext *cct = m_image_ctx.cct;
     ldout(cct, 5) << this << " " << __func__ << dendl;
-
-    for (uint64_t i = 0; i < m_object_map.size(); ++i) {
-      if (m_object_map[i] == OBJECT_EXISTS_CLEAN &&
+
+    auto it = m_object_map.begin();
+    auto end_it = m_object_map.end();
+    auto snap_it = m_snap_object_map.begin();
+    uint64_t i = 0;
+    for (; it != end_it; ++it) {
+      if (*it == OBJECT_EXISTS_CLEAN &&
           (i >= m_snap_object_map.size() ||
-           m_snap_object_map[i] == OBJECT_EXISTS)) {
-        m_object_map[i] = OBJECT_EXISTS;
+           *snap_it == OBJECT_EXISTS)) {
+        *it = OBJECT_EXISTS;
+      }
+      if (i < m_snap_object_map.size()) {
+        ++snap_it;
       }
+      ++i;
     }
   }
 }

diff --git a/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc b/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc
@@ -268,26 +268,44 @@ TEST_F(TestMockObjectMapSnapshotRemoveRequest, ScrubCleanObjects) {
 
   librbd::ImageCtx *ictx;
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
+  librbd::NoOpProgressContext prog_ctx;
+  uint64_t size = 4294967296; // 4GB = 1024 * 4MB
+  ASSERT_EQ(0, resize(ictx, size));
+
+  // update image objectmap for snap inherit 
+  ceph::BitVector<2> object_map;
+  object_map.resize(1024);
+  for (uint64_t i = 512; i < object_map.size(); ++i) {
+    object_map[i] = i % 2 == 0 ? OBJECT_EXISTS : OBJECT_NONEXISTENT;
+  }
+
+  C_SaferCond cond_ctx1;
+  {
+    librbd::ObjectMap om(*ictx, ictx->snap_id);
+    RWLock::RLocker owner_locker(ictx->owner_lock);
+    RWLock::WLocker snap_locker(ictx->snap_lock);
+    om.set_object_map(object_map);
+    om.aio_save(&cond_ctx1);
+  }
+  ASSERT_EQ(0, cond_ctx1.wait());
   ASSERT_EQ(0, snap_create(*ictx, "snap1"));
   ASSERT_EQ(0, ictx->state->refresh_if_required());
 
-  uint64_t snap_id = ictx->snap_info.rbegin()->first;
-
-  ceph::BitVector<2> object_map;
-  object_map.resize(1024);
+  // simutate the image objectmap state after creating snap
   for (uint64_t i = 512; i < object_map.size(); ++i) {
     object_map[i] = i % 2 == 0 ? OBJECT_EXISTS_CLEAN : OBJECT_NONEXISTENT;
   }
 
-  C_SaferCond cond_ctx;
+  C_SaferCond cond_ctx2;
+  uint64_t snap_id = ictx->snap_info.rbegin()->first;
   AsyncRequest<> *request = new SnapshotRemoveRequest(
-    *ictx, &object_map, snap_id, &cond_ctx);
+    *ictx, &object_map, snap_id, &cond_ctx2);
   {
     RWLock::RLocker owner_locker(ictx->owner_lock);
     RWLock::WLocker snap_locker(ictx->snap_lock);
     request->send();
   }
-  ASSERT_EQ(0, cond_ctx.wait());
+  ASSERT_EQ(0, cond_ctx2.wait());
 
   for (uint64_t i = 512; i < object_map.size(); ++i) {
     ASSERT_EQ(i % 2 == 0 ? OBJECT_EXISTS : OBJECT_NONEXISTENT,

diff --git a/src/test/librbd/test_fixture.cc b/src/test/librbd/test_fixture.cc
@@ -98,6 +98,11 @@ int TestFixture::flatten(librbd::ImageCtx &ictx,
   return ictx.operations->flatten(prog_ctx);
 }
 
+int TestFixture::resize(librbd::ImageCtx *ictx, uint64_t size){
+  librbd::NoOpProgressContext prog_ctx;
+  return ictx->operations->resize(size, true, prog_ctx);
+}
+
 void TestFixture::close_image(librbd::ImageCtx *ictx) {
   m_ictxs.erase(ictx);
 

diff --git a/src/test/librbd/test_fixture.h b/src/test/librbd/test_fixture.h
@@ -30,6 +30,7 @@ class TestFixture : public ::testing::Test {
   int snap_protect(librbd::ImageCtx &ictx, const std::string &snap_name);
 
   int flatten(librbd::ImageCtx &ictx, librbd::ProgressContext &prog_ctx);
+  int resize(librbd::ImageCtx *ictx, uint64_t size);
 
   int lock_image(librbd::ImageCtx &ictx, ClsLockType lock_type,
                  const std::string &cookie);