EclipseLinkJpaDialect: Unexpected default isolation levels

[avgustinmm]

Affects: 6.0.4
(and before)

---

It seems that in some cases EclipseLinkJpaDialect doesn't handle correctly isolation levels.
For instance in the following flow:

1. Execute transaction with SERIALIZABLE isolation level -> EclipseLinkJpaDialect#beginTransaction sets to DatabaseLogin / DatabasePlatform the isolation level and probably all is ok and the SERIALIZABLE isolation level is applied. After that transaction the DatabasePlatform's isolation level isn't cleared / reset and stays SERIALIZABLE
2. Then start a new transaction with DEFAULT isolation level which, according to documentation, should be the default isolation of the underlying database. However, since EclipseLinkJpaDialect#beginTransaction doesn't do anything with isolation levels (when used DEFAULT) the isolation level in DatabasePlatform stays SERIALIZABLE and it seems that isolation level is used instead of DEFAULT.

I suppose that EclipseLinkJpaDialect#beginTransaction should set the DEFAULT isolation level to DatabasePlatform instead explicitly doing nothing in that case. Or, somehow DatabasePlatform's isolation level shall be reset after transaction.

I'm attaching a reproducer (reproducer.zip, main method to be considered is Reproducer#repoduce) that shows that:

• before first transaction the DatabasePlatform isolation level is -1 (DEFAULT)
• after a transaction with SERIALIZABLE isolation level in the DatabasePlatform stays SERIALIZABLE - not reset
• the method reproduce fails (against mysql) with:

com.mysql.cj.jdbc.exceptions.MySQLTransactionRollbackException: Lock wait timeout exceeded; try restarting transaction

• when however line 40 (entityManager.unwrap(UnitOfWork.class).getPlatform().setTransactionIsolation(-1);) is uncommented and default level is reset to -1 (as at the beginning) the reproduce method passes. So we have different behavior of depending on the DatabasePlatform's isolation level before the last 2 transactions
• If you comment / remove the first transaction (SERIALIZABLE) and execute just the last 2 transactions - reproduce passes. So, the behavior of the last 2 transaction is influenced by the first one - which should not be the case

Note: reproducer is configured to use local mysql database (see the application properties) .

[jhoeller]

Relates to #24802 - the isolation level support for EclipseLink is somewhat broken and only really works properly in single-threaded scenarios with consistent isolation level declarations. We need to revisit what we're doing about isolation levels there: It might be better to deprecate isolation level support with EclipseLink to begin with - since we do not have APIs for proper per-transaction transaction isolation there but rather apparently just a system-wide shared isolation level setting in DatabaseLogin.

[jhoeller]

Since EclipseLink applies a custom transaction isolation value to its shared DatabasePlatform instance, we need to restore the original shared value afterwards. We set and reset the shared isolation value within a synchronized transaction begin sequence in EclipseLinkJpaDialect now (only in case of a custom isolation level specified) in order to neither interfere with subsequent nor with concurrent transactions. This comes at a bit of synchronization cost but since we're only paying that for actual custom transaction isolations, it should be worth making it work properly that way.

[avgustinmm]

Hi,
sorry for commenting a closed issue but I think there still has a problem with isolation levels and concurrency.
AFAIU, after the fix a default isolation level transaction could run concurrently with a custom isolation level transaction hence could use the set custom isolation level instead of the default.

[jhoeller]

Good point, this can be improved through identical synchronization around the beginEarlyTransaction call for the default isolation case. So this can work reliably for non-readOnly and non-lazy transaction scenarios at least. For the rest, we should document a potential isolation level mismatch in the default case which might be acceptable for read-only scenarios. In any case, read-only as well as lazy transactions are entirely opt-in - and not typically used in isolation level scenarios, I suppose.

[jhoeller]

The additional synchronization applied to the regular non-lazy transaction begin case is making it into 6.0.10 now, whereas the 5.3.28 backport will remain limited to the original purpose of a more reliable code path for consistent isolation level usage (while leaving the default code path untouched). Corresponding documentation of the care to be taken will be available in the javadoc.