[vParquet2] The next version of Tempo's parquet based block format #2244

stoewer · 2023-03-21T12:35:41Z

What this PR does:

Creates a new tempodb encoding named vParquet2. The new encoding implements parquet schema changes that are not compatible with vParquet:

Renamed columns to make the schema more consistent and closer to names and terminologies used by OTEL
Add a dedicated span column for duration
Additional columns that will enable new TraceQL features
Represent repeated groups as nested list types to improve the compatibility with other parquet based tools

Review hints

The PR contains a lot of code coppied from vParquet which makes it difficult to spot the differences between vParquet and vParquet2. I created a second PR [vParquet2] all commits for vParquet2 as changes of the existing vParquet #2243 that implements all changes for vParquet2 in the vParquet tree
Commits in this PR are atomic and can be reviewed separately. This may also help with the review.
Consider rebase merge instead of squash merge in order to preserve this individual vParquet2 schema changes

Which issue(s) this PR fixes:
Contributes to grafana/tempo-squad#179 and #2226

Checklist

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

joe-elliott

This is surprisingly complete for a first pass. Nice work. I have some comments on cleaning up the encodings, but other than that this looks good.

As discussed we will get the parent,nestedsetleft/right columns in this PR, but follow up with actually populating them later.

tempodb/compactor_test.go

tempodb/encoding/versioned.go

tempodb/tempodb_test.go

tempodb/wal/wal_test.go

tempodb/encoding/vparquet2/block_traceql.go

tempodb/encoding/vparquet2/schema.go

mdisibio · 2023-03-28T11:04:31Z

Do you have a benchmark on hand that shows the benefit from the new span duration column? It would be neat to see the improvement for a query like { duration > 1s }

stoewer · 2023-03-28T11:14:32Z

Do you have a benchmark on hand that shows the benefit from the new span duration column?

No, I didn't benchmark it. Should I add one?

tempodb/encoding/vparquet2/block_traceql.go

mdisibio · 2023-03-30T13:33:27Z

Do you have a benchmark on hand that shows the benefit from the new span duration column?

No, I didn't benchmark it. Should I add one?

No that's ok, I think this is tough to benchmark on synthetic data but if you had some numbers offhand I was curious. I didn't see a benefit locally between vparquet and vparquet2 which is how I came across the predicate recommendation.

Make column names using timestamps more consistent and more similar the names used by OTEL

Rename columns to match the latest OTEL standard

Increases performance of queries for span duration, as it is no longer required to search two columns to get the span duration

The columns are a prerequisite to improve structural TraceQL queries The new columns are not populated yet

This makes the schema more similar to the OTEL standard and makes it possible to add a numeric span ID later (see ParentID vs ParentSpanID)

This improves the compatibility with other tooling using the parquet format such as parquet-mr/parquet-cli

This prevents potential bugs with nil predicates where '== nil' is false because the value pointer is nil but the type pointer is not

Since there is now a dedicated duration column, queries for duration can be treated more like any other integer column

mdisibio

LGTM. Last changes for duration look good, seeing a benchmark improvement for those queries as expected.

stoewer changed the title ~~Next version of Tempo's parquet based block format~~ [vParquet2] The next version of Tempo's parquet based block format Mar 21, 2023

stoewer force-pushed the vparquet2-main branch 2 times, most recently from 45853ad to a4ed8f1 Compare March 22, 2023 03:29

joe-elliott reviewed Mar 22, 2023

View reviewed changes

tempodb/encoding/vparquet2/schema.go Show resolved Hide resolved

stoewer force-pushed the vparquet2-main branch from 9c9d24d to 14fb52d Compare March 28, 2023 06:05

stoewer marked this pull request as ready for review March 28, 2023 08:14

stoewer requested review from knylander-grafana, annanay25, mdisibio, mapno, kvrhdn and zalegrala as code owners March 28, 2023 08:14

mdisibio reviewed Mar 28, 2023

View reviewed changes

tempodb/encoding/vparquet2/block_traceql.go Show resolved Hide resolved

