diff --git a/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java b/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java index ffc8df4a8..f0c567c28 100644 --- a/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java +++ b/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java @@ -17,25 +17,16 @@ package google.registry.batch; import static com.google.common.base.Preconditions.checkArgument; import static google.registry.util.DateTimeUtils.isBeforeOrAt; -import com.google.appengine.api.taskqueue.Queue; -import com.google.appengine.api.taskqueue.TaskOptions; -import com.google.appengine.api.taskqueue.TaskOptions.Method; -import com.google.appengine.api.taskqueue.TransientFailureException; import com.google.common.base.Joiner; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Multimap; import com.google.common.flogger.FluentLogger; -import google.registry.config.RegistryConfig.Config; import google.registry.model.EppResource; -import google.registry.model.eppcommon.Trid; -import google.registry.model.host.Host; import google.registry.persistence.VKey; import google.registry.request.Action.Service; import google.registry.util.CloudTasksUtils; -import google.registry.util.Retrier; import javax.inject.Inject; -import javax.inject.Named; import org.joda.time.DateTime; import org.joda.time.Duration; @@ -44,41 +35,23 @@ public final class AsyncTaskEnqueuer { /** The HTTP parameter names used by async flows. */ public static final String PARAM_RESOURCE_KEY = "resourceKey"; - public static final String PARAM_REQUESTING_CLIENT_ID = "requestingClientId"; - public static final String PARAM_CLIENT_TRANSACTION_ID = "clientTransactionId"; - public static final String PARAM_SERVER_TRANSACTION_ID = "serverTransactionId"; - public static final String PARAM_IS_SUPERUSER = "isSuperuser"; public static final String PARAM_HOST_KEY = "hostKey"; public static final String PARAM_REQUESTED_TIME = "requestedTime"; public static final String PARAM_RESAVE_TIMES = "resaveTimes"; /** The task queue names used by async flows. */ public static final String QUEUE_ASYNC_ACTIONS = "async-actions"; - public static final String QUEUE_ASYNC_DELETE = "async-delete-pull"; + public static final String QUEUE_ASYNC_HOST_RENAME = "async-host-rename-pull"; private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final Duration MAX_ASYNC_ETA = Duration.standardDays(30); - private final Duration asyncDeleteDelay; - private final Queue asyncDeletePullQueue; - private final Queue asyncDnsRefreshPullQueue; - private final Retrier retrier; - private final CloudTasksUtils cloudTasksUtils; @Inject - public AsyncTaskEnqueuer( - @Named(QUEUE_ASYNC_DELETE) Queue asyncDeletePullQueue, - @Named(QUEUE_ASYNC_HOST_RENAME) Queue asyncDnsRefreshPullQueue, - @Config("asyncDeleteDelay") Duration asyncDeleteDelay, - CloudTasksUtils cloudTasksUtils, - Retrier retrier) { - this.asyncDeletePullQueue = asyncDeletePullQueue; - this.asyncDnsRefreshPullQueue = asyncDnsRefreshPullQueue; - this.asyncDeleteDelay = asyncDeleteDelay; + public AsyncTaskEnqueuer(CloudTasksUtils cloudTasksUtils) { this.cloudTasksUtils = cloudTasksUtils; - this.retrier = retrier; } /** Enqueues a task to asynchronously re-save an entity at some point in the future. */ @@ -118,46 +91,4 @@ public final class AsyncTaskEnqueuer { cloudTasksUtils.createPostTaskWithDelay( ResaveEntityAction.PATH, Service.BACKEND.toString(), params, etaDuration)); } - - /** Enqueues a task to asynchronously delete a contact or host, by key. */ - public void enqueueAsyncDelete( - EppResource resourceToDelete, - DateTime now, - String requestingRegistrarId, - Trid trid, - boolean isSuperuser) { - logger.atInfo().log( - "Enqueuing async deletion of %s on behalf of registrar %s.", - resourceToDelete.getRepoId(), requestingRegistrarId); - TaskOptions task = - TaskOptions.Builder.withMethod(Method.PULL) - .countdownMillis(asyncDeleteDelay.getMillis()) - .param(PARAM_RESOURCE_KEY, resourceToDelete.createVKey().stringify()) - .param(PARAM_REQUESTING_CLIENT_ID, requestingRegistrarId) - .param(PARAM_SERVER_TRANSACTION_ID, trid.getServerTransactionId()) - .param(PARAM_IS_SUPERUSER, Boolean.toString(isSuperuser)) - .param(PARAM_REQUESTED_TIME, now.toString()); - trid.getClientTransactionId() - .ifPresent(clTrid -> task.param(PARAM_CLIENT_TRANSACTION_ID, clTrid)); - addTaskToQueueWithRetry(asyncDeletePullQueue, task); - } - - /** Enqueues a task to asynchronously refresh DNS for a renamed host. */ - public void enqueueAsyncDnsRefresh(Host host, DateTime now) { - VKey hostKey = host.createVKey(); - logger.atInfo().log("Enqueuing async DNS refresh for renamed host %s.", hostKey); - addTaskToQueueWithRetry( - asyncDnsRefreshPullQueue, - TaskOptions.Builder.withMethod(Method.PULL) - .param(PARAM_HOST_KEY, hostKey.stringify()) - .param(PARAM_REQUESTED_TIME, now.toString())); - } - - /** - * Adds a task to a queue with retrying, to avoid aborting the entire flow over a transient issue - * enqueuing a task. - */ - private void addTaskToQueueWithRetry(final Queue queue, final TaskOptions task) { - retrier.callWithRetry(() -> queue.add(task), TransientFailureException.class); - } } diff --git a/core/src/main/java/google/registry/batch/BatchModule.java b/core/src/main/java/google/registry/batch/BatchModule.java index cdac49629..a29a54d20 100644 --- a/core/src/main/java/google/registry/batch/BatchModule.java +++ b/core/src/main/java/google/registry/batch/BatchModule.java @@ -14,13 +14,9 @@ package google.registry.batch; -import static com.google.appengine.api.taskqueue.QueueFactory.getQueue; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_REQUESTED_TIME; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESAVE_TIMES; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESOURCE_KEY; -import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_ACTIONS; -import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_DELETE; -import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_HOST_RENAME; import static google.registry.batch.CannedScriptExecutionAction.SCRIPT_PARAM; import static google.registry.request.RequestParameters.extractBooleanParameter; import static google.registry.request.RequestParameters.extractIntParameter; @@ -33,13 +29,11 @@ import static google.registry.request.RequestParameters.extractRequiredDatetimeP import static google.registry.request.RequestParameters.extractRequiredParameter; import static google.registry.request.RequestParameters.extractSetOfDatetimeParameters; -import com.google.appengine.api.taskqueue.Queue; import com.google.common.collect.ImmutableSet; import dagger.Module; import dagger.Provides; import google.registry.request.Parameter; import java.util.Optional; -import javax.inject.Named; import javax.servlet.http.HttpServletRequest; import org.joda.time.DateTime; @@ -129,24 +123,6 @@ public class BatchModule { return extractBooleanParameter(req, PARAM_DRY_RUN); } - @Provides - @Named(QUEUE_ASYNC_ACTIONS) - static Queue provideAsyncActionsPushQueue() { - return getQueue(QUEUE_ASYNC_ACTIONS); - } - - @Provides - @Named(QUEUE_ASYNC_DELETE) - static Queue provideAsyncDeletePullQueue() { - return getQueue(QUEUE_ASYNC_DELETE); - } - - @Provides - @Named(QUEUE_ASYNC_HOST_RENAME) - static Queue provideAsyncHostRenamePullQueue() { - return getQueue(QUEUE_ASYNC_HOST_RENAME); - } - // TODO(b/234424397): remove method after credential changes are rolled out. @Provides @Parameter(SCRIPT_PARAM) diff --git a/core/src/main/java/google/registry/config/RegistryConfig.java b/core/src/main/java/google/registry/config/RegistryConfig.java index faca77ebf..0987b553b 100644 --- a/core/src/main/java/google/registry/config/RegistryConfig.java +++ b/core/src/main/java/google/registry/config/RegistryConfig.java @@ -1027,38 +1027,6 @@ public final class RegistryConfig { return 50; } - /** - * Returns the delay before executing async delete flow mapreduces. - * - *

