Skip to content

Conversation

harry671003
Copy link
Contributor

@harry671003 harry671003 commented Oct 30, 2024

What this PR does:
Updating thanos dependency to thanos-io/thanos@6203811.

The only major change that will impact cortex is:

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@dosubot dosubot bot added dependencies Pull requests that update a dependency file go Pull requests that update Go code labels Oct 30, 2024
@yeya24
Copy link
Contributor

yeya24 commented Oct 30, 2024

For the lint issue, let's upgrade the dependency to v0.3.4, similar to prometheus/prometheus#15166?

@yeya24
Copy link
Contributor

yeya24 commented Oct 30, 2024

@SungJin1212 https://github.com/cortexproject/cortex/actions/runs/11585758704/job/32255315640?pr=6294
This may related to the new disable chunk trimming change

Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
@harry671003
Copy link
Contributor Author

Looks like the test is flaky.

@SungJin1212
Copy link
Member

@harry671003
Yes, it seems that.
I get the intermittent error even if two Cortexs use the same image, quay.io/cortexproject/cortex:v1.18.0. It seems to be a bug in the test code.

Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Oct 30, 2024
@yeya24 yeya24 merged commit 00c7b68 into cortexproject:master Oct 30, 2024
16 checks passed
CharlieTLe pushed a commit to CharlieTLe/cortex that referenced this pull request Dec 3, 2024
diff --git a/go.mod b/go.mod
index acdf36a..8b32f21 100644
--- a/go.mod
+++ b/go.mod
@@ -53,7 +53,7 @@ require (
 	github.com/stretchr/testify v1.9.0
 	github.com/thanos-io/objstore v0.0.0-20240913074259-63feed0da069
 	github.com/thanos-io/promql-engine v0.0.0-20240921092401-37747eddbd31
-	github.com/thanos-io/thanos v0.35.2-0.20241011111532-af0900bfd290
+	github.com/thanos-io/thanos v0.35.2-0.20241029125830-62038110b1bc
 	github.com/uber/jaeger-client-go v2.30.0+incompatible
 	github.com/weaveworks/common v0.0.0-20230728070032-dd9e68f319d5
 	go.etcd.io/etcd/api/v3 v3.5.16
diff --git a/go.sum b/go.sum
index a1b95fa..7b6d62f 100644
--- a/go.sum
+++ b/go.sum
@@ -1688,8 +1688,8 @@ github.com/thanos-io/objstore v0.0.0-20240913074259-63feed0da069 h1:TUPZ6euAh8I6
 github.com/thanos-io/objstore v0.0.0-20240913074259-63feed0da069/go.mod h1:Cba80S8NbVBBdyZKzra7San/jXvpAxArbpFymWzIZhg=
 github.com/thanos-io/promql-engine v0.0.0-20240921092401-37747eddbd31 h1:xPaP58g+3EPohdw4cv+6jv5+LcX6LynhHvQcYwTAMxQ=
 github.com/thanos-io/promql-engine v0.0.0-20240921092401-37747eddbd31/go.mod h1:wx0JlRZtsB2S10JYUgeg5GqLfMxw31SzArP+28yyE00=
-github.com/thanos-io/thanos v0.35.2-0.20241011111532-af0900bfd290 h1:d58OLbcIC6F3TviRFK85Ucdxbs/3cPI1fcLRBWiNThA=
-github.com/thanos-io/thanos v0.35.2-0.20241011111532-af0900bfd290/go.mod h1:kqF1rQspIAL+rktXL3OSKfiDjJmsCRezQblsR56degY=
+github.com/thanos-io/thanos v0.35.2-0.20241029125830-62038110b1bc h1:AoIDFFb3xjED+mmf5g6bnULDgNJWeXtKyjAQ1CKtqfk=
+github.com/thanos-io/thanos v0.35.2-0.20241029125830-62038110b1bc/go.mod h1:6Q6xfe4mUYCYbrixod2dNZqKCK4J9pGRKqF29/cUr/A=
 github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926/go.mod h1:9ESjWnEqriFuLhtthL60Sar/7RFoluCcXsuvEwTV5KM=
 github.com/uber/jaeger-client-go v2.28.0+incompatible/go.mod h1:WVhlPFC8FDjOFMMWRy2pZqQJSXxYSwNYOkTr/Z6d3Kk=
 github.com/uber/jaeger-client-go v2.30.0+incompatible h1:D6wyKGCecFaSRUpo8lCVbaOOb6ThwMmTEbhRwtKR97o=
diff --git a/vendor/github.com/thanos-io/thanos/pkg/store/bucket.go b/vendor/github.com/thanos-io/thanos/pkg/store/bucket.go
index 865ea38..7ab47f4 100644
--- a/vendor/github.com/thanos-io/thanos/pkg/store/bucket.go
+++ b/vendor/github.com/thanos-io/thanos/pkg/store/bucket.go
@@ -1574,8 +1574,6 @@ func (s *BucketStore) Series(req *storepb.SeriesRequest, seriesSrv storepb.Store
 				tenant,
 			)

-			defer blockClient.Close()
-
 			g.Go(func() error {

 				span, _ := tracing.StartSpan(gctx, "bucket_store_block_series", tracing.Tags{
@@ -1992,6 +1990,14 @@ func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesR

 	resHints := &hintspb.LabelValuesResponseHints{}

+	var hasMetricNameEqMatcher = false
+	for _, m := range reqSeriesMatchers {
+		if m.Name == labels.MetricName && m.Type == labels.MatchEqual {
+			hasMetricNameEqMatcher = true
+			break
+		}
+	}
+
 	g, gctx := errgroup.WithContext(ctx)

 	var reqBlockMatchers []*labels.Matcher
@@ -2015,6 +2021,7 @@ func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesR
 	var seriesLimiter = s.seriesLimiterFactory(s.metrics.queriesDropped.WithLabelValues("series", tenant))
 	var bytesLimiter = s.bytesLimiterFactory(s.metrics.queriesDropped.WithLabelValues("bytes", tenant))
 	var logger = s.requestLoggerFunc(ctx, s.logger)
+	var stats = &queryStats{}

 	for _, b := range s.blocks {
 		b := b
@@ -2033,7 +2040,8 @@ func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesR

 		// If we have series matchers and the Label is not an external one, add <labelName> != "" matcher
 		// to only select series that have given label name.
-		if len(reqSeriesMatchersNoExtLabels) > 0 && !b.extLset.Has(req.Label) {
+		// We don't need such matcher if matchers already contain __name__=="something" matcher.
+		if !hasMetricNameEqMatcher && len(reqSeriesMatchersNoExtLabels) > 0 && !b.extLset.Has(req.Label) {
 			m, err := labels.NewMatcher(labels.MatchNotEqual, req.Label, "")
 			if err != nil {
 				return nil, status.Error(codes.InvalidArgument, err.Error())
@@ -2101,7 +2109,12 @@ func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesR
 					s.metrics.lazyExpandedPostingSeriesOverfetchedSizeBytes,
 					tenant,
 				)
-				defer blockClient.Close()
+				defer func() {
+					mtx.Lock()
+					stats = blockClient.MergeStats(stats)
+					mtx.Unlock()
+					blockClient.Close()
+				}()

 				if err := blockClient.ExpandPostings(
 					sortedReqSeriesMatchersNoExtLabels,
@@ -2130,7 +2143,7 @@ func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesR
 					}

 					val := labelpb.ZLabelsToPromLabels(ls.GetSeries().Labels).Get(req.Label)
-					if val != "" { // Should never be empty since we added labelName!="" matcher to the list of matchers.
+					if val != "" {
 						values[val] = struct{}{}
 					}
 				}
@@ -2154,6 +2167,15 @@ func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesR

 	s.mtx.RUnlock()

+	defer func() {
+		if s.debugLogging {
+			level.Debug(logger).Log("msg", "stats query processed",
+				"request", req,
+				"tenant", tenant,
+				"stats", fmt.Sprintf("%+v", stats), "err", err)
+		}
+	}()
+
 	if err := g.Wait(); err != nil {
 		code := codes.Internal
 		if s, ok := status.FromError(errors.Cause(err)); ok {
@@ -3356,6 +3378,11 @@ func (r *bucketIndexReader) Close() error {
 	return nil
 }

+func (b *blockSeriesClient) CloseSend() error {
+	b.Close()
+	return nil
+}
+
 // LookupLabelsSymbols allows populates label set strings from symbolized label set.
 func (r *bucketIndexReader) LookupLabelsSymbols(ctx context.Context, symbolized []symbolizedLabel, b *labels.Builder) error {
 	b.Reset(labels.EmptyLabels())
diff --git a/vendor/github.com/thanos-io/thanos/pkg/store/proxy_merge.go b/vendor/github.com/thanos-io/thanos/pkg/store/proxy_merge.go
index 29d1e65..4442cf8 100644
--- a/vendor/github.com/thanos-io/thanos/pkg/store/proxy_merge.go
+++ b/vendor/github.com/thanos-io/thanos/pkg/store/proxy_merge.go
@@ -219,6 +219,7 @@ func (l *lazyRespSet) StoreLabels() map[string]struct{} {
 type lazyRespSet struct {
 	// Generic parameters.
 	span           opentracing.Span
+	cl             storepb.Store_SeriesClient
 	closeSeries    context.CancelFunc
 	storeName      string
 	storeLabelSets []labels.Labels
@@ -318,6 +319,7 @@ func newLazyRespSet(
 		frameTimeout:         frameTimeout,
 		storeName:            storeName,
 		storeLabelSets:       storeLabelSets,
+		cl:                   cl,
 		closeSeries:          closeSeries,
 		span:                 span,
 		dataOrFinishEvent:    dataAvailable,
@@ -446,8 +448,10 @@ func newAsyncRespSet(
 	emptyStreamResponses prometheus.Counter,
 ) (respSet, error) {

-	var span opentracing.Span
-	var closeSeries context.CancelFunc
+	var (
+		span   opentracing.Span
+		cancel context.CancelFunc
+	)

 	storeID, storeAddr, isLocalStore := storeInfo(st)
 	seriesCtx := grpc_opentracing.ClientAddContextTags(ctx, opentracing.Tags{
@@ -459,7 +463,7 @@ func newAsyncRespSet(
 		"store.addr":     storeAddr,
 	})

-	seriesCtx, closeSeries = context.WithCancel(seriesCtx)
+	seriesCtx, cancel = context.WithCancel(seriesCtx)

 	shardMatcher := shardInfo.Matcher(buffers)

@@ -474,7 +478,7 @@ func newAsyncRespSet(

 		span.SetTag("err", err.Error())
 		span.Finish()
-		closeSeries()
+		cancel()
 		return nil, err
 	}

@@ -497,7 +501,7 @@ func newAsyncRespSet(
 			frameTimeout,
 			st.String(),
 			st.LabelSets(),
-			closeSeries,
+			cancel,
 			cl,
 			shardMatcher,
 			applySharding,
@@ -509,7 +513,7 @@ func newAsyncRespSet(
 			frameTimeout,
 			st.String(),
 			st.LabelSets(),
-			closeSeries,
+			cancel,
 			cl,
 			shardMatcher,
 			applySharding,
@@ -530,6 +534,7 @@ func (l *lazyRespSet) Close() {
 	l.dataOrFinishEvent.Signal()

 	l.shardMatcher.Close()
+	_ = l.cl.CloseSend()
 }

 // eagerRespSet is a SeriesSet that blocks until all data is retrieved from
@@ -539,6 +544,7 @@ type eagerRespSet struct {
 	// Generic parameters.
 	span opentracing.Span

+	cl           storepb.Store_SeriesClient
 	closeSeries  context.CancelFunc
 	frameTimeout time.Duration

@@ -569,6 +575,7 @@ func newEagerRespSet(
 ) respSet {
 	ret := &eagerRespSet{
 		span:              span,
+		cl:                cl,
 		closeSeries:       closeSeries,
 		frameTimeout:      frameTimeout,
 		bufferedResponses: []*storepb.SeriesResponse{},
@@ -717,6 +724,7 @@ func (l *eagerRespSet) Close() {
 		l.closeSeries()
 	}
 	l.shardMatcher.Close()
+	_ = l.cl.CloseSend()
 }

 func (l *eagerRespSet) At() *storepb.SeriesResponse {
diff --git a/vendor/github.com/thanos-io/thanos/pkg/store/storepb/inprocess.go b/vendor/github.com/thanos-io/thanos/pkg/store/storepb/inprocess.go
index a5b792b..0c3e764 100644
--- a/vendor/github.com/thanos-io/thanos/pkg/store/storepb/inprocess.go
+++ b/vendor/github.com/thanos-io/thanos/pkg/store/storepb/inprocess.go
@@ -55,6 +55,9 @@ func (c *inProcessClient) Recv() (*SeriesResponse, error) {
 		return nil, err
 	}
 	if !ok {
+		if c.ctx.Err() != nil {
+			return nil, c.ctx.Err()
+		}
 		return nil, io.EOF
 	}
 	return resp, err
diff --git a/vendor/github.com/thanos-io/thanos/pkg/store/storepb/rpc.pb.go b/vendor/github.com/thanos-io/thanos/pkg/store/storepb/rpc.pb.go
index 0c2d67d..9d39f27 100644
--- a/vendor/github.com/thanos-io/thanos/pkg/store/storepb/rpc.pb.go
+++ b/vendor/github.com/thanos-io/thanos/pkg/store/storepb/rpc.pb.go
@@ -824,6 +824,9 @@ type StoreClient interface {
 	///
 	/// There is no requirements on chunk sorting, however it is recommended to have chunk sorted by chunk min time.
 	/// This heavily optimizes the resource usage on Querier / Federated Queries.
+	///
+	/// Chunks can span a range larger than the requested min and max time and it is up to the query engine to discard samples
+	/// which fall outside of the query range.
 	Series(ctx context.Context, in *SeriesRequest, opts ...grpc.CallOption) (Store_SeriesClient, error)
 	/// LabelNames returns all label names constrained by the given matchers.
 	LabelNames(ctx context.Context, in *LabelNamesRequest, opts ...grpc.CallOption) (*LabelNamesResponse, error)
@@ -900,6 +903,9 @@ type StoreServer interface {
 	///
 	/// There is no requirements on chunk sorting, however it is recommended to have chunk sorted by chunk min time.
 	/// This heavily optimizes the resource usage on Querier / Federated Queries.
+	///
+	/// Chunks can span a range larger than the requested min and max time and it is up to the query engine to discard samples
+	/// which fall outside of the query range.
 	Series(*SeriesRequest, Store_SeriesServer) error
 	/// LabelNames returns all label names constrained by the given matchers.
 	LabelNames(context.Context, *LabelNamesRequest) (*LabelNamesResponse, error)
diff --git a/vendor/github.com/thanos-io/thanos/pkg/store/storepb/rpc.proto b/vendor/github.com/thanos-io/thanos/pkg/store/storepb/rpc.proto
index 2a6313e..2de086b 100644
--- a/vendor/github.com/thanos-io/thanos/pkg/store/storepb/rpc.proto
+++ b/vendor/github.com/thanos-io/thanos/pkg/store/storepb/rpc.proto
@@ -33,6 +33,9 @@ service Store {
   ///
   /// There is no requirements on chunk sorting, however it is recommended to have chunk sorted by chunk min time.
   /// This heavily optimizes the resource usage on Querier / Federated Queries.
+  ///
+  /// Chunks can span a range larger than the requested min and max time and it is up to the query engine to discard samples
+  /// which fall outside of the query range.
   rpc Series(SeriesRequest) returns (stream SeriesResponse);

   /// LabelNames returns all label names constrained by the given matchers.
diff --git a/vendor/github.com/thanos-io/thanos/pkg/store/tsdb.go b/vendor/github.com/thanos-io/thanos/pkg/store/tsdb.go
index 08c3122..737fee3 100644
--- a/vendor/github.com/thanos-io/thanos/pkg/store/tsdb.go
+++ b/vendor/github.com/thanos-io/thanos/pkg/store/tsdb.go
@@ -272,9 +272,10 @@ func (s *TSDBStore) Series(r *storepb.SeriesRequest, seriesSrv storepb.Store_Ser
 	}

 	hints := &storage.SelectHints{
-		Start: r.MinTime,
-		End:   r.MaxTime,
-		Limit: int(r.Limit),
+		Start:           r.MinTime,
+		End:             r.MaxTime,
+		Limit:           int(r.Limit),
+		DisableTrimming: true,
 	}
 	set := q.Select(srv.Context(), true, hints, matchers...)

diff --git a/vendor/github.com/thanos-io/thanos/pkg/strutil/labels.go b/vendor/github.com/thanos-io/thanos/pkg/strutil/labels.go
new file mode 100644
index 000000000..5423fd3
--- /dev/null
+++ b/vendor/github.com/thanos-io/thanos/pkg/strutil/labels.go
@@ -0,0 +1,25 @@
+// Copyright (c) The Thanos Authors.
+// Licensed under the Apache License 2.0.
+
+package strutil
+
+import (
+	"slices"
+	"strings"
+)
+
+// ParseFlagLabels helps handle lists of labels passed from kingpin flags.
+// * Split flag parts that are comma separated.
+// * Remove any empty strings.
+// * Sort and deduplicate the slice.
+func ParseFlagLabels(f []string) []string {
+	var result []string
+	for _, l := range f {
+		if l == "" {
+			continue
+		}
+		result = append(result, strings.Split(l, ",")...)
+	}
+	slices.Sort(result)
+	return slices.Compact(result)
+}
diff --git a/vendor/github.com/thanos-io/thanos/pkg/tls/options.go b/vendor/github.com/thanos-io/thanos/pkg/tls/options.go
index 362f737..3b6a52e 100644
--- a/vendor/github.com/thanos-io/thanos/pkg/tls/options.go
+++ b/vendor/github.com/thanos-io/thanos/pkg/tls/options.go
@@ -6,8 +6,11 @@ package tls
 import (
 	"crypto/tls"
 	"crypto/x509"
+	"fmt"
 	"os"
 	"path/filepath"
+	"sort"
+	"strings"
 	"sync"
 	"time"

@@ -17,7 +20,7 @@ import (
 )

 // NewServerConfig provides new server TLS configuration.
-func NewServerConfig(logger log.Logger, certPath, keyPath, clientCA string) (*tls.Config, error) {
+func NewServerConfig(logger log.Logger, certPath, keyPath, clientCA, tlsMinVersion string) (*tls.Config, error) {
 	if keyPath == "" && certPath == "" {
 		if clientCA != "" {
 			return nil, errors.New("when a client CA is used a server key and certificate must also be provided")
@@ -33,8 +36,13 @@ func NewServerConfig(logger log.Logger, certPath, keyPath, clientCA string) (*tl
 		return nil, errors.New("both server key and certificate must be provided")
 	}

+	minTlsVersion, err := getTlsVersion(tlsMinVersion)
+	if err != nil {
+		return nil, err
+	}
+
 	tlsCfg := &tls.Config{
-		MinVersion: tls.VersionTLS13,
+		MinVersion: minTlsVersion,
 	}
 	// Certificate is loaded during server startup to check for any errors.
 	certificate, err := tls.LoadX509KeyPair(certPath, keyPath)
@@ -190,3 +198,35 @@ func (m *clientTLSManager) getClientCertificate(*tls.CertificateRequestInfo) (*t

 	return m.cert, nil
 }
+
+type validOption struct {
+	tlsOption map[string]uint16
+}
+
+func (validOption validOption) joinString() string {
+	var keys []string
+
+	for key := range validOption.tlsOption {
+		keys = append(keys, key)
+	}
+	sort.Strings(keys)
+	return strings.Join(keys, ", ")
+}
+
+func getTlsVersion(tlsMinVersion string) (uint16, error) {
+
+	validOption := validOption{
+		tlsOption: map[string]uint16{
+			"1.0": tls.VersionTLS10,
+			"1.1": tls.VersionTLS11,
+			"1.2": tls.VersionTLS12,
+			"1.3": tls.VersionTLS13,
+		},
+	}
+
+	if _, ok := validOption.tlsOption[tlsMinVersion]; !ok {
+		return 0, errors.New(fmt.Sprintf("invalid TLS version: %s, valid values are %s", tlsMinVersion, validOption.joinString()))
+	}
+
+	return validOption.tlsOption[tlsMinVersion], nil
+}
diff --git a/vendor/modules.txt b/vendor/modules.txt
index 2eb463c..4eb5acd 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -982,7 +982,7 @@ github.com/thanos-io/promql-engine/query
 github.com/thanos-io/promql-engine/ringbuffer
 github.com/thanos-io/promql-engine/storage
 github.com/thanos-io/promql-engine/storage/prometheus
-# github.com/thanos-io/thanos v0.35.2-0.20241011111532-af0900bfd290
+# github.com/thanos-io/thanos v0.35.2-0.20241029125830-62038110b1bc
 ## explicit; go 1.23.0
 github.com/thanos-io/thanos/pkg/api/query/querypb
 github.com/thanos-io/thanos/pkg/block
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file go Pull requests that update Go code lgtm This PR has been approved by a maintainer size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants