Skip to content

[TraceQL] Reduce work performed for structural operators due to a bug#3088

Merged
joe-elliott merged 3 commits intografana:mainfrom
joe-elliott:structural-improvements
Oct 31, 2023
Merged

[TraceQL] Reduce work performed for structural operators due to a bug#3088
joe-elliott merged 3 commits intografana:mainfrom
joe-elliott:structural-improvements

Conversation

@joe-elliott
Copy link
Copy Markdown
Collaborator

@joe-elliott joe-elliott commented Oct 30, 2023

What this PR does:
Prevents additional columns from being added as iterators when doing structural queries. For the query { resource.service.name = "loki-querier" } >> { resource.service.name = "loki-querier" }, the following iterators were generated.

Before:

rebatchIterator: 
	JoinIterator: 0: traceCollector()	
		LeftJoinIterator: 1: batchCollector(true, 0)
		required: 
			LeftJoinIterator: 3: spanCollector(0)
			required: 
				SyncIterator: rs.list.element.ss.list.element.Spans.list.element.Kind : InstrumentedPredicate{2, nil}
			optional: 
				LeftJoinIterator: 4: attributeCollector{}
				required: 
					SyncIterator: rs.list.element.ss.list.element.Spans.list.element.Attrs.list.element.Key : InstrumentedPredicate{0, StringInPredicate{intrinsic(11), }}
				optional: 
					SyncIterator: rs.list.element.ss.list.element.Spans.list.element.Attrs.list.element.Value : InstrumentedPredicate{8, nil}
					SyncIterator: rs.list.element.ss.list.element.Spans.list.element.Attrs.list.element.ValueInt : InstrumentedPredicate{8, nil}
					SyncIterator: rs.list.element.ss.list.element.Spans.list.element.Attrs.list.element.ValueDouble : InstrumentedPredicate{8, nil}
					SyncIterator: rs.list.element.ss.list.element.Spans.list.element.Attrs.list.element.ValueBool : InstrumentedPredicate{8, nil}
				SyncIterator: rs.list.element.ss.list.element.Spans.list.element.NestedSetLeft : InstrumentedPredicate{2, nil}
				SyncIterator: rs.list.element.ss.list.element.Spans.list.element.NestedSetRight : InstrumentedPredicate{2, nil}
		optional: 
			SyncIterator: rs.list.element.Resource.ServiceName : InstrumentedPredicate{0, StringInPredicate{loki-querier, }}
			SyncIterator: rs.list.element.Resource.ServiceName : InstrumentedPredicate{0, StringInPredicate{loki-querier, }}
		SyncIterator: StartTimeUnixNano : InstrumentedPredicate{0, IntBetweenPredicate{0,1001000000000}}
		SyncIterator: EndTimeUnixNano : InstrumentedPredicate{0, IntBetweenPredicate{1000000000000,9223372036854775807}})

After:

rebatchIterator: 
	JoinIterator: 0: traceCollector()	
		LeftJoinIterator: 1: batchCollector(true, 0)
		required: 
			LeftJoinIterator: 3: spanCollector(0)
			required: 
				SyncIterator: rs.list.element.ss.list.element.Spans.list.element.Kind : InstrumentedPredicate{2, nil}
			optional: 
				SyncIterator: rs.list.element.ss.list.element.Spans.list.element.NestedSetLeft : InstrumentedPredicate{2, nil}
				SyncIterator: rs.list.element.ss.list.element.Spans.list.element.NestedSetRight : InstrumentedPredicate{2, nil}
		optional: 
			SyncIterator: rs.list.element.Resource.ServiceName : InstrumentedPredicate{0, StringInPredicate{loki-querier, }}
			SyncIterator: rs.list.element.Resource.ServiceName : InstrumentedPredicate{0, StringInPredicate{loki-querier, }}
		SyncIterator: StartTimeUnixNano : InstrumentedPredicate{0, IntBetweenPredicate{0,1001000000000}}
		SyncIterator: EndTimeUnixNano : InstrumentedPredicate{0, IntBetweenPredicate{1000000000000,9223372036854775807}})

Also finds a minor performance improvement for situations where a span has a nil intrinsic. This happens regularly for queries like { status = error } where most entries in the StatusCode column are nil.

Checklist

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

Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
@joe-elliott
Copy link
Copy Markdown
Collaborator Author

Initial benchmarks. I'm suspicious off the +1/2% and will re-run.

