diff options
25 files changed, 433 insertions, 38 deletions
diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index 3c4eb1046bc..838fd9b6b26 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -133,6 +133,7 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"vekterli"}) default boolean useTwoPhaseDocumentGc() { return false; } @ModelFeatureFlag(owners = {"hmusum"}) default int clusterControllerStateGatherCount() { return 2; } @ModelFeatureFlag(owners = {"tokle"}) default boolean useRestrictedDataPlaneBindings() { return false; } + @ModelFeatureFlag(owners = {"baldersheim", "vekterli"}) default boolean computeCoverageFromTargetActiveDocs() { return false; } } /** Warning: As elsewhere in this package, do not make backwards incompatible changes that will break old config models! */ diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java index 033fc4cd8de..b7b54e749f9 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java @@ -90,6 +90,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea private int mbus_network_threads = 1; private Architecture adminClusterNodeResourcesArchitecture = Architecture.getDefault(); private boolean useRestrictedDataPlaneBindings = false; + private boolean computeCoverageFromTargetActiveDocs = false; @Override public ModelContext.FeatureFlags featureFlags() { return this; } @Override public boolean multitenant() { return multitenant; } @@ -155,6 +156,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea @Override public boolean useTwoPhaseDocumentGc() { return useTwoPhaseDocumentGc; } @Override public String phraseOptimization() { return phraseOptimization; } @Override public boolean useRestrictedDataPlaneBindings() { return useRestrictedDataPlaneBindings; } + @Override public boolean computeCoverageFromTargetActiveDocs() { return computeCoverageFromTargetActiveDocs; } public TestProperties sharedStringRepoNoReclaim(boolean sharedStringRepoNoReclaim) { this.sharedStringRepoNoReclaim = sharedStringRepoNoReclaim; @@ -432,6 +434,11 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea return this; } + public TestProperties setComputeCoverageFromTargetActiveDocs(boolean computeCoverageFromTargetActiveDocs) { + this.computeCoverageFromTargetActiveDocs = computeCoverageFromTargetActiveDocs; + return this; + } + public static class Spec implements ConfigServerSpec { private final String hostName; diff --git a/config-model/src/main/java/com/yahoo/schema/derived/SummaryClass.java b/config-model/src/main/java/com/yahoo/schema/derived/SummaryClass.java index b8aa89fc82a..b9355693da8 100644 --- a/config-model/src/main/java/com/yahoo/schema/derived/SummaryClass.java +++ b/config-model/src/main/java/com/yahoo/schema/derived/SummaryClass.java @@ -5,6 +5,7 @@ import com.yahoo.config.application.api.DeployLogger; import com.yahoo.document.DataType; import com.yahoo.prelude.fastsearch.DocsumDefinitionSet; import com.yahoo.schema.Schema; +import com.yahoo.schema.processing.DynamicSummaryTransformUtils; import com.yahoo.vespa.config.search.SummaryConfig; import com.yahoo.vespa.documentmodel.DocumentSummary; import com.yahoo.vespa.documentmodel.SummaryField; @@ -158,14 +159,10 @@ public class SummaryClass extends Derived { summaryField.getTransform() == SummaryTransform.MATCHED_ATTRIBUTE_ELEMENTS_FILTER) { return summaryField.getSingleSource(); + } else if (summaryField.getTransform().isDynamic()) { + return DynamicSummaryTransformUtils.getSource(summaryField); } else { - // Note: Currently source mapping is handled in the indexing statement, - // by creating a summary field for each of the values - // This works, but is suboptimal. We could consolidate to a minimal set and - // use the right value from the minimal set as the third parameter here, - // and add "override" commands to multiple static values - boolean useFieldNameAsArgument = summaryField.getTransform().isDynamic(); - return useFieldNameAsArgument ? summaryField.getName() : ""; + return ""; } } diff --git a/config-model/src/main/java/com/yahoo/schema/processing/AddExtraFieldsToDocument.java b/config-model/src/main/java/com/yahoo/schema/processing/AddExtraFieldsToDocument.java index 3afc25131c0..77a17878840 100644 --- a/config-model/src/main/java/com/yahoo/schema/processing/AddExtraFieldsToDocument.java +++ b/config-model/src/main/java/com/yahoo/schema/processing/AddExtraFieldsToDocument.java @@ -10,6 +10,7 @@ import com.yahoo.schema.document.ImmutableSDField; import com.yahoo.schema.document.SDDocumentType; import com.yahoo.schema.document.SDField; import com.yahoo.vespa.documentmodel.SummaryField; +import com.yahoo.vespa.documentmodel.SummaryTransform; import com.yahoo.vespa.model.container.search.QueryProfiles; /** @@ -33,15 +34,14 @@ public class AddExtraFieldsToDocument extends Processor { } for (var docsum : schema.getSummaries().values()) { for (var summaryField : docsum.getSummaryFields().values()) { - switch (summaryField.getTransform()) { - case NONE: - case BOLDED: - case DYNAMICBOLDED: - case DYNAMICTEASER: - case DOCUMENT_ID: // TODO: Adding the 'documentid' field should no longer be needed when the docsum framework in the backend has been simplified and the transform is always used. + var transform = summaryField.getTransform(); + if (transform.isDynamic() && DynamicSummaryTransformUtils.summaryFieldIsRequiredInDocumentType(summaryField) || + transform == SummaryTransform.NONE || + transform == SummaryTransform.DOCUMENT_ID) + { + // TODO: Adding the 'documentid' field should no longer be needed when the docsum framework in the backend has been simplified and the transform is always used. addSummaryField(schema, document, summaryField, validate); - break; - default: + } else { // skip: generated from attribute or similar, // so does not need to be included as an extra // field in the document type diff --git a/config-model/src/main/java/com/yahoo/schema/processing/Bolding.java b/config-model/src/main/java/com/yahoo/schema/processing/Bolding.java index 53a3d462d54..73ad4225c88 100644 --- a/config-model/src/main/java/com/yahoo/schema/processing/Bolding.java +++ b/config-model/src/main/java/com/yahoo/schema/processing/Bolding.java @@ -3,7 +3,6 @@ package com.yahoo.schema.processing; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.schema.RankProfileRegistry; -import com.yahoo.document.DataType; import com.yahoo.schema.document.ImmutableSDField; import com.yahoo.schema.Schema; import com.yahoo.vespa.documentmodel.SummaryField; @@ -26,16 +25,12 @@ public class Bolding extends Processor { if ( ! validate) return; for (ImmutableSDField field : schema.allConcreteFields()) { for (SummaryField summary : field.getSummaryFields().values()) { - if (summary.getTransform().isBolded() && - !((summary.getDataType() == DataType.STRING) || (summary.getDataType() == DataType.URI))) - { + if (summary.getTransform().isBolded() && !DynamicSummaryTransformUtils.hasSupportedType(summary)) { throw new IllegalArgumentException("'bolding: on' for non-text field " + "'" + field.getName() + "'" + " (" + summary.getDataType() + ")" + " is not allowed"); - } else if (summary.getTransform().isDynamic() && - !((summary.getDataType() == DataType.STRING) || (summary.getDataType() == DataType.URI))) - { + } else if (summary.getTransform().isDynamic() && !DynamicSummaryTransformUtils.hasSupportedType(summary)) { throw new IllegalArgumentException("'summary: dynamic' for non-text field " + "'" + field.getName() + "'" + " (" + summary.getDataType() + ")" + diff --git a/config-model/src/main/java/com/yahoo/schema/processing/DynamicSummaryTransformUtils.java b/config-model/src/main/java/com/yahoo/schema/processing/DynamicSummaryTransformUtils.java new file mode 100644 index 00000000000..20597eca64f --- /dev/null +++ b/config-model/src/main/java/com/yahoo/schema/processing/DynamicSummaryTransformUtils.java @@ -0,0 +1,59 @@ +package com.yahoo.schema.processing; + +import com.yahoo.document.DataType; +import com.yahoo.vespa.documentmodel.SummaryField; + +/** + * This class contains utils used when handling summary fields with dynamic transforms during processing and deriving. + * + * Originally (before Vespa 8.52), dynamic transforms where only supported for string fields. + * Due to legacy functionality in the backend docsum framework, + * such summary fields are in some cases added as extra document fields and populated in indexing scripts. + * This is something we want to avoid in the future, but it might not be entirely possible before Vespa 9. + * + * With the introduction of dynamic transform for array of string fields, + * we move in the right direction and avoid adding extra document fields with indexing script population for this type. + * Instead, we configure the dynamic transform in the backend to use the original source field directly. + * + * See SummaryTransform.isDynamic() for which transforms this applies to. + */ +public class DynamicSummaryTransformUtils { + + public static boolean hasSupportedType(SummaryField field) { + return isSupportedType(field.getDataType()); + } + + public static boolean isSupportedType(DataType type) { + return isOriginalSupportedType(type) || isNewSupportedType(type); + } + + private static boolean isOriginalSupportedType(DataType type) { + return (type == DataType.STRING) || + (type == DataType.URI); + } + + private static boolean isNewSupportedType(DataType type) { + return (type.equals(DataType.getArray(DataType.STRING))); + } + + /** + * Whether a summary field must be populated by the source field with the given type in an indexing script. + */ + public static boolean summaryFieldIsPopulatedBySourceField(DataType sourceFieldType) { + return isOriginalSupportedType(sourceFieldType); + } + + /** + * Whether a summary field is required as an extra field in the document type. + */ + public static boolean summaryFieldIsRequiredInDocumentType(SummaryField summaryField) { + return summaryFieldIsPopulatedBySourceField(summaryField.getDataType()); + } + + public static String getSource(SummaryField summaryField) { + // Summary fields with the original supported type is always present in the document type, + // and we must use that field as source at run-time. + return isOriginalSupportedType(summaryField.getDataType()) ? + summaryField.getName() : summaryField.getSingleSource(); + } +} diff --git a/config-model/src/main/java/com/yahoo/schema/processing/IndexingOutputs.java b/config-model/src/main/java/com/yahoo/schema/processing/IndexingOutputs.java index ea65a223686..2c24d3e53e1 100644 --- a/config-model/src/main/java/com/yahoo/schema/processing/IndexingOutputs.java +++ b/config-model/src/main/java/com/yahoo/schema/processing/IndexingOutputs.java @@ -68,11 +68,13 @@ public class IndexingOutputs extends Processor { } if (summaryTransform.isDynamic()) { DataType fieldType = field.getDataType(); - if (fieldType != DataType.URI && fieldType != DataType.STRING) { - warn(schema, field, "Dynamic summaries are only supported for fields of type " + - "string, ignoring summary field '" + summaryField.getName() + - "' for sd field '" + field.getName() + "' of type " + - fieldType.getName() + "."); + if (!DynamicSummaryTransformUtils.summaryFieldIsPopulatedBySourceField(fieldType)) { + if (!DynamicSummaryTransformUtils.isSupportedType(fieldType)) { + warn(schema, field, "Dynamic summaries are only supported for fields of type " + + "string and array<string>, ignoring summary field '" + summaryField.getName() + + "' for sd field '" + field.getName() + "' of type " + + fieldType.getName() + "."); + } return; } dynamicSummary.add(summaryName); diff --git a/config-model/src/main/java/com/yahoo/schema/processing/SummaryDynamicStructsArrays.java b/config-model/src/main/java/com/yahoo/schema/processing/SummaryDynamicStructsArrays.java index ed1f47611eb..a899f5e82ab 100644 --- a/config-model/src/main/java/com/yahoo/schema/processing/SummaryDynamicStructsArrays.java +++ b/config-model/src/main/java/com/yahoo/schema/processing/SummaryDynamicStructsArrays.java @@ -13,7 +13,7 @@ import com.yahoo.vespa.model.container.search.QueryProfiles; /** * Fail if: - * An SD field explicitly says summary:dynamic , but the field is wset, array or struct. + * An SD field explicitly says summary:dynamic , but the field is non-string array, wset, or struct. * If there is an explicitly defined summary class, saying dynamic in one of its summary * fields is always legal. * @@ -31,7 +31,7 @@ public class SummaryDynamicStructsArrays extends Processor { for (SDField field : schema.allConcreteFields()) { DataType type = field.getDataType(); - if (type instanceof ArrayDataType || type instanceof WeightedSetDataType || type instanceof StructDataType) { + if (isNonStringArray(type) || type instanceof WeightedSetDataType || type instanceof StructDataType) { for (SummaryField sField : field.getSummaryFields().values()) { if (sField.getTransform().equals(SummaryTransform.DYNAMICTEASER)) { throw new IllegalArgumentException("For field '"+field.getName()+"': dynamic summary is illegal " + @@ -44,4 +44,8 @@ public class SummaryDynamicStructsArrays extends Processor { } } + private boolean isNonStringArray(DataType type) { + return (type instanceof ArrayDataType) && (!type.equals(DataType.getArray(DataType.STRING))); + } + } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java index a3ee8d56142..94eb13dd967 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java @@ -60,6 +60,7 @@ public class IndexedSearchCluster extends SearchCluster private DispatchSpec dispatchSpec; private final List<SearchNode> searchNodes = new ArrayList<>(); private final DispatchTuning.DispatchPolicy defaultDispatchPolicy; + private final boolean computeCoverageFromTargetActiveDocs; /** * Returns the document selector that is able to resolve what documents are to be routed to this search cluster. @@ -76,6 +77,7 @@ public class IndexedSearchCluster extends SearchCluster documentDbsConfigProducer = new MultipleDocumentDatabasesConfigProducer(this, documentDbs); rootDispatch = new DispatchGroup(this); defaultDispatchPolicy = DispatchTuning.Builder.toDispatchPolicy(featureFlags.queryDispatchPolicy()); + computeCoverageFromTargetActiveDocs = featureFlags.computeCoverageFromTargetActiveDocs(); } @Override @@ -320,6 +322,7 @@ public class IndexedSearchCluster extends SearchCluster builder.searchableCopies(rootDispatch.getSearchableCopies()); builder.redundancy(rootDispatch.getRedundancy()); + builder.computeCoverageFromTargetActiveDocs(computeCoverageFromTargetActiveDocs); if (searchCoverage != null) { if (searchCoverage.getMinimum() != null) builder.minSearchCoverage(searchCoverage.getMinimum() * 100.0); diff --git a/config-model/src/test/derived/bolding_dynamic_summary/documenttypes.cfg b/config-model/src/test/derived/bolding_dynamic_summary/documenttypes.cfg new file mode 100644 index 00000000000..f88a22d8979 --- /dev/null +++ b/config-model/src/test/derived/bolding_dynamic_summary/documenttypes.cfg @@ -0,0 +1,104 @@ +ignoreundefinedfields false +usev8geopositions false +doctype[].name "document" +doctype[].idx 10000 +doctype[].internalid 8 +doctype[].contentstruct 10001 +doctype[].primitivetype[].idx 10002 +doctype[].primitivetype[].name "bool" +doctype[].primitivetype[].idx 10003 +doctype[].primitivetype[].name "byte" +doctype[].primitivetype[].idx 10004 +doctype[].primitivetype[].name "double" +doctype[].primitivetype[].idx 10005 +doctype[].primitivetype[].name "float" +doctype[].primitivetype[].idx 10006 +doctype[].primitivetype[].name "float16" +doctype[].primitivetype[].idx 10007 +doctype[].primitivetype[].name "int" +doctype[].primitivetype[].idx 10008 +doctype[].primitivetype[].name "long" +doctype[].primitivetype[].idx 10010 +doctype[].primitivetype[].name "predicate" +doctype[].primitivetype[].idx 10011 +doctype[].primitivetype[].name "raw" +doctype[].primitivetype[].idx 10012 +doctype[].primitivetype[].name "string" +doctype[].primitivetype[].idx 10014 +doctype[].primitivetype[].name "uri" +doctype[].wsettype[].idx 10013 +doctype[].wsettype[].elementtype 10012 +doctype[].wsettype[].createifnonexistent true +doctype[].wsettype[].removeifzero true +doctype[].wsettype[].internalid 18 +doctype[].structtype[].idx 10001 +doctype[].structtype[].name "document.header" +doctype[].structtype[].internalid -284186494 +doctype[].structtype[].idx 10009 +doctype[].structtype[].name "position" +doctype[].structtype[].field[].name "x" +doctype[].structtype[].field[].internalid 914677694 +doctype[].structtype[].field[].type 10007 +doctype[].structtype[].field[].name "y" +doctype[].structtype[].field[].internalid 900009410 +doctype[].structtype[].field[].type 10007 +doctype[].structtype[].internalid 1381038251 +doctype[].name "test" +doctype[].idx 10015 +doctype[].internalid -877171244 +doctype[].inherits[].idx 10000 +doctype[].contentstruct 10016 +doctype[].fieldsets{[]}.fields[] "arr_1" +doctype[].fieldsets{[]}.fields[] "arr_2" +doctype[].fieldsets{[]}.fields[] "arr_3" +doctype[].fieldsets{[]}.fields[] "arr_4" +doctype[].fieldsets{[]}.fields[] "str_1" +doctype[].fieldsets{[]}.fields[] "str_2" +doctype[].fieldsets{[]}.fields[] "str_3" +doctype[].fieldsets{[]}.fields[] "str_4" +doctype[].arraytype[].idx 10017 +doctype[].arraytype[].elementtype 10012 +doctype[].arraytype[].internalid -1486737430 +doctype[].arraytype[].idx 10018 +doctype[].arraytype[].elementtype 10012 +doctype[].arraytype[].internalid -1486737430 +doctype[].arraytype[].idx 10019 +doctype[].arraytype[].elementtype 10012 +doctype[].arraytype[].internalid -1486737430 +doctype[].arraytype[].idx 10020 +doctype[].arraytype[].elementtype 10012 +doctype[].arraytype[].internalid -1486737430 +doctype[].structtype[].idx 10016 +doctype[].structtype[].name "test.header" +doctype[].structtype[].field[].name "str_1" +doctype[].structtype[].field[].internalid 91206053 +doctype[].structtype[].field[].type 10012 +doctype[].structtype[].field[].name "str_2" +doctype[].structtype[].field[].internalid 1236519725 +doctype[].structtype[].field[].type 10012 +doctype[].structtype[].field[].name "str_3" +doctype[].structtype[].field[].internalid 1257861515 +doctype[].structtype[].field[].type 10012 +doctype[].structtype[].field[].name "str_4" +doctype[].structtype[].field[].internalid 2009526185 +doctype[].structtype[].field[].type 10012 +doctype[].structtype[].field[].name "arr_1" +doctype[].structtype[].field[].internalid 618314164 +doctype[].structtype[].field[].type 10017 +doctype[].structtype[].field[].name "arr_2" +doctype[].structtype[].field[].internalid 1363829203 +doctype[].structtype[].field[].type 10018 +doctype[].structtype[].field[].name "arr_3" +doctype[].structtype[].field[].internalid 815179933 +doctype[].structtype[].field[].type 10019 +doctype[].structtype[].field[].name "arr_4" +doctype[].structtype[].field[].internalid 142340927 +doctype[].structtype[].field[].type 10020 +doctype[].structtype[].field[].name "str_3_dyn" +doctype[].structtype[].field[].internalid 1779429789 +doctype[].structtype[].field[].type 10012 +doctype[].structtype[].field[].name "str_4_bold" +doctype[].structtype[].field[].internalid 1385662257 +doctype[].structtype[].field[].type 10012 +doctype[].structtype[].internalid 306916075 + diff --git a/config-model/src/test/derived/bolding_dynamic_summary/ilscripts.cfg b/config-model/src/test/derived/bolding_dynamic_summary/ilscripts.cfg new file mode 100644 index 00000000000..f9fdab87297 --- /dev/null +++ b/config-model/src/test/derived/bolding_dynamic_summary/ilscripts.cfg @@ -0,0 +1,19 @@ +maxtermoccurrences 100 +fieldmatchmaxlength 1000000 +ilscript[].doctype "test" +ilscript[].docfield[] "str_1" +ilscript[].docfield[] "str_2" +ilscript[].docfield[] "str_3" +ilscript[].docfield[] "str_4" +ilscript[].docfield[] "arr_1" +ilscript[].docfield[] "arr_2" +ilscript[].docfield[] "arr_3" +ilscript[].docfield[] "arr_4" +ilscript[].content[] "clear_state | guard { input str_1 | tokenize normalize stem:\"BEST\" | index str_1 | summary str_1; }" +ilscript[].content[] "clear_state | guard { input str_2 | tokenize normalize stem:\"BEST\" | index str_2 | summary str_2; }" +ilscript[].content[] "clear_state | guard { input str_3 | tokenize normalize stem:\"BEST\" | index str_3 | summary str_3 | summary str_3_dyn; }" +ilscript[].content[] "clear_state | guard { input str_4 | tokenize normalize stem:\"BEST\" | index str_4 | summary str_4 | summary str_4_bold; }" +ilscript[].content[] "clear_state | guard { input arr_1 | for_each { tokenize normalize stem:\"BEST\" } | index arr_1 | summary arr_1; }" +ilscript[].content[] "clear_state | guard { input arr_2 | for_each { tokenize normalize stem:\"BEST\" } | index arr_2 | summary arr_2; }" +ilscript[].content[] "clear_state | guard { input arr_3 | for_each { tokenize normalize stem:\"BEST\" } | index arr_3 | summary arr_3; }" +ilscript[].content[] "clear_state | guard { input arr_4 | for_each { tokenize normalize stem:\"BEST\" } | index arr_4 | summary arr_4; }" diff --git a/config-model/src/test/derived/bolding_dynamic_summary/summary.cfg b/config-model/src/test/derived/bolding_dynamic_summary/summary.cfg new file mode 100644 index 00000000000..b056ada8bd5 --- /dev/null +++ b/config-model/src/test/derived/bolding_dynamic_summary/summary.cfg @@ -0,0 +1,92 @@ +defaultsummaryid 1128441658 +usev8geopositions true +classes[].id 1128441658 +classes[].name "default" +classes[].omitsummaryfeatures false +classes[].fields[].name "str_1" +classes[].fields[].type "longstring" +classes[].fields[].command "dynamicteaser" +classes[].fields[].source "str_1" +classes[].fields[].name "str_2" +classes[].fields[].type "longstring" +classes[].fields[].command "dynamicteaser" +classes[].fields[].source "str_2" +classes[].fields[].name "str_3" +classes[].fields[].type "longstring" +classes[].fields[].command "" +classes[].fields[].source "" +classes[].fields[].name "str_4" +classes[].fields[].type "longstring" +classes[].fields[].command "" +classes[].fields[].source "" +classes[].fields[].name "arr_1" +classes[].fields[].type "jsonstring" +classes[].fields[].command "dynamicteaser" +classes[].fields[].source "arr_1" +classes[].fields[].name "arr_2" +classes[].fields[].type "jsonstring" +classes[].fields[].command "dynamicteaser" +classes[].fields[].source "arr_2" +classes[].fields[].name "arr_3" +classes[].fields[].type "jsonstring" +classes[].fields[].command "" +classes[].fields[].source "" +classes[].fields[].name "arr_4" +classes[].fields[].type "jsonstring" +classes[].fields[].command "" +classes[].fields[].source "" +classes[].fields[].name "rankfeatures" +classes[].fields[].type "featuredata" +classes[].fields[].command "rankfeatures" +classes[].fields[].source "" +classes[].fields[].name "summaryfeatures" +classes[].fields[].type "featuredata" +classes[].fields[].command "summaryfeatures" +classes[].fields[].source "" +classes[].fields[].name "str_3_dyn" +classes[].fields[].type "longstring" +classes[].fields[].command "dynamicteaser" +classes[].fields[].source "str_3_dyn" +classes[].fields[].name "arr_3_dyn" +classes[].fields[].type "jsonstring" +classes[].fields[].command "dynamicteaser" +classes[].fields[].source "arr_3" +classes[].fields[].name "str_4_bold" +classes[].fields[].type "longstring" +classes[].fields[].command "dynamicteaser" +classes[].fields[].source "str_4_bold" +classes[].fields[].name "arr_4_bold" +classes[].fields[].type "jsonstring" +classes[].fields[].command "dynamicteaser" +classes[].fields[].source "arr_4" +classes[].fields[].name "documentid" +classes[].fields[].type "longstring" +classes[].fields[].command "documentid" +classes[].fields[].source "" +classes[].id 2139497711 +classes[].name "dyn" +classes[].omitsummaryfeatures false +classes[].fields[].name "str_3_dyn" +classes[].fields[].type "longstring" +classes[].fields[].command "dynamicteaser" +classes[].fields[].source "str_3_dyn" +classes[].fields[].name "arr_3_dyn" +classes[].fields[].type "jsonstring" +classes[].fields[].command "dynamicteaser" +classes[].fields[].source "arr_3" +classes[].fields[].name "str_4_bold" +classes[].fields[].type "longstring" +classes[].fields[].command "dynamicteaser" +classes[].fields[].source "str_4_bold" +classes[].fields[].name "arr_4_bold" +classes[].fields[].type "jsonstring" +classes[].fields[].command "dynamicteaser" +classes[].fields[].source "arr_4" +classes[].fields[].name "rankfeatures" +classes[].fields[].type "featuredata" +classes[].fields[].command "rankfeatures" +classes[].fields[].source "" +classes[].fields[].name "summaryfeatures" +classes[].fields[].type "featuredata" +classes[].fields[].command "summaryfeatures" +classes[].fields[].source "" diff --git a/config-model/src/test/derived/bolding_dynamic_summary/test.sd b/config-model/src/test/derived/bolding_dynamic_summary/test.sd new file mode 100644 index 00000000000..caa7ca2cd2e --- /dev/null +++ b/config-model/src/test/derived/bolding_dynamic_summary/test.sd @@ -0,0 +1,50 @@ +schema test { + document test { + field str_1 type string { + indexing: index | summary + summary: dynamic + } + field str_2 type string { + indexing: index | summary + bolding: on + } + field str_3 type string { + indexing: index | summary + } + field str_4 type string { + indexing: index | summary + } + field arr_1 type array<string> { + indexing: index | summary + summary: dynamic + } + field arr_2 type array<string> { + indexing: index | summary + bolding: on + } + field arr_3 type array<string> { + indexing: index | summary + } + field arr_4 type array<string> { + indexing: index | summary + } + } + document-summary dyn { + summary str_3_dyn type string { + source: str_3 + dynamic + } + summary arr_3_dyn type array<string> { + source: arr_3 + dynamic + } + summary str_4_bold type string { + source: str_4 + bolding: on + } + summary arr_4_bold type array<string> { + source: arr_4 + bolding: on + } + } +} diff --git a/config-model/src/test/java/com/yahoo/schema/derived/ExportingTestCase.java b/config-model/src/test/java/com/yahoo/schema/derived/ExportingTestCase.java index b5b075ae260..c1e65abb5a5 100644 --- a/config-model/src/test/java/com/yahoo/schema/derived/ExportingTestCase.java +++ b/config-model/src/test/java/com/yahoo/schema/derived/ExportingTestCase.java @@ -192,4 +192,9 @@ public class ExportingTestCase extends AbstractExportingTestCase { assertCorrectDeriving("structandfieldset"); } + @Test + void testBoldingAndDynamicSummary() throws IOException, ParseException { + assertCorrectDeriving("bolding_dynamic_summary"); + } + } diff --git a/config-model/src/test/java/com/yahoo/schema/processing/BoldingTestCase.java b/config-model/src/test/java/com/yahoo/schema/processing/BoldingTestCase.java index fe38ea1f1b1..7162cf7717b 100644 --- a/config-model/src/test/java/com/yahoo/schema/processing/BoldingTestCase.java +++ b/config-model/src/test/java/com/yahoo/schema/processing/BoldingTestCase.java @@ -39,10 +39,10 @@ public class BoldingTestCase extends AbstractSchemaTestCase { } } - private final String boldonarray = - "search boldonarray {\n" + - " document boldonarray {\n" + - " field myarray type array<string> {\n" + + private final String boldonwset = + "search test {\n" + + " document test {\n" + + " field mywset type weightedset<string> {\n" + " indexing: summary | index\n" + " bolding: on\n" + " }\n" + @@ -50,12 +50,12 @@ public class BoldingTestCase extends AbstractSchemaTestCase { "}\n"; @Test - void testBoldOnArray() throws ParseException { + void testBoldOnWsetThrowsException() throws ParseException { try { - ApplicationBuilder.createFromString(boldonarray); + ApplicationBuilder.createFromString(boldonwset); fail("Expected exception"); } catch (IllegalArgumentException e) { - assertEquals("'bolding: on' for non-text field 'myarray' (datatype Array<string> (code: -1486737430)) is not allowed", + assertEquals("'bolding: on' for non-text field 'mywset' (datatype WeightedSet<string> (code: 1328286588)) is not allowed", e.getMessage()); } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java index ac291fc578f..132255570ac 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java @@ -1036,6 +1036,27 @@ public class ContentClusterTest extends ContentBaseTest { } } + private boolean coverageIsComputedFromTargetActive(Boolean coverageFromTargetActive) { + TestProperties properties = new TestProperties(); + if (coverageFromTargetActive != null) { + properties.setComputeCoverageFromTargetActiveDocs(coverageFromTargetActive); + } + VespaModel model = createEnd2EndOneNode(properties); + + ContentCluster cc = model.getContentClusters().get("storage"); + DispatchConfig.Builder builder = new DispatchConfig.Builder(); + cc.getSearch().getConfig(builder); + + return (new DispatchConfig(builder)).computeCoverageFromTargetActiveDocs(); + } + + @Test + public void coverage_from_target_active_dispatch_config_is_controlled_by_properties() { + assertFalse(coverageIsComputedFromTargetActive(null)); // TODO update when default changes + assertFalse(coverageIsComputedFromTargetActive(false)); + assertTrue(coverageIsComputedFromTargetActive(true)); + } + private boolean resolveThreePhaseUpdateConfigWithFeatureFlag(boolean flagEnableThreePhase) { VespaModel model = createEnd2EndOneNode(new TestProperties().setUseThreePhaseUpdates(flagEnableThreePhase)); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java index 764e56c1bba..ee5cf57a396 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java @@ -15,6 +15,7 @@ import java.util.List; import static com.yahoo.config.model.test.TestUtil.joinLines; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; /** @@ -97,6 +98,7 @@ public class ClusterTest { DispatchConfig config = new DispatchConfig(builder); assertEquals(2, config.searchableCopies()); assertEquals(3, config.redundancy()); + assertFalse(config.computeCoverageFromTargetActiveDocs()); assertEquals(DispatchConfig.DistributionPolicy.ADAPTIVE, config.distributionPolicy()); assertEquals(1.0, config.maxWaitAfterCoverageFactor(), DELTA); assertEquals(0, config.minWaitAfterCoverageFactor(), DELTA); diff --git a/configdefinitions/src/vespa/dispatch.def b/configdefinitions/src/vespa/dispatch.def index 9addfca1559..fb3fc4a331a 100644 --- a/configdefinitions/src/vespa/dispatch.def +++ b/configdefinitions/src/vespa/dispatch.def @@ -73,3 +73,7 @@ node[].port int # TODO(bjorncs) Remove after May 2022 # Temporary feature flag mergeGroupingResultInSearchInvokerEnabled bool default=false + +# Whether degraded coverage computation will take target active docs into +# account, not just currently active docs. +computeCoverageFromTargetActiveDocs bool default=false diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java index cdbf66f1734..d59b5da84da 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java @@ -225,6 +225,7 @@ public class ModelContextImpl implements ModelContext { private final int rpc_events_before_wakeup; private final int clusterControllerStateGatherCount; private final boolean useRestrictedDataPlaneBindings; + private final boolean computeCoverageFromTargetActiveDocs; public FeatureFlags(FlagSource source, ApplicationId appId, Version version) { this.defaultTermwiseLimit = flagValue(source, appId, version, Flags.DEFAULT_TERM_WISE_LIMIT); @@ -286,6 +287,7 @@ public class ModelContextImpl implements ModelContext { this.phraseOptimization = flagValue(source, appId, version, Flags.PHRASE_OPTIMIZATION); this.clusterControllerStateGatherCount = flagValue(source, appId, version, Flags.CLUSTER_CONTROLLER_STATE_GATHER_COUNT); this.useRestrictedDataPlaneBindings = flagValue(source, appId, version, Flags.RESTRICT_DATA_PLANE_BINDINGS); + this.computeCoverageFromTargetActiveDocs = flagValue(source, appId, version, Flags.COMPUTE_COVERAGE_FROM_TARGET_ACTIVE_DOCS); } @Override public String queryDispatchPolicy() { return queryDispatchPolicy; } @@ -355,6 +357,7 @@ public class ModelContextImpl implements ModelContext { @Override public boolean useTwoPhaseDocumentGc() { return useTwoPhaseDocumentGc; } @Override public int clusterControllerStateGatherCount() { return clusterControllerStateGatherCount; } @Override public boolean useRestrictedDataPlaneBindings() { return useRestrictedDataPlaneBindings; } + @Override public boolean computeCoverageFromTargetActiveDocs() { return computeCoverageFromTargetActiveDocs; } private static <V> V flagValue(FlagSource source, ApplicationId appId, Version vespaVersion, UnboundFlag<? extends V, ?, ?> flag) { return flag.bindTo(source) diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java index 82d3d98d9ef..25617e48aa1 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java @@ -307,6 +307,7 @@ public class SearchCluster implements NodeManager<Node> { if (fullCoverage) { log.info("Cluster " + clusterId + ": " + group + " has full coverage. " + "Active documents: " + group.activeDocuments() + "/" + medianDocuments + ", " + + "Target active documents: " + group.targetActiveDocuments() + ", " + "working nodes: " + group.workingNodes() + "/" + group.nodes().size()); } else { StringBuilder unresponsive = new StringBuilder(); @@ -316,6 +317,7 @@ public class SearchCluster implements NodeManager<Node> { } log.warning("Cluster " + clusterId + ": " + group + " has reduced coverage: " + "Active documents: " + group.activeDocuments() + "/" + medianDocuments + ", " + + "Target active documents: " + group.targetActiveDocuments() + ", " + "working nodes: " + group.workingNodes() + "/" + group.nodes().size() + ", unresponsive nodes: " + (unresponsive.toString().isEmpty() ? " none" : unresponsive)); } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index b3257ec0052..12bf24eb737 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -515,6 +515,14 @@ public class Flags { "Lock scheme for tenant-related controller locks (valid values: OLD, BOTH, NEW)", "Takes effect immediately"); + public static final UnboundBooleanFlag COMPUTE_COVERAGE_FROM_TARGET_ACTIVE_DOCS = defineFeatureFlag( + "compute-coverage-from-target-active-docs", false, + List.of("baldersheim", "vekterli"), "2022-09-13", "2022-11-01", + "Whether degraded coverage computation will take target active docs into " + + "account, not just currently active docs", + "Takes effect at redeployment", + ZONE_ID, APPLICATION_ID); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, List<String> owners, String createdAt, String expiresAt, String description, diff --git a/searchcore/src/tests/proton/docsummary/docsummary.cpp b/searchcore/src/tests/proton/docsummary/docsummary.cpp index 6abaec6ec93..6001fb0c000 100644 --- a/searchcore/src/tests/proton/docsummary/docsummary.cpp +++ b/searchcore/src/tests/proton/docsummary/docsummary.cpp @@ -323,6 +323,9 @@ public: void insert_juniper_field(vespalib::stringref input, vespalib::slime::Inserter&) override { _result = input; } + void insert_juniper_field(const document::StringFieldValue& input, vespalib::slime::Inserter&) override { + _result = input.getValueRef(); + } const vespalib::string& get_result() const noexcept { return _result; } }; diff --git a/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp index 4fb20bb1458..fcdb81defbf 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp @@ -5,6 +5,7 @@ #include "i_docsum_store_document.h" #include "i_juniper_converter.h" #include "juniper_query_adapter.h" +#include <vespa/document/fieldvalue/stringfieldvalue.h> #include <vespa/vespalib/objects/hexdump.h> #include <vespa/juniper/config.h> #include <vespa/juniper/result.h> @@ -114,6 +115,7 @@ public: JuniperConverter(const DynamicTeaserDFW& writer, uint32_t doc_id, GetDocsumsState& state); ~JuniperConverter() override; void insert_juniper_field(vespalib::stringref input, vespalib::slime::Inserter& inserter) override; + void insert_juniper_field(const document::StringFieldValue &input, vespalib::slime::Inserter& inserter) override; }; JuniperConverter::JuniperConverter(const DynamicTeaserDFW& writer, uint32_t doc_id, GetDocsumsState& state) @@ -132,6 +134,12 @@ JuniperConverter::insert_juniper_field(vespalib::stringref input, vespalib::slim _writer.insert_juniper_field(_doc_id, input, _state, inserter); } +void +JuniperConverter::insert_juniper_field(const document::StringFieldValue& input, vespalib::slime::Inserter& inserter) +{ + _writer.insert_juniper_field(_doc_id, input.getValueRef(), _state, inserter); +} + } void diff --git a/searchsummary/src/vespa/searchsummary/docsummary/i_juniper_converter.h b/searchsummary/src/vespa/searchsummary/docsummary/i_juniper_converter.h index f0b8ec4309f..00751082567 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/i_juniper_converter.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/i_juniper_converter.h @@ -4,6 +4,7 @@ #include <vespa/vespalib/stllike/string.h> +namespace document { class StringFieldValue; } namespace vespalib::slime { struct Inserter; } namespace search::docsummary { @@ -11,12 +12,17 @@ namespace search::docsummary { /** * Interface class for inserting a dynamic string based on an * annotated full string and query context. + * + * For streaming search we use the same interface in an adapter that + * calls a snippet modifier (vsm::SnippetModifier) to add the annotation + * needed by juniper. */ class IJuniperConverter { public: virtual ~IJuniperConverter() = default; virtual void insert_juniper_field(vespalib::stringref input, vespalib::slime::Inserter& inserter) = 0; + virtual void insert_juniper_field(const document::StringFieldValue &input, vespalib::slime::Inserter& inserter) = 0; }; } diff --git a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.cpp b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.cpp index 725ebff1f77..b3d3fde7150 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.cpp @@ -189,9 +189,9 @@ SlimeFiller::visit(const StringFieldValue& value) } } else { if (_juniper_converter != nullptr) { - _juniper_converter->insert_juniper_field(value.getValue(), _inserter); + _juniper_converter->insert_juniper_field(value, _inserter); } else { - _inserter.insertString(Memory(value.getValue())); + _inserter.insertString(Memory(value.getValueRef())); } } } |