Query frontend now discards cached results that are outside of tenant's configured TTL #4439
Conversation
| // If we don't know the query time, it's too old. | ||
| extentsOutOfTTL++ | ||
| continue |
There was a problem hiding this comment.
Have you considered doing the opposite? If we don't know the QueryTime, then we consider this extent within the TTL. I would avoid loosing all cached query results when rolling out this change. Over time (next 7d?) all cached entries will converge.
There was a problem hiding this comment.
I haven't considered it. It would mean that "per-tenant TTL" feature only starts working correctly 7 days after updating your Mimir installation. On the other hand, people can always flush cache manually, if needed, so I'm fine with that approach too.
There was a problem hiding this comment.
Actually keeping extents with QueryTime=0 can prolong their life -- when we merge them with additional extents, we will keep QueryTime=0 and put it back to cache. I guess we could set timestamp of extent in such case. 🤔
There was a problem hiding this comment.
I've implemented this, but the previous logic was easier to understand, especially when merging multiple extents.
When extents are used, we use stored query time to verify if extent can be reused or not. Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
pstibrany
force-pushed
the
add-timestamp-to-extents
branch
from
37dc86f
to
a76269a
Compare
|
Addressed all feedback in e1e8855
|
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
What this PR does
This PR adds query timestamp into cached results. This allows query-frontend to discard any cached results that are no longer valid due to tenant's configured cache results TTL.
This fixes a problem described in #4383 (comment): changing TTL for cached results used by specific tenant didn't invalidate previously cached results that are now outside of TTL.
All entries cached by previous version of Mimir will be invalidated by this change.
Which issue(s) this PR fixes or relates to
Fixes #4385
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]