Spring EclipseLinkJpaDialect applies per-transaction isolation level to shared DatabasePlatform

spring orm version: 5.1.4.RELEASE

In EclipseLinkJpaDialect class line 81

if (definition.getIsolationLevel() != TransactionDefinition.ISOLATION_DEFAULT) {
    // Pass custom isolation level on to EclipseLink's DatabaseLogin configuration
    // (since Spring 4.1.2)
    UnitOfWork uow = entityManager.unwrap(UnitOfWork.class);
    uow.getLogin().setTransactionIsolation(definition.getIsolationLevel());
}

for instance: If txA beginTransaction and there is Isolation.SERIALIZABLE around the repository class, the TransactionIsolation in uow.getLogin() will be changed to Isolation.SERIALIZABLE. Then txB beginTransaction with Isolation.DEFAULT, the condition is not met, TransactionIsolation in uow.getLogin() still be SERIALIZABLE.

I think the right code should like this:

if (definition.getIsolationLevel() != TransactionDefinition.ISOLATION_DEFAULT
   && definition.getIsolationLevel() != entityManager.unwrap(UnitOfWork.class).getLogin().setTransactionIsolation()) {
    // ... 
}

Comment From: sbrannen

@deanwong, I think you meant to use getTransactionIsolation() instead of setTransactionIsolation() -- right?

Comment From: deanwong

@sbrannen yes

correct

definition.getIsolationLevel() != entityManager.unwrap(UnitOfWork.class).getLogin().getTransactionIsolation()

Comment From: jhoeller

UnitOfWork is supposed to be an independent instance for every EntityManager, so I wonder why there would be old serialization settings exposed there? Every fresh UnitOfWork within a new transaction should come with EclipseLink's default settings... We just override those if necessary, and at the end of every transaction the UnitOfWork and its settings gets closed and disposed.

Comment From: deanwong

We use Hikari as connection pool, does that affect the behavior?

Comment From: jhoeller

I suspect that EclipseLink applies the transaction isolation level to the underlying JDBC Connection but does not restore the original isolation level there before it returns the Connection to the pool. As far as I can see, it does not expose the current connection isolation level via DatabaseLogin.getTransactionIsolation() though, so the suggested check above won't help. We'd rather have to make sure to restore the original JDBC isolation level ourselves, similar to how we do it with Hibernate. A bit of a shame since EclipseLink has a higher-level API for this, in contrast to Hibernate, so it seems a bit awkward to manually dive down to JDBC level for restoring.

Comment From: deanwong

@jhoeller I just create a minimize project to reproduce this problem. Hope that could help. There are 4 interactions with DB, second query surrounding with SERIALIZABLE, and the following queries all use this isolation level.

Comment From: toalexivanov

transactionIsolation field in org.eclipse.persistence.internal.databaseaccess.DatabasePlatform is not synchronized. This is the root cause.

DatabasePlatform stores various database settings that rarely change at runtime. Perhaps that is what implementors of EclipseLink meant for transactionIsolation as well, set it once and forget. This is how most database applications use it anyway. It is rare for an application to use transaction isolation level other than the default.

An instance of this class is called by JpaTransactionManager which is a Spring bean in our application. Application accesses this bean from multiple threads. Now you can imagine multiple service threads starting transactions with different isolation levels concurrently. Also statement in EclipseLinkAdapter.beginTransaction() checks requested transaction isolation level and simply returns without setting it if it is equal to a default value. Transaction isolation level is set to default automatically in Spring if it is not set explicitly. In majority of our usages, we don't set this level explicitly for one simple reason - default isolation level READ_COMMITTED is sufficient for our purposes most of the time. However it means that transaction by explicitly setting non-default isolation level overrides this level for all subsequent transactions. Not only that, depending on timing, a transaction in one thread could silently override transaction isolation level in another thread.

To remedy this problem I supplied CustomOraclePlatform bean where I override setTransactionIsolation() and getTransactionIsolation(). These methods make use of a ThreadLocal field represeting isolation level. Now each thread gets its own isolation level. However transactions with different isolation levels in the same thread would still experience the problem. Also cached thread pools do not reset ThreadLocals. For that we need to prevent EclipseLinkJpaDialect from invoking short-circuit check for default level. This can be be done by translating TransactionDefinition.ISOLATION_DEFAULT to TransactionDefinition.ISOLATION_READ_COMMITTED (or whatever default level is appropriate for your database) in EclipseLinkJpaDialect.beginTransaction(). This is exactly what I did in CustomEclipseLinkJpaDialect.

Comment From: jhoeller

Well spotted, these calls internally delegate to the shared DatabasePlatform instance... despite appearing on an independent UnitOfWork instance and its DatabaseLogin. This means that EclipseLink's own support for transaction isolation levels is effectively unusable for our purposes :-( We'll see whether there is some way to apply an independent isolation level without modifying the DatabasePlatform implementation; otherwise we'll have to document this as being severely limited.

Comment From: jhoeller

I've locally applied a different patch: Using a custom TransactionalDatabaseLogin class for the UnitOfWork which locally stores the isolation level and only falls back to the platform-defined isolation level if no transaction-specific isolation level has been set. This seems to work fine so far, and decouples the solution from the vendor-specific DatabasePlatform implementation. It also covers the default-isolation-level problem (when mixing transactions with a custom isolation level next to transactions without a specific isolation level) as far as it will set the platform-defined transaction isolation by default, so we don't need to reset the isolation level ourselves.

Comment From: jhoeller

Quick update: There seems to be a problem with propagating such a custom login instance to the actual session used. EclipseLink's internal architecture doesn't make this very obvious... let's see whether we can reach a solution here, otherwise we might have to go with a different approach.

Comment From: jhoeller

It looks like this can only really be solved through a custom DatabasePlatform subclass for your particular dialect, overriding set/getTransactionIsolation to take the Spring transaction isolation level into account in a thread-local fashion, in interaction with a custom variant of EclipseLinkJpaDialect. To some degree I wonder whether we should revert to rejecting custom isolation levels by default in EclipseLinkJpaDialect... but then this would also prevent single-threaded scenarios and such custom DatabasePlatform variants. Maybe there's something we can do to make the latter easier to implement, and for the standard case we'll document the limitations.

Comment From: toalexivanov

Can you just set transaction isolation level directly on a JDBC Connection thus overriding whatever EclipseLink does?

Comment From: jhoeller

We could try to operate directly on the JDBC Connection but we'd have to clarify when exactly an EclipseLink Session obtains its JDBC Connection and when exactly it returns the Connection to the pool. Hibernate has well-defined connection policies for that purpose, whereas with EclipseLink we'd probably have to rely on implementation behavior (hoping that it won't change).

Comment From: jhoeller

We're rolling an EclipseLinkJpaDialect refinement into 6.0.10 and 5.3.28 now: see https://github.com/spring-projects/spring-framework/issues/29997#issuecomment-1575604420

While this still operates on a shared DatabasePlatform instance, it should work reliably in concurrent scenarios as well as in scenarios with inconsistent transaction isolation definitions now. This seems to be the best we can do with current EclipseLink, even in its 3.0 and 4.0 version.