pacific: librbd: improve rbd_diff_iterate2() performance in fast-diff mode #55256
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Use a range where only snap_id_start is invalid. Signed-off-by: Ilya Dryomov <idryomov@gmail.com> (cherry picked from commit 64a5afc)
In preparation for multiple similarly configured MockTestImageCtx objects being used in a single test, centralize their creation and add a couple of helpers for setting expectations from a callback. Signed-off-by: Ilya Dryomov <idryomov@gmail.com> (cherry picked from commit 718f6b5)
Commit 399a45e ("librbd/object_map: rbd diff between two snapshots lists entire image content") fixed most of the damage caused by commit b81cd24 ("librbd/object_map: diff state machine should track object existence"), but the case of a "resize diff" when diffing from snapshot was missed. An area that was freshly allocated in image resize is the same in principle as a freshly created image and objects marked OBJECT_EXISTS_CLEAN are no exception. Diff for such objects in such an area should be set to DIFF_STATE_DATA_UPDATED, however currently when diffing from snapshot, it's set to DIFF_STATE_DATA. Signed-off-by: Ilya Dryomov <idryomov@gmail.com> (cherry picked from commit 34386d2)
The new "track the largest of all versions in the set, diff state is only ever grown" semantics introduced in commit 330f2a7 ("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 <idryomov@gmail.com> (cherry picked from commit 19c7c4a) Conflicts: src/librbd/object_map/DiffRequest.cc [ commit ef12761 ("librbd: use uint64 for return value of size()") not in pacific ]
…rate 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 be507aa ("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 <idryomov@gmail.com> (cherry picked from commit 23c675f)
For each covered edge case or error, run through the following scenarios: - where the edge case concerns snap_id_start - where the edge case concerns snap_id_end - where the edge case concerns intermediate snapshot and snap_id_start == 0 (diff against the beginning of time) - where the edge case concerns intermediate snapshot and snap_id_start != 0 (diff from snapshot) Signed-off-by: Ilya Dryomov <idryomov@gmail.com> (cherry picked from commit 9931282)
Currently diff-iterate in fast-diff mode is performed on the entire
image no matter what image extent is passed to the API. Then, unused
diff just gets discarded as DiffIterate ends up querying only objects
that the passed image extent maps to. This hasn't been an issue for
internal consumers ("rbd du", "rbd diff", etc) because they work on the
entire image, but turns out to lead to quadratic slowdown in some QEMU
use cases.
0..UINT64_MAX range is carved out for deep-copy which is unranged by
definition. To get effectively unranged diff-iterate, 0..UINT64_MAX-1
range can be used.
Fixes: https://tracker.ceph.com/issues/63341
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit 0b5ba5f)
Conflicts:
src/librbd/object_map/DiffRequest.cc [ commit ef12761
("librbd: use uint64 for return value of size()") not in
pacific ]
When getting parent diff, pass the overlap-reduced image extent instead of the entire 0..overlap range to avoid a similar quadratic slowdown on cloned images. Signed-off-by: Ilya Dryomov <idryomov@gmail.com> (cherry picked from commit 7677d4b) Conflicts: src/librbd/api/DiffIterate.cc [ ImageArea support not in pacific ]
It's totally broken: instead of returning the current position and moving to the next position, it returns the next position and doesn't move anywhere. Luckily it hasn't been used until now. Signed-off-by: Ilya Dryomov <idryomov@gmail.com> (cherry picked from commit 2ab5b52)
Currently it's done in two cases: - if the loaded object map is larger than expected based on byte size, it's truncated to expected number of objects - in case of deep-copy, if the loaded object map is smaller than diff state, it's expanded to get "track the largest of all versions in the set" semantics Both of these cases can be easily dealt with without modifying the object map. Being able to process a const object map is needed for working on in-memory object map which is external to DiffRequest. Signed-off-by: Ilya Dryomov <idryomov@gmail.com> (cherry picked from commit 275a299)
Signed-off-by: Ilya Dryomov <idryomov@gmail.com> (cherry picked from commit 232ad1a)
T (ConstIterator or Iterator) is confused with const T here: IteratorImpl dereference operator is wrongly overloaded on const and returns Reference instead of ConstReference for ConstIterator. This then fails inside bufferlist bowels because Reference is incompatible with bufferlist::const_iterator. Signed-off-by: Ilya Dryomov <idryomov@gmail.com> (cherry picked from commit 45d5345)
In preparation for potentially using in-memory object map, decouple object map processing from loading object maps and place the logic in prepare_for_object_map() and process_object_map(). Signed-off-by: Ilya Dryomov <idryomov@gmail.com> (cherry picked from commit dabb677)
If the object map for the end version is around (already loaded in memory, either due to the end version being a snapshot or due to exclusive lock being held), use it to run diff-iterate against the beginning of time. Since it's the only object map needed in that case, such calls would be satisfied locally. Fixes: https://tracker.ceph.com/issues/63341 Signed-off-by: Ilya Dryomov <idryomov@gmail.com> (cherry picked from commit 0c4bb58) Conflicts: src/test/librbd/mock/MockObjectMap.h [ commit 87459c2 ("librbd,rbd_mirror: do not include RWLock.h unless it is used") not in pacific ]
As an optimization, try to ensure that the object map for the end version is preloaded through the acquisition of exclusive lock and as a consequence remains around until exclusive lock is released. If it's not around, DiffRequest would (re)load it on each call. Signed-off-by: Ilya Dryomov <idryomov@gmail.com> (cherry picked from commit 89b0d9e) Conflicts: src/librbd/api/DiffIterate.cc [ ImageArea support not in pacific ]
Signed-off-by: Ilya Dryomov <idryomov@gmail.com> (cherry picked from commit 40e8813) Conflicts: PendingReleaseNotes [ moved to >=16.2.15 section ]
|
jenkins test docs |
trociny
approved these changes
Jan 21, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
backport tracker: https://tracker.ceph.com/issues/64108
backport of #55127
parent tracker: https://tracker.ceph.com/issues/63341