On 18 February 2016 at 08:43, Marek Posolda <mposolda(a)redhat.com> wrote:
On 18/02/16 08:07, Stian Thorgersen wrote:
Having two many joins (fetching everything about a realm in one query) is
probably going to be bad for performance, especially if there are loads of
clients and roles. There can also be large difference between different
vendors.
Maybe clients and roles (and groups?) shouldn't be loaded eagerly with
realm, but I guess most of the other stuff can be (ie. federation
providers, identity providers, SMTP configurations etc). Also for clients
we can eagerly load most of the stuff (ie. redirectUris, scopes ). ATM we
eagerly preload whole realm in CachedRealm constructor, but we have
fetchType.LAZY everywhere. IMO this is not optimal and would mean lot of
SQL at startup during realm preload (I did not doublecheck myself).
I don't really think it's a priority and I suspect performance might
actually be worse if we do a lot of eager loading than lazy loading.
Depends on how Hibernate loads it of course, but if it does it through a
crazy join then performance will suffer.
Marek
Another thing in the future we should separate clients out into a separate
store. There could be thousands of clients or even more. So they should be
treated in a similar fashion to users. Does that have impact on how we
improve/refactor/fix caching now?
On 17 February 2016 at 21:56, Bill Burke <bburke(a)redhat.com> wrote:
>
>
> On 2/17/2016 3:09 PM, Marek Posolda wrote:
>
> On 17/02/16 18:11, Bill Burke wrote:
>
> Currently, adding or deleting a client, or updating a realm causes
> invalidation/eviction of the realm and all clients in that realm. To
> make matters worse, the next time the realm is accessed, it queries and
> loads each client and its relationships. Why do we do this? When a
> realm invalidation happens, the cache has no idea if the realm is just
> being updated or removed entirely from the DB. With a realm removal, you
> also need to evict the cache of all clients within the realm. So, a
> cached realm MUST have a list of all clients within it. As a result,
> the more clients that get added, keycloak gets slower and slower.
> Eventually though the cache stabilizes after inserts/update/deletes
> subside and we get back to normal performance, but you can see a nasty
> blip for a little bit.
>
> We don't need to eagerly preload all clients when realm is loaded.
> Infinispan has streaming/predicate API and we are using it in many places
> (see InfinispanUserSessionProvider and all the stuff in package
> org.keycloak.models.sessions.infinispan.stream ). So when entry is
> invalidated, we can have cacheListener, which will query infinispan to
> return all cached clients of the realm and remove them. I can see that we
> already have listener. So possibly we can just change this line to use
> predicate query:
>
<
https://github.com/keycloak/keycloak/blob/master/model/infinispan/src/mai...
>
https://github.com/keycloak/keycloak/blob/master/model/infinispan/src/mai...
>
>
>
> Lol, i was sort of kind of doing that doing cache.values(). This makes
> it much easier with much less refactoring.
>
> Other option is, that we can try to optimize eager preload and see if we
> can reduce number of SQL queries at startup. For example, it seems that
> CachedClient constructor sends SQL query to preload just realm roles. And
> then each client sends another SQL query to preload his client roles
> (method CachedClient.cacheRoles ). Couldn't we instead send one SQL query
> to load all roles of realm (both realm and client roles)? Maybe we can even
> send one big SQL query to load realm with all it's objects :-) Possibly
> some FetchType.EAGER in Hibernate entities could help here.
>
>
> yeah, we should start looking into stuff like this. We can at least
> reduce the number of sql calls for caching a client or realm individually.
> I'm not sure how feasible it is though or how much better it is. Realms
> and clients contain more than a few one to many relationships which would
> make the query result HUGE if I remember how joins work.
>
> --
> Bill Burke
> JBoss, a division of Red
Hathttp://bill.burkecentral.com
>
>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev(a)lists.jboss.org
>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
>