diff options
Diffstat (limited to 'searchcore')
8 files changed, 70 insertions, 53 deletions
diff --git a/searchcore/src/tests/proton/matching/query_test.cpp b/searchcore/src/tests/proton/matching/query_test.cpp index 575a52d01fb..d098bdde8b6 100644 --- a/searchcore/src/tests/proton/matching/query_test.cpp +++ b/searchcore/src/tests/proton/matching/query_test.cpp @@ -711,7 +711,7 @@ void Test::requireThatQueryGluesEverythingTogether() { EXPECT_EQUAL(1u, md->getNumTermFields()); query.optimize(); - query.fetchPostings(); + query.fetchPostings(requestContext.getDoom()); SearchIterator::UP search = query.createSearch(*md); ASSERT_TRUE(search.get()); } @@ -744,7 +744,7 @@ void checkQueryAddsLocation(const string &loc_in, const string &loc_out) { MatchData::UP md = mdl.createMatchData(); EXPECT_EQUAL(2u, md->getNumTermFields()); - query.fetchPostings(); + query.fetchPostings(requestContext.getDoom()); SearchIterator::UP search = query.createSearch(*md); ASSERT_TRUE(search.get()); if (!EXPECT_NOT_EQUAL(string::npos, search->asString().find(loc_out))) { @@ -966,7 +966,7 @@ Test::requireThatWhiteListBlueprintCanBeUsed() MatchData::UP md = mdl.createMatchData(); query.optimize(); - query.fetchPostings(); + query.fetchPostings(requestContext.getDoom()); SearchIterator::UP search = query.createSearch(*md); SimpleResult exp = SimpleResult().addHit(1).addHit(5).addHit(7).addHit(11); SimpleResult act; diff --git a/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp b/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp index be1c8941f65..e1820ece0e3 100644 --- a/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp +++ b/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp @@ -52,22 +52,11 @@ DocsumContext::initState() _docsumState._args.initFromDocsumRequest(req); _docsumState._docsumbuf.clear(); _docsumState._docsumbuf.reserve(req.hits.size()); - for (uint32_t i = 0; i < req.hits.size(); i++) { - _docsumState._docsumbuf.push_back(req.hits[i].docid); + for (const auto & hit : req.hits) { + _docsumState._docsumbuf.push_back(hit.docid); } } -namespace { - -vespalib::Slime::Params -makeSlimeParams(size_t chunkSize) { - Slime::Params params; - params.setChunkSize(chunkSize); - return params; -} - -} - vespalib::Slime::UP DocsumContext::createSlimeReply() { @@ -75,11 +64,11 @@ DocsumContext::createSlimeReply() _docsumState._args.get_fields()); _docsumWriter.initState(_attrMgr, _docsumState, rci); const size_t estimatedChunkSize(std::min(0x200000ul, _docsumState._docsumbuf.size()*0x400ul)); - vespalib::Slime::UP response(std::make_unique<vespalib::Slime>(makeSlimeParams(estimatedChunkSize))); + auto response = std::make_unique<vespalib::Slime>(Slime::Params(estimatedChunkSize)); Cursor & root = response->setObject(); Cursor & array = root.setArray(DOCSUMS); const Symbol docsumSym = response->insert(DOCSUM); - _docsumState._omit_summary_features = (rci.res_class != nullptr) ? rci.res_class->omit_summary_features() : true; + _docsumState._omit_summary_features = (rci.res_class == nullptr) || rci.res_class->omit_summary_features(); uint32_t num_ok(0); for (uint32_t docId : _docsumState._docsumbuf) { if (_request.expired() ) { break; } diff --git a/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.cpp b/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.cpp index 1528b327747..b8027bff04a 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/attribute_limiter.cpp @@ -6,6 +6,7 @@ #include <vespa/searchlib/fef/matchdatalayout.h> #include <vespa/searchlib/queryeval/searchable.h> #include <vespa/searchlib/queryeval/blueprint.h> +#include <vespa/searchlib/queryeval/irequestcontext.h> #include <vespa/searchlib/query/tree/range.h> #include <vespa/searchlib/query/tree/simplequery.h> @@ -98,7 +99,7 @@ AttributeLimiter::create_search(size_t want_hits, size_t max_group_size, bool st FieldSpecList field; // single field API is protected field.add(FieldSpec(_attribute_name, my_field_id, my_handle)); _blueprint = _searchable_attributes.createBlueprint(_requestContext, field, node); - _blueprint->fetchPostings(ExecuteInfo::create(strictSearch)); + _blueprint->fetchPostings(ExecuteInfo::create(strictSearch, &_requestContext.getDoom())); _estimatedHits.store(_blueprint->getState().estimate().estHits, std::memory_order_relaxed); _blueprint->freeze(); } diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp index b57346611f1..14a238330ba 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp @@ -222,6 +222,8 @@ MatchThread::match_loop(MatchTools &tools, HitCollector &hits) !docid_range.empty(); docid_range = scheduler.next_range(thread_id)) { + // Due to some schedulers communicating across threads, it is vital that all complete this + // loop. Do not break out. if (!softDoomed) { uint32_t lastCovered = inner_match_loop<Strategy, do_rank, do_limit, do_share_work, use_rank_drop_limit>(context, tools, docid_range); softDoomed = (lastCovered < docid_range.end); @@ -311,6 +313,41 @@ MatchThread::match_loop_helper(MatchTools &tools, HitCollector &hits) } } +void +MatchThread::secondPhase(MatchTools & tools, HitCollector & hits) { + trace->addEvent(4, "Start second phase rerank"); + auto sorted_hit_seq = matchToolsFactory.should_diversify() + ? hits.getSortedHitSequence(matchParams.arraySize) + : hits.getSortedHitSequence(matchParams.heapSize); + trace->addEvent(5, "Synchronize before second phase rerank"); + WaitTimer get_second_phase_work_timer(wait_time_s); + /** + * All, or none of the threads in the bundle should call communicator.get_second_phase_work and + * communicator.complete_second_phase. + * Avoid early return and handle doom with care. + */ + auto my_work = communicator.get_second_phase_work(sorted_hit_seq, thread_id); + get_second_phase_work_timer.done(); + if (tools.getDoom().hard_doom()) { + my_work.clear(); + } + if (!my_work.empty()) { + tools.setup_second_phase(second_phase_profiler.get()); + DocumentScorer scorer(tools.rank_program(), tools.search()); + scorer.score(my_work); + } + thread_stats.docsReRanked(my_work.size()); + trace->addEvent(5, "Synchronize before rank scaling"); + WaitTimer complete_second_phase_timer(wait_time_s); + auto [kept_hits, ranges] = communicator.complete_second_phase(my_work, thread_id); + complete_second_phase_timer.done(); + hits.setReRankedHits(std::move(kept_hits)); + hits.setRanges(ranges); + if (auto onReRankTask = matchToolsFactory.createOnSecondPhaseTask()) { + onReRankTask->run(hits.getReRankedHits()); + } +} + search::ResultSet::UP MatchThread::findMatches(MatchTools &tools) { @@ -332,34 +369,15 @@ MatchThread::findMatches(MatchTools &tools) } HitCollector hits(matchParams.numDocs, matchParams.arraySize); trace->addEvent(4, "Start match and first phase rank"); + /** + * All, or none of the threads in the bundle must execute the match loop. + * The same goes for secondPhase. + * This is due to all the threads in the bundle needs to meet up and exchange information. + * If not you will have deadlock. + */ match_loop_helper(tools, hits); if (tools.has_second_phase_rank()) { - trace->addEvent(4, "Start second phase rerank"); - auto sorted_hit_seq = matchToolsFactory.should_diversify() - ? hits.getSortedHitSequence(matchParams.arraySize) - : hits.getSortedHitSequence(matchParams.heapSize); - trace->addEvent(5, "Synchronize before second phase rerank"); - WaitTimer get_second_phase_work_timer(wait_time_s); - auto my_work = communicator.get_second_phase_work(sorted_hit_seq, thread_id); - get_second_phase_work_timer.done(); - if (tools.getDoom().hard_doom()) { - my_work.clear(); - } - if (!my_work.empty()) { - tools.setup_second_phase(second_phase_profiler.get()); - DocumentScorer scorer(tools.rank_program(), tools.search()); - scorer.score(my_work); - } - thread_stats.docsReRanked(my_work.size()); - trace->addEvent(5, "Synchronize before rank scaling"); - WaitTimer complete_second_phase_timer(wait_time_s); - auto [kept_hits, ranges] = communicator.complete_second_phase(my_work, thread_id); - complete_second_phase_timer.done(); - hits.setReRankedHits(std::move(kept_hits)); - hits.setRanges(ranges); - if (auto onReRankTask = matchToolsFactory.createOnSecondPhaseTask()) { - onReRankTask->run(hits.getReRankedHits()); - } + secondPhase(tools, hits); } trace->addEvent(4, "Create result set"); return hits.getResultSet(fallback_rank_value()); @@ -463,6 +481,11 @@ MatchThread::run() auto capture_issues = vespalib::Issue::listen(my_issues); trace->addEvent(4, "Start MatchThread::run"); MatchTools::UP matchTools = matchToolsFactory.createMatchTools(); + /** + * All, or none of the threads in the bundle must call findMatches. + * All, or none of the threads in the bundle must call mergeDirector.dualMerge. + * Avoid early return and handle doom with care. + */ search::ResultSet::UP result = findMatches(*matchTools); match_time_s = vespalib::to_s(match_time.elapsed()); resultContext = resultProcessor.createThreadContext(matchTools->getDoom(), thread_id, _distributionKey); @@ -475,6 +498,7 @@ MatchThread::run() result->getNumHits(), resultContext->sort->hasSortData(), bool(resultContext->grouping))); + (void) processToken; // Avoid unused warning get_token_timer.done(); trace->addEvent(5, "Start result processing"); processResult(matchTools->getDoom(), std::move(result), *resultContext); diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_thread.h b/searchcore/src/vespa/searchcore/proton/matching/match_thread.h index 03ba34eca1f..ad864a98227 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_thread.h +++ b/searchcore/src/vespa/searchcore/proton/matching/match_thread.h @@ -113,6 +113,7 @@ private: void match_loop_helper(MatchTools &tools, HitCollector &hits); search::ResultSet::UP findMatches(MatchTools &tools); + void secondPhase(MatchTools & tools, HitCollector & hits); void processResult(const Doom & doom, search::ResultSet::UP result, ResultProcessor::Context &context); diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp index 5ae671b88cb..758ef35ebc9 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp @@ -201,9 +201,9 @@ MatchToolsFactory(QueryLimiter & queryLimiter, trace.addEvent(5, "Optimize query execution plan"); _query.optimize(); trace.addEvent(4, "Perform dictionary lookups and posting lists initialization"); - _query.fetchPostings(); + _query.fetchPostings(_requestContext.getDoom()); if (is_search) { - _query.handle_global_filter(searchContext.getDocIdLimit(), + _query.handle_global_filter(_requestContext.getDoom(), searchContext.getDocIdLimit(), _attribute_blueprint_params.global_filter_lower_limit, _attribute_blueprint_params.global_filter_upper_limit, thread_bundle, trace); diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.cpp b/searchcore/src/vespa/searchcore/proton/matching/query.cpp index d0738f1857f..22f6ec9cc88 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/query.cpp @@ -247,13 +247,14 @@ Query::optimize() } void -Query::fetchPostings() +Query::fetchPostings(const vespalib::Doom & doom) { - _blueprint->fetchPostings(search::queryeval::ExecuteInfo::create(true, 1.0)); + _blueprint->fetchPostings(search::queryeval::ExecuteInfo::create(true, &doom)); } void -Query::handle_global_filter(uint32_t docid_limit, double global_filter_lower_limit, double global_filter_upper_limit, +Query::handle_global_filter(const vespalib::Doom & doom, uint32_t docid_limit, + double global_filter_lower_limit, double global_filter_upper_limit, vespalib::ThreadBundle &thread_bundle, search::engine::Trace& trace) { if (!handle_global_filter(*_blueprint, docid_limit, global_filter_lower_limit, global_filter_upper_limit, thread_bundle, &trace)) { @@ -264,7 +265,7 @@ Query::handle_global_filter(uint32_t docid_limit, double global_filter_lower_lim _blueprint = Blueprint::optimize(std::move(_blueprint)); LOG(debug, "blueprint after handle_global_filter:\n%s\n", _blueprint->asString().c_str()); // strictness may change if optimized order changed: - fetchPostings(); + fetchPostings(doom); } bool diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.h b/searchcore/src/vespa/searchcore/proton/matching/query.h index b0299307e92..1a3136042a7 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.h +++ b/searchcore/src/vespa/searchcore/proton/matching/query.h @@ -98,9 +98,10 @@ public: * test to verify the original query without optimization. **/ void optimize(); - void fetchPostings(); + void fetchPostings(const vespalib::Doom & doom); - void handle_global_filter(uint32_t docid_limit, double global_filter_lower_limit, double global_filter_upper_limit, + void handle_global_filter(const vespalib::Doom & doom, uint32_t docid_limit, + double global_filter_lower_limit, double global_filter_upper_limit, vespalib::ThreadBundle &thread_bundle, search::engine::Trace& trace); /** |