Merged
Conversation
The typical read flow consists of 4 steps: - read meta, e.g., extent-map - read data from cache - read cache-missing data off the disk - decompress, verify checksum, and generate the final output stream Hence we split _do_read into corresponding small helpers, which are easy to maintain and for reuse. Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
Member
Author
|
@liewegas the test result looks fine: http://pulpito.ceph.com/xxg-2019-08-31_09:22:11-rados:thrash-wip-sparse-read-distro-basic-smithi/ the change set is actually small (the main big part is about the read func refactor) and should be easy to review |
Member
|
I like this much better! At first I thought it was weird that you have to call fiemap first... but I guess the recovery caller we care about already has that info on hand? It also struck me as a bit funny that the result is all concatenated into a single return bufferlist. Given that, I think |
b85797c to
9daffd3
Compare
Member
Author
yeah. actually build_push_op might call fiemap once and then split the whole fiemap into several parts for transmitting, e.g., due to the max-size limit of single push op.
I agree. Repushed. |
Currently only works for bluestore. The default version still reads each extent separately synchronously, but I think that should not be a concern? Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
However, this is only meaningful for ReplicatedBackend since ECBackend is already reading everything in asynchronous way. Also the osd_verify_sparse_read_holes option could be reliably dropped as it seems never (ever?) actually get a chance to be exercised ? Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
9daffd3 to
2296019
Compare
liewegas
approved these changes
Sep 4, 2019
Member
Author
|
retest this please |
3 tasks
tchaikov
reviewed
Sep 9, 2019
| req.regs.emplace_back(region_t(pos, b_off, l, front)); | ||
| r2r.emplace_back(std::move(req)); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
@xiexingguo i'd suggest avoid massively reformatting source code in future.
Member
Author
There was a problem hiding this comment.
sorry, will pay attention to next time
tchaikov
added a commit
to tchaikov/ceph
that referenced
this pull request
Sep 9, 2019
after ceph#30061, extents returned by sparse read will always be an empty map as long as the extents to be read is empty or the extent(s) in it are empty, even of the objectstorage does not support sparse read. Fixes: https://tracker.ceph.com/issues/41721 Signed-off-by: Kefu Chai <kchai@redhat.com>
3 tasks
tchaikov
added a commit
to tchaikov/ceph
that referenced
this pull request
Sep 9, 2019
after ceph#30061, extents returned by sparse read will always be an empty map as long as the extents to be read is empty or the extent(s) in it are empty, even of the objectstorage does not support sparse read. Fixes: https://tracker.ceph.com/issues/41721 Signed-off-by: Kefu Chai <kchai@redhat.com>
tchaikov
added a commit
to tchaikov/ceph
that referenced
this pull request
Sep 9, 2019
after ceph#30061, extents returned by sparse read will always be an empty map as long as the extents to be read is empty or the extent(s) in it are empty, even of the objectstorage does not support sparse read. Fixes: https://tracker.ceph.com/issues/41721 Signed-off-by: Kefu Chai <kchai@redhat.com>
pdvian
pushed a commit
to pdvian/ceph
that referenced
this pull request
Sep 12, 2019
after ceph#30061, extents returned by sparse read will always be an empty map as long as the extents to be read is empty or the extent(s) in it are empty, even of the objectstorage does not support sparse read. Fixes: https://tracker.ceph.com/issues/41721 Signed-off-by: Kefu Chai <kchai@redhat.com> (cherry picked from commit 855d84d)
hairesis
pushed a commit
to hairesis/ceph
that referenced
this pull request
Sep 18, 2019
after ceph#30061, extents returned by sparse read will always be an empty map as long as the extents to be read is empty or the extent(s) in it are empty, even of the objectstorage does not support sparse read. Fixes: https://tracker.ceph.com/issues/41721 Signed-off-by: Kefu Chai <kchai@redhat.com>
alfonsomthd
pushed a commit
to rhcs-dashboard/ceph
that referenced
this pull request
Sep 18, 2019
after ceph#30061, extents returned by sparse read will always be an empty map as long as the extents to be read is empty or the extent(s) in it are empty, even of the objectstorage does not support sparse read. Fixes: https://tracker.ceph.com/issues/41721 Signed-off-by: Kefu Chai <kchai@redhat.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Here is the background for the proposal: #29588
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard backendjenkins test docsjenkins render docs