diff --git a/core/src/main/java/google/registry/backup/DeleteOldCommitLogsAction.java b/core/src/main/java/google/registry/backup/DeleteOldCommitLogsAction.java index d9e2f3323..9c7b7fe10 100644 --- a/core/src/main/java/google/registry/backup/DeleteOldCommitLogsAction.java +++ b/core/src/main/java/google/registry/backup/DeleteOldCommitLogsAction.java @@ -66,6 +66,8 @@ import org.joda.time.Duration; service = Action.Service.BACKEND, path = "/_dr/task/deleteOldCommitLogs", auth = Auth.AUTH_INTERNAL_OR_ADMIN) +// No longer needed in SQL. Subject to future removal. +@Deprecated public final class DeleteOldCommitLogsAction implements Runnable { private static final int NUM_MAP_SHARDS = 20; diff --git a/core/src/main/java/google/registry/batch/ResaveAllEppResourcesAction.java b/core/src/main/java/google/registry/batch/ResaveAllEppResourcesAction.java index b18bb7168..e42af204e 100644 --- a/core/src/main/java/google/registry/batch/ResaveAllEppResourcesAction.java +++ b/core/src/main/java/google/registry/batch/ResaveAllEppResourcesAction.java @@ -54,6 +54,8 @@ import javax.inject.Inject; service = Action.Service.BACKEND, path = "/_dr/task/resaveAllEppResources", auth = Auth.AUTH_INTERNAL_OR_ADMIN) +// No longer needed in SQL. Subject to future removal. +@Deprecated public class ResaveAllEppResourcesAction implements Runnable { @Inject MapreduceRunner mrRunner; diff --git a/core/src/main/java/google/registry/tools/server/KillAllCommitLogsAction.java b/core/src/main/java/google/registry/tools/server/KillAllCommitLogsAction.java index 3966d1ee8..9437a1bb5 100644 --- a/core/src/main/java/google/registry/tools/server/KillAllCommitLogsAction.java +++ b/core/src/main/java/google/registry/tools/server/KillAllCommitLogsAction.java @@ -49,6 +49,8 @@ import javax.inject.Inject; path = "/_dr/task/killAllCommitLogs", method = POST, auth = Auth.AUTH_INTERNAL_OR_ADMIN) +// No longer needed in SQL. Subject to future removal. +@Deprecated public class KillAllCommitLogsAction implements Runnable { @Inject MapreduceRunner mrRunner; diff --git a/core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java b/core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java index a0cc8aafc..fc77fedf5 100644 --- a/core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java +++ b/core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java @@ -18,6 +18,8 @@ import static com.google.common.base.Preconditions.checkArgument; import static google.registry.mapreduce.inputs.EppResourceInputs.createEntityInput; import static google.registry.model.EppResourceUtils.isActive; import static google.registry.model.registry.Registries.assertTldsExist; +import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.request.RequestParameters.PARAM_TLDS; import com.google.appengine.tools.mapreduce.Mapper; @@ -32,11 +34,12 @@ import google.registry.request.Action; import google.registry.request.Parameter; import google.registry.request.Response; import google.registry.request.auth.Auth; +import google.registry.util.Clock; import google.registry.util.NonFinalForTesting; import java.util.Random; import javax.inject.Inject; +import org.apache.http.HttpStatus; import org.joda.time.DateTime; -import org.joda.time.DateTimeZone; import org.joda.time.Duration; /** @@ -74,6 +77,8 @@ public class RefreshDnsForAllDomainsAction implements Runnable { @Parameter("smearMinutes") int smearMinutes; + @Inject DnsQueue dnsQueue; + @Inject Clock clock; @Inject Random random; @Inject @@ -83,14 +88,41 @@ public class RefreshDnsForAllDomainsAction implements Runnable { public void run() { assertTldsExist(tlds); checkArgument(smearMinutes > 0, "Must specify a positive number of smear minutes"); - mrRunner - .setJobName("Refresh DNS for all domains") - .setModuleName("tools") - .setDefaultMapShards(10) - .runMapOnly( - new RefreshDnsForAllDomainsActionMapper(tlds, smearMinutes, random), - ImmutableList.of(createEntityInput(DomainBase.class))) - .sendLinkToMapreduceConsole(response); + if (tm().isOfy()) { + mrRunner + .setJobName("Refresh DNS for all domains") + .setModuleName("tools") + .setDefaultMapShards(10) + .runMapOnly( + new RefreshDnsForAllDomainsActionMapper(tlds, smearMinutes, random, clock.nowUtc()), + ImmutableList.of(createEntityInput(DomainBase.class))) + .sendLinkToMapreduceConsole(response); + } else { + tm().transact( + () -> + jpaTm() + .query( + "SELECT fullyQualifiedDomainName FROM Domain " + + "WHERE tld IN (:tlds) " + + "AND deletionTime > :now", + String.class) + .setParameter("tlds", tlds) + .setParameter("now", clock.nowUtc()) + .getResultStream() + .forEach( + domainName -> { + try { + // Smear the task execution time over the next N minutes. + dnsQueue.addDomainRefreshTask( + domainName, + Duration.standardMinutes(random.nextInt(smearMinutes))); + } catch (Throwable t) { + logger.atSevere().withCause(t).log( + "Error while enqueuing DNS refresh for domain %s", domainName); + response.setStatus(HttpStatus.SC_INTERNAL_SERVER_ERROR); + } + })); + } } /** Mapper to refresh DNS for all active domain resources. */ @@ -104,19 +136,21 @@ public class RefreshDnsForAllDomainsAction implements Runnable { private final ImmutableSet tlds; private final int smearMinutes; private final Random random; + private final DateTime now; RefreshDnsForAllDomainsActionMapper( - ImmutableSet tlds, int smearMinutes, Random random) { + ImmutableSet tlds, int smearMinutes, Random random, DateTime now) { this.tlds = tlds; this.smearMinutes = smearMinutes; this.random = random; + this.now = now; } @Override public void map(final DomainBase domain) { String domainName = domain.getDomainName(); if (tlds.contains(domain.getTld())) { - if (isActive(domain, DateTime.now(DateTimeZone.UTC))) { + if (isActive(domain, now)) { try { // Smear the task execution time over the next N minutes. dnsQueue.addDomainRefreshTask( @@ -124,7 +158,7 @@ public class RefreshDnsForAllDomainsAction implements Runnable { getContext().incrementCounter("active domains refreshed"); } catch (Throwable t) { logger.atSevere().withCause(t).log( - "Error while refreshing DNS for domain %s", domainName); + "Error while enqueuing DNS refresh for domain %s", domainName); getContext().incrementCounter("active domains errored"); } } else { diff --git a/core/src/main/java/google/registry/tools/server/ResaveAllHistoryEntriesAction.java b/core/src/main/java/google/registry/tools/server/ResaveAllHistoryEntriesAction.java index 811e3b2f3..e87f318f3 100644 --- a/core/src/main/java/google/registry/tools/server/ResaveAllHistoryEntriesAction.java +++ b/core/src/main/java/google/registry/tools/server/ResaveAllHistoryEntriesAction.java @@ -42,6 +42,8 @@ import javax.inject.Inject; service = Action.Service.TOOLS, path = "/_dr/task/resaveAllHistoryEntries", auth = Auth.AUTH_INTERNAL_OR_ADMIN) +// No longer needed in SQL. Subject to future removal. +@Deprecated public class ResaveAllHistoryEntriesAction implements Runnable { @Inject MapreduceRunner mrRunner; diff --git a/core/src/test/java/google/registry/tools/server/RefreshDnsForAllDomainsActionTest.java b/core/src/test/java/google/registry/tools/server/RefreshDnsForAllDomainsActionTest.java index 06d0b87f1..7bc0f9a09 100644 --- a/core/src/test/java/google/registry/tools/server/RefreshDnsForAllDomainsActionTest.java +++ b/core/src/test/java/google/registry/tools/server/RefreshDnsForAllDomainsActionTest.java @@ -18,37 +18,52 @@ import static com.google.common.truth.Truth.assertThat; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.persistActiveDomain; import static google.registry.testing.DatabaseHelper.persistDeletedDomain; -import static org.joda.time.DateTimeZone.UTC; +import static google.registry.testing.TaskQueueHelper.assertDnsTasksEnqueued; import static org.joda.time.Duration.standardMinutes; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import com.google.common.collect.ImmutableSet; import google.registry.dns.DnsQueue; +import google.registry.model.ofy.Ofy; +import google.registry.testing.DualDatabaseTest; +import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.testing.InjectExtension; +import google.registry.testing.TestOfyAndSql; +import google.registry.testing.TestSqlOnly; import google.registry.testing.mapreduce.MapreduceTestCase; import google.registry.tools.server.RefreshDnsForAllDomainsAction.RefreshDnsForAllDomainsActionMapper; import java.util.Random; +import org.apache.http.HttpStatus; import org.joda.time.DateTime; import org.joda.time.Duration; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Order; import org.junit.jupiter.api.extension.RegisterExtension; import org.mockito.ArgumentCaptor; /** Unit tests for {@link RefreshDnsForAllDomainsAction}. */ +@DualDatabaseTest public class RefreshDnsForAllDomainsActionTest extends MapreduceTestCase { - @RegisterExtension public final InjectExtension inject = new InjectExtension(); - + private final FakeClock clock = new FakeClock(DateTime.parse("2020-02-02T02:02:02Z")); private final DnsQueue dnsQueue = mock(DnsQueue.class); private DnsQueue origDnsQueue; + private FakeResponse response = new FakeResponse(); + + @Order(Order.DEFAULT - 1) + @RegisterExtension + public final InjectExtension inject = + new InjectExtension().withStaticFieldOverride(Ofy.class, "clock", clock); @BeforeEach void beforeEach() { @@ -60,6 +75,9 @@ public class RefreshDnsForAllDomainsActionTest action.random.setSeed(123L); action.mrRunner = makeDefaultRunner(); action.response = new FakeResponse(); + action.clock = clock; + action.dnsQueue = dnsQueue; + action.response = response; createTld("bar"); } @@ -70,58 +88,74 @@ public class RefreshDnsForAllDomainsActionTest .isEqualTo(dnsQueue); } - private void runMapreduce() throws Exception { + private void runAction() throws Exception { action.run(); executeTasksUntilEmpty("mapreduce"); } - @Test + @TestSqlOnly + void test_runAction_errorEnqueuingToDnsQueue() throws Exception { + persistActiveDomain("foo.bar"); + persistActiveDomain("baz.bar"); + persistActiveDomain("low.bar"); + action.tlds = ImmutableSet.of("bar"); + DnsQueue faultyQueue = spy(origDnsQueue); + doThrow(new RuntimeException("Error enqueuing task.")) + .when(faultyQueue) + .addDomainRefreshTask(eq("baz.bar"), any(Duration.class)); + action.dnsQueue = faultyQueue; + runAction(); + assertDnsTasksEnqueued("foo.bar", "low.bar"); + assertThat(response.getStatus()).isEqualTo(HttpStatus.SC_INTERNAL_SERVER_ERROR); + } + + @TestOfyAndSql void test_runAction_successfullyEnqueuesDnsRefreshes() throws Exception { persistActiveDomain("foo.bar"); persistActiveDomain("low.bar"); action.tlds = ImmutableSet.of("bar"); - runMapreduce(); + runAction(); verify(dnsQueue).addDomainRefreshTask("foo.bar", Duration.ZERO); verify(dnsQueue).addDomainRefreshTask("low.bar", Duration.ZERO); } - @Test + @TestOfyAndSql void test_runAction_smearsOutDnsRefreshes() throws Exception { persistActiveDomain("foo.bar"); persistActiveDomain("low.bar"); action.tlds = ImmutableSet.of("bar"); action.smearMinutes = 1000; - runMapreduce(); + runAction(); ArgumentCaptor captor = ArgumentCaptor.forClass(Duration.class); verify(dnsQueue).addDomainRefreshTask(eq("foo.bar"), captor.capture()); verify(dnsQueue).addDomainRefreshTask(eq("low.bar"), captor.capture()); assertThat(captor.getAllValues()).containsExactly(standardMinutes(450), standardMinutes(782)); } - @Test + @TestOfyAndSql void test_runAction_doesntRefreshDeletedDomain() throws Exception { persistActiveDomain("foo.bar"); - persistDeletedDomain("deleted.bar", DateTime.now(UTC).minusYears(1)); + persistDeletedDomain("deleted.bar", clock.nowUtc().minusYears(1)); action.tlds = ImmutableSet.of("bar"); - runMapreduce(); + runAction(); verify(dnsQueue).addDomainRefreshTask("foo.bar", Duration.ZERO); verify(dnsQueue, never()).addDomainRefreshTask("deleted.bar", Duration.ZERO); } - @Test + @TestOfyAndSql void test_runAction_ignoresDomainsOnOtherTlds() throws Exception { createTld("baz"); persistActiveDomain("foo.bar"); persistActiveDomain("low.bar"); persistActiveDomain("ignore.baz"); action.tlds = ImmutableSet.of("bar"); - runMapreduce(); + runAction(); verify(dnsQueue).addDomainRefreshTask("foo.bar", Duration.ZERO); verify(dnsQueue).addDomainRefreshTask("low.bar", Duration.ZERO); verify(dnsQueue, never()).addDomainRefreshTask("ignore.baz", Duration.ZERO); } - @Test + @TestOfyAndSql void test_smearMinutesMustBeSpecified() { action.tlds = ImmutableSet.of("bar"); action.smearMinutes = 0;