diff options
128 files changed, 2605 insertions, 1140 deletions
diff --git a/abi-check-plugin/src/main/java/com/yahoo/abicheck/mojo/AbiCheck.java b/abi-check-plugin/src/main/java/com/yahoo/abicheck/mojo/AbiCheck.java index 06d2628228c..3ae1102014d 100644 --- a/abi-check-plugin/src/main/java/com/yahoo/abicheck/mojo/AbiCheck.java +++ b/abi-check-plugin/src/main/java/com/yahoo/abicheck/mojo/AbiCheck.java @@ -10,6 +10,7 @@ import com.yahoo.abicheck.collector.AnnotationCollector; import com.yahoo.abicheck.collector.PublicSignatureCollector; import com.yahoo.abicheck.setmatcher.SetMatcher; import com.yahoo.abicheck.signature.JavaClassSignature; +import java.io.File; import java.io.FileReader; import java.io.FileWriter; import java.io.IOException; @@ -52,8 +53,8 @@ public class AbiCheck extends AbstractMojo { // CLOVER:OFF // Testing that Gson can read JSON files is not very useful - private static Map<String, JavaClassSignature> readSpec(String fileName) throws IOException { - try (FileReader reader = new FileReader(fileName)) { + private static Map<String, JavaClassSignature> readSpec(File file) throws IOException { + try (FileReader reader = new FileReader(file)) { TypeToken<Map<String, JavaClassSignature>> typeToken = new TypeToken<Map<String, JavaClassSignature>>() { }; @@ -65,10 +66,10 @@ public class AbiCheck extends AbstractMojo { // CLOVER:OFF // Testing that Gson can write JSON files is not very useful - private static void writeSpec(Map<String, JavaClassSignature> signatures, String fileName) + private static void writeSpec(Map<String, JavaClassSignature> signatures, File file) throws IOException { Gson gson = new GsonBuilder().setPrettyPrinting().create(); - try (FileWriter writer = new FileWriter(fileName)) { + try (FileWriter writer = new FileWriter(file)) { gson.toJson(signatures, writer); } } @@ -157,6 +158,7 @@ public class AbiCheck extends AbstractMojo { @Override public void execute() throws MojoExecutionException, MojoFailureException { Artifact mainArtifact = project.getArtifact(); + File specFile = new File(project.getBasedir(), specFileName); if (mainArtifact.getFile() == null) { throw new MojoExecutionException("Missing project artifact file"); } else if (!mainArtifact.getType().equals("jar")) { @@ -165,6 +167,7 @@ public class AbiCheck extends AbstractMojo { getLog().debug("Analyzing " + mainArtifact.getFile()); + try (JarFile jarFile = new JarFile(mainArtifact.getFile())) { ClassFileTree tree = ClassFileTree.fromJar(jarFile); Map<String, JavaClassSignature> signatures = new LinkedHashMap<>(); @@ -172,10 +175,10 @@ public class AbiCheck extends AbstractMojo { signatures.putAll(collectPublicAbiSignatures(pkg, publicApiAnnotation)); } if (System.getProperty(WRITE_SPEC_PROPERTY) != null) { - getLog().info("Writing ABI specs to " + specFileName); - writeSpec(signatures, specFileName); + getLog().info("Writing ABI specs to " + specFile.getPath()); + writeSpec(signatures, specFile); } else { - Map<String, JavaClassSignature> abiSpec = readSpec(specFileName); + Map<String, JavaClassSignature> abiSpec = readSpec(specFile); if (!compareSignatures(abiSpec, signatures, getLog())) { throw new MojoFailureException("ABI spec mismatch"); } diff --git a/config-model/src/main/java/com/yahoo/documentmodel/DataTypeRepo.java b/config-model/src/main/java/com/yahoo/documentmodel/DataTypeRepo.java index cb4b3f6f532..7e5f393c3e0 100644 --- a/config-model/src/main/java/com/yahoo/documentmodel/DataTypeRepo.java +++ b/config-model/src/main/java/com/yahoo/documentmodel/DataTypeRepo.java @@ -12,8 +12,8 @@ import java.util.Map; */ public class DataTypeRepo implements DataTypeCollection { - Map<Integer, DataType> typeById = new LinkedHashMap<>(); - Map<String, DataType> typeByName = new LinkedHashMap<>(); + private Map<Integer, DataType> typeById = new LinkedHashMap<>(); + private Map<String, DataType> typeByName = new LinkedHashMap<>(); public DataType getDataType(String name) { return typeByName.get(name); diff --git a/config-model/src/main/java/com/yahoo/documentmodel/DocumentTypeCollection.java b/config-model/src/main/java/com/yahoo/documentmodel/DocumentTypeCollection.java index 3e711a48109..debb22ece9e 100644 --- a/config-model/src/main/java/com/yahoo/documentmodel/DocumentTypeCollection.java +++ b/config-model/src/main/java/com/yahoo/documentmodel/DocumentTypeCollection.java @@ -7,7 +7,9 @@ import java.util.Collection; * @author baldersheim */ public interface DocumentTypeCollection { - public NewDocumentType getDocumentType(NewDocumentType.Name name); - public NewDocumentType getDocumentType(int id); - public Collection<NewDocumentType> getTypes(); + + NewDocumentType getDocumentType(NewDocumentType.Name name); + NewDocumentType getDocumentType(int id); + Collection<NewDocumentType> getTypes(); + } diff --git a/config-model/src/main/java/com/yahoo/documentmodel/DocumentTypeRepo.java b/config-model/src/main/java/com/yahoo/documentmodel/DocumentTypeRepo.java index 5138caf0b28..7ab8d0f1d8d 100644 --- a/config-model/src/main/java/com/yahoo/documentmodel/DocumentTypeRepo.java +++ b/config-model/src/main/java/com/yahoo/documentmodel/DocumentTypeRepo.java @@ -10,8 +10,8 @@ import java.util.Map; */ public class DocumentTypeRepo implements DocumentTypeCollection { - final Map<Integer, NewDocumentType> typeById = new LinkedHashMap<>(); - final Map<NewDocumentType.Name, NewDocumentType> typeByName = new LinkedHashMap<>(); + private final Map<Integer, NewDocumentType> typeById = new LinkedHashMap<>(); + private final Map<NewDocumentType.Name, NewDocumentType> typeByName = new LinkedHashMap<>(); public final NewDocumentType getDocumentType(String name) { return typeByName.get(new NewDocumentType.Name(name)); diff --git a/config-model/src/main/java/com/yahoo/documentmodel/NewDocumentType.java b/config-model/src/main/java/com/yahoo/documentmodel/NewDocumentType.java index 06bb32213f9..fc42864f1d0 100644 --- a/config-model/src/main/java/com/yahoo/documentmodel/NewDocumentType.java +++ b/config-model/src/main/java/com/yahoo/documentmodel/NewDocumentType.java @@ -32,7 +32,6 @@ import static java.util.Collections.emptySet; */ public final class NewDocumentType extends StructuredDataType implements DataTypeCollection { - public static final class Name { // TODO: privatize @@ -381,10 +380,7 @@ public final class NewDocumentType extends StructuredDataType implements DataTyp return this; } - /** - * The field sets defined for this type and its {@link Search} - * @return fieldsets - */ + /** The field sets defined for this type and its {@link Search} */ public Set<FieldSet> getFieldSets() { return Collections.unmodifiableSet(fieldSets); } diff --git a/config-model/src/main/java/com/yahoo/documentmodel/VespaDocumentType.java b/config-model/src/main/java/com/yahoo/documentmodel/VespaDocumentType.java index 2f2c308e633..69fe6f74d27 100644 --- a/config-model/src/main/java/com/yahoo/documentmodel/VespaDocumentType.java +++ b/config-model/src/main/java/com/yahoo/documentmodel/VespaDocumentType.java @@ -6,7 +6,7 @@ import com.yahoo.document.DataTypeName; import com.yahoo.document.PositionDataType; /** - * This class represents the builtin 'doument' document type that all other documenttypes inherits. + * This class represents the builtin 'document' document type that all other documenttypes inherits. * Remember that changes here must be compatible. Changes to types of fields can not be done here. * This must also match the mirroring class in c++. * @@ -32,6 +32,8 @@ public class VespaDocumentType { vespa.add(PositionDataType.INSTANCE); vespa.add(DataType.URI); vespa.add(DataType.PREDICATE); + vespa.add(DataType.BOOL); + vespa.add(DataType.FLOAT16); return vespa; } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/SummaryClassField.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/SummaryClassField.java index bdfebfd0546..4375b446e98 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/SummaryClassField.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/SummaryClassField.java @@ -22,10 +22,12 @@ public class SummaryClassField { /** The summary field type enumeration */ public enum Type { + BOOL("bool"), BYTE("byte"), SHORT("short"), INTEGER("integer"), INT64("int64"), + FLOAT16("float16"), FLOAT("float"), DOUBLE("double"), STRING("string"), @@ -77,10 +79,14 @@ public class SummaryClassField { return Type.INTEGER; } else if (fval instanceof LongFieldValue) { return Type.INT64; + } else if (fval instanceof Float16FieldValue) { + return Type.FLOAT16; } else if (fval instanceof FloatFieldValue) { return Type.FLOAT; } else if (fval instanceof DoubleFieldValue) { return Type.DOUBLE; + } else if (fval instanceof BoolFieldValue) { + return Type.BOOL; } else if (fval instanceof ByteFieldValue) { return Type.BYTE; } else if (fval instanceof Raw) { @@ -102,8 +108,7 @@ public class SummaryClassField { } else if (fieldType instanceof ReferenceDataType) { return Type.LONGSTRING; } else { - throw new IllegalArgumentException("Don't know which summary type to " + - "convert " + fieldType + " to"); + throw new IllegalArgumentException("Don't know which summary type to convert " + fieldType + " to"); } } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/VsmFields.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/VsmFields.java index ef85c0617bc..3f1474704fe 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/VsmFields.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/VsmFields.java @@ -1,8 +1,16 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchdefinition.derived; -import com.yahoo.document.*; -import com.yahoo.document.datatypes.*; +import com.yahoo.document.CollectionDataType; +import com.yahoo.document.DataType; +import com.yahoo.document.NumericDataType; +import com.yahoo.document.ReferenceDataType; +import com.yahoo.document.datatypes.BoolFieldValue; +import com.yahoo.document.datatypes.FieldValue; +import com.yahoo.document.datatypes.PredicateFieldValue; +import com.yahoo.document.datatypes.Raw; +import com.yahoo.document.datatypes.StringFieldValue; +import com.yahoo.document.datatypes.TensorFieldValue; import com.yahoo.searchdefinition.FieldSets; import com.yahoo.searchdefinition.Search; import com.yahoo.searchdefinition.document.FieldSet; @@ -114,14 +122,16 @@ public class VsmFields extends Derived implements VsmfieldsConfig.Producer { /** The streaming field type enumeration */ public static class Type { - public static Type INT8=new Type("int8","INT8"); - public static Type INT16=new Type("int16","INT16"); - public static Type INT32=new Type("int32","INT32"); - public static Type INT64=new Type("int64","INT64"); - public static Type FLOAT=new Type("float","FLOAT"); - public static Type DOUBLE=new Type("double","DOUBLE"); - public static Type STRING=new Type("string","AUTOUTF8"); - public static Type UNSEARCHABLESTRING=new Type("string","NONE"); + public static Type INT8 = new Type("int8","INT8"); + public static Type INT16 = new Type("int16","INT16"); + public static Type INT32 = new Type("int32","INT32"); + public static Type INT64 = new Type("int64","INT64"); + public static Type FLOAT16 = new Type("float16", "FLOAT16"); + public static Type FLOAT = new Type("float","FLOAT"); + public static Type DOUBLE = new Type("double","DOUBLE"); + public static Type STRING = new Type("string","AUTOUTF8"); + public static Type BOOL = new Type("bool","BOOL"); + public static Type UNSEARCHABLESTRING = new Type("string","NONE"); private String name; @@ -148,6 +158,7 @@ public class VsmFields extends Derived implements VsmfieldsConfig.Producer { return this.name.equals(((Type)other).name); } + @Override public String toString() { return "type: " + name; } @@ -168,18 +179,24 @@ public class VsmFields extends Derived implements VsmfieldsConfig.Producer { /** Converts to the right index type from a field datatype */ private static Type convertType(DataType fieldType) { FieldValue fval = fieldType.createFieldValue(); - if (fieldType.equals(DataType.FLOAT)) { + if (fieldType.equals(DataType.FLOAT16)) { + return Type.FLOAT16; + } else if (fieldType.equals(DataType.FLOAT)) { return Type.FLOAT; } else if (fieldType.equals(DataType.LONG)) { return Type.INT64; } else if (fieldType.equals(DataType.DOUBLE)) { return Type.DOUBLE; + } else if (fieldType.equals(DataType.BOOL)) { + return Type.BOOL; } else if (fieldType.equals(DataType.BYTE)) { return Type.INT8; } else if (fieldType instanceof NumericDataType) { return Type.INT32; } else if (fval instanceof StringFieldValue) { return Type.STRING; + } else if (fval instanceof BoolFieldValue) { + return Type.BOOL; } else if (fval instanceof Raw) { return Type.STRING; } else if (fval instanceof PredicateFieldValue) { @@ -191,8 +208,8 @@ public class VsmFields extends Derived implements VsmfieldsConfig.Producer { } else if (fieldType instanceof ReferenceDataType) { return Type.UNSEARCHABLESTRING; } else { - throw new IllegalArgumentException("Don't know which streaming" + - " field type to " + "convert " + fieldType + " to"); + throw new IllegalArgumentException("Don't know which streaming field type to convert " + + fieldType + " to"); } } @@ -245,6 +262,7 @@ public class VsmFields extends Derived implements VsmfieldsConfig.Producer { } private static class StreamingDocumentType { + private final String name; private final Map<String, FieldSet> fieldSets = new LinkedHashMap<>(); private final Map<String, FieldSet> userFieldSets; @@ -282,4 +300,5 @@ public class VsmFields extends Derived implements VsmfieldsConfig.Producer { fs.addFieldName(fieldName); } } + } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/document/Attribute.java b/config-model/src/main/java/com/yahoo/searchdefinition/document/Attribute.java index bdd027f4687..fbcaf2a3a80 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/document/Attribute.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/document/Attribute.java @@ -11,6 +11,7 @@ import com.yahoo.document.StructuredDataType; import com.yahoo.document.TemporaryStructuredDataType; import com.yahoo.document.TensorDataType; import com.yahoo.document.WeightedSetDataType; +import com.yahoo.document.datatypes.BoolFieldValue; import com.yahoo.document.datatypes.ByteFieldValue; import com.yahoo.document.datatypes.DoubleFieldValue; import com.yahoo.document.datatypes.FieldValue; @@ -19,6 +20,7 @@ import com.yahoo.document.datatypes.IntegerFieldValue; import com.yahoo.document.datatypes.LongFieldValue; import com.yahoo.document.datatypes.PredicateFieldValue; import com.yahoo.document.datatypes.Raw; +import com.yahoo.document.datatypes.Float16FieldValue; import com.yahoo.document.datatypes.StringFieldValue; import com.yahoo.document.datatypes.TensorFieldValue; import com.yahoo.tensor.TensorType; @@ -82,9 +84,11 @@ public final class Attribute implements Cloneable, Serializable { SHORT("short", "INT16"), INTEGER("integer", "INT32"), LONG("long", "INT64"), + FLOAT16("float16", "FLOAT16"), FLOAT("float", "FLOAT"), DOUBLE("double", "DOUBLE"), STRING("string", "STRING"), + BOOL("bool", "BOOL"), PREDICATE("predicate", "PREDICATE"), TENSOR("tensor", "TENSOR"), REFERENCE("reference", "REFERENCE"); @@ -235,6 +239,10 @@ public final class Attribute implements Cloneable, Serializable { return Type.FLOAT; } else if (fval instanceof DoubleFieldValue) { return Type.DOUBLE; + } else if (fval instanceof BoolFieldValue) { + return Type.BOOL; + } else if (fval instanceof Float16FieldValue) { + return Type.FLOAT16; } else if (fval instanceof ByteFieldValue) { return Type.BYTE; } else if (fval instanceof Raw) { @@ -288,8 +296,10 @@ public final class Attribute implements Cloneable, Serializable { case STRING : return DataType.STRING; case INTEGER: return DataType.INT; case LONG: return DataType.LONG; + case FLOAT16: return DataType.FLOAT16; case FLOAT: return DataType.FLOAT; case DOUBLE: return DataType.DOUBLE; + case BOOL: return DataType.BOOL; case BYTE: return DataType.BYTE; case PREDICATE: return DataType.PREDICATE; case TENSOR: return DataType.getTensor(tensorType.orElseThrow(IllegalStateException::new)); 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 d2d28dadfda..16e1e2e4e1d 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 @@ -146,14 +146,13 @@ public class SDField extends Field implements TypedKey, FieldOperationContainer, } /** - Creates a new field. - - @param name The name of the field - @param dataType The datatype of the field - @param isHeader Whether this is a "header" field or a "content" field - (true = "header"). - @param owner the owning document (used to check for id collisions) - */ + * Creates a new field. + * + * @param name The name of the field + * @param dataType The datatype of the field + * @param isHeader Whether this is a "header" field or a "content" field (true = "header"). + * @param owner the owning document (used to check for id collisions) + */ protected SDField(SDDocumentType repo, String name, DataType dataType, boolean isHeader, SDDocumentType owner, boolean populate) { super(name, dataType, isHeader, owner == null ? null : owner.getDocumentType()); this.ownerDocType=owner; @@ -161,30 +160,29 @@ public class SDField extends Field implements TypedKey, FieldOperationContainer, } /** - Creates a new field. - - @param name The name of the field - @param dataType The datatype of the field - @param isHeader Whether this is a "header" field or a "content" field - (true = "header"). - @param owner The owning document (used to check for id collisions) - @param fieldMatching The matching object to set for the field - */ - protected SDField(SDDocumentType repo, String name, DataType dataType, boolean isHeader, SDDocumentType owner, Matching fieldMatching, boolean populate, int recursion) { + * Creates a new field. + * + * @param name The name of the field + * @param dataType The datatype of the field + * @param isHeader Whether this is a "header" field or a "content" field (true = "header"). + * @param owner The owning document (used to check for id collisions) + * @param fieldMatching The matching object to set for the field + */ + protected SDField(SDDocumentType repo, String name, DataType dataType, boolean isHeader, SDDocumentType owner, + Matching fieldMatching, boolean populate, int recursion) { super(name, dataType, isHeader, owner == null ? null : owner.getDocumentType()); this.ownerDocType=owner; - if (fieldMatching != null) { + if (fieldMatching != null) this.setMatching(fieldMatching); - } populate(populate, repo, name, dataType, isHeader, fieldMatching, recursion); } /** - Constructor for <b>header</b> fields - - @param name The name of the field - @param dataType The datatype of the field - */ + * Constructor for <b>header</b> fields + * + * @param name The name of the field + * @param dataType The datatype of the field + */ public SDField(SDDocumentType repo, String name, DataType dataType) { this(repo, name,dataType,true, true); } @@ -759,6 +757,7 @@ public class SDField extends Field implements TypedKey, FieldOperationContainer, /** * A list of query commands + * * @return a list of strings with query commands. */ @Override @@ -783,19 +782,18 @@ public class SDField extends Field implements TypedKey, FieldOperationContainer, return ownerDocType; } - /** - * Two fields are equal if they have the same name - * No, they are not. - */ + @Override public boolean equals(Object other) { if ( ! (other instanceof SDField)) return false; return super.equals(other); } + @Override public int hashCode() { return getName().hashCode(); } + @Override public String toString() { return "field '" + getName() + "'"; } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/document/Sorting.java b/config-model/src/main/java/com/yahoo/searchdefinition/document/Sorting.java index dd65cc27625..7e80b967f17 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/document/Sorting.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/document/Sorting.java @@ -7,7 +7,7 @@ import java.io.Serializable; * A search-time document attribute sort specification(per-document in-memory value). * This belongs to the attribute or field(implicitt attribute). * - * @author baldersheim + * @author baldersheim */ public final class Sorting implements Cloneable, Serializable { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/SearchCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/search/SearchCluster.java index 34a2fc7ab0e..60b3cb6987c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/search/SearchCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/search/SearchCluster.java @@ -86,13 +86,13 @@ public abstract class SearchCluster extends AbstractSearchCluster SummaryConfig.Builder summaryConfigBuilder = new SummaryConfig.Builder(); summaryConfigProducer.getConfig(summaryConfigBuilder); - SummaryConfig summaryConfig = new SummaryConfig(summaryConfigBuilder); + SummaryConfig summaryConfig = summaryConfigBuilder.build(); SummarymapConfig summarymapConfig = null; if (summarymapConfigProducer != null) { SummarymapConfig.Builder summarymapConfigBuilder = new SummarymapConfig.Builder(); summarymapConfigProducer.getConfig(summarymapConfigBuilder); - summarymapConfig = new SummarymapConfig(summarymapConfigBuilder); + summarymapConfig = summarymapConfigBuilder.build(); } for (SummaryConfig.Classes sclass : summaryConfig.classes()) { diff --git a/config-model/src/main/javacc/SDParser.jj b/config-model/src/main/javacc/SDParser.jj index 31f81b76ea4..e50cbabeb9f 100644 --- a/config-model/src/main/javacc/SDParser.jj +++ b/config-model/src/main/javacc/SDParser.jj @@ -828,14 +828,13 @@ SDDocumentType structDefinition(Search search, SDDocumentType repo) : */ DataType dataType() : { - String typeName=null; + String typeName = null; boolean isArrayOldStyle = false; - DataType mapType=null; - DataType arrayType=null; - DataType wsetType=null; + DataType mapType = null; + DataType arrayType = null; + DataType wsetType = null; TensorType tensorType; TemporaryStructuredDataType referenceType; - } { ( LOOKAHEAD(<ARRAY> <LESSTHAN>) ( <ARRAY> <LESSTHAN> arrayType = dataType() <GREATERTHAN> { return DataType.getArray(arrayType); } ) @@ -849,10 +848,9 @@ DataType dataType() : { DataType type = VespaDocumentType.INSTANCE.getDataType(typeName); - //is type still null? if (type == null) { - //we are basically creating TemporaryStructDataType instances for ANYTHING here!! - //we must do this and clean them up later. + // we are basically creating TemporaryStructDataType instances for ANYTHING here!! + // we must do this and clean them up later. type = TemporaryStructuredDataType.create(typeName); } diff --git a/config-model/src/test/derived/types/attributes.cfg b/config-model/src/test/derived/types/attributes.cfg index cf3fcebfbcd..e6ffc37e871 100644 --- a/config-model/src/test/derived/types/attributes.cfg +++ b/config-model/src/test/derived/types/attributes.cfg @@ -40,6 +40,48 @@ attribute[].upperbound 9223372036854775807 attribute[].densepostinglistthreshold 0.4 attribute[].tensortype "" attribute[].imported false +attribute[].name "abool" +attribute[].datatype BOOL +attribute[].collectiontype SINGLE +attribute[].removeifzero false +attribute[].createifnonexistent false +attribute[].fastsearch false +attribute[].huge false +attribute[].ismutable false +attribute[].sortascending true +attribute[].sortfunction UCA +attribute[].sortstrength PRIMARY +attribute[].sortlocale "" +attribute[].enablebitvectors false +attribute[].enableonlybitvector false +attribute[].fastaccess false +attribute[].arity 8 +attribute[].lowerbound -9223372036854775808 +attribute[].upperbound 9223372036854775807 +attribute[].densepostinglistthreshold 0.4 +attribute[].tensortype "" +attribute[].imported false +attribute[].name "ashortfloat" +attribute[].datatype FLOAT16 +attribute[].collectiontype SINGLE +attribute[].removeifzero false +attribute[].createifnonexistent false +attribute[].fastsearch false +attribute[].huge false +attribute[].ismutable false +attribute[].sortascending true +attribute[].sortfunction UCA +attribute[].sortstrength PRIMARY +attribute[].sortlocale "" +attribute[].enablebitvectors false +attribute[].enableonlybitvector false +attribute[].fastaccess false +attribute[].arity 8 +attribute[].lowerbound -9223372036854775808 +attribute[].upperbound 9223372036854775807 +attribute[].densepostinglistthreshold 0.4 +attribute[].tensortype "" +attribute[].imported false attribute[].name "arrayfield" attribute[].datatype INT32 attribute[].collectiontype ARRAY diff --git a/config-model/src/test/derived/types/documentmanager.cfg b/config-model/src/test/derived/types/documentmanager.cfg index 647c26e1316..0644659cae7 100644 --- a/config-model/src/test/derived/types/documentmanager.cfg +++ b/config-model/src/test/derived/types/documentmanager.cfg @@ -131,6 +131,12 @@ datatype[].structtype[].field[].detailedtype "" datatype[].structtype[].field[].name "along" datatype[].structtype[].field[].datatype 4 datatype[].structtype[].field[].detailedtype "" +datatype[].structtype[].field[].name "abool" +datatype[].structtype[].field[].datatype 6 +datatype[].structtype[].field[].detailedtype "" +datatype[].structtype[].field[].name "ashortfloat" +datatype[].structtype[].field[].datatype 7 +datatype[].structtype[].field[].detailedtype "" datatype[].structtype[].field[].name "arrayfield" datatype[].structtype[].field[].datatype -1245117006 datatype[].structtype[].field[].detailedtype "" @@ -232,6 +238,7 @@ datatype[].documenttype[].inherits[].version 0 datatype[].documenttype[].headerstruct 1328581348 datatype[].documenttype[].bodystruct 348447225 datatype[].documenttype[].fieldsets{[document]}.fields[] "Folders" +datatype[].documenttype[].fieldsets{[document]}.fields[] "abool" datatype[].documenttype[].fieldsets{[document]}.fields[] "abyte" datatype[].documenttype[].fieldsets{[document]}.fields[] "album0" datatype[].documenttype[].fieldsets{[document]}.fields[] "album1" @@ -239,6 +246,7 @@ datatype[].documenttype[].fieldsets{[document]}.fields[] "along" datatype[].documenttype[].fieldsets{[document]}.fields[] "arrarr" datatype[].documenttype[].fieldsets{[document]}.fields[] "arrayfield" datatype[].documenttype[].fieldsets{[document]}.fields[] "arraymapfield" +datatype[].documenttype[].fieldsets{[document]}.fields[] "ashortfloat" datatype[].documenttype[].fieldsets{[document]}.fields[] "complexarray" datatype[].documenttype[].fieldsets{[document]}.fields[] "doublemapfield" datatype[].documenttype[].fieldsets{[document]}.fields[] "floatmapfield" diff --git a/config-model/src/test/derived/types/ilscripts.cfg b/config-model/src/test/derived/types/ilscripts.cfg index 92ca728cea7..3bcdd16e3d6 100644 --- a/config-model/src/test/derived/types/ilscripts.cfg +++ b/config-model/src/test/derived/types/ilscripts.cfg @@ -3,6 +3,8 @@ fieldmatchmaxlength 1000000 ilscript[].doctype "types" ilscript[].docfield[] "abyte" ilscript[].docfield[] "along" +ilscript[].docfield[] "abool" +ilscript[].docfield[] "ashortfloat" ilscript[].docfield[] "arrayfield" ilscript[].docfield[] "setfield" ilscript[].docfield[] "setfield2" @@ -30,6 +32,8 @@ ilscript[].docfield[] "complexarray" ilscript[].content[] "clear_state | guard { input along | attribute other; }" ilscript[].content[] "clear_state | guard { input abyte | summary abyte | attribute abyte; }" ilscript[].content[] "clear_state | guard { input along | summary along | attribute along; }" +ilscript[].content[] "clear_state | guard { input abool | summary abool | attribute abool; }" +ilscript[].content[] "clear_state | guard { input ashortfloat | summary ashortfloat | attribute ashortfloat; }" ilscript[].content[] "clear_state | guard { input arrayfield | attribute arrayfield; }" ilscript[].content[] "clear_state | guard { input setfield | attribute setfield; }" ilscript[].content[] "clear_state | guard { input setfield2 | attribute setfield2; }" diff --git a/config-model/src/test/derived/types/index-info.cfg b/config-model/src/test/derived/types/index-info.cfg index 32375dc09f4..576a95de06f 100644 --- a/config-model/src/test/derived/types/index-info.cfg +++ b/config-model/src/test/derived/types/index-info.cfg @@ -15,6 +15,18 @@ indexinfo[].command[].indexname "along" indexinfo[].command[].command "attribute" indexinfo[].command[].indexname "along" indexinfo[].command[].command "numerical" +indexinfo[].command[].indexname "abool" +indexinfo[].command[].command "index" +indexinfo[].command[].indexname "abool" +indexinfo[].command[].command "attribute" +indexinfo[].command[].indexname "abool" +indexinfo[].command[].command "word" +indexinfo[].command[].indexname "ashortfloat" +indexinfo[].command[].command "index" +indexinfo[].command[].indexname "ashortfloat" +indexinfo[].command[].command "attribute" +indexinfo[].command[].indexname "ashortfloat" +indexinfo[].command[].command "numerical" indexinfo[].command[].indexname "arrayfield" indexinfo[].command[].command "index" indexinfo[].command[].indexname "arrayfield" diff --git a/config-model/src/test/derived/types/summary.cfg b/config-model/src/test/derived/types/summary.cfg index 3a73185b325..e5485a24c8c 100644 --- a/config-model/src/test/derived/types/summary.cfg +++ b/config-model/src/test/derived/types/summary.cfg @@ -1,10 +1,14 @@ -defaultsummaryid 1103008471 -classes[].id 1103008471 +defaultsummaryid 1131946680 +classes[].id 1131946680 classes[].name "default" classes[].fields[].name "abyte" classes[].fields[].type "byte" classes[].fields[].name "along" classes[].fields[].type "int64" +classes[].fields[].name "abool" +classes[].fields[].type "bool" +classes[].fields[].name "ashortfloat" +classes[].fields[].type "float16" classes[].fields[].name "tagfield" classes[].fields[].type "jsonstring" classes[].fields[].name "stringmapfield" @@ -19,7 +23,7 @@ classes[].fields[].name "summaryfeatures" classes[].fields[].type "featuredata" classes[].fields[].name "documentid" classes[].fields[].type "longstring" -classes[].id 278794929 +classes[].id 1027812395 classes[].name "attributeprefetch" classes[].fields[].name "other" classes[].fields[].type "int64" @@ -27,6 +31,10 @@ classes[].fields[].name "abyte" classes[].fields[].type "byte" classes[].fields[].name "along" classes[].fields[].type "int64" +classes[].fields[].name "abool" +classes[].fields[].type "bool" +classes[].fields[].name "ashortfloat" +classes[].fields[].type "float16" classes[].fields[].name "juletre" classes[].fields[].type "int64" classes[].fields[].name "rankfeatures" diff --git a/config-model/src/test/derived/types/summarymap.cfg b/config-model/src/test/derived/types/summarymap.cfg index 0cb8b6129fa..b87200f6573 100644 --- a/config-model/src/test/derived/types/summarymap.cfg +++ b/config-model/src/test/derived/types/summarymap.cfg @@ -5,6 +5,12 @@ override[].arguments "abyte" override[].field "along" override[].command "attribute" override[].arguments "along" +override[].field "abool" +override[].command "attribute" +override[].arguments "abool" +override[].field "ashortfloat" +override[].command "attribute" +override[].arguments "ashortfloat" override[].field "tagfield" override[].command "attribute" override[].arguments "tagfield" diff --git a/config-model/src/test/derived/types/types.sd b/config-model/src/test/derived/types/types.sd index c908b648340..839cb08dbd6 100644 --- a/config-model/src/test/derived/types/types.sd +++ b/config-model/src/test/derived/types/types.sd @@ -11,6 +11,14 @@ search types { indexing: index | summary | attribute } + field abool type bool { + indexing: summary | attribute + } + + field ashortfloat type float16 { + indexing: summary | attribute + } + field arrayfield type array<int> { indexing: attribute } diff --git a/config-model/src/test/derived/types/vsmsummary.cfg b/config-model/src/test/derived/types/vsmsummary.cfg index b1a29b94491..5ee589ccf9f 100644 --- a/config-model/src/test/derived/types/vsmsummary.cfg +++ b/config-model/src/test/derived/types/vsmsummary.cfg @@ -5,6 +5,12 @@ fieldmap[].command NONE fieldmap[].summary "along" fieldmap[].document[].field "along" fieldmap[].command NONE +fieldmap[].summary "abool" +fieldmap[].document[].field "abool" +fieldmap[].command NONE +fieldmap[].summary "ashortfloat" +fieldmap[].document[].field "ashortfloat" +fieldmap[].command NONE fieldmap[].summary "tagfield" fieldmap[].document[].field "tagfield" fieldmap[].command NONE diff --git a/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadApplier.java b/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadApplier.java index 3efa7fdd879..d406dfe2d77 100644 --- a/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadApplier.java +++ b/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadApplier.java @@ -94,7 +94,9 @@ public class ConfigPayloadApplier<T extends ConfigInstance.Builder> { trace("top of stack=" + stack.peek().toString()); String name = stack.peek().nameStack().peek(); if (inspector.type().equals(Type.OBJECT)) { - stack.push(createBuilder(stack.peek(), name)); + NamedBuilder builder = createBuilder(stack.peek(), name); + if (builder == null) return; // Ignore non-existent struct array class + stack.push(builder); } handleValue(inspector); if (inspector.type().equals(Type.OBJECT)) { @@ -143,7 +145,9 @@ public class ConfigPayloadApplier<T extends ConfigInstance.Builder> { parentBuilder.nameStack().pop(); return; } else { - stack.push(createBuilder(parentBuilder, name)); + NamedBuilder builder = createBuilder(parentBuilder, name); + if (builder == null) return; // Ignore non-existent struct class + stack.push(builder); } } else if (inspector.type().equals(Type.ARRAY)) { for (int i = 0; i < inspector.children(); i++) { @@ -179,6 +183,8 @@ public class ConfigPayloadApplier<T extends ConfigInstance.Builder> { private void handleInnerMap(String name, Inspector inspector) { NamedBuilder builder = createBuilder(stack.peek(), stack.peek().peekName()); + if (builder == null) + throw new RuntimeException("Missing map builder (this should never happen): " + stack.peek()); setMapLeafValue(name, builder.builder()); stack.push(builder); inspector.traverse(new ObjectTraverser() { @@ -222,7 +228,8 @@ public class ConfigPayloadApplier<T extends ConfigInstance.Builder> { NamedBuilder createBuilder(NamedBuilder parentBuilder, String name) { Object builder = parentBuilder.builder(); - Object newBuilder = getBuilderForStruct(findBuilderName(name), name, builder.getClass().getDeclaringClass()); + Object newBuilder = getBuilderForStruct(name, builder.getClass().getDeclaringClass()); + if (newBuilder == null) return null; trace("New builder for " + name + "=" + newBuilder); trace("Pushing builder for " + name + "=" + newBuilder + " onto stack"); return new NamedBuilder((ConfigBuilder) newBuilder, name); @@ -366,38 +373,51 @@ public class ConfigPayloadApplier<T extends ConfigInstance.Builder> { } - private String findBuilderName(String name) { + private String capitalize(String name) { StringBuilder sb = new StringBuilder(); sb.append(name.substring(0, 1).toUpperCase()).append(name.substring(1)); return sb.toString(); } - private Constructor<?> lookupBuilderForStruct(String builderName, String name, Class<?> currentClass) { + private Constructor<?> lookupBuilderForStruct(String structName, String name, Class<?> currentClass) { final String currentClassName = currentClass.getName(); - trace("builderName=" + builderName + ", name=" + name + ",current class=" + currentClassName); - Class<?> structClass = findClass(currentClass, currentClassName + "$" + builderName); - Class<?> structBuilderClass = findClass(structClass, currentClassName + "$" + builderName + "$Builder"); + trace("structName=" + structName + ", name=" + name + ",current class=" + currentClassName); + Class<?> structClass = getInnerClass(currentClass, currentClassName + "$" + structName); + if (structClass == null) { + log.info("Could not find nested class '" + currentClassName + "$" + structName + + "'. Ignoring it, assuming it's been added to a newer version of the config."); + return null; + } + return getStructBuilderConstructor(structClass, currentClassName, structName); + } + + private Constructor<?> getStructBuilderConstructor(Class<?> structClass, String currentClassName, String builderName) { + String structBuilderName = currentClassName + "$" + builderName + "$Builder"; + Class<?> structBuilderClass = getInnerClass(structClass, structBuilderName); + if (structBuilderClass == null) + throw new RuntimeException("Could not find builder class " + structBuilderName); try { return structBuilderClass.getDeclaredConstructor(new Class<?>[]{}); } catch (NoSuchMethodException e) { throw new RuntimeException("Could not create class '" + "'" + structBuilderClass.getName() + "'"); } + } /** - * Finds a nested class or builder class with the given <code>name</code>name in <code>clazz</code> + * Finds a nested class with the given <code>name</code>name in <code>clazz</code> * @param clazz a Class * @param name a name - * @return class found, or throws an exception is no class is found + * @return class found, or null if no class is found */ - private Class<?> findClass(Class<?> clazz, String name) { + private Class<?> getInnerClass(Class<?> clazz, String name) { for (Class<?> cls : clazz.getDeclaredClasses()) { if (cls.getName().equals(name)) { trace("Found class " + cls.getName()); return cls; } } - throw new RuntimeException("could not find class representing '" + printCurrentConfigName() + "'"); + return null; } private final Map<String, Constructor<?>> constructorCache = new HashMap<>(); @@ -405,11 +425,13 @@ public class ConfigPayloadApplier<T extends ConfigInstance.Builder> { return builderName + "." + name + "." + currentClass.getName(); } - private Object getBuilderForStruct(String builderName, String name, Class<?> currentClass) { - String key = constructorCacheKey(builderName, name, currentClass); + private Object getBuilderForStruct(String name, Class<?> currentClass) { + String structName = capitalize(name); + String key = constructorCacheKey(structName, name, currentClass); Constructor<?> ctor = constructorCache.get(key); if (ctor == null) { - ctor = lookupBuilderForStruct(builderName, name, currentClass); + ctor = lookupBuilderForStruct(structName, name, currentClass); + if (ctor == null) return null; constructorCache.put(key, ctor); } Object builder; @@ -421,22 +443,6 @@ public class ConfigPayloadApplier<T extends ConfigInstance.Builder> { return builder; } - private String printCurrentConfigName() { - StringBuilder sb = new StringBuilder(); - ArrayList<String> stackElements = new ArrayList<>(); - Stack<String> nameStack = stack.peek().nameStack(); - while (!nameStack.empty()) { - stackElements.add(nameStack.pop()); - } - Collections.reverse(stackElements); - for (String s : stackElements) { - sb.append(s); - sb.append("."); - } - sb.deleteCharAt(sb.length() - 1); // remove last . - return sb.toString(); - } - private void debug(String message) { if (log.isLoggable(LogLevel.DEBUG)) { log.log(LogLevel.DEBUG, message); diff --git a/config/src/test/java/com/yahoo/vespa/config/ConfigPayloadTest.java b/config/src/test/java/com/yahoo/vespa/config/ConfigPayloadTest.java index a21a08f88d6..63a55d20edf 100644 --- a/config/src/test/java/com/yahoo/vespa/config/ConfigPayloadTest.java +++ b/config/src/test/java/com/yahoo/vespa/config/ConfigPayloadTest.java @@ -1,9 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config; -import com.yahoo.foo.*; import com.yahoo.config.codegen.DefParser; import com.yahoo.config.codegen.InnerCNode; +import com.yahoo.foo.AppConfig; +import com.yahoo.foo.ArraytypesConfig; +import com.yahoo.foo.IntConfig; +import com.yahoo.foo.MaptypesConfig; +import com.yahoo.foo.SimpletypesConfig; +import com.yahoo.foo.StructtypesConfig; import com.yahoo.slime.Cursor; import com.yahoo.slime.Slime; import com.yahoo.text.StringUtilities; @@ -13,7 +18,11 @@ import java.io.IOException; import java.io.StringReader; import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; /** * @author Ulf Lilleengen 3 @@ -253,14 +262,16 @@ public class ConfigPayloadTest { @Test public void test_simple_struct() throws Exception { - StructtypesConfig config = createStructtypesConfigSimple("foobar", "MALE", new String[] { "foo@bar", "bar@foo" }); + Slime slime = new Slime(); + addStructFields(slime.setObject().setObject("simple"), "foobar", "MALE", new String[] { "foo@bar", "bar@foo" }); + StructtypesConfig config = new ConfigPayload(slime).toInstance(StructtypesConfig.class, ""); + assertThat(config.simple().name(), is("foobar")); assertThat(config.simple().gender(), is(StructtypesConfig.Simple.Gender.Enum.MALE)); assertThat(config.simple().emails(0), is("foo@bar")); assertThat(config.simple().emails(1), is("bar@foo")); } - @Test public void test_simple_struct_arrays() throws Exception { StructtypesConfig config = createStructtypesConfigArray(new String[] { "foo", "bar" }, @@ -325,13 +336,121 @@ public class ConfigPayloadTest { } @Test - public void test_function_test() { - // TODO: Test function test config as a complete config example + public void test_simple_map() { + Slime slime = new Slime(); + Cursor map = slime.setObject().setObject("stringmap"); + map.setString("key","val"); + + MaptypesConfig config = new ConfigPayload(slime).toInstance(MaptypesConfig.class, ""); + assertThat(config.stringmap("key"), is("val")); + } + + @Test + public void test_map_of_struct() { + Slime slime = new Slime(); + Cursor map = slime.setObject().setObject("innermap"); + map.setObject("one").setLong("foo", 1); + map.setObject("two").setLong("foo", 2); + + MaptypesConfig config = new ConfigPayload(slime).toInstance(MaptypesConfig.class, ""); + assertThat(config.innermap("one").foo(), is(1)); + assertThat(config.innermap("two").foo(), is(2)); + } + + @Test + public void test_map_of_map() { + Slime slime = new Slime(); + Cursor map = slime.setObject().setObject("nestedmap").setObject("my-nested").setObject("inner"); + map.setLong("one", 1); + map.setLong("two", 2); + + MaptypesConfig config = new ConfigPayload(slime).toInstance(MaptypesConfig.class, ""); + assertThat(config.nestedmap("my-nested").inner("one"), is(1)); + assertThat(config.nestedmap("my-nested").inner("two"), is(2)); + } + + /* Non existent fields of all types must be ignored to allow adding new fields to a config definition + * without breaking hosted Vespa applications that have config overrides for the given config class. + * The generated payload for the user override will contain the new field (empty as the user doesn't + * override it), because it exists in the latest config def version. The config class version follows + * the applications's Vespa version, which may be older and doesn't have the new struct. Hence, we just + * ignore unknown fields in the payload. + */ + + @Test + public void non_existent_leaf_in_payload_is_ignored() { + SimpletypesConfig config = createSimpletypesConfig("non_existent", ""); + assertNotNull(config); + } + + @Test + public void non_existent_struct_in_payload_is_ignored() { + Slime slime = new Slime(); + addStructFields(slime.setObject().setObject("non_existent"), "", "", null); + StructtypesConfig config = new ConfigPayload(slime).toInstance(StructtypesConfig.class, ""); + assertNotNull(config); + } + + @Test + public void non_existent_struct_in_struct_in_payload_is_ignored() { + Slime slime = new Slime(); + addStructFields(slime.setObject().setObject("nested").setObject("non_existent_inner"), "", "", null); + StructtypesConfig config = new ConfigPayload(slime).toInstance(StructtypesConfig.class, ""); + assertNotNull(config); + } + + @Test + public void non_existent_array_of_struct_in_payload_is_ignored() { + Slime slime = new Slime(); + Cursor array = slime.setObject().setArray("non_existent_arr"); + array.addObject().setString("name", "val"); + StructtypesConfig config = new ConfigPayload(slime).toInstance(StructtypesConfig.class, ""); + assertNotNull(config); } @Test - public void test_set_nonexistent_field() throws Exception { - createSimpletypesConfig("doesnotexist", "blabla"); + public void non_existent_struct_in_array_of_struct_in_payload_is_ignored() { + Slime slime = new Slime(); + Cursor nestedArrEntry = slime.setObject().setArray("nestedarr").addObject(); + addStructFields(nestedArrEntry.setObject("inner"), "existing", "MALE", null); + addStructFields(nestedArrEntry.setObject("non_existent"), "non-existent", "MALE", null); + + StructtypesConfig config = new ConfigPayload(slime).toInstance(StructtypesConfig.class, ""); + assertThat(config.nestedarr(0).inner().name(), is("existing")); + } + + @Test + public void non_existent_simple_map_in_payload_is_ignored() { + Slime slime = new Slime(); + Cursor map = slime.setObject().setObject("non_existent_map"); + map.setString("key","val"); + MaptypesConfig config = new ConfigPayload(slime).toInstance(MaptypesConfig.class, ""); + assertNotNull(config); + } + + @Test + public void non_existent_map_of_struct_in_payload_is_ignored() { + Slime slime = new Slime(); + Cursor map = slime.setObject().setObject("non_existent_inner_map"); + map.setObject("one").setLong("foo", 1); + MaptypesConfig config = new ConfigPayload(slime).toInstance(MaptypesConfig.class, ""); + assertNotNull(config); + } + + @Test + public void non_existent_map_in_map_in_payload_is_ignored() { + Slime slime = new Slime(); + Cursor map = slime.setObject().setObject("nestedmap").setObject("my-nested"); + map.setObject("inner").setLong("one", 1); + map.setObject("non_existent").setLong("non-existent", 0); + + MaptypesConfig config = new ConfigPayload(slime).toInstance(MaptypesConfig.class, ""); + assertThat(config.nestedmap("my-nested").inner("one"), is(1)); + } + + @Test + public void test_function_test() { + // TODO: Test function test config as a complete config example } @Test @@ -362,7 +481,7 @@ public class ConfigPayloadTest { assertThat(payload.toString(true), is("{\"boolval\":false,\"doubleval\":0.0,\"enumval\":\"VAL1\",\"intval\":0,\"longval\":0,\"stringval\":\"s\",\"newfield\":\"3\"}")); } - /** + /* * TODO: Test invalid slime trees? * TODO: Test sending in wrong class */ @@ -412,12 +531,6 @@ public class ConfigPayloadTest { } } - private StructtypesConfig createStructtypesConfigSimple(String name, String gender, String [] emails) { - Slime slime = new Slime(); - addStructFields(slime.setObject().setObject("simple"), name, gender, emails); - return new ConfigPayload(slime).toInstance(StructtypesConfig.class, ""); - } - private StructtypesConfig createStructtypesConfigArray(String[] names, String[] genders) { Slime slime = new Slime(); Cursor array = slime.setObject().setArray("simplearr"); diff --git a/configdefinitions/src/vespa/attributes.def b/configdefinitions/src/vespa/attributes.def index 08ab9337e46..344322d3873 100644 --- a/configdefinitions/src/vespa/attributes.def +++ b/configdefinitions/src/vespa/attributes.def @@ -2,7 +2,7 @@ namespace=vespa.config.search attribute[].name string -attribute[].datatype enum { STRING, UINT1, UINT2, UINT4, INT8, INT16, INT32, INT64, FLOAT, DOUBLE, PREDICATE, TENSOR, REFERENCE, NONE } default=NONE +attribute[].datatype enum { STRING, BOOL, UINT1, UINT2, UINT4, INT8, INT16, INT32, INT64, FLOAT16, FLOAT, DOUBLE, PREDICATE, TENSOR, REFERENCE, NONE } default=NONE attribute[].collectiontype enum { SINGLE, ARRAY, WEIGHTEDSET } default=SINGLE attribute[].removeifzero bool default=false attribute[].createifnonexistent bool default=false diff --git a/configutil/src/tests/config_status/config_status_test.cpp b/configutil/src/tests/config_status/config_status_test.cpp index 304d4518b41..3cc5b2f4525 100644 --- a/configutil/src/tests/config_status/config_status_test.cpp +++ b/configutil/src/tests/config_status/config_status_test.cpp @@ -19,7 +19,7 @@ private: bool _fail; Portal::Token::UP _root; - void get(Portal::GetRequest request) const override { + void get(Portal::GetRequest request) override { if (_fail) { request.respond_with_error(500, "Error"); } else { diff --git a/container-search/src/main/java/com/yahoo/data/JsonProducer.java b/container-search/src/main/java/com/yahoo/data/JsonProducer.java index dbc652d2d7d..6d925b41379 100644 --- a/container-search/src/main/java/com/yahoo/data/JsonProducer.java +++ b/container-search/src/main/java/com/yahoo/data/JsonProducer.java @@ -19,9 +19,11 @@ public interface JsonProducer { /** * Convenience method equivalent to: - * makeJson(new StringBuilder()).toString() + * writeJson(new StringBuilder()).toString() * @return String containing JSON representation of this object's data. */ - String toJson(); + default String toJson() { + return writeJson(new StringBuilder()).toString(); + } } diff --git a/container-search/src/main/java/com/yahoo/data/XmlProducer.java b/container-search/src/main/java/com/yahoo/data/XmlProducer.java index 93b0e8d296c..cdbfa61d2c2 100644 --- a/container-search/src/main/java/com/yahoo/data/XmlProducer.java +++ b/container-search/src/main/java/com/yahoo/data/XmlProducer.java @@ -15,10 +15,11 @@ public interface XmlProducer { /** * Convenience method equivalent to: - * makeXML(new StringBuilder()).toString() + * writeXML(new StringBuilder()).toString() * @return String containing XML representation of this object's data. */ - String toXML(); + default String toXML() { + return writeXML(new StringBuilder()).toString(); + } } - diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/BoolField.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/BoolField.java new file mode 100644 index 00000000000..633e76a2d9c --- /dev/null +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/BoolField.java @@ -0,0 +1,24 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +/** + * Class representing a byte field in the result set + * + */ + +package com.yahoo.prelude.fastsearch; + +import com.yahoo.data.access.Inspector; +import com.yahoo.search.result.NanNumber; + +/** + * @author bratseth + */ +public class BoolField extends DocsumField { + + public BoolField(String name) { + super(name); + } + + @Override + public Object convert(Inspector value) { return value.asBool(); } + +} diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/ByteField.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/ByteField.java index 256edc66793..22069d0270c 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/ByteField.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/ByteField.java @@ -6,9 +6,6 @@ package com.yahoo.prelude.fastsearch; - -import java.nio.ByteBuffer; - import com.yahoo.search.result.NanNumber; import com.yahoo.data.access.Inspector; diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinition.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinition.java index 675c1af6545..cdf696ebe1e 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinition.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinition.java @@ -4,11 +4,9 @@ package com.yahoo.prelude.fastsearch; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.yahoo.data.access.Inspector; -import com.yahoo.vespa.config.search.SummaryConfig; import com.yahoo.container.search.LegacyEmulationConfig; import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumField.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumField.java index 7d68e7b6679..777919286dd 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumField.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumField.java @@ -48,10 +48,12 @@ public abstract class DocsumField { fieldFactory = new FieldFactory(); try { + fieldFactory.put("bool", BoolField.class); fieldFactory.put("byte", ByteField.class); fieldFactory.put("short", ShortField.class); fieldFactory.put("integer", IntegerField.class); fieldFactory.put("int64", Int64Field.class); + fieldFactory.put("float16", Float16Field.class); fieldFactory.put("float", FloatField.class); fieldFactory.put("double", DoubleField.class); fieldFactory.put("string", StringField.class); diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/Float16Field.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/Float16Field.java new file mode 100644 index 00000000000..f00d6b19d67 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/Float16Field.java @@ -0,0 +1,33 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.prelude.fastsearch; + +import com.yahoo.data.access.Inspector; +import com.yahoo.search.result.NanNumber; + +/** + * A 16-bit float, represented as a (32-bit) Float in Java, as there is no 16-bit float support. + * + * @author bratseth + */ +public class Float16Field extends DocsumField { + + static final double EMPTY_VALUE = Float.NaN; + + public Float16Field(String name) { + super(name); + } + + private Object convert(float value) { + if (Float.isNaN(value)) { + return NanNumber.NaN; + } else { + return Float.valueOf(value); + } + } + + @Override + public Object convert(Inspector value) { + return convert((float)value.asDouble(EMPTY_VALUE)); + } + +} diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/XMLField.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/XMLField.java index 5c85c3641cf..cb115502468 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/XMLField.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/XMLField.java @@ -1,18 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. /** - * Class representing a string field in the result set - * + * Class converting data (historically XML-encoded) from a document summary field. + * This has only been used to represent geographical positions. */ package com.yahoo.prelude.fastsearch; - -import java.nio.ByteBuffer; - -import com.yahoo.io.SlowInflate; -import com.yahoo.prelude.hitfield.XMLString; -import com.yahoo.text.Utf8; import com.yahoo.data.access.Inspector; - +import com.yahoo.data.access.Type; +import com.yahoo.prelude.hitfield.XMLString; +import com.yahoo.search.result.PositionsData; /** * @author Steinar Knutsen @@ -34,6 +30,13 @@ public class XMLField extends DocsumField { @Override public Object convert(Inspector value) { + /* In Vespa 6 the backend will send an XML-formatted string to represent + * positions data. This will change in next version to sending an array + * of objects instead, suitable for the PositionsData class. + */ + if (value.type() == Type.ARRAY) { + return new PositionsData(value); + } return convert(value.asString("")); } diff --git a/container-search/src/main/java/com/yahoo/prelude/hitfield/XMLString.java b/container-search/src/main/java/com/yahoo/prelude/hitfield/XMLString.java index b728d8802e3..5aa7c62279b 100644 --- a/container-search/src/main/java/com/yahoo/prelude/hitfield/XMLString.java +++ b/container-search/src/main/java/com/yahoo/prelude/hitfield/XMLString.java @@ -1,12 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude.hitfield; +import com.yahoo.data.XmlProducer; + /** * A representation of an XML chunk. * - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen */ -public class XMLString { +public class XMLString implements XmlProducer { private final String content; @@ -18,4 +20,14 @@ public class XMLString { return content; } + public StringBuilder writeXML(StringBuilder target) { + target.append(content); + return target; + } + + @Override + public String toXML() { + return content; + } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/Item.java b/container-search/src/main/java/com/yahoo/prelude/query/Item.java index 0ba4133901a..d8b5fc9451a 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/Item.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/Item.java @@ -304,7 +304,7 @@ public abstract class Item implements Cloneable { public static void putString(String s, ByteBuffer buffer) { putBytes(Utf8.toBytes(s), buffer); } - public static void putBytes(byte [] bytes, ByteBuffer buffer) { + public static void putBytes(byte[] bytes, ByteBuffer buffer) { IntegerCompressor.putCompressedPositiveNumber(bytes.length, buffer); buffer.put(bytes); } @@ -324,6 +324,7 @@ public abstract class Item implements Cloneable { * TODO: Change the output query language into a canonical form of the input * query language */ + @Override public String toString() { StringBuilder buffer = new StringBuilder(); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/TermItem.java b/container-search/src/main/java/com/yahoo/prelude/query/TermItem.java index e3edf3ea780..764fb2b1118 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/TermItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/TermItem.java @@ -8,12 +8,8 @@ import java.nio.ByteBuffer; /** - * <p>A query term, that is, not only a term in the query language - * (an <i>item</i>), but also a term to be found in (or excluded from) - * the search index.</p> - * - * <p>Optionally, a TermItem may also specify the name of an - * index backend to search.</p> + * Superclass of "leaf" conditions containing a single entity which is either matched in + * a field or not. * * @author bratseth * @author havardpe @@ -84,6 +80,7 @@ public abstract class TermItem extends SimpleIndexedItem implements BlockItem { */ public boolean isFromQuery() { return isFromQuery; } + @Override public abstract boolean isWords(); /** Sets the origin of this */ diff --git a/container-search/src/main/java/com/yahoo/prelude/query/WordItem.java b/container-search/src/main/java/com/yahoo/prelude/query/WordItem.java index 246924ea9de..0b3d11158f1 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/WordItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/WordItem.java @@ -9,14 +9,14 @@ import com.yahoo.protect.Validator; import java.nio.ByteBuffer; /** - * A term item containing a single word. + * A simple word or token to match in some field. * * @author bratseth * @author havardpe */ public class WordItem extends TermItem { - /** True if this is <b>not</b> part of the special tokens dictionary */ + /** True if this is not part of the special tokens dictionary */ private boolean words = true; /** Is this word stemmed? */ @@ -141,24 +141,20 @@ public class WordItem extends TermItem { } /** Word items uses a empty heading instead of "WORD " */ + @Override protected void appendHeadingString(StringBuilder buffer) {} + @Override public int hashCode() { return word.hashCode() + 71 * super.hashCode(); } + @Override public boolean equals(Object object) { - if (!super.equals(object)) { - return false; - } + if (!super.equals(object)) return false; WordItem other = (WordItem) object; // Ensured by superclass - - if (!this.word.equals(other.word)) { - return false; - } - - return true; + return this.word.equals(other.word); } public int getNumWords() { @@ -188,4 +184,5 @@ public class WordItem extends TermItem { discloser.addProperty("stemmed", stemmed); discloser.addProperty("words", words); } + } diff --git a/container-search/src/main/java/com/yahoo/search/rendering/DefaultRenderer.java b/container-search/src/main/java/com/yahoo/search/rendering/DefaultRenderer.java index 3562a1a9572..30695338741 100644 --- a/container-search/src/main/java/com/yahoo/search/rendering/DefaultRenderer.java +++ b/container-search/src/main/java/com/yahoo/search/rendering/DefaultRenderer.java @@ -2,6 +2,7 @@ package com.yahoo.search.rendering; import com.yahoo.concurrent.CopyOnWriteHashMap; +import com.yahoo.data.XmlProducer; import com.yahoo.io.ByteWriter; import com.yahoo.net.URI; import com.yahoo.prelude.fastsearch.GroupingListHit; @@ -198,6 +199,8 @@ public final class DefaultRenderer extends AsynchronousSectionedRenderer<Result> private String asXML(Object value) { if (value == null) return "(null)"; + else if (value instanceof XmlProducer) + return ((XmlProducer)value).toXML(); else if (value instanceof HitField) return ((HitField)value).quotedContent(false); else if (value instanceof StructuredData || value instanceof XMLString || value instanceof JSONString) diff --git a/container-search/src/main/java/com/yahoo/search/rendering/XmlRenderer.java b/container-search/src/main/java/com/yahoo/search/rendering/XmlRenderer.java index 2a822f89352..5f99c531c95 100644 --- a/container-search/src/main/java/com/yahoo/search/rendering/XmlRenderer.java +++ b/container-search/src/main/java/com/yahoo/search/rendering/XmlRenderer.java @@ -2,6 +2,7 @@ package com.yahoo.search.rendering; import com.yahoo.concurrent.CopyOnWriteHashMap; +import com.yahoo.data.XmlProducer; import com.yahoo.io.ByteWriter; import com.yahoo.net.URI; import com.yahoo.prelude.fastsearch.GroupingListHit; @@ -195,6 +196,8 @@ public final class XmlRenderer extends AsynchronousSectionedRenderer<Result> { private String asXML(Object value) { if (value == null) return "(null)"; + else if (value instanceof XmlProducer) + return ((XmlProducer)value).toXML(); else if (value instanceof HitField) return ((HitField)value).quotedContent(false); else if (value instanceof StructuredData || value instanceof XMLString || value instanceof JSONString) diff --git a/container-search/src/main/java/com/yahoo/search/result/PositionsData.java b/container-search/src/main/java/com/yahoo/search/result/PositionsData.java new file mode 100644 index 00000000000..a07eaa2438d --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/result/PositionsData.java @@ -0,0 +1,64 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.result; + +import com.yahoo.data.access.Inspector; +import com.yahoo.data.access.Inspectable; +import com.yahoo.data.access.Type; +import com.yahoo.data.JsonProducer; +import com.yahoo.data.XmlProducer; +import com.yahoo.data.access.simple.JsonRender; + +/** + * A wrapper for structured data representing an array of position values. + **/ +public class PositionsData implements Inspectable, JsonProducer, XmlProducer { + + private final Inspector value; + + public PositionsData(Inspector value) { + this.value = value; + if (value.type() != Type.ARRAY) { + throw new IllegalArgumentException("PositionsData expects an array of positions, got: "+value); + } + } + + @Override + public Inspector inspect() { + return value; + } + + public String toString() { + return toJson(); + } + + @Override + public StringBuilder writeJson(StringBuilder target) { + return JsonRender.render(value, target, true); + } + + @Override + public StringBuilder writeXML(StringBuilder target) { + for (int i = 0; i < value.entryCount(); i++) { + Inspector pos = value.entry(i); + target.append("<position "); + for (java.util.Map.Entry<String, Inspector> entry : pos.fields()) { + Inspector v = entry.getValue(); + if (v.type() == Type.STRING) { + target.append(entry.getKey()); + target.append("=\""); + target.append(entry.getValue().asString()); + target.append("\" "); + } + if (v.type() == Type.LONG) { + target.append(entry.getKey()); + target.append("=\""); + target.append(entry.getValue().asLong()); + target.append("\" "); + } + } + target.append("/>"); + } + return target; + } + +} diff --git a/container-search/src/main/java/com/yahoo/search/result/StructuredData.java b/container-search/src/main/java/com/yahoo/search/result/StructuredData.java index 1cee997e67f..8640180fad4 100644 --- a/container-search/src/main/java/com/yahoo/search/result/StructuredData.java +++ b/container-search/src/main/java/com/yahoo/search/result/StructuredData.java @@ -29,11 +29,6 @@ public class StructuredData implements Inspectable, JsonProducer, XmlProducer { } @Override - public String toXML() { - return writeXML(new StringBuilder()).toString(); - } - - @Override public StringBuilder writeXML(StringBuilder target) { return XmlRenderer.render(target, value); } diff --git a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java index 6eaa1592a3e..5928dd23b28 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java +++ b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java @@ -52,6 +52,7 @@ import com.yahoo.prelude.query.Substring; import com.yahoo.prelude.query.SubstringItem; import com.yahoo.prelude.query.SuffixItem; import com.yahoo.prelude.query.TaggableItem; +import com.yahoo.prelude.query.TermItem; import com.yahoo.prelude.query.ToolBox; import com.yahoo.prelude.query.ToolBox.QueryVisitor; import com.yahoo.prelude.query.WandItem; @@ -942,13 +943,19 @@ public class YqlParser implements Parser { } @NonNull - private IntItem buildEquals(OperatorNode<ExpressionOperator> ast) { - IntItem number = new IntItem(fetchConditionWord(ast), fetchConditionIndex(ast)); - if (isIndexOnLeftHandSide(ast)) { - return leafStyleSettings(ast.getArgument(1, OperatorNode.class), number); - } else { - return leafStyleSettings(ast.getArgument(0, OperatorNode.class), number); - } + private TermItem buildEquals(OperatorNode<ExpressionOperator> ast) { + String value = fetchConditionWord(ast); + + TermItem item; + if (value.equals("true") || value.equals("false")) + item = new WordItem(value, fetchConditionIndex(ast)); + else + item = new IntItem(value, fetchConditionIndex(ast)); + + if (isIndexOnLeftHandSide(ast)) + return leafStyleSettings(ast.getArgument(1, OperatorNode.class), item); + else + return leafStyleSettings(ast.getArgument(0, OperatorNode.class), item); } @NonNull diff --git a/container-search/src/test/java/com/yahoo/search/result/PositionsDataTestCase.java b/container-search/src/test/java/com/yahoo/search/result/PositionsDataTestCase.java new file mode 100644 index 00000000000..c77ff2eca3c --- /dev/null +++ b/container-search/src/test/java/com/yahoo/search/result/PositionsDataTestCase.java @@ -0,0 +1,55 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.result; + +import com.yahoo.data.access.simple.Value; + +import static org.junit.Assert.*; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +/** + * @author Arne Juul + */ +public class PositionsDataTestCase { + + @Test + public void testRendering() { + Value.ArrayValue arr = new Value.ArrayValue(); + Value.ObjectValue p1 = new Value.ObjectValue(); + p1.put("x", new Value.LongValue(-122057174)); + p1.put("y", new Value.LongValue(37374821)); + p1.put("latlong", new Value.StringValue("N37.374821;W122.057174")); + arr.add(p1); + + PositionsData pd = new PositionsData(arr.inspect()); + + String rendered = pd.toXML(); + String correct = "<position x=\"-122057174\" y=\"37374821\" latlong=\"N37.374821;W122.057174\" />"; + assertEquals(correct, rendered); + + rendered = pd.toJson(); + correct = "[{\"x\":-122057174,\"y\":37374821,\"latlong\":\"N37.374821;W122.057174\"}]"; + assertEquals(correct, rendered); + + Value.ObjectValue p2 = new Value.ObjectValue(); + p2.put("x", new Value.LongValue(3)); + p2.put("y", new Value.LongValue(-7)); + p2.put("latlong", new Value.StringValue("S0.000007;E0.000003")); + arr.add(p2); + + pd = new PositionsData(arr.inspect()); + + rendered = pd.toXML(); + correct = "<position x=\"-122057174\" y=\"37374821\" latlong=\"N37.374821;W122.057174\" />" + + "<position x=\"3\" y=\"-7\" latlong=\"S0.000007;E0.000003\" />"; + assertEquals(correct, rendered); + + rendered = pd.toJson(); + correct = "[{\"x\":-122057174,\"y\":37374821,\"latlong\":\"N37.374821;W122.057174\"}," + + "{\"x\":3,\"y\":-7,\"latlong\":\"S0.000007;E0.000003\"}]"; + assertEquals(correct, rendered); + } + +} diff --git a/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java index 8c47ae377c3..f2c53fb6b31 100644 --- a/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java @@ -247,6 +247,12 @@ public class YqlParserTestCase { } @Test + public void testBoolean() { + assertParse("select foo from bar where flag = true;", "flag:true"); + assertParse("select foo from bar where flag = false;", "flag:false"); + } + + @Test public void testTermAnnotations() { assertEquals("merkelapp", getRootWord("select foo from bar where baz contains " + diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java index e5f34f3721a..5a4086f9abb 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java @@ -16,8 +16,6 @@ public enum JobType { component ("component" , null , null ), systemTest ("system-test" , ZoneId.from("test" , "us-east-1") , ZoneId.from("test" , "cd-us-central-1") ), stagingTest ("staging-test" , ZoneId.from("staging", "us-east-3") , ZoneId.from("staging", "cd-us-central-1") ), - // TODO: Remove after corp zone disappears - productionCorpUsEast1 ("production-corp-us-east-1" , ZoneId.from("prod" , "corp-us-east-1") , null ), productionUsEast3 ("production-us-east-3" , ZoneId.from("prod" , "us-east-3") , null ), productionUsWest1 ("production-us-west-1" , ZoneId.from("prod" , "us-west-1") , null ), productionUsCentral1 ("production-us-central-1" , ZoneId.from("prod" , "us-central-1") , null ), @@ -48,7 +46,7 @@ public enum JobType { /** Returns the zone for this job in the given system, or throws if this job does not have a zone */ public ZoneId zone(SystemName system) { if ( ! zones.containsKey(system)) - throw new AssertionError(this + " does not have any zones in " + system + "."); + throw new IllegalArgumentException(this + " does not have any zones in " + system); return zones.get(system); } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mail.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mail.java new file mode 100644 index 00000000000..c7e8339aa28 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mail.java @@ -0,0 +1,32 @@ +package com.yahoo.vespa.hosted.controller.api.integration.organization; + +import com.google.common.collect.ImmutableList; + +import java.util.List; +import java.util.Objects; + +/** + * A message with a sender and a set of recipients. + * + * @author jonmv + */ +public class Mail { + + private final List<String> recipients; + private final String subject; + private final String message; + + public Mail(List<String> recipients, String subject, String message) { + if (recipients.isEmpty()) + throw new IllegalArgumentException("Empty recipient list is not allowed."); + recipients.forEach(Objects::requireNonNull); + this.recipients = ImmutableList.copyOf(recipients); + this.subject = Objects.requireNonNull(subject); + this.message = Objects.requireNonNull(message); + } + + public List<String> recipients() { return recipients; } + public String subject() { return subject; } + public String message() { return message; } + +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mailer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mailer.java new file mode 100644 index 00000000000..a915f22c20c --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mailer.java @@ -0,0 +1,19 @@ +package com.yahoo.vespa.hosted.controller.api.integration.organization; + +/** + * Allows sending e-mail from a particular user@domain. + * + * @author jonmv + */ +public interface Mailer { + + /** Sends the given mail as the configured user@domain. */ + void send(Mail mail); + + /** Returns the user this is configured to use. */ + String user(); + + /** Returns the domain this is configured to use. */ + String domain(); + +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockMailer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockMailer.java new file mode 100644 index 00000000000..9373d97a11c --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockMailer.java @@ -0,0 +1,30 @@ +package com.yahoo.vespa.hosted.controller.api.integration.organization; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +public class MockMailer implements Mailer { + + public final Map<String, List<Mail>> mails = new HashMap<>(); + + @Override + public void send(Mail mail) { + for (String recipient : mail.recipients()) { + mails.putIfAbsent(recipient, new ArrayList<>()); + mails.get(recipient).add(mail); + } + } + + @Override + public String user() { + return "user"; + } + + @Override + public String domain() { + return "domain"; + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java index 6820c4ba6b6..a22e5259919 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.rotation; -import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.Environment; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.ApplicationController; @@ -69,8 +68,6 @@ public class RotationRepository { } long productionZones = application.deploymentSpec().zones().stream() .filter(zone -> zone.deploysTo(Environment.prod)) - // Global rotations don't work for nodes in corp network - .filter(zone -> !isCorp(zone)) .count(); if (productionZones < 2) { throw new IllegalArgumentException("global-service-id is set but less than 2 prod zones are defined"); @@ -104,11 +101,6 @@ public class RotationRepository { return allRotations.get(rotation); } - // TODO: Remove after corp zones disappear - private static boolean isCorp(DeploymentSpec.DeclaredZone zone) { - return zone.region().isPresent() && zone.region().get().value().contains("corp"); - } - /** Returns a immutable map of rotation ID to rotation sorted by rotation ID */ private static Map<RotationId, Rotation> from(RotationsConfig rotationConfig) { return rotationConfig.rotations().entrySet().stream() diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepositoryTest.java index 1d5ddaa1ec0..666c7774cf5 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepositoryTest.java @@ -4,7 +4,6 @@ package com.yahoo.vespa.hosted.controller.rotation; import com.yahoo.config.provision.SystemName; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.ControllerTester; -import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; @@ -24,7 +23,7 @@ import static org.junit.Assert.assertFalse; * @author Oyvind Gronnesby * @author mpolden */ -public class RotationTest { +public class RotationRepositoryTest { @Rule public ExpectedException thrown = ExpectedException.none(); @@ -145,40 +144,6 @@ public class RotationTest { } @Test - public void application_with_only_one_non_corp_region() { - tester.controllerTester().zoneRegistry().setZones(ZoneId.from("prod", "corp-us-east-1"), - ZoneId.from("prod", "us-east-3")); - ApplicationPackage applicationPackage = new ApplicationPackageBuilder() - .globalServiceId("foo") - .region("us-east-3") - .region("corp-us-east-1") - .build(); - Application application = tester.createApplication("app2", "tenant2", 22L, - 2L); - thrown.expect(RuntimeException.class); - thrown.expectMessage("less than 2 prod zones are defined"); - tester.deployCompletely(application, applicationPackage); - } - - @Test - public void application_with_corp_region_and_two_non_corp_region() { - tester.controllerTester().zoneRegistry().setZones(ZoneId.from("prod", "corp-us-east-1"), - ZoneId.from("prod", "us-east-3"), - ZoneId.from("prod", "us-west-1")); - ApplicationPackage applicationPackage = new ApplicationPackageBuilder() - .globalServiceId("foo") - .region("us-east-3") - .region("corp-us-east-1") - .region("us-west-1") - .build(); - Application application = tester.createApplication("app2", "tenant2", 22L, - 2L); - tester.deployCompletely(application, applicationPackage); - assertEquals(new RotationId("foo-1"), tester.applications().require(application.id()) - .rotation().get()); - } - - @Test public void prefixes_system_when_not_main() { ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .globalServiceId("foo") @@ -193,4 +158,5 @@ public class RotationTest { assertEquals("https://cd--app2--tenant2.global.vespa.yahooapis.com:4443/", tester.applications().require(application.id()) .globalDnsName(SystemName.cd).get().secureUrl().toString()); } + } diff --git a/document/src/main/java/com/yahoo/document/DataType.java b/document/src/main/java/com/yahoo/document/DataType.java index dfe55f5229c..fa5dffd042a 100644 --- a/document/src/main/java/com/yahoo/document/DataType.java +++ b/document/src/main/java/com/yahoo/document/DataType.java @@ -3,6 +3,7 @@ package com.yahoo.document; import com.yahoo.collections.Pair; import com.yahoo.concurrent.CopyOnWriteHashMap; +import com.yahoo.document.datatypes.BoolFieldValue; import com.yahoo.document.datatypes.ByteFieldValue; import com.yahoo.document.datatypes.DoubleFieldValue; import com.yahoo.document.datatypes.FieldValue; @@ -11,8 +12,8 @@ import com.yahoo.document.datatypes.IntegerFieldValue; import com.yahoo.document.datatypes.LongFieldValue; import com.yahoo.document.datatypes.PredicateFieldValue; import com.yahoo.document.datatypes.Raw; +import com.yahoo.document.datatypes.Float16FieldValue; import com.yahoo.document.datatypes.StringFieldValue; -import com.yahoo.document.datatypes.TensorFieldValue; import com.yahoo.document.datatypes.UriFieldValue; import com.yahoo.tensor.TensorType; import com.yahoo.vespa.objects.Identifiable; @@ -39,7 +40,6 @@ public abstract class DataType extends Identifiable implements Serializable, Com // NOTE: These types are also defined in // document/src/vespa/document/datatype/datatype.h // Changes here must also be done there - public final static NumericDataType NONE = new NumericDataType("none", -1, IntegerFieldValue.class, IntegerFieldValue.getFactory()); public final static NumericDataType INT = new NumericDataType("int", 0, IntegerFieldValue.class, IntegerFieldValue.getFactory()); public final static NumericDataType FLOAT = new NumericDataType("float", 1, FloatFieldValue.class, FloatFieldValue.getFactory()); @@ -47,6 +47,8 @@ public abstract class DataType extends Identifiable implements Serializable, Com public final static PrimitiveDataType RAW = new PrimitiveDataType("raw", 3, Raw.class, Raw.getFactory()); public final static NumericDataType LONG = new NumericDataType("long", 4, LongFieldValue.class, LongFieldValue.getFactory()); public final static NumericDataType DOUBLE = new NumericDataType("double", 5, DoubleFieldValue.class, DoubleFieldValue.getFactory()); + public final static PrimitiveDataType BOOL = new PrimitiveDataType("bool", 6, BoolFieldValue.class, BoolFieldValue.getFactory()); + public final static NumericDataType FLOAT16 = new NumericDataType("float16", 7, Float16FieldValue.class, Float16FieldValue.getFactory()); 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()); @@ -82,7 +84,6 @@ public abstract class DataType extends Identifiable implements Serializable, Com this.dataTypeId = dataTypeId; } - @SuppressWarnings("CloneDoesntDeclareCloneNotSupportedException") public DataType clone() { return (DataType)super.clone(); } diff --git a/document/src/main/java/com/yahoo/document/NumericDataType.java b/document/src/main/java/com/yahoo/document/NumericDataType.java index f4a92eca5a1..26da90a709c 100644 --- a/document/src/main/java/com/yahoo/document/NumericDataType.java +++ b/document/src/main/java/com/yahoo/document/NumericDataType.java @@ -4,9 +4,10 @@ package com.yahoo.document; import com.yahoo.vespa.objects.Ids; /** - * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a> + * @author Einar M R Rosenvinge */ public class NumericDataType extends PrimitiveDataType { + // The global class identifier shared with C++. public static int classId = registerClass(Ids.document + 52, NumericDataType.class); /** @@ -24,4 +25,5 @@ public class NumericDataType extends PrimitiveDataType { public NumericDataType clone() { return (NumericDataType) super.clone(); } + } diff --git a/document/src/main/java/com/yahoo/document/datatypes/BoolFieldValue.java b/document/src/main/java/com/yahoo/document/datatypes/BoolFieldValue.java new file mode 100644 index 00000000000..2a48b550658 --- /dev/null +++ b/document/src/main/java/com/yahoo/document/datatypes/BoolFieldValue.java @@ -0,0 +1,120 @@ +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.document.datatypes; + +import com.yahoo.document.DataType; +import com.yahoo.document.Field; +import com.yahoo.document.PrimitiveDataType; +import com.yahoo.document.serialization.FieldReader; +import com.yahoo.document.serialization.FieldWriter; +import com.yahoo.document.serialization.XmlSerializationHelper; +import com.yahoo.document.serialization.XmlStream; +import com.yahoo.vespa.objects.Ids; + +/** + * A boolean field value + * + * @author bratseth + */ +public class BoolFieldValue extends FieldValue { + + private static class Factory extends PrimitiveDataType.Factory { + public FieldValue create() { + return new BoolFieldValue(); + } + } + + public static PrimitiveDataType.Factory getFactory() { return new Factory(); } + public static final int classId = registerClass(Ids.document + 17, BoolFieldValue.class); + private boolean value; + + public BoolFieldValue() { + this(false); + } + + public BoolFieldValue(boolean value) { + this.value = value; + } + + public BoolFieldValue(String s) { value = Boolean.parseBoolean(s); } + + @Override + public BoolFieldValue clone() { + return (BoolFieldValue)super.clone(); + } + + @Override + public void clear() { + value = false; + } + + @Override + public void assign(Object o) { + if ( ! checkAssign(o)) return; + if (o instanceof String || o instanceof StringFieldValue) { + value = Boolean.parseBoolean(o.toString()); + } else { + throw new IllegalArgumentException("Class " + o.getClass() + " not applicable to an " + this.getClass() + " instance."); + } + } + + public boolean getBoolean() { + return value; + } + + @Override + public Object getWrappedValue() { + return value; + } + + @Override + public DataType getDataType() { + return DataType.BOOL; + } + + @Override + public void printXml(XmlStream xml) { + XmlSerializationHelper.printBoolXml(this, xml); + } + + @Override + public String toString() { + return "" + value; + } + + @Override + public int hashCode() { + return super.hashCode() + ( value ? 3 : 0); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if ( ! (o instanceof BoolFieldValue)) return false; + if ( ! super.equals(o)) return false; + + BoolFieldValue that = (BoolFieldValue) o; + if (value != that.value) return false; + return true; + } + + @Override + public void serialize(Field field, FieldWriter writer) { + writer.write(field, this); + } + + /* (non-Javadoc) + * @see com.yahoo.document.datatypes.FieldValue#deserialize(com.yahoo.document.Field, com.yahoo.document.serialization.FieldReader) + */ + @Override + public void deserialize(Field field, FieldReader reader) { + reader.read(field, this); + } + + @Override + public int compareTo(FieldValue other) { + int comp = super.compareTo(other); + if (comp != 0) return comp; + return Boolean.compare(value, ((BoolFieldValue)other).value); + } + +} diff --git a/document/src/main/java/com/yahoo/document/datatypes/ByteFieldValue.java b/document/src/main/java/com/yahoo/document/datatypes/ByteFieldValue.java index 4aa4c29db48..2f8cb96219c 100644 --- a/document/src/main/java/com/yahoo/document/datatypes/ByteFieldValue.java +++ b/document/src/main/java/com/yahoo/document/datatypes/ByteFieldValue.java @@ -11,16 +11,18 @@ import com.yahoo.document.serialization.XmlStream; import com.yahoo.vespa.objects.Ids; /** - * FieldValue which encapsulates a byte. + * A byte field value * - * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a> + * @author Einar M R Rosenvinge */ public class ByteFieldValue extends NumericFieldValue { + private static class Factory extends PrimitiveDataType.Factory { public FieldValue create() { return new ByteFieldValue(); } } + public static PrimitiveDataType.Factory getFactory() { return new Factory(); } public static final int classId = registerClass(Ids.document + 10, ByteFieldValue.class); private byte value; @@ -125,9 +127,8 @@ public class ByteFieldValue extends NumericFieldValue { } /* (non-Javadoc) - * @see com.yahoo.document.datatypes.FieldValue#deserialize(com.yahoo.document.Field, com.yahoo.document.serialization.FieldReader) - */ - + * @see com.yahoo.document.datatypes.FieldValue#deserialize(com.yahoo.document.Field, com.yahoo.document.serialization.FieldReader) + */ @Override public void deserialize(Field field, FieldReader reader) { reader.read(field, this); @@ -151,4 +152,5 @@ public class ByteFieldValue extends NumericFieldValue { return 0; } } + } diff --git a/document/src/main/java/com/yahoo/document/datatypes/CollectionFieldValue.java b/document/src/main/java/com/yahoo/document/datatypes/CollectionFieldValue.java index fe135ccfd05..ad6d329cbae 100644 --- a/document/src/main/java/com/yahoo/document/datatypes/CollectionFieldValue.java +++ b/document/src/main/java/com/yahoo/document/datatypes/CollectionFieldValue.java @@ -7,9 +7,9 @@ import java.util.Collection; import java.util.Iterator; /** - * Date: Apr 16, 2008 + * Superclass of multivalue field values * - * @author <a href="mailto:humbe@yahoo-inc.com">Håkon Humberset</a> + * @author Håkon Humberset */ public abstract class CollectionFieldValue<T extends FieldValue> extends CompositeFieldValue { @@ -79,4 +79,5 @@ public abstract class CollectionFieldValue<T extends FieldValue> extends Composi } public abstract int size(); + } diff --git a/document/src/main/java/com/yahoo/document/datatypes/CompositeFieldValue.java b/document/src/main/java/com/yahoo/document/datatypes/CompositeFieldValue.java index 3202c6b40af..6efd5a86f75 100644 --- a/document/src/main/java/com/yahoo/document/datatypes/CompositeFieldValue.java +++ b/document/src/main/java/com/yahoo/document/datatypes/CompositeFieldValue.java @@ -4,6 +4,7 @@ package com.yahoo.document.datatypes; import com.yahoo.document.DataType; public abstract class CompositeFieldValue extends FieldValue { + private DataType dataType; public CompositeFieldValue(DataType dataType) { @@ -36,4 +37,5 @@ public abstract class CompositeFieldValue extends FieldValue { result = 31 * result + (dataType != null ? dataType.hashCode() : 0); return result; } + } diff --git a/document/src/main/java/com/yahoo/document/datatypes/DoubleFieldValue.java b/document/src/main/java/com/yahoo/document/datatypes/DoubleFieldValue.java index 0f1fe50818b..99bac017b78 100644 --- a/document/src/main/java/com/yahoo/document/datatypes/DoubleFieldValue.java +++ b/document/src/main/java/com/yahoo/document/datatypes/DoubleFieldValue.java @@ -11,16 +11,18 @@ import com.yahoo.document.serialization.XmlStream; import com.yahoo.vespa.objects.Ids; /** - * FieldValue which encapsulates a double. + * A 64-bit float field value * - * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a> + * @author Einar M R Rosenvinge */ public final class DoubleFieldValue extends NumericFieldValue { + private static class Factory extends PrimitiveDataType.Factory { public FieldValue create() { return new DoubleFieldValue(); } } + public static PrimitiveDataType.Factory getFactory() { return new Factory(); } public static final int classId = registerClass(Ids.document + 14, DoubleFieldValue.class); private double value; diff --git a/document/src/main/java/com/yahoo/document/datatypes/FieldPathIteratorHandler.java b/document/src/main/java/com/yahoo/document/datatypes/FieldPathIteratorHandler.java index de645aff297..9189a97dbf6 100644 --- a/document/src/main/java/com/yahoo/document/datatypes/FieldPathIteratorHandler.java +++ b/document/src/main/java/com/yahoo/document/datatypes/FieldPathIteratorHandler.java @@ -1,12 +1,11 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.document.datatypes; -import java.util.HashMap; import java.util.Map; import java.util.TreeMap; /** - * @author <a href="mailto:thomasg@yahoo-inc.com">Thomas Gundersen</a> + * @author Thomas Gundersen */ public abstract class FieldPathIteratorHandler { @@ -110,4 +109,5 @@ public abstract class FieldPathIteratorHandler { public VariableMap getVariables() { return variables; } + } diff --git a/document/src/main/java/com/yahoo/document/datatypes/FieldValue.java b/document/src/main/java/com/yahoo/document/datatypes/FieldValue.java index 0460e303426..dc3fd36b367 100644 --- a/document/src/main/java/com/yahoo/document/datatypes/FieldValue.java +++ b/document/src/main/java/com/yahoo/document/datatypes/FieldValue.java @@ -31,6 +31,7 @@ public abstract class FieldValue extends Identifiable implements Comparable<Fiel /** * Get XML representation of a single field and all its children, if any. + * * @return XML representation of field in a <value> element */ public String toXml() { diff --git a/document/src/main/java/com/yahoo/document/datatypes/Float16FieldValue.java b/document/src/main/java/com/yahoo/document/datatypes/Float16FieldValue.java new file mode 100644 index 00000000000..013f656b7b5 --- /dev/null +++ b/document/src/main/java/com/yahoo/document/datatypes/Float16FieldValue.java @@ -0,0 +1,137 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.document.datatypes; + +import com.yahoo.document.DataType; +import com.yahoo.document.Field; +import com.yahoo.document.PrimitiveDataType; +import com.yahoo.document.serialization.FieldReader; +import com.yahoo.document.serialization.FieldWriter; +import com.yahoo.document.serialization.XmlSerializationHelper; +import com.yahoo.document.serialization.XmlStream; +import com.yahoo.vespa.objects.Ids; + +/** + * A 16-bit float field value + * + * @author bratseth + */ +public final class Float16FieldValue extends NumericFieldValue { + + private static class Factory extends PrimitiveDataType.Factory { + public FieldValue create() { + return new Float16FieldValue(); + } + } + + public static PrimitiveDataType.Factory getFactory() { return new Factory(); } + public static final int classId = registerClass(Ids.document + 18, Float16FieldValue.class); + private float value; // 16-bit not supported in Java yet + + public Float16FieldValue() { + this((float) 0); + } + + public Float16FieldValue(float value) { + this.value = value; + } + + public Float16FieldValue(Float value) { + this.value = value; + } + + public Float16FieldValue(String s) { value = Float.parseFloat(s); } + + @Override + public Float16FieldValue clone() { + Float16FieldValue val = (Float16FieldValue) super.clone(); + val.value = value; + return val; + } + + @Override + public Number getNumber() { + return value; + } + + @Override + public void clear() { + value = 0.0f; + } + + @Override + public void assign(Object obj) { + if (!checkAssign(obj)) return; + + if (obj instanceof Number) + value = ((Number) obj).floatValue(); + else if (obj instanceof NumericFieldValue) + value = (((NumericFieldValue) obj).getNumber().floatValue()); + else if (obj instanceof String || obj instanceof StringFieldValue) + value = Float.parseFloat(obj.toString()); + else + throw new IllegalArgumentException("Class " + obj.getClass() + " not applicable to an " + this.getClass() + " instance."); + } + + public float getFloat() { + return value; + } + + @Override + public Object getWrappedValue() { + return value; + } + + @Override + public DataType getDataType() { + return DataType.FLOAT16; + } + + @Override + public void printXml(XmlStream xml) { + XmlSerializationHelper.printShortfloatXml(this, xml); + } + + @Override + public String toString() { + return String.valueOf(value); + } + + @Override + public int hashCode() { + int result = super.hashCode(); + result = 31 * result + (value != +0.0f ? Float.floatToIntBits(value) : 0); + return result; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof Float16FieldValue)) return false; + if (!super.equals(o)) return false; + + Float16FieldValue that = (Float16FieldValue) o; + if (Float.compare(that.value, value) != 0) return false; + return true; + } + + @Override + public void serialize(Field field, FieldWriter writer) { + writer.write(field, this); + } + + /* (non-Javadoc) + * @see com.yahoo.document.datatypes.FieldValue#deserialize(com.yahoo.document.Field, com.yahoo.document.serialization.FieldReader) + */ + @Override + public void deserialize(Field field, FieldReader reader) { + reader.read(field, this); + } + + @Override + public int compareTo(FieldValue fieldValue) { + int comp = super.compareTo(fieldValue); + if (comp != 0) return comp; + return Float.compare(value, ((Float16FieldValue) fieldValue).value); + } + +} diff --git a/document/src/main/java/com/yahoo/document/datatypes/FloatFieldValue.java b/document/src/main/java/com/yahoo/document/datatypes/FloatFieldValue.java index 5a94bb43a77..e09cbea861c 100644 --- a/document/src/main/java/com/yahoo/document/datatypes/FloatFieldValue.java +++ b/document/src/main/java/com/yahoo/document/datatypes/FloatFieldValue.java @@ -11,16 +11,18 @@ import com.yahoo.document.serialization.XmlStream; import com.yahoo.vespa.objects.Ids; /** - * FieldValue which encapsulates a float. + * A 32-bit float field value * - * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a> + * @author Einar M R Rosenvinge */ public final class FloatFieldValue extends NumericFieldValue { + private static class Factory extends PrimitiveDataType.Factory { public FieldValue create() { return new FloatFieldValue(); } } + public static PrimitiveDataType.Factory getFactory() { return new Factory(); } public static final int classId = registerClass(Ids.document + 13, FloatFieldValue.class); private float value; @@ -120,9 +122,8 @@ public final class FloatFieldValue extends NumericFieldValue { } /* (non-Javadoc) - * @see com.yahoo.document.datatypes.FieldValue#deserialize(com.yahoo.document.Field, com.yahoo.document.serialization.FieldReader) - */ - + * @see com.yahoo.document.datatypes.FieldValue#deserialize(com.yahoo.document.Field, com.yahoo.document.serialization.FieldReader) + */ @Override public void deserialize(Field field, FieldReader reader) { reader.read(field, this); diff --git a/document/src/main/java/com/yahoo/document/datatypes/IntegerFieldValue.java b/document/src/main/java/com/yahoo/document/datatypes/IntegerFieldValue.java index 19f34acde9a..62090bab8cb 100644 --- a/document/src/main/java/com/yahoo/document/datatypes/IntegerFieldValue.java +++ b/document/src/main/java/com/yahoo/document/datatypes/IntegerFieldValue.java @@ -11,9 +11,9 @@ import com.yahoo.document.serialization.XmlStream; import com.yahoo.vespa.objects.Ids; /** - * FieldValue which encapsulates an int. + * A 32-bit integer field value * - * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a> + * @author Einar M R Rosenvinge */ public final class IntegerFieldValue extends NumericFieldValue { diff --git a/document/src/main/java/com/yahoo/document/datatypes/LongFieldValue.java b/document/src/main/java/com/yahoo/document/datatypes/LongFieldValue.java index 2d3797f6735..45d044bcb9a 100644 --- a/document/src/main/java/com/yahoo/document/datatypes/LongFieldValue.java +++ b/document/src/main/java/com/yahoo/document/datatypes/LongFieldValue.java @@ -11,11 +11,12 @@ import com.yahoo.document.serialization.XmlStream; import com.yahoo.vespa.objects.Ids; /** - * FieldValue which encapsulates a long. + * A 64-bit integer field value * - * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a> + * @author Einar M R Rosenvinge */ public final class LongFieldValue extends NumericFieldValue { + private static class Factory extends PrimitiveDataType.Factory { public FieldValue create() { return new LongFieldValue(); @@ -148,4 +149,5 @@ public final class LongFieldValue extends NumericFieldValue { return 0; } } + } diff --git a/document/src/main/java/com/yahoo/document/datatypes/Raw.java b/document/src/main/java/com/yahoo/document/datatypes/Raw.java index 23ed0cee23e..7900615aa93 100644 --- a/document/src/main/java/com/yahoo/document/datatypes/Raw.java +++ b/document/src/main/java/com/yahoo/document/datatypes/Raw.java @@ -15,7 +15,7 @@ import java.nio.ByteBuffer; import java.util.Arrays; /** - * FieldValue which encapsulates a Raw value + * A field value which is an array of byte data * * @author Einar M R Rosenvinge */ diff --git a/document/src/main/java/com/yahoo/document/datatypes/ReferenceFieldValue.java b/document/src/main/java/com/yahoo/document/datatypes/ReferenceFieldValue.java index 0097e5c93d0..034167b1121 100644 --- a/document/src/main/java/com/yahoo/document/datatypes/ReferenceFieldValue.java +++ b/document/src/main/java/com/yahoo/document/datatypes/ReferenceFieldValue.java @@ -14,21 +14,20 @@ import java.util.Objects; import java.util.Optional; /** - * A reference field value allows search queries to access fields in other document instances + * <p>A reference field value allows search queries to access fields in other document instances * as if they were fields natively stored within the searched document. This allows modelling * one-to-many relations such as a parent document with many children containing references - * back to the parent. + * back to the parent.</p> * - * Each <code>ReferenceFieldValue</code> may contain a single document ID which specifies the + * <p>Each <code>ReferenceFieldValue</code> may contain a single document ID which specifies the * instance the field should refer to. This document ID must have a type matching that of the - * reference data type of the field itself. + * reference data type of the field itself.</p> * - * Note that references are not polymorphic. This means that if you have a document type + * <p>Note that references are not polymorphic. This means that if you have a document type * "foo" inheriting "bar", you cannot have a <code>reference<bar></code> field containing - * a document ID for a "foo" document. + * a document ID for a "foo" document.</p> * * @author vekterli - * @since 6.65 */ public class ReferenceFieldValue extends FieldValue { diff --git a/document/src/main/java/com/yahoo/document/datatypes/Struct.java b/document/src/main/java/com/yahoo/document/datatypes/Struct.java index 7d4e615b27b..fc75870bb94 100644 --- a/document/src/main/java/com/yahoo/document/datatypes/Struct.java +++ b/document/src/main/java/com/yahoo/document/datatypes/Struct.java @@ -12,9 +12,7 @@ import com.yahoo.vespa.objects.Ids; import java.util.*; /** - * Date: Apr 15, 2008 - * - * @author humbe + * @author Håkon Humberset */ public class Struct extends StructuredFieldValue { diff --git a/document/src/main/java/com/yahoo/document/datatypes/UriFieldValue.java b/document/src/main/java/com/yahoo/document/datatypes/UriFieldValue.java index 13688c8e311..1b51d6625e2 100644 --- a/document/src/main/java/com/yahoo/document/datatypes/UriFieldValue.java +++ b/document/src/main/java/com/yahoo/document/datatypes/UriFieldValue.java @@ -10,12 +10,10 @@ import com.yahoo.net.Url; import java.net.URI; /** - * Created with IntelliJ IDEA. - * User: magnarn - * Date: 11/2/12 - * Time: 2:37 PM + * @author Magnar Nedland */ public class UriFieldValue extends StringFieldValue { + public static class Factory extends PrimitiveDataType.Factory { public FieldValue create() { return new UriFieldValue(); @@ -46,4 +44,5 @@ public class UriFieldValue extends StringFieldValue { super.deserialize(field, reader); Url.fromString(toString()); // Throws if value is invalid. } + } diff --git a/document/src/main/java/com/yahoo/document/serialization/XmlSerializationHelper.java b/document/src/main/java/com/yahoo/document/serialization/XmlSerializationHelper.java index 941dbc8d406..a0e7f81ea73 100644 --- a/document/src/main/java/com/yahoo/document/serialization/XmlSerializationHelper.java +++ b/document/src/main/java/com/yahoo/document/serialization/XmlSerializationHelper.java @@ -7,7 +7,6 @@ import com.yahoo.document.datatypes.*; import com.yahoo.text.Utf8; import org.apache.commons.codec.binary.Base64; -import java.io.UnsupportedEncodingException; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -67,6 +66,14 @@ public class XmlSerializationHelper { xml.addContent(f.toString()); } + public static void printShortfloatXml(Float16FieldValue f, XmlStream xml) { + xml.addContent(f.toString()); + } + + public static void printBoolXml(BoolFieldValue f, XmlStream xml) { + xml.addContent(f.toString()); + } + public static void printIntegerXml(IntegerFieldValue f, XmlStream xml) { xml.addContent(f.toString()); } diff --git a/document/src/test/java/com/yahoo/document/datatypes/StringTestCase.java b/document/src/test/java/com/yahoo/document/datatypes/StringTestCase.java index ffc372d4851..e9fce35ed01 100644 --- a/document/src/test/java/com/yahoo/document/datatypes/StringTestCase.java +++ b/document/src/test/java/com/yahoo/document/datatypes/StringTestCase.java @@ -20,9 +20,6 @@ import com.yahoo.io.GrowableByteBuffer; import com.yahoo.vespa.objects.BufferSerializer; import org.junit.Test; -import java.util.Collection; -import java.util.Iterator; -import java.util.ListIterator; import java.util.Map; import static org.junit.Assert.assertEquals; @@ -31,12 +28,12 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; /** - * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a> + * @author Einar M R Rosenvinge */ public class StringTestCase extends AbstractTypesTest { @Test - public void testDeserialize() throws Exception { + public void testDeserialize() { byte[] buf = new byte[1500]; GrowableByteBuffer data = GrowableByteBuffer.wrap(buf); //short string @@ -122,17 +119,8 @@ public class StringTestCase extends AbstractTypesTest { assertTrue(blah.equals(blah2)); } -/* public void testSpanTree() { - StringFieldValue annotatedText = new StringFieldValue("banana airlines"); - SpanList lingTree = new SpanList(); - annotatedText.setSpanTree("linguistics", lingTree); - for (Annotation anAnnotation : annotatedText.getSpanTree("linguistics")) { - System.err.println(anAnnotation); - } - }*/ - @Test - public void testSerializeDeserialize() throws Exception { + public void testSerializeDeserialize() { java.lang.String test = "Hello hello"; BufferSerializer data = new BufferSerializer(new GrowableByteBuffer(100, 2.0f)); StringFieldValue value = new StringFieldValue(test); @@ -171,7 +159,7 @@ public class StringTestCase extends AbstractTypesTest { } @Test - public void testNestedSpanTreeBug4187377() { + public void testNestedSpanTree() { AnnotationType type = new AnnotationType("ann", DataType.STRING); StringFieldValue outerString = new StringFieldValue("Ballooo"); @@ -224,8 +212,6 @@ public class StringTestCase extends AbstractTypesTest { doc = annotate(doc, manager); doc = serializeAndDeserialize(doc, manager); - doc = consume(doc, manager); - System.err.println(doc); } private Document serializeAndDeserialize(Document doc, DocumentTypeManager manager) { @@ -247,11 +233,6 @@ public class StringTestCase extends AbstractTypesTest { AnnotationType location = registry.getType("location"); Map<String, AnnotationType> m = registry.getTypes(); - for (String key : m.keySet()) { - System.out.println("Key: " + key); - AnnotationType val = m.get(key); - parseAnnotationType(val); - } SpanTree tree = new SpanTree("testannotations"); SpanList root = (SpanList)tree.getRoot(); @@ -335,7 +316,7 @@ public class StringTestCase extends AbstractTypesTest { root.remove(branch); tree.cleanup(); - System.out.println("No. Of Annotations: " + tree.numAnnotations()); + assertEquals(5, tree.numAnnotations()); body.setSpanTree(tree); document.setFieldValue(document.getField("body"), body); @@ -343,93 +324,4 @@ public class StringTestCase extends AbstractTypesTest { return document; } - @SuppressWarnings("deprecation") - public Document consume(Document document, DocumentTypeManager docTypeMgr) { - DocumentType type = docTypeMgr.getDocumentType("blog"); - Collection<Field> fc = type.getFields(); - for (Field f : fc) { - System.out.println("\n\nField Name: " + f.getName()); - System.out.println("DataType: " + f.getDataType()); - System.out.println("isHeader? " + f.isHeader()); - - FieldValue val = document.getFieldValue(f); - if (val instanceof StringFieldValue) { - StringFieldValue sfv = (StringFieldValue)val; - System.out.println(f.getName() + " is a StringField. Field Value: " + sfv.getString()); - Collection<SpanTree> c = sfv.getSpanTrees(); - for (SpanTree tree : c) { - System.out.println(f.getName() + " has annotations"); - consumeAnnotations(tree, (SpanList)tree.getRoot()); - } - } - } - - return document; - } - - public void consumeAnnotations(SpanTree tree, SpanList root) { - System.out.println("\n\nSpanList: " + root + " num Children: " + root.numChildren()); - System.out.println("-------------------"); - Iterator<SpanNode> childIterator = root.childIterator(); - while (childIterator.hasNext()) { - SpanNode node = childIterator.next(); - System.out.println("Span Node: " + node); // + " Span Text: " + node.getText(fieldValStr)); - if (node instanceof SpanList) { - System.out.println("Encountered another span list"); - SpanList spl = (SpanList)node; - ListIterator<SpanNode> lli = spl.childIterator(); - while (lli.hasNext()) { - System.out.print(" " + lli.next() + " "); - } - consumeAnnotations(tree, (SpanList)node); - } else { - System.out.println("\nGetting annotations for this span node: " + node); - getAnnotationsForNode(tree, node); - } - } - System.out.println("\nGetting annotations for the SpanList itself : " + root); - getAnnotationsForNode(tree, root); - } - - public void getAnnotationsForNode(SpanTree tree, SpanNode node) { - Iterator<Annotation> iter = tree.iterator(node); - boolean annotationPresent = false; - while (iter.hasNext()) { - annotationPresent = true; - Annotation xx = iter.next(); - Struct fValue = (Struct)xx.getFieldValue(); - System.out.println("Annotation: " + xx); - if (fValue == null) { - System.out.println("Field Value is null"); - return; - } - Iterator fieldIter = fValue.iterator(); - while (fieldIter.hasNext()) { - Map.Entry m = (Map.Entry)fieldIter.next(); - Field f = (Field)m.getKey(); - FieldValue val = (FieldValue)m.getValue(); - System.out.println("Field : " + f + " Value: " + val); - } - } - if (!annotationPresent) { - System.out.println("****No annotations found for the span node: " + node); - } - } - - public void parseAnnotationType(AnnotationType t) { - System.out.println("Type Name: " + t.getName()); - System.out.println("Type ID: " + t.getId()); - DataType dt = t.getDataType(); - String dataTypeStr; - if (dt == DataType.STRING) { - dataTypeStr = "String"; - } else if (dt == DataType.INT) { - dataTypeStr = "Integer"; - } else if (dt == DataType.URI) { - dataTypeStr = "URL"; - } else { - dataTypeStr = "UNKNOWN"; - } - System.out.println("Type DataType: " + dataTypeStr); - } } diff --git a/document/src/test/java/com/yahoo/document/json/JsonReaderTestCase.java b/document/src/test/java/com/yahoo/document/json/JsonReaderTestCase.java index 32f63e6c0b3..b3d502ea56f 100644 --- a/document/src/test/java/com/yahoo/document/json/JsonReaderTestCase.java +++ b/document/src/test/java/com/yahoo/document/json/JsonReaderTestCase.java @@ -22,6 +22,7 @@ import com.yahoo.document.StructDataType; import com.yahoo.document.TensorDataType; import com.yahoo.document.WeightedSetDataType; import com.yahoo.document.datatypes.Array; +import com.yahoo.document.datatypes.BoolFieldValue; import com.yahoo.document.datatypes.FieldValue; import com.yahoo.document.datatypes.IntegerFieldValue; import com.yahoo.document.datatypes.MapFieldValue; @@ -97,6 +98,7 @@ public class JsonReaderTestCase { x.addField(new Field("something", DataType.STRING)); x.addField(new Field("nalle", DataType.STRING)); x.addField(new Field("int1", DataType.INT)); + x.addField(new Field("flag", DataType.BOOL)); types.registerDocumentType(x); } { @@ -172,13 +174,20 @@ public class JsonReaderTestCase { } @Test - public final void readSingleDocumentPut() { - InputStream rawDoc = new ByteArrayInputStream( - Utf8.toBytes("{\"put\": \"id:unittest:smoke::whee\"," - + " \"fields\": { \"something\": \"smoketest\"," - + " \"nalle\": \"bamse\"}}")); + public void readSingleDocumentPut() { + String doc = + "{ \"put\": \"id:unittest:smoke::doc1\"," + + " \"fields\": { " + + " \"something\": \"smoketest\"," + + " \"flag\": true," + + " \"nalle\": \"bamse\"" + + " } " + + "}"; + + InputStream rawDoc = new ByteArrayInputStream(Utf8.toBytes(doc)); JsonReader r = new JsonReader(types, rawDoc, parserFactory); - DocumentPut put = (DocumentPut) r.readSingleDocument(DocumentParser.SupportedOperation.PUT, "id:unittest:smoke::whee"); + DocumentPut put = (DocumentPut) r.readSingleDocument(DocumentParser.SupportedOperation.PUT, + "id:unittest:smoke::doc1"); smokeTestDoc(put.getDocument()); } @@ -196,7 +205,7 @@ public class JsonReaderTestCase { } @Test - public final void readClearField() { + public void readClearField() { InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("{\"update\": \"id:unittest:smoke::whee\"," + " \"fields\": { \"int1\": {" @@ -211,11 +220,17 @@ public class JsonReaderTestCase { @Test - public final void smokeTest() throws IOException { - InputStream rawDoc = new ByteArrayInputStream( - Utf8.toBytes("{\"put\": \"id:unittest:smoke::whee\"," - + " \"fields\": { \"something\": \"smoketest\"," - + " \"nalle\": \"bamse\"}}")); + public void smokeTest() throws IOException { + String doc = + "{ \"put\": \"id:unittest:smoke::doc1\"," + + " \"fields\": { " + + " \"something\": \"smoketest\"," + + " \"flag\": true," + + " \"nalle\": \"bamse\"" + + " } " + + "}"; + + InputStream rawDoc = new ByteArrayInputStream(Utf8.toBytes(doc)); JsonReader r = new JsonReader(types, rawDoc, parserFactory); DocumentParseInfo parseInfo = r.parseDocument().get(); DocumentType docType = r.readDocumentType(parseInfo.documentId); @@ -225,13 +240,18 @@ public class JsonReaderTestCase { } @Test - public final void docIdLookaheadTest() throws IOException { - InputStream rawDoc = new ByteArrayInputStream( - Utf8.toBytes("{" - + " \"fields\": { \"something\": \"smoketest\"," - + " \"nalle\": \"bamse\"}," - + "\"put\": \"id:unittest:smoke::whee\"" - + "}")); + public void docIdLookaheadTest() throws IOException { + String doc = + "{ \"fields\": { " + + " \"something\": \"smoketest\"," + + " \"flag\": true," + + " \"nalle\": \"bamse\"" + + " }," + + " \"put\": \"id:unittest:smoke::doc1\"" + + " } " + + "}"; + + InputStream rawDoc = new ByteArrayInputStream(Utf8.toBytes(doc)); JsonReader r = new JsonReader(types, rawDoc, parserFactory); DocumentParseInfo parseInfo = r.parseDocument().get(); DocumentType docType = r.readDocumentType(parseInfo.documentId); @@ -242,10 +262,10 @@ public class JsonReaderTestCase { @Test - public final void emptyDocTest() throws IOException { + public void emptyDocTest() throws IOException { InputStream rawDoc = new ByteArrayInputStream( - Utf8.toBytes("{\"put\": \"id:unittest:smoke::whee\"," - + " \"fields\": {}}")); + Utf8.toBytes("{\"put\": \"id:unittest:smoke::whee\"," + + " \"fields\": {}}")); JsonReader r = new JsonReader(types, rawDoc, parserFactory); DocumentParseInfo parseInfo = r.parseDocument().get(); DocumentType docType = r.readDocumentType(parseInfo.documentId); @@ -255,7 +275,7 @@ public class JsonReaderTestCase { } @Test - public final void testStruct() throws IOException { + public void testStruct() throws IOException { InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("{\"put\": \"id:unittest:mirrors::whee\"," + " \"fields\": { " @@ -285,7 +305,7 @@ public class JsonReaderTestCase { } @Test - public final void testStructUpdate() throws IOException { + public void testStructUpdate() throws IOException { DocumentUpdate put = parseUpdate("{\"update\": \"id:unittest:mirrors:g=test:whee\"," + "\"create\": true," + " \"fields\": { " @@ -331,7 +351,7 @@ public class JsonReaderTestCase { } @Test - public final void testUpdateArray() throws IOException { + public void testUpdateArray() throws IOException { DocumentUpdate doc = parseUpdate("{\"update\": \"id:unittest:testarray::whee\"," + " \"fields\": { " + "\"actualarray\": {" + " \"add\": [" @@ -341,7 +361,7 @@ public class JsonReaderTestCase { } @Test - public final void testUpdateWeighted() throws IOException { + public void testUpdateWeighted() throws IOException { DocumentUpdate doc = parseUpdate("{\"update\": \"id:unittest:testset::whee\"," + " \"fields\": { " + "\"actualset\": {" + " \"add\": {" @@ -365,7 +385,7 @@ public class JsonReaderTestCase { } @Test - public final void testUpdateMatch() throws IOException { + public void testUpdateMatch() throws IOException { DocumentUpdate doc = parseUpdate("{\"update\": \"id:unittest:testset::whee\"," + " \"fields\": { " + "\"actualset\": {" + " \"match\": {" @@ -391,7 +411,7 @@ public class JsonReaderTestCase { @SuppressWarnings({ "cast", "unchecked", "rawtypes" }) @Test - public final void testArithmeticOperators() throws IOException { + public void testArithmeticOperators() throws IOException { Tuple2[] operations = new Tuple2[] { new Tuple2<String, Operator>(UPDATE_DECREMENT, ArithmeticValueUpdate.Operator.SUB), @@ -428,7 +448,7 @@ public class JsonReaderTestCase { @SuppressWarnings("rawtypes") @Test - public final void testArrayIndexing() throws IOException { + public void testArrayIndexing() throws IOException { DocumentUpdate doc = parseUpdate("{\"update\": \"id:unittest:testarray::whee\"," + " \"fields\": { " + "\"actualarray\": {" + " \"match\": {" @@ -451,7 +471,7 @@ public class JsonReaderTestCase { } @Test - public final void testDocumentRemove() { + public void testDocumentRemove() { InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("{\"remove\": \"id:unittest:smoke::whee\"" + " }}")); @@ -461,7 +481,7 @@ public class JsonReaderTestCase { } @Test - public final void testWeightedSet() throws IOException { + public void testWeightedSet() throws IOException { InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("{\"put\": \"id:unittest:testset::whee\"," + " \"fields\": { \"actualset\": {" @@ -482,7 +502,7 @@ public class JsonReaderTestCase { } @Test - public final void testArray() throws IOException { + public void testArray() throws IOException { InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("{\"put\": \"id:unittest:testarray::whee\"," + " \"fields\": { \"actualarray\": [" @@ -503,7 +523,7 @@ public class JsonReaderTestCase { } @Test - public final void testMap() throws IOException { + public void testMap() throws IOException { InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("{\"put\": \"id:unittest:testmap::whee\"," + " \"fields\": { \"actualmap\": {" @@ -523,7 +543,7 @@ public class JsonReaderTestCase { } @Test - public final void testOldMap() throws IOException { + public void testOldMap() throws IOException { InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("{\"put\": \"id:unittest:testmap::whee\"," + " \"fields\": { \"actualmap\": [" @@ -544,7 +564,7 @@ public class JsonReaderTestCase { } @Test - public final void testPositionPositive() throws IOException { + public void testPositionPositive() throws IOException { InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("{\"put\": \"id:unittest:testsinglepos::bamf\"," + " \"fields\": { \"singlepos\": \"N63.429722;E10.393333\" }}")); @@ -561,7 +581,7 @@ public class JsonReaderTestCase { } @Test - public final void testPositionNegative() throws IOException { + public void testPositionNegative() throws IOException { InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("{\"put\": \"id:unittest:testsinglepos::bamf\"," + " \"fields\": { \"singlepos\": \"W46.63;S23.55\" }}")); @@ -578,7 +598,7 @@ public class JsonReaderTestCase { } @Test - public final void testRaw() throws IOException { + public void testRaw() throws IOException { String stuff = new String(new JsonStringEncoder().quoteAsString(new Base64().encodeToString(Utf8.toBytes("smoketest")))); InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("{\"put\": \"id:unittest:testraw::whee\"," @@ -600,7 +620,7 @@ public class JsonReaderTestCase { } @Test - public final void testMapStringToArrayOfInt() throws IOException { + public void testMapStringToArrayOfInt() throws IOException { InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("{\"put\": \"id:unittest:testMapStringToArrayOfInt::whee\"," + " \"fields\": { \"actualMapStringToArrayOfInt\": { \"bamse\": [1, 2, 3] }}}")); @@ -621,7 +641,7 @@ public class JsonReaderTestCase { } @Test - public final void testOldMapStringToArrayOfInt() throws IOException { + public void testOldMapStringToArrayOfInt() throws IOException { InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("{\"put\": \"id:unittest:testMapStringToArrayOfInt::whee\"," + " \"fields\": { \"actualMapStringToArrayOfInt\": [" @@ -644,7 +664,7 @@ public class JsonReaderTestCase { } @Test - public final void testAssignToString() throws IOException { + public void testAssignToString() throws IOException { DocumentUpdate doc = parseUpdate("{\"update\": \"id:unittest:smoke::whee\"," + " \"fields\": { \"something\": {" + " \"assign\": \"orOther\" }}" + " }"); @@ -655,7 +675,7 @@ public class JsonReaderTestCase { } @Test - public final void testAssignToArray() throws IOException { + public void testAssignToArray() throws IOException { DocumentUpdate doc = parseUpdate("{\"update\": \"id:unittest:testMapStringToArrayOfInt::whee\"," + " \"fields\": { \"actualMapStringToArrayOfInt\": {" + " \"assign\": { \"bamse\": [1, 2, 3] }}}}"); @@ -671,7 +691,7 @@ public class JsonReaderTestCase { } @Test - public final void testOldAssignToArray() throws IOException { + public void testOldAssignToArray() throws IOException { DocumentUpdate doc = parseUpdate("{\"update\": \"id:unittest:testMapStringToArrayOfInt::whee\"," + " \"fields\": { \"actualMapStringToArrayOfInt\": {" + " \"assign\": [" @@ -689,7 +709,7 @@ public class JsonReaderTestCase { } @Test - public final void testAssignToWeightedSet() throws IOException { + public void testAssignToWeightedSet() throws IOException { DocumentUpdate doc = parseUpdate("{\"update\": \"id:unittest:testset::whee\"," + " \"fields\": { " + "\"actualset\": {" + " \"assign\": {" @@ -706,10 +726,11 @@ public class JsonReaderTestCase { @Test - public final void testCompleteFeed() { + public void testCompleteFeed() { InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("[{\"put\": \"id:unittest:smoke::whee\"," + " \"fields\": { \"something\": \"smoketest\"," + + " \"flag\": true," + " \"nalle\": \"bamse\"}}" + ", " + "{\"update\": \"id:unittest:testarray::whee\"," + " \"fields\": { " + "\"actualarray\": {" @@ -722,10 +743,11 @@ public class JsonReaderTestCase { } @Test - public final void testCompleteFeedWithCreateAndCondition() { + public void testCompleteFeedWithCreateAndCondition() { InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("[{\"put\": \"id:unittest:smoke::whee\"," + " \"fields\": { \"something\": \"smoketest\"," + + " \"flag\": true," + " \"nalle\": \"bamse\"}}" + ", " + "{" + "\"condition\":\"bla\"," @@ -755,14 +777,14 @@ public class JsonReaderTestCase { } @Test - public final void testUpdateWithConditionAndCreateInDifferentOrdering() { - final int documentsCreated = 106; + public void testUpdateWithConditionAndCreateInDifferentOrdering() { + int documentsCreated = 106; List<String> parts = Arrays.asList( "\"condition\":\"bla\"", "\"update\": \"id:unittest:testarray::whee\"", " \"fields\": { " + "\"actualarray\": { \"add\": [" + " \"person\",\"another person\"]}}", " \"create\":true"); - final Random random = new Random(42); + Random random = new Random(42); StringBuilder documents = new StringBuilder("["); for (int x = 0; x < documentsCreated; x++) { Collections.shuffle(parts, random); @@ -790,7 +812,7 @@ public class JsonReaderTestCase { @Test(expected=RuntimeException.class) - public final void testCreateIfNonExistentInPut() { + public void testCreateIfNonExistentInPut() { InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("[{" + " \"create\":true," @@ -803,10 +825,11 @@ public class JsonReaderTestCase { } @Test - public final void testCompleteFeedWithIdAfterFields() { + public void testCompleteFeedWithIdAfterFields() { InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("[{" + " \"fields\": { \"something\": \"smoketest\"," + + " \"flag\": true," + " \"nalle\": \"bamse\"}," + "\"put\": \"id:unittest:smoke::whee\"" + "}" + ", " @@ -840,7 +863,7 @@ public class JsonReaderTestCase { @Test - public final void testCompleteFeedWithEmptyDoc() { + public void testCompleteFeedWithEmptyDoc() { InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("[{\"put\": \"id:unittest:smoke::whee\"," + " \"fields\": {}}" + ", " @@ -879,10 +902,13 @@ public class JsonReaderTestCase { } private void smokeTestDoc(Document doc) { - FieldValue f = doc.getFieldValue(doc.getField("nalle")); - assertSame(StringFieldValue.class, f.getClass()); - StringFieldValue s = (StringFieldValue) f; - assertEquals("bamse", s.getString()); + FieldValue boolField = doc.getFieldValue(doc.getField("flag")); + assertSame(BoolFieldValue.class, boolField.getClass()); + assertTrue((Boolean)boolField.getWrappedValue()); + + FieldValue stringField = doc.getFieldValue(doc.getField("nalle")); + assertSame(StringFieldValue.class, stringField.getClass()); + assertEquals("bamse", ((StringFieldValue) stringField).getString()); } @Test @@ -901,7 +927,7 @@ public class JsonReaderTestCase { } @Test - public final void feedWithBasicErrorTest() { + public void feedWithBasicErrorTest() { InputStream rawDoc = new ByteArrayInputStream( Utf8.toBytes("[" + " { \"put\": \"id:test:smoke::0\", \"fields\": { \"something\": \"foo\" } }," @@ -915,10 +941,11 @@ public class JsonReaderTestCase { } @Test - public final void idAsAliasForPutTest() throws IOException{ + public void idAsAliasForPutTest() throws IOException{ InputStream rawDoc = new ByteArrayInputStream( - Utf8.toBytes("{\"id\": \"id:unittest:smoke::whee\"," + Utf8.toBytes("{\"id\": \"id:unittest:smoke::doc1\"," + " \"fields\": { \"something\": \"smoketest\"," + + " \"flag\": true," + " \"nalle\": \"bamse\"}}")); JsonReader r = new JsonReader(types, rawDoc, parserFactory); DocumentParseInfo parseInfo = r.parseDocument().get(); @@ -948,7 +975,7 @@ public class JsonReaderTestCase { } @Test - public final void testFeedWithTestAndSetConditionOrderingOne() { + public void testFeedWithTestAndSetConditionOrderingOne() { testFeedWithTestAndSetCondition( inputJson("[", " {", diff --git a/documentapi/src/main/java/com/yahoo/documentapi/Session.java b/documentapi/src/main/java/com/yahoo/documentapi/Session.java index 8ab098397de..3f296723079 100644 --- a/documentapi/src/main/java/com/yahoo/documentapi/Session.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/Session.java @@ -18,7 +18,7 @@ public interface Session { * * @return the next response, or null if no response is ready at this time */ - public Response getNext(); + Response getNext(); /** * Returns the next response of this session. This will block until a response is ready @@ -28,12 +28,12 @@ public interface Session { * @return the next response, or null if no response becomes ready before the timeout expires * @throws InterruptedException if this thread is interrupted while waiting */ - public Response getNext(int timeoutMilliseconds) throws InterruptedException; + Response getNext(int timeoutMilliseconds) throws InterruptedException; /** * Destroys this session and frees up any resources it has held. Making further calls on a destroyed * session causes a runtime exception. */ - public void destroy(); + void destroy(); } diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusAsyncSession.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusAsyncSession.java index eb71b3cfe47..8a6fa85c68b 100644 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusAsyncSession.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusAsyncSession.java @@ -32,7 +32,7 @@ public class MessageBusAsyncSession implements MessageBusSession, AsyncSession { private static final Logger log = Logger.getLogger(MessageBusAsyncSession.class.getName()); private final AtomicLong requestId = new AtomicLong(0); - private final BlockingQueue<Response> responses = new LinkedBlockingQueue<Response>(); + private final BlockingQueue<Response> responses = new LinkedBlockingQueue<>(); private final ThrottlePolicy throttlePolicy; private final SourceSession session; private String route; @@ -120,8 +120,8 @@ public class MessageBusAsyncSession implements MessageBusSession, AsyncSession { * A convenience method for assigning the internal trace level and route string to a message before sending it * through the internal mbus session object. * - * @param msg The message to send. - * @return The document api result object. + * @param msg the message to send. + * @return the document api result object. */ public Result send(Message msg) { try { @@ -292,4 +292,5 @@ public class MessageBusAsyncSession implements MessageBusSession, AsyncSession { } } } + } diff --git a/jrt/src/com/yahoo/jrt/MaybeTlsCryptoEngine.java b/jrt/src/com/yahoo/jrt/MaybeTlsCryptoEngine.java index e4355ae793a..d888690c39c 100644 --- a/jrt/src/com/yahoo/jrt/MaybeTlsCryptoEngine.java +++ b/jrt/src/com/yahoo/jrt/MaybeTlsCryptoEngine.java @@ -9,7 +9,7 @@ import java.nio.channels.SocketChannel; * auto-detected using clever heuristics. The use of tls for outgoing * connections is controlled by the useTlsWhenClient flag given to the * constructor. - **/ + */ public class MaybeTlsCryptoEngine implements CryptoEngine { private final TlsCryptoEngine tlsEngine; @@ -20,7 +20,8 @@ public class MaybeTlsCryptoEngine implements CryptoEngine { this.useTlsWhenClient = useTlsWhenClient; } - @Override public CryptoSocket createCryptoSocket(SocketChannel channel, boolean isServer) { + @Override + public CryptoSocket createCryptoSocket(SocketChannel channel, boolean isServer) { if (isServer) { return new MaybeTlsCryptoSocket(channel, tlsEngine); } else if (useTlsWhenClient) { @@ -30,7 +31,8 @@ public class MaybeTlsCryptoEngine implements CryptoEngine { } } - @Override public String toString() { return "MaybeTlsCryptoEngine(useTlsWhenClient:" + useTlsWhenClient + ")"; } + @Override + public String toString() { return "MaybeTlsCryptoEngine(useTlsWhenClient:" + useTlsWhenClient + ")"; } @Override public void close() { diff --git a/messagebus/src/main/java/com/yahoo/messagebus/Routable.java b/messagebus/src/main/java/com/yahoo/messagebus/Routable.java index b797c103e67..e761a874ac1 100755 --- a/messagebus/src/main/java/com/yahoo/messagebus/Routable.java +++ b/messagebus/src/main/java/com/yahoo/messagebus/Routable.java @@ -69,41 +69,27 @@ public abstract class Routable { return callStack.pop(this); } - /** - * Return the context of this routable. - * - * @return The context. - */ + /** Returns the context of this routable. */ public Object getContext() { return context; } /** - * Set a new context for this routable. Please note that the context is <u>not</u> something that is passed along a + * Sets a new context for this routable. Please note that the context is <u>not</u> something that is passed along a * message, it is simply a user context for the handler currently manipulating a message. When the corresponding * reply reaches the registered reply handler, its content will be the same as that of the outgoing message. More * technically, this context is contained in the callstack of a routable. - * - * @param context The new context. */ public void setContext(Object context) { this.context = context; } - /** - * Return the callstack of this routable. - * - * @return The callstack. - */ + /** Returns the callstack of this routable. */ public CallStack getCallStack() { return callStack; } - /** - * Returns the trace object of this routable. - * - * @return The trace object. - */ + /** Returns the trace object of this routable. */ public Trace getTrace() { return trace; } @@ -112,15 +98,14 @@ public abstract class Routable { * Return the name of the protocol that defines this routable. This must be implemented by all inheriting classes, * and should then return the result of {@link com.yahoo.messagebus.Protocol#getName} of its protocol. * - * @return The name of the protocol defining this message. + * @return the name of the protocol defining this message. */ public abstract Utf8String getProtocol(); /** - * Obtain the type of this routable. The id '0' is reserved for the EmptyReply class. Other ids must be defined by + * Returns the type of this routable. The id '0' is reserved for the EmptyReply class. Other ids must be defined by * the application protocol. - * - * @return The message type. */ public abstract int getType(); + } diff --git a/messagebus/src/main/java/com/yahoo/messagebus/SendProxy.java b/messagebus/src/main/java/com/yahoo/messagebus/SendProxy.java index ecc6bfebc41..df60fc5bdbf 100644 --- a/messagebus/src/main/java/com/yahoo/messagebus/SendProxy.java +++ b/messagebus/src/main/java/com/yahoo/messagebus/SendProxy.java @@ -90,4 +90,5 @@ public class SendProxy implements MessageHandler, ReplyHandler { handler.handleReply(reply); } } + } diff --git a/messagebus/src/main/java/com/yahoo/messagebus/Sequencer.java b/messagebus/src/main/java/com/yahoo/messagebus/Sequencer.java index b804c776969..35218364c8f 100644 --- a/messagebus/src/main/java/com/yahoo/messagebus/Sequencer.java +++ b/messagebus/src/main/java/com/yahoo/messagebus/Sequencer.java @@ -103,7 +103,7 @@ public class Sequencer implements MessageHandler, ReplyHandler { * sequencing-id, it is simply passed through to the next handler in the chain. Sequenced messages are sent only if * there is no queue for their id, otherwise they are queued. * - * @param msg The message to send. + * @param msg the message to send. */ @Override public void handleMessage(Message msg) { diff --git a/messagebus/src/main/java/com/yahoo/messagebus/SourceSession.java b/messagebus/src/main/java/com/yahoo/messagebus/SourceSession.java index 3bfdfaa1f4a..4fdfa341e3b 100644 --- a/messagebus/src/main/java/com/yahoo/messagebus/SourceSession.java +++ b/messagebus/src/main/java/com/yahoo/messagebus/SourceSession.java @@ -1,25 +1,22 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.messagebus; -import com.yahoo.log.LogLevel; import com.yahoo.messagebus.routing.Route; import com.yahoo.messagebus.routing.RoutingTable; -import com.yahoo.text.Utf8String; -import java.util.*; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.Queue; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.logging.Logger; /** - * <p>A session supporting sending new messages.</p> + * A session supporting sending new messages. * * @author Simon Thoresen Hult */ public final class SourceSession implements ReplyHandler, MessageBus.SendBlockedMessages { - private static Logger log = Logger.getLogger(SourceSession.class.getName()); private final AtomicBoolean destroyed = new AtomicBoolean(false); private final CountDownLatch done = new CountDownLatch(1); private final Object lock = new Object(); @@ -116,11 +113,12 @@ public final class SourceSession implements ReplyHandler, MessageBus.SendBlocked * </code> * * @param msg the message to send - * @return The result of <i>initiating</i> sending of this message. + * @return the result of <i>initiating</i> sending of this message */ public Result send(Message msg) { return sendInternal(updateTiming(msg)); } + private Message updateTiming(Message msg) { msg.setTimeReceivedNow(); if (msg.getTimeRemaining() <= 0) { @@ -128,13 +126,14 @@ public final class SourceSession implements ReplyHandler, MessageBus.SendBlocked } return msg; } + private Result sendInternal(Message msg) { synchronized (lock) { if (closed) { return new Result(ErrorCode.SEND_QUEUE_CLOSED, "Source session is closed."); } - if (throttlePolicy != null && !throttlePolicy.canSend(msg, pendingCount)) { + if (throttlePolicy != null && ! throttlePolicy.canSend(msg, pendingCount)) { return new Result(ErrorCode.SEND_QUEUE_FULL, "Too much pending data (" + pendingCount + " messages)."); } @@ -247,7 +246,7 @@ public final class SourceSession implements ReplyHandler, MessageBus.SendBlocked private void expireStalledBlockedMessages() { synchronized (lock) { - final Iterator<BlockedMessage> each = blockedQ.iterator(); + Iterator<BlockedMessage> each = blockedQ.iterator(); while (each.hasNext()) { if (each.next().notifyIfExpired()) { each.remove(); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java index 852d47a4882..8b66b7f3fea 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java @@ -81,7 +81,6 @@ public class NodeRepository extends AbstractComponent { private final NameResolver nameResolver; private final DockerImage dockerImage; private final OsVersions osVersions; - private final Flags flags; /** * Creates a node repository from a zookeeper provider. @@ -98,28 +97,19 @@ public class NodeRepository extends AbstractComponent { */ public NodeRepository(NodeFlavors flavors, Curator curator, Clock clock, Zone zone, NameResolver nameResolver, DockerImage dockerImage, boolean useCuratorClientCache) { - this.db = new CuratorDatabaseClient(flavors, curator, clock, zone, useCacheIn(zone, useCuratorClientCache)); + this.db = new CuratorDatabaseClient(flavors, curator, clock, zone, useCuratorClientCache); this.zone = zone; this.clock = clock; this.flavors = flavors; this.nameResolver = nameResolver; this.dockerImage = dockerImage; this.osVersions = new OsVersions(this.db); - this.flags = new Flags(this.db); // read and write all nodes to make sure they are stored in the latest version of the serialized format for (Node.State state : Node.State.values()) db.writeTo(state, db.getNodes(state), Agent.system, Optional.empty()); } - private static boolean useCacheIn(Zone zone, boolean useCache) { - if (zone.region().value().equals("cd-us-central-1")) { - // TODO: Temporarily disabled in CD to see if allocation conflict is related to caching - return false; - } - return useCache; - } - /** Returns the curator database client used by this */ public CuratorDatabaseClient database() { return db; } @@ -134,7 +124,7 @@ public class NodeRepository extends AbstractComponent { /** Returns feature flags of this node repository */ public Flags flags() { - return flags; + return db.flags(); } // ---------------- Query API ---------------------------------------------------------------- diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/flag/FlagId.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/flag/FlagId.java index 1b798c14588..f0714111cea 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/flag/FlagId.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/flag/FlagId.java @@ -11,7 +11,10 @@ import java.util.Arrays; public enum FlagId { /** Indicates whether a exclusive load balancer should be provisioned */ - exclusiveLoadBalancer("exclusive-load-balancer"); + exclusiveLoadBalancer("exclusive-load-balancer"), + + /** Temporary. Indicates whether to use the new cache generation counting, or the old one (with a known bug) */ + newCacheCounting("new-cache-counting"); private final String serializedValue; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Maintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Maintainer.java index 89f77d02b6e..f5576ae00fc 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Maintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Maintainer.java @@ -35,7 +35,7 @@ public abstract class Maintainer extends AbstractComponent implements Runnable { this.jobControl = jobControl; HostName hostname = HostName.from(com.yahoo.net.HostName.getLocalhost()); - long delay = staggeredDelay(nodeRepository.database().curator().cluster(), hostname, nodeRepository.clock().instant(), interval); + long delay = staggeredDelay(nodeRepository.database().cluster(), hostname, nodeRepository.clock().instant(), interval); service = new ScheduledThreadPoolExecutor(1); service.scheduleAtFixedRate(this, delay, interval.toMillis(), TimeUnit.MILLISECONDS); jobControl.started(name()); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CountingCuratorTransaction.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CountingCuratorTransaction.java new file mode 100644 index 00000000000..de803281a87 --- /dev/null +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CountingCuratorTransaction.java @@ -0,0 +1,61 @@ +package com.yahoo.vespa.hosted.provision.persistence; + +import com.yahoo.path.Path; +import com.yahoo.vespa.curator.Curator; +import com.yahoo.vespa.curator.Lock; +import com.yahoo.vespa.curator.recipes.CuratorCounter; +import com.yahoo.vespa.curator.transaction.CuratorOperation; +import com.yahoo.vespa.curator.transaction.CuratorTransaction; +import com.yahoo.vespa.curator.transaction.TransactionChanges; + +/** + * CuratorTransaction wrapper which increments a counter, to signal invalidation of node repository caches. + * + * This class ensures a CuratorTransaction against the cached data (the node repository data) is + * accompanied by an increment of the data generation counter. An increment must occur <em>after</em> + * the write has completed, successfully or not. It is therefore placed in a {@code finally} block, + * wrapping the super class' {@link #commit()}. + * Likewise, {@link #prepare()} is also wrapped with an increment, in case it fails due to an inconsistent cache. + * The cache is invalid whenever the generation counter is higher than what the cache contents were read with. + * The usual locking for modifications of shared data is then enough to ensure the cache provides a + * consistent view of the shared data, with one exception: when incrementing the counter fails. This is + * assumed to be extremely rare, and the consequence is temporary neglect of cache invalidation. + * + * @author jonmv + */ +class CountingCuratorTransaction extends CuratorTransaction { + + private final CuratorCounter counter; + + public CountingCuratorTransaction(Curator curator, CuratorCounter counter) { + super(curator); + this.counter = counter; + } + + @Override + public void prepare() { + try { + counter.get(); + super.prepare(); + } + finally { + counter.next(); + } + } + + @Override + public void commit() { + try { + super.commit(); + } + finally { + counter.next(); + } + } + + @Override + public String toString() { + return "(" + super.toString() + "), INCREMENT " + counter; + } + +} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabase.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabase.java index d8c1323151d..dd1b58dee8e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabase.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabase.java @@ -23,9 +23,10 @@ import java.util.stream.Collectors; * This encapsulated the curator database of the node repo. * It serves reads from an in-memory cache of the content which is invalidated when changed on another node * using a global, shared counter. The counter is updated on all write operations, ensured by wrapping write - * operations in a 2pc transaction containing the counter update. + * operations in a try block, with the counter increment in a finally block. Locks must be used to ensure consistency. * * @author bratseth + * @author jonmv */ public class CuratorDatabase { @@ -86,6 +87,15 @@ public class CuratorDatabase { * Important: It is the nested transaction which must be committed - never the curator transaction directly. */ public CuratorTransaction newCuratorTransactionIn(NestedTransaction transaction) { + // Wrap the curator transaction with an increment of the generation counter. + CountingCuratorTransaction curatorTransaction = new CountingCuratorTransaction(curator, changeGenerationCounter); + transaction.add(curatorTransaction); + return curatorTransaction; + } + + // TODO jvenstad: remove. + /** Kept for now to be able to revert to old caching behaviour. */ + CuratorTransaction newEagerCuratorTransactionIn(NestedTransaction transaction) { // Add a counting transaction first, to make sure we always invalidate the current state on any transaction commit transaction.add(new EagerCountingCuratorTransaction(changeGenerationCounter), CuratorTransaction.class); CuratorTransaction curatorTransaction = new CuratorTransaction(curator); @@ -94,60 +104,35 @@ public class CuratorDatabase { } /** Creates a path in curator and all its parents as necessary. If the path already exists this does nothing. */ - // As this operation does not depend on the prior state we do not need to increment the write counter - public void create(Path path) { + void create(Path path) { curator.create(path); + changeGenerationCounter.next(); // Increment counter to ensure getChildren sees any change. } /** Returns whether given path exists */ - public boolean exists(Path path) { + boolean exists(Path path) { return curator.exists(path); } // --------- Read operations ------------------------------------------------------------------------------- // These can read from the memory file system, which accurately mirrors the ZooKeeper content IF + // the current generation counter is the same as it was when data was put into the cache, AND + // the data to read is protected by a lock which is held now, and during any writes of the data. /** Returns the immediate, local names of the children under this node in any order */ - public List<String> getChildren(Path path) { return getCache().getChildren(path); } + List<String> getChildren(Path path) { return getCache().getChildren(path); } - public Optional<byte[]> getData(Path path) { return getCache().getData(path); } + Optional<byte[]> getData(Path path) { return getCache().getData(path); } - private static class CacheAndGeneration { - public CacheAndGeneration(CuratorDatabaseCache cache, long generation) - { - this.cache = cache; - this.generation = generation; - } - public boolean expired() { - return generation != cache.generation(); - } - public CuratorDatabaseCache validCache() { - if (expired()) { - throw new IllegalStateException("The cache has generation " + cache.generation() + - " while the root genration counter in zookeeper says " + generation + - ". That is totally unacceptable and must be a sever programming error in my close vicinity."); - } - return cache; - } - - private CuratorDatabaseCache cache; - private long generation; - } - private CacheAndGeneration getCacheSnapshot() { - return new CacheAndGeneration(cache.get(), changeGenerationCounter.get()); - } + /** Invalidates the current cache if outdated. */ private CuratorDatabaseCache getCache() { - CacheAndGeneration cacheAndGeneration = getCacheSnapshot(); - while (cacheAndGeneration.expired()) { - synchronized (cacheCreationLock) { // Prevent a race for creating new caches - cacheAndGeneration = getCacheSnapshot(); - if (cacheAndGeneration.expired()) { + if (changeGenerationCounter.get() != cache.get().generation) + synchronized (cacheCreationLock) { + while (changeGenerationCounter.get() != cache.get().generation) cache.set(newCache(changeGenerationCounter.get())); - cacheAndGeneration = getCacheSnapshot(); - } } - } - return cacheAndGeneration.validCache(); + + return cache.get(); } /** Caches must only be instantiated using this method */ @@ -174,7 +159,7 @@ public class CuratorDatabase { private final Map<Path, Optional<byte[]>> data = new ConcurrentHashMap<>(); /** Create an empty snapshot at a given generation (as an empty snapshot is a valid partial snapshot) */ - public CuratorDatabaseCache(long generation, Curator curator) { + private CuratorDatabaseCache(long generation, Curator curator) { this.generation = generation; this.curator = curator; } @@ -183,18 +168,16 @@ public class CuratorDatabase { /** * Returns the children of this path, which may be empty. - * Returns null only if it is not present in this state mirror */ public List<String> getChildren(Path path) { return children.computeIfAbsent(path, key -> ImmutableList.copyOf(curator.getChildren(path))); } /** - * Returns the content of this child - which may be empty. - * Returns null only if it is not present in this state mirror + * Returns the a copy of the content of this child - which may be empty. */ public Optional<byte[]> getData(Path path) { - return data.computeIfAbsent(path, key -> curator.getData(path).map(data -> Arrays.copyOf(data, data.length))); + return data.computeIfAbsent(path, key -> curator.getData(path)).map(data -> Arrays.copyOf(data, data.length)); } } @@ -202,7 +185,7 @@ public class CuratorDatabase { /** An implementation of the curator database cache which does no caching */ private static class DeactivatedCache extends CuratorDatabaseCache { - public DeactivatedCache(long generation, Curator curator) { super(generation, curator); } + private DeactivatedCache(long generation, Curator curator) { super(generation, curator); } @Override public List<String> getChildren(Path path) { return curator.getChildren(path); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java index fb3f801cf65..9bcfdd8b494 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java @@ -5,6 +5,7 @@ import com.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationLockException; +import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.NodeFlavors; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.Zone; @@ -19,6 +20,7 @@ import com.yahoo.vespa.curator.transaction.CuratorTransaction; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.flag.Flag; import com.yahoo.vespa.hosted.provision.flag.FlagId; +import com.yahoo.vespa.hosted.provision.flag.Flags; import com.yahoo.vespa.hosted.provision.lb.LoadBalancer; import com.yahoo.vespa.hosted.provision.lb.LoadBalancerId; import com.yahoo.vespa.hosted.provision.node.Agent; @@ -67,19 +69,17 @@ public class CuratorDatabaseClient { private final CuratorDatabase curatorDatabase; private final Clock clock; private final Zone zone; + private final Flags flags; public CuratorDatabaseClient(NodeFlavors flavors, Curator curator, Clock clock, Zone zone, boolean useCache) { this.nodeSerializer = new NodeSerializer(flavors); this.zone = zone; + this.flags = new Flags(this); this.curatorDatabase = new CuratorDatabase(curator, root, useCache); this.clock = clock; initZK(); } - public CuratorDatabase curator() { - return curatorDatabase; - } - private void initZK() { curatorDatabase.create(root); for (Node.State state : Node.State.values()) @@ -91,12 +91,20 @@ public class CuratorDatabaseClient { curatorDatabase.create(flagsRoot); } + public List<HostName> cluster() { + return curatorDatabase.cluster(); + } + + public Flags flags() { + return flags; + } + /** * Adds a set of nodes. Rollbacks/fails transaction if any node is not in the expected state. */ public List<Node> addNodesInState(List<Node> nodes, Node.State expectedState) { NestedTransaction transaction = new NestedTransaction(); - CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction); + CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction); for (Node node : nodes) { if (node.state() != expectedState) throw new IllegalArgumentException(node + " is not in the " + node.state() + " state"); @@ -131,7 +139,7 @@ public class CuratorDatabaseClient { for (Node node : nodes) { Path path = toPath(node.state(), node.hostname()); - CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction); + CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction); curatorTransaction.add(CuratorOperations.delete(path.getAbsolute())); } @@ -201,7 +209,7 @@ public class CuratorDatabaseClient { List<Node> writtenNodes = new ArrayList<>(nodes.size()); - CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction); + CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction); for (Node node : nodes) { Node newNode = new Node(node.openStackId(), node.ipAddresses(), node.ipAddressPool().asSet(), node.hostname(), node.parentHostname(), node.flavor(), @@ -360,7 +368,7 @@ public class CuratorDatabaseClient { public void writeInactiveJobs(Set<String> inactiveJobs) { NestedTransaction transaction = new NestedTransaction(); - CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction); + CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction); curatorTransaction.add(CuratorOperations.setData(inactiveJobsPath().getAbsolute(), stringSetSerializer.toJson(inactiveJobs))); transaction.commit(); @@ -380,7 +388,7 @@ public class CuratorDatabaseClient { public void writeInfrastructureVersions(Map<NodeType, Version> infrastructureVersions) { NestedTransaction transaction = new NestedTransaction(); - CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction); + CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction); curatorTransaction.add(CuratorOperations.setData(infrastructureVersionsPath().getAbsolute(), NodeTypeVersionsSerializer.toJson(infrastructureVersions))); transaction.commit(); @@ -400,7 +408,7 @@ public class CuratorDatabaseClient { public void writeOsVersions(Map<NodeType, Version> versions) { NestedTransaction transaction = new NestedTransaction(); - CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction); + CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction); curatorTransaction.add(CuratorOperations.setData(osVersionsPath().getAbsolute(), NodeTypeVersionsSerializer.toJson(versions))); transaction.commit(); @@ -441,7 +449,7 @@ public class CuratorDatabaseClient { } public void writeLoadBalancers(Collection<LoadBalancer> loadBalancers, NestedTransaction transaction) { - CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction); + CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction); loadBalancers.forEach(loadBalancer -> { curatorTransaction.add(createOrSet(loadBalancerPath(loadBalancer.id()), LoadBalancerSerializer.toJson(loadBalancer))); @@ -450,7 +458,7 @@ public class CuratorDatabaseClient { public void removeLoadBalancer(LoadBalancer loadBalancer) { NestedTransaction transaction = new NestedTransaction(); - CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction); + CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction); curatorTransaction.add(CuratorOperations.delete(loadBalancerPath(loadBalancer.id()).getAbsolute())); transaction.commit(); } @@ -466,7 +474,7 @@ public class CuratorDatabaseClient { public void writeFlag(Flag flag) { Path path = flagPath(flag.id()); NestedTransaction transaction = new NestedTransaction(); - CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction); + CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction); curatorTransaction.add(createOrSet(path, FlagSerializer.toJson(flag))); transaction.commit(); } @@ -490,4 +498,10 @@ public class CuratorDatabaseClient { return CuratorOperations.create(path.getAbsolute(), data); } + private CuratorTransaction newCuratorTransactionIn(NestedTransaction transaction) { + return flags.get(FlagId.newCacheCounting).isEnabled() + ? curatorDatabase.newCuratorTransactionIn(transaction) + : curatorDatabase.newEagerCuratorTransactionIn(transaction); + } + } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseTest.java index bf82128dfb7..149510bdc97 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseTest.java @@ -3,14 +3,17 @@ package com.yahoo.vespa.hosted.provision.persistence; import com.yahoo.path.Path; import com.yahoo.transaction.NestedTransaction; +import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; +import com.yahoo.vespa.curator.transaction.CuratorOperation; import com.yahoo.vespa.curator.transaction.CuratorOperations; import com.yahoo.vespa.curator.transaction.CuratorTransaction; +import com.yahoo.vespa.curator.transaction.TransactionChanges; import org.junit.Test; import java.util.List; -import java.util.Optional; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertFalse; @@ -26,7 +29,7 @@ import static org.junit.Assert.fail; public class CuratorDatabaseTest { @Test - public void testTransactionsIncreaseTimer() throws Exception { + public void testTransactionsIncreaseCounter() throws Exception { MockCurator curator = new MockCurator(); CuratorDatabase database = new CuratorDatabase(curator, Path.fromString("/"), true); @@ -36,9 +39,8 @@ public class CuratorDatabaseTest { commitCreate("/2", database); commitCreate("/1/1", database); commitCreate("/2/1", database); - assertEquals(4L, (long)curator.counter("/changeCounter").get().get().postValue()); + assertEquals(8L, (long)curator.counter("/changeCounter").get().get().postValue()); - List<String> children1Call0 = database.getChildren(Path.fromString("/1")); // prime the db; this call returns a different instance List<String> children1Call1 = database.getChildren(Path.fromString("/1")); List<String> children1Call2 = database.getChildren(Path.fromString("/1")); assertTrue("We reuse cached data when there are no commits", children1Call1 == children1Call2); @@ -49,7 +51,24 @@ public class CuratorDatabaseTest { assertEquals(2, database.getChildren(Path.fromString("/2")).size()); assertFalse("We do not reuse cached data in different parts of the tree when there are commits", children1Call3 == children1Call2); - + } + + @Test + public void testCacheInvalidation() throws Exception { + MockCurator curator = new MockCurator(); + CuratorDatabase database = new CuratorDatabase(curator, Path.fromString("/"), true); + + assertEquals(0L, (long)curator.counter("/changeCounter").get().get().postValue()); + commitCreate("/1", database); + assertArrayEquals(new byte[0], database.getData(Path.fromString("/1")).get()); + commitReadingWrite("/1", "hello".getBytes(), database); + // Data cached during commit of write transaction. Should be invalid now, and re-read. + assertEquals(4L, (long)curator.counter("/changeCounter").get().get().postValue()); + assertArrayEquals("hello".getBytes(), database.getData(Path.fromString("/1")).get()); + + assertEquals(0, database.getChildren(Path.fromString("/1")).size()); + commitCreate("/1/1", database); + assertEquals(1, database.getChildren(Path.fromString("/1")).size()); } @Test @@ -63,7 +82,7 @@ public class CuratorDatabaseTest { commitCreate("/2", database); commitCreate("/1/1", database); commitCreate("/2/1", database); - assertEquals(4L, (long)curator.counter("/changeCounter").get().get().postValue()); + assertEquals(8L, (long)curator.counter("/changeCounter").get().get().postValue()); List<String> children1Call0 = database.getChildren(Path.fromString("/1")); // prime the db; this call returns a different instance List<String> children1Call1 = database.getChildren(Path.fromString("/1")); @@ -72,7 +91,7 @@ public class CuratorDatabaseTest { } @Test - public void testThatCounterIncreasesAlsoOnCommitFailure() throws Exception { + public void testThatCounterIncreasesExactlyOnCommitFailure() throws Exception { MockCurator curator = new MockCurator(); CuratorDatabase database = new CuratorDatabase(curator, Path.fromString("/"), true); @@ -85,32 +104,16 @@ public class CuratorDatabaseTest { catch (Exception expected) { // expected because the parent does not exist } + // Counter increased once, since prepare failed. assertEquals(1L, (long)curator.counter("/changeCounter").get().get().postValue()); - } - - @Test - public void testThatCounterIncreasesAlsoOnCommitFailureFromExistingTransaction() throws Exception { - MockCurator curator = new MockCurator(); - CuratorDatabase database = new CuratorDatabase(curator, Path.fromString("/"), true); - - assertEquals(0L, (long)curator.counter("/changeCounter").get().get().postValue()); try { - NestedTransaction t = new NestedTransaction(); - CuratorTransaction separateC = new CuratorTransaction(curator); - separateC.add(CuratorOperations.create("/1/2")); // fail as parent does not exist - t.add(separateC); - - CuratorTransaction c = database.newCuratorTransactionIn(t); - c.add(CuratorOperations.create("/1")); // does not fail - - t.commit(); + commitFailing(database); // fail during commit fail("Expected exception"); } - catch (Exception expected) { - // expected because the parent does not exist - } - assertEquals(1L, (long)curator.counter("/changeCounter").get().get().postValue()); + catch (Exception expected) { } + // Counter increased, even though commit failed. + assertEquals(3L, (long)curator.counter("/changeCounter").get().get().postValue()); } private void commitCreate(String path, CuratorDatabase database) { @@ -120,4 +123,41 @@ public class CuratorDatabaseTest { t.commit(); } + private void commitReadingWrite(String path, byte[] data, CuratorDatabase database) { + NestedTransaction transaction = new NestedTransaction(); + byte[] oldData = database.getData(Path.fromString(path)).get(); + CuratorTransaction curatorTransaction = database.newCuratorTransactionIn(transaction); + // Add a dummy operation which reads the data and populates the cache during commit of the write. + curatorTransaction.add(new DummyOperation(() -> assertArrayEquals(oldData, database.getData(Path.fromString(path)).get()))); + curatorTransaction.add(CuratorOperations.setData(path, data)); + transaction.commit(); + } + + /** Commit an operation which fails during commit. */ + private void commitFailing(CuratorDatabase database) { + NestedTransaction t = new NestedTransaction(); + CuratorTransaction c = database.newCuratorTransactionIn(t); + c.add(new DummyOperation(() -> { throw new RuntimeException(); })); + t.commit(); + } + + static class DummyOperation implements CuratorOperation { + + private final Runnable task; + + public DummyOperation(Runnable task) { + this.task = task; + } + + @Override + public org.apache.curator.framework.api.transaction.CuratorTransaction and(org.apache.curator.framework.api.transaction.CuratorTransaction transaction) { + task.run(); + return transaction; + } + + @Override + public void check(Curator curator, TransactionChanges changes) { } + + } + } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/flags1.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/flags1.json index 8fd09b4a274..f27545f6094 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/flags1.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/flags1.json @@ -5,6 +5,13 @@ "enabled": false, "enabledHostnames": [], "enabledApplications": [] + }, + { + "id": "new-cache-counting", + "enabled": false, + "enabledHostnames": [], + "enabledApplications": [] } + ] } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/flags2.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/flags2.json index 78de52e4e85..a0e9954bec4 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/flags2.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/flags2.json @@ -9,6 +9,12 @@ "enabledApplications": [ "foo:bar:default" ] + }, + { + "id": "new-cache-counting", + "enabled": false, + "enabledHostnames": [], + "enabledApplications": [] } ] } diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/duper/DuperModel.java b/service-monitor/src/main/java/com/yahoo/vespa/service/duper/DuperModel.java index 024282d3d21..f559e9336c8 100644 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/duper/DuperModel.java +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/duper/DuperModel.java @@ -13,7 +13,7 @@ import java.util.TreeMap; import java.util.logging.Logger; /** - * A non-thread-safe mutable container of ApplicationInfo in the DuperModel, also taking care of listeners on changes. + * A non-thread-safe mutable container of ApplicationInfo, also taking care of listeners on changes. * * @author hakonhall */ diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/duper/InfraApplication.java b/service-monitor/src/main/java/com/yahoo/vespa/service/duper/InfraApplication.java index 8c74fe0396e..09140423010 100644 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/duper/InfraApplication.java +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/duper/InfraApplication.java @@ -17,7 +17,7 @@ import com.yahoo.vespa.applicationmodel.ClusterId; import com.yahoo.vespa.applicationmodel.ConfigId; import com.yahoo.vespa.applicationmodel.ServiceType; import com.yahoo.vespa.applicationmodel.TenantId; -import com.yahoo.vespa.service.health.ApplicationHealthMonitor; +import com.yahoo.vespa.service.health.StateV1HealthModel; import com.yahoo.vespa.service.model.ModelGenerator; import com.yahoo.vespa.service.monitor.InfraApplicationApi; @@ -107,7 +107,7 @@ public abstract class InfraApplication implements InfraApplicationApi { } private HostInfo makeHostInfo(HostName hostname) { - PortInfo portInfo = new PortInfo(healthPort, ApplicationHealthMonitor.PORT_TAGS_HEALTH); + PortInfo portInfo = new PortInfo(healthPort, StateV1HealthModel.HTTP_HEALTH_PORT_TAGS); Map<String, String> properties = new HashMap<>(); properties.put(ModelGenerator.CLUSTER_ID_PROPERTY_NAME, getClusterId().s()); diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/executor/Cancellable.java b/service-monitor/src/main/java/com/yahoo/vespa/service/executor/Cancellable.java new file mode 100644 index 00000000000..80c35851fa5 --- /dev/null +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/executor/Cancellable.java @@ -0,0 +1,10 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.executor; + +/** + * @author hakonhall + */ +@FunctionalInterface +public interface Cancellable { + void cancel(); +} diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/executor/CancellableImpl.java b/service-monitor/src/main/java/com/yahoo/vespa/service/executor/CancellableImpl.java new file mode 100644 index 00000000000..316b810c682 --- /dev/null +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/executor/CancellableImpl.java @@ -0,0 +1,103 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.executor; + +import com.yahoo.log.LogLevel; + +import java.time.Duration; +import java.util.Optional; +import java.util.logging.Logger; + +/** + * Provides the {@link Cancellable} returned by {@link RunletExecutorImpl#scheduleWithFixedDelay(Runlet, Duration)}, + * and ensuring the correct semantic execution of the {@link Runlet}. + * + * @author hakonhall + */ +class CancellableImpl implements Cancellable, Runnable { + private static final Logger logger = Logger.getLogger(CancellableImpl.class.getName()); + + private final Object monitor = new Object(); + private Runlet runlet; + private Optional<Runnable> periodicExecutionCancellation = Optional.empty(); + private boolean running = false; + private boolean cancelled = false; + + public CancellableImpl(Runlet runlet) { + this.runlet = runlet; + } + + /** + * Provide a way for {@code this} to cancel the periodic execution of {@link #run()}. + * + * <p>Must be called happens-before {@link #cancel()}. + */ + void setPeriodicExecutionCancellationCallback(Runnable periodicExecutionCancellation) { + synchronized (monitor) { + if (cancelled) { + throw new IllegalStateException("Cancellation callback set after cancel()"); + } + + this.periodicExecutionCancellation = Optional.of(periodicExecutionCancellation); + } + } + + /** + * Cancel the execution of the {@link Runlet}. + * + * <ul> + * <li>Either the runlet will not execute any more {@link Runlet#run()}s, and {@link Runlet#close()} + * and then {@code periodicExecutionCancellation} will be called synchronously, or + * <li>{@link #run()} is executing concurrently by another thread {@code T}. The last call to + * {@link Runlet#run()} will be called by {@code T} shortly, is in progress, or has completed. + * Then {@code T} will call {@link Runlet#close()} followed by {@code periodicExecutionCancellation}, + * before the return of {@link #run()}. + * </ul> + * + * <p>{@link #setPeriodicExecutionCancellationCallback(Runnable)} must be called happens-before this method. + */ + @Override + public void cancel() { + synchronized (monitor) { + if (!periodicExecutionCancellation.isPresent()) { + throw new IllegalStateException("setPeriodicExecutionCancellationCallback has not been called before cancel"); + } + + cancelled = true; + if (running) return; + } + + runlet.close(); + periodicExecutionCancellation.get().run(); + } + + /** + * Must be called periodically in happens-before order, but may be called concurrently with + * {@link #setPeriodicExecutionCancellationCallback(Runnable)} and {@link #cancel()}. + */ + @Override + public void run() { + try { + synchronized (monitor) { + if (cancelled) return; + running = true; + } + + runlet.run(); + + synchronized (monitor) { + running = false; + if (!cancelled) return; + + if (!periodicExecutionCancellation.isPresent()) { + // This should be impossible given the implementation of cancel() + throw new IllegalStateException("Cancelled before cancellation callback was set"); + } + } + + runlet.close(); + periodicExecutionCancellation.get().run(); + } catch (Throwable e) { + logger.log(LogLevel.ERROR, "Failed run of periodic execution", e); + } + } +} diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/executor/Runlet.java b/service-monitor/src/main/java/com/yahoo/vespa/service/executor/Runlet.java new file mode 100644 index 00000000000..a41bd0f777e --- /dev/null +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/executor/Runlet.java @@ -0,0 +1,19 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.executor; + +/** + * A {@code Runlet} joins {@link AutoCloseable} with {@link Runnable} with the following semantics: + * + * <ul> + * <li>The {@link #run()} method may be called any number of times, followed by a single call to {@link #close()}. + * <li>The caller must ensure the calls are ordered by {@code happens-before}, i.e. the class can be thread-unsafe. + * </ul> + * + * @author hakonhall + */ +public interface Runlet extends AutoCloseable, Runnable { + void run(); + + @Override + void close(); +} diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/executor/RunletExecutor.java b/service-monitor/src/main/java/com/yahoo/vespa/service/executor/RunletExecutor.java new file mode 100644 index 00000000000..4d6dc4b316f --- /dev/null +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/executor/RunletExecutor.java @@ -0,0 +1,21 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.executor; + +import java.time.Duration; + +/** + * @author hakonhall + */ +public interface RunletExecutor extends AutoCloseable { + /** + * Execute the task periodically with a fixed delay. + * + * <p>If the execution is {@link Cancellable#cancel() cancelled}, the runlet is {@link Runlet#close() closed} + * as soon as possible. + */ + Cancellable scheduleWithFixedDelay(Runlet runlet, Duration delay); + + /** Shuts down and waits for all execution to wind down. */ + @Override + void close(); +} diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/executor/RunletExecutorImpl.java b/service-monitor/src/main/java/com/yahoo/vespa/service/executor/RunletExecutorImpl.java new file mode 100644 index 00000000000..1f647a7fb31 --- /dev/null +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/executor/RunletExecutorImpl.java @@ -0,0 +1,71 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.executor; + +import com.yahoo.log.LogLevel; + +import java.time.Duration; +import java.util.Random; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.logging.Logger; + +/** + * @author hakonhall + */ +public class RunletExecutorImpl implements RunletExecutor { + private static Logger logger = Logger.getLogger(RunletExecutorImpl.class.getName()); + + // About 'static': Javadoc says "Instances of java.util.Random are threadsafe." + private static final Random random = new Random(); + + private final AtomicInteger executionId = new AtomicInteger(0); + private final ConcurrentHashMap<Integer, CancellableImpl> cancellables = new ConcurrentHashMap<>(); + private final ScheduledThreadPoolExecutor executor; + + public RunletExecutorImpl(int threadPoolSize) { + executor = new ScheduledThreadPoolExecutor(threadPoolSize); + } + + public Cancellable scheduleWithFixedDelay(Runlet runlet, Duration delay) { + Duration initialDelay = Duration.ofMillis((long) random.nextInt((int) delay.toMillis())); + CancellableImpl cancellable = new CancellableImpl(runlet); + ScheduledFuture<?> future = executor.scheduleWithFixedDelay(cancellable, initialDelay.toMillis(), delay.toMillis(), TimeUnit.MILLISECONDS); + cancellable.setPeriodicExecutionCancellationCallback(() -> future.cancel(false)); + Integer id = executionId.incrementAndGet(); + cancellables.put(id, cancellable); + return () -> cancelRunlet(id); + } + + private void cancelRunlet(Integer id) { + CancellableImpl cancellable = cancellables.remove(id); + if (cancellable != null) { + cancellable.cancel(); + } + } + + @Override + public void close() { + // At this point no-one should be scheduling new runlets, so this ought to clear the map. + cancellables.keySet().forEach(this::cancelRunlet); + + if (cancellables.size() > 0) { + throw new IllegalStateException("Runlets scheduled while closing the executor"); + } + + // The cancellables will cancel themselves from the executor only after up-to delay time, + // so wait until all have drained. + while (executor.getQueue().size() > 0) { + try { Thread.sleep(200); } catch (InterruptedException ignored) { } + } + + executor.shutdown(); + try { + executor.awaitTermination(10, TimeUnit.MINUTES); + } catch (InterruptedException e) { + logger.log(LogLevel.WARNING, "Timed out waiting for termination of executor", e); + } + } +} diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/ApacheHttpClient.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/ApacheHttpClient.java new file mode 100644 index 00000000000..4a382ee8d94 --- /dev/null +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/health/ApacheHttpClient.java @@ -0,0 +1,87 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.health; + +import org.apache.http.HttpResponse; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.config.Registry; +import org.apache.http.config.RegistryBuilder; +import org.apache.http.conn.ConnectionKeepAliveStrategy; +import org.apache.http.conn.HttpClientConnectionManager; +import org.apache.http.conn.socket.ConnectionSocketFactory; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.DefaultConnectionKeepAliveStrategy; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.impl.conn.BasicHttpClientConnectionManager; +import org.apache.http.protocol.HttpContext; + +import java.io.IOException; +import java.net.URL; +import java.time.Duration; + +/** + * @author hakonhall + */ +class ApacheHttpClient implements AutoCloseable { + private final URL url; + private final CloseableHttpClient client; + + @FunctionalInterface + interface Handler<T> { + T handle(CloseableHttpResponse httpResponse) throws Exception; + } + + static CloseableHttpClient makeCloseableHttpClient(URL url, Duration timeout, Duration keepAlive, ConnectionSocketFactory socketFactory) { + Registry<ConnectionSocketFactory> registry = RegistryBuilder.<ConnectionSocketFactory>create() + .register(url.getProtocol(),socketFactory) + .build(); + + HttpClientConnectionManager connectionManager = new BasicHttpClientConnectionManager(registry); + + RequestConfig requestConfig = RequestConfig.custom() + .setConnectTimeout((int) timeout.toMillis()) // establishment of connection + .setConnectionRequestTimeout((int) timeout.toMillis()) // connection from connection manager + .setSocketTimeout((int) timeout.toMillis()) // waiting for data + .build(); + + ConnectionKeepAliveStrategy keepAliveStrategy = + new DefaultConnectionKeepAliveStrategy() { + @Override + public long getKeepAliveDuration(HttpResponse response, HttpContext context) { + long keepAliveMillis = super.getKeepAliveDuration(response, context); + if (keepAliveMillis == -1) { + keepAliveMillis = keepAlive.toMillis(); + } + return keepAliveMillis; + } + }; + + return HttpClients.custom() + .setKeepAliveStrategy(keepAliveStrategy) + .setConnectionManager(connectionManager) + .disableAutomaticRetries() + .setDefaultRequestConfig(requestConfig) + .build(); + } + + ApacheHttpClient(URL url, Duration timeout, Duration keepAlive, ConnectionSocketFactory socketFactory) { + this(url, makeCloseableHttpClient(url, timeout, keepAlive, socketFactory)); + } + + ApacheHttpClient(URL url, CloseableHttpClient client) { + this.url = url; + this.client = client; + } + + <T> T get(Handler<T> handler) throws Exception { + try (CloseableHttpResponse httpResponse = client.execute(new HttpGet(url.toString()))) { + return handler.handle(httpResponse); + } + } + + @Override + public void close() throws IOException { + client.close(); + } +} diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/ApplicationHealthMonitor.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/ApplicationHealthMonitor.java index 2d81474853c..5fab8ac8591 100644 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/health/ApplicationHealthMonitor.java +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/health/ApplicationHealthMonitor.java @@ -2,53 +2,48 @@ package com.yahoo.vespa.service.health; import com.yahoo.config.model.api.ApplicationInfo; -import com.yahoo.config.model.api.HostInfo; -import com.yahoo.config.model.api.PortInfo; -import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.HostName; import com.yahoo.vespa.applicationmodel.ClusterId; import com.yahoo.vespa.applicationmodel.ConfigId; import com.yahoo.vespa.applicationmodel.ServiceStatus; import com.yahoo.vespa.applicationmodel.ServiceType; -import com.yahoo.vespa.service.monitor.ServiceStatusProvider; -import com.yahoo.vespa.service.model.ApplicationInstanceGenerator; import com.yahoo.vespa.service.model.ServiceId; +import com.yahoo.vespa.service.monitor.ServiceStatusProvider; -import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; -import java.util.List; +import java.util.HashSet; import java.util.Map; -import java.util.Optional; -import java.util.function.Function; +import java.util.Set; /** * Responsible for monitoring a whole application using /state/v1/health. * * @author hakon */ -public class ApplicationHealthMonitor implements ServiceStatusProvider, AutoCloseable { - public static final String PORT_TAG_STATE = "STATE"; - public static final String PORT_TAG_HTTP = "HTTP"; - /** Port tags implying /state/v1/health is served */ - public static final List<String> PORT_TAGS_HEALTH = - Collections.unmodifiableList(Arrays.asList(PORT_TAG_HTTP, PORT_TAG_STATE)); +class ApplicationHealthMonitor implements ServiceStatusProvider, AutoCloseable { + private final ApplicationId applicationId; + private final StateV1HealthModel healthModel; + private final Map<ServiceId, HealthMonitor> monitors = new HashMap<>(); - private final Map<ServiceId, HealthMonitor> healthMonitors; - - public static ApplicationHealthMonitor startMonitoring(ApplicationInfo application) { - return startMonitoring(application, HealthMonitor::new); + ApplicationHealthMonitor(ApplicationId applicationId, StateV1HealthModel healthModel) { + this.applicationId = applicationId; + this.healthModel = healthModel; } - /** For testing. */ - static ApplicationHealthMonitor startMonitoring(ApplicationInfo application, - Function<HealthEndpoint, HealthMonitor> mapper) { - return new ApplicationHealthMonitor(makeHealthMonitors(application, mapper)); - } + void monitor(ApplicationInfo applicationInfo) { + if (!applicationInfo.getApplicationId().equals(applicationId)) { + throw new IllegalArgumentException("Monitors " + applicationId + " but was asked to monitor " + applicationInfo.getApplicationId()); + } + + Map<ServiceId, HealthEndpoint> endpoints = healthModel.extractHealthEndpoints(applicationInfo); + + // Remove obsolete monitors + Set<ServiceId> removed = new HashSet<>(monitors.keySet()); + removed.removeAll(endpoints.keySet()); + removed.stream().map(monitors::remove).forEach(HealthMonitor::close); - private ApplicationHealthMonitor(Map<ServiceId, HealthMonitor> healthMonitors) { - this.healthMonitors = healthMonitors; + // Add new monitors. + endpoints.forEach((serviceId, endpoint) -> monitors.computeIfAbsent(serviceId, ignoredId -> endpoint.startMonitoring())); } @Override @@ -62,7 +57,7 @@ public class ApplicationHealthMonitor implements ServiceStatusProvider, AutoClos ServiceType serviceType, ConfigId configId) { ServiceId serviceId = new ServiceId(applicationId, clusterId, serviceType, configId); - HealthMonitor monitor = healthMonitors.get(serviceId); + HealthMonitor monitor = monitors.get(serviceId); if (monitor == null) { return ServiceStatus.NOT_CHECKED; } @@ -72,45 +67,7 @@ public class ApplicationHealthMonitor implements ServiceStatusProvider, AutoClos @Override public void close() { - healthMonitors.values().forEach(HealthMonitor::close); - healthMonitors.clear(); - } - - private static Map<ServiceId, HealthMonitor> makeHealthMonitors( - ApplicationInfo application, Function<HealthEndpoint, HealthMonitor> monitorFactory) { - Map<ServiceId, HealthMonitor> healthMonitors = new HashMap<>(); - for (HostInfo hostInfo : application.getModel().getHosts()) { - for (ServiceInfo serviceInfo : hostInfo.getServices()) { - for (PortInfo portInfo : serviceInfo.getPorts()) { - maybeCreateHealthMonitor( - application, - hostInfo, - serviceInfo, - portInfo, - monitorFactory) - .ifPresent(healthMonitor -> healthMonitors.put( - ApplicationInstanceGenerator.getServiceId(application, serviceInfo), - healthMonitor)); - } - } - } - return healthMonitors; - } - - private static Optional<HealthMonitor> maybeCreateHealthMonitor( - ApplicationInfo applicationInfo, - HostInfo hostInfo, - ServiceInfo serviceInfo, - PortInfo portInfo, - Function<HealthEndpoint, HealthMonitor> monitorFactory) { - if (portInfo.getTags().containsAll(PORT_TAGS_HEALTH)) { - HostName hostname = HostName.from(hostInfo.getHostname()); - HealthEndpoint endpoint = HealthEndpoint.forHttp(hostname, portInfo.getPort()); - HealthMonitor healthMonitor = monitorFactory.apply(endpoint); - healthMonitor.startMonitoring(); - return Optional.of(healthMonitor); - } - - return Optional.empty(); + monitors.values().forEach(HealthMonitor::close); + monitors.clear(); } } diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/ApplicationHealthMonitorFactory.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/ApplicationHealthMonitorFactory.java index 43be236268c..a747753160e 100644 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/health/ApplicationHealthMonitorFactory.java +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/health/ApplicationHealthMonitorFactory.java @@ -1,12 +1,12 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.service.health; -import com.yahoo.config.model.api.ApplicationInfo; +import com.yahoo.config.provision.ApplicationId; /** * @author hakonhall */ @FunctionalInterface interface ApplicationHealthMonitorFactory { - ApplicationHealthMonitor create(ApplicationInfo applicationInfo); + ApplicationHealthMonitor create(ApplicationId applicationId); } diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthClient.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthClient.java deleted file mode 100644 index 129cc799a25..00000000000 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthClient.java +++ /dev/null @@ -1,142 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.service.health; - -import com.fasterxml.jackson.databind.ObjectMapper; -import org.apache.http.HttpEntity; -import org.apache.http.HttpResponse; -import org.apache.http.client.config.RequestConfig; -import org.apache.http.client.methods.CloseableHttpResponse; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.config.Registry; -import org.apache.http.config.RegistryBuilder; -import org.apache.http.conn.ConnectionKeepAliveStrategy; -import org.apache.http.conn.HttpClientConnectionManager; -import org.apache.http.conn.socket.ConnectionSocketFactory; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.DefaultConnectionKeepAliveStrategy; -import org.apache.http.impl.client.HttpClients; -import org.apache.http.impl.conn.BasicHttpClientConnectionManager; -import org.apache.http.protocol.HttpContext; -import org.apache.http.util.EntityUtils; - -import java.util.function.Function; - -import static com.yahoo.yolean.Exceptions.uncheck; - -/** - * Health client - * - * NOT thread-safe. - * - * @author hakon - */ -public class HealthClient implements AutoCloseable { - private static final ObjectMapper mapper = new ObjectMapper(); - private static final long MAX_CONTENT_LENGTH = 1L << 20; // 1 MB - private static final int DEFAULT_TIMEOUT_MILLIS = 1_000; - - private static final ConnectionKeepAliveStrategy KEEP_ALIVE_STRATEGY = - new DefaultConnectionKeepAliveStrategy() { - @Override - public long getKeepAliveDuration(HttpResponse response, HttpContext context) { - long keepAlive = super.getKeepAliveDuration(response, context); - if (keepAlive == -1) { - // Keep connections alive 60 seconds if a keep-alive value - // has not be explicitly set by the server - keepAlive = 60000; - } - return keepAlive; - } - }; - - private final HealthEndpoint endpoint; - private final CloseableHttpClient httpClient; - private final Function<HttpEntity, String> getContentFunction; - - public HealthClient(HealthEndpoint endpoint) { - this(endpoint, - makeCloseableHttpClient(endpoint), - entity -> uncheck(() -> EntityUtils.toString(entity))); - } - - /** For testing. */ - HealthClient(HealthEndpoint endpoint, - CloseableHttpClient httpClient, - Function<HttpEntity, String> getContentFunction) { - this.endpoint = endpoint; - this.httpClient = httpClient; - this.getContentFunction = getContentFunction; - } - - public HealthEndpoint getEndpoint() { - return endpoint; - } - - public HealthInfo getHealthInfo() { - try { - return probeHealth(); - } catch (Exception e) { - return HealthInfo.fromException(e); - } - } - - @Override - public void close() { - try { - httpClient.close(); - } catch (Exception e) { - // ignore - } - } - - private static CloseableHttpClient makeCloseableHttpClient(HealthEndpoint endpoint) { - Registry<ConnectionSocketFactory> registry = RegistryBuilder.<ConnectionSocketFactory>create() - .register(endpoint.getStateV1HealthUrl().getProtocol(), endpoint.getConnectionSocketFactory()) - .build(); - - HttpClientConnectionManager connectionManager = new BasicHttpClientConnectionManager(registry); - - RequestConfig requestConfig = RequestConfig.custom() - .setConnectTimeout(DEFAULT_TIMEOUT_MILLIS) // establishment of connection - .setConnectionRequestTimeout(DEFAULT_TIMEOUT_MILLIS) // connection from connection manager - .setSocketTimeout(DEFAULT_TIMEOUT_MILLIS) // waiting for data - .build(); - - return HttpClients.custom() - .setKeepAliveStrategy(KEEP_ALIVE_STRATEGY) - .setConnectionManager(connectionManager) - .disableAutomaticRetries() - .setDefaultRequestConfig(requestConfig) - .build(); - } - - private HealthInfo probeHealth() throws Exception { - HttpGet httpget = new HttpGet(endpoint.getStateV1HealthUrl().toString()); - - CloseableHttpClient httpClient = this.httpClient; - if (httpClient == null) { - throw new IllegalStateException("HTTP client never started or has closed"); - } - - try (CloseableHttpResponse httpResponse = httpClient.execute(httpget)) { - int httpStatusCode = httpResponse.getStatusLine().getStatusCode(); - if (httpStatusCode < 200 || httpStatusCode >= 300) { - return HealthInfo.fromBadHttpStatusCode(httpStatusCode); - } - - HttpEntity bodyEntity = httpResponse.getEntity(); - long contentLength = bodyEntity.getContentLength(); - if (contentLength > MAX_CONTENT_LENGTH) { - throw new IllegalArgumentException("Content too long: " + contentLength + " bytes"); - } - String body = getContentFunction.apply(bodyEntity); - HealthResponse healthResponse = mapper.readValue(body, HealthResponse.class); - - if (healthResponse.status == null || healthResponse.status.code == null) { - return HealthInfo.fromHealthStatusCode(HealthResponse.Status.DEFAULT_STATUS); - } else { - return HealthInfo.fromHealthStatusCode(healthResponse.status.code); - } - } - } -} diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthEndpoint.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthEndpoint.java index e15d82ea70b..8c4997634a0 100644 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthEndpoint.java +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthEndpoint.java @@ -1,38 +1,15 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.service.health; -import com.yahoo.config.provision.HostName; -import com.yahoo.vespa.athenz.api.AthenzIdentity; -import com.yahoo.vespa.athenz.identity.ServiceIdentityProvider; -import com.yahoo.vespa.athenz.tls.AthenzIdentityVerifier; -import org.apache.http.conn.socket.ConnectionSocketFactory; - -import javax.net.ssl.HostnameVerifier; -import java.net.URL; -import java.util.Collections; - -import static com.yahoo.yolean.Exceptions.uncheck; +import com.yahoo.vespa.service.model.ServiceId; /** + * An endpoint 1-1 with a service and that can be health monitored. + * * @author hakon */ -public interface HealthEndpoint { - - static HealthEndpoint forHttp(HostName hostname, int port) { - URL url = uncheck(() -> new URL("http", hostname.value(), port, "/state/v1/health")); - return new HttpHealthEndpoint(url); - } - - static HealthEndpoint forHttps(HostName hostname, - int port, - ServiceIdentityProvider serviceIdentityProvider, - AthenzIdentity remoteIdentity) { - URL url = uncheck(() -> new URL("https", hostname.value(), port, "/state/v1/health")); - HostnameVerifier peerVerifier = new AthenzIdentityVerifier(Collections.singleton(remoteIdentity)); - return new HttpsHealthEndpoint(url, serviceIdentityProvider, peerVerifier); - } - - URL getStateV1HealthUrl(); - ConnectionSocketFactory getConnectionSocketFactory(); +interface HealthEndpoint { + ServiceId getServiceId(); String description(); + HealthMonitor startMonitoring(); } diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthMonitor.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthMonitor.java index d6dc1942404..f0e13548f58 100644 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthMonitor.java +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthMonitor.java @@ -1,90 +1,14 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.service.health; -import com.yahoo.log.LogLevel; import com.yahoo.vespa.applicationmodel.ServiceStatus; -import java.time.Duration; -import java.util.Random; -import java.util.concurrent.ScheduledThreadPoolExecutor; -import java.util.concurrent.TimeUnit; -import java.util.logging.Logger; - /** - * Used to monitor the health of a single URL endpoint. - * - * <p>Must be closed on successful start of monitoring ({} - * - * <p>Thread-safe - * - * @author hakon + * @author hakonhall */ -public class HealthMonitor implements AutoCloseable { - private static final Logger logger = Logger.getLogger(HealthMonitor.class.getName()); - - /** The duration between each health request. */ - private static final Duration DEFAULT_DELAY = Duration.ofSeconds(10); - - // About 'static': Javadoc says "Instances of java.util.Random are threadsafe." - private static final Random random = new Random(); - - private final ScheduledThreadPoolExecutor executor = new ScheduledThreadPoolExecutor(1); - private final HealthClient healthClient; - private final Duration delay; - - private volatile HealthInfo lastHealthInfo = HealthInfo.empty(); - - public HealthMonitor(HealthEndpoint stateV1HealthEndpoint) { - this(new HealthClient(stateV1HealthEndpoint), DEFAULT_DELAY); - } - - /** For testing. */ - HealthMonitor(HealthClient healthClient, Duration delay) { - this.healthClient = healthClient; - this.delay = delay; - } - - public void startMonitoring() { - executor.scheduleWithFixedDelay( - this::updateSynchronously, - initialDelayInMillis(delay.toMillis()), - delay.toMillis(), - TimeUnit.MILLISECONDS); - } - - public ServiceStatus getStatus() { - return lastHealthInfo.toServiceStatus(); - } +interface HealthMonitor extends AutoCloseable { + ServiceStatus getStatus(); @Override - public void close() { - executor.shutdown(); - - try { - executor.awaitTermination(2, TimeUnit.SECONDS); - } catch (InterruptedException e) { - logger.log(LogLevel.INFO, "Interrupted while waiting for health monitor termination: " + - e.getMessage()); - } - - healthClient.close(); - } - - private long initialDelayInMillis(long maxInitialDelayInMillis) { - if (maxInitialDelayInMillis >= Integer.MAX_VALUE) { - throw new IllegalArgumentException("Max initial delay is out of bounds: " + maxInitialDelayInMillis); - } - - return (long) random.nextInt((int) maxInitialDelayInMillis); - } - - private void updateSynchronously() { - try { - lastHealthInfo = healthClient.getHealthInfo(); - } catch (Throwable t) { - // An uncaught exception will kill the executor.scheduleWithFixedDelay thread! - logger.log(LogLevel.WARNING, "Failed to get health info for " + - healthClient.getEndpoint().description(), t); - } - } + void close(); } diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthMonitorManager.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthMonitorManager.java index e9a5ec314f6..2ad37faf593 100644 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthMonitorManager.java +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthMonitorManager.java @@ -12,8 +12,10 @@ import com.yahoo.vespa.flags.FeatureFlag; import com.yahoo.vespa.flags.FileFlagSource; import com.yahoo.vespa.service.duper.DuperModelManager; import com.yahoo.vespa.service.duper.ZoneApplication; +import com.yahoo.vespa.service.executor.RunletExecutorImpl; import com.yahoo.vespa.service.manager.MonitorManager; +import java.time.Duration; import java.util.concurrent.ConcurrentHashMap; /** @@ -22,6 +24,28 @@ import java.util.concurrent.ConcurrentHashMap; * @author hakon */ public class HealthMonitorManager implements MonitorManager { + // Weight the following against each other: + // - The number of threads N working on health checking + // - The health request timeout T + // - The max staleness S of the health of an endpoint + // - The ideal staleness I of the health of an endpoint + // + // The largest zone is main.prod.us-west-1: + // - 314 tenant host admins + // - 7 proxy host admins + // - 3 config host admins + // - 3 config servers + // for a total of E = 327 endpoints + private static final int MAX_ENDPOINTS = 500; + private static final Duration HEALTH_REQUEST_TIMEOUT = Duration.ofSeconds(1); + private static final Duration TARGET_HEALTH_STALENESS = Duration.ofSeconds(10); + private static final Duration MAX_HEALTH_STALENESS = Duration.ofSeconds(60); + static final int THREAD_POOL_SIZE = (int) Math.ceil(MAX_ENDPOINTS * HEALTH_REQUEST_TIMEOUT.toMillis() / (double) MAX_HEALTH_STALENESS.toMillis()); + + // Keep connections alive 60 seconds (>=MAX_HEALTH_STALENESS) if a keep-alive value has not be + // explicitly set by the server. + private static final Duration KEEP_ALIVE = Duration.ofSeconds(60); + private final ConcurrentHashMap<ApplicationId, ApplicationHealthMonitor> healthMonitors = new ConcurrentHashMap<>(); private final DuperModelManager duperModel; private final ApplicationHealthMonitorFactory applicationHealthMonitorFactory; @@ -31,22 +55,29 @@ public class HealthMonitorManager implements MonitorManager { public HealthMonitorManager(DuperModelManager duperModel, FileFlagSource flagSource) { this(duperModel, new FeatureFlag("healthmonitor-monitorinfra", true, flagSource), - ApplicationHealthMonitor::startMonitoring); + new StateV1HealthModel(TARGET_HEALTH_STALENESS, HEALTH_REQUEST_TIMEOUT, KEEP_ALIVE, new RunletExecutorImpl(THREAD_POOL_SIZE))); + } + + private HealthMonitorManager(DuperModelManager duperModel, + FeatureFlag monitorInfra, + StateV1HealthModel healthModel) { + this(duperModel, monitorInfra, id -> new ApplicationHealthMonitor(id, healthModel)); } HealthMonitorManager(DuperModelManager duperModel, FeatureFlag monitorInfra, ApplicationHealthMonitorFactory applicationHealthMonitorFactory) { this.duperModel = duperModel; - this.applicationHealthMonitorFactory = applicationHealthMonitorFactory; this.monitorInfra = monitorInfra; + this.applicationHealthMonitorFactory = applicationHealthMonitorFactory; } @Override public void applicationActivated(ApplicationInfo application) { if (wouldMonitor(application.getApplicationId())) { - ApplicationHealthMonitor monitor = applicationHealthMonitorFactory.create(application); - healthMonitors.put(application.getApplicationId(), monitor); + healthMonitors + .computeIfAbsent(application.getApplicationId(), applicationHealthMonitorFactory::create) + .monitor(application); } } diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthUpdater.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthUpdater.java new file mode 100644 index 00000000000..4ed49e17e9f --- /dev/null +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthUpdater.java @@ -0,0 +1,14 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.health; + +import com.yahoo.vespa.service.executor.Runlet; + +/** + * A {@link HealthUpdater} will probe the health with {@link #run()}, whose result can be fetched with the + * thread-safe method {@link #getLatestHealthInfo()}. + * + * @author hakonhall + */ +interface HealthUpdater extends Runlet { + HealthInfo getLatestHealthInfo(); +} diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/HttpHealthEndpoint.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/HttpHealthEndpoint.java deleted file mode 100644 index 793c1a93379..00000000000 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/health/HttpHealthEndpoint.java +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.service.health; - -import org.apache.http.conn.socket.ConnectionSocketFactory; -import org.apache.http.conn.socket.PlainConnectionSocketFactory; - -import java.net.URL; - -/** - * @author hakon - */ -class HttpHealthEndpoint implements HealthEndpoint { - private final URL url; - private final ConnectionSocketFactory socketFactory; - - HttpHealthEndpoint(URL url) { - this.url = url; - this.socketFactory = PlainConnectionSocketFactory.getSocketFactory(); - } - - @Override - public URL getStateV1HealthUrl() { - return url; - } - - @Override - public ConnectionSocketFactory getConnectionSocketFactory() { - return socketFactory; - } - - @Override - public String description() { - return url.toString(); - } -} diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/HttpsHealthEndpoint.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/HttpsHealthEndpoint.java deleted file mode 100644 index 42e408256c5..00000000000 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/health/HttpsHealthEndpoint.java +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.service.health; - -import com.yahoo.vespa.athenz.identity.ServiceIdentityProvider; -import com.yahoo.vespa.athenz.identity.ServiceIdentitySslSocketFactory; -import org.apache.http.conn.socket.ConnectionSocketFactory; -import org.apache.http.conn.ssl.SSLConnectionSocketFactory; - -import javax.net.ssl.HostnameVerifier; -import java.net.URL; - -/** - * @author hakon - */ -public class HttpsHealthEndpoint implements HealthEndpoint { - private final URL url; - private final HostnameVerifier hostnameVerifier; - private final ServiceIdentityProvider serviceIdentityProvider; - - HttpsHealthEndpoint(URL url, - ServiceIdentityProvider serviceIdentityProvider, - HostnameVerifier hostnameVerifier) { - this.url = url; - this.serviceIdentityProvider = serviceIdentityProvider; - this.hostnameVerifier = hostnameVerifier; - } - - @Override - public URL getStateV1HealthUrl() { - return url; - } - - @Override - public ConnectionSocketFactory getConnectionSocketFactory() { - return new SSLConnectionSocketFactory(new ServiceIdentitySslSocketFactory(serviceIdentityProvider), hostnameVerifier); - } - - @Override - public String description() { - return url.toString(); - } -} diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthClient.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthClient.java new file mode 100644 index 00000000000..88aefe42a14 --- /dev/null +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthClient.java @@ -0,0 +1,74 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.health; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.yahoo.log.LogLevel; +import org.apache.http.HttpEntity; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.conn.socket.PlainConnectionSocketFactory; +import org.apache.http.util.EntityUtils; + +import java.io.IOException; +import java.net.URL; +import java.time.Duration; +import java.util.function.Function; +import java.util.logging.Logger; + +import static com.yahoo.yolean.Exceptions.uncheck; + +/** + * A thread-unsafe /state/v1/health endpoint client. + * + * @author hakonhall + */ +public class StateV1HealthClient implements AutoCloseable { + private static final long MAX_CONTENT_LENGTH = 1L << 20; // 1 MB + private static final ObjectMapper MAPPER = new ObjectMapper(); + private static final Logger logger = Logger.getLogger(StateV1HealthClient.class.getName()); + private final ApacheHttpClient httpClient; + private final Function<HttpEntity, String> getContentFunction; + + StateV1HealthClient(URL url, Duration requestTimeout, Duration connectionKeepAlive) { + this(new ApacheHttpClient(url, requestTimeout, connectionKeepAlive, PlainConnectionSocketFactory.getSocketFactory()), + entity -> uncheck(() -> EntityUtils.toString(entity))); + } + + StateV1HealthClient(ApacheHttpClient apacheHttpClient, Function<HttpEntity, String> getContentFunction) { + httpClient = apacheHttpClient; + this.getContentFunction = getContentFunction; + } + + HealthInfo get() throws Exception { + return httpClient.get(this::handle); + } + + private HealthInfo handle(CloseableHttpResponse httpResponse) throws IOException { + int httpStatusCode = httpResponse.getStatusLine().getStatusCode(); + if (httpStatusCode < 200 || httpStatusCode >= 300) { + return HealthInfo.fromBadHttpStatusCode(httpStatusCode); + } + + HttpEntity bodyEntity = httpResponse.getEntity(); + long contentLength = bodyEntity.getContentLength(); + if (contentLength > MAX_CONTENT_LENGTH) { + throw new IllegalArgumentException("Content too long: " + contentLength + " bytes"); + } + String body = getContentFunction.apply(bodyEntity); + HealthResponse healthResponse = MAPPER.readValue(body, HealthResponse.class); + + if (healthResponse.status == null || healthResponse.status.code == null) { + return HealthInfo.fromHealthStatusCode(HealthResponse.Status.DEFAULT_STATUS); + } else { + return HealthInfo.fromHealthStatusCode(healthResponse.status.code); + } + } + + @Override + public void close() { + try { + httpClient.close(); + } catch (Exception e) { + logger.log(LogLevel.WARNING, "Failed to close CloseableHttpClient", e); + } + } +} diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthEndpoint.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthEndpoint.java new file mode 100644 index 00000000000..8eca03c616f --- /dev/null +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthEndpoint.java @@ -0,0 +1,59 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.health; + +import com.yahoo.config.provision.HostName; +import com.yahoo.vespa.service.executor.RunletExecutor; +import com.yahoo.vespa.service.model.ServiceId; + +import java.net.URL; +import java.time.Duration; + +import static com.yahoo.yolean.Exceptions.uncheck; + +/** + * @author hakonhall + */ +class StateV1HealthEndpoint implements HealthEndpoint { + private final ServiceId serviceId; + private final URL url; + private final Duration requestTimeout; + private final Duration connectionKeepAlive; + private final Duration delay; + private final RunletExecutor executor; + + StateV1HealthEndpoint(ServiceId serviceId, + HostName hostname, + int port, + Duration delay, + Duration requestTimeout, + Duration connectionKeepAlive, + RunletExecutor executor) { + this.serviceId = serviceId; + this.delay = delay; + this.executor = executor; + this.url = uncheck(() -> new URL("http", hostname.value(), port, "/state/v1/health")); + this.requestTimeout = requestTimeout; + this.connectionKeepAlive = connectionKeepAlive; + } + + @Override + public ServiceId getServiceId() { + return serviceId; + } + + @Override + public HealthMonitor startMonitoring() { + StateV1HealthUpdater updater = new StateV1HealthUpdater(url, requestTimeout, connectionKeepAlive); + return new StateV1HealthMonitor(updater, executor, delay); + } + + @Override + public String description() { + return url.toString(); + } + + @Override + public String toString() { + return description(); + } +} diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthModel.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthModel.java new file mode 100644 index 00000000000..5e8979deb9f --- /dev/null +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthModel.java @@ -0,0 +1,74 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.health; + +import com.yahoo.config.model.api.ApplicationInfo; +import com.yahoo.config.model.api.HostInfo; +import com.yahoo.config.model.api.PortInfo; +import com.yahoo.config.model.api.ServiceInfo; +import com.yahoo.config.provision.HostName; +import com.yahoo.vespa.service.executor.RunletExecutor; +import com.yahoo.vespa.service.model.ApplicationInstanceGenerator; +import com.yahoo.vespa.service.model.ServiceId; + +import java.time.Duration; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** + * @author hakonhall + */ +public class StateV1HealthModel implements AutoCloseable { + private static final String PORT_TAG_STATE = "STATE"; + private static final String PORT_TAG_HTTP = "HTTP"; + + /** Port tags implying /state/v1/health is served on HTTP. */ + public static final List<String> HTTP_HEALTH_PORT_TAGS = Arrays.asList(PORT_TAG_HTTP, PORT_TAG_STATE); + private final Duration targetHealthStaleness; + private final Duration requestTimeout; + private final Duration connectionKeepAlive; + private final RunletExecutor executor; + + StateV1HealthModel(Duration targetHealthStaleness, + Duration requestTimeout, + Duration connectionKeepAlive, + RunletExecutor executor) { + this.targetHealthStaleness = targetHealthStaleness; + this.requestTimeout = requestTimeout; + this.connectionKeepAlive = connectionKeepAlive; + this.executor = executor; + } + + Map<ServiceId, HealthEndpoint> extractHealthEndpoints(ApplicationInfo application) { + Map<ServiceId, HealthEndpoint> endpoints = new HashMap<>(); + + for (HostInfo hostInfo : application.getModel().getHosts()) { + HostName hostname = HostName.from(hostInfo.getHostname()); + for (ServiceInfo serviceInfo : hostInfo.getServices()) { + ServiceId serviceId = ApplicationInstanceGenerator.getServiceId(application, serviceInfo); + for (PortInfo portInfo : serviceInfo.getPorts()) { + if (portInfo.getTags().containsAll(HTTP_HEALTH_PORT_TAGS)) { + StateV1HealthEndpoint endpoint = new StateV1HealthEndpoint( + serviceId, + hostname, + portInfo.getPort(), + targetHealthStaleness, + requestTimeout, + connectionKeepAlive, + executor); + endpoints.put(serviceId, endpoint); + break; // Avoid >1 endpoints per serviceId + } + } + } + } + + return endpoints; + } + + @Override + public void close() { + executor.close(); + } +} diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthMonitor.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthMonitor.java new file mode 100644 index 00000000000..d37797c7be9 --- /dev/null +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthMonitor.java @@ -0,0 +1,33 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.health; + +import com.yahoo.vespa.applicationmodel.ServiceStatus; +import com.yahoo.vespa.service.executor.Cancellable; +import com.yahoo.vespa.service.executor.RunletExecutor; + +import java.time.Duration; + +/** + * Used to monitor the health of a single URL endpoint. + * + * @author hakon + */ +class StateV1HealthMonitor implements HealthMonitor { + private final StateV1HealthUpdater updater; + private final Cancellable periodicExecution; + + StateV1HealthMonitor(StateV1HealthUpdater updater, RunletExecutor executor, Duration delay) { + this.updater = updater; + this.periodicExecution = executor.scheduleWithFixedDelay(updater, delay); + } + + @Override + public ServiceStatus getStatus() { + return updater.getLatestHealthInfo().toServiceStatus(); + } + + @Override + public void close() { + periodicExecution.cancel(); + } +} diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthUpdater.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthUpdater.java new file mode 100644 index 00000000000..011ec3b3212 --- /dev/null +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthUpdater.java @@ -0,0 +1,41 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.health; + +import java.net.URL; +import java.time.Duration; + +/** + * @author hakonhall + */ +class StateV1HealthUpdater implements HealthUpdater { + private final StateV1HealthClient healthClient; + + private volatile HealthInfo lastHealthInfo = HealthInfo.empty(); + + StateV1HealthUpdater(URL url, Duration requestTimeout, Duration connectionKeepAlive) { + this(new StateV1HealthClient(url, requestTimeout, connectionKeepAlive)); + } + + StateV1HealthUpdater(StateV1HealthClient healthClient) { + this.healthClient = healthClient; + } + + @Override + public HealthInfo getLatestHealthInfo() { + return lastHealthInfo; + } + + @Override + public void run() { + try { + lastHealthInfo = healthClient.get(); + } catch (Exception e) { + lastHealthInfo = HealthInfo.fromException(e); + } + } + + @Override + public void close() { + healthClient.close(); + } +} diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/executor/CancellableImplTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/executor/CancellableImplTest.java new file mode 100644 index 00000000000..eb6f92d928c --- /dev/null +++ b/service-monitor/src/test/java/com/yahoo/vespa/service/executor/CancellableImplTest.java @@ -0,0 +1,79 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.executor; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.time.Duration; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * @author hakonhall + */ +public class CancellableImplTest { + private final TestExecutor executor = new TestExecutor(); + private final TestRunlet runlet = new TestRunlet(); + private final Cancellable cancellable = executor.scheduleWithFixedDelay(runlet, Duration.ofSeconds(1)); + + @After + public void tearDown() { + executor.close(); + } + + @Before + public void setUp() { + assertEquals(0, runlet.getRunsStarted()); + executor.runToCompletion(1); + assertEquals(1, runlet.getRunsStarted()); + executor.runToCompletion(2); + assertEquals(2, runlet.getRunsStarted()); + assertTrue(executor.isExecutionRunning()); + assertFalse(runlet.isClosed()); + assertTrue(executor.isExecutionRunning()); + assertFalse(runlet.isClosed()); + } + + @Test + public void testCancelWhileIdle() { + // Cancel while runlet is not running and verify closure and executor cancellation + cancellable.cancel(); + assertFalse(executor.isExecutionRunning()); + assertTrue(runlet.isClosed()); + + // Ensure a spurious run is ignored. + executor.runAsync(); + executor.runToCompletion(3); + assertEquals(2, runlet.getRunsStarted()); + } + + @Test + public void testCancelWhileRunning() { + // halt execution in runlet + runlet.shouldWaitInRun(true); + executor.runAsync(); + runlet.waitUntilInRun(); + assertEquals(3, runlet.getRunsStarted()); + assertEquals(2, runlet.getRunsCompleted()); + assertTrue(executor.isExecutionRunning()); + assertFalse(runlet.isClosed()); + + // Cancel now + cancellable.cancel(); + assertTrue(executor.isExecutionRunning()); + assertFalse(runlet.isClosed()); + + // Complete the runlet.run(), and verify the close and executor cancellation takes effect + runlet.shouldWaitInRun(false); + executor.waitUntilRunCompleted(3); + assertFalse(executor.isExecutionRunning()); + assertTrue(runlet.isClosed()); + + // Ensure a spurious run is ignored. + executor.runToCompletion(4); + assertEquals(3, runlet.getRunsStarted()); + } +}
\ No newline at end of file diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/executor/RunletExecutorImplTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/executor/RunletExecutorImplTest.java new file mode 100644 index 00000000000..9828d6300ed --- /dev/null +++ b/service-monitor/src/test/java/com/yahoo/vespa/service/executor/RunletExecutorImplTest.java @@ -0,0 +1,71 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.executor; + +import org.junit.After; +import org.junit.Test; + +import java.time.Duration; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * @author hakonhall + */ +public class RunletExecutorImplTest { + private final RunletExecutorImpl executor = new RunletExecutorImpl(2); + + @After + public void tearDown() { + executor.close(); + } + + @Test + public void testAFewCancellations() { + for (int i = 0; i < 10; ++i) { + TestRunlet runlet = new TestRunlet(); + Cancellable cancellable = schedule(runlet); + runlet.waitUntilCompleted(5); + cancellable.cancel(); + runlet.waitUntilClosed(); + } + } + + @Test + public void testCongestedThreadPool() { + TestRunlet runlet1 = new TestRunlet(); + runlet1.shouldWaitInRun(true); + Cancellable cancellable1 = schedule(runlet1); + runlet1.waitUntilInRun(); + + TestRunlet runlet2 = new TestRunlet(); + runlet2.shouldWaitInRun(true); + Cancellable cancellable2 = schedule(runlet2); + runlet2.waitUntilInRun(); + + TestRunlet runlet3 = new TestRunlet(); + Cancellable cancellable3 = schedule(runlet3); + try { Thread.sleep(10); } catch (InterruptedException ignored) { } + assertEquals(0, runlet3.getRunsStarted()); + + cancellable3.cancel(); + assertTrue(runlet3.isClosed()); + assertEquals(0, runlet3.getRunsStarted()); + + runlet1.shouldWaitInRun(false); + runlet2.shouldWaitInRun(false); + cancellable1.cancel(); + cancellable2.cancel(); + } + + @Test + public void testWithoutCancellation() { + TestRunlet runlet = new TestRunlet(); + Cancellable toBeIgnored = schedule(runlet); + runlet.waitUntilCompleted(2); + } + + private Cancellable schedule(Runlet runlet) { + return executor.scheduleWithFixedDelay(runlet, Duration.ofMillis(20)); + } +}
\ No newline at end of file diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/executor/TestExecutor.java b/service-monitor/src/test/java/com/yahoo/vespa/service/executor/TestExecutor.java new file mode 100644 index 00000000000..c40fc03ea00 --- /dev/null +++ b/service-monitor/src/test/java/com/yahoo/vespa/service/executor/TestExecutor.java @@ -0,0 +1,105 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.executor; + +import java.time.Duration; +import java.util.ArrayList; +import java.util.List; + +/** + * @author hakonhall + */ +public class TestExecutor implements RunletExecutor { + private List<Thread> threads = new ArrayList<>(); + + private Runlet runlet; + private CancellableImpl cancellable; + + private final Object monitor = new Object(); + private boolean afterRun = false; + private boolean waitAfterRun = false; + private int runsCompleted = 0; + + private final Runnable cancelExecution = () -> executionRunning = false; + private volatile boolean executionRunning = true; + + @Override + public Cancellable scheduleWithFixedDelay(Runlet runlet, Duration delay) { + if (this.runlet != null) { + throw new IllegalStateException("TestExecutor only supports execution of one runlet"); + } + + this.runlet = runlet; + this.cancellable = new CancellableImpl(runlet); + this.cancellable.setPeriodicExecutionCancellationCallback(cancelExecution); + return this::cancel; + } + + private void cancel() { + cancellable.cancel(); + } + + boolean isExecutionRunning() { + return executionRunning; + } + + void runAsync() { + Thread thread = new Thread(this::threadMain); + thread.start(); + threads.add(thread); + } + + void runToCompletion(int run) { + runAsync(); + waitUntilRunCompleted(run); + } + + private void threadMain() { + cancellable.run(); + + synchronized (monitor) { + ++runsCompleted; + afterRun = true; + monitor.notifyAll(); + + while (waitAfterRun) { + monitor.notifyAll(); + } + afterRun = false; + } + } + + void setWaitAfterRun(boolean waitAfterRun) { + synchronized (monitor) { + this.waitAfterRun = waitAfterRun; + } + } + + void waitUntilAfterRun() { + synchronized (monitor) { + while (!afterRun) { + uncheckedWait(); + } + } + } + + void waitUntilRunCompleted(int run) { + synchronized (monitor) { + while (runsCompleted < run) { + uncheckedWait(); + } + } + } + + void uncheckedWait() { + try { + monitor.wait(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + + @Override + public void close() { + threads.forEach(thread -> { try { thread.join(); } catch (InterruptedException ignored) {} }); + } +} diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/executor/TestRunlet.java b/service-monitor/src/test/java/com/yahoo/vespa/service/executor/TestRunlet.java new file mode 100644 index 00000000000..7e671dccd96 --- /dev/null +++ b/service-monitor/src/test/java/com/yahoo/vespa/service/executor/TestRunlet.java @@ -0,0 +1,98 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.executor; + +/** + * @author hakonhall + */ +public class TestRunlet implements Runlet { + private final Object monitor = new Object(); + private boolean running = false; + private boolean shouldWaitInRun = false; + private boolean closed = false; + private int runsStarted = 0; + private int runsCompleted = 0; + + int getRunsStarted() { + synchronized (monitor) { + return runsStarted; + } + } + + int getRunsCompleted() { + return runsCompleted; + } + + boolean isClosed() { + synchronized (monitor) { + return closed; + } + } + + void shouldWaitInRun(boolean value) { + synchronized (monitor) { + shouldWaitInRun = value; + monitor.notifyAll(); + } + } + + void waitUntilInRun() { + synchronized (monitor) { + while (!running) { + uncheckedWait(); + } + } + } + + void waitUntilCompleted(int runsCompleted) { + synchronized (monitor) { + while (this.runsCompleted < runsCompleted) { + uncheckedWait(); + } + } + } + + void waitUntilClosed() { + synchronized (monitor) { + while (!closed) { + uncheckedWait(); + } + } + } + + @Override + public void run() { + synchronized (monitor) { + if (closed) { + throw new IllegalStateException("run after close"); + } + + ++runsStarted; + running = true; + monitor.notifyAll(); + + while (shouldWaitInRun) { + uncheckedWait(); + } + + ++runsCompleted; + running = false; + monitor.notifyAll(); + } + } + + @Override + public void close() { + synchronized (monitor) { + closed = true; + monitor.notifyAll(); + } + } + + private void uncheckedWait() { + try { + monitor.wait(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } +} diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/health/ApplicationHealthMonitorTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/health/ApplicationHealthMonitorTest.java index 0dfca12099e..821f5282998 100644 --- a/service-monitor/src/test/java/com/yahoo/vespa/service/health/ApplicationHealthMonitorTest.java +++ b/service-monitor/src/test/java/com/yahoo/vespa/service/health/ApplicationHealthMonitorTest.java @@ -1,47 +1,92 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.service.health; +import com.yahoo.config.model.api.ApplicationInfo; import com.yahoo.config.provision.HostName; import com.yahoo.vespa.applicationmodel.ServiceStatus; import com.yahoo.vespa.service.duper.ConfigServerApplication; +import com.yahoo.vespa.service.model.ServiceId; import com.yahoo.vespa.service.monitor.ConfigserverUtil; import org.junit.Test; import java.util.HashMap; import java.util.Map; -import java.util.Objects; -import java.util.function.Function; import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class ApplicationHealthMonitorTest { private final ConfigServerApplication configServerApplication = new ConfigServerApplication(); @Test - public void sanityCheck() { - MonitorFactory monitorFactory = new MonitorFactory(); - + public void activationAndRemoval() { HealthMonitor monitor1 = mock(HealthMonitor.class); HealthMonitor monitor2 = mock(HealthMonitor.class); HealthMonitor monitor3 = mock(HealthMonitor.class); - monitorFactory.expectEndpoint("http://cfg1:19071/state/v1/health", monitor1); - monitorFactory.expectEndpoint("http://cfg2:19071/state/v1/health", monitor2); - monitorFactory.expectEndpoint("http://cfg3:19071/state/v1/health", monitor3); + ApplicationInfo configServer = ConfigserverUtil.makeExampleConfigServer(); + StateV1HealthModel model = mock(StateV1HealthModel.class); + ApplicationHealthMonitor applicationMonitor = new ApplicationHealthMonitor(configServer.getApplicationId(), model); + + // Activate with cfg1-2 + HealthEndpoint endpoint1 = mock(HealthEndpoint.class); + HealthEndpoint endpoint2 = mock(HealthEndpoint.class); + Map<ServiceId, HealthEndpoint> initialEndpoints = new HashMap<>(); + initialEndpoints.put(serviceIdOf("cfg1"), endpoint1); + initialEndpoints.put(serviceIdOf("cfg2"), endpoint2); + + when(model.extractHealthEndpoints(configServer)).thenReturn(initialEndpoints); + when(endpoint1.startMonitoring()).thenReturn(monitor1); + when(endpoint2.startMonitoring()).thenReturn(monitor2); + applicationMonitor.monitor(configServer); + + verify(endpoint1, times(1)).startMonitoring(); + verify(endpoint2, times(1)).startMonitoring(); when(monitor1.getStatus()).thenReturn(ServiceStatus.UP); when(monitor2.getStatus()).thenReturn(ServiceStatus.DOWN); - when(monitor3.getStatus()).thenReturn(ServiceStatus.NOT_CHECKED); - - ApplicationHealthMonitor applicationMonitor = ApplicationHealthMonitor.startMonitoring( - ConfigserverUtil.makeExampleConfigServer(), - monitorFactory); + when(monitor3.getStatus()).thenReturn(ServiceStatus.UP); assertEquals(ServiceStatus.UP, getStatus(applicationMonitor, "cfg1")); assertEquals(ServiceStatus.DOWN, getStatus(applicationMonitor, "cfg2")); assertEquals(ServiceStatus.NOT_CHECKED, getStatus(applicationMonitor, "cfg3")); + + // Update application to contain cfg2-3 + HealthEndpoint endpoint3 = mock(HealthEndpoint.class); + when(endpoint3.startMonitoring()).thenReturn(monitor3); + Map<ServiceId, HealthEndpoint> endpoints = new HashMap<>(); + endpoints.put(serviceIdOf("cfg2"), endpoint2); + endpoints.put(serviceIdOf("cfg3"), endpoint3); + when(model.extractHealthEndpoints(configServer)).thenReturn(endpoints); + applicationMonitor.monitor(configServer); + + // Only monitor1 has been removed and had its close called + verify(monitor1, times(1)).close(); + verify(monitor2, never()).close(); + verify(monitor3, never()).close(); + + // Only endpoint3 started monitoring from last monitor() + verify(endpoint1, times(1)).startMonitoring(); + verify(endpoint2, times(1)).startMonitoring(); + verify(endpoint3, times(1)).startMonitoring(); + + // Now cfg1 will be NOT_CHECKED, while cfg3 should be UP. + assertEquals(ServiceStatus.NOT_CHECKED, getStatus(applicationMonitor, "cfg1")); + assertEquals(ServiceStatus.DOWN, getStatus(applicationMonitor, "cfg2")); + assertEquals(ServiceStatus.UP, getStatus(applicationMonitor, "cfg3")); + + applicationMonitor.close(); + } + + private ServiceId serviceIdOf(String hostname) { + return new ServiceId(configServerApplication.getApplicationId(), + configServerApplication.getClusterId(), + configServerApplication.getServiceType(), + configServerApplication.configIdFor(HostName.from(hostname))); } private ServiceStatus getStatus(ApplicationHealthMonitor monitor, String hostname) { @@ -51,70 +96,4 @@ public class ApplicationHealthMonitorTest { configServerApplication.getServiceType(), configServerApplication.configIdFor(HostName.from(hostname))); } - - private static class MonitorFactory implements Function<HealthEndpoint, HealthMonitor> { - private Map<String, EndpointInfo> endpointMonitors = new HashMap<>(); - - public void expectEndpoint(String url, HealthMonitor monitorToReturn) { - endpointMonitors.put(url, new EndpointInfo(url, monitorToReturn)); - } - - @Override - public HealthMonitor apply(HealthEndpoint endpoint) { - String url = endpoint.getStateV1HealthUrl().toString(); - EndpointInfo info = endpointMonitors.get(url); - if (info == null) { - throw new IllegalArgumentException("Endpoint not expected: " + url); - } - - if (info.isEndpointDiscovered()) { - throw new IllegalArgumentException("A HealthMonitor has already been created to " + url); - } - - info.setEndpointDiscovered(true); - - return info.getMonitorToReturn(); - } - } - - private static class EndpointInfo { - private final String url; - private final HealthMonitor monitorToReturn; - - private boolean endpointDiscovered = false; - - private EndpointInfo(String url, HealthMonitor monitorToReturn) { - this.url = url; - this.monitorToReturn = monitorToReturn; - } - - public String getUrl() { - return url; - } - - public boolean isEndpointDiscovered() { - return endpointDiscovered; - } - - public void setEndpointDiscovered(boolean endpointDiscovered) { - this.endpointDiscovered = endpointDiscovered; - } - - public HealthMonitor getMonitorToReturn() { - return monitorToReturn; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - EndpointInfo that = (EndpointInfo) o; - return Objects.equals(url, that.url); - } - - @Override - public int hashCode() { - return Objects.hash(url); - } - } }
\ No newline at end of file diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/health/HealthMonitorManagerTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/health/HealthMonitorManagerTest.java index f420f5c1284..86b0ee4a8f3 100644 --- a/service-monitor/src/test/java/com/yahoo/vespa/service/health/HealthMonitorManagerTest.java +++ b/service-monitor/src/test/java/com/yahoo/vespa/service/health/HealthMonitorManagerTest.java @@ -49,7 +49,9 @@ public class HealthMonitorManagerTest { when(monitorInfra.value()).thenReturn(false); ApplicationInfo applicationInfo = ConfigserverUtil.makeExampleConfigServer(); manager.applicationActivated(applicationInfo); + verify(monitor, times(1)).monitor(applicationInfo); manager.applicationRemoved(applicationInfo.getApplicationId()); + verify(monitor, times(1)).close(); } @Test @@ -73,7 +75,7 @@ public class HealthMonitorManagerTest { ApplicationInfo proxyHostApplicationInfo = proxyHostApplication.makeApplicationInfo(hostnames); manager.applicationActivated(proxyHostApplicationInfo); - verify(monitorFactory, never()).create(proxyHostApplicationInfo); + verify(monitorFactory, never()).create(proxyHostApplicationInfo.getApplicationId()); assertStatus(ServiceStatus.NOT_CHECKED, 0, proxyHostApplication, "proxyhost1"); } @@ -88,7 +90,7 @@ public class HealthMonitorManagerTest { ApplicationInfo proxyHostApplicationInfo = proxyHostApplication.makeApplicationInfo(hostnames); manager.applicationActivated(proxyHostApplicationInfo); - verify(monitorFactory, times(1)).create(proxyHostApplicationInfo); + verify(monitorFactory, times(1)).create(proxyHostApplicationInfo.getApplicationId()); when(monitor.getStatus(any(), any(), any(), any())).thenReturn(ServiceStatus.UP); assertStatus(ServiceStatus.UP, 1, proxyHostApplication, "proxyhost1"); @@ -98,6 +100,11 @@ public class HealthMonitorManagerTest { assertStatus(ServiceStatus.NOT_CHECKED, 0, controllerHostApplication, "controllerhost1"); } + @Test + public void threadPoolSize() { + assertEquals(9, HealthMonitorManager.THREAD_POOL_SIZE); + } + private void assertStatus(ServiceStatus expected, int verifyTimes, InfraApplication infraApplication, String hostname) { ServiceStatus actual = manager.getStatus( infraApplication.getApplicationId(), diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/health/HealthMonitorTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/health/HealthMonitorTest.java deleted file mode 100644 index 94ba4726ad0..00000000000 --- a/service-monitor/src/test/java/com/yahoo/vespa/service/health/HealthMonitorTest.java +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.service.health; - -import com.yahoo.vespa.applicationmodel.ServiceStatus; -import org.junit.Test; - -import java.time.Duration; - -import static org.junit.Assert.assertEquals; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -public class HealthMonitorTest { - @Test - public void initiallyDown() { - HealthClient healthClient = mock(HealthClient.class); - try (HealthMonitor monitor = new HealthMonitor(healthClient, Duration.ofHours(12))) { - monitor.startMonitoring(); - assertEquals(ServiceStatus.DOWN, monitor.getStatus()); - } - } - - @Test - public void eventuallyUp() { - HealthClient healthClient = mock(HealthClient.class); - when(healthClient.getHealthInfo()).thenReturn(HealthInfo.fromHealthStatusCode(HealthInfo.UP_STATUS_CODE)); - try (HealthMonitor monitor = new HealthMonitor(healthClient, Duration.ofMillis(10))) { - monitor.startMonitoring(); - - while (monitor.getStatus() != ServiceStatus.UP) { - try { - Thread.sleep(1); - } catch (InterruptedException e) { - // ignore - } - } - } - } -}
\ No newline at end of file diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/health/StateV1HealthModelTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/health/StateV1HealthModelTest.java new file mode 100644 index 00000000000..480691772bb --- /dev/null +++ b/service-monitor/src/test/java/com/yahoo/vespa/service/health/StateV1HealthModelTest.java @@ -0,0 +1,66 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.health; + +import com.yahoo.config.model.api.ApplicationInfo; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.HostName; +import com.yahoo.vespa.applicationmodel.ClusterId; +import com.yahoo.vespa.applicationmodel.ConfigId; +import com.yahoo.vespa.applicationmodel.ServiceStatus; +import com.yahoo.vespa.applicationmodel.ServiceType; +import com.yahoo.vespa.service.duper.ProxyHostApplication; +import com.yahoo.vespa.service.executor.Cancellable; +import com.yahoo.vespa.service.executor.RunletExecutor; +import com.yahoo.vespa.service.model.ServiceId; +import org.junit.Test; + +import java.time.Duration; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * @author hakonhall + */ +public class StateV1HealthModelTest { + private RunletExecutor executor = mock(RunletExecutor.class); + private Duration healthStaleness = Duration.ofSeconds(1); + private Duration requestTimeout = Duration.ofSeconds(2); + private Duration keepAlive = Duration.ofSeconds(3); + private final StateV1HealthModel model = new StateV1HealthModel(healthStaleness, requestTimeout, keepAlive, executor); + private final ProxyHostApplication proxyHostApplication = new ProxyHostApplication(); + private final List<HostName> hostnames = Stream.of("host1", "host2").map(HostName::from).collect(Collectors.toList()); + private final ApplicationInfo proxyHostApplicationInfo = proxyHostApplication.makeApplicationInfo(hostnames); + private final Map<ServiceId, HealthEndpoint> endpoints = model.extractHealthEndpoints(proxyHostApplicationInfo); + + @Test + public void test() { + assertEquals(2, endpoints.size()); + + ApplicationId applicationId = ApplicationId.from("hosted-vespa", "proxy-host", "default"); + ClusterId clusterId = new ClusterId("proxy-host"); + ServiceId hostAdmin1 = new ServiceId(applicationId, clusterId, ServiceType.HOST_ADMIN, new ConfigId("proxy-host/host1")); + ServiceId hostAdmin2 = new ServiceId(applicationId, clusterId, ServiceType.HOST_ADMIN, new ConfigId("proxy-host/host2")); + + HealthEndpoint endpoint1 = endpoints.get(hostAdmin1); + assertNotNull(endpoint1); + assertEquals("http://host1:8080/state/v1/health", endpoint1.description()); + + HealthEndpoint endpoint2 = endpoints.get(hostAdmin2); + assertNotNull(endpoint2); + assertEquals("http://host2:8080/state/v1/health", endpoint2.description()); + + Cancellable cancellable = mock(Cancellable.class); + when(executor.scheduleWithFixedDelay(any(), any())).thenReturn(cancellable); + try (HealthMonitor healthMonitor = endpoint1.startMonitoring()) { + assertEquals(ServiceStatus.DOWN, healthMonitor.getStatus()); + } + } +}
\ No newline at end of file diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/health/StateV1HealthMonitorTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/health/StateV1HealthMonitorTest.java new file mode 100644 index 00000000000..c892118990f --- /dev/null +++ b/service-monitor/src/test/java/com/yahoo/vespa/service/health/StateV1HealthMonitorTest.java @@ -0,0 +1,37 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.service.health; + +import com.yahoo.vespa.applicationmodel.ServiceStatus; +import com.yahoo.vespa.service.executor.RunletExecutor; +import com.yahoo.vespa.service.executor.RunletExecutorImpl; +import org.junit.Test; + +import java.time.Duration; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class StateV1HealthMonitorTest { + @Test + public void downThenUpThenDown() throws Exception { + StateV1HealthClient client = mock(StateV1HealthClient.class); + when(client.get()).thenReturn(HealthInfo.empty()); + + StateV1HealthUpdater updater = new StateV1HealthUpdater(client); + RunletExecutor executor = new RunletExecutorImpl(2); + try (StateV1HealthMonitor monitor = new StateV1HealthMonitor(updater, executor, Duration.ofMillis(10))) { + assertEquals(ServiceStatus.DOWN, monitor.getStatus()); + + when(client.get()).thenReturn(HealthInfo.fromHealthStatusCode(HealthInfo.UP_STATUS_CODE)); + while (monitor.getStatus() != ServiceStatus.UP) { + try { Thread.sleep(2); } catch (InterruptedException ignored) { } + } + + when(client.get()).thenReturn(HealthInfo.fromException(new IllegalStateException("foo"))); + while (monitor.getStatus() != ServiceStatus.DOWN) { + try { Thread.sleep(2); } catch (InterruptedException ignored) { } + } + } + } +}
\ No newline at end of file diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/health/HealthClientTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/health/StateV1HealthUpdaterTest.java index 157b5565071..e7b7a829dac 100644 --- a/service-monitor/src/test/java/com/yahoo/vespa/service/health/HealthClientTest.java +++ b/service-monitor/src/test/java/com/yahoo/vespa/service/health/StateV1HealthUpdaterTest.java @@ -1,16 +1,18 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.service.health; -import com.yahoo.config.provision.HostName; import com.yahoo.vespa.applicationmodel.ServiceStatus; import org.apache.http.HttpEntity; import org.apache.http.StatusLine; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.conn.ConnectTimeoutException; import org.apache.http.impl.client.CloseableHttpClient; +import org.junit.Before; import org.junit.Test; import java.io.IOException; +import java.net.URL; +import java.util.function.Function; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -19,7 +21,14 @@ import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -public class HealthClientTest { +public class StateV1HealthUpdaterTest { + private URL url; + + @Before + public void setUp() throws Exception{ + url = new URL("http://host.com:19071"); + } + @Test public void successfulRequestResponse() throws IOException { HealthInfo info = getHealthInfoFromJsonResponse("{\n" + @@ -96,7 +105,6 @@ public class HealthClientTest { private HealthInfo getHealthInfoFromJsonResponse(String content) throws IOException { - HealthEndpoint endpoint = HealthEndpoint.forHttp(HostName.from("host.com"), 19071); CloseableHttpClient client = mock(CloseableHttpClient.class); CloseableHttpResponse response = mock(CloseableHttpResponse.class); @@ -110,22 +118,22 @@ public class HealthClientTest { HttpEntity httpEntity = mock(HttpEntity.class); when(response.getEntity()).thenReturn(httpEntity); - try (HealthClient healthClient = new HealthClient(endpoint, client, entry -> content)) { - + try (StateV1HealthUpdater updater = makeUpdater(client, entry -> content)) { when(httpEntity.getContentLength()).thenReturn((long) content.length()); - return healthClient.getHealthInfo(); + updater.run(); + return updater.getLatestHealthInfo(); } } @Test public void testRequestException() throws IOException { - HealthEndpoint endpoint = HealthEndpoint.forHttp(HostName.from("host.com"), 19071); CloseableHttpClient client = mock(CloseableHttpClient.class); when(client.execute(any())).thenThrow(new ConnectTimeoutException("exception string")); - try (HealthClient healthClient = new HealthClient(endpoint, client, entry -> "")) { - HealthInfo info = healthClient.getHealthInfo(); + try (StateV1HealthUpdater updater = makeUpdater(client, entry -> "")) { + updater.run(); + HealthInfo info = updater.getLatestHealthInfo(); assertFalse(info.isHealthy()); assertEquals(ServiceStatus.DOWN, info.toServiceStatus()); assertEquals("Exception: exception string", info.toString()); @@ -135,7 +143,6 @@ public class HealthClientTest { @Test public void testBadHttpResponseCode() throws IOException { - HealthEndpoint endpoint = HealthEndpoint.forHttp(HostName.from("host.com"), 19071); CloseableHttpClient client = mock(CloseableHttpClient.class); CloseableHttpResponse response = mock(CloseableHttpResponse.class); @@ -150,13 +157,19 @@ public class HealthClientTest { when(response.getEntity()).thenReturn(httpEntity); String content = "{}"; - try (HealthClient healthClient = new HealthClient(endpoint, client, entry -> content)) { - + try (HealthUpdater updater = makeUpdater(client, entry -> content)) { when(httpEntity.getContentLength()).thenReturn((long) content.length()); - HealthInfo info = healthClient.getHealthInfo(); + updater.run(); + HealthInfo info = updater.getLatestHealthInfo(); assertFalse(info.isHealthy()); assertEquals(ServiceStatus.DOWN, info.toServiceStatus()); assertEquals("Bad HTTP response status code 500", info.toString()); } } + + private StateV1HealthUpdater makeUpdater(CloseableHttpClient client, Function<HttpEntity, String> getContentFunction) { + ApacheHttpClient apacheHttpClient = new ApacheHttpClient(url, client); + StateV1HealthClient healthClient = new StateV1HealthClient(apacheHttpClient, getContentFunction); + return new StateV1HealthUpdater(healthClient); + } }
\ No newline at end of file diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java index b68dc2dd758..d81c9f064b1 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java @@ -122,7 +122,8 @@ public class DefaultZmsClient extends ClientBase implements ZmsClient { @Override public boolean hasAccess(AthenzResourceName resource, String action, AthenzIdentity identity) { - URI uri = zmsUrl.resolve(String.format("access/%s/%s", action, resource.toResourceNameString())); + URI uri = zmsUrl.resolve(String.format("access/%s/%s?principal=%s", + action, resource.toResourceNameString(), identity.getFullName())); HttpUriRequest request = RequestBuilder.get() .setUri(uri) .build(); diff --git a/vespaclient-core/src/main/java/com/yahoo/feedapi/MessageBusSessionFactory.java b/vespaclient-core/src/main/java/com/yahoo/feedapi/MessageBusSessionFactory.java index ce6df32a9a1..54e638717e0 100755 --- a/vespaclient-core/src/main/java/com/yahoo/feedapi/MessageBusSessionFactory.java +++ b/vespaclient-core/src/main/java/com/yahoo/feedapi/MessageBusSessionFactory.java @@ -76,6 +76,7 @@ public class MessageBusSessionFactory implements SessionFactory { } private class SourceSessionWrapper extends SendSession { + private final SourceSession session; private final Metric metric; private final Metric.Context context; diff --git a/vespajlib/src/main/java/com/yahoo/text/Utf8.java b/vespajlib/src/main/java/com/yahoo/text/Utf8.java index 1630d267302..6f40b590a64 100644 --- a/vespajlib/src/main/java/com/yahoo/text/Utf8.java +++ b/vespajlib/src/main/java/com/yahoo/text/Utf8.java @@ -109,12 +109,13 @@ public final class Utf8 { /** * Will try an optimistic approach to utf8 encoding. * That is 4.6x faster that the brute encode for ascii, not accounting for reduced memory footprint and GC. - * @param str The string to encode. + * + * @param string The string to encode. * @return Utf8 encoded array */ - public static byte[] toBytes(String str) { - byte [] utf8 = toBytesAscii(str); - return utf8 != null ? utf8 : str.getBytes(StandardCharsets.UTF_8); + public static byte[] toBytes(String string) { + byte [] utf8 = toBytesAscii(string); + return utf8 != null ? utf8 : string.getBytes(StandardCharsets.UTF_8); } /** * Will try an optimistic approach to utf8 decoding. diff --git a/vespajlib/src/main/java/com/yahoo/vespa/objects/Identifiable.java b/vespajlib/src/main/java/com/yahoo/vespa/objects/Identifiable.java index 0aee5fcdea5..3e392448ce2 100644 --- a/vespajlib/src/main/java/com/yahoo/vespa/objects/Identifiable.java +++ b/vespajlib/src/main/java/com/yahoo/vespa/objects/Identifiable.java @@ -175,9 +175,9 @@ public class Identifiable extends Selectable implements Cloneable { * * <code>public static int classId = registerClass(<id>, <ClassName>.class);</code> * - * @param id The class identifier to register with. - * @param spec The class to register. - * @return the identifier argument. + * @param id the class identifier to register with + * @param spec the class to register + * @return the identifier argument */ protected static int registerClass(int id, Class<? extends Identifiable> spec) { if (registry == null) { diff --git a/vespajlib/src/main/java/com/yahoo/vespa/objects/Ids.java b/vespajlib/src/main/java/com/yahoo/vespa/objects/Ids.java index 64a62ae37f3..02ae6a58bf7 100644 --- a/vespajlib/src/main/java/com/yahoo/vespa/objects/Ids.java +++ b/vespajlib/src/main/java/com/yahoo/vespa/objects/Ids.java @@ -8,8 +8,10 @@ package com.yahoo.vespa.objects; * @author baldersheim */ public interface Ids { - public static int document = 0x1000; - public static int searchlib = 0x4000; - public static int vespa_configmodel = 0x7000; - public static int annotation = 0x10000; + + int document = 0x1000; + int searchlib = 0x4000; + int vespa_configmodel = 0x7000; + int annotation = 0x10000; + } diff --git a/vespalib/src/tests/portal/portal_test.cpp b/vespalib/src/tests/portal/portal_test.cpp index 6d0d04620d0..299340fd131 100644 --- a/vespalib/src/tests/portal/portal_test.cpp +++ b/vespalib/src/tests/portal/portal_test.cpp @@ -83,7 +83,7 @@ struct MyGetHandler : public Portal::GetHandler { std::function<void(Portal::GetRequest)> fun; template <typename F> MyGetHandler(F &&f) : fun(std::move(f)) {} - void get(Portal::GetRequest request) const override { + void get(Portal::GetRequest request) override { fun(std::move(request)); } ~MyGetHandler(); diff --git a/vespalib/src/vespa/vespalib/portal/portal.cpp b/vespalib/src/vespa/vespalib/portal/portal.cpp index 031f364faff..0d62d5728d1 100644 --- a/vespalib/src/vespa/vespalib/portal/portal.cpp +++ b/vespalib/src/vespa/vespalib/portal/portal.cpp @@ -88,7 +88,7 @@ Portal::cancel_token(Token &token) } portal::HandleGuard -Portal::lookup_get_handler(const vespalib::string &uri, const GetHandler *&handler) +Portal::lookup_get_handler(const vespalib::string &uri, GetHandler *&handler) { std::lock_guard guard(_lock); for (const auto &entry: _bind_list) { @@ -130,7 +130,7 @@ Portal::handle_http(portal::HttpConnection *conn) } else if (!conn->get_request().is_get()) { conn->respond_with_error(501, "Not Implemented"); } else { - const GetHandler *get_handler = nullptr; + GetHandler *get_handler = nullptr; auto guard = lookup_get_handler(conn->get_request().get_uri(), get_handler); if (guard.valid()) { assert(get_handler != nullptr); @@ -182,7 +182,7 @@ Portal::create(CryptoEngine::SP crypto, int port) } Portal::Token::UP -Portal::bind(const vespalib::string &path_prefix, const GetHandler &handler) +Portal::bind(const vespalib::string &path_prefix, GetHandler &handler) { auto token = make_token(); std::lock_guard guard(_lock); diff --git a/vespalib/src/vespa/vespalib/portal/portal.h b/vespalib/src/vespa/vespalib/portal/portal.h index aa696c85fa2..93424dda90c 100644 --- a/vespalib/src/vespa/vespalib/portal/portal.h +++ b/vespalib/src/vespa/vespalib/portal/portal.h @@ -67,7 +67,7 @@ public: }; struct GetHandler { - virtual void get(GetRequest request) const = 0; + virtual void get(GetRequest request) = 0; virtual ~GetHandler(); }; @@ -75,8 +75,8 @@ private: struct BindState { uint64_t handle; vespalib::string prefix; - const GetHandler *handler; - BindState(uint64_t handle_in, vespalib::string prefix_in, const GetHandler &handler_in) + GetHandler *handler; + BindState(uint64_t handle_in, vespalib::string prefix_in, GetHandler &handler_in) : handle(handle_in), prefix(prefix_in), handler(&handler_in) {} bool operator<(const BindState &rhs) const { if (prefix.size() == rhs.prefix.size()) { @@ -98,7 +98,7 @@ private: Token::UP make_token(); void cancel_token(Token &token); - portal::HandleGuard lookup_get_handler(const vespalib::string &uri, const GetHandler *&handler); + portal::HandleGuard lookup_get_handler(const vespalib::string &uri, GetHandler *&handler); void evict_handle(uint64_t handle); void handle_accept(portal::HandleGuard guard, SocketHandle socket); @@ -110,7 +110,7 @@ public: static SP create(CryptoEngine::SP crypto, int port); int listen_port() const { return _listener->listen_port(); } const vespalib::string &my_host() const { return _my_host; } - Token::UP bind(const vespalib::string &path_prefix, const GetHandler &handler); + Token::UP bind(const vespalib::string &path_prefix, GetHandler &handler); }; } // namespace vespalib diff --git a/vsm/src/vespa/vsm/config/vsmfields.def b/vsm/src/vespa/vsm/config/vsmfields.def index 0b17875af69..ca48692b526 100644 --- a/vsm/src/vespa/vsm/config/vsmfields.def +++ b/vsm/src/vespa/vsm/config/vsmfields.def @@ -12,7 +12,7 @@ searchall int default=1 fieldspec[].name string ## The search method for a given field. Note: same field in 2 different document types must match on type if not a random result might be expected. -fieldspec[].searchmethod enum { NONE, AUTOUTF8, UTF8, SSE2UTF8, INT8, INT16, INT32, INT64, FLOAT, DOUBLE } default=AUTOUTF8 +fieldspec[].searchmethod enum { NONE, BOOL, AUTOUTF8, UTF8, SSE2UTF8, INT8, INT16, INT32, INT64, FLOAT16, FLOAT, DOUBLE } default=AUTOUTF8 fieldspec[].arg1 string default="" ## Maximum number of chars to search per field. diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/transaction/CuratorTransaction.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/transaction/CuratorTransaction.java index ab1d27ac325..92972c99194 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/transaction/CuratorTransaction.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/transaction/CuratorTransaction.java @@ -68,7 +68,7 @@ public class CuratorTransaction extends AbstractTransaction { @Override public String toString() { - return String.join(",", operations().stream().map(operation -> operation.toString()).collect(Collectors.toList())); + return operations().stream().map(Object::toString).collect(Collectors.joining(",")); } } |