Skip to content

Commit

Permalink
Merge pull request #33100 from vespa-engine/toregge/report-issues-for…
Browse files Browse the repository at this point in the history
…-streaming-search-instead-of-logging-warnings

Report issues for streaming search instead of logging warnings.
  • Loading branch information
toregge authored Jan 10, 2025
2 parents cba6b5a + 82a1c29 commit 1a416d2
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 35 deletions.
10 changes: 6 additions & 4 deletions streamingvisitors/src/vespa/searchvisitor/queryenvironment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <vespa/searchlib/common/geo_location.h>
#include <vespa/searchlib/common/geo_location_spec.h>
#include <vespa/searchlib/common/geo_location_parser.h>
#include <vespa/vespalib/util/issue.h>

#include <vespa/log/log.h>
LOG_SETUP(".searchvisitor.queryenvironment");
Expand All @@ -13,6 +14,7 @@ using search::common::GeoLocationParser;
using search::common::GeoLocationSpec;
using search::fef::Properties;
using std::string;
using vespalib::Issue;

namespace streaming {

Expand All @@ -27,8 +29,8 @@ parseLocation(const string & location_str)
}
GeoLocationParser locationParser;
if (!locationParser.parseWithField(location_str)) {
LOG(warning, "Location parse error (location: '%s'): %s. Location ignored.",
location_str.c_str(), locationParser.getParseError());
Issue::report("Location parse error (location: '%s'): %s. Location ignored.",
location_str.c_str(), locationParser.getParseError());
return fefLocations;
}
auto loc = locationParser.getGeoLocation();
Expand Down Expand Up @@ -57,8 +59,8 @@ QueryEnvironment::~QueryEnvironment() {}
void QueryEnvironment::addGeoLocation(const std::string &field, const std::string &location_str) {
GeoLocationParser locationParser;
if (! locationParser.parseNoField(location_str)) {
LOG(warning, "Location parse error (location: '%s'): %s. Location ignored.",
location_str.c_str(), locationParser.getParseError());
Issue::report("Location parse error (location: '%s'): %s. Location ignored.",
location_str.c_str(), locationParser.getParseError());
return;
}
auto loc = locationParser.getGeoLocation();
Expand Down
10 changes: 6 additions & 4 deletions streamingvisitors/src/vespa/searchvisitor/rankprocessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <vespa/searchlib/query/streaming/nearest_neighbor_query_node.h>
#include <vespa/vsm/vsm/fieldsearchspec.h>
#include <vespa/vespalib/stllike/hash_set.h>
#include <vespa/vespalib/util/issue.h>
#include <algorithm>
#include <cmath>
#include <vespa/log/log.h>
Expand All @@ -32,6 +33,7 @@ using search::streaming::MultiTerm;
using search::streaming::Query;
using search::streaming::QueryTerm;
using search::streaming::QueryTermList;
using vespalib::Issue;
using vdslib::SearchResult;

namespace streaming {
Expand Down Expand Up @@ -68,8 +70,8 @@ RankProcessor::resolve_fields_from_children(QueryTermData& qtd, const MultiTerm&
field_ids.insert(field_id);
}
} else {
LOG(warning, "Could not find a view for index '%s'. Ranking no fields.",
getIndexName(subterm->index(), expandedIndexName).c_str());
Issue::report("Could not find a view for index '%s'. Ranking no fields.",
getIndexName(subterm->index(), expandedIndexName).c_str());
}
}
std::vector<uint32_t> sorted_field_ids;
Expand All @@ -93,8 +95,8 @@ RankProcessor::resolve_fields_from_term(QueryTermData& qtd, const search::stream
qtd.getTermData().addField(field_id).setHandle(_mdLayout.allocTermField(field_id));
}
} else {
LOG(warning, "Could not find a view for index '%s'. Ranking no fields.",
getIndexName(term.index(), expandedIndexName).c_str());
Issue::report("Could not find a view for index '%s'. Ranking no fields.",
getIndexName(term.index(), expandedIndexName).c_str());
}
LOG(debug, "Setup query term '%s:%s'",
getIndexName(term.index(), expandedIndexName).c_str(),
Expand Down
36 changes: 18 additions & 18 deletions streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ SearchVisitor::init(const Parameters & params)
for (uint32_t i = 0; i < cnt; ++i) {
search::fs4transport::FS4Properties prop;
if (!prop.decode(src, len)) {
LOG(warning, "Could not decode rank properties");
Issue::report("Could not decode rank properties");
} else {
LOG(debug, "Properties[%u]: name '%s', size '%u'", i, prop.name().c_str(), prop.size());
if (prop.name() == "rank") { // pick up rank properties
Expand Down Expand Up @@ -516,7 +516,7 @@ SearchVisitor::init(const Parameters & params)
if (params.get("querystackcount", stackCount)) {
_summaryGenerator.set_stack_dump(std::vector<char>(queryBlob.begin(), queryBlob.end()));
} else {
LOG(warning, "Request without query stack count");
Issue::report("Request without query stack count");
}

StringFieldIdTMap fieldsInQuery = setupFieldSearchers();
Expand All @@ -540,7 +540,7 @@ SearchVisitor::init(const Parameters & params)
// and IQueryEnvironment (from setupRankProcessors).
prepare_field_searchers();
} else {
LOG(warning, "No query received");
Issue::report("No query received");
}

if (hasGrouping) {
Expand All @@ -552,7 +552,7 @@ SearchVisitor::init(const Parameters & params)
}

} else {
LOG(warning, "No searchcluster specified");
Issue::report("No searchcluster specified");
}

if ( params.lookup("unique", valueRef) ) {
Expand Down Expand Up @@ -674,13 +674,13 @@ SearchVisitor::RankController::processAccessedAttributes(const QueryEnvironment
LOG(debug, "Add attribute '%s' with field id '%u' to the list of needed attributes", name.c_str(), fid);
attributeFields.emplace_back(fid, std::move(attr));
} else {
LOG(warning, "Cannot locate attribute '%s' in the attribute manager. "
"Ignore access hint about this attribute", name.c_str());
Issue::report("Cannot locate attribute '%s' in the attribute manager. "
"Ignore access hint about this attribute", name.c_str());
}
}
} else {
LOG(warning, "Cannot locate field '%s' in the index environment. Ignore access hint about this attribute",
name.c_str());
Issue::report("Cannot locate field '%s' in the index environment. Ignore access hint about this attribute",
name.c_str());
}
}
}
Expand Down Expand Up @@ -902,8 +902,8 @@ SearchVisitor::setupScratchDocument(const StringFieldIdTMap & fieldsInQuery)
}
// Setup document type mapping
if (_fieldSearchSpecMap.documentTypeMap().size() != 1) {
LOG(warning, "We have %zd document types in the vsmfields config when we expected 1. Using the first one",
_fieldSearchSpecMap.documentTypeMap().size());
Issue::report("We have %zd document types in the vsmfields config when we expected 1. Using the first one",
_fieldSearchSpecMap.documentTypeMap().size());
}
_fieldsUnion = fieldsInQuery.map();
for(const auto & entry : _fieldSearchSpecMap.nameIdMap().map()) {
Expand All @@ -930,7 +930,7 @@ SearchVisitor::setupDocsumObjects()
if (docsum_tools) {
_summaryGenerator.setDocsumWriter(*docsum_tools->getDocsumWriter());
} else {
LOG(warning, "No docsum tools available");
Issue::report("No docsum tools available");
}
}

Expand Down Expand Up @@ -977,8 +977,8 @@ void SearchVisitor::setupAttributeVector(const FieldPath &fieldPath) {
LOG(debug, "Adding attribute '%s' for field '%s' with data type '%s' (%s)",
attr->getName().c_str(), attrName.c_str(), fv.getDataType()->getName().c_str(), fv.className());
if ( ! _attrMan.add(attr) ) {
LOG(warning, "Failed adding attribute '%s' for field '%s' with data type '%s' (%s)",
attr->getName().c_str(), attrName.c_str(), fv.getDataType()->getName().c_str(), fv.className());
Issue::report("Failed adding attribute '%s' for field '%s' with data type '%s' (%s)",
attr->getName().c_str(), attrName.c_str(), fv.getDataType()->getName().c_str(), fv.className());
}
} else {
LOG(debug, "Cannot setup attribute for field '%s' with data type '%s' (%s). Aggregation and sorting will not work for this field",
Expand Down Expand Up @@ -1060,12 +1060,12 @@ SearchVisitor::setupGrouping(const std::vector<char> & groupingBlob)
if (!grouping.getAll() || (preparator.getNumHitsAggregators() == 0)) {
_groupingList.push_back(groupingPtr);
} else {
LOG(warning, "You can not collect hits with an all aggregator yet.");
Issue::report("You can not collect hits with an all aggregator yet.");
}
} catch (const document::FieldNotFoundException & e) {
LOG(warning, "Could not locate field for grouping number %ld : %s", i, e.getMessage().c_str());
Issue::report("Could not locate field for grouping number %ld : %s", i, e.getMessage().c_str());
} catch (const std::exception & e) {
LOG(error, "Unknown issue for grouping number %ld : %s", i, e.what());
Issue::report("Unknown issue for grouping number %ld : %s", i, e.what());
}
}
}
Expand Down Expand Up @@ -1118,8 +1118,8 @@ SearchVisitor::handleDocuments(const document::BucketId&, DocEntryList & entries
handleDocument(document);
}
} catch (const std::exception & e) {
LOG(warning, "Caught exception handling document '%s'. Exception='%s'",
document->docDoc().getId().getScheme().toString().c_str(), e.what());
Issue::report("Caught exception handling document '%s'. Exception='%s'",
document->docDoc().getId().getScheme().toString().c_str(), e.what());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
#include <vespa/document/repo/documenttyperepo.h>
#include <vespa/document/datatype/documenttype.h>
#include <vespa/vespalib/stllike/hash_map.hpp>
#include <vespa/vespalib/util/issue.h>

#include <vespa/log/log.h>
LOG_SETUP(".vsm.common.documenttypemapping");

using vespalib::Issue;

namespace vsm {

DocumentTypeMapping::DocumentTypeMapping() :
Expand Down Expand Up @@ -49,8 +52,8 @@ bool DocumentTypeMapping::prepareBaseDoc(SharedFieldPathMap & map) const
LOG(debug, "Found FieldPathMap for default document type '%s' with %zd elements",
_defaultDocumentTypeName.c_str(), map->size());
} else {
LOG(warning, "No FieldPathMap found for default document type '%s'. Using empty one",
_defaultDocumentTypeName.c_str());
Issue::report("No FieldPathMap found for default document type '%s'. Using empty one",
_defaultDocumentTypeName.c_str());
map = std::make_shared<FieldPathMapT>();
}
return true;
Expand Down
11 changes: 7 additions & 4 deletions streamingvisitors/src/vespa/vsm/vsm/docsumfilter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <vespa/document/datatype/datatype.h>
#include <vespa/document/fieldvalue/stringfieldvalue.h>
#include <vespa/vespalib/data/slime/inserter.h>
#include <vespa/vespalib/util/issue.h>
#include <cassert>

#include <vespa/log/log.h>
Expand All @@ -20,6 +21,8 @@ using namespace search::docsummary;
using document::FieldPathEntry;
using FieldMap = vsm::StringFieldIdTMap;

using vespalib::Issue;

namespace vsm {

namespace {
Expand Down Expand Up @@ -80,11 +83,11 @@ prepareFieldSpec(DocsumFieldSpec & spec, const DocsumTools::FieldSpec & toolsSpe
spec.set_struct_or_multivalue(true);
}
} else {
LOG(warning, "Could not find a field path for field '%s' with id '%d'", name.c_str(), field);
Issue::report("Could not find a field path for field '%s' with id '%d'", name.c_str(), field);
spec.setOutputField(DocsumFieldSpec::FieldIdentifier(field, FieldPath()));
}
} else {
LOG(warning, "Could not find output summary field '%s'", name.c_str());
Issue::report("Could not find output summary field '%s'", name.c_str());
}
}
// setup input fields
Expand All @@ -100,11 +103,11 @@ prepareFieldSpec(DocsumFieldSpec & spec, const DocsumTools::FieldSpec & toolsSpe
LOG(debug, "field %u < map size %zu", field, fieldPathMap.size());
spec.getInputFields().push_back(DocsumFieldSpec::FieldIdentifier(field, copyPathButFirst(fieldPathMap[field])));
} else {
LOG(warning, "Could not find a field path for field '%s' with id '%d'", name.c_str(), field);
Issue::report("Could not find a field path for field '%s' with id '%d'", name.c_str(), field);
spec.getInputFields().push_back(DocsumFieldSpec::FieldIdentifier(field, FieldPath()));
}
} else {
LOG(warning, "Could not find input summary field '%s'", name.c_str());
Issue::report("Could not find input summary field '%s'", name.c_str());
}
SlimeFillerFilter::add_remaining(filter, name);
}
Expand Down
8 changes: 5 additions & 3 deletions streamingvisitors/src/vespa/vsm/vsm/fieldsearchspec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <vespa/searchlib/fef/iindexenvironment.h>
#include <vespa/searchlib/query/streaming/equiv_query_node.h>
#include <vespa/vespalib/stllike/asciistream.h>
#include <vespa/vespalib/util/issue.h>
#include <vespa/vsm/searcher/boolfieldsearcher.h>
#include <vespa/vsm/searcher/floatfieldsearcher.h>
#include <vespa/vsm/searcher/futf8strchrfieldsearcher.h>
Expand All @@ -24,6 +25,7 @@ LOG_SETUP(".vsm.fieldsearchspec");
using search::streaming::ConstQueryTermList;
using search::streaming::Query;
using search::streaming::QueryTerm;
using vespalib::Issue;

