From 2b03b30725f83ba5222f627fd660fcfbbfaed128 Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Sat, 23 Dec 2023 14:47:54 +0100 Subject: [PATCH] librbd/object_map: allow intermediate snaps to be skipped on diff-iterate In case of diff-iterate against the beginning of time, the result depends only on the end version. Loading and processing object maps or intermediate snapshots is redundant and can be skipped. This optimization is made possible by commit be507aaed15f ("librbd: diff-iterate shouldn't ever report "new hole" against a hole") and, to a lesser extent, the previous commit. Getting FastDiffInvalid, LoadObjectMapError and ObjectMapTooSmall to pass required tweaking not just expectations, but also start/end snap ids and thus also the meaning of these tests. This is addressed in the next commit. Fixes: https://tracker.ceph.com/issues/63341 Signed-off-by: Ilya Dryomov (cherry picked from commit 23c675f04a4b4bb00794bbc59493d9591625a0a7) --- src/librbd/object_map/DiffRequest.cc | 21 +- .../object_map/test_mock_DiffRequest.cc | 218 +++++++++++------- 2 files changed, 148 insertions(+), 91 deletions(-) diff --git a/src/librbd/object_map/DiffRequest.cc b/src/librbd/object_map/DiffRequest.cc index d14367010ed..4d23bcbad4d 100644 --- a/src/librbd/object_map/DiffRequest.cc +++ b/src/librbd/object_map/DiffRequest.cc @@ -38,16 +38,19 @@ void DiffRequest::send() { m_object_diff_state->clear(); - // collect all the snap ids in the provided range (inclusive) - if (m_snap_id_start != 0) { - m_snap_ids.insert(m_snap_id_start); - } - + // collect all the snap ids in the provided range (inclusive) unless + // this is diff-iterate against the beginning of time, in which case + // only the end version matters std::shared_lock image_locker{m_image_ctx->image_lock}; - auto snap_info_it = m_image_ctx->snap_info.upper_bound(m_snap_id_start); - auto snap_info_it_end = m_image_ctx->snap_info.lower_bound(m_snap_id_end); - for (; snap_info_it != snap_info_it_end; ++snap_info_it) { - m_snap_ids.insert(snap_info_it->first); + if (!m_diff_iterate_range || m_snap_id_start != 0) { + if (m_snap_id_start != 0) { + m_snap_ids.insert(m_snap_id_start); + } + auto snap_info_it = m_image_ctx->snap_info.upper_bound(m_snap_id_start); + auto snap_info_it_end = m_image_ctx->snap_info.lower_bound(m_snap_id_end); + for (; snap_info_it != snap_info_it_end; ++snap_info_it) { + m_snap_ids.insert(snap_info_it->first); + } } m_snap_ids.insert(m_snap_id_end); diff --git a/src/test/librbd/object_map/test_mock_DiffRequest.cc b/src/test/librbd/object_map/test_mock_DiffRequest.cc index 8d9ded985a2..8646f09354b 100644 --- a/src/test/librbd/object_map/test_mock_DiffRequest.cc +++ b/src/test/librbd/object_map/test_mock_DiffRequest.cc @@ -271,13 +271,15 @@ TEST_P(TestMockObjectMapDiffRequest, FastDiffDisabled) { } } -TEST_P(TestMockObjectMapDiffRequest, FastDiffInvalid) { +TEST_P(TestMockObjectMapDiffRequest, StartFastDiffInvalid) { REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF); uint32_t object_count = 5; m_image_ctx->size = object_count * (1 << m_image_ctx->order); m_image_ctx->snap_info = { {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}}, m_image_ctx->size, {}, + {}, {}, {}}}, + {2U, {"snap2", {cls::rbd::UserSnapshotNamespace{}}, m_image_ctx->size, {}, {}, {}, {}}} }; @@ -285,9 +287,9 @@ TEST_P(TestMockObjectMapDiffRequest, FastDiffInvalid) { expect_get_flags(mock_image_ctx, 1, RBD_FLAG_FAST_DIFF_INVALID, 0); }; if (is_diff_iterate()) { - ASSERT_EQ(-EINVAL, do_diff(get_flags, 0, CEPH_NOSNAP, true)); + ASSERT_EQ(-EINVAL, do_diff(get_flags, 1, 2, true)); } else { - ASSERT_EQ(-EINVAL, do_diff(get_flags, 0, CEPH_NOSNAP, false)); + ASSERT_EQ(-EINVAL, do_diff(get_flags, 1, 2, false)); } } @@ -349,15 +351,19 @@ TEST_P(TestMockObjectMapDiffRequest, FromBeginningToSnapIntermediateSnap) { } } - 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()) { + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, 2, 0, 0); + expect_load_map(mock_image_ctx, 2, object_map_2, 0); + }; test_diff_iterate(load, 0, 2, expected_diff_state); } else { + 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); + }; test_deep_copy(load, 0, 2, expected_diff_state); } } @@ -395,15 +401,19 @@ TEST_P(TestMockObjectMapDiffRequest, FromBeginningToSnapIntermediateSnapGrow) { expected_diff_state[i] = from_beginning_table[i - object_count_1][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()) { + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, 2, 0, 0); + expect_load_map(mock_image_ctx, 2, object_map_2, 0); + }; test_diff_iterate(load, 0, 2, expected_diff_state); } else { + 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); + }; test_deep_copy(load, 0, 2, expected_diff_state); } } @@ -429,15 +439,19 @@ TEST_P(TestMockObjectMapDiffRequest, FromBeginningToSnapIntermediateSnapGrowFrom expected_diff_state[i] = from_beginning_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()) { + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, 2, 0, 0); + expect_load_map(mock_image_ctx, 2, object_map_2, 0); + }; test_diff_iterate(load, 0, 2, expected_diff_state); } else { + 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); + }; test_deep_copy(load, 0, 2, expected_diff_state); } } @@ -481,15 +495,19 @@ TEST_P(TestMockObjectMapDiffRequest, FromBeginningToSnapIntermediateSnapShrink) } } - 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()) { + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, 2, 0, 0); + expect_load_map(mock_image_ctx, 2, object_map_2, 0); + }; test_diff_iterate(load, 0, 2, expected_diff_state); } else { + 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); + }; test_deep_copy(load, 0, 2, expected_diff_state); } } @@ -519,15 +537,19 @@ TEST_P(TestMockObjectMapDiffRequest, FromBeginningToSnapIntermediateSnapShrinkTo } } - 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()) { + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, 2, 0, 0); + expect_load_map(mock_image_ctx, 2, object_map_2, 0); + }; test_diff_iterate(load, 0, 2, expected_diff_state); } else { + 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); + }; test_deep_copy(load, 0, 2, expected_diff_state); } } @@ -584,15 +606,19 @@ TEST_P(TestMockObjectMapDiffRequest, FromBeginningToHeadIntermediateSnap) { } } - 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()) { + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, CEPH_NOSNAP, 0, 0); + expect_load_map(mock_image_ctx, CEPH_NOSNAP, object_map_head, 0); + }; test_diff_iterate(load, 0, CEPH_NOSNAP, expected_diff_state); } else { + 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); + }; test_deep_copy(load, 0, CEPH_NOSNAP, expected_diff_state); } } @@ -628,15 +654,19 @@ TEST_P(TestMockObjectMapDiffRequest, FromBeginningToHeadIntermediateSnapGrow) { expected_diff_state[i] = from_beginning_table[i - object_count_1][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()) { + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, CEPH_NOSNAP, 0, 0); + expect_load_map(mock_image_ctx, CEPH_NOSNAP, object_map_head, 0); + }; test_diff_iterate(load, 0, CEPH_NOSNAP, expected_diff_state); } else { + 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); + }; test_deep_copy(load, 0, CEPH_NOSNAP, expected_diff_state); } } @@ -660,15 +690,19 @@ TEST_P(TestMockObjectMapDiffRequest, FromBeginningToHeadIntermediateSnapGrowFrom expected_diff_state[i] = from_beginning_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()) { + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, CEPH_NOSNAP, 0, 0); + expect_load_map(mock_image_ctx, CEPH_NOSNAP, object_map_head, 0); + }; test_diff_iterate(load, 0, CEPH_NOSNAP, expected_diff_state); } else { + 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); + }; test_deep_copy(load, 0, CEPH_NOSNAP, expected_diff_state); } } @@ -710,15 +744,19 @@ TEST_P(TestMockObjectMapDiffRequest, FromBeginningToHeadIntermediateSnapShrink) } } - 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()) { + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, CEPH_NOSNAP, 0, 0); + expect_load_map(mock_image_ctx, CEPH_NOSNAP, object_map_head, 0); + }; test_diff_iterate(load, 0, CEPH_NOSNAP, expected_diff_state); } else { + 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); + }; test_deep_copy(load, 0, CEPH_NOSNAP, expected_diff_state); } } @@ -747,15 +785,19 @@ TEST_P(TestMockObjectMapDiffRequest, FromBeginningToHeadIntermediateSnapShrinkTo } } - 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()) { + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, CEPH_NOSNAP, 0, 0); + expect_load_map(mock_image_ctx, CEPH_NOSNAP, object_map_head, 0); + }; test_diff_iterate(load, 0, CEPH_NOSNAP, expected_diff_state); } else { + 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); + }; test_deep_copy(load, 0, CEPH_NOSNAP, expected_diff_state); } } @@ -1305,16 +1347,20 @@ TEST_P(TestMockObjectMapDiffRequest, IntermediateSnapDNE) { expected_diff_state.resize(object_count); expected_diff_state[1] = DIFF_STATE_DATA_UPDATED; - 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, - [&mock_image_ctx]() { mock_image_ctx.snap_info.erase(2); }); - 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()) { + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, CEPH_NOSNAP, 0, 0); + expect_load_map(mock_image_ctx, CEPH_NOSNAP, object_map_head, 0); + }; test_diff_iterate(load, 0, CEPH_NOSNAP, expected_diff_state); } else { + 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, + [&mock_image_ctx]() { mock_image_ctx.snap_info.erase(2); }); + expect_get_flags(mock_image_ctx, CEPH_NOSNAP, 0, 0); + expect_load_map(mock_image_ctx, CEPH_NOSNAP, object_map_head, 0); + }; test_deep_copy(load, 0, CEPH_NOSNAP, expected_diff_state); } } @@ -1356,26 +1402,32 @@ TEST_P(TestMockObjectMapDiffRequest, LoadIntermediateObjectMapDNE) { expected_diff_state.resize(object_count); expected_diff_state[1] = DIFF_STATE_DATA_UPDATED; - 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, -ENOENT); - 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()) { + auto load = [&](MockTestImageCtx& mock_image_ctx) { + expect_get_flags(mock_image_ctx, CEPH_NOSNAP, 0, 0); + expect_load_map(mock_image_ctx, CEPH_NOSNAP, object_map_head, 0); + }; test_diff_iterate(load, 0, CEPH_NOSNAP, expected_diff_state); } else { + 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, -ENOENT); + expect_get_flags(mock_image_ctx, CEPH_NOSNAP, 0, 0); + expect_load_map(mock_image_ctx, CEPH_NOSNAP, object_map_head, 0); + }; test_deep_copy(load, 0, CEPH_NOSNAP, expected_diff_state); } } -TEST_P(TestMockObjectMapDiffRequest, LoadObjectMapError) { +TEST_P(TestMockObjectMapDiffRequest, StartObjectMapLoadError) { REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF); uint32_t object_count = 5; m_image_ctx->size = object_count * (1 << m_image_ctx->order); m_image_ctx->snap_info = { {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}}, m_image_ctx->size, {}, + {}, {}, {}}}, + {2U, {"snap2", {cls::rbd::UserSnapshotNamespace{}}, m_image_ctx->size, {}, {}, {}, {}}} }; @@ -1386,19 +1438,21 @@ TEST_P(TestMockObjectMapDiffRequest, LoadObjectMapError) { expect_load_map(mock_image_ctx, 1, object_map_1, -EPERM); }; if (is_diff_iterate()) { - ASSERT_EQ(-EPERM, do_diff(load, 0, CEPH_NOSNAP, true)); + ASSERT_EQ(-EPERM, do_diff(load, 1, 2, true)); } else { - ASSERT_EQ(-EPERM, do_diff(load, 0, CEPH_NOSNAP, false)); + ASSERT_EQ(-EPERM, do_diff(load, 1, 2, false)); } } -TEST_P(TestMockObjectMapDiffRequest, ObjectMapTooSmall) { +TEST_P(TestMockObjectMapDiffRequest, StartObjectMapTooSmall) { REQUIRE_FEATURE(RBD_FEATURE_FAST_DIFF); uint32_t object_count = 5; m_image_ctx->size = object_count * (1 << m_image_ctx->order); m_image_ctx->snap_info = { {1U, {"snap1", {cls::rbd::UserSnapshotNamespace{}}, m_image_ctx->size, {}, + {}, {}, {}}}, + {2U, {"snap2", {cls::rbd::UserSnapshotNamespace{}}, m_image_ctx->size, {}, {}, {}, {}}} }; @@ -1410,9 +1464,9 @@ TEST_P(TestMockObjectMapDiffRequest, ObjectMapTooSmall) { expect_load_map(mock_image_ctx, 1, object_map_1, 0); }; if (is_diff_iterate()) { - ASSERT_EQ(-EINVAL, do_diff(load, 0, CEPH_NOSNAP, true)); + ASSERT_EQ(-EINVAL, do_diff(load, 1, 2, true)); } else { - ASSERT_EQ(-EINVAL, do_diff(load, 0, CEPH_NOSNAP, false)); + ASSERT_EQ(-EINVAL, do_diff(load, 1, 2, false)); } } -- 2.39.5