Cache labels and series results #3315
Conversation
| case *ThanosLabelsRequest: | ||
| return fmt.Sprintf("%s:%d", tr.Label, currentInterval) | ||
| case *ThanosSeriesRequest: | ||
| return fmt.Sprintf("%s:%d", tr.Matchers, currentInterval) | ||
| } | ||
| return fmt.Sprintf("%s:%d:%d", r.GetQuery(), r.GetStep(), currentInterval) |
There was a problem hiding this comment.
I am not sure but this shouldn't happen. It would error out in the first place when decoding the HTTP request if the request is not valid.
CHANGELOG.md
Outdated
| @@ -13,6 +13,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re | |||
|
|
|||
| - [#3261](https://github.com/thanos-io/thanos/pull/3261) Thanos Store: Use segment files specified in meta.json file, if present. If not present, Store does the LIST operation as before. | |||
| - [#3276](https://github.com/thanos-io/thanos/pull/3276) Query Frontend: Support query splitting and retry for labels and series requests. | |||
| - [#3315](https://github.com/thanos-io/thanos/pull/3315) Query Frontend: Support results caching for labels and series requests. | |||
There was a problem hiding this comment.
label names ? label values? both?
There was a problem hiding this comment.
Yes both. I will make it more clear.
| i := 0 | ||
| for ; i < len(t.resolutions) && t.resolutions[i] > tr.MaxSourceResolution; i++ { | ||
| } | ||
| return fmt.Sprintf("%s:%d:%d:%d", tr.Query, tr.Step, currentInterval, i) | ||
| case *ThanosLabelsRequest: |
There was a problem hiding this comment.
ThanosLabelsRequest already covers both label_values query and label_names query. If it is a label_names query, then tr.Label is just an empty string. For label_values query, that's the queried label name.
I am using the same struct for them since their query response is the same []string
| // GetStep returns the step of the request in milliseconds. | ||
| func (r *ThanosLabelsRequest) GetStep() int64 { return 0 } | ||
| // GetStep returns the step of the request in milliseconds. Returns 1 is a trick to avoid panic in | ||
| // https://github.com/cortexproject/cortex/blob/master/pkg/querier/queryrange/results_cache.go#L447. |
There was a problem hiding this comment.
(: Yea it's getting very tricky without Cortex actually support our case, and it would be nice to help them as well 🤔
This is fine for now, but wonder how long term solution of truly shared code with cortex would looks like
There was a problem hiding this comment.
Yeah, I will take a look later. Would be good to have these query APIs natively supported on Cortex then we can re-use.
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
yeya24
force-pushed
the
cache-labelseries
branch
from
97eb6cd to
651b1bd
Compare
|
Conflict fixed. Please give another round of review if you have time. |
Signed-off-by: Ben Ye [email protected]
Changes
Fixes #3230
Verification