Skip to content

Conversation

56quarters
Copy link
Contributor

@56quarters 56quarters commented Jun 6, 2022

What this PR does

The default value, shared with all other memcache caches, of 200ms
is too aggressive in most cases. This results in TSDB data often being
fetched from object storage in cases where a slighly longer timeout
would result in a cache hit.

This is set in Jsonnet and Helm instead of as a default of the CLI
flag since the flags (and hence their defaults) are shared among all
caches (index, chunks, metadata, results).

Signed-off-by: Nick Pillitteri nick.pillitteri@grafana.com

Checklist

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

The default value, shared with all other memcache caches, of 200ms
is too aggressive in most cases. This results in TSDB data often being
fetched from object storage in cases where a slighly longer timeout
would result in a cache hit.

This is set in Jsonnet and Helm instead of as a default of the CLI
flag since the flags (and hence their defaults) are shared among all
caches (index, chunks, metadata, results).

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>
@56quarters 56quarters force-pushed the 56quarters/chunks-cache-timeout branch from 19e0a07 to e98891b Compare June 6, 2022 18:14
@56quarters 56quarters marked this pull request as ready for review June 6, 2022 18:26
Copy link
Contributor

@johannaratliff johannaratliff left a comment

Choose a reason for hiding this comment

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

Awesome. excited for more cache hits

Copy link
Contributor

@replay replay left a comment

Choose a reason for hiding this comment

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

That's great, thx!
At the same time, it does concern me a bit if Memcached takes >200ms to answer, but that's another story.

@56quarters 56quarters merged commit 631126f into main Jun 6, 2022
@56quarters 56quarters deleted the 56quarters/chunks-cache-timeout branch June 6, 2022 19:18
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.

3 participants