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

Add support for Cortex Background Cache #640

Merged
merged 11 commits into from
Apr 15, 2021

Conversation

dgzlopes
Copy link
Member

@dgzlopes dgzlopes commented Apr 12, 2021

What this PR does:

  • Add support for Cortex Background Cache.
  • Refactor our code to use the Cortex Cache interface.

Which issue(s) this PR fixes:
Fixes #528

Checklist

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

@dgzlopes dgzlopes marked this pull request as ready for review April 12, 2021 14:01
Copy link
Member

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

Very nice work. I'm impressed with how light of a touch this is. I really expected to have some nasty breaking changes with our Redis/Memcached configs.

docs/tempo/website/configuration/_index.md Outdated Show resolved Hide resolved
tempodb/backend/cache/redis/redis.go Outdated Show resolved Hide resolved
Signed-off-by: Daniel González Lopes <[email protected]>
Signed-off-by: Daniel González Lopes <[email protected]>
Signed-off-by: Daniel González Lopes <[email protected]>
Signed-off-by: Daniel González Lopes <[email protected]>
Signed-off-by: Daniel González Lopes <[email protected]>
Signed-off-by: Daniel González Lopes <[email protected]>
Signed-off-by: Daniel González Lopes <[email protected]>
Signed-off-by: Daniel González Lopes <[email protected]>
Copy link
Contributor

@annanay25 annanay25 left a comment

Choose a reason for hiding this comment

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

Love how lean and clean our cache package is becoming! Great work Daniel. Left a few minor comments.

modules/storage/config.go Outdated Show resolved Hide resolved
Comment on lines -17 to -21
type Client interface {
Fetch(ctx context.Context, key string) []byte
Store(ctx context.Context, key string, val []byte)
Shutdown()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Amazing work. Have been wanting to reuse the Cortex Cache interfaces for a long time!

docs/tempo/website/configuration/_index.md Outdated Show resolved Hide resolved
Signed-off-by: Daniel González Lopes <[email protected]>
Signed-off-by: Daniel González Lopes <[email protected]>
Signed-off-by: Daniel González Lopes <[email protected]>
@annanay25 annanay25 merged commit 86f2509 into grafana:master Apr 15, 2021
@dgzlopes dgzlopes deleted the add-magic-cache branch April 15, 2021 12:40
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.

Add Cortex Background Cache
3 participants