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.

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

store-gateway: wait for query gate after loading blocks #5507

Merged
merged 3 commits into from
Jul 18, 2023

Conversation

dimitarvdimitrov
Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov commented Jul 14, 2023

A surge in lazy loading index headers can saturate the query concurrency limit and effectively block any other queries from even starting. This was evident in a recent similar incident where the request rate on the full query path was impacted by store-gateways lazy-loading blocks. During the time between 00:00 and 00:30 99.2% of queries were issues for the last 12 hours only.

Below are screenshots of request rate, latency, lazy loading latency and query gate latency reported by store-gateways.

Screenshot 2023-07-14 at 06 39 44
Screenshot 2023-07-14 at 06 40 18
Screenshot 2023-07-14 at 18 25 21

We already have a concurrency gate for lazy loading index headers added by @heatheryuan , which prevents from overloading the disk. If we decouple the query gate and the header loading gate, queries that require loading many index headers will be waiting on a separate gate and not impact queries which are querying already loaded blocks.

Notes to reviewers

  • This is a draft PR because I suspect it will conflict with the changes in Stream chunks from storegateway to queriers #5182 so I will rebase it after merging that PR first.
  • I removed the if because the query gate is either a noop gate or a real gate, but never nil.

A surge in lazy loading index headers can saturate the query concurrency limit and effectively block any other queries from even starting.
This was evident in a recent similar incident where the request rate on the full query path was impacted by store-gateways lazy-loading blocks.

Below are screenshots of request rate, latency, lazy loading latency and query gate latency reported by store-gateways.

We already have a concurrency gate for lazy loading index headers added by Heather, which prevents from overloading the disk.
If we decouple the query gate and the header loading gate, queries that require loading many index headers will be waiting
on a separate gate and not impact queries which are querying already loaded blocks.

#### Note to reviewers

This is a draft PR because I suspect it will conflict with the changes in stream-store, so I will rebase it after merging that PR first.

Signed-off-by: Dimitar Dimitrov <[email protected]>
Copy link
Contributor

@56quarters 56quarters left a comment

Choose a reason for hiding this comment

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

Makes sense to me 👍

Copy link
Contributor

@heatheryuan heatheryuan left a comment

Choose a reason for hiding this comment

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

looks good to me!

@dimitarvdimitrov dimitarvdimitrov marked this pull request as ready for review July 18, 2023 07:55
@dimitarvdimitrov dimitarvdimitrov requested a review from a team as a code owner July 18, 2023 07:55
Signed-off-by: Dimitar Dimitrov <[email protected]>
Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Makes a lot of sense

@dimitarvdimitrov dimitarvdimitrov merged commit d38715b into main Jul 18, 2023
28 checks passed
@dimitarvdimitrov dimitarvdimitrov deleted the dimitar/st-gw/separeate-concurrency-gates branch July 18, 2023 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants