diff options
61 files changed, 561 insertions, 684 deletions
diff --git a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java index 36009682022..7ca9bcf48f3 100644 --- a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java +++ b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java @@ -45,6 +45,7 @@ import java.security.MessageDigest; import java.util.*; import java.util.jar.JarFile; import java.util.logging.Logger; +import java.util.stream.Collectors; import static com.yahoo.text.Lowercase.toLowerCase; @@ -164,7 +165,7 @@ public class FilesApplicationPackage implements ApplicationPackage { return metaData; } - private List<NamedReader> getFiles(Path relativePath,String namePrefix,String suffix,boolean recurse) { + private List<NamedReader> getFiles(Path relativePath, String namePrefix, String suffix, boolean recurse) { try { List<NamedReader> readers=new ArrayList<>(); File dir = new File(appDir, relativePath.getRelative()); diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java b/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java index dd54fe11c39..a71a0878d3d 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java @@ -88,12 +88,14 @@ public interface ApplicationPackage { /** * Contents of services.xml. Caller must close reader after use. + * * @return a Reader, or null if no services.xml/vespa-services.xml present */ Reader getServices(); /** * Contents of hosts.xml. Caller must close reader after use. + * * @return a Reader, or null if no hosts.xml/vespa-hosts.xml present */ Reader getHosts(); @@ -160,8 +162,8 @@ public interface ApplicationPackage { * Gets a file from the root of the application package * * - * @param relativePath The relative path of the file within this application package. - * @return reader for file + * @param relativePath the relative path of the file within this application package. + * @return information abut the file * @throws IllegalArgumentException if the given path does not exist */ ApplicationFile getFile(Path relativePath); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java index 7b4d70d85b1..6311751bb88 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java @@ -52,11 +52,11 @@ public class RankProfileRegistry { } private void checkForDuplicateRankProfile(RankProfile rankProfile) { - final String rankProfileName = rankProfile.getName(); + String rankProfileName = rankProfile.getName(); RankProfile existingRangProfileWithSameName = rankProfiles.get(rankProfile.getSearch()).get(rankProfileName); if (existingRangProfileWithSameName == null) return; - if (!overridableRankProfileNames.contains(rankProfileName)) { + if ( ! overridableRankProfileNames.contains(rankProfileName)) { throw new IllegalArgumentException("Cannot add rank profile '" + rankProfileName + "' in search definition '" + rankProfile.getSearch().getName() + "', since it already exists"); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/ConvertedModel.java b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/ConvertedModel.java index 867740c7912..d85d0983509 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/ConvertedModel.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/ConvertedModel.java @@ -1,11 +1,11 @@ package com.yahoo.searchdefinition.expressiontransforms; -import com.google.common.base.Joiner; import com.yahoo.collections.Pair; import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.model.application.provider.FilesApplicationPackage; import com.yahoo.io.IOUtils; +import com.yahoo.io.reader.NamedReader; import com.yahoo.path.Path; import com.yahoo.search.query.profile.QueryProfileRegistry; import com.yahoo.searchdefinition.FeatureNames; @@ -39,11 +39,14 @@ import com.yahoo.tensor.serialization.TypedBinaryFormat; import java.io.BufferedReader; import java.io.File; +import java.io.FileNotFoundException; import java.io.IOException; +import java.io.Reader; import java.io.StringReader; import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -62,69 +65,155 @@ import java.util.stream.Collectors; */ public class ConvertedModel { - private final ExpressionNode convertedExpression; + private final String modelName; + private final Path modelPath; - public ConvertedModel(FeatureArguments arguments, + /** + * The ranking expressions of this, indexed by their name. which is a 1-3 part string separated by dots + * where the first part is always the model name, the second the signature or (if none) + * expression name (if more than one), and the third is the output name (if any). + */ + private final Map<String, RankingExpression> expressions; + + public ConvertedModel(Path modelPath, RankProfileTransformContext context, ModelImporter modelImporter, - Map<Path, ImportedModel> importedModels) { - ModelStore store = new ModelStore(context.rankProfile().getSearch().sourceApplication(), arguments); + FeatureArguments arguments) { // TODO: Remove + this.modelPath = modelPath; + this.modelName = toModelName(modelPath); + ModelStore store = new ModelStore(context.rankProfile().getSearch().sourceApplication(), modelPath); if ( ! store.hasStoredModel()) // not converted yet - access from models/ directory - convertedExpression = importModel(store, context.rankProfile(), context.queryProfiles(), modelImporter, importedModels); + expressions = importModel(store, context.rankProfile(), context.queryProfiles(), modelImporter, arguments); else - convertedExpression = transformFromStoredModel(store, context.rankProfile()); + expressions = transformFromStoredModel(store, context.rankProfile()); } - private ExpressionNode importModel(ModelStore store, - RankProfile profile, - QueryProfileRegistry queryProfiles, - ModelImporter modelImporter, - Map<Path, ImportedModel> importedModels) { - ImportedModel model = importedModels.computeIfAbsent(store.arguments().modelPath(), - k -> modelImporter.importModel(store.arguments().modelName(), - store.modelDir())); - return transformFromImportedModel(model, store, profile, queryProfiles); + private Map<String, RankingExpression> importModel(ModelStore store, + RankProfile profile, + QueryProfileRegistry queryProfiles, + ModelImporter modelImporter, + FeatureArguments arguments) { + ImportedModel model = modelImporter.importModel(store.modelFiles.modelName(), store.modelDir()); + return transformFromImportedModel(model, store, profile, queryProfiles, arguments); + } + + /** Returns the expression matching the given arguments */ + public ExpressionNode expression(FeatureArguments arguments) { + if (expressions.isEmpty()) + throw new IllegalArgumentException("No expressions available in " + this); + + RankingExpression expression = expressions.get(arguments.toName()); + if (expression != null) return expression.getRoot(); + + if ( ! arguments.signature().isPresent()) { + if (expressions.size() > 1) + throw new IllegalArgumentException("Multiple candidate expressions " + missingExpressionMessageSuffix()); + return expressions.values().iterator().next().getRoot(); + } + + if ( ! arguments.output().isPresent()) { + List<Map.Entry<String, RankingExpression>> entriesWithTheRightPrefix = + expressions.entrySet().stream().filter(entry -> entry.getKey().startsWith(modelName + "." + arguments.signature().get() + ".")).collect(Collectors.toList()); + if (entriesWithTheRightPrefix.size() < 1) + throw new IllegalArgumentException("No expressions named '" + arguments.signature().get() + + missingExpressionMessageSuffix()); + if (entriesWithTheRightPrefix.size() > 1) + throw new IllegalArgumentException("Multiple candidate expression named '" + arguments.signature().get() + + missingExpressionMessageSuffix()); + return entriesWithTheRightPrefix.get(0).getValue().getRoot(); + } + + throw new IllegalArgumentException("No expression '" + arguments.toName() + missingExpressionMessageSuffix()); } - public ExpressionNode expression() { return convertedExpression; } + private String missingExpressionMessageSuffix() { + return "' in model '" + this.modelPath + "'. " + + "Available expressions: " + expressions.keySet().stream().collect(Collectors.joining(", ")); + } - private ExpressionNode transformFromImportedModel(ImportedModel model, - ModelStore store, - RankProfile profile, - QueryProfileRegistry queryProfiles) { + private Map<String, RankingExpression> transformFromImportedModel(ImportedModel model, + ModelStore store, + RankProfile profile, + QueryProfileRegistry queryProfiles, + FeatureArguments arguments) { // Add constants Set<String> constantsReplacedByMacros = new HashSet<>(); model.smallConstants().forEach((k, v) -> transformSmallConstant(store, profile, k, v)); model.largeConstants().forEach((k, v) -> transformLargeConstant(store, profile, queryProfiles, constantsReplacedByMacros, k, v)); - // Find the specified expression - ImportedModel.Signature signature = chooseSignature(model, store.arguments().signature()); - String output = chooseOutput(signature, store.arguments().output()); - if (signature.skippedOutputs().containsKey(output)) { - String message = "Could not import model output '" + output + "'"; - if (!signature.skippedOutputs().get(output).isEmpty()) { - message += ": " + signature.skippedOutputs().get(output); + // Add macros + addGeneratedMacros(model, profile); + + // Add expressions + Map<String, RankingExpression> expressions = new HashMap<>(); + for (Map.Entry<String, ImportedModel.Signature> signatureEntry : model.signatures().entrySet()) { + if ( ! matches(signatureEntry.getValue(), arguments, Optional.empty())) continue; + + for (Map.Entry<String, String> outputEntry : signatureEntry.getValue().outputs().entrySet()) { + if ( ! matches(signatureEntry.getValue(), arguments, Optional.of(outputEntry.getKey()))) continue; + addExpression(model.expressions().get(outputEntry.getValue()), + modelName + "." + signatureEntry.getKey() + "." + outputEntry.getKey(), + constantsReplacedByMacros, + model, store, profile, queryProfiles, + expressions); } - if (!signature.importWarnings().isEmpty()) { - message += ": " + String.join(", ", signature.importWarnings()); + if (signatureEntry.getValue().outputs().isEmpty()) { // fallback: Signature without outputs + addExpression(model.expressions().get(signatureEntry.getKey()), + modelName + "." + signatureEntry.getKey(), + constantsReplacedByMacros, + model, store, profile, queryProfiles, + expressions); } - throw new IllegalArgumentException(message); } + if (model.signatures().isEmpty()) { // fallback: Model without signatures + if (model.expressions().size() == 1) { // Use just model name + addExpression(model.expressions().values().iterator().next(), + modelName, + constantsReplacedByMacros, + model, store, profile, queryProfiles, + expressions); + } + else { + for (Map.Entry<String, RankingExpression> expressionEntry : model.expressions().entrySet()) { + addExpression(expressionEntry.getValue(), + modelName + "." + expressionEntry.getKey(), + constantsReplacedByMacros, + model, store, profile, queryProfiles, + expressions); + } + } + } + + // Transform and save macro - must come after reading expressions due to optimization transforms + model.macros().forEach((k, v) -> transformGeneratedMacro(store, constantsReplacedByMacros, k, v)); - RankingExpression expression = model.expressions().get(output); + return expressions; + } + + private boolean matches(ImportedModel.Signature signature, FeatureArguments arguments, Optional<String> output) { + if ( ! modelName.equals(arguments.modelName)) return false; + if ( arguments.signature.isPresent() && ! signature.name().equals(arguments.signature().get())) return false; + if (output.isPresent() && arguments.output().isPresent() && ! output.get().matches(arguments.output().get())) return false; + return true; + } + + private void addExpression(RankingExpression expression, + String expressionName, + Set<String> constantsReplacedByMacros, + ImportedModel model, + ModelStore store, + RankProfile profile, + QueryProfileRegistry queryProfiles, + Map<String, RankingExpression> expressions) { expression = replaceConstantsByMacros(expression, constantsReplacedByMacros); verifyRequiredMacros(expression, model, profile, queryProfiles); - addGeneratedMacros(model, profile); reduceBatchDimensions(expression, model, profile, queryProfiles); - - model.macros().forEach((k, v) -> transformGeneratedMacro(store, constantsReplacedByMacros, k, v)); - - store.writeConverted(expression); - return expression.getRoot(); + store.writeExpression(expressionName, expression); + expressions.put(expressionName, expression); } - ExpressionNode transformFromStoredModel(ModelStore store, RankProfile profile) { + private Map<String, RankingExpression> transformFromStoredModel(ModelStore store, RankProfile profile) { for (Pair<String, Tensor> constant : store.readSmallConstants()) profile.addConstant(constant.getFirst(), asValue(constant.getSecond())); @@ -137,60 +226,7 @@ public class ConvertedModel { addGeneratedMacroToProfile(profile, macro.getFirst(), macro.getSecond()); } - return store.readConverted().getRoot(); - } - - /** - * Returns the specified, existing signature, or the only signature if none is specified. - * Throws IllegalArgumentException in all other cases. - */ - private ImportedModel.Signature chooseSignature(ImportedModel importResult, Optional<String> signatureName) { - if ( ! signatureName.isPresent()) { - if (importResult.signatures().size() == 0) - throw new IllegalArgumentException("No signatures are available"); - if (importResult.signatures().size() > 1) - throw new IllegalArgumentException("Model has multiple signatures (" + - Joiner.on(", ").join(importResult.signatures().keySet()) + - "), one must be specified " + - "as a second argument to tensorflow()"); - return importResult.signatures().values().stream().findFirst().get(); - } - else { - ImportedModel.Signature signature = importResult.signatures().get(signatureName.get()); - if (signature == null) - throw new IllegalArgumentException("Model does not have the specified signature '" + - signatureName.get() + "'"); - return signature; - } - } - - /** - * Returns the specified, existing output expression, or the only output expression if no output name is specified. - * Throws IllegalArgumentException in all other cases. - */ - private String chooseOutput(ImportedModel.Signature signature, Optional<String> outputName) { - if ( ! outputName.isPresent()) { - if (signature.outputs().size() == 0) - throw new IllegalArgumentException("No outputs are available" + skippedOutputsDescription(signature)); - if (signature.outputs().size() > 1) - throw new IllegalArgumentException(signature + " has multiple outputs (" + - Joiner.on(", ").join(signature.outputs().keySet()) + - "), one must be specified " + - "as a third argument to tensorflow()"); - return signature.outputs().get(signature.outputs().keySet().stream().findFirst().get()); - } - else { - String output = signature.outputs().get(outputName.get()); - if (output == null) { - if (signature.skippedOutputs().containsKey(outputName.get())) - throw new IllegalArgumentException("Could not use output '" + outputName.get() + "': " + - signature.skippedOutputs().get(outputName.get())); - else - throw new IllegalArgumentException("Model does not have the specified output '" + - outputName.get() + "'"); - } - return output; - } + return store.readExpressions(); } private void transformSmallConstant(ModelStore store, RankProfile profile, String constantName, Tensor constantValue) { @@ -227,22 +263,15 @@ public class ConvertedModel { } private void addGeneratedMacroToProfile(RankProfile profile, String macroName, RankingExpression expression) { - if (profile.getMacros().containsKey(macroName)) { + if (profile.getMacros().containsKey(macroName)) throw new IllegalArgumentException("Generated macro '" + macroName + "' already exists."); - } + profile.addMacro(macroName, false); // todo: inline if only used once RankProfile.Macro macro = profile.getMacros().get(macroName); macro.setRankingExpression(expression); macro.setTextualExpression(expression.getRoot().toString()); } - private String skippedOutputsDescription(ImportedModel.Signature signature) { - if (signature.skippedOutputs().isEmpty()) return ""; - StringBuilder b = new StringBuilder(": "); - signature.skippedOutputs().forEach((k, v) -> b.append("Skipping output '").append(k).append("': ").append(v)); - return b.toString(); - } - /** * Verify that the macros referred in the given expression exists in the given rank profile, * and return tensors of the types specified in requiredMacros. @@ -375,8 +404,8 @@ public class ConvertedModel { /** * If batch dimensions have been reduced away above, bring them back here * for any following computation of the tensor. - * Todo: determine when this is not necessary! */ + // TODO: determine when this is not necessary! private ExpressionNode expandBatchDimensionsAtOutput(ExpressionNode node, TensorType before, TensorType after) { if (after.equals(before)) { return node; @@ -452,24 +481,29 @@ public class ConvertedModel { return new TensorValue(tensor); } + private static String toModelName(Path modelPath) { + return modelPath.toString().replace("/", "_"); + } + + @Override + public String toString() { return "model '" + modelName + "'"; } + /** * Provides read/write access to the correct directories of the application package given by the feature arguments */ static class ModelStore { private final ApplicationPackage application; - private final FeatureArguments arguments; + private final ModelFiles modelFiles; - ModelStore(ApplicationPackage application, FeatureArguments arguments) { + ModelStore(ApplicationPackage application, Path modelPath) { this.application = application; - this.arguments = arguments; + this.modelFiles = new ModelFiles(modelPath); } - public FeatureArguments arguments() { return arguments; } - public boolean hasStoredModel() { try { - return application.getFile(arguments.expressionPath()).exists(); + return application.getFileReference(modelFiles.storedModelPath()).exists(); } catch (UnsupportedOperationException e) { return false; @@ -480,40 +514,49 @@ public class ConvertedModel { * Returns the directory which contains the source model to use for these arguments */ public File modelDir() { - return application.getFileReference(ApplicationPackage.MODELS_DIR.append(arguments.modelPath())); + return application.getFileReference(ApplicationPackage.MODELS_DIR.append(modelFiles.modelPath())); } /** * Adds this expression to the application package, such that it can be read later. + * + * @param name the name of this ranking expression - may have 1-3 parts separated by dot where the first part + * is always the model name */ - void writeConverted(RankingExpression expression) { - application.getFile(arguments.expressionPath()) + void writeExpression(String name, RankingExpression expression) { + application.getFile(modelFiles.expressionPath(name)) .writeFile(new StringReader(expression.getRoot().toString())); } - /** Reads the previously stored ranking expression for these arguments */ - RankingExpression readConverted() { - try { - return new RankingExpression(application.getFile(arguments.expressionPath()).createReader()); - } - catch (IOException e) { - throw new UncheckedIOException("Could not read " + arguments.expressionPath(), e); - } - catch (ParseException e) { - throw new IllegalStateException("Could not parse " + arguments.expressionPath(), e); + Map<String, RankingExpression> readExpressions() { + Map<String, RankingExpression> expressions = new HashMap<>(); + ApplicationFile expressionPath = application.getFile(modelFiles.expressionsPath()); + if ( ! expressionPath.exists() || ! expressionPath.isDirectory()) return Collections.emptyMap(); + for (ApplicationFile expressionFile : expressionPath.listFiles()) { + try { + String name = expressionFile.getPath().getName(); + expressions.put(name, new RankingExpression(name, expressionFile.createReader())); + } + catch (FileNotFoundException e) { + throw new IllegalStateException("Expression file removed while reading: " + expressionFile, e); + } + catch (ParseException e) { + throw new IllegalStateException("Invalid stored expression in " + expressionFile, e); + } } + return expressions; } /** Adds this macro expression to the application package to it can be read later. */ void writeMacro(String name, RankingExpression expression) { - application.getFile(arguments.macrosPath()).appendFile(name + "\t" + + application.getFile(modelFiles.macrosPath()).appendFile(name + "\t" + expression.getRoot().toString() + "\n"); } /** Reads the previously stored macro expressions for these arguments */ List<Pair<String, RankingExpression>> readMacros() { try { - ApplicationFile file = application.getFile(arguments.macrosPath()); + ApplicationFile file = application.getFile(modelFiles.macrosPath()); if (!file.exists()) return Collections.emptyList(); List<Pair<String, RankingExpression>> macros = new ArrayList<>(); @@ -527,7 +570,7 @@ public class ConvertedModel { macros.add(new Pair<>(name, expression)); } catch (ParseException e) { - throw new IllegalStateException("Could not parse " + arguments.expressionPath(), e); + throw new IllegalStateException("Could not parse " + name, e); } } return macros; @@ -544,7 +587,7 @@ public class ConvertedModel { List<RankingConstant> readLargeConstants() { try { List<RankingConstant> constants = new ArrayList<>(); - for (ApplicationFile constantFile : application.getFile(arguments.largeConstantsPath()).listFiles()) { + for (ApplicationFile constantFile : application.getFile(modelFiles.largeConstantsPath()).listFiles()) { String[] parts = IOUtils.readAll(constantFile.createReader()).split(":"); constants.add(new RankingConstant(parts[0], TensorType.fromSpec(parts[1]), parts[2])); } @@ -562,13 +605,13 @@ public class ConvertedModel { * @return the path to the stored constant, relative to the application package root */ Path writeLargeConstant(String name, Tensor constant) { - Path constantsPath = ApplicationPackage.MODELS_GENERATED_DIR.append(arguments.modelPath).append("constants"); + Path constantsPath = ApplicationPackage.MODELS_GENERATED_DIR.append(modelFiles.modelPath()).append("constants"); // "tbf" ending for "typed binary format" - recognized by the nodes receiving the file: Path constantPath = constantsPath.append(name + ".tbf"); // Remember the constant in a file we replicate in ZooKeeper - application.getFile(arguments.largeConstantsPath().append(name + ".constant")) + application.getFile(modelFiles.largeConstantsPath().append(name + ".constant")) .writeFile(new StringReader(name + ":" + constant.type() + ":" + correct(constantPath))); // Write content explicitly as a file on the file system as this is distributed using file distribution @@ -579,7 +622,7 @@ public class ConvertedModel { private List<Pair<String, Tensor>> readSmallConstants() { try { - ApplicationFile file = application.getFile(arguments.smallConstantsPath()); + ApplicationFile file = application.getFile(modelFiles.smallConstantsPath()); if (!file.exists()) return Collections.emptyList(); List<Pair<String, Tensor>> constants = new ArrayList<>(); @@ -604,7 +647,7 @@ public class ConvertedModel { */ public void writeSmallConstant(String name, Tensor constant) { // Secret file format for remembering constants: - application.getFile(arguments.smallConstantsPath()).appendFile(name + "\t" + + application.getFile(modelFiles.smallConstantsPath()).appendFile(name + "\t" + constant.type().toString() + "\t" + constant.toString() + "\n"); } @@ -628,26 +671,24 @@ public class ConvertedModel { } } + private void close(Reader reader) { + try { + if (reader != null) + reader.close(); + } + catch (IOException e) { + // ignore + } + } + } - /** Encapsulates the arguments to the import feature */ - static class FeatureArguments { + static class ModelFiles { Path modelPath; - /** Optional arguments */ - Optional<String> signature, output; - - public FeatureArguments(Arguments arguments) { - this(Path.fromString(asString(arguments.expressions().get(0))), - optionalArgument(1, arguments), - optionalArgument(2, arguments)); - } - - public FeatureArguments(Path modelPath, Optional<String> signature, Optional<String> output) { + public ModelFiles(Path modelPath) { this.modelPath = modelPath; - this.signature = signature; - this.output = output; } /** Returns modelPath with slashes replaced by underscores */ @@ -655,37 +696,66 @@ public class ConvertedModel { /** Returns relative path to this model below the "models/" dir in the application package */ public Path modelPath() { return modelPath; } - public Optional<String> signature() { return signature; } - public Optional<String> output() { return output; } - /** Path to the small constants file */ + public Path storedModelPath() { + return ApplicationPackage.MODELS_GENERATED_REPLICATED_DIR.append(modelPath()); + } + + public Path expressionPath(String name) { + return storedModelPath().append("expressions").append(name); + } + + public Path expressionsPath() { + return storedModelPath().append("expressions"); + } + public Path smallConstantsPath() { - return ApplicationPackage.MODELS_GENERATED_DIR.append(modelPath).append("constants.txt"); + return storedModelPath().append("constants.txt"); } /** Path to the large (ranking) constants directory */ public Path largeConstantsPath() { - return ApplicationPackage.MODELS_GENERATED_REPLICATED_DIR.append(modelPath).append("constants"); + return storedModelPath().append("constants"); } /** Path to the macros file */ public Path macrosPath() { - return ApplicationPackage.MODELS_GENERATED_REPLICATED_DIR.append(modelPath).append("macros.txt"); + return storedModelPath().append("macros.txt"); } - public Path expressionPath() { - return ApplicationPackage.MODELS_GENERATED_REPLICATED_DIR - .append(modelPath).append("expressions").append(expressionFileName()); + } + + /** Encapsulates the arguments of a specific model output */ + static class FeatureArguments { + + private final String modelName; + private final Path modelPath; + + /** Optional arguments */ + private final Optional<String> signature, output; + + public FeatureArguments(Arguments arguments) { + this(Path.fromString(asString(arguments.expressions().get(0))), + optionalArgument(1, arguments), + optionalArgument(2, arguments)); } - private String expressionFileName() { - StringBuilder fileName = new StringBuilder(); - signature.ifPresent(s -> fileName.append(s).append(".")); - output.ifPresent(s -> fileName.append(s).append(".")); - if (fileName.length() == 0) // single signature and output - fileName.append("single."); - fileName.append("expression"); - return fileName.toString(); + public FeatureArguments(Path modelPath, Optional<String> signature, Optional<String> output) { + this.modelPath = modelPath; + this.modelName = toModelName(modelPath); + this.signature = signature; + this.output = output; + } + + public Path modelPath() { return modelPath; } + + public Optional<String> signature() { return signature; } + public Optional<String> output() { return output; } + + public String toName() { + return modelName + + (signature.isPresent() ? "." + signature.get() : "") + + (output.isPresent() ? "." + output.get() : ""); } private static Optional<String> optionalArgument(int argumentIndex, Arguments arguments) { @@ -694,7 +764,7 @@ public class ConvertedModel { return Optional.of(asString(arguments.expressions().get(argumentIndex))); } - private static String asString(ExpressionNode node) { + public static String asString(ExpressionNode node) { if ( ! (node instanceof ConstantNode)) throw new IllegalArgumentException("Expected a constant string as argument, but got '" + node); return stripQuotes(((ConstantNode)node).sourceString()); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/OnnxFeatureConverter.java b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/OnnxFeatureConverter.java index d31ffefde65..97395c1aad3 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/OnnxFeatureConverter.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/OnnxFeatureConverter.java @@ -31,7 +31,7 @@ public class OnnxFeatureConverter extends ExpressionTransformer<RankProfileTrans private final OnnxImporter onnxImporter = new OnnxImporter(); /** A cache of imported models indexed by model path. This avoids importing the same model multiple times. */ - private final Map<Path, ImportedModel> importedModels = new HashMap<>(); + private final Map<Path, ConvertedModel> convertedModels = new HashMap<>(); @Override public ExpressionNode transform(ExpressionNode node, RankProfileTransformContext context) { @@ -47,9 +47,9 @@ public class OnnxFeatureConverter extends ExpressionTransformer<RankProfileTrans if ( ! feature.getName().equals("onnx")) return feature; try { - ConvertedModel convertedModel = new ConvertedModel(asFeatureArguments(feature.getArguments()), - context, onnxImporter, importedModels); - return convertedModel.expression(); + Path modelPath = Path.fromString(ConvertedModel.FeatureArguments.asString(feature.getArguments().expressions().get(0))); + ConvertedModel convertedModel = convertedModels.computeIfAbsent(modelPath, path -> new ConvertedModel(path, context, onnxImporter, new ConvertedModel.FeatureArguments(feature.getArguments()))); + return convertedModel.expression(asFeatureArguments(feature.getArguments())); } catch (IllegalArgumentException | UncheckedIOException e) { throw new IllegalArgumentException("Could not use Onnx model from " + feature, e); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java index d28299b1d30..b3778e2af84 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java @@ -28,7 +28,7 @@ public class TensorFlowFeatureConverter extends ExpressionTransformer<RankProfil private final TensorFlowImporter tensorFlowImporter = new TensorFlowImporter(); /** A cache of imported models indexed by model path. This avoids importing the same model multiple times. */ - private final Map<Path, ImportedModel> importedModels = new HashMap<>(); + private final Map<Path, ConvertedModel> convertedModels = new HashMap<>(); @Override public ExpressionNode transform(ExpressionNode node, RankProfileTransformContext context) { @@ -44,9 +44,9 @@ public class TensorFlowFeatureConverter extends ExpressionTransformer<RankProfil if ( ! feature.getName().equals("tensorflow")) return feature; try { - ConvertedModel convertedModel = new ConvertedModel(asFeatureArguments(feature.getArguments()), - context, tensorFlowImporter, importedModels); - return convertedModel.expression(); + Path modelPath = Path.fromString(ConvertedModel.FeatureArguments.asString(feature.getArguments().expressions().get(0))); + ConvertedModel convertedModel = convertedModels.computeIfAbsent(modelPath, path -> new ConvertedModel(path, context, tensorFlowImporter, new ConvertedModel.FeatureArguments(feature.getArguments()))); + return convertedModel.expression(asFeatureArguments(feature.getArguments())); } catch (IllegalArgumentException | UncheckedIOException e) { throw new IllegalArgumentException("Could not use tensorflow model from " + feature, e); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/XgboostFeatureConverter.java b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/XgboostFeatureConverter.java index 4ae223ec3a5..62f43e15849 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/XgboostFeatureConverter.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/XgboostFeatureConverter.java @@ -37,7 +37,8 @@ public class XgboostFeatureConverter extends ExpressionTransformer<RankProfileTr try { ConvertedModel.FeatureArguments arguments = asFeatureArguments(feature.getArguments()); - ConvertedModel.ModelStore store = new ConvertedModel.ModelStore(context.rankProfile().getSearch().sourceApplication(), arguments); + ConvertedModel.ModelStore store = new ConvertedModel.ModelStore(context.rankProfile().getSearch().sourceApplication(), + arguments.modelPath()); RankingExpression expression = xgboostImporter.parseModel(store.modelDir().toString()); return expression.getRoot(); } catch (IllegalArgumentException | UncheckedIOException e) { @@ -48,7 +49,7 @@ public class XgboostFeatureConverter extends ExpressionTransformer<RankProfileTr private ConvertedModel.FeatureArguments asFeatureArguments(Arguments arguments) { if (arguments.isEmpty()) throw new IllegalArgumentException("An xgboost node must take an argument pointing to " + - "the xgboost model directory under [application]/models"); + "the xgboost model directory under [application]/models"); if (arguments.expressions().size() > 1) throw new IllegalArgumentException("An xgboost feature can have at most 1 argument"); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java index ff8d3211d47..e34d490afe1 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java @@ -28,6 +28,7 @@ import java.util.stream.Collectors; * @author bratseth */ public class DomAdminV4Builder extends DomAdminBuilderBase { + private ApplicationId ZONE_APPLICATION_ID = ApplicationId.from("hosted-vespa", "routing", "default"); private final Collection<ContainerModel> containerModels; diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java index 0ce6129ef7f..ab689b88993 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java @@ -10,7 +10,9 @@ import com.yahoo.searchdefinition.Search; import com.yahoo.searchdefinition.SearchBuilder; import com.yahoo.searchdefinition.parser.ParseException; +import java.util.HashMap; import java.util.List; +import java.util.Map; import static org.junit.Assert.assertEquals; @@ -25,6 +27,7 @@ class RankProfileSearchFixture { private RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); private final QueryProfileRegistry queryProfileRegistry; private Search search; + private Map<String, RankProfile> compiledRankProfiles = new HashMap<>(); RankProfileSearchFixture(String rankProfiles) throws ParseException { this(MockApplicationPackage.createEmpty(), new QueryProfileRegistry(), rankProfiles); @@ -54,25 +57,38 @@ class RankProfileSearchFixture { } public void assertFirstPhaseExpression(String expExpression, String rankProfile) { - assertEquals(expExpression, rankProfile(rankProfile).getFirstPhaseRanking().getRoot().toString()); + assertEquals(expExpression, compiledRankProfile(rankProfile).getFirstPhaseRanking().getRoot().toString()); } public void assertSecondPhaseExpression(String expExpression, String rankProfile) { - assertEquals(expExpression, rankProfile(rankProfile).getSecondPhaseRanking().getRoot().toString()); + assertEquals(expExpression, compiledRankProfile(rankProfile).getSecondPhaseRanking().getRoot().toString()); } public void assertRankProperty(String expValue, String name, String rankProfile) { - List<RankProfile.RankProperty> rankPropertyList = rankProfile(rankProfile).getRankPropertyMap().get(name); + List<RankProfile.RankProperty> rankPropertyList = compiledRankProfile(rankProfile).getRankPropertyMap().get(name); assertEquals(1, rankPropertyList.size()); assertEquals(expValue, rankPropertyList.get(0).getValue()); } - public void assertMacro(String expExpression, String macroName, String rankProfile) { - assertEquals(expExpression, rankProfile(rankProfile).getMacros().get(macroName).getRankingExpression().getRoot().toString()); + public void assertMacro(String expexctedExpression, String macroName, String rankProfile) { + assertEquals(expexctedExpression, + compiledRankProfile(rankProfile).getMacros().get(macroName).getRankingExpression().getRoot().toString()); } + public RankProfile compileRankProfile(String rankProfile) { + RankProfile compiled = rankProfileRegistry.getRankProfile(search, rankProfile).compile(queryProfileRegistry); + compiledRankProfiles.put(rankProfile, compiled); + return compiled; + } + + /** Returns the given uncompiled profile */ public RankProfile rankProfile(String rankProfile) { - return rankProfileRegistry.getRankProfile(search, rankProfile).compile(queryProfileRegistry); + return rankProfileRegistry.getRankProfile(search, rankProfile); + } + + /** Returns the given compiled profile, or null if not compiled yet or not present at all */ + public RankProfile compiledRankProfile(String rankProfile) { + return compiledRankProfiles.get(rankProfile); } public Search search() { return search; } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java index b2ef08dcc36..a7465fa9695 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java @@ -123,6 +123,7 @@ public class RankingExpressionWithOnnxTestCase { " expression: onnx('mnist_softmax.onnx')" + " }\n" + " }"); + search.compileRankProfile("my_profile"); search.assertFirstPhaseExpression(vespaExpression, "my_profile"); fail("Expecting exception"); } @@ -164,7 +165,8 @@ public class RankingExpressionWithOnnxTestCase { catch (IllegalArgumentException expected) { assertEquals("Rank profile 'my_profile' is invalid: Could not use Onnx model from " + "onnx('mnist_softmax.onnx','y'): " + - "Model does not have the specified signature 'y'", + "No expressions available in model 'mnist_softmax.onnx'", +// "No expressions named 'y' in model 'mnist_softmax.onnx'. Available expressions: mnist_softmax.onnx.default.add", Exceptions.toMessageString(expected)); } } @@ -220,7 +222,8 @@ public class RankingExpressionWithOnnxTestCase { String vespaExpressionWithoutConstant = "join(reduce(join(rename(Placeholder, (d0, d1), (d0, d2)), mnist_softmax_onnx_Variable, f(a,b)(a * b)), sum, d2), constant(mnist_softmax_onnx_Variable_1), f(a,b)(a + b))"; - RankProfileSearchFixture search = fixtureWith(rankProfile, new StoringApplicationPackage(applicationDir)); + RankProfileSearchFixture search = uncompiledFixtureWith(rankProfile, new StoringApplicationPackage(applicationDir)); + search.compileRankProfile("my_profile"); search.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile"); assertNull("Constant overridden by macro is not added", @@ -234,7 +237,8 @@ public class RankingExpressionWithOnnxTestCase { IOUtils.copyDirectory(applicationDir.append(ApplicationPackage.MODELS_GENERATED_DIR).toFile(), storedApplicationDirectory.append(ApplicationPackage.MODELS_GENERATED_DIR).toFile()); StoringApplicationPackage storedApplication = new StoringApplicationPackage(storedApplicationDirectory); - RankProfileSearchFixture searchFromStored = fixtureWith(rankProfile, storedApplication); + RankProfileSearchFixture searchFromStored = uncompiledFixtureWith(rankProfile, storedApplication); + searchFromStored.compileRankProfile("my_profile"); searchFromStored.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile"); assertNull("Constant overridden by macro is not added", searchFromStored.search().getRankingConstants().get("mnist_softmax_onnx_Variable")); @@ -271,19 +275,19 @@ public class RankingExpressionWithOnnxTestCase { private RankProfileSearchFixture fixtureWith(String placeholderExpression, String firstPhaseExpression) { return fixtureWith(placeholderExpression, firstPhaseExpression, null, null, "Placeholder", - new StoringApplicationPackage(applicationDir)); + new StoringApplicationPackage(applicationDir)); } private RankProfileSearchFixture fixtureWith(String placeholderExpression, String firstPhaseExpression, String constant, String field) { return fixtureWith(placeholderExpression, firstPhaseExpression, constant, field, "Placeholder", - new StoringApplicationPackage(applicationDir)); + new StoringApplicationPackage(applicationDir)); } - private RankProfileSearchFixture fixtureWith(String rankProfile, StoringApplicationPackage application) { + private RankProfileSearchFixture uncompiledFixtureWith(String rankProfile, StoringApplicationPackage application) { try { return new RankProfileSearchFixture(application, application.getQueryProfiles(), - rankProfile, null, null); + rankProfile, null, null); } catch (ParseException e) { throw new IllegalArgumentException(e); @@ -297,7 +301,7 @@ public class RankingExpressionWithOnnxTestCase { String macroName, StoringApplicationPackage application) { try { - return new RankProfileSearchFixture( + RankProfileSearchFixture fixture = new RankProfileSearchFixture( application, application.getQueryProfiles(), " rank-profile my_profile {\n" + @@ -310,6 +314,8 @@ public class RankingExpressionWithOnnxTestCase { " }", constant, field); + fixture.compileRankProfile("my_profile"); + return fixture; } catch (ParseException e) { throw new IllegalArgumentException(e); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java index 7228af2b0de..29859817736 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java @@ -6,6 +6,7 @@ import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.model.test.MockApplicationPackage; import com.yahoo.io.GrowableByteBuffer; import com.yahoo.io.IOUtils; +import com.yahoo.io.reader.NamedReader; import com.yahoo.path.Path; import com.yahoo.search.query.profile.QueryProfileRegistry; import com.yahoo.searchdefinition.RankingConstant; @@ -22,10 +23,12 @@ import java.io.BufferedInputStream; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; +import java.io.FileReader; import java.io.IOException; import java.io.InputStream; import java.io.Reader; import java.io.UncheckedIOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.Iterator; @@ -156,6 +159,7 @@ public class RankingExpressionWithTensorFlowTestCase { " expression: tensorflow('mnist_softmax/saved')" + " }\n" + " }"); + search.compileRankProfile("my_profile"); search.assertFirstPhaseExpression(vespaExpression, "my_profile"); fail("Expecting exception"); } @@ -196,7 +200,9 @@ public class RankingExpressionWithTensorFlowTestCase { catch (IllegalArgumentException expected) { assertEquals("Rank profile 'my_profile' is invalid: Could not use tensorflow model from " + "tensorflow('mnist_softmax/saved','serving_defaultz'): " + - "Model does not have the specified signature 'serving_defaultz'", + "No expressions available in model 'mnist_softmax_saved'", +// "No expressions named 'serving_defaultz' in model 'mnist_softmax/saved'. "+ +// "Available expressions: mnist_softmax_saved.serving_default.y", Exceptions.toMessageString(expected)); } } @@ -212,7 +218,9 @@ public class RankingExpressionWithTensorFlowTestCase { catch (IllegalArgumentException expected) { assertEquals("Rank profile 'my_profile' is invalid: Could not use tensorflow model from " + "tensorflow('mnist_softmax/saved','serving_default','x'): " + - "Model does not have the specified output 'x'", + "No expressions available in model 'mnist_softmax_saved'", +// "No expression 'mnist_softmax_saved.serving_default.x' in model 'mnist_softmax/saved'. " + +// "Available expressions: mnist_softmax_saved.serving_default.y", Exceptions.toMessageString(expected)); } } @@ -268,7 +276,8 @@ public class RankingExpressionWithTensorFlowTestCase { String vespaExpressionWithoutConstant = "join(reduce(join(rename(Placeholder, (d0, d1), (d0, d2)), mnist_softmax_saved_layer_Variable_read, f(a,b)(a * b)), sum, d2), constant(mnist_softmax_saved_layer_Variable_1_read), f(a,b)(a + b))"; - RankProfileSearchFixture search = fixtureWith(rankProfile, new StoringApplicationPackage(applicationDir)); + RankProfileSearchFixture search = fixtureWithUncompiled(rankProfile, new StoringApplicationPackage(applicationDir)); + search.compileRankProfile("my_profile"); search.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile"); assertNull("Constant overridden by macro is not added", @@ -282,7 +291,8 @@ public class RankingExpressionWithTensorFlowTestCase { IOUtils.copyDirectory(applicationDir.append(ApplicationPackage.MODELS_GENERATED_DIR).toFile(), storedApplicationDirectory.append(ApplicationPackage.MODELS_GENERATED_DIR).toFile()); StoringApplicationPackage storedApplication = new StoringApplicationPackage(storedApplicationDirectory); - RankProfileSearchFixture searchFromStored = fixtureWith(rankProfile, storedApplication); + RankProfileSearchFixture searchFromStored = fixtureWithUncompiled(rankProfile, storedApplication); + searchFromStored.compileRankProfile("my_profile"); searchFromStored.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile"); assertNull("Constant overridden by macro is not added", searchFromStored.search().getRankingConstants().get("mnist_softmax_saved_layer_Variable_read")); @@ -297,7 +307,7 @@ public class RankingExpressionWithTensorFlowTestCase { public void testTensorFlowReduceBatchDimension() { final String expression = "join(join(reduce(join(reduce(rename(Placeholder, (d0, d1), (d0, d2)), sum, d0), constant(mnist_softmax_saved_layer_Variable_read), f(a,b)(a * b)), sum, d2), constant(mnist_softmax_saved_layer_Variable_1_read), f(a,b)(a + b)), tensor(d0[1])(1.0), f(a,b)(a * b))"; RankProfileSearchFixture search = fixtureWith("tensor(d0[1],d1[784])(0.0)", - "tensorflow('mnist_softmax/saved')"); + "tensorflow('mnist_softmax/saved')"); search.assertFirstPhaseExpression(expression, "my_profile"); assertLargeConstant("mnist_softmax_saved_layer_Variable_1_read", search, Optional.of(10L)); assertLargeConstant("mnist_softmax_saved_layer_Variable_read", search, Optional.of(7840L)); @@ -362,7 +372,7 @@ public class RankingExpressionWithTensorFlowTestCase { } private void assertSmallConstant(String name, TensorType type, RankProfileSearchFixture search) { - Value value = search.rankProfile("my_profile").getConstants().get(name); + Value value = search.compiledRankProfile("my_profile").getConstants().get(name); assertNotNull(value); assertEquals(type, value.type()); } @@ -410,7 +420,7 @@ public class RankingExpressionWithTensorFlowTestCase { String macroName, StoringApplicationPackage application) { try { - return new RankProfileSearchFixture( + RankProfileSearchFixture fixture = new RankProfileSearchFixture( application, application.getQueryProfiles(), " rank-profile my_profile {\n" + @@ -423,13 +433,15 @@ public class RankingExpressionWithTensorFlowTestCase { " }", constant, field); + fixture.compileRankProfile("my_profile"); + return fixture; } catch (ParseException e) { throw new IllegalArgumentException(e); } } - private RankProfileSearchFixture fixtureWith(String rankProfile, StoringApplicationPackage application) { + private RankProfileSearchFixture fixtureWithUncompiled(String rankProfile, StoringApplicationPackage application) { try { return new RankProfileSearchFixture(application, application.getQueryProfiles(), rankProfile, null, null); @@ -463,6 +475,21 @@ public class RankingExpressionWithTensorFlowTestCase { return new StoringApplicationPackageFile(file, Path.fromString(root.toString())); } + @Override + public List<NamedReader> getFiles(Path path, String suffix) { + List<NamedReader> readers = new ArrayList<>(); + for (File file : getFileReference(path).listFiles()) { + if ( ! file.getName().endsWith(suffix)) continue; + try { + readers.add(new NamedReader(file.getName(), new FileReader(file))); + } + catch (IOException e) { + throw new UncheckedIOException(e); + } + } + return readers; + } + } static class StoringApplicationPackageFile extends ApplicationFile { diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorTestCase.java index dba2bdbfbbf..0866d3192cf 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorTestCase.java @@ -25,6 +25,7 @@ public class RankingExpressionWithTensorTestCase { " }\n" + " }\n" + " }"); + f.compileRankProfile("my_profile"); f.assertFirstPhaseExpression("reduce(constant(my_tensor), sum)", "my_profile"); f.assertRankProperty("{{x:1,y:2}:1.0,{x:2,y:1}:2.0}", "constant(my_tensor).value", "my_profile"); f.assertRankProperty("tensor(x{},y{})", "constant(my_tensor).type", "my_profile"); @@ -47,6 +48,7 @@ public class RankingExpressionWithTensorTestCase { " }\n" + " }\n" + " }"); + f.compileRankProfile("my_profile"); f.assertFirstPhaseExpression("reduce(constant(my_tensor), sum)", "my_profile"); f.assertRankProperty("{{x:1,y:2}:1.0,{x:2,y:1}:2.0}", "constant(my_tensor).value", "my_profile"); f.assertRankProperty("tensor(x{},y{})", "constant(my_tensor).type", "my_profile"); @@ -65,6 +67,7 @@ public class RankingExpressionWithTensorTestCase { " }\n" + " }\n" + " }"); + f.compileRankProfile("my_profile"); f.assertSecondPhaseExpression("reduce(constant(my_tensor), sum)", "my_profile"); f.assertRankProperty("{{x:1}:1.0}", "constant(my_tensor).value", "my_profile"); f.assertRankProperty("tensor(x{})", "constant(my_tensor).type", "my_profile"); @@ -85,6 +88,7 @@ public class RankingExpressionWithTensorTestCase { " expression: sum(my_tensor)\n" + " }\n" + " }"); + f.compileRankProfile("my_profile"); f.assertFirstPhaseExpression("reduce(constant(my_tensor), sum)", "my_profile"); f.assertRankProperty("{{x:1}:1.0}", "constant(my_tensor).value", "my_profile"); f.assertRankProperty("tensor(x{})", "constant(my_tensor).type", "my_profile"); @@ -106,6 +110,7 @@ public class RankingExpressionWithTensorTestCase { " }\n" + " }\n" + " }"); + f.compileRankProfile("my_profile"); f.assertFirstPhaseExpression("5.0 + my_macro", "my_profile"); f.assertMacro("reduce(constant(my_tensor), sum)", "my_macro", "my_profile"); f.assertRankProperty("{{x:1}:1.0}", "constant(my_tensor).value", "my_profile"); @@ -127,6 +132,7 @@ public class RankingExpressionWithTensorTestCase { " my_number_2: 5.0\n" + " }\n" + " }"); + f.compileRankProfile("my_profile"); f.assertFirstPhaseExpression("3.0 + reduce(constant(my_tensor), sum) + 5.0", "my_profile"); f.assertRankProperty("{{x:1}:1.0}", "constant(my_tensor).value", "my_profile"); f.assertRankProperty("tensor(x{})", "constant(my_tensor).type", "my_profile"); @@ -139,7 +145,7 @@ public class RankingExpressionWithTensorTestCase { public void requireThatInvalidTensorTypeSpecThrowsException() throws ParseException { exception.expect(IllegalArgumentException.class); exception.expectMessage("For constant tensor 'my_tensor' in rank profile 'my_profile': Illegal tensor type spec: Failed parsing element 'x' in type spec 'tensor(x)'"); - new RankProfileSearchFixture( + RankProfileSearchFixture f = new RankProfileSearchFixture( " rank-profile my_profile {\n" + " constants {\n" + " my_tensor {\n" + @@ -148,6 +154,7 @@ public class RankingExpressionWithTensorTestCase { " }\n" + " }\n" + " }"); + f.compileRankProfile("my_profile"); } } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithXgboostTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithXgboostTestCase.java index b65cb0b3d5f..f98783ad671 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithXgboostTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithXgboostTestCase.java @@ -36,7 +36,7 @@ public class RankingExpressionWithXgboostTestCase { String field, RankingExpressionWithTensorFlowTestCase.StoringApplicationPackage application) { try { - return new RankProfileSearchFixture( + RankProfileSearchFixture fixture = new RankProfileSearchFixture( application, application.getQueryProfiles(), " rank-profile my_profile {\n" + @@ -46,6 +46,8 @@ public class RankingExpressionWithXgboostTestCase { " }", constant, field); + fixture.compileRankProfile("my_profile"); + return fixture; } catch (ParseException e) { throw new IllegalArgumentException(e); } diff --git a/configd/src/apps/sentinel/metrics.cpp b/configd/src/apps/sentinel/metrics.cpp index df1b0076001..812ab56cd15 100644 --- a/configd/src/apps/sentinel/metrics.cpp +++ b/configd/src/apps/sentinel/metrics.cpp @@ -1,13 +1,9 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "metrics.h" -#include <vespa/log/log.h> -LOG_SETUP(".metrics"); - #include <vespa/vespalib/metrics/simple_metrics.h> -namespace config { -namespace sentinel { +namespace config::sentinel { using vespalib::metrics::SimpleMetricsManager; using vespalib::metrics::SimpleManagerConfig; @@ -17,9 +13,7 @@ StartMetrics::StartMetrics() producer(metrics), currentlyRunningServices(0), totalRestartsCounter(0), - totalRestartsLastPeriod(1), startedTime(time(nullptr)), - lastLoggedTime(startedTime - 55), sentinel_restarts(metrics->counter("sentinel.restarts", "how many times sentinel restarted a service")), sentinel_totalRestarts(metrics->gauge("sentinel.totalRestarts", @@ -33,20 +27,7 @@ StartMetrics::StartMetrics() sentinel_restarts.add(); } -void -StartMetrics::output() -{ - EV_VALUE("currently_running_services", currentlyRunningServices); - EV_VALUE("total_restarts_last_period", totalRestartsLastPeriod); - EV_COUNT("total_restarts_counter", totalRestartsCounter); -} - -void -StartMetrics::reset(unsigned long curTime) -{ - totalRestartsLastPeriod = 0; - lastLoggedTime = curTime; -} +StartMetrics::~StartMetrics() = default; void StartMetrics::maybeLog() @@ -55,11 +36,6 @@ StartMetrics::maybeLog() sentinel_totalRestarts.sample(totalRestartsCounter); sentinel_running.sample(currentlyRunningServices); sentinel_uptime.sample(curTime - startedTime); - if (curTime > lastLoggedTime + 59) { - output(); - reset(curTime); - } } -} // end namespace config::sentinel -} // end namespace config +} diff --git a/configd/src/apps/sentinel/metrics.h b/configd/src/apps/sentinel/metrics.h index 2378a055663..2263d70fb60 100644 --- a/configd/src/apps/sentinel/metrics.h +++ b/configd/src/apps/sentinel/metrics.h @@ -15,19 +15,15 @@ struct StartMetrics { vespalib::metrics::Producer producer; unsigned long currentlyRunningServices; unsigned long totalRestartsCounter; - unsigned long totalRestartsLastPeriod; long startedTime; - long lastLoggedTime; Counter sentinel_restarts; Gauge sentinel_totalRestarts; Gauge sentinel_running; Gauge sentinel_uptime; StartMetrics(); - ~StartMetrics() {} + ~StartMetrics(); - void output(); - void reset(unsigned long curTime); void maybeLog(); }; diff --git a/configd/src/apps/sentinel/service.cpp b/configd/src/apps/sentinel/service.cpp index e38328975dc..866171bf75f 100644 --- a/configd/src/apps/sentinel/service.cpp +++ b/configd/src/apps/sentinel/service.cpp @@ -336,7 +336,6 @@ Service::youExited(int status) LOG(debug, "%s: Has autorestart flag, restarting.", name().c_str()); setState(READY); _metrics.totalRestartsCounter++; - _metrics.totalRestartsLastPeriod++; _metrics.sentinel_restarts.add(); start(); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java index 2c46f591037..e9b4d6ac1aa 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java @@ -24,12 +24,14 @@ import java.io.File; import java.io.Reader; import java.io.StringReader; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.stream.Collectors; /** * Represents an application residing in zookeeper. @@ -200,16 +202,17 @@ public class ZKApplicationPackage implements ApplicationPackage { return ret; } - //Returns readers for all the children of a node. - //The node is looked up relative to the location of the active application package - //in zookeeper. + /** + * Returns readers for all the children of a node. + * The node is looked up relative to the location of the active application package in zookeeper. + */ @Override - public List<NamedReader> getFiles(Path relativePath,String suffix,boolean recurse) { + public List<NamedReader> getFiles(Path relativePath, String suffix, boolean recurse) { return liveApp.getAllDataFromDirectory(ConfigCurator.USERAPP_ZK_SUBPATH + '/' + relativePath.getRelative(), suffix, recurse); } @Override - public ApplicationFile getFile(Path file) { // foo/bar/baz.json + public ApplicationFile getFile(Path file) { return new ZKApplicationFile(file, liveApp); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKLiveApp.java b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKLiveApp.java index d7d43dea022..956af02e36f 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKLiveApp.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKLiveApp.java @@ -69,7 +69,8 @@ public class ZKLiveApp { log.finer("ZKApplicationPackage: Skipped '" + child + "' (did not match suffix " + fileNameSuffix + ")"); } if (recursive) - result.addAll(getAllDataFromDirectory(path + "/" + child, namePrefix + child + "/", fileNameSuffix, recursive)); + result.addAll(getAllDataFromDirectory(path + "/" + child, + namePrefix + child + "/", fileNameSuffix, recursive)); } if (log.isLoggable(Level.FINE)) log.fine("ZKApplicationPackage: Found '" + result.size() + "' files in " + fullPath); @@ -80,14 +81,15 @@ public class ZKLiveApp { } /** - * Retrieves a node relative to the node of the live application, e.g. /vespa/config/apps/$lt;app_id>/<path>/<node> + * Retrieves a node relative to the node of the live application, + * e.g. /vespa/config/apps/$lt;app_id>/<path>/<node> * * @param path a path relative to the currently active application * @param node a path relative to the path above * @return a Reader that can be used to get the data */ public Reader getDataReader(String path, String node) { - final String data = getData(path, node); + String data = getData(path, node); if (data == null) { throw new IllegalArgumentException("No node for " + getFullPath(path) + "/" + node + " exists"); } @@ -98,7 +100,8 @@ public class ZKLiveApp { try { return zk.getData(getFullPath(path), node); } catch (Exception e) { - throw new IllegalArgumentException("Could not retrieve node '" + getFullPath(path) + "/" + node + "' in zookeeper", e); + throw new IllegalArgumentException("Could not retrieve node '" + + getFullPath(path) + "/" + node + "' in zookeeper", e); } } @@ -205,5 +208,6 @@ public class ZKLiveApp { } return reader(data); } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/WeightedSetItem.java b/container-search/src/main/java/com/yahoo/prelude/query/WeightedSetItem.java index ecebcca6f95..131d4fcc9da 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/WeightedSetItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/WeightedSetItem.java @@ -44,6 +44,7 @@ public class WeightedSetItem extends SimpleTaggableItem { public Integer addToken(long value, int weight) { return addInternal(value, weight); } + /** * Add weighted token. * If token is already in the set, the maximum weight is kept. @@ -54,6 +55,7 @@ public class WeightedSetItem extends SimpleTaggableItem { if (token == null) throw new IllegalArgumentException("token must be a string"); return addInternal(token, weight); } + private Integer addInternal(Object token, int weight) { Integer newWeight = weight; Integer oldWeight = set.put(token, newWeight); @@ -172,4 +174,5 @@ public class WeightedSetItem extends SimpleTaggableItem { clone.set = this.set.clone(); return clone; } + } diff --git a/container-search/src/main/java/com/yahoo/search/querytransform/LowercasingSearcher.java b/container-search/src/main/java/com/yahoo/search/querytransform/LowercasingSearcher.java index 58486c98dbd..26c52a24e5c 100644 --- a/container-search/src/main/java/com/yahoo/search/querytransform/LowercasingSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/querytransform/LowercasingSearcher.java @@ -91,7 +91,7 @@ public abstract class LowercasingSearcher extends Searcher { } private void lowerCase(WeightedSetItem set, IndexFacts.Session indexFacts) { - if (!syntheticLowerCaseCheck(set.getIndexName(), indexFacts, true)) { + if ( ! syntheticLowerCaseCheck(set.getIndexName(), indexFacts, true)) { return; } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java index e51814e44f8..d166bb0d3fb 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java @@ -50,14 +50,6 @@ public class Node { this.wantedRebootGeneration = wantedRebootGeneration; } - // TODO: Remove once internal code supports new constructor - public Node(HostName hostname, State state, NodeType type, Optional<ApplicationId> owner, - Version currentVersion, Version wantedVersion, ServiceState serviceState, - long restartGeneration, long wantedRestartGeneration, long rebootGeneration, long wantedRebootGeneration) { - this(hostname, state, type, owner, currentVersion, wantedVersion, Version.emptyVersion, Version.emptyVersion, - serviceState, restartGeneration, wantedRestartGeneration, rebootGeneration, wantedRebootGeneration); - } - @TestOnly public Node(HostName hostname, State state, NodeType type, Optional<ApplicationId> owner, Version currentVersion, Version wantedVersion) { diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java index 92d07edaca8..b0acea188d0 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java @@ -31,8 +31,6 @@ public interface NodeRepository { void upgrade(ZoneId zone, NodeType type, Version version); /** Upgrade OS for all nodes of given type to a new version */ - default void upgradeOs(ZoneId zone, NodeType type, Version version) { - // TODO: Remove default implementation once implemented in internal code - } + void upgradeOs(ZoneId zone, NodeType type, Version version); } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/CloudName.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/CloudName.java new file mode 100644 index 00000000000..e7a6b32b36e --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/CloudName.java @@ -0,0 +1,62 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.api.integration.zone; + +import org.jetbrains.annotations.NotNull; + +import java.util.Objects; + +/** + * Represents a cloud provider used in a hosted Vespa system. + * + * @author mpolden + */ +public class CloudName implements Comparable<CloudName> { + + private final static CloudName defaultCloud = from("default"); + + private final String cloud; + + private CloudName(String cloud) { + this.cloud = cloud; + } + + public String value() { + return cloud; + } + + public boolean isDefault() { + return defaultName().equals(this); + } + + public static CloudName defaultName() { + return defaultCloud; + } + + public static CloudName from(String cloud) { + return new CloudName(cloud); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + CloudName cloudName = (CloudName) o; + return Objects.equals(cloud, cloudName.cloud); + } + + @Override + public int hashCode() { + return Objects.hash(cloud); + } + + @Override + public String toString() { + return cloud; + } + + @Override + public int compareTo(@NotNull CloudName o) { + return cloud.compareTo(o.cloud); + } + +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneFilterMock.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneFilterMock.java index 67d2fd14e6b..10cf862fa30 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneFilterMock.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneFilterMock.java @@ -9,20 +9,21 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.function.Predicate; import java.util.stream.Collectors; /** - * A Zones.List implementation which assumes all zones are controllerManaged. + * A ZoneList implementation which assumes all zones are controllerManaged. * * @author jonmv */ public class ZoneFilterMock implements ZoneList { - private final java.util.List<ZoneId> zones; + private final List<ZoneId> zones; private final boolean negate; - private ZoneFilterMock(java.util.List<ZoneId> zones, boolean negate) { + private ZoneFilterMock(List<ZoneId> zones, boolean negate) { this.negate = negate; this.zones = zones; } @@ -67,7 +68,7 @@ public class ZoneFilterMock implements ZoneList { } @Override - public java.util.List<ZoneId> ids() { + public List<ZoneId> ids() { return Collections.unmodifiableList(zones); } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneId.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneId.java index b53b81398c6..c98f6da3f29 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneId.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneId.java @@ -18,10 +18,16 @@ public class ZoneId { private final Environment environment; private final RegionName region; + private final CloudName cloud; - private ZoneId(Environment environment, RegionName region) { + private ZoneId(Environment environment, RegionName region, CloudName cloud) { this.environment = Objects.requireNonNull(environment); this.region = Objects.requireNonNull(region); + this.cloud = cloud; + } + + private ZoneId(Environment environment, RegionName region) { + this(environment, region, CloudName.defaultName()); } public static ZoneId from(Environment environment, RegionName region) { @@ -31,12 +37,21 @@ public class ZoneId { public static ZoneId from(String environment, String region) { return from(Environment.from(environment), RegionName.from(region)); } + /** Create from a serialised ZoneId. Inverse of {@code ZoneId.value()}. */ public static ZoneId from(String value) { String[] parts = value.split("\\."); return from(parts[0], parts[1]); } + public static ZoneId from(Environment environment, RegionName region, CloudName cloud) { + return new ZoneId(environment, region, cloud); + } + + public static ZoneId from(String environment, String region, String cloud) { + return new ZoneId(Environment.from(environment), RegionName.from(region), CloudName.from(cloud)); + } + public Environment environment() { return environment; } @@ -45,6 +60,10 @@ public class ZoneId { return region; } + public CloudName cloud() { + return cloud; + } + /** Returns the serialised value of this. Inverse of {@code ZoneId.from(String value)}. */ public String value() { return environment + "." + region; @@ -52,21 +71,22 @@ public class ZoneId { @Override public String toString() { - return "zone " + value(); + return "zone " + value() + " in " + cloud; } @Override public boolean equals(Object o) { if (this == o) return true; - if ( ! (o instanceof ZoneId)) return false; - ZoneId id = (ZoneId) o; - return environment == id.environment && - Objects.equals(region, id.region); + if (o == null || getClass() != o.getClass()) return false; + ZoneId zoneId = (ZoneId) o; + return environment == zoneId.environment && + Objects.equals(region, zoneId.region) && + Objects.equals(cloud, zoneId.cloud); } @Override public int hashCode() { - return Objects.hash(environment, region); + return Objects.hash(environment, region, cloud); } } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneRegistry.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneRegistry.java index f721564c80e..4fbad88df8d 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneRegistry.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneRegistry.java @@ -53,9 +53,14 @@ public interface ZoneRegistry { UpgradePolicy upgradePolicy(); /** Returns the OS upgrade policy to use for zones in this registry */ - // TODO: Remove default once internal code implements this + // TODO: Remove default UpgradePolicy osUpgradePolicy() { return upgradePolicy(); } + /** Returns the OS upgrade policy to use for zones belonging to given cloud, in this registry */ + default UpgradePolicy osUpgradePolicy(CloudName cloud) { + return osUpgradePolicy(); // TODO: Remove default implementation + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index 9711c1d8533..e87db02c9bd 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -524,6 +524,8 @@ public class InternalStepRunner implements StepRunner { String timestamp = timestampFormat.format(new Date(record.getMillis())); for (String line : record.getMessage().split("\n")) out.println(timestamp + ": " + line); + if (record.getThrown() != null) + record.getThrown().printStackTrace(out); record.setSourceClassName(null); // Makes the root logger's ConsoleHandler use the logger name instead, when printing. getParent().log(record); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java index 671bab9aae6..1ec64e1b3d6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java @@ -9,6 +9,7 @@ import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.JobStatus; +import java.util.Objects; import java.util.Optional; import static java.util.Objects.requireNonNull; @@ -71,6 +72,22 @@ public class Versions { } @Override + public boolean equals(Object o) { + if (this == o) return true; + if ( ! (o instanceof Versions)) return false; + Versions versions = (Versions) o; + return Objects.equals(targetPlatform, versions.targetPlatform) && + Objects.equals(targetApplication, versions.targetApplication) && + Objects.equals(sourcePlatform, versions.sourcePlatform) && + Objects.equals(sourceApplication, versions.sourceApplication); + } + + @Override + public int hashCode() { + return Objects.hash(targetPlatform, targetApplication, sourcePlatform, sourceApplication); + } + + @Override public String toString() { return String.format("platform %s%s, application %s%s", sourcePlatform.filter(source -> !source.equals(targetPlatform)) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java index a71e6a393ac..47a690459a8 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java @@ -111,10 +111,10 @@ class RunSerializer { ? Optional.of(Version.fromString(versionsObject.field(sourceField).field(platformVersionField).asString())) : Optional.empty(); Optional<ApplicationVersion> sourceApplicationVersion = versionsObject.field(sourceField).valid() - ? Optional.of(ApplicationVersion.from(new SourceRevision(versionsObject.field(repositoryField).asString(), - versionsObject.field(branchField).asString(), - versionsObject.field(commitField).asString()), - versionsObject.field(buildField).asLong())) + ? Optional.of(ApplicationVersion.from(new SourceRevision(versionsObject.field(sourceField).field(repositoryField).asString(), + versionsObject.field(sourceField).field(branchField).asString(), + versionsObject.field(sourceField).field(commitField).asString()), + versionsObject.field(sourceField).field(buildField).asLong())) : Optional.empty(); return new Versions(targetPlatformVersion, targetApplicationVersion, sourcePlatformVersion, sourceApplicationVersion); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java index 10537f42c0b..a1fa4c1cd29 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java @@ -1,10 +1,13 @@ package com.yahoo.vespa.hosted.controller.persistence; import com.google.common.collect.ImmutableMap; +import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; +import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.application.SourceRevision; import com.yahoo.vespa.hosted.controller.deployment.Run; import com.yahoo.vespa.hosted.controller.deployment.RunStatus; import com.yahoo.vespa.hosted.controller.deployment.Step; @@ -67,6 +70,18 @@ public class RunSerializerTest { assertFalse(run.hasEnded()); assertEquals(running, run.status()); assertEquals(3, run.lastTestRecord()); + assertEquals(new Version(1, 2, 3), run.versions().targetPlatform()); + assertEquals(ApplicationVersion.from(new SourceRevision("git@github.com:user/repo.git", + "master", + "f00bad"), + 123), + run.versions().targetApplication()); + assertEquals(new Version(1, 2, 2), run.versions().sourcePlatform().get()); + assertEquals(ApplicationVersion.from(new SourceRevision("git@github.com:user/repo.git", + "master", + "badb17"), + 122), + run.versions().sourceApplication().get()); assertEquals(ImmutableMap.<Step, Step.Status>builder() .put(deployInitialReal, unfinished) .put(installInitialReal, failed) @@ -91,6 +106,8 @@ public class RunSerializerTest { assertEquals(run.start(), phoenix.start()); assertEquals(run.end(), phoenix.end()); assertEquals(run.status(), phoenix.status()); + assertEquals(run.lastTestRecord(), phoenix.lastTestRecord()); + assertEquals(run.versions(), phoenix.versions()); assertEquals(run.steps(), phoenix.steps()); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/run-status.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/run-status.json index 0e0149897fc..ceeef722e90 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/run-status.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/run-status.json @@ -26,7 +26,7 @@ "commit": "f00bad", "build": 123, "source": { - "platform": "1.2.3", + "platform": "1.2.2", "repository": "git@github.com:user/repo.git", "branch": "master", "commit": "badb17", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java index f0cb7ca645d..9e8dfad82c1 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java @@ -161,7 +161,7 @@ public class JobControllerApiHandlerHelperTest { private void assertFile(HttpResponse response, String resourceName) { try { - Path path = Paths.get(getClass().getClassLoader().getResource(resourceName).getPath()); + Path path = Paths.get("src/test/resources/").resolve(resourceName); String expected = new String(Files.readAllBytes(path)); compare(response, expected); } catch (Exception e) { diff --git a/parent/pom.xml b/parent/pom.xml index 3badece1f86..c7e0f490905 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -150,6 +150,8 @@ <artifactId>maven-javadoc-plugin</artifactId> <configuration> <doclint>${doclint},-missing</doclint> + <quiet>true</quiet> + <show>protected</show> </configuration> <version>3.0.1</version> </plugin> @@ -270,6 +272,7 @@ <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-javadoc-plugin</artifactId> <configuration> + <quiet>true</quiet> <!-- Avoid javadoc warning ".. not specified the version of HTML .." --> <additionalJOption>-html4</additionalJOption> </configuration> @@ -327,7 +330,8 @@ <doclint>${doclint},-missing</doclint> <failOnError>${javadoc.failOnError}</failOnError> <quiet>true</quiet> - <show>private</show> + <show>protected</show> + <header><a href="https://docs.vespa.ai"><img src="https://docs.vespa.ai/img/vespa-logo.png" width="100" height="28" style="padding-top:7px"/></a></header> </configuration> </plugin> </plugins> diff --git a/processing/src/main/java/com/yahoo/processing/execution/AsyncExecution.java b/processing/src/main/java/com/yahoo/processing/execution/AsyncExecution.java index 75b5e4d8a22..eac96e9b408 100644 --- a/processing/src/main/java/com/yahoo/processing/execution/AsyncExecution.java +++ b/processing/src/main/java/com/yahoo/processing/execution/AsyncExecution.java @@ -15,7 +15,7 @@ import java.util.List; import java.util.concurrent.*; /** - * Provides asynchronous execution of processing chains. Usage: + * <p>Provides asynchronous execution of processing chains. Usage:</p> * * <pre> * Execution execution = new Execution(chain); diff --git a/searchcommon/src/vespa/searchcommon/attribute/basictype.h b/searchcommon/src/vespa/searchcommon/attribute/basictype.h index 3a2a319afd8..5bc51cdbea8 100644 --- a/searchcommon/src/vespa/searchcommon/attribute/basictype.h +++ b/searchcommon/src/vespa/searchcommon/attribute/basictype.h @@ -27,13 +27,10 @@ class BasicType MAX_TYPE }; - explicit - BasicType(int t) : _type(Type(t)) { } - explicit - BasicType(unsigned int t) : _type(Type(t)) { } + explicit BasicType(int t) : _type(Type(t)) { } + explicit BasicType(unsigned int t) : _type(Type(t)) { } BasicType(Type t) : _type(t) { } - explicit - BasicType(const vespalib::string & t) : _type(asType(t)) { } + explicit BasicType(const vespalib::string & t) : _type(asType(t)) { } Type type() const { return _type; } const char * asString() const { return asString(_type); } diff --git a/searchcore/src/apps/fdispatch/fdispatch.cpp b/searchcore/src/apps/fdispatch/fdispatch.cpp index 97cfdcb408c..cd3ef3b7550 100644 --- a/searchcore/src/apps/fdispatch/fdispatch.cpp +++ b/searchcore/src/apps/fdispatch/fdispatch.cpp @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <vespa/searchcore/fdispatch/program/fdispatch.h> -#include <vespa/searchcore/fdispatch/common/perftask.h> #include <vespa/vespalib/net/state_server.h> #include <vespa/vespalib/net/simple_health_producer.h> #include <vespa/vespalib/net/simple_metrics_producer.h> @@ -96,7 +95,6 @@ FastS_FDispatchApp::Main() vespalib::SimpleHealthProducer health; vespalib::SimpleMetricsProducer metrics; vespalib::StateServer stateServer(myfdispatch->getHealthPort(), health, metrics, myfdispatch->getComponentConfig()); - FastS_PerfTask perfTask(*myfdispatch, 300.0); while (!CheckShutdownFlags()) { if (myfdispatch->Failed()) { throw std::runtime_error("myfdispatch->Failed()"); diff --git a/searchcore/src/vespa/searchcore/fdispatch/common/CMakeLists.txt b/searchcore/src/vespa/searchcore/fdispatch/common/CMakeLists.txt index 7a91efcd2ec..45261162e93 100644 --- a/searchcore/src/vespa/searchcore/fdispatch/common/CMakeLists.txt +++ b/searchcore/src/vespa/searchcore/fdispatch/common/CMakeLists.txt @@ -2,8 +2,6 @@ vespa_add_library(searchcore_fdcommon STATIC SOURCES appcontext.cpp - perftask.cpp - queryperf.cpp rpc.cpp search.cpp timestat.cpp diff --git a/searchcore/src/vespa/searchcore/fdispatch/common/appcontext.cpp b/searchcore/src/vespa/searchcore/fdispatch/common/appcontext.cpp index 248babad316..78efc1f7429 100644 --- a/searchcore/src/vespa/searchcore/fdispatch/common/appcontext.cpp +++ b/searchcore/src/vespa/searchcore/fdispatch/common/appcontext.cpp @@ -28,53 +28,38 @@ FastS_AppContext::FastS_AppContext() _createTime = _timeKeeper.GetTime(); } - -FastS_AppContext::~FastS_AppContext() -{ -} - +FastS_AppContext::~FastS_AppContext() = default; FNET_Transport * FastS_AppContext::GetFNETTransport() { - return NULL; + return nullptr; } - FNET_Scheduler * FastS_AppContext::GetFNETScheduler() { - return NULL; + return nullptr; } - FastS_NodeManager * FastS_AppContext::GetNodeManager() { - return NULL; + return nullptr; } - FastS_DataSetCollection * FastS_AppContext::GetDataSetCollection() { - return NULL; + return nullptr; } - FastOS_ThreadPool * FastS_AppContext::GetThreadPool() { - return NULL; -} - - -void -FastS_AppContext::logPerformance() -{ + return nullptr; } - uint32_t FastS_AppContext::getDispatchLevel() { diff --git a/searchcore/src/vespa/searchcore/fdispatch/common/appcontext.h b/searchcore/src/vespa/searchcore/fdispatch/common/appcontext.h index 7a5652577a0..fce468b4532 100644 --- a/searchcore/src/vespa/searchcore/fdispatch/common/appcontext.h +++ b/searchcore/src/vespa/searchcore/fdispatch/common/appcontext.h @@ -41,7 +41,6 @@ public: virtual FNET_Scheduler *GetFNETScheduler(); virtual FastS_DataSetCollection *GetDataSetCollection(); virtual FastOS_ThreadPool *GetThreadPool(); - virtual void logPerformance(); virtual uint32_t getDispatchLevel(); private: }; diff --git a/searchcore/src/vespa/searchcore/fdispatch/common/perftask.cpp b/searchcore/src/vespa/searchcore/fdispatch/common/perftask.cpp deleted file mode 100644 index 17113285fab..00000000000 --- a/searchcore/src/vespa/searchcore/fdispatch/common/perftask.cpp +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "perftask.h" -#include "appcontext.h" - -#include <vespa/log/log.h> -LOG_SETUP(".perftask"); - -FastS_PerfTask::FastS_PerfTask(FastS_AppContext &ctx, double delay) - : FNET_Task(ctx.GetFNETScheduler()), - _ctx(ctx), - _delay(delay), - _valid(ctx.GetFNETScheduler() != NULL) -{ - if (_valid) { - ScheduleNow(); - } else { - LOG(warning, "Performance monitoring disabled; " - "no scheduler found in application context"); - } -} - - -FastS_PerfTask::~FastS_PerfTask() -{ - if (_valid) { - Kill(); - } -} - - -void -FastS_PerfTask::PerformTask() -{ - Schedule(_delay); - _ctx.logPerformance(); -} diff --git a/searchcore/src/vespa/searchcore/fdispatch/common/perftask.h b/searchcore/src/vespa/searchcore/fdispatch/common/perftask.h deleted file mode 100644 index 09c3b9840ab..00000000000 --- a/searchcore/src/vespa/searchcore/fdispatch/common/perftask.h +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -#include <vespa/fnet/task.h> - -class FastS_AppContext; - -class FastS_PerfTask : public FNET_Task -{ -private: - FastS_AppContext &_ctx; - double _delay; - bool _valid; - - FastS_PerfTask(const FastS_PerfTask &); - FastS_PerfTask &operator=(const FastS_PerfTask &); - -public: - FastS_PerfTask(FastS_AppContext &ctx, double delay); - ~FastS_PerfTask(); - void PerformTask() override; - bool isValid() const { return _valid; } -}; - diff --git a/searchcore/src/vespa/searchcore/fdispatch/common/queryperf.cpp b/searchcore/src/vespa/searchcore/fdispatch/common/queryperf.cpp deleted file mode 100644 index a0cc89d15c6..00000000000 --- a/searchcore/src/vespa/searchcore/fdispatch/common/queryperf.cpp +++ /dev/null @@ -1,84 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "queryperf.h" - -#include <vespa/log/log.h> -LOG_SETUP(".queryperf"); - -namespace { - -struct MyLogTask : vespalib::Executor::Task { - uint32_t queueLen; - uint32_t activeCnt; - uint32_t queryCnt; - uint32_t dropCnt; - uint32_t timeoutCnt; - double avgQueryTime; - MyLogTask(uint32_t queueLen_in, - uint32_t activeCnt_in, - uint32_t queryCnt_in, - uint32_t dropCnt_in, - uint32_t timeoutCnt_in, - double avgQueryTime_in) - : queueLen(queueLen_in), - activeCnt(activeCnt_in), - queryCnt(queryCnt_in), - dropCnt(dropCnt_in), - timeoutCnt(timeoutCnt_in), - avgQueryTime(avgQueryTime_in) - { - } - void run() override { - EV_VALUE("queued_queries", queueLen); - EV_VALUE("active_queries", activeCnt); - EV_COUNT("queries", queryCnt); - EV_COUNT("dropped_queries", dropCnt); - EV_COUNT("timedout_queries", timeoutCnt); - if (avgQueryTime > 0.0) { - EV_VALUE("query_eval_time_avg_s", avgQueryTime); - } - } -}; - -} // namespace <unnamed> - -FastS_QueryPerf::FastS_QueryPerf() - : queueLen(0), - activeCnt(0), - queryCnt(0), - queryTime(0), - dropCnt(0), - timeoutCnt(0), - _lastQueryCnt(0), - _lastQueryTime(0.0) -{ -} - -void -FastS_QueryPerf::reset() -{ - queueLen = 0; - activeCnt = 0; - queryCnt = 0; - queryTime = 0; - dropCnt = 0; - timeoutCnt = 0; -} - -vespalib::Executor::Task::UP -FastS_QueryPerf::make_log_task() -{ - double avgQueryTime = 0.0; - if (queryCnt > _lastQueryCnt) { - avgQueryTime = (queryTime - _lastQueryTime) - / ((double)(queryCnt - _lastQueryCnt)); - } - _lastQueryCnt = queryCnt; - _lastQueryTime = queryTime; - return std::make_unique<MyLogTask>(queueLen, - activeCnt, - queryCnt, - dropCnt, - timeoutCnt, - avgQueryTime); -} diff --git a/searchcore/src/vespa/searchcore/fdispatch/common/queryperf.h b/searchcore/src/vespa/searchcore/fdispatch/common/queryperf.h deleted file mode 100644 index ee31a8e58b2..00000000000 --- a/searchcore/src/vespa/searchcore/fdispatch/common/queryperf.h +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -#include <cstdint> -#include <vespa/vespalib/util/executor.h> - -struct FastS_QueryPerf -{ - uint32_t queueLen; - uint32_t activeCnt; - uint32_t queryCnt; - double queryTime; - uint32_t dropCnt; - uint32_t timeoutCnt; - - FastS_QueryPerf(); - - /** - * reset all values except the cached 'old' values. This will - * prepare the object for reuse logging wise. - **/ - void reset(); - vespalib::Executor::Task::UP make_log_task(); - -private: - uint32_t _lastQueryCnt; - double _lastQueryTime; -}; - diff --git a/searchcore/src/vespa/searchcore/fdispatch/program/fdispatch.cpp b/searchcore/src/vespa/searchcore/fdispatch/program/fdispatch.cpp index b68566c3c9b..93680a95d44 100644 --- a/searchcore/src/vespa/searchcore/fdispatch/program/fdispatch.cpp +++ b/searchcore/src/vespa/searchcore/fdispatch/program/fdispatch.cpp @@ -389,13 +389,6 @@ Fdispatch::Init() return true; } - -void -Fdispatch::logPerformance() -{ - _nodeManager->logPerformance(_executor); -} - uint32_t Fdispatch::getDispatchLevel() { diff --git a/searchcore/src/vespa/searchcore/fdispatch/program/fdispatch.h b/searchcore/src/vespa/searchcore/fdispatch/program/fdispatch.h index 6cfb4bfb5a1..093308d68d2 100644 --- a/searchcore/src/vespa/searchcore/fdispatch/program/fdispatch.h +++ b/searchcore/src/vespa/searchcore/fdispatch/program/fdispatch.h @@ -96,7 +96,6 @@ public: virtual FastS_NodeManager *GetNodeManager() override; virtual FastS_DataSetCollection *GetDataSetCollection() override; virtual FastOS_ThreadPool *GetThreadPool() override; - virtual void logPerformance() override; virtual uint32_t getDispatchLevel() override; bool CheckTempFail(); bool Failed(); diff --git a/searchcore/src/vespa/searchcore/fdispatch/search/dataset_base.cpp b/searchcore/src/vespa/searchcore/fdispatch/search/dataset_base.cpp index 1b96a48d35b..519960bfad0 100644 --- a/searchcore/src/vespa/searchcore/fdispatch/search/dataset_base.cpp +++ b/searchcore/src/vespa/searchcore/fdispatch/search/dataset_base.cpp @@ -248,14 +248,12 @@ FastS_DataSetBase::AbortQueryQueue_HasLock() } } - void FastS_DataSetBase::AddCost() { _totalrefcost += _unitrefcost; } - void FastS_DataSetBase::SubCost() { @@ -263,10 +261,8 @@ FastS_DataSetBase::SubCost() _totalrefcost -= _unitrefcost; } - void -FastS_DataSetBase::UpdateSearchTime(double tnow, - double elapsed, bool timedout) +FastS_DataSetBase::UpdateSearchTime(double tnow, double elapsed, bool timedout) { int slot; auto dsGuard(getDsGuard()); @@ -279,36 +275,18 @@ FastS_DataSetBase::UpdateSearchTime(double tnow, _total._normalTimeStat.Update(tnow, elapsed, timedout); } - void FastS_DataSetBase::UpdateEstimateCount() { ++_total._estimates; } - void FastS_DataSetBase::CountTimeout() { ++_total._nTimedOut; } - -void -FastS_DataSetBase::addPerformance(FastS_QueryPerf &qp) -{ - FastS_TimeStatTotals totals; - auto dsGuard(getDsGuard()); - _total._normalTimeStat.AddTotal(&totals); - qp.queueLen += _queryQueue.GetQueueLen(); - qp.activeCnt += _queryQueue.GetActiveQueries(); - qp.queryCnt += totals._totalCount; - qp.queryTime += totals._totalAccTime; - qp.dropCnt += _total._nOverload; - qp.timeoutCnt += _total._nTimedOut; -} - - ChildInfo FastS_DataSetBase::getChildInfo() const { diff --git a/searchcore/src/vespa/searchcore/fdispatch/search/dataset_base.h b/searchcore/src/vespa/searchcore/fdispatch/search/dataset_base.h index 042e18bf96f..f4f69285e89 100644 --- a/searchcore/src/vespa/searchcore/fdispatch/search/dataset_base.h +++ b/searchcore/src/vespa/searchcore/fdispatch/search/dataset_base.h @@ -20,7 +20,6 @@ class FastS_QueryResult; class FastS_PlainDataSet; class FastS_FNET_DataSet; class FastS_AppContext; -class FastS_QueryPerf; class FNET_Task; //--------------------------------------------------------------------------- @@ -220,13 +219,9 @@ public: FastS_TimeKeeper *timeKeeper, bool async) = 0; virtual void Free() = 0; - virtual void addPerformance(FastS_QueryPerf &qp); // typesafe down-cast //------------------- virtual FastS_PlainDataSet *GetPlainDataSet() { return nullptr; } virtual FastS_FNET_DataSet *GetFNETDataSet() { return nullptr; } }; - -//--------------------------------------------------------------------------- - diff --git a/searchcore/src/vespa/searchcore/fdispatch/search/nodemanager.cpp b/searchcore/src/vespa/searchcore/fdispatch/search/nodemanager.cpp index 879009c384f..f2af03d4c52 100644 --- a/searchcore/src/vespa/searchcore/fdispatch/search/nodemanager.cpp +++ b/searchcore/src/vespa/searchcore/fdispatch/search/nodemanager.cpp @@ -88,7 +88,6 @@ FastS_NodeManager::FastS_NodeManager(vespalib::SimpleComponentConfigProducer &co _mldDocStamp(0), _mldDocStampMin(0), _gencnt(0), - _queryPerf(), _fetcher(), _configUri(config::ConfigUri::createEmpty()), _lastPartMap(NULL), @@ -389,24 +388,6 @@ FastS_NodeManager::getChildInfo() return r; } - -void -FastS_NodeManager::logPerformance(vespalib::Executor &executor) -{ - _queryPerf.reset(); - FastS_DataSetCollection *dsc = GetDataSetCollection(); - - for (unsigned int i = 0; i < dsc->GetMaxNumDataSets(); i++) { - if (dsc->PeekDataSet(i) != NULL) { - dsc->PeekDataSet(i)->addPerformance(_queryPerf); - } - } - - dsc->subRef(); - executor.execute(_queryPerf.make_log_task()); -} - - void FastS_NodeManager::CheckEvents(FastS_TimeKeeper *timeKeeper) { diff --git a/searchcore/src/vespa/searchcore/fdispatch/search/nodemanager.h b/searchcore/src/vespa/searchcore/fdispatch/search/nodemanager.h index 77d4482fba7..70dc80914f4 100644 --- a/searchcore/src/vespa/searchcore/fdispatch/search/nodemanager.h +++ b/searchcore/src/vespa/searchcore/fdispatch/search/nodemanager.h @@ -5,7 +5,6 @@ #include "child_info.h" #include "configdesc.h" #include <vespa/config/helper/configfetcher.h> -#include <vespa/searchcore/fdispatch/common/queryperf.h> #include <vespa/vespalib/net/simple_component_config_producer.h> #include <vespa/config/subscription/configuri.h> #include <vespa/vespalib/util/executor.h> @@ -34,7 +33,6 @@ private: uint32_t _mldDocStampMin; // Bumped for global cache flush uint32_t _gencnt; - FastS_QueryPerf _queryPerf; std::unique_ptr<config::ConfigFetcher> _fetcher; @@ -89,12 +87,6 @@ public: ChildInfo getChildInfo(); void ShutdownConfig(); - /** - * log query performance. This method should only be invoked from - * the FNET thread. - **/ - void logPerformance(vespalib::Executor &executor); - void CheckEvents(FastS_TimeKeeper *timeKeeper); // invoked by FNET thread }; diff --git a/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.cpp b/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.cpp index d8fc9557023..9be1156b154 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.cpp @@ -8,8 +8,7 @@ using namespace search::fef; -namespace proton { -namespace matching { +namespace proton::matching { void IndexEnvironment::extractFields(const search::index::Schema &schema) @@ -17,29 +16,20 @@ IndexEnvironment::extractFields(const search::index::Schema &schema) typedef search::index::Schema::Field SchemaField; for (uint32_t i = 0; i < schema.getNumAttributeFields(); ++i) { const SchemaField &field = schema.getAttributeField(i); - FieldInfo fieldInfo(FieldType::ATTRIBUTE, - field.getCollectionType(), - field.getName(), _fields.size()); + FieldInfo fieldInfo(FieldType::ATTRIBUTE, field.getCollectionType(), field.getName(), _fields.size()); fieldInfo.set_data_type(field.getDataType()); insertField(fieldInfo); } for (uint32_t i = 0; i < schema.getNumIndexFields(); ++i) { const SchemaField &field = schema.getIndexField(i); - FieldInfo fieldInfo(FieldType::INDEX, - field.getCollectionType(), - field.getName(), _fields.size()); + FieldInfo fieldInfo(FieldType::INDEX, field.getCollectionType(), field.getName(), _fields.size()); fieldInfo.set_data_type(field.getDataType()); - if (indexproperties::IsFilterField::check( - _properties, field.getName())) - { + if (indexproperties::IsFilterField::check(_properties, field.getName())) { fieldInfo.setFilter(true); } FieldNameMap::const_iterator itr = _fieldNames.find(field.getName()); if (itr != _fieldNames.end()) { // override the attribute field - FieldInfo shadow_field(fieldInfo.type(), - fieldInfo.collection(), - fieldInfo.name(), - itr->second); + FieldInfo shadow_field(fieldInfo.type(), fieldInfo.collection(), fieldInfo.name(), itr->second); shadow_field.set_data_type(fieldInfo.get_data_type()); shadow_field.addAttribute(); // tell ranking about the shadowed attribute _fields[itr->second] = shadow_field; @@ -48,19 +38,15 @@ IndexEnvironment::extractFields(const search::index::Schema &schema) } } for (const auto &attr : schema.getImportedAttributeFields()) { - FieldInfo field(FieldType::ATTRIBUTE, - attr.getCollectionType(), - attr.getName(), _fields.size()); + FieldInfo field(FieldType::ATTRIBUTE, attr.getCollectionType(), attr.getName(), _fields.size()); field.set_data_type(attr.getDataType()); insertField(field); } //TODO: This is a kludge to get [documentmetastore] searchable { - FieldInfo fieldInfo(FieldType::HIDDEN_ATTRIBUTE, - FieldInfo::CollectionType::SINGLE, - DocumentMetaStore::getFixedName(), - _fields.size()); + FieldInfo fieldInfo(FieldType::HIDDEN_ATTRIBUTE, FieldInfo::CollectionType::SINGLE, + DocumentMetaStore::getFixedName(), _fields.size()); fieldInfo.set_data_type(FieldInfo::DataType::RAW); fieldInfo.setFilter(true); insertField(fieldInfo); @@ -85,9 +71,7 @@ IndexEnvironment::IndexEnvironment(const search::index::Schema &schema, _motivation(UNKNOWN), _constantValueRepo(constantValueRepo) { - _tableManager.addFactory( - search::fef::ITableFactory::SP( - new search::fef::FunctionTableFactory(256))); + _tableManager.addFactory(std::make_shared<search::fef::FunctionTableFactory>(256)); extractFields(schema); } @@ -124,38 +108,26 @@ IndexEnvironment::getFieldByName(const string &name) const } const search::fef::ITableManager & -IndexEnvironment::getTableManager() const -{ +IndexEnvironment::getTableManager() const { return _tableManager; } IIndexEnvironment::FeatureMotivation -IndexEnvironment::getFeatureMotivation() const -{ +IndexEnvironment::getFeatureMotivation() const { return _motivation; } void -IndexEnvironment::hintFeatureMotivation(FeatureMotivation motivation) const -{ +IndexEnvironment::hintFeatureMotivation(FeatureMotivation motivation) const { _motivation = motivation; } void -IndexEnvironment::hintFieldAccess(uint32_t fieldId) const -{ - (void) fieldId; -} +IndexEnvironment::hintFieldAccess(uint32_t ) const { } void -IndexEnvironment::hintAttributeAccess(const string &name) const -{ - (void) name; -} +IndexEnvironment::hintAttributeAccess(const string &) const { } -IndexEnvironment::~IndexEnvironment() -{ -} +IndexEnvironment::~IndexEnvironment() = default; -} // namespace matching -} // namespace proton +} diff --git a/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.h b/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.h index 09ff1fef2ea..a17ef1677a5 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.h +++ b/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.h @@ -10,8 +10,7 @@ #include <vespa/searchcommon/common/schema.h> #include <vespa/eval/eval/value_cache/constant_value.h> -namespace proton { -namespace matching { +namespace proton::matching { /** * Index environment implementation for the proton matching pipeline. @@ -47,40 +46,21 @@ public: const search::fef::Properties &props, const IConstantValueRepo &constantValueRepo); - // inherited from search::fef::IIndexEnvironment - virtual const search::fef::Properties &getProperties() const override; - - // inherited from search::fef::IIndexEnvironment - virtual uint32_t getNumFields() const override; - - // inherited from search::fef::IIndexEnvironment - virtual const search::fef::FieldInfo *getField(uint32_t id) const override; - - // inherited from search::fef::IIndexEnvironment - virtual const search::fef::FieldInfo * - getFieldByName(const string &name) const override; - - // inherited from search::fef::IIndexEnvironment - virtual const search::fef::ITableManager &getTableManager() const override; - - virtual FeatureMotivation getFeatureMotivation() const override; - - // inherited from search::fef::IIndexEnvironment - virtual void hintFeatureMotivation(FeatureMotivation motivation) const override; - - // inherited from search::fef::IIndexEnvironment - virtual void hintFieldAccess(uint32_t fieldId) const override; - - // inherited from search::fef::IIndexEnvironment - virtual void hintAttributeAccess(const string &name) const override; - - virtual vespalib::eval::ConstantValue::UP getConstantValue(const vespalib::string &name) const override { + const search::fef::Properties &getProperties() const override; + uint32_t getNumFields() const override; + const search::fef::FieldInfo *getField(uint32_t id) const override; + const search::fef::FieldInfo *getFieldByName(const string &name) const override; + const search::fef::ITableManager &getTableManager() const override; + FeatureMotivation getFeatureMotivation() const override; + void hintFeatureMotivation(FeatureMotivation motivation) const override; + void hintFieldAccess(uint32_t fieldId) const override; + void hintAttributeAccess(const string &name) const override; + + vespalib::eval::ConstantValue::UP getConstantValue(const vespalib::string &name) const override { return _constantValueRepo.getConstant(name); } - virtual ~IndexEnvironment(); + ~IndexEnvironment() override; }; -} // namespace matching -} // namespace proton - +} diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp index ec3f1c7223f..6bce05b6c26 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp @@ -69,10 +69,9 @@ MatchMaster::match(const MatchParams ¶ms, std::vector<MatchThread::UP> threadState; std::vector<vespalib::Runnable*> targets; for (size_t i = 0; i < threadBundle.size(); ++i) { - IMatchLoopCommunicator &com = - (i == 0)? - static_cast<IMatchLoopCommunicator&>(timedCommunicator) : - static_cast<IMatchLoopCommunicator&>(communicator); + IMatchLoopCommunicator &com = (i == 0) + ? static_cast<IMatchLoopCommunicator&>(timedCommunicator) + : static_cast<IMatchLoopCommunicator&>(communicator); threadState.emplace_back(std::make_unique<MatchThread>(i, threadBundle.size(), params, matchToolsFactory, com, *scheduler, resultProcessor, mergeDirector, distributionKey)); @@ -101,10 +100,10 @@ MatchMaster::match(const MatchParams ¶ms, } FeatureSet::SP -MatchMaster::getFeatureSet(const MatchToolsFactory &matchToolsFactory, +MatchMaster::getFeatureSet(const MatchToolsFactory &mtf, const std::vector<uint32_t> &docs, bool summaryFeatures) { - MatchTools::UP matchTools = matchToolsFactory.createMatchTools(); + MatchTools::UP matchTools = mtf.createMatchTools(); if (summaryFeatures) { matchTools->setup_summary(); } else { @@ -118,7 +117,7 @@ MatchMaster::getFeatureSet(const MatchToolsFactory &matchToolsFactory, for (size_t i = 0; i < resolver.num_features(); ++i) { featureNames.emplace_back(resolver.name_of(i)); } - FeatureSet::SP retval(new FeatureSet(featureNames, docs.size())); + auto retval = std::make_shared<FeatureSet>(featureNames, docs.size()); if (docs.empty()) { return retval; } diff --git a/searchcore/src/vespa/searchcore/proton/server/searchview.cpp b/searchcore/src/vespa/searchcore/proton/server/searchview.cpp index 1e22eec2d55..232f0bdba6c 100644 --- a/searchcore/src/vespa/searchcore/proton/server/searchview.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/searchview.cpp @@ -96,7 +96,7 @@ convertLidsToGids(DocsumReply &reply, const DocsumRequest &request) DocsumReply::UP createEmptyReply(const DocsumRequest & request) { - DocsumReply::UP reply(new DocsumReply()); + auto reply = std::make_unique<DocsumReply>(); for (size_t i = 0; i < request.hits.size(); ++i) { reply->docsums.push_back(DocsumReply::Docsum()); reply->docsums.back().gid = request.hits[i].gid; @@ -113,7 +113,7 @@ SearchView::SearchView(const ISummaryManager::ISummarySetup::SP & summarySetup, _matchView(matchView) { } -SearchView::~SearchView() {} +SearchView::~SearchView() = default; DocsumReply::UP SearchView::getDocsums(const DocsumRequest & req) diff --git a/searchlib/src/vespa/searchlib/queryeval/hitcollector.h b/searchlib/src/vespa/searchlib/queryeval/hitcollector.h index 4429d28d1a4..97beaa0fd55 100644 --- a/searchlib/src/vespa/searchlib/queryeval/hitcollector.h +++ b/searchlib/src/vespa/searchlib/queryeval/hitcollector.h @@ -141,6 +141,9 @@ private: VESPA_DLL_LOCAL void sortHitsByDocId(); public: + HitCollector(const HitCollector &) = delete; + HitCollector &operator=(const HitCollector &) = delete; + /** * Creates a hit collector used to store hits for doc ids in the * range [0, numDocs>. Doc id and rank score are stored for the n @@ -174,6 +177,8 @@ public: */ SortedHitSequence getSortedHitSequence(size_t max_hits); + const std::vector<Hit> & getReRankedHits() const { return _reRankedHits; } + /** * Re-ranks the given hits by invoking the score() method on the * given document scorer. The hits are sorted on doc id so that @@ -193,10 +198,6 @@ public: * @param default_value rank value to be used for results without rank value **/ std::unique_ptr<ResultSet> getResultSet(HitRank default_value = default_rank_value); - -private: - HitCollector(const HitCollector &); // Not implemented - HitCollector &operator=(const HitCollector &); // Not implemented }; } diff --git a/searchlib/src/vespa/searchlib/transactionlog/translogserver.cpp b/searchlib/src/vespa/searchlib/transactionlog/translogserver.cpp index f9a82c8d20f..65bb682a389 100644 --- a/searchlib/src/vespa/searchlib/transactionlog/translogserver.cpp +++ b/searchlib/src/vespa/searchlib/transactionlog/translogserver.cpp @@ -165,7 +165,6 @@ void TransLogServer::run() { FRT_RPCRequest *req(NULL); bool hasPacket(false); - logMetric(); do { for (req = NULL; (hasPacket = _reqQ.pop(req, 60000)) && (req != NULL); req = NULL) { bool immediate = true; @@ -199,22 +198,10 @@ void TransLogServer::run() req->Return(); } } - logMetric(); } while (running() && !(hasPacket && (req == NULL))); LOG(info, "TLS Stopped"); } -void TransLogServer::logMetric() const -{ - Guard domainGuard(_lock); - for (DomainList::const_iterator it(_domains.begin()), mt(_domains.end()); it != mt; it++) { - vespalib::string prefix("translogserver." + it->first + ".serialnum."); - EV_COUNT((prefix + "last").c_str(), it->second->end()); - EV_COUNT((prefix + "first").c_str(), it->second->begin()); - EV_VALUE((prefix + "numused").c_str(), it->second->size()); - } -} - DomainStats TransLogServer::getDomainStats() const { diff --git a/searchlib/src/vespa/searchlib/transactionlog/translogserver.h b/searchlib/src/vespa/searchlib/transactionlog/translogserver.h index 9dcc17a4a1f..189be8c38d8 100644 --- a/searchlib/src/vespa/searchlib/transactionlog/translogserver.h +++ b/searchlib/src/vespa/searchlib/transactionlog/translogserver.h @@ -67,7 +67,6 @@ private: void finiSession(FRT_RPCRequest *req); void downSession(FRT_RPCRequest *req); - void logMetric() const; std::vector<vespalib::string> getDomainNames(); Domain::SP findDomain(vespalib::stringref name); vespalib::string dir() const { return _baseDir + "/" + _name; } diff --git a/storage/src/vespa/storage/bucketmover/bucketmover.cpp b/storage/src/vespa/storage/bucketmover/bucketmover.cpp index b47b3b59b4f..904e8a66c27 100644 --- a/storage/src/vespa/storage/bucketmover/bucketmover.cpp +++ b/storage/src/vespa/storage/bucketmover/bucketmover.cpp @@ -6,7 +6,6 @@ #include <vespa/storage/common/bucketmessages.h> #include <vespa/storage/common/content_bucket_space_repo.h> #include <vespa/storage/common/nodestateupdater.h> -#include <vespa/storage/storageutil/log.h> #include <vespa/config/common/exceptions.h> #include <vespa/vespalib/util/stringfmt.h> #include <vespa/document/bucket/fixed_bucket_spaces.h> diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp index d1f0a24178a..bf0244255c1 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp @@ -11,7 +11,6 @@ #include <vespa/storage/config/config-stor-server.h> #include <vespa/storage/persistence/bucketownershipnotifier.h> #include <vespa/storage/persistence/persistencethread.h> -#include <vespa/storage/storageutil/log.h> #include <vespa/storageapi/message/batch.h> #include <vespa/storageapi/message/bucketsplitting.h> #include <vespa/storageapi/message/state.h> diff --git a/storage/src/vespa/storage/storageserver/bucketintegritychecker.cpp b/storage/src/vespa/storage/storageserver/bucketintegritychecker.cpp index 01a84177e43..650637f206d 100644 --- a/storage/src/vespa/storage/storageserver/bucketintegritychecker.cpp +++ b/storage/src/vespa/storage/storageserver/bucketintegritychecker.cpp @@ -2,7 +2,6 @@ #include "bucketintegritychecker.h" #include <vespa/storage/common/bucketmessages.h> -#include <vespa/storage/storageutil/log.h> #include <vespa/storage/bucketdb/storbucketdb.h> #include <vespa/storageapi/message/state.h> #include <vespa/vdslib/distribution/distribution.h> diff --git a/storage/src/vespa/storage/storageutil/log.h b/storage/src/vespa/storage/storageutil/log.h deleted file mode 100644 index f7e89443b32..00000000000 --- a/storage/src/vespa/storage/storageutil/log.h +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#pragma once -#include <vespa/log/log.h> - -#define STORAGE_LOG_INTERVAL 30 - -#define STORAGE_LOG_COUNT(name, interval) do { \ - static uint64_t C_count ## name = 0; \ - static time_t C_last ## name = time(NULL); \ - C_count ## name ++; \ - time_t C_now ## name = time(NULL); \ - if (C_now ## name - C_last ## name >= interval) { \ - EV_COUNT(#name, C_count ## name); \ - C_last ## name = C_now ## name; \ - } } while (false) - -#define STORAGE_LOG_AVERAGE(name, value, interval) do { \ - static uint64_t A_count ## name = 0; \ - static float A_total ## name = 0.0; \ - static time_t A_last ## name = time(NULL); \ - A_count ## name ++; \ - A_total ## name += value; \ - time_t A_now ## name = time(NULL); \ - if (A_now ## name - A_last ## name >= interval) { \ - EV_VALUE(#name, A_total ## name / A_count ## name); \ - A_count ## name = 0; \ - A_total ## name = 0; \ - A_last ## name = A_now ## name; \ - }} while (false) - diff --git a/vespajlib/src/main/java/com/yahoo/path/Path.java b/vespajlib/src/main/java/com/yahoo/path/Path.java index c466fe50d6f..2806631be18 100644 --- a/vespajlib/src/main/java/com/yahoo/path/Path.java +++ b/vespajlib/src/main/java/com/yahoo/path/Path.java @@ -84,6 +84,7 @@ public final class Path { /** * Get the name of this path element, typically the last element in the path string. + * * @return the name */ public String getName() { |