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

Add query sharding documentation #960

Merged
merged 14 commits into from
Feb 24, 2022
Merged

Add query sharding documentation #960

merged 14 commits into from
Feb 24, 2022

Conversation

simonswine
Copy link
Contributor

@simonswine simonswine commented Jan 28, 2022 •
edited
Loading

What this PR does:

This adds some documentation for the query_range sharding.

The details how the query-sharding works involves quite a lot of understanding about how the query-frontend works in general. I hope I haven't made that too complicate.

Next steps:

  • Explain the flow of a query through the query-frontend and how that influences query shard count

I would really appreciate some feedback on the content the level of depth from @pracucci

@simonswine simonswine force-pushed the 20220128_add-query-sharding-docs branch from cd5963e to e5f483a Compare January 28, 2022 17:10
@simonswine
Copy link
Contributor Author

@KMiller-Grafana is preparing a rewrite of this, so a review of grammar/spelling is most likely wasting time.

It would be good though to get some feedback what content is missing

@pracucci
Copy link
Collaborator

Fixes #934

@pracucci pracucci added the type/docs Improvements or additions to documentation label Jan 28, 2022
@jdbaldry jdbaldry mentioned this pull request Jan 31, 2022
Closed
@simonswine @KMiller-Grafana
Address review comments
d85fca8 
Co-authored-by: Karen Miller <karen.miller@grafana.com>
Signed-off-by: Christian Simon <simon@swine.de>
@simonswine simonswine force-pushed the 20220128_add-query-sharding-docs branch from f6a79fc to d85fca8 Compare January 31, 2022 15:03
@jdbaldry jdbaldry mentioned this pull request Jan 31, 2022
Closed
53 tasks
osg-grafana
osg-grafana approved these changes Feb 1, 2022
View reviewed changes
Copy link
Contributor

@osg-grafana osg-grafana left a comment

Choose a reason for hiding this comment

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

Unblocking, and did not review.

pracucci
pracucci reviewed Feb 2, 2022
View reviewed changes
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.

Good job! I think it's a good starting point. I've left few comments I would be glad if you could take a look at.

docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
docs/sources/guides/query-sharding.md Show resolved Hide resolved
@osg-grafana
Copy link
Contributor

@jdbaldry, @pracucci, and @KMiller-Grafana, let's ship this first version today and iterate in a subsequent technical review.

@osg-grafana
Copy link
Contributor

@jdbaldry, let's re-order this so that it's in place for the next steps. @simonswine, I understand that doing so is exceptional and not a best practice, and we are doing this re-ordering as a one-off.

@osg-grafana
Copy link
Contributor

@jdbaldry, let's re-order this so that it's in place for the next steps. @simonswine, I understand that doing so is exceptional and not a best practice, and we are doing this re-ordering as a one-off.

@simonswine, we'll wait on a confirmation from you to move forward.

@simonswine simonswine marked this pull request as ready for review February 9, 2022 09:40
@simonswine
Copy link
Contributor Author

@simonswine, we'll wait on a confirmation from you to move forward.

Yes I have incorporated @pracucci's feedback, please go ahead

@osg-grafana
Copy link
Contributor

@simonswine, feel free to commit @replay's suggestions since they are already there at our fingertips.

replay
replay approved these changes Feb 15, 2022
View reviewed changes
Copy link
Contributor

@replay replay left a comment

Choose a reason for hiding this comment

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

👍 thank you

simonswine and others added 2 commits February 15, 2022 12:53
@simonswine @replay
Apply suggestions from code review
d6410ea 
Co-authored-by: Mauro Stettler <mauro.stettler@gmail.com>
@simonswine
Remove comments
ae62270
@simonswine simonswine force-pushed the 20220128_add-query-sharding-docs branch from 5aa9571 to ae62270 Compare February 15, 2022 12:53
pracucci
pracucci reviewed Feb 17, 2022
View reviewed changes
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.

You did a magnificent job! This doc is lovely and very clear (considered the topic complexity). I've left few comments I would be glad to see addressed before merging. Thanks!

docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
@pracucci pracucci mentioned this pull request Feb 18, 2022
Merged
3 tasks
@simonswine
Copy link
Contributor Author

@osg-grafana I will take another at this soon to incorporate Marcos feedback. Let me know if I shouldn't.

@simonswine
Copy link
Contributor Author

@pracucci thank you for the suggestions. I have incorporated them and would like you to take another (final?) look.

pracucci
pracucci approved these changes Feb 24, 2022
View reviewed changes
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.

Thanks for patiently address my feedback. LGTM! (modulo few last nits)

docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
docs/sources/guides/query-sharding.md Outdated Show resolved Hide resolved
@simonswine @pracucci
Apply suggestions from code review
11bc934 
Co-authored-by: Marco Pracucci <marco@pracucci.com>
@simonswine
Copy link
Contributor Author

@osg-grafana I think this PR would be ready to be merged from my point of view

@pracucci
Copy link
Collaborator

@simonswine could you fix the broken links, please?

ERROR 2022/02/24 13:41:37 [en] REF_NOT_FOUND: Ref "../blocks-storage/bucket-index.md": "/hugo/content/docs/mimir/latest/architecture/querier.md:19:48": page not found
ERROR 2022/02/24 13:41:37 [en] REF_NOT_FOUND: Ref "../blocks-storage/bucket-index.md": "/hugo/content/docs/mimir/latest/architecture/querier.md:32:21": page not found

@simonswine
Copy link
Contributor Author

@simonswine could you fix the broken links, please?

@pracucci: Done in #1294

@osg-grafana
Copy link
Contributor

@osg-grafana I think this PR would be ready to be merged from my point of view

Ship it!

@pracucci pracucci merged commit e56c145 into main Feb 24, 2022
@pracucci pracucci deleted the 20220128_add-query-sharding-docs branch February 24, 2022 16:24
pracucci added a commit that referenced this pull request Feb 28, 2022
@simonswine @KMiller-Grafana
@replay @pracucci
Add query sharding documentation (#960)
c6a11d3 
* Add query sharding documentation

* Address review comments

Co-authored-by: Karen Miller <karen.miller@grafana.com>
Signed-off-by: Christian Simon <simon@swine.de>

* Provide concrete example for shardable queries

* Remove debugging header `Sharding-Control`

* Remove outdated feature flag

* Rewrite the steps necessary to configure query-sharding

* Mention the need to raise -querier.max-query-paralleism

* Prettify markdown

* Apply suggestions from code review

Co-authored-by: Mauro Stettler <mauro.stettler@gmail.com>

* Remove comments

* Apply suggestions from code review

Co-authored-by: Marco Pracucci <marco@pracucci.com>

* Add example 3 to show flow of a query with 2 shardable portions.

* Prettifier

* Apply suggestions from code review

Co-authored-by: Marco Pracucci <marco@pracucci.com>

Co-authored-by: Karen Miller <karen.miller@grafana.com>
Co-authored-by: Mauro Stettler <mauro.stettler@gmail.com>
Co-authored-by: Marco Pracucci <marco@pracucci.com>
This was referenced Feb 28, 2022
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@replay replay replay approved these changes

@pracucci pracucci pracucci approved these changes

@osg-grafana osg-grafana osg-grafana approved these changes

Assignees
No one assigned
Labels
type/docs Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@simonswine @pracucci @osg-grafana @replay