From 8e4b10408dacf7891d48b70eba451a95bcf72853 Mon Sep 17 00:00:00 2001 From: Sandeep Sukhani Date: Wed, 9 Apr 2025 14:54:28 +0530 Subject: [PATCH] fix: avoid copying label values from tsdb unless required (#17077) --- .../shipper/indexshipper/tsdb/head_read.go | 12 ---------- .../shipper/indexshipper/tsdb/index/index.go | 12 ++-------- .../indexshipper/tsdb/index/index_test.go | 3 ++- .../shipper/indexshipper/tsdb/querier.go | 3 --- .../indexshipper/tsdb/single_file_index.go | 22 ++++++++++++++++--- 5 files changed, 23 insertions(+), 29 deletions(-) diff --git a/pkg/storage/stores/shipper/indexshipper/tsdb/head_read.go b/pkg/storage/stores/shipper/indexshipper/tsdb/head_read.go index cf709e7bd9..64f180d54f 100644 --- a/pkg/storage/stores/shipper/indexshipper/tsdb/head_read.go +++ b/pkg/storage/stores/shipper/indexshipper/tsdb/head_read.go @@ -55,18 +55,6 @@ func (h *headIndexReader) Symbols() index.StringIter { return h.head.postings.Symbols() } -// SortedLabelValues returns label values present in the head for the -// specific label name that are within the time range mint to maxt. -// If matchers are specified the returned result set is reduced -// to label values of metrics matching the matchers. -func (h *headIndexReader) SortedLabelValues(name string, matchers ...*labels.Matcher) ([]string, error) { - values, err := h.LabelValues(name, matchers...) - if err == nil { - sort.Strings(values) - } - return values, err -} - // LabelValues returns label values present in the head for the // specific label name that are within the time range mint to maxt. // If matchers are specified the returned result set is reduced diff --git a/pkg/storage/stores/shipper/indexshipper/tsdb/index/index.go b/pkg/storage/stores/shipper/indexshipper/tsdb/index/index.go index 0e10f8648a..9967599b63 100644 --- a/pkg/storage/stores/shipper/indexshipper/tsdb/index/index.go +++ b/pkg/storage/stores/shipper/indexshipper/tsdb/index/index.go @@ -1617,16 +1617,8 @@ func (r *Reader) SymbolTableSize() uint64 { return uint64(r.symbols.Size()) } -// SortedLabelValues returns value tuples that exist for the given label name. -func (r *Reader) SortedLabelValues(name string, matchers ...*labels.Matcher) ([]string, error) { - values, err := r.LabelValues(name, matchers...) - if err == nil && r.version == FormatV1 { - sort.Strings(values) - } - return values, err -} - // LabelValues returns value tuples that exist for the given label name. +// The returned values should be copied if they need to be used beyond the current tsdb read operation, including sending back as response. // TODO(replay): Support filtering by matchers func (r *Reader) LabelValues(name string, matchers ...*labels.Matcher) ([]string, error) { if len(matchers) > 0 { @@ -1670,7 +1662,7 @@ func (r *Reader) LabelValues(name string, matchers ...*labels.Matcher) ([]string } else { d.Skip(skip) } - s := string(d.UvarintBytes()) // Label value. + s := yoloString(d.UvarintBytes()) // Label value. values = append(values, s) if s == lastVal { break diff --git a/pkg/storage/stores/shipper/indexshipper/tsdb/index/index_test.go b/pkg/storage/stores/shipper/indexshipper/tsdb/index/index_test.go index 8b2ac832a5..53f25d911f 100644 --- a/pkg/storage/stores/shipper/indexshipper/tsdb/index/index_test.go +++ b/pkg/storage/stores/shipper/indexshipper/tsdb/index/index_test.go @@ -449,9 +449,10 @@ func TestPersistence_index_e2e(t *testing.T) { for k, v := range labelPairs { sort.Strings(v) - res, err := ir.SortedLabelValues(k) + res, err := ir.LabelValues(k) require.NoError(t, err) + sort.Strings(res) require.Equal(t, len(v), len(res)) for i := 0; i < len(v); i++ { require.Equal(t, v[i], res[i]) diff --git a/pkg/storage/stores/shipper/indexshipper/tsdb/querier.go b/pkg/storage/stores/shipper/indexshipper/tsdb/querier.go index 60ec32ee95..d9f886e254 100644 --- a/pkg/storage/stores/shipper/indexshipper/tsdb/querier.go +++ b/pkg/storage/stores/shipper/indexshipper/tsdb/querier.go @@ -51,9 +51,6 @@ type IndexReader interface { // beyond the lifetime of the index reader. Symbols() index.StringIter - // SortedLabelValues returns sorted possible label values. - SortedLabelValues(name string, matchers ...*labels.Matcher) ([]string, error) - // LabelValues returns possible label values which may not be sorted. LabelValues(name string, matchers ...*labels.Matcher) ([]string, error) diff --git a/pkg/storage/stores/shipper/indexshipper/tsdb/single_file_index.go b/pkg/storage/stores/shipper/indexshipper/tsdb/single_file_index.go index 6bd7e6e79a..2ce9bcd04b 100644 --- a/pkg/storage/stores/shipper/indexshipper/tsdb/single_file_index.go +++ b/pkg/storage/stores/shipper/indexshipper/tsdb/single_file_index.go @@ -7,6 +7,7 @@ import ( "io" "math" "path/filepath" + "strings" "time" "github.com/opentracing/opentracing-go" @@ -268,10 +269,17 @@ func (i *TSDBIndex) LabelNames(_ context.Context, _ string, _, _ model.Time, mat } func (i *TSDBIndex) LabelValues(_ context.Context, _ string, _, _ model.Time, name string, matchers ...*labels.Matcher) ([]string, error) { - if len(matchers) == 0 { - return i.reader.LabelValues(name) + if len(matchers) != 0 { + return labelValuesWithMatchers(i.reader, name, matchers...) + } + + labelValues, err := i.reader.LabelValues(name) + if err != nil { + return nil, err } - return labelValuesWithMatchers(i.reader, name, matchers...) + + // cloning the string + return cloneStringList(labelValues), nil } func (i *TSDBIndex) Checksum() uint32 { @@ -454,3 +462,11 @@ func (i *TSDBIndex) Volume( return p.Err() }) } + +func cloneStringList(strs []string) []string { + res := make([]string, 0, len(strs)) + for _, str := range strs { + res = append(res, strings.Clone(str)) + } + return res +}