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

osd: filestore: restructure journal and op queue throttling #7767

Merged
merged 8 commits into from Mar 1, 2016

Conversation

Projects
None yet
4 participants
@athanatos
Contributor

athanatos commented Feb 23, 2016

Tested, see below.

@athanatos athanatos added this to the jewel milestone Feb 23, 2016

@athanatos

This comment has been minimized.

Show comment
Hide comment
@athanatos

athanatos Feb 23, 2016

Contributor

@liewegas Does this make sense?

Contributor

athanatos commented Feb 23, 2016

@liewegas Does this make sense?

@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas Feb 24, 2016

Member

looks good to me, yep!

Member

liewegas commented Feb 24, 2016

looks good to me, yep!

@liewegas liewegas added the needs-qa label Feb 24, 2016

athanatos added some commits Feb 3, 2016

throttle: add a BackoffThrottle implementation
Signed-off-by: Samuel Just <sjust@redhat.com>
FileStore: use BackoffThrottle for the op queue
Signed-off-by: Samuel Just <sjust@redhat.com>
FileJournal: use queue size explicitely in aio backoff
Using the throttle here is sketchy, mainly because we won't wake
up if something new is queued.  Let's do it explicitely.

Signed-off-by: Samuel Just <sjust@redhat.com>
Journal: replace the journal throttle with fullness backoff throttle
The existing FileJournal::throttle_(ops|bytes) throttles overlap with
the FileStore op queue throttles.  It doesn't seem important whether
pending ops are waiting on the journal or the backing fs, so the
FileJournal ones are out.  Instead, there is now a throttle which
is taken in queue_transaction and released in _committed_thru
(after sync) which reflects the current fullness of the journal
and gradually delays ops as the journal fills up.  The intention is
to smooth out workloads on small journals.

Signed-off-by: Samuel Just <sjust@redhat.com>
doc/.../throttles*: update the docs to reflect the throttle changes
Signed-off-by: Samuel Just <sjust@redhat.com>
Throttle: add BackoffThrottle unit tests
Signed-off-by: Samuel Just <sjust@redhat.com>
@somnathr

This comment has been minimized.

Show comment
Hide comment
@somnathr

somnathr Feb 26, 2016

Contributor

@athanatos The commit for changing filestore_max_byte to OPT_U64 is missing in this PR

Contributor

somnathr commented Feb 26, 2016

@athanatos The commit for changing filestore_max_byte to OPT_U64 is missing in this PR

@somnathr

This comment has been minimized.

Show comment
Hide comment
@somnathr

somnathr Feb 26, 2016

@athanatos I guess we don't need aio_write_queue_ops any more..

@athanatos I guess we don't need aio_write_queue_ops any more..

athanatos added some commits Feb 22, 2016

config_opts: make filestore_queue_max_(ops|bytes) U64
Signed-off-by: Samuel Just <sjust@redhat.com>
test_filejournal: reserve throttle as needed
This is awkward, if there are later users other than this test
and JournalingObjectStore, it'll be worth making it a bit less
clunky and error prone.

Signed-off-by: Samuel Just <sjust@redhat.com>

@athanatos athanatos changed the title from DNM: Wip sam journal throttle 4 to Wip sam journal throttle 4 Mar 1, 2016

@athanatos

This comment has been minimized.

Show comment
Hide comment
@athanatos

athanatos Mar 1, 2016

Contributor

@liewegas Tested and ready for merge. Looks ok? sjust@teuthology:/a/samuelj-2016-02-29_16:00:45-rados-wip-sam-testing-distro-basic-smithi

Contributor

athanatos commented Mar 1, 2016

@liewegas Tested and ready for merge. Looks ok? sjust@teuthology:/a/samuelj-2016-02-29_16:00:45-rados-wip-sam-testing-distro-basic-smithi

@liewegas liewegas changed the title from Wip sam journal throttle 4 to osd: filestore: restructure journal and op queue throttling Mar 1, 2016

@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas Mar 1, 2016

Member

lgtm

Member

liewegas commented Mar 1, 2016

lgtm

athanatos added a commit that referenced this pull request Mar 1, 2016

Merge pull request #7767 from athanatos/wip-sam-journal-throttle-4
osd: filestore: restructure journal and op queue throttling

Reviewed-by: Samuel Just <sjust@redhat.com>

@athanatos athanatos merged commit c1e41af into ceph:master Mar 1, 2016

1 of 2 checks passed

default Build finished. No test results found.
Details
Signed-off-by all commits in this PR are signed 1 tests run, 0 skipped, 0 failed.
Details
@somnathr

This comment has been minimized.

Show comment
Hide comment
@somnathr

somnathr Mar 11, 2016

@athanatos while writing the doc I noticed this could be a problem. We can't have this 0 as well because further down the line we are doing this..
high_delay_per_count = _high_multiple / _expected_throughput;

@athanatos while writing the doc I noticed this could be a problem. We can't have this 0 as well because further down the line we are doing this..
high_delay_per_count = _high_multiple / _expected_throughput;

@somnathr

This comment has been minimized.

Show comment
Hide comment
@somnathr

somnathr Mar 12, 2016

@athanatos cosmetic , should be threshold (extra 'h')..

@athanatos cosmetic , should be threshold (extra 'h')..

@wildinto

This comment has been minimized.

Show comment
Hide comment
@wildinto

wildinto commented Mar 31, 2017

cool

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