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: ExpandedPostings shortcut: avoid LabelValues unless necessary #3872

Merged

Conversation

dimitarvdimitrov
Copy link
Contributor

This is a proposed implementation of this comment from #3848

@dimitarvdimitrov

This comment was marked as outdated.

Base automatically changed from dimitar/st-gw-shortcut-queries-with-0-series to main January 9, 2023 10:54
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
@dimitarvdimitrov dimitarvdimitrov force-pushed the dimitar/st-gw-shortcut-queries-with-0-series-labelvalues branch from cb73e8c to 90bcc56 Compare January 9, 2023 10:56
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
@dimitarvdimitrov
Copy link
Contributor Author

@pracucci @colega can please you take a look? I think I addressed everything we discussed with @colega

Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
@dimitarvdimitrov

This comment was marked as outdated.

dimitarvdimitrov and others added 2 commits January 9, 2023 16:55
Co-authored-by: Oleg Zaytsev <mail@olegzaytsev.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
@dimitarvdimitrov
Copy link
Contributor Author

some updated benchmarks based on the latest commit here

Details
name                                                                        old time/op    new time/op    delta
BucketIndexReader_ExpandedPostings/n="X",i=~"^.+$",i!~"^.*2.*$",j="foo"-10    1.13µs ± 0%    1.85µs ± 1%  +63.42%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="X",i=~"^.+$",j="foo"-10                 1.07µs ± 2%    1.67µs ± 1%  +55.62%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="X",j="foo"-10                           1.02µs ± 1%    1.48µs ± 1%  +44.98%  (p=0.016 n=5+4)
BucketIndexReader_ExpandedPostings/n="X",j!="foo"-10                          1.12µs ±18%    1.50µs ± 3%  +33.91%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i=~"[0-2]xxx"-10                           60.3µs ± 1%    75.1µs ±10%  +24.63%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i=~"X|Y|Z",j="foo"-10                2.08µs ± 1%    2.56µs ± 2%  +22.78%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i=~"(0|1|2)xxx"-10                         60.4µs ± 2%    73.4µs ± 2%  +21.57%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i=~"0xxx|1xxx|2xxx"-10                     60.4µs ± 1%    70.2µs ±11%  +16.29%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i=~"X|Y|Z"-10                              1.32µs ± 1%    1.42µs ± 0%   +7.49%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/j=~"X.+"-10                                1.31µs ± 1%    1.39µs ± 3%   +6.61%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/j=~"XXX|YYY"-10                            1.04µs ± 4%    1.09µs ± 1%   +4.91%  (p=0.016 n=5+5)
BucketIndexReader_ExpandedPostings/i!~[0-2]xxx-10                             98.4ms ± 1%   103.0ms ± 4%   +4.73%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="X"-10                                   1.06µs ±29%    1.06µs ± 5%     ~     (p=0.548 n=5+5)
BucketIndexReader_ExpandedPostings/n="1"-10                                   5.16ms ± 2%    5.15ms ± 5%     ~     (p=0.548 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",j="foo"-10                           46.9ms ± 1%    48.1ms ± 6%     ~     (p=0.151 n=5+5)
BucketIndexReader_ExpandedPostings/j="foo",n="1"-10                           46.7ms ± 0%    47.3ms ± 2%     ~     (p=0.056 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",j!="foo"-10                          37.0ms ± 2%    37.2ms ± 1%     ~     (p=0.151 n=5+5)
BucketIndexReader_ExpandedPostings/i=~".*"-10                                 88.4ms ± 1%    88.4ms ± 2%     ~     (p=0.690 n=5+5)
BucketIndexReader_ExpandedPostings/i=~".+"-10                                  276ms ± 1%     276ms ± 1%     ~     (p=0.548 n=5+5)
BucketIndexReader_ExpandedPostings/i=~""-10                                    404ms ± 1%     405ms ± 0%     ~     (p=0.222 n=5+5)
BucketIndexReader_ExpandedPostings/i!=""-10                                    275ms ± 2%     275ms ± 1%     ~     (p=1.000 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i=~".*",j="foo"-10                   49.2ms ± 1%    48.9ms ± 0%     ~     (p=0.151 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i!=""-10                              148ms ± 0%     149ms ± 1%     ~     (p=0.095 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i=~".+",j="foo"-10                    176ms ± 1%     174ms ± 1%     ~     (p=0.095 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i=~".+",i!="2",j="foo"-10             186ms ± 1%     186ms ± 4%     ~     (p=0.310 n=5+5)
BucketIndexReader_ExpandedPostings/i=~".*",_i!~[0-2]xxx-10                     103ms ± 3%     101ms ± 1%     ~     (p=0.056 n=5+5)
BucketIndexReader_ExpandedPostings/p!=""-10                                   20.9ms ± 0%    20.9ms ± 7%     ~     (p=0.690 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i=~".*",i!="2",j="foo"-10            58.4ms ± 1%    57.5ms ± 0%   -1.56%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i!="",j="foo"-10                      176ms ± 1%     173ms ± 1%   -1.70%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i=~"1.+",j="foo"-10                  28.0ms ± 1%    27.4ms ± 1%   -2.15%  (p=0.016 n=5+4)
BucketIndexReader_ExpandedPostings/n="1",i=~".+",i!~"2.*",j="foo"-10           206ms ± 1%     201ms ± 2%   -2.41%  (p=0.032 n=5+5)
BucketIndexReader_ExpandedPostings/i=~"^.+$",j=~"X.+"-10                      4.65ms ± 2%    3.11ms ± 1%  -33.09%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i!="",j=~"X.+"-10                    3.98ms ± 3%    2.45ms ± 3%  -38.47%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i!="",j=~"XXX|YYY"-10                3.69ms ± 6%    0.00ms ± 1%  -99.94%  (p=0.008 n=5+5)

name                                                                        old alloc/op   new alloc/op   delta
BucketIndexReader_ExpandedPostings/n="X",i=~"^.+$",i!~"^.*2.*$",j="foo"-10    1.06kB ± 0%    1.67kB ± 0%  +56.34%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="X",i=~"^.+$",j="foo"-10                 1.00kB ± 0%    1.49kB ± 0%  +48.75%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="X",j="foo"-10                             953B ± 0%     1313B ± 0%  +37.78%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="X",j!="foo"-10                            953B ± 0%     1313B ± 0%  +37.78%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i=~"X|Y|Z",j="foo"-10                1.30kB ± 0%    1.62kB ± 0%  +24.67%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="X"-10                                     889B ± 0%      977B ± 0%   +9.90%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/j=~"XXX|YYY"-10                              936B ± 0%      992B ± 0%   +5.98%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i=~"X|Y|Z"-10                                984B ± 0%     1040B ± 0%   +5.69%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/j=~"X.+"-10                                1.87kB ± 0%    1.93kB ± 0%   +2.99%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i=~"(0|1|2)xxx"-10                         14.0kB ± 0%    14.1kB ± 0%   +0.34%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i=~"[0-2]xxx"-10                           14.0kB ± 0%    14.1kB ± 0%   +0.34%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i=~"0xxx|1xxx|2xxx"-10                     14.1kB ± 0%    14.2kB ± 0%   +0.34%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/j="foo",n="1"-10                           19.8MB ± 0%    19.8MB ± 0%   +0.00%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i=~".*",j="foo"-10                   21.4MB ± 0%    21.4MB ± 0%   +0.00%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",j="foo"-10                           19.8MB ± 0%    19.8MB ± 0%   +0.00%  (p=0.016 n=4+5)
BucketIndexReader_ExpandedPostings/n="1",i!="",j="foo"-10                     93.5MB ± 0%    93.5MB ± 0%   +0.00%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="1"-10                                   12.7MB ± 0%    12.7MB ± 0%   +0.00%  (p=0.032 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i!=""-10                             86.4MB ± 0%    86.4MB ± 0%   +0.00%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i=~".+"-10                                  327MB ± 0%     327MB ± 0%   +0.00%  (p=0.024 n=5+5)
BucketIndexReader_ExpandedPostings/i=~""-10                                    113MB ± 0%     113MB ± 0%   +0.00%  (p=0.048 n=5+5)
BucketIndexReader_ExpandedPostings/i=~".*",_i!~[0-2]xxx-10                     287MB ± 0%     287MB ± 0%   +0.00%  (p=0.032 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",j!="foo"-10                          19.8MB ± 0%    19.8MB ± 0%     ~     (p=0.595 n=5+5)
BucketIndexReader_ExpandedPostings/i=~".*"-10                                  287MB ± 0%     287MB ± 0%     ~     (p=0.056 n=5+5)
BucketIndexReader_ExpandedPostings/i!=""-10                                    327MB ± 0%     327MB ± 0%     ~     (p=0.175 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i=~".*",i!="2",j="foo"-10            22.7MB ± 0%    22.7MB ± 0%     ~     (p=0.151 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i=~".+",j="foo"-10                   93.5MB ± 0%    93.5MB ± 0%     ~     (p=0.690 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i=~"1.+",j="foo"-10                  23.9MB ± 0%    23.9MB ± 0%     ~     (p=0.135 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i=~".+",i!="2",j="foo"-10            94.8MB ± 0%    94.8MB ± 0%     ~     (p=0.151 n=5+5)
BucketIndexReader_ExpandedPostings/p!=""-10                                   58.8MB ± 0%    58.8MB ± 0%     ~     (p=0.690 n=5+5)
BucketIndexReader_ExpandedPostings/i!~[0-2]xxx-10                              285MB ± 0%     285MB ± 0%   -0.00%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i=~".+",i!~"2.*",j="foo"-10           103MB ± 0%     100MB ± 0%   -3.54%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i!="",j=~"X.+"-10                    8.01MB ± 0%    4.81MB ± 0%  -39.97%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i=~"^.+$",j=~"X.+"-10                      8.01MB ± 0%    4.81MB ± 0%  -39.97%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i!="",j=~"XXX|YYY"-10                8.01MB ± 0%    0.00MB ± 0%  -99.98%  (p=0.008 n=5+5)

name                                                                        old allocs/op  new allocs/op  delta
BucketIndexReader_ExpandedPostings/n="X",i=~"^.+$",i!~"^.*2.*$",j="foo"-10      20.0 ± 0%      31.0 ± 0%  +55.00%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="X",i=~"^.+$",j="foo"-10                   20.0 ± 0%      30.0 ± 0%  +50.00%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="X",j="foo"-10                             20.0 ± 0%      28.0 ± 0%  +40.00%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="X",j!="foo"-10                            20.0 ± 0%      28.0 ± 0%  +40.00%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="X"-10                                     20.0 ± 0%      23.0 ± 0%  +15.00%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i=~"X|Y|Z",j="foo"-10                  29.0 ± 0%      33.0 ± 0%  +13.79%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/j=~"X.+"-10                                  21.0 ± 0%      23.0 ± 0%   +9.52%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/j=~"XXX|YYY"-10                              22.0 ± 0%      24.0 ± 0%   +9.09%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i=~"X|Y|Z"-10                                23.0 ± 0%      25.0 ± 0%   +8.70%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i=~"^.+$",j=~"X.+"-10                        27.0 ± 0%      28.0 ± 0%   +3.70%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="1"-10                                     92.0 ± 0%      93.0 ± 0%   +1.09%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i=~"0xxx|1xxx|2xxx"-10                        117 ± 0%       118 ± 0%   +0.85%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i=~"(0|1|2)xxx"-10                            117 ± 0%       118 ± 0%   +0.85%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i=~"[0-2]xxx"-10                              117 ± 0%       118 ± 0%   +0.85%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i=~".+"-10                                   600k ± 0%      600k ± 0%   +0.00%  (p=0.016 n=4+5)
BucketIndexReader_ExpandedPostings/i!=""-10                                     600k ± 0%      600k ± 0%   +0.00%  (p=0.016 n=4+5)
BucketIndexReader_ExpandedPostings/n="1",j="foo"-10                              121 ± 0%       121 ± 0%     ~     (all equal)
BucketIndexReader_ExpandedPostings/j="foo",n="1"-10                              121 ± 0%       122 ± 0%     ~     (p=0.238 n=4+5)
BucketIndexReader_ExpandedPostings/i=~".*"-10                                    108 ± 1%       108 ± 0%     ~     (p=0.556 n=5+4)
BucketIndexReader_ExpandedPostings/i=~""-10                                     600k ± 0%      600k ± 0%     ~     (all equal)
BucketIndexReader_ExpandedPostings/n="1",i!=""-10                               600k ± 0%      600k ± 0%     ~     (all equal)
BucketIndexReader_ExpandedPostings/p!=""-10                                     98.0 ± 0%      98.4 ± 1%     ~     (p=0.556 n=4+5)
BucketIndexReader_ExpandedPostings/n="1",i!="",j="foo"-10                       600k ± 0%      600k ± 0%   -0.00%  (p=0.048 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i=~".+",j="foo"-10                     600k ± 0%      600k ± 0%   -0.00%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i=~".+",i!="2",j="foo"-10              600k ± 0%      600k ± 0%   -0.00%  (p=0.000 n=4+5)
BucketIndexReader_ExpandedPostings/n="1",i=~".+",i!~"2.*",j="foo"-10            667k ± 0%      667k ± 0%   -0.00%  (p=0.029 n=4+4)
BucketIndexReader_ExpandedPostings/n="1",i=~"1.+",j="foo"-10                   66.8k ± 0%     66.8k ± 0%   -0.00%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i=~".*",_i!~[0-2]xxx-10                       172 ± 0%       171 ± 0%   -0.58%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/i!~[0-2]xxx-10                                167 ± 0%       166 ± 0%   -0.60%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",j!="foo"-10                             120 ± 0%       119 ± 0%   -0.83%  (p=0.029 n=4+4)
BucketIndexReader_ExpandedPostings/n="1",i=~".*",j="foo"-10                      126 ± 0%       124 ± 0%   -1.59%  (p=0.029 n=4+4)
BucketIndexReader_ExpandedPostings/n="1",i=~".*",i!="2",j="foo"-10               153 ± 0%       150 ± 0%   -2.22%  (p=0.000 n=5+4)
BucketIndexReader_ExpandedPostings/n="1",i!="",j=~"X.+"-10                      33.0 ± 0%      31.0 ± 0%   -6.06%  (p=0.008 n=5+5)
BucketIndexReader_ExpandedPostings/n="1",i!="",j=~"XXX|YYY"-10                  34.0 ± 0%      30.0 ± 0%  -11.76%  (p=0.008 n=5+5)

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.

Nice job! I took an initial pass, without reviewing the logic very accurately (yet).

Why some benchmarks allocate more? Have you looked it more in details? In theory we're just delaying some postings "resolution" (lazy postings) but the total allocations shouldn't change.

pkg/storegateway/bucket_index_postings.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_postings.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_postings.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_postings.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_postings.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_reader.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_postings.go Outdated Show resolved Hide resolved
@dimitarvdimitrov
Copy link
Contributor Author

Why some benchmarks allocate more? Have you looked it more in details? In theory we're just delaying some postings "resolution" (lazy postings) but the total allocations shouldn't change.

They increase because now we have an intermediate slice of label values that we create before checking the index header: here instead of checking the header first, we immediately allocate a postingGroup and here a slice for the labels.

if m.Value != "" {
// Fast-path for equal matching.
// Works for every case except for `foo=""`, which is a special case, see below.
if m.Type == labels.MatchEqual {
return newRawPostingGroup(false, m.Name, []labels.Label{{Name: m.Name, Value: m.Value}})
}

Those are later filtered here

if _, err := r.PostingsOffset(l.Name, l.Value); errors.Is(err, indexheader.NotFoundRangeErr) {
// This label name and value doesn't exist in this block, so there are 0 postings we can match.
// Set it to an empty value, so we can filter it out later.
keys[i] = labels.Label{}
// Try with the rest of the set matchers, maybe they can match some series.
continue
} else if err != nil {

But in the previous implementation the filtering was happening before the posting group and the slice of keys was even created:

if m.Value != "" {
// Fast-path for equal matching.
// Works for every case except for `foo=""`, which is a special case, see below.
if m.Type == labels.MatchEqual {
if _, err := lvr.PostingsOffset(m.Name, m.Value); errors.Is(err, indexheader.NotFoundRangeErr) {
// This label name or value doesn't exist in this block, so there are 0 postings we can match.
return newPostingGroup(false, nil, nil), nil
}
return newPostingGroup(false, []labels.Label{{Name: m.Name, Value: m.Value}}, nil), nil
}

This is more evident in benchmark cases where we shortcut and don't fetch posting lists. In cases where there are posting lists, the memory is dominated by them.

The aim of this PR is to reduce the calls to LabelValues and avoid reading all label values from the index header. So cases like n="1",i!="",j=~"XXX|YYY" have a 99% reduction in memory because we don't need to hold the label values for i!="" in memory before realizing the matchers select no series.

dimitarvdimitrov and others added 5 commits January 12, 2023 19:11
Co-authored-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
@dimitarvdimitrov
Copy link
Contributor Author

there is a race condition although this code should only be running in one goroutine. I will investigate

This reverts commit 02a6dc2

Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
@dimitarvdimitrov
Copy link
Contributor Author

data race was because the SetMatchers of a matcher were reused between goroutines of different blocks

if setMatches := m.SetMatches(); len(setMatches) > 0 && (m.Type == labels.MatchRegexp || m.Type == labels.MatchNotRegexp) {
if m.Type == labels.MatchNotRegexp {
return newRawSubtractingPostingGroup(m.Name, setMatches)
}
return newRawIntersectingPostingGroup(m.Name, setMatches)
}

Since we use the raw slices of strings instead of copying them, we effectively modify the set matchers here (done for each block)

To make the CI passing I reverted the commit that uses the raw string slices from SetMatchers

Copy link
Contributor

@colega colega left a comment

Choose a reason for hiding this comment

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

LGTM, all my comments are nitpicks.

pkg/storegateway/bucket_index_postings.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_postings.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_postings.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_postings.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_postings.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_reader.go Show resolved Hide resolved
pkg/storegateway/bucket_index_reader.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_reader.go Outdated Show resolved Hide resolved
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.

Great job, LGTM! 👍 on Oleg comments. I also left few nits, really minor things.

pkg/storegateway/bucket_index_postings.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_postings.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_reader.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_reader.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_reader.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_reader.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_reader.go Outdated Show resolved Hide resolved
pkg/storegateway/bucket_index_reader.go Outdated Show resolved Hide resolved
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
dimitarvdimitrov and others added 6 commits January 19, 2023 11:30
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Co-authored-by: Marco Pracucci <marco@pracucci.com>
Co-authored-by: Oleg Zaytsev <mail@olegzaytsev.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
…ngGroup

Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
@dimitarvdimitrov
Copy link
Contributor Author

Thanks for the reviews @colega, @pracucci. I think I addressed all your comments, PTAL :)

Copy link
Contributor

@colega colega 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!

@pracucci
Copy link
Collaborator

Ship it! 🚀

Thanks for addressing my comments!

@dimitarvdimitrov dimitarvdimitrov merged commit 5fe3b80 into main Jan 19, 2023
@dimitarvdimitrov dimitarvdimitrov deleted the dimitar/st-gw-shortcut-queries-with-0-series-labelvalues branch January 19, 2023 13:44
fayzal-g added a commit that referenced this pull request Jan 20, 2023
* Update test

* Add missing changelog entries for commits since Mimir 2.5 (#4006)

All other commits weren't user-facing or were helm-chart specific.

See #3979

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>

* Add --concurrency support to 'mimirtool rules sync' command (#3996)

* Add --concurrency support to 'mimirtool rules sync' command

Signed-off-by: Marco Pracucci <marco@pracucci.com>

* Update pkg/mimirtool/commands/rules.go

Co-authored-by: Patrick Oyarzun <patrick.oyarzun@grafana.com>

Signed-off-by: Marco Pracucci <marco@pracucci.com>
Co-authored-by: Patrick Oyarzun <patrick.oyarzun@grafana.com>

* store-gateway: ExpandedPostings shortcut: avoid LabelValues unless necessary (#3872)

* Return `Canceled` rather than `Aborted` when a `Series` request to a store-gateway is cancelled by the calling querier. (#4007)

* Return Canceled rather than Aborted when a Series request to a store-gateway is cancelled.

* Add changelog entry.

* Update mimir-prometheus, add support for align_evaluation_time_on_interval. (#4013)

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

* Fix title of guide in link text; reword phrase. (#4008)

* Fix ExampleInitLogger to work in UTC (#4016)

The test didn't pass in my time zone (tm).

--- FAIL: ExampleInitLogger (0.00s)
got:
ts=1970-01-01T01:00:00+01:00 caller=log_test.go:31 level=info test=1
ts=1970-01-01T01:00:00+01:00 caller=log_test.go:33 level=info msg="test 3"
want:
ts=1970-01-01T00:00:00Z caller=log_test.go:31 level=info test=1
ts=1970-01-01T00:00:00Z caller=log_test.go:33 level=info msg="test 3"

Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>

Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>

* Create outline of Mimir 2.6 release notes (#4002)

Includes notable features and bugfixes based on the CHANGELOG. Helm changes
to be filled out later by product and engineering.

See #3979

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>

* Fix post-merge comments from PR #4013. (#4014)

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

* Update CODEOWNERS to include mimir-ruler-and-alertmanager-maintainers (#4019)

For those who only want notifications re the ruler or Alertmanager.

* Remove internal use of store.max-query-length (#4017)

Make deprecation of the option more obvious and attempt to remove
any use of store.max-query-length in our documentation, jsonnet, helm,
and integration tests.

See #2793
See #3825

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>

* [otlp] Update OTel Collector to latest release (#3852)

* [otlp] Update otel collector dependecy to latest
* Update code to deal with deprecated functions

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>

* [otlp] Docs: Highlight common issues with OTLP --> Prometheus (#3629)

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
Co-authored-by: Ursula Kallio <ursula.kallio@grafana.com>

* make it possible to inject memberlist kv codecs (#4018)

* make it possible to inject memberlist kv codecs

Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>

* add comment

Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>

* improve comment wording

Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>

Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>

* Limits and errors for ephemeral storage (#4004)

* Add limits for ephemeral storage.
* Add new reason when ingestion of ephemeral metrics fails.
* Add tests for max ephemeral series limit.
* Introduce new discard reasons when ingesting ephemeral series.

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

* Reduce maintainership and step down as team member. (#4023)

* Reduce maintainership and step down as team member.

My future priorities will be on the alerting aspects of Mimir, so I think it is
right to reduce my maintainership accordingly and allow others to take my place.
Similarly, remove myself as a team member.

* Sort previous team members.

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com>
Co-authored-by: Marco Pracucci <marco@pracucci.com>
Co-authored-by: Patrick Oyarzun <patrick.oyarzun@grafana.com>
Co-authored-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Co-authored-by: Charles Korn <charleskorn@users.noreply.github.com>
Co-authored-by: Peter Štibraný <pstibrany@gmail.com>
Co-authored-by: Ursula Kallio <ursula.kallio@grafana.com>
Co-authored-by: Oleg Zaytsev <mail@olegzaytsev.com>
Co-authored-by: gotjosh <josue.abreu@gmail.com>
Co-authored-by: Goutham Veeramachaneni <gouthamve@gmail.com>
Co-authored-by: Mauro Stettler <mauro.stettler@gmail.com>
Co-authored-by: Steve Simpson <steve.simpson@grafana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants