Generate default-get route for document retrievals

4 files changed, 111 insertions, 120 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/routing/DocumentProtocol.java b/config-model/src/main/java/com/yahoo/vespa/model/routing/DocumentProtocol.java
index 1d1b6c6c43a..97236222338 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/routing/DocumentProtocol.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/routing/DocumentProtocol.java
@@ -132,8 +132,8 @@ public final class DocumentProtocol implements Protocol, Documentrouteselectorpo
         // Build the indexing hop if it is possible to derive.
         addIndexingHop(content, table);
 
-        // Build the default route if is is possible to derive.
-        addDefaultRoute(content, containerClusters, table);
+        // Build the default routes if possible
+        addDefaultRoutes(content, containerClusters, table);
 
         // Return the complete routing table.
         simplifyRouteNames(table);
@@ -225,49 +225,40 @@ public final class DocumentProtocol implements Protocol, Documentrouteselectorpo
     }
 
     /**
-     * Create the "default" route for the Document protocol. This route will be either a route to storage or a route to
-     * search. Since we will be supporting recovery from storage, storage takes precedence over search when deciding on
-     * the final target of the default route. If there is an unambigous docproc cluster in the application, the default
-     * route will pass through this.
+     * Create the {@code default} and {@code default-get} routes for the Document protocol. The {@code default}
+     * route will be either a route to storage or a route to search. Since recovery from storage is supported,
+     * storage takes precedence over search when deciding on the final target of the default route. If there
+     * is an unambiguous docproc cluster in the application, the {@code default} route will pass through it.
+     * The {@code default-get} route skips the docproc but is otherwise identical to the {@code default} route.
      *
      * @param content The content model from {@link com.yahoo.vespa.model.VespaModel}.
      * @param containerClusters a collection of {@link com.yahoo.vespa.model.container.ContainerCluster}s
      * @param table   The routing table to add to.
      */
-    private static void addDefaultRoute(List<ContentCluster> content,
-                                        Collection<ContainerCluster> containerClusters,
-                                        RoutingTableSpec table) {
-        List<String> hops = new ArrayList<>();
-        if (!content.isEmpty()) {
-            boolean found = false;
-            for (int i = 0, len = table.getNumHops(); i < len; ++i) {
-                if (table.getHop(i).getName().equals("indexing")) {
-                    found = true;
-                    break;
-                }
-            }
-            if (found) {
-                hops.add("indexing");
-            }
+    private static void addDefaultRoutes(List<ContentCluster> content,
+                                         Collection<ContainerCluster> containerClusters,
+                                         RoutingTableSpec table) {
+        if (content.isEmpty() || !indexingHopExists(table)) {
+            return;
         }
-        if (!hops.isEmpty()) {
-            RouteSpec route = new RouteSpec("default");
-            String hop = getContainerClustersDocprocHop(containerClusters);
-            if (hop != null) {
-                route.addHop(hop);
-            }
-            int numHops = hops.size();
-            if (numHops == 1) {
-                route.addHop(hops.get(0));
-            } else {
-                StringBuilder str = new StringBuilder();
-                for (int i = 0; i < numHops; ++i) {
-                    str.append(hops.get(i)).append(i < numHops - 1 ? " " : "");
-                }
-                route.addHop("[AND:" + str.toString() + "]");
+        RouteSpec route = new RouteSpec("default");
+        String hop = getContainerClustersDocprocHop(containerClusters);
+        if (hop != null) {
+            route.addHop(hop);
+        }
+        route.addHop("indexing");
+        table.addRoute(route);
+
+        table.addRoute(new RouteSpec("default-get").addHop("indexing"));
+    }
+
+    private static boolean indexingHopExists(RoutingTableSpec table) {
+        for (int i = 0, len = table.getNumHops(); i < len; ++i) {
+            if (table.getHop(i).getName().equals("indexing")) {
+                return true;
             }
-            table.addRoute(route);
         }
+        return false;
     }
 
     private static String getContainerClustersDocprocHop(Collection<ContainerCluster> containerClusters) {
diff --git a/config-model/src/test/cfg/routing/replacehop/messagebus.cfg b/config-model/src/test/cfg/routing/replacehop/messagebus.cfg
index ad8fb260824..12701af6db3 100755
--- a/config-model/src/test/cfg/routing/replacehop/messagebus.cfg
+++ b/config-model/src/test/cfg/routing/replacehop/messagebus.cfg
@@ -11,12 +11,14 @@ routingtable[0].hop[2].selector "foo"
 routingtable[0].hop[2].ignoreresult false
 routingtable[0].route[0].name "default"
 routingtable[0].route[0].hop[0] "indexing"
-routingtable[0].route[1].name "music"
-routingtable[0].route[1].hop[0] "[MessageType:music]"
-routingtable[0].route[2].name "music-direct"
-routingtable[0].route[2].hop[0] "[Content:cluster=music]"
-routingtable[0].route[3].name "music-index"
-routingtable[0].route[3].hop[0] "docproc/cluster.music.indexing/chain.indexing"
-routingtable[0].route[3].hop[1] "[Content:cluster=music]"
-routingtable[0].route[4].name "storage/cluster.music"
-routingtable[0].route[4].hop[0] "route:music"
+routingtable[0].route[1].name "default-get"
+routingtable[0].route[1].hop[0] "indexing"
+routingtable[0].route[2].name "music"
+routingtable[0].route[2].hop[0] "[MessageType:music]"
+routingtable[0].route[3].name "music-direct"
+routingtable[0].route[3].hop[0] "[Content:cluster=music]"
+routingtable[0].route[4].name "music-index"
+routingtable[0].route[4].hop[0] "docproc/cluster.music.indexing/chain.indexing"
+routingtable[0].route[4].hop[1] "[Content:cluster=music]"
+routingtable[0].route[5].name "storage/cluster.music"
+routingtable[0].route[5].hop[0] "route:music"
diff --git a/config-model/src/test/cfg/routing/replaceroute/messagebus.cfg b/config-model/src/test/cfg/routing/replaceroute/messagebus.cfg
index e9389a2a6d9..d3af5b36c26 100755
--- a/config-model/src/test/cfg/routing/replaceroute/messagebus.cfg
+++ b/config-model/src/test/cfg/routing/replaceroute/messagebus.cfg
@@ -8,12 +8,14 @@ routingtable[0].hop[1].recipient[0] "music"
 routingtable[0].hop[1].ignoreresult false
 routingtable[0].route[0].name "default"
 routingtable[0].route[0].hop[0] "foo"
-routingtable[0].route[1].name "music"
-routingtable[0].route[1].hop[0] "[MessageType:music]"
-routingtable[0].route[2].name "music-direct"
-routingtable[0].route[2].hop[0] "[Content:cluster=music]"
-routingtable[0].route[3].name "music-index"
-routingtable[0].route[3].hop[0] "docproc/cluster.music.indexing/chain.indexing"
-routingtable[0].route[3].hop[1] "[Content:cluster=music]"
-routingtable[0].route[4].name "storage/cluster.music"
-routingtable[0].route[4].hop[0] "route:music"
+routingtable[0].route[1].name "default-get"
+routingtable[0].route[1].hop[0] "indexing"
+routingtable[0].route[2].name "music"
+routingtable[0].route[2].hop[0] "[MessageType:music]"
+routingtable[0].route[3].name "music-direct"
+routingtable[0].route[3].hop[0] "[Content:cluster=music]"
+routingtable[0].route[4].name "music-index"
+routingtable[0].route[4].hop[0] "docproc/cluster.music.indexing/chain.indexing"
+routingtable[0].route[4].hop[1] "[Content:cluster=music]"
+routingtable[0].route[5].name "storage/cluster.music"
+routingtable[0].route[5].hop[0] "route:music"
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/IndexedTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/IndexedTest.java
index e059a673f41..2a152219990 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/IndexedTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/IndexedTest.java
@@ -3,12 +3,11 @@ package com.yahoo.vespa.model.content;
 
 import com.yahoo.cloud.config.ClusterListConfig;
 import com.yahoo.config.model.deploy.DeployState;
-import com.yahoo.vespa.config.search.core.ProtonConfig;
-import com.yahoo.vespa.config.content.core.StorServerConfig;
 import com.yahoo.documentmodel.NewDocumentType;
 import com.yahoo.messagebus.routing.RouteSpec;
 import com.yahoo.messagebus.routing.RoutingTableSpec;
-import com.yahoo.vespa.model.HostResource;
+import com.yahoo.vespa.config.content.core.StorServerConfig;
+import com.yahoo.vespa.config.search.core.ProtonConfig;
 import com.yahoo.vespa.model.VespaModel;
 import com.yahoo.vespa.model.container.ContainerCluster;
 import com.yahoo.vespa.model.content.cluster.ContentCluster;
@@ -23,7 +22,10 @@ import java.util.Arrays;
 import java.util.List;
 
 import static org.hamcrest.core.Is.is;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
 
 /**
  * Test for using the content model to create indexed search clusters.
@@ -138,12 +140,13 @@ public class IndexedTest extends ContentBaseTest {
     @Test
     public void requireIndexedOnlyServices() {
         VespaModel model = getIndexedVespaModel();
-        HostResource h = model.getHostSystem().getHosts().get(0);
-        String [] expectedServices = {"logserver", "configserver", "adminserver", "slobrok",
-                                      "logd", "configproxy","config-sentinel",
-                                      "qrserver", "fleetcontroller", "topleveldispatch",
-                                      "storagenode", "searchnode", "distributor", "transactionlogserver"};
-        // TODO DomContentBuilderTest.assertServices(h, expectedServices);
+        // TODO
+        // HostResource h = model.getHostSystem().getHosts().get(0);
+        // String [] expectedServices = {"logserver", "configserver", "adminserver", "slobrok",
+        //                               "logd", "configproxy","config-sentinel",
+        //                               "qrserver", "fleetcontroller", "topleveldispatch",
+        //                               "storagenode", "searchnode", "distributor", "transactionlogserver"};
+        // DomContentBuilderTest.assertServices(h, expectedServices);
         Routing routing = model.getRouting();
         assertNotNull(routing);
         assertEquals("[]", routing.getErrors().toString());
@@ -154,43 +157,36 @@ public class IndexedTest extends ContentBaseTest {
         assertEquals("indexing", spec.getHop(0).getName());
         assertEquals("jdisc/chain.indexing", spec.getHop(1).getName());
 
-        RouteSpec r;
-        r = spec.getRoute(0);
-        assertEquals("default", r.getName());
-        assertEquals(1, r.getNumHops());
-        assertEquals("indexing", r.getHop(0));
-        r = spec.getRoute(1);
-        assertEquals("storage/cluster.test", r.getName());
-        assertEquals(1, r.getNumHops());
-        assertEquals("route:test", r.getHop(0));
-        r = spec.getRoute(2);
-        assertEquals("test", r.getName());
-        assertEquals(1, r.getNumHops());
-        assertEquals("[MessageType:test]", r.getHop(0));
-        r = spec.getRoute(3);
-        assertEquals("test-direct", r.getName());
-        assertEquals(1, r.getNumHops());
-        assertEquals("[Content:cluster=test]", r.getHop(0));
-        r = spec.getRoute(4);
-        assertEquals("test-index", r.getName());
-        assertEquals(2, r.getNumHops());
-        assertEquals("jdisc/chain.indexing", r.getHop(0));
-        assertEquals("[Content:cluster=test]", r.getHop(1));
+        assertRoute(spec.getRoute(0), "default", "indexing");
+        assertRoute(spec.getRoute(1), "default-get", "indexing");
+        assertRoute(spec.getRoute(2), "storage/cluster.test", "route:test");
+        assertRoute(spec.getRoute(3), "test", "[MessageType:test]");
+        assertRoute(spec.getRoute(4), "test-direct", "[Content:cluster=test]");
+        assertRoute(spec.getRoute(5), "test-index", "jdisc/chain.indexing", "[Content:cluster=test]");
+    }
+
+    private static void assertRoute(RouteSpec r, String name, String... hops) {
+        assertEquals(name, r.getName());
+        assertEquals(hops.length, r.getNumHops());
+        for(int i = 0; i < hops.length; i++) {
+            assertEquals(hops[i], r.getHop(i));
+        }
     }
+
     @Test
     public void requireProtonStreamingOnly()
     {
         VespaModel model = getStreamingVespaModel();
-        HostResource h = model.getHostSystem().getHosts().get(0);
-        String [] expectedServices = {"logserver", "configserver", "adminserver", "slobrok",
-                                      "logd", "configproxy","config-sentinel",
-                                      "qrserver", "storagenode", "searchnode", "distributor",
-                                      "transactionlogserver"};
-// TODO        DomContentBuilderTest.assertServices(h, expectedServices);
+        // TODO
+        // HostResource h = model.getHostSystem().getHosts().get(0);
+        // String [] expectedServices = {"logserver", "configserver", "adminserver", "slobrok",
+        //                               "logd", "configproxy","config-sentinel",
+        //                               "qrserver", "storagenode", "searchnode", "distributor",
+        //                               "transactionlogserver"};
+        // DomContentBuilderTest.assertServices(h, expectedServices);
         ContentCluster s = model.getContentClusters().get("test");
         assertFalse(s.getSearch().hasIndexedCluster());
 
-
         StorServerConfig.Builder builder = new StorServerConfig.Builder();
         s.getStorageNodes().getConfig(builder);
         s.getStorageNodes().getChildren().get("3").getConfig(builder);
@@ -211,35 +207,35 @@ public class IndexedTest extends ContentBaseTest {
 
     @Test
     public void testContentSummaryStore() {
-        String services= 
+        String services=
                 "<services version='1.0'>" +
                 "<admin version='2.0'><adminserver hostalias='node0' /></admin>" +
-                "<content id='docstore' version='1.0'>\n" + 
-                "    <redundancy>1</redundancy>\n" + 
-                "    <documents>\n" + 
-                "      <document mode='index' type='docstorebench'/>\n" + 
-                "    </documents>\n" + 
+                "<content id='docstore' version='1.0'>\n" +
+                "    <redundancy>1</redundancy>\n" +
+                "    <documents>\n" +
+                "      <document mode='index' type='docstorebench'/>\n" +
+                "    </documents>\n" +
                 "    <group>\n" +
-                "      <node distribution-key='0' hostalias='node0'/>\n" + 
-                "    </group>\n" + 
-                "    <engine>\n" + 
-                "      <proton>\n" + 
-                "        <searchable-copies>1</searchable-copies>\n" + 
-                "        <tuning>\n" + 
-                "          <searchnode>\n" + 
-                "            <summary>\n" + 
-                "              <store>\n" + 
-                "                <logstore>\n" + 
-                "                  <chunk>\n" + 
-                "                    <maxsize>2048</maxsize>\n" + 
-                "                  </chunk>\n" + 
-                "                </logstore>\n" + 
-                "              </store>\n" + 
-                "            </summary>\n" + 
-                "          </searchnode>\n" + 
-                "        </tuning>\n" + 
-                "      </proton>\n" + 
-                "    </engine>\n" + 
+                "      <node distribution-key='0' hostalias='node0'/>\n" +
+                "    </group>\n" +
+                "    <engine>\n" +
+                "      <proton>\n" +
+                "        <searchable-copies>1</searchable-copies>\n" +
+                "        <tuning>\n" +
+                "          <searchnode>\n" +
+                "            <summary>\n" +
+                "              <store>\n" +
+                "                <logstore>\n" +
+                "                  <chunk>\n" +
+                "                    <maxsize>2048</maxsize>\n" +
+                "                  </chunk>\n" +
+                "                </logstore>\n" +
+                "              </store>\n" +
+                "            </summary>\n" +
+                "          </searchnode>\n" +
+                "        </tuning>\n" +
+                "      </proton>\n" +
+                "    </engine>\n" +
                 "  </content>\n" +
                 "  </services>";