From 0c0b0df36ebb9c628f6bf893bba9131ad1df342e Mon Sep 17 00:00:00 2001 From: gbrodman Date: Thu, 21 Nov 2024 17:16:26 -0500 Subject: [PATCH] Skip poll messages on deletions for configured registrars (#2613) See b/379331882 for more details --- .../registry/config/RegistryConfig.java | 11 ++++ .../config/RegistryConfigSettings.java | 1 + .../registry/config/files/default-config.yaml | 3 + .../config/files/nomulus-config-unittest.yaml | 2 + .../flows/domain/DomainDeleteFlow.java | 19 ++++-- .../domain/DomainRestoreRequestFlow.java | 4 +- .../flows/domain/DomainDeleteFlowTest.java | 11 ++++ .../domain/DomainRestoreRequestFlowTest.java | 62 +++++++++++-------- .../registry/testing/DatabaseHelper.java | 4 ++ 9 files changed, 87 insertions(+), 30 deletions(-) diff --git a/core/src/main/java/google/registry/config/RegistryConfig.java b/core/src/main/java/google/registry/config/RegistryConfig.java index 884e3213b..118f30f26 100644 --- a/core/src/main/java/google/registry/config/RegistryConfig.java +++ b/core/src/main/java/google/registry/config/RegistryConfig.java @@ -1743,6 +1743,17 @@ public final class RegistryConfig { CONFIG_SETTINGS.get().registryPolicy.tieredPricingPromotionRegistrarIds); } + /** + * Set of registrars for which we do not send poll messages on standard domain deletion. + * + *

For these registrars we won't send a poll message in order to avoid database contention. See + * b/379331882 for more details. + */ + public static ImmutableSet getNoPollMessageOnDeletionRegistrarIds() { + return ImmutableSet.copyOf( + CONFIG_SETTINGS.get().registryPolicy.noPollMessageOnDeletionRegistrarIds); + } + /** * Memoizes loading of the {@link RegistryConfigSettings} POJO. * diff --git a/core/src/main/java/google/registry/config/RegistryConfigSettings.java b/core/src/main/java/google/registry/config/RegistryConfigSettings.java index 47c0e4bde..bcd195770 100644 --- a/core/src/main/java/google/registry/config/RegistryConfigSettings.java +++ b/core/src/main/java/google/registry/config/RegistryConfigSettings.java @@ -115,6 +115,7 @@ public class RegistryConfigSettings { public boolean requireSslCertificates; public double sunriseDomainCreateDiscount; public Set tieredPricingPromotionRegistrarIds; + public Set noPollMessageOnDeletionRegistrarIds; } /** Configuration for Hibernate. */ 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 0dffa7f8f..e432edc35 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 @@ -220,6 +220,9 @@ registryPolicy: # In addition, we will return the non-promotional (i.e. incorrect) price on # domain create requests. tieredPricingPromotionRegistrarIds: [] + # List of registrars for which we won't send poll message on standard domain + # deletions. + noPollMessageOnDeletionRegistrarIds: [] hibernate: # If set to false, calls to tm().transact() cannot be nested. If set to true, diff --git a/core/src/main/java/google/registry/config/files/nomulus-config-unittest.yaml b/core/src/main/java/google/registry/config/files/nomulus-config-unittest.yaml index fb288c3b3..72b95f6a9 100644 --- a/core/src/main/java/google/registry/config/files/nomulus-config-unittest.yaml +++ b/core/src/main/java/google/registry/config/files/nomulus-config-unittest.yaml @@ -11,6 +11,8 @@ registryPolicy: Line 2 is this 1. tieredPricingPromotionRegistrarIds: - NewRegistrar + noPollMessageOnDeletionRegistrarIds: + - NewRegistrar caching: singletonCacheRefreshSeconds: 0 diff --git a/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java b/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java index 13c17c6cc..4dfeabfe3 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java @@ -44,7 +44,9 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Sets; +import com.google.common.flogger.FluentLogger; import google.registry.batch.AsyncTaskEnqueuer; +import google.registry.config.RegistryConfig; import google.registry.flows.EppException; import google.registry.flows.EppException.AssociationProhibitsOperationException; import google.registry.flows.ExtensionManager; @@ -117,6 +119,8 @@ import org.joda.time.Duration; @ReportingSpec(ActivityReportField.DOMAIN_DELETE) public final class DomainDeleteFlow implements MutatingFlow { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private static final ImmutableSet DISALLOWED_STATUSES = ImmutableSet.of( StatusValue.CLIENT_DELETE_PROHIBITED, StatusValue.PENDING_DELETE, @@ -212,10 +216,17 @@ public final class DomainDeleteFlow implements MutatingFlow { // superuser (i.e. the registrar didn't request this delete and thus should be notified even if // it is synchronous). if (durationUntilDelete.isLongerThan(Duration.ZERO) || isSuperuser) { - PollMessage.OneTime deletePollMessage = - createDeletePollMessage(existingDomain, domainHistoryId, deletionTime); - entitiesToSave.add(deletePollMessage); - builder.setDeletePollMessage(deletePollMessage.createVKey()); + if (RegistryConfig.getNoPollMessageOnDeletionRegistrarIds() + .contains(existingDomain.getCurrentSponsorRegistrarId())) { + logger.atInfo().log( + "Skipping poll message on domain deletion for registrar %s due to configuration", + existingDomain.getCurrentSponsorRegistrarId()); + } else { + PollMessage.OneTime deletePollMessage = + createDeletePollMessage(existingDomain, domainHistoryId, deletionTime); + entitiesToSave.add(deletePollMessage); + builder.setDeletePollMessage(deletePollMessage.createVKey()); + } } // Send a second poll message immediately if the domain is being deleted asynchronously by a diff --git a/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java b/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java index d19cb8b47..b34c5f6bd 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java @@ -183,7 +183,9 @@ public final class DomainRestoreRequestFlow implements MutatingFlow { DomainHistory domainHistory = buildDomainHistory(newDomain, now); entitiesToSave.add(newDomain, domainHistory, autorenewEvent, autorenewPollMessage); tm().putAll(entitiesToSave.build()); - tm().delete(existingDomain.getDeletePollMessage()); + if (existingDomain.getDeletePollMessage() != null) { + tm().delete(existingDomain.getDeletePollMessage()); + } requestDomainDnsRefresh(existingDomain.getDomainName()); return responseBuilder .setExtensions(createResponseExtensions(feesAndCredits, feeUpdate, isExpired)) diff --git a/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java index fcd167f22..0afa55d23 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java @@ -1247,4 +1247,15 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase deletePollMessage = domain.getDeletePollMessage(); + persistResource(domain.asBuilder().setDeletePollMessage(null).build()); + DatabaseHelper.deleteByKey(deletePollMessage); + runFlowAssertResponse(loadFile("generic_success_response.xml")); + } + @Test void testFailure_doesNotExist() throws Exception { ResourceDoesNotExistException thrown = diff --git a/core/src/test/java/google/registry/testing/DatabaseHelper.java b/core/src/test/java/google/registry/testing/DatabaseHelper.java index 28f28f0cc..abda280d8 100644 --- a/core/src/test/java/google/registry/testing/DatabaseHelper.java +++ b/core/src/test/java/google/registry/testing/DatabaseHelper.java @@ -1168,6 +1168,10 @@ public final class DatabaseHelper { tm().transact(() -> tm().delete(resource)); } + public static void deleteByKey(VKey key) { + tm().transact(() -> tm().delete(key)); + } + /** Force the create and update timestamps to get written into the resource. */ public static R cloneAndSetAutoTimestamps(final R resource) { // We have to separate the read and write operation into different transactions otherwise JPA