diff --git a/core/src/main/java/google/registry/model/ofy/CommitLogMutation.java b/core/src/main/java/google/registry/model/ofy/CommitLogMutation.java index 65ba03823..87df17d7e 100644 --- a/core/src/main/java/google/registry/model/ofy/CommitLogMutation.java +++ b/core/src/main/java/google/registry/model/ofy/CommitLogMutation.java @@ -69,7 +69,7 @@ public class CommitLogMutation extends ImmutableObject implements DatastoreOnlyE * converted to a raw Datastore Entity, serialized to bytes, and stored within the mutation. */ public static CommitLogMutation create(Key parent, Object entity) { - return createFromRaw(parent, auditedOfy().save().toEntity(entity)); + return createFromRaw(parent, auditedOfy().saveIgnoringReadOnlyWithBackup().toEntity(entity)); } /** diff --git a/core/src/main/java/google/registry/model/ofy/CommitLoggedWork.java b/core/src/main/java/google/registry/model/ofy/CommitLoggedWork.java index e3a1e1319..59be67df0 100644 --- a/core/src/main/java/google/registry/model/ofy/CommitLoggedWork.java +++ b/core/src/main/java/google/registry/model/ofy/CommitLoggedWork.java @@ -156,7 +156,7 @@ public class CommitLoggedWork implements Runnable { .map(entity -> (ImmutableObject) CommitLogMutation.create(manifestKey, entity)) .collect(toImmutableSet()); auditedOfy() - .saveWithoutBackup() + .saveIgnoringReadOnlyWithoutBackup() .entities( new ImmutableSet.Builder<>() .add(manifest) diff --git a/core/src/main/java/google/registry/persistence/transaction/TransactionManagerFactory.java b/core/src/main/java/google/registry/persistence/transaction/TransactionManagerFactory.java index 41572a105..215e6bf67 100644 --- a/core/src/main/java/google/registry/persistence/transaction/TransactionManagerFactory.java +++ b/core/src/main/java/google/registry/persistence/transaction/TransactionManagerFactory.java @@ -17,7 +17,6 @@ package google.registry.persistence.transaction; import static com.google.common.base.Preconditions.checkState; import static google.registry.model.common.DatabaseMigrationStateSchedule.MigrationState.DATASTORE_PRIMARY_NO_ASYNC; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; -import static org.joda.time.DateTimeZone.UTC; import com.google.appengine.api.utils.SystemProperty; import com.google.appengine.api.utils.SystemProperty.Environment.Value; @@ -30,10 +29,11 @@ import google.registry.model.common.DatabaseMigrationStateSchedule.PrimaryDataba import google.registry.model.ofy.DatastoreTransactionManager; import google.registry.persistence.DaggerPersistenceComponent; import google.registry.tools.RegistryToolEnvironment; +import google.registry.util.Clock; import google.registry.util.NonFinalForTesting; +import google.registry.util.SystemClock; import java.util.Optional; import java.util.function.Supplier; -import org.joda.time.DateTime; /** Factory class to create {@link TransactionManager} instance. */ // TODO: Rename this to PersistenceFactory and move to persistence package. @@ -44,6 +44,9 @@ public final class TransactionManagerFactory { /** Optional override to manually set the transaction manager per-test. */ private static Optional tmForTest = Optional.empty(); + /** The current clock (defined as a variable so we can override it in tests) */ + private static Clock clock = new SystemClock(); + /** Supplier for jpaTm so that it is initialized only once, upon first usage. */ @NonFinalForTesting private static Supplier jpaTm = @@ -105,7 +108,7 @@ public final class TransactionManagerFactory { if (onBeam) { return jpaTm(); } - return DatabaseMigrationStateSchedule.getValueAtTime(DateTime.now(UTC)) + return DatabaseMigrationStateSchedule.getValueAtTime(clock.nowUtc()) .getPrimaryDatabase() .equals(PrimaryDatabase.DATASTORE) ? ofyTm() @@ -195,7 +198,7 @@ public final class TransactionManagerFactory { } public static void assertNotReadOnlyMode() { - if (DatabaseMigrationStateSchedule.getValueAtTime(DateTime.now(UTC)).isReadOnly()) { + if (DatabaseMigrationStateSchedule.getValueAtTime(clock.nowUtc()).isReadOnly()) { throw new ReadOnlyModeException(); } } @@ -210,12 +213,18 @@ public final class TransactionManagerFactory { */ @DeleteAfterMigration public static void assertAsyncActionsAreAllowed() { - if (DatabaseMigrationStateSchedule.getValueAtTime(DateTime.now(UTC)) + if (DatabaseMigrationStateSchedule.getValueAtTime(clock.nowUtc()) .equals(DATASTORE_PRIMARY_NO_ASYNC)) { throw new ReadOnlyModeException(); } } + /** Allows us to set the clock used by the factory in unit tests. */ + @VisibleForTesting + public static void setClockForTesting(Clock clock) { + TransactionManagerFactory.clock = clock; + } + /** Registry is currently undergoing maintenance and is in read-only mode. */ public static class ReadOnlyModeException extends IllegalStateException { public ReadOnlyModeException() { diff --git a/core/src/test/java/google/registry/model/replay/ReplicateToDatastoreActionTest.java b/core/src/test/java/google/registry/model/replay/ReplicateToDatastoreActionTest.java index 509ce63da..208c1e19d 100644 --- a/core/src/test/java/google/registry/model/replay/ReplicateToDatastoreActionTest.java +++ b/core/src/test/java/google/registry/model/replay/ReplicateToDatastoreActionTest.java @@ -44,6 +44,7 @@ import google.registry.model.server.Lock; import google.registry.persistence.VKey; import google.registry.persistence.transaction.JpaTransactionManagerImpl; import google.registry.persistence.transaction.TransactionEntity; +import google.registry.persistence.transaction.TransactionManagerFactory; import google.registry.testing.AppEngineExtension; import google.registry.testing.DatabaseHelper; import google.registry.testing.FakeClock; @@ -52,6 +53,7 @@ import google.registry.testing.InjectExtension; import google.registry.testing.ReplayExtension; import google.registry.testing.TestObject; import google.registry.util.RequestStatusChecker; +import google.registry.util.SystemClock; import java.lang.reflect.Proxy; import java.util.List; import java.util.logging.Level; @@ -105,6 +107,7 @@ public class ReplicateToDatastoreActionTest { void tearDown() { DatabaseHelper.removeDatabaseMigrationSchedule(); fakeClock.disableAutoIncrement(); + TransactionManagerFactory.setClockForTesting(new SystemClock()); } @RetryingTest(4) @@ -392,6 +395,50 @@ public class ReplicateToDatastoreActionTest { + " DATASTORE_PRIMARY."); } + @Test + void replicationWorksInReadOnly() { + + // Put us in SQL primary now, readonly in an hour, then in datastore primary after 25 hours. + // And we'll need the TransactionManagerFactory to use the fake clock. + DateTime now = fakeClock.nowUtc(); + TransactionManagerFactory.setClockForTesting(fakeClock); + jpaTm() + .transact( + () -> + DatabaseMigrationStateSchedule.set( + ImmutableSortedMap.naturalOrder() + .put(START_OF_TIME, MigrationState.DATASTORE_ONLY) + .put(START_OF_TIME.plusHours(1), MigrationState.DATASTORE_PRIMARY) + .put(START_OF_TIME.plusHours(2), MigrationState.DATASTORE_PRIMARY_NO_ASYNC) + .put(START_OF_TIME.plusHours(3), MigrationState.DATASTORE_PRIMARY_READ_ONLY) + .put(START_OF_TIME.plusHours(4), MigrationState.SQL_PRIMARY_READ_ONLY) + .put(START_OF_TIME.plusHours(5), MigrationState.SQL_PRIMARY) + .put(now.plusHours(1), MigrationState.SQL_PRIMARY_READ_ONLY) + .put(now.plusHours(25), MigrationState.DATASTORE_PRIMARY_READ_ONLY) + .put(now.plusHours(26), MigrationState.DATASTORE_PRIMARY_READ_ONLY) + .build())); + + TestObject foo = TestObject.create("foo"); + insertInDb(foo); + TestObject bar = TestObject.create("bar"); + insertInDb(bar); + TestObject baz = TestObject.create("baz"); + insertInDb(baz); + jpaTm().transact(() -> jpaTm().delete(baz.key())); + + // get to read-only + fakeClock.advanceBy(Duration.standardDays(1)); + + // process the transaction in readonly. + action.run(); + + // Forward the next day (datastore primary). Verify that datastore has all of the changes. + fakeClock.advanceBy(Duration.standardDays(1)); + assertThat(ofyTm().transact(() -> ofyTm().loadByKey(foo.key()))).isEqualTo(foo); + assertThat(ofyTm().transact(() -> ofyTm().loadByKey(bar.key()))).isEqualTo(bar); + assertThat(ofyTm().transact(() -> ofyTm().loadByKeyIfPresent(baz.key()).isPresent())).isFalse(); + } + @Test void testFailure_cannotAcquireLock() { assumeTrue(ReplayExtension.replayTestsEnabled());