diff options
author | Jon Marius Venstad <jvenstad@yahoo-inc.com> | 2017-12-14 09:26:22 +0100 |
---|---|---|
committer | Jon Marius Venstad <jvenstad@yahoo-inc.com> | 2017-12-14 09:26:22 +0100 |
commit | a3fc6158428e18169ee379f405ee35181e71c443 (patch) | |
tree | 78220382451bd0dc048ca2c9148ebc435f9f8aba | |
parent | c403f41f013ca98726b4c34a1be1c6ec5924ec7f (diff) | |
parent | e494bf9f475d72f0a6f429e73dff03560f2c659f (diff) |
Conflict resolved
97 files changed, 999 insertions, 683 deletions
diff --git a/bootstrap-cpp.sh b/bootstrap-cpp.sh index 47d2a82622a..e6b4c816065 100755 --- a/bootstrap-cpp.sh +++ b/bootstrap-cpp.sh @@ -33,7 +33,7 @@ mkdir -p "${BUILD_DIR}" || { BUILD_DIR=$(realpath "${BUILD_DIR}") # Build it -source /opt/rh/devtoolset-6/enable || true +source /opt/rh/devtoolset-7/enable || true cd "${SOURCE_DIR}" bash ./bootstrap.sh full cd "${BUILD_DIR}" 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 990bce539ba..3f1c0046a85 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 @@ -11,12 +11,21 @@ import java.util.Set; /** * Interface for models towards filedistribution. * - * @author lulf - * @since 5.1 + * @author Ulf Lilleengen */ public interface FileDistribution { void sendDeployedFiles(String hostName, Set<FileReference> fileReferences); + + /** + * Notifies client which file references to download. Used to start downloading early (while + * preparing application package). + * + * @param hostName host which should be notified about file references to download + * @param fileReferences set of file references to start downloading + */ + void startDownload(String hostName, Set<FileReference> fileReferences); + void reloadDeployFileDistributor(); void removeDeploymentsThatHaveDifferentApplicationId(Collection<String> targetHostnames); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/DistributorCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/DistributorCluster.java index 81aca977400..0405f96cd89 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/DistributorCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/DistributorCluster.java @@ -1,8 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.content; -import com.yahoo.documentmodel.NewDocumentType; -import com.yahoo.vespa.config.content.core.BucketspacesConfig; import com.yahoo.vespa.config.content.core.StorDistributormanagerConfig; import com.yahoo.vespa.config.content.core.StorServerConfig; import com.yahoo.document.select.DocumentSelector; @@ -16,15 +14,13 @@ import org.w3c.dom.Element; import java.util.logging.Logger; - /** * Generates distributor-specific configuration. */ public class DistributorCluster extends AbstractConfigProducer<Distributor> implements StorDistributormanagerConfig.Producer, StorServerConfig.Producer, - MetricsmanagerConfig.Producer, - BucketspacesConfig.Producer { + MetricsmanagerConfig.Producer { public static final Logger log = Logger.getLogger(DistributorCluster.class.getPackage().toString()); @@ -151,20 +147,6 @@ public class DistributorCluster extends AbstractConfigProducer<Distributor> impl builder.is_distributor(true); } - private static final String DEFAULT_BUCKET_SPACE = "default"; - private static final String GLOBAL_BUCKET_SPACE = "global"; - - @Override - public void getConfig(BucketspacesConfig.Builder builder) { - for (NewDocumentType docType : parent.getDocumentDefinitions().values()) { - BucketspacesConfig.Documenttype.Builder docTypeBuilder = new BucketspacesConfig.Documenttype.Builder(); - docTypeBuilder.name(docType.getName()); - String bucketSpace = (parent.isGloballyDistributed(docType) ? GLOBAL_BUCKET_SPACE : DEFAULT_BUCKET_SPACE); - docTypeBuilder.bucketspace(bucketSpace); - builder.documenttype(docTypeBuilder); - } - } - public String getClusterName() { return parent.getName(); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java index 7889b857fff..7654fbc217b 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java @@ -7,12 +7,11 @@ import com.yahoo.config.model.ConfigModelContext; import com.yahoo.config.model.producer.AbstractConfigProducerRoot; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.RegionName; -import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.config.content.MessagetyperouteselectorpolicyConfig; import com.yahoo.vespa.config.content.FleetcontrollerConfig; import com.yahoo.vespa.config.content.StorDistributionConfig; +import com.yahoo.vespa.config.content.core.BucketspacesConfig; import com.yahoo.vespa.config.content.core.StorDistributormanagerConfig; import com.yahoo.documentmodel.NewDocumentType; import com.yahoo.documentapi.messagebus.protocol.DocumentProtocol; @@ -59,7 +58,8 @@ public class ContentCluster extends AbstractConfigProducer implements StorDistri StorDistributormanagerConfig.Producer, FleetcontrollerConfig.Producer, MetricsmanagerConfig.Producer, - MessagetyperouteselectorpolicyConfig.Producer { + MessagetyperouteselectorpolicyConfig.Producer, + BucketspacesConfig.Producer { // TODO: Make private private String documentSelection; @@ -694,4 +694,18 @@ public class ContentCluster extends AbstractConfigProducer implements StorDistri } } + + private static final String DEFAULT_BUCKET_SPACE = "default"; + private static final String GLOBAL_BUCKET_SPACE = "global"; + + @Override + public void getConfig(BucketspacesConfig.Builder builder) { + for (NewDocumentType docType : getDocumentDefinitions().values()) { + BucketspacesConfig.Documenttype.Builder docTypeBuilder = new BucketspacesConfig.Documenttype.Builder(); + docTypeBuilder.name(docType.getName()); + String bucketSpace = (isGloballyDistributed(docType) ? GLOBAL_BUCKET_SPACE : DEFAULT_BUCKET_SPACE); + docTypeBuilder.bucketspace(bucketSpace); + builder.documenttype(docTypeBuilder); + } + } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java index f6cc9203d00..8fcece3aa80 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java @@ -9,13 +9,10 @@ import com.yahoo.vespa.model.Host; import java.util.*; import java.util.stream.Collectors; -import static java.util.Arrays.asList; - - /** * Responsible for directing distribution of files to hosts. * - * @author tonytv + * @author Tony Vaagenes */ public class FileDistributor { @@ -99,6 +96,7 @@ public class FileDistributor { for (Host host : getTargetHosts()) { if ( ! host.getHostName().equals(fileSourceHost)) { dbHandler.sendDeployedFiles(host.getHostName(), filesToSendToHost(host)); + dbHandler.startDownload(host.getHostName(), filesToSendToHost(host)); } } dbHandler.sendDeployedFiles(fileSourceHost, allFilesToSend()); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentSearchClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentSearchClusterTest.java index 5f18b28d6ce..41bba055a50 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentSearchClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentSearchClusterTest.java @@ -1,9 +1,11 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.content; +import com.yahoo.vespa.config.content.core.BucketspacesConfig; import com.yahoo.vespa.config.search.core.ProtonConfig; import com.yahoo.vespa.model.content.cluster.ContentCluster; import com.yahoo.vespa.model.content.utils.ContentClusterBuilder; +import com.yahoo.vespa.model.content.utils.DocType; import com.yahoo.vespa.model.content.utils.SearchDefinitionBuilder; import org.junit.Test; @@ -15,6 +17,7 @@ import static com.yahoo.config.model.test.TestUtil.joinLines; import static com.yahoo.vespa.model.content.utils.ContentClusterUtils.createCluster; import static com.yahoo.vespa.model.content.utils.SearchDefinitionBuilder.createSearchDefinitions; import static junit.framework.TestCase.assertEquals; +import static junit.framework.TestCase.assertFalse; /** * Unit tests for content search cluster. @@ -36,8 +39,8 @@ public class ContentSearchClusterTest { private static ContentCluster createClusterWithGlobalType() throws Exception { return createCluster(new ContentClusterBuilder().docTypes(Arrays.asList( - new ContentClusterBuilder.DocType("global", true), - new ContentClusterBuilder.DocType("regular"))).getXml(), + DocType.indexGlobal("global"), + DocType.index("regular"))).getXml(), createSearchDefinitions("global", "regular")); } @@ -108,10 +111,31 @@ public class ContentSearchClusterTest { .content("field ref_to_c type reference<c> { indexing: attribute }").build()); searchDefinitions.add(new SearchDefinitionBuilder().name("c").build()); return createCluster(new ContentClusterBuilder().docTypes(Arrays.asList( - new ContentClusterBuilder.DocType("a"), - new ContentClusterBuilder.DocType("b", true), - new ContentClusterBuilder.DocType("c", true))).getXml(), + DocType.index("a"), + DocType.indexGlobal("b"), + DocType.indexGlobal("c"))).getXml(), searchDefinitions); } + private static BucketspacesConfig getBucketspacesConfig(ContentCluster cluster) { + BucketspacesConfig.Builder builder = new BucketspacesConfig.Builder(); + cluster.getConfig(builder); + return new BucketspacesConfig(builder); + } + + private static void assertDocumentType(String expName, String expBucketSpace, BucketspacesConfig.Documenttype docType) { + assertEquals(expName, docType.name()); + assertEquals(expBucketSpace, docType.bucketspace()); + } + + @Test + public void require_that_bucket_spaces_config_is_produced_for_content_cluster() throws Exception { + BucketspacesConfig config = getBucketspacesConfig(createClusterWithGlobalType()); + assertEquals(2, config.documenttype().size()); + assertDocumentType("global", "global", config.documenttype(0)); + assertDocumentType("regular", "default", config.documenttype(1)); + // Safeguard against flipping the switch + assertFalse(config.enable_multiple_bucket_spaces()); + } + } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/DistributorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/DistributorTest.java index d4e804d3f95..48b7ccdad6b 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/DistributorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/DistributorTest.java @@ -1,23 +1,22 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.content; -import com.yahoo.vespa.config.content.core.BucketspacesConfig; import com.yahoo.vespa.config.content.core.StorCommunicationmanagerConfig; import com.yahoo.vespa.config.content.core.StorDistributormanagerConfig; import com.yahoo.vespa.config.content.core.StorServerConfig; import com.yahoo.config.model.test.MockRoot; import com.yahoo.vespa.model.content.cluster.ContentCluster; import com.yahoo.vespa.model.content.utils.ContentClusterUtils; +import com.yahoo.vespa.model.content.utils.DocType; import com.yahoo.vespa.model.test.utils.ApplicationPackageUtils; import org.junit.Test; -import java.util.Arrays; import java.util.List; -import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; import static org.hamcrest.Matchers.*; + /** * Test for content DistributorCluster. */ @@ -303,100 +302,48 @@ public class DistributorTest { return new StorDistributormanagerConfig(builder); } - private static class DocDef { - public final String type; - public final String mode; - public final boolean global; - - private DocDef(String type, String mode, boolean global) { - this.type = type; - this.mode = mode; - this.global = global; - } - - public static DocDef storeOnly(String type) { - return new DocDef(type, "store-only", false); - } - - public static DocDef index(String type) { - return new DocDef(type, "index", false); - } - - public static DocDef indexGlobal(String type) { - return new DocDef(type, "index", true); - } - - public static DocDef streaming(String type) { - return new DocDef(type, "streaming", false); - } - } - - private String generateXmlForDocDefs(DocDef... defs) { + private String generateXmlForDocTypes(DocType... docTypes) { return "<content id='storage'>\n" + - " <documents>\n" + - Arrays.stream(defs) - .map(def -> String.format(" <document type='%s' mode='%s' global='%s'/>", - def.type, def.mode, (def.global ? "true" : "false"))) - .collect(Collectors.joining("\n")) + - "\n </documents>\n" + - "</content>"; + DocType.listToXml(docTypes) + + "\n</content>"; } @Test public void bucket_activation_disabled_if_no_documents_in_indexed_mode() { StorDistributormanagerConfig config = clusterXmlToConfig( - generateXmlForDocDefs(DocDef.storeOnly("music"))); + generateXmlForDocTypes(DocType.storeOnly("music"))); assertThat(config.disable_bucket_activation(), is(true)); } @Test public void bucket_activation_enabled_with_single_indexed_document() { StorDistributormanagerConfig config = clusterXmlToConfig( - generateXmlForDocDefs(DocDef.index("music"))); + generateXmlForDocTypes(DocType.index("music"))); assertThat(config.disable_bucket_activation(), is(false)); } @Test public void bucket_activation_enabled_with_multiple_indexed_documents() { StorDistributormanagerConfig config = clusterXmlToConfig( - generateXmlForDocDefs(DocDef.index("music"), - DocDef.index("movies"))); + generateXmlForDocTypes(DocType.index("music"), + DocType.index("movies"))); assertThat(config.disable_bucket_activation(), is(false)); } @Test public void bucket_activation_enabled_if_at_least_one_document_indexed() { StorDistributormanagerConfig config = clusterXmlToConfig( - generateXmlForDocDefs(DocDef.storeOnly("music"), - DocDef.streaming("bunnies"), - DocDef.index("movies"))); + generateXmlForDocTypes(DocType.storeOnly("music"), + DocType.streaming("bunnies"), + DocType.index("movies"))); assertThat(config.disable_bucket_activation(), is(false)); } @Test public void bucket_activation_disabled_for_single_streaming_type() { StorDistributormanagerConfig config = clusterXmlToConfig( - generateXmlForDocDefs(DocDef.streaming("music"))); + generateXmlForDocTypes(DocType.streaming("music"))); assertThat(config.disable_bucket_activation(), is(true)); } - private BucketspacesConfig clusterXmlToBucketspacesConfig(String xml) { - BucketspacesConfig.Builder builder = new BucketspacesConfig.Builder(); - parse(xml).getConfig(builder); - return new BucketspacesConfig(builder); - } - - private void assertDocumentType(String expName, String expBucketSpace, BucketspacesConfig.Documenttype docType) { - assertEquals(expName, docType.name()); - assertEquals(expBucketSpace, docType.bucketspace()); - } - - @Test - public void bucket_spaces_config_is_produced_for_distributor_cluster() { - BucketspacesConfig config = clusterXmlToBucketspacesConfig( - generateXmlForDocDefs(DocDef.index("music"), DocDef.indexGlobal("movies"))); - assertEquals(2, config.documenttype().size()); - assertDocumentType("movies", "global", config.documenttype(0)); - assertDocumentType("music", "default", config.documenttype(1)); - } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterBuilder.java b/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterBuilder.java index 592e90efd22..95c57bb544c 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterBuilder.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterBuilder.java @@ -18,29 +18,10 @@ import static com.yahoo.config.model.test.TestUtil.joinLines; */ public class ContentClusterBuilder { - public static class DocType { - private final String name; - private final boolean global; - - public DocType(String name, boolean global) { - this.name = name; - this.global = global; - } - - public DocType(String name) { - this(name, false); - } - - public String toXml() { - return (global ? "<document mode='index' type='" + name + "' global='true'/>" : - "<document mode='index' type='" + name + "'/>"); - } - } - private String name = "mycluster"; private int redundancy = 1; private int searchableCopies = 1; - private List<DocType> docTypes = Arrays.asList(new DocType("test")); + private List<DocType> docTypes = Arrays.asList(DocType.index("test")); private String groupXml = getSimpleGroupXml(); private Optional<String> dispatchXml = Optional.empty(); private Optional<Double> protonDiskLimit = Optional.empty(); @@ -66,7 +47,7 @@ public class ContentClusterBuilder { public ContentClusterBuilder docTypes(String ... docTypes) { this.docTypes = Arrays.asList(docTypes).stream(). - map(type -> new DocType(type)). + map(type -> DocType.index(type)). collect(Collectors.toList()); return this; } @@ -103,9 +84,7 @@ public class ContentClusterBuilder { public String getXml() { String xml = joinLines("<content version='1.0' id='" + name + "'>", " <redundancy>" + redundancy + "</redundancy>", - " <documents>", - docTypes.stream().map(DocType::toXml).collect(Collectors.joining("\n")), - " </documents>", + DocType.listToXml(docTypes), " <engine>", " <proton>", " <searchable-copies>" + searchableCopies + "</searchable-copies>", diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/utils/DocType.java b/config-model/src/test/java/com/yahoo/vespa/model/content/utils/DocType.java new file mode 100644 index 00000000000..3a5f679509b --- /dev/null +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/utils/DocType.java @@ -0,0 +1,54 @@ +package com.yahoo.vespa.model.content.utils; + +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +/** + * Definition of a document type used for testing. + * + * @author geirst + */ +public class DocType { + private final String type; + private final String mode; + private final boolean global; + + private DocType(String type, String mode, boolean global) { + this.type = type; + this.mode = mode; + this.global = global; + } + + public String toXml() { + return (global ? "<document mode='" + mode + "' type='" + type + "' global='true'/>" : + "<document mode='" + mode + "' type='" + type + "'/>"); + } + + public static DocType storeOnly(String type) { + return new DocType(type, "store-only", false); + } + + public static DocType index(String type) { + return new DocType(type, "index", false); + } + + public static DocType indexGlobal(String type) { + return new DocType(type, "index", true); + } + + public static DocType streaming(String type) { + return new DocType(type, "streaming", false); + } + + public static String listToXml(DocType... docTypes) { + return listToXml(Arrays.asList(docTypes)); + } + + public static String listToXml(List<DocType> docTypes) { + return "<documents>\n" + + docTypes.stream().map(DocType::toXml).collect(Collectors.joining("\n")) + "\n" + + "</documents>"; + } + +} diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Deployer.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Deployer.java index 77faa45ebe5..111073aca77 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Deployer.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Deployer.java @@ -11,8 +11,20 @@ import java.util.Optional; */ public interface Deployer { + + /** + * Creates a new deployment from the active application, if available. Will use the default timeout for deployment. + * + * @param application the active application to be redeployed + * @return a new deployment from the local active, or empty if a local active application + * was not present for this id (meaning it either is not active or active on another + * node in the config server cluster) + */ + Optional<Deployment> deployFromLocalActive(ApplicationId application); + /** - * Creates a new deployment from the active application, if available. + * Creates a new deployment from the active application, if available. Prefer {@link #deployFromLocalActive(ApplicationId)} + * if possible, this method is for testing and will override the default timeout for deployment. * * @param application the active application to be redeployed * @param timeout the timeout to use for each individual deployment operation diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ClientUpdater.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ClientUpdater.java index a4d5fd16990..0ed5d04e36e 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ClientUpdater.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ClientUpdater.java @@ -21,9 +21,7 @@ class ClientUpdater { private final RpcServer rpcServer; private final DelayedResponses delayedResponses; - ClientUpdater(RpcServer rpcServer, - ConfigProxyStatistics statistics, - DelayedResponses delayedResponses) { + ClientUpdater(RpcServer rpcServer, ConfigProxyStatistics statistics, DelayedResponses delayedResponses) { this.rpcServer = rpcServer; this.statistics = statistics; this.delayedResponses = delayedResponses; @@ -37,28 +35,20 @@ class ClientUpdater { * @param config new config */ void updateSubscribers(RawConfig config) { - if (log.isLoggable(LogLevel.DEBUG)) { - log.log(LogLevel.DEBUG, "Config updated for " + config.getKey() + "," + config.getGeneration()); - } + log.log(LogLevel.DEBUG, () -> "Config updated for " + config.getKey() + "," + config.getGeneration()); sendResponse(config); } private void sendResponse(RawConfig config) { if (config.isError()) { statistics.incErrorCount(); } - if (log.isLoggable(LogLevel.DEBUG)) { - log.log(LogLevel.DEBUG, "Sending response for " + config.getKey() + "," + config.getGeneration()); - } + log.log(LogLevel.DEBUG, () -> "Sending response for " + config.getKey() + "," + config.getGeneration()); DelayQueue<DelayedResponse> responseDelayQueue = delayedResponses.responses(); - if (log.isLoggable(LogLevel.SPAM)) { - log.log(LogLevel.SPAM, "Delayed response queue: " + responseDelayQueue); - } + log.log(LogLevel.SPAM, () -> "Delayed response queue: " + responseDelayQueue); if (responseDelayQueue.size() == 0) { - log.log(LogLevel.DEBUG, "There exists no matching element on delayed response queue for " + config.getKey()); + log.log(LogLevel.DEBUG, () -> "There exists no matching element on delayed response queue for " + config.getKey()); return; } else { - if (log.isLoggable(LogLevel.DEBUG)) { - log.log(LogLevel.DEBUG, "Delayed response queue has " + responseDelayQueue.size() + " elements"); - } + log.log(LogLevel.DEBUG, () -> "Delayed response queue has " + responseDelayQueue.size() + " elements"); } DelayedResponse[] responses = responseDelayQueue.toArray(new DelayedResponse[0]); boolean found = false; @@ -67,21 +57,17 @@ class ClientUpdater { if (request.getConfigKey().equals(config.getKey())) { if (delayedResponses.remove(response)) { found = true; - if (log.isLoggable(LogLevel.DEBUG)) { - log.log(LogLevel.DEBUG, "Call returnOkResponse for " + config.getKey() + "," + config.getGeneration()); - } + log.log(LogLevel.DEBUG, () -> "Call returnOkResponse for " + config.getKey() + "," + config.getGeneration()); rpcServer.returnOkResponse(request, config); } else { - log.log(LogLevel.INFO, "Could not remove " + config.getKey() + " from delayed delayedResponses queue, already removed"); + log.log(LogLevel.INFO, "Could not remove " + config.getKey() + " from delayedResponses queue, already removed"); } } } if (!found) { - log.log(LogLevel.DEBUG, "Found no recipient for " + config.getKey() + " in delayed response queue"); - } - if (log.isLoggable(LogLevel.DEBUG)) { - log.log(LogLevel.DEBUG, "Finished updating config for " + config.getKey() + "," + config.getGeneration()); + log.log(LogLevel.DEBUG, () -> "Found no recipient for " + config.getKey() + " in delayed response queue"); } + log.log(LogLevel.DEBUG, () -> "Finished updating config for " + config.getKey() + "," + config.getGeneration()); } } 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 fe5976e9c1f..68985bd598a 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 @@ -20,7 +20,6 @@ import java.util.logging.Logger; * * @author hmusum */ -// TODO: Rename now that it also support file distribution request public class ConfigProxyRpcServer implements Runnable, TargetWatcher, RpcServer { private final static Logger log = Logger.getLogger(ConfigProxyRpcServer.class.getName()); diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCache.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCache.java index 7fd173ab98d..7914b7a80b6 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCache.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCache.java @@ -19,7 +19,6 @@ import java.util.logging.Logger; /** * @author hmusum - * @since 5.1.9 */ public class MemoryCache { private static final Logger log = Logger.getLogger(MemoryCache.class.getName()); @@ -41,9 +40,7 @@ public class MemoryCache { public void put(RawConfig config) { if (config.isError()) return; - if (log.isLoggable(LogLevel.DEBUG)) { - log.log(LogLevel.DEBUG, "Putting '" + config + "' into memory cache"); - } + log.log(LogLevel.DEBUG, () -> "Putting '" + config + "' into memory cache"); cache.put(new ConfigCacheKey(config.getKey(), config.getDefMd5()), config); } diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCacheConfigClient.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCacheConfigClient.java index a1c07b91155..4b6e9172f8b 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCacheConfigClient.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCacheConfigClient.java @@ -10,8 +10,9 @@ import java.util.List; import java.util.logging.Logger; /** + * The client used for getting config when running in 'memorycache' mode. + * * @author hmusum - * @since 5.1.10 */ class MemoryCacheConfigClient implements ConfigSourceClient { @@ -30,11 +31,11 @@ class MemoryCacheConfigClient implements ConfigSourceClient { */ @Override public RawConfig getConfig(RawConfig input, JRTServerConfigRequest request) { - log.log(LogLevel.DEBUG, "Getting config from cache"); + log.log(LogLevel.DEBUG, () -> "Getting config from cache"); ConfigKey<?> key = input.getKey(); RawConfig cached = cache.get(new ConfigCacheKey(key, input.getDefMd5())); if (cached != null) { - log.log(LogLevel.DEBUG, "Found config " + key + " in cache"); + log.log(LogLevel.DEBUG, () -> "Found config " + key + " in cache"); return cached; } else { return null; diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java index d0095121c8e..0f80f228b36 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java @@ -23,7 +23,6 @@ import java.util.logging.Logger; * An Rpc client to a config source * * @author hmusum - * @since 5.1.9 */ class RpcConfigSourceClient implements ConfigSourceClient { @@ -83,7 +82,7 @@ class RpcConfigSourceClient implements ConfigSourceClient { Target target = supervisor.connect(spec); target.invokeSync(req, 30.0); if (target.isValid()) { - log.log(LogLevel.DEBUG, "Created connection to config source at " + spec.toString()); + log.log(LogLevel.DEBUG, () -> "Created connection to config source at " + spec.toString()); return; } else { log.log(LogLevel.INFO, "Could not connect to config source at " + spec.toString()); @@ -123,13 +122,11 @@ class RpcConfigSourceClient implements ConfigSourceClient { RawConfig ret = null; if (cachedConfig != null) { - log.log(LogLevel.DEBUG, "Found config " + configCacheKey + " in cache, generation=" + cachedConfig.getGeneration() + + log.log(LogLevel.DEBUG, () -> "Found config " + configCacheKey + " in cache, generation=" + cachedConfig.getGeneration() + ",configmd5=" + cachedConfig.getConfigMd5()); - if (log.isLoggable(LogLevel.SPAM)) { - log.log(LogLevel.SPAM, "input config=" + input + ",cached config=" + cachedConfig); - } + log.log(LogLevel.SPAM, () -> "input config=" + input + ",cached config=" + cachedConfig); if (ProxyServer.configOrGenerationHasChanged(cachedConfig, request)) { - log.log(LogLevel.SPAM, "Cached config is not equal to requested, will return it"); + log.log(LogLevel.SPAM, () -> "Cached config is not equal to requested, will return it"); if (delayedResponses.remove(delayedResponse)) { // unless another thread already did it ret = cachedConfig; @@ -148,9 +145,9 @@ class RpcConfigSourceClient implements ConfigSourceClient { private void subscribeToConfig(RawConfig input, ConfigCacheKey configCacheKey) { synchronized (activeSubscribersLock) { if (activeSubscribers.containsKey(configCacheKey)) { - log.log(LogLevel.DEBUG, "Already a subscriber running for: " + configCacheKey); + log.log(LogLevel.DEBUG, () -> "Already a subscriber running for: " + configCacheKey); } else { - log.log(LogLevel.DEBUG, "Could not find good config in cache, creating subscriber for: " + configCacheKey); + log.log(LogLevel.DEBUG, () -> "Could not find good config in cache, creating subscriber for: " + configCacheKey); UpstreamConfigSubscriber subscriber = new UpstreamConfigSubscriber(input, clientUpdater, configSourceSet, timingValues, requesterPool, memoryCache); try { subscriber.subscribe(); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index 13401d4a4a6..a4dd943aa47 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -112,6 +112,18 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye * Creates a new deployment from the active application, if available. * * @param application the active application to be redeployed + * @return a new deployment from the local active, or empty if a local active application + * was not present for this id (meaning it either is not active or active on another + * node in the config server cluster) + */ + public Optional<com.yahoo.config.provision.Deployment> deployFromLocalActive(ApplicationId application) { + return deployFromLocalActive(application, Duration.ofSeconds(configserverConfig.zookeeper().barrierTimeout()).plus(Duration.ofSeconds(5))); + } + + /** + * Creates a new deployment from the active application, if available. + * + * @param application the active application to be redeployed * @param timeout the timeout to use for each individual deployment operation * @return a new deployment from the local active, or empty if a local active application * was not present for this id (meaning it either is not active or active on another @@ -352,7 +364,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye private void redeployApplication(ApplicationId applicationId, Deployer deployer, ExecutorService deploymentExecutor) { log.log(LogLevel.DEBUG, () -> "Redeploying " + applicationId); - deployer.deployFromLocalActive(applicationId, Duration.ofMinutes(30)) + deployer.deployFromLocalActive(applicationId) .ifPresent(deployment -> deploymentExecutor.execute(() -> { try { deployment.activate(); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/CombinedLegacyDistribution.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/CombinedLegacyDistribution.java index 588f2d1d63f..f0e64a936a5 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/CombinedLegacyDistribution.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/CombinedLegacyDistribution.java @@ -3,22 +3,43 @@ package com.yahoo.vespa.config.server.filedistribution; import com.yahoo.config.FileReference; import com.yahoo.config.model.api.FileDistribution; +import com.yahoo.jrt.Request; +import com.yahoo.jrt.Spec; +import com.yahoo.jrt.StringArray; +import com.yahoo.jrt.Supervisor; +import com.yahoo.jrt.Target; +import com.yahoo.jrt.Transport; +import com.yahoo.log.LogLevel; import java.util.Collection; import java.util.Set; +import java.util.logging.Logger; +/** + * @author baldersheim + */ public class CombinedLegacyDistribution implements FileDistribution { + private final static Logger log = Logger.getLogger(CombinedLegacyDistribution.class.getName()); + + private final Supervisor supervisor = new Supervisor(new Transport()); private final FileDistribution legacy; CombinedLegacyDistribution(FileDBHandler legacy) { this.legacy = legacy; } + @Override public void sendDeployedFiles(String hostName, Set<FileReference> fileReferences) { legacy.sendDeployedFiles(hostName, fileReferences); } @Override + public void startDownload(String hostName, Set<FileReference> fileReferences) { + // TODO: Not active for now + // startDownloadingFileReferences(hostName, fileReferences); + } + + @Override public void reloadDeployFileDistributor() { legacy.reloadDeployFileDistributor(); } @@ -27,4 +48,18 @@ public class CombinedLegacyDistribution implements FileDistribution { public void removeDeploymentsThatHaveDifferentApplicationId(Collection<String> targetHostnames) { legacy.removeDeploymentsThatHaveDifferentApplicationId(targetHostnames); } + + // Notifies config proxy which file references it should start downloading. It's OK if the call does not succeed, + // as downloading will then start synchronously when a service requests a file reference instead + private void startDownloadingFileReferences(String hostName, Set<FileReference> fileReferences) { + Target target = supervisor.connect(new Spec(hostName, 19090)); + double timeout = 0.1; + Request request = new Request("filedistribution.setFileReferencesToDownload"); + request.parameters().add(new StringArray(fileReferences.stream().map(FileReference::value).toArray(String[]::new))); + log.log(LogLevel.INFO, "Executing " + request.methodName() + " against " + target.toString()); + target.invokeSync(request, timeout); + if (request.isError()) { + log.log(LogLevel.INFO, request.methodName() + " failed: " + request.errorCode() + " (" + request.errorMessage() + ")"); + } + } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBHandler.java index f0ce6104496..9b3f4c39a45 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBHandler.java @@ -10,9 +10,8 @@ import java.util.*; /** * Implements invoker of filedistribution using manager with JNI. * - * @author tonytv - * @author lulf - * @since 5.1.14 + * @author Tony Vaagenes + * @author Ulf Lilleengen */ public class FileDBHandler implements FileDistribution { private final FileDistributionManager manager; @@ -31,6 +30,11 @@ public class FileDBHandler implements FileDistribution { } @Override + public void startDownload(String hostName, Set<FileReference> fileReferences) { + throw new UnsupportedOperationException("Not valid for this Filedistribution implementation"); + } + + @Override public void removeDeploymentsThatHaveDifferentApplicationId(Collection<String> targetHostnames) { manager.removeDeploymentsThatHaveDifferentApplicationId(targetHostnames); } 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 index 81f5e62016a..a9c56cf99d0 100644 --- 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 @@ -6,12 +6,12 @@ import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.config.FileReference; import com.yahoo.config.model.api.FileDistribution; 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.StringValue; import com.yahoo.jrt.Supervisor; import com.yahoo.jrt.Transport; +import com.yahoo.log.LogLevel; import com.yahoo.net.HostName; import com.yahoo.vespa.config.Connection; import com.yahoo.vespa.config.ConnectionPool; @@ -108,8 +108,7 @@ public class FileServer { private void serveFile(FileReference reference, Receiver target) { File file = root.getFile(reference); - // TODO remove once verified in system tests. - log.info("Start serving reference '" + reference.value() + "' with file '" + file.getAbsolutePath() + "'"); + log.log(LogLevel.DEBUG, "Start serving reference '" + reference.value() + "' with file '" + file.getAbsolutePath() + "'"); boolean success = false; String errorDescription = "OK"; FileReferenceData fileData = FileReferenceDataBlob.empty(reference, file.getName()); @@ -122,8 +121,7 @@ public class FileServer { } target.receive(fileData, new ReplayStatus(success ? 0 : 1, success ? "OK" : errorDescription)); - // TODO remove once verified in system tests. - log.info("Done serving reference '" + reference.toString() + "' with file '" + file.getAbsolutePath() + "'"); + log.log(LogLevel.DEBUG, "Done serving reference '" + reference.toString() + "' with file '" + file.getAbsolutePath() + "'"); } private FileReferenceData readFileReferenceData(FileReference reference) throws IOException { @@ -143,8 +141,7 @@ public class FileServer { private void serveFile(String fileReference, Request request, Receiver receiver) { FileApiErrorCodes result; try { - // TODO remove once verified in system tests. - log.info("Received request for reference '" + fileReference + "'"); + log.log(LogLevel.DEBUG, "Received request for reference '" + fileReference + "'"); result = hasFile(fileReference) ? FileApiErrorCodes.OK : FileApiErrorCodes.NOT_FOUND; diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileDBHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileDBHandler.java index eb23d38e23e..caf64cca4d0 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileDBHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileDBHandler.java @@ -8,8 +8,7 @@ import java.util.Collection; import java.util.Set; /** - * @author lulf - * @since 5.1 + * @author Ulf Lilleengen */ public class MockFileDBHandler implements FileDistribution { public int sendDeployedFilesCalled = 0; @@ -22,6 +21,9 @@ public class MockFileDBHandler implements FileDistribution { } @Override + public void startDownload(String hostName, Set<FileReference> fileReferences) { /* not implemented */ } + + @Override public void reloadDeployFileDistributor() { reloadDeployFileDistributorCalled++; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java index 384ae0853d8..5d573323bb6 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java @@ -29,7 +29,6 @@ import java.io.File; import java.io.FileReader; import java.time.Clock; import java.util.ArrayList; -import java.util.Optional; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertFalse; @@ -71,7 +70,7 @@ public class ConfigServerBootstrapTest extends TestWithTenant { VersionState versionState = new VersionState(versionFile); assertTrue(versionState.isUpgraded()); ConfigServerBootstrap bootstrap = - new ConfigServerBootstrap(applicationRepository, rpc, (application, timeout) -> Optional.empty(), versionState, + new ConfigServerBootstrap(applicationRepository, rpc, new MockDeployer(), versionState, new StateMonitor(new HealthMonitorConfig(new HealthMonitorConfig.Builder()), new SystemTimer())); waitUntilStarted(rpc, 60000); assertFalse(versionState.isUpgraded()); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/MockDeployer.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/MockDeployer.java index 4fc54943a79..051e7c9a8f9 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/MockDeployer.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/MockDeployer.java @@ -15,6 +15,11 @@ public class MockDeployer implements com.yahoo.config.provision.Deployer { public ApplicationId lastDeployed; @Override + public Optional<Deployment> deployFromLocalActive(ApplicationId application) { + return deployFromLocalActive(application, Duration.ofSeconds(60)); + } + + @Override public Optional<Deployment> deployFromLocalActive(ApplicationId application, Duration timeout) { lastDeployed = application; return Optional.empty(); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index b025a522580..28fa311b841 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -21,6 +21,10 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.identifiers.Hostname; import com.yahoo.vespa.hosted.controller.api.identifiers.RevisionId; import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; +import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory; +import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken; +import com.yahoo.vespa.hosted.controller.api.integration.athenz.ZmsClient; +import com.yahoo.vespa.hosted.controller.api.integration.athenz.ZmsException; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerClient; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log; import com.yahoo.vespa.hosted.controller.api.integration.configserver.NoInstanceException; @@ -41,10 +45,6 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobReport; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.application.SourceRevision; -import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory; -import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken; -import com.yahoo.vespa.hosted.controller.api.integration.athenz.ZmsClient; -import com.yahoo.vespa.hosted.controller.api.integration.athenz.ZmsException; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.maintenance.DeploymentExpirer; import com.yahoo.vespa.hosted.controller.persistence.ControllerDb; @@ -270,10 +270,11 @@ public class ApplicationController { public ActivateResult deployApplication(ApplicationId applicationId, ZoneId zone, ApplicationPackage applicationPackage, DeployOptions options) { try (Lock lock = lock(applicationId)) { - // TODO: Shouldn't this go through the above method? Seems you can cheat the checks here ... ? - LockedApplication application = get(applicationId).map(application1 -> new LockedApplication(application1, lock)).orElse(new LockedApplication( - new Application(applicationId), lock) - ); + // Not ideal, but since we create on missing and return a result computed inside the lock, + // the lock-with-action methods cannot be used + LockedApplication application = get(applicationId) + .map(app -> new LockedApplication(app, lock)) + .orElseGet(() -> new LockedApplication(new Application(applicationId), lock)); // Determine what we are doing Version version; @@ -504,31 +505,43 @@ public class ApplicationController { } /** - * Deletes the application with this id + * Deletes the the given application. All known instances of the applications will be deleted, + * including PR instances. * * @throws IllegalArgumentException if the application has deployments or the caller is not authorized - * @throws NotExistsException if the application does not exist + * @throws NotExistsException if no instances of the application exist */ - public void deleteApplication(ApplicationId id, Optional<NToken> token) { - if ( ! controller.applications().get(id).isPresent()) - throw new NotExistsException("Could not delete application '" + id + "': Application not found"); + public void deleteApplication(ApplicationId applicationId, Optional<NToken> token) { + // Find all instances of the application + List<ApplicationId> instances = controller.applications().asList(applicationId.tenant()) + .stream() + .map(Application::id) + .filter(id -> id.application().equals(applicationId.application()) && + id.tenant().equals(applicationId.tenant())) + .collect(Collectors.toList()); + if (instances.isEmpty()) { + throw new NotExistsException("Could not delete application '" + applicationId + "': Application not found"); + } - lockOrThrow(id, application -> { + // TODO: Make this one transaction when database is moved to ZooKeeper + instances.forEach(id -> lockOrThrow(id, application -> { if ( ! application.deployments().isEmpty()) throw new IllegalArgumentException("Could not delete '" + application + "': It has active deployments"); - + Tenant tenant = controller.tenants().tenant(new TenantId(id.tenant().value())).get(); if (tenant.isAthensTenant() && ! token.isPresent()) throw new IllegalArgumentException("Could not delete '" + application + "': No NToken provided"); - // NB: Next 2 lines should have been one transaction - if (tenant.isAthensTenant()) + // Only delete in Athenz once + if (id.instance().isDefault() && tenant.isAthensTenant()) { zmsClientFactory.createZmsClientWithAuthorizedServiceToken(token.get()) - .deleteApplication(tenant.getAthensDomain().get(), new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId(id.application().value())); + .deleteApplication(tenant.getAthensDomain().get(), + new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId(id.application().value())); + } db.deleteApplication(id); log.info("Deleted " + application); - }); + })); } /** diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzTrustStoreConfigurator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzTrustStoreConfigurator.java new file mode 100644 index 00000000000..939a5667a36 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzTrustStoreConfigurator.java @@ -0,0 +1,45 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.athenz.filter; + +import com.google.inject.Inject; +import com.yahoo.jdisc.http.ssl.SslTrustStoreConfigurator; +import com.yahoo.jdisc.http.ssl.SslTrustStoreContext; +import com.yahoo.vespa.hosted.controller.athenz.config.AthenzConfig; + +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.NoSuchAlgorithmException; +import java.security.cert.CertificateException; + +/** + * Load trust store with Athenz CA certificates + * + * @author bjorncs + */ +public class AthenzTrustStoreConfigurator implements SslTrustStoreConfigurator { + + private final KeyStore trustStore; + + @Inject + public AthenzTrustStoreConfigurator(AthenzConfig config) { + this.trustStore = createTrustStore(new File(config.athenzCaTrustStore())); + } + + private static KeyStore createTrustStore(File trustStoreFile) { + try (FileInputStream in = new FileInputStream(trustStoreFile)) { + KeyStore trustStore = KeyStore.getInstance("JKS"); + trustStore.load(in, "changeit".toCharArray()); + return trustStore; + } catch (IOException | CertificateException | NoSuchAlgorithmException | KeyStoreException e) { + throw new RuntimeException(e); + } + } + + @Override + public void configure(SslTrustStoreContext context) { + context.updateTrustStore(trustStore); + } +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzSslContextProviderImpl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzSslContextProviderImpl.java index ab653f48388..3a7a72ac8ae 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzSslContextProviderImpl.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzSslContextProviderImpl.java @@ -58,14 +58,15 @@ public class AthenzSslContextProviderImpl implements AthenzSslContextProvider { try { AthenzIdentityCertificate identityCertificate = ztsClient.getIdentityCertificate(); KeyStore keyStore = KeyStore.getInstance("JKS"); + keyStore.load(null); keyStore.setKeyEntry("athenz-controller-key", identityCertificate.getPrivateKey(), new char[0], new Certificate[]{identityCertificate.getCertificate()}); - KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance("X509"); + KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); keyManagerFactory.init(keyStore, new char[0]); return keyManagerFactory.getKeyManagers(); - } catch (KeyStoreException | NoSuchAlgorithmException | UnrecoverableKeyException e) { + } catch (KeyStoreException | NoSuchAlgorithmException | UnrecoverableKeyException | CertificateException | IOException e) { throw new RuntimeException(e); } } @@ -76,7 +77,7 @@ public class AthenzSslContextProviderImpl implements AthenzSslContextProvider { try (FileInputStream in = new FileInputStream(config.athenzCaTrustStore())) { trustStore.load(in, "changeit".toCharArray()); } - TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance("X509"); + TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); trustManagerFactory.init(trustStore); return trustManagerFactory.getTrustManagers(); } catch (CertificateException | IOException | KeyStoreException | NoSuchAlgorithmException e) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 5e0514de949..a7d072d1dae 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 @@ -379,10 +379,10 @@ public class ApplicationApiHandler extends LoggingRequestHandler { // Rotation Cursor globalRotationsArray = object.setArray("globalRotations"); - Map<String, RotationStatus> rotationHealthStatus = application.rotation() - .map(rotation -> controller.getHealthStatus(rotation.dnsName())) - .orElse(Collections.emptyMap()); - application.rotation().ifPresent(rotation -> globalRotationsArray.addString(rotation.url().toString())); + application.rotation().ifPresent(rotation -> { + globalRotationsArray.addString(rotation.url().toString()); + object.setString("rotationId", rotation.id().asString()); + }); // Deployments sorted according to deployment spec List<Deployment> deployments = controller.applications().deploymentTrigger() @@ -395,8 +395,12 @@ public class ApplicationApiHandler extends LoggingRequestHandler { deploymentObject.setString("environment", deployment.zone().environment().value()); deploymentObject.setString("region", deployment.zone().region().value()); deploymentObject.setString("instance", application.id().instance().value()); // pointless - if (application.rotation().isPresent()) + if (application.rotation().isPresent()) { + Map<String, RotationStatus> rotationHealthStatus = application.rotation() + .map(rotation -> controller.getHealthStatus(rotation.dnsName())) + .orElse(Collections.emptyMap()); setRotationStatus(deployment, rotationHealthStatus, deploymentObject); + } if (recurseOverDeployments(request)) // List full deployment information when recursive. toSlime(deploymentObject, new DeploymentId(application.id(), deployment.zone()), deployment, request); 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 4ed1edad57f..17801bde546 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 @@ -22,6 +22,7 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; import com.yahoo.vespa.hosted.controller.api.identifiers.UserGroup; import com.yahoo.vespa.hosted.controller.api.integration.BuildService.BuildJob; +import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken; import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; @@ -31,7 +32,6 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType; import com.yahoo.vespa.hosted.controller.application.JobStatus; -import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken; import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzDbMock; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.BuildSystem; @@ -298,10 +298,14 @@ public class ControllerTest { // staging deployment long app1ProjectId = 22; - ApplicationId app1 = tester.createAndDeploy("tenant1", "domain1", "application1", Environment.staging, app1ProjectId).id(); + ApplicationId app1 = tester.createAndDeploy("tenant1", "domain1", + "application1", Environment.staging, + app1ProjectId).id(); // pull-request deployment - uses different instance id - ApplicationId app1pr = tester.createAndDeploy("tenant1", "domain1", "application1", "default-pr1", Environment.staging, app1ProjectId, null).id(); + ApplicationId app1pr = tester.createAndDeploy("tenant1", "domain1", + "application1", "default-pr1", + Environment.staging, app1ProjectId, null).id(); assertTrue(applications.get(app1).isPresent()); assertEquals(app1, applications.get(app1).get().id()); @@ -316,6 +320,20 @@ public class ControllerTest { assertEquals(app1, applications.get(app1).get().id()); assertTrue(applications.get(app1pr).isPresent()); assertEquals(app1pr, applications.get(app1pr).get().id()); + + // Deleting application also removes PR instance + ApplicationId app2 = tester.createAndDeploy("tenant1", "domain1", + "application2", Environment.staging, + 33).id(); + tester.controller().applications().deleteApplication(app1, Optional.of(new NToken("ntoken"))); + assertEquals("All instances deleted", 0, + tester.controller().applications().asList(app1.tenant()).stream() + .filter(app -> app.id().application().equals(app1.application())) + .count()); + assertEquals("Other application survives", 1, + tester.controller().applications().asList(app1.tenant()).stream() + .filter(app -> app.id().application().equals(app2.application())) + .count()); } @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 961e005bfbd..e46c755e8bf 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 @@ -207,6 +207,7 @@ "globalRotations": [ "http://application1.tenant1.global.vespa.yahooapis.com:4080/" ], + "rotationId": "rotation-id-1", "instances": [ { "environment": "prod", 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 3924cf51ca9..b5ed2d407df 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 @@ -148,6 +148,7 @@ "globalRotations": [ "http://application1.tenant1.global.vespa.yahooapis.com:4080/" ], + "rotationId": "rotation-id-1", "instances": [ { "environment": "dev", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json index 5030fc7d0a6..caca0ad8970 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json @@ -148,6 +148,7 @@ "globalRotations": [ "http://application1.tenant1.global.vespa.yahooapis.com:4080/" ], + "rotationId": "rotation-id-1", "instances": [ @include(dev-us-west-1.json), @include(prod-corp-us-east-1.json) diff --git a/dist/vespa.spec b/dist/vespa.spec index c17454804dc..17a9aeb03f4 100644 --- a/dist/vespa.spec +++ b/dist/vespa.spec @@ -18,11 +18,11 @@ Source0: vespa-%{version}.tar.gz %if 0%{?centos} BuildRequires: epel-release BuildRequires: centos-release-scl -BuildRequires: devtoolset-6-gcc-c++ -BuildRequires: devtoolset-6-libatomic-devel -BuildRequires: devtoolset-6-binutils +BuildRequires: devtoolset-7-gcc-c++ +BuildRequires: devtoolset-7-libatomic-devel +BuildRequires: devtoolset-7-binutils BuildRequires: rh-maven33 -%define _devtoolset_enable /opt/rh/devtoolset-6/enable +%define _devtoolset_enable /opt/rh/devtoolset-7/enable %define _rhmaven33_enable /opt/rh/rh-maven33/enable %endif %if 0%{?fedora} diff --git a/document/src/vespa/document/select/CMakeLists.txt b/document/src/vespa/document/select/CMakeLists.txt index eba5ddde40c..6dadd35e98a 100644 --- a/document/src/vespa/document/select/CMakeLists.txt +++ b/document/src/vespa/document/select/CMakeLists.txt @@ -39,3 +39,6 @@ vespa_add_library(document_select OBJECT AFTER document_documentconfig ) + +#TODO Remove once we have a recently new flex compiler. At least 2.5.38/39 or 2.6 +set_source_files_properties(${FLEX_DocSelLexer_OUTPUTS} PROPERTIES COMPILE_FLAGS -Wno-register) diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionRpcServer.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionRpcServer.java index d09cf17b9e3..2e58455bc39 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionRpcServer.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionRpcServer.java @@ -118,7 +118,7 @@ public class FileDistributionRpcServer { List<FileReference> fileReferences = Stream.of(fileReferenceStrings) .map(FileReference::new) .collect(Collectors.toList()); - downloader.queueForDownload(fileReferences); + downloader.queueForAsyncDownload(fileReferences); req.returnValues().add(new Int32Value(0)); } diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java index 6610893d8ae..05bcaacb107 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java @@ -67,12 +67,19 @@ public class FileDownloader { } else { log.log(LogLevel.INFO, "File reference '" + fileReference.value() + "' not found in " + directory.getAbsolutePath() + ", starting download"); - return queueForDownload(fileReference, timeout); + return queueForAsyncDownload(fileReference, timeout); } } - public void queueForDownload(List<FileReference> fileReferences) { - fileReferences.forEach(this::queueForDownload); + // Start downloading, but there is no Future used get file being downloaded + public void queueForAsyncDownload(List<FileReference> fileReferences) { + fileReferences.forEach(fileReference -> { + if (fileReferenceDownloader.isDownloading(fileReference)) { + log.log(LogLevel.DEBUG, "Already downloading '" + fileReference.value() + "'"); + } else { + queueForAsyncDownload(fileReference).cancel(false); + } + }); } void receiveFile(FileReferenceData fileReferenceData) { @@ -107,24 +114,20 @@ public class FileDownloader { return Optional.empty(); } - private synchronized Future<Optional<File>> queueForDownload(FileReference fileReference, Duration timeout) { + private synchronized Future<Optional<File>> queueForAsyncDownload(FileReference fileReference, Duration timeout) { Future<Optional<File>> inProgress = fileReferenceDownloader.addDownloadListener(fileReference, () -> getFile(fileReference)); if (inProgress != null) { log.log(LogLevel.DEBUG, "Already downloading '" + fileReference.value() + "'"); return inProgress; } - Future<Optional<File>> future = queueForDownload(fileReference); + Future<Optional<File>> future = queueForAsyncDownload(fileReference); log.log(LogLevel.INFO, "Queued '" + fileReference.value() + "' for download with timeout " + timeout); return future; } - // We don't care about the future in this call - private Future<Optional<File>> queueForDownload(FileReference fileReference) { - return queueForDownload(new FileReferenceDownload(fileReference, SettableFuture.create())); - } - - private Future<Optional<File>> queueForDownload(FileReferenceDownload fileReferenceDownload) { + private Future<Optional<File>> queueForAsyncDownload(FileReference fileReference) { + FileReferenceDownload fileReferenceDownload = new FileReferenceDownload(fileReference, SettableFuture.create()); fileReferenceDownloader.addToDownloadQueue(fileReferenceDownload); return fileReferenceDownload.future(); } diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReceiver.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReceiver.java index c4e010e57d5..d57ce4ca5de 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReceiver.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReceiver.java @@ -23,6 +23,13 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Logger; +/** + * When asking for a file reference, this handles RPC callbacks from config server with file data and metadata. + * Uses the same Supervisor as the original caller that requests files, so communication uses the same + * connection in both directions. + * + * @author baldersheim + */ public class FileReceiver { private final static Logger log = Logger.getLogger(FileReceiver.class.getName()); @@ -125,6 +132,10 @@ public class FileReceiver { } return file; } + + double percentageReceived() { + return (double)currentFileSize/(double)fileSize; + } } FileReceiver(Supervisor supervisor, FileReferenceDownloader downloader, File downloadDirectory, File tmpDirectory) { @@ -287,6 +298,9 @@ public class FileReceiver { log.severe("Got exception + " + e); retval = 1; } + double completeness = (double) session.currentFileSize / (double) session.fileSize; + log.log(LogLevel.DEBUG, String.format("%.1f percent of '%s' downloaded", completeness * 100, reference.value())); + downloader.setDownloadStatus(reference, completeness); req.returnValues().add(new Int32Value(retval)); } @@ -306,12 +320,12 @@ public class FileReceiver { req.returnValues().add(new Int32Value(retval)); } - private final Session getSession(Integer sessionId) { + private Session getSession(Integer sessionId) { synchronized (sessions) { return sessions.get(sessionId); } } - private static final int verifySession(Session session, int sessionId, FileReference reference) { + private static int verifySession(Session session, int sessionId, FileReference reference) { if (session == null) { log.severe("session-id " + sessionId + " does not exist."); return 1; diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownload.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownload.java index 9de4c1fcd5b..048287f0892 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownload.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownload.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.filedistribution; -import com.google.common.util.concurrent.AtomicDouble; import com.google.common.util.concurrent.SettableFuture; import com.yahoo.config.FileReference; diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownloader.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownloader.java index 7793e25f7b2..b8402c8aacf 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownloader.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownloader.java @@ -18,7 +18,6 @@ import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; import java.util.Optional; -import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.logging.Logger; @@ -50,9 +49,8 @@ public class FileReferenceDownloader { this.fileReceiver = new FileReceiver(connectionPool.getSupervisor(), this, downloadDirectory, tmpDirectory); } - private void startDownload(FileReference fileReference, Duration timeout, - FileReferenceDownload fileReferenceDownload) - { + private void startDownload(Duration timeout, FileReferenceDownload fileReferenceDownload) { + FileReference fileReference = fileReferenceDownload.fileReference(); synchronized (downloads) { downloads.put(fileReference, fileReferenceDownload); downloadStatus.put(fileReference, 0.0); @@ -67,7 +65,7 @@ public class FileReferenceDownloader { Thread.sleep(10); } } - catch (InterruptedException | ExecutionException e) {} + catch (InterruptedException e) {} } if ( !downloadStarted) { @@ -80,7 +78,7 @@ public class FileReferenceDownloader { void addToDownloadQueue(FileReferenceDownload fileReferenceDownload) { log.log(LogLevel.DEBUG, "Will download file reference '" + fileReferenceDownload.fileReference().value() + "'"); - downloadExecutor.submit(() -> startDownload(fileReferenceDownload.fileReference(), downloadTimeout, fileReferenceDownload)); + downloadExecutor.submit(() -> startDownload(downloadTimeout, fileReferenceDownload)); } void receiveFile(FileReferenceData fileReferenceData) { @@ -100,7 +98,7 @@ public class FileReferenceDownloader { } } - private boolean startDownloadRpc(FileReference fileReference) throws ExecutionException, InterruptedException { + private boolean startDownloadRpc(FileReference fileReference) { Connection connection = connectionPool.getCurrent(); Request request = new Request("filedistribution.serveFile"); request.parameters().add(new StringValue(fileReference.value())); @@ -173,8 +171,12 @@ public class FileReferenceDownloader { } void setDownloadStatus(String file, double completeness) { + setDownloadStatus(new FileReference(file), completeness); + } + + void setDownloadStatus(FileReference fileReference, double completeness) { synchronized (downloads) { - downloadStatus.put(new FileReference(file), completeness); + downloadStatus.put(fileReference, completeness); } } diff --git a/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java b/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java index 60478550084..d2da020539a 100644 --- a/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java +++ b/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java @@ -189,7 +189,7 @@ public class FileDownloaderTest { FileReference foo = new FileReference("foo"); FileReference bar = new FileReference("bar"); List<FileReference> fileReferences = Arrays.asList(foo, bar); - fileDownloader.queueForDownload(fileReferences); + fileDownloader.queueForAsyncDownload(fileReferences); // Verify download status assertDownloadStatus(fileDownloader, foo, 0.0); diff --git a/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileReceiverTest.java b/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileReceiverTest.java index 34ce53ad4c8..762817c27ef 100644 --- a/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileReceiverTest.java +++ b/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileReceiverTest.java @@ -33,7 +33,6 @@ public class FileReceiverTest { @Test public void receiveMultiPartFile() throws IOException{ - String [] parts = new String[3]; parts[0] = "first part\n"; parts[1] = "second part\n"; @@ -43,16 +42,12 @@ public class FileReceiverTest { sb.append(s); } String all = sb.toString(); - String allRead = transferParts(new FileReference("ref-a"), "myfile-1", all, 1); - assertEquals(all, allRead); - allRead = transferParts(new FileReference("ref-a"), "myfile-2", all, 2); - assertEquals(all, allRead); - allRead = transferParts(new FileReference("ref-a"), "myfile-3", all, 3); - assertEquals(all, allRead); - + transferPartsAndAssert(new FileReference("ref-a"), "myfile-1", all, 1); + transferPartsAndAssert(new FileReference("ref-a"), "myfile-2", all, 2); + transferPartsAndAssert(new FileReference("ref-a"), "myfile-3", all, 3); } - private String transferParts(FileReference ref, String fileName, String all, int numParts) throws IOException { + private void transferPartsAndAssert(FileReference ref, String fileName, String all, int numParts) throws IOException { byte [] allContent = Utf8.toBytes(all); FileReceiver.Session session = new FileReceiver.Session(root, tempDir, 1, ref, @@ -63,12 +58,14 @@ public class FileReceiverTest { byte [] buf = new byte[Math.min(partSize, allContent.length - pos)]; bb.get(buf); session.addPart(i, buf); + // Small numbers, so need a large delta + assertEquals((double)(i+1)/(double)numParts, session.percentageReceived(), 0.04); pos += buf.length; } - File file = session.close(hasher.hash(ByteBuffer.wrap(allContent), 0)); + File file = session.close(hasher.hash(ByteBuffer.wrap(Utf8.toBytes(all)), 0)); byte [] allReadBytes = Files.readAllBytes(file.toPath()); file.delete(); - return Utf8.toString(allReadBytes); + assertEquals(all, Utf8.toString(allReadBytes)); } } diff --git a/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp b/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp index e07f0684584..de410289ec0 100644 --- a/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp +++ b/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp @@ -4,7 +4,7 @@ #include <vespa/vespalib/net/socket_address.h> #include <vespa/filedistribution/common/logfwd.h> #include <vespa/defaults.h> -#include <vespa/vespalib/util/sync.h> +#include <vespa/vespalib/util/gate.h> #include <vespa/vespalib/text/stringtokenizer.h> #include <zookeeper/zookeeper.h> #include <sstream> diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JerseyJaxRsClientFactory.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JerseyJaxRsClientFactory.java index deacfcdd494..0b3c7ad8d3b 100644 --- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JerseyJaxRsClientFactory.java +++ b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JerseyJaxRsClientFactory.java @@ -7,7 +7,6 @@ import org.glassfish.jersey.client.HttpUrlConnectorProvider; import org.glassfish.jersey.client.proxy.WebResourceFactory; import javax.net.ssl.SSLContext; -import javax.ws.rs.client.Client; import javax.ws.rs.client.ClientBuilder; import javax.ws.rs.client.ClientRequestFilter; import javax.ws.rs.client.WebTarget; @@ -62,6 +61,7 @@ public class JerseyJaxRsClientFactory implements JaxRsClientFactory { .property(ClientProperties.FOLLOW_REDIRECTS, true); if (sslContext != null) { builder.sslContext(sslContext); + builder.hostnameVerifier((s, sslSession) -> true); } if (userAgent != null) { builder.register((ClientRequestFilter) context -> diff --git a/messagebus/src/vespa/messagebus/messagebus.cpp b/messagebus/src/vespa/messagebus/messagebus.cpp index fa646b55221..fd1ad2908c7 100644 --- a/messagebus/src/vespa/messagebus/messagebus.cpp +++ b/messagebus/src/vespa/messagebus/messagebus.cpp @@ -8,6 +8,7 @@ #include "protocolrepository.h" #include <vespa/messagebus/network/inetwork.h> #include <vespa/vespalib/util/exceptions.h> +#include <vespa/vespalib/util/gate.h> #include <vespa/log/log.h> LOG_SETUP(".messagebus"); diff --git a/messagebus/src/vespa/messagebus/messenger.cpp b/messagebus/src/vespa/messagebus/messenger.cpp index 4b612b66c31..63df7f2f482 100644 --- a/messagebus/src/vespa/messagebus/messenger.cpp +++ b/messagebus/src/vespa/messagebus/messenger.cpp @@ -2,6 +2,7 @@ #include "messenger.h" #include <vespa/vespalib/util/sync.h> +#include <vespa/vespalib/util/gate.h> #include <vespa/log/log.h> LOG_SETUP(".messenger"); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java index bdfa98a5113..3576f37eb9a 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.node.admin.util; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.http.HttpHeaders; +import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpDelete; import org.apache.http.client.methods.HttpGet; @@ -63,8 +64,19 @@ public class ConfigServerHttpRequestExecutor { PoolingHttpClientConnectionManager cm = new PoolingHttpClientConnectionManager(getConnectionSocketFactoryRegistry()); cm.setMaxTotal(200); // Increase max total connections to 200, which should be enough + // Have experienced hang in socket read, which may have been because of + // system defaults, therefore set explicit timeouts. Set arbitrarily to + // 15s > 10s used by Orchestrator lock timeout. + int timeoutMs = 15_000; + RequestConfig requestBuilder = RequestConfig.custom() + .setConnectTimeout(timeoutMs) // establishment of connection + .setConnectionRequestTimeout(timeoutMs) // connection from connection manager + .setSocketTimeout(timeoutMs) // waiting for data + .build(); + return new ConfigServerHttpRequestExecutor(randomizeConfigServerUris(configServerUris), HttpClientBuilder.create() + .setDefaultRequestConfig(requestBuilder) .disableAutomaticRetries() .setUserAgent("node-admin") .setConnectionManager(cm).build()); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java index 500861ad0d2..6a2376d748b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java @@ -75,7 +75,7 @@ public abstract class ApplicationMaintainer extends Maintainer { // Lock is acquired with a low timeout to reduce the chance of colliding with an external deployment. try (Mutex lock = nodeRepository().lock(application, Duration.ofSeconds(1))) { if ( ! isActive(application)) return; // became inactive since deployment was requested - Optional<Deployment> deployment = deployer.deployFromLocalActive(application, Duration.ofMinutes(30)); + Optional<Deployment> deployment = deployer.deployFromLocalActive(application); if ( ! deployment.isPresent()) return; // this will be done at another config server deployment.get().activate(); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Expirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Expirer.java index 48ec71642c9..a3b4917147e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Expirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Expirer.java @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.maintenance; -import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.History; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java index 1e0202d4735..031d56e3164 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.maintenance; +import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.NodeType; @@ -21,65 +22,108 @@ import java.util.stream.Collectors; /** * This moves expired failed nodes: * <ul> - * <li>To parked: If the node has known hardware failure, docker hosts are moved to parked only when all its + * <li>To parked: If the node has known hardware failure, Docker hosts are moved to parked only when all of their * children are already in parked - * <li>To dirty: If the node has failed less than 5 times OR the environment is dev, test or perf OR system is CD, - * as those environments have no protection against users running bogus applications, so + * <li>To dirty: If the node has failed less than 5 times OR the environment is dev, test or perf. + * Those environments have no protection against users running bogus applications, so * we cannot use the node failure count to conclude the node has a failure. * <li>Otherwise the node will remain in failed * </ul> - * Failed nodes are typically given a long expiry time to enable us to manually moved them back to + * Failed content nodes are given a long expiry time to enable us to manually moved them back to * active to recover data in cases where the node was failed accidentally. * <p> + * Failed container (Vespa, not Docker) nodes are expired early as there's no data to potentially recover. + * </p> + * <p> * The purpose of the automatic recycling to dirty + fail count is that nodes which were moved * to failed due to some undetected hardware failure will end up being failed again. * When that has happened enough they will not be recycled. * <p> - * The Chef recipe running locally on the node may set the hardwareFailureDescription to avoid the node + * The Chef recipe running locally on the node may set hardwareFailureDescription to avoid the node * being automatically recycled in cases where an error has been positively detected. * * @author bratseth + * @author mpolden */ -public class FailedExpirer extends Expirer { +public class FailedExpirer extends Maintainer { private static final Logger log = Logger.getLogger(NodeRetirer.class.getName()); + + private static final Duration defaultExpiry = Duration.ofDays(4); // Grace period to allow recovery of data + private static final Duration containerExpiry = Duration.ofHours(1); // Stateless nodes, no data to recover + private static final int maxAllowedFailures = 5; // Stop recycling nodes after this number of failures + private final NodeRepository nodeRepository; private final Zone zone; + private final Clock clock; - public FailedExpirer(NodeRepository nodeRepository, Zone zone, Clock clock, - Duration failTimeout, JobControl jobControl) { - super(Node.State.failed, History.Event.Type.failed, nodeRepository, clock, failTimeout, jobControl); + public FailedExpirer(NodeRepository nodeRepository, Zone zone, Clock clock, Duration interval, + JobControl jobControl) { + super(nodeRepository, interval, jobControl); this.nodeRepository = nodeRepository; this.zone = zone; + this.clock = clock; } @Override - protected void expire(List<Node> expired) { + protected void maintain() { + List<Node> containerNodes = getExpiredNodes(containerExpiry) + .stream() + .filter(node -> node.allocation().isPresent() && + node.allocation().get().membership().cluster().type() == ClusterSpec.Type.container) + .collect(Collectors.toList()); + List<Node> remainingNodes = getExpiredNodes(defaultExpiry); + remainingNodes.removeAll(containerNodes); + recycle(containerNodes); + recycle(remainingNodes); + } + + /** Get failed nodes that have expired according to given expiry */ + private List<Node> getExpiredNodes(Duration expiry) { + return nodeRepository.getNodes(Node.State.failed).stream() + .filter(node -> node.history().event(History.Event.Type.failed) + .map(event -> event.at().plus(expiry).isBefore(clock.instant())) + .orElse(false)) + .collect(Collectors.toList()); + } + + /** Move eligible nodes to dirty. This may be a subset of the given nodes */ + private void recycle(List<Node> nodes) { List<Node> nodesToRecycle = new ArrayList<>(); - for (Node recycleCandidate : expired) { - if (recycleCandidate.status().hardwareFailureDescription().isPresent() || recycleCandidate.status().hardwareDivergence().isPresent()) { - List<String> nonParkedChildren = recycleCandidate.type() != NodeType.host ? Collections.emptyList() : - nodeRepository.getChildNodes(recycleCandidate.hostname()).stream() + for (Node candidate : nodes) { + if (hasHardwareIssue(candidate)) { + List<String> unparkedChildren = candidate.type() != NodeType.host ? Collections.emptyList() : + nodeRepository.getChildNodes(candidate.hostname()).stream() .filter(node -> node.state() != Node.State.parked) .map(Node::hostname) .collect(Collectors.toList()); - if (nonParkedChildren.isEmpty()) { - nodeRepository.park(recycleCandidate.hostname(), Agent.system, "Parked by FailedExpirer due to HW failure/divergence on node"); + if (unparkedChildren.isEmpty()) { + nodeRepository.park(candidate.hostname(), Agent.system, + "Parked by FailedExpirer due to hardware issue"); } else { - log.info(String.format("Expired failed node %s with HW failure/divergence is not parked because some of its children" + - " (%s) are not yet parked", recycleCandidate.hostname(), String.join(", ", nonParkedChildren))); + log.info(String.format("Expired failed node %s with hardware issue was not parked because of " + + "unparked children: %s", candidate.hostname(), + String.join(", ", unparkedChildren))); } - } else if (! failCountIndicatesHwFail(zone, recycleCandidate) || recycleCandidate.status().failCount() < 5) { - nodesToRecycle.add(recycleCandidate); + } else if (!failCountIndicatesHardwareIssue(candidate)) { + nodesToRecycle.add(candidate); } } nodeRepository.setDirty(nodesToRecycle); } - private boolean failCountIndicatesHwFail(Zone zone, Node node) { + /** Returns whether the current node fail count should be used as an indicator of hardware issue */ + private boolean failCountIndicatesHardwareIssue(Node node) { if (node.flavor().getType() == Flavor.Type.DOCKER_CONTAINER) return false; - return zone.environment() == Environment.prod || zone.environment() == Environment.staging; + return (zone.environment() == Environment.prod || zone.environment() == Environment.staging) && + node.status().failCount() >= maxAllowedFailures; + } + + /** Returns whether node has any kind of hardware issue */ + private static boolean hasHardwareIssue(Node node) { + return node.status().hardwareFailureDescription().isPresent() || + node.status().hardwareDivergence().isPresent(); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java index 1fffde874fd..9e826bfcb9a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java @@ -70,9 +70,9 @@ public class NodeRepositoryMaintenance extends AbstractComponent { zooKeeperAccessMaintainer = new ZooKeeperAccessMaintainer(nodeRepository, curator, durationFromEnv("zookeeper_access_maintenance_interval").orElse(defaults.zooKeeperAccessMaintenanceInterval), jobControl); reservationExpirer = new ReservationExpirer(nodeRepository, clock, durationFromEnv("reservation_expiry").orElse(defaults.reservationExpiry), jobControl); retiredExpirer = new RetiredExpirer(nodeRepository, deployer, clock, durationFromEnv("retired_expiry").orElse(defaults.retiredExpiry), jobControl); - retiredEarlyExpirer = new RetiredEarlyExpirer(nodeRepository, zone, durationFromEnv("retired_early_interval").orElse(defaults.retiredEarlyInterval), jobControl, deployer, orchestrator); + retiredEarlyExpirer = new RetiredEarlyExpirer(nodeRepository, durationFromEnv("retired_early_interval").orElse(defaults.retiredEarlyInterval), jobControl, deployer, orchestrator); inactiveExpirer = new InactiveExpirer(nodeRepository, clock, durationFromEnv("inactive_expiry").orElse(defaults.inactiveExpiry), jobControl); - failedExpirer = new FailedExpirer(nodeRepository, zone, clock, durationFromEnv("failed_expiry").orElse(defaults.failedExpiry), jobControl); + failedExpirer = new FailedExpirer(nodeRepository, zone, clock, durationFromEnv("failed_expirer_interval").orElse(defaults.failedExpirerInterval), jobControl); dirtyExpirer = new DirtyExpirer(nodeRepository, clock, durationFromEnv("dirty_expiry").orElse(defaults.dirtyExpiry), jobControl); provisionedExpirer = new ProvisionedExpirer(nodeRepository, clock, durationFromEnv("provisioned_expiry").orElse(defaults.provisionedExpiry), jobControl); nodeRebooter = new NodeRebooter(nodeRepository, clock, durationFromEnv("reboot_interval").orElse(defaults.rebootInterval), jobControl); @@ -134,7 +134,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { private final Duration reservationExpiry; private final Duration inactiveExpiry; private final Duration retiredExpiry; - private final Duration failedExpiry; + private final Duration failedExpirerInterval; private final Duration dirtyExpiry; private final Duration provisionedExpiry; private final Duration rebootInterval; @@ -156,7 +156,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { inactiveExpiry = Duration.ofHours(4); // enough time for the application owner to discover and redeploy retiredExpiry = Duration.ofDays(4); // enough time to migrate data retiredEarlyInterval = Duration.ofMinutes(29); - failedExpiry = Duration.ofDays(4); // enough time to recover data even if it happens friday night + failedExpirerInterval = Duration.ofMinutes(10); dirtyExpiry = Duration.ofHours(2); // enough time to clean the node provisionedExpiry = Duration.ofHours(4); rebootInterval = Duration.ofDays(30); @@ -174,7 +174,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { inactiveExpiry = Duration.ofSeconds(2); // support interactive wipe start over retiredExpiry = Duration.ofMinutes(1); retiredEarlyInterval = Duration.ofMinutes(5); - failedExpiry = Duration.ofMinutes(10); + failedExpirerInterval = Duration.ofMinutes(10); dirtyExpiry = Duration.ofMinutes(30); provisionedExpiry = Duration.ofHours(4); rebootInterval = Duration.ofDays(30); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirer.java index 12b63f66d3f..30b5f6f737d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirer.java @@ -123,7 +123,7 @@ public class NodeRetirer extends Maintainer { entry -> filterRetireableNodes(entry.getValue()))); if (retireableNodesByCluster.values().stream().mapToInt(Set::size).sum() == 0) continue; - Optional<Deployment> deployment = deployer.deployFromLocalActive(applicationId, Duration.ofMinutes(30)); + Optional<Deployment> deployment = deployer.deployFromLocalActive(applicationId); if ( ! deployment.isPresent()) continue; // this will be done at another config server Set<Node> replaceableNodes = retireableNodesByCluster.entrySet().stream() diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredEarlyExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredEarlyExpirer.java index cb2fcb89284..00543058520 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredEarlyExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredEarlyExpirer.java @@ -5,7 +5,6 @@ import com.yahoo.collections.ListMap; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Deployer; import com.yahoo.config.provision.Deployment; -import com.yahoo.config.provision.Zone; import com.yahoo.vespa.applicationmodel.HostName; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; @@ -20,12 +19,19 @@ import java.util.Optional; import java.util.logging.Level; import java.util.stream.Collectors; +/** + * Maintenance job which deactivates retired nodes, if given permission by orchestrator. + * + * @author hakon + */ +// TODO: This should be consolidated with RetiredExpirer. The only difference between this and RetiredExpirer is that +// this runs more often by default and asks orchestrator for permission to retire nodes. public class RetiredEarlyExpirer extends Maintainer { + private final Deployer deployer; private final Orchestrator orchestrator; public RetiredEarlyExpirer(NodeRepository nodeRepository, - Zone zone, Duration interval, JobControl jobControl, Deployer deployer, @@ -51,12 +57,12 @@ public class RetiredEarlyExpirer extends Maintainer { List<Node> retiredNodes = entry.getValue(); try { - Optional<Deployment> deployment = deployer.deployFromLocalActive(application, Duration.ofMinutes(30)); + Optional<Deployment> deployment = deployer.deployFromLocalActive(application); if ( ! deployment.isPresent()) continue; // this will be done at another config server List<Node> nodesToRemove = new ArrayList<>(); for (Node node : retiredNodes) { - if (nodeCanBeRemoved(node)) { + if (canRemove(node)) { nodesToRemove.add(node); } } @@ -79,7 +85,8 @@ public class RetiredEarlyExpirer extends Maintainer { } } - boolean nodeCanBeRemoved(Node node) { + /** Returns whether orchestrator permits given node to be removed */ + private boolean canRemove(Node node) { try { orchestrator.acquirePermissionToRemove(new HostName(node.hostname())); return true; @@ -88,4 +95,5 @@ public class RetiredEarlyExpirer extends Maintainer { return false; } } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java index 9ef09858bfa..4c5c8adf576 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java @@ -50,7 +50,7 @@ public class RetiredExpirer extends Expirer { ApplicationId application = entry.getKey(); List<Node> nodesToRemove = entry.getValue(); try { - Optional<Deployment> deployment = deployer.deployFromLocalActive(application, Duration.ofMinutes(30)); + Optional<Deployment> deployment = deployer.deployFromLocalActive(application); if ( ! deployment.isPresent()) continue; // this will be done at another config server nodeRepository.setRemovable(application, nodesToRemove); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java index 1de15f4e31a..7ef609d6311 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java @@ -52,7 +52,7 @@ public class NodeRepositoryProvisioner implements Provisioner { @Inject public NodeRepositoryProvisioner(NodeRepository nodeRepository, NodeFlavors flavors, Zone zone) { - this(nodeRepository, flavors, zone, Clock.systemUTC(), (x,y) -> {}); + this(nodeRepository, flavors, zone, Clock.systemUTC(), (x, y) -> {}); } public NodeRepositoryProvisioner(NodeRepository nodeRepository, NodeFlavors flavors, Zone zone, Clock clock, BiConsumer<List<Node>, String> debugRecorder) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java index a05b0be344d..c880c66abc7 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java @@ -46,6 +46,11 @@ public class MockDeployer implements Deployer { } @Override + public Optional<Deployment> deployFromLocalActive(ApplicationId application) { + return deployFromLocalActive(application, Duration.ofSeconds(60)); + } + + @Override public Optional<Deployment> deployFromLocalActive(ApplicationId id, Duration timeout) { return Optional.of(new MockDeployment(provisioner, applications.get(id))); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java index 51991a844d7..720c5b05443 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java @@ -9,6 +9,7 @@ import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.Flavor; +import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.NodeFlavors; import com.yahoo.config.provision.NodeType; @@ -18,7 +19,6 @@ import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.Zone; import com.yahoo.test.ManualClock; import com.yahoo.transaction.NestedTransaction; -import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.curator.transaction.CuratorTransaction; import com.yahoo.vespa.hosted.provision.Node; @@ -26,15 +26,15 @@ import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder; import com.yahoo.vespa.hosted.provision.provisioning.NodeRepositoryProvisioner; -import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester; import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver; import org.junit.Test; import java.time.Duration; -import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.Set; +import java.util.UUID; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -42,155 +42,268 @@ import static org.junit.Assert.assertEquals; /** * @author bratseth + * @author mpolden */ public class FailedExpirerTest { - private final Curator curator = new MockCurator(); - private final ManualClock clock = new ManualClock(); - private FailedExpirer failedExpirer; - @Test - public void ensure_failed_nodes_are_deallocated_in_prod() throws InterruptedException { - failureScenarioIn(SystemName.main, Environment.prod, "default"); - clock.advance(Duration.ofDays(5)); - failedExpirer.run(); - - assertNodeHostnames(Node.State.failed, "node1"); - assertNodeHostnames(Node.State.parked, "node2", "node3"); + public void ensure_failed_nodes_are_deallocated_in_prod() { + FailureScenario scenario = new FailureScenario(SystemName.main, Environment.prod) + .withNode("node1") + .withNode("node2") + .withNode("node3") + .setReady("node1", "node2", "node3") + .allocate(ClusterSpec.Type.content, "node1", "node2", "node3") + .failNode(4, "node1") + .failWithHardwareFailure("node2", "node3"); + + scenario.clock().advance(Duration.ofDays(3)); + scenario.expirer().run(); + scenario.assertNodesIn(Node.State.failed, "node1", "node2", "node3"); // None moved yet + + scenario.clock().advance(Duration.ofDays(2)); + scenario.expirer().run(); + scenario.assertNodesIn(Node.State.failed, "node1"); + scenario.assertNodesIn(Node.State.parked, "node2", "node3"); } @Test - public void ensure_failed_nodes_are_deallocated_in_dev() throws InterruptedException { - failureScenarioIn(SystemName.main, Environment.dev, "default"); - clock.advance(Duration.ofDays(5)); - failedExpirer.run(); - - assertNodeHostnames(Node.State.parked, "node2", "node3"); - assertNodeHostnames(Node.State.dirty, "node1"); + public void ensure_failed_nodes_are_deallocated_in_dev() { + FailureScenario scenario = new FailureScenario(SystemName.main, Environment.dev) + .withNode("node1") + .withNode("node2") + .withNode("node3") + .setReady("node1", "node2", "node3") + .allocate(ClusterSpec.Type.content, "node1", "node2", "node3") + .failNode(4, "node1") + .failWithHardwareFailure("node2", "node3"); + + scenario.clock().advance(Duration.ofDays(5)); + scenario.expirer().run(); + + scenario.assertNodesIn(Node.State.parked, "node2", "node3"); + scenario.assertNodesIn(Node.State.dirty, "node1"); } @Test - public void ensure_failed_nodes_are_deallocated_in_cd() throws InterruptedException { - failureScenarioIn(SystemName.cd, Environment.prod, "default"); - clock.advance(Duration.ofDays(5)); - failedExpirer.run(); - - assertNodeHostnames(Node.State.failed, "node1"); - assertNodeHostnames(Node.State.parked, "node2", "node3"); + public void ensure_failed_nodes_are_deallocated_in_cd() { + FailureScenario scenario = new FailureScenario(SystemName.cd, Environment.prod) + .withNode("node1") + .withNode("node2") + .withNode("node3") + .setReady("node1", "node2", "node3") + .allocate(ClusterSpec.Type.content, "node1", "node2", "node3") + .failNode(4, "node1") + .failWithHardwareFailure("node2", "node3"); + + scenario.clock().advance(Duration.ofDays(5)); + scenario.expirer().run(); + + scenario.assertNodesIn(Node.State.failed, "node1"); + scenario.assertNodesIn(Node.State.parked, "node2", "node3"); } @Test - public void ensure_failed_docker_nodes_are_deallocated() throws InterruptedException { - failureScenarioIn(SystemName.main, Environment.prod, "docker"); - clock.advance(Duration.ofDays(5)); - failedExpirer.run(); - - assertNodeHostnames(Node.State.parked, "node2", "node3"); - assertNodeHostnames(Node.State.dirty, "node1"); + public void ensure_failed_docker_nodes_are_deallocated() { + FailureScenario scenario = new FailureScenario(SystemName.main, Environment.prod) + .withNode(NodeType.host, FailureScenario.defaultFlavor, "parent1") + .withNode(NodeType.host, FailureScenario.defaultFlavor, "parent2") + .withNode(NodeType.host, FailureScenario.defaultFlavor, "parent3") + .withNode(NodeType.tenant, FailureScenario.dockerFlavor, "node1", "parent1") + .withNode(NodeType.tenant, FailureScenario.dockerFlavor, "node2", "parent2") + .withNode(NodeType.tenant, FailureScenario.dockerFlavor, "node3", "parent3") + .setReady("node1", "node2", "node3") + .allocate(ClusterSpec.Type.content, FailureScenario.dockerFlavor, "node1", "node2", "node3") + .failNode(4, "node1") + .failWithHardwareFailure("node2", "node3"); + + scenario.clock().advance(Duration.ofDays(5)); + scenario.expirer().run(); + + scenario.assertNodesIn(Node.State.parked, "node2", "node3"); + scenario.assertNodesIn(Node.State.dirty, "node1"); } @Test - public void ensure_parked_docker_host() throws InterruptedException { - failureScenarioIn(SystemName.main, Environment.prod, "docker"); - - failNode("parent2"); - setHWFailureForNode("parent2"); - - clock.advance(Duration.ofDays(5)); - failedExpirer.run(); // Run twice because parent can only be parked after the child - failedExpirer.run(); - - assertNodeHostnames(Node.State.parked, "parent2", "node2", "node3"); + public void ensure_parked_docker_host() { + FailureScenario scenario = new FailureScenario(SystemName.main, Environment.prod) + .withNode(NodeType.host, FailureScenario.defaultFlavor, "parent1") + .withNode(NodeType.host, FailureScenario.defaultFlavor, "parent2") + .withNode(NodeType.host, FailureScenario.defaultFlavor, "parent3") + .withNode(NodeType.tenant, FailureScenario.dockerFlavor, "node1", "parent1") + .withNode(NodeType.tenant, FailureScenario.dockerFlavor, "node2", "parent2") + .withNode(NodeType.tenant, FailureScenario.dockerFlavor, "node3", "parent3") + .setReady("node1", "node2", "node3") + .allocate(ClusterSpec.Type.content, FailureScenario.dockerFlavor, "node1", "node2", "node3") + .failNode(8, "node3") + .failWithHardwareFailure("node2", "node3") + .failWithHardwareFailure("parent2"); + + scenario.clock.advance(Duration.ofDays(5)); + scenario.expirer().run(); // Run twice because parent can only be parked after the child + scenario.expirer().run(); + scenario.assertNodesIn(Node.State.parked, "parent2", "node2", "node3"); } @Test - public void ensure_failed_docker_host_is_not_parked_unless_all_children_are() throws InterruptedException { - failureScenarioIn(SystemName.cd, Environment.prod, "docker"); - - failNode("parent1"); - setHWFailureForNode("parent1"); - clock.advance(Duration.ofDays(2)); - failNode("node4"); - failNode("node5"); - clock.advance(Duration.ofDays(3)); - - failedExpirer.run(); // Run twice because parent can only be parked after the child - failedExpirer.run(); - - assertNodeHostnames(Node.State.failed, "parent1", "node4", "node5"); + public void ensure_failed_docker_host_is_not_parked_unless_all_children_are() { + FailureScenario scenario = new FailureScenario(SystemName.cd, Environment.prod) + .withNode(NodeType.host, FailureScenario.defaultFlavor, "parent1") + .withNode(NodeType.host, FailureScenario.defaultFlavor, "parent2") + .withNode(NodeType.host, FailureScenario.defaultFlavor, "parent3") + .withNode(NodeType.tenant, FailureScenario.dockerFlavor, "node1", "parent1") + .withNode(NodeType.tenant, FailureScenario.dockerFlavor, "node2", "parent2") + .withNode(NodeType.tenant, FailureScenario.dockerFlavor, "node3", "parent3") + .withNode(NodeType.tenant, FailureScenario.dockerFlavor, "node4", "parent1") + .withNode(NodeType.tenant, FailureScenario.dockerFlavor, "node5", "parent1") + .setReady("node1", "node2", "node3") + .allocate(ClusterSpec.Type.content, FailureScenario.dockerFlavor, "node1", "node2", "node3") + .failWithHardwareFailure("parent1"); + + scenario.clock().advance(Duration.ofDays(2)); + scenario.failNode(1, "node4", "node5"); + scenario.clock().advance(Duration.ofDays(3)); + + scenario.expirer().run(); // Run twice because parent can only be parked after the child + scenario.expirer().run(); + + scenario.assertNodesIn(Node.State.failed, "parent1", "node4", "node5"); } - private void assertNodeHostnames(Node.State state, String... hostnames) { - assertEquals(Stream.of(hostnames).collect(Collectors.toSet()), - failedExpirer.nodeRepository().getNodes(state).stream().map(Node::hostname).collect(Collectors.toSet())); - } - - private void setHWFailureForNode(String hostname) { - Node node2 = failedExpirer.nodeRepository().getNode(hostname).get(); - node2 = node2.with(node2.status().withHardwareFailureDescription(Optional.of("memory_mcelog"))); - failedExpirer.nodeRepository().write(node2); + @Test + public void ensure_container_nodes_are_recycled_early() { + FailureScenario scenario = new FailureScenario(SystemName.main, Environment.prod) + .withNode("node1") + .withNode("node2") + .withNode("node3") + .withNode("node4") + .withNode("node5") + .withNode("node6") + .setReady("node1", "node2", "node3", "node4", "node5", "node6") + .allocate(ClusterSpec.Type.content, "node1", "node2", "node3") + .allocate(ClusterSpec.Type.container, "node4", "node5", "node6"); + + // Vespa container fails + scenario.failNode(1, "node4"); + + // 30 minutes pass, nothing happens + scenario.clock().advance(Duration.ofMinutes(30)); + scenario.expirer().run(); + scenario.assertNodesIn(Node.State.dirty); + + // Recycles container when more than 1 hour passes + scenario.clock().advance(Duration.ofMinutes(30).plus(Duration.ofSeconds(1))); + scenario.expirer().run(); + scenario.assertNodesIn(Node.State.dirty, "node4"); } - private void failNode(String hostname) { - failedExpirer.nodeRepository().fail(hostname, Agent.system, "Failing to unit test"); + private static class FailureScenario { + + private static final NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("default", "docker"); + public static final Flavor defaultFlavor = nodeFlavors.getFlavorOrThrow("default"); + public static final Flavor dockerFlavor = nodeFlavors.getFlavorOrThrow("docker"); + + private final MockCurator curator = new MockCurator(); + private final ManualClock clock = new ManualClock(); + private final ApplicationId applicationId = ApplicationId.from(TenantName.from("foo"), + ApplicationName.from("bar"), + InstanceName.from("default")); + + private final NodeRepository nodeRepository; + private final NodeRepositoryProvisioner provisioner; + private final FailedExpirer expirer; + + public FailureScenario(SystemName system, Environment environment) { + Zone zone = new Zone(system, environment, RegionName.defaultName()); + this.nodeRepository = new NodeRepository(nodeFlavors, curator, clock, zone, + new MockNameResolver().mockAnyLookup(), + new DockerImage("docker-image")); + this.provisioner = new NodeRepositoryProvisioner(nodeRepository, nodeFlavors, Zone.defaultZone(), clock, + (x, y) -> {}); + this.expirer = new FailedExpirer(nodeRepository, zone, clock, Duration.ofMinutes(30), + new JobControl(nodeRepository.database())); + } + + public ManualClock clock() { + return clock; + } + + public FailedExpirer expirer() { + return expirer; + } + + public Node get(String hostname) { + return nodeRepository.getNode(hostname) + .orElseThrow(() -> new IllegalArgumentException("No such node: " + hostname)); + } + + public FailureScenario withNode(NodeType type, Flavor flavor, String hostname, String parentHostname) { + nodeRepository.addNodes(Collections.singletonList( + nodeRepository.createNode(UUID.randomUUID().toString(), hostname, + Optional.ofNullable(parentHostname), flavor, type) + )); + return this; + } + + public FailureScenario withNode(NodeType type, Flavor flavor, String hostname) { + return withNode(type, flavor, hostname,null); + } + + public FailureScenario withNode(String hostname) { + return withNode(NodeType.tenant, defaultFlavor, hostname, null); + } + + public FailureScenario failNode(int times, String... hostname) { + Stream.of(hostname).forEach(h -> { + Node node = get(h); + nodeRepository.write(node.with(node.status().setFailCount(times))); + nodeRepository.fail(h, Agent.system, "Failed by unit test"); + }); + return this; + } + + public FailureScenario failWithHardwareFailure(String... hostname) { + Stream.of(hostname).forEach(h -> { + Node node = get(h); + nodeRepository.write(node.with(node.status().withHardwareFailureDescription( + Optional.of("memory_mcelog")))); + nodeRepository.fail(h, Agent.system, "Failed by unit test"); + }); + return this; + } + + public FailureScenario setReady(String... hostname) { + List<Node> nodes = Stream.of(hostname) + .map(this::get) + .collect(Collectors.toList()); + nodeRepository.setReady(nodeRepository.setDirty(nodes)); + return this; + } + + public FailureScenario allocate(ClusterSpec.Type clusterType, String... hostname) { + return allocate(clusterType, defaultFlavor, hostname); + } + + public FailureScenario allocate(ClusterSpec.Type clusterType, Flavor flavor, String... hostname) { + Set<HostSpec> hosts = Stream.of(hostname) + .map(h -> new HostSpec(h, Optional.empty())) + .collect(Collectors.toSet()); + ClusterSpec clusterSpec = ClusterSpec.request(clusterType, ClusterSpec.Id.from("test"), + Version.fromString("6.42")); + provisioner.prepare(applicationId, clusterSpec, Capacity.fromNodeCount(hostname.length, flavor.name()), + 1, null); + NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator)); + provisioner.activate(transaction, applicationId, hosts); + transaction.commit(); + return this; + } + + public void assertNodesIn(Node.State state, String... hostnames) { + assertEquals(Stream.of(hostnames).collect(Collectors.toSet()), + nodeRepository.getNodes(state).stream() + .map(Node::hostname) + .collect(Collectors.toSet())); + } } - private void failureScenarioIn(SystemName system, Environment environment, String flavorName) { - NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("default", flavorName); - NodeRepository nodeRepository = new NodeRepository(nodeFlavors, curator, clock, Zone.defaultZone(), - new MockNameResolver().mockAnyLookup(), - new DockerImage("docker-registry.domain.tld:8080/dist/vespa")); - NodeRepositoryProvisioner provisioner = new NodeRepositoryProvisioner(nodeRepository, nodeFlavors, Zone.defaultZone(), clock, (x,y) -> {}); - failedExpirer = new FailedExpirer(nodeRepository, new Zone(system, environment, RegionName.from("us-west-1")), clock, Duration.ofDays(4), new JobControl(nodeRepository.database())); - - Flavor defaultFlavor = nodeFlavors.getFlavorOrThrow("default"); - List<Node> hostNodes = new ArrayList<>(3); - hostNodes.add(nodeRepository.createNode("parent1", "parent1", Optional.empty(), defaultFlavor, NodeType.host)); - hostNodes.add(nodeRepository.createNode("parent2", "parent2", Optional.empty(), defaultFlavor, NodeType.host)); - hostNodes.add(nodeRepository.createNode("parent3", "parent3", Optional.empty(), defaultFlavor, NodeType.host)); - nodeRepository.addNodes(hostNodes); - - Flavor flavor = nodeFlavors.getFlavorOrThrow(flavorName); - List<Node> nodes = new ArrayList<>(3); - Optional<String> parentHost1 = flavorName.equals("docker") ? Optional.of("parent1") : Optional.empty(); - Optional<String> parentHost2 = flavorName.equals("docker") ? Optional.of("parent2") : Optional.empty(); - Optional<String> parentHost3 = flavorName.equals("docker") ? Optional.of("parent3") : Optional.empty(); - nodes.add(nodeRepository.createNode("node1", "node1", parentHost1, flavor, NodeType.tenant)); - nodes.add(nodeRepository.createNode("node2", "node2", parentHost2, flavor, NodeType.tenant)); - nodes.add(nodeRepository.createNode("node3", "node3", parentHost3, flavor, NodeType.tenant)); - nodeRepository.addNodes(nodes); - - // Set node1 to have failed 4 times before - Node node1 = nodeRepository.getNode("node1").get(); - node1 = node1.with(node1.status().setFailCount(4)); - nodeRepository.write(node1); - - // Set node2 to have a detected hardware failure - setHWFailureForNode("node2"); - - // Set node3 to have failed 8 times before and have a HW failure - Node node3 = nodeRepository.getNode("node3").get(); - node3 = node1.with(node3.status().setFailCount(8)); - nodeRepository.write(node3); - setHWFailureForNode("node3"); - - // Allocate the nodes - List<Node> provisioned = nodeRepository.getNodes(NodeType.tenant, Node.State.provisioned); - nodeRepository.setReady(nodeRepository.setDirty(provisioned)); - nodeRepository.addNodes(Arrays.asList( - nodeRepository.createNode("node4", "node4", parentHost1, flavor, NodeType.tenant), - nodeRepository.createNode("node5", "node5", parentHost1, flavor, NodeType.tenant))); - - ApplicationId applicationId = ApplicationId.from(TenantName.from("foo"), ApplicationName.from("bar"), InstanceName.from("fuz")); - ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("test"), Version.fromString("6.42")); - provisioner.prepare(applicationId, cluster, Capacity.fromNodeCount(3, flavorName), 1, null); - NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator)); - provisioner.activate(transaction, applicationId, ProvisioningTester.toHostSpecs(nodes)); - transaction.commit(); - assertEquals(3, nodeRepository.getNodes(NodeType.tenant, Node.State.active).size()); - - // Fail the nodes - nodes.forEach(node -> failNode(node.hostname())); - assertEquals(3, nodeRepository.getNodes(NodeType.tenant, Node.State.failed).size()); - } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveAndFailedExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveAndFailedExpirerTest.java index 704ded54479..048856bc698 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveAndFailedExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveAndFailedExpirerTest.java @@ -40,7 +40,7 @@ public class InactiveAndFailedExpirerTest { InstanceName.from("fuz")); @Test - public void inactive_and_failed_times_out() throws InterruptedException { + public void inactive_and_failed_times_out() { ProvisioningTester tester = new ProvisioningTester(new Zone(Environment.prod, RegionName.from("us-east"))); List<Node> nodes = tester.makeReadyNodes(2, "default"); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java index 8195321be03..12e2eb3f323 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java @@ -52,7 +52,7 @@ public class RetiredExpirerTest { private Curator curator = new MockCurator(); @Test - public void ensure_retired_nodes_time_out() throws InterruptedException { + public void ensure_retired_nodes_time_out() { ManualClock clock = new ManualClock(); Zone zone = new Zone(Environment.prod, RegionName.from("us-east")); NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("default"); @@ -92,7 +92,7 @@ public class RetiredExpirerTest { } @Test - public void ensure_retired_groups_time_out() throws InterruptedException { + public void ensure_retired_groups_time_out() { ManualClock clock = new ManualClock(); Zone zone = new Zone(Environment.prod, RegionName.from("us-east")); NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("default"); @@ -170,7 +170,6 @@ public class RetiredExpirerTest { new RetiredEarlyExpirer( nodeRepository, - zone, Duration.ofDays(30), new JobControl(nodeRepository.database()), deployer, diff --git a/searchcore/src/tests/proton/attribute/exclusive_attribute_read_accessor/exclusive_attribute_read_accessor_test.cpp b/searchcore/src/tests/proton/attribute/exclusive_attribute_read_accessor/exclusive_attribute_read_accessor_test.cpp index b8a3cceb153..6d69d9b225b 100644 --- a/searchcore/src/tests/proton/attribute/exclusive_attribute_read_accessor/exclusive_attribute_read_accessor_test.cpp +++ b/searchcore/src/tests/proton/attribute/exclusive_attribute_read_accessor/exclusive_attribute_read_accessor_test.cpp @@ -5,7 +5,7 @@ #include <vespa/searchcommon/attribute/config.h> #include <vespa/searchlib/attribute/attributefactory.h> #include <vespa/searchlib/common/sequencedtaskexecutor.h> -#include <vespa/vespalib/util/sync.h> +#include <vespa/vespalib/util/gate.h> using namespace proton; using namespace search; diff --git a/searchcore/src/tests/proton/documentdb/job_tracked_maintenance_job/job_tracked_maintenance_job_test.cpp b/searchcore/src/tests/proton/documentdb/job_tracked_maintenance_job/job_tracked_maintenance_job_test.cpp index 0a4b83350ba..c1626e94809 100644 --- a/searchcore/src/tests/proton/documentdb/job_tracked_maintenance_job/job_tracked_maintenance_job_test.cpp +++ b/searchcore/src/tests/proton/documentdb/job_tracked_maintenance_job/job_tracked_maintenance_job_test.cpp @@ -7,7 +7,7 @@ LOG_SETUP("job_tracked_maintenance_test"); #include <vespa/searchcore/proton/test/simple_job_tracker.h> #include <vespa/vespalib/testkit/testapp.h> #include <vespa/vespalib/util/closuretask.h> -#include <vespa/vespalib/util/sync.h> +#include <vespa/vespalib/util/gate.h> #include <vespa/vespalib/util/threadstackexecutor.h> using namespace proton; diff --git a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp index 0f69684373c..bf88a0c3003 100644 --- a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp +++ b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp @@ -32,8 +32,9 @@ #include <vespa/vespalib/data/slime/slime.h> #include <vespa/vespalib/testkit/testapp.h> #include <vespa/vespalib/util/closuretask.h> -#include <vespa/vespalib/util/sync.h> +#include <vespa/vespalib/util/gate.h> #include <vespa/vespalib/util/threadstackexecutor.h> +#include <unistd.h> #include <vespa/log/log.h> LOG_SETUP("maintenancecontroller_test"); diff --git a/searchcore/src/tests/proton/metrics/job_tracked_flush/job_tracked_flush_test.cpp b/searchcore/src/tests/proton/metrics/job_tracked_flush/job_tracked_flush_test.cpp index a0dd3ee6214..4ed01f8caf7 100644 --- a/searchcore/src/tests/proton/metrics/job_tracked_flush/job_tracked_flush_test.cpp +++ b/searchcore/src/tests/proton/metrics/job_tracked_flush/job_tracked_flush_test.cpp @@ -9,7 +9,7 @@ LOG_SETUP("job_tracked_flush_test"); #include <vespa/vespalib/testkit/testapp.h> #include <vespa/vespalib/util/closuretask.h> #include <vespa/vespalib/util/threadstackexecutor.h> -#include <vespa/vespalib/util/sync.h> +#include <vespa/vespalib/util/gate.h> using namespace proton; using namespace searchcorespi; @@ -17,7 +17,6 @@ using search::SerialNum; using test::SimpleJobTracker; using vespalib::makeTask; using vespalib::makeClosure; -using vespalib::CountDownLatch; using vespalib::Gate; using vespalib::ThreadStackExecutor; diff --git a/searchcore/src/vespa/searchcore/proton/attribute/exclusive_attribute_read_accessor.cpp b/searchcore/src/vespa/searchcore/proton/attribute/exclusive_attribute_read_accessor.cpp index c2c26e52a21..d9a0ff3d8dd 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/exclusive_attribute_read_accessor.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/exclusive_attribute_read_accessor.cpp @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "exclusive_attribute_read_accessor.h" +#include <vespa/vespalib/util/gate.h> #include <vespa/searchlib/attribute/attributevector.h> #include <vespa/searchlib/common/isequencedtaskexecutor.h> diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/transport_latch.h b/searchcore/src/vespa/searchcore/proton/persistenceengine/transport_latch.h index 27b9ffa9b88..2fb55b14fdf 100644 --- a/searchcore/src/vespa/searchcore/proton/persistenceengine/transport_latch.h +++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/transport_latch.h @@ -4,7 +4,7 @@ #include <vespa/persistence/spi/result.h> #include <vespa/searchcore/proton/common/feedtoken.h> #include <vespa/vespalib/util/sequence.h> -#include <vespa/vespalib/util/sync.h> +#include <vespa/vespalib/util/count_down_latch.h> #include <mutex> namespace proton { diff --git a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp index 2ae8d826ebc..fad00fa00e6 100644 --- a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp @@ -6,7 +6,7 @@ #include "lid_space_compaction_job.h" #include <vespa/searchcore/proton/common/eventlogger.h> #include <vespa/searchlib/common/gatecallback.h> -#include <vespa/vespalib/util/sync.h> +#include <vespa/vespalib/util/gate.h> #include <cassert> #include <vespa/log/log.h> diff --git a/searchcore/src/vespa/searchcore/proton/server/packetwrapper.h b/searchcore/src/vespa/searchcore/proton/server/packetwrapper.h index 118f9601abd..6224b3b693a 100644 --- a/searchcore/src/vespa/searchcore/proton/server/packetwrapper.h +++ b/searchcore/src/vespa/searchcore/proton/server/packetwrapper.h @@ -4,7 +4,7 @@ #include "tls_replay_progress.h" #include <vespa/searchlib/transactionlog/common.h> -#include <vespa/vespalib/util/sync.h> +#include <vespa/vespalib/util/gate.h> namespace proton { /** diff --git a/searchcore/src/vespa/searchcore/proton/test/simple_job_tracker.h b/searchcore/src/vespa/searchcore/proton/test/simple_job_tracker.h index a80eb7b670e..648ca0aa8ad 100644 --- a/searchcore/src/vespa/searchcore/proton/test/simple_job_tracker.h +++ b/searchcore/src/vespa/searchcore/proton/test/simple_job_tracker.h @@ -2,7 +2,7 @@ #pragma once #include <vespa/searchcore/proton/metrics/i_job_tracker.h> -#include <vespa/vespalib/util/sync.h> +#include <vespa/vespalib/util/count_down_latch.h> namespace proton { namespace test { diff --git a/searchcorespi/src/vespa/searchcorespi/index/indexwriteutilities.cpp b/searchcorespi/src/vespa/searchcorespi/index/indexwriteutilities.cpp index 3d3231a5e09..c8b2e81a9c0 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/indexwriteutilities.cpp +++ b/searchcorespi/src/vespa/searchcorespi/index/indexwriteutilities.cpp @@ -7,6 +7,7 @@ #include <vespa/fastlib/io/bufferedfile.h> #include <vespa/vespalib/util/exceptions.h> #include <sstream> +#include <unistd.h> #include <vespa/log/log.h> LOG_SETUP(".searchcorespi.index.indexwriteutilities"); diff --git a/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.cpp b/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.cpp index cd0a4caf33b..75412dcf8e9 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.cpp +++ b/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.cpp @@ -117,7 +117,7 @@ WarmupIndexCollection::fireWarmup(Task::UP task) if (now < _warmupEndTime) { _executor.execute(std::move(task)); } else { - vespalib::LockGuard guard(_lock); + std::unique_lock<std::mutex> guard(_lock); if (_warmupEndTime != 0) { _warmupEndTime = 0; guard.unlock(); @@ -133,7 +133,7 @@ WarmupIndexCollection::handledBefore(uint32_t fieldId, const Node &term) const StringBase * sb(dynamic_cast<const StringBase *>(&term)); if (sb != NULL) { const vespalib::string & s = sb->getTerm(); - vespalib::LockGuard guard(_lock); + std::lock_guard<std::mutex> guard(_lock); TermMap::insert_result found = (*_handledTerms)[fieldId].insert(s); return ! found.second; } diff --git a/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.h b/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.h index 96f46610a29..f6d6bc89fc4 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.h +++ b/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.h @@ -99,7 +99,7 @@ private: vespalib::ThreadExecutor & _executor; IWarmupDone & _warmupDone; fastos::TimeStamp _warmupEndTime; - vespalib::Lock _lock; + std::mutex _lock; std::unique_ptr<FieldTermMap> _handledTerms; }; diff --git a/searchlib/src/tests/attribute/benchmark/attributebenchmark.cpp b/searchlib/src/tests/attribute/benchmark/attributebenchmark.cpp index 8eef498d85c..5722b7c90ca 100644 --- a/searchlib/src/tests/attribute/benchmark/attributebenchmark.cpp +++ b/searchlib/src/tests/attribute/benchmark/attributebenchmark.cpp @@ -8,11 +8,12 @@ #include <vespa/searchlib/attribute/singlestringattribute.h> #include <vespa/searchlib/attribute/multistringattribute.h> #include <vespa/searchlib/attribute/attrvector.h> +#include <vespa/fastos/thread.h> #include <vespa/fastos/app.h> #include <iostream> #include <fstream> -#include "../attributesearcher.h" -#include "../attributeupdater.h" +#include "attributesearcher.h" +#include "attributeupdater.h" #include <sys/resource.h> #include <vespa/log/log.h> @@ -21,8 +22,6 @@ LOG_SETUP("attributebenchmark"); #include <vespa/searchlib/attribute/attributevector.hpp> -using vespalib::Monitor; -using vespalib::MonitorGuard; using std::shared_ptr; typedef std::vector<uint32_t> NumVector; @@ -268,11 +267,11 @@ AttributeBenchmark::benchmarkSearch(const AttributePtr & ptr, const std::vector< for (uint32_t i = 0; i < _config._numSearchers; ++i) { if (_config._rangeSearch) { RangeSpec spec(_config._rangeStart, _config._rangeEnd, _config._rangeDelta); - searchers.push_back(new AttributeRangeSearcher(i, ptr, spec, _config._numQueries)); + searchers.push_back(new AttributeRangeSearcher(ptr, spec, _config._numQueries)); } else if (_config._prefixSearch) { - searchers.push_back(new AttributePrefixSearcher(i, ptr, prefixStrings, _config._numQueries)); + searchers.push_back(new AttributePrefixSearcher(ptr, prefixStrings, _config._numQueries)); } else { - searchers.push_back(new AttributeFindSearcher<T>(i, ptr, values, _config._numQueries)); + searchers.push_back(new AttributeFindSearcher<T>(ptr, values, _config._numQueries)); } _threadPool->NewThread(searchers.back()); } diff --git a/searchlib/src/tests/attribute/attributesearcher.h b/searchlib/src/tests/attribute/benchmark/attributesearcher.h index bd86f291f7b..f8cd614c48c 100644 --- a/searchlib/src/tests/attribute/attributesearcher.h +++ b/searchlib/src/tests/attribute/benchmark/attributesearcher.h @@ -2,7 +2,7 @@ #pragma once -#include "runnable.h" +#include <vespa/searchlib/util/runnable.h> #include <vespa/searchlib/attribute/attribute.h> #include <vespa/searchlib/attribute/attributeguard.h> #include <vespa/searchlib/queryeval/hitcollector.h> @@ -67,8 +67,8 @@ protected: AttributeSearcherStatus _status; public: - AttributeSearcher(uint32_t id, const AttributePtr & attrPtr) : - Runnable(id), _attrPtr(attrPtr), _timer(), _status() + AttributeSearcher(const AttributePtr & attrPtr) : + Runnable(), _attrPtr(attrPtr), _timer(), _status() { _status._numClients = 1; } @@ -108,9 +108,9 @@ private: std::vector<char> _query; public: - AttributeFindSearcher(uint32_t id, const AttributePtr & attrPtr, const std::vector<T> & values, + AttributeFindSearcher(const AttributePtr & attrPtr, const std::vector<T> & values, uint32_t numQueries) : - AttributeSearcher(id, attrPtr), _values(values), _query() + AttributeSearcher(attrPtr), _values(values), _query() { _status._numQueries = numQueries; } @@ -186,9 +186,9 @@ private: std::vector<char> _query; public: - AttributeRangeSearcher(uint32_t id, const AttributePtr & attrPtr, const RangeSpec & spec, + AttributeRangeSearcher(const AttributePtr & attrPtr, const RangeSpec & spec, uint32_t numQueries) : - AttributeSearcher(id, attrPtr), _spec(spec), _query() + AttributeSearcher(attrPtr), _spec(spec), _query() { _status._numQueries = numQueries; } @@ -228,9 +228,9 @@ private: std::vector<char> _query; public: - AttributePrefixSearcher(uint32_t id, const AttributePtr & attrPtr, + AttributePrefixSearcher(const AttributePtr & attrPtr, const std::vector<vespalib::string> & values, uint32_t numQueries) : - AttributeSearcher(id, attrPtr), _values(values), _query() + AttributeSearcher(attrPtr), _values(values), _query() { _status._numQueries = numQueries; } diff --git a/searchlib/src/tests/attribute/attributeupdater.h b/searchlib/src/tests/attribute/benchmark/attributeupdater.h index 3f41b79f331..13360e58b2d 100644 --- a/searchlib/src/tests/attribute/attributeupdater.h +++ b/searchlib/src/tests/attribute/benchmark/attributeupdater.h @@ -3,7 +3,7 @@ #pragma once #include <vespa/searchlib/util/randomgenerator.h> -#include "runnable.h" +#include <vespa/searchlib/util/runnable.h> #include <vespa/searchlib/attribute/attribute.h> #define VALIDATOR_STR(str) #str @@ -169,7 +169,7 @@ AttributeUpdaterThread<Vector, T, BT>::AttributeUpdaterThread(const AttributePtr RandomGenerator & rndGen, bool validate, uint32_t commitFreq, uint32_t minValueCount, uint32_t maxValueCount) : AttributeUpdater<Vector, T, BT>(attrPtr, values, rndGen, validate, commitFreq, minValueCount, maxValueCount), - Runnable(0) + Runnable() {} template <typename Vector, typename T, typename BT> AttributeUpdaterThread<Vector, T, BT>::~AttributeUpdaterThread() { } diff --git a/searchlib/src/tests/attribute/runnable.h b/searchlib/src/tests/attribute/runnable.h deleted file mode 100644 index b95d7d3843f..00000000000 --- a/searchlib/src/tests/attribute/runnable.h +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -#include <vespa/vespalib/util/sync.h> -#include <vespa/fastos/thread.h> - -namespace search { - -class Runnable : public FastOS_Runnable -{ -protected: - uint32_t _id; - vespalib::Monitor _cond; - bool _done; - bool _stopped; - -public: - Runnable(uint32_t id) : - _id(id), _cond(), _done(false), _stopped(false) - { } - void Run(FastOS_ThreadInterface *, void *) override { - doRun(); - - vespalib::MonitorGuard guard(_cond); - _stopped = true; - guard.broadcast(); - } - virtual void doRun() = 0; - void stop() { - vespalib::MonitorGuard guard(_cond); - _done = true; - } - void join() { - vespalib::MonitorGuard guard(_cond); - while (!_stopped) { - guard.wait(); - } - } -}; - -} // search - diff --git a/searchlib/src/vespa/searchlib/attribute/attributecontext.cpp b/searchlib/src/vespa/searchlib/attribute/attributecontext.cpp index 770d5c653a3..0a3fe963aae 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributecontext.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attributecontext.cpp @@ -43,19 +43,19 @@ AttributeContext::~AttributeContext() { } const IAttributeVector * AttributeContext::getAttribute(const string & name) const { - vespalib::LockGuard guard(_cacheLock); + std::lock_guard<std::mutex> guard(_cacheLock); return getAttribute(_attributes, name, false); } const IAttributeVector * AttributeContext::getAttributeStableEnum(const string & name) const { - vespalib::LockGuard guard(_cacheLock); + std::lock_guard<std::mutex> guard(_cacheLock); return getAttribute(_enumAttributes, name, true); } void AttributeContext::releaseEnumGuards() { - vespalib::LockGuard guard(_cacheLock); + std::lock_guard<std::mutex> guard(_cacheLock); _enumAttributes.clear(); } diff --git a/searchlib/src/vespa/searchlib/attribute/attributecontext.h b/searchlib/src/vespa/searchlib/attribute/attributecontext.h index b7373f343ec..80abe84f8ef 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributecontext.h +++ b/searchlib/src/vespa/searchlib/attribute/attributecontext.h @@ -3,9 +3,9 @@ #pragma once #include <vespa/searchcommon/attribute/iattributecontext.h> -#include <vespa/vespalib/util/sync.h> #include <vespa/vespalib/stllike/hash_map.h> #include "iattributemanager.h" +#include <mutex> namespace search { @@ -21,7 +21,7 @@ private: const search::IAttributeManager & _manager; mutable AttributeMap _attributes; mutable AttributeMap _enumAttributes; - mutable vespalib::Lock _cacheLock; + mutable std::mutex _cacheLock; const attribute::IAttributeVector * getAttribute(AttributeMap & map, const string & name, bool stableEnum) const; diff --git a/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp b/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp index 0122516b767..f10f3491ac0 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp @@ -12,14 +12,14 @@ #include <vespa/log/log.h> LOG_SETUP(".searchlib.attributemanager"); -using vespalib::LockGuard; using vespalib::string; using vespalib::IllegalStateException; using search::attribute::IAttributeContext; namespace { -vespalib::Monitor baseDirMonitor; +std::mutex baseDirLock; +std::condition_variable baseDirCond; typedef std::set<string> BaseDirSet; BaseDirSet baseDirSet; @@ -27,7 +27,7 @@ static void waitBaseDir(const string &baseDir) { if (baseDir.empty()) { return; } - vespalib::MonitorGuard guard(baseDirMonitor); + std::unique_lock<std::mutex> guard(baseDirLock); bool waited = false; BaseDirSet::iterator it = baseDirSet.find(baseDir); @@ -36,7 +36,7 @@ waitBaseDir(const string &baseDir) waited = true; LOG(debug, "AttributeManager: Waiting for basedir %s to be available", baseDir.c_str()); } - guard.wait(); + baseDirCond.wait(guard); it = baseDirSet.find(baseDir); } @@ -52,7 +52,7 @@ dropBaseDir(const string &baseDir) { if (baseDir.empty()) return; - vespalib::MonitorGuard guard(baseDirMonitor); + std::lock_guard<std::mutex> guard(baseDirLock); BaseDirSet::iterator it = baseDirSet.find(baseDir); if (it == baseDirSet.end()) { @@ -60,7 +60,7 @@ dropBaseDir(const string &baseDir) } else { baseDirSet.erase(it); } - guard.broadcast(); + baseDirCond.notify_all(); } } @@ -147,7 +147,7 @@ AttributeManager::findAndLoadAttribute(const string & name) const if (found != _attributes.end()) { AttributeVector & vec = *found->second; if ( ! vec.isLoaded() ) { - vespalib::LockGuard loadGuard(_loadLock); + std::lock_guard<std::mutex> loadGuard(_loadLock); if ( ! vec.isLoaded() ) { vec.load(); } else { diff --git a/searchlib/src/vespa/searchlib/attribute/attributemanager.h b/searchlib/src/vespa/searchlib/attribute/attributemanager.h index f69d4cb9295..3b23881e3bc 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributemanager.h +++ b/searchlib/src/vespa/searchlib/attribute/attributemanager.h @@ -7,7 +7,7 @@ #include <vespa/searchlib/common/indexmetainfo.h> #include <vespa/searchcommon/attribute/config.h> #include <vespa/vespalib/stllike/hash_map.h> -#include <vespa/vespalib/util/sync.h> +#include <mutex> namespace search { @@ -56,7 +56,7 @@ public: protected: typedef vespalib::hash_map<string, VectorHolder> AttributeMap; AttributeMap _attributes; - vespalib::Lock _loadLock; + mutable std::mutex _loadLock; private: const VectorHolder * findAndLoadAttribute(const string & name) const; string createBaseFileName(const string & name, bool useSnapshot) const; diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.h b/searchlib/src/vespa/searchlib/attribute/attributevector.h index 13ba3f801b7..b514a5830ba 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributevector.h +++ b/searchlib/src/vespa/searchlib/attribute/attributevector.h @@ -19,8 +19,7 @@ #include <vespa/searchlib/queryeval/searchiterator.h> #include <vespa/vespalib/objects/identifiable.h> #include <vespa/vespalib/stllike/asciistream.h> -#include <vespa/vespalib/util/rwlock.h> -#include <vespa/vespalib/util/sync.h> +#include <vespa/fastos/time.h> #include <cmath> #include <mutex> #include <shared_mutex> diff --git a/searchlib/src/vespa/searchlib/common/bitvectorcache.cpp b/searchlib/src/vespa/searchlib/common/bitvectorcache.cpp index 04909da8ea7..a063ed347a4 100644 --- a/searchlib/src/vespa/searchlib/common/bitvectorcache.cpp +++ b/searchlib/src/vespa/searchlib/common/bitvectorcache.cpp @@ -29,7 +29,7 @@ BitVectorCache::computeCountVector(KeySet & keys, CountVector & v) const std::vector<CondensedBitVector::KeySet> keySets; ChunkV chunks; { - vespalib::LockGuard guard(_lock); + std::lock_guard<std::mutex> guard(_lock); keySets.resize(_chunks.size()); Key2Index::const_iterator end(_keys.end()); for (Key k : keys) { @@ -61,7 +61,7 @@ BitVectorCache::KeySet BitVectorCache::lookupCachedSet(const KeyAndCountSet & keys) { KeySet cached(keys.size()*3); - vespalib::LockGuard guard(_lock); + std::lock_guard<std::mutex> guard(_lock); _lookupCount++; if (_lookupCount == 2000) { _needPopulation = true; @@ -101,7 +101,7 @@ BitVectorCache::getSorted(Key2Index & keys) } bool -BitVectorCache::hasCostChanged(const vespalib::LockGuard & guard) +BitVectorCache::hasCostChanged(const std::lock_guard<std::mutex> & guard) { (void) guard; if ( ! _chunks.empty()) { @@ -163,17 +163,17 @@ BitVectorCache::populate(Key2Index & newKeys, CondensedBitVector & chunk, const void BitVectorCache::populate(uint32_t sz, const PopulateInterface & lookup) { - vespalib::LockGuard guard1(_lock); + std::unique_lock<std::mutex> guard(_lock); if (! _needPopulation) { return; } Key2Index newKeys(_keys); - guard1.unlock(); + guard.unlock(); CondensedBitVector::UP chunk(CondensedBitVector::create(sz, _genHolder)); populate(newKeys, *chunk, lookup); - vespalib::LockGuard guard2(_lock); + guard.lock(); _chunks.push_back(std::move(chunk)); _keys.swap(newKeys); _needPopulation = false; @@ -182,7 +182,7 @@ BitVectorCache::populate(uint32_t sz, const PopulateInterface & lookup) void BitVectorCache::set(Key key, uint32_t index, bool v) { - vespalib::LockGuard guard(_lock); + std::lock_guard<std::mutex> guard(_lock); auto found = _keys.find(key); if (found != _keys.end()) { const KeyMeta & m(found->second); @@ -202,7 +202,7 @@ BitVectorCache::get(Key key, uint32_t index) const void BitVectorCache::removeIndex(uint32_t index) { - vespalib::LockGuard guard(_lock); + std::lock_guard<std::mutex> guard(_lock); for (auto & chunk : _chunks) { chunk->clearIndex(index); } diff --git a/searchlib/src/vespa/searchlib/common/bitvectorcache.h b/searchlib/src/vespa/searchlib/common/bitvectorcache.h index 0d27701e1e1..c1415d9130f 100644 --- a/searchlib/src/vespa/searchlib/common/bitvectorcache.h +++ b/searchlib/src/vespa/searchlib/common/bitvectorcache.h @@ -2,9 +2,9 @@ #pragma once #include "condensedbitvectors.h" -#include <vespa/vespalib/util/sync.h> #include <vespa/vespalib/stllike/hash_set.h> #include <vespa/fastos/dynamiclibrary.h> +#include <mutex> namespace search { @@ -74,11 +74,11 @@ private: VESPA_DLL_LOCAL static SortedKeyMeta getSorted(Key2Index & keys); VESPA_DLL_LOCAL static void populate(Key2Index & newKeys, CondensedBitVector & chunk, const PopulateInterface & lookup); - VESPA_DLL_LOCAL bool hasCostChanged(const vespalib::LockGuard &); + VESPA_DLL_LOCAL bool hasCostChanged(const std::lock_guard<std::mutex> &); uint64_t _lookupCount; bool _needPopulation; - vespalib::Lock _lock; + mutable std::mutex _lock; Key2Index _keys; ChunkV _chunks; GenerationHolder &_genHolder; diff --git a/searchlib/src/vespa/searchlib/common/gatecallback.cpp b/searchlib/src/vespa/searchlib/common/gatecallback.cpp index a853909be71..29346d7ad9c 100644 --- a/searchlib/src/vespa/searchlib/common/gatecallback.cpp +++ b/searchlib/src/vespa/searchlib/common/gatecallback.cpp @@ -1,7 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "gatecallback.h" -#include <vespa/vespalib/util/sync.h> +#include <vespa/vespalib/util/gate.h> namespace search { diff --git a/searchlib/src/vespa/searchlib/common/sortspec.cpp b/searchlib/src/vespa/searchlib/common/sortspec.cpp index f43596f9c7f..694443b00ba 100644 --- a/searchlib/src/vespa/searchlib/common/sortspec.cpp +++ b/searchlib/src/vespa/searchlib/common/sortspec.cpp @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "sortspec.h" #include <vespa/vespalib/util/stringfmt.h> -#include <vespa/vespalib/util/sync.h> #include <vespa/fastlib/text/normwordfolder.h> #include <vespa/vespalib/text/utf8.h> diff --git a/searchlib/src/vespa/searchlib/uca/ucaconverter.cpp b/searchlib/src/vespa/searchlib/uca/ucaconverter.cpp index db16d165cec..47d66a94d9e 100644 --- a/searchlib/src/vespa/searchlib/uca/ucaconverter.cpp +++ b/searchlib/src/vespa/searchlib/uca/ucaconverter.cpp @@ -2,8 +2,8 @@ #include "ucaconverter.h" #include <vespa/vespalib/util/stringfmt.h> -#include <vespa/vespalib/util/sync.h> #include <vespa/vespalib/text/utf8.h> +#include <mutex> #include <vespa/log/log.h> LOG_SETUP(".search.common.sortspec"); @@ -14,7 +14,7 @@ using vespalib::ConstBufferRef; using vespalib::make_string; namespace { - vespalib::Lock _GlobalDirtyICUThreadSafeLock; +std::mutex _GlobalDirtyICUThreadSafeLock; } BlobConverter::UP @@ -30,7 +30,7 @@ UcaConverter::UcaConverter(vespalib::stringref locale, vespalib::stringref stren UErrorCode status = U_ZERO_ERROR; Collator *coll(NULL); { - vespalib::LockGuard guard(_GlobalDirtyICUThreadSafeLock); + std::lock_guard<std::mutex> guard(_GlobalDirtyICUThreadSafeLock); coll = Collator::createInstance(icu::Locale(locale.c_str()), status); } if(U_SUCCESS(status)) { diff --git a/searchlib/src/vespa/searchlib/util/runnable.h b/searchlib/src/vespa/searchlib/util/runnable.h index a8dc8e56856..a95d4d68368 100644 --- a/searchlib/src/vespa/searchlib/util/runnable.h +++ b/searchlib/src/vespa/searchlib/util/runnable.h @@ -2,37 +2,39 @@ #pragma once -#include <vespa/vespalib/util/sync.h> +#include <mutex> +#include <condition_variable> namespace search { class Runnable : public FastOS_Runnable { protected: - vespalib::Monitor _cond; - bool _done; - bool _stopped; + std::mutex _lock; + std::condition_variable _cond; + bool _done; + bool _stopped; public: Runnable() : - _cond(), _done(false), _stopped(false) + _lock(), _cond(), _done(false), _stopped(false) { } - void Run(FastOS_ThreadInterface *, void *) override{ + void Run(FastOS_ThreadInterface *, void *) override { doRun(); - vespalib::MonitorGuard guard(_cond); + std::lock_guard<std::mutex> guard(_lock); _stopped = true; - guard.broadcast(); + _cond.notify_all(); } virtual void doRun() = 0; void stop() { - vespalib::MonitorGuard guard(_cond); + std::lock_guard<std::mutex> guard(_lock); _done = true; } void join() { - vespalib::MonitorGuard guard(_cond); + std::unique_lock<std::mutex> guard(_lock); while (!_stopped) { - guard.wait(); + _cond.wait(guard); } } }; diff --git a/storage/src/vespa/storage/config/bucketspaces.def b/storage/src/vespa/storage/config/bucketspaces.def index 3ed1abba0b4..4db107ec1ee 100644 --- a/storage/src/vespa/storage/config/bucketspaces.def +++ b/storage/src/vespa/storage/config/bucketspaces.def @@ -9,3 +9,6 @@ documenttype[].name string ## The bucket space this document type belongs to. documenttype[].bucketspace string + +## Switch to enable multiple bucket spaces in content layer and content nodes. +enable_multiple_bucket_spaces bool default=false diff --git a/streamingvisitors/src/vespa/searchvisitor/searchenvironment.cpp b/streamingvisitors/src/vespa/searchvisitor/searchenvironment.cpp index 64c2c9a6429..7ebb71f1855 100644 --- a/streamingvisitors/src/vespa/searchvisitor/searchenvironment.cpp +++ b/streamingvisitors/src/vespa/searchvisitor/searchenvironment.cpp @@ -15,9 +15,7 @@ __thread SearchEnvironment::EnvMap * SearchEnvironment::_localEnvMap=0; SearchEnvironment::Env::Env(const vespalib::string & muffens, const config::ConfigUri & configUri, Fast_NormalizeWordFolder & wf) : _configId(configUri.getConfigId()), - _configurer(config::SimpleConfigRetriever::UP( - new config::SimpleConfigRetriever(createKeySet(configUri.getConfigId()), configUri.getContext())), - this), + _configurer(std::make_unique<config::SimpleConfigRetriever>(createKeySet(configUri.getConfigId()), configUri.getContext()), this), _vsmAdapter(new VSMAdapter(muffens, _configId, wf)), _rankManager(new RankManager(_vsmAdapter.get())) { @@ -63,13 +61,15 @@ SearchEnvironment::~SearchEnvironment() _threadLocals.clear(); } -SearchEnvironment::Env & SearchEnvironment::getEnv(const vespalib::string & searchCluster) +SearchEnvironment::Env & +SearchEnvironment::getEnv(const vespalib::string & searchCluster) { config::ConfigUri searchClusterUri(_configUri.createWithNewId(searchCluster)); if (_localEnvMap == NULL) { - _localEnvMap = new EnvMap; + EnvMapUP envMap = std::make_unique<EnvMap>(); + _localEnvMap = envMap.get(); vespalib::LockGuard guard(_lock); - _threadLocals.push_back(EnvMapUP(_localEnvMap)); + _threadLocals.emplace_back(std::move(envMap)); } EnvMap::iterator localFound = _localEnvMap->find(searchCluster); if (localFound == _localEnvMap->end()) { @@ -77,7 +77,8 @@ SearchEnvironment::Env & SearchEnvironment::getEnv(const vespalib::string & sear EnvMap::iterator found = _envMap.find(searchCluster); if (found == _envMap.end()) { LOG(debug, "Init VSMAdapter with config id = '%s'", searchCluster.c_str()); - _envMap[searchCluster].reset(new Env("*", searchClusterUri, _wordFolder)); + Env::SP env = std::make_shared<Env>("*", searchClusterUri, _wordFolder); + _envMap[searchCluster] = std::move(env); found = _envMap.find(searchCluster); } _localEnvMap->insert(*found); diff --git a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp index 51c01a40fac..e3483e8beac 100644 --- a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp +++ b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp @@ -31,6 +31,8 @@ using search::attribute::IAttributeVector; using search::aggregation::HitsAggregationResult; using search::expression::ConfigureStaticParams; using vdslib::Parameters; +using document::PositionDataType; +using document::DataType; class ForceWordfolderInit { @@ -52,25 +54,25 @@ static ForceWordfolderInit _G_forceNormWordFolderInit; AttributeVector::SP createMultiValueAttribute(const vespalib::string & name, const document::FieldValue & fv, bool arrayType) { - const document::DataType * ndt = fv.getDataType(); + const DataType * ndt = fv.getDataType(); if (ndt->inherits(document::CollectionDataType::classId)) { ndt = &(static_cast<const document::CollectionDataType *>(ndt))->getNestedType(); } LOG(debug, "Create %s attribute '%s' with data type '%s' (%s)", arrayType ? "array" : "weighted set", name.c_str(), ndt->getName().c_str(), fv.getClass().name()); AttributeVector::SP attr; - if (ndt->getId() == document::DataType::T_BYTE || - ndt->getId() == document::DataType::T_INT || - ndt->getId() == document::DataType::T_LONG) + if (ndt->getId() == DataType::T_BYTE || + ndt->getId() == DataType::T_INT || + ndt->getId() == DataType::T_LONG) { attr.reset(arrayType ? static_cast<AttributeVector *>(new search::MultiIntegerExtAttribute(name)) : static_cast<AttributeVector *>(new search::WeightedSetIntegerExtAttribute(name))); - } else if (ndt->getId() == document::DataType::T_DOUBLE || - ndt->getId() == document::DataType::T_FLOAT) + } else if (ndt->getId() == DataType::T_DOUBLE || + ndt->getId() == DataType::T_FLOAT) { attr.reset(arrayType ? static_cast<AttributeVector *>(new search::MultiFloatExtAttribute(name)) : static_cast<AttributeVector *>(new search::WeightedSetFloatExtAttribute(name))); - } else if (ndt->getId() == document::DataType::T_STRING) { + } else if (ndt->getId() == DataType::T_STRING) { attr.reset(arrayType ? static_cast<AttributeVector *>(new search::MultiStringExtAttribute(name)) : static_cast<AttributeVector *>(new search::WeightedSetStringExtAttribute(name))); } else { @@ -204,27 +206,26 @@ void SearchVisitor::init(const Parameters & params) _attrMan.add(_rankAttributeBacking); Parameters::ValueRef valueRef; if ( params.get("summaryclass", valueRef) ) { - _summaryClass = vespalib::string(static_cast<const char *>(valueRef.data()), - static_cast<unsigned>(valueRef.size())); + _summaryClass = vespalib::string(valueRef.data(), valueRef.size()); LOG(debug, "Received summary class: %s", _summaryClass.c_str()); } size_t wantedSummaryCount(10); if (params.get("summarycount", valueRef) ) { - vespalib::string tmp(static_cast<const char *>(valueRef.data()), valueRef.size()); + vespalib::string tmp(valueRef.data(), valueRef.size()); wantedSummaryCount = strtoul(tmp.c_str(), NULL, 0); LOG(debug, "Received summary count: %ld", wantedSummaryCount); } _queryResult->getSearchResult().setWantedHitCount(wantedSummaryCount); if (params.get("rankprofile", valueRef) ) { - vespalib::string tmp(static_cast<const char *>(valueRef.data()), valueRef.size()); + vespalib::string tmp(valueRef.data(), valueRef.size()); _rankController.setRankProfile(tmp); LOG(debug, "Received rank profile: %s", _rankController.getRankProfile().c_str()); } if (params.get("queryflags", valueRef) ) { - vespalib::string tmp(static_cast<const char *>(valueRef.data()), valueRef.size()); + vespalib::string tmp(valueRef.data(), valueRef.size()); LOG(debug, "Received query flags: 0x%lx", strtoul(tmp.c_str(), NULL, 0)); uint32_t queryFlags = strtoul(tmp.c_str(), NULL, 0); _rankController.setDumpFeatures((queryFlags & search::fs4transport::QFLAG_DUMP_FEATURES) != 0); @@ -234,7 +235,7 @@ void SearchVisitor::init(const Parameters & params) if (params.get("rankproperties", valueRef) && valueRef.size() > 0) { LOG(spam, "Received rank properties of %zd bytes", valueRef.size()); uint32_t len = static_cast<uint32_t>(valueRef.size()); - char * data = const_cast<char *>(static_cast<const char *>(valueRef.data())); + char * data = const_cast<char *>(valueRef.data()); FNET_DataBuffer src(data, len); uint32_t cnt = src.ReadInt32(); len -= sizeof(uint32_t); @@ -259,7 +260,7 @@ void SearchVisitor::init(const Parameters & params) } if (params.get("rankprofile", valueRef)) { - vespalib::string tmp(static_cast<const char *>(valueRef.data()), valueRef.size()); + vespalib::string tmp(valueRef.data(), valueRef.size()); _summaryGenerator.getDocsumState()._args.SetRankProfile(tmp); } @@ -270,7 +271,7 @@ void SearchVisitor::init(const Parameters & params) vespalib::string location; if (params.get("location", valueRef)) { - location = vespalib::string(static_cast<const char *>(valueRef.data()), valueRef.size()); + location = vespalib::string(valueRef.data(), valueRef.size()); LOG(debug, "Location = '%s'", location.c_str()); _summaryGenerator.getDocsumState()._args.SetLocation(valueRef.size(), (const char*)valueRef.data()); } @@ -278,14 +279,12 @@ void SearchVisitor::init(const Parameters & params) Parameters::ValueRef searchClusterBlob; if (params.get("searchcluster", searchClusterBlob)) { LOG(spam, "Received searchcluster blob of %zd bytes", searchClusterBlob.size()); - vespalib::string searchCluster(static_cast<const char *>(searchClusterBlob.data()), searchClusterBlob.size()); + vespalib::string searchCluster(searchClusterBlob.data(), searchClusterBlob.size()); _vsmAdapter = _env.getVSMAdapter(searchCluster); if ( params.get("sort", valueRef) ) { search::uca::UcaConverterFactory ucaFactory; - _sortSpec = search::common::SortSpec(vespalib::string(static_cast<const char *>(valueRef.data()), - static_cast<unsigned>(valueRef.size())), - ucaFactory); + _sortSpec = search::common::SortSpec(vespalib::string(valueRef.data(), valueRef.size()), ucaFactory); LOG(debug, "Received sort specification: '%s'", _sortSpec.getSpec().c_str()); } @@ -388,16 +387,16 @@ SearchVisitor::AttributeInserter::onPrimitive(uint32_t, const Content & c) } } -SearchVisitor::AttributeInserter::AttributeInserter(search::AttributeVector & attribute, search::AttributeVector::DocId docId) : +SearchVisitor::AttributeInserter::AttributeInserter(AttributeVector & attribute, AttributeVector::DocId docId) : _attribute(attribute), _docId(docId) { } -SearchVisitor::PositionInserter::PositionInserter(search::AttributeVector & attribute, search::AttributeVector::DocId docId) : +SearchVisitor::PositionInserter::PositionInserter(AttributeVector & attribute, AttributeVector::DocId docId) : AttributeInserter(attribute, docId), - _fieldX(document::PositionDataType::getInstance().getField(document::PositionDataType::FIELD_X)), - _fieldY(document::PositionDataType::getInstance().getField(document::PositionDataType::FIELD_Y)) + _fieldX(PositionDataType::getInstance().getField(PositionDataType::FIELD_X)), + _fieldY(PositionDataType::getInstance().getField(PositionDataType::FIELD_Y)) { } @@ -605,7 +604,7 @@ void SearchVisitor::SyntheticFieldsController::onDocumentMatch(StorageDocument & document, const vespalib::string & documentId) { - document.setField(_documentIdFId, document::FieldValue::UP(new document::StringFieldValue(documentId))); + document.setField(_documentIdFId, std::make_unique<document::StringFieldValue>(documentId)); } void @@ -617,8 +616,8 @@ SearchVisitor::registerAdditionalFields(const std::vector<vsm::DocsumTools::Fiel const std::vector<vespalib::string> & inputNames = spec.getInputNames(); for (size_t j = 0; j < inputNames.size(); ++j) { fieldList.push_back(inputNames[j]); - if (document::PositionDataType::isZCurveFieldName(inputNames[j])) { - fieldList.push_back(document::PositionDataType::cutZCurveFieldName(inputNames[j])); + if (PositionDataType::isZCurveFieldName(inputNames[j])) { + fieldList.push_back(PositionDataType::cutZCurveFieldName(inputNames[j])); } } } @@ -991,16 +990,16 @@ SearchVisitor::fillAttributeVectors(const vespalib::string & documentId, const S { for (const AttrInfo & finfo : _attributeFields) { const AttributeGuard &finfoGuard(*finfo._attr); - bool isPosition = finfoGuard->getClass().inherits(search::IntegerAttribute::classId) && document::PositionDataType::isZCurveFieldName(finfoGuard->getName()); + bool isPosition = finfoGuard->getClass().inherits(search::IntegerAttribute::classId) && PositionDataType::isZCurveFieldName(finfoGuard->getName()); LOG(debug, "Filling attribute '%s', isPosition='%s'", finfoGuard->getName().c_str(), isPosition ? "true" : "false"); uint32_t fieldId = finfo._field; if (isPosition) { - vespalib::stringref org = document::PositionDataType::cutZCurveFieldName(finfoGuard->getName()); + vespalib::stringref org = PositionDataType::cutZCurveFieldName(finfoGuard->getName()); fieldId = _fieldsUnion.find(org)->second; } const StorageDocument::SubDocument & subDoc = document.getComplexField(fieldId); - search::AttributeVector & attrV = const_cast<search::AttributeVector & >(*finfoGuard); - search::AttributeVector::DocId docId(0); + AttributeVector & attrV = const_cast<AttributeVector & >(*finfoGuard); + AttributeVector::DocId docId(0); attrV.addDoc(docId); if (subDoc.getFieldValue() != NULL) { LOG(debug, "value = '%s'", subDoc.getFieldValue()->toString().c_str()); diff --git a/travis/travis-build-full.sh b/travis/travis-build-full.sh index fc0efb843aa..53e174b534b 100755 --- a/travis/travis-build-full.sh +++ b/travis/travis-build-full.sh @@ -6,7 +6,7 @@ export SOURCE_DIR=/source export NUM_THREADS=6 export MALLOC_ARENA_MAX=1 export MAVEN_OPTS="-Xms128m -Xmx2g" -source /etc/profile.d/devtoolset-6.sh || true +source /etc/profile.d/devtoolset-7.sh || true ccache --max-size=1250M ccache --set-config=compression=true diff --git a/vbench/src/vbench/core/dispatcher.h b/vbench/src/vbench/core/dispatcher.h index 6336df17965..212cf04a06e 100644 --- a/vbench/src/vbench/core/dispatcher.h +++ b/vbench/src/vbench/core/dispatcher.h @@ -6,6 +6,7 @@ #include "provider.h" #include "closeable.h" #include <vespa/vespalib/util/sync.h> +#include <vespa/vespalib/util/gate.h> #include <vector> namespace vbench { diff --git a/vespalib/src/vespa/vespalib/testkit/test_hook.h b/vespalib/src/vespa/vespalib/testkit/test_hook.h index 336e965b0b1..28419c0b31b 100644 --- a/vespalib/src/vespa/vespalib/testkit/test_hook.h +++ b/vespalib/src/vespa/vespalib/testkit/test_hook.h @@ -2,7 +2,7 @@ #pragma once -#include <vespa/vespalib/util/sync.h> +#include <vespa/vespalib/util/count_down_latch.h> #include <vespa/vespalib/util/barrier.h> #include <string> #include <vector> diff --git a/vespalib/src/vespa/vespalib/testkit/time_bomb.h b/vespalib/src/vespa/vespalib/testkit/time_bomb.h index 5b39e27db79..8412e4b8661 100644 --- a/vespalib/src/vespa/vespalib/testkit/time_bomb.h +++ b/vespalib/src/vespa/vespalib/testkit/time_bomb.h @@ -2,7 +2,7 @@ #pragma once -#include <vespa/vespalib/util/sync.h> +#include <vespa/vespalib/util/gate.h> #include <thread> namespace vespalib { diff --git a/vespalib/src/vespa/vespalib/util/count_down_latch.h b/vespalib/src/vespa/vespalib/util/count_down_latch.h new file mode 100644 index 00000000000..66ef1e44cee --- /dev/null +++ b/vespalib/src/vespa/vespalib/util/count_down_latch.h @@ -0,0 +1,95 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <mutex> +#include <condition_variable> +#include <chrono> + +namespace vespalib { + +/** + * A countdown latch helps one or more threads wait for the completion + * of a number of operations performed by other threads. Specifically, + * any thread invoking the await method will block until the countDown + * method has been invoked an appropriate number of times. The + * countdown latch is created with a count. Each invocation of + * countDown will reduce the current count. When the count reaches 0, + * the threads blocked in await will be unblocked. When the count is + * 0, additional invocations of await will not block and additional + * invocations of countDown will have no effect. + **/ +class CountDownLatch +{ +private: + std::mutex _lock; + std::condition_variable _cond; + uint32_t _count; + + CountDownLatch(const CountDownLatch &rhs) = delete; + CountDownLatch(CountDownLatch &&rhs) = delete; + CountDownLatch &operator=(const CountDownLatch &rhs) = delete; + CountDownLatch &operator=(CountDownLatch &&rhs) = delete; + +public: + /** + * Create a countdown latch with the given initial count. + * + * @param cnt initial count + **/ + CountDownLatch(uint32_t cnt) : _lock(), _cond(), _count(cnt) {} + + /** + * Count down this latch. When the count reaches 0, all threads + * blocked in the await method will be unblocked. + **/ + void countDown() { + std::lock_guard<std::mutex> guard(_lock); + if (_count != 0) { + --_count; + if (_count == 0) { + _cond.notify_all(); + } + } + } + + /** + * Wait for this latch to count down to 0. This method will block + * until the countDown method has been invoked enough times to + * reduce the count to 0. + **/ + void await() { + std::unique_lock<std::mutex> guard(_lock); + _cond.wait(guard, [this]() { return (_count == 0); }); + } + + /** + * Wait for this latch to count down to 0. This method will block + * until the countDown method has been invoked enough times to + * reduce the count to 0 or the given amount of time has elapsed. + * + * @param maxwait the maximum number of milliseconds to wait + * @return true if the counter reached 0, false if we timed out + **/ + bool await(int maxwait) { + auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(maxwait); + std::unique_lock<std::mutex> guard(_lock); + return _cond.wait_until(guard, deadline, [this]() { return (_count == 0); }); + } + + /** + * Obtain the current count for this latch. This method is mostly + * useful for debugging and testing. + * + * @return current count + **/ + uint32_t getCount() const { return _count; } + + /** + * Empty. Needs to be virtual to reduce compiler warnings. + **/ + virtual ~CountDownLatch() = default; +}; + +} // namespace vespalib + diff --git a/vespalib/src/vespa/vespalib/util/gate.h b/vespalib/src/vespa/vespalib/util/gate.h new file mode 100644 index 00000000000..7d913a7a039 --- /dev/null +++ b/vespalib/src/vespa/vespalib/util/gate.h @@ -0,0 +1,22 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "count_down_latch.h" + +namespace vespalib { + +/** + * A gate is a countdown latch with an initial count of 1, indicating + * that we are only waiting for a single operation to complete. + **/ +class Gate : public CountDownLatch +{ +public: + /** + * Sets the initial count to 1. + **/ + Gate() : CountDownLatch(1) {} +}; + +} // namespace vespalib diff --git a/vespalib/src/vespa/vespalib/util/simple_thread_bundle.h b/vespalib/src/vespa/vespalib/util/simple_thread_bundle.h index 8fb7e1d04fc..4fa73c1112f 100644 --- a/vespalib/src/vespa/vespalib/util/simple_thread_bundle.h +++ b/vespalib/src/vespa/vespalib/util/simple_thread_bundle.h @@ -3,6 +3,7 @@ #pragma once #include "sync.h" +#include "count_down_latch.h" #include "thread.h" #include "runnable.h" #include "thread_bundle.h" diff --git a/vespalib/src/vespa/vespalib/util/sync.h b/vespalib/src/vespa/vespalib/util/sync.h index c3d8ea86aec..e3f3d44122c 100644 --- a/vespalib/src/vespa/vespalib/util/sync.h +++ b/vespalib/src/vespa/vespalib/util/sync.h @@ -527,108 +527,5 @@ public: } }; - -/** - * A countdown latch helps one or more threads wait for the completion - * of a number of operations performed by other threads. Specifically, - * any thread invoking the await method will block until the countDown - * method has been invoked an appropriate number of times. The - * countdown latch is created with a count. Each invocation of - * countDown will reduce the current count. When the count reaches 0, - * the threads blocked in await will be unblocked. When the count is - * 0, additional invocations of await will not block and additional - * invocations of countDown will have no effect. - **/ -class CountDownLatch -{ -private: - Monitor _monitor; - uint32_t _count; - - CountDownLatch(const CountDownLatch &rhs) = delete; - CountDownLatch &operator=(const CountDownLatch &rhs) = delete; - -public: - /** - * Create a countdown latch with the given initial count. - * - * @param cnt initial count - **/ - CountDownLatch(uint32_t cnt) : _monitor(), _count(cnt) {} - - /** - * Count down this latch. When the count reaches 0, all threads - * blocked in the await method will be unblocked. - **/ - void countDown() { - MonitorGuard guard(_monitor); - if (_count == 0) { - return; - } - --_count; - if (_count == 0) { - guard.broadcast(); - } - } - - /** - * Wait for this latch to count down to 0. This method will block - * until the countDown method has been invoked enough times to - * reduce the count to 0. - **/ - void await() { - MonitorGuard guard(_monitor); - while (_count != 0) { - guard.wait(); - } - } - - /** - * Wait for this latch to count down to 0. This method will block - * until the countDown method has been invoked enough times to - * reduce the count to 0 or the given amount of time has elapsed. - * - * @param maxwait the maximum number of milliseconds to wait - * @return true if the counter reached 0, false if we timed out - **/ - bool await(int maxwait) { - MonitorGuard guard(_monitor); - TimedWaiter waiter(guard, maxwait); - while (_count != 0 && waiter.hasTime()) { - waiter.wait(); - } - return (_count == 0); - } - - /** - * Obtain the current count for this latch. This method is mostly - * useful for debugging and testing. - * - * @return current count - **/ - uint32_t getCount() const { - return _count; - } - - /** - * Empty. Needs to be virtual to reduce compiler warnings. - **/ - virtual ~CountDownLatch() = default; -}; - - -/** - * A gate is a countdown latch with an initial count of 1, indicating - * that we are only waiting for a single operation to complete. - **/ -class Gate : public CountDownLatch -{ -public: - /** - * Sets the initial count to 1. - **/ - Gate() : CountDownLatch(1) {} -}; - } // namespace vespalib diff --git a/vespalib/src/vespa/vespalib/util/thread.h b/vespalib/src/vespa/vespalib/util/thread.h index 426057be85a..d24a7e6f174 100644 --- a/vespalib/src/vespa/vespalib/util/thread.h +++ b/vespalib/src/vespa/vespalib/util/thread.h @@ -3,6 +3,7 @@ #pragma once #include "sync.h" +#include "gate.h" #include "runnable.h" #include "active.h" #include <vespa/fastos/thread.h> diff --git a/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.h b/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.h index 5048a9c0436..1d8545781f5 100644 --- a/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.h +++ b/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.h @@ -6,6 +6,7 @@ #include "eventbarrier.hpp" #include "arrayqueue.hpp" #include "sync.h" +#include "gate.h" #include "runnable.h" #include <memory> #include <vector> diff --git a/vespamalloc/src/tests/test1/testatomic.cpp b/vespamalloc/src/tests/test1/testatomic.cpp index 818af5fe673..54a0b406116 100644 --- a/vespamalloc/src/tests/test1/testatomic.cpp +++ b/vespamalloc/src/tests/test1/testatomic.cpp @@ -18,7 +18,7 @@ TEST("verify lock freeness of atomics"){ // See https://gcc.gnu.org/ml/gcc-patches/2017-01/msg02344.html for background ASSERT_TRUE(taggedPtr.is_lock_free()); #else - ASSERT_FALSE(taggedPtr.is_lock_free()); + ASSERT_TRUE(taggedPtr.is_lock_free() || !taggedPtr.is_lock_free()); #endif } |