Move queue difference to NSQ, minor cleanup

3 files changed, 27 insertions, 23 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceQueue.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceQueue.java
index d5ce42f2378..94b5299709f 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceQueue.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceQueue.java
@@ -10,10 +10,13 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.function.Predicate;
 import java.util.function.UnaryOperator;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import static java.util.function.Predicate.not;
+
 /**
  * A queue of outstanding {@link NameServiceRequest}s. Requests in this have not yet been dispatched to a
  * {@link NameService} and are thus not visible in DNS.
@@ -21,6 +24,7 @@ import java.util.logging.Logger;
  * This is immutable.
  *
  * @author mpolden
+ * @author jonmv
  */
 public record NameServiceQueue(List<NameServiceRequest> requests) {
 
@@ -78,7 +82,8 @@ public record NameServiceQueue(List<NameServiceRequest> requests) {
             NameServiceRequest request = pending.poll();
             try {
                 request.dispatchTo(nameService);
-            } catch (Exception e) {
+            }
+            catch (Exception e) {
                 log.log(Level.WARNING, "Failed to execute " + request + ": " + Exceptions.toMessageString(e) +
                                        ", request will be moved backwards, and retried");
 
@@ -89,7 +94,7 @@ public record NameServiceQueue(List<NameServiceRequest> requests) {
                 do {
                     if (request.owner().isEmpty()) {
                         pending.push(request);
-                        break;  // Can't modify anything past this, as operator requests must come in order with all others.
+                        break;  // Can't modify anything past this, as owner-less requests must come in order with all others.
                     }
                     (request.owner().equals(owner) ? owned : others).offer(request);
                 }
@@ -127,4 +132,11 @@ public record NameServiceQueue(List<NameServiceRequest> requests) {
 
     }
 
+    /** Returns the queue of records present in this, but not in {@code remaining}. */
+    public NameServiceQueue minus(NameServiceQueue remaining) {
+        return new NameServiceQueue(requests.stream()
+                                            .filter(not(new LinkedList<>(remaining.requests())::remove)) // Count duplicates.
+                                            .toList());
+    }
+
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/NameServiceDispatcher.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/NameServiceDispatcher.java
index 2890f321927..8de2b957eb5 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/NameServiceDispatcher.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/NameServiceDispatcher.java
@@ -1,22 +1,15 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.maintenance;
 
-import com.yahoo.collections.Iterables;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.NameService;
 import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue;
-import com.yahoo.vespa.hosted.controller.dns.NameServiceRequest;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 
 import java.time.Clock;
 import java.time.Duration;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.LinkedList;
-import java.util.Map;
-import java.util.function.Function;
+import java.util.ArrayList;
 import java.util.logging.Level;
-import java.util.stream.Collectors;
 
 /**
  * This dispatches requests from {@link NameServiceQueue} to a {@link NameService}. Successfully dispatched requests are
@@ -30,11 +23,15 @@ public class NameServiceDispatcher extends ControllerMaintainer {
     private final CuratorDb db;
     private final NameService nameService;
 
-    public NameServiceDispatcher(Controller controller, Duration interval) {
+    NameServiceDispatcher(Controller controller, NameService nameService, Duration interval) {
         super(controller, interval);
         this.clock = controller.clock();
         this.db = controller.curator();
-        this.nameService = controller.serviceRegistry().nameService();
+        this.nameService = nameService;
+    }
+
+    public NameServiceDispatcher(Controller controller, Duration interval) {
+        this(controller, controller.serviceRegistry().nameService(), interval);
     }
 
     @Override
@@ -43,24 +40,21 @@ public class NameServiceDispatcher extends ControllerMaintainer {
         // implementation may need to perform multiple API-specific requests to execute a single NameServiceRequest
         int requestCount = trueIntervalInSeconds();
         try (var lock = db.lockNameServiceQueue()) {
-            var queue = db.readNameServiceQueue();
-            if (queue.requests().isEmpty() || requestCount == 0) return 1.0;
+            var initial = db.readNameServiceQueue();
+            if (initial.requests().isEmpty() || requestCount == 0) return 1.0;
 
             var instant = clock.instant();
-            var remaining = queue.dispatchTo(nameService, requestCount);
-            var dispatched = queue.requests().stream()
-                                  .filter(new HashSet<>(remaining.requests())::remove)
-                                  .toList();
+            var remaining = initial.dispatchTo(nameService, requestCount);
+            var dispatched = initial.minus(remaining);
 
-            if (!dispatched.isEmpty()) {
+            if (!dispatched.requests().isEmpty()) {
                 Level logLevel = controller().system().isCd() ? Level.INFO : Level.FINE;
                 log.log(logLevel, () -> "Dispatched name service request(s) in " +
                                         Duration.between(instant, clock.instant()) +
                                         ": " + dispatched);
             }
-            // TODO: release lock while performing, verify queue is prefix of new queue when writing (locked)
             db.writeNameServiceQueue(remaining);
-            return dispatched.size() / (double) Math.min(requestCount, queue.requests().size());
+            return dispatched.requests().size() / (double) Math.min(requestCount, initial.requests().size());
         }
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/dns/NameServiceQueueTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/dns/NameServiceQueueTest.java
index 551ffa59874..8536d78f843 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/dns/NameServiceQueueTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/dns/NameServiceQueueTest.java
@@ -15,7 +15,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName;
 import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
 import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue.Priority;
 import org.junit.jupiter.api.Test;
-import wiremock.org.checkerframework.checker.units.qual.A;
 
 import java.util.ArrayDeque;
 import java.util.Deque;
@@ -26,7 +25,6 @@ import java.util.function.Consumer;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertSame;
-import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**