Batch Iterator optimization

[alanprot]

What this PR does:

This PR is till a work in progress but seems we can speed up some query ranges quite a bit:

We noticed a very high cpu utilization by the seek method when running query ranges that does not have any overlapping steps:

Ex: q=sum(rate(up[1m]))&step=120 (1 min matrix selector with a 2 min step)

This type of queries and steps are quite common since grafana default to 120s when displaying an 1 day graph.

In our test environment this change reduced for the query sum(rate(up[1m]))&step=120&start=now-600m&end=now over 30K series from 30s to 5s

One interesting point is that if we set the step to 60s the seek is not performed and the query returns in 9s instead of 43s

We can see on the benchmarks bellow that there is a sweet spot and this change can be slower than the current implementation when the steps are skipping lots of samples inside the same chunk (20% slower). Said that, this change seems to still make sense as: 1. It improves 90%+ the most commons use cases; 2. The penalty only happens in cases that is already very fast (~15ms) 3. it makes the latency way less sensitive to the steps (before a 30s step was taking 600ms and 300s step was taking 12ms and now all the cases are under 20ms.

The Crux of this change is that the seek cause the iterator to transverse all the samples from the beginning of the chunk to the time t. In other words, with a step=60s, we we transverse the samples O(n) times but with the step=120s we transverse them O(n*s) where s is the number of steps.

See this:

cortex/pkg/querier/batch/chunk.go

Line 45 in f694529

if i.it.FindAtOrAfter(model.Time(t)) {

cortex/pkg/chunk/encoding/prometheus_chunk.go

Lines 90 to 95 in f694529

	func (p *prometheusChunkIterator) FindAtOrAfter(time model.Time) bool {
	// FindAtOrAfter must return OLDEST value at given time. That means we need to start with a fresh iterator,
	// otherwise we cannot guarantee OLDEST.
	p.it = p.c.Iterator(p.it)
	return p.it.Seek(int64(time)) != chunkenc.ValNone
	}

https://github.com/prometheus/prometheus/blob/211ae4f1f0a2cdaae09c4c52735f75345c1817c6/tsdb/chunkenc/xor.go#L247

We could create a new iterator only if t is before the current at this place but the batching makes this not possible.

NewChunkMergeIterator_Seek/scrapeInterval_30s_seekStep:_15s-16       618ms ± 0%      19ms ± 0%  -96.88%  (p=0.008 n=5+5)
NewChunkMergeIterator_Seek/scrapeInterval_30s_seekStep:_30s-16       617ms ± 0%      18ms ± 0%  -97.01%  (p=0.008 n=5+5)
NewChunkMergeIterator_Seek/scrapeInterval_30s_seekStep:_60s-16       306ms ± 0%      18ms ± 0%  -94.10%  (p=0.008 n=5+5)
NewChunkMergeIterator_Seek/scrapeInterval_30s_seekStep:_300s-16     58.6ms ± 0%    20.0ms ± 0%  -65.91%  (p=0.008 n=5+5)
NewChunkMergeIterator_Seek/scrapeInterval_30s_seekStep:_900s-16     17.2ms ± 0%    18.5ms ± 0%   +7.29%  (p=0.029 n=4+4)
NewChunkMergeIterator_Seek/scrapeInterval_30s_seekStep:_1500s-16    12.3ms ± 0%    15.8ms ± 0%  +28.27%  (p=0.008 n=5+5)
NewChunkMergeIterator_Seek/scrapeInterval_30s_seekStep:_3000s-16    6.00ms ± 0%    7.79ms ± 0%  +29.81%  (p=0.008 n=5+5)
NewChunkMergeIterator_Seek/scrapeInterval_30s_seekStep:_6000s-16    2.87ms ± 0%    2.88ms ± 0%   +0.29%  (p=0.032 n=5+5)
NewChunkMergeIterator_Seek/scrapeInterval_10s_seekStep:_5s-16        615ms ± 0%      19ms ± 0%  -96.88%  (p=0.008 n=5+5)
NewChunkMergeIterator_Seek/scrapeInterval_10s_seekStep:_10s-16       614ms ± 0%      18ms ± 0%  -97.01%  (p=0.008 n=5+5)
NewChunkMergeIterator_Seek/scrapeInterval_10s_seekStep:_20s-16       305ms ± 0%      18ms ± 0%  -94.09%  (p=0.008 n=5+5)
NewChunkMergeIterator_Seek/scrapeInterval_10s_seekStep:_100s-16     58.4ms ± 0%    19.9ms ± 0%  -65.93%  (p=0.008 n=5+5)
NewChunkMergeIterator_Seek/scrapeInterval_10s_seekStep:_300s-16     17.1ms ± 0%    18.5ms ± 0%   +7.98%  (p=0.008 n=5+5)
NewChunkMergeIterator_Seek/scrapeInterval_10s_seekStep:_500s-16     12.2ms ± 0%    15.8ms ± 0%  +28.78%  (p=0.008 n=5+5)
NewChunkMergeIterator_Seek/scrapeInterval_10s_seekStep:_1000s-16    5.99ms ± 0%    7.79ms ± 1%  +30.09%  (p=0.016 n=4+5)
NewChunkMergeIterator_Seek/scrapeInterval_10s_seekStep:_2000s-16    2.87ms ± 0%    2.88ms ± 0%   +0.36%  (p=0.016 n=5+5)

** Memory allocations benchmarks were omitted as there is no change at all on them **

Which issue(s) this PR fixes:
Fixes #

Checklist

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

[yeya24]

Great work overall. This is huge! Thanks

[yeya24]

Thanks!