Merge pull request #4891 from vespa-engine/balder/handle-timedout-hits

Propagate timeout errors up in fill phase.

6 files changed, 122 insertions, 36 deletions

diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinitionSet.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinitionSet.java
index b02701f5c8b..8d882adeb02 100644
--- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinitionSet.java
+++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinitionSet.java
@@ -2,6 +2,7 @@
 package com.yahoo.prelude.fastsearch;
 
 import com.yahoo.slime.BinaryFormat;
+import com.yahoo.data.access.Inspector;
 import com.yahoo.slime.Slime;
 import com.yahoo.data.access.slime.SlimeAdapter;
 import com.yahoo.prelude.ConfigurationException;
@@ -13,6 +14,8 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.logging.Logger;
 
+import static com.yahoo.data.access.Type.OBJECT;
+
 /**
  * A set of docsum definitions
  *
@@ -55,9 +58,10 @@ public final class DocsumDefinitionSet {
      * @param summaryClass the requested summary class
      * @param data docsum data from backend
      * @param hit the Hit corresponding to this document summary
+     * @return Error message or null on success.
      * @throws ConfigurationException if the summary class of this hit is missing
      */
-    public final void lazyDecode(String summaryClass, byte[] data, FastHit hit) {
+    public final String lazyDecode(String summaryClass, byte[] data, FastHit hit) {
         ByteBuffer buffer = ByteBuffer.wrap(data);
         buffer.order(ByteOrder.LITTLE_ENDIAN);
         long docsumClassId = buffer.getInt();
@@ -66,7 +70,12 @@ public final class DocsumDefinitionSet {
         }
         DocsumDefinition docsumDefinition = lookupDocsum(summaryClass);
         Slime value = BinaryFormat.decode(buffer.array(), buffer.arrayOffset()+buffer.position(), buffer.remaining());
-        hit.addSummary(docsumDefinition, new SlimeAdapter(value.get()));
+        Inspector docsum = new SlimeAdapter(value.get());
+        if (docsum.type() != OBJECT) {
+            return "Hit " + hit + " failed: " + docsum.asString();
+        }
+        hit.addSummary(docsumDefinition, docsum);
+        return null;
     }
 
     private DocsumDefinition lookupDocsum(String summaryClass) {
diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java
index 52650279ad5..6ce08c75dd4 100644
--- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java
+++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java
@@ -317,7 +317,12 @@ public class FastSearcher extends VespaBackEndSearcher {
 
             int skippedHits;
             try {
-                skippedHits = fillHits(result, 0, receivedPackets, summaryClass);
+                FillHitsResult fillHitsResult = fillHits(result, 0, receivedPackets, summaryClass);
+                skippedHits = fillHitsResult.skippedHits;
+                if (fillHitsResult.error != null) {
+                    result.hits().addError(ErrorMessage.createTimeout(fillHitsResult.error));
+                    return;
+                }
             } catch (TimeoutException e) {
                 result.hits().addError(ErrorMessage.createTimeout(e.getMessage()));
                 return;
diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java
index 05e306a7dec..ea354e1291f 100644
--- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java
+++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java
@@ -471,25 +471,44 @@ public abstract class VespaBackEndSearcher extends PingableSearcher {
         }
     }
 
-    private boolean fillHit(FastHit hit, DocsumPacket packet, String summaryClass) {
+    static private class FillHitResult {
+        final boolean ok;
+        final String error;
+        FillHitResult(boolean ok) {
+            this(ok, null);
+        }
+        FillHitResult(boolean ok, String error) {
+            this.ok = ok;
+            this.error = error;
+        }
+    }
+    private FillHitResult fillHit(FastHit hit, DocsumPacket packet, String summaryClass) {
         if (packet != null) {
             byte[] docsumdata = packet.getData();
             if (docsumdata.length > 0) {
-                decodeSummary(summaryClass, hit, docsumdata);
-                return true;
+                return new FillHitResult(true, decodeSummary(summaryClass, hit, docsumdata));
             }
         }
-        return false;
+        return new FillHitResult(false);
     }
 
+    static protected class FillHitsResult {
+        public final int skippedHits; // Number of hits not producing a summary.
+        public final String error; // Optional error message
+        FillHitsResult(int skippedHits, String error) {
+            this.skippedHits = skippedHits;
+            this.error = error;
+        }
+    }
     /**
      * Fills the hits.
      *
-     * @return the number of hits that we did not return data for, i.e
+     * @return the number of hits that we did not return data for, and an optional error message.
      *         when things are working normally we return 0.
      */
-    protected int fillHits(Result result, int packetIndex, Packet[] packets, String summaryClass) throws IOException {
+    protected FillHitsResult fillHits(Result result, int packetIndex, Packet[] packets, String summaryClass) throws IOException {
         int skippedHits=0;
+        String lastError = null;
         for (Iterator<Hit> i = hitIterator(result); i.hasNext();) {
             Hit hit = i.next();
 
@@ -500,12 +519,19 @@ public abstract class VespaBackEndSearcher extends PingableSearcher {
                 DocsumPacket docsum = (DocsumPacket) packets[packetIndex];
 
                 packetIndex++;
-                if ( ! fillHit(fastHit, docsum, summaryClass))
+                FillHitResult fr = fillHit(fastHit, docsum, summaryClass);
+                if ( ! fr.ok ) {
                     skippedHits++;
+                }
+                if (fr.error != null) {
+                    result.hits().addError(ErrorMessage.createTimeout(fr.error));
+                    skippedHits++;
+                    lastError = fr.error;
+                }
             }
         }
         result.hits().setSorted(false);
-        return skippedHits;
+        return new FillHitsResult(skippedHits, lastError);
     }
 
     /**
@@ -548,7 +574,10 @@ public abstract class VespaBackEndSearcher extends PingableSearcher {
                 byte[] docsumdata = docsum.getData();
 
                 if (docsumdata.length > 0) {
-                    decodeSummary(summaryClass, hit, docsumdata);
+                    String error = decodeSummary(summaryClass, hit, docsumdata);
+                    if (error != null) {
+                        filledAllOfEm = false;
+                    }
                 } else {
                     filledAllOfEm = false;
                 }
@@ -595,9 +624,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher {
                     FastHit fastHit = (FastHit) hit;
                     DocsumPacketKey key = new DocsumPacketKey(fastHit.getGlobalId(), fastHit.getPartId(), summaryClass);
 
-                    if (fillHit(fastHit,
-                            (DocsumPacket) packetWrapper.getPacket(key),
-                            summaryClass)) {
+                    if (fillHit(fastHit, (DocsumPacket) packetWrapper.getPacket(key), summaryClass).ok) {
                         fastHit.setCached(true);
                     }
 
@@ -615,15 +642,18 @@ public abstract class VespaBackEndSearcher extends PingableSearcher {
         return db.getDocsumDefinitionSet();
     }
 
-    private void decodeSummary(String summaryClass, FastHit hit, byte[] docsumdata) {
+    private String decodeSummary(String summaryClass, FastHit hit, byte[] docsumdata) {
         DocumentDatabase db = getDocumentDatabase(hit.getQuery());
         hit.setField(Hit.SDDOCNAME_FIELD, db.getName());
-        decodeSummary(summaryClass, hit, docsumdata, db.getDocsumDefinitionSet());
+        return decodeSummary(summaryClass, hit, docsumdata, db.getDocsumDefinitionSet());
     }
 
-    private void decodeSummary(String summaryClass, FastHit hit, byte[] docsumdata, DocsumDefinitionSet docsumSet) {
-        docsumSet.lazyDecode(summaryClass, docsumdata, hit);
-        hit.setFilled(summaryClass);
+    private String decodeSummary(String summaryClass, FastHit hit, byte[] docsumdata, DocsumDefinitionSet docsumSet) {
+        String error = docsumSet.lazyDecode(summaryClass, docsumdata, hit);
+        if (error == null) {
+            hit.setFilled(summaryClass);
+        }
+        return error;
     }
 
     /**
diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java
index 207f532f99a..7a390ac279b 100644
--- a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java
+++ b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java
@@ -8,6 +8,8 @@ import com.yahoo.component.AbstractComponent;
 import com.yahoo.compress.CompressionType;
 import com.yahoo.compress.Compressor;
 import com.yahoo.container.handler.VipStatus;
+import com.yahoo.container.protect.Error;
+import com.yahoo.slime.ArrayTraverser;
 import com.yahoo.data.access.slime.SlimeAdapter;
 import com.yahoo.prelude.fastsearch.FS4ResourcePool;
 import com.yahoo.prelude.fastsearch.FastHit;
@@ -17,12 +19,15 @@ import com.yahoo.search.Result;
 import com.yahoo.search.query.SessionId;
 import com.yahoo.search.result.ErrorMessage;
 import com.yahoo.search.result.Hit;
+import com.yahoo.data.access.Inspector;
 import com.yahoo.slime.BinaryFormat;
 import com.yahoo.slime.Cursor;
+import com.yahoo.slime.JsonFormat;
 import com.yahoo.slime.Slime;
-import com.yahoo.data.access.Inspector;
 import com.yahoo.vespa.config.search.DispatchConfig;
 
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -221,13 +226,31 @@ public class Dispatcher extends AbstractComponent {
             }
         }
 
+        private void addErrors(com.yahoo.slime.Inspector errors) {
+            errors.traverse((ArrayTraverser) (int index, com.yahoo.slime.Inspector value) -> {
+                int errorCode = ("timeout".equalsIgnoreCase(value.field("type").asString()))
+                        ? Error.TIMEOUT.code
+                        : Error.UNSPECIFIED.code;
+                result.hits().addError(new ErrorMessage(errorCode,
+                        value.field("message").asString(), value.field("details").asString()));
+            });
+        }
+
         private void fill(List<FastHit> hits, byte[] slimeBytes) {
-            Inspector summaries = new SlimeAdapter(BinaryFormat.decode(slimeBytes).get().field("docsums"));
-            if ( ! summaries.valid())
+            com.yahoo.slime.Inspector root = BinaryFormat.decode(slimeBytes).get();
+            com.yahoo.slime.Inspector errors = root.field("errors");
+            boolean hasErrors = errors.valid() && (errors.entries() > 0);
+            if (hasErrors) {
+                addErrors(errors);
+            }
+
+            Inspector summaries = new SlimeAdapter(root.field("docsums"));
+            if ( ! summaries.valid() && !hasErrors)
                 throw new IllegalArgumentException("Expected a Slime root object containing a 'docsums' field");
             for (int i = 0; i < hits.size(); i++) {
                 fill(hits.get(i), summaries.entry(i).field("docsum"));
             }
+
         }
 
         private void fill(FastHit hit, Inspector summary) {
diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java
index c2a2d160c49..4ebe29cdacd 100644
--- a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java
+++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java
@@ -194,10 +194,17 @@ public class VdsStreamingSearcher extends VespaBackEndSearcher {
 
         int skippedHits;
         try {
-            skippedHits = fillHits(result, 0, summaryPackets, query.getPresentation().getSummary());
+            FillHitsResult fillHitsResult = fillHits(result, 0, summaryPackets, query.getPresentation().getSummary());
+            skippedHits = fillHitsResult.skippedHits;
+            if (fillHitsResult.error != null) {
+                result.hits().addError(ErrorMessage.createTimeout(fillHitsResult.error));
+                return result;
+            }
+        } catch (TimeoutException e) {
+            result.hits().addError(ErrorMessage.createTimeout(e.getMessage()));
+            return result;
         } catch (IOException e) {
-            return new Result(query, ErrorMessage.createBackendCommunicationError(
-                    "Error filling hits with summary fields"));
+            return new Result(query, ErrorMessage.createBackendCommunicationError("Error filling hits with summary fields"));
         }
 
         if (skippedHits==0) {
diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java
index 5494d1965f8..c2a50884b2b 100644
--- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java
@@ -33,7 +33,7 @@ public class SlimeSummaryTestCase {
         DocsumDefinitionSet set = createDocsumDefinitionSet(summary_cf, emul);
         byte[] docsum = makeEmptyDocsum();
         FastHit hit = new FastHit();
-        set.lazyDecode("default", docsum, hit);
+        assertNull(set.lazyDecode("default", docsum, hit));
         assertThat(hit.getField("integer_field"), equalTo(NanNumber.NaN));
         assertThat(hit.getField("short_field"),   equalTo(NanNumber.NaN));
         assertThat(hit.getField("byte_field"),    equalTo(NanNumber.NaN));
@@ -66,7 +66,7 @@ public class SlimeSummaryTestCase {
         DocsumDefinitionSet set = createDocsumDefinitionSet(summary_cf, new LegacyEmulationConfig(new LegacyEmulationConfig.Builder().forceFillEmptyFields(false)));
         byte[] docsum = makeEmptyDocsum();
         FastHit hit = new FastHit();
-        set.lazyDecode("default", docsum, hit);
+        assertNull(set.lazyDecode("default", docsum, hit));
         assertThat(hit.getField("integer_field"), equalTo(null));
         assertThat(hit.getField("short_field"),   equalTo(null));
         assertThat(hit.getField("byte_field"),    equalTo(null));
@@ -89,6 +89,9 @@ public class SlimeSummaryTestCase {
     private byte[] makeEmptyDocsum() {
         Slime slime = new Slime();
         Cursor docsum = slime.setObject();
+        return encode((slime));
+    }
+    private byte [] encode(Slime slime) {
         byte[] tmp = BinaryFormat.encode(slime);
         ByteBuffer buf = ByteBuffer.allocate(tmp.length + 4);
         buf.order(ByteOrder.LITTLE_ENDIAN);
@@ -99,6 +102,15 @@ public class SlimeSummaryTestCase {
     }
 
     @Test
+    public void testTimeout() {
+        String summary_cf = "file:src/test/java/com/yahoo/prelude/fastsearch/summary.cfg";
+        DocsumDefinitionSet set = createDocsumDefinitionSet(summary_cf);
+        byte[] docsum = makeTimeout();
+        FastHit hit = new FastHit();
+        assertEquals("Hit hit index:0/0/0/000000000000000000000000 (relevance null) [fasthit, globalid: 0 0 0 0 0 0 0 0 0 0 0 0, partId: 0, distributionkey: 0] failed: Timed out....", set.lazyDecode("default", docsum, hit));
+    }
+
+    @Test
     public void testDecoding() {
         Tensor tensor1 = Tensor.from("tensor(x{},y{}):{{x:foo,y:bar}:0.1}");
         Tensor tensor2 = Tensor.from("tensor(x[],y[1]):{{x:0,y:0}:-0.3}");
@@ -107,7 +119,7 @@ public class SlimeSummaryTestCase {
         DocsumDefinitionSet set = createDocsumDefinitionSet(summary_cf);
         byte[] docsum = makeDocsum(tensor1, tensor2);
         FastHit hit = new FastHit();
-        set.lazyDecode("default", docsum, hit);
+        assertNull(set.lazyDecode("default", docsum, hit));
         assertThat(hit.getField("integer_field"), equalTo(4));
         assertThat(hit.getField("short_field"),   equalTo((short)2));
         assertThat(hit.getField("byte_field"),    equalTo((byte)1));
@@ -170,13 +182,13 @@ public class SlimeSummaryTestCase {
         }
         docsum.setData("tensor_field1", TypedBinaryFormat.encode(tensor1));
         docsum.setData("tensor_field2", TypedBinaryFormat.encode(tensor2));
-        byte[] tmp = BinaryFormat.encode(slime);
-        ByteBuffer buf = ByteBuffer.allocate(tmp.length + 4);
-        buf.order(ByteOrder.LITTLE_ENDIAN);
-        buf.putInt(DocsumDefinitionSet.SLIME_MAGIC_ID);
-        buf.order(ByteOrder.BIG_ENDIAN);
-        buf.put(tmp);
-        return buf.array();
+        return encode((slime));
+    }
+
+    private byte [] makeTimeout() {
+        Slime slime = new Slime();
+        slime.setString("Timed out....");
+        return encode((slime));
     }
 
 }