diff options
68 files changed, 1234 insertions, 475 deletions
diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java index f46fe39e8c4..818f7498af4 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java @@ -90,9 +90,6 @@ public class DeploymentSpec { /** Adds missing required steps and reorders steps to a permissible order */ private static List<Step> completeSteps(List<Step> steps) { - // Ensure no duplicate deployments to the same zone - steps = new ArrayList<>(new LinkedHashSet<>(steps)); - // Add staging if required and missing if (steps.stream().anyMatch(step -> step.deploysTo(Environment.prod)) && steps.stream().noneMatch(step -> step.deploysTo(Environment.staging))) { diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java b/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java index a5d54fb84b3..d2365bcbf00 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java @@ -22,8 +22,12 @@ public interface FileDistribution { void limitSendingOfDeployedFilesTo(Collection<String> hostNames); void removeDeploymentsThatHaveDifferentApplicationId(Collection<String> targetHostnames); + static String getDefaultFileDBRoot() { + return Defaults.getDefaults().underVespaHome("var/db/vespa/filedistribution"); + } + static File getDefaultFileDBPath() { - return new File(Defaults.getDefaults().underVespaHome("var/db/vespa/filedistribution")); + return new File(getDefaultFileDBRoot()); } } diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index 5b79415c132..eef90975035 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -10,6 +10,7 @@ import com.yahoo.config.provision.Rotation; import com.yahoo.config.provision.Zone; import java.io.File; +import java.net.URI; import java.util.List; import java.util.Optional; import java.util.Set; @@ -41,6 +42,7 @@ public interface ModelContext { boolean multitenant(); ApplicationId applicationId(); List<ConfigServerSpec> configServerSpecs(); + URI loadBalancerAddress(); boolean hostedVespa(); Zone zone(); Set<Rotation> rotations(); diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java index 8bab2f83448..5050f88af31 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java @@ -97,12 +97,10 @@ public class DeploymentSpecTest { StringReader r = new StringReader( "<deployment version='1.0'>" + " <test/>" + - " <test/>" + " <staging/>" + " <prod>" + " <region active='false'>us-east1</region>" + - " <region active='false'>us-east1</region>" + - " <delay hours='3' minutes='30'/>" + + " <delay hours='3' minutes='30'/>" + " <region active='true'>us-west1</region>" + " </prod>" + "</deployment>" diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployProperties.java index aec4c5b3ec6..942320ecd40 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployProperties.java @@ -2,8 +2,11 @@ package com.yahoo.config.model.deploy; import com.yahoo.config.model.api.ConfigServerSpec; -import com.yahoo.config.provision.*; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.Version; +import com.yahoo.config.provision.Zone; +import java.net.URI; import java.util.ArrayList; import java.util.List; @@ -18,6 +21,7 @@ public class DeployProperties { private final boolean multitenant; private final ApplicationId applicationId; private final List<ConfigServerSpec> serverSpecs = new ArrayList<>(); + private final URI loadBalancerAddress; private final boolean hostedVespa; private final Version vespaVersion; private final Zone zone; @@ -25,7 +29,11 @@ public class DeployProperties { private DeployProperties(boolean multitenant, ApplicationId applicationId, List<ConfigServerSpec> configServerSpecs, - boolean hostedVespa, Version vespaVersion, Zone zone) { + URI loadBalancerAddress, + boolean hostedVespa, + Version vespaVersion, + Zone zone) { + this.loadBalancerAddress = loadBalancerAddress; this.vespaVersion = vespaVersion; this.zone = zone; this.multitenant = multitenant || hostedVespa || Boolean.getBoolean("multitenant"); @@ -47,6 +55,10 @@ public class DeployProperties { return serverSpecs; } + public URI loadBalancerAddress() { + return loadBalancerAddress; + } + public boolean hostedVespa() { return hostedVespa; } @@ -63,6 +75,7 @@ public class DeployProperties { private ApplicationId applicationId = ApplicationId.defaultId(); private boolean multitenant = false; private List<ConfigServerSpec> configServerSpecs = new ArrayList<>(); + private URI loadBalancerAddress; private boolean hostedVespa = false; private Version vespaVersion = Version.fromIntValues(1, 0, 0); private Zone zone = Zone.defaultZone(); @@ -82,6 +95,11 @@ public class DeployProperties { return this; } + public Builder loadBalancerAddress(URI loadBalancerAddress) { + this.loadBalancerAddress = loadBalancerAddress; + return this; + } + public Builder vespaVersion(Version version) { this.vespaVersion = version; return this; @@ -98,7 +116,7 @@ public class DeployProperties { } public DeployProperties build() { - return new DeployProperties(multitenant, applicationId, configServerSpecs, hostedVespa, vespaVersion, zone); + return new DeployProperties(multitenant, applicationId, configServerSpecs, loadBalancerAddress, hostedVespa, vespaVersion, zone); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java b/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java index fc27f9e8dc7..56db1542de8 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java @@ -147,6 +147,7 @@ public class VespaModelFactory implements ModelFactory { return new DeployProperties.Builder() .applicationId(properties.applicationId()) .configServerSpecs(properties.configServerSpecs()) + .loadBalancerAddress(properties.loadBalancerAddress()) .multitenant(properties.multitenant()) .hostedVespa(properties.hostedVespa()) .vespaVersion(getVersion()) diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ConfigServerContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ConfigServerContainerModelBuilder.java index c9cc51af867..4004d72c808 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ConfigServerContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ConfigServerContainerModelBuilder.java @@ -2,15 +2,12 @@ package com.yahoo.vespa.model.container.xml; import com.yahoo.config.application.Xml; -import com.yahoo.config.model.ConfigModelContext; import com.yahoo.config.application.api.ApplicationPackage; - +import com.yahoo.config.model.ConfigModelContext; import com.yahoo.path.Path; -import com.yahoo.vespa.model.container.ContainerCluster; import com.yahoo.vespa.model.container.ContainerModel; -import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions; import com.yahoo.vespa.model.container.configserver.ConfigserverCluster; - +import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions; import org.w3c.dom.Element; import org.w3c.dom.Node; @@ -54,14 +51,4 @@ public class ConfigServerContainerModelBuilder extends ContainerModelBuilder { } } } - - @Override - protected void addDefaultComponents(ContainerCluster containerCluster) { - // To avoid search specific stuff. - } - - @Override - protected void addDefaultHandlers(ContainerCluster containerCluster) { - addDefaultHandlersExceptStatus(containerCluster); - } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index ce9d0ed27f1..32f2a59a881 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -18,6 +18,7 @@ import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.NodeType; import com.yahoo.container.jdisc.config.MetricDefaultsConfig; +import com.yahoo.log.LogLevel; import com.yahoo.search.rendering.RendererRegistry; import com.yahoo.text.XML; import com.yahoo.vespa.defaults.Defaults; @@ -58,12 +59,14 @@ import com.yahoo.vespa.model.content.StorageGroup; import org.w3c.dom.Element; import org.w3c.dom.Node; +import java.net.URI; import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.function.Consumer; +import java.util.logging.Logger; import java.util.stream.Collectors; /** @@ -93,6 +96,8 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { private static final String xmlRendererId = RendererRegistry.xmlRendererId.getName(); private static final String jsonRendererId = RendererRegistry.jsonRendererId.getName(); + private static final Logger logger = Logger.getLogger(ContainerModelBuilder.class.getName()); + public ContainerModelBuilder(boolean standaloneBuilder, Networking networking) { super(ContainerModel.class); this.standaloneBuilder = standaloneBuilder; @@ -163,7 +168,8 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { // Athenz copper argos // NOTE: Must be done after addNodes() - addIdentity(spec, cluster, context.getDeployState().getProperties().configServerSpecs()); + addIdentity(spec, cluster, context.getDeployState().getProperties().configServerSpecs(), + context.getDeployState().getProperties().loadBalancerAddress()); //TODO: overview handler, see DomQrserverClusterBuilder } @@ -691,13 +697,19 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { } } - private void addIdentity(Element element, ContainerCluster cluster, List<ConfigServerSpec> configServerSpecs) { + private void addIdentity(Element element, ContainerCluster cluster, List<ConfigServerSpec> configServerSpecs, URI loadBalancerAddress) { Element identityElement = XML.getChild(element, "identity"); if(identityElement != null) { String domain = XML.getValue(XML.getChild(identityElement, "domain")); String service = XML.getValue(XML.getChild(identityElement, "service")); + // TODO: Remove after verifying that this is propagated correctly + logger.log(LogLevel.INFO, String.format("loadBalancerAddress: %s", loadBalancerAddress)); + // TODO: Inject the load balancer address. For now only add first configserver + // TODO: The loadBalancerAddress is a URI, not specific host. + // TODO: Either rename loadBalancerAddress -> loadBalancerName (or similar) or + // TODO: make consumers of it use URI. String cfgHostName = configServerSpecs.stream().findFirst().map(ConfigServerSpec::getHostName) .orElse(""); // How to test this? diff --git a/config-model/src/test/java/com/yahoo/config/model/MockModelContext.java b/config-model/src/test/java/com/yahoo/config/model/MockModelContext.java index 83b5003579e..ff37fb1fad3 100644 --- a/config-model/src/test/java/com/yahoo/config/model/MockModelContext.java +++ b/config-model/src/test/java/com/yahoo/config/model/MockModelContext.java @@ -14,6 +14,7 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Rotation; import com.yahoo.config.provision.Zone; +import java.net.URI; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -95,6 +96,11 @@ public class MockModelContext implements ModelContext { } @Override + public URI loadBalancerAddress() { + return null; + } + + @Override public boolean hostedVespa() {return false; } @Override diff --git a/config-model/src/test/java/com/yahoo/vespa/model/VespaModelFactoryTest.java b/config-model/src/test/java/com/yahoo/vespa/model/VespaModelFactoryTest.java index cef94c97a2c..873883716e4 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/VespaModelFactoryTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/VespaModelFactoryTest.java @@ -27,6 +27,7 @@ import com.yahoo.config.provision.Zone; import org.junit.Before; import org.junit.Test; +import java.net.URI; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; @@ -193,6 +194,11 @@ public class VespaModelFactoryTest { public List<ConfigServerSpec> configServerSpecs() { return Collections.emptyList(); } + + @Override + public URI loadBalancerAddress() { + return null; + } }; } }; diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java index 1fced0b1e3d..ae0360fecf2 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java @@ -11,8 +11,12 @@ import com.yahoo.vespa.config.protocol.JRTServerConfigRequest; import com.yahoo.vespa.config.protocol.JRTServerConfigRequestV3; import java.io.File; -import java.lang.*; -import java.util.*; +import java.util.Arrays; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; import java.util.logging.Logger; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -109,12 +113,12 @@ public class ConfigProxyRpcServer implements Runnable, TargetWatcher, RpcServer // Legacy method, needs to be the same name as used in filedistributor supervisor.addMethod(new Method("waitFor", "s", "s", this, "getFile") - .methodDesc("wait for file reference") + .methodDesc("get path to file reference") .paramDesc(0, "file reference", "file reference") .returnDesc(0, "path", "path to file")); supervisor.addMethod(new Method("filedistribution.getFile", "s", "s", this, "getFile") - .methodDesc("wait for file reference") + .methodDesc("get path to file reference") .paramDesc(0, "file reference", "file reference") .returnDesc(0, "path", "path to file")); supervisor.addMethod(new Method("filedistribution.getActiveFileReferencesStatus", "", "SD", @@ -127,6 +131,16 @@ public class ConfigProxyRpcServer implements Runnable, TargetWatcher, RpcServer .methodDesc("set which file references to download") .paramDesc(0, "file references", "file reference to download") .returnDesc(0, "ret", "0 if success, 1 otherwise")); + supervisor.addMethod(new Method("filedistribution.receiveFile", "ssxlis", "i", // TODO Temporary method to get started with testing + this, "receiveFile") + .methodDesc("receive file reference content") + .paramDesc(0, "file references", "file reference to download") + .paramDesc(1, "filename", "filename") + .paramDesc(2, "content", "array of bytes") + .paramDesc(3, "hash", "xx64hash of the file content") + .paramDesc(4, "errorcode", "Error code. 0 if none") + .paramDesc(5, "error-description", "Error description.") + .returnDesc(0, "ret", "0 if success, 1 otherwise")); } //---------------- RPC methods ------------------------------------ @@ -235,17 +249,33 @@ public class ConfigProxyRpcServer implements Runnable, TargetWatcher, RpcServer req.returnValues().add(new StringValue(memoryCache.dumpCacheToDisk(req.parameters().get(0).asString(), memoryCache))); } + // TODO: Duplicate of code in FileAcquirereImpl. Find out where to put it. What about C++ code using this RPC call? + private static final int baseErrorCode = 0x10000; + private static final int baseFileProviderErrorCode = baseErrorCode + 0x1000; + + private static final int fileReferenceDoesNotExists = baseFileProviderErrorCode; + private static final int fileReferenceRemoved = fileReferenceDoesNotExists + 1; + private static final int fileReferenceInternalError = fileReferenceRemoved + 1; + @SuppressWarnings({"UnusedDeclaration"}) public final void getFile(Request req) { - // TODO: Detach to avoid holding transport thread + req.detach(); FileReference fileReference = new FileReference(req.parameters().get(0).asString()); - String pathToFile = proxyServer.fileDownloader() - .getFile(fileReference) - .orElseGet(() -> new File("")) - .getAbsolutePath(); - - log.log(LogLevel.INFO, "File reference '" + fileReference.value() + "' available at " + pathToFile); - req.returnValues().add(new StringValue(pathToFile)); + log.log(LogLevel.DEBUG, "getFile() called for file reference '" + fileReference.value() + "'"); + Optional<File> pathToFile = proxyServer.fileDownloader().getFile(fileReference); + try { + if (pathToFile.isPresent()) { + req.returnValues().add(new StringValue(pathToFile.get().getAbsolutePath())); + log.log(LogLevel.INFO, "File reference '" + fileReference.value() + "' available at " + pathToFile.get()); + } else { + log.log(LogLevel.INFO, "File reference '" + fileReference.value() + "' not found, returning error"); + req.setError(fileReferenceDoesNotExists, "File reference '" + fileReference.value() + "' not found"); + } + } catch (Throwable e) { + log.log(LogLevel.WARNING, "File reference '" + fileReference.value() + "' got exeption: " + e.getMessage()); + req.setError(fileReferenceInternalError, "File reference '" + fileReference.value() + "' removed"); + } + req.returnRequest(); } @SuppressWarnings({"UnusedDeclaration"}) @@ -279,6 +309,15 @@ public class ConfigProxyRpcServer implements Runnable, TargetWatcher, RpcServer req.returnValues().add(new Int32Value(0)); } + @SuppressWarnings({"UnusedDeclaration"}) + public final void receiveFile(Request req) { + FileReference fileReference = new FileReference(req.parameters().get(0).asString()); + String filename = req.parameters().get(1).asString(); + byte[] content = req.parameters().get(2).asData(); + proxyServer.fileDownloader().receiveFile(fileReference, filename, content); + req.returnValues().add(new Int32Value(0)); + } + //---------------------------------------------------- private boolean isProtocolVersionSupported(JRTServerConfigRequest request) { diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java index 4ee77beb2d7..5668852311f 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java @@ -83,7 +83,7 @@ public class ProxyServer implements Runnable { this.rpcServer = createRpcServer(spec); clientUpdater = new ClientUpdater(rpcServer, statistics, delayedResponses); this.configClient = createClient(clientUpdater, delayedResponses, source, timingValues, memoryCache, configClient); - this.fileDownloader = new FileDownloader(source); + this.fileDownloader = new FileDownloader(new JRTConnectionPool(source)); } static ProxyServer createTestServer(ConfigSourceSet source) { diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileDownloader.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileDownloader.java index 9074527e4e4..f3c694f31ab 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileDownloader.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileDownloader.java @@ -1,98 +1,153 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.proxy.filedistribution; -import com.google.common.collect.ImmutableSet; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.SettableFuture; import com.yahoo.config.FileReference; -import com.yahoo.config.subscription.ConfigSourceSet; import com.yahoo.log.LogLevel; +import com.yahoo.vespa.config.ConnectionPool; import com.yahoo.vespa.defaults.Defaults; +import com.yahoo.yolean.Exceptions; import java.io.File; import java.time.Duration; -import java.time.Instant; -import java.util.*; + +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.logging.Logger; /** - * Keeps track of files to download and download status + * Handles downloads of files (file references only for now) * * @author hmusum */ public class FileDownloader { - private final static Logger log = Logger.getLogger(FileDownloader.class.getName()); + private final static Logger log = Logger.getLogger(FileDownloader.class.getName()); - private final String filesDirectory; - private final ConfigSourceSet configSourceSet; + private final File downloadDirectory; private final Duration timeout; - private final Map<FileReference, Double> downloadStatus = new HashMap<>(); - private final Set<FileReference> queuedForDownload = new LinkedHashSet<>(); + private final FileReferenceDownloader fileReferenceDownloader; - public FileDownloader(ConfigSourceSet configSourceSet) { - this(configSourceSet, - Defaults.getDefaults().underVespaHome("var/db/vespa/filedistribution"), + public FileDownloader(ConnectionPool connectionPool) { + this(connectionPool, + new File(Defaults.getDefaults().underVespaHome("var/db/vespa/filedistribution")), Duration.ofMinutes(15)); } - FileDownloader(ConfigSourceSet configSourceSet, String filesDirectory, Duration timeout) { - this.configSourceSet = configSourceSet; - this.filesDirectory = filesDirectory; + FileDownloader(ConnectionPool connectionPool, File downloadDirectory, Duration timeout) { + this.downloadDirectory = downloadDirectory; this.timeout = timeout; + this.fileReferenceDownloader = new FileReferenceDownloader(downloadDirectory, connectionPool, timeout); } public Optional<File> getFile(FileReference fileReference) { Objects.requireNonNull(fileReference, "file reference cannot be null"); - File directory = new File(filesDirectory, fileReference.value()); // directory with one file - + File directory = new File(downloadDirectory, fileReference.value()); log.log(LogLevel.DEBUG, "Checking if there is a file in '" + directory.getAbsolutePath() + "' "); - Instant end = Instant.now().plus(timeout); - do { - File[] files = directory.listFiles(); - if (directory.exists() && directory.isDirectory() && files != null && files.length > 0) { - if (files.length != 1) { - throw new RuntimeException("More than one file in '" + fileReference.value() + - "', expected only one, unable to proceed"); - } - File file = files[0]; - if (!file.exists()) { - throw new RuntimeException("File with reference '" + fileReference.value() + - "' does not exist"); - } else if (!file.canRead()) { - throw new RuntimeException("File with reference '" + fileReference.value() + - "'exists, but unable to read it"); - } else { - downloadStatus.put(fileReference, 100.0); - return Optional.of(file); - } + + Optional<File> file = getFileFromFileSystem(fileReference, directory); + if (file.isPresent()) { + return file; + } else { + log.log(LogLevel.INFO, "File reference '" + fileReference.value() + "' not found in " + + directory.getAbsolutePath() + ", starting download"); + return queueForDownload(fileReference, timeout); + } + } + + public void queueForDownload(List<FileReference> fileReferences) { + fileReferences.forEach(this::queueForDownload); + } + + public void receiveFile(FileReference fileReference, String filename, byte[] content) { + fileReferenceDownloader.receiveFile(fileReference, filename, content); + } + + double downloadStatus(FileReference fileReference) { + return fileReferenceDownloader.downloadStatus(fileReference.value()); + } + + public Map<FileReference, Double> downloadStatus() { + return fileReferenceDownloader.downloadStatus(); + } + + File downloadDirectory() { + return downloadDirectory; + } + + private Optional<File> getFileFromFileSystem(FileReference fileReference, File directory) { + File[] files = directory.listFiles(); + if (directory.exists() && directory.isDirectory() && files != null && files.length > 0) { + if (files.length != 1) { + throw new RuntimeException("More than one file in '" + fileReference.value() + + "', expected only one, unable to proceed"); + } + File file = files[0]; + if (!file.exists()) { + throw new RuntimeException("File with reference '" + fileReference.value() + + "' does not exist"); + } else if (!file.canRead()) { + throw new RuntimeException("File with reference '" + fileReference.value() + + "'exists, but unable to read it"); } else { - queueForDownload(fileReference); + fileReferenceDownloader.setDownloadStatus(fileReference.value(), 100.0); + return Optional.of(file); } + } + return Optional.empty(); + } + + private synchronized Optional<File> queueForDownload(FileReference fileReference, Duration timeout) { + if (fileReferenceDownloader.isDownloading(fileReference)) { + log.log(LogLevel.INFO, "Already downloading '" + fileReference.value() + "'"); + ListenableFuture<Optional<File>> future = + fileReferenceDownloader.addDownloadListener(fileReference, () -> getFile(fileReference)); try { - Thread.sleep(100); - } catch (InterruptedException e) { - e.printStackTrace(); + return future.get(); + } catch (InterruptedException | ExecutionException e) { + throw new RuntimeException("Failed downloading file reference '" + fileReference.value() + "': " + + Exceptions.toMessageString(e)); } - } while (Instant.now().isBefore(end)); + } - return Optional.empty(); - } + SettableFuture<Optional<File>> future = SettableFuture.create(); + queueForDownload(new FileReferenceDownload(fileReference, future)); + log.log(LogLevel.INFO, "Queued '" + fileReference.value() + "' for download with timeout " + timeout); - public Map<FileReference, Double> downloadStatus() { - return downloadStatus; + try { + Optional<File> fileDownloaded; + try { + log.log(LogLevel.INFO, "Waiting for '" + fileReference.value() + "' to download"); + fileDownloaded = future.get(timeout.getSeconds() - 1, TimeUnit.SECONDS); + log.log(LogLevel.INFO, "'" + fileReference.value() + "' downloaded"); + } catch (TimeoutException e) { + log.log(LogLevel.WARNING, "Downloading '" + fileReference.value() + "' timed out"); + return Optional.empty(); + } + return fileDownloaded; + } catch (InterruptedException | ExecutionException e) { + throw new RuntimeException("Could not download '" + fileReference.value() + "'"); + } } - public void queueForDownload(List<FileReference> fileReferences) { - fileReferences.forEach(this::queueForDownload); + // We don't care about the future in this call + private synchronized void queueForDownload(FileReference fileReference) { + queueForDownload(new FileReferenceDownload(fileReference, SettableFuture.create())); } - private void queueForDownload(FileReference fileReference) { - log.log(LogLevel.INFO, "Queued '" + fileReference.value() + "' for download "); - queuedForDownload.add(fileReference); - downloadStatus.put(fileReference, 0.0); + private synchronized void queueForDownload(FileReferenceDownload fileReferenceDownload) { + fileReferenceDownloader.addToDownloadQueue(fileReferenceDownload); } - ImmutableSet<FileReference> queuedForDownload() { - return ImmutableSet.copyOf(queuedForDownload); + Set<FileReference> queuedDownloads() { + return fileReferenceDownloader.queuedDownloads(); } -}
\ No newline at end of file +} diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileReferenceDownload.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileReferenceDownload.java new file mode 100644 index 00000000000..ce5a30dc7ad --- /dev/null +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileReferenceDownload.java @@ -0,0 +1,28 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +package com.yahoo.vespa.config.proxy.filedistribution; + +import com.google.common.util.concurrent.SettableFuture; +import com.yahoo.config.FileReference; + +import java.io.File; +import java.util.Optional; + +public class FileReferenceDownload { + private final FileReference fileReference; + private final SettableFuture<Optional<File>> future; + + FileReferenceDownload(FileReference fileReference, SettableFuture<Optional<File>> future) { + this.fileReference = fileReference; + this.future = future; + } + + FileReference fileReference() { + return fileReference; + } + + SettableFuture<Optional<File>> future() { + return future; + } + +} diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileReferenceDownloader.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileReferenceDownloader.java new file mode 100644 index 00000000000..917374740f1 --- /dev/null +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileReferenceDownloader.java @@ -0,0 +1,183 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.config.proxy.filedistribution; + +import com.google.common.collect.ImmutableMap; +import com.google.common.util.concurrent.ListenableFuture; +import com.yahoo.concurrent.DaemonThreadFactory; +import com.yahoo.config.FileReference; +import com.yahoo.jrt.Request; +import com.yahoo.jrt.StringValue; +import com.yahoo.log.LogLevel; +import com.yahoo.vespa.config.Connection; +import com.yahoo.vespa.config.ConnectionPool; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.time.Duration; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.logging.Logger; +import java.util.stream.Collectors; + +/** + * Downloads file reference using rpc requests to config server and keeps track of files being downloaded + * <p> + * Some methods are synchronized to make sure access to downloads is atomic + * + * @author hmusum + */ +// TODO: Add retries when a config server does not have a file reference +// TODO: Handle shutdown of executors +class FileReferenceDownloader { + + private final static Logger log = Logger.getLogger(FileReferenceDownloader.class.getName()); + private final static Duration rpcTimeout = Duration.ofSeconds(10); + + private final File downloadDirectory; + private final ExecutorService downloadExecutor = + Executors.newFixedThreadPool(10, new DaemonThreadFactory("filereference downloader")); + private ExecutorService readFromQueueExecutor = + Executors.newFixedThreadPool(1, new DaemonThreadFactory("filereference download queue")); + private final ConnectionPool connectionPool; + private final ConcurrentLinkedQueue<FileReferenceDownload> downloadQueue = new ConcurrentLinkedQueue<>(); + private final Map<FileReference, FileReferenceDownload> downloads = new LinkedHashMap<>(); + private final Map<FileReference, Double> downloadStatus = new HashMap<>(); + private final Duration downloadTimeout; + + FileReferenceDownloader(File downloadDirectory, ConnectionPool connectionPool, Duration timeout) { + this.downloadDirectory = downloadDirectory; + this.connectionPool = connectionPool; + this.downloadTimeout = timeout; + if (connectionPool != null) + readFromQueueExecutor.submit(this::readFromQueue); + } + + private synchronized Optional<File> startDownload(FileReference fileReference, + Duration timeout, + FileReferenceDownload fileReferenceDownload) + throws ExecutionException, InterruptedException, TimeoutException { + downloads.put(fileReference, fileReferenceDownload); + setDownloadStatus(fileReference.value(), 0.0); + if (startDownloadRpc(fileReference)) + return fileReferenceDownload.future().get(timeout.toMillis(), TimeUnit.MILLISECONDS); + else { + fileReferenceDownload.future().setException(new RuntimeException("Failed getting file")); + downloads.remove(fileReference); + return Optional.empty(); + } + } + + synchronized void addToDownloadQueue(FileReferenceDownload fileReferenceDownload) { + downloadQueue.add(fileReferenceDownload); + } + + void receiveFile(FileReference fileReference, String filename, byte[] content) { + File fileReferenceDir = new File(downloadDirectory, fileReference.value()); + try { + Files.createDirectories(fileReferenceDir.toPath()); + File file = new File(fileReferenceDir, filename); + log.log(LogLevel.INFO, "Writing data to " + file.getAbsolutePath()); + Files.write(file.toPath(), content); + completedDownloading(fileReference, file); + } catch (IOException e) { + log.log(LogLevel.ERROR, "Failed writing file: " + e.getMessage()); + throw new RuntimeException("Failed writing file: ", e); + } + } + + synchronized Set<FileReference> queuedDownloads() { + return downloadQueue.stream() + .map(FileReferenceDownload::fileReference) + .collect(Collectors.toCollection(LinkedHashSet::new)); + } + + private void readFromQueue() { + do { + FileReferenceDownload fileReferenceDownload = downloadQueue.poll(); + if (fileReferenceDownload == null) { + try { + Thread.sleep(10); + } catch (InterruptedException e) { /* ignore for now */} + } else { + log.log(LogLevel.INFO, "Polling queue, found file reference '" + + fileReferenceDownload.fileReference().value() + "' to download"); + downloadExecutor.submit(() -> startDownload(fileReferenceDownload.fileReference(), downloadTimeout, fileReferenceDownload)); + } + } while (true); + } + + private synchronized void completedDownloading(FileReference fileReference, File file) { + downloads.get(fileReference).future().set(Optional.of(file)); + downloadStatus.put(fileReference, 100.0); + } + + private boolean startDownloadRpc(FileReference fileReference) throws ExecutionException, InterruptedException { + Connection connection = connectionPool.getCurrent(); + Request request = new Request("filedistribution.serveFile"); + request.parameters().add(new StringValue(fileReference.value())); + + execute(request, connection); + if (validateResponse(request)) { + log.log(LogLevel.DEBUG, "Request callback, OK. Req: " + request + "\nSpec: " + connection); + if (request.returnValues().get(0).asInt32() == 0) + log.log(LogLevel.INFO, "Found file reference '" + fileReference.value() + "' available at " + connection.getAddress()); + else + log.log(LogLevel.INFO, "File reference '" + fileReference.value() + "' not found for " + connection.getAddress()); + return true; + } else { + log.log(LogLevel.WARNING, "Request failed. Req: " + request + "\nSpec: " + connection.getAddress()); + connection.setError(request.errorCode()); + // TODO: Retry with another config server + return false; + } + } + + synchronized boolean isDownloading(FileReference fileReference) { + return downloads.containsKey(fileReference); + } + + synchronized ListenableFuture<Optional<File>> addDownloadListener(FileReference fileReference, Runnable runnable) { + FileReferenceDownload fileReferenceDownload = downloads.get(fileReference); + fileReferenceDownload.future().addListener(runnable, downloadExecutor); + return fileReferenceDownload.future(); + } + + private void execute(Request request, Connection connection) { + connection.invokeSync(request, (double) rpcTimeout.getSeconds()); + } + + private boolean validateResponse(Request request) { + if (request.isError()) { + return false; + } else if (request.returnValues().size() == 0) { + return false; + } else if (!request.checkReturnTypes("i")) { + log.log(LogLevel.WARNING, "Invalid return types for response: " + request.errorMessage()); + return false; + } + return true; + } + + double downloadStatus(String file) { + return downloadStatus.getOrDefault(new FileReference(file), 0.0); + } + + void setDownloadStatus(String file, double percentageDownloaded) { + downloadStatus.put(new FileReference(file), percentageDownloaded); + } + + Map<FileReference, Double> downloadStatus() { + return ImmutableMap.copyOf(downloadStatus); + } +} diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServerTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServerTest.java index c1e9826e29f..f9b334a6f87 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServerTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServerTest.java @@ -38,8 +38,7 @@ public class ConfigProxyRpcServerTest { @Test public void basic() { - ConfigSourceSet configSources = new ConfigSourceSet(); - ProxyServer proxy = ProxyServer.createTestServer(configSources); + ProxyServer proxy = ProxyServer.createTestServer(new MockConfigSource(new MockClientUpdater())); Spec spec = new Spec("localhost", 12345); ConfigProxyRpcServer server = new ConfigProxyRpcServer(proxy, spec); assertThat(server.getSpec(), is(spec)); diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MockConfigSource.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MockConfigSource.java index dc9a3408510..2b26996fbdc 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MockConfigSource.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MockConfigSource.java @@ -1,12 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.proxy; -import com.yahoo.config.subscription.ConfigSource; import com.yahoo.config.subscription.ConfigSourceSet; import com.yahoo.vespa.config.ConfigKey; import com.yahoo.vespa.config.RawConfig; +import java.util.Collection; +import java.util.Collections; import java.util.HashMap; +import java.util.Set; /** * A simple class to be able to test config proxy without having an RPC config @@ -37,4 +39,9 @@ class MockConfigSource extends ConfigSourceSet { backing.clear(); } + @Override + public Set<String> getSources() { + return Collections.singleton("tcp/localhost:19070,tcp/localhost:19071,tcp/localhost:19072"); + } + } diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java index f82d9e90184..3cd0f1043cc 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java @@ -95,7 +95,6 @@ public class ProxyServerTest { */ @Test public void testModeSwitch() { - ConfigSourceSet source = new ConfigSourceSet(); // Need to use a ConfigSourceSet to test modes ProxyServer proxy = ProxyServer.createTestServer(source); assertTrue(proxy.getMode().isDefault()); diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/filedistribution/FileDownloaderTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/filedistribution/FileDownloaderTest.java index ea880e451b6..18d49e9a224 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/filedistribution/FileDownloaderTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/filedistribution/FileDownloaderTest.java @@ -1,79 +1,229 @@ package com.yahoo.vespa.config.proxy.filedistribution; import com.yahoo.config.FileReference; -import com.yahoo.config.subscription.ConfigSourceSet; import com.yahoo.io.IOUtils; +import com.yahoo.jrt.Int32Value; +import com.yahoo.jrt.Request; +import com.yahoo.jrt.RequestWaiter; +import com.yahoo.text.Utf8; +import com.yahoo.vespa.config.Connection; +import com.yahoo.vespa.config.ConnectionPool; +import org.junit.Before; import org.junit.Test; import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.time.Duration; -import java.util.*; +import java.util.Arrays; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class FileDownloaderTest { - private static final ConfigSourceSet configSourceSet = new ConfigSourceSet(); + + private MockConnection connection; + private FileDownloader fileDownloader; + + @Before + public void setup() { + try { + File downloadDir = Files.createTempDirectory("filedistribution").toFile(); + connection = new MockConnection(); + fileDownloader = new FileDownloader(connection, downloadDir, Duration.ofMillis(3000)); + } catch (IOException e) { + e.printStackTrace(); + fail(e.getMessage()); + } + } @Test - public void download() throws IOException { - File downloadDir = Files.createTempDirectory("filedistribution").toFile(); - FileDownloader fileDownloader = new FileDownloader(configSourceSet, downloadDir.getAbsolutePath(), Duration.ofMillis(200)); + public void getFile() throws IOException { + File downloadDir = fileDownloader.downloadDirectory(); - // Write a file to download directory to simulate download going OK - String fileReferenceString = "somehash"; - String fileName = "foo.jar"; - File fileReferenceFullPath = fileReferenceFullPath(downloadDir, fileReferenceString); - FileReference fileReference = writeFileReference(downloadDir, fileReferenceString, fileName); + { + // fileReference already exists on disk, does not have to be downloaded - // Check that we get correct path and content when asking for file reference - Optional<File> pathToFile = fileDownloader.getFile(fileReference); - assertTrue(pathToFile.isPresent()); - String downloadedFile = new File(fileReferenceFullPath, fileName).getAbsolutePath(); - assertEquals(new File(fileReferenceFullPath, fileName).getAbsolutePath(), downloadedFile); - assertEquals("content", IOUtils.readFile(pathToFile.get())); + String fileReferenceString = "foo"; + String filename = "foo.jar"; + File fileReferenceFullPath = fileReferenceFullPath(downloadDir, fileReferenceString); + FileReference fileReference = new FileReference(fileReferenceString); + writeFileReference(downloadDir, fileReferenceString, filename); - // Verify download status - Map<FileReference, Double> downloadStatus = fileDownloader.downloadStatus(); - assertEquals(1, downloadStatus.size()); - assertDownloadStatus(Collections.singletonList(fileReference), downloadStatus.entrySet().iterator().next(), 100.0); + // Check that we get correct path and content when asking for file reference + Optional<File> pathToFile = fileDownloader.getFile(fileReference); + assertTrue(pathToFile.isPresent()); + String downloadedFile = new File(fileReferenceFullPath, filename).getAbsolutePath(); + assertEquals(new File(fileReferenceFullPath, filename).getAbsolutePath(), downloadedFile); + assertEquals("content", IOUtils.readFile(pathToFile.get())); + + // Verify download status when downloaded + assertDownloadStatus(fileDownloader, fileReference, 100.0); + } + + { + // fileReference does not exist on disk, needs to be downloaded, but fails when asking upstream for file) + + connection.setResponseHandler(new MockConnection.UnknownFileReferenceResponseHandler()); + + FileReference fileReference = new FileReference("bar"); + File fileReferenceFullPath = fileReferenceFullPath(downloadDir, fileReference.value()); + assertFalse(fileReferenceFullPath.getAbsolutePath(), fileDownloader.getFile(fileReference).isPresent()); + + // Verify download status when unable to download + assertDownloadStatus(fileDownloader, fileReference, 0.0); + } + + { + // fileReference does not exist on disk, needs to be downloaded) - // Non-existing file - assertFalse(fileReferenceFullPath.getAbsolutePath(), fileDownloader.getFile(new FileReference("doesnotexist")).isPresent()); + FileReference fileReference = new FileReference("fileReference"); + File fileReferenceFullPath = fileReferenceFullPath(downloadDir, fileReference.value()); + assertFalse(fileReferenceFullPath.getAbsolutePath(), fileDownloader.getFile(fileReference).isPresent()); + + // Verify download status + assertDownloadStatus(fileDownloader, fileReference, 0.0); + + // Receives fileReference, should return and make it available to caller + String filename = "abc.jar"; + fileDownloader.receiveFile(fileReference, filename, Utf8.toBytes("some other content")); + Optional<File> downloadedFile = fileDownloader.getFile(fileReference); + + assertTrue(downloadedFile.isPresent()); + File downloadedFileFullPath = new File(fileReferenceFullPath, filename); + assertEquals(downloadedFileFullPath.getAbsolutePath(), downloadedFile.get().getAbsolutePath()); + assertEquals("some other content", IOUtils.readFile(downloadedFile.get())); + + // Verify download status when downloaded + assertDownloadStatus(fileDownloader, fileReference, 100.0); + } } @Test public void setFilesToDownload() throws IOException { File downloadDir = Files.createTempDirectory("filedistribution").toFile(); - FileDownloader fileDownloader = new FileDownloader(configSourceSet, downloadDir.getAbsolutePath(), Duration.ofMillis(200)); - List<FileReference> fileReferences = Arrays.asList(new FileReference("foo"), new FileReference("bar")); + FileDownloader fileDownloader = new FileDownloader(null, downloadDir, Duration.ofMillis(200)); + FileReference foo = new FileReference("foo"); + FileReference bar = new FileReference("bar"); + List<FileReference> fileReferences = Arrays.asList(foo, bar); fileDownloader.queueForDownload(fileReferences); - assertEquals(fileReferences, fileDownloader.queuedForDownload().asList()); + // All requested file references should be in queue (since FileDownloader was created without ConnectionPool) + assertEquals(new LinkedHashSet<>(fileReferences), new LinkedHashSet<>(fileDownloader.queuedDownloads())); // Verify download status - Map<FileReference, Double> downloadStatus = fileDownloader.downloadStatus(); - assertEquals(2, downloadStatus.size()); - - assertDownloadStatus(fileReferences, downloadStatus.entrySet().iterator().next(), 0.0); - assertDownloadStatus(fileReferences, downloadStatus.entrySet().iterator().next(), 0.0); + assertDownloadStatus(fileDownloader, foo, 0.0); + assertDownloadStatus(fileDownloader, bar, 0.0); } - private FileReference writeFileReference(File dir, String fileReferenceString, String fileName) throws IOException { + private void writeFileReference(File dir, String fileReferenceString, String fileName) throws IOException { File file = new File(new File(dir, fileReferenceString), fileName); IOUtils.writeFile(file, "content", false); - return new FileReference(fileReferenceString); } private File fileReferenceFullPath(File dir, String fileReferenceString) { return new File(dir, fileReferenceString); } - private void assertDownloadStatus(List<FileReference> fileReferences, Map.Entry<FileReference, Double> entry, double expectedDownloadStatus) { - assertTrue(fileReferences.contains(new FileReference(entry.getKey().value()))); - assertEquals(expectedDownloadStatus, entry.getValue(), 0.0001); + private void assertDownloadStatus(FileDownloader fileDownloader, FileReference fileReference, double expectedDownloadStatus) { + double downloadStatus = fileDownloader.downloadStatus(fileReference); + assertEquals(expectedDownloadStatus, downloadStatus, 0.0001); + } + + private static class MockConnection implements ConnectionPool, com.yahoo.vespa.config.Connection { + + private ResponseHandler responseHandler; + + MockConnection() { + this(new FileReferenceFoundResponseHandler()); + } + + MockConnection(ResponseHandler responseHandler) { + this.responseHandler = responseHandler; + } + + @Override + public void invokeAsync(Request request, double jrtTimeout, RequestWaiter requestWaiter) { + responseHandler.request(request); + } + + @Override + public void invokeSync(Request request, double jrtTimeout) { + responseHandler.request(request); + } + + @Override + public void setError(int errorCode) { + } + + @Override + public void setSuccess() { + } + + @Override + public String getAddress() { + return null; + } + + @Override + public void close() { + } + + @Override + public void setError(Connection connection, int errorCode) { + connection.setError(errorCode); + } + + @Override + public Connection getCurrent() { + return this; + } + + @Override + public Connection setNewCurrentConnection() { + return this; + } + + @Override + public int getSize() { + return 1; + } + + public void setResponseHandler(ResponseHandler responseHandler) { + this.responseHandler = responseHandler; + } + + static class FileReferenceFoundResponseHandler implements ResponseHandler { + + @Override + public void request(Request request) { + if (request.methodName().equals("filedistribution.serveFile")) + request.returnValues().add(new Int32Value(0)); + } + } + + static class UnknownFileReferenceResponseHandler implements ResponseHandler { + + @Override + public void request(Request request) { + if (request.methodName().equals("filedistribution.serveFile")) + request.returnValues().add(new Int32Value(1)); + } + } + + public interface ResponseHandler { + + void request(Request request); + + } + } + } diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/MockConnection.java b/config/src/main/java/com/yahoo/config/subscription/impl/MockConnection.java index 3509a960740..bd9a49c2fe2 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/MockConnection.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/MockConnection.java @@ -53,6 +53,12 @@ public class MockConnection implements ConnectionPool, com.yahoo.vespa.config.Co } @Override + public void invokeSync(Request request, double jrtTimeout) { + numberOfRequests++; + lastRequest = request; + } + + @Override public void setError(int errorCode) { numberOfFailovers++; } @@ -68,9 +74,7 @@ public class MockConnection implements ConnectionPool, com.yahoo.vespa.config.Co } @Override - public void close() { - - } + public void close() {} @Override public void setError(Connection connection, int errorCode) { @@ -109,7 +113,6 @@ public class MockConnection implements ConnectionPool, com.yahoo.vespa.config.Co } } - public interface ResponseHandler extends Runnable { RequestWaiter requestWaiter(); diff --git a/config/src/main/java/com/yahoo/vespa/config/Connection.java b/config/src/main/java/com/yahoo/vespa/config/Connection.java index 3d487198450..e39175a3a78 100644 --- a/config/src/main/java/com/yahoo/vespa/config/Connection.java +++ b/config/src/main/java/com/yahoo/vespa/config/Connection.java @@ -11,6 +11,8 @@ public interface Connection { void invokeAsync(Request request, double jrtTimeout, RequestWaiter requestWaiter); + void invokeSync(Request request, double jrtTimeout); + void setError(int errorCode); void setSuccess(); diff --git a/config/src/main/java/com/yahoo/vespa/config/JRTConnection.java b/config/src/main/java/com/yahoo/vespa/config/JRTConnection.java index 96dd6f62244..01da823b87b 100644 --- a/config/src/main/java/com/yahoo/vespa/config/JRTConnection.java +++ b/config/src/main/java/com/yahoo/vespa/config/JRTConnection.java @@ -13,6 +13,7 @@ import java.util.logging.Logger; * @author <a href="mailto:gunnarga@yahoo-inc.com">Gunnar Gauslaa Bergem</a> */ public class JRTConnection implements Connection { + public final static Logger logger = Logger.getLogger(JRTConnection.class.getPackage().getName()); private final String address; private final Supervisor supervisor; @@ -30,17 +31,20 @@ public class JRTConnection implements Connection { yyyyMMddz.setTimeZone(TimeZone.getTimeZone("GMT")); } + + public JRTConnection(String address, Supervisor supervisor) { + this.address = address; + this.supervisor = supervisor; + } + @Override public void invokeAsync(Request request, double jrtTimeout, RequestWaiter requestWaiter) { getTarget().invokeAsync(request, jrtTimeout, requestWaiter); } - public final static Logger logger = Logger.getLogger(JRTConnection.class.getPackage().getName()); - - - public JRTConnection(String address, Supervisor supervisor) { - this.address = address; - this.supervisor = supervisor; + @Override + public void invokeSync(Request request, double jrtTimeout) { + getTarget().invokeSync(request, jrtTimeout); } public String getAddress() { diff --git a/config/src/main/java/com/yahoo/vespa/config/JRTConnectionPool.java b/config/src/main/java/com/yahoo/vespa/config/JRTConnectionPool.java index b27f83851b4..bb8f7e9f9ce 100644 --- a/config/src/main/java/com/yahoo/vespa/config/JRTConnectionPool.java +++ b/config/src/main/java/com/yahoo/vespa/config/JRTConnectionPool.java @@ -21,7 +21,7 @@ import java.util.logging.Logger; * The current connection is available with {@link #getCurrent()}. * When calling {@link #setError(Connection, int)}, {#link #setNewCurrentConnection} will always be called. * - * @author <a href="mailto:gunnarga@yahoo-inc.com">Gunnar Gauslaa Bergem</a> + * @author Gunnar Gauslaa Bergem * @author hmusum */ public class JRTConnectionPool implements ConnectionPool { diff --git a/configserver/pom.xml b/configserver/pom.xml index 8776fbd5ad1..4ebb76bd5fe 100644 --- a/configserver/pom.xml +++ b/configserver/pom.xml @@ -172,6 +172,11 @@ <artifactId>jersey-proxy-client</artifactId> </dependency> <dependency> + <groupId>net.jpountz.lz4</groupId> + <artifactId>lz4</artifactId> + <scope>compile</scope> + </dependency> + <dependency> <groupId>org.mockito</groupId> <artifactId>mockito-core</artifactId> <scope>test</scope> diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java index 70b677b4057..1b96ba46907 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java @@ -11,6 +11,7 @@ import com.yahoo.config.provision.Rotation; import com.yahoo.config.provision.Zone; import java.io.File; +import java.net.URI; import java.util.List; import java.util.Optional; import java.util.Set; @@ -132,6 +133,7 @@ public class ModelContextImpl implements ModelContext { private final ApplicationId applicationId; private final boolean multitenant; private final List<ConfigServerSpec> configServerSpecs; + private final URI loadBalancerAddress; private final boolean hostedVespa; private final Zone zone; private final Set<Rotation> rotations; @@ -139,12 +141,14 @@ public class ModelContextImpl implements ModelContext { public Properties(ApplicationId applicationId, boolean multitenant, List<ConfigServerSpec> configServerSpecs, + URI loadBalancerAddress, boolean hostedVespa, Zone zone, Set<Rotation> rotations) { this.applicationId = applicationId; this.multitenant = multitenant; this.configServerSpecs = configServerSpecs; + this.loadBalancerAddress = loadBalancerAddress; this.hostedVespa = hostedVespa; this.zone = zone; this.rotations = rotations; @@ -166,6 +170,11 @@ public class ModelContextImpl implements ModelContext { } @Override + public URI loadBalancerAddress() { + return loadBalancerAddress; + } + + @Override public boolean hostedVespa() { return hostedVespa; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionProvider.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionProvider.java index 3693bfb361c..36b0138ad36 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionProvider.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionProvider.java @@ -22,11 +22,8 @@ public class FileDistributionProvider { public FileDistributionProvider(File applicationDir, String zooKeepersSpec, String applicationId, Lock fileDistributionLock) { ensureDirExists(FileDistribution.getDefaultFileDBPath()); final FileDistributionManager manager = new FileDistributionManager( - FileDistribution.getDefaultFileDBPath(), - applicationDir, - zooKeepersSpec, - applicationId, - fileDistributionLock); + FileDistribution.getDefaultFileDBPath(), applicationDir, + zooKeepersSpec, applicationId, fileDistributionLock); this.fileDistribution = new FileDBHandler(manager); this.fileRegistry = new FileDBRegistry(manager); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java new file mode 100644 index 00000000000..1c77ee66d0c --- /dev/null +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java @@ -0,0 +1,115 @@ +package com.yahoo.vespa.config.server.filedistribution; + +import com.google.inject.Inject; +import com.yahoo.config.FileReference; +import com.yahoo.config.model.api.FileDistribution; +import com.yahoo.io.IOUtils; + +import java.io.File; +import java.io.FilenameFilter; +import java.io.IOException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.logging.Logger; + +public class FileServer { + private static final Logger log = Logger.getLogger(FileServer.class.getName()); + private final String rootDir; + private final ExecutorService executor; + + public static class ReplayStatus { + private final int code; + private final String description; + public ReplayStatus(int code, String description) { + this.code = code; + this.description = description; + } + public boolean ok() { return code == 0; } + public int getCode() { return code; } + public String getDescription() { return description; } + } + + public interface Receiver { + void receive(FileReference reference, String filename, byte [] content, ReplayStatus status); + } + + private String getPath(FileReference ref) { + return rootDir + "/" + ref.value(); + } + + static private class Filter implements FilenameFilter { + @Override + public boolean accept(File dir, String name) { + return !".".equals(name) && !"..".equals(name) ; + } + } + private File getFile(FileReference reference) { + File dir = new File(getPath(reference)); + if (!dir.exists()) { + throw new IllegalArgumentException("File reference '" + reference.toString() + "' with absolute path '" + dir.getAbsolutePath() + "' does not exist."); + } + if (!dir.isDirectory()) { + throw new IllegalArgumentException("File reference '" + reference.toString() + "' with absolute path '" + dir.getAbsolutePath() + "' is not a directory."); + } + File [] files = dir.listFiles(new Filter()); + if (files.length != 1) { + StringBuilder msg = new StringBuilder(); + for (File f: files) { + msg.append(f.getName()).append("\n"); + } + throw new IllegalArgumentException("File reference '" + reference.toString() + "' with absolute path '" + dir.getAbsolutePath() + " does not contain exactly one file, but [" + msg.toString() + "]"); + } + return files[0]; + } + + @Inject + public FileServer() { + this(FileDistribution.getDefaultFileDBRoot()); + } + + public FileServer(String rootDir) { + this(rootDir, Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors())); + } + + public FileServer(String rootDir, ExecutorService executor) { + this.rootDir = rootDir; + this.executor = executor; + } + public boolean hasFile(String fileName) { + return hasFile(new FileReference(fileName)); + } + public boolean hasFile(FileReference reference) { + try { + return getFile(reference).exists(); + } catch (IllegalArgumentException e) { + log.warning("Failed locating file reference '" + reference + "' with error " + e.toString()); + } + return false; + } + public boolean startFileServing(String fileName, Receiver target) { + FileReference reference = new FileReference(fileName); + File file = getFile(reference); + + if (file.exists()) { + executor.execute(() -> serveFile(reference, target)); + } + return false; + } + + private void serveFile(FileReference reference, Receiver target) { + + File file = getFile(reference); + byte [] blob = new byte [0]; + boolean success = false; + String errorDescription = "OK"; + try { + blob = IOUtils.readFileBytes(file); + success = true; + } catch (IOException e) { + errorDescription = "For file reference '" + reference.value() + "' I failed reading file '" + file.getAbsolutePath() + "'"; + log.warning(errorDescription + "for sending to '" + target.toString() + "'. " + e.toString()); + } + target.receive(reference, file.getName(), blob, + new ReplayStatus(success ? 0 : 1, success ? "OK" : errorDescription)); + } +} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java index af4d998c347..fac73dcac77 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java @@ -19,6 +19,7 @@ import com.yahoo.vespa.config.server.deploy.ModelContextImpl; import com.yahoo.vespa.config.server.http.UnknownVespaVersionException; import com.yahoo.vespa.config.server.provision.StaticProvisioner; +import java.net.URI; import java.time.Instant; import java.util.ArrayList; import java.util.Collections; @@ -170,9 +171,10 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> { ConfigserverConfig configserverConfig, Zone zone, Set<Rotation> rotations) { - return new ModelContextImpl.Properties(applicationId, + return new ModelContextImpl.Properties(applicationId, configserverConfig.multitenant(), ConfigServerSpec.fromConfig(configserverConfig), + URI.create(configserverConfig.loadBalancerAddress()), configserverConfig.hostedVespa(), zone, rotations); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java index 3c9917bf17e..662da63d198 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java @@ -4,19 +4,24 @@ package com.yahoo.vespa.config.server.rpc; import com.google.inject.Inject; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.concurrent.ThreadFactoryFactory; +import com.yahoo.config.FileReference; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.HostLivenessTracker; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.Version; import com.yahoo.jrt.Acceptor; +import com.yahoo.jrt.DataValue; import com.yahoo.jrt.Int32Value; +import com.yahoo.jrt.Int64Value; import com.yahoo.jrt.ListenFailedException; import com.yahoo.jrt.Method; import com.yahoo.jrt.Request; import com.yahoo.jrt.Spec; import com.yahoo.jrt.StringValue; import com.yahoo.jrt.Supervisor; +import com.yahoo.jrt.Target; import com.yahoo.jrt.Transport; +import com.yahoo.jrt.Value; import com.yahoo.log.LogLevel; import com.yahoo.vespa.config.ErrorCode; import com.yahoo.vespa.config.JRTMethods; @@ -27,6 +32,7 @@ import com.yahoo.vespa.config.protocol.Trace; import com.yahoo.vespa.config.server.SuperModelRequestHandler; import com.yahoo.vespa.config.server.application.ApplicationSet; import com.yahoo.vespa.config.server.GetConfigContext; +import com.yahoo.vespa.config.server.filedistribution.FileServer; import com.yahoo.vespa.config.server.host.HostRegistries; import com.yahoo.vespa.config.server.host.HostRegistry; import com.yahoo.vespa.config.server.ReloadListener; @@ -36,7 +42,10 @@ import com.yahoo.vespa.config.server.monitoring.MetricUpdaterFactory; import com.yahoo.vespa.config.server.tenant.TenantHandlerProvider; import com.yahoo.vespa.config.server.tenant.TenantListener; import com.yahoo.vespa.config.server.tenant.Tenants; +import net.jpountz.xxhash.XXHash64; +import net.jpountz.xxhash.XXHashFactory; +import java.nio.ByteBuffer; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -68,6 +77,18 @@ public class RpcServer implements Runnable, ReloadListener, TenantListener { static final int TRACELEVEL_DEBUG = 9; private static final String THREADPOOL_NAME = "rpcserver worker pool"; private static final long SHUTDOWN_TIMEOUT = 60; + private enum FileApiErrorCodes { + OK(0, "OK"), + NOT_FOUND(1, "Filereference not found"); + private final int code; + private final String description; + FileApiErrorCodes(int code, String description) { + this.code = code; + this.description = description; + } + int getCode() { return code; } + String getDescription() { return description; } + } private final Supervisor supervisor = new Supervisor(new Transport()); private Spec spec = null; private final boolean useRequestVersion; @@ -83,6 +104,7 @@ public class RpcServer implements Runnable, ReloadListener, TenantListener { private final MetricUpdater metrics; private final MetricUpdaterFactory metricUpdaterFactory; private final HostLivenessTracker hostLivenessTracker; + private final FileServer fileServer; private final ThreadPoolExecutor executorService; private volatile boolean allTenantsLoaded = false; @@ -93,20 +115,23 @@ public class RpcServer implements Runnable, ReloadListener, TenantListener { * @param config The config to use for setting up this server */ @Inject - public RpcServer(ConfigserverConfig config, SuperModelRequestHandler superModelRequestHandler, MetricUpdaterFactory metrics, - HostRegistries hostRegistries, HostLivenessTracker hostLivenessTracker) { + public RpcServer(ConfigserverConfig config, SuperModelRequestHandler superModelRequestHandler, + MetricUpdaterFactory metrics, HostRegistries hostRegistries, + HostLivenessTracker hostLivenessTracker, FileServer fileServer) { this.superModelRequestHandler = superModelRequestHandler; - this.metricUpdaterFactory = metrics; - this.supervisor.setMaxOutputBufferSize(config.maxoutputbuffersize()); + metricUpdaterFactory = metrics; + supervisor.setMaxOutputBufferSize(config.maxoutputbuffersize()); this.metrics = metrics.getOrCreateMetricUpdater(Collections.<String, String>emptyMap()); this.hostLivenessTracker = hostLivenessTracker; BlockingQueue<Runnable> workQueue = new LinkedBlockingQueue<>(config.maxgetconfigclients()); - executorService = new ThreadPoolExecutor(config.numthreads(), config.numthreads(), 0, TimeUnit.SECONDS, workQueue, ThreadFactoryFactory.getThreadFactory(THREADPOOL_NAME)); + executorService = new ThreadPoolExecutor(config.numthreads(), config.numthreads(), + 0, TimeUnit.SECONDS, workQueue, ThreadFactoryFactory.getThreadFactory(THREADPOOL_NAME)); delayedConfigResponses = new DelayedConfigResponses(this, config.numDelayedResponseThreads()); spec = new Spec(null, config.rpcport()); hostRegistry = hostRegistries.getTenantHostRegistry(); this.useRequestVersion = config.useVespaVersionInRequest(); this.hostedVespa = config.hostedVespa(); + this.fileServer = fileServer; setUpHandlers(); } @@ -180,6 +205,7 @@ public class RpcServer implements Runnable, ReloadListener, TenantListener { getSupervisor().addMethod(new Method("printStatistics", "", "s", this, "printStatistics") .methodDesc("printStatistics") .returnDesc(0, "statistics", "Statistics for server")); + getSupervisor().addMethod(new Method("filedistribution.serveFile", "s", "is", this, "serveFile")); } /** @@ -402,4 +428,48 @@ public class RpcServer implements Runnable, ReloadListener, TenantListener { return useRequestVersion; } + class FileReceiver implements FileServer.Receiver { + Target target; + FileReceiver(Target target) { + this.target = target; + } + + @Override + public String toString() { + return target.toString(); + } + + @Override + public void receive(FileReference reference, String filename, byte [] content, FileServer.ReplayStatus status) { + XXHash64 hasher = XXHashFactory.fastestInstance().hash64(); + Request fileBlob = new Request("filedistribution.receiveFile"); + fileBlob.parameters().add(new StringValue(reference.value())); + fileBlob.parameters().add(new StringValue(filename)); + fileBlob.parameters().add(new DataValue(content)); + fileBlob.parameters().add(new Int64Value(hasher.hash(ByteBuffer.wrap(content), 0))); + fileBlob.parameters().add(new Int32Value(status.getCode())); + fileBlob.parameters().add(new StringValue(status.getDescription())); + target.invokeSync(fileBlob, 600); + } + } + + @SuppressWarnings("UnusedDeclaration") + public final void serveFile(Request request) { + String fileReference = request.parameters().get(0).asString(); + FileApiErrorCodes result; + try { + result = fileServer.hasFile(fileReference) + ? FileApiErrorCodes.OK + : FileApiErrorCodes.NOT_FOUND; + if (result == FileApiErrorCodes.OK) { + fileServer.startFileServing(fileReference, new FileReceiver(request.target())); + } + } catch (IllegalArgumentException e) { + result = FileApiErrorCodes.NOT_FOUND; + log.warning("Failed serving file reference '" + fileReference + "' with error " + e.toString()); + } + request.returnValues() + .add(new Int32Value(result.getCode())) + .add(new StringValue(result.getDescription())); + } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java index 31be18d9b22..6154be52bcc 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java @@ -9,26 +9,33 @@ import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.application.api.FileRegistry; import com.yahoo.config.model.api.ConfigDefinitionRepo; import com.yahoo.config.model.api.ModelContext; -import com.yahoo.config.provision.*; +import com.yahoo.config.provision.AllocatedHosts; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.OutOfCapacityException; +import com.yahoo.config.provision.Rotation; +import com.yahoo.config.provision.Version; +import com.yahoo.config.provision.Zone; import com.yahoo.lang.SettableOptional; import com.yahoo.log.LogLevel; import com.yahoo.path.Path; -import com.yahoo.vespa.config.server.application.ApplicationSet; import com.yahoo.vespa.config.server.ConfigServerSpec; +import com.yahoo.vespa.config.server.application.ApplicationSet; import com.yahoo.vespa.config.server.application.PermanentApplicationPackage; -import com.yahoo.vespa.config.server.modelfactory.ModelFactoryRegistry; -import com.yahoo.vespa.config.server.tenant.Rotations; import com.yahoo.vespa.config.server.configchange.ConfigChangeActions; import com.yahoo.vespa.config.server.deploy.ModelContextImpl; import com.yahoo.vespa.config.server.deploy.ZooKeeperDeployer; import com.yahoo.vespa.config.server.http.InvalidApplicationException; +import com.yahoo.vespa.config.server.modelfactory.ModelFactoryRegistry; import com.yahoo.vespa.config.server.modelfactory.PreparedModelsBuilder; import com.yahoo.vespa.config.server.provision.HostProvisionerProvider; - +import com.yahoo.vespa.config.server.tenant.Rotations; import com.yahoo.vespa.curator.Curator; import org.xml.sax.SAXException; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.transform.TransformerException; import java.io.IOException; +import java.net.URI; import java.time.Instant; import java.util.List; import java.util.Map; @@ -37,9 +44,6 @@ import java.util.Set; import java.util.logging.Logger; import java.util.stream.Collectors; -import javax.xml.parsers.ParserConfigurationException; -import javax.xml.transform.TransformerException; - /** * A SessionPreparer is responsible for preparing a session given an application package. * @@ -148,6 +152,7 @@ public class SessionPreparer { this.properties = new ModelContextImpl.Properties(params.getApplicationId(), configserverConfig.multitenant(), ConfigServerSpec.fromConfig(configserverConfig), + URI.create(configserverConfig.loadBalancerAddress()), configserverConfig.hostedVespa(), zone, rotationsSet); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenants.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenants.java index 60200e34cdf..866d1563e3f 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenants.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenants.java @@ -108,6 +108,8 @@ public class Tenants implements ConnectionStateListener, PathChildrenCacheListen metricUpdater = metrics.getOrCreateMetricUpdater(Collections.emptyMap()); this.tenantListeners.add(globalComponentRegistry.getTenantListener()); curator.create(tenantsPath); + createSystemTenants(globalComponentRegistry.getConfigserverConfig()); + createTenants(); this.directoryCache = curator.createDirectoryCache(tenantsPath.getAbsolute(), false, false, pathChildrenExecutor); this.tenants.putAll(addTenants(tenants)); } @@ -147,7 +149,7 @@ public class Tenants implements ConnectionStateListener, PathChildrenCacheListen return tenants; } - synchronized void createTenants() throws Exception { + synchronized void createTenants() { Set<TenantName> allTenants = readTenantsFromZooKeeper(); log.log(LogLevel.DEBUG, "Create tenants, tenants found in zookeeper: " + allTenants); checkForRemovedTenants(allTenants); @@ -167,7 +169,7 @@ public class Tenants implements ConnectionStateListener, PathChildrenCacheListen } } - private void checkForAddedTenants(Set<TenantName> newTenants) throws Exception { + private void checkForAddedTenants(Set<TenantName> newTenants) { ExecutorService executor = Executors.newFixedThreadPool(globalComponentRegistry.getConfigserverConfig().numParallelTenantLoaders()); for (TenantName tenantName : newTenants) { // Note: the http handler will check if the tenant exists, and throw accordingly @@ -178,7 +180,11 @@ public class Tenants implements ConnectionStateListener, PathChildrenCacheListen } } executor.shutdown(); - executor.awaitTermination(365, TimeUnit.DAYS); // Timeout should never happen + try { + executor.awaitTermination(365, TimeUnit.DAYS); // Timeout should never happen + } catch (InterruptedException e) { + throw new RuntimeException("Executor for creating tenants did not terminate within timeout"); + } } private void createTenant(TenantName tenantName) { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/InjectedGlobalComponentRegistryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/InjectedGlobalComponentRegistryTest.java index 3efad7ac133..acda60049ab 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/InjectedGlobalComponentRegistryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/InjectedGlobalComponentRegistryTest.java @@ -5,8 +5,10 @@ import com.google.common.io.Files; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.config.model.NullConfigModelRegistry; import com.yahoo.config.model.api.ConfigDefinitionRepo; +import com.yahoo.config.model.api.FileDistribution; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.config.server.application.PermanentApplicationPackage; +import com.yahoo.vespa.config.server.filedistribution.FileServer; import com.yahoo.vespa.config.server.host.ConfigRequestHostLivenessTracker; import com.yahoo.vespa.config.server.host.HostRegistries; import com.yahoo.vespa.config.server.http.SessionHandlerTest; @@ -61,7 +63,7 @@ public class InjectedGlobalComponentRegistryTest { serverDB = new ConfigServerDB(configserverConfig); sessionPreparer = new SessionTest.MockSessionPreparer(); rpcServer = new RpcServer(configserverConfig, null, Metrics.createTestMetrics(), - new HostRegistries(), new ConfigRequestHostLivenessTracker()); + new HostRegistries(), new ConfigRequestHostLivenessTracker(), new FileServer(FileDistribution.getDefaultFileDBRoot())); generationCounter = new SuperModelGenerationCounter(curator); defRepo = new StaticConfigDefinitionRepo(); permanentApplicationPackage = new PermanentApplicationPackage(configserverConfig); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ModelContextImplTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ModelContextImplTest.java index b53f82b25f3..aed0a6a9750 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ModelContextImplTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ModelContextImplTest.java @@ -45,6 +45,7 @@ public class ModelContextImplTest { ApplicationId.defaultId(), true, Collections.emptyList(), + null, false, Zone.defaultZone(), rotations), diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java index 64932700173..43fd8a92189 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java @@ -103,7 +103,7 @@ public class DeployTester { configserverConfig, clock); try { this.testApp = new File(appPath); - this.tenants = new Tenants(componentRegistry, metrics); + this.tenants = new Tenants(componentRegistry, metrics, Collections.emptySet()); } catch (Exception e) { throw new IllegalArgumentException(e); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java new file mode 100644 index 00000000000..0c2ace38389 --- /dev/null +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java @@ -0,0 +1,80 @@ +package com.yahoo.vespa.config.server.filedistribution; + +import com.yahoo.config.FileReference; +import com.yahoo.io.IOUtils; +import org.junit.Test; + +import java.io.File; +import java.io.IOException; +import java.util.LinkedList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertFalse; + +public class FileServerTest { + + FileServer fs = new FileServer("."); + List<File> created = new LinkedList<>(); + + private void createCleanDir(String name) throws IOException{ + File dir = new File(name); + IOUtils.recursiveDeleteDir(dir); + IOUtils.createDirectory(dir.getName()); + File dummy = new File(dir.getName() +"/dummy"); + IOUtils.writeFile(dummy, "test", true); + assertTrue(dummy.delete()); + created.add(dir); + } + + @Test + public void requireThatExistingFileCanbeFound() throws IOException { + createCleanDir("123"); + IOUtils.writeFile("123/f1", "test", true); + assertTrue(fs.hasFile("123")); + cleanup(); + } + + @Test + public void requireThatNonExistingFileCanNotBeFound() throws IOException { + assertFalse(fs.hasFile("12x")); + createCleanDir("12x"); + assertFalse(fs.hasFile("12x")); + cleanup(); + } + + private static class FileReceiver implements FileServer.Receiver { + CompletableFuture<byte []> content; + FileReceiver(CompletableFuture<byte []> content) { + this.content = content; + } + @Override + public void receive(FileReference reference, String filename, byte[] content, FileServer.ReplayStatus status) { + this.content.complete(content); + } + } + @Test + public void requireThatWeCanReplayFile() throws IOException, InterruptedException, ExecutionException { + createCleanDir("12y"); + IOUtils.writeFile("12y/f1", "dummy-data", true); + CompletableFuture<byte []> content = new CompletableFuture<>(); + fs.startFileServing("12y", new FileReceiver(content)); + assertEquals(new String(content.get()), "dummy-data"); + cleanup(); + } + + private void cleanup() { + created.forEach((file) -> IOUtils.recursiveDeleteDir(file)); + created.clear(); + } + + @Override + protected void finalize() throws Throwable { + super.finalize(); + cleanup(); + } + +} diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpc.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpc.java index a08514e8afb..b094a741f34 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpc.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpc.java @@ -2,11 +2,13 @@ package com.yahoo.vespa.config.server.rpc; import com.yahoo.cloud.config.ConfigserverConfig; +import com.yahoo.config.model.api.FileDistribution; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.Version; import com.yahoo.vespa.config.protocol.ConfigResponse; import com.yahoo.vespa.config.protocol.JRTServerConfigRequest; import com.yahoo.vespa.config.server.GetConfigContext; +import com.yahoo.vespa.config.server.filedistribution.FileServer; import com.yahoo.vespa.config.server.host.ConfigRequestHostLivenessTracker; import com.yahoo.vespa.config.server.host.HostRegistries; import com.yahoo.vespa.config.server.monitoring.Metrics; @@ -37,7 +39,7 @@ public class MockRpc extends RpcServer { public MockRpc(int port, boolean createDefaultTenant, boolean pretendToHaveLoadedAnyApplication) { super(createConfig(port), null, Metrics.createTestMetrics(), - new HostRegistries(), new ConfigRequestHostLivenessTracker()); + new HostRegistries(), new ConfigRequestHostLivenessTracker(), new FileServer(FileDistribution.getDefaultFileDBRoot())); if (createDefaultTenant) { onTenantCreate(TenantName.from("default"), new MockTenantProvider(pretendToHaveLoadedAnyApplication)); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/TestWithRpc.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/TestWithRpc.java index fa6adb64a8a..933cb770dd1 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/TestWithRpc.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/TestWithRpc.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.config.server.rpc; import com.yahoo.cloud.config.ConfigserverConfig; +import com.yahoo.config.model.api.FileDistribution; import com.yahoo.config.provision.HostLivenessTracker; import com.yahoo.config.provision.TenantName; import com.yahoo.jrt.Request; @@ -12,6 +13,7 @@ import com.yahoo.net.HostName; import com.yahoo.test.ManualClock; import com.yahoo.vespa.config.GenerationCounter; import com.yahoo.vespa.config.server.*; +import com.yahoo.vespa.config.server.filedistribution.FileServer; import com.yahoo.vespa.config.server.host.ConfigRequestHostLivenessTracker; import com.yahoo.vespa.config.server.host.HostRegistries; import com.yahoo.vespa.config.server.monitoring.Metrics; @@ -88,7 +90,7 @@ public class TestWithRpc { emptyNodeFlavors(), generationCounter)), Metrics.createTestMetrics(), new HostRegistries(), - hostLivenessTracker); + hostLivenessTracker, new FileServer(FileDistribution.getDefaultFileDBRoot())); rpcServer.onTenantCreate(TenantName.from("default"), tenantProvider); t = new Thread(rpcServer); t.start(); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantsTestCase.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantsTestCase.java index d1724986d5e..5802edfad36 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantsTestCase.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantsTestCase.java @@ -20,6 +20,7 @@ import org.xml.sax.SAXException; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Set; @@ -147,23 +148,24 @@ public class TenantsTestCase extends TestWithCurator { public void testTenantWatching() throws Exception { TestComponentRegistry reg = new TestComponentRegistry.Builder().curator(curator).build(); Tenants t = new Tenants(reg, Metrics.createTestMetrics()); + TenantName newTenant = TenantName.from("newTenant"); + List<TenantName> expectedTenants = Arrays.asList(TenantName.defaultName(), newTenant); try { - assertTrue(t.getAllTenantNames().contains(TenantName.defaultName())); - reg.getCurator().framework().create().forPath(tenants.tenantZkPath(TenantName.from("newTenant"))); + t.addTenant(newTenant); // Poll for the watcher to pick up the tenant from zk, and add it int tries=0; while(true) { - if (tries > 500) fail("Didn't react on watch"); - if (t.getAllTenantNames().contains(TenantName.from("newTenant"))) { - return; + if (tries > 5000) fail("Didn't react on watch"); + if (t.getAllTenantNames().containsAll(expectedTenants)) { + break; } tries++; - Thread.sleep(100); + Thread.sleep(10); } } finally { + assertTrue(t.getAllTenantNames().containsAll(expectedTenants)); t.close(); } } - } diff --git a/container-dev/pom.xml b/container-dev/pom.xml index 8eb8cab1677..d02ec233d96 100644 --- a/container-dev/pom.xml +++ b/container-dev/pom.xml @@ -81,12 +81,9 @@ <groupId>org.apache.felix</groupId> <artifactId>org.apache.felix.main</artifactId> </dependency> - <!-- This version is exported by jdisc via jcl-over-slf4j, so should be the one used by customer bundles --> - <!-- Set explicitly here because org.apache.httpcomponents 4.3 wants to pull in 1.1.3 instead. --> <dependency> <groupId>commons-logging</groupId> <artifactId>commons-logging</artifactId> - <version>1.1.1</version> </dependency> <dependency> <groupId>com.yahoo.vespa</groupId> diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/ContainerThreadFactory.java b/container-disc/src/main/java/com/yahoo/container/jdisc/ContainerThreadFactory.java index 379116a5d94..50798a82b60 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/ContainerThreadFactory.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/ContainerThreadFactory.java @@ -8,7 +8,7 @@ import com.yahoo.jdisc.application.MetricConsumer; import java.util.concurrent.ThreadFactory; /** - * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen Hult</a> + * @author Simon Thoresen Hult */ public class ContainerThreadFactory implements ThreadFactory { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java index ae015c36aac..45746b23908 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java @@ -83,11 +83,21 @@ public class ApplicationList { return listOf(list.stream().filter(application -> ! isDeployingApplicationChange(application))); } + /** Returns the subset of applications which is currently not deploying a change */ + public ApplicationList notDeploying() { + return listOf(list.stream().filter(application -> ! application.deploying().isPresent())); + } + /** Returns the subset of applications which currently does not have any failing jobs */ public ApplicationList notFailing() { return listOf(list.stream().filter(application -> ! application.deploymentJobs().hasFailures())); } + /** Returns the subset of applications which currently have failing jobs */ + public ApplicationList failing() { + return listOf(list.stream().filter(application -> application.deploymentJobs().hasFailures())); + } + /** Returns the subset of applications which have been failing an upgrade to the given version since the given instant */ public ApplicationList failingUpgradeToVersionSince(Version version, Instant threshold) { return listOf(list.stream().filter(application -> failingUpgradeToVersionSince(application, version, threshold))); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java index f94b0f7284b..2d9bf45a39a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java @@ -105,12 +105,12 @@ public class DeploymentJobs { /** Returns whether this has some job status which is not a success */ public boolean hasFailures() { - return status.values().stream().anyMatch(jobStatus -> ! jobStatus.isSuccess()); + return JobList.from(status.values()).failing().anyMatch(); } /** Returns whether any job is currently in progress */ public boolean isRunning(Instant timeoutLimit) { - return status.values().stream().anyMatch(job -> job.isRunning(timeoutLimit)); + return JobList.from(status.values()).running(timeoutLimit).anyMatch(); } /** Returns whether the given job type is currently running and was started after timeoutLimit */ @@ -166,33 +166,44 @@ public class DeploymentJobs { public Optional<IssueId> issueId() { return issueId; } + private static Optional<Long> requireId(Optional<Long> id, String message) { + Objects.requireNonNull(id, message); + if ( ! id.isPresent()) { + return id; + } + if (id.get() <= 0) { + throw new IllegalArgumentException(message); + } + return id; + } + /** Job types that exist in the build system */ public enum JobType { - component("component"), - systemTest("system-test", zone(SystemName.cd, "test", "cd-us-central-1"), zone("test", "us-east-1")), - stagingTest("staging-test", zone(SystemName.cd, "staging", "cd-us-central-1"), zone("staging", "us-east-3")), - productionCorpUsEast1("production-corp-us-east-1", zone("prod", "corp-us-east-1")), - productionUsEast3("production-us-east-3", zone("prod", "us-east-3")), - productionUsWest1("production-us-west-1", zone("prod", "us-west-1")), - productionUsCentral1("production-us-central-1", zone("prod", "us-central-1")), - productionApNortheast1("production-ap-northeast-1", zone("prod", "ap-northeast-1")), - productionApNortheast2("production-ap-northeast-2", zone("prod", "ap-northeast-2")), - productionApSoutheast1("production-ap-southeast-1", zone("prod", "ap-southeast-1")), - productionEuWest1("production-eu-west-1", zone("prod", "eu-west-1")), - productionCdUsCentral1("production-cd-us-central-1", zone(SystemName.cd, "prod", "cd-us-central-1")), - productionCdUsCentral2("production-cd-us-central-2", zone(SystemName.cd, "prod", "cd-us-central-2")); - - private final String id; + component ("component") , + systemTest ("system-test" , zone("test" , "us-east-1" ), zone(SystemName.cd, "test" , "cd-us-central-1")), + stagingTest ("staging-test" , zone("staging", "us-east-3" ), zone(SystemName.cd, "staging", "cd-us-central-1")), + productionCorpUsEast1 ("production-corp-us-east-1" , zone("prod" , "corp-us-east-1")), + productionUsEast3 ("production-us-east-3" , zone("prod" , "us-east-3" )), + productionUsWest1 ("production-us-west-1" , zone("prod" , "us-west-1" )), + productionUsCentral1 ("production-us-central-1" , zone("prod" , "us-central-1" )), + productionApNortheast1 ("production-ap-northeast-1" , zone("prod" , "ap-northeast-1")), + productionApNortheast2 ("production-ap-northeast-2" , zone("prod" , "ap-northeast-2")), + productionApSoutheast1 ("production-ap-southeast-1" , zone("prod" , "ap-southeast-1")), + productionEuWest1 ("production-eu-west-1" , zone("prod" , "eu-west-1" )), + productionCdUsCentral1 ("production-cd-us-central-1", zone(SystemName.cd, "prod", "cd-us-central-1")), + productionCdUsCentral2 ("production-cd-us-central-2", zone(SystemName.cd, "prod", "cd-us-central-2")); + + private final String jobName; private final ImmutableMap<SystemName, Zone> zones; - JobType(String id, Zone... zones) { - this.id = id; + JobType(String jobName, Zone... zones) { + this.jobName = jobName; this.zones = ImmutableMap.copyOf(Stream.of(zones).collect(Collectors.toMap(zone -> zone.system(), zone -> zone))); } - public String id() { return id; } + public String jobName() { return jobName; } /** Returns the zone for this job in the given system, or empty if this job does not have a zone */ public Optional<Zone> zone(SystemName system) { @@ -217,33 +228,17 @@ public class DeploymentJobs { return zone(system).map(Zone::region); } - public static JobType fromId(String id) { - switch (id) { - case "component" : return component; - case "system-test" : return systemTest; - case "staging-test" : return stagingTest; - case "production-corp-us-east-1" : return productionCorpUsEast1; - case "production-us-east-3" : return productionUsEast3; - case "production-us-west-1" : return productionUsWest1; - case "production-us-central-1" : return productionUsCentral1; - case "production-ap-northeast-1" : return productionApNortheast1; - case "production-ap-northeast-2" : return productionApNortheast2; - case "production-ap-southeast-1" : return productionApSoutheast1; - case "production-eu-west-1" : return productionEuWest1; - case "production-cd-us-central-1" : return productionCdUsCentral1; - case "production-cd-us-central-2" : return productionCdUsCentral2; - default : throw new IllegalArgumentException("Unknown job id '" + id + "'"); - } + public static JobType fromJobName(String jobName) { + return Stream.of(values()) + .filter(jobType -> jobType.jobName.equals(jobName)) + .findAny().orElseThrow(() -> new IllegalArgumentException("Unknown job name '" + jobName + "'")); } /** Returns the job type for the given zone */ public static Optional<JobType> from(SystemName system, Zone zone) { - for (JobType job : values()) { - Optional<Zone> jobZone = job.zone(system); - if (jobZone.isPresent() && jobZone.get().equals(zone)) - return Optional.of(job); - } - return Optional.empty(); + return Stream.of(values()) + .filter(job -> job.zone(system).filter(zone::equals).isPresent()) + .findAny(); } /** Returns the job job type for the given environment and region or null if none */ @@ -290,7 +285,7 @@ public class DeploymentJobs { public JobType jobType() { return jobType; } public long projectId() { return projectId; } public long buildNumber() { return buildNumber; } - public boolean success() { return !jobError.isPresent(); } + public boolean success() { return ! jobError.isPresent(); } public Optional<JobError> jobError() { return jobError; } } @@ -298,23 +293,6 @@ public class DeploymentJobs { public enum JobError { unknown, outOfCapacity; - - public static Optional<JobError> from(boolean success) { - return Optional.of(success) - .filter(b -> !b) - .map(ignored -> unknown); - } - } - - private static Optional<Long> requireId(Optional<Long> id, String message) { - Objects.requireNonNull(id, message); - if (!id.isPresent()) { - return id; - } - if (id.get() <= 0) { - throw new IllegalArgumentException(message); - } - return id; } }
\ No newline at end of file diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java index b7f0e71efb7..7d1795aeaa3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java @@ -68,7 +68,10 @@ public class JobList { /** Returns the subset of jobs which are current upgrading */ public JobList upgrading() { // TODO: Centralise and standardise reasoning about upgrades and revisions. - return filter(job -> job.lastSuccess().isPresent() && job.lastSuccess().get().version().isBefore(job.lastTriggered().get().version())); + return filter(job -> job.lastSuccess().isPresent() + && job.lastTriggered().isPresent() + && ! job.lastTriggered().get().at().isBefore(job.lastCompleted().get().at()) + && job.lastSuccess().get().version().isBefore(job.lastTriggered().get().version())); } /** Returns the subset of jobs which are currently running, according to the given timeout */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java index 8fb01577e2c..9201c74e761 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.controller.deployment; import com.yahoo.config.application.api.DeploymentSpec; -import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; @@ -16,19 +15,16 @@ import com.yahoo.vespa.hosted.controller.application.JobStatus; import java.time.Clock; import java.time.Duration; import java.time.Instant; +import java.util.Collection; import java.util.Collections; -import java.util.Comparator; -import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.Optional; -import java.util.function.Function; import java.util.logging.Logger; -import java.util.stream.Collector; -import java.util.stream.Collectors; +import static java.util.Comparator.comparingInt; import static java.util.stream.Collectors.collectingAndThen; +import static java.util.stream.Collectors.toList; /** * This class determines the order of deployments according to an application's deployment spec. @@ -64,7 +60,7 @@ public class DeploymentOrder { // At this point we have deployed to system test, so deployment spec is available List<DeploymentSpec.Step> deploymentSteps = deploymentSteps(application); Optional<DeploymentSpec.Step> currentStep = fromJob(job, application); - if (!currentStep.isPresent()) { + if ( ! currentStep.isPresent()) { return Collections.emptyList(); } @@ -75,13 +71,13 @@ public class DeploymentOrder { } // Postpone if step hasn't completed all its jobs for this change - if (!completedSuccessfully(currentStep.get(), application.deploying().get(), application)) { + if ( ! completedSuccessfully(currentStep.get(), application.deploying().get(), application)) { return Collections.emptyList(); } // Postpone next job if delay has not passed yet Duration delay = delayAfter(currentStep.get(), application); - if (postponeDeployment(delay, job, application)) { + if (shouldPostponeDeployment(delay, job, application)) { log.info(String.format("Delaying next job after %s of %s by %s", job, application, delay)); return Collections.emptyList(); } @@ -89,11 +85,11 @@ public class DeploymentOrder { DeploymentSpec.Step nextStep = deploymentSteps.get(currentIndex + 1); return nextStep.zones().stream() .map(this::toJob) - .collect(collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); + .collect(collectingAndThen(toList(), Collections::unmodifiableList)); } - /** Returns whether the given job is first in a deployment */ - public boolean isFirst(JobType job) { + /** Returns whether the given job causes an application change */ + public boolean givesApplicationChange(JobType job) { return job == JobType.component; } @@ -113,35 +109,33 @@ public class DeploymentOrder { public List<JobType> jobsFrom(DeploymentSpec deploymentSpec) { return deploymentSpec.steps().stream() .flatMap(step -> jobsFrom(step).stream()) - .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); + .collect(collectingAndThen(toList(), Collections::unmodifiableList)); } /** Returns job status sorted according to deployment spec */ - public Map<JobType, JobStatus> sortBy(DeploymentSpec deploymentSpec, Map<JobType, JobStatus> jobStatus) { - List<DeploymentJobs.JobType> jobs = jobsFrom(deploymentSpec); - return jobStatus.entrySet().stream() - .sorted(Comparator.comparingInt(kv -> jobs.indexOf(kv.getKey()))) - .collect(Collectors.collectingAndThen(toLinkedMap(Map.Entry::getKey, Map.Entry::getValue), - Collections::unmodifiableMap)); + public List<JobStatus> sortBy(DeploymentSpec deploymentSpec, Collection<JobStatus> jobStatus) { + List<DeploymentJobs.JobType> sortedJobs = jobsFrom(deploymentSpec); + return jobStatus.stream() + .sorted(comparingInt(job -> sortedJobs.indexOf(job.type()))) + .collect(collectingAndThen(toList(), Collections::unmodifiableList)); } /** Returns deployments sorted according to declared zones */ - public Map<Zone, Deployment> sortBy(List<DeploymentSpec.DeclaredZone> zones, Map<Zone, Deployment> deployments) { + public List<Deployment> sortBy(List<DeploymentSpec.DeclaredZone> zones, Collection<Deployment> deployments) { List<Zone> productionZones = zones.stream() - .filter(z -> z.environment() == Environment.prod && z.region().isPresent()) + .filter(z -> z.region().isPresent()) .map(z -> new Zone(z.environment(), z.region().get())) - .collect(Collectors.toList()); - return deployments.entrySet().stream() - .sorted(Comparator.comparingInt(kv -> productionZones.indexOf(kv.getKey()))) - .collect(Collectors.collectingAndThen(toLinkedMap(Map.Entry::getKey, Map.Entry::getValue), - Collections::unmodifiableMap)); + .collect(toList()); + return deployments.stream() + .sorted(comparingInt(deployment -> productionZones.indexOf(deployment.zone()))) + .collect(collectingAndThen(toList(), Collections::unmodifiableList)); } /** Returns jobs for the given step */ private List<JobType> jobsFrom(DeploymentSpec.Step step) { return step.zones().stream() .map(this::toJob) - .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); + .collect(collectingAndThen(toList(), Collections::unmodifiableList)); } /** Returns whether all jobs have completed successfully for given step */ @@ -167,7 +161,7 @@ public class DeploymentOrder { } /** Returns whether deployment should be postponed according to delay */ - private boolean postponeDeployment(Duration delay, JobType job, Application application) { + private boolean shouldPostponeDeployment(Duration delay, JobType job, Application application) { Optional<Instant> lastSuccess = Optional.ofNullable(application.deploymentJobs().jobStatus().get(job)) .flatMap(JobStatus::lastSuccess) .map(JobStatus.JobRun::at); @@ -177,9 +171,8 @@ public class DeploymentOrder { /** Find all steps that deploy to one or more zones */ private static List<DeploymentSpec.Step> deploymentSteps(Application application) { return application.deploymentSpec().steps().stream() - .filter(step -> step instanceof DeploymentSpec.DeclaredZone || - step instanceof DeploymentSpec.ParallelZones) - .collect(Collectors.toList()); + .filter(step -> ! step.zones().isEmpty()) + .collect(toList()); } /** Determines the delay that should pass after the given step */ @@ -200,13 +193,4 @@ public class DeploymentOrder { return totalDelay; } - private static <T, K, U> Collector<T, ?, Map<K,U>> toLinkedMap(Function<? super T, ? extends K> keyMapper, - Function<? super T, ? extends U> valueMapper) { - return Collectors.toMap(keyMapper, valueMapper, - (u, v) -> { - throw new IllegalStateException(String.format("Duplicate key %s", u)); - }, - LinkedHashMap::new); - } - } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java index e5c7b7f1c2f..3d2c2ed0319 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java @@ -82,7 +82,7 @@ public class DeploymentTrigger { // Handle successful starting and ending if (report.success()) { - if (order.isFirst(report.jobType())) { // the first job tells us that a change occurred + if (order.givesApplicationChange(report.jobType())) { if (acceptNewRevisionNow(application)) { // Set this as the change we are doing, unless we are already pushing a platform change if ( ! ( application.deploying().isPresent() && @@ -103,16 +103,13 @@ public class DeploymentTrigger { // Trigger next if (report.success()) application = trigger(order.nextAfter(report.jobType(), application), application, - String.format("%s completed successfully in build %d", - report.jobType(), report.buildNumber())); + report.jobType().jobName() + " completed"); else if (isCapacityConstrained(report.jobType()) && shouldRetryOnOutOfCapacity(application, report.jobType())) application = trigger(report.jobType(), application, true, - String.format("Retrying due to out of capacity in build %d", - report.buildNumber())); + "Retrying on out of capacity"); else if (shouldRetryNow(application)) application = trigger(report.jobType(), application, false, - String.format("Retrying as build %d just started failing", - report.buildNumber())); + "Immediate retry on failure"); applications().store(application); } @@ -164,7 +161,7 @@ public class DeploymentTrigger { nextToTrigger.add(nextJobType); } // Trigger them in parallel - application = trigger(nextToTrigger, application, "Triggering previously blocked jobs"); + application = trigger(nextToTrigger, application, "Available change in " + jobType.jobName()); controller.applications().store(application); } } @@ -271,7 +268,7 @@ public class DeploymentTrigger { application = application.withDeploying(Optional.of(change)); if (change instanceof Change.ApplicationChange) application = application.withOutstandingChange(false); - application = trigger(JobType.systemTest, application, false, "Deploying change"); + application = trigger(JobType.systemTest, application, false, "Deploying " + change); applications().store(application); } } @@ -365,18 +362,18 @@ public class DeploymentTrigger { * @param jobType the type of the job to trigger, or null to trigger nothing * @param application the application to trigger the job for * @param first whether to trigger the job before other jobs - * @param cause describes why the job is triggered + * @param reason describes why the job is triggered * @return the application in the triggered state, which *must* be stored by the caller */ - private LockedApplication trigger(JobType jobType, LockedApplication application, boolean first, String cause) { + private LockedApplication trigger(JobType jobType, LockedApplication application, boolean first, String reason) { if (isRunningProductionJob(application)) return application; - return triggerAllowParallel(jobType, application, first, false, cause); + return triggerAllowParallel(jobType, application, first, false, reason); } - private LockedApplication trigger(List<JobType> jobs, LockedApplication application, String cause) { + private LockedApplication trigger(List<JobType> jobs, LockedApplication application, String reason) { if (isRunningProductionJob(application)) return application; for (JobType job : jobs) - application = triggerAllowParallel(job, application, false, false, cause); + application = triggerAllowParallel(job, application, false, false, reason); return application; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/PolledBuildSystem.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/PolledBuildSystem.java index 56b4023f932..e25db10a8cd 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/PolledBuildSystem.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/PolledBuildSystem.java @@ -87,9 +87,9 @@ public class PolledBuildSystem implements BuildSystem { Optional<Long> projectId = projectId(application); if (projectId.isPresent()) { - jobsToRun.add(new BuildJob(projectId.get(), jobType.id())); + jobsToRun.add(new BuildJob(projectId.get(), jobType.jobName())); } else { - log.warning("Not queuing " + jobType.id() + " for " + application.toShortString() + + log.warning("Not queuing " + jobType.jobName() + " for " + application.toShortString() + " because project ID is missing"); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java index 36b87e4cead..e4f4bd2ee96 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java @@ -41,23 +41,32 @@ public class Upgrader extends Maintainer { * Schedule application upgrades. Note that this implementation must be idempotent. */ @Override - public void maintain() { - ApplicationList applications = applications(); - + public void maintain() { // Determine target versions for each upgrade policy Optional<Version> canaryTarget = controller().versionStatus().systemVersion().map(VespaVersion::versionNumber); Optional<Version> defaultTarget = newestVersionWithConfidence(VespaVersion.Confidence.normal); Optional<Version> conservativeTarget = newestVersionWithConfidence(VespaVersion.Confidence.high); - // Cancel any upgrades to the wrong targets - cancelUpgradesOf(applications.with(UpgradePolicy.canary).upgrading().notUpgradingTo(canaryTarget)); - cancelUpgradesOf(applications.with(UpgradePolicy.defaultPolicy).upgrading().notUpgradingTo(defaultTarget)); - cancelUpgradesOf(applications.with(UpgradePolicy.conservative).upgrading().notUpgradingTo(conservativeTarget)); + // Cancel upgrades to broken targets (let other ongoing upgrades complete to avoid starvation + for (VespaVersion version : controller().versionStatus().versions()) { + if (version.confidence() == VespaVersion.Confidence.broken) + cancelUpgradesOf(applications().without(UpgradePolicy.canary).upgradingTo(version.versionNumber()), + version.versionNumber() + " is broken"); + } + + // Canaries should always try the canary target + cancelUpgradesOf(applications().with(UpgradePolicy.canary).upgrading().notUpgradingTo(canaryTarget), + "Outdated target version for Canaries"); + + // Cancel *failed* upgrades to earlier versions, as the new version may fix it + String reason = "Failing on outdated version"; + cancelUpgradesOf(applications().with(UpgradePolicy.defaultPolicy).upgrading().failing().notUpgradingTo(defaultTarget), reason); + cancelUpgradesOf(applications().with(UpgradePolicy.conservative).upgrading().failing().notUpgradingTo(conservativeTarget), reason); // Schedule the right upgrades - canaryTarget.ifPresent(target -> upgrade(applications.with(UpgradePolicy.canary), target)); - defaultTarget.ifPresent(target -> upgrade(applications.with(UpgradePolicy.defaultPolicy), target)); - conservativeTarget.ifPresent(target -> upgrade(applications.with(UpgradePolicy.conservative), target)); + canaryTarget.ifPresent(target -> upgrade(applications().with(UpgradePolicy.canary), target)); + defaultTarget.ifPresent(target -> upgrade(applications().with(UpgradePolicy.defaultPolicy), target)); + conservativeTarget.ifPresent(target -> upgrade(applications().with(UpgradePolicy.conservative), target)); } private Optional<Version> newestVersionWithConfidence(VespaVersion.Confidence confidence) { @@ -79,13 +88,11 @@ public class Upgrader extends Maintainer { private void upgrade(ApplicationList applications, Version version) { Change.VersionChange change = new Change.VersionChange(version); - cancelUpgradesOf(applications.upgradingToLowerThan(version)); applications = applications.notPullRequest(); // Pull requests are deployed as separate applications to test then deleted; No need to upgrade applications = applications.hasProductionDeployment(); applications = applications.onLowerVersionThan(version); - applications = applications.notDeployingApplication(); // wait with applications deploying an application change + applications = applications.notDeploying(); // wait with applications deploying an application change or already upgrading applications = applications.notFailingOn(version); // try to upgrade only if it hasn't failed on this version - applications = applications.notCurrentlyUpgrading(change, controller().applications().deploymentTrigger().jobTimeoutLimit()); applications = applications.canUpgradeAt(controller().clock().instant()); // wait with applications that are currently blocking upgrades applications = applications.byIncreasingDeployedVersion(); // start with lowest versions applications = applications.first(numberOfApplicationsToUpgrade()); // throttle upgrades @@ -98,9 +105,9 @@ public class Upgrader extends Maintainer { } } - private void cancelUpgradesOf(ApplicationList applications) { + private void cancelUpgradesOf(ApplicationList applications, String reason) { if (applications.isEmpty()) return; - log.info("Cancelling upgrading of " + applications.asList().size() + " applications"); + log.info("Cancelling upgrading of " + applications.asList().size() + " applications: " + reason); for (Application application : applications.asList()) controller().applications().deploymentTrigger().cancelChange(application.id()); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java index 3bd1abdf607..2745862f68a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java @@ -215,7 +215,7 @@ public class ApplicationSerializer { } private void toSlime(JobStatus jobStatus, Cursor object) { - object.setString(jobTypeField, jobStatus.type().id()); + object.setString(jobTypeField, jobStatus.type().jobName()); if (jobStatus.jobError().isPresent()) object.setString(errorField, jobStatus.jobError().get().name()); @@ -373,7 +373,7 @@ public class ApplicationSerializer { } private JobStatus jobStatusFromSlime(Inspector object) { - DeploymentJobs.JobType jobType = DeploymentJobs.JobType.fromId(object.field(jobTypeField).asString()); + DeploymentJobs.JobType jobType = DeploymentJobs.JobType.fromJobName(object.field(jobTypeField).asString()); Optional<JobError> jobError = Optional.empty(); if (object.field(errorField).valid()) 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 a259e221a1e..50b80e34788 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 @@ -354,14 +354,14 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } // Jobs sorted according to deployment spec - Map<DeploymentJobs.JobType, JobStatus> jobStatus = controller.applications().deploymentTrigger() + List<JobStatus> jobStatus = controller.applications().deploymentTrigger() .deploymentOrder() - .sortBy(application.deploymentSpec(), application.deploymentJobs().jobStatus()); + .sortBy(application.deploymentSpec(), application.deploymentJobs().jobStatus().values()); Cursor deploymentsArray = response.setArray("deploymentJobs"); - for (JobStatus job : jobStatus.values()) { + for (JobStatus job : jobStatus) { Cursor jobObject = deploymentsArray.addObject(); - jobObject.setString("type", job.type().id()); + jobObject.setString("type", job.type().jobName()); jobObject.setBool("success", job.isSuccess()); job.lastTriggered().ifPresent(jobRun -> toSlime(jobRun, jobObject.setObject("lastTriggered"))); @@ -382,11 +382,11 @@ public class ApplicationApiHandler extends LoggingRequestHandler { globalRotationsArray.addString(rotation.toString()); // Deployments sorted according to deployment spec - Map<Zone, Deployment> deployments = controller.applications().deploymentTrigger() + List<Deployment> deployments = controller.applications().deploymentTrigger() .deploymentOrder() - .sortBy(application.deploymentSpec().zones(), application.deployments()); + .sortBy(application.deploymentSpec().zones(), application.deployments().values()); Cursor instancesArray = response.setArray("instances"); - for (Deployment deployment : deployments.values()) { + for (Deployment deployment : deployments) { Cursor deploymentObject = instancesArray.addObject(); deploymentObject.setString("environment", deployment.zone().environment().value()); deploymentObject.setString("region", deployment.zone().region().value()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java index bcc72245f0b..529ac292d8b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java @@ -101,7 +101,7 @@ public class DeploymentApiHandler extends LoggingRequestHandler { firstFailingOn(version.versionNumber(), application).ifPresent(firstFailing -> { Cursor applicationObject = failingArray.addObject(); toSlime(applicationObject, application, request); - applicationObject.setString("failing", firstFailing.type().id()); + applicationObject.setString("failing", firstFailing.type().jobName()); }); }); } @@ -124,7 +124,7 @@ public class DeploymentApiHandler extends LoggingRequestHandler { lastDeployingTo(version.versionNumber(), application).ifPresent(lastDeploying -> { Cursor applicationObject = runningArray.addObject(); toSlime(applicationObject, application, request); - applicationObject.setString("running", lastDeploying.type().id()); + applicationObject.setString("running", lastDeploying.type().jobName()); }); }); } @@ -181,8 +181,7 @@ public class DeploymentApiHandler extends LoggingRequestHandler { /** The last triggered upgrade to this version, for this application */ private Optional<JobStatus> lastDeployingTo(Version version, Application application) { return JobList.from(application) - .running(controller.applications().deploymentTrigger().jobTimeoutLimit()) - .lastTriggered().upgrade() + .upgrading() .asList().stream() .max(comparing(job -> job.lastTriggered().get().at())); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java index 3dbff0b4aa3..a92e2533092 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java @@ -108,19 +108,19 @@ public class ScrewdriverApiHandler extends LoggingRequestHandler { LockedApplication application = controller.applications().require(applicationId, lock); JobType jobType = Optional.of(asString(request.getData())) .filter(s -> !s.isEmpty()) - .map(JobType::fromId) + .map(JobType::fromJobName) .orElse(JobType.component); // Since this is a manual operation we likely want it to trigger as soon as possible so we add it at to the // front of the queue application = controller.applications().deploymentTrigger().triggerAllowParallel( jobType, application, true, true, - "Triggered from the screwdriver/v1 web service" + "Triggered from screwdriver/v1" ); controller.applications().store(application); Slime slime = new Slime(); Cursor cursor = slime.setObject(); - cursor.setString("message", "Triggered " + jobType.id() + " for " + applicationId); + cursor.setString("message", "Triggered " + jobType.jobName() + " for " + applicationId); return new SlimeJsonResponse(slime); } } @@ -174,7 +174,7 @@ public class ScrewdriverApiHandler extends LoggingRequestHandler { report.field("tenant").asString(), report.field("application").asString(), report.field("instance").asString()), - JobType.fromId(report.field("jobName").asString()), + JobType.fromJobName(report.field("jobName").asString()), report.field("projectId").asLong(), report.field("buildNumber").asLong(), jobError diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java index cebfff2ec41..d152cf80472 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java @@ -165,7 +165,6 @@ public class VersionStatus { // Deploying versions JobList.from(application) - .running(jobTimeoutLimit) .upgrading() .mapToList(job -> job.lastTriggered().get().version()) .forEach(version -> versionMap.put(version, versionMap.getOrDefault(version, DeploymentStatistics.empty(version)).withDeploying(application.id()))); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index aea66f3cd67..7875c1f4964 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -94,7 +94,7 @@ public class ControllerTest { // staging job - succeeding Version version1 = Version.fromString("6.1"); // Set in config server mock Application app1 = tester.createApplication("app1", "tenant1", 1, 11L); - applications.notifyJobCompletion(mockReport(app1, component, true)); + tester.notifyJobCompletion(component, app1, true); assertFalse("Revision is currently not known", ((Change.ApplicationChange)tester.controller().applications().require(app1.id()).deploying().get()).revision().isPresent()); tester.deployAndNotify(app1, applicationPackage, true, systemTest); @@ -136,7 +136,7 @@ public class ControllerTest { tester.clock().advance(Duration.ofSeconds(1)); // system and staging test job - succeeding - applications.notifyJobCompletion(mockReport(app1, component, true)); + tester.notifyJobCompletion(component, app1, true); tester.deployAndNotify(app1, applicationPackage, true, false, systemTest); assertStatus(JobStatus.initial(systemTest) .withTriggering(-1, version1, revision, false, "", tester.clock().instant()) @@ -163,7 +163,7 @@ public class ControllerTest { .environment(Environment.prod) .region("us-east-3") .build(); - applications.notifyJobCompletion(mockReport(app1, component, true)); + tester.notifyJobCompletion(component, app1, true); try { tester.deploy(systemTest, app1, applicationPackage); fail("Expected exception due to unallowed production deployment removal"); @@ -176,7 +176,7 @@ public class ControllerTest { JobStatus jobStatus = applications.require(app1.id()).deploymentJobs().jobStatus().get(productionCorpUsEast1); assertNotNull("Deployment job was not removed", jobStatus); assertEquals(42, jobStatus.lastCompleted().get().id()); - assertEquals("stagingTest completed successfully in build 42", jobStatus.lastCompleted().get().reason()); + assertEquals("staging-test completed", jobStatus.lastCompleted().get().reason()); // prod zone removal is allowed with override applicationPackage = new ApplicationPackageBuilder() @@ -205,7 +205,7 @@ public class ControllerTest { Application app1 = tester.createApplication("application1", "tenant1", 1, 1L); // First deployment: An application change - applications.notifyJobCompletion(mockReport(app1, component, true)); + tester.notifyJobCompletion(component, app1, true); tester.deployAndNotify(app1, applicationPackage, true, systemTest); tester.deployAndNotify(app1, applicationPackage, true, stagingTest); tester.deployAndNotify(app1, applicationPackage, true, productionUsWest1); @@ -228,7 +228,7 @@ public class ControllerTest { .region("us-west-1") .region("us-east-3") .build(); - applications.notifyJobCompletion(mockReport(app1, component, true)); + tester.notifyJobCompletion(component, app1, true); tester.deployAndNotify(app1, applicationPackage, true, systemTest); tester.deployAndNotify(app1, applicationPackage, true, stagingTest); tester.deployAndNotify(app1, applicationPackage, true, productionUsWest1); @@ -425,7 +425,7 @@ public class ControllerTest { // app1: staging-test job fails with out of capacity and is added to the front of the queue tester.deploy(stagingTest, app1, applicationPackage); tester.notifyJobCompletion(stagingTest, app1, Optional.of(JobError.outOfCapacity)); - assertEquals(stagingTest.id(), buildSystem.jobs().get(0).jobName()); + assertEquals(stagingTest.jobName(), buildSystem.jobs().get(0).jobName()); assertEquals(project1, buildSystem.jobs().get(0).projectId()); // app2 and app3: Completes deployment @@ -461,15 +461,15 @@ public class ControllerTest { List<BuildJob> nextJobs = buildSystem.takeJobsToRun(); assertEquals(2, nextJobs.size()); - assertEquals(stagingTest.id(), nextJobs.get(0).jobName()); + assertEquals(stagingTest.jobName(), nextJobs.get(0).jobName()); assertEquals(project2, nextJobs.get(0).projectId()); - assertEquals(stagingTest.id(), nextJobs.get(1).jobName()); + assertEquals(stagingTest.jobName(), nextJobs.get(1).jobName()); assertEquals(project3, nextJobs.get(1).projectId()); // And finally the requeued job for app1 nextJobs = buildSystem.takeJobsToRun(); assertEquals(1, nextJobs.size()); - assertEquals(stagingTest.id(), nextJobs.get(0).jobName()); + assertEquals(stagingTest.jobName(), nextJobs.get(0).jobName()); assertEquals(project1, nextJobs.get(0).projectId()); } @@ -480,20 +480,6 @@ public class ControllerTest { assertEquals(expectedStatus, existingStatus); } - private JobReport mockReport(Application application, JobType jobType, Optional<JobError> jobError) { - return new JobReport( - application.id(), - jobType, - application.deploymentJobs().projectId().get(), - 42, - jobError - ); - } - - private JobReport mockReport(Application application, JobType jobType, boolean success) { - return mockReport(application, jobType, JobError.from(success)); - } - @Test public void testGlobalRotations() throws IOException { // Setup tester and app def diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java index 9e228e0becb..773ecf313cc 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java @@ -28,6 +28,7 @@ import java.util.Optional; import java.util.UUID; import java.util.stream.Collectors; +import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError.unknown; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -140,6 +141,20 @@ public class DeploymentTester { completeDeployment(application, applicationPackage, Optional.empty(), true); } + public static DeploymentJobs.JobReport jobReport(Application application, JobType jobType, boolean success) { + return jobReport(application, jobType, Optional.ofNullable(success ? null : unknown)); + } + + public static DeploymentJobs.JobReport jobReport(Application application, JobType jobType, Optional<DeploymentJobs.JobError> jobError) { + return new DeploymentJobs.JobReport( + application.id(), + jobType, + application.deploymentJobs().projectId().get(), + 42, + jobError + ); + } + /** Deploy application using the given application package, but expecting to stop after test phases */ public void deployTestOnly(Application application, ApplicationPackage applicationPackage) { notifyJobCompletion(JobType.component, application, true); @@ -172,7 +187,7 @@ public class DeploymentTester { } public void notifyJobCompletion(JobType jobType, Application application, boolean success) { - notifyJobCompletion(jobType, application, DeploymentJobs.JobError.from(success)); + notifyJobCompletion(jobType, application, Optional.ofNullable(success ? null : unknown)); } public void notifyJobCompletion(JobType jobType, Application application, Optional<DeploymentJobs.JobError> jobError) { @@ -228,7 +243,7 @@ public class DeploymentTester { for (JobType job : jobs) { BuildService.BuildJob buildJob = findJob(application, job); assertEquals((long) application.deploymentJobs().projectId().get(), buildJob.projectId()); - assertEquals(job.id(), buildJob.jobName()); + assertEquals(job.jobName(), buildJob.jobName()); } if (expectOnlyTheseJobs) assertEquals(jobs.length, countJobsOf(application)); @@ -237,7 +252,7 @@ public class DeploymentTester { private BuildService.BuildJob findJob(Application application, JobType jobType) { for (BuildService.BuildJob job : buildSystem().jobs()) - if (job.projectId() == application.deploymentJobs().projectId().get() && job.jobName().equals(jobType.id())) + if (job.projectId() == application.deploymentJobs().projectId().get() && job.jobName().equals(jobType.jobName())) return job; throw new NoSuchElementException(jobType + " is not scheduled for " + application); } @@ -247,15 +262,6 @@ public class DeploymentTester { .filter(job -> job.projectId() == application.deploymentJobs().projectId().get()) .count(); } - private DeploymentJobs.JobReport jobReport(Application application, JobType jobType, Optional<DeploymentJobs.JobError> jobError) { - return new DeploymentJobs.JobReport( - application.id(), - jobType, - application.deploymentJobs().projectId().get(), - 42, - jobError - ); - } private static ApplicationPackage applicationPackage(String upgradePolicy) { return new ApplicationPackageBuilder() diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java index 022fa705def..5a753617761 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java @@ -72,7 +72,7 @@ public class DeploymentTriggerTest { tester.clock().advance(Duration.ofHours(12).plus(Duration.ofSeconds(1))); tester.failureRedeployer().maintain(); assertEquals("Retried dead job", 1, tester.buildSystem().jobs().size()); - assertEquals(JobType.stagingTest.id(), tester.buildSystem().jobs().get(0).jobName()); + assertEquals(JobType.stagingTest.jobName(), tester.buildSystem().jobs().get(0).jobName()); } @Test @@ -131,7 +131,7 @@ public class DeploymentTriggerTest { // Consume us-west-1 job without reporting completion assertEquals(1, buildSystem.jobs().size()); - assertEquals(JobType.productionUsWest1.id(), buildSystem.jobs().get(0).jobName()); + assertEquals(JobType.productionUsWest1.jobName(), buildSystem.jobs().get(0).jobName()); buildSystem.takeJobsToRun(); // 3 minutes pass, delayed trigger does nothing as us-west-1 is still in progress @@ -184,8 +184,8 @@ public class DeploymentTriggerTest { // Deploys in two regions in parallel assertEquals(2, tester.buildSystem().jobs().size()); - assertEquals(JobType.productionUsEast3.id(), tester.buildSystem().jobs().get(0).jobName()); - assertEquals(JobType.productionUsWest1.id(), tester.buildSystem().jobs().get(1).jobName()); + assertEquals(JobType.productionUsEast3.jobName(), tester.buildSystem().jobs().get(0).jobName()); + assertEquals(JobType.productionUsWest1.jobName(), tester.buildSystem().jobs().get(1).jobName()); tester.buildSystem().takeJobsToRun(); tester.deploy(JobType.productionUsWest1, application, applicationPackage, false); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/MockBuildService.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/MockBuildService.java index 0293ea08d65..1b1a4feaa4e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/MockBuildService.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/MockBuildService.java @@ -13,6 +13,7 @@ import java.time.Duration; import java.util.EnumMap; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import java.util.function.Supplier; import static com.yahoo.vespa.hosted.controller.deployment.MockBuildService.JobStatus.QUEUED; @@ -161,11 +162,11 @@ public class MockBuildService implements BuildService { jobType, projectId, 42, - JobError.from(success) + Optional.ofNullable(success ? null : JobError.unknown) )); } - private BuildJob buildJob() { return new BuildJob(projectId, jobType.id()); } + private BuildJob buildJob() { return new BuildJob(projectId, jobType.jobName()); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java index 2782dd6ec3b..87ef7ed07b1 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java @@ -70,7 +70,7 @@ public class FailureRedeployerTest { tester.clock().advance(Duration.ofMinutes(1)); tester.failureRedeployer().maintain(); assertFalse("Job is not retried", tester.buildSystem().jobs().stream() - .anyMatch(j -> j.jobName().equals(DeploymentJobs.JobType.productionUsEast3.id()))); + .anyMatch(j -> j.jobName().equals(DeploymentJobs.JobType.productionUsEast3.jobName()))); // Test environments pass tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); @@ -109,14 +109,14 @@ public class FailureRedeployerTest { tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); // staging-test starts, but does not complete - assertEquals(DeploymentJobs.JobType.stagingTest.id(), tester.buildSystem().takeJobsToRun().get(0).jobName()); + assertEquals(DeploymentJobs.JobType.stagingTest.jobName(), tester.buildSystem().takeJobsToRun().get(0).jobName()); tester.failureRedeployer().maintain(); assertTrue("No jobs retried", tester.buildSystem().jobs().isEmpty()); // Just over 12 hours pass, job is retried tester.clock().advance(Duration.ofHours(12).plus(Duration.ofSeconds(1))); tester.failureRedeployer().maintain(); - assertEquals(DeploymentJobs.JobType.stagingTest.id(), tester.buildSystem().takeJobsToRun().get(0).jobName()); + assertEquals(DeploymentJobs.JobType.stagingTest.jobName(), tester.buildSystem().takeJobsToRun().get(0).jobName()); // Deployment completes tester.deploy(DeploymentJobs.JobType.stagingTest, app, applicationPackage, true); @@ -212,7 +212,7 @@ public class FailureRedeployerTest { // Production job starts, but does not complete assertEquals(1, tester.buildSystem().jobs().size()); - assertEquals("Production job triggered", DeploymentJobs.JobType.productionCdUsCentral1.id(), tester.buildSystem().jobs().get(0).jobName()); + assertEquals("Production job triggered", DeploymentJobs.JobType.productionCdUsCentral1.jobName(), tester.buildSystem().jobs().get(0).jobName()); tester.buildSystem().takeJobsToRun(); // Failure re-deployer runs diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java index 4886eba40b6..13636122cfd 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java @@ -49,7 +49,7 @@ public class OutstandingChangeDeployerTest { List<BuildService.BuildJob> jobs = tester.buildSystem().jobs(); assertEquals(1, jobs.size()); assertEquals(11, jobs.get(0).projectId()); - assertEquals(DeploymentJobs.JobType.systemTest.id(), jobs.get(0).jobName()); + assertEquals(DeploymentJobs.JobType.systemTest.jobName(), jobs.get(0).jobName()); assertFalse(tester.application("app1").hasOutstandingChange()); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java index 0414cda3f55..41f1d1914f3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java @@ -51,7 +51,7 @@ public class UpgraderTest { tester.upgrader().maintain(); assertEquals("All already on the right version: Nothing to do", 0, tester.buildSystem().jobs().size()); - // --- A new version is released - everything goes smoothly + // --- 5.1 is released - everything goes smoothly version = Version.fromString("5.1"); tester.updateVersionStatus(version); assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber()); @@ -86,7 +86,7 @@ public class UpgraderTest { tester.upgrader().maintain(); assertEquals("Nothing to do", 0, tester.buildSystem().jobs().size()); - // --- A new version is released - which fails a Canary + // --- 5.2 is released - which fails a Canary version = Version.fromString("5.2"); tester.updateVersionStatus(version); assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber()); @@ -148,51 +148,55 @@ public class UpgraderTest { assertEquals("Applications are on 5.3 - nothing to do", 0, tester.buildSystem().jobs().size()); // --- Starting upgrading to a new version which breaks, causing upgrades to commence on the previous version - version = Version.fromString("5.4"); + Version version54 = Version.fromString("5.4"); Application default3 = tester.createAndDeploy("default3", 5, "default"); // need 4 to break a version Application default4 = tester.createAndDeploy("default4", 5, "default"); - tester.updateVersionStatus(version); + tester.updateVersionStatus(version54); tester.upgrader().maintain(); // cause canary upgrades to 5.4 - tester.completeUpgrade(canary0, version, "canary"); - tester.completeUpgrade(canary1, version, "canary"); - tester.updateVersionStatus(version); + tester.completeUpgrade(canary0, version54, "canary"); + tester.completeUpgrade(canary1, version54, "canary"); + tester.updateVersionStatus(version54); assertEquals(VespaVersion.Confidence.normal, tester.controller().versionStatus().systemVersion().get().confidence()); tester.upgrader().maintain(); assertEquals("Upgrade of defaults are scheduled", 5, tester.buildSystem().jobs().size()); - assertEquals(version, ((Change.VersionChange)tester.application(default0.id()).deploying().get()).version()); - assertEquals(version, ((Change.VersionChange)tester.application(default1.id()).deploying().get()).version()); - assertEquals(version, ((Change.VersionChange)tester.application(default2.id()).deploying().get()).version()); - assertEquals(version, ((Change.VersionChange)tester.application(default3.id()).deploying().get()).version()); - assertEquals(version, ((Change.VersionChange)tester.application(default4.id()).deploying().get()).version()); - tester.completeUpgrade(default0, version, "default"); + assertEquals(version54, ((Change.VersionChange)tester.application(default0.id()).deploying().get()).version()); + assertEquals(version54, ((Change.VersionChange)tester.application(default1.id()).deploying().get()).version()); + assertEquals(version54, ((Change.VersionChange)tester.application(default2.id()).deploying().get()).version()); + assertEquals(version54, ((Change.VersionChange)tester.application(default3.id()).deploying().get()).version()); + assertEquals(version54, ((Change.VersionChange)tester.application(default4.id()).deploying().get()).version()); + tester.completeUpgrade(default0, version54, "default"); // State: Default applications started upgrading to 5.4 (and one completed) - version = Version.fromString("5.5"); - tester.updateVersionStatus(version); + Version version55 = Version.fromString("5.5"); + tester.updateVersionStatus(version55); tester.upgrader().maintain(); // cause canary upgrades to 5.5 - tester.completeUpgrade(canary0, version, "canary"); - tester.completeUpgrade(canary1, version, "canary"); - tester.updateVersionStatus(version); + tester.completeUpgrade(canary0, version55, "canary"); + tester.completeUpgrade(canary1, version55, "canary"); + tester.updateVersionStatus(version55); assertEquals(VespaVersion.Confidence.normal, tester.controller().versionStatus().systemVersion().get().confidence()); tester.upgrader().maintain(); assertEquals("Upgrade of defaults are scheduled", 5, tester.buildSystem().jobs().size()); - assertEquals(version, ((Change.VersionChange)tester.application(default0.id()).deploying().get()).version()); - assertEquals(version, ((Change.VersionChange)tester.application(default1.id()).deploying().get()).version()); - assertEquals(version, ((Change.VersionChange)tester.application(default2.id()).deploying().get()).version()); - assertEquals(version, ((Change.VersionChange)tester.application(default3.id()).deploying().get()).version()); - assertEquals(version, ((Change.VersionChange)tester.application(default4.id()).deploying().get()).version()); + assertEquals(version55, ((Change.VersionChange)tester.application(default0.id()).deploying().get()).version()); + assertEquals(version54, ((Change.VersionChange)tester.application(default1.id()).deploying().get()).version()); + assertEquals(version54, ((Change.VersionChange)tester.application(default2.id()).deploying().get()).version()); + assertEquals(version54, ((Change.VersionChange)tester.application(default3.id()).deploying().get()).version()); + assertEquals(version54, ((Change.VersionChange)tester.application(default4.id()).deploying().get()).version()); + tester.completeUpgrade(default1, version54, "default"); + tester.completeUpgrade(default2, version54, "default"); + tester.completeUpgradeWithError(default3, version54, "default", DeploymentJobs.JobType.stagingTest); + tester.completeUpgradeWithError(default4, version54, "default", DeploymentJobs.JobType.productionUsWest1); // State: Default applications started upgrading to 5.5 - tester.completeUpgradeWithError(default0, version, "default", DeploymentJobs.JobType.stagingTest); - tester.completeUpgradeWithError(default1, version, "default", DeploymentJobs.JobType.stagingTest); - tester.completeUpgradeWithError(default2, version, "default", DeploymentJobs.JobType.stagingTest); - tester.completeUpgradeWithError(default3, version, "default", DeploymentJobs.JobType.productionUsWest1); - tester.completeUpgrade(default4, version, "default"); - tester.updateVersionStatus(version); + tester.upgrader().maintain(); + tester.completeUpgradeWithError(default0, version55, "default", DeploymentJobs.JobType.stagingTest); + tester.completeUpgradeWithError(default1, version55, "default", DeploymentJobs.JobType.stagingTest); + tester.completeUpgradeWithError(default2, version55, "default", DeploymentJobs.JobType.stagingTest); + tester.completeUpgradeWithError(default3, version55, "default", DeploymentJobs.JobType.productionUsWest1); + tester.completeUpgrade(default4, version55, "default"); + tester.updateVersionStatus(version55); assertEquals(VespaVersion.Confidence.broken, tester.controller().versionStatus().systemVersion().get().confidence()); tester.upgrader().maintain(); - assertEquals("Upgrade of defaults are scheduled on 5.4 instead, since 5.5 broken", - 3, tester.buildSystem().jobs().size()); - assertEquals("5.4", ((Change.VersionChange)tester.application(default1.id()).deploying().get()).version().toString()); - assertEquals("5.4", ((Change.VersionChange)tester.application(default2.id()).deploying().get()).version().toString()); + assertEquals("Upgrade of defaults are scheduled on 5.4 instead, since 5.5 broken: " + + "This is default3 since it failed upgrade on both 5.4 and 5.5", + 1, tester.buildSystem().jobs().size()); assertEquals("5.4", ((Change.VersionChange)tester.application(default3.id()).deploying().get()).version().toString()); } @@ -637,16 +641,6 @@ public class UpgraderTest { assertEquals(version, tester.application(default1.id()).deployedVersion().get()); assertEquals(version, tester.application(default2.id()).deployedVersion().get()); assertEquals(version, tester.application(default3.id()).deployedVersion().get()); - - // Over 12 hours pass and upgrade is rescheduled for 5th app - assertEquals(0, tester.buildSystem().jobs().size()); - tester.clock().advance(Duration.ofHours(12).plus(Duration.ofSeconds(1))); - tester.upgrader().maintain(); - assertEquals(1, tester.buildSystem().jobs().size()); - assertEquals("Upgrade is rescheduled", DeploymentJobs.JobType.systemTest.id(), - tester.buildSystem().jobs().get(0).jobName()); - tester.deployCompletely(default4, applicationPackage); - assertEquals(version, tester.application(default4.id()).deployedVersion().get()); } @Test diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json index fe9c373b7d5..a1bcc3eeb67 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json @@ -30,7 +30,7 @@ "gitCommit": "commit1" } }, - "reason": "component completed successfully in build 42", + "reason": "component completed", "at": "(ignore)" }, "lastCompleted": { @@ -44,7 +44,7 @@ "gitCommit": "commit1" } }, - "reason": "component completed successfully in build 42", + "reason": "component completed", "at": "(ignore)" }, "lastSuccess": { @@ -58,7 +58,7 @@ "gitCommit": "commit1" } }, - "reason": "component completed successfully in build 42", + "reason": "component completed", "at": "(ignore)" } }, @@ -76,7 +76,7 @@ "gitCommit": "commit1" } }, - "reason":"systemTest completed successfully in build 42", + "reason":"system-test completed", "at": "(ignore)" }, "lastCompleted": { @@ -90,7 +90,7 @@ "gitCommit": "commit1" } }, - "reason":"systemTest completed successfully in build 42", + "reason":"system-test completed", "at": "(ignore)" }, "lastSuccess": { @@ -104,7 +104,7 @@ "gitCommit": "commit1" } }, - "reason":"systemTest completed successfully in build 42", + "reason":"system-test completed", "at": "(ignore)" } }, @@ -122,7 +122,7 @@ "gitCommit": "commit1" } }, - "reason":"stagingTest completed successfully in build 42", + "reason":"staging-test completed", "at": "(ignore)" }, "lastCompleted": { @@ -136,7 +136,7 @@ "gitCommit": "commit1" } }, - "reason":"stagingTest completed successfully in build 42", + "reason":"staging-test completed", "at": "(ignore)" }, "lastSuccess": { @@ -150,7 +150,7 @@ "gitCommit": "commit1" } }, - "reason":"stagingTest completed successfully in build 42", + "reason":"staging-test completed", "at": "(ignore)" } }, @@ -168,7 +168,7 @@ "gitCommit": "commit1" } }, - "reason":"productionUsWest1 completed successfully in build 42", + "reason":"production-us-west-1 completed", "at": "(ignore)" }, "lastCompleted": { @@ -182,7 +182,7 @@ "gitCommit": "commit1" } }, - "reason":"productionUsWest1 completed successfully in build 42", + "reason":"production-us-west-1 completed", "at": "(ignore)" }, "lastSuccess": { @@ -196,7 +196,7 @@ "gitCommit": "commit1" } }, - "reason":"productionUsWest1 completed successfully in build 42", + "reason":"production-us-west-1 completed", "at": "(ignore)" } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json index 3dca8103ed7..f399d6c9188 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json @@ -63,7 +63,7 @@ "gitCommit": "commit1" } }, - "reason": "systemTest completed successfully in build 42", + "reason": "system-test completed", "at": "(ignore)" }, "lastCompleted": { @@ -77,7 +77,7 @@ "gitCommit": "commit1" } }, - "reason": "systemTest completed successfully in build 42", + "reason": "system-test completed", "at": "(ignore)" }, "lastSuccess": { @@ -91,7 +91,7 @@ "gitCommit": "commit1" } }, - "reason": "systemTest completed successfully in build 42", + "reason": "system-test completed", "at": "(ignore)" } }, @@ -109,7 +109,7 @@ "gitCommit": "commit1" } }, - "reason": "Retrying as build 42 just started failing", + "reason": "Immediate retry on failure", "at": "(ignore)" }, "lastCompleted": { @@ -123,7 +123,7 @@ "gitCommit": "commit1" } }, - "reason": "stagingTest completed successfully in build 42", + "reason": "staging-test completed", "at": "(ignore)" }, "firstFailing": { @@ -137,7 +137,7 @@ "gitCommit": "commit1" } }, - "reason": "stagingTest completed successfully in build 42", + "reason": "staging-test completed", "at": "(ignore)" } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java index 1638a2845ed..ee8ac5e3b8e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java @@ -96,14 +96,14 @@ public class ScrewdriverApiTest extends ControllerContainerTest { Response response; response = container.handleRequest(new Request("http://localhost:8080/screwdriver/v1/jobsToRun", "", Request.Method.GET)); - assertTrue("Response contains system-test", response.getBodyAsString().contains(JobType.systemTest.id())); - assertTrue("Response contains staging-test", response.getBodyAsString().contains(JobType.stagingTest.id())); + assertTrue("Response contains system-test", response.getBodyAsString().contains(JobType.systemTest.jobName())); + assertTrue("Response contains staging-test", response.getBodyAsString().contains(JobType.stagingTest.jobName())); assertEquals("Response contains only two items", 2, SlimeUtils.jsonToSlime(response.getBody()).get().entries()); // Check that GET didn't affect the enqueued jobs. response = container.handleRequest(new Request("http://localhost:8080/screwdriver/v1/jobsToRun", "", Request.Method.DELETE)); - assertTrue("Response contains system-test", response.getBodyAsString().contains(JobType.systemTest.id())); - assertTrue("Response contains staging-test", response.getBodyAsString().contains(JobType.stagingTest.id())); + assertTrue("Response contains system-test", response.getBodyAsString().contains(JobType.systemTest.jobName())); + assertTrue("Response contains staging-test", response.getBodyAsString().contains(JobType.stagingTest.jobName())); assertEquals("Response contains only two items", 2, SlimeUtils.jsonToSlime(response.getBody()).get().entries()); Thread.sleep(50); @@ -163,7 +163,7 @@ public class ScrewdriverApiTest extends ControllerContainerTest { assertResponse(new Request("http://localhost:8080/screwdriver/v1/trigger/tenant/" + app.id().tenant().value() + "/application/" + app.id().application().value(), "invalid".getBytes(StandardCharsets.UTF_8), Request.Method.POST), - 400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Unknown job id 'invalid'\"}"); + 400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Unknown job name 'invalid'\"}"); // component is triggered if no job is specified in request body assertResponse(new Request("http://localhost:8080/screwdriver/v1/trigger/tenant/" + @@ -172,7 +172,7 @@ public class ScrewdriverApiTest extends ControllerContainerTest { 200, "{\"message\":\"Triggered component for tenant1.application1\"}"); assertFalse(buildSystem.jobs().isEmpty()); - assertEquals(JobType.component.id(), buildSystem.jobs().get(0).jobName()); + assertEquals(JobType.component.jobName(), buildSystem.jobs().get(0).jobName()); assertEquals(1L, buildSystem.jobs().get(0).projectId()); buildSystem.takeJobsToRun(); @@ -182,7 +182,7 @@ public class ScrewdriverApiTest extends ControllerContainerTest { "staging-test".getBytes(StandardCharsets.UTF_8), Request.Method.POST), 200, "{\"message\":\"Triggered staging-test for tenant1.application1\"}"); assertFalse(buildSystem.jobs().isEmpty()); - assertEquals(JobType.stagingTest.id(), buildSystem.jobs().get(0).jobName()); + assertEquals(JobType.stagingTest.jobName(), buildSystem.jobs().get(0).jobName()); assertEquals(1L, buildSystem.jobs().get(0).projectId()); } @@ -197,14 +197,14 @@ public class ScrewdriverApiTest extends ControllerContainerTest { Optional<JobError> jobError) { return "{\n" + - " \"projectId\" : " + projectId + ",\n" + - " \"jobName\" :\"" + jobType.id() + "\",\n" + - " \"buildNumber\" : " + buildNumber + ",\n" + - jobError.map(message -> " \"jobError\" : \"" + message + "\",\n").orElse("") + - " \"tenant\" :\"" + applicationId.tenant().value() + "\",\n" + - " \"application\" :\"" + applicationId.application().value() + "\",\n" + - " \"instance\" :\"" + applicationId.instance().value() + "\"\n" + - "}"; + " \"projectId\" : " + projectId + ",\n" + + " \"jobName\" :\"" + jobType.jobName() + "\",\n" + + " \"buildNumber\" : " + buildNumber + ",\n" + + jobError.map(message -> " \"jobError\" : \"" + message + "\",\n").orElse("") + + " \"tenant\" :\"" + applicationId.tenant().value() + "\",\n" + + " \"application\" :\"" + applicationId.application().value() + "\",\n" + + " \"instance\" :\"" + applicationId.instance().value() + "\"\n" + + "}"; } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java index 1157863f009..f67863370fc 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java @@ -204,6 +204,7 @@ public class VersionStatusTest { // Another default application upgrades, raising confidence to high tester.completeUpgrade(default8, version2, "default"); + tester.completeUpgrade(default9, version2, "default"); tester.updateVersionStatus(); assertEquals("Confidence remains unchanged for version0: High", @@ -294,8 +295,8 @@ public class VersionStatusTest { Version versionWithUnknownTag = new Version("6.1.2"); Application app = tester.createAndDeploy("tenant1", "domain1","application1", Environment.test, 11); - applications.notifyJobCompletion(mockReport(app, component, true)); - applications.notifyJobCompletion(mockReport(app, systemTest, true)); + applications.notifyJobCompletion(DeploymentTester.jobReport(app, component, true)); + applications.notifyJobCompletion(DeploymentTester.jobReport(app, systemTest, true)); List<VespaVersion> vespaVersions = VersionStatus.compute(tester.controller()).versions(); @@ -312,14 +313,4 @@ public class VersionStatusTest { .orElseThrow(() -> new IllegalArgumentException("Expected to find version: " + version)); } - private DeploymentJobs.JobReport mockReport(Application application, DeploymentJobs.JobType jobType, boolean success) { - return new DeploymentJobs.JobReport( - application.id(), - jobType, - application.deploymentJobs().projectId().get(), - 42, - JobError.from(success) - ); - } - } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/application/ContainerThread.java b/jdisc_core/src/main/java/com/yahoo/jdisc/application/ContainerThread.java index 4506a63ac6d..1e947d44fcd 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/application/ContainerThread.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/application/ContainerThread.java @@ -22,8 +22,8 @@ public class ContainerThread extends Thread { * Allocates a new ContainerThread object. This constructor calls the parent {@link Thread#Thread(Runnable)} * constructor. * - * @param target The object whose <code>run</code> method is called. - * @param consumer The MetricConsumer of this thread. + * @param target the object whose <code>run</code> method is called. + * @param consumer the MetricConsumer of this thread. */ public ContainerThread(Runnable target, MetricConsumer consumer) { super(target); @@ -56,6 +56,7 @@ public class ContainerThread extends Thread { public Thread newThread(Runnable target) { return new ContainerThread(target, provider.get()); } + } } 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 509bf42d466..7feca14ef29 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 @@ -324,8 +324,8 @@ public class JettyHttpServer extends AbstractServerProvider { return bundleContext.getService(ref); } - private static ExecutorService newJanitor(final ThreadFactory factory) { - final int threadPoolSize = Runtime.getRuntime().availableProcessors(); + private static ExecutorService newJanitor(ThreadFactory factory) { + int threadPoolSize = Runtime.getRuntime().availableProcessors(); log.info("Creating janitor executor with " + threadPoolSize + " threads"); return Executors.newFixedThreadPool( threadPoolSize, @@ -544,6 +544,12 @@ <version>${commons-lang.version}</version> </dependency> <dependency> + <!-- This version is exported by jdisc via jcl-over-slf4j. --> + <groupId>commons-logging</groupId> + <artifactId>commons-logging</artifactId> + <version>1.1.1</version> + </dependency> + <dependency> <groupId>commons-net</groupId> <artifactId>commons-net</artifactId> <version>2.0</version> diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp index 816a434e56a..a6329db2aee 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp @@ -274,7 +274,8 @@ RemoveTask::run() const auto &attributes = _wc.getAttributes(); for (auto &attrp : attributes) { AttributeVector &attr = *attrp; - if (attr.getStatus().getLastSyncToken() < _serialNum) { + // Must use <= due to how move operations are handled + if (attr.getStatus().getLastSyncToken() <= _serialNum) { applyRemoveToAttribute(_serialNum, _lid, _immediateCommit, attr, _onWriteDone); } } diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/DaemonThreadFactory.java b/vespajlib/src/main/java/com/yahoo/concurrent/DaemonThreadFactory.java index 4c15b6e2365..6c5dd5e3ba5 100644 --- a/vespajlib/src/main/java/com/yahoo/concurrent/DaemonThreadFactory.java +++ b/vespajlib/src/main/java/com/yahoo/concurrent/DaemonThreadFactory.java @@ -45,4 +45,5 @@ public class DaemonThreadFactory implements ThreadFactory { } return t; } + } |