New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os/bluestore: try to unshare blobs for EC overwrite workload #14239

Merged
merged 4 commits into from Jun 5, 2017

Conversation

Projects
None yet
2 participants
@liewegas
Member

liewegas commented Mar 29, 2017

OSD EC write pattern clones a to-be-written range to a
gen object and then deletes it shortly after. Try to
unshare the original blob in the nogen object if it is
in the cache so reduce the shared blob tracking overhead
and copy-on-write behavior.

This doesn't make our write pattern perfect, by any
means, since a small overwrite will generally

  • clone the range to the gen object
  • write the same range, creating a new blob on the nogen object
  • remove the clone, and unshare the original blob

This doesn't fix that the overwrite was written
somewhere else and we probably have two competing blobs.
However, future small writes will find the alternate
blob in _do_small_write and reuse the same allocation,
which is something.

Show outdated Hide outdated src/os/bluestore/BlueStore.cc Outdated

@liewegas liewegas added the needs-qa label May 9, 2017

@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas May 9, 2017

Member

@ifed01 removed the MUTABLE flag, squashed the earlier fixes, and rebased.

Member

liewegas commented May 9, 2017

@ifed01 removed the MUTABLE flag, squashed the earlier fixes, and rebased.

@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas May 12, 2017

Member

@ifed01 mind taking another look?

Member

liewegas commented May 12, 2017

@ifed01 mind taking another look?

@ifed01

This comment has been minimized.

Show comment
Hide comment
@ifed01

ifed01 May 12, 2017

Contributor

will do next Monday.

Contributor

ifed01 commented May 12, 2017

will do next Monday.

cout << m << " " << r << std::endl;
maybe_unshared = true;
m.put(10, 30, &r, &maybe_unshared);
cout << m << " " << r << " " << (int)maybe_unshared << std::endl;

This comment has been minimized.

@ifed01

ifed01 May 15, 2017

Contributor

missed ASSERT_FALSE(maybe_unshared)?

@ifed01

ifed01 May 15, 2017

Contributor

missed ASSERT_FALSE(maybe_unshared)?

Show outdated Hide outdated src/os/bluestore/BlueStore.cc Outdated
@ifed01

ifed01 approved these changes May 15, 2017

LGTM except some nits

@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas May 16, 2017

Member
 0> 2017-05-16 03:23:54.279744 7f68ce185700 -1 /build/ceph-12.0.1-2422-g0d194cc/src/os/bluestore/BlueStore.cc: In function 'void BlueStore::Collection::load_shared_blob(BlueStore::SharedBlobRef)' thread 7f68ce185700 time 2017-05-16 03:23:54.276309

/build/ceph-12.0.1-2422-g0d194cc/src/os/bluestore/BlueStore.cc: 3050: FAILED assert(0 == "uh oh, missing shared_blob")

ceph version 12.0.1-2422-g0d194cc (0d194cc84cf22302e6f62c1cba71d85c99637937)
1: (ceph::__ceph_assert_fail(char const*, char const*, int, char const*)+0x10e) [0x55aa13934a9e]
2: (BlueStore::Collection::load_shared_blob(boost::intrusive_ptrBlueStore::SharedBlob)+0x6f5) [0x55aa137d3265]
3: (BlueStore::_wctx_finish(BlueStore::TransContext*, boost::intrusive_ptrBlueStore::Collection&, boost::intrusive_ptrBlueStore::Onode, BlueStore::WriteContext*, std::set<BlueStore::SharedBlob*, std::lessBlueStore::SharedBlob*, std::allocatorBlueStore::SharedBlob* >)+0x60f) [0x55aa137fa75f]
4: (BlueStore::_do_truncate(BlueStore::TransContext
, boost::intrusive_ptrBlueStore::Collection&, boost::intrusive_ptrBlueStore::Onode, unsigned long, std::set<BlueStore::SharedBlob*, std::lessBlueStore::SharedBlob*, std::allocatorBlueStore::SharedBlob* >)+0x19f) [0x55aa138055df]
5: (BlueStore::_do_remove(BlueStore::TransContext
, boost::intrusive_ptrBlueStore::Collection&, boost::intrusive_ptrBlueStore::Onode)+0xba) [0x55aa13805eda]
6: (BlueStore::_remove(BlueStore::TransContext*, boost::intrusive_ptrBlueStore::Collection&, boost::intrusive_ptrBlueStore::Onode&)+0x8a) [0x55aa1380768a]
7: (BlueStore::_txc_add_transaction(BlueStore::TransContext*, ObjectStore::Transaction*)+0x1917) [0x55aa13825d27]
8: (BlueStore::queue_transactions(ObjectStore::Sequencer*, std::vector<ObjectStore::Transaction, std::allocatorObjectStore::Transaction >&, boost::intrusive_ptr, ThreadPool::TPHandle*)+0x388) [0x55aa138268c8]

