feat(helm): Implementing support of l2 chunksCache memcache

[andrejshapal]

What this PR does / why we need it:
Not sure about speed (which should be the case), but cache reduces costs very much.
Even having 100 GB logs per day, the caching levels could help to avoid extra bucket operations.
Currently, the helm chart support chunksCache with ability to configure persistent storage and use memcached feature to store cold data on the disk. Without this feature, the volume of cache was limited by memory (memory is expensive...). Storing on disk made it possible to increase the cache volume, but there is a significant problem. In most of cases, the recent logs are queried (like few days old) and then there is a huge range of old logs which could be potentially queried (maybe just once). In cache we try to store the logs which likely to be queried. But when the disk space finished, we start to replace data. When old logs are queried, we put them in cache and potentially remove fresh logs. So, we keep in cache logs which likely will be not queried again and we remove those probably will be queried. To avoid this, we should avoid replacing new logs with old logs. This is where l2 cache appears. The logs older than n days are not being put in l1 cache and instead in l2 small cache. The old logs persist there for short period of time (in case if query will be executed multiple times while somebody searching something).

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Checklist

• Reviewed the CONTRIBUTING.md guide (required)
• Documentation added
• Tests updated
• Title matches the required conventional commits format, see here
  • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
• Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
• If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

[CLAassistant]

All committers have signed the CLA.

[jkroepke]

context: #10061

For records, I can't really find documentation.

Please do a rebase first, and avoid any changes not related to L2 - then I can do a full review again.

[andrejshapal]

context: #10061

For records, I can't really find documentation.

Please do a rebase first, and avoid any changes not related to L2 - then I can do a full review again.

Done. Thank you.

[jkroepke]

Hi, I go through your PR and it looks good for now. Thanks a lot.

Here are my thoughts:

Could you please add a file named "non-default.values.yaml" in https://github.com/grafana/loki/tree/main/production/helm/loki/ci

and enable both cache l1 and l2? Then this larger code would be part of the CI tests.

[jkroepke]

What is this?

[andrejshapal]

I think tests were failing. But don't remember honestly. Reverted for now.

[andrejshapal]

@jkroepke I think all comments resolved.

[jkroepke]

I do not have permissions yet to approve the CI, looking forward to next week for now

[JStickler]

I kicked off the CI tests for you.

Conventional commits - The heading should be either fix(helm): or feat(helm):.

[andrejshapal]

I kicked off the CI tests for you.

Conventional commits - The heading should be either fix(helm): or feat(helm):.

fixed, lint stage also

[andrejshapal]

I kicked off the CI tests for you.

Conventional commits - The heading should be either fix(helm): or feat(helm):.

Rebased as well. Please, trigger pipeline again.

[jkroepke]

something looks odd here. maybe a rebase, then merge from origin mess-up.

[andrejshapal]

something looks odd here. maybe a rebase, then merge from origin mess-up.

I had to force push amending previous commit due to missing email on my another PC and failing CLA signature.

[nicolevanderhoeven]

@andrejshapal Sorry about that. Please go ahead!

[jkroepke]

And sorry for the trouble. We have a very high merge volume in the last 3 weeks. Merge conflicts are expectable.

[andrejshapal]

@jkroepke Should be better. Could you launch tests?

There is actually thing I doubt. Now the pdb should not work with 1 replica. But I think people could be using it as prevention to restart memcache single pod loosing all the collected cache in memory (when pv is not used).

non-default values ci values should have only pdb for l2 cache to check functionality.

[jkroepke]

But I think people could be using it as prevention to restart memcache single pod loosing all the collected cache in memory (when pv is not used).

I can understand that i feel thats a bad design to choice a PDB as protection.If there is a node drain request incoming, the situation might be in stale unless the pod is removed. We should think about this request, if there is an user request for it.

[andrejshapal]

But I think people could be using it as prevention to restart memcache single pod loosing all the collected cache in memory (when pv is not used).

I can understand that i feel thats a bad design to choice a PDB as protection.If there is a node drain request incoming, the situation might be in stale unless the pod is removed. We should think about this request, if there is an user request for it.

Well, it anyway is not related to my MR, but rather to #18321

[jkroepke]

PDB tested locally.

LGTM!

[jkroepke]

PDB tested locally.

LGTM!

[jkroepke]

LGTM

[andrejshapal]

@QuentinBisson Could you double check as you are commenting ci test values?

[QuentinBisson]

Same as @jkroepke LGTM. This might need some extra documentation to explain how and why to use this cache but this can bé done later. This is some really nice improvement

[jkroepke]

This might need some extra documentation to explain how and why to use this cache but this can bé done later.

Yeah, I agree. But this is more in scope of the Loki product, not the Helm Chart.

#10061 is all what I found.

[QuentinBisson]

100% but once it's documented it needs to bé linked here later still :)

[andrejshapal]

@jkroepke @QuentinBisson fixed/resolved comments above, created task for docs #18563

[nicolevanderhoeven]

Thanks @andrejshapal for your work here-- this is a great addition to the Helm chart! Thanks @jkroepke and @QuentinBisson for your reviews!