mirrors/nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus synced 2026-09-06 16:17:13 +00:00
Code Issues Releases Wiki Activity

Add the ability to specify per-transaction isolation level (#2104)

Browse Source 
A config file field is added to control if per-transaction isolation
level is actually used. If set to true, nested transactions will throw
a runtime exception as the enclosing transaction may run at a different
isolation level.

In this PR we only add the ability to specify the isolation level,
without enabling it in any environment (including unit tests), or
actually specifying one for any query. This should allow us to set up
the system without impacting anything currently working.
This commit is contained in:
Lai Jiang
2023-08-21 18:48:34 -04:00
committed by GitHub
parent cfcafeefc6
commit f59c387b9c
14 changed files with 345 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/test/java/google/registry/flows/FlowRunnerTest.java
+37 -4
View File
@@ -17,6 +17,7 @@ package google.registry.flows;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.common.truth.Truth8.assertThat;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.testing.TestDataHelper.loadFile;
import static google.registry.testing.TestLogHandlerUtils.findFirstLogMessageByPrefix;
import static google.registry.util.DateTimeUtils.START_OF_TIME;
@@ -30,11 +31,13 @@ import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.testing.TestLogHandler;
import google.registry.config.RegistryConfig;
import google.registry.flows.certs.CertificateChecker;
import google.registry.model.eppcommon.Trid;
import google.registry.model.eppoutput.EppOutput.ResponseOrGreeting;
import google.registry.model.eppoutput.EppResponse;
import google.registry.monitoring.whitebox.EppMetric;
import google.registry.persistence.PersistenceModule.TransactionIsolationLevel;
import google.registry.persistence.transaction.JpaTestExtensions;
import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension;
import google.registry.testing.FakeClock;
@@ -46,7 +49,6 @@ import org.joda.time.DateTime;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.mockito.Mockito;
/** Unit tests for {@link FlowRunner}. */
class FlowRunnerTest {
@@ -78,6 +80,21 @@ class FlowRunnerTest {
}
}
static class TestTransactionalFlow implements TransactionalFlow {
private final Optional<TransactionIsolationLevel> isolationLevel;
TestTransactionalFlow(Optional<TransactionIsolationLevel> isolationLevel) {
this.isolationLevel = isolationLevel;
}
@Override
public ResponseOrGreeting run() {
tm().assertTransactionIsolationLevel(
isolationLevel.orElse(tm().getDefaultTransactionIsolationLevel()));
return mock(EppResponse.class);
}
}
@BeforeEach
void beforeEach() {
JdkLoggerConfig.getConfig(FlowRunner.class).addHandler(handler);
@@ -90,23 +107,39 @@ class FlowRunnerTest {
flowRunner.isDryRun = false;
flowRunner.isSuperuser = false;
flowRunner.isTransactional = false;
flowRunner.isolationLevelOverride = Optional.empty();
flowRunner.sessionMetadata =
new StatelessRequestSessionMetadata("TheRegistrar", ImmutableSet.of());
flowRunner.trid = Trid.create("client-123", "server-456");
flowRunner.flowReporter = Mockito.mock(FlowReporter.class);
flowRunner.flowReporter = mock(FlowReporter.class);
}
@Test
void testRun_nonTransactionalCommand_setsCommandNameOnMetric() throws Exception {
flowRunner.isTransactional = true;
flowRunner.run(eppMetricBuilder);
assertThat(eppMetricBuilder.build().getCommandName()).hasValue("TestCommand");
}
@Test
void testRun_transactionalCommand_setsCommandNameOnMetric() throws Exception {
flowRunner.isTransactional = true;
flowRunner.flowClass = TestTransactionalFlow.class;
flowRunner.flowProvider = () -> new TestTransactionalFlow(Optional.empty());
flowRunner.run(eppMetricBuilder);
assertThat(eppMetricBuilder.build().getCommandName()).hasValue("TestCommand");
assertThat(eppMetricBuilder.build().getCommandName()).hasValue("TestTransactional");
}
@Test
void testRun_transactionalCommand_isolationLevelOverride() throws Exception {
flowRunner.isTransactional = true;
flowRunner.isolationLevelOverride =
Optional.of(TransactionIsolationLevel.TRANSACTION_READ_UNCOMMITTED);
flowRunner.flowClass = TestTransactionalFlow.class;
flowRunner.flowProvider = () -> new TestTransactionalFlow(flowRunner.isolationLevelOverride);
if (RegistryConfig.getHibernatePerTransactionIsolationEnabled()) {
flowRunner.run(eppMetricBuilder);
assertThat(eppMetricBuilder.build().getCommandName()).hasValue("TestTransactional");
}
}
@Test
core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerImplTest.java
+60 -10
View File
@@ -16,6 +16,8 @@ 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_UNCOMMITTED;
import static google.registry.persistence.PersistenceModule.TransactionIsolationLevel.TRANSACTION_REPEATABLE_READ;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.testing.DatabaseHelper.assertDetachedFromEntityManager;
import static google.registry.testing.DatabaseHelper.existsInDb;
@@ -31,6 +33,7 @@ import static org.mockito.Mockito.verify;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import google.registry.config.RegistryConfig;
import google.registry.model.ImmutableObject;
import google.registry.persistence.VKey;
import google.registry.persistence.transaction.JpaTestExtensions.JpaUnitTestExtension;
@@ -81,7 +84,7 @@ class JpaTransactionManagerImplTest {
.buildUnitTestExtension();
@Test
void transact_succeeds() {
void transact_success() {
assertPersonEmpty();
assertCompanyEmpty();
tm().transact(
@@ -89,6 +92,7 @@ class JpaTransactionManagerImplTest {
insertPerson(10);
insertCompany("Foo");
insertCompany("Bar");
tm().assertTransactionIsolationLevel(tm().getDefaultTransactionIsolationLevel());
});
assertPersonCount(1);
assertPersonExist(10);
@@ -97,6 +101,52 @@ class JpaTransactionManagerImplTest {
assertCompanyExist("Bar");
}
@Test
void transact_setIsolationLevel() {
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(
RegistryConfig.getHibernatePerTransactionIsolationEnabled()
? TRANSACTION_READ_UNCOMMITTED
: tm().getDefaultTransactionIsolationLevel());
return null;
},
TRANSACTION_READ_UNCOMMITTED);
// Make sure that we can start a new transaction on the same thread with a different isolation
// level.
tm().transact(
() -> {
tm().assertTransactionIsolationLevel(
RegistryConfig.getHibernatePerTransactionIsolationEnabled()
? TRANSACTION_REPEATABLE_READ
: tm().getDefaultTransactionIsolationLevel());
return null;
},
TRANSACTION_REPEATABLE_READ);
}
@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 =
assertThrows(
IllegalArgumentException.class,
() ->
tm().transact(
() -> {
tm().transact(() -> {});
}));
assertThat(e).hasMessageThat().isEqualTo("Nested transaction detected");
} else {
tm().transact(
() -> {
tm().transact(() -> {});
});
}
}
@Test
void transact_hasNoEffectWithPartialSuccess() {
assertPersonEmpty();
@@ -606,19 +656,19 @@ class JpaTransactionManagerImplTest {
verify(spyJpaTm, times(3)).delete(theEntityKey);
}
private void insertPerson(int age) {
private static void insertPerson(int age) {
tm().getEntityManager()
.createNativeQuery(String.format("INSERT INTO Person (age) VALUES (%d)", age))
.executeUpdate();
}
private void insertCompany(String name) {
private static void insertCompany(String name) {
tm().getEntityManager()
.createNativeQuery(String.format("INSERT INTO Company (name) VALUES ('%s')", name))
.executeUpdate();
}
private void assertPersonExist(int age) {
private static void assertPersonExist(int age) {
tm().transact(
() -> {
EntityManager em = tm().getEntityManager();
@@ -631,7 +681,7 @@ class JpaTransactionManagerImplTest {
});
}
private void assertCompanyExist(String name) {
private static void assertCompanyExist(String name) {
tm().transact(
() -> {
String maybeName =
@@ -644,23 +694,23 @@ class JpaTransactionManagerImplTest {
});
}
private void assertPersonCount(int count) {
private static void assertPersonCount(int count) {
assertThat(countTable("Person")).isEqualTo(count);
}
private void assertCompanyCount(int count) {
private static void assertCompanyCount(int count) {
assertThat(countTable("Company")).isEqualTo(count);
}
private void assertPersonEmpty() {
private static void assertPersonEmpty() {
assertPersonCount(0);
}
private void assertCompanyEmpty() {
private static void assertCompanyEmpty() {
assertCompanyCount(0);
}
private int countTable(String tableName) {
private static int countTable(String tableName) {
return tm().transact(
() -> {
BigInteger colCount =
core/src/test/java/google/registry/persistence/transaction/ReplicaSimulatingJpaTransactionManager.java
+44 -6
View File
@@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import google.registry.model.ImmutableObject;
import google.registry.persistence.PersistenceModule.TransactionIsolationLevel;
import google.registry.persistence.VKey;
import java.util.Optional;
import java.util.function.Supplier;
@@ -49,6 +50,21 @@ public class ReplicaSimulatingJpaTransactionManager implements JpaTransactionMan
delegate.teardown();
}
@Override
public TransactionIsolationLevel getDefaultTransactionIsolationLevel() {
return delegate.getDefaultTransactionIsolationLevel();
}
@Override
public TransactionIsolationLevel getCurrentTransactionIsolationLevel() {
return delegate.getCurrentTransactionIsolationLevel();
}
@Override
public void assertTransactionIsolationLevel(TransactionIsolationLevel expectedLevel) {
delegate.assertTransactionIsolationLevel(expectedLevel);
}
@Override
public EntityManager getStandaloneEntityManager() {
return delegate.getStandaloneEntityManager();
@@ -85,7 +101,7 @@ public class ReplicaSimulatingJpaTransactionManager implements JpaTransactionMan
}
@Override
public <T> T transact(Supplier<T> work) {
public <T> T transact(Supplier<T> work, TransactionIsolationLevel isolationLevel) {
if (delegate.inTransaction()) {
return work.get();
}
@@ -96,26 +112,48 @@ public class ReplicaSimulatingJpaTransactionManager implements JpaTransactionMan
.createNativeQuery("SET TRANSACTION READ ONLY")
.executeUpdate();
return work.get();
});
},
isolationLevel);
}
@Override
public <T> T transact(Supplier<T> work) {
return transact(work, null);
}
@Override
public <T> T transactNoRetry(Supplier<T> work, TransactionIsolationLevel isolationLevel) {
return transact(work, isolationLevel);
}
@Override
public <T> T transactNoRetry(Supplier<T> work) {
return transact(work);
return transactNoRetry(work, null);
}
@Override
public void transact(Runnable work) {
public void transact(Runnable work, TransactionIsolationLevel isolationLevel) {
transact(
() -> {
work.run();
return null;
});
},
isolationLevel);
}
@Override
public void transact(Runnable work) {
transact(work, null);
}
@Override
public void transactNoRetry(Runnable work, TransactionIsolationLevel isolationLevel) {
transact(work, isolationLevel);
}
@Override
public void transactNoRetry(Runnable work) {
transact(work);
transactNoRetry(work, null);
}
@Override