tempodb/encoding/vparquet2/block_traceql.go Outdated Show resolved Hide resolved

tempodb/encoding/vparquet2/block_traceql.go Outdated Show resolved Hide resolved

joe-elliott closed this Mar 29, 2023

joe-elliott reopened this Mar 29, 2023

stoewer force-pushed the vparquet2-main branch from 14fb52d to 284beca Compare March 30, 2023 11:07

joe-elliott mentioned this pull request Mar 30, 2023

Add scope parameter to /api/search/tags #2282

Merged

3 tasks

stoewer added 9 commits April 3, 2023 15:04

[vParquet2] Create new block encoding by copying vparquet

6d85444

[vParquet2] Rename columns containing timestamps and use suffix *Nano

401c6b3

Make column names using timestamps more consistent and more similar the names used by OTEL

[vParquet2] Rename InstrumentationLibrary/il to Scope

0c6823f

Rename columns to match the latest OTEL standard

Fix capitalization in Span interface function EndTimeUnixNanos

423da8e

[vParquet2] Remove span column EndTimeUnixNano and add DurationNano

bcdaf27

Increases performance of queries for span duration, as it is no longer required to search two columns to get the span duration

[vParquet2] Add columns for parent relation and nested set model

76f4965

The columns are a prerequisite to improve structural TraceQL queries The new columns are not populated yet

[vParquet2] Rename Span.ID to Span.SpanID

d32853c

This makes the schema more similar to the OTEL standard and makes it possible to add a numeric span ID later (see ParentID vs ParentSpanID)

[vParquet2] Annotate repeated groups as nested list types

fd0f55c

This improves the compatibility with other tooling using the parquet format such as parquet-mr/parquet-cli

[vParquet2] Convert block in vparquet2/test-data to new schema

09d539f

stoewer added 4 commits April 3, 2023 15:04

[vParquet2] tempo-cli sub command to convert from vParquet to vParquet2

8091f60

[vParquet2] changelog entry

db4442e

Add vParquet2 to AllEncodings() and use it consistently in tests

b825ffa

[vParquet2] mention the new block format in the configuration docs

d8d9f5a

stoewer force-pushed the vparquet2-main branch from 2b5aea4 to a91fac2 Compare April 3, 2023 05:08

stoewer added 2 commits April 3, 2023 15:57

Change return type of createIntPredicate to predicate interface

11ca1fe

This prevents potential bugs with nil predicates where '== nil' is false because the value pointer is nil but the type pointer is not

[vParquet2] Optimize duration queries and remove extra duration logic

f8cd5ac

Since there is now a dedicated duration column, queries for duration can be treated more like any other integer column

stoewer force-pushed the vparquet2-main branch from a91fac2 to f8cd5ac Compare April 3, 2023 05:58

[vParquet2] Fix duration bug in vparquet 1 to 2 converter

c33dc3d

mdisibio approved these changes Apr 4, 2023

View reviewed changes

mdisibio merged commit b990eeb into grafana:main Apr 4, 2023

stoewer deleted the vparquet2-main branch April 6, 2023 04:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[vParquet2] The next version of Tempo's parquet based block format #2244

[vParquet2] The next version of Tempo's parquet based block format #2244

stoewer commented Mar 21, 2023 •

edited

Loading

joe-elliott left a comment

mdisibio commented Mar 28, 2023

stoewer commented Mar 28, 2023

mdisibio commented Mar 30, 2023

mdisibio left a comment

[vParquet2] The next version of Tempo's parquet based block format #2244

[vParquet2] The next version of Tempo's parquet based block format #2244

Conversation

stoewer commented Mar 21, 2023 • edited Loading

joe-elliott left a comment

Choose a reason for hiding this comment

mdisibio commented Mar 28, 2023

stoewer commented Mar 28, 2023

mdisibio commented Mar 30, 2023

mdisibio left a comment

Choose a reason for hiding this comment

stoewer commented Mar 21, 2023 •

edited

Loading