diff options
author | Jon Bratseth <bratseth@verizonmedia.com> | 2019-06-10 23:09:55 +0200 |
---|---|---|
committer | Jon Bratseth <bratseth@verizonmedia.com> | 2019-06-10 23:09:55 +0200 |
commit | 99cd8027a4f0f031294c3a53f10158953cb4e77b (patch) | |
tree | 988236bedadf69a284d245be0385640c624e0ca9 | |
parent | c760fc10a1759e903aef9d2e7d78eae3cce9817f (diff) |
Holistic query profile validation
8 files changed, 114 insertions, 13 deletions
diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java index 937d511bb09..c19865fafc9 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java @@ -341,7 +341,7 @@ public class DeployState implements ConfigDefinitionStore { public DeployState build(ValidationParameters validationParameters) { RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); - QueryProfiles queryProfiles = new QueryProfilesBuilder().build(applicationPackage); + QueryProfiles queryProfiles = new QueryProfilesBuilder().build(applicationPackage, logger); SemanticRules semanticRules = new SemanticRuleBuilder().build(applicationPackage); SearchDocumentModel searchDocumentModel = createSearchDocumentModel(rankProfileRegistry, logger, queryProfiles, validationParameters); return new DeployState(applicationPackage, diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java b/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java index bd4daa58253..9804b0b6329 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java @@ -246,7 +246,7 @@ public class SearchBuilder { DocumentModelBuilder builder = new DocumentModelBuilder(model); for (Search search : new SearchOrderer().order(searchList)) { new FieldOperationApplierForSearch().process(search); // TODO: Why is this not in the regular list? - process(search, deployLogger, new QueryProfiles(queryProfileRegistry), validate); + process(search, deployLogger, new QueryProfiles(queryProfileRegistry, deployLogger), validate); built.add(search); } builder.addToModel(searchList); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/search/QueryProfiles.java b/config-model/src/main/java/com/yahoo/vespa/model/container/search/QueryProfiles.java index 526b2abe1e1..c7114178ad6 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/search/QueryProfiles.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/search/QueryProfiles.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.container.search; +import com.yahoo.config.application.api.DeployLogger; import com.yahoo.search.query.profile.BackedOverridableQueryProfile; import com.yahoo.search.query.profile.QueryProfile; import com.yahoo.search.query.profile.QueryProfileRegistry; @@ -12,6 +13,8 @@ import com.yahoo.search.query.profile.config.QueryProfilesConfig; import java.io.Serializable; import java.util.*; import java.util.Map.Entry; +import java.util.logging.Level; +import java.util.stream.Collectors; /** * Owns the query profiles and query profile types to be handed to the qrs nodes. @@ -29,8 +32,9 @@ public class QueryProfiles implements Serializable, QueryProfilesConfig.Producer * @param registry the registry containing the query profiles and types of this. * The given registry cannot be frozen on calling this. */ - public QueryProfiles(QueryProfileRegistry registry) { + public QueryProfiles(QueryProfileRegistry registry, DeployLogger logger) { this.registry = registry; + validate(registry, logger); } public QueryProfiles() { @@ -41,6 +45,28 @@ public class QueryProfiles implements Serializable, QueryProfilesConfig.Producer return registry; } + /** Emits warnings/hints on some common configuration errors */ + private void validate(QueryProfileRegistry registry, DeployLogger logger) { + Set<String> tensorFields = new HashSet<>(); + for (QueryProfileType type : registry.getTypeRegistry().allComponents()) { + for (var fieldEntry : type.fields().entrySet()) { + if (fieldEntry.getValue().getType().asTensorType().rank() > 0) + tensorFields.add(fieldEntry.getKey()); + } + } + + if ( registry.getTypeRegistry().hasApplicationTypes() && registry.allComponents().isEmpty()) { + logger.log(Level.WARNING, "This application define query profile types, but has " + + "no query profiles referencing them so they have no effect. " + + (tensorFields.isEmpty() + ? "" + : "In particular, the tensors (" + String.join(", ", tensorFields) + + ") will be interpreted as strings, not tensors if sent in requests. ") + + "See https://docs.vespa.ai/documentation/query-profiles.html"); + } + + } + @Override public void getConfig(QueryProfilesConfig.Builder builder) { for (QueryProfile profile : registry.allComponents()) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/search/QueryProfilesBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/search/QueryProfilesBuilder.java index b85cb88bf2e..b832c1bbdcd 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/search/QueryProfilesBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/search/QueryProfilesBuilder.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.container.search; +import com.yahoo.config.application.api.DeployLogger; import com.yahoo.io.reader.NamedReader; import com.yahoo.search.query.profile.config.QueryProfileXMLReader; import com.yahoo.config.application.api.ApplicationPackage; @@ -17,13 +18,14 @@ import java.util.List; public class QueryProfilesBuilder { /** Build the set of query profiles for an application package */ - public QueryProfiles build(ApplicationPackage applicationPackage) { + public QueryProfiles build(ApplicationPackage applicationPackage, DeployLogger logger) { List<NamedReader> queryProfileTypeFiles = null; List<NamedReader> queryProfileFiles = null; try { queryProfileTypeFiles = applicationPackage.getQueryProfileTypeFiles(); queryProfileFiles = applicationPackage.getQueryProfileFiles(); - return new QueryProfiles(new QueryProfileXMLReader().read(queryProfileTypeFiles, queryProfileFiles)); + return new QueryProfiles(new QueryProfileXMLReader().read(queryProfileTypeFiles, queryProfileFiles), + logger); } finally { NamedReader.closeAll(queryProfileTypeFiles); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/QueryProfileVariantsTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/QueryProfileVariantsTestCase.java index 5833bc79ebf..5e559b64bd1 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/QueryProfileVariantsTestCase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/QueryProfileVariantsTestCase.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.container.search.test; +import com.yahoo.config.application.api.DeployLogger; import com.yahoo.search.query.profile.QueryProfile; import com.yahoo.search.query.profile.QueryProfileRegistry; import com.yahoo.search.query.profile.config.QueryProfileXMLReader; @@ -8,6 +9,7 @@ import com.yahoo.vespa.model.container.search.QueryProfiles; import org.junit.Test; import java.io.IOException; +import java.util.logging.Level; import static helpers.CompareConfigTestHelper.assertSerializedConfigFileEquals; import static org.junit.Assert.assertEquals; @@ -22,28 +24,28 @@ public class QueryProfileVariantsTestCase { @Test public void testConfigCreation() throws IOException { QueryProfileRegistry registry = new QueryProfileXMLReader().read(root + "queryprofilevariants"); - QueryProfiles profiles = new QueryProfiles(registry); + QueryProfiles profiles = new QueryProfiles(registry, new SilentDeployLogger()); assertSerializedConfigFileEquals(root + "query-profile-variants-configuration.cfg", profiles.getConfig().toString()); } @Test public void testConfigCreation2() throws IOException { QueryProfileRegistry registry = new QueryProfileXMLReader().read("src/test/java/com/yahoo/vespa/model/container/search/test/queryprofilevariants2"); - QueryProfiles profiles = new QueryProfiles(registry); + QueryProfiles profiles = new QueryProfiles(registry, new SilentDeployLogger()); assertSerializedConfigFileEquals(root + "query-profile-variants2-configuration.cfg", profiles.getConfig().toString()); } @Test public void testConfigCreationNewsBESimple() throws IOException { QueryProfileRegistry registry = new QueryProfileXMLReader().read(root + "newsbesimple"); - QueryProfiles profiles = new QueryProfiles(registry); + QueryProfiles profiles = new QueryProfiles(registry, new SilentDeployLogger()); assertSerializedConfigFileEquals(root + "newsbe-query-profiles-simple.cfg", profiles.getConfig().toString()); } @Test public void testConfigCreationNewsFESimple() throws IOException { QueryProfileRegistry registry = new QueryProfileXMLReader().read(root + "newsfesimple"); - QueryProfiles profiles = new QueryProfiles(registry); + QueryProfiles profiles = new QueryProfiles(registry, new SilentDeployLogger()); assertSerializedConfigFileEquals(root + "newsfe-query-profiles-simple.cfg", profiles.getConfig().toString()); } @@ -63,7 +65,7 @@ public class QueryProfileVariantsTestCase { registry.register(a1); registry.register(profile); - QueryProfiles profiles = new QueryProfiles(registry); + QueryProfiles profiles = new QueryProfiles(registry, new SilentDeployLogger()); assertSerializedConfigFileEquals(root + "variants-of-explicit-compound.cfg", profiles.getConfig().toString()); } @@ -88,7 +90,7 @@ public class QueryProfileVariantsTestCase { registry.register(a2); registry.register(profile); - QueryProfiles profiles = new QueryProfiles(registry); + QueryProfiles profiles = new QueryProfiles(registry, new SilentDeployLogger()); assertSerializedConfigFileEquals(root + "variants-of-explicit-compound-with-reference.cfg", profiles.getConfig().toString()); } @@ -108,8 +110,15 @@ public class QueryProfileVariantsTestCase { registry.register(a1); registry.register(profile); - QueryProfiles profiles = new QueryProfiles(registry); + QueryProfiles profiles = new QueryProfiles(registry, new SilentDeployLogger()); assertSerializedConfigFileEquals(root + "explicit-reference-override.cfg", profiles.getConfig().toString()); } + private static class SilentDeployLogger implements DeployLogger { + + @Override + public void log(Level level, String message) {} + + } + } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/QueryProfilesTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/QueryProfilesTestCase.java index 8c725ecc43c..746e771667f 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/QueryProfilesTestCase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/QueryProfilesTestCase.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.model.container.search.test; import com.yahoo.component.ComponentId; +import com.yahoo.config.application.api.DeployLogger; import com.yahoo.search.query.profile.QueryProfile; import com.yahoo.search.query.profile.QueryProfileRegistry; import com.yahoo.search.query.profile.compiled.CompiledQueryProfileRegistry; @@ -11,12 +12,18 @@ import com.yahoo.search.query.profile.types.FieldDescription; import com.yahoo.search.query.profile.types.FieldType; import com.yahoo.search.query.profile.types.QueryProfileType; import com.yahoo.search.query.profile.types.QueryProfileTypeRegistry; +import com.yahoo.searchdefinition.SearchBuilder; +import com.yahoo.searchdefinition.parser.ParseException; import com.yahoo.vespa.model.container.search.QueryProfiles; +import com.yahoo.vespa.model.test.utils.DeployLoggerStub; import org.junit.Test; import java.io.IOException; +import java.util.logging.Level; +import java.util.logging.Logger; import static helpers.CompareConfigTestHelper.assertSerializedConfigFileEquals; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -112,15 +119,63 @@ public class QueryProfilesTestCase { registry.register(untypedUser); assertConfig("query-profiles.cfg",registry); + + DeployLoggerStub logger = new DeployLoggerStub(); + new QueryProfiles(registry, logger); + assertTrue(logger.entries.isEmpty()); + } + + @Test + public void testValidation() { + QueryProfileRegistry registry = new QueryProfileRegistry(); + QueryProfileTypeRegistry typeRegistry = registry.getTypeRegistry(); + + QueryProfileType userType = new QueryProfileType("user"); + typeRegistry.register(userType); + + DeployLoggerStub logger = new DeployLoggerStub(); + new QueryProfiles(registry, logger); + assertEquals(1, logger.entries.size()); + assertEquals("This application define query profile types, but has no query profiles referencing them " + + "so they have no effect. " + + "See https://docs.vespa.ai/documentation/query-profiles.html", + logger.entries.get(0).message); + } + + @Test + public void testValidationWithTensorFields() { + QueryProfileRegistry registry = new QueryProfileRegistry(); + QueryProfileTypeRegistry typeRegistry = registry.getTypeRegistry(); + + QueryProfileType userType = new QueryProfileType("user"); + userType.addField(new FieldDescription("vector", FieldType.fromString("tensor(x[5])", typeRegistry))); + userType.addField(new FieldDescription("matrix", FieldType.fromString("tensor(x[5],y[5])", typeRegistry))); + typeRegistry.register(userType); + + DeployLoggerStub logger = new DeployLoggerStub(); + new QueryProfiles(registry, logger); + assertEquals(1, logger.entries.size()); + assertEquals("This application define query profile types, but has no query profiles referencing them " + + "so they have no effect. " + + "In particular, the tensors (vector, matrix) will be interpreted as strings, not tensors if sent in requests. " + + "See https://docs.vespa.ai/documentation/query-profiles.html", + logger.entries.get(0).message); } protected void assertConfig(String correctFileName, QueryProfileRegistry check) throws IOException { assertSerializedConfigFileEquals(root + "/" + correctFileName, - com.yahoo.text.StringUtilities.implodeMultiline(com.yahoo.config.ConfigInstance.serialize(new QueryProfiles(check).getConfig()))); + com.yahoo.text.StringUtilities.implodeMultiline(com.yahoo.config.ConfigInstance.serialize(new QueryProfiles(check, new SilentDeployLogger()).getConfig()))); // Also assert that the correct config config can actually be read as a config source QueryProfileConfigurer configurer = new QueryProfileConfigurer("file:" + root + "empty.cfg"); configurer.shutdown(); } + private static class SilentDeployLogger implements DeployLogger { + + @Override + public void log(Level level, String message) {} + + } + } diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index 80038da6750..38ce2aa2cf2 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -6177,6 +6177,7 @@ "methods": [ "public void <init>()", "public void register(com.yahoo.search.query.profile.types.QueryProfileType)", + "public boolean hasApplicationTypes()", "public void freeze()", "public static com.yahoo.search.query.profile.types.QueryProfileTypeRegistry emptyFrozen()" ], diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/types/QueryProfileTypeRegistry.java b/container-search/src/main/java/com/yahoo/search/query/profile/types/QueryProfileTypeRegistry.java index ff8c4845845..b76ae88ede1 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/types/QueryProfileTypeRegistry.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/types/QueryProfileTypeRegistry.java @@ -12,8 +12,11 @@ import com.yahoo.search.query.profile.QueryProfileRegistry; */ public class QueryProfileTypeRegistry extends ComponentRegistry<QueryProfileType> { + private final int nativeProfileCount; + public QueryProfileTypeRegistry() { Query.addNativeQueryProfileTypesTo(this); + nativeProfileCount = allComponents().size(); } /** Register this type by its id */ @@ -21,6 +24,11 @@ public class QueryProfileTypeRegistry extends ComponentRegistry<QueryProfileType super.register(type.getId(), type); } + /** Returns true if this has types in addition to the native Vespa types */ + public boolean hasApplicationTypes() { + return allComponents().size() > nativeProfileCount; + } + @Override public void freeze() { if (isFrozen()) return; |