Test and complete local dispatch coverage condition

7 files changed, 83 insertions, 12 deletions

diff --git a/container-search/src/main/java/com/yahoo/fs4/PongPacket.java b/container-search/src/main/java/com/yahoo/fs4/PongPacket.java
index f0c892519db..58391952370 100644
--- a/container-search/src/main/java/com/yahoo/fs4/PongPacket.java
+++ b/container-search/src/main/java/com/yahoo/fs4/PongPacket.java
@@ -24,11 +24,16 @@ public class PongPacket extends BasicPacket {
     private int totalPartitions; // configured partitions
     private Optional<Integer> activePartitions = Optional.empty(); // number of partitions that are up
 
-    private Optional<Long> activeDocs = Optional.empty(); // how many documents are searchable (sum)
+    private Optional<Long> activeDocuments = Optional.empty(); // how many documents are searchable (sum)
 
     public PongPacket() {
     }
 
+    /** For testing */
+    public PongPacket(long activeDocuments) {
+        this.activeDocuments = Optional.of(activeDocuments);      
+    }
+
     private int code;
     protected void codeDecodedHook(int code) { this.code = code; }
     public int getCode() { return code; }
@@ -50,7 +55,7 @@ public class PongPacket extends BasicPacket {
             buffer.getInt(); // ignore rflags (historical field)
         }
         if ((features & MRF_ACTIVEDOCS) != 0) {
-            activeDocs = Optional.of(Long.valueOf(buffer.getLong()));
+            activeDocuments = Optional.of(Long.valueOf(buffer.getLong()));
         }
     }
 
