On 07/12/15 09:36, Stian Thorgersen wrote:


On 3 December 2015 at 16:10, Bill Burke <bburke@redhat.com> wrote:
On 12/3/2015 7:50 AM, Stian Thorgersen wrote:
> There's still some outstanding issues with the realm cache. It works,
> but can and should be improved for 1.8.
>
> One issue was that once the realm is updated any methods on clients,
> roles or groups returns the underlying adapter instead of the cache
> adapters. As a work around in 1.7 it now ejects all clients for a realm
> when it sees any changes.
>

Why is that a bad thing?  Usually, roles groups, and clients are not
accessed in the same session as a realm update.  Realms are usually not
updated.  Client registration/unregistration is rare too for most apps.
  The vast majority (90%+?) of access is read-only for realms and clients.

The problem is that once you return the jpa client adapter instead of the cache client adapter the cache no longer knows it should be invalidated. So if you do:

    realm.set...
    client = realm.getClientById("")
    client.set...

The realm is invalidated, but not the client. That's why I had to add a work-around that invalidates all clients of a realm when the realm is changed.

I agree writes are rare. If we end up invalidating everything for a realm for every update that's not a good thing either though.
Depends. It seems that update realm settings is so rare operation, that it's likely not a problem to be rather defensive and invalidate everything (unless we figure out easy and safe way to not do it). On the other hand add/remove role is possibly more common, so here we should be likely more smart in invalidating stuff.

Marek


Also, we need to consider the way things are invalidated. Sending a single message to the cluster that a realm is invalidated is ok. However, if we send a message for each realm, client and role for each update that would be terrible.

 

> We have a few potential ways to solve this:
>
> a) try to always return cache adapters - I went down this road attacking
> it from a few different approaches, but was never successful as there
> was always something that didn't work

See above, I don't think this is an issue.  What we should do is
identify if any updates are performed on realms/clients per login/token
refresh and remove or isolate them so that the realm/client caches
aren't invalidated.

+1 At least for 1.x that would be the target. After 1.x I'd like to have a separate store for clients like we do for users. I think in some scenarios there may be 1K+ clients.
 

> b) only cache realms and have everything else hang off it - this is my
> preferred option for now. As long as updating clients requires
> invalidating the realm it seems a bit over the top to have separate
> caches for everything

Why can't you keep it as it is?

RealmAdapter.getDelegateForUpdate() always registers a realm
invalidation.  add/remove client are methods on RealmModel so the realm
cache was always invalidated.  The only time you need to invalidate the
realm is when clientId is changed.

As I said above if realm returns jpa adapter for clients when you retrieve clients then updates to clients are not visible to the cache after the realm has been updated.
 

> c) make the cache smarter - instead of invalidating a realm, make sure
> we add/remove the clients, etc..
>

Its an invalidation cache, so "C" won't work unless you have a separate
cache for the client list.  So you'd need a realm cache, client list
cache, and client cache.

> We also need more automated testing around clustering. Late in 1.7
> release process I identified that caches where invalidated when other
> nodes loaded things to it, so effectively the cache wasn't working at
> all in a cluster.
>
> Thoughts?
>

I think this is a bit of effort for little gain.  users will only see a
difference if there is a lot of realm adminstration happening.

For small/medium installations there's probably little realm admin going on. However, on big installations where performance is crucial I imagine that some admin is going to be relatively frequent so it's worth limiting what is invalidated.
 


--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
_______________________________________________
keycloak-dev mailing list
keycloak-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev



_______________________________________________
keycloak-dev mailing list
keycloak-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev