Add dual read claims list (#413)

* Add dual read claims list

* Improve warning log and use longer duration for cache

* Extract the comparison logic to a method

* Move cache to DAO

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 <T> Supplier<T> memoizeWithShortExpiration(Supplier<T> 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 <T> Supplier<T> tryMemoizeWithExpiration(
+      Duration expiration, Supplier<T> original) {
     return expiration.isEqual(ZERO)
         ? original
         : memoizeWithExpiration(original, expiration.getMillis(), MILLISECONDS);

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()
+                        tm().transactNewReadOnly(
-                            .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<ClaimsList> maybeCloudSqlList = ClaimsListDao.getLatestRevision();
+    if (maybeCloudSqlList.isPresent()) {
+      ClaimsList cloudSqlList = maybeCloudSqlList.get();
+      MapDifference<String, String> 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<String> 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.

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<Optional<ClaimsList>> 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
+   * Returns the most recent revision of the {@link ClaimsList} in Cloud SQL, if it exists.
-   * is no claims in the table.
+   * 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<ClaimsList> 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<ClaimsList> getLatestRevisionCached() {
+    return cacheClaimsList.get();
+  }
+
   private ClaimsListDao() {}
 }

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() {
+  public void getCurrent_returnsEmptyListIfTableIsEmpty() {
-    assertThrows(NoResultException.class, ClaimsListDao::getCurrent);
+    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) {