Cache: support redis cache backend #4888

hanjm · 2021-11-22T10:55:36Z

Hi, I am attempt to add redis cache support, like cortex does.
First. i will make it a draft PR.

Signed-off-by: Jimmie Han [email protected]

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

Changes

add redis support for bucket-cache/index-cache/query-frontend

Verification

unit test.
continning test in production deployment.

GiedriusS

I haven't checked the code yet but have you seen the implementation on Cortex side https://github.com/cortexproject/cortex/blob/34bb142b129bd781539001fb1629cf2f3ed54b82/pkg/chunk/cache/redis_cache.go? Maybe there is an opportunity to share some code here?

hanjm · 2021-11-24T13:59:49Z

I haven't checked the code yet but have you seen the implementation on Cortex side https://github.com/cortexproject/cortex/blob/34bb142b129bd781539001fb1629cf2f3ed54b82/pkg/chunk/cache/redis_cache.go? Maybe there is an opportunity to share some code here?

Thank you your review, I see the redis code in cortex not have a expire ttl in store function，it use a fixed ttl. so it can not reuse for our interface https://github.com/thanos-io/thanos/blob/main/pkg/cache/cache.go#L16. the redis support code is not very complex，i think write it again is seems not matter. (:

hanjm · 2021-11-26T12:23:22Z

I found the redis client invoke mget has too many keys. let me implement split it to batch.

hanjm · 2021-11-28T14:57:52Z

🚀 Ready to review. I am also deploymented it to production to continuing verify. it works well than memcached in my scene(no i/o timeout, no mem leak).

Signed-off-by: Jimmie Han <[email protected]>

make linter happy Signed-off-by: Jimmie Han <[email protected]>

Signed-off-by: Jimmie Han <[email protected]>

bwplotka

Great job! Just comment to fix, but not a blocker, we can fix later. LGTM! 💪🏽

pkg/queryfrontend/cache.go

bwplotka · 2021-12-07T16:17:12Z

Retried e2e test, let's see if that's a flakiness.

hanjm · 2021-12-08T00:10:18Z

Thank you🚀, I will fix it later.

colega · 2021-12-10T12:37:03Z

pkg/cacheutil/redis_client.go

