-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
feat: ability to ignore cache for volume queries #13945
Conversation
@@ -413,6 +413,9 @@ func (Codec) DecodeRequest(_ context.Context, r *http.Request, _ []string) (quer | |||
Step: 0, | |||
TargetLabels: req.TargetLabels, | |||
AggregateBy: req.AggregateBy, | |||
CachingOptions: queryrangebase.CachingOptions{ |
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.
should we document that these are now not cached by default? (and caching of them is not configurable by the user if we're hard coding this here?)
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.
oh, hmm, I missed this.... how are they not cached by default? My intent here was to keep the behavior but allow them to not be cached by setting the Cache-Control
header.
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.
Nevermind, I misunderstood based on the field name. I assumed Disabled
would only be present if the results caching was disabled. Missing the historical context of why the field is named Disabled
and not something like ShouldCache
.
(cherry picked from commit b1dc076)
What this PR does / why we need it:
Add caching option to volume requests so we can issue queries that ignore the results cache (important for some performance testing and tuning)