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
This commit is contained in:
nickfelt 2016-10-14 09:05:25 -07:00 committed by Ben McIlwain
parent 17475fe7e8
commit e6ba5687b1
4 changed files with 7 additions and 25 deletions

View file

@ -15,7 +15,6 @@
package google.registry.dns; package google.registry.dns;
import static com.google.common.base.Preconditions.checkArgument; 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_PULL_QUEUE_NAME;
import static google.registry.dns.DnsConstants.DNS_TARGET_NAME_PARAM; import static google.registry.dns.DnsConstants.DNS_TARGET_NAME_PARAM;
import static google.registry.dns.DnsConstants.DNS_TARGET_TYPE_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.base.Optional;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.net.InternetDomainName; import com.google.common.net.InternetDomainName;
import google.registry.config.ConfigModule.Config;
import google.registry.dns.DnsConstants.TargetType; import google.registry.dns.DnsConstants.TargetType;
import google.registry.model.registry.Registries; import google.registry.model.registry.Registries;
import google.registry.util.FormattingLogger; import google.registry.util.FormattingLogger;
import java.util.List; import java.util.List;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import javax.annotation.Nullable;
import javax.inject.Inject; import javax.inject.Inject;
import javax.inject.Named; import javax.inject.Named;
import org.joda.time.Duration; import org.joda.time.Duration;
@ -51,7 +48,6 @@ public class DnsQueue {
private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass();
@Inject @Config("dnsWriteLockTimeout") Duration writeLockTimeout;
@Inject @Named(DNS_PULL_QUEUE_NAME) Queue queue; @Inject @Named(DNS_PULL_QUEUE_NAME) Queue queue;
@Inject DnsQueue() {} @Inject DnsQueue() {}
@ -93,23 +89,10 @@ public class DnsQueue {
return addToQueue(TargetType.ZONE, fullyQualifiedZoneName, fullyQualifiedZoneName); return addToQueue(TargetType.ZONE, fullyQualifiedZoneName, fullyQualifiedZoneName);
} }
/** /** Returns handles for a batch of tasks, leased for the specified duration. */
* Returns a batch of pending tasks. public List<TaskHandle> leaseTasks(Duration leaseDuration) {
*/
public List<TaskHandle> leaseTasks() {
return leaseTasks(null);
}
/**
* Returns a batch of pending tasks.
*
* @param tag the filter used to lease only those tasks that match
*/
public List<TaskHandle> leaseTasks(@Nullable String tag) {
try { try {
return isNullOrEmpty(tag) return queue.leaseTasks(leaseDuration.getMillis(), MILLISECONDS, writeBatchSize);
? queue.leaseTasks(writeLockTimeout.getMillis(), MILLISECONDS, writeBatchSize)
: queue.leaseTasksByTag(writeLockTimeout.getMillis(), MILLISECONDS, writeBatchSize, tag);
} catch (TransientFailureException | DeadlineExceededException e) { } catch (TransientFailureException | DeadlineExceededException e) {
logger.severe(e, "Failed leasing tasks too fast"); logger.severe(e, "Failed leasing tasks too fast");
return ImmutableList.of(); return ImmutableList.of();
@ -154,7 +137,6 @@ public class DnsQueue {
*/ */
public static DnsQueue create() { public static DnsQueue create() {
DnsQueue result = new DnsQueue(); DnsQueue result = new DnsQueue();
result.writeLockTimeout = Duration.standardSeconds(120);
result.queue = QueueFactory.getQueue(DNS_PULL_QUEUE_NAME); result.queue = QueueFactory.getQueue(DNS_PULL_QUEUE_NAME);
return result; return result;
} }

View file

@ -50,6 +50,7 @@ import java.util.Random;
import java.util.Set; import java.util.Set;
import javax.inject.Inject; import javax.inject.Inject;
import javax.inject.Named; 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. * 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(); private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass();
@Inject @Config("dnsTldUpdateBatchSize") int tldUpdateBatchSize; @Inject @Config("dnsTldUpdateBatchSize") int tldUpdateBatchSize;
@Inject @Config("dnsWriteLockTimeout") Duration writeLockTimeout;
@Inject @Named(DNS_PUBLISH_PUSH_QUEUE_NAME) Queue dnsPublishPushQueue; @Inject @Named(DNS_PUBLISH_PUSH_QUEUE_NAME) Queue dnsPublishPushQueue;
@Inject @Parameter(JITTER_SECONDS_PARAM) Optional<Integer> jitterSeconds; @Inject @Parameter(JITTER_SECONDS_PARAM) Optional<Integer> jitterSeconds;
@Inject @Parameter(KEEP_TASKS_PARAM) boolean keepTasks; @Inject @Parameter(KEEP_TASKS_PARAM) boolean keepTasks;
@ -104,7 +106,7 @@ public final class ReadDnsQueueAction implements Runnable {
public void run() { public void run() {
Set<String> tldsOfInterest = getTlds(); Set<String> tldsOfInterest = getTlds();
List<TaskHandle> tasks = dnsQueue.leaseTasks(); List<TaskHandle> tasks = dnsQueue.leaseTasks(writeLockTimeout);
if (tasks.isEmpty()) { if (tasks.isEmpty()) {
return; return;
} }

View file

@ -22,7 +22,6 @@ import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued;
import google.registry.testing.AppEngineRule; import google.registry.testing.AppEngineRule;
import google.registry.testing.ExceptionRule; import google.registry.testing.ExceptionRule;
import google.registry.testing.TaskQueueHelper.TaskMatcher; import google.registry.testing.TaskQueueHelper.TaskMatcher;
import org.joda.time.Duration;
import org.junit.Before; import org.junit.Before;
import org.junit.Rule; import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
@ -49,7 +48,6 @@ public class DnsQueueTest {
dnsQueue = new DnsQueue(); dnsQueue = new DnsQueue();
dnsQueue.queue = getQueue("dns-pull"); dnsQueue.queue = getQueue("dns-pull");
dnsQueue.writeBatchSize = 10; dnsQueue.writeBatchSize = 10;
dnsQueue.writeLockTimeout = Duration.standardSeconds(30);
} }
@Test @Test

View file

@ -88,12 +88,12 @@ public class ReadDnsQueueActionTest {
persistResource(Registry.get("example").asBuilder().setTldType(TldType.TEST).build()); persistResource(Registry.get("example").asBuilder().setTldType(TldType.TEST).build());
dnsQueue = new DnsQueue(); dnsQueue = new DnsQueue();
dnsQueue.queue = getQueue(DNS_PULL_QUEUE_NAME); dnsQueue.queue = getQueue(DNS_PULL_QUEUE_NAME);
dnsQueue.writeLockTimeout = Duration.standardSeconds(10);
} }
private void run(boolean keepTasks) throws Exception { private void run(boolean keepTasks) throws Exception {
ReadDnsQueueAction action = new ReadDnsQueueAction(); ReadDnsQueueAction action = new ReadDnsQueueAction();
action.tldUpdateBatchSize = TEST_TLD_UPDATE_BATCH_SIZE; action.tldUpdateBatchSize = TEST_TLD_UPDATE_BATCH_SIZE;
action.writeLockTimeout = Duration.standardSeconds(10);
action.dnsQueue = dnsQueue; action.dnsQueue = dnsQueue;
action.dnsPublishPushQueue = QueueFactory.getQueue(DNS_PUBLISH_PUSH_QUEUE_NAME); action.dnsPublishPushQueue = QueueFactory.getQueue(DNS_PUBLISH_PUSH_QUEUE_NAME);
action.taskEnqueuer = new TaskEnqueuer(new Retrier(null, 1)); action.taskEnqueuer = new TaskEnqueuer(new Retrier(null, 1));