@@ -71,7 +76,7 @@ public class PongPacket extends BasicPacket {
      * in the monitored backend.
      **/
     public Optional<Long> getActiveDocuments() {
-        return activeDocs;
+        return activeDocuments;
     }
 
     public Optional<Integer> getActiveNodes() {
diff --git a/container-search/src/main/java/com/yahoo/prelude/Pong.java b/container-search/src/main/java/com/yahoo/prelude/Pong.java
index 24c2797fe87..45671c73dec 100644
--- a/container-search/src/main/java/com/yahoo/prelude/Pong.java
+++ b/container-search/src/main/java/com/yahoo/prelude/Pong.java
@@ -51,9 +51,9 @@ public class Pong {
     public Optional<PongPacket> getPongPacket() { return pongPacket; }
     
     /** Returns the number of active documents in the backend responding in this Pong, if available */
-    public Optional<Integer> activeDocuments() {
-        if (!pongPacket.isPresent()) return Optional.empty();
-        return pongPacket.get().getActiveNodes();
+    public Optional<Long> activeDocuments() {
+        if ( ! pongPacket.isPresent()) return Optional.empty();
+        return pongPacket.get().getActiveDocuments();
     }
 
     public List<ErrorMessage> getErrors() {
diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/SearchCluster.java b/container-search/src/main/java/com/yahoo/search/dispatch/SearchCluster.java
index 902d075aabc..52e547cea00 100644
--- a/container-search/src/main/java/com/yahoo/search/dispatch/SearchCluster.java
+++ b/container-search/src/main/java/com/yahoo/search/dispatch/SearchCluster.java
@@ -137,8 +137,11 @@ public class SearchCluster implements NodeManager<SearchCluster.Node> {
                 if ( otherGroup != currentGroup)
                     sumOfAactiveDocumentsInOtherGroups += otherGroup.getActiveDocuments();
             long averageDocumentsInOtherGroups = sumOfAactiveDocumentsInOtherGroups / (groups.size() - 1);
-            currentGroup.setHasSufficientCoverage(
-                    100 * currentGroup.getActiveDocuments() / averageDocumentsInOtherGroups > minActivedocsCoveragePercentage);
+            if (averageDocumentsInOtherGroups == 0)
+                currentGroup.setHasSufficientCoverage(true); // no information about any group; assume coverage
+            else
+                currentGroup.setHasSufficientCoverage(
+                        100 * (double)currentGroup.getActiveDocuments() / averageDocumentsInOtherGroups > minActivedocsCoveragePercentage);
         }
         
     }
diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/DirectSearchTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/DirectSearchTestCase.java
index 9568ce58a98..7cf765eda9e 100644
--- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/DirectSearchTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/DirectSearchTestCase.java
@@ -58,12 +58,48 @@ public class DirectSearchTestCase {
         FastSearcherTester tester = new FastSearcherTester(2, FastSearcherTester.selfHostname + ":9999:0", "otherhost:9999:1");
         tester.setResponding(FastSearcherTester.selfHostname, false);
         assertEquals("1 ping request, 0 search requests", 1, tester.requestCount(FastSearcherTester.selfHostname, 9999));
-        tester.search("?query=test&dispatch.direct=true");
+        tester.search("?query=test&dispatch.direct=true&nocache");
         assertEquals("1 ping request, 0 search requests", 1, tester.requestCount(FastSearcherTester.selfHostname, 9999));
         tester.setResponding(FastSearcherTester.selfHostname, true);
         assertEquals("2 ping requests, 0 search request", 2, tester.requestCount(FastSearcherTester.selfHostname, 9999));
         tester.search("?query=test&dispatch.direct=true&nocache");
         assertEquals("2 ping requests, 1 search request", 3, tester.requestCount(FastSearcherTester.selfHostname, 9999));
     }
+    
+    @Test
+    public void testNoDirectDispatchWhenInsufficientCoverage() {
+        FastSearcherTester tester = new FastSearcherTester(3, 
+                                                           FastSearcherTester.selfHostname + ":9999:0", 
+                                                           "host1:9999:1",
+                                                           "host2:9999:2");
+        double k = 38.78955; // multiply all document counts by some number > 1 to test that we compute % correctly
+        
+        tester.setActiveDocuments(FastSearcherTester.selfHostname, (long)(96 * k));
+        tester.setActiveDocuments("host1", (long)(100 * k));
+        tester.setActiveDocuments("host2", (long)(100 * k));
+        assertEquals("1 ping request, 0 search requests", 1, tester.requestCount(FastSearcherTester.selfHostname, 9999));
+        tester.search("?query=test&dispatch.direct=true&nocache");
+        assertEquals("Still 1 ping request, 0 search requests because the default coverage is 97%, and we only have 96% locally", 
+                     1, tester.requestCount(FastSearcherTester.selfHostname, 9999));
+
+        tester.setActiveDocuments(FastSearcherTester.selfHostname, (long)(99 * k));
+        assertEquals("2 ping request, 0 search requests", 2, tester.requestCount(FastSearcherTester.selfHostname, 9999));
+        tester.search("?query=test&dispatch.direct=true&nocache");
+        assertEquals("2 ping request, 1 search requests because we now have 99% locally",
+                     3, tester.requestCount(FastSearcherTester.selfHostname, 9999));
+
+
+        tester.setActiveDocuments("host1", (long)(104 * k));
+        assertEquals("2 ping request, 1 search requests", 3, tester.requestCount(FastSearcherTester.selfHostname, 9999));
+        tester.search("?query=test&dispatch.direct=true&nocache");
+        assertEquals("2 ping request, 2 search requests because 99/((104+100)/2) > 0.97",
+                     4, tester.requestCount(FastSearcherTester.selfHostname, 9999));
+
+        tester.setActiveDocuments("host2", (long)(102 * k));
+        assertEquals("2 ping request, 2 search requests", 4, tester.requestCount(FastSearcherTester.selfHostname, 9999));
+        tester.search("?query=test&dispatch.direct=true&nocache");
+        assertEquals("Still 2 ping request, 2 search requests because 99/((104+102)/2) < 0.97",
+                     4, tester.requestCount(FastSearcherTester.selfHostname, 9999));
+    }
 
 }
diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTester.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTester.java
index d3314d7c038..ee79deaf8c3 100644
--- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTester.java
+++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTester.java
@@ -73,6 +73,7 @@ class FastSearcherTester {
         return mockFS4ResourcePool.requestCount(hostname, port);
     }
 
+    /** Sets the response status of a node and ping it to update the monitor status */
     public void setResponding(String hostname, boolean responding) {
         // Start/stop returning a failing backend
         mockFS4ResourcePool.setResponding(hostname, responding);
@@ -82,4 +83,14 @@ class FastSearcherTester {
         mockDispatcher.searchCluster().ping(node, MoreExecutors.directExecutor());
     }
 
+    /** Sets the response status of a node and ping it to update the monitor status */
+    public void setActiveDocuments(String hostname, long activeDocuments) {
+        mockFS4ResourcePool.setActiveDocuments(hostname, activeDocuments);
+
+        // Make the search cluster monitor notice right now in this thread
+        SearchCluster.Node node = mockDispatcher.searchCluster().nodesByHost().get(hostname).iterator().next();
+        mockDispatcher.searchCluster().ping(node, MoreExecutors.directExecutor());
+        mockDispatcher.searchCluster().pingIterationCompleted();
+    }
+
 }
diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/fs4mock/MockFS4ResourcePool.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/fs4mock/MockFS4ResourcePool.java
index f599f4a1c8f..91f12184884 100644
--- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/fs4mock/MockFS4ResourcePool.java
+++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/fs4mock/MockFS4ResourcePool.java
@@ -15,6 +15,7 @@ public class MockFS4ResourcePool extends FS4ResourcePool {
 
     private final Map<String, Integer> requestsPerBackend = new HashMap<>();
     private final Set<String> nonRespondingBackends = new HashSet<>();
+    private final Map<String, Long> activeDocumentsInBackend = new HashMap<>();    
 
     public MockFS4ResourcePool() {
         super(1);
@@ -26,13 +27,19 @@ public class MockFS4ResourcePool extends FS4ResourcePool {
         if (nonRespondingBackends.contains(hostname))
             return new MockBackend(hostname, NonWorkingMockFSChannel::new);
         else
-            return new MockBackend(hostname, MockFSChannel::new);
+            return new MockBackend(hostname, 
+                                   () -> new MockFSChannel(activeDocumentsInBackend.getOrDefault(hostname, 0L)));
     }
 
     /** Returns the number of times a backend for this hostname and port has been requested */
     public int requestCount(String hostname, int port) {
         return requestsPerBackend.getOrDefault(hostname + ":" + port, 0);
     }
+    
+    /** sets the number of active documents the given host will report to have in ping responses */
+    public void setActiveDocuments(String hostname, long activeDocuments) {
+        activeDocumentsInBackend.put(hostname, activeDocuments);
+    }
 
     private void countRequest(String hostAndPort) {
         requestsPerBackend.put(hostAndPort, requestsPerBackend.getOrDefault(hostAndPort, 0) + 1);
diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/fs4mock/MockFSChannel.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/fs4mock/MockFSChannel.java
index 6edc80756e1..fbdd6d526a5 100644
--- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/fs4mock/MockFSChannel.java
+++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/fs4mock/MockFSChannel.java
@@ -24,7 +24,16 @@ import java.util.List;
  */
 public class MockFSChannel extends FS4Channel {
 
-    public MockFSChannel() {}
+    /** The number of active documents this should report in ping reponses */
+    private final long activeDocuments;
+    
+    public MockFSChannel() { 
+        this(0); 
+    }
+
+    public MockFSChannel(long activeDocuments) {
+        this.activeDocuments = activeDocuments;
+    }
 
     private BasicPacket lastReceived = null;
 
@@ -100,7 +109,7 @@ public class MockFSChannel extends FS4Channel {
             addDocsums(packets, lastQueryPacket);
         }
         else if (lastReceived instanceof PingPacket) {
-            packets.add(new PongPacket());
+            packets.add(new PongPacket(activeDocuments));
         }
         while (packetCount >= 0 && packets.size() > packetCount) {
             packets.remove(packets.size() - 1);