From: Song Shun Date: Wed, 17 Jan 2018 01:32:45 +0000 (+0800) Subject: librbd: fix snap create/rm may taking long time X-Git-Tag: v10.2.11~47^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=f8ef84fc2a64364b60888364f137545de0cbfff5;p=ceph.git 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 (cherry picked from commit d04ed348a1b20e5ea5bedada2462cb41f0f1d85a) --- diff --git a/src/cls/rbd/cls_rbd.cc b/src/cls/rbd/cls_rbd.cc index 6b960aa615a26..7f78426a051fb 100644 --- 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 da855d02683c7..fcc19b46a9e59 100644 --- a/src/librbd/ObjectMap.cc +++ b/src/librbd/ObjectMap.cc @@ -135,6 +135,17 @@ void ObjectMap::close(Context *on_finish) { req->send(); } +template +bool ObjectMap::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 void ObjectMap::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 7096db716e6f4..801974b74aa2c 100644 --- 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 5d77d2630d385..acee5ff1d306e 100644 --- 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 821dd5b8781bb..d5c7a3de10404 100644 --- 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 c1fadcfb39043..c7bc71610615f 100644 --- 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 d3283c5cee2d7..3fb0c0159ffc3 100644 --- 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 a6fa6af77b76a..9e20b44ea56fe 100644 --- 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);