Skip to content
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

Store: make index cache shared cache friendly #1825

Merged

Conversation

pracucci
Copy link
Contributor

@pracucci pracucci commented Dec 3, 2019

The IndexCache is an in-memory cache whose exposed interface allow to cache single Postings and Series. In Cortex we've an experimental TSDB blocks storage integration based on Thanos and we would like to introduce a shared index cache (based on Memcached).

The current IndexCache interface is not shared cache friendly, due to the fact that the exposed functions allow to fetch/store single items. In this PR I'm proposing to switch IndexCache to an interface which allows to fetch multiple postings and series at once, in order to make it shared cache friendly.

Fixed #1815

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

Changes

  • Renamed store cache.go to inmemory.go
  • Refactored IndexCache to multiple fetch postings and series within a single call
  • Prefixed fetch/store functions with Fetch and Store for clarity

Due to the file renaming, the diff is quite difficult to follow, reason why I've splitted into two separate commits (it should help).

Verification

Unit and manual tests.

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Super elegant code, just one minor nit for comment on exported method useful for goDoc (: Otherwise happy to merge.

pkg/store/cache/inmemory.go Show resolved Hide resolved
@pracucci pracucci force-pushed the make-index-cache-shared-cache-friendly branch from 86c081f to 6d2d018 Compare December 4, 2019 10:25
@pracucci
Copy link
Contributor Author

pracucci commented Dec 4, 2019

Thanks @bwplotka for your review! What's about now?

@bwplotka bwplotka merged commit c134ad1 into thanos-io:master Dec 4, 2019
@bwplotka
Copy link
Member

bwplotka commented Dec 4, 2019

Thanks!

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

Successfully merging this pull request may close these issues.

Make the IndexCache shared cache friendly
2 participants