zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-07-23 19:20:44 +02:00
Code Issues Projects Releases Packages Wiki Activity

Allow nested transactions when per-transaction isolation level is on (#2121)

Browse source 
It turns out that disallowing all nested transaction is impractical. So
in this PR we make it possible to run nested transactions (which are not
really nested as far as SQL is concerned, but rather lexically nested
calls to tm().transact() which will NOT open new transactions when
called within a transaction) as long as there is no conflict between the
specified isolation levels between the enclosing and the enclosed
transactions.

Note that this will change the behavior of calling tm().transact() with
no isolation level override, or a null override INSIDE a transaction.
The lack of the override will allow the nested transaction to run at
whatever level the enclosing transaction runs at, instead of at the
default level specified in the config file.
This commit is contained in:
Lai Jiang 2023-08-24 14:35:59 -04:00 • committed by GitHub
parent da28a2021c
commit a91ed0f1ad
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 127 additions and 24 deletions
Download patch file Download diff file Expand all files Collapse all files

2
core/src/main/java/google/registry/config/files/nomulus-config-unittest.yaml
View file

@ -28,4 +28,4 @@ misc:
transientFailureRetries: 3
hibernate:
perTransactionIsolation: false
perTransactionIsolation: true

15
core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java
View file

@ -167,13 +167,18 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
public <T> T transactNoRetry(
Supplier<T> work, @Nullable TransactionIsolationLevel isolationLevel) {
if (inTransaction()) {
if (getHibernatePerTransactionIsolationEnabled()) {
throw new IllegalStateException("Nested transaction detected");
} else {
logger.atWarning().log("Nested transaction detected");
return work.get();
if (isolationLevel != null && getHibernatePerTransactionIsolationEnabled()) {
TransactionIsolationLevel enclosingLevel = getCurrentTransactionIsolationLevel();
if (isolationLevel != enclosingLevel) {
throw new IllegalStateException(
String.format(
"Isolation level conflict detected in nested transactions.\n"
+ "Enclosing transaction: %s\nCurrent transaction: %s",
enclosingLevel, isolationLevel));
}
}
return work.get();
}
TransactionInfo txnInfo = transactionInfo.get();
txnInfo.entityManager = emf.createEntityManager();
EntityTransaction txn = txnInfo.entityManager.getTransaction();

120
core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerImplTest.java
View file

@ -16,6 +16,7 @@ package google.registry.persistence.transaction;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
import static google.registry.persistence.PersistenceModule.TransactionIsolationLevel.TRANSACTION_READ_COMMITTED;
import static google.registry.persistence.PersistenceModule.TransactionIsolationLevel.TRANSACTION_READ_UNCOMMITTED;
import static google.registry.persistence.PersistenceModule.TransactionIsolationLevel.TRANSACTION_REPEATABLE_READ;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
@ -126,25 +127,122 @@ class JpaTransactionManagerImplTest {
}
@Test
void transact_nestedTransactions() {
// Unit tests always allows for per-transaction isolation level (and therefore throws when a
// nested transaction is detected).
if (RegistryConfig.getHibernatePerTransactionIsolationEnabled()) {
IllegalArgumentException e =
void transact_nestedTransactions_perTransactionIsolationLevelEnabled() {
if (!RegistryConfig.getHibernatePerTransactionIsolationEnabled()) {
return;
}
// Nested transactions allowed (both at the default isolation level).
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(tm().getDefaultTransactionIsolationLevel());
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(
tm().getDefaultTransactionIsolationLevel());
});
});
// Nested transactions allowed (enclosed transaction does not have an override, using the
// enclosing transaction's level).
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(TRANSACTION_READ_UNCOMMITTED);
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(TRANSACTION_READ_UNCOMMITTED);
});
},
TRANSACTION_READ_UNCOMMITTED);
// Nested transactions allowed (Both have the same override).
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(TRANSACTION_REPEATABLE_READ);
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(TRANSACTION_REPEATABLE_READ);
},
TRANSACTION_REPEATABLE_READ);
},
TRANSACTION_REPEATABLE_READ);
// Nested transactions disallowed (enclosed transaction has an override that conflicts from the
// default).
IllegalStateException e =
assertThrows(
IllegalArgumentException.class,
IllegalStateException.class,
() ->
tm().transact(
() -> {
tm().transact(() -> {});
tm().transact(() -> {}, TRANSACTION_READ_COMMITTED);
}));
assertThat(e).hasMessageThat().isEqualTo("Nested transaction detected");
} else {
assertThat(e).hasMessageThat().contains("conflict detected");
// Nested transactions disallowed (conflicting overrides).
e =
assertThrows(
IllegalStateException.class,
() ->
tm().transact(
() -> {
tm().transact(() -> {});
});
tm().transact(() -> {}, TRANSACTION_READ_COMMITTED);
},
TRANSACTION_REPEATABLE_READ));
assertThat(e).hasMessageThat().contains("conflict detected");
}
@Test
void transact_nestedTransactions_perTransactionIsolationLevelDisabled() {
if (RegistryConfig.getHibernatePerTransactionIsolationEnabled()) {
return;
}
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(tm().getDefaultTransactionIsolationLevel());
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(
tm().getDefaultTransactionIsolationLevel());
});
});
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(tm().getDefaultTransactionIsolationLevel());
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(
tm().getDefaultTransactionIsolationLevel());
});
},
TRANSACTION_READ_UNCOMMITTED);
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(tm().getDefaultTransactionIsolationLevel());
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(
tm().getDefaultTransactionIsolationLevel());
},
TRANSACTION_READ_UNCOMMITTED);
});
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(tm().getDefaultTransactionIsolationLevel());
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(
tm().getDefaultTransactionIsolationLevel());
},
TRANSACTION_READ_UNCOMMITTED);
},
TRANSACTION_READ_UNCOMMITTED);
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(tm().getDefaultTransactionIsolationLevel());
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(
tm().getDefaultTransactionIsolationLevel());
},
TRANSACTION_READ_COMMITTED);
},
TRANSACTION_READ_UNCOMMITTED);
}
@Test