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

run multiple queries per table at once with boltdb-shipper #2656

Merged

Conversation

sandeepsukhani
Copy link
Contributor

@sandeepsukhani sandeepsukhani commented Sep 22, 2020

What this PR does / why we need it:
While investigating query performance I found labels query to be slower because it does a lot of index queries, with our internal cluster it was close to 33k queries. After digging deeper I found we were doing mutex and channel operation in downloads path for each query here which is inefficient.

This PR changes the code to do mutex and channel operation and run all queries per table at once and run. With my local testing I found 7x improvement in query performance with this change.

Checklist

  • Tests updated

@codecov-commenter
Copy link

codecov-commenter commented Sep 22, 2020

Codecov Report

Merging #2656 into master will increase coverage by 0.05%.
The diff coverage is 58.53%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2656      +/-   ##
==========================================
+ Coverage   61.19%   61.25%   +0.05%     
==========================================
  Files         172      173       +1     
  Lines       13363    13390      +27     
==========================================
+ Hits         8178     8202      +24     
  Misses       4438     4438              
- Partials      747      750       +3     
Impacted Files Coverage Δ
pkg/storage/stores/shipper/uploads/table.go 67.10% <0.00%> (ø)
pkg/storage/stores/shipper/util/queries.go 53.33% <53.33%> (ø)
.../storage/stores/shipper/downloads/table_manager.go 64.70% <60.00%> (-0.73%) ⬇️
pkg/storage/stores/shipper/downloads/table.go 65.53% <66.66%> (+0.68%) ⬆️
...kg/storage/stores/shipper/uploads/table_manager.go 63.55% <71.42%> (-0.53%) ⬇️
pkg/logql/ast.go 89.41% <100.00%> (ø)
pkg/logql/evaluator.go 92.81% <0.00%> (+0.42%) ⬆️
pkg/querier/queryrange/downstreamer.go 97.64% <0.00%> (+2.35%) ⬆️
pkg/promtail/targets/file/filetarget.go 66.19% <0.00%> (+2.81%) ⬆️

@sandeepsukhani sandeepsukhani force-pushed the boltdb-shipper-multi-query branch 2 times, most recently from d6081db to 2a17fea Compare September 23, 2020 09:22
Copy link
Collaborator

@slim-bean slim-bean left a comment

Choose a reason for hiding this comment

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

LGTM!

@sandeepsukhani sandeepsukhani merged commit 79c0275 into grafana:master Sep 25, 2020
cyriltovena pushed a commit to cyriltovena/loki that referenced this pull request Jun 11, 2021
It broke ingester hand-overs, and is not needed now we use BigChunk.

I left in the code which allows non-full-size chunks to be read in:
maybe someone has some in a database.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants