-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Define a reason why the fifocache has evicted an item #6335
Define a reason why the fifocache has evicted an item #6335
Conversation
Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
ff9f72a
to
34d6d99
Compare
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Minor suggestions on reason
constants.
Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a minor non-blocking suggestion
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
Signed-off-by: Danny Kopping danny.kopping@grafana.com
What this PR does / why we need it:
This PR adds an eviction reason for the fifocache, which will help an operator know if an entry was evicted because the TTL expired vs the cache is full.
Which issue(s) this PR fixes:
N/A
Special notes for your reviewer:
We use a fifocache when running boltdb-shipper for storing ingesters' index caches locally. We do this because we cannot share the cache between ingesters, to prevent missing out chunks that may not exist in other ingesters.
https://github.com/grafana/loki/blob/main/pkg/loki/modules.go#L414-L425
The cache is configured with a fixed size of 200MB, and is currently not configurable; we could consider making this configurable however even in our biggest clusters we're only seeing ~11MB of use currently.
Checklist
CHANGELOG.md
.docs/sources/upgrading/_index.md