diff options
123 files changed, 1248 insertions, 1767 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 7ca9bcf48f3..36009682022 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,7 +45,6 @@ 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; @@ -165,7 +164,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 a71a0878d3d..dd54fe11c39 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,14 +88,12 @@ 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(); @@ -162,8 +160,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 information abut the file + * @param relativePath The relative path of the file within this application package. + * @return reader for 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 6311751bb88..7b4d70d85b1 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) { - String rankProfileName = rankProfile.getName(); + final 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 d85d0983509..867740c7912 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,14 +39,11 @@ 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; @@ -65,155 +62,69 @@ import java.util.stream.Collectors; */ public class ConvertedModel { - private final String modelName; - private final Path modelPath; + private final ExpressionNode convertedExpression; - /** - * 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, + public ConvertedModel(FeatureArguments arguments, RankProfileTransformContext context, ModelImporter modelImporter, - FeatureArguments arguments) { // TODO: Remove - this.modelPath = modelPath; - this.modelName = toModelName(modelPath); - ModelStore store = new ModelStore(context.rankProfile().getSearch().sourceApplication(), modelPath); + Map<Path, ImportedModel> importedModels) { + ModelStore store = new ModelStore(context.rankProfile().getSearch().sourceApplication(), arguments); if ( ! store.hasStoredModel()) // not converted yet - access from models/ directory - expressions = importModel(store, context.rankProfile(), context.queryProfiles(), modelImporter, arguments); + convertedExpression = importModel(store, context.rankProfile(), context.queryProfiles(), modelImporter, importedModels); else - expressions = transformFromStoredModel(store, context.rankProfile()); + convertedExpression = transformFromStoredModel(store, context.rankProfile()); } - 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()); + 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 String missingExpressionMessageSuffix() { - return "' in model '" + this.modelPath + "'. " + - "Available expressions: " + expressions.keySet().stream().collect(Collectors.joining(", ")); - } + public ExpressionNode expression() { return convertedExpression; } - private Map<String, RankingExpression> transformFromImportedModel(ImportedModel model, - ModelStore store, - RankProfile profile, - QueryProfileRegistry queryProfiles, - FeatureArguments arguments) { + private ExpressionNode transformFromImportedModel(ImportedModel model, + ModelStore store, + RankProfile profile, + QueryProfileRegistry queryProfiles) { // 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)); - // 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); + // 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); } - if (signatureEntry.getValue().outputs().isEmpty()) { // fallback: Signature without outputs - addExpression(model.expressions().get(signatureEntry.getKey()), - modelName + "." + signatureEntry.getKey(), - constantsReplacedByMacros, - model, store, profile, queryProfiles, - expressions); + if (!signature.importWarnings().isEmpty()) { + message += ": " + String.join(", ", signature.importWarnings()); } + 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)); - 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) { + RankingExpression expression = model.expressions().get(output); expression = replaceConstantsByMacros(expression, constantsReplacedByMacros); verifyRequiredMacros(expression, model, profile, queryProfiles); + addGeneratedMacros(model, profile); reduceBatchDimensions(expression, model, profile, queryProfiles); - store.writeExpression(expressionName, expression); - expressions.put(expressionName, expression); + + model.macros().forEach((k, v) -> transformGeneratedMacro(store, constantsReplacedByMacros, k, v)); + + store.writeConverted(expression); + return expression.getRoot(); } - private Map<String, RankingExpression> transformFromStoredModel(ModelStore store, RankProfile profile) { + ExpressionNode transformFromStoredModel(ModelStore store, RankProfile profile) { for (Pair<String, Tensor> constant : store.readSmallConstants()) profile.addConstant(constant.getFirst(), asValue(constant.getSecond())); @@ -226,7 +137,60 @@ public class ConvertedModel { addGeneratedMacroToProfile(profile, macro.getFirst(), macro.getSecond()); } - return store.readExpressions(); + 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; + } } private void transformSmallConstant(ModelStore store, RankProfile profile, String constantName, Tensor constantValue) { @@ -263,15 +227,22 @@ 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. @@ -404,8 +375,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; @@ -481,29 +452,24 @@ 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 ModelFiles modelFiles; + private final FeatureArguments arguments; - ModelStore(ApplicationPackage application, Path modelPath) { + ModelStore(ApplicationPackage application, FeatureArguments arguments) { this.application = application; - this.modelFiles = new ModelFiles(modelPath); + this.arguments = arguments; } + public FeatureArguments arguments() { return arguments; } + public boolean hasStoredModel() { try { - return application.getFileReference(modelFiles.storedModelPath()).exists(); + return application.getFile(arguments.expressionPath()).exists(); } catch (UnsupportedOperationException e) { return false; @@ -514,49 +480,40 @@ 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(modelFiles.modelPath())); + return application.getFileReference(ApplicationPackage.MODELS_DIR.append(arguments.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 writeExpression(String name, RankingExpression expression) { - application.getFile(modelFiles.expressionPath(name)) + void writeConverted(RankingExpression expression) { + application.getFile(arguments.expressionPath()) .writeFile(new StringReader(expression.getRoot().toString())); } - 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); - } + /** 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); } - return expressions; } /** Adds this macro expression to the application package to it can be read later. */ void writeMacro(String name, RankingExpression expression) { - application.getFile(modelFiles.macrosPath()).appendFile(name + "\t" + + application.getFile(arguments.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(modelFiles.macrosPath()); + ApplicationFile file = application.getFile(arguments.macrosPath()); if (!file.exists()) return Collections.emptyList(); List<Pair<String, RankingExpression>> macros = new ArrayList<>(); @@ -570,7 +527,7 @@ public class ConvertedModel { macros.add(new Pair<>(name, expression)); } catch (ParseException e) { - throw new IllegalStateException("Could not parse " + name, e); + throw new IllegalStateException("Could not parse " + arguments.expressionPath(), e); } } return macros; @@ -587,7 +544,7 @@ public class ConvertedModel { List<RankingConstant> readLargeConstants() { try { List<RankingConstant> constants = new ArrayList<>(); - for (ApplicationFile constantFile : application.getFile(modelFiles.largeConstantsPath()).listFiles()) { + for (ApplicationFile constantFile : application.getFile(arguments.largeConstantsPath()).listFiles()) { String[] parts = IOUtils.readAll(constantFile.createReader()).split(":"); constants.add(new RankingConstant(parts[0], TensorType.fromSpec(parts[1]), parts[2])); } @@ -605,13 +562,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(modelFiles.modelPath()).append("constants"); + Path constantsPath = ApplicationPackage.MODELS_GENERATED_DIR.append(arguments.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(modelFiles.largeConstantsPath().append(name + ".constant")) + application.getFile(arguments.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 @@ -622,7 +579,7 @@ public class ConvertedModel { private List<Pair<String, Tensor>> readSmallConstants() { try { - ApplicationFile file = application.getFile(modelFiles.smallConstantsPath()); + ApplicationFile file = application.getFile(arguments.smallConstantsPath()); if (!file.exists()) return Collections.emptyList(); List<Pair<String, Tensor>> constants = new ArrayList<>(); @@ -647,7 +604,7 @@ public class ConvertedModel { */ public void writeSmallConstant(String name, Tensor constant) { // Secret file format for remembering constants: - application.getFile(modelFiles.smallConstantsPath()).appendFile(name + "\t" + + application.getFile(arguments.smallConstantsPath()).appendFile(name + "\t" + constant.type().toString() + "\t" + constant.toString() + "\n"); } @@ -671,24 +628,26 @@ public class ConvertedModel { } } - private void close(Reader reader) { - try { - if (reader != null) - reader.close(); - } - catch (IOException e) { - // ignore - } - } - } - static class ModelFiles { + /** Encapsulates the arguments to the import feature */ + static class FeatureArguments { Path modelPath; - public 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) { this.modelPath = modelPath; + this.signature = signature; + this.output = output; } /** Returns modelPath with slashes replaced by underscores */ @@ -696,66 +655,37 @@ 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; } - 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"); - } - + /** Path to the small constants file */ public Path smallConstantsPath() { - return storedModelPath().append("constants.txt"); + return ApplicationPackage.MODELS_GENERATED_DIR.append(modelPath).append("constants.txt"); } /** Path to the large (ranking) constants directory */ public Path largeConstantsPath() { - return storedModelPath().append("constants"); + return ApplicationPackage.MODELS_GENERATED_REPLICATED_DIR.append(modelPath).append("constants"); } /** Path to the macros file */ public Path macrosPath() { - return storedModelPath().append("macros.txt"); + return ApplicationPackage.MODELS_GENERATED_REPLICATED_DIR.append(modelPath).append("macros.txt"); } - } - - /** 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)); + public Path expressionPath() { + return ApplicationPackage.MODELS_GENERATED_REPLICATED_DIR + .append(modelPath).append("expressions").append(expressionFileName()); } - 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 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(); } private static Optional<String> optionalArgument(int argumentIndex, Arguments arguments) { @@ -764,7 +694,7 @@ public class ConvertedModel { return Optional.of(asString(arguments.expressions().get(argumentIndex))); } - public static String asString(ExpressionNode node) { + private 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 97395c1aad3..d31ffefde65 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, ConvertedModel> convertedModels = new HashMap<>(); + private final Map<Path, ImportedModel> importedModels = 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 { - 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())); + ConvertedModel convertedModel = new ConvertedModel(asFeatureArguments(feature.getArguments()), + context, onnxImporter, importedModels); + return convertedModel.expression(); } 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 b3778e2af84..d28299b1d30 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, ConvertedModel> convertedModels = new HashMap<>(); + private final Map<Path, ImportedModel> importedModels = 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 { - 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())); + ConvertedModel convertedModel = new ConvertedModel(asFeatureArguments(feature.getArguments()), + context, tensorFlowImporter, importedModels); + return convertedModel.expression(); } 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 62f43e15849..4ae223ec3a5 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,8 +37,7 @@ public class XgboostFeatureConverter extends ExpressionTransformer<RankProfileTr try { ConvertedModel.FeatureArguments arguments = asFeatureArguments(feature.getArguments()); - ConvertedModel.ModelStore store = new ConvertedModel.ModelStore(context.rankProfile().getSearch().sourceApplication(), - arguments.modelPath()); + ConvertedModel.ModelStore store = new ConvertedModel.ModelStore(context.rankProfile().getSearch().sourceApplication(), arguments); RankingExpression expression = xgboostImporter.parseModel(store.modelDir().toString()); return expression.getRoot(); } catch (IllegalArgumentException | UncheckedIOException e) { @@ -49,7 +48,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/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java index ab689b88993..0ce6129ef7f 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,9 +10,7 @@ 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; @@ -27,7 +25,6 @@ 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); @@ -57,38 +54,25 @@ class RankProfileSearchFixture { } public void assertFirstPhaseExpression(String expExpression, String rankProfile) { - assertEquals(expExpression, compiledRankProfile(rankProfile).getFirstPhaseRanking().getRoot().toString()); + assertEquals(expExpression, rankProfile(rankProfile).getFirstPhaseRanking().getRoot().toString()); } public void assertSecondPhaseExpression(String expExpression, String rankProfile) { - assertEquals(expExpression, compiledRankProfile(rankProfile).getSecondPhaseRanking().getRoot().toString()); + assertEquals(expExpression, rankProfile(rankProfile).getSecondPhaseRanking().getRoot().toString()); } public void assertRankProperty(String expValue, String name, String rankProfile) { - List<RankProfile.RankProperty> rankPropertyList = compiledRankProfile(rankProfile).getRankPropertyMap().get(name); + List<RankProfile.RankProperty> rankPropertyList = rankProfile(rankProfile).getRankPropertyMap().get(name); assertEquals(1, rankPropertyList.size()); assertEquals(expValue, rankPropertyList.get(0).getValue()); } - public void assertMacro(String expexctedExpression, String macroName, String rankProfile) { - assertEquals(expexctedExpression, - compiledRankProfile(rankProfile).getMacros().get(macroName).getRankingExpression().getRoot().toString()); + public void assertMacro(String expExpression, String macroName, String rankProfile) { + assertEquals(expExpression, rankProfile(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); - } - - /** 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); + return rankProfileRegistry.getRankProfile(search, rankProfile).compile(queryProfileRegistry); } 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 a7465fa9695..b2ef08dcc36 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,7 +123,6 @@ public class RankingExpressionWithOnnxTestCase { " expression: onnx('mnist_softmax.onnx')" + " }\n" + " }"); - search.compileRankProfile("my_profile"); search.assertFirstPhaseExpression(vespaExpression, "my_profile"); fail("Expecting exception"); } @@ -165,8 +164,7 @@ public class RankingExpressionWithOnnxTestCase { catch (IllegalArgumentException expected) { assertEquals("Rank profile 'my_profile' is invalid: Could not use Onnx model from " + "onnx('mnist_softmax.onnx','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", + "Model does not have the specified signature 'y'", Exceptions.toMessageString(expected)); } } @@ -222,8 +220,7 @@ 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 = uncompiledFixtureWith(rankProfile, new StoringApplicationPackage(applicationDir)); - search.compileRankProfile("my_profile"); + RankProfileSearchFixture search = fixtureWith(rankProfile, new StoringApplicationPackage(applicationDir)); search.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile"); assertNull("Constant overridden by macro is not added", @@ -237,8 +234,7 @@ 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 = uncompiledFixtureWith(rankProfile, storedApplication); - searchFromStored.compileRankProfile("my_profile"); + RankProfileSearchFixture searchFromStored = fixtureWith(rankProfile, storedApplication); searchFromStored.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile"); assertNull("Constant overridden by macro is not added", searchFromStored.search().getRankingConstants().get("mnist_softmax_onnx_Variable")); @@ -275,19 +271,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 uncompiledFixtureWith(String rankProfile, StoringApplicationPackage application) { + private RankProfileSearchFixture fixtureWith(String rankProfile, StoringApplicationPackage application) { try { return new RankProfileSearchFixture(application, application.getQueryProfiles(), - rankProfile, null, null); + rankProfile, null, null); } catch (ParseException e) { throw new IllegalArgumentException(e); @@ -301,7 +297,7 @@ public class RankingExpressionWithOnnxTestCase { String macroName, StoringApplicationPackage application) { try { - RankProfileSearchFixture fixture = new RankProfileSearchFixture( + return new RankProfileSearchFixture( application, application.getQueryProfiles(), " rank-profile my_profile {\n" + @@ -314,8 +310,6 @@ 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 29859817736..7228af2b0de 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,7 +6,6 @@ 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; @@ -23,12 +22,10 @@ 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; @@ -159,7 +156,6 @@ public class RankingExpressionWithTensorFlowTestCase { " expression: tensorflow('mnist_softmax/saved')" + " }\n" + " }"); - search.compileRankProfile("my_profile"); search.assertFirstPhaseExpression(vespaExpression, "my_profile"); fail("Expecting exception"); } @@ -200,9 +196,7 @@ 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'): " + - "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", + "Model does not have the specified signature 'serving_defaultz'", Exceptions.toMessageString(expected)); } } @@ -218,9 +212,7 @@ 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'): " + - "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", + "Model does not have the specified output 'x'", Exceptions.toMessageString(expected)); } } @@ -276,8 +268,7 @@ 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 = fixtureWithUncompiled(rankProfile, new StoringApplicationPackage(applicationDir)); - search.compileRankProfile("my_profile"); + RankProfileSearchFixture search = fixtureWith(rankProfile, new StoringApplicationPackage(applicationDir)); search.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile"); assertNull("Constant overridden by macro is not added", @@ -291,8 +282,7 @@ 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 = fixtureWithUncompiled(rankProfile, storedApplication); - searchFromStored.compileRankProfile("my_profile"); + RankProfileSearchFixture searchFromStored = fixtureWith(rankProfile, storedApplication); searchFromStored.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile"); assertNull("Constant overridden by macro is not added", searchFromStored.search().getRankingConstants().get("mnist_softmax_saved_layer_Variable_read")); @@ -307,7 +297,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)); @@ -372,7 +362,7 @@ public class RankingExpressionWithTensorFlowTestCase { } private void assertSmallConstant(String name, TensorType type, RankProfileSearchFixture search) { - Value value = search.compiledRankProfile("my_profile").getConstants().get(name); + Value value = search.rankProfile("my_profile").getConstants().get(name); assertNotNull(value); assertEquals(type, value.type()); } @@ -420,7 +410,7 @@ public class RankingExpressionWithTensorFlowTestCase { String macroName, StoringApplicationPackage application) { try { - RankProfileSearchFixture fixture = new RankProfileSearchFixture( + return new RankProfileSearchFixture( application, application.getQueryProfiles(), " rank-profile my_profile {\n" + @@ -433,15 +423,13 @@ public class RankingExpressionWithTensorFlowTestCase { " }", constant, field); - fixture.compileRankProfile("my_profile"); - return fixture; } catch (ParseException e) { throw new IllegalArgumentException(e); } } - private RankProfileSearchFixture fixtureWithUncompiled(String rankProfile, StoringApplicationPackage application) { + private RankProfileSearchFixture fixtureWith(String rankProfile, StoringApplicationPackage application) { try { return new RankProfileSearchFixture(application, application.getQueryProfiles(), rankProfile, null, null); @@ -475,21 +463,6 @@ 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 0866d3192cf..dba2bdbfbbf 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,7 +25,6 @@ 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"); @@ -48,7 +47,6 @@ 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"); @@ -67,7 +65,6 @@ 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"); @@ -88,7 +85,6 @@ 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"); @@ -110,7 +106,6 @@ 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"); @@ -132,7 +127,6 @@ 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"); @@ -145,7 +139,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)'"); - RankProfileSearchFixture f = new RankProfileSearchFixture( + new RankProfileSearchFixture( " rank-profile my_profile {\n" + " constants {\n" + " my_tensor {\n" + @@ -154,7 +148,6 @@ 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 f98783ad671..b65cb0b3d5f 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 { - RankProfileSearchFixture fixture = new RankProfileSearchFixture( + return new RankProfileSearchFixture( application, application.getQueryProfiles(), " rank-profile my_profile {\n" + @@ -46,8 +46,6 @@ 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 530f852e80a..e70304efcb5 100644 --- a/configd/src/apps/sentinel/service.cpp +++ b/configd/src/apps/sentinel/service.cpp @@ -339,7 +339,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 e9b4d6ac1aa..2c46f591037 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,14 +24,12 @@ 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. @@ -202,17 +200,16 @@ 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) { + public ApplicationFile getFile(Path file) { // foo/bar/baz.json 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 956af02e36f..d7d43dea022 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,8 +69,7 @@ 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); @@ -81,15 +80,14 @@ 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) { - String data = getData(path, node); + final String data = getData(path, node); if (data == null) { throw new IllegalArgumentException("No node for " + getFullPath(path) + "/" + node + " exists"); } @@ -100,8 +98,7 @@ 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); } } @@ -208,6 +205,5 @@ public class ZKLiveApp { } return reader(data); } - } 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 c98f6da3f29..1b13d9a5760 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 @@ -21,9 +21,9 @@ public class ZoneId { private final CloudName cloud; private ZoneId(Environment environment, RegionName region, CloudName cloud) { - this.environment = Objects.requireNonNull(environment); - this.region = Objects.requireNonNull(region); - this.cloud = cloud; + this.environment = Objects.requireNonNull(environment, "environment must be non-null"); + this.region = Objects.requireNonNull(region, "region must be non-null"); + this.cloud = Objects.requireNonNull(cloud, "cloud must be non-null"); } private ZoneId(Environment environment, RegionName region) { @@ -80,13 +80,12 @@ public class ZoneId { 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); + Objects.equals(region, zoneId.region); } @Override public int hashCode() { - return Objects.hash(environment, region, cloud); + return Objects.hash(environment, region); } } 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 4fbad88df8d..419e532c531 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 @@ -9,6 +9,7 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import java.net.URI; import java.time.Duration; +import java.util.Collections; import java.util.List; import java.util.Optional; @@ -58,6 +59,12 @@ public interface ZoneRegistry { return upgradePolicy(); } + // TODO: Remove default implementation + /** Returns all OS upgrade policies */ + default List<UpgradePolicy> osUpgradePolicies() { + return Collections.singletonList(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/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java index 95542141e47..3733022766e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java @@ -26,10 +26,12 @@ import com.yahoo.vespa.hosted.controller.api.integration.organization.Organizati import com.yahoo.vespa.hosted.controller.api.integration.routing.GlobalRoutingService; import com.yahoo.vespa.hosted.controller.api.integration.routing.RotationStatus; import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGenerator; +import com.yahoo.vespa.hosted.controller.api.integration.zone.CloudName; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry; import com.yahoo.vespa.hosted.controller.deployment.JobController; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.rotation.Rotation; +import com.yahoo.vespa.hosted.controller.versions.OsVersion; import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; @@ -42,6 +44,8 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.Set; +import java.util.TreeSet; import java.util.function.Predicate; import java.util.function.Supplier; import java.util.logging.Logger; @@ -217,14 +221,22 @@ public class Controller extends AbstractComponent { /** Returns the target OS version for infrastructure in this system. The controller will drive infrastructure OS * upgrades to this version */ - public Optional<Version> osVersion() { - return curator.readOsTargetVersion(); + public Optional<OsVersion> osVersion(CloudName cloud) { + return osVersions().stream().filter(osVersion -> osVersion.cloud().equals(cloud)).findFirst(); } - /** Set the target OS version for infrastructure in this system. */ - public void upgradeOs(Version version) { - try (Lock lock = curator.lockOsTargetVersion()) { - curator.writeOsTargetVersion(version); + /** Returns all target OS versions in this system */ + public Set<OsVersion> osVersions() { + return curator.readOsVersions(); + } + + /** Set the target OS version for infrastructure on cloud in this system */ + public void upgradeOsIn(CloudName cloud, Version version) { + try (Lock lock = curator.lockOsVersions()) { + Set<OsVersion> versions = new TreeSet<>(curator.readOsVersions()); + versions.removeIf(osVersion -> osVersion.cloud().equals(cloud)); // Only allow a single target per cloud + versions.add(new OsVersion(version, cloud)); + curator.writeOsVersions(versions); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java index fedd38c56b8..cc4f236f3b2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java @@ -14,6 +14,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import java.util.Collections; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; /** * This represents a system-level application in hosted Vespa. E.g. the zone-application. @@ -59,7 +60,7 @@ public enum SystemApplication { /** Returns whether this system application has an application package */ public boolean hasApplicationPackage() { - return nodeTypes.contains(NodeType.proxy); + return this == zone; } /** Returns whether config for this application has converged in given zone */ @@ -78,11 +79,7 @@ public enum SystemApplication { /** Returns the node types of this that should receive OS upgrades */ public Set<NodeType> nodeTypesWithUpgradableOs() { - // TODO: Change this to include all node types that are Docker hosts - if (this != zone) { - return Collections.emptySet(); - } - return Collections.singleton(NodeType.host); + return nodeTypes().stream().filter(NodeType::isDockerHost).collect(Collectors.toSet()); } /** All known system applications */ 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..ead9388fc3b 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 @@ -51,6 +51,7 @@ import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Con import static com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException.ErrorCode.APPLICATION_LOCK_FAILURE; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException.ErrorCode.OUT_OF_CAPACITY; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.active; +import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.failed; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.reserved; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.deploymentFailed; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.error; @@ -114,7 +115,7 @@ public class InternalStepRunner implements StepRunner { return Optional.empty(); } catch (RuntimeException e) { - logger.log(INFO, "Unexpected exception running " + id + ": " + Exceptions.toMessageString(e)); + logger.log(INFO, "Unexpected exception running " + id, e); if (JobProfile.of(id.type()).alwaysRun().contains(step.get())) { logger.log("Will keep trying, as this is a cleanup step."); return Optional.empty(); @@ -227,6 +228,11 @@ public class InternalStepRunner implements StepRunner { } private Optional<RunStatus> installReal(RunId id, boolean setTheStage, ByteArrayLogger logger) { + if (expired(id.application(), id.type())) { + logger.log(INFO, "Deployment expired before installation was successful."); + return Optional.of(installationFailed); + } + Versions versions = controller.jobController().run(id).get().versions(); Version platform = setTheStage ? versions.sourcePlatform().orElse(versions.targetPlatform()) : versions.targetPlatform(); ApplicationVersion application = setTheStage ? versions.sourceApplication().orElse(versions.targetApplication()) : versions.targetApplication(); @@ -248,6 +254,10 @@ public class InternalStepRunner implements StepRunner { private Optional<RunStatus> installTester(RunId id, ByteArrayLogger logger) { logger.log("Checking installation of tester container ..."); + if (expired(id.application(), id.type())) { + logger.log(INFO, "Deployment expired before tester was installed."); + return Optional.of(installationFailed); + } if (servicesConverged(testerOf(id.application()), id.type())) { logger.log("Tester container successfully installed!"); @@ -289,6 +299,11 @@ public class InternalStepRunner implements StepRunner { private Optional<RunStatus> startTests(RunId id, ByteArrayLogger logger) { logger.log("Attempting to find endpoints ..."); + if (expired(id.application(), id.type())) { + logger.log(INFO, "Deployment expired before tests could start."); + return Optional.of(installationFailed); + } + Map<ZoneId, List<URI>> endpoints = deploymentEndpoints(id.application()); logger.log("Found endpoints:\n" + endpoints.entrySet().stream() @@ -378,6 +393,11 @@ public class InternalStepRunner implements StepRunner { return application(id).deployments().get(type.zone(controller.system())).at().isBefore(controller.clock().instant().minus(timeout)); } + /** Returns whether the real deployment for the given job type has expired, i.e., no longer exists. */ + private boolean expired(ApplicationId id, JobType type) { + return ! application(id).deployments().containsKey(type.zone(controller.system())); + } + /** Returns a generated job report for the given run. */ private DeploymentJobs.JobReport report(Run run) { return new DeploymentJobs.JobReport(run.id().application(), @@ -524,6 +544,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/Step.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Step.java index 6ad57851846..cb5b70ef80a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Step.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Step.java @@ -40,7 +40,7 @@ public enum Step { deployTester, /** See that tester is done deploying, and is ready to serve. */ - installTester(deployTester), + installTester(deployReal, deployTester), /** Ask the tester to run its tests. */ startTests(installReal, installTester), 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 719fa7b46dc..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; @@ -74,22 +75,16 @@ public class Versions { public boolean equals(Object o) { if (this == o) return true; if ( ! (o instanceof Versions)) return false; - Versions versions = (Versions) o; - - if ( ! targetPlatform.equals(versions.targetPlatform)) return false; - if ( ! targetApplication.equals(versions.targetApplication)) return false; - if ( ! sourcePlatform.equals(versions.sourcePlatform)) return false; - return sourceApplication.equals(versions.sourceApplication); + 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() { - int result = targetPlatform.hashCode(); - result = 31 * result + targetApplication.hashCode(); - result = 31 * result + sourcePlatform.hashCode(); - result = 31 * result + sourceApplication.hashCode(); - return result; + return Objects.hash(targetPlatform, targetApplication, sourcePlatform, sourceApplication); } @Override diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java index 0214eb3cd6c..fd71020343e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java @@ -10,11 +10,15 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.NameService; import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryClientInterface; import com.yahoo.vespa.hosted.controller.api.integration.organization.DeploymentIssues; import com.yahoo.vespa.hosted.controller.api.integration.organization.OwnershipIssues; +import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.deployment.InternalStepRunner; import com.yahoo.vespa.hosted.controller.maintenance.config.MaintainerConfig; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import java.time.Duration; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; /** * Maintenance jobs of the controller. @@ -40,7 +44,7 @@ public class ControllerMaintenance extends AbstractComponent { private final ApplicationOwnershipConfirmer applicationOwnershipConfirmer; private final DnsMaintainer dnsMaintainer; private final SystemUpgrader systemUpgrader; - private final OsUpgrader osUpgrader; + private final List<OsUpgrader> osUpgraders; private final OsVersionStatusUpdater osVersionStatusUpdater; private final JobRunner jobRunner; @@ -65,7 +69,7 @@ public class ControllerMaintenance extends AbstractComponent { dnsMaintainer = new DnsMaintainer(controller, Duration.ofHours(12), jobControl, nameService); systemUpgrader = new SystemUpgrader(controller, Duration.ofMinutes(1), jobControl); jobRunner = new JobRunner(controller, Duration.ofSeconds(30), jobControl, new InternalStepRunner(controller, testerCloud)); - osUpgrader = new OsUpgrader(controller, Duration.ofMinutes(1), jobControl); + osUpgraders = osUpgraders(controller, jobControl); osVersionStatusUpdater = new OsVersionStatusUpdater(controller, maintenanceInterval, jobControl); } @@ -89,9 +93,19 @@ public class ControllerMaintenance extends AbstractComponent { applicationOwnershipConfirmer.deconstruct(); dnsMaintainer.deconstruct(); systemUpgrader.deconstruct(); - osUpgrader.deconstruct(); + osUpgraders.forEach(Maintainer::deconstruct); osVersionStatusUpdater.deconstruct(); jobRunner.deconstruct(); } + /** Create one OS upgrader per cloud found in the zone registry of controller */ + private static List<OsUpgrader> osUpgraders(Controller controller, JobControl jobControl) { + return controller.zoneRegistry().zones().controllerUpgraded().ids().stream() + .map(ZoneId::cloud) + .distinct() + .sorted() + .map(cloud -> new OsUpgrader(controller, Duration.ofMinutes(1), jobControl, cloud)) + .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java index 9c4deaa8370..a8e70aedacb 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java @@ -27,8 +27,9 @@ public abstract class InfrastructureUpgrader extends Maintainer { private final UpgradePolicy upgradePolicy; - public InfrastructureUpgrader(Controller controller, Duration interval, JobControl jobControl, UpgradePolicy upgradePolicy) { - super(controller, interval, jobControl); + public InfrastructureUpgrader(Controller controller, Duration interval, JobControl jobControl, + UpgradePolicy upgradePolicy, String name) { + super(controller, interval, jobControl, name); this.upgradePolicy = upgradePolicy; } @@ -74,26 +75,26 @@ public abstract class InfrastructureUpgrader extends Maintainer { return applications.stream().allMatch(application -> convergedOn(target, application, zone)); } - /** Upgrade components to target version. Implementation should be idempotent */ + /** Upgrade component to target version. Implementation should be idempotent */ protected abstract void upgrade(Version target, SystemApplication application, ZoneId zone); /** Returns whether application has converged to target version in zone */ protected abstract boolean convergedOn(Version target, SystemApplication application, ZoneId zone); - /** Returns target version for components upgraded by this */ + /** Returns the target version for the component upgraded by this, if any */ protected abstract Optional<Version> targetVersion(); - /** Returns whether the upgrader should require given node to upgrade in application */ - protected abstract boolean requireUpgradeOf(Node node, SystemApplication application); + /** Returns whether the upgrader should require given node to upgrade */ + protected abstract boolean requireUpgradeOf(Node node, SystemApplication application, ZoneId zone); /** Find the minimum value of a version field in a zone */ - protected Optional<Version> minVersion(ZoneId zone, SystemApplication application, Function<Node, Version> versionField) { + protected final Optional<Version> minVersion(ZoneId zone, SystemApplication application, Function<Node, Version> versionField) { try { return controller().configServer() .nodeRepository() .list(zone, application.id()) .stream() - .filter((node) -> requireUpgradeOf(node, application)) + .filter((node) -> requireUpgradeOf(node, application, zone)) .map(versionField) .min(Comparator.naturalOrder()); } catch (Exception e) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java index f6ccbf6aa4e..d76f557fce1 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java @@ -1,12 +1,9 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.maintenance; -import com.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.component.AbstractComponent; -import com.yahoo.component.ComponentId; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import java.time.Duration; import java.util.concurrent.ScheduledExecutorService; @@ -29,11 +26,17 @@ public abstract class Maintainer extends AbstractComponent implements Runnable { private final Duration maintenanceInterval; private final JobControl jobControl; private final ScheduledExecutorService service; + private final String name; public Maintainer(Controller controller, Duration interval, JobControl jobControl) { + this(controller, interval, jobControl, null); + } + + public Maintainer(Controller controller, Duration interval, JobControl jobControl, String name) { this.controller = controller; this.maintenanceInterval = interval; this.jobControl = jobControl; + this.name = name; service = new ScheduledThreadPoolExecutor(1); service.scheduleAtFixedRate(this, interval.toMillis(), interval.toMillis(), TimeUnit.MILLISECONDS); @@ -69,7 +72,9 @@ public abstract class Maintainer extends AbstractComponent implements Runnable { public Duration maintenanceInterval() { return maintenanceInterval; } - public String name() { return this.getClass().getSimpleName(); } + public final String name() { + return name == null ? this.getClass().getSimpleName() : name; + } /** Returns the name of this */ @Override diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java index 2937c2a1fed..bf9fbeb26d3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java @@ -6,12 +6,15 @@ import com.yahoo.component.Version; import com.yahoo.config.provision.SystemName; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node; +import com.yahoo.vespa.hosted.controller.api.integration.zone.CloudName; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.SystemApplication; +import com.yahoo.vespa.hosted.controller.versions.OsVersion; import java.time.Duration; import java.util.Optional; import java.util.Set; +import java.util.logging.Logger; /** * Maintenance job that schedules upgrades of OS / kernel on nodes in the system. @@ -20,14 +23,19 @@ import java.util.Set; */ public class OsUpgrader extends InfrastructureUpgrader { + private static final Logger log = Logger.getLogger(OsUpgrader.class.getName()); + private static final Set<Node.State> upgradableNodeStates = ImmutableSet.of( Node.State.ready, Node.State.active, Node.State.reserved ); - public OsUpgrader(Controller controller, Duration interval, JobControl jobControl) { - super(controller, interval, jobControl, controller.zoneRegistry().osUpgradePolicy()); + private final CloudName cloud; + + public OsUpgrader(Controller controller, Duration interval, JobControl jobControl, CloudName cloud) { + super(controller, interval, jobControl, controller.zoneRegistry().osUpgradePolicy(cloud), name(cloud)); + this.cloud = cloud; } @Override @@ -38,6 +46,8 @@ public class OsUpgrader extends InfrastructureUpgrader { @Override protected void upgrade(Version target, SystemApplication application, ZoneId zone) { + log.info(String.format("Upgrading OS of %s to version %s in %s", application.id(), target, zone)); + // Node repository ensures the upgrade call is idempotent application.nodeTypesWithUpgradableOs().forEach(nodeType -> controller().configServer().nodeRepository() .upgradeOs(zone, nodeType, target)); } @@ -48,26 +58,42 @@ public class OsUpgrader extends InfrastructureUpgrader { } @Override - protected boolean requireUpgradeOf(Node node, SystemApplication application) { - return eligibleForUpgrade(node, application); - } - - private Version currentVersion(ZoneId zone, SystemApplication application, Version defaultVersion) { - return minVersion(zone, application, Node::currentOsVersion).orElse(defaultVersion); + protected boolean requireUpgradeOf(Node node, SystemApplication application, ZoneId zone) { + return cloud.equals(zone.cloud()) && eligibleForUpgrade(node, application); } @Override protected Optional<Version> targetVersion() { - // Only schedule upgrades if we have nodes in the system on a lower version - return controller().curator().readOsTargetVersion().filter( - target -> controller().osVersionStatus().versions().stream() - .anyMatch(osVersion -> osVersion.version().isBefore(target)) - ); + // Return target if we have nodes in this cloud on a lower version + return controller().osVersion(cloud) + .filter(target -> controller().osVersionStatus().nodeVersionsIn(cloud).stream() + .anyMatch(node -> node.version().isBefore(target.version()))) + .map(OsVersion::version); + } + + private Version currentVersion(ZoneId zone, SystemApplication application, Version defaultVersion) { + return minVersion(zone, application, Node::currentOsVersion).orElse(defaultVersion); } /** Returns whether node in application should be upgraded by this */ public static boolean eligibleForUpgrade(Node node, SystemApplication application) { - return upgradableNodeStates.contains(node.state()) && application.nodeTypesWithUpgradableOs().contains(node.type()); + return upgradableNodeStates.contains(node.state()) && + application.nodeTypesWithUpgradableOs().contains(node.type()); + } + + private static String name(CloudName cloud) { + return capitalize(cloud.value()) + OsUpgrader.class.getSimpleName(); // Prefix maintainer name with cloud name + } + + private static String capitalize(String s) { + if (s.isEmpty()) { + return s; + } + char firstLetter = Character.toUpperCase(s.charAt(0)); + if (s.length() > 1) { + return firstLetter + s.substring(1).toLowerCase(); + } + return String.valueOf(firstLetter); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java index afeca83d0d8..5c9c39e645e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java @@ -26,7 +26,7 @@ public class SystemUpgrader extends InfrastructureUpgrader { private static final Set<Node.State> upgradableNodeStates = ImmutableSet.of(Node.State.active, Node.State.reserved); public SystemUpgrader(Controller controller, Duration interval, JobControl jobControl) { - super(controller, interval, jobControl, controller.zoneRegistry().upgradePolicy()); + super(controller, interval, jobControl, controller.zoneRegistry().upgradePolicy(), null); } @Override @@ -44,7 +44,7 @@ public class SystemUpgrader extends InfrastructureUpgrader { } @Override - protected boolean requireUpgradeOf(Node node, SystemApplication application) { + protected boolean requireUpgradeOf(Node node, SystemApplication application, ZoneId zone) { return eligibleForUpgrade(node); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index 8bc0f5786c6..2ed69ad9be8 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -21,6 +21,7 @@ import com.yahoo.vespa.hosted.controller.deployment.Step; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.vespa.hosted.controller.tenant.UserTenant; +import com.yahoo.vespa.hosted.controller.versions.OsVersion; import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; @@ -71,7 +72,8 @@ public class CuratorDb { private final TenantSerializer tenantSerializer = new TenantSerializer(); private final ApplicationSerializer applicationSerializer = new ApplicationSerializer(); private final RunSerializer runSerializer = new RunSerializer(); - private final OsVersionStatusSerializer osVersionStatusSerializer = new OsVersionStatusSerializer(); + private final OsVersionSerializer osVersionSerializer = new OsVersionSerializer(); + private final OsVersionStatusSerializer osVersionStatusSerializer = new OsVersionStatusSerializer(osVersionSerializer); private final Curator curator; @@ -151,7 +153,7 @@ public class CuratorDb { return lock(lockRoot.append("openStackServerPoolLock"), Duration.ofSeconds(1)); } - public Lock lockOsTargetVersion() { + public Lock lockOsVersions() { return lock(lockRoot.append("osTargetVersion"), defaultLockTimeout); } @@ -247,12 +249,12 @@ public class CuratorDb { // Infrastructure upgrades - public void writeOsTargetVersion(Version version) { - curator.set(osTargetVersionPath(), asJson(versionSerializer.toSlime(version))); + public void writeOsVersions(Set<OsVersion> versions) { + curator.set(osTargetVersionPath(), asJson(osVersionSerializer.toSlime(versions))); } - public Optional<Version> readOsTargetVersion() { - return readSlime(osTargetVersionPath()).map(versionSerializer::fromSlime); + public Set<OsVersion> readOsVersions() { + return readSlime(osTargetVersionPath()).map(osVersionSerializer::fromSlime).orElseGet(Collections::emptySet); } public void writeOsVersionStatus(OsVersionStatus status) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializer.java new file mode 100644 index 00000000000..aee19644b82 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializer.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.persistence; + +import com.yahoo.component.Version; +import com.yahoo.slime.ArrayTraverser; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Inspector; +import com.yahoo.slime.Slime; +import com.yahoo.vespa.hosted.controller.api.integration.zone.CloudName; +import com.yahoo.vespa.hosted.controller.versions.OsVersion; + +import java.util.Collections; +import java.util.Set; +import java.util.TreeSet; + +/** + * Serializer for an OS version. + * + * @author mpolden + */ +public class OsVersionSerializer { + + private static final String versionsField = "versions"; + private static final String versionField = "version"; + private static final String cloudField = "cloud"; + + public Slime toSlime(Set<OsVersion> osVersions) { + Slime slime = new Slime(); + Cursor root = slime.setObject(); + Cursor array = root.setArray(versionsField); + osVersions.forEach(osVersion -> toSlime(osVersion, array.addObject())); + return slime; + } + + public void toSlime(OsVersion osVersion, Cursor object) { + object.setString(versionField, osVersion.version().toFullString()); + object.setString(cloudField, osVersion.cloud().value()); + } + + public Set<OsVersion> fromSlime(Slime slime) { + Inspector array = slime.get().field(versionsField); + Set<OsVersion> osVersions = new TreeSet<>(); + array.traverse((ArrayTraverser) (i, inspector) -> osVersions.add(fromSlime(inspector))); + return Collections.unmodifiableSet(osVersions); + } + + public OsVersion fromSlime(Inspector object) { + return new OsVersion( + Version.fromString(object.field(versionField).asString()), + cloudFrom(object.field(cloudField)) + ); + } + + // TODO: Simplify and inline after 2018-09-01 + private static CloudName cloudFrom(Inspector field) { + if (!field.valid()) { + return CloudName.defaultName(); + } + return CloudName.from(field.asString()); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java index 19beca661be..b0557863426 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java @@ -15,6 +15,9 @@ import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.TreeMap; /** * Serializer for OS version status. @@ -30,11 +33,21 @@ public class OsVersionStatusSerializer { private static final String regionField = "region"; private static final String environmentField = "environment"; + private final OsVersionSerializer osVersionSerializer; + + public OsVersionStatusSerializer(OsVersionSerializer osVersionSerializer) { + this.osVersionSerializer = Objects.requireNonNull(osVersionSerializer, "osVersionSerializer must be non-null"); + } + public Slime toSlime(OsVersionStatus status) { Slime slime = new Slime(); Cursor root = slime.setObject(); Cursor versions = root.setArray(versionsField); - status.versions().forEach(version -> osVersionToSlime(version, versions.addObject())); + status.versions().forEach((version, nodes) -> { + Cursor object = versions.addObject(); + osVersionSerializer.toSlime(version, object); + nodesToSlime(nodes, object.setArray(nodesField)); + }); return slime; } @@ -42,43 +55,35 @@ public class OsVersionStatusSerializer { return new OsVersionStatus(osVersionsFromSlime(slime.get().field(versionsField))); } - private void osVersionToSlime(OsVersion version, Cursor object) { - object.setString(versionField, version.version().toFullString()); - nodesToSlime(version.nodes(), object.setArray(nodesField)); - } - - private void nodesToSlime(List<OsVersion.Node> nodes, Cursor array) { + private void nodesToSlime(List<OsVersionStatus.Node> nodes, Cursor array) { nodes.forEach(node -> nodeToSlime(node, array.addObject())); } - private void nodeToSlime(OsVersion.Node node, Cursor object) { + private void nodeToSlime(OsVersionStatus.Node node, Cursor object) { object.setString(hostnameField, node.hostname().value()); object.setString(versionField, node.version().toFullString()); object.setString(regionField, node.region().value()); object.setString(environmentField, node.environment().value()); } - private List<OsVersion> osVersionsFromSlime(Inspector array) { - List<OsVersion> versions = new ArrayList<>(); - array.traverse((ArrayTraverser) (i, object) -> versions.add(osVersionFromSlime(object))); - return Collections.unmodifiableList(versions); - } - - private OsVersion osVersionFromSlime(Inspector object) { - return new OsVersion( - Version.fromString(object.field(versionField).asString()), - nodesFromSlime(object.field(nodesField)) - ); + private Map<OsVersion, List<OsVersionStatus.Node>> osVersionsFromSlime(Inspector array) { + Map<OsVersion, List<OsVersionStatus.Node>> versions = new TreeMap<>(); + array.traverse((ArrayTraverser) (i, object) -> { + OsVersion osVersion = osVersionSerializer.fromSlime(object); + List<OsVersionStatus.Node> nodes = nodesFromSlime(object.field(nodesField)); + versions.put(osVersion, nodes); + }); + return Collections.unmodifiableMap(versions); } - private List<OsVersion.Node> nodesFromSlime(Inspector array) { - List<OsVersion.Node> nodes = new ArrayList<>(); + private List<OsVersionStatus.Node> nodesFromSlime(Inspector array) { + List<OsVersionStatus.Node> nodes = new ArrayList<>(); array.traverse((ArrayTraverser) (i, object) -> nodes.add(nodeFromSlime(object))); return Collections.unmodifiableList(nodes); } - private OsVersion.Node nodeFromSlime(Inspector object) { - return new OsVersion.Node( + private OsVersionStatus.Node nodeFromSlime(Inspector object) { + return new OsVersionStatus.Node( HostName.from(object.field(hostnameField).asString()), Version.fromString(object.field(versionField).asString()), Environment.from(object.field(environmentField).asString()), diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersion.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersion.java index e132236d07c..89009270f10 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersion.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersion.java @@ -1,28 +1,25 @@ // 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.versions; -import com.google.common.collect.ImmutableList; import com.yahoo.component.Version; -import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.RegionName; +import com.yahoo.vespa.hosted.controller.api.integration.zone.CloudName; +import org.jetbrains.annotations.NotNull; -import java.util.List; import java.util.Objects; /** - * Information about a given OS version in the system. + * An OS version for a cloud in this system. * * @author mpolden */ -public class OsVersion { +public class OsVersion implements Comparable<OsVersion> { private final Version version; - private final List<Node> nodes; + private final CloudName cloud; - public OsVersion(Version version, List<Node> nodes) { - this.version = version; - this.nodes = ImmutableList.copyOf(nodes); + public OsVersion(Version version, CloudName cloud) { + this.version = Objects.requireNonNull(version, "version must be non-null"); + this.cloud = Objects.requireNonNull(cloud, "cloud must be non-null"); } /** The version number of this */ @@ -30,56 +27,37 @@ public class OsVersion { return version; } - /** Nodes on this version */ - public List<Node> nodes() { - return nodes; + /** The cloud where this OS version is used */ + public CloudName cloud() { + return cloud; } - public static class Node { - - private final HostName hostname; - private final Version version; - private final Environment environment; - private final RegionName region; - - public Node(HostName hostname, Version version, Environment environment, RegionName region) { - this.hostname = hostname; - this.version = version; - this.environment = environment; - this.region = region; - } - - public HostName hostname() { - return hostname; - } - - public Version version() { - return version; - } - - public Environment environment() { - return environment; - } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + OsVersion osVersion = (OsVersion) o; + return Objects.equals(version, osVersion.version) && + Objects.equals(cloud, osVersion.cloud); + } - public RegionName region() { - return region; - } + @Override + public int hashCode() { + return Objects.hash(version, cloud); + } - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - Node node = (Node) o; - return Objects.equals(hostname, node.hostname) && - Objects.equals(version, node.version) && - environment == node.environment && - Objects.equals(region, node.region); - } + @Override + public String toString() { + return "version " + version + " for " + cloud + " cloud"; + } - @Override - public int hashCode() { - return Objects.hash(hostname, version, environment, region); + @Override + public int compareTo(@NotNull OsVersion o) { + int cloudCmp = cloud.compareTo(o.cloud()); + if (cloudCmp == 0) { // Same cloud, sort by version + return version.compareTo(o.version()); } + return cloudCmp; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionStatus.java index 518394b46fc..871a7872a9c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionStatus.java @@ -1,21 +1,28 @@ // 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.versions; -import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.yahoo.component.Version; +import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.HostName; +import com.yahoo.config.provision.RegionName; import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.api.integration.zone.CloudName; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.vespa.hosted.controller.maintenance.OsUpgrader; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; -import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.stream.Collectors; +import static java.util.stream.Collectors.collectingAndThen; + /** * Information about OS versions in this system. * @@ -23,36 +30,44 @@ import java.util.stream.Collectors; */ public class OsVersionStatus { - public static final OsVersionStatus empty = new OsVersionStatus(Collections.emptyList()); + public static final OsVersionStatus empty = new OsVersionStatus(Collections.emptyMap()); - private final List<OsVersion> versions; + private final Map<OsVersion, List<Node>> versions; - public OsVersionStatus(List<OsVersion> versions) { - this.versions = ImmutableList.copyOf(versions); + /** Public for serialization purpose only. Use {@link OsVersionStatus#compute(Controller)} for an up-to-date status */ + public OsVersionStatus(Map<OsVersion, List<Node>> versions) { + this.versions = ImmutableMap.copyOf(Objects.requireNonNull(versions, "versions must be non-null")); } - /** All known OS versions */ - public List<OsVersion> versions() { + /** All known OS versions and their nodes */ + public Map<OsVersion, List<Node>> versions() { return versions; } - /** - * Compute the current OS version status in this status. This is expensive as all config servers in the system - * must be queried. - */ + /** Returns node versions that exist in given cloud */ + public List<Node> nodeVersionsIn(CloudName cloud) { + return versions.entrySet().stream() + .filter(entry -> entry.getKey().cloud().equals(cloud)) + .flatMap(entry -> entry.getValue().stream()) + .collect(collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); + } + + /** Compute the current OS versions in this system. This is expensive and should be called infrequently */ public static OsVersionStatus compute(Controller controller) { - Map<Version, List<OsVersion.Node>> versions = new HashMap<>(); - // Always include target version, if set - controller.osVersion().ifPresent(version -> versions.put(version, new ArrayList<>())); + Map<OsVersion, List<Node>> versions = new HashMap<>(); + + // Always include all target versions + controller.osVersions().forEach(osVersion -> versions.put(osVersion, new ArrayList<>())); + for (SystemApplication application : SystemApplication.all()) { if (application.nodeTypesWithUpgradableOs().isEmpty()) { - continue; // Avoid querying applications that do not have nodes with upgradable OS + continue; // Avoid querying applications that do not contain nodes with upgradable OS } - for (ZoneId zone : controller.zoneRegistry().zones().controllerUpgraded().ids()) { + for (ZoneId zone : zonesToUpgrade(controller)) { controller.configServer().nodeRepository().list(zone, application.id()).stream() .filter(node -> OsUpgrader.eligibleForUpgrade(node, application)) - .map(node -> new OsVersion.Node(node.hostname(), node.currentOsVersion(), zone.environment(), zone.region())) - .forEach(node -> versions.compute(node.version(), (ignored, nodes) -> { + .map(node -> new Node(node.hostname(), node.currentOsVersion(), zone.environment(), zone.region())) + .forEach(node -> versions.compute(new OsVersion(node.version(), zone.cloud()), (ignored, nodes) -> { if (nodes == null) { nodes = new ArrayList<>(); } @@ -61,11 +76,63 @@ public class OsVersionStatus { })); } } - return new OsVersionStatus(versions.entrySet() - .stream() - .map(kv -> new OsVersion(kv.getKey(), kv.getValue())) - .sorted(Comparator.comparing(OsVersion::version)) - .collect(Collectors.toList())); + + return new OsVersionStatus(versions); + } + + private static List<ZoneId> zonesToUpgrade(Controller controller) { + return controller.zoneRegistry().osUpgradePolicies().stream() + .flatMap(upgradePolicy -> upgradePolicy.asList().stream()) + .flatMap(Collection::stream) + .collect(Collectors.toList()); + } + + /** A node in this system and its current OS version */ + public static class Node { + + private final HostName hostname; + private final Version version; + private final Environment environment; + private final RegionName region; + + public Node(HostName hostname, Version version, Environment environment, RegionName region) { + this.hostname = Objects.requireNonNull(hostname, "hostname must be non-null"); + this.version = Objects.requireNonNull(version, "version must be non-null"); + this.environment = Objects.requireNonNull(environment, "environment must be non-null"); + this.region = Objects.requireNonNull(region, "region must be non-null"); + } + + public HostName hostname() { + return hostname; + } + + public Version version() { + return version; + } + + public Environment environment() { + return environment; + } + + public RegionName region() { + return region; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Node node = (Node) o; + return Objects.equals(hostname, node.hostname) && + Objects.equals(version, node.version) && + environment == node.environment && + Objects.equals(region, node.region); + } + + @Override + public int hashCode() { + return Objects.hash(hostname, version, environment, region); + } } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java index 66756dc4415..ba0607892e6 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java @@ -291,6 +291,34 @@ public class InternalStepRunnerTest { } @Test + public void installationFailsIfDeploymentExpires() { + newRun(JobType.systemTest); + runner.run(); + tester.configServer().convergeServices(appId, JobType.systemTest.zone(tester.controller().system())); + runner.run(); + assertEquals(succeeded, jobs.last(appId, JobType.systemTest).get().steps().get(Step.installReal)); + + tester.applications().deactivate(appId, JobType.systemTest.zone(tester.controller().system())); + runner.run(); + assertEquals(failed, jobs.last(appId, JobType.systemTest).get().steps().get(Step.installTester)); + assertTrue(jobs.last(appId, JobType.systemTest).get().hasEnded()); + assertTrue(jobs.last(appId, JobType.systemTest).get().hasFailed()); + } + + @Test + public void startTestsFailsIfDeploymentExpires() { + newRun(JobType.systemTest); + runner.run(); + tester.configServer().convergeServices(appId, JobType.systemTest.zone(tester.controller().system())); + tester.configServer().convergeServices(testerOf(appId), JobType.systemTest.zone(tester.controller().system())); + runner.run(); + + tester.applications().deactivate(appId, JobType.systemTest.zone(tester.controller().system())); + runner.run(); + assertEquals(failed, jobs.last(appId, JobType.systemTest).get().steps().get(Step.startTests)); + } + + @Test public void testsFailIfEndpointsAreGone() { RunId id = startSystemTestTests(); cloud.set(new byte[0], TesterCloud.Status.NOT_STARTED); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java index a243744a337..44823ab5777 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java @@ -5,17 +5,19 @@ import com.yahoo.component.Version; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.SystemName; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node; +import com.yahoo.vespa.hosted.controller.api.integration.zone.CloudName; import com.yahoo.vespa.hosted.controller.api.integration.zone.UpgradePolicy; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import com.yahoo.vespa.hosted.controller.integration.NodeRepositoryMock; -import com.yahoo.vespa.hosted.controller.versions.OsVersion; +import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus; import org.junit.Before; import org.junit.Test; import java.time.Duration; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.function.Function; @@ -34,6 +36,7 @@ public class OsUpgraderTest { private static final ZoneId zone2 = ZoneId.from("prod", "us-west-1"); private static final ZoneId zone3 = ZoneId.from("prod", "us-central-1"); private static final ZoneId zone4 = ZoneId.from("prod", "us-east-3"); + private static final ZoneId zone5 = ZoneId.from("prod", "us-north-1", "other"); private DeploymentTester tester; private OsVersionStatusUpdater statusUpdater; @@ -51,18 +54,19 @@ public class OsUpgraderTest { UpgradePolicy.create() .upgrade(zone1) .upgradeInParallel(zone2, zone3) - .upgrade(zone4) + .upgrade(zone5) // Belongs to a different cloud and is ignored by this upgrader + .upgrade(zone4), + SystemName.cd ); // Bootstrap system - tester.configServer().bootstrap(Arrays.asList(zone1, zone2, zone3, zone4), + tester.configServer().bootstrap(Arrays.asList(zone1, zone2, zone3, zone4, zone5), singletonList(SystemApplication.zone), Optional.of(NodeType.host)); // Add system applications that exist in a real system, but are currently not upgraded - tester.configServer().addNodes(Arrays.asList(zone1, zone2, zone3, zone4), - Arrays.asList(SystemApplication.configServer, - SystemApplication.configServerHost), + tester.configServer().addNodes(Arrays.asList(zone1, zone2, zone3, zone4, zone5), + Collections.singletonList(SystemApplication.configServer), Optional.empty()); // Fail a few nodes. Failed nodes should not affect versions @@ -71,7 +75,10 @@ public class OsUpgraderTest { // New OS version released Version version1 = Version.fromString("7.1"); - tester.controller().upgradeOs(version1); + CloudName cloud = CloudName.defaultName(); + tester.controller().upgradeOsIn(cloud, Version.fromString("7.0")); + tester.controller().upgradeOsIn(cloud, version1); + assertEquals(1, tester.controller().osVersions().size()); // Only allows one version per cloud statusUpdater.maintain(); // zone 1: begins upgrading @@ -85,7 +92,7 @@ public class OsUpgraderTest { completeUpgrade(version1, SystemApplication.zone, zone1); statusUpdater.maintain(); assertEquals(2, nodesOn(version1).size()); - assertEquals(8, nodesOn(Version.emptyVersion).size()); + assertEquals(11, nodesOn(Version.emptyVersion).size()); // zone 2 and 3: begins upgrading osUpgrader.maintain(); @@ -108,14 +115,41 @@ public class OsUpgraderTest { osUpgrader.maintain(); assertWanted(version1, SystemApplication.zone, zone1, zone2, zone3, zone4); statusUpdater.maintain(); - assertTrue("All nodes on target version", tester.controller().osVersionStatus().versions().stream() - .allMatch(osVersion -> osVersion.version().equals(version1))); + assertTrue("All nodes on target version", tester.controller().osVersionStatus().nodeVersionsIn(cloud).stream() + .allMatch(node -> node.version().equals(version1))); } - private List<OsVersion.Node> nodesOn(Version version) { - return tester.controller().osVersionStatus().versions().stream() - .filter(osVersion -> osVersion.version().equals(version)) - .flatMap(osVersion -> osVersion.nodes().stream()) + // TODO: Remove once enabled in all systems + @Test + public void os_upgrade_in_main_does_nothing() { + OsUpgrader osUpgrader = osUpgrader( + UpgradePolicy.create() + .upgrade(zone1) + .upgradeInParallel(zone2, zone3) + .upgrade(zone4), + SystemName.main + ); + + // Bootstrap system + tester.configServer().bootstrap(Arrays.asList(zone1, zone2, zone3, zone4, zone5), + singletonList(SystemApplication.zone), + Optional.of(NodeType.host)); + + // New OS is released + CloudName cloud = CloudName.defaultName(); + Version version1 = Version.fromString("7.1"); + tester.controller().upgradeOsIn(cloud, version1); + statusUpdater.maintain(); + + // Nothing happens as main is explicitly disabled + osUpgrader.maintain(); + assertWanted(Version.emptyVersion, SystemApplication.zone, zone1); + } + + private List<OsVersionStatus.Node> nodesOn(Version version) { + return tester.controller().osVersionStatus().versions().entrySet().stream() + .filter(entry -> entry.getKey().version().equals(version)) + .flatMap(entry -> entry.getValue().stream()) .collect(Collectors.toList()); } @@ -172,11 +206,13 @@ public class OsUpgraderTest { return tester.controllerTester().configServer().nodeRepository(); } - private OsUpgrader osUpgrader(UpgradePolicy upgradePolicy) { - tester.controllerTester().zoneRegistry().setSystemName(SystemName.cd); - tester.controllerTester().zoneRegistry().setUpgradePolicy(upgradePolicy); + private OsUpgrader osUpgrader(UpgradePolicy upgradePolicy, SystemName system) { + tester.controllerTester().zoneRegistry() + .setZones(zone1, zone2, zone3, zone4, zone5) + .setSystemName(system) + .setUpgradePolicy(upgradePolicy); return new OsUpgrader(tester.controller(), Duration.ofDays(1), - new JobControl(tester.controllerTester().curator())); + new JobControl(tester.controllerTester().curator()), CloudName.defaultName()); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java index 48e1308a892..98ed64ba879 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java @@ -3,6 +3,9 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.component.Version; import com.yahoo.vespa.hosted.controller.ControllerTester; +import com.yahoo.vespa.hosted.controller.api.integration.zone.CloudName; +import com.yahoo.vespa.hosted.controller.api.integration.zone.UpgradePolicy; +import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; import com.yahoo.vespa.hosted.controller.versions.OsVersion; import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus; @@ -10,10 +13,12 @@ import org.junit.Test; import java.time.Duration; import java.util.List; +import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; /** * @author mpolden @@ -25,17 +30,26 @@ public class OsVersionStatusUpdaterTest { ControllerTester tester = new ControllerTester(); OsVersionStatusUpdater statusUpdater = new OsVersionStatusUpdater(tester.controller(), Duration.ofDays(1), new JobControl(new MockCuratorDb())); + // Add all zones to upgrade policy + UpgradePolicy upgradePolicy = UpgradePolicy.create(); + for (ZoneId zone : tester.zoneRegistry().zones().controllerUpgraded().ids()) { + upgradePolicy = upgradePolicy.upgrade(zone); + } + tester.zoneRegistry().setUpgradePolicy(upgradePolicy); // Initially empty assertSame(OsVersionStatus.empty, tester.controller().osVersionStatus()); // Setting a new target adds it to current status Version version1 = Version.fromString("7.1"); - tester.controller().upgradeOs(version1); + CloudName cloud = CloudName.defaultName(); + tester.controller().upgradeOsIn(cloud, version1); statusUpdater.maintain(); - List<OsVersion> osVersions = tester.controller().osVersionStatus().versions(); - assertFalse(osVersions.isEmpty()); - assertEquals(version1, osVersions.get(0).version()); + + Map<OsVersion, List<OsVersionStatus.Node>> osVersions = tester.controller().osVersionStatus().versions(); + assertEquals(2, osVersions.size()); + assertFalse("All nodes on unknown version", osVersions.get(new OsVersion(Version.emptyVersion, cloud)).isEmpty()); + assertTrue("No nodes on current target", osVersions.get(new OsVersion(version1, cloud)).isEmpty()); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializerTest.java new file mode 100644 index 00000000000..bd510cc9aef --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializerTest.java @@ -0,0 +1,30 @@ +// 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.persistence; + +import com.google.common.collect.ImmutableSet; +import com.yahoo.component.Version; +import com.yahoo.vespa.hosted.controller.api.integration.zone.CloudName; +import com.yahoo.vespa.hosted.controller.versions.OsVersion; +import org.junit.Test; + +import java.util.Set; + +import static org.junit.Assert.assertEquals; + +/** + * @author mpolden + */ +public class OsVersionSerializerTest { + + @Test + public void test_serialization() { + OsVersionSerializer serializer = new OsVersionSerializer(); + Set<OsVersion> osVersions = ImmutableSet.of( + new OsVersion(Version.fromString("7.1"), CloudName.defaultName()), + new OsVersion(Version.fromString("7.1"), CloudName.from("foo")) + ); + Set<OsVersion> serialized = serializer.fromSlime(serializer.toSlime(osVersions)); + assertEquals(osVersions, serialized); + } + +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializerTest.java index e0d3fe3ba29..9840bec0f9a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializerTest.java @@ -5,12 +5,15 @@ import com.yahoo.component.Version; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.RegionName; +import com.yahoo.vespa.hosted.controller.api.integration.zone.CloudName; import com.yahoo.vespa.hosted.controller.versions.OsVersion; import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus; import org.junit.Test; import java.util.Arrays; import java.util.List; +import java.util.Map; +import java.util.TreeMap; import static org.junit.Assert.assertEquals; @@ -23,27 +26,22 @@ public class OsVersionStatusSerializerTest { public void test_serialization() { Version version1 = Version.fromString("7.1"); Version version2 = Version.fromString("7.2"); - List<OsVersion> osVersions = Arrays.asList( - new OsVersion(version1, Arrays.asList( - new OsVersion.Node(HostName.from("node1"), version1, Environment.prod, RegionName.from("us-west")), - new OsVersion.Node(HostName.from("node2"), version1, Environment.prod, RegionName.from("us-east")) - )), - new OsVersion(version2, Arrays.asList( - new OsVersion.Node(HostName.from("node3"), version2, Environment.prod, RegionName.from("us-west")), - new OsVersion.Node(HostName.from("node4"), version2, Environment.prod, RegionName.from("us-east")) - )) - ); - - OsVersionStatusSerializer serializer = new OsVersionStatusSerializer(); - OsVersionStatus status = new OsVersionStatus(osVersions); - OsVersionStatus serialized = serializer.fromSlime(serializer.toSlime(status)); + Map<OsVersion, List<OsVersionStatus.Node>> versions = new TreeMap<>(); + + versions.put(new OsVersion(version1, CloudName.defaultName()), Arrays.asList( + new OsVersionStatus.Node(HostName.from("node1"), version1, Environment.prod, RegionName.from("us-west")), + new OsVersionStatus.Node(HostName.from("node2"), version1, Environment.prod, RegionName.from("us-east")) + )); + versions.put(new OsVersion(version2, CloudName.defaultName()), Arrays.asList( + new OsVersionStatus.Node(HostName.from("node3"), version2, Environment.prod, RegionName.from("us-west")), + new OsVersionStatus.Node(HostName.from("node4"), version2, Environment.prod, RegionName.from("us-east")) - for (int i = 0; i < status.versions().size(); i++) { - OsVersion a = status.versions().get(i); - OsVersion b = serialized.versions().get(i); - assertEquals(a.version(), b.version()); - assertEquals(a.nodes(), b.nodes()); - } + )); + + OsVersionStatusSerializer serializer = new OsVersionStatusSerializer(new OsVersionSerializer()); + OsVersionStatus status = new OsVersionStatus(versions); + OsVersionStatus serialized = serializer.fromSlime(serializer.toSlime(status)); + assertEquals(status.versions(), serialized.versions()); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json index 5449e6387a9..2b847010482 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json @@ -10,6 +10,9 @@ "name": "ClusterUtilizationMaintainer" }, { + "name": "DefaultOsUpgrader" + }, + { "name": "DeploymentExpirer" }, { @@ -28,9 +31,6 @@ "name": "MetricsReporter" }, { - "name": "OsUpgrader" - }, - { "name": "OsVersionStatusUpdater" }, { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java index 9a94946981f..1fe2719d2a0 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java @@ -159,7 +159,7 @@ public class StorageMaintainer { private SecretAgentCheckConfig annotatedCheck(NodeSpec node, SecretAgentCheckConfig check) { check.withTag("namespace", "Vespa") - .withTag("role", "tenants") + .withTag("role", SecretAgentCheckConfig.nodeTypeToRole(node.getNodeType())) .withTag("flavor", node.getFlavor()) .withTag("canonicalFlavor", node.getCanonicalFlavor()) .withTag("state", node.getState().toString()) diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/SecretAgentCheckConfig.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/SecretAgentCheckConfig.java index 50e325c2149..09c6891612e 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/SecretAgentCheckConfig.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/SecretAgentCheckConfig.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.util; +import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.node.admin.task.util.file.FileWriter; import java.io.IOException; @@ -74,4 +75,17 @@ public class SecretAgentCheckConfig { return stringBuilder.toString(); } + + // TODO: Change role dimension to nodeType? + public static String nodeTypeToRole(NodeType nodeType) { + switch (nodeType) { + case tenant: return "tenants"; + case host: return "docker"; + case proxy: return "routing"; + case proxyhost: return "routinghost"; + case config: return "configserver"; + case confighost: return "configserverhost"; + default: throw new IllegalArgumentException("Unknown node type " + nodeType); + } + } } 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/tests/proton/attribute/attribute_initializer/attribute_initializer_test.cpp b/searchcore/src/tests/proton/attribute/attribute_initializer/attribute_initializer_test.cpp index 4957d3aead2..b28e04f659f 100644 --- a/searchcore/src/tests/proton/attribute/attribute_initializer/attribute_initializer_test.cpp +++ b/searchcore/src/tests/proton/attribute/attribute_initializer/attribute_initializer_test.cpp @@ -8,7 +8,6 @@ #include <vespa/searchcore/proton/test/attribute_utils.h> #include <vespa/searchlib/attribute/attributefactory.h> #include <vespa/searchlib/test/directory_handler.h> -#include <vespa/vespalib/stllike/string.h> #include <vespa/log/log.h> LOG_SETUP("attribute_initializer_test"); @@ -88,7 +87,7 @@ Fixture::Fixture() { } -Fixture::~Fixture() {} +Fixture::~Fixture() = default; std::unique_ptr<AttributeInitializer> Fixture::createInitializer(const AttributeSpec &spec, SerialNum serialNum) diff --git a/searchcore/src/tests/proton/attribute/exclusive_attribute_read_accessor/exclusive_attribute_read_accessor_test.cpp b/searchcore/src/tests/proton/attribute/exclusive_attribute_read_accessor/exclusive_attribute_read_accessor_test.cpp index c5ae0f97875..420e18db5af 100644 --- a/searchcore/src/tests/proton/attribute/exclusive_attribute_read_accessor/exclusive_attribute_read_accessor_test.cpp +++ b/searchcore/src/tests/proton/attribute/exclusive_attribute_read_accessor/exclusive_attribute_read_accessor_test.cpp @@ -4,6 +4,7 @@ #include <vespa/searchcore/proton/attribute/exclusive_attribute_read_accessor.h> #include <vespa/searchcommon/attribute/config.h> #include <vespa/searchlib/attribute/attributefactory.h> +#include <vespa/searchlib/attribute/attributevector.h> #include <vespa/searchlib/common/sequencedtaskexecutor.h> #include <vespa/vespalib/util/gate.h> diff --git a/searchcore/src/tests/proton/reprocessing/attribute_reprocessing_initializer/attribute_reprocessing_initializer_test.cpp b/searchcore/src/tests/proton/reprocessing/attribute_reprocessing_initializer/attribute_reprocessing_initializer_test.cpp index 72e558fd25f..219c2c42bd7 100644 --- a/searchcore/src/tests/proton/reprocessing/attribute_reprocessing_initializer/attribute_reprocessing_initializer_test.cpp +++ b/searchcore/src/tests/proton/reprocessing/attribute_reprocessing_initializer/attribute_reprocessing_initializer_test.cpp @@ -1,6 +1,4 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/log/log.h> -LOG_SETUP("attribute_reprocessing_initializer_test"); #include <vespa/searchcore/proton/attribute/attribute_directory.h> #include <vespa/searchcore/proton/attribute/attribute_populator.h> @@ -19,6 +17,9 @@ LOG_SETUP("attribute_reprocessing_initializer_test"); #include <vespa/vespalib/test/insertion_operators.h> #include <vespa/vespalib/testkit/testapp.h> +#include <vespa/log/log.h> +LOG_SETUP("attribute_reprocessing_initializer_test"); + using namespace proton; using namespace search; using namespace search::index; @@ -91,7 +92,7 @@ MyConfig::MyConfig() _attributeFieldWriter, _hwInfo)), _schema() {} -MyConfig::~MyConfig() {} +MyConfig::~MyConfig() = default; struct MyDocTypeInspector : public IDocumentTypeInspector { @@ -162,17 +163,13 @@ struct Fixture "test", INIT_SERIAL_NUM)); _initializer->initialize(_handler); } - Fixture &addOldConfig(const StringVector &fields, - const StringVector &attrs) { + Fixture &addOldConfig(const StringVector &fields, const StringVector &attrs) { return addConfig(fields, attrs, _oldCfg); } - Fixture &addNewConfig(const StringVector &fields, - const StringVector &attrs) { + Fixture &addNewConfig(const StringVector &fields, const StringVector &attrs) { return addConfig(fields, attrs, _newCfg); } - Fixture &addConfig(const StringVector &fields, - const StringVector &attrs, - MyConfig &cfg) { + Fixture &addConfig(const StringVector &fields, const StringVector &attrs, MyConfig &cfg) { cfg.addFields(fields); cfg.addAttrs(attrs); return *this; @@ -181,10 +178,8 @@ struct Fixture if (expAttrs.empty()) { if (!EXPECT_TRUE(_handler._reader.get() == nullptr)) return false; } else { - const AttributePopulator &populator = - dynamic_cast<const AttributePopulator &>(*_handler._reader); - std::vector<search::AttributeVector *> attrList = - populator.getWriter().getWritableAttributes(); + const auto & populator = dynamic_cast<const AttributePopulator &>(*_handler._reader); + std::vector<search::AttributeVector *> attrList = populator.getWriter().getWritableAttributes(); std::set<vespalib::string> actAttrs; for (const auto attr : attrList) { actAttrs.insert(attr->getName()); @@ -199,8 +194,7 @@ struct Fixture } else { StringSet actFields; for (auto rewriter : _handler._rewriters) { - const DocumentFieldPopulator &populator = - dynamic_cast<const DocumentFieldPopulator &>(*rewriter); + const auto & populator = dynamic_cast<const DocumentFieldPopulator &>(*rewriter); actFields.insert(populator.getAttribute().getName()); } if (!EXPECT_EQUAL(expFields, actFields)) return false; @@ -273,16 +267,14 @@ TEST_F("require that initializer can setup both attribute and document field pop TEST_F("require that tensor fields are not populated from attribute", Fixture) { - f.addOldConfig({"a", "b", "c", "d", "tensor"}, - {"a", "b", "c", "d", "tensor"}). + f.addOldConfig({"a", "b", "c", "d", "tensor"}, {"a", "b", "c", "d", "tensor"}). addNewConfig({"a", "b", "c", "d", "tensor"}, {"a", "b"}).init(); EXPECT_TRUE(f.assertFields({"c", "d"})); } TEST_F("require that predicate fields are not populated from attribute", Fixture) { - f.addOldConfig({"a", "b", "c", "d", "predicate"}, - {"a", "b", "c", "d", "predicate"}). + f.addOldConfig({"a", "b", "c", "d", "predicate"}, {"a", "b", "c", "d", "predicate"}). addNewConfig({"a", "b", "c", "d", "predicate"}, {"a", "b"}).init(); EXPECT_TRUE(f.assertFields({"c", "d"})); } 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/attribute/attribute_directory.h b/searchcore/src/vespa/searchcore/proton/attribute/attribute_directory.h index 6127e12b94a..603727dbb75 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_directory.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_directory.h @@ -26,12 +26,12 @@ public: private: std::weak_ptr<AttributeDiskLayout> _diskLayout; - const vespalib::string _name; - fastos::TimeStamp _lastFlushTime; - Writer *_writer; // current writer - mutable std::mutex _mutex; + const vespalib::string _name; + fastos::TimeStamp _lastFlushTime; + Writer *_writer; // current writer + mutable std::mutex _mutex; std::condition_variable _cv; - search::IndexMetaInfo _snapInfo; + search::IndexMetaInfo _snapInfo; void saveSnapInfo(); vespalib::string getSnapshotDir(SerialNum serialNum); @@ -50,6 +50,8 @@ public: const vespalib::string &name); ~AttributeDirectory(); + const vespalib::string & getAttrName() const { return _name; } + /* * Class to make changes to an attribute directory in a * controlled manner. An exclusive lock is held during lifetime to diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_factory.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_factory.cpp index 063886ab64f..7d91538f657 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_factory.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_factory.cpp @@ -2,18 +2,17 @@ #include "attribute_factory.h" #include <vespa/searchlib/attribute/attributefactory.h> +#include <vespa/searchlib/attribute/attributevector.h> + namespace proton { using search::AttributeVector; -AttributeFactory::AttributeFactory() -{ -} +AttributeFactory::AttributeFactory() = default; AttributeVector::SP -AttributeFactory::create(const vespalib::string &name, - const search::attribute::Config &cfg) const +AttributeFactory::create(const vespalib::string &name, const search::attribute::Config &cfg) const { AttributeVector::SP v(search::AttributeFactory::createAttribute(name, cfg)); v->enableEnumeratedSave(true); @@ -21,8 +20,7 @@ AttributeFactory::create(const vespalib::string &name, } void -AttributeFactory::setupEmpty(const AttributeVector::SP &vec, - search::SerialNum serialNum) const +AttributeFactory::setupEmpty(const AttributeVector::SP &vec, search::SerialNum serialNum) const { vec->setCreateSerialNum(serialNum); vec->addReservedDoc(); diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_factory.h b/searchcore/src/vespa/searchcore/proton/attribute/attribute_factory.h index e25715eaa14..21abad1a2a6 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_factory.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_factory.h @@ -15,13 +15,8 @@ public: typedef std::shared_ptr<AttributeFactory> SP; AttributeFactory(); - // Implements IAttributeFactory - virtual AttributeVectorSP create(const vespalib::string &name, - const search::attribute::Config &cfg) const override; - - virtual void setupEmpty(const AttributeVectorSP &vec, - search::SerialNum serialNum) const override; + AttributeVectorSP create(const vespalib::string &name, const search::attribute::Config &cfg) const override; + void setupEmpty(const AttributeVectorSP &vec, search::SerialNum serialNum) const override; }; -} // namespace proton - +} diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_initializer.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_initializer.cpp index a98becb7af7..d8970d79ef2 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_initializer.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_initializer.cpp @@ -121,30 +121,21 @@ extractHeader(const vespalib::string &attrFileName) } void -logAttributeTooNew(const AttributeVector::SP &attr, - const AttributeHeader &header, - uint64_t serialNum) +logAttributeTooNew(const AttributeHeader &header, uint64_t serialNum) { LOG(info, "Attribute vector '%s' is too new (%" PRIu64 " > %" PRIu64 ")", - attr->getBaseFileName().c_str(), - header.getCreateSerialNum(), - serialNum); + header.getFileName().c_str(), header.getCreateSerialNum(), serialNum); } void -logAttributeTooOld(const AttributeVector::SP &attr, - search::SerialNum flushedSerialNum, - uint64_t serialNum) +logAttributeTooOld(const AttributeHeader &header, uint64_t flushedSerialNum, uint64_t serialNum) { LOG(info, "Attribute vector '%s' is too old (%" PRIu64 " < %" PRIu64 ")", - attr->getBaseFileName().c_str(), - flushedSerialNum, - serialNum); + header.getFileName().c_str(), flushedSerialNum, serialNum); } void -logAttributeWrongType(const AttributeVector::SP &attr, - const AttributeHeader &header) +logAttributeWrongType(const AttributeVector::SP &attr, const AttributeHeader &header) { const Config &cfg(attr->getConfig()); vespalib::string extraCfgType = extraType(cfg); @@ -152,13 +143,8 @@ logAttributeWrongType(const AttributeVector::SP &attr, vespalib::string cfgCollStr = collectionTypeString(cfg.collectionType(), true); vespalib::string headerCollStr = collectionTypeString(header.getCollectionType(), header.getCollectionTypeParamsSet()); LOG(info, "Attribute vector '%s' is of wrong type (expected %s/%s/%s, got %s/%s/%s)", - attr->getBaseFileName().c_str(), - cfg.basicType().asString(), - cfgCollStr.c_str(), - extraCfgType.c_str(), - header.getBasicType().asString(), - headerCollStr.c_str(), - extraHeaderType.c_str()); + header.getFileName().c_str(), cfg.basicType().asString(), cfgCollStr.c_str(), extraCfgType.c_str(), + header.getBasicType().asString(), headerCollStr.c_str(), extraHeaderType.c_str()); } } @@ -192,9 +178,8 @@ AttributeInitializer::loadAttribute(const AttributeVectorSP &attr, fastos::TimeStamp startTime = fastos::ClockSystem::now(); EventLogger::loadAttributeStart(_documentSubDbName, attr->getName()); if (!attr->load()) { - LOG(warning, "Could not load attribute vector '%s' from disk. " - "Returning empty attribute vector", - attr->getBaseFileName().c_str()); + LOG(warning, "Could not load attribute vector '%s' from disk. Returning empty attribute vector", + attr->getBaseFileName().c_str()); return false; } else { attr->commit(serialNum, serialNum); @@ -206,21 +191,19 @@ AttributeInitializer::loadAttribute(const AttributeVectorSP &attr, } void -AttributeInitializer::setupEmptyAttribute(AttributeVectorSP &attr, - search::SerialNum serialNum, +AttributeInitializer::setupEmptyAttribute(AttributeVectorSP &attr, search::SerialNum serialNum, const AttributeHeader &header) const { if (header.getCreateSerialNum() > _currentSerialNum) { - logAttributeTooNew(attr, header, _currentSerialNum); + logAttributeTooNew(header, _currentSerialNum); } if (serialNum < _currentSerialNum) { - logAttributeTooOld(attr, serialNum, _currentSerialNum); + logAttributeTooOld(header, serialNum, _currentSerialNum); } if (!headerTypeOK(header, attr->getConfig())) { logAttributeWrongType(attr, header); } - LOG(info, "Returning empty attribute vector for '%s'", - attr->getBaseFileName().c_str()); + LOG(info, "Returning empty attribute vector for '%s'", attr->getBaseFileName().c_str()); _factory.setupEmpty(attr, _currentSerialNum); attr->commit(serialNum, serialNum); } @@ -228,8 +211,7 @@ AttributeInitializer::setupEmptyAttribute(AttributeVectorSP &attr, AttributeVector::SP AttributeInitializer::createAndSetupEmptyAttribute() const { - vespalib::string attrFileName = _attrDir->getAttributeFileName(0); - AttributeVector::SP attr = _factory.create(attrFileName, _spec.getConfig()); + AttributeVector::SP attr = _factory.create(_attrDir->getAttrName(), _spec.getConfig()); _factory.setupEmpty(attr, _currentSerialNum); return attr; } @@ -247,7 +229,7 @@ AttributeInitializer::AttributeInitializer(const std::shared_ptr<AttributeDirect { } -AttributeInitializer::~AttributeInitializer() {} +AttributeInitializer::~AttributeInitializer() = default; AttributeInitializerResult AttributeInitializer::init() const diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_initializer.h b/searchcore/src/vespa/searchcore/proton/attribute/attribute_initializer.h index 5c03a657f4f..c2ea83812cb 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_initializer.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_initializer.h @@ -8,9 +8,7 @@ #include <vespa/searchlib/common/serialnum.h> #include <vespa/searchcommon/attribute/persistent_predicate_params.h> -namespace search { -namespace attribute { class AttributeHeader; } -} +namespace search::attribute { class AttributeHeader; } namespace proton { @@ -35,21 +33,16 @@ private: AttributeVectorSP tryLoadAttribute() const; - bool loadAttribute(const AttributeVectorSP &attr, - search::SerialNum serialNum) const; + bool loadAttribute(const AttributeVectorSP &attr, search::SerialNum serialNum) const; - void setupEmptyAttribute(AttributeVectorSP &attr, - search::SerialNum serialNum, + void setupEmptyAttribute(AttributeVectorSP &attr, search::SerialNum serialNum, const search::attribute::AttributeHeader &header) const; AttributeVectorSP createAndSetupEmptyAttribute() const; public: - AttributeInitializer(const std::shared_ptr<AttributeDirectory> &attrDir, - const vespalib::string &documentSubDbName, - const AttributeSpec &spec, - uint64_t currentSerialNum, - const IAttributeFactory &factory); + AttributeInitializer(const std::shared_ptr<AttributeDirectory> &attrDir, const vespalib::string &documentSubDbName, + const AttributeSpec &spec, uint64_t currentSerialNum, const IAttributeFactory &factory); ~AttributeInitializer(); AttributeInitializerResult init() const; diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_spec.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_spec.cpp index 45b800c64cf..20e6798c79b 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_spec.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_spec.cpp @@ -21,7 +21,7 @@ AttributeSpec::AttributeSpec(AttributeSpec &&) = default; AttributeSpec & AttributeSpec::operator=(AttributeSpec &&) = default; -AttributeSpec::~AttributeSpec() { } +AttributeSpec::~AttributeSpec() = default; bool AttributeSpec::operator==(const AttributeSpec &rhs) const diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attributedisklayout.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attributedisklayout.cpp index a675927b85f..5f057bbd7dc 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attributedisklayout.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attributedisklayout.cpp @@ -17,9 +17,7 @@ AttributeDiskLayout::AttributeDiskLayout(const vespalib::string &baseDir, Privat vespalib::File::sync(vespalib::dirname(_baseDir)); } -AttributeDiskLayout::~AttributeDiskLayout() -{ -} +AttributeDiskLayout::~AttributeDiskLayout() = default; std::vector<vespalib::string> AttributeDiskLayout::listAttributes() diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp index abe4863955f..87248e060c9 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp @@ -145,7 +145,7 @@ AttributeManager::internalAddAttribute(const AttributeSpec &spec, void AttributeManager::addAttribute(const AttributeWrap &attribute, const ShrinkerSP &shrinker) { - LOG(debug, "Adding attribute vector '%s'", attribute.getAttribute()->getBaseFileName().c_str()); + LOG(debug, "Adding attribute vector '%s'", attribute.getAttribute()->getName().c_str()); _attributes[attribute.getAttribute()->getName()] = attribute; assert(attribute.getAttribute()->getInterlock() == _interlock); if ( ! attribute.isExtra() ) { diff --git a/searchcore/src/vespa/searchcore/proton/attribute/flushableattribute.cpp b/searchcore/src/vespa/searchcore/proton/attribute/flushableattribute.cpp index da6700739b6..1d4133c8162 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/flushableattribute.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/flushableattribute.cpp @@ -2,20 +2,20 @@ #include "attributedisklayout.h" #include "flushableattribute.h" +#include "attribute_directory.h" #include <vespa/searchlib/attribute/attributefilesavetarget.h> #include <vespa/searchlib/attribute/attributesaver.h> #include <vespa/searchlib/util/dirtraverse.h> #include <vespa/searchlib/util/filekit.h> #include <vespa/vespalib/io/fileutil.h> #include <vespa/vespalib/util/closuretask.h> -#include <fstream> #include <vespa/searchlib/common/serialnumfileheadercontext.h> #include <vespa/searchlib/common/isequencedtaskexecutor.h> #include <vespa/searchlib/attribute/attributememorysavetarget.h> #include <vespa/searchlib/attribute/attributevector.h> -#include <future> -#include "attribute_directory.h" #include <vespa/vespalib/util/stringfmt.h> +#include <fstream> +#include <future> #include <vespa/log/log.h> LOG_SETUP(".proton.attribute.flushableattribute"); @@ -166,10 +166,7 @@ FlushableAttribute::FlushableAttribute(const AttributeVectorSP attr, search::ISequencedTaskExecutor & attributeFieldWriter, const HwInfo &hwInfo) - : IFlushTarget(vespalib::make_string( - "attribute.flush.%s", - attr->getName().c_str()), - Type::SYNC, Component::ATTRIBUTE), + : IFlushTarget(make_string("attribute.flush.%s", attr->getName().c_str()), Type::SYNC, Component::ATTRIBUTE), _attr(attr), _cleanUpAfterFlush(true), _lastStats(), @@ -183,10 +180,7 @@ FlushableAttribute::FlushableAttribute(const AttributeVectorSP attr, } -FlushableAttribute::~FlushableAttribute() -{ -} - +FlushableAttribute::~FlushableAttribute() = default; IFlushTarget::SerialNum FlushableAttribute::getFlushedSerialNum() const diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp index 4be05d7d64a..c03c93b5fd9 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp @@ -301,10 +301,8 @@ DocumentMetaStore::onLoad() } bool -DocumentMetaStore::checkBuckets(const GlobalId &gid, - const BucketId &bucketId, - const TreeType::Iterator &itr, - bool found) +DocumentMetaStore::checkBuckets(const GlobalId &gid, const BucketId &bucketId, + const TreeType::Iterator &itr, bool found) { bool success = true; #if 0 @@ -394,8 +392,7 @@ DocumentMetaStore::updateMetaDataAndBucketDB(const GlobalId &gid, } -namespace -{ +namespace { void unloadBucket(BucketDBOwner &db, const BucketId &id, const BucketState &delta) diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h index d8384de3596..2fd3bcdae64 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h @@ -17,17 +17,15 @@ #include <vespa/searchlib/queryeval/blueprint.h> #include <vespa/searchlib/docstore/ibucketizer.h> -namespace proton { - -namespace bucketdb { - -class SplitBucketSession; -class JoinBucketsSession; - +namespace proton::bucketdb { + class SplitBucketSession; + class JoinBucketsSession; } -namespace documentmetastore { class Reader; } +namespace proton::documentmetastore { class Reader; } +namespace proton { + /** * This class provides a storage of <lid, meta data> pairs (local * document id, meta data (including global document id)) and mapping @@ -63,10 +61,8 @@ private: // Lids are stored as keys in the tree, sorted by their gid // counterpart. The LidGidKeyComparator class maps from lids -> metadata by // using the metadata store. - typedef search::btree::BTree<DocId, - search::btree::BTreeNoLeafData, - search::btree::NoAggregated, - const KeyComp &> TreeType; + typedef search::btree::BTree<DocId, search::btree::BTreeNoLeafData, + search::btree::NoAggregated, const KeyComp &> TreeType; MetaDataStore _metaDataStore; TreeType _gidToLidMap; @@ -164,14 +160,9 @@ public: * map is then re-built the same way it was originally where add() * was used to create the <lid, gid> pairs. **/ - Result put(const GlobalId &gid, - const BucketId &bucketId, - const Timestamp ×tamp, - uint32_t docSize, - DocId lid) override; - bool updateMetaData(DocId lid, - const BucketId &bucketId, - const Timestamp ×tamp) override; + Result put(const GlobalId &gid, const BucketId &bucketId, + const Timestamp ×tamp, uint32_t docSize, DocId lid) override; + bool updateMetaData(DocId lid, const BucketId &bucketId, const Timestamp ×tamp) override; bool remove(DocId lid) override; BucketId getBucketOf(const vespalib::GenerationHandler::Guard & guard, uint32_t lid) const override; @@ -213,8 +204,7 @@ public: */ SearchContext::UP getSearch(std::unique_ptr<search::QueryTermSimple> qTerm, - const search::attribute::SearchContextParams ¶ms) - const override; + const search::attribute::SearchContextParams ¶ms) const override; /** * Implements proton::IDocumentMetaStore @@ -270,11 +260,11 @@ public: */ void unblockShrinkLidSpace(); void onShrinkLidSpace() override; - virtual size_t getEstimatedShrinkLidSpaceGain() const override; + size_t getEstimatedShrinkLidSpaceGain() const override; uint64_t getEstimatedSaveByteSize() const override; - virtual uint32_t getVersion() const override; + uint32_t getVersion() const override; void setTrackDocumentSizes(bool trackDocumentSizes) { _trackDocumentSizes = trackDocumentSizes; } - virtual void foreach(const search::IGidToLidMapperVisitor &visitor) const override; + void foreach(const search::IGidToLidMapperVisitor &visitor) const override; }; } diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreflushtarget.cpp b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreflushtarget.cpp index 241a2449e17..56ecdaa0cb0 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreflushtarget.cpp +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreflushtarget.cpp @@ -48,14 +48,9 @@ public: bool flush(AttributeDirectory::Writer &writer); void updateStats(); bool cleanUp(AttributeDirectory::Writer &writer); - // Implements vespalib::Executor::Task - virtual void run() override; + void run() override; - virtual SerialNum - getFlushSerial() const override - { - return _syncToken; - } + SerialNum getFlushSerial() const override { return _syncToken; } }; DocumentMetaStoreFlushTarget::Flusher:: @@ -163,12 +158,9 @@ DocumentMetaStoreFlushTarget::Flusher::run() } DocumentMetaStoreFlushTarget:: -DocumentMetaStoreFlushTarget(const DocumentMetaStore::SP dms, - ITlsSyncer &tlsSyncer, - const vespalib::string & baseDir, - const TuneFileAttributes &tuneFileAttributes, - const FileHeaderContext &fileHeaderContext, - const HwInfo &hwInfo) +DocumentMetaStoreFlushTarget(const DocumentMetaStore::SP dms, ITlsSyncer &tlsSyncer, + const vespalib::string & baseDir, const TuneFileAttributes &tuneFileAttributes, + const FileHeaderContext &fileHeaderContext, const HwInfo &hwInfo) : IFlushTarget("documentmetastore.flush", Type::SYNC, Component::ATTRIBUTE), _dms(dms), _tlsSyncer(tlsSyncer), @@ -186,9 +178,7 @@ DocumentMetaStoreFlushTarget(const DocumentMetaStore::SP dms, } -DocumentMetaStoreFlushTarget::~DocumentMetaStoreFlushTarget() -{ -} +DocumentMetaStoreFlushTarget::~DocumentMetaStoreFlushTarget() = default; IFlushTarget::SerialNum @@ -225,21 +215,18 @@ DocumentMetaStoreFlushTarget::initFlush(SerialNum currentSerial) { // Called by document db executor _dms->removeAllOldGenerations(); - SerialNum syncToken = std::max(currentSerial, - _dms->getStatus().getLastSyncToken()); + SerialNum syncToken = std::max(currentSerial, _dms->getStatus().getLastSyncToken()); auto writer = _dmsDir->tryGetWriter(); if (!writer) { return Task::UP(); } if (syncToken <= getFlushedSerialNum()) { writer->setLastFlushTime(fastos::ClockSystem::now()); - LOG(debug, - "No document meta store to flush." - " Update flush time to current: lastFlushTime(%f)", + LOG(debug, "No document meta store to flush. Update flush time to current: lastFlushTime(%f)", getLastFlushTime().sec()); return Task::UP(); } - return Task::UP(new Flusher(*this, syncToken, *writer)); + return std::make_unique<Flusher>(*this, syncToken, *writer); } @@ -249,5 +236,4 @@ DocumentMetaStoreFlushTarget::getApproxBytesToWriteToDisk() const return _dms->getEstimatedSaveByteSize(); } - } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreflushtarget.h b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreflushtarget.h index fb2ce5845a7..5cc674d3614 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreflushtarget.h +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreflushtarget.h @@ -6,20 +6,9 @@ #include <vespa/searchlib/common/tunefileinfo.h> #include <vespa/searchcore/proton/common/hw_info.h> -namespace search -{ - -namespace common -{ - -class FileHeaderContext; +namespace search::common { class FileHeaderContext; } -} - -} - -namespace proton -{ +namespace proton { class ITlsSyncer; class AttributeDiskLayout; @@ -57,30 +46,23 @@ public: * Creates a new instance using the given attribute vector and the * given base dir where all attribute vectors are located. **/ - DocumentMetaStoreFlushTarget(const DocumentMetaStoreSP dms, - ITlsSyncer &tlsSyncer, - const vespalib::string &baseDir, - const search::TuneFileAttributes & - tuneFileAttributes, - const search::common::FileHeaderContext & - fileHeaderContext, - const HwInfo &hwInfo); + DocumentMetaStoreFlushTarget(const DocumentMetaStoreSP dms, ITlsSyncer &tlsSyncer, + const vespalib::string &baseDir, const search::TuneFileAttributes &tuneFileAttributes, + const search::common::FileHeaderContext &fileHeaderContext, const HwInfo &hwInfo); - virtual - ~DocumentMetaStoreFlushTarget(); + ~DocumentMetaStoreFlushTarget() override; void setCleanUpAfterFlush(bool cleanUp) { _cleanUpAfterFlush = cleanUp; } - // Implements IFlushTarget - virtual MemoryGain getApproxMemoryGain() const override; - virtual DiskGain getApproxDiskGain() const override; - virtual Time getLastFlushTime() const override; - virtual SerialNum getFlushedSerialNum() const override; - virtual Task::UP initFlush(SerialNum currentSerial) override; - virtual FlushStats getLastFlushStats() const override { return _lastStats; } + MemoryGain getApproxMemoryGain() const override; + DiskGain getApproxDiskGain() const override; + Time getLastFlushTime() const override; + SerialNum getFlushedSerialNum() const override; + Task::UP initFlush(SerialNum currentSerial) override; + FlushStats getLastFlushStats() const override { return _lastStats; } static void initCleanup(const vespalib::string &baseDir); - virtual uint64_t getApproxBytesToWriteToDisk() const override; + uint64_t getApproxBytesToWriteToDisk() const override; }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreinitializer.cpp b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreinitializer.cpp index 53f6a047fc9..5703f03b32e 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreinitializer.cpp +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreinitializer.cpp @@ -13,8 +13,7 @@ using vespalib::IllegalStateException; using proton::initializer::InitializerTask; using vespalib::make_string; -namespace proton { -namespace documentmetastore { +namespace proton::documentmetastore { DocumentMetaStoreInitializer:: DocumentMetaStoreInitializer(const vespalib::string baseDir, @@ -61,7 +60,4 @@ DocumentMetaStoreInitializer::run() } } - -} // namespace proton::documentmetastore - -} // namespace proton +} diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreinitializer.h b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreinitializer.h index 86f05866c72..6f313645cb2 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreinitializer.h +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreinitializer.h @@ -8,11 +8,7 @@ #include <vespa/searchcommon/common/growstrategy.h> #include <vespa/vespalib/stllike/string.h> -namespace proton -{ - -namespace documentmetastore -{ +namespace proton::documentmetastore { /* * Class representing an Initializer task for loading document meta store @@ -33,10 +29,8 @@ public: const vespalib::string &subDbName, const vespalib::string &docTypeName, DocumentMetaStore::SP dms); - virtual void run() override; + void run() override; }; -} // namespace proton::documentmetastore - -} // namespace proton +} diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp index 2d3c204d259..acdda10a7ef 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp @@ -62,13 +62,10 @@ IIndexWriter::SP nullIndexWriter; } -StoreOnlyDocSubDB::Config::Config(const DocTypeName &docTypeName, - const vespalib::string &subName, +StoreOnlyDocSubDB::Config::Config(const DocTypeName &docTypeName, const vespalib::string &subName, const vespalib::string &baseDir, - const search::GrowStrategy &attributeGrow, - size_t attributeGrowNumDocs, - uint32_t subDbId, - SubDbType subDbType) + const search::GrowStrategy &attributeGrow, size_t attributeGrowNumDocs, + uint32_t subDbId, SubDbType subDbType) : _docTypeName(docTypeName), _subName(subName), _baseDir(baseDir + "/" + subName), @@ -77,7 +74,7 @@ StoreOnlyDocSubDB::Config::Config(const DocTypeName &docTypeName, _subDbId(subDbId), _subDbType(subDbType) { } -StoreOnlyDocSubDB::Config::~Config() { } +StoreOnlyDocSubDB::Config::~Config() = default; StoreOnlyDocSubDB::Context::Context(IDocumentSubDBOwner &owner, search::transactionlog::SyncProxy &tlSyncer, @@ -102,7 +99,7 @@ StoreOnlyDocSubDB::Context::Context(IDocumentSubDBOwner &owner, _configMutex(configMutex), _hwInfo(hwInfo) { } -StoreOnlyDocSubDB::Context::~Context() { } +StoreOnlyDocSubDB::Context::~Context() = default; StoreOnlyDocSubDB::StoreOnlyDocSubDB(const Config &cfg, const Context &ctx) : DocSubDB(ctx._owner, ctx._tlSyncer), @@ -161,11 +158,7 @@ StoreOnlyDocSubDB::clearViews() { size_t StoreOnlyDocSubDB::getNumDocs() const { - if (_metaStoreCtx) { - return _metaStoreCtx->get().getNumUsedLids(); - } else { - return 0u; - } + return (_metaStoreCtx) ? _metaStoreCtx->get().getNumUsedLids() : 0u; } size_t @@ -209,9 +202,8 @@ StoreOnlyDocSubDB::onReplayDone() void -StoreOnlyDocSubDB::onReprocessDone(SerialNum serialNum) +StoreOnlyDocSubDB::onReprocessDone(SerialNum) { - (void) serialNum; _commitTimeTracker.setReplayDone(); } @@ -292,9 +284,8 @@ StoreOnlyDocSubDB::setupDocumentMetaStore(DocumentMetaStoreInitializerResult::SP if (dms->isLoaded()) { _flushedDocumentMetaStoreSerialNum = dms->getStatus().getLastSyncToken(); } - _bucketDBHandlerInitializer. - addDocumentMetaStore(dms.get(), _flushedDocumentMetaStoreSerialNum); - _metaStoreCtx.reset(new DocumentMetaStoreContext(dms)); + _bucketDBHandlerInitializer.addDocumentMetaStore(dms.get(), _flushedDocumentMetaStoreSerialNum); + _metaStoreCtx = std::make_shared<DocumentMetaStoreContext>(dms); LOG(debug, "Added document meta store '%s' with flushed serial num %lu", name.c_str(), _flushedDocumentMetaStoreSerialNum); _dms = dms; @@ -313,18 +304,15 @@ StoreOnlyDocSubDB::createInitializer(const DocumentDBConfig &configSnapshot, Ser { (void) configSerialNum; (void) indexCfg; - auto result = std::make_unique<DocumentSubDbInitializer> - (const_cast<StoreOnlyDocSubDB &>(*this), - _writeService.master()); - auto dmsInitTask = - createDocumentMetaStoreInitializer(configSnapshot.getTuneFileDocumentDBSP()->_attr, - result->writableResult().writableDocumentMetaStore()); + auto result = std::make_unique<DocumentSubDbInitializer>(const_cast<StoreOnlyDocSubDB &>(*this), + _writeService.master()); + auto dmsInitTask = createDocumentMetaStoreInitializer(configSnapshot.getTuneFileDocumentDBSP()->_attr, + result->writableResult().writableDocumentMetaStore()); result->addDocumentMetaStoreInitTask(dmsInitTask); - auto summaryTask = - createSummaryManagerInitializer(configSnapshot.getStoreConfig(), - configSnapshot.getTuneFileDocumentDBSP()->_summary, - result->result().documentMetaStore()->documentMetaStore(), - result->writableResult().writableSummaryManager()); + auto summaryTask = createSummaryManagerInitializer(configSnapshot.getStoreConfig(), + configSnapshot.getTuneFileDocumentDBSP()->_summary, + result->result().documentMetaStore()->documentMetaStore(), + result->writableResult().writableSummaryManager()); result->addDependency(summaryTask); summaryTask->addDependency(dmsInitTask); @@ -348,8 +336,7 @@ StoreOnlyDocSubDB::getFlushTargets() { IFlushTarget::List ret; for (const auto &target : getFlushTargetsInternal()) { - ret.push_back(IFlushTarget::SP - (new ThreadedFlushTarget(_writeService.master(), _getSerialNum, target, _subName))); + ret.push_back(std::make_shared<ThreadedFlushTarget>(_writeService.master(), _getSerialNum, target, _subName)); } return ret; } @@ -366,13 +353,9 @@ StoreOnlyDocSubDB::getFlushTargetsInternal() StoreOnlyFeedView::Context StoreOnlyDocSubDB::getStoreOnlyFeedViewContext(const DocumentDBConfig &configSnapshot) { - return StoreOnlyFeedView::Context(getSummaryAdapter(), - configSnapshot.getSchemaSP(), - _metaStoreCtx, - *_gidToLidChangeHandler, - configSnapshot.getDocumentTypeRepoSP(), - _writeService, - *_lidReuseDelayer, _commitTimeTracker); + return StoreOnlyFeedView::Context(getSummaryAdapter(), configSnapshot.getSchemaSP(), _metaStoreCtx, + *_gidToLidChangeHandler, configSnapshot.getDocumentTypeRepoSP(), _writeService, + *_lidReuseDelayer, _commitTimeTracker); } StoreOnlyFeedView::PersistentParams @@ -384,8 +367,7 @@ StoreOnlyDocSubDB::getFeedViewPersistentParams() } void -StoreOnlyDocSubDB::initViews(const DocumentDBConfig &configSnapshot, - const SessionManager::SP &sessionManager) +StoreOnlyDocSubDB::initViews(const DocumentDBConfig &configSnapshot, const SessionManager::SP &sessionManager) { assert(_writeService.master().isCurrentThread()); _iSearchView.set(ISearchHandler::SP(new EmptySearchView)); @@ -400,9 +382,8 @@ void StoreOnlyDocSubDB::initFeedView(const DocumentDBConfig &configSnapshot) { assert(_writeService.master().isCurrentThread()); - StoreOnlyFeedView::UP feedView(new StoreOnlyFeedView( - getStoreOnlyFeedViewContext(configSnapshot), - getFeedViewPersistentParams())); + auto feedView = std::make_unique<StoreOnlyFeedView>(getStoreOnlyFeedViewContext(configSnapshot), + getFeedViewPersistentParams()); // XXX: Not exception safe. _iFeedView.set(StoreOnlyFeedView::SP(feedView.release())); @@ -414,8 +395,7 @@ StoreOnlyDocSubDB::getSubDbName() const { } void -StoreOnlyDocSubDB::updateLidReuseDelayer(const DocumentDBConfig * - newConfigSnapshot) +StoreOnlyDocSubDB::updateLidReuseDelayer(const DocumentDBConfig * newConfigSnapshot) { LidReuseDelayerConfig lidReuseDelayerConfig(*newConfigSnapshot); updateLidReuseDelayer(lidReuseDelayerConfig); @@ -485,11 +465,9 @@ StoreOnlyDocSubDB::pruneRemovedFields(SerialNum) } void -StoreOnlyDocSubDB::setIndexSchema(const Schema::SP &schema, SerialNum serialNum) +StoreOnlyDocSubDB::setIndexSchema(const Schema::SP &, SerialNum ) { assert(_writeService.master().isCurrentThread()); - (void) schema; - (void) serialNum; } search::SearchableStats @@ -501,12 +479,9 @@ StoreOnlyDocSubDB::getSearchableStats() const IDocumentRetriever::UP StoreOnlyDocSubDB::getDocumentRetriever() { - return IDocumentRetriever::UP(new MinimalDocumentRetriever( - _docTypeName, - _iFeedView.get()->getDocumentTypeRepo(), - *_metaStoreCtx, - _iSummaryMgr->getBackingStore(), - _subDbType != SubDbType::REMOVED)); + return std::make_unique<MinimalDocumentRetriever>(_docTypeName, _iFeedView.get()->getDocumentTypeRepo(), + *_metaStoreCtx, _iSummaryMgr->getBackingStore(), + _subDbType != SubDbType::REMOVED); } MatchingStats @@ -548,17 +523,13 @@ StoreOnlySubDBFileHeaderContext(StoreOnlyDocSubDB &owner, _subDB() { size_t pos = baseDir.rfind('/'); - if (pos != vespalib::string::npos) - _subDB = baseDir.substr(pos + 1); - else - _subDB = baseDir; + _subDB = (pos != vespalib::string::npos) ? baseDir.substr(pos + 1) : baseDir; } -StoreOnlySubDBFileHeaderContext::~StoreOnlySubDBFileHeaderContext() {} +StoreOnlySubDBFileHeaderContext::~StoreOnlySubDBFileHeaderContext() = default; void -StoreOnlyDocSubDB::tearDownReferences(IDocumentDBReferenceResolver &resolver) +StoreOnlyDocSubDB::tearDownReferences(IDocumentDBReferenceResolver &) { - (void) resolver; } void @@ -571,5 +542,4 @@ addTags(vespalib::GenericHeader &header, const vespalib::string &name) const header.putTag(Tag("subDB", _subDB)); } - } // namespace proton diff --git a/searchlib/src/tests/attribute/attribute_test.cpp b/searchlib/src/tests/attribute/attribute_test.cpp index f44a1c39baf..d4f716acb7a 100644 --- a/searchlib/src/tests/attribute/attribute_test.cpp +++ b/searchlib/src/tests/attribute/attribute_test.cpp @@ -31,19 +31,20 @@ using search::common::FileHeaderContext; using search::index::DummyFileHeaderContext; using search::attribute::BasicType; using search::attribute::IAttributeVector; +using vespalib::stringref; +using vespalib::string; namespace { -vespalib::string empty; -vespalib::string tmpDir("tmp"); -vespalib::string clsDir("clstmp"); -vespalib::string asuDir("asutmp"); +string empty; +string tmpDir("tmp"); +string clsDir("clstmp"); +string asuDir("asutmp"); bool isUnsignedSmallIntAttribute(const BasicType::Type &type) { - switch (type) - { + switch (type) { case BasicType::UINT1: case BasicType::UINT2: case BasicType::UINT4: @@ -68,13 +69,13 @@ expectZero(const BufferType &b) template <> void -expectZero(const vespalib::string &b) +expectZero(const string &b) { EXPECT_EQUAL(empty, b); } uint64_t -statSize(const vespalib::string &fileName) +statSize(const string &fileName) { FastOS_StatInfo statInfo; if (EXPECT_TRUE(FastOS_File::Stat(fileName.c_str(), &statInfo))) { @@ -112,14 +113,14 @@ preciseEstimatedSize(const AttributeVector &a) return true; } -vespalib::string -baseFileName(const vespalib::string &attrName) +string +baseFileName(const string &attrName) { return tmpDir + "/" + attrName; } AttributeVector::SP -createAttribute(const vespalib::string &attrName, const search::attribute::Config &cfg) +createAttribute(stringref attrName, const search::attribute::Config &cfg) { return search::AttributeFactory::createAttribute(baseFileName(attrName), cfg); } @@ -162,7 +163,7 @@ private: template <typename T> void fillNumeric(std::vector<T> & values, uint32_t numValues); - void fillString(std::vector<vespalib::string> & values, uint32_t numValues); + void fillString(std::vector<string> & values, uint32_t numValues); template <typename VectorType, typename BufferType> bool appendToVector(VectorType & v, uint32_t doc, uint32_t valueCount, const std::vector<BufferType> & values); @@ -218,9 +219,7 @@ private: template <typename VectorType, typename BufferType> void - testCompactLidSpace(const Config &config, - bool fs, - bool es); + testCompactLidSpace(const Config &config, bool fs, bool es); template <typename VectorType, typename BufferType> void @@ -427,7 +426,7 @@ void AttributeTest::testReloadString(const AttributePtr & a, const AttributePtr if (a->hasWeightedSetType()) { testReload<StringAttribute, StringAttribute::WeightedString>(a, b, c); } else { - testReload<StringAttribute, vespalib::string>(a, b, c); + testReload<StringAttribute, string>(a, b, c); } } @@ -696,7 +695,7 @@ AttributeTest::testMemorySaverString(const AttributePtr & a, const AttributePtr if (a->hasWeightedSetType()) { testMemorySaver<StringAttribute, StringAttribute::WeightedString>(a, b); } else { - testMemorySaver<StringAttribute, vespalib::string>(a, b); + testMemorySaver<StringAttribute, string>(a, b); } } @@ -762,7 +761,6 @@ AttributeTest::testMemorySaver() } } - template <typename T> void AttributeTest::fillNumeric(std::vector<T> & values, uint32_t numValues) @@ -775,7 +773,7 @@ AttributeTest::fillNumeric(std::vector<T> & values, uint32_t numValues) } void -AttributeTest::fillString(std::vector<vespalib::string> & values, uint32_t numValues) +AttributeTest::fillString(std::vector<string> & values, uint32_t numValues) { values.clear(); values.reserve(numValues); @@ -941,19 +939,19 @@ AttributeTest::testSingle() } { - std::vector<vespalib::string> values; + std::vector<string> values; fillString(values, numUniques); { AttributePtr ptr = createAttribute("sv-string", Config(BasicType::STRING, CollectionType::SINGLE)); addDocs(ptr, numDocs); - testSingle<StringAttribute, vespalib::string, vespalib::string>(ptr, values); + testSingle<StringAttribute, string, string>(ptr, values); } { Config cfg(Config(BasicType::STRING, CollectionType::SINGLE)); cfg.setFastSearch(true); AttributePtr ptr = createAttribute("sv-fs-string", cfg); addDocs(ptr, numDocs); - testSingle<StringAttribute, vespalib::string, vespalib::string>(ptr, values); + testSingle<StringAttribute, string, string>(ptr, values); } } } @@ -1052,7 +1050,6 @@ AttributeTest::testArray(const AttributePtr & ptr, const std::vector<BufferType> } EXPECT_TRUE(!v.remove(ptr->getNumDocs(), values[0], 1)); - // test clearDoc() for (uint32_t doc = 0; doc < ptr->getNumDocs(); ++doc) { uint32_t valueCount = doc % numUniques; @@ -1132,19 +1129,19 @@ AttributeTest::testArray() } } { // StringAttribute - std::vector<vespalib::string> values; + std::vector<string> values; fillString(values, numUniques); { AttributePtr ptr = createAttribute("a-string", Config(BasicType::STRING, CollectionType::ARRAY)); addDocs(ptr, numDocs); - testArray<StringAttribute, vespalib::string>(ptr, values); + testArray<StringAttribute, string>(ptr, values); } { Config cfg(BasicType::STRING, CollectionType::ARRAY); cfg.setFastSearch(true); AttributePtr ptr = createAttribute("afs-string", cfg); addDocs(ptr, numDocs); - testArray<StringAttribute, vespalib::string>(ptr, values); + testArray<StringAttribute, string>(ptr, values); } } } @@ -1266,8 +1263,7 @@ AttributeTest::testWeightedSet() } { - AttributePtr ptr = createAttribute - ("wsint32", Config(BasicType::INT32, CollectionType::WSET)); + AttributePtr ptr = createAttribute("wsint32", Config(BasicType::INT32, CollectionType::WSET)); addDocs(ptr, numDocs); testWeightedSet<IntegerAttribute, AttributeVector::WeightedInt>(ptr, values); } @@ -1319,8 +1315,7 @@ AttributeTest::testWeightedSet() } { - AttributePtr ptr = createAttribute - ("wsstr", Config(BasicType::STRING, CollectionType::WSET)); + AttributePtr ptr = createAttribute("wsstr", Config(BasicType::STRING, CollectionType::WSET)); addDocs(ptr, numDocs); testWeightedSet<StringAttribute, AttributeVector::WeightedString>(ptr, values); } @@ -1571,14 +1566,10 @@ AttributeTest::testMapValueUpdate(const AttributePtr & ptr, BufferType initValue EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 6u); EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 0u); - EXPECT_TRUE(ptr->apply(0, MapVU(initFieldValue, - ArithVU(ArithVU::Add, 10)))); - EXPECT_TRUE(ptr->apply(1, MapVU(initFieldValue, - ArithVU(ArithVU::Sub, 10)))); - EXPECT_TRUE(ptr->apply(2, MapVU(initFieldValue, - ArithVU(ArithVU::Mul, 10)))); - EXPECT_TRUE(ptr->apply(3, MapVU(initFieldValue, - ArithVU(ArithVU::Div, 10)))); + EXPECT_TRUE(ptr->apply(0, MapVU(initFieldValue, ArithVU(ArithVU::Add, 10)))); + EXPECT_TRUE(ptr->apply(1, MapVU(initFieldValue, ArithVU(ArithVU::Sub, 10)))); + EXPECT_TRUE(ptr->apply(2, MapVU(initFieldValue, ArithVU(ArithVU::Mul, 10)))); + EXPECT_TRUE(ptr->apply(3, MapVU(initFieldValue, ArithVU(ArithVU::Div, 10)))); ptr->commit(); EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 10u); EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 4u); @@ -1594,8 +1585,7 @@ AttributeTest::testMapValueUpdate(const AttributePtr & ptr, BufferType initValue EXPECT_EQUAL(buf[0].getWeight(), 10); // removeifzero - EXPECT_TRUE(ptr->apply(4, MapVU(initFieldValue, - ArithVU(ArithVU::Sub, 100)))); + EXPECT_TRUE(ptr->apply(4, MapVU(initFieldValue, ArithVU(ArithVU::Sub, 100)))); ptr->commit(); if (removeIfZero) { EXPECT_EQUAL(ptr->get(4, &buf[0], 2), uint32_t(0)); @@ -1607,8 +1597,7 @@ AttributeTest::testMapValueUpdate(const AttributePtr & ptr, BufferType initValue EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 5u); // createifnonexistant - EXPECT_TRUE(ptr->apply(5, MapVU(nonExistant, - ArithVU(ArithVU::Add, 10)))); + EXPECT_TRUE(ptr->apply(5, MapVU(nonExistant, ArithVU(ArithVU::Add, 10)))); ptr->commit(); if (createIfNonExistant) { EXPECT_EQUAL(ptr->get(5, &buf[0], 2), uint32_t(2)); @@ -1639,25 +1628,21 @@ void AttributeTest::testMapValueUpdate() { { // regular set - AttributePtr ptr = createAttribute - ("wsint32", Config(BasicType::INT32, CollectionType::WSET)); + AttributePtr ptr = createAttribute("wsint32", Config(BasicType::INT32, CollectionType::WSET)); testMapValueUpdate<IntegerAttribute, AttributeVector::WeightedInt> - (ptr, AttributeVector::WeightedInt(64, 1), IntFieldValue(64), - IntFieldValue(32), false, false); + (ptr, AttributeVector::WeightedInt(64, 1), IntFieldValue(64), IntFieldValue(32), false, false); } { // remove if zero - AttributePtr ptr = createAttribute - ("wsint32", Config(BasicType::INT32, CollectionType(CollectionType::WSET, true, false))); + AttributePtr ptr = createAttribute("wsint32", Config(BasicType::INT32, + CollectionType(CollectionType::WSET, true, false))); testMapValueUpdate<IntegerAttribute, AttributeVector::WeightedInt> - (ptr, AttributeVector::WeightedInt(64, 1), IntFieldValue(64), - IntFieldValue(32), true, false); + (ptr, AttributeVector::WeightedInt(64, 1), IntFieldValue(64), IntFieldValue(32), true, false); } { // create if non existant - AttributePtr ptr = createAttribute - ("wsint32", Config(BasicType::INT32, CollectionType(CollectionType::WSET, false, true))); + AttributePtr ptr = createAttribute("wsint32", Config(BasicType::INT32, + CollectionType(CollectionType::WSET, false, true))); testMapValueUpdate<IntegerAttribute, AttributeVector::WeightedInt> - (ptr, AttributeVector::WeightedInt(64, 1), IntFieldValue(64), - IntFieldValue(32), false, true); + (ptr, AttributeVector::WeightedInt(64, 1), IntFieldValue(64), IntFieldValue(32), false, true); } Config setCfg(Config(BasicType::STRING, CollectionType::WSET)); @@ -1667,20 +1652,17 @@ AttributeTest::testMapValueUpdate() { // regular set AttributePtr ptr = createAttribute("wsstr", setCfg); testMapValueUpdate<StringAttribute, AttributeVector::WeightedString> - (ptr, AttributeVector::WeightedString("first", 1), StringFieldValue("first"), - StringFieldValue("second"), false, false); + (ptr, AttributeVector::WeightedString("first", 1), StringFieldValue("first"), StringFieldValue("second"), false, false); } { // remove if zero AttributePtr ptr = createAttribute("wsstr", setRemoveCfg); testMapValueUpdate<StringAttribute, AttributeVector::WeightedString> - (ptr, AttributeVector::WeightedString("first", 1), StringFieldValue("first"), - StringFieldValue("second"), true, false); + (ptr, AttributeVector::WeightedString("first", 1), StringFieldValue("first"), StringFieldValue("second"), true, false); } { // create if non existant AttributePtr ptr = createAttribute("wsstr", setCreateCfg); testMapValueUpdate<StringAttribute, AttributeVector::WeightedString> - (ptr, AttributeVector::WeightedString("first", 1), StringFieldValue("first"), - StringFieldValue("second"), false, true); + (ptr, AttributeVector::WeightedString("first", 1), StringFieldValue("first"), StringFieldValue("second"), false, true); } // fast-search - posting lists @@ -1688,46 +1670,38 @@ AttributeTest::testMapValueUpdate() setCfg.setFastSearch(true); AttributePtr ptr = createAttribute("wsfsstr", setCfg); testMapValueUpdate<StringAttribute, AttributeVector::WeightedString> - (ptr, AttributeVector::WeightedString("first", 1), StringFieldValue("first"), - StringFieldValue("second"), false, false); + (ptr, AttributeVector::WeightedString("first", 1), StringFieldValue("first"), StringFieldValue("second"), false, false); } { // remove if zero setRemoveCfg.setFastSearch(true); AttributePtr ptr = createAttribute("wsfsstr", setRemoveCfg); testMapValueUpdate<StringAttribute, AttributeVector::WeightedString> - (ptr, AttributeVector::WeightedString("first", 1), StringFieldValue("first"), - StringFieldValue("second"), true, false); + (ptr, AttributeVector::WeightedString("first", 1), StringFieldValue("first"), StringFieldValue("second"), true, false); } { // create if non existant setCreateCfg.setFastSearch(true); AttributePtr ptr = createAttribute("wsfsstr", setCreateCfg); testMapValueUpdate<StringAttribute, AttributeVector::WeightedString> - (ptr, AttributeVector::WeightedString("first", 1), StringFieldValue("first"), - StringFieldValue("second"), false, true); + (ptr, AttributeVector::WeightedString("first", 1), StringFieldValue("first"), StringFieldValue("second"), false, true); } } - - void AttributeTest::commit(const AttributePtr & ptr) { ptr->commit(); } - void AttributeTest::testStatus() { - std::vector<vespalib::string> values; + std::vector<string> values; fillString(values, 16); uint32_t numDocs = 100; // No posting list - static constexpr size_t LeafNodeSize = - 4 + sizeof(EnumStoreBase::Index) * EnumTreeTraits::LEAF_SLOTS; + static constexpr size_t LeafNodeSize = 4 + sizeof(EnumStoreBase::Index) * EnumTreeTraits::LEAF_SLOTS; static constexpr size_t InternalNodeSize = - 8 + (sizeof(EnumStoreBase::Index) + - sizeof(datastore::EntryRef)) * EnumTreeTraits::INTERNAL_SLOTS; + 8 + (sizeof(EnumStoreBase::Index) + sizeof(datastore::EntryRef)) * EnumTreeTraits::INTERNAL_SLOTS; static constexpr size_t NestedVectorSize = 24; // sizeof(vespalib::Array) { @@ -1782,9 +1756,9 @@ AttributeTest::testNullProtection() size_t len1 = strlen("evil"); size_t len2 = strlen("string"); size_t len = len1 + 1 + len2; - vespalib::string good("good"); - vespalib::string evil("evil string"); - vespalib::string pureEvil("evil"); + string good("good"); + string evil("evil string"); + string pureEvil("evil"); EXPECT_EQUAL(strlen(evil.data()), len); EXPECT_EQUAL(strlen(evil.c_str()), len); evil[len1] = 0; // replace space with '\0' @@ -1797,7 +1771,7 @@ AttributeTest::testNullProtection() EXPECT_EQUAL(evil.size(), len); { // string AttributeVector::DocId docId; - std::vector<vespalib::string> buf(16); + std::vector<string> buf(16); AttributePtr attr = createAttribute("string", Config(BasicType::STRING, CollectionType::SINGLE)); StringAttribute &v = static_cast<StringAttribute &>(*attr.get()); EXPECT_TRUE(v.addDoc(docId)); @@ -1809,7 +1783,7 @@ AttributeTest::testNullProtection() } { // string array AttributeVector::DocId docId; - std::vector<vespalib::string> buf(16); + std::vector<string> buf(16); AttributePtr attr = createAttribute("string", Config(BasicType::STRING, CollectionType::ARRAY)); StringAttribute &v = static_cast<StringAttribute &>(*attr.get()); EXPECT_TRUE(v.addDoc(docId)); @@ -2047,7 +2021,6 @@ AttributeTest::testCompactLidSpace(const Config &config, compare<VectorType, BufferType>(v, v3); } - template <typename VectorType, typename BufferType> void AttributeTest::testCompactLidSpace(const Config &config) @@ -2061,7 +2034,6 @@ AttributeTest::testCompactLidSpace(const Config &config) testCompactLidSpace<VectorType, BufferType>(config, true, true); } - void AttributeTest::testCompactLidSpace(const Config &config) { @@ -2074,28 +2046,24 @@ AttributeTest::testCompactLidSpace(const Config &config) case BasicType::INT32: case BasicType::INT64: if (config.collectionType() == CollectionType::WSET) { - testCompactLidSpace<IntegerAttribute, - IntegerAttribute::WeightedInt>(config); + testCompactLidSpace<IntegerAttribute, IntegerAttribute::WeightedInt>(config); } else { - testCompactLidSpace<IntegerAttribute, - IntegerAttribute::largeint_t>(config); + testCompactLidSpace<IntegerAttribute, IntegerAttribute::largeint_t>(config); } break; case BasicType::FLOAT: case BasicType::DOUBLE: if (config.collectionType() == CollectionType::WSET) { - testCompactLidSpace<FloatingPointAttribute, - FloatingPointAttribute::WeightedFloat>(config); + testCompactLidSpace<FloatingPointAttribute, FloatingPointAttribute::WeightedFloat>(config); } else { testCompactLidSpace<FloatingPointAttribute, double>(config); } break; case BasicType::STRING: if (config.collectionType() == CollectionType::WSET) { - testCompactLidSpace<StringAttribute, - StringAttribute::WeightedString>(config); + testCompactLidSpace<StringAttribute, StringAttribute::WeightedString>(config); } else { - testCompactLidSpace<StringAttribute, vespalib::string>(config); + testCompactLidSpace<StringAttribute, string>(config); } break; default: @@ -2103,58 +2071,33 @@ AttributeTest::testCompactLidSpace(const Config &config) } } - void AttributeTest::testCompactLidSpace() { - TEST_DO(testCompactLidSpace(Config(BasicType::UINT1, - CollectionType::SINGLE))); - TEST_DO(testCompactLidSpace(Config(BasicType::UINT2, - CollectionType::SINGLE))); - TEST_DO(testCompactLidSpace(Config(BasicType::UINT4, - CollectionType::SINGLE))); - TEST_DO(testCompactLidSpace(Config(BasicType::INT8, - CollectionType::SINGLE))); - TEST_DO(testCompactLidSpace(Config(BasicType::INT8, - CollectionType::ARRAY))); - TEST_DO(testCompactLidSpace(Config(BasicType::INT8, - CollectionType::WSET))); - TEST_DO(testCompactLidSpace(Config(BasicType::INT16, - CollectionType::SINGLE))); - TEST_DO(testCompactLidSpace(Config(BasicType::INT16, - CollectionType::ARRAY))); - TEST_DO(testCompactLidSpace(Config(BasicType::INT16, - CollectionType::WSET))); - TEST_DO(testCompactLidSpace(Config(BasicType::INT32, - CollectionType::SINGLE))); - TEST_DO(testCompactLidSpace(Config(BasicType::INT32, - CollectionType::ARRAY))); - TEST_DO(testCompactLidSpace(Config(BasicType::INT32, - CollectionType::WSET))); - TEST_DO(testCompactLidSpace(Config(BasicType::INT64, - CollectionType::SINGLE))); - TEST_DO(testCompactLidSpace(Config(BasicType::INT64, - CollectionType::ARRAY))); - TEST_DO(testCompactLidSpace(Config(BasicType::INT64, - CollectionType::WSET))); - TEST_DO(testCompactLidSpace(Config(BasicType::FLOAT, - CollectionType::SINGLE))); - TEST_DO(testCompactLidSpace(Config(BasicType::FLOAT, - CollectionType::ARRAY))); - TEST_DO(testCompactLidSpace(Config(BasicType::FLOAT, - CollectionType::WSET))); - TEST_DO(testCompactLidSpace(Config(BasicType::DOUBLE, - CollectionType::SINGLE))); - TEST_DO(testCompactLidSpace(Config(BasicType::DOUBLE, - CollectionType::ARRAY))); - TEST_DO(testCompactLidSpace(Config(BasicType::DOUBLE, - CollectionType::WSET))); - TEST_DO(testCompactLidSpace(Config(BasicType::STRING, - CollectionType::SINGLE))); - TEST_DO(testCompactLidSpace(Config(BasicType::STRING, - CollectionType::ARRAY))); - TEST_DO(testCompactLidSpace(Config(BasicType::STRING, - CollectionType::WSET))); + TEST_DO(testCompactLidSpace(Config(BasicType::UINT1, CollectionType::SINGLE))); + TEST_DO(testCompactLidSpace(Config(BasicType::UINT2, CollectionType::SINGLE))); + TEST_DO(testCompactLidSpace(Config(BasicType::UINT4, CollectionType::SINGLE))); + TEST_DO(testCompactLidSpace(Config(BasicType::INT8, CollectionType::SINGLE))); + TEST_DO(testCompactLidSpace(Config(BasicType::INT8, CollectionType::ARRAY))); + TEST_DO(testCompactLidSpace(Config(BasicType::INT8, CollectionType::WSET))); + TEST_DO(testCompactLidSpace(Config(BasicType::INT16, CollectionType::SINGLE))); + TEST_DO(testCompactLidSpace(Config(BasicType::INT16, CollectionType::ARRAY))); + TEST_DO(testCompactLidSpace(Config(BasicType::INT16, CollectionType::WSET))); + TEST_DO(testCompactLidSpace(Config(BasicType::INT32, CollectionType::SINGLE))); + TEST_DO(testCompactLidSpace(Config(BasicType::INT32, CollectionType::ARRAY))); + TEST_DO(testCompactLidSpace(Config(BasicType::INT32, CollectionType::WSET))); + TEST_DO(testCompactLidSpace(Config(BasicType::INT64, CollectionType::SINGLE))); + TEST_DO(testCompactLidSpace(Config(BasicType::INT64, CollectionType::ARRAY))); + TEST_DO(testCompactLidSpace(Config(BasicType::INT64, CollectionType::WSET))); + TEST_DO(testCompactLidSpace(Config(BasicType::FLOAT, CollectionType::SINGLE))); + TEST_DO(testCompactLidSpace(Config(BasicType::FLOAT, CollectionType::ARRAY))); + TEST_DO(testCompactLidSpace(Config(BasicType::FLOAT, CollectionType::WSET))); + TEST_DO(testCompactLidSpace(Config(BasicType::DOUBLE, CollectionType::SINGLE))); + TEST_DO(testCompactLidSpace(Config(BasicType::DOUBLE, CollectionType::ARRAY))); + TEST_DO(testCompactLidSpace(Config(BasicType::DOUBLE, CollectionType::WSET))); + TEST_DO(testCompactLidSpace(Config(BasicType::STRING, CollectionType::SINGLE))); + TEST_DO(testCompactLidSpace(Config(BasicType::STRING, CollectionType::ARRAY))); + TEST_DO(testCompactLidSpace(Config(BasicType::STRING, CollectionType::WSET))); } template <typename AttributeType> @@ -2220,7 +2163,6 @@ AttributeTest::requireThatAddressSpaceUsageIsReported() TEST_DO(requireThatAddressSpaceUsageIsReported<StringAttribute>(Config(BasicType::STRING, CollectionType::ARRAY))); } - template <typename AttributeType, typename BufferType> void AttributeTest::testReaderDuringLastUpdate(const Config &config, bool fs, bool compact) @@ -2230,7 +2172,7 @@ AttributeTest::testReaderDuringLastUpdate(const Config &config, bool fs, bool co config.collectionType().asString() << (fs ? "-fs" : "") << (compact ? "-compact" : ""); - vespalib::string name(ss.str()); + string name(ss.str()); Config cfg = config; cfg.setFastSearch(fs); cfg.setGrowStrategy(GrowStrategy::make(100, 50, 0)); @@ -2261,7 +2203,6 @@ AttributeTest::testReaderDuringLastUpdate(const Config &config, bool fs, bool co } } - template <typename AttributeType, typename BufferType> void AttributeTest::testReaderDuringLastUpdate(const Config &config) @@ -2281,8 +2222,8 @@ AttributeTest::testReaderDuringLastUpdate() TEST_DO((testReaderDuringLastUpdate<FloatingPointAttribute,double>(Config(BasicType::FLOAT, CollectionType::SINGLE)))); TEST_DO((testReaderDuringLastUpdate<FloatingPointAttribute,double>(Config(BasicType::FLOAT, CollectionType::ARRAY)))); TEST_DO((testReaderDuringLastUpdate<FloatingPointAttribute,FloatingPointAttribute::WeightedFloat>(Config(BasicType::FLOAT, CollectionType::WSET)))); - TEST_DO((testReaderDuringLastUpdate<StringAttribute,vespalib::string>(Config(BasicType::STRING, CollectionType::SINGLE)))); - TEST_DO((testReaderDuringLastUpdate<StringAttribute,vespalib::string>(Config(BasicType::STRING, CollectionType::ARRAY)))); + TEST_DO((testReaderDuringLastUpdate<StringAttribute,string>(Config(BasicType::STRING, CollectionType::SINGLE)))); + TEST_DO((testReaderDuringLastUpdate<StringAttribute,string>(Config(BasicType::STRING, CollectionType::ARRAY)))); TEST_DO((testReaderDuringLastUpdate<StringAttribute,StringAttribute::WeightedString>(Config(BasicType::STRING, CollectionType::WSET)))); } @@ -2374,5 +2315,4 @@ int AttributeTest::Main() } - TEST_APPHOOK(search::AttributeTest); diff --git a/searchlib/src/tests/attribute/attributemanager/attributemanager_test.cpp b/searchlib/src/tests/attribute/attributemanager/attributemanager_test.cpp index 3e6760ec5e4..5911d74dcb0 100644 --- a/searchlib/src/tests/attribute/attributemanager/attributemanager_test.cpp +++ b/searchlib/src/tests/attribute/attributemanager/attributemanager_test.cpp @@ -1,6 +1,5 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/log/log.h> -LOG_SETUP("attribute_test"); + #include <vespa/searchlib/attribute/attribute.h> #include <vespa/searchlib/attribute/attributeguard.h> #include <vespa/searchlib/attribute/attributefactory.h> @@ -10,13 +9,16 @@ LOG_SETUP("attribute_test"); #include <vespa/searchlib/attribute/multinumericattribute.hpp> #include <vespa/searchlib/attribute/stringattribute.h> #include <vespa/vespalib/testkit/testapp.h> -#include <algorithm> + +#include <vespa/log/log.h> +LOG_SETUP("attribute_test"); using namespace config; using namespace vespa::config::search; using namespace search; using namespace search::attribute; using std::shared_ptr; +using vespalib::stringref; typedef BasicType BT; typedef CollectionType CT; @@ -59,40 +61,14 @@ class TestAttribute : public TestAttributeBase { public: TestAttribute(const std::string &name) - : - TestAttributeBase(name) - { - } - - generation_t - getGen() const - { - return getCurrentGeneration(); - } - - uint32_t - getRefCount(generation_t gen) const - { - return getGenerationRefCount(gen); - } - - void - incGen() - { - incGeneration(); - } - - void - updateFirstUsedGen() - { - updateFirstUsedGeneration(); - } - - generation_t - getFirstUsedGen() const - { - return getFirstUsedGeneration(); - } + : TestAttributeBase(name) + {} + + generation_t getGen() const { return getCurrentGeneration(); } + uint32_t getRefCount(generation_t gen) const { return getGenerationRefCount(gen); } + void incGen() { incGeneration(); } + void updateFirstUsedGen() { updateFirstUsedGeneration(); } + generation_t getFirstUsedGen() const { return getFirstUsedGeneration(); } }; @@ -216,8 +192,7 @@ AttributeManagerTest::testLoad() bool -AttributeManagerTest::assertDataType(BT::Type exp, - AttributesConfig::Attribute::Datatype in) +AttributeManagerTest::assertDataType(BT::Type exp, AttributesConfig::Attribute::Datatype in) { AttributesConfig::Attribute a; a.datatype = in; @@ -227,10 +202,8 @@ AttributeManagerTest::assertDataType(BT::Type exp, bool AttributeManagerTest:: -assertCollectionType(CollectionType exp, - AttributesConfig::Attribute::Collectiontype in, - bool removeIfZ, - bool createIfNe) +assertCollectionType(CollectionType exp, AttributesConfig::Attribute::Collectiontype in, + bool removeIfZ, bool createIfNe) { AttributesConfig::Attribute a; a.collectiontype = in; @@ -239,8 +212,7 @@ assertCollectionType(CollectionType exp, AttributeVector::Config out = ConfigConverter::convert(a); return EXPECT_EQUAL(exp.type(), out.collectionType().type()) && EXPECT_EQUAL(exp.removeIfZero(), out.collectionType().removeIfZero()) && - EXPECT_EQUAL(exp.createIfNonExistant(), - out.collectionType().createIfNonExistant()); + EXPECT_EQUAL(exp.createIfNonExistant(), out.collectionType().createIfNonExistant()); } @@ -308,14 +280,10 @@ AttributeManagerTest::testContext() { std::vector<AVSP> attrs; // create various attributes vectors - attrs.push_back(AttributeFactory::createAttribute("sint32", - Config(BT::INT32, CT::SINGLE))); - attrs.push_back(AttributeFactory::createAttribute("aint32", - Config(BT::INT32, CT::ARRAY))); - attrs.push_back(AttributeFactory::createAttribute("wsint32", - Config(BT::INT32, CT::WSET))); - attrs.push_back(AttributeFactory::createAttribute("dontcare", - Config(BT::INT32, CT::SINGLE))); + attrs.push_back(AttributeFactory::createAttribute("sint32", Config(BT::INT32, CT::SINGLE))); + attrs.push_back(AttributeFactory::createAttribute("aint32", Config(BT::INT32, CT::ARRAY))); + attrs.push_back(AttributeFactory::createAttribute("wsint32", Config(BT::INT32, CT::WSET))); + attrs.push_back(AttributeFactory::createAttribute("dontcare", Config(BT::INT32, CT::SINGLE))); // add docs for (uint32_t i = 0; i < attrs.size(); ++i) { diff --git a/searchlib/src/tests/attribute/enumeratedsave/enumeratedsave_test.cpp b/searchlib/src/tests/attribute/enumeratedsave/enumeratedsave_test.cpp index 8ec9d59da86..432396d635e 100644 --- a/searchlib/src/tests/attribute/enumeratedsave/enumeratedsave_test.cpp +++ b/searchlib/src/tests/attribute/enumeratedsave/enumeratedsave_test.cpp @@ -103,32 +103,24 @@ public: typedef std::shared_ptr<MemAttr> SP; MemAttr(); - ~MemAttr(); - - // Implements IAttributeSaveTarget - virtual bool setup() override { return true; } - virtual void close() override {} - virtual IAttributeFileWriter &datWriter() override { return _datWriter; } - virtual IAttributeFileWriter &idxWriter() override { return _idxWriter; } - virtual IAttributeFileWriter &weightWriter() override { + ~MemAttr() override; + + bool setup() override { return true; } + void close() override {} + IAttributeFileWriter &datWriter() override { return _datWriter; } + IAttributeFileWriter &idxWriter() override { return _idxWriter; } + IAttributeFileWriter &weightWriter() override { return _weightWriter; } - virtual IAttributeFileWriter &udatWriter() override { return _udatWriter; } + IAttributeFileWriter &udatWriter() override { return _udatWriter; } - bool - bufEqual(const Buffer &lhs, const Buffer &rhs) const; + bool bufEqual(const Buffer &lhs, const Buffer &rhs) const; - bool - operator==(const MemAttr &rhs) const; + bool operator==(const MemAttr &rhs) const; }; -MemAttr::MemAttr() - : _datWriter(), - _idxWriter(), - _weightWriter(), - _udatWriter() -{ } -MemAttr::~MemAttr() {} +MemAttr::MemAttr() = default; +MemAttr::~MemAttr() = default; class EnumeratedSaveTest { @@ -136,20 +128,11 @@ private: typedef AttributeVector::SP AttributePtr; template <typename VectorType> - VectorType & - as(AttributePtr &v); - - IntegerAttribute & - asInt(AttributePtr &v); - - StringAttribute & - asString(AttributePtr &v); - - FloatingPointAttribute & - asFloat(AttributePtr &v); - - void - addDocs(const AttributePtr &v, size_t sz); + VectorType & as(AttributePtr &v); + IntegerAttribute & asInt(AttributePtr &v); + StringAttribute & asString(AttributePtr &v); + FloatingPointAttribute & asFloat(AttributePtr &v); + void addDocs(const AttributePtr &v, size_t sz); template <typename VectorType> void populate(VectorType &v, unsigned seed, BasicType bt); @@ -157,65 +140,34 @@ private: template <typename VectorType, typename BufferType> void compare(VectorType &a, VectorType &b); - void - buildTermQuery(std::vector<char> & buffer, - const vespalib::string & index, - const vespalib::string & term, bool prefix); + void buildTermQuery(std::vector<char> & buffer, const vespalib::string & index, + const vespalib::string & term, bool prefix); template <typename V, typename T> - SearchContextPtr - getSearch(const V & vec, const T & term, bool prefix); + SearchContextPtr getSearch(const V & vec, const T & term, bool prefix); template <typename V> - SearchContextPtr - getSearch(const V & vec); - - MemAttr::SP - saveMem(AttributeVector &v); + SearchContextPtr getSearch(const V & vec); - void - checkMem(AttributeVector &v, const MemAttr &e, bool enumerated); - - MemAttr::SP - saveBoth(AttributePtr v); - - AttributePtr - make(Config cfg, - const vespalib::string &pref, - bool fastSearch = false); - - void - load(AttributePtr v, const vespalib::string &name); + MemAttr::SP saveMem(AttributeVector &v); + void checkMem(AttributeVector &v, const MemAttr &e, bool enumerated); + MemAttr::SP saveBoth(AttributePtr v); + AttributePtr make(Config cfg, const vespalib::string &pref, bool fastSearch = false); + void load(AttributePtr v, const vespalib::string &name); template <typename VectorType, typename BufferType> - void - checkLoad(AttributePtr v, - const vespalib::string &name, - AttributePtr ev); + void checkLoad(AttributePtr v, const vespalib::string &name, AttributePtr ev); template <typename VectorType, typename BufferType> - void - testReload(AttributePtr v0, - AttributePtr v1, - AttributePtr v2, - MemAttr::SP mv0, - MemAttr::SP mv1, - MemAttr::SP mv2, - MemAttr::SP emv0, - MemAttr::SP emv1, - MemAttr::SP emv2, - Config cfg, - const vespalib::string &pref, - bool fastSearch); + void testReload(AttributePtr v0, AttributePtr v1, AttributePtr v2, + MemAttr::SP mv0, MemAttr::SP mv1, MemAttr::SP mv2, + MemAttr::SP emv0, MemAttr::SP emv1, MemAttr::SP emv2, + Config cfg, const vespalib::string &pref, bool fastSearch); public: template <typename VectorType, typename BufferType> - void - test(BasicType bt, CollectionType ct, const vespalib::string &pref); + void test(BasicType bt, CollectionType ct, const vespalib::string &pref); - EnumeratedSaveTest() - { - } }; @@ -605,9 +557,7 @@ EnumeratedSaveTest::saveBoth(AttributePtr v) EnumeratedSaveTest::AttributePtr -EnumeratedSaveTest::make(Config cfg, - const vespalib::string &pref, - bool fastSearch) +EnumeratedSaveTest::make(Config cfg, const vespalib::string &pref, bool fastSearch) { cfg.setFastSearch(fastSearch); AttributePtr v = AttributeFactory::createAttribute(pref, cfg); @@ -751,13 +701,11 @@ EnumeratedSaveTest::test(BasicType bt, CollectionType ct, TEST_DO((testReload<VectorType, BufferType>(v0, v1, v2, mv0, mv1, mv2, emv0, emv1, emv2, - cfg, pref, - false))); + cfg, pref, false))); TEST_DO((testReload<VectorType, BufferType>(v0, v1, v2, mv0, mv1, mv2, emv0, emv1, emv2, - cfg, pref, - true))); + cfg, pref, true))); } TEST_F("Test enumerated save with single value int8", EnumeratedSaveTest) diff --git a/searchlib/src/tests/attribute/postinglistattribute/postinglistattribute_test.cpp b/searchlib/src/tests/attribute/postinglistattribute/postinglistattribute_test.cpp index d5b1c9566c4..c6270e32c85 100644 --- a/searchlib/src/tests/attribute/postinglistattribute/postinglistattribute_test.cpp +++ b/searchlib/src/tests/attribute/postinglistattribute/postinglistattribute_test.cpp @@ -1003,10 +1003,8 @@ PostingListAttributeTest::testMinMax() { Config cfg(Config(BasicType::INT32, CollectionType::WSET)); cfg.setFastSearch(true); - AttributePtr ptr1 = - AttributeFactory::createAttribute("wsint32_1", cfg); - AttributePtr ptr2 = - AttributeFactory::createAttribute("wsint32_2", cfg); + AttributePtr ptr1 = AttributeFactory::createAttribute("wsint32_1", cfg); + AttributePtr ptr2 = AttributeFactory::createAttribute("wsint32_2", cfg); testMinMax<IntegerAttribute>(ptr1, ptr2); } { diff --git a/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp b/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp index 69aa612d5b8..bc6ce97fc37 100644 --- a/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp +++ b/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp @@ -1,6 +1,4 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/log/log.h> -LOG_SETUP("stringattribute_test"); #include <vespa/vespalib/testkit/testapp.h> #include <vespa/searchlib/attribute/enumstore.h> #include <vespa/searchlib/attribute/singlestringattribute.h> @@ -14,6 +12,9 @@ LOG_SETUP("stringattribute_test"); #include <vespa/searchlib/attribute/multistringattribute.h> #include <vespa/searchlib/attribute/multistringpostattribute.hpp> +#include <vespa/log/log.h> +LOG_SETUP("stringattribute_test"); + namespace search { using attribute::CollectionType; @@ -430,11 +431,8 @@ StringAttributeTest::Main() TEST_INIT("stringattribute_test"); testMultiValue(); - testMultiValueMultipleClearDocBetweenCommit(); - testMultiValueRemove(); - testSingleValue(); TEST_DONE(); diff --git a/searchlib/src/tests/features/prod_features_attributematch.cpp b/searchlib/src/tests/features/prod_features_attributematch.cpp index 4ddb3170efe..e5d37e62bee 100644 --- a/searchlib/src/tests/features/prod_features_attributematch.cpp +++ b/searchlib/src/tests/features/prod_features_attributematch.cpp @@ -1,10 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/log/log.h> -LOG_SETUP(".prod_features_attributematch"); #include "prod_features.h" #include <vespa/searchlib/features/attributematchfeature.h> #include <vespa/searchlib/attribute/attributefactory.h> +#include <vespa/searchlib/attribute/attributevector.h> + +#include <vespa/log/log.h> +LOG_SETUP(".prod_features_attributematch"); using namespace search::features; using namespace search::fef; diff --git a/searchlib/src/vespa/searchlib/attribute/attributefactory.cpp b/searchlib/src/vespa/searchlib/attribute/attributefactory.cpp index fd1a8ffe952..8f03c3b3851 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributefactory.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attributefactory.cpp @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "attributefactory.h" +#include "attributevector.h" #include <vespa/log/log.h> LOG_SETUP(".searchlib.attributefactory"); @@ -10,45 +11,45 @@ namespace search { using attribute::CollectionType; AttributeVector::SP -AttributeFactory::createAttribute(const vespalib::string & baseFileName, const Config & cfg) +AttributeFactory::createAttribute(stringref name, const Config & cfg) { AttributeVector::SP ret; if (cfg.collectionType().type() == CollectionType::ARRAY) { if (cfg.fastSearch()) { - ret = createArrayFastSearch(baseFileName, cfg); - if (ret.get() == NULL) { + ret = createArrayFastSearch(name, cfg); + if ( ! ret) { LOG(warning, "Cannot apply fastsearch hint on attribute %s of type array<%s>. " "Falling back to normal. You should correct your .sd file.", - baseFileName.c_str(), cfg.basicType().asString()); - ret = createArrayStd(baseFileName, cfg); + name.data(), cfg.basicType().asString()); + ret = createArrayStd(name, cfg); } } else { - ret = createArrayStd(baseFileName, cfg); + ret = createArrayStd(name, cfg); } } else if (cfg.collectionType().type() == CollectionType::WSET) { // Ignore if noupdate has been set. if (cfg.fastSearch()) { - ret = createSetFastSearch(baseFileName, cfg); - if (ret.get() == NULL) { + ret = createSetFastSearch(name, cfg); + if ( ! ret) { LOG(warning, "Cannot apply fastsearch hint on attribute %s of type set<%s>. " "Falling back to normal. You should correct your .sd file.", - baseFileName.c_str(), cfg.basicType().asString()); - ret = createSetStd(baseFileName, cfg); + name.data(), cfg.basicType().asString()); + ret = createSetStd(name, cfg); } } else { - ret = createSetStd(baseFileName, cfg); + ret = createSetStd(name, cfg); } } else { if (cfg.fastSearch()) { - ret = createSingleFastSearch(baseFileName, cfg); - if (ret.get() == NULL) { + ret = createSingleFastSearch(name, cfg); + if ( ! ret) { LOG(warning, "Cannot apply fastsearch hint on attribute %s of type %s. " "Falling back to normal. You should correct your .sd file.", - baseFileName.c_str(), cfg.basicType().asString()); - ret = createSingleStd(baseFileName, cfg); + name.data(), cfg.basicType().asString()); + ret = createSingleStd(name, cfg); } } else { - ret = createSingleStd(baseFileName, cfg); + ret = createSingleStd(name, cfg); } } return ret; diff --git a/searchlib/src/vespa/searchlib/attribute/attributefactory.h b/searchlib/src/vespa/searchlib/attribute/attributefactory.h index 39f662e040c..edcdee4ff0a 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributefactory.h +++ b/searchlib/src/vespa/searchlib/attribute/attributefactory.h @@ -2,31 +2,31 @@ #pragma once -#include "attributevector.h" +#include <vespa/searchcommon/attribute/config.h> namespace search { +class AttributeVector; + /** * Factory for creating attribute vector instances. **/ class AttributeFactory { private: - typedef attribute::Config Config; - static AttributeVector::SP createArrayStd(const vespalib::string & baseFileName, const Config & cfg); - static AttributeVector::SP createArrayFastSearch(const vespalib::string & baseFileName, const Config & cfg); - static AttributeVector::SP createSetStd(const vespalib::string & baseFileName, const Config & cfg); - static AttributeVector::SP createSetFastSearch(const vespalib::string & baseFileName, const Config & cfg); - static AttributeVector::SP createSingleStd(const vespalib::string & baseFileName, const Config & cfg); - static AttributeVector::SP createSingleFastSearch(const vespalib::string & baseFileName, const Config & cfg); - static AttributeVector::SP createSingleFastAggregate(const vespalib::string & baseFileName, const Config & cfg); - static AttributeVector::SP createArrayFastAggregate(const vespalib::string & baseFileName, const Config & cfg); - static AttributeVector::SP createSetFastAggregate(const vespalib::string & baseFileName, const Config & cfg); - + using stringref = vespalib::stringref; + using Config = attribute::Config; + using AttributeSP = std::shared_ptr<AttributeVector>; + static AttributeSP createArrayStd(stringref name, const Config & cfg); + static AttributeSP createArrayFastSearch(stringref name, const Config & cfg); + static AttributeSP createSetStd(stringref name, const Config & cfg); + static AttributeSP createSetFastSearch(stringref name, const Config & cfg); + static AttributeSP createSingleStd(stringref name, const Config & cfg); + static AttributeSP createSingleFastSearch(stringref name, const Config & cfg); public: /** * Create an attribute vector with the given name based on the given config. **/ - static AttributeVector::SP createAttribute(const vespalib::string & baseFileName, const Config & cfg); + static AttributeSP createAttribute(stringref name, const Config & cfg); }; } diff --git a/searchlib/src/vespa/searchlib/attribute/attributefilesavetarget.h b/searchlib/src/vespa/searchlib/attribute/attributefilesavetarget.h index e961396750e..acb3daf82e0 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributefilesavetarget.h +++ b/searchlib/src/vespa/searchlib/attribute/attributefilesavetarget.h @@ -21,21 +21,20 @@ private: public: AttributeFileSaveTarget(const TuneFileAttributes &tuneFileAttributes, - const search::common::FileHeaderContext & - fileHeaderContext); - ~AttributeFileSaveTarget(); + const search::common::FileHeaderContext &fileHeaderContext); + ~AttributeFileSaveTarget() override; // Implements IAttributeSaveTarget /** Setups this saveTarget by opening the relevant files **/ - virtual bool setup() override; + bool setup() override; /** Closes the files used **/ - virtual void close() override; + void close() override; - virtual IAttributeFileWriter &datWriter() override; - virtual IAttributeFileWriter &idxWriter() override; - virtual IAttributeFileWriter &weightWriter() override; - virtual IAttributeFileWriter &udatWriter() override; + IAttributeFileWriter &datWriter() override; + IAttributeFileWriter &idxWriter() override; + IAttributeFileWriter &weightWriter() override; + IAttributeFileWriter &udatWriter() override; }; } // namespace search diff --git a/searchlib/src/vespa/searchlib/attribute/attributefilewriter.h b/searchlib/src/vespa/searchlib/attribute/attributefilewriter.h index 06f350754ae..85a686ee02a 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributefilewriter.h +++ b/searchlib/src/vespa/searchlib/attribute/attributefilewriter.h @@ -38,9 +38,9 @@ public: const attribute::AttributeHeader &header, const vespalib::string &desc); ~AttributeFileWriter(); - virtual Buffer allocBuf(size_t size) override; - virtual void writeBuf(Buffer buf) override; - virtual std::unique_ptr<BufferWriter> allocBufferWriter() override; + Buffer allocBuf(size_t size) override; + void writeBuf(Buffer buf) override; + std::unique_ptr<BufferWriter> allocBufferWriter() override; bool open(const vespalib::string &fileName); void close(); }; diff --git a/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp b/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp index c129d23fcc5..6e8736a535c 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp @@ -161,7 +161,7 @@ AttributeManager::getAttribute(const string & name) const { AttributeGuard::UP attrGuard(new AttributeGuard(VectorHolder())); const VectorHolder * vh = findAndLoadAttribute(name); - if ( vh != NULL ) { + if ( vh != nullptr ) { attrGuard.reset(new AttributeGuard(*vh)); } return attrGuard; diff --git a/searchlib/src/vespa/searchlib/attribute/attributememorysavetarget.h b/searchlib/src/vespa/searchlib/attribute/attributememorysavetarget.h index c4928f8277b..0be594c38d0 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributememorysavetarget.h +++ b/searchlib/src/vespa/searchlib/attribute/attributememorysavetarget.h @@ -38,17 +38,15 @@ public: /** * Write the underlying buffer(s) to file(s). **/ - bool - writeToFile(const TuneFileAttributes &tuneFileAttributes, - const search::common::FileHeaderContext &fileHeaderContext); - - // Implements IAttributeSaveTarget - virtual bool setup() override { return true; } - virtual void close() override {} - virtual IAttributeFileWriter &datWriter() override; - virtual IAttributeFileWriter &idxWriter() override; - virtual IAttributeFileWriter &weightWriter() override; - virtual IAttributeFileWriter &udatWriter() override; + bool writeToFile(const TuneFileAttributes &tuneFileAttributes, + const search::common::FileHeaderContext &fileHeaderContext); + + bool setup() override { return true; } + void close() override {} + IAttributeFileWriter &datWriter() override; + IAttributeFileWriter &idxWriter() override; + IAttributeFileWriter &weightWriter() override; + IAttributeFileWriter &udatWriter() override; }; } // namespace search diff --git a/searchlib/src/vespa/searchlib/attribute/attributesaver.cpp b/searchlib/src/vespa/searchlib/attribute/attributesaver.cpp index 1f4cefb7658..f2852fe3050 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributesaver.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attributesaver.cpp @@ -15,10 +15,7 @@ AttributeSaver::AttributeSaver(GenerationHandler::Guard &&guard, } -AttributeSaver::~AttributeSaver() -{ -} - +AttributeSaver::~AttributeSaver() = default; bool AttributeSaver::save(IAttributeSaveTarget &saveTarget) diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.cpp b/searchlib/src/vespa/searchlib/attribute/attributevector.cpp index 4e3f8247ec2..d0886c25898 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributevector.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attributevector.cpp @@ -460,7 +460,7 @@ const char * AttributeVector::getStringFromEnum(EnumHandle) const { return nullp AttributeVector::SearchContext::SearchContext(const AttributeVector &attr) : _attr(attr), - _plsc(NULL) + _plsc(nullptr) { } AttributeVector::SearchContext::UP @@ -470,8 +470,7 @@ AttributeVector::getSearch(QueryPacketT searchSpec, const SearchContextParams &p } attribute::ISearchContext::UP -AttributeVector::createSearchContext(QueryTermSimpleUP term, - const attribute::SearchContextParams ¶ms) const +AttributeVector::createSearchContext(QueryTermSimpleUP term, const attribute::SearchContextParams ¶ms) const { return getSearch(std::move(term), params); } @@ -481,7 +480,7 @@ AttributeVector::SearchContext::~SearchContext() = default; unsigned int AttributeVector::SearchContext::approximateHits() const { - if (_plsc != NULL) { + if (_plsc != nullptr) { return _plsc->approximateHits(); } return std::max(uint64_t(_attr.getNumDocs()), @@ -492,7 +491,7 @@ SearchIterator::UP AttributeVector::SearchContext:: createIterator(fef::TermFieldMatchData *matchData, bool strict) { - if (_plsc != NULL) { + if (_plsc != nullptr) { SearchIterator::UP res = _plsc->createPostingIterator(matchData, strict); if (res) { return res; @@ -507,25 +506,21 @@ AttributeVector::SearchContext:: createFilterIterator(fef::TermFieldMatchData *matchData, bool strict) { if (!valid()) - return SearchIterator::UP(new queryeval::EmptySearch()); + return std::make_unique<queryeval::EmptySearch>(); if (getIsFilter()) { return SearchIterator::UP(strict ? - new FilterAttributeIteratorStrict<AttributeVector::SearchContext> - (*this, matchData) : - new FilterAttributeIteratorT<AttributeVector::SearchContext> - (*this, matchData)); + new FilterAttributeIteratorStrict<AttributeVector::SearchContext>(*this, matchData) : + new FilterAttributeIteratorT<AttributeVector::SearchContext>(*this, matchData)); } return SearchIterator::UP(strict ? - new AttributeIteratorStrict<AttributeVector::SearchContext> - (*this, matchData) : - new AttributeIteratorT<AttributeVector::SearchContext> - (*this, matchData)); + new AttributeIteratorStrict<AttributeVector::SearchContext>(*this, matchData) : + new AttributeIteratorT<AttributeVector::SearchContext>(*this, matchData)); } void AttributeVector::SearchContext::fetchPostings(bool strict) { - if (_plsc != NULL) + if (_plsc != nullptr) _plsc->fetchPostings(strict); } @@ -536,8 +531,7 @@ AttributeVector::apply(DocId doc, const MapValueUpdate &map) { if (retval) { const ValueUpdate & vu(map.getUpdate()); if (vu.inherits(ArithmeticValueUpdate::classId)) { - const ArithmeticValueUpdate & - au(static_cast<const ArithmeticValueUpdate &>(vu)); + const ArithmeticValueUpdate &au(static_cast<const ArithmeticValueUpdate &>(vu)); retval = applyWeight(doc, map.getKey(), au); } else { retval = false; diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.h b/searchlib/src/vespa/searchlib/attribute/attributevector.h index 5163457b3c8..8cf4079ccfa 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributevector.h +++ b/searchlib/src/vespa/searchlib/attribute/attributevector.h @@ -121,6 +121,7 @@ protected: using QueryTermSimpleUP = std::unique_ptr<QueryTermSimple>; using QueryPacketT = vespalib::stringref; using LoadedBufferUP = std::unique_ptr<fileutil::LoadedBuffer>; + using stringref = vespalib::stringref; public: typedef std::shared_ptr<AttributeVector> SP; class BaseName : public vespalib::string @@ -226,7 +227,7 @@ protected: private: AttributeVector * stealAttr() const { AttributeVector * ret(_attr); - _attr = NULL; + _attr = nullptr; return ret; } @@ -237,8 +238,7 @@ protected: { std::unique_lock<std::shared_timed_mutex> _enumLock; public: - EnumModifier(std::shared_timed_mutex &lock, - attribute::InterlockGuard &interlockGuard) + EnumModifier(std::shared_timed_mutex &lock, attribute::InterlockGuard &interlockGuard) : _enumLock(lock) { (void) interlockGuard; @@ -557,8 +557,8 @@ public: }; SearchContext::UP getSearch(QueryPacketT searchSpec, const attribute::SearchContextParams ¶ms) const; - virtual attribute::ISearchContext::UP createSearchContext(QueryTermSimpleUP term, - const attribute::SearchContextParams ¶ms) const override; + attribute::ISearchContext::UP createSearchContext(QueryTermSimpleUP term, + const attribute::SearchContextParams ¶ms) const override; virtual SearchContext::UP getSearch(QueryTermSimpleUP term, const attribute::SearchContextParams ¶ms) const = 0; virtual const EnumStoreBase *getEnumStoreBase() const; virtual const attribute::MultiValueMappingBase *getMultiValueBase() const; @@ -648,15 +648,15 @@ public: bool hasPostings(); virtual uint64_t getUniqueValueCount() const; virtual uint64_t getTotalValueCount() const; - virtual void compactLidSpace(uint32_t wantedLidLimit) override; + void compactLidSpace(uint32_t wantedLidLimit) override; virtual void clearDocs(DocId lidLow, DocId lidLimit); bool wantShrinkLidSpace() const { return _committedDocIdLimit < getNumDocs(); } - virtual bool canShrinkLidSpace() const override; - virtual void shrinkLidSpace() override; + bool canShrinkLidSpace() const override; + void shrinkLidSpace() override; virtual void onShrinkLidSpace(); - virtual size_t getEstimatedShrinkLidSpaceGain() const override; + size_t getEstimatedShrinkLidSpaceGain() const override; - virtual std::unique_ptr<attribute::AttributeReadGuard> makeReadGuard(bool stableEnumGuard) const override; + std::unique_ptr<attribute::AttributeReadGuard> makeReadGuard(bool stableEnumGuard) const override; void setInterlock(const std::shared_ptr<attribute::Interlock> &interlock); diff --git a/searchlib/src/vespa/searchlib/attribute/attrvector.cpp b/searchlib/src/vespa/searchlib/attribute/attrvector.cpp index 41945dff1fe..0304aa8f38e 100644 --- a/searchlib/src/vespa/searchlib/attribute/attrvector.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attrvector.cpp @@ -18,7 +18,7 @@ StringDirectAttribute(const vespalib::string & baseFileName, const Config & c) { } -StringDirectAttribute::~StringDirectAttribute() {} +StringDirectAttribute::~StringDirectAttribute() = default; bool StringDirectAttribute::findEnum(const char * key, EnumHandle & e) const { diff --git a/searchlib/src/vespa/searchlib/attribute/createarrayfastsearch.cpp b/searchlib/src/vespa/searchlib/attribute/createarrayfastsearch.cpp index 79266fb7128..17e6c91daba 100644 --- a/searchlib/src/vespa/searchlib/attribute/createarrayfastsearch.cpp +++ b/searchlib/src/vespa/searchlib/attribute/createarrayfastsearch.cpp @@ -28,7 +28,7 @@ using attribute::BasicType; #define CREATEFLOATARRAY(T, fname, info) static_cast<AttributeVector *>(new FLOATARRAY(T)(fname, info)) AttributeVector::SP -AttributeFactory::createArrayFastSearch(const vespalib::string & baseFileName, const Config & info) +AttributeFactory::createArrayFastSearch(stringref name, const Config & info) { assert(info.collectionType().type() == attribute::CollectionType::ARRAY); assert(info.fastSearch()); @@ -39,25 +39,25 @@ AttributeFactory::createArrayFastSearch(const vespalib::string & baseFileName, c case BasicType::UINT4: break; case BasicType::INT8: - ret.reset(static_cast<AttributeVector *>(new FlagAttribute(baseFileName, info))); + ret.reset(static_cast<AttributeVector *>(new FlagAttribute(name, info))); break; case BasicType::INT16: - ret.reset(CREATEINTARRAY(int16_t, baseFileName, info)); + ret.reset(CREATEINTARRAY(int16_t, name, info)); break; case BasicType::INT32: - ret.reset(CREATEINTARRAY(int32_t, baseFileName, info)); + ret.reset(CREATEINTARRAY(int32_t, name, info)); break; case BasicType::INT64: - ret.reset(CREATEINTARRAY(int64_t, baseFileName, info)); + ret.reset(CREATEINTARRAY(int64_t, name, info)); break; case BasicType::FLOAT: - ret.reset(CREATEFLOATARRAY(float, baseFileName, info)); + ret.reset(CREATEFLOATARRAY(float, name, info)); break; case BasicType::DOUBLE: - ret.reset(CREATEFLOATARRAY(double, baseFileName, info)); + ret.reset(CREATEFLOATARRAY(double, name, info)); break; case BasicType::STRING: - ret.reset(static_cast<AttributeVector *>(new ArrayStringPostingAttribute(baseFileName, info))); + ret.reset(static_cast<AttributeVector *>(new ArrayStringPostingAttribute(name, info))); break; default: break; diff --git a/searchlib/src/vespa/searchlib/attribute/createarraystd.cpp b/searchlib/src/vespa/searchlib/attribute/createarraystd.cpp index 9e2d4d764e7..ac82e4ba27a 100644 --- a/searchlib/src/vespa/searchlib/attribute/createarraystd.cpp +++ b/searchlib/src/vespa/searchlib/attribute/createarraystd.cpp @@ -22,7 +22,7 @@ using attribute::BasicType; AttributeVector::SP -AttributeFactory::createArrayStd(const vespalib::string & baseFileName, const Config & info) +AttributeFactory::createArrayStd(stringref name, const Config & info) { assert(info.collectionType().type() == attribute::CollectionType::ARRAY); AttributeVector::SP ret; @@ -32,25 +32,25 @@ AttributeFactory::createArrayStd(const vespalib::string & baseFileName, const Co case BasicType::UINT4: break; case BasicType::INT8: - ret.reset(CREATEINTARRAY(int8_t, baseFileName, info)); + ret.reset(CREATEINTARRAY(int8_t, name, info)); break; case BasicType::INT16: - ret.reset(CREATEINTARRAY(int16_t, baseFileName, info)); + ret.reset(CREATEINTARRAY(int16_t, name, info)); break; case BasicType::INT32: - ret.reset(CREATEINTARRAY(int32_t, baseFileName, info)); + ret.reset(CREATEINTARRAY(int32_t, name, info)); break; case BasicType::INT64: - ret.reset(CREATEINTARRAY(int64_t, baseFileName, info)); + ret.reset(CREATEINTARRAY(int64_t, name, info)); break; case BasicType::FLOAT: - ret.reset(CREATEFLOATARRAY(float, baseFileName, info)); + ret.reset(CREATEFLOATARRAY(float, name, info)); break; case BasicType::DOUBLE: - ret.reset(CREATEFLOATARRAY(double, baseFileName, info)); + ret.reset(CREATEFLOATARRAY(double, name, info)); break; case BasicType::STRING: - ret.reset(static_cast<AttributeVector *>(new ArrayStringAttribute(baseFileName, info))); + ret.reset(static_cast<AttributeVector *>(new ArrayStringAttribute(name, info))); break; default: break; diff --git a/searchlib/src/vespa/searchlib/attribute/createsetfastsearch.cpp b/searchlib/src/vespa/searchlib/attribute/createsetfastsearch.cpp index 1068032eecf..323d073589f 100644 --- a/searchlib/src/vespa/searchlib/attribute/createsetfastsearch.cpp +++ b/searchlib/src/vespa/searchlib/attribute/createsetfastsearch.cpp @@ -29,7 +29,7 @@ using attribute::BasicType; AttributeVector::SP -AttributeFactory::createSetFastSearch(const vespalib::string & baseFileName, const Config & info) +AttributeFactory::createSetFastSearch(stringref name, const Config & info) { assert(info.collectionType().type() == attribute::CollectionType::WSET); assert(info.fastSearch()); @@ -40,25 +40,25 @@ AttributeFactory::createSetFastSearch(const vespalib::string & baseFileName, con case BasicType::UINT4: break; case BasicType::INT8: - ret.reset(CREATEINTSET(int8_t, baseFileName, info)); + ret.reset(CREATEINTSET(int8_t, name, info)); break; case BasicType::INT16: - ret.reset(CREATEINTSET(int16_t, baseFileName, info)); + ret.reset(CREATEINTSET(int16_t, name, info)); break; case BasicType::INT32: - ret.reset(CREATEINTSET(int32_t, baseFileName, info)); + ret.reset(CREATEINTSET(int32_t, name, info)); break; case BasicType::INT64: - ret.reset(CREATEINTSET(int64_t, baseFileName, info)); + ret.reset(CREATEINTSET(int64_t, name, info)); break; case BasicType::FLOAT: - ret.reset(CREATEFLOATSET(float, baseFileName, info)); + ret.reset(CREATEFLOATSET(float, name, info)); break; case BasicType::DOUBLE: - ret.reset(CREATEFLOATSET(double, baseFileName, info)); + ret.reset(CREATEFLOATSET(double, name, info)); break; case BasicType::STRING: - ret.reset(static_cast<AttributeVector *>(new WeightedSetStringPostingAttribute(baseFileName, info))); + ret.reset(static_cast<AttributeVector *>(new WeightedSetStringPostingAttribute(name, info))); break; default: break; diff --git a/searchlib/src/vespa/searchlib/attribute/createsetstd.cpp b/searchlib/src/vespa/searchlib/attribute/createsetstd.cpp index 8ef75de2b44..74e478c67b8 100644 --- a/searchlib/src/vespa/searchlib/attribute/createsetstd.cpp +++ b/searchlib/src/vespa/searchlib/attribute/createsetstd.cpp @@ -21,7 +21,7 @@ using attribute::BasicType; AttributeVector::SP -AttributeFactory::createSetStd(const vespalib::string & baseFileName, const Config & info) +AttributeFactory::createSetStd(stringref name, const Config & info) { assert(info.collectionType().type() == attribute::CollectionType::WSET); AttributeVector::SP ret; @@ -31,25 +31,25 @@ AttributeFactory::createSetStd(const vespalib::string & baseFileName, const Conf case BasicType::UINT4: break; case BasicType::INT8: - ret.reset(CREATEINTSET(int8_t, baseFileName, info)); + ret.reset(CREATEINTSET(int8_t, name, info)); break; case BasicType::INT16: - ret.reset(CREATEINTSET(int16_t, baseFileName, info)); + ret.reset(CREATEINTSET(int16_t, name, info)); break; case BasicType::INT32: - ret.reset(CREATEINTSET(int32_t, baseFileName, info)); + ret.reset(CREATEINTSET(int32_t, name, info)); break; case BasicType::INT64: - ret.reset(CREATEINTSET(int64_t, baseFileName, info)); + ret.reset(CREATEINTSET(int64_t, name, info)); break; case BasicType::FLOAT: - ret.reset(CREATEFLOATSET(float, baseFileName, info)); + ret.reset(CREATEFLOATSET(float, name, info)); break; case BasicType::DOUBLE: - ret.reset(CREATEFLOATSET(double, baseFileName, info)); + ret.reset(CREATEFLOATSET(double, name, info)); break; case BasicType::STRING: - ret.reset(static_cast<AttributeVector *>(new WeightedSetStringAttribute(baseFileName, info))); + ret.reset(static_cast<AttributeVector *>(new WeightedSetStringAttribute(name, info))); break; default: break; diff --git a/searchlib/src/vespa/searchlib/attribute/createsinglefastsearch.cpp b/searchlib/src/vespa/searchlib/attribute/createsinglefastsearch.cpp index ec200124286..f3dd642828e 100644 --- a/searchlib/src/vespa/searchlib/attribute/createsinglefastsearch.cpp +++ b/searchlib/src/vespa/searchlib/attribute/createsinglefastsearch.cpp @@ -23,7 +23,7 @@ namespace search { using attribute::BasicType; AttributeVector::SP -AttributeFactory::createSingleFastSearch(const vespalib::string & baseFileName, const Config & info) +AttributeFactory::createSingleFastSearch(stringref name, const Config & info) { assert(info.collectionType().type() == attribute::CollectionType::SINGLE); assert(info.fastSearch()); @@ -34,25 +34,25 @@ AttributeFactory::createSingleFastSearch(const vespalib::string & baseFileName, case BasicType::UINT4: break; case BasicType::INT8: - ret.reset(new INTPOSTING(int8_t)(baseFileName, info)); + ret.reset(new INTPOSTING(int8_t)(name, info)); break; case BasicType::INT16: - ret.reset(new INTPOSTING(int16_t)(baseFileName, info)); + ret.reset(new INTPOSTING(int16_t)(name, info)); break; case BasicType::INT32: - ret.reset(new INTPOSTING(int32_t)(baseFileName, info)); + ret.reset(new INTPOSTING(int32_t)(name, info)); break; case BasicType::INT64: - ret.reset(new INTPOSTING(int64_t)(baseFileName, info)); + ret.reset(new INTPOSTING(int64_t)(name, info)); break; case BasicType::FLOAT: - ret.reset(new FLOATPOSTING(float)(baseFileName, info)); + ret.reset(new FLOATPOSTING(float)(name, info)); break; case BasicType::DOUBLE: - ret.reset(new FLOATPOSTING(double)(baseFileName, info)); + ret.reset(new FLOATPOSTING(double)(name, info)); break; case BasicType::STRING: - ret.reset(new SingleValueStringPostingAttribute(baseFileName, info)); + ret.reset(new SingleValueStringPostingAttribute(name, info)); break; default: break; diff --git a/searchlib/src/vespa/searchlib/attribute/createsinglestd.cpp b/searchlib/src/vespa/searchlib/attribute/createsinglestd.cpp index 6fefc7a1852..cc3d9fe4b37 100644 --- a/searchlib/src/vespa/searchlib/attribute/createsinglestd.cpp +++ b/searchlib/src/vespa/searchlib/attribute/createsinglestd.cpp @@ -18,54 +18,54 @@ namespace search { using attribute::BasicType; AttributeVector::SP -AttributeFactory::createSingleStd(const vespalib::string & baseFileName, const Config & info) +AttributeFactory::createSingleStd(stringref name, const Config & info) { assert(info.collectionType().type() == attribute::CollectionType::SINGLE); AttributeVector::SP ret; switch(info.basicType().type()) { case BasicType::UINT1: - ret.reset(new SingleValueBitNumericAttribute(baseFileName, info.getGrowStrategy())); + ret.reset(new SingleValueBitNumericAttribute(name, info.getGrowStrategy())); break; case BasicType::UINT2: - ret.reset(new SingleValueSemiNibbleNumericAttribute(baseFileName, info.getGrowStrategy())); + ret.reset(new SingleValueSemiNibbleNumericAttribute(name, info.getGrowStrategy())); break; case BasicType::UINT4: - ret.reset(new SingleValueNibbleNumericAttribute(baseFileName, info.getGrowStrategy())); + ret.reset(new SingleValueNibbleNumericAttribute(name, info.getGrowStrategy())); break; case BasicType::INT8: - ret.reset(new SingleValueNumericAttribute<IntegerAttributeTemplate<int8_t> >(baseFileName, info)); + ret.reset(new SingleValueNumericAttribute<IntegerAttributeTemplate<int8_t> >(name, info)); break; case BasicType::INT16: // XXX: Unneeded since we don't have short document fields in java. - ret.reset(new SingleValueNumericAttribute<IntegerAttributeTemplate<int16_t> >(baseFileName, info)); + ret.reset(new SingleValueNumericAttribute<IntegerAttributeTemplate<int16_t> >(name, info)); break; case BasicType::INT32: - ret.reset(new SingleValueNumericAttribute<IntegerAttributeTemplate<int32_t> >(baseFileName, info)); + ret.reset(new SingleValueNumericAttribute<IntegerAttributeTemplate<int32_t> >(name, info)); break; case BasicType::INT64: - ret.reset(new SingleValueNumericAttribute<IntegerAttributeTemplate<int64_t> >(baseFileName, info)); + ret.reset(new SingleValueNumericAttribute<IntegerAttributeTemplate<int64_t> >(name, info)); break; case BasicType::FLOAT: - ret.reset(new SingleValueNumericAttribute<FloatingPointAttributeTemplate<float> >(baseFileName, info)); + ret.reset(new SingleValueNumericAttribute<FloatingPointAttributeTemplate<float> >(name, info)); break; case BasicType::DOUBLE: - ret.reset(new SingleValueNumericAttribute<FloatingPointAttributeTemplate<double> >(baseFileName, info)); + ret.reset(new SingleValueNumericAttribute<FloatingPointAttributeTemplate<double> >(name, info)); break; case BasicType::STRING: - ret.reset(new SingleValueStringAttribute(baseFileName, info)); + ret.reset(new SingleValueStringAttribute(name, info)); break; case BasicType::PREDICATE: - ret.reset(new PredicateAttribute(baseFileName, info)); + ret.reset(new PredicateAttribute(name, info)); break; case BasicType::TENSOR: if (info.tensorType().is_dense()) { - ret.reset(new tensor::DenseTensorAttribute(baseFileName, info)); + ret.reset(new tensor::DenseTensorAttribute(name, info)); } else { - ret.reset(new tensor::GenericTensorAttribute(baseFileName, info)); + ret.reset(new tensor::GenericTensorAttribute(name, info)); } break; case BasicType::REFERENCE: - ret = std::make_shared<attribute::ReferenceAttribute>(baseFileName, info); + ret = std::make_shared<attribute::ReferenceAttribute>(name, info); break; default: break; diff --git a/searchlib/src/vespa/searchlib/attribute/extendableattributes.h b/searchlib/src/vespa/searchlib/attribute/extendableattributes.h index dde8dda46f7..ce57daa775d 100644 --- a/searchlib/src/vespa/searchlib/attribute/extendableattributes.h +++ b/searchlib/src/vespa/searchlib/attribute/extendableattributes.h @@ -108,7 +108,7 @@ protected: using QueryTermSimpleUP = AttributeVector::QueryTermSimpleUP; MultiExtAttribute(const vespalib::string &name, const attribute::CollectionType &ctype) - : Super(name, Config(BasicType::fromType(T()), ctype)) + : Super(name, Config(BasicType::fromType(T()), ctype)) { } private: AttributeVector::SearchContext::UP diff --git a/searchlib/src/vespa/searchlib/attribute/flagattribute.cpp b/searchlib/src/vespa/searchlib/attribute/flagattribute.cpp index 0e5027ea46e..449ec0cd86d 100644 --- a/searchlib/src/vespa/searchlib/attribute/flagattribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/flagattribute.cpp @@ -65,10 +65,10 @@ FlagAttributeT<B>::getSearch(QueryTermSimple::UP qTerm, template <typename B> void FlagAttributeT<B>::clearOldValues(DocId doc) { - const typename B::WType * values(NULL); + const typename B::WType * values(nullptr); for (uint32_t i(0), m(this->get(doc, values)); i < m; i++) { BitVector * bv = _bitVectors[getOffset(values[i].value())]; - if (bv != NULL) { + if (bv != nullptr) { bv->clearBit(doc); } } diff --git a/searchlib/src/vespa/searchlib/attribute/iattributefilewriter.h b/searchlib/src/vespa/searchlib/attribute/iattributefilewriter.h index 7de103ee770..bb00124c9fc 100644 --- a/searchlib/src/vespa/searchlib/attribute/iattributefilewriter.h +++ b/searchlib/src/vespa/searchlib/attribute/iattributefilewriter.h @@ -4,8 +4,7 @@ #include <vespa/vespalib/data/databuffer.h> -namespace search -{ +namespace search { class BufferWriter; diff --git a/searchlib/src/vespa/searchlib/attribute/iattributesavetarget.h b/searchlib/src/vespa/searchlib/attribute/iattributesavetarget.h index 838e9a78744..9f90544bb83 100644 --- a/searchlib/src/vespa/searchlib/attribute/iattributesavetarget.h +++ b/searchlib/src/vespa/searchlib/attribute/iattributesavetarget.h @@ -2,9 +2,6 @@ #pragma once -#include <vespa/vespalib/stllike/string.h> -#include <vespa/vespalib/data/databuffer.h> -#include <stdint.h> #include "iattributefilewriter.h" #include "attribute_header.h" @@ -21,6 +18,7 @@ protected: public: IAttributeSaveTarget() : _header() {} void setHeader(const attribute::AttributeHeader & header) { _header = header; } + const attribute::AttributeHeader & getHeader() const { return _header; } bool getEnumerated() const { return _header.getEnumerated(); } diff --git a/searchlib/src/vespa/searchlib/attribute/multienumattribute.h b/searchlib/src/vespa/searchlib/attribute/multienumattribute.h index b58ed8c4ae8..bcad27f046e 100644 --- a/searchlib/src/vespa/searchlib/attribute/multienumattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/multienumattribute.h @@ -2,9 +2,9 @@ #pragma once -#include <vespa/searchlib/attribute/multivalueattribute.h> -#include <vespa/searchlib/attribute/enumstorebase.h> -#include <vespa/searchlib/attribute/loadedenumvalue.h> +#include "multivalueattribute.h" +#include "enumstorebase.h" +#include "loadedenumvalue.h" namespace search { diff --git a/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp index d5432bf56de..158343ef7c0 100644 --- a/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp @@ -2,8 +2,8 @@ #pragma once -#include <vespa/searchlib/attribute/multienumattribute.h> -#include <vespa/searchlib/attribute/multivalueattribute.hpp> +#include "multienumattribute.h" +#include "multivalueattribute.hpp" #include "multienumattributesaver.h" #include "load_utils.h" diff --git a/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.hpp index 004c754bb3a..d94a59555bd 100644 --- a/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.hpp @@ -16,8 +16,7 @@ using fileutil::LoadedBuffer; template <typename B, typename M> MultiValueNumericEnumAttribute<B, M>:: -MultiValueNumericEnumAttribute(const vespalib::string & baseFileName, - const AttributeVector::Config & cfg) +MultiValueNumericEnumAttribute(const vespalib::string & baseFileName, const AttributeVector::Config & cfg) : MultiValueEnumAttribute<B, M>(baseFileName, cfg) { } @@ -70,14 +69,10 @@ MultiValueNumericEnumAttribute<B, M>::onLoadEnumerated(ReaderBase &attrReader) EnumVector enumHist; if (this->hasPostings()) { loaded.reserve(numValues); - this->fillEnumIdx(attrReader, - eidxs, - loaded); + this->fillEnumIdx(attrReader, eidxs, loaded); } else { EnumVector(eidxs.size(), 0).swap(enumHist); - this->fillEnumIdx(attrReader, - eidxs, - enumHist); + this->fillEnumIdx(attrReader, eidxs, enumHist); } EnumIndexVector().swap(eidxs); if (this->hasPostings()) { @@ -131,15 +126,15 @@ MultiValueNumericEnumAttribute<B, M>::getSearch(QueryTermSimple::UP qTerm, QueryTermSimple::RangeResult<T> res = qTerm->getRange<T>(); if (this->hasArrayType()) { if (res.isEqual()) { - return AttributeVector::SearchContext::UP(new ArraySearchContext(std::move(qTerm), *this)); + return std::make_unique<ArraySearchContext>(std::move(qTerm), *this); } else { - return AttributeVector::SearchContext::UP(new ArraySearchContext(std::move(qTerm), *this)); + return std::make_unique<ArraySearchContext>(std::move(qTerm), *this); } } else { if (res.isEqual()) { - return AttributeVector::SearchContext::UP(new SetSearchContext(std::move(qTerm), *this)); + return std::make_unique<SetSearchContext>(std::move(qTerm), *this); } else { - return AttributeVector::SearchContext::UP(new SetSearchContext(std::move(qTerm), *this)); + return std::make_unique<SetSearchContext>(std::move(qTerm), *this); } } } @@ -162,7 +157,7 @@ std::unique_ptr<queryeval::SearchIterator> MultiValueNumericEnumAttribute<B, M>::SetSearchContext::createFilterIterator(fef::TermFieldMatchData * matchData, bool strict) { if (!valid()) { - return queryeval::SearchIterator::UP(new queryeval::EmptySearch()); + return std::make_unique<queryeval::EmptySearch>(); } if (getIsFilter()) { return queryeval::SearchIterator::UP @@ -181,7 +176,7 @@ std::unique_ptr<queryeval::SearchIterator> MultiValueNumericEnumAttribute<B, M>::ArraySearchContext::createFilterIterator(fef::TermFieldMatchData * matchData, bool strict) { if (!valid()) { - return queryeval::SearchIterator::UP(new queryeval::EmptySearch()); + return std::make_unique<queryeval::EmptySearch>(); } if (getIsFilter()) { return queryeval::SearchIterator::UP diff --git a/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp b/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp index 424405a520e..3d197fc59cd 100644 --- a/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp @@ -1,12 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "predicate_attribute.h" +#include "iattributesavetarget.h" +#include "attribute_header.h" #include <vespa/searchlib/util/fileutil.h> #include <vespa/document/fieldvalue/predicatefieldvalue.h> #include <vespa/document/predicate/predicate.h> #include <vespa/vespalib/data/slime/slime.h> -#include "iattributesavetarget.h" -#include "attribute_header.h" #include <vespa/log/log.h> LOG_SETUP(".searchlib.attribute.predicate_attribute"); @@ -154,8 +154,7 @@ struct DocIdLimitFinderAndMinFeatureFiller : SimpleIndexDeserializeObserver<> { uint32_t _highest_doc_id; V & _min_feature; PredicateIndex &_index; - DocIdLimitFinderAndMinFeatureFiller(V & min_feature, - PredicateIndex &index) : + DocIdLimitFinderAndMinFeatureFiller(V & min_feature, PredicateIndex &index) : _highest_doc_id(0), _min_feature(min_feature), _index(index) @@ -195,15 +194,13 @@ bool PredicateAttribute::onLoad() DocId highest_doc_id; if (version == 0) { DocIdLimitFinderAndMinFeatureFiller<MinFeatureVector> observer(_min_feature, *_index); - _index.reset(new PredicateIndex(getGenerationHandler(), getGenerationHolder(), - _limit_provider, createSimpleIndexConfig(getConfig()), - buffer, observer, 0)); + _index = std::make_unique<PredicateIndex>(getGenerationHandler(), getGenerationHolder(), _limit_provider, + createSimpleIndexConfig(getConfig()), buffer, observer, 0); highest_doc_id = observer._highest_doc_id; } else { DummyObserver observer; - _index.reset( - new PredicateIndex(getGenerationHandler(), getGenerationHolder(), _limit_provider, - createSimpleIndexConfig(getConfig()), buffer, observer, version)); + _index = std::make_unique<PredicateIndex>(getGenerationHandler(), getGenerationHolder(), _limit_provider, + createSimpleIndexConfig(getConfig()), buffer, observer, version); highest_doc_id = buffer.readInt32(); // Deserialize min feature vector _min_feature.ensure_size(highest_doc_id + 1, PredicateAttribute::MIN_FEATURE_FILL); @@ -264,8 +261,7 @@ PredicateAttribute::updateValue(uint32_t doc_id, const PredicateFieldValue &valu return; } PredicateTreeAnnotations result; - PredicateTreeAnnotator::annotate(inspector, result, - _lower_bound, _upper_bound); + PredicateTreeAnnotator::annotate(inspector, result, _lower_bound, _upper_bound); _index->indexDocument(doc_id, result); assert(result.min_feature <= MAX_MIN_FEATURE); uint8_t minFeature = static_cast<uint8_t>(result.min_feature); diff --git a/searchlib/src/vespa/searchlib/attribute/predicate_attribute.h b/searchlib/src/vespa/searchlib/attribute/predicate_attribute.h index f318e39f4fa..18a3664a8bd 100644 --- a/searchlib/src/vespa/searchlib/attribute/predicate_attribute.h +++ b/searchlib/src/vespa/searchlib/attribute/predicate_attribute.h @@ -55,8 +55,7 @@ public: uint32_t clearDoc(DocId doc_id) override; uint32_t getValueCount(DocId doc) const override; - void updateValue(uint32_t doc_id, - const document::PredicateFieldValue &value); + void updateValue(uint32_t doc_id, const document::PredicateFieldValue &value); /** * Will return a handle with a pointer to the min_features and how many there are. @@ -97,8 +96,7 @@ public: static constexpr uint8_t MIN_FEATURE_FILL = 255; static constexpr uint32_t PREDICATE_ATTRIBUTE_VERSION = 2; - virtual uint32_t getVersion() const override; - + uint32_t getVersion() const override; }; } // namespace search diff --git a/searchlib/src/vespa/searchlib/attribute/readerbase.cpp b/searchlib/src/vespa/searchlib/attribute/readerbase.cpp index a8c042c1a44..62936ecaaf4 100644 --- a/searchlib/src/vespa/searchlib/attribute/readerbase.cpp +++ b/searchlib/src/vespa/searchlib/attribute/readerbase.cpp @@ -97,7 +97,7 @@ ReaderBase::ReaderBase(AttributeVector &attr) } -ReaderBase::~ReaderBase() { } +ReaderBase::~ReaderBase() = default; bool ReaderBase::hasWeight() const { diff --git a/searchlib/src/vespa/searchlib/attribute/readerbase.h b/searchlib/src/vespa/searchlib/attribute/readerbase.h index e1c3b64b8a9..09db52f5e25 100644 --- a/searchlib/src/vespa/searchlib/attribute/readerbase.h +++ b/searchlib/src/vespa/searchlib/attribute/readerbase.h @@ -32,8 +32,7 @@ public: } static bool - extractFileSize(const vespalib::GenericHeader &header, - FastOS_FileInterface &file, uint64_t &fileSize); + extractFileSize(const vespalib::GenericHeader &header, FastOS_FileInterface &file, uint64_t &fileSize); size_t getNumValues(); int32_t getNextWeight() { return _weightReader.readHostOrder(); } diff --git a/searchlib/src/vespa/searchlib/attribute/reference_attribute.h b/searchlib/src/vespa/searchlib/attribute/reference_attribute.h index 004339573bc..1670ebdc969 100644 --- a/searchlib/src/vespa/searchlib/attribute/reference_attribute.h +++ b/searchlib/src/vespa/searchlib/attribute/reference_attribute.h @@ -67,9 +67,9 @@ public: DECLARE_IDENTIFIABLE_ABSTRACT(ReferenceAttribute); ReferenceAttribute(const vespalib::stringref baseFileName, const Config & cfg); - virtual ~ReferenceAttribute(); - virtual bool addDoc(DocId &doc) override; - virtual uint32_t clearDoc(DocId doc) override; + ~ReferenceAttribute() override; + bool addDoc(DocId &doc) override; + uint32_t clearDoc(DocId doc) override; void update(DocId doc, const GlobalId &gid); const Reference *getReference(DocId doc); void setGidToLidMapperFactory(std::shared_ptr<IGidToLidMapperFactory> gidToLidMapperFactory); @@ -83,8 +83,8 @@ public: void notifyReferencedPut(const GlobalId &gid, DocId targetLid); void notifyReferencedRemove(const GlobalId &gid); void populateTargetLids(); - virtual void clearDocs(DocId lidLow, DocId lidLimit) override; - virtual void onShrinkLidSpace() override; + void clearDocs(DocId lidLow, DocId lidLimit) override; + void onShrinkLidSpace() override; template <typename FunctionType> void diff --git a/searchlib/src/vespa/searchlib/attribute/stringbase.cpp b/searchlib/src/vespa/searchlib/attribute/stringbase.cpp index 04ca9d216a3..855bf6c7a57 100644 --- a/searchlib/src/vespa/searchlib/attribute/stringbase.cpp +++ b/searchlib/src/vespa/searchlib/attribute/stringbase.cpp @@ -231,13 +231,13 @@ StringAttribute::StringSearchContext::StringSearchContext(QueryTermSimple::UP qT _buffer() { if (isRegex()) { - _regex.reset(new Regexp(_queryTerm->getTerm(), Regexp::Flags().enableICASE())); + _regex = std::make_unique<Regexp>(_queryTerm->getTerm(), Regexp::Flags().enableICASE()); } } StringAttribute::StringSearchContext::~StringSearchContext() { - if (_buffer != NULL) { + if (_buffer != nullptr) { delete [] _buffer; } } @@ -386,13 +386,9 @@ StringAttribute::onLoadEnumerated(ReaderBase &attrReader) timer.SetNow(); LOG(debug, "start fillEnumIdx"); if(hasPostings()) { - fillEnumIdx(attrReader, - eidxs, - loaded); + fillEnumIdx(attrReader, eidxs, loaded); } else { - fillEnumIdx(attrReader, - eidxs, - enumHist); + fillEnumIdx(attrReader, eidxs, enumHist); } LOG(debug, "done fillEnumIdx, %8.3f s elapsed", timer.MilliSecsToNow() / 1000); @@ -405,8 +401,7 @@ StringAttribute::onLoadEnumerated(ReaderBase &attrReader) attribute::sortLoadedByEnum(loaded); - LOG(debug, "done sort loaded, %8.3f s elapsed", - timer.MilliSecsToNow() / 1000); + LOG(debug, "done sort loaded, %8.3f s elapsed", timer.MilliSecsToNow() / 1000); LOG(debug, "start fillPostingsFixupEnum"); timer.SetNow(); @@ -473,74 +468,53 @@ bool StringAttribute::onLoad() } bool -StringAttribute::onAddDoc(DocId doc) +StringAttribute::onAddDoc(DocId ) { - (void) doc; return false; } -void StringAttribute::fillPostings(LoadedVector & loaded) +void StringAttribute::fillPostings(LoadedVector &) { - (void) loaded; } -void StringAttribute::fillEnum(LoadedVector & loaded) +void StringAttribute::fillEnum(LoadedVector &) { - (void) loaded; } -void StringAttribute::fillValues(LoadedVector & loaded) +void StringAttribute::fillValues(LoadedVector & ) { - (void) loaded; } void -StringAttribute::fillEnum0(const void *src, - size_t srcLen, - EnumIndexVector &eidxs) +StringAttribute::fillEnum0(const void *, size_t , EnumIndexVector &) { - (void) src; - (void) srcLen; - (void) eidxs; fprintf(stderr, "StringAttribute::fillEnum0\n"); } void -StringAttribute::fillEnumIdx(ReaderBase &attrReader, - const EnumIndexVector &eidxs, - LoadedEnumAttributeVector &loaded) +StringAttribute::fillEnumIdx(ReaderBase &, const EnumIndexVector &, LoadedEnumAttributeVector &) { - (void) attrReader; - (void) eidxs; - (void) loaded; fprintf(stderr, "StringAttribute::fillEnumIdx (loaded)\n"); } void -StringAttribute::fillEnumIdx(ReaderBase &attrReader, - const EnumIndexVector &eidxs, - EnumVector &enumHist) +StringAttribute::fillEnumIdx(ReaderBase &, const EnumIndexVector &, EnumVector &) { - (void) attrReader; - (void) eidxs; - (void) enumHist; fprintf(stderr, "StringAttribute::fillEnumIdx (enumHist)\n"); } void -StringAttribute::fillPostingsFixupEnum(const LoadedEnumAttributeVector &loaded) +StringAttribute::fillPostingsFixupEnum(const LoadedEnumAttributeVector &) { - (void) loaded; fprintf(stderr, "StringAttribute::fillPostingsFixupEnum\n"); } void -StringAttribute::fixupEnumRefCounts(const EnumVector &enumHist) +StringAttribute::fixupEnumRefCounts(const EnumVector &) { - (void) enumHist; fprintf(stderr, "StringAttribute::fixupEnumRefCounts\n"); } diff --git a/searchlib/src/vespa/searchlib/features/attributefeature.cpp b/searchlib/src/vespa/searchlib/features/attributefeature.cpp index e7f4ab0e352..b3ebd0f3822 100644 --- a/searchlib/src/vespa/searchlib/features/attributefeature.cpp +++ b/searchlib/src/vespa/searchlib/features/attributefeature.cpp @@ -274,9 +274,7 @@ AttributeBlueprint::AttributeBlueprint() : { } -AttributeBlueprint::~AttributeBlueprint() -{ -} +AttributeBlueprint::~AttributeBlueprint() = default; void AttributeBlueprint::visitDumpFeatures(const search::fef::IIndexEnvironment &, diff --git a/searchlib/src/vespa/searchlib/tensor/dense_tensor_attribute.cpp b/searchlib/src/vespa/searchlib/tensor/dense_tensor_attribute.cpp index 932d5a8d038..e669e31e2cf 100644 --- a/searchlib/src/vespa/searchlib/tensor/dense_tensor_attribute.cpp +++ b/searchlib/src/vespa/searchlib/tensor/dense_tensor_attribute.cpp @@ -16,9 +16,7 @@ using vespalib::tensor::MutableDenseTensorView; using vespalib::tensor::Tensor; using vespalib::tensor::TensorMapper; -namespace search { - -namespace tensor { +namespace search::tensor { namespace { @@ -59,7 +57,7 @@ TensorReader::TensorReader(AttributeVector &attr) } _unboundDimSizes.resize(_numUnboundDims); } -TensorReader::~TensorReader() { } +TensorReader::~TensorReader() = default; size_t TensorReader::getNumCells() { @@ -73,8 +71,7 @@ TensorReader::getNumCells() { } size_t numCells = _numBoundCells; if (_numUnboundDims != 0) { - _datFile->ReadBuf(&_unboundDimSizes[0], - _numUnboundDims * sizeof(uint32_t)); + _datFile->ReadBuf(&_unboundDimSizes[0], _numUnboundDims * sizeof(uint32_t)); for (auto i = 0u; i < _numUnboundDims; ++i) { assert(_unboundDimSizes[i] != 0u); numCells *= _unboundDimSizes[i]; @@ -189,6 +186,4 @@ DenseTensorAttribute::getVersion() const return DENSE_TENSOR_ATTRIBUTE_VERSION; } -} // namespace search::tensor - -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/tensor/dense_tensor_attribute_saver.h b/searchlib/src/vespa/searchlib/tensor/dense_tensor_attribute_saver.h index 8799bed2329..1f6596e82f5 100644 --- a/searchlib/src/vespa/searchlib/tensor/dense_tensor_attribute_saver.h +++ b/searchlib/src/vespa/searchlib/tensor/dense_tensor_attribute_saver.h @@ -2,12 +2,10 @@ #pragma once -#include <vespa/searchlib/attribute/attributesaver.h> #include "tensor_attribute.h" +#include <vespa/searchlib/attribute/attributesaver.h> -namespace search { - -namespace tensor { +namespace search::tensor { class DenseTensorStore; @@ -23,16 +21,12 @@ private: const DenseTensorStore &_tensorStore; using GenerationHandler = vespalib::GenerationHandler; - virtual bool onSave(IAttributeSaveTarget &saveTarget) override; + bool onSave(IAttributeSaveTarget &saveTarget) override; public: - DenseTensorAttributeSaver(GenerationHandler::Guard &&guard, - const attribute::AttributeHeader &header, - RefCopyVector &&refs, - const DenseTensorStore &tensorStore); + DenseTensorAttributeSaver(GenerationHandler::Guard &&guard, const attribute::AttributeHeader &header, + RefCopyVector &&refs, const DenseTensorStore &tensorStore); - virtual ~DenseTensorAttributeSaver(); + ~DenseTensorAttributeSaver() override; }; -} // namespace search::tensor - -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/tensor/generic_tensor_attribute.cpp b/searchlib/src/vespa/searchlib/tensor/generic_tensor_attribute.cpp index 31eab642a77..77abe876d87 100644 --- a/searchlib/src/vespa/searchlib/tensor/generic_tensor_attribute.cpp +++ b/searchlib/src/vespa/searchlib/tensor/generic_tensor_attribute.cpp @@ -13,9 +13,7 @@ using vespalib::eval::ValueType; using vespalib::tensor::Tensor; using vespalib::tensor::TensorMapper; -namespace search { - -namespace tensor { +namespace search::tensor { namespace { @@ -36,8 +34,8 @@ public: } -GenericTensorAttribute::GenericTensorAttribute(vespalib::stringref baseFileName, const Config &cfg) - : TensorAttribute(baseFileName, cfg, _genericTensorStore) +GenericTensorAttribute::GenericTensorAttribute(stringref name, const Config &cfg) + : TensorAttribute(name, cfg, _genericTensorStore) { } @@ -120,7 +118,4 @@ GenericTensorAttribute::compactWorst() doCompactWorst<GenericTensorStore::RefType>(); } - -} // namespace search::tensor - -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp index 494c12dcff9..ac7dac0086a 100644 --- a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp +++ b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp @@ -38,10 +38,8 @@ shouldCreateMapper(const ValueType &tensorType) } -TensorAttribute::TensorAttribute(vespalib::stringref baseFileName, - const Config &cfg, - TensorStore &tensorStore) - : NotImplementedAttribute(baseFileName, cfg), +TensorAttribute::TensorAttribute(vespalib::stringref name, const Config &cfg, TensorStore &tensorStore) + : NotImplementedAttribute(name, cfg), _refVector(cfg.getGrowStrategy().getDocsInitialCapacity(), cfg.getGrowStrategy().getDocsGrowPercent(), cfg.getGrowStrategy().getDocsGrowDelta(), @@ -56,9 +54,7 @@ TensorAttribute::TensorAttribute(vespalib::stringref baseFileName, } -TensorAttribute::~TensorAttribute() -{ -} +TensorAttribute::~TensorAttribute() = default; const ITensorAttribute * TensorAttribute::asTensorAttribute() const diff --git a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.h b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.h index 1ef3d8a1a96..c50ec0b2f72 100644 --- a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.h +++ b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.h @@ -8,9 +8,7 @@ #include <vespa/searchlib/common/rcuvector.h> #include <vespa/eval/tensor/tensor_mapper.h> -namespace search { - -namespace tensor { +namespace search::tensor { /** * Attribute vector class used to store tensors for all documents in memory. @@ -32,28 +30,24 @@ protected: public: DECLARE_IDENTIFIABLE_ABSTRACT(TensorAttribute); using RefCopyVector = vespalib::Array<RefType>; - TensorAttribute(vespalib::stringref baseFileName, const Config &cfg, - TensorStore &tensorStore); - virtual ~TensorAttribute(); - virtual const ITensorAttribute *asTensorAttribute() const override; - - virtual uint32_t clearDoc(DocId docId) override; - virtual void onCommit() override; - virtual void onUpdateStat() override; - virtual void removeOldGenerations(generation_t firstUsed) override; - virtual void onGenerationChange(generation_t generation) override; - virtual bool addDoc(DocId &docId) override; - virtual std::unique_ptr<Tensor> getEmptyTensor() const override; - virtual vespalib::eval::ValueType getTensorType() const override; - virtual void clearDocs(DocId lidLow, DocId lidLimit) override; - virtual void onShrinkLidSpace() override; - virtual uint32_t getVersion() const override; + TensorAttribute(vespalib::stringref name, const Config &cfg, TensorStore &tensorStore); + ~TensorAttribute() override; + const ITensorAttribute *asTensorAttribute() const override; + + uint32_t clearDoc(DocId docId) override; + void onCommit() override; + void onUpdateStat() override; + void removeOldGenerations(generation_t firstUsed) override; + void onGenerationChange(generation_t generation) override; + bool addDoc(DocId &docId) override; + std::unique_ptr<Tensor> getEmptyTensor() const override; + vespalib::eval::ValueType getTensorType() const override; + void clearDocs(DocId lidLow, DocId lidLimit) override; + void onShrinkLidSpace() override; + uint32_t getVersion() const override; RefCopyVector getRefCopy() const; virtual void setTensor(DocId docId, const Tensor &tensor) = 0; virtual void compactWorst() = 0; }; - -} // namespace search::tensor - -} // namespace search +} 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 2806631be18..c466fe50d6f 100644 --- a/vespajlib/src/main/java/com/yahoo/path/Path.java +++ b/vespajlib/src/main/java/com/yahoo/path/Path.java @@ -84,7 +84,6 @@ 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() { |