Merge pull request #25127 from vespa-engine/toregge/more-restrictive-…

…active-lids-blueprint-try-2 Turn off FullSearch optimization for active lids blueprint createLe…
vespa-engine · Dec 6, 2022 · 77cb75a · 77cb75a
2 parents a7c1295 + 1c80df9
commit 77cb75a
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 16 deletions.
diff --git a/searchcore/src/tests/proton/documentmetastore/lid_allocator/lid_allocator_test.cpp b/searchcore/src/tests/proton/documentmetastore/lid_allocator/lid_allocator_test.cpp
@@ -1,14 +1,17 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include <vespa/searchcore/proton/documentmetastore/lid_allocator.h>
+#include <vespa/searchlib/fef/matchdata.h>
 #include <vespa/searchlib/queryeval/searchiterator.h>
 #include <vespa/searchlib/queryeval/simpleresult.h>
 #include <vespa/vespalib/util/generationholder.h>
 #include <vespa/vespalib/util/time.h>
 #include <vespa/vespalib/gtest/gtest.h>
 #include <iostream>
 
+using search::fef::MatchData;
 using search::queryeval::Blueprint;
+using search::queryeval::SearchIterator;
 using search::queryeval::SimpleResult;
 using vespalib::GenerationHolder;
 using vespalib::Timer;
@@ -99,19 +102,25 @@ class LidAllocatorTest : public ::testing::Test
         return result;
     }
 
-    SimpleResult get_active_lids_in_search_iterator(uint32_t docid_limit) {
+    SimpleResult get_active_lids_in_search_iterator(uint32_t docid_limit, bool filter) {
         auto blueprint = _allocator.createWhiteListBlueprint();
         blueprint->setDocIdLimit(docid_limit);
-        auto iterator = blueprint->createFilterSearch(true, search::queryeval::Blueprint::FilterConstraint::UPPER_BOUND);
+        std::unique_ptr<SearchIterator> iterator;
+        MatchData md(MatchData::params());
+        if (filter) {
+            iterator = blueprint->createFilterSearch(true, Blueprint::FilterConstraint::UPPER_BOUND);
+        } else {
+            iterator = blueprint->createSearch(md, true);
+        }
         SimpleResult res;
         res.search(*iterator, docid_limit);
         return res;
     }
 
-    Trinary search_iterator_matches_any(uint32_t docid_limit) {
+    Trinary filter_search_iterator_matches_any(uint32_t docid_limit) {
         auto blueprint = _allocator.createWhiteListBlueprint();
         blueprint->setDocIdLimit(docid_limit);
-        auto iterator = blueprint->createFilterSearch(true, search::queryeval::Blueprint::FilterConstraint::UPPER_BOUND);
+        auto iterator = blueprint->createFilterSearch(true, Blueprint::FilterConstraint::UPPER_BOUND);
         return iterator->matches_any();
     }
 
@@ -146,16 +155,18 @@ TEST_F(LidAllocatorTest, active_lids_are_available_in_search_iterator)
 {
     register_lids({ 1, 2, 3, 4 });
     activate_lids({ 1, 2, 4 }, true);
-    EXPECT_EQ(Trinary::Undefined, search_iterator_matches_any(5));
-    EXPECT_EQ(SimpleResult({1, 2, 4}), get_active_lids_in_search_iterator(5));
+    EXPECT_EQ(Trinary::Undefined, filter_search_iterator_matches_any(5));
+    EXPECT_EQ(SimpleResult({1, 2, 4}), get_active_lids_in_search_iterator(5, true));
+    EXPECT_EQ(SimpleResult({1, 2, 4}), get_active_lids_in_search_iterator(5, false));
 }
 
-TEST_F(LidAllocatorTest, search_iterator_matches_all_when_all_lids_are_active)
+TEST_F(LidAllocatorTest, filter_search_iterator_matches_all_when_all_lids_are_active)
 {
     register_lids({ 1, 2, 3, 4 });
     activate_lids({ 1, 2, 3, 4 }, true);
-    EXPECT_EQ(Trinary::True, search_iterator_matches_any(5));
-    EXPECT_EQ(SimpleResult({1, 2, 3, 4}), get_active_lids_in_search_iterator(5));
+    EXPECT_EQ(Trinary::True, filter_search_iterator_matches_any(6));
+    EXPECT_EQ(SimpleResult({1, 2, 3, 4, 5}), get_active_lids_in_search_iterator(6, true));
+    EXPECT_EQ(SimpleResult({1, 2, 3, 4}), get_active_lids_in_search_iterator(6, false));
 }
 
 class LidAllocatorPerformanceTest : public LidAllocatorTest,

diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/lid_allocator.cpp b/searchcore/src/vespa/searchcore/proton/documentmetastore/lid_allocator.cpp
@@ -188,12 +188,20 @@ class WhiteListBlueprint : public SimpleLeafBlueprint
     mutable std::mutex _lock;
     mutable std::vector<search::fef::TermFieldMatchData *> _matchDataVector;
 
+    std::unique_ptr<SearchIterator> create_search_helper(bool strict) const {
+        auto tfmd = new search::fef::TermFieldMatchData;
+        {
+            std::lock_guard<std::mutex> lock(_lock);
+            _matchDataVector.push_back(tfmd);
+        }
+        return search::BitVectorIterator::create(&_activeLids, get_docid_limit(), *tfmd, strict);
+    }
     SearchIterator::UP
     createLeafSearch(const TermFieldMatchDataArray &tfmda, bool strict) const override
     {
         assert(tfmda.size() == 0);
         (void) tfmda;
-        return createFilterSearch(strict, FilterConstraint::UPPER_BOUND);
+        return create_search_helper(strict);
     }
 public:
     WhiteListBlueprint(const search::BitVector &activeLids, bool all_lids_active)
@@ -212,12 +220,7 @@ class WhiteListBlueprint : public SimpleLeafBlueprint
         if (_all_lids_active) {
             return std::make_unique<FullSearch>();
         }
-        auto tfmd = new search::fef::TermFieldMatchData;
-        {
-            std::lock_guard<std::mutex> lock(_lock);
-            _matchDataVector.push_back(tfmd);
-        }
-        return search::BitVectorIterator::create(&_activeLids, get_docid_limit(), *tfmd, strict);
+        return create_search_helper(strict);
     }
 
     ~WhiteListBlueprint() {