Introduces cache to TSDB postings #9621
Conversation
DylanGuedes
force-pushed
the
check-index-postings-calls
branch
from
9cf0bea to
2753a89
Compare
DylanGuedes
force-pushed
the
check-index-postings-calls
branch
from
02d4206 to
6878e4b
Compare
DylanGuedes
force-pushed
the
check-index-postings-calls
branch
from
00ddd35 to
e9d2650
Compare
| }) | ||
|
|
||
| // restart ingester which should flush the chunks and index | ||
| require.NoError(t, tIngester.Restart()) |
There was a problem hiding this comment.
we could also hit /flush here to be even more deliberate
There was a problem hiding this comment.
this part was just copied from the other tests
| tIndexGateway = clu.AddComponent( | ||
| "index-gateway", | ||
| "-target=index-gateway", | ||
| "-tsdb.enable-postings-cache=true", |
There was a problem hiding this comment.
this test passes with this flag set to false. is there any way to this test to fail when caching is not happening? where are the objects coming from (local filesystem?), could we change the data at rest to prove the data we're getting is from cache?
There was a problem hiding this comment.
I added an assertion that checks the cache added/misses/gets metrics results. they guarantee we're covering the cache behavior using the FIFO cache.
| "github.com/grafana/loki/pkg/util/encoding" | ||
| ) | ||
|
|
||
| type PostingsReader interface { |
| for i := range ms { | ||
| sorted[i] = labels.Matcher{Type: ms[i].Type, Name: ms[i].Name, Value: ms[i].Value} | ||
| } | ||
| sort.Sort(sortedLabelMatchers(sorted)) |
There was a problem hiding this comment.
nit: it's a little weird we're sorting something called sorted...
There was a problem hiding this comment.
sorteableLabelMatchers maybe?
This reverts commit 1221658.
This reverts commit 1221658. **What this PR does / why we need it**: We decided to discontinue the postings cache a few releases ago for not finding any evidences that postings was the culprit for the bad igw performance. Plus, we noticed a major issue with it: it doesn't bubble down shards correctly, so every query end up processing every shard.
) This reverts commit 1221658. **What this PR does / why we need it**: We decided to discontinue the postings cache a few releases ago for not finding any evidences that postings was the culprit for the bad igw performance. Plus, we noticed a major issue with it: it doesn't bubble down shards correctly, so every query end up processing every shard.
What this PR does / why we need it:
Introduces a new interface for fetching TSDB postings and implements a cached TSDB postings.
PostingsReaderand contains two implementations: a simple one, which is identical to the former Postings behavior, and a new one, that caches postings using the existingindex_queries_cache_config. By default, the simple/former implementation is used. The new cached implementation can be configured in the following way:In the future we'll remove the
cache_postingsflag/configuration and change the behavior to cache postings by default.Which issue(s) this PR fixes:
N/A
Special notes for your reviewer:
Checklist
CONTRIBUTING.mdguide (required)CHANGELOG.mdupdateddocs/sources/upgrading/_index.mdproduction/helm/loki/Chart.yamland updateproduction/helm/loki/CHANGELOG.mdandproduction/helm/loki/README.md. Example PR