Parquet Update + Fetch Spans Cleanup (#2191)

* update Signed-off-by: Joe Elliott <number101010@gmail.com> * simplify span release Signed-off-by: Joe Elliott <number101010@gmail.com> --------- Signed-off-by: Joe Elliott <number101010@gmail.com>
grafana · Mar 20, 2023 · 2fd223b · 2fd223b
1 parent ddca387
commit 2fd223b
Show file tree

Hide file tree

Showing 16 changed files with 60 additions and 69 deletions.
diff --git a/cmd/tempo-serverless/cloud-run/go.mod b/cmd/tempo-serverless/cloud-run/go.mod
@@ -99,7 +99,7 @@ require (
 	github.com/rivo/uniseg v0.2.0 // indirect
 	github.com/rs/xid v1.2.1 // indirect
 	github.com/segmentio/encoding v0.3.5 // indirect
-	github.com/segmentio/parquet-go v0.0.0-20230201213121-e1109e2f6a20 // indirect
+	github.com/segmentio/parquet-go v0.0.0-20230309140036-b6d0a6236da6 // indirect
 	github.com/sercand/kuberesolver v2.4.0+incompatible // indirect
 	github.com/sirupsen/logrus v1.8.1 // indirect
 	github.com/sony/gobreaker v0.4.1 // indirect

diff --git a/cmd/tempo-serverless/cloud-run/go.sum b/cmd/tempo-serverless/cloud-run/go.sum
@@ -467,8 +467,8 @@ github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQD
 github.com/segmentio/asm v1.1.3/go.mod h1:Ld3L4ZXGNcSLRg4JBsZ3//1+f/TjYl0Mzen/DQy1EJg=
 github.com/segmentio/encoding v0.3.5 h1:UZEiaZ55nlXGDL92scoVuw00RmiRCazIEmvPSbSvt8Y=
 github.com/segmentio/encoding v0.3.5/go.mod h1:n0JeuIqEQrQoPDGsjo8UNd1iA0U8d8+oHAA4E3G3OxM=
-github.com/segmentio/parquet-go v0.0.0-20230201213121-e1109e2f6a20 h1:4BzlTkAqAj8HsgqXBDiFppY7AHWaeeZmBSdQtwXk/qg=
-github.com/segmentio/parquet-go v0.0.0-20230201213121-e1109e2f6a20/go.mod h1:SclLlCfB7c7CH0YerV+OtYmZExyK5rhVOd6UT90erVw=
+github.com/segmentio/parquet-go v0.0.0-20230309140036-b6d0a6236da6 h1:XH/DKem9en9377KZvez9O/QwkXsFJGfuDzjM4c/NX4Y=
+github.com/segmentio/parquet-go v0.0.0-20230309140036-b6d0a6236da6/go.mod h1:SclLlCfB7c7CH0YerV+OtYmZExyK5rhVOd6UT90erVw=
 github.com/sercand/kuberesolver v2.4.0+incompatible h1:WE2OlRf6wjLxHwNkkFLQGaZcVLEXjMjBPjjEU5vksH8=
 github.com/sercand/kuberesolver v2.4.0+incompatible/go.mod h1:lWF3GL0xptCB/vCiJPl/ZshwPsX/n4Y7u0CW9E7aQIQ=
 github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo=

diff --git a/cmd/tempo-serverless/lambda/go.mod b/cmd/tempo-serverless/lambda/go.mod
@@ -101,7 +101,7 @@ require (
 	github.com/rivo/uniseg v0.2.0 // indirect
 	github.com/rs/xid v1.2.1 // indirect
 	github.com/segmentio/encoding v0.3.5 // indirect
-	github.com/segmentio/parquet-go v0.0.0-20230201213121-e1109e2f6a20 // indirect
+	github.com/segmentio/parquet-go v0.0.0-20230309140036-b6d0a6236da6 // indirect
 	github.com/sercand/kuberesolver v2.4.0+incompatible // indirect
 	github.com/sirupsen/logrus v1.8.1 // indirect
 	github.com/sony/gobreaker v0.4.1 // indirect

diff --git a/cmd/tempo-serverless/lambda/go.sum b/cmd/tempo-serverless/lambda/go.sum
@@ -472,8 +472,8 @@ github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQD
 github.com/segmentio/asm v1.1.3/go.mod h1:Ld3L4ZXGNcSLRg4JBsZ3//1+f/TjYl0Mzen/DQy1EJg=
 github.com/segmentio/encoding v0.3.5 h1:UZEiaZ55nlXGDL92scoVuw00RmiRCazIEmvPSbSvt8Y=
 github.com/segmentio/encoding v0.3.5/go.mod h1:n0JeuIqEQrQoPDGsjo8UNd1iA0U8d8+oHAA4E3G3OxM=
-github.com/segmentio/parquet-go v0.0.0-20230201213121-e1109e2f6a20 h1:4BzlTkAqAj8HsgqXBDiFppY7AHWaeeZmBSdQtwXk/qg=
-github.com/segmentio/parquet-go v0.0.0-20230201213121-e1109e2f6a20/go.mod h1:SclLlCfB7c7CH0YerV+OtYmZExyK5rhVOd6UT90erVw=
+github.com/segmentio/parquet-go v0.0.0-20230309140036-b6d0a6236da6 h1:XH/DKem9en9377KZvez9O/QwkXsFJGfuDzjM4c/NX4Y=
+github.com/segmentio/parquet-go v0.0.0-20230309140036-b6d0a6236da6/go.mod h1:SclLlCfB7c7CH0YerV+OtYmZExyK5rhVOd6UT90erVw=
 github.com/sercand/kuberesolver v2.4.0+incompatible h1:WE2OlRf6wjLxHwNkkFLQGaZcVLEXjMjBPjjEU5vksH8=
 github.com/sercand/kuberesolver v2.4.0+incompatible/go.mod h1:lWF3GL0xptCB/vCiJPl/ZshwPsX/n4Y7u0CW9E7aQIQ=
 github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc=

diff --git a/go.mod b/go.mod
@@ -61,7 +61,7 @@ require (
 	github.com/prometheus/prometheus v1.8.2-0.20221021121301-51a44e6657c3
 	github.com/prometheus/statsd_exporter v0.21.0 // indirect
 	github.com/segmentio/fasthash v0.0.0-20180216231524-a72b379d632e
-	github.com/segmentio/parquet-go v0.0.0-20230201213121-e1109e2f6a20
+	github.com/segmentio/parquet-go v0.0.0-20230309140036-b6d0a6236da6
 	github.com/sirupsen/logrus v1.8.1
 	github.com/sony/gobreaker v0.4.1
 	github.com/spf13/viper v1.12.0

diff --git a/go.sum b/go.sum
@@ -905,8 +905,8 @@ github.com/segmentio/encoding v0.3.5 h1:UZEiaZ55nlXGDL92scoVuw00RmiRCazIEmvPSbSv
 github.com/segmentio/encoding v0.3.5/go.mod h1:n0JeuIqEQrQoPDGsjo8UNd1iA0U8d8+oHAA4E3G3OxM=
 github.com/segmentio/fasthash v0.0.0-20180216231524-a72b379d632e h1:uO75wNGioszjmIzcY/tvdDYKRLVvzggtAmmJkn9j4GQ=
 github.com/segmentio/fasthash v0.0.0-20180216231524-a72b379d632e/go.mod h1:tm/wZFQ8e24NYaBGIlnO2WGCAi67re4HHuOm0sftE/M=
-github.com/segmentio/parquet-go v0.0.0-20230201213121-e1109e2f6a20 h1:4BzlTkAqAj8HsgqXBDiFppY7AHWaeeZmBSdQtwXk/qg=
-github.com/segmentio/parquet-go v0.0.0-20230201213121-e1109e2f6a20/go.mod h1:SclLlCfB7c7CH0YerV+OtYmZExyK5rhVOd6UT90erVw=
+github.com/segmentio/parquet-go v0.0.0-20230309140036-b6d0a6236da6 h1:XH/DKem9en9377KZvez9O/QwkXsFJGfuDzjM4c/NX4Y=
+github.com/segmentio/parquet-go v0.0.0-20230309140036-b6d0a6236da6/go.mod h1:SclLlCfB7c7CH0YerV+OtYmZExyK5rhVOd6UT90erVw=
 github.com/sercand/kuberesolver v2.4.0+incompatible h1:WE2OlRf6wjLxHwNkkFLQGaZcVLEXjMjBPjjEU5vksH8=
 github.com/sercand/kuberesolver v2.4.0+incompatible/go.mod h1:lWF3GL0xptCB/vCiJPl/ZshwPsX/n4Y7u0CW9E7aQIQ=
 github.com/shirou/gopsutil/v3 v3.22.6 h1:FnHOFOh+cYAM0C30P+zysPISzlknLC5Z1G4EAElznfQ=

diff --git a/pkg/traceql/ast_test.go b/pkg/traceql/ast_test.go
@@ -236,8 +236,6 @@ type mockSpan struct {
 	startTimeUnixNanos uint64
 	endTimeUnixNanos   uint64
 	attributes         map[Attribute]Static
-
-	wasReleased bool
 }
 
 func (m *mockSpan) Attributes() map[Attribute]Static {
@@ -252,6 +250,3 @@ func (m *mockSpan) StartTimeUnixNanos() uint64 {
 func (m *mockSpan) EndtimeUnixNanos() uint64 {
 	return m.endTimeUnixNanos
 }
-func (m *mockSpan) Release() {
-	m.wasReleased = true
-}
diff --git a/pkg/traceql/engine.go b/pkg/traceql/engine.go
@@ -52,12 +52,6 @@ func (e *Engine) Execute(ctx context.Context, searchReq *tempopb.SearchRequest,
 
 		spansetsEvaluated++
 		if len(evalSS) == 0 {
-			// this is an easy place to release. the engine rejected every single span. just release
-			// them all back to the fetch layer
-			for _, s := range inSS.Spans {
-				s.Release()
-			}
-
 			return nil, nil
 		}
 

diff --git a/pkg/traceql/engine_test.go b/pkg/traceql/engine_test.go
@@ -136,38 +136,6 @@ func TestEngine_Execute(t *testing.T) {
 	assert.Equal(t, expectedTraceSearchMetadata, response.Traces)
 }
 
-func TestEngine_ReleasesOnDrop(t *testing.T) {
-	e := Engine{}
-
-	req := &tempopb.SearchRequest{
-		Query: `{ false }`,
-	}
-	span := &mockSpan{}
-	spanSetFetcher := MockSpanSetFetcher{
-		iterator: &MockSpanSetIterator{
-			results: []*Spanset{
-				{
-					Spans: []Span{span},
-				},
-			},
-		},
-	}
-
-	// should release because all spans dropped
-	_, err := e.Execute(context.Background(), req, &spanSetFetcher)
-	require.NoError(t, err)
-	require.True(t, span.wasReleased)
-
-	// reset
-	req.Query = `{ true }`
-	span.wasReleased = false
-
-	// should keep because some spans kept
-	_, err = e.Execute(context.Background(), req, &spanSetFetcher)
-	require.NoError(t, err)
-	require.False(t, span.wasReleased)
-}
-
 func TestEngine_asTraceSearchMetadata(t *testing.T) {
 	now := time.Now()
 

diff --git a/pkg/traceql/storage.go b/pkg/traceql/storage.go
@@ -54,8 +54,6 @@ type Span interface {
 	ID() []byte
 	StartTimeUnixNanos() uint64
 	EndtimeUnixNanos() uint64
-
-	Release()
 }
 
 type Spanset struct {

diff --git a/tempodb/encoding/vparquet/block_traceql.go b/tempodb/encoding/vparquet/block_traceql.go
@@ -279,6 +279,13 @@ func (i *spansetIterator) Next() (*span, error) {
 			if err != nil {
 				return nil, err
 			}
+			// if the filter removed all spansets then let's release all back to the pool
+			// no reason to try anything more nuanced than this. it will handle nearly all cases
+			if len(filteredSpansets) == 0 {
+				for _, s := range spanset.Spans {
+					putSpan(s.(*span))
+				}
+			}
 		} else {
 			filteredSpansets = []*traceql.Spanset{spanset}
 		}

diff --git a/vendor/github.com/segmentio/parquet-go/buffer.go b/vendor/github.com/segmentio/parquet-go/buffer.go
diff --git a/vendor/github.com/segmentio/parquet-go/config.go b/vendor/github.com/segmentio/parquet-go/config.go
diff --git a/vendor/github.com/segmentio/parquet-go/file.go b/vendor/github.com/segmentio/parquet-go/file.go
diff --git a/vendor/github.com/segmentio/parquet-go/row.go b/vendor/github.com/segmentio/parquet-go/row.go
diff --git a/vendor/modules.txt b/vendor/modules.txt
@@ -898,7 +898,7 @@ github.com/segmentio/encoding/thrift
 # github.com/segmentio/fasthash v0.0.0-20180216231524-a72b379d632e
 ## explicit
 github.com/segmentio/fasthash/fnv1a
-# github.com/segmentio/parquet-go v0.0.0-20230201213121-e1109e2f6a20
+# github.com/segmentio/parquet-go v0.0.0-20230309140036-b6d0a6236da6
 ## explicit; go 1.19
 github.com/segmentio/parquet-go
 github.com/segmentio/parquet-go/bloom