I'm still working on stuff, but here is a summary of things so far: * ConcurrenyTest is passing * Caching implementation is now a pessimistic locking style based on versions. Locking never happens on reads, only writes. There are two caches. One cache holds the actual data, and is pretty much a vanilla invalidation cache. The other cache olds versioning information and is a local-only cache and just is an id->version map. Given an id, what is the current version of it. Hopefully, the implementation makes sure that you never are able to add or obtain a stale version of an object from the cache. Locking happens for updates. At the end of a transaction, all registered invalidations are iterated on, and a version cache lock on that id is obtain. The db is updated, and after that the locks are released. A lock is also obtained whever something is added to the cache. Again a lock is only obtained on the version cache, so any reads will never block. * KeycloakTransactionManager now has an enlistPrepare method. * There is a new RealmProvider.getClientByClientId() method. Clients now have a clientId "index" in the cache. * There is a new RealmProvider.removeClient() method. This was needed to support getClientByClientId() Some other things that were done: * Unnecessary @JoinTables were removed for certain @OneToMany relationships. * getId() will no longer cause a DB query if you are invoking on a reference to a JPA adapter * RealmModel.getClients() and getClientNameMap() is no longer used to implement getClientByClientId() :) * CachedRealm now stores PrivateKey, PublicKey, and Certificate in a transient variable. We were actually unmarshalling from the cached pem format every time these things were access, which is like few times per login. Next steps: * Create a client list cache for each realm. Currently, if you add/remove a client, this invalidates the realm cache and its doing a big query for each client. * Do some profiling to see if there's other things we can do.

Even if you have caching off this is a problem. You have to do this: * In RepresentationToModel, don't call setters on properties that have equal values * If its not already set, change hibernate to optimized updates, don't update fields that haven't changed On 2/17/2016 8:55 AM, Marek Posolda wrote: > Should this be done for user's cache too? Couldn't it happen that > same user is concurrently updated by 2 threads? > > Also wonder about scenario like: > - 2 admin users are editing realm at the same time > - Admin1 enables user's registration for the realm and click "update" > - Admin2 changed access token timeout and then click "update" . AFAIK > at this point, he overwrites the change by admin1 because we send > whole realm representation in the request. > > Do we want to address this? Or is this rather a corner case? > > Marek > > > On 12/02/16 19:47, Bill Burke wrote: >> I'm still working on stuff, but here is a summary of things so far: >> * ConcurrenyTest is passing >> * Caching implementation is now a pessimistic locking style based on >> versions. Locking never happens on reads, only writes. There are two >> caches. One cache holds the actual data, and is pretty much a vanilla >> invalidation cache. The other cache olds versioning information and is >> a local-only cache and just is an id->version map. Given an id, >> what is >> the current version of it. Hopefully, the implementation makes sure >> that you never are able to add or obtain a stale version of an object >> from the cache. Locking happens for updates. At the end of a >> transaction, all registered invalidations are iterated on, and a >> version >> cache lock on that id is obtain. The db is updated, and after that the >> locks are released. A lock is also obtained whever something is added >> to the cache. Again a lock is only obtained on the version cache, so >> any reads will never block. >> * KeycloakTransactionManager now has an enlistPrepare method. >> * There is a new RealmProvider.getClientByClientId() method. Clients >> now >> have a clientId "index" in the cache. >> * There is a new RealmProvider.removeClient() method. This was needed >> to support getClientByClientId() >> >> Some other things that were done: >> * Unnecessary @JoinTables were removed for certain @OneToMany >> relationships. >> * getId() will no longer cause a DB query if you are invoking on a >> reference to a JPA adapter >> * RealmModel.getClients() and getClientNameMap() is no longer used to >> implement getClientByClientId() :) >> * CachedRealm now stores PrivateKey, PublicKey, and Certificate in a >> transient variable. We were actually unmarshalling from the cached pem >> format every time these things were access, which is like few times per >> login. >> >> Next steps: >> * Create a client list cache for each realm. Currently, if you >> add/remove a client, this invalidates the realm cache and its doing a >> big query for each client. >> * Do some profiling to see if there's other things we can do. >> >

