Skip to content

Add redis and inmemory cache backend for blocks storage #3573

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

kamijin-fanta
Copy link
Contributor

@kamijin-fanta kamijin-fanta commented Dec 8, 2020

What this PR does:

  • add Redis cache backend
  • add configure options
    • -blocks-storage.bucket-store.index-cache.backend=redis
    • -blocks-storage.bucket-store.index-cache.redis.*
    • -blocks-storage.bucket-store.metadata-cache.backend=redis
    • -blocks-storage.bucket-store.metadata-cache.redis.*
    • -blocks-storage.bucket-store.chunks-cache.backend.backend=redis
    • -blocks-storage.bucket-store.chunks-cache.backend.redis.*
    • -blocks-storage.bucket-store.metadata-cache.inmemory.*
    • -blocks-storage.bucket-store.chunks-cache.inmemory.*

Which issue(s) this PR fixes:
fix #3568

Checklist

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

@kamijin-fanta
Copy link
Contributor Author

kamijin-fanta commented Dec 8, 2020

I have some questions.

  • There are several variations of RedisConfig. Please let me know if there is a reference implementation of RegisterFlagsWithPrefix.
    • The TTL is passed from thanos to the metadata and chunks cache. -blocks-storage.bucket-store.[metadata-cache|chunks-cache].redis.expiration is no longer respected. I want to remove this.
    • The index cache wants to set the default expiration to 24h. However, it is currently set to 0.
  • How should I set the Prometheus metric name of RedisIndexCache?
    • The current code uses the same thanos_store_index_cache_requests_total as Thanos. I think there is a possibility of using the cortex namespace.
  • Are there any tests I should add?

@pstibrany
Copy link
Contributor

I believe there is an ongoing work in Thanos to move IndexCache to use cache.Cache interface. I would wait until it lands, to avoid copying Thanos' MemcachedIndexCache here.

(I cannot find an issue for it though. I think it is being done by @Sudhar287 as part of thanos-io/thanos#3408, but I may be confusing the two, and it may in fact be unrelated.)

@kamijin-fanta kamijin-fanta force-pushed the add-redis-cache branch 3 times, most recently from d6f4239 to 9b601fb Compare February 25, 2021 04:22
@kamijin-fanta kamijin-fanta changed the title Add redis cache backend for blocks storage Add redis and inmemory cache backend for blocks storage Feb 25, 2021
@kamijin-fanta
Copy link
Contributor Author

Since the related PR is closed, work will resume. I also added an InMemory implementation to the cache.

@stale
Copy link

stale bot commented Jul 7, 2021

This issue has been automatically marked as stale because it has not had any activity in the past 60 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 7, 2021
@alanprot
Copy link
Member

I think this is an interesting option. Are we going to go forward with this?

@stale stale bot closed this Jul 30, 2021
@phin1x
Copy link

phin1x commented Aug 6, 2021

@kamijin-fanta @pstibrany @pracucci can we re-open this pr? redis is also very popular and should be an alternative besides memcached.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for Redis Cache storage in TSDB
5 participants