From: Jason Dillaman Date: Thu, 28 Jan 2021 23:30:16 +0000 (-0500) Subject: librbd/object_map: diff state machine should track object existence X-Git-Tag: v17.1.0~3006^2~9 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=b81cd2460de748c71210520f8c819895f257f0c7;p=ceph-ci.git librbd/object_map: diff state machine should track object existence The deep-copy snapshot-create state machine initializes the object-map state to non-existent for all objects. There was an assumption that the deep-copy object-copy state machine would always update the object map but that was being skipped for clean objects as an optimization. This change will support a future commit to run the object-copy state machine for existing objects. Signed-off-by: Jason Dillaman --- diff --git a/src/librbd/api/DiffIterate.cc b/src/librbd/api/DiffIterate.cc index b2ed9a82420..70f1074e8fa 100644 --- a/src/librbd/api/DiffIterate.cc +++ b/src/librbd/api/DiffIterate.cc @@ -30,12 +30,6 @@ namespace api { namespace { -enum ObjectDiffState { - OBJECT_DIFF_STATE_NONE = 0, - OBJECT_DIFF_STATE_UPDATED = 1, - OBJECT_DIFF_STATE_HOLE = 2 -}; - struct DiffContext { DiffIterate<>::Callback callback; void *callback_arg; @@ -282,7 +276,8 @@ int DiffIterate::execute() { ldout(cct, 20) << "object " << object << dendl; const uint64_t object_no = extents.front().objectno; - if (object_diff_state[object_no] == OBJECT_DIFF_STATE_NONE && + uint8_t diff_state = object_diff_state[object_no]; + if (diff_state == object_map::DIFF_STATE_HOLE && from_snap_id == 0 && !diff_context.parent_diff.empty()) { // no data in child object -- report parent diff instead for (auto& oe : extents) { @@ -300,9 +295,9 @@ int DiffIterate::execute() { } } } - } else if (object_diff_state[object_no] != OBJECT_DIFF_STATE_NONE) { - bool updated = (object_diff_state[object_no] == - OBJECT_DIFF_STATE_UPDATED); + } else if (diff_state == object_map::DIFF_STATE_HOLE_UPDATED || + diff_state == object_map::DIFF_STATE_DATA_UPDATED) { + bool updated = (diff_state == object_map::DIFF_STATE_DATA_UPDATED); for (auto& oe : extents) { r = m_callback(off + oe.offset, oe.length, updated, m_callback_arg); if (r < 0) { diff --git a/src/librbd/deep_copy/ImageCopyRequest.cc b/src/librbd/deep_copy/ImageCopyRequest.cc index c31f4b87ab8..4adb836a65b 100644 --- a/src/librbd/deep_copy/ImageCopyRequest.cc +++ b/src/librbd/deep_copy/ImageCopyRequest.cc @@ -184,7 +184,7 @@ int ImageCopyRequest::send_next_object_copy() { bool skip = true; for (auto src_ono : src_objects) { if (src_ono >= m_object_diff_state.size() || - m_object_diff_state[src_ono] != object_map::DIFF_STATE_NONE) { + m_object_diff_state[src_ono] != object_map::DIFF_STATE_HOLE) { skip = false; break; } diff --git a/src/librbd/object_map/DiffRequest.cc b/src/librbd/object_map/DiffRequest.cc index 1058288a6ba..a8331bc5afc 100644 --- a/src/librbd/object_map/DiffRequest.cc +++ b/src/librbd/object_map/DiffRequest.cc @@ -175,58 +175,67 @@ void DiffRequest::handle_load_object_map(int r) { m_object_map.resize(num_objs); } - if (m_object_diff_state->size() < num_objs) { + size_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 - // map + // diff state m_object_map.resize(m_object_diff_state->size()); } - uint64_t overlap = std::min(m_object_map.size(), m_prev_object_map.size()); + uint64_t overlap = std::min(m_object_map.size(), prev_object_diff_state_size); auto it = m_object_map.begin(); auto overlap_end_it = it + overlap; - auto pre_it = m_prev_object_map.begin(); auto diff_it = m_object_diff_state->begin(); uint64_t i = 0; - for (; it != overlap_end_it; ++it, ++pre_it, ++diff_it, ++i) { - ldout(cct, 20) << "object state: " << i << " " - << static_cast(*pre_it) - << "->" << static_cast(*it) << dendl; - if (*it == OBJECT_NONEXISTENT) { - if (*pre_it != OBJECT_NONEXISTENT) { - *diff_it = DIFF_STATE_HOLE; - } - } else if (*it == OBJECT_EXISTS || - (*pre_it != *it && - !(*pre_it == OBJECT_EXISTS && - *it == OBJECT_EXISTS_CLEAN))) { - *diff_it = DIFF_STATE_UPDATED; + for (; it != overlap_end_it; ++it, ++diff_it, ++i) { + uint8_t object_map_state = *it; + uint8_t prev_object_diff_state = *diff_it; + if (object_map_state == OBJECT_EXISTS || + object_map_state == OBJECT_PENDING || + (object_map_state == OBJECT_EXISTS_CLEAN && + prev_object_diff_state != DIFF_STATE_DATA && + prev_object_diff_state != DIFF_STATE_DATA_UPDATED)) { + *diff_it = DIFF_STATE_DATA_UPDATED; + } else if (object_map_state == OBJECT_NONEXISTENT && + prev_object_diff_state != DIFF_STATE_HOLE && + prev_object_diff_state != DIFF_STATE_HOLE_UPDATED) { + *diff_it = DIFF_STATE_HOLE_UPDATED; } + + ldout(cct, 20) << "object state: " << i << " " + << static_cast(prev_object_diff_state) + << "->" << static_cast(*diff_it) << " (" + << static_cast(object_map_state) << ")" + << dendl; } ldout(cct, 20) << "computed overlap diffs" << dendl; bool diff_from_start = (m_snap_id_start == 0); auto end_it = m_object_map.end(); - if (m_object_map.size() > m_prev_object_map.size() && - (diff_from_start || m_prev_object_map_valid)) { + if (m_object_map.size() > prev_object_diff_state_size) { for (; it != end_it; ++it,++diff_it, ++i) { - ldout(cct, 20) << "object state: " << i << " " - << "->" << static_cast(*it) << dendl; - if (*it == OBJECT_NONEXISTENT) { - *diff_it = DIFF_STATE_NONE; + uint8_t object_map_state = *it; + if (object_map_state == OBJECT_NONEXISTENT) { + *diff_it = DIFF_STATE_HOLE; + } else if (diff_from_start || object_map_state != OBJECT_EXISTS_CLEAN) { + *diff_it = DIFF_STATE_DATA_UPDATED; } else { - *diff_it = DIFF_STATE_UPDATED; + *diff_it = DIFF_STATE_DATA; } + + ldout(cct, 20) << "object state: " << i << " " + << "->" << static_cast(*diff_it) << " (" + << static_cast(*it) << ")" << dendl; } } ldout(cct, 20) << "computed resize diffs" << dendl; - m_prev_object_map = m_object_map; - m_prev_object_map_valid = true; + m_object_diff_state_valid = true; std::shared_lock image_locker{m_image_ctx->image_lock}; load_object_map(&image_locker); diff --git a/src/librbd/object_map/DiffRequest.h b/src/librbd/object_map/DiffRequest.h index 31a7a1bea25..e83a1629e62 100644 --- a/src/librbd/object_map/DiffRequest.h +++ b/src/librbd/object_map/DiffRequest.h @@ -68,8 +68,7 @@ private: uint64_t m_current_size = 0; BitVector<2> m_object_map; - BitVector<2> m_prev_object_map; - bool m_prev_object_map_valid = false; + bool m_object_diff_state_valid = false; bufferlist m_out_bl; diff --git a/src/librbd/object_map/Types.h b/src/librbd/object_map/Types.h index b3741929d77..0ce91bd96a1 100644 --- a/src/librbd/object_map/Types.h +++ b/src/librbd/object_map/Types.h @@ -8,9 +8,10 @@ namespace librbd { namespace object_map { enum DiffState { - DIFF_STATE_NONE = 0, - DIFF_STATE_UPDATED = 1, - DIFF_STATE_HOLE = 2 + DIFF_STATE_HOLE = 0, /* unchanged hole */ + DIFF_STATE_DATA = 1, /* unchanged data */ + DIFF_STATE_HOLE_UPDATED = 2, /* new hole */ + DIFF_STATE_DATA_UPDATED = 3 /* new data */ }; } // namespace object_map diff --git a/src/test/librbd/object_map/test_mock_DiffRequest.cc b/src/test/librbd/object_map/test_mock_DiffRequest.cc index 67d30ee476e..f90d488a555 100644 --- a/src/test/librbd/object_map/test_mock_DiffRequest.cc +++ b/src/test/librbd/object_map/test_mock_DiffRequest.cc @@ -186,9 +186,9 @@ TEST_F(TestMockObjectMapDiffRequest, FullDelta) { BitVector<2> expected_diff_state; expected_diff_state.resize(object_count); - expected_diff_state[1] = DIFF_STATE_UPDATED; - expected_diff_state[2] = DIFF_STATE_UPDATED; - expected_diff_state[3] = DIFF_STATE_HOLE; + expected_diff_state[1] = DIFF_STATE_DATA_UPDATED; + expected_diff_state[2] = DIFF_STATE_DATA_UPDATED; + expected_diff_state[3] = DIFF_STATE_HOLE_UPDATED; ASSERT_EQ(expected_diff_state, m_object_diff_state); } @@ -233,8 +233,9 @@ TEST_F(TestMockObjectMapDiffRequest, IntermediateDelta) { BitVector<2> expected_diff_state; expected_diff_state.resize(object_count); - expected_diff_state[2] = DIFF_STATE_UPDATED; - expected_diff_state[3] = DIFF_STATE_UPDATED; + expected_diff_state[1] = DIFF_STATE_DATA; + expected_diff_state[2] = DIFF_STATE_DATA_UPDATED; + expected_diff_state[3] = DIFF_STATE_DATA_UPDATED; ASSERT_EQ(expected_diff_state, m_object_diff_state); } @@ -279,7 +280,9 @@ TEST_F(TestMockObjectMapDiffRequest, EndDelta) { BitVector<2> expected_diff_state; expected_diff_state.resize(object_count); - expected_diff_state[3] = DIFF_STATE_HOLE; + expected_diff_state[1] = DIFF_STATE_DATA; + expected_diff_state[2] = DIFF_STATE_DATA_UPDATED; + expected_diff_state[3] = DIFF_STATE_HOLE_UPDATED; ASSERT_EQ(expected_diff_state, m_object_diff_state); } @@ -370,7 +373,7 @@ TEST_F(TestMockObjectMapDiffRequest, IntermediateSnapDNE) { BitVector<2> expected_diff_state; expected_diff_state.resize(object_count); - expected_diff_state[1] = DIFF_STATE_UPDATED; + expected_diff_state[1] = DIFF_STATE_DATA_UPDATED; ASSERT_EQ(expected_diff_state, m_object_diff_state); } @@ -430,7 +433,7 @@ TEST_F(TestMockObjectMapDiffRequest, LoadIntermediateObjectMapDNE) { BitVector<2> expected_diff_state; expected_diff_state.resize(object_count); - expected_diff_state[1] = DIFF_STATE_UPDATED; + expected_diff_state[1] = DIFF_STATE_DATA_UPDATED; ASSERT_EQ(expected_diff_state, m_object_diff_state); }