Feat/blocks grpc #1845

thorfour · 2019-11-20T22:02:16Z

What this PR does: Implements gRPC streaming from TSDB based ingesters.

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

Checklist

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

pracucci

Thanks @thorfour for the PR! I left a couple of high-level comments to better understand the rationale behind the design decisions. Without looking into it too much, I was expecting to reuse QueryStream() and I would like to better understand if we really need to duplicate part of the logic introducing QueryTSDBStream and a different response type.

pkg/querier/querier.go

pkg/ingester/client/cortex.proto

pstibrany

This PR introduces an important piece to the TSDB puzzle. I'm not happy with all design decisions, but can understand why they were made. Overall, it looks good, but I've left some comments to consider before approving.

pkg/querier/querier.go

pkg/distributor/query.go

pkg/ingester/client/cortex.proto

pstibrany

LGTM, thanks for your changes!

Please don't forget to update CHANGELOG as well.

pkg/ingester/client/cortex.proto

pkg/ingester/ingester_v2.go

pracucci

Thanks @thorfour and @pstibrany for reworking the initial design. I think the current design is better than the initial one, but unfortunately doesn't look working (at least, querying ingesters always result into an empty result to me... see related comment in ingester_streaming_queryable.go).

pkg/ingester/ingester_v2.go

pkg/querier/tsdb_ingester_streaming_queryable.go

pkg/querier/ingester_streaming_queryable.go

pkg/querier/timeseries_series_set.go

Signed-off-by: Thor <thansen@digitalocean.com>

Signed-off-by: Marco Pracucci <marco@pracucci.com>

pracucci · 2020-03-05T14:08:48Z

I've rebased master

pracucci

Thanks @thorfour for your work! I've pushed few changes to fix a couple of issues. I'm a bit concerned about the samples sorting performances, but let's move on and hopefully we'll get rid of this once we'll able to iterate chunks on TSDB 🤞

thorfour · 2020-03-05T15:57:11Z

Sounds good to me

pstibrany

Still looks good.

pkg/querier/timeseries_series_set.go

Co-Authored-By: Marco Pracucci <marco@pracucci.com> Signed-off-by: Thor <8681572+thorfour@users.noreply.github.com>

Signed-off-by: Thor <thansen@digitalocean.com>

Signed-off-by: Marco Pracucci <marco@pracucci.com>

Signed-off-by: Thor <thansen@digitalocean.com>

thorfour force-pushed the feat/blocks-grpc branch from 5286293 to 9aa20ec Compare November 20, 2019 22:04

pracucci reviewed Nov 21, 2019

View reviewed changes

pkg/querier/querier.go Outdated Show resolved Hide resolved

pkg/ingester/client/cortex.proto Outdated Show resolved Hide resolved

thorfour force-pushed the feat/blocks-grpc branch 3 times, most recently from 009759a to 6e6cd17 Compare December 11, 2019 21:38

pstibrany reviewed Jan 23, 2020

View reviewed changes

pkg/querier/querier.go Outdated Show resolved Hide resolved

pkg/querier/querier.go Outdated Show resolved Hide resolved

pkg/distributor/query.go Outdated Show resolved Hide resolved

pkg/ingester/client/cortex.proto Outdated Show resolved Hide resolved

thorfour force-pushed the feat/blocks-grpc branch 11 times, most recently from 234bc15 to e0f2e0a Compare January 23, 2020 18:47

pstibrany approved these changes Jan 24, 2020

View reviewed changes

pkg/ingester/client/cortex.proto Outdated Show resolved Hide resolved

thorfour force-pushed the feat/blocks-grpc branch 2 times, most recently from 9ba6a2e to feb5963 Compare January 24, 2020 14:39

pracucci reviewed Jan 24, 2020

View reviewed changes

pkg/ingester/client/cortex.proto Show resolved Hide resolved

pracucci reviewed Jan 24, 2020

View reviewed changes

pkg/ingester/ingester_v2.go Outdated Show resolved Hide resolved

thorfour force-pushed the feat/blocks-grpc branch from feb5963 to 38cade2 Compare January 24, 2020 17:57

pracucci reviewed Jan 27, 2020

View reviewed changes

thorfour force-pushed the feat/blocks-grpc branch 3 times, most recently from 2d91ab4 to 93d95a2 Compare January 27, 2020 17:45

pstibrany reviewed Jan 29, 2020

