-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
scanner: Allow full throttle if there is no parallel disk ops #18109
Conversation
The QUESTION is if we really want to do that by default? |
The main goal is to make scanning goes in full throttle when we know that there is no ongoing S3 operations; Since one S3 operation involves multiple disks in different nodes, I thought the easiest way is to keep track of the number of concurrent S3 operations per disk using context.Context concept; (this seems the cheapest option in my opinion) So, if an S3 request lands one node; we add an attribute to context.Context which will be passed down to xl-storage, that way we can count the number of concurrent s3 ops per disk. To make sure that remote disks get the same attribute, we can pass that in internode calls headers; Now each disk that is currently scanning knows how many concurrent requests that is hitting and dynamically decides if it should go in full throtlle or uses the usual sleeping mechanism |
Yes. But I am not too sure everybody will appreciate the extra wear on the hardware.
It is a hacky, fragile solution. When we already monitor running disk operations we should hook into that instead. You can add a parameter in |
baa248e
to
14d5b47
Compare
Yes, maybe let's discuss this first; what we can possibly do is to avoid running the scanner when we know there is no write s3 operations since the last scanner run; I actually started to believe we need to merge the other scanner PR first, #18107 ; I have few left things to check before making it ready for review |
918ed82
to
8fc6660
Compare
I have rebased and pushed the changes @vadmeste |
8fc6660
to
7d986ea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
This PR is merged, @vadmeste will send another PR to allow this to be configurable behavior via |
Community Contribution License
All community contributions in this pull request are licensed to the project maintainers
under the terms of the [Apache 2 license] (https://www.apache.org/licenses/LICENSE-2.0).
By creating this pull request I represent that I have the right to license the
contributions to the project maintainers under the Apache 2 license.
Description
When there is no ongoing xl storage operations, it is okay to allow the scanner to go
on full speed scan.
Motivation and Context
How to test this PR?
Types of changes
Checklist:
commit-id
orPR #
here)