Lock entity and refresh L1 and L2 cache in a single database call

[AntoineDuComptoirDesPharmacies]

Hi everyone, Hi @rbygrave,

It could be interesting to have capability to refresh L1 and L2 cache while locking a row in database through SELECT FOR UPDATE.
Indeed, it will allow us to do only one SELECT to lock and refresh the data instead of two.

At the moment, if you lock the row without doing a clear call to refresh, you may experience strange behaviors like the following scenario :

In a transaction block :

1. We SELECT an object by Id (L1 and L2 cache set)
2. We lock this object through a SELECT FOR UPDATE command, (eg. DB.lock from ENH: Add lock(bean) method as convenience for DB pessimistic locking #2362). (Do not refresh L1 and L2 cache)
3. Later in the transaction, we SELECT object by this same id (Return value from step 1)

What we noticed is that, if the row in database is updated by another process between step 1 and step 2, the step 3 will only see old values of the Bean because it will grab it from level 2 cache set in step 1.

If we disable level 2 cache on step 3, we will still see the old value because none caches are refreshed in step 2.

If we set PersistenceContext to QUERY in the SELECT FOR UPDATE of step 2, the Bean returned contains the fresh values (from database) but it do not refresh level 1 and level 2 cache neither.

Solution is to call DB.refresh after DB.lock in step 2 but it imply two database query instead of one.

Yours faithfully,
LCDP

[rbygrave]

So I think the question becomes:

If we execute a query with pessimistic locking via any of query.forUpdate() or query.withLock(...) or database.lock(...)
... AND there is already the same bean in the persistence context (which potentially old values)

should that select for update be [automatically] treated like a refresh meaning that it will use the new database values and update the existing instance with those values?

---

My thought is that, well I haven't hit this issue and I'd say that is because I generally see code that is performing the select for update first ... and is not doing a prior normal select or prior 'hit L2 cache'. That said, I can easily see the desire to do that in some cases, for example hitting L2 cache first, checking some business logic before we decide to proceed and really go for the DB lock via select for update.

So the use case seems good and valid.

If we set PersistenceContext to QUERY in the SELECT FOR UPDATE of step 2, the Bean returned contains the fresh values (from database) but it do not refresh level 1 and level 2 cache neither.

Right, as in yes that is what we currently expect to see ... but that doesn't feel like the desirable behavior.

Solution is to call DB.refresh after DB.lock in step 2 but it imply two database query instead of one.

Yes, that doesn't seem great, does not seem to be what we want.

[rbygrave]

Need to check query.setBeanCacheMode(CacheMode.PUT) ... but I don't think that works as the values are not refreshed from the database (this is really about the L1 cache / Persistence Context and the values on the bean not getting refreshed in this FOR UPDATE case).

[AntoineDuComptoirDesPharmacies]

You are totally right about the use case, we are checking through some business logic before deciding to really lock or not.
Our code under lock is robust and idempotent so if the row change between our business check and lock, it is acceptable as soon as the modified data is available in the Bean (modified data loaded through FOR UPDATE or refresh called).

I suppose that adding the refresh feature to the SELECT FOR UPDATE query may imply a bit more of complexity/cost for this request as today it is only fetching the "id" field, am I wrong ?
So, if SELECT FOR UPDATE refresh the Bean, either it have to reset the laziness state of the Bean or it have to add all loaded fields of the Bean inside the generated SQL query.

It do not seems to be an easy feature to implement but it may improve the implicit behavior of SELECT FOR UPDATE on already loaded Bean.

[jnehlmeier]

With JPA you have

entityManager.find(entityClass, id, lockMode, properties)
entityManager.lock(entity, lockMode, properties)
entityManager.refresh(entity, lockMode, properties)

Generally speaking whenever I do a pessimistic lock I want to have the newest data and make sure everybody else has to wait because I have to do the read-decide-write cycle atomically.

Looking at the code of EclipseLink entityManager.lock() does refresh the entity by default for any pessimistic lock but not for optimistic locks. On the other hand entityManager.refresh() always refreshes the entity regardless of the lock type. Both methods also cascade that refresh operation down to relations by default if they decide to refresh the entity. This behavior can be changed by providing corresponding properties.

I think this is the perfect default.

• For pessimistic locks refresh() and lock() behave the same as they refresh the object by default to make sure you have the most up-to-date data to work with and you can do that atomic work you need to do.
• For optimistic locks only refresh() does a refresh since that is the purpose of the method. lock() does not since you accept to fail yourself if changes have been made by others in the meantime. You would fail when calling lock() or when committing/flushing and versions are compared.

[rbygrave]

Currently I think this means we could treat this like a bug. What that means is that:

• I can't think of a use case where the current behavior is useful / what people would ever want. If the use case somehow does not want refresh then that means it should want query.setPersistenceContextScope(QUERY) rather than 'just ignore the fresh database values' (the current behavior does not seem useful in any way)
• I think we should look to just change the behavior and not try and support some feature toggle / property configuration that turns it off because it seems like the current behavior of ignoring the fresh db values is never useful plus it would align with the behavior with EclipseLink (and maybe other JPA providers)

It do not seems to be an easy feature to implement

Lets see. If we return true here - https://github.com/ebean-orm/ebean/blob/master/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeLoadBean.java#L193 ... then we do 'refresh' which will load the values into the existing bean
... so it should be something like return true if 'refresh' or 'pessimistic locking / select for update' (which would be determined when this SqlTreeLoadBean is created).

[rbygrave]

Created Issue #3020 for this

[rob-bygrave]

This was treated as a bug and fixed in 13.17.0