[keycloak-dev] caching changes

[Marek Posolda]

Yeah, so question is if we need it? It looks to me like corner scenario 
and nobody complained so far AFAIK, so maybe not...

Marel

On 17/02/16 17:06, Bill Burke wrote:
> 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.
>>>>>
>>>>
>>>
>>
>