namespace vsm {

Expand Down Expand Up @@ -73,7 +75,7 @@ FieldSearchSpec::FieldSearchSpec(const FieldIdT & fid, const std::string & fname
{
switch(searchDef) {
default:
LOG(warning, "Unknown searchdef = %d. Defaulting to AUTOUTF8", static_cast<int>(searchDef));
Issue::report("Unknown searchdef = %d. Defaulting to AUTOUTF8", static_cast<int>(searchDef));
[[fallthrough]];
case VsmfieldsConfig::Fieldspec::Searchmethod::AUTOUTF8:
case VsmfieldsConfig::Fieldspec::Searchmethod::NONE:
Expand Down Expand Up @@ -211,7 +213,7 @@ FieldSearchSpecMap::addFieldsFromIndex(std::string_view rawIndex, StringFieldIdT
}
}
} else {
LOG(warning, "No valid indexes registered for index %s", std::string(rawIndex).c_str());
Issue::report("No valid indexes registered for index %s", std::string(rawIndex).c_str());
}
}
}
Expand Down Expand Up @@ -265,7 +267,7 @@ buildFieldSet(const VsmfieldsConfig::Documenttype::Index & ci, const FieldSearch
if (foundField != specMap.end()) {
ifm.push_back(foundField->second.id());
} else {
LOG(warning, "Field %s not defined. Ignoring....", cf.name.c_str());
Issue::report("Field %s not defined. Ignoring....", cf.name.c_str());
}
}
}
Expand Down

0 comments on commit 1a416d2

Please sign in to comment.