Strict group ordering in round-robin; corrected comparison of group coverages

2 files changed, 18 insertions, 6 deletions

diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java b/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java
index 9eac9b9b63d..4962746cdea 100644
--- a/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java
+++ b/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java
@@ -5,7 +5,6 @@ import com.yahoo.search.Query;
 import com.yahoo.search.dispatch.SearchCluster.Group;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.logging.Level;
@@ -33,10 +32,9 @@ public class LoadBalancer {
         }
         this.scoreboard = new ArrayList<>(searchCluster.groups().size());
 
-        for (Group group : searchCluster.groups().values()) {
+        for (Group group : searchCluster.orderedGroups()) {
             scoreboard.add(new GroupSchedule(group));
         }
-        Collections.shuffle(scoreboard);
     }
 
     /**
@@ -74,16 +72,18 @@ public class LoadBalancer {
     private Optional<Group> allocateNextGroup() {
         synchronized (this) {
             GroupSchedule bestSchedule = null;
+            int bestIndex = needle;
 
             int index = needle;
             for (int i = 0; i < scoreboard.size(); i++) {
                 GroupSchedule sched = scoreboard.get(index);
                 if (sched.isPreferredOver(bestSchedule)) {
                     bestSchedule = sched;
+                    bestIndex = index;
                 }
                 index = nextScoreboardIndex(index);
             }
-            needle = nextScoreboardIndex(needle);
+            needle = nextScoreboardIndex(bestIndex);
 
             Group ret = null;
             if (bestSchedule != null) {
@@ -118,9 +118,18 @@ public class LoadBalancer {
             if (other == null) {
                 return true;
             }
-            if (! group.hasSufficientCoverage()) {
-                return false;
+
+            // different coverage
+            if (this.group.hasSufficientCoverage() != other.group.hasSufficientCoverage()) {
+                if (! this.group.hasSufficientCoverage()) {
+                    // this doesn't have coverage, other does
+                    return false;
+                } else {
+                    // other doesn't have coverage, this does
+                    return true;
+                }
             }
+
             return this.score < other.score;
         }
 
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 0d50702acfd..f806d4e685a 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
@@ -153,6 +153,9 @@ public class SearchCluster implements NodeManager<SearchCluster.Node> {
     /** Returns the groups of this cluster as an immutable map indexed by group id */
     public ImmutableMap<Integer, Group> groups() { return groups; }
 
+    /** Returns the groups of this cluster as an immutable list in introduction order */
+    public ImmutableList<Group> orderedGroups() { return orderedGroups; }
+
     /** Returns the n'th (zero-indexed) group in the cluster if possible */
     public Optional<Group> group(int n) {
         if (orderedGroups.size() > n) {