This delay should be sufficiently longer than a transaction, to solve the following - * problem: - * - *

- * - *

Although we try not to add references to a PENDING_DELETE resource, strictly speaking that - * is ok as long as the mapreduce eventually sees the new reference (and therefore - * asynchronously fails the delete). Without this delay, the mapreduce might have started before - * the domain flow committed, and could potentially miss the reference. - * - *

If you are using EPP resource caching (eppResourceCachingEnabled in YAML), then this - * duration should also be longer than that cache duration (eppResourceCachingSeconds). - * - * @see google.registry.config.RegistryConfigSettings.Caching - * @see google.registry.batch.AsyncTaskEnqueuer - */ - @Provides - @Config("asyncDeleteDelay") - public static Duration provideAsyncDeleteDelay(RegistryConfigSettings config) { - return Duration.standardSeconds(config.misc.asyncDeleteDelaySeconds); - } - /** * The server ID used in the 'svID' element of an EPP 'greeting'. * diff --git a/core/src/main/java/google/registry/config/RegistryConfigSettings.java b/core/src/main/java/google/registry/config/RegistryConfigSettings.java index 37901912d..af5214884 100644 --- a/core/src/main/java/google/registry/config/RegistryConfigSettings.java +++ b/core/src/main/java/google/registry/config/RegistryConfigSettings.java @@ -207,7 +207,6 @@ public class RegistryConfigSettings { public String alertRecipientEmailAddress; public String spec11OutgoingEmailAddress; public List spec11BccEmailAddresses; - public int asyncDeleteDelaySeconds; public int transientFailureRetries; } diff --git a/core/src/main/java/google/registry/config/files/default-config.yaml b/core/src/main/java/google/registry/config/files/default-config.yaml index 96a61545b..da7c00659 100644 --- a/core/src/main/java/google/registry/config/files/default-config.yaml +++ b/core/src/main/java/google/registry/config/files/default-config.yaml @@ -421,11 +421,6 @@ misc: spec11BccEmailAddresses: - abuse@example.com - # How long to delay processing of asynchronous deletions. This should always - # be longer than eppResourceCachingSeconds, to prevent deleted contacts or - # hosts from being used on domains. - asyncDeleteDelaySeconds: 90 - # Number of times to retry a GAE operation when a transient exception is thrown. # The number of milliseconds it'll sleep before giving up is (2^n - 2) * 100. transientFailureRetries: 12 diff --git a/core/src/main/java/google/registry/env/common/default/WEB-INF/queue.xml b/core/src/main/java/google/registry/env/common/default/WEB-INF/queue.xml index db12c3164..6fc9c9ab1 100644 --- a/core/src/main/java/google/registry/env/common/default/WEB-INF/queue.xml +++ b/core/src/main/java/google/registry/env/common/default/WEB-INF/queue.xml @@ -18,86 +18,6 @@ - - async-delete-pull - pull - - - - async-host-rename-pull - pull - - - - export-commits - 10/s - 100 - - - 1 - 60 - - - - - - - export-bigquery-poll - - 5/s - 5 - 5 - - - 20 - 300 - 2 - - - - - - export-snapshot - 1/s - - - 22h - - 300 - 1800 - 10 - - - - - - export-snapshot-poll - 5/m - - - 22h - - 60 - 300 - - - - - - export-snapshot-update-view - 1/s - - - 22h - - 10 - 60 - 10 - - - rde-upload 10/m @@ -149,7 +69,7 @@ - + nordn 1/s @@ -171,17 +91,6 @@ pull - - - mapreduce - - 500/s - 100 - - sheet @@ -208,71 +117,4 @@ 5 - - - replay-commit-logs-to-sql - 1/s - - - - - load0 - 500/s - 500 - - - - load1 - 500/s - 500 - - - - load2 - 500/s - 500 - - - - load3 - 500/s - 500 - - - - load4 - 500/s - 500 - - - - load5 - 500/s - 500 - - - - load6 - 500/s - 500 - - - - load7 - 500/s - 500 - - - - load8 - 500/s - 500 - - - - load9 - 500/s - 500 - diff --git a/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java b/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java index f4948ab19..665ad7a9d 100644 --- a/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java @@ -268,7 +268,7 @@ public final class HostUpdateFlow implements TransactionalFlow { } // 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. - asyncTaskEnqueuer.enqueueAsyncDnsRefresh(existingHost, tm().getTransactionTime()); + // TODO(jianglai): implement a SQL based solution. } } diff --git a/core/src/test/java/google/registry/batch/AsyncTaskEnqueuerTest.java b/core/src/test/java/google/registry/batch/AsyncTaskEnqueuerTest.java index a09a3a30e..32da6b9f6 100644 --- a/core/src/test/java/google/registry/batch/AsyncTaskEnqueuerTest.java +++ b/core/src/test/java/google/registry/batch/AsyncTaskEnqueuerTest.java @@ -14,16 +14,12 @@ package google.registry.batch; -import static com.google.appengine.api.taskqueue.QueueFactory.getQueue; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_REQUESTED_TIME; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESAVE_TIMES; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESOURCE_KEY; import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_ACTIONS; -import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_DELETE; -import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_HOST_RENAME; import static google.registry.testing.DatabaseHelper.persistActiveContact; import static google.registry.testing.TestLogHandlerUtils.assertLogMessage; -import static org.joda.time.Duration.standardSeconds; import com.google.cloud.tasks.v2.HttpMethod; import com.google.common.collect.ImmutableSortedSet; @@ -32,11 +28,9 @@ import google.registry.testing.AppEngineExtension; import google.registry.testing.CloudTasksHelper; import google.registry.testing.CloudTasksHelper.TaskMatcher; import google.registry.testing.FakeClock; -import google.registry.testing.FakeSleeper; import google.registry.util.CapturingLogHandler; import google.registry.util.CloudTasksUtils; import google.registry.util.JdkLoggerConfig; -import google.registry.util.Retrier; import java.util.logging.Level; import org.joda.time.DateTime; import org.joda.time.Duration; @@ -64,18 +58,11 @@ public class AsyncTaskEnqueuerTest { @BeforeEach void beforeEach() { JdkLoggerConfig.getConfig(AsyncTaskEnqueuer.class).addHandler(logHandler); - asyncTaskEnqueuer = - createForTesting(cloudTasksHelper.getTestCloudTasksUtils(), clock, standardSeconds(90)); + asyncTaskEnqueuer = createForTesting(cloudTasksHelper.getTestCloudTasksUtils()); } - public static AsyncTaskEnqueuer createForTesting( - CloudTasksUtils cloudTasksUtils, FakeClock clock, Duration asyncDeleteDelay) { - return new AsyncTaskEnqueuer( - getQueue(QUEUE_ASYNC_DELETE), - getQueue(QUEUE_ASYNC_HOST_RENAME), - asyncDeleteDelay, - cloudTasksUtils, - new Retrier(new FakeSleeper(clock), 1)); + public static AsyncTaskEnqueuer createForTesting(CloudTasksUtils cloudTasksUtils) { + return new AsyncTaskEnqueuer(cloudTasksUtils); } @Test diff --git a/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java b/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java index b129669f8..6fb60005e 100644 --- a/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java +++ b/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java @@ -41,7 +41,6 @@ import google.registry.testing.CloudTasksHelper.TaskMatcher; import google.registry.testing.DatabaseHelper; import google.registry.testing.FakeClock; import org.joda.time.DateTime; -import org.joda.time.Duration; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -67,8 +66,7 @@ public class ResaveEntityActionTest { @BeforeEach void beforeEach() { asyncTaskEnqueuer = - AsyncTaskEnqueuerTest.createForTesting( - cloudTasksHelper.getTestCloudTasksUtils(), clock, Duration.ZERO); + AsyncTaskEnqueuerTest.createForTesting(cloudTasksHelper.getTestCloudTasksUtils()); createTld("tld"); } diff --git a/core/src/test/java/google/registry/flows/EppTestComponent.java b/core/src/test/java/google/registry/flows/EppTestComponent.java index 2da289725..beeda8fba 100644 --- a/core/src/test/java/google/registry/flows/EppTestComponent.java +++ b/core/src/test/java/google/registry/flows/EppTestComponent.java @@ -14,8 +14,6 @@ package google.registry.flows; -import static org.joda.time.Duration.standardSeconds; - import dagger.Component; import dagger.Module; import dagger.Provides; @@ -73,8 +71,7 @@ public interface EppTestComponent { FakesAndMocksModule instance = new FakesAndMocksModule(); CloudTasksHelper cloudTasksHelper = new CloudTasksHelper(clock); instance.asyncTaskEnqueuer = - AsyncTaskEnqueuerTest.createForTesting( - cloudTasksHelper.getTestCloudTasksUtils(), clock, standardSeconds(90)); + AsyncTaskEnqueuerTest.createForTesting(cloudTasksHelper.getTestCloudTasksUtils()); instance.clock = clock; instance.domainFlowTmchUtils = new DomainFlowTmchUtils( diff --git a/core/src/test/java/google/registry/flows/contact/ContactDeleteFlowTest.java b/core/src/test/java/google/registry/flows/contact/ContactDeleteFlowTest.java index 8f2f4cfcd..4df9d0332 100644 --- a/core/src/test/java/google/registry/flows/contact/ContactDeleteFlowTest.java +++ b/core/src/test/java/google/registry/flows/contact/ContactDeleteFlowTest.java @@ -16,7 +16,6 @@ package google.registry.flows.contact; import static com.google.common.collect.MoreCollectors.onlyElement; import static com.google.common.truth.Truth.assertThat; -import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_DELETE; import static google.registry.testing.ContactSubject.assertAboutContacts; import static google.registry.testing.DatabaseHelper.assertNoBillingEvents; import static google.registry.testing.DatabaseHelper.createTld; @@ -27,7 +26,6 @@ import static google.registry.testing.DatabaseHelper.persistContactWithPendingTr import static google.registry.testing.DatabaseHelper.persistDeletedContact; import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.testing.EppExceptionSubject.assertAboutEppExceptions; -import static google.registry.testing.TaskQueueHelper.assertNoTasksEnqueued; import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.ImmutableSet; @@ -238,7 +236,6 @@ class ContactDeleteFlowTest extends ResourceFlowTestCase { assertNoDnsTasksEnqueued(); } assertLastHistoryContainsResource(deletedHost); - assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); } private void assertSqlDeleteSuccess() throws Exception { diff --git a/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java b/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java index 790eba820..35daa495d 100644 --- a/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java +++ b/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java @@ -16,8 +16,6 @@ package google.registry.flows.host; import static com.google.common.base.Strings.nullToEmpty; import static com.google.common.truth.Truth.assertThat; -import static google.registry.batch.AsyncTaskEnqueuer.PARAM_HOST_KEY; -import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_HOST_RENAME; import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.testing.DatabaseHelper.assertNoBillingEvents; import static google.registry.testing.DatabaseHelper.createTld; @@ -37,7 +35,6 @@ import static google.registry.testing.HistoryEntrySubject.assertAboutHistoryEntr import static google.registry.testing.HostSubject.assertAboutHosts; import static google.registry.testing.TaskQueueHelper.assertDnsTasksEnqueued; import static google.registry.testing.TaskQueueHelper.assertNoDnsTasksEnqueued; -import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued; import static google.registry.util.DateTimeUtils.END_OF_TIME; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -79,7 +76,6 @@ import google.registry.model.transfer.DomainTransferData; import google.registry.model.transfer.TransferStatus; import google.registry.persistence.VKey; import google.registry.testing.DatabaseHelper; -import google.registry.testing.TaskQueueHelper.TaskMatcher; import javax.annotation.Nullable; import org.joda.time.DateTime; import org.junit.jupiter.api.Test; @@ -203,11 +199,12 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { Host renamedHost = doSuccessfulTest(); assertThat(renamedHost.isSubordinate()).isTrue(); // Task enqueued to change the NS record of the referencing domain. - assertTasksEnqueued( - QUEUE_ASYNC_HOST_RENAME, - new TaskMatcher() - .param(PARAM_HOST_KEY, renamedHost.createVKey().stringify()) - .param("requestedTime", clock.nowUtc().toString())); + // TODO(jianglai): add assertion on host rename refresh based on SQL impementation. + // assertTasksEnqueued( + // QUEUE_ASYNC_HOST_RENAME, + // new TaskMatcher() + // .param(PARAM_HOST_KEY, renamedHost.createVKey().stringify()) + // .param("requestedTime", clock.nowUtc().toString())); } @Test diff --git a/core/src/test/java/google/registry/testing/CloudTasksHelper.java b/core/src/test/java/google/registry/testing/CloudTasksHelper.java index 722397bd7..c985fb5af 100644 --- a/core/src/test/java/google/registry/testing/CloudTasksHelper.java +++ b/core/src/test/java/google/registry/testing/CloudTasksHelper.java @@ -227,7 +227,7 @@ public class CloudTasksHelper implements Serializable { String service; // App Engine TaskOption methods default to "POST". This isn't obvious from the code, so we // default it to POST here so that we don't accidentally create an entry with a GET method when - // converting to CloudTaskUtils, which requires that the method be specified explicitly. + // converting to CloudTasksUtils, which requires that the method be specified explicitly. // Should we ever actually want to do a GET, we'll likewise have to set this explicitly for the // tests. HttpMethod method = HttpMethod.POST;