From d04ed348a1b20e5ea5bedada2462cb41f0f1d85a Mon Sep 17 00:00:00 2001 From: Song Shun Date: Wed, 17 Jan 2018 09:32:45 +0800 Subject: [PATCH] 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 --- src/cls/rbd/cls_rbd.cc | 23 +++++++++---- src/librbd/ObjectMap.cc | 11 +++++++ src/librbd/ObjectMap.h | 2 +- .../object_map/SnapshotCreateRequest.cc | 10 +++--- .../object_map/SnapshotRemoveRequest.cc | 18 ++++++++--- .../test_mock_SnapshotRemoveRequest.cc | 32 +++++++++++++++---- src/test/librbd/test_fixture.cc | 5 +++ src/test/librbd/test_fixture.h | 1 + 8 files changed, 78 insertions(+), 24 deletions(-) diff --git a/src/cls/rbd/cls_rbd.cc b/src/cls/rbd/cls_rbd.cc index 0bb3778fbacdf..46b50b65e7ac7 100644 --- 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 index 257554dc1718b..cfc15df629bc2 100644 --- a/src/librbd/ObjectMap.cc +++ b/src/librbd/ObjectMap.cc @@ -141,6 +141,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 ebd1a9ba3102d..f82f11b72b496 100644 --- a/src/librbd/ObjectMap.h +++ b/src/librbd/ObjectMap.h @@ -46,7 +46,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 a8b132206e105..aae615aabdc08 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 ab88028097ce9..0df878e13c395 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 941a865c4cac2..33c93cf94fd62 100644 --- 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 index 139c99875753b..a16e9f66d12c7 100644 --- 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 index 27eea61be442d..9bdbfcd27b9c8 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); -- 2.39.5