diff --git a/core/src/main/java/google/registry/model/CacheUtils.java b/core/src/main/java/google/registry/model/CacheUtils.java index 8ea1e6333..dad662629 100644 --- a/core/src/main/java/google/registry/model/CacheUtils.java +++ b/core/src/main/java/google/registry/model/CacheUtils.java @@ -32,7 +32,15 @@ public class CacheUtils { * lists downloaded from the TMCH get updated in Datastore and the caches need to be refreshed.) */ public static Supplier memoizeWithShortExpiration(Supplier original) { - Duration expiration = getSingletonCacheRefreshDuration(); + return tryMemoizeWithExpiration(getSingletonCacheRefreshDuration(), original); + } + + /** + * Memoize a supplier with the given expiration. If the expiration is zero(likely happens in a + * unit test), it returns the original supplier. + */ + public static Supplier tryMemoizeWithExpiration( + Duration expiration, Supplier original) { return expiration.isEqual(ZERO) ? original : memoizeWithExpiration(original, expiration.getMillis(), MILLISECONDS); diff --git a/core/src/main/java/google/registry/model/tmch/ClaimsListShard.java b/core/src/main/java/google/registry/model/tmch/ClaimsListShard.java index 46cdcaf32..00cb8f70d 100644 --- a/core/src/main/java/google/registry/model/tmch/ClaimsListShard.java +++ b/core/src/main/java/google/registry/model/tmch/ClaimsListShard.java @@ -27,6 +27,10 @@ import static google.registry.util.DateTimeUtils.START_OF_TIME; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.MapDifference; +import com.google.common.collect.MapDifference.ValueDifference; +import com.google.common.collect.Maps; +import com.google.common.flogger.FluentLogger; import com.google.common.util.concurrent.UncheckedExecutionException; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.EmbedMap; @@ -40,6 +44,8 @@ import google.registry.model.annotations.NotBackedUp; import google.registry.model.annotations.NotBackedUp.Reason; import google.registry.model.annotations.VirtualEntity; import google.registry.model.common.CrossTldSingleton; +import google.registry.schema.tmch.ClaimsList; +import google.registry.schema.tmch.ClaimsListDao; import google.registry.util.CollectionUtils; import google.registry.util.Concurrent; import google.registry.util.Retrier; @@ -71,6 +77,8 @@ import org.joda.time.DateTime; @NotBackedUp(reason = Reason.EXTERNALLY_SOURCED) public class ClaimsListShard extends ImmutableObject { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + /** The number of claims list entries to store per shard. */ private static final int SHARD_SIZE = 10000; @@ -112,8 +120,7 @@ public class ClaimsListShard extends ImmutableObject { Concurrent.transform( shardKeys, key -> - tm() - .transactNewReadOnly( + tm().transactNewReadOnly( () -> { ClaimsListShard claimsListShard = ofy().load().key(key).now(); checkState( @@ -142,9 +149,52 @@ public class ClaimsListShard extends ImmutableObject { } } } - return create(creationTime, ImmutableMap.copyOf(combinedLabelsToKeys)); + + ClaimsListShard datastoreList = + create(creationTime, ImmutableMap.copyOf(combinedLabelsToKeys)); + // Also load the list from Cloud SQL, compare the two lists, and log if different. + try { + loadAndCompareCloudSqlList(datastoreList); + } catch (Throwable t) { + logger.atSevere().withCause(t).log("Error comparing reserved lists."); + } + return datastoreList; }; + private static final void loadAndCompareCloudSqlList(ClaimsListShard datastoreList) { + Optional maybeCloudSqlList = ClaimsListDao.getLatestRevision(); + if (maybeCloudSqlList.isPresent()) { + ClaimsList cloudSqlList = maybeCloudSqlList.get(); + MapDifference diff = + Maps.difference(datastoreList.labelsToKeys, cloudSqlList.getLabelsToKeys()); + if (!diff.areEqual()) { + if (diff.entriesDiffering().size() > 10) { + logger.atWarning().log( + String.format( + "Unequal claims lists detected, Cloud SQL list with revision id %d has %d" + + " different records than the current Datastore list.", + cloudSqlList.getRevisionId(), diff.entriesDiffering().size())); + } else { + StringBuilder diffMessage = new StringBuilder("Unequal claims lists detected:\n"); + diff.entriesDiffering().entrySet().stream() + .forEach( + entry -> { + String label = entry.getKey(); + ValueDifference valueDiff = entry.getValue(); + diffMessage.append( + String.format( + "Domain label %s has key %s in Datastore and key %s in Cloud" + + " SQL.\n", + label, valueDiff.leftValue(), valueDiff.rightValue())); + }); + logger.atWarning().log(diffMessage.toString()); + } + } + } else { + logger.atWarning().log("Claims list in Cloud SQL is empty."); + } + } + /** * A cached supplier that fetches the claims list shards from Datastore and recombines them into a * single {@link ClaimsListShard} object. diff --git a/core/src/main/java/google/registry/schema/tmch/ClaimsListDao.java b/core/src/main/java/google/registry/schema/tmch/ClaimsListDao.java index ae9b520e4..2ff809a83 100644 --- a/core/src/main/java/google/registry/schema/tmch/ClaimsListDao.java +++ b/core/src/main/java/google/registry/schema/tmch/ClaimsListDao.java @@ -14,9 +14,14 @@ package google.registry.schema.tmch; +import static google.registry.config.RegistryConfig.getDomainLabelListCacheDuration; +import static google.registry.model.CacheUtils.tryMemoizeWithExpiration; import static google.registry.model.transaction.TransactionManagerFactory.jpaTm; +import com.google.common.base.Supplier; import com.google.common.flogger.FluentLogger; +import google.registry.util.NonFinalForTesting; +import java.util.Optional; import javax.persistence.EntityManager; /** Data access object for {@link ClaimsList}. */ @@ -24,6 +29,11 @@ public class ClaimsListDao { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + /** In-memory cache for claims list. */ + @NonFinalForTesting + private static Supplier> cacheClaimsList = + tryMemoizeWithExpiration(getDomainLabelListCacheDuration(), ClaimsListDao::getLatestRevision); + private static void save(ClaimsList claimsList) { jpaTm().transact(() -> jpaTm().getEntityManager().persist(claimsList)); } @@ -47,10 +57,12 @@ public class ClaimsListDao { } /** - * Returns the current revision of the {@link ClaimsList} in Cloud SQL. Throws exception if there - * is no claims in the table. + * Returns the most recent revision of the {@link ClaimsList} in Cloud SQL, if it exists. + * TODO(shicong): Change this method to package level access after dual-read phase. + * ClaimsListShard uses this method to retrieve claims list in Cloud SQL for the comparison, and + * ClaimsListShard is not in this package. */ - public static ClaimsList getCurrent() { + public static Optional getLatestRevision() { return jpaTm() .transact( () -> { @@ -63,9 +75,15 @@ public class ClaimsListDao { + " :revisionId", ClaimsList.class) .setParameter("revisionId", revisionId) - .getSingleResult(); + .getResultStream() + .findFirst(); }); } + /** Returns the most recent revision of the {@link ClaimsList}, from cache. */ + public static Optional getLatestRevisionCached() { + return cacheClaimsList.get(); + } + private ClaimsListDao() {} } diff --git a/core/src/test/java/google/registry/schema/tmch/ClaimsListDaoTest.java b/core/src/test/java/google/registry/schema/tmch/ClaimsListDaoTest.java index 2509ba356..1fb986383 100644 --- a/core/src/test/java/google/registry/schema/tmch/ClaimsListDaoTest.java +++ b/core/src/test/java/google/registry/schema/tmch/ClaimsListDaoTest.java @@ -15,13 +15,11 @@ package google.registry.schema.tmch; import static com.google.common.truth.Truth.assertThat; -import static google.registry.testing.JUnitBackports.assertThrows; import com.google.common.collect.ImmutableMap; import google.registry.model.transaction.JpaTestRules; import google.registry.model.transaction.JpaTestRules.JpaIntegrationTestRule; import google.registry.testing.FakeClock; -import javax.persistence.NoResultException; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -42,7 +40,7 @@ public class ClaimsListDaoTest { ClaimsList claimsList = ClaimsList.create(fakeClock.nowUtc(), ImmutableMap.of("label1", "key1", "label2", "key2")); ClaimsListDao.trySave(claimsList); - ClaimsList insertedClaimsList = ClaimsListDao.getCurrent(); + ClaimsList insertedClaimsList = ClaimsListDao.getLatestRevision().get(); assertClaimsListEquals(claimsList, insertedClaimsList); assertThat(insertedClaimsList.getCreationTimestamp()).isEqualTo(jpaRule.getTxnClock().nowUtc()); } @@ -52,7 +50,7 @@ public class ClaimsListDaoTest { ClaimsList claimsList = ClaimsList.create(fakeClock.nowUtc(), ImmutableMap.of("label1", "key1", "label2", "key2")); ClaimsListDao.trySave(claimsList); - ClaimsList insertedClaimsList = ClaimsListDao.getCurrent(); + ClaimsList insertedClaimsList = ClaimsListDao.getLatestRevision().get(); assertClaimsListEquals(claimsList, insertedClaimsList); // Save ClaimsList with existing revisionId should fail because revisionId is the primary key. ClaimsListDao.trySave(insertedClaimsList); @@ -62,14 +60,14 @@ public class ClaimsListDaoTest { public void trySave_claimsListWithNoEntries() { ClaimsList claimsList = ClaimsList.create(fakeClock.nowUtc(), ImmutableMap.of()); ClaimsListDao.trySave(claimsList); - ClaimsList insertedClaimsList = ClaimsListDao.getCurrent(); + ClaimsList insertedClaimsList = ClaimsListDao.getLatestRevision().get(); assertClaimsListEquals(claimsList, insertedClaimsList); assertThat(insertedClaimsList.getLabelsToKeys()).isEmpty(); } @Test - public void getCurrent_throwsNoResultExceptionIfTableIsEmpty() { - assertThrows(NoResultException.class, ClaimsListDao::getCurrent); + public void getCurrent_returnsEmptyListIfTableIsEmpty() { + assertThat(ClaimsListDao.getLatestRevision().isPresent()).isFalse(); } @Test @@ -80,7 +78,7 @@ public class ClaimsListDaoTest { ClaimsList.create(fakeClock.nowUtc(), ImmutableMap.of("label3", "key3", "label4", "key4")); ClaimsListDao.trySave(oldClaimsList); ClaimsListDao.trySave(newClaimsList); - assertClaimsListEquals(newClaimsList, ClaimsListDao.getCurrent()); + assertClaimsListEquals(newClaimsList, ClaimsListDao.getLatestRevision().get()); } private void assertClaimsListEquals(ClaimsList left, ClaimsList right) {