diff options
99 files changed, 715 insertions, 1277 deletions
diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index 4ccf34d30b0..1b5a8924fea 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -71,17 +71,13 @@ "public" ], "methods": [ - "public void <init>(java.io.File, java.lang.String, java.lang.String, java.lang.Long, boolean, java.lang.String, java.lang.Long, long)", - "public void <init>(java.lang.String, java.lang.String, java.lang.Long, boolean, java.lang.String, java.lang.String, java.lang.Long, long)", "public void <init>(java.lang.String, java.lang.String, java.lang.Long, boolean, com.yahoo.config.provision.ApplicationId, java.lang.String, java.lang.Long, long)", - "public java.lang.String getApplicationName()", "public java.lang.String getDeployedByUser()", "public java.lang.String getDeployPath()", "public com.yahoo.config.provision.ApplicationId getApplicationId()", "public java.lang.Long getDeployTimestamp()", "public java.lang.Long getGeneration()", "public boolean isInternalRedeploy()", - "public java.lang.String getCheckSum()", "public java.lang.String getChecksum()", "public long getPreviousActiveGeneration()", "public java.lang.String toString()", diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationMetaData.java b/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationMetaData.java index c4dee70cd86..480d12b6700 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationMetaData.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationMetaData.java @@ -2,14 +2,16 @@ package com.yahoo.config.application.api; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.ApplicationName; -import com.yahoo.config.provision.InstanceName; -import com.yahoo.config.provision.TenantName; -import com.yahoo.slime.*; - +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Inspector; +import com.yahoo.slime.JsonDecoder; +import com.yahoo.slime.JsonFormat; +import com.yahoo.slime.Slime; import com.yahoo.text.Utf8; -import java.io.*; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; /** * Metadata about an application package. @@ -27,32 +29,6 @@ public class ApplicationMetaData { private final long generation; private final long previousActiveGeneration; - // TODO: Remove after September 2019 - public ApplicationMetaData(File appDir, - String deployedByUser, - String deployedFromDir, - Long deployTimestamp, - boolean internalRedeploy, - String checksum, - Long generation, - long previousActiveGeneration) { - this(deployedByUser, deployedFromDir, deployTimestamp, internalRedeploy, - appDir.getName(), checksum, generation, previousActiveGeneration); - } - - // TODO: Remove after September 2019 - public ApplicationMetaData(String deployedByUser, String deployedFromDir, Long deployTimestamp, boolean internalRedeploy, - String applicationName, String checksum, Long generation, long previousActiveGeneration) { - this(deployedByUser, - deployedFromDir, - deployTimestamp, - internalRedeploy, - ApplicationId.from(TenantName.defaultName(), ApplicationName.from(applicationName), InstanceName.from("default")), - checksum, - generation, - previousActiveGeneration); - } - public ApplicationMetaData(String deployedByUser, String deployedFromDir, Long deployTimestamp, boolean internalRedeploy, ApplicationId applicationId, String checksum, Long generation, long previousActiveGeneration) { this.deployedByUser = deployedByUser; @@ -66,15 +42,6 @@ public class ApplicationMetaData { } /** - * Gets the name of the application (name of the directory from which application was deployed. - * Will return null if a problem occurred while getting metadata - * - * @return application name - */ - // TODO: Remove after September 2019 - public String getApplicationName() { return applicationId.application().toString(); } - - /** * Gets the user who deployed the application. * Will return null if a problem occurred while getting metadata * @@ -117,10 +84,6 @@ public class ApplicationMetaData { public boolean isInternalRedeploy() { return internalRedeploy; } /** Returns an md5 hash of the contents of the application package */ - // TODO: Remove after September 2019 - public String getCheckSum() { return checksum; } - - /** Returns an md5 hash of the contents of the application package */ public String getChecksum() { return checksum; } /** Returns the previously active generation at the point when this application was created. */ @@ -140,18 +103,11 @@ public class ApplicationMetaData { Inspector deploy = root.field("deploy"); Inspector app = root.field("application"); - // TODO: Simplify to just ApplicationId.fromSerializedForm(app.field("id").asString()) after September 2019 - ApplicationId applicationId = app.field("id").valid() ? - ApplicationId.fromSerializedForm(app.field("id").asString()) : - ApplicationId.from(TenantName.defaultName(), - ApplicationName.from(app.field("name").asString()), - InstanceName.from("default")); - return new ApplicationMetaData(deploy.field("user").asString(), deploy.field("from").asString(), deploy.field("timestamp").asLong(), booleanField("internalRedeploy", false, deploy), - applicationId, + ApplicationId.fromSerializedForm(app.field("id").asString()), app.field("checksum").asString(), app.field("generation").asLong(), app.field("previousActiveGeneration").asLong()); @@ -170,7 +126,6 @@ public class ApplicationMetaData { deploy.setBool("internalRedeploy", internalRedeploy); Cursor app = meta.setObject("application"); app.setString("id", applicationId.serializedForm()); - app.setString("name", applicationId.application().value()); // TODO: Remove after September 2019 app.setString("checksum", checksum); app.setLong("generation", generation); app.setLong("previousActiveGeneration", previousActiveGeneration); @@ -188,7 +143,7 @@ public class ApplicationMetaData { ByteArrayOutputStream baos = new ByteArrayOutputStream(); try { new JsonFormat(false).encode(baos, slime); - return baos.toString("UTF-8"); + return baos.toString(StandardCharsets.UTF_8); } catch (IOException e) { throw new RuntimeException("Unable to encode metadata", e); } diff --git a/config-model/src/main/java/com/yahoo/config/model/test/MockRoot.java b/config-model/src/main/java/com/yahoo/config/model/test/MockRoot.java index 13f271ebe9d..87d6554f691 100644 --- a/config-model/src/main/java/com/yahoo/config/model/test/MockRoot.java +++ b/config-model/src/main/java/com/yahoo/config/model/test/MockRoot.java @@ -26,6 +26,7 @@ import java.io.IOException; import java.io.StringReader; import java.util.ArrayList; import java.util.Collections; +import java.util.List; import java.util.Set; @@ -66,7 +67,7 @@ public class MockRoot extends AbstractConfigProducerRoot { super(rootConfigId); hostSystem = new HostSystem(this, "hostsystem", deployState.getProvisioner(), deployState.getDeployLogger()); this.deployState = deployState; - fileDistributor = new FileDistributor(deployState.getFileRegistry(), null); + fileDistributor = new FileDistributor(deployState.getFileRegistry(), List.of(), deployState.isHosted()); } public FileDistributionConfigProducer getFileDistributionConfigProducer() { diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/IndexInfo.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/IndexInfo.java index a631aa19968..da25680ca47 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/IndexInfo.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/IndexInfo.java @@ -43,6 +43,7 @@ public class IndexInfo extends Derived implements IndexInfoConfig.Producer { private static final String CMD_PLAIN_TOKENS = "plain-tokens"; private static final String CMD_MULTIVALUE = "multivalue"; private static final String CMD_FAST_SEARCH = "fast-search"; + private static final String CMD_PREDICATE = "predicate"; private static final String CMD_PREDICATE_BOUNDS = "predicate-bounds"; private static final String CMD_NUMERICAL = "numerical"; private static final String CMD_PHRASE_SEGMENTING = "phrase-segmenting"; @@ -100,6 +101,7 @@ public class IndexInfo extends Derived implements IndexInfoConfig.Producer { protected void derive(ImmutableSDField field, Search search, boolean inPosition) { if (field.getDataType().equals(DataType.PREDICATE)) { + addIndexCommand(field, CMD_PREDICATE); Index index = field.getIndex(field.getName()); if (index != null) { BooleanIndexDefinition options = index.getBooleanIndexDefiniton(); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java b/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java index 72c475fb091..75b33e184d9 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java @@ -461,6 +461,7 @@ public abstract class AbstractService extends AbstractConfigProducer<AbstractCon public FileReference sendFile(String relativePath) { return getRoot().getFileDistributor().sendFileToHost(relativePath, getHost()); } + public FileReference sendUri(String uri) { return getRoot().getFileDistributor().sendUriToHost(uri, getHost()); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java b/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java index 89cd5c2e735..6f2123f248c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java @@ -209,7 +209,8 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri /** Creates a mutable model with no services instantiated */ public static VespaModel createIncomplete(DeployState deployState) throws IOException, SAXException { - return new VespaModel(new NullConfigModelRegistry(), deployState, false, new FileDistributor(deployState.getFileRegistry(), null)); + return new VespaModel(new NullConfigModelRegistry(), deployState, false, + new FileDistributor(deployState.getFileRegistry(), List.of(), deployState.isHosted())); } private void validateWrapExceptions() { 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 09128e741b9..141794256b8 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 @@ -186,6 +186,7 @@ public class VespaMetricSet { metrics.add(new Metric("jdisc.http.request.content_size.average")); // TODO: Remove in Vespa 8 metrics.add(new Metric("jdisc.http.ssl.handshake.failure.missing_client_cert.rate")); + metrics.add(new Metric("jdisc.http.ssl.handshake.failure.expired_client_cert.rate")); metrics.add(new Metric("jdisc.http.ssl.handshake.failure.invalid_client_cert.rate")); metrics.add(new Metric("jdisc.http.ssl.handshake.failure.incompatible_protocols.rate")); metrics.add(new Metric("jdisc.http.ssl.handshake.failure.incompatible_ciphers.rate")); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminBuilderBase.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminBuilderBase.java index a146e2cd7e7..d2f06da992c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminBuilderBase.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminBuilderBase.java @@ -71,7 +71,7 @@ public abstract class DomAdminBuilderBase extends VespaDomBuilder.DomConfigProdu Monitoring monitoring = getMonitoring(XML.getChild(adminElement,"monitoring"), deployState.isHosted()); Metrics metrics = new MetricsBuilder(applicationType, predefinedMetricSets) .buildMetrics(XML.getChild(adminElement, "metrics")); - FileDistributionConfigProducer fileDistributionConfigProducer = getFileDistributionConfigProducer(parent); + FileDistributionConfigProducer fileDistributionConfigProducer = getFileDistributionConfigProducer(parent, deployState.isHosted()); Admin admin = new Admin(parent, monitoring, metrics, multitenant, fileDistributionConfigProducer, deployState.isHosted()); admin.setApplicationType(applicationType); @@ -81,8 +81,8 @@ public abstract class DomAdminBuilderBase extends VespaDomBuilder.DomConfigProdu return admin; } - private FileDistributionConfigProducer getFileDistributionConfigProducer(AbstractConfigProducer parent) { - return new FileDistributionConfigProducer(parent, fileRegistry, configServerSpecs); + private FileDistributionConfigProducer getFileDistributionConfigProducer(AbstractConfigProducer parent, boolean isHosted) { + return new FileDistributionConfigProducer(parent, fileRegistry, configServerSpecs, isHosted); } protected abstract void doBuildAdmin(DeployState deployState, Admin admin, Element adminE); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributionConfigProducer.java b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributionConfigProducer.java index 9662540e8df..2c6808b5773 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributionConfigProducer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributionConfigProducer.java @@ -20,8 +20,9 @@ public class FileDistributionConfigProducer extends AbstractConfigProducer { private final Map<Host, FileDistributionConfigProvider> fileDistributionConfigProviders = new IdentityHashMap<>(); private final FileDistributor fileDistributor; - public FileDistributionConfigProducer(AbstractConfigProducer ancestor, FileRegistry fileRegistry, List<ConfigServerSpec> configServerSpec) { - this(ancestor, new FileDistributor(fileRegistry, configServerSpec)); + public FileDistributionConfigProducer(AbstractConfigProducer ancestor, FileRegistry fileRegistry, + List<ConfigServerSpec> configServerSpec, boolean isHosted) { + this(ancestor, new FileDistributor(fileRegistry, configServerSpec, isHosted)); } private FileDistributionConfigProducer(AbstractConfigProducer parent, FileDistributor fileDistributor) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java index 576b009c846..5ccf86f9ba8 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java @@ -11,7 +11,10 @@ import com.yahoo.vespa.model.Host; import java.util.*; /** - * Responsible for directing distribution of files to hosts. + * Sends RPC requests to hosts (tenant hosts and config servers) to start download of files. This is used during prepare + * of an application. Services themselves will also request files, the work done in this class is done so that hosts can + * start downloading files before services gets new config that needs these files. This also tries to make sure that + * all config servers (not just the one where the application was deployed) have the files available. * * @author Tony Vaagenes */ @@ -19,61 +22,42 @@ public class FileDistributor { private final FileRegistry fileRegistry; private final List<ConfigServerSpec> configServerSpecs; + private final boolean isHosted; - /** A map from files to the hosts to which that file should be distributed */ + /** A map from file reference to the hosts to which that file reference should be distributed */ private final Map<FileReference, Set<Host>> filesToHosts = new LinkedHashMap<>(); + public FileDistributor(FileRegistry fileRegistry, List<ConfigServerSpec> configServerSpecs, boolean isHosted) { + this.fileRegistry = fileRegistry; + this.configServerSpecs = configServerSpecs; + this.isHosted = isHosted; + } + /** * Adds the given file to the associated application packages' registry of file and marks the file - * for distribution to the given hosts. + * for distribution to the given host. * <b>Note: This class receives ownership of the given collection.</b> * * @return the reference to the file, created by the application package */ - public FileReference sendFileToHosts(String relativePath, Collection<Host> hosts) { - FileReference reference = fileRegistry.addFile(relativePath); - addToFilesToDistribute(reference, hosts); - - return reference; + public FileReference sendFileToHost(String relativePath, Host host) { + return addFileReference(fileRegistry.addFile(relativePath), host); } /** * Adds the given file to the associated application packages' registry of file and marks the file - * for distribution to the given hosts. + * for distribution to the given host. * <b>Note: This class receives ownership of the given collection.</b> * * @return the reference to the file, created by the application package */ - public FileReference sendUriToHosts(String uri, Collection<Host> hosts) { - FileReference reference = fileRegistry.addUri(uri); - if (reference != null) { - addToFilesToDistribute(reference, hosts); - } - - return reference; - } - - /** Same as sendFileToHost(relativePath,Collections.singletonList(host) */ - public FileReference sendFileToHost(String relativePath, Host host) { - return sendFileToHosts(relativePath, Arrays.asList(host)); - } - public FileReference sendUriToHost(String uri, Host host) { - return sendUriToHosts(uri, Arrays.asList(host)); - } - - private void addToFilesToDistribute(FileReference reference, Collection<Host> hosts) { - Set<Host> oldHosts = getHosts(reference); - oldHosts.addAll(hosts); - } - - private Set<Host> getHosts(FileReference reference) { - return filesToHosts.computeIfAbsent(reference, k -> new HashSet<>()); + return addFileReference(fileRegistry.addUri(uri), host); } - public FileDistributor(FileRegistry fileRegistry, List<ConfigServerSpec> configServerSpecs) { - this.fileRegistry = fileRegistry; - this.configServerSpecs = configServerSpecs; + private FileReference addFileReference(FileReference reference, Host host) { + filesToHosts.computeIfAbsent(reference, k -> new HashSet<>()).add(host); + return reference; } /** Returns the files which has been marked for distribution to the given host */ @@ -107,16 +91,20 @@ public class FileDistributor { // should only be called during deploy public void sendDeployedFiles(FileDistribution dbHandler) { String fileSourceHost = fileSourceHost(); - for (Host host : getTargetHosts()) { - if ( ! host.getHostname().equals(fileSourceHost)) { - dbHandler.startDownload(host.getHostname(), ConfigProxy.BASEPORT, filesToSendToHost(host)); - } - } + // Ask other config servers to download, for redundancy - if (configServerSpecs != null) - configServerSpecs.stream() - .filter(configServerSpec -> !configServerSpec.getHostName().equals(fileSourceHost)) - .forEach(spec -> dbHandler.startDownload(spec.getHostName(), spec.getConfigServerPort(), allFilesToSend())); + configServerSpecs.stream() + .filter(spec -> !spec.getHostName().equals(fileSourceHost)) + .forEach(spec -> dbHandler.startDownload(spec.getHostName(), spec.getConfigServerPort(), allFilesToSend())); + + // Skip starting download for application hosts when on hosted, since this is just a hint and requests for files + // will fail until the application is activated (this call is done when preparing an application deployment) + // due to authorization of RPC requests on config servers only considering files belonging to active applications + if (isHosted) return; + + getTargetHosts().stream() + .filter(host -> ! host.getHostname().equals(fileSourceHost)) + .forEach(host -> dbHandler.startDownload(host.getHostname(), ConfigProxy.BASEPORT, filesToSendToHost(host))); } } diff --git a/config-model/src/test/derived/predicate_attribute/index-info.cfg b/config-model/src/test/derived/predicate_attribute/index-info.cfg index 3d9f57dd84b..4ebac65e1f5 100644 --- a/config-model/src/test/derived/predicate_attribute/index-info.cfg +++ b/config-model/src/test/derived/predicate_attribute/index-info.cfg @@ -4,6 +4,8 @@ indexinfo[].command[].command "index" indexinfo[].command[].indexname "sddocname" indexinfo[].command[].command "word" indexinfo[].command[].indexname "some_predicate_field" +indexinfo[].command[].command "predicate" +indexinfo[].command[].indexname "some_predicate_field" indexinfo[].command[].command "predicate-bounds [3..200]" indexinfo[].command[].indexname "some_predicate_field" indexinfo[].command[].command "index" diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterUtils.java b/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterUtils.java index db9153fcf23..b1550d90f35 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterUtils.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterUtils.java @@ -60,7 +60,7 @@ public class ContentClusterUtils { public static ContentCluster createCluster(String clusterXml, MockRoot root) { Document doc = XML.getDocument(clusterXml); Admin admin = new Admin(root, new DefaultMonitoring("vespa", 60), new Metrics(), false, - new FileDistributionConfigProducer(root, new MockFileRegistry(), null), + new FileDistributionConfigProducer(root, new MockFileRegistry(), List.of(), false), root.getDeployState().isHosted()); ConfigModelContext context = ConfigModelContext.create(null, root.getDeployState(), null,null, root, null); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/FileDistributorTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/FileDistributorTestCase.java index 131a5344116..55672c5aa16 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/FileDistributorTestCase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/FileDistributorTestCase.java @@ -10,31 +10,34 @@ import org.junit.Test; import java.io.File; import java.util.Arrays; import java.util.HashSet; +import java.util.List; import java.util.Set; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertNotNull; /** * @author bratseth */ public class FileDistributorTestCase { + @Test public void fileDistributor() { MockHosts hosts = new MockHosts(); - FileDistributor fileDistributor = new FileDistributor(new MockFileRegistry(), null); + FileDistributor fileDistributor = new FileDistributor(new MockFileRegistry(), List.of(), false); String file1 = "component/path1"; String file2 = "component/path2"; - FileReference ref1 = fileDistributor.sendFileToHosts(file1, Arrays.asList(hosts.host1, hosts.host2)); - FileReference ref2 = fileDistributor.sendFileToHosts(file2, Arrays.asList(hosts.host3)); + FileReference ref1 = fileDistributor.sendFileToHost(file1, hosts.host1); + fileDistributor.sendFileToHost(file1, hosts.host2); // same file reference as above + FileReference ref2 = fileDistributor.sendFileToHost(file2, hosts.host3); assertEquals(new HashSet<>(Arrays.asList(hosts.host1, hosts.host2, hosts.host3)), fileDistributor.getTargetHosts()); - assertTrue( ref1 != null ); - assertTrue( ref2 != null ); + assertNotNull(ref1); + assertNotNull(ref2); MockFileDistribution dbHandler = new MockFileDistribution(); fileDistributor.sendDeployedFiles(dbHandler); @@ -54,4 +57,5 @@ public class FileDistributorTestCase { return null; } } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java index 7ec0f49ed60..7ff5d41485d 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java @@ -21,10 +21,8 @@ import org.apache.curator.framework.recipes.cache.PathChildrenCacheEvent; import java.time.Duration; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; import java.util.logging.Logger; @@ -49,7 +47,6 @@ public class TenantApplications { private final Path locksPath; private final Curator.DirectoryCache directoryCache; private final ReloadHandler reloadHandler; - private final Map<ApplicationId, Lock> locks; private final Executor zkWatcherExecutor; private TenantApplications(Curator curator, ReloadHandler reloadHandler, TenantName tenant, @@ -57,7 +54,6 @@ public class TenantApplications { this.curator = curator; this.applicationsPath = TenantRepository.getApplicationsPath(tenant); this.locksPath = TenantRepository.getLocksPath(tenant); - this.locks = new ConcurrentHashMap<>(2); this.reloadHandler = reloadHandler; this.zkWatcherExecutor = command -> zkWatcherExecutor.execute(tenant, command); this.directoryCache = curator.createDirectoryCache(applicationsPath.getAbsolute(), false, false, zkCacheExecutor); @@ -148,10 +144,7 @@ public class TenantApplications { /** Returns the lock for changing the session status of the given application. */ public Lock lock(ApplicationId id) { - curator.create(lockPath(id)); - Lock lock = locks.computeIfAbsent(id, __ -> new Lock(lockPath(id).getAbsolute(), curator)); - lock.acquire(Duration.ofMinutes(1)); // These locks shouldn't be held for very long. - return lock; + return curator.lock(lockPath(id), Duration.ofMinutes(1)); // These locks shouldn't be held for very long. } private void childEvent(CuratorFramework client, PathChildrenCacheEvent event) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java index b3c5b38b9ba..82dc17c3678 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java @@ -131,10 +131,12 @@ public class Deployment implements com.yahoo.config.provision.Deployment { TimeoutBudget timeoutBudget = new TimeoutBudget(clock, timeout); ApplicationId applicationId = session.getApplicationId(); + LocalSession previousActiveSession; try (Lock lock = tenant.getApplicationRepo().lock(applicationId)) { validateSessionStatus(session); NestedTransaction transaction = new NestedTransaction(); - transaction.add(deactivateCurrentActivateNew(applicationRepository.getActiveSession(applicationId), session, ignoreSessionStaleFailure)); + previousActiveSession = applicationRepository.getActiveSession(applicationId); + transaction.add(deactivateCurrentActivateNew(previousActiveSession, session, ignoreSessionStaleFailure)); hostProvisioner.ifPresent(provisioner -> provisioner.activate(transaction, applicationId, session.getAllocatedHosts().getHosts())); transaction.commit(); } @@ -151,6 +153,7 @@ public class Deployment implements com.yahoo.config.provision.Deployment { " activated successfully using " + (hostProvisioner.isPresent() ? hostProvisioner.get().getClass().getSimpleName() : "no host provisioner") + ". Config generation " + session.getMetaData().getGeneration() + + (previousActiveSession != null ? ". Activated session based on previous active session " + previousActiveSession.getSessionId() : "") + ". File references used: " + applicationRepository.getFileReferences(applicationId)); } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/Maintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/Maintainer.java index d2fd4efa7dc..e40d9153ad1 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/Maintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/Maintainer.java @@ -48,7 +48,7 @@ public abstract class Maintainer extends AbstractComponent implements Runnable { @Override @SuppressWarnings({"try", "unused"}) public void run() { - try (Lock lock = lock(lockRoot.append(name()))) { + try (Lock lock = curator.lock(lockRoot.append(name()), Duration.ofSeconds(1))) { maintain(); } catch (UncheckedTimeoutException e) { // another config server instance is running this job at the moment; ok @@ -57,12 +57,6 @@ public abstract class Maintainer extends AbstractComponent implements Runnable { } } - private Lock lock(Path path) { - Lock lock = new Lock(path.getAbsolute(), curator); - lock.acquire(Duration.ofSeconds(1)); - return lock; - } - @Override public void deconstruct() { this.service.shutdown(); 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 d730b21762d..d57e2389cb1 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 @@ -90,7 +90,7 @@ public class ZKApplicationPackageTest { private void feed(ConfigCurator zk, File dirToFeed) throws IOException { assertTrue(dirToFeed.isDirectory()); zk.feedZooKeeper(dirToFeed, "/0" + ConfigCurator.USERAPP_ZK_SUBPATH, null, true); - String metaData = "{\"deploy\":{\"user\":\"foo\",\"from\":\"bar\",\"timestamp\":1},\"application\":{\"name\":\"foo\",\"checksum\":\"abc\",\"generation\":4,\"previousActiveGeneration\":3}}"; + String metaData = "{\"deploy\":{\"user\":\"foo\",\"from\":\"bar\",\"timestamp\":1},\"application\":{\"id\":\"foo:foo:default\",\"checksum\":\"abc\",\"generation\":4,\"previousActiveGeneration\":3}}"; zk.putData("/0", ConfigCurator.META_ZK_PATH, metaData); zk.putData("/0/" + ZKApplicationPackage.fileRegistryNode + "/3.0.0", "dummyfiles"); zk.putData("/0/" + ZKApplicationPackage.allocatedHostsNode, toJson(ALLOCATED_HOSTS)); diff --git a/container-disc/abi-spec.json b/container-disc/abi-spec.json index 81de014c6ad..d3ad495ff71 100644 --- a/container-disc/abi-spec.json +++ b/container-disc/abi-spec.json @@ -17,7 +17,8 @@ "public abstract java.lang.String getAccessToken(java.lang.String)", "public abstract java.lang.String getAccessToken(java.lang.String, java.util.List)", "public abstract java.util.List getIdentityCertificate()", - "public abstract java.security.PrivateKey getPrivateKey()" + "public abstract java.security.PrivateKey getPrivateKey()", + "public abstract java.nio.file.Path trustStorePath()" ], "fields": [] }, diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/AthenzIdentityProviderProvider.java b/container-disc/src/main/java/com/yahoo/container/jdisc/AthenzIdentityProviderProvider.java index bb862aeca82..0e3110e26a8 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/AthenzIdentityProviderProvider.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/AthenzIdentityProviderProvider.java @@ -5,6 +5,7 @@ import com.yahoo.container.di.componentgraph.Provider; import com.yahoo.container.jdisc.athenz.AthenzIdentityProvider; import javax.net.ssl.SSLContext; +import java.nio.file.Path; import java.security.PrivateKey; import java.security.cert.X509Certificate; import java.util.List; @@ -77,5 +78,10 @@ public class AthenzIdentityProviderProvider implements Provider<AthenzIdentityPr public PrivateKey getPrivateKey() { throw new UnsupportedOperationException(message); } + + @Override + public Path trustStorePath() { + throw new UnsupportedOperationException(message); + } } } diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/AthenzIdentityProvider.java b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/AthenzIdentityProvider.java index 696aab85b0c..10bf96749e8 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/AthenzIdentityProvider.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/AthenzIdentityProvider.java @@ -2,6 +2,7 @@ package com.yahoo.container.jdisc.athenz; import javax.net.ssl.SSLContext; +import java.nio.file.Path; import java.security.PrivateKey; import java.security.cert.X509Certificate; import java.util.List; @@ -20,4 +21,5 @@ public interface AthenzIdentityProvider { String getAccessToken(String domain, List<String> roles); List<X509Certificate> getIdentityCertificate(); PrivateKey getPrivateKey(); + Path trustStorePath(); } diff --git a/container-search/src/main/java/com/yahoo/prelude/Index.java b/container-search/src/main/java/com/yahoo/prelude/Index.java index 5e7fddd7fe7..0dfbf6470ad 100644 --- a/container-search/src/main/java/com/yahoo/prelude/Index.java +++ b/container-search/src/main/java/com/yahoo/prelude/Index.java @@ -63,6 +63,7 @@ public class Index { private boolean normalize = false; private boolean literalBoost = false; private boolean numerical = false; + private boolean predicate = false; private long predicateUpperBound = Long.MAX_VALUE; private long predicateLowerBound = Long.MIN_VALUE; @@ -181,6 +182,8 @@ public class Index { setLiteralBoost(true); } else if (commandString.equals("numerical")) { setNumerical(true); + } else if (commandString.equals("predicate")) { + setPredicate(true); } else if (commandString.startsWith("predicate-bounds ")) { setPredicateBounds(commandString.substring(17)); } else if (commandString.equals("phrase-segmenting")) { @@ -304,6 +307,10 @@ public class Index { public boolean isNumerical() { return numerical; } + public void setPredicate(boolean isPredicate) { this.predicate = isPredicate; } + + public boolean isPredicate() { return predicate; } + public long getPredicateUpperBound() { return predicateUpperBound; } public long getPredicateLowerBound() { return predicateLowerBound; } diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/ValidatePredicateSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/ValidatePredicateSearcher.java index 9b6f5926b61..a8b3c76fe00 100644 --- a/container-search/src/main/java/com/yahoo/prelude/searcher/ValidatePredicateSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/searcher/ValidatePredicateSearcher.java @@ -1,12 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude.searcher; -import java.util.Optional; import com.yahoo.component.chain.dependencies.After; import com.yahoo.prelude.Index; import com.yahoo.prelude.IndexFacts; import com.yahoo.prelude.query.Item; import com.yahoo.prelude.query.PredicateQueryItem; +import com.yahoo.prelude.query.SimpleIndexedItem; import com.yahoo.prelude.query.ToolBox; import com.yahoo.search.Query; import com.yahoo.search.Result; @@ -15,7 +15,8 @@ import com.yahoo.search.querytransform.BooleanSearcher; import com.yahoo.search.result.ErrorMessage; import com.yahoo.search.searchchain.Execution; -import java.util.Collection; +import java.util.ArrayList; +import java.util.List; /** * Checks that predicate queries don't use values outside the defined upper/lower bounds. @@ -27,26 +28,26 @@ public class ValidatePredicateSearcher extends Searcher { @Override public Result search(Query query, Execution execution) { - Optional<ErrorMessage> e = validate(query, execution.context().getIndexFacts().newSession(query)); - if (e.isPresent()) { + List<ErrorMessage> errorMessages = validate(query, execution.context().getIndexFacts().newSession(query)); + if (!errorMessages.isEmpty()) { Result r = new Result(query); - r.hits().addError(e.get()); + errorMessages.forEach(msg -> r.hits().addError(msg)); return r; } return execution.search(query); } - private Optional<ErrorMessage> validate(Query query, IndexFacts.Session indexFacts) { + private List<ErrorMessage> validate(Query query, IndexFacts.Session indexFacts) { ValidatePredicateVisitor visitor = new ValidatePredicateVisitor(indexFacts); ToolBox.visit(visitor, query.getModel().getQueryTree().getRoot()); - return visitor.errorMessage; + return visitor.errorMessages; } private static class ValidatePredicateVisitor extends ToolBox.QueryVisitor { private final IndexFacts.Session indexFacts; - public Optional<ErrorMessage> errorMessage = Optional.empty(); + final List<ErrorMessage> errorMessages = new ArrayList<>(); public ValidatePredicateVisitor(IndexFacts.Session indexFacts) { this.indexFacts = indexFacts; @@ -57,22 +58,37 @@ public class ValidatePredicateSearcher extends Searcher { if (item instanceof PredicateQueryItem) { visit((PredicateQueryItem) item); } + if (item instanceof SimpleIndexedItem) { + visit((SimpleIndexedItem) item); + } return true; } private void visit(PredicateQueryItem item) { - Index index = getIndexFromUnionOfDocumentTypes(item); + Index index = getIndexFromUnionOfDocumentTypes(item.getIndexName()); + if (!index.isPredicate()) { + errorMessages.add(ErrorMessage.createIllegalQuery(String.format("Index '%s' is not a predicate attribute.", index.getName()))); + } for (PredicateQueryItem.RangeEntry entry : item.getRangeFeatures()) { long value = entry.getValue(); if (value < index.getPredicateLowerBound() || value > index.getPredicateUpperBound()) { - errorMessage = Optional.of(ErrorMessage.createIllegalQuery( - String.format("%s=%d outside configured predicate bounds.", entry.getKey(), value))); + errorMessages.add( + ErrorMessage.createIllegalQuery(String.format("%s=%d outside configured predicate bounds.", entry.getKey(), value))); } } } - private Index getIndexFromUnionOfDocumentTypes(PredicateQueryItem item) { - return indexFacts.getIndex(item.getIndexName()); + private void visit(SimpleIndexedItem item) { + String indexName = item.getIndexName(); + Index index = getIndexFromUnionOfDocumentTypes(indexName); + if (index.isPredicate()) { + errorMessages.add( + ErrorMessage.createIllegalQuery(String.format("Index '%s' is predicate attribute and can only be used in conjunction with a predicate query operator.", indexName))); + } + } + + private Index getIndexFromUnionOfDocumentTypes(String indexName) { + return indexFacts.getIndex(indexName); } @Override diff --git a/container-search/src/test/java/com/yahoo/prelude/searcher/test/ValidatePredicateSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/searcher/test/ValidatePredicateSearcherTestCase.java index 1b9ca1cd29b..2187cb89ae2 100644 --- a/container-search/src/test/java/com/yahoo/prelude/searcher/test/ValidatePredicateSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/searcher/test/ValidatePredicateSearcherTestCase.java @@ -2,7 +2,6 @@ package com.yahoo.prelude.searcher.test; import com.google.common.util.concurrent.MoreExecutors; -import com.yahoo.language.Linguistics; import com.yahoo.language.simple.SimpleLinguistics; import com.yahoo.prelude.Index; import com.yahoo.prelude.IndexFacts; @@ -20,8 +19,6 @@ import com.yahoo.search.searchchain.Execution; import com.yahoo.search.yql.YqlParser; import org.junit.Test; -import java.util.*; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; @@ -46,6 +43,14 @@ public class ValidatePredicateSearcherTestCase { assertEquals(ErrorMessage.createIllegalQuery("age=200 outside configured predicate bounds."), r.hits().getError()); } + @Test + public void queryFailsWhenPredicateFieldIsUsedInTermSearch() { + ValidatePredicateSearcher searcher = new ValidatePredicateSearcher(); + String q = "select * from sources * where predicate_field CONTAINS \"true\";"; + Result r = doSearch(searcher, q, "predicate-bounds [0..99]"); + assertEquals(ErrorMessage.createIllegalQuery("Index 'predicate_field' is predicate attribute and can only be used in conjunction with a predicate query operator."), r.hits().getError()); + } + private static Result doSearch(ValidatePredicateSearcher searcher, String yqlQuery, String command) { QueryTree queryTree = new YqlParser(new ParserEnvironment()).parse(new Parsable().setQuery(yqlQuery)); Query query = new Query(); @@ -53,6 +58,7 @@ public class ValidatePredicateSearcherTestCase { SearchDefinition searchDefinition = new SearchDefinition("document"); Index index = new Index("predicate_field"); + index.setPredicate(true); index.addCommand(command); searchDefinition.addIndex(index); IndexFacts indexFacts = new IndexFacts(new IndexModel(searchDefinition)); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index 4fdd66ee100..77cdd8d342d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -109,12 +109,6 @@ public class CuratorDb { // For each job id (path), store the ZK node version and its deserialised data — update when version changes. private final Map<Path, Pair<Integer, NavigableMap<RunId, Run>>> cachedHistoricRuns = new ConcurrentHashMap<>(); - /** - * All keys, to allow reentrancy. - * This will grow forever, but this should be too slow to be a problem. - */ - private final ConcurrentHashMap<Path, Lock> locks = new ConcurrentHashMap<>(); - @Inject public CuratorDb(Curator curator) { this(curator, defaultTryLockTimeout); @@ -136,28 +130,20 @@ public class CuratorDb { // -------------- Locks --------------------------------------------------- - /** Creates a reentrant lock */ - private Lock lock(Path path, Duration timeout) { - curator.create(path); - Lock lock = locks.computeIfAbsent(path, (pathArg) -> new Lock(pathArg.getAbsolute(), curator)); - lock.acquire(timeout); - return lock; - } - public Lock lock(TenantName name) { - return lock(lockPath(name), defaultLockTimeout.multipliedBy(2)); + return curator.lock(lockPath(name), defaultLockTimeout.multipliedBy(2)); } public Lock lock(TenantAndApplicationId id) { - return lock(lockPath(id), defaultLockTimeout.multipliedBy(2)); + return curator.lock(lockPath(id), defaultLockTimeout.multipliedBy(2)); } public Lock lockForDeployment(ApplicationId id, ZoneId zone) { - return lock(lockPath(id, zone), deployLockTimeout); + return curator.lock(lockPath(id, zone), deployLockTimeout); } public Lock lock(ApplicationId id, JobType type) { - return lock(lockPath(id, type), defaultLockTimeout); + return curator.lock(lockPath(id, type), defaultLockTimeout); } public Lock lock(ApplicationId id, JobType type, Step step) throws TimeoutException { @@ -165,15 +151,15 @@ public class CuratorDb { } public Lock lockRotations() { - return lock(lockRoot.append("rotations"), defaultLockTimeout); + return curator.lock(lockRoot.append("rotations"), defaultLockTimeout); } public Lock lockConfidenceOverrides() { - return lock(lockRoot.append("confidenceOverrides"), defaultLockTimeout); + return curator.lock(lockRoot.append("confidenceOverrides"), defaultLockTimeout); } public Lock lockInactiveJobs() { - return lock(lockRoot.append("inactiveJobsLock"), defaultLockTimeout); + return curator.lock(lockRoot.append("inactiveJobsLock"), defaultLockTimeout); } public Lock lockMaintenanceJob(String jobName) throws TimeoutException { @@ -182,27 +168,27 @@ public class CuratorDb { @SuppressWarnings("unused") // Called by internal code public Lock lockProvisionState(String provisionStateId) { - return lock(lockPath(provisionStateId), Duration.ofSeconds(1)); + return curator.lock(lockPath(provisionStateId), Duration.ofSeconds(1)); } public Lock lockOsVersions() { - return lock(lockRoot.append("osTargetVersion"), defaultLockTimeout); + return curator.lock(lockRoot.append("osTargetVersion"), defaultLockTimeout); } public Lock lockOsVersionStatus() { - return lock(lockRoot.append("osVersionStatus"), defaultLockTimeout); + return curator.lock(lockRoot.append("osVersionStatus"), defaultLockTimeout); } public Lock lockRoutingPolicies() { - return lock(lockRoot.append("routingPolicies"), defaultLockTimeout); + return curator.lock(lockRoot.append("routingPolicies"), defaultLockTimeout); } public Lock lockAuditLog() { - return lock(lockRoot.append("auditLog"), defaultLockTimeout); + return curator.lock(lockRoot.append("auditLog"), defaultLockTimeout); } public Lock lockNameServiceQueue() { - return lock(lockRoot.append("nameServiceQueue"), defaultLockTimeout); + return curator.lock(lockRoot.append("nameServiceQueue"), defaultLockTimeout); } // -------------- Helpers ------------------------------------------ @@ -213,7 +199,7 @@ public class CuratorDb { */ private Lock tryLock(Path path) throws TimeoutException { try { - return lock(path, tryLockTimeout); + return curator.lock(path, tryLockTimeout); } catch (UncheckedTimeoutException e) { throw new TimeoutException(e.getMessage()); 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 d7ad96ec5e7..455759a3b41 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 @@ -784,17 +784,6 @@ public class ApplicationApiHandler extends LoggingRequestHandler { if ( ! status.outstandingChange(instance.name()).isEmpty()) toSlime(object.setObject("outstandingChange"), status.outstandingChange(instance.name())); - Cursor deploymentJobsArray = object.setArray("deploymentJobs"); - for (JobStatus job : jobStatus) { - Cursor jobObject = deploymentJobsArray.addObject(); - jobObject.setString("type", job.id().type().jobName()); - jobObject.setBool("success", job.isSuccess()); - job.lastTriggered().ifPresent(jobRun -> toSlime(jobRun, jobObject.setObject("lastTriggered"))); - job.lastCompleted().ifPresent(jobRun -> toSlime(jobRun, jobObject.setObject("lastCompleted"))); - job.firstFailing().ifPresent(jobRun -> toSlime(jobRun, jobObject.setObject("firstFailing"))); - job.lastSuccess().ifPresent(jobRun -> toSlime(jobRun, jobObject.setObject("lastSuccess"))); - } - // Change blockers Cursor changeBlockers = object.setArray("changeBlockers"); deploymentSpec.instance(instance.name()).ifPresent(spec -> spec.changeBlocker().forEach(changeBlocker -> { @@ -896,18 +885,6 @@ public class ApplicationApiHandler extends LoggingRequestHandler { if ( ! status.outstandingChange(instance.name()).isEmpty()) toSlime(object.setObject("outstandingChange"), status.outstandingChange(instance.name())); - Cursor deploymentsArray = object.setArray("deploymentJobs"); - for (JobStatus job : jobStatus) { - Cursor jobObject = deploymentsArray.addObject(); - jobObject.setString("type", job.id().type().jobName()); - jobObject.setBool("success", job.isSuccess()); - - job.lastTriggered().ifPresent(jobRun -> toSlime(jobRun, jobObject.setObject("lastTriggered"))); - job.lastCompleted().ifPresent(jobRun -> toSlime(jobRun, jobObject.setObject("lastCompleted"))); - job.firstFailing().ifPresent(jobRun -> toSlime(jobRun, jobObject.setObject("firstFailing"))); - job.lastSuccess().ifPresent(jobRun -> toSlime(jobRun, jobObject.setObject("lastSuccess"))); - } - // Change blockers Cursor changeBlockers = object.setArray("changeBlockers"); application.deploymentSpec().instance(instance.name()).ifPresent(spec -> spec.changeBlocker().forEach(changeBlocker -> { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2-with-patches.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2-with-patches.json index 59c32bb2f1c..5c073173544 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2-with-patches.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2-with-patches.json @@ -39,109 +39,6 @@ "commit": "commit1" } }, - "deploymentJobs": [ - { - "type": "system-test", - "success": false, - "lastTriggered": { - "id": 1, - "version": "7.0.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - } - }, - { - "type": "staging-test", - "success": false, - "lastTriggered": { - "id": 1, - "version": "7.0.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - } - }, - { - "type": "production-us-west-1", - "success": true, - "lastTriggered": { - "id": 2, - "version": "7.0.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastCompleted": { - "id": 1, - "version": "7.0.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastSuccess": { - "id": 1, - "version": "7.0.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - } - }, - { - "type": "production-us-east-3", - "success": false - } - ], "changeBlockers": [], "globalRotations": [ "https://instance1--application2--tenant2.global.vespa.oath.cloud:4443/" diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2.json index 6934d81a8d9..01fd88a599f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2.json @@ -38,109 +38,6 @@ "commit": "commit1" } }, - "deploymentJobs": [ - { - "type": "system-test", - "success": false, - "lastTriggered": { - "id": 1, - "version": "7.0.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - } - }, - { - "type": "staging-test", - "success": false, - "lastTriggered": { - "id": 1, - "version": "7.0.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - } - }, - { - "type": "production-us-west-1", - "success": true, - "lastTriggered": { - "id": 2, - "version": "7.0.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastCompleted": { - "id": 1, - "version": "7.0.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastSuccess": { - "id": 1, - "version": "7.0.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - } - }, - { - "type": "production-us-east-3", - "success": false - } - ], "changeBlockers": [], "globalRotations": [ "https://instance1--application2--tenant2.global.vespa.oath.cloud:4443/" diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance-with-routing-policy.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance-with-routing-policy.json index e6b5dac0c37..33b1d95b5ca 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance-with-routing-policy.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance-with-routing-policy.json @@ -11,173 +11,6 @@ "sourceUrl": "repository1/tree/commit1", "commit": "commit1", "projectId": 1000, - "deploymentJobs": [ - { - "type": "system-test", - "success": true, - "lastTriggered": { - "id": 1, - "version": "(ignore)", - "revision": { - "buildNumber": "(ignore)", - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastCompleted": { - "id": 1, - "version": "(ignore)", - "revision": { - "buildNumber": "(ignore)", - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastSuccess": { - "id": 1, - "version": "(ignore)", - "revision": { - "buildNumber": "(ignore)", - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - } - }, - { - "type": "staging-test", - "success": true, - "lastTriggered": { - "id": 1, - "version": "(ignore)", - "revision": { - "buildNumber": "(ignore)", - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastCompleted": { - "id": 1, - "version": "(ignore)", - "revision": { - "buildNumber": "(ignore)", - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastSuccess": { - "id": 1, - "version": "(ignore)", - "revision": { - "buildNumber": "(ignore)", - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - } - }, - { - "type": "production-us-west-1", - "success": true, - "lastTriggered": { - "id": 1, - "version": "(ignore)", - "revision": { - "buildNumber": "(ignore)", - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastCompleted": { - "id": 1, - "version": "(ignore)", - "revision": { - "buildNumber": "(ignore)", - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastSuccess": { - "id": 1, - "version": "(ignore)", - "revision": { - "buildNumber": "(ignore)", - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - } - } - ], "changeBlockers": [], "globalRotations": [], "instances": [ diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance.json index 7b264c8e28e..1b2c0b4e237 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance.json @@ -24,210 +24,6 @@ "commit": "commit1" } }, - "deploymentJobs": [ - { - "type": "system-test", - "success": true, - "lastTriggered": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastCompleted": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastSuccess": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - } - }, - { - "type": "staging-test", - "success": true, - "lastTriggered": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastCompleted": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastSuccess": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - } - }, - { - "type": "production-us-central-1", - "success": true, - "lastTriggered": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastCompleted": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastSuccess": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - } - }, - { - "type": "production-us-east-3", - "success": true, - "lastTriggered": { - "id": 2, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastCompleted": { - "id": 1, - "version": "6.1.0", - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastSuccess": { - "id": 1, - "version": "6.1.0", - "reason": "unknown reason", - "at": "(ignore)" - } - }, - { - "type": "production-us-west-1", - "success": false - } - ], "changeBlockers": [ { "versions": true, diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance1-recursive.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance1-recursive.json index 389b07dcdfb..19676decdb8 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance1-recursive.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance1-recursive.json @@ -24,210 +24,6 @@ "commit": "commit1" } }, - "deploymentJobs": [ - { - "type": "system-test", - "success": true, - "lastTriggered": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastCompleted": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastSuccess": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - } - }, - { - "type": "staging-test", - "success": true, - "lastTriggered": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastCompleted": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastSuccess": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - } - }, - { - "type": "production-us-central-1", - "success": true, - "lastTriggered": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastCompleted": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastSuccess": { - "id": 1, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - } - }, - { - "type": "production-us-east-3", - "success": true, - "lastTriggered": { - "id": 2, - "version": "6.1.0", - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" - }, - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastCompleted": { - "id": 1, - "version": "6.1.0", - "reason": "unknown reason", - "at": "(ignore)" - }, - "lastSuccess": { - "id": 1, - "version": "6.1.0", - "reason": "unknown reason", - "at": "(ignore)" - } - }, - { - "type": "production-us-west-1", - "success": false - } - ], "changeBlockers": [ { "versions": true, diff --git a/document/src/main/java/com/yahoo/document/idstring/IdIdString.java b/document/src/main/java/com/yahoo/document/idstring/IdIdString.java index 4c7f71dd712..28573296370 100644 --- a/document/src/main/java/com/yahoo/document/idstring/IdIdString.java +++ b/document/src/main/java/com/yahoo/document/idstring/IdIdString.java @@ -47,6 +47,10 @@ public class IdIdString extends IdString { super(Scheme.id, namespace, localId); this.type = type; boolean hasSetLocation = false; + if (namespace.length() + type.length() + keyValues.length() + 5 >= IdString.MAX_LENGTH_EXCEPT_NAMESPACE_SPECIFIC) { + throw new IllegalArgumentException("Length of namespace(" + namespace.length() + ") + doctype(" + type.length() + + ") + key/values(" + keyValues.length() +"), is longer than " + (MAX_LENGTH_EXCEPT_NAMESPACE_SPECIFIC - 5)); + } for(String pair : keyValues.split(",")) { int pos = pair.indexOf('='); if (pos == -1) { diff --git a/document/src/main/java/com/yahoo/document/idstring/IdString.java b/document/src/main/java/com/yahoo/document/idstring/IdString.java index 0fe382be914..d25c39f3b44 100644 --- a/document/src/main/java/com/yahoo/document/idstring/IdString.java +++ b/document/src/main/java/com/yahoo/document/idstring/IdString.java @@ -42,6 +42,7 @@ public abstract class IdString { private final String namespace; private final String namespaceSpecific; private Utf8String cache; + static final int MAX_LENGTH_EXCEPT_NAMESPACE_SPECIFIC = 0xfffe; /** * Creates a IdString based on the given document id string. @@ -115,6 +116,8 @@ public abstract class IdString { colonPos = id.indexOf(":", currPos); if (colonPos < 0) { throw new IllegalArgumentException("Unparseable id '" + id + "': Key/value section missing"); + } else if (colonPos >= MAX_LENGTH_EXCEPT_NAMESPACE_SPECIFIC) { + throw new IllegalArgumentException("Document id prior to the namespace specific part, " + colonPos + ", is longer than " + MAX_LENGTH_EXCEPT_NAMESPACE_SPECIFIC + " id: " + id); } String keyValues = id.substring(currPos, colonPos); diff --git a/document/src/test/java/com/yahoo/document/IdIdStringTest.java b/document/src/test/java/com/yahoo/document/IdIdStringTest.java index 88b5f38ec0c..b321d91e6e9 100644 --- a/document/src/test/java/com/yahoo/document/IdIdStringTest.java +++ b/document/src/test/java/com/yahoo/document/IdIdStringTest.java @@ -2,9 +2,11 @@ package com.yahoo.document; import com.yahoo.document.idstring.IdIdString; +import com.yahoo.document.idstring.IdString; import org.junit.Test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; /** @@ -50,6 +52,7 @@ public class IdIdStringTest { new IdIdString("namespace", "type", "illegal=key", "foo"); fail(); } catch (IllegalArgumentException e) { + assertEquals("Illegal key 'illegal'", e.getMessage()); } } @@ -59,6 +62,35 @@ public class IdIdStringTest { new IdIdString("namespace", "type", "illegal-pair", "foo"); fail(); } catch (IllegalArgumentException e) { + assertEquals("Illegal key-value pair 'illegal-pair'", e.getMessage()); + } + } + + @Test + public void requireThatTooLongPreNamspaceSpecificThrowsWhileParsing() throws Exception { + StringBuilder builder = new StringBuilder("id:"); + for (int i = 0; i < 0x10000; i++) { + builder.append('n'); + } + builder.append(":type::namespacespecificpart_01"); + try { + IdString.createIdString(builder.toString()); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("Document id prior to the namespace specific part, 65545, is longer than 65534", e.getMessage().substring(0, 77)); + } + } + @Test + public void requireThatTooLongPreNamspaceSpecificThrowsOnConstruction() { + StringBuilder builder = new StringBuilder(); + for (int i = 0; i < 0x10000; i++) { + builder.append('n'); + } + try { + new IdIdString(builder.toString(), "type", "", "namespacespecificpart_01"); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("Length of namespace(65536) + doctype(4) + key/values(0), is longer than 65529", e.getMessage()); } } diff --git a/fastos/src/vespa/fastos/CMakeLists.txt b/fastos/src/vespa/fastos/CMakeLists.txt index 1437f5c55f3..f062432d967 100644 --- a/fastos/src/vespa/fastos/CMakeLists.txt +++ b/fastos/src/vespa/fastos/CMakeLists.txt @@ -4,6 +4,7 @@ vespa_add_library(fastos_objects OBJECT app.cpp backtrace.c file.cpp + file_rw_ops.cpp linux_file.cpp process.cpp thread.cpp diff --git a/fastos/src/vespa/fastos/file_rw_ops.cpp b/fastos/src/vespa/fastos/file_rw_ops.cpp new file mode 100644 index 00000000000..f5d81eab1cf --- /dev/null +++ b/fastos/src/vespa/fastos/file_rw_ops.cpp @@ -0,0 +1,13 @@ +// Copyright 2020 Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "file_rw_ops.h" +#include <unistd.h> + +namespace fastos { + +File_RW_Ops::ReadFunc File_RW_Ops::_read = ::read; +File_RW_Ops::WriteFunc File_RW_Ops::_write = ::write; +File_RW_Ops::PreadFunc File_RW_Ops::_pread = ::pread; +File_RW_Ops::PwriteFunc File_RW_Ops::_pwrite = ::pwrite; + +} diff --git a/fastos/src/vespa/fastos/file_rw_ops.h b/fastos/src/vespa/fastos/file_rw_ops.h new file mode 100644 index 00000000000..9328bdbf9b4 --- /dev/null +++ b/fastos/src/vespa/fastos/file_rw_ops.h @@ -0,0 +1,33 @@ +// Copyright 2020 Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <sys/types.h> + + +namespace fastos { + +/* + * Class handling pointers to functions used by FastOS_File for read + * and writa access. Unit tests might modify pointers to inject errors. + */ +class File_RW_Ops +{ + using ReadFunc = ssize_t (*)(int fd, void* buf, size_t count); + using WriteFunc = ssize_t (*)(int fd, const void* buf, size_t count); + using PreadFunc = ssize_t (*)(int fd, void* buf, size_t count, off_t offset); + using PwriteFunc = ssize_t (*)(int fd, const void* buf, size_t count, off_t offset); + +public: + static ReadFunc _read; + static WriteFunc _write; + static PreadFunc _pread; + static PwriteFunc _pwrite; + + static ssize_t read(int fd, void* buf, size_t count) { return _read(fd, buf, count); } + static ssize_t write(int fd, const void* buf, size_t count) { return _write(fd, buf, count); } + static ssize_t pread(int fd, void* buf, size_t count, off_t offset) { return _pread(fd, buf, count, offset); } + static ssize_t pwrite(int fd, const void* buf, size_t count, off_t offset) { return _pwrite(fd, buf, count, offset); } +}; + +} diff --git a/fastos/src/vespa/fastos/linux_file.cpp b/fastos/src/vespa/fastos/linux_file.cpp index b4acaaa6073..56d9e246a97 100644 --- a/fastos/src/vespa/fastos/linux_file.cpp +++ b/fastos/src/vespa/fastos/linux_file.cpp @@ -12,6 +12,9 @@ #include <sstream> #include <unistd.h> #include <fcntl.h> +#include "file_rw_ops.h" + +using fastos::File_RW_Ops; const size_t FastOS_Linux_File::_directIOFileAlign = 4096; const size_t FastOS_Linux_File::_directIOMemAlign = 4096; @@ -31,7 +34,7 @@ FastOS_Linux_File::FastOS_Linux_File(const char *filename) ssize_t FastOS_Linux_File::readInternal(int fh, void *buffer, size_t length, int64_t readOffset) { - ssize_t readResult = ::pread(fh, buffer, length, readOffset); + ssize_t readResult = File_RW_Ops::pread(fh, buffer, length, readOffset); if (readResult < 0 && _failedHandler != nullptr) { int error = errno; const char *fileName = GetFileName(); @@ -45,7 +48,7 @@ FastOS_Linux_File::readInternal(int fh, void *buffer, size_t length, int64_t rea ssize_t FastOS_Linux_File::readInternal(int fh, void *buffer, size_t length) { - ssize_t readResult = ::read(fh, buffer, length); + ssize_t readResult = File_RW_Ops::read(fh, buffer, length); if (readResult < 0 && _failedHandler != nullptr) { int error = errno; int64_t readOffset = GetPosition(); @@ -60,7 +63,7 @@ FastOS_Linux_File::readInternal(int fh, void *buffer, size_t length) ssize_t FastOS_Linux_File::writeInternal(int fh, const void *buffer, size_t length, int64_t writeOffset) { - ssize_t writeRes = ::pwrite(fh, buffer, length, writeOffset); + ssize_t writeRes = File_RW_Ops::pwrite(fh, buffer, length, writeOffset); if (writeRes < 0 && _failedHandler != nullptr) { int error = errno; const char *fileName = GetFileName(); @@ -73,7 +76,7 @@ FastOS_Linux_File::writeInternal(int fh, const void *buffer, size_t length, int6 ssize_t FastOS_Linux_File::writeInternal(int fh, const void *buffer, size_t length) { - ssize_t writeRes = ::write(fh, buffer, length); + ssize_t writeRes = File_RW_Ops::write(fh, buffer, length); if (writeRes < 0 && _failedHandler != nullptr) { int error = errno; int64_t writeOffset = GetPosition(); diff --git a/fastos/src/vespa/fastos/unix_file.cpp b/fastos/src/vespa/fastos/unix_file.cpp index e1ecb13d6e0..06a48a26482 100644 --- a/fastos/src/vespa/fastos/unix_file.cpp +++ b/fastos/src/vespa/fastos/unix_file.cpp @@ -21,11 +21,14 @@ #else #include <sys/mount.h> #endif +#include "file_rw_ops.h" + +using fastos::File_RW_Ops; ssize_t FastOS_UNIX_File::Read(void *buffer, size_t len) { - ssize_t nRead = read(_filedes, buffer, len); + ssize_t nRead = File_RW_Ops::read(_filedes, buffer, len); if (nRead < 0 && _failedHandler != nullptr) { int error = errno; int64_t readOffset = GetPosition(); @@ -40,7 +43,7 @@ FastOS_UNIX_File::Read(void *buffer, size_t len) ssize_t FastOS_UNIX_File::Write2(const void *buffer, size_t len) { - ssize_t writeRes = write(_filedes, buffer, len); + ssize_t writeRes = File_RW_Ops::write(_filedes, buffer, len); if (writeRes < 0 && _failedHandler != nullptr) { int error = errno; int64_t writeOffset = GetPosition(); @@ -71,7 +74,7 @@ void FastOS_UNIX_File::ReadBuf(void *buffer, size_t length, { ssize_t readResult; - readResult = pread(_filedes, buffer, length, readOffset); + readResult = File_RW_Ops::pread(_filedes, buffer, length, readOffset); if (readResult < 0 && _failedHandler != nullptr) { int error = errno; const char *fileName = GetFileName(); diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/Metric.java b/jdisc_core/src/main/java/com/yahoo/jdisc/Metric.java index d6206dcf966..0e9db8fb1ea 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/Metric.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/Metric.java @@ -22,7 +22,7 @@ import java.util.Map; public interface Metric { /** - * <p>Set a metric value. This is typically used with histogram-type metrics.</p> + * Set a metric value. This is typically used with histogram-type metrics. * * @param key The name of the metric to modify. * @param val The value to assign to the named metric. @@ -31,28 +31,28 @@ public interface Metric { void set(String key, Number val, Context ctx); /** - * <p>Add to a metric value. This is typically used with counter-type metrics.</p> + * Add to a metric value. This is typically used with counter-type metrics. * - * @param key The name of the metric to modify. - * @param val The value to add to the named metric. - * @param ctx The context to further describe this entry. + * @param key the name of the metric to modify + * @param val the value to add to the named metric + * @param ctx the context to further describe this entry */ void add(String key, Number val, Context ctx); /** - * <p>Creates a {@link MetricConsumer}-specific {@link Context} object that encapsulates the given properties. The + * Creates a {@link MetricConsumer}-specific {@link Context} object that encapsulates the given properties. The * returned Context object should be passed along every future call to {@link #set(String, Number, Context)} and - * {@link #add(String, Number, Context)} where the properties match those given here.</p> + * {@link #add(String, Number, Context)} where the properties match those given here. * - * @param properties The properties to incorporate in the context. - * @return The created context. + * @param properties the properties to incorporate in the context + * @return the created context */ Context createContext(Map<String, ?> properties); /** - * <p>Declares the interface for the arbitrary context object to pass to both the {@link + * Declares the interface for the arbitrary context object to pass to both the {@link * #set(String, Number, Context)} and {@link #add(String, Number, Context)} methods. This is intentionally empty so - * that implementations can vary.</p> + * that implementations can vary. */ interface Context { diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java index c5f42ff9cc5..04db58f6d07 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java @@ -110,6 +110,7 @@ public class JettyHttpServer extends AbstractServerProvider { String CONTENT_SIZE = "jdisc.http.request.content_size"; String SSL_HANDSHAKE_FAILURE_MISSING_CLIENT_CERT = "jdisc.http.ssl.handshake.failure.missing_client_cert"; + String SSL_HANDSHAKE_FAILURE_EXPIRED_CLIENT_CERT = "jdisc.http.ssl.handshake.failure.expired_client_cert"; String SSL_HANDSHAKE_FAILURE_INVALID_CLIENT_CERT = "jdisc.http.ssl.handshake.failure.invalid_client_cert"; String SSL_HANDSHAKE_FAILURE_INCOMPATIBLE_PROTOCOLS = "jdisc.http.ssl.handshake.failure.incompatible_protocols"; String SSL_HANDSHAKE_FAILURE_INCOMPATIBLE_CIPHERS = "jdisc.http.ssl.handshake.failure.incompatible_ciphers"; diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/SslHandshakeFailedListener.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/SslHandshakeFailedListener.java index 886071243ba..75df82036a2 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/SslHandshakeFailedListener.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/SslHandshakeFailedListener.java @@ -56,6 +56,10 @@ class SslHandshakeFailedListener implements SslHandshakeListener { MISSING_CLIENT_CERT( Metrics.SSL_HANDSHAKE_FAILURE_MISSING_CLIENT_CERT, "Empty server certificate chain"), + EXPIRED_CLIENT_CERTIFICATE( + Metrics.SSL_HANDSHAKE_FAILURE_EXPIRED_CLIENT_CERT, + // Note: this pattern will match certificates with too late notBefore as well + "PKIX path validation failed: java.security.cert.CertPathValidatorException: validity check failed"), INVALID_CLIENT_CERT( Metrics.SSL_HANDSHAKE_FAILURE_INVALID_CLIENT_CERT, "PKIX path (building|validation) failed: .+"); diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java index f2f3fb0ef11..dfa4fe37ef3 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java @@ -27,6 +27,8 @@ import com.yahoo.jdisc.http.server.jetty.JettyHttpServer.Metrics; import com.yahoo.jdisc.http.server.jetty.TestDrivers.TlsClientAuth; import com.yahoo.jdisc.service.BindingSetNotFoundException; import com.yahoo.security.KeyUtils; +import com.yahoo.security.Pkcs10Csr; +import com.yahoo.security.Pkcs10CsrBuilder; import com.yahoo.security.SslContextBuilder; import com.yahoo.security.X509CertificateBuilder; import com.yahoo.security.X509CertificateUtils; @@ -56,6 +58,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.security.KeyPair; +import java.security.PrivateKey; import java.security.cert.X509Certificate; import java.time.Instant; import java.time.temporal.ChronoUnit; @@ -667,6 +670,29 @@ public class HttpServerTest { } @Test + public void requireThatMetricIsIncrementedWhenClientUsesExpiredCertificateInHandshake() throws IOException { + Path rootPrivateKeyFile = tmpFolder.newFile().toPath(); + Path rootCertificateFile = tmpFolder.newFile().toPath(); + Path privateKeyFile = tmpFolder.newFile().toPath(); + Path certificateFile = tmpFolder.newFile().toPath(); + Instant notAfter = Instant.now().minus(100, ChronoUnit.DAYS); + generatePrivateKeyAndCertificate(rootPrivateKeyFile, rootCertificateFile, privateKeyFile, certificateFile, notAfter); + var metricConsumer = new MetricConsumerMock(); + TestDriver driver = createSslTestDriver(rootCertificateFile, rootPrivateKeyFile, metricConsumer); + + SSLContext clientCtx = new SslContextBuilder() + .withTrustStore(rootCertificateFile) + .withKeyStore(privateKeyFile, certificateFile) + .build(); + + assertHttpsRequestTriggersSslHandshakeException( + driver, clientCtx, null, null, "Received fatal alert: certificate_unknown"); + verify(metricConsumer.mockitoMock()) + .add(Metrics.SSL_HANDSHAKE_FAILURE_EXPIRED_CLIENT_CERT, 1L, MetricConsumerMock.STATIC_CONTEXT); + assertThat(driver.close(), is(true)); + } + + @Test public void requireThatProxyProtocolIsAcceptedAndActualRemoteAddressStoredInAccessLog() throws Exception { Path privateKeyFile = tmpFolder.newFile().toPath(); Path certificateFile = tmpFolder.newFile().toPath(); @@ -788,6 +814,21 @@ public class HttpServerTest { Files.writeString(certificateFile, X509CertificateUtils.toPem(certificate)); } + private static void generatePrivateKeyAndCertificate(Path rootPrivateKeyFile, Path rootCertificateFile, + Path privateKeyFile, Path certificateFile, Instant notAfter) throws IOException { + generatePrivateKeyAndCertificate(rootPrivateKeyFile, rootCertificateFile); + X509Certificate rootCertificate = X509CertificateUtils.fromPem(Files.readString(rootCertificateFile)); + PrivateKey privateKey = KeyUtils.fromPemEncodedPrivateKey(Files.readString(rootPrivateKeyFile)); + + KeyPair keyPair = KeyUtils.generateKeypair(RSA, 2048); + Files.writeString(privateKeyFile, KeyUtils.toPem(keyPair.getPrivate())); + Pkcs10Csr csr = Pkcs10CsrBuilder.fromKeypair(new X500Principal("CN=myclient"), keyPair, SHA256_WITH_RSA).build(); + X509Certificate certificate = X509CertificateBuilder + .fromCsr(csr, rootCertificate.getSubjectX500Principal(), Instant.EPOCH, notAfter, privateKey, SHA256_WITH_RSA, BigInteger.ONE) + .build(); + Files.writeString(certificateFile, X509CertificateUtils.toPem(certificate)); + } + private static RequestHandler mockRequestHandler() { final RequestHandler mockRequestHandler = mock(RequestHandler.class); when(mockRequestHandler.refer()).thenReturn(References.NOOP_REFERENCE); 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 6f7be09aa0e..40cd6b03e0a 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 @@ -5,7 +5,6 @@ import com.google.inject.Inject; import com.yahoo.collections.ListMap; import com.yahoo.component.AbstractComponent; import com.yahoo.component.Version; -import com.yahoo.vespa.hosted.provision.Node.State; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Flavor; @@ -17,8 +16,7 @@ import com.yahoo.config.provisioning.NodeRepositoryConfig; import com.yahoo.transaction.Mutex; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.curator.Curator; -import com.yahoo.vespa.flags.FlagSource; -import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.hosted.provision.Node.State; import com.yahoo.vespa.hosted.provision.applications.Applications; import com.yahoo.vespa.hosted.provision.lb.LoadBalancer; import com.yahoo.vespa.hosted.provision.lb.LoadBalancerId; @@ -130,6 +128,9 @@ public class NodeRepository extends AbstractComponent { // read and write all nodes to make sure they are stored in the latest version of the serialized format for (State state : State.values()) + // TODO(mpolden): Add per-node locking. In its current state this may collide with other callers making + // node state changes. Example: A redeployment on another config server which moves a node + // to another state while this is constructed. db.writeTo(state, db.getNodes(state), Agent.system, Optional.empty()); } @@ -756,6 +757,17 @@ public class NodeRepository extends AbstractComponent { return resultingNodes; } + public boolean canAllocateTenantNodeTo(Node host) { + if (!host.type().canRun(NodeType.tenant)) return false; + + // Do not allocate to hosts we want to retire or are currently retiring + if (host.status().wantToRetire() || host.allocation().map(alloc -> alloc.membership().retired()).orElse(false)) + return false; + + if (!zone.cloud().value().equals("aws")) return host.state() == State.active; + else return EnumSet.of(State.active, State.ready, State.provisioned).contains(host.state()); + } + /** Returns the time keeper of this system */ public Clock clock() { return clock; } @@ -763,6 +775,7 @@ public class NodeRepository extends AbstractComponent { public Zone zone() { return zone; } /** Create a lock which provides exclusive rights to making changes to the given application */ + // TODO(mpolden): Make this delegate to CuratorDatabaseClient#lockConfig instead public Mutex lock(ApplicationId application) { return db.lock(application); } /** Create a lock with a timeout which provides exclusive rights to making changes to the given application */ 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 e9e09781e31..44dd023677c 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 @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.provision.maintenance; import com.yahoo.concurrent.DaemonThreadFactory; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Deployer; +import com.yahoo.jdisc.Metric; import com.yahoo.log.LogLevel; import com.yahoo.vespa.hosted.provision.NodeRepository; @@ -22,6 +23,7 @@ import java.util.concurrent.TimeUnit; public abstract class ApplicationMaintainer extends Maintainer { private final Deployer deployer; + private final Metric metric; private final CopyOnWriteArrayList<ApplicationId> pendingDeployments = new CopyOnWriteArrayList<>(); // Use a fixed thread pool to avoid overload on config servers. Resource usage when deploying varies @@ -32,9 +34,10 @@ public abstract class ApplicationMaintainer extends Maintainer { new LinkedBlockingQueue<>(), new DaemonThreadFactory("node repo application maintainer")); - protected ApplicationMaintainer(Deployer deployer, NodeRepository nodeRepository, Duration interval) { + protected ApplicationMaintainer(Deployer deployer, Metric metric, NodeRepository nodeRepository, Duration interval) { super(nodeRepository, interval); this.deployer = deployer; + this.metric = metric; } @Override @@ -73,7 +76,7 @@ public abstract class ApplicationMaintainer extends Maintainer { /** Redeploy this application. A lock will be taken for the duration of the deployment activation */ protected final void deployWithLock(ApplicationId application) { - try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, nodeRepository())) { + try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, metric, nodeRepository())) { if ( ! deployment.isValid()) return; // this will be done at another config server if ( ! canDeployNow(application)) return; // redeployment is no longer needed deployment.activate(); 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 abfe65408b6..0d7d10663c5 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 @@ -6,6 +6,7 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Deployer; import com.yahoo.config.provision.NodeResources; +import com.yahoo.jdisc.Metric; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.applications.Application; @@ -32,15 +33,18 @@ public class AutoscalingMaintainer extends Maintainer { private final Autoscaler autoscaler; private final Deployer deployer; + private final Metric metric; private final Map<Pair<ApplicationId, ClusterSpec.Id>, Instant> lastLogged = new HashMap<>(); public AutoscalingMaintainer(NodeRepository nodeRepository, HostResourcesCalculator hostResourcesCalculator, NodeMetricsDb metricsDb, Deployer deployer, + Metric metric, Duration interval) { super(nodeRepository, interval); this.autoscaler = new Autoscaler(hostResourcesCalculator, metricsDb, nodeRepository); + this.metric = metric; this.deployer = deployer; } @@ -52,7 +56,7 @@ public class AutoscalingMaintainer extends Maintainer { } private void autoscale(ApplicationId application, List<Node> applicationNodes) { - try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, nodeRepository())) { + try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, metric, nodeRepository())) { if ( ! deployment.isValid()) return; // Another config server will consider this application nodesByCluster(applicationNodes).forEach((clusterId, clusterNodes) -> autoscale(application, clusterId, clusterNodes, deployment)); } 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 e0c0e288406..906e6921f99 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 @@ -18,7 +18,6 @@ import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.provisioning.FatalProvisioningException; import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner; import com.yahoo.vespa.hosted.provision.provisioning.HostResourcesCalculator; -import com.yahoo.vespa.hosted.provision.provisioning.NodePrioritizer; import com.yahoo.vespa.hosted.provision.provisioning.NodeResourceComparator; import com.yahoo.vespa.hosted.provision.provisioning.ProvisionedHost; import com.yahoo.yolean.Exceptions; @@ -111,7 +110,7 @@ public class DynamicProvisioningMaintainer extends Maintainer { for (Iterator<NodeResources> it = preProvisionCapacity.iterator(); it.hasNext() && !removableHosts.isEmpty();) { NodeResources resources = it.next(); removableHosts.stream() - .filter(host -> NodePrioritizer.ALLOCATABLE_HOST_STATES.contains(host.state())) + .filter(nodeRepository()::canAllocateTenantNodeTo) .filter(host -> hostResourcesCalculator.advertisedResourcesOf(host.flavor()).satisfies(resources)) .min(Comparator.comparingInt(n -> n.flavor().cost())) .ifPresent(host -> { 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 e2b70608d58..7beb717ea8b 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 @@ -120,11 +120,13 @@ public class LoadBalancerExpirer extends Maintainer { /** Apply operation to all load balancers that exist in given state, while holding lock */ private void withLoadBalancersIn(LoadBalancer.State state, Consumer<LoadBalancer> operation) { for (var id : db.readLoadBalancerIds()) { - try (var lock = db.lockLoadBalancers(id.application())) { - var loadBalancer = db.readLoadBalancer(id); - if (loadBalancer.isEmpty()) continue; // Load balancer was removed during loop - if (loadBalancer.get().state() != state) continue; // Wrong state - operation.accept(loadBalancer.get()); + try (var lock = db.lockConfig(id.application())) { + try (var legacyLock = db.lockLoadBalancers(id.application())) { + var loadBalancer = db.readLoadBalancer(id); + if (loadBalancer.isEmpty()) continue; // Load balancer was removed during loop + if (loadBalancer.get().state() != state) continue; // Wrong state + operation.accept(loadBalancer.get()); + } } } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java index d9e06f87db7..8e1cd801a15 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java @@ -6,6 +6,7 @@ import com.yahoo.config.provision.ApplicationLockException; import com.yahoo.config.provision.Deployer; import com.yahoo.config.provision.Deployment; import com.yahoo.config.provision.TransientException; +import com.yahoo.jdisc.Metric; import com.yahoo.log.LogLevel; import com.yahoo.transaction.Mutex; import com.yahoo.vespa.hosted.provision.Node; @@ -14,6 +15,7 @@ import com.yahoo.yolean.Exceptions; import java.io.Closeable; import java.time.Duration; +import java.util.Map; import java.util.Optional; import java.util.logging.Logger; @@ -28,13 +30,18 @@ class MaintenanceDeployment implements Closeable { private static final Logger log = Logger.getLogger(MaintenanceDeployment.class.getName()); private final ApplicationId application; + private final Metric metric; private final Optional<Mutex> lock; private final Optional<Deployment> deployment; private boolean closed = false; - public MaintenanceDeployment(ApplicationId application, Deployer deployer, NodeRepository nodeRepository) { + public MaintenanceDeployment(ApplicationId application, + Deployer deployer, + Metric metric, + NodeRepository nodeRepository) { this.application = application; + this.metric = metric; Optional<Mutex> lock = tryLock(application, nodeRepository); try { deployment = tryDeployment(lock, application, deployer, nodeRepository); @@ -75,10 +82,12 @@ class MaintenanceDeployment implements Closeable { action.run(); return true; } catch (TransientException e) { + metric.add("maintenanceDeployment.transientFailure", 1, metric.createContext(Map.of())); log.log(LogLevel.INFO, "Failed to maintenance deploy " + application + " with a transient error: " + Exceptions.toMessageString(e)); return false; } catch (RuntimeException e) { + metric.add("maintenanceDeployment.failure", 1, metric.createContext(Map.of())); log.log(LogLevel.WARNING, "Exception on maintenance deploy of " + application, e); return false; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java index 104ab72a6a2..054c273dc99 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java @@ -72,10 +72,10 @@ public class NodeRepositoryMaintenance extends AbstractComponent { DefaultTimes defaults = new DefaultTimes(zone); nodeFailer = new NodeFailer(deployer, hostLivenessTracker, serviceMonitor, nodeRepository, defaults.failGrace, clock, orchestrator, throttlePolicyFromEnv().orElse(defaults.throttlePolicy), metric); - periodicApplicationMaintainer = new PeriodicApplicationMaintainer(deployer, nodeRepository, defaults.redeployMaintainerInterval, defaults.periodicRedeployInterval); - operatorChangeApplicationMaintainer = new OperatorChangeApplicationMaintainer(deployer, nodeRepository, defaults.operatorChangeRedeployInterval); + periodicApplicationMaintainer = new PeriodicApplicationMaintainer(deployer, metric, nodeRepository, defaults.redeployMaintainerInterval, defaults.periodicRedeployInterval); + operatorChangeApplicationMaintainer = new OperatorChangeApplicationMaintainer(deployer, metric, nodeRepository, defaults.operatorChangeRedeployInterval); reservationExpirer = new ReservationExpirer(nodeRepository, clock, defaults.reservationExpiry); - retiredExpirer = new RetiredExpirer(nodeRepository, orchestrator, deployer, clock, defaults.retiredInterval, defaults.retiredExpiry); + retiredExpirer = new RetiredExpirer(nodeRepository, orchestrator, deployer, metric, clock, defaults.retiredInterval, defaults.retiredExpiry); inactiveExpirer = new InactiveExpirer(nodeRepository, clock, defaults.inactiveExpiry); failedExpirer = new FailedExpirer(nodeRepository, zone, clock, defaults.failedExpirerInterval); dirtyExpirer = new DirtyExpirer(nodeRepository, clock, defaults.dirtyExpiry); @@ -91,7 +91,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { osUpgradeActivator = new OsUpgradeActivator(nodeRepository, defaults.osUpgradeActivatorInterval); rebalancer = new Rebalancer(deployer, nodeRepository, provisionServiceProvider.getHostResourcesCalculator(), provisionServiceProvider.getHostProvisioner(), metric, clock, defaults.rebalancerInterval); nodeMetricsDbMaintainer = new NodeMetricsDbMaintainer(nodeRepository, nodeMetrics, nodeMetricsDb, defaults.nodeMetricsCollectionInterval); - autoscalingMaintainer = new AutoscalingMaintainer(nodeRepository, provisionServiceProvider.getHostResourcesCalculator(), nodeMetricsDb, deployer, defaults.autoscalingInterval); + autoscalingMaintainer = new AutoscalingMaintainer(nodeRepository, provisionServiceProvider.getHostResourcesCalculator(), nodeMetricsDb, deployer, metric, defaults.autoscalingInterval); // The DuperModel is filled with infrastructure applications by the infrastructure provisioner, so explicitly run that now infrastructureProvisioner.maintainButThrowOnException(); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java index e7406bf3478..9f829c095f4 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.provision.maintenance; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Deployer; import com.yahoo.config.provision.NodeType; +import com.yahoo.jdisc.Metric; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.Agent; @@ -31,8 +32,8 @@ import java.util.stream.Collectors; */ public class OperatorChangeApplicationMaintainer extends ApplicationMaintainer { - OperatorChangeApplicationMaintainer(Deployer deployer, NodeRepository nodeRepository, Duration interval) { - super(deployer, nodeRepository, interval); + OperatorChangeApplicationMaintainer(Deployer deployer, Metric metric, NodeRepository nodeRepository, Duration interval) { + super(deployer, metric, nodeRepository, interval); } @Override diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java index 6ab85e76ba2..d06d24872e1 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.provision.maintenance; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Deployer; +import com.yahoo.jdisc.Metric; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; @@ -29,9 +30,9 @@ public class PeriodicApplicationMaintainer extends ApplicationMaintainer { private final Clock clock; private final Instant start; - PeriodicApplicationMaintainer(Deployer deployer, NodeRepository nodeRepository, + PeriodicApplicationMaintainer(Deployer deployer, Metric metric, NodeRepository nodeRepository, Duration interval, Duration minTimeBetweenRedeployments) { - super(deployer, nodeRepository, interval); + super(deployer, metric, nodeRepository, interval); this.minTimeBetweenRedeployments = minTimeBetweenRedeployments; this.clock = nodeRepository.clock(); this.start = clock.instant(); 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 db99f8544f1..6e8af120b3b 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 @@ -14,7 +14,6 @@ import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.provisioning.DockerHostCapacity; import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner; import com.yahoo.vespa.hosted.provision.provisioning.HostResourcesCalculator; -import com.yahoo.vespa.hosted.provision.provisioning.NodePrioritizer; import java.time.Clock; import java.time.Duration; @@ -93,7 +92,7 @@ public class Rebalancer extends Maintainer { if (node.parentHostname().isEmpty()) continue; if (node.allocation().get().owner().instance().isTester()) continue; if (node.allocation().get().owner().application().value().equals("lsbe-dictionaries")) continue; // TODO: Remove - for (Node toHost : allNodes.nodeType(NodeType.host).state(NodePrioritizer.ALLOCATABLE_HOST_STATES)) { + for (Node toHost : allNodes.filter(nodeRepository()::canAllocateTenantNodeTo)) { if (toHost.hostname().equals(node.parentHostname().get())) continue; if ( ! capacity.freeCapacityOf(toHost).satisfies(node.flavor().resources())) continue; @@ -129,7 +128,7 @@ public class Rebalancer extends Maintainer { */ private boolean deployTo(Move move) { ApplicationId application = move.node.allocation().get().owner(); - try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, nodeRepository())) { + try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, metric, nodeRepository())) { if ( ! deployment.isValid()) return false; boolean couldMarkRetiredNow = markWantToRetire(move.node, true); 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 1d31917b3e1..3c01c8bc23c 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 @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.provision.maintenance; import com.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Deployer; +import com.yahoo.jdisc.Metric; import com.yahoo.vespa.applicationmodel.HostName; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; @@ -26,6 +27,7 @@ import java.util.stream.Collectors; public class RetiredExpirer extends Maintainer { private final Deployer deployer; + private final Metric metric; private final Orchestrator orchestrator; private final Duration retiredExpiry; private final Clock clock; @@ -33,11 +35,13 @@ public class RetiredExpirer extends Maintainer { public RetiredExpirer(NodeRepository nodeRepository, Orchestrator orchestrator, Deployer deployer, + Metric metric, Clock clock, Duration maintenanceInterval, Duration retiredExpiry) { super(nodeRepository, maintenanceInterval); this.deployer = deployer; + this.metric = metric; this.orchestrator = orchestrator; this.retiredExpiry = retiredExpiry; this.clock = clock; @@ -56,7 +60,7 @@ public class RetiredExpirer extends Maintainer { ApplicationId application = entry.getKey(); List<Node> retiredNodes = entry.getValue(); - try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, nodeRepository())) { + try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, metric, nodeRepository())) { if ( ! deployment.isValid()) continue; // this will be done at another config server List<Node> nodesToRemove = retiredNodes.stream().filter(this::canRemove).collect(Collectors.toList()); @@ -103,7 +107,7 @@ public class RetiredExpirer extends Maintainer { log.info("Node " + node + " has been granted permission to be removed"); return true; } catch (UncheckedTimeoutException e) { - log.info("Timed out trying to aquire permission to remove " + node.hostname() + ": " + e.getMessage()); + log.info("Timed out trying to acquire permission to remove " + node.hostname() + ": " + e.getMessage()); return false; } catch (OrchestrationException e) { log.info("Did not get permission to remove retired " + node + ": " + e.getMessage()); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabase.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabase.java index e12cd3e53b9..08f2cfec40f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabase.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabase.java @@ -38,18 +38,12 @@ public class CuratorDatabase { /** A partial cache of the Curator database, which is only valid if generations match */ private final AtomicReference<Cache> cache = new AtomicReference<>(); - /** Whether we should return data from the cache or always read fro ZooKeeper */ + /** Whether we should return data from the cache or always read from ZooKeeper */ private final boolean useCache; private final Object cacheCreationLock = new Object(); /** - * All keys, to allow reentrancy. - * This will grow forever with the number of applications seen, but this should be too slow to be a problem. - */ - private final ConcurrentHashMap<Path, Lock> locks = new ConcurrentHashMap<>(); - - /** * Creates a curator database * * @param curator the curator instance @@ -72,11 +66,8 @@ public class CuratorDatabase { } /** Create a reentrant lock */ - // Locks are not cached in the in-memory state public Lock lock(Path path, Duration timeout) { - Lock lock = locks.computeIfAbsent(path, (pathArg) -> new Lock(pathArg.getAbsolute(), curator)); - lock.acquire(timeout); - return lock; + return curator.lock(path, timeout); } // --------- Write operations ------------------------------------------------------------------------------ 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 8ecdb0cbb1f..6036cc2366f 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 @@ -62,6 +62,7 @@ public class CuratorDatabaseClient { private static final Path root = Path.fromString("/provision/v1"); private static final Path lockRoot = root.append("locks"); + private static final Path configLockRoot = Path.fromString("/config/v2/locks/"); private static final Path loadBalancersRoot = root.append("loadBalancers"); private static final Duration defaultLockTimeout = Duration.ofMinutes(2); @@ -312,7 +313,7 @@ public class CuratorDatabaseClient { return root.append(toDir(nodeState)).append(nodeName); } - /** Creates an returns the path to the lock for this application */ + /** Creates and returns the path to the lock for this application */ private Path lockPath(ApplicationId application) { Path lockPath = lockRoot @@ -323,6 +324,14 @@ public class CuratorDatabaseClient { return lockPath; } + /** Creates and returns the path to the config server lock for this application */ + private Path configLockPath(ApplicationId application) { + // This must match the lock path used by com.yahoo.vespa.config.server.application.TenantApplications + Path lockPath = configLockRoot.append(application.tenant().value()).append(application.serializedForm()); + curatorDatabase.create(lockPath); + return lockPath; + } + private String toDir(Node.State state) { switch (state) { case active: return "allocated"; // legacy name @@ -358,6 +367,28 @@ public class CuratorDatabaseClient { } } + /** + * Acquires the single cluster-global, re-entrant config lock for given application. Note that this is the same lock + * that configserver itself takes when modifying applications. + * + * This lock must be taken when writes to paths owned by this class may happen on both the configserver and + * node-repository side. This behaviour is obviously wrong, but since we pass a NestedTransaction across the + * configserver and node-repository boundary, the ownership semantics of the transaction (and its operations) + * becomes unclear. + * + * Example of when to use: The config server creates a new transaction and passes the transaction to + * {@link com.yahoo.vespa.hosted.provision.provisioning.NodeRepositoryProvisioner}, which appends operations to the + * transaction. The config server then commits (writes) the transaction which may include operations that modify + * data in paths owned by this class. + */ + public Lock lockConfig(ApplicationId application) { + try { + return lock(configLockPath(application), defaultLockTimeout); + } catch (UncheckedTimeoutException e) { + throw new ApplicationLockException(e); + } + } + private Lock lock(Path path, Duration timeout) { return curatorDatabase.lock(path, timeout); } @@ -525,6 +556,7 @@ public class CuratorDatabaseClient { transaction.commit(); } + // TODO(mpolden): Remove this and usages after April 2020 public Lock lockLoadBalancers(ApplicationId application) { return lock(lockRoot.append("loadBalancersLock2").append(application.serializedForm()), defaultLockTimeout); } 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 e90f177d99d..8143076a3b2 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 @@ -79,7 +79,7 @@ public class GroupPreparer { prioritizer.addApplicationNodes(); prioritizer.addSurplusNodes(surplusActiveNodes); prioritizer.addReadyNodes(); - prioritizer.addNewDockerNodes(); + prioritizer.addNewDockerNodes(nodeRepository::canAllocateTenantNodeTo); // Allocate from the prioritized list NodeAllocation allocation = new NodeAllocation(nodeList, application, cluster, requestedNodes, diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java index 1d3fe114e23..b694a7c3cd4 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java @@ -52,9 +52,11 @@ public class LoadBalancerProvisioner { this.service = service; // Read and write all load balancers to make sure they are stored in the latest version of the serialization format for (var id : db.readLoadBalancerIds()) { - try (var lock = db.lockLoadBalancers(id.application())) { - var loadBalancer = db.readLoadBalancer(id); - loadBalancer.ifPresent(db::writeLoadBalancer); + try (var lock = db.lockConfig(id.application())) { + try (var legacyLock = db.lockLoadBalancers(id.application())) { + var loadBalancer = db.readLoadBalancer(id); + loadBalancer.ifPresent(db::writeLoadBalancer); + } } } } @@ -73,8 +75,10 @@ public class LoadBalancerProvisioner { if (requestedNodes.type() != NodeType.tenant) return; // Nothing to provision for this node type if (!cluster.type().isContainer()) return; // Nothing to provision for this cluster type if (application.instance().isTester()) return; // Do not provision for tester instances - try (var lock = db.lockLoadBalancers(application)) { - provision(application, effectiveId(cluster), false, lock); + try (var lock = db.lockConfig(application)) { + try (var legacyLock = db.lockLoadBalancers(application)) { + provision(application, effectiveId(cluster), false, lock); + } } } @@ -90,15 +94,17 @@ public class LoadBalancerProvisioner { */ public void activate(ApplicationId application, Set<ClusterSpec> clusters, @SuppressWarnings("unused") Mutex applicationLock, NestedTransaction transaction) { - try (var lock = db.lockLoadBalancers(application)) { - var containerClusters = containerClustersOf(clusters); - for (var clusterId : containerClusters) { - // Provision again to ensure that load balancer instance is re-configured with correct nodes - provision(application, clusterId, true, lock); + try (var lock = db.lockConfig(application)) { + try (var legacyLock = db.lockLoadBalancers(application)) { + var containerClusters = containerClustersOf(clusters); + for (var clusterId : containerClusters) { + // Provision again to ensure that load balancer instance is re-configured with correct nodes + provision(application, clusterId, true, legacyLock); + } + // Deactivate any surplus load balancers, i.e. load balancers for clusters that have been removed + var surplusLoadBalancers = surplusLoadBalancersOf(application, containerClusters); + deactivate(surplusLoadBalancers, transaction); } - // Deactivate any surplus load balancers, i.e. load balancers for clusters that have been removed - var surplusLoadBalancers = surplusLoadBalancersOf(application, containerClusters); - deactivate(surplusLoadBalancers, transaction); } } @@ -108,8 +114,10 @@ public class LoadBalancerProvisioner { */ public void deactivate(ApplicationId application, NestedTransaction transaction) { try (var applicationLock = nodeRepository.lock(application)) { - try (var lock = db.lockLoadBalancers(application)) { - deactivate(nodeRepository.loadBalancers(application).asList(), transaction); + try (var lock = db.lockConfig(application)) { + try (var legacyLock = db.lockLoadBalancers(application)) { + deactivate(nodeRepository.loadBalancers(application).asList(), transaction); + } } } } 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 1ef23209369..524b524f8c4 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 @@ -18,6 +18,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.function.Predicate; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -31,9 +32,6 @@ import java.util.stream.Collectors; */ public class NodePrioritizer { - /** Node states in which host can get new nodes allocated in, ordered by preference (ascending) */ - public static final List<Node.State> ALLOCATABLE_HOST_STATES = - List.of(Node.State.provisioned, Node.State.ready, Node.State.active); private final static Logger log = Logger.getLogger(NodePrioritizer.class.getName()); private final Map<Node, PrioritizableNode> nodes = new HashMap<>(); @@ -119,11 +117,11 @@ public class NodePrioritizer { } /** Add a node on each docker host with enough capacity for the requested flavor */ - void addNewDockerNodes() { + void addNewDockerNodes(Predicate<Node> canAllocateTenantNodeTo) { if ( ! isDocker) return; LockedNodeList candidates = allNodes - .filter(node -> node.type() != NodeType.host || ALLOCATABLE_HOST_STATES.contains(node.state())) + .filter(node -> node.type() != NodeType.host || canAllocateTenantNodeTo.test(node)) .filter(node -> node.reservedTo().isEmpty() || node.reservedTo().get().equals(application.tenant())); if (allocateFully) { @@ -145,9 +143,6 @@ public class NodePrioritizer { NodeResources wantedResources = resources(requestedNodes); for (Node host : candidates) { - if (!host.type().canRun(requestedNodes.type())) continue; - if (host.status().wantToRetire()) continue; - boolean hostHasCapacityForWantedFlavor = capacity.hasCapacity(host, wantedResources); boolean conflictingCluster = allNodes.childrenOf(host).owner(application).asList().stream() .anyMatch(child -> child.allocation().get().membership().cluster().id().equals(clusterSpec.id())); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java index 6183bffe5ba..516bb2e84ea 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java @@ -4,10 +4,9 @@ package com.yahoo.vespa.hosted.provision.provisioning; import com.yahoo.config.provision.NodeResources; import com.yahoo.vespa.hosted.provision.Node; +import java.util.List; import java.util.Optional; -import static com.yahoo.vespa.hosted.provision.provisioning.NodePrioritizer.ALLOCATABLE_HOST_STATES; - /** * A node with additional information required to prioritize it for allocation. * @@ -15,6 +14,10 @@ import static com.yahoo.vespa.hosted.provision.provisioning.NodePrioritizer.ALLO */ class PrioritizableNode implements Comparable<PrioritizableNode> { + /** List of host states ordered by preference (ascending) */ + private static final List<Node.State> HOST_STATE_PRIORITY = + List.of(Node.State.provisioned, Node.State.ready, Node.State.active); + private static final NodeResources zeroResources = new NodeResources(0, 0, 0, 0, NodeResources.DiskSpeed.any, NodeResources.StorageType.any); @@ -111,8 +114,8 @@ class PrioritizableNode implements Comparable<PrioritizableNode> { if (other.node.flavor().cost() < this.node.flavor().cost()) return 1; // Choose nodes where host is in more desirable state - int thisHostStatePri = this.parent.map(host -> ALLOCATABLE_HOST_STATES.indexOf(host.state())).orElse(-2); - int otherHostStatePri = other.parent.map(host -> ALLOCATABLE_HOST_STATES.indexOf(host.state())).orElse(-2); + int thisHostStatePri = this.parent.map(host -> HOST_STATE_PRIORITY.indexOf(host.state())).orElse(-2); + int otherHostStatePri = other.parent.map(host -> HOST_STATE_PRIORITY.indexOf(host.state())).orElse(-2); if (thisHostStatePri != otherHostStatePri) return otherHostStatePri - thisHostStatePri; // All else equal choose hostname alphabetically diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java index da169cba08f..8f8f8d0f38b 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java @@ -59,6 +59,7 @@ public class AutoscalingMaintainerTest { tester.identityHostResourcesCalculator(), nodeMetricsDb, deployer, + new TestMetric(), Duration.ofMinutes(1)); maintainer.maintain(); // noop assertTrue(deployer.lastDeployTime(app1).isEmpty()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java index d434de2a83b..ebec07fe5dc 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java @@ -3,12 +3,16 @@ package com.yahoo.vespa.hosted.provision.maintenance; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.DockerImage; +import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.NodeFlavors; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; import com.yahoo.test.ManualClock; import com.yahoo.vespa.curator.mock.MockCurator; @@ -47,7 +51,6 @@ import static com.yahoo.vespa.hosted.provision.maintenance.DynamicProvisioningMa import static com.yahoo.vespa.hosted.provision.maintenance.DynamicProvisioningMaintainerTest.HostProvisionerTester.tenantApp; import static com.yahoo.vespa.hosted.provision.maintenance.DynamicProvisioningMaintainerTest.HostProvisionerTester.tenantHostApp; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -121,8 +124,8 @@ public class DynamicProvisioningMaintainerTest { verify(hostProvisioner).deprovision(argThatLambda(node -> node.hostname().equals("host2"))); verify(hostProvisioner).deprovision(argThatLambda(node -> node.hostname().equals("host3"))); verifyNoMoreInteractions(hostProvisioner); - assertEquals(Node.State.deprovisioned, tester.nodeRepository.getNode("host2").get().state()); - assertEquals(Node.State.deprovisioned, tester.nodeRepository.getNode("host3").get().state()); + assertTrue(tester.nodeRepository.getNode("host2").isEmpty()); + assertTrue(tester.nodeRepository.getNode("host3").isEmpty()); } @Test @@ -144,7 +147,7 @@ public class DynamicProvisioningMaintainerTest { addNodes(); maintainer.convergeToCapacity(tester.nodeRepository.list()); - assertEquals(Node.State.deprovisioned, tester.nodeRepository.getNode("host2").get().state()); + assertTrue(tester.nodeRepository.getNode("host2").isEmpty()); assertTrue(tester.nodeRepository.getNode("host3").isPresent()); verify(hostProvisioner).deprovision(argThatLambda(node -> node.hostname().equals("host2"))); verify(hostProvisioner, times(2)).provisionHosts(argThatLambda(list -> list.size() == 1), eq(new NodeResources(2, 3, 2, 1)), any()); @@ -208,8 +211,9 @@ public class DynamicProvisioningMaintainerTest { static final ApplicationId proxyApp = ApplicationId.from("vespa", "proxy", "default"); private final ManualClock clock = new ManualClock(); + private final Zone zone = new Zone(CloudName.from("aws"), SystemName.defaultSystem(), Environment.defaultEnvironment(), RegionName.defaultName()); private final NodeRepository nodeRepository = new NodeRepository( - nodeFlavors, new MockCurator(), clock, Zone.defaultZone(), new MockNameResolver().mockAnyLookup(), + nodeFlavors, new MockCurator(), clock, zone, new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-image"), true); Node addNode(String hostname, Optional<String> parentHostname, NodeType nodeType, Node.State state, Optional<ApplicationId> application) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveAndFailedExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveAndFailedExpirerTest.java index 9bb3a55abfd..4fcd5793b8a 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveAndFailedExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveAndFailedExpirerTest.java @@ -19,6 +19,7 @@ import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.History; import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester; import com.yahoo.vespa.hosted.provision.testutils.MockDeployer; +import com.yahoo.vespa.hosted.provision.testutils.MockNodeMetrics; import com.yahoo.vespa.orchestrator.OrchestrationException; import com.yahoo.vespa.orchestrator.Orchestrator; import org.junit.Test; @@ -152,7 +153,7 @@ public class InactiveAndFailedExpirerTest { ); Orchestrator orchestrator = mock(Orchestrator.class); doThrow(new RuntimeException()).when(orchestrator).acquirePermissionToRemove(any()); - new RetiredExpirer(tester.nodeRepository(), tester.orchestrator(), deployer, tester.clock(), Duration.ofDays(30), + new RetiredExpirer(tester.nodeRepository(), tester.orchestrator(), deployer, new TestMetric(), tester.clock(), Duration.ofDays(30), Duration.ofMinutes(10)).run(); assertEquals(1, tester.nodeRepository().getNodes(Node.State.inactive).size()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java index ca7c33f96bd..665dd74176d 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java @@ -36,9 +36,7 @@ import org.junit.Test; import java.time.Clock; import java.time.Duration; import java.time.Instant; -import java.util.ArrayList; import java.util.HashMap; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -224,70 +222,4 @@ public class MetricsReporterTest { return Optional.empty(); } - public static class TestMetric implements Metric { - - public Map<String, Number> values = new LinkedHashMap<>(); - public Map<String, List<Context>> context = new LinkedHashMap<>(); - - @Override - public void set(String key, Number val, Context ctx) { - values.put(key, val); - if (ctx != null) { - //Create one context pr value added - copy the context to not have side effects - TestContext kontekst = (TestContext)createContext(((TestContext) ctx).properties); - if (!context.containsKey(key)) { - context.put(key, new ArrayList<>()); - } - kontekst.setValue(val); - context.get(key).add(kontekst); - } - } - - @Override - public void add(String key, Number val, Context ctx) { - values.put(key, val); - if (ctx != null) { - //Create one context pr value added - copy the context to not have side effects - TestContext copy = (TestContext) createContext(((TestContext) ctx).properties); - if (!context.containsKey(key)) { - context.put(key, new ArrayList<>()); - } - copy.setValue(val); - context.get(key).add(copy); - } - } - - @Override - public Context createContext(Map<String, ?> properties) { - return new TestContext(properties); - } - - double sumDoubleValues(String key, Context sumContext) { - double sum = 0.0; - for(Context c : context.get(key)) { - TestContext tc = (TestContext) c; - if (tc.value instanceof Double && tc.properties.equals(((TestContext) sumContext).properties)) { - sum += (double) tc.value; - } - } - return sum; - } - - /** - * Context where the propertymap is not shared - but unique to each value. - */ - private static class TestContext implements Context{ - Number value; - Map<String, ?> properties; - - public TestContext(Map<String, ?> properties) { - this.properties = properties; - } - - public void setValue(Number value) { - this.value = value; - } - } - } - } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java index 585d57aae4e..ab97de80418 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java @@ -65,7 +65,7 @@ public class NodeFailTester { public NodeFailer failer; public ServiceMonitorStub serviceMonitor; public MockDeployer deployer; - public MetricsReporterTest.TestMetric metric; + public TestMetric metric; private final TestHostLivenessTracker hostLivenessTracker; private final Orchestrator orchestrator; private final NodeRepositoryProvisioner provisioner; @@ -103,7 +103,7 @@ public class NodeFailTester { app2, new MockDeployer.ApplicationContext(app2, clusterApp2, capacity2)); tester.deployer = new MockDeployer(tester.provisioner, tester.clock(), apps); tester.serviceMonitor = new ServiceMonitorStub(apps, tester.nodeRepository); - tester.metric = new MetricsReporterTest.TestMetric(); + tester.metric = new TestMetric(); tester.failer = tester.createFailer(); return tester; } @@ -139,7 +139,7 @@ public class NodeFailTester { app2, new MockDeployer.ApplicationContext(app2, clusterApp2, capacity2)); tester.deployer = new MockDeployer(tester.provisioner, tester.clock(), apps); tester.serviceMonitor = new ServiceMonitorStub(apps, tester.nodeRepository); - tester.metric = new MetricsReporterTest.TestMetric(); + tester.metric = new TestMetric(); tester.failer = tester.createFailer(); return tester; } @@ -158,7 +158,7 @@ public class NodeFailTester { app1, new MockDeployer.ApplicationContext(app1, clusterApp1, allNodes)); tester.deployer = new MockDeployer(tester.provisioner, tester.clock(), apps); tester.serviceMonitor = new ServiceMonitorStub(apps, tester.nodeRepository); - tester.metric = new MetricsReporterTest.TestMetric(); + tester.metric = new TestMetric(); tester.failer = tester.createFailer(); return tester; } @@ -167,7 +167,7 @@ public class NodeFailTester { NodeFailTester tester = new NodeFailTester(); tester.deployer = new MockDeployer(tester.provisioner, tester.clock(), Map.of()); tester.serviceMonitor = new ServiceMonitorStub(Map.of(), tester.nodeRepository); - tester.metric = new MetricsReporterTest.TestMetric(); + tester.metric = new TestMetric(); tester.failer = tester.createFailer(); return tester; } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java index 2dda6c714a7..eb2a1d4db68 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java @@ -66,7 +66,10 @@ public class OperatorChangeApplicationMaintainerTest { // Create applications fixture.activate(); assertEquals("Initial applications are deployed", 3, fixture.deployer.redeployments); - OperatorChangeApplicationMaintainer maintainer = new OperatorChangeApplicationMaintainer(fixture.deployer, nodeRepository, Duration.ofMinutes(1)); + OperatorChangeApplicationMaintainer maintainer = new OperatorChangeApplicationMaintainer(fixture.deployer, + new TestMetric(), + nodeRepository, + Duration.ofMinutes(1)); clock.advance(Duration.ofMinutes(2)); maintainer.maintain(); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java index 3037d5972e5..63a22cc3029 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java @@ -303,7 +303,7 @@ public class PeriodicApplicationMaintainerTest { TestablePeriodicApplicationMaintainer(Deployer deployer, NodeRepository nodeRepository, Duration interval, Duration minTimeBetweenRedeployments) { - super(deployer, nodeRepository, interval, minTimeBetweenRedeployments); + super(deployer, new TestMetric(), nodeRepository, interval, minTimeBetweenRedeployments); } @Override diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RebalancerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RebalancerTest.java index 387f614c5eb..d25ae234f35 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RebalancerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RebalancerTest.java @@ -17,7 +17,6 @@ import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder; -import com.yahoo.vespa.hosted.provision.provisioning.HostResourcesCalculator; import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester; import com.yahoo.vespa.hosted.provision.testutils.MockDeployer; import org.junit.Test; @@ -46,7 +45,7 @@ public class RebalancerTest { NodeResources memResources = new NodeResources(4, 9, 1, 0.1); ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.perf, RegionName.from("us-east"))).flavorsConfig(flavorsConfig()).build(); - MetricsReporterTest.TestMetric metric = new MetricsReporterTest.TestMetric(); + TestMetric metric = new TestMetric(); Map<ApplicationId, MockDeployer.ApplicationContext> apps = Map.of( cpuApp, new MockDeployer.ApplicationContext(cpuApp, clusterSpec("c"), Capacity.from(new ClusterResources(1, 1, cpuResources))), diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java index 276b9484ad4..7ece8cba65e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java @@ -241,6 +241,7 @@ public class RetiredExpirerTest { nodeRepository, orchestrator, deployer, + new TestMetric(), clock, Duration.ofDays(30), /* Maintenance interval, use large value so it never runs by itself */ RETIRED_EXPIRATION); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/TestMetric.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/TestMetric.java new file mode 100644 index 00000000000..c98216f9d14 --- /dev/null +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/TestMetric.java @@ -0,0 +1,75 @@ +// 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.maintenance; + +import com.yahoo.jdisc.Metric; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +public class TestMetric implements Metric { + + public Map<String, Number> values = new LinkedHashMap<>(); + public Map<String, List<Context>> context = new LinkedHashMap<>(); + + @Override + public void set(String key, Number val, Context ctx) { + values.put(key, val); + if (ctx != null) { + //Create one context pr value added - copy the context to not have side effects + TestContext kontekst = (TestContext)createContext(((TestContext) ctx).properties); + if (!context.containsKey(key)) { + context.put(key, new ArrayList<>()); + } + kontekst.setValue(val); + context.get(key).add(kontekst); + } + } + + @Override + public void add(String key, Number val, Context ctx) { + values.put(key, val); + if (ctx != null) { + //Create one context pr value added - copy the context to not have side effects + TestContext copy = (TestContext) createContext(((TestContext) ctx).properties); + if (!context.containsKey(key)) { + context.put(key, new ArrayList<>()); + } + copy.setValue(val); + context.get(key).add(copy); + } + } + + @Override + public Context createContext(Map<String, ?> properties) { + return new TestContext(properties); + } + + double sumDoubleValues(String key, Context sumContext) { + double sum = 0.0; + for(Context c : context.get(key)) { + TestContext tc = (TestContext) c; + if (tc.value instanceof Double && tc.properties.equals(((TestContext) sumContext).properties)) { + sum += (double) tc.value; + } + } + return sum; + } + + /** + * Context where the propertymap is not shared - but unique to each value. + */ + private static class TestContext implements Context{ + Number value; + Map<String, ?> properties; + + public TestContext(Map<String, ?> properties) { + this.properties = properties; + } + + public void setValue(Number value) { + this.value = value; + } + } +} diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/MultigroupProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/MultigroupProvisioningTest.java index 4a75d86f530..1265961e351 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/MultigroupProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/MultigroupProvisioningTest.java @@ -12,6 +12,7 @@ import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.maintenance.RetiredExpirer; +import com.yahoo.vespa.hosted.provision.maintenance.TestMetric; import com.yahoo.vespa.hosted.provision.testutils.MockDeployer; import org.junit.Ignore; import org.junit.Test; @@ -140,8 +141,13 @@ public class MultigroupProvisioningTest { Collections.singletonMap(application1, new MockDeployer.ApplicationContext(application1, cluster(), Capacity.from(new ClusterResources(8, 1, large), false, true)))); - new RetiredExpirer(tester.nodeRepository(), tester.orchestrator(), deployer, tester.clock(), Duration.ofDays(30), - Duration.ofHours(12)).run(); + new RetiredExpirer(tester.nodeRepository(), + tester.orchestrator(), + deployer, + new TestMetric(), + tester.clock(), + Duration.ofDays(30), + Duration.ofHours(12)).run(); assertEquals(8, tester.getNodes(application1, Node.State.inactive).resources(small).size()); deploy(application1, 8, 8, large, tester); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/NodeTypeProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/NodeTypeProvisioningTest.java index 8420bdeacfe..98133898cf6 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/NodeTypeProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/NodeTypeProvisioningTest.java @@ -8,6 +8,7 @@ import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.maintenance.RetiredExpirer; +import com.yahoo.vespa.hosted.provision.maintenance.TestMetric; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.testutils.MockDeployer; import org.junit.Before; @@ -94,8 +95,13 @@ public class NodeTypeProvisioningTest { tester.clock(), Collections.singletonMap( application, new MockDeployer.ApplicationContext(application, clusterSpec, capacity))); - RetiredExpirer retiredExpirer = new RetiredExpirer(tester.nodeRepository(), tester.orchestrator(), deployer, - tester.clock(), Duration.ofDays(30), Duration.ofMinutes(10)); + RetiredExpirer retiredExpirer = new RetiredExpirer(tester.nodeRepository(), + tester.orchestrator(), + deployer, + new TestMetric(), + tester.clock(), + Duration.ofDays(30), + Duration.ofMinutes(10)); { // Deploy List<HostSpec> hosts = deployProxies(application, tester); @@ -161,6 +167,7 @@ public class NodeTypeProvisioningTest { RetiredExpirer retiredExpirer = new RetiredExpirer(tester.nodeRepository(), tester.orchestrator(), deployer, + new TestMetric(), tester.clock(), Duration.ofDays(30), Duration.ofMinutes(10)); diff --git a/persistence/src/vespa/persistence/CMakeLists.txt b/persistence/src/vespa/persistence/CMakeLists.txt index ec569a23029..77f3f865b6f 100644 --- a/persistence/src/vespa/persistence/CMakeLists.txt +++ b/persistence/src/vespa/persistence/CMakeLists.txt @@ -9,7 +9,6 @@ vespa_add_library(persistence vespa_add_library(persistence_persistence_conformancetest SOURCES $<TARGET_OBJECTS:persistence_conformancetest_lib> - INSTALL lib64 DEPENDS persistence gtest diff --git a/searchcore/src/vespa/searchcore/proton/server/proton.cpp b/searchcore/src/vespa/searchcore/proton/server/proton.cpp index 3f8db3f2ff9..1ab1ba72b4f 100644 --- a/searchcore/src/vespa/searchcore/proton/server/proton.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/proton.cpp @@ -662,6 +662,7 @@ Proton::ping(MonitorRequest::UP request, MonitorClient & client) ret.timestamp = (_matchEngine->isOnline()) ? 42 : 0; ret.activeDocs = (_matchEngine->isOnline()) ? getNumActiveDocs() : 0; ret.activeDocsRequested = request->reportActiveDocs; + ret.is_blocking_writes = !_diskMemUsageSampler->writeFilter().acceptWriteOperation(); return reply; } diff --git a/searchlib/src/tests/engine/proto_converter/proto_converter_test.cpp b/searchlib/src/tests/engine/proto_converter/proto_converter_test.cpp index 3e129457d46..16e4f93b6fb 100644 --- a/searchlib/src/tests/engine/proto_converter/proto_converter_test.cpp +++ b/searchlib/src/tests/engine/proto_converter/proto_converter_test.cpp @@ -535,6 +535,16 @@ TEST_F(MonitorReplyTest, require_that_distribution_key_is_converted) { EXPECT_EQ(proto.distribution_key(), 7); } +TEST_F(MonitorReplyTest, require_that_is_blocking_writes_is_converted) { + reply.is_blocking_writes = false; + convert(); + EXPECT_FALSE(proto.is_blocking_writes()); + + reply.is_blocking_writes = true; + convert(); + EXPECT_TRUE(proto.is_blocking_writes()); +} + //----------------------------------------------------------------------------- GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/util/ioerrorhandler/ioerrorhandler_test.cpp b/searchlib/src/tests/util/ioerrorhandler/ioerrorhandler_test.cpp index fb1c1a356f6..ec2831360c5 100644 --- a/searchlib/src/tests/util/ioerrorhandler/ioerrorhandler_test.cpp +++ b/searchlib/src/tests/util/ioerrorhandler/ioerrorhandler_test.cpp @@ -11,95 +11,66 @@ #include <iostream> #include <fstream> #include <string> -#include <setjmp.h> -#include <dlfcn.h> #include <unistd.h> +#include <vespa/fastos/file_rw_ops.h> #include <vespa/log/log.h> LOG_SETUP("ioerrorhandler_test"); -extern "C" { - -ssize_t read(int fd, void *buf, size_t count); -ssize_t write(int fd, const void *buf, size_t count); -ssize_t pread(int fd, void *buf, size_t count, off_t offset); -ssize_t pwrite(int fd, const void *buf, size_t count, off_t offset); - -} - -using ReadFunc = ssize_t (*)(int fd, void *buf, size_t count); -using WriteFunc = ssize_t (*)(int fd, const void *buf, size_t count); -using PreadFunc = ssize_t (*)(int fd, void *buf, size_t count, off_t offset); -using PwriteFunc = ssize_t (*)(int fd, const void *buf, size_t count, off_t offset); - using namespace search::test::statefile; using namespace search::test::statestring; -namespace { - -ReadFunc libc_read; -WriteFunc libc_write; -PreadFunc libc_pread; -PwriteFunc libc_pwrite; - -} - int injectErrno; std::atomic<int> injectreadErrnoTrigger; std::atomic<int> injectpreadErrnoTrigger; std::atomic<int> injectwriteErrnoTrigger; std::atomic<int> injectpwriteErrnoTrigger; -ssize_t read(int fd, void *buf, size_t count) +ssize_t error_injecting_read(int fd, void* buf, size_t count) { if (--injectreadErrnoTrigger == 0) { errno = injectErrno; return -1; } - if (!libc_read) { - libc_read = reinterpret_cast<ReadFunc>(dlsym(RTLD_NEXT, "read")); - } - return libc_read(fd, buf, count); + return read(fd, buf, count); } -ssize_t write(int fd, const void *buf, size_t count) +ssize_t error_injecting_write(int fd, const void* buf, size_t count) { if (--injectwriteErrnoTrigger == 0) { errno = injectErrno; return -1; } - if (!libc_write) { - libc_write = reinterpret_cast<WriteFunc>(dlsym(RTLD_NEXT, "write")); - } - return libc_write(fd, buf, count); + return write(fd, buf, count); } -ssize_t pread(int fd, void *buf, size_t count, off_t offset) +ssize_t error_injecting_pread(int fd, void* buf, size_t count, off_t offset) { if (--injectpreadErrnoTrigger == 0) { errno = injectErrno; return -1; } - if (!libc_pread) { - libc_pread = reinterpret_cast<PreadFunc>(dlsym(RTLD_NEXT, "pread")); - } - return libc_pread(fd, buf, count, offset); + return pread(fd, buf, count, offset); } -ssize_t pwrite(int fd, const void *buf, size_t count, off_t offset) +ssize_t error_injecting_pwrite(int fd, const void* buf, size_t count, off_t offset) { if (--injectpwriteErrnoTrigger == 0) { errno = injectErrno; return -1; } - if (!libc_pwrite) { - libc_pwrite = reinterpret_cast<PwriteFunc>(dlsym(RTLD_NEXT, "pwrite")); - } - return libc_pwrite(fd, buf, count, offset); + return pwrite(fd, buf, count, offset); } - +void setup_error_injections() +{ + using Ops = fastos::File_RW_Ops; + Ops::_read = error_injecting_read; + Ops::_write = error_injecting_write; + Ops::_pread = error_injecting_pread; + Ops::_pwrite = error_injecting_pwrite; +} namespace search { @@ -171,14 +142,6 @@ Fixture::openFile() } void -Fixture::openFileDIO() -{ - file.reset(new FastOS_File); - file->EnableDirectIO(); - file->OpenReadWrite("testfile"); -} - -void Fixture::writeTestString() { file->WriteBuf(testString, strlen(testString)); @@ -310,6 +273,15 @@ TEST_F("Test that ioerror handler can process write error", Fixture) } +#ifdef __linux__ +void +Fixture::openFileDIO() +{ + file.reset(new FastOS_File); + file->EnableDirectIO(); + file->OpenReadWrite("testfile"); +} + TEST_F("Test that ioerror handler can process pwrite error", Fixture) { IOErrorHandler ioeh(f.sf.get()); @@ -342,11 +314,13 @@ TEST_F("Test that ioerror handler can process pwrite error", Fixture) TEST_DO(assertHistory(exp, act)); } } +#endif } TEST_MAIN() { + setup_error_injections(); TEST_RUN_ALL(); search::StateFile::erase("state"); unlink("testfile"); diff --git a/searchlib/src/tests/util/sigbushandler/sigbushandler_test.cpp b/searchlib/src/tests/util/sigbushandler/sigbushandler_test.cpp index 658ad17544a..e687eff3ca7 100644 --- a/searchlib/src/tests/util/sigbushandler/sigbushandler_test.cpp +++ b/searchlib/src/tests/util/sigbushandler/sigbushandler_test.cpp @@ -66,16 +66,22 @@ TEST("Test that sigbus handler can trap synthetic sigbus") LOG_ABORT("Should never get here"); } EXPECT_TRUE(sbh.fired()); +#ifdef __APPLE__ + vespalib::string exp_state = "state=down ts=0.0 operation=sigbus errno=0 code=2 addr=0x0000000000000000\n"; +#else + vespalib::string exp_state = "state=down ts=0.0 operation=sigbus errno=0 code=0\n"; +#endif { vespalib::string act = readState(sf); normalizeTimestamp(act); - EXPECT_EQUAL("state=down ts=0.0 operation=sigbus errno=0 code=0\n", - act); + normalizeAddr(act, nullptr); + EXPECT_EQUAL(exp_state, act); } { - strvec exp({"state=down ts=0.0 operation=sigbus errno=0 code=0\n" }); + strvec exp({exp_state}); std::vector<vespalib::string> act(readHistory("state.history")); normalizeTimestamps(act); + normalizeAddrs(act, nullptr); TEST_DO(assertHistory(exp, act)); } } diff --git a/searchlib/src/vespa/searchlib/engine/monitorreply.cpp b/searchlib/src/vespa/searchlib/engine/monitorreply.cpp index c0f7fa0d42a..3414603a893 100644 --- a/searchlib/src/vespa/searchlib/engine/monitorreply.cpp +++ b/searchlib/src/vespa/searchlib/engine/monitorreply.cpp @@ -15,7 +15,8 @@ MonitorReply::MonitorReply() totalParts(), activeParts(), activeDocs(0), - flags() + flags(), + is_blocking_writes(false) { } } diff --git a/searchlib/src/vespa/searchlib/engine/monitorreply.h b/searchlib/src/vespa/searchlib/engine/monitorreply.h index 7d1f0ce1cef..f66a30fdd89 100644 --- a/searchlib/src/vespa/searchlib/engine/monitorreply.h +++ b/searchlib/src/vespa/searchlib/engine/monitorreply.h @@ -21,6 +21,7 @@ struct MonitorReply uint32_t activeParts; // mld uint64_t activeDocs; uint32_t flags; + bool is_blocking_writes; MonitorReply(); }; diff --git a/searchlib/src/vespa/searchlib/engine/proto_converter.cpp b/searchlib/src/vespa/searchlib/engine/proto_converter.cpp index e5846734a8d..2876fa99434 100644 --- a/searchlib/src/vespa/searchlib/engine/proto_converter.cpp +++ b/searchlib/src/vespa/searchlib/engine/proto_converter.cpp @@ -170,6 +170,7 @@ ProtoConverter::monitor_reply_to_proto(const MonitorReply &reply, ProtoMonitorRe proto.set_online(reply.timestamp != 0); proto.set_active_docs(reply.activeDocs); proto.set_distribution_key(reply.distribution_key); + proto.set_is_blocking_writes(reply.is_blocking_writes); } //----------------------------------------------------------------------------- diff --git a/searchlib/src/vespa/searchlib/expression/integerresultnode.h b/searchlib/src/vespa/searchlib/expression/integerresultnode.h index e428c0448bb..7f2ec823432 100644 --- a/searchlib/src/vespa/searchlib/expression/integerresultnode.h +++ b/searchlib/src/vespa/searchlib/expression/integerresultnode.h @@ -4,6 +4,7 @@ #include "numericresultnode.h" #include <vespa/vespalib/util/sort.h> #include <limits> +#include <type_traits> namespace search::expression { @@ -29,7 +30,13 @@ public: } void add(const ResultNode & b) override { _value += b.getInteger(); } void negate() override { _value = - _value; } - void multiply(const ResultNode & b) override { _value *= b.getInteger(); } + void multiply(const ResultNode & b) override { + if constexpr (std::is_same_v<T, bool>) { + _value = (_value && (b.getInteger() != 0)); + } else { + _value *= b.getInteger(); + } + } void divide(const ResultNode & b) override { int64_t val = b.getInteger(); _value = (val == 0) ? 0 : (_value / val); diff --git a/searchlib/src/vespa/searchlib/tensor/default_nearest_neighbor_index_factory.cpp b/searchlib/src/vespa/searchlib/tensor/default_nearest_neighbor_index_factory.cpp index 34950825652..067280e9a23 100644 --- a/searchlib/src/vespa/searchlib/tensor/default_nearest_neighbor_index_factory.cpp +++ b/searchlib/src/vespa/searchlib/tensor/default_nearest_neighbor_index_factory.cpp @@ -1,7 +1,6 @@ // Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "default_nearest_neighbor_index_factory.h" -#include "distance_functions.h" #include "hnsw_index.h" #include "random_level_generator.h" #include "inv_log_level_generator.h" diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_graph.cpp b/searchlib/src/vespa/searchlib/tensor/hnsw_graph.cpp index b6b3e670f08..10113160263 100644 --- a/searchlib/src/vespa/searchlib/tensor/hnsw_graph.cpp +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_graph.cpp @@ -81,3 +81,9 @@ HnswGraph::histograms() const } } // namespace + +namespace vespalib { + +template class RcuVectorBase<search::datastore::AtomicEntryRef>; + +} diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index_loader.h b/searchlib/src/vespa/searchlib/tensor/hnsw_index_loader.h index 174f66b95ec..9f5ae66011f 100644 --- a/searchlib/src/vespa/searchlib/tensor/hnsw_index_loader.h +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index_loader.h @@ -8,7 +8,7 @@ namespace search::fileutil { class LoadedBuffer; } namespace search::tensor { -class HnswGraph; +struct HnswGraph; /** * Implements loading of HNSW graph structure from binary format. diff --git a/searchlib/src/vespa/searchlib/test/statestring.cpp b/searchlib/src/vespa/searchlib/test/statestring.cpp index 33e47b3d961..480e6bbc275 100644 --- a/searchlib/src/vespa/searchlib/test/statestring.cpp +++ b/searchlib/src/vespa/searchlib/test/statestring.cpp @@ -7,7 +7,7 @@ namespace search::test::statestring { bool testStartPos(vespalib::string &s, size_t pos) { - return (pos < s.size() && (pos == 0 || s[pos - 1] == ' ')); + return ((pos >= s.size()) || (pos == 0) || (s[pos - 1] == ' ')); } size_t diff --git a/staging_vespalib/CMakeLists.txt b/staging_vespalib/CMakeLists.txt index e76d3078630..f2f8a41b68d 100644 --- a/staging_vespalib/CMakeLists.txt +++ b/staging_vespalib/CMakeLists.txt @@ -1,4 +1,9 @@ # Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +if(NOT CMAKE_HOST_SYSTEM_NAME STREQUAL "Darwin") +set(STAGING_VESPALIB_DIRECTIO_TESTDIR src/tests/directio) +set(STAGING_VESPALIB_PROCESS_MEMORY_STATS_TESTDIR src/tests/util/process_memory_stats) +endif() + vespa_define_module( DEPENDS fastos @@ -12,7 +17,7 @@ vespa_define_module( src/tests/bits src/tests/clock src/tests/crc - src/tests/directio + ${STAGING_VESPALIB_DIRECTIO_TESTDIR} src/tests/encoding/base64 src/tests/fileheader src/tests/floatingpointtype @@ -33,7 +38,7 @@ vespa_define_module( src/tests/sequencedtaskexecutor src/tests/singleexecutor src/tests/timer - src/tests/util/process_memory_stats + ${STAGING_VESPALIB_PROCESS_MEMORY_STATS_TESTDIR} src/tests/xmlserializable LIBS diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java index 5d6f0e3ce16..144ffaea5b4 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java @@ -219,6 +219,11 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen } @Override + public Path trustStorePath() { + return trustStore; + } + + @Override public List<X509Certificate> getIdentityCertificate() { return Collections.singletonList(credentials.getCertificate()); } diff --git a/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/FeederParams.java b/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/FeederParams.java index fde3e13ada3..cad8bb7b312 100644 --- a/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/FeederParams.java +++ b/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/FeederParams.java @@ -34,6 +34,7 @@ class FeederParams { private boolean benchmarkMode = false; private int numDispatchThreads = 1; private int maxPending = 0; + private double timeout = 180.0; private double windowSizeBackOff = 0.95; private double windowDecrementFactor = 1.2; @@ -100,6 +101,9 @@ class FeederParams { public double getWindowResizeRate() { return windowResizeRate; } + public double getTimeout() { + return timeout; + } public int getWindowIncrementSize() { return windowIncrementSize; @@ -137,6 +141,7 @@ class FeederParams { opts.addOption("b", "mode", true, "Mode for benchmarking."); opts.addOption("o", "output", true, "File to write to. Extensions gives format (.xml, .json, .vespa) json will be produced if no extension."); opts.addOption("c", "numconnections", true, "Number of connections per host."); + opts.addOption("t", "timeout", true, "Timeout for a message in seconds. default = " + timeout); opts.addOption("l", "nummessages", true, "Number of messages to send (all is default)."); opts.addOption("wi", "window_incrementsize", true, "Dynamic window increment step size. default = " + windowIncrementSize); opts.addOption("wd", "window_decrementfactor", true, "Dynamic window decrement step size factor. default = " + windowDecrementFactor); @@ -169,6 +174,9 @@ class FeederParams { if (cmd.hasOption('r')) { route = Route.parse(cmd.getOptionValue('r').trim()); } + if (cmd.hasOption("t")) { + timeout = Double.valueOf(cmd.getOptionValue("t").trim()); + } benchmarkMode = cmd.hasOption('b'); if (cmd.hasOption('o')) { String fileName = cmd.getOptionValue('o').trim(); diff --git a/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/SimpleFeeder.java b/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/SimpleFeeder.java index 18c0de339e6..14123c7a73c 100644 --- a/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/SimpleFeeder.java +++ b/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/SimpleFeeder.java @@ -129,11 +129,13 @@ public class SimpleFeeder implements ReplyHandler { private final PrintStream err; private final Route route; private final SourceSession session; + private final long timeoutMS; private final AtomicReference<Throwable> failure; - MbusDestination(SourceSession session, Route route, AtomicReference<Throwable> failure, PrintStream err) { + MbusDestination(SourceSession session, Route route, double timeoutS, AtomicReference<Throwable> failure, PrintStream err) { this.route = route; this.err = err; this.session = session; + this.timeoutMS = (long)(timeoutS * 1000.0); this.failure = failure; } public void send(FeedOperation op) { @@ -142,6 +144,7 @@ public class SimpleFeeder implements ReplyHandler { err.println("ignoring operation; " + op.getType()); return; } + msg.setTimeRemaining(timeoutMS); msg.setContext(System.currentTimeMillis()); msg.setRoute(route); try { @@ -352,7 +355,7 @@ public class SimpleFeeder implements ReplyHandler { benchmarkMode = params.isBenchmarkMode(); destination = (params.getDumpStream() != null) ? createDumper(params) - : new MbusDestination(session, params.getRoute(), failure, params.getStdErr()); + : new MbusDestination(session, params.getRoute(), params.getTimeout(), failure, params.getStdErr()); } SourceSession getSourceSession() { return session; } diff --git a/vespa_feed_perf/src/test/java/com/yahoo/vespa/feed/perf/FeederParamsTest.java b/vespa_feed_perf/src/test/java/com/yahoo/vespa/feed/perf/FeederParamsTest.java index d4b114112b6..6f575038f75 100644 --- a/vespa_feed_perf/src/test/java/com/yahoo/vespa/feed/perf/FeederParamsTest.java +++ b/vespa_feed_perf/src/test/java/com/yahoo/vespa/feed/perf/FeederParamsTest.java @@ -28,6 +28,8 @@ public class FeederParamsTest { private static final String TESTFILE_JSON = "test.json"; private static final String TESTFILE_VESPA = "test.vespa"; private static final String TESTFILE_UNKNOWN = "test.xyz"; + private static final double EPSILON = 0.000000000001; + @Test public void requireThatAccessorsWork() { @@ -94,6 +96,13 @@ public class FeederParamsTest { } @Test + public void requireThatTimeoutIsParsed() throws ParseException, FileNotFoundException { + assertEquals(180.0, new FeederParams().getTimeout(), EPSILON); + assertEquals(16.7, new FeederParams().parseArgs("-t 16.7").getTimeout(), EPSILON); + assertEquals(1700.9, new FeederParams().parseArgs("--timeout", "1700.9").getTimeout(), EPSILON); + } + + @Test public void requireThatNumMessagesToSendAreParsed() throws ParseException, FileNotFoundException { assertEquals(Long.MAX_VALUE, new FeederParams().getNumMessagesToSend()); assertEquals(18, new FeederParams().parseArgs("-l 18").getNumMessagesToSend()); @@ -106,8 +115,6 @@ public class FeederParamsTest { assertEquals(17, new FeederParams().parseArgs("--window_incrementsize", "17").getWindowIncrementSize()); } - static final double EPSILON = 0.000000000001; - @Test public void requireThatWindowSizeDecrementFactorIsParsed() throws ParseException, FileNotFoundException { assertEquals(1.2, new FeederParams().getWindowDecrementFactor(), EPSILON); diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java index 0485689b15d..1224e668bc0 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java @@ -35,6 +35,7 @@ import java.time.Instant; import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; /** * Sends operations to messagebus via document api. @@ -352,12 +353,16 @@ public class OperationHandlerImpl implements OperationHandler { protected static ClusterDef resolveClusterDef(Optional<String> wantedCluster, List<ClusterDef> clusters) throws RestApiException { if (clusters.size() == 0) { throw new IllegalArgumentException("Your Vespa cluster does not have any content clusters " + - "declared. Visiting feature is not available."); + "declared. Visiting feature is not available."); } if (! wantedCluster.isPresent()) { if (clusters.size() != 1) { - throw new RestApiException(Response.createErrorResponse(400, "Several clusters exist: " + - clusterListToString(clusters) + " you must specify one. ", RestUri.apiErrorCodes.SEVERAL_CLUSTERS)); + String message = "Several clusters exist: " + + clusters.stream().map(c -> "'" + c.getName() + "'").collect(Collectors.joining(", ")) + + ". You must specify one."; + throw new RestApiException(Response.createErrorResponse(400, + message, + RestUri.apiErrorCodes.SEVERAL_CLUSTERS)); } return clusters.get(0); } @@ -367,20 +372,18 @@ public class OperationHandlerImpl implements OperationHandler { return clusterDef; } } - throw new RestApiException(Response.createErrorResponse(400, "Your vespa cluster contains the content clusters " + - clusterListToString(clusters) + " not " + wantedCluster.get() + ". Please select a valid vespa cluster.", RestUri.apiErrorCodes.MISSING_CLUSTER)); + String message = "Your vespa cluster contains the content clusters " + + clusters.stream().map(c -> "'" + c.getName() + "'").collect(Collectors.joining(", ")) + + ", not '" + wantedCluster.get() + "'. Please select a valid vespa cluster."; + throw new RestApiException(Response.createErrorResponse(400, + message, + RestUri.apiErrorCodes.MISSING_CLUSTER)); } protected static String clusterDefToRoute(ClusterDef clusterDef) { return "[Storage:cluster=" + clusterDef.getName() + ";clusterconfigid=" + clusterDef.getConfigId() + "]"; } - private static String clusterListToString(List<ClusterDef> clusters) { - StringBuilder clusterListString = new StringBuilder(); - clusters.forEach(x -> clusterListString.append(x.getName()).append(" (").append(x.getConfigId()).append("), ")); - return clusterListString.toString(); - } - private static String buildAugmentedDocumentSelection(RestUri restUri, String documentSelection) { if (restUri.isRootOnly()) { return documentSelection; // May be empty, that's fine. diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/OperationHandlerImplTest.java b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/OperationHandlerImplTest.java index 4e5092cd416..bda49ecd3f5 100644 --- a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/OperationHandlerImplTest.java +++ b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/OperationHandlerImplTest.java @@ -94,8 +94,8 @@ public class OperationHandlerImplTest { assertThat(e.getResponse().getStatus(), is(400)); String errorMsg = renderRestApiExceptionAsString(e); assertThat(errorMsg, is("{\"errors\":[{\"description\":" + - "\"MISSING_CLUSTER Your vespa cluster contains the content clusters foo2 (configId2), foo (configId)," + - " foo3 (configId2), not wrong. Please select a valid vespa cluster.\",\"id\":-9}]}")); + "\"MISSING_CLUSTER Your vespa cluster contains the content clusters 'foo2', 'foo'," + + " 'foo3', not 'wrong'. Please select a valid vespa cluster.\",\"id\":-9}]}")); return; } fail("Expected exception"); diff --git a/vespaclient-core/src/main/java/com/yahoo/feedapi/MessagePropertyProcessor.java b/vespaclient-core/src/main/java/com/yahoo/feedapi/MessagePropertyProcessor.java index 74a4f916c07..581a51ffc02 100644 --- a/vespaclient-core/src/main/java/com/yahoo/feedapi/MessagePropertyProcessor.java +++ b/vespaclient-core/src/main/java/com/yahoo/feedapi/MessagePropertyProcessor.java @@ -208,14 +208,6 @@ public class MessagePropertyProcessor implements ConfigSubscriber.SingleSubscrib this.route = route; } - public long getTimeout() { - return timeout; - } - - public void setTimeout(long timeout) { - this.timeout = timeout; - } - public DocumentProtocol.Priority getPriority() { return priority; } diff --git a/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisit.java b/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisit.java index 307cf979c33..83199c76e5a 100644 --- a/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisit.java +++ b/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisit.java @@ -25,6 +25,7 @@ import org.apache.commons.cli.Options; import java.io.*; import java.nio.charset.Charset; import java.util.Map; +import java.util.stream.Collectors; /** * Client using visiting, used by the vespa-visit command line tool. @@ -572,18 +573,14 @@ public class VdsVisit { protected static String resolveClusterRoute(ClusterList clusters, String wantedCluster) { if (clusters.getStorageClusters().size() == 0) { throw new IllegalArgumentException("Your Vespa cluster does not have any content clusters " + - "declared. Visiting feature is not available."); + "declared. Visiting feature is not available."); } ClusterDef found = null; - String names = ""; - for (ClusterDef c : clusters.getStorageClusters()) { - if (!names.isEmpty()) { - names += ", "; - } - names += c.getName(); - } + String names = clusters.getStorageClusters() + .stream().map(c -> "'" + c.getName() + "'") + .collect(Collectors.joining(", ")); if (wantedCluster != null) { for (ClusterDef c : clusters.getStorageClusters()) { if (c.getName().equals(wantedCluster)) { @@ -592,13 +589,14 @@ public class VdsVisit { } if (found == null) { throw new IllegalArgumentException("Your vespa cluster contains the content clusters " + - names + ", not " + wantedCluster + ". Please select a valid vespa cluster."); + names + ", not '" + wantedCluster + + "'. Please select a valid vespa cluster."); } } else if (clusters.getStorageClusters().size() == 1) { found = clusters.getStorageClusters().get(0); } else { throw new IllegalArgumentException("Your vespa cluster contains the content clusters " + - names + ". Please use the -c option to select one of them as a target for visiting."); + names + ". Please use the -c option to select one of them as a target for visiting."); } return "[Storage:cluster=" + found.getName() + ";clusterconfigid=" + found.getConfigId() + "]"; diff --git a/vespaclient-java/src/test/java/com/yahoo/vespavisit/VdsVisitTestCase.java b/vespaclient-java/src/test/java/com/yahoo/vespavisit/VdsVisitTestCase.java index 20f3cbcfe23..d9efe2c5129 100644 --- a/vespaclient-java/src/test/java/com/yahoo/vespavisit/VdsVisitTestCase.java +++ b/vespaclient-java/src/test/java/com/yahoo/vespavisit/VdsVisitTestCase.java @@ -242,7 +242,9 @@ public class VdsVisitTestCase { try { VdsVisit.resolveClusterRoute(clusterList, "borkbork"); } catch (IllegalArgumentException e) { - assertTrue(e.getMessage().contains("Your vespa cluster contains the content clusters storage, not borkbork.")); + assertEquals("Your vespa cluster contains the content clusters 'storage', not 'borkbork'. " + + "Please select a valid vespa cluster.", + e.getMessage()); } } diff --git a/vespalib/src/tests/exception_classes/silenceuncaught_test.cpp b/vespalib/src/tests/exception_classes/silenceuncaught_test.cpp index ebb98823033..f94a298d19f 100644 --- a/vespalib/src/tests/exception_classes/silenceuncaught_test.cpp +++ b/vespalib/src/tests/exception_classes/silenceuncaught_test.cpp @@ -35,6 +35,9 @@ TEST("that mmap within limits are fine cause exitcode 0") { EXPECT_EQUAL(proc.getExitCode(), 0); } +#ifdef __APPLE__ +// setrlimit with RLIMIT_AS is broken on Darwin +#else TEST("that mmap beyond limits cause negative exitcode.") { SlaveProc proc("ulimit -c 0 && exec ./vespalib_mmap_app 100000000 10485760 10"); proc.wait(); @@ -46,5 +49,6 @@ TEST("that mmap beyond limits with set VESPA_SILENCE_CORE_ON_OOM cause exitcode proc.wait(); EXPECT_EQUAL(proc.getExitCode(), 66); } +#endif TEST_MAIN_WITH_PROCESS_PROXY() { TEST_RUN_ALL(); } diff --git a/vespalib/src/vespa/vespalib/test/socket_options_verifier.h b/vespalib/src/vespa/vespalib/test/socket_options_verifier.h index 72694f643e2..8f9901e8acc 100644 --- a/vespalib/src/vespa/vespalib/test/socket_options_verifier.h +++ b/vespalib/src/vespa/vespalib/test/socket_options_verifier.h @@ -18,7 +18,7 @@ void verify_bool_opt(int fd, int level, int name, bool expect) { socklen_t size = sizeof(data); EXPECT_EQUAL(getsockopt(fd, level, name, &data, &size), 0); EXPECT_EQUAL(size, sizeof(data)); - EXPECT_EQUAL(data, int(expect)); + EXPECT_EQUAL(data != 0, expect); } } // namespace vespalib::test::<unnamed> diff --git a/zkfacade/abi-spec.json b/zkfacade/abi-spec.json index 05fb985dbaf..f4ad1ab4372 100644 --- a/zkfacade/abi-spec.json +++ b/zkfacade/abi-spec.json @@ -85,6 +85,7 @@ "public java.util.List getChildren(com.yahoo.path.Path)", "public java.util.Optional getData(com.yahoo.path.Path)", "public java.util.Optional getStat(com.yahoo.path.Path)", + "public com.yahoo.vespa.curator.Lock lock(com.yahoo.path.Path, java.time.Duration)", "public org.apache.curator.framework.CuratorFramework framework()", "public void close()", "public java.lang.String zooKeeperEnsembleConnectionSpec()", diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java index 7c5b1ae319a..6d4f6beece1 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java @@ -32,6 +32,7 @@ import java.time.Duration; import java.util.Arrays; import java.util.List; import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.function.Function; import java.util.logging.Logger; @@ -48,27 +49,26 @@ import java.util.logging.Logger; */ public class Curator implements AutoCloseable { - private static final Logger logger = Logger.getLogger(Curator.class.getName()); - - private static final int ZK_SESSION_TIMEOUT = 30000; - private static final int ZK_CONNECTION_TIMEOUT = 30000; - - private static final int BASE_SLEEP_TIME = 1000; //ms + private static final Logger LOG = Logger.getLogger(Curator.class.getName()); + private static final File ZK_CLIENT_CONFIG_FILE = new File(Defaults.getDefaults().underVespaHome("conf/zookeeper/zookeeper-client.cfg")); + private static final Duration ZK_SESSION_TIMEOUT = Duration.ofSeconds(30); + private static final Duration ZK_CONNECTION_TIMEOUT = Duration.ofSeconds(30); + private static final Duration BASE_SLEEP_TIME = Duration.ofSeconds(1); private static final int MAX_RETRIES = 10; - private static final File zkClientConfigFile = new File(Defaults.getDefaults().underVespaHome("conf/zookeeper/zookeeper-client.cfg")); - protected final RetryPolicy retryPolicy; private final CuratorFramework curatorFramework; private final String connectionSpec; // May be a subset of the servers in the ensemble - private final String zooKeeperEnsembleConnectionSpec; private final int zooKeeperEnsembleCount; + // All lock keys, to allow re-entrancy. This will grow forever, but this should be too slow to be a problem + private final ConcurrentHashMap<Path, Lock> locks = new ConcurrentHashMap<>(); + /** Creates a curator instance from a comma-separated string of ZooKeeper host:port strings */ public static Curator create(String connectionSpec) { - return new Curator(connectionSpec, connectionSpec, Optional.of(zkClientConfigFile)); + return new Curator(connectionSpec, connectionSpec, Optional.of(ZK_CLIENT_CONFIG_FILE)); } // For testing only, use Optional.empty for clientConfigFile parameter to create default zookeeper client config @@ -80,7 +80,7 @@ public class Curator implements AutoCloseable { // TODO: Move zookeeperserver config out of configserverconfig (requires update of controller services.xml as well) @Inject public Curator(ConfigserverConfig configserverConfig, VespaZooKeeperServer server) { - this(configserverConfig, Optional.of(zkClientConfigFile)); + this(configserverConfig, Optional.of(ZK_CLIENT_CONFIG_FILE)); } Curator(ConfigserverConfig configserverConfig, Optional<File> clientConfigFile) { @@ -93,8 +93,8 @@ public class Curator implements AutoCloseable { (retryPolicy) -> CuratorFrameworkFactory .builder() .retryPolicy(retryPolicy) - .sessionTimeoutMs(ZK_SESSION_TIMEOUT) - .connectionTimeoutMs(ZK_CONNECTION_TIMEOUT) + .sessionTimeoutMs((int) ZK_SESSION_TIMEOUT.toMillis()) + .connectionTimeoutMs((int) ZK_CONNECTION_TIMEOUT.toMillis()) .connectString(connectionSpec) .zookeeperFactory(new VespaZooKeeperFactory(createClientConfig(clientConfigFile))) .dontUseContainerParents() // TODO: Remove when we know ZooKeeper 3.5 works fine, consider waiting until Vespa 8 @@ -105,7 +105,7 @@ public class Curator implements AutoCloseable { String zooKeeperEnsembleConnectionSpec, Function<RetryPolicy, CuratorFramework> curatorFactory) { this(connectionSpec, zooKeeperEnsembleConnectionSpec, curatorFactory, - new ExponentialBackoffRetry(BASE_SLEEP_TIME, MAX_RETRIES)); + new ExponentialBackoffRetry((int) BASE_SLEEP_TIME.toMillis(), MAX_RETRIES)); } private Curator(String connectionSpec, @@ -193,7 +193,7 @@ public class Curator implements AutoCloseable { /** For internal use; prefer creating a {@link CuratorCounter} */ public DistributedAtomicLong createAtomicCounter(String path) { - return new DistributedAtomicLong(curatorFramework, path, new ExponentialBackoffRetry(BASE_SLEEP_TIME, MAX_RETRIES)); + return new DistributedAtomicLong(curatorFramework, path, new ExponentialBackoffRetry((int) BASE_SLEEP_TIME.toMillis(), MAX_RETRIES)); } /** For internal use; prefer creating a {@link com.yahoo.vespa.curator.Lock} */ @@ -204,9 +204,9 @@ public class Curator implements AutoCloseable { private void addLoggingListener() { curatorFramework.getConnectionStateListenable().addListener((curatorFramework, connectionState) -> { switch (connectionState) { - case SUSPENDED: logger.info("ZK connection state change: SUSPENDED"); break; - case RECONNECTED: logger.info("ZK connection state change: RECONNECTED"); break; - case LOST: logger.warning("ZK connection state change: LOST"); break; + case SUSPENDED: LOG.info("ZK connection state change: SUSPENDED"); break; + case RECONNECTED: LOG.info("ZK connection state change: RECONNECTED"); break; + case LOST: LOG.warning("ZK connection state change: LOST"); break; } }); } @@ -351,6 +351,14 @@ public class Curator implements AutoCloseable { } } + /** Create and acquire a re-entrant lock in given path */ + public Lock lock(Path path, Duration timeout) { + create(path); + Lock lock = locks.computeIfAbsent(path, (pathArg) -> new Lock(pathArg.getAbsolute(), this)); + lock.acquire(timeout); + return lock; + } + /** Returns the curator framework API */ public CuratorFramework framework() { return curatorFramework; diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java index 2e554d39e44..d97d8f5ed71 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.curator; import com.google.common.util.concurrent.UncheckedTimeoutException; +import com.yahoo.path.Path; import com.yahoo.transaction.Mutex; import org.apache.curator.framework.recipes.locks.InterProcessLock; @@ -10,7 +11,10 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantLock; /** - * A cluster-wide re-entrant mutex which is released on (the last symmetric) close + * A cluster-wide re-entrant mutex which is released on (the last symmetric) close. + * + * Re-entrancy is limited to the instance of this. To ensure re-entrancy callers should access the lock through + * {@link Curator#lock(Path, Duration)} instead of constructing this directly. * * @author bratseth */ |