diff options
183 files changed, 1493 insertions, 888 deletions
diff --git a/config-application-package/src/main/java/com/yahoo/config/application/Xml.java b/config-application-package/src/main/java/com/yahoo/config/application/Xml.java index c48a41083c7..f2a837026ea 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/Xml.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/Xml.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.config.application; import com.yahoo.config.application.api.ApplicationPackage; @@ -75,10 +75,6 @@ public class Xml { return factory.newDocumentBuilder(); } - static File getServices(File app) { - return new File(app, "services.xml"); // TODO Do not hard-code - } - static Document copyDocument(Document input) throws TransformerException { Transformer transformer = TransformerFactory.newInstance().newTransformer(); DOMSource source = new DOMSource(input); @@ -142,9 +138,7 @@ public class Xml { List<Element> children = XML.getChildren(parent, name); List<Element> allFromFiles = allElemsFromPath(app, pathFromAppRoot); for (Element fromFile : allFromFiles) { - for (Element inThatFile : XML.getChildren(fromFile, name)) { - children.add(inThatFile); - } + children.addAll(XML.getChildren(fromFile, name)); } return children; } diff --git a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java index db875f669fe..81fbc764bb6 100644 --- a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java +++ b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java @@ -301,7 +301,7 @@ public class FilesApplicationPackage implements ApplicationPackage { } @Override - public Collection<NamedReader> searchDefinitionContents() { + public Collection<NamedReader> getSchemas() { Set<NamedReader> ret = new LinkedHashSet<>(); try { for (File f : getSearchDefinitionFiles()) { @@ -575,11 +575,6 @@ public class FilesApplicationPackage implements ApplicationPackage { IOUtils.writeFile(metaFile, metaData.asJsonBytes()); } - @Override - public Collection<NamedReader> getSearchDefinitions() { - return searchDefinitionContents(); - } - private void preprocessXML(File destination, File inputXml, Zone zone) throws IOException { if ( ! inputXml.exists()) return; try { diff --git a/config-application-package/src/test/java/com/yahoo/config/application/IncludeProcessorTest.java b/config-application-package/src/test/java/com/yahoo/config/application/IncludeProcessorTest.java index 562970c266f..f8484a8e455 100644 --- a/config-application-package/src/test/java/com/yahoo/config/application/IncludeProcessorTest.java +++ b/config-application-package/src/test/java/com/yahoo/config/application/IncludeProcessorTest.java @@ -1,15 +1,16 @@ -// 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.config.application; +import com.yahoo.config.application.api.ApplicationPackage; import org.junit.Test; import org.w3c.dom.Document; import org.xml.sax.SAXException; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.ParserConfigurationException; -import javax.xml.stream.XMLStreamException; -import javax.xml.transform.*; -import java.io.*; +import javax.xml.transform.TransformerException; +import java.io.File; +import java.io.IOException; import java.nio.file.NoSuchFileException; /** @@ -72,7 +73,7 @@ public class IncludeProcessorTest { " </nodes>\n" + "</container></services>"; - Document doc = new IncludeProcessor(app).process(docBuilder.parse(Xml.getServices(app))); + Document doc = new IncludeProcessor(app).process(docBuilder.parse(getServices(app))); // System.out.println(Xml.documentAsString(doc)); TestBase.assertDocument(expected, doc); } @@ -81,7 +82,11 @@ public class IncludeProcessorTest { public void testRequiredIncludeIsDefault() throws ParserConfigurationException, IOException, SAXException, TransformerException { File app = new File("src/test/resources/multienvapp_failrequired"); DocumentBuilder docBuilder = Xml.getPreprocessDocumentBuilder(); - new IncludeProcessor(app).process(docBuilder.parse(Xml.getServices(app))); + new IncludeProcessor(app).process(docBuilder.parse(getServices(app))); + } + + static File getServices(File app) { + return new File(app, ApplicationPackage.SERVICES); } } diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index bdf2b53bc92..58d2693aace 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -103,7 +103,7 @@ "public abstract java.io.Reader getHosts()", "public java.util.List getUserIncludeDirs()", "public void validateIncludeDir(java.lang.String)", - "public abstract java.util.Collection searchDefinitionContents()", + "public java.util.Collection searchDefinitionContents()", "public abstract java.util.Map getAllExistingConfigDefs()", "public abstract java.util.List getFiles(com.yahoo.path.Path, java.lang.String, boolean)", "public java.util.List getFiles(com.yahoo.path.Path, java.lang.String)", @@ -127,7 +127,8 @@ "public void writeMetaData()", "public java.util.Optional getAllocatedHosts()", "public java.util.Map getFileRegistries()", - "public abstract java.util.Collection getSearchDefinitions()", + "public java.util.Collection getSearchDefinitions()", + "public abstract java.util.Collection getSchemas()", "public com.yahoo.config.application.api.ApplicationPackage preprocess(com.yahoo.config.provision.Zone, com.yahoo.config.application.api.DeployLogger)" ], "fields": [ diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java b/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java index d97ff5ca774..2aefc985f4b 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java @@ -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.config.application.api; import com.yahoo.component.Version; @@ -79,7 +79,7 @@ public interface ApplicationPackage { * @return the name of the application (i.e the directory where the application package was deployed from) * @deprecated do not use */ - @Deprecated // TODO: Remove on Vespa 8 + @Deprecated // TODO: Remove in Vespa 8 String getApplicationName(); ApplicationId getApplicationId(); @@ -87,14 +87,14 @@ public interface ApplicationPackage { /** * Contents of services.xml. Caller must close reader after use. * - * @return a Reader, or null if no services.xml/vespa-services.xml present + * @return a Reader, or null if no services.xml present */ Reader getServices(); /** * Contents of hosts.xml. Caller must close reader after use. * - * @return a Reader, or null if no hosts.xml/vespa-hosts.xml present + * @return a Reader, or null if no hosts.xml present */ Reader getHosts(); @@ -113,9 +113,12 @@ public interface ApplicationPackage { /** * Readers for all the search definition files for this. + * @deprecated use {@link #getSchemas()} instead * @return a list of readers for search definitions */ - Collection<NamedReader> searchDefinitionContents(); + @Deprecated + // TODO: Remove in Vespa 8 + default Collection<NamedReader> searchDefinitionContents() { return getSchemas(); } /** * Returns all the config definitions available in this package as unparsed data. @@ -143,10 +146,11 @@ public interface ApplicationPackage { /** Returns the major version this application is valid for, or empty if it is valid for all versions */ default Optional<Integer> getMajorVersion() { - if ( ! getDeployment().isPresent()) return Optional.empty(); + if (getDeployment().isEmpty()) return Optional.empty(); Element deployElement = XML.getDocument(getDeployment().get()).getDocumentElement(); if (deployElement == null) return Optional.empty(); + String majorVersionString = deployElement.getAttribute("major-version"); if (majorVersionString == null || majorVersionString.isEmpty()) return Optional.empty(); @@ -178,7 +182,6 @@ public interface ApplicationPackage { /** Returns handle for the file containing client certificate authorities */ default ApplicationFile getClientSecurityFile() { return getFile(SECURITY_DIR.append("clients.pem")); } - //For generating error messages String getHostSource(); String getServicesSource(); @@ -235,7 +238,18 @@ public interface ApplicationPackage { return Collections.emptyMap(); } - Collection<NamedReader> getSearchDefinitions(); + /** + * @deprecated use {@link #getSchemas()} instead + */ + @Deprecated + // TODO: Remove in Vespa 8 + default Collection<NamedReader> getSearchDefinitions() { return getSchemas(); } + + /** + * Readers for all the schema files. + * @return a collection of readers for schemas + */ + Collection<NamedReader> getSchemas(); /** * Preprocess an application for a given zone and return a new application package pointing to the preprocessed 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 8845431c71b..ea27b7f70d8 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 @@ -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.config.application.api; import com.google.common.collect.ImmutableList; @@ -67,7 +67,7 @@ public class ValidationOverrides { public boolean allows(String validationIdString, Instant now) { Optional<ValidationId> validationId = ValidationId.from(validationIdString); - if ( ! validationId.isPresent()) return false; // unknown id -> not allowed + if (validationId.isEmpty()) return false; // unknown id -> not allowed return allows(validationId.get(), now); } @@ -125,8 +125,8 @@ public class ValidationOverrides { .atStartOfDay().atZone(ZoneOffset.UTC).toInstant() .plus(Duration.ofDays(1)); // Make the override valid *on* the "until" date Optional<ValidationId> validationId = ValidationId.from(XML.getValue(allow)); - if (validationId.isPresent()) // skip unknown ids as they may be valid for other model versions - overrides.add(new ValidationOverrides.Allow(validationId.get(), until)); + // skip unknown ids as they may be valid for other model versions + validationId.ifPresent(id -> overrides.add(new Allow(id, until))); } return new ValidationOverrides(overrides, xmlForm); } diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index 42ef763fc62..5158f3ec488 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -83,6 +83,8 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"baldersheim"}) default boolean useBucketExecutorForPruneRemoved() { return true; } @ModelFeatureFlag(owners = {"baldersheim"}) default boolean useExternalRankExpressions() { return false; } @ModelFeatureFlag(owners = {"baldersheim"}) default boolean distributeExternalRankExpressions() { return false; } + @ModelFeatureFlag(owners = {"baldersheim"}) default int maxConcurrentMergesPerNode() { throw new UnsupportedOperationException("TODO specify default value"); } + @ModelFeatureFlag(owners = {"baldersheim"}) default int maxMergeQueueSize() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"geirst"}) default boolean enableFeedBlockInDistributor() { return true; } @ModelFeatureFlag(owners = {"hmusum"}, removeAfter = "7.406") default int clusterControllerMaxHeapSizeInMb() { return 128; } @ModelFeatureFlag(owners = {"hmusum"}) default int metricsProxyMaxHeapSizeInMb(ClusterSpec.Type type) { return 256; } @@ -93,6 +95,7 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"bjorncs", "jonmv"}, removeAfter = "7.409") default boolean enableJdiscHttp2() { return true; } @ModelFeatureFlag(owners = {"tokle", "bjorncs"}) default boolean enableCustomAclMapping() { return false; } @ModelFeatureFlag(owners = {"geirst", "vekterli"}) default int numDistributorStripes() { return 0; } + @ModelFeatureFlag(owners = {"arnej"}) default boolean requireConnectivityCheck() { return false; } } /** Warning: As elsewhere in this package, do not make backwards incompatible changes that will break old config models! */ diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java index 1d0541b67d1..68924dde3e1 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java @@ -22,7 +22,7 @@ import com.yahoo.config.model.api.ValidationParameters; import com.yahoo.config.model.application.provider.BaseDeployLogger; import com.yahoo.config.model.application.provider.MockFileRegistry; import com.yahoo.config.model.provision.HostsXmlProvisioner; -import com.yahoo.config.model.provision.SingleNodeProvisioner; +import com.yahoo.config.model.provision .SingleNodeProvisioner; import com.yahoo.config.model.test.MockApplicationPackage; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Zone; @@ -460,7 +460,7 @@ public class DeployState implements ConfigDefinitionStore { private SearchDocumentModel createSearchDocumentModel(RankProfileRegistry rankProfileRegistry, QueryProfiles queryProfiles, ValidationParameters validationParameters) { - Collection<NamedReader> readers = applicationPackage.getSearchDefinitions(); + Collection<NamedReader> readers = applicationPackage.getSchemas(); Map<String, String> names = new LinkedHashMap<>(); SearchBuilder builder = new SearchBuilder(applicationPackage, logger, properties, rankProfileRegistry, queryProfiles.getRegistry()); for (NamedReader reader : readers) { @@ -470,14 +470,14 @@ public class DeployState implements ConfigDefinitionStore { String sdName = stripSuffix(readerName, ApplicationPackage.SD_NAME_SUFFIX); names.put(topLevelName, sdName); if ( ! sdName.equals(topLevelName)) { - throw new IllegalArgumentException("Schema definition file name ('" + sdName + "') and name of " + + throw new IllegalArgumentException("Schema file name ('" + sdName + "') and name of " + "top level element ('" + topLevelName + "') are not equal for file '" + readerName + "'"); } } catch (ParseException e) { - throw new IllegalArgumentException("Could not parse sd file '" + reader.getName() + "'", e); + throw new IllegalArgumentException("Could not parse schema file '" + reader.getName() + "'", e); } catch (IOException e) { - throw new IllegalArgumentException("Could not read sd file '" + reader.getName() + "'", e); + throw new IllegalArgumentException("Could not read schema file '" + reader.getName() + "'", e); } finally { closeIgnoreException(reader.getReader()); } diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java index 304855e545d..2927df57bc1 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java @@ -59,6 +59,8 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea private List<TenantSecretStore> tenantSecretStores = Collections.emptyList(); private String jvmOmitStackTraceInFastThrowOption; private int numDistributorStripes = 0; + private int maxConcurrentMergesPerNode = 16; + private int maxMergeQueueSize = 1024; private boolean allowDisableMtls = true; private List<X509Certificate> operatorCertificates = Collections.emptyList(); @@ -102,6 +104,8 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea @Override public List<X509Certificate> operatorCertificates() { return operatorCertificates; } @Override public boolean useExternalRankExpressions() { return useExternalRankExpression; } @Override public boolean distributeExternalRankExpressions() { return useExternalRankExpression; } + @Override public int maxConcurrentMergesPerNode() { return maxConcurrentMergesPerNode; } + @Override public int maxMergeQueueSize() { return maxMergeQueueSize; } public TestProperties useExternalRankExpression(boolean value) { useExternalRankExpression = value; @@ -134,6 +138,15 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea return this; } + public TestProperties setMaxConcurrentMergesPerNode(int maxConcurrentMergesPerNode) { + this.maxConcurrentMergesPerNode = maxConcurrentMergesPerNode; + return this; + } + public TestProperties setMaxMergeQueueSize(int maxMergeQueueSize) { + this.maxMergeQueueSize = maxMergeQueueSize; + return this; + } + public TestProperties setDefaultTermwiseLimit(double limit) { defaultTermwiseLimit = limit; return this; diff --git a/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java b/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java index e9fe0824f30..411a37bb70a 100644 --- a/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java +++ b/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java @@ -115,7 +115,7 @@ public class MockApplicationPackage implements ApplicationPackage { } @Override - public List<NamedReader> getSearchDefinitions() { + public List<NamedReader> getSchemas() { ArrayList<NamedReader> readers = new ArrayList<>(); SearchBuilder searchBuilder = new SearchBuilder(this, new BaseDeployLogger(), @@ -134,11 +134,6 @@ public class MockApplicationPackage implements ApplicationPackage { } @Override - public List<NamedReader> searchDefinitionContents() { - return new ArrayList<>(); - } - - @Override public Map<ConfigDefinitionKey, UnparsedConfigDefinition> getAllExistingConfigDefs() { return Collections.emptyMap(); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/ConfigSentinel.java b/config-model/src/main/java/com/yahoo/vespa/model/ConfigSentinel.java index d15db6b4a55..800bf73cdbb 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/ConfigSentinel.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/ConfigSentinel.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; +import com.yahoo.config.model.api.ModelContext; import com.yahoo.cloud.config.SentinelConfig; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Zone; @@ -18,16 +19,20 @@ public class ConfigSentinel extends AbstractService implements SentinelConfig.Pr private final ApplicationId applicationId; private final Zone zone; + private final boolean requireConnectivityCheck; /** * Constructs a new ConfigSentinel for the given host. * * @param host Physical host on which to run. */ - public ConfigSentinel(Host host, ApplicationId applicationId, Zone zone) { + public ConfigSentinel(Host host, ApplicationId applicationId, Zone zone, + ModelContext.FeatureFlags featureFlags) + { super(host, "sentinel"); this.applicationId = applicationId; this.zone = zone; + this.requireConnectivityCheck = featureFlags.requireConnectivityCheck(); portsMeta.on(0).tag("rpc").tag("admin"); portsMeta.on(1).tag("telnet").tag("interactive").tag("http").tag("state"); setProp("clustertype", "hosts"); @@ -75,6 +80,19 @@ public class ConfigSentinel extends AbstractService implements SentinelConfig.Pr builder.service(getServiceConfig(s)); } } + builder.connectivity(getConnectivityConfig(requireConnectivityCheck)); + } + + private SentinelConfig.Connectivity.Builder getConnectivityConfig(boolean enable) { + var builder = new SentinelConfig.Connectivity.Builder(); + if (enable) { + builder.maxBadOutPercent(60); + builder.maxBadReverseCount(3); + } else { + builder.maxBadOutPercent(100); + builder.maxBadReverseCount(Integer.MAX_VALUE); + } + return builder; } private SentinelConfig.Application.Builder getApplicationConfig() { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/HostSystem.java b/config-model/src/main/java/com/yahoo/vespa/model/HostSystem.java index 09bbd446803..53f42866d8d 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/HostSystem.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/HostSystem.java @@ -47,16 +47,15 @@ public class HostSystem extends AbstractConfigProducer<Host> { void checkName(String hostname) { // Give a warning if the host does not exist try { - @SuppressWarnings("unused") - Object ignore = java.net.InetAddress.getByName(hostname); + var inetAddr = java.net.InetAddress.getByName(hostname); + String canonical = inetAddr.getCanonicalHostName(); + if (! hostname.equals(canonical)) { + deployLogger.logApplicationPackage(Level.WARNING, "Host named '" + hostname + "' may not receive any config " + + "since it differs from its canonical hostname '" + canonical + "' (check DNS and /etc/hosts)."); + } } catch (UnknownHostException e) { deployLogger.logApplicationPackage(Level.WARNING, "Unable to lookup IP address of host: " + hostname); } - if (! hostname.contains(".")) { - deployLogger.logApplicationPackage(Level.WARNING, "Host named '" + hostname + "' may not receive any config " + - "since it is not a canonical hostname. " + - "Disregard this warning when testing in a Docker container."); - } } /** diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java index e080ce43730..a2a6ada9093 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.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.admin; +import com.yahoo.config.model.api.ModelContext; import com.yahoo.cloud.config.SlobroksConfig; import com.yahoo.cloud.config.ZookeepersConfig; import com.yahoo.cloud.config.log.LogdConfig; @@ -242,7 +243,8 @@ public class Admin extends AbstractConfigProducer<Admin> implements Serializable } private void addCommonServices(HostResource host, DeployState deployState) { - addConfigSentinel(deployState.getDeployLogger(), host, deployState.getProperties().applicationId(), deployState.zone()); + addConfigSentinel(deployState.getDeployLogger(), host, deployState.getProperties().applicationId(), deployState.zone(), + deployState.featureFlags()); addLogd(deployState.getDeployLogger(), host); addConfigProxy(deployState.getDeployLogger(), host); addFileDistribution(host); @@ -262,8 +264,10 @@ public class Admin extends AbstractConfigProducer<Admin> implements Serializable } } - private void addConfigSentinel(DeployLogger deployLogger, HostResource host, ApplicationId applicationId, Zone zone) { - ConfigSentinel configSentinel = new ConfigSentinel(host.getHost(), applicationId, zone); + private void addConfigSentinel(DeployLogger deployLogger, HostResource host, + ApplicationId applicationId, Zone zone, ModelContext.FeatureFlags featureFlags) + { + ConfigSentinel configSentinel = new ConfigSentinel(host.getHost(), applicationId, zone, featureFlags); addAndInitializeService(deployLogger, host, configSentinel); host.getHost().setConfigSentinel(configSentinel); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java index 034bf772ffc..114a3e380ef 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java @@ -256,8 +256,11 @@ public class VespaMetricSet { metrics.add(new Metric("cluster-controller.node-event.count")); metrics.add(new Metric("cluster-controller.resource_usage.nodes_above_limit.last")); + metrics.add(new Metric("cluster-controller.resource_usage.nodes_above_limit.max")); metrics.add(new Metric("cluster-controller.resource_usage.max_memory_utilization.last")); + metrics.add(new Metric("cluster-controller.resource_usage.max_memory_utilization.max")); metrics.add(new Metric("cluster-controller.resource_usage.max_disk_utilization.last")); + metrics.add(new Metric("cluster-controller.resource_usage.max_disk_utilization.max")); metrics.add(new Metric("cluster-controller.resource_usage.disk_limit.last")); metrics.add(new Metric("cluster-controller.resource_usage.memory_limit.last")); @@ -762,6 +765,15 @@ public class VespaMetricSet { metrics.add(new Metric("vds.bouncer.clock_skew_aborts.count")); + metrics.add(new Metric("vds.mergethrottler.averagequeuewaitingtime.max")); + metrics.add(new Metric("vds.mergethrottler.averagequeuewaitingtime.sum")); + metrics.add(new Metric("vds.mergethrottler.averagequeuewaitingtime.count")); + metrics.add(new Metric("vds.mergethrottler.queuesize.max")); + metrics.add(new Metric("vds.mergethrottler.queuesize.sum")); + metrics.add(new Metric("vds.mergethrottler.queuesize.count")); + metrics.add(new Metric("vds.mergethrottler.bounced_due_to_back_pressure.rate")); + metrics.add(new Metric("vds.mergethrottler.locallyexecutedmerges.ok.rate")); + metrics.add(new Metric("vds.mergethrottler.mergechains.ok.rate")); return metrics; } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorServerProducer.java b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorServerProducer.java index 40a634fbfe8..e89d45e8b83 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorServerProducer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorServerProducer.java @@ -6,6 +6,8 @@ import com.yahoo.vespa.config.content.core.StorServerConfig; import com.yahoo.vespa.model.content.cluster.ContentCluster; import com.yahoo.vespa.model.builder.xml.dom.ModelElement; +import java.util.Optional; + /** * Serves config for stor-server for storage clusters (clusters of storage nodes). */ @@ -14,7 +16,7 @@ public class StorServerProducer implements StorServerConfig.Producer { StorServerProducer build(ModelContext.Properties properties, ModelElement element) { ModelElement tuning = element.child("tuning"); - StorServerProducer producer = new StorServerProducer(ContentCluster.getClusterId(element)); + StorServerProducer producer = new StorServerProducer(ContentCluster.getClusterId(element), properties.featureFlags()); if (tuning == null) return producer; ModelElement merges = tuning.child("merges"); @@ -32,11 +34,15 @@ public class StorServerProducer implements StorServerConfig.Producer { private Integer bucketDBStripeBits; private StorServerProducer setMaxMergesPerNode(Integer value) { - maxMergesPerNode = value; + if (value != null) { + maxMergesPerNode = value; + } return this; } private StorServerProducer setMaxQueueSize(Integer value) { - queueSize = value; + if (value != null) { + queueSize = value; + } return this; } private StorServerProducer setBucketDBStripeBits(Integer value) { @@ -44,8 +50,10 @@ public class StorServerProducer implements StorServerConfig.Producer { return this; } - public StorServerProducer(String clusterName) { + StorServerProducer(String clusterName, ModelContext.FeatureFlags featureFlags) { this.clusterName = clusterName; + maxMergesPerNode = featureFlags.maxConcurrentMergesPerNode(); + queueSize = featureFlags.maxMergeQueueSize(); } @Override diff --git a/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java b/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java index e7622816603..8f751631fb5 100644 --- a/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java +++ b/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java @@ -54,7 +54,7 @@ public class ApplicationDeployTest { @Test public void testVespaModel() throws SAXException, IOException { ApplicationPackageTester tester = ApplicationPackageTester.create(TESTDIR + "app1"); - VespaModel model = new VespaModel(tester.app()); + new VespaModel(tester.app()); List<NamedSchema> schemas = tester.getSchemas(); assertEquals(schemas.size(), 5); for (NamedSchema searchDefinition : schemas) { diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java index b1b386924d1..b0ddadf11bd 100644 --- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java +++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java @@ -49,6 +49,7 @@ import static com.yahoo.config.model.test.TestUtil.joinLines; import static com.yahoo.vespa.defaults.Defaults.getDefaults; import static com.yahoo.vespa.model.search.NodeResourcesTuning.GB; import static com.yahoo.vespa.model.search.NodeResourcesTuning.reservedMemoryGb; +import static com.yahoo.vespa.model.test.utils.ApplicationPackageUtils.generateSchemas; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -2015,7 +2016,7 @@ public class ModelProvisioningTest { } private VespaModel createNonProvisionedModel(boolean multitenant, String hosts, String services) { - VespaModelCreatorWithMockPkg modelCreatorWithMockPkg = new VespaModelCreatorWithMockPkg(hosts, services, ApplicationPackageUtils.generateSearchDefinition("type1")); + VespaModelCreatorWithMockPkg modelCreatorWithMockPkg = new VespaModelCreatorWithMockPkg(hosts, services, generateSchemas("type1")); ApplicationPackage appPkg = modelCreatorWithMockPkg.appPkg; DeployState deployState = new DeployState.Builder().applicationPackage(appPkg). properties((new TestProperties()).setMultitenant(multitenant)). @@ -2023,7 +2024,7 @@ public class ModelProvisioningTest { return modelCreatorWithMockPkg.create(false, deployState); } - private int physicalMemoryPercentage(ContainerCluster cluster) { + private int physicalMemoryPercentage(ContainerCluster<?> cluster) { QrStartConfig.Builder b = new QrStartConfig.Builder(); cluster.getConfig(b); return b.build().jvm().heapSizeAsPercentageOfPhysicalMemory(); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java index 76f34cf4a81..341a90c6618 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java @@ -15,6 +15,7 @@ import org.junit.rules.ExpectedException; import org.xml.sax.SAXException; import java.io.IOException; +import java.util.List; import static com.yahoo.config.model.test.TestUtil.joinLines; @@ -99,17 +100,17 @@ public class ComplexAttributeFieldsValidatorTestCase { "}")); } - private static void createModelAndValidate(String searchDefinition) throws IOException, SAXException { - DeployState deployState = createDeployState(servicesXml(), searchDefinition); + private static void createModelAndValidate(String schema) throws IOException, SAXException { + DeployState deployState = createDeployState(servicesXml(), schema); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); ValidationParameters validationParameters = new ValidationParameters(CheckRouting.FALSE); Validation.validate(model, validationParameters, deployState); } - private static DeployState createDeployState(String servicesXml, String searchDefinition) { + private static DeployState createDeployState(String servicesXml, String schema) { ApplicationPackage app = new MockApplicationPackage.Builder() .withServices(servicesXml) - .withSearchDefinition(searchDefinition) + .withSchemas(List.of(schema)) .build(); return new DeployState.Builder().applicationPackage(app).build(); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/StorageClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/StorageClusterTest.java index 5cf57430f91..9a681003293 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/StorageClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/StorageClusterTest.java @@ -17,6 +17,7 @@ import com.yahoo.vespa.config.content.PersistenceConfig; import com.yahoo.config.model.test.MockRoot; import com.yahoo.documentmodel.NewDocumentType; import static com.yahoo.vespa.defaults.Defaults.getDefaults; +import static com.yahoo.config.model.test.TestUtil.joinLines; import com.yahoo.vespa.model.content.cluster.ContentCluster; import com.yahoo.vespa.model.content.storagecluster.StorageCluster; import com.yahoo.vespa.model.content.utils.ContentClusterUtils; @@ -44,10 +45,17 @@ public class StorageClusterTest { return parse(xml, root); } - StorageCluster parse(String xml) { - MockRoot root = new MockRoot(); + StorageCluster parse(String xml, ModelContext.Properties properties) { + MockRoot root = new MockRoot("", + new DeployState.Builder() + .properties(properties) + .applicationPackage(new MockApplicationPackage.Builder().build()) + .build()); return parse(xml, root); } + StorageCluster parse(String xml) { + return parse(xml, new TestProperties()); + } StorageCluster parse(String xml, MockRoot root) { root.getDeployState().getDocumentModel().getDocumentManager().add( new NewDocumentType(new NewDocumentType.Name("music")) @@ -61,13 +69,23 @@ public class StorageClusterTest { return cluster.getStorageNodes(); } + private static String group() { + return joinLines( + "<group>", + " <node distribution-key=\"0\" hostalias=\"mockhost\"/>", + "</group>"); + } + private static String cluster(String clusterName, String insert) { + return joinLines( + "<content id=\"" + clusterName + "\">", + "<documents/>", + insert, + group(), + "</content>"); + } @Test public void testBasics() { - StorageCluster storage = parse("<content id=\"foofighters\"><documents/>\n" + - " <group>" + - " <node distribution-key=\"0\" hostalias=\"mockhost\"/>" + - " </group>" + - "</content>\n"); + StorageCluster storage = parse(cluster("foofighters", "")); assertEquals(1, storage.getChildren().size()); StorServerConfig.Builder builder = new StorServerConfig.Builder(); @@ -79,11 +97,7 @@ public class StorageClusterTest { } @Test public void testCommunicationManagerDefaults() { - StorageCluster storage = parse("<content id=\"foofighters\"><documents/>\n" + - " <group>" + - " <node distribution-key=\"0\" hostalias=\"mockhost\"/>" + - " </group>" + - "</content>\n"); + StorageCluster storage = parse(cluster("foofighters", "")); StorCommunicationmanagerConfig.Builder builder = new StorCommunicationmanagerConfig.Builder(); storage.getChildren().get("0").getConfig(builder); StorCommunicationmanagerConfig config = new StorCommunicationmanagerConfig(builder); @@ -97,40 +111,49 @@ public class StorageClusterTest { } @Test + public void testMergeDefaults() { + StorServerConfig.Builder builder = new StorServerConfig.Builder(); + parse(cluster("foofighters", "")).getConfig(builder); + + StorServerConfig config = new StorServerConfig(builder); + assertEquals(16, config.max_merges_per_node()); + assertEquals(1024, config.max_merge_queue_size()); + } + + @Test public void testMerges() { StorServerConfig.Builder builder = new StorServerConfig.Builder(); - parse("" + - "<content id=\"foofighters\">\n" + - " <documents/>" + - " <tuning>" + - " <merges max-per-node=\"1K\" max-queue-size=\"10K\"/>\n" + - " </tuning>" + - " <group>" + - " <node distribution-key=\"0\" hostalias=\"mockhost\"/>" + - " </group>" + - "</content>" + parse(cluster("foofighters", joinLines( + "<tuning>", + " <merges max-per-node=\"1K\" max-queue-size=\"10K\"/>", + "</tuning>")), + new TestProperties().setMaxMergeQueueSize(1919).setMaxConcurrentMergesPerNode(37) ).getConfig(builder); StorServerConfig config = new StorServerConfig(builder); assertEquals(1024, config.max_merges_per_node()); assertEquals(1024*10, config.max_merge_queue_size()); } + @Test + public void testMergeFeatureFlags() { + StorServerConfig.Builder builder = new StorServerConfig.Builder(); + parse(cluster("foofighters", ""), new TestProperties().setMaxMergeQueueSize(1919).setMaxConcurrentMergesPerNode(37)).getConfig(builder); + + StorServerConfig config = new StorServerConfig(builder); + assertEquals(37, config.max_merges_per_node()); + assertEquals(1919, config.max_merge_queue_size()); + } @Test public void testVisitors() { StorVisitorConfig.Builder builder = new StorVisitorConfig.Builder(); - parse( - "<cluster id=\"bees\">\n" + - " <documents/>" + - " <tuning>\n" + - " <visitors thread-count=\"7\" max-queue-size=\"1000\">\n" + - " <max-concurrent fixed=\"42\" variable=\"100\"/>\n" + - " </visitors>\n" + - " </tuning>\n" + - " <group>" + - " <node distribution-key=\"0\" hostalias=\"mockhost\"/>" + - " </group>" + - "</cluster>" + parse(cluster("bees", + joinLines( + "<tuning>", + " <visitors thread-count=\"7\" max-queue-size=\"1000\">", + " <max-concurrent fixed=\"42\" variable=\"100\"/>", + " </visitors>", + "</tuning>")) ).getConfig(builder); StorVisitorConfig config = new StorVisitorConfig(builder); @@ -143,16 +166,10 @@ public class StorageClusterTest { @Test public void testPersistenceThreads() { - StorageCluster stc = parse( - "<cluster id=\"bees\">\n" + - " <documents/>" + - " <tuning>\n" + - " <persistence-threads count=\"7\"/>\n" + - " </tuning>\n" + - " <group>" + - " <node distribution-key=\"0\" hostalias=\"mockhost\"/>" + - " </group>" + - "</cluster>", + StorageCluster stc = parse(cluster("bees",joinLines( + "<tuning>", + " <persistence-threads count=\"7\"/>", + "</tuning>")), new Flavor(new FlavorsConfig.Flavor.Builder().name("test-flavor").minCpuCores(9).build()) ); @@ -178,16 +195,10 @@ public class StorageClusterTest { @Test public void testResponseThreads() { - StorageCluster stc = parse( - "<cluster id=\"bees\">\n" + - " <documents/>" + - " <tuning>\n" + - " <persistence-threads count=\"7\"/>\n" + - " </tuning>\n" + - " <group>" + - " <node distribution-key=\"0\" hostalias=\"mockhost\"/>" + - " </group>" + - "</cluster>", + StorageCluster stc = parse(cluster("bees",joinLines( + "<tuning>", + " <persistence-threads count=\"7\"/>", + "</tuning>")), new Flavor(new FlavorsConfig.Flavor.Builder().name("test-flavor").minCpuCores(9).build()) ); StorFilestorConfig.Builder builder = new StorFilestorConfig.Builder(); @@ -201,20 +212,14 @@ public class StorageClusterTest { @Test public void testPersistenceThreadsOld() { - StorageCluster stc = parse( - "<cluster id=\"bees\">\n" + - " <documents/>" + - " <tuning>\n" + - " <persistence-threads>\n" + - " <thread lowest-priority=\"VERY_LOW\" count=\"2\"/>\n" + - " <thread lowest-priority=\"VERY_HIGH\" count=\"1\"/>\n" + - " <thread count=\"1\"/>\n" + - " </persistence-threads>\n" + - " </tuning>\n" + - " <group>" + - " <node distribution-key=\"0\" hostalias=\"mockhost\"/>" + - " </group>" + - "</cluster>", + StorageCluster stc = parse(cluster("bees", joinLines( + "<tuning>", + " <persistence-threads>", + " <thread lowest-priority=\"VERY_LOW\" count=\"2\"/>", + " <thread lowest-priority=\"VERY_HIGH\" count=\"1\"/>", + " <thread count=\"1\"/>", + " </persistence-threads>", + "</tuning>")), new Flavor(new FlavorsConfig.Flavor.Builder().name("test-flavor").minCpuCores(9).build()) ); @@ -238,15 +243,7 @@ public class StorageClusterTest { @Test public void testNoPersistenceThreads() { - StorageCluster stc = parse( - "<cluster id=\"bees\">\n" + - " <documents/>" + - " <tuning>\n" + - " </tuning>\n" + - " <group>" + - " <node distribution-key=\"0\" hostalias=\"mockhost\"/>" + - " </group>" + - "</cluster>", + StorageCluster stc = parse(cluster("bees", ""), new Flavor(new FlavorsConfig.Flavor.Builder().name("test-flavor").minCpuCores(9).build()) ); @@ -267,13 +264,7 @@ public class StorageClusterTest { } private StorageCluster simpleCluster(ModelContext.Properties properties) { - return parse( - "<cluster id=\"bees\">\n" + - " <documents/>" + - " <group>" + - " <node distribution-key=\"0\" hostalias=\"mockhost\"/>" + - " </group>" + - "</cluster>", + return parse(cluster("bees", ""), new Flavor(new FlavorsConfig.Flavor.Builder().name("test-flavor").minCpuCores(9).build()), properties); } @@ -302,14 +293,7 @@ public class StorageClusterTest { @Test public void integrity_checker_explicitly_disabled_when_not_running_with_vds_provider() { StorIntegritycheckerConfig.Builder builder = new StorIntegritycheckerConfig.Builder(); - parse( - "<cluster id=\"bees\">\n" + - " <documents/>" + - " <group>" + - " <node distribution-key=\"0\" hostalias=\"mockhost\"/>" + - " </group>" + - "</cluster>" - ).getConfig(builder); + parse(cluster("bees", "")).getConfig(builder); StorIntegritycheckerConfig config = new StorIntegritycheckerConfig(builder); // '-' --> don't run on the given week day assertEquals("-------", config.weeklycycle()); @@ -317,15 +301,15 @@ public class StorageClusterTest { @Test public void testCapacity() { - String xml = - "<cluster id=\"storage\">\n" + - " <documents/>" + - " <group>\n" + - " <node distribution-key=\"0\" hostalias=\"mockhost\"/>\n" + - " <node distribution-key=\"1\" hostalias=\"mockhost\" capacity=\"1.5\"/>\n" + - " <node distribution-key=\"2\" hostalias=\"mockhost\" capacity=\"2.0\"/>\n" + - " </group>\n" + - "</cluster>"; + String xml = joinLines( + "<cluster id=\"storage\">", + " <documents/>", + " <group>", + " <node distribution-key=\"0\" hostalias=\"mockhost\"/>", + " <node distribution-key=\"1\" hostalias=\"mockhost\" capacity=\"1.5\"/>", + " <node distribution-key=\"2\" hostalias=\"mockhost\" capacity=\"2.0\"/>", + " </group>", + "</cluster>"); ContentCluster cluster = ContentClusterUtils.createCluster(xml, new MockRoot()); @@ -341,15 +325,7 @@ public class StorageClusterTest { @Test public void testRootFolder() { - String xml = - "<cluster id=\"storage\">\n" + - " <documents/>" + - " <group>\n" + - " <node distribution-key=\"0\" hostalias=\"mockhost\"/>\n" + - " </group>\n" + - "</cluster>"; - - ContentCluster cluster = ContentClusterUtils.createCluster(xml, new MockRoot()); + ContentCluster cluster = ContentClusterUtils.createCluster(cluster("storage", ""), new MockRoot()); StorageNode node = cluster.getStorageNodes().getChildren().get("0"); @@ -372,18 +348,18 @@ public class StorageClusterTest { @Test public void testGenericPersistenceTuning() { - String xml = - "<cluster id=\"storage\">\n" + - "<documents/>" + - "<engine>\n" + - " <fail-partition-on-error>true</fail-partition-on-error>\n" + - " <revert-time>34m</revert-time>\n" + - " <recovery-time>5d</recovery-time>\n" + - "</engine>" + - " <group>\n" + - " <node distribution-key=\"0\" hostalias=\"mockhost\"/>\n" + - " </group>\n" + - "</cluster>"; + String xml = joinLines( + "<cluster id=\"storage\">", + " <documents/>", + " <engine>", + " <fail-partition-on-error>true</fail-partition-on-error>", + " <revert-time>34m</revert-time>", + " <recovery-time>5d</recovery-time>", + " </engine>", + " <group>", + " node distribution-key=\"0\" hostalias=\"mockhost\"/>", + " </group>", + "</cluster>"); ContentCluster cluster = ContentClusterUtils.createCluster(xml, new MockRoot()); @@ -398,21 +374,21 @@ public class StorageClusterTest { @Test public void requireThatUserDoesNotSpecifyBothGroupAndNodes() { - String xml = - "<cluster id=\"storage\">\n" + - "<documents/>\n" + - "<engine>\n" + - " <fail-partition-on-error>true</fail-partition-on-error>\n" + - " <revert-time>34m</revert-time>\n" + - " <recovery-time>5d</recovery-time>\n" + - "</engine>" + - " <group>\n" + - " <node distribution-key=\"0\" hostalias=\"mockhost\"/>\n" + - " </group>\n" + - " <nodes>\n" + - " <node distribution-key=\"1\" hostalias=\"mockhost\"/>\n" + - " </nodes>\n" + - "</cluster>"; + String xml = joinLines( + "<cluster id=\"storage\">", + " <documents/>", + " <engine>", + " <fail-partition-on-error>true</fail-partition-on-error>", + " <revert-time>34m</revert-time>", + " <recovery-time>5d</recovery-time>", + " </engine>", + " <group>", + " <node distribution-key=\"0\" hostalias=\"mockhost\"/>", + " </group>", + " <nodes>", + " <node distribution-key=\"1\" hostalias=\"mockhost\"/>", + " </nodes>", + "</cluster>"); try { final MockRoot root = new MockRoot(); @@ -429,20 +405,20 @@ public class StorageClusterTest { @Test public void requireThatGroupNamesMustBeUniqueAmongstSiblings() { - String xml = - "<cluster id=\"storage\">\n" + - " <redundancy>2</redundancy>" + - " <documents/>\n" + - " <group>\n" + - " <distribution partitions=\"*\"/>\n" + - " <group distribution-key=\"0\" name=\"bar\">\n" + - " <node distribution-key=\"0\" hostalias=\"mockhost\"/>\n" + - " </group>\n" + - " <group distribution-key=\"0\" name=\"bar\">\n" + - " <node distribution-key=\"1\" hostalias=\"mockhost\"/>\n" + - " </group>\n" + - " </group>\n" + - "</cluster>"; + String xml = joinLines( + "<cluster id=\"storage\">", + " <redundancy>2</redundancy>", + " <documents/>", + " <group>", + " <distribution partitions=\"*\"/>", + " <group distribution-key=\"0\" name=\"bar\">", + " <node distribution-key=\"0\" hostalias=\"mockhost\"/>", + " </group>", + " <group distribution-key=\"0\" name=\"bar\">", + " <node distribution-key=\"1\" hostalias=\"mockhost\"/>", + " </group>", + " </group>", + "</cluster>"); try { ContentClusterUtils.createCluster(xml, new MockRoot()); @@ -455,24 +431,24 @@ public class StorageClusterTest { @Test public void requireThatGroupNamesCanBeDuplicatedAcrossLevels() { - String xml = - "<cluster id=\"storage\">\n" + - " <redundancy>2</redundancy>" + - "<documents/>\n" + - " <group>\n" + - " <distribution partitions=\"*\"/>\n" + - " <group distribution-key=\"0\" name=\"bar\">\n" + - " <group distribution-key=\"0\" name=\"foo\">\n" + - " <node distribution-key=\"0\" hostalias=\"mockhost\"/>\n" + - " </group>\n" + - " </group>\n" + - " <group distribution-key=\"0\" name=\"foo\">\n" + - " <group distribution-key=\"0\" name=\"bar\">\n" + - " <node distribution-key=\"1\" hostalias=\"mockhost\"/>\n" + - " </group>\n" + - " </group>\n" + - " </group>\n" + - "</cluster>"; + String xml = joinLines( + "<cluster id=\"storage\">", + " <redundancy>2</redundancy>", + " <documents/>", + " <group>", + " <distribution partitions=\"*\"/>", + " <group distribution-key=\"0\" name=\"bar\">", + " <group distribution-key=\"0\" name=\"foo\">", + " <node distribution-key=\"0\" hostalias=\"mockhost\"/>", + " </group>", + " </group>", + " <group distribution-key=\"0\" name=\"foo\">", + " <group distribution-key=\"0\" name=\"bar\">", + " <node distribution-key=\"1\" hostalias=\"mockhost\"/>", + " </group>", + " </group>", + " </group>", + "</cluster>"); // Should not throw. ContentClusterUtils.createCluster(xml, new MockRoot()); @@ -480,18 +456,18 @@ public class StorageClusterTest { @Test public void requireThatNestedGroupsRequireDistribution() { - String xml = - "<cluster id=\"storage\">\n" + - "<documents/>\n" + - " <group>\n" + - " <group distribution-key=\"0\" name=\"bar\">\n" + - " <node distribution-key=\"0\" hostalias=\"mockhost\"/>\n" + - " </group>\n" + - " <group distribution-key=\"0\" name=\"baz\">\n" + - " <node distribution-key=\"1\" hostalias=\"mockhost\"/>\n" + - " </group>\n" + - " </group>\n" + - "</cluster>"; + String xml = joinLines( + "<cluster id=\"storage\">", + " <documents/>", + " <group>", + " <group distribution-key=\"0\" name=\"bar\">", + " <node distribution-key=\"0\" hostalias=\"mockhost\"/>", + " </group>", + " <group distribution-key=\"0\" name=\"baz\">", + " <node distribution-key=\"1\" hostalias=\"mockhost\"/>", + " </group>", + " </group>", + "</cluster>"); try { ContentClusterUtils.createCluster(xml, new MockRoot()); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java index 7c93b4ef02b..afeffbbc875 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java @@ -15,9 +15,7 @@ import java.util.List; import static com.yahoo.config.model.test.TestUtil.joinLines; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; /** * @author Simon Thoresen Hult @@ -179,7 +177,7 @@ public class ClusterTest { " </tuning>", " </content>", "</services>")) - .withSchemas(ApplicationPackageUtils.generateSearchDefinition("my_document")) + .withSchemas(ApplicationPackageUtils.generateSchemas("my_document")) .build(); List<Content> contents = new TestDriver().buildModel(app).getConfigModels(Content.class); assertEquals(1, contents.size()); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTestCase.java index 9eb7ca0ac02..2b36bfc47b2 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTestCase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTestCase.java @@ -192,7 +192,7 @@ public class VespaModelTestCase { " </documents>" + "</content>" + "</services>", - ApplicationPackageUtils.generateSearchDefinition("music")) + ApplicationPackageUtils.generateSchemas("music")) .create(); MessagebusConfig.Builder mBusB = new MessagebusConfig.Builder(); model.getConfig(mBusB, "client"); @@ -316,10 +316,18 @@ public class VespaModelTestCase { .build(); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); Validation.validate(model, new ValidationParameters(), deployState); + assertContainsWarning(logger.msgs, "Directory searchdefinitions/ should not be used for schemas, use schemas/ instead"); + } - assertEquals(3, logger.msgs.size()); - assertEquals("WARNING", logger.msgs.get(1).getFirst().getName()); - assertEquals("Directory searchdefinitions/ should not be used for schemas, use schemas/ instead", logger.msgs.get(1).getSecond()); + private void assertContainsWarning(List<Pair<Level,String>> msgs, String text) { + boolean foundCorrectWarning = false; + for (var msg : msgs) + if (msg.getFirst().getName().equals("WARNING") && msg.getSecond().equals(text)) { + foundCorrectWarning = true; + } + if (! foundCorrectWarning) for (var msg : msgs) System.err.println("MSG: "+msg); + assertTrue(msgs.size() > 0); + assertTrue(foundCorrectWarning); } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java index ba975e52d1a..7e34e9efbbf 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java @@ -21,7 +21,6 @@ import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.ProvisionLogger; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.test.utils.ApplicationPackageUtils; import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg; import java.util.ArrayList; @@ -31,6 +30,8 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import static com.yahoo.vespa.model.test.utils.ApplicationPackageUtils.generateSchemas; + /** * Helper class which sets up a system with multiple hosts. * Usage: @@ -168,7 +169,7 @@ public class VespaModelTester { boolean alwaysReturnOneNode, int startIndexForClusters, Optional<VespaModel> previousModel, DeployState.Builder deployStatebuilder, String ... retiredHostNames) { - VespaModelCreatorWithMockPkg modelCreatorWithMockPkg = new VespaModelCreatorWithMockPkg(null, services, ApplicationPackageUtils.generateSearchDefinition("type1")); + VespaModelCreatorWithMockPkg modelCreatorWithMockPkg = new VespaModelCreatorWithMockPkg(null, services, generateSchemas("type1")); ApplicationPackage appPkg = modelCreatorWithMockPkg.appPkg; provisioner = hosted ? new ProvisionerAdapter(new InMemoryProvisioner(hostsByResources, diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/utils/ApplicationPackageUtils.java b/config-model/src/test/java/com/yahoo/vespa/model/test/utils/ApplicationPackageUtils.java index df62a3bff07..1f7deaf1991 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/test/utils/ApplicationPackageUtils.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/test/utils/ApplicationPackageUtils.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.model.test.utils; import java.util.ArrayList; @@ -43,10 +43,6 @@ public class ApplicationPackageUtils { "}"; } - public static List<String> generateSearchDefinition(String name) { - return generateSchemas(name); - } - public static List<String> generateSchemas(String ... sdNames) { return generateSchemas(Arrays.asList(sdNames)); } diff --git a/configd/src/apps/sentinel/CMakeLists.txt b/configd/src/apps/sentinel/CMakeLists.txt index e77abc19077..43b4f79a0b2 100644 --- a/configd/src/apps/sentinel/CMakeLists.txt +++ b/configd/src/apps/sentinel/CMakeLists.txt @@ -4,6 +4,7 @@ vespa_add_executable(configd_config-sentinel_app check-completion-handler.cpp cmdq.cpp config-owner.cpp + connectivity.cpp env.cpp line-splitter.cpp manager.cpp diff --git a/configd/src/apps/sentinel/config-owner.cpp b/configd/src/apps/sentinel/config-owner.cpp index d5f06dff76b..840c5b1add1 100644 --- a/configd/src/apps/sentinel/config-owner.cpp +++ b/configd/src/apps/sentinel/config-owner.cpp @@ -27,7 +27,7 @@ ConfigOwner::doConfigure() _currGeneration = _subscriber.getGeneration(); const SentinelConfig& config(*_currConfig); const auto & app = config.application; - LOG(config, "Sentinel got %zd service elements [tenant(%s), application(%s), instance(%s)] for config generation %zd", + LOG(config, "Sentinel got %zd service elements [tenant(%s), application(%s), instance(%s)] for config generation %" PRId64, config.service.size(), app.tenant.c_str(), app.name.c_str(), app.instance.c_str(), _currGeneration); } @@ -52,7 +52,7 @@ ConfigOwner::fetchModelConfig(std::chrono::milliseconds timeout) tempSubscriber.subscribe<ModelConfig>("admin/model", timeout); if (tempSubscriber.nextGenerationNow()) { modelConfig = modelHandle->getConfig(); - LOG(config, "Sentinel got model info [version %s] for %zd hosts [config generation %zd", + LOG(config, "Sentinel got model info [version %s] for %zd hosts [config generation %" PRId64 "]", modelConfig->vespaVersion.c_str(), modelConfig->hosts.size(), tempSubscriber.getGeneration()); } diff --git a/configd/src/apps/sentinel/connectivity.cpp b/configd/src/apps/sentinel/connectivity.cpp new file mode 100644 index 00000000000..9cced1d3475 --- /dev/null +++ b/configd/src/apps/sentinel/connectivity.cpp @@ -0,0 +1,112 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "connectivity.h" +#include "outward-check.h" +#include <vespa/defaults.h> +#include <vespa/log/log.h> +#include <vespa/vespalib/util/exceptions.h> +#include <vespa/vespalib/util/stringfmt.h> +#include <thread> +#include <chrono> + +LOG_SETUP(".connectivity"); + +using vespalib::make_string_short::fmt; +using namespace std::chrono_literals; + +namespace config::sentinel { + +Connectivity::Connectivity(const SentinelConfig::Connectivity & config, RpcServer &rpcServer) + : _config(config), + _rpcServer(rpcServer) +{ + LOG(config, "connectivity.maxBadReverseCount = %d", _config.maxBadReverseCount); + LOG(config, "connectivity.maxBadOutPercent = %d", _config.maxBadOutPercent); +} + +Connectivity::~Connectivity() = default; + +namespace { + +const char *toString(CcResult value) { + switch (value) { + case CcResult::UNKNOWN: return "BAD: missing result"; // very very bad + case CcResult::REVERSE_FAIL: return "connect OK, but reverse check FAILED"; // very bad + case CcResult::CONN_FAIL: return "failed to connect"; // bad + case CcResult::REVERSE_UNAVAIL: return "connect OK (but reverse check unavailable)"; // unfortunate + case CcResult::ALL_OK: return "OK: both ways connectivity verified"; // good + } + LOG(error, "Unknown CcResult enum value: %d", (int)value); + LOG_ABORT("Unknown CcResult enum value"); +} + +std::map<std::string, std::string> specsFrom(const ModelConfig &model) { + std::map<std::string, std::string> checkSpecs; + for (const auto & h : model.hosts) { + bool foundSentinelPort = false; + for (const auto & s : h.services) { + if (s.name == "config-sentinel") { + for (const auto & p : s.ports) { + if (p.tags.find("rpc") != p.tags.npos) { + auto spec = fmt("tcp/%s:%d", h.name.c_str(), p.number); + checkSpecs[h.name] = spec; + foundSentinelPort = true; + } + } + } + } + if (! foundSentinelPort) { + LOG(warning, "Did not find 'config-sentinel' RPC port in model for host %s [%zd services]", + h.name.c_str(), h.services.size()); + } + } + return checkSpecs; +} + +} + +Connectivity::CheckResult +Connectivity::checkConnectivity(const ModelConfig &model) { + const auto checkSpecs = specsFrom(model); + size_t clusterSize = checkSpecs.size(); + OutwardCheckContext checkContext(clusterSize, + vespa::Defaults::vespaHostname(), + _rpcServer.getPort(), + _rpcServer.orb()); + std::map<std::string, OutwardCheck> connectivityMap; + for (const auto & [ hn, spec ] : checkSpecs) { + connectivityMap.try_emplace(hn, spec, checkContext); + } + checkContext.latch.await(); + size_t numFailedConns = 0; + size_t numFailedReverse = 0; + bool allChecksOk = true; + for (const auto & [hostname, check] : connectivityMap) { + LOG_ASSERT(check.result() != CcResult::UNKNOWN); + if (check.result() == CcResult::CONN_FAIL) ++numFailedConns; + if (check.result() == CcResult::REVERSE_FAIL) ++numFailedReverse; + } + if (numFailedReverse > size_t(_config.maxBadReverseCount)) { + LOG(warning, "%zu of %zu nodes report problems connecting to me (max is %d)", + numFailedReverse, clusterSize, _config.maxBadReverseCount); + allChecksOk = false; + } + if (numFailedConns * 100.0 > _config.maxBadOutPercent * clusterSize) { + double pct = numFailedConns * 100.0 / clusterSize; + LOG(warning, "Problems connecting to %zu of %zu nodes, %.2f %% (max is %d)", + numFailedConns, clusterSize, pct, _config.maxBadOutPercent); + allChecksOk = false; + } + std::vector<std::string> details; + for (const auto & [hostname, check] : connectivityMap) { + std::string detail = fmt("%s -> %s", hostname.c_str(), toString(check.result())); + details.push_back(detail); + } + CheckResult result{false, false, {}}; + result.enoughOk = allChecksOk; + result.allOk = (numFailedConns == 0) && (numFailedReverse == 0); + result.details = std::move(details); + return result; +} + +} diff --git a/configd/src/apps/sentinel/connectivity.h b/configd/src/apps/sentinel/connectivity.h new file mode 100644 index 00000000000..0e32b5243e0 --- /dev/null +++ b/configd/src/apps/sentinel/connectivity.h @@ -0,0 +1,36 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "rpcserver.h" +#include <vespa/config-sentinel.h> +#include <vespa/config-model.h> +#include <string> +#include <vector> + +using cloud::config::SentinelConfig; +using cloud::config::ModelConfig; + +namespace config::sentinel { + +/** + * Utility class for running connectivity check. + **/ +class Connectivity { +public: + Connectivity(const SentinelConfig::Connectivity & config, RpcServer &rpcServer); + ~Connectivity(); + + struct CheckResult { + bool enoughOk; + bool allOk; + std::vector<std::string> details; + }; + + CheckResult checkConnectivity(const ModelConfig &model); +private: + const SentinelConfig::Connectivity _config; + RpcServer &_rpcServer; +}; + +} diff --git a/configd/src/apps/sentinel/sentinel.cpp b/configd/src/apps/sentinel/sentinel.cpp index 18d4dc28f8a..7f3ddcc5882 100644 --- a/configd/src/apps/sentinel/sentinel.cpp +++ b/configd/src/apps/sentinel/sentinel.cpp @@ -15,8 +15,6 @@ LOG_SETUP("config-sentinel"); using namespace config; -constexpr std::chrono::milliseconds CONFIG_TIMEOUT_MS(3 * 60 * 1000); - static bool stop() { return (vespalib::SignalHandler::INT.check() || diff --git a/configdefinitions/src/vespa/sentinel.def b/configdefinitions/src/vespa/sentinel.def index d318a55cccf..45ef9b21cfd 100644 --- a/configdefinitions/src/vespa/sentinel.def +++ b/configdefinitions/src/vespa/sentinel.def @@ -17,6 +17,17 @@ application.environment string default="default" application.instance string default="default" application.region string default="default" +# Connectivity checks run before starting services and measure how +# many nodes in the Vespa cluster we can connect to and how many of +# those that can connect back to us. We delay starting services +# if we have more problems than the following limits allow: + +## Percentage we fail to talk to, maximum +connectivity.maxBadOutPercent int default=100 + +## Absolute number of nodes that fail to talk back to us, maximum +connectivity.maxBadReverseCount int default=999999999 + ## The command to run. This will be run by sh -c, and the following ## environment variables are defined: $ROOT, $VESPA_SERVICE_NAME, ## $VESPA_CONFIG_ID diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java index d110370e72b..8805c339482 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java @@ -177,6 +177,9 @@ public class ModelContextImpl implements ModelContext { private final boolean useExternalRankExpression; private final boolean distributeExternalRankExpressions; private final int numDistributorStripes; + private final boolean requireConnectivityCheck; + private final int maxConcurrentMergesPerContentNode; + private final int maxMergeQueueSize; public FeatureFlags(FlagSource source, ApplicationId appId) { this.dedicatedClusterControllerFlavor = parseDedicatedClusterControllerFlavor(flagValue(source, appId, Flags.DEDICATED_CLUSTER_CONTROLLER_FLAVOR)); @@ -199,6 +202,9 @@ public class ModelContextImpl implements ModelContext { this.numDistributorStripes = flagValue(source, appId, Flags.NUM_DISTRIBUTOR_STRIPES); this.useExternalRankExpression = flagValue(source, appId, Flags.USE_EXTERNAL_RANK_EXPRESSION); this.distributeExternalRankExpressions = flagValue(source, appId, Flags.DISTRIBUTE_EXTERNAL_RANK_EXPRESSION); + this.requireConnectivityCheck = flagValue(source, appId, Flags.REQUIRE_CONNECTIVITY_CHECK); + this.maxConcurrentMergesPerContentNode = flagValue(source, appId, Flags.MAX_CONCURRENT_MERGES_PER_NODE); + this.maxMergeQueueSize = flagValue(source, appId, Flags.MAX_MERGE_QUEUE_SIZE); } @Override public Optional<NodeResources> dedicatedClusterControllerFlavor() { return Optional.ofNullable(dedicatedClusterControllerFlavor); } @@ -223,6 +229,9 @@ public class ModelContextImpl implements ModelContext { @Override public int numDistributorStripes() { return numDistributorStripes; } @Override public boolean useExternalRankExpressions() { return useExternalRankExpression; } @Override public boolean distributeExternalRankExpressions() { return distributeExternalRankExpressions; } + @Override public boolean requireConnectivityCheck() { return requireConnectivityCheck; } + @Override public int maxConcurrentMergesPerNode() { return maxConcurrentMergesPerContentNode; } + @Override public int maxMergeQueueSize() { return maxMergeQueueSize; } private static <V> V flagValue(FlagSource source, ApplicationId appId, UnboundFlag<? extends V, ?, ?> flag) { return flag.bindTo(source) diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClient.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClient.java index ca7489012e2..f0a63757477 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClient.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClient.java @@ -35,7 +35,7 @@ import static com.yahoo.vespa.config.server.zookeeper.ConfigCurator.USERAPP_ZK_S import static com.yahoo.vespa.config.server.zookeeper.ConfigCurator.USER_DEFCONFIGS_ZK_SUBPATH; /** - * A class used for reading and writing application data to zookeeper. + * Reads and writes application package to and from ZooKeeper. * * @author hmusum */ @@ -76,13 +76,13 @@ public class ZooKeeperClient { * * @param app the application package to feed to zookeeper */ - void write(ApplicationPackage app) { + void writeApplicationPackage(ApplicationPackage app) { try { writeUserDefs(app); writeSomeOf(app); - writeSearchDefinitions(app); + writeSchemas(app); writeUserIncludeDirs(app, app.getUserIncludeDirs()); - write(app.getMetaData()); + writeMetadata(app.getMetaData()); } catch (Exception e) { throw new IllegalStateException("Unable to write vespa model to config server(s) " + System.getProperty("configsources") + "\n" + "Please ensure that config server is started " + @@ -90,8 +90,8 @@ public class ZooKeeperClient { } } - private void writeSearchDefinitions(ApplicationPackage app) throws IOException { - Collection<NamedReader> sds = app.getSearchDefinitions(); + private void writeSchemas(ApplicationPackage app) throws IOException { + Collection<NamedReader> sds = app.getSchemas(); if (sds.isEmpty()) return; Path zkPath = getZooKeeperAppPath(USERAPP_ZK_SUBPATH).append(SCHEMAS_DIR); @@ -153,7 +153,6 @@ public class ZooKeeperClient { for (ApplicationFile file : listFiles(dir, filenameFilter)) { String name = file.getPath().getName(); if (name.startsWith(".")) continue; //.svn , .git ... - if ("CVS".equals(name)) continue; if (file.isDirectory()) { configCurator.createNode(path.append(name).getAbsolute()); if (recurse) { @@ -198,7 +197,6 @@ public class ZooKeeperClient { } private void writeUserIncludeDirs(ApplicationPackage applicationPackage, List<String> userIncludeDirs) throws IOException { - // User defined include directories for (String userInclude : userIncludeDirs) { ApplicationFile dir = applicationPackage.getFile(Path.fromString(userInclude)); final List<ApplicationFile> files = dir.listFiles(); @@ -238,12 +236,12 @@ public class ZooKeeperClient { } /** - * Feeds application metadata to zookeeper. Used by vespamodel to create config - * for application metadata (used by ApplicationStatusHandler) + * Feeds application metadata to zookeeper. Used by config model to create config + * for application metadata * * @param metaData The application metadata. */ - private void write(ApplicationMetaData metaData) { + private void writeMetadata(ApplicationMetaData metaData) { configCurator.putData(getZooKeeperAppPath(META_ZK_PATH).getAbsolute(), metaData.asJsonBytes()); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ZooKeeperDeployer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ZooKeeperDeployer.java index 12aa5b7cc35..8c7d6ea28dd 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ZooKeeperDeployer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ZooKeeperDeployer.java @@ -33,7 +33,7 @@ public class ZooKeeperDeployer { public void deploy(ApplicationPackage applicationPackage, Map<Version, FileRegistry> fileRegistryMap, AllocatedHosts allocatedHosts) throws IOException { zooKeeperClient.initialize(); - zooKeeperClient.write(applicationPackage); + zooKeeperClient.writeApplicationPackage(applicationPackage); zooKeeperClient.write(fileRegistryMap); zooKeeperClient.write(allocatedHosts); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationApiHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationApiHandler.java index 5b520b10fcf..dfbce72d4ba 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationApiHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationApiHandler.java @@ -82,7 +82,7 @@ public class ApplicationApiHandler extends SessionHandler { .collect(Collectors.toMap(Part::getName, p -> p)); byte[] params = parts.get(MULTIPART_PARAMS).getInputStream().readAllBytes(); - log.log(Level.FINE, "Deploy parameters: [{}]", new String(params, StandardCharsets.UTF_8)); + log.log(Level.FINE, "Deploy parameters: [{0}]", new String(params, StandardCharsets.UTF_8)); prepareParams = PrepareParams.fromJson(params, tenantName, zookeeperBarrierTimeout); Part appPackagePart = parts.get(MULTIPART_APPLICATION_PACKAGE); compressedStream = createFromCompressedStream(appPackagePart.getInputStream(), appPackagePart.getContentType()); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java index 5519ffc1bdc..003b4fbb345 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java @@ -48,8 +48,9 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer { } @Override - protected boolean maintain() { - boolean success = true; + protected double maintain() { + int attempts = 0; + int failures = 0; try (var fileDownloader = new FileDownloader(connectionPool, downloadDirectory)) { for (var applicationId : applicationRepository.listApplications()) { @@ -62,11 +63,12 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer { log.fine(() -> "Verifying application package file reference " + applicationPackage + " for session " + sessionId); if (applicationPackage != null) { + attempts++; if (! fileReferenceExistsOnDisk(downloadDirectory, applicationPackage)) { log.fine(() -> "Downloading missing application package for application " + applicationId + " - session " + sessionId); if (fileDownloader.getFile(applicationPackage).isEmpty()) { - success = false; + failures++; log.warning("Failed to download application package for application " + applicationId + " - session " + sessionId); continue; } @@ -75,7 +77,7 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer { } } } - return success; + return asSuccessFactor(attempts, failures); } @Override diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintainer.java index 4938f34131e..e0f0a4b4099 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintainer.java @@ -35,14 +35,24 @@ public abstract class ConfigServerMaintainer extends Maintainer { ConfigServerMaintainer(ApplicationRepository applicationRepository, Curator curator, FlagSource flagSource, Instant now, Duration interval) { super(null, interval, now, new JobControl(new JobControlFlags(curator, flagSource)), - jobMetrics(applicationRepository.metric()), cluster(curator), false); + new ConfigServerJobMetrics(applicationRepository.metric()), cluster(curator), false); this.applicationRepository = applicationRepository; } - private static JobMetrics jobMetrics(Metric metric) { - return new JobMetrics((job, consecutiveFailures) -> { + private static class ConfigServerJobMetrics extends JobMetrics { + + private final Metric metric; + + public ConfigServerJobMetrics(Metric metric) { + this.metric = metric; + } + + @Override + protected void recordCompletion(String job, Long consecutiveFailures, double successFactor) { metric.set("maintenance.consecutiveFailures", consecutiveFailures, metric.createContext(Map.of("job", job))); - }); + metric.set("maintenance.successFactor", successFactor, metric.createContext(Map.of("job", job))); + } + } private static class JobControlFlags implements JobControlState { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java index b0876fb57e8..ca8db30c21f 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java @@ -33,9 +33,9 @@ public class FileDistributionMaintainer extends ConfigServerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { applicationRepository.deleteUnusedFiledistributionReferences(fileReferencesDir, maxUnusedFileReferenceAge); - return true; + return 1.0; } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ReindexingMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ReindexingMaintainer.java index 971c2c20ae9..af9ea917aaf 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ReindexingMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ReindexingMaintainer.java @@ -22,6 +22,7 @@ import java.util.Comparator; import java.util.Map; import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.function.Supplier; import java.util.logging.Level; @@ -51,8 +52,9 @@ public class ReindexingMaintainer extends ConfigServerMaintainer { } @Override - protected boolean maintain() { - AtomicBoolean success = new AtomicBoolean(true); + protected double maintain() { + AtomicInteger attempts = new AtomicInteger(0); + AtomicInteger failures = new AtomicInteger(0); for (Tenant tenant : applicationRepository.tenantRepository().getAllTenants()) { ApplicationCuratorDatabase database = tenant.getApplicationRepo().database(); for (ApplicationId id : database.activeApplications()) @@ -60,6 +62,7 @@ public class ReindexingMaintainer extends ConfigServerMaintainer { .map(application -> application.getForVersionOrLatest(Optional.empty(), clock.instant())) .ifPresent(application -> { try { + attempts.incrementAndGet(); applicationRepository.modifyReindexing(id, reindexing -> { reindexing = withNewReady(reindexing, lazyGeneration(application), clock.instant()); reindexing = withOnlyCurrentData(reindexing, application); @@ -68,11 +71,11 @@ public class ReindexingMaintainer extends ConfigServerMaintainer { } catch (RuntimeException e) { log.log(Level.INFO, "Failed to update reindexing status for " + id + ": " + Exceptions.toMessageString(e)); - success.set(false); + failures.incrementAndGet(); } }); } - return success.get(); + return asSuccessFactor(attempts.get(), failures.get()); } private Supplier<Long> lazyGeneration(Application application) { 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 7482980e221..1f85dd4579d 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 @@ -25,7 +25,7 @@ public class SessionsMaintainer extends ConfigServerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { if (iteration % 10 == 0) log.log(Level.INFO, () -> "Running " + SessionsMaintainer.class.getSimpleName() + ", iteration " + iteration); @@ -38,7 +38,7 @@ public class SessionsMaintainer extends ConfigServerMaintainer { } iteration++; - return true; + return 1.0; } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java index 7c01045ee72..0a7df2c9d21 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java @@ -31,12 +31,12 @@ public class TenantsMaintainer extends ConfigServerMaintainer { } @Override - protected boolean maintain() { - if ( ! applicationRepository.configserverConfig().hostedVespa()) return true; + protected double maintain() { + if ( ! applicationRepository.configserverConfig().hostedVespa()) return 1.0; Set<TenantName> tenants = applicationRepository.deleteUnusedTenants(ttlForUnusedTenant, clock.instant()); if (tenants.size() > 0) log.log(Level.INFO, "Deleted tenants " + tenants); - return true; + return 1.0; } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java index 1b43e57c01a..071a0dd8f0c 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java @@ -306,7 +306,7 @@ public final class PrepareParams { .athenzDomain(SlimeUtils.optionalString(params.field(ATHENZ_DOMAIN)).orElse(null)) .applicationRoles(ApplicationRoles.fromString(SlimeUtils.optionalString(params.field(APPLICATION_HOST_ROLE)).orElse(null), SlimeUtils.optionalString(params.field(APPLICATION_CONTAINER_ROLE)).orElse(null))) .quota(deserialize(params.field(QUOTA_PARAM_NAME), Quota::fromSlime)) - .tenantSecretStores(SlimeUtils.optionalString(params.field(TENANT_SECRET_STORES_PARAM_NAME)).orElse(null)) + .tenantSecretStores(deserialize(params.field(TENANT_SECRET_STORES_PARAM_NAME), TenantSecretStoreSerializer::listFromSlime, List.of())) .force(booleanValue(params, FORCE_PARAM_NAME)) .waitForResourcesInPrepare(booleanValue(params, WAIT_FOR_RESOURCES_IN_PREPARE)) .withOperatorCertificates(deserialize(params.field(OPERATOR_CERTIFICATES), PrepareParams::readOperatorCertificates, Collections.emptyList())) diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java index 48c47587c8b..9f1bfa0b4e4 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java @@ -132,13 +132,13 @@ public class ZKApplicationPackage implements ApplicationPackage { } @Override - public List<NamedReader> searchDefinitionContents() { + public List<NamedReader> getSchemas() { List<NamedReader> schemas = new ArrayList<>(); for (String sd : zkApplication.getChildren(ConfigCurator.USERAPP_ZK_SUBPATH + "/" + SCHEMAS_DIR)) { if (sd.endsWith(SD_NAME_SUFFIX)) schemas.add(new NamedReader(sd, new StringReader(zkApplication.getData(ConfigCurator.USERAPP_ZK_SUBPATH + "/" + SCHEMAS_DIR, sd)))); } - // TODO: Remove when everything is written to SCHEMAS_DIR (July 2021) + // TODO: Remove when 7.414.19 is oldest version in use for (String sd : zkApplication.getChildren(ConfigCurator.USERAPP_ZK_SUBPATH + "/" + SEARCH_DEFINITIONS_DIR)) { if (sd.endsWith(SD_NAME_SUFFIX)) schemas.add(new NamedReader(sd, new StringReader(zkApplication.getData(ConfigCurator.USERAPP_ZK_SUBPATH + "/" + SEARCH_DEFINITIONS_DIR, sd)))); @@ -165,11 +165,6 @@ public class ZKApplicationPackage implements ApplicationPackage { return fileRegistry; } - @Override - public List<NamedReader> getSearchDefinitions() { - return searchDefinitionContents(); - } - private Reader retrieveConfigDefReader(String def) { try { return zkApplication.getDataReader(ConfigCurator.DEFCONFIGS_ZK_SUBPATH, def); @@ -263,6 +258,7 @@ public class ZKApplicationPackage implements ApplicationPackage { @Override public Reader getRankingExpression(String name) { Optional<Reader> reader = zkApplication.getOptionalDataReader(ConfigCurator.USERAPP_ZK_SUBPATH + "/" + SCHEMAS_DIR, name); + // TODO: Remove when 7.414.19 is oldest version in use return reader.orElseGet(() -> zkApplication.getDataReader(ConfigCurator.USERAPP_ZK_SUBPATH + "/" + SEARCH_DEFINITIONS_DIR, name)); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClientTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClientTest.java index 7d14b1996b0..e20363af4e9 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClientTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClientTest.java @@ -64,7 +64,7 @@ public class ZooKeeperClientTest { Map<Version, FileRegistry> fileRegistries = createFileRegistries(); app.writeMetaData(); zkc.initialize(); - zkc.write(app); + zkc.writeApplicationPackage(app); zkc.write(fileRegistries); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/PrepareParamsTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/PrepareParamsTest.java index 08794cf0b78..f68e79ae266 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/PrepareParamsTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/PrepareParamsTest.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.config.server.session; import com.yahoo.config.model.api.ApplicationRoles; import com.yahoo.config.model.api.ContainerEndpoint; import com.yahoo.config.model.api.EndpointCertificateMetadata; +import com.yahoo.config.model.api.TenantSecretStore; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.TenantName; import com.yahoo.container.jdisc.HttpRequest; @@ -24,6 +25,7 @@ import com.yahoo.slime.SlimeInserter; import com.yahoo.slime.SlimeUtils; import com.yahoo.vespa.config.server.tenant.ContainerEndpointSerializer; import com.yahoo.vespa.config.server.tenant.EndpointCertificateMetadataSerializer; +import com.yahoo.vespa.config.server.tenant.TenantSecretStoreSerializer; import org.junit.Test; import javax.security.auth.x500.X500Principal; @@ -203,6 +205,26 @@ public class PrepareParamsTest { assertEquals(certificate, prepareParams.operatorCertificates().get(0)); } + @Test + public void testSecretStores() throws IOException { + List<TenantSecretStore> secretStores = List.of(new TenantSecretStore("name", "awsId", "role")); + Slime secretStoreSlime = TenantSecretStoreSerializer.toSlime(secretStores); + String secretStoreParam = new String(SlimeUtils.toJsonBytes(secretStoreSlime), StandardCharsets.UTF_8); + + var prepareParams = createParams(request + "&" + PrepareParams.TENANT_SECRET_STORES_PARAM_NAME + "=" + URLEncoder.encode(secretStoreParam, StandardCharsets.UTF_8), TenantName.from("foo")); + assertEquals(1, prepareParams.tenantSecretStores().size()); + TenantSecretStore tenantSecretStore = prepareParams.tenantSecretStores().get(0); + assertEquals("name", tenantSecretStore.getName()); + assertEquals("awsId", tenantSecretStore.getAwsId()); + assertEquals("role", tenantSecretStore.getRole()); + + // Verify using json object + var root = SlimeUtils.jsonToSlime(json); + new Injector().inject(secretStoreSlime.get(), new ObjectInserter(root.get(), PrepareParams.TENANT_SECRET_STORES_PARAM_NAME)); + PrepareParams prepareParamsJson = PrepareParams.fromJson(SlimeUtils.toJsonBytes(root), TenantName.from("foo"), Duration.ofSeconds(60)); + assertPrepareParamsEqual(prepareParams, prepareParamsJson); + } + private void assertPrepareParamsEqual(PrepareParams urlParams, PrepareParams jsonParams) { assertEquals(urlParams.ignoreValidationErrors(), jsonParams.ignoreValidationErrors()); assertEquals(urlParams.isDryRun(), jsonParams.isDryRun()); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java index 80d01fa4d36..458cdb82066 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java @@ -78,7 +78,7 @@ public class ZKApplicationPackageTest { assertTrue(Pattern.compile(".*<slobroks>.*",Pattern.MULTILINE+Pattern.DOTALL).matcher(IOUtils.readAll(zkApp.getFile(Path.fromString("services.xml")).createReader())).matches()); DeployState deployState = new DeployState.Builder().applicationPackage(zkApp).build(); assertEquals(deployState.getSchemas().size(), 5); - assertEquals(zkApp.searchDefinitionContents().size(), 5); + assertEquals(zkApp.getSchemas().size(), 5); assertEquals(IOUtils.readAll(zkApp.getRankingExpression("foo.expression")), "foo()+1\n"); assertEquals(zkApp.getFiles(Path.fromString(""), "xml").size(), 3); assertEquals(zkApp.getFileReference(Path.fromString("components/file.txt")).getAbsolutePath(), "/home/vespa/test/file.txt"); @@ -124,8 +124,7 @@ public class ZKApplicationPackageTest { } /** - * Takes for instance the dir /app and puts the contents into the given ZK path. Ignores files starting with dot, - * and dirs called CVS. + * Takes for instance the dir /app and puts the contents into the given ZK path. Ignores files starting with dot. * * @param dir directory which holds the summary class part files * @param path zookeeper path @@ -142,7 +141,6 @@ public class ZKApplicationPackageTest { } for (File file : listFiles(dir, filenameFilter)) { if (file.getName().startsWith(".")) continue; //.svn , .git ... - if ("CVS".equals(file.getName())) continue; if (file.isFile()) { String contents = IOUtils.readFile(file); zk.putData(path, file.getName(), contents); diff --git a/container-core/abi-spec.json b/container-core/abi-spec.json index efe6701342f..02d43104a3f 100644 --- a/container-core/abi-spec.json +++ b/container-core/abi-spec.json @@ -1039,6 +1039,7 @@ "public com.yahoo.jdisc.http.ConnectorConfig$Builder maxRequestsPerConnection(int)", "public com.yahoo.jdisc.http.ConnectorConfig$Builder maxConnectionLife(double)", "public com.yahoo.jdisc.http.ConnectorConfig$Builder http2Enabled(boolean)", + "public com.yahoo.jdisc.http.ConnectorConfig$Builder http2(com.yahoo.jdisc.http.ConnectorConfig$Http2$Builder)", "public final boolean dispatchGetConfig(com.yahoo.config.ConfigInstance$Producer)", "public final java.lang.String getDefMd5()", "public final java.lang.String getDefName()", @@ -1053,7 +1054,8 @@ "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder tlsClientAuthEnforcer", "public com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy$Builder healthCheckProxy", "public com.yahoo.jdisc.http.ConnectorConfig$ProxyProtocol$Builder proxyProtocol", - "public com.yahoo.jdisc.http.ConnectorConfig$SecureRedirect$Builder secureRedirect" + "public com.yahoo.jdisc.http.ConnectorConfig$SecureRedirect$Builder secureRedirect", + "public com.yahoo.jdisc.http.ConnectorConfig$Http2$Builder http2" ] }, "com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy$Builder": { @@ -1089,6 +1091,37 @@ ], "fields": [] }, + "com.yahoo.jdisc.http.ConnectorConfig$Http2$Builder": { + "superClass": "java.lang.Object", + "interfaces": [ + "com.yahoo.config.ConfigBuilder" + ], + "attributes": [ + "public" + ], + "methods": [ + "public void <init>()", + "public void <init>(com.yahoo.jdisc.http.ConnectorConfig$Http2)", + "public com.yahoo.jdisc.http.ConnectorConfig$Http2$Builder streamIdleTimeout(double)", + "public com.yahoo.jdisc.http.ConnectorConfig$Http2$Builder maxConcurrentStreams(int)", + "public com.yahoo.jdisc.http.ConnectorConfig$Http2 build()" + ], + "fields": [] + }, + "com.yahoo.jdisc.http.ConnectorConfig$Http2": { + "superClass": "com.yahoo.config.InnerNode", + "interfaces": [], + "attributes": [ + "public", + "final" + ], + "methods": [ + "public void <init>(com.yahoo.jdisc.http.ConnectorConfig$Http2$Builder)", + "public double streamIdleTimeout()", + "public int maxConcurrentStreams()" + ], + "fields": [] + }, "com.yahoo.jdisc.http.ConnectorConfig$Producer": { "superClass": "java.lang.Object", "interfaces": [ @@ -1361,7 +1394,8 @@ "public com.yahoo.jdisc.http.ConnectorConfig$SecureRedirect secureRedirect()", "public int maxRequestsPerConnection()", "public double maxConnectionLife()", - "public boolean http2Enabled()" + "public boolean http2Enabled()", + "public com.yahoo.jdisc.http.ConnectorConfig$Http2 http2()" ], "fields": [ "public static final java.lang.String CONFIG_DEF_MD5", diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java index fc39de72018..4ad39f91a83 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java @@ -75,7 +75,7 @@ public class ConnectorFactory { connector.setName(connectorConfig.name()); connector.setAcceptQueueSize(connectorConfig.acceptQueueSize()); connector.setReuseAddress(connectorConfig.reuseAddress()); - connector.setIdleTimeout(idleTimeoutInMillis()); + connector.setIdleTimeout(toMillis(connector.getIdleTimeout())); return connector; } @@ -162,8 +162,8 @@ public class ConnectorFactory { private HTTP2ServerConnectionFactory newHttp2ConnectionFactory() { HTTP2ServerConnectionFactory factory = new HTTP2ServerConnectionFactory(newHttpConfiguration()); - factory.setStreamIdleTimeout(idleTimeoutInMillis()); - factory.setMaxConcurrentStreams(4096); + factory.setStreamIdleTimeout(toMillis(connectorConfig.http2().streamIdleTimeout())); + factory.setMaxConcurrentStreams(connectorConfig.http2().maxConcurrentStreams()); return factory; } @@ -194,6 +194,6 @@ public class ConnectorFactory { || (config.implicitTlsEnabled() && TransportSecurityUtils.isTransportSecurityEnabled()); } - private long idleTimeoutInMillis() { return (long) (connectorConfig.idleTimeout() * 1000.0); } + private static long toMillis(double seconds) { return (long)(seconds * 1000); } } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java index 7828751df5a..ba292062197 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java @@ -26,6 +26,7 @@ import java.time.Instant; import java.util.Arrays; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -119,6 +120,10 @@ class HttpRequestDispatch { error, () -> "Network connection was unexpectedly terminated: " + parent.jettyRequest.getRequestURI()); parent.metricReporter.prematurelyClosed(); + } else if (isErrorOfType(error, TimeoutException.class)) { + log.log(Level.FINE, + error, + () -> "Request/stream was timed out by Jetty: " + parent.jettyRequest.getRequestURI()); } else if (!isErrorOfType(error, OverloadException.class, BindingNotFoundException.class, RequestException.class)) { log.log(Level.WARNING, "Request failed: " + parent.jettyRequest.getRequestURI(), error); } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java index 31fa9e9ebaa..d61a3745653 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java @@ -20,6 +20,7 @@ import java.nio.ByteBuffer; import java.util.Map; import java.util.Optional; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeoutException; import java.util.logging.Level; import java.util.logging.Logger; @@ -75,6 +76,9 @@ public class ServletResponseController { return HttpServletResponse.SC_NOT_FOUND; } else if (t instanceof RequestException) { return ((RequestException)t).getResponseStatus(); + } else if (t instanceof TimeoutException) { + // E.g stream idle timeout for HTTP/2 + return HttpServletResponse.SC_SERVICE_UNAVAILABLE; } else { return HttpServletResponse.SC_INTERNAL_SERVER_ERROR; } diff --git a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def index 09b883a620e..0b01f690aea 100644 --- a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def +++ b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def @@ -128,3 +128,7 @@ maxConnectionLife double default=0.0 # Enable HTTP/2 (in addition to HTTP/1.1 using ALPN) http2Enabled bool default=true + +http2.streamIdleTimeout double default=600 + +http2.maxConcurrentStreams int default=4096 diff --git a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java index ba9f1804d34..e2c15b6e35b 100644 --- a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java +++ b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java @@ -532,12 +532,13 @@ public class SearchHandler extends LoggingRequestHandler { if (query.getHits() > maxHits) { return new Result(query, ErrorMessage.createIllegalQuery(query.getHits() + - " hits requested, configured limit: " + maxHits + ".")); + " hits requested, configured limit: " + maxHits + + ". See https://docs.vespa.ai/en/reference/query-api-reference.html#native-execution-parameters")); } else if (query.getOffset() > maxOffset) { - return new Result(query, - ErrorMessage.createIllegalQuery("Offset of " + query.getOffset() + - " requested, configured limit: " + maxOffset + ".")); + return new Result(query, ErrorMessage.createIllegalQuery("Offset of " + query.getOffset() + + " requested, configured limit: " + maxOffset + + ". See https://docs.vespa.ai/en/reference/query-api-reference.html#native-execution-parameters")); } return null; } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Cluster.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Cluster.java index b3317c7f268..ccfd3241810 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Cluster.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Cluster.java @@ -23,6 +23,7 @@ public class Cluster { private final Optional<ClusterResources> suggested; private final Utilization utilization; private final List<ScalingEvent> scalingEvents; + private final String autoscalingStatusCode; private final String autoscalingStatus; private final Duration scalingDuration; private final double maxQueryGrowthRate; @@ -37,6 +38,7 @@ public class Cluster { Optional<ClusterResources> suggested, Utilization utilization, List<ScalingEvent> scalingEvents, + String autoscalingStatusCode, String autoscalingStatus, Duration scalingDuration, double maxQueryGrowthRate, @@ -50,6 +52,7 @@ public class Cluster { this.suggested = suggested; this.utilization = utilization; this.scalingEvents = scalingEvents; + this.autoscalingStatusCode = autoscalingStatusCode; this.autoscalingStatus = autoscalingStatus; this.scalingDuration = scalingDuration; this.maxQueryGrowthRate = maxQueryGrowthRate; @@ -65,6 +68,7 @@ public class Cluster { public Optional<ClusterResources> suggested() { return suggested; } public Utilization utilization() { return utilization; } public List<ScalingEvent> scalingEvents() { return scalingEvents; } + public String autoscalingStatusCode() { return autoscalingStatusCode; } public String autoscalingStatus() { return autoscalingStatus; } public Duration scalingDuration() { return scalingDuration; } public double maxQueryGrowthRate() { return maxQueryGrowthRate; } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/ClusterData.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/ClusterData.java index b6163809f26..6f9b2b496bf 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/ClusterData.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/ClusterData.java @@ -35,6 +35,8 @@ public class ClusterData { public ClusterUtilizationData utilization; @JsonProperty("scalingEvents") public List<ScalingEventData> scalingEvents; + @JsonProperty("autoscalingStatusCode") + public String autoscalingStatusCode; @JsonProperty("autoscalingStatus") public String autoscalingStatus; @JsonProperty("scalingDuration") @@ -55,6 +57,7 @@ public class ClusterData { utilization == null ? Cluster.Utilization.empty() : utilization.toClusterUtilization(), scalingEvents == null ? List.of() : scalingEvents.stream().map(data -> data.toScalingEvent()).collect(Collectors.toList()), + autoscalingStatusCode, autoscalingStatus, scalingDuration == null ? Duration.ofMillis(0) : Duration.ofMillis(scalingDuration), maxQueryGrowthRate == null ? -1 : maxQueryGrowthRate, diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationMetaDataGarbageCollector.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationMetaDataGarbageCollector.java index 7d94a4c728f..9ec8e4d1a2d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationMetaDataGarbageCollector.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationMetaDataGarbageCollector.java @@ -19,14 +19,14 @@ public class ApplicationMetaDataGarbageCollector extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { try { controller().applications().applicationStore().pruneMeta(controller().clock().instant().minus(Duration.ofDays(365))); - return true; + return 1.0; } catch (Exception e) { log.log(Level.WARNING, "Exception pruning old application meta data", e); - return false; + return 0.0; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java index 1f20e48edf5..69e0eb26f16 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java @@ -18,6 +18,7 @@ import com.yahoo.yolean.Exceptions; import java.time.Duration; import java.util.HashMap; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Level; /** @@ -39,15 +40,17 @@ public class ApplicationOwnershipConfirmer extends ControllerMaintainer { } @Override - protected boolean maintain() { - return confirmApplicationOwnerships() & - ensureConfirmationResponses() & - updateConfirmedApplicationOwners(); + protected double maintain() { + return ( confirmApplicationOwnerships() + + ensureConfirmationResponses() + + updateConfirmedApplicationOwners() ) + / 3; } /** File an ownership issue with the owners of all applications we know about. */ - private boolean confirmApplicationOwnerships() { - AtomicBoolean success = new AtomicBoolean(true); + private double confirmApplicationOwnerships() { + AtomicInteger attempts = new AtomicInteger(0); + AtomicInteger failures = new AtomicInteger(0); applications() .withProjectId() .withProductionDeployment() @@ -56,6 +59,7 @@ public class ApplicationOwnershipConfirmer extends ControllerMaintainer { .filter(application -> application.createdAt().isBefore(controller().clock().instant().minus(Duration.ofDays(90)))) .forEach(application -> { try { + attempts.incrementAndGet(); // TODO jvenstad: Makes sense to require, and run this only in main? tenantOf(application.id()).contact().flatMap(contact -> { return ownershipIssues.confirmOwnership(application.ownershipIssueId(), @@ -65,17 +69,17 @@ public class ApplicationOwnershipConfirmer extends ControllerMaintainer { }).ifPresent(newIssueId -> store(newIssueId, application.id())); } catch (RuntimeException e) { // Catch errors due to wrong data in the controller, or issues client timeout. - success.set(false); + failures.incrementAndGet(); log.log(Level.INFO, "Exception caught when attempting to file an issue for '" + application.id() + "': " + Exceptions.toMessageString(e)); } }); - return success.get(); + return asSuccessFactor(attempts.get(), failures.get()); } private ApplicationSummary summaryOf(TenantAndApplicationId application) { var app = applications.requireApplication(application); var metrics = new HashMap<ZoneId, ApplicationSummary.Metric>(); - for (Instance instance : app.instances().values()) + for (Instance instance : app.instances().values()) { for (var kv : instance.deployments().entrySet()) { var zone = kv.getKey(); var deploymentMetrics = kv.getValue().metrics(); @@ -83,28 +87,31 @@ public class ApplicationOwnershipConfirmer extends ControllerMaintainer { deploymentMetrics.queriesPerSecond(), deploymentMetrics.writesPerSecond())); } + } return new ApplicationSummary(app.id().defaultInstance(), app.activity().lastQueried(), app.activity().lastWritten(), app.latestVersion().flatMap(version -> version.buildTime()), metrics); } /** Escalate ownership issues which have not been closed before a defined amount of time has passed. */ - private boolean ensureConfirmationResponses() { - AtomicBoolean success = new AtomicBoolean(true); + private double ensureConfirmationResponses() { + AtomicInteger attempts = new AtomicInteger(0); + AtomicInteger failures = new AtomicInteger(0); for (Application application : applications()) application.ownershipIssueId().ifPresent(issueId -> { try { + attempts.incrementAndGet(); Tenant tenant = tenantOf(application.id()); ownershipIssues.ensureResponse(issueId, tenant.contact()); } catch (RuntimeException e) { - success.set(false); + failures.incrementAndGet(); log.log(Level.INFO, "Exception caught when attempting to escalate issue with id '" + issueId + "': " + Exceptions.toMessageString(e)); } }); - return success.get(); + return asSuccessFactor(attempts.get(), failures.get()); } - private boolean updateConfirmedApplicationOwners() { + private double updateConfirmedApplicationOwners() { applications() .withProjectId() .withProductionDeployment() @@ -118,7 +125,7 @@ public class ApplicationOwnershipConfirmer extends ControllerMaintainer { controller().applications().store(lockedApplication.withOwner(owner))); }); }); - return true; + return 1.0; } private ApplicationList applications() { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ArchiveAccessMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ArchiveAccessMaintainer.java index 1a9889284e1..b096a853541 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ArchiveAccessMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ArchiveAccessMaintainer.java @@ -37,8 +37,7 @@ public class ArchiveAccessMaintainer extends ControllerMaintainer { } @Override - protected boolean maintain() { - + protected double maintain() { // Count buckets - so we can alert if we get close to the account limit of 1000 zoneRegistry.zones().all().ids().forEach(zoneId -> metric.set(bucketCountMetricName, archiveBucketDb.buckets(zoneId).size(), @@ -59,6 +58,7 @@ public class ArchiveAccessMaintainer extends ControllerMaintainer { ) ); - return true; + return 1.0; } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ArchiveUriUpdater.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ArchiveUriUpdater.java index d2141b097b3..ab8e5efa0bd 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ArchiveUriUpdater.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ArchiveUriUpdater.java @@ -38,7 +38,7 @@ public class ArchiveUriUpdater extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { Map<ZoneId, Set<TenantName>> tenantsByZone = new HashMap<>(); for (var application : applications.asList()) { for (var instance : application.instances().values()) { @@ -63,7 +63,7 @@ public class ArchiveUriUpdater extends ControllerMaintainer { .forEach(tenant -> nodeRepository.removeArchiveUri(zone, tenant)); }); - return true; + return 1.0; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainer.java index 1f360c477b9..14e3e685a8a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainer.java @@ -43,14 +43,14 @@ public class ChangeRequestMaintainer extends ControllerMaintainer { @Override - protected boolean maintain() { + protected double maintain() { var currentChangeRequests = pruneOldChangeRequests(); var changeRequests = changeRequestClient.getChangeRequests(currentChangeRequests); logger.fine(() -> "Found requests: " + changeRequests); storeChangeRequests(changeRequests); - return true; + return 1.0; } private void storeChangeRequests(List<ChangeRequest> changeRequests) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporter.java index d923db936cb..5acd0c63670 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporter.java @@ -38,7 +38,7 @@ public class CloudEventReporter extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { for (var region : zonesByCloudNativeRegion.keySet()) { List<CloudEvent> events = eventFetcher.getEvents(region); for (var event : events) { @@ -48,7 +48,7 @@ public class CloudEventReporter extends ControllerMaintainer { deprovisionAffectedHosts(region, event); } } - return true; + return 1.0; } /** Deprovision any host affected by given event */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java index 7b846fa288c..5ee39f7c8f2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java @@ -35,12 +35,14 @@ public class ContactInformationMaintainer extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { TenantController tenants = controller().tenants(); - boolean success = true; + int attempts = 0; + int failures = 0; for (Tenant tenant : tenants.asList()) { log.log(FINE, () -> "Updating contact information for " + tenant); try { + attempts++; switch (tenant.type()) { case athenz: tenants.lockIfPresent(tenant.name(), LockedTenant.Athenz.class, lockedTenant -> { @@ -56,13 +58,13 @@ public class ContactInformationMaintainer extends ControllerMaintainer { throw new IllegalArgumentException("Unexpected tenant type '" + tenant.type() + "'."); } } catch (Exception e) { - success = false; + failures++; log.log(Level.WARNING, "Failed to update contact information for " + tenant + ": " + Exceptions.toMessageString(e) + ". Retrying in " + interval()); } } - return success; + return asSuccessFactor(attempts, failures); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContainerImageExpirer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContainerImageExpirer.java index ff5fc4d2051..f1574381a3d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContainerImageExpirer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContainerImageExpirer.java @@ -34,7 +34,7 @@ public class ContainerImageExpirer extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { Instant now = controller().clock().instant(); VersionStatus versionStatus = controller().readVersionStatus(); List<ContainerImage> imagesToExpire = controller().serviceRegistry().containerRegistry().list().stream() @@ -44,7 +44,7 @@ public class ContainerImageExpirer extends ControllerMaintainer { log.log(Level.INFO, "Expiring " + imagesToExpire.size() + " container images: " + imagesToExpire); controller().serviceRegistry().containerRegistry().deleteAll(imagesToExpire); } - return true; + return 1.0; } /** Returns whether given image is expired */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintainer.java index 03a6268397e..810c412fcc0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintainer.java @@ -34,7 +34,7 @@ public abstract class ControllerMaintainer extends Maintainer { public ControllerMaintainer(Controller controller, Duration interval, String name, Set<SystemName> activeSystems) { super(name, interval, controller.clock().instant(), controller.jobControl(), - jobMetrics(controller.metric()), controller.curator().cluster(), true); + new ControllerJobMetrics(controller.metric()), controller.curator().cluster(), true); this.controller = controller; this.activeSystems = Set.copyOf(Objects.requireNonNull(activeSystems)); } @@ -47,10 +47,20 @@ public abstract class ControllerMaintainer extends Maintainer { super.run(); } - private static JobMetrics jobMetrics(Metric metric) { - return new JobMetrics((job, consecutiveFailures) -> { - metric.set("maintenance.consecutiveFailures", consecutiveFailures, metric.createContext(Map.of("job", job))); - }); + private static class ControllerJobMetrics extends JobMetrics { + + private final Metric metric; + + public ControllerJobMetrics(Metric metric) { + this.metric = metric; + } + + @Override + protected void recordCompletion(String job, Long incompleteRuns, double successFactor) { + metric.set("maintenance.consecutiveFailures", incompleteRuns, metric.createContext(Map.of("job", job))); + metric.set("maintenance.successFactor", successFactor, metric.createContext(Map.of("job", job))); + } + } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CostReportMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CostReportMaintainer.java index 28b64b5bfe0..21cda09d92a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CostReportMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CostReportMaintainer.java @@ -31,10 +31,10 @@ public class CostReportMaintainer extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { var csv = CostCalculator.resourceShareByPropertyToCsv(nodeRepository, controller(), clock, consumer.fixedAllocations()); consumer.consume(csv); - return true; + return 1.0; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirer.java index e5316788802..9e3da506ca8 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirer.java @@ -28,8 +28,9 @@ public class DeploymentExpirer extends ControllerMaintainer { } @Override - protected boolean maintain() { - boolean success = true; + protected double maintain() { + int attempts = 0; + int failures = 0; for (Application application : controller().applications().readable()) { for (Instance instance : application.instances().values()) for (Deployment deployment : instance.deployments().values()) { @@ -37,16 +38,17 @@ public class DeploymentExpirer extends ControllerMaintainer { try { log.log(Level.INFO, "Expiring deployment of " + instance.id() + " in " + deployment.zone()); + attempts++; controller().applications().deactivate(instance.id(), deployment.zone()); } catch (Exception e) { - success = false; + failures++; log.log(Level.WARNING, "Could not expire " + deployment + " of " + instance + ": " + Exceptions.toMessageString(e) + ". Retrying in " + interval()); } } } - return success; + return asSuccessFactor(attempts, failures); } /** Returns whether given deployment has expired according to its TTL */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java index a3070ef55a0..4e53e07f5af 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java @@ -21,6 +21,7 @@ import java.util.Collection; import java.util.List; import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Level; import static com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence.broken; @@ -45,10 +46,11 @@ public class DeploymentIssueReporter extends ControllerMaintainer { } @Override - protected boolean maintain() { - return maintainDeploymentIssues(applications()) & - maintainPlatformIssue(applications()) & - escalateInactiveDeploymentIssues(applications()); + protected double maintain() { + return ( maintainDeploymentIssues(applications()) + + maintainPlatformIssue(applications()) + + escalateInactiveDeploymentIssues(applications())) + / 3; } /** Returns the applications to maintain issue status for. */ @@ -63,7 +65,7 @@ public class DeploymentIssueReporter extends ControllerMaintainer { * and store the issue id for the filed issues. Also, clear the issueIds of applications * where deployment has not failed for this amount of time. */ - private boolean maintainDeploymentIssues(List<Application> applications) { + private double maintainDeploymentIssues(List<Application> applications) { List<TenantAndApplicationId> failingApplications = controller().jobController().deploymentStatuses(ApplicationList.from(applications)) .failingApplicationChangeSince(controller().clock().instant().minus(maxFailureAge)) .mapToList(status -> status.application().id()); @@ -73,7 +75,7 @@ public class DeploymentIssueReporter extends ControllerMaintainer { fileDeploymentIssueFor(application); else store(application.id(), null); - return true; + return 1.0; } /** @@ -81,27 +83,26 @@ public class DeploymentIssueReporter extends ControllerMaintainer { * applications that have been failing the upgrade to the system version for * longer than the set grace period, or update this list if the issue already exists. */ - private boolean maintainPlatformIssue(List<Application> applications) { - boolean success = true; + private double maintainPlatformIssue(List<Application> applications) { if (controller().system() == SystemName.cd) - return success; + return 1.0; VersionStatus versionStatus = controller().readVersionStatus(); Version systemVersion = controller().systemVersion(versionStatus); if (versionStatus.version(systemVersion).confidence() != broken) - return success; + return 1.0; DeploymentStatusList statuses = controller().jobController().deploymentStatuses(ApplicationList.from(applications)); if (statuses.failingUpgradeToVersionSince(systemVersion, controller().clock().instant().minus(upgradeGracePeriod)).isEmpty()) - return success; + return 1.0; List<ApplicationId> failingApplications = statuses.failingUpgradeToVersionSince(systemVersion, controller().clock().instant()) .mapToList(status -> status.application().id().defaultInstance()); // TODO jonmv: Send only tenant and application, here and elsewhere in this. deploymentIssues.fileUnlessOpen(failingApplications, systemVersion); - return success; + return 1.0; } private Tenant ownerOf(TenantAndApplicationId applicationId) { @@ -126,21 +127,23 @@ public class DeploymentIssueReporter extends ControllerMaintainer { } /** Escalate issues for which there has been no activity for a certain amount of time. */ - private boolean escalateInactiveDeploymentIssues(Collection<Application> applications) { - AtomicBoolean success = new AtomicBoolean(true); + private double escalateInactiveDeploymentIssues(Collection<Application> applications) { + AtomicInteger attempts = new AtomicInteger(0); + AtomicInteger failures = new AtomicInteger(0); applications.forEach(application -> application.deploymentIssueId().ifPresent(issueId -> { try { + attempts.incrementAndGet(); Tenant tenant = ownerOf(application.id()); deploymentIssues.escalateIfInactive(issueId, maxInactivity, tenant.type() == Tenant.Type.athenz ? tenant.contact() : Optional.empty()); } catch (RuntimeException e) { - success.set(false); + failures.incrementAndGet(); log.log(Level.INFO, "Exception caught when attempting to escalate issue with id '" + issueId + "': " + Exceptions.toMessageString(e)); } })); - return success.get(); + return asSuccessFactor(attempts.get(), failures.get()); } private void store(TenantAndApplicationId id, IssueId issueId) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java index a8214ac8a09..20154c4f122 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java @@ -44,7 +44,7 @@ public class DeploymentMetricsMaintainer extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { AtomicInteger failures = new AtomicInteger(0); AtomicInteger attempts = new AtomicInteger(0); AtomicReference<Exception> lastException = new AtomicReference<>(null); @@ -92,7 +92,7 @@ public class DeploymentMetricsMaintainer extends ControllerMaintainer { } catch (InterruptedException e) { throw new RuntimeException(e); } - return lastException.get() == null; + return asSuccessFactor(attempts.get(), failures.get()); } static DeploymentMetrics updateDeploymentMetrics(DeploymentMetrics current, List<ClusterMetrics> metrics) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java index 55a957f0247..85a69b0f338 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java @@ -54,7 +54,7 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { try { // In order of importance deployRefreshedCertificates(); @@ -62,10 +62,10 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { deleteUnusedCertificates(); } catch (Exception e) { log.log(LogLevel.ERROR, "Exception caught while maintaining endpoint certificates", e); - return false; + return 0.0; } - return true; + return 1.0; } private void updateRefreshedCertificates() { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/HostInfoUpdater.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/HostInfoUpdater.java index 83ccda422e6..10e6f9eb039 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/HostInfoUpdater.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/HostInfoUpdater.java @@ -38,7 +38,7 @@ public class HostInfoUpdater extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { Map<String, NodeEntity> nodeEntities = controller().serviceRegistry().entityService().listNodes().stream() .collect(Collectors.toMap(NodeEntity::hostname, Function.identity())); @@ -62,7 +62,7 @@ public class HostInfoUpdater extends ControllerMaintainer { LOG.info("Updated information for " + hostsUpdated + " hosts(s)"); } } - return true; + return 1.0; } private static Optional<String> buildModelName(NodeEntity nodeEntity) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java index 9859d12510a..5101de73a33 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java @@ -39,23 +39,28 @@ public abstract class InfrastructureUpgrader<VERSION> extends ControllerMaintain } @Override - protected boolean maintain() { - targetVersion().ifPresent(target -> upgradeAll(target, managedApplications)); - return true; + protected double maintain() { + if (targetVersion().isEmpty()) return 1.0; + return upgradeAll(targetVersion().get(), managedApplications); } /** Deploy a list of system applications until they converge on the given version */ - private void upgradeAll(VERSION target, List<SystemApplication> applications) { + private double upgradeAll(VERSION target, List<SystemApplication> applications) { + int attempts = 0; + int failures = 0; for (List<ZoneApi> zones : upgradePolicy.asList()) { boolean converged = true; for (ZoneApi zone : zones) { try { + attempts++; converged &= upgradeAll(target, applications, zone); } catch (UnreachableNodeRepositoryException e) { + failures++; converged = false; log.warning(String.format("%s: Failed to communicate with node repository in %s, continuing with next parallel zone: %s", this, zone, Exceptions.toMessageString(e))); } catch (Exception e) { + failures++; converged = false; log.warning(String.format("%s: Failed to upgrade zone: %s, continuing with next parallel zone: %s", this, zone, Exceptions.toMessageString(e))); @@ -65,6 +70,7 @@ public abstract class InfrastructureUpgrader<VERSION> extends ControllerMaintain break; } } + return asSuccessFactor(attempts, failures); } /** Returns whether all applications have converged to the target version in zone */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java index b84cfe5af9b..25207b733f0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java @@ -49,10 +49,10 @@ public class JobRunner extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { jobs.active().forEach(this::advance); jobs.collectGarbage(); - return true; + return 1.0; } @Override diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java index b26b94f0b28..3f65c2e49cd 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java @@ -73,7 +73,7 @@ public class MetricsReporter extends ControllerMaintainer { } @Override - public boolean maintain() { + public double maintain() { reportDeploymentMetrics(); reportRemainingRotations(); reportQueuedNameServiceRequests(); @@ -82,7 +82,7 @@ public class MetricsReporter extends ControllerMaintainer { reportAuditLog(); reportBrokenSystemVersion(versionStatus); reportTenantMetrics(); - return true; + return 1.0; } private void reportBrokenSystemVersion(VersionStatus versionStatus) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/NameServiceDispatcher.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/NameServiceDispatcher.java index e57affdc15d..fe20db00e05 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/NameServiceDispatcher.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/NameServiceDispatcher.java @@ -37,13 +37,12 @@ public class NameServiceDispatcher extends ControllerMaintainer { } @Override - protected boolean maintain() { - boolean success = true; + protected double maintain() { try (var lock = db.lockNameServiceQueue()) { var queue = db.readNameServiceQueue(); var instant = clock.instant(); var remaining = queue.dispatchTo(nameService, requestCount); - if (queue == remaining) return success; // Queue unchanged + if (queue == remaining) return 1.0; // Queue unchanged var dispatched = queue.first(requestCount); if (!dispatched.requests().isEmpty()) { @@ -54,7 +53,7 @@ public class NameServiceDispatcher extends ControllerMaintainer { } db.writeNameServiceQueue(remaining); } - return success; + return 1.0; } private static int requestCount(SystemName system) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java index e1618f05a7d..666d1c3b23a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java @@ -42,13 +42,13 @@ public class OsUpgradeScheduler extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { for (var cloud : supportedClouds()) { Optional<Version> newTarget = newTargetIn(cloud); if (newTarget.isEmpty()) continue; controller().upgradeOsIn(cloud, newTarget.get(), upgradeBudget(), false); } - return true; + return 1.0; } /** Returns the new target version for given cloud, if any */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdater.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdater.java index cbd9207fda4..271dd277e1c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdater.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdater.java @@ -18,16 +18,16 @@ public class OsVersionStatusUpdater extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { try { OsVersionStatus newStatus = OsVersionStatus.compute(controller()); controller().updateOsVersionStatus(newStatus); - return true; + return 1.0; } catch (Exception e) { log.log(Level.WARNING, "Failed to compute OS version status: " + Exceptions.toMessageString(e) + ". Retrying in " + interval()); } - return false; + return 0.0; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java index a032f266de5..9d93ac719b7 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java @@ -19,13 +19,13 @@ public class OutstandingChangeDeployer extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { for (Application application : ApplicationList.from(controller().applications().readable()) .withProductionDeployment() .withDeploymentSpec() .asList()) controller().applications().deploymentTrigger().triggerNewRevision(application.id()); - return true; + return 1.0; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ReadyJobsTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ReadyJobsTrigger.java index a626f21359a..ffe958cb63a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ReadyJobsTrigger.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ReadyJobsTrigger.java @@ -17,9 +17,9 @@ public class ReadyJobsTrigger extends ControllerMaintainer { } @Override - public boolean maintain() { + public double maintain() { controller().applications().deploymentTrigger().triggerReadyJobs(); - return true; + return 1.0; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ReindexingTriggerer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ReindexingTriggerer.java index 263a33cf266..0bd74c844ae 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ReindexingTriggerer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ReindexingTriggerer.java @@ -40,7 +40,7 @@ public class ReindexingTriggerer extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { try { Instant now = controller().clock().instant(); for (Application application : controller().applications().asList()) @@ -51,11 +51,11 @@ public class ReindexingTriggerer extends ControllerMaintainer { && reindexingIsReady(controller().applications().applicationReindexing(id, deployment.zone()), now)) controller().applications().reindex(id, deployment.zone(), List.of(), List.of(), true); }); - return true; + return 1.0; } catch (RuntimeException e) { log.log(Level.WARNING, "Failed to trigger reindexing: " + Exceptions.toMessageString(e)); - return false; + return 0.0; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java index aed2e637e4b..39ad233ce46 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java @@ -79,19 +79,19 @@ public class ResourceMeterMaintainer extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { Collection<ResourceSnapshot> resourceSnapshots; try { resourceSnapshots = getAllResourceSnapshots(); } catch (Exception e) { log.log(Level.WARNING, "Failed to collect resource snapshots. Retrying in " + interval() + ". Error: " + Exceptions.toMessageString(e)); - return false; + return 0.0; } if (systemName.isPublic()) reportResourceSnapshots(resourceSnapshots); updateDeploymentCost(resourceSnapshots); - return true; + return 1.0; } void updateDeploymentCost(Collection<ResourceSnapshot> resourceSnapshots) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceTagMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceTagMaintainer.java index c7bf7e765ed..ab988bcf0ac 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceTagMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceTagMaintainer.java @@ -28,7 +28,7 @@ public class ResourceTagMaintainer extends ControllerMaintainer { } @Override - public boolean maintain() { + public double maintain() { controller().zoneRegistry().zones() .ofCloud(CloudName.from("aws")) .reachable() @@ -38,7 +38,7 @@ public class ResourceTagMaintainer extends ControllerMaintainer { if (taggedResources > 0) log.log(Level.INFO, "Tagged " + taggedResources + " resources in " + zone.getId()); }); - return true; + return 1.0; } private Map<HostName, Optional<ApplicationId>> getTenantOfParentHosts(ZoneId zoneId) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemRoutingPolicyMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemRoutingPolicyMaintainer.java index 3b0a1fca4af..e40d772a673 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemRoutingPolicyMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemRoutingPolicyMaintainer.java @@ -21,14 +21,14 @@ public class SystemRoutingPolicyMaintainer extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { for (var zone : controller().zoneRegistry().zones().all().ids()) { for (var application : SystemApplication.values()) { if (!application.hasEndpoint()) continue; controller().routing().policies().refresh(application.id(), DeploymentSpec.empty, zone); } } - return true; + return 1.0; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/TenantRoleMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/TenantRoleMaintainer.java index 1265d687850..637ae10bcc6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/TenantRoleMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/TenantRoleMaintainer.java @@ -23,7 +23,7 @@ public class TenantRoleMaintainer extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { var roleService = controller().serviceRegistry().roleService(); var tenants = controller().tenants().asList(); var tenantsWithRoles = tenants.stream() @@ -31,7 +31,7 @@ public class TenantRoleMaintainer extends ControllerMaintainer { .filter(this::hasProductionDeployment) .collect(Collectors.toList()); roleService.maintainRoles(tenantsWithRoles); - return true; + return 1.0; } private boolean hasProductionDeployment(TenantName tenant) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/TrafficShareUpdater.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/TrafficShareUpdater.java index fbe9faa9754..0af0d01478b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/TrafficShareUpdater.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/TrafficShareUpdater.java @@ -36,30 +36,34 @@ public class TrafficShareUpdater extends ControllerMaintainer { } @Override - protected boolean maintain() { - boolean success = false; + protected double maintain() { Exception lastException = null; + int attempts = 0; + int failures = 0; for (var application : applications.asList()) { for (var instance : application.instances().values()) { for (var deployment : instance.deployments().values()) { if ( ! deployment.zone().environment().isProduction()) continue; try { - success |= updateTrafficFraction(instance, deployment); + attempts++; + updateTrafficFraction(instance, deployment); } catch (Exception e) { // Some failures due to locked applications are expected and benign + failures++; lastException = e; } } } } - if ( ! success && lastException != null) // log on complete failure + double successFactor = asSuccessFactor(attempts, failures); + if ( successFactor == 0 ) log.log(Level.WARNING, "Could not update traffic share on any applications", lastException); - return success; + return successFactor; } - private boolean updateTrafficFraction(Instance instance, Deployment deployment) { + private void updateTrafficFraction(Instance instance, Deployment deployment) { double qpsInZone = deployment.metrics().queriesPerSecond(); double totalQps = instance.deployments().values().stream() .filter(i -> i.zone().environment().isProduction()) @@ -73,7 +77,6 @@ public class TrafficShareUpdater extends ControllerMaintainer { maxReadShare = currentReadShare; // distribution can be incorrect nodeRepository.patchApplication(deployment.zone(), instance.id(), currentReadShare, maxReadShare); - return true; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java index 8d5019904fa..2326f7b66ee 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java @@ -51,7 +51,7 @@ public class Upgrader extends ControllerMaintainer { * Schedule application upgrades. Note that this implementation must be idempotent. */ @Override - public boolean maintain() { + public double maintain() { // Determine target versions for each upgrade policy VersionStatus versionStatus = controller().readVersionStatus(); Version canaryTarget = controller().systemVersion(versionStatus); @@ -91,7 +91,7 @@ public class Upgrader extends ControllerMaintainer { upgrade(instances.with(UpgradePolicy.canary), canaryTarget, targetMajorVersion, instances.size()); defaultTargets.forEach(target -> upgrade(instances.with(UpgradePolicy.defaultPolicy), target, targetMajorVersion, numberOfApplicationsToUpgrade())); conservativeTargets.forEach(target -> upgrade(instances.with(UpgradePolicy.conservative), target, targetMajorVersion, numberOfApplicationsToUpgrade())); - return true; + return 1.0; } /** Returns the target versions for given confidence, one per major version in the system */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VCMRMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VCMRMaintainer.java index fedf3d90760..4cd24289676 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VCMRMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VCMRMaintainer.java @@ -57,7 +57,7 @@ public class VCMRMaintainer extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { var changeRequests = curator.readChangeRequests() .stream() .filter(shouldUpdate()) @@ -81,7 +81,7 @@ public class VCMRMaintainer extends ControllerMaintainer { }); } }); - return true; + return 1.0; } /** diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java index a3e9672b715..e4866c43f13 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java @@ -29,7 +29,7 @@ public class VersionStatusUpdater extends ControllerMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { try { VersionStatus newStatus = VersionStatus.compute(controller()); controller().updateVersionStatus(newStatus); @@ -37,12 +37,12 @@ public class VersionStatusUpdater extends ControllerMaintainer { controller().serviceRegistry().systemMonitor().reportSystemVersion(version.versionNumber(), convert(version.confidence())); }); - return true; + return 1.0; } catch (Exception e) { log.log(Level.WARNING, "Failed to compute version status: " + Exceptions.toMessageString(e) + ". Retrying in " + interval()); } - return false; + return 0.0; } static SystemMonitor.Confidence convert(VespaVersion.Confidence confidence) { 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 d7b805c0949..d1cbe8e14b4 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 @@ -901,6 +901,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { cluster.suggested().ifPresent(suggested -> toSlime(suggested, clusterObject.setObject("suggested"))); utilizationToSlime(cluster.utilization(), clusterObject.setObject("utilization")); scalingEventsToSlime(cluster.scalingEvents(), clusterObject.setArray("scalingEvents")); + clusterObject.setString("autoscalingStatusCode", cluster.autoscalingStatusCode()); clusterObject.setString("autoscalingStatus", cluster.autoscalingStatus()); clusterObject.setLong("scalingDuration", cluster.scalingDuration().toMillis()); clusterObject.setDouble("maxQueryGrowthRate", cluster.maxQueryGrowthRate()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/support/access/SupportAccessControl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/support/access/SupportAccessControl.java index ccee1b4af43..6bbec918ba9 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/support/access/SupportAccessControl.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/support/access/SupportAccessControl.java @@ -87,8 +87,8 @@ public class SupportAccessControl { if (supportAccess.currentStatus(now).state() == NOT_ALLOWED) return List.of(); return supportAccess.grantHistory().stream() - .filter(grant -> !grant.certificate().getNotBefore().toInstant().isBefore(now)) - .filter(grant -> !grant.certificate().getNotAfter().toInstant().isAfter(now)) + .filter(grant -> now.isAfter(grant.certificate().getNotBefore().toInstant())) + .filter(grant -> now.isBefore(grant.certificate().getNotAfter().toInstant())) .collect(Collectors.toUnmodifiableList()); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index c3a527a0bd9..4203051965b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -122,7 +122,8 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer current, Instant.ofEpochMilli(1234), Optional.of(Instant.ofEpochMilli(2234)))), - "the autoscaling status", + "ideal", + "Cluster is ideally scaled", Duration.ofMinutes(6), 0.7, 0.3); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintainerTest.java index 27b4f3744e7..7dc5cb34818 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintainerTest.java @@ -39,13 +39,16 @@ public class ControllerMaintainerTest { TestControllerMaintainer maintainer = new TestControllerMaintainer(tester.controller(), SystemName.main, new AtomicInteger()); maintainer.run(); assertEquals(0L, consecutiveFailuresMetric()); + assertEquals(1.0, successFactorMetric(), 0.0000001); maintainer.success = false; maintainer.run(); maintainer.run(); assertEquals(2L, consecutiveFailuresMetric()); + assertEquals(0.0, successFactorMetric(), 0.0000001); maintainer.success = true; maintainer.run(); assertEquals(0, consecutiveFailuresMetric()); + assertEquals(1.0, successFactorMetric(), 0.0000001); } private long consecutiveFailuresMetric() { @@ -54,6 +57,12 @@ public class ControllerMaintainerTest { "maintenance.consecutiveFailures").get().longValue(); } + private long successFactorMetric() { + MetricsMock metrics = (MetricsMock) tester.controller().metric(); + return metrics.getMetric((context) -> "TestControllerMaintainer".equals(context.get("job")), + "maintenance.successFactor").get().longValue(); + } + private static class TestControllerMaintainer extends ControllerMaintainer { private final AtomicInteger executions; @@ -65,9 +74,9 @@ public class ControllerMaintainerTest { } @Override - protected boolean maintain() { + protected double maintain() { executions.incrementAndGet(); - return success; + return success ? 1.0 : 0.0; } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java index 66bda66bbf9..ce219b8beed 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java @@ -33,7 +33,7 @@ public class EndpointCertificateMaintainerTest { @Test public void old_and_unused_cert_is_deleted() { tester.curator().writeEndpointCertificateMetadata(ApplicationId.defaultId(), exampleMetadata); - assertTrue(maintainer.maintain()); + assertEquals(1.0, maintainer.maintain(), 0.0000001); assertTrue(tester.curator().readEndpointCertificateMetadata(ApplicationId.defaultId()).isEmpty()); } @@ -41,7 +41,7 @@ public class EndpointCertificateMaintainerTest { public void unused_but_recently_used_cert_is_not_deleted() { EndpointCertificateMetadata recentlyRequestedCert = exampleMetadata.withLastRequested(tester.clock().instant().minusSeconds(3600).getEpochSecond()); tester.curator().writeEndpointCertificateMetadata(ApplicationId.defaultId(), recentlyRequestedCert); - assertTrue(maintainer.maintain()); + assertEquals(1.0, maintainer.maintain(), 0.0000001); assertEquals(Optional.of(recentlyRequestedCert), tester.curator().readEndpointCertificateMetadata(ApplicationId.defaultId())); } @@ -53,7 +53,7 @@ public class EndpointCertificateMaintainerTest { secretStore.setSecret(exampleMetadata.keyName(), "foo", 1); secretStore.setSecret(exampleMetadata.certName(), "bar", 1); - assertTrue(maintainer.maintain()); + assertEquals(1.0, maintainer.maintain(), 0.0000001); var updatedCert = Optional.of(recentlyRequestedCert.withLastRefreshed(tester.clock().instant().getEpochSecond()).withVersion(1)); @@ -77,7 +77,7 @@ public class EndpointCertificateMaintainerTest { tester.curator().writeEndpointCertificateMetadata(appId, exampleMetadata); - assertTrue(maintainer.maintain()); + assertEquals(1.0, maintainer.maintain(), 0.0000001); assertTrue(tester.curator().readEndpointCertificateMetadata(appId).isPresent()); // cert should not be deleted, the app is deployed! } @@ -97,7 +97,7 @@ public class EndpointCertificateMaintainerTest { tester.curator().writeEndpointCertificateMetadata(appId, exampleMetadata); - assertTrue(maintainer.maintain()); + assertEquals(1.0, maintainer.maintain(), 0.0000001); assertTrue(tester.curator().readEndpointCertificateMetadata(appId).isPresent()); // cert should not be deleted, the app is deployed! tester.clock().advance(Duration.ofDays(3)); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/TrafficShareUpdaterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/TrafficShareUpdaterTest.java index 2afa3a0faea..7b4882de3ff 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/TrafficShareUpdaterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/TrafficShareUpdaterTest.java @@ -39,7 +39,7 @@ public class TrafficShareUpdaterTest { // Single zone setQpsMetric(50.0, application.application().id().defaultInstance(), prod1, tester); deploymentMetricsMaintainer.maintain(); - assertTrue(updater.maintain()); + assertEquals(1.0, updater.maintain(), 0.0000001); assertTrafficFraction(1.0, 1.0, application.instanceId(), prod1, tester); // Two zones @@ -48,14 +48,14 @@ public class TrafficShareUpdaterTest { setQpsMetric(50.0, application.application().id().defaultInstance(), prod1, tester); setQpsMetric(0.0, application.application().id().defaultInstance(), prod2, tester); deploymentMetricsMaintainer.maintain(); - assertTrue(updater.maintain()); + assertEquals(1.0, updater.maintain(), 0.0000001); assertTrafficFraction(1.0, 1.0, application.instanceId(), prod1, tester); assertTrafficFraction(0.0, 1.0, application.instanceId(), prod2, tester); // - both hot setQpsMetric(53.0, application.application().id().defaultInstance(), prod1, tester); setQpsMetric(47.0, application.application().id().defaultInstance(), prod2, tester); deploymentMetricsMaintainer.maintain(); - assertTrue(updater.maintain()); + assertEquals(1.0, updater.maintain(), 0.0000001); assertTrafficFraction(0.53, 1.0, application.instanceId(), prod1, tester); assertTrafficFraction(0.47, 1.0, application.instanceId(), prod2, tester); @@ -66,7 +66,7 @@ public class TrafficShareUpdaterTest { setQpsMetric(47.0, application.application().id().defaultInstance(), prod2, tester); setQpsMetric(0.0, application.application().id().defaultInstance(), prod3, tester); deploymentMetricsMaintainer.maintain(); - assertTrue(updater.maintain()); + assertEquals(1.0, updater.maintain(), 0.0000001); assertTrafficFraction(0.53, 0.53, application.instanceId(), prod1, tester); assertTrafficFraction(0.47, 0.50, application.instanceId(), prod2, tester); assertTrafficFraction(0.00, 0.50, application.instanceId(), prod3, tester); @@ -75,7 +75,7 @@ public class TrafficShareUpdaterTest { setQpsMetric(25.0, application.application().id().defaultInstance(), prod2, tester); setQpsMetric(25.0, application.application().id().defaultInstance(), prod3, tester); deploymentMetricsMaintainer.maintain(); - assertTrue(updater.maintain()); + assertEquals(1.0, updater.maintain(), 0.0000001); assertTrafficFraction(0.50, 0.5, application.instanceId(), prod1, tester); assertTrafficFraction(0.25, 0.5, application.instanceId(), prod2, tester); assertTrafficFraction(0.25, 0.5, application.instanceId(), prod3, tester); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index ce7b4a6123b..f4b8a643e28 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -71,6 +71,7 @@ import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; import com.yahoo.vespa.hosted.controller.routing.GlobalRouting; import com.yahoo.vespa.hosted.controller.security.AthenzCredentials; import com.yahoo.vespa.hosted.controller.security.AthenzTenantSpec; +import com.yahoo.vespa.hosted.controller.support.access.SupportAccessGrant; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import com.yahoo.vespa.hosted.controller.tenant.LastLoginInfo; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; @@ -1549,6 +1550,10 @@ public class ApplicationApiTest extends ControllerContainerTest { grantResponse, 200 ); + // Should be 1 available grant + List<SupportAccessGrant> activeGrants = tester.controller().supportAccess().activeGrantsFor(new DeploymentId(ApplicationId.fromSerializedForm("tenant1:application1:instance1"), zone)); + assertEquals(1, activeGrants.size()); + // DELETE removes access String disallowedResponse = grantResponse .replaceAll("ALLOWED\".*?}", "NOT_ALLOWED\"}") @@ -1557,6 +1562,10 @@ public class ApplicationApiTest extends ControllerContainerTest { .userIdentity(USER_ID), disallowedResponse, 200 ); + + // Should be no available grant + activeGrants = tester.controller().supportAccess().activeGrantsFor(new DeploymentId(ApplicationId.fromSerializedForm("tenant1:application1:instance1"), zone)); + assertEquals(0, activeGrants.size()); } private static String serializeInstant(Instant i) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-clusters.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-clusters.json index cd1fe5acf6a..fc40a9ce692 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-clusters.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-clusters.json @@ -97,7 +97,8 @@ "completion": 2234 } ], - "autoscalingStatus": "the autoscaling status", + "autoscalingStatusCode": "ideal", + "autoscalingStatus": "Cluster is ideally scaled", "scalingDuration": 360000, "maxQueryGrowthRate": 0.7, "currentQueryFractionOfMax":0.3 diff --git a/default_build_settings.cmake b/default_build_settings.cmake index 326481e25c7..3fd93bd0c25 100644 --- a/default_build_settings.cmake +++ b/default_build_settings.cmake @@ -302,8 +302,8 @@ function(vespa_use_default_cxx_compiler) unset(DEFAULT_CMAKE_CXX_COMPILER) if(NOT DEFINED VESPA_COMPILER_VARIANT OR VESPA_COMPILER_VARIANT STREQUAL "gcc") if(APPLE) - set(DEFAULT_CMAKE_C_COMPILER "/usr/local/bin/gcc-10") - set(DEFAULT_CMAKE_CXX_COMPILER "/usr/local/bin/g++-10") + set(DEFAULT_CMAKE_C_COMPILER "/usr/local/bin/gcc-11") + set(DEFAULT_CMAKE_CXX_COMPILER "/usr/local/bin/g++-11") elseif(VESPA_OS_DISTRO_COMBINED STREQUAL "amzn 2") set(DEFAULT_CMAKE_C_COMPILER "/usr/bin/gcc10-gcc") set(DEFAULT_CMAKE_CXX_COMPILER "/usr/bin/gcc10-g++") diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index c1750c73c2b..cfa14979d3f 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -41,15 +41,8 @@ import static com.yahoo.vespa.flags.FetchVector.Dimension.ZONE_ID; * @author hakonhall */ public class Flags { - private static volatile TreeMap<FlagId, FlagDefinition> flags = new TreeMap<>(); - public static final UnboundStringFlag ALLOCATE_OS_REQUIREMENT = defineStringFlag( - "allocate-os-requirement", "any", - List.of("hakonhall"), "2021-01-26", "2021-07-26", - "Allocations of new nodes are limited to the given host OS. Must be one of 'rhel7', " + - "'rhel8', or 'any'", - "Takes effect on next (re)deployment.", - APPLICATION_ID); + private static volatile TreeMap<FlagId, FlagDefinition> flags = new TreeMap<>(); public static final UnboundDoubleFlag DEFAULT_TERM_WISE_LIMIT = defineDoubleFlag( "default-term-wise-limit", 1.0, @@ -146,14 +139,14 @@ public class Flags { public static final UnboundBooleanFlag ENCRYPT_DISK = defineFeatureFlag( "encrypt-disk", false, - List.of("hakonhall"), "2021-05-05", "2021-06-05", + List.of("hakonhall"), "2021-05-05", "2021-08-05", "Allow migrating an unencrypted data partition to being encrypted.", "Takes effect on next host-admin tick."); public static final UnboundBooleanFlag ENCRYPT_DIRTY_DISK = defineFeatureFlag( "encrypt-dirty-disk", false, - List.of("hakonhall"), "2021-05-14", "2021-06-05", - "Allow migrating an unencrypted data partition to being encrypted when provisioned or dirty.", + List.of("hakonhall"), "2021-05-14", "2021-08-05", + "Allow migrating an unencrypted data partition to being encrypted when (de)provisioned.", "Takes effect on next host-admin tick."); public static final UnboundBooleanFlag ENABLE_FEED_BLOCK_IN_DISTRIBUTOR = defineFeatureFlag( @@ -211,6 +204,20 @@ public class Flags { "Takes effect after distributor restart", ZONE_ID, APPLICATION_ID); + public static final UnboundIntFlag MAX_CONCURRENT_MERGES_PER_NODE = defineIntFlag( + "max-concurrent-merges-per-node", 16, + List.of("balder", "vekterli"), "2021-06-06", "2021-08-01", + "Specifies max concurrent merges per content node.", + "Takes effect at redeploy", + ZONE_ID, APPLICATION_ID); + + public static final UnboundIntFlag MAX_MERGE_QUEUE_SIZE = defineIntFlag( + "max-merge-queue-size", 1024, + List.of("balder", "vekterli"), "2021-06-06", "2021-08-01", + "Specifies max size of merge queue.", + "Takes effect at redeploy", + ZONE_ID, APPLICATION_ID); + public static final UnboundBooleanFlag USE_EXTERNAL_RANK_EXPRESSION = defineFeatureFlag( "use-external-rank-expression", false, List.of("baldersheim"), "2021-05-24", "2021-07-01", @@ -252,6 +259,13 @@ public class Flags { "The maximum number of hosts allowed to encrypt their disk concurrently", "Takes effect on next run of HostEncrypter, but any currently encrypting hosts will not be cancelled when reducing the limit"); + public static final UnboundBooleanFlag REQUIRE_CONNECTIVITY_CHECK = defineFeatureFlag( + "require-connectivity-check", false, + List.of("arnej"), "2021-06-03", "2021-09-01", + "Require that config-sentinel connectivity check passes with good quality before starting services", + "Takes effect on next restart", + ZONE_ID, APPLICATION_ID); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, List<String> owners, String createdAt, String expiresAt, String description, diff --git a/metrics/src/vespa/metrics/countmetric.h b/metrics/src/vespa/metrics/countmetric.h index 02a6827d1ce..1701071104e 100644 --- a/metrics/src/vespa/metrics/countmetric.h +++ b/metrics/src/vespa/metrics/countmetric.h @@ -105,7 +105,7 @@ public: void addToSnapshot(Metric&, std::vector<Metric::UP> &) const override; }; -typedef CountMetric<uint64_t, true> LongCountMetric; +using LongCountMetric = CountMetric<uint64_t, true>; } // metrics diff --git a/metrics/src/vespa/metrics/metricvalueset.h b/metrics/src/vespa/metrics/metricvalueset.h index 2463990378e..c522876f5b1 100644 --- a/metrics/src/vespa/metrics/metricvalueset.h +++ b/metrics/src/vespa/metrics/metricvalueset.h @@ -76,12 +76,6 @@ public: */ bool setValues(const ValueClass& values); - /** - * Retrieve and reset in a single operation, to minimize chance of - * alteration in the process. - */ - ValueClass getValuesAndReset(); - void reset() { setFlag(RESET); } @@ -105,9 +99,6 @@ public: _flags.store(_flags.load(std::memory_order_relaxed) & ~flags, std::memory_order_relaxed); } - uint32_t getFlags() const { - return _flags.load(std::memory_order_relaxed); - } }; } // metrics diff --git a/metrics/src/vespa/metrics/metricvalueset.hpp b/metrics/src/vespa/metrics/metricvalueset.hpp index 8c5b32afcf8..57b3e7f9901 100644 --- a/metrics/src/vespa/metrics/metricvalueset.hpp +++ b/metrics/src/vespa/metrics/metricvalueset.hpp @@ -70,14 +70,6 @@ MetricValueSet<ValueClass>::setValues(const ValueClass& values) { } template<typename ValueClass> -ValueClass -MetricValueSet<ValueClass>::getValuesAndReset() { - ValueClass result(getValues()); - setFlag(RESET); - return result; -} - -template<typename ValueClass> std::string MetricValueSet<ValueClass>::toString() { std::ostringstream ost; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java index 6d9eae5c4dc..e510618c5a4 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java @@ -4,7 +4,6 @@ package com.yahoo.vespa.hosted.node.admin.nodeadmin; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import java.time.Duration; -import java.util.List; import java.util.Set; /** @@ -41,10 +40,9 @@ public interface NodeAdmin { Duration subsystemFreezeDuration(); /** - * Stop services on these nodes - * @param nodes List of hostnames to suspend + * Stop all services on these nodes */ - void stopNodeAgentServices(List<String> nodes); + void stopNodeAgentServices(); /** * Start node-admin schedulers. diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java index caffe5ef2f1..ef9520969af 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java @@ -15,7 +15,6 @@ import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.HashSet; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -157,12 +156,11 @@ public class NodeAdminImpl implements NodeAdmin { } @Override - public void stopNodeAgentServices(List<String> hostnames) { + public void stopNodeAgentServices() { // Each container may spend 1-1:30 minutes stopping - hostnames.parallelStream() - .filter(nodeAgentWithSchedulerByHostname::containsKey) - .map(nodeAgentWithSchedulerByHostname::get) - .forEach(NodeAgentWithScheduler::stopForHostSuspension); + nodeAgentWithSchedulerByHostname.values() + .parallelStream() + .forEach(NodeAgentWithScheduler::stopForHostSuspension); } @Override diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java index 58ca4ae3f41..c24b2261f42 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java @@ -150,7 +150,7 @@ public class NodeAdminStateUpdater { // The node agent services are stopped by this thread, which is OK only // because the node agents are frozen (see above). - nodeAdmin.stopNodeAgentServices(nodesInActiveState); + nodeAdmin.stopNodeAgentServices(); break; default: throw new IllegalStateException("Unknown wanted state " + wantedState); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterTest.java index 4a678597e41..8ee3a95744b 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterTest.java @@ -102,7 +102,7 @@ public class NodeAdminStateUpdaterTest { // At this point orchestrator will say its OK to suspend, but something goes wrong when we try to stop services final String exceptionMessage = "Failed to stop services"; verify(orchestrator, times(0)).suspend(eq(hostHostname.value()), eq(suspendHostnames)); - doThrow(new RuntimeException(exceptionMessage)).doNothing().when(nodeAdmin).stopNodeAgentServices(eq(activeHostnames)); + doThrow(new RuntimeException(exceptionMessage)).doNothing().when(nodeAdmin).stopNodeAgentServices(); assertConvergeError(SUSPENDED, exceptionMessage); verify(orchestrator, times(1)).suspend(eq(hostHostname.value()), eq(suspendHostnames)); // Make sure we dont roll back if we fail to stop services - we will try to stop again next tick 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 d113ca68d01..f084b83bf97 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 @@ -109,7 +109,7 @@ public class NodeRepository extends AbstractComponent { "dynamicProvisioning property must be 1-to-1 with availability of HostProvisioner, was: dynamicProvisioning=%s, hostProvisioner=%s", zone.getCloud().dynamicProvisioning(), provisionServiceProvider.getHostProvisioner().map(__ -> "present").orElse("empty"))); - this.db = new CuratorDatabaseClient(flavors, curator, clock, zone, useCuratorClientCache, nodeCacheSize); + this.db = new CuratorDatabaseClient(flavors, curator, clock, useCuratorClientCache, nodeCacheSize); this.zone = zone; this.clock = clock; this.nodes = new Nodes(db, zone, clock); 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 5eb01b4fe72..c8d5e4361a5 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 @@ -62,7 +62,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(), AutoscalingStatus.empty()); else cluster = cluster.withConfiguration(exclusive, min, max); return with(cluster); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/AutoscalingStatus.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/AutoscalingStatus.java new file mode 100644 index 00000000000..c40408c9109 --- /dev/null +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/AutoscalingStatus.java @@ -0,0 +1,69 @@ +package com.yahoo.vespa.hosted.provision.applications; + +import com.yahoo.vespa.hosted.provision.autoscale.Autoscaler; + +import java.util.Objects; + +/** + * The current autoscaling status of a cluster. + * A value object. + * + * @author bratseth + */ +public class AutoscalingStatus { + + public enum Status { + + /** No status is available: Aautoscaling is disabled, or a brand new application. */ + unavailable, + + /** Autoscaling is not taking any action at the moment due to recent changes or a lack of data */ + waiting, + + /** The cluster is ideally scaled to the current load */ + ideal, + + /** The cluster should be rescaled further, but no better configuration is allowed by the current limits */ + insufficient, + + /** Rescaling of this cluster has been scheduled */ + rescaling + + }; + + private final Status status; + private final String description; + + public AutoscalingStatus(Status status, String description) { + this.status = status; + this.description = description; + } + + public Status status() { return status; } + public String description() { return description; } + + public static AutoscalingStatus empty() { return new AutoscalingStatus(Status.unavailable, ""); } + + @Override + public boolean equals(Object o) { + if (o == this) return true; + if ( ! ( o instanceof AutoscalingStatus)) return false; + + AutoscalingStatus other = (AutoscalingStatus)o; + if ( other.status != this.status ) return false; + if ( ! other.description.equals(this.description) ) return false; + return true; + } + + @Override + public int hashCode() { + return Objects.hash(status, description); + } + + @Override + public String toString() { + return "autoscaling status: " + status + + ( description.isEmpty() ? "" : " (" + description + ")"); + } + +} 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 59b70ff1ef0..d4bbe6adc1b 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 @@ -5,7 +5,6 @@ import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.vespa.hosted.provision.autoscale.Autoscaler; -import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.List; @@ -31,7 +30,7 @@ public class Cluster { /** The maxScalingEvents last scaling events of this, sorted by increasing time (newest last) */ private final List<ScalingEvent> scalingEvents; - private final String autoscalingStatus; + private final AutoscalingStatus autoscalingStatus; public Cluster(ClusterSpec.Id id, boolean exclusive, @@ -40,7 +39,7 @@ public class Cluster { Optional<Suggestion> suggestedResources, Optional<ClusterResources> targetResources, List<ScalingEvent> scalingEvents, - String autoscalingStatus) { + AutoscalingStatus autoscalingStatus) { this.id = Objects.requireNonNull(id); this.exclusive = exclusive; this.min = Objects.requireNonNull(minResources); @@ -95,8 +94,8 @@ public class Cluster { 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; } + /** The latest autoscaling status of this cluster, or unknown (never null) if none */ + public AutoscalingStatus autoscalingStatus() { return autoscalingStatus; } public Cluster withConfiguration(boolean exclusive, ClusterResources min, ClusterResources max) { return new Cluster(id, exclusive, min, max, suggested, target, scalingEvents, autoscalingStatus); @@ -124,7 +123,7 @@ public class Cluster { return new Cluster(id, exclusive, min, max, suggested, target, scalingEvents, autoscalingStatus); } - public Cluster withAutoscalingStatus(String autoscalingStatus) { + public Cluster with(AutoscalingStatus autoscalingStatus) { if (autoscalingStatus.equals(this.autoscalingStatus)) return this; return new Cluster(id, exclusive, min, max, suggested, target, scalingEvents, autoscalingStatus); } 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 1d0ba3da6c5..ccaf23c49ed 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 @@ -7,6 +7,8 @@ import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.applications.Application; +import com.yahoo.vespa.hosted.provision.applications.AutoscalingStatus; +import com.yahoo.vespa.hosted.provision.applications.AutoscalingStatus.Status; import com.yahoo.vespa.hosted.provision.applications.Cluster; import java.time.Duration; @@ -52,7 +54,8 @@ public class Autoscaler { * @return scaling advice for this cluster */ public Advice autoscale(Application application, Cluster cluster, NodeList clusterNodes) { - if (cluster.minResources().equals(cluster.maxResources())) return Advice.none("Autoscaling is not enabled"); + if (cluster.minResources().equals(cluster.maxResources())) + return Advice.none(Status.unavailable, "Autoscaling is not enabled"); return autoscale(application, cluster, clusterNodes, Limits.of(cluster)); } @@ -65,17 +68,20 @@ public class Autoscaler { nodeRepository.clock()); if ( ! clusterIsStable(clusterNodes, nodeRepository)) - return Advice.none("Cluster change in progress"); + return Advice.none(Status.waiting, "Cluster change in progress"); if (scaledIn(clusterModel.scalingDuration(), cluster)) - return Advice.dontScale("Won't autoscale now: Less than " + clusterModel.scalingDuration() + " since last resource change"); + return Advice.dontScale(Status.waiting, + "Won't autoscale now: Less than " + clusterModel.scalingDuration() + " since last resource change"); if (clusterModel.nodeTimeseries().measurementsPerNode() < minimumMeasurementsPerNode(clusterModel.scalingDuration())) - return Advice.none("Collecting more data before making new scaling decisions: Need to measure for " + + return Advice.none(Status.waiting, + "Collecting more data before making new scaling decisions: Need to measure for " + clusterModel.scalingDuration() + " since the last resource change completed"); if (clusterModel.nodeTimeseries().nodesMeasured() != clusterNodes.size()) - return Advice.none("Collecting more data before making new scaling decisions: " + + return Advice.none(Status.waiting, + "Collecting more data before making new scaling decisions: " + "Have measurements from " + clusterModel.nodeTimeseries().nodesMeasured() + " nodes, but require from " + clusterNodes.size()); @@ -85,13 +91,18 @@ public class Autoscaler { Optional<AllocatableClusterResources> bestAllocation = allocationOptimizer.findBestAllocation(target, currentAllocation, clusterModel, limits); if (bestAllocation.isEmpty()) - return Advice.dontScale("No allocation improvements are possible within configured limits"); + return Advice.dontScale(Status.insufficient, "No allocations are possible within configured limits"); - if (similar(bestAllocation.get().realResources(), currentAllocation.realResources())) - return Advice.dontScale("Cluster is ideally scaled within configured limits"); + if (similar(bestAllocation.get().realResources(), currentAllocation.realResources())) { + if (bestAllocation.get().fulfilment() < 1) + return Advice.dontScale(Status.insufficient, "Configured limits prevents better scaling of this cluster"); + else + return Advice.dontScale(Status.ideal, "Cluster is ideally scaled"); + } if (isDownscaling(bestAllocation.get(), currentAllocation) && scaledIn(clusterModel.scalingDuration().multipliedBy(3), cluster)) - return Advice.dontScale("Waiting " + clusterModel.scalingDuration().multipliedBy(3) + + return Advice.dontScale(Status.waiting, + "Waiting " + clusterModel.scalingDuration().multipliedBy(3) + " since the last change before reducing resources"); return Advice.scaleTo(bestAllocation.get().advertisedResources()); @@ -154,9 +165,9 @@ public class Autoscaler { private final boolean present; private final Optional<ClusterResources> target; - private final String reason; + private final AutoscalingStatus reason; - private Advice(Optional<ClusterResources> target, boolean present, String reason) { + private Advice(Optional<ClusterResources> target, boolean present, AutoscalingStatus reason) { this.target = target; this.present = present; this.reason = Objects.requireNonNull(reason); @@ -175,12 +186,20 @@ public class Autoscaler { public boolean isPresent() { return present; } /** The reason for this advice */ - public String reason() { return reason; } + public AutoscalingStatus reason() { return reason; } + + private static Advice none(Status status, String description) { + return new Advice(Optional.empty(), false, new AutoscalingStatus(status, description)); + } + + private static Advice dontScale(Status status, String description) { + return new Advice(Optional.empty(), true, new AutoscalingStatus(status, description)); + } - 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, "Scheduled scaling to " + target + " due to load changes"); + return new Advice(Optional.of(target), true, + new AutoscalingStatus(AutoscalingStatus.Status.rescaling, + "Scheduled scaling to " + target + " due to load changes")); } @Override diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Limits.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Limits.java index 3dcaec63448..cafea4b0eaf 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Limits.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Limits.java @@ -60,12 +60,11 @@ public class Limits { } public Limits fullySpecified(ClusterSpec.Type type, NodeRepository nodeRepository) { - if (this.isEmpty()) return this; - CapacityPolicies capacityPolicies = new CapacityPolicies(nodeRepository); - var specifiedMin = min.with(min.nodeResources().isUnspecified() ? - capacityPolicies.defaultNodeResources(type) : min.nodeResources()); - var specifiedMax = max.with(max.nodeResources().isUnspecified() ? - capacityPolicies.defaultNodeResources(type) : max.nodeResources()); + if (this.isEmpty()) throw new IllegalStateException("Unspecified limits can not be made fully specified"); + + var defaultResources = new CapacityPolicies(nodeRepository).defaultNodeResources(type); + var specifiedMin = min.nodeResources().isUnspecified() ? min.with(defaultResources) : min; + var specifiedMax = max.nodeResources().isUnspecified() ? max.with(defaultResources) : max; return new Limits(specifiedMin, specifiedMax); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java index 9ac1ca2b4c1..24160c19dfa 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java @@ -41,9 +41,9 @@ public abstract class ApplicationMaintainer extends NodeRepositoryMaintainer { } @Override - protected final boolean maintain() { + protected final double maintain() { applicationsNeedingMaintenance().forEach(this::deploy); - return true; + return 1.0; } /** Returns the number of deployments that are pending execution */ 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 7da6e0d3ebe..3914a0c9f07 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 @@ -14,16 +14,13 @@ import com.yahoo.vespa.hosted.provision.applications.Applications; import com.yahoo.vespa.hosted.provision.applications.Cluster; import com.yahoo.vespa.hosted.provision.autoscale.AllocatableClusterResources; import com.yahoo.vespa.hosted.provision.autoscale.Autoscaler; -import com.yahoo.vespa.hosted.provision.autoscale.MetricsDb; import com.yahoo.vespa.hosted.provision.autoscale.NodeMetricSnapshot; -import com.yahoo.vespa.hosted.provision.autoscale.NodeTimeseries; import com.yahoo.vespa.hosted.provision.node.History; import java.time.Duration; import java.time.Instant; import java.util.Map; import java.util.Optional; -import java.util.Set; /** * Maintainer making automatic scaling decisions @@ -47,14 +44,13 @@ public class AutoscalingMaintainer extends NodeRepositoryMaintainer { } @Override - protected boolean maintain() { - if ( ! nodeRepository().nodes().isWorking()) return false; + protected double maintain() { + if ( ! nodeRepository().nodes().isWorking()) return 0.0; - boolean success = true; - if ( ! nodeRepository().zone().environment().isAnyOf(Environment.dev, Environment.prod)) return success; + if ( ! nodeRepository().zone().environment().isAnyOf(Environment.dev, Environment.prod)) return 1.0; activeNodesByApplication().forEach(this::autoscale); - return success; + return 1.0; } private void autoscale(ApplicationId application, NodeList applicationNodes) { @@ -81,7 +77,7 @@ public class AutoscalingMaintainer extends NodeRepositoryMaintainer { // 1. Update cluster info updatedCluster = updateCompletion(cluster.get(), clusterNodes) - .withAutoscalingStatus(advice.reason()) + .with(advice.reason()) .withTarget(advice.target()); applications().put(application.get().with(updatedCluster), lock); if (advice.isPresent() && advice.target().isPresent() && !cluster.get().targetResources().equals(advice.target())) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java index 4224667a726..0eb2038e233 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java @@ -12,13 +12,10 @@ import com.yahoo.config.provision.OutOfCapacityException; import com.yahoo.jdisc.Metric; import com.yahoo.lang.MutableInteger; import com.yahoo.transaction.Mutex; -import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagSource; -import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.JacksonFlag; import com.yahoo.vespa.flags.ListFlag; import com.yahoo.vespa.flags.PermanentFlags; -import com.yahoo.vespa.flags.StringFlag; import com.yahoo.vespa.flags.custom.ClusterCapacity; import com.yahoo.vespa.flags.custom.SharedHost; import com.yahoo.vespa.hosted.provision.LockedNodeList; @@ -64,7 +61,6 @@ public class DynamicProvisioningMaintainer extends NodeRepositoryMaintainer { private final HostProvisioner hostProvisioner; private final ListFlag<ClusterCapacity> preprovisionCapacityFlag; private final JacksonFlag<SharedHost> sharedHostFlag; - private final StringFlag allocateOsRequirement; DynamicProvisioningMaintainer(NodeRepository nodeRepository, Duration interval, @@ -75,17 +71,16 @@ public class DynamicProvisioningMaintainer extends NodeRepositoryMaintainer { this.hostProvisioner = hostProvisioner; this.preprovisionCapacityFlag = PermanentFlags.PREPROVISION_CAPACITY.bindTo(flagSource); this.sharedHostFlag = PermanentFlags.SHARED_HOST.bindTo(flagSource); - this.allocateOsRequirement = Flags.ALLOCATE_OS_REQUIREMENT.bindTo(flagSource); } @Override - protected boolean maintain() { + protected double maintain() { try (Mutex lock = nodeRepository().nodes().lockUnallocated()) { NodeList nodes = nodeRepository().nodes().list(); resumeProvisioning(nodes, lock); convergeToCapacity(nodes); } - return true; + return 1.0; } /** Resume provisioning of already provisioned hosts and their children */ @@ -301,12 +296,9 @@ public class DynamicProvisioningMaintainer extends NodeRepositoryMaintainer { NodeSpec nodeSpec = NodeSpec.from(clusterCapacity.count(), nodeResources, false, true); int wantedGroups = 1; - String allocateOsRequirement = this.allocateOsRequirement - .with(FetchVector.Dimension.APPLICATION_ID, ApplicationId.defaultId().serializedForm()) - .value(); NodePrioritizer prioritizer = new NodePrioritizer(nodeList, applicationId, clusterSpec, nodeSpec, wantedGroups, true, nodeRepository().nameResolver(), nodeRepository().resourcesCalculator(), - nodeRepository().spareCount(), allocateOsRequirement); + nodeRepository().spareCount()); List<NodeCandidate> nodeCandidates = prioritizer.collect(List.of()); MutableInteger index = new MutableInteger(0); return nodeCandidates diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Expirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Expirer.java index 2443a12d198..25108425e6e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Expirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Expirer.java @@ -40,7 +40,7 @@ public abstract class Expirer extends NodeRepositoryMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { NodeList expired = nodeRepository().nodes().list(fromState).matching(this::isExpired); if ( ! expired.isEmpty()) { @@ -49,7 +49,7 @@ public abstract class Expirer extends NodeRepositoryMaintainer { } metric.add("expired." + fromState, expired.size(), null); - return true; + return 1.0; } protected boolean isExpired(Node node) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java index e98da35aa6a..7505ce42668 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java @@ -66,7 +66,7 @@ public class FailedExpirer extends NodeRepositoryMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { List<Node> remainingNodes = new ArrayList<>(nodeRepository.nodes().list(Node.State.failed) .nodeType(NodeType.tenant, NodeType.host) .asList()); @@ -78,7 +78,7 @@ public class FailedExpirer extends NodeRepositoryMaintainer { recycleIf(remainingNodes, node -> node.allocation().get().membership().cluster().isStateful() && node.history().hasEventBefore(History.Event.Type.failed, clock().instant().minus(statefulExpiry))); - return true; + return 1.0; } /** Recycle the nodes matching condition, and remove those nodes from the nodes list. */ diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypter.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypter.java index caf20463f60..80f74a011c0 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypter.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypter.java @@ -43,7 +43,7 @@ public class HostEncrypter extends NodeRepositoryMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { Instant now = nodeRepository().clock().instant(); NodeList allNodes = nodeRepository().nodes().list(); for (var nodeType : NodeType.values()) { @@ -51,7 +51,7 @@ public class HostEncrypter extends NodeRepositoryMaintainer { if (upgradingVespa(allNodes, nodeType)) continue; unencryptedHosts(allNodes, nodeType).forEach(host -> encrypt(host, now)); } - return true; + return 1.0; } /** Returns whether any node of given type is currently upgrading its Vespa version */ diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/InfrastructureProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/InfrastructureProvisioner.java index e317333135c..d9f5ea6a7a9 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/InfrastructureProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/InfrastructureProvisioner.java @@ -39,9 +39,9 @@ public class InfrastructureProvisioner extends NodeRepositoryMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { infraDeployer.activateAllSupportedInfraApplications(false); - return true; + return 1.0; } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java index 10069fd1a18..ac9d8d6671a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.provision.maintenance; import com.google.common.collect.Sets; import com.yahoo.jdisc.Metric; +import com.yahoo.lang.MutableInteger; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.lb.LoadBalancer; @@ -54,9 +55,9 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { expireReserved(); - return removeInactive() & pruneReals(); + return ( removeInactive() + pruneReals() ) / 2; } /** Move reserved load balancer that have expired to inactive */ @@ -68,7 +69,8 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { } /** Deprovision inactive load balancers that have expired */ - private boolean removeInactive() { + private double removeInactive() { + MutableInteger attempts = new MutableInteger(0); var failed = new ArrayList<LoadBalancerId>(); var lastException = new AtomicReference<Exception>(); var expiry = nodeRepository().clock().instant().minus(inactiveExpiry); @@ -76,6 +78,7 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { lb.changedAt().isBefore(expiry) && allocatedNodes(lb.id()).isEmpty(), lb -> { try { + attempts.add(1); log.log(Level.INFO, () -> "Removing expired inactive load balancer " + lb.id()); service.remove(lb.id().application(), lb.id().cluster()); db.removeLoadBalancer(lb.id()); @@ -92,11 +95,12 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { .collect(Collectors.joining(", ")), interval())); } - return lastException.get() == null; + return asSuccessFactor(attempts.get(), failed.size()); } /** Remove reals from inactive load balancers */ - private boolean pruneReals() { + private double pruneReals() { + var attempts = new MutableInteger(0); var failed = new ArrayList<LoadBalancerId>(); var lastException = new AtomicReference<Exception>(); patchLoadBalancers(lb -> lb.state() == State.inactive, lb -> { @@ -107,6 +111,7 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { reals.removeIf(real -> !allocatedNodes.contains(real.hostname().value())); if (reals.equals(lb.instance().get().reals())) return; // Nothing to remove try { + attempts.add(1); LOG.log(Level.INFO, () -> "Removing reals from inactive load balancer " + lb.id() + ": " + Sets.difference(lb.instance().get().reals(), reals)); service.create(new LoadBalancerSpec(lb.id().application(), lb.id().cluster(), reals), true); db.writeLoadBalancer(lb.with(lb.instance().map(instance -> instance.withReals(reals)))); @@ -124,7 +129,7 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { interval()), lastException.get()); } - return lastException.get() == null; + return asSuccessFactor(attempts.get(), failed.size()); } /** Patch load balancers matching given filter, while holding lock */ diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java index 85437b3e78a..3990c5099eb 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java @@ -65,7 +65,7 @@ public class MetricsReporter extends NodeRepositoryMaintainer { } @Override - public boolean maintain() { + public double maintain() { NodeList nodes = nodeRepository().nodes().list(); ServiceModel serviceModel = serviceMonitor.getServiceModelSnapshot(); @@ -80,7 +80,7 @@ public class MetricsReporter extends NodeRepositoryMaintainer { updateRepairTicketMetrics(nodes); updateAllocationMetrics(nodes); updateExclusiveSwitchMetrics(nodes); - return true; + return 1.0; } private void updateAllocationMetrics(NodeList nodes) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java index effa41dc69f..f16459ee8b9 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java @@ -72,17 +72,21 @@ public class NodeFailer extends NodeRepositoryMaintainer { } @Override - protected boolean maintain() { - if ( ! nodeRepository().nodes().isWorking()) return false; + protected double maintain() { + if ( ! nodeRepository().nodes().isWorking()) return 0.0; + int attempts = 0; + int failures = 0; int throttledHostFailures = 0; int throttledNodeFailures = 0; // Ready nodes try (Mutex lock = nodeRepository().nodes().lockUnallocated()) { for (Map.Entry<Node, String> entry : getReadyNodesByFailureReason().entrySet()) { + attempts++; Node node = entry.getKey(); if (throttle(node)) { + failures++; if (node.type().isHost()) throttledHostFailures++; else @@ -96,10 +100,12 @@ public class NodeFailer extends NodeRepositoryMaintainer { // Active nodes for (Map.Entry<Node, String> entry : getActiveNodesByFailureReason().entrySet()) { + attempts++; Node node = entry.getKey(); if (!failAllowedFor(node.type())) continue; if (throttle(node)) { + failures++; if (node.type().isHost()) throttledHostFailures++; else @@ -116,11 +122,15 @@ public class NodeFailer extends NodeRepositoryMaintainer { if ( ! activeNodes.childrenOf(host).isEmpty()) continue; Optional<NodeMutex> locked = Optional.empty(); try { + attempts++; locked = nodeRepository().nodes().lockAndGet(host); if (locked.isEmpty()) continue; nodeRepository().nodes().fail(List.of(locked.get().node()), Agent.NodeFailer, "Host should be failed and have no tenant nodes"); } + catch (Exception e) { + failures++; + } finally { locked.ifPresent(NodeMutex::close); } @@ -130,7 +140,7 @@ public class NodeFailer extends NodeRepositoryMaintainer { metric.set(throttlingActiveMetric, throttlingActive, null); metric.set(throttledHostFailuresMetric, throttledHostFailures, null); metric.set(throttledNodeFailuresMetric, throttledNodeFailures, null); - return throttlingActive == 0; + return asSuccessFactor(attempts, failures); } private Map<Node, String> getReadyNodesByFailureReason() { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeHealthTracker.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeHealthTracker.java index fe2fb5229f9..37969a30b81 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeHealthTracker.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeHealthTracker.java @@ -5,6 +5,7 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationLockException; import com.yahoo.config.provision.HostLivenessTracker; import com.yahoo.jdisc.Metric; +import com.yahoo.lang.MutableInteger; import com.yahoo.transaction.Mutex; import com.yahoo.vespa.applicationmodel.ServiceInstance; import com.yahoo.vespa.applicationmodel.ServiceStatus; @@ -48,13 +49,11 @@ public class NodeHealthTracker extends NodeRepositoryMaintainer { } @Override - protected boolean maintain() { - updateReadyNodeLivenessEvents(); - updateActiveNodeDownState(); - return true; + protected double maintain() { + return ( updateReadyNodeLivenessEvents() + updateActiveNodeDownState() ) / 2; } - private void updateReadyNodeLivenessEvents() { + private double updateReadyNodeLivenessEvents() { // Update node last request events through ZooKeeper to collect request to all config servers. // We do this here ("lazily") to avoid writing to zk for each config request. try (Mutex lock = nodeRepository().nodes().lockUnallocated()) { @@ -69,13 +68,16 @@ public class NodeHealthTracker extends NodeRepositoryMaintainer { } } } + return 1.0; } /** * If the node is down (see {@link #allDown}), and there is no "down" history record, we add it. * Otherwise we remove any "down" history record. */ - private void updateActiveNodeDownState() { + private double updateActiveNodeDownState() { + var attempts = new MutableInteger(0); + var failures = new MutableInteger(0); NodeList activeNodes = nodeRepository().nodes().list(Node.State.active); serviceMonitor.getServiceModelSnapshot().getServiceInstancesByHostName().forEach((hostname, serviceInstances) -> { Optional<Node> node = activeNodes.node(hostname.toString()); @@ -90,6 +92,7 @@ public class NodeHealthTracker extends NodeRepositoryMaintainer { try (var lock = nodeRepository().nodes().lock(owner)) { node = getNode(hostname.toString(), owner, lock); // Re-get inside lock if (node.isEmpty()) return; // Node disappeared or changed allocation + attempts.add(1); if (isDown) { recordAsDown(node.get(), lock); } else { @@ -98,8 +101,10 @@ public class NodeHealthTracker extends NodeRepositoryMaintainer { } catch (ApplicationLockException e) { // Fine, carry on with other nodes. We'll try updating this one in the next run log.log(Level.WARNING, "Could not lock " + owner + ": " + Exceptions.toMessageString(e)); + failures.add(1); } }); + return asSuccessFactor(attempts.get(), failures.get()); } /** diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java index 1ea4577f7fe..d671900d08c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java @@ -33,19 +33,21 @@ public class NodeMetricsDbMaintainer extends NodeRepositoryMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { + int attempts = 0; + var failures = new MutableInteger(0); try { - var warnings = new MutableInteger(0); Set<ApplicationId> applications = activeNodesByApplication().keySet(); - if (applications.isEmpty()) return true; + if (applications.isEmpty()) return 1.0; long pauseMs = interval().toMillis() / applications.size() - 1; // spread requests over interval int done = 0; for (ApplicationId application : applications) { + attempts++; metricsFetcher.fetchMetrics(application) .whenComplete((metricsResponse, exception) -> handleResponse(metricsResponse, exception, - warnings, + failures, application)); if (++done < applications.size()) Thread.sleep(pauseMs); @@ -56,23 +58,22 @@ public class NodeMetricsDbMaintainer extends NodeRepositoryMaintainer { nodeRepository().metricsDb().gc(); - // Suppress failures for manual zones for now to avoid noise - return nodeRepository().zone().environment().isManuallyDeployed() || warnings.get() == 0; + return asSuccessFactor(attempts, failures.get()); } catch (InterruptedException e) { - return false; + return asSuccessFactor(attempts, failures.get()); } } private void handleResponse(MetricsResponse response, Throwable exception, - MutableInteger warnings, + MutableInteger failures, ApplicationId application) { if (exception != null) { - if (warnings.get() < maxWarningsPerInvocation) + if (failures.get() < maxWarningsPerInvocation) log.log(Level.WARNING, "Could not update metrics for " + application + ": " + Exceptions.toMessageString(exception)); - warnings.add(1); + failures.add(1); } else if (response != null) { nodeRepository().metricsDb().addNodeMetrics(response.nodeMetrics()); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRebooter.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRebooter.java index 6ee657beadd..c282fcdb7fc 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRebooter.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRebooter.java @@ -37,7 +37,7 @@ public class NodeRebooter extends NodeRepositoryMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { // Reboot candidates: Nodes in long-term states, where we know we can safely orchestrate a reboot List<Node> nodesToReboot = nodeRepository().nodes().list(Node.State.active, Node.State.ready).stream() .filter(node -> node.type().isHost()) @@ -46,7 +46,7 @@ public class NodeRebooter extends NodeRepositoryMaintainer { if (!nodesToReboot.isEmpty()) nodeRepository().nodes().reboot(NodeListFilter.from(nodesToReboot)); - return true; + return 1.0; } private boolean shouldReboot(Node node) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintainer.java index 0a1f6961f9f..fe5cd419b31 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintainer.java @@ -25,7 +25,7 @@ public abstract class NodeRepositoryMaintainer extends Maintainer { public NodeRepositoryMaintainer(NodeRepository nodeRepository, Duration interval, Metric metric) { super(null, interval, nodeRepository.clock().instant(), nodeRepository.jobControl(), - jobMetrics(metric), nodeRepository.database().cluster(), true); + new NodeRepositoryJobMetrics(metric), nodeRepository.database().cluster(), true); this.nodeRepository = nodeRepository; } @@ -48,10 +48,20 @@ public abstract class NodeRepositoryMaintainer extends Maintainer { .groupingBy(node -> node.allocation().get().owner()); } - private static JobMetrics jobMetrics(Metric metric) { - return new JobMetrics((job, consecutiveFailures) -> { + private static class NodeRepositoryJobMetrics extends JobMetrics { + + private final Metric metric; + + public NodeRepositoryJobMetrics(Metric metric) { + this.metric = metric; + } + + @Override + protected void recordCompletion(String job, Long consecutiveFailures, double successFactor) { metric.set("maintenance.consecutiveFailures", consecutiveFailures, metric.createContext(Map.of("job", job))); - }); + metric.set("maintenance.successFactor", successFactor, metric.createContext(Map.of("job", job))); + } + } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivator.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivator.java index 4eba15307cb..749603a373d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivator.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivator.java @@ -23,13 +23,13 @@ public class OsUpgradeActivator extends NodeRepositoryMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { for (var nodeType : NodeType.values()) { if (!nodeType.isHost()) continue; boolean resume = canUpgradeOsOf(nodeType); nodeRepository().osVersions().resumeUpgradeOf(nodeType, resume); } - return true; + return 1.0; } /** Returns whether to allow OS upgrade of nodes of given type */ diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Rebalancer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Rebalancer.java index 1543506a78e..7bb748c92c9 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Rebalancer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Rebalancer.java @@ -33,19 +33,18 @@ public class Rebalancer extends NodeMover<Rebalancer.Move> { } @Override - protected boolean maintain() { - if ( ! nodeRepository().nodes().isWorking()) return false; + protected double maintain() { + if ( ! nodeRepository().nodes().isWorking()) return 0.0; - boolean success = true; - if (nodeRepository().zone().getCloud().dynamicProvisioning()) return success; // Rebalancing not necessary - if (nodeRepository().zone().environment().isTest()) return success; // Short lived deployments; no need to rebalance + if (nodeRepository().zone().getCloud().dynamicProvisioning()) return 1.0; // Rebalancing not necessary + if (nodeRepository().zone().environment().isTest()) return 1.0; // Short lived deployments; no need to rebalance // Work with an unlocked snapshot as this can take a long time and full consistency is not needed NodeList allNodes = nodeRepository().nodes().list(); updateSkewMetric(allNodes); - if ( ! zoneIsStable(allNodes)) return success; + if ( ! zoneIsStable(allNodes)) return 1.0; findBestMove(allNodes).execute(true, Agent.Rebalancer, deployer, metric, nodeRepository()); - return success; + return 1.0; } @Override diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java index f72daf1bc2b..3f5893b368a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java @@ -48,7 +48,10 @@ public class RetiredExpirer extends NodeRepositoryMaintainer { } @Override - protected boolean maintain() { + protected double maintain() { + int attempts = 0; + int successes = 0; + NodeList activeNodes = nodeRepository().nodes().list(Node.State.active); Map<ApplicationId, NodeList> retiredNodesByApplication = activeNodes.retired().groupingBy(node -> node.allocation().get().owner()); for (Map.Entry<ApplicationId, NodeList> entry : retiredNodesByApplication.entrySet()) { @@ -57,17 +60,19 @@ public class RetiredExpirer extends NodeRepositoryMaintainer { List<Node> nodesToRemove = retiredNodes.stream().filter(n -> canRemove(n, activeNodes)).collect(Collectors.toList()); if (nodesToRemove.isEmpty()) continue; + attempts++; try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, metric, nodeRepository())) { if ( ! deployment.isValid()) continue; nodeRepository().nodes().setRemovable(application, nodesToRemove); boolean success = deployment.activate().isPresent(); - if ( ! success) return success; + if ( ! success) continue; String nodeList = nodesToRemove.stream().map(Node::hostname).collect(Collectors.joining(", ")); log.info("Redeployed " + application + " to deactivate retired nodes: " + nodeList); + successes++; } } - return true; + return attempts == 0 ? 1.0 : ((double)successes / attempts); } /** diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainer.java index c217580872b..888f06a5004 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainer.java @@ -36,13 +36,16 @@ public class ScalingSuggestionsMaintainer extends NodeRepositoryMaintainer { } @Override - protected boolean maintain() { - if ( ! nodeRepository().zone().environment().isProduction()) return true; + protected double maintain() { + if ( ! nodeRepository().zone().environment().isProduction()) return 1.0; + int attempts = 0; int successes = 0; - for (var application : activeNodesByApplication().entrySet()) + for (var application : activeNodesByApplication().entrySet()) { + attempts++; successes += suggest(application.getKey(), application.getValue()); - return successes > 0; + } + return attempts == 0 ? 1.0 : ((double)successes / attempts); } private int suggest(ApplicationId application, NodeList applicationNodes) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainer.java index 0307ae13b24..0589571e9d8 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainer.java @@ -66,12 +66,11 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { } @Override - protected boolean maintain() { - if ( ! nodeRepository().nodes().isWorking()) return false; + protected double maintain() { + if ( ! nodeRepository().nodes().isWorking()) return 0.0; - boolean success = true; // Don't need to maintain spare capacity in dynamically provisioned zones; can provision more on demand. - if (nodeRepository().zone().getCloud().dynamicProvisioning()) return success; + if (nodeRepository().zone().getCloud().dynamicProvisioning()) return 1.0; NodeList allNodes = nodeRepository().nodes().list(); CapacityChecker capacityChecker = new CapacityChecker(allNodes); @@ -80,6 +79,7 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { metric.set("overcommittedHosts", overcommittedHosts.size(), null); retireOvercommitedHosts(allNodes, overcommittedHosts); + boolean success = true; Optional<CapacityChecker.HostFailurePath> failurePath = capacityChecker.worstCaseHostLossLeadingToFailure(); if (failurePath.isPresent()) { int spareHostCapacity = failurePath.get().hostsCausingFailure.size() - 1; @@ -96,7 +96,7 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { } metric.set("spareHostCapacity", spareHostCapacity, null); } - return success; + return success ? 1.0 : 0.0; } private boolean execute(List<Move> mitigation, CapacityChecker.HostFailurePath failurePath) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancer.java index cfab980570d..44890f2f5af 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancer.java @@ -33,14 +33,14 @@ public class SwitchRebalancer extends NodeMover<Move> { } @Override - protected boolean maintain() { - if (!nodeRepository().nodes().isWorking()) return false; - if (!nodeRepository().zone().environment().isProduction()) return true; + protected double maintain() { + if (!nodeRepository().nodes().isWorking()) return 0.0; + if (!nodeRepository().zone().environment().isProduction()) return 1.0; NodeList allNodes = nodeRepository().nodes().list(); // Lockless as strong consistency is not needed - if (!zoneIsStable(allNodes)) return true; + if (!zoneIsStable(allNodes)) return 1.0; findBestMove(allNodes).execute(false, Agent.SwitchRebalancer, deployer, metric, nodeRepository()); - return true; + return 1.0; } @Override 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 c8b928779b9..e3fb3379da1 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 @@ -10,6 +10,7 @@ import com.yahoo.slime.ObjectTraverser; import com.yahoo.slime.Slime; import com.yahoo.slime.SlimeUtils; import com.yahoo.vespa.hosted.provision.applications.Application; +import com.yahoo.vespa.hosted.provision.applications.AutoscalingStatus; import com.yahoo.vespa.hosted.provision.applications.Cluster; import com.yahoo.vespa.hosted.provision.applications.ScalingEvent; import com.yahoo.vespa.hosted.provision.applications.Status; @@ -55,6 +56,8 @@ public class ApplicationSerializer { private static final String nodeResourcesKey = "resources"; private static final String scalingEventsKey = "scalingEvents"; private static final String autoscalingStatusKey = "autoscalingStatus"; + private static final String autoscalingStatusObjectKey = "autoscalingStatusObject"; + private static final String descriptionKey = "description"; private static final String fromKey = "from"; private static final String toKey = "to"; private static final String generationKey = "generation"; @@ -118,7 +121,8 @@ public class ApplicationSerializer { cluster.suggestedResources().ifPresent(suggested -> toSlime(suggested, clusterObject.setObject(suggestedKey))); cluster.targetResources().ifPresent(target -> toSlime(target, clusterObject.setObject(targetResourcesKey))); scalingEventsToSlime(cluster.scalingEvents(), clusterObject.setArray(scalingEventsKey)); - clusterObject.setString(autoscalingStatusKey, cluster.autoscalingStatus()); + clusterObject.setString(autoscalingStatusKey, cluster.autoscalingStatus().description()); // TODO: Remove after June 2021 + toSlime(cluster.autoscalingStatus(), clusterObject.setObject(autoscalingStatusObjectKey)); } private static Cluster clusterFromSlime(String id, Inspector clusterObject) { @@ -129,7 +133,7 @@ public class ApplicationSerializer { optionalSuggestionFromSlime(clusterObject.field(suggestedKey)), optionalClusterResourcesFromSlime(clusterObject.field(targetResourcesKey)), scalingEventsFromSlime(clusterObject.field(scalingEventsKey)), - clusterObject.field(autoscalingStatusKey).asString()); + autoscalingStatusFromSlime(clusterObject.field(autoscalingStatusObjectKey), clusterObject)); } private static void toSlime(Cluster.Suggestion suggestion, Cursor suggestionObject) { @@ -188,6 +192,42 @@ public class ApplicationSerializer { optionalInstant(inspector.field(completionKey))); } + private static void toSlime(AutoscalingStatus status, Cursor object) { + object.setString(statusKey, toAutoscalingStatusCode(status.status())); + object.setString(descriptionKey, status.description()); + } + + private static AutoscalingStatus autoscalingStatusFromSlime(Inspector object, Inspector parent) { + // TODO: Remove this clause after June 2021 + if ( ! object.valid()) return new AutoscalingStatus(AutoscalingStatus.Status.unavailable, + parent.field(autoscalingStatusKey).asString()); + + return new AutoscalingStatus(fromAutoscalingStatusCode(object.field(statusKey).asString()), + object.field(descriptionKey).asString()); + } + + private static String toAutoscalingStatusCode(AutoscalingStatus.Status status) { + switch (status) { + case unavailable : return "unavailable"; + case waiting : return "waiting"; + case ideal : return "ideal"; + case insufficient : return "insufficient"; + case rescaling : return "rescaling"; + default : throw new IllegalArgumentException("Unknown autoscaling status " + status); + } + } + + private static AutoscalingStatus.Status fromAutoscalingStatusCode(String code) { + switch (code) { + case "unavailable" : return AutoscalingStatus.Status.unavailable; + case "waiting" : return AutoscalingStatus.Status.waiting; + case "ideal" : return AutoscalingStatus.Status.ideal; + case "insufficient" : return AutoscalingStatus.Status.insufficient; + case "rescaling" : return AutoscalingStatus.Status.rescaling; + default : throw new IllegalArgumentException("Unknown autoscaling status '" + code + "'"); + } + } + private static Optional<Instant> optionalInstant(Inspector inspector) { return inspector.valid() ? Optional.of(Instant.ofEpochMilli(inspector.asLong())) : Optional.empty(); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java index d1f881f8b7a..fb4799be29c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java @@ -11,7 +11,6 @@ import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.NodeFlavors; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.TenantName; -import com.yahoo.config.provision.Zone; import com.yahoo.path.Path; import com.yahoo.transaction.NestedTransaction; import com.yahoo.transaction.Transaction; @@ -77,13 +76,10 @@ public class CuratorDatabaseClient { private final NodeSerializer nodeSerializer; private final CuratorDatabase db; private final Clock clock; - private final Zone zone; private final CuratorCounter provisionIndexCounter; - public CuratorDatabaseClient(NodeFlavors flavors, Curator curator, Clock clock, Zone zone, boolean useCache, - long nodeCacheSize) { + public CuratorDatabaseClient(NodeFlavors flavors, Curator curator, Clock clock, boolean useCache, long nodeCacheSize) { this.nodeSerializer = new NodeSerializer(flavors, nodeCacheSize); - this.zone = zone; this.db = new CuratorDatabase(curator, root, useCache); this.clock = clock; this.provisionIndexCounter = new CuratorCounter(curator, root.append("provisionIndexCounter").getAbsolute()); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java index 6d06dc31a42..cb965e87739 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java @@ -13,6 +13,7 @@ import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeMutex; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.applications.Application; +import com.yahoo.vespa.hosted.provision.applications.AutoscalingStatus; import com.yahoo.vespa.hosted.provision.applications.ScalingEvent; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.Allocation; @@ -117,7 +118,8 @@ class Activator { } if (cluster.targetResources().isPresent() && cluster.targetResources().get().justNumbers().equals(currentResources.justNumbers())) { - cluster = cluster.withAutoscalingStatus("Cluster is ideally scaled within configured limits"); + cluster = cluster.with(new AutoscalingStatus(AutoscalingStatus.Status.ideal, + "Cluster is ideally scaled within configured limits")); } if (cluster != modified.cluster(clusterEntry.getKey()).get()) modified = modified.with(cluster); 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 def992a264b..5d45bed19e8 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 @@ -7,10 +7,6 @@ import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.OutOfCapacityException; import com.yahoo.transaction.Mutex; -import com.yahoo.vespa.flags.FetchVector; -import com.yahoo.vespa.flags.FlagSource; -import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.flags.StringFlag; import com.yahoo.vespa.hosted.provision.LockedNodeList; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; @@ -33,14 +29,10 @@ public class GroupPreparer { private final NodeRepository nodeRepository; private final Optional<HostProvisioner> hostProvisioner; - private final StringFlag allocateOsRequirementFlag; - public GroupPreparer(NodeRepository nodeRepository, - Optional<HostProvisioner> hostProvisioner, - FlagSource flagSource) { + public GroupPreparer(NodeRepository nodeRepository, Optional<HostProvisioner> hostProvisioner) { this.nodeRepository = nodeRepository; this.hostProvisioner = hostProvisioner; - this.allocateOsRequirementFlag = Flags.ALLOCATE_OS_REQUIREMENT.bindTo(flagSource); } /** @@ -60,17 +52,11 @@ public class GroupPreparer { // active config model which is changed on activate public List<Node> prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, List<Node> surplusActiveNodes, NodeIndices indices, int wantedGroups) { - - String allocateOsRequirement = allocateOsRequirementFlag - .with(FetchVector.Dimension.APPLICATION_ID, application.serializedForm()) - .value(); - // Try preparing in memory without global unallocated lock. Most of the time there should be no changes and we // can return nodes previously allocated. { NodeAllocation probeAllocation = prepareAllocation(application, cluster, requestedNodes, surplusActiveNodes, - indices::probeNext, wantedGroups, PROBE_LOCK, - allocateOsRequirement); + indices::probeNext, wantedGroups, PROBE_LOCK); if (probeAllocation.fulfilledAndNoChanges()) { List<Node> acceptedNodes = probeAllocation.finalNodes(); surplusActiveNodes.removeAll(acceptedNodes); @@ -85,21 +71,18 @@ public class GroupPreparer { Mutex allocationLock = nodeRepository.nodes().lockUnallocated()) { NodeAllocation allocation = prepareAllocation(application, cluster, requestedNodes, surplusActiveNodes, - indices::next, wantedGroups, allocationLock, - allocateOsRequirement); + indices::next, wantedGroups, allocationLock); NodeType hostType = allocation.nodeType().hostType(); if (canProvisionDynamically(hostType)) { HostSharing sharing = hostSharing(requestedNodes, hostType); + Version osVersion = nodeRepository.osVersions().targetFor(hostType).orElse(Version.emptyVersion); List<ProvisionedHost> provisionedHosts = allocation.hostDeficit() - .map(deficit -> - hostProvisioner.get().provisionHosts(allocation.provisionIndices(deficit.count()), - hostType, - deficit.resources(), - application, - decideOsVersion(allocateOsRequirement, hostType), - sharing) - ) - .orElseGet(List::of); + .map(deficit -> hostProvisioner.get().provisionHosts(allocation.provisionIndices(deficit.count()), + hostType, + deficit.resources(), + application, osVersion, + sharing)) + .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 @@ -131,7 +114,7 @@ public class GroupPreparer { private NodeAllocation prepareAllocation(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, List<Node> surplusActiveNodes, Supplier<Integer> nextIndex, int wantedGroups, - Mutex allocationLock, String allocateOsRequirement) { + Mutex allocationLock) { LockedNodeList allNodes = nodeRepository.nodes().list(allocationLock); NodeAllocation allocation = new NodeAllocation(allNodes, application, cluster, requestedNodes, nextIndex, nodeRepository); NodePrioritizer prioritizer = new NodePrioritizer(allNodes, @@ -142,8 +125,7 @@ public class GroupPreparer { nodeRepository.zone().getCloud().dynamicProvisioning(), nodeRepository.nameResolver(), nodeRepository.resourcesCalculator(), - nodeRepository.spareCount(), - allocateOsRequirement); + nodeRepository.spareCount()); allocation.offer(prioritizer.collect(surplusActiveNodes)); return allocation; } @@ -161,13 +143,4 @@ public class GroupPreparer { return sharing; } - private Version decideOsVersion(String allocateOsRequirement, NodeType hostType) { - if (allocateOsRequirement.equals("rhel8")) - return new Version(8, Integer.MAX_VALUE /* always use latest 8 version */, 0); - else if (allocateOsRequirement.equals("rhel7")) - return new Version(7, Integer.MAX_VALUE /* always use latest 7 version */, 0); - else - return nodeRepository.osVersions().targetFor(hostType).orElse(Version.emptyVersion); - } - } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java index 8eca4ff2d95..695f0dd8659 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java @@ -1,7 +1,6 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.provisioning; -import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.NodeType; @@ -32,7 +31,6 @@ public class NodePrioritizer { private final List<NodeCandidate> nodes = new ArrayList<>(); private final LockedNodeList allNodes; - private final String allocateOsRequirement; private final HostCapacity capacity; private final NodeSpec requestedNodes; private final ApplicationId application; @@ -48,9 +46,8 @@ public class NodePrioritizer { public NodePrioritizer(LockedNodeList allNodes, ApplicationId application, ClusterSpec clusterSpec, NodeSpec nodeSpec, int wantedGroups, boolean dynamicProvisioning, NameResolver nameResolver, - HostResourcesCalculator hostResourcesCalculator, int spareCount, String allocateOsRequirement) { + HostResourcesCalculator hostResourcesCalculator, int spareCount) { this.allNodes = allNodes; - this.allocateOsRequirement = allocateOsRequirement; this.capacity = new HostCapacity(allNodes, hostResourcesCalculator); this.requestedNodes = nodeSpec; this.clusterSpec = clusterSpec; @@ -140,13 +137,6 @@ public class NodePrioritizer { if (host.reservedTo().isPresent() && !host.reservedTo().get().equals(application.tenant())) continue; if (host.reservedTo().isPresent() && application.instance().isTester()) continue; if (host.exclusiveTo().isPresent()) continue; // Never allocate new nodes to exclusive hosts - - if (host.status().osVersion().isBefore(new Version(8))) { - if (allocateOsRequirement.equals("rhel8")) continue; - } else { - if (allocateOsRequirement.equals("rhel7")) continue; - } - if (spareHosts.contains(host) && !canAllocateToSpareHosts) continue; if ( ! capacity.hasCapacity(host, requestedNodes.resources().get())) continue; if ( ! allNodes.childrenOf(host).owner(application).cluster(clusterSpec.id()).isEmpty()) continue; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java index 2cc7d7e2555..24d23f13bb5 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java @@ -71,7 +71,6 @@ public class NodeRepositoryProvisioner implements Provisioner { .map(lbService -> new LoadBalancerProvisioner(nodeRepository, lbService)); this.nodeResourceLimits = new NodeResourceLimits(nodeRepository); this.preparer = new Preparer(nodeRepository, - flagSource, provisionServiceProvider.getHostProvisioner(), loadBalancerProvisioner); this.activator = new Activator(nodeRepository, loadBalancerProvisioner); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java index 97f935d273b..3fa44d4c091 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -5,7 +5,6 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.OutOfCapacityException; -import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; @@ -28,11 +27,11 @@ class Preparer { private final GroupPreparer groupPreparer; private final Optional<LoadBalancerProvisioner> loadBalancerProvisioner; - public Preparer(NodeRepository nodeRepository, FlagSource flagSource, Optional<HostProvisioner> hostProvisioner, + public Preparer(NodeRepository nodeRepository, Optional<HostProvisioner> hostProvisioner, Optional<LoadBalancerProvisioner> loadBalancerProvisioner) { this.nodeRepository = nodeRepository; this.loadBalancerProvisioner = loadBalancerProvisioner; - this.groupPreparer = new GroupPreparer(nodeRepository, hostProvisioner, flagSource); + this.groupPreparer = new GroupPreparer(nodeRepository, hostProvisioner); } /** Prepare all required resources for the given application and cluster */ 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 9c6efd9efe6..e800f6c9c84 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 @@ -73,7 +73,8 @@ public class ApplicationSerializer { cluster.targetResources().ifPresent(target -> toSlime(target, clusterObject.setObject("target"))); clusterModel.ifPresent(model -> clusterUtilizationToSlime(model, clusterObject.setObject("utilization"))); scalingEventsToSlime(cluster.scalingEvents(), clusterObject.setArray("scalingEvents")); - clusterObject.setString("autoscalingStatus", cluster.autoscalingStatus()); + clusterObject.setString("autoscalingStatusCode", cluster.autoscalingStatus().status().name()); + clusterObject.setString("autoscalingStatus", cluster.autoscalingStatus().description()); clusterModel.ifPresent(model -> clusterObject.setLong("scalingDuration", model.scalingDuration().toMillis())); clusterModel.ifPresent(model -> clusterObject.setDouble("maxQueryGrowthRate", model.maxQueryGrowthRate())); clusterModel.ifPresent(model -> clusterObject.setDouble("currentQueryFractionOfMax", model.queryFractionOfMax())); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodeSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodeSerializer.java index b72d021e4f5..706c36b35ac 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodeSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodeSerializer.java @@ -1,9 +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.hosted.provision.restapi; -import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; -import com.yahoo.slime.Cursor; import com.yahoo.vespa.hosted.provision.Node; /** diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModelTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModelTest.java index c6575931c6d..af1bd2aa231 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModelTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModelTest.java @@ -7,6 +7,7 @@ import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.NodeResources; import com.yahoo.test.ManualClock; import com.yahoo.vespa.hosted.provision.applications.Application; +import com.yahoo.vespa.hosted.provision.applications.AutoscalingStatus; import com.yahoo.vespa.hosted.provision.applications.Cluster; import com.yahoo.vespa.hosted.provision.applications.Status; import org.junit.Test; @@ -79,7 +80,7 @@ public class ClusterModelTest { Optional.empty(), Optional.empty(), List.of(), - ""); + AutoscalingStatus.empty()); } /** Creates the given number of measurements, spaced 5 minutes between, using the given function */ diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainerTest.java index 5b2f7ce91e8..cfe6e4d348d 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainerTest.java @@ -43,7 +43,7 @@ public class NodeMetricsDbMaintainerTest { fetcher, Duration.ofHours(1), new TestMetric()); - assertTrue(maintainer.maintain()); + assertEquals(maintainer.maintain(), 1.0, 0.0000001); List<NodeTimeseries> timeseriesList = tester.nodeRepository().metricsDb().getNodeTimeseries(Duration.ofDays(1), Set.of("host-1.yahoo.com", "host-2.yahoo.com")); assertEquals(2, timeseriesList.size()); 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 9cac6430d6e..61ff494679f 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 @@ -6,6 +6,7 @@ import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.NodeResources; import com.yahoo.vespa.hosted.provision.applications.Application; +import com.yahoo.vespa.hosted.provision.applications.AutoscalingStatus; import com.yahoo.vespa.hosted.provision.applications.Cluster; import com.yahoo.vespa.hosted.provision.applications.ScalingEvent; import com.yahoo.vespa.hosted.provision.applications.Status; @@ -35,7 +36,7 @@ public class ApplicationSerializerTest { Optional.empty(), Optional.empty(), List.of(), - "")); + AutoscalingStatus.empty())); var minResources = new NodeResources(1, 2, 3, 4); clusters.add(new Cluster(ClusterSpec.Id.from("c2"), true, @@ -50,7 +51,7 @@ public class ApplicationSerializerTest { 7L, Instant.ofEpochMilli(12345L), Optional.of(Instant.ofEpochMilli(67890L)))), - "Autoscaling status")); + new AutoscalingStatus(AutoscalingStatus.Status.insufficient, "Autoscaling status"))); Application original = new Application(ApplicationId.from("myTenant", "myApplication", "myInstance"), Status.initial().withCurrentReadShare(0.3).withMaxReadShare(0.5), clusters); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClientTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClientTest.java index f47fb7f23be..99f6ce4fb00 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClientTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClientTest.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.hosted.provision.persistence; import com.yahoo.config.provision.ApplicationId; @@ -6,7 +6,6 @@ import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.TenantName; -import com.yahoo.config.provision.Zone; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.hosted.provision.Node; @@ -25,7 +24,7 @@ public class CuratorDatabaseClientTest { private final Curator curator = new MockCurator(); private final CuratorDatabaseClient zkClient = new CuratorDatabaseClient( - FlavorConfigBuilder.createDummies("default"), curator, Clock.systemUTC(), Zone.defaultZone(), true, 1000); + FlavorConfigBuilder.createDummies("default"), curator, Clock.systemUTC(), true, 1000); @Test public void can_read_stored_host_information() throws Exception { 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 1083930e294..ca0c548c1be 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 @@ -104,6 +104,7 @@ "at" : 123 } ], + "autoscalingStatusCode": "unavailable", "autoscalingStatus": "", "scalingDuration": 600000, "maxQueryGrowthRate": 0.1, 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 61e0569d349..c7eaa4af974 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 @@ -80,6 +80,7 @@ "at" : 123 } ], + "autoscalingStatusCode": "unavailable", "autoscalingStatus" : "", "scalingDuration": 43200000, "maxQueryGrowthRate": 0.1, diff --git a/searchlib/src/tests/attribute/attribute_test.cpp b/searchlib/src/tests/attribute/attribute_test.cpp index af1fcea2e21..79e120d0683 100644 --- a/searchlib/src/tests/attribute/attribute_test.cpp +++ b/searchlib/src/tests/attribute/attribute_test.cpp @@ -1670,8 +1670,8 @@ AttributeTest::testStatus() AttributePtr ptr = createAttribute("as", cfg); addDocs(ptr, numDocs); auto & sa = *(static_cast<StringAttribute *>(ptr.get())); - const size_t numUniq(16); - const size_t numValuesPerDoc(16); + const size_t numValuesPerDoc(values.size()); + const size_t numUniq(numValuesPerDoc); for (uint32_t i = 0; i < numDocs; ++i) { EXPECT_TRUE(appendToVector(sa, i, numValuesPerDoc, values)); } @@ -1680,11 +1680,11 @@ AttributeTest::testStatus() EXPECT_EQUAL(ptr->getStatus().getNumValues(), numDocs*numValuesPerDoc); EXPECT_EQUAL(ptr->getStatus().getNumUniqueValues(), numUniq); size_t expUsed = 0; - expUsed += 1 * InternalNodeSize + 1 * LeafNodeSize; // enum store tree - expUsed += numUniq * 32; // enum store (16 unique values, 32 bytes per entry) + expUsed += 1 * InternalNodeSize + 1 * LeafNodeSize; // Approximate enum store tree + expUsed += 272; // TODO Approximate... enum store (16 unique values, 17 bytes per entry) // multi value mapping (numdocs * sizeof(MappingIndex) + numvalues * sizeof(EnumIndex) + - // numdocs * sizeof(Array<EnumIndex>) (due to vector vector)) - expUsed += numDocs * sizeof(vespalib::datastore::EntryRef) + numDocs * numValuesPerDoc * sizeof(IEnumStore::Index) + ((numValuesPerDoc > 1024) ? numDocs * NestedVectorSize : 0); + // 32 + numdocs * sizeof(Array<EnumIndex>) (due to vector vector)) + expUsed += 32 + numDocs * sizeof(vespalib::datastore::EntryRef) + numDocs * numValuesPerDoc * sizeof(IEnumStore::Index) + ((numValuesPerDoc > 1024) ? numDocs * NestedVectorSize : 0); EXPECT_GREATER_EQUAL(ptr->getStatus().getUsed(), expUsed); EXPECT_GREATER_EQUAL(ptr->getStatus().getAllocated(), expUsed); } diff --git a/searchlib/src/tests/attribute/changevector/changevector_test.cpp b/searchlib/src/tests/attribute/changevector/changevector_test.cpp index ad33774e904..93a23630407 100644 --- a/searchlib/src/tests/attribute/changevector/changevector_test.cpp +++ b/searchlib/src/tests/attribute/changevector/changevector_test.cpp @@ -2,17 +2,27 @@ #include <vespa/vespalib/testkit/testapp.h> #include <vespa/searchlib/attribute/changevector.hpp> - +#include <vespa/vespalib/stllike/hash_set.h> using namespace search; +using Change = ChangeTemplate<NumericChangeData<long>>; +using CV = ChangeVectorT<Change>; + template <typename T> void verifyStrictOrdering(const T & v) { - long count(0); - for (const auto & c : v) { - count++; - EXPECT_EQUAL(count, c._data.get()); + vespalib::hash_set<uint32_t> complete; + uint32_t prev_doc(0); + uint32_t prev_value(0); + for (const auto & c : v.getDocIdInsertOrder()) { + if (prev_doc != c._doc) { + complete.insert(prev_doc); + EXPECT_FALSE(complete.contains(c._doc)); + prev_doc = c._doc; + } else { + EXPECT_GREATER(c._data, prev_value); + } + prev_value = c._data; } - EXPECT_EQUAL(v.size(), size_t(count)); } class Accessor { @@ -30,8 +40,6 @@ private: TEST("require insert ordering is preserved for same doc") { - typedef ChangeTemplate<NumericChangeData<long>> Change; - typedef ChangeVectorT<Change> CV; CV a; a.push_back(Change(Change::NOOP, 7, 1)); EXPECT_EQUAL(1u, a.size()); @@ -42,8 +50,6 @@ TEST("require insert ordering is preserved for same doc") TEST("require insert ordering is preserved ") { - typedef ChangeTemplate<NumericChangeData<long>> Change; - typedef ChangeVectorT<Change> CV; CV a; a.push_back(Change(Change::NOOP, 7, 1)); EXPECT_EQUAL(1u, a.size()); @@ -56,8 +62,6 @@ TEST("require insert ordering is preserved ") TEST("require insert ordering is preserved with mix") { - typedef ChangeTemplate<NumericChangeData<long>> Change; - typedef ChangeVectorT<Change> CV; CV a; a.push_back(Change(Change::NOOP, 7, 1)); EXPECT_EQUAL(1u, a.size()); @@ -77,8 +81,6 @@ TEST("require insert ordering is preserved with mix") } TEST("require that inserting empty vector does not affect the vector.") { - typedef ChangeTemplate<NumericChangeData<long>> Change; - typedef ChangeVectorT<Change> CV; CV a; std::vector<long> v; Accessor ac(v); @@ -86,4 +88,42 @@ TEST("require that inserting empty vector does not affect the vector.") { EXPECT_EQUAL(0u, a.size()); } +TEST("require that we have control over buffer construction size") { + CV a; + EXPECT_EQUAL(0u, a.size()); + EXPECT_EQUAL(256u, a.capacity()); + a.clear(); + EXPECT_EQUAL(0u, a.size()); + EXPECT_EQUAL(256u, a.capacity()); +} + +TEST("require that buffer can grow some") { + CV a; + for (size_t i(0); i < 1024; i++) { + a.push_back(Change(Change::NOOP, i, i)); + } + EXPECT_EQUAL(1024u, a.size()); + EXPECT_EQUAL(1024u, a.capacity()); + a.clear(); + EXPECT_EQUAL(0u, a.size()); + EXPECT_EQUAL(1024u, a.capacity()); +} + +TEST("require that buffer can grow some, but not unbound") { + CV a; + for (size_t i(0); i < 1025; i++) { + a.push_back(Change(Change::NOOP, i, i)); + } + EXPECT_EQUAL(1025u, a.size()); + EXPECT_EQUAL(2048u, a.capacity()); + a.clear(); + EXPECT_EQUAL(0u, a.size()); + EXPECT_EQUAL(256u, a.capacity()); +} + +TEST("Control Change size") { + EXPECT_EQUAL(32u, sizeof(ChangeTemplate<NumericChangeData<long>>)); + EXPECT_EQUAL(88u, sizeof(ChangeTemplate<StringChangeData>)); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.hpp b/searchlib/src/vespa/searchlib/attribute/attributevector.hpp index efc96bc57c2..616096e9091 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributevector.hpp +++ b/searchlib/src/vespa/searchlib/attribute/attributevector.hpp @@ -89,40 +89,36 @@ AttributeVector::adjustWeight(ChangeVectorT< ChangeTemplate<T> >& changes, DocId template<typename T> bool -AttributeVector::applyArithmetic(ChangeVectorT< ChangeTemplate<T> > & changes, DocId doc, const T & v, +AttributeVector::applyArithmetic(ChangeVectorT< ChangeTemplate<T> > & changes, DocId doc, const T &, const ArithmeticValueUpdate & arithm) { - (void) v; - bool retval(!hasMultiValue() && (doc < getNumDocs())); - if (retval) { - size_t oldSz(changes.size()); - ArithmeticValueUpdate::Operator op(arithm.getOperator()); - double aop = arithm.getOperand(); - if (op == ArithmeticValueUpdate::Add) { - changes.push_back(ChangeTemplate<T>(ChangeBase::ADD, doc, 0, 0)); - } else if (op == ArithmeticValueUpdate::Sub) { - changes.push_back(ChangeTemplate<T>(ChangeBase::SUB, doc, 0, 0)); - } else if (op == ArithmeticValueUpdate::Mul) { - changes.push_back(ChangeTemplate<T>(ChangeBase::MUL, doc, 0, 0)); - } else if (op == ArithmeticValueUpdate::Div) { - if (this->getClass().inherits(IntegerAttribute::classId) && aop == 0) { - divideByZeroWarning(); - } else { - changes.push_back(ChangeTemplate<T>(ChangeBase::DIV, doc, 0, 0)); - } + if (hasMultiValue() || (doc >= getNumDocs())) return false; + + size_t oldSz(changes.size()); + ArithmeticValueUpdate::Operator op(arithm.getOperator()); + double aop = arithm.getOperand(); + if (op == ArithmeticValueUpdate::Add) { + changes.push_back(ChangeTemplate<T>(ChangeBase::ADD, doc, 0, 0)); + } else if (op == ArithmeticValueUpdate::Sub) { + changes.push_back(ChangeTemplate<T>(ChangeBase::SUB, doc, 0, 0)); + } else if (op == ArithmeticValueUpdate::Mul) { + changes.push_back(ChangeTemplate<T>(ChangeBase::MUL, doc, 0, 0)); + } else if (op == ArithmeticValueUpdate::Div) { + if (this->getClass().inherits(IntegerAttribute::classId) && aop == 0) { + divideByZeroWarning(); } else { - retval = false; - } - if (retval) { - const size_t diff = changes.size() - oldSz; - _status.incNonIdempotentUpdates(diff); - _status.incUpdates(diff); - if (diff > 0) { - changes.back()._arithOperand = aop; - } + changes.push_back(ChangeTemplate<T>(ChangeBase::DIV, doc, 0, 0)); } + } else { + return false; } - return retval; + const size_t diff = changes.size() - oldSz; + _status.incNonIdempotentUpdates(diff); + _status.incUpdates(diff); + if (diff > 0) { + changes.back()._arithOperand = aop; + } + return true; } template<typename T> diff --git a/searchlib/src/vespa/searchlib/attribute/changevector.h b/searchlib/src/vespa/searchlib/attribute/changevector.h index c3b4d0ce3b0..d63ef2e2b35 100644 --- a/searchlib/src/vespa/searchlib/attribute/changevector.h +++ b/searchlib/src/vespa/searchlib/attribute/changevector.h @@ -2,8 +2,8 @@ #pragma once -#include <vespa/vespalib/stllike/hash_map.h> #include <vespa/searchcommon/common/undefinedvalues.h> +#include <vespa/vespalib/stllike/allocator.h> #include <vector> namespace vespalib { class MemoryUsage; } @@ -26,11 +26,10 @@ struct ChangeBase { DIV, CLEARDOC }; - enum {TAIL=0, UNSET_ENUM = 0xffffffffu}; + enum {UNSET_ENUM = 0xffffffffu}; ChangeBase() : _type(NOOP), - _next(TAIL), _doc(0), _weight(1), _enumScratchPad(UNSET_ENUM), @@ -39,7 +38,6 @@ struct ChangeBase { ChangeBase(Type type, uint32_t d, int32_t w = 1) : _type(type), - _next(TAIL), _doc(d), _weight(w), _enumScratchPad(UNSET_ENUM), @@ -48,18 +46,11 @@ struct ChangeBase { int cmp(const ChangeBase &b) const { int diff(_doc - b._doc); return diff; } bool operator <(const ChangeBase & b) const { return cmp(b) < 0; } - bool isAtEnd() const { return _next == TAIL; } - uint32_t getNext() const { return _next; } - void setNext(uint32_t next) { _next = next; } uint32_t getEnum() const { return _enumScratchPad; } void setEnum(uint32_t value) const { _enumScratchPad = value; } bool isEnumValid() const { return _enumScratchPad != UNSET_ENUM; } - void invalidateEnum() const { _enumScratchPad = UNSET_ENUM; } Type _type; -private: - uint32_t _next; -public: uint32_t _doc; int32_t _weight; mutable uint32_t _enumScratchPad; @@ -108,7 +99,7 @@ struct ChangeTemplate : public ChangeBase { ChangeBase(type, d, w), _data(v) { } - T _data; + T _data; }; template <> @@ -131,54 +122,66 @@ NumericChangeData<double>::operator<(const NumericChangeData<double> &rhs) const return _v < rhs._v; } -class ChangeVectorBase { -protected: -}; - /** - * Maintains a list of changes where changes to the same docid are adjacent, but ordered by insertion order. - * Apart from that no ordering by docid. + * Maintains a list of changes. + * You can select to view the in insert order, + * or unordered, but changes to the same docid are adjacent and ordered by insertion order. */ template <typename T> -class ChangeVectorT : public ChangeVectorBase { +class ChangeVectorT { private: - using Map = vespalib::hash_map<uint32_t, uint32_t>; - using Vector = std::vector<T>; + using Vector = std::vector<T, vespalib::allocator_large<T>>; public: + using const_iterator = typename Vector::const_iterator; ChangeVectorT(); ~ChangeVectorT(); - class const_iterator { - public: - const_iterator(const Vector & vector, uint32_t next) : _v(&vector), _next(next) { } - bool operator == (const const_iterator & rhs) const { return _v == rhs._v && _next == rhs._next; } - bool operator != (const const_iterator & rhs) const { return _v != rhs._v || _next != rhs._next; } - const_iterator& operator++() { advance(); return *this; } - const_iterator operator++(int) { const_iterator other(*this); advance(); return other; } - const T & operator * () const { return v()[_next]; } - const T * operator -> () const { return &v()[_next]; } - private: - void advance() { _next = v()[_next].getNext(); } - const Vector & v() const { return *_v; } - const Vector * _v; - uint32_t _next; - }; - void push_back(const T & c); template <typename Accessor> void push_back(uint32_t doc, Accessor & ac); - const T & back() const { return _v.back(); } T & back() { return _v.back(); } size_t size() const { return _v.size(); } + size_t capacity() const { return _v.capacity(); } bool empty() const { return _v.empty(); } void clear(); - const_iterator begin() const { return const_iterator(_v, 0); } - const_iterator end() const { return const_iterator(_v, size()); } + class InsertOrder { + public: + InsertOrder(const Vector & v) : _v(v) { } + const_iterator begin() const { return _v.begin(); } + const_iterator end() const { return _v.end(); } + private: + const Vector &_v; + }; + class DocIdInsertOrder { + using AdjacentDocIds = std::vector<uint64_t, vespalib::allocator_large<uint64_t>>; + public: + class const_iterator { + public: + const_iterator(const Vector & vector, const AdjacentDocIds & order, uint32_t cur) + : _v(&vector), _o(&order), _cur(cur) { } + bool operator == (const const_iterator & rhs) const { return _v == rhs._v && _cur == rhs._cur; } + bool operator != (const const_iterator & rhs) const { return _v != rhs._v || _cur != rhs._cur; } + const_iterator& operator++() { _cur++; return *this; } + const_iterator operator++(int) { const_iterator other(*this); _cur++; return other; } + const T & operator * () const { return v(); } + const T * operator -> () const { return &v(); } + private: + const T & v() const { return (*_v)[(*_o)[_cur] & 0xffffffff]; } + const Vector * _v; + const AdjacentDocIds * _o; + uint32_t _cur; + }; + DocIdInsertOrder(const Vector & v); + const_iterator begin() const { return const_iterator(_v, _adjacent, 0); } + const_iterator end() const { return const_iterator(_v, _adjacent, _v.size()); } + private: + const Vector &_v; + AdjacentDocIds _adjacent; + }; + InsertOrder getInsertOrder() const { return InsertOrder(_v); } + DocIdInsertOrder getDocIdInsertOrder() const { return DocIdInsertOrder(_v); } vespalib::MemoryUsage getMemoryUsage() const; private: - void linkIn(uint32_t doc, size_t index, size_t last); - Vector _v; - Map _docs; - uint32_t _tail; + Vector _v; }; } // namespace search diff --git a/searchlib/src/vespa/searchlib/attribute/changevector.hpp b/searchlib/src/vespa/searchlib/attribute/changevector.hpp index dcb31ebae73..5052f4b9a10 100644 --- a/searchlib/src/vespa/searchlib/attribute/changevector.hpp +++ b/searchlib/src/vespa/searchlib/attribute/changevector.hpp @@ -4,9 +4,12 @@ #include "changevector.h" #include <vespa/vespalib/util/memoryusage.h> +#include <vespa/vespalib/util/alloc.h> namespace search { +using vespalib::roundUp2inN; + namespace { // This number is selected to be large enough to hold bursts between commits @@ -16,11 +19,9 @@ constexpr size_t NUM_ELEMS_TO_RESERVE = 200; template <typename T> ChangeVectorT<T>::ChangeVectorT() - : _v(), - _docs(NUM_ELEMS_TO_RESERVE*2), - _tail(0) + : _v() { - _v.reserve(vespalib::roundUp2inN(NUM_ELEMS_TO_RESERVE, sizeof(T))); + _v.reserve(roundUp2inN<T>(NUM_ELEMS_TO_RESERVE)); } template <typename T> @@ -29,17 +30,21 @@ ChangeVectorT<T>::~ChangeVectorT() = default; template <typename T> void ChangeVectorT<T>::clear() { - _v.clear(); - _docs.clear(); + if (_v.capacity() > roundUp2inN<T>(NUM_ELEMS_TO_RESERVE * 5)) { + // Ensure we do not keep insanely large buffers over time, due to abnormal peaks + // caused by hickups else where. + _v = Vector(); + _v.reserve(roundUp2inN<T>(NUM_ELEMS_TO_RESERVE)); + } else { + _v.clear(); + } } template <typename T> void ChangeVectorT<T>::push_back(const T & c) { - size_t index(size()); _v.push_back(c); - linkIn(c._doc, index, index); } template <typename T> @@ -49,48 +54,32 @@ ChangeVectorT<T>::push_back(uint32_t doc, Accessor & ac) { if (ac.size() <= 0) { return; } - size_t index(size()); - _v.reserve(vespalib::roundUp2inN(index + ac.size(), sizeof(T))); + _v.reserve(roundUp2inN<T>(size() + ac.size())); for (size_t i(0), m(ac.size()); i < m; i++, ac.next()) { _v.push_back(T(ChangeBase::APPEND, doc, typename T::DataType(ac.value()), ac.weight())); - _v.back().setNext(index + i + 1); } - linkIn(doc, index, size() - 1); } template <typename T> -void -ChangeVectorT<T>::linkIn(uint32_t doc, size_t first, size_t last) +vespalib::MemoryUsage +ChangeVectorT<T>::getMemoryUsage() const { - if (first != 0 && (_v[_tail]._doc == doc)) { - _v[_tail].setNext(first); - _tail = last; - } else { - Map::iterator found(_docs.find(doc)); - if (found == _docs.end()) { - _docs[doc] = last; - if (_tail != first) { - _v[_tail].setNext(first); - } - _tail = last; - } else { - uint32_t prev(found->second); - for (; _v[_v[prev].getNext()]._doc == doc; prev = _v[prev].getNext()); - _v[last].setNext(_v[prev].getNext()); - _v[prev].setNext(first); - found->second = last; - } - } - _v[_tail].setNext(size()); + size_t usedBytes = _v.size() * sizeof(T); + size_t allocBytes = _v.capacity() * sizeof(T); + return vespalib::MemoryUsage(allocBytes, usedBytes, 0, 0); } template <typename T> -vespalib::MemoryUsage -ChangeVectorT<T>::getMemoryUsage() const +ChangeVectorT<T>::DocIdInsertOrder::DocIdInsertOrder(const Vector & v) + : _v(v), + _adjacent() { - size_t usedBytes = _v.size() * sizeof(T) + _docs.getMemoryUsed(); - size_t allocBytes = _v.capacity() * sizeof(T) + _docs.getMemoryConsumption(); - return vespalib::MemoryUsage(allocBytes, usedBytes, 0, 0); + _adjacent.reserve(v.size()); + uint32_t index(0); + for (const auto & c : _v) { + _adjacent.push_back((uint64_t(c._doc) << 32) | index++); + } + std::sort(_adjacent.begin(), _adjacent.end()); } } // namespace search diff --git a/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp index fd576b3a9ba..164bb411061 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp @@ -63,7 +63,7 @@ void EnumAttribute<B>::insertNewUniqueValues(EnumStoreBatchUpdater& updater) { // find and insert new unique strings - for (const auto & data : this->_changes) { + for (const auto & data : this->_changes.getInsertOrder()) { considerAttributeChange(data, updater); } } diff --git a/searchlib/src/vespa/searchlib/attribute/enumstore.hpp b/searchlib/src/vespa/searchlib/attribute/enumstore.hpp index 90bcf92a103..9885613f4e3 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumstore.hpp +++ b/searchlib/src/vespa/searchlib/attribute/enumstore.hpp @@ -207,7 +207,7 @@ template <typename EntryT> vespalib::MemoryUsage EnumStoreT<EntryT>::update_stat() { - auto &store = _store.get_allocator().get_data_store(); + auto &store = _store.get_data_store(); _cached_values_memory_usage = store.getMemoryUsage(); _cached_values_address_space_usage = store.getAddressSpaceUsage(); _cached_dictionary_btree_usage = _dict->get_btree_memory_usage(); diff --git a/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp index 8475451ba60..072398abcf9 100644 --- a/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp @@ -25,7 +25,7 @@ template <typename B, typename M> bool MultiValueEnumAttribute<B, M>::extractChangeData(const Change & c, EnumIndex & idx) { - if (c._enumScratchPad == Change::UNSET_ENUM) { + if ( ! c.isEnumValid() ) { return this->_enumStore.find_index(c._data.raw(), idx); } idx = EnumIndex(vespalib::datastore::EntryRef(c._enumScratchPad)); diff --git a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.h b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.h index 66ca6bd2eac..d36777a25a9 100644 --- a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.h @@ -20,7 +20,6 @@ protected: typedef typename B::DocId DocId; typedef typename B::Change Change; typedef typename B::ChangeVector ChangeVector; - typedef typename B::ChangeVector::const_iterator ChangeVectorIterator; using MultiValueType = M; using MultiValueMapping = attribute::MultiValueMapping<MultiValueType>; diff --git a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp index 0cd2e0bbc27..2e73909ea1e 100644 --- a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp @@ -78,11 +78,12 @@ void MultiValueAttribute<B, M>::apply_attribute_changes_to_array(DocumentValues& docValues) { // compute new values for each document with changes - for (ChangeVectorIterator current(this->_changes.begin()), end(this->_changes.end()); (current != end); ) { + auto iterable = this->_changes.getDocIdInsertOrder(); + for (auto current(iterable.begin()), end(iterable.end()); (current != end); ) { DocId doc = current->_doc; // find last clear doc - ChangeVectorIterator last_clear_doc = end; - for (ChangeVectorIterator iter = current; (iter != end) && (iter->_doc == doc); ++iter) { + auto last_clear_doc = end; + for (auto iter = current; (iter != end) && (iter->_doc == doc); ++iter) { if (iter->_type == ChangeBase::CLEARDOC) { last_clear_doc = iter; } @@ -137,12 +138,13 @@ void MultiValueAttribute<B, M>::apply_attribute_changes_to_wset(DocumentValues& docValues) { // compute new values for each document with changes - for (ChangeVectorIterator current(this->_changes.begin()), end(this->_changes.end()); (current != end); ) { + auto iterable = this->_changes.getDocIdInsertOrder(); + for (auto current(iterable.begin()), end(iterable.end()); (current != end); ) { const DocId doc = current->_doc; // find last clear doc - ChangeVectorIterator last_clear_doc = end; + auto last_clear_doc = end; size_t max_elems_inserted = 0; - for (ChangeVectorIterator iter = current; (iter != end) && (iter->_doc == doc); ++iter) { + for (auto iter = current; (iter != end) && (iter->_doc == doc); ++iter) { if (iter->_type == ChangeBase::CLEARDOC) { last_clear_doc = iter; } diff --git a/searchlib/src/vespa/searchlib/attribute/postingstore.cpp b/searchlib/src/vespa/searchlib/attribute/postingstore.cpp index 6c62e650345..477917debf0 100644 --- a/searchlib/src/vespa/searchlib/attribute/postingstore.cpp +++ b/searchlib/src/vespa/searchlib/attribute/postingstore.cpp @@ -696,7 +696,10 @@ PostingStore<DataT>::move(EntryRef ref) if (!_store.getCompacting(ref)) { return ref; } - return allocBitVectorCopy(*bve).ref; + auto new_ref = allocBitVectorCopy(*bve).ref; + _bvs.erase(ref.ref()); + _bvs.insert(new_ref.ref()); + return new_ref; } else { if (!_store.getCompacting(ref)) { return ref; diff --git a/searchlib/src/vespa/searchlib/attribute/singleboolattribute.cpp b/searchlib/src/vespa/searchlib/attribute/singleboolattribute.cpp index 2b40150f87b..6c8edea13cf 100644 --- a/searchlib/src/vespa/searchlib/attribute/singleboolattribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/singleboolattribute.cpp @@ -59,7 +59,7 @@ SingleBoolAttribute::onCommit() { if ( ! _changes.empty()) { // apply updates ValueModifier valueGuard(getValueModifier()); - for (const auto & change : _changes) { + for (const auto & change : _changes.getInsertOrder()) { if (change._type == ChangeBase::UPDATE) { std::atomic_thread_fence(std::memory_order_release); setBit(change._doc, change._data != 0); diff --git a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp index b39bdeb3b00..bf75400b157 100644 --- a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp @@ -175,7 +175,7 @@ void SingleValueEnumAttribute<B>::applyValueChanges(EnumStoreBatchUpdater& updater) { ValueModifier valueGuard(this->getValueModifier()); - for (const auto& change : this->_changes) { + for (const auto& change : this->_changes.getInsertOrder()) { if (change._type == ChangeBase::UPDATE) { applyUpdateValueChange(change, updater); } else if (change._type >= ChangeBase::ADD && change._type <= ChangeBase::DIV) { diff --git a/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.hpp index fd913f34c3a..671bdc44e22 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.hpp @@ -37,7 +37,7 @@ SingleValueNumericAttribute<B>::onCommit() { // apply updates typename B::ValueModifier valueGuard(this->getValueModifier()); - for (const auto & change : this->_changes) { + for (const auto & change : this->_changes.getInsertOrder()) { if (change._type == ChangeBase::UPDATE) { std::atomic_thread_fence(std::memory_order_release); _data[change._doc] = change._data; diff --git a/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp index f5ab855565c..e1c2a817af7 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp @@ -84,7 +84,7 @@ SingleValueNumericPostingAttribute<B>::applyValueChanges(EnumStoreBatchUpdater& // used to make sure several arithmetic operations on the same document in a single commit works std::map<DocId, EnumIndex> currEnumIndices; - for (const auto& change : this->_changes) { + for (const auto& change : this->_changes.getInsertOrder()) { auto enumIter = currEnumIndices.find(change._doc); EnumIndex oldIdx; if (enumIter != currEnumIndices.end()) { diff --git a/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.cpp b/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.cpp index f1d0da42165..8d460b5c661 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.cpp @@ -53,7 +53,7 @@ SingleValueSmallNumericAttribute::onCommit() { // apply updates B::ValueModifier valueGuard(getValueModifier()); - for (const auto & change : _changes) { + for (const auto & change : _changes.getInsertOrder()) { if (change._type == ChangeBase::UPDATE) { std::atomic_thread_fence(std::memory_order_release); set(change._doc, change._data); diff --git a/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp index 39ad8d71021..4432acf2c55 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp @@ -85,7 +85,7 @@ SingleValueStringPostingAttributeT<B>::applyValueChanges(EnumStoreBatchUpdater& // used to make sure several arithmetic operations on the same document in a single commit works std::map<DocId, EnumIndex> currEnumIndices; - for (const auto& change : this->_changes) { + for (const auto& change : this->_changes.getInsertOrder()) { auto enumIter = currEnumIndices.find(change._doc); EnumIndex oldIdx; if (enumIter != currEnumIndices.end()) { diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp b/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp index 31ff3845d74..465793739ff 100644 --- a/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp @@ -25,7 +25,7 @@ namespace { // TODO: Move this to MemoryAllocator, with name PAGE_SIZE. constexpr size_t small_page_size = 4_Ki; -constexpr size_t min_num_arrays_for_new_buffer = 8_Ki; +constexpr size_t min_num_arrays_for_new_buffer = 64_Ki; constexpr float alloc_grow_factor = 0.2; // TODO: Adjust these numbers to what we accept as max in config. constexpr size_t max_level_array_size = 16; diff --git a/slobrok/src/vespa/slobrok/sbmirror.cpp b/slobrok/src/vespa/slobrok/sbmirror.cpp index 5f6a54504e5..8102e1fecbf 100644 --- a/slobrok/src/vespa/slobrok/sbmirror.cpp +++ b/slobrok/src/vespa/slobrok/sbmirror.cpp @@ -26,7 +26,6 @@ MirrorAPI::MirrorAPI(FRT_Supervisor &orb, const ConfiguratorFactory & config) _configurator(config.create(_slobrokSpecs)), _currSlobrok(""), _rpc_ms(100), - _idx(0), _backOff(), _target(0), _req(0) diff --git a/slobrok/src/vespa/slobrok/sbmirror.h b/slobrok/src/vespa/slobrok/sbmirror.h index ad86daa56bb..ec1ce22194b 100644 --- a/slobrok/src/vespa/slobrok/sbmirror.h +++ b/slobrok/src/vespa/slobrok/sbmirror.h @@ -101,7 +101,6 @@ private: Configurator::UP _configurator; std::string _currSlobrok; int _rpc_ms; - uint32_t _idx; BackOff _backOff; FRT_Target *_target; FRT_RPCRequest *_req; diff --git a/staging_vespalib/src/vespa/vespalib/util/programoptions.cpp b/staging_vespalib/src/vespa/vespalib/util/programoptions.cpp index e38c54aaba0..8da7780f203 100644 --- a/staging_vespalib/src/vespa/vespalib/util/programoptions.cpp +++ b/staging_vespalib/src/vespa/vespalib/util/programoptions.cpp @@ -3,6 +3,7 @@ #include <vespa/vespalib/util/stringfmt.h> #include <vespa/vespalib/util/exceptions.h> #include <boost/lexical_cast.hpp> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".programoptions"); diff --git a/storage/src/tests/storageserver/mergethrottlertest.cpp b/storage/src/tests/storageserver/mergethrottlertest.cpp index 12ed9ead1b6..3a153fef9c3 100644 --- a/storage/src/tests/storageserver/mergethrottlertest.cpp +++ b/storage/src/tests/storageserver/mergethrottlertest.cpp @@ -1233,6 +1233,7 @@ TEST_F(MergeThrottlerTest, busy_returned_on_full_queue) { // Wait till we have maxPending replies and maxQueue queued _topLinks[0]->waitForMessages(maxPending, _messageWaitTime); + EXPECT_EQ(19, _throttlers[0]->getMetrics().queueSize.getMaximum()); waitUntilMergeQueueIs(*_throttlers[0], maxQueue, _messageWaitTime); // Clear all forwarded merges diff --git a/storage/src/vespa/storage/storageserver/mergethrottler.cpp b/storage/src/vespa/storage/storageserver/mergethrottler.cpp index dc8457769a2..a9bd7ade270 100644 --- a/storage/src/vespa/storage/storageserver/mergethrottler.cpp +++ b/storage/src/vespa/storage/storageserver/mergethrottler.cpp @@ -71,6 +71,7 @@ MergeThrottler::ChainedMergeState::~ChainedMergeState() = default; MergeThrottler::Metrics::Metrics(metrics::MetricSet* owner) : metrics::MetricSet("mergethrottler", {}, "", owner), averageQueueWaitingTime("averagequeuewaitingtime", {}, "Average time a merge spends in the throttler queue", this), + queueSize("queuesize", {}, "Length of merge queue", this), bounced_due_to_back_pressure("bounced_due_to_back_pressure", {}, "Number of merges bounced due to resource exhaustion back-pressure", this), chaining("mergechains", this), local("locallyexecutedmerges", this) @@ -415,6 +416,7 @@ MergeThrottler::enqueueMerge( if (!validateNewMerge(mergeCmd, nodeSeq, msgGuard)) { return; } + _metrics->queueSize.set(_queue.size()); _queue.insert(MergePriorityQueue::value_type(msg, _queueSequence++)); } diff --git a/storage/src/vespa/storage/storageserver/mergethrottler.h b/storage/src/vespa/storage/storageserver/mergethrottler.h index e8815eee680..0c608f29196 100644 --- a/storage/src/vespa/storage/storageserver/mergethrottler.h +++ b/storage/src/vespa/storage/storageserver/mergethrottler.h @@ -57,12 +57,13 @@ public: MergeFailureMetrics failures; MergeOperationMetrics(const std::string& name, metrics::MetricSet* owner); - ~MergeOperationMetrics(); + ~MergeOperationMetrics() override; }; class Metrics : public metrics::MetricSet { public: metrics::DoubleAverageMetric averageQueueWaitingTime; + metrics::LongValueMetric queueSize; metrics::LongCountMetric bounced_due_to_back_pressure; MergeOperationMetrics chaining; MergeOperationMetrics local; diff --git a/vdslib/src/vespa/vdslib/distribution/redundancygroupdistribution.cpp b/vdslib/src/vespa/vdslib/distribution/redundancygroupdistribution.cpp index 6ac521a0f01..863ac45baa8 100644 --- a/vdslib/src/vespa/vdslib/distribution/redundancygroupdistribution.cpp +++ b/vdslib/src/vespa/vdslib/distribution/redundancygroupdistribution.cpp @@ -5,6 +5,7 @@ #include <vespa/vespalib/text/stringtokenizer.h> #include <boost/lexical_cast.hpp> #include <algorithm> +#include <cassert> namespace storage::lib { diff --git a/vdstestlib/src/vespa/vdstestlib/config/dirconfig.cpp b/vdstestlib/src/vespa/vdstestlib/config/dirconfig.cpp index 3a26ed9dec8..6cd9a132d5e 100644 --- a/vdstestlib/src/vespa/vdstestlib/config/dirconfig.cpp +++ b/vdstestlib/src/vespa/vdstestlib/config/dirconfig.cpp @@ -7,6 +7,7 @@ #include <vespa/vespalib/util/stringfmt.h> #include <fstream> #include <atomic> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".dirconfig"); diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/DefaultZtsClient.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/DefaultZtsClient.java index cdbf0755059..24234757590 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/DefaultZtsClient.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/DefaultZtsClient.java @@ -165,7 +165,7 @@ public class DefaultZtsClient extends ClientBase implements ZtsClient { @Override public X509Certificate getRoleCertificate(AthenzRole role, Pkcs10Csr csr, Duration expiry) { RoleCertificateRequestEntity requestEntity = new RoleCertificateRequestEntity(csr, expiry); - URI uri = ztsUrl.resolve(String.format("domain/%s/role/%s/token", role.domain().getName(), role.roleName())); + URI uri = ztsUrl.resolve("rolecert"); HttpUriRequest request = RequestBuilder.post(uri) .setEntity(toJsonStringEntity(requestEntity)) .build(); diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/bindings/RoleCertificateRequestEntity.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/bindings/RoleCertificateRequestEntity.java index 89bfce91154..16dd1d914ef 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/bindings/RoleCertificateRequestEntity.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/bindings/RoleCertificateRequestEntity.java @@ -38,7 +38,7 @@ public class RoleCertificateRequestEntity { public void serialize(Duration duration, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) throws IOException { - jsonGenerator.writeNumber(duration.getSeconds()); + jsonGenerator.writeNumber(duration.toMinutes()); } } } diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/bindings/RoleCertificateResponseEntity.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/bindings/RoleCertificateResponseEntity.java index 857bfad9143..cd9a12c0074 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/bindings/RoleCertificateResponseEntity.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/bindings/RoleCertificateResponseEntity.java @@ -16,12 +16,9 @@ import java.time.Instant; @JsonIgnoreProperties(ignoreUnknown = true) public class RoleCertificateResponseEntity { public final X509Certificate certificate; - public final Instant expiry; @JsonCreator - public RoleCertificateResponseEntity(@JsonProperty("token") @JsonDeserialize(using = X509CertificateDeserializer.class) X509Certificate certificate, - @JsonProperty("expiryTime") Instant expiry) { + public RoleCertificateResponseEntity(@JsonProperty("x509Certificate") @JsonDeserialize(using = X509CertificateDeserializer.class) X509Certificate certificate) { this.certificate = certificate; - this.expiry = expiry; } } diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java index 547ea524041..68cca286dac 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java @@ -8,11 +8,11 @@ import com.yahoo.security.SslContextBuilder; import com.yahoo.vespa.http.client.config.ConnectionParams; import com.yahoo.vespa.http.client.config.Endpoint; import com.yahoo.vespa.http.client.config.FeedParams; -import com.yahoo.vespa.http.client.core.Vtag; import com.yahoo.vespa.http.client.core.Document; import com.yahoo.vespa.http.client.core.Encoder; import com.yahoo.vespa.http.client.core.Headers; import com.yahoo.vespa.http.client.core.ServerResponseException; +import com.yahoo.vespa.http.client.core.Vtag; import org.apache.http.Header; import org.apache.http.HttpHost; import org.apache.http.HttpResponse; @@ -392,10 +392,12 @@ class ApacheGatewayConnection implements GatewayConnection { */ public static class HttpClientFactory { + private final FeedParams feedParams; final ConnectionParams connectionParams; final boolean useSsl; - public HttpClientFactory(ConnectionParams connectionParams, boolean useSsl) { + public HttpClientFactory(FeedParams feedParams, ConnectionParams connectionParams, boolean useSsl) { + this.feedParams = feedParams; this.connectionParams = connectionParams; this.useSsl = useSsl; } @@ -427,8 +429,10 @@ class ApacheGatewayConnection implements GatewayConnection { clientBuilder.setMaxConnTotal(1); clientBuilder.setUserAgent(String.format("vespa-http-client (%s)", Vtag.V_TAG_COMPONENT)); clientBuilder.setDefaultHeaders(Collections.singletonList(new BasicHeader(Headers.CLIENT_VERSION, Vtag.V_TAG_COMPONENT))); - RequestConfig.Builder requestConfigBuilder = RequestConfig.custom(); - requestConfigBuilder.setSocketTimeout(0); + int millisTotalTimeout = (int) (feedParams.getClientTimeout(TimeUnit.MILLISECONDS) + feedParams.getServerTimeout(TimeUnit.MILLISECONDS)); + RequestConfig.Builder requestConfigBuilder = RequestConfig.custom() + .setSocketTimeout(millisTotalTimeout) + .setConnectTimeout(millisTotalTimeout); if (connectionParams.getProxyHost() != null) { requestConfigBuilder.setProxy(new HttpHost(connectionParams.getProxyHost(), connectionParams.getProxyPort())); } diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ClusterConnection.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ClusterConnection.java index 16afd001c46..9dc214fb93d 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ClusterConnection.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ClusterConnection.java @@ -78,7 +78,7 @@ public class ClusterConnection implements AutoCloseable { feedParams, cluster.getRoute(), connectionParams, - new ApacheGatewayConnection.HttpClientFactory(connectionParams, endpoint.isUseSsl()), + new ApacheGatewayConnection.HttpClientFactory(feedParams, connectionParams, endpoint.isUseSsl()), operationProcessor.getClientId(), clock ); diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobMetrics.java b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobMetrics.java index fcc5b8e57a2..73a5dc77743 100644 --- a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobMetrics.java +++ b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobMetrics.java @@ -2,25 +2,18 @@ package com.yahoo.concurrent.maintenance; import java.util.concurrent.ConcurrentHashMap; -import java.util.function.BiConsumer; /** * Tracks and forwards maintenance job metrics. * * @author mpolden */ -public class JobMetrics { - - private final BiConsumer<String, Long> metricConsumer; +public abstract class JobMetrics { private final ConcurrentHashMap<String, Long> incompleteRuns = new ConcurrentHashMap<>(); - public JobMetrics(BiConsumer<String, Long> metricConsumer) { - this.metricConsumer = metricConsumer; - } - - /** Record a run for given job */ - public void recordRunOf(String job) { + /** Record starting of a run of a job */ + public void starting(String job) { incompleteRuns.merge(job, 1L, Long::sum); } @@ -29,12 +22,17 @@ public class JobMetrics { incompleteRuns.put(job, 0L); } - /** Forward metrics for given job to metric consumer */ - public void forward(String job) { + /** + * Records completion of a run of a job. + * This is guaranteed to always be called once whenever starting has been called. + */ + public void completed(String job, double successFactor) { Long incompleteRuns = this.incompleteRuns.get(job); if (incompleteRuns != null) { - metricConsumer.accept(job, incompleteRuns); + recordCompletion(job, incompleteRuns, successFactor); } } + protected abstract void recordCompletion(String job, Long incompleteRuns, double successFactor); + } diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java index 734c46a2819..2a9e6dda6b6 100644 --- a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java +++ b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java @@ -83,8 +83,19 @@ public abstract class Maintainer implements Runnable { @Override public final String toString() { return name(); } - /** Called once each time this maintenance job should run. Returns whether the maintenance run was successful */ - protected abstract boolean maintain(); + /** + * Called once each time this maintenance job should run. + * + * @return the degree to which the run was successful - a number between 0 (no success), to 1 (complete success). + * Note that this indicates whether something is wrong, so e.g if the call did nothing because it should do + * nothing, 1.0 should be returned. + */ + protected abstract double maintain(); + + /** Convenience methods to convert attempts and failures into a success factor */ + protected final double asSuccessFactor(int attempts, int failures) { + return attempts == 0 ? 1.0 : 1 - (double)failures / attempts; + } /** Returns the interval at which this job is set to run */ protected Duration interval() { return interval; } @@ -93,9 +104,12 @@ public abstract class Maintainer implements Runnable { public final void lockAndMaintain(boolean force) { if (!force && !jobControl.isActive(name())) return; log.log(Level.FINE, () -> "Running " + this.getClass().getSimpleName()); - jobMetrics.recordRunOf(name()); + jobMetrics.starting(name()); + double successFactor = 0; try (var lock = jobControl.lockJob(name())) { - if (maintain()) jobMetrics.recordCompletionOf(name()); + successFactor = maintain(); + if (successFactor > 0.0) + jobMetrics.recordCompletionOf(name()); } catch (UncheckedTimeoutException e) { if (ignoreCollision) { jobMetrics.recordCompletionOf(name()); @@ -105,7 +119,7 @@ public abstract class Maintainer implements Runnable { } catch (Throwable e) { log.log(Level.WARNING, this + " failed. Will retry in " + interval, e); } finally { - jobMetrics.forward(name()); + jobMetrics.completed(name(), successFactor); } log.log(Level.FINE, () -> "Finished " + this.getClass().getSimpleName()); } diff --git a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java index 139a2901cd3..01560c050ff 100644 --- a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java +++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java @@ -3,6 +3,8 @@ package com.yahoo.concurrent.maintenance; import org.junit.Test; +import java.util.concurrent.atomic.AtomicLong; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -19,9 +21,8 @@ public class JobControlTest { String job1 = "Job1"; String job2 = "Job2"; - JobMetrics metrics = new JobMetrics((job, instant) -> {}); - TestMaintainer maintainer1 = new TestMaintainer(job1, jobControl, metrics); - TestMaintainer maintainer2 = new TestMaintainer(job2, jobControl, metrics); + TestMaintainer maintainer1 = new TestMaintainer(job1, jobControl, new NoopJobMetrics()); + TestMaintainer maintainer2 = new TestMaintainer(job2, jobControl, new NoopJobMetrics()); assertEquals(2, jobControl.jobs().size()); assertTrue(jobControl.jobs().contains(job1)); assertTrue(jobControl.jobs().contains(job2)); @@ -62,7 +63,7 @@ public class JobControlTest { public void testJobControlMayDeactivateJobs() { JobControlStateMock state = new JobControlStateMock(); JobControl jobControl = new JobControl(state); - TestMaintainer mockMaintainer = new TestMaintainer(null, jobControl, new JobMetrics((job, instant) -> {})); + TestMaintainer mockMaintainer = new TestMaintainer(null, jobControl, new NoopJobMetrics()); assertTrue(jobControl.jobs().contains("TestMaintainer")); @@ -80,4 +81,11 @@ public class JobControlTest { assertEquals(2, mockMaintainer.totalRuns()); } + private static class NoopJobMetrics extends JobMetrics { + + @Override + protected void recordCompletion(String job, Long incompleteRuns, double successFactor) { } + + } + } diff --git a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/MaintainerTest.java b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/MaintainerTest.java index e881d4b3ff6..d2db380f4a1 100644 --- a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/MaintainerTest.java +++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/MaintainerTest.java @@ -42,35 +42,45 @@ public class MaintainerTest { @Test public void success_metric() { - AtomicLong consecutiveFailures = new AtomicLong(); - JobMetrics jobMetrics = new JobMetrics((job, count) -> consecutiveFailures.set(count)); + TestJobMetrics jobMetrics = new TestJobMetrics(); TestMaintainer maintainer = new TestMaintainer(null, jobControl, jobMetrics); // Maintainer fails twice in a row maintainer.successOnNextRun(false).run(); - assertEquals(1, consecutiveFailures.get()); + assertEquals(1, jobMetrics.consecutiveFailures.get()); maintainer.successOnNextRun(false).run(); - assertEquals(2, consecutiveFailures.get()); + assertEquals(2, jobMetrics.consecutiveFailures.get()); // Maintainer runs successfully maintainer.successOnNextRun(true).run(); - assertEquals(0, consecutiveFailures.get()); + assertEquals(0, jobMetrics.consecutiveFailures.get()); // Maintainer runs successfully again maintainer.run(); - assertEquals(0, consecutiveFailures.get()); + assertEquals(0, jobMetrics.consecutiveFailures.get()); // Maintainer throws maintainer.throwOnNextRun(new RuntimeException()).run(); - assertEquals(1, consecutiveFailures.get()); + assertEquals(1, jobMetrics.consecutiveFailures.get()); // Maintainer recovers maintainer.throwOnNextRun(null).run(); - assertEquals(0, consecutiveFailures.get()); + assertEquals(0, jobMetrics.consecutiveFailures.get()); // Lock exception is treated as a failure maintainer.throwOnNextRun(new UncheckedTimeoutException()).run(); - assertEquals(1, consecutiveFailures.get()); + assertEquals(1, jobMetrics.consecutiveFailures.get()); + } + + private static class TestJobMetrics extends JobMetrics { + + AtomicLong consecutiveFailures = new AtomicLong(); + + @Override + protected void recordCompletion(String job, Long incompleteRuns, double successFactor) { + consecutiveFailures.set(incompleteRuns); + } + } } diff --git a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/TestMaintainer.java b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/TestMaintainer.java index 44a00a37a83..7424b17cab2 100644 --- a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/TestMaintainer.java +++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/TestMaintainer.java @@ -33,10 +33,10 @@ class TestMaintainer extends Maintainer { } @Override - protected boolean maintain() { + protected double maintain() { if (exceptionToThrow != null) throw exceptionToThrow; totalRuns++; - return success; + return success ? 1.0 : 0.0; } } diff --git a/vespalib/src/tests/stllike/hashtable_test.cpp b/vespalib/src/tests/stllike/hashtable_test.cpp index cbd8b28d9a8..ac364fdf0df 100644 --- a/vespalib/src/tests/stllike/hashtable_test.cpp +++ b/vespalib/src/tests/stllike/hashtable_test.cpp @@ -5,9 +5,9 @@ #include <vespa/vespalib/stllike/hash_fun.h> #include <vespa/vespalib/stllike/identity.h> #include <vespa/vespalib/testkit/testapp.h> +#include <vespa/vespalib/stllike/hash_map.hpp> #include <memory> #include <vector> -#include <vespa/vespalib/stllike/hash_map.h> using vespalib::hashtable; using std::vector; @@ -134,6 +134,79 @@ TEST("require that hashtable<vector<int>> can be copied") { EXPECT_EQUAL(6, (*table.find(2))[2]); } +/** + * Test to profile destruction and recreation of hash map. + * It revealed some unexpected behaviour. Results with 10k iterations on 2018 macbook pro 2.6 Ghz i7 + * 1 - previous - 14.7s hash_node() : _node(), _next(invalid) {} + * 2 - test - 6.6s hash_node() : _next(invalid) { memset(_node, 0, sizeof(node)); } + * 3 - current - 2.3s hash_node() : _next(invalid) {} + */ +TEST("benchmark hash table reconstruction with POD objects") { + vespalib::hash_map<uint32_t, uint32_t> m(1000000); + constexpr size_t NUM_ITER = 10; // Set to 1k-10k to get measurable numbers 10k ~= 2.3s + for (size_t i(0); i < NUM_ITER; i++) { + m[46] = 17; + EXPECT_FALSE(m.empty()); + EXPECT_EQUAL(1u, m.size()); + EXPECT_EQUAL(1048576u, m.capacity()); + m.clear(); + EXPECT_TRUE(m.empty()); + EXPECT_EQUAL(1048576u, m.capacity()); + } +} + +class NonPOD { +public: + NonPOD() noexcept + : _v(rand()) + { + construction_count++; + } + NonPOD(NonPOD && rhs) noexcept { _v = rhs._v; rhs._v = -1; } + NonPOD & operator =(NonPOD && rhs) noexcept { _v = rhs._v; rhs._v = -1; return *this; } + NonPOD(const NonPOD &) = delete; + NonPOD & operator =(const NonPOD &) = delete; + ~NonPOD() { + if (_v != -1) { + destruction_count++; + } + } + int32_t _v; + static size_t construction_count; + static size_t destruction_count; +}; + +size_t NonPOD::construction_count = 0; +size_t NonPOD::destruction_count = 0; + +/** + * Performance is identical for NonPOD objects as with POD object. + * Object are are only constructed on insert, and destructed on erase/clear. + */ +TEST("benchmark hash table reconstruction with non POD objects") { + vespalib::hash_map<uint32_t, NonPOD> m(1000000); + constexpr size_t NUM_ITER = 10; // Set to 1k-10k to get measurable numbers 10k ~= 2.3s + NonPOD::construction_count = 0; + NonPOD::destruction_count = 0; + for (size_t i(0); i < NUM_ITER; i++) { + EXPECT_EQUAL(i, NonPOD::construction_count); + EXPECT_EQUAL(i, NonPOD::destruction_count); + m.insert(std::make_pair(46, NonPOD())); + EXPECT_EQUAL(i+1, NonPOD::construction_count); + EXPECT_EQUAL(i, NonPOD::destruction_count); + EXPECT_FALSE(m.empty()); + EXPECT_EQUAL(1u, m.size()); + EXPECT_EQUAL(1048576u, m.capacity()); + m.clear(); + EXPECT_EQUAL(i+1, NonPOD::construction_count); + EXPECT_EQUAL(i+1, NonPOD::destruction_count); + EXPECT_TRUE(m.empty()); + EXPECT_EQUAL(1048576u, m.capacity()); + } + EXPECT_EQUAL(NUM_ITER, NonPOD::construction_count); + EXPECT_EQUAL(NUM_ITER, NonPOD::destruction_count); +} + } // namespace TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/vespalib/src/vespa/vespalib/stllike/hashtable.h b/vespalib/src/vespa/vespalib/stllike/hashtable.h index b94672aaa06..ede18f89dc2 100644 --- a/vespalib/src/vespa/vespalib/stllike/hashtable.h +++ b/vespalib/src/vespa/vespalib/stllike/hashtable.h @@ -102,7 +102,9 @@ class hash_node { public: using next_t=hashtable_base::next_t; enum {npos=-1u, invalid=-2u}; - hash_node() : _node(), _next(invalid) {} + hash_node() + : _next(invalid) + {} hash_node(const V & node, next_t next=npos) : _next(next) { diff --git a/vespalib/src/vespa/vespalib/stllike/hashtable.hpp b/vespalib/src/vespa/vespalib/stllike/hashtable.hpp index d80113a8f55..494dc223f5b 100644 --- a/vespalib/src/vespa/vespalib/stllike/hashtable.hpp +++ b/vespalib/src/vespa/vespalib/stllike/hashtable.hpp @@ -146,6 +146,8 @@ hashtable<Key, Value, Hash, Equal, KeyExtract, Modulator>::erase(const Key & key template< typename Key, typename Value, typename Hash, typename Equal, typename KeyExtract, typename Modulator > void hashtable<Key, Value, Hash, Equal, KeyExtract, Modulator>::clear() { + if (_count == 0) return; // Already empty and properly initialized + _nodes.clear(); _count = 0; _nodes.resize(getTableSize()); diff --git a/vespalib/src/vespa/vespalib/util/alloc.h b/vespalib/src/vespa/vespalib/util/alloc.h index f608a244035..a97e8c9f25e 100644 --- a/vespalib/src/vespa/vespalib/util/alloc.h +++ b/vespalib/src/vespa/vespalib/util/alloc.h @@ -102,13 +102,21 @@ private: namespace vespalib { /// Rounds up to the closest number that is a power of 2 -inline size_t roundUp2inN(size_t minimum) { +inline size_t +roundUp2inN(size_t minimum) { return 2ul << Optimized::msbIdx(minimum - 1); } /// Rounds minElems up to the closest number where minElems*elemSize is a power of 2 -inline size_t roundUp2inN(size_t minElems, size_t elemSize) { +inline size_t +roundUp2inN(size_t minElems, size_t elemSize) { return roundUp2inN(minElems * elemSize)/elemSize; } +template <typename T> +size_t +roundUp2inN(size_t elems) { + return roundUp2inN(elems, sizeof(T)); +} + } diff --git a/vespalib/src/vespa/vespalib/util/optimized.h b/vespalib/src/vespa/vespalib/util/optimized.h index 92cf1f0ca24..6c6d1b12a71 100644 --- a/vespalib/src/vespa/vespalib/util/optimized.h +++ b/vespalib/src/vespa/vespalib/util/optimized.h @@ -22,9 +22,9 @@ public: static int lsbIdx(unsigned int v); static int lsbIdx(unsigned long v); static int lsbIdx(unsigned long long v); - static int popCount(unsigned int v) { return __builtin_popcount(v); } - static int popCount(unsigned long v) { return __builtin_popcountl(v); } - static int popCount(unsigned long long v) { return __builtin_popcountll(v); } + static constexpr int popCount(unsigned int v) { return __builtin_popcount(v); } + static constexpr int popCount(unsigned long v) { return __builtin_popcountl(v); } + static constexpr int popCount(unsigned long long v) { return __builtin_popcountll(v); } }; /** |