Allow number of commit log buckets to be increased

Also increases the number of commit log buckets on alpha to 397 and correspondingly
reduces the frequency of commit log diff exporting to once every 3 minutes.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=191440586

java/google/registry/backup/ExportCommitLogDiffAction.java

@@ -14,6 +14,7 @@
 
 package google.registry.backup;
 
+import static com.google.common.base.MoreObjects.firstNonNull;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Verify.verifyNotNull;
 import static com.google.common.collect.ImmutableList.toImmutableList;
@@ -175,10 +176,13 @@ public final class ExportCommitLogDiffAction implements Runnable {
       @Nullable CommitLogCheckpoint lowerCheckpoint,
       CommitLogCheckpoint upperCheckpoint,
       int bucketNum) {
-    // If no lower checkpoint exists, use START_OF_TIME as the effective exclusive lower bound.
+    // If no lower checkpoint exists, or if it exists but had no timestamp for this bucket number
-    DateTime lowerCheckpointBucketTime = lowerCheckpoint == null
+    // (because the bucket count was increased between these checkpoints), then use START_OF_TIME
-        ? START_OF_TIME
+    // as the effective exclusive lower bound.
-        : lowerCheckpoint.getBucketTimestamps().get(bucketNum);
+    DateTime lowerCheckpointBucketTime =
+        firstNonNull(
+            (lowerCheckpoint == null) ? null : lowerCheckpoint.getBucketTimestamps().get(bucketNum),
+            START_OF_TIME);
     // Since START_OF_TIME=0 is not a valid id in a key, add 1 to both bounds. Then instead of
     // loading lowerBound < x <= upperBound, we can load lowerBound <= x < upperBound.
     DateTime lowerBound = lowerCheckpointBucketTime.plusMillis(1);

java/google/registry/env/alpha/default/WEB-INF/cron.xml

@@ -128,7 +128,7 @@
     <description>
       This job checkpoints the commit log buckets and exports the diff since last checkpoint to GCS.
     </description>
-    <schedule>every 1 minutes synchronized</schedule>
+    <schedule>every 3 minutes synchronized</schedule>
     <target>backend</target>
   </cron>
 

java/google/registry/model/ofy/CommitLogCheckpoint.java

@@ -17,6 +17,7 @@ package google.registry.model.ofy;
 import static com.google.common.base.Preconditions.checkArgument;
 import static org.joda.time.DateTimeZone.UTC;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.googlecode.objectify.Key;
@@ -71,12 +72,11 @@ public class CommitLogCheckpoint extends ImmutableObject {
   }
 
   /**
-   * Creates a CommitLogCheckpoint for the given wall time and bucket checkpoint times, specified
+   * Creates a CommitLogCheckpoint for the given wall time and bucket checkpoint times, specified as
-   * as a map from bucket ID to bucket commit timestamp.
+   * a map from bucket ID to bucket commit timestamp.
    */
   public static CommitLogCheckpoint create(
-      DateTime checkpointTime,
+      DateTime checkpointTime, ImmutableMap<Integer, DateTime> bucketTimestamps) {
-      ImmutableMap<Integer, DateTime> bucketTimestamps) {
     checkArgument(
         Objects.equals(CommitLogBucket.getBucketIds().asList(), bucketTimestamps.keySet().asList()),
         "Bucket ids are incorrect: %s",
@@ -87,6 +87,20 @@ public class CommitLogCheckpoint extends ImmutableObject {
     return instance;
   }
 
+  /**
+   * Creates a CommitLogCheckpoint for the given wall time and bucket checkpoint times. Test only.
+   *
+   * <p>This lacks validation on the bucketTimestamps map.
+   */
+  @VisibleForTesting
+  public static CommitLogCheckpoint createForTest(
+      DateTime checkpointTime, ImmutableMap<Integer, DateTime> bucketTimestamps) {
+    CommitLogCheckpoint instance = new CommitLogCheckpoint();
+    instance.checkpointTime = checkpointTime.getMillis();
+    instance.bucketTimestamps = ImmutableList.copyOf(bucketTimestamps.values());
+    return instance;
+  }
+
   /** Creates a key for the CommitLogCheckpoint for the given wall time. */
   public static Key<CommitLogCheckpoint> createKey(DateTime checkpointTime) {
     return Key.create(

javatests/google/registry/backup/ExportCommitLogDiffActionTest.java

@@ -319,6 +319,88 @@ public class ExportCommitLogDiffActionTest {
     assertThat(exported).containsExactly(upperCheckpoint);
   }
 
+  @Test
+  public void testRun_checkpointDiffWithNonExistentBucketTimestamps_exportsCorrectly()
+      throws Exception {
+    // Non-existent bucket timestamps can exist when the commit log bucket count was increased
+    // recently.
+
+    task.lowerCheckpointTime = oneMinuteAgo;
+    task.upperCheckpointTime = now;
+
+    // No lower checkpoint times are persisted for buckets 2 and 3 (simulating a recent increase in
+    // the number of commit log buckets from 1 to 3), so all mutations on buckets 2 and 3, even
+    // those older than the lower checkpoint, will be exported.
+    persistResource(
+        CommitLogCheckpoint.createForTest(oneMinuteAgo, ImmutableMap.of(1, oneMinuteAgo)));
+    CommitLogCheckpoint upperCheckpoint =
+        persistResource(
+            CommitLogCheckpoint.create(
+                now,
+                ImmutableMap.of(
+                    1, now,
+                    2, now.minusDays(1),
+                    3, oneMinuteAgo.minusDays(2))));
+
+    // These shouldn't be in the diff because the lower bound is exclusive.
+    persistManifestAndMutation(1, oneMinuteAgo);
+    // These shouldn't be in the diff because they are above the upper bound.
+    persistManifestAndMutation(1, now.plusMillis(1));
+    persistManifestAndMutation(2, now.minusDays(1).plusMillis(1));
+    persistManifestAndMutation(3, oneMinuteAgo.minusDays(2).plusMillis(1));
+    // These should be in the diff because they happened after START_OF_TIME on buckets with
+    // non-existent timestamps.
+    persistManifestAndMutation(2, oneMinuteAgo.minusDays(1));
+    persistManifestAndMutation(3, oneMinuteAgo.minusDays(2));
+    // These should be in the diff because they are between the bounds.
+    persistManifestAndMutation(1, now.minusMillis(1));
+    persistManifestAndMutation(2, now.minusDays(1).minusMillis(1));
+    // These should be in the diff because they are at the upper bound.
+    persistManifestAndMutation(1, now);
+    persistManifestAndMutation(2, now.minusDays(1));
+
+    task.run();
+
+    GcsFilename expectedFilename = new GcsFilename("gcs bucket", "commit_diff_until_" + now);
+    assertWithMessage("GCS file not found: " + expectedFilename)
+        .that(gcsService.getMetadata(expectedFilename))
+        .isNotNull();
+    assertThat(gcsService.getMetadata(expectedFilename).getOptions().getUserMetadata())
+        .containsExactly(
+            LOWER_BOUND_CHECKPOINT,
+            oneMinuteAgo.toString(),
+            UPPER_BOUND_CHECKPOINT,
+            now.toString(),
+            NUM_TRANSACTIONS,
+            "6");
+    List<ImmutableObject> exported =
+        deserializeEntities(GcsTestingUtils.readGcsFile(gcsService, expectedFilename));
+    assertThat(exported.get(0)).isEqualTo(upperCheckpoint);
+    // We expect these manifests, in time order, with matching mutations.
+    CommitLogManifest manifest1 = createManifest(3, oneMinuteAgo.minusDays(2));
+    CommitLogManifest manifest2 = createManifest(2, oneMinuteAgo.minusDays(1));
+    CommitLogManifest manifest3 = createManifest(2, now.minusDays(1).minusMillis(1));
+    CommitLogManifest manifest4 = createManifest(2, now.minusDays(1));
+    CommitLogManifest manifest5 = createManifest(1, now.minusMillis(1));
+    CommitLogManifest manifest6 = createManifest(1, now);
+    assertThat(exported)
+        .containsExactly(
+            upperCheckpoint,
+            manifest1,
+            createMutation(manifest1),
+            manifest2,
+            createMutation(manifest2),
+            manifest3,
+            createMutation(manifest3),
+            manifest4,
+            createMutation(manifest4),
+            manifest5,
+            createMutation(manifest5),
+            manifest6,
+            createMutation(manifest6))
+        .inOrder();
+  }
+
   @Test
   public void testRun_exportingFromStartOfTime_exportsAllCommits() throws Exception {
     task.lowerCheckpointTime = START_OF_TIME;