+ }
+ duration := promauto.With(reg).NewHistogramVec(prometheus.HistogramOpts{
+ Name: "thanos_redis_operation_duration_seconds",
+ Help: "Duration of operations against memcached.",


Typo: should be Duration of operations against redis.

let me fix it.

pkg/store/cache/caching_bucket_factory.go

pkg/store/cache/memcached.go

* Cache: support redis cache backend Signed-off-by: Jimmie Han <[email protected]> * Cache: add get gate make linter happy Signed-off-by: Jimmie Han <[email protected]> * add concurrent config control Signed-off-by: Jimmie Han <[email protected]> * add redis cache doc Signed-off-by: Jimmie Han <[email protected]> * use doWithBatch to optmize concurrent batch code. Signed-off-by: Jimmie Han <[email protected]> * support db select Signed-off-by: Jimmie Han <[email protected]> * fix query-frontend cache config Signed-off-by: Jimmie Han <[email protected]>

* Cache: support redis cache backend Signed-off-by: Jimmie Han <[email protected]> * Cache: add get gate make linter happy Signed-off-by: Jimmie Han <[email protected]> * add concurrent config control Signed-off-by: Jimmie Han <[email protected]> * add redis cache doc Signed-off-by: Jimmie Han <[email protected]> * use doWithBatch to optmize concurrent batch code. Signed-off-by: Jimmie Han <[email protected]> * support db select Signed-off-by: Jimmie Han <[email protected]> * fix query-frontend cache config Signed-off-by: Jimmie Han <[email protected]> Signed-off-by: wenmaoba <[email protected]>

pgold30 · 2022-02-02T11:53:16Z

Love to see this, do you have a performance comparison with / without this to see if its better ? @hanjm specially the comparison you did vs "memcached in my scene(no i/o timeout, no mem leak)." , would be great if you have some numbers :)

fagossa · 2022-02-04T15:09:11Z

Hello,
any idea when this is going to be released?
thanks a lot for the great job 👍

hanjm force-pushed the feature/store-redis-cache branch from dace033 to 64fedeb Compare November 22, 2021 11:04

hanjm marked this pull request as draft November 22, 2021 11:06

GiedriusS reviewed Nov 23, 2021

View reviewed changes

hanjm force-pushed the feature/store-redis-cache branch 4 times, most recently from 0887806 to c6dccaa Compare November 26, 2021 02:20

hanjm force-pushed the feature/store-redis-cache branch 3 times, most recently from 1eddee6 to 801384f Compare November 28, 2021 14:36

hanjm marked this pull request as ready for review November 28, 2021 14:57

hanjm requested review from yeya24, GiedriusS and bwplotka November 28, 2021 14:58

hanjm added 6 commits December 3, 2021 11:25

Cache: support redis cache backend

6d5e3b7

Signed-off-by: Jimmie Han <[email protected]>

Cache: add get gate

aefb373

make linter happy Signed-off-by: Jimmie Han <[email protected]>

add concurrent config control

542f305

Signed-off-by: Jimmie Han <[email protected]>

add redis cache doc

4d8f638

Signed-off-by: Jimmie Han <[email protected]>

use doWithBatch to optmize concurrent batch code.

0d2a5c0

Signed-off-by: Jimmie Han <[email protected]>

support db select

6c43e70

Signed-off-by: Jimmie Han <[email protected]>

hanjm force-pushed the feature/store-redis-cache branch 2 times, most recently from 0a590f7 to f0a9278 Compare December 3, 2021 08:30

fix query-frontend cache config

3b8f154

Signed-off-by: Jimmie Han <[email protected]>

hanjm force-pushed the feature/store-redis-cache branch from f0a9278 to 3b8f154 Compare December 3, 2021 09:09

bwplotka approved these changes Dec 7, 2021

View reviewed changes

pkg/queryfrontend/cache.go Show resolved Hide resolved

bwplotka enabled auto-merge (squash) December 7, 2021 16:17

bwplotka merged commit 397fcb1 into thanos-io:main Dec 7, 2021

hanjm deleted the feature/store-redis-cache branch December 8, 2021 11:13

hanjm mentioned this pull request Dec 8, 2021

docs: add changelog about redis cache #4931

Merged

2 tasks

colega reviewed Dec 10, 2021

View reviewed changes

pkg/store/cache/caching_bucket_factory.go Show resolved Hide resolved

colega reviewed Dec 10, 2021

View reviewed changes

pkg/store/cache/memcached.go Show resolved Hide resolved

siggy mentioned this pull request Dec 13, 2021

Update Thanos to latest main cortexproject/cortex#4585

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cache: support redis cache backend #4888

Cache: support redis cache backend #4888

hanjm commented Nov 22, 2021 •

edited

Loading

GiedriusS left a comment

hanjm commented Nov 24, 2021 •

edited

Loading

hanjm commented Nov 26, 2021

hanjm commented Nov 28, 2021 •

edited

Loading

bwplotka left a comment

bwplotka commented Dec 7, 2021

hanjm commented Dec 8, 2021

colega Dec 10, 2021

hanjm Dec 10, 2021

pgold30 commented Feb 2, 2022 •

edited

Loading

fagossa commented Feb 4, 2022 •

edited

Loading

Cache: support redis cache backend #4888

Cache: support redis cache backend #4888

Conversation

hanjm commented Nov 22, 2021 • edited Loading

Changes

Verification

GiedriusS left a comment

Choose a reason for hiding this comment

hanjm commented Nov 24, 2021 • edited Loading

hanjm commented Nov 26, 2021

hanjm commented Nov 28, 2021 • edited Loading

bwplotka left a comment

Choose a reason for hiding this comment

bwplotka commented Dec 7, 2021

hanjm commented Dec 8, 2021

colega Dec 10, 2021

Choose a reason for hiding this comment

hanjm Dec 10, 2021

Choose a reason for hiding this comment

pgold30 commented Feb 2, 2022 • edited Loading

fagossa commented Feb 4, 2022 • edited Loading

hanjm commented Nov 22, 2021 •

edited

Loading

hanjm commented Nov 24, 2021 •

edited

Loading

hanjm commented Nov 28, 2021 •

edited

Loading

pgold30 commented Feb 2, 2022 •

edited

Loading

fagossa commented Feb 4, 2022 •

edited

Loading