Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Move output IO throttler to IO queue level #2332

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft

Move output IO throttler to IO queue level #2332

wants to merge 12 commits into from

Commits on Jul 5, 2024

  1. fair_queue tests: Remember it is time-based 

    Current tests on fair queue try to make the queue submit requests in
extremely controllable way -- one-by-one. However, the fair queue
nowadays is driven by rated token bucket and is very sensitive to time
and durations. It's better to teach the test accept the fact that it
cannot control fair-queue requests submissions on per-request
granularity and tunes its accounting instead.

The change affects two places.

Main loop. Before the change it called fair_queue::dispatch_requests()
as many times are the number of requests test case wants to pass, then
performed the necessary checks. Now, the method is called infinitely,
and the handling only processes the requested amount of requests. The
rest is ignored.

Drain. Before the change it called dispatch_requests() in a loop until
it returned anything. Now it's called in a loop until fair queue
explicitly reports that it's empty.

Signed-off-by: Pavel Emelyanov <[email protected]>
    @xemul
    xemul committed Jul 5, 2024
    Configuration menu
    Copy the full SHA
    497f4dd View commit details
    Browse the repository at this point in the history

  2. fair_queue: Rename fair_group to throttle 

    The class in question only controls the output flow of capacities, it's
not about fair queueing at all. There is an effort to make cross-shard
fairness, that needs fair_group however, but we're not yet there.

refs: scylladb#2294

Signed-off-by: Pavel Emelyanov <[email protected]>
    @xemul
    xemul committed Jul 5, 2024
    Configuration menu
    Copy the full SHA
    33e1356 View commit details
    Browse the repository at this point in the history

  3. io_queue: Rename _fgs to _throttle 

    The io_group carries throttle instances for each of the streams, but is
still named as _f[air]g[roup]s. Effectively, this is the continuation of
the previous patch.

Signed-off-by: Pavel Emelyanov <[email protected]>
    @xemul
    xemul committed Jul 5, 2024
    Configuration menu
    Copy the full SHA
    f2229aa View commit details
    Browse the repository at this point in the history

  4. throttle: Move throttle code to its own .hh and .cc files 

    The move is very smoth, no changes in code needed but the definition of
throttle::capacity_t. It used to be the same as
fair_queue_entry::capacity_t, since we use the same notion of capacity
for both -- fair queuing and capacity throttling.

Signed-off-by: Pavel Emelyanov <[email protected]>
    @xemul
    xemul committed Jul 5, 2024
    Configuration menu
    Copy the full SHA
    c2816ec View commit details
    Browse the repository at this point in the history

  5. fair_queue: Grab request capacity by capacity value 

    The capacity grabbing method now accepts fq entry referece, but this
code is going to be moved to throttle.(cc|hh) which doesn't work with fq
entries.

Signed-off-by: Pavel Emelyanov <[email protected]>
    @xemul
    xemul committed Jul 5, 2024
    Configuration menu
    Copy the full SHA
    c618cdf View commit details
    Browse the repository at this point in the history

  6. fair_queue: Collect local throttle on a helper class 

    Shard-local fair_queue class makes some shard local bookkeeping of
throttled capacities. Shared bookkeeping was done by fair_group which
was renamed and moved away, now it's time to collect shard-local
throttle facilities.

Signed-off-by: Pavel Emelyanov <[email protected]>
    @xemul
    xemul committed Jul 5, 2024
    Configuration menu
    Copy the full SHA
    c34e6f1 View commit details
    Browse the repository at this point in the history

  7. fair_queue: Move shard-local throttle closer to shared_throttle 

    No function changes, just move the code.

Signed-off-by: Pavel Emelyanov <[email protected]>
    @xemul
    xemul committed Jul 5, 2024
    Configuration menu
    Copy the full SHA
    8f1e3f9 View commit details
    Browse the repository at this point in the history

  8. io_queue: Wrap fair_queue into stream 

    IO queue uses the concept of "stream" to handle duplex devices. The
stream is now a fair_queue, but for future patching it's needed to put
more onto stream, so this patch wraps the on-io-queue fair_queues into
helper struct stream.

Signed-off-by: Pavel Emelyanov <[email protected]>
    @xemul
    xemul committed Jul 5, 2024
    Configuration menu
    Copy the full SHA
    505cfe7 View commit details
    Browse the repository at this point in the history

  9. fair_queue: Dispatch requests via virtual method 

    The fair queue code doesn't "know" which entries it schedules. For that
there's an "abstract" fair_queue_entry that are scheduled and to
dispatch those the caller must provide std::function<> callback to the
fair_queue::dispatch_requests() method.

Next patches will need to make fair_queue call more code on those
entries. Not to introduce another std::function<> callback, change the
dispatch indirection from callback to pure virtual .dispatch() method
of the fair_queue_entry.

Signed-off-by: Pavel Emelyanov <[email protected]>
    @xemul
    xemul committed Jul 5, 2024
    Configuration menu
    Copy the full SHA
    d15fcc6 View commit details
    Browse the repository at this point in the history

  10. fair_queue/io_queue: Throttle requests on IO queue level 

    When fair_queue tries to dispatch a request it tries to grab the
capacity from throttler on its own. That's wrong place, fair group is
just about providing cross-classes fairness of requests. Throttling
output stream of requests should be done by io_queue itself.

Signed-off-by: Pavel Emelyanov <[email protected]>
    @xemul
    xemul committed Jul 5, 2024
    Configuration menu
    Copy the full SHA
    fe12391 View commit details
    Browse the repository at this point in the history

  11. fair_queue: Remove completion notifications 

    Those notifications were used to update output throttler with the result
of requests dispatching/cancellation/etc. Now when the throttler is on
the io_queue level, fair queue can be freed from this duty.

Signed-off-by: Pavel Emelyanov <[email protected]>
    @xemul
    xemul committed Jul 5, 2024
    Configuration menu
    Copy the full SHA
    679683f View commit details
    Browse the repository at this point in the history

  12. io_queue: Remove empty notifications 

    The method is empty now.

Signed-off-by: Pavel Emelyanov <[email protected]>
    @xemul
    xemul committed Jul 5, 2024
    Configuration menu
    Copy the full SHA
    f3a3afd View commit details
    Browse the repository at this point in the history