From 594411de8ea870117235f2c9a6237868ad0d7f45 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Thu, 26 Oct 2023 09:59:39 +0200 Subject: [PATCH] Upgrade mimir-prometheus to 478c4325 Signed-off-by: Arve Knudsen --- CHANGELOG.md | 1 + go.mod | 2 +- go.sum | 4 +- .../tsdb/postings_for_matchers_cache.go | 42 +++++++++++++------ vendor/modules.txt | 4 +- 5 files changed, 35 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bf04aa5ab0..fdef5620f78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ * [BUGFIX] Query-scheduler: don't retain connections from queriers that have shut down, leading to gradually increasing enqueue latency over time. #6100 #6145 * [BUGFIX] Ingester: prevent query logic from continuing to execute after queries are canceled. #6085 * [BUGFIX] Ensure correct nesting of children of the `querier.Select` tracing span. #6085 +* [BUGFIX] Ingester: Don't cache context cancellation error when querying. #6486 ### Mixin diff --git a/go.mod b/go.mod index 86e88e50910..e90082e1711 100644 --- a/go.mod +++ b/go.mod @@ -251,7 +251,7 @@ require ( ) // Using a fork of Prometheus with Mimir-specific changes. -replace github.com/prometheus/prometheus => github.com/grafana/mimir-prometheus v0.0.0-20230929133245-5e1fe508c771 +replace github.com/prometheus/prometheus => github.com/grafana/mimir-prometheus v0.0.0-20231026075524-478c4325d169 // Replace memberlist with our fork which includes some fixes that haven't been // merged upstream yet: diff --git a/go.sum b/go.sum index bd97978f194..6e4423e3893 100644 --- a/go.sum +++ b/go.sum @@ -555,8 +555,8 @@ github.com/grafana/gomemcache v0.0.0-20230914135007-70d78eaabfe1 h1:MLYY2R60/74h github.com/grafana/gomemcache v0.0.0-20230914135007-70d78eaabfe1/go.mod h1:PGk3RjYHpxMM8HFPhKKo+vve3DdlPUELZLSDEFehPuU= github.com/grafana/memberlist v0.3.1-0.20220714140823-09ffed8adbbe h1:yIXAAbLswn7VNWBIvM71O2QsgfgW9fRXZNR0DXe6pDU= github.com/grafana/memberlist v0.3.1-0.20220714140823-09ffed8adbbe/go.mod h1:MS2lj3INKhZjWNqd3N0m3J+Jxf3DAOnAH9VT3Sh9MUE= -github.com/grafana/mimir-prometheus v0.0.0-20230929133245-5e1fe508c771 h1:YVKeiFLJUGG/kdx3sdkmz4kS6gw+5OVAIOJBHwKuaIk= -github.com/grafana/mimir-prometheus v0.0.0-20230929133245-5e1fe508c771/go.mod h1:FS+VpDcgSX2unPDcuzLAH4+qdraB8f/Kwy73bYwxFJo= +github.com/grafana/mimir-prometheus v0.0.0-20231026075524-478c4325d169 h1:WcnFfpmUVS0aQ98Dc6f2z+w1R6FyzuVDkowfeKDZp0k= +github.com/grafana/mimir-prometheus v0.0.0-20231026075524-478c4325d169/go.mod h1:FS+VpDcgSX2unPDcuzLAH4+qdraB8f/Kwy73bYwxFJo= github.com/grafana/opentracing-contrib-go-stdlib v0.0.0-20230509071955-f410e79da956 h1:em1oddjXL8c1tL0iFdtVtPloq2hRPen2MJQKoAWpxu0= github.com/grafana/opentracing-contrib-go-stdlib v0.0.0-20230509071955-f410e79da956/go.mod h1:qtI1ogk+2JhVPIXVc6q+NHziSmy2W5GbdQZFUHADCBU= github.com/grafana/regexp v0.0.0-20221005093135-b4c2bcb0a4b6 h1:A3dhViTeFDSQcGOXuUi6ukCQSMyDtDISBp2z6OOo2YM= diff --git a/vendor/github.com/prometheus/prometheus/tsdb/postings_for_matchers_cache.go b/vendor/github.com/prometheus/prometheus/tsdb/postings_for_matchers_cache.go index aea96ca3dcf..865224f7f31 100644 --- a/vendor/github.com/prometheus/prometheus/tsdb/postings_for_matchers_cache.go +++ b/vendor/github.com/prometheus/prometheus/tsdb/postings_for_matchers_cache.go @@ -78,37 +78,53 @@ func (c *PostingsForMatchersCache) PostingsForMatchers(ctx context.Context, ix I return c.postingsForMatchers(ctx, ix, ms...) } c.expire() - return c.postingsForMatchersPromise(ctx, ix, ms)() + return c.postingsForMatchersPromise(ix, ms)(ctx) } type postingsForMatcherPromise struct { - sync.WaitGroup + done chan struct{} cloner *index.PostingsCloner err error } -func (p *postingsForMatcherPromise) result() (index.Postings, error) { - p.Wait() - if p.err != nil { - return nil, p.err +func (p *postingsForMatcherPromise) result(ctx context.Context) (index.Postings, error) { + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-p.done: + // Checking context error is necessary for deterministic tests, + // as channel selection order is random + if ctx.Err() != nil { + return nil, ctx.Err() + } + if p.err != nil { + return nil, p.err + } + return p.cloner.Clone(), nil } - return p.cloner.Clone(), nil } -func (c *PostingsForMatchersCache) postingsForMatchersPromise(ctx context.Context, ix IndexPostingsReader, ms []*labels.Matcher) func() (index.Postings, error) { +func (c *PostingsForMatchersCache) postingsForMatchersPromise(ix IndexPostingsReader, ms []*labels.Matcher) func(context.Context) (index.Postings, error) { promise := new(postingsForMatcherPromise) - promise.Add(1) + promise.done = make(chan struct{}) key := matchersKey(ms) oldPromise, loaded := c.calls.LoadOrStore(key, promise) if loaded { - promise = oldPromise.(*postingsForMatcherPromise) - return promise.result + // promise was not stored, we return a previously stored promise, that's possibly being fulfilled in another goroutine + close(promise.done) + return oldPromise.(*postingsForMatcherPromise).result } - defer promise.Done() - if postings, err := c.postingsForMatchers(ctx, ix, ms...); err != nil { + // promise was stored, close its channel after fulfilment + defer close(promise.done) + + // Don't let context cancellation fail the promise, since it may be used by multiple goroutines, each with + // its own context. Also, keep the call independent of this particular context, since the promise will be reused. + // FIXME: do we need to cancel the call to postingsForMatchers if all the callers waiting for the result have + // cancelled their context? + if postings, err := c.postingsForMatchers(context.Background(), ix, ms...); err != nil { promise.err = err } else { promise.cloner = index.NewPostingsCloner(postings) diff --git a/vendor/modules.txt b/vendor/modules.txt index e26472f8633..9bd9199ca3b 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -917,7 +917,7 @@ github.com/prometheus/exporter-toolkit/web github.com/prometheus/procfs github.com/prometheus/procfs/internal/fs github.com/prometheus/procfs/internal/util -# github.com/prometheus/prometheus v1.8.2-0.20220620125440-d7e7b8e04b5e => github.com/grafana/mimir-prometheus v0.0.0-20230929133245-5e1fe508c771 +# github.com/prometheus/prometheus v1.8.2-0.20220620125440-d7e7b8e04b5e => github.com/grafana/mimir-prometheus v0.0.0-20231026075524-478c4325d169 ## explicit; go 1.20 github.com/prometheus/prometheus/config github.com/prometheus/prometheus/discovery @@ -1497,7 +1497,7 @@ sigs.k8s.io/kustomize/kyaml/yaml/walk # sigs.k8s.io/yaml v1.3.0 ## explicit; go 1.12 sigs.k8s.io/yaml -# github.com/prometheus/prometheus => github.com/grafana/mimir-prometheus v0.0.0-20230929133245-5e1fe508c771 +# github.com/prometheus/prometheus => github.com/grafana/mimir-prometheus v0.0.0-20231026075524-478c4325d169 # github.com/hashicorp/memberlist => github.com/grafana/memberlist v0.3.1-0.20220714140823-09ffed8adbbe # gopkg.in/yaml.v3 => github.com/colega/go-yaml-yaml v0.0.0-20220720105220-255a8d16d094 # github.com/grafana/regexp => github.com/grafana/regexp v0.0.0-20221005093135-b4c2bcb0a4b6