From 89d2a13db67945bfd7c6d6fe79584f00574b8ba3 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 3 Feb 2021 13:21:34 -0500 Subject: [PATCH] librbd/io: track object non-existence when computing snapshot deltas Re-use the existing DNE state to track whether or not the object already exists when computing snapshot deltas from an arbitrary set of snapshots. Previously, the non-existence of the object was only computed for snap id 0 for tracking whiteouts. In a future commit, the deep-copy object-copy state machine will be able to properly update the object-map state to indicate exists clean vs non-existent state. Signed-off-by: Jason Dillaman --- src/librbd/api/DiffIterate.cc | 5 ++- src/librbd/deep_copy/ObjectCopyRequest.cc | 15 +++---- src/librbd/io/ObjectRequest.cc | 32 +++++++-------- src/librbd/io/ObjectRequest.h | 2 +- src/test/librbd/io/test_mock_ObjectRequest.cc | 39 ++++++++++++++++++- 5 files changed, 64 insertions(+), 29 deletions(-) diff --git a/src/librbd/api/DiffIterate.cc b/src/librbd/api/DiffIterate.cc index 70f1074e8fa..dcd07ea56a3 100644 --- a/src/librbd/api/DiffIterate.cc +++ b/src/librbd/api/DiffIterate.cc @@ -122,8 +122,9 @@ private: auto state = snapshot_extent.get_val().state; // ignore DNE object (and parent) - if (key == io::WriteReadSnapIds{0, 0} && - state != io::SPARSE_EXTENT_STATE_DATA) { + if ((state == io::SPARSE_EXTENT_STATE_DNE) || + (key == io::INITIAL_WRITE_READ_SNAP_IDS && + state == io::SPARSE_EXTENT_STATE_ZEROED)) { continue; } diff --git a/src/librbd/deep_copy/ObjectCopyRequest.cc b/src/librbd/deep_copy/ObjectCopyRequest.cc index 80b06c15cfb..3c8c596b19d 100644 --- a/src/librbd/deep_copy/ObjectCopyRequest.cc +++ b/src/librbd/deep_copy/ObjectCopyRequest.cc @@ -465,7 +465,7 @@ void ObjectCopyRequest::compute_read_ops() { io::WriteReadSnapIds write_read_snap_ids{key}; // advance the src write snap id to the first valid snap id - if (write_read_snap_ids != io::INITIAL_WRITE_READ_SNAP_IDS) { + if (write_read_snap_ids.first > m_src_snap_id_start) { // don't attempt to read from snapshots that shouldn't exist in // case the OSD fails to give a correct snap list auto snap_map_it = m_snap_map.find(write_read_snap_ids.first); @@ -485,10 +485,10 @@ void ObjectCopyRequest::compute_read_ops() { auto state = image_interval.get_val().state; switch (state) { case io::SPARSE_EXTENT_STATE_DNE: - ceph_assert(write_read_snap_ids == io::INITIAL_WRITE_READ_SNAP_IDS); - if (read_from_parent) { - // special-case for DNE object-extents since when flattening we need - // to read data from the parent images extents + if (write_read_snap_ids == io::INITIAL_WRITE_READ_SNAP_IDS && + read_from_parent) { + // special-case for DNE initial object-extents since when flattening + // we need to read data from the parent images extents ldout(m_cct, 20) << "DNE extent: " << image_interval.get_off() << "~" << image_interval.get_len() << dendl; @@ -626,14 +626,15 @@ void ObjectCopyRequest::compute_zero_ops() { auto state = image_interval.get_val().state; switch (state) { case io::SPARSE_EXTENT_STATE_ZEROED: - if (write_read_snap_ids != io::WriteReadSnapIds{0, 0}) { + if (write_read_snap_ids.first > m_src_snap_id_start) { ldout(m_cct, 20) << "zeroed extent: " << "src_snap_seq=" << src_snap_seq << " " << image_interval.get_off() << "~" << image_interval.get_len() << dendl; m_dst_zero_interval[src_snap_seq].union_insert( image_interval.get_off(), image_interval.get_len()); - } else if (hide_parent) { + } else if (write_read_snap_ids == io::INITIAL_WRITE_READ_SNAP_IDS && + hide_parent) { auto first_src_snap_id = m_snap_map.begin()->first; ldout(m_cct, 20) << "zeroed (hide parent) extent: " << "src_snap_seq=" << first_src_snap_id << " " diff --git a/src/librbd/io/ObjectRequest.cc b/src/librbd/io/ObjectRequest.cc index 2bb3f523450..e4ee152cf36 100644 --- a/src/librbd/io/ObjectRequest.cc +++ b/src/librbd/io/ObjectRequest.cc @@ -784,9 +784,14 @@ void ObjectListSnapsRequest::handle_list_snaps(int r) { m_snapshot_delta->clear(); auto& snapshot_delta = *m_snapshot_delta; + ceph_assert(!m_snap_ids.empty()); + librados::snap_t start_snap_id = 0; + librados::snap_t first_snap_id = *m_snap_ids.begin(); + librados::snap_t last_snap_id = *m_snap_ids.rbegin(); + if (r == -ENOENT) { // the object does not exist -- mark the missing extents - zero_initial_extent(true); + zero_extent(first_snap_id, true); list_from_parent(); return; } else if (r < 0) { @@ -800,11 +805,6 @@ void ObjectListSnapsRequest::handle_list_snaps(int r) { librados::snap_set_t snap_set; convert_snap_set(m_snap_set, &snap_set); - ceph_assert(!m_snap_ids.empty()); - librados::snap_t start_snap_id = 0; - librados::snap_t first_snap_id = *m_snap_ids.begin(); - librados::snap_t last_snap_id = *m_snap_ids.rbegin(); - bool initial_extents_written = false; interval_set object_interval; @@ -924,7 +924,7 @@ void ObjectListSnapsRequest::handle_list_snaps(int r) { bool snapshot_delta_empty = snapshot_delta.empty(); if (!initial_extents_written) { - zero_initial_extent(false); + zero_extent(first_snap_id, first_snap_id > 0); } ldout(cct, 20) << "snapshot_delta=" << snapshot_delta << dendl; @@ -1003,8 +1003,7 @@ void ObjectListSnapsRequest::handle_list_from_parent(int r) { << "parent_snapshot_delta=" << m_parent_snapshot_delta << dendl; - // ignore special-case of fully empty dataset - m_parent_snapshot_delta.erase(INITIAL_WRITE_READ_SNAP_IDS); + // ignore special-case of fully empty dataset (we ignore zeroes) if (m_parent_snapshot_delta.empty()) { this->finish(0); return; @@ -1036,27 +1035,24 @@ void ObjectListSnapsRequest::handle_list_from_parent(int r) { } template -void ObjectListSnapsRequest::zero_initial_extent(bool dne) { +void ObjectListSnapsRequest::zero_extent(uint64_t snap_id, bool dne) { I *image_ctx = this->m_ictx; auto cct = image_ctx->cct; - ceph_assert(!m_snap_ids.empty()); - librados::snap_t snap_id_start = *m_snap_ids.begin(); - // the object does not exist or is (partially) under whiteout -- mark the // missing extents which would be any portion of the object that does not // have data in the initial snapshot set - if ((snap_id_start == 0) && - ((m_list_snaps_flags & LIST_SNAPS_FLAG_IGNORE_ZEROED_EXTENTS) == 0)) { + if ((m_list_snaps_flags & LIST_SNAPS_FLAG_IGNORE_ZEROED_EXTENTS) == 0) { interval_set interval; for (auto [object_offset, object_length] : m_object_extents) { interval.insert(object_offset, object_length); } for (auto [offset, length] : interval) { - ldout(cct, 20) << "zeroing initial extent " << offset << "~" << length - << dendl; - (*m_snapshot_delta)[INITIAL_WRITE_READ_SNAP_IDS].insert( + ldout(cct, 20) << "snapshot " << snap_id << ": " + << (dne ? "DNE" : "zeroed") << " extent " + << offset << "~" << length << dendl; + (*m_snapshot_delta)[{snap_id, snap_id}].insert( offset, length, SparseExtent( (dne ? SPARSE_EXTENT_STATE_DNE : SPARSE_EXTENT_STATE_ZEROED), diff --git a/src/librbd/io/ObjectRequest.h b/src/librbd/io/ObjectRequest.h index 8ba244b3d46..89ca224cc90 100644 --- a/src/librbd/io/ObjectRequest.h +++ b/src/librbd/io/ObjectRequest.h @@ -484,7 +484,7 @@ private: void list_from_parent(); void handle_list_from_parent(int r); - void zero_initial_extent(bool dne); + void zero_extent(uint64_t snap_id, bool dne); }; } // namespace io diff --git a/src/test/librbd/io/test_mock_ObjectRequest.cc b/src/test/librbd/io/test_mock_ObjectRequest.cc index 18069b9c99d..c9fa33c43e1 100644 --- a/src/test/librbd/io/test_mock_ObjectRequest.cc +++ b/src/test/librbd/io/test_mock_ObjectRequest.cc @@ -1777,7 +1777,7 @@ TEST_F(TestMockIoObjectRequest, ListSnaps) { ASSERT_EQ(expected_snapshot_delta, snapshot_delta); } -TEST_F(TestMockIoObjectRequest, ListSnapsDNE) { +TEST_F(TestMockIoObjectRequest, ListSnapsENOENT) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); @@ -1800,6 +1800,43 @@ TEST_F(TestMockIoObjectRequest, ListSnapsDNE) { ASSERT_EQ(expected_snapshot_delta, snapshot_delta); } +TEST_F(TestMockIoObjectRequest, ListSnapsDNE) { + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + mock_image_ctx.snaps = {2, 3, 4}; + + librados::snap_set_t snap_set; + snap_set.seq = 6; + librados::clone_info_t clone_info; + + clone_info.cloneid = 4; + clone_info.snaps = {3, 4}; + clone_info.overlap = std::vector>{ + {0, 4194304}}; + clone_info.size = 4194304; + snap_set.clones.push_back(clone_info); + + expect_list_snaps(mock_image_ctx, snap_set, 0); + + SnapshotDelta snapshot_delta; + C_SaferCond ctx; + auto req = MockObjectListSnapsRequest::create( + &mock_image_ctx, 0, + {{440320, 1024}}, + {2, 3, 4}, 0, {}, &snapshot_delta, &ctx); + req->send(); + ASSERT_EQ(0, ctx.wait()); + + SnapshotDelta expected_snapshot_delta; + expected_snapshot_delta[{2,2}].insert( + 440320, 1024, {SPARSE_EXTENT_STATE_DNE, 1024}); + expected_snapshot_delta[{3,4}].insert( + 440320, 1024, {SPARSE_EXTENT_STATE_DATA, 1024}); + ASSERT_EQ(expected_snapshot_delta, snapshot_delta); +} + TEST_F(TestMockIoObjectRequest, ListSnapsEmpty) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); -- 2.39.5