Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Iterator Perfomance Improvements #3667

Merged
merged 21 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 9 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
22 changes: 8 additions & 14 deletions pkg/parquetquery/iters.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 6 additions & 6 deletions pkg/parquetquery/iters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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())
}

Expand All @@ -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,
Expand All @@ -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())
}
}

Expand All @@ -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,
Expand All @@ -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())
}
}

Expand Down
Loading
Loading