Re: [keycloak-dev] Issues with clustered invalidation caches

+1 to evict the user cache just for the UserStorageProvider updates (or it's subcomponents). Not evict for every realm update as that sounds like the unecesary performance penalty. Marek On 05/11/16 15:11, Bill Burke wrote: > We don't currently evict the user cache if a realm has changed. This is > an issue if somebody has modified a UserStorageProvider. Should we evict > user cache for any realm changes? Or should I modify the cache > RealmAdapter and detect whether a UserStorageProvider is being modified > (or any of its subcomponents(mappers)).? > > > On 11/3/16 1:10 PM, Bill Burke wrote: >> +1 to this. We are already hacking Infinispan horribly and doing nasty >> work arounds to get the behavior we want. >> >> Some notes: >> >> * You have to be sure to invalidate indexes (email->user, >> username->user) correctly >> >> * We cache relationships in some scenarios, i.e. the list of clients in >> a realm. Your proposal makes things so much easier for caching >> relationships as we know the difference between a realm delete and a >> realm update and a client create, update, and removal >> >> * Cache is currently not interacted with at all for queries that return >> multiple users (getUser(), searchForUser, etc.). We invalidate the >> realm on these queries as we don't know if the caller is going to >> perform any updates on those users. >> >> * Role and group removal is a tricky thing as role/group mappings are >> cached with the user. You still have the iteration problem discusssed >> earlier and you have to decide which is more efficient, evicting all >> users in the realm, or iterating over all users that might have this >> role/group mapping. There's also client scope mapping, client mappers, >> and broker mappers which may also reference the role/group. >> >> * If a user storage provider is removed, this is also a tricky scenario >> and you may be better off evicting all users instead of iterating over >> all cached users to find all that were loaded by this provider. >> >> * Client removal is tricky because you also have to remove all user role >> mappings, scope mappings, that are cached. >> >> There may be other cases I'm missing. >> >> >> On 11/3/16 7:06 AM, Marek Posolda wrote: >>> I was looking at the cache issue reported by customer. I found the cause >>> of it and couple of other related issues: >>> >>> KEYCLOAK-3857 - Bad performance with clustered invalidation cache when >>> updating object >>> KEYCLOAK-3858 - Removing model object send lots of invalidation messages >>> across cluster >>> KEYCLOAK-3859 - Lots of userCache invalidation messages when >>> invalidating realm >>> KEYCLOAK-3860 - All realm users are invalidated from cache when removing >>> some realm object >>> >>> >>> In shortcut, our cache works fine in local mode. But in cluster, there >>> are issues with the invalidation caches . We don't have issues with >>> stale entries, but this is purchased but lots of various performance >>> issues like: >>> >>> - There are lots of invalidation messages sent across the cluster >>> >>> - Eviction on the node, which received invalidation event, is also very >>> uneffective. For example evicting realm with 1000 roles needs to call >>> 1000 predicates, which iterates the cache 1000 times. >>> >>> - Invalidation cache doesn't allow to differ between the context why the >>> object was invalidated. For example when I update realm settings on >>> node1, I need to invalidate just the CachedRealm object, but not all the >>> other objects dependent on the realm. However the invalidation event >>> received on node2 doesn't know, if I invalidated CachedRealm because of >>> realm update or because of realm removal. So for more safety, it assumes >>> removal, which evicts all realm objects! See >>> https://issues.jboss.org/browse/KEYCLOAK-3857 for details. >>> >>> - Finally we have the workaround with the "invalidation.key" objects in >>> our invalidation caches. This is currently needed because when >>> invalidating object on node1, the invalidation event is NOT received on >>> node2 unless the object is there. Hence the workaround with the >>> "invalidation.key" records just to avoid this limitation of invalidation >>> cache. >>> >>> >>> For solve all these issues, I propose: >>> - Instead of relying on invalidation caches, we will send notification >>> across cluster what happened (eg. message "realm XY was updated"). All >>> the nodes will receive this notification and will evict all their >>> locally cached objects accordingly and bump their revisions locally. >>> This would be much more stable, performant and will allow us to remove >>> some workarounds. >>> >>> Some details: >>> >>> - The caches "realms" and "users" won't be "invalidation" caches, but >>> they will be "local" caches. >>> >>> - When any object needs to be removed from cache because of some reason >>> (eg. updating realm), the notification message will be sent from node1 >>> to all other cluster nodes. We will use the replicated cache for that. >>> Node1 will send the notification message like "realm XY was updated" . >>> >>> - Other cluster nodes will receive this message and they will locally >>> trigger evictions of all the objects dependent on particular realm. In >>> case of realm update, it's just the CachedRealm object itself. In case >>> of realm removal, it is all realm objects etc. >>> >>> - Note message will contain also context "realm XY was updated" or >>> "realm XY was removed" . Not just "invalidate realm XY". This allows >>> much more flexibility and in particular avoid the issues like >>> https://issues.jboss.org/browse/KEYCLOAK-3857 . >>> >>> - We already have replicated cache called "work", which we are using to >>> notify other cluster nodes about various events. So we will just use >>> this one. No need to add another replicated cache, we will probably just >>> need to configure LRU eviction for the existing one. >>> >>> - Also note that messages will be always received. We won't need >>> workaround with "invalidation.key" objects anymore. >>> >>> - Also we don't need recursive evictions (which has very poor >>> performance. See https://issues.jboss.org/browse/KEYCLOAK-3857 ), >>> because receiving node will know exactly what happened. It will remove >>> objects just the same way like the "sender" node. >>> >>> - Finally the amount of traffic sent across the cluster will be much lower. >>> >>> This sounds like the big step, but IMO it's not that bad :) Note that we >>> already have all the predicates in place for individual objects. The >>> only change will be about sending/receiving notifications across >>> cluster. I think I am able to prototype something by tomorrow to >>> doublecheck this approach working and then finish it somewhen middle >>> next week. WDYT? >>> >>> Marek >>> >>> _______________________________________________ >>> keycloak-dev mailing list >>> keycloak-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/keycloak-dev >> _______________________________________________ >> keycloak-dev mailing list >> keycloak-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/keycloak-dev > _______________________________________________ > keycloak-dev mailing list > keycloak-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/keycloak-dev _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev