Support snappy compression in query frontend cache by yeya24 · Pull Request #3207 · thanos-io/thanos

yeya24 · 2020-09-23T15:35:55Z

Signed-off-by: Ben Ye yb532204897@gmail.com

I added CHANGELOG entry for this change.
Change is not relevant to the end user.

Changes

This pr adds snappy compression support to the query frontend and bumps Cortex dependency to the latest master.

Verification

Signed-off-by: Ben Ye <yb532204897@gmail.com>

bwplotka

👍

LGTM just one question.

bwplotka · 2020-09-23T16:50:42Z

cmd/thanos/query_frontend.go


 	cfg.CachePathOrContent = *extflag.RegisterPathOrContent(cmd, "query-range.response-cache-config", "YAML file that contains response cache configuration.", false)

+	cmd.Flag("query-range.compression", "Use compression in results cache. Supported values are: 'snappy' and '' (disable compression).").


Can we elaborate when it is used? On response? or when put in Memcached ?

Also, maybe query-range.compression-type?

Yes, it is used before storing the response to Memcached. It doesn't make sense to use it when only fifo-cache is enabled. I should document that.

I am okay with either query-range.compression-type or query-range.compression. The flag is named to compression in Cortex

Yes, it is used before storing the response to Memcached. It doesn't make sense to use it when only fifo-cache is enabled. I should document that.

But it will work for other non FIFO backends as well, right? 🤔

Yes, it compresses the response and then sends it to the results cache backend. If it is fifo-cache, because snappy cache and fifo cache are in memory, so not needed.

