Map storage: caching getRealmbyName() and getClientByClientId() within a session

[ahus1]

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() and realm.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 MapClientProvider and MapRealmProvider. This has been implemented to show the performance impact.

Here's a draft PR for this: #12329

Benefits

• 14% less CPU usage of total CPU usage for request handling, mostly during creation of the service user token and validation of the service token.

Caching in two places for realms and clients
	with change	without change
Total samples in 60 sec interval	38000	50000
getClientByClientId	2056	6284
getRealmByName	6115	11515
Rel percentage	0.21	0.35
Delta	-0.14

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 the findFirst() 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

./.

[vramik]

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.

[hmlnarik]

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.

[ahus1]

Hi @hmlnarik - please note that getRealmByName() and getClientByClientId() will use a criteria query to first identify the entities, and this criteria query will touch the DB in the JPA case.

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.

[hmlnarik]

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.