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
results cache: add new stat query_length_served
to measure cache effectiveness
#11589
Conversation
query_length_served
to measure cache effectiveness
Trivy scan found the following vulnerabilities:
|
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.
Trying to understand: how would this will be used to measure cache effectiveness? Would we use this field and perform some arithmetic against the query length to determine the overall "hit rate"?
On its own it'll be difficult to use as a measure of cache effectiveness, I feel.
yes @dannykopping, this is what I was thinking. Just logging the actual number same as what we do with other stats like timings. |
Gotcha, ok. Can we even do arithmetic on durations in this way? |
not pretty, but that should do it I think
|
Seems plausible, can you confirm before we merge? |
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
…fectiveness (grafana#11589) **What this PR does / why we need it**: cache hit rate that is currently being measured using metrics and stats does not account for the fact that a cache hit could return partial results. When we query the cache for a key, we get back a list of extents and these need no cover the entire (split) range of the cache key. This pr adds a new cache stat called `query_length_served` to better measure the cache effectiveness. ``` query_length - sum(length of downstream queries) ``` **Which issue(s) this PR fixes**: Fixes #<issue number> **Special notes for your reviewer**: **Checklist** - [ ] Reviewed the [`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md) guide (**required**) - [x] Documentation added - [x] Tests updated - [x] `CHANGELOG.md` updated - [ ] If the change is worth mentioning in the release notes, add `add-to-release-notes` label - [ ] Changes that require user attention or interaction to upgrade are documented in `docs/sources/setup/upgrade/_index.md` - [ ] For Helm chart changes bump the Helm chart version in `production/helm/loki/Chart.yaml` and update `production/helm/loki/CHANGELOG.md` and `production/helm/loki/README.md`. [Example PR](grafana@d10549e) - [ ] If the change is deprecating or removing a configuration option, update the `deprecated-config.yaml` and `deleted-config.yaml` files respectively in the `tools/deprecated-config-checker` directory. [Example PR](grafana@0d4416a)
What this PR does / why we need it:
cache hit rate that is currently being measured using metrics and stats does not account for the fact that a cache hit could return partial results. When we query the cache for a key, we get back a list of extents and these need no cover the entire (split) range of the cache key.
This pr adds a new cache stat called
query_length_served
to better measure the cache effectiveness.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Checklist
CONTRIBUTING.md
guide (required)CHANGELOG.md
updatedadd-to-release-notes
labeldocs/sources/setup/upgrade/_index.md
production/helm/loki/Chart.yaml
and updateproduction/helm/loki/CHANGELOG.md
andproduction/helm/loki/README.md
. Example PRdeprecated-config.yaml
anddeleted-config.yaml
files respectively in thetools/deprecated-config-checker
directory. Example PR