diff options
28 files changed, 255 insertions, 95 deletions
diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index ada3119f5bb..0c061dd8222 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -884,7 +884,8 @@ "public boolean useNewAthenzFilter()", "public boolean usePhraseSegmenting()", "public java.lang.String proxyProtocol()", - "public java.util.Optional athenzDomain()" + "public java.util.Optional athenzDomain()", + "public boolean useDedicatedNodesWhenUnspecified()" ], "fields": [] }, 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 7fcde1b5e6b..b9ada59cb0b 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 @@ -67,6 +67,7 @@ public interface ModelContext { default boolean usePhraseSegmenting() { return false; } default String proxyProtocol() { return "https-only"; } default Optional<AthenzDomain> athenzDomain() { return Optional.empty(); } + default boolean useDedicatedNodesWhenUnspecified() { return false; } } } 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 e49ceffabc1..802fdcc1dda 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 @@ -44,6 +44,7 @@ public class TestProperties implements ModelContext.Properties { private Optional<EndpointCertificateSecrets> endpointCertificateSecrets = Optional.empty(); private boolean useNewAthenzFilter = false; private boolean usePhraseSegmenting = false; + private boolean useDedicatedNodesWhenUnspecified = false; private AthenzDomain athenzDomain; @Override public boolean multitenant() { return multitenant; } @@ -66,6 +67,7 @@ public class TestProperties implements ModelContext.Properties { @Override public boolean useBucketSpaceMetric() { return true; } @Override public boolean useNewAthenzFilter() { return useNewAthenzFilter; } @Override public boolean usePhraseSegmenting() { return usePhraseSegmenting; } + @Override public boolean useDedicatedNodesWhenUnspecified() { return useDedicatedNodesWhenUnspecified; } @Override public Optional<AthenzDomain> athenzDomain() { return Optional.ofNullable(athenzDomain); } public TestProperties setDefaultTermwiseLimit(double limit) { @@ -123,6 +125,11 @@ public class TestProperties implements ModelContext.Properties { return this; } + public TestProperties setUseDedicatedNodesWhenUnspecified(boolean useDedicatedNodesWhenUnspecified) { + this.useDedicatedNodesWhenUnspecified = useDedicatedNodesWhenUnspecified; + return this; + } + public TestProperties setAthenzDomain(AthenzDomain domain) { this.athenzDomain = domain; return this; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index 165404ce9d4..1b2df3d47e3 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -570,12 +570,8 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { private void addNodesFromXml(ApplicationContainerCluster cluster, Element containerElement, ConfigModelContext context) { Element nodesElement = XML.getChild(containerElement, "nodes"); - if (nodesElement == null) { // default single node on localhost - ApplicationContainer node = new ApplicationContainer(cluster, "container.0", 0, cluster.isHostedVespa()); - HostResource host = allocateSingleNodeHost(cluster, log, containerElement, context); - node.setHostResource(host); - node.initService(context.getDeployLogger()); - cluster.addContainers(Collections.singleton(node)); + if (nodesElement == null) { + cluster.addContainers(allocateWithoutNodesTag(cluster, containerElement, context)); } else { List<ApplicationContainer> nodes = createNodes(cluster, nodesElement, context); @@ -652,29 +648,53 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { } } - /** Creates a single host when there is no nodes tag */ - private HostResource allocateSingleNodeHost(ApplicationContainerCluster cluster, DeployLogger logger, Element containerElement, ConfigModelContext context) { + /** Allocate a container cluster without a nodes tag */ + private List<ApplicationContainer> allocateWithoutNodesTag(ApplicationContainerCluster cluster, Element containerElement, ConfigModelContext context) { DeployState deployState = context.getDeployState(); HostSystem hostSystem = cluster.hostSystem(); if (deployState.isHosted()) { - Optional<HostResource> singleContentHost = getHostResourceFromContentClusters(cluster, containerElement, context); - if (singleContentHost.isPresent()) { // there is a content cluster; put the container on its first node - return singleContentHost.get(); + // TODO(mpolden): The old way of allocating. Remove when 7.198 is the oldest model in production and the + // feature flag is set to true in all zones. + if (!context.properties().useDedicatedNodesWhenUnspecified()) { + Optional<HostResource> singleContentHost = getHostResourceFromContentClusters(cluster, containerElement, context); + if (singleContentHost.isPresent()) { // there is a content cluster; put the container on its first node + return singleHostContainerCluster(cluster, singleContentHost.get(), context); + } + else { // request 1 node + ClusterSpec clusterSpec = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from(cluster.getName())) + .vespaVersion(deployState.getWantedNodeVespaVersion()) + .dockerImageRepo(deployState.getWantedDockerImageRepo()) + .build(); + Capacity capacity = Capacity.fromCount(1, + Optional.empty(), + false, + ! deployState.getProperties().isBootstrap()); + HostResource host = hostSystem.allocateHosts(clusterSpec, capacity, 1, log).keySet().iterator().next(); + return singleHostContainerCluster(cluster, host, context); + } } - else { // request 1 node - ClusterSpec clusterSpec = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from(cluster.getName())) - .vespaVersion(deployState.getWantedNodeVespaVersion()) - .dockerImageRepo(deployState.getWantedDockerImageRepo()) - .build(); - Capacity capacity = Capacity.fromCount(1, - Optional.empty(), - false, - ! deployState.getProperties().isBootstrap()); - return hostSystem.allocateHosts(clusterSpec, capacity, 1, logger).keySet().iterator().next(); - } - } else { - return hostSystem.getHost(Container.SINGLENODE_CONTAINER_SERVICESPEC); - } + // request just enough nodes to satisfy environment capacity requirement + ClusterSpec clusterSpec = ClusterSpec.request(ClusterSpec.Type.container, + ClusterSpec.Id.from(cluster.getName())) + .vespaVersion(deployState.getWantedNodeVespaVersion()) + .dockerImageRepo(deployState.getWantedDockerImageRepo()) + .build(); + int nodeCount = deployState.zone().environment().isProduction() ? 2 : 1; + Capacity capacity = Capacity.fromCount(nodeCount, + Optional.empty(), + false, + !deployState.getProperties().isBootstrap()); + var hosts = hostSystem.allocateHosts(clusterSpec, capacity, 1, log); + return createNodesFromHosts(log, hosts, cluster); + } + return singleHostContainerCluster(cluster, hostSystem.getHost(Container.SINGLENODE_CONTAINER_SERVICESPEC), context); + } + + private List<ApplicationContainer> singleHostContainerCluster(ApplicationContainerCluster cluster, HostResource host, ConfigModelContext context) { + ApplicationContainer node = new ApplicationContainer(cluster, "container.0", 0, cluster.isHostedVespa()); + node.setHostResource(host); + node.initService(context.getDeployLogger()); + return List.of(node); } private List<ApplicationContainer> createNodesFromNodeCount(ApplicationContainerCluster cluster, Element nodesElement, ConfigModelContext context) { diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java index d215fdbb7a0..1670ac23ba4 100644 --- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java +++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java @@ -1398,6 +1398,31 @@ public class ModelProvisioningTest { } @Test + public void testNoNodeTagMeansTwoNodesInContainerClusterWithFeatureFlag() { + String services = + "<?xml version='1.0' encoding='utf-8' ?>\n" + + "<services>" + + " <container id='foo' version='1.0'>" + + " <search/>" + + " <document-api/>" + + " </container>" + + " <content version='1.0' id='bar'>" + + " <documents>" + + " <document type='type1' mode='index'/>" + + " </documents>" + + " </content>" + + "</services>"; + VespaModelTester tester = new VespaModelTester(); + tester.setUseDedicatedNodesWhenUnspecified(true); + tester.addHosts(3); + VespaModel model = tester.createModel(services, true); + assertEquals(3, model.getRoot().hostSystem().getHosts().size()); + assertEquals(2, model.getAdmin().getSlobroks().size()); + assertEquals(2, model.getContainerClusters().get("foo").getContainers().size()); + assertEquals(1, model.getContentClusters().get("bar").getRootGroup().countNodes()); + } + + @Test public void testNoNodeTagMeans1NodeNoContent() { String services = "<?xml version='1.0' encoding='utf-8' ?>\n" + diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java index b6180ab78b9..fd837c6dea3 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java @@ -48,6 +48,7 @@ public class VespaModelTester { private Map<NodeResources, Collection<Host>> hostsByResources = new HashMap<>(); private ApplicationId applicationId = ApplicationId.defaultId(); private boolean useDedicatedNodeForLogserver = false; + private boolean useDedicatedNodesWhenUnspecified = false; public VespaModelTester() { this(new NullConfigModelRegistry()); @@ -97,6 +98,10 @@ public class VespaModelTester { this.useDedicatedNodeForLogserver = useDedicatedNodeForLogserver; } + public void setUseDedicatedNodesWhenUnspecified(boolean useDedicatedNodesWhenUnspecified) { + this.useDedicatedNodesWhenUnspecified = useDedicatedNodesWhenUnspecified; + } + /** Creates a model which uses 0 as start index and fails on out of capacity */ public VespaModel createModel(String services, String ... retiredHostNames) { return createModel(Zone.defaultZone(), services, true, retiredHostNames); @@ -137,7 +142,8 @@ public class VespaModelTester { .setMultitenant(true) .setHostedVespa(hosted) .setApplicationId(applicationId) - .setUseDedicatedNodeForLogserver(useDedicatedNodeForLogserver); + .setUseDedicatedNodeForLogserver(useDedicatedNodeForLogserver) + .setUseDedicatedNodesWhenUnspecified(useDedicatedNodesWhenUnspecified); DeployState deployState = new DeployState.Builder() .applicationPackage(appPkg) 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 930bdaadcea..a292ea65d9d 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,6 +146,7 @@ public class ModelContextImpl implements ModelContext { private final boolean usePhraseSegmenting; private final String proxyProtocol; private final Optional<AthenzDomain> athenzDomain; + private final boolean useDedicatedNodesWhenUnspecified; public Properties(ApplicationId applicationId, boolean multitenantFromConfig, @@ -186,6 +187,8 @@ public class ModelContextImpl implements ModelContext { this.proxyProtocol = Flags.PROXY_PROTOCOL.bindTo(flagSource) .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); this.athenzDomain = athenzDomain; + this.useDedicatedNodesWhenUnspecified = Flags.DEDICATED_NODES_WHEN_UNSPECIFIED.bindTo(flagSource) + .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); } @Override @@ -251,6 +254,12 @@ public class ModelContextImpl implements ModelContext { @Override public Optional<AthenzDomain> athenzDomain() { return athenzDomain; } + + @Override + public boolean useDedicatedNodesWhenUnspecified() { + return useDedicatedNodesWhenUnspecified; + } + } } diff --git a/documentapi/test/crosslanguagefiles/6.221-cpp-CreateVisitorMessage.dat b/documentapi/test/crosslanguagefiles/6.221-cpp-CreateVisitorMessage.dat Binary files differindex 27e64170701..a7bb5b0e896 100644 --- a/documentapi/test/crosslanguagefiles/6.221-cpp-CreateVisitorMessage.dat +++ b/documentapi/test/crosslanguagefiles/6.221-cpp-CreateVisitorMessage.dat diff --git a/eval/src/tests/eval/node_types/node_types_test.cpp b/eval/src/tests/eval/node_types/node_types_test.cpp index cabae6307eb..8eaa7a80a81 100644 --- a/eval/src/tests/eval/node_types/node_types_test.cpp +++ b/eval/src/tests/eval/node_types/node_types_test.cpp @@ -223,7 +223,7 @@ TEST("require that merge resolves to the appropriate type") { TEST_DO(verify(strfmt(pattern, "tensor<float>(x[5])", "double"), "error")); } -TEST("require that lambda tensor resolves correct type") { +TEST("require that static tensor lambda resolves correct type") { TEST_DO(verify("tensor(x[5])(1.0)", "tensor(x[5])")); TEST_DO(verify("tensor(x[5],y[10])(1.0)", "tensor(x[5],y[10])")); TEST_DO(verify("tensor(x[5],y[10],z[15])(1.0)", "tensor(x[5],y[10],z[15])")); @@ -242,11 +242,12 @@ TEST("require that tensor create resolves correct type") { TEST_DO(verify("tensor(x[3]):{{x:0}:double,{x:1}:error,{x:2}:double}", "error")); } -TEST("require that tensor lambda resolves correct type") { +TEST("require that dynamic tensor lambda resolves correct type") { TEST_DO(verify("tensor(x[3])(error)", "error")); TEST_DO(verify("tensor(x[3])(double)", "tensor(x[3])")); TEST_DO(verify("tensor<float>(x[3])(double)", "tensor<float>(x[3])")); TEST_DO(verify("tensor(x[3])(tensor(x[2]))", "error")); + TEST_DO(verify("tensor(x[3])(reduce(tensor(x[2])+tensor(x[4]),sum))", "error")); } TEST("require that tensor peek resolves correct type") { diff --git a/eval/src/vespa/eval/eval/node_types.cpp b/eval/src/vespa/eval/eval/node_types.cpp index 924de97c470..5fe441b7a4e 100644 --- a/eval/src/vespa/eval/eval/node_types.cpp +++ b/eval/src/vespa/eval/eval/node_types.cpp @@ -83,7 +83,13 @@ struct TypeResolver : public NodeVisitor, public NodeTraverser { return state.type(node); } - void import(const NodeTypes &types) { + void import_errors(const NodeTypes &types) { + for (const auto &err: types.errors()) { + state.add_error(fmt("[lambda]: %s", err.c_str())); + } + } + + void import_types(const NodeTypes &types) { types.each([&](const Node &node, const ValueType &type) { state.bind(type, node); @@ -189,10 +195,13 @@ struct TypeResolver : public NodeVisitor, public NodeTraverser { arg_types.push_back(param_type(binding)); } NodeTypes lambda_types(node.lambda(), arg_types); - if (!lambda_types.get_type(node.lambda().root()).is_double()) { - return fail(node, "lambda function produces non-double result", false); + const ValueType &lambda_type = lambda_types.get_type(node.lambda().root()); + if (!lambda_type.is_double()) { + import_errors(lambda_types); + return fail(node, fmt("lambda function has non-double result type: %s", + lambda_type.to_spec().c_str()), false); } - import(lambda_types); + import_types(lambda_types); bind(node.type(), node); } void visit(const TensorPeek &node) override { 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 d535450797a..b549fb74db4 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -250,7 +250,7 @@ public class Flags { "Takes effect on next deployment of the application", APPLICATION_ID); public static final UnboundBooleanFlag PHRASE_SEGMENTING = defineFeatureFlag( - "phrase-segmenting", true, + "phrase-segmenting", false, "Should 'implicit phrases' in queries we parsed to a phrase or and?", "Takes effect on redeploy", ZONE_ID, APPLICATION_ID); @@ -273,6 +273,12 @@ public class Flags { "Takes effect immediately", APPLICATION_ID); + public static final UnboundBooleanFlag DEDICATED_NODES_WHEN_UNSPECIFIED = defineFeatureFlag( + "dedicated-nodes-when-unspecified", false, + "Whether config-server should allocate dedicated container nodes when <nodes/> is not specified in services.xml", + "Takes effect on redeploy", + APPLICATION_ID); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, String description, String modificationEffect, FetchVector.Dimension... dimensions) { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java index a49dec4c0f2..f5bb46ef62a 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java @@ -295,7 +295,9 @@ public class DockerOperationsImpl implements DockerOperations { } else if (context.nodeType() == NodeType.tenant) paths.add(varLibSia); - paths.forEach(path -> command.withVolume(context.pathOnHostFromPathInNode(path), path)); + paths.forEach(path -> command.withVolume( + context.pathOnHostFromPathInNode(path), + context.rewritePathInNodeForWantedDockerImage(path))); // Shared paths diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java index 821765fea20..2820fb2fa70 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java @@ -12,6 +12,7 @@ import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking; import java.nio.file.Path; +import java.nio.file.Paths; public interface NodeAgentContext extends TaskContext { @@ -82,4 +83,19 @@ public interface NodeAgentContext extends TaskContext { /** @see #pathInNodeUnderVespaHome(Path) */ Path pathInNodeUnderVespaHome(String relativePath); + + /** + * Rewrite the given path in node to a path required by the image. + * WARNING: This method should only be used when starting the docker container, e.g. writing container data or + * configuring mounts. + * TODO: Remove when everyone has migrated of vespa/ci image + */ + default Path rewritePathInNodeForWantedDockerImage(Path path) { + if (!node().wantedDockerImage().get().repository().endsWith("/vespa/ci")) return path; + + Path originalVespaHome = pathInNodeUnderVespaHome(""); + if (!path.startsWith(originalVespaHome)) return path; + + return Paths.get("/home/y").resolve(originalVespaHome.relativize(path)); + } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java index e84d8345815..250c005566b 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java @@ -1,10 +1,13 @@ // Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.nodeagent; +import com.yahoo.config.provision.DockerImage; import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.Test; import java.nio.file.FileSystem; +import java.nio.file.Path; +import java.nio.file.Paths; import static org.junit.Assert.assertEquals; @@ -69,4 +72,23 @@ public class NodeAgentContextImplTest { public void path_under_vespa_home_must_be_relative() { context.pathInNodeUnderVespaHome("/home"); } + + @Test + public void rewrites_vespa_home_mount_point() { + assertRewrite("docker.tld/vespa/ci:1.2.3", "/var/log", "/var/log"); + assertRewrite("docker.tld/vespa/ci:1.2.3", "/home/y/log", "/home/y/log"); + assertRewrite("docker.tld/vespa/ci:1.2.3", "/opt/vespa/log", "/home/y/log"); + + assertRewrite("docker.tld/vespa/hosted:1.2.3", "/var/log", "/var/log"); + assertRewrite("docker.tld/vespa/hosted:1.2.3", "/home/y/log", "/home/y/log"); + assertRewrite("docker.tld/vespa/hosted:1.2.3", "/opt/vespa/log", "/opt/vespa/log"); + } + + private static void assertRewrite(String dockerImage, String path, String expected) { + NodeAgentContext context = new NodeAgentContextImpl.Builder("node123") + .nodeSpecBuilder(ns -> ns.wantedDockerImage(DockerImage.fromString(dockerImage))) + .build(); + Path actual = context.rewritePathInNodeForWantedDockerImage(Paths.get(path)); + assertEquals(Paths.get(expected), actual); + } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java index 2bbd429fe4e..d2fa773a5a2 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java @@ -44,7 +44,7 @@ public class AllocatableClusterResources { } /** - * Returns the resources which will actually be available in this cluster with this allocation. + * Returns the resources which will actually be available per node in this cluster with this allocation. * These should be used for reasoning about allocation to meet measured demand. */ public NodeResources realResources() { return realResources; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java index eaa5aebda90..d4a237f8e23 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java @@ -75,7 +75,7 @@ public class AutoscalingMaintainer extends Maintainer { private String toString(int nodes, int groups, NodeResources resources) { return String.format(nodes + (groups > 1 ? " (in " + groups + " groups)" : "") + " * [vcpu: %1$.1f, memory: %2$.1f Gb, disk %3$.1f Gb]" + - " (total: [vcpu: %4$.1f, memory: %5$.1f Gb, disk %6$.1f Gb])," + + " (total: [vcpu: %4$.1f, memory: %5$.1f Gb, disk: %6$.1f Gb])," + resources.vcpu(), resources.memoryGb(), resources.diskGb(), nodes * resources.vcpu(), nodes * resources.memoryGb(), nodes * resources.diskGb()); } diff --git a/searchcore/src/tests/proton/docsummary/docsummary.cpp b/searchcore/src/tests/proton/docsummary/docsummary.cpp index c15fd2ccfc1..3c3ff863345 100644 --- a/searchcore/src/tests/proton/docsummary/docsummary.cpp +++ b/searchcore/src/tests/proton/docsummary/docsummary.cpp @@ -774,8 +774,8 @@ Test::requireThatAttributesAreUsed() "bd:[20,30]," "be:[20.2,30.3]," "bf:['bar','baz']," - "bg:[{item:50,weight:3},{item:40,weight:2}]," - "bh:[{item:40.4,weight:4},{item:50.5,weight:5}]," + "bg:[document_sub{item:40,weight:2},{item:50,weight:3}]," + "bh:[{item:50.5,weight:5},{item:40.4,weight:4}]," "bi:[{item:'quux',weight:7},{item:'qux',weight:6}]," "bj:'0x01020178017901016601674008000000000000'}", *rep, 0, true)); TEST_DO(assertTensor(make_tensor(TensorSpec("tensor(x{},y{})") diff --git a/searchcore/src/tests/proton/documentdb/document_subdbs/document_subdbs_test.cpp b/searchcore/src/tests/proton/documentdb/document_subdbs/document_subdbs_test.cpp index 7139eb0d82d..11054566985 100644 --- a/searchcore/src/tests/proton/documentdb/document_subdbs/document_subdbs_test.cpp +++ b/searchcore/src/tests/proton/documentdb/document_subdbs/document_subdbs_test.cpp @@ -447,8 +447,8 @@ void assertAttributes2(const AttributeGuardList &attributes) { EXPECT_EQUAL(2u, attributes.size()); - EXPECT_EQUAL("attr1", attributes[0]->getName()); - EXPECT_EQUAL("attr2", attributes[1]->getName()); + EXPECT_EQUAL("attr2", attributes[0]->getName()); + EXPECT_EQUAL("attr1", attributes[1]->getName()); } void @@ -544,7 +544,7 @@ requireThatAttributeManagerCanBeReconfigured(Fixture &f) f.basicReconfig(10); std::vector<AttributeGuard> attributes; f.getAttributeManager()->getAttributeList(attributes); - assertAttributes2(attributes); + TEST_DO(assertAttributes2(attributes)); } TEST_F("require that attribute manager can be reconfigured", FastAccessFixture) @@ -791,13 +791,13 @@ assertAttribute(const AttributeGuard &attr, const vespalib::string &name, uint32 void assertAttribute1(const AttributeGuard &attr, SerialNum createSerialNum, SerialNum lastSerialNum) { - assertAttribute(attr, "attr1", 3, 22, 44, createSerialNum, lastSerialNum); + TEST_DO(assertAttribute(attr, "attr1", 3, 22, 44, createSerialNum, lastSerialNum)); } void assertAttribute2(const AttributeGuard &attr, SerialNum createSerialNum, SerialNum lastSerialNum) { - assertAttribute(attr, "attr2", 3, 33, 55, createSerialNum, lastSerialNum); + TEST_DO(assertAttribute(attr, "attr2", 3, 33, 55, createSerialNum, lastSerialNum)); } TEST_F("require that fast-access attributes are populated during feed", FastAccessOnlyFixture) @@ -833,8 +833,8 @@ requireThatAttributesArePopulatedDuringReprocessing(FixtureType &f) std::vector<AttributeGuard> attrs; f.getAttributeManager()->getAttributeList(attrs); EXPECT_EQUAL(2u, attrs.size()); - assertAttribute1(attrs[0], CFG_SERIAL, 40); - assertAttribute2(attrs[1], 40, 40); + TEST_DO(assertAttribute1(attrs[1], CFG_SERIAL, 40)); + TEST_DO(assertAttribute2(attrs[0], 40, 40)); } } diff --git a/searchlib/src/tests/predicate/document_features_store_test.cpp b/searchlib/src/tests/predicate/document_features_store_test.cpp index d817b1668ff..72f83e533c9 100644 --- a/searchlib/src/tests/predicate/document_features_store_test.cpp +++ b/searchlib/src/tests/predicate/document_features_store_test.cpp @@ -208,9 +208,9 @@ TEST("require that serialization cleans up wordstore") { EXPECT_EQUAL(460u, features_store.getMemoryUsage().usedBytes()); annotations.range_features.push_back({"bar", 100, 199}); features_store.insert(annotations, doc_id + 1); - EXPECT_EQUAL(800u, features_store.getMemoryUsage().usedBytes()); + EXPECT_EQUAL(848u, features_store.getMemoryUsage().usedBytes()); features_store.remove(doc_id + 1); - EXPECT_EQUAL(752u, features_store.getMemoryUsage().usedBytes()); + EXPECT_EQUAL(800u, features_store.getMemoryUsage().usedBytes()); vespalib::DataBuffer buffer; features_store.serialize(buffer); diff --git a/searchlib/src/tests/ranksetup/ranksetup_test.cpp b/searchlib/src/tests/ranksetup/ranksetup_test.cpp index 7a26180eed2..0a20ddb3739 100644 --- a/searchlib/src/tests/ranksetup/ranksetup_test.cpp +++ b/searchlib/src/tests/ranksetup/ranksetup_test.cpp @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <vespa/vespalib/testkit/testapp.h> +#include <vespa/vespalib/util/stringfmt.h> #include <vespa/searchlib/common/feature.h> #include <vespa/searchlib/attribute/attributeguard.h> @@ -39,6 +40,7 @@ using namespace search::fef; using namespace search::features; using namespace search::fef::test; using search::feature_t; +using vespalib::make_string_short::fmt; typedef FeatureNameBuilder FNB; @@ -477,12 +479,22 @@ RankSetupTest::testCompilation() rs.setFirstPhaseRank(oss.str()); EXPECT_TRUE(!rs.compile()); } - { // cycle + { // short cycle RankSetup rs(_factory, _indexEnv); // c(c,4,2) -> c(c,3,2) -> c(c,2,2) -> c(c,1,2) -> c(c,2,2) rs.setFirstPhaseRank("chain(cycle,4,2)"); EXPECT_TRUE(!rs.compile()); } + { // cycle with max back-trace + RankSetup rs(_factory, _indexEnv); + rs.setFirstPhaseRank(fmt("chain(cycle,%d,2)", BlueprintResolver::MAX_TRACE_SIZE)); + EXPECT_TRUE(!rs.compile()); + } + { // cycle with max+1 back-trace (skip 2) + RankSetup rs(_factory, _indexEnv); + rs.setFirstPhaseRank(fmt("chain(cycle,%d,2)", BlueprintResolver::MAX_TRACE_SIZE + 1)); + EXPECT_TRUE(!rs.compile()); + } } void RankSetupTest::testRankSetup() diff --git a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp index 26c61ba6b3c..bf0a731eaef 100644 --- a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp +++ b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp @@ -295,7 +295,7 @@ RankingExpressionBlueprint::setup(const fef::IIndexEnvironment &env, for (const auto &type_error: node_types.errors()) { LOG(warning, "type error: %s", type_error.c_str()); } - return fail("rank expression contains type errors: %s\n", script.c_str()); + return fail("rank expression contains type errors: %s", script.c_str()); } auto compile_issues = CompiledFunction::detect_issues(*rank_function); auto interpret_issues = InterpretedFunction::detect_issues(*rank_function); diff --git a/searchlib/src/vespa/searchlib/fef/blueprintresolver.cpp b/searchlib/src/vespa/searchlib/fef/blueprintresolver.cpp index c0ca8c857f0..6b4a61e1c0f 100644 --- a/searchlib/src/vespa/searchlib/fef/blueprintresolver.cpp +++ b/searchlib/src/vespa/searchlib/fef/blueprintresolver.cpp @@ -11,11 +11,14 @@ #include <vespa/log/log.h> LOG_SETUP(".fef.blueprintresolver"); +using vespalib::make_string_short::fmt; + namespace search::fef { namespace { -static const size_t MAX_TRACE_SIZE = 16; +constexpr int MAX_TRACE_SIZE = BlueprintResolver::MAX_TRACE_SIZE; +constexpr int TRACE_SKIP_POS = 10; using Accept = Blueprint::AcceptInput; @@ -84,22 +87,37 @@ struct Compiler : public Blueprint::DependencyHandler { Frame &self() { return resolve_stack.back(); } bool failed() const { return !failed_set.empty(); } + vespalib::string make_trace(bool skip_self) { + vespalib::string trace; + auto pos = resolve_stack.rbegin(); + auto end = resolve_stack.rend(); + if ((pos != end) && skip_self) { + ++pos; + } + size_t i = 0; + size_t n = (end - pos); + for (; (pos != end); ++pos, ++i) { + failed_set.insert(pos->parser.featureName()); + bool should_trace = (n <= MAX_TRACE_SIZE); + should_trace |= (i < TRACE_SKIP_POS); + should_trace |= ((end - pos) < (MAX_TRACE_SIZE - TRACE_SKIP_POS)); + if (should_trace) { + trace += fmt(" ... needed by rank feature '%s'\n", pos->parser.featureName().c_str()); + } else if (i == TRACE_SKIP_POS) { + trace += fmt(" (skipped %zu entries)\n", (n - MAX_TRACE_SIZE) + 1); + } + } + return trace; + } + FeatureRef fail(const vespalib::string &feature_name, const vespalib::string &reason, bool skip_self = false) { if (failed_set.count(feature_name) == 0) { failed_set.insert(feature_name); - LOG(warning, "invalid rank feature '%s': %s", feature_name.c_str(), reason.c_str()); - size_t trace_size = 0; - for (size_t i = resolve_stack.size(); i > 0; --i) { - const auto &frame = resolve_stack[i - 1]; - failed_set.insert(frame.parser.featureName()); - if (!skip_self || (&frame != &self())) { - if (++trace_size <= MAX_TRACE_SIZE) { - LOG(warning, " ... needed by rank feature '%s'", frame.parser.featureName().c_str()); - } - } - } - if (trace_size > MAX_TRACE_SIZE) { - LOG(warning, " ... (%zu more)", (trace_size - MAX_TRACE_SIZE)); + auto trace = make_trace(skip_self); + if (trace.empty()) { + LOG(warning, "invalid rank feature '%s': %s", feature_name.c_str(), reason.c_str()); + } else { + LOG(warning, "invalid rank feature '%s': %s\n%s", feature_name.c_str(), reason.c_str(), trace.c_str()); } } return FeatureRef(); @@ -114,8 +132,8 @@ struct Compiler : public Blueprint::DependencyHandler { bool is_object = spec.output_types[ref.output].is_object(); if (!is_compatible(is_object, accept_type)) { return fail(parser.featureName(), - vespalib::make_string("output '%s' has wrong type: was %s, expected %s", - parser.output().c_str(), type_str(is_object), accept_type_str(accept_type))); + fmt("output '%s' has wrong type: was %s, expected %s", + parser.output().c_str(), type_str(is_object), accept_type_str(accept_type))); } return ref; } @@ -136,8 +154,7 @@ struct Compiler : public Blueprint::DependencyHandler { } spec_list.push_back(self().spec); // keep all feature_map refs valid } else { - fail(parser.featureName(), - vespalib::make_string("unknown basename: '%s'", parser.baseName().c_str())); + fail(parser.featureName(), fmt("unknown basename: '%s'", parser.baseName().c_str())); } } } @@ -167,8 +184,7 @@ struct Compiler : public Blueprint::DependencyHandler { if (new_feature != feature_map.end()) { return verify_type(parser, new_feature->second, accept_type); } - return fail(parser.featureName(), - vespalib::make_string("unknown output: '%s'", parser.output().c_str())); + return fail(parser.featureName(), fmt("unknown output: '%s'", parser.output().c_str())); } std::optional<FeatureType> resolve_input(const vespalib::string &feature_name, Accept accept_type) override { diff --git a/searchlib/src/vespa/searchlib/fef/blueprintresolver.h b/searchlib/src/vespa/searchlib/fef/blueprintresolver.h index 8fcfb1be86d..60b1eacb803 100644 --- a/searchlib/src/vespa/searchlib/fef/blueprintresolver.h +++ b/searchlib/src/vespa/searchlib/fef/blueprintresolver.h @@ -68,7 +68,14 @@ public: * problems for 'sane' developers and low enough to avoid stack * overflow. **/ - static const uint32_t MAX_DEP_DEPTH = 256; + static constexpr uint32_t MAX_DEP_DEPTH = 256; + + /** + * The maximum size of back-traces. Longer back-traces will be + * logged with skipped entries somewhere in the middle. Exposed + * for testing purposes. + **/ + static constexpr int MAX_TRACE_SIZE = 16; private: const BlueprintFactory &_factory; diff --git a/staging_vespalib/src/vespa/vespalib/stllike/lrucache_map.h b/staging_vespalib/src/vespa/vespalib/stllike/lrucache_map.h index 07137263cf6..777230566f2 100644 --- a/staging_vespalib/src/vespa/vespalib/stllike/lrucache_map.h +++ b/staging_vespalib/src/vespa/vespalib/stllike/lrucache_map.h @@ -28,32 +28,32 @@ struct LinkedValue : public LinkedValueBase template<typename K, typename V, typename H = vespalib::hash<K>, typename EQ = std::equal_to<K> > struct LruParam { - typedef LinkedValue<V> LV; - typedef std::pair< K, LV > value_type; - typedef vespalib::Select1st< value_type > select_key; - typedef K Key; - typedef V Value; - typedef H Hash; - typedef EQ Equal; - typedef hashtable< Key, value_type, Hash, Equal, select_key > HashTable; + using LV = LinkedValue<V>; + using value_type = std::pair< K, LV >; + using select_key = vespalib::Select1st< value_type >; + using Key = K; + using Value = V; + using Hash = H; + using Equal = EQ; + using HashTable = hashtable< Key, value_type, Hash, Equal, select_key >; }; template< typename P > class lrucache_map : private P::HashTable { private: - typedef typename P::HashTable HashTable; - typedef typename P::Value V; - typedef typename P::Key K; - typedef typename P::value_type value_type; - typedef typename P::LV LV; - typedef typename HashTable::iterator internal_iterator; - typedef typename HashTable::next_t next_t; - typedef typename HashTable::NodeStore NodeStore; + using HashTable = typename P::HashTable; + using V = typename P::Value; + using K = typename P::Key; + using value_type = typename P::value_type; + using LV = typename P::LV; + using internal_iterator = typename HashTable::iterator; + using next_t = typename HashTable::next_t; + using NodeStore = typename HashTable::NodeStore; protected: static constexpr size_t UNLIMITED = std::numeric_limits<size_t>::max(); public: - typedef typename HashTable::insert_result insert_result; + using insert_result = typename HashTable::insert_result; class iterator { public: @@ -177,8 +177,8 @@ public: void swap(lrucache_map & rhs); private: - typedef std::pair<uint32_t, uint32_t> MoveRecord; - typedef std::vector<MoveRecord> MoveRecords; + using MoveRecord = std::pair<uint32_t, uint32_t>; + using MoveRecords = std::vector<MoveRecord>; /** * Implements the resize of the hashtable */ diff --git a/staging_vespalib/src/vespa/vespalib/stllike/lrucache_map.hpp b/staging_vespalib/src/vespa/vespalib/stllike/lrucache_map.hpp index 61147229497..d8d55c9b8c4 100644 --- a/staging_vespalib/src/vespa/vespalib/stllike/lrucache_map.hpp +++ b/staging_vespalib/src/vespa/vespalib/stllike/lrucache_map.hpp @@ -74,7 +74,7 @@ lrucache_map<P>::lrucache_map(size_t maxElems) : { } template< typename P > -lrucache_map<P>::~lrucache_map() { } +lrucache_map<P>::~lrucache_map() = default; template< typename P > void diff --git a/vespalib/src/vespa/vespalib/stllike/hash_map.h b/vespalib/src/vespa/vespalib/stllike/hash_map.h index 3dc5de65285..29a5ef01a9f 100644 --- a/vespalib/src/vespa/vespalib/stllike/hash_map.h +++ b/vespalib/src/vespa/vespalib/stllike/hash_map.h @@ -7,7 +7,7 @@ namespace vespalib { -template< typename K, typename V, typename H = vespalib::hash<K>, typename EQ = std::equal_to<>, typename M=hashtable_base::prime_modulator > +template< typename K, typename V, typename H = vespalib::hash<K>, typename EQ = std::equal_to<>, typename M=hashtable_base::and_modulator > class hash_map { public: diff --git a/vespalib/src/vespa/vespalib/stllike/hash_set.h b/vespalib/src/vespa/vespalib/stllike/hash_set.h index 08288086bf3..0c3f2dcb220 100644 --- a/vespalib/src/vespa/vespalib/stllike/hash_set.h +++ b/vespalib/src/vespa/vespalib/stllike/hash_set.h @@ -8,7 +8,7 @@ namespace vespalib { -template< typename K, typename H = vespalib::hash<K>, typename EQ = std::equal_to<>, typename M=hashtable_base::prime_modulator> +template< typename K, typename H = vespalib::hash<K>, typename EQ = std::equal_to<>, typename M=hashtable_base::and_modulator> class hash_set { private: diff --git a/vespalib/src/vespa/vespalib/stllike/hash_set.hpp b/vespalib/src/vespa/vespalib/stllike/hash_set.hpp index 19114798806..3e48e62f2c7 100644 --- a/vespalib/src/vespa/vespalib/stllike/hash_set.hpp +++ b/vespalib/src/vespa/vespalib/stllike/hash_set.hpp @@ -85,11 +85,11 @@ hash_set<K, H, EQ, M>::insert(K &&value) { #define VESPALIB_HASH_SET_INSTANTIATE(K) \ template class vespalib::hash_set<K>; \ - template class vespalib::hashtable<K, K, vespalib::hash<K>, std::equal_to<>, vespalib::Identity>; \ + template class vespalib::hashtable<K, K, vespalib::hash<K>, std::equal_to<>, vespalib::Identity, vespalib::hashtable_base::and_modulator>; \ template class vespalib::Array<vespalib::hash_node<K>>; #define VESPALIB_HASH_SET_INSTANTIATE_H(K, H) \ template class vespalib::hash_set<K, H>; \ - template class vespalib::hashtable<K, K, H, std::equal_to<>, vespalib::Identity>; \ + template class vespalib::hashtable<K, K, H, std::equal_to<>, vespalib::Identity, vespalib::hashtable_base::and_modulator>; \ template class vespalib::Array<vespalib::hash_node<K>>; |