Technically it could be needed -> It might reduce memory consumed as well (:
But yea, the question is what is available right now (:

Yeah you are right. Sorry, my mistake. It is a tradeoff between CPU and Memory & network traffic

No need to be sorry - I think it's extremely useful. The only question is, do we need this for query range or some global cache option? (for future labels and instant)?

I think we need it for all so maybe some generic:

Suggested change

cmd.Flag("query-range.compression", "Use compression in results cache. Supported values are: 'snappy' and '' (disable compression).").

cmd.Flag("cache-compression-type", "Use compression in results cache. Supported values are: 'snappy' and '' (disable compression).").

I think the option can be global. But considering other caching configs, I am not sure. This is another problem and maybe we can have a separate issue to talk about it. The main question is, can we use different cache backends and different cache configs for each endpoint we want to cache? @bwplotka @kakkoyun

For example, it is not very good to reuse the same cache config because the response sizes are different for range queries and instant queries. They might need different limits and different max_items configs.

Totally agree. To me, something will be the same, some things different (:

onprem

Nice! LGTM.

Signed-off-by: Ben Ye <yb532204897@gmail.com>

yeya24 · 2020-09-24T02:07:02Z

Umm, this flaky test is new. Is it related to the new TSDB update? @bwplotka Can you please take a look? https://app.circleci.com/jobs/github/thanos-io/thanos/11833

Creating 1 50000-sample series in /tmp/testorbench-multitsdbseries256659744/0
Creating 1 50000-sample series in /tmp/testorbench-multitsdbseries256659744/1
Creating 1 50000-sample series in /tmp/testorbench-multitsdbseries256659744/2
Creating 1 50000-sample series in /tmp/testorbench-multitsdbseries256659744/3
Creating 50000 1-sample series in /tmp/testorbench-proxyseries345881914/0
Creating 50000 1-sample series in /tmp/testorbench-proxyseries345881914/1
Creating 50000 1-sample series in /tmp/testorbench-proxyseries345881914/2
Creating 50000 1-sample series in /tmp/testorbench-proxyseries345881914/3
Creating 5000 2-sample series in /tmp/testorbench-proxyseries834212689/0
Creating 5000 2-sample series in /tmp/testorbench-proxyseries834212689/1
Creating 5000 2-sample series in /tmp/testorbench-proxyseries834212689/2
Creating 5000 2-sample series in /tmp/testorbench-proxyseries834212689/3
Creating 1 50000-sample series in /tmp/testorbench-proxyseries876832380/0
Creating 1 50000-sample series in /tmp/testorbench-proxyseries876832380/1
Creating 1 50000-sample series in /tmp/testorbench-proxyseries876832380/2
Creating 1 50000-sample series in /tmp/testorbench-proxyseries876832380/3
--- FAIL: TestTSDBStore_Series_SplitSamplesIntoChunksWithMaxSizeOf120 (0.23s)
    prometheus_test.go:608: prometheus_test.go:608:
        
        	exp: 1093
        
        	got: 1092
        
        
Creating 2 300-sample series in /tmp/test011123234
Creating 2 300-sample series in /tmp/test011123234
Creating 2 300-sample series in /tmp/test354000153
Creating 2 300-sample series in /tmp/test354000153

kakkoyun

tiny nit, otherwise lgtm.

kakkoyun · 2020-09-24T07:29:27Z

CHANGELOG.md

 - [#3154](https://github.com/thanos-io/thanos/pull/3154) Query Frontend: Add metric `thanos_memcached_getmulti_gate_queries_max`.
 - [#3146](https://github.com/thanos-io/thanos/pull/3146) Sidecar: Add `thanos_sidecar_prometheus_store_received_frames` histogram metric.
 - [#3147](https://github.com/thanos-io/thanos/pull/3147) Querier: Add `query.metadata.default-time-range` flag to specify the default metadata time range duration for retrieving labels through Labels and Series API when the range parameters are not specified. The zero value means range covers the time since the beginning.
+- [#3207](https://github.com/thanos-io/thanos/pull/3207) Query Frontend: Add `query-range.compression` flag to use compression in the query frontend cache.


Suggested change

- [#3207](https://github.com/thanos-io/thanos/pull/3207) Query Frontend: Add `query-range.compression` flag to use compression in the query frontend cache.

- [#3207](https://github.com/thanos-io/thanos/pull/3207) Query Frontend: Add `cache-compression-type` flag to use compression in the query frontend cache.

Thanks for the catch! Please take a look again.

Signed-off-by: Ben Ye <yb532204897@gmail.com>

bwplotka

Lgtm!

bwplotka · 2020-09-25T08:39:48Z

Thanks! ♥️

support snappy compression in query frontend cache

e67dd85

Signed-off-by: Ben Ye <yb532204897@gmail.com>

yeya24 requested review from bwplotka, kakkoyun and onprem September 23, 2020 15:36

add changelog

a13c445

Signed-off-by: Ben Ye <yb532204897@gmail.com>

bwplotka approved these changes Sep 23, 2020

View reviewed changes

onprem approved these changes Sep 23, 2020

View reviewed changes

rename flag

800e2df

Signed-off-by: Ben Ye <yb532204897@gmail.com>

yeya24 force-pushed the add-snappy-query-frontend branch from 57ac41d to 800e2df Compare September 24, 2020 01:42

kakkoyun reviewed Sep 24, 2020

View reviewed changes

fix changelog entry

4a6c803

Signed-off-by: Ben Ye <yb532204897@gmail.com>

bwplotka approved these changes Sep 25, 2020

View reviewed changes

bwplotka merged commit 5a776d7 into thanos-io:master Sep 25, 2020

yeya24 deleted the add-snappy-query-frontend branch September 25, 2020 12:20

yeya24 mentioned this pull request Sep 27, 2020

query-frontend: Split and cache metadata API responses by time interval #3230

Closed

4 tasks


		cfg.CachePathOrContent = *extflag.RegisterPathOrContent(cmd, "query-range.response-cache-config", "YAML file that contains response cache configuration.", false)

		cmd.Flag("query-range.compression", "Use compression in results cache. Supported values are: 'snappy' and '' (disable compression).").

	cmd.Flag("query-range.compression", "Use compression in results cache. Supported values are: 'snappy' and '' (disable compression).").
	cmd.Flag("cache-compression-type", "Use compression in results cache. Supported values are: 'snappy' and '' (disable compression).").

	- [#3207](https://github.com/thanos-io/thanos/pull/3207) Query Frontend: Add `query-range.compression` flag to use compression in the query frontend cache.
	- [#3207](https://github.com/thanos-io/thanos/pull/3207) Query Frontend: Add `cache-compression-type` flag to use compression in the query frontend cache.

Conversation

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

Uh oh!

Changes

Verification

Uh oh!

bwplotka 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!

Choose a reason for hiding this comment

Uh oh!

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

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!

onprem left a comment

Choose a reason for hiding this comment

Uh oh!

yeya24 commented Sep 24, 2020

Uh oh!

kakkoyun 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!

bwplotka left a comment

Choose a reason for hiding this comment

Uh oh!

bwplotka commented Sep 25, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yeya24 commented Sep 23, 2020 •

edited

Loading

bwplotka Sep 23, 2020 •

edited

Loading