View reviewed changes

pkg/querier/timeseries_series_set.go Outdated Show resolved Hide resolved

pstibrany mentioned this pull request Jan 31, 2020

Refactor Cortex querier to use Queryable interface directly #2060

Merged

3 tasks

thorfour force-pushed the feat/blocks-grpc branch from 474e535 to b3e846b Compare February 29, 2020 01:44

Thor and others added 9 commits March 5, 2020 15:07

ingester_v2: grpc streaming

bce79f1

Signed-off-by: Thor <thansen@digitalocean.com>

return timeseries if populated

698a1e4

Signed-off-by: Thor <thansen@digitalocean.com>

fixed unit test

f0873c5

Signed-off-by: Thor <thansen@digitalocean.com>

unexported time series set

57c27f0

Signed-off-by: Thor <thansen@digitalocean.com>

Don't panic if t.i < 0

ccc1b59

Signed-off-by: Thor <thansen@digitalocean.com>

fixed proto file after rebase

9b3eae2

Signed-off-by: Thor <thansen@digitalocean.com>

fixed unit test

44a23be

Signed-off-by: Thor <thansen@digitalocean.com>

goimports ordering

e2dab0e

Signed-off-by: Thor <thansen@digitalocean.com>

Added integration test

4e5a41b

Signed-off-by: Marco Pracucci <marco@pracucci.com>

pracucci force-pushed the feat/blocks-grpc branch from b3e846b to dd32a60 Compare March 5, 2020 14:07

pracucci approved these changes Mar 5, 2020

View reviewed changes

pstibrany approved these changes Mar 5, 2020

View reviewed changes

pkg/querier/timeseries_series_set.go Outdated Show resolved Hide resolved

pkg/querier/timeseries_series_set.go Outdated Show resolved Hide resolved

thorfour force-pushed the feat/blocks-grpc branch from 2ab048f to e667242 Compare March 6, 2020 03:04

thorfour and others added 10 commits March 5, 2020 21:26

Update CHANGELOG.md

1837cce

Co-Authored-By: Marco Pracucci <marco@pracucci.com> Signed-off-by: Thor <8681572+thorfour@users.noreply.github.com>

return error during iteration

771cd92

Signed-off-by: Thor <thansen@digitalocean.com>

updated TimeSeriesSeriesIterator seek functions

73ecbfc

Signed-off-by: Thor <thansen@digitalocean.com>

sort time series before returning

743ca78

Signed-off-by: Thor <thansen@digitalocean.com>

merge integration test

de3b4ac

Signed-off-by: Thor <thansen@digitalocean.com>

Fixed linter

1599c79

Signed-off-by: Marco Pracucci <marco@pracucci.com>

Fixed series set error handling in v2QueryStream()

c59b653

Signed-off-by: Marco Pracucci <marco@pracucci.com>

Fixed and simplified QueryStream() unit test

f0bf23b

Signed-off-by: Marco Pracucci <marco@pracucci.com>

Deleted benchmark (was not even running with the blocks storage)

58dc385

Signed-off-by: Marco Pracucci <marco@pracucci.com>

timerseries series set: unexported types

de1ef2a

Signed-off-by: Thor <thansen@digitalocean.com>

thorfour force-pushed the feat/blocks-grpc branch from e667242 to de1ef2a Compare March 6, 2020 03:26

pracucci merged commit 0de497a into cortexproject:master Mar 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/blocks grpc #1845

Feat/blocks grpc #1845

thorfour commented Nov 20, 2019 •

edited

Loading

pracucci left a comment

pstibrany left a comment

pstibrany left a comment

pracucci left a comment

pracucci commented Mar 5, 2020

pracucci left a comment

thorfour commented Mar 5, 2020

pstibrany left a comment

Feat/blocks grpc #1845

Feat/blocks grpc #1845

Conversation

thorfour commented Nov 20, 2019 • edited Loading

pracucci left a comment

Choose a reason for hiding this comment

pstibrany left a comment

Choose a reason for hiding this comment

pstibrany left a comment

Choose a reason for hiding this comment

pracucci left a comment

Choose a reason for hiding this comment

pracucci commented Mar 5, 2020

pracucci left a comment

Choose a reason for hiding this comment

thorfour commented Mar 5, 2020

pstibrany left a comment

Choose a reason for hiding this comment

thorfour commented Nov 20, 2019 •

edited

Loading