Iterator Perfomance Improvements (#3667)

* steal benches Signed-off-by: Joe Elliott <[email protected]> * Gen floats, ints and bool predicates Signed-off-by: Joe Elliott <[email protected]> * remove string allocs Signed-off-by: Joe Elliott <[email protected]> * added gen to make Signed-off-by: Joe Elliott <[email protected]> * Gen strings Signed-off-by: Joe Elliott <[email protected]> * patch up vp2 and vp4 Signed-off-by: Joe Elliott <[email protected]> * remove instrumented predicate Signed-off-by: Joe Elliott <[email protected]> * int32 rns Signed-off-by: Joe Elliott <[email protected]> * skip nulls Signed-off-by: Joe Elliott <[email protected]> * vp4 unsafe string Signed-off-by: Joe Elliott <[email protected]> * revert isnull check due to impact on query range Signed-off-by: Joe Elliott <[email protected]> * remove string range cond Signed-off-by: Joe Elliott <[email protected]> * changelog Signed-off-by: Joe Elliott <[email protected]> * lint and tests Signed-off-by: Joe Elliott <[email protected]> * exclude Signed-off-by: Joe Elliott <[email protected]> * again Signed-off-by: Joe Elliott <[email protected]> * fix second line Signed-off-by: Joe Elliott <[email protected]> * fix bool pred Signed-off-by: Joe Elliott <[email protected]> * change generated file name and exclude Signed-off-by: Joe Elliott <[email protected]> * additional tests Signed-off-by: Joe Elliott <[email protected]> --------- Signed-off-by: Joe Elliott <[email protected]>
grafana · May 15, 2024 · 9775796 · 9775796
1 parent 16c6694
commit 9775796
Show file tree

Hide file tree

Showing 20 changed files with 1,364 additions and 415 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -45,7 +45,8 @@
 * [ENHANCEMENT] Add support for sharded ingester queries  [#3574](https://github.com/grafana/tempo/pull/3574) (@zalegrala)
 * [ENHANCEMENT] TraceQL - Add support for scoped intrinsics using `:` [#3629](https://github.com/grafana/tempo/pull/3629) (@ie-pham)
   available scoped intrinsics: trace:duration, trace:rootName, trace:rootService, span:duration, span:kind, span:name, span:status, span:statusMessage
-* [ENHANCEMENT] Performance improvements on SearchTagValuesV2. [#3650](https://github.com/grafana/tempo/pull/3650) (@joe-elliott)
+* [ENHANCEMENT] Performance improvements on TraceQL and tag value search. [#3650](https://github.com/grafana/tempo/pull/3650),[#3667](https://github.com/grafana/tempo/pull/3667) (@joe-elliott)
+* [BUGFIX] Update golang.org/x/net package to 0.24.0 to fix CVE-2023-45288 [#3613](https://github.com/grafana/tempo/pull/3613) (@pavolloffay)
 * [BUGFIX] Fix metrics query results when filtering and rating on the same attribute [#3428](https://github.com/grafana/tempo/issues/3428) (@mdisibio)
 * [BUGFIX] Fix metrics query results when series contain empty strings or nil values [#3429](https://github.com/grafana/tempo/issues/3429) (@mdisibio)
 * [BUGFIX] Fix metrics query duration check, add per-tenant override for max metrics query duration [#3479](https://github.com/grafana/tempo/issues/3479) (@mdisibio)

diff --git a/Makefile b/Makefile
@@ -55,7 +55,7 @@ ifeq ($(UNAME), Darwin)
     SED_OPTS := ''
 endif
 
-FILES_TO_FMT=$(shell find . -type d \( -path ./vendor -o -path ./opentelemetry-proto -o -path ./vendor-fix \) -prune -o -name '*.go' -not -name "*.pb.go" -not -name '*.y.go' -print)
+FILES_TO_FMT=$(shell find . -type d \( -path ./vendor -o -path ./opentelemetry-proto -o -path ./vendor-fix \) -prune -o -name '*.go' -not -name "*.pb.go" -not -name '*.y.go' -not -name '*.gen.go' -print)
 FILES_TO_JSONNETFMT=$(shell find ./operations/jsonnet ./operations/tempo-mixin -type f \( -name '*.libsonnet' -o -name '*.jsonnet' \) -not -path "*/vendor/*" -print)
 
 ### Build
@@ -268,9 +268,16 @@ gen-traceql:
 gen-traceql-local:
 	goyacc -o pkg/traceql/expr.y.go pkg/traceql/expr.y && rm y.output
 
+# ##############
+# Gen Parquet-Query
+# ##############
+.PHONY: gen-parquet-query
+gen-parquet-query:
+	go run ./pkg/parquetquerygen/predicates.go > ./pkg/parquetquery/predicates.gen.go
+
 ### Check vendored and generated files are up to date
 .PHONY: vendor-check
-vendor-check: gen-proto update-mod gen-traceql
+vendor-check: gen-proto update-mod gen-traceql gen-parquet-query
 	git diff --exit-code -- **/go.sum **/go.mod vendor/ pkg/tempopb/ pkg/traceql/
 
 ### Tidy dependencies for tempo and tempo-serverless modules

diff --git a/pkg/parquetquery/iters.go b/pkg/parquetquery/iters.go
@@ -31,7 +31,7 @@ import (
 //	  E        0,  2, -1
 //
 // Currently supports 6 levels of nesting which should be enough for anybody. :)
-type RowNumber [6]int64
+type RowNumber [6]int32
 
 const MaxDefinitionLevel = 5
 
@@ -42,7 +42,7 @@ func EmptyRowNumber() RowNumber {
 
 // MaxRowNumber is a helper that represents the maximum(-ish) representable value.
 func MaxRowNumber() RowNumber {
-	return RowNumber{math.MaxInt64}
+	return RowNumber{math.MaxInt32}
 }
 
 // CompareRowNumbers compares the sequences of row numbers in
@@ -314,7 +314,7 @@ func (t *RowNumber) nextSlow(repetitionLevel, definitionLevel int) {
 
 // Skip rows at the root-level.
 func (t *RowNumber) Skip(numRows int64) {
-	t[0] += numRows
+	t[0] += int32(numRows)
 	for i := 1; i < len(t); i++ {
 		t[i] = -1
 	}
@@ -331,7 +331,7 @@ func (t RowNumber) Preceding() RowNumber {
 		case -1:
 			continue
 		case 0:
-			t[i] = math.MaxInt64
+			t[i] = math.MaxInt32
 		default:
 			t[i]--
 			return t
@@ -533,7 +533,7 @@ type SyncIterator struct {
 	rgsMin     []RowNumber
 	rgsMax     []RowNumber // Exclusive, row number of next one past the row group
 	readSize   int
-	filter     *InstrumentedPredicate
+	filter     Predicate
 
 	// Status
 	span            opentracing.Span
@@ -596,7 +596,7 @@ func NewSyncIterator(ctx context.Context, rgs []pq.RowGroup, column int, columnN
 		readSize:   readSize,
 		rgsMin:     rgsMin,
 		rgsMax:     rgsMax,
-		filter:     &InstrumentedPredicate{pred: filter},
+		filter:     filter,
 		curr:       EmptyRowNumber(),
 		at:         at,
 	}
@@ -821,12 +821,12 @@ func (c *SyncIterator) seekWithinPage(to RowNumber, definitionLevel int) {
 	if rowSkip < 1 {
 		return
 	}
-	if rowSkip > c.currPage.NumRows() {
+	if rowSkip > int32(c.currPage.NumRows()) {
 		return
 	}
 
 	// reslice the page to jump directly to the desired row number
-	pg := c.currPage.Slice(rowSkip-1, c.currPage.NumRows())
+	pg := c.currPage.Slice(int64(rowSkip-1), c.currPage.NumRows())
 
 	// remove all detail below the row number
 	c.curr = TruncateRowNumber(0, to)
@@ -997,12 +997,6 @@ func (c *SyncIterator) makeResult(t RowNumber, v *pq.Value) *IteratorResult {
 func (c *SyncIterator) Close() {
 	c.closeCurrRowGroup()
 
-	c.span.SetTag("inspectedColumnChunks", c.filter.InspectedColumnChunks)
-	c.span.SetTag("inspectedPages", c.filter.InspectedPages)
-	c.span.SetTag("inspectedValues", c.filter.InspectedValues)
-	c.span.SetTag("keptColumnChunks", c.filter.KeptColumnChunks)
-	c.span.SetTag("keptPages", c.filter.KeptPages)
-	c.span.SetTag("keptValues", c.filter.KeptValues)
 	c.span.Finish()
 
 	if c.intern && c.interner != nil {

diff --git a/pkg/parquetquery/iters_test.go b/pkg/parquetquery/iters_test.go
@@ -89,7 +89,7 @@ func TestRowNumberPreceding(t *testing.T) {
 		start, preceding RowNumber
 	}{
 		{RowNumber{1000, -1, -1, -1, -1, -1}, RowNumber{999, -1, -1, -1, -1, -1}},
-		{RowNumber{1000, 0, 0, 0, 0, 0}, RowNumber{999, math.MaxInt64, math.MaxInt64, math.MaxInt64, math.MaxInt64, math.MaxInt64}},
+		{RowNumber{1000, 0, 0, 0, 0, 0}, RowNumber{999, math.MaxInt32, math.MaxInt32, math.MaxInt32, math.MaxInt32, math.MaxInt32}},
 	}
 
 	for _, tc := range testCases {
@@ -117,7 +117,7 @@ func testColumnIterator(t *testing.T, makeIter makeTestIterFn) {
 		res, err := iter.Next()
 		require.NoError(t, err)
 		require.NotNil(t, res, "i=%d", i)
-		require.Equal(t, RowNumber{int64(i), -1, -1, -1, -1, -1}, res.RowNumber)
+		require.Equal(t, RowNumber{int32(i), -1, -1, -1, -1, -1}, res.RowNumber)
 		require.Equal(t, int64(i), res.ToMap()["A"][0].Int64())
 	}
 
@@ -142,7 +142,7 @@ func testColumnIteratorSeek(t *testing.T, makeIter makeTestIterFn) {
 	iter := makeIter(pf, idx, nil, "A")
 	defer iter.Close()
 
-	seekTos := []int64{
+	seekTos := []int32{
 		100,
 		1234,
 		4567,
@@ -157,7 +157,7 @@ func testColumnIteratorSeek(t *testing.T, makeIter makeTestIterFn) {
 		require.NoError(t, err)
 		require.NotNil(t, res, "seekTo=%v", seekTo)
 		require.Equal(t, RowNumber{seekTo, -1, -1, -1, -1, -1}, res.RowNumber)
-		require.Equal(t, seekTo, res.ToMap()["A"][0].Int64())
+		require.Equal(t, seekTo, res.ToMap()["A"][0].Int32())
 	}
 }
 
@@ -179,7 +179,7 @@ func testColumnIteratorPredicate(t *testing.T, makeIter makeTestIterFn) {
 	iter := makeIter(pf, idx, pred, "A")
 	defer iter.Close()
 
-	expectedResults := []int64{
+	expectedResults := []int32{
 		7001,
 		7002,
 		7003,
@@ -190,7 +190,7 @@ func testColumnIteratorPredicate(t *testing.T, makeIter makeTestIterFn) {
 		require.NoError(t, err)
 		require.NotNil(t, res)
 		require.Equal(t, RowNumber{expectedResult, -1, -1, -1, -1, -1}, res.RowNumber)
-		require.Equal(t, expectedResult, res.ToMap()["A"][0].Int64())
+		require.Equal(t, expectedResult, res.ToMap()["A"][0].Int32())
 	}
 }