Turn softtimeout on by default

6 files changed, 45 insertions, 22 deletions

diff --git a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java
index 5308dc1a9bc..d5c4d3b2d11 100644
--- a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java
+++ b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java
@@ -31,6 +31,7 @@ import com.yahoo.prelude.query.parser.ParseException;
 import com.yahoo.prelude.query.parser.SpecialTokenRegistry;
 import com.yahoo.processing.rendering.Renderer;
 import com.yahoo.processing.request.CompoundName;
+import com.yahoo.search.query.ranking.SoftTimeout;
 import com.yahoo.slime.Inspector;
 import com.yahoo.slime.ObjectTraverser;
 import com.yahoo.vespa.config.SlimeUtils;
@@ -247,8 +248,12 @@ public class SearchHandler extends LoggingRequestHandler {
 
         Query query = new Query(request, requestMap, queryProfile);
 
-        boolean benchmarkOutput = VespaHeaders.benchmarkOutput(request);
-        boolean benchmarkCoverage = VespaHeaders.benchmarkCoverage(benchmarkOutput, request.getJDiscRequest().headers());
+        boolean benchmarking = VespaHeaders.benchmarkOutput(request);
+        boolean benchmarkCoverage = VespaHeaders.benchmarkCoverage(benchmarking, request.getJDiscRequest().headers());
+
+        // Don't use soft timeout by default when benchmarking to avoid wrong conclusions by excluding nodes
+        if (benchmarking && ! request.hasProperty(SoftTimeout.enableProperty.toString()))
+            query.properties().set(SoftTimeout.enableProperty, false);
 
         // Find and execute search chain if we have a valid query
         String invalidReason = query.validate();
@@ -292,7 +297,7 @@ public class SearchHandler extends LoggingRequestHandler {
         if (hostResponseHeaderKey.isPresent())
             response.headers().add(hostResponseHeaderKey.get(), selfHostname);
 
-        if (benchmarkOutput)
+        if (benchmarking)
             VespaHeaders.benchmarkOutput(response.headers(), benchmarkCoverage, response.getTiming(),
                                          response.getHitCounts(), getErrors(result), response.getCoverage());
 
diff --git a/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java b/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java
index 60427aeb0af..9eadd22ed43 100644
--- a/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java
+++ b/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java
@@ -217,9 +217,9 @@ public class QueryProperties extends Properties {
                 }
                 else if (key.size() == 3 && key.get(1).equals(Ranking.SOFTTIMEOUT)) {
                     SoftTimeout soft = ranking.getSoftTimeout();
-                    if (key.last().equals(SoftTimeout.ENABLE)) soft.setEnable(asBoolean(value, false));
-                    if (key.last().equals(SoftTimeout.FACTOR)) soft.setFactor(asDouble(value, 0.50));
-                    if (key.last().equals(SoftTimeout.TAILCOST)) soft.setTailcost(asDouble(value, 0.10));
+                    if (key.last().equals(SoftTimeout.ENABLE)) soft.setEnable(asBoolean(value, true));
+                    if (key.last().equals(SoftTimeout.FACTOR)) soft.setFactor(asDouble(value, null));
+                    if (key.last().equals(SoftTimeout.TAILCOST)) soft.setTailcost(asDouble(value, null));
                 }
                 else if (key.size() == 3 && key.get(1).equals(Ranking.MATCHING)) {
                     Matching matching = ranking.getMatching();
diff --git a/container-search/src/main/java/com/yahoo/search/query/ranking/SoftTimeout.java b/container-search/src/main/java/com/yahoo/search/query/ranking/SoftTimeout.java
index ca6fd44af50..a05ecee8a1e 100644
--- a/container-search/src/main/java/com/yahoo/search/query/ranking/SoftTimeout.java
+++ b/container-search/src/main/java/com/yahoo/search/query/ranking/SoftTimeout.java
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.search.query.ranking;
 
+import com.yahoo.processing.request.CompoundName;
 import com.yahoo.search.query.Ranking;
 import com.yahoo.search.query.profile.types.FieldDescription;
 import com.yahoo.search.query.profile.types.QueryProfileType;
@@ -21,6 +22,10 @@ public class SoftTimeout implements Cloneable {
     public static final String FACTOR = "factor";
     public static final String TAILCOST = "tailcost";
 
+    /** The full property name for turning softtimeout on or off */
+    public static final CompoundName enableProperty =
+            CompoundName.fromComponents(Ranking.RANKING, Ranking.SOFTTIMEOUT, ENABLE);
+
     static {
         argumentType = new QueryProfileType(Ranking.SOFTTIMEOUT);
         argumentType.setStrict(true);
@@ -31,14 +36,15 @@ public class SoftTimeout implements Cloneable {
     }
     public static QueryProfileType getArgumentType() { return argumentType; }
 
-    public Boolean enable = null;
+    private boolean enabled = true;
     private Double factor = null;
     private Double tailcost = null;
 
-    public void setEnable(boolean enable) { this.enable = enable; }
+    public void setEnable(boolean enable) { this.enabled = enable; }
 
-    public Boolean getEnable() { return enable; }
+    public Boolean getEnable() { return enabled; }
 
+    /** Override the adaptive factor determined on the content nodes */
     public void setFactor(double factor) {
         if ((factor < 0.0) || (factor > 1.0)) {
             throw new IllegalArgumentException("factor must be in the range [0.0, 1.0], got " + factor);
@@ -48,6 +54,7 @@ public class SoftTimeout implements Cloneable {
 
     public Double getFactor() { return factor; }
 
+    /** Override the tail cost factor determined on the content nodes */
     public void setTailcost(double tailcost) {
         if ((tailcost < 0.0) || (tailcost > 1.0)) {
             throw new IllegalArgumentException("tailcost must be in the range [0.0, 1.0], got " + tailcost);
@@ -59,15 +66,13 @@ public class SoftTimeout implements Cloneable {
 
     /** Internal operation - DO NOT USE */
     public void prepare(RankProperties rankProperties) {
-        if (enable != null) {
-            rankProperties.put("vespa.softtimeout.enable", String.valueOf(enable));
-        }
-        if (factor != null) {
+        if ( !enabled) return;
+
+        rankProperties.put("vespa.softtimeout.enable", "true");
+        if (factor != null)
             rankProperties.put("vespa.softtimeout.factor", String.valueOf(factor));
-        }
-        if (tailcost != null) {
+        if (tailcost != null)
             rankProperties.put("vespa.softtimeout.tailcost", String.valueOf(tailcost));
-        }
     }
 
     @Override
@@ -83,7 +88,7 @@ public class SoftTimeout implements Cloneable {
     @Override
     public int hashCode() {
         int hash = 0;
-        if (enable != null) hash += 11 * enable.hashCode();
+        if (enabled) hash += 11;
         if (factor != null) hash += 13 * factor.hashCode();
         if (tailcost != null) hash += 17 * tailcost.hashCode();
         return hash;
@@ -95,7 +100,7 @@ public class SoftTimeout implements Cloneable {
         if ( ! (o instanceof SoftTimeout)) return false;
 
         SoftTimeout other = (SoftTimeout)o;
-        if ( ! Objects.equals(this.enable, other.enable)) return false;
+        if ( ! Objects.equals(this.enabled, other.enabled)) return false;
         if ( ! Objects.equals(this.factor, other.factor)) return false;
         if ( ! Objects.equals(this.tailcost, other.tailcost)) return false;
         return true;
diff --git a/container-search/src/test/java/com/yahoo/fs4/test/QueryTestCase.java b/container-search/src/test/java/com/yahoo/fs4/test/QueryTestCase.java
index 78b2f65305a..e2ce26cb908 100644
--- a/container-search/src/test/java/com/yahoo/fs4/test/QueryTestCase.java
+++ b/container-search/src/test/java/com/yahoo/fs4/test/QueryTestCase.java
@@ -12,6 +12,7 @@ import com.yahoo.prelude.query.PhraseSegmentItem;
 import com.yahoo.prelude.query.WeightedSetItem;
 import com.yahoo.prelude.query.WordItem;
 import com.yahoo.search.Query;
+import com.yahoo.search.query.ranking.SoftTimeout;
 import org.junit.Test;
 
 import java.nio.ByteBuffer;
@@ -52,6 +53,8 @@ public class QueryTestCase {
     @Test
     public void testEncodeQueryPacketWithSomeAdditionalFeatures() {
         Query query = new Query("/?query=chain&dataset=10&type=phrase&timeout=0");
+        query.properties().set(SoftTimeout.enableProperty, false);
+
         // Because the rank mapping now needs config and a searcher,
         // we do the sledgehammer dance:
         query.getRanking().setProfile("two");
@@ -79,7 +82,7 @@ public class QueryTestCase {
                                 "&ranking.features.query(foo)=30.3&ranking.features.query(bar)=0" +
                                 "&ranking.properties.property.p1=v1&ranking.properties.property.p2=v2" +
                                 "&pos.ll=S22.4532;W123.9887&pos.radius=3&pos.attribute=place&ranking.freshness=37" +
-                                "&model.searchPath=7/3");
+                                "&model.searchPath=7/3&ranking.softtimeout.enable=false");
         query.getRanking().setFreshness(new Freshness("123456"));
         query.getRanking().setSorting("+field1 -field2");
         query.getRanking().setProfile("two");
@@ -105,7 +108,7 @@ public class QueryTestCase {
                                 "&ranking.features.query(foo)=30.3&ranking.features.query(bar)=0" +
                                 "&ranking.properties.property.p1=v1&ranking.properties.property.p2=v2" +
                                 "&pos.ll=S22.4532;W123.9887&pos.radius=3&pos.attribute=place&ranking.freshness=37" +
-                                "&model.searchPath=7/3");
+                                "&model.searchPath=7/3&ranking.softtimeout.enable=false");
         query.getRanking().setFreshness("123456");
         query.getRanking().setSorting("+field1 -field2");
         query.getRanking().setProfile("two");
@@ -127,6 +130,7 @@ public class QueryTestCase {
     @Test
     public void testEncodeQueryPacketWithLabelsConnectivityAndSignificance() {
         Query query = new Query();
+        query.properties().set(SoftTimeout.enableProperty, false);
         AndItem and = new AndItem();
         WeightedSetItem taggable1 = new WeightedSetItem("field1");
         taggable1.setLabel("foo");
diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java
index c4d32111124..0501a4d0060 100644
--- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java
@@ -185,6 +185,7 @@ public class FastSearcherTestCase {
                                                      documentdbConfigWithOneDb);
 
         Query query = new Query("?query=foo&model.restrict=testDb");
+        query.getRanking().getSoftTimeout().setEnable(false);
         query.prepare();
         doSearch(fastSearcher, query, 0, 10);
 
diff --git a/container-search/src/test/java/com/yahoo/search/query/SoftTimeoutTestCase.java b/container-search/src/test/java/com/yahoo/search/query/SoftTimeoutTestCase.java
index 83ef955a6d9..5026250ab91 100644
--- a/container-search/src/test/java/com/yahoo/search/query/SoftTimeoutTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/query/SoftTimeoutTestCase.java
@@ -13,14 +13,14 @@ public class SoftTimeoutTestCase {
     @Test
     public void testDefaultsInQuery() {
         Query query=new Query("?query=test");
-        assertNull(query.getRanking().getSoftTimeout().getEnable());
+        assertTrue(query.getRanking().getSoftTimeout().getEnable());
         assertNull(query.getRanking().getSoftTimeout().getFactor());
         assertNull(query.getRanking().getSoftTimeout().getTailcost());
     }
 
     @Test
     public void testQueryOverride() {
-        Query query=new Query("?query=test&ranking.softtimeout.enable&ranking.softtimeout.factor=0.7&ranking.softtimeout.tailcost=0.3");
+        Query query=new Query("?query=test&ranking.softtimeout.factor=0.7&ranking.softtimeout.tailcost=0.3");
         assertTrue(query.getRanking().getSoftTimeout().getEnable());
         assertEquals(Double.valueOf(0.7), query.getRanking().getSoftTimeout().getFactor());
         assertEquals(Double.valueOf(0.3), query.getRanking().getSoftTimeout().getTailcost());
@@ -30,6 +30,14 @@ public class SoftTimeoutTestCase {
         assertEquals("0.3", query.getRanking().getProperties().get("vespa.softtimeout.tailcost").get(0));
     }
 
+    @Test
+    public void testDisable() {
+        Query query=new Query("?query=test&ranking.softtimeout.enable=false");
+        assertFalse(query.getRanking().getSoftTimeout().getEnable());
+        query.prepare();
+        assertTrue(query.getRanking().getProperties().isEmpty());
+    }
+
     private void verifyException(String key, String value) {
         try {
             new Query("?query=test&ranking.softtimeout."+key+"="+value);