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: unify throttling model #14306

Merged
merged 5 commits into from Apr 7, 2017

Conversation

Projects
None yet
4 participants
@liewegas
Member

liewegas commented Apr 3, 2017

No description provided.

@liewegas liewegas requested a review from ivancich Apr 3, 2017

@ivancich

This comment has been minimized.

Show comment
Hide comment
@ivancich

ivancich Apr 4, 2017

Member

This looks good.

So cost per io is in units of bytes, in other words, how many bytes could have been written during an average prep/seek time?

Member

ivancich commented Apr 4, 2017

This looks good.

So cost per io is in units of bytes, in other words, how many bytes could have been written during an average prep/seek time?

@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas Apr 4, 2017

Member
Member

liewegas commented Apr 4, 2017

@ivancich

I really like this approach. And it'd be fairly easy to rebase the adaptive throttle work on top of this.

@markhpc

This comment has been minimized.

Show comment
Hide comment
@markhpc

markhpc Apr 5, 2017

Member

In tests last night this seemed to help on spinning disk, but I'm not sure it's helping (and might be hurting) on HDD/NVMe and NVMe only tests. Granted, these are only small preliminary tests on a single OSD.
preextend_tests.xlsx

Member

markhpc commented Apr 5, 2017

In tests last night this seemed to help on spinning disk, but I'm not sure it's helping (and might be hurting) on HDD/NVMe and NVMe only tests. Granted, these are only small preliminary tests on a single OSD.
preextend_tests.xlsx

liewegas added some commits Apr 3, 2017

os/bluestore: release throttle before commit
Signed-off-by: Sage Weil <sage@redhat.com>
os/bluestore: unify throttling model
Implement a super simple model for the cost of a transaction
for the purposes of throttling.  This replaces two independent
throttles (one for transaction ops, one for bytes) and puts
them both under the 'bytes' throttle.  The txc model cost is
expressed in terms of bytes as this is probably the simplest
thing for users to reason about.

Signed-off-by: Sage Weil <sage@redhat.com>
os/bluestore: bluestore_deferred_batch_ops = 32 (not 8)
A bit more batching.

Signed-off-by: Sage Weil <sage@redhat.com>
os/bluestore: do not wake kv thread if only deferred events pending
No need to wake up if there is only deferred work; we can do
it lazily.

Signed-off-by: Sage Weil <sage@redhat.com>
@@ -7786,12 +7784,19 @@ void BlueStore::_kv_sync_thread()
}
}
}
if (num_aios) {

This comment has been minimized.

@ifed01

ifed01 Apr 6, 2017

Contributor

why is this check removed?

@ifed01

ifed01 Apr 6, 2017

Contributor

why is this check removed?

This comment has been minimized.

@liewegas

liewegas Apr 6, 2017

Member

it was an optimization of dubious value (avoid iterating over the list if we know there are no aios), but with the throttle change we have to do the iteration anyway.

@liewegas

liewegas Apr 6, 2017

Member

it was an optimization of dubious value (avoid iterating over the list if we know there are no aios), but with the throttle change we have to do the iteration anyway.

@ifed01

ifed01 approved these changes Apr 6, 2017

@liewegas liewegas added the needs-qa label Apr 6, 2017

@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas Apr 6, 2017

Member

retest this please

Member

liewegas commented Apr 6, 2017

retest this please

@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas Apr 6, 2017

Member

retest this please

Member

liewegas commented Apr 6, 2017

retest this please

@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas Apr 6, 2017

Member

retest this please

Member

liewegas commented Apr 6, 2017

retest this please

os/bluestore: make deferred_aggressive a counter
Multiple threads may run _osr_drain_preceding; use a count to
prevent the first finisher from prematurely disabling
aggressive mode.

Fixes: http://tracker.ceph.com/issues/19542
Signed-off-by: Sage Weil <sage@redhat.com>

@liewegas liewegas merged commit c8a6ed8 into ceph:master Apr 7, 2017

2 of 3 checks passed

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

@liewegas liewegas deleted the liewegas:wip-bluestore-throttle-model branch Apr 7, 2017

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