diff options
174 files changed, 2214 insertions, 1185 deletions
diff --git a/README.md b/README.md index 09a3f9a7124..194d24885f1 100644 --- a/README.md +++ b/README.md @@ -63,17 +63,13 @@ You do not need to build Vespa to use it, but if you want to contribute you need This section explains how to build and test Vespa. To understand where to make changes, see [Code-map.md](Code-map.md). Some suggested improvements with pointers to code are in [TODO.md](TODO.md). -### Set up the build environment +### Development environment -C++ and Java building is supported on CentOS 7. The Java source can also be built on any platform having Java 11 and Maven installed. -We recommend using the following environment: [Create C++ / Java dev environment on CentOS using VirtualBox and Vagrant](vagrant/README.md). -You can also setup CentOS 7 natively and install the following build dependencies: - - sudo yum-config-manager --add-repo https://copr.fedorainfracloud.org/coprs/g/vespa/vespa/repo/epel-7/group_vespa-vespa-epel-7.repo - sudo yum -y install epel-release centos-release-scl yum-utils - sudo yum -y install ccache \ - rpm-build - yum-builddep -y <vespa-source>/dist/vespa.spec +C++ and Java building is supported on CentOS 7. +The Java source can also be built on any platform having Java 11 and Maven installed. +Use the following guide to setup a complete development environment using Docker +for building Vespa, running unit tests and running system tests: +[Vespa development on CentOS 7](https://github.com/vespa-engine/docker-image-dev#vespa-development-on-centos-7). ### Build Java modules @@ -82,20 +78,13 @@ You can also setup CentOS 7 natively and install the following build dependencie bash bootstrap.sh java mvn -T <num-threads> install -### Build C++ modules +Use this if you only need to build the Java modules, otherwise follow the complete development guide above. -Replace `<build-dir>` with the name of the directory in which you'd like to build Vespa. -Replace `<source-dir>` with the directory in which you've cloned/unpacked the source tree. +### Build RPM packages - bash bootstrap-cpp.sh <source-dir> <build-dir> - cd <build-dir> - make -j <num-threads> - ctest3 -j <num-threads> +See [Building Vespa RPM](docker/README.md#building-vespa-rpm) for details. -### Create RPM packages - sh dist.sh VERSION && rpmbuild -ba ~/rpmbuild/SPECS/vespa-VERSION.spec - ## License Code licensed under the Apache 2.0 license. See [LICENSE](LICENSE) for terms. diff --git a/application/pom.xml b/application/pom.xml index d173fec19cf..cd27b53f557 100644 --- a/application/pom.xml +++ b/application/pom.xml @@ -120,6 +120,10 @@ <groupId>org.apache.httpcomponents</groupId> <artifactId>httpclient</artifactId> </dependency> + <dependency> + <groupId>org.apache.commons</groupId> + <artifactId>commons-math3</artifactId> + </dependency> <dependency> <groupId>com.yahoo.vespa</groupId> diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidator.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidator.java index 89e7e340641..3a926922f2b 100644 --- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidator.java +++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidator.java @@ -85,11 +85,11 @@ public class InstanceValidator { return false; } - log.log(LogLevel.INFO, () -> String.format("Validating instance %s.", providerUniqueId)); + log.log(LogLevel.DEBUG, () -> String.format("Validating instance %s.", providerUniqueId)); PublicKey publicKey = keyProvider.getPublicKey(signedIdentityDocument.signingKeyVersion()); if (signer.hasValidSignature(signedIdentityDocument, publicKey)) { - log.log(LogLevel.INFO, () -> String.format("Instance %s is valid.", providerUniqueId)); + log.log(LogLevel.DEBUG, () -> String.format("Instance %s is valid.", providerUniqueId)); return true; } log.log(LogLevel.ERROR, () -> String.format("Instance %s has invalid signature.", providerUniqueId)); @@ -100,7 +100,7 @@ public class InstanceValidator { // We'll have to perform some validation on the instance id and other fields of the attribute map. // Separate between tenant and node certificate as well. public boolean isValidRefresh(InstanceConfirmation confirmation) { - log.log(LogLevel.INFO, () -> String.format("Accepting refresh for instance with identity '%s', provider '%s', instanceId '%s'.", + log.log(LogLevel.DEBUG, () -> String.format("Accepting refresh for instance with identity '%s', provider '%s', instanceId '%s'.", new AthenzService(confirmation.domain, confirmation.service).getFullName(), confirmation.provider, confirmation.attributes.get(SAN_DNS_ATTRNAME))); diff --git a/bootstrap-cmake.sh b/bootstrap-cmake.sh index 27c7ed6c21b..dbe27f5b8bf 100755 --- a/bootstrap-cmake.sh +++ b/bootstrap-cmake.sh @@ -39,7 +39,8 @@ else fi if [ -z "$VESPA_LLVM_VERSION" ]; then - VESPA_LLVM_VERSION=5.0 + VESPA_LLVM_VERSION_PATH=7.0 + VESPA_LLVM_VERSION=7 fi if $UNPRIVILEGED; then @@ -54,8 +55,8 @@ cmake3 \ -DCMAKE_INSTALL_PREFIX=${VESPA_INSTALL_PREFIX} \ -DJAVA_HOME=${JAVA_HOME:-/usr/lib/jvm/java-openjdk} \ -DCMAKE_PREFIX_PATH="/opt/vespa-deps" \ - -DEXTRA_LINK_DIRECTORY="/opt/vespa-deps/lib64;/usr/lib64/llvm$VESPA_LLVM_VERSION/lib" \ - -DEXTRA_INCLUDE_DIRECTORY="/opt/vespa-deps/include;/usr/include/llvm$VESPA_LLVM_VERSION;/usr/include/openblas" \ + -DEXTRA_LINK_DIRECTORY="/opt/vespa-deps/lib64;/usr/lib64/llvm$VESPA_LLVM_VERSION_PATH/lib" \ + -DEXTRA_INCLUDE_DIRECTORY="/opt/vespa-deps/include;/usr/include/llvm$VESPA_LLVM_VERSION_PATH;/usr/include/openblas" \ -DCMAKE_INSTALL_RPATH="${VESPA_INSTALL_PREFIX}/lib64;/opt/vespa-deps/lib64;/usr/lib/jvm/java-1.8.0/jre/lib/amd64/server;/usr/lib64/llvm$VESPA_LLVM_VERSION/lib" \ ${UNPRIVILEGED_ARGS} \ ${EXTRA_CMAKE_ARGS} \ diff --git a/bootstrap-cpp.sh b/bootstrap-cpp.sh index 5fe4723ea90..5753d4cfcfd 100755 --- a/bootstrap-cpp.sh +++ b/bootstrap-cpp.sh @@ -48,7 +48,7 @@ mkdir -p "${BUILD_DIR}" || { BUILD_DIR=$(realpath "${BUILD_DIR}") # Build it -source /opt/rh/devtoolset-8/enable || true +source /opt/rh/devtoolset-9/enable || true cd "${SOURCE_DIR}" bash ./bootstrap.sh full cd "${BUILD_DIR}" diff --git a/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java index 15b775a4543..23273f9f1bf 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java @@ -47,11 +47,6 @@ class OverrideProcessor implements PreProcessor { private static final String ENVIRONMENT_ATTRIBUTE = "environment"; private static final String REGION_ATTRIBUTE = "region"; - // TODO: Remove after September 2019 - public OverrideProcessor(Environment environment, RegionName region) { - this(InstanceName.from("default"), environment, region); - } - public OverrideProcessor(InstanceName instance, Environment environment, RegionName region) { this.instance = instance; this.environment = environment; diff --git a/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java index 73f97cf516f..c49a799f2b7 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java @@ -38,20 +38,10 @@ public class XmlPreProcessor { private final RegionName region; private final List<PreProcessor> chain; - // TODO: Remove after September 2019 - public XmlPreProcessor(File applicationDir, File xmlInput, Environment environment, RegionName region) throws IOException { - this(applicationDir, new FileReader(xmlInput), InstanceName.from("default"), environment, region); - } - public XmlPreProcessor(File applicationDir, File xmlInput, InstanceName instance, Environment environment, RegionName region) throws IOException { this(applicationDir, new FileReader(xmlInput), instance, environment, region); } - // TODO: Remove after September 2019 - public XmlPreProcessor(File applicationDir, Reader xmlInput, Environment environment, RegionName region) throws IOException { - this(applicationDir, xmlInput, InstanceName.from("default"), environment, region); - } - public XmlPreProcessor(File applicationDir, Reader xmlInput, InstanceName instance, Environment environment, RegionName region) { this.applicationDir = applicationDir; this.xmlInput = xmlInput; diff --git a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/DeployData.java b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/DeployData.java index 344035745f8..82da1e2c6ea 100644 --- a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/DeployData.java +++ b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/DeployData.java @@ -2,9 +2,6 @@ package com.yahoo.config.model.application.provider; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.ApplicationName; -import com.yahoo.config.provision.InstanceName; -import com.yahoo.config.provision.TenantName; /** * Data generated or computed during deployment @@ -31,22 +28,6 @@ public class DeployData { private final long generation; private final long currentlyActiveGeneration; - // TODO: Remove after September 2019 - public DeployData(String deployedByUser, - String deployedFromDir, - String applicationName, - Long deployTimestamp, - boolean internalRedeploy, - Long generation, - long currentlyActiveGeneration) { - this(deployedByUser, - deployedFromDir, - ApplicationId.from(TenantName.defaultName(), ApplicationName.from(applicationName), InstanceName.from("default")), - deployTimestamp, - internalRedeploy, - generation, currentlyActiveGeneration); - } - public DeployData(String deployedByUser, String deployedFromDir, ApplicationId applicationId, @@ -77,7 +58,4 @@ public class DeployData { public ApplicationId getApplicationId() { return applicationId; } - // TODO: remove after September 2019 - public String getApplicationName() { return applicationId.application().toString(); } - } diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index b8f03794301..a074c4af3dc 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -60,13 +60,23 @@ public interface ModelContext { // TODO: Only needed for LbServicesProducerTest default boolean useDedicatedNodeForLogserver() { return true; } + // TODO Revisit in May or June 2020 boolean useAdaptiveDispatch(); // TODO: Remove after April 2020 default Optional<TlsSecrets> tlsSecrets() { return Optional.empty(); } default Optional<EndpointCertificateSecrets> endpointCertificateSecrets() { return Optional.empty(); } + + // TODO Revisit in May or June 2020 double defaultTermwiseLimit(); + + // TODO Revisit in May or June 2020 + double defaultSoftStartSeconds(); + + // TODO Revisit in May or June 2020 + double defaultTopKProbability(); + boolean useBucketSpaceMetric(); default boolean useNewAthenzFilter() { return true; } // TODO bjorncs: Remove after end of April diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java index 4c9e9489c63..99225beba4f 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java @@ -40,7 +40,9 @@ public class TestProperties implements ModelContext.Properties { private boolean isFirstTimeDeployment = false; private boolean useDedicatedNodeForLogserver = false; private boolean useAdaptiveDispatch = false; + private double topKProbability = 1.0; private double defaultTermwiseLimit = 1.0; + private double softStartSeconds = 0.0; private Optional<EndpointCertificateSecrets> endpointCertificateSecrets = Optional.empty(); private AthenzDomain athenzDomain; @@ -61,6 +63,13 @@ public class TestProperties implements ModelContext.Properties { @Override public Optional<EndpointCertificateSecrets> endpointCertificateSecrets() { return endpointCertificateSecrets; } @Override public Optional<TlsSecrets> tlsSecrets() { return endpointCertificateSecrets.map(TlsSecrets::new); } @Override public double defaultTermwiseLimit() { return defaultTermwiseLimit; } + + @Override + public double defaultSoftStartSeconds() { + return softStartSeconds; + } + + @Override public double defaultTopKProbability() { return topKProbability; } @Override public boolean useBucketSpaceMetric() { return true; } @Override public Optional<AthenzDomain> athenzDomain() { return Optional.ofNullable(athenzDomain); } @@ -69,6 +78,15 @@ public class TestProperties implements ModelContext.Properties { return this; } + public TestProperties setTopKProbability(double probability) { + topKProbability = probability; + return this; + } + public TestProperties setSoftStartSeconds(double softStartSeconds) { + this.softStartSeconds = softStartSeconds; + return this; + } + public TestProperties setApplicationId(ApplicationId applicationId) { this.applicationId = applicationId; return this; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java index 38037c8a522..24e88d7ef7d 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java @@ -11,7 +11,7 @@ import com.yahoo.vespa.model.container.component.Handler; /** * @author hmusum */ -public class LogserverContainerCluster extends ContainerCluster<LogserverContainer> implements ThreadpoolConfig.Producer { +public class LogserverContainerCluster extends ContainerCluster<LogserverContainer> { public LogserverContainerCluster(AbstractConfigProducer<?> parent, String name, DeployState deployState) { super(parent, name, name, deployState); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java index 2de4e5f5950..41d9df414ea 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java @@ -11,8 +11,7 @@ import com.yahoo.vespa.model.container.ContainerCluster; * * @author gjoranv */ -public class ClusterControllerContainerCluster extends ContainerCluster<ClusterControllerContainer> implements - ThreadpoolConfig.Producer +public class ClusterControllerContainerCluster extends ContainerCluster<ClusterControllerContainer> { public ClusterControllerContainerCluster(AbstractConfigProducer<?> parent, String subId, String name, DeployState deployState) { super(parent, subId, name, deployState); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java index 4cf0aa151b1..20f2bfe6636 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java @@ -71,7 +71,6 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC ConsumersConfig.Producer, MonitoringConfig.Producer, TelegrafConfig.Producer, - ThreadpoolConfig.Producer, MetricsNodesConfig.Producer { public static final Logger log = Logger.getLogger(MetricsProxyContainerCluster.class.getName()); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java index f9bb7e1e425..63e6af03c44 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java @@ -10,6 +10,7 @@ import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.container.BundlesConfig; import com.yahoo.container.bundle.BundleInstantiationSpecification; +import com.yahoo.container.handler.ThreadpoolConfig; import com.yahoo.container.handler.metrics.MetricsProxyApiConfig; import com.yahoo.container.handler.metrics.MetricsV2Handler; import com.yahoo.container.jdisc.ContainerMbusConfig; @@ -68,6 +69,7 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat private MbusParams mbusParams; private boolean messageBusEnabled = true; + private final double softStartSeconds; private Integer memoryPercentage = null; @@ -84,6 +86,7 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat addSimpleComponent("com.yahoo.container.jdisc.AthenzIdentityProviderProvider"); addMetricsV2Handler(); addTestrunnerComponentsIfTester(deployState); + softStartSeconds = deployState.getProperties().defaultSoftStartSeconds(); } @Override @@ -248,6 +251,11 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat null)))); } + @Override + public void getConfig(ThreadpoolConfig.Builder builder) { + builder.softStartSeconds(softStartSeconds); + } + public static class MbusParams { // the amount of the maxpendingbytes to process concurrently, typically 0.2 (20%) final Double maxConcurrentFactor; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java index 95e867db8d1..e2b1f97a6eb 100755 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java @@ -19,6 +19,7 @@ import com.yahoo.container.bundle.BundleInstantiationSpecification; import com.yahoo.container.core.ApplicationMetadataConfig; import com.yahoo.container.core.document.ContainerDocumentConfig; import com.yahoo.container.handler.ThreadPoolProvider; +import com.yahoo.container.handler.ThreadpoolConfig; import com.yahoo.container.jdisc.JdiscBindingsConfig; import com.yahoo.container.jdisc.config.HealthMonitorConfig; import com.yahoo.container.jdisc.state.StateHandler; @@ -99,7 +100,9 @@ public abstract class ContainerCluster<CONTAINER extends Container> DocprocConfig.Producer, ClusterInfoConfig.Producer, RoutingProviderConfig.Producer, - ConfigserverConfig.Producer { + ConfigserverConfig.Producer, + ThreadpoolConfig.Producer +{ /** * URI prefix used for internal, usually programmatic, APIs. URIs using this diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/DispatchTuning.java b/config-model/src/main/java/com/yahoo/vespa/model/content/DispatchTuning.java index 0d15207b6ce..0f9eb5341ab 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/DispatchTuning.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/DispatchTuning.java @@ -11,18 +11,25 @@ public class DispatchTuning { public static final DispatchTuning empty = new DispatchTuning.Builder().build(); - public enum DispatchPolicy { ROUNDROBIN, ADAPTIVE}; + public enum DispatchPolicy { ROUNDROBIN, ADAPTIVE} private final Integer maxHitsPerPartition; private DispatchPolicy dispatchPolicy; private final Double minGroupCoverage; private final Double minActiveDocsCoverage; + public Double getTopkProbability() { + return topkProbability; + } + + private final Double topkProbability; + private DispatchTuning(Builder builder) { maxHitsPerPartition = builder.maxHitsPerPartition; dispatchPolicy = builder.dispatchPolicy; minGroupCoverage = builder.minGroupCoverage; minActiveDocsCoverage = builder.minActiveDocsCoverage; + topkProbability = builder.topKProbability; } /** Returns the max number of hits to fetch from each partition, or null to fetch all */ @@ -46,6 +53,7 @@ public class DispatchTuning { private DispatchPolicy dispatchPolicy; private Double minGroupCoverage; private Double minActiveDocsCoverage; + private Double topKProbability; public DispatchTuning build() { return new DispatchTuning(this); @@ -55,6 +63,10 @@ public class DispatchTuning { this.maxHitsPerPartition = maxHitsPerPartition; return this; } + public Builder setTopKProbability(Double topKProbability) { + this.topKProbability = topKProbability; + return this; + } public Builder setDispatchPolicy(String policy) { if (policy != null) dispatchPolicy = toDispatchPolicy(policy); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/DomTuningDispatchBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/DomTuningDispatchBuilder.java index b53d66632a8..d599a1a1aca 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/DomTuningDispatchBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/DomTuningDispatchBuilder.java @@ -23,6 +23,7 @@ public class DomTuningDispatchBuilder { return builder.build(); } builder.setMaxHitsPerPartition(dispatchElement.childAsInteger("max-hits-per-partition")); + builder.setTopKProbability(dispatchElement.childAsDouble("top-k-probability")); builder.setDispatchPolicy(dispatchElement.childAsString("dispatch-policy")); builder.setMinGroupCoverage(dispatchElement.childAsDouble("min-group-coverage")); builder.setMinActiveDocsCoverage(dispatchElement.childAsDouble("min-active-docs-coverage")); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java index 9746c50450e..56adc227df4 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java @@ -53,6 +53,7 @@ public class IndexedSearchCluster extends SearchCluster private final DispatchGroup rootDispatch; private DispatchSpec dispatchSpec; private final boolean useAdaptiveDispatch; + private final double defaultTopKProbability; private List<SearchNode> searchNodes = new ArrayList<>(); /** @@ -70,6 +71,7 @@ public class IndexedSearchCluster extends SearchCluster unionCfg = new UnionConfiguration(this, documentDbs); rootDispatch = new DispatchGroup(this); useAdaptiveDispatch = deployState.getProperties().useAdaptiveDispatch(); + defaultTopKProbability = deployState.getProperties().defaultTopKProbability(); } @Override @@ -307,7 +309,11 @@ public class IndexedSearchCluster extends SearchCluster } if (useAdaptiveDispatch) builder.distributionPolicy(DistributionPolicy.ADAPTIVE); - + if (tuning.dispatch.getTopkProbability() != null) { + builder.topKProbability(tuning.dispatch.getTopkProbability()); + } else { + builder.topKProbability(defaultTopKProbability); + } if (tuning.dispatch.getMinActiveDocsCoverage() != null) builder.minActivedocsPercentage(tuning.dispatch.getMinActiveDocsCoverage()); if (tuning.dispatch.getMinGroupCoverage() != null) diff --git a/config-model/src/main/resources/schema/content.rnc b/config-model/src/main/resources/schema/content.rnc index b1821680b14..481d82ebb4b 100644 --- a/config-model/src/main/resources/schema/content.rnc +++ b/config-model/src/main/resources/schema/content.rnc @@ -85,6 +85,7 @@ DispatchTuning = element dispatch { element dispatch-policy { string "round-robin" | string "adaptive" | string "random" }? & element min-group-coverage { xsd:double }? & element min-active-docs-coverage { xsd:double }? & + element top-k-probability { xsd:double }? & element use-local-node { string "true" | string "false" }? } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java index ce565989c18..33cf0635349 100755 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java @@ -202,6 +202,34 @@ public class ContainerClusterTest { } @Test + public void requireThatSoftStartSecondsCanBeControlledByProperties() { + DeployState state = new DeployState.Builder().properties(new TestProperties().setSoftStartSeconds(300.0)) + .build(); + MockRoot root = new MockRoot("foo", state); + ApplicationContainerCluster cluster = createContainerCluster(root, false); + addContainer(root.deployLogger(), cluster, "c1", "host-c1"); + + ThreadpoolConfig.Builder tpBuilder = new ThreadpoolConfig.Builder(); + cluster.getConfig(tpBuilder); + ThreadpoolConfig threadpoolConfig = new ThreadpoolConfig(tpBuilder); + assertEquals(500, threadpoolConfig.maxthreads()); + assertEquals(300.0, threadpoolConfig.softStartSeconds(), 0.0); + } + + @Test + public void requireThatDefaultThreadPoolConfigIsSane() { + MockRoot root = new MockRoot("foo"); + ApplicationContainerCluster cluster = createContainerCluster(root, false); + addContainer(root.deployLogger(), cluster, "c1", "host-c1"); + + ThreadpoolConfig.Builder tpBuilder = new ThreadpoolConfig.Builder(); + cluster.getConfig(tpBuilder); + ThreadpoolConfig threadpoolConfig = new ThreadpoolConfig(tpBuilder); + assertEquals(500, threadpoolConfig.maxthreads()); + assertEquals(0.0, threadpoolConfig.softStartSeconds(), 0.0); + } + + @Test public void requireThatRoutingProviderIsDisabledForNonHosted() { DeployState state = new DeployState.Builder().properties(new TestProperties().setHostedVespa(false)).build(); MockRoot root = new MockRoot("foo", state); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java index b08cc92d20c..4d5df7c1965 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java @@ -933,6 +933,17 @@ public class ContentClusterTest extends ContentBaseTest { assertEquals(distributionBits, storDistributormanagerConfig.minsplitcount()); } + private void verifyTopKProbabilityPropertiesControl(double topKProbability) { + VespaModel model = createEnd2EndOneNode(new TestProperties().setTopKProbability(topKProbability)); + + ContentCluster cc = model.getContentClusters().get("storage"); + DispatchConfig.Builder builder = new DispatchConfig.Builder(); + cc.getSearch().getConfig(builder); + + DispatchConfig cfg = new DispatchConfig(builder); + assertEquals(topKProbability, cfg.topKProbability(), 0.0); + } + private void verifyRoundRobinPropertiesControl(boolean useAdaptiveDispatch) { VespaModel model = createEnd2EndOneNode(new TestProperties().setUseAdaptiveDispatch(useAdaptiveDispatch)); @@ -946,7 +957,6 @@ public class ContentClusterTest extends ContentBaseTest { } else { assertEquals(DispatchConfig.DistributionPolicy.ROUNDROBIN, cfg.distributionPolicy()); } - } @Test @@ -955,5 +965,12 @@ public class ContentClusterTest extends ContentBaseTest { verifyRoundRobinPropertiesControl(true); } + @Test + public void default_topKprobability_controlled_by_properties() { + verifyTopKProbabilityPropertiesControl(1.0); + verifyTopKProbabilityPropertiesControl(0.999); + verifyTopKProbabilityPropertiesControl(0.77); + } + } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/DispatchTuningTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/DispatchTuningTest.java index f708d7673e2..8a46aaaa230 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/DispatchTuningTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/DispatchTuningTest.java @@ -19,11 +19,13 @@ public class DispatchTuningTest { .setDispatchPolicy("round-robin") .setMinGroupCoverage(7.5) .setMinActiveDocsCoverage(12.5) + .setTopKProbability(18.3) .build(); assertEquals(69, dispatch.getMaxHitsPerPartition().intValue()); assertEquals(7.5, dispatch.getMinGroupCoverage().doubleValue(), 0.0); assertEquals(12.5, dispatch.getMinActiveDocsCoverage().doubleValue(), 0.0); assertTrue(DispatchTuning.DispatchPolicy.ROUNDROBIN == dispatch.getDispatchPolicy()); + assertEquals(18.3, dispatch.getTopkProbability(), 0.0); } @Test public void requireThatRandomDispatchWork() { @@ -52,6 +54,7 @@ public class DispatchTuningTest { assertNull(dispatch.getDispatchPolicy()); assertNull(dispatch.getMinActiveDocsCoverage()); assertNull(dispatch.getMinGroupCoverage()); + assertNull(dispatch.getTopkProbability()); } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/DomDispatchTuningBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/DomDispatchTuningBuilderTest.java index 7fa27f74d74..abfb03e41dd 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/DomDispatchTuningBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/DomDispatchTuningBuilderTest.java @@ -47,6 +47,7 @@ public class DomDispatchTuningBuilderTest { assertNull(dispatch.getMinGroupCoverage()); assertNull(dispatch.getMinActiveDocsCoverage()); assertNull(dispatch.getDispatchPolicy()); + assertNull(dispatch.getTopkProbability()); } @Test @@ -58,12 +59,14 @@ public class DomDispatchTuningBuilderTest { " <max-hits-per-partition>69</max-hits-per-partition>" + " <min-group-coverage>7.5</min-group-coverage>" + " <min-active-docs-coverage>12.5</min-active-docs-coverage>" + + " <top-k-probability>0.999</top-k-probability>" + " </dispatch>" + " </tuning>" + "</content>"); assertEquals(69, dispatch.getMaxHitsPerPartition().intValue()); assertEquals(7.5, dispatch.getMinGroupCoverage().doubleValue(), 0.0); assertEquals(12.5, dispatch.getMinActiveDocsCoverage().doubleValue(), 0.0); + assertEquals(0.999, dispatch.getTopkProbability().doubleValue(), 0.0); } @Test public void requireThatTuningDispatchPolicyRoundRobin() throws Exception { diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java index 48b4e9d91bc..09ee25fb437 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java @@ -21,6 +21,8 @@ public final class Capacity { private final NodeType type; private Capacity(ClusterResources min, ClusterResources max, boolean required, boolean canFail, NodeType type) { + validate(min); + validate(max); if (max.smallerThan(min)) throw new IllegalArgumentException("The max capacity must be larger than the min capacity, but got min " + min + " and max " + max); @@ -31,6 +33,13 @@ public final class Capacity { this.type = type; } + private static void validate(ClusterResources resources) { + if (resources.nodes() == 0 && resources.groups() == 0) return; // unspecified + if (resources.nodes() % resources.groups() != 0) + throw new IllegalArgumentException("The number of nodes (" + resources.nodes() + + ") must be divisible by the number of groups (" + resources.groups() + ")"); + } + /** Returns the number of nodes requested */ @Deprecated // TODO: Remove after April 2020 public int nodeCount() { return min.nodes(); } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterResources.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterResources.java index 87b5133d4eb..11ae0845fb0 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterResources.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterResources.java @@ -20,9 +20,6 @@ public class ClusterResources { private final NodeResources nodeResources; public ClusterResources(int nodes, int groups, NodeResources nodeResources) { - if (nodes > 0 && groups > 0 && nodes % groups != 0) - throw new IllegalArgumentException("The number of nodes (" + nodes + - ") must be divisible by the number of groups (" + groups + ")"); this.nodes = nodes; this.groups = groups; this.nodeResources = Objects.requireNonNull(nodeResources); diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java index 147066b82e7..f7aacbc757b 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java @@ -77,18 +77,6 @@ public final class ClusterSpec { return new ClusterSpec(type, id, groupId, vespaVersion, exclusive, combinedId, dockerImageRepo); } - // TODO: Remove when when 7.200 is oldest model version in use - public static ClusterSpec request(Type type, Id id, Version vespaVersion, boolean exclusive, - Optional<Id> combinedId, Optional<String> dockerImageRepo) { - return new ClusterSpec(type, id, Optional.empty(), vespaVersion, exclusive, combinedId, dockerImageRepo); - } - - // TODO: Remove when when 7.200 is oldest model version in use - public static ClusterSpec from(Type type, Id id, Group groupId, Version vespaVersion, boolean exclusive, - Optional<Id> combinedId, Optional<String> dockerImageRepo) { - return new ClusterSpec(type, id, Optional.of(groupId), vespaVersion, exclusive, combinedId, dockerImageRepo); - } - /** Creates a ClusterSpec when requesting a cluster */ public static Builder request(Type type, Id id) { return new Builder(type, id, false); diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/HostSpec.java b/config-provisioning/src/main/java/com/yahoo/config/provision/HostSpec.java index c49448a2e43..2a5d27a0fe7 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/HostSpec.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/HostSpec.java @@ -29,7 +29,7 @@ public class HostSpec implements Comparable<HostSpec> { private final Optional<Version> version; - private final Optional<String> dockerImageRepo; + private final Optional<DockerImage> dockerImageRepo; private final Optional<NetworkPorts> networkPorts; @@ -79,14 +79,14 @@ public class HostSpec implements Comparable<HostSpec> { public HostSpec(String hostname, List<String> aliases, Optional<Flavor> flavor, Optional<ClusterMembership> membership, Optional<Version> version, Optional<NetworkPorts> networkPorts, Optional<NodeResources> requestedResources, - Optional<String> dockerImageRepo) { + Optional<DockerImage> dockerImageRepo) { if (hostname == null || hostname.isEmpty()) throw new IllegalArgumentException("Hostname must be specified"); this.hostname = hostname; this.aliases = List.copyOf(aliases); this.flavor = flavor; this.membership = membership; - this.version = Objects.requireNonNull(version, "Version cannot be null but can be empty");; - this.networkPorts = Objects.requireNonNull(networkPorts, "Network ports cannot be null but can be empty");; + this.version = Objects.requireNonNull(version, "Version cannot be null but can be empty"); + this.networkPorts = Objects.requireNonNull(networkPorts, "Network ports cannot be null but can be empty"); this.requestedResources = Objects.requireNonNull(requestedResources, "RequestedResources cannot be null"); this.dockerImageRepo = Objects.requireNonNull(dockerImageRepo, "Docker image repo cannot be null but can be empty"); } @@ -111,7 +111,7 @@ public class HostSpec implements Comparable<HostSpec> { /** Returns the requested resources leading to this host being provisioned, or empty if not known */ public Optional<NodeResources> requestedResources() { return requestedResources; } - public Optional<String> dockerImageRepo() { return dockerImageRepo; } + public Optional<DockerImage> dockerImageRepo() { return dockerImageRepo; } public HostSpec withPorts(Optional<NetworkPorts> ports) { return new HostSpec(hostname, aliases, flavor, membership, version, ports, requestedResources, dockerImageRepo); diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java b/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java index cee15cd6571..56c5544f6d7 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java @@ -3,6 +3,7 @@ package com.yahoo.config.provision.serialization; import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.config.provision.ClusterMembership; +import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.NodeFlavors; @@ -139,7 +140,7 @@ public class AllocatedHostsSerializer { optionalString(object.field(hostSpecCurrentVespaVersionKey)).map(com.yahoo.component.Version::new), NetworkPortsSerializer.fromSlime(object.field(hostSpecNetworkPortsKey)), nodeResourcesFromSlime(object.field(requestedResourcesKey)), - optionalString(object.field(hostSpecDockerImageRepoKey))); + optionalDockerImage(object.field(hostSpecDockerImageRepoKey))); } private static List<String> aliasesFromSlime(Inspector object) { @@ -215,4 +216,10 @@ public class AllocatedHostsSerializer { if ( ! inspector.valid()) return Optional.empty(); return Optional.of(inspector.asString()); } + + private static Optional<DockerImage> optionalDockerImage(Inspector inspector) { + if ( ! inspector.valid()) return Optional.empty(); + return Optional.of(DockerImage.fromString(inspector.asString())); + } + } diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java index f41b36b34ca..f2163608050 100644 --- a/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java +++ b/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java @@ -4,13 +4,13 @@ package com.yahoo.config.provision.serialization; import com.yahoo.component.Version; import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.config.provision.ClusterMembership; +import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.NetworkPorts; import com.yahoo.config.provision.NodeFlavors; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provisioning.FlavorsConfig; -import com.yahoo.text.Utf8; import org.junit.Test; import java.io.IOException; @@ -43,7 +43,7 @@ public class AllocatedHostsSerializerTest { Optional.of(ClusterMembership.from("container/test/0/0", Version.fromString("6.73.1"), Optional.of("docker.foo.com:4443/vespa/bar"))), Optional.empty(), Optional.empty(), Optional.empty(), - Optional.of("docker.foo.com:4443/vespa/bar"))); + Optional.of(DockerImage.fromString("docker.foo.com:4443/vespa/bar")))); hosts.add(new HostSpec("flavor-from-resources-1", Collections.emptyList(), new Flavor(new NodeResources(0.5, 3.1, 4, 1)))); hosts.add(new HostSpec("flavor-from-resources-2", diff --git a/config-proxy/src/main/sh/vespa-config-ctl.sh b/config-proxy/src/main/sh/vespa-config-ctl.sh index 4db9e406fa0..91e4cd39bd2 100755 --- a/config-proxy/src/main/sh/vespa-config-ctl.sh +++ b/config-proxy/src/main/sh/vespa-config-ctl.sh @@ -153,6 +153,12 @@ case $1 in elif ! vespa-ping-configproxy -s $hname then echo "failed to ping config proxy $hname" 1>&2 + # TODO: Dump stack trace for debugging, remove after April 2020 + kill -3 `pgrep -f -n configproxy` + sleep 2 + kill -3 `pgrep -f -n configproxy` + sleep 2 + kill -3 `pgrep -f -n configproxy` fi echo "look for reason in vespa.log, last part follows..." 1>&2 diff --git a/configdefinitions/src/vespa/dispatch.def b/configdefinitions/src/vespa/dispatch.def index 21001eb3af0..0776e648ad7 100644 --- a/configdefinitions/src/vespa/dispatch.def +++ b/configdefinitions/src/vespa/dispatch.def @@ -23,6 +23,15 @@ distributionPolicy enum { ROUNDROBIN, ADAPTIVE } default=ROUNDROBIN ## don't use it if you don't (really) mean it. maxHitsPerNode int default=2147483647 +## Probability for getting the K best hits (topK). +## A value of 1.0 will ask all N partitions for K hits. +## Any value between <0, 1> will use a Student T with 30 degrees freedom and compute a value Q that +## will give you the globally K best hits according to this formula with the desired probability. +## q = k/n + qT (p',30) x √(k × (1/n) × (1 − 1/n)) +## With a probability of 0.999 and K=200 and N=10 will give a Q of 38, meaning that you only need to fetch 19% compared to +## default setting of 1.0. This is a significant optimisation with with very little loss in presicion. +topKProbability double default=1.0 + # Is multi-level dispatch configured for this cluster # Deprecated, will go away soon, NOOP useMultilevelDispatch bool default=false diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java index 63b1df3d634..2a426e4cccc 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java @@ -231,20 +231,17 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable new DaemonThreadFactory("redeploy apps")); // Keep track of deployment per application Map<ApplicationId, Future<?>> futures = new HashMap<>(); - Set<ApplicationId> failedDeployments = new HashSet<>(); - - for (ApplicationId appId : applicationIds) { - Optional<Deployment> deploymentOptional = applicationRepository.deployFromLocalActive(appId, true /* bootstrap */); - if (deploymentOptional.isEmpty()) continue; - - futures.put(appId, executor.submit(deploymentOptional.get()::activate)); - } + applicationIds.forEach(applicationId -> futures.put(applicationId, executor.submit(() -> { + applicationRepository.deployFromLocalActive(applicationId, true /* bootstrap */) + .ifPresent(Deployment::activate); + }))); + Set<ApplicationId> failedDeployments = new HashSet<>(); for (Map.Entry<ApplicationId, Future<?>> f : futures.entrySet()) { - ApplicationId app = f.getKey(); try { f.getValue().get(); } catch (ExecutionException e) { + ApplicationId app = f.getKey(); if (e.getCause() instanceof TransientException) { log.log(LogLevel.INFO, "Redeploying " + app + " failed with transient error, will retry after bootstrap: " + Exceptions.toMessageString(e)); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java index 82dc17c3678..4bd5cf30cc6 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java @@ -171,14 +171,13 @@ public class Deployment implements com.yahoo.config.provision.Deployment { /** Exposes the session of this for testing only */ public LocalSession session() { return session; } - private long validateSessionStatus(LocalSession localSession) { + private void validateSessionStatus(LocalSession localSession) { long sessionId = localSession.getSessionId(); if (Session.Status.NEW.equals(localSession.getStatus())) { throw new IllegalStateException(localSession.logPre() + "Session " + sessionId + " is not prepared"); } else if (Session.Status.ACTIVATE.equals(localSession.getStatus())) { throw new IllegalStateException(localSession.logPre() + "Session " + sessionId + " is already active"); } - return sessionId; } private Transaction deactivateCurrentActivateNew(LocalSession active, LocalSession prepared, boolean ignoreStaleSessionFailure) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java index 2b25b69d09c..aaa671d5ad3 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java @@ -146,8 +146,10 @@ public class ModelContextImpl implements ModelContext { private final boolean isBootstrap; private final boolean isFirstTimeDeployment; private final boolean useAdaptiveDispatch; + private final double defaultTopKprobability; private final Optional<EndpointCertificateSecrets> endpointCertificateSecrets; private final double defaultTermwiseLimit; + private final double defaultSoftStartSeconds; private final boolean useBucketSpaceMetric; private final String proxyProtocol; private final Optional<AthenzDomain> athenzDomain; @@ -182,6 +184,10 @@ public class ModelContextImpl implements ModelContext { this.endpointCertificateSecrets = endpointCertificateSecrets; defaultTermwiseLimit = Flags.DEFAULT_TERM_WISE_LIMIT.bindTo(flagSource) .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); + defaultSoftStartSeconds = Flags.DEFAULT_SOFT_START_SECONDS.bindTo(flagSource) + .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); + defaultTopKprobability = Flags.DEFAULT_TOP_K_PROBABILITY.bindTo(flagSource) + .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); this.useBucketSpaceMetric = Flags.USE_BUCKET_SPACE_METRIC.bindTo(flagSource) .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); this.proxyProtocol = Flags.PROXY_PROTOCOL.bindTo(flagSource) @@ -239,6 +245,16 @@ public class ModelContextImpl implements ModelContext { public double defaultTermwiseLimit() { return defaultTermwiseLimit; } @Override + public double defaultSoftStartSeconds() { + return 0; + } + + @Override + public double defaultTopKProbability() { + return defaultTopKprobability; + } + + @Override public boolean useBucketSpaceMetric() { return useBucketSpaceMetric; } @Override diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java index 06a3dfa8777..5ae1289033d 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java @@ -127,7 +127,7 @@ public class SessionFactoryImpl implements SessionFactory, LocalSessionLoader { ApplicationId existingApplicationId = existingSession.getApplicationId(); long activeSessionId = getActiveSessionId(existingApplicationId); - logger.log(LogLevel.DEBUG, "Create from existing application id " + existingApplicationId + ", active session id is " + activeSessionId); + logger.log(LogLevel.DEBUG, "Create new session for application id '" + existingApplicationId + "' from existing active session " + activeSessionId); LocalSession session = create(existingApp, existingApplicationId, activeSessionId, internalRedeploy, timeoutBudget); // Note: Needs to be kept in sync with calls in SessionPreparer.writeStateToZooKeeper() session.setApplicationId(existingApplicationId); @@ -141,7 +141,6 @@ public class SessionFactoryImpl implements SessionFactory, LocalSessionLoader { boolean internalRedeploy, TimeoutBudget timeoutBudget) { long sessionId = sessionCounter.nextSessionId(); Path sessionIdPath = sessionsPath.append(String.valueOf(sessionId)); - log.log(LogLevel.DEBUG, TenantRepository.logPre(tenant) + "Next session id is " + sessionId + " , sessionIdPath=" + sessionIdPath.getAbsolute()); try { ensureZKPathDoesNotExist(sessionIdPath); SessionZooKeeperClient sessionZooKeeperClient = new SessionZooKeeperClient(curator, diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ModelContextImplTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ModelContextImplTest.java index 39dccc6b482..03c6bad79a8 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ModelContextImplTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ModelContextImplTest.java @@ -84,6 +84,9 @@ public class ModelContextImplTest { assertEquals(Optional.empty(), context.wantedDockerImageRepository()); assertEquals(new Version(7), context.modelVespaVersion()); assertEquals(new Version(8), context.wantedNodeVespaVersion()); + assertEquals(1.0, context.properties().defaultTermwiseLimit(), 0.0); + assertEquals(1.0, context.properties().defaultTopKProbability(), 0.0); + assertFalse(context.properties().useAdaptiveDispatch()); } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java index d57e2389cb1..9c7da7134e6 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java @@ -5,6 +5,7 @@ import com.yahoo.component.Version; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.AllocatedHosts; +import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.NodeFlavors; @@ -40,7 +41,7 @@ public class ZKApplicationPackageTest { private static final AllocatedHosts ALLOCATED_HOSTS = AllocatedHosts.withHosts( Collections.singleton(new HostSpec("foo.yahoo.com", Collections.emptyList(), TEST_FLAVOR, Optional.empty(), Optional.of(Version.fromString("6.0.1")), Optional.empty(), - Optional.empty(), Optional.of("docker repo")))); + Optional.empty(), Optional.of(DockerImage.fromString("docker repo"))))); private ConfigCurator configCurator; diff --git a/container-core/src/main/java/com/yahoo/container/core/config/BundleInstaller.java b/container-core/src/main/java/com/yahoo/container/core/config/BundleInstaller.java new file mode 100644 index 00000000000..fc919571b6c --- /dev/null +++ b/container-core/src/main/java/com/yahoo/container/core/config/BundleInstaller.java @@ -0,0 +1,21 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.core.config; + +import com.yahoo.config.FileReference; +import com.yahoo.osgi.Osgi; +import org.osgi.framework.Bundle; + +import java.util.List; + +/** + * @author gjoranv + */ +public interface BundleInstaller { + + /** + * Installs the bundle with the given file reference, plus all bundles in its X-JDisc-Preinstall-Bundle directive. + * Returns all bundles installed to the given OSGi framework as a result of this call. + */ + List<Bundle> installBundles(FileReference reference, Osgi osgi) throws InterruptedException; + +} diff --git a/container-core/src/main/java/com/yahoo/container/core/config/BundleLoader.java b/container-core/src/main/java/com/yahoo/container/core/config/BundleManager.java index ca11ad387ee..5e9b42a5fda 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/BundleLoader.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/BundleManager.java @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.core.config; import com.yahoo.collections.PredicateSplit; @@ -9,13 +9,12 @@ import com.yahoo.osgi.Osgi; import org.osgi.framework.Bundle; import org.osgi.framework.wiring.BundleRevision; -import java.io.File; +import java.util.ArrayList; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.TimeUnit; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -23,12 +22,12 @@ import static com.yahoo.collections.PredicateSplit.partition; import static com.yahoo.container.core.BundleLoaderProperties.DISK_BUNDLE_PREFIX; /** - * Manages the set of installed 3rd-party component bundles. + * Manages the set of installed and active/inactive bundles. * - * @author Tony Vaagenes * @author gjoranv + * @author Tony Vaagenes */ -public class BundleLoader { +public class BundleManager { /* Map of file refs of active bundles (not scheduled for uninstall) to a list of all bundles that were installed * (pre-install directive) by the bundle pointed to by the file ref (including itself). @@ -40,20 +39,96 @@ public class BundleLoader { */ private final Map<FileReference, List<Bundle>> reference2Bundles = new LinkedHashMap<>(); - private final Logger log = Logger.getLogger(BundleLoader.class.getName()); + private final Logger log = Logger.getLogger(BundleManager.class.getName()); private final Osgi osgi; - public BundleLoader(Osgi osgi) { + // A custom bundle installer for non-disk bundles, to be used for testing + private BundleInstaller customBundleInstaller = null; + + public BundleManager(Osgi osgi) { this.osgi = osgi; } - private void install(List<FileReference> references) { + /** + * Installs the given set of bundles and returns the set of bundles that is no longer used + * by the application, and should therefore be scheduled for uninstall. + */ + public synchronized Set<Bundle> use(List<FileReference> newFileReferences) { + // Must be done before allowing duplicates because allowed duplicates affect osgi.getCurrentBundles + Set<Bundle> bundlesToUninstall = getObsoleteBundles(newFileReferences); + + Set<FileReference> obsoleteReferences = getObsoleteFileReferences(newFileReferences); + allowDuplicateBundles(obsoleteReferences); + removeInactiveFileReferences(obsoleteReferences); + + installBundles(newFileReferences); + startBundles(); + + bundlesToUninstall.removeAll(allActiveBundles()); + log.info("Bundles to schedule for uninstall: " + bundlesToUninstall); + + log.info(installedBundlesMessage()); + return bundlesToUninstall; + } + + /** + * Returns the bundles that are not assumed to be retained by the new application generation. + * Note that at this point we don't yet know the full set of new bundles, because of the potential + * pre-install directives in the new bundles. However, only "disk bundles" (file:) can be listed + * in the pre-install directive, so we know about all the obsolete application bundles. + */ + private Set<Bundle> getObsoleteBundles(List<FileReference> newReferences) { + Set<Bundle> bundlesToRemove = new HashSet<>(osgi.getCurrentBundles()); + + for (FileReference fileReferenceToKeep : newReferences) { + if (reference2Bundles.containsKey(fileReferenceToKeep)) { + bundlesToRemove.removeAll(reference2Bundles.get(fileReferenceToKeep)); + } + } + bundlesToRemove.removeAll(osgi.getInitialBundles()); + return bundlesToRemove; + } + + + private Set<FileReference> getObsoleteFileReferences(List<FileReference> newReferences) { + Set<FileReference> obsoleteReferences = new HashSet<>(reference2Bundles.keySet()); + obsoleteReferences.removeAll(newReferences); + return obsoleteReferences; + } + + /** + * Allow duplicates (bsn+version) for each bundle that corresponds to obsolete file references, + * and avoid allowing duplicates for bundles that were installed via the + * X-JDisc-Preinstall-Bundle directive. These bundles are always "disk bundles" (library + * bundles installed on the node, and not transferred via file distribution). + * Such bundles will never have duplicates because they always have the same location id. + */ + private void allowDuplicateBundles(Set<FileReference> obsoleteReferences) { + // The bundle at index 0 for each file reference always corresponds to the bundle at the file reference location + Set<Bundle> allowedDuplicates = obsoleteReferences.stream() + .filter(reference -> ! isDiskBundle(reference)) + .map(reference -> reference2Bundles.get(reference).get(0)) + .collect(Collectors.toSet()); + + log.info(() -> allowedDuplicates.isEmpty() ? "" : "Adding bundles to allowed duplicates: " + allowedDuplicates); + osgi.allowDuplicateBundles(allowedDuplicates); + } + + /** + * Cleans up the map of active file references + */ + private void removeInactiveFileReferences(Set<FileReference> fileReferencesToRemove) { + // Clean up the map of active bundles + fileReferencesToRemove.forEach(reference2Bundles::remove); + } + + private void installBundles(List<FileReference> references) { Set<FileReference> bundlesToInstall = new HashSet<>(references); // This is just an optimization, as installing a bundle with the same location id returns the already installed bundle. bundlesToInstall.removeAll(reference2Bundles.keySet()); - PredicateSplit<FileReference> bundlesToInstall_isDisk = partition(bundlesToInstall, BundleLoader::isDiskBundle); + PredicateSplit<FileReference> bundlesToInstall_isDisk = partition(bundlesToInstall, BundleManager::isDiskBundle); installBundlesFromDisk(bundlesToInstall_isDisk.trueValues); installBundlesFromFileDistribution(bundlesToInstall_isDisk.falseValues); @@ -81,7 +156,9 @@ public class BundleLoader { FileAcquirer fileAcquirer = Container.get().getFileAcquirer(); boolean hasFileDistribution = (fileAcquirer != null); if (hasFileDistribution) { - installWithFileDistribution(bundlesToInstall, fileAcquirer); + installWithFileDistribution(bundlesToInstall, new FileAcquirerBundleInstaller(fileAcquirer)); + } else if (customBundleInstaller != null) { + installWithFileDistribution(bundlesToInstall, customBundleInstaller); } else { log.warning("Can't retrieve bundles since file distribution is disabled."); } @@ -89,25 +166,18 @@ public class BundleLoader { } private void installBundleFromDisk(FileReference reference) { - assert(reference.value().startsWith(DISK_BUNDLE_PREFIX)); - String referenceFileName = reference.value().substring(DISK_BUNDLE_PREFIX.length()); log.info("Installing bundle from disk with reference '" + reference.value() + "'"); - File file = new File(referenceFileName); - if ( ! file.exists()) { - throw new IllegalArgumentException("Reference '" + reference.value() + "' not found on disk."); - } - - List<Bundle> bundles = osgi.install(file.getAbsolutePath()); - + var bundleInstaller = new DiskBundleInstaller(); + List<Bundle> bundles = bundleInstaller.installBundles(reference, osgi); reference2Bundles.put(reference, bundles); } - private void installWithFileDistribution(List<FileReference> bundlesToInstall, FileAcquirer fileAcquirer) { + private void installWithFileDistribution(List<FileReference> bundlesToInstall, BundleInstaller bundleInstaller) { for (FileReference reference : bundlesToInstall) { try { log.info("Installing bundle with reference '" + reference.value() + "'"); - List<Bundle> bundles = obtainBundles(reference, fileAcquirer); + List<Bundle> bundles = bundleInstaller.installBundles(reference, osgi); reference2Bundles.put(reference, bundles); } catch(Exception e) { @@ -116,11 +186,6 @@ public class BundleLoader { } } - private List<Bundle> obtainBundles(FileReference reference, FileAcquirer fileAcquirer) throws InterruptedException { - File file = fileAcquirer.waitFor(reference, 7, TimeUnit.DAYS); - return osgi.install(file.getAbsolutePath()); - } - /** * Resolves and starts (calls the Bundles BundleActivator) all bundles. Bundle resolution must take place * after all bundles are installed to ensure that the framework can resolve dependencies between bundles. @@ -146,87 +211,12 @@ public class BundleLoader { return (bundleRevision.getTypes() & BundleRevision.TYPE_FRAGMENT) != 0; } - /** - * Returns the bundles that are not assumed to be retained by the new application generation. - * Note that at this point we don't yet know the full set of new bundles, because of the potential - * pre-install directives in the new bundles. However, only "disk bundles" (file:) can be listed - * in the pre-install directive, so we know about all the obsolete application bundles. - */ - private Set<Bundle> getObsoleteBundles(List<FileReference> newReferences) { - Set<Bundle> bundlesToRemove = new HashSet<>(osgi.getCurrentBundles()); - - for (FileReference fileReferenceToKeep : newReferences) { - if (reference2Bundles.containsKey(fileReferenceToKeep)) { - bundlesToRemove.removeAll(reference2Bundles.get(fileReferenceToKeep)); - } - } - bundlesToRemove.removeAll(osgi.getInitialBundles()); - return bundlesToRemove; - } - - /** - * Cleans up the map of active file references - */ - private void removeInactiveFileReferences(List<FileReference> newReferences) { - // Clean up the map of active bundles - Set<FileReference> fileReferencesToRemove = getObsoleteFileReferences(newReferences); - fileReferencesToRemove.forEach(reference2Bundles::remove); - } - - - /** - * Allow duplicates (bsn+version) for each bundle that corresponds to obsolete file references, - * and avoid allowing duplicates for bundles that were installed via the - * X-JDisc-Preinstall-Bundle directive. These bundles are always "disk bundles" (library - * bundles installed on the node, and not transferred via file distribution). - * Such bundles will never have duplicates because they always have the same location id. - */ - private void allowDuplicateBundles(List<FileReference> newReferences) { - Set<FileReference> obsoleteReferences = getObsoleteFileReferences(newReferences); - - // The bundle at index 0 for each file reference always corresponds to the bundle at the file reference location - Set<Bundle> allowedDuplicates = obsoleteReferences.stream() - .filter(reference -> ! isDiskBundle(reference)) - .map(reference -> reference2Bundles.get(reference).get(0)) - .collect(Collectors.toSet()); - - log.info(() -> allowedDuplicates.isEmpty() ? "" : "Adding bundles to allowed duplicates: " + allowedDuplicates); - osgi.allowDuplicateBundles(allowedDuplicates); - } - - private Set<FileReference> getObsoleteFileReferences(List<FileReference> newReferences) { - Set<FileReference> obsoleteReferences = new HashSet<>(reference2Bundles.keySet()); - obsoleteReferences.removeAll(newReferences); - return obsoleteReferences; - } - private Set<Bundle> allActiveBundles() { return reference2Bundles.keySet().stream() .flatMap(reference -> reference2Bundles.get(reference).stream()) .collect(Collectors.toSet()); } - /** - * Installs the given set of bundles and returns the set of bundles that is no longer used - * by the application, and should therefore be scheduled for uninstall. - */ - public synchronized Set<Bundle> use(List<FileReference> newBundles) { - // Must be done before allowing duplicates because allowed duplicates affect osgi.getCurrentBundles - Set<Bundle> bundlesToUninstall = getObsoleteBundles(newBundles); - - allowDuplicateBundles(newBundles); - removeInactiveFileReferences(newBundles); - - install(newBundles); - startBundles(); - - bundlesToUninstall.removeAll(allActiveBundles()); - log.info("Bundles to schedule for uninstall: " + bundlesToUninstall); - - log.info(installedBundlesMessage()); - return bundlesToUninstall; - } - private String installedBundlesMessage() { StringBuilder sb = new StringBuilder("Installed bundles: {" ); for (Bundle b : osgi.getBundles()) @@ -236,4 +226,14 @@ public class BundleLoader { return sb.toString(); } + // Only for testing + void useCustomBundleInstaller(BundleInstaller bundleInstaller) { + customBundleInstaller = bundleInstaller; + } + + // Only for testing + List<FileReference> getActiveFileReferences() { + return new ArrayList<>(reference2Bundles.keySet()); + } + } diff --git a/container-core/src/main/java/com/yahoo/container/core/config/DiskBundleInstaller.java b/container-core/src/main/java/com/yahoo/container/core/config/DiskBundleInstaller.java new file mode 100644 index 00000000000..3edabe9f861 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/container/core/config/DiskBundleInstaller.java @@ -0,0 +1,31 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.core.config; + +import com.yahoo.config.FileReference; +import com.yahoo.osgi.Osgi; +import org.osgi.framework.Bundle; + +import java.io.File; +import java.util.List; + +import static com.yahoo.container.core.BundleLoaderProperties.DISK_BUNDLE_PREFIX; + +/** + * @author gjoranv + */ +public class DiskBundleInstaller implements BundleInstaller { + + @Override + public List<Bundle> installBundles(FileReference reference, Osgi osgi) { + assert(reference.value().startsWith(DISK_BUNDLE_PREFIX)); + String referenceFileName = reference.value().substring(DISK_BUNDLE_PREFIX.length()); + + File file = new File(referenceFileName); + if ( ! file.exists()) { + throw new IllegalArgumentException("Reference '" + reference.value() + "' not found on disk."); + } + + return osgi.install(file.getAbsolutePath()); + } + +} diff --git a/container-core/src/main/java/com/yahoo/container/core/config/FileAcquirerBundleInstaller.java b/container-core/src/main/java/com/yahoo/container/core/config/FileAcquirerBundleInstaller.java new file mode 100644 index 00000000000..6f6f86a6366 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/container/core/config/FileAcquirerBundleInstaller.java @@ -0,0 +1,30 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.core.config; + +import com.yahoo.config.FileReference; +import com.yahoo.filedistribution.fileacquirer.FileAcquirer; +import com.yahoo.osgi.Osgi; +import org.osgi.framework.Bundle; + +import java.io.File; +import java.util.List; +import java.util.concurrent.TimeUnit; + +/** + * @author gjoranv + */ +public class FileAcquirerBundleInstaller implements BundleInstaller { + + private final FileAcquirer fileAcquirer; + + public FileAcquirerBundleInstaller(FileAcquirer fileAcquirer) { + this.fileAcquirer = fileAcquirer; + } + + @Override + public List<Bundle> installBundles(FileReference reference, Osgi osgi) throws InterruptedException { + File file = fileAcquirer.waitFor(reference, 7, TimeUnit.DAYS); + return osgi.install(file.getAbsolutePath()); + } + +} diff --git a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java index ef132694e10..d87b38e8b18 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java @@ -9,7 +9,6 @@ import com.yahoo.component.ComponentSpecification; import com.yahoo.component.provider.ComponentRegistry; import com.yahoo.concurrent.ThreadFactoryFactory; import com.yahoo.config.FileReference; -import com.yahoo.container.core.config.testutil.MockOsgiWrapper; import com.yahoo.container.di.ComponentDeconstructor; import com.yahoo.container.di.Container; import com.yahoo.container.di.componentgraph.core.ComponentGraph; @@ -82,7 +81,7 @@ public class HandlersConfigurerDi { OsgiFramework osgiFramework) { this(subscriberFactory, vespaContainer, configId, deconstructor, discInjector, - new ContainerAndDiOsgi(osgiFramework, new BundleLoader(new OsgiImpl(osgiFramework)))); + new ContainerAndDiOsgi(osgiFramework, new BundleManager(new OsgiImpl(osgiFramework)))); } // Only public for testing @@ -102,9 +101,9 @@ public class HandlersConfigurerDi { private static class ContainerAndDiOsgi extends OsgiImpl implements OsgiWrapper { private final OsgiFramework osgiFramework; - private final BundleLoader bundleLoader; + private final BundleManager bundleLoader; - public ContainerAndDiOsgi(OsgiFramework osgiFramework, BundleLoader bundleLoader) { + public ContainerAndDiOsgi(OsgiFramework osgiFramework, BundleManager bundleLoader) { super(osgiFramework); this.osgiFramework = osgiFramework; this.bundleLoader = bundleLoader; diff --git a/container-core/src/test/java/com/yahoo/container/core/config/BundleManagerTest.java b/container-core/src/test/java/com/yahoo/container/core/config/BundleManagerTest.java new file mode 100644 index 00000000000..414e6b05128 --- /dev/null +++ b/container-core/src/test/java/com/yahoo/container/core/config/BundleManagerTest.java @@ -0,0 +1,106 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.core.config; + +import com.yahoo.config.FileReference; +import org.junit.Before; +import org.junit.Test; +import org.osgi.framework.Bundle; + +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * @author gjoranv + */ +public class BundleManagerTest { + + private static final FileReference BUNDLE_1_REF = new FileReference("bundle-1"); + private static final Bundle BUNDLE_1 = new TestBundle(BUNDLE_1_REF.value()); + private static final FileReference BUNDLE_2_REF = new FileReference("bundle-2"); + private static final Bundle BUNDLE_2 = new TestBundle(BUNDLE_2_REF.value()); + + private BundleManager bundleLoader; + private TestOsgi osgi; + + @Before + public void setup() { + osgi = new TestOsgi(testBundles()); + var bundleInstaller = new TestBundleInstaller(); + bundleLoader = new BundleManager(osgi); + bundleLoader.useCustomBundleInstaller(bundleInstaller); + } + + @Test + public void bundles_are_installed_and_started() { + bundleLoader.use(List.of(BUNDLE_1_REF)); + assertEquals(1, osgi.getInstalledBundles().size()); + + // The bundle is installed and started + TestBundle installedBundle = (TestBundle)osgi.getInstalledBundles().get(0); + assertEquals(BUNDLE_1.getSymbolicName(), installedBundle.getSymbolicName()); + assertTrue(installedBundle.started); + + // The file reference is active + assertEquals(1, bundleLoader.getActiveFileReferences().size()); + assertEquals(BUNDLE_1_REF, bundleLoader.getActiveFileReferences().get(0)); + } + + @Test + public void new_bundle_can_be_installed_in_reconfig() { + bundleLoader.use(List.of(BUNDLE_1_REF)); + Set<Bundle> obsoleteBundles = bundleLoader.use(List.of(BUNDLE_1_REF, BUNDLE_2_REF)); + + // No bundles are obsolete + assertTrue(obsoleteBundles.isEmpty()); + + // Both bundles are installed + assertEquals(2, osgi.getInstalledBundles().size()); + assertEquals(BUNDLE_1.getSymbolicName(), osgi.getInstalledBundles().get(0).getSymbolicName()); + assertEquals(BUNDLE_2.getSymbolicName(), osgi.getInstalledBundles().get(1).getSymbolicName()); + + // Both bundles are current + assertEquals(2, osgi.getCurrentBundles().size()); + assertEquals(BUNDLE_1.getSymbolicName(), osgi.getCurrentBundles().get(0).getSymbolicName()); + assertEquals(BUNDLE_2.getSymbolicName(), osgi.getCurrentBundles().get(1).getSymbolicName()); + + + // Both file references are active + assertEquals(2, bundleLoader.getActiveFileReferences().size()); + assertEquals(BUNDLE_1_REF, bundleLoader.getActiveFileReferences().get(0)); + assertEquals(BUNDLE_2_REF, bundleLoader.getActiveFileReferences().get(1)); + } + + @Test + public void unused_bundle_is_marked_obsolete_after_reconfig() { + bundleLoader.use(List.of(BUNDLE_1_REF)); + Set<Bundle> obsoleteBundles = bundleLoader.use(List.of(BUNDLE_2_REF)); + + // The returned set of obsolete bundles contains bundle-1 + assertEquals(1, obsoleteBundles.size()); + assertEquals(BUNDLE_1.getSymbolicName(), obsoleteBundles.iterator().next().getSymbolicName()); + + // Both bundles are installed + assertEquals(2, osgi.getInstalledBundles().size()); + assertEquals(BUNDLE_1.getSymbolicName(), osgi.getInstalledBundles().get(0).getSymbolicName()); + assertEquals(BUNDLE_2.getSymbolicName(), osgi.getInstalledBundles().get(1).getSymbolicName()); + + // Only bundle-2 is current + assertEquals(1, osgi.getCurrentBundles().size()); + assertEquals(BUNDLE_2.getSymbolicName(), osgi.getCurrentBundles().get(0).getSymbolicName()); + + // Only the bundle-2 file reference is active + assertEquals(1, bundleLoader.getActiveFileReferences().size()); + assertEquals(BUNDLE_2_REF, bundleLoader.getActiveFileReferences().get(0)); + } + + + private static Map<String, Bundle> testBundles() { + return Map.of(BUNDLE_1_REF.value(), BUNDLE_1, + BUNDLE_2_REF.value(), BUNDLE_2); + } + +} diff --git a/container-core/src/test/java/com/yahoo/container/core/config/TestBundle.java b/container-core/src/test/java/com/yahoo/container/core/config/TestBundle.java new file mode 100644 index 00000000000..421f4302c27 --- /dev/null +++ b/container-core/src/test/java/com/yahoo/container/core/config/TestBundle.java @@ -0,0 +1,102 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.core.config; + +import com.yahoo.container.bundle.MockBundle; +import org.osgi.framework.Bundle; +import org.osgi.framework.Version; +import org.osgi.framework.wiring.BundleCapability; +import org.osgi.framework.wiring.BundleRequirement; +import org.osgi.framework.wiring.BundleRevision; +import org.osgi.framework.wiring.BundleWiring; +import org.osgi.resource.Capability; +import org.osgi.resource.Requirement; + +import java.util.List; + +/** + * @author gjoranv + */ +class TestBundle extends MockBundle { + + private static final BundleRevision revision = new TestBundleRevision(); + + private final String symbolicName; + + boolean started = false; + + TestBundle(String symbolicName) { + this.symbolicName = symbolicName; + } + + @Override + public void start() { + started = true; + } + + @Override + public String getSymbolicName() { + return symbolicName; + } + + + @SuppressWarnings("unchecked") + @Override + public <T> T adapt(Class<T> type) { + if (type.equals(BundleRevision.class)) { + return (T) revision; + } else { + throw new UnsupportedOperationException(); + } + } + + + static class TestBundleRevision implements BundleRevision { + + // Ensure this is not seen as a fragment bundle. + @Override + public int getTypes() { + return 0; + } + + @Override + public String getSymbolicName() { + throw new UnsupportedOperationException(); + } + + @Override + public Version getVersion() { + throw new UnsupportedOperationException(); + } + + @Override + public List<BundleCapability> getDeclaredCapabilities(String namespace) { + throw new UnsupportedOperationException(); + } + + @Override + public List<BundleRequirement> getDeclaredRequirements(String namespace) { + throw new UnsupportedOperationException(); + } + + @Override + public BundleWiring getWiring() { + throw new UnsupportedOperationException(); + } + + @Override + public List<Capability> getCapabilities(String namespace) { + throw new UnsupportedOperationException(); + } + + @Override + public List<Requirement> getRequirements(String namespace) { + throw new UnsupportedOperationException(); + } + + @Override + public Bundle getBundle() { + throw new UnsupportedOperationException(); + } + } + +} diff --git a/container-core/src/test/java/com/yahoo/container/core/config/TestBundleInstaller.java b/container-core/src/test/java/com/yahoo/container/core/config/TestBundleInstaller.java new file mode 100644 index 00000000000..43a5268eabf --- /dev/null +++ b/container-core/src/test/java/com/yahoo/container/core/config/TestBundleInstaller.java @@ -0,0 +1,20 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.core.config; + +import com.yahoo.config.FileReference; +import com.yahoo.osgi.Osgi; +import org.osgi.framework.Bundle; + +import java.util.List; + +/** + * @author gjoranv + */ +class TestBundleInstaller implements BundleInstaller { + + @Override + public List<Bundle> installBundles(FileReference reference, Osgi osgi) { + return osgi.install(reference.value()); + } + +} diff --git a/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java b/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java new file mode 100644 index 00000000000..54a3159239c --- /dev/null +++ b/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java @@ -0,0 +1,57 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.core.config; + +import com.yahoo.osgi.MockOsgi; +import org.osgi.framework.Bundle; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Map; + +/** + * @author gjoranv + */ +class TestOsgi extends MockOsgi { + + private final Map<String, Bundle> availableBundles; + + private final List<Bundle> installedBundles = new ArrayList<>(); + private final List<Bundle> allowedDuplicates = new ArrayList<>(); + + TestOsgi(Map<String, Bundle> availableBundles) { + this.availableBundles = availableBundles; + } + + @Override + public List<Bundle> install(String fileReferenceValue) { + if (! availableBundles.containsKey(fileReferenceValue)) + throw new IllegalArgumentException("No such bundle: " + fileReferenceValue); + + Bundle bundle = availableBundles.get(fileReferenceValue); + installedBundles.add(bundle); + return List.of(bundle); + } + + @Override + public Bundle[] getBundles() { + return installedBundles.toArray(new Bundle[0]); + } + + public List<Bundle> getInstalledBundles() { + return installedBundles; + } + + @Override + public List<Bundle> getCurrentBundles() { + var currentBundles = new ArrayList<>(installedBundles); + currentBundles.removeAll(allowedDuplicates); + return currentBundles; + } + + @Override + public void allowDuplicateBundles(Collection<Bundle> bundles) { + allowedDuplicates.addAll(bundles); + } + +} diff --git a/container-dependency-versions/pom.xml b/container-dependency-versions/pom.xml index 3e73e8bdf5e..b06734b3f87 100644 --- a/container-dependency-versions/pom.xml +++ b/container-dependency-versions/pom.xml @@ -437,7 +437,7 @@ <properties> <aopalliance.version>1.0</aopalliance.version> - <bouncycastle.version>1.63</bouncycastle.version> + <bouncycastle.version>1.65</bouncycastle.version> <felix.version>6.0.3</felix.version> <felix.log.version>1.0.1</felix.log.version> <findbugs.version>1.3.9</findbugs.version> @@ -459,7 +459,7 @@ <hk2.version>2.5.0-b32</hk2.version> <hk2.osgi-resource-locator.version>1.0.1</hk2.osgi-resource-locator.version> <jackson2.version>2.8.11</jackson2.version> - <jackson-databind.version>${jackson2.version}.2</jackson-databind.version> + <jackson-databind.version>${jackson2.version}.6</jackson-databind.version> <javassist.version>3.20.0-GA</javassist.version> <javax.annotation-api.version>1.2</javax.annotation-api.version> <javax.validation-api.version>1.1.0.Final</javax.validation-api.version> diff --git a/container-dev/pom.xml b/container-dev/pom.xml index 738a4cc8700..1bb06ab9694 100644 --- a/container-dev/pom.xml +++ b/container-dev/pom.xml @@ -176,6 +176,10 @@ <groupId>com.google.protobuf</groupId> <artifactId>protobuf-java</artifactId> </exclusion> + <exclusion> + <groupId>org.apache.commons</groupId> + <artifactId>commons-math3</artifactId> + </exclusion> </exclusions> </dependency> <dependency> diff --git a/container-search/pom.xml b/container-search/pom.xml index 84ee5b2bc65..6fa32947869 100644 --- a/container-search/pom.xml +++ b/container-search/pom.xml @@ -132,6 +132,11 @@ <scope>compile</scope> </dependency> <dependency> + <groupId>org.apache.commons</groupId> + <artifactId>commons-math3</artifactId> + <scope>compile</scope> + </dependency> + <dependency> <groupId>javax.xml.bind</groupId> <artifactId>jaxb-api</artifactId> <scope>test</scope> diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/SpecialTokens.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/SpecialTokens.java index c206ff7567e..8020088c2e3 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/SpecialTokens.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/SpecialTokens.java @@ -159,7 +159,7 @@ public class SpecialTokens { @Override public int hashCode() { return token.hashCode(); } - public Token toToken(int start,String rawSource) { + public Token toToken(int start, String rawSource) { return new Token(Token.Kind.WORD, replace(), true, new Substring(start, start + token.length(), rawSource)); // XXX: Unsafe? } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java index 9c46d194fb3..626cf087aca 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java @@ -32,6 +32,7 @@ import java.util.List; import java.util.Optional; import java.util.OptionalInt; import java.util.Set; +import java.util.stream.Collectors; /** * A dispatcher communicates with search nodes to perform queries and fill hits. @@ -50,6 +51,7 @@ public class Dispatcher extends AbstractComponent { public static final String DISPATCH = "dispatch"; private static final String INTERNAL = "internal"; private static final String PROTOBUF = "protobuf"; + private static final String TOP_K_PROBABILITY = "topKProbability"; private static final String INTERNAL_METRIC = "dispatch_internal"; @@ -58,6 +60,9 @@ public class Dispatcher extends AbstractComponent { /** If enabled, search queries will use protobuf rpc */ public static final CompoundName dispatchProtobuf = CompoundName.fromComponents(DISPATCH, PROTOBUF); + /** If set will control computation of how many hits will be fetched from each partition.*/ + public static final CompoundName topKProbability = CompoundName.fromComponents(DISPATCH, TOP_K_PROBABILITY); + /** A model of the search cluster this dispatches to */ private final SearchCluster searchCluster; private final ClusterMonitor clusterMonitor; @@ -79,6 +84,7 @@ public class Dispatcher extends AbstractComponent { argumentType.setBuiltin(true); argumentType.addField(new FieldDescription(INTERNAL, FieldType.booleanType)); argumentType.addField(new FieldDescription(PROTOBUF, FieldType.booleanType)); + argumentType.addField(new FieldDescription(TOP_K_PROBABILITY, FieldType.doubleType)); argumentType.freeze(); } @@ -215,7 +221,7 @@ public class Dispatcher extends AbstractComponent { int covered = searchCluster.groupsWithSufficientCoverage(); int groups = searchCluster.orderedGroups().size(); int max = Integer.min(Integer.min(covered + 1, groups), MAX_GROUP_SELECTION_ATTEMPTS); - Set<Integer> rejected = null; + Set<Integer> rejected = rejectGroupBlockingFeed(searchCluster.orderedGroups()); for (int i = 0; i < max; i++) { Optional<Group> groupInCluster = loadBalancer.takeGroup(rejected); if (groupInCluster.isEmpty()) break; // No groups available @@ -244,4 +250,21 @@ public class Dispatcher extends AbstractComponent { throw new IllegalStateException("No suitable groups to dispatch query. Rejected: " + rejected); } + /** + * We want to avoid groups blocking feed because their data may be out of date. + * If there is a single group blocking feed, we want to reject it. + * If multiple groups are blocking feed we should use them anyway as we may not have remaining + * capacity otherwise. Same if there are no other groups. + * + * @return a modifiable set containing the single group to reject, or null otherwise + */ + private Set<Integer> rejectGroupBlockingFeed(List<Group> groups) { + if (groups.size() == 1) return null; + List<Group> groupsRejectingFeed = groups.stream().filter(Group::isBlockingWrites).collect(Collectors.toList()); + if (groupsRejectingFeed.size() != 1) return null; + Set<Integer> rejected = new HashSet<>(); + rejected.add(groupsRejectingFeed.get(0).id()); + return rejected; + } + } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java b/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java index cec3e94d551..e62848a7f9e 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java @@ -81,7 +81,12 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM int originalHits = query.getHits(); int originalOffset = query.getOffset(); - query.setHits(query.getHits() + query.getOffset()); + int neededHits = originalHits + originalOffset; + Double topkProbabilityOverrride = query.properties().getDouble(Dispatcher.topKProbability); + int q = (topkProbabilityOverrride != null) + ? searchCluster.estimateHitsToFetch(neededHits, invokers.size(), topkProbabilityOverrride) + : searchCluster.estimateHitsToFetch(neededHits, invokers.size()); + query.setHits(q); query.setOffset(0); for (SearchInvoker invoker : invokers) { @@ -321,4 +326,7 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM protected LinkedBlockingQueue<SearchInvoker> newQueue() { return new LinkedBlockingQueue<>(); } + + // For testing + Collection<SearchInvoker> invokers() { return invokers; } } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java b/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java index 1c3a90ac6ab..03160e6c9c7 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java @@ -43,7 +43,7 @@ public abstract class InvokerFactory { * @param nodes pre-selected list of content nodes * @param acceptIncompleteCoverage if some of the nodes are unavailable and this parameter is * false, verify that the remaining set of nodes has sufficient coverage - * @return Optional containing the SearchInvoker or empty if some node in the + * @return the invoker or empty if some node in the * list is invalid and the remaining coverage is not sufficient */ public Optional<SearchInvoker> createSearchInvoker(VespaBackEndSearcher searcher, @@ -82,7 +82,7 @@ public abstract class InvokerFactory { if ( ! searchCluster.isPartialGroupCoverageSufficient(groupId, success) && !acceptIncompleteCoverage) { return Optional.empty(); } - if(invokers.size() == 0) { + if (invokers.size() == 0) { return Optional.of(createCoverageErrorInvoker(nodes, failed)); } } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/LeanHit.java b/container-search/src/main/java/com/yahoo/search/dispatch/LeanHit.java index 86f1999d8b4..8a90557fa3b 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/LeanHit.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/LeanHit.java @@ -12,15 +12,11 @@ public class LeanHit implements Comparable<LeanHit> { private final int distributionKey; public LeanHit(byte [] gid, int partId, int distributionKey, double relevance) { - this.gid = gid; - this.relevance = Double.isNaN(relevance) ? Double.NEGATIVE_INFINITY : relevance; - this.sortData = null; - this.partId = partId; - this.distributionKey = distributionKey; + this(gid, partId, distributionKey, relevance, null); } - public LeanHit(byte [] gid, int partId, int distributionKey, byte [] sortData) { + public LeanHit(byte [] gid, int partId, int distributionKey, double relevance, byte [] sortData) { this.gid = gid; - this.relevance = 0.0; + this.relevance = Double.isNaN(relevance) ? Double.NEGATIVE_INFINITY : relevance; this.sortData = sortData; this.partId = partId; this.distributionKey = distributionKey; diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java b/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java index 210ab5777d2..05e1ea6e2f9 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java @@ -134,6 +134,7 @@ public class LoadBalancer { } private static class RoundRobinScheduler implements GroupScheduler { + private int needle = 0; private final List<GroupStatus> scoreboard; @@ -204,6 +205,7 @@ public class LoadBalancer { } static class AdaptiveScheduler implements GroupScheduler { + private final Random random; private final List<GroupStatus> scoreboard; @@ -251,4 +253,5 @@ public class LoadBalancer { return selectGroup(needle, false, rejectedGroups); } } + } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/TopKEstimator.java b/container-search/src/main/java/com/yahoo/search/dispatch/TopKEstimator.java new file mode 100644 index 00000000000..8003d9c6744 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/dispatch/TopKEstimator.java @@ -0,0 +1,42 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.dispatch; + +import org.apache.commons.math3.distribution.TDistribution; + +/** + * Use StudentT distribution and estimate how many hits you need from each partition + * to to get the globally top-k documents with the desired probability + * @author baldersheim + */ +public class TopKEstimator { + private final TDistribution studentT; + private final double defaultP; + private final boolean estimate; + + private static boolean needEstimate(double p) { + return (0.0 < p) && (p < 1.0); + } + public TopKEstimator(double freedom, double defaultProbability) { + this.studentT = new TDistribution(null, freedom); + defaultP = defaultProbability; + estimate = needEstimate(defaultP); + } + double estimateExactK(double k, double n, double p) { + double variance = k * 1/n * (1 - 1/n); + double p_inverse = 1 - (1 - p)/n; + return k/n + studentT.inverseCumulativeProbability(p_inverse) * Math.sqrt(variance); + } + double estimateExactK(double k, double n) { + return estimateExactK(k, n, defaultP); + } + public int estimateK(int k, int n) { + return (estimate && n > 1) + ? (int)Math.ceil(estimateExactK(k, n, defaultP)) + : k; + } + public int estimateK(int k, int n, double p) { + return (needEstimate(p) && (n > 1)) + ? (int)Math.ceil(estimateExactK(k, n, p)) + : k; + } +} diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java index ae2258c4546..51290c245ac 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java @@ -214,7 +214,7 @@ public class ProtobufSerialization { for (var replyHit : protobuf.getHitsList()) { LeanHit hit = (replyHit.getSortData().isEmpty()) ? new LeanHit(replyHit.getGlobalId().toByteArray(), partId, distKey, replyHit.getRelevance()) - : new LeanHit(replyHit.getGlobalId().toByteArray(), partId, distKey, replyHit.getSortData().toByteArray()); + : new LeanHit(replyHit.getGlobalId().toByteArray(), partId, distKey, replyHit.getRelevance(), replyHit.getSortData().toByteArray()); result.getLeanHits().add(hit); } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java index 0e4e87b9a6a..ec616a18e09 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java @@ -21,6 +21,7 @@ public class Group { private final AtomicBoolean hasSufficientCoverage = new AtomicBoolean(true); private final AtomicBoolean hasFullCoverage = new AtomicBoolean(true); private final AtomicLong activeDocuments = new AtomicLong(0); + private final AtomicBoolean isBlockingWrites = new AtomicBoolean(false); public Group(int id, List<Node> nodes) { this.id = id; @@ -61,21 +62,16 @@ public class Group { return nodesUp; } - void aggregateActiveDocuments() { - long activeDocumentsInGroup = 0; - for (Node node : nodes) { - if (node.isWorking() == Boolean.TRUE) { - activeDocumentsInGroup += node.getActiveDocuments(); - } - } - activeDocuments.set(activeDocumentsInGroup); - + void aggregateNodeValues() { + activeDocuments.set(nodes.stream().filter(node -> node.isWorking() == Boolean.TRUE).mapToLong(Node::getActiveDocuments).sum()); + isBlockingWrites.set(nodes.stream().anyMatch(node -> node.isBlockingWrites())); } /** Returns the active documents on this node. If unknown, 0 is returned. */ - long getActiveDocuments() { - return this.activeDocuments.get(); - } + long getActiveDocuments() { return activeDocuments.get(); } + + /** Returns whether any node in this group is currently blocking write operations */ + public boolean isBlockingWrites() { return isBlockingWrites.get(); } public boolean isFullCoverageStatusChanged(boolean hasFullCoverageNow) { boolean previousState = hasFullCoverage.getAndSet(hasFullCoverageNow); diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java index e93b633f09d..8f465070de4 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java @@ -23,6 +23,7 @@ public class Node { private final AtomicLong activeDocuments = new AtomicLong(0); private final AtomicLong pingSequence = new AtomicLong(0); private final AtomicLong lastPong = new AtomicLong(0); + private final AtomicBoolean isBlockingWrites = new AtomicBoolean(false); public Node(int key, String hostname, int group) { this.key = key; @@ -70,14 +71,14 @@ public class Node { } /** Updates the active documents on this node */ - void setActiveDocuments(long activeDocuments) { - this.activeDocuments.set(activeDocuments); - } + void setActiveDocuments(long activeDocuments) { this.activeDocuments.set(activeDocuments); } /** Returns the active documents on this node. If unknown, 0 is returned. */ - long getActiveDocuments() { - return activeDocuments.get(); - } + long getActiveDocuments() { return activeDocuments.get(); } + + public void setBlockingWrites(boolean isBlockingWrites) { this.isBlockingWrites.set(isBlockingWrites); } + + boolean isBlockingWrites() { return isBlockingWrites.get(); } @Override public int hashCode() { return Objects.hash(hostname, key, pathIndex, group); } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java index 7862648ba51..7dfc03fd2d7 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java @@ -10,6 +10,7 @@ import com.yahoo.net.HostName; import com.yahoo.prelude.Pong; import com.yahoo.search.cluster.ClusterMonitor; import com.yahoo.search.cluster.NodeManager; +import com.yahoo.search.dispatch.TopKEstimator; import com.yahoo.vespa.config.search.DispatchConfig; import java.util.LinkedHashMap; @@ -38,6 +39,7 @@ public class SearchCluster implements NodeManager<Node> { private final ImmutableList<Group> orderedGroups; private final VipStatus vipStatus; private final PingFactory pingFactory; + private final TopKEstimator hitEstimator; private long nextLogTime = 0; /** @@ -76,6 +78,7 @@ public class SearchCluster implements NodeManager<Node> { for (Node node : nodes) nodesByHostBuilder.put(node.hostname(), node); this.nodesByHost = nodesByHostBuilder.build(); + hitEstimator = new TopKEstimator(30.0, dispatchConfig.topKProbability()); this.localCorpusDispatchTarget = findLocalCorpusDispatchTarget(HostName.getLocalhost(), size, @@ -91,7 +94,7 @@ public class SearchCluster implements NodeManager<Node> { } public void addMonitoring(ClusterMonitor clusterMonitor) { - for (var group : orderedGroups) { + for (var group : orderedGroups()) { for (var node : group.nodes()) clusterMonitor.add(node, true); } @@ -147,8 +150,8 @@ public class SearchCluster implements NodeManager<Node> { /** Returns the n'th (zero-indexed) group in the cluster if possible */ public Optional<Group> group(int n) { - if (orderedGroups.size() > n) { - return Optional.of(orderedGroups.get(n)); + if (orderedGroups().size() > n) { + return Optional.of(orderedGroups().get(n)); } else { return Optional.empty(); } @@ -156,13 +159,13 @@ public class SearchCluster implements NodeManager<Node> { /** Returns the number of nodes per group - size()/groups.size() */ public int groupSize() { - if (groups.size() == 0) return size(); - return size() / groups.size(); + if (groups().size() == 0) return size(); + return size() / groups().size(); } public int groupsWithSufficientCoverage() { int covered = 0; - for (Group g : orderedGroups) { + for (Group g : orderedGroups()) { if (g.hasSufficientCoverage()) { covered++; } @@ -178,7 +181,7 @@ public class SearchCluster implements NodeManager<Node> { if ( localCorpusDispatchTarget.isEmpty()) return Optional.empty(); // Only use direct dispatch if the local group has sufficient coverage - Group localSearchGroup = groups.get(localCorpusDispatchTarget.get().group()); + Group localSearchGroup = groups().get(localCorpusDispatchTarget.get().group()); if ( ! localSearchGroup.hasSufficientCoverage()) return Optional.empty(); // Only use direct dispatch if the local search node is not down @@ -240,6 +243,13 @@ public class SearchCluster implements NodeManager<Node> { vipStatus.removeFromRotation(clusterId); } + public int estimateHitsToFetch(int wantedHits, int numPartitions) { + return hitEstimator.estimateK(wantedHits, numPartitions); + } + public int estimateHitsToFetch(int wantedHits, int numPartitions, double topKProbability) { + return hitEstimator.estimateK(wantedHits, numPartitions, topKProbability); + } + public boolean hasInformationAboutAllNodes() { return nodesByHost.values().stream().allMatch(node -> node.isWorking() != null); } @@ -257,12 +267,15 @@ public class SearchCluster implements NodeManager<Node> { } private static class PongCallback implements PongHandler { + private final ClusterMonitor<Node> clusterMonitor; private final Node node; + PongCallback(Node node, ClusterMonitor<Node> clusterMonitor) { this.node = node; this.clusterMonitor = clusterMonitor; } + @Override public void handle(Pong pong) { if (pong.badResponse()) { @@ -270,10 +283,12 @@ public class SearchCluster implements NodeManager<Node> { } else { if (pong.activeDocuments().isPresent()) { node.setActiveDocuments(pong.activeDocuments().get()); + node.setBlockingWrites(pong.isBlockingWrites()); } clusterMonitor.responded(node); } } + } /** Used by the cluster monitor to manage node status */ @@ -284,8 +299,8 @@ public class SearchCluster implements NodeManager<Node> { } private void pingIterationCompletedSingleGroup() { - Group group = groups.values().iterator().next(); - group.aggregateActiveDocuments(); + Group group = groups().values().iterator().next(); + group.aggregateNodeValues(); // With just one group sufficient coverage may not be the same as full coverage, as the // group will always be marked sufficient for use. updateSufficientCoverage(group, true); @@ -295,21 +310,20 @@ public class SearchCluster implements NodeManager<Node> { } private void pingIterationCompletedMultipleGroups() { - int numGroups = orderedGroups.size(); + int numGroups = orderedGroups().size(); // Update active documents per group and use it to decide if the group should be active - long[] activeDocumentsInGroup = new long[numGroups]; long sumOfActiveDocuments = 0; for(int i = 0; i < numGroups; i++) { - Group group = orderedGroups.get(i); - group.aggregateActiveDocuments(); + Group group = orderedGroups().get(i); + group.aggregateNodeValues(); activeDocumentsInGroup[i] = group.getActiveDocuments(); sumOfActiveDocuments += activeDocumentsInGroup[i]; } boolean anyGroupsSufficientCoverage = false; for (int i = 0; i < numGroups; i++) { - Group group = orderedGroups.get(i); + Group group = orderedGroups().get(i); long activeDocuments = activeDocumentsInGroup[i]; long averageDocumentsInOtherGroups = (sumOfActiveDocuments - activeDocuments) / (numGroups - 1); boolean sufficientCoverage = isGroupCoverageSufficient(group.workingNodes(), group.nodes().size(), activeDocuments, averageDocumentsInOtherGroups); @@ -326,7 +340,7 @@ public class SearchCluster implements NodeManager<Node> { */ @Override public void pingIterationCompleted() { - int numGroups = orderedGroups.size(); + int numGroups = orderedGroups().size(); if (numGroups == 1) { pingIterationCompletedSingleGroup(); } else { @@ -357,7 +371,7 @@ public class SearchCluster implements NodeManager<Node> { * Calculate whether a subset of nodes in a group has enough coverage */ public boolean isPartialGroupCoverageSufficient(OptionalInt knownGroupId, List<Node> nodes) { - if (orderedGroups.size() == 1) { + if (orderedGroups().size() == 1) { boolean sufficient = nodes.size() >= groupSize() - dispatchConfig.maxNodesDownPerGroup(); return sufficient; } @@ -366,14 +380,14 @@ public class SearchCluster implements NodeManager<Node> { return false; } int groupId = knownGroupId.getAsInt(); - Group group = groups.get(groupId); + Group group = groups().get(groupId); if (group == null) { return false; } int nodesInGroup = group.nodes().size(); long sumOfActiveDocuments = 0; int otherGroups = 0; - for (Group g : orderedGroups) { + for (Group g : orderedGroups()) { if (g.id() != groupId) { sumOfActiveDocuments += g.getActiveDocuments(); otherGroups++; diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java index 5433a28dd6e..eaafb2d8b8a 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java @@ -35,7 +35,7 @@ public class DispatcherTest { q.getModel().setSearchPath("1/0"); // second node in first group MockInvokerFactory invokerFactory = new MockInvokerFactory(cl, (nodes, a) -> { assertEquals(1, nodes.size()); - assertEquals(2, nodes.get(0).key()); + assertEquals(1, nodes.get(0).key()); return true; }); Dispatcher disp = new Dispatcher(new ClusterMonitor(cl, false), cl, createDispatchConfig(), invokerFactory, new MockMetric()); @@ -92,6 +92,53 @@ public class DispatcherTest { } } + @Test + public void testGroup0IsSelected() { + SearchCluster cluster = new MockSearchCluster("1", 3, 1); + Dispatcher dispatcher = new Dispatcher(new ClusterMonitor(cluster, false), cluster, createDispatchConfig(), new MockInvokerFactory(cluster, (n, a) -> true), new MockMetric()); + cluster.pingIterationCompleted(); + assertEquals(0, + dispatcher.getSearchInvoker(new Query(), null).distributionKey().get().longValue()); + dispatcher.deconstruct(); + } + + @Test + public void testGroup0IsSkippedWhenItIsBlockingFeed() { + SearchCluster cluster = new MockSearchCluster("1", 3, 1); + Dispatcher dispatcher = new Dispatcher(new ClusterMonitor(cluster, false), cluster, createDispatchConfig(), new MockInvokerFactory(cluster, (n, a) -> true), new MockMetric()); + cluster.group(0).get().nodes().get(0).setBlockingWrites(true); + cluster.pingIterationCompleted(); + assertEquals("Blocking group is avoided", + 1, + (dispatcher.getSearchInvoker(new Query(), null).distributionKey().get()).longValue()); + dispatcher.deconstruct(); + } + + @Test + public void testGroup0IsSelectedWhenMoreAreBlockingFeed() { + SearchCluster cluster = new MockSearchCluster("1", 3, 1); + Dispatcher dispatcher = new Dispatcher(new ClusterMonitor(cluster, false), cluster, createDispatchConfig(), new MockInvokerFactory(cluster, (n, a) -> true), new MockMetric()); + cluster.group(0).get().nodes().get(0).setBlockingWrites(true); + cluster.group(1).get().nodes().get(0).setBlockingWrites(true); + cluster.pingIterationCompleted(); + assertEquals("Blocking group is used when multiple groups are blocking", + 0, + dispatcher.getSearchInvoker(new Query(), null).distributionKey().get().longValue()); + dispatcher.deconstruct(); + } + + @Test + public void testGroup0IsSelectedWhenItIsBlockingFeedWhenNoOthers() { + SearchCluster cluster = new MockSearchCluster("1", 1, 1); + Dispatcher dispatcher = new Dispatcher(new ClusterMonitor(cluster, false), cluster, createDispatchConfig(), new MockInvokerFactory(cluster, (n, a) -> true), new MockMetric()); + cluster.group(0).get().nodes().get(0).setBlockingWrites(true); + cluster.pingIterationCompleted(); + assertEquals("Blocking group is used when there is no alternative", + 0, + (dispatcher.getSearchInvoker(new Query(), null).distributionKey().get()).longValue()); + dispatcher.deconstruct(); + } + interface FactoryStep { boolean returnInvoker(List<Node> nodes, boolean acceptIncompleteCoverage); } @@ -119,7 +166,7 @@ public class DispatcherTest { boolean nonEmpty = events[step].returnInvoker(nodes, acceptIncompleteCoverage); step++; if (nonEmpty) { - return Optional.of(new MockInvoker(1)); + return Optional.of(new MockInvoker(nodes.get(0).key())); } else { return Optional.empty(); } @@ -150,4 +197,5 @@ public class DispatcherTest { return null; } } + } diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java index 27685426cf8..e16f09a58ab 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java @@ -204,6 +204,33 @@ public class InterleavedSearchInvokerTest { private static final List<Double> A5Aux = Arrays.asList(-1.0,11.0,8.5,7.5,-7.0,3.0,2.0); private static final List<Double> B5Aux = Arrays.asList(9.0,8.0,-3.0,7.0,6.0,1.0, -1.0); + private void validateThatTopKProbabilityOverrideTakesEffect(Double topKProbability, int expectedK) throws IOException { + InterleavedSearchInvoker invoker = createInterLeavedTestInvoker(A5, B5); + query.setHits(8); + query.properties().set(Dispatcher.topKProbability, topKProbability); + SearchInvoker [] invokers = invoker.invokers().toArray(new SearchInvoker[0]); + Result result = invoker.search(query, null); + assertEquals(2, invokers.length); + assertEquals(expectedK, ((MockInvoker)invokers[0]).hitsRequested); + assertEquals(8, result.hits().size()); + assertEquals(11.0, result.hits().get(0).getRelevance().getScore(), DELTA); + assertEquals(9.0, result.hits().get(1).getRelevance().getScore(), DELTA); + assertEquals(8.5, result.hits().get(2).getRelevance().getScore(), DELTA); + assertEquals(8.0, result.hits().get(3).getRelevance().getScore(), DELTA); + assertEquals(7.5, result.hits().get(4).getRelevance().getScore(), DELTA); + assertEquals(7.0, result.hits().get(5).getRelevance().getScore(), DELTA); + assertEquals(6.0, result.hits().get(6).getRelevance().getScore(), DELTA); + assertEquals(3.0, result.hits().get(7).getRelevance().getScore(), DELTA); + assertEquals(0, result.getQuery().getOffset()); + assertEquals(8, result.getQuery().getHits()); + } + + @Test + public void requireThatTopKProbabilityOverrideTakesEffect() throws IOException { + validateThatTopKProbabilityOverrideTakesEffect(null, 8); + validateThatTopKProbabilityOverrideTakesEffect(0.8, 6); + } + @Test public void requireThatMergeOfConcreteHitsObeySorting() throws IOException { InterleavedSearchInvoker invoker = createInterLeavedTestInvoker(A5, B5); diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/LeanHitTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/LeanHitTest.java index 085a9b24993..8d81c5d8521 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/LeanHitTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/LeanHitTest.java @@ -37,10 +37,14 @@ public class LeanHitTest { } @Test public void testOrderingBySortData() { - assertEquals(0, new LeanHit(gidA, 0, 0, gidA).compareTo(new LeanHit(gidA, 0, 0, gidA))); - verifyTransitiveOrdering(new LeanHit(gidA, 0, 0, gidA), - new LeanHit(gidA, 0, 0, gidB), - new LeanHit(gidA, 0, 0, gidC)); + assertEquals(0, new LeanHit(gidA, 0, 0, 0.0, gidA).compareTo(new LeanHit(gidA, 0, 0, 0.0, gidA))); + verifyTransitiveOrdering(new LeanHit(gidA, 0, 0, 0.0, gidA), + new LeanHit(gidA, 0, 0, 0.0, gidB), + new LeanHit(gidA, 0, 0, 0.0, gidC)); + } + @Test + public void testRelevanceIsKeptEvenWithBySortData() { + assertEquals(1.3, new LeanHit(gidA, 0, 0, 1.3, gidA).getRelevance(), 0.0); } @Test public void testNaN2negativeInfinity() { diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/MockInvoker.java b/container-search/src/test/java/com/yahoo/search/dispatch/MockInvoker.java index c5fbda7c2f5..c159293d7d9 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/MockInvoker.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/MockInvoker.java @@ -17,6 +17,7 @@ class MockInvoker extends SearchInvoker { private final Coverage coverage; private Query query; private List<Hit> hits; + int hitsRequested; protected MockInvoker(int key, Coverage coverage) { super(Optional.of(new Node(key, "?", 0))); @@ -35,6 +36,7 @@ class MockInvoker extends SearchInvoker { @Override protected void sendSearchRequest(Query query) throws IOException { this.query = query; + hitsRequested = query.getHits(); } @Override diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java b/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java index 32c6738fc3b..cafba58662e 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java @@ -35,15 +35,14 @@ public class MockSearchCluster extends SearchCluster { ImmutableList.Builder<Group> orderedGroupBuilder = ImmutableList.builder(); ImmutableMap.Builder<Integer, Group> groupBuilder = ImmutableMap.builder(); ImmutableMultimap.Builder<String, Node> hostBuilder = ImmutableMultimap.builder(); - int dk = 1; + int distributionKey = 0; for (int group = 0; group < groups; group++) { List<Node> nodes = new ArrayList<>(); for (int node = 0; node < nodesPerGroup; node++) { - Node n = new Node(dk, "host" + dk, group); - n.setWorking(true); + Node n = new Node(distributionKey, "host" + distributionKey, group); nodes.add(n); hostBuilder.put(n.hostname(), n); - dk++; + distributionKey++; } Group g = new Group(group, nodes); groupBuilder.put(group, g); diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java index 5a4457780e2..58042dcf228 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java @@ -18,6 +18,7 @@ import static org.junit.Assert.assertThat; * @author ollivir */ public class SearchPathTest { + @Test public void requreThatSearchPathsAreParsedCorrectly() { assertThat(SearchPath.fromString("0/0").get().toString(), equalTo("0/0")); @@ -71,11 +72,11 @@ public class SearchPathTest { public void searchPathMustFilterNodesBasedOnDefinition() { MockSearchCluster cluster = new MockSearchCluster("a",3, 3); - assertThat(distKeysAsString(SearchPath.selectNodes("1/1", cluster)), equalTo("5")); - assertThat(distKeysAsString(SearchPath.selectNodes("/1", cluster)), equalTo("4,5,6")); - assertThat(distKeysAsString(SearchPath.selectNodes("0,1/2", cluster)), equalTo("7,8")); - assertThat(distKeysAsString(SearchPath.selectNodes("[1,3>/1", cluster)), equalTo("5,6")); - assertThat(distKeysAsString(SearchPath.selectNodes("[1,88>/1", cluster)), equalTo("5,6")); + assertThat(distKeysAsString(SearchPath.selectNodes("1/1", cluster)), equalTo("4")); + assertThat(distKeysAsString(SearchPath.selectNodes("/1", cluster)), equalTo("3,4,5")); + assertThat(distKeysAsString(SearchPath.selectNodes("0,1/2", cluster)), equalTo("6,7")); + assertThat(distKeysAsString(SearchPath.selectNodes("[1,3>/1", cluster)), equalTo("4,5")); + assertThat(distKeysAsString(SearchPath.selectNodes("[1,88>/1", cluster)), equalTo("4,5")); } private static String distKeysAsString(Collection<Node> nodes) { diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/TopKEstimatorTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/TopKEstimatorTest.java new file mode 100644 index 00000000000..c14e4f984f1 --- /dev/null +++ b/container-search/src/test/java/com/yahoo/search/dispatch/TopKEstimatorTest.java @@ -0,0 +1,28 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.dispatch; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +public class TopKEstimatorTest { + @Test + public void requireHitsAreEstimatedAccordingToPartitionsAndProbability() { + TopKEstimator estimator = new TopKEstimator(30, 0.999); + assertEquals(91.97368471911312, estimator.estimateExactK(200, 3), 0.0); + assertEquals(92, estimator.estimateK(200, 3)); + assertEquals(37.96328109101396, estimator.estimateExactK(200, 10), 0.0); + assertEquals(38, estimator.estimateK(200, 10)); + assertEquals(23.815737601023095, estimator.estimateExactK(200, 20), 0.0); + assertEquals(24, estimator.estimateK(200, 20)); + + assertEquals(37.96328109101396, estimator.estimateExactK(200, 10, 0.999), 0.0); + assertEquals(38, estimator.estimateK(200, 10, 0.999)); + assertEquals(34.36212304875885, estimator.estimateExactK(200, 10, 0.99), 0.0); + assertEquals(35, estimator.estimateK(200, 10, 0.99)); + assertEquals(41.44244358524574, estimator.estimateExactK(200, 10, 0.9999), 0.0); + assertEquals(42, estimator.estimateK(200, 10, 0.9999)); + assertEquals(44.909040374464155, estimator.estimateExactK(200, 10, 0.99999), 0.0); + assertEquals(45, estimator.estimateK(200, 10, 0.99999)); + } +} diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java index ad281aeda7d..09024150a9a 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java @@ -8,6 +8,7 @@ import com.yahoo.net.HostName; import com.yahoo.prelude.Pong; import com.yahoo.search.cluster.ClusterMonitor; import com.yahoo.search.dispatch.MockSearchCluster; +import com.yahoo.search.dispatch.TopKEstimator; import com.yahoo.search.result.ErrorMessage; import org.junit.Test; diff --git a/controller-api/pom.xml b/controller-api/pom.xml index 5c744db2d6d..5d0bbff1c62 100644 --- a/controller-api/pom.xml +++ b/controller-api/pom.xml @@ -53,6 +53,13 @@ <version>${project.version}</version> </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>vespajlib</artifactId> + <scope>provided</scope> + <version>${project.version}</version> + </dependency> + <!-- compile --> <dependency> diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java index 0cb4baab790..1e42efdd256 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java @@ -1,8 +1,16 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.api.systemflags.v1; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.SystemName; +import com.yahoo.text.JSON; +import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagId; +import com.yahoo.vespa.flags.json.DimensionHelper; import com.yahoo.vespa.flags.json.FlagData; import java.io.BufferedInputStream; @@ -18,8 +26,11 @@ import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.TreeMap; +import java.util.function.Consumer; import java.util.stream.Collectors; import java.util.stream.Stream; import java.util.zip.ZipEntry; @@ -38,6 +49,8 @@ import static com.yahoo.yolean.Exceptions.uncheck; */ public class SystemFlagsDataArchive { + private static final ObjectMapper mapper = new ObjectMapper(); + private final Map<FlagId, Map<String, FlagData>> files; private SystemFlagsDataArchive(Map<FlagId, Map<String, FlagData>> files) { @@ -147,16 +160,65 @@ public class SystemFlagsDataArchive { if (rawData.isBlank()) { flagData = new FlagData(directoryDeducedFlagId); } else { - flagData = FlagData.deserialize(rawData); + String normalizedRawData = normalizeJson(rawData); + flagData = FlagData.deserialize(normalizedRawData); if (!directoryDeducedFlagId.equals(flagData.id())) { throw new IllegalArgumentException( String.format("Flag data file with flag id '%s' in directory for '%s'", - flagData.id(), directoryDeducedFlagId.toString())); + flagData.id(), directoryDeducedFlagId.toString())); + } + + String serializedData = flagData.serializeToJson(); + if (!JSON.equals(serializedData, normalizedRawData)) { + throw new IllegalArgumentException(filePath + " contains unknown non-comment fields: " + + "after removing any comment fields the JSON is:\n " + + normalizedRawData + + "\nbut deserializing this ended up with a JSON that are missing some of the fields:\n " + + serializedData + + "\nSee https://git.ouroath.com/vespa/hosted-feature-flags for more info on the JSON syntax"); } } builder.addFile(filename, flagData); } + static String normalizeJson(String json) { + JsonNode root = uncheck(() -> mapper.readTree(json)); + removeCommentsRecursively(root); + verifyValues(root); + return root.toString(); + } + + private static void verifyValues(JsonNode root) { + var cursor = new JsonAccessor(root); + cursor.get("rules").forEachArrayElement(rule -> rule.get("conditions").forEachArrayElement(condition -> { + var dimension = condition.get("dimension"); + if (dimension.isEqualTo(DimensionHelper.toWire(FetchVector.Dimension.APPLICATION_ID))) { + condition.get("values").forEachArrayElement(conditionValue -> { + String applicationIdString = conditionValue.asString() + .orElseThrow(() -> new IllegalArgumentException("Non-string application ID: " + conditionValue)); + // Throws exception if not recognized + ApplicationId.fromSerializedForm(applicationIdString); + }); + } else if (dimension.isEqualTo(DimensionHelper.toWire(FetchVector.Dimension.NODE_TYPE))) { + condition.get("values").forEachArrayElement(conditionValue -> { + String nodeTypeString = conditionValue.asString() + .orElseThrow(() -> new IllegalArgumentException("Non-string node type: " + conditionValue)); + // Throws exception if not recognized + NodeType.valueOf(nodeTypeString); + }); + } + })); + } + + private static void removeCommentsRecursively(JsonNode node) { + if (node instanceof ObjectNode) { + ObjectNode objectNode = (ObjectNode) node; + objectNode.remove("comment"); + } + + node.forEach(SystemFlagsDataArchive::removeCommentsRecursively); + } + private static String toFilePath(FlagId flagId, String filename) { return "flags/" + flagId.toString() + "/" + filename; } @@ -178,4 +240,40 @@ public class SystemFlagsDataArchive { } } + + private static class JsonAccessor { + private final JsonNode jsonNode; + + public JsonAccessor(JsonNode jsonNode) { + this.jsonNode = jsonNode; + } + + public JsonAccessor get(String fieldName) { + if (jsonNode == null) { + return this; + } else { + return new JsonAccessor(jsonNode.get(fieldName)); + } + } + + public Optional<String> asString() { + return jsonNode != null && jsonNode.isTextual() ? Optional.of(jsonNode.textValue()) : Optional.empty(); + } + + public void forEachArrayElement(Consumer<JsonAccessor> consumer) { + if (jsonNode != null && jsonNode.isArray()) { + jsonNode.forEach(jsonNodeElement -> consumer.accept(new JsonAccessor(jsonNodeElement))); + } + } + + /** Returns true if this (JsonNode) is a string and equal to value. */ + public boolean isEqualTo(String value) { + return jsonNode != null && jsonNode.isTextual() && Objects.equals(jsonNode.textValue(), value); + } + + @Override + public String toString() { + return jsonNode == null ? "undefined" : jsonNode.toString(); + } + } } diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java index 6e78b8da91c..35f6794e28d 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java @@ -6,6 +6,7 @@ import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.text.JSON; import com.yahoo.vespa.athenz.api.AthenzService; import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagId; @@ -32,6 +33,9 @@ import java.util.Set; import static java.util.stream.Collectors.toList; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * @author bjorncs @@ -104,6 +108,98 @@ public class SystemFlagsDataArchiveTest { archive.validateAllFilesAreForTargets(SystemName.main, Set.of(mainControllerTarget, prodUsWestCfgTarget)); } + @Test + public void throws_on_unknown_field() { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage( + "flags/my-test-flag/main.prod.us-west-1.json contains unknown non-comment fields: after removing any comment fields the JSON is:\n" + + " {\"id\":\"my-test-flag\",\"rules\":[{\"condition\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\",\"values\":[\"foo.com\"]}],\"value\":\"default\"}]}\n" + + "but deserializing this ended up with a JSON that are missing some of the fields:\n" + + " {\"id\":\"my-test-flag\",\"rules\":[{\"value\":\"default\"}]}\n" + + "See https://git.ouroath.com/vespa/hosted-feature-flags for more info on the JSON syntax"); + SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-unknown-field-name/")); + } + + @Test + public void remove_comments() { + assertTrue(JSON.equals("{\n" + + " \"a\": {\n" + + " \"b\": 1\n" + + " },\n" + + " \"list\": [\n" + + " {\n" + + " \"c\": 2\n" + + " },\n" + + " {\n" + + " }\n" + + " ]\n" + + "}", + SystemFlagsDataArchive.normalizeJson("{\n" + + " \"comment\": \"comment a\",\n" + + " \"a\": {\n" + + " \"comment\": \"comment b\",\n" + + " \"b\": 1\n" + + " },\n" + + " \"list\": [\n" + + " {\n" + + " \"comment\": \"comment c\",\n" + + " \"c\": 2\n" + + " },\n" + + " {\n" + + " \"comment\": \"comment d\"\n" + + " }\n" + + " ]\n" + + "}"))); + } + + @Test + public void normalize_json_fail_on_invalid_application() { + try { + SystemFlagsDataArchive.normalizeJson("{\n" + + " \"id\": \"foo\",\n" + + " \"rules\": [\n" + + " {\n" + + " \"conditions\": [\n" + + " {\n" + + " \"type\": \"whitelist\",\n" + + " \"dimension\": \"application\",\n" + + " \"values\": [ \"a.b.c\" ]\n" + + " }\n" + + " ],\n" + + " \"value\": true\n" + + " }\n" + + " ]\n" + + "}\n"); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("Application ids must be on the form tenant:application:instance, but was a.b.c", e.getMessage()); + } + } + + @Test + public void normalize_json_fail_on_invalid_node_type() { + try { + SystemFlagsDataArchive.normalizeJson("{\n" + + " \"id\": \"foo\",\n" + + " \"rules\": [\n" + + " {\n" + + " \"conditions\": [\n" + + " {\n" + + " \"type\": \"whitelist\",\n" + + " \"dimension\": \"node-type\",\n" + + " \"values\": [ \"footype\" ]\n" + + " }\n" + + " ],\n" + + " \"value\": true\n" + + " }\n" + + " ]\n" + + "}\n"); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("No enum constant com.yahoo.config.provision.NodeType.footype", e.getMessage()); + } + } + private static void assertArchiveReturnsCorrectTestFlagDataForTarget(SystemFlagsDataArchive archive) { assertFlagDataHasValue(archive, MY_TEST_FLAG, mainControllerTarget, "main.controller"); assertFlagDataHasValue(archive, MY_TEST_FLAG, prodUsWestCfgTarget, "main.prod.us-west-1"); diff --git a/controller-api/src/test/resources/system-flags-with-unknown-field-name/flags/my-test-flag/main.prod.us-west-1.json b/controller-api/src/test/resources/system-flags-with-unknown-field-name/flags/my-test-flag/main.prod.us-west-1.json new file mode 100644 index 00000000000..c41083fc7ab --- /dev/null +++ b/controller-api/src/test/resources/system-flags-with-unknown-field-name/flags/my-test-flag/main.prod.us-west-1.json @@ -0,0 +1,15 @@ +{ + "id" : "my-test-flag", + "rules" : [ + { + "condition": [ + { + "type": "whitelist", + "dimension": "hostname", + "values": ["foo.com"] + } + ], + "value" : "default" + } + ] +} diff --git a/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.json b/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.json index cef75be02b7..29160dc0081 100644 --- a/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.json +++ b/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.json @@ -1,7 +1,9 @@ { + "comment": "comment a", "id" : "flag-with-empty-data", "rules" : [ { + "comment": "comment b", "value" : "main" } ] diff --git a/default_build_settings.cmake b/default_build_settings.cmake index 622755495c2..0a38da145d0 100644 --- a/default_build_settings.cmake +++ b/default_build_settings.cmake @@ -9,9 +9,9 @@ endfunction() function(setup_vespa_default_build_settings_rhel_7_7) message("-- Setting up default build settings for rhel 7.7") - set(DEFAULT_EXTRA_LINK_DIRECTORY "${VESPA_DEPS}/lib64" "/usr/lib64/llvm5.0/lib" PARENT_SCOPE) - set(DEFAULT_EXTRA_INCLUDE_DIRECTORY "${VESPA_DEPS}/include" "/usr/include/llvm5.0" "/usr/include/openblas" PARENT_SCOPE) - set(DEFAULT_VESPA_LLVM_VERSION "5.0" PARENT_SCOPE) + set(DEFAULT_EXTRA_LINK_DIRECTORY "${VESPA_DEPS}/lib64" "/usr/lib64/llvm7.0/lib" PARENT_SCOPE) + set(DEFAULT_EXTRA_INCLUDE_DIRECTORY "${VESPA_DEPS}/include" "/usr/include/llvm7.0" "/usr/include/openblas" PARENT_SCOPE) + set(DEFAULT_VESPA_LLVM_VERSION "7" PARENT_SCOPE) endfunction() function(setup_vespa_default_build_settings_rhel_8_1) @@ -21,9 +21,9 @@ endfunction() function(setup_vespa_default_build_settings_centos_7) message("-- Setting up default build settings for centos 7") - set(DEFAULT_EXTRA_LINK_DIRECTORY "${VESPA_DEPS}/lib64" "/usr/lib64/llvm5.0/lib" PARENT_SCOPE) - set(DEFAULT_EXTRA_INCLUDE_DIRECTORY "${VESPA_DEPS}/include" "/usr/include/llvm5.0" "/usr/include/openblas" PARENT_SCOPE) - set(DEFAULT_VESPA_LLVM_VERSION "5.0" PARENT_SCOPE) + set(DEFAULT_EXTRA_LINK_DIRECTORY "${VESPA_DEPS}/lib64" "/usr/lib64/llvm7.0/lib" PARENT_SCOPE) + set(DEFAULT_EXTRA_INCLUDE_DIRECTORY "${VESPA_DEPS}/include" "/usr/include/llvm7.0" "/usr/include/openblas" PARENT_SCOPE) + set(DEFAULT_VESPA_LLVM_VERSION "7" PARENT_SCOPE) endfunction() function(setup_vespa_default_build_settings_centos_8) diff --git a/dist/vespa.spec b/dist/vespa.spec index 16222ed3852..b2f3d1d5df4 100644 --- a/dist/vespa.spec +++ b/dist/vespa.spec @@ -30,11 +30,11 @@ BuildRequires: centos-release-scl %endif %endif %if 0%{?el7} -BuildRequires: devtoolset-8-gcc-c++ -BuildRequires: devtoolset-8-libatomic-devel -BuildRequires: devtoolset-8-binutils +BuildRequires: devtoolset-9-gcc-c++ +BuildRequires: devtoolset-9-libatomic-devel +BuildRequires: devtoolset-9-binutils BuildRequires: rh-maven35 -%define _devtoolset_enable /opt/rh/devtoolset-8/enable +%define _devtoolset_enable /opt/rh/devtoolset-9/enable %define _rhmaven35_enable /opt/rh/rh-maven35/enable %endif %if 0%{?el8} @@ -49,7 +49,7 @@ BuildRequires: libatomic BuildRequires: Judy-devel %if 0%{?el7} BuildRequires: cmake3 -BuildRequires: llvm5.0-devel +BuildRequires: llvm7.0-devel BuildRequires: vespa-boost-devel >= 1.59.0-6 BuildRequires: vespa-gtest >= 1.8.1-1 BuildRequires: vespa-protobuf-devel >= 3.7.0-4 @@ -154,14 +154,14 @@ Requires: perf Requires: gdb Requires: net-tools %if 0%{?el7} -Requires: llvm5.0 +Requires: llvm7.0 Requires: vespa-openssl >= 1.1.1c-1 Requires: vespa-icu >= 65.1.0-1 Requires: vespa-protobuf >= 3.7.0-4 Requires: vespa-telegraf >= 1.1.1-1 -%define _vespa_llvm_version 5.0 -%define _extra_link_directory /usr/lib64/llvm5.0/lib;%{_vespa_deps_prefix}/lib64 -%define _extra_include_directory /usr/include/llvm5.0;%{_vespa_deps_prefix}/include;/usr/include/openblas +%define _vespa_llvm_version 7 +%define _extra_link_directory /usr/lib64/llvm7.0/lib;%{_vespa_deps_prefix}/lib64 +%define _extra_include_directory /usr/include/llvm7.0;%{_vespa_deps_prefix}/include;/usr/include/openblas %endif %if 0%{?el8} Requires: llvm-libs >= 8.0.1 diff --git a/docker/enter-build-container.sh b/docker/enter-build-container.sh index 45188cb4df3..ff7a30ecc8f 100755 --- a/docker/enter-build-container.sh +++ b/docker/enter-build-container.sh @@ -10,7 +10,7 @@ fi DIR=$(cd $(dirname "${BASH_SOURCE[0]}") && pwd) cd $DIR -DOCKER_IMAGE="vespaengine/vespa-dev:latest" +DOCKER_IMAGE="vespaengine/vespa-build-centos7:latest" docker pull ${DOCKER_IMAGE} docker run -ti --rm -v $(pwd)/..:/vespa --entrypoint /vespa/docker/build/enter-build-container-internal.sh "$DOCKER_IMAGE" diff --git a/docker/vespa-ci.sh b/docker/vespa-ci.sh index f4c46f8919a..9ac12f155cc 100755 --- a/docker/vespa-ci.sh +++ b/docker/vespa-ci.sh @@ -12,7 +12,7 @@ DIR=$(cd "${RELATIVE_DIR}" && pwd) cd "${DIR}" GIT_COMMIT=$1 -DOCKER_IMAGE="vespaengine/vespa-dev:latest" +DOCKER_IMAGE="vespaengine/vespa-build-centos7:latest" INTERNAL_DIR=/vespa LOG_PREFIX=vespa-ci-$(date +%Y-%m-%dT%H:%M:%S) diff --git a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadManager.java b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadManager.java deleted file mode 100644 index 6fd4beac056..00000000000 --- a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadManager.java +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.docproc.jdisc; - -import com.yahoo.document.DocumentUtil; -import com.yahoo.log.LogLevel; - -import java.util.concurrent.atomic.AtomicLong; -import java.util.logging.Logger; - -/** - * @author Einar M R Rosenvinge - */ -class DocprocThreadManager { - - private static Logger log = Logger.getLogger(DocprocThreadManager.class.getName()); - - private final long maxConcurrentByteSize; - private final AtomicLong bytesStarted = new AtomicLong(0); - private final AtomicLong bytesFinished = new AtomicLong(0); - - DocprocThreadManager(double maxConcurrentFactor, double documentExpansionFactor, int containerCoreMemoryMb) { - this((long) (((double) DocumentUtil.calculateMaxPendingSize(maxConcurrentFactor, documentExpansionFactor, - containerCoreMemoryMb)) * maxConcurrentFactor)); - } - - DocprocThreadManager(long maxConcurrentByteSize) { - final int MINCONCURRENTBYTES=256*1024*1024; //256M - if (maxConcurrentByteSize < MINCONCURRENTBYTES) { - maxConcurrentByteSize = MINCONCURRENTBYTES; - } - - this.maxConcurrentByteSize = maxConcurrentByteSize; - log.log(LogLevel.CONFIG, "Docproc service allowed to concurrently process " - + (((double) maxConcurrentByteSize) / 1024.0d / 1024.0d) + " megabytes of input data."); - } - - boolean isAboveLimit() { - return (bytesFinished.get() - bytesStarted.get() > maxConcurrentByteSize); - } - void beforeExecute(DocumentProcessingTask task) { - bytesStarted.getAndAdd(task.getApproxSize()); - } - - void afterExecute(DocumentProcessingTask task) { - bytesFinished.getAndAdd(task.getApproxSize()); - } - void shutdown() { - } - -} diff --git a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadPoolExecutor.java b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadPoolExecutor.java deleted file mode 100644 index e1a902c8d5c..00000000000 --- a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadPoolExecutor.java +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.docproc.jdisc; - -import com.yahoo.concurrent.DaemonThreadFactory; -import com.yahoo.log.LogLevel; - -import java.util.List; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.TimeUnit; -import java.util.logging.Logger; - -/** - * @author Einar M R Rosenvinge - */ -public class DocprocThreadPoolExecutor extends ThreadPoolExecutor { - - private static Logger log = Logger.getLogger(DocprocThreadPoolExecutor.class.getName()); - private DocprocThreadManager threadManager; - - public DocprocThreadPoolExecutor(int maxNumThreads, BlockingQueue<Runnable> queue, DocprocThreadManager threadMgr) { - super((maxNumThreads > 0) ? maxNumThreads : Runtime.getRuntime().availableProcessors(), - (maxNumThreads > 0) ? maxNumThreads : 2048, - 1, TimeUnit.SECONDS, - queue, - new DaemonThreadFactory("docproc-")); - this.threadManager = threadMgr; - allowCoreThreadTimeOut(false); - log.log(LogLevel.DEBUG, "Created docproc thread pool with " + super.getCorePoolSize() + " worker threads."); - } - - @Override - protected void beforeExecute(Thread thread, Runnable runnable) { - threadManager.beforeExecute((DocumentProcessingTask) runnable); - } - - @Override - protected void afterExecute(Runnable runnable, Throwable throwable) { - threadManager.afterExecute((DocumentProcessingTask) runnable); - } - - @Override - public void shutdown() { - super.shutdown(); - threadManager.shutdown(); - } - - @Override - public List<Runnable> shutdownNow() { - List<Runnable> list = super.shutdownNow(); - threadManager.shutdown(); - return list; - } - - boolean isAboveLimit() { - return threadManager.isAboveLimit(); - } - -} diff --git a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandler.java b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandler.java index 5b7b9d85a91..d34c300b8d0 100644 --- a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandler.java +++ b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandler.java @@ -35,6 +35,7 @@ import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.SynchronousQueue; +import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.logging.Logger; @@ -52,7 +53,7 @@ public class DocumentProcessingHandler extends AbstractRequestHandler { private final ComponentRegistry<DocprocService> docprocServiceRegistry; private final ComponentRegistry<AbstractConcreteDocumentFactory> docFactoryRegistry; private final ChainRegistry<DocumentProcessor> chainRegistry = new ChainRegistry<>(); - private DocprocThreadPoolExecutor threadPool; + private ThreadPoolExecutor threadPool; private final ScheduledThreadPoolExecutor laterExecutor = new ScheduledThreadPoolExecutor(2, new DaemonThreadFactory("docproc-later-")); private ContainerDocumentConfig containerDocConfig; @@ -61,7 +62,7 @@ public class DocumentProcessingHandler extends AbstractRequestHandler { public DocumentProcessingHandler(ComponentRegistry<DocprocService> docprocServiceRegistry, ComponentRegistry<DocumentProcessor> documentProcessorComponentRegistry, ComponentRegistry<AbstractConcreteDocumentFactory> docFactoryRegistry, - DocprocThreadPoolExecutor threadPool, DocumentTypeManager documentTypeManager, + ThreadPoolExecutor threadPool, DocumentTypeManager documentTypeManager, ChainsModel chainsModel, SchemaMap schemaMap, Statistics statistics, Metric metric, ContainerDocumentConfig containerDocConfig) { @@ -88,28 +89,27 @@ public class DocumentProcessingHandler extends AbstractRequestHandler { } } + private static int computeNumThreads(int maxThreads) { + return (maxThreads > 0) ? maxThreads : Runtime.getRuntime().availableProcessors(); + } + public DocumentProcessingHandler(ComponentRegistry<DocprocService> docprocServiceRegistry, ComponentRegistry<DocumentProcessor> documentProcessorComponentRegistry, ComponentRegistry<AbstractConcreteDocumentFactory> docFactoryRegistry, DocumentProcessingHandlerParameters params) { this(docprocServiceRegistry, documentProcessorComponentRegistry, docFactoryRegistry, - new DocprocThreadPoolExecutor(params.getMaxNumThreads(), - chooseQueueType(params.getMaxNumThreads()), - new DocprocThreadManager(params.getMaxConcurrentFactor(), - params.getDocumentExpansionFactor(), - params.getContainerCoreMemoryMb())), + new ThreadPoolExecutor(computeNumThreads(params.getMaxNumThreads()), + computeNumThreads(params.getMaxNumThreads()), + 0,TimeUnit.SECONDS, + new LinkedBlockingQueue<>(), + new DaemonThreadFactory("docproc-") + ), params.getDocumentTypeManager(), params.getChainsModel(), params.getSchemaMap(), params.getStatisticsManager(), params.getMetric(), params.getContainerDocConfig()); } - private static BlockingQueue<Runnable> chooseQueueType(int maxNumThreads) { - return (maxNumThreads > 0) - ? new LinkedBlockingQueue<>() - : new SynchronousQueue<>(); - } - @Inject public DocumentProcessingHandler(ComponentRegistry<DocumentProcessor> documentProcessorComponentRegistry, ComponentRegistry<AbstractConcreteDocumentFactory> docFactoryRegistry, @@ -124,9 +124,6 @@ public class DocumentProcessingHandler extends AbstractRequestHandler { this(new ComponentRegistry<>(), documentProcessorComponentRegistry, docFactoryRegistry, new DocumentProcessingHandlerParameters().setMaxNumThreads (docprocConfig.numthreads()) - .setMaxConcurrentFactor(containerMbusConfig.maxConcurrentFactor()) - .setDocumentExpansionFactor(containerMbusConfig.documentExpansionFactor()) - .setContainerCoreMemoryMb(containerMbusConfig.containerCoreMemory()) .setDocumentTypeManager(new DocumentTypeManager(docManConfig)) .setChainsModel(buildFromConfig(chainsConfig)).setSchemaMap(configureMapping(mappingConfig)) .setStatisticsManager(manager) @@ -198,14 +195,10 @@ public class DocumentProcessingHandler extends AbstractRequestHandler { } private void submit(DocumentProcessingTask task) { - if (threadPool.isAboveLimit()) { + try { + threadPool.execute(task); + } catch (RejectedExecutionException ree) { task.queueFull(); - } else { - try { - threadPool.execute(task); - } catch (RejectedExecutionException ree) { - task.queueFull(); - } } } diff --git a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandlerParameters.java b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandlerParameters.java index bf308e39218..b8a6aa9c105 100644 --- a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandlerParameters.java +++ b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandlerParameters.java @@ -18,9 +18,6 @@ import com.yahoo.statistics.Statistics; public class DocumentProcessingHandlerParameters { private int maxNumThreads = 0; - private double maxConcurrentFactor = 0.2; - private double documentExpansionFactor = 20.0; - private int containerCoreMemoryMb = 50; private DocumentTypeManager documentTypeManager = null; private ChainsModel chainsModel = null; private SchemaMap schemaMap = null; @@ -28,22 +25,7 @@ public class DocumentProcessingHandlerParameters { private Metric metric = new NullMetric(); private ContainerDocumentConfig containerDocConfig; - public DocumentProcessingHandlerParameters() { - } - - /** - * Returns the number of megabytes of memory reserved for container core classes and data. - * - * @return the number of megabytes of memory reserved for container core classes and data. - */ - public int getContainerCoreMemoryMb() { - return containerCoreMemoryMb; - } - public DocumentProcessingHandlerParameters setContainerCoreMemoryMb(int containerCoreMemoryMb) { - this.containerCoreMemoryMb = containerCoreMemoryMb; - return this; - } public Metric getMetric() { return metric; @@ -55,36 +37,6 @@ public class DocumentProcessingHandlerParameters { } /** - * Returns the document expansion factor, i.e. by what factor a serialized and possibly compressed - * input document is expected to expand during deserialization, including any temporary memory needed - * when processing it. - * - * @return the document expansion factor. - */ - public double getDocumentExpansionFactor() { - return documentExpansionFactor; - } - - public DocumentProcessingHandlerParameters setDocumentExpansionFactor(double documentExpansionFactor) { - this.documentExpansionFactor = documentExpansionFactor; - return this; - } - - /** - * Returns the max concurrent factor. - * - * @return the max concurrent factor. - */ - public double getMaxConcurrentFactor() { - return maxConcurrentFactor; - } - - public DocumentProcessingHandlerParameters setMaxConcurrentFactor(double maxConcurrentFactor) { - this.maxConcurrentFactor = maxConcurrentFactor; - return this; - } - - /** * Returns the maximum number of thread that the thread pool will ever attempt to run simultaneously. * * @return the maximum number of thread that the thread pool will ever attempt to run simultaneously. diff --git a/docproc/src/test/java/com/yahoo/docproc/jdisc/DocprocThreadPoolExecutorTestCase.java b/docproc/src/test/java/com/yahoo/docproc/jdisc/DocprocThreadPoolExecutorTestCase.java deleted file mode 100644 index a74fb9a3edf..00000000000 --- a/docproc/src/test/java/com/yahoo/docproc/jdisc/DocprocThreadPoolExecutorTestCase.java +++ /dev/null @@ -1,83 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.docproc.jdisc; - -import org.junit.Test; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Set; -import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.TimeUnit; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - -/** - * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a> - */ -public class DocprocThreadPoolExecutorTestCase { - private final Set<Long> threadIds = Collections.synchronizedSet(new HashSet<Long>()); - - @Test - public void threadPool() throws InterruptedException { - int numThreads = 8; - int numTasks = 200; - - LinkedBlockingQueue<Runnable> q = new LinkedBlockingQueue<>(); - DocprocThreadManager mgr = new DocprocThreadManager(1000l); - DocprocThreadPoolExecutor pool = new DocprocThreadPoolExecutor(numThreads, q, mgr); - - List<MockedDocumentProcessingTask> tasks = new ArrayList<>(numTasks); - for (int i = 0; i < numTasks; i++) { - tasks.add(new MockedDocumentProcessingTask()); - } - - for (int i = 0; i < numTasks; i++) { - pool.execute(tasks.get(i)); - } - pool.shutdown(); - pool.awaitTermination(120L, TimeUnit.SECONDS); - - for (int i = 0; i < numTasks; i++) { - assertTrue(tasks.get(i).hasBeenRun()); - } - - System.err.println(threadIds); - assertEquals(numThreads, threadIds.size()); - } - - private class MockedDocumentProcessingTask extends DocumentProcessingTask { - private boolean hasBeenRun = false; - - public MockedDocumentProcessingTask() { - super(null, null, null); - } - - @Override - public void run() { - threadIds.add(Thread.currentThread().getId()); - System.err.println(System.currentTimeMillis() + " MOCK Thread " + Thread.currentThread().getId() + " running task " + this); - for (int i = 0; i < 100000; i++) { - Math.sin((double) (System.currentTimeMillis() / 10000L)); - } - System.err.println(System.currentTimeMillis() + " MOCK Thread " + Thread.currentThread().getId() + " DONE task " + this); - hasBeenRun = true; - } - - @Override - public int getApproxSize() { - return 333; - } - - @Override - public String toString() { - return "seqNum " + getSeqNum(); - } - - public boolean hasBeenRun() { - return hasBeenRun; - } - } -} diff --git a/document/src/main/java/com/yahoo/document/DocumentOperation.java b/document/src/main/java/com/yahoo/document/DocumentOperation.java index 8209322c472..bb1e5d2b357 100644 --- a/document/src/main/java/com/yahoo/document/DocumentOperation.java +++ b/document/src/main/java/com/yahoo/document/DocumentOperation.java @@ -4,10 +4,7 @@ package com.yahoo.document; /** * Base class for "document operations". * These include "put" (DocumentPut), "update" (DocumentUpdate), "remove" (DocumentRemove) - * and "get" (DocumentGet). The latter only for internal use. - * Historically, put operations were represented by the Document class alone, - * but since it doesn't make much sense to put a *test and set* condition in Document, - * a more uniform interface for document operations was needed. + * and "get" (DocumentGet). * * @author Vegard Sjonfjell */ diff --git a/document/src/main/java/com/yahoo/document/DocumentPut.java b/document/src/main/java/com/yahoo/document/DocumentPut.java index 5906a9ca0ba..c5ce2e7e181 100644 --- a/document/src/main/java/com/yahoo/document/DocumentPut.java +++ b/document/src/main/java/com/yahoo/document/DocumentPut.java @@ -30,7 +30,8 @@ public class DocumentPut extends DocumentOperation { /** * Copy constructor - * @param other DocumentPut to copy + * + * @param other the DocumentPut to copy */ public DocumentPut(DocumentPut other) { super(other); diff --git a/document/src/main/java/com/yahoo/document/idstring/IdIdString.java b/document/src/main/java/com/yahoo/document/idstring/IdIdString.java index 28573296370..9c75cf6828b 100644 --- a/document/src/main/java/com/yahoo/document/idstring/IdIdString.java +++ b/document/src/main/java/com/yahoo/document/idstring/IdIdString.java @@ -11,11 +11,13 @@ import com.yahoo.text.Utf8; * Time: 11:02 AM */ public class IdIdString extends IdString { - private String type; + private final String type; private String group; private long location; private boolean hasGroup; private boolean hasNumber; + private static final int SIZE_OF_ID_AND_3_COLONS = 2 + 3; // "id:::" + private static final int MAX_LENGTH = IdString.MAX_LENGTH_EXCEPT_NAMESPACE_SPECIFIC - SIZE_OF_ID_AND_3_COLONS; public static String replaceType(String id, String typeName) { int typeStartPos = id.indexOf(":", 3) + 1; @@ -47,9 +49,9 @@ public class IdIdString extends IdString { super(Scheme.id, namespace, localId); this.type = type; boolean hasSetLocation = false; - if (namespace.length() + type.length() + keyValues.length() + 5 >= IdString.MAX_LENGTH_EXCEPT_NAMESPACE_SPECIFIC) { + if (namespace.length() + type.length() + keyValues.length() >= MAX_LENGTH) { throw new IllegalArgumentException("Length of namespace(" + namespace.length() + ") + doctype(" + type.length() + - ") + key/values(" + keyValues.length() +"), is longer than " + (MAX_LENGTH_EXCEPT_NAMESPACE_SPECIFIC - 5)); + ") + key/values(" + keyValues.length() +"), is longer than " + MAX_LENGTH); } for(String pair : keyValues.split(",")) { int pos = pair.indexOf('='); diff --git a/document/src/main/java/com/yahoo/document/idstring/IdString.java b/document/src/main/java/com/yahoo/document/idstring/IdString.java index d25c39f3b44..2114a480ec3 100644 --- a/document/src/main/java/com/yahoo/document/idstring/IdString.java +++ b/document/src/main/java/com/yahoo/document/idstring/IdString.java @@ -42,6 +42,7 @@ public abstract class IdString { private final String namespace; private final String namespaceSpecific; private Utf8String cache; + // This max unsigned 16 bit integer - 1 as the offset will be length + 1 static final int MAX_LENGTH_EXCEPT_NAMESPACE_SPECIFIC = 0xfffe; /** diff --git a/document/src/test/java/com/yahoo/document/IdIdStringTest.java b/document/src/test/java/com/yahoo/document/IdIdStringTest.java index b321d91e6e9..7915f2c8b45 100644 --- a/document/src/test/java/com/yahoo/document/IdIdStringTest.java +++ b/document/src/test/java/com/yahoo/document/IdIdStringTest.java @@ -67,7 +67,7 @@ public class IdIdStringTest { } @Test - public void requireThatTooLongPreNamspaceSpecificThrowsWhileParsing() throws Exception { + public void requireThatTooLongPreNamespaceSpecificThrowsWhileParsing() throws Exception { StringBuilder builder = new StringBuilder("id:"); for (int i = 0; i < 0x10000; i++) { builder.append('n'); @@ -81,7 +81,7 @@ public class IdIdStringTest { } } @Test - public void requireThatTooLongPreNamspaceSpecificThrowsOnConstruction() { + public void requireThatTooLongPreNamespaceSpecificThrowsOnConstruction() { StringBuilder builder = new StringBuilder(); for (int i = 0; i < 0x10000; i++) { builder.append('n'); diff --git a/documentapi/src/main/java/com/yahoo/documentapi/DocumentOpVisitorResponse.java b/documentapi/src/main/java/com/yahoo/documentapi/DocumentOpVisitorResponse.java index f5641b915f4..4316003acc6 100644 --- a/documentapi/src/main/java/com/yahoo/documentapi/DocumentOpVisitorResponse.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/DocumentOpVisitorResponse.java @@ -10,6 +10,7 @@ import com.yahoo.document.DocumentOperation; * @author Arne H Juul */ public class DocumentOpVisitorResponse extends VisitorResponse { + private DocumentOperation op; /** @@ -25,4 +26,5 @@ public class DocumentOpVisitorResponse extends VisitorResponse { /** @return the document operation */ public DocumentOperation getDocumentOperation() { return op; } + } diff --git a/documentapi/src/main/java/com/yahoo/documentapi/SyncSession.java b/documentapi/src/main/java/com/yahoo/documentapi/SyncSession.java index 6c4306b683c..cc0f6dc7cd5 100755 --- a/documentapi/src/main/java/com/yahoo/documentapi/SyncSession.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/SyncSession.java @@ -23,15 +23,15 @@ public interface SyncSession extends Session { * Puts a document. When this method returns, the document is safely * received. This enables setting condition compared to using Document. * - * @param documentPut The DocumentPut operation + * @param documentPut the DocumentPut operation */ void put(DocumentPut documentPut); /** * Puts a document. When this method returns, the document is safely received. * - * @param documentPut The DocumentPut operation - * @param priority The priority with which to perform this operation. + * @param documentPut the DocumentPut operation + * @param priority the priority with which to perform this operation */ default void put(DocumentPut documentPut, DocumentProtocol.Priority priority) { put(documentPut); @@ -40,11 +40,9 @@ public interface SyncSession extends Session { /** * Gets a document. * - * @param id The id of the document to get. - * @return The known document having this id, or null if there is no - * document having this id. - * @throws UnsupportedOperationException Thrown if this access does not - * support retrieving. + * @param id the id of the document to get. + * @return the known document having this id, or null if there is no document having this id + * @throws UnsupportedOperationException thrown if this access does not support retrieving */ default Document get(DocumentId id) { return get(id, null); } @@ -64,8 +62,8 @@ public interface SyncSession extends Session { /** * Gets a document with timeout. * - * @param id The id of the document to get - * @param timeout Timeout. If timeout is null, an unspecified default will be used + * @param id the id of the document to get + * @param timeout timeout. If timeout is null, an unspecified default will be used * @return the document with this id, or null if there is none * @throws UnsupportedOperationException thrown if this access does not support retrieving * @throws DocumentAccessException on any messagebus error, including timeout ({@link com.yahoo.messagebus.ErrorCode#TIMEOUT}). @@ -75,53 +73,50 @@ public interface SyncSession extends Session { /** * Gets a document with timeout. * - * @param id The id of the document to get. - * @param fieldSet A comma-separated list of fields to retrieve - * @param priority The priority with which to perform this operation. - * @param timeout Timeout. If timeout is null, an unspecified default will be used. - * @return The known document having this id, or null if there is no - * document having this id. - * @throws UnsupportedOperationException Thrown if this access does not support retrieving. - * @throws DocumentAccessException on any messagebus error, including timeout ({@link com.yahoo.messagebus.ErrorCode#TIMEOUT}). + * @param id the id of the document to get + * @param fieldSet a comma-separated list of fields to retrieve + * @param priority the priority with which to perform this operation + * @param timeout timeout. If timeout is null, an unspecified default will be used + * @return the known document having this id, or null if there is no document having this id + * @throws UnsupportedOperationException thrown if this access does not support retrieving + * @throws DocumentAccessException on any messagebus error, including timeout ({@link com.yahoo.messagebus.ErrorCode#TIMEOUT}) */ Document get(DocumentId id, String fieldSet, DocumentProtocol.Priority priority, Duration timeout); /** - * <p>Removes a document if it is present and condition is fulfilled.</p> + * Removes a document if it is present and condition is fulfilled. + * * @param documentRemove document to delete - * @return true if the document with this id was removed, false otherwise. + * @return true if the document with this id was removed, false otherwise */ boolean remove(DocumentRemove documentRemove); /** * Removes a document if it is present. * - * @param documentRemove Document remove operation - * @param priority The priority with which to perform this operation. - * @return true If the document with this id was removed, false otherwise. - * @throws UnsupportedOperationException Thrown if this access does not - * support removal. + * @param documentRemove document remove operation + * @param priority the priority with which to perform this operation + * @return true if the document with this id was removed, false otherwise. + * @throws UnsupportedOperationException thrown if this access does not support removal */ boolean remove(DocumentRemove documentRemove, DocumentProtocol.Priority priority); /** * Updates a document. * - * @param update The updates to perform. - * @return True, if the document was found and updated. - * @throws UnsupportedOperationException Thrown if this access does not - * support update. + * @param update the updates to perform + * @return true, if the document was found and updated + * @throws UnsupportedOperationException thrown if this access does not support update */ boolean update(DocumentUpdate update); /** * Updates a document. * - * @param update The updates to perform. - * @param priority The priority with which to perform this operation. - * @return True, if the document was found and updated. - * @throws UnsupportedOperationException Thrown if this access does not - * support update. + * @param update the updates to perform. + * @param priority the priority with which to perform this operation + * @return true, if the document was found and updated + * @throws UnsupportedOperationException thrown if this access does not support update */ boolean update(DocumentUpdate update, DocumentProtocol.Priority priority); diff --git a/eval/src/tests/eval/function/function_test.cpp b/eval/src/tests/eval/function/function_test.cpp index 11fd8abf549..598d9c251a6 100644 --- a/eval/src/tests/eval/function/function_test.cpp +++ b/eval/src/tests/eval/function/function_test.cpp @@ -977,9 +977,9 @@ TEST("require that tensor peek can contain expressions") { } TEST("require that trivial tensor peek number expressions are converted to verbatim labels") { - TEST_DO(verify_parse("t{x:(5.7)}", "f(t)(t{x:\"6\"})")); + TEST_DO(verify_parse("t{x:(5.7)}", "f(t)(t{x:\"5\"})")); TEST_DO(verify_parse("t{x:(5.3)}", "f(t)(t{x:\"5\"})")); - TEST_DO(verify_parse("t{x:(-5.7)}", "f(t)(t{x:\"-6\"})")); + TEST_DO(verify_parse("t{x:(-5.7)}", "f(t)(t{x:\"-5\"})")); TEST_DO(verify_parse("t{x:(-5.3)}", "f(t)(t{x:\"-5\"})")); } diff --git a/eval/src/tests/tensor/dense_tensor_peek_function/dense_tensor_peek_function_test.cpp b/eval/src/tests/tensor/dense_tensor_peek_function/dense_tensor_peek_function_test.cpp index a3afb710d1a..5bbdfeaf543 100644 --- a/eval/src/tests/tensor/dense_tensor_peek_function/dense_tensor_peek_function_test.cpp +++ b/eval/src/tests/tensor/dense_tensor_peek_function/dense_tensor_peek_function_test.cpp @@ -73,13 +73,13 @@ TEST("require that tensor peek is not optimized for mixed tensor") { TEST_DO(verify("xmy2{x:(a),y:(b)}", 0.0, 0, 1)); } -TEST("require that indexes are rounded to nearest integer") { - TEST_DO(verify("x3{x:(a-0.3)}", 2.0, 1, 0)); +TEST("require that indexes are truncated when converted to integers") { + TEST_DO(verify("x3{x:(a+0.7)}", 2.0, 1, 0)); TEST_DO(verify("x3{x:(a+0.3)}", 2.0, 1, 0)); - TEST_DO(verify("xm{x:(a-0.3)}", 1.0, 0, 1)); + TEST_DO(verify("xm{x:(a+0.7)}", 1.0, 0, 1)); TEST_DO(verify("xm{x:(a+0.3)}", 1.0, 0, 1)); + TEST_DO(verify("xm{x:(-a-0.7)}", 4.0, 0, 1)); TEST_DO(verify("xm{x:(-a-0.3)}", 4.0, 0, 1)); - TEST_DO(verify("xm{x:(-a+0.3)}", 4.0, 0, 1)); } TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/eval/src/vespa/eval/eval/function.cpp b/eval/src/vespa/eval/eval/function.cpp index caa5e94a681..5d4d597bbf8 100644 --- a/eval/src/vespa/eval/eval/function.cpp +++ b/eval/src/vespa/eval/eval/function.cpp @@ -804,7 +804,7 @@ void parse_tensor_peek(ParseContext &ctx) { if (ctx.get() == '(') { auto expr = get_expression(ctx); if (auto num = nodes::as<nodes::Number>(*expr)) { - peek_spec.emplace(dim_name, make_string("%" PRId64, int64_t(round(num->value())))); + peek_spec.emplace(dim_name, make_string("%" PRId64, int64_t(num->value()))); } else { peek_spec.emplace(dim_name, std::move(expr)); } diff --git a/eval/src/vespa/eval/eval/tensor_function.cpp b/eval/src/vespa/eval/eval/tensor_function.cpp index 9b7968f5092..2656e240a5b 100644 --- a/eval/src/vespa/eval/eval/tensor_function.cpp +++ b/eval/src/vespa/eval/eval/tensor_function.cpp @@ -182,7 +182,7 @@ void op_tensor_peek(State &state, uint64_t param) { addr.emplace(pos->first, label); }, [&](const TensorFunction::Child &) { - double index = round(state.peek(child_cnt++).as_double()); + double index = state.peek(child_cnt++).as_double(); size_t dim_idx = self.param_type().dimension_index(pos->first); assert(dim_idx != ValueType::Dimension::npos); const auto ¶m_dim = self.param_type().dimensions()[dim_idx]; diff --git a/eval/src/vespa/eval/tensor/dense/dense_tensor_peek_function.cpp b/eval/src/vespa/eval/tensor/dense/dense_tensor_peek_function.cpp index 6dc081cba1c..5cb1cbfd88f 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_tensor_peek_function.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_tensor_peek_function.cpp @@ -29,7 +29,7 @@ void my_tensor_peek_op(eval::InterpretedFunction::State &state, uint64_t param) if (dim.first >= 0) { idx += (dim.first * factor); } else { - size_t dim_idx(round(state.peek(0).as_double())); + size_t dim_idx = state.peek(0).as_double(); state.stack.pop_back(); valid &= (dim_idx < dim.second); idx += (dim_idx * factor); diff --git a/fastos/src/vespa/fastos/file.cpp b/fastos/src/vespa/fastos/file.cpp index 56f6addd743..861dd9f4259 100644 --- a/fastos/src/vespa/fastos/file.cpp +++ b/fastos/src/vespa/fastos/file.cpp @@ -184,12 +184,23 @@ FastOS_FileInterface::DirectIOPadding(int64_t offset, void * -FastOS_FileInterface::AllocateDirectIOBuffer(size_t byteSize, void *&realPtr) +FastOS_FileInterface::allocateGenericDirectIOBuffer(size_t byteSize, void *&realPtr) { realPtr = malloc(byteSize); // Default - use malloc allignment return realPtr; } +size_t +FastOS_FileInterface::getMaxDirectIOMemAlign() +{ + return 1u; +} + +void * +FastOS_FileInterface::AllocateDirectIOBuffer(size_t byteSize, void *&realPtr) +{ + return allocateGenericDirectIOBuffer(byteSize, realPtr); +} void FastOS_FileInterface::enableMemoryMap(int mmapFlags) diff --git a/fastos/src/vespa/fastos/file.h b/fastos/src/vespa/fastos/file.h index 15c4dfa33cd..3d2194229e7 100644 --- a/fastos/src/vespa/fastos/file.h +++ b/fastos/src/vespa/fastos/file.h @@ -559,6 +559,24 @@ public: * This value is always set. * @return Alligned pointer value or nullptr if out of memory */ + static void *allocateGenericDirectIOBuffer(size_t byteSize, void *&realPtr); + + /** + * Get maximum memory alignment for directio buffers. + * @return maximum memory alignment for directio buffers. + */ + static size_t getMaxDirectIOMemAlign(); + + /** + * Allocate a buffer properly alligned with regards to direct io + * access restrictions. + * @param byteSize Number of bytes to be allocated + * @param realPtr Reference where the actual pointer returned + * from malloc will be saved. Use free() with + * this pointer to deallocate the buffer. + * This value is always set. + * @return Alligned pointer value or nullptr if out of memory + */ virtual void *AllocateDirectIOBuffer(size_t byteSize, void *&realPtr); /** diff --git a/fastos/src/vespa/fastos/linux_file.cpp b/fastos/src/vespa/fastos/linux_file.cpp index 56d9e246a97..0f91a8ff41c 100644 --- a/fastos/src/vespa/fastos/linux_file.cpp +++ b/fastos/src/vespa/fastos/linux_file.cpp @@ -13,6 +13,9 @@ #include <unistd.h> #include <fcntl.h> #include "file_rw_ops.h" +#include <cstdio> +#include <cstring> +#include <system_error> using fastos::File_RW_Ops; @@ -431,6 +434,27 @@ FastOS_Linux_File::InitializeClass() return FastOS_UNIX_File::InitializeClass(); } +int +FastOS_Linux_File::count_open_files() +{ + static const char * const fd_dir_name = "/proc/self/fd"; + int count = 0; + DIR *dp = opendir(fd_dir_name); + if (dp != nullptr) { + struct dirent *ptr; + while ((ptr = readdir(dp)) != nullptr) { + if ((strcmp(".", ptr->d_name) != 0) && (strcmp("..", ptr->d_name) != 0)) { + ++count; + } + } + closedir(dp); + } else { + std::error_code ec(errno, std::system_category()); + fprintf(stderr, "could not scan directory %s: %s\n", fd_dir_name, ec.message().c_str()); + } + return count; +} + #include <vespa/fastos/backtrace.h> void forceStaticLinkOf_backtrace() diff --git a/fastos/src/vespa/fastos/linux_file.h b/fastos/src/vespa/fastos/linux_file.h index e304b6f0ce8..06f54de6870 100644 --- a/fastos/src/vespa/fastos/linux_file.h +++ b/fastos/src/vespa/fastos/linux_file.h @@ -45,6 +45,7 @@ public: static bool InitializeClass(); static size_t getMaxDirectIOMemAlign(); static void *allocateGenericDirectIOBuffer(size_t byteSize, void *&realPtr); + static int count_open_files(); private: ssize_t internalWrite2(const void *buffer, size_t len); ssize_t readUnalignedEnd(void *buffer, size_t length, int64_t readOffset); diff --git a/fastos/src/vespa/fastos/unix_file.cpp b/fastos/src/vespa/fastos/unix_file.cpp index 06a48a26482..ef52d34e49f 100644 --- a/fastos/src/vespa/fastos/unix_file.cpp +++ b/fastos/src/vespa/fastos/unix_file.cpp @@ -21,6 +21,10 @@ #else #include <sys/mount.h> #endif +#ifdef __APPLE__ +#include <libproc.h> +#include <sys/proc_info.h> +#endif #include "file_rw_ops.h" using fastos::File_RW_Ops; @@ -498,6 +502,17 @@ int64_t FastOS_UNIX_File::GetFreeDiskSpace (const char *path) return freeSpace; } +int +FastOS_UNIX_File::count_open_files() +{ +#ifdef __APPLE__ + int buffer_size = proc_pidinfo(getpid(), PROC_PIDLISTFDS, 0, nullptr, 0); + return buffer_size / sizeof(proc_fdinfo); +#else + return 0; +#endif +} + FastOS_UNIX_DirectoryScan::FastOS_UNIX_DirectoryScan(const char *searchPath) : FastOS_DirectoryScanInterface(searchPath), _statRun(false), diff --git a/fastos/src/vespa/fastos/unix_file.h b/fastos/src/vespa/fastos/unix_file.h index 3dffe1fc089..3c3ffedc171 100644 --- a/fastos/src/vespa/fastos/unix_file.h +++ b/fastos/src/vespa/fastos/unix_file.h @@ -96,6 +96,7 @@ public: static Error TranslateError(const int osError); static std::string getErrorString(const int osError); static int64_t GetFreeDiskSpace (const char *path); + static int count_open_files(); }; #include <dirent.h> diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/CompressedFileReference.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/CompressedFileReference.java index a1ad5c8a200..ddab35ba14d 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/CompressedFileReference.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/CompressedFileReference.java @@ -75,7 +75,6 @@ public class CompressedFileReference { int entries = 0; ArchiveEntry entry; while ((entry = archiveInputStream.getNextEntry()) != null) { - log.log(LogLevel.DEBUG, "Unpacking " + entry.getName()); File outFile = new File(outputFile, entry.getName()); if (entry.isDirectory()) { if (!(outFile.exists() && outFile.isDirectory())) { 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 c4487252e27..ff9efc74929 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReceiver.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReceiver.java @@ -96,9 +96,9 @@ public class FileReceiver { try { Files.write(inprogressFile.toPath(), part, StandardOpenOption.WRITE, StandardOpenOption.APPEND); } catch (IOException e) { - log.log(LogLevel.ERROR, "Failed writing to file(" + inprogressFile.toPath() + "): " + e.getMessage(), e); + log.log(LogLevel.ERROR, "Failed writing to file (" + inprogressFile.toPath() + "): " + e.getMessage(), e); inprogressFile.delete(); - throw new RuntimeException("Failed writing to file(" + inprogressFile.toPath() + "): ", e); + throw new RuntimeException("Failed writing to file (" + inprogressFile.toPath() + "): ", e); } currentFileSize += part.length; currentPartId++; @@ -114,7 +114,6 @@ public class FileReceiver { // Unpack if necessary if (fileType == FileReferenceData.Type.compressed) { File decompressedDir = Files.createTempDirectory(tmpDir.toPath(), "archive").toFile(); - log.log(LogLevel.DEBUG, () -> "Archived file, unpacking " + inprogressFile + " to " + decompressedDir); CompressedFileReference.decompress(inprogressFile, decompressedDir); moveFileToDestination(decompressedDir, fileReferenceDir); } else { @@ -256,7 +255,7 @@ public class FileReceiver { 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())); + log.log(LogLevel.SPAM, () -> String.format("%.1f percent of '%s' downloaded", completeness * 100, reference.value())); downloader.setDownloadStatus(reference, completeness); req.returnValues().add(new Int32Value(retval)); } 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 c4fe257c991..f1d31156d79 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownloader.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownloader.java @@ -12,11 +12,11 @@ import com.yahoo.vespa.config.ConnectionPool; import java.io.File; import java.time.Duration; +import java.time.Instant; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; import java.util.Optional; -import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; @@ -54,12 +54,12 @@ public class FileReferenceDownloader { new FileReceiver(connectionPool.getSupervisor(), this, downloadDirectory, tmpDirectory); } - private void startDownload(Duration timeout, FileReferenceDownload fileReferenceDownload) { + private void startDownload(FileReferenceDownload fileReferenceDownload) { FileReference fileReference = fileReferenceDownload.fileReference(); - long end = System.currentTimeMillis() + timeout.toMillis(); + Instant end = Instant.now().plus(downloadTimeout); boolean downloadStarted = false; int retryCount = 0; - while ((System.currentTimeMillis() < end) && !downloadStarted) { + do { try { if (startDownloadRpc(fileReferenceDownload, retryCount)) { downloadStarted = true; @@ -69,7 +69,7 @@ public class FileReferenceDownloader { } } catch (InterruptedException e) { /* ignored */} - } + } while (Instant.now().isBefore(end) && !downloadStarted); if ( !downloadStarted) { fileReferenceDownload.future().completeExceptionally(new RuntimeException("Failed getting file reference '" + fileReference.value() + "'")); @@ -86,7 +86,7 @@ public class FileReferenceDownloader { downloads.put(fileReference, fileReferenceDownload); downloadStatus.put(fileReference, 0.0); } - downloadExecutor.submit(() -> startDownload(downloadTimeout, fileReferenceDownload)); + downloadExecutor.submit(() -> startDownload(fileReferenceDownload)); } void completedDownloading(FileReference fileReference, File file) { @@ -116,10 +116,10 @@ public class FileReferenceDownloader { request.parameters().add(new StringValue(fileReference)); request.parameters().add(new Int32Value(fileReferenceDownload.downloadFromOtherSourceIfNotFound() ? 0 : 1)); - execute(request, connection); + connection.invokeSync(request, (double) rpcTimeout.getSeconds()); Level logLevel = (retryCount > 0 ? LogLevel.INFO : LogLevel.DEBUG); if (validateResponse(request)) { - log.log(logLevel, () -> "Request callback, OK. Req: " + request + "\nSpec: " + connection); + log.log(logLevel, () -> "Request callback, OK. Req: " + request + "\nSpec: " + connection + ", retry count " + retryCount); if (request.returnValues().get(0).asInt32() == 0) { log.log(logLevel, () -> "Found file reference '" + fileReference + "' available at " + connection.getAddress()); return true; @@ -129,8 +129,9 @@ public class FileReferenceDownloader { return false; } } else { - log.log(logLevel, "Request failed. Req: " + request + "\nSpec: " + connection.getAddress() + - ", error code: " + request.errorCode() + ", set error for connection and use another for next request"); + log.log(logLevel, () -> "Request failed. Req: " + request + "\nSpec: " + connection.getAddress() + + ", error code: " + request.errorCode() + ", set error for spec, use another spec for next request" + + ", retry count " + retryCount); connectionPool.setError(connection, request.errorCode()); return false; } @@ -148,10 +149,6 @@ public class FileReferenceDownloader { } } - private void execute(Request request, Connection connection) { - connection.invokeSync(request, (double) rpcTimeout.getSeconds()); - } - private boolean validateResponse(Request request) { if (request.isError()) { return false; diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 5166568348d..a4e34098025 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -119,7 +119,7 @@ public class Flags { "use-adaptive-dispatch", false, "Should adaptive dispatch be used over round robin", "Takes effect at redeployment", - APPLICATION_ID); + ZONE_ID, APPLICATION_ID); public static final UnboundIntFlag REBOOT_INTERVAL_IN_DAYS = defineIntFlag( "reboot-interval-in-days", 30, @@ -148,9 +148,21 @@ public class Flags { public static final UnboundDoubleFlag DEFAULT_TERM_WISE_LIMIT = defineDoubleFlag( "default-term-wise-limit", 1.0, - "Node resource memory in Gb for admin cluster nodes", + "Default limit for when to apply termwise query evaluation", "Takes effect at redeployment", - APPLICATION_ID); + ZONE_ID, APPLICATION_ID); + + public static final UnboundDoubleFlag DEFAULT_SOFT_START_SECONDS = defineDoubleFlag( + "default-soft-start-seconds", 0.0, + "Default number of seconds that a soft start shall use", + "Takes effect at redeployment", + ZONE_ID, APPLICATION_ID); + + public static final UnboundDoubleFlag DEFAULT_TOP_K_PROBABILITY = defineDoubleFlag( + "default-top-k-probability", 1.0, + "Default probability that you will get the globally top K documents when merging many partitions.", + "Takes effect at redeployment", + ZONE_ID, APPLICATION_ID); public static final UnboundBooleanFlag HOST_HARDENING = defineFeatureFlag( "host-hardening", false, diff --git a/logd/src/logd/watcher.cpp b/logd/src/logd/watcher.cpp index e481d64f721..a91de132061 100644 --- a/logd/src/logd/watcher.cpp +++ b/logd/src/logd/watcher.cpp @@ -116,6 +116,8 @@ void Watcher::watchfile() { struct donecache already; + char newfn[FILENAME_MAX]; + int spamfill_counter = 0; char *target = getenv("VESPA_LOG_TARGET"); if (target == nullptr || strncmp(target, "file:", 5) != 0) { @@ -214,17 +216,30 @@ Watcher::watchfile() if (rotate) { vespalib::duration rotTime = rotTimer.elapsed(); - if (rotTime > 59s || (sb.st_size == offset && rotTime > 4s)) { - removeOldLogs(filename); + off_t overflow_size = (1.1 * _confsubscriber.getRotateSize()); + if ((rotTime > 59s) || + (sb.st_size == offset && rotTime > 4s) || + (sb.st_size > overflow_size && rotTime > 2s)) + { if (sb.st_size != offset) { LOG(warning, "logfile rotation incomplete after %2.3f s (dropping %" PRIu64 " bytes)", vespalib::to_s(rotTime), static_cast<uint64_t>(sb.st_size - offset)); } else { LOG(debug, "logfile rotation complete after %2.3f s", vespalib::to_s(rotTime)); } + if (((now - created) < (rotTime + 180s)) && (sb.st_size > overflow_size)) { + ++spamfill_counter; + } else { + spamfill_counter = 0; + } created = now; rotate = false; close(_wfd); + if (spamfill_counter > 2) { + LOG(warning, "logfile spamming %d times, aggressively removing %s", spamfill_counter, newfn); + unlink(newfn); + } + removeOldLogs(filename); goto again; } } else if (stat(filename, &sb) != 0 @@ -240,7 +255,6 @@ Watcher::watchfile() rotTimer = vespalib::Timer(); LOG(debug, "preparing to rotate logfile, old logfile size %d, age %2.3f seconds", (int)offset, vespalib::to_s(now-created)); - char newfn[FILENAME_MAX]; int l = strlen(filename); strcpy(newfn, filename); time_t seconds = vespalib::count_s(now.time_since_epoch()); diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/telegraf/Telegraf.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/telegraf/Telegraf.java index 549371ffb74..9600dfca042 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/telegraf/Telegraf.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/telegraf/Telegraf.java @@ -40,13 +40,13 @@ public class Telegraf extends AbstractComponent { public Telegraf(TelegrafRegistry telegrafRegistry, TelegrafConfig telegrafConfig) { this.telegrafRegistry = telegrafRegistry; telegrafRegistry.addInstance(this); - writeConfig(telegrafConfig, uncheck(() -> new FileWriter(TELEGRAF_CONFIG_PATH))); + writeConfig(telegrafConfig, uncheck(() -> new FileWriter(TELEGRAF_CONFIG_PATH)), TELEGRAF_LOG_FILE_PATH); restartTelegraf(); } - protected static void writeConfig(TelegrafConfig telegrafConfig, Writer writer) { + protected static void writeConfig(TelegrafConfig telegrafConfig, Writer writer, String logFilePath) { VelocityContext context = new VelocityContext(); - context.put("logFilePath", TELEGRAF_LOG_FILE_PATH); + context.put("logFilePath", logFilePath); context.put("intervalSeconds", telegrafConfig.intervalSeconds()); context.put("cloudwatchPlugins", telegrafConfig.cloudWatch()); context.put("protocol", telegrafConfig.isHostedVespa() ? "https" : "http"); diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/telegraf/TelegrafTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/telegraf/TelegrafTest.java index d35cc12918c..b4f2b4ab10c 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/telegraf/TelegrafTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/telegraf/TelegrafTest.java @@ -35,7 +35,8 @@ public class TelegrafTest { .isHostedVespa(true) .build(); StringWriter stringWriter = new StringWriter(); - Telegraf.writeConfig(telegrafConfig, stringWriter); + String logFilePath = "/path/to/logs/telegraf/telegraf.log"; + Telegraf.writeConfig(telegrafConfig, stringWriter, logFilePath); String expectedConfig = TestUtil.getFileContents( "telegraf-config-with-two-cloudwatch-plugins.txt"); assertEquals(expectedConfig, stringWriter.toString()); } diff --git a/metrics-proxy/src/test/resources/telegraf-config-with-two-cloudwatch-plugins.txt b/metrics-proxy/src/test/resources/telegraf-config-with-two-cloudwatch-plugins.txt index 3569703ad03..3194b290b78 100644 --- a/metrics-proxy/src/test/resources/telegraf-config-with-two-cloudwatch-plugins.txt +++ b/metrics-proxy/src/test/resources/telegraf-config-with-two-cloudwatch-plugins.txt @@ -9,7 +9,7 @@ flush_jitter = "0s" precision = "" logtarget = "file" - logfile = "/opt/vespa/logs/telegraf/telegraf.log" + logfile = "/path/to/logs/telegraf/telegraf.log" logfile_rotation_interval = "1d" logfile_rotation_max_size = "20MB" logfile_rotation_max_archives = 5 diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java index 9b0a35d4b96..6aa5a8d9fc1 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java @@ -2,6 +2,11 @@ package com.yahoo.vespa.hosted.node.admin.maintenance.coredump; import com.fasterxml.jackson.databind.ObjectMapper; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.vespa.hosted.dockerapi.metrics.Dimensions; +import com.yahoo.vespa.hosted.dockerapi.metrics.Metrics; +import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeMembership; +import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder; import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; @@ -51,6 +56,7 @@ public class CoredumpHandler { private final Path doneCoredumpsPath; private final String operatorGroupName; private final Supplier<String> coredumpIdSupplier; + private final Metrics metrics; /** * @param crashPathInContainer path inside the container where core dump are dumped @@ -58,19 +64,20 @@ public class CoredumpHandler { * @param operatorGroupName name of the group that will be set as the owner of the processed coredump */ public CoredumpHandler(Terminal terminal, CoreCollector coreCollector, CoredumpReporter coredumpReporter, - Path crashPathInContainer, Path doneCoredumpsPath, String operatorGroupName) { + Path crashPathInContainer, Path doneCoredumpsPath, String operatorGroupName, Metrics metrics) { this(terminal, coreCollector, coredumpReporter, crashPathInContainer, doneCoredumpsPath, - operatorGroupName, () -> UUID.randomUUID().toString()); + operatorGroupName, metrics, () -> UUID.randomUUID().toString()); } CoredumpHandler(Terminal terminal, CoreCollector coreCollector, CoredumpReporter coredumpReporter, - Path crashPathInContainer, Path doneCoredumpsPath, String operatorGroupName, Supplier<String> coredumpIdSupplier) { + Path crashPathInContainer, Path doneCoredumpsPath, String operatorGroupName, Metrics metrics, Supplier<String> coredumpIdSupplier) { this.terminal = terminal; this.coreCollector = coreCollector; this.coredumpReporter = coredumpReporter; this.crashPatchInContainer = crashPathInContainer; this.doneCoredumpsPath = doneCoredumpsPath; this.operatorGroupName = operatorGroupName; + this.metrics = metrics; this.coredumpIdSupplier = coredumpIdSupplier; } @@ -85,6 +92,8 @@ public class CoredumpHandler { .maxDepth(1) .deleteRecursively(context); + updateMetrics(context, containerCrashPathOnHost); + // Check if we have already started to process a core dump or we can enqueue a new core one getCoredumpToProcess(containerCrashPathOnHost, containerProcessingPathOnHost) .ifPresent(path -> processAndReportSingleCoredump(context, path, nodeAttributesSupplier)); @@ -180,8 +189,9 @@ public class CoredumpHandler { new UnixPath(compressedCoreFile).setGroup(operatorGroupName).setPermissions("rw-r-----"); Files.delete(coreFile); - Path newCoredumpDirectory = doneCoredumpsPath.resolve(coredumpDirectory.getFileName()); - Files.move(coredumpDirectory, newCoredumpDirectory); + Path newCoredumpDirectory = doneCoredumpsPath.resolve(context.containerName().asString()); + uncheck(() -> Files.createDirectories(newCoredumpDirectory)); + Files.move(coredumpDirectory, newCoredumpDirectory.resolve(coredumpDirectory.getFileName())); } Path findCoredumpFileInProcessingDirectory(Path coredumpProccessingDirectory) { @@ -194,4 +204,57 @@ public class CoredumpHandler { .orElseThrow(() -> new IllegalStateException( "No coredump file found in processing directory " + coredumpProccessingDirectory)); } + + void updateMetrics(NodeAgentContext context, Path containerCrashPathOnHost) { + Dimensions dimensions = generateDimensions(context); + + // Unprocessed coredumps + int numberOfUnprocessedCoredumps = FileFinder.files(containerCrashPathOnHost) + .match(nameStartsWith(".").negate()) + .match(nameMatches(HS_ERR_PATTERN).negate()) + .maxDepth(1) + .list().size(); + + metrics.declareGauge(Metrics.APPLICATION_NODE, "coredumps.enqueued", dimensions, Metrics.DimensionType.PRETAGGED).sample(numberOfUnprocessedCoredumps); + + // Processed coredumps + Path processedCoredumpsPath = doneCoredumpsPath.resolve(context.containerName().asString()); + int numberOfProcessedCoredumps = FileFinder.directories(processedCoredumpsPath) + .maxDepth(1) + .list().size(); + + metrics.declareGauge(Metrics.APPLICATION_NODE, "coredumps.processed", dimensions, Metrics.DimensionType.PRETAGGED).sample(numberOfProcessedCoredumps); + } + + private Dimensions generateDimensions(NodeAgentContext context) { + NodeSpec node = context.node(); + Dimensions.Builder dimensionsBuilder = new Dimensions.Builder() + .add("host", node.hostname()) + .add("flavor", node.flavor()) + .add("state", node.state().toString()) + .add("zone", context.zone().getId().value()); + + node.owner().ifPresent(owner -> + dimensionsBuilder + .add("tenantName", owner.tenant().value()) + .add("applicationName", owner.application().value()) + .add("instanceName", owner.instance().value()) + .add("app", String.join(".", owner.application().value(), owner.instance().value())) + .add("applicationId", owner.toFullString()) + ); + + node.membership().ifPresent(membership -> + dimensionsBuilder + .add("clustertype", membership.clusterType()) + .add("clusterid", membership.clusterId()) + ); + + node.parentHostname().ifPresent(parent -> dimensionsBuilder.add("parentHostname", parent)); + node.allowedToBeDown().ifPresent(allowed -> + dimensionsBuilder.add("orchestratorState", allowed ? "ALLOWED_TO_BE_DOWN" : "NO_REMARKS")); + node.currentVespaVersion().ifPresent(vespaVersion -> dimensionsBuilder.add("vespaVersion", vespaVersion.toFullString())); + + return dimensionsBuilder.build(); + } + } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumCommand.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumCommand.java index 12029c4fe7e..7b543304e6e 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumCommand.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumCommand.java @@ -5,6 +5,7 @@ import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandLine; import com.yahoo.vespa.hosted.node.admin.task.util.process.Terminal; +import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.regex.Matcher; @@ -41,6 +42,7 @@ public abstract class YumCommand<T extends YumCommand<T>> { private final String yumCommand; private final Pattern commandOutputNoopPattern; private final List<YumPackageName> packages; + private final List<String> options = new ArrayList<>(); GenericYumCommand(Terminal terminal, String yumCommand, List<YumPackageName> packages, Pattern commandOutputNoopPattern) { this.terminal = terminal; @@ -48,12 +50,31 @@ public abstract class YumCommand<T extends YumCommand<T>> { this.packages = packages; this.commandOutputNoopPattern = commandOutputNoopPattern; + switch (yumCommand) { + case "install": { + if (packages.size() > 1) options.add("skip_missing_names_on_install=False"); + break; + } + case "upgrade": { + if (packages.size() > 1) options.add("skip_missing_names_on_update=False"); + break; + } + case "remove": break; + default: throw new IllegalArgumentException("Unknown yum command: " + yumCommand); + } + if (packages.isEmpty() && ! "upgrade".equals(yumCommand)) { throw new IllegalArgumentException("No packages specified"); } } @Override + protected void addParametersToCommandLine(CommandLine commandLine) { + super.addParametersToCommandLine(commandLine); + options.forEach(option -> commandLine.add("--setopt", option)); + } + + @Override public boolean converge(TaskContext context) { if (packages.isEmpty() && ! "upgrade".equals(yumCommand)) { throw new IllegalArgumentException("No packages specified"); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTester.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTester.java index 4d04e1199c5..c5b46473218 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTester.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTester.java @@ -73,6 +73,10 @@ public class YumTester extends Yum { StringBuilder cmd = new StringBuilder(); cmd.append("yum ").append(command).append(" --assumeyes"); enableRepos.forEach(repo -> cmd.append(" --enablerepo=").append(repo)); + if ("install".equals(command) && packages.size() > 1) + cmd.append(" --setopt skip_missing_names_on_install=False"); + if ("upgrade".equals(command) && packages.size() > 1) + cmd.append(" --setopt skip_missing_names_on_update=False"); packages.forEach(pkg -> cmd.append(" ").append(pkg.toName())); cmd.append(" 2>&1"); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java index 3d9e3c08276..60a4462e9e9 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java @@ -1,6 +1,8 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.maintenance.coredump; +import com.yahoo.vespa.hosted.dockerapi.metrics.DimensionMetrics; +import com.yahoo.vespa.hosted.dockerapi.metrics.Metrics; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl; import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; @@ -46,16 +48,17 @@ public class CoredumpHandlerTest { private final Path donePath = fileSystem.getPath("/home/docker/dumps"); private final NodeAgentContext context = new NodeAgentContextImpl.Builder("container-123.domain.tld") .fileSystem(fileSystem).build(); - private final Path crashPathInContainer = Paths.get("/var/crash"); + private final Path crashPathInContainer = fileSystem.getPath("/var/crash"); private final Path doneCoredumpsPath = fileSystem.getPath("/home/docker/dumps"); private final TestTerminal terminal = new TestTerminal(); private final CoreCollector coreCollector = mock(CoreCollector.class); private final CoredumpReporter coredumpReporter = mock(CoredumpReporter.class); + private final Metrics metrics = new Metrics(); @SuppressWarnings("unchecked") private final Supplier<String> coredumpIdSupplier = mock(Supplier.class); private final CoredumpHandler coredumpHandler = new CoredumpHandler(terminal, coreCollector, coredumpReporter, - crashPathInContainer, doneCoredumpsPath, "users", coredumpIdSupplier); + crashPathInContainer, doneCoredumpsPath, "users", metrics, coredumpIdSupplier); @Test @@ -206,13 +209,30 @@ public class CoredumpHandlerTest { verify(coreCollector, never()).collect(any(), any()); verify(coredumpReporter, times(1)).reportCoredump(eq("id-123"), eq("metadata")); assertFalse(Files.exists(coredumpDirectory)); - assertFolderContents(doneCoredumpsPath, "id-123"); - assertFolderContents(doneCoredumpsPath.resolve("id-123"), "metadata.json", "dump_bash.core.431.lz4"); + assertFolderContents(doneCoredumpsPath.resolve("container-123"), "id-123"); + assertFolderContents(doneCoredumpsPath.resolve("container-123").resolve("id-123"), "metadata.json", "dump_bash.core.431.lz4"); + } + + @Test + public void report_enqueued_and_processed_metrics() throws IOException { + Files.createFile(crashPathInContainer.resolve("dump-1")); + Files.createFile(crashPathInContainer.resolve("dump-2")); + Files.createFile(crashPathInContainer.resolve("hs_err_pid2.log")); + new UnixPath(doneCoredumpsPath.resolve("container-123").resolve("dump-3-folder").resolve("dump-3")) + .createParents() + .createNewFile(); + + coredumpHandler.updateMetrics(context, crashPathInContainer); + List<DimensionMetrics> updatedMetrics = metrics.getMetricsByType(Metrics.DimensionType.PRETAGGED); + assertEquals(1, updatedMetrics.size()); + Map<String, Number> values = updatedMetrics.get(0).getMetrics(); + assertEquals(2, values.get("coredumps.enqueued").intValue()); + assertEquals(1, values.get("coredumps.processed").intValue()); } @Before public void setup() throws IOException { - Files.createDirectories(donePath); + Files.createDirectories(crashPathInContainer); } @After diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTest.java index a0887e74e47..dc9c036b96b 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTest.java @@ -75,7 +75,7 @@ public class YumTest { @Test public void testAlreadyInstalled() { terminal.expectCommand( - "yum install --assumeyes --enablerepo=repo1 --enablerepo=repo2 package-1 package-2 2>&1", + "yum install --assumeyes --enablerepo=repo1 --enablerepo=repo2 --setopt skip_missing_names_on_install=False package-1 package-2 2>&1", 0, "foobar\nNothing to do\n"); @@ -88,7 +88,7 @@ public class YumTest { @Test public void testAlreadyUpgraded() { terminal.expectCommand( - "yum upgrade --assumeyes package-1 package-2 2>&1", + "yum upgrade --assumeyes --setopt skip_missing_names_on_update=False package-1 package-2 2>&1", 0, "foobar\nNo packages marked for update\n"); @@ -112,7 +112,7 @@ public class YumTest { @Test public void testInstall() { terminal.expectCommand( - "yum install --assumeyes package-1 package-2 2>&1", + "yum install --assumeyes --setopt skip_missing_names_on_install=False package-1 package-2 2>&1", 0, "installing, installing"); @@ -124,7 +124,7 @@ public class YumTest { @Test public void testInstallWithEnablerepo() { terminal.expectCommand( - "yum install --assumeyes --enablerepo=repo-name package-1 package-2 2>&1", + "yum install --assumeyes --enablerepo=repo-name --setopt skip_missing_names_on_install=False package-1 package-2 2>&1", 0, "installing, installing"); @@ -210,7 +210,7 @@ public class YumTest { @Test(expected = ChildProcessFailureException.class) public void testFailedInstall() { terminal.expectCommand( - "yum install --assumeyes --enablerepo=repo-name package-1 package-2 2>&1", + "yum install --assumeyes --enablerepo=repo-name --setopt skip_missing_names_on_install=False package-1 package-2 2>&1", 1, "error"); @@ -224,7 +224,7 @@ public class YumTest { @Test public void testUnknownPackages() { terminal.expectCommand( - "yum install --assumeyes package-1 package-2 package-3 2>&1", + "yum install --assumeyes --setopt skip_missing_names_on_install=False package-1 package-2 package-3 2>&1", 0, "Loaded plugins: fastestmirror, langpacks\n" + "Loading mirror speeds from cached hostfile\n" + diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java index 40cd6b03e0a..40bccf22434 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java @@ -629,8 +629,10 @@ public class NodeRepository extends AbstractComponent { List<Node> removed = new ArrayList<>(children); if (zone.cloud().value().equals("aws")) db.removeNodes(List.of(node)); - else + else { + node = node.with(IP.Config.EMPTY); move(node, State.deprovisioned, Agent.system, Optional.empty()); + } removed.add(node); return removed; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java index d96282f1722..652fd5c6861 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java @@ -55,6 +55,9 @@ class NodeAllocation { /** The number of nodes in the accepted nodes which are of the requested flavor */ private int acceptedOfRequestedFlavor = 0; + /** The number of nodes in the accepted nodes which are of the requested flavor and not already retired */ + private int acceptedNonretiredOfRequestedFlavor = 0; + /** The number of nodes rejected because of clashing parentHostname */ private int rejectedWithClashingParentHost = 0; @@ -115,7 +118,7 @@ class NodeAllocation { if (offered.status().wantToRetire()) wantToRetireNode = true; if (requestedNodes.isExclusive() && ! hostsOnly(application.tenant(), offered.parentHostname())) wantToRetireNode = true; - if (( ! saturated() && hasCompatibleFlavor(node)) || acceptToRetire(node)) + if (( ! saturatedByNonretired() && hasCompatibleFlavor(node)) || acceptToRetire(node)) accepted.add(acceptNode(node, wantToRetireNode, node.isResizable)); } else { @@ -213,6 +216,7 @@ class NodeAllocation { private boolean acceptToRetire(PrioritizableNode node) { if (node.node.state() != Node.State.active) return false; if (! node.node.allocation().get().membership().cluster().group().equals(cluster.group())) return false; + if (node.node.allocation().get().membership().retired()) return true; // don't second-guess if already retired return cluster.type().isContent() || (cluster.type() == ClusterSpec.Type.container && !hasCompatibleFlavor(node)); @@ -229,17 +233,15 @@ class NodeAllocation { node = node.with(node.allocation().get().withRequestedResources(requestedNodes.resources().orElse(node.flavor().resources()))); if (! wantToRetire) { - if (resize) { - NodeResources hostResources = allNodes.parentOf(node).get().flavor().resources(); - node = node.with(new Flavor(requestedNodes.resources().get() - .with(hostResources.diskSpeed()) - .with(hostResources.storageType()))); - } + if (resize && ! ( node.allocation().isPresent() && node.allocation().get().membership().retired())) + node = resize(node); if (node.state() != Node.State.active) // reactivated node - make sure its not retired node = node.unretire(); acceptedOfRequestedFlavor++; + if ( ! (node.allocation().isPresent() && node.allocation().get().membership().retired())) + acceptedNonretiredOfRequestedFlavor++; } else { ++wasRetiredJustNow; // Retire nodes which are of an unwanted flavor, retired flavor or have an overlapping parent host @@ -256,6 +258,13 @@ class NodeAllocation { return node; } + private Node resize(Node node) { + NodeResources hostResources = allNodes.parentOf(node).get().flavor().resources(); + return node.with(new Flavor(requestedNodes.resources().get() + .with(hostResources.diskSpeed()) + .with(hostResources.storageType()))); + } + private Node setCluster(ClusterSpec cluster, Node node) { ClusterMembership membership = node.allocation().get().membership().with(cluster); return node.with(node.allocation().get().with(membership)); @@ -266,6 +275,11 @@ class NodeAllocation { return requestedNodes.saturatedBy(acceptedOfRequestedFlavor); } + /** Returns true if no more nodes are needed in this list to not make changes to the retired set */ + private boolean saturatedByNonretired() { + return requestedNodes.saturatedBy(acceptedNonretiredOfRequestedFlavor); + } + /** Returns true if the content of this list is sufficient to meet the request */ boolean fulfilled() { return requestedNodes.fulfilledBy(acceptedOfRequestedFlavor); @@ -320,8 +334,10 @@ class NodeAllocation { } } else if (deltaRetiredCount < 0) { // unretire until deltaRetiredCount is 0 - for (PrioritizableNode node : byIncreasingIndex(nodes)) { - if ( node.node.allocation().get().membership().retired() && hasCompatibleFlavor(node)) { + for (PrioritizableNode node : byUnretiringPriority(nodes)) { + if ( node.node.allocation().get().membership().retired() && hasCompatibleFlavor(node) ) { + if (node.isResizable) + node.node = resize(node.node); node.node = node.node.unretire(); if (++deltaRetiredCount == 0) break; } @@ -332,7 +348,7 @@ class NodeAllocation { // Set whether the node is exclusive Allocation allocation = node.node.allocation().get(); node.node = node.node.with(allocation.with(allocation.membership() - .with(allocation.membership().cluster().exclusive(requestedNodes.isExclusive())))); + .with(allocation.membership().cluster().exclusive(requestedNodes.isExclusive())))); } return nodes.stream().map(n -> n.node).collect(Collectors.toList()); @@ -363,8 +379,12 @@ class NodeAllocation { return nodes.stream().sorted(nodeIndexComparator().reversed()).collect(Collectors.toList()); } - private List<PrioritizableNode> byIncreasingIndex(Set<PrioritizableNode> nodes) { - return nodes.stream().sorted(nodeIndexComparator()).collect(Collectors.toList()); + /** Prefer to unretire nodes we don't want to retire, and otherwise those with lower index */ + private List<PrioritizableNode> byUnretiringPriority(Set<PrioritizableNode> nodes) { + return nodes.stream() + .sorted(Comparator.comparing((PrioritizableNode n) -> n.node.status().wantToRetire()) + .thenComparing(n -> n.node.allocation().get().membership().index())) + .collect(Collectors.toList()); } private Comparator<PrioritizableNode> nodeIndexComparator() { 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 37d0e9bbfb8..6802196f80d 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 @@ -6,7 +6,6 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.HostFilter; import com.yahoo.config.provision.HostSpec; @@ -23,7 +22,6 @@ import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.applications.Application; -import com.yahoo.vespa.hosted.provision.applications.Cluster; import com.yahoo.vespa.hosted.provision.node.Allocation; import com.yahoo.vespa.hosted.provision.node.filter.ApplicationFilter; import com.yahoo.vespa.hosted.provision.node.filter.NodeHostFilter; @@ -206,7 +204,7 @@ public class NodeRepositoryProvisioner implements Provisioner { node.status().vespaVersion(), nodeAllocation.networkPorts(), requestedResources == NodeResources.unspecified ? Optional.empty() : Optional.of(requestedResources), - node.status().dockerImage().map(DockerImage::repository))); + node.status().dockerImage())); if (nodeAllocation.networkPorts().isPresent()) { log.log(LogLevel.DEBUG, () -> "Prepared node " + node.hostname() + " has port allocations"); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java index 516bb2e84ea..8cfcbcb3797 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java @@ -145,6 +145,18 @@ class PrioritizableNode implements Comparable<PrioritizableNode> { return node.id(); } + @Override + public int hashCode() { + return node.hashCode(); + } + + @Override + public boolean equals(Object other) { + if (other == this) return true; + if ( ! (other instanceof PrioritizableNode)) return false; + return this.node.equals(((PrioritizableNode)other).node); + } + static class Builder { public final Node node; diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java index 38b46df77f5..7bb80fe2a21 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java @@ -5,6 +5,7 @@ import com.yahoo.config.provision.NodeType; import com.yahoo.test.ManualClock; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.History; +import com.yahoo.vespa.hosted.provision.node.IP; import com.yahoo.vespa.hosted.provision.node.Report; import com.yahoo.vespa.hosted.provision.node.Reports; import org.junit.Test; @@ -123,6 +124,10 @@ public class NodeRepositoryTest { tester.addNode("node20", "node20", "host2", "docker", NodeType.tenant); assertEquals(6, tester.nodeRepository().getNodes().size()); + Node node = tester.nodeRepository().getNode("host1").get(); + IP.Config cfg = new IP.Config(Set.of("127.0.0.1"), Set.of()); + node = node.with(cfg); + tester.setNodeState("node11", Node.State.active); try { @@ -145,6 +150,7 @@ public class NodeRepositoryTest { assertEquals(4, tester.nodeRepository().getNodes().size()); tester.nodeRepository().removeRecursively("host1"); assertEquals(Node.State.deprovisioned, tester.nodeRepository().getNode("host1").get().state()); + assertEquals(IP.Config.EMPTY.primary(), tester.nodeRepository().getNode("host1").get().ipConfig().primary()); } @Test diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java index 50399e9c87f..7e9c3ef09dc 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java @@ -52,7 +52,7 @@ public class InPlaceResizeProvisionTest { private static final ClusterSpec container1 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("container1")).vespaVersion("7.157.9").build(); private static final ClusterSpec container2 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("container2")).vespaVersion("7.157.9").build(); - private static final ClusterSpec content1 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("content1")).vespaVersion("7.157.9").build(); + private static final ClusterSpec content1 = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("content1")).vespaVersion("7.157.9").build(); private final InMemoryFlagSource flagSource = new InMemoryFlagSource(); private final ProvisioningTester tester = new ProvisioningTester.Builder() @@ -66,10 +66,10 @@ public class InPlaceResizeProvisionTest { addParentHosts(4, largeResources.with(fast).with(local)); new PrepareHelper(tester, app).prepare(container1, 4, 1, mediumResources).activate(); - assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); new PrepareHelper(tester, app).prepare(container1, 4, 1, largeResources).activate(); - assertClusterSizeAndResources(container1, 4, new NodeResources(8, 16, 32, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(8, 16, 32, 1, fast, local)); assertEquals("No nodes are retired", 0, tester.getNodes(app, Node.State.active).retired().size()); } @@ -78,10 +78,10 @@ public class InPlaceResizeProvisionTest { addParentHosts(4, mediumResources.with(fast).with(local)); new PrepareHelper(tester, app).prepare(container1, 4, 1, mediumResources).activate(); - assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); new PrepareHelper(tester, app).prepare(container1, 4, 1, smallResources).activate(); - assertClusterSizeAndResources(container1, 4, new NodeResources(2, 4, 8, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(2, 4, 8, 1, fast, local)); assertEquals("No nodes are retired", 0, tester.getNodes(app, Node.State.active).retired().size()); } @@ -94,16 +94,16 @@ public class InPlaceResizeProvisionTest { .prepare(container2, 4, 1, mediumResources) .activate(); Set<String> container1Hostnames = listCluster(container1).stream().map(Node::hostname).collect(Collectors.toSet()); - assertClusterSizeAndResources(container1, 4, new NodeResources(2, 4, 8, 1, fast, local)); - assertClusterSizeAndResources(container2, 4, new NodeResources(4, 8, 16, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(2, 4, 8, 1, fast, local)); + assertSizeAndResources(container2, 4, new NodeResources(4, 8, 16, 1, fast, local)); new PrepareHelper(tester, app) .prepare(container1, 4, 1, mediumResources) .prepare(container2, 4, 1, smallResources) .activate(); assertEquals(container1Hostnames, listCluster(container1).stream().map(Node::hostname).collect(Collectors.toSet())); - assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); - assertClusterSizeAndResources(container2, 4, new NodeResources(2, 4, 8, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); + assertSizeAndResources(container2, 4, new NodeResources(2, 4, 8, 1, fast, local)); assertEquals("No nodes are retired", 0, tester.getNodes(app, Node.State.active).retired().size()); } @@ -117,7 +117,7 @@ public class InPlaceResizeProvisionTest { new PrepareHelper(tester, app) .prepare(container1, 6, 1, largeResources).activate(); assertTrue(listCluster(container1).stream().map(Node::hostname).collect(Collectors.toSet()).containsAll(initialHostnames)); - assertClusterSizeAndResources(container1, 6, new NodeResources(8, 16, 32, 1, fast, local)); + assertSizeAndResources(container1, 6, new NodeResources(8, 16, 32, 1, fast, local)); assertEquals("No nodes are retired", 0, tester.getNodes(app, Node.State.active).retired().size()); } @@ -156,13 +156,34 @@ public class InPlaceResizeProvisionTest { assertTrue("All initial nodes should still be allocated to the application", initialHostnames.isEmpty()); } + /** In this scenario there should be no resizing */ + @Test + public void increase_size_decrease_resources() { + addParentHosts(12, largeResources.with(fast)); + + NodeResources resources = new NodeResources(4, 8, 16, 1); + NodeResources halvedResources = new NodeResources(2, 4, 8, 1); + + new PrepareHelper(tester, app).prepare(container1, 4, 1, resources).activate(); + assertSizeAndResources(container1, 4, resources); + + // No resizing since it would initially (before redistribution) lead to too few resources: + new PrepareHelper(tester, app).prepare(container1, 8, 1, halvedResources).activate(); + assertSizeAndResources(listCluster(container1).retired(), 4, resources); + assertSizeAndResources(listCluster(container1).not().retired(), 8, halvedResources); + + // Redeploying the same capacity should also not lead to any resizing + new PrepareHelper(tester, app).prepare(container1, 8, 1, halvedResources).activate(); + assertSizeAndResources(listCluster(container1).retired(), 4, resources); + assertSizeAndResources(listCluster(container1).not().retired(), 8, halvedResources); + } @Test(expected = OutOfCapacityException.class) public void cannot_inplace_decrease_resources_while_increasing_cluster_size() { addParentHosts(6, mediumResources.with(fast).with(local)); new PrepareHelper(tester, app).prepare(container1, 4, 1, mediumResources).activate(); - assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); new PrepareHelper(tester, app).prepare(container1, 6, 1, smallResources); } @@ -172,7 +193,7 @@ public class InPlaceResizeProvisionTest { addParentHosts(4, largeResources.with(fast).with(local)); new PrepareHelper(tester, app).prepare(container1, 4, 1, mediumResources).activate(); - assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); new PrepareHelper(tester, app).prepare(container1, 2, 1, smallResources); } @@ -182,7 +203,7 @@ public class InPlaceResizeProvisionTest { addParentHosts(4, largeResources.with(fast).with(local)); new PrepareHelper(tester, app).prepare(container1, 4, 1, mediumResources).activate(); - assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); new PrepareHelper(tester, app).prepare(container1, 4, 2, smallResources); } @@ -192,10 +213,13 @@ public class InPlaceResizeProvisionTest { tester.prepareAndActivateInfraApplication(infraApp, NodeType.host); } - private void assertClusterSizeAndResources(ClusterSpec cluster, int clusterSize, NodeResources resources) { - NodeList nodes = listCluster(cluster); - nodes.forEach(node -> assertEquals(node.toString(), node.flavor().resources(), resources)); - assertEquals(clusterSize, nodes.size()); + private void assertSizeAndResources(ClusterSpec cluster, int clusterSize, NodeResources resources) { + assertSizeAndResources(listCluster(cluster), clusterSize, resources); + } + + private void assertSizeAndResources(NodeList nodes, int size, NodeResources resources) { + assertEquals(size, nodes.size()); + nodes.forEach(n -> assertEquals(resources, n.flavor().resources())); } private NodeList listCluster(ClusterSpec cluster) { diff --git a/parent/pom.xml b/parent/pom.xml index a6c606d2c31..454b4677cfa 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -479,6 +479,11 @@ <version>${athenz.version}</version> </dependency> <dependency> + <groupId>org.apache.commons</groupId> + <artifactId>commons-math3</artifactId> + <version>${commons.math3.version}</version> + </dependency> + <dependency> <groupId>commons-cli</groupId> <artifactId>commons-cli</artifactId> <version>1.4</version> @@ -755,6 +760,7 @@ --> <curator.version>2.9.1</curator.version> <jna.version>4.5.2</jna.version> + <commons.math3.version>3.6.1</commons.math3.version> <junit.version>5.4.2</junit.version> <maven-assembly-plugin.version>3.1.1</maven-assembly-plugin.version> <maven-bundle-plugin.version>3.5.0</maven-bundle-plugin.version> diff --git a/processing/src/main/java/com/yahoo/processing/request/Properties.java b/processing/src/main/java/com/yahoo/processing/request/Properties.java index cadc658417b..9362de59203 100644 --- a/processing/src/main/java/com/yahoo/processing/request/Properties.java +++ b/processing/src/main/java/com/yahoo/processing/request/Properties.java @@ -543,7 +543,7 @@ public class Properties implements Cloneable { /** * Returns a property as a Double * - * @return the integer value of the name, or null if the property is null + * @return the double value of the name, or null if the property is null * @throws NumberFormatException if the given parameter exists but have a value which * is not parseable as a number */ @@ -554,7 +554,7 @@ public class Properties implements Cloneable { /** * Returns a property as a Double * - * @return the integer value of the name, or null if the property is null + * @return the double value of the name, or null if the property is null * @throws NumberFormatException if the given parameter exists but have a value which * is not parseable as a number */ diff --git a/searchcore/src/vespa/searchcore/proton/server/proton.cpp b/searchcore/src/vespa/searchcore/proton/server/proton.cpp index 1ab1ba72b4f..c886b371064 100644 --- a/searchcore/src/vespa/searchcore/proton/server/proton.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/proton.cpp @@ -685,25 +685,6 @@ Proton::prepareRestart() namespace { -int countOpenFiles() -{ - static const char * const fd_dir_name = "/proc/self/fd"; - int count = 0; - DIR *dp = opendir(fd_dir_name); - if (dp != nullptr) { - struct dirent *ptr; - while ((ptr = readdir(dp)) != nullptr) { - if (strcmp(".", ptr->d_name) == 0) continue; - if (strcmp("..", ptr->d_name) == 0) continue; - ++count; - } - closedir(dp); - } else { - LOG(warning, "could not scan directory %s: %s", fd_dir_name, strerror(errno)); - } - return count; -} - void updateExecutorMetrics(ExecutorMetrics &metrics, const vespalib::ThreadStackExecutor::Stats &stats) @@ -730,7 +711,7 @@ Proton::updateMetrics(const vespalib::MonitorGuard &) metrics.resourceUsage.memory.set(usageState.memoryState().usage()); metrics.resourceUsage.memoryUtilization.set(usageState.memoryState().utilization()); metrics.resourceUsage.memoryMappings.set(usageFilter.getMemoryStats().getMappingsCount()); - metrics.resourceUsage.openFileDescriptors.set(countOpenFiles()); + metrics.resourceUsage.openFileDescriptors.set(FastOS_File::count_open_files()); metrics.resourceUsage.feedingBlocked.set((usageFilter.acceptWriteOperation() ? 0.0 : 1.0)); } { diff --git a/searchlib/src/main/javacc/RankingExpressionParser.jj b/searchlib/src/main/javacc/RankingExpressionParser.jj index 6fa915a134d..8aa10bf7b34 100755 --- a/searchlib/src/main/javacc/RankingExpressionParser.jj +++ b/searchlib/src/main/javacc/RankingExpressionParser.jj @@ -598,21 +598,21 @@ TensorFunctionNode tensorXwPlusB() : TensorFunctionNode tensorArgmax() : { ExpressionNode tensor; - String dimension; + List<String> dimensions = null; } { - <ARGMAX> <LBRACE> tensor = expression() <COMMA> dimension = identifier() <RBRACE> - { return new TensorFunctionNode(new Argmax(TensorFunctionNode.wrap(tensor), dimension)); } + <ARGMAX> <LBRACE> tensor = expression() dimensions = tagCommaLeadingList() <RBRACE> + { return new TensorFunctionNode(new Argmax(TensorFunctionNode.wrap(tensor), dimensions)); } } TensorFunctionNode tensorArgmin() : { ExpressionNode tensor; - String dimension; + List<String> dimensions = null; } { - <ARGMIN> <LBRACE> tensor = expression() <COMMA> dimension = identifier() <RBRACE> - { return new TensorFunctionNode(new Argmin(TensorFunctionNode.wrap(tensor), dimension)); } + <ARGMIN> <LBRACE> tensor = expression() dimensions = tagCommaLeadingList() <RBRACE> + { return new TensorFunctionNode(new Argmin(TensorFunctionNode.wrap(tensor), dimensions)); } } LambdaFunctionNode lambdaFunction() : diff --git a/searchlib/src/tests/attribute/attribute_test.cpp b/searchlib/src/tests/attribute/attribute_test.cpp index f402f536ae2..4715292f86a 100644 --- a/searchlib/src/tests/attribute/attribute_test.cpp +++ b/searchlib/src/tests/attribute/attribute_test.cpp @@ -3,6 +3,7 @@ #include <vespa/document/fieldvalue/intfieldvalue.h> #include <vespa/document/fieldvalue/stringfieldvalue.h> #include <vespa/document/update/arithmeticvalueupdate.h> +#include <vespa/document/update/assignvalueupdate.h> #include <vespa/document/update/mapvalueupdate.h> #include <vespa/fastlib/io/bufferedfile.h> #include <vespa/searchlib/attribute/attribute.h> @@ -1493,20 +1494,21 @@ AttributeTest::testMapValueUpdate(const AttributePtr & ptr, BufferType initValue typedef ArithmeticValueUpdate ArithVU; auto & vec = static_cast<VectorType &>(*ptr.get()); - addDocs(ptr, 6); - for (uint32_t doc = 0; doc < 6; ++doc) { + addDocs(ptr, 7); + for (uint32_t doc = 0; doc < 7; ++doc) { ASSERT_TRUE(vec.append(doc, initValue.getValue(), 100)); } - EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 6u); + EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 7u); EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 0u); EXPECT_TRUE(ptr->apply(0, MapVU(initFieldValue, ArithVU(ArithVU::Add, 10)))); EXPECT_TRUE(ptr->apply(1, MapVU(initFieldValue, ArithVU(ArithVU::Sub, 10)))); EXPECT_TRUE(ptr->apply(2, MapVU(initFieldValue, ArithVU(ArithVU::Mul, 10)))); EXPECT_TRUE(ptr->apply(3, MapVU(initFieldValue, ArithVU(ArithVU::Div, 10)))); + EXPECT_TRUE(ptr->apply(6, MapVU(initFieldValue, AssignValueUpdate(IntFieldValue(70))))); ptr->commit(); - EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 10u); - EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 4u); + EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 12u); + EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 5u); std::vector<BufferType> buf(2); ptr->get(0, &buf[0], 2); @@ -1517,6 +1519,8 @@ AttributeTest::testMapValueUpdate(const AttributePtr & ptr, BufferType initValue EXPECT_EQUAL(buf[0].getWeight(), 1000); ptr->get(3, &buf[0], 2); EXPECT_EQUAL(buf[0].getWeight(), 10); + ptr->get(6, &buf[0], 2); + EXPECT_EQUAL(buf[0].getWeight(), 70); // removeifzero EXPECT_TRUE(ptr->apply(4, MapVU(initFieldValue, ArithVU(ArithVU::Sub, 100)))); @@ -1527,8 +1531,8 @@ AttributeTest::testMapValueUpdate(const AttributePtr & ptr, BufferType initValue EXPECT_EQUAL(ptr->get(4, &buf[0], 2), uint32_t(1)); EXPECT_EQUAL(buf[0].getWeight(), 0); } - EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 11u); - EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 5u); + EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 13u); + EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 6u); // createifnonexistant EXPECT_TRUE(ptr->apply(5, MapVU(nonExistant, ArithVU(ArithVU::Add, 10)))); @@ -1542,18 +1546,18 @@ AttributeTest::testMapValueUpdate(const AttributePtr & ptr, BufferType initValue EXPECT_EQUAL(ptr->get(5, &buf[0], 2), uint32_t(1)); EXPECT_EQUAL(buf[0].getWeight(), 100); } - EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 12u); - EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 6u); + EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 14u); + EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 7u); // try divide by zero (should be ignored) vec.clearDoc(0); - EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 13u); + EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 15u); ASSERT_TRUE(vec.append(0, initValue.getValue(), 12345)); - EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 14u); + EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 16u); EXPECT_TRUE(ptr->apply(0, MapVU(initFieldValue, ArithVU(ArithVU::Div, 0)))); - EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 14u); - EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 6u); + EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 16u); + EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 7u); ptr->commit(); ptr->get(0, &buf[0], 1); EXPECT_EQUAL(buf[0].getWeight(), 12345); diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.cpp b/searchlib/src/vespa/searchlib/attribute/attributevector.cpp index 1f002ce612c..08c7186e8c7 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributevector.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attributevector.cpp @@ -11,6 +11,7 @@ #include "ipostinglistattributebase.h" #include "ipostinglistsearchcontext.h" #include "stringbase.h" +#include <vespa/document/update/assignvalueupdate.h> #include <vespa/document/update/mapvalueupdate.h> #include <vespa/fastlib/io/bufferedfile.h> #include <vespa/searchlib/common/tunefileinfo.h> @@ -28,6 +29,7 @@ LOG_SETUP(".searchlib.attribute.attributevector"); using vespalib::getLastErrorString; using document::ValueUpdate; +using document::AssignValueUpdate; using vespalib::make_string; using vespalib::Array; using vespalib::IllegalStateException; @@ -467,6 +469,9 @@ AttributeVector::apply(DocId doc, const MapValueUpdate &map) { if (vu.inherits(ArithmeticValueUpdate::classId)) { const ArithmeticValueUpdate &au(static_cast<const ArithmeticValueUpdate &>(vu)); retval = applyWeight(doc, map.getKey(), au); + } else if (vu.inherits(AssignValueUpdate::classId)) { + const AssignValueUpdate &au(static_cast<const AssignValueUpdate &>(vu)); + retval = applyWeight(doc, map.getKey(), au); } else { retval = false; } @@ -477,6 +482,7 @@ AttributeVector::apply(DocId doc, const MapValueUpdate &map) { bool AttributeVector::applyWeight(DocId, const FieldValue &, const ArithmeticValueUpdate &) { return false; } +bool AttributeVector::applyWeight(DocId, const FieldValue&, const AssignValueUpdate&) { return false; } void AttributeVector::removeAllOldGenerations() { diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.h b/searchlib/src/vespa/searchlib/attribute/attributevector.h index 4a53f2dd5a2..75699868691 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributevector.h +++ b/searchlib/src/vespa/searchlib/attribute/attributevector.h @@ -30,6 +30,7 @@ class FastOS_FileInterface; namespace document { class ArithmeticValueUpdate; + class AssignValueUpdate; class MapValueUpdate; class FieldValue; } @@ -332,6 +333,9 @@ protected: template<typename T> bool adjustWeight(ChangeVectorT< ChangeTemplate<T> > &changes, DocId doc, const T &v, const ArithmeticValueUpdate &wd); + template<typename T> + bool adjustWeight(ChangeVectorT< ChangeTemplate<T> > &changes, DocId doc, const T &v, const document::AssignValueUpdate &wu); + template <typename T> static int32_t applyWeightChange(int32_t weight, const ChangeTemplate<T> &weightChange) { @@ -341,6 +345,8 @@ protected: return weight * weightChange._weight; } else if (weightChange._type == ChangeBase::DIVWEIGHT) { return weight / weightChange._weight; + } else if (weightChange._type == ChangeBase::SETWEIGHT) { + return weightChange._weight; } return weight; } @@ -561,6 +567,7 @@ private: virtual void onAddDocs(DocId docIdLimit) = 0; void divideByZeroWarning(); virtual bool applyWeight(DocId doc, const FieldValue &fv, const ArithmeticValueUpdate &wAdjust); + virtual bool applyWeight(DocId doc, const FieldValue& fv, const document::AssignValueUpdate& wAdjust); virtual void onSave(IAttributeSaveTarget & saveTarget); virtual bool onLoad(); diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.hpp b/searchlib/src/vespa/searchlib/attribute/attributevector.hpp index 66c2678497a..efc96bc57c2 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributevector.hpp +++ b/searchlib/src/vespa/searchlib/attribute/attributevector.hpp @@ -3,7 +3,9 @@ #include "attributevector.h" #include "integerbase.h" +#include <vespa/document/fieldvalue/intfieldvalue.h> #include <vespa/document/update/arithmeticvalueupdate.h> +#include <vespa/document/update/assignvalueupdate.h> #include <cmath> namespace search { @@ -61,6 +63,32 @@ AttributeVector::adjustWeight(ChangeVectorT< ChangeTemplate<T> > & changes, DocI template<typename T> bool +AttributeVector::adjustWeight(ChangeVectorT< ChangeTemplate<T> >& changes, DocId doc, const T& v, const document::AssignValueUpdate& wu) +{ + bool retval(hasWeightedSetType() && (doc < getNumDocs())); + if (retval) { + size_t oldSz(changes.size()); + if (wu.hasValue()) { + const FieldValue &wv = wu.getValue(); + if (wv.inherits(document::IntFieldValue::classId)) { + changes.push_back(ChangeTemplate<T>(ChangeBase::SETWEIGHT, doc, v, wv.getAsInt())); + } else { + retval = false; + } + } else { + retval = false; + } + if (retval) { + const size_t diff = changes.size() - oldSz; + _status.incNonIdempotentUpdates(diff); + _status.incUpdates(diff); + } + } + return retval; +} + +template<typename T> +bool AttributeVector::applyArithmetic(ChangeVectorT< ChangeTemplate<T> > & changes, DocId doc, const T & v, const ArithmeticValueUpdate & arithm) { diff --git a/searchlib/src/vespa/searchlib/attribute/changevector.h b/searchlib/src/vespa/searchlib/attribute/changevector.h index 75eac911f6c..af4a4fd6618 100644 --- a/searchlib/src/vespa/searchlib/attribute/changevector.h +++ b/searchlib/src/vespa/searchlib/attribute/changevector.h @@ -18,6 +18,7 @@ struct ChangeBase { INCREASEWEIGHT, MULWEIGHT, DIVWEIGHT, + SETWEIGHT, ADD, SUB, MUL, diff --git a/searchlib/src/vespa/searchlib/attribute/floatbase.cpp b/searchlib/src/vespa/searchlib/attribute/floatbase.cpp index 813180bc5d7..4ffc6e333b6 100644 --- a/searchlib/src/vespa/searchlib/attribute/floatbase.cpp +++ b/searchlib/src/vespa/searchlib/attribute/floatbase.cpp @@ -76,6 +76,12 @@ bool FloatingPointAttribute::applyWeight(DocId doc, const FieldValue & fv, const return AttributeVector::adjustWeight(_changes, doc, NumericChangeData<double>(v), wAdjust); } +bool FloatingPointAttribute::applyWeight(DocId doc, const FieldValue& fv, const document::AssignValueUpdate& wAdjust) +{ + double v = fv.getAsDouble(); + return AttributeVector::adjustWeight(_changes, doc, NumericChangeData<double>(v), wAdjust); +} + bool FloatingPointAttribute::apply(DocId doc, const ArithmeticValueUpdate & op) { bool retval(doc < getNumDocs()); diff --git a/searchlib/src/vespa/searchlib/attribute/floatbase.h b/searchlib/src/vespa/searchlib/attribute/floatbase.h index ce8d22c2706..65a146d7ba7 100644 --- a/searchlib/src/vespa/searchlib/attribute/floatbase.h +++ b/searchlib/src/vespa/searchlib/attribute/floatbase.h @@ -28,6 +28,7 @@ public: } bool apply(DocId doc, const ArithmeticValueUpdate & op); bool applyWeight(DocId doc, const FieldValue & fv, const ArithmeticValueUpdate & wAdjust) override; + bool applyWeight(DocId doc, const FieldValue& fv, const document::AssignValueUpdate& wAdjust) override; uint32_t clearDoc(DocId doc) override; protected: const char * getString(DocId doc, char * s, size_t sz) const override; diff --git a/searchlib/src/vespa/searchlib/attribute/integerbase.cpp b/searchlib/src/vespa/searchlib/attribute/integerbase.cpp index f065bc63648..a150b5e1699 100644 --- a/searchlib/src/vespa/searchlib/attribute/integerbase.cpp +++ b/searchlib/src/vespa/searchlib/attribute/integerbase.cpp @@ -80,6 +80,12 @@ bool IntegerAttribute::applyWeight(DocId doc, const FieldValue & fv, const Arith return AttributeVector::adjustWeight(_changes, doc, NumericChangeData<largeint_t>(v), wAdjust); } +bool IntegerAttribute::applyWeight(DocId doc, const FieldValue& fv, const document::AssignValueUpdate& wAdjust) +{ + largeint_t v = fv.getAsLong(); + return AttributeVector::adjustWeight(_changes, doc, NumericChangeData<largeint_t>(v), wAdjust); +} + bool IntegerAttribute::apply(DocId doc, const ArithmeticValueUpdate & op) { bool retval(doc < getNumDocs()); diff --git a/searchlib/src/vespa/searchlib/attribute/integerbase.h b/searchlib/src/vespa/searchlib/attribute/integerbase.h index 46774f9cebf..4e5fe8fec19 100644 --- a/searchlib/src/vespa/searchlib/attribute/integerbase.h +++ b/searchlib/src/vespa/searchlib/attribute/integerbase.h @@ -28,6 +28,7 @@ public: } bool apply(DocId doc, const ArithmeticValueUpdate & op); bool applyWeight(DocId doc, const FieldValue & fv, const ArithmeticValueUpdate & wAdjust) override; + bool applyWeight(DocId doc, const FieldValue& fv, const document::AssignValueUpdate& wAdjust) override; uint32_t clearDoc(DocId doc) override; protected: IntegerAttribute(const vespalib::string & name, const Config & c); diff --git a/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp index 219b3dd2473..40f211c2621 100644 --- a/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp @@ -37,7 +37,7 @@ MultiValueEnumAttribute<B, M>::considerAttributeChange(const Change & c, UniqueS { if (c._type == ChangeBase::APPEND || (this->getInternalCollectionType().createIfNonExistant() && - (c._type >= ChangeBase::INCREASEWEIGHT && c._type <= ChangeBase::DIVWEIGHT))) + (c._type >= ChangeBase::INCREASEWEIGHT && c._type <= ChangeBase::SETWEIGHT))) { EnumIndex idx; if (!this->_enumStore.find_index(c._data.raw(), idx)) { diff --git a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp index b23860ebd31..0ae536aee23 100644 --- a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp @@ -172,7 +172,7 @@ MultiValueAttribute<B, M>::apply_attribute_changes_to_wset(DocumentValues& docVa wset_inserted[data] = current->_weight; } else if (current->_type == ChangeBase::REMOVE) { wset_inserted.erase(data); - } else if ((current->_type >= ChangeBase::INCREASEWEIGHT) && (current->_type <= ChangeBase::DIVWEIGHT)) { + } else if ((current->_type >= ChangeBase::INCREASEWEIGHT) && (current->_type <= ChangeBase::SETWEIGHT)) { auto existing = wset_inserted.find(data); if (existing != wset_inserted.end()) { existing->second = this->applyWeightChange(existing->second, *current); diff --git a/searchlib/src/vespa/searchlib/attribute/stringbase.cpp b/searchlib/src/vespa/searchlib/attribute/stringbase.cpp index 40e706e924d..d888ead21bf 100644 --- a/searchlib/src/vespa/searchlib/attribute/stringbase.cpp +++ b/searchlib/src/vespa/searchlib/attribute/stringbase.cpp @@ -309,6 +309,12 @@ bool StringAttribute::applyWeight(DocId doc, const FieldValue & fv, const Arithm return AttributeVector::adjustWeight(_changes, doc, StringChangeData(v), wAdjust); } +bool StringAttribute::applyWeight(DocId doc, const FieldValue& fv, const document::AssignValueUpdate& wAdjust) +{ + vespalib::string v = fv.getAsString(); + return AttributeVector::adjustWeight(_changes, doc, StringChangeData(v), wAdjust); +} + bool StringAttribute::apply(DocId, const ArithmeticValueUpdate & ) { return false; diff --git a/searchlib/src/vespa/searchlib/attribute/stringbase.h b/searchlib/src/vespa/searchlib/attribute/stringbase.h index 3518544cbdc..e6238cc0f94 100644 --- a/searchlib/src/vespa/searchlib/attribute/stringbase.h +++ b/searchlib/src/vespa/searchlib/attribute/stringbase.h @@ -44,6 +44,7 @@ public: } bool apply(DocId doc, const ArithmeticValueUpdate & op); bool applyWeight(DocId doc, const FieldValue & fv, const ArithmeticValueUpdate & wAdjust) override; + bool applyWeight(DocId doc, const FieldValue& fv, const document::AssignValueUpdate& wAdjust) override; bool findEnum(const char * value, EnumHandle & e) const override = 0; std::vector<EnumHandle> findFoldedEnums(const char *value) const override = 0; uint32_t get(DocId doc, largeint_t * v, uint32_t sz) const override; diff --git a/storage/src/tests/distributor/putoperationtest.cpp b/storage/src/tests/distributor/putoperationtest.cpp index bd76b559490..f5bacfae252 100644 --- a/storage/src/tests/distributor/putoperationtest.cpp +++ b/storage/src/tests/distributor/putoperationtest.cpp @@ -182,7 +182,7 @@ TEST_F(PutOperationTest, do_not_send_inline_split_if_not_configured) { _sender.getCommands(true, true)); } -TEST_F(PutOperationTest, node_removed_on_reply) { +TEST_F(PutOperationTest, return_success_if_op_acked_on_all_replicas_even_if_bucket_concurrently_removed_from_db) { setupDistributor(2, 2, "storage:2 distributor:1"); createAndSendSampleDocument(TIMEOUT); @@ -194,14 +194,19 @@ TEST_F(PutOperationTest, node_removed_on_reply) { getExternalOperationHandler().removeNodeFromDB(makeDocumentBucket(document::BucketId(16, 0x1dd4)), 0); + // If we get an ACK from the backend nodes, the operation has been persisted OK. + // Even if the bucket has been removed from the DB in the meantime (usually would + // happen due to ownership changes) there is no reason for us to trigger a client + // resend in this scenario. + // If a node goes down (as opposed to distributor ownership transfer) and therefore + // has its replicas removed from the DB, this by definition has happened-after + // the ACK was sent from the node, so returning OK here still maintains the + // backend persistence property. sendReply(0); sendReply(1); ASSERT_EQ("PutReply(id:test:testdoctype1::, BucketId(0x0000000000000000), " - "timestamp 100) ReturnCode(BUCKET_DELETED, " - "Bucket(BucketSpace(0x0000000000000001), BucketId(0x4000000000001dd4)) was deleted from nodes [0] " - "after message was sent but before it was done. " - "Sent to [0,1])", + "timestamp 100) ReturnCode(NONE)", _sender.getLastReply()); } diff --git a/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp b/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp index bc0baf2faf7..d98a2ce6a74 100644 --- a/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp +++ b/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp @@ -150,52 +150,6 @@ PersistenceMessageTrackerImpl::canSendReplyEarly() const } void -PersistenceMessageTrackerImpl::checkCopiesDeleted() -{ - if ( ! _reply) { - return; - } - - // Don't check the buckets that have been remapped here, as we will - // create them. - const auto &bucketSpaceRepo(_manager.getBucketSpaceRepo()); - for (const auto & entry : _bucketInfo) { - const auto &bucketSpace(bucketSpaceRepo.get(entry.first.getBucketSpace())); - const auto &bucketDb(bucketSpace.getBucketDatabase()); - BucketDatabase::Entry dbentry = bucketDb.get(entry.first.getBucketId()); - - if (!dbentry.valid()) { - continue; - } - - std::vector<uint16_t> missing; - std::vector<uint16_t> total; - - for (const BucketCopy & bucketCopy : entry.second) { - if (dbentry->getNode(bucketCopy.getNode()) == nullptr) { - missing.push_back(bucketCopy.getNode()); - } - - total.push_back(bucketCopy.getNode()); - } - - if (!missing.empty()) { - std::ostringstream msg; - msg << entry.first.toString() << " was deleted from nodes [" - << commaSeparated(missing) - << "] after message was sent but before it was done. Sent to [" - << commaSeparated(total) - << "]"; - - LOG(debug, "%s", msg.str().c_str()); - _reply->setResult(api::ReturnCode(api::ReturnCode::BUCKET_DELETED, - msg.str())); - break; - } - } -} - -void PersistenceMessageTrackerImpl::addBucketInfoFromReply( uint16_t node, const api::BucketInfoReply& reply) @@ -334,7 +288,6 @@ PersistenceMessageTrackerImpl::updateFromReply( if (finished()) { bool doRevert(shouldRevert()); - checkCopiesDeleted(); updateDB(); if (!hasSentReply()) { @@ -345,7 +298,6 @@ PersistenceMessageTrackerImpl::updateFromReply( } } else if (canSendReplyEarly()) { LOG(debug, "Sending reply early because initial redundancy has been reached"); - checkCopiesDeleted(); sendReply(sender); } } diff --git a/storage/src/vespa/storage/distributor/persistencemessagetracker.h b/storage/src/vespa/storage/distributor/persistencemessagetracker.h index 5a4d8e17452..21977ddd881 100644 --- a/storage/src/vespa/storage/distributor/persistencemessagetracker.h +++ b/storage/src/vespa/storage/distributor/persistencemessagetracker.h @@ -84,7 +84,6 @@ private: bool hasSentReply() const { return _reply.get() == 0; } bool shouldRevert() const; void sendReply(MessageSender& sender); - void checkCopiesDeleted(); void updateFailureResult(const api::BucketInfoReply& reply); void handleCreateBucketReply(api::BucketInfoReply& reply, uint16_t node); void handlePersistenceReply(api::BucketInfoReply& reply, uint16_t node); diff --git a/travis/travis-build-full.sh b/travis/travis-build-full.sh index 62821e0e06d..f44e5e55d7a 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="-Xss1m -Xms128m -Xmx2g" -source /etc/profile.d/enable-devtoolset-8.sh +source /etc/profile.d/enable-devtoolset-9.sh source /etc/profile.d/enable-rh-maven35.sh ccache --max-size=1250M diff --git a/travis/travis.sh b/travis/travis.sh index 77bfebf2505..315771b43b9 100755 --- a/travis/travis.sh +++ b/travis/travis.sh @@ -10,7 +10,7 @@ function bell() { done } -DOCKER_IMAGE=vespaengine/vespa-dev:latest +DOCKER_IMAGE=vespaengine/vespa-build-centos7:latest bell & docker run --rm -v ${HOME}/.m2:/root/.m2 -v ${HOME}/.ccache:/root/.ccache -v $(pwd):/source \ diff --git a/vagrant/.gitignore b/vagrant/.gitignore deleted file mode 100644 index d16c9dbeeb5..00000000000 --- a/vagrant/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -.vagrant/ -*.box
\ No newline at end of file diff --git a/vagrant/README.md b/vagrant/README.md deleted file mode 100644 index 71fa071cb46..00000000000 --- a/vagrant/README.md +++ /dev/null @@ -1,107 +0,0 @@ -<!-- Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> - -# Create C++ / Java dev environment on CentOS using VirtualBox and Vagrant - -## Prerequisites -* [Install VirtualBox](https://www.virtualbox.org/wiki/Downloads) -* [Install Vagrant](https://www.vagrantup.com/downloads.html) - -## Create dev environment - -#### 1. Change working directory to <vespa-source>/vagrant - - cd <vespa-source>/vagrant - -#### 2. Choose dev environment - -##### a. For a dev environment with plain centos/7 and no GUI: - - export VESPA_VAGRANT_VM_BOX=centos/7 - export VESPA_VAGRANT_DISABLE_GUI=true - -##### b. For a dev environment with GUI and CLion: - -Create centos7-desktop box: - -* Install packer by following guide at [packer.io](https://www.packer.io/intro/getting-started/install.html) - -* Clone boxcutter centos repo and build the box: -``` -git clone https://github.com/boxcutter/centos.git -./bin/box build centos7-desktop.json virtualbox -``` - -Example exports: - - export VESPA_VAGRANT_VM_BOX="centos7-desktop" - export VESPA_VAGRANT_VM_BOX_URL="$HOME/git/boxcutter/centos/box/virtualbox/centos7-desktop-xx.yyyy.z.box" - - -#### 3. Install Vagrant VirtualBox Guest Additions plugin -This is required for mounting shared folders and get mouse pointer integration and seamless windows in the virtual CentOS desktop. - - vagrant plugin install vagrant-vbguest - -#### 4. Start and provision the environment - - vagrant up - -#### 5. Connect to machine via SSH -SSH agent forwarding is enabled to ensure easy interaction with GitHub inside the machine. - - vagrant ssh - -#### 6. Checkout vespa source inside virtual machine -This is needed in order to compile and run tests fast on the local file system inside the virtual machine. - - git clone git@github.com:vespa-engine/vespa.git - -## Build Java modules -Please follow the build instructions described [here](../README.md#build-java-modules). - - -## Build C++ modules -Please follow the build instructions described [here](../README.md#build-c-modules). -Skip these steps if doing development with CLion. - - -## Build and Develop using CLion -CLion is installed as part of the environment and is recommended for C++ development. - -#### 1. Bootstrap C++ building -cd to the vespa/ directory created by git clone and execute: - - ./bootstrap.sh java - ./bootstrap-cpp.sh . . - -#### 2. Start CLion -Open a terminal inside the virtual CentOS desktop (password is "vagrant") and run: - - clion - -When prompted, configure toolchains as follows: - - CMake: /usr/bin/cmake3 - Make: /usr/bin/make - C Compiler: /opt/rh/devtoolset-8/root/usr/bin/cc - C++ Compiler: /opt/rh/devtoolset-8/root/usr/bin/c++ - -#### 3. Open the Vespa Project -Go to *File* -> *Open* and choose <vespa-source>>/CMakeLists.txt. - -#### 4. Set compiler threads -Go to *File* -> *Settings* -> *Build, Execution, Deployment* -> *CMake*. -Under *Build Options* specify "-j 4" and click *Apply*. - -#### 5. Run bootstrap again - - ./bootstrap-cpp.sh . . - -(Some of the changes made by it are undone by clion on the first startup.) - -#### 6. Build all modules -Choose target **all_modules** from the set of build targets at the top right and click build. - -## Starting and stopping the Vagrant machine -Use `vagrant suspend` to suspend the machine and then `vagrant resume` to resume it later on. -Alternatively use `vagrant halt` + `vagrant up` to shutdown and reboot. Latter approach is slower but requires less disk space since RAM content is not persisted to host. diff --git a/vagrant/Vagrantfile b/vagrant/Vagrantfile deleted file mode 100644 index be0824dbd55..00000000000 --- a/vagrant/Vagrantfile +++ /dev/null @@ -1,93 +0,0 @@ -# -*- mode: ruby -*- -# vi: set ft=ruby : - -disable_gui = ENV['VESPA_VAGRANT_DISABLE_GUI'] - -def get_mandatory_env_value(name) - opt = ENV[name] - if opt.nil? or opt.empty? - raise Vagrant::Errors::VagrantError.new, "Environment variable #{name} must be set to a valid value before running vagrant" - end - return opt -end - -def get_env_value(name, fallback) - opt = ENV[name] - if opt.nil? or opt.empty? - return fallback - end - return opt -end - -vm_box = get_mandatory_env_value('VESPA_VAGRANT_VM_BOX') -vm_memory = get_env_value('VESPA_VAGRANT_VM_MEMORY', "8192") -vm_cpus = get_env_value('VESPA_VAGRANT_VM_CPUS', 4) - -unless disable_gui - vm_box_url = get_mandatory_env_value('VESPA_VAGRANT_VM_BOX_URL') -end - -# For a complete reference, please see the online documentation at https://docs.vagrantup.com. -Vagrant.configure("2") do |config| - - config.vm.box = vm_box - config.vm.box_url = vm_box_url unless disable_gui - - config.ssh.forward_agent = true - - config.vm.synced_folder "../dist", "/vagrant/dist" - - config.vm.provider "virtualbox" do |vb| - # Display the VirtualBox GUI when booting the machine - vb.gui = true unless disable_gui - vb.name = "vespa-dev" - - vb.memory = vm_memory - vb.cpus = vm_cpus - end - - # Install required and nice-to-have packages - config.vm.provision "shell", inline: <<-SHELL - yum-config-manager --add-repo https://copr.fedorainfracloud.org/coprs/g/vespa/vespa/repo/epel-7/group_vespa-vespa-epel-7.repo - yum -y install epel-release - yum -y install centos-release-scl - yum -y install yum-utils - yum -y install git \ - ccache \ - maven \ - rpm-build \ - valgrind \ - sudo \ - firefox \ - vim \ - emacs - sed -e '/^BuildRequires:/d' -e 's/^Requires:/BuildRequires:/' /vagrant/dist/vespa.spec > /tmp/vesparun.spec - yum-builddep -y /vagrant/dist/vespa.spec /tmp/vesparun.spec - rm /tmp/vesparun.spec - echo -e "* soft nproc 409600\n* hard nproc 409600" > /etc/security/limits.d/99-nproc.conf - echo -e "* soft nofile 262144\n* hard nofile 262144" > /etc/security/limits.d/99-nofile.conf - - unless disable_gui - echo -e "fs.inotify.max_user_watches = 524288" > /etc/sysctl.d/clion.conf - wget -q -O - https://download.jetbrains.com/cpp/CLion-2018.1.6.tar.gz | tar -C /opt -zx - ln -sf /opt/clion-2018.1.6/bin/clion.sh /usr/bin/clion - end - - yum update -y - hostname localhost - SHELL - - # Add settings for Vespa and dev tools as the default user, usually 'vagrant' (privileged: false) - # NOTE: adding these settings to .bashrc would break vagrant suspend/resume/provision - # due to env vars modified by /opt/rh/devtoolset-8/enable. - config.vm.provision "shell", privileged: false, inline: <<-SCRIPT - grep -l VESPA_HOME ~/.bash_profile >/dev/null || (\ - printf "%s\n" \ - 'export VESPA_HOME=$HOME/vespa' \ - 'export PATH=$PATH:$VESPA_HOME/bin' \ - 'source /opt/rh/rh-maven35/enable' \ - 'source /opt/rh/devtoolset-8/enable' \ - >> ~/.bash_profile ) - SCRIPT - -end diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/utils/RoleCsrGenerator.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/utils/RoleCsrGenerator.java new file mode 100644 index 00000000000..102bfd82646 --- /dev/null +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/utils/RoleCsrGenerator.java @@ -0,0 +1,39 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.athenz.client.zts.utils; + +import com.yahoo.security.Pkcs10Csr; +import com.yahoo.security.Pkcs10CsrBuilder; +import com.yahoo.security.SubjectAlternativeName.Type; +import com.yahoo.vespa.athenz.api.AthenzIdentity; +import com.yahoo.vespa.athenz.api.AthenzRole; +import com.yahoo.vespa.athenz.client.zts.ZtsClient; + +import javax.security.auth.x500.X500Principal; +import java.security.KeyPair; + +import static com.yahoo.security.SignatureAlgorithm.SHA256_WITH_RSA; + +/** + * Generates a {@link Pkcs10Csr} instance for use with {@link ZtsClient#getRoleCertificate(AthenzRole, Pkcs10Csr)}. + * + * @author bjorncs + */ +public class RoleCsrGenerator { + + private final String dnsSuffix; + + public RoleCsrGenerator(String dnsSuffix) { + this.dnsSuffix = dnsSuffix; + } + + public Pkcs10Csr generateCsr(AthenzIdentity identity, AthenzRole role, KeyPair keyPair) { + return Pkcs10CsrBuilder.fromKeypair(new X500Principal("CN=" + role.toResourceNameString()), keyPair, SHA256_WITH_RSA) + .addSubjectAlternativeName( + Type.DNS_NAME, + String.format("%s.%s.%s", identity.getName(), identity.getDomainName().replace(".", "-"), dnsSuffix)) + .addSubjectAlternativeName( + Type.RFC822_NAME, + String.format("%s@%s", identity.getFullName(), dnsSuffix)) + .build(); + } +} diff --git a/vespa-hadoop/src/main/java/com/yahoo/vespa/hadoop/pig/VespaDocumentOperation.java b/vespa-hadoop/src/main/java/com/yahoo/vespa/hadoop/pig/VespaDocumentOperation.java index 94176bbb658..b6fe19f8cde 100644 --- a/vespa-hadoop/src/main/java/com/yahoo/vespa/hadoop/pig/VespaDocumentOperation.java +++ b/vespa-hadoop/src/main/java/com/yahoo/vespa/hadoop/pig/VespaDocumentOperation.java @@ -67,6 +67,7 @@ public class VespaDocumentOperation extends EvalFunc<String> { private static final String PROPERTY_CREATE_IF_NON_EXISTENT = "create-if-non-existent"; private static final String PROPERTY_ID_TEMPLATE = "docid"; private static final String PROPERTY_OPERATION = "operation"; + private static final String PROPERTY_VERBOSE = "verbose"; private static final String BAG_AS_MAP_FIELDS = "bag-as-map-fields"; private static final String SIMPLE_ARRAY_FIELDS = "simple-array-fields"; private static final String SIMPLE_OBJECT_FIELDS = "simple-object-fields"; @@ -99,6 +100,7 @@ public class VespaDocumentOperation extends EvalFunc<String> { partialOperationMap.put(UPDATE_MAP_FIELDS, PARTIAL_UPDATE_ASSIGN); } + private final boolean verbose; private final String template; private final Operation operation; private final Properties properties; @@ -113,6 +115,7 @@ public class VespaDocumentOperation extends EvalFunc<String> { properties = VespaConfiguration.loadProperties(params); template = properties.getProperty(PROPERTY_ID_TEMPLATE); operation = Operation.fromString(properties.getProperty(PROPERTY_OPERATION, "put")); + verbose = Boolean.parseBoolean(properties.getProperty(PROPERTY_VERBOSE, "false")); } @Override @@ -145,6 +148,9 @@ public class VespaDocumentOperation extends EvalFunc<String> { Schema inputSchema = getInputSchema(); Map<String, Object> fields = TupleTools.tupleMap(inputSchema, tuple); String docId = TupleTools.toString(fields, template); + if (verbose) { + System.out.println("Processing docId: "+ docId); + } // create json json = create(operation, docId, fields, properties, inputSchema); if (json == null || json.length() == 0) { diff --git a/vespa-hadoop/src/test/java/com/yahoo/vespa/hadoop/pig/VespaDocumentOperationTest.java b/vespa-hadoop/src/test/java/com/yahoo/vespa/hadoop/pig/VespaDocumentOperationTest.java index 67003273cac..e278026b00d 100644 --- a/vespa-hadoop/src/test/java/com/yahoo/vespa/hadoop/pig/VespaDocumentOperationTest.java +++ b/vespa-hadoop/src/test/java/com/yahoo/vespa/hadoop/pig/VespaDocumentOperationTest.java @@ -6,9 +6,14 @@ import org.apache.pig.impl.logicalLayer.FrontendException; import org.apache.pig.impl.logicalLayer.schema.Schema; import org.codehaus.jackson.JsonNode; import org.codehaus.jackson.map.ObjectMapper; +import org.hamcrest.CoreMatchers; +import org.junit.After; +import org.junit.Before; import org.junit.Test; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.PrintStream; import java.util.HashMap; import java.util.Iterator; import java.util.Map; @@ -16,9 +21,22 @@ import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; @SuppressWarnings("serial") public class VespaDocumentOperationTest { + private final ByteArrayOutputStream outContent = new ByteArrayOutputStream(); + private final PrintStream originalOut = System.out; + + @Before + public void setUpStreams() { + System.setOut(new PrintStream(outContent)); + } + + @After + public void restoreStreams() { + System.setOut(originalOut); + } @Test public void requireThatUDFReturnsCorrectJson() throws Exception { String json = getDocumentOperationJson("docid=id:<application>:metrics::<name>-<date>"); @@ -539,6 +557,59 @@ public class VespaDocumentOperationTest { assertEquals(234567, bagNode.get("234567").asInt()); } + @Test + public void requireThatUDFPrintIdWhenVerbose() throws IOException { + DataBag bag = BagFactory.getInstance().newDefaultBag(); + + Schema objectSchema = new Schema(); + Tuple objectTuple = TupleFactory.getInstance().newTuple(); + addToTuple("key", DataType.CHARARRAY, "123456", objectSchema, objectTuple); + addToTuple("value", DataType.INTEGER, 123456, objectSchema, objectTuple); + bag.add(objectTuple); + + objectSchema = new Schema(); + objectTuple = TupleFactory.getInstance().newTuple(); + addToTuple("key", DataType.CHARARRAY, "234567", objectSchema, objectTuple); + addToTuple("value", DataType.INTEGER, 234567, objectSchema, objectTuple); + bag.add(objectTuple); + + Schema schema = new Schema(); + Tuple tuple = TupleFactory.getInstance().newTuple(); + addToTuple("bag", DataType.BAG, bag, objectSchema, schema, tuple); + + VespaDocumentOperation docOp = new VespaDocumentOperation("docid=7654321", "bag-as-map-fields=bag","verbose=true"); + docOp.setInputSchema(schema); + String json = docOp.exec(tuple); + + assertThat(outContent.toString(), CoreMatchers.containsString("Processing docId: 7654321")); + } + + @Test + public void requireThatUDFVerboseSetToFalseByDefault() throws IOException { + DataBag bag = BagFactory.getInstance().newDefaultBag(); + + Schema objectSchema = new Schema(); + Tuple objectTuple = TupleFactory.getInstance().newTuple(); + addToTuple("key", DataType.CHARARRAY, "123456", objectSchema, objectTuple); + addToTuple("value", DataType.INTEGER, 123456, objectSchema, objectTuple); + bag.add(objectTuple); + + objectSchema = new Schema(); + objectTuple = TupleFactory.getInstance().newTuple(); + addToTuple("key", DataType.CHARARRAY, "234567", objectSchema, objectTuple); + addToTuple("value", DataType.INTEGER, 234567, objectSchema, objectTuple); + bag.add(objectTuple); + + Schema schema = new Schema(); + Tuple tuple = TupleFactory.getInstance().newTuple(); + addToTuple("bag", DataType.BAG, bag, objectSchema, schema, tuple); + + VespaDocumentOperation docOp = new VespaDocumentOperation("docid=7654321", "bag-as-map-fields=bag"); + docOp.setInputSchema(schema); + String json = docOp.exec(tuple); + + assertEquals("", outContent.toString()); + } private void addToTuple(String alias, byte type, Object value, Schema schema, Tuple tuple) { schema.add(new Schema.FieldSchema(alias, type)); diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/FeedClient.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/FeedClient.java index 8e820a89eb1..8cd22fe2e5c 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/FeedClient.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/FeedClient.java @@ -30,7 +30,7 @@ public interface FeedClient extends AutoCloseable { * produced faster than the can be handled. Transient failured are retried internally by this client. * Exactly one callback will always be received for each (completed) call to this. * - * @param documentId the document id of the document. + * @param documentId the document id of the document * @param documentData the document data as JSON or XML (as specified when using the factory to create the API) */ default void stream(String documentId, CharSequence documentData) { @@ -43,7 +43,7 @@ public interface FeedClient extends AutoCloseable { * produced faster than the can be handled. Transient failured are retried internally by this client. * Exactly one callback will always be received for each (completed) call to this. * - * @param documentId the document id of the document. + * @param documentId the document id of the document * @param documentData the document data as JSON or XML (as specified when using the factory to create the API) * @param context a context object which will be accessible in the result of the callback, or null if none */ @@ -57,7 +57,7 @@ public interface FeedClient extends AutoCloseable { * produced faster than the can be handled. Transient failured are retried internally by this client. * Exactly one callback will always be received for each (completed) call to this. * - * @param documentId the document id of the document. + * @param documentId the document id of the document * @param operationId the id to use for this operation, or null to let the client decide an operation id. * This id must be unique for every operation. Passing the operation id allows clients * to prepare to receive a response for it before issuing the operation to the client. @@ -74,9 +74,9 @@ public interface FeedClient extends AutoCloseable { void close(); /** - * Returns stats about the cluster. + * Returns stats about the cluster * - * @return JSON string with information about cluster. + * @return JSON string with information about cluster */ String getStatsAsJson(); @@ -132,7 +132,7 @@ public interface FeedClient extends AutoCloseable { * Document specific errors will be reported back through {@link #onCompletion(String, Result)}. * * @see FeedEndpointException - * @param exception An exception specifying endpoint and cause. See {@link FeedEndpointException} for details. + * @param exception an exception specifying endpoint and cause. See {@link FeedEndpointException} for details. */ default void onEndpointException(FeedEndpointException exception) {} diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/Result.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/Result.java index d4f3732fcd7..473b9494ba4 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/Result.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/Result.java @@ -34,11 +34,7 @@ public class Result { public Result(Document document, Collection<Detail> values, StringBuilder localTrace) { this.document = document; this.details = Collections.unmodifiableList(new ArrayList<>(values)); - boolean totalSuccess = true; - for (Detail d : details) { - if (d.getResultType() != ResultType.OPERATION_EXECUTED) {totalSuccess = false; } - } - this.success = totalSuccess; + this.success = details.stream().allMatch(d -> d.getResultType() == ResultType.OPERATION_EXECUTED); this.localTrace = localTrace == null ? null : localTrace.toString(); } @@ -70,18 +66,12 @@ public class Result { public List<Detail> getDetails() { return details; } - /** - * Checks if operation has been set up with local tracing. - * - * @return true if operation has local trace. - */ + /** Returns whether the operation has been set up with local tracing */ public boolean hasLocalTrace() { return localTrace != null; } - /** - * Information in a Result for a single operation sent to a single endpoint. - */ + /** Information in a Result for a single operation sent to a single endpoint. */ public static final class Detail { private final ResultType resultType; @@ -105,43 +95,29 @@ public class Result { } /** - * Returns the endpoint from which the result was received. - * - * @return the endpoint from which the result was received. + * Returns the endpoint from which the result was received, + * or null if this failed before being assigned an endpoint */ public Endpoint getEndpoint() { return endpoint; } - /** - * Check if operation was successful. - * - * @return true if the operation was successful. - */ + /** Returns whether the operation was successful */ public boolean isSuccess() { return resultType == ResultType.OPERATION_EXECUTED; } - /** - * Returns the result of the operation. - */ + /** Returns the result of the operation */ public ResultType getResultType() { return resultType; } - /** - * Returns any exception related to this Detail, if unsuccessful. Might be null. - * - * @return any exception related to this Detail, if unsuccessful. Might be null. - */ + /** Returns any exception related to this Detail, if unsuccessful. Might be null. */ public Exception getException() { return exception; } - /** - * Returns trace message if any from gateway. - * @return null or trace message. - */ + /** Returns any trace message produces, or null if none */ public String getTraceMessage() { return traceMessage; } @@ -151,16 +127,16 @@ public class Result { StringBuilder b = new StringBuilder(); b.append("Detail "); b.append("resultType=").append(resultType); - if (exception != null) { + if (exception != null) b.append(" exception='").append(Exceptions.toMessageString(exception)).append("'"); - } - if (traceMessage != null && ! traceMessage.isEmpty()) { + if (traceMessage != null && ! traceMessage.isEmpty()) b.append(" trace='").append(traceMessage).append("'"); - } - b.append(" endpoint=").append(endpoint); + if (endpoint != null) + b.append(" endpoint=").append(endpoint); b.append(" resultTimeLocally=").append(timeStampMillis).append("\n"); return b.toString(); } + } @Override diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/Cluster.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/Cluster.java index 4707cdb705c..fccb79c77b4 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/Cluster.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/Cluster.java @@ -13,9 +13,7 @@ import java.util.List; */ public final class Cluster { - /** - * Builder for {@link Cluster}. - */ + /** Builder for {@link Cluster}. */ public final static class Builder { private final List<Endpoint> endpoints = new LinkedList<>(); private String route = null; @@ -23,8 +21,8 @@ public final class Cluster { /** * Adds an Endpoint (a HTTP gateway) to this Cluster. * - * @param endpoint the Endpoint to add. - * @return this, for chaining. + * @param endpoint the Endpoint to add + * @return this, for chaining */ public Builder addEndpoint(Endpoint endpoint) { endpoints.add(endpoint); @@ -34,8 +32,8 @@ public final class Cluster { /** * Sets a route specific to this cluster, which overrides the route set in {@link com.yahoo.vespa.http.client.config.FeedParams#getRoute()}. * - * @param route a route specific to this cluster. - * @return this, for chaining. + * @param route a route specific to this cluster + * @return this, for chaining */ public Builder setRoute(String route) { this.route = route; @@ -68,7 +66,7 @@ public final class Cluster { @Override public String toString() { - return "Cluster " + endpoints + ", route " + route; + return "cluster with endpoints " + endpoints + " and route '" + route + "'"; } } diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/Encoder.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/Encoder.java index 135b2021a16..7bb65827b51 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/Encoder.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/Encoder.java @@ -15,10 +15,8 @@ public final class Encoder { /** * ISO 646.irv:1991 safe quoting into a StringBuilder instance. * - * @param input - * the string to encode - * @param output - * the destination buffer + * @param input the string to encode + * @param output the destination buffer * @return the destination buffer given as input */ public static StringBuilder encode(String input, StringBuilder output) { @@ -47,28 +45,20 @@ public final class Encoder { /** * ISO 646.irv:1991 safe unquoting into a StringBuilder instance. * - * @param input - * the string to decode - * @param output - * the destination buffer + * @param input the string to decode + * @param output the destination buffer * @return the destination buffer given as input - * @throws IllegalArgumentException - * if the input string contains unexpected or invalid data + * @throws IllegalArgumentException if the input string contains unexpected or invalid data */ public static StringBuilder decode(String input, StringBuilder output) { for (int i = 0; i < input.length(); i = input.offsetByCodePoints(i, 1)) { int c = input.codePointAt(i); - if (c > '~') { - throw new IllegalArgumentException("Input contained character above printable ASCII."); - } - switch (c) { - case '{': - i = decode(input, i, output); - break; - default: - output.append((char) c); - break; - } + if (c > '~') + throw new IllegalArgumentException("Input contained character above printable ASCII at position " + i); + if (c == '{') + i = decode(input, i, output); + else + output.append((char) c); } return output; } diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java index f59d4a4bbba..d510ce4b7ea 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java @@ -149,16 +149,12 @@ class ApacheGatewayConnection implements GatewayConnection { private InputStream write(List<Document> docs, boolean drain, boolean useCompression) throws ServerResponseException, IOException { - HttpPost httpPost = createPost(drain, useCompression, false /* this is not hanshake */); + HttpPost httpPost = createPost(drain, useCompression, false); - final ByteBuffer[] buffers = getDataWithStartAndEndOfFeed(docs, negotiatedVersion); - final InputStream inputStream = new ByteBufferInputStream(buffers); - final InputStreamEntity reqEntity; - if (useCompression ) { - reqEntity = zipAndCreateEntity(inputStream); - } else { - reqEntity = new InputStreamEntity(inputStream, -1); - } + ByteBuffer[] buffers = getDataWithStartAndEndOfFeed(docs, negotiatedVersion); + InputStream inputStream = new ByteBufferInputStream(buffers); + InputStreamEntity reqEntity = useCompression ? zipAndCreateEntity(inputStream) + : new InputStreamEntity(inputStream, -1); reqEntity.setChunked(true); httpPost.setEntity(reqEntity); return executePost(httpPost); diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ClusterConnection.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ClusterConnection.java index 6e1f3419e8e..98755320d74 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ClusterConnection.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ClusterConnection.java @@ -24,10 +24,18 @@ import java.util.concurrent.TimeUnit; */ public class ClusterConnection implements AutoCloseable { + private static final JsonFactory jsonFactory = new JsonFactory(); + private static final ObjectMapper objectMapper = new ObjectMapper(); + private final List<IOThread> ioThreads = new ArrayList<>(); private final int clusterId; - private static JsonFactory jsonFactory = new JsonFactory(); - private static ObjectMapper objectMapper = new ObjectMapper(); + private final ThreadGroup ioThreadGroup; + + /** The shared queue of document operations the io threads will take from */ + private final DocumentQueue documentQueue; + + /** The single endpoint this sends to, or null if it will send to multiple endpoints */ + private final Endpoint singleEndpoint; public ClusterConnection(OperationProcessor operationProcessor, FeedParams feedParams, @@ -37,16 +45,17 @@ public class ClusterConnection implements AutoCloseable { int clientQueueSizePerCluster, ScheduledThreadPoolExecutor timeoutExecutor) { if (cluster.getEndpoints().isEmpty()) - throw new IllegalArgumentException("Cannot feed to empty cluster."); + throw new IllegalArgumentException("At least a single endpoint is required in " + cluster); this.clusterId = clusterId; int totalNumberOfEndpointsInThisCluster = cluster.getEndpoints().size() * connectionParams.getNumPersistentConnectionsPerEndpoint(); - if (totalNumberOfEndpointsInThisCluster == 0) return; - - // Lower than 1 does not make any sense. + if (totalNumberOfEndpointsInThisCluster == 0) + throw new IllegalArgumentException("At least 1 persistent connection per endpoint is required in " + cluster); int maxInFlightPerSession = Math.max(1, feedParams.getMaxInFlightRequests() / totalNumberOfEndpointsInThisCluster); - DocumentQueue documentQueue = null; + documentQueue = new DocumentQueue(clientQueueSizePerCluster); + ioThreadGroup = operationProcessor.getIoThreadGroup(); + singleEndpoint = cluster.getEndpoints().size() == 1 ? cluster.getEndpoints().get(0) : null; for (Endpoint endpoint : cluster.getEndpoints()) { EndpointResultQueue endpointResultQueue = new EndpointResultQueue(operationProcessor, endpoint, @@ -58,18 +67,14 @@ public class ClusterConnection implements AutoCloseable { if (connectionParams.isDryRun()) { gatewayConnection = new DryRunGatewayConnection(endpoint); } else { - gatewayConnection = new ApacheGatewayConnection( - endpoint, - feedParams, - cluster.getRoute(), - connectionParams, - new ApacheGatewayConnection.HttpClientFactory(connectionParams, endpoint.isUseSsl()), - operationProcessor.getClientId() + gatewayConnection = new ApacheGatewayConnection(endpoint, + feedParams, + cluster.getRoute(), + connectionParams, + new ApacheGatewayConnection.HttpClientFactory(connectionParams, endpoint.isUseSsl()), + operationProcessor.getClientId() ); } - if (documentQueue == null) { - documentQueue = new DocumentQueue(clientQueueSizePerCluster); - } IOThread ioThread = new IOThread(operationProcessor.getIoThreadGroup(), endpointResultQueue, gatewayConnection, @@ -89,15 +94,10 @@ public class ClusterConnection implements AutoCloseable { } public void post(Document document) throws EndpointIOException { - String documentIdStr = document.getDocumentId(); - // The same document ID must always go to the same destination - // In noHandshakeMode this has no effect as the documentQueue is shared between the IOThreads. - int hash = documentIdStr.hashCode() & 0x7FFFFFFF; // Strip sign bit - IOThread ioThread = ioThreads.get(hash % ioThreads.size()); try { - ioThread.post(document); - } catch (Throwable t) { - throw new EndpointIOException(ioThread.getEndpoint(), "While sending", t); + documentQueue.put(document, Thread.currentThread().getThreadGroup() == ioThreadGroup); + } catch (Throwable t) { // InterruptedException if shutting down, IllegalStateException if already shut down + throw new EndpointIOException(singleEndpoint, "While sending", t); } } @@ -165,4 +165,5 @@ public class ClusterConnection implements AutoCloseable { public int hashCode() { return clusterId; } + } diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/EndpointIOException.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/EndpointIOException.java index ae15f6ec22b..d8efeea93bc 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/EndpointIOException.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/EndpointIOException.java @@ -20,8 +20,7 @@ public class EndpointIOException extends IOException { this.endpoint = endpoint; } - public Endpoint getEndpoint() { - return endpoint; - } + /** Returns the endpoint, or null if the failure occurred before this was assigned to a unique endpoint */ + public Endpoint getEndpoint() { return endpoint; } } diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/IOThread.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/IOThread.java index 77ed8464284..44799e598b0 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/IOThread.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/IOThread.java @@ -17,6 +17,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Optional; +import java.util.Random; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -36,7 +37,6 @@ class IOThread implements Runnable, AutoCloseable { private final DocumentQueue documentQueue; private final EndpointResultQueue resultQueue; private final Thread thread; - private final ThreadGroup ioThreadGroup; private final int clusterId; private final CountDownLatch running = new CountDownLatch(1); private final CountDownLatch stopSignal = new CountDownLatch(1); @@ -44,6 +44,7 @@ class IOThread implements Runnable, AutoCloseable { private final int maxInFlightRequests; private final long localQueueTimeOut; private final GatewayThrottler gatewayThrottler; + private final Random random = new Random(); private enum ThreadState { DISCONNECTED, CONNECTED, SESSION_SYNCED }; private final AtomicInteger wrongSessionDetectedCounter = new AtomicInteger(0); @@ -74,7 +75,6 @@ class IOThread implements Runnable, AutoCloseable { this.maxInFlightRequests = maxInFlightRequests; this.gatewayThrottler = new GatewayThrottler(maxSleepTimeMs); this.thread = new Thread(ioThreadGroup, this, "IOThread " + endpoint); - this.ioThreadGroup = ioThreadGroup; thread.setDaemon(true); this.localQueueTimeOut = localQueueTimeOut; thread.start(); @@ -165,8 +165,9 @@ class IOThread implements Runnable, AutoCloseable { log.fine("Session to " + endpoint + " closed."); } + /** For testing only */ public void post(Document document) throws InterruptedException { - documentQueue.put(document, Thread.currentThread().getThreadGroup() == ioThreadGroup); + documentQueue.put(document, true); } @Override @@ -174,7 +175,6 @@ class IOThread implements Runnable, AutoCloseable { return "I/O thread (for " + endpoint + ")"; } - List<Document> getNextDocsForFeeding(long maxWaitUnits, TimeUnit timeUnit) { List<Document> docsForSendChunk = new ArrayList<>(); int chunkSizeBytes = 0; @@ -186,26 +186,35 @@ class IOThread implements Runnable, AutoCloseable { chunkSizeBytes = doc.size(); } } catch (InterruptedException ie) { - log.fine("Got break signal while waiting for new documents to feed."); + log.fine("Got break signal while waiting for new documents to feed"); return docsForSendChunk; } int pendingSize = 1 + resultQueue.getPendingSize(); + // see if we can get more documents without blocking - while (chunkSizeBytes < maxChunkSizeBytes && pendingSize < maxInFlightRequests) { + // slightly randomize how much is taken to avoid harmonic interactions leading + // to some threads consistently taking more than others + int thisMaxChunkSizeBytes = randomize(maxChunkSizeBytes); + int thisMaxInFlightRequests = randomize(maxInFlightRequests); + while (chunkSizeBytes < thisMaxChunkSizeBytes && pendingSize < thisMaxInFlightRequests) { drainFirstDocumentsInQueueIfOld(); - Document d = documentQueue.poll(); - if (d == null) { - break; - } - docsForSendChunk.add(d); - chunkSizeBytes += d.size(); + Document document = documentQueue.poll(); + if (document == null) break; + docsForSendChunk.add(document); + chunkSizeBytes += document.size(); pendingSize++; } - log.finest("Chunk has " + docsForSendChunk.size() + " docs with a size " + chunkSizeBytes + " bytes."); + if (log.isLoggable(Level.FINEST)) + log.finest("Chunk has " + docsForSendChunk.size() + " docs with a size " + chunkSizeBytes + " bytes"); docsReceivedCounter.addAndGet(docsForSendChunk.size()); return docsForSendChunk; } + private int randomize(int limit) { + double multiplier = 0.75 + 0.25 * random.nextDouble(); + return Math.max(1, (int)(limit * multiplier)); + } + private void addDocumentsToResultQueue(List<Document> docs) { for (Document doc : docs) { resultQueue.operationSent(doc.getOperationId()); @@ -273,10 +282,9 @@ class IOThread implements Runnable, AutoCloseable { int pendingResultQueueSize = resultQueue.getPendingSize(); pendingDocumentStatusCount.set(pendingResultQueueSize); - List<Document> nextDocsForFeeding = - (pendingResultQueueSize > maxInFlightRequests) - ? new ArrayList<>() // The queue is full, will not send more documents. - : getNextDocsForFeeding(maxWaitTimeMs, TimeUnit.MILLISECONDS); + List<Document> nextDocsForFeeding = (pendingResultQueueSize > maxInFlightRequests) + ? new ArrayList<>() // The queue is full, will not send more documents + : getNextDocsForFeeding(maxWaitTimeMs, TimeUnit.MILLISECONDS); if (nextDocsForFeeding.isEmpty() && pendingResultQueueSize == 0) { //we have no unfinished business with the server now. @@ -288,8 +296,7 @@ class IOThread implements Runnable, AutoCloseable { if (pendingResultQueueSize > maxInFlightRequests && processResponse.processResultsCount == 0) { try { - // Max outstanding document operations, no more results on server side, wait a bit - // before asking again. + // Max outstanding document operations, no more results on server side, wait a bit before asking again Thread.sleep(300); } catch (InterruptedException e) { // Ignore diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/operationProcessor/EndPointResultFactory.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/operationProcessor/EndPointResultFactory.java index 95df465c7ca..205153a7a00 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/operationProcessor/EndPointResultFactory.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/operationProcessor/EndPointResultFactory.java @@ -38,30 +38,28 @@ public final class EndPointResultFactory { return results; } - public static EndpointResult createError( - Endpoint endpoint, String operationId, Exception exception) { - return new EndpointResult(operationId, new Result.Detail( - endpoint, Result.ResultType.FATAL_ERROR, null, exception)); + public static EndpointResult createError(Endpoint endpoint, String operationId, Exception exception) { + return new EndpointResult(operationId, new Result.Detail(endpoint, + Result.ResultType.FATAL_ERROR, + null, + exception)); } - public static EndpointResult createTransientError( - Endpoint endpoint, String operationId, Exception exception) { - return new EndpointResult(operationId, new Result.Detail( - endpoint, Result.ResultType.TRANSITIVE_ERROR, null, exception)); + public static EndpointResult createTransientError(Endpoint endpoint, String operationId, Exception exception) { + return new EndpointResult(operationId, new Result.Detail(endpoint, + Result.ResultType.TRANSITIVE_ERROR, + null, + exception)); } private static Result.ResultType replyToResultType(OperationStatus reply) { - final Result.ResultType resultType; // The ordering below is important, e.g. if success, it is never a transient error even if isTransient is true. - if (reply.errorCode.isSuccess()) { + if (reply.errorCode.isSuccess()) return Result.ResultType.OPERATION_EXECUTED; - } - if (reply.isConditionNotMet) { + if (reply.isConditionNotMet) return Result.ResultType.CONDITION_NOT_MET; - } - if (reply.errorCode.isTransient()) { + if (reply.errorCode.isTransient()) return Result.ResultType.TRANSITIVE_ERROR; - } return Result.ResultType.FATAL_ERROR; } diff --git a/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/AbstractVespaDeploymentMojo.java b/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/AbstractVespaDeploymentMojo.java index d9ac88c6ff4..f9c07d29ee1 100644 --- a/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/AbstractVespaDeploymentMojo.java +++ b/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/AbstractVespaDeploymentMojo.java @@ -12,6 +12,8 @@ import org.apache.maven.plugins.annotations.Parameter; */ public abstract class AbstractVespaDeploymentMojo extends AbstractVespaMojo { + protected ZoneId zone; + @Parameter(property = "environment") protected String environment; @@ -20,13 +22,21 @@ public abstract class AbstractVespaDeploymentMojo extends AbstractVespaMojo { protected ZoneId zoneOf(String environment, String region) { if (region == null) - return controller.defaultZone(environment != null ? Environment.from(environment) - : Environment.dev); + return zone = controller.defaultZone(environment != null ? Environment.from(environment) + : Environment.dev); if (environment == null) throw new IllegalArgumentException("Environment must be specified if region is specified"); - return ZoneId.from(environment, region); + return zone = ZoneId.from(environment, region); + } + + @Override + protected String name() { + return super.name() + "." + instance + " in " + + (zone != null ? zone.region() + " in " + zone.environment() + : (region == null ? "default region" : region) + " in " + + (environment == null ? "default environment (dev)" : environment)); } } diff --git a/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/AbstractVespaMojo.java b/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/AbstractVespaMojo.java index 8602d89c90c..0adb139f2c1 100644 --- a/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/AbstractVespaMojo.java +++ b/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/AbstractVespaMojo.java @@ -63,13 +63,16 @@ public abstract class AbstractVespaMojo extends AbstractMojo { throw e; } catch (Exception e) { - throw new MojoExecutionException("Execution failed for application '" + id + "':", e); + throw new MojoExecutionException("Execution failed for application " + name() + ":", e); } } /** Override this in subclasses, instead of {@link #execute()}. */ protected abstract void doExecute() throws Exception; + /** Return the name of the relevant entity, e.g., application with or without instance. */ + protected String name() { return tenant + "." + application; } + protected void setup() { tenant = firstNonBlank(tenant, project.getProperties().getProperty("tenant")); application = firstNonBlank(application, project.getProperties().getProperty("application")); diff --git a/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/EffectiveServicesMojo.java b/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/EffectiveServicesMojo.java index 91c7809dc76..ea536ffa6b3 100644 --- a/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/EffectiveServicesMojo.java +++ b/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/EffectiveServicesMojo.java @@ -2,6 +2,7 @@ package ai.vespa.hosted.plugin; import com.yahoo.config.application.XmlPreProcessor; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.zone.ZoneId; import org.apache.maven.plugins.annotations.Mojo; import org.apache.maven.plugins.annotations.Parameter; @@ -43,12 +44,17 @@ public class EffectiveServicesMojo extends AbstractVespaDeploymentMojo { ZoneId zone = zoneOf(environment, region); Path output = Paths.get(outputDirectory).resolve("services-" + zone.environment().value() + "-" + zone.region().value() + ".xml"); - Files.write(output, effectiveServices(services, zone).getBytes(StandardCharsets.UTF_8)); + Files.write(output, effectiveServices(services, zone, InstanceName.from(instance)).getBytes(StandardCharsets.UTF_8)); getLog().info("Effective services for " + zone + " written to " + output); } - static String effectiveServices(File servicesFile, ZoneId zone) throws Exception { - Document processedServicesXml = new XmlPreProcessor(servicesFile.getParentFile(), servicesFile, zone.environment(), zone.region()).run(); + static String effectiveServices(File servicesFile, ZoneId zone, InstanceName instance) throws Exception { + Document processedServicesXml = new XmlPreProcessor(servicesFile.getParentFile(), + servicesFile, + instance, + zone.environment(), + zone.region()) + .run(); Transformer transformer = TransformerFactory.newInstance().newTransformer(); transformer.setOutputProperty(OutputKeys.INDENT, "yes"); Writer writer = new StringWriter(); diff --git a/vespa-maven-plugin/src/test/java/ai/vespa/hosted/plugin/EffectiveServicesMojoTest.java b/vespa-maven-plugin/src/test/java/ai/vespa/hosted/plugin/EffectiveServicesMojoTest.java index 35f3425706d..5b472574efd 100644 --- a/vespa-maven-plugin/src/test/java/ai/vespa/hosted/plugin/EffectiveServicesMojoTest.java +++ b/vespa-maven-plugin/src/test/java/ai/vespa/hosted/plugin/EffectiveServicesMojoTest.java @@ -1,6 +1,7 @@ // Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.hosted.plugin; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.zone.ZoneId; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -9,6 +10,7 @@ import java.io.File; import java.nio.file.Files; import java.nio.file.Paths; +import static ai.vespa.hosted.plugin.EffectiveServicesMojo.effectiveServices; import static org.junit.jupiter.api.Assertions.assertEquals; /** @@ -24,21 +26,21 @@ class EffectiveServicesMojoTest { @DisplayName("when zone matches environment-only directive") void devServices() throws Exception { assertEquals(Files.readString(Paths.get("src/test/resources/effective-services/dev.xml")), - EffectiveServicesMojo.effectiveServices(servicesFile, ZoneId.from("dev", "us-east-3"))); + effectiveServices(servicesFile, ZoneId.from("dev", "us-east-3"), InstanceName.defaultName())); } @Test @DisplayName("when zone matches region-and-environment directive") void prodUsEast3() throws Exception { assertEquals(Files.readString(Paths.get("src/test/resources/effective-services/prod_us-east-3.xml")), - EffectiveServicesMojo.effectiveServices(servicesFile, ZoneId.from("prod", "us-east-3"))); + effectiveServices(servicesFile, ZoneId.from("prod", "us-east-3"), InstanceName.defaultName())); } @Test @DisplayName("when zone doesn't match any directives") void prodUsWest1Services() throws Exception { assertEquals(Files.readString(Paths.get("src/test/resources/effective-services/prod_us-west-1.xml")), - EffectiveServicesMojo.effectiveServices(servicesFile, ZoneId.from("prod", "us-west-1"))); + effectiveServices(servicesFile, ZoneId.from("prod", "us-west-1"), InstanceName.defaultName())); } } diff --git a/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/SimpleFeeder.java b/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/SimpleFeeder.java index 14123c7a73c..aac7ab750bd 100644 --- a/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/SimpleFeeder.java +++ b/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/SimpleFeeder.java @@ -46,9 +46,10 @@ import java.io.PrintStream; import java.nio.ByteBuffer; import java.util.Arrays; import java.util.List; +import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; -import java.util.concurrent.SynchronousQueue; +import java.util.concurrent.RejectedExecutionHandler; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; @@ -154,7 +155,7 @@ public class SimpleFeeder implements ReplyHandler { } } catch (InterruptedException e) {} } - public void close() throws Exception { + public void close() { session.destroy(); } } @@ -277,7 +278,7 @@ public class SimpleFeeder implements ReplyHandler { } } - class LazyDocumentOperation extends ConditionalFeedOperation { + static class LazyDocumentOperation extends ConditionalFeedOperation { private final DocumentDeserializer deserializer; LazyDocumentOperation(DocumentDeserializer deserializer, TestAndSetCondition condition) { super(Type.DOCUMENT, condition); @@ -289,7 +290,7 @@ public class SimpleFeeder implements ReplyHandler { return new Document(deserializer); } } - class LazyUpdateOperation extends ConditionalFeedOperation { + static class LazyUpdateOperation extends ConditionalFeedOperation { private final DocumentDeserializer deserializer; LazyUpdateOperation(DocumentDeserializer deserializer, TestAndSetCondition condition) { super(Type.UPDATE, condition); @@ -377,13 +378,22 @@ public class SimpleFeeder implements ReplyHandler { } + static class RetryExecutionhandler implements RejectedExecutionHandler { + + @Override + public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) { + try { + executor.getQueue().put(r); + } catch (InterruptedException e) {} + } + } SimpleFeeder run() throws Throwable { ExecutorService executor = (numThreads > 1) ? new ThreadPoolExecutor(numThreads, numThreads, 0L, TimeUnit.SECONDS, - new SynchronousQueue<>(false), + new ArrayBlockingQueue<>(numThreads*100), ThreadFactoryFactory.getDaemonThreadFactory("perf-feeder"), - new ThreadPoolExecutor.CallerRunsPolicy()) + new RetryExecutionhandler()) : null; printHeader(out); long numMessagesSent = 0; diff --git a/vespajlib/abi-spec.json b/vespajlib/abi-spec.json index 04f859e2802..d9467a41f78 100644 --- a/vespajlib/abi-spec.json +++ b/vespajlib/abi-spec.json @@ -1538,7 +1538,9 @@ "public" ], "methods": [ + "public void <init>(com.yahoo.tensor.functions.TensorFunction)", "public void <init>(com.yahoo.tensor.functions.TensorFunction, java.lang.String)", + "public void <init>(com.yahoo.tensor.functions.TensorFunction, java.util.List)", "public java.util.List arguments()", "public com.yahoo.tensor.functions.TensorFunction withArguments(java.util.List)", "public com.yahoo.tensor.functions.PrimitiveTensorFunction toPrimitive()", @@ -1553,7 +1555,9 @@ "public" ], "methods": [ + "public void <init>(com.yahoo.tensor.functions.TensorFunction)", "public void <init>(com.yahoo.tensor.functions.TensorFunction, java.lang.String)", + "public void <init>(com.yahoo.tensor.functions.TensorFunction, java.util.List)", "public java.util.List arguments()", "public com.yahoo.tensor.functions.TensorFunction withArguments(java.util.List)", "public com.yahoo.tensor.functions.PrimitiveTensorFunction toPrimitive()", @@ -2859,7 +2863,8 @@ ], "methods": [ "public static java.lang.String encode(java.util.Map)", - "public static java.lang.String escape(java.lang.String)" + "public static java.lang.String escape(java.lang.String)", + "public static boolean equals(java.lang.String, java.lang.String)" ], "fields": [] }, diff --git a/vespajlib/src/main/java/com/yahoo/slime/JsonDecoder.java b/vespajlib/src/main/java/com/yahoo/slime/JsonDecoder.java index f199fefd185..f677ae23a45 100644 --- a/vespajlib/src/main/java/com/yahoo/slime/JsonDecoder.java +++ b/vespajlib/src/main/java/com/yahoo/slime/JsonDecoder.java @@ -3,10 +3,8 @@ package com.yahoo.slime; import com.yahoo.text.Text; import com.yahoo.text.Utf8; -import org.w3c.dom.CharacterData; import java.io.ByteArrayOutputStream; -import java.nio.charset.StandardCharsets; /** * A port of the C++ json decoder intended to be fast. @@ -47,6 +45,17 @@ public class JsonDecoder { return slime; } + /** Decode bytes as a UTF-8 JSON into Slime, or throw {@link JsonParseException} on invalid JSON. */ + public Slime decodeOrThrow(Slime slime, byte[] bytes) { + in = new BufferedInput(bytes); + next(); + decodeValue(slimeInserter.adjust(slime)); + if (in.failed()) { + throw new JsonParseException(in); + } + return slime; + } + private void decodeValue(Inserter inserter) { skipWhiteSpace(); switch (c) { diff --git a/vespajlib/src/main/java/com/yahoo/slime/JsonParseException.java b/vespajlib/src/main/java/com/yahoo/slime/JsonParseException.java new file mode 100644 index 00000000000..6c42f7d38c1 --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/slime/JsonParseException.java @@ -0,0 +1,23 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.slime; + +/** + * @author hakonhall + */ +public class JsonParseException extends RuntimeException { + + private static final long serialVersionUID = 1586949558L; + + private final BufferedInput input; + + JsonParseException(BufferedInput input) { + super(input.getErrorMessage()); + this.input = input; + } + + public byte[] getOffendingBytes() { + // potentially expensive array copy + return input.getOffending(); + } + +} diff --git a/vespajlib/src/main/java/com/yahoo/slime/Slime.java b/vespajlib/src/main/java/com/yahoo/slime/Slime.java index 8357e3035c0..83934e0c206 100644 --- a/vespajlib/src/main/java/com/yahoo/slime/Slime.java +++ b/vespajlib/src/main/java/com/yahoo/slime/Slime.java @@ -159,11 +159,10 @@ public final class Slime { } /** - * Tests whether this is equal to Inspector. + * Tests whether the two Inspectors are equal. * - * Since equality of two Inspectors is subtle, {@link Object#equals(Object)} is not used. + * <p>Since equality of two Inspectors is subtle, {@link Object#equals(Object)} is not used.</p> * - * @param that inspector. * @return true if they are equal. */ public boolean equalTo(Slime that) { diff --git a/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java b/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java index 4d0b3a8ff2e..2ed7331a60c 100644 --- a/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java +++ b/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java @@ -104,6 +104,17 @@ public class SlimeUtils { return jsonToSlime(json.getBytes(StandardCharsets.UTF_8)); } + /** Throws {@link JsonParseException} on invalid JSON. */ + public static Slime jsonToSlimeOrThrow(String json) { + return jsonToSlimeOrThrow(json.getBytes(StandardCharsets.UTF_8)); + } + + public static Slime jsonToSlimeOrThrow(byte[] json) { + Slime slime = new Slime(); + new JsonDecoder().decodeOrThrow(slime, json); + return slime; + } + public static Optional<String> optionalString(Inspector inspector) { return Optional.of(inspector.asString()).filter(s -> !s.isEmpty()); } diff --git a/vespajlib/src/main/java/com/yahoo/tensor/functions/Argmax.java b/vespajlib/src/main/java/com/yahoo/tensor/functions/Argmax.java index a365f0f4bdc..a4b68a662da 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/functions/Argmax.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/functions/Argmax.java @@ -1,10 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.tensor.functions; +import com.google.common.collect.ImmutableList; import com.yahoo.tensor.evaluation.Name; import java.util.Collections; import java.util.List; +import java.util.Objects; /** * @author bratseth @@ -12,11 +14,20 @@ import java.util.List; public class Argmax<NAMETYPE extends Name> extends CompositeTensorFunction<NAMETYPE> { private final TensorFunction<NAMETYPE> argument; - private final String dimension; + private final List<String> dimensions; + + public Argmax(TensorFunction<NAMETYPE> argument) { + this(argument, Collections.emptyList()); + } public Argmax(TensorFunction<NAMETYPE> argument, String dimension) { + this(argument, Collections.singletonList(dimension)); + } + + public Argmax(TensorFunction<NAMETYPE> argument, List<String> dimensions) { + Objects.requireNonNull(dimensions, "The dimensions cannot be null"); this.argument = argument; - this.dimension = dimension; + this.dimensions = ImmutableList.copyOf(dimensions); } @Override @@ -24,22 +35,21 @@ public class Argmax<NAMETYPE extends Name> extends CompositeTensorFunction<NAMET @Override public TensorFunction<NAMETYPE> withArguments(List<TensorFunction<NAMETYPE>> arguments) { - if ( arguments.size() != 1) + if (arguments.size() != 1) throw new IllegalArgumentException("Argmax must have 1 argument, got " + arguments.size()); - return new Argmax<>(arguments.get(0), dimension); + return new Argmax<>(arguments.get(0), dimensions); } @Override public PrimitiveTensorFunction<NAMETYPE> toPrimitive() { TensorFunction<NAMETYPE> primitiveArgument = argument.toPrimitive(); - return new Join<>(primitiveArgument, - new Reduce<>(primitiveArgument, Reduce.Aggregator.max, dimension), - ScalarFunctions.equal()); + TensorFunction<NAMETYPE> reduce = new Reduce<>(primitiveArgument, Reduce.Aggregator.max, dimensions); + return new Join<>(primitiveArgument, reduce, ScalarFunctions.equal()); } @Override public String toString(ToStringContext context) { - return "argmax(" + argument.toString(context) + ", " + dimension + ")"; + return "argmax(" + argument.toString(context) + Reduce.commaSeparated(dimensions) + ")"; } } diff --git a/vespajlib/src/main/java/com/yahoo/tensor/functions/Argmin.java b/vespajlib/src/main/java/com/yahoo/tensor/functions/Argmin.java index 32ccdf51336..ad14bc1f1f2 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/functions/Argmin.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/functions/Argmin.java @@ -1,10 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.tensor.functions; +import com.google.common.collect.ImmutableList; import com.yahoo.tensor.evaluation.Name; import java.util.Collections; import java.util.List; +import java.util.Objects; /** * @author bratseth @@ -12,11 +14,20 @@ import java.util.List; public class Argmin<NAMETYPE extends Name> extends CompositeTensorFunction<NAMETYPE> { private final TensorFunction<NAMETYPE> argument; - private final String dimension; + private final List<String> dimensions; + + public Argmin(TensorFunction<NAMETYPE> argument) { + this(argument, Collections.emptyList()); + } public Argmin(TensorFunction<NAMETYPE> argument, String dimension) { + this(argument, Collections.singletonList(dimension)); + } + + public Argmin(TensorFunction<NAMETYPE> argument, List<String> dimensions) { + Objects.requireNonNull(dimensions, "The dimensions cannot be null"); this.argument = argument; - this.dimension = dimension; + this.dimensions = ImmutableList.copyOf(dimensions); } @Override @@ -24,22 +35,21 @@ public class Argmin<NAMETYPE extends Name> extends CompositeTensorFunction<NAMET @Override public TensorFunction<NAMETYPE> withArguments(List<TensorFunction<NAMETYPE>> arguments) { - if ( arguments.size() != 1) + if (arguments.size() != 1) throw new IllegalArgumentException("Argmin must have 1 argument, got " + arguments.size()); - return new Argmin<>(arguments.get(0), dimension); + return new Argmin<>(arguments.get(0), dimensions); } @Override public PrimitiveTensorFunction<NAMETYPE> toPrimitive() { TensorFunction<NAMETYPE> primitiveArgument = argument.toPrimitive(); - return new Join<>(primitiveArgument, - new Reduce<>(primitiveArgument, Reduce.Aggregator.min, dimension), - ScalarFunctions.equal()); + TensorFunction<NAMETYPE> reduce = new Reduce<>(primitiveArgument, Reduce.Aggregator.min, dimensions); + return new Join<>(primitiveArgument, reduce, ScalarFunctions.equal()); } @Override public String toString(ToStringContext context) { - return "argmin(" + argument.toString(context) + ", " + dimension + ")"; + return "argmin(" + argument.toString(context) + Reduce.commaSeparated(dimensions) + ")"; } } diff --git a/vespajlib/src/main/java/com/yahoo/text/JSON.java b/vespajlib/src/main/java/com/yahoo/text/JSON.java index cfff16c9aba..2757bd7945c 100644 --- a/vespajlib/src/main/java/com/yahoo/text/JSON.java +++ b/vespajlib/src/main/java/com/yahoo/text/JSON.java @@ -1,10 +1,13 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.text; +import com.yahoo.slime.Slime; +import com.yahoo.slime.SlimeUtils; + import java.util.Map; /** - * Static methods for working with the map textual format which is parsed by {@link MapParser} + * Static methods for working with JSON. * * @author bratseth */ @@ -56,4 +59,20 @@ public final class JSON { return b != null ? b.toString() : s; } + /** + * Test whether two JSON strings are equal, e.g. the order of fields in an object is irrelevant. + * + * <p>When comparing two numbers of the two JSON strings, the result is only guaranteed to be + * correct if (a) both are integers (without fraction and exponent) and each fits in a long, or + * (b) both are non-integers, fits in a double, and are syntactically identical. Examples + * of pairs that may not be equal: 1 and 1.0 (different types), 0.1 and 1e-1, 0.0 and 0.00.</p> + * + * @throws RuntimeException on invalid JSON + */ + public static boolean equals(String left, String right) { + Slime leftSlime = SlimeUtils.jsonToSlimeOrThrow(left); + Slime rightSlime = SlimeUtils.jsonToSlimeOrThrow(right); + return leftSlime.equalTo(rightSlime); + } + } diff --git a/vespajlib/src/test/java/com/yahoo/slime/SlimeUtilsTest.java b/vespajlib/src/test/java/com/yahoo/slime/SlimeUtilsTest.java index 8b13ee74aed..237b1575bfb 100644 --- a/vespajlib/src/test/java/com/yahoo/slime/SlimeUtilsTest.java +++ b/vespajlib/src/test/java/com/yahoo/slime/SlimeUtilsTest.java @@ -7,8 +7,10 @@ import org.junit.Test; import java.io.IOException; import static org.hamcrest.core.Is.is; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * @author Ulf Lilleengen @@ -78,4 +80,21 @@ public class SlimeUtilsTest { assertTrue(slime.get().field("bar").valid()); } + @Test + public void test_json_to_slime_or_throw() { + Slime slime = SlimeUtils.jsonToSlimeOrThrow("{\"foo\":\"foobie\",\"bar\":{}}"); + assertThat(slime.get().field("foo").asString(), is("foobie")); + assertTrue(slime.get().field("bar").valid()); + } + + @Test + public void test_invalid_json() { + try { + SlimeUtils.jsonToSlimeOrThrow("foo"); + fail(); + } catch (RuntimeException e) { + assertEquals("Unexpected character 'o'", e.getMessage()); + } + } + } diff --git a/vespajlib/src/test/java/com/yahoo/tensor/functions/TensorFunctionTestCase.java b/vespajlib/src/test/java/com/yahoo/tensor/functions/TensorFunctionTestCase.java index 625d5d44b19..05f7d27907c 100644 --- a/vespajlib/src/test/java/com/yahoo/tensor/functions/TensorFunctionTestCase.java +++ b/vespajlib/src/test/java/com/yahoo/tensor/functions/TensorFunctionTestCase.java @@ -21,6 +21,8 @@ public class TensorFunctionTestCase { new Diag<>(new TensorType.Builder().indexed("y",3).indexed("x",2).indexed("z",4).build())); assertTranslated("join(tensor(x{}):{1:1.0,3:5.0,9:3.0}, reduce(tensor(x{}):{1:1.0,3:5.0,9:3.0}, max, x), f(a,b)(a==b))", new Argmax<>(new ConstantTensor<>("{ {x:1}:1, {x:3}:5, {x:9}:3 }"), "x")); + assertTranslated("join(tensor(x{}):{1:1.0,3:5.0,9:3.0}, reduce(tensor(x{}):{1:1.0,3:5.0,9:3.0}, max), f(a,b)(a==b))", + new Argmax<>(new ConstantTensor<>("{ {x:1}:1, {x:3}:5, {x:9}:3 }"))); } private void assertTranslated(String expectedTranslation, TensorFunction<Name> inputFunction) { diff --git a/vespajlib/src/test/java/com/yahoo/text/JSONTest.java b/vespajlib/src/test/java/com/yahoo/text/JSONTest.java index 22174761571..fbd0f9d0403 100644 --- a/vespajlib/src/test/java/com/yahoo/text/JSONTest.java +++ b/vespajlib/src/test/java/com/yahoo/text/JSONTest.java @@ -2,11 +2,16 @@ package com.yahoo.text; import org.junit.Test; -import static org.junit.Assert.assertEquals; import java.util.LinkedHashMap; import java.util.Map; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + /** * @author bratseth */ @@ -22,4 +27,88 @@ public class JSONTest { assertEquals("{\"a \\\"key\\\"\":3,\"key2\":\"value\",\"key3\":3.3}", JSON.encode(map)); } + @Test + public void testEquals() { + assertTrue(JSON.equals("{}", "{}")); + + // Whitespace is irrelevant + assertTrue(JSON.equals("{}", "\n{ }")); + + // Order of fields in object is irrelevant + assertTrue(JSON.equals("{\"a\":0, \"c\":1}", "{\"c\":1, \"a\":0}")); + + // Object equality is not using subset + assertFalse(JSON.equals("{\"a\":0}", "{\"a\":0, \"b\":0}")); + assertFalse(JSON.equals("{\"a\":0, \"b\":0}", "{\"a\":0}")); + + // Order of elements of array is significant + assertFalse(JSON.equals("[\"a\",\"b\"]", "[\"b\",\"a\"]")); + + // Verify null-valued fields are not ignored + assertFalse(JSON.equals("{\"a\":null}", "{}")); + + // Current impl uses BigInteger if integer doesn't fit in a long. + assertEquals(9223372036854775807L, Long.MAX_VALUE); + assertTrue(JSON.equals("{\"a\": 9223372036854775807}", "{\"a\": 9223372036854775807}")); + + // double 1.0 and int 1 are different + assertTrue(JSON.equals("{\"a\": 1}", "{\"a\": 1}")); + assertTrue(JSON.equals("{\"a\": 1.0}", "{\"a\": 1.0}")); + assertFalse(JSON.equals("{\"a\": 1.0}", "{\"a\": 1}")); + + // Double-precision on numbers. Constant from Math.E. + assertTrue(JSON.equals("{\"e\": 2.71828182845904}", "{\"e\": 2.71828182845904}")); + + // Double.MAX_VALUE is 1.7976931348623157e+308 + assertTrue(JSON.equals("{\"e\": 1.7976931348623156e+308}", "{\"e\": 1.7976931348623156e+308}")); + + // Justification of above float values + double e1 = 2.7182818284590452354; + double e2 = 2.718281828459045; + double e3 = 2.71828182845904; + assertEquals(e1, Math.E, -1); + assertEquals(e1, e2, -1); + assertNotEquals(e1, e3, -1); + + // Invalid JSON throws RuntimeException + assertRuntimeException(() -> JSON.equals("", "{}")); + assertRuntimeException(() -> JSON.equals("{}", "")); + assertRuntimeException(() -> JSON.equals("{", "{}")); + assertRuntimeException(() -> JSON.equals("{}", "{")); + } + + @Test + public void implementationSpecificEqualsBehavior() { + // Exception thrown if outside a long + assertTrue( JSON.equals("{\"a\": 9223372036854775807}", "{\"a\": 9223372036854775807}")); + assertRuntimeException(() -> JSON.equals("{\"a\": 9223372036854775808}", "{\"a\": 9223372036854775808}")); + + // Infinity if floating point number outside of double, and hence equal + assertTrue(JSON.equals("{\"a\": 2.7976931348623158e+308}", "{\"a\": 2.7976931348623158e+308}")); + + // Ignores extraneous precision + assertTrue(JSON.equals( "{\"e\": 2.7182818284590452354}", + "{\"e\": 2.7182818284590452354}")); + assertTrue(JSON.equals( "{\"e\": 2.7182818284590452354}", + "{\"e\": 2.7182818284590452355}")); + assertFalse(JSON.equals("{\"e\": 2.7182818284590452354}", + "{\"e\": 2.71828182845904}")); + + // Comparing equal but syntactically different numbers + assertFalse(JSON.equals("{\"a\": 1.0}", "{\"a\":1}")); + assertTrue(JSON.equals("{\"a\": 1.0}", "{\"a\":1.00}")); + assertTrue(JSON.equals("{\"a\": 1.0}", "{\"a\":1.0000000000000000000000000000}")); + assertTrue(JSON.equals("{\"a\": 10.0}", "{\"a\":1e1}")); + assertTrue(JSON.equals("{\"a\": 1.2}", "{\"a\":12e-1}")); + } + + private static void assertRuntimeException(Runnable runnable) { + try { + runnable.run(); + fail("Expected RuntimeException to be thrown, but no exception was thrown"); + } catch (RuntimeException e) { + // OK + } + } + } diff --git a/vespalib/src/tests/alloc/alloc_test.cpp b/vespalib/src/tests/alloc/alloc_test.cpp index 4a569050f45..d46d2374dfc 100644 --- a/vespalib/src/tests/alloc/alloc_test.cpp +++ b/vespalib/src/tests/alloc/alloc_test.cpp @@ -155,7 +155,7 @@ void ensureRoomForExtension(const Alloc & buf, Alloc & reserved) { // So in order to verify this we first mmap a reserved area that we unmap // before we test extension. if (reserved.get() > buf.get()) { - EXPECT_EQUAL(reserved.get(), static_cast<const char *>(buf.get()) + buf.size()); + EXPECT_EQUAL(reserved.get(), static_cast<const void *>(static_cast<const char *>(buf.get()) + buf.size())); { Alloc().swap(reserved); } @@ -166,14 +166,15 @@ void verifyNoExtensionWhenNoRoom(Alloc & buf, Alloc & reserved, size_t sz) { if (reserved.get() > buf.get()) { // Normally mmapping starts at the top and grows down in address space. // Then there is no room to extend the last mapping. - EXPECT_EQUAL(reserved.get(), static_cast<const char *>(buf.get()) + buf.size()); + EXPECT_EQUAL(reserved.get(), static_cast<const void *>(static_cast<const char *>(buf.get()) + buf.size())); TEST_DO(verifyExtension(buf, sz, sz)); } else { - EXPECT_EQUAL(buf.get(), static_cast<const char *>(reserved.get()) + reserved.size()); + EXPECT_EQUAL(buf.get(), static_cast<const void *>(static_cast<const char *>(reserved.get()) + reserved.size())); TEST_DO(verifyExtension(reserved, sz, sz)); } } +#ifdef __linux__ TEST("auto alloced mmap alloc can be extended if room") { static constexpr size_t SZ = MemoryAllocator::HUGEPAGE_SIZE*2; Alloc reserved = Alloc::alloc(SZ); @@ -207,6 +208,7 @@ TEST("mmap alloc can not be extended if no room") { TEST_DO(verifyNoExtensionWhenNoRoom(buf, reserved, 4096)); } +#endif TEST("heap alloc can not be shrinked") { Alloc buf = Alloc::allocHeap(101); diff --git a/vespalog/src/vespa/log/log.cpp b/vespalog/src/vespa/log/log.cpp index 635d982f529..47f7377ac4d 100644 --- a/vespalog/src/vespa/log/log.cpp +++ b/vespalog/src/vespa/log/log.cpp @@ -39,6 +39,22 @@ char Logger::_hostname[1024] = { '\0'}; char Logger::_serviceName[1024] = {'\0' }; ControlFile *Logger::_controlFile = 0; +namespace { + +class GetTid { +public: + unsigned long operator()(const void *tid) const { + return reinterpret_cast<uint64_t>(tid) >> 3; + } + unsigned long operator()(unsigned long tid) const { + return tid; + } +}; + +GetTid gettid; + +} + void Logger::ensureControlName() { @@ -265,7 +281,7 @@ void Logger::doLogCore(uint64_t timestamp, LogLevel level, // threads, only showing the least significant bits will hopefully // distinguish between all threads in your application. Alter later if // found to be too inaccurate. - int32_t tid = (fakePid ? -1 : pthread_self() % 0xffff); + int32_t tid = (fakePid ? -1 : gettid(pthread_self()) % 0xffff); if (_target->makeHumanReadable()) { time_t secs = static_cast<time_t>(timestamp / 1000000); |