query-frontend: Fix cache keys for dynamic split intervals #7832
Conversation
|
e2e run locally after a few retries |
lachruzam
force-pushed
the
qf-fix-cache-for-dynamic-intervals
branch
from
9355e26
to
f808233
Compare
pkg/queryfrontend/cache.go
Outdated
| } | ||
| return fmt.Sprintf("fe:%s:%s:%d:%d", userID, r.GetQuery(), r.GetStep(), currentInterval) | ||
|
|
||
| return "request_type_not_supported" |
There was a problem hiding this comment.
This is a condition that we should never hit in prod, right? Perhaps we should just panic() here?
pkg/queryfrontend/cache.go
Outdated
| switch tr := r.(type) { | ||
| case *ThanosQueryRangeRequest: | ||
| i := 0 | ||
| for ; i < len(t.resolutions) && t.resolutions[i] > tr.MaxSourceResolution; i++ { | ||
| } | ||
| shardInfoKey := generateShardInfoKey(tr) | ||
| return fmt.Sprintf("fe:%s:%s:%d:%d:%d:%s:%d:%s", userID, tr.Query, tr.Step, currentInterval, i, shardInfoKey, tr.LookbackDelta, tr.Engine) | ||
| splitInterval := tr.SplitInterval.Milliseconds() |
There was a problem hiding this comment.
I wonder if it's possible to write a function for these splitInterval/currentInterval calculations instead of copy/paste
lachruzam
force-pushed
the
qf-fix-cache-for-dynamic-intervals
branch
2 times, most recently
from
86930d4
to
6554d24
Compare
|
@pedro-stanaka maybe you'd like to voice your opinion here |
|
|
|
I think you need to rebase on top of latest main to get the github actions changes |
Head branch was pushed to by a user without write access
Signed-off-by: Michał Mazur <mmazur.box@gmail.com>
Signed-off-by: Michał Mazur <mmazur.box@gmail.com>
Signed-off-by: Michał Mazur <mmazur.box@gmail.com>
lachruzam
force-pushed
the
qf-fix-cache-for-dynamic-intervals
branch
from
f30035d
to
d4725c3
Compare
|
@yeya24 Rebased without any new change. |
Changes
The same interval is used for query splitting and cache key generation (the split interval is stored in requests created during the split).
Split interval has been added to cache keys to avoid overlaps for queries based on different intervals.
Support for caching generic PrometheusRequest has been removed - already PrometheusRequest was not been created by any configured decoder.
Verification
Run local tests
make test, run query-frontend on a test environment.