From e6ba5687b1707812b134c58a3c123a775cb202f2 Mon Sep 17 00:00:00 2001 From: nickfelt Date: Fri, 14 Oct 2016 09:05:25 -0700 Subject: [PATCH] Migrate writeLockTimeout field out of DnsQueue This makes the usage of DnsQueue.create() safer, since we're no longer forced to hardcode a copy of the @Config("dnsWriteLockTimeout") value within that method. That value is only needed for leaseTasks(), which is only called in one place (ReadDnsQueueAction), so we can just pass it in from that callsite. Also removes an unused overload of leaseTasks() that allowed specifying a tag, which is a feature we no longer need. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=136162491 --- java/google/registry/dns/DnsQueue.java | 24 +++---------------- .../registry/dns/ReadDnsQueueAction.java | 4 +++- .../google/registry/dns/DnsQueueTest.java | 2 -- .../registry/dns/ReadDnsQueueActionTest.java | 2 +- 4 files changed, 7 insertions(+), 25 deletions(-) diff --git a/java/google/registry/dns/DnsQueue.java b/java/google/registry/dns/DnsQueue.java index c94f56ec9..0934595ac 100644 --- a/java/google/registry/dns/DnsQueue.java +++ b/java/google/registry/dns/DnsQueue.java @@ -15,7 +15,6 @@ package google.registry.dns; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Strings.isNullOrEmpty; import static google.registry.dns.DnsConstants.DNS_PULL_QUEUE_NAME; import static google.registry.dns.DnsConstants.DNS_TARGET_NAME_PARAM; import static google.registry.dns.DnsConstants.DNS_TARGET_TYPE_PARAM; @@ -35,13 +34,11 @@ import com.google.apphosting.api.DeadlineExceededException; import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; import com.google.common.net.InternetDomainName; -import google.registry.config.ConfigModule.Config; import google.registry.dns.DnsConstants.TargetType; import google.registry.model.registry.Registries; import google.registry.util.FormattingLogger; import java.util.List; import java.util.concurrent.TimeUnit; -import javax.annotation.Nullable; import javax.inject.Inject; import javax.inject.Named; import org.joda.time.Duration; @@ -51,7 +48,6 @@ public class DnsQueue { private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); - @Inject @Config("dnsWriteLockTimeout") Duration writeLockTimeout; @Inject @Named(DNS_PULL_QUEUE_NAME) Queue queue; @Inject DnsQueue() {} @@ -93,23 +89,10 @@ public class DnsQueue { return addToQueue(TargetType.ZONE, fullyQualifiedZoneName, fullyQualifiedZoneName); } - /** - * Returns a batch of pending tasks. - */ - public List leaseTasks() { - return leaseTasks(null); - } - - /** - * Returns a batch of pending tasks. - * - * @param tag the filter used to lease only those tasks that match - */ - public List leaseTasks(@Nullable String tag) { + /** Returns handles for a batch of tasks, leased for the specified duration. */ + public List leaseTasks(Duration leaseDuration) { try { - return isNullOrEmpty(tag) - ? queue.leaseTasks(writeLockTimeout.getMillis(), MILLISECONDS, writeBatchSize) - : queue.leaseTasksByTag(writeLockTimeout.getMillis(), MILLISECONDS, writeBatchSize, tag); + return queue.leaseTasks(leaseDuration.getMillis(), MILLISECONDS, writeBatchSize); } catch (TransientFailureException | DeadlineExceededException e) { logger.severe(e, "Failed leasing tasks too fast"); return ImmutableList.of(); @@ -154,7 +137,6 @@ public class DnsQueue { */ public static DnsQueue create() { DnsQueue result = new DnsQueue(); - result.writeLockTimeout = Duration.standardSeconds(120); result.queue = QueueFactory.getQueue(DNS_PULL_QUEUE_NAME); return result; } diff --git a/java/google/registry/dns/ReadDnsQueueAction.java b/java/google/registry/dns/ReadDnsQueueAction.java index 2ebb0a844..fe5899056 100644 --- a/java/google/registry/dns/ReadDnsQueueAction.java +++ b/java/google/registry/dns/ReadDnsQueueAction.java @@ -50,6 +50,7 @@ import java.util.Random; import java.util.Set; import javax.inject.Inject; import javax.inject.Named; +import org.joda.time.Duration; /** * Action for fanning out DNS refresh tasks by TLD, using data taken from the DNS pull queue. @@ -72,6 +73,7 @@ public final class ReadDnsQueueAction implements Runnable { private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); @Inject @Config("dnsTldUpdateBatchSize") int tldUpdateBatchSize; + @Inject @Config("dnsWriteLockTimeout") Duration writeLockTimeout; @Inject @Named(DNS_PUBLISH_PUSH_QUEUE_NAME) Queue dnsPublishPushQueue; @Inject @Parameter(JITTER_SECONDS_PARAM) Optional jitterSeconds; @Inject @Parameter(KEEP_TASKS_PARAM) boolean keepTasks; @@ -104,7 +106,7 @@ public final class ReadDnsQueueAction implements Runnable { public void run() { Set tldsOfInterest = getTlds(); - List tasks = dnsQueue.leaseTasks(); + List tasks = dnsQueue.leaseTasks(writeLockTimeout); if (tasks.isEmpty()) { return; } diff --git a/javatests/google/registry/dns/DnsQueueTest.java b/javatests/google/registry/dns/DnsQueueTest.java index 259db7d13..70c681481 100644 --- a/javatests/google/registry/dns/DnsQueueTest.java +++ b/javatests/google/registry/dns/DnsQueueTest.java @@ -22,7 +22,6 @@ import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued; import google.registry.testing.AppEngineRule; import google.registry.testing.ExceptionRule; import google.registry.testing.TaskQueueHelper.TaskMatcher; -import org.joda.time.Duration; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -49,7 +48,6 @@ public class DnsQueueTest { dnsQueue = new DnsQueue(); dnsQueue.queue = getQueue("dns-pull"); dnsQueue.writeBatchSize = 10; - dnsQueue.writeLockTimeout = Duration.standardSeconds(30); } @Test diff --git a/javatests/google/registry/dns/ReadDnsQueueActionTest.java b/javatests/google/registry/dns/ReadDnsQueueActionTest.java index 20b8ca21d..d2ba37cd0 100644 --- a/javatests/google/registry/dns/ReadDnsQueueActionTest.java +++ b/javatests/google/registry/dns/ReadDnsQueueActionTest.java @@ -88,12 +88,12 @@ public class ReadDnsQueueActionTest { persistResource(Registry.get("example").asBuilder().setTldType(TldType.TEST).build()); dnsQueue = new DnsQueue(); dnsQueue.queue = getQueue(DNS_PULL_QUEUE_NAME); - dnsQueue.writeLockTimeout = Duration.standardSeconds(10); } private void run(boolean keepTasks) throws Exception { ReadDnsQueueAction action = new ReadDnsQueueAction(); action.tldUpdateBatchSize = TEST_TLD_UPDATE_BATCH_SIZE; + action.writeLockTimeout = Duration.standardSeconds(10); action.dnsQueue = dnsQueue; action.dnsPublishPushQueue = QueueFactory.getQueue(DNS_PUBLISH_PUSH_QUEUE_NAME); action.taskEnqueuer = new TaskEnqueuer(new Retrier(null, 1));