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: keep statfs replica in RAM to avoid expensive KV retrieval #15309

Merged
merged 4 commits into from Jun 8, 2017

Conversation

Projects
None yet
3 participants
@ifed01
Contributor

ifed01 commented May 26, 2017

Introduce some debug switches as well.
Signed-off-by: Igor Fedotov ifedotov@mirantis.com

@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas May 26, 2017

Member

retest this please

Member

liewegas commented May 26, 2017

retest this please

@@ -7445,6 +7454,11 @@ void BlueStore::_txc_update_store_statfs(TransContext *txc)
logger->inc(l_bluestore_compressed_allocated, txc->statfs_delta.compressed_allocated());
logger->inc(l_bluestore_compressed_original, txc->statfs_delta.compressed_original());
{
std::lock_guard<std::mutex> l(vstatfs_lock);

This comment has been minimized.

@varadakari

varadakari May 26, 2017

Contributor

@ifed01 did you do benchmark runs with this lock introduction? Mostly will be better than reading from DB, did we observe any improvement?

@varadakari

varadakari May 26, 2017

Contributor

@ifed01 did you do benchmark runs with this lock introduction? Mostly will be better than reading from DB, did we observe any improvement?

This comment has been minimized.

@ifed01

ifed01 May 30, 2017

Contributor

@varadakari - No, I don't expect any performance difference here - single lock acquisition is a pretty simple operation, we have tons of them along the write path. This was mainly to resolve Sage's complaint on significant amount of time spent in BlueStore::statfs func

@ifed01

ifed01 May 30, 2017

Contributor

@varadakari - No, I don't expect any performance difference here - single lock acquisition is a pretty simple operation, we have tons of them along the write path. This was mainly to resolve Sage's complaint on significant amount of time spent in BlueStore::statfs func

@varadakari

Rest looks good.

Show outdated Hide outdated src/os/bluestore/BlueStore.cc
@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas May 31, 2017

Member

rebase please?

Member

liewegas commented May 31, 2017

rebase please?

@ifed01

This comment has been minimized.

Show comment
Hide comment
@ifed01

ifed01 Jun 1, 2017

Contributor

rebased!

Contributor

ifed01 commented Jun 1, 2017

rebased!

@ifed01

This comment has been minimized.

Show comment
Hide comment
@ifed01

ifed01 Jun 1, 2017

Contributor

retest this please

Contributor

ifed01 commented Jun 1, 2017

retest this please

Igor Fedotov added some commits May 26, 2017

Igor Fedotov
os/bluestore: move volatile_statfs struct out of TransContext
Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
Igor Fedotov
os/bluestore: introduce a debug switch to bypass kv update.
Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
Igor Fedotov
os/bluestore: keep statfs replica in memory to avoid expensive KV access
Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
Igor Fedotov
os/bluestore: introduce debug parameter to bypass bdev verification
E.g. this allows to backup WAL/DB volumes after preconditioning and quickly rollback to that state when needed.

Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
@ifed01

This comment has been minimized.

Show comment
Hide comment
@ifed01

ifed01 Jun 8, 2017

Contributor

@liewegas - ping

Contributor

ifed01 commented Jun 8, 2017

@liewegas - ping

@liewegas liewegas merged commit de6b679 into ceph:master Jun 8, 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

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas Jun 8, 2017

Member

Thanks, I lost track of this!

Member

liewegas commented Jun 8, 2017

Thanks, I lost track of this!

@ifed01 ifed01 deleted the ifed01:wip-bluestore-boost-statfs branch Jun 13, 2017

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