/a/sage-2017-05-16_02:42:05-rados-wip-sage-testing2---basic-smithi/1182375

Member

liewegas commented May 16, 2017

 0> 2017-05-16 03:23:54.279744 7f68ce185700 -1 /build/ceph-12.0.1-2422-g0d194cc/src/os/bluestore/BlueStore.cc: In function 'void BlueStore::Collection::load_shared_blob(BlueStore::SharedBlobRef)' thread 7f68ce185700 time 2017-05-16 03:23:54.276309

/build/ceph-12.0.1-2422-g0d194cc/src/os/bluestore/BlueStore.cc: 3050: FAILED assert(0 == "uh oh, missing shared_blob")

ceph version 12.0.1-2422-g0d194cc (0d194cc84cf22302e6f62c1cba71d85c99637937)
1: (ceph::__ceph_assert_fail(char const*, char const*, int, char const*)+0x10e) [0x55aa13934a9e]
2: (BlueStore::Collection::load_shared_blob(boost::intrusive_ptrBlueStore::SharedBlob)+0x6f5) [0x55aa137d3265]
3: (BlueStore::_wctx_finish(BlueStore::TransContext*, boost::intrusive_ptrBlueStore::Collection&, boost::intrusive_ptrBlueStore::Onode, BlueStore::WriteContext*, std::set<BlueStore::SharedBlob*, std::lessBlueStore::SharedBlob*, std::allocatorBlueStore::SharedBlob* >)+0x60f) [0x55aa137fa75f]
4: (BlueStore::_do_truncate(BlueStore::TransContext
, boost::intrusive_ptrBlueStore::Collection&, boost::intrusive_ptrBlueStore::Onode, unsigned long, std::set<BlueStore::SharedBlob*, std::lessBlueStore::SharedBlob*, std::allocatorBlueStore::SharedBlob* >)+0x19f) [0x55aa138055df]
5: (BlueStore::_do_remove(BlueStore::TransContext
, boost::intrusive_ptrBlueStore::Collection&, boost::intrusive_ptrBlueStore::Onode)+0xba) [0x55aa13805eda]
6: (BlueStore::_remove(BlueStore::TransContext*, boost::intrusive_ptrBlueStore::Collection&, boost::intrusive_ptrBlueStore::Onode&)+0x8a) [0x55aa1380768a]
7: (BlueStore::_txc_add_transaction(BlueStore::TransContext*, ObjectStore::Transaction*)+0x1917) [0x55aa13825d27]
8: (BlueStore::queue_transactions(ObjectStore::Sequencer*, std::vector<ObjectStore::Transaction, std::allocatorObjectStore::Transaction >&, boost::intrusive_ptr, ThreadPool::TPHandle*)+0x388) [0x55aa138268c8]

/a/sage-2017-05-16_02:42:05-rados-wip-sage-testing2---basic-smithi/1182375

liewegas added some commits Mar 29, 2017

os/bluestore: drop unused dirty_range arg
Signed-off-by: Sage Weil <sage@redhat.com>
os/bluestore/bluestore_types: fix bluestore_shared_blob_t printer
Signed-off-by: Sage Weil <sage@redhat.com>
os/bluestore: remove MUTABLE flag
This flag is confusingly redundant as it is equivalent to !compressed &&
!shared.  Remove the flag and reimplement is_mutable() in terms of the
other two flags.

Signed-off-by: Sage Weil <sage@redhat.com>
os/bluestore: try to unshare blobs from gen objects
OSD EC write pattern clones a to-be-written range to a
gen object and then deletes it shortly after.  Try to
unshare the original blob in the nogen object if it is
in the cache so reduce the shared blob tracking overhead
and copy-on-write behavior.

This doesn't make our write pattern perfect, by any
means, since a small overwrite will generally

 - clone the range to the gen object
 - write the same range, creating a new blob on the
   nogen object
 - remove the clone, and unshare the original blob

This doesn't fix that the overwrite was written
somewhere else and we probably have two competing blobs.
However, future small writes will find the alternate
blob in _do_small_write and reuse the same allocation,
which is something.

Signed-off-by: Sage Weil <sage@redhat.com>

@liewegas liewegas merged commit 080f343 into ceph:master Jun 5, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@liewegas liewegas deleted the liewegas:wip-bluestore-ec-clone branch Jun 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment