From d853e59c7f56f583cbc8a0e7f3fdaeb3ae218f91 Mon Sep 17 00:00:00 2001 From: mcilwain Date: Mon, 3 Oct 2016 09:29:19 -0700 Subject: [PATCH] Cut over to batched DNS refreshing on host renames TESTED=I deployed it on alpha, renamed some hosts, and verified that the [] ran as expected. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=134991941 --- .../ProductionRegistryConfigExample.java | 5 ---- .../registry/config/RegistryConfig.java | 9 ------- .../registry/config/TestRegistryConfig.java | 5 ---- .../flows/async/AsyncFlowEnqueuer.java | 25 ++----------------- .../registry/flows/host/HostUpdateFlow.java | 2 +- .../google/registry/rde/RdeStagingAction.java | 4 +-- .../flows/host/HostUpdateFlowTest.java | 9 +++---- 7 files changed, 9 insertions(+), 50 deletions(-) diff --git a/java/google/registry/config/ProductionRegistryConfigExample.java b/java/google/registry/config/ProductionRegistryConfigExample.java index b04dcb27b..d7d1de6c5 100644 --- a/java/google/registry/config/ProductionRegistryConfigExample.java +++ b/java/google/registry/config/ProductionRegistryConfigExample.java @@ -218,9 +218,4 @@ public final class ProductionRegistryConfigExample implements RegistryConfig { public String getCheckApiServletRegistrarClientId() { return "TheRegistrar"; } - - @Override - public Duration getAsyncFlowFailureBackoff() { - return Duration.standardMinutes(10); - } } diff --git a/java/google/registry/config/RegistryConfig.java b/java/google/registry/config/RegistryConfig.java index 6592469f8..552c41a45 100644 --- a/java/google/registry/config/RegistryConfig.java +++ b/java/google/registry/config/RegistryConfig.java @@ -211,14 +211,5 @@ public interface RegistryConfig { */ public String getCheckApiServletRegistrarClientId(); - /** - * Returns the amount of time to back off following an async flow task failure. - * - * This should be ~orders of magnitude larger than the rate on the queue, in order to prevent - * the logs from filling up with unnecessarily failures. - */ - // TODO(b/26140521): Remove this configuration option along with non-batched async operations. - public Duration getAsyncFlowFailureBackoff(); - // XXX: Please consider using ConfigModule instead of adding new methods to this file. } diff --git a/java/google/registry/config/TestRegistryConfig.java b/java/google/registry/config/TestRegistryConfig.java index 352f9ffdc..0211ffdd3 100644 --- a/java/google/registry/config/TestRegistryConfig.java +++ b/java/google/registry/config/TestRegistryConfig.java @@ -166,9 +166,4 @@ public class TestRegistryConfig implements RegistryConfig { public String getCheckApiServletRegistrarClientId() { return "TheRegistrar"; } - - @Override - public Duration getAsyncFlowFailureBackoff() { - return Duration.standardMinutes(10); - } } diff --git a/java/google/registry/flows/async/AsyncFlowEnqueuer.java b/java/google/registry/flows/async/AsyncFlowEnqueuer.java index 33943978b..fb314572d 100644 --- a/java/google/registry/flows/async/AsyncFlowEnqueuer.java +++ b/java/google/registry/flows/async/AsyncFlowEnqueuer.java @@ -14,22 +14,18 @@ package google.registry.flows.async; -import static com.google.appengine.api.taskqueue.QueueFactory.getQueue; import static google.registry.flows.async.DeleteContactsAndHostsAction.PARAM_IS_SUPERUSER; import static google.registry.flows.async.DeleteContactsAndHostsAction.PARAM_REQUESTING_CLIENT_ID; import static google.registry.flows.async.DeleteContactsAndHostsAction.PARAM_RESOURCE_KEY; import static google.registry.flows.async.DnsRefreshForHostRenameAction.PARAM_HOST_KEY; import static google.registry.flows.async.RefreshDnsOnHostRenameAction.QUEUE_ASYNC_HOST_RENAME; -import static google.registry.request.Actions.getPathForAction; import com.google.appengine.api.taskqueue.Queue; -import com.google.appengine.api.taskqueue.RetryOptions; import com.google.appengine.api.taskqueue.TaskOptions; import com.google.appengine.api.taskqueue.TaskOptions.Method; import com.google.appengine.api.taskqueue.TransientFailureException; import com.googlecode.objectify.Key; import google.registry.config.ConfigModule.Config; -import google.registry.config.RegistryEnvironment; import google.registry.model.EppResource; import google.registry.model.host.HostResource; import google.registry.util.FormattingLogger; @@ -55,7 +51,7 @@ public final class AsyncFlowEnqueuer { EppResource resourceToDelete, String requestingClientId, boolean isSuperuser) { Key resourceKey = Key.create(resourceToDelete); logger.infofmt( - "Enqueueing async deletion of %s on behalf of registrar %s.", + "Enqueuing async deletion of %s on behalf of registrar %s.", resourceKey, requestingClientId); TaskOptions task = TaskOptions.Builder @@ -70,29 +66,12 @@ public final class AsyncFlowEnqueuer { /** Enqueues a task to asynchronously refresh DNS for a renamed host. */ public void enqueueAsyncDnsRefresh(HostResource host) { Key hostKey = Key.create(host); - logger.infofmt("Enqueueing async DNS refresh for renamed host %s.", hostKey); + logger.infofmt("Enqueuing async DNS refresh for renamed host %s.", hostKey); addTaskToQueueWithRetry( asyncDnsRefreshPullQueue, TaskOptions.Builder.withMethod(Method.PULL).param(PARAM_HOST_KEY, hostKey.getString())); } - /** Enqueues a task to asynchronously refresh DNS for a renamed host. */ - //TODO(b/26140521): Delete this once non-batched DNS host refresh mapreduce is deleted. - @Deprecated - public void enqueueLegacyAsyncDnsRefresh(HostResource host) { - logger.infofmt("Enqueueing async DNS refresh for host %s", Key.create(host)); - // Aggressively back off if the task fails, to minimize flooding the logs. - RetryOptions retryOptions = - RetryOptions.Builder.withMinBackoffSeconds( - RegistryEnvironment.get().config().getAsyncFlowFailureBackoff().getStandardSeconds()); - final TaskOptions task = - TaskOptions.Builder.withUrl(getPathForAction(DnsRefreshForHostRenameAction.class)) - .retryOptions(retryOptions) - .param(PARAM_HOST_KEY, Key.create(host).getString()) - .method(Method.GET); - addTaskToQueueWithRetry(getQueue("flows-async"), task); - } - /** * Adds a task to a queue with retrying, to avoid aborting the entire flow over a transient issue * enqueuing a task. diff --git a/java/google/registry/flows/host/HostUpdateFlow.java b/java/google/registry/flows/host/HostUpdateFlow.java index 511bf2134..276aff0e9 100644 --- a/java/google/registry/flows/host/HostUpdateFlow.java +++ b/java/google/registry/flows/host/HostUpdateFlow.java @@ -230,7 +230,7 @@ public final class HostUpdateFlow extends LoggedInFlow implements TransactionalF } // We must also enqueue updates for all domains that use this host as their nameserver so // that their NS records can be updated to point at the new name. - asyncFlowEnqueuer.enqueueLegacyAsyncDnsRefresh(existingResource); + asyncFlowEnqueuer.enqueueAsyncDnsRefresh(existingResource); } } diff --git a/java/google/registry/rde/RdeStagingAction.java b/java/google/registry/rde/RdeStagingAction.java index 6c7803e12..d4d68f92c 100644 --- a/java/google/registry/rde/RdeStagingAction.java +++ b/java/google/registry/rde/RdeStagingAction.java @@ -126,10 +126,10 @@ import org.joda.time.Duration; * Duplicate jobs may exist {@code <=cursor}. So a transaction will not bother changing the cursor * if it's already been rolled forward. * - *

Enqueueing {@code RdeUploadAction} is also part of the cursor transaction. This is necessary + *

Enqueuing {@code RdeUploadAction} is also part of the cursor transaction. This is necessary * because the first thing the upload task does is check the staging cursor to verify it's been * completed, so we can't enqueue before we roll. We also can't enqueue after the roll, because then - * if enqueueing fails, the upload might never be enqueued. + * if enqueuing fails, the upload might never be enqueued. * *

Determinism

* diff --git a/javatests/google/registry/flows/host/HostUpdateFlowTest.java b/javatests/google/registry/flows/host/HostUpdateFlowTest.java index 54f00f63d..277e713bc 100644 --- a/javatests/google/registry/flows/host/HostUpdateFlowTest.java +++ b/javatests/google/registry/flows/host/HostUpdateFlowTest.java @@ -15,8 +15,8 @@ package google.registry.flows.host; import static com.google.common.truth.Truth.assertThat; +import static google.registry.flows.async.RefreshDnsOnHostRenameAction.QUEUE_ASYNC_HOST_RENAME; import static google.registry.model.EppResourceUtils.loadByForeignKey; -import static google.registry.request.Actions.getPathForAction; import static google.registry.testing.DatastoreHelper.assertNoBillingEvents; import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType; @@ -43,7 +43,6 @@ import google.registry.flows.EppRequestSource; import google.registry.flows.ResourceFlowTestCase; import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException; import google.registry.flows.ResourceFlowUtils.ResourceNotOwnedException; -import google.registry.flows.async.DnsRefreshForHostRenameAction; import google.registry.flows.exceptions.ResourceHasClientUpdateProhibitedException; import google.registry.flows.exceptions.ResourceStatusProhibitsOperationException; import google.registry.flows.exceptions.StatusNotClientSettableException; @@ -160,9 +159,9 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase