diff --git a/java/google/registry/dns/DnsModule.java b/java/google/registry/dns/DnsModule.java index bd98e72f7..5cebb11fe 100644 --- a/java/google/registry/dns/DnsModule.java +++ b/java/google/registry/dns/DnsModule.java @@ -19,8 +19,6 @@ import static google.registry.dns.DnsConstants.DNS_PULL_QUEUE_NAME; import static google.registry.dns.PublishDnsUpdatesAction.PARAM_DNS_WRITER; import static google.registry.dns.PublishDnsUpdatesAction.PARAM_DOMAINS; import static google.registry.dns.PublishDnsUpdatesAction.PARAM_HOSTS; -import static google.registry.dns.ReadDnsQueueAction.PARAM_KEEP_TASKS; -import static google.registry.request.RequestParameters.extractBooleanParameter; import static google.registry.request.RequestParameters.extractEnumParameter; import static google.registry.request.RequestParameters.extractRequiredParameter; import static google.registry.request.RequestParameters.extractSetOfParameters; @@ -76,12 +74,6 @@ public abstract class DnsModule { return extractSetOfParameters(req, PARAM_HOSTS); } - @Provides - @Parameter(PARAM_KEEP_TASKS) - static boolean provideKeepTasks(HttpServletRequest req) { - return extractBooleanParameter(req, PARAM_KEEP_TASKS); - } - @Provides @Parameter("domainOrHostName") static String provideName(HttpServletRequest req) { diff --git a/java/google/registry/dns/ReadDnsQueueAction.java b/java/google/registry/dns/ReadDnsQueueAction.java index 380b52931..8fd64c7d2 100644 --- a/java/google/registry/dns/ReadDnsQueueAction.java +++ b/java/google/registry/dns/ReadDnsQueueAction.java @@ -61,8 +61,6 @@ import org.joda.time.Duration; * * */ @Action( @@ -72,8 +70,6 @@ import org.joda.time.Duration; ) public final class ReadDnsQueueAction implements Runnable { - public static final String PARAM_KEEP_TASKS = "keepTasks"; - private static final String PARAM_JITTER_SECONDS = "jitterSeconds"; private static final Random random = new Random(); private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); @@ -82,7 +78,6 @@ public final class ReadDnsQueueAction implements Runnable { @Inject @Config("dnsWriteLockTimeout") Duration writeLockTimeout; @Inject @Named(DNS_PUBLISH_PUSH_QUEUE_NAME) Queue dnsPublishPushQueue; @Inject @Parameter(PARAM_JITTER_SECONDS) Optional jitterSeconds; - @Inject @Parameter(PARAM_KEEP_TASKS) boolean keepTasks; @Inject DnsQueue dnsQueue; @Inject TaskEnqueuer taskEnqueuer; @Inject ReadDnsQueueAction() {} @@ -187,21 +182,13 @@ public final class ReadDnsQueueAction implements Runnable { } } Set tasksToDelete = difference(ImmutableSet.copyOf(tasks), tasksToKeep); - // In keepTasks mode, never delete any tasks. - if (keepTasks) { - logger.infofmt("Would have deleted %d DNS update tasks.", tasksToDelete.size()); - for (TaskHandle task : tasks) { - dnsQueue.dropTaskLease(task); - } - // Otherwise, either delete or drop the lease of each task. - } else { - logger.infofmt("Deleting %d DNS update tasks.", tasksToDelete.size()); - dnsQueue.deleteTasks(ImmutableList.copyOf(tasksToDelete)); - logger.infofmt("Dropping %d DNS update tasks.", tasksToKeep.size()); - for (TaskHandle task : tasksToKeep) { - dnsQueue.dropTaskLease(task); - } - logger.infofmt("Done processing DNS tasks."); + // Either delete or drop the lease of each task. + logger.infofmt("Deleting %d DNS update tasks.", tasksToDelete.size()); + dnsQueue.deleteTasks(ImmutableList.copyOf(tasksToDelete)); + logger.infofmt("Dropping %d DNS update tasks.", tasksToKeep.size()); + for (TaskHandle task : tasksToKeep) { + dnsQueue.dropTaskLease(task); } + logger.infofmt("Done processing DNS tasks."); } } diff --git a/javatests/google/registry/dns/ReadDnsQueueActionTest.java b/javatests/google/registry/dns/ReadDnsQueueActionTest.java index 09da23eb3..55d72880b 100644 --- a/javatests/google/registry/dns/ReadDnsQueueActionTest.java +++ b/javatests/google/registry/dns/ReadDnsQueueActionTest.java @@ -28,7 +28,6 @@ import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued; import com.google.appengine.api.taskqueue.QueueFactory; import com.google.appengine.api.taskqueue.TaskOptions; import com.google.appengine.api.taskqueue.TaskOptions.Method; -import com.google.appengine.api.taskqueue.dev.QueueStateInfo.TaskStateInfo; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMultimap; @@ -39,7 +38,6 @@ import google.registry.model.registry.Registry; import google.registry.model.registry.Registry.TldType; import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; -import google.registry.testing.TaskQueueHelper; import google.registry.testing.TaskQueueHelper.TaskMatcher; import google.registry.util.Retrier; import google.registry.util.TaskEnqueuer; @@ -99,7 +97,7 @@ public class ReadDnsQueueActionTest { dnsQueue = DnsQueue.create(); } - private void run(boolean keepTasks) throws Exception { + private void run() throws Exception { ReadDnsQueueAction action = new ReadDnsQueueAction(); action.tldUpdateBatchSize = TEST_TLD_UPDATE_BATCH_SIZE; action.writeLockTimeout = Duration.standardSeconds(10); @@ -107,7 +105,6 @@ public class ReadDnsQueueActionTest { action.dnsPublishPushQueue = QueueFactory.getQueue(DNS_PUBLISH_PUSH_QUEUE_NAME); action.taskEnqueuer = new TaskEnqueuer(new Retrier(null, 1)); action.jitterSeconds = Optional.empty(); - action.keepTasks = keepTasks; // Advance the time a little, to ensure that leaseTasks() returns all tasks. clock.setTo(DateTime.now(DateTimeZone.UTC).plusMillis(1)); action.run(); @@ -141,7 +138,7 @@ public class ReadDnsQueueActionTest { dnsQueue.addDomainRefreshTask("domain.com"); dnsQueue.addDomainRefreshTask("domain.net"); dnsQueue.addDomainRefreshTask("domain.example"); - run(false); + run(); assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); assertTasksEnqueued( DNS_PUBLISH_PUSH_QUEUE_NAME, @@ -155,7 +152,7 @@ public class ReadDnsQueueActionTest { dnsQueue.addDomainRefreshTask("domain.com"); dnsQueue.addDomainRefreshTask("domain.net"); dnsQueue.addDomainRefreshTask("domain.example"); - run(false); + run(); assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); assertTldsEnqueuedInPushQueue( ImmutableMultimap.of("com", "comWriter", "net", "netWriter", "example", "exampleWriter")); @@ -169,32 +166,18 @@ public class ReadDnsQueueActionTest { .setDnsWriters(ImmutableSet.of("comWriter", "otherWriter")) .build()); dnsQueue.addDomainRefreshTask("domain.com"); - run(false); + run(); assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); assertTldsEnqueuedInPushQueue(ImmutableMultimap.of("com", "comWriter", "com", "otherWriter")); } - @Test - public void testSuccess_allTldsKeepTasks() throws Exception { - dnsQueue.addDomainRefreshTask("domain.com"); - dnsQueue.addDomainRefreshTask("domain.net"); - dnsQueue.addDomainRefreshTask("domain.example"); - List preexistingTasks = - TaskQueueHelper.getQueueInfo(DNS_PULL_QUEUE_NAME).getTaskInfo(); - run(true); - assertTldsEnqueuedInPushQueue( - ImmutableMultimap.of("com", "comWriter", "net", "netWriter", "example", "exampleWriter")); - // Check that keepTasks was honored and the pull queue tasks are still present in the queue. - assertTasksEnqueued(DNS_PULL_QUEUE_NAME, preexistingTasks); - } - @Test public void testSuccess_oneTldPaused() throws Exception { persistResource(Registry.get("net").asBuilder().setDnsPaused(true).build()); dnsQueue.addDomainRefreshTask("domain.com"); dnsQueue.addDomainRefreshTask("domain.net"); dnsQueue.addDomainRefreshTask("domain.example"); - run(false); + run(); assertTasksEnqueued(DNS_PULL_QUEUE_NAME, new TaskMatcher()); assertTldsEnqueuedInPushQueue( ImmutableMultimap.of("com", "comWriter", "example", "exampleWriter")); @@ -205,7 +188,7 @@ public class ReadDnsQueueActionTest { dnsQueue.addHostRefreshTask("ns1.domain.com"); dnsQueue.addDomainRefreshTask("domain.net"); dnsQueue.addZoneRefreshTask("example"); - run(false); + run(); assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); assertTasksEnqueued( DNS_PUBLISH_PUSH_QUEUE_NAME, @@ -230,10 +213,6 @@ public class ReadDnsQueueActionTest { refreshItemsInTask = 0; } switch (thingType) { - default: - dnsQueue.addDomainRefreshTask(domainName); - task.param("domains", domainName); - break; case 1: getQueue(DNS_PULL_QUEUE_NAME) .add(createRefreshTask("ns1." + domainName, TargetType.HOST)); @@ -244,6 +223,10 @@ public class ReadDnsQueueActionTest { .add(createRefreshTask("ns2." + domainName, TargetType.HOST)); task.param("hosts", "ns2." + domainName); break; + default: + dnsQueue.addDomainRefreshTask(domainName); + task.param("domains", domainName); + break; } // If this task is now full up, wash our hands of it, so that we'll start a new one the // next time through the loop. @@ -254,7 +237,7 @@ public class ReadDnsQueueActionTest { } } } - run(false); + run(); assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); assertTasksEnqueued(DNS_PUBLISH_PUSH_QUEUE_NAME, expectedTasks); }