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 <song.shun3@zte.com.cn>
(cherry picked from commit d04ed348a1b20e5ea5bedada2462cb41f0f1d85a)

diff --git a/src/cls/rbd/cls_rbd.cc b/src/cls/rbd/cls_rbd.cc
index 6b960aa615a2613eae32d15abf6e8310e600ffab..7f78426a051fb1221f4f9c756f3094b1e13ae435 100644 (file)
--- a/src/cls/rbd/cls_rbd.cc
+++ b/src/cls/rbd/cls_rbd.cc
@@ -2462,9 +2462,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;
     }
   }
@@ -2505,12 +2507,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
index da855d02683c7391af97efc2fd63da664ba86f29..fcc19b46a9e5917b7e45cd496ee2acf303e3de12 100644 (file)
--- a/src/librbd/ObjectMap.cc
+++ b/src/librbd/ObjectMap.cc
@@ -135,6 +135,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
index 7096db716e6f4ade8f0951f9109eb39ec4ec21a1..801974b74aa2cb7e70781c85ec3f6e853594ba9b 100644 (file)
--- a/src/librbd/ObjectMap.h
+++ b/src/librbd/ObjectMap.h
@@ -47,7 +47,7 @@ public:
 
   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
index 5d77d2630d385a13bdb4bd375fbc085be91ea4c0..acee5ff1d306e2d12fd9b2a07b7538a17843aee2 100644 (file)
--- 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
index 821dd5b8781bb6137470c147032c16f3c171cc01..d5c7a3de1040447b10165ef0284fa16ef1c646e5 100644 (file)
--- 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
index c1fadcfb390434b25d8222ac38b1f8e7ae7a4c21..c7bc71610615fba764c80682fedb9503fd1cff4a 100644 (file)
--- a/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc
+++ b/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc
@@ -1,4 +1,4 @@
-// -*- mode:C; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
 // vim: ts=8 sw=2 smarttab
 
 #include "test/librbd/test_mock_fixture.h"
@@ -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
index d3283c5cee2d703336e438519cbf051724db83a1..3fb0c0159ffc386bfca725c57945dc1e3d9b2236 100644 (file)
--- a/src/test/librbd/test_fixture.cc
+++ b/src/test/librbd/test_fixture.cc
@@ -78,6 +78,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, 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
index a6fa6af77b76a0dcd6c3e5b1d7e1097a0e87f485..9e20b44ea56fe79c6518a397717392ab09096941 100644 (file)
--- a/src/test/librbd/test_fixture.h
+++ b/src/test/librbd/test_fixture.h
@@ -30,6 +30,7 @@ public:
   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);