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

Prefix chunks cache flags. #2241

Merged
merged 7 commits into from
Mar 24, 2020

Conversation

gouthamve
Copy link
Contributor

@gouthamve gouthamve commented Mar 10, 2020

Fixes #2191 #553

Signed-off-by: Goutham Veeramachaneni gouthamve@gmail.com

Checklist

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

@gouthamve gouthamve force-pushed the chunk-cache-prefix branch from c1a5d49 to 9acf0f5 Compare March 10, 2020 13:46
@pull-request-size pull-request-size bot added size/L and removed size/S labels Mar 10, 2020
@tomwilkie tomwilkie self-requested a review March 11, 2020 09:40
@Andrewpk Andrewpk mentioned this pull request Mar 11, 2020
3 tasks
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

@gouthamve For each changed CLI flag could you search references in docs/ and update it, please? There are some. It's a annoying work, but we have to do it to avoid doc drifts.

@gouthamve
Copy link
Contributor Author

Thanks @pracucci, I thought I got them all, but somehow missed 2. Fixed those two as well!

@gouthamve gouthamve force-pushed the chunk-cache-prefix branch 2 times, most recently from dfca015 to 05ae255 Compare March 13, 2020 08:32
CHANGELOG.md Outdated Show resolved Hide resolved
pkg/chunk/cache/background.go Outdated Show resolved Hide resolved
k8s/memcached-dep.yaml Outdated Show resolved Hide resolved
@gouthamve gouthamve force-pushed the chunk-cache-prefix branch from e03b8ab to 0ea70e8 Compare March 13, 2020 14:50
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Amazing work @gouthamve ! LGTM.

I left few minor comments. Pay attention that there are still some old flags references:

  • -memcached. (2 times) and -redis. (1 time) in arguments.md

# memcache.
# CLI flag: -memcache.write-back-goroutines
# Cache config for chunks. At what concurrency to write back to cache.
# CLI flag: -store.chunks-cache.cache.write-back-concurrency
[writeback_goroutines: <int> | default = 10]
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think it's worth changing the YAML accordingly? Keep in mind that in other YAML config options the writeback is spelled writeback (without _ separator).

pkg/chunk/chunk_store.go Show resolved Hide resolved
pkg/chunk/cache/background.go Outdated Show resolved Hide resolved
k8s/memcached-dep.yaml Outdated Show resolved Hide resolved
docs/guides/caching.md Outdated Show resolved Hide resolved
@tomwilkie tomwilkie removed their request for review March 15, 2020 16:33
Fixes cortexproject#2191

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
Fixes cortexproject#553

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
@gouthamve gouthamve force-pushed the chunk-cache-prefix branch from 0ea70e8 to 50804fd Compare March 24, 2020 17:28
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
@gouthamve gouthamve force-pushed the chunk-cache-prefix branch from 50804fd to f486175 Compare March 24, 2020 17:40
@gouthamve gouthamve merged commit 9bf77e7 into cortexproject:master Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a prefix to chunk cache flags
3 participants