Repository navigation
Replies: 2 comments 2 replies
|
Thanks for bringing this up! In case we decide we'll go with the approach it could be also enhanced to cache model which are being created within the same session. |
|
Thank you for the write-up. Having a session-level cache makes perfect sense. In fact, this is exactly what map storage transaction does for CHM and Infinispan - before reading an object by ID in the datastore, it searches the list of previously read objects in a keycloak-side (not datastore-side) transaction, and only looks into the datastore itself when needed. Such an optimization is not needed in Hot Rod or CHM exactly for this reason, and needs thus be selectively enabled only for certain stores. Therefore, this optimization should not touch map providers. The good news is that the tree store will enable such an optimization: Consider a map store which has session lifespan for its objects, and would not contain any persistent storage. Putting such a storage on top of say JPA storage would lead to exactly the same caching strategy as suggested in this discussion without changing the provider logic. |
|
Hi @hmlnarik - please note that Therefore a simply caching of entities by ID will not be sufficient here (which JPA already does with its persistence context). Still, I'll let this issue rest until the tree store arrives and I can analyze if it will support this case. |
|
That is correct, and the caching store can and should support it. While I don't want to go into the details, let me state the idea as technically this can be very straightforward, by using a map from criteria to query result. Indeed the logic of updating the map needs to be defined, but as a sample of a simple one, the logic might be cache if and only if the criteria contains reference to an ID and (realm / role / group / client / user / ...) name. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Problem statement
When Keycloak handles one request, it creates one Keycloak session. With the current implementation of the Map storage (similar to the original store), each call to
sessions.realms().getRealmByName()andrealm.getClientByClientId()will search the store, even when called multiple times within the same session.For the JPA store and running the keycloak-benchmark of
CreateDeleteClients, these multiple calls lead to an CPU overhead of 15% in total processing time due the additional calls to Hibernate's dirty handling and calls to the database.In a setup with the legacy store and with the cache enabled, the impact would be negligible as the cache would provide the result. Still, with the legacy store there is a performance impact for those who disabled the cache.
Approach
When retrieving that information, it should be cached within the current session, that is: repeated calls should always retrieve the same realm and client given the same parameters and given an unchanged realm name and clientId.
As this applies to all Map stores (and not to a particular one), this first naive approach handles this in
MapClientProviderandMapRealmProvider. This has been implemented to show the performance impact.Here's a draft PR for this: #12329
Benefits
Alternatives
This could be implemented as a caching facade for all
ClientProvider/RealmProvider.This might be handled by a caching map store implementation - even if it wouldn't cache cross-session, it would be a good start to cache per-session. The logic would be more generic, and would involve a higher complexity with regards to invalidation and caching itself: The criteria queries at hand apply pagination, and also streaming with
findFirst()which "aborts" the stream after the first element and make it therefore difficult to stream the full set of results. Maybe thefindFirst()could be re-written using pagination.Not do anything, as 14% performance improvement is not considered worth the change.
Stay with the approach, but do this later.
Other
./.
All reactions