Split incoming requests by day, run them in parallel, and cache query results. by tomwilkie · Pull Request #1029 · cortexproject/cortex

tomwilkie · 2018-09-23T18:09:13Z

~~Includes #995~~, fixes #977, fixes #963, fixes #266

Continuation of the work proposed in https://docs.google.com/document/d/1lsvSkv0tiAMPQv-V8vI2LZ8f4i9JuTRsuPI_i-XcAqY/edit?usp=drive_web&ouid=103586900408483314805.

Generic code to parse incoming query_range requests, mutate them and round trip them.
Align incoming queries with their step to make the results more cachable.
Split queries along day boundaries, modulo step.
Run queries in parallel and combine their results.
Cache query results by (userid, promql query, step, day).
Work out intersection of query and cached result and only query for difference.

tomwilkie · 2018-09-24T10:22:03Z

This is rebased on the new caching interfaces and should be ready for review now.

csmarchbanks

Would you mind creating some docs for the new flags/behavior?

csmarchbanks · 2018-09-24T15:56:17Z

Gopkg.toml

Looks like the referenced pr has been merged, so this can all be removed.

csmarchbanks · 2018-09-24T15:58:07Z

pkg/querier/frontend/frontend.go

Typo Aling -> Align

csmarchbanks · 2018-09-24T15:58:46Z

pkg/querier/frontend/frontend.go

Should this be a separate flag from caching? Seems like caching results wouldn't work very well without this.

The main criticism I've heard against trickster is that it mutates the request. Also, we've put the step alignment into Grafana now. So I wanted to make this optional as I think it shouldn't be necessary in some cases. Its a niche case, for sure.

csmarchbanks · 2018-09-24T16:01:48Z

pkg/querier/frontend/query_range_test.go

These responses don't look empty

csmarchbanks · 2018-09-24T16:09:53Z

pkg/querier/frontend/results_cache_test.go

Parititon -> Partition

Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>

Split incoming requests by day and run them in parallel. - Generic code to parse incoming query_range requests, mutate them and round trip them. - Split queries along day boundaries, modulo step. - Run queries in parallel and combine their results. - Ensure we propagate org ids correctly; add e2e tests. - Take care to ensure we propagate trace IDs correctly; involved updating weaveworks/common. Query results caching. - Align incoming queries with their step to make the results more cachable. - Work out intersection of query and cached result and only query for difference. - Cache query results using the key `hash(userid, promql query, step, day)`. - Cache multiple, non-overlapping time ranges per day. Don't cache the last 1 minute. - Hash the key for the query results cache. - Don't cache the last minute, as results can still be in flux. - Make sure the trimmed results should also align with step. Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>

dep couldn't find a solution so I had to encourage it to use the later grpc and proto versions - I think this is because the constraint added into common on gogo/status. Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>

tomwilkie · 2018-09-27T18:06:05Z

Would you mind creating some docs for the new flags/behavior?

Done!

…m the upstream repo. Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>

Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>

csmarchbanks

Is the Check endpoint on an ingester still going to work? Seems like client.RegisterIngesterServer will no longer register the Check method, so another register will need to be added to cmd/ingester/main.go?

csmarchbanks · 2018-09-28T16:37:08Z

Gopkg.toml

Are these new constraints necessary? Or were they just needed for when you were pointing to a branch?

I needed them to get dep to successfully find a solution, but now it has, I don't seem to need them. Removed.

Looks like weaveworks/common is still new and unneeded

pkg/chunk/cache/mock.go

tomwilkie · 2018-09-28T16:59:48Z

Is the Check endpoint on an ingester still going to work? Seems like client.RegisterIngesterServer will no longer register the Check method, so another register will need to be added to cmd/ingester/main.go?

Good catch! I had fixed but forgot to push...

Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>

tomwilkie · 2018-09-28T20:03:56Z

BTW we've deployed this in production and it makes a huge difference to query performance.

csmarchbanks · 2018-09-28T20:41:35Z

One last thing to remove in Gopkg.toml, then this LGTM

Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>

tomwilkie · 2018-09-29T10:56:48Z

Thanks!

tomwilkie force-pushed the query-results-cache branch 3 times, most recently from 8b29dca to 266cb9c Compare September 24, 2018 10:20

tomwilkie changed the title ~~[WIP] Query results caching in the frontend~~ Query results caching in the frontend Sep 24, 2018

csmarchbanks reviewed Sep 24, 2018

View reviewed changes

tomwilkie mentioned this pull request Sep 26, 2018

Split incoming requests by day and run them in parallel. #995

Closed

tomwilkie changed the title ~~Query results caching in the frontend~~ Split incoming requests by day and run them in parallel; also cache query results. Sep 26, 2018

tomwilkie force-pushed the query-results-cache branch 5 times, most recently from bb805ef to 7315796 Compare September 27, 2018 16:24

tomwilkie added 2 commits September 27, 2018 17:27

Add snappy cache middleware, fixes cortexproject#1015

f35295f

Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>

tomwilkie force-pushed the query-results-cache branch from 7315796 to 649666f Compare September 27, 2018 16:27

Update vendor for new weaveworks/common.

7b2283f

dep couldn't find a solution so I had to encourage it to use the later grpc and proto versions - I think this is because the constraint added into common on gogo/status. Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>

tomwilkie force-pushed the query-results-cache branch from 649666f to a22db82 Compare September 27, 2018 16:28

tomwilkie changed the title ~~Split incoming requests by day and run them in parallel; also cache query results.~~ Split incoming requests by day, run them in parallel, and cache query results. Sep 27, 2018

tomwilkie added 2 commits September 28, 2018 11:18

Rejig how we implement HealthClient as the proto has been removed fro…

7372142

…m the upstream repo. Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>

Add some documentation about the new query frontend features.

17a544b

Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>

csmarchbanks reviewed Sep 28, 2018

View reviewed changes

tomwilkie force-pushed the query-results-cache branch from c91dac1 to 17a544b Compare September 28, 2018 16:59

Remove extra constraints

e3f722d

Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>

Remove constaint on weaveworks/common.

ac55af8

Signed-off-by: Tom Wilkie <tom.wilkie@gmail.com>

tomwilkie merged commit ee74728 into cortexproject:master Sep 29, 2018

tomwilkie deleted the query-results-cache branch September 29, 2018 10:56

tomwilkie mentioned this pull request Oct 1, 2018

Write only as many chunk bytes as needed #1044

Merged

Conversation

tomwilkie commented Sep 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tomwilkie commented Sep 24, 2018

Uh oh!

csmarchbanks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomwilkie commented Sep 27, 2018

Uh oh!

csmarchbanks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tomwilkie commented Sep 28, 2018

Uh oh!

tomwilkie commented Sep 28, 2018

Uh oh!

csmarchbanks commented Sep 28, 2018

Uh oh!

tomwilkie commented Sep 29, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tomwilkie commented Sep 23, 2018 •

edited

Loading