os/bluestore: go deferred for 'big' writes. #33434
Conversation
src/os/bluestore/BlueStore.cc
Outdated
| bufferlist::iterator& blp, | ||
| WriteContext* wctx) | ||
| { | ||
| bluestore_deferred_op_t* op = _get_deferred_op(txc); |
There was a problem hiding this comment.
This function, if stripped of "big" namings, could be used as part of _do_write_small.
There was a problem hiding this comment.
That's right but in fact its usage from _do_write_small isn't that straightforward so I'd like to postpone this sort of cleanup for later refactoring.
Intended to ensure spinner's read performance for 4K min alloc size to be on par with 64K one. Degradation is caused by additional fragmentation produced by partial overwrites when 4K min alloc size is set./. E.g. consider 4K partial overwrite in the middle of the previously written contiguous 64K blob. For 64K min alloc size deferred write procedure comes in and preserve data continuity. When min alloc size set to 4K overwritten blob is broken into 3 parts whithout the patch. This patch applies deferred writing processing for the case and hence avoids blob continuity breakage. At cost of write performance drop though. Signed-off-by: Igor Fedotov <ifedotov@suse.com>
This allow different callback function signatures. Signed-off-by: Igor Fedotov <ifedotov@suse.com>
It makes no sense if affected blob's range is already non-continuous or full overwrite takes place. Signed-off-by: Igor Fedotov <ifedotov@suse.com>
| // will read some data to fill out the chunk? | ||
| head_read = p2phase<uint64_t>(b_off, chunk_size); | ||
| tail_read = p2nphase<uint64_t>(b_off + used, chunk_size); | ||
| b_off -= head_read; |
There was a problem hiding this comment.
Is it possible to get head_read != 0 or tail_read != 0 ?
I think that such cases will be caught by do_write_small.
There was a problem hiding this comment.
It's possible to have chunk size > alloc unit sometimes. E.g. when expected_write_size is provided, see _choose_write_options(...)
Hence we one can get head/tail at checksum chunk boundaries rather than at allocation unit ones. But do_write_small/do_write_big are distinguished on the latter only.
There was a problem hiding this comment.
Absolutely! More tests are always better :) Will add, thanks!
src/os/bluestore/BlueStore.cc
Outdated
| ceph_assert(blob_aligned_len() % chunk_size == 0); | ||
|
|
||
| res = blob_aligned_len() <= prefer_deferred_size && | ||
| blob_aligned_len() < ondisk && |
There was a problem hiding this comment.
If head_read can be !=0 or tail_read can be !=0 than condition should be blob_aligned_len() <= ondisk.
ifed01
force-pushed
the
wip-ifed-big-writes-deferred
branch
2 times, most recently
from
54facd2
to
511c208
Compare
Signed-off-by: Igor Fedotov <ifedotov@suse.com>
This applies to bluestore_write_small_deferred and bluestore_write_small_new counters. In fact they apply to both big and small writes. Signed-off-by: Igor Fedotov <ifedotov@suse.com>
Signed-off-by: Igor Fedotov <ifedotov@suse.com>
'bluestore_debug_omit_block_device_write' wasn't respected for 'big' deferred writes. Plus a minor cleanup around calc_csum calls. Signed-off-by: Igor Fedotov <ifedotov@suse.com>
ifed01
force-pushed
the
wip-ifed-big-writes-deferred
branch
from
511c208
to
9b92674
Compare
Signed-off-by: Adam Kupczyk <akupczyk@redhat.com>
|
If this is backported we will require #34754 |
|
How will we know if/when it gets backported? |
|
|
Was this backported to nautilus? |
|
Intended to ensure spinner's read performance for 4K min alloc size to be
on par with 64K one. Degradation is caused by additional fragmentation
produced by partial overwrites when 4K min alloc size is set.
E.g. consider 4K partial overwrite in the middle of the previously written contiguous 64K blob.
For 64K min alloc size deferred write procedure comes in and preserve data continuity.
When min alloc size set to 4K overwritten blob is broken into 3 parts
without the patch. This patch applies deferred writing processing for
the case and hence avoids blob continuity breakage. At cost of write performance drop though.
Signed-off-by: Igor Fedotov ifedotov@suse.com
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard backendjenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume tox