Requeue domains on wrong DnsWriter.

Currently, if for some reason publishDnsUpdates gets a request to publish
domains to a DnsWriter that doesn't belong to said domain - it logs a warning
but published anyway.

This can happen when Writers are changed (swapped for a different writer)
leaving update commands "stuck" with the wrong writer.

Normally you'd expect these update commands to just publish their data and be
on their way. However, if the update fails for some reason (likely - if the
Writer change happened BECAUSE the updates are failing) then the same
publishDnsUpdate command will continue to run forever.

This CL changes the behavior for "publish to wrong DnsWriter" to instead
requeue the batched domains / hosts back to the Dns-pull queue, allowing them
to be re-batched (and hence published) with the correct DnsWriter(s). This
re-batching will take place in ReadDnsQueueAction.java

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=177863076

java/google/registry/dns/DnsWriterProxy.java

@@ -36,12 +36,17 @@ public final class DnsWriterProxy {
     this.dnsWriters = ImmutableMap.copyOf(dnsWriters);
   }
 
-  /** Returns the instance of {@link DnsWriter} by class name. */
+  /**
+   * Returns the instance of {@link DnsWriter} by class name.
+   *
+   * If the DnsWriter doesn't belong to this TLD, will return null.
+   */
   public DnsWriter getByClassNameForTld(String className, String tld) {
     if (!Registry.get(tld).getDnsWriters().contains(className)) {
       logger.warningfmt(
-          "Loaded potentially stale DNS writer %s which is no longer active on TLD %s.",
+          "Loaded potentially stale DNS writer %s which is not active on TLD %s.",
           className, tld);
+      return null;
     }
     DnsWriter dnsWriter = dnsWriters.get(className);
     checkState(dnsWriter != null, "Could not load DnsWriter %s for TLD %s", className, tld);

java/google/registry/dns/PublishDnsUpdatesAction.java

@@ -96,12 +96,30 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable<Void> {
     return null;
   }
 
+  /** Adds all the domains and hosts in the batch back to the queue to be processed later. */
+  private void requeueBatch() {
+    for (String domain : nullToEmpty(domains)) {
+      dnsQueue.addDomainRefreshTask(domain);
+    }
+    for (String host : nullToEmpty(hosts)) {
+      dnsQueue.addHostRefreshTask(host);
+    }
+  }
+
   /** Steps through the domain and host refreshes contained in the parameters and processes them. */
   private void processBatch() {
     DateTime timeAtStart = clock.nowUtc();
 
     DnsWriter writer = dnsWriterProxy.getByClassNameForTld(dnsWriter, tld);
 
+    if (writer == null) {
+      logger.warningfmt(
+          "Couldn't get writer %s for TLD %s, pushing domains back to the queue for retry",
+          dnsWriter, tld);
+      requeueBatch();
+      return;
+    }
+
     int domainsPublished = 0;
     int domainsRejected = 0;
     for (String domain : nullToEmpty(domains)) {

javatests/google/registry/dns/PublishDnsUpdatesActionTest.java

@@ -64,6 +64,7 @@ public class PublishDnsUpdatesActionTest {
   private final FakeLockHandler lockHandler = new FakeLockHandler(true);
   private final DnsWriter dnsWriter = mock(DnsWriter.class);
   private final DnsMetrics dnsMetrics = mock(DnsMetrics.class);
+  private final DnsQueue dnsQueue = mock(DnsQueue.class);
   private PublishDnsUpdatesAction action;
 
   @Before
@@ -71,7 +72,10 @@ public class PublishDnsUpdatesActionTest {
     inject.setStaticField(Ofy.class, "clock", clock);
     createTld("xn--q9jyb4c");
     persistResource(
-        Registry.get("xn--q9jyb4c").asBuilder().setDnsWriters(ImmutableSet.of("mock")).build());
+        Registry.get("xn--q9jyb4c")
+            .asBuilder()
+            .setDnsWriters(ImmutableSet.of("correctWriter"))
+            .build());
     DomainResource domain1 = persistActiveDomain("example.xn--q9jyb4c");
     persistActiveSubordinateHost("ns1.example.xn--q9jyb4c", domain1);
     persistActiveSubordinateHost("ns2.example.xn--q9jyb4c", domain1);
@@ -86,9 +90,10 @@ public class PublishDnsUpdatesActionTest {
     action.tld = tld;
     action.hosts = ImmutableSet.of();
     action.domains = ImmutableSet.of();
-    action.dnsWriter = "mock";
+    action.dnsWriter = "correctWriter";
-    action.dnsWriterProxy = new DnsWriterProxy(ImmutableMap.of("mock", dnsWriter));
+    action.dnsWriterProxy = new DnsWriterProxy(ImmutableMap.of("correctWriter", dnsWriter));
     action.dnsMetrics = dnsMetrics;
+    action.dnsQueue = dnsQueue;
     action.lockHandler = lockHandler;
     action.clock = clock;
     return action;
@@ -110,6 +115,8 @@ public class PublishDnsUpdatesActionTest {
     verify(dnsMetrics).incrementPublishHostRequests(0, PublishStatus.REJECTED);
     verify(dnsMetrics).recordCommit(CommitStatus.SUCCESS, Duration.ZERO, 0, 1);
     verifyNoMoreInteractions(dnsMetrics);
+
+    verifyNoMoreInteractions(dnsQueue);
   }
 
   @Test
@@ -128,6 +135,8 @@ public class PublishDnsUpdatesActionTest {
     verify(dnsMetrics).incrementPublishHostRequests(0, PublishStatus.REJECTED);
     verify(dnsMetrics).recordCommit(CommitStatus.SUCCESS, Duration.ZERO, 1, 0);
     verifyNoMoreInteractions(dnsMetrics);
+
+    verifyNoMoreInteractions(dnsQueue);
   }
 
   @Test
@@ -152,6 +161,8 @@ public class PublishDnsUpdatesActionTest {
     verify(dnsMetrics).incrementPublishHostRequests(0, PublishStatus.REJECTED);
     verify(dnsMetrics).recordCommit(CommitStatus.SUCCESS, Duration.ZERO, 2, 3);
     verifyNoMoreInteractions(dnsMetrics);
+
+    verifyNoMoreInteractions(dnsQueue);
   }
 
   @Test
@@ -170,6 +181,8 @@ public class PublishDnsUpdatesActionTest {
     verify(dnsMetrics).incrementPublishHostRequests(3, PublishStatus.REJECTED);
     verify(dnsMetrics).recordCommit(CommitStatus.SUCCESS, Duration.ZERO, 0, 0);
     verifyNoMoreInteractions(dnsMetrics);
+
+    verifyNoMoreInteractions(dnsQueue);
   }
 
   @Test
@@ -180,5 +193,31 @@ public class PublishDnsUpdatesActionTest {
     action.hosts = ImmutableSet.of("ns1.example.com", "ns2.example.com", "ns1.example2.com");
     action.lockHandler = new FakeLockHandler(false);
     action.run();
+
+    verifyNoMoreInteractions(dnsWriter);
+
+    verifyNoMoreInteractions(dnsMetrics);
+
+    verifyNoMoreInteractions(dnsQueue);
+  }
+
+  @Test
+  public void testWrongDnsWriter() throws Exception {
+    action = createAction("xn--q9jyb4c");
+    action.domains = ImmutableSet.of("example.com", "example2.com");
+    action.hosts = ImmutableSet.of("ns1.example.com", "ns2.example.com", "ns1.example2.com");
+    action.dnsWriter = "wrongWriter";
+    action.run();
+
+    verifyNoMoreInteractions(dnsWriter);
+
+    verifyNoMoreInteractions(dnsMetrics);
+
+    verify(dnsQueue).addDomainRefreshTask("example.com");
+    verify(dnsQueue).addDomainRefreshTask("example2.com");
+    verify(dnsQueue).addHostRefreshTask("ns1.example.com");
+    verify(dnsQueue).addHostRefreshTask("ns2.example.com");
+    verify(dnsQueue).addHostRefreshTask("ns1.example2.com");
+    verifyNoMoreInteractions(dnsQueue);
   }
 }