diff options
76 files changed, 851 insertions, 610 deletions
diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index 9df68821454..fd229b35778 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -529,15 +529,14 @@ "public static final enum com.yahoo.config.application.api.ValidationId indexModeChange", "public static final enum com.yahoo.config.application.api.ValidationId fieldTypeChange", "public static final enum com.yahoo.config.application.api.ValidationId clusterSizeReduction", + "public static final enum com.yahoo.config.application.api.ValidationId tensorTypeChange", "public static final enum com.yahoo.config.application.api.ValidationId resourcesReduction", "public static final enum com.yahoo.config.application.api.ValidationId contentTypeRemoval", "public static final enum com.yahoo.config.application.api.ValidationId contentClusterRemoval", "public static final enum com.yahoo.config.application.api.ValidationId deploymentRemoval", - "public static final enum com.yahoo.config.application.api.ValidationId skipAutomaticTenantUpgradeTests", "public static final enum com.yahoo.config.application.api.ValidationId globalDocumentChange", "public static final enum com.yahoo.config.application.api.ValidationId configModelVersionMismatch", "public static final enum com.yahoo.config.application.api.ValidationId skipOldConfigModels", - "public static final enum com.yahoo.config.application.api.ValidationId forceAutomaticTenantUpgradeTests", "public static final enum com.yahoo.config.application.api.ValidationId accessControl", "public static final enum com.yahoo.config.application.api.ValidationId globalEndpointChange" ] @@ -577,10 +576,7 @@ "attributes": [ "public" ], - "methods": [ - "public com.yahoo.config.application.api.ValidationId validationId()", - "public java.lang.String getMessage()" - ], + "methods": [], "fields": [] }, "com.yahoo.config.application.api.ValidationOverrides": { @@ -591,6 +587,7 @@ ], "methods": [ "public void <init>(java.util.List)", + "public void invalid(java.util.Map, java.time.Instant)", "public void invalid(com.yahoo.config.application.api.ValidationId, java.lang.String, java.time.Instant)", "public boolean allows(java.lang.String, java.time.Instant)", "public boolean allows(com.yahoo.config.application.api.ValidationId, java.time.Instant)", diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationId.java b/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationId.java index c0bae137b0d..4c76d42a17e 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationId.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationId.java @@ -14,15 +14,14 @@ public enum ValidationId { indexModeChange("indexing-mode-change"), // Changing the index mode (streaming, indexed, store-only) of documents fieldTypeChange("field-type-change"), // Field type changes clusterSizeReduction("cluster-size-reduction"), // Large reductions in cluster size + tensorTypeChange("tensor-type-change"), // Tensor type change resourcesReduction("resources-reduction"), // Large reductions in node resources contentTypeRemoval("content-type-removal"), // Removal of a data type (causes deletion of all data) contentClusterRemoval("content-cluster-removal"), // Removal (or id change) of content clusters deploymentRemoval("deployment-removal"), // Removal of production zones from deployment.xml - skipAutomaticTenantUpgradeTests("skip-automatic-tenant-upgrade-test"), // Skip platform supplied staging tests globalDocumentChange("global-document-change"), // Changing global attribute for document types in content clusters configModelVersionMismatch("config-model-version-mismatch"), // Internal use skipOldConfigModels("skip-old-config-models"), // Internal use - forceAutomaticTenantUpgradeTests("force-automatic-tenant-upgrade-test"), // Internal use accessControl("access-control"), // Internal use, used in zones where there should be no access-control globalEndpointChange("global-endpoint-change"); // Changing global endpoints diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationOverrides.java b/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationOverrides.java index a85e109f731..c1f30d6788e 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationOverrides.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationOverrides.java @@ -14,9 +14,12 @@ import java.time.LocalDate; import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.logging.Level; +import java.util.stream.Collectors; /** * A set of allows which suppresses specific validations in limited time periods. @@ -47,6 +50,14 @@ public class ValidationOverrides { this.xmlForm = xmlForm; } + /** Throws a ValidationException unless all given validation is overridden at this time */ + public void invalid(Map<ValidationId, List<String>> messagesByValidationId, Instant now) { + Map<ValidationId, List<String>> disallowed = new HashMap<>(messagesByValidationId); + disallowed.keySet().removeIf(id -> allows(id, now)); + if ( ! disallowed.isEmpty()) + throw new ValidationException(disallowed); + } + /** Throws a ValidationException unless this validation is overridden at this time */ public void invalid(ValidationId validationId, String message, Instant now) { if ( ! allows(validationId, now)) @@ -146,26 +157,21 @@ public class ValidationOverrides { /** * A deployment validation exception. * Deployment validations can be {@link ValidationOverrides overridden} based on their id. - * The purpose of this exception is to model that id as a separate field. */ public static class ValidationException extends IllegalArgumentException { static final long serialVersionUID = 789984668; - private final ValidationId validationId; - private ValidationException(ValidationId validationId, String message) { - super(message); - this.validationId = validationId; + super(validationId + ": " + message + ". " + toAllowMessage(validationId)); } - /** Returns the unique id of this validation, which can be used to {@link ValidationOverrides override} it */ - public ValidationId validationId() { return validationId; } - - /** Returns "validationId: message" */ - @Override - public String getMessage() { - return validationId + ": " + super.getMessage() + ". " + toAllowMessage(validationId); + private ValidationException(Map<ValidationId, List<String>> messagesById) { + super(messagesById.entrySet().stream() + .map(messages -> messages.getKey() + ":\n\t" + + String.join("\n\t", messages.getValue()) + "\n" + + toAllowMessage(messages.getKey())) + .collect(Collectors.joining("\n"))); } } diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeAction.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeAction.java index 1248560c931..87a150a6c3c 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeAction.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeAction.java @@ -1,9 +1,11 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.model.api; +import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.provision.ClusterSpec; import java.util.List; +import java.util.Optional; /** * Contains the action to be performed on the given services to handle a config change @@ -37,12 +39,11 @@ public interface ConfigChangeAction { /** Returns the list of services where the action must be performed */ List<ServiceInfo> getServices(); - /** Returns whether this change should be allowed */ - boolean allowed(); + /** When this is non-empty, validation may fail unless this validation id is allowed by validation overrides. */ + default Optional<ValidationId> validationId() { return Optional.empty(); } /** The id of the cluster that needs this action applied */ - // TODO: Remove this default implementation after October 2020 - default ClusterSpec.Id clusterId() { return null; } + ClusterSpec.Id clusterId(); /** Returns whether this change should be ignored for internal redeploy */ default boolean ignoreForInternalRedeploy() { diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeRefeedAction.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeRefeedAction.java index c5a7bd030e2..13109088dcd 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeRefeedAction.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeRefeedAction.java @@ -1,6 +1,8 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.model.api; +import com.yahoo.config.application.api.ValidationId; + /** * Represents an action to re-feed a document type in order to handle a config change. * @@ -12,7 +14,7 @@ public interface ConfigChangeRefeedAction extends ConfigChangeAction { default Type getType() { return Type.REFEED; } /** Returns the name identifying this kind of change, used to identify names which should be allowed */ - String name(); + default String name() { return validationId().orElseThrow().value(); } /** Returns the name of the document type that one must re-feed to handle this config change */ String getDocumentType(); diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeReindexAction.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeReindexAction.java index 085638e31ff..bb714a55f94 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeReindexAction.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeReindexAction.java @@ -1,6 +1,8 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.model.api; +import com.yahoo.config.application.api.ValidationId; + /** * Represents an action to re-index a document type in order to handle a config change. * @@ -11,7 +13,7 @@ public interface ConfigChangeReindexAction extends ConfigChangeAction { @Override default Type getType() { return Type.REINDEX; } /** @return name identifying this kind of change, used to identify names which should be allowed */ - String name(); + default String name() { return validationId().orElseThrow().value(); } /** @return name of the document type that must be re-indexed */ String getDocumentType(); diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeRestartAction.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeRestartAction.java index f178180b6e0..c13399a42f5 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeRestartAction.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeRestartAction.java @@ -11,8 +11,4 @@ public interface ConfigChangeRestartAction extends ConfigChangeAction { @Override default Type getType() { return Type.RESTART; } - /** Restarts are handled automatically so they are allowed */ - @Override - default boolean allowed() { return true; } - } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java index d300e31c3dc..90b8686991b 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.model.application.validation; import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.api.Model; @@ -28,10 +29,13 @@ import java.time.Instant; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; +import static java.util.stream.Collectors.groupingBy; +import static java.util.stream.Collectors.mapping; import static java.util.stream.Collectors.toList; /** @@ -99,9 +103,16 @@ public class Validation { new ContainerRestartValidator(), new NodeResourceChangeValidator() }; - return Arrays.stream(validators) - .flatMap(v -> v.validate(currentModel, nextModel, overrides, now).stream()) - .collect(toList()); + List<ConfigChangeAction> actions = Arrays.stream(validators) + .flatMap(v -> v.validate(currentModel, nextModel, overrides, now).stream()) + .collect(toList()); + + Map<ValidationId, List<String>> disallowableActions = actions.stream() + .filter(action -> action.validationId().isPresent()) + .collect(groupingBy(action -> action.validationId().orElseThrow(), + mapping(ConfigChangeAction::getMessage, toList()))); + overrides.invalid(disallowableActions, now); + return actions; } private static void validateFirstTimeDeployment(VespaModel model, DeployState deployState) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/IndexedSearchClusterChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/IndexedSearchClusterChangeValidator.java index b321d5f3fd7..58cea8c23e5 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/IndexedSearchClusterChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/IndexedSearchClusterChangeValidator.java @@ -13,7 +13,10 @@ import com.yahoo.vespa.model.search.DocumentDatabase; import com.yahoo.vespa.model.search.IndexedSearchCluster; import java.time.Instant; -import java.util.*; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.stream.Collectors; /** diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidator.java index e3f16baf95a..385a678d452 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidator.java @@ -46,22 +46,22 @@ public class IndexingModeChangeValidator implements ChangeValidator { ContentSearchCluster currentSearchCluster = currentCluster.getSearch(); ContentSearchCluster nextSearchCluster = nextCluster.getSearch(); findDocumentTypesWithActionableIndexingModeChange( - actions, overrides, nextCluster, now, + actions, nextCluster, toDocumentTypeNames(currentSearchCluster.getDocumentTypesWithStreamingCluster()), toDocumentTypeNames(nextSearchCluster.getDocumentTypesWithIndexedCluster()), "streaming", "indexed"); findDocumentTypesWithActionableIndexingModeChange( - actions, overrides, nextCluster, now, + actions, nextCluster, toDocumentTypeNames(currentSearchCluster.getDocumentTypesWithIndexedCluster()), toDocumentTypeNames(nextSearchCluster.getDocumentTypesWithStreamingCluster()), "indexed", "streaming"); findDocumentTypesWithActionableIndexingModeChange( - actions, overrides, nextCluster, now, + actions, nextCluster, toDocumentTypeNames(currentSearchCluster.getDocumentTypesWithStoreOnly()), toDocumentTypeNames(nextSearchCluster.getDocumentTypesWithIndexedCluster()), "store-only", "indexed"); findDocumentTypesWithActionableIndexingModeChange( - actions, overrides, nextCluster, now, + actions, nextCluster, toDocumentTypeNames(currentSearchCluster.getDocumentTypesWithIndexedCluster()), toDocumentTypeNames(nextSearchCluster.getDocumentTypesWithStoreOnly()), "indexed", "store-only"); @@ -69,7 +69,7 @@ public class IndexingModeChangeValidator implements ChangeValidator { } private static void findDocumentTypesWithActionableIndexingModeChange( - List<ConfigChangeAction> actions, ValidationOverrides overrides, ContentCluster nextCluster, Instant now, + List<ConfigChangeAction> actions, ContentCluster nextCluster, Set<String> currentTypes, Set<String> nextTypes, String currentIndexMode, String nextIndexingMode) { for (String type : nextTypes) { if (currentTypes.contains(type)) { @@ -78,14 +78,13 @@ public class IndexingModeChangeValidator implements ChangeValidator { .collect(Collectors.toList()); actions.add(VespaReindexAction.of( nextCluster.id(), - ValidationId.indexModeChange.value(), - overrides, + ValidationId.indexModeChange, String.format( "Document type '%s' in cluster '%s' changed indexing mode from '%s' to '%s'", type, nextCluster.getName(), currentIndexMode, nextIndexingMode), services, - type, - now)); + type + )); } } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/StreamingSearchClusterChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/StreamingSearchClusterChangeValidator.java index d85d9bd2db5..2f13caa4e09 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/StreamingSearchClusterChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/StreamingSearchClusterChangeValidator.java @@ -78,7 +78,7 @@ public class StreamingSearchClusterChangeValidator implements ChangeValidator { NewDocumentType nextDocType, ValidationOverrides overrides, Instant now) { - return new DocumentTypeChangeValidator(id, currentDocType, nextDocType).validate(overrides, now); + return new DocumentTypeChangeValidator(id, currentDocType, nextDocType).validate(); } private static NewDocumentType getDocumentType(ContentCluster cluster, StreamingSearchCluster streamingCluster) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaRefeedAction.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaRefeedAction.java index 19c63431c03..6a335447a31 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaRefeedAction.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaRefeedAction.java @@ -1,6 +1,7 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation.change; +import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.model.api.ConfigChangeRefeedAction; import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.application.api.ValidationOverrides; @@ -8,6 +9,7 @@ import com.yahoo.config.provision.ClusterSpec; import java.time.Instant; import java.util.List; +import java.util.Optional; /** * Represents an action to re-feed a document type in order to handle a config change. @@ -22,45 +24,39 @@ public class VespaRefeedAction extends VespaConfigChangeAction implements Config * the validation ids belong to the Vespa model while these names are exposed to the config server, * which is model version independent. */ - private final String name; - + private final ValidationId validationId; private final String documentType; - private final boolean allowed; - private VespaRefeedAction(ClusterSpec.Id id, String name, String message, List<ServiceInfo> services, String documentType, boolean allowed) { + private VespaRefeedAction(ClusterSpec.Id id, ValidationId validationId, String message, List<ServiceInfo> services, String documentType) { super(id, message, services); - this.name = name; + this.validationId = validationId; this.documentType = documentType; - this.allowed = allowed; } /** Creates a refeed action with some missing information */ // TODO: We should require document type or model its absence properly - public static VespaRefeedAction of(ClusterSpec.Id id, String name, ValidationOverrides overrides, String message, Instant now) { - return new VespaRefeedAction(id, name, message, List.of(), "", overrides.allows(name, now)); + public static VespaRefeedAction of(ClusterSpec.Id id, ValidationId validationId, String message) { + return new VespaRefeedAction(id, validationId, message, List.of(), ""); } /** Creates a refeed action */ - public static VespaRefeedAction of(ClusterSpec.Id id, String name, ValidationOverrides overrides, String message, - List<ServiceInfo> services, String documentType, Instant now) { - return new VespaRefeedAction(id, name, message, services, documentType, overrides.allows(name, now)); + public static VespaRefeedAction of(ClusterSpec.Id id, ValidationId validationId, String message, + List<ServiceInfo> services, String documentType) { + return new VespaRefeedAction(id, validationId, message, services, documentType); } @Override public VespaConfigChangeAction modifyAction(String newMessage, List<ServiceInfo> newServices, String documentType) { - return new VespaRefeedAction(clusterId(), name, newMessage, newServices, documentType, allowed); + return new VespaRefeedAction(clusterId(), validationId, newMessage, newServices, documentType); } @Override - public String name() { return name; } + public Optional<ValidationId> validationId() { return Optional.of(validationId); } @Override public String getDocumentType() { return documentType; } @Override - public boolean allowed() { return allowed; } - - @Override public boolean ignoreForInternalRedeploy() { return false; } @@ -76,14 +72,13 @@ public class VespaRefeedAction extends VespaConfigChangeAction implements Config if ( ! (o instanceof VespaRefeedAction)) return false; VespaRefeedAction other = (VespaRefeedAction)o; if ( ! this.documentType.equals(other.documentType)) return false; - if ( ! this.name.equals(other.name)) return false; - if ( ! this.allowed == other.allowed) return false; + if ( ! this.validationId.equals(other.validationId)) return false; return true; } @Override public int hashCode() { - return 31 * super.hashCode() + 11 * name.hashCode() + documentType.hashCode(); + return 31 * super.hashCode() + 11 * validationId.hashCode() + documentType.hashCode(); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaReindexAction.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaReindexAction.java index f10802afc31..8b4060e7d19 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaReindexAction.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaReindexAction.java @@ -1,14 +1,14 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation.change; -import com.yahoo.config.application.api.ValidationOverrides; +import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.model.api.ConfigChangeReindexAction; import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.provision.ClusterSpec; -import java.time.Instant; import java.util.List; import java.util.Objects; +import java.util.Optional; /** * Represents an action to re-index a document type in order to handle a config change. @@ -22,36 +22,32 @@ public class VespaReindexAction extends VespaConfigChangeAction implements Confi * the validation ids belong to the Vespa model while these names are exposed to the config server, * which is model version independent. */ - private final String name; + private final ValidationId validationId; private final String documentType; - private final boolean allowed; - private VespaReindexAction(ClusterSpec.Id id, String name, String message, List<ServiceInfo> services, String documentType, boolean allowed) { + private VespaReindexAction(ClusterSpec.Id id, ValidationId validationId, String message, List<ServiceInfo> services, String documentType) { super(id, message, services); - this.name = name; + this.validationId = validationId; this.documentType = documentType; - this.allowed = allowed; } - public static VespaReindexAction of( - ClusterSpec.Id id, String name, ValidationOverrides overrides, String message, Instant now) { - return new VespaReindexAction(id, name, message, List.of(), /*documentType*/null, overrides.allows(name, now)); + public static VespaReindexAction of(ClusterSpec.Id id, ValidationId validationId, String message) { + return new VespaReindexAction(id, validationId, message, List.of(), /*documentType*/null); } public static VespaReindexAction of( - ClusterSpec.Id id, String name, ValidationOverrides overrides, String message, - List<ServiceInfo> services, String documentType, Instant now) { - return new VespaReindexAction(id, name, message, services, documentType, overrides.allows(name, now)); + ClusterSpec.Id id, ValidationId validationId, String message, + List<ServiceInfo> services, String documentType) { + return new VespaReindexAction(id, validationId, message, services, documentType); } @Override public VespaConfigChangeAction modifyAction(String newMessage, List<ServiceInfo> newServices, String documentType) { - return new VespaReindexAction(clusterId(), name, newMessage, newServices, documentType, allowed); + return new VespaReindexAction(clusterId(), validationId, newMessage, newServices, documentType); } - @Override public String name() { return name; } + @Override public Optional<ValidationId> validationId() { return Optional.of(validationId); } @Override public String getDocumentType() { return documentType; } - @Override public boolean allowed() { return allowed; } @Override public boolean ignoreForInternalRedeploy() { return false; } @Override public String toString() { return super.toString() + ", documentType='" + documentType + "'"; } @@ -61,10 +57,10 @@ public class VespaReindexAction extends VespaConfigChangeAction implements Confi if (o == null || getClass() != o.getClass()) return false; if (!super.equals(o)) return false; VespaReindexAction that = (VespaReindexAction) o; - return allowed == that.allowed && - Objects.equals(name, that.name) && - Objects.equals(documentType, that.documentType); + return Objects.equals(validationId, that.validationId) && + Objects.equals(documentType, that.documentType); } - @Override public int hashCode() { return Objects.hash(super.hashCode(), name, documentType, allowed); } + @Override public int hashCode() { return Objects.hash(super.hashCode(), validationId, documentType); } + } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/DocumentDatabaseChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/DocumentDatabaseChangeValidator.java index 3dcfbe3629d..ce435a4c157 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/DocumentDatabaseChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/DocumentDatabaseChangeValidator.java @@ -72,7 +72,7 @@ public class DocumentDatabaseChangeValidator { private List<VespaConfigChangeAction> validateDocumentTypeChanges(ValidationOverrides overrides, Instant now) { return new DocumentTypeChangeValidator(id, currentDocType, nextDocType) - .validate(overrides, now); + .validate(); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/DocumentTypeChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/DocumentTypeChangeValidator.java index b66145a10c5..5b7fdfad0f7 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/DocumentTypeChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/DocumentTypeChangeValidator.java @@ -1,15 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation.change.search; +import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.document.StructDataType; import com.yahoo.documentmodel.NewDocumentType; import com.yahoo.document.Field; -import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.vespa.model.application.validation.change.VespaConfigChangeAction; import com.yahoo.vespa.model.application.validation.change.VespaRefeedAction; -import java.time.Instant; import java.util.List; import java.util.stream.Collectors; @@ -136,17 +135,16 @@ public class DocumentTypeChangeValidator { this.nextDocType = nextDocType; } - public List<VespaConfigChangeAction> validate(ValidationOverrides overrides, Instant now) { + public List<VespaConfigChangeAction> validate() { return currentDocType.getAllFields().stream(). map(field -> createFieldChange(field, nextDocType)). filter(fieldChange -> fieldChange.valid() && fieldChange.changedType()). map(fieldChange -> VespaRefeedAction.of(id, - "field-type-change", - overrides, + ValidationId.fieldTypeChange, new ChangeMessageBuilder(fieldChange.fieldName()). addChange("data type", fieldChange.currentTypeName(), - fieldChange.nextTypeName()).build(), - now)). + fieldChange.nextTypeName()).build() + )). collect(Collectors.toList()); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/IndexingScriptChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/IndexingScriptChangeValidator.java index 8f9b1a3ed77..e3f3abf0747 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/IndexingScriptChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/IndexingScriptChangeValidator.java @@ -41,21 +41,20 @@ public class IndexingScriptChangeValidator { String fieldName = nextField.getName(); ImmutableSDField currentField = currentSearch.getConcreteField(fieldName); if (currentField != null) { - validateScripts(currentField, nextField, overrides, now).ifPresent(r -> result.add(r)); + validateScripts(currentField, nextField).ifPresent(r -> result.add(r)); } } return result; } - private Optional<VespaConfigChangeAction> validateScripts(ImmutableSDField currentField, ImmutableSDField nextField, - ValidationOverrides overrides, Instant now) { + private Optional<VespaConfigChangeAction> validateScripts(ImmutableSDField currentField, ImmutableSDField nextField) { ScriptExpression currentScript = currentField.getIndexingScript(); ScriptExpression nextScript = nextField.getIndexingScript(); if ( ! equalScripts(currentScript, nextScript)) { ChangeMessageBuilder messageBuilder = new ChangeMessageBuilder(nextField.getName()); new IndexingScriptChangeMessageBuilder(currentSearch, currentField, nextSearch, nextField).populate(messageBuilder); messageBuilder.addChange("indexing script", currentScript.toString(), nextScript.toString()); - return Optional.of(VespaReindexAction.of(id, ValidationId.indexingChange.value(), overrides, messageBuilder.build(), now)); + return Optional.of(VespaReindexAction.of(id, ValidationId.indexingChange, messageBuilder.build())); } return Optional.empty(); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigChangeTestUtils.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigChangeTestUtils.java index f3f9022f6f0..2157839ef5c 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigChangeTestUtils.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigChangeTestUtils.java @@ -1,9 +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.model.application.validation.change; +import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.api.ServiceInfo; -import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.provision.ClusterSpec; import java.time.Instant; @@ -24,26 +24,22 @@ public class ConfigChangeTestUtils { return new VespaRestartAction(id, message, services); } - public static VespaConfigChangeAction newRefeedAction(ClusterSpec.Id id, String name, String message) { - return VespaRefeedAction.of(id, name, ValidationOverrides.empty, message, Instant.now()); + public static VespaConfigChangeAction newRefeedAction(ClusterSpec.Id id, ValidationId validationId, String message) { + return VespaRefeedAction.of(id, validationId, message); } - public static VespaConfigChangeAction newRefeedAction(ClusterSpec.Id id, String name, ValidationOverrides overrides, String message, Instant now) { - return VespaRefeedAction.of(id, name, overrides, message, now); + public static VespaConfigChangeAction newRefeedAction(ClusterSpec.Id id, ValidationId validationId, String message, + List<ServiceInfo> services, String documentType) { + return VespaRefeedAction.of(id, validationId, message, services, documentType); } - public static VespaConfigChangeAction newRefeedAction(ClusterSpec.Id id, String name, ValidationOverrides overrides, String message, - List<ServiceInfo> services, String documentType, Instant now) { - return VespaRefeedAction.of(id, name, overrides, message, services, documentType, now); + public static VespaConfigChangeAction newReindexAction(ClusterSpec.Id id, ValidationId validationId, String message) { + return VespaReindexAction.of(id, validationId, message); } - public static VespaConfigChangeAction newReindexAction(ClusterSpec.Id id, String name, ValidationOverrides overrides, String message, Instant now) { - return VespaReindexAction.of(id, name, overrides, message, now); - } - - public static VespaConfigChangeAction newReindexAction(ClusterSpec.Id id, String name, ValidationOverrides overrides, String message, - List<ServiceInfo> services, String documentType, Instant now) { - return VespaReindexAction.of(id, name, overrides, message, services, documentType, now); + public static VespaConfigChangeAction newReindexAction(ClusterSpec.Id id, ValidationId validationId, String message, + List<ServiceInfo> services, String documentType) { + return VespaReindexAction.of(id, validationId, message, services, documentType); } public static List<ConfigChangeAction> normalizeServicesInActions(List<ConfigChangeAction> result) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexedSearchClusterChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexedSearchClusterChangeValidatorTest.java index 8d365f24c7f..2b211c561d9 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexedSearchClusterChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexedSearchClusterChangeValidatorTest.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.model.application.validation.change; +import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.provision.ClusterSpec; @@ -146,9 +147,8 @@ public class IndexedSearchClusterChangeValidatorTest { public void requireThatChangingFieldTypeIsDiscovered() { Fixture f = Fixture.newOneDocFixture(STRING_FIELD, INT_FIELD); f.assertValidation(List.of(newRefeedAction(ClusterSpec.Id.from("test"), - "field-type-change", - ValidationOverrides.empty, - "Document type 'd1': " + FIELD_TYPE_CHANGE_MSG, FOO_SERVICE, "d1", Instant.now()))); + ValidationId.fieldTypeChange, + "Document type 'd1': " + FIELD_TYPE_CHANGE_MSG, FOO_SERVICE, "d1"))); } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidatorTest.java index 8e2cecf89b4..ba9dfcdc388 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidatorTest.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.model.application.validation.change; +import com.yahoo.config.application.api.ValidationOverrides.ValidationException; import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.api.ConfigChangeReindexAction; import com.yahoo.config.provision.Environment; @@ -11,8 +12,10 @@ import org.junit.Test; import java.util.List; import java.util.stream.Collectors; +import static java.util.stream.Collectors.joining; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * @author bratseth @@ -21,6 +24,25 @@ import static org.junit.Assert.assertTrue; public class IndexingModeChangeValidatorTest { @Test + public void testChangingIndexModeFromIndexedToStreamingWhenDisallowed() { + ValidationTester tester = new ValidationTester(); + + VespaModel oldModel = + tester.deploy(null, getServices("index"), Environment.prod, "<validation-overrides />").getFirst(); + try { + List<ConfigChangeAction> changeActions = + tester.deploy(oldModel, getServices("streaming"), Environment.prod, "<calidation-overrides />").getSecond(); + fail("Should throw on disallowed config change action"); + } + catch (ValidationException e) { + assertEquals("indexing-mode-change:\n" + + "\tDocument type 'music' in cluster 'default' changed indexing mode from 'indexed' to 'streaming'\n" + + "To allow this add <allow until='yyyy-mm-dd'>indexing-mode-change</allow> to validation-overrides.xml, see https://docs.vespa.ai/documentation/reference/validation-overrides.html", + e.getMessage()); + } + } + + @Test public void testChangingIndexModeFromIndexedToStreaming() { ValidationTester tester = new ValidationTester(); @@ -29,9 +51,9 @@ public class IndexingModeChangeValidatorTest { List<ConfigChangeAction> changeActions = tester.deploy(oldModel, getServices("streaming"), Environment.prod, validationOverrides).getSecond(); - assertReindexingChange(true, // allowed=true due to validation override - "Document type 'music' in cluster 'default' changed indexing mode from 'indexed' to 'streaming'", - changeActions); + assertReindexingChange( // allowed=true due to validation override + "Document type 'music' in cluster 'default' changed indexing mode from 'indexed' to 'streaming'", + changeActions); } @Test @@ -43,23 +65,22 @@ public class IndexingModeChangeValidatorTest { List<ConfigChangeAction> changeActions = tester.deploy(oldModel, getServices("store-only"), Environment.prod, validationOverrides).getSecond(); - assertReindexingChange(true, // allowed=true due to validation override - "Document type 'music' in cluster 'default' changed indexing mode from 'indexed' to 'store-only'", - changeActions); + assertReindexingChange( // allowed=true due to validation override + "Document type 'music' in cluster 'default' changed indexing mode from 'indexed' to 'store-only'", + changeActions); } - private void assertReindexingChange(boolean allowed, String message, List<ConfigChangeAction> changeActions) { + private void assertReindexingChange(String message, List<ConfigChangeAction> changeActions) { List<ConfigChangeAction> reindexingActions = changeActions.stream() .filter(a -> a instanceof ConfigChangeReindexAction) .collect(Collectors.toList()); assertEquals(1, reindexingActions.size()); - assertEquals(allowed, reindexingActions.get(0).allowed()); assertTrue(reindexingActions.get(0) instanceof ConfigChangeReindexAction); assertEquals("indexing-mode-change", ((ConfigChangeReindexAction)reindexingActions.get(0)).name()); assertEquals(message, reindexingActions.get(0).getMessage()); } - private static final String getServices(String indexingMode) { + private static String getServices(String indexingMode) { return "<services version='1.0'>" + " <content id='default' version='1.0'>" + " <redundancy>1</redundancy>" + diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/StreamingSearchClusterChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/StreamingSearchClusterChangeValidatorTest.java index f5ef50ee3a4..18aac032fe7 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/StreamingSearchClusterChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/StreamingSearchClusterChangeValidatorTest.java @@ -1,6 +1,7 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation.change; +import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.provision.ClusterSpec; @@ -164,10 +165,9 @@ public class StreamingSearchClusterChangeValidatorTest { private static VespaConfigChangeAction createFieldTypeChangeRefeedAction(String docType, List<ServiceInfo> service) { return ConfigChangeTestUtils.newRefeedAction(ClusterSpec.Id.from("test"), - "field-type-change", - ValidationOverrides.empty, - "Document type '" + docType + "': Field 'f1' changed: data type: 'string' -> 'int'", - service, docType, Instant.now()); + ValidationId.fieldTypeChange, + "Document type '" + docType + "': Field 'f1' changed: data type: 'string' -> 'int'", + service, docType); } private static VespaConfigChangeAction createAddFastAccessRestartAction() { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/DocumentDatabaseChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/DocumentDatabaseChangeValidatorTest.java index a4fbf474a7f..1f64d41e371 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/DocumentDatabaseChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/DocumentDatabaseChangeValidatorTest.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.model.application.validation.change.search; +import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.vespa.model.application.validation.change.VespaConfigChangeAction; @@ -47,20 +48,17 @@ public class DocumentDatabaseChangeValidatorTest { "field f2 type string { indexing: index | summary } " + "field f3 type string { indexing: summary } " + "field f4 type array<s> { struct-field s1 { indexing: attribute } }"); + Instant.now(); f.assertValidation(Arrays.asList( newRestartAction(ClusterSpec.Id.from("test"), "Field 'f1' changed: add attribute aspect"), newRestartAction(ClusterSpec.Id.from("test"), "Field 'f4.s1' changed: add attribute aspect"), newReindexAction(ClusterSpec.Id.from("test"), - "indexing-change", - ValidationOverrides.empty, - "Field 'f2' changed: add index aspect, indexing script: '{ input f2 | summary f2; }' -> " + - "'{ input f2 | tokenize normalize stem:\"BEST\" | index f2 | summary f2; }'", Instant.now()), - newRefeedAction(ClusterSpec.Id.from("test"), - "field-type-change", - ValidationOverrides.empty, - "Field 'f3' changed: data type: 'int' -> 'string'", Instant.now()))); + ValidationId.indexingChange, + "Field 'f2' changed: add index aspect, indexing script: '{ input f2 | summary f2; }' -> " + + "'{ input f2 | tokenize normalize stem:\"BEST\" | index f2 | summary f2; }'"), + newRefeedAction(ClusterSpec.Id.from("test"), ValidationId.fieldTypeChange, "Field 'f3' changed: data type: 'int' -> 'string'"))); } @Test diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/DocumentTypeChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/DocumentTypeChangeValidatorTest.java index a074f961a53..8ee2a924503 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/DocumentTypeChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/DocumentTypeChangeValidatorTest.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.model.application.validation.change.search; +import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.document.DocumentType; import com.yahoo.document.Field; @@ -8,7 +9,6 @@ import com.yahoo.document.ReferenceDataType; import com.yahoo.document.StructDataType; import com.yahoo.documentmodel.NewDocumentType; import com.yahoo.searchdefinition.FieldSets; -import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.vespa.model.application.validation.change.VespaConfigChangeAction; import com.yahoo.vespa.model.application.validation.change.VespaRefeedAction; import org.junit.Test; @@ -42,7 +42,7 @@ public class DocumentTypeChangeValidatorTest { @Override public List<VespaConfigChangeAction> validate() { - return validator.validate(ValidationOverrides.empty, Instant.now()); + return validator.validate(); } } @@ -65,21 +65,16 @@ public class DocumentTypeChangeValidatorTest { public void requireThatDataTypeChangeIsNotOK() throws Exception { Fixture f = new Fixture("field f1 type string { indexing: summary }", "field f1 type int { indexing: summary }"); - f.assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), - "field-type-change", - ValidationOverrides.empty, - "Field 'f1' changed: data type: 'string' -> 'int'", - Instant.now())); + Instant.now(); + f.assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), ValidationId.fieldTypeChange, "Field 'f1' changed: data type: 'string' -> 'int'")); } @Test public void requireThatAddingCollectionTypeIsNotOK() throws Exception { Fixture f = new Fixture("field f1 type string { indexing: summary }", "field f1 type array<string> { indexing: summary }"); - f.assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), - "field-type-change", - ValidationOverrides.empty, - "Field 'f1' changed: data type: 'string' -> 'Array<string>'", Instant.now())); + Instant.now(); + f.assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), ValidationId.fieldTypeChange, "Field 'f1' changed: data type: 'string' -> 'Array<string>'")); } @@ -94,34 +89,26 @@ public class DocumentTypeChangeValidatorTest { public void requireThatNestedDataTypeChangeIsNotOK() throws Exception { Fixture f = new Fixture("field f1 type array<string> { indexing: summary }", "field f1 type array<int> { indexing: summary }"); - f.assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), - "field-type-change", - ValidationOverrides.empty, - "Field 'f1' changed: data type: 'Array<string>' -> 'Array<int>'", Instant.now())); + Instant.now(); + f.assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), ValidationId.fieldTypeChange, "Field 'f1' changed: data type: 'Array<string>' -> 'Array<int>'")); } @Test public void requireThatChangedCollectionTypeIsNotOK() throws Exception { Fixture f = new Fixture("field f1 type array<string> { indexing: summary }", "field f1 type weightedset<string> { indexing: summary }"); - f.assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), - "field-type-change", - ValidationOverrides.empty, - "Field 'f1' changed: data type: 'Array<string>' -> 'WeightedSet<string>'", Instant.now())); + Instant.now(); + f.assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), ValidationId.fieldTypeChange, "Field 'f1' changed: data type: 'Array<string>' -> 'WeightedSet<string>'")); } @Test public void requireThatMultipleDataTypeChangesIsNotOK() throws Exception { Fixture f = new Fixture("field f1 type string { indexing: summary } field f2 type int { indexing: summary }" , "field f2 type string { indexing: summary } field f1 type int { indexing: summary }"); - f.assertValidation(Arrays.asList(newRefeedAction(ClusterSpec.Id.from("test"), - "field-type-change", - ValidationOverrides.empty, - "Field 'f1' changed: data type: 'string' -> 'int'", Instant.now()), - newRefeedAction(ClusterSpec.Id.from("test"), - "field-type-change", - ValidationOverrides.empty, - "Field 'f2' changed: data type: 'int' -> 'string'", Instant.now()))); + Instant.now(); + Instant.now(); + f.assertValidation(Arrays.asList(newRefeedAction(ClusterSpec.Id.from("test"), ValidationId.fieldTypeChange, "Field 'f1' changed: data type: 'string' -> 'int'"), + newRefeedAction(ClusterSpec.Id.from("test"), ValidationId.fieldTypeChange, "Field 'f2' changed: data type: 'int' -> 'string'"))); } @Test @@ -156,40 +143,32 @@ public class DocumentTypeChangeValidatorTest { public void requireThatDataTypeChangeInStructFieldIsNotOK() throws Exception { Fixture f = new Fixture("struct s1 { field f1 type string {} } field f2 type s1 { indexing: summary }", "struct s1 { field f1 type int {} } field f2 type s1 { indexing: summary }"); - f.assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), - "field-type-change", - ValidationOverrides.empty, - "Field 'f2' changed: data type: 's1:{f1:string}' -> 's1:{f1:int}'", Instant.now())); + Instant.now(); + f.assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), ValidationId.fieldTypeChange, "Field 'f2' changed: data type: 's1:{f1:string}' -> 's1:{f1:int}'")); } @Test public void requireThatNestedDataTypeChangeInStructFieldIsNotOK() throws Exception { Fixture f = new Fixture("struct s1 { field f1 type array<string> {} } field f2 type s1 { indexing: summary }", "struct s1 { field f1 type array<int> {} } field f2 type s1 { indexing: summary }"); - f.assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), - "field-type-change", - ValidationOverrides.empty, - "Field 'f2' changed: data type: 's1:{f1:Array<string>}' -> 's1:{f1:Array<int>}'", Instant.now())); + Instant.now(); + f.assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), ValidationId.fieldTypeChange, "Field 'f2' changed: data type: 's1:{f1:Array<string>}' -> 's1:{f1:Array<int>}'")); } @Test public void requireThatDataTypeChangeInNestedStructFieldIsNotOK() throws Exception { Fixture f = new Fixture("struct s1 { field f1 type string {} } struct s2 { field f2 type s1 {} } field f3 type s2 { indexing: summary }", "struct s1 { field f1 type int {} } struct s2 { field f2 type s1 {} } field f3 type s2 { indexing: summary }"); - f.assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), - "field-type-change", - ValidationOverrides.empty, - "Field 'f3' changed: data type: 's2:{s1:{f1:string}}' -> 's2:{s1:{f1:int}}'", Instant.now())); + Instant.now(); + f.assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), ValidationId.fieldTypeChange, "Field 'f3' changed: data type: 's2:{s1:{f1:string}}' -> 's2:{s1:{f1:int}}'")); } @Test public void requireThatMultipleDataTypeChangesInStructFieldIsNotOK() throws Exception { Fixture f = new Fixture("struct s1 { field f1 type string {} field f2 type int {} } field f3 type s1 { indexing: summary }", "struct s1 { field f1 type int {} field f2 type string {} } field f3 type s1 { indexing: summary }"); - f.assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), - "field-type-change", - ValidationOverrides.empty, - "Field 'f3' changed: data type: 's1:{f1:string,f2:int}' -> 's1:{f1:int,f2:string}'", Instant.now())); + Instant.now(); + f.assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), ValidationId.fieldTypeChange, "Field 'f3' changed: data type: 's1:{f1:string,f2:int}' -> 's1:{f1:int,f2:string}'")); } @Test @@ -197,7 +176,7 @@ public class DocumentTypeChangeValidatorTest { var validator = new DocumentTypeChangeValidator(ClusterSpec.Id.from("test"), createDocumentTypeWithReferenceField("oldDoc"), createDocumentTypeWithReferenceField("newDoc")); - List<VespaConfigChangeAction> result = validator.validate(ValidationOverrides.empty, Instant.now()); + List<VespaConfigChangeAction> result = validator.validate(); assertEquals(1, result.size()); VespaConfigChangeAction action = result.get(0); assertTrue(action instanceof VespaRefeedAction); @@ -210,21 +189,17 @@ public class DocumentTypeChangeValidatorTest { @Test public void changing_tensor_type_of_tensor_field_requires_refeed() throws Exception { + Instant.now(); new Fixture( "field f1 type tensor(x[2]) { indexing: attribute }", "field f1 type tensor(x[3]) { indexing: attribute }") - .assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), - "field-type-change", - ValidationOverrides.empty, - "Field 'f1' changed: data type: 'tensor(x[2])' -> 'tensor(x[3])'", Instant.now())); + .assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), ValidationId.fieldTypeChange, "Field 'f1' changed: data type: 'tensor(x[2])' -> 'tensor(x[3])'")); + Instant.now(); new Fixture( "field f1 type tensor(x[5]) { indexing: attribute }", "field f1 type tensor(x[3]) { indexing: attribute }") - .assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), - "field-type-change", - ValidationOverrides.empty, - "Field 'f1' changed: data type: 'tensor(x[5])' -> 'tensor(x[3])'", Instant.now())); + .assertValidation(newRefeedAction(ClusterSpec.Id.from("test"), ValidationId.fieldTypeChange, "Field 'f1' changed: data type: 'tensor(x[5])' -> 'tensor(x[3])'")); } private static NewDocumentType createDocumentTypeWithReferenceField(String nameReferencedDocumentType) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/IndexingScriptChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/IndexingScriptChangeValidatorTest.java index 9f418476a24..2e1ec53f886 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/IndexingScriptChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/IndexingScriptChangeValidatorTest.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.model.application.validation.change.search; +import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.vespa.indexinglanguage.expressions.ScriptExpression; @@ -56,12 +57,10 @@ public class IndexingScriptChangeValidatorTest { private static VespaConfigChangeAction expectedReindexingAction(String field, String changedMsg, String fromScript, String toScript) { return VespaReindexAction.of(ClusterSpec.Id.from("test"), - "indexing-change", - ValidationOverrides.empty, - "Field '" + field + "' changed: " + + ValidationId.indexingChange, + "Field '" + field + "' changed: " + (changedMsg.isEmpty() ? "" : changedMsg + ", ") + - "indexing script: '" + fromScript + "' -> '" + toScript + "'", - Instant.now()); + "indexing script: '" + fromScript + "' -> '" + toScript + "'"); } @Test diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/StructFieldAttributeChangeValidatorTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/StructFieldAttributeChangeValidatorTestCase.java index 04efffab438..0bc4ecbfdfd 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/StructFieldAttributeChangeValidatorTestCase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/StructFieldAttributeChangeValidatorTestCase.java @@ -36,7 +36,7 @@ public class StructFieldAttributeChangeValidatorTestCase { public List<VespaConfigChangeAction> validate() { List<VespaConfigChangeAction> result = new ArrayList<>(); result.addAll(structFieldAttributeValidator.validate(ValidationOverrides.empty, Instant.now())); - result.addAll(docTypeValidator.validate(ValidationOverrides.empty, Instant.now())); + result.addAll(docTypeValidator.validate()); return result; } } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterResources.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterResources.java index f1c86485a64..8f4c9f81d7f 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterResources.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterResources.java @@ -1,6 +1,8 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.provision; +import com.yahoo.config.Node; + import java.util.List; import java.util.Objects; @@ -53,6 +55,14 @@ public class ClusterResources { return true; } + /** Returns the total resources of this, that is the number of nodes times the node resources */ + public NodeResources totalResources() { + return nodeResources.withVcpu(nodeResources.vcpu() * nodes) + .withMemoryGb(nodeResources.memoryGb() * nodes) + .withDiskGb(nodeResources.diskGb() * nodes) + .withBandwidthGbps(nodeResources.bandwidthGbps() * nodes); + } + @Override public boolean equals(Object o) { if (o == this) return true; diff --git a/config/src/main/java/com/yahoo/vespa/config/ConfigVerification.java b/config/src/main/java/com/yahoo/vespa/config/ConfigVerification.java index cea40da52b9..f79abbddc56 100644 --- a/config/src/main/java/com/yahoo/vespa/config/ConfigVerification.java +++ b/config/src/main/java/com/yahoo/vespa/config/ConfigVerification.java @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config; import ai.vespa.util.http.VespaHttpClientBuilder; @@ -72,23 +72,22 @@ public class ConfigVerification { for (Map.Entry<String, Stack<String>> entry : mappings.entrySet()) { recurseUrls.add(entry.getValue().pop()); } - int ret = compareOutputs(performRequests(recurseUrls, httpClient)); - if (ret != 0) { - return ret; - } + if ( ! equalOutputs(performRequests(recurseUrls, httpClient))) + return -1; } return 0; } - private static int compareOutputs(Map<String, String> outputs) { + private static boolean equalOutputs(Map<String, String> outputs) { Map.Entry<String, String> firstEntry = outputs.entrySet().iterator().next(); for (Map.Entry<String, String> entry : outputs.entrySet()) { if (!entry.getValue().equals(firstEntry.getValue())) { - System.out.println("output from '" + entry.getKey() + "' did not equal output from '" + firstEntry.getKey() + "'"); - return -1; + System.out.println("output from '" + entry.getKey() + "': '" + entry.getValue() + + "' did not equal output from '" + firstEntry.getKey() + "': '" + firstEntry.getValue() + "'"); + return false; } } - return 0; + return true; } private static String performRequest(String url, CloseableHttpClient httpClient) throws IOException { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index b356b99c50a..6cc05a0f69e 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -326,12 +326,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye long sessionId = createSession(applicationId, prepareParams.getTimeoutBudget(), applicationPackage); Deployment deployment = prepare(sessionId, prepareParams, logger); - if (deployment.configChangeActions().getRefeedActions().getEntries().stream().anyMatch(entry -> ! entry.allowed())) - logger.log(Level.WARNING, "Activation rejected because of disallowed re-feed actions"); - else if (deployment.configChangeActions().getReindexActions().getEntries().stream().anyMatch(entry -> ! entry.allowed())) - logger.log(Level.WARNING, "Activation rejected because of disallowed re-index actions"); - else - deployment.activate(); + deployment.activate(); return new PrepareResult(sessionId, deployment.configChangeActions(), logger); } @@ -1014,21 +1009,19 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye RestartActions restartActions = actions.getRestartActions(); if ( ! restartActions.isEmpty()) { logger.log(Level.WARNING, "Change(s) between active and new application that require restart:\n" + - restartActions.format()); + restartActions.format()); } RefeedActions refeedActions = actions.getRefeedActions(); if ( ! refeedActions.isEmpty()) { - boolean allAllowed = refeedActions.getEntries().stream().allMatch(RefeedActions.Entry::allowed); - logger.log(allAllowed ? Level.INFO : Level.WARNING, + logger.log(Level.WARNING, "Change(s) between active and new application that may require re-feed:\n" + - refeedActions.format()); + refeedActions.format()); } ReindexActions reindexActions = actions.getReindexActions(); if ( ! reindexActions.isEmpty()) { - boolean allAllowed = reindexActions.getEntries().stream().allMatch(ReindexActions.Entry::allowed); - logger.log(allAllowed ? Level.INFO : Level.WARNING, - "Change(s) between active and new application that may require re-index:\n" + - reindexActions.format()); + logger.log(Level.WARNING, + "Change(s) between active and new application that may require re-index:\n" + + reindexActions.format()); } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java index 2e73a02c75b..8d001d5d5df 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java @@ -130,7 +130,13 @@ public class Application implements ModelResult { if (logDebug()) { debug("Resolving " + configKey + " with config definition " + def); } - ConfigPayload payload = model.getConfig(configKey, def); + + ConfigPayload payload = null; + try { + payload = model.getConfig(configKey, def); + } catch (Exception e) { + throw new ConfigurationRuntimeException("Unable to get config for " + app, e); + } if (payload == null) { metricUpdater.incrementFailedRequests(); throw new ConfigurationRuntimeException("Unable to resolve config " + configKey); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsSlimeConverter.java b/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsSlimeConverter.java index ec48b671a5b..1a0d109b6c9 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsSlimeConverter.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsSlimeConverter.java @@ -42,7 +42,6 @@ public class ConfigChangeActionsSlimeConverter { for (RefeedActions.Entry entry : actions.getRefeedActions().getEntries()) { Cursor entryCursor = refeedCursor.addObject(); entryCursor.setString("name", entry.name()); - entryCursor.setBool("allowed", entry.allowed()); entryCursor.setString("documentType", entry.getDocumentType()); entryCursor.setString("clusterName", entry.getClusterName()); messagesToSlime(entryCursor, entry.getMessages()); @@ -55,7 +54,6 @@ public class ConfigChangeActionsSlimeConverter { for (ReindexActions.Entry entry : actions.getReindexActions().getEntries()) { Cursor entryCursor = refeedCursor.addObject(); entryCursor.setString("name", entry.name()); - entryCursor.setBool("allowed", entry.allowed()); entryCursor.setString("documentType", entry.getDocumentType()); entryCursor.setString("clusterName", entry.getClusterName()); messagesToSlime(entryCursor, entry.getMessages()); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/RefeedActions.java b/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/RefeedActions.java index c20b8527f2e..b2221cbcf6c 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/RefeedActions.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/RefeedActions.java @@ -5,7 +5,13 @@ import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.api.ConfigChangeRefeedAction; import com.yahoo.config.model.api.ServiceInfo; -import java.util.*; +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; +import java.util.TreeSet; /** * Represents all actions to re-feed document types in order to handle config changes. @@ -17,15 +23,13 @@ public class RefeedActions { public static class Entry { private final String name; - private final boolean allowed; private final String documentType; private final String clusterName; private final Set<ServiceInfo> services = new LinkedHashSet<>(); private final Set<String> messages = new TreeSet<>(); - private Entry(String name, boolean allowed, String documentType, String clusterName) { + private Entry(String name, String documentType, String clusterName) { this.name = name; - this.allowed = allowed; this.documentType = documentType; this.clusterName = clusterName; } @@ -42,8 +46,6 @@ public class RefeedActions { public String name() { return name; } - public boolean allowed() { return allowed; } - public String getDocumentType() { return documentType; } public String getClusterName() { return clusterName; } @@ -54,12 +56,12 @@ public class RefeedActions { } - private Entry addEntry(String name, boolean allowed, String documentType, ServiceInfo service) { + private Entry addEntry(String name, String documentType, ServiceInfo service) { String clusterName = service.getProperty("clustername").orElse(""); - String entryId = name + "." + allowed + "." + clusterName + "." + documentType; + String entryId = name + "." + "." + clusterName + "." + documentType; Entry entry = actions.get(entryId); if (entry == null) { - entry = new Entry(name, allowed, documentType, clusterName); + entry = new Entry(name, documentType, clusterName); actions.put(entryId, entry); } return entry; @@ -75,7 +77,7 @@ public class RefeedActions { if (action.getType().equals(ConfigChangeAction.Type.REFEED)) { ConfigChangeRefeedAction refeedAction = (ConfigChangeRefeedAction) action; for (ServiceInfo service : refeedAction.getServices()) { - addEntry(refeedAction.name(), refeedAction.allowed(), refeedAction.getDocumentType(), service). + addEntry(refeedAction.name(), refeedAction.getDocumentType(), service). addService(service). addMessage(action.getMessage()); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/RefeedActionsFormatter.java b/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/RefeedActionsFormatter.java index 425276cebd6..6e2e23ab6be 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/RefeedActionsFormatter.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/RefeedActionsFormatter.java @@ -18,8 +18,6 @@ public class RefeedActionsFormatter { public String format() { StringBuilder builder = new StringBuilder(); for (RefeedActions.Entry entry : actions.getEntries()) { - if (entry.allowed()) - builder.append("(allowed) "); builder.append(entry.name() + ": Consider removing data and re-feed document type '" + entry.getDocumentType() + "' in cluster '" + entry.getClusterName() + "' because:\n"); int counter = 1; diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/ReindexActions.java b/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/ReindexActions.java index e328f9595b7..6ed1c43623f 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/ReindexActions.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/ReindexActions.java @@ -27,7 +27,7 @@ public class ReindexActions { if (action.getType().equals(ConfigChangeAction.Type.REINDEX)) { ConfigChangeReindexAction reindexChange = (ConfigChangeReindexAction) action; for (ServiceInfo service : reindexChange.getServices()) { - addEntry(reindexChange.name(), reindexChange.allowed(), reindexChange.getDocumentType(), service). + addEntry(reindexChange.name(), reindexChange.getDocumentType(), service). addService(service). addMessage(action.getMessage()); } @@ -35,12 +35,12 @@ public class ReindexActions { } } - private Entry addEntry(String name, boolean allowed, String documentType, ServiceInfo service) { + private Entry addEntry(String name, String documentType, ServiceInfo service) { String clusterName = service.getProperty("clustername").orElse(""); - String entryId = name + "." + allowed + "." + clusterName + "." + documentType; + String entryId = name + "." + "." + clusterName + "." + documentType; Entry entry = actions.get(entryId); if (entry == null) { - entry = new Entry(name, allowed, documentType, clusterName); + entry = new Entry(name, documentType, clusterName); actions.put(entryId, entry); } return entry; @@ -53,15 +53,13 @@ public class ReindexActions { public static class Entry { private final String name; - private final boolean allowed; private final String documentType; private final String clusterName; private final Set<ServiceInfo> services = new LinkedHashSet<>(); private final Set<String> messages = new TreeSet<>(); - private Entry(String name, boolean allowed, String documentType, String clusterName) { + private Entry(String name, String documentType, String clusterName) { this.name = name; - this.allowed = allowed; this.documentType = documentType; this.clusterName = clusterName; } @@ -77,7 +75,6 @@ public class ReindexActions { } public String name() { return name; } - public boolean allowed() { return allowed; } public String getDocumentType() { return documentType; } public String getClusterName() { return clusterName; } public Set<ServiceInfo> getServices() { return services; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/ReindexActionsFormatter.java b/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/ReindexActionsFormatter.java index e89bfd522cd..bdd01404f64 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/ReindexActionsFormatter.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/ReindexActionsFormatter.java @@ -17,8 +17,6 @@ class ReindexActionsFormatter { String format() { StringBuilder builder = new StringBuilder(); for (ReindexActions.Entry entry : actions.getEntries()) { - if (entry.allowed()) - builder.append("(allowed) "); builder.append(entry.name() + ": Consider re-indexing document type '" + entry.getDocumentType() + "' in cluster '" + entry.getClusterName() + "' because:\n"); int counter = 1; diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java index 19534bba810..556a2a5b8d3 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java @@ -1,4 +1,4 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.maintenance; import com.yahoo.log.LogLevel; @@ -17,6 +17,7 @@ import java.time.Duration; */ public class SessionsMaintainer extends ConfigServerMaintainer { private final boolean hostedVespa; + private int iteration = 0; SessionsMaintainer(ApplicationRepository applicationRepository, Curator curator, Duration interval, FlagSource flagSource) { super(applicationRepository, curator, flagSource, Duration.ofMinutes(1), interval); @@ -25,6 +26,9 @@ public class SessionsMaintainer extends ConfigServerMaintainer { @Override protected boolean maintain() { + if (iteration % 10 == 0) + log.log(LogLevel.INFO, () -> "Running " + SessionsMaintainer.class.getSimpleName() + ", iteration " + iteration); + applicationRepository.deleteExpiredLocalSessions(); if (hostedVespa) { @@ -33,6 +37,7 @@ public class SessionsMaintainer extends ConfigServerMaintainer { log.log(LogLevel.FINE, () -> "Deleted " + deleted + " expired remote sessions older than " + expiryTime); } + iteration++; return true; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsBuilder.java b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsBuilder.java index b5194432682..876b169742f 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsBuilder.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsBuilder.java @@ -16,7 +16,6 @@ import java.util.List; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; /** * @author geirst @@ -44,20 +43,16 @@ public class ConfigChangeActionsBuilder { } - ConfigChangeActionsBuilder refeed(String name, boolean allowed, String message, String documentType, String clusterName, String serviceName) { - actions.add(new MockRefeedAction(name, - allowed, + ConfigChangeActionsBuilder refeed(ValidationId validationId, String message, String documentType, String clusterName, String serviceName) { + actions.add(new MockRefeedAction(validationId, message, List.of(createService(clusterName, "myclustertype", "myservicetype", serviceName)), documentType)); return this; } - ConfigChangeActionsBuilder reindex(String name, boolean allowed, String message, String documentType, String clusterName, String serviceName) { + ConfigChangeActionsBuilder reindex(ValidationId validationId, String message, String documentType, String clusterName, String serviceName) { List<ServiceInfo> services = List.of(createService(clusterName, "myclustertype", "myservicetype", serviceName)); - ValidationOverrides overrides = mock(ValidationOverrides.class); - when(overrides.allows((String) any(), any())).thenReturn(allowed); - when(overrides.allows((ValidationId) any(), any())).thenReturn(allowed); - actions.add(VespaReindexAction.of(ClusterSpec.Id.from(clusterName), name, overrides, message, services, documentType, Instant.now())); + actions.add(VespaReindexAction.of(ClusterSpec.Id.from(clusterName), validationId, message, services, documentType)); return this; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsSlimeConverterTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsSlimeConverterTest.java index d145a796725..d75f95d4c48 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsSlimeConverterTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsSlimeConverterTest.java @@ -89,16 +89,15 @@ public class ConfigChangeActionsSlimeConverterTest { @Test public void json_representation_of_refeed_actions() throws IOException { ConfigChangeActions actions = new ConfigChangeActionsBuilder(). - refeed(CHANGE_ID, true, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_TYPE). - refeed(CHANGE_ID_2, false, CHANGE_MSG, DOC_TYPE_2, CLUSTER, SERVICE_TYPE).build(); + refeed(CHANGE_ID, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_TYPE). + refeed(CHANGE_ID_2, CHANGE_MSG, DOC_TYPE_2, CLUSTER, SERVICE_TYPE).build(); assertEquals("{\n" + " \"configChangeActions\": {\n" + " \"restart\": [\n" + " ],\n" + " \"refeed\": [\n" + " {\n" + - " \"name\": \"change-id\",\n" + - " \"allowed\": true,\n" + + " \"name\": \"field-type-change\",\n" + " \"documentType\": \"music\",\n" + " \"clusterName\": \"foo\",\n" + " \"messages\": [\n" + @@ -114,8 +113,7 @@ public class ConfigChangeActionsSlimeConverterTest { " ]\n" + " },\n" + " {\n" + - " \"name\": \"other-change-id\",\n" + - " \"allowed\": false,\n" + + " \"name\": \"indexing-change\",\n" + " \"documentType\": \"book\",\n" + " \"clusterName\": \"foo\",\n" + " \"messages\": [\n" + @@ -141,7 +139,7 @@ public class ConfigChangeActionsSlimeConverterTest { @Test public void json_representation_of_reindex_actions() throws IOException { ConfigChangeActions actions = new ConfigChangeActionsBuilder(). - reindex(CHANGE_ID, true, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_TYPE).build(); + reindex(CHANGE_ID, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_TYPE).build(); assertEquals( "{\n" + " \"configChangeActions\": {\n" + @@ -151,8 +149,7 @@ public class ConfigChangeActionsSlimeConverterTest { " ],\n" + " \"reindex\": [\n" + " {\n" + - " \"name\": \"change-id\",\n" + - " \"allowed\": true,\n" + + " \"name\": \"field-type-change\",\n" + " \"documentType\": \"music\",\n" + " \"clusterName\": \"foo\",\n" + " \"messages\": [\n" + diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/MockRefeedAction.java b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/MockRefeedAction.java index 11f2a46994c..615d4c86c1d 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/MockRefeedAction.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/MockRefeedAction.java @@ -1,32 +1,35 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.configchange; +import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.model.api.ConfigChangeRefeedAction; import com.yahoo.config.model.api.ServiceInfo; +import com.yahoo.config.provision.ClusterSpec; import java.util.List; +import java.util.Optional; /** * @author geirst */ public class MockRefeedAction extends MockConfigChangeAction implements ConfigChangeRefeedAction { - private final String name; - private final boolean allowed; + private final ValidationId validationId; private final String documentType; - public MockRefeedAction(String name, boolean allowed, String message, List<ServiceInfo> services, String documentType) { + public MockRefeedAction(ValidationId validationId, String message, List<ServiceInfo> services, String documentType) { super(message, services); - this.name = name; - this.allowed = allowed; + this.validationId = validationId; this.documentType = documentType; } @Override - public String name() { return name; } + public Optional<ValidationId> validationId() { return Optional.of(validationId); } @Override - public boolean allowed() { return allowed; } + public ClusterSpec.Id clusterId() { + return null; + } @Override public boolean ignoreForInternalRedeploy() { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RefeedActionsFormatterTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RefeedActionsFormatterTest.java index 48d6833129e..4b898b501ec 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RefeedActionsFormatterTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RefeedActionsFormatterTest.java @@ -15,9 +15,9 @@ public class RefeedActionsFormatterTest { @Test public void formatting_of_single_action() { RefeedActions actions = new ConfigChangeActionsBuilder(). - refeed(CHANGE_ID, false, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME). + refeed(CHANGE_ID, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME). build().getRefeedActions(); - assertEquals("change-id: Consider removing data and re-feed document type 'music' in cluster 'foo' because:\n" + + assertEquals("field-type-change: Consider removing data and re-feed document type 'music' in cluster 'foo' because:\n" + " 1) change\n", new RefeedActionsFormatter(actions).format()); } @@ -25,20 +25,18 @@ public class RefeedActionsFormatterTest { @Test public void formatting_of_multiple_actions() { RefeedActions actions = new ConfigChangeActionsBuilder(). - refeed(CHANGE_ID, false, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME). - refeed(CHANGE_ID, false, CHANGE_MSG_2, DOC_TYPE, CLUSTER, SERVICE_NAME). - refeed(CHANGE_ID_2, false, CHANGE_MSG_2, DOC_TYPE, CLUSTER, SERVICE_NAME). - refeed(CHANGE_ID_2, true, CHANGE_MSG_2, DOC_TYPE, CLUSTER, SERVICE_NAME). - refeed(CHANGE_ID, false, CHANGE_MSG_2, DOC_TYPE_2, CLUSTER, SERVICE_NAME). + refeed(CHANGE_ID, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME). + refeed(CHANGE_ID, CHANGE_MSG_2, DOC_TYPE, CLUSTER, SERVICE_NAME). + refeed(CHANGE_ID_2, CHANGE_MSG_2, DOC_TYPE, CLUSTER, SERVICE_NAME). + refeed(CHANGE_ID_2, CHANGE_MSG_2, DOC_TYPE, CLUSTER, SERVICE_NAME). + refeed(CHANGE_ID, CHANGE_MSG_2, DOC_TYPE_2, CLUSTER, SERVICE_NAME). build().getRefeedActions(); - assertEquals("change-id: Consider removing data and re-feed document type 'book' in cluster 'foo' because:\n" + + assertEquals("field-type-change: Consider removing data and re-feed document type 'book' in cluster 'foo' because:\n" + " 1) other change\n" + - "change-id: Consider removing data and re-feed document type 'music' in cluster 'foo' because:\n" + + "field-type-change: Consider removing data and re-feed document type 'music' in cluster 'foo' because:\n" + " 1) change\n" + " 2) other change\n" + - "other-change-id: Consider removing data and re-feed document type 'music' in cluster 'foo' because:\n" + - " 1) other change\n" + - "(allowed) other-change-id: Consider removing data and re-feed document type 'music' in cluster 'foo' because:\n" + + "indexing-change: Consider removing data and re-feed document type 'music' in cluster 'foo' because:\n" + " 1) other change\n", new RefeedActionsFormatter(actions).format()); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RefeedActionsTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RefeedActionsTest.java index 7235b8905c5..24e81dc3f99 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RefeedActionsTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RefeedActionsTest.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.config.server.configchange; +import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.model.api.ServiceInfo; import org.junit.Test; @@ -32,8 +33,8 @@ public class RefeedActionsTest { @Test public void action_with_multiple_reasons() { List<RefeedActions.Entry> entries = new ConfigChangeActionsBuilder(). - refeed("change-id", false, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME). - refeed("change-id", false, CHANGE_MSG_2, DOC_TYPE, CLUSTER, SERVICE_NAME). + refeed(ValidationId.indexModeChange, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME). + refeed(ValidationId.indexModeChange, CHANGE_MSG_2, DOC_TYPE, CLUSTER, SERVICE_NAME). build().getRefeedActions().getEntries(); assertThat(entries.size(), is(1)); assertThat(toString(entries.get(0)), equalTo("music.foo:[baz][change,other change]")); @@ -42,8 +43,8 @@ public class RefeedActionsTest { @Test public void actions_with_multiple_services() { List<RefeedActions.Entry> entries = new ConfigChangeActionsBuilder(). - refeed("change-id", false, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME). - refeed("change-id", false, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME_2). + refeed(ValidationId.indexModeChange, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME). + refeed(ValidationId.indexModeChange, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME_2). build().getRefeedActions().getEntries(); assertThat(entries.size(), is(1)); assertThat(toString(entries.get(0)), equalTo("music.foo:[baz,qux][change]")); @@ -52,8 +53,8 @@ public class RefeedActionsTest { @Test public void actions_with_multiple_document_types() { List<RefeedActions.Entry> entries = new ConfigChangeActionsBuilder(). - refeed("change-id", false, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME). - refeed("change-id", false, CHANGE_MSG, DOC_TYPE_2, CLUSTER, SERVICE_NAME). + refeed(ValidationId.indexModeChange, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME). + refeed(ValidationId.indexModeChange, CHANGE_MSG, DOC_TYPE_2, CLUSTER, SERVICE_NAME). build().getRefeedActions().getEntries(); assertThat(entries.size(), is(2)); assertThat(toString(entries.get(0)), equalTo("book.foo:[baz][change]")); @@ -63,8 +64,8 @@ public class RefeedActionsTest { @Test public void actions_with_multiple_clusters() { List<RefeedActions.Entry> entries = new ConfigChangeActionsBuilder(). - refeed("change-id", false, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME). - refeed("change-id", false, CHANGE_MSG, DOC_TYPE, CLUSTER_2, SERVICE_NAME). + refeed(ValidationId.indexModeChange, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME). + refeed(ValidationId.indexModeChange, CHANGE_MSG, DOC_TYPE, CLUSTER_2, SERVICE_NAME). build().getRefeedActions().getEntries(); assertThat(entries.size(), is(2)); assertThat(toString(entries.get(0)), equalTo("music.bar:[baz][change]")); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ReindexActionsFormatterTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ReindexActionsFormatterTest.java index e9dd3f3bbfc..b07d002a431 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ReindexActionsFormatterTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ReindexActionsFormatterTest.java @@ -21,9 +21,9 @@ public class ReindexActionsFormatterTest { @Test public void formatting_of_single_action() { ReindexActions actions = new ConfigChangeActionsBuilder(). - reindex(CHANGE_ID, false, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME). + reindex(CHANGE_ID, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME). build().getReindexActions(); - assertEquals("change-id: Consider re-indexing document type 'music' in cluster 'foo' because:\n" + + assertEquals("field-type-change: Consider re-indexing document type 'music' in cluster 'foo' because:\n" + " 1) change\n", new ReindexActionsFormatter(actions).format()); } @@ -31,20 +31,18 @@ public class ReindexActionsFormatterTest { @Test public void formatting_of_multiple_actions() { ReindexActions actions = new ConfigChangeActionsBuilder(). - reindex(CHANGE_ID, false, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME). - reindex(CHANGE_ID, false, CHANGE_MSG_2, DOC_TYPE, CLUSTER, SERVICE_NAME). - reindex(CHANGE_ID_2, false, CHANGE_MSG_2, DOC_TYPE, CLUSTER, SERVICE_NAME). - reindex(CHANGE_ID_2, true, CHANGE_MSG_2, DOC_TYPE, CLUSTER, SERVICE_NAME). - reindex(CHANGE_ID, false, CHANGE_MSG_2, DOC_TYPE_2, CLUSTER, SERVICE_NAME). + reindex(CHANGE_ID, CHANGE_MSG, DOC_TYPE, CLUSTER, SERVICE_NAME). + reindex(CHANGE_ID, CHANGE_MSG_2, DOC_TYPE, CLUSTER, SERVICE_NAME). + reindex(CHANGE_ID_2, CHANGE_MSG_2, DOC_TYPE, CLUSTER, SERVICE_NAME). + reindex(CHANGE_ID_2, CHANGE_MSG_2, DOC_TYPE, CLUSTER, SERVICE_NAME). + reindex(CHANGE_ID, CHANGE_MSG_2, DOC_TYPE_2, CLUSTER, SERVICE_NAME). build().getReindexActions(); - assertEquals("change-id: Consider re-indexing document type 'book' in cluster 'foo' because:\n" + + assertEquals("field-type-change: Consider re-indexing document type 'book' in cluster 'foo' because:\n" + " 1) other change\n" + - "change-id: Consider re-indexing document type 'music' in cluster 'foo' because:\n" + + "field-type-change: Consider re-indexing document type 'music' in cluster 'foo' because:\n" + " 1) change\n" + " 2) other change\n" + - "other-change-id: Consider re-indexing document type 'music' in cluster 'foo' because:\n" + - " 1) other change\n" + - "(allowed) other-change-id: Consider re-indexing document type 'music' in cluster 'foo' because:\n" + + "indexing-change: Consider re-indexing document type 'music' in cluster 'foo' because:\n" + " 1) other change\n", new ReindexActionsFormatter(actions).format()); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/Utils.java b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/Utils.java index 8499c12f648..e02e1e2b143 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/Utils.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/Utils.java @@ -1,14 +1,16 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.configchange; +import com.yahoo.config.application.api.ValidationId; + /** * @author geirst * @since 5.44 */ public class Utils { - final static String CHANGE_ID = "change-id"; - final static String CHANGE_ID_2 = "other-change-id"; + final static ValidationId CHANGE_ID = ValidationId.fieldTypeChange; + final static ValidationId CHANGE_ID_2 = ValidationId.indexingChange; final static String CHANGE_MSG = "change"; final static String CHANGE_MSG_2 = "other change"; final static String DOC_TYPE = "music"; diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java index 4d1b9341e7f..341fa7109da 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java @@ -3,7 +3,7 @@ package com.yahoo.vespa.config.server.deploy; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.component.Version; -import com.yahoo.config.application.api.ValidationOverrides; +import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.api.ModelContext; import com.yahoo.config.model.api.ModelCreateResult; @@ -49,6 +49,7 @@ import static com.yahoo.vespa.config.server.deploy.DeployTester.createFailingMod import static com.yahoo.vespa.config.server.deploy.DeployTester.createHostedModelFactory; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -390,27 +391,6 @@ public class HostedDeployTest { } @Test - public void testThatDisallowedConfigChangeActionsBlockDeployment() throws IOException { - List<Host> hosts = List.of(createHost("host1", "6.1.0"), - createHost("host2", "6.1.0"), - createHost("host3", "6.1.0"), - createHost("host4", "6.1.0")); - List<ServiceInfo> services = List.of( - new ServiceInfo("serviceName", "serviceType", null, Map.of("clustername", "cluster"), "configId", "hostName")); - - ManualClock clock = new ManualClock(Instant.EPOCH); - List<ModelFactory> modelFactories = List.of( - new ConfigChangeActionsModelFactory(Version.fromString("6.1.0"), - VespaReindexAction.of(ClusterSpec.Id.from("test"), "indexing-mode-change", ValidationOverrides.empty, - "reindex please", services, "music", clock.instant()), - new VespaRestartAction(ClusterSpec.Id.from("test"), "change", services))); - - DeployTester tester = createTester(hosts, modelFactories, prodZone, clock); - PrepareResult prepareResult = tester.deployApp("src/test/apps/hosted/", "6.1.0"); - assertNull("Deployment was not activated", tester.applicationRepository().getActiveSession(tester.applicationId())); - } - - @Test public void testThatAllowedConfigChangeActionsAreActedUpon() throws IOException { List<Host> hosts = List.of(createHost("host1", "6.1.0"), createHost("host2", "6.1.0"), @@ -422,8 +402,8 @@ public class HostedDeployTest { ManualClock clock = new ManualClock(Instant.EPOCH); List<ModelFactory> modelFactories = List.of( new ConfigChangeActionsModelFactory(Version.fromString("6.1.0"), - VespaReindexAction.of(ClusterSpec.Id.from("test"), "indexing-mode-change", ValidationOverrides.all, - "reindex please", services, "music", clock.instant()), + VespaReindexAction.of(ClusterSpec.Id.from("test"), ValidationId.indexModeChange, + "reindex please", services, "music"), new VespaRestartAction(ClusterSpec.Id.from("test"), "change", services))); DeployTester tester = createTester(hosts, modelFactories, prodZone, clock); diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index 19fb1862262..cd3f011352d 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -6089,6 +6089,7 @@ "public final java.lang.Object get(java.lang.String, java.util.Map)", "public final java.lang.Object get(java.lang.String, java.util.Map, com.yahoo.processing.request.Properties)", "public final java.lang.Object get(com.yahoo.processing.request.CompoundName, java.util.Map, com.yahoo.processing.request.Properties)", + "public final com.yahoo.search.query.profile.compiled.DimensionalMap getEntries()", "public com.yahoo.search.query.profile.compiled.CompiledQueryProfile clone()", "public java.lang.String toString()", "public bridge synthetic com.yahoo.component.AbstractComponent clone()", diff --git a/container-search/src/main/java/com/yahoo/search/Query.java b/container-search/src/main/java/com/yahoo/search/Query.java index 4995927f7a2..ce31b9a3ba3 100644 --- a/container-search/src/main/java/com/yahoo/search/Query.java +++ b/container-search/src/main/java/com/yahoo/search/Query.java @@ -181,7 +181,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { //---------------- Tracing ---------------------------------------------------- - private static Logger log = Logger.getLogger(Query.class.getName()); + private static final Logger log = Logger.getLogger(Query.class.getName()); /** The time this query was created */ private long startTime; @@ -200,7 +200,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { public static final CompoundName TIMEOUT = new CompoundName("timeout"); - private static QueryProfileType argumentType; + private static final QueryProfileType argumentType; static { argumentType = new QueryProfileType("native"); argumentType.setBuiltin(true); @@ -226,7 +226,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { public static QueryProfileType getArgumentType() { return argumentType; } /** The aliases of query properties */ - private static Map<String, CompoundName> propertyAliases; + private static final Map<String, CompoundName> propertyAliases; static { Map<String,CompoundName> propertyAliasesBuilder = new HashMap<>(); addAliases(Query.getArgumentType(), propertyAliasesBuilder); @@ -316,7 +316,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { * Creates a query from a request * * @param request the HTTP request from which this is created - * @param queryProfile the query profile to use for this query, or null if none. + * @param queryProfile the query profile to use for this query, or null if none */ public Query(HttpRequest request, CompiledQueryProfile queryProfile) { this(request, request.propertyMap(), queryProfile); @@ -325,9 +325,9 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { /** * Creates a query from a request * - * @param request the HTTP request from which this is created. - * @param requestMap the property map of the query. - * @param queryProfile the query profile to use for this query, or null if none. + * @param request the HTTP request from which this is created + * @param requestMap the property map of the query + * @param queryProfile the query profile to use for this query, or null if none */ public Query(HttpRequest request, Map<String, String> requestMap, CompiledQueryProfile queryProfile) { super(new QueryPropertyAliases(propertyAliases)); diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java index c6b0f4a533b..2439908183c 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java @@ -183,6 +183,11 @@ public class CompiledQueryProfile extends AbstractComponent implements Cloneable return substitute(value.value(), context, substitution); } + /** Returns all the entries from the profile **/ + public final DimensionalMap<ValueWithSource> getEntries() { + return this.entries; + } + private Object substitute(Object value, Map<String, String> context, Properties substitution) { if (value == null) return value; if (substitution == null) return value; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/RefeedAction.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/RefeedAction.java index faa2c39ee65..799bc814abe 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/RefeedAction.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/RefeedAction.java @@ -14,7 +14,6 @@ import java.util.List; public class RefeedAction { public final String name; - public final boolean allowed; public final String documentType; public final String clusterName; public final List<ServiceInfo> services; @@ -22,13 +21,11 @@ public class RefeedAction { @JsonCreator public RefeedAction(@JsonProperty("name") String name, - @JsonProperty("allowed") boolean allowed, @JsonProperty("documentType") String documentType, @JsonProperty("clusterName") String clusterName, @JsonProperty("services") List<ServiceInfo> services, @JsonProperty("messages") List<String> messages) { this.name = name; - this.allowed = allowed; this.documentType = documentType; this.clusterName = clusterName; this.services = services; @@ -39,7 +36,6 @@ public class RefeedAction { public String toString() { return "RefeedAction{" + "name='" + name + '\'' + - ", allowed=" + allowed + ", documentType='" + documentType + '\'' + ", clusterName='" + clusterName + '\'' + ", services=" + services + diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/ReindexAction.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/ReindexAction.java index c5735fbd4a6..c2b28a94c66 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/ReindexAction.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/ReindexAction.java @@ -14,7 +14,6 @@ import java.util.List; public class ReindexAction { public final String name; - public final boolean allowed; public final String documentType; public final String clusterName; public final List<ServiceInfo> services; @@ -22,13 +21,11 @@ public class ReindexAction { @JsonCreator public ReindexAction(@JsonProperty("name") String name, - @JsonProperty("allowed") boolean allowed, @JsonProperty("documentType") String documentType, @JsonProperty("clusterName") String clusterName, @JsonProperty("services") List<ServiceInfo> services, @JsonProperty("messages") List<String> messages) { this.name = name; - this.allowed = allowed; this.documentType = documentType; this.clusterName = clusterName; this.services = services; @@ -39,7 +36,6 @@ public class ReindexAction { public String toString() { return "ReindexAction{" + "name='" + name + '\'' + - ", allowed=" + allowed + ", documentType='" + documentType + '\'' + ", clusterName='" + clusterName + '\'' + ", services=" + services + diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java index 8fd294f64f8..7d85c11789b 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java @@ -14,10 +14,12 @@ import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeHist import java.time.Instant; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.Set; /** * A node in hosted Vespa. @@ -57,6 +59,8 @@ public class Node { private final Optional<ApplicationId> exclusiveTo; private final Map<String, JsonNode> reports; private final List<NodeHistory> history; + private final Set<String> additionalIpAddresses; + private final String openStackId; public Node(HostName hostname, Optional<HostName> parentHostname, State state, NodeType type, NodeResources resources, Optional<ApplicationId> owner, Version currentVersion, Version wantedVersion, Version currentOsVersion, Version wantedOsVersion, @@ -64,7 +68,8 @@ public class Node { Optional<Instant> suspendedSince, long restartGeneration, long wantedRestartGeneration, long rebootGeneration, long wantedRebootGeneration, int cost, String flavor, String clusterId, ClusterType clusterType, boolean wantToRetire, boolean wantToDeprovision, Optional<TenantName> reservedTo, Optional<ApplicationId> exclusiveTo, - DockerImage wantedDockerImage, DockerImage currentDockerImage, Map<String, JsonNode> reports, List<NodeHistory> history) { + DockerImage wantedDockerImage, DockerImage currentDockerImage, Map<String, JsonNode> reports, List<NodeHistory> history, + Set<String> additionalIpAddresses, String openStackId) { this.hostname = hostname; this.parentHostname = parentHostname; this.state = state; @@ -95,6 +100,8 @@ public class Node { this.currentDockerImage = currentDockerImage; this.reports = reports; this.history = history; + this.openStackId = openStackId; + this.additionalIpAddresses = additionalIpAddresses; } public HostName hostname() { @@ -211,6 +218,14 @@ public class Node { return history; } + public Set<String> additionalIpAddresses() { + return additionalIpAddresses; + } + + public String openStackId() { + return openStackId; + } + @Override public boolean equals(Object o) { if (this == o) return true; @@ -285,6 +300,8 @@ public class Node { private Optional<ApplicationId> exclusiveTo = Optional.empty(); private Map<String, JsonNode> reports = new HashMap<>(); private List<NodeHistory> history = new ArrayList<>(); + private Set<String> additionalIpAddresses = new HashSet<>(); + private String openStackId; public Builder() { } @@ -319,6 +336,8 @@ public class Node { this.exclusiveTo = node.exclusiveTo; this.reports = node.reports; this.history = node.history; + this.additionalIpAddresses = node.additionalIpAddresses; + this.openStackId = node.openStackId; } public Builder hostname(HostName hostname) { @@ -466,12 +485,22 @@ public class Node { return this; } + public Builder additionalIpAddresses(Set<String> additionalIpAddresses) { + this.additionalIpAddresses = additionalIpAddresses; + return this; + } + + public Builder openStackId(String openStackId) { + this.openStackId = openStackId; + return this; + } + public Node build() { return new Node(hostname, parentHostname, state, type, resources, owner, currentVersion, wantedVersion, currentOsVersion, wantedOsVersion, currentFirmwareCheck, wantedFirmwareCheck, serviceState, suspendedSince, restartGeneration, wantedRestartGeneration, rebootGeneration, wantedRebootGeneration, cost, flavor, clusterId, clusterType, wantToRetire, wantToDeprovision, reservedTo, exclusiveTo, - wantedDockerImage, currentDockerImage, reports, history); + wantedDockerImage, currentDockerImage, reports, history, additionalIpAddresses, openStackId); } } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java index ca8af48e4fd..af1b3fa53fc 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java @@ -135,7 +135,9 @@ public interface NodeRepository { dockerImageFrom(node.getWantedDockerImage()), dockerImageFrom(node.getCurrentDockerImage()), node.getReports(), - node.getHistory()); + node.getHistory(), + node.getAdditionalIpAddresses(), + node.getOpenStackId()); } private static String clusterIdOf(NodeMembership nodeMembership) { diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/PrepareResponse.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/PrepareResponse.java index 6054e05149b..5c946538625 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/PrepareResponse.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/PrepareResponse.java @@ -15,7 +15,6 @@ import java.util.List; @JsonIgnoreProperties(ignoreUnknown = true) public class PrepareResponse { public TenantId tenant; - @JsonProperty("activate") public URI activationUri; public String message; public List<Log> log; public ConfigChangeActions configChangeActions; 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 9b959bf1765..ed1e442f266 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 @@ -219,43 +219,11 @@ public class InternalStepRunner implements StepRunner { LogEntry.typeOf(LogLevel.parse(entry.level)), entry.message)) .collect(toList())); - if ( ! prepareResponse.configChangeActions.refeedActions.stream().allMatch(action -> action.allowed)) { - List<String> messages = new ArrayList<>(); - messages.add("Deploy failed due to non-compatible changes that require re-feed."); - messages.add("Your options are:"); - messages.add("1. Revert the incompatible changes."); - messages.add("2. If you think it is safe in your case, you can override this validation, see"); - messages.add(" http://docs.vespa.ai/documentation/reference/validation-overrides.html"); - messages.add("3. Deploy as a new application under a different name."); - messages.add("Illegal actions:"); - prepareResponse.configChangeActions.refeedActions.stream() - .filter(action -> ! action.allowed) - .flatMap(action -> action.messages.stream()) - .forEach(messages::add); - logger.log(messages); - return Optional.of(deploymentFailed); - } - - if ( ! prepareResponse.configChangeActions.reindexActions.stream().allMatch(action -> action.allowed)) { - List<String> messages = new ArrayList<>(); - messages.add("Deploy failed due to non-compatible changes that require re-index."); - messages.add("Your options are:"); - messages.add("1. Revert the incompatible changes."); - messages.add("2. If you think it is safe in your case, you can override this validation, see"); - messages.add(" http://docs.vespa.ai/documentation/reference/validation-overrides.html"); - messages.add("3. Deploy as a new application under a different name."); - messages.add("Illegal actions:"); - prepareResponse.configChangeActions.reindexActions.stream() - .filter(action -> ! action.allowed) - .flatMap(action -> action.messages.stream()) - .forEach(messages::add); - logger.log(messages); - return Optional.of(deploymentFailed); - } logger.log("Deployment successful."); if (prepareResponse.message != null) logger.log(prepareResponse.message); + return Optional.of(running); } catch (ConfigServerException e) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 4d928a6b8a7..fc0b8eb9fee 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -2054,7 +2054,6 @@ public class ApplicationApiHandler extends LoggingRequestHandler { for (RefeedAction refeedAction : result.prepareResponse().configChangeActions.refeedActions) { Cursor refeedActionObject = refeedActionsArray.addObject(); refeedActionObject.setString("name", refeedAction.name); - refeedActionObject.setBool("allowed", refeedAction.allowed); refeedActionObject.setString("documentType", refeedAction.documentType); refeedActionObject.setString("clusterName", refeedAction.clusterName); serviceInfosToSlime(refeedAction.services, refeedActionObject.setArray("services")); 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 7b248052eac..d54971f5b1d 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 @@ -125,34 +125,7 @@ public class InternalStepRunnerTest { } @Test - public void reindexRequirementBlocksDeployment() { - tester.configServer().setConfigChangeActions(new ConfigChangeActions(List.of(), - List.of(), - List.of(new ReindexAction("Reindex", - false, - "doctype", - "cluster", - Collections.emptyList(), - List.of("Reindex it!"))))); - tester.jobs().deploy(app.instanceId(), JobType.devUsEast1, Optional.empty(), applicationPackage); - assertEquals(failed, tester.jobs().last(app.instanceId(), JobType.devUsEast1).get().stepStatuses().get(Step.deployReal)); - } - - @Test - public void refeedRequirementBlocksDeployment() { - tester.configServer().setConfigChangeActions(new ConfigChangeActions(List.of(), - List.of(new RefeedAction("Refeed", - false, - "doctype", - "cluster", - Collections.emptyList(), - singletonList("Refeed it!"))), - List.of())); - tester.jobs().deploy(app.instanceId(), JobType.devUsEast1, Optional.empty(), applicationPackage); - assertEquals(failed, tester.jobs().last(app.instanceId(), JobType.devUsEast1).get().stepStatuses().get(Step.deployReal)); - } - - @Test + // TODO jonmv: Change to only wait for restarts, and remove triggering of restarts from runner. public void restartsServicesAndWaitsForRestartAndReboot() { RunId id = app.newRun(JobType.productionUsCentral1); ZoneId zone = id.type().zone(system()); diff --git a/eval/src/tests/tensor/instruction_benchmark/instruction_benchmark.cpp b/eval/src/tests/tensor/instruction_benchmark/instruction_benchmark.cpp index 1a796aac88f..e4c1af3100a 100644 --- a/eval/src/tests/tensor/instruction_benchmark/instruction_benchmark.cpp +++ b/eval/src/tests/tensor/instruction_benchmark/instruction_benchmark.cpp @@ -354,6 +354,7 @@ MyParam::~MyParam() = default; struct EvalOp { using UP = std::unique_ptr<EvalOp>; + Stash my_stash; const Impl &impl; MyParam my_param; std::vector<Value::UP> values; @@ -361,8 +362,8 @@ struct EvalOp { EvalSingle single; EvalOp(const EvalOp &) = delete; EvalOp &operator=(const EvalOp &) = delete; - EvalOp(Instruction op, const std::vector<CREF<TensorSpec>> &stack_spec, const Impl &impl_in) - : impl(impl_in), my_param(), values(), stack(), single(impl.engine, op) + EvalOp(Stash &&stash_in, Instruction op, const std::vector<CREF<TensorSpec>> &stack_spec, const Impl &impl_in) + : my_stash(std::move(stash_in)), impl(impl_in), my_param(), values(), stack(), single(impl.engine, op) { for (const TensorSpec &spec: stack_spec) { values.push_back(impl.create_value(spec)); @@ -371,14 +372,51 @@ struct EvalOp { stack.push_back(*value.get()); } } - EvalOp(Instruction op, const TensorSpec &p0, const Impl &impl_in) - : impl(impl_in), my_param(p0, impl), values(), stack(), single(impl.engine, op, my_param) + EvalOp(Stash &&stash_in, Instruction op, const TensorSpec &p0, const Impl &impl_in) + : my_stash(std::move(stash_in)), impl(impl_in), my_param(p0, impl), values(), stack(), single(impl.engine, op, my_param) { } TensorSpec result() { return impl.create_spec(single.eval(stack)); } - double estimate_cost_us() { - auto actual = [&](){ single.eval(stack); }; - return BenchmarkTimer::benchmark(actual, budget) * 1000.0 * 1000.0; + size_t suggest_loop_cnt() { + size_t loop_cnt = 1; + auto my_loop = [&](){ + for (size_t i = 0; i < loop_cnt; ++i) { + single.eval(stack); + } + }; + for (;;) { + vespalib::BenchmarkTimer timer(0.0); + for (size_t i = 0; i < 5; ++i) { + timer.before(); + my_loop(); + timer.after(); + } + double min_time = timer.min_time(); + if (min_time > 0.004) { + break; + } else { + loop_cnt *= 2; + } + } + return std::max(loop_cnt, size_t(8)); + } + double estimate_cost_us(size_t self_loop_cnt, size_t ref_loop_cnt) { + size_t loop_cnt = ((self_loop_cnt * 128) < ref_loop_cnt) ? self_loop_cnt : ref_loop_cnt; + assert((loop_cnt % 8) == 0); + auto my_loop = [&](){ + for (size_t i = 0; (i + 7) < loop_cnt; i += 8) { + for (size_t j = 0; j < 8; ++j) { + single.eval(stack); + } + } + }; + BenchmarkTimer timer(budget); + while (timer.has_budget()) { + timer.before(); + my_loop(); + timer.after(); + } + return timer.min_time() * 1000.0 * 1000.0 / double(loop_cnt); } }; @@ -396,8 +434,12 @@ void benchmark(const vespalib::string &desc, const std::vector<EvalOp::UP> &list } } BenchmarkResult result(desc, list.size()); + std::vector<size_t> loop_cnt(list.size()); for (const auto &eval: list) { - double time = eval->estimate_cost_us(); + loop_cnt[eval->impl.order] = eval->suggest_loop_cnt(); + } + for (const auto &eval: list) { + double time = eval->estimate_cost_us(loop_cnt[eval->impl.order], loop_cnt[1]); result.sample(eval->impl.order, time); fprintf(stderr, " %s(%s): %10.3f us\n", eval->impl.name.c_str(), eval->impl.short_name.c_str(), time); } @@ -420,9 +462,10 @@ void benchmark_join(const vespalib::string &desc, const TensorSpec &lhs, ASSERT_FALSE(res_type.is_error()); std::vector<EvalOp::UP> list; for (const Impl &impl: impl_list) { - auto op = impl.create_join(lhs_type, rhs_type, function, stash); + Stash my_stash; + auto op = impl.create_join(lhs_type, rhs_type, function, my_stash); std::vector<CREF<TensorSpec>> stack_spec({lhs, rhs}); - list.push_back(std::make_unique<EvalOp>(op, stack_spec, impl)); + list.push_back(std::make_unique<EvalOp>(std::move(my_stash), op, stack_spec, impl)); } benchmark(desc, list); } @@ -439,9 +482,10 @@ void benchmark_reduce(const vespalib::string &desc, const TensorSpec &lhs, ASSERT_FALSE(res_type.is_error()); std::vector<EvalOp::UP> list; for (const Impl &impl: impl_list) { - auto op = impl.create_reduce(lhs_type, aggr, dims, stash); + Stash my_stash; + auto op = impl.create_reduce(lhs_type, aggr, dims, my_stash); std::vector<CREF<TensorSpec>> stack_spec({lhs}); - list.push_back(std::make_unique<EvalOp>(op, stack_spec, impl)); + list.push_back(std::make_unique<EvalOp>(std::move(my_stash), op, stack_spec, impl)); } benchmark(desc, list); } @@ -459,9 +503,10 @@ void benchmark_rename(const vespalib::string &desc, const TensorSpec &lhs, ASSERT_FALSE(res_type.is_error()); std::vector<EvalOp::UP> list; for (const Impl &impl: impl_list) { - auto op = impl.create_rename(lhs_type, from, to, stash); + Stash my_stash; + auto op = impl.create_rename(lhs_type, from, to, my_stash); std::vector<CREF<TensorSpec>> stack_spec({lhs}); - list.push_back(std::make_unique<EvalOp>(op, stack_spec, impl)); + list.push_back(std::make_unique<EvalOp>(std::move(my_stash), op, stack_spec, impl)); } benchmark(desc, list); } @@ -480,9 +525,10 @@ void benchmark_merge(const vespalib::string &desc, const TensorSpec &lhs, ASSERT_FALSE(res_type.is_error()); std::vector<EvalOp::UP> list; for (const Impl &impl: impl_list) { - auto op = impl.create_merge(lhs_type, rhs_type, function, stash); + Stash my_stash; + auto op = impl.create_merge(lhs_type, rhs_type, function, my_stash); std::vector<CREF<TensorSpec>> stack_spec({lhs, rhs}); - list.push_back(std::make_unique<EvalOp>(op, stack_spec, impl)); + list.push_back(std::make_unique<EvalOp>(std::move(my_stash), op, stack_spec, impl)); } benchmark(desc, list); } @@ -496,9 +542,10 @@ void benchmark_map(const vespalib::string &desc, const TensorSpec &lhs, operatio ASSERT_FALSE(lhs_type.is_error()); std::vector<EvalOp::UP> list; for (const Impl &impl: impl_list) { - auto op = impl.create_map(lhs_type, function, stash); + Stash my_stash; + auto op = impl.create_map(lhs_type, function, my_stash); std::vector<CREF<TensorSpec>> stack_spec({lhs}); - list.push_back(std::make_unique<EvalOp>(op, stack_spec, impl)); + list.push_back(std::make_unique<EvalOp>(std::move(my_stash), op, stack_spec, impl)); } benchmark(desc, list); } @@ -517,9 +564,10 @@ void benchmark_concat(const vespalib::string &desc, const TensorSpec &lhs, ASSERT_FALSE(res_type.is_error()); std::vector<EvalOp::UP> list; for (const Impl &impl: impl_list) { - auto op = impl.create_concat(lhs_type, rhs_type, dimension, stash); + Stash my_stash; + auto op = impl.create_concat(lhs_type, rhs_type, dimension, my_stash); std::vector<CREF<TensorSpec>> stack_spec({lhs, rhs}); - list.push_back(std::make_unique<EvalOp>(op, stack_spec, impl)); + list.push_back(std::make_unique<EvalOp>(std::move(my_stash), op, stack_spec, impl)); } benchmark(desc, list); } @@ -536,10 +584,11 @@ void benchmark_tensor_create(const vespalib::string &desc, const TensorSpec &pro } std::vector<EvalOp::UP> list; for (const Impl &impl: impl_list) { - auto op = impl.create_tensor_create(proto_type, proto, stash); - list.push_back(std::make_unique<EvalOp>(op, stack_spec, impl)); + Stash my_stash; + auto op = impl.create_tensor_create(proto_type, proto, my_stash); + list.push_back(std::make_unique<EvalOp>(std::move(my_stash), op, stack_spec, impl)); } - benchmark(desc, list); + benchmark(desc, list); } //----------------------------------------------------------------------------- @@ -550,8 +599,9 @@ void benchmark_tensor_lambda(const vespalib::string &desc, const ValueType &type ASSERT_FALSE(p0_type.is_error()); std::vector<EvalOp::UP> list; for (const Impl &impl: impl_list) { - auto op = impl.create_tensor_lambda(type, function, p0_type, stash); - list.push_back(std::make_unique<EvalOp>(op, p0, impl)); + Stash my_stash; + auto op = impl.create_tensor_lambda(type, function, p0_type, my_stash); + list.push_back(std::make_unique<EvalOp>(std::move(my_stash), op, p0, impl)); } benchmark(desc, list); } @@ -571,8 +621,9 @@ void benchmark_tensor_peek(const vespalib::string &desc, const TensorSpec &lhs, } std::vector<EvalOp::UP> list; for (const Impl &impl: impl_list) { - auto op = impl.create_tensor_peek(type, peek_spec, stash); - list.push_back(std::make_unique<EvalOp>(op, stack_spec, impl)); + Stash my_stash; + auto op = impl.create_tensor_peek(type, peek_spec, my_stash); + list.push_back(std::make_unique<EvalOp>(std::move(my_stash), op, stack_spec, impl)); } benchmark(desc, list); } @@ -610,11 +661,11 @@ void benchmark_encode_decode(const vespalib::string &desc, const TensorSpec &pro BenchmarkResult encode_result(desc + " <encode>", impl_list.size()); BenchmarkResult decode_result(desc + " <decode>", impl_list.size()); for (const Impl &impl: impl_list) { - constexpr size_t loop_cnt = 16; + constexpr size_t loop_cnt = 32; auto value = impl.create_value(proto); BenchmarkTimer encode_timer(2 * budget); BenchmarkTimer decode_timer(2 * budget); - while (encode_timer.has_budget() || decode_timer.has_budget()) { + while (encode_timer.has_budget()) { std::array<vespalib::nbostream, loop_cnt> data; std::array<Value::UP, loop_cnt> object; encode_timer.before(); diff --git a/eval/src/tests/tensor/partial_remove/partial_remove_test.cpp b/eval/src/tests/tensor/partial_remove/partial_remove_test.cpp index e182fffa890..5af2396f5ec 100644 --- a/eval/src/tests/tensor/partial_remove/partial_remove_test.cpp +++ b/eval/src/tests/tensor/partial_remove/partial_remove_test.cpp @@ -124,20 +124,31 @@ expect_partial_remove(const TensorSpec& input, const TensorSpec& remove, const T } TEST(PartialRemoveTest, remove_where_address_is_not_fully_specified) { - auto input = TensorSpec("tensor(x{},y{})"). + auto input_sparse = TensorSpec("tensor(x{},y{})"). add({{"x", "a"},{"y", "c"}}, 3.0). add({{"x", "a"},{"y", "d"}}, 5.0). add({{"x", "b"},{"y", "c"}}, 7.0); - expect_partial_remove(input,TensorSpec("tensor(x{})").add({{"x", "a"}}, 1.0), + expect_partial_remove(input_sparse, TensorSpec("tensor(x{})").add({{"x", "a"}}, 1.0), TensorSpec("tensor(x{},y{})").add({{"x", "b"},{"y", "c"}}, 7.0)); - expect_partial_remove(input, TensorSpec("tensor(y{})").add({{"y", "c"}}, 1.0), + expect_partial_remove(input_sparse, TensorSpec("tensor(y{})").add({{"y", "c"}}, 1.0), TensorSpec("tensor(x{},y{})").add({{"x", "a"},{"y", "d"}}, 5.0)); - expect_partial_remove(input, TensorSpec("tensor(y{})").add({{"y", "d"}}, 1.0), + expect_partial_remove(input_sparse, TensorSpec("tensor(y{})").add({{"y", "d"}}, 1.0), TensorSpec("tensor(x{},y{})").add({{"x", "a"},{"y", "c"}}, 3.0) .add({{"x", "b"},{"y", "c"}}, 7.0)); + + auto input_mixed = TensorSpec("tensor(x{},y{},z[1])"). + add({{"x", "a"},{"y", "c"},{"z", 0}}, 3.0). + add({{"x", "a"},{"y", "d"},{"z", 0}}, 5.0). + add({{"x", "b"},{"y", "c"},{"z", 0}}, 7.0); + + expect_partial_remove(input_mixed,TensorSpec("tensor(x{})").add({{"x", "a"}}, 1.0), + TensorSpec("tensor(x{},y{},z[1])").add({{"x", "b"},{"y", "c"},{"z", 0}}, 7.0)); + + expect_partial_remove(input_mixed, TensorSpec("tensor(y{})").add({{"y", "c"}}, 1.0), + TensorSpec("tensor(x{},y{},z[1])").add({{"x", "a"},{"y", "d"},{"z", 0}}, 5.0)); } GTEST_MAIN_RUN_ALL_TESTS() diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java index d0ee6229428..00327dc0002 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java @@ -94,7 +94,7 @@ public final class Node implements Nodelike { requireNonEmpty(ipConfig.primary(), "Active node " + hostname + " must have at least one valid IP address"); if (parentHostname.isPresent()) { - if (!ipConfig.pool().isEmpty()) throw new IllegalArgumentException("A child node cannot have an IP address pool"); + if (!ipConfig.pool().getIpSet().isEmpty()) throw new IllegalArgumentException("A child node cannot have an IP address pool"); if (modelName.isPresent()) throw new IllegalArgumentException("A child node cannot have model name set"); if (switchHostname.isPresent()) throw new IllegalArgumentException("A child node cannot have switch hostname set"); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java index 05bdfd25b76..03ff89d36dc 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java @@ -460,7 +460,7 @@ public class NodeRepository extends AbstractComponent { .map(node -> { if (node.state() != State.provisioned && node.state() != State.dirty) illegal("Can not set " + node + " ready. It is not provisioned or dirty."); - if (node.type() == NodeType.host && node.ipConfig().pool().isEmpty()) + if (node.type() == NodeType.host && node.ipConfig().pool().getIpSet().isEmpty()) illegal("Can not set host " + node + " ready. Its IP address pool is empty."); return node.withWantToRetire(false, false, Agent.system, clock.instant()); }) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Application.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Application.java index fd92b5b0ca0..847b825a7a4 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Application.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Application.java @@ -57,7 +57,7 @@ public class Application { public Application withCluster(ClusterSpec.Id id, boolean exclusive, ClusterResources min, ClusterResources max) { Cluster cluster = clusters.get(id); if (cluster == null) - cluster = new Cluster(id, exclusive, min, max, Optional.empty(), Optional.empty(), List.of()); + cluster = new Cluster(id, exclusive, min, max, Optional.empty(), Optional.empty(), List.of(), ""); else cluster = cluster.withConfiguration(exclusive, min, max); return with(cluster); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java index a17ee081447..90133f7499e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java @@ -25,6 +25,7 @@ public class Cluster { private final Optional<ClusterResources> suggested; private final Optional<ClusterResources> target; private final List<ScalingEvent> scalingEvents; + private final String autoscalingStatus; public Cluster(ClusterSpec.Id id, boolean exclusive, @@ -32,7 +33,8 @@ public class Cluster { ClusterResources maxResources, Optional<ClusterResources> suggestedResources, Optional<ClusterResources> targetResources, - List<ScalingEvent> scalingEvents) { + List<ScalingEvent> scalingEvents, + String autoscalingStatus) { this.id = Objects.requireNonNull(id); this.exclusive = exclusive; this.min = Objects.requireNonNull(minResources); @@ -44,6 +46,7 @@ public class Cluster { else this.target = targetResources; this.scalingEvents = scalingEvents; + this.autoscalingStatus = autoscalingStatus; } public ClusterSpec.Id id() { return id; } @@ -73,21 +76,33 @@ public class Cluster { /** Returns the recent scaling events in this cluster */ public List<ScalingEvent> scalingEvents() { return scalingEvents; } + public Optional<ScalingEvent> lastScalingEvent() { + if (scalingEvents.isEmpty()) return Optional.empty(); + return Optional.of(scalingEvents.get(scalingEvents.size() - 1)); + } + + /** The latest autoscaling status of this cluster, or empty (never null) if none */ + public String autoscalingStatus() { return autoscalingStatus; } + public Cluster withConfiguration(boolean exclusive, ClusterResources min, ClusterResources max) { - return new Cluster(id, exclusive, min, max, suggested, target, scalingEvents); + return new Cluster(id, exclusive, min, max, suggested, target, scalingEvents, autoscalingStatus); } public Cluster withSuggested(Optional<ClusterResources> suggested) { - return new Cluster(id, exclusive, min, max, suggested, target, scalingEvents); + return new Cluster(id, exclusive, min, max, suggested, target, scalingEvents, autoscalingStatus); } public Cluster withTarget(Optional<ClusterResources> target) { - return new Cluster(id, exclusive, min, max, suggested, target, scalingEvents); + return new Cluster(id, exclusive, min, max, suggested, target, scalingEvents, autoscalingStatus); } public Cluster with(ScalingEvent scalingEvent) { // NOTE: We're just storing the latest scaling event so far - return new Cluster(id, exclusive, min, max, suggested, target, List.of(scalingEvent)); + return new Cluster(id, exclusive, min, max, suggested, target, List.of(scalingEvent), autoscalingStatus); + } + + public Cluster withAutoscalingStatus(String autoscalingStatus) { + return new Cluster(id, exclusive, min, max, suggested, target, scalingEvents, autoscalingStatus); } @Override diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java index 1a8c4c8a6c2..d3ed7c6d850 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java @@ -3,14 +3,16 @@ package com.yahoo.vespa.hosted.provision.autoscale; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.NodeResources; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.applications.Cluster; import java.time.Duration; +import java.time.Instant; import java.util.List; +import java.util.Objects; import java.util.Optional; -import java.util.logging.Logger; /** * The autoscaler makes decisions about the flavor and node count that should be allocated to a cluster @@ -20,8 +22,6 @@ import java.util.logging.Logger; */ public class Autoscaler { - private static final Logger log = Logger.getLogger(Autoscaler.class.getName()); - /** What cost difference factor is worth a reallocation? */ private static final double costDifferenceWorthReallocation = 0.1; /** What difference factor for a resource is worth a reallocation? */ @@ -55,39 +55,37 @@ public class Autoscaler { * @return scaling advice for this cluster */ public Advice autoscale(Cluster cluster, List<Node> clusterNodes) { - if (cluster.minResources().equals(cluster.maxResources())) return Advice.none(); // Shortcut + if (cluster.minResources().equals(cluster.maxResources())) return Advice.none("Autoscaling is disabled"); // Shortcut return autoscale(cluster, clusterNodes, Limits.of(cluster), cluster.exclusive()); } private Advice autoscale(Cluster cluster, List<Node> clusterNodes, Limits limits, boolean exclusive) { - log.fine(() -> "Autoscale " + cluster.toString()); - - if (unstable(clusterNodes, nodeRepository)) { - log.fine(() -> "Unstable - Advice.none " + cluster.toString()); - return Advice.none(); - } + if (unstable(clusterNodes, nodeRepository)) + return Advice.none("Cluster change in progress"); - AllocatableClusterResources currentAllocation = new AllocatableClusterResources(clusterNodes, nodeRepository, cluster.exclusive()); + AllocatableClusterResources currentAllocation = + new AllocatableClusterResources(clusterNodes, nodeRepository, cluster.exclusive()); ClusterTimeseries clusterTimeseries = new ClusterTimeseries(cluster, clusterNodes, metricsDb, nodeRepository); Optional<Double> cpuLoad = clusterTimeseries.averageLoad(Resource.cpu, cluster); Optional<Double> memoryLoad = clusterTimeseries.averageLoad(Resource.memory, cluster); Optional<Double> diskLoad = clusterTimeseries.averageLoad(Resource.disk, cluster); - if (cpuLoad.isEmpty() || memoryLoad.isEmpty() || diskLoad.isEmpty()) return Advice.none(); + if (cpuLoad.isEmpty() || memoryLoad.isEmpty() || diskLoad.isEmpty()) + return Advice.none("Collecting more data before making new scaling decisions"); var target = ResourceTarget.idealLoad(cpuLoad.get(), memoryLoad.get(), diskLoad.get(), currentAllocation); Optional<AllocatableClusterResources> bestAllocation = allocationOptimizer.findBestAllocation(target, currentAllocation, limits, exclusive); - if (bestAllocation.isEmpty()) { - log.fine(() -> "bestAllocation.isEmpty: Advice.dontScale for " + cluster.toString()); - return Advice.dontScale(); - } - if (similar(bestAllocation.get(), currentAllocation)) { - log.fine(() -> "Current allocation similar: Advice.dontScale for " + cluster.toString()); - return Advice.dontScale(); - } + if (bestAllocation.isEmpty()) + return Advice.dontScale("No allocation changes are possible within configured limits"); + + if (similar(bestAllocation.get(), currentAllocation)) + return Advice.dontScale("Cluster is ideally scaled (within configured limits)"); + if (isDownscaling(bestAllocation.get(), currentAllocation) && recentlyScaled(cluster, clusterNodes)) + return Advice.dontScale("Waiting a while before scaling down"); + return Advice.scaleTo(bestAllocation.get().toAdvertisedClusterResources()); } @@ -106,10 +104,23 @@ public class Autoscaler { return Math.abs(r1 - r2) / (( r1 + r2) / 2) < threshold; } + /** Returns true if this reduces total resources in any dimension */ + private boolean isDownscaling(AllocatableClusterResources target, AllocatableClusterResources current) { + NodeResources targetTotal = target.toAdvertisedClusterResources().totalResources(); + NodeResources currentTotal = current.toAdvertisedClusterResources().totalResources(); + return ! targetTotal.justNumbers().satisfies(currentTotal.justNumbers()); + } + + private boolean recentlyScaled(Cluster cluster, List<Node> clusterNodes) { + Duration downscalingDelay = downscalingDelay(clusterNodes.get(0).allocation().get().membership().cluster().type()); + return cluster.lastScalingEvent().map(event -> event.at()).orElse(Instant.MIN) + .isAfter(nodeRepository.clock().instant().minus(downscalingDelay)); + } + /** The duration of the window we need to consider to make a scaling decision. See also minimumMeasurementsPerNode */ static Duration scalingWindow(ClusterSpec.Type clusterType) { if (clusterType.isContent()) return Duration.ofHours(12); - return Duration.ofHours(1); + return Duration.ofMinutes(30); } static Duration maxScalingWindow() { @@ -122,6 +133,15 @@ public class Autoscaler { return 20; } + /** + * We should wait a while before scaling down after a scaling event as a peak in usage + * indicates more peaks may arrive in the near future. + */ + static Duration downscalingDelay(ClusterSpec.Type clusterType) { + if (clusterType.isContent()) return Duration.ofHours(12); + return Duration.ofHours(1); + } + public static boolean unstable(List<Node> nodes, NodeRepository nodeRepository) { // The cluster is processing recent changes if (nodes.stream().anyMatch(node -> node.status().wantToRetire() || @@ -140,10 +160,12 @@ public class Autoscaler { private final boolean present; private final Optional<ClusterResources> target; + private final String reason; - private Advice(Optional<ClusterResources> target, boolean present) { + private Advice(Optional<ClusterResources> target, boolean present, String reason) { this.target = target; this.present = present; + this.reason = Objects.requireNonNull(reason); } /** @@ -158,10 +180,14 @@ public class Autoscaler { /** True if this provides advice (which may be to keep the current allocation) */ public boolean isPresent() { return present; } - private static Advice none() { return new Advice(Optional.empty(), false); } - private static Advice dontScale() { return new Advice(Optional.empty(), true); } - private static Advice scaleTo(ClusterResources target) { return new Advice(Optional.of(target), true); } + /** The reason for this advice */ + public String reason() { return reason; } + private static Advice none(String reason) { return new Advice(Optional.empty(), false, reason); } + private static Advice dontScale(String reason) { return new Advice(Optional.empty(), true, reason); } + private static Advice scaleTo(ClusterResources target) { + return new Advice(Optional.of(target), true, "Scaling due to load changes"); + } } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterTimeseries.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterTimeseries.java index bb91b77dce5..3c93e7ee7f6 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterTimeseries.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterTimeseries.java @@ -77,16 +77,8 @@ public class ClusterTimeseries { // Require a total number of measurements scaling with the number of nodes, // but don't require that we have at least that many from every node int measurementCount = currentMeasurements.stream().mapToInt(m -> m.size()).sum(); - if (measurementCount / clusterNodes.size() < Autoscaler.minimumMeasurementsPerNode(clusterType)) { - log.fine(() -> "Too few measurements per node for " + cluster.toString() + ": measurementCount " + measurementCount + - " (" + nodeTimeseries.stream().mapToInt(m -> m.size()).sum() + " before filtering"); - return Optional.empty(); - } - if (currentMeasurements.size() != clusterNodes.size()) { - log.fine(() -> "Mssing measurements from some nodes for " + cluster.toString() + ": Has from " + currentMeasurements.size() + - "but need " + clusterNodes.size() + "(before filtering: " + nodeTimeseries.size() + ")"); - return Optional.empty(); - } + if (measurementCount / clusterNodes.size() < Autoscaler.minimumMeasurementsPerNode(clusterType)) return Optional.empty(); + if (currentMeasurements.size() != clusterNodes.size()) return Optional.empty(); double measurementSum = currentMeasurements.stream().flatMap(m -> m.asList().stream()).mapToDouble(m -> value(resource, m)).sum(); return Optional.of(measurementSum / measurementCount); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java index b53f56e4743..809c54146d0 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java @@ -17,7 +17,6 @@ import com.yahoo.vespa.hosted.provision.autoscale.MetricsDb; import java.time.Duration; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; @@ -70,13 +69,13 @@ public class AutoscalingMaintainer extends NodeRepositoryMaintainer { Optional<Cluster> cluster = application.cluster(clusterId); if (cluster.isEmpty()) return; - log.fine(() -> "Autoscale " + application.toString()); - var advice = autoscaler.autoscale(cluster.get(), clusterNodes); - if (advice.isEmpty()) return; - - if ( ! cluster.get().targetResources().equals(advice.target())) { + application = application.with(cluster.get().withAutoscalingStatus(advice.reason())); + if (advice.isEmpty()) { + applications().put(application, deployment.applicationLock().get()); + } + else if ( ! cluster.get().targetResources().equals(advice.target())) { applications().put(application.with(cluster.get().withTarget(advice.target())), deployment.applicationLock().get()); if (advice.target().isPresent()) { logAutoscaling(advice.target().get(), applicationId, cluster.get(), clusterNodes); @@ -100,11 +99,7 @@ public class AutoscalingMaintainer extends NodeRepositoryMaintainer { } static String toString(ClusterResources r) { - return String.format(Locale.US, "%d%s * [vcpu: %.1f, memory: %.1f Gb, disk %.1f Gb]" + - " (total: [vcpu: %.1f, memory: %.1f Gb, disk: %.1f Gb])", - r.nodes(), r.groups() > 1 ? " (in " + r.groups() + " groups)" : "", - r.nodeResources().vcpu(), r.nodeResources().memoryGb(), r.nodeResources().diskGb(), - r.nodes() * r.nodeResources().vcpu(), r.nodes() * r.nodeResources().memoryGb(), r.nodes() * r.nodeResources().diskGb()); + return r + " (total: " + r.totalResources() + ")"; } private Map<ClusterSpec.Id, List<Node>> nodesByCluster(List<Node> applicationNodes) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java index 41d6c1e5425..bac31c40418 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java @@ -20,6 +20,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; +import java.util.stream.Stream; import static com.yahoo.config.provision.NodeType.confighost; import static com.yahoo.config.provision.NodeType.controllerhost; @@ -254,18 +255,25 @@ public class IP { * @return an allocation from the pool, if any can be made */ public Optional<Allocation> findAllocation(LockedNodeList nodes, NameResolver resolver) { + if (ipAddresses.asSet().isEmpty()) { + // IP addresses have not yet been resolved and should be done later. + return findUnusedAddressStream(nodes) + .map(Allocation::ofAddress) + .findFirst(); + } + if (ipAddresses.protocol == IpAddresses.Protocol.ipv4) { - return findUnused(nodes).stream() + return findUnusedIpAddresses(nodes).stream() .findFirst() .map(addr -> Allocation.ofIpv4(addr, resolver)); } - var unusedAddresses = findUnused(nodes); + var unusedAddresses = findUnusedIpAddresses(nodes); var allocation = unusedAddresses.stream() .filter(IP::isV6) .findFirst() .map(addr -> Allocation.ofIpv6(addr, resolver)); - allocation.flatMap(Allocation::secondary).ifPresent(ipv4Address -> { + allocation.flatMap(Allocation::ipv4Address).ifPresent(ipv4Address -> { if (!unusedAddresses.contains(ipv4Address)) { throw new IllegalArgumentException("Allocation resolved " + ipv4Address + " from hostname " + allocation.get().hostname + @@ -276,17 +284,43 @@ public class IP { } /** - * Finds all unused addresses in this pool + * Finds all unused IP addresses in this pool * * @param nodes a list of all nodes in the repository */ - public Set<String> findUnused(NodeList nodes) { + public Set<String> findUnusedIpAddresses(NodeList nodes) { var unusedAddresses = new LinkedHashSet<>(getIpSet()); nodes.matching(node -> node.ipConfig().primary().stream().anyMatch(ip -> getIpSet().contains(ip))) .forEach(node -> unusedAddresses.removeAll(node.ipConfig().primary())); return Collections.unmodifiableSet(unusedAddresses); } + /** + * Returns the number of unused IP addresses in the pool, assuming any and all unaccounted for hostnames + * in the pool are resolved to exactly 1 IP address (or 2 with {@link IpAddresses.Protocol#dualStack}). + */ + public int eventuallyUnusedAddressCount(NodeList nodes) { + // The address pool is filled immediately upon provisioning in dynamically provisioned zones, + // and within short time the IP address pool is filled. For all other cases, the IP address + // pool is already filled. + // + // The count in this method relies on the size of the IP address pool if that's non-empty, + // otherwise fall back to the address/hostname pool. + + + Set<String> currentIpAddresses = this.ipAddresses.asSet(); + if (!currentIpAddresses.isEmpty()) { + return findUnusedIpAddresses(nodes).size(); + } + + return (int) findUnusedAddressStream(nodes).count(); + } + + private Stream<Address> findUnusedAddressStream(NodeList nodes) { + Set<String> hostnames = nodes.stream().map(Node::hostname).collect(Collectors.toSet()); + return addresses.stream().filter(address -> !hostnames.contains(address.hostname())); + } + public IpAddresses.Protocol getProtocol() { return ipAddresses.protocol; } @@ -299,10 +333,6 @@ public class IP { return addresses; } - public boolean isEmpty() { - return getIpSet().isEmpty(); - } - public Pool withIpAddresses(Set<String> ipAddresses) { return Pool.of(ipAddresses, addresses); } @@ -326,22 +356,17 @@ public class IP { } - /** An IP address allocation from a pool */ + /** An address allocation from a pool */ public static class Allocation { private final String hostname; - private final String primary; - private final Optional<String> secondary; - - private Allocation(String hostname, String primary, Optional<String> secondary) { - Objects.requireNonNull(primary, "primary must be non-null"); - Objects.requireNonNull(secondary, "ipv4Address must be non-null"); - if (secondary.isPresent() && !isV4(secondary.get())) { // Secondary must be IPv4, if present - throw new IllegalArgumentException("Invalid IPv4 address '" + secondary + "'"); - } + private final Optional<String> ipv4Address; + private final Optional<String> ipv6Address; + + private Allocation(String hostname, Optional<String> ipv4Address, Optional<String> ipv6Address) { this.hostname = Objects.requireNonNull(hostname, "hostname must be non-null"); - this.primary = primary; - this.secondary = secondary; + this.ipv4Address = Objects.requireNonNull(ipv4Address, "ipv4Address must be non-null"); + this.ipv6Address = Objects.requireNonNull(ipv6Address, "ipv6Address must be non-null"); } /** @@ -350,13 +375,17 @@ public class IP { * A successful allocation is guaranteed to have an IPv6 address, but may also have an IPv4 address if the * hostname of the IPv6 address has an A record. * - * @param ipAddress Unassigned IPv6 address + * @param ipv6Address Unassigned IPv6 address * @param resolver DNS name resolver to use * @throws IllegalArgumentException if DNS is misconfigured * @return An allocation containing 1 IPv6 address and 1 IPv4 address (if hostname is dual-stack) */ - private static Allocation ofIpv6(String ipAddress, NameResolver resolver) { - String hostname6 = resolver.resolveHostname(ipAddress).orElseThrow(() -> new IllegalArgumentException("Could not resolve IP address: " + ipAddress)); + private static Allocation ofIpv6(String ipv6Address, NameResolver resolver) { + if (!isV6(ipv6Address)) { + throw new IllegalArgumentException("Invalid IPv6 address '" + ipv6Address + "'"); + } + + String hostname6 = resolver.resolveHostname(ipv6Address).orElseThrow(() -> new IllegalArgumentException("Could not resolve IP address: " + ipv6Address)); List<String> ipv4Addresses = resolver.resolveAll(hostname6).stream() .filter(IP::isV4) .collect(Collectors.toList()); @@ -369,10 +398,10 @@ public class IP { if (!hostname6.equals(hostname4)) { throw new IllegalArgumentException(String.format("Hostnames resolved from each IP address do not " + "point to the same hostname [%s -> %s, %s -> %s]", - ipAddress, hostname6, addr, hostname4)); + ipv6Address, hostname6, addr, hostname4)); } }); - return new Allocation(hostname6, ipAddress, ipv4Address); + return new Allocation(hostname6, ipv4Address, Optional.of(ipv6Address)); } /** @@ -391,7 +420,11 @@ public class IP { throw new IllegalArgumentException("Hostname " + hostname4 + " did not resolve to exactly 1 address. " + "Resolved: " + addresses); } - return new Allocation(hostname4, addresses.get(0), Optional.empty()); + return new Allocation(hostname4, Optional.of(addresses.get(0)), Optional.empty()); + } + + private static Allocation ofAddress(Address address) { + return new Allocation(address.hostname(), Optional.empty(), Optional.empty()); } /** Hostname pointing to the IP addresses in this */ @@ -399,27 +432,28 @@ public class IP { return hostname; } - /** Primary address of this allocation */ - public String primary() { - return primary; + /** IPv4 address of this allocation */ + public Optional<String> ipv4Address() { + return ipv4Address; } - /** Secondary address of this allocation */ - public Optional<String> secondary() { - return secondary; + /** IPv6 address of this allocation */ + public Optional<String> ipv6Address() { + return ipv6Address; } /** All IP addresses in this */ public Set<String> addresses() { ImmutableSet.Builder<String> builder = ImmutableSet.builder(); - secondary.ifPresent(builder::add); - builder.add(primary); + ipv4Address.ifPresent(builder::add); + ipv6Address.ifPresent(builder::add); return builder.build(); } @Override public String toString() { - return String.format("IP allocation [primary=%s, secondary=%s]", primary, secondary.orElse("<none>")); + return String.format("Address allocation [hostname=%s, IPv4=%s, IPv6=%s]", + hostname, ipv4Address.orElse("<none>"), ipv6Address.orElse("<none>")); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializer.java index 2ddbd6def6f..3979b898145 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializer.java @@ -47,6 +47,7 @@ public class ApplicationSerializer { private static final String groupsKey = "groups"; private static final String nodeResourcesKey = "resources"; private static final String scalingEventsKey = "scalingEvents"; + private static final String autoscalingStatusKey = "autoscalingStatus"; private static final String fromKey = "from"; private static final String toKey = "to"; private static final String generationKey = "generation"; @@ -95,6 +96,7 @@ public class ApplicationSerializer { cluster.suggestedResources().ifPresent(suggested -> toSlime(suggested, clusterObject.setObject(suggestedResourcesKey))); cluster.targetResources().ifPresent(target -> toSlime(target, clusterObject.setObject(targetResourcesKey))); scalingEventsToSlime(cluster.scalingEvents(), clusterObject.setArray(scalingEventsKey)); + clusterObject.setString(autoscalingStatusKey, cluster.autoscalingStatus()); } private static Cluster clusterFromSlime(String id, Inspector clusterObject) { @@ -104,7 +106,8 @@ public class ApplicationSerializer { clusterResourcesFromSlime(clusterObject.field(maxResourcesKey)), optionalClusterResourcesFromSlime(clusterObject.field(suggestedResourcesKey)), optionalClusterResourcesFromSlime(clusterObject.field(targetResourcesKey)), - scalingEventsFromSlime(clusterObject.field(scalingEventsKey))); + scalingEventsFromSlime(clusterObject.field(scalingEventsKey)), + clusterObject.field(autoscalingStatusKey).asString()); } private static void toSlime(ClusterResources resources, Cursor clusterResourcesObject) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java index b0baae650e4..6462fb6f19d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java @@ -71,47 +71,47 @@ public class GroupPreparer { } // There were some changes, so re-do the allocation with locks - try (Mutex lock = nodeRepository.lock(application)) { - try (Mutex allocationLock = nodeRepository.lockUnallocated()) { - NodeAllocation allocation = prepareAllocation(application, cluster, requestedNodes, surplusActiveNodes, - highestIndex, wantedGroups, allocationLock); - - if (nodeRepository.zone().getCloud().dynamicProvisioning()) { - Version osVersion = nodeRepository.osVersions().targetFor(NodeType.host).orElse(Version.emptyVersion); - List<ProvisionedHost> provisionedHosts = allocation.getFulfilledDockerDeficit() - .map(deficit -> hostProvisioner.get().provisionHosts(nodeRepository.database().getProvisionIndexes(deficit.getCount()), - deficit.getFlavor(), - application, - osVersion, - requestedNodes.isExclusive() ? HostSharing.exclusive : HostSharing.any)) - .orElseGet(List::of); - - // At this point we have started provisioning of the hosts, the first priority is to make sure that - // the returned hosts are added to the node-repo so that they are tracked by the provision maintainers - List<Node> hosts = provisionedHosts.stream() - .map(ProvisionedHost::generateHost) - .collect(Collectors.toList()); - nodeRepository.addNodes(hosts, Agent.application); - - // Offer the nodes on the newly provisioned hosts, this should be enough to cover the deficit - List<NodeCandidate> candidates = provisionedHosts.stream() - .map(host -> NodeCandidate.createNewExclusiveChild(host.generateNode(), - host.generateHost())) - .collect(Collectors.toList()); - allocation.offer(candidates); - } - - if (! allocation.fulfilled() && requestedNodes.canFail()) - throw new OutOfCapacityException((cluster.group().isPresent() ? "Out of capacity on " + cluster.group().get() :"") + - allocation.outOfCapacityDetails()); - - // Carry out and return allocation - nodeRepository.reserve(allocation.reservableNodes()); - nodeRepository.addDockerNodes(new LockedNodeList(allocation.newNodes(), allocationLock)); - List<Node> acceptedNodes = allocation.finalNodes(); - surplusActiveNodes.removeAll(acceptedNodes); - return acceptedNodes; + try (Mutex lock = nodeRepository.lock(application); + Mutex allocationLock = nodeRepository.lockUnallocated()) { + + NodeAllocation allocation = prepareAllocation(application, cluster, requestedNodes, surplusActiveNodes, + highestIndex, wantedGroups, allocationLock); + + if (nodeRepository.zone().getCloud().dynamicProvisioning()) { + Version osVersion = nodeRepository.osVersions().targetFor(NodeType.host).orElse(Version.emptyVersion); + List<ProvisionedHost> provisionedHosts = allocation.getFulfilledDockerDeficit() + .map(deficit -> hostProvisioner.get().provisionHosts(nodeRepository.database().getProvisionIndexes(deficit.getCount()), + deficit.getFlavor(), + application, + osVersion, + requestedNodes.isExclusive() ? HostSharing.exclusive : HostSharing.any)) + .orElseGet(List::of); + + // At this point we have started provisioning of the hosts, the first priority is to make sure that + // the returned hosts are added to the node-repo so that they are tracked by the provision maintainers + List<Node> hosts = provisionedHosts.stream() + .map(ProvisionedHost::generateHost) + .collect(Collectors.toList()); + nodeRepository.addNodes(hosts, Agent.application); + + // Offer the nodes on the newly provisioned hosts, this should be enough to cover the deficit + List<NodeCandidate> candidates = provisionedHosts.stream() + .map(host -> NodeCandidate.createNewExclusiveChild(host.generateNode(), + host.generateHost())) + .collect(Collectors.toList()); + allocation.offer(candidates); } + + if (! allocation.fulfilled() && requestedNodes.canFail()) + throw new OutOfCapacityException((cluster.group().isPresent() ? "Out of capacity on " + cluster.group().get() :"") + + allocation.outOfCapacityDetails()); + + // Carry out and return allocation + nodeRepository.reserve(allocation.reservableNodes()); + nodeRepository.addDockerNodes(new LockedNodeList(allocation.newNodes(), allocationLock)); + List<Node> acceptedNodes = allocation.finalNodes(); + surplusActiveNodes.removeAll(acceptedNodes); + return acceptedNodes; } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacity.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacity.java index 96053fdaa91..af3bde02421 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacity.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacity.java @@ -82,7 +82,11 @@ public class HostCapacity { * Number of free (not allocated) IP addresses assigned to the dockerhost. */ int freeIPs(Node dockerHost) { - return dockerHost.ipConfig().pool().findUnused(allNodes).size(); + if (dockerHost.type() == NodeType.host) { + return dockerHost.ipConfig().pool().eventuallyUnusedAddressCount(allNodes); + } else { + return dockerHost.ipConfig().pool().findUnusedIpAddresses(allNodes).size(); + } } /** diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java index f8231072a28..14937e6afeb 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java @@ -363,11 +363,11 @@ abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidate> { try { allocation = parent.get().ipConfig().pool().findAllocation(allNodes, nodeRepository.nameResolver()); if (allocation.isEmpty()) return new InvalidNodeCandidate(resources, freeParentCapacity, parent.get(), - "No IP addresses available on parent host"); + "No addresses available on parent host"); } catch (Exception e) { - log.warning("Failed allocating IP address on " + parent.get() +": " + Exceptions.toMessageString(e)); + log.warning("Failed allocating address on " + parent.get() +": " + Exceptions.toMessageString(e)); return new InvalidNodeCandidate(resources, freeParentCapacity, parent.get(), - "Failed when allocating IP address on host"); + "Failed when allocating address on host"); } Node node = Node.createDockerNode(allocation.get().addresses(), diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java index 9433b89ddc4..91b54fa37e9 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java @@ -8,6 +8,7 @@ import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.applications.Application; import com.yahoo.vespa.hosted.provision.applications.Cluster; +import com.yahoo.vespa.hosted.provision.applications.ScalingEvent; import com.yahoo.vespa.hosted.provision.autoscale.AllocatableClusterResources; import java.net.URI; @@ -51,6 +52,8 @@ public class ApplicationSerializer { toSlime(currentResources, clusterObject.setObject("current")); cluster.suggestedResources().ifPresent(suggested -> toSlime(suggested, clusterObject.setObject("suggested"))); cluster.targetResources().ifPresent(target -> toSlime(target, clusterObject.setObject("target"))); + scalingEventsToSlime(cluster.scalingEvents(), clusterObject.setArray("scalingEvents")); + clusterObject.setString("autoscalingStatus", cluster.autoscalingStatus()); } private static void toSlime(ClusterResources resources, Cursor clusterResourcesObject) { @@ -59,4 +62,13 @@ public class ApplicationSerializer { NodeResourcesSerializer.toSlime(resources.nodeResources(), clusterResourcesObject.setObject("resources")); } + private static void scalingEventsToSlime(List<ScalingEvent> scalingEvents, Cursor scalingEventsArray) { + for (ScalingEvent scalingEvent : scalingEvents) { + Cursor scalingEventObject = scalingEventsArray.addObject(); + toSlime(scalingEvent.from(), scalingEventObject.setObject("from")); + toSlime(scalingEvent.to(), scalingEventObject.setObject("to")); + scalingEventObject.setLong("at", scalingEvent.at().toEpochMilli()); + } + } + } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java index 5813a7067cd..5393aa7cfb8 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java @@ -17,6 +17,7 @@ import com.yahoo.vespa.hosted.provision.Nodelike; import com.yahoo.vespa.hosted.provision.provisioning.HostResourcesCalculator; import org.junit.Test; +import java.time.Duration; import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -44,11 +45,13 @@ public class AutoscalingTest { // deploy tester.deploy(application1, cluster1, 5, 1, hostResources); + tester.clock().advance(Duration.ofDays(1)); assertTrue("No measurements -> No change", tester.autoscale(application1, cluster1.id(), min, max).isEmpty()); tester.addCpuMeasurements(0.25f, 1f, 59, application1); assertTrue("Too few measurements -> No change", tester.autoscale(application1, cluster1.id(), min, max).isEmpty()); + tester.clock().advance(Duration.ofDays(1)); tester.addCpuMeasurements(0.25f, 1f, 60, application1); ClusterResources scaledResources = tester.assertResources("Scaling up since resource usage is too high", 15, 1, 1.3, 28.6, 28.6, @@ -58,6 +61,8 @@ public class AutoscalingTest { assertTrue("Cluster in flux -> No further change", tester.autoscale(application1, cluster1.id(), min, max).isEmpty()); tester.deactivateRetired(application1, cluster1, scaledResources); + + tester.clock().advance(Duration.ofDays(1)); tester.addCpuMeasurements(0.8f, 1f, 3, application1); assertTrue("Load change is large, but insufficient measurements for new config -> No change", tester.autoscale(application1, cluster1.id(), min, max).isEmpty()); @@ -112,6 +117,7 @@ public class AutoscalingTest { tester.nodeRepository().getNodes(application1).stream() .allMatch(n -> n.allocation().get().requestedResources().diskSpeed() == NodeResources.DiskSpeed.slow); + tester.clock().advance(Duration.ofDays(1)); tester.addCpuMeasurements(0.25f, 1f, 120, application1); // Changing min and max from slow to any ClusterResources min = new ClusterResources( 2, 1, @@ -184,7 +190,7 @@ public class AutoscalingTest { } @Test - public void test_autoscaling_limits_when_min_equals_xax() { + public void test_autoscaling_limits_when_min_equals_max() { NodeResources resources = new NodeResources(3, 100, 100, 1); ClusterResources min = new ClusterResources( 2, 1, new NodeResources(1, 1, 1, 1)); ClusterResources max = min; @@ -195,6 +201,7 @@ public class AutoscalingTest { // deploy tester.deploy(application1, cluster1, 5, 1, resources); + tester.clock().advance(Duration.ofDays(1)); tester.addCpuMeasurements(0.25f, 1f, 120, application1); assertTrue(tester.autoscale(application1, cluster1.id(), min, max).isEmpty()); } @@ -283,6 +290,31 @@ public class AutoscalingTest { // deploy tester.deploy(application1, cluster1, 6, 1, hostResources.withVcpu(hostResources.vcpu() / 2)); + tester.clock().advance(Duration.ofDays(1)); + tester.addMemMeasurements(0.02f, 0.95f, 120, application1); + tester.assertResources("Scaling down", + 6, 1, 2.8, 4.0, 95.0, + tester.autoscale(application1, cluster1.id(), min, max).target()); + } + + @Test + public void scaling_down_only_after_delay() { + NodeResources hostResources = new NodeResources(6, 100, 100, 1); + ClusterResources min = new ClusterResources( 2, 1, new NodeResources(1, 1, 1, 1)); + ClusterResources max = new ClusterResources(20, 1, new NodeResources(100, 1000, 1000, 1)); + AutoscalingTester tester = new AutoscalingTester(hostResources); + + ApplicationId application1 = tester.applicationId("application1"); + ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.content, "cluster1"); + + tester.deploy(application1, cluster1, 6, 1, hostResources.withVcpu(hostResources.vcpu() / 2)); + + // No autoscaling as it is too soon to scale down after initial deploy (counting as a scaling event) + tester.addMemMeasurements(0.02f, 0.95f, 120, application1); + assertTrue(tester.autoscale(application1, cluster1.id(), min, max).target().isEmpty()); + + // Trying the same a day later causes autoscaling + tester.clock().advance(Duration.ofDays(1)); tester.addMemMeasurements(0.02f, 0.95f, 120, application1); tester.assertResources("Scaling down", 6, 1, 2.8, 4.0, 95.0, @@ -344,6 +376,7 @@ public class AutoscalingTest { // deploy (Why 103 Gb memory? See AutoscalingTester.MockHostResourcesCalculator tester.deploy(application1, cluster1, 5, 1, new NodeResources(3, 103, 100, 1)); + tester.clock().advance(Duration.ofDays(1)); tester.addMemMeasurements(0.9f, 0.6f, 120, application1); ClusterResources scaledResources = tester.assertResources("Scaling up since resource usage is too high.", 8, 1, 3, 83, 34.3, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java index 5e318e00288..4b14174488e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java @@ -110,9 +110,8 @@ public class AutoscalingMaintainerTest { assertEquals(firstMaintenanceTime.toEpochMilli(), tester.deployer().lastDeployTime(app1).get().toEpochMilli()); // Add measurement of the expected generation, leading to rescaling - tester.clock().advance(Duration.ofSeconds(1)); + tester.clock().advance(Duration.ofHours(2)); tester.addMeasurements(0.1f, 0.1f, 0.1f, 1, 500, app1); - //tester.clock().advance(Duration.ofSeconds(1)); Instant lastMaintenanceTime = tester.clock().instant(); tester.maintainer().maintain(); assertEquals(lastMaintenanceTime.toEpochMilli(), tester.deployer().lastDeployTime(app1).get().toEpochMilli()); @@ -122,10 +121,10 @@ public class AutoscalingMaintainerTest { @Test public void test_toString() { - assertEquals("4 * [vcpu: 1.0, memory: 2.0 Gb, disk 4.0 Gb] (total: [vcpu: 4.0, memory: 8.0 Gb, disk: 16.0 Gb])", + assertEquals("4 nodes with [vcpu: 1.0, memory: 2.0 Gb, disk 4.0 Gb, bandwidth: 1.0 Gbps] (total: [vcpu: 4.0, memory: 8.0 Gb, disk 16.0 Gb, bandwidth: 4.0 Gbps])", AutoscalingMaintainer.toString(new ClusterResources(4, 1, new NodeResources(1, 2, 4, 1)))); - assertEquals("4 (in 2 groups) * [vcpu: 1.0, memory: 2.0 Gb, disk 4.0 Gb] (total: [vcpu: 4.0, memory: 8.0 Gb, disk: 16.0 Gb])", + assertEquals("4 nodes (in 2 groups) with [vcpu: 1.0, memory: 2.0 Gb, disk 4.0 Gb, bandwidth: 1.0 Gbps] (total: [vcpu: 4.0, memory: 8.0 Gb, disk 16.0 Gb, bandwidth: 4.0 Gbps])", AutoscalingMaintainer.toString(new ClusterResources(4, 2, new NodeResources(1, 2, 4, 1)))); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java index 22eec482b02..2833c4e11ba 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java @@ -209,12 +209,12 @@ public class DynamicProvisioningMaintainerTest { tester.maintainer.maintain(); assertTrue("No IP addresses written as DNS updates are failing", - provisioning.get().stream().allMatch(host -> host.ipConfig().pool().isEmpty())); + provisioning.get().stream().allMatch(host -> host.ipConfig().pool().getIpSet().isEmpty())); tester.hostProvisioner.without(Behaviour.failDnsUpdate); tester.maintainer.maintain(); assertTrue("IP addresses written as DNS updates are succeeding", - provisioning.get().stream().noneMatch(host -> host.ipConfig().pool().isEmpty())); + provisioning.get().stream().noneMatch(host -> host.ipConfig().pool().getIpSet().isEmpty())); } private static class DynamicProvisioningTester { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/node/IPTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/node/IPTest.java index fb9c1ad0e5a..8101405ad7f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/node/IPTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/node/IPTest.java @@ -86,8 +86,8 @@ public class IPTest { resolver.addReverseRecord("::2", "host1"); Optional<IP.Allocation> allocation = pool.findAllocation(emptyList, resolver); - assertEquals("::1", allocation.get().primary()); - assertFalse(allocation.get().secondary().isPresent()); + assertEquals(Optional.of("::1"), allocation.get().ipv6Address()); + assertFalse(allocation.get().ipv4Address().isPresent()); assertEquals("host3", allocation.get().hostname()); // Allocation fails if DNS record is missing @@ -105,16 +105,16 @@ public class IPTest { var pool = testPool(false); var allocation = pool.findAllocation(emptyList, resolver); assertFalse("Found allocation", allocation.isEmpty()); - assertEquals("127.0.0.1", allocation.get().primary()); - assertTrue("No secondary address", allocation.get().secondary().isEmpty()); + assertEquals(Optional.of("127.0.0.1"), allocation.get().ipv4Address()); + assertTrue("No IPv6 address", allocation.get().ipv6Address().isEmpty()); } @Test public void test_find_allocation_dual_stack() { IP.Pool pool = testPool(true); Optional<IP.Allocation> allocation = pool.findAllocation(emptyList, resolver); - assertEquals("::1", allocation.get().primary()); - assertEquals("127.0.0.2", allocation.get().secondary().get()); + assertEquals(Optional.of("::1"), allocation.get().ipv6Address()); + assertEquals("127.0.0.2", allocation.get().ipv4Address().get()); assertEquals("host3", allocation.get().hostname()); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializerTest.java index 72f9e9597de..e63f31cf304 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializerTest.java @@ -33,7 +33,8 @@ public class ApplicationSerializerTest { new ClusterResources(12, 6, new NodeResources(3, 6, 21, 24)), Optional.empty(), Optional.empty(), - List.of())); + List.of(), + "")); var minResources = new NodeResources(1, 2, 3, 4); clusters.add(new Cluster(ClusterSpec.Id.from("c2"), true, @@ -44,7 +45,8 @@ public class ApplicationSerializerTest { List.of(new ScalingEvent(new ClusterResources(10, 5, minResources), new ClusterResources(12, 6, minResources), 7L, - Instant.ofEpochMilli(12345L))))); + Instant.ofEpochMilli(12345L))), + "Autoscaling status")); Application original = new Application(ApplicationId.from("myTenant", "myApplication", "myInstance"), clusters); @@ -65,6 +67,7 @@ public class ApplicationSerializerTest { assertEquals(originalCluster.suggestedResources(), serializedCluster.suggestedResources()); assertEquals(originalCluster.targetResources(), serializedCluster.targetResources()); assertEquals(originalCluster.scalingEvents(), serializedCluster.scalingEvents()); + assertEquals(originalCluster.autoscalingStatus(), serializedCluster.autoscalingStatus()); } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java index c6e89680e85..808770f42dc 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java @@ -7,6 +7,7 @@ import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.provision.LockedNodeList; import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.node.Address; import com.yahoo.vespa.hosted.provision.node.IP; import org.junit.Before; import org.junit.Test; @@ -15,6 +16,8 @@ import java.util.ArrayList; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -32,8 +35,8 @@ public class HostCapacityTest { private HostCapacity capacity; private List<Node> nodes; private Node host1, host2, host3; - private final NodeResources resources1 = new NodeResources(1, 30, 20, 1.5); - private final NodeResources resources2 = new NodeResources(2, 40, 40, 0.5); + private final NodeResources dockerResources = new NodeResources(1, 30, 20, 1.5); + private final NodeResources docker2Resources = new NodeResources(2, 40, 40, 0.5); @Before public void setup() { @@ -48,15 +51,15 @@ public class HostCapacityTest { host3 = Node.create("host3", IP.Config.of(Set.of("::21"), generateIPs(22, 1), List.of()), "host3", nodeFlavors.getFlavorOrThrow("host"), NodeType.host).build(); // Add two containers to host1 - var nodeA = Node.createDockerNode(Set.of("::2"), "nodeA", "host1", resources1, NodeType.tenant).build(); - var nodeB = Node.createDockerNode(Set.of("::3"), "nodeB", "host1", resources1, NodeType.tenant).build(); + var nodeA = Node.createDockerNode(Set.of("::2"), "nodeA", "host1", dockerResources, NodeType.tenant).build(); + var nodeB = Node.createDockerNode(Set.of("::3"), "nodeB", "host1", dockerResources, NodeType.tenant).build(); // Add two containers to host 2 (same as host 1) - var nodeC = Node.createDockerNode(Set.of("::12"), "nodeC", "host2", resources1, NodeType.tenant).build(); - var nodeD = Node.createDockerNode(Set.of("::13"), "nodeD", "host2", resources1, NodeType.tenant).build(); + var nodeC = Node.createDockerNode(Set.of("::12"), "nodeC", "host2", dockerResources, NodeType.tenant).build(); + var nodeD = Node.createDockerNode(Set.of("::13"), "nodeD", "host2", dockerResources, NodeType.tenant).build(); // Add a larger container to host3 - var nodeE = Node.createDockerNode(Set.of("::22"), "nodeE", "host3", resources2, NodeType.tenant).build(); + var nodeE = Node.createDockerNode(Set.of("::22"), "nodeE", "host3", docker2Resources, NodeType.tenant).build(); // init docker host capacity nodes = new ArrayList<>(List.of(host1, host2, host3, nodeA, nodeB, nodeC, nodeD, nodeE)); @@ -65,19 +68,19 @@ public class HostCapacityTest { @Test public void hasCapacity() { - assertTrue(capacity.hasCapacity(host1, resources1)); - assertTrue(capacity.hasCapacity(host1, resources2)); - assertTrue(capacity.hasCapacity(host2, resources1)); - assertTrue(capacity.hasCapacity(host2, resources2)); - assertFalse(capacity.hasCapacity(host3, resources1)); // No ip available - assertFalse(capacity.hasCapacity(host3, resources2)); // No ip available + assertTrue(capacity.hasCapacity(host1, dockerResources)); + assertTrue(capacity.hasCapacity(host1, docker2Resources)); + assertTrue(capacity.hasCapacity(host2, dockerResources)); + assertTrue(capacity.hasCapacity(host2, docker2Resources)); + assertFalse(capacity.hasCapacity(host3, dockerResources)); // No ip available + assertFalse(capacity.hasCapacity(host3, docker2Resources)); // No ip available // Add a new node to host1 to deplete the memory resource - Node nodeF = Node.createDockerNode(Set.of("::6"), "nodeF", "host1", resources1, NodeType.tenant).build(); + Node nodeF = Node.createDockerNode(Set.of("::6"), "nodeF", "host1", dockerResources, NodeType.tenant).build(); nodes.add(nodeF); capacity = new HostCapacity(new LockedNodeList(nodes, () -> {}), hostResourcesCalculator); - assertFalse(capacity.hasCapacity(host1, resources1)); - assertFalse(capacity.hasCapacity(host1, resources2)); + assertFalse(capacity.hasCapacity(host1, dockerResources)); + assertFalse(capacity.hasCapacity(host1, docker2Resources)); } @Test @@ -112,19 +115,78 @@ public class HostCapacityTest { var nodeFlavors = FlavorConfigBuilder.createDummies("devhost", "container"); var devHost = Node.create("devhost", new IP.Config(Set.of("::1"), generateIPs(2, 10)), "devhost", nodeFlavors.getFlavorOrThrow("devhost"), NodeType.devhost).build(); - var cfg = Node.createDockerNode(Set.of("::2"), "cfg", "devhost", resources1, NodeType.config).build(); + var cfg = Node.createDockerNode(Set.of("::2"), "cfg", "devhost", dockerResources, NodeType.config).build(); var nodes = new ArrayList<>(List.of(cfg)); var capacity = new HostCapacity(new LockedNodeList(nodes, () -> {}), hostResourcesCalculator); - assertTrue(capacity.hasCapacity(devHost, resources1)); + assertTrue(capacity.hasCapacity(devHost, dockerResources)); - var container1 = Node.createDockerNode(Set.of("::3"), "container1", "devhost", resources1, NodeType.tenant).build(); + var container1 = Node.createDockerNode(Set.of("::3"), "container1", "devhost", dockerResources, NodeType.tenant).build(); nodes = new ArrayList<>(List.of(cfg, container1)); capacity = new HostCapacity(new LockedNodeList(nodes, () -> {}), hostResourcesCalculator); - assertFalse(capacity.hasCapacity(devHost, resources1)); + assertFalse(capacity.hasCapacity(devHost, dockerResources)); } + @Test + public void verifyCapacityFromAddresses() { + Node nodeA = Node.createDockerNode(Set.of("::2"), "nodeA", "host1", dockerResources, NodeType.tenant).build(); + Node nodeB = Node.createDockerNode(Set.of("::3"), "nodeB", "host1", dockerResources, NodeType.tenant).build(); + Node nodeC = Node.createDockerNode(Set.of("::4"), "nodeC", "host1", dockerResources, NodeType.tenant).build(); + + // host1 is a host with resources = 7-100-120-5 (7 vcpus, 100G memory, 120G disk, and 5Gbps), + // while nodeA-C have resources = dockerResources = 1-30-20-1.5 + + Node host1 = setupHostWithAdditionalHostnames("host1", "nodeA"); + // Allocating nodeA should be OK + assertTrue(hasCapacity(dockerResources, host1)); + // then, the second node lacks hostname address + assertFalse(hasCapacity(dockerResources, host1, nodeA)); + + host1 = setupHostWithAdditionalHostnames("host1", "nodeA", "nodeB"); + // Allocating nodeA and nodeB should be OK + assertTrue(hasCapacity(dockerResources, host1)); + assertTrue(hasCapacity(dockerResources, host1, nodeA)); + // but the third node lacks hostname address + assertFalse(hasCapacity(dockerResources, host1, nodeA, nodeB)); + + host1 = setupHostWithAdditionalHostnames("host1", "nodeA", "nodeB", "nodeC"); + // Allocating nodeA, nodeB, and nodeC should be OK + assertTrue(hasCapacity(dockerResources, host1)); + assertTrue(hasCapacity(dockerResources, host1, nodeA)); + assertTrue(hasCapacity(dockerResources, host1, nodeA, nodeB)); + // but the fourth node lacks hostname address + assertFalse(hasCapacity(dockerResources, host1, nodeA, nodeB, nodeC)); + + host1 = setupHostWithAdditionalHostnames("host1", "nodeA", "nodeB", "nodeC", "nodeD"); + // Allocating nodeA, nodeB, and nodeC should be OK + assertTrue(hasCapacity(dockerResources, host1)); + assertTrue(hasCapacity(dockerResources, host1, nodeA)); + assertTrue(hasCapacity(dockerResources, host1, nodeA, nodeB)); + // but the fourth lacks memory (host has 100G, while 4x30G = 120G + assertFalse(hasCapacity(dockerResources, host1, nodeA, nodeB, nodeC)); + } + + private Node setupHostWithAdditionalHostnames(String hostHostname, String... additionalHostnames) { + List<Address> addresses = Stream.of(additionalHostnames).map(Address::new).collect(Collectors.toList()); + + doAnswer(invocation -> ((Flavor)invocation.getArguments()[0]).resources()) + .when(hostResourcesCalculator).advertisedResourcesOf(any()); + + NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies( + "host", // 7-100-120-5 + "docker"); // 2- 40- 40-0.5 = docker2Resources + + return Node.create(hostHostname, IP.Config.of(Set.of("::1"), Set.of(), addresses), hostHostname, + nodeFlavors.getFlavorOrThrow("host"), NodeType.host).build(); + } + + private boolean hasCapacity(NodeResources requestedCapacity, Node host, Node... remainingNodes) { + List<Node> nodes = Stream.concat(Stream.of(host), Stream.of(remainingNodes)).collect(Collectors.toList()); + var capacity = new HostCapacity(new LockedNodeList(nodes, () -> {}), hostResourcesCalculator); + return capacity.hasCapacity(host, requestedCapacity); + } + private Set<String> generateIPs(int start, int count) { // Allow 4 containers Set<String> ipAddressPool = new LinkedHashSet<>(); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json index 456ed18334e..82f7e04f92b 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json @@ -62,7 +62,37 @@ "diskSpeed" : "fast", "storageType" : "any" } - } + }, + "scalingEvents" : [ + { + "from": { + "nodes": 0, + "groups": 0, + "resources": { + "vcpu" : 0.0, + "memoryGb": 0.0, + "diskGb": 0.0, + "bandwidthGbps": 0.0, + "diskSpeed": "fast", + "storageType": "any" + } + }, + "to": { + "nodes": 2, + "groups": 1, + "resources" : { + "vcpu": 2.0, + "memoryGb": 8.0, + "diskGb": 50.0, + "bandwidthGbps": 1.0, + "diskSpeed": "fast", + "storageType": "local" + } + }, + "at" : 123 + } + ], + "autoscalingStatus" : "" } } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json index bd22087ecfa..0ee590f60e0 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json @@ -38,7 +38,37 @@ "diskSpeed": "fast", "storageType": "local" } - } + }, + "scalingEvents" : [ + { + "from": { + "nodes": 0, + "groups": 0, + "resources": { + "vcpu" : 0.0, + "memoryGb": 0.0, + "diskGb": 0.0, + "bandwidthGbps": 0.0, + "diskSpeed": "fast", + "storageType": "any" + } + }, + "to": { + "nodes": 2, + "groups": 1, + "resources" : { + "vcpu": 2.0, + "memoryGb": 8.0, + "diskGb": 50.0, + "bandwidthGbps": 1.0, + "diskSpeed": "fast", + "storageType": "local" + } + }, + "at" : 123 + } + ], + "autoscalingStatus" : "" } } } |