zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-04-30 12:07:51 +02:00
Code Issues Projects Releases Packages Wiki Activity

Remove "keepTasks" from ReadDnsQueueAction

Browse source 
"keepTasks" is a flag that prevents ReadDnsQueueAction from removing dns-update
tasks from the dns-pull queue, while still launching PublishDnsUpdates tasks to
update the DNS (meaning these tasks will be updated again in the next
ReadDnsQueueAction).

I'm not sure what's the purpose of this flag, but given we now allow multiple
writers (meaning we can already publish the same DNS multiple times) and given
that we can now recover from a bad writer (if a writer doesn't belong to a TLD,
we put the dns-updates queued for that writer back into the dns-pull queue) - I
suspect we don't need it anymore.

Alternative considered: changing this to a "dryRun" flag that won't actually
launch PublishDnsUpdates tasks, but will log which tasks it would have
launched. Decided against it because we will still need to "own" any task for a
significant amount of time if there are many (tens of thousands) tasks in the
queue. Hence a "dryRun" will still affect any actual runs for some time.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=183997187
This commit is contained in:
guyben 2018-01-31 08:50:45 -08:00 • committed by Ben McIlwain
parent 4b23523d74
commit 6bcd40f18a
3 changed files with 18 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
java/google/registry/dns/DnsModule.java
View file

@ -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) {

27
java/google/registry/dns/ReadDnsQueueAction.java
View file

@ -61,8 +61,6 @@ import org.joda.time.Duration;
*
* <ul>
* <li>{@code jitterSeconds} Randomly delay each task by up to this many seconds.
* <li>{@code keepTasks} Do not delete any tasks from the pull queue, whether they are processed or
* not.
* </ul>
*/
@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<Integer> 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<TaskHandle> 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.");
}
}

39
javatests/google/registry/dns/ReadDnsQueueActionTest.java
View file

@ -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<TaskStateInfo> 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);
}