Skip to content

Option IO throttling of the merger.#2163

Closed
fulmicoton wants to merge 1 commit intomainfrom
throttling-2
Closed

Option IO throttling of the merger.#2163
fulmicoton wants to merge 1 commit intomainfrom
throttling-2

Conversation

@fulmicoton
Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread quickwit/quickwit-indexing/src/actors/merge_executor.rs Outdated
@fulmicoton fulmicoton force-pushed the throttling-2 branch 8 times, most recently from 1cd2afb to 9254c59 Compare October 24, 2022 08:37
use std::time::{Duration, Instant};

use quickwit_common::metrics::IntCounter;
pub struct Throttle {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might not need a custom implementation: the governor rate limiter wraps an AtomicU64, so cloneable with an Arc and has sync (check*) and async until* APIs.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I am scared with governor is that
a) the docs only talks about requests / operations etc. and does not talk about using it to throttle a throughput
b) the logic that throttles is a loop that have tasks wait for the capacity to be available. I am not fond of that:
I don't know what limits the number of times a thread will have to wake up, I am not sure if there can be some wait of time here.
c) I don't understand how it works so I am not able to reason with it:
For instance, if I have two writers, sharing a rate limiter, and being throttled. One is writing 1kB at a time, when the other is doing it 10kb at a time, what throughput do they achieve?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

created an issue on github: boinkor-net/governor#154

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

created an issue on github: boinkor-net/governor#154

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/tikv/async-speed-limit seems to do exactly what we need and more (can wrap async read/write directly).

@fulmicoton
Copy link
Copy Markdown
Collaborator Author

Switching to draft, will replace with async-speed-limit

@fulmicoton fulmicoton marked this pull request as draft October 27, 2022 08:11
@fulmicoton
Copy link
Copy Markdown
Collaborator Author

Closed for #2205

@fulmicoton fulmicoton closed this Oct 31, 2022
@guilload guilload deleted the throttling-2 branch January 26, 2023 04:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants