diff options
54 files changed, 1028 insertions, 438 deletions
diff --git a/application/src/main/java/com/yahoo/application/container/DocumentProcessing.java b/application/src/main/java/com/yahoo/application/container/DocumentProcessing.java index f89e79c4cfa..f0f85fb78dd 100644 --- a/application/src/main/java/com/yahoo/application/container/DocumentProcessing.java +++ b/application/src/main/java/com/yahoo/application/container/DocumentProcessing.java @@ -62,8 +62,10 @@ public final class DocumentProcessing { @SuppressWarnings("removal") // TODO Vespa 8: remove public DocumentProcessor.Progress process(ComponentSpecification chain, com.yahoo.docproc.Processing processing) { DocprocExecutor executor = getExecutor(chain); - // TODO Vespa 8: Use TBD instead, this method will be removed + + // TODO Vespa 8: Remove statement (registry will be removed from Processing) processing.setDocprocServiceRegistry(handler.getDocprocServiceRegistry()); + return executor.processUntilDone(processing); } @@ -83,8 +85,10 @@ public final class DocumentProcessing { @SuppressWarnings("removal") // TODO Vespa 8: remove public DocumentProcessor.Progress processOnce(ComponentSpecification chain, com.yahoo.docproc.Processing processing) { DocprocExecutor executor = getExecutor(chain); - // TODO Vespa 8: Use TBD instead, this method will be removed + + // TODO Vespa 8: Remove statement (registry will be removed from Processing) processing.setDocprocServiceRegistry(handler.getDocprocServiceRegistry()); + return executor.process(processing); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/DocumentModelBuilder.java b/config-model/src/main/java/com/yahoo/searchdefinition/DocumentModelBuilder.java index 88a98b94ddc..0db810d5933 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/DocumentModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/DocumentModelBuilder.java @@ -34,6 +34,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.IdentityHashMap; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; @@ -155,9 +156,9 @@ public class DocumentModelBuilder { private static void addSearchField(SDField field, SearchDef searchDef) { SearchField searchField = - new SearchField(field, - field.getIndices().containsKey(field.getName()) && field.getIndices().get(field.getName()).getType().equals(Index.Type.VESPA), - field.getAttributes().containsKey(field.getName())); + new SearchField(field, + field.getIndices().containsKey(field.getName()) && field.getIndices().get(field.getName()).getType().equals(Index.Type.VESPA), + field.getAttributes().containsKey(field.getName())); searchDef.add(searchField); // Add field to views @@ -302,89 +303,6 @@ public class DocumentModelBuilder { return null; } - @SuppressWarnings("deprecation") - private static void specialHandleAnnotationReference(NewDocumentType docType, Field field) { - DataType fieldType = specialHandleAnnotationReferenceRecurse(docType, field.getName(), field.getDataType()); - if (fieldType == null) { - return; - } - field.setDataType(fieldType); // XXX deprecated - } - - private static DataType specialHandleAnnotationReferenceRecurse(NewDocumentType docType, String fieldName, - DataType dataType) { - if (dataType instanceof TemporaryAnnotationReferenceDataType) { - TemporaryAnnotationReferenceDataType refType = (TemporaryAnnotationReferenceDataType)dataType; - if (refType.getId() != 0) { - return null; - } - AnnotationType target = docType.getAnnotationType(refType.getTarget()); - if (target == null) { - throw new RetryLaterException("Annotation '" + refType.getTarget() + "' in reference '" + fieldName + - "' does not exist."); - } - dataType = new AnnotationReferenceDataType(target); - addType(docType, dataType); - return dataType; - } - else if (dataType instanceof MapDataType) { - MapDataType t = (MapDataType)dataType; - DataType valueType = specialHandleAnnotationReferenceRecurse(docType, fieldName, t.getValueType()); - if (valueType == null) { - return null; - } - var mapType = new MapDataType(t.getKeyType(), valueType, t.getId()); - addType(docType, mapType); - return mapType; - } - else if (dataType instanceof ArrayDataType) { - ArrayDataType t = (ArrayDataType) dataType; - DataType nestedType = specialHandleAnnotationReferenceRecurse(docType, fieldName, t.getNestedType()); - if (nestedType == null) { - return null; - } - var lstType = new ArrayDataType(nestedType, t.getId()); - addType(docType, lstType); - return lstType; - } - else if (dataType instanceof WeightedSetDataType) { - WeightedSetDataType t = (WeightedSetDataType) dataType; - DataType nestedType = specialHandleAnnotationReferenceRecurse(docType, fieldName, t.getNestedType()); - if (nestedType == null) { - return null; - } - boolean c = t.createIfNonExistent(); - boolean r = t.removeIfZero(); - var lstType = new WeightedSetDataType(nestedType, c, r, t.getId()); - addType(docType, lstType); - return lstType; - } - return null; - } - - private static StructDataType handleStruct(NewDocumentType dt, SDDocumentType type) { - StructDataType s = new StructDataType(type.getName()); - for (Field f : type.getDocumentType().contentStruct().getFieldsThisTypeOnly()) { - specialHandleAnnotationReference(dt, f); - s.addField(f); - } - for (StructDataType inherited : type.getDocumentType().contentStruct().getInheritedTypes()) { - s.inherit(inherited); - } - extractNestedTypes(dt, s); - addType(dt, s); - return s; - } - - private static StructDataType handleStruct(NewDocumentType dt, StructDataType s) { - for (Field f : s.getFieldsThisTypeOnly()) { - specialHandleAnnotationReference(dt, f); - } - extractNestedTypes(dt, s); - addType(dt, s); - return s; - } - private static boolean anyParentsHavePayLoad(SDAnnotationType sa, SDDocumentType sdoc) { if (sa.getInherits() != null) { AnnotationType tmp = sdoc.findAnnotation(sa.getInherits()); @@ -395,8 +313,6 @@ public class DocumentModelBuilder { } private NewDocumentType convert(SDDocumentType sdoc) { - Map<AnnotationType, String> annotationInheritance = new HashMap<>(); - Map<StructDataType, String> structInheritance = new HashMap<>(); NewDocumentType dt = new NewDocumentType(new NewDocumentType.Name(sdoc.getName()), sdoc.getDocumentType().contentStruct(), sdoc.getFieldSets(), @@ -404,63 +320,223 @@ public class DocumentModelBuilder { convertTemporaryImportedFieldsToNames(sdoc.getTemporaryImportedFields())); for (SDDocumentType n : sdoc.getInheritedTypes()) { NewDocumentType.Name name = new NewDocumentType.Name(n.getName()); - NewDocumentType inherited = model.getDocumentManager().getDocumentType(name); - if (inherited != null) { - dt.inherit(inherited); - } - } - for (SDDocumentType type : sdoc.getTypes()) { - if (type.isStruct()) { - handleStruct(dt, type); - } else { - throw new IllegalArgumentException("Data type '" + sdoc.getName() + "' is not a struct => tostring='" + sdoc.toString() + "'."); - } - } - for (SDDocumentType type : sdoc.getTypes()) { - for (SDDocumentType proxy : type.getInheritedTypes()) { - var inherited = dt.getDataTypeRecursive(proxy.getName()); - var converted = (StructDataType) dt.getDataType(type.getName()); - converted.inherit((StructDataType) inherited); + NewDocumentType inherited = model.getDocumentManager().getDocumentType(name); + if (inherited != null) { + dt.inherit(inherited); } } - for (AnnotationType annotation : sdoc.getAnnotations().values()) { - dt.add(annotation); + var extractor = new TypeExtractor(dt); + extractor.extract(sdoc); + return dt; + } + + static class TypeExtractor { + private final NewDocumentType targetDt; + Map<AnnotationType, String> annotationInheritance = new HashMap<>(); + Map<StructDataType, String> structInheritance = new HashMap<>(); + private final Map<Object, Object> inProgress = new IdentityHashMap<>(); + TypeExtractor(NewDocumentType target) { + this.targetDt = target; } - for (AnnotationType annotation : sdoc.getAnnotations().values()) { - SDAnnotationType sa = (SDAnnotationType) annotation; - if (annotation.getInheritedTypes().isEmpty() && (sa.getInherits() != null) ) { - annotationInheritance.put(annotation, sa.getInherits()); + + void extract(SDDocumentType sdoc) { + for (SDDocumentType type : sdoc.getTypes()) { + if (type.isStruct()) { + handleStruct(type); + } else { + throw new IllegalArgumentException("Data type '" + type.getName() + "' is not a struct => tostring='" + type.toString() + "'."); + } + } + for (SDDocumentType type : sdoc.getTypes()) { + for (SDDocumentType proxy : type.getInheritedTypes()) { + var inherited = targetDt.getDataTypeRecursive(proxy.getName()); + var converted = (StructDataType) targetDt.getDataType(type.getName()); + converted.inherit((StructDataType) inherited); + } } - if (annotation.getDataType() == null) { - if (sa.getSdDocType() != null) { - StructDataType s = handleStruct(dt, sa.getSdDocType()); - annotation.setDataType(s); - if ((sa.getInherits() != null)) { + for (AnnotationType annotation : sdoc.getAnnotations().values()) { + targetDt.add(annotation); + } + for (AnnotationType annotation : sdoc.getAnnotations().values()) { + SDAnnotationType sa = (SDAnnotationType) annotation; + if (annotation.getInheritedTypes().isEmpty() && (sa.getInherits() != null) ) { + annotationInheritance.put(annotation, sa.getInherits()); + } + if (annotation.getDataType() == null) { + if (sa.getSdDocType() != null) { + StructDataType s = handleStruct(sa.getSdDocType()); + annotation.setDataType(s); + if ((sa.getInherits() != null)) { + structInheritance.put(s, "annotation."+sa.getInherits()); + } + } else if (sa.getInherits() != null) { + StructDataType s = new StructDataType("annotation."+annotation.getName()); + if (anyParentsHavePayLoad(sa, sdoc)) { + annotation.setDataType(s); + addType(s); + } structInheritance.put(s, "annotation."+sa.getInherits()); } - } else if (sa.getInherits() != null) { - StructDataType s = new StructDataType("annotation."+annotation.getName()); - if (anyParentsHavePayLoad(sa, sdoc)) { - annotation.setDataType(s); - addType(dt, s); + } + } + for (Map.Entry<AnnotationType, String> e : annotationInheritance.entrySet()) { + e.getKey().inherit(targetDt.getAnnotationType(e.getValue())); + } + for (Map.Entry<StructDataType, String> e : structInheritance.entrySet()) { + StructDataType s = (StructDataType)targetDt.getDataType(e.getValue()); + if (s != null) { + e.getKey().inherit(s); + } + } + handleStruct(sdoc.getDocumentType().contentStruct()); + + extractDataTypesFromFields(sdoc.fieldSet()); + } + + private void extractDataTypesFromFields(Collection<Field> fields) { + for (Field f : fields) { + DataType type = f.getDataType(); + if (testAddType(type)) { + extractNestedTypes(type); + addType(type); + } + } + } + + private void extractNestedTypes(DataType type) { + if (inProgress.containsKey(type)) { + return; + } + inProgress.put(type, this); + if (type instanceof StructDataType) { + StructDataType tmp = (StructDataType) type; + extractDataTypesFromFields(tmp.getFieldsThisTypeOnly()); + } else if (type instanceof DocumentType) { + throw new IllegalArgumentException("Can not handle nested document definitions. In document type '" + targetDt.getName().toString() + + "', we can not define document type '" + type.toString()); + } else if (type instanceof CollectionDataType) { + CollectionDataType tmp = (CollectionDataType) type; + extractNestedTypes(tmp.getNestedType()); + addType(tmp.getNestedType()); + } else if (type instanceof MapDataType) { + MapDataType tmp = (MapDataType) type; + extractNestedTypes(tmp.getKeyType()); + extractNestedTypes(tmp.getValueType()); + addType(tmp.getKeyType()); + addType(tmp.getValueType()); + } else if (type instanceof TemporaryAnnotationReferenceDataType) { + throw new IllegalArgumentException(type.toString()); + } + } + + private boolean testAddType(DataType type) { return internalAddType(type, true); } + + private boolean addType(DataType type) { return internalAddType(type, false); } + + private boolean internalAddType(DataType type, boolean dryRun) { + DataType oldType = targetDt.getDataTypeRecursive(type.getId()); + if (oldType == null) { + if ( ! dryRun) { + targetDt.add(type); + } + return true; + } else if ((type instanceof StructDataType) && (oldType instanceof StructDataType)) { + StructDataType s = (StructDataType) type; + StructDataType os = (StructDataType) oldType; + if ((os.getFieldCount() == 0) && (s.getFieldCount() > os.getFieldCount())) { + if ( ! dryRun) { + targetDt.replace(type); } - structInheritance.put(s, "annotation."+sa.getInherits()); + return true; } } + return false; } - for (Map.Entry<AnnotationType, String> e : annotationInheritance.entrySet()) { - e.getKey().inherit(dt.getAnnotationType(e.getValue())); + + + @SuppressWarnings("deprecation") + private void specialHandleAnnotationReference(Field field) { + DataType fieldType = specialHandleAnnotationReferenceRecurse(field.getName(), field.getDataType()); + if (fieldType == null) { + return; + } + field.setDataType(fieldType); // XXX deprecated + } + + private DataType specialHandleAnnotationReferenceRecurse(String fieldName, + DataType dataType) { + if (dataType instanceof TemporaryAnnotationReferenceDataType) { + TemporaryAnnotationReferenceDataType refType = (TemporaryAnnotationReferenceDataType)dataType; + if (refType.getId() != 0) { + return null; + } + AnnotationType target = targetDt.getAnnotationType(refType.getTarget()); + if (target == null) { + throw new RetryLaterException("Annotation '" + refType.getTarget() + "' in reference '" + fieldName + + "' does not exist."); + } + dataType = new AnnotationReferenceDataType(target); + addType(dataType); + return dataType; + } + else if (dataType instanceof MapDataType) { + MapDataType t = (MapDataType)dataType; + DataType valueType = specialHandleAnnotationReferenceRecurse(fieldName, t.getValueType()); + if (valueType == null) { + return null; + } + var mapType = new MapDataType(t.getKeyType(), valueType, t.getId()); + addType(mapType); + return mapType; + } + else if (dataType instanceof ArrayDataType) { + ArrayDataType t = (ArrayDataType) dataType; + DataType nestedType = specialHandleAnnotationReferenceRecurse(fieldName, t.getNestedType()); + if (nestedType == null) { + return null; + } + var lstType = new ArrayDataType(nestedType, t.getId()); + addType(lstType); + return lstType; + } + else if (dataType instanceof WeightedSetDataType) { + WeightedSetDataType t = (WeightedSetDataType) dataType; + DataType nestedType = specialHandleAnnotationReferenceRecurse(fieldName, t.getNestedType()); + if (nestedType == null) { + return null; + } + boolean c = t.createIfNonExistent(); + boolean r = t.removeIfZero(); + var lstType = new WeightedSetDataType(nestedType, c, r, t.getId()); + addType(lstType); + return lstType; + } + return null; } - for (Map.Entry<StructDataType, String> e : structInheritance.entrySet()) { - StructDataType s = (StructDataType)dt.getDataType(e.getValue()); - if (s != null) { - e.getKey().inherit(s); + + private StructDataType handleStruct(SDDocumentType type) { + StructDataType s = new StructDataType(type.getName()); + for (Field f : type.getDocumentType().contentStruct().getFieldsThisTypeOnly()) { + specialHandleAnnotationReference(f); + s.addField(f); + } + for (StructDataType inherited : type.getDocumentType().contentStruct().getInheritedTypes()) { + s.inherit(inherited); } + extractNestedTypes(s); + addType(s); + return s; } - handleStruct(dt, sdoc.getDocumentType().contentStruct()); - extractDataTypesFromFields(dt, sdoc.fieldSet()); - return dt; + private StructDataType handleStruct(StructDataType s) { + for (Field f : s.getFieldsThisTypeOnly()) { + specialHandleAnnotationReference(f); + } + extractNestedTypes(s); + addType(s); + return s; + } + } private static Set<NewDocumentType.Name> convertDocumentReferencesToNames(Optional<DocumentReferences> documentReferences) { @@ -468,9 +544,9 @@ public class DocumentModelBuilder { return Set.of(); } return documentReferences.get().referenceMap().values().stream() - .map(documentReference -> documentReference.targetSearch().getDocument()) - .map(documentType -> new NewDocumentType.Name(documentType.getName())) - .collect(Collectors.toCollection(() -> new LinkedHashSet<>())); + .map(documentReference -> documentReference.targetSearch().getDocument()) + .map(documentType -> new NewDocumentType.Name(documentType.getName())) + .collect(Collectors.toCollection(() -> new LinkedHashSet<>())); } private static Set<String> convertTemporaryImportedFieldsToNames(TemporaryImportedFields importedFields) { @@ -480,62 +556,6 @@ public class DocumentModelBuilder { return Collections.unmodifiableSet(importedFields.fields().keySet()); } - private static void extractDataTypesFromFields(NewDocumentType dt, Collection<Field> fields) { - for (Field f : fields) { - DataType type = f.getDataType(); - if (testAddType(dt, type)) { - extractNestedTypes(dt, type); - addType(dt, type); - } - } - } - - private static void extractNestedTypes(NewDocumentType dt, DataType type) { - if (type instanceof StructDataType) { - StructDataType tmp = (StructDataType) type; - extractDataTypesFromFields(dt, tmp.getFieldsThisTypeOnly()); - } else if (type instanceof DocumentType) { - throw new IllegalArgumentException("Can not handle nested document definitions. In document type '" + dt.getName().toString() + - "', we can not define document type '" + type.toString()); - } else if (type instanceof CollectionDataType) { - CollectionDataType tmp = (CollectionDataType) type; - extractNestedTypes(dt, tmp.getNestedType()); - addType(dt, tmp.getNestedType()); - } else if (type instanceof MapDataType) { - MapDataType tmp = (MapDataType) type; - extractNestedTypes(dt, tmp.getKeyType()); - extractNestedTypes(dt, tmp.getValueType()); - addType(dt, tmp.getKeyType()); - addType(dt, tmp.getValueType()); - } else if (type instanceof TemporaryAnnotationReferenceDataType) { - throw new IllegalArgumentException(type.toString()); - } - } - - private static boolean testAddType(NewDocumentType dt, DataType type) { return internalAddType(dt, type, true); } - - private static boolean addType(NewDocumentType dt, DataType type) { return internalAddType(dt, type, false); } - - private static boolean internalAddType(NewDocumentType dt, DataType type, boolean dryRun) { - DataType oldType = dt.getDataTypeRecursive(type.getId()); - if (oldType == null) { - if ( ! dryRun) { - dt.add(type); - } - return true; - } else if ((type instanceof StructDataType) && (oldType instanceof StructDataType)) { - StructDataType s = (StructDataType) type; - StructDataType os = (StructDataType) oldType; - if ((os.getFieldCount() == 0) && (s.getFieldCount() > os.getFieldCount())) { - if ( ! dryRun) { - dt.replace(type); - } - return true; - } - } - return false; - } - public static class RetryLaterException extends IllegalArgumentException { public RetryLaterException(String message) { super(message); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/document/SDField.java b/config-model/src/main/java/com/yahoo/searchdefinition/document/SDField.java index d9bdf5dc917..0d1222e737b 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/document/SDField.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/document/SDField.java @@ -9,6 +9,7 @@ import com.yahoo.document.MapDataType; import com.yahoo.document.StructDataType; import com.yahoo.document.TemporaryStructuredDataType; import com.yahoo.document.TensorDataType; +import com.yahoo.document.WeightedSetDataType; import com.yahoo.language.Linguistics; import com.yahoo.language.process.Embedder; import com.yahoo.language.simple.SimpleLinguistics; @@ -186,6 +187,10 @@ public class SDField extends Field implements TypedKey, FieldOperationContainer, ": Dense tensor dimensions must have a size"); addQueryCommand("type " + type); } + else if (dataType instanceof WeightedSetDataType) { + var nested = ((WeightedSetDataType) dataType).getNestedType().getName(); + addQueryCommand("type WeightedSet<" + nested + ">"); + } else { addQueryCommand("type " + dataType.getName()); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertParsedFields.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertParsedFields.java index e7423b17830..3cf5628d282 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertParsedFields.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertParsedFields.java @@ -138,6 +138,7 @@ public class ConvertParsedFields { if (indexing.isPresent()) { field.setIndexingScript(indexing.get().script()); } + parsed.getWeight().ifPresent(value -> field.setWeight(value)); parsed.getStemming().ifPresent(value -> field.setStemming(value)); parsed.getNormalizing().ifPresent(value -> convertNormalizing(field, value)); for (var attribute : parsed.getAttributes()) { diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertSchemaCollection.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertSchemaCollection.java index 332a2153516..21a68744c19 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertSchemaCollection.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertSchemaCollection.java @@ -239,7 +239,7 @@ public class ConvertSchemaCollection { if (parsed.hasStemming()) { schema.setStemming(parsed.getStemming()); } - schema.enableRawAsBase64(parsed.getRawAsBase64()); + parsed.getRawAsBase64().ifPresent(value -> schema.enableRawAsBase64(value)); var typeContext = typeConverter.makeContext(parsed.getDocument()); var fieldConverter = new ConvertParsedFields(typeContext); convertDocument(schema, parsed.getDocument(), fieldConverter); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedField.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedField.java index 5bcfa841bae..ca876997dc6 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedField.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedField.java @@ -58,6 +58,7 @@ class ParsedField extends ParsedBlock { List<String> getQueryCommands() { return List.copyOf(queryCommands); } String lookupAliasedFrom(String alias) { return aliases.get(alias); } ParsedMatchSettings matchSettings() { return this.matchInfo; } + Optional<Integer> getWeight() { return Optional.ofNullable(weight); } Optional<Stemming> getStemming() { return Optional.ofNullable(stemming); } Optional<String> getNormalizing() { return Optional.ofNullable(normalizing); } Optional<ParsedIndexingOp> getIndexing() { return Optional.ofNullable(indexingOp); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedSchema.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedSchema.java index 0004094e1c2..599dd6e2a7a 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedSchema.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedSchema.java @@ -9,6 +9,7 @@ import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Optional; /** * This class holds the extracted information after parsing @@ -30,7 +31,7 @@ public class ParsedSchema extends ParsedBlock { } private boolean documentWithoutSchema = false; - private boolean rawAsBase64 = false; // TODO Vespa 8 flip default + private Boolean rawAsBase64 = null; private ParsedDocument myDocument = null; private Stemming defaultStemming = null; private final List<ImportedField> importedFields = new ArrayList<>(); @@ -53,7 +54,7 @@ public class ParsedSchema extends ParsedBlock { } boolean getDocumentWithoutSchema() { return documentWithoutSchema; } - boolean getRawAsBase64() { return rawAsBase64; } + Optional<Boolean> getRawAsBase64() { return Optional.ofNullable(rawAsBase64); } boolean hasDocument() { return myDocument != null; } ParsedDocument getDocument() { return myDocument; } boolean hasStemming() { return defaultStemming != null; } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedType.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedType.java index 3aed90a58e1..d04277706a1 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedType.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedType.java @@ -46,6 +46,7 @@ class ParsedType { case "raw": return Variant.BUILTIN; case "tag": return Variant.BUILTIN; case "position": return Variant.POSITION; + case "float16": return Variant.BUILTIN; } return Variant.UNKNOWN; } diff --git a/config-model/src/test/derived/types/index-info.cfg b/config-model/src/test/derived/types/index-info.cfg index 2db4ead180b..7f43cd67a6b 100644 --- a/config-model/src/test/derived/types/index-info.cfg +++ b/config-model/src/test/derived/types/index-info.cfg @@ -96,7 +96,7 @@ indexinfo[].command[].command "multivalue" indexinfo[].command[].indexname "tagfield" indexinfo[].command[].command "attribute" indexinfo[].command[].indexname "tagfield" -indexinfo[].command[].command "type tag" +indexinfo[].command[].command "type WeightedSet<string>" indexinfo[].command[].indexname "structfield.s1" indexinfo[].command[].command "index" indexinfo[].command[].indexname "structfield.s1" @@ -704,4 +704,4 @@ indexinfo[].command[].command "index" indexinfo[].command[].indexname "pst_sta_boldingoff_nomatch_tag_01" indexinfo[].command[].command "multivalue" indexinfo[].command[].indexname "pst_sta_boldingoff_nomatch_tag_01" -indexinfo[].command[].command "type tag" +indexinfo[].command[].command "type WeightedSet<string>" diff --git a/config-proxy/src/main/sh/vespa-config-ctl.sh b/config-proxy/src/main/sh/vespa-config-ctl.sh index a7f6a2a97a7..63122c0e0ec 100755 --- a/config-proxy/src/main/sh/vespa-config-ctl.sh +++ b/config-proxy/src/main/sh/vespa-config-ctl.sh @@ -109,7 +109,7 @@ case $1 in nohup nice sbin/vespa-retention-enforcer > ${LOGDIR}/vre-start.log 2>&1 </dev/null & configsources=`bin/vespa-print-default configservers_rpc` userargs=$VESPA_CONFIGPROXY_JVMARGS - jvmopts="-Xms32M -Xmx128M -XX:CompressedClassSpaceSize=32m -XX:MaxDirectMemorySize=32m -XX:ThreadStackSize=256 -XX:MaxJavaStackTraceDepth=1000 -XX:-OmitStackTraceInFastThrow" + jvmopts="-Xms32M -Xmx128M -XX:CompressedClassSpaceSize=32m -XX:MaxDirectMemorySize=32m -XX:ThreadStackSize=448 -XX:MaxJavaStackTraceDepth=1000 -XX:-OmitStackTraceInFastThrow" VESPA_SERVICE_NAME=configproxy export VESPA_SERVICE_NAME diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index 9cc519ed9e2..0e45aa59421 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -4121,6 +4121,19 @@ ], "fields": [] }, + "com.yahoo.search.grouping.result.FlatteningSearcher": { + "superClass": "com.yahoo.search.Searcher", + "interfaces": [], + "attributes": [ + "public" + ], + "methods": [ + "public void <init>()", + "public com.yahoo.search.Result search(com.yahoo.search.Query, com.yahoo.search.searchchain.Execution)", + "public void flatten(com.yahoo.search.result.HitGroup, com.yahoo.search.Result)" + ], + "fields": [] + }, "com.yahoo.search.grouping.result.Group": { "superClass": "com.yahoo.search.result.HitGroup", "interfaces": [], diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/MultipleResultsSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/MultipleResultsSearcher.java index d017cce0d44..3c61a361cbb 100644 --- a/container-search/src/main/java/com/yahoo/prelude/searcher/MultipleResultsSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/searcher/MultipleResultsSearcher.java @@ -13,9 +13,8 @@ import com.yahoo.search.searchchain.Execution; import java.util.*; /** - * <p> Groups hits according to sddocname. </p> - * - * <p> For each group, the desired number of hits can be specified. </p> + * Groups hits according to document type. + * For each group, the desired number of hits can be specified. * * @author Tony Vaagenes */ @@ -48,7 +47,7 @@ public class MultipleResultsSearcher extends Searcher { } } - private class HitsRetriever { + private static class HitsRetriever { PartitionedResult partitionedResult; @@ -58,12 +57,12 @@ public class MultipleResultsSearcher extends Searcher { private final Parameters parameters; private final int hits; private final int offset; - private Execution execution; - private Result initialResult; + private final Execution execution; + private final Result initialResult; HitsRetriever(Query query, Execution execution, Parameters parameters) throws ParameterException { - this.offset=query.getOffset(); - this.hits=query.getHits(); + this.offset = query.getOffset(); + this.hits = query.getHits(); this.nextOffset = query.getOffset() + query.getHits(); this.query = query; this.parameters = parameters; @@ -362,13 +361,14 @@ public class MultipleResultsSearcher extends Searcher { } } - @SuppressWarnings("serial") private static class ParameterException extends Exception { + String msg; ParameterException(String msg) { this.msg = msg; } + } } diff --git a/container-search/src/main/java/com/yahoo/search/Query.java b/container-search/src/main/java/com/yahoo/search/Query.java index fb7281e1f24..83fa18d847f 100644 --- a/container-search/src/main/java/com/yahoo/search/Query.java +++ b/container-search/src/main/java/com/yahoo/search/Query.java @@ -726,8 +726,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { * if the trace level of the query is sufficiently high. * * @param message the message to add - * @param includeQuery true to append the query root stringValue - * at the end of the message + * @param includeQuery true to append the query root stringValue at the end of the message * @param traceLevel the context level of the message, this method will do nothing * if the traceLevel of the query is lower than this value */ diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java b/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java index d7c9f1dce53..5e38f6b4bdd 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java @@ -116,8 +116,6 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM InvokerResult result = new InvokerResult(query, query.getHits()); List<LeanHit> merged = Collections.emptyList(); long nextTimeout = query.getTimeLeft(); - boolean extraDebug = (query.getOffset() == 0) && (query.getHits() == 7) && log.isLoggable(java.util.logging.Level.FINE); - List<InvokerResult> processed = new ArrayList<>(); var groupingResultAggregator = new GroupingResultAggregator(); try { while (!invokers.isEmpty() && nextTimeout >= 0) { @@ -127,9 +125,6 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM break; } else { InvokerResult toMerge = invoker.getSearchResult(execution); - if (extraDebug) { - processed.add(toMerge); - } merged = mergeResult(result.getResult(), toMerge, merged, groupingResultAggregator); ejectInvoker(invoker); } @@ -143,32 +138,6 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM insertNetworkErrors(result.getResult()); result.getResult().setCoverage(createCoverage()); - if (extraDebug && merged.size() > 0) { - int firstPartId = merged.get(0).getPartId(); - for (int index = 1; index < merged.size(); index++) { - if (merged.get(index).getPartId() != firstPartId) { - extraDebug = false; - log.fine("merged["+index+"/"+merged.size()+"] from partId "+merged.get(index).getPartId()+", first "+firstPartId); - break; - } - } - } - if (extraDebug) { - log.fine("Interleaved "+processed.size()+" results"); - for (int pIdx = 0; pIdx < processed.size(); ++pIdx) { - var p = processed.get(pIdx); - log.fine("InvokerResult "+pIdx+" total hits "+p.getResult().getTotalHitCount()); - var lean = p.getLeanHits(); - for (int idx = 0; idx < lean.size(); ++idx) { - var hit = lean.get(idx); - log.fine("lean hit "+idx+" relevance "+hit.getRelevance()+" partid "+hit.getPartId()); - } - } - for (int mIdx = 0; mIdx < merged.size(); ++mIdx) { - var hit = merged.get(mIdx); - log.fine("merged hit "+mIdx+" relevance "+hit.getRelevance()+" partid "+hit.getPartId()); - } - } int needed = query.getOffset() + query.getHits(); for (int index = query.getOffset(); (index < merged.size()) && (index < needed); index++) { result.getLeanHits().add(merged.get(index)); diff --git a/container-search/src/main/java/com/yahoo/search/grouping/result/FlatteningSearcher.java b/container-search/src/main/java/com/yahoo/search/grouping/result/FlatteningSearcher.java new file mode 100644 index 00000000000..321f86facd0 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/grouping/result/FlatteningSearcher.java @@ -0,0 +1,50 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.grouping.result; + +import com.yahoo.component.chain.dependencies.Before; +import com.yahoo.search.Query; +import com.yahoo.search.Result; +import com.yahoo.search.Searcher; +import com.yahoo.search.grouping.vespa.GroupingExecutor; +import com.yahoo.search.result.Hit; +import com.yahoo.search.result.HitGroup; +import com.yahoo.search.searchchain.Execution; + +import java.util.Iterator; + +/** + * Flattens a grouping result into a flat list of hits on the top level in the returned result. + * Useful when using grouping to create hits with diversity and similar. + * + * @author bratseth + */ +@Before(GroupingExecutor.COMPONENT_NAME) +public class FlatteningSearcher extends Searcher { + + @Override + public Result search(Query query, Execution execution) { + if ( ! query.properties().getBoolean("flatten", true)) return execution.search(query); + + query.trace("Flattening groups", 2); + int originalHits = query.getHits(); + query.setHits(0); + Result result = execution.search(query); + query.setHits(originalHits); + flatten(result.hits(), result); + return result; + } + + public void flatten(HitGroup hits, Result result) { + int hitsLeft = hits.size(); // Iterate only through the initial size + for (Iterator<Hit> i = hits.iterator(); i.hasNext() && hitsLeft-- > 0;) { + Hit hit = i.next(); + if (hit instanceof HitGroup) { + flatten((HitGroup)hit, result); + i.remove(); + } else { + result.hits().add(hit); + } + } + } + +} diff --git a/container-search/src/main/java/com/yahoo/search/result/HitGroup.java b/container-search/src/main/java/com/yahoo/search/result/HitGroup.java index 6d09bf66175..efe25e04f2e 100644 --- a/container-search/src/main/java/com/yahoo/search/result/HitGroup.java +++ b/container-search/src/main/java/com/yahoo/search/result/HitGroup.java @@ -249,9 +249,7 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable< return hit; } - /** - * Adds a list of hits to this group, the same - */ + /** Adds a list of hits to this group, the same as calling add for each item in the list. */ public void addAll(List<Hit> hits) { for (Hit hit : hits) add(hit); diff --git a/container-search/src/test/java/com/yahoo/search/grouping/result/FlatteningSearcherTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/result/FlatteningSearcherTestCase.java new file mode 100644 index 00000000000..9e5bfb4a9f5 --- /dev/null +++ b/container-search/src/test/java/com/yahoo/search/grouping/result/FlatteningSearcherTestCase.java @@ -0,0 +1,126 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.grouping.result; + +import com.yahoo.component.ComponentId; +import com.yahoo.component.chain.dependencies.After; +import com.yahoo.document.GlobalId; +import com.yahoo.prelude.fastsearch.FastHit; +import com.yahoo.prelude.fastsearch.GroupingListHit; +import com.yahoo.search.Query; +import com.yahoo.search.Result; +import com.yahoo.search.Searcher; +import com.yahoo.search.grouping.GroupingRequest; +import com.yahoo.search.grouping.request.GroupingOperation; +import com.yahoo.search.grouping.vespa.GroupingExecutor; +import com.yahoo.search.result.Hit; +import com.yahoo.search.result.HitGroup; +import com.yahoo.search.searchchain.Execution; +import com.yahoo.search.searchchain.SearchChain; +import com.yahoo.searchlib.aggregation.FS4Hit; +import com.yahoo.searchlib.aggregation.Group; +import com.yahoo.searchlib.aggregation.Grouping; +import com.yahoo.searchlib.aggregation.HitsAggregationResult; +import com.yahoo.searchlib.expression.StringResultNode; +import org.junit.Test; + +import java.util.Arrays; +import java.util.LinkedList; +import java.util.List; +import java.util.Queue; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * @author bratseth + */ +public class FlatteningSearcherTestCase { + + @Test + public void testFlatteningSearcher() { + Query query = new Query("?query=test"); + GroupingRequest req = GroupingRequest.newInstance(query); + req.setRootOperation(GroupingOperation.fromString("all(group(foo) each(each(output(summary(bar)))))")); + + Grouping group0 = new Grouping(0); + group0.setRoot(new com.yahoo.searchlib.aggregation.Group() + .addChild(new Group().setId(new StringResultNode("unique1")) + .addAggregationResult(new HitsAggregationResult(3, "bar") + ) + ) + .addChild(new Group().setId(new StringResultNode("unique2")) + .addAggregationResult(new HitsAggregationResult(3, "bar") + ) + )); + Grouping group1 = new Grouping(0); + group1.setRoot(new com.yahoo.searchlib.aggregation.Group() + .addChild(new Group().setId(new StringResultNode("unique1")) + .addAggregationResult(new HitsAggregationResult(3, "bar") + .addHit(fs4Hit(0.7)) + .addHit(fs4Hit(0.6)) + .addHit(fs4Hit(0.3)) + ) + ) + .addChild(new Group().setId(new StringResultNode("unique2")) + .addAggregationResult(new HitsAggregationResult(3, "bar") + .addHit(fs4Hit(0.5)) + .addHit(fs4Hit(0.4)) + ) + )); + Execution execution = newExecution(new FlatteningSearcher(), + new GroupingExecutor(ComponentId.fromString("grouping")), + new ResultProvider(Arrays.asList( + new GroupingListHit(List.of(group0), null), + new GroupingListHit(List.of(group1), null)))); + Result result = execution.search(query); + assertEquals(5, result.hits().size()); + assertFlat(result); + } + + private void assertFlat(Result result) { + for (var hit : result.hits()) + assertTrue(hit instanceof FastHit); + } + + private FS4Hit fs4Hit(double relevance) { + return new FS4Hit(0, new GlobalId(new byte[GlobalId.LENGTH]), relevance); + } + + private void dump(Hit hit, String indent) { + System.out.println(indent + hit); + if (hit instanceof HitGroup) { + for (var child : (HitGroup)hit) + dump(child, indent + " "); + } + } + + private static Execution newExecution(Searcher... searchers) { + return new Execution(new SearchChain(new ComponentId("foo"), Arrays.asList(searchers)), + Execution.Context.createContextStub()); + } + + @After (GroupingExecutor.COMPONENT_NAME) + private static class ResultProvider extends Searcher { + + final Queue<GroupingListHit> hits = new LinkedList<>(); + int pass = 0; + + ResultProvider(List<GroupingListHit> hits) { + this.hits.addAll(hits); + } + + @Override + public Result search(Query query, Execution exec) { + GroupingListHit hit = hits.poll(); + for (Grouping group : hit.getGroupingList()) { + group.setFirstLevel(pass); + group.setLastLevel(pass); + } + ++pass; + Result result = exec.search(query); + result.hits().add(hit); + return result; + } + } + +} diff --git a/container-search/src/test/java/com/yahoo/search/grouping/result/HitRendererTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/result/HitRendererTestCase.java index 7f70178bcf7..1035c9d9284 100644 --- a/container-search/src/test/java/com/yahoo/search/grouping/result/HitRendererTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/grouping/result/HitRendererTestCase.java @@ -151,7 +151,6 @@ public class HitRendererTestCase { return new Group(id, new Relevance(1)); } - @SuppressWarnings("deprecation") private static void assertRender(HitGroup hit, String expectedXml) { StringWriter str = new StringWriter(); XMLWriter out = new XMLWriter(str, 0, -1); diff --git a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java index 637666a6a19..1d2e3c240b9 100644 --- a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java @@ -948,7 +948,7 @@ public class JsonRendererTestCase { } }); Group g = new Group(new StringId("Jones"), new Relevance(1.0)); - g.setField("count()", Integer.valueOf(7)); + g.setField("count()", 7); gl.add(g); rg.add(gl); r.hits().add(rg); @@ -1443,8 +1443,7 @@ public class JsonRendererTestCase { ByteArrayOutputStream bs = new ByteArrayOutputStream(); ListenableFuture<Boolean> f = renderer.render(bs, r, execution, null); assertTrue(f.get()); - String summary = Utf8.toString(bs.toByteArray()); - return summary; + return Utf8.toString(bs.toByteArray()); } @SuppressWarnings("unchecked") diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java index 4ab7901cd62..30f416747e0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java @@ -215,6 +215,7 @@ public class Application { public Optional<ApplicationVersion> oldestDeployedApplication() { return productionDeployments().values().stream().flatMap(List::stream) .map(Deployment::applicationVersion) + .filter(version -> ! version.isUnknown() && ! version.isDeployedDirectly()) .min(Comparator.naturalOrder()); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 88df49ad3ab..5c3c43461dc 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -446,21 +446,17 @@ public class ApplicationController { controller.notificationsDb().removeNotifications(notification.source()); } - var oldestDeployedVersion = application.get().productionDeployments().values().stream() - .flatMap(List::stream) - .map(Deployment::applicationVersion) - .filter(version -> ! version.isDeployedDirectly()) - .min(naturalOrder()) - .orElse(ApplicationVersion.unknown); - - var olderVersions = application.get().versions().stream() - .filter(version -> version.compareTo(oldestDeployedVersion) < 0) - .sorted() - .collect(Collectors.toList()); + ApplicationVersion oldestDeployedVersion = application.get().oldestDeployedApplication() + .orElse(ApplicationVersion.unknown); + + List<ApplicationVersion> olderVersions = application.get().versions().stream() + .filter(version -> version.compareTo(oldestDeployedVersion) < 0) + .sorted() + .collect(Collectors.toList()); // Remove any version not deployed anywhere - but keep one - for (int i = 0; i < olderVersions.size() - 1; i++) { - application = application.withoutVersion(olderVersions.get(i)); + for (ApplicationVersion version : olderVersions) { + application = application.withoutVersion(version); } store(application); 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 8c933f98277..09fd9ca86bc 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 @@ -43,7 +43,6 @@ import static com.yahoo.config.provision.Environment.staging; import static com.yahoo.config.provision.Environment.test; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.stagingTest; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.systemTest; -import static java.util.Collections.reverseOrder; import static java.util.Comparator.comparing; import static java.util.Comparator.naturalOrder; import static java.util.Objects.requireNonNull; @@ -201,7 +200,7 @@ public class DeploymentStatus { jobs.putAll(productionJobs); // Add runs for idle, declared test jobs if they have no successes on their instance's change's versions. jobSteps.forEach((job, step) -> { - if ( ! step.isDeclared() || step.type() != StepType.test || jobs.containsKey(job)) + if ( ! step.isDeclared() || job.type().isProduction() || jobs.containsKey(job)) return; Change change = changes.get(job.application().instance()); @@ -212,9 +211,8 @@ public class DeploymentStatus { .filter(jobId -> jobId.type().isProduction() && jobId.type().isDeployment()) .filter(jobId -> deploymentFor(jobId).isPresent()) .findFirst(); - Versions versions = Versions.from(change, application, firstProductionJobWithDeployment.flatMap(this::deploymentFor), systemVersion); - if (step.completedAt(change, firstProductionJobWithDeployment).isEmpty()) + if (step.completedAt(change, Optional.empty()).isEmpty()) jobs.merge(job, List.of(new Job(job.type(), versions, step.readyAt(change), change)), DeploymentStatus::union); }); return Collections.unmodifiableMap(jobs); @@ -266,16 +264,18 @@ public class DeploymentStatus { * For the "exclusive" revision upgrade policy it is the oldest such revision; otherwise, it is the latest. */ public Change outstandingChange(InstanceName instance) { - return Optional.ofNullable(instanceSteps().get(instance)) - .flatMap(instanceStatus -> application.deployableVersions(application.deploymentSpec().requireInstance(instance).revisionTarget() == next).stream() - .filter(version -> instanceStatus.dependenciesCompletedAt(Change.of(version), Optional.empty()).map(at -> ! at.isAfter(now)).orElse(false)) - .filter(version -> application.productionDeployments().getOrDefault(instance, List.of()).stream() - .noneMatch(deployment -> deployment.applicationVersion().compareTo(version) > 0)) - .map(Change::of) - .filter(change -> application.require(instance).change().application().map(change::upgrades).orElse(true)) - .filter(change -> ! hasCompleted(instance, change)) - .findFirst()) - .orElse(Change.empty()); + StepStatus status = instanceSteps().get(instance); + if (status == null) return Change.empty(); + for (ApplicationVersion version : application.deployableVersions(application.deploymentSpec().requireInstance(instance).revisionTarget() == next)) { + if (status.dependenciesCompletedAt(Change.of(version), Optional.empty()).map(now::isBefore).orElse(true)) continue; + Change change = Change.of(version); + if (application.productionDeployments().getOrDefault(instance, List.of()).stream() + .anyMatch(deployment -> change.downgrades(deployment.applicationVersion()))) continue; + if ( ! application.require(instance).change().application().map(change::upgrades).orElse(true)) continue; + if (hasCompleted(instance, change)) continue; + return change; + } + return Change.empty(); } /** Earliest instant when job was triggered with given versions, or both system and staging tests were successful. */ @@ -805,7 +805,7 @@ public class DeploymentStatus { .orElse(false)) return job.lastCompleted().flatMap(Run::end); - return (dependent.equals(job()) ? job.lastSuccess().stream() + return (dependent.equals(job()) ? job.lastTriggered().filter(run -> run.status() == RunStatus.success).stream() : RunList.from(job).status(RunStatus.success).asList().stream()) .filter(run -> change.platform().map(run.versions().targetPlatform()::equals).orElse(true) && change.application().map(run.versions().targetApplication()::equals).orElse(true)) @@ -857,10 +857,13 @@ public class DeploymentStatus { @Override Optional<Instant> completedAt(Change change, Optional<JobId> dependent) { return RunList.from(job) - .matching(run -> run.versions().targetsMatch(Versions.from(change, - status.application, - dependent.flatMap(status::deploymentFor), - status.systemVersion))) + .matching(run -> dependent.flatMap(status::deploymentFor) + .map(deployment -> run.versions().targetsMatch(Versions.from(change, + status.application, + Optional.of(deployment), + status.systemVersion))) + .orElseGet(() -> (change.platform().isEmpty() || change.platform().get().equals(run.versions().targetPlatform())) + && (change.application().isEmpty() || change.application().get().equals(run.versions().targetApplication())))) .status(RunStatus.success) .asList().stream() .map(run -> run.end().get()) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 66a30050f7a..647b52f7d51 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -607,11 +607,7 @@ public class JobController { private void prunePackages(TenantAndApplicationId id) { controller.applications().lockApplicationIfPresent(id, application -> { - application.get().productionDeployments().values().stream() - .flatMap(List::stream) - .map(Deployment::applicationVersion) - .filter(version -> ! version.isUnknown() && ! version.isDeployedDirectly()) - .min(naturalOrder()) + application.get().oldestDeployedApplication() .ifPresent(oldestDeployed -> { controller.applications().applicationStore().prune(id.tenant(), id.application(), oldestDeployed); controller.applications().applicationStore().pruneTesters(id.tenant(), id.application(), oldestDeployed); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java index 2b52143f574..189192d8d9a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java @@ -266,10 +266,12 @@ class JobControllerApiHandlerHelper { stepObject.setBool("declared", stepStatus.isDeclared()); stepObject.setString("instance", stepStatus.instance().value()); - stepStatus.readyAt(change).ifPresent(ready -> stepObject.setLong("readyAt", ready.toEpochMilli())); - stepStatus.readyAt(change) - .filter(controller.clock().instant()::isBefore) - .ifPresent(until -> stepObject.setLong("delayedUntil", until.toEpochMilli())); + // TODO: recursively search dependents for what is the relevant partial change when this is a delay step ... + Optional<Instant> readyAt = stepStatus.job().map(jobsToRun::get).map(jobs -> jobs.get(0).readyAt()) + .orElse(stepStatus.readyAt(change)); + readyAt.ifPresent(ready -> stepObject.setLong("readyAt", ready.toEpochMilli())); + readyAt.filter(controller.clock().instant()::isBefore) + .ifPresent(until -> stepObject.setLong("delayedUntil", until.toEpochMilli())); stepStatus.pausedUntil().ifPresent(until -> stepObject.setLong("pausedUntil", until.toEpochMilli())); stepStatus.coolingDownUntil(change).ifPresent(until -> stepObject.setLong("coolingDownUntil", until.toEpochMilli())); stepStatus.blockedUntil(Change.of(controller.systemVersion(versionStatus))) // Dummy version — just anything with a platform. @@ -304,17 +306,17 @@ class JobControllerApiHandlerHelper { for (VespaVersion available : availablePlatforms) { if ( deployments.stream().anyMatch(deployment -> deployment.version().isAfter(available.versionNumber())) || deployments.stream().noneMatch(deployment -> deployment.version().isBefore(available.versionNumber())) && ! deployments.isEmpty() - || change.platform().map(available.versionNumber()::compareTo).orElse(1) < 0) + || status.hasCompleted(stepStatus.instance(), Change.of(available.versionNumber())) + || change.platform().map(available.versionNumber()::compareTo).orElse(1) <= 0) break; - Cursor availableObject = availableArray.addObject(); - availableObject.setString("platform", available.versionNumber().toFullString()); + availableArray.addObject().setString("platform", available.versionNumber().toFullString()); } + change.platform().ifPresent(version -> availableArray.addObject().setString("platform", version.toFullString())); toSlime(latestPlatformObject.setArray("blockers"), blockers.stream().filter(ChangeBlocker::blocksVersions)); } - List<ApplicationVersion> availableApplications = new ArrayList<>(application.versions()); + List<ApplicationVersion> availableApplications = new ArrayList<>(application.deployableVersions(false)); if ( ! availableApplications.isEmpty()) { - Collections.reverse(availableApplications); var latestApplication = availableApplications.get(0); Cursor latestApplicationObject = latestVersionsObject.setObject("application"); toSlime(latestApplicationObject.setObject("application"), latestApplication); @@ -326,12 +328,13 @@ class JobControllerApiHandlerHelper { for (ApplicationVersion available : availableApplications) { if ( deployments.stream().anyMatch(deployment -> deployment.applicationVersion().compareTo(available) > 0) || deployments.stream().noneMatch(deployment -> deployment.applicationVersion().compareTo(available) < 0) && ! deployments.isEmpty() - || change.application().map(available::compareTo).orElse(1) < 0) + || status.hasCompleted(stepStatus.instance(), Change.of(available)) + || change.application().map(available::compareTo).orElse(1) <= 0) break; - Cursor availableObject = availableArray.addObject(); - toSlime(availableObject.setObject("application"), available); + toSlime(availableArray.addObject().setObject("application"), available); } + change.application().ifPresent(version -> toSlime(availableArray.addObject().setObject("application"), version)); toSlime(latestApplicationObject.setArray("blockers"), blockers.stream().filter(ChangeBlocker::blocksRevisions)); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java index 21cc69369d8..989a7c31821 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java @@ -236,8 +236,13 @@ public class DeploymentContext { /** Flush count pending DNS updates */ public DeploymentContext flushDnsUpdates(int count) { var dispatcher = new NameServiceDispatcher(tester.controller(), Duration.ofSeconds(count)); - dispatcher.run(); - return this; + try { + dispatcher.run(); + return this; + } + finally { + dispatcher.shutdown(); + } } /** Add a routing policy for this in given zone, with status set to inactive */ @@ -255,6 +260,11 @@ public class DeploymentContext { } /** Submit given application package for deployment */ + public DeploymentContext resubmit(ApplicationPackage applicationPackage) { + return submit(applicationPackage, Optional.of(defaultSourceRevision), salt.get()); + } + + /** Submit given application package for deployment */ public DeploymentContext submit(ApplicationPackage applicationPackage) { return submit(applicationPackage, Optional.of(defaultSourceRevision)); } @@ -266,7 +276,7 @@ public class DeploymentContext { /** Submit given application package for deployment */ public DeploymentContext submit(ApplicationPackage applicationPackage, Optional<SourceRevision> sourceRevision) { - return submit(applicationPackage, sourceRevision, salt.getAndIncrement()); + return submit(applicationPackage, sourceRevision, salt.incrementAndGet()); } /** Submit given application package for deployment */ @@ -597,8 +607,9 @@ public class DeploymentContext { runner.advance(currentRun(job)); assertTrue(jobs.run(id).get().hasEnded()); assertFalse(jobs.run(id).get().hasFailed()); - assertEquals(job.type().isProduction(), instance().deployments().containsKey(zone)); - assertTrue(configServer().nodeRepository().list(zone, NodeFilter.all().applications(TesterId.of(id.application()).id())).isEmpty()); + Instance instance = tester.application(TenantAndApplicationId.from(instanceId)).require(id.application().instance()); + assertEquals(job.type().isProduction(), instance.deployments().containsKey(zone)); + assertTrue(configServer().nodeRepository().list(zone, NodeFilter.all().applications(TesterId.of(instance.id()).id())).isEmpty()); } private JobId jobId(JobType type) { 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 b2847a29654..c8126207a73 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 @@ -1873,7 +1873,7 @@ public class DeploymentTriggerTest { // Deploy manually again, then submit new package. app.runJob(productionCdUsEast1, cdPackage); app.submit(cdPackage); - app.runJob(systemTest); + app.triggerJobs().jobAborted(systemTest).runJob(systemTest); // Staging test requires unknown initial version, and is broken. tester.controller().applications().deploymentTrigger().forceTrigger(app.instanceId(), productionCdUsEast1, "user", false, true, true); app.runJob(productionCdUsEast1) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java index 265dedec8d0..78341682f75 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java @@ -945,9 +945,9 @@ public class UpgraderTest { // Application downgrades to pinned version. tester.abortAll(); - context.runJob(stagingTest).runJob(productionUsCentral1); + context.runJob(stagingTest).runJob(productionUsCentral1).runJob(productionUsWest1); assertTrue(context.instance().change().hasTargets()); - context.runJob(productionUsWest1); // us-east-3 never upgraded, so no downgrade is needed. + context.runJob(productionUsEast3); assertFalse(context.instance().change().hasTargets()); } @@ -1011,7 +1011,7 @@ public class UpgraderTest { tester.controllerTester().upgradeSystem(v2); tester.upgrader().maintain(); assertEquals(Change.of(v2), application.instance().change()); - application.runJob(systemTest).runJob(stagingTest).runJob(productionUsCentral1); + application.runJob(systemTest).runJob(stagingTest).runJob(productionUsCentral1).timeOutConvergence(productionUsWest1); tester.triggerJobs(); // While second deployment completes upgrade, version confidence becomes broken and upgrade is cancelled diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java index 6b13e6c951a..87fe8dd17fe 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java @@ -35,10 +35,12 @@ public class ControllerContainerTest { @Before public void startContainer() { - container = JDisc.fromServicesXml(controllerServicesXml(), Networking.disable); + container = JDisc.fromServicesXml(controllerServicesXml(), networking()); addUserToHostedOperatorRole(hostedOperator); } + protected Networking networking() { return Networking.disable; } + @After public void stopContainer() { container.close(); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json index 8677b621f0d..c0a6829b026 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json @@ -579,14 +579,6 @@ }, { "application": { - "build": 3, - "compileVersion": "6.1.0", - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - } - }, - { - "application": { "build": 2, "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/playground/AllowingFilter.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/playground/AllowingFilter.java new file mode 100644 index 00000000000..de7f2515979 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/playground/AllowingFilter.java @@ -0,0 +1,41 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.restapi.playground; + +import com.yahoo.jdisc.http.filter.DiscFilterRequest; +import com.yahoo.jdisc.http.filter.security.base.JsonSecurityRequestFilterBase; +import com.yahoo.vespa.athenz.api.AthenzDomain; +import com.yahoo.vespa.athenz.api.AthenzPrincipal; +import com.yahoo.vespa.athenz.api.AthenzUser; +import com.yahoo.vespa.hosted.controller.api.integration.user.User; +import com.yahoo.vespa.hosted.controller.api.role.Role; +import com.yahoo.vespa.hosted.controller.api.role.SecurityContext; +import com.yahoo.yolean.Exceptions; + +import java.time.Instant; +import java.util.Optional; +import java.util.Set; + +public class AllowingFilter extends JsonSecurityRequestFilterBase { + + static final AthenzPrincipal user = new AthenzPrincipal(new AthenzUser("demo")); + static final AthenzDomain domain = new AthenzDomain("domain"); + + @Override + protected Optional<ErrorResponse> filter(DiscFilterRequest request) { + try { + request.setUserPrincipal(user); + request.setAttribute(User.ATTRIBUTE_NAME, new User("mail@mail", user.getName(), "demo", null, true, -1, User.NO_DATE)); + request.setAttribute("okta.identity-token", "okta-it"); + request.setAttribute("okta.access-token", "okta-at"); + request.setAttribute(SecurityContext.ATTRIBUTE_NAME, + new SecurityContext(user, + Set.of(Role.hostedOperator()), + Instant.now().minusSeconds(3600))); + return Optional.empty(); + } + catch (Throwable t) { + return Optional.of(new ErrorResponse(500, Exceptions.toMessageString(t))); + } + } + +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/playground/DeploymentPlayground.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/playground/DeploymentPlayground.java new file mode 100644 index 00000000000..2c91aceb8b0 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/playground/DeploymentPlayground.java @@ -0,0 +1,226 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.restapi.playground; + +import com.yahoo.application.Networking; +import com.yahoo.component.Version; +import com.yahoo.vespa.hosted.controller.ControllerTester; +import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzDbMock; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; +import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; +import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext; +import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; +import com.yahoo.vespa.hosted.controller.deployment.Run; +import com.yahoo.vespa.hosted.controller.restapi.ContainerTester; +import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.time.Duration; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.BiConsumer; + +public class DeploymentPlayground extends ControllerContainerTest { + + private final Object monitor = new Object(); + private DeploymentTester deploymentTester; + + @Override + protected Networking networking() { return Networking.enable; } + + public static void main(String[] args) throws IOException, InterruptedException { + DeploymentPlayground test = null; + try { + test = new DeploymentPlayground(); + test.startContainer(); + test.run(); + } + catch (Throwable t) { + t.printStackTrace(); + } + if (test != null && test.container != null) { + test.stopContainer(); + test.deploymentTester.runner().shutdown(); + test.deploymentTester.upgrader().shutdown(); + test.deploymentTester.readyJobsTrigger().shutdown(); + test.deploymentTester.outstandingChangeDeployer().shutdown(); + } + } + + public void run() throws IOException { + ContainerTester tester = new ContainerTester(container, ""); + deploymentTester = new DeploymentTester(new ControllerTester(tester)); + deploymentTester.controllerTester().computeVersionStatus(); + + AthenzDbMock.Domain domainMock = tester.athenzClientFactory().getSetup().getOrCreateDomain(AllowingFilter.domain); + domainMock.markAsVespaTenant(); + domainMock.admin(AllowingFilter.user.getIdentity()); + + Map<String, DeploymentContext> instances = new LinkedHashMap<>(); + for (String name : List.of("alpha", "beta", "prod5", "prod25", "prod100")) + instances.put(name, deploymentTester.newDeploymentContext("gemini", "core", name)); + + instances.values().iterator().next().submit(ApplicationPackageBuilder.fromDeploymentXml(readDeploymentXml())).deploy(); + + repl(instances); + } + + static String readDeploymentXml() throws IOException { + return Files.readString(Paths.get(System.getProperty("user.home") + "/git/" + + "vespa/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/playground/deployment.xml")); + } + + void repl(Map<String, DeploymentContext> instances) throws IOException { + String[] command; + BufferedReader in = new BufferedReader(new InputStreamReader(System.in)); + AtomicBoolean on = new AtomicBoolean(); + Thread auto = new Thread(() -> auto(instances, on)); + auto.setDaemon(true); + auto.start(); + while (true) { + try { + command = in.readLine().trim().split("\\s+"); + if (command.length == 0 || command[0].isEmpty()) continue; + synchronized (monitor) { + switch (command[0]) { + case "exit": + auto.interrupt(); + return; + case "tick": + deploymentTester.controllerTester().computeVersionStatus(); + deploymentTester.outstandingChangeDeployer().run(); + deploymentTester.upgrader().run(); + deploymentTester.triggerJobs(); + deploymentTester.runner().run(); + break; + case "run": + run(instances.get(command[1]), DeploymentContext::runJob, List.of(command).subList(2, command.length)); + break; + case "fail": + run(instances.get(command[1]), DeploymentContext::failDeployment, List.of(command).subList(2, command.length)); + break; + case "upgrade": + deploymentTester.controllerTester().upgradeSystem(new Version(command[1])); + deploymentTester.controllerTester().computeVersionStatus(); + break; + case "submit": + instances.values().iterator().next().submit(ApplicationPackageBuilder.fromDeploymentXml(readDeploymentXml())); + break; + case "resubmit": + instances.values().iterator().next().resubmit(ApplicationPackageBuilder.fromDeploymentXml(readDeploymentXml())); + break; + case "advance": + deploymentTester.clock().advance(Duration.ofMinutes(Long.parseLong(command[1]))); + break; + case "auto": + switch (command[1]) { + case "on": on.set(true); break; + case "off": on.set(false); break; + default: System.err.println("Argument to 'auto' must be 'on' or 'off'"); break; + } + break; + default: + System.err.println("Cannot run '" + String.join(" ", command) + "'"); + } + } + } + catch (Throwable t) { + t.printStackTrace(); + } + } + } + + void auto(Map<String, DeploymentContext> instances, AtomicBoolean on) { + while ( ! Thread.currentThread().isInterrupted()) { + try { + synchronized (monitor) { + monitor.wait(6000); + if ( ! on.get()) + continue; + + System.err.println("auto running"); + deploymentTester.clock().advance(Duration.ofSeconds(60)); + deploymentTester.controllerTester().computeVersionStatus(); + deploymentTester.outstandingChangeDeployer().run(); + deploymentTester.upgrader().run(); + deploymentTester.triggerJobs(); + deploymentTester.runner().run(); + for (Run run : deploymentTester.jobs().active()) + if (run.versions().sourcePlatform().map(run.versions().targetPlatform()::equals).orElse(true) || Math.random() < 0.4) + instances.get(run.id().application().instance().value()).runJob(run.id().type()); + } + } + catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + catch (Throwable t) { + t.printStackTrace(); + } + } + } + + void run(DeploymentContext instance, BiConsumer<DeploymentContext, JobType> action, List<String> jobs) { + for (boolean triggered = true; triggered; ) { + triggered = false; + for (Run run : deploymentTester.jobs().active(instance.instanceId())) + if (jobs.isEmpty() || jobs.contains(run.id().type().jobName().replace("production-", ""))) { + action.accept(instance, run.id().type()); + triggered = true; + } + } + } + + @Override + protected String variablePartXml() { + return " <component id='com.yahoo.vespa.hosted.controller.security.AthenzAccessControlRequests'/>\n" + + " <component id='com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade'/>\n" + + + " <handler id='com.yahoo.vespa.hosted.controller.restapi.application.ApplicationApiHandler'>\n" + + " <binding>http://*/application/v4/*</binding>\n" + + " </handler>\n" + + " <handler id='com.yahoo.vespa.hosted.controller.restapi.athenz.AthenzApiHandler'>\n" + + " <binding>http://*/athenz/v1/*</binding>\n" + + " </handler>\n" + + " <handler id='com.yahoo.vespa.hosted.controller.restapi.zone.v1.ZoneApiHandler'>\n" + + " <binding>http://*/zone/v1</binding>\n" + + " <binding>http://*/zone/v1/*</binding>\n" + + " </handler>\n" + + + " <http>\n" + + " <server id='default' port='8080' />\n" + + " <filtering>\n" + + " <request-chain id='default'>\n" + + " <filter id='com.yahoo.jdisc.http.filter.security.cors.CorsPreflightRequestFilter'>\n" + + " <config name=\"jdisc.http.filter.security.cors.cors-filter\">" + + " <allowedUrls>\n" + + " <item>http://localhost:3000</item>\n" + + " <item>http://localhost:8080</item>\n" + + " </allowedUrls>\n" + + " </config>\n" + + " </filter>\n" + + " <filter id='com.yahoo.vespa.hosted.controller.restapi.playground.AllowingFilter'/>\n" + + " <filter id='com.yahoo.vespa.hosted.controller.restapi.filter.ControllerAuthorizationFilter'/>\n" + + " <binding>http://*/*</binding>\n" + + " </request-chain>\n" + + " <response-chain id='responses'>\n" + + " <filter id='com.yahoo.jdisc.http.filter.security.cors.CorsResponseFilter'>\n" + + " <config name=\"jdisc.http.filter.security.cors.cors-filter\">" + + " <allowedUrls>\n" + + " <item>http://localhost:3000</item>\n" + + " <item>http://localhost:8080</item>\n" + + " </allowedUrls>\n" + + " </config>\n" + + " </filter>\n" + + " <binding>http://*/*</binding>\n" + + " </response-chain>\n" + + " </filtering>\n" + + " </http>\n"; + } + +} + diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/playground/deployment.xml b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/playground/deployment.xml new file mode 100644 index 00000000000..a3642acb21a --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/playground/deployment.xml @@ -0,0 +1,71 @@ +<deployment> + <instance id='alpha'> + <upgrade rollout="simultaneous" revision-change="when-failing" revision-target="next" /> + <test /> + <staging /> + </instance> + <instance id='beta'> + <upgrade rollout="simultaneous" revision-change="when-clear" revision-target="latest" /> + <prod> + <region>us-east-3</region> + <test>us-east-3</test> + </prod> + </instance> + <instance id='prod5'> + <upgrade rollout="simultaneous" revision-change="when-clear" revision-target="next" /> + <prod> + <parallel> + <steps> + <region>us-east-3</region> + <test>us-east-3</test> + </steps> + <steps> + <region>us-central-1</region> + <test>us-central-1</test> + </steps> + <steps> + <region>us-west-1</region> + <test>us-west-1</test> + </steps> + </parallel> + </prod> + </instance> + <instance id='prod25'> + <upgrade rollout="simultaneous" revision-change="when-failing" revision-target="next" /> + <prod> + <parallel> + <steps> + <region>us-east-3</region> + <test>us-east-3</test> + </steps> + <steps> + <region>us-central-1</region> + <test>us-central-1</test> + </steps> + <steps> + <region>us-west-1</region> + <test>us-west-1</test> + </steps> + </parallel> + </prod> + </instance> + <instance id='prod100'> + <upgrade rollout="simultaneous" revision-change="when-clear" revision-target="next" /> + <prod> + <parallel> + <steps> + <region>us-east-3</region> + <test>us-east-3</test> + </steps> + <steps> + <region>us-central-1</region> + <test>us-central-1</test> + </steps> + <steps> + <region>us-west-1</region> + <test>us-west-1</test> + </steps> + </parallel> + </prod> + </instance> +</deployment> diff --git a/default_build_settings.cmake b/default_build_settings.cmake index 1432ea7164c..1925e5f42b6 100644 --- a/default_build_settings.cmake +++ b/default_build_settings.cmake @@ -86,6 +86,12 @@ function(setup_vespa_default_build_settings_fedora_36) set(DEFAULT_VESPA_LLVM_VERSION "13" PARENT_SCOPE) endfunction() +function(setup_vespa_default_build_settings_fedora_37) + message("-- Setting up default build settings for fedora 37") + set(DEFAULT_EXTRA_INCLUDE_DIRECTORY "${VESPA_DEPS}/include" "/usr/include/openblas" PARENT_SCOPE) + set(DEFAULT_VESPA_LLVM_VERSION "13" PARENT_SCOPE) +endfunction() + function(setup_vespa_default_build_settings_amzn_2) message("-- Setting up default build settings for amzn 2") set(DEFAULT_EXTRA_LINK_DIRECTORY "${VESPA_DEPS}/lib64" "/usr/lib64/llvm7.0/lib" PARENT_SCOPE) @@ -206,6 +212,8 @@ function(vespa_use_default_build_settings) setup_vespa_default_build_settings_fedora_35() elseif(VESPA_OS_DISTRO_COMBINED STREQUAL "fedora 36") setup_vespa_default_build_settings_fedora_36() + elseif(VESPA_OS_DISTRO_COMBINED STREQUAL "fedora 37") + setup_vespa_default_build_settings_fedora_37() elseif(VESPA_OS_DISTRO_COMBINED STREQUAL "amzn 2") setup_vespa_default_build_settings_amzn_2() elseif(VESPA_OS_DISTRO STREQUAL "ubuntu") diff --git a/dist/vespa.spec b/dist/vespa.spec index 74ef6a6f8fc..e6ef819b458 100644 --- a/dist/vespa.spec +++ b/dist/vespa.spec @@ -177,7 +177,14 @@ BuildRequires: gmock-devel %endif %if 0%{?fc36} BuildRequires: protobuf-devel -BuildRequires: llvm-devel >= 13.0.0 +BuildRequires: llvm-devel >= 13.0.1 +BuildRequires: boost-devel >= 1.76 +BuildRequires: gtest-devel +BuildRequires: gmock-devel +%endif +%if 0%{?fc37} +BuildRequires: protobuf-devel +BuildRequires: llvm-devel >= 13.0.1 BuildRequires: boost-devel >= 1.76 BuildRequires: gtest-devel BuildRequires: gmock-devel @@ -316,6 +323,9 @@ Requires: gtest %if 0%{?fc36} %define _vespa_llvm_version 13 %endif +%if 0%{?fc37} +%define _vespa_llvm_version 13 +%endif %define _extra_link_directory %{_vespa_deps_prefix}/lib64 %define _extra_include_directory %{_vespa_deps_prefix}/include;/usr/include/openblas %endif @@ -434,7 +444,10 @@ Requires: llvm-libs >= 12.0.0 Requires: llvm-libs >= 13.0.0 %endif %if 0%{?fc36} -Requires: llvm-libs >= 13.0.0 +Requires: llvm-libs >= 13.0.1 +%endif +%if 0%{?fc37} +Requires: llvm-libs >= 13.0.1 %endif %endif Requires: vespa-onnxruntime = 1.7.1 diff --git a/docproc/src/main/java/com/yahoo/docproc/Processing.java b/docproc/src/main/java/com/yahoo/docproc/Processing.java index 47ebb6a7988..834d63c5a86 100644 --- a/docproc/src/main/java/com/yahoo/docproc/Processing.java +++ b/docproc/src/main/java/com/yahoo/docproc/Processing.java @@ -140,10 +140,8 @@ public class Processing { } /** - * @deprecated Use TBD instead + * @deprecated This method will be removed without replacement in Vespa 8. */ - // TODO: used to: processing.setDocprocServiceRegistry(this.documentProcessingHandler.getDocprocServiceRegistry()); - // from Processor and LoggingRequestHandler @Deprecated(forRemoval = true, since="7") @SuppressWarnings("removal") // TODO Vespa 8: remove public void setDocprocServiceRegistry(ComponentRegistry<DocprocService> docprocServiceRegistry) { @@ -166,9 +164,9 @@ public class Processing { * if #getServiceName returns a name that is not registered in {@link com.yahoo.docproc.DocprocService}. * * @return the service processing this, or null if unknown. - * @deprecated Use TBD instead + * @deprecated Formerly used to retrieve the {@link com.yahoo.document.DocumentTypeManager}, + * which can now be directly injected via your component constructor. */ - // TODO: used to getService().getDocumentTypeManager() in subclasses of DocumentProcessor @Deprecated(forRemoval = true, since="7") @SuppressWarnings("removal") // TODO Vespa 8: remove public DocprocService getService() { diff --git a/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/ProcessingFactory.java b/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/ProcessingFactory.java index e8a2e214776..33cd6647ede 100644 --- a/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/ProcessingFactory.java +++ b/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/ProcessingFactory.java @@ -105,8 +105,10 @@ class ProcessingFactory { Processing processing = new Processing(); processing.addDocumentOperation(documentOperation); processing.setServiceName(serviceName); - // TODO Vespa 8: Use TBD instead, this method will be removed + + // TODO Vespa 8: Remove statement (registry will be removed from Processing) processing.setDocprocServiceRegistry(docprocServiceComponentRegistry); + processing.setVariable("route", message.getRoute()); processing.setVariable("timeout", message.getTimeRemaining()); return processing; diff --git a/document/src/main/java/com/yahoo/document/DataType.java b/document/src/main/java/com/yahoo/document/DataType.java index bb1777954a3..697536a8ac0 100644 --- a/document/src/main/java/com/yahoo/document/DataType.java +++ b/document/src/main/java/com/yahoo/document/DataType.java @@ -52,6 +52,7 @@ public abstract class DataType extends Identifiable implements Serializable, Com public final static DocumentType DOCUMENT = new DocumentType("document"); public final static PrimitiveDataType URI = new PrimitiveDataType("uri", 10, UriFieldValue.class, new UriFieldValue.Factory()); public final static NumericDataType BYTE = new NumericDataType("byte", 16, ByteFieldValue.class, ByteFieldValue.getFactory()); + final static int TAG_ID = 18; public final static PrimitiveDataType PREDICATE = new PrimitiveDataType("predicate", 20, PredicateFieldValue.class, PredicateFieldValue.getFactory()); public final static int tensorDataTypeCode = 21; // All TensorDataType instances have id=21 but carries additional type information serialized separately // ADDITIONAL parametrized types added at runtime: map, struct, array, weighted set, annotation reference, tensor diff --git a/document/src/main/java/com/yahoo/document/WeightedSetDataType.java b/document/src/main/java/com/yahoo/document/WeightedSetDataType.java index 35dd13efb0b..b21f059bd7d 100644 --- a/document/src/main/java/com/yahoo/document/WeightedSetDataType.java +++ b/document/src/main/java/com/yahoo/document/WeightedSetDataType.java @@ -24,25 +24,33 @@ public class WeightedSetDataType extends CollectionDataType { public WeightedSetDataType(DataType nestedType, boolean createIfNonExistent, boolean removeIfZero) { this(nestedType, createIfNonExistent, removeIfZero, 0); - if ((nestedType == STRING) && createIfNonExistent && removeIfZero) { // the tag type definition - setId(18); - } else { - setId(getName().toLowerCase().hashCode()); - } } public WeightedSetDataType(DataType nestedType, boolean createIfNonExistent, boolean removeIfZero, int id) { super(createName(nestedType, createIfNonExistent, removeIfZero), id, nestedType); this.createIfNonExistent = createIfNonExistent; this.removeIfZero = removeIfZero; + if (id == 0) { + if ((nestedType == STRING) && createIfNonExistent && removeIfZero) { // the tag type definition + setId(TAG_ID); + } else { + setId(getName().toLowerCase().hashCode()); + } + } + int code = getId(); + if ((code >= 0) && (code <= DataType.lastPredefinedDataTypeId()) && (code != TAG_ID)) { + throw new IllegalArgumentException("Cannot create a weighted set datatype with code " + code); + } } + /* + * @deprecated // TODO remove on Vespa 8 + * Do not use - use one of the constructors above. + * Note: ignores typeName argument. + */ + @Deprecated public WeightedSetDataType(String typeName, int code, DataType nestedType, boolean createIfNonExistent, boolean removeIfZero) { - super(typeName != null ? createName(nestedType, createIfNonExistent, removeIfZero) : null, code, nestedType); - if ((code >= 0) && (code <= DataType.lastPredefinedDataTypeId()) && (code != 18)) // 18 == DataType.TAG.getId() is not yet initialized - throw new IllegalArgumentException("Cannot create a weighted set datatype with code " + code); - this.createIfNonExistent = createIfNonExistent; - this.removeIfZero = removeIfZero; + this(nestedType, createIfNonExistent, removeIfZero, code); } @Override diff --git a/logserver/bin/logserver-start.sh b/logserver/bin/logserver-start.sh index 0853413ec68..384abdc31e9 100755 --- a/logserver/bin/logserver-start.sh +++ b/logserver/bin/logserver-start.sh @@ -78,7 +78,7 @@ ROOT=${VESPA_HOME%/} export ROOT cd $ROOT || { echo "Cannot cd to $ROOT" 1>&2; exit 1; } -addopts="-server -Xms32m -Xmx256m -XX:CompressedClassSpaceSize=32m -XX:MaxDirectMemorySize=32m -XX:ThreadStackSize=256 -XX:MaxJavaStackTraceDepth=1000 -XX:ActiveProcessorCount=2 -XX:-OmitStackTraceInFastThrow -Djava.io.tmpdir=${VESPA_HOME}/tmp" +addopts="-server -Xms32m -Xmx256m -XX:CompressedClassSpaceSize=32m -XX:MaxDirectMemorySize=32m -XX:ThreadStackSize=448 -XX:MaxJavaStackTraceDepth=1000 -XX:ActiveProcessorCount=2 -XX:-OmitStackTraceInFastThrow -Djava.io.tmpdir=${VESPA_HOME}/tmp" oomopt="-XX:+ExitOnOutOfMemoryError" diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepoStats.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepoStats.java index ba8aa49d87c..338187c5270 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepoStats.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepoStats.java @@ -12,6 +12,7 @@ import com.yahoo.vespa.hosted.provision.autoscale.NodeTimeseries; import java.time.Duration; import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -122,6 +123,10 @@ public class NodeRepoStats { public static class ApplicationStats implements Comparable<ApplicationStats> { + private static final Comparator<ApplicationStats> comparator = Comparator.comparingDouble(ApplicationStats::unutilizedCost).reversed() + .thenComparingDouble(ApplicationStats::cost) + .thenComparing(ApplicationStats::id); + private final ApplicationId id; private final Load load; private final double cost; @@ -148,7 +153,7 @@ public class NodeRepoStats { @Override public int compareTo(NodeRepoStats.ApplicationStats other) { - return -Double.compare(this.unutilizedCost(), other.unutilizedCost()); + return comparator.compare(this, other); } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/stats.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/stats.json index 8867520fef6..8a46f8115be 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/stats.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/stats.json @@ -11,7 +11,7 @@ }, "applications": [ { - "id": "tenant3.application3.instance3", + "id": "tenant1.application1.instance1", "load": { "cpu": 0.0, "memory": 0.0, @@ -31,7 +31,7 @@ "unutilizedCost": 0.0 }, { - "id": "tenant4.application4.instance4", + "id": "tenant3.application3.instance3", "load": { "cpu": 0.0, "memory": 0.0, @@ -41,7 +41,7 @@ "unutilizedCost": 0.0 }, { - "id": "tenant1.application1.instance1", + "id": "tenant4.application4.instance4", "load": { "cpu": 0.0, "memory": 0.0, diff --git a/searchcommon/src/vespa/searchcommon/attribute/status.cpp b/searchcommon/src/vespa/searchcommon/attribute/status.cpp index 0509e9a51c4..a7d1f5b3d38 100644 --- a/searchcommon/src/vespa/searchcommon/attribute/status.cpp +++ b/searchcommon/src/vespa/searchcommon/attribute/status.cpp @@ -1,6 +1,10 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "status.h" +#include <vespa/vespalib/util/atomic.h> + +using namespace vespalib::atomic; + namespace search::attribute { Status::Status() @@ -21,15 +25,15 @@ Status::Status() } Status::Status(const Status& rhs) - : _numDocs(rhs._numDocs), - _numValues(rhs._numValues), - _numUniqueValues(rhs._numUniqueValues), - _allocated(rhs._allocated), - _used(rhs._used), - _dead(rhs._dead), - _unused(rhs._unused), - _onHold(rhs._onHold), - _onHoldMax(rhs._onHoldMax), + : _numDocs(load_relaxed(rhs._numDocs)), + _numValues(load_relaxed(rhs._numValues)), + _numUniqueValues(load_relaxed(rhs._numUniqueValues)), + _allocated(load_relaxed(rhs._allocated)), + _used(load_relaxed(rhs._used)), + _dead(load_relaxed(rhs._dead)), + _unused(load_relaxed(rhs._unused)), + _onHold(load_relaxed(rhs._onHold)), + _onHoldMax(load_relaxed(rhs._onHoldMax)), _lastSyncToken(rhs.getLastSyncToken()), _updates(rhs._updates), _nonIdempotentUpdates(rhs._nonIdempotentUpdates), @@ -40,15 +44,15 @@ Status::Status(const Status& rhs) Status& Status::operator=(const Status& rhs) { - _numDocs = rhs._numDocs; - _numValues = rhs._numValues; - _numUniqueValues = rhs._numUniqueValues; - _allocated = rhs._allocated; - _used = rhs._used; - _dead = rhs._dead; - _unused = rhs._unused; - _onHold = rhs._onHold; - _onHoldMax = rhs._onHoldMax; + store_relaxed(_numDocs, load_relaxed(rhs._numDocs)); + store_relaxed(_numValues, load_relaxed(rhs._numValues)); + store_relaxed(_numUniqueValues, load_relaxed(rhs._numUniqueValues)); + store_relaxed(_allocated, load_relaxed(rhs._allocated)); + store_relaxed(_used, load_relaxed(rhs._used)); + store_relaxed(_dead, load_relaxed(rhs._dead)); + store_relaxed(_unused, load_relaxed(rhs._unused)); + store_relaxed(_onHold, load_relaxed(rhs._onHold)); + store_relaxed(_onHoldMax, load_relaxed(rhs._onHoldMax)); setLastSyncToken(rhs.getLastSyncToken()); _updates = rhs._updates; _nonIdempotentUpdates = rhs._nonIdempotentUpdates; @@ -69,14 +73,14 @@ void Status::updateStatistics(uint64_t numValues, uint64_t numUniqueValue, uint64_t allocated, uint64_t used, uint64_t dead, uint64_t onHold) { - _numValues = numValues; - _numUniqueValues = numUniqueValue; - _allocated = allocated; - _used = used; - _dead = dead; - _unused = allocated - used; - _onHold = onHold; - _onHoldMax = std::max(_onHoldMax, onHold); + store_relaxed(_numValues, numValues); + store_relaxed(_numUniqueValues, numUniqueValue); + store_relaxed(_allocated, allocated); + store_relaxed(_used, used); + store_relaxed(_dead, dead); + store_relaxed(_unused, allocated - used); + store_relaxed(_onHold, onHold); + store_relaxed(_onHoldMax, std::max(load_relaxed(_onHoldMax), onHold)); } } diff --git a/searchcommon/src/vespa/searchcommon/attribute/status.h b/searchcommon/src/vespa/searchcommon/attribute/status.h index 0280bfaae4e..f2212d4c76a 100644 --- a/searchcommon/src/vespa/searchcommon/attribute/status.h +++ b/searchcommon/src/vespa/searchcommon/attribute/status.h @@ -17,22 +17,23 @@ public: void updateStatistics(uint64_t numValues, uint64_t numUniqueValue, uint64_t allocated, uint64_t used, uint64_t dead, uint64_t onHold); - uint64_t getNumDocs() const { return _numDocs; } - uint64_t getNumValues() const { return _numValues; } - uint64_t getNumUniqueValues() const { return _numUniqueValues; } - uint64_t getAllocated() const { return _allocated; } - uint64_t getUsed() const { return _used; } - uint64_t getDead() const { return _dead; } - uint64_t getOnHold() const { return _onHold; } - uint64_t getOnHoldMax() const { return _onHoldMax; } + uint64_t getNumDocs() const { return _numDocs.load(std::memory_order_relaxed); } + uint64_t getNumValues() const { return _numValues.load(std::memory_order_relaxed); } + uint64_t getNumUniqueValues() const { return _numUniqueValues.load(std::memory_order_relaxed); } + uint64_t getAllocated() const { return _allocated.load(std::memory_order_relaxed); } + uint64_t getUsed() const { return _used.load(std::memory_order_relaxed); } + uint64_t getDead() const { return _dead.load(std::memory_order_relaxed); } + uint64_t getOnHold() const { return _onHold.load(std::memory_order_relaxed); } + uint64_t getOnHoldMax() const { return _onHoldMax.load(std::memory_order_relaxed); } // This might be accessed from other threads than the writer thread. uint64_t getLastSyncToken() const { return _lastSyncToken.load(std::memory_order_relaxed); } uint64_t getUpdateCount() const { return _updates; } uint64_t getNonIdempotentUpdateCount() const { return _nonIdempotentUpdates; } uint32_t getBitVectors() const { return _bitVectors; } - void setNumDocs(uint64_t v) { _numDocs = v; } - void incNumDocs() { ++_numDocs; } + void setNumDocs(uint64_t v) { _numDocs.store(v, std::memory_order_relaxed); } + void incNumDocs() { _numDocs.store(_numDocs.load(std::memory_order_relaxed) + 1u, + std::memory_order_relaxed); } void setLastSyncToken(uint64_t v) { _lastSyncToken.store(v, std::memory_order_relaxed); } void incUpdates(uint64_t v=1) { _updates += v; } void incNonIdempotentUpdates(uint64_t v = 1) { _nonIdempotentUpdates += v; } @@ -42,15 +43,15 @@ public: static vespalib::string createName(vespalib::stringref index, vespalib::stringref attr); private: - uint64_t _numDocs; - uint64_t _numValues; - uint64_t _numUniqueValues; - uint64_t _allocated; - uint64_t _used; - uint64_t _dead; - uint64_t _unused; - uint64_t _onHold; - uint64_t _onHoldMax; + std::atomic<uint64_t> _numDocs; + std::atomic<uint64_t> _numValues; + std::atomic<uint64_t> _numUniqueValues; + std::atomic<uint64_t> _allocated; + std::atomic<uint64_t> _used; + std::atomic<uint64_t> _dead; + std::atomic<uint64_t> _unused; + std::atomic<uint64_t> _onHold; + std::atomic<uint64_t> _onHoldMax; std::atomic<uint64_t> _lastSyncToken; uint64_t _updates; uint64_t _nonIdempotentUpdates; diff --git a/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.cpp b/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.cpp index 8489a68af15..0ec27ac419f 100644 --- a/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.cpp +++ b/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.cpp @@ -177,14 +177,14 @@ MatchEngine::performSearch(search::engine::SearchRequest::Source req) } bool MatchEngine::isOnline() const { - return _nodeUp; + return _nodeUp.load(std::memory_order_relaxed); } void MatchEngine::setNodeUp(bool nodeUp) { - _nodeUp = nodeUp; + _nodeUp.store(nodeUp, std::memory_order_relaxed); } void @@ -192,7 +192,7 @@ MatchEngine::setNodeMaintenance(bool nodeMaintenance) { _nodeMaintenance = nodeMaintenance; if (nodeMaintenance) { - _nodeUp = false; + _nodeUp.store(false, std::memory_order_relaxed); } } diff --git a/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.h b/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.h index 3d3be775a4a..fcafdc5a5f8 100644 --- a/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.h +++ b/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.h @@ -25,7 +25,7 @@ private: HandlerMap<ISearchHandler> _handlers; vespalib::ThreadStackExecutor _executor; vespalib::SimpleThreadBundle::Pool _threadBundlePool; - bool _nodeUp; + std::atomic<bool> _nodeUp; bool _nodeMaintenance; public: diff --git a/searchcore/src/vespa/searchcore/proton/server/feedhandler.cpp b/searchcore/src/vespa/searchcore/proton/server/feedhandler.cpp index e80d8033751..402de8ce7ea 100644 --- a/searchcore/src/vespa/searchcore/proton/server/feedhandler.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/feedhandler.cpp @@ -20,6 +20,7 @@ #include <vespa/searchcorespi/index/ithreadingservice.h> #include <vespa/vespalib/util/destructor_callbacks.h> #include <vespa/searchlib/transactionlog/client_session.h> +#include <vespa/vespalib/util/atomic.h> #include <vespa/vespalib/util/exceptions.h> #include <vespa/vespalib/util/lambdatask.h> #include <cassert> @@ -42,6 +43,7 @@ using vespalib::make_string; using std::make_unique; using std::make_shared; using search::CommitParam; +using namespace vespalib::atomic; namespace proton { @@ -305,13 +307,13 @@ void FeedHandler::performEof() { assert(_writeService.master().isCurrentThread()); - _activeFeedView->forceCommitAndWait(CommitParam(_serialNum)); + _activeFeedView->forceCommitAndWait(CommitParam(load_relaxed(_serialNum))); LOG(debug, "Visiting done for transaction log domain '%s', eof received", _tlsMgr.getDomainName().c_str()); // Replay must be complete - if (_replay_end_serial_num != _serialNum) { + if (_replay_end_serial_num != load_relaxed(_serialNum)) { LOG(warning, "Expected replay end serial number %" PRIu64 ", got serial number %" PRIu64, - _replay_end_serial_num, _serialNum); - assert(_replay_end_serial_num == _serialNum); + _replay_end_serial_num, load_relaxed(_serialNum)); + assert(_replay_end_serial_num == load_relaxed(_serialNum)); } _owner.onTransactionLogReplayDone(); _tlsMgr.replayDone(); @@ -444,7 +446,7 @@ void FeedHandler::init(SerialNum oldestConfigSerial) { _tlsMgr.init(oldestConfigSerial, _prunedSerialNum, _replay_end_serial_num); - _serialNum = _prunedSerialNum; + store_relaxed(_serialNum, _prunedSerialNum); if (_tlsWriter == nullptr) { _tlsMgrWriter = std::make_unique<TlsMgrWriter>(_tlsMgr, _tlsWriterfactory); _tlsWriter = _tlsMgrWriter.get(); @@ -458,7 +460,7 @@ void FeedHandler::close() { if (_allowSync) { - syncTls(_serialNum); + syncTls(load_relaxed(_serialNum)); } _allowSync = false; _tlsMgr.close(); @@ -484,8 +486,8 @@ FeedHandler::replayTransactionLog(SerialNum flushedIndexMgrSerial, SerialNum flu TransactionLogManager::prepareReplay(_tlsMgr.getClient(), _docTypeName.getName(), flushedIndexMgrSerial, flushedSummaryMgrSerial, config_store); - _tlsReplayProgress = _tlsMgr.make_replay_progress(_serialNum, _replay_end_serial_num); - _tlsMgr.startReplay(_serialNum, _replay_end_serial_num, *this); + _tlsReplayProgress = _tlsMgr.make_replay_progress(load_relaxed(_serialNum), _replay_end_serial_num); + _tlsMgr.startReplay(load_relaxed(_serialNum), _replay_end_serial_num, *this); } void @@ -549,7 +551,7 @@ FeedHandler::initiateCommit(vespalib::steady_time start_time) { if (_activeFeedView) { using KeepAlivePair = vespalib::KeepAlive<std::pair<CommitResult, DoneCallback>>; auto pair = std::make_pair(std::move(commitResult), std::move(onCommitDoneContext)); - _activeFeedView->forceCommit(CommitParam(_serialNum, CommitParam::UpdateStats::SKIP), std::make_shared<KeepAlivePair>(std::move(pair))); + _activeFeedView->forceCommit(CommitParam(load_relaxed(_serialNum), CommitParam::UpdateStats::SKIP), std::make_shared<KeepAlivePair>(std::move(pair))); } } @@ -773,7 +775,7 @@ FeedHandler::heartBeat() { assert(_writeService.master().isCurrentThread()); _heart_beat_time.store(vespalib::steady_clock::now()); - _activeFeedView->heartBeat(_serialNum, vespalib::IDestructorCallback::SP()); + _activeFeedView->heartBeat(load_relaxed(_serialNum), vespalib::IDestructorCallback::SP()); } FeedHandler::RPC::Result diff --git a/searchcore/src/vespa/searchcore/proton/server/feedhandler.h b/searchcore/src/vespa/searchcore/proton/server/feedhandler.h index 32a70f7c2b0..4e9c016af9b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/feedhandler.h +++ b/searchcore/src/vespa/searchcore/proton/server/feedhandler.h @@ -78,7 +78,7 @@ private: TlsWriter *_tlsWriter; TlsReplayProgress::UP _tlsReplayProgress; // the serial num of the last feed operation processed by feed handler. - SerialNum _serialNum; + std::atomic<SerialNum> _serialNum; // the serial num considered to be fully procssessed and flushed to stable storage. Used to prune transaction log. SerialNum _prunedSerialNum; // the serial num of the last feed operation in the transaction log at startup before replay @@ -215,9 +215,15 @@ public: _bucketDBHandler = bucketDBHandler; } - void setSerialNum(SerialNum serialNum) { _serialNum = serialNum; } - SerialNum inc_serial_num() override { return ++_serialNum; } - SerialNum getSerialNum() const override { return _serialNum; } + // Must only be called from writer thread: + void setSerialNum(SerialNum serialNum) { _serialNum.store(serialNum, std::memory_order_relaxed); } + SerialNum inc_serial_num() override { + const auto post_inc = _serialNum.load(std::memory_order_relaxed) + 1u; + _serialNum.store(post_inc, std::memory_order_relaxed); + return post_inc; + } + // May be called from non-writer threads: + SerialNum getSerialNum() const override { return _serialNum.load(std::memory_order_relaxed); } // The two following methods are used when saving initial config SerialNum get_replay_end_serial_num() const { return _replay_end_serial_num; } SerialNum inc_replay_end_serial_num() { return ++_replay_end_serial_num; } diff --git a/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.cpp b/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.cpp index e6e7d777656..bf437dd7ee3 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.cpp +++ b/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.cpp @@ -242,7 +242,7 @@ WarmupIndexCollection::WarmupTask::WarmupTask(std::unique_ptr<MatchData> md, std _retainGuard(_warmup->_pendingTasks), _matchData(std::move(md)), _bluePrint(), - _requestContext(warmup->_clock) + _requestContext(_warmup->_clock) { } diff --git a/searchlib/src/vespa/searchlib/attribute/atomic_utils.h b/searchlib/src/vespa/searchlib/attribute/atomic_utils.h new file mode 100644 index 00000000000..48914de8942 --- /dev/null +++ b/searchlib/src/vespa/searchlib/attribute/atomic_utils.h @@ -0,0 +1,34 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +namespace vespalib::datastore { + +class AtomicEntryRef; +class EntryRef; + +} + +namespace search::attribute::atomic_utils { + +/* + * Helper class to map from atomic value to non-atomic value, e.g. + * from AtomicEntryRef to EntryRef. + */ +template <typename MaybeAtomicValue> +class NonAtomicValue { +public: + using type = MaybeAtomicValue; +}; + +template <> +class NonAtomicValue<vespalib::datastore::AtomicEntryRef> +{ +public: + using type = vespalib::datastore::EntryRef; +}; + +template <class MaybeAtomicValue> +using NonAtomicValue_t = typename NonAtomicValue<MaybeAtomicValue>::type; + +} diff --git a/searchlib/src/vespa/searchlib/attribute/load_utils.cpp b/searchlib/src/vespa/searchlib/attribute/load_utils.cpp index 2c53003112a..707cdc6af76 100644 --- a/searchlib/src/vespa/searchlib/attribute/load_utils.cpp +++ b/searchlib/src/vespa/searchlib/attribute/load_utils.cpp @@ -85,12 +85,12 @@ template uint32_t loadFromEnumeratedMultiValue(MultiValueMapping<Value<ValueType #define INSTANTIATE_WSET(ValueType, Saver) \ template uint32_t loadFromEnumeratedMultiValue(MultiValueMapping<WeightedValue<ValueType>> &, ReaderBase &, vespalib::ConstArrayRef<ValueType>, vespalib::ConstArrayRef<uint32_t>, Saver) #define INSTANTIATE_SINGLE(ValueType, Saver) \ -template void loadFromEnumeratedSingleValue(vespalib::RcuVectorBase<ValueType> &, vespalib::GenerationHolder &, ReaderBase &, vespalib::ConstArrayRef<load_utils::NonAtomicValue_t<ValueType>>, vespalib::ConstArrayRef<uint32_t>, Saver) +template void loadFromEnumeratedSingleValue(vespalib::RcuVectorBase<ValueType> &, vespalib::GenerationHolder &, ReaderBase &, vespalib::ConstArrayRef<atomic_utils::NonAtomicValue_t<ValueType>>, vespalib::ConstArrayRef<uint32_t>, Saver) #define INSTANTIATE_SINGLE_ARRAY_WSET(ValueType, Saver) \ INSTANTIATE_SINGLE(ValueType, Saver); \ -INSTANTIATE_ARRAY(load_utils::NonAtomicValue_t<ValueType>, Saver); \ -INSTANTIATE_WSET(load_utils::NonAtomicValue_t<ValueType>, Saver) +INSTANTIATE_ARRAY(atomic_utils::NonAtomicValue_t<ValueType>, Saver); \ +INSTANTIATE_WSET(atomic_utils::NonAtomicValue_t<ValueType>, Saver) #define INSTANTIATE_ENUM(Saver) \ INSTANTIATE_SINGLE_ARRAY_WSET(AtomicEntryRef, Saver) diff --git a/searchlib/src/vespa/searchlib/attribute/load_utils.h b/searchlib/src/vespa/searchlib/attribute/load_utils.h index fe41811dcfa..f9f933f3726 100644 --- a/searchlib/src/vespa/searchlib/attribute/load_utils.h +++ b/searchlib/src/vespa/searchlib/attribute/load_utils.h @@ -2,6 +2,7 @@ #pragma once +#include "atomic_utils.h" #include "attributevector.h" #include "readerbase.h" #include <vespa/vespalib/util/arrayref.h> @@ -15,30 +16,6 @@ class EntryRef; namespace search::attribute { -namespace load_utils { - -/* - * Helper class to map from atomic value to non-atomic value, e.g. - * from AtomicEntryRef to EntryRef. - */ -template <typename MaybeAtomicValue> -class NonAtomicValue { -public: - using type = MaybeAtomicValue; -}; - -template <> -class NonAtomicValue<vespalib::datastore::AtomicEntryRef> -{ -public: - using type = vespalib::datastore::EntryRef; -}; - -template <class MaybeAtomicValue> -using NonAtomicValue_t = typename NonAtomicValue<MaybeAtomicValue>::type; - -} - /** * Helper functions used to open / load attribute vector data files from disk. */ @@ -82,7 +59,7 @@ void loadFromEnumeratedSingleValue(Vector &vector, vespalib::GenerationHolder &genHolder, ReaderBase &attrReader, - vespalib::ConstArrayRef<load_utils::NonAtomicValue_t<typename Vector::ValueType>> enumValueToValueMap, + vespalib::ConstArrayRef<atomic_utils::NonAtomicValue_t<typename Vector::ValueType>> enumValueToValueMap, vespalib::ConstArrayRef<uint32_t> enum_value_remapping, Saver saver) __attribute((noinline)); diff --git a/searchlib/src/vespa/searchlib/attribute/load_utils.hpp b/searchlib/src/vespa/searchlib/attribute/load_utils.hpp index 92cbc72ae2c..74126299919 100644 --- a/searchlib/src/vespa/searchlib/attribute/load_utils.hpp +++ b/searchlib/src/vespa/searchlib/attribute/load_utils.hpp @@ -54,12 +54,12 @@ void loadFromEnumeratedSingleValue(Vector &vector, vespalib::GenerationHolder &genHolder, ReaderBase &attrReader, - vespalib::ConstArrayRef<load_utils::NonAtomicValue_t<typename Vector::ValueType>> enumValueToValueMap, + vespalib::ConstArrayRef<atomic_utils::NonAtomicValue_t<typename Vector::ValueType>> enumValueToValueMap, vespalib::ConstArrayRef<uint32_t> enum_value_remapping, Saver saver) { using ValueType = typename Vector::ValueType; - using NonAtomicValueType = load_utils::NonAtomicValue_t<ValueType>; + using NonAtomicValueType = atomic_utils::NonAtomicValue_t<ValueType>; uint32_t numDocs = attrReader.getEnumCount(); genHolder.clearHoldLists(); vector.reset(); diff --git a/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java b/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java index 2c93312eb6d..f035e2c6f00 100644 --- a/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java +++ b/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java @@ -8,6 +8,7 @@ import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -99,7 +100,7 @@ public abstract class AbstractFilteringList<Type, ListType extends AbstractFilte /** Returns the items grouped by the given classifier. */ public final <OtherType> Map<OtherType, ListType> groupingBy(Function<Type, OtherType> classifier) { return items.stream().collect(Collectors.groupingBy(classifier, - HashMap::new, + LinkedHashMap::new, Collectors.collectingAndThen(toUnmodifiableList(), (list) -> constructor.apply(list, false)))); } diff --git a/vespalib/src/tests/btree/btree-stress/btree_stress_test.cpp b/vespalib/src/tests/btree/btree-stress/btree_stress_test.cpp index 7d11990577d..4716e91c2c4 100644 --- a/vespalib/src/tests/btree/btree-stress/btree_stress_test.cpp +++ b/vespalib/src/tests/btree/btree-stress/btree_stress_test.cpp @@ -196,9 +196,11 @@ protected: using MyTree = typename Params::MyTree; using MyTreeIterator = typename MyTree::Iterator; using MyTreeConstIterator = typename MyTree::ConstIterator; + using KeyStore = IntStore; + using ValueStore = IntStore; GenerationHandler _generationHandler; - IntStore _keys; - IntStore _values; + KeyStore _keys; + ValueStore _values; MyTree _tree; MyTreeIterator _writeItr; vespalib::ThreadStackExecutor _writer; // 1 write thread @@ -344,7 +346,7 @@ template <typename Params> void Fixture<Params>::compact_keys() { - if constexpr (_keys.is_indirect) { + if constexpr (KeyStore::is_indirect) { auto to_hold = _keys.start_compact(); EntryRefFilter filter(_keys.get_num_buffers(), _keys.get_offset_bits()); filter.add_buffers(to_hold); @@ -366,7 +368,7 @@ template <typename Params> void Fixture<Params>::compact_values() { - if constexpr (_values.is_indirect) { + if constexpr (ValueStore::is_indirect) { auto to_hold = _values.start_compact(); EntryRefFilter filter(_values.get_num_buffers(), _values.get_offset_bits()); filter.add_buffers(to_hold); @@ -391,12 +393,12 @@ Fixture<Params>::consider_compact(uint32_t idx) if (_compact_tree.consider(idx) && !_tree.getAllocator().getNodeStore().has_held_buffers()) { compact_tree(); } - if constexpr (_keys.is_indirect) { + if constexpr (KeyStore::is_indirect) { if (_compact_keys.consider(idx) && !_keys.has_held_buffers()) { compact_keys(); } } - if constexpr (_values.is_indirect) { + if constexpr (ValueStore::is_indirect) { if (_compact_values.consider(idx) && !_values.has_held_buffers()) { compact_values(); } |