Hibernate/JPA supports optimistic locking with a version column. On 2/17/2016 10:03 AM, Marek Posolda wrote: > The thing is, when admin2 press "update realm" , the data on server > are already updated from admin1. So he will see the value of > "registrationEnabled" field is true and overrides it with his value > false. > > It seems the possibilitity to address this might be to add "version" > column in DB. The version is sent to UI of admin console, but it's > hidden. When admin2 press "update", he will send the version back and > he will see that version in DB is bigger than expected, so someone > updated realm in the meantime. > > But I don't know if this is really issue which needs to be solved or > rather corner case... > > Marek > > On 17/02/16 15:44, Bill Burke wrote: >> Even if you have caching off this is a problem. You have to do this: >> >> * In RepresentationToModel, don't call setters on properties that >> have equal values >> * If its not already set, change hibernate to optimized updates, >> don't update fields that haven't changed >> >> On 2/17/2016 8:55 AM, Marek Posolda wrote: >>> Should this be done for user's cache too? Couldn't it happen that >>> same user is concurrently updated by 2 threads? >>> >>> Also wonder about scenario like: >>> - 2 admin users are editing realm at the same time >>> - Admin1 enables user's registration for the realm and click "update" >>> - Admin2 changed access token timeout and then click "update" . >>> AFAIK at this point, he overwrites the change by admin1 because we >>> send whole realm representation in the request. >>> >>> Do we want to address this? Or is this rather a corner case? >>> >>> Marek >>> >>> >>> On 12/02/16 19:47, Bill Burke wrote: >>>> I'm still working on stuff, but here is a summary of things so far: >>>> * ConcurrenyTest is passing >>>> * Caching implementation is now a pessimistic locking style based on >>>> versions. Locking never happens on reads, only writes. There are two >>>> caches. One cache holds the actual data, and is pretty much a >>>> vanilla >>>> invalidation cache. The other cache olds versioning information >>>> and is >>>> a local-only cache and just is an id->version map. Given an id, >>>> what is >>>> the current version of it. Hopefully, the implementation makes sure >>>> that you never are able to add or obtain a stale version of an object >>>> from the cache. Locking happens for updates. At the end of a >>>> transaction, all registered invalidations are iterated on, and a >>>> version >>>> cache lock on that id is obtain. The db is updated, and after >>>> that the >>>> locks are released. A lock is also obtained whever something is >>>> added >>>> to the cache. Again a lock is only obtained on the version cache, so >>>> any reads will never block. >>>> * KeycloakTransactionManager now has an enlistPrepare method. >>>> * There is a new RealmProvider.getClientByClientId() method. >>>> Clients now >>>> have a clientId "index" in the cache. >>>> * There is a new RealmProvider.removeClient() method. This was needed >>>> to support getClientByClientId() >>>> >>>> Some other things that were done: >>>> * Unnecessary @JoinTables were removed for certain @OneToMany >>>> relationships. >>>> * getId() will no longer cause a DB query if you are invoking on a >>>> reference to a JPA adapter >>>> * RealmModel.getClients() and getClientNameMap() is no longer used to >>>> implement getClientByClientId() :) >>>> * CachedRealm now stores PrivateKey, PublicKey, and Certificate in a >>>> transient variable. We were actually unmarshalling from the >>>> cached pem >>>> format every time these things were access, which is like few >>>> times per >>>> login. >>>> >>>> Next steps: >>>> * Create a client list cache for each realm. Currently, if you >>>> add/remove a client, this invalidates the realm cache and its doing a >>>> big query for each client. >>>> * Do some profiling to see if there's other things we can do. >>>> >>> >> >