Stream chunks from blocks-ingester to querier #3889

pstibrany · 2021-02-27T21:18:02Z

What this PR does: This PR implements streaming of TSDB chunks from blocks-based ingester to querier. It reuses chunks-based querier, and just introduces new type of chunk: read-only wrapper around TSDB chunk.

~~State of PR: manual testing shows that it works, unit tests not updated yet.~~

PR is ready for review. Feature is currently protected by the flag (both CLI and runtime-config to avoid rollout of ingesters when playing with this), but will be enabled by default in the future.

Which issue(s) this PR fixes:
Fixes #3831

Checklist

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

pstibrany · 2021-03-01T16:44:50Z

Benchmark comparing old (samples based) and new (chunk based) streaming in blocks-ingester.

name                      old time/op    new time/op    delta
Ingester_v2QueryStream-4    6.21ms ± 1%    1.13ms ± 2%  -81.88%  (p=0.000 n=10+10)

name                      old alloc/op   new alloc/op   delta
Ingester_v2QueryStream-4    3.55MB ± 0%    0.36MB ± 0%  -89.92%  (p=0.000 n=10+10)

name                      old allocs/op  new allocs/op  delta
Ingester_v2QueryStream-4     5.66k ± 0%     3.85k ± 0%  -31.88%  (p=0.000 n=10+10)

pstibrany

I think this is ready for review now.

pstibrany · 2021-03-01T20:31:47Z

pkg/ingester/ingester_v2.go

+	}
+	defer q.Close()
+
+	// It's not required to return sorted series because series are sorted by the Cortex querier.


This is also true for chunks.

pstibrany · 2021-03-01T20:32:31Z

pkg/ingester/ingester_v2.go

+
+			// It is not guaranteed that chunk returned by iterator is populated.
+			// For now just return error. We could also try to figure out how to read the chunk.
+			if meta.Chunk == nil {


It's not clear to me if this can actually happen. Needs some investigation.

Asked here: prometheus/prometheus#8559

jtlisi · 2021-03-02T03:48:17Z

Fixes #2845

pracucci

Wow, very good job! And good tests 👏 I left a couple of nits, but nothing particularly relevant.

pkg/chunk/encoding/factory.go

pkg/chunk/encoding/prometheus_chunk.go

pkg/ingester/ingester.go

Signed-off-by: Peter Štibraný <[email protected]>

… the tests. Signed-off-by: Peter Štibraný <[email protected]>

Added tests and benchmarks. Signed-off-by: Peter Štibraný <[email protected]>

Signed-off-by: Peter Štibraný <[email protected]>

pracucci · 2021-03-04T08:19:39Z

Thanks @pstibrany for addressing my feedback! LGTM. I don't feel strong about the double CLI flags considering they're temporarily I'm fine with that.

pstibrany · 2021-03-16T10:15:32Z

Using this feature makes average time for rule group evaluation to go up:
avg (cortex_prometheus_rule_group_last_duration_seconds)

Signed-off-by: Peter Štibraný <[email protected]>

pull-request-size bot added the size/L label Feb 27, 2021

pstibrany marked this pull request as draft February 27, 2021 21:18

pstibrany force-pushed the blocks-chunks branch 2 times, most recently from c3b7e3c to 66b47f0 Compare February 27, 2021 21:30

pull-request-size bot added size/XL and removed size/L labels Mar 1, 2021

pstibrany marked this pull request as ready for review March 1, 2021 20:34

pstibrany requested a review from pracucci March 1, 2021 20:34

pstibrany commented Mar 1, 2021

View reviewed changes

pstibrany changed the title ~~[WIP] Stream chunks from blocks-ingester to querier~~ Stream chunks from blocks-ingester to querier Mar 1, 2021

pracucci approved these changes Mar 3, 2021

View reviewed changes

pstibrany and others added 12 commits March 4, 2021 09:00

Initial support for streaming chunks from v2 ingester.

d09d4e3

Signed-off-by: Peter Štibraný <[email protected]>

Fix bug in PrometheusXorChunk.FindAtOrAfter method, and include it in…

3fc9cf7

… the tests. Signed-off-by: Peter Štibraný <[email protected]>

Support streaming with both timeseries and chunks for now.

0aa1eac

Added tests and benchmarks. Signed-off-by: Peter Štibraný <[email protected]>

Revert changes to test.

b6b5682

Signed-off-by: Peter Štibraný <[email protected]>

Revert changes to test.

b77bbb9

Signed-off-by: Peter Štibraný <[email protected]>

Fix comment.

1b8e367

Signed-off-by: Peter Štibraný <[email protected]>

Extend integration test with chunk streaming.

5302470

Signed-off-by: Peter Štibraný <[email protected]>

Add runtime-config for switching between types of query-stream.

2a72c96

Signed-off-by: Peter Štibraný <[email protected]>

CHANGELOG.md, v1-guarantees.

4ca0d0e

Signed-off-by: Peter Štibraný <[email protected]>

Docs.

abf32bd

Signed-off-by: Peter Štibraný <[email protected]>

Clean white space.

4e3e8c6

Signed-off-by: Peter Štibraný <[email protected]>

Review feedback.

1243d2f

Signed-off-by: Peter Štibraný <[email protected]>

pstibrany force-pushed the blocks-chunks branch from 8399fc4 to 1243d2f Compare March 4, 2021 08:01

Merge branch 'master' into blocks-chunks

267b9e6

pstibrany merged commit 5dccf08 into cortexproject:master Mar 4, 2021

pstibrany mentioned this pull request Mar 16, 2021

Average time to evaluate rule groups is higher when using streaming of chunks #3957

Open

jtlisi mentioned this pull request Mar 18, 2021

Stream chunks from ingesters to querier when running the blocks storage #2845

Closed

harry671003 mentioned this pull request Mar 22, 2021

SIGSEGV in ingesters after enabling ingester_stream_chunks_when_using_blocks #3994

Closed

2 tasks

roystchiang pushed a commit to roystchiang/cortex that referenced this pull request Apr 6, 2022

Stream chunks from blocks-ingester to querier (cortexproject#3889)

c333577

Signed-off-by: Peter Štibraný <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stream chunks from blocks-ingester to querier #3889

Stream chunks from blocks-ingester to querier #3889

pstibrany commented Feb 27, 2021 •

edited

Loading

pstibrany commented Mar 1, 2021 •

edited

Loading

pstibrany left a comment

pstibrany Mar 1, 2021

pstibrany Mar 1, 2021

pstibrany Mar 3, 2021 •

edited

Loading

jtlisi commented Mar 2, 2021 •

edited

Loading

pracucci left a comment

pracucci commented Mar 4, 2021

pstibrany commented Mar 16, 2021

Stream chunks from blocks-ingester to querier #3889

Stream chunks from blocks-ingester to querier #3889

Conversation

pstibrany commented Feb 27, 2021 • edited Loading

pstibrany commented Mar 1, 2021 • edited Loading

pstibrany left a comment

Choose a reason for hiding this comment

pstibrany Mar 1, 2021

Choose a reason for hiding this comment

pstibrany Mar 1, 2021

Choose a reason for hiding this comment

pstibrany Mar 3, 2021 • edited Loading

Choose a reason for hiding this comment

jtlisi commented Mar 2, 2021 • edited Loading

pracucci left a comment

Choose a reason for hiding this comment

pracucci commented Mar 4, 2021

pstibrany commented Mar 16, 2021

pstibrany commented Feb 27, 2021 •

edited

Loading

pstibrany commented Mar 1, 2021 •

edited

Loading

pstibrany Mar 3, 2021 •

edited

Loading

jtlisi commented Mar 2, 2021 •

edited

Loading