From: Ilya Dryomov Date: Fri, 22 Dec 2023 17:50:20 +0000 (+0100) Subject: librbd/object_map: resurrect diff-iterate behavior when image is shrunk X-Git-Tag: v18.2.4~277^2~14 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=84de1b1cd3c54da45a3168e75b570413c8b4906d;p=ceph.git librbd/object_map: resurrect diff-iterate behavior when image is shrunk The new "track the largest of all versions in the set, diff state is only ever grown" semantics introduced in commit 330f2a7bb94f ("librbd: helper state machine for computing diffs between object-maps") don't make sense for diff-iterate. It's a waste because DiffIterate won't query beyond the end version size -- this is baked into the API. Limit this behavior to deep-copy and resurrect the original behavior from 2015 for diff-iterate. Signed-off-by: Ilya Dryomov (cherry picked from commit 19c7c4a5359fa9c1d06cc11187e300251249ad9e) --- diff --git a/src/librbd/object_map/DiffRequest.cc b/src/librbd/object_map/DiffRequest.cc index 08a25b026527c..d14367010ed71 100644 --- a/src/librbd/object_map/DiffRequest.cc +++ b/src/librbd/object_map/DiffRequest.cc @@ -176,15 +176,21 @@ void DiffRequest::handle_load_object_map(int r) { } uint64_t prev_object_diff_state_size = m_object_diff_state->size(); - if (prev_object_diff_state_size < num_objs) { - // the diff state should be the largest of all snapshots in the set - m_object_diff_state->resize(num_objs); - } - if (m_object_map.size() < m_object_diff_state->size()) { - // the image was shrunk so expanding the object map will flag end objects - // as non-existent and they will be compared against the previous object - // diff state - m_object_map.resize(m_object_diff_state->size()); + if (m_diff_iterate_range) { + if (m_object_diff_state->size() != m_object_map.size()) { + m_object_diff_state->resize(m_object_map.size()); + } + } else { + // for deep-copy, the object diff state should be the largest of + // all versions in the set, so it's only ever grown + if (m_object_diff_state->size() < m_object_map.size()) { + m_object_diff_state->resize(m_object_map.size()); + } else if (m_object_diff_state->size() > m_object_map.size()) { + // the image was shrunk so expanding the object map will flag end objects + // as non-existent and they will be compared against the previous object + // diff state + m_object_map.resize(m_object_diff_state->size()); + } } uint64_t overlap = std::min(m_object_map.size(), prev_object_diff_state_size); diff --git a/src/test/librbd/object_map/test_mock_DiffRequest.cc b/src/test/librbd/object_map/test_mock_DiffRequest.cc index 74dd2215ff389..8d9ded985a266 100644 --- a/src/test/librbd/object_map/test_mock_DiffRequest.cc +++ b/src/test/librbd/object_map/test_mock_DiffRequest.cc @@ -150,6 +150,14 @@ static constexpr uint8_t from_snap_intermediate_table[][5] = { { OBJECT_EXISTS_CLEAN, OBJECT_EXISTS_CLEAN, OBJECT_EXISTS_CLEAN, DIFF_STATE_DATA, DIFF_STATE_DATA } }; +static constexpr uint8_t shrink_table[][2] = { + // shrunk deep-copy expected + { OBJECT_NONEXISTENT, DIFF_STATE_HOLE }, + { OBJECT_EXISTS, DIFF_STATE_HOLE_UPDATED }, + { OBJECT_PENDING, DIFF_STATE_HOLE_UPDATED }, + { OBJECT_EXISTS_CLEAN, DIFF_STATE_HOLE_UPDATED } +}; + class TestMockObjectMapDiffRequest : public TestMockFixture, public ::testing::WithParamInterface { public: @@ -434,6 +442,96 @@ TEST_P(TestMockObjectMapDiffRequest, FromBeginningToSnapIntermediateSnapGrowFrom } } +TEST_P(TestMockObjectMapDiffRequest, FromBeginningToSnapIntermediateSnapShrink) { + REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF); + + uint32_t object_count_2 = std::size(from_beginning_intermediate_table); + uint32_t object_count_1 = object_count_2 + std::size(shrink_table); + m_image_ctx->size = object_count_2 * (1 << m_image_ctx->order); + m_image_ctx->snap_info = { + {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}}, + object_count_1 * (1 << m_image_ctx->order), {}, {}, {}, {}}}, + {2U, {"snap2", {cls::rbd::UserSnapshotNamespace{}}, + object_count_2 * (1 << m_image_ctx->order), {}, {}, {}, {}}} + }; + + BitVector<2> object_map_1; + object_map_1.resize(object_count_1); + BitVector<2> object_map_2; + object_map_2.resize(object_count_2); + BitVector<2> expected_diff_state; + if (is_diff_iterate()) { + expected_diff_state.resize(object_count_2); + } else { + expected_diff_state.resize(object_count_1); + } + for (uint32_t i = 0; i < object_count_2; i++) { + object_map_1[i] = from_beginning_intermediate_table[i][0]; + object_map_2[i] = from_beginning_intermediate_table[i][1]; + if (is_diff_iterate()) { + expected_diff_state[i] = from_beginning_intermediate_table[i][2]; + } else { + expected_diff_state[i] = from_beginning_intermediate_table[i][3]; + } + } + for (uint32_t i = object_count_2; i < object_count_1; i++) { + object_map_1[i] = shrink_table[i - object_count_2][0]; + if (!is_diff_iterate()) { + expected_diff_state[i] = shrink_table[i - object_count_2][1]; + } + } + + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, 1, 0, 0); + expect_load_map(mock_image_ctx, 1, object_map_1, 0); + expect_get_flags(mock_image_ctx, 2, 0, 0); + expect_load_map(mock_image_ctx, 2, object_map_2, 0); + }; + if (is_diff_iterate()) { + test_diff_iterate(load, 0, 2, expected_diff_state); + } else { + test_deep_copy(load, 0, 2, expected_diff_state); + } +} + +TEST_P(TestMockObjectMapDiffRequest, FromBeginningToSnapIntermediateSnapShrinkToZero) { + REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF); + + uint32_t object_count_1 = std::size(shrink_table); + m_image_ctx->size = 0; + m_image_ctx->snap_info = { + {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}}, + object_count_1 * (1 << m_image_ctx->order), {}, {}, {}, {}}}, + {2U, {"snap2", {cls::rbd::UserSnapshotNamespace{}}, {}, {}, {}, {}, {}}} + }; + + BitVector<2> object_map_1; + object_map_1.resize(object_count_1); + BitVector<2> object_map_2; + BitVector<2> expected_diff_state; + if (!is_diff_iterate()) { + expected_diff_state.resize(object_count_1); + } + for (uint32_t i = 0; i < object_count_1; i++) { + object_map_1[i] = shrink_table[i][0]; + if (!is_diff_iterate()) { + expected_diff_state[i] = shrink_table[i][1]; + } + } + + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, 1, 0, 0); + expect_load_map(mock_image_ctx, 1, object_map_1, 0); + expect_get_flags(mock_image_ctx, 2, 0, 0); + expect_load_map(mock_image_ctx, 2, object_map_2, 0); + }; + if (is_diff_iterate()) { + test_diff_iterate(load, 0, 2, expected_diff_state); + } else { + test_deep_copy(load, 0, 2, expected_diff_state); + } +} + TEST_P(TestMockObjectMapDiffRequest, FromBeginningToHead) { REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF); @@ -575,6 +673,93 @@ TEST_P(TestMockObjectMapDiffRequest, FromBeginningToHeadIntermediateSnapGrowFrom } } +TEST_P(TestMockObjectMapDiffRequest, FromBeginningToHeadIntermediateSnapShrink) { + REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF); + + uint32_t object_count_head = std::size(from_beginning_intermediate_table); + uint32_t object_count_1 = object_count_head + std::size(shrink_table); + m_image_ctx->size = object_count_head * (1 << m_image_ctx->order); + m_image_ctx->snap_info = { + {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}}, + object_count_1 * (1 << m_image_ctx->order), {}, {}, {}, {}}} + }; + + BitVector<2> object_map_1; + object_map_1.resize(object_count_1); + BitVector<2> object_map_head; + object_map_head.resize(object_count_head); + BitVector<2> expected_diff_state; + if (is_diff_iterate()) { + expected_diff_state.resize(object_count_head); + } else { + expected_diff_state.resize(object_count_1); + } + for (uint32_t i = 0; i < object_count_head; i++) { + object_map_1[i] = from_beginning_intermediate_table[i][0]; + object_map_head[i] = from_beginning_intermediate_table[i][1]; + if (is_diff_iterate()) { + expected_diff_state[i] = from_beginning_intermediate_table[i][2]; + } else { + expected_diff_state[i] = from_beginning_intermediate_table[i][3]; + } + } + for (uint32_t i = object_count_head; i < object_count_1; i++) { + object_map_1[i] = shrink_table[i - object_count_head][0]; + if (!is_diff_iterate()) { + expected_diff_state[i] = shrink_table[i - object_count_head][1]; + } + } + + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, 1, 0, 0); + expect_load_map(mock_image_ctx, 1, object_map_1, 0); + expect_get_flags(mock_image_ctx, CEPH_NOSNAP, 0, 0); + expect_load_map(mock_image_ctx, CEPH_NOSNAP, object_map_head, 0); + }; + if (is_diff_iterate()) { + test_diff_iterate(load, 0, CEPH_NOSNAP, expected_diff_state); + } else { + test_deep_copy(load, 0, CEPH_NOSNAP, expected_diff_state); + } +} + +TEST_P(TestMockObjectMapDiffRequest, FromBeginningToHeadIntermediateSnapShrinkToZero) { + REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF); + + uint32_t object_count_1 = std::size(shrink_table); + m_image_ctx->size = 0; + m_image_ctx->snap_info = { + {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}}, + object_count_1 * (1 << m_image_ctx->order), {}, {}, {}, {}}} + }; + + BitVector<2> object_map_1; + object_map_1.resize(object_count_1); + BitVector<2> object_map_head; + BitVector<2> expected_diff_state; + if (!is_diff_iterate()) { + expected_diff_state.resize(object_count_1); + } + for (uint32_t i = 0; i < object_count_1; i++) { + object_map_1[i] = shrink_table[i][0]; + if (!is_diff_iterate()) { + expected_diff_state[i] = shrink_table[i][1]; + } + } + + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, 1, 0, 0); + expect_load_map(mock_image_ctx, 1, object_map_1, 0); + expect_get_flags(mock_image_ctx, CEPH_NOSNAP, 0, 0); + expect_load_map(mock_image_ctx, CEPH_NOSNAP, object_map_head, 0); + }; + if (is_diff_iterate()) { + test_diff_iterate(load, 0, CEPH_NOSNAP, expected_diff_state); + } else { + test_deep_copy(load, 0, CEPH_NOSNAP, expected_diff_state); + } +} + TEST_P(TestMockObjectMapDiffRequest, FromSnapToSnap) { REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF); @@ -688,6 +873,92 @@ TEST_P(TestMockObjectMapDiffRequest, FromSnapToSnapGrowFromZero) { } } +TEST_P(TestMockObjectMapDiffRequest, FromSnapToSnapShrink) { + REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF); + + uint32_t object_count_2 = std::size(from_snap_table); + uint32_t object_count_1 = object_count_2 + std::size(shrink_table); + m_image_ctx->size = object_count_2 * (1 << m_image_ctx->order); + m_image_ctx->snap_info = { + {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}}, + object_count_1 * (1 << m_image_ctx->order), {}, {}, {}, {}}}, + {2U, {"snap2", {cls::rbd::UserSnapshotNamespace{}}, + object_count_2 * (1 << m_image_ctx->order), {}, {}, {}, {}}} + }; + + BitVector<2> object_map_1; + object_map_1.resize(object_count_1); + BitVector<2> object_map_2; + object_map_2.resize(object_count_2); + BitVector<2> expected_diff_state; + if (is_diff_iterate()) { + expected_diff_state.resize(object_count_2); + } else { + expected_diff_state.resize(object_count_1); + } + for (uint32_t i = 0; i < object_count_2; i++) { + object_map_1[i] = from_snap_table[i][0]; + object_map_2[i] = from_snap_table[i][1]; + expected_diff_state[i] = from_snap_table[i][2]; + } + for (uint32_t i = object_count_2; i < object_count_1; i++) { + object_map_1[i] = shrink_table[i - object_count_2][0]; + if (!is_diff_iterate()) { + expected_diff_state[i] = shrink_table[i - object_count_2][1]; + } + } + + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, 1, 0, 0); + expect_load_map(mock_image_ctx, 1, object_map_1, 0); + expect_get_flags(mock_image_ctx, 2, 0, 0); + expect_load_map(mock_image_ctx, 2, object_map_2, 0); + }; + if (is_diff_iterate()) { + test_diff_iterate(load, 1, 2, expected_diff_state); + } else { + test_deep_copy(load, 1, 2, expected_diff_state); + } +} + +TEST_P(TestMockObjectMapDiffRequest, FromSnapToSnapShrinkToZero) { + REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF); + + uint32_t object_count_1 = std::size(shrink_table); + m_image_ctx->size = 0; + m_image_ctx->snap_info = { + {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}}, + object_count_1 * (1 << m_image_ctx->order), {}, {}, {}, {}}}, + {2U, {"snap2", {cls::rbd::UserSnapshotNamespace{}}, {}, {}, {}, {}, {}}} + }; + + BitVector<2> object_map_1; + object_map_1.resize(object_count_1); + BitVector<2> object_map_2; + BitVector<2> expected_diff_state; + if (!is_diff_iterate()) { + expected_diff_state.resize(object_count_1); + } + for (uint32_t i = 0; i < object_count_1; i++) { + object_map_1[i] = shrink_table[i][0]; + if (!is_diff_iterate()) { + expected_diff_state[i] = shrink_table[i][1]; + } + } + + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, 1, 0, 0); + expect_load_map(mock_image_ctx, 1, object_map_1, 0); + expect_get_flags(mock_image_ctx, 2, 0, 0); + expect_load_map(mock_image_ctx, 2, object_map_2, 0); + }; + if (is_diff_iterate()) { + test_diff_iterate(load, 1, 2, expected_diff_state); + } else { + test_deep_copy(load, 1, 2, expected_diff_state); + } +} + TEST_P(TestMockObjectMapDiffRequest, FromSnapToSnapIntermediateSnap) { REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF); @@ -843,6 +1114,89 @@ TEST_P(TestMockObjectMapDiffRequest, FromSnapToHeadGrowFromZero) { } } +TEST_P(TestMockObjectMapDiffRequest, FromSnapToHeadShrink) { + REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF); + + uint32_t object_count_head = std::size(from_snap_table); + uint32_t object_count_1 = object_count_head + std::size(shrink_table); + m_image_ctx->size = object_count_head * (1 << m_image_ctx->order); + m_image_ctx->snap_info = { + {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}}, + object_count_1 * (1 << m_image_ctx->order), {}, {}, {}, {}}} + }; + + BitVector<2> object_map_1; + object_map_1.resize(object_count_1); + BitVector<2> object_map_head; + object_map_head.resize(object_count_head); + BitVector<2> expected_diff_state; + if (is_diff_iterate()) { + expected_diff_state.resize(object_count_head); + } else { + expected_diff_state.resize(object_count_1); + } + for (uint32_t i = 0; i < object_count_head; i++) { + object_map_1[i] = from_snap_table[i][0]; + object_map_head[i] = from_snap_table[i][1]; + expected_diff_state[i] = from_snap_table[i][2]; + } + for (uint32_t i = object_count_head; i < object_count_1; i++) { + object_map_1[i] = shrink_table[i - object_count_head][0]; + if (!is_diff_iterate()) { + expected_diff_state[i] = shrink_table[i - object_count_head][1]; + } + } + + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, 1, 0, 0); + expect_load_map(mock_image_ctx, 1, object_map_1, 0); + expect_get_flags(mock_image_ctx, CEPH_NOSNAP, 0, 0); + expect_load_map(mock_image_ctx, CEPH_NOSNAP, object_map_head, 0); + }; + if (is_diff_iterate()) { + test_diff_iterate(load, 1, CEPH_NOSNAP, expected_diff_state); + } else { + test_deep_copy(load, 1, CEPH_NOSNAP, expected_diff_state); + } +} + +TEST_P(TestMockObjectMapDiffRequest, FromSnapToHeadShrinkToZero) { + REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF); + + uint32_t object_count_1 = std::size(shrink_table); + m_image_ctx->size = 0; + m_image_ctx->snap_info = { + {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}}, + object_count_1 * (1 << m_image_ctx->order), {}, {}, {}, {}}} + }; + + BitVector<2> object_map_1; + object_map_1.resize(object_count_1); + BitVector<2> object_map_head; + BitVector<2> expected_diff_state; + if (!is_diff_iterate()) { + expected_diff_state.resize(object_count_1); + } + for (uint32_t i = 0; i < object_count_1; i++) { + object_map_1[i] = shrink_table[i][0]; + if (!is_diff_iterate()) { + expected_diff_state[i] = shrink_table[i][1]; + } + } + + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, 1, 0, 0); + expect_load_map(mock_image_ctx, 1, object_map_1, 0); + expect_get_flags(mock_image_ctx, CEPH_NOSNAP, 0, 0); + expect_load_map(mock_image_ctx, CEPH_NOSNAP, object_map_head, 0); + }; + if (is_diff_iterate()) { + test_diff_iterate(load, 1, CEPH_NOSNAP, expected_diff_state); + } else { + test_deep_copy(load, 1, CEPH_NOSNAP, expected_diff_state); + } +} + TEST_P(TestMockObjectMapDiffRequest, FromSnapToHeadIntermediateSnap) { REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF);