Skip to content

fix: add caching to Entitlement#527

Open
dattito wants to merge 4 commits intomainfrom
push-qryvxvskpkru
Open

fix: add caching to Entitlement#527
dattito wants to merge 4 commits intomainfrom
push-qryvxvskpkru

Conversation

@dattito
Copy link
Copy Markdown
Contributor

@dattito dattito commented Feb 25, 2026

Fixes #256.

Because a lot of Entitlement CRs in a cluster lead to a burst of API requests against the BTP server, this PR introduces a chaching layer that prevents to many GET calls to the BTP server. The cache invalidation timeout is 10 seconds.

Some modifications, like Create, Update or Delete, will automatically invalidate the cache, because we can be sure that the external state has changed.

@dattito dattito marked this pull request as ready for review February 25, 2026 15:58
@dattito dattito added the release-notes/bugfix Marks PR as bugfix for release note generation label Feb 25, 2026
@dattito dattito force-pushed the push-qryvxvskpkru branch from 73611e5 to 919261a Compare March 3, 2026 13:16
@dattito dattito force-pushed the push-qryvxvskpkru branch from 919261a to 7ab885b Compare March 3, 2026 13:29
@dattito dattito force-pushed the push-qryvxvskpkru branch from 7ab885b to b40ae0b Compare March 3, 2026 13:32
Comment thread internal/clients/entitlement/entitlement.go Outdated
Comment thread internal/controller/account/entitlement/entitlement.go Outdated
Comment thread internal/clients/entitlement/cache.go Outdated
Copy link
Copy Markdown
Member

@maximilianbraun maximilianbraun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

additionally to my comment:

The final implementation (we should validate how well the approach works in real life, beforehand) should expose metrics (https://book.kubebuilder.io/reference/metrics.html), ie. how the cache hit / miss rate is. This would anyway be interesting for evaluation in real life and to fine tune parameters. The Library is exposing different metrics already. Maybe logging them every 30s might already help for validation.

I wonder on the other side how its about syncronisation. The cache might only be effective if consecuritve runs wait for each other, the entitlement get call is quite expensive (terms of size) and might take some time to transport.

I think the loader can help with that. https://maypok86.github.io/otter/user-guide/v2/examples/#loading

Comment thread internal/clients/entitlement/client.go
@dattito
Copy link
Copy Markdown
Contributor Author

dattito commented Mar 5, 2026

Thank you for your feedback. All changes are incorporated!

The new Prometheus metrics were not tested, as I first would have to build the setup for this. I can however do this if required.

@jtucci
Copy link
Copy Markdown
Contributor

jtucci commented Mar 27, 2026

Hey @dattito , great work on this PR! I noticed it also fixes the bug reported in #597 (entitlement quota being transiently zeroed when deleting one of multiple CRs).

Since the caching feature is getting some review feedback, would you consider splitting this into two PRs, one with just the bug fix so it can be fast-tracked, and one for the caching layer? That way the bug fix can land sooner without being blocked by the caching discussion. Alternatively, if @maximilianbraun is not expecting much more work on this PR, is there a chance it could be merged soon as-is? Happy to help if useful!

dattito added 3 commits March 27, 2026 16:31
On-behalf-of: David Siregar <david.siregar@sap.com>
Signed-off-by: David Siregar <david.siregar@sap.com>
On-behalf-of: David Siregar <david.siregar@sap.com>
Signed-off-by: David Siregar <david.siregar@sap.com>
On-behalf-of: David Siregar <david.siregar@sap.com>
Signed-off-by: David Siregar <david.siregar@sap.com>
@dattito
Copy link
Copy Markdown
Contributor Author

dattito commented Mar 27, 2026

just rebased on main and fixed merge conflicts.

On-behalf-of: David Siregar <david.siregar@sap.com>
Signed-off-by: David Siregar <david.siregar@sap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes/bugfix Marks PR as bugfix for release note generation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Spike] Optimization of Entitlement: many network requests

4 participants