name                                          old time/op    new time/op    delta
BackendBlockTraceQL/spanAttNameNoMatch-8        6.20ms ± 3%    6.21ms ± 1%     ~     (p=0.310 n=5+5)
BackendBlockTraceQL/spanAttIntrinsicMatch-8     42.9ms ± 1%    43.7ms ± 1%   +1.78%  (p=0.008 n=5+5)
BackendBlockTraceQL/statusError-8                122ms ± 1%     125ms ± 2%   +2.77%  (p=0.008 n=5+5)
BackendBlockTraceQL/resourceAttNameNoMatch-8    6.12ms ± 1%    6.23ms ± 2%   +1.90%  (p=0.032 n=5+5)
BackendBlockTraceQL/mixedNameNoMatch-8           407ms ± 2%     487ms ±20%     ~     (p=0.056 n=5+5)
BackendBlockTraceQL/desc-8                       2.09s ± 6%     1.63s ± 6%  -22.12%  (p=0.008 n=5+5)

name                                          old speed      new speed      delta
BackendBlockTraceQL/spanAttNameNoMatch-8       273MB/s ± 3%   273MB/s ± 1%     ~     (p=0.310 n=5+5)
BackendBlockTraceQL/spanAttIntrinsicMatch-8    405MB/s ± 1%   398MB/s ± 1%   -1.75%  (p=0.008 n=5+5)
BackendBlockTraceQL/statusError-8              144MB/s ± 1%   140MB/s ± 2%   -2.69%  (p=0.008 n=5+5)
BackendBlockTraceQL/resourceAttNameNoMatch-8   190MB/s ± 1%   186MB/s ± 2%   -1.85%  (p=0.032 n=5+5)
BackendBlockTraceQL/mixedNameNoMatch-8        12.1MB/s ± 2%  10.3MB/s ±18%     ~     (p=0.056 n=5+5)
BackendBlockTraceQL/desc-8                    3.22MB/s ± 6%  1.35MB/s ± 6%  -57.92%  (p=0.008 n=5+5)

name                                          old MB_io/op   new MB_io/op   delta
BackendBlockTraceQL/spanAttNameNoMatch-8          1.69 ± 0%      1.69 ± 0%     ~     (all equal)
BackendBlockTraceQL/spanAttIntrinsicMatch-8       17.4 ± 0%      17.4 ± 0%     ~     (all equal)
BackendBlockTraceQL/statusError-8                 17.5 ± 0%      17.5 ± 0%     ~     (all equal)
BackendBlockTraceQL/resourceAttNameNoMatch-8      1.16 ± 0%      1.16 ± 0%     ~     (all equal)
BackendBlockTraceQL/mixedNameNoMatch-8            4.91 ± 0%      4.91 ± 0%     ~     (all equal)
BackendBlockTraceQL/desc-8                        6.72 ± 0%      2.20 ± 0%  -67.26%  (p=0.008 n=5+5)

name                                          old alloc/op   new alloc/op   delta
BackendBlockTraceQL/spanAttNameNoMatch-8        2.95MB ± 2%    2.93MB ± 3%     ~     (p=0.310 n=5+5)
BackendBlockTraceQL/spanAttIntrinsicMatch-8     3.22MB ± 5%    3.31MB ± 6%     ~     (p=0.421 n=5+5)
BackendBlockTraceQL/statusError-8               14.1MB ± 2%    13.9MB ± 5%     ~     (p=0.222 n=5+5)
BackendBlockTraceQL/resourceAttNameNoMatch-8    2.95MB ± 3%    2.95MB ± 2%     ~     (p=0.690 n=5+5)
BackendBlockTraceQL/mixedNameNoMatch-8          7.48MB ±17%    6.78MB ±11%     ~     (p=0.310 n=5+5)
BackendBlockTraceQL/desc-8                      57.8MB ±84%   45.6MB ±107%     ~     (p=0.310 n=5+5)

name                                          old allocs/op  new allocs/op  delta
BackendBlockTraceQL/spanAttNameNoMatch-8         44.0k ± 0%     44.0k ± 0%     ~     (p=0.968 n=4+5)
BackendBlockTraceQL/spanAttIntrinsicMatch-8      45.4k ± 0%     45.4k ± 0%     ~     (p=0.587 n=5+5)
BackendBlockTraceQL/statusError-8                 142k ± 0%      142k ± 0%     ~     (p=0.381 n=5+5)
BackendBlockTraceQL/resourceAttNameNoMatch-8     44.1k ± 0%     44.1k ± 0%     ~     (p=1.000 n=5+5)
BackendBlockTraceQL/mixedNameNoMatch-8           44.7k ± 0%     44.7k ± 0%     ~     (p=0.452 n=5+5)
BackendBlockTraceQL/desc-8                       1.49M ± 6%     0.67M ±14%  -54.98%  (p=0.008 n=5+5)

Signed-off-by: Joe Elliott <number101010@gmail.com>
@joe-elliott joe-elliott merged commit b1a28f7 into grafana:main Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants