Use schematic 'type', 'message', 'details' error schema.

5 files changed, 84 insertions, 51 deletions

diff --git a/searchcore/src/tests/proton/docsummary/docsummary.cpp b/searchcore/src/tests/proton/docsummary/docsummary.cpp
index ff393682f9b..518ec07330a 100644
--- a/searchcore/src/tests/proton/docsummary/docsummary.cpp
+++ b/searchcore/src/tests/proton/docsummary/docsummary.cpp
@@ -57,6 +57,7 @@ using vespalib::tensor::Tensor;
 using vespalib::tensor::TensorCells;
 using vespalib::tensor::TensorDimensions;
 using vespalib::tensor::TensorFactory;
+using namespace vespalib::slime;
 
 typedef std::unique_ptr<GeneralResult> GeneralResultPtr;
 
@@ -143,18 +144,18 @@ Tensor::UP createTensor(const TensorCells &cells,
 vespalib::string asVstring(vespalib::Memory str) {
     return vespalib::string(str.data, str.size);
 }
-vespalib::string asVstring(const vespalib::slime::Inspector &value) {
+vespalib::string asVstring(const Inspector &value) {
     return asVstring(value.asString());
 }
 
 void decode(const ResEntry *entry, vespalib::Slime &slime) {
     vespalib::Memory mem(entry->_dataval,
                          entry->_datalen);
-    size_t decodeRes = vespalib::slime::BinaryFormat::decode(mem, slime);
+    size_t decodeRes = BinaryFormat::decode(mem, slime);
     ASSERT_EQUAL(decodeRes, mem.size);
 }
 
-std::string b64encode(const vespalib::slime::Inspector &value) {
+std::string b64encode(const Inspector &value) {
     vespalib::Memory mem = value.asData();
     std::string str(mem.data, mem.size);
     return vespalib::Base64::encode(str);
@@ -274,12 +275,6 @@ private:
     ResultConfig          _resultCfg;
     std::set<vespalib::string> _markupFields;
 
-    const vespa::config::search::SummaryConfig &
-    getSummaryConfig() const
-    {
-        return *_summaryCfg;
-    }
-
     const ResultConfig &getResultConfig() const
     {
         return _resultCfg;
@@ -335,6 +330,7 @@ private:
     void requireThatPositionsAreUsed();
     void requireThatRawFieldsWorks();
     void requireThatFieldCacheRepoCanReturnDefaultFieldCache();
+    void requireThatSummariesTimeout();
 
 public:
     Test();
@@ -409,7 +405,7 @@ Test::assertTensor(const Tensor::UP & exp, const std::string & fieldName,
     vespalib::Slime slime;
     vespalib::Memory serialized(docsum.data.c_str() + sizeof(classId),
                                 docsum.data.size() - sizeof(classId));
-    size_t decodeRes = vespalib::slime::BinaryFormat::decode(serialized,
+    size_t decodeRes = BinaryFormat::decode(serialized,
                                                              slime);
     ASSERT_EQUAL(decodeRes, serialized.size);
 
@@ -424,9 +420,8 @@ Test::assertTensor(const Tensor::UP & exp, const std::string & fieldName,
     }
 }
 
-
-bool
-Test::assertSlime(const std::string &exp, const DocsumReply &reply, uint32_t id, bool relaxed)
+vespalib::Slime
+getSlime(const DocsumReply &reply, uint32_t id, bool relaxed)
 {
     const DocsumReply::Docsum & docsum = reply.docsums[id];
     uint32_t classId;
@@ -435,20 +430,26 @@ Test::assertSlime(const std::string &exp, const DocsumReply &reply, uint32_t id,
     ASSERT_EQUAL(::search::fs4transport::SLIME_MAGIC_ID, classId);
     vespalib::Slime slime;
     vespalib::Memory serialized(docsum.data.c_str() + sizeof(classId),
-                                       docsum.data.size() - sizeof(classId));
-    size_t decodeRes = vespalib::slime::BinaryFormat::decode(serialized,
-                                                             slime);
+                                docsum.data.size() - sizeof(classId));
+    size_t decodeRes = BinaryFormat::decode(serialized, slime);
     ASSERT_EQUAL(decodeRes, serialized.size);
     if (relaxed) {
         vespalib::SimpleBuffer buf;
-        vespalib::slime::JsonFormat::encode(slime, buf, false);
+        JsonFormat::encode(slime, buf, false);
         vespalib::Slime tmpSlime;
-        size_t used = vespalib::slime::JsonFormat::decode(buf.get(), tmpSlime);
+        size_t used = JsonFormat::decode(buf.get(), tmpSlime);
         EXPECT_TRUE(used > 0);
         slime = std::move(tmpSlime);
     }
+    return slime;
+}
+
+bool
+Test::assertSlime(const std::string &exp, const DocsumReply &reply, uint32_t id, bool relaxed)
+{
+    vespalib::Slime slime = getSlime(reply, id, relaxed);
     vespalib::Slime expSlime;
-    size_t used = vespalib::slime::JsonFormat::decode(exp, expSlime);
+    size_t used = JsonFormat::decode(exp, expSlime);
     EXPECT_TRUE(used > 0);
     return EXPECT_EQUAL(expSlime, slime);
 }
@@ -664,9 +665,40 @@ Test::requireThatRewritersAreUsed()
     req.hits.push_back(DocsumRequest::Hit(gid1));
     DocsumReply::UP rep = dc._ddb->getDocsums(req);
     EXPECT_EQUAL(1u, rep->docsums.size());
-    EXPECT_TRUE(assertSlime("{aa:20}", *rep, 0, false));
+    vespalib::SimpleBuffer buf;
+    vespalib::Slime summary = getSlime(*rep, 0, false);
+    JsonFormat::encode(summary, buf, false);
+    EXPECT_TRUE(vespalib::Regexp("Timed out with -[0-9]+us left.").match(buf.get().make_stringref()));
 }
 
+void
+Test::requireThatSummariesTimeout()
+{
+    Schema s;
+    s.addSummaryField(Schema::SummaryField("aa", schema::DataType::INT32));
+    s.addSummaryField(Schema::SummaryField("ab", schema::DataType::INT32));
+
+    BuildContext bc(s);
+    DBContext dc(bc._repo, getDocTypeName());
+    dc.put(*bc._bld.startDocument("doc::1").
+                   startSummaryField("aa").
+                   addInt(10).
+                   endField().
+                   startSummaryField("ab").
+                   addInt(20).
+                   endField().
+                   endDocument(),
+           1);
+
+    DocsumRequest req;
+    req.setTimeout(0);
+    EXPECT_TRUE(req.expired());
+    req.resultClassName = "class2";
+    req.hits.push_back(DocsumRequest::Hit(gid1));
+    DocsumReply::UP rep = dc._ddb->getDocsums(req);
+    EXPECT_EQUAL(1u, rep->docsums.size());
+    EXPECT_TRUE(assertSlime("{aa:20}", *rep, 0, false));
+}
 
 void
 addField(Schema & s,
@@ -1288,6 +1320,7 @@ Test::Main()
     TEST_DO(requireThatPositionsAreUsed());
     TEST_DO(requireThatRawFieldsWorks());
     TEST_DO(requireThatFieldCacheRepoCanReturnDefaultFieldCache());
+    TEST_DO(requireThatSummariesTimeout());
 
     TEST_DONE();
 }
diff --git a/searchcore/src/tests/proton/summaryengine/summaryengine.cpp b/searchcore/src/tests/proton/summaryengine/summaryengine.cpp
index 4951b1cd569..408ca27c16e 100644
--- a/searchcore/src/tests/proton/summaryengine/summaryengine.cpp
+++ b/searchcore/src/tests/proton/summaryengine/summaryengine.cpp
@@ -9,9 +9,6 @@
 #include <vespa/vespalib/util/compressor.h>
 #include <vespa/searchlib/common/transport.h>
 #include <vespa/fnet/frt/rpcrequest.h>
-#include <mutex>
-#include <condition_variable>
-#include <chrono>
 
 #include <vespa/log/log.h>
 LOG_SETUP("summaryengine_test");
diff --git a/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp b/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp
index 7cc2a724707..da936a9d7af 100644
--- a/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp
+++ b/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp
@@ -7,6 +7,7 @@
 #include <vespa/searchlib/common/transport.h>
 #include <vespa/vespalib/data/slime/slime.h>
 #include <vespa/vespalib/util/stringfmt.h>
+
 #include <vespa/log/log.h>
 LOG_SETUP(".proton.docsummary.docsumcontext");
 
@@ -21,6 +22,7 @@ using vespalib::slime::Symbol;
 using vespalib::slime::Inserter;
 using vespalib::slime::ObjectSymbolInserter;
 using vespalib::Slime;
+using vespalib::make_string;
 using namespace search;
 using namespace search::attribute;
 using namespace search::engine;
@@ -34,10 +36,11 @@ namespace {
 
 Memory DOCSUMS("docsums");
 Memory DOCSUM("docsum");
-
-void addTimedOut(Inserter & inserter, fastos::TimeStamp left) {
-    inserter.insertObject().setString("error", vespalib::make_string("Timed out with %ldus left.", left.us()));
-}
+Memory ERRORS("errors");
+Memory TYPE("type");
+Memory MESSAGE("message");
+Memory DETAILS("details");
+Memory TIMEOUT("timeout");
 
 }
 
@@ -49,11 +52,10 @@ DocsumContext::initState()
     _docsumState._args.SetQueryFlags(req.queryFlags & ~search::fs4transport::QFLAG_DROP_SORTDATA);
     _docsumState._docsumcnt = req.hits.size();
 
-    if (_docsumState._docsumcnt > 0) {
-        _docsumState._docsumbuf = (uint32_t*)malloc(sizeof(uint32_t) * _docsumState._docsumcnt);
-    } else {
-        _docsumState._docsumbuf = NULL;
-    }
+    _docsumState._docsumbuf = (_docsumState._docsumcnt > 0)
+                              ? (uint32_t*)malloc(sizeof(uint32_t) * _docsumState._docsumcnt)
+                              : nullptr;
+
     for (uint32_t i = 0; i < _docsumState._docsumcnt; i++) {
         _docsumState._docsumbuf[i] = req.hits[i].docid;
     }
@@ -76,7 +78,7 @@ DocsumContext::createReply()
             Slime slime(Slime::Params(std::move(symbols)));
             vespalib::slime::SlimeInserter inserter(slime);
             if (_request.expired()) {
-                addTimedOut(inserter, _request.getTimeLeft());
+                inserter.insertString(make_string("Timed out with %ldus left.", _request.getTimeLeft().us()));
             } else {
                 _docsumWriter.insertDocsum(rci, docId, &_docsumState, &_docsumStore, slime, inserter);
             }
@@ -110,30 +112,32 @@ DocsumContext::createSlimeReply()
     Cursor & root = response->setObject();
     Cursor & array = root.setArray(DOCSUMS);
     const Symbol docsumSym = response->insert(DOCSUM);
-    IDocsumWriter::ResolveClassInfo rci = _docsumWriter.resolveClassInfo(_docsumState._args.getResultClassName(), _docsumStore.getSummaryClassId());
-    for (uint32_t i = 0; (i < _docsumState._docsumcnt); ++i) {
+    IDocsumWriter::ResolveClassInfo rci = _docsumWriter.resolveClassInfo(_docsumState._args.getResultClassName(),
+                                                                         _docsumStore.getSummaryClassId());
+    uint32_t i(0);
+    for (i = 0; (i < _docsumState._docsumcnt) && !_request.expired(); ++i) {
         uint32_t docId = _docsumState._docsumbuf[i];
         Cursor & docSumC = array.addObject();
         ObjectSymbolInserter inserter(docSumC, docsumSym);
         if ((docId != search::endDocId) && !rci.mustSkip) {
-            if (_request.expired()) {
-                addTimedOut(inserter, _request.getTimeLeft());
-            } else {
-                _docsumWriter.insertDocsum(rci, docId, &_docsumState, &_docsumStore, *response, inserter);
-            }
+            _docsumWriter.insertDocsum(rci, docId, &_docsumState, &_docsumStore, *response, inserter);
         }
     }
+    if (i != _docsumState._docsumcnt) {
+        const uint32_t numTimedOut = _docsumState._docsumcnt - 1;
+        Cursor & errors = root.setArray(ERRORS);
+        Cursor & timeout = errors.addObject();
+        timeout.setString(TYPE, TIMEOUT);
+        timeout.setString(MESSAGE, make_string("Timed out %d summaries with %ldus left.",
+                                               numTimedOut, _request.getTimeLeft().us()));
+    }
     return response;
 }
 
-DocsumContext::DocsumContext(const DocsumRequest & request,
-                             IDocsumWriter & docsumWriter,
-                             IDocsumStore & docsumStore,
-                             const Matcher::SP & matcher,
-                             ISearchContext & searchCtx,
-                             IAttributeContext & attrCtx,
-                             search::IAttributeManager & attrMgr,
-                             SessionManager & sessionMgr) :
+DocsumContext::DocsumContext(const DocsumRequest & request, IDocsumWriter & docsumWriter,
+                             IDocsumStore & docsumStore, const Matcher::SP & matcher,
+                             ISearchContext & searchCtx, IAttributeContext & attrCtx,
+                             search::IAttributeManager & attrMgr, SessionManager & sessionMgr) :
     _request(request),
     _docsumWriter(docsumWriter),
     _docsumStore(docsumStore),
@@ -195,8 +199,7 @@ Location *getLocation(const string &loc_str, search::IAttributeManager &attrMgr)
         loc->setVecGuard(std::move(vec));
         location = loc_str.substr(pos + 1);
     } else {
-        LOG(warning, "Location string lacks attribute vector specification. "
-            "loc='%s'", loc_str.c_str());
+        LOG(warning, "Location string lacks attribute vector specification. loc='%s'", loc_str.c_str());
         location = loc_str;
     }
     loc->parse(location);
diff --git a/searchcore/src/vespa/searchcore/proton/docsummary/documentstoreadapter.cpp b/searchcore/src/vespa/searchcore/proton/docsummary/documentstoreadapter.cpp
index eb1302f9d4d..85ae621db48 100644
--- a/searchcore/src/vespa/searchcore/proton/docsummary/documentstoreadapter.cpp
+++ b/searchcore/src/vespa/searchcore/proton/docsummary/documentstoreadapter.cpp
@@ -7,6 +7,7 @@
 #include <vespa/eval/tensor/serialization/typed_binary_format.h>
 #include <vespa/vespalib/objects/nbostream.h>
 #include <vespa/document/fieldvalue/tensorfieldvalue.h>
+
 #include <vespa/log/log.h>
 LOG_SETUP(".proton.docsummary.documentstoreadapter");
 
diff --git a/searchcore/src/vespa/searchcore/proton/server/rpc_hooks.cpp b/searchcore/src/vespa/searchcore/proton/server/rpc_hooks.cpp
index 913efa469d3..b7210988de2 100644
--- a/searchcore/src/vespa/searchcore/proton/server/rpc_hooks.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/rpc_hooks.cpp
@@ -4,7 +4,6 @@
 #include "proton.h"
 #include <vespa/vespalib/util/closuretask.h>
 #include <vespa/fnet/frt/supervisor.h>
-#include <chrono>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".proton.server.rtchooks");