diff options
13 files changed, 222 insertions, 43 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java index cb59cc2b663..d96384946f7 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java @@ -255,30 +255,63 @@ public class DeploymentStatus { if (change == null || ! change.hasTargets()) return; - Collection<Optional<JobId>> firstProductionJobsWithDeployment = jobSteps.keySet().stream() - .filter(jobId -> jobId.type().isProduction() && jobId.type().isDeployment()) - .filter(jobId -> deploymentFor(jobId).isPresent()) - .collect(groupingBy(jobId -> findCloud(jobId.type()), - Collectors.reducing((o, n) -> o))) // Take the first. - .values(); - if (firstProductionJobsWithDeployment.isEmpty()) - firstProductionJobsWithDeployment = List.of(Optional.empty()); - - for (Optional<JobId> firstProductionJobWithDeploymentInCloud : firstProductionJobsWithDeployment) { + Map<CloudName, Optional<JobId>> firstProductionJobsWithDeployment = firstDependentProductionJobsWithDeployment(job.application().instance()); + firstProductionJobsWithDeployment.forEach((cloud, firstProductionJobWithDeploymentInCloud) -> { Versions versions = Versions.from(change, application, firstProductionJobWithDeploymentInCloud.flatMap(this::deploymentFor), fallbackPlatform(change, job)); if (step.completedAt(change, firstProductionJobWithDeploymentInCloud).isEmpty()) { - CloudName cloud = firstProductionJobWithDeploymentInCloud.map(JobId::type).map(this::findCloud).orElse(zones.systemZone().getCloudName()); JobType typeWithZone = job.type().isSystemTest() ? JobType.systemTest(zones, cloud) : JobType.stagingTest(zones, cloud); jobs.merge(job, List.of(new Job(typeWithZone, versions, step.readyAt(change), change)), DeploymentStatus::union); } - } + }); }); return Collections.unmodifiableMap(jobs); } + /** + * Returns the clouds, and their first production deployments, that depend on this instance; or, + * if no such deployments exist, all clouds the application deploy to, and their first production deployments; or + * if no clouds are deployed to at all, the system default cloud. + */ + Map<CloudName, Optional<JobId>> firstDependentProductionJobsWithDeployment(InstanceName testInstance) { + // Find instances' dependencies on each other: these are topologically ordered, so a simple traversal does it. + Map<InstanceName, Set<InstanceName>> dependencies = new HashMap<>(); + instanceSteps().forEach((name, step) -> { + dependencies.put(name, new HashSet<>()); + dependencies.get(name).add(name); + for (StepStatus dependency : step.dependencies()) { + dependencies.get(name).add(dependency.instance()); + dependencies.get(name).addAll(dependencies.get(dependency.instance)); + } + }); + + Map<CloudName, Optional<JobId>> independentJobsPerCloud = new HashMap<>(); + Map<CloudName, Optional<JobId>> jobsPerCloud = new HashMap<>(); + jobSteps.forEach((job, step) -> { + if ( ! job.type().isProduction() || ! job.type().isDeployment()) + return; + + (dependencies.get(step.instance()).contains(testInstance) ? jobsPerCloud + : independentJobsPerCloud) + .merge(findCloud(job.type()), + Optional.of(job), + (o, n) -> o.filter(v -> deploymentFor(v).isPresent()) // Keep first if its deployment is present. + .or(() -> n.filter(v -> deploymentFor(v).isPresent())) // Use next if only its deployment is present. + .or(() -> o)); // Keep first if none have deployments. + }); + + if (jobsPerCloud.isEmpty()) + jobsPerCloud.putAll(independentJobsPerCloud); + + if (jobsPerCloud.isEmpty()) + jobsPerCloud.put(zones.systemZone().getCloudName(), Optional.empty()); + + return jobsPerCloud; + } + + /** Fall back to the newest, deployable platform, which is compatible with what we want to deploy. */ public Version fallbackPlatform(Change change, JobId job) { Optional<Version> compileVersion = change.revision().map(application.revisions()::get).flatMap(ApplicationVersion::compileVersion); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java index 238489e521a..c18b333a91b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.deployment; import com.yahoo.component.Version; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; @@ -41,6 +42,7 @@ import java.util.stream.Collectors; import static ai.vespa.validation.Validation.require; import static com.yahoo.config.provision.SystemName.cd; +import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.applicationPackage; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.productionApNortheast1; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.productionApNortheast2; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.productionApSoutheast1; @@ -64,8 +66,10 @@ import static java.util.Collections.emptyList; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; /** * Tests a wide variety of deployment scenarios and configurations @@ -2050,16 +2054,16 @@ public class DeploymentTriggerTest { var conservative = tester.newDeploymentContext("t", "a", "default"); canary.runJob(systemTest) - .runJob(stagingTest); + .runJob(stagingTest); conservative.runJob(productionEuWest1) - .runJob(testEuWest1); + .runJob(testEuWest1); canary.submit(applicationPackage) - .runJob(systemTest) - .runJob(stagingTest); + .runJob(systemTest) + .runJob(stagingTest); tester.outstandingChangeDeployer().run(); conservative.runJob(productionEuWest1) - .runJob(testEuWest1); + .runJob(testEuWest1); tester.controllerTester().upgradeSystem(new Version("6.7.7")); tester.upgrader().maintain(); @@ -2068,7 +2072,7 @@ public class DeploymentTriggerTest { .runJob(stagingTest); tester.upgrader().maintain(); conservative.runJob(productionEuWest1) - .runJob(testEuWest1); + .runJob(testEuWest1); } @@ -2349,7 +2353,7 @@ public class DeploymentTriggerTest { Version version3 = new Version("6.4"); tester.controllerTester().upgradeSystem(version3); tests.runJob(systemTest) // Success in default cloud. - .failDeployment(systemTest); // Failure in centauri cloud. + .failDeployment(systemTest); // Failure in centauri cloud. tester.upgrader().run(); assertEquals(Change.of(version3), tests.instance().change()); @@ -2445,6 +2449,91 @@ public class DeploymentTriggerTest { assertEquals(Set.of(), tests.deploymentStatus().jobsToRun().keySet()); } + + @Test + void testInstancesWithMultipleClouds() { + String spec = """ + <deployment> + <parallel> + <instance id='independent'> + <test /> + </instance> + <steps> + <parallel> + <instance id='alpha'> + <test /> + <prod> + <region>us-east-3</region> + </prod> + </instance> + <instance id='beta'> + <test /> + <prod> + <region>alpha-centauri</region> + </prod> + </instance> + <instance id='gamma'> + <test /> + </instance> + </parallel> + <instance id='nu'> + <staging /> + </instance> + <instance id='omega'> + <prod> + <region>alpha-centauri</region> + </prod> + </instance> + </steps> + <instance id='separate'> + <staging /> + <prod> + <region>alpha-centauri</region> + </prod> + </instance> + </parallel> + </deployment> + """; + + RegionName alphaCentauri = RegionName.from("alpha-centauri"); + ZoneApiMock.Builder builder = ZoneApiMock.newBuilder().withCloud("centauri").withSystem(tester.controller().system()); + ZoneApi testAlphaCentauri = builder.with(ZoneId.from(Environment.test, alphaCentauri)).build(); + ZoneApi stagingAlphaCentauri = builder.with(ZoneId.from(Environment.staging, alphaCentauri)).build(); + ZoneApi prodAlphaCentauri = builder.with(ZoneId.from(Environment.prod, alphaCentauri)).build(); + + tester.controllerTester().zoneRegistry().addZones(testAlphaCentauri, stagingAlphaCentauri, prodAlphaCentauri); + tester.controllerTester().setRoutingMethod(tester.controllerTester().zoneRegistry().zones().all().ids(), RoutingMethod.sharedLayer4); + tester.configServer().bootstrap(tester.controllerTester().zoneRegistry().zones().all().ids(), SystemApplication.notController()); + + ApplicationPackage appPackage = ApplicationPackageBuilder.fromDeploymentXml(spec); + DeploymentContext app = tester.newDeploymentContext("tenant", "application", "alpha").submit(appPackage); + Map<JobId, List<DeploymentStatus.Job>> jobs = app.deploymentStatus().jobsToRun(); + + JobType centauriTest = JobType.systemTest(tester.controller().zoneRegistry(), CloudName.from("centauri")); + JobType centauriStaging = JobType.stagingTest(tester.controller().zoneRegistry(), CloudName.from("centauri")); + assertQueued("independent", jobs, systemTest, centauriTest); + assertQueued("alpha", jobs, systemTest); + assertQueued("beta", jobs, centauriTest); + assertQueued("gamma", jobs, centauriTest); + assertQueued("nu", jobs, stagingTest); + assertQueued("separate", jobs, centauriStaging); + + // Once alpha runs its default system test, it also runs the centauri system test, as omega depends on it. + app.runJob(systemTest); + assertQueued("alpha", app.deploymentStatus().jobsToRun(), centauriTest); + } + + private static void assertQueued(String instance, Map<JobId, List<DeploymentStatus.Job>> jobs, JobType... expected) { + List<DeploymentStatus.Job> queued = jobs.get(new JobId(ApplicationId.from("tenant", "application", instance), expected[0])); + Set<ZoneId> remaining = new HashSet<>(); + for (JobType ex : expected) remaining.add(ex.zone()); + for (DeploymentStatus.Job q : queued) + if ( ! remaining.remove(q.type().zone())) + fail("unexpected queued job for " + instance + ": " + q.type()); + if ( ! remaining.isEmpty()) + fail("expected tests for " + instance + " were not queued in : " + remaining); + } + @Test void testNoTests() { DeploymentContext app = tester.newDeploymentContext(); diff --git a/searchcore/src/tests/proton/matching/matching_test.cpp b/searchcore/src/tests/proton/matching/matching_test.cpp index b404330df9b..df8f2a9476b 100644 --- a/searchcore/src/tests/proton/matching/matching_test.cpp +++ b/searchcore/src/tests/proton/matching/matching_test.cpp @@ -66,6 +66,8 @@ using vespalib::eval::SimpleValue; using vespalib::eval::TensorSpec; using vespalib::nbostream; +vespalib::ThreadBundle &ttb() { return vespalib::ThreadBundle::trivial(); } + void inject_match_phase_limiting(Properties &setup, const vespalib::string &attribute, size_t max_hits, bool descending) { Properties cfg; @@ -374,7 +376,7 @@ struct MyWorld { void verify_diversity_filter(const SearchRequest & req, bool expectDiverse) { Matcher::SP matcher = createMatcher(); search::fef::Properties overrides; - auto mtf = matcher->create_match_tools_factory(req, searchContext, attributeContext, metaStore, overrides, true); + auto mtf = matcher->create_match_tools_factory(req, searchContext, attributeContext, metaStore, overrides, ttb(), true); auto diversity = mtf->createDiversifier(HeapSize::lookup(config)); EXPECT_EQUAL(expectDiverse, static_cast<bool>(diversity)); } @@ -384,7 +386,7 @@ struct MyWorld { SearchRequest::SP request = createSimpleRequest("f1", "spread"); search::fef::Properties overrides; MatchToolsFactory::UP match_tools_factory = matcher->create_match_tools_factory( - *request, searchContext, attributeContext, metaStore, overrides, true); + *request, searchContext, attributeContext, metaStore, overrides, ttb(), true); MatchTools::UP match_tools = match_tools_factory->createMatchTools(); match_tools->setup_first_phase(nullptr); return match_tools->match_data().get_termwise_limit(); diff --git a/searchcore/src/tests/proton/matching/query_test.cpp b/searchcore/src/tests/proton/matching/query_test.cpp index 78bb679a7dc..4c81aa7e347 100644 --- a/searchcore/src/tests/proton/matching/query_test.cpp +++ b/searchcore/src/tests/proton/matching/query_test.cpp @@ -29,6 +29,7 @@ #include <vespa/searchlib/parsequery/stackdumpiterator.h> #include <vespa/document/datatype/positiondatatype.h> #include <vespa/vespalib/stllike/asciistream.h> +#include <vespa/vespalib/util/thread_bundle.h> #include <vespa/vespalib/testkit/testapp.h> #include <vespa/log/log.h> @@ -166,6 +167,8 @@ fef_test::IndexEnvironment plain_index_env; fef_test::IndexEnvironment resolved_index_env; fef_test::IndexEnvironment attribute_index_env; +vespalib::ThreadBundle &ttb() { return vespalib::ThreadBundle::trivial(); } + vespalib::string termAsString(const search::query::Range &term) { vespalib::asciistream os; @@ -1141,21 +1144,21 @@ Test::global_filter_is_calculated_and_handled() uint32_t docid_limit = 10; { // global filter is not wanted GlobalFilterBlueprint bp(result, false); - auto res = Query::handle_global_filter(bp, docid_limit, 0, 1, nullptr); + auto res = Query::handle_global_filter(bp, docid_limit, 0, 1, ttb(), nullptr); EXPECT_FALSE(res); EXPECT_FALSE(bp.filter); EXPECT_EQUAL(-1.0, bp.estimated_hit_ratio); } { // estimated_hit_ratio < global_filter_lower_limit GlobalFilterBlueprint bp(result, true); - auto res = Query::handle_global_filter(bp, docid_limit, 0.31, 1, nullptr); + auto res = Query::handle_global_filter(bp, docid_limit, 0.31, 1, ttb(), nullptr); EXPECT_FALSE(res); EXPECT_FALSE(bp.filter); EXPECT_EQUAL(-1.0, bp.estimated_hit_ratio); } { // estimated_hit_ratio <= global_filter_upper_limit GlobalFilterBlueprint bp(result, true); - auto res = Query::handle_global_filter(bp, docid_limit, 0, 0.3, nullptr); + auto res = Query::handle_global_filter(bp, docid_limit, 0, 0.3, ttb(), nullptr); EXPECT_TRUE(res); EXPECT_TRUE(bp.filter); EXPECT_TRUE(bp.filter->is_active()); @@ -1168,7 +1171,7 @@ Test::global_filter_is_calculated_and_handled() } { // estimated_hit_ratio > global_filter_upper_limit GlobalFilterBlueprint bp(result, true); - auto res = Query::handle_global_filter(bp, docid_limit, 0, 0.29, nullptr); + auto res = Query::handle_global_filter(bp, docid_limit, 0, 0.29, ttb(), nullptr); EXPECT_TRUE(res); EXPECT_TRUE(bp.filter); EXPECT_FALSE(bp.filter->is_active()); diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp index 0dfb6953cc9..e5a0ac3c3fe 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp @@ -14,6 +14,7 @@ #include <vespa/vespalib/data/slime/inject.h> #include <vespa/vespalib/data/slime/inserter.h> #include <vespa/vespalib/util/issue.h> +#include <vespa/vespalib/util/thread_bundle.h> using search::queryeval::IDiversifier; using search::attribute::diversity::DiversityFilter; @@ -171,6 +172,7 @@ MatchToolsFactory(QueryLimiter & queryLimiter, const RankSetup & rankSetup, const Properties & rankProperties, const Properties & featureOverrides, + vespalib::ThreadBundle & thread_bundle, bool is_search) : _queryLimiter(queryLimiter), _global_filter_params(extract_global_filter_params(rankSetup, rankProperties, metaStore.getNumActiveLids(), searchContext.getDocIdLimit())), @@ -203,7 +205,7 @@ MatchToolsFactory(QueryLimiter & queryLimiter, _query.handle_global_filter(searchContext.getDocIdLimit(), _global_filter_params.global_filter_lower_limit, _global_filter_params.global_filter_upper_limit, - trace); + thread_bundle, trace); } _query.freeze(); trace.addEvent(5, "Prepare shared state for multi-threaded rank executors"); diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_tools.h b/searchcore/src/vespa/searchcore/proton/matching/match_tools.h index 1d50b44e839..8012d7b1ec5 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_tools.h +++ b/searchcore/src/vespa/searchcore/proton/matching/match_tools.h @@ -19,6 +19,7 @@ #include <vespa/vespalib/util/clock.h> namespace vespalib { class ExecutionProfiler; } +namespace vespalib { class ThreadBundle; } namespace search::engine { class Trace; } @@ -143,6 +144,7 @@ public: const RankSetup &rankSetup, const Properties &rankProperties, const Properties &featureOverrides, + vespalib::ThreadBundle &thread_bundle, bool is_search); ~MatchToolsFactory(); bool valid() const { return _valid; } diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp index e8c7f2c5e44..95fe6fd6c3e 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp @@ -141,7 +141,8 @@ using search::fef::indexproperties::softtimeout::Factor; std::unique_ptr<MatchToolsFactory> Matcher::create_match_tools_factory(const search::engine::Request &request, ISearchContext &searchContext, IAttributeContext &attrContext, const search::IDocumentMetaStore &metaStore, - const Properties &feature_overrides, bool is_search) const + const Properties &feature_overrides, vespalib::ThreadBundle &thread_bundle, + bool is_search) const { const Properties & rankProperties = request.propertiesMap.rankProperties(); bool softTimeoutEnabled = Enabled::lookup(rankProperties, _rankSetup->getSoftTimeoutEnabled()); @@ -161,7 +162,7 @@ Matcher::create_match_tools_factory(const search::engine::Request &request, ISea return std::make_unique<MatchToolsFactory>(_queryLimiter, doom, searchContext, attrContext, request.trace(), request.getStackRef(), request.location, _viewResolver, metaStore, _indexEnv, *_rankSetup, - rankProperties, feature_overrides, is_search); + rankProperties, feature_overrides, thread_bundle, is_search); } size_t @@ -221,8 +222,8 @@ Matcher::match(const SearchRequest &request, vespalib::ThreadBundle &threadBundl feature_overrides = owned_objects.feature_overrides.get(); } - MatchToolsFactory::UP mtf = create_match_tools_factory(request, searchContext, attrContext, - metaStore, *feature_overrides, true); + MatchToolsFactory::UP mtf = create_match_tools_factory(request, searchContext, attrContext, metaStore, + *feature_overrides, threadBundle, true); isDoomExplicit = mtf->getRequestContext().getDoom().isExplicitSoftDoom(); traceQuery(6, request.trace(), mtf->query()); if (!mtf->valid()) { @@ -372,7 +373,8 @@ Matcher::create_docsum_matcher(const DocsumRequest &req, ISearchContext &search_ } StupidMetaStore meta; MatchToolsFactory::UP mtf = create_match_tools_factory(req, search_ctx, attr_ctx, meta, - req.propertiesMap.featureOverrides(), false); + req.propertiesMap.featureOverrides(), + vespalib::ThreadBundle::trivial(), false); if (!mtf->valid()) { LOG(warning, "could not initialize docsum matching: %s", (expectedSessionCached) ? "session has expired" : "invalid query"); diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.h b/searchcore/src/vespa/searchcore/proton/matching/matcher.h index 409f0ddeae5..4071f95b0c8 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matcher.h +++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.h @@ -104,7 +104,8 @@ public: std::unique_ptr<MatchToolsFactory> create_match_tools_factory(const search::engine::Request &request, ISearchContext &searchContext, IAttributeContext &attrContext, const search::IDocumentMetaStore &metaStore, - const Properties &feature_overrides, bool is_search) const; + const Properties &feature_overrides, vespalib::ThreadBundle &thread_bundle, + bool is_search) const; /** * Perform a search against this matcher. diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.cpp b/searchcore/src/vespa/searchcore/proton/matching/query.cpp index ec289c06122..d8951eb1084 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/query.cpp @@ -14,6 +14,7 @@ #include <vespa/searchlib/parsequery/stackdumpiterator.h> #include <vespa/searchlib/queryeval/intermediate_blueprints.h> #include <vespa/vespalib/util/issue.h> +#include <vespa/vespalib/util/thread_bundle.h> #include <vespa/log/log.h> LOG_SETUP(".proton.matching.query"); @@ -35,6 +36,7 @@ using search::query::Weight; using search::queryeval::AndBlueprint; using search::queryeval::AndNotBlueprint; using search::queryeval::Blueprint; +using search::queryeval::GlobalFilter; using search::queryeval::IRequestContext; using search::queryeval::IntermediateBlueprint; using search::queryeval::RankBlueprint; @@ -246,9 +248,9 @@ Query::fetchPostings() void Query::handle_global_filter(uint32_t docid_limit, double global_filter_lower_limit, double global_filter_upper_limit, - search::engine::Trace& trace) + vespalib::ThreadBundle &thread_bundle, search::engine::Trace& trace) { - if (!handle_global_filter(*_blueprint, docid_limit, global_filter_lower_limit, global_filter_upper_limit, &trace)) { + if (!handle_global_filter(*_blueprint, docid_limit, global_filter_lower_limit, global_filter_upper_limit, thread_bundle, &trace)) { return; } // optimized order may change after accounting for global filter: @@ -259,10 +261,21 @@ Query::handle_global_filter(uint32_t docid_limit, double global_filter_lower_lim fetchPostings(); } +std::shared_ptr<GlobalFilter> +Query::create_global_filter(Blueprint& blueprint, uint32_t docid_limit, vespalib::ThreadBundle &thread_bundle) +{ + (void) thread_bundle; // multi-threaded filter generation coming soon + bool strict = true; + auto constraint = Blueprint::FilterConstraint::UPPER_BOUND; + auto filter_iterator = blueprint.createFilterSearch(strict, constraint); + filter_iterator->initRange(1, docid_limit); + return GlobalFilter::create(filter_iterator->get_hits(1)); +} + bool Query::handle_global_filter(Blueprint& blueprint, uint32_t docid_limit, double global_filter_lower_limit, double global_filter_upper_limit, - search::engine::Trace* trace) + vespalib::ThreadBundle &thread_bundle, search::engine::Trace* trace) { using search::queryeval::GlobalFilter; double estimated_hit_ratio = blueprint.getState().hit_ratio(docid_limit); @@ -284,12 +297,7 @@ Query::handle_global_filter(Blueprint& blueprint, uint32_t docid_limit, trace->addEvent(5, vespalib::make_string("Calculate global filter (estimated_hit_ratio (%f) <= upper_limit (%f))", estimated_hit_ratio, global_filter_upper_limit)); } - auto constraint = Blueprint::FilterConstraint::UPPER_BOUND; - bool strict = true; - auto filter_iterator = blueprint.createFilterSearch(strict, constraint); - filter_iterator->initRange(1, docid_limit); - auto white_list = filter_iterator->get_hits(1); - global_filter = GlobalFilter::create(std::move(white_list)); + global_filter = create_global_filter(blueprint, docid_limit, thread_bundle); } else { if (trace && trace->shouldTrace(5)) { trace->addEvent(5, vespalib::make_string("Create match all global filter (estimated_hit_ratio (%f) > upper_limit (%f))", diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.h b/searchcore/src/vespa/searchcore/proton/matching/query.h index 78bd6a0bacb..8517ec2153f 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.h +++ b/searchcore/src/vespa/searchcore/proton/matching/query.h @@ -10,6 +10,7 @@ #include <vespa/searchlib/queryeval/blueprint.h> #include <vespa/searchlib/queryeval/irequestcontext.h> +namespace vespalib { class ThreadBundle; } namespace search::engine { class Trace; } namespace proton::matching { @@ -21,6 +22,7 @@ class Query { private: using Blueprint = search::queryeval::Blueprint; + using GlobalFilter = search::queryeval::GlobalFilter; search::query::Node::UP _query_tree; Blueprint::UP _blueprint; Blueprint::UP _whiteListBlueprint; @@ -95,7 +97,11 @@ public: void fetchPostings(); void handle_global_filter(uint32_t docid_limit, double global_filter_lower_limit, double global_filter_upper_limit, - search::engine::Trace& trace); + vespalib::ThreadBundle &thread_bundle, search::engine::Trace& trace); + + // Create a global filter. Called by handle_global_filter if needed. + static std::shared_ptr<GlobalFilter> create_global_filter(Blueprint& blueprint, uint32_t docid_limit, + vespalib::ThreadBundle &thread_bundle); /** * Calculates and handles the global filter if needed by the blueprint tree. @@ -112,7 +118,7 @@ public: */ static bool handle_global_filter(Blueprint& blueprint, uint32_t docid_limit, double global_filter_lower_limit, double global_filter_upper_limit, - search::engine::Trace* trace); + vespalib::ThreadBundle &thread_bundle, search::engine::Trace* trace); void freeze(); diff --git a/vespalib/src/tests/simple_thread_bundle/simple_thread_bundle_test.cpp b/vespalib/src/tests/simple_thread_bundle/simple_thread_bundle_test.cpp index 5d69a18112a..debb34724f6 100644 --- a/vespalib/src/tests/simple_thread_bundle/simple_thread_bundle_test.cpp +++ b/vespalib/src/tests/simple_thread_bundle/simple_thread_bundle_test.cpp @@ -89,6 +89,17 @@ TEST_FF("require that having too many targets fails", SimpleThreadBundle(1), Sta f2.check(Box<size_t>().add(0).add(0)); } +TEST_F("require that ThreadBundle::trivial works the same as SimpleThreadBundle(1)", State(2)) { + ThreadBundle &bundle = ThreadBundle::trivial(); + EXPECT_EQUAL(bundle.size(), 1u); + bundle.run(f.getTargets(0)); + f.check({0,0}); + bundle.run(f.getTargets(1)); + f.check({1,0}); + EXPECT_EXCEPTION(bundle.run(f.getTargets(2)), IllegalArgumentException, ""); + f.check({1,0}); +} + TEST_FF("require that bundles with multiple internal threads work", SimpleThreadBundle(3), State(3)) { f1.run(f2.getTargets(3)); f2.check(Box<size_t>().add(1).add(1).add(1)); diff --git a/vespalib/src/vespa/vespalib/util/thread_bundle.cpp b/vespalib/src/vespa/vespalib/util/thread_bundle.cpp index 7068224229e..9f953abfce7 100644 --- a/vespalib/src/vespa/vespalib/util/thread_bundle.cpp +++ b/vespalib/src/vespa/vespalib/util/thread_bundle.cpp @@ -1,7 +1,24 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "thread_bundle.h" +#include "exceptions.h" namespace vespalib { +ThreadBundle & +ThreadBundle::trivial() { + struct TrivialThreadBundle : ThreadBundle { + size_t size() const override { return 1; } + void run(const std::vector<Runnable*> &targets) override { + if (targets.size() == 1) { + targets[0]->run(); + } else if (targets.size() > 1) { + throw IllegalArgumentException("too many targets"); + } + }; + }; + static TrivialThreadBundle trivial_thread_bundle; + return trivial_thread_bundle; +} + } // namespace vespalib diff --git a/vespalib/src/vespa/vespalib/util/thread_bundle.h b/vespalib/src/vespa/vespalib/util/thread_bundle.h index 32b7d32b2a7..699fd8e27a0 100644 --- a/vespalib/src/vespa/vespalib/util/thread_bundle.h +++ b/vespalib/src/vespa/vespalib/util/thread_bundle.h @@ -33,6 +33,9 @@ struct ThreadBundle { * Empty virtual destructor to enable subclassing. **/ virtual ~ThreadBundle() {} + + // a thread bundle that can only run things in the current thread. + static ThreadBundle &trivial(); }; } // namespace vespalib |