From e4eaa047d8a40e63e3a279677dc40552c177cff8 Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Wed, 30 Oct 2024 09:58:07 +0100 Subject: [PATCH 1/2] Update mimir-prometheus Signed-off-by: Marco Pracucci --- go.mod | 2 +- go.sum | 4 +- .../tsdb/postings_for_matchers_cache.go | 43 ++++++++++++++----- vendor/modules.txt | 4 +- 4 files changed, 37 insertions(+), 16 deletions(-) diff --git a/go.mod b/go.mod index 282f6d8af6f..0df5b949837 100644 --- a/go.mod +++ b/go.mod @@ -282,7 +282,7 @@ require ( ) // Using a fork of Prometheus with Mimir-specific changes. -replace github.com/prometheus/prometheus => github.com/grafana/mimir-prometheus v0.0.0-20241029123536-5710f65f1444 +replace github.com/prometheus/prometheus => github.com/grafana/mimir-prometheus v0.0.0-20241030085501-6c2603082009 // 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 f11d67a9e41..f84eab43443 100644 --- a/go.sum +++ b/go.sum @@ -527,8 +527,8 @@ github.com/grafana/gomemcache v0.0.0-20241016125027-0a5bcc5aef40 h1:1TeKhyS+pvzO github.com/grafana/gomemcache v0.0.0-20241016125027-0a5bcc5aef40/go.mod h1:IGRj8oOoxwJbHBYl1+OhS9UjQR0dv6SQOep7HqmtyFU= 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-20241029123536-5710f65f1444 h1:o3Plh90SIbYgW6penf8BV3QUH0gwBJou+Cd2uCfamRk= -github.com/grafana/mimir-prometheus v0.0.0-20241029123536-5710f65f1444/go.mod h1:7SuFBLahBoRY7KcgzWzK0p1n5QL+5dyr/Ysat6v1378= +github.com/grafana/mimir-prometheus v0.0.0-20241030085501-6c2603082009 h1:ZIlymN/o3WNl8jzBRkQXLvtW5/5wF9bZ5Cy9dxHwku0= +github.com/grafana/mimir-prometheus v0.0.0-20241030085501-6c2603082009/go.mod h1:7SuFBLahBoRY7KcgzWzK0p1n5QL+5dyr/Ysat6v1378= 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/prometheus-alertmanager v0.25.1-0.20240930132144-b5e64e81e8d3 h1:6D2gGAwyQBElSrp3E+9lSr7k8gLuP3Aiy20rweLWeBw= 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 49d4d75ea02..ca2e5295e32 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 @@ -14,6 +14,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/trace" + "go.uber.org/atomic" "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/tsdb/index" @@ -49,8 +50,9 @@ type IndexPostingsReader interface { // If `force` is true, then all requests go through cache, regardless of the `concurrent` param provided to the PostingsForMatchers method. func NewPostingsForMatchersCache(ttl time.Duration, maxItems int, maxBytes int64, force bool) *PostingsForMatchersCache { b := &PostingsForMatchersCache{ - calls: &sync.Map{}, - cached: list.New(), + calls: &sync.Map{}, + cached: list.New(), + expireInProgress: atomic.NewBool(false), ttl: ttl, maxItems: maxItems, @@ -81,6 +83,10 @@ type PostingsForMatchersCache struct { maxBytes int64 force bool + // Signal whether there's already a call to expire() in progress, in order to avoid multiple goroutines + // cleaning up expired entries at the same time (1 at a time is enough). + expireInProgress *atomic.Bool + // timeNow is the time.Now that can be replaced for testing purposes timeNow func() time.Time @@ -248,6 +254,14 @@ func (c *PostingsForMatchersCache) expire() { return } + // Ensure there's no other cleanup in progress. It's not a technical issue if there are two ongoing cleanups, + // but it's a waste of resources and it adds extra pressure to the mutex. One cleanup at a time is enough. + if !c.expireInProgress.CompareAndSwap(false, true) { + return + } + + defer c.expireInProgress.Store(false) + c.cachedMtx.RLock() if !c.shouldEvictHead() { c.cachedMtx.RUnlock() @@ -314,19 +328,26 @@ func (c *PostingsForMatchersCache) onPromiseExecutionDone(ctx context.Context, k return } - c.cachedMtx.Lock() - defer c.cachedMtx.Unlock() + // Cache the promise. + var lastCachedBytes int64 + { + c.cachedMtx.Lock() + + c.cached.PushBack(&postingsForMatchersCachedCall{ + key: key, + ts: ts, + sizeBytes: sizeBytes, + }) + c.cachedBytes += sizeBytes + lastCachedBytes = c.cachedBytes + + c.cachedMtx.Unlock() + } - c.cached.PushBack(&postingsForMatchersCachedCall{ - key: key, - ts: ts, - sizeBytes: sizeBytes, - }) - c.cachedBytes += sizeBytes span.AddEvent("added cached value to expiry queue", trace.WithAttributes( attribute.Stringer("timestamp", ts), attribute.Int64("size in bytes", sizeBytes), - attribute.Int64("cached bytes", c.cachedBytes), + attribute.Int64("cached bytes", lastCachedBytes), )) } diff --git a/vendor/modules.txt b/vendor/modules.txt index 30fc93c71f4..4139edadb3e 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1008,7 +1008,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.99.0 => github.com/grafana/mimir-prometheus v0.0.0-20241029123536-5710f65f1444 +# github.com/prometheus/prometheus v1.99.0 => github.com/grafana/mimir-prometheus v0.0.0-20241030085501-6c2603082009 ## explicit; go 1.22.0 github.com/prometheus/prometheus/config github.com/prometheus/prometheus/discovery @@ -1676,7 +1676,7 @@ sigs.k8s.io/kustomize/kyaml/yaml/walk sigs.k8s.io/yaml sigs.k8s.io/yaml/goyaml.v2 sigs.k8s.io/yaml/goyaml.v3 -# github.com/prometheus/prometheus => github.com/grafana/mimir-prometheus v0.0.0-20241029123536-5710f65f1444 +# github.com/prometheus/prometheus => github.com/grafana/mimir-prometheus v0.0.0-20241030085501-6c2603082009 # 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-20240531075221-3685f1377d7b From 2d53ef8adbe7e35f612e9dcdfb59adefb9f6f7ef Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Wed, 30 Oct 2024 10:02:02 +0100 Subject: [PATCH 2/2] Updated CHANGELOG Signed-off-by: Marco Pracucci --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index effad973374..d5fd07d55e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ * [ENHANCEMENT] Ingester: Emit traces for block syncing, to join up block-upload traces. #9656 * [ENHANCEMENT] Querier: Enable the optional querying of additional storage queryables. #9712 * [ENHANCEMENT] Ingester: Disable the push circuit breaker when ingester is in read-only mode. #9760 +* [ENHANCEMENT] Ingester: Reduced lock contention in the `PostingsForMatchers` cache. #9773 * [BUGFIX] Fix issue where functions such as `rate()` over native histograms could return incorrect values if a float stale marker was present in the selected range. #9508 * [BUGFIX] Fix issue where negation of native histograms (eg. `-some_native_histogram_series`) did nothing. #9508 * [BUGFIX] Fix issue where `metric might not be a counter, name does not end in _total/_sum/_count/_bucket` annotation would be emitted even if `rate` or `increase` did not have enough samples to compute a result. #9508