Skip to content
This repository has been archived by the owner on Aug 13, 2019. It is now read-only.

Improve Merge performance #531

Merged
merged 1 commit into from
Feb 28, 2019
Merged

Improve Merge performance #531

merged 1 commit into from
Feb 28, 2019

Conversation

brian-brazil
Copy link
Contributor

@brian-brazil brian-brazil commented Feb 21, 2019 •
edited

Use a heap for Next for merges, and
pre-compute if there's many postings on the
unset path.

Add posting lookup benchmarks

Signed-off-by: Brian Brazil brian.brazil@robustperception.io

Fixes prometheus/prometheus#5173

FYI @jacksontj @bwplotka

@brian-brazil
Copy link
Contributor Author

Benchmarks relative to before #486. The extremely slow case is still fixed, and some fairly fast cases get a little slower. I do have ideas for further optimisations.

benchmark                                                            old ns/op        new ns/op      delta
BenchmarkHeadPostingForMatchers/n="1"-4                              5420             5742           +5.94%
BenchmarkHeadPostingForMatchers/n="1",j="foo"-4                      6994             6531           -6.62%
BenchmarkHeadPostingForMatchers/j="foo",n="1"-4                      6038             5512           -8.71%
BenchmarkHeadPostingForMatchers/n="1",j!="foo"-4                     10364            10272          -0.89%
BenchmarkHeadPostingForMatchers/i=~".*"-4                            3292078491       3565717098     +8.31%
BenchmarkHeadPostingForMatchers/i=~".+"-4                            4104045278       5388378129     +31.29%
BenchmarkHeadPostingForMatchers/i=~""-4                              172650291999     1806584165     -98.95%
BenchmarkHeadPostingForMatchers/i!=""-4                              3715780926       4580742079     +23.28%
BenchmarkHeadPostingForMatchers/n="1",i=~".*",j="foo"-4              112866419        152246610      +34.89%
BenchmarkHeadPostingForMatchers/n="1",i=~".*",i!="2",j="foo"-4       196685677        214845649      +9.23%
BenchmarkHeadPostingForMatchers/n="1",i!="",j="foo"-4                264144048        268635277      +1.70%
BenchmarkHeadPostingForMatchers/n="1",i=~".+",j="foo"-4              292865856        292393747      -0.16%
BenchmarkHeadPostingForMatchers/n="1",i=~"1.+",j="foo"-4             113954164        122264453      +7.29%
BenchmarkHeadPostingForMatchers/n="1",i=~".+",i!="2",j="foo"-4       383920764        357080814      -6.99%
BenchmarkHeadPostingForMatchers/n="1",i=~".+",i!~"2.*",j="foo"-4     411214259        509801731      +23.97%

benchmark                                                            old allocs     new allocs     delta
BenchmarkHeadPostingForMatchers/n="1"-4                              8              8              +0.00%
BenchmarkHeadPostingForMatchers/n="1",j="foo"-4                      11             11             +0.00%
BenchmarkHeadPostingForMatchers/j="foo",n="1"-4                      11             11             +0.00%
BenchmarkHeadPostingForMatchers/n="1",j!="foo"-4                     19             19             +0.00%
BenchmarkHeadPostingForMatchers/i=~".*"-4                            62             68             +9.68%
BenchmarkHeadPostingForMatchers/i=~".+"-4                            200119         100123         -49.97%
BenchmarkHeadPostingForMatchers/i=~""-4                              200996         100126         -50.19%
BenchmarkHeadPostingForMatchers/i!=""-4                              200111         100114         -49.97%
BenchmarkHeadPostingForMatchers/n="1",i=~".*",j="foo"-4              20             20             +0.00%
BenchmarkHeadPostingForMatchers/n="1",i=~".*",i!="2",j="foo"-4       30             31             +3.33%
BenchmarkHeadPostingForMatchers/n="1",i!="",j="foo"-4                200076         100110         -49.96%
BenchmarkHeadPostingForMatchers/n="1",i=~".+",j="foo"-4              200076         100110         -49.96%
BenchmarkHeadPostingForMatchers/n="1",i=~"1.+",j="foo"-4             22274          11176          -49.82%
BenchmarkHeadPostingForMatchers/n="1",i=~".+",i!="2",j="foo"-4       200086         100109         -49.97%
BenchmarkHeadPostingForMatchers/n="1",i=~".+",i!~"2.*",j="foo"-4     222345         111293         -49.95%

benchmark                                                            old bytes     new bytes     delta
BenchmarkHeadPostingForMatchers/n="1"-4                              1360          1360          +0.00%
BenchmarkHeadPostingForMatchers/n="1",j="foo"-4                      1392          1392          +0.00%
BenchmarkHeadPostingForMatchers/j="foo",n="1"-4                      1392          1392          +0.00%
BenchmarkHeadPostingForMatchers/n="1",j!="foo"-4                     1616          1616          +0.00%
BenchmarkHeadPostingForMatchers/i=~".*"-4                            106202232     106284872     +0.08%
BenchmarkHeadPostingForMatchers/i=~".+"-4                            132704968     129518904     -2.40%
BenchmarkHeadPostingForMatchers/i=~""-4                              33211096      113464616     +241.65%
BenchmarkHeadPostingForMatchers/i!=""-4                              132696208     129501952     -2.41%
BenchmarkHeadPostingForMatchers/n="1",i=~".*",j="foo"-4              1608950       1609769       +0.05%
BenchmarkHeadPostingForMatchers/n="1",i=~".*",i!="2",j="foo"-4       3216492       3214998       -0.05%
BenchmarkHeadPostingForMatchers/n="1",i!="",j="foo"-4                28106771      73079923      +160.01%
BenchmarkHeadPostingForMatchers/n="1",i=~".+",j="foo"-4              28108409      73078400      +159.99%
BenchmarkHeadPostingForMatchers/n="1",i=~"1.+",j="foo"-4             4149430       5434900       +30.98%
BenchmarkHeadPostingForMatchers/n="1",i=~".+",i!="2",j="foo"-4       29713221      58628874      +97.32%
BenchmarkHeadPostingForMatchers/n="1",i=~".+",i!~"2.*",j="foo"-4     32256656      70100792      +117.32%

@brian-brazil brian-brazil mentioned this pull request Feb 21, 2019
Closed
@brian-brazil brian-brazil force-pushed the index-perf branch 3 times, most recently from 10f171a to d520618 Compare February 22, 2019 11:16
@tomwilkie
Copy link
Contributor

We should talk: https://github.com/cortexproject/cortex/blob/master/pkg/querier/batch/merge.go#L11

;-)

krasi-georgiev
krasi-georgiev reviewed Feb 25, 2019
View reviewed changes
index/postings.go Outdated Show resolved Hide resolved
index/postings.go Show resolved Hide resolved
querier.go Outdated Show resolved Hide resolved
index/postings.go Outdated Show resolved Hide resolved
krasi-georgiev
krasi-georgiev reviewed Feb 26, 2019
View reviewed changes
index/postings.go Outdated Show resolved Hide resolved
index/postings.go Show resolved Hide resolved
querier.go Outdated Show resolved Hide resolved
index/postings.go Outdated Show resolved Hide resolved
index/postings.go Outdated Show resolved Hide resolved
@brian-brazil
Copy link
Contributor Author

Updated.

@brian-brazil brian-brazil force-pushed the index-perf branch 2 times, most recently from 926e856 to 709fe89 Compare February 27, 2019 15:34
krasi-georgiev
krasi-georgiev reviewed Feb 27, 2019
View reviewed changes
head_bench_test.go Outdated Show resolved Hide resolved
head_bench_test.go Show resolved Hide resolved
@brian-brazil
Improve Merge performance
d6a81d9 
Use a heap for Next for merges, and
pre-compute if there's many postings on the
unset path.

Add posting lookup benchmarks

Signed-off-by: Brian Brazil <brian.brazil@robustperception.io>
@krasi-georgiev
Copy link
Contributor

LGTM, thanks Brian!

bwplotka
bwplotka approved these changes Feb 28, 2019
View reviewed changes
Copy link
Contributor

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

querier.go
@@ -354,11 +354,23 @@ func postingsForUnsetLabelMatcher(ix IndexReader, m labels.Matcher) (index.Posti
rit = append(rit, it)
}

merged := index.Merge(rit...)
// With many many postings, it's best to pre-calculate
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, ping for me to update Thanos code as well: https://github.com/improbable-eng/thanos/blob/master/pkg/store/bucket.go#L1249

@brian-brazil brian-brazil merged commit 62b652f into master Feb 28, 2019
@brian-brazil brian-brazil deleted the index-perf branch February 28, 2019 17:23
@hoffie hoffie mentioned this pull request Mar 1, 2019
Merged
mikkeloscar added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Apr 7, 2019
@mikkeloscar