diff options
90 files changed, 1149 insertions, 872 deletions
diff --git a/cloud-tenant-base-dependencies-enforcer/pom.xml b/cloud-tenant-base-dependencies-enforcer/pom.xml index 8d1f257fca8..074fe6be94e 100644 --- a/cloud-tenant-base-dependencies-enforcer/pom.xml +++ b/cloud-tenant-base-dependencies-enforcer/pom.xml @@ -21,7 +21,7 @@ <properties> <aopalliance.version>1.0</aopalliance.version> <athenz.version>1.10.11</athenz.version> - <bouncycastle.version>1.65</bouncycastle.version> + <bouncycastle.version>1.68</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> @@ -96,8 +96,6 @@ <include>com.sun.xml.bind:jaxb-core:[${jaxb.version}]:jar:provided</include> <include>com.sun.xml.bind:jaxb-impl:[${jaxb.version}]:jar:provided</include> <include>commons-logging:commons-logging:[1.2]:jar:provided</include> - <include>jakarta.activation:jakarta.activation-api:[1.2.1]:jar:provided</include> - <include>jakarta.xml.bind:jakarta.xml.bind-api:[2.3.2]:jar:provided</include> <include>javax.annotation:javax.annotation-api:[${javax.annotation-api.version}]:jar:provided</include> <include>javax.inject:javax.inject:[${javax.inject.version}]:jar:provided</include> <include>javax.servlet:javax.servlet-api:[${javax.servlet-api.version}]:jar:provided</include> diff --git a/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java b/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java index 188504edd18..e41e7309986 100644 --- a/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java +++ b/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java @@ -63,6 +63,8 @@ public class InMemoryProvisioner implements HostProvisioner { private final boolean useMaxResources; + private final boolean alwaysReturnOneNode; + private Provisioned provisioned = new Provisioned(); /** Creates this with a number of nodes with resources 1, 3, 9, 1 */ @@ -72,37 +74,39 @@ public class InMemoryProvisioner implements HostProvisioner { /** Creates this with a number of nodes with given resources */ public InMemoryProvisioner(int nodeCount, NodeResources resources, boolean sharedHosts) { - this(Map.of(resources, createHostInstances(nodeCount)), true, false, sharedHosts, 0); + this(Map.of(resources, createHostInstances(nodeCount)), true, false, false, sharedHosts, 0); } /** Creates this with a set of host names of the flavor 'default' */ public InMemoryProvisioner(boolean failOnOutOfCapacity, boolean sharedHosts, String... hosts) { - this(Map.of(defaultResources, toHostInstances(hosts)), failOnOutOfCapacity, false, sharedHosts, 0); + this(Map.of(defaultResources, toHostInstances(hosts)), failOnOutOfCapacity, false, false, sharedHosts, 0); } /** Creates this with a set of host names of the flavor 'default' */ public InMemoryProvisioner(boolean failOnOutOfCapacity, boolean sharedHosts, List<String> hosts) { - this(Map.of(defaultResources, toHostInstances(hosts.toArray(new String[0]))), failOnOutOfCapacity, false, sharedHosts, 0); + this(Map.of(defaultResources, toHostInstances(hosts.toArray(new String[0]))), failOnOutOfCapacity, false, false, sharedHosts, 0); } /** Creates this with a set of hosts of the flavor 'default' */ public InMemoryProvisioner(Hosts hosts, boolean failOnOutOfCapacity, boolean sharedHosts, String ... retiredHostNames) { - this(Map.of(defaultResources, hosts.asCollection()), failOnOutOfCapacity, false, sharedHosts, 0, retiredHostNames); + this(Map.of(defaultResources, hosts.asCollection()), failOnOutOfCapacity, false, false, sharedHosts, 0, retiredHostNames); } /** Creates this with a set of hosts of the flavor 'default' */ public InMemoryProvisioner(Hosts hosts, boolean failOnOutOfCapacity, boolean sharedHosts, int startIndexForClusters, String ... retiredHostNames) { - this(Map.of(defaultResources, hosts.asCollection()), failOnOutOfCapacity, false, sharedHosts, startIndexForClusters, retiredHostNames); + this(Map.of(defaultResources, hosts.asCollection()), failOnOutOfCapacity, false, false, sharedHosts, startIndexForClusters, retiredHostNames); } public InMemoryProvisioner(Map<NodeResources, Collection<Host>> hosts, boolean failOnOutOfCapacity, boolean useMaxResources, + boolean alwaysReturnOneNode, boolean sharedHosts, int startIndexForClusters, String ... retiredHostNames) { this.failOnOutOfCapacity = failOnOutOfCapacity; this.useMaxResources = useMaxResources; + this.alwaysReturnOneNode = alwaysReturnOneNode; for (Map.Entry<NodeResources, Collection<Host>> hostsWithResources : hosts.entrySet()) for (Host host : hostsWithResources.getValue()) freeNodes.put(hostsWithResources.getKey(), host); @@ -142,16 +146,20 @@ public class InMemoryProvisioner implements HostProvisioner { public List<HostSpec> prepare(ClusterSpec cluster, ClusterResources requested, boolean required, boolean canFail) { if (cluster.group().isPresent() && requested.groups() > 1) throw new IllegalArgumentException("Cannot both be specifying a group and ask for groups to be created"); - int capacity = failOnOutOfCapacity || required + + int nodes = failOnOutOfCapacity || required ? requested.nodes() : Math.min(requested.nodes(), freeNodes.get(defaultResources).size() + totalAllocatedTo(cluster)); - int groups = requested.groups() > capacity ? capacity : requested.groups(); + if (alwaysReturnOneNode) + nodes = 1; + + int groups = requested.groups() > nodes ? nodes : requested.groups(); List<HostSpec> allocation = new ArrayList<>(); if (groups == 1) { allocation.addAll(allocateHostGroup(cluster.with(Optional.of(ClusterSpec.Group.from(0))), requested.nodeResources(), - capacity, + nodes, startIndexForClusters, canFail)); } @@ -159,7 +167,7 @@ public class InMemoryProvisioner implements HostProvisioner { for (int i = 0; i < groups; i++) { allocation.addAll(allocateHostGroup(cluster.with(Optional.of(ClusterSpec.Group.from(i))), requested.nodeResources(), - capacity / groups, + nodes / groups, allocation.size(), canFail)); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/HostResource.java b/config-model/src/main/java/com/yahoo/vespa/model/HostResource.java index 78d9dd473b3..3bc07db9507 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/HostResource.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/HostResource.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.model; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.api.HostInfo; +import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.NodeResources; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/docproc/DocprocChain.java b/config-model/src/main/java/com/yahoo/vespa/model/container/docproc/DocprocChain.java index ee246b5e485..2b2b17c76c3 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/docproc/DocprocChain.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/docproc/DocprocChain.java @@ -23,8 +23,9 @@ public class DocprocChain extends Chain<DocumentProcessor> { } /** - * The field name schema map that applies to this whole chain - * @return doctype,from → to + * The field name schema map that applies to this whole chain. + * + * @return doctype, from → to */ public Map<Pair<String,String>,String> fieldNameSchemaMap() { return fieldNameSchemaMap; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/StorageGroup.java b/config-model/src/main/java/com/yahoo/vespa/model/content/StorageGroup.java index f106b1f7bd5..44365bbbe27 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/StorageGroup.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/StorageGroup.java @@ -162,10 +162,10 @@ public class StorageGroup { } /** Returns the total number of nodes below this group */ - public int countNodes() { - int nodeCount = nodes.size(); + public int countNodes(boolean includeRetired) { + int nodeCount = (int)nodes.stream().filter(node -> includeRetired || ! node.isRetired()).count(); for (StorageGroup group : subgroups) - nodeCount += group.countNodes(); + nodeCount += group.countNodes(includeRetired); return nodeCount; } @@ -220,7 +220,7 @@ public class StorageGroup { ? groupBuilder.buildHosted(deployState, owner, Optional.empty()) : groupBuilder.buildNonHosted(deployState, owner, Optional.empty()); Redundancy redundancy = redundancyBuilder.build(owner.getName(), owner.isHosted(), storageGroup.subgroups.size(), - storageGroup.getNumberOfLeafGroups(), storageGroup.countNodes(), + storageGroup.getNumberOfLeafGroups(), storageGroup.countNodes(false), maxRedundancy); owner.setRedundancy(redundancy); if (storageGroup.partitions.isEmpty() && (redundancy.groups() > 1)) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/IndexingDocprocChain.java b/config-model/src/main/java/com/yahoo/vespa/model/search/IndexingDocprocChain.java index 2c1d979e2c4..8fe6b51f2b4 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/search/IndexingDocprocChain.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/search/IndexingDocprocChain.java @@ -2,14 +2,12 @@ package com.yahoo.vespa.model.search; import com.yahoo.component.ComponentId; -import com.yahoo.component.ComponentSpecification; import com.yahoo.component.chain.Phase; import com.yahoo.component.chain.model.ChainSpecification; import com.yahoo.vespa.configdefinition.SpecialtokensConfig; import com.yahoo.vespa.model.container.docproc.DocprocChain; import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Set; 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 ecb63740fca..326bf577acc 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 @@ -35,7 +35,6 @@ import com.yahoo.vespa.model.test.VespaModelTester; import com.yahoo.vespa.model.test.utils.ApplicationPackageUtils; import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg; import com.yahoo.yolean.Exceptions; -import org.junit.Ignore; import org.junit.Test; import java.io.StringReader; @@ -1069,6 +1068,35 @@ public class ModelProvisioningTest { } @Test + public void testRedundancy2DownscaledToOneNodeButOneRetired() { + String services = + "<?xml version='1.0' encoding='utf-8' ?>" + + "<services>" + + " <content version='1.0' id='bar'>" + + " <redundancy>2</redundancy>" + + " <documents>" + + " <document type='type1' mode='index'/>" + + " </documents>" + + " <nodes count='2'/>" + + " </content>" + + "</services>"; + + int numberOfHosts = 3; + VespaModelTester tester = new VespaModelTester(); + tester.addHosts(numberOfHosts); + VespaModel model = tester.createModel(services, false, false, true, "node-1-3-10-03"); + assertEquals(numberOfHosts, model.getRoot().hostSystem().getHosts().size()); + + ContentCluster cluster = model.getContentClusters().get("bar"); + assertEquals(2, cluster.getStorageNodes().getChildren().size()); + assertEquals(1, cluster.redundancy().effectiveInitialRedundancy()); + assertEquals(1, cluster.redundancy().effectiveFinalRedundancy()); + assertEquals(1, cluster.redundancy().effectiveReadyCopies()); + assertEquals(2, cluster.getRootGroup().getNodes().size()); + assertEquals(0, cluster.getRootGroup().getSubgroups().size()); + } + + @Test public void testUsingNodesCountAttributesAndGettingTooFewNodes() { String services = "<?xml version='1.0' encoding='utf-8' ?>" + @@ -1534,7 +1562,7 @@ public class ModelProvisioningTest { assertEquals("We get 1 node per cluster and no admin node apart from the dedicated cluster controller", 3, model.getHosts().size()); assertEquals(1, model.getContainerClusters().size()); assertEquals(1, model.getContainerClusters().get("foo").getContainers().size()); - assertEquals(1, model.getContentClusters().get("bar").getRootGroup().countNodes()); + assertEquals(1, model.getContentClusters().get("bar").getRootGroup().countNodes(true)); assertEquals(1, model.getAdmin().getClusterControllers().getContainers().size()); } @@ -1587,7 +1615,7 @@ public class ModelProvisioningTest { assertEquals(6, model.getRoot().hostSystem().getHosts().size()); assertEquals(5, model.getAdmin().getSlobroks().size()); assertEquals(2, model.getContainerClusters().get("foo").getContainers().size()); - assertEquals(1, model.getContentClusters().get("bar").getRootGroup().countNodes()); + assertEquals(1, model.getContentClusters().get("bar").getRootGroup().countNodes(true)); } @Test @@ -1657,7 +1685,7 @@ public class ModelProvisioningTest { assertEquals(1, model.getRoot().hostSystem().getHosts().size()); assertEquals(1, model.getAdmin().getSlobroks().size()); assertEquals(1, model.getContainerClusters().get("foo").getContainers().size()); - assertEquals(1, model.getContentClusters().get("bar").getRootGroup().countNodes()); + assertEquals(1, model.getContentClusters().get("bar").getRootGroup().countNodes(true)); } /** Recreate the combination used in some factory tests */ @@ -1940,7 +1968,7 @@ public class ModelProvisioningTest { assertTrue("Initial servers are not joining", config.build().server().stream().noneMatch(ZookeeperServerConfig.Server::joining)); } { - VespaModel nextModel = tester.createModel(Zone.defaultZone(), servicesXml.apply(5), true, false, 0, Optional.of(model), new DeployState.Builder()); + VespaModel nextModel = tester.createModel(Zone.defaultZone(), servicesXml.apply(5), true, false, false, 0, Optional.of(model), new DeployState.Builder()); ApplicationContainerCluster cluster = nextModel.getContainerClusters().get("zk"); ZookeeperServerConfig.Builder config = new ZookeeperServerConfig.Builder(); cluster.getContainers().forEach(c -> c.getConfig(config)); 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 b72ae088484..ba975e52d1a 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 @@ -52,6 +52,7 @@ public class VespaModelTester { private final Map<NodeResources, Collection<Host>> hostsByResources = new HashMap<>(); private ApplicationId applicationId = ApplicationId.defaultId(); private boolean useDedicatedNodeForLogserver = false; + private HostProvisioner provisioner; public VespaModelTester() { this(new NullConfigModelRegistry()); @@ -61,6 +62,12 @@ public class VespaModelTester { this.configModelRegistry = configModelRegistry; } + public HostProvisioner provisioner() { + if (provisioner instanceof ProvisionerAdapter) + return ((ProvisionerAdapter)provisioner).provisioner(); + return provisioner; + } + /** Adds some nodes with resources 1, 3, 10 */ public Hosts addHosts(int count) { return addHosts(InMemoryProvisioner.defaultResources, count); } @@ -108,37 +115,43 @@ public class VespaModelTester { /** Creates a model which uses 0 as start index */ public VespaModel createModel(String services, boolean failOnOutOfCapacity, String ... retiredHostNames) { - return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, false, 0, + return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, false, false, 0, Optional.empty(), new DeployState.Builder(), retiredHostNames); } /** Creates a model which uses 0 as start index */ public VespaModel createModel(String services, boolean failOnOutOfCapacity, DeployState.Builder builder) { - return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, false, 0, Optional.empty(), builder); + return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, false, false, 0, Optional.empty(), builder); } /** Creates a model which uses 0 as start index */ public VespaModel createModel(String services, boolean failOnOutOfCapacity, boolean useMaxResources, String ... retiredHostNames) { - return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, useMaxResources, 0, + return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, useMaxResources, false, 0, + Optional.empty(), new DeployState.Builder(), retiredHostNames); + } + + /** Creates a model which uses 0 as start index */ + public VespaModel createModel(String services, boolean failOnOutOfCapacity, boolean useMaxResources, boolean alwaysReturnOneNode, String ... retiredHostNames) { + return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, useMaxResources, alwaysReturnOneNode, 0, Optional.empty(), new DeployState.Builder(), retiredHostNames); } /** Creates a model which uses 0 as start index */ public VespaModel createModel(String services, boolean failOnOutOfCapacity, int startIndexForClusters, String ... retiredHostNames) { - return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, false, startIndexForClusters, + return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, false, false, startIndexForClusters, Optional.empty(), new DeployState.Builder(), retiredHostNames); } /** Creates a model which uses 0 as start index */ public VespaModel createModel(Zone zone, String services, boolean failOnOutOfCapacity, String ... retiredHostNames) { - return createModel(zone, services, failOnOutOfCapacity, false, 0, + return createModel(zone, services, failOnOutOfCapacity, false, false, 0, Optional.empty(), new DeployState.Builder(), retiredHostNames); } /** Creates a model which uses 0 as start index */ public VespaModel createModel(Zone zone, String services, boolean failOnOutOfCapacity, DeployState.Builder deployStateBuilder, String ... retiredHostNames) { - return createModel(zone, services, failOnOutOfCapacity, false, 0, + return createModel(zone, services, failOnOutOfCapacity, false, false, 0, Optional.empty(), deployStateBuilder, retiredHostNames); } @@ -152,15 +165,16 @@ public class VespaModelTester { * @return the resulting model */ public VespaModel createModel(Zone zone, String services, boolean failOnOutOfCapacity, boolean useMaxResources, + boolean alwaysReturnOneNode, int startIndexForClusters, Optional<VespaModel> previousModel, DeployState.Builder deployStatebuilder, String ... retiredHostNames) { VespaModelCreatorWithMockPkg modelCreatorWithMockPkg = new VespaModelCreatorWithMockPkg(null, services, ApplicationPackageUtils.generateSearchDefinition("type1")); ApplicationPackage appPkg = modelCreatorWithMockPkg.appPkg; - HostProvisioner provisioner = hosted ? - new ProvisionerAdapter(new InMemoryProvisioner(hostsByResources, + provisioner = hosted ? new ProvisionerAdapter(new InMemoryProvisioner(hostsByResources, failOnOutOfCapacity, useMaxResources, + alwaysReturnOneNode, false, startIndexForClusters, retiredHostNames)) : @@ -184,12 +198,14 @@ public class VespaModelTester { /** To verify that we don't call allocateHost(alias) in hosted environments */ private static class ProvisionerAdapter implements HostProvisioner { - private final HostProvisioner provisioner; + private final InMemoryProvisioner provisioner; - public ProvisionerAdapter(HostProvisioner provisioner) { + public ProvisionerAdapter(InMemoryProvisioner provisioner) { this.provisioner = provisioner; } + public InMemoryProvisioner provisioner() { return provisioner; } + @Override public HostSpec allocateHost(String alias) { throw new UnsupportedOperationException("Allocating hosts using <node> tags is not supported in hosted environments, " + diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/ApplicationFileManager.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/ApplicationFileManager.java index 2cb94b4bb9e..4152c92c289 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/ApplicationFileManager.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/ApplicationFileManager.java @@ -1,10 +1,11 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.filedistribution; import com.yahoo.config.FileReference; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; +import java.net.SocketTimeoutException; import java.net.URL; import java.nio.channels.Channels; import java.nio.channels.ReadableByteChannel; @@ -44,6 +45,8 @@ public class ApplicationFileManager implements AddFileInterface { rbc = Channels.newChannel(website.openStream()); fos = new FileOutputStream(file.getAbsolutePath()); fos.getChannel().transferFrom(rbc, 0, Long.MAX_VALUE); + } catch (SocketTimeoutException e) { + throw new IllegalArgumentException("Failed connecting to or reading from " + uri, e); } catch (IOException e) { throw new IllegalArgumentException("Failed creating directory " + file.getParent(), e); } finally { diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/HttpRequest.java b/container-core/src/main/java/com/yahoo/container/jdisc/HttpRequest.java index e202442479f..96a7902a076 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/HttpRequest.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/HttpRequest.java @@ -573,7 +573,7 @@ public class HttpRequest { @Override public long currentTimeMillis() { - return 0; + return System.currentTimeMillis(); } }; } diff --git a/container-dependencies-enforcer/pom.xml b/container-dependencies-enforcer/pom.xml index ab2cfdda1a1..d692adac3c3 100644 --- a/container-dependencies-enforcer/pom.xml +++ b/container-dependencies-enforcer/pom.xml @@ -90,8 +90,6 @@ <include>com.sun.xml.bind:jaxb-core:[${jaxb.version}]:jar:provided</include> <include>com.sun.xml.bind:jaxb-impl:[${jaxb.version}]:jar:provided</include> <include>commons-logging:commons-logging:[1.2]:jar:provided</include> - <include>jakarta.activation:jakarta.activation-api:[1.2.1]:jar:provided</include> - <include>jakarta.xml.bind:jakarta.xml.bind-api:[2.3.2]:jar:provided</include> <include>javax.annotation:javax.annotation-api:[${javax.annotation-api.version}]:jar:provided</include> <include>javax.inject:javax.inject:[${javax.inject.version}]:jar:provided</include> <include>javax.servlet:javax.servlet-api:[${javax.servlet-api.version}]:jar:provided</include> diff --git a/container-dependency-versions/pom.xml b/container-dependency-versions/pom.xml index 5e4a45074ae..b392b0b80ac 100644 --- a/container-dependency-versions/pom.xml +++ b/container-dependency-versions/pom.xml @@ -165,18 +165,6 @@ <artifactId>javax.activation</artifactId> <version>1.2.0</version> </dependency> - <dependency> - <!-- Needed by jackson-module-jaxb-annotations --> - <groupId>jakarta.xml.bind</groupId> - <artifactId>jakarta.xml.bind-api</artifactId> - <version>${jakarta-xml-bind-api.version}</version> - </dependency> - <dependency> - <!-- Needed by jackson-module-jaxb-annotations --> - <groupId>jakarta.activation</groupId> - <artifactId>jakarta.activation-api</artifactId> - <version>${jakarta-activation-api.version}</version> - </dependency> <!-- jaxb end --> <dependency> @@ -383,7 +371,7 @@ <properties> <aopalliance.version>1.0</aopalliance.version> - <bouncycastle.version>1.65</bouncycastle.version> + <bouncycastle.version>1.68</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> @@ -407,8 +395,6 @@ <hk2.osgi-resource-locator.version>1.0.1</hk2.osgi-resource-locator.version> <jackson2.version>2.12.1</jackson2.version> <jackson-databind.version>${jackson2.version}</jackson-databind.version> - <jakarta-activation-api.version>1.2.1</jakarta-activation-api.version> - <jakarta-xml-bind-api.version>2.3.2</jakarta-xml-bind-api.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 cfe9c0a6d8e..06c68b52e67 100644 --- a/container-dev/pom.xml +++ b/container-dev/pom.xml @@ -104,6 +104,18 @@ <artifactId>vespa_jersey2</artifactId> <version>${project.version}</version> <type>pom</type> + <exclusions> + <exclusion> + <!-- Conflicts with javax.xml.bind:jaxb-api:2.3, which is "exported" via jdisc_core.--> + <groupId>jakarta.xml.bind</groupId> + <artifactId>jakarta.xml.bind-api</artifactId> + </exclusion> + <exclusion> + <!-- Conflicts with javax.activation:javax.activation-api:1.2.0, which is "exported" via jdisc_core. --> + <groupId>jakarta.activation</groupId> + <artifactId>jakarta.activation-api</artifactId> + </exclusion> + </exclusions> </dependency> <dependency> <groupId>com.yahoo.vespa</groupId> diff --git a/container-disc/pom.xml b/container-disc/pom.xml index e537b8f1c35..f87148fabaa 100644 --- a/container-disc/pom.xml +++ b/container-disc/pom.xml @@ -220,8 +220,8 @@ jackson-jaxrs-base-${jackson2.version}.jar, jackson-jaxrs-json-provider-${jackson2.version}.jar, jackson-module-jaxb-annotations-${jackson2.version}.jar, - jakarta.activation-api-${jakarta-activation-api.version}.jar, - jakarta.xml.bind-api-${jakarta-xml-bind-api.version}.jar, + jakarta.activation-api-1.2.1.jar, + jakarta.xml.bind-api-2.3.2.jar, javassist-${javassist.version}.jar, javax.ws.rs-api-${javax.ws.rs-api.version}.jar, jersey-client-${jersey2.version}.jar, diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index b5933936adf..74ed9d33f04 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -5574,8 +5574,8 @@ "public com.yahoo.search.query.parser.ParserEnvironment setIndexFacts(com.yahoo.prelude.IndexFacts)", "public com.yahoo.language.Linguistics getLinguistics()", "public com.yahoo.search.query.parser.ParserEnvironment setLinguistics(com.yahoo.language.Linguistics)", - "public com.yahoo.prelude.query.parser.SpecialTokens getSpecialTokens()", - "public com.yahoo.search.query.parser.ParserEnvironment setSpecialTokens(com.yahoo.prelude.query.parser.SpecialTokens)", + "public com.yahoo.language.process.SpecialTokens getSpecialTokens()", + "public com.yahoo.search.query.parser.ParserEnvironment setSpecialTokens(com.yahoo.language.process.SpecialTokens)", "public static com.yahoo.search.query.parser.ParserEnvironment fromExecutionContext(com.yahoo.search.searchchain.Execution$Context)", "public static com.yahoo.search.query.parser.ParserEnvironment fromParserEnvironment(com.yahoo.search.query.parser.ParserEnvironment)" ], @@ -7765,7 +7765,7 @@ "final" ], "methods": [ - "public void <init>(com.yahoo.search.searchchain.SearchChainRegistry, com.yahoo.prelude.IndexFacts, com.yahoo.prelude.query.parser.SpecialTokenRegistry, com.yahoo.search.rendering.RendererRegistry, com.yahoo.language.Linguistics)", + "public void <init>(com.yahoo.search.searchchain.SearchChainRegistry, com.yahoo.prelude.IndexFacts, com.yahoo.language.process.SpecialTokenRegistry, com.yahoo.search.rendering.RendererRegistry, com.yahoo.language.Linguistics)", "public static com.yahoo.search.searchchain.Execution$Context createContextStub()", "public static com.yahoo.search.searchchain.Execution$Context createContextStub(com.yahoo.prelude.IndexFacts)", "public static com.yahoo.search.searchchain.Execution$Context createContextStub(com.yahoo.search.searchchain.SearchChainRegistry, com.yahoo.prelude.IndexFacts)", @@ -7779,8 +7779,8 @@ "public void setIndexFacts(com.yahoo.prelude.IndexFacts)", "public com.yahoo.search.searchchain.SearchChainRegistry searchChainRegistry()", "public com.yahoo.search.rendering.RendererRegistry rendererRegistry()", - "public com.yahoo.prelude.query.parser.SpecialTokenRegistry getTokenRegistry()", - "public void setTokenRegistry(com.yahoo.prelude.query.parser.SpecialTokenRegistry)", + "public com.yahoo.language.process.SpecialTokenRegistry getTokenRegistry()", + "public void setTokenRegistry(com.yahoo.language.process.SpecialTokenRegistry)", "public void setDetailedDiagnostics(boolean)", "public boolean getDetailedDiagnostics()", "public boolean getBreakdown()", diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java index 902be7e15dd..732466748eb 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java @@ -19,7 +19,6 @@ import java.util.*; * @author bratseth * @author Steinar Knutsen */ -@SuppressWarnings("deprecation") public abstract class AbstractParser implements CustomParser { /** The current submodes of this parser */ @@ -48,7 +47,7 @@ public abstract class AbstractParser implements CustomParser { * of these may be active at the same time. SubModes are activated or * deactivated by specifying special indexes in the query. */ - final class Submodes { + static final class Submodes { /** * Url mode allows "_" and "-" as word characters. Default is false diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/SpecialTokenRegistry.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/SpecialTokenRegistry.java deleted file mode 100644 index be2d9f9f68b..00000000000 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/SpecialTokenRegistry.java +++ /dev/null @@ -1,137 +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.prelude.query.parser; - -import com.yahoo.config.subscription.ConfigGetter; -import com.yahoo.config.subscription.ConfigSubscriber; -import com.yahoo.vespa.configdefinition.SpecialtokensConfig; -import com.yahoo.vespa.configdefinition.SpecialtokensConfig.Tokenlist; -import com.yahoo.vespa.configdefinition.SpecialtokensConfig.Tokenlist.Tokens; - -import java.util.*; -import java.util.logging.Logger; - - -/** - * A <i>registry</i> which is responsible for knowing the current - * set of special tokens. The default registry returns empty token lists - * for all names. Usage of this registry is multithread safe. - * - * @author bratseth - */ -public class SpecialTokenRegistry { - - /** The log of this */ - private static final Logger log = Logger.getLogger(SpecialTokenRegistry.class.getName()); - - private static final SpecialTokens nullSpecialTokens = new SpecialTokens(); - - /** - * The current authorative special token lists, indexed on name. - * These lists are unmodifiable and used directly by clients of this - */ - private Map<String,SpecialTokens> specialTokenMap = new HashMap<>(); - - private boolean frozen = false; - - /** - * Creates an empty special token registry which - * does not subscribe to any configuration - */ - public SpecialTokenRegistry() {} - - /** - * Create a special token registry which subscribes to the specialtokens - * configuration. Only used for testing. - */ - public SpecialTokenRegistry(String configId) { - try { - build(new ConfigGetter<>(SpecialtokensConfig.class).getConfig(configId)); - } catch (Exception e) { - log.config( - "No special tokens are configured (" + e.getMessage() + ")"); - } - } - - /** - * Create a special token registry from a configuration object. This is the production code path. - */ - public SpecialTokenRegistry(SpecialtokensConfig config) { - if (config != null) { - build(config); - } - freeze(); - } - - private void freeze() { - frozen = true; - } - - private void build(SpecialtokensConfig config) { - List<SpecialTokens> list = new ArrayList<>(); - for (Iterator<Tokenlist> i = config.tokenlist().iterator(); i.hasNext();) { - Tokenlist tokenList = i.next(); - SpecialTokens tokens = new SpecialTokens(tokenList.name()); - - for (Iterator<Tokens> j = tokenList.tokens().iterator(); j.hasNext();) { - Tokens token = j.next(); - tokens.addSpecialToken(token.token(), token.replace()); - } - tokens.freeze(); - list.add(tokens); - } - addSpecialTokens(list); - } - - /** - * Adds a SpecialTokens instance to the registry. That is, add the - * tokens contained for the name of the SpecialTokens instance - * given. - * - * @param specialTokens the SpecialTokens object to add - */ - public void addSpecialTokens(SpecialTokens specialTokens) { - ensureNotFrozen(); - List<SpecialTokens> list = new ArrayList<>(); - list.add(specialTokens); - addSpecialTokens(list); - - } - - private void ensureNotFrozen() { - if (frozen) { - throw new IllegalStateException("Tried to modify a frozen SpecialTokenRegistry instance."); - } - } - - private void addSpecialTokens(List<SpecialTokens> list) { - HashMap<String,SpecialTokens> tokens = new HashMap<>(specialTokenMap); - for(SpecialTokens t: list) { - tokens.put(t.getName(),t); - } - specialTokenMap = tokens; - } - - - /** - * Returns the currently authorative list of special tokens for - * a given name. - * - * @param name the name of the special tokens to return - * null, the empth string or the string "default" returns - * the default ones - * @return a read-only list of SpecialToken instances, an empty list if this name - * has no special tokens - */ - public SpecialTokens getSpecialTokens(String name) { - if (name == null || name.trim().equals("")) { - name = "default"; - } - SpecialTokens specialTokens = specialTokenMap.get(name); - - if (specialTokens == null) { - return nullSpecialTokens; - } - return specialTokens; - } - -} 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 deleted file mode 100644 index f45ecefefa6..00000000000 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/SpecialTokens.java +++ /dev/null @@ -1,167 +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.prelude.query.parser; - -import java.util.logging.Level; -import com.yahoo.prelude.query.Substring; - -import java.util.*; -import java.util.logging.Logger; - -import static com.yahoo.language.LinguisticsCase.toLowerCase; - -/** - * A list of special tokens - string that should be treated as word - * no matter what they contain. Special tokens are case insensitive. - * - * @author bratseth - */ -public class SpecialTokens { - - private static final Logger log = Logger.getLogger(SpecialTokens.class.getName()); - - private final String name; - - private final List<SpecialToken> specialTokens = new ArrayList<>(); - - private boolean frozen = false; - - private int currentMaximumLength = 0; - - /** Creates a null list of special tokens */ - public SpecialTokens() { - this.name = "(null)"; - } - - public SpecialTokens(String name) { - this.name = name; - } - - /** Returns the name of this special tokens list */ - public String getName() { - return name; - } - - /** - * Adds a special token to this - * - * @param token the special token string to add - * @param replace the token to replace instances of the special token with, or null to keep the token - */ - public void addSpecialToken(String token, String replace) { - ensureNotFrozen(); - if (!caseIndependentLength(token)) { - return; - } - // TODO are special tokens correctly unicode normalized in reagards to query parsing? - final SpecialToken specialTokenToAdd = new SpecialToken(token, replace); - currentMaximumLength = Math.max(currentMaximumLength, specialTokenToAdd.token.length()); - specialTokens.add(specialTokenToAdd); - Collections.sort(specialTokens); - } - - private boolean caseIndependentLength(String token) { - // XXX not fool proof length test, should test codepoint by codepoint for mixed case user input? not even that will necessarily be 100% robust... - String asLow = toLowerCase(token); - // TODO put along with the global toLowerCase - String asHigh = token.toUpperCase(Locale.ENGLISH); - if (asLow.length() != token.length() || asHigh.length() != token.length()) { - log.log(Level.SEVERE, "Special token '" + token + "' has case sensitive length. Ignoring the token." - + " Please report this message in a bug to the Vespa team."); - return false; - } else { - return true; - } - } - - /** - * Returns the special token starting at the start of the given string, or null if no - * special token starts at this string - * - * @param string the string to search for a special token at the start position - * @param substring true to allow the special token to be followed by a character which does not - * mark the end of a token - */ - public SpecialToken tokenize(String string, boolean substring) { - // XXX detonator pattern token.length may be != the length of the - // matching data in string, ref caseIndependentLength(String) - final String input = toLowerCase(string.substring(0, Math.min(string.length(), currentMaximumLength))); - for (Iterator<SpecialToken> i = specialTokens.iterator(); i.hasNext();) { - SpecialTokens.SpecialToken special = i.next(); - - if (input.startsWith(special.token())) { - if (string.length() == special.token().length() || substring || tokenEndsAt(special.token().length(), string)) - return special; - } - } - return null; - } - - private boolean tokenEndsAt(int position,String string) { - return !Character.isLetterOrDigit(string.charAt(position)); - } - - /** Returns the number of special tokens in this */ - public int size() { - return specialTokens.size(); - } - - private void ensureNotFrozen() { - if (frozen) { - throw new IllegalStateException("Tried to modify a frozen SpecialTokens instance."); - } - } - - public void freeze() { - frozen = true; - } - - /** An immutable special token */ - public final static class SpecialToken implements Comparable<SpecialToken> { - - private String token; - - private String replace; - - public SpecialToken(String token, String replace) { - this.token = toLowerCase(token); - if (replace == null || replace.trim().equals("")) { - this.replace = this.token; - } else { - this.replace = toLowerCase(replace); - } - } - - /** Returns the special token */ - public String token() { - return token; - } - - /** Returns the right replace value, never null or an empty string */ - public String replace() { - return replace; - } - - @Override - public int compareTo(SpecialToken other) { - if (this.token().length() < other.token().length()) return 1; - if (this.token().length() == other.token().length()) return 0; - return -1; - } - - @Override - public boolean equals(Object other) { - if (other == this) return true; - if ( ! (other instanceof SpecialToken)) return false; - return Objects.equals(this.token, ((SpecialToken)other).token); - } - - @Override - public int hashCode() { return token.hashCode(); } - - 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/prelude/query/parser/Tokenizer.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/Tokenizer.java index 2dc2254df68..b71bd57539f 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/Tokenizer.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/Tokenizer.java @@ -3,6 +3,7 @@ package com.yahoo.prelude.query.parser; import com.yahoo.language.Linguistics; import com.yahoo.language.process.CharacterClasses; +import com.yahoo.language.process.SpecialTokens; import com.yahoo.prelude.Index; import com.yahoo.prelude.IndexFacts; import com.yahoo.prelude.query.Substring; @@ -200,7 +201,7 @@ public final class Tokenizer { } StringBuilder tmp = new StringBuilder(); for (int i = 0; i < tokencnt; i++) { - Token useToken = tokens.get(backtrack+i); + Token useToken = tokens.get(backtrack + i); tmp.append(useToken.image); } String indexName = tmp.toString(); @@ -216,20 +217,20 @@ public final class Tokenizer { } private int consumeSpecialToken(int start) { - SpecialTokens.SpecialToken specialToken=getSpecialToken(start); - if (specialToken==null) return start; - tokens.add(specialToken.toToken(start,source)); - return start + specialToken.token().length(); + SpecialTokens.Token token = getSpecialToken(start); + if (token == null) return start; + tokens.add(toToken(token, start, source)); + return start + token.token().length(); } - private SpecialTokens.SpecialToken getSpecialToken(int start) { + private SpecialTokens.Token getSpecialToken(int start) { if (specialTokens == null) return null; return specialTokens.tokenize(source.substring(start), substringSpecialTokens); } private int consumeExact(int start,Index index) { if (index.getExactTerminator() == null) return consumeHeuristicExact(start); - return consumeToTerminator(start,index.getExactTerminator()); + return consumeToTerminator(start, index.getExactTerminator()); } private boolean looksLikeExactEnd(int end) { @@ -467,7 +468,7 @@ public final class Tokenizer { /** Consumes a word or number <i>and/or possibly</i> a special token starting within this word or number */ private int consumeWordOrNumber(int start, Index currentIndex) { int tokenEnd = start; - SpecialTokens.SpecialToken substringSpecialToken = null; + SpecialTokens.Token substringToken = null; boolean digitsOnly = true; // int underscores = 0; // boolean underscoresOnly = true; @@ -475,8 +476,8 @@ public final class Tokenizer { while (tokenEnd < source.length()) { if (substringSpecialTokens) { - substringSpecialToken = getSpecialToken(tokenEnd); - if (substringSpecialToken != null) break; + substringToken = getSpecialToken(tokenEnd); + if (substringToken != null) break; } int c = source.codePointAt(tokenEnd); @@ -524,11 +525,11 @@ public final class Tokenizer { } } - if (substringSpecialToken == null) + if (substringToken == null) return --tokenEnd; // TODO: test the logic around tokenEnd with friends - addToken(substringSpecialToken.toToken(tokenEnd, source)); - return --tokenEnd + substringSpecialToken.token().length(); + addToken(toToken(substringToken, tokenEnd, source)); + return --tokenEnd + substringToken.token().length(); } private void addToken(Token.Kind kind, String word, int start, int end) { @@ -539,4 +540,11 @@ public final class Tokenizer { tokens.add(token); } + public Token toToken(SpecialTokens.Token specialToken, int start, String rawSource) { + return new Token(Token.Kind.WORD, + specialToken.replacement(), + true, + new Substring(start, start + specialToken.token().length(), rawSource)); // XXX: Unsafe? + } + } diff --git a/container-search/src/main/java/com/yahoo/search/Query.java b/container-search/src/main/java/com/yahoo/search/Query.java index ce31b9a3ba3..4ecede819de 100644 --- a/container-search/src/main/java/com/yahoo/search/Query.java +++ b/container-search/src/main/java/com/yahoo/search/Query.java @@ -7,7 +7,6 @@ import com.yahoo.collections.Tuple2; import com.yahoo.component.Version; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.fs4.MapEncoder; -import java.util.logging.Level; import com.yahoo.prelude.fastsearch.DocumentDatabase; import com.yahoo.prelude.query.Highlight; import com.yahoo.prelude.query.textualrepresentation.TextualQueryRepresentation; @@ -56,6 +55,8 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.TimeUnit; +import java.util.logging.Level; import java.util.logging.Logger; /** @@ -336,7 +337,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { } private void init(Map<String, String> requestMap, CompiledQueryProfile queryProfile) { - startTime = System.currentTimeMillis(); + startTime = httpRequest.getJDiscRequest().creationTime(TimeUnit.MILLISECONDS); if (queryProfile != null) { // Move all request parameters to the query profile just to validate that the parameter settings are legal Properties queryProfileProperties = new QueryProfileProperties(queryProfile); diff --git a/container-search/src/main/java/com/yahoo/search/query/parser/ParserEnvironment.java b/container-search/src/main/java/com/yahoo/search/query/parser/ParserEnvironment.java index 94b9bf6ce65..df96d314455 100644 --- a/container-search/src/main/java/com/yahoo/search/query/parser/ParserEnvironment.java +++ b/container-search/src/main/java/com/yahoo/search/query/parser/ParserEnvironment.java @@ -4,7 +4,7 @@ package com.yahoo.search.query.parser; import com.yahoo.language.Linguistics; import com.yahoo.language.simple.SimpleLinguistics; import com.yahoo.prelude.IndexFacts; -import com.yahoo.prelude.query.parser.SpecialTokens; +import com.yahoo.language.process.SpecialTokens; import com.yahoo.search.Searcher; import com.yahoo.search.searchchain.Execution; @@ -18,7 +18,7 @@ public final class ParserEnvironment { private IndexFacts indexFacts = new IndexFacts(); private Linguistics linguistics = new SimpleLinguistics(); - private SpecialTokens specialTokens = new SpecialTokens(); + private SpecialTokens specialTokens = SpecialTokens.empty(); public IndexFacts getIndexFacts() { return indexFacts; diff --git a/container-search/src/main/java/com/yahoo/search/searchchain/Execution.java b/container-search/src/main/java/com/yahoo/search/searchchain/Execution.java index 84fe88d0292..0574fc660c3 100644 --- a/container-search/src/main/java/com/yahoo/search/searchchain/Execution.java +++ b/container-search/src/main/java/com/yahoo/search/searchchain/Execution.java @@ -6,7 +6,7 @@ import com.yahoo.language.Linguistics; import com.yahoo.prelude.IndexFacts; import com.yahoo.prelude.Ping; import com.yahoo.prelude.Pong; -import com.yahoo.prelude.query.parser.SpecialTokenRegistry; +import com.yahoo.language.process.SpecialTokenRegistry; import com.yahoo.processing.Processor; import com.yahoo.processing.Request; import com.yahoo.processing.Response; @@ -17,8 +17,6 @@ import com.yahoo.search.cluster.PingableSearcher; import com.yahoo.search.rendering.RendererRegistry; import com.yahoo.search.statistics.TimeTracker; -import java.util.logging.Logger; - /** * <p>An execution of a search chain. This keeps track of the call state for an execution (in the calling thread) * of the searchers of a search chain.</p> @@ -111,7 +109,7 @@ public class Execution extends com.yahoo.processing.execution.Execution { public Context(SearchChainRegistry searchChainRegistry, IndexFacts indexFacts, SpecialTokenRegistry tokenRegistry, RendererRegistry rendererRegistry, Linguistics linguistics) { - owner=null; + owner = null; // The next time something is added here, compose into wrapper objects. Many arguments... // Four methods need to be updated when adding something: diff --git a/container-search/src/main/java/com/yahoo/search/searchchain/ExecutionFactory.java b/container-search/src/main/java/com/yahoo/search/searchchain/ExecutionFactory.java index 31b6d06f78e..a813229c984 100644 --- a/container-search/src/main/java/com/yahoo/search/searchchain/ExecutionFactory.java +++ b/container-search/src/main/java/com/yahoo/search/searchchain/ExecutionFactory.java @@ -13,7 +13,7 @@ import com.yahoo.language.Linguistics; import com.yahoo.language.simple.SimpleLinguistics; import com.yahoo.prelude.IndexFacts; import com.yahoo.prelude.IndexModel; -import com.yahoo.prelude.query.parser.SpecialTokenRegistry; +import com.yahoo.language.process.SpecialTokenRegistry; import com.yahoo.processing.rendering.Renderer; import com.yahoo.search.Searcher; import com.yahoo.search.config.IndexInfoConfig; diff --git a/container-search/src/test/java/com/yahoo/prelude/query/parser/test/ParseTestCase.java b/container-search/src/test/java/com/yahoo/prelude/query/parser/test/ParseTestCase.java index 6afea895f3a..cef8ae1751c 100644 --- a/container-search/src/test/java/com/yahoo/prelude/query/parser/test/ParseTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/query/parser/test/ParseTestCase.java @@ -18,16 +18,14 @@ import com.yahoo.prelude.query.PhraseSegmentItem; import com.yahoo.prelude.query.PrefixItem; import com.yahoo.prelude.query.RankItem; import com.yahoo.prelude.query.SubstringItem; -import com.yahoo.prelude.query.SubstringItem; import com.yahoo.prelude.query.SuffixItem; import com.yahoo.prelude.query.TaggableItem; import com.yahoo.prelude.query.WordItem; -import com.yahoo.prelude.query.parser.SpecialTokens; +import com.yahoo.language.process.SpecialTokens; import com.yahoo.prelude.query.parser.TestLinguistics; import com.yahoo.search.Query; import org.junit.Test; -import java.util.Collections; import java.util.Iterator; import static org.junit.Assert.assertEquals; @@ -1639,7 +1637,7 @@ public class ParseTestCase { @Test public void testNonSpecialTokenParsing() { - ParsingTester customTester = new ParsingTester(new SpecialTokens("default")); + ParsingTester customTester = new ParsingTester(SpecialTokens.empty()); customTester.assertParsed("OR c or c with (AND tcp ip)", "c# or c++ with tcp/ip", Query.Type.ANY); } diff --git a/container-search/src/test/java/com/yahoo/prelude/query/parser/test/ParsingTester.java b/container-search/src/test/java/com/yahoo/prelude/query/parser/test/ParsingTester.java index 17155fff5de..fd7e4cbe0e6 100644 --- a/container-search/src/test/java/com/yahoo/prelude/query/parser/test/ParsingTester.java +++ b/container-search/src/test/java/com/yahoo/prelude/query/parser/test/ParsingTester.java @@ -11,8 +11,8 @@ import com.yahoo.prelude.IndexFacts; import com.yahoo.prelude.IndexModel; import com.yahoo.prelude.query.Item; import com.yahoo.prelude.query.NullItem; -import com.yahoo.prelude.query.parser.SpecialTokenRegistry; -import com.yahoo.prelude.query.parser.SpecialTokens; +import com.yahoo.language.process.SpecialTokenRegistry; +import com.yahoo.language.process.SpecialTokens; import com.yahoo.search.Query; import com.yahoo.search.config.IndexInfoConfig; import com.yahoo.search.query.parser.Parsable; @@ -20,6 +20,9 @@ import com.yahoo.search.query.parser.Parser; import com.yahoo.search.query.parser.ParserEnvironment; import com.yahoo.search.query.parser.ParserFactory; +import java.util.ArrayList; +import java.util.List; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -32,7 +35,7 @@ import static org.junit.Assert.assertTrue; public class ParsingTester { private static final Linguistics linguistics = new SimpleLinguistics(); - private IndexFacts indexFacts; + private final IndexFacts indexFacts; private SpecialTokenRegistry tokenRegistry; public ParsingTester() { @@ -49,11 +52,10 @@ public class ParsingTester { public ParsingTester(IndexFacts indexFacts, SpecialTokens specialTokens) { indexFacts.freeze(); - specialTokens.freeze(); this.indexFacts = indexFacts; tokenRegistry = new SpecialTokenRegistry(); - tokenRegistry.addSpecialTokens(specialTokens); + tokenRegistry = new SpecialTokenRegistry(List.of(specialTokens)); } /** @@ -72,13 +74,13 @@ public class ParsingTester { * This can be used to add new tokens and passing the resulting special tokens to the constructor of this. */ public static SpecialTokens createSpecialTokens() { - SpecialTokens tokens = new SpecialTokens("default"); - tokens.addSpecialToken("c++", null); - tokens.addSpecialToken(".net", "dotnet"); - tokens.addSpecialToken("tcp/ip", null); - tokens.addSpecialToken("c#", null); - tokens.addSpecialToken("special-token-fs","firstsecond"); - return tokens; + List<SpecialTokens.Token> tokens = new ArrayList<>(); + tokens.add(new SpecialTokens.Token("c++")); + tokens.add(new SpecialTokens.Token(".net", "dotnet")); + tokens.add(new SpecialTokens.Token("tcp/ip")); + tokens.add(new SpecialTokens.Token("c#")); + tokens.add(new SpecialTokens.Token("special-token-fs","firstsecond")); + return new SpecialTokens("default", tokens); } /** diff --git a/container-search/src/test/java/com/yahoo/prelude/query/parser/test/TokenizerTestCase.java b/container-search/src/test/java/com/yahoo/prelude/query/parser/test/TokenizerTestCase.java index aa2e9dbcf75..e10fbd71c72 100644 --- a/container-search/src/test/java/com/yahoo/prelude/query/parser/test/TokenizerTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/query/parser/test/TokenizerTestCase.java @@ -6,12 +6,13 @@ import com.yahoo.prelude.Index; import com.yahoo.prelude.IndexFacts; import com.yahoo.prelude.IndexModel; import com.yahoo.prelude.SearchDefinition; -import com.yahoo.prelude.query.parser.SpecialTokenRegistry; -import com.yahoo.prelude.query.parser.SpecialTokens; +import com.yahoo.language.process.SpecialTokenRegistry; +import com.yahoo.language.process.SpecialTokens; import com.yahoo.prelude.query.parser.Token; import com.yahoo.prelude.query.parser.Tokenizer; import org.junit.Test; +import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -39,13 +40,11 @@ import static org.junit.Assert.assertTrue; */ public class TokenizerTestCase { - private SpecialTokenRegistry defaultRegistry = new SpecialTokenRegistry("file:src/test/java/com/yahoo/prelude/query/parser/test/replacingtokens.cfg"); - @Test public void testPlainTokenization() { Tokenizer tokenizer = new Tokenizer(new SimpleLinguistics()); - tokenizer.setSpecialTokens(createSpecialTokens()); + tokenizer.setSpecialTokens(createSpecialTokens().getSpecialTokens("default")); List<?> tokens = tokenizer.tokenize("drive (to hwy88, 88) +or language:en ugcapi_1 & &a"); assertEquals(new Token(WORD, "drive"), tokens.get(0)); @@ -87,7 +86,7 @@ public class TokenizerTestCase { public void testOneSpecialToken() { Tokenizer tokenizer = new Tokenizer(new SimpleLinguistics()); - tokenizer.setSpecialTokens(createSpecialTokens()); + tokenizer.setSpecialTokens(createSpecialTokens().getSpecialTokens("default")); List<?> tokens = tokenizer.tokenize("c++ lovers, please apply"); assertEquals(new Token(WORD, "c++"), tokens.get(0)); @@ -97,7 +96,7 @@ public class TokenizerTestCase { public void testSpecialTokenCombination() { Tokenizer tokenizer = new Tokenizer(new SimpleLinguistics()); - tokenizer.setSpecialTokens(createSpecialTokens()); + tokenizer.setSpecialTokens(createSpecialTokens().getSpecialTokens("default")); List<?> tokens = tokenizer.tokenize("c#, c++ or .net know, not tcp/ip"); assertEquals(new Token(WORD, "c#"), tokens.get(0)); @@ -123,10 +122,9 @@ public class TokenizerTestCase { */ @Test public void testSpecialTokenCJK() { - assertEquals("Special tokens configured", 6, defaultRegistry.getSpecialTokens("default").size()); Tokenizer tokenizer = new Tokenizer(new SimpleLinguistics()); tokenizer.setSubstringSpecialTokens(true); - tokenizer.setSpecialTokens(defaultRegistry.getSpecialTokens("default")); + tokenizer.setSpecialTokens(createSpecialTokens().getSpecialTokens("replacing")); List<?> tokens = tokenizer.tokenize("fooc#bar,c++with spacebarknowknowknow,knowknownot know"); assertEquals(new Token(WORD, "foo"), tokens.get(0)); @@ -151,7 +149,7 @@ public class TokenizerTestCase { public void testSpecialTokenCaseInsensitive() { Tokenizer tokenizer = new Tokenizer(new SimpleLinguistics()); - tokenizer.setSpecialTokens(createSpecialTokens()); + tokenizer.setSpecialTokens(createSpecialTokens().getSpecialTokens("default")); List<?> tokens = tokenizer.tokenize("The AS/400 is great"); assertEquals(new Token(WORD, "The"), tokens.get(0)); @@ -167,7 +165,7 @@ public class TokenizerTestCase { public void testSpecialTokenNonMatch() { Tokenizer tokenizer = new Tokenizer(new SimpleLinguistics()); - tokenizer.setSpecialTokens(createSpecialTokens()); + tokenizer.setSpecialTokens(createSpecialTokens().getSpecialTokens("default")); List<?> tokens = tokenizer.tokenize("c++ c+ aS/400 i/o .net i/ooo ap.net"); assertEquals(new Token(WORD, "c++"), tokens.get(0)); @@ -190,18 +188,9 @@ public class TokenizerTestCase { @Test public void testSpecialTokenConfigurationDefault() { - String tokenFile = "file:src/test/java/com/yahoo/prelude/query/parser/test/specialtokens.cfg"; - - SpecialTokenRegistry r = new SpecialTokenRegistry(tokenFile); - assertEquals("Special tokens configured", 6, - r.getSpecialTokens("default").size()); - assertEquals("Special tokens configured", 4, - r.getSpecialTokens("other").size()); - Tokenizer tokenizer = new Tokenizer(new SimpleLinguistics()); - tokenizer.setSpecialTokens( - r.getSpecialTokens("default")); + tokenizer.setSpecialTokens(createSpecialTokens().getSpecialTokens("default")); List<?> tokens = tokenizer.tokenize( "with space, c++ or .... know, not b.s.d."); @@ -224,18 +213,9 @@ public class TokenizerTestCase { @Test public void testSpecialTokenConfigurationOther() { - String tokenFile = "file:src/test/java/com/yahoo/prelude/query/parser/test/specialtokens.cfg"; - - SpecialTokenRegistry r = new SpecialTokenRegistry(tokenFile); - assertEquals("Special tokens configured", 6, - r.getSpecialTokens("default").size()); - assertEquals("Special tokens configured", 4, - r.getSpecialTokens("other").size()); - Tokenizer tokenizer = new Tokenizer(new SimpleLinguistics()); - tokenizer.setSpecialTokens( - r.getSpecialTokens("other")); + tokenizer.setSpecialTokens(createSpecialTokens().getSpecialTokens("other")); List<?> tokens = tokenizer.tokenize( "with space,!!!*** [huh] or ------ " + "know, &&&%%% b.s.d."); @@ -267,26 +247,9 @@ public class TokenizerTestCase { } @Test - public void testSpecialTokenConfigurationMissing() { - String tokenFile = "file:source/bogus/specialtokens.cfg"; - - SpecialTokenRegistry r = new SpecialTokenRegistry(tokenFile); - - Tokenizer tokenizer = new Tokenizer(new SimpleLinguistics()); - - tokenizer.setSpecialTokens(r.getSpecialTokens("other")); - List<?> tokens = tokenizer.tokenize("c++"); - - assertEquals(new Token(WORD, "c"), tokens.get(0)); - assertEquals(new Token(PLUS, "+"), tokens.get(1)); - assertEquals(new Token(PLUS, "+"), tokens.get(2)); - } - - @Test public void testTokenReplacing() { - assertEquals("Special tokens configured", 6, defaultRegistry.getSpecialTokens("default").size()); Tokenizer tokenizer = new Tokenizer(new SimpleLinguistics()); - tokenizer.setSpecialTokens(defaultRegistry.getSpecialTokens("default")); + tokenizer.setSpecialTokens(createSpecialTokens().getSpecialTokens("replacing")); List<?> tokens = tokenizer.tokenize("with space, c++ or .... know, not b.s.d."); assertEquals(new Token(WORD, "with-space"), tokens.get(0)); @@ -745,7 +708,7 @@ public class TokenizerTestCase { public void testSingleQuoteAsWordCharacter() { Tokenizer tokenizer = new Tokenizer(new SimpleLinguistics()); - tokenizer.setSpecialTokens(createSpecialTokens()); + tokenizer.setSpecialTokens(createSpecialTokens().getSpecialTokens("default")); List<?> tokens = tokenizer.tokenize("drive (to hwy88, 88) +or language:en nalle:a'a ugcapi_1 'a' 'a a'"); assertEquals(new Token(WORD, "drive"), tokens.get(0)); @@ -781,17 +744,38 @@ public class TokenizerTestCase { assertEquals(new Token(WORD, "a'"), tokens.get(30)); } - private SpecialTokens createSpecialTokens() { - SpecialTokens tokens = new SpecialTokens("default"); - - tokens.addSpecialToken("c+", null); - tokens.addSpecialToken("c++", null); - tokens.addSpecialToken(".net", null); - tokens.addSpecialToken("tcp/ip", null); - tokens.addSpecialToken("i/o", null); - tokens.addSpecialToken("c#", null); - tokens.addSpecialToken("AS/400", null); - return tokens; + private SpecialTokenRegistry createSpecialTokens() { + List<SpecialTokens.Token> tokens = new ArrayList<>(); + tokens.add(new SpecialTokens.Token("c+")); + tokens.add(new SpecialTokens.Token("c++")); + tokens.add(new SpecialTokens.Token(".net")); + tokens.add(new SpecialTokens.Token("tcp/ip")); + tokens.add(new SpecialTokens.Token("i/o")); + tokens.add(new SpecialTokens.Token("c#")); + tokens.add(new SpecialTokens.Token("AS/400")); + tokens.add(new SpecialTokens.Token("....")); + tokens.add(new SpecialTokens.Token("b.s.d.")); + tokens.add(new SpecialTokens.Token("with space")); + tokens.add(new SpecialTokens.Token("dvd\\xB1r")); + SpecialTokens defaultTokens = new SpecialTokens("default", tokens); + + tokens = new ArrayList<>(); + tokens.add(new SpecialTokens.Token("[huh]")); + tokens.add(new SpecialTokens.Token("&&&%%%")); + tokens.add(new SpecialTokens.Token("------")); + tokens.add(new SpecialTokens.Token("!!!***")); + SpecialTokens otherTokens = new SpecialTokens("other", tokens); + + tokens = new ArrayList<>(); + tokens.add(new SpecialTokens.Token("....")); + tokens.add(new SpecialTokens.Token("c++", "cpp")); + tokens.add(new SpecialTokens.Token("b.s.d.")); + tokens.add(new SpecialTokens.Token("with space", "with-space")); + tokens.add(new SpecialTokens.Token("c#")); + tokens.add(new SpecialTokens.Token("know", "knuwww")); + SpecialTokens replacingTokens = new SpecialTokens("replacing", tokens); + + return new SpecialTokenRegistry(List.of(defaultTokens, otherTokens, replacingTokens)); } } diff --git a/container-search/src/test/java/com/yahoo/prelude/query/parser/test/replacingtokens.cfg b/container-search/src/test/java/com/yahoo/prelude/query/parser/test/replacingtokens.cfg deleted file mode 100644 index 6a189de0164..00000000000 --- a/container-search/src/test/java/com/yahoo/prelude/query/parser/test/replacingtokens.cfg +++ /dev/null @@ -1,12 +0,0 @@ -tokenlist[1] -tokenlist[0].name default -tokenlist[0].tokens[6] -tokenlist[0].tokens[0].token .... -tokenlist[0].tokens[1].token c++ -tokenlist[0].tokens[1].replace cpp -tokenlist[0].tokens[2].token b.s.d. -tokenlist[0].tokens[3].token with space -tokenlist[0].tokens[3].replace with-space -tokenlist[0].tokens[4].token c# -tokenlist[0].tokens[5].token know -tokenlist[0].tokens[5].replace knuwww diff --git a/container-search/src/test/java/com/yahoo/search/query/rewrite/RewriterFeaturesTestCase.java b/container-search/src/test/java/com/yahoo/search/query/rewrite/RewriterFeaturesTestCase.java index 5508c2a73a7..08146bbe069 100644 --- a/container-search/src/test/java/com/yahoo/search/query/rewrite/RewriterFeaturesTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/query/rewrite/RewriterFeaturesTestCase.java @@ -8,7 +8,7 @@ import org.junit.Test; import com.yahoo.prelude.query.AndItem; import com.yahoo.prelude.query.CompositeItem; import com.yahoo.prelude.query.Item; -import com.yahoo.prelude.query.parser.SpecialTokenRegistry; +import com.yahoo.language.process.SpecialTokenRegistry; import com.yahoo.search.Query; import com.yahoo.search.searchchain.Execution; import com.yahoo.search.searchchain.Execution.Context; diff --git a/container-test/pom.xml b/container-test/pom.xml index 3eebd64cc82..ee39fa7db3f 100644 --- a/container-test/pom.xml +++ b/container-test/pom.xml @@ -39,6 +39,18 @@ <dependency> <groupId>com.fasterxml.jackson.dataformat</groupId> <artifactId>jackson-dataformat-xml</artifactId> + <exclusions> + <exclusion> + <!-- See comment in container-dev pom --> + <groupId>jakarta.xml.bind</groupId> + <artifactId>jakarta.xml.bind-api</artifactId> + </exclusion> + <exclusion> + <!-- See comment in container-dev pom --> + <groupId>jakarta.activation</groupId> + <artifactId>jakarta.activation-api</artifactId> + </exclusion> + </exclusions> </dependency> <dependency> <groupId>io.airlift</groupId> diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java index c7cc0f361bc..52ad813d7cc 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java @@ -95,7 +95,7 @@ public class RoutingController { if (!policy.status().isActive()) continue; for (var routingMethod : controller.zoneRegistry().routingMethods(policy.id().zone())) { if (routingMethod.isDirect() && !isSystemApplication && !canRouteDirectlyTo(deployment, application.get())) continue; - endpoints.add(policy.endpointIn(controller.system(), routingMethod, controller.zoneRegistry())); + endpoints.addAll(policy.endpointsIn(controller.system(), routingMethod, controller.zoneRegistry())); endpoints.add(policy.regionEndpointIn(controller.system(), routingMethod)); } } @@ -140,7 +140,7 @@ public class RoutingController { public Map<ZoneId, List<Endpoint>> zoneEndpointsOf(Collection<DeploymentId> deployments) { var endpoints = new TreeMap<ZoneId, List<Endpoint>>(Comparator.comparing(ZoneId::value)); for (var deployment : deployments) { - EndpointList zoneEndpoints = endpointsOf(deployment).scope(Endpoint.Scope.zone); + EndpointList zoneEndpoints = endpointsOf(deployment).scope(Endpoint.Scope.zone).not().legacy(); zoneEndpoints = directEndpoints(zoneEndpoints, deployment.applicationId()); if ( ! zoneEndpoints.isEmpty()) { endpoints.put(deployment.zoneId(), zoneEndpoints.asList()); @@ -189,9 +189,7 @@ public class RoutingController { /** Returns the global endpoints for given deployment as container endpoints */ public Set<ContainerEndpoint> containerEndpointsOf(Application application, InstanceName instanceName, ZoneId zone) { Instance instance = application.require(instanceName); - boolean registerLegacyNames = application.deploymentSpec().instance(instanceName) - .flatMap(DeploymentInstanceSpec::globalServiceId) - .isPresent(); + boolean registerLegacyNames = legacyNamesAvailable(application, instanceName); Set<ContainerEndpoint> containerEndpoints = new HashSet<>(); EndpointList endpoints = endpointsOf(application, instanceName); // Add endpoints backed by a rotation, and register them in DNS if necessary @@ -305,6 +303,7 @@ public class RoutingController { var directMethods = 0; var zones = deployments.stream().map(DeploymentId::zoneId).collect(Collectors.toList()); var availableRoutingMethods = routingMethodsOfAll(deployments, application); + boolean legacyNamesAvailable = legacyNamesAvailable(application, routingId.application().instance()); for (var method : availableRoutingMethods) { if (method.isDirect() && ++directMethods > 1) { @@ -316,8 +315,8 @@ public class RoutingController { .on(Port.fromRoutingMethod(method)) .routingMethod(method) .in(controller.system())); - // TODO(mpolden): Remove this once all applications have migrated away from legacy endpoints - if (method == RoutingMethod.shared) { + // Add legacy endpoints + if (legacyNamesAvailable && method == RoutingMethod.shared) { endpoints.add(Endpoint.of(routingId.application()) .target(routingId.endpointId(), cluster, zones) .on(Port.plain(4080)) @@ -335,6 +334,13 @@ public class RoutingController { return endpoints; } + /** Whether legacy global DNS names should be available for given application */ + private static boolean legacyNamesAvailable(Application application, InstanceName instanceName) { + return application.deploymentSpec().instance(instanceName) + .flatMap(DeploymentInstanceSpec::globalServiceId) + .isPresent(); + } + /** Returns direct routing endpoints if any exist and feature flag is set for given application */ // TODO: Remove this when feature flag is removed, and in-line .direct() filter where relevant public EndpointList directEndpoints(EndpointList endpoints, ApplicationId application) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java index 04b06b3e1f6..e1618f05a7d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java @@ -27,7 +27,7 @@ import java.util.stream.Collectors; public class OsUpgradeScheduler extends ControllerMaintainer { /** Trigger a new upgrade when the current target version reaches this age */ - private static final Duration MAX_VERSION_AGE = Duration.ofDays(30); + private static final Duration MAX_VERSION_AGE = Duration.ofDays(45); /** * The interval at which new versions become available. We use this to avoid scheduling upgrades to a version that diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/TenantRoleMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/TenantRoleMaintainer.java index e3a7d91d7cc..1265d687850 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/TenantRoleMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/TenantRoleMaintainer.java @@ -13,6 +13,7 @@ import com.yahoo.vespa.hosted.controller.tenant.Tenant; import java.time.Duration; import java.util.List; import java.util.Map; +import java.util.function.Predicate; import java.util.stream.Collectors; public class TenantRoleMaintainer extends ControllerMaintainer { @@ -36,6 +37,6 @@ public class TenantRoleMaintainer extends ControllerMaintainer { private boolean hasProductionDeployment(TenantName tenant) { return controller().applications().asList(tenant).stream() .map(Application::productionInstances) - .noneMatch(Map::isEmpty); + .anyMatch(Predicate.not(Map::isEmpty)); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java index 112b0fbc04d..e6435178d08 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java @@ -49,8 +49,9 @@ public class NotificationsDb { try (Lock lock = curatorDb.lockNotifications(tenant)) { List<Notification> initial = curatorDb.readNotifications(tenant); List<Notification> filtered = initial.stream() - .filter(notification -> notification.type() == Type.applicationPackage && - notification.source().instance().isPresent() && notification.source().zoneId().isEmpty()) + .filter(notification -> notification.type() != Type.applicationPackage || + notification.source().instance().isEmpty() || + notification.source().zoneId().isPresent()) .collect(Collectors.toUnmodifiableList()); if (initial.size() > filtered.size()) curatorDb.writeNotifications(tenant, filtered); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java index 898b2531460..0d1469a61fc 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java @@ -206,10 +206,11 @@ public class RoutingPolicies { /** Update zone DNS record for given policy */ private void updateZoneDnsOf(RoutingPolicy policy) { - var name = RecordName.from(policy.endpointIn(controller.system(), RoutingMethod.exclusive, controller.zoneRegistry()) - .dnsName()); - var data = RecordData.fqdn(policy.canonicalName().value()); - nameServiceForwarderIn(policy.id().zone()).createCname(name, data, Priority.normal); + for (var endpoint : policy.endpointsIn(controller.system(), RoutingMethod.exclusive, controller.zoneRegistry())) { + var name = RecordName.from(endpoint.dnsName()); + var data = RecordData.fqdn(policy.canonicalName().value()); + nameServiceForwarderIn(policy.id().zone()).createCname(name, data, Priority.normal); + } } /** Remove policies and zone DNS records unreferenced by given load balancers */ @@ -221,11 +222,12 @@ public class RoutingPolicies { // Leave active load balancers and irrelevant zones alone if (activeIds.contains(policy.id()) || !policy.id().zone().equals(allocation.deployment.zoneId())) continue; - - var dnsName = policy.endpointIn(controller.system(), RoutingMethod.exclusive, controller.zoneRegistry()).dnsName(); - nameServiceForwarderIn(allocation.deployment.zoneId()).removeRecords(Record.Type.CNAME, - RecordName.from(dnsName), - Priority.normal); + for (var endpoint : policy.endpointsIn(controller.system(), RoutingMethod.exclusive, controller.zoneRegistry())) { + var dnsName = endpoint.dnsName(); + nameServiceForwarderIn(allocation.deployment.zoneId()).removeRecords(Record.Type.CNAME, + RecordName.from(dnsName), + Priority.normal); + } newPolicies.remove(policy.id()); } db.writeRoutingPolicies(allocation.deployment.applicationId(), newPolicies); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java index a0fecbdf9e1..3ece10337f1 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java @@ -11,6 +11,8 @@ import com.yahoo.vespa.hosted.controller.application.Endpoint.Port; import com.yahoo.vespa.hosted.controller.application.EndpointId; import com.yahoo.vespa.hosted.controller.application.SystemApplication; +import java.util.ArrayList; +import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -69,12 +71,28 @@ public class RoutingPolicy { return new RoutingPolicy(id, canonicalName, dnsZone, endpoints, status); } - /** Returns the zone endpoint of this */ - public Endpoint endpointIn(SystemName system, RoutingMethod routingMethod, ZoneRegistry zoneRegistry) { + /** Returns the zone endpoints of this */ + public List<Endpoint> endpointsIn(SystemName system, RoutingMethod routingMethod, ZoneRegistry zoneRegistry) { Optional<Endpoint> infraEndpoint = SystemApplication.matching(id.owner()) .flatMap(app -> app.endpointIn(id.zone(), zoneRegistry)); - return infraEndpoint.orElseGet(() -> endpoint(routingMethod).target(id.cluster(), id.zone()) - .in(system)); + List<Endpoint> endpoints = new ArrayList<>(3); + if (infraEndpoint.isPresent()) { + endpoints.add(infraEndpoint.get()); + } else { + endpoints.add(endpoint(routingMethod).target(id.cluster(), id.zone()).in(system)); + // Add legacy endpoints + if (routingMethod == RoutingMethod.shared) { + endpoints.add(endpoint(routingMethod).target(id.cluster(), id.zone()) + .on(Port.plain(4080)) + .legacy() + .in(system)); + endpoints.add(endpoint(routingMethod).target(id.cluster(), id.zone()) + .on(Port.tls(4443)) + .legacy() + .in(system)); + } + } + return endpoints; } /** Returns the region endpoint of this */ diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index 0c0d1d80adb..422c856ca01 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -197,7 +197,7 @@ public class ControllerTest { } @Test - public void testGlobalRotations() { + public void testGlobalRotationStatus() { var context = tester.newDeploymentContext(); var zone1 = ZoneId.from("prod", "us-west-1"); var zone2 = ZoneId.from("prod", "us-east-3"); @@ -229,7 +229,7 @@ public class ControllerTest { } @Test - public void testDnsAliasRegistration() { + public void testDnsUpdatesForGlobalEndpoint() { var context = tester.newDeploymentContext("tenant1", "app1", "default"); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .endpoint("default", "foo") @@ -254,10 +254,15 @@ public class ControllerTest { assertTrue(record.isPresent()); assertEquals("app1--tenant1.global.vespa.oath.cloud", record.get().name().asString()); assertEquals("rotation-fqdn-01.", record.get().data().asString()); + + List<String> globalDnsNames = tester.controller().routing().endpointsOf(context.instanceId()) + .scope(Endpoint.Scope.global) + .mapToList(Endpoint::dnsName); + assertEquals(List.of("app1--tenant1.global.vespa.oath.cloud"), globalDnsNames); } @Test - public void testDnsAliasRegistrationLegacy() { + public void testDnsUpdatesForGlobalEndpointLegacySyntax() { var context = tester.newDeploymentContext("tenant1", "app1", "default"); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .globalServiceId("foo") @@ -293,10 +298,18 @@ public class ControllerTest { assertTrue(record.isPresent()); assertEquals("app1.tenant1.global.vespa.yahooapis.com", record.get().name().asString()); assertEquals("rotation-fqdn-01.", record.get().data().asString()); + + List<String> globalDnsNames = tester.controller().routing().endpointsOf(context.instanceId()) + .scope(Endpoint.Scope.global) + .mapToList(Endpoint::dnsName); + assertEquals(List.of("app1--tenant1.global.vespa.oath.cloud", + "app1.tenant1.global.vespa.yahooapis.com", + "app1--tenant1.global.vespa.yahooapis.com"), + globalDnsNames); } @Test - public void testDnsAliasRegistrationWithEndpoints() { + public void testDnsUpdatesForMultipleGlobalEndpoints() { var context = tester.newDeploymentContext("tenant1", "app1", "default"); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .endpoint("foobar", "qrs", "us-west-1", "us-central-1") // Rotation 01 @@ -349,7 +362,7 @@ public class ControllerTest { } @Test - public void testDnsAliasRegistrationWithChangingEndpoints() { + public void testDnsUpdatesForGlobalEndpointChanges() { var context = tester.newDeploymentContext("tenant1", "app1", "default"); var west = ZoneId.from("prod", "us-west-1"); var central = ZoneId.from("prod", "us-central-1"); @@ -491,7 +504,7 @@ public class ControllerTest { } @Test - public void testUpdatesExistingDnsAlias() { + public void testDnsUpdatesWithChangeInRotationAssignment() { // Application 1 is deployed and deleted { var context = tester.newDeploymentContext("tenant1", "app1", "default"); @@ -719,7 +732,7 @@ public class ControllerTest { } @Test - public void testDeployWithCrossCloudEndpoints() { + public void testDeployWithGlobalEndpointsInMultipleClouds() { tester.controllerTester().zoneRegistry().setZones( ZoneApiMock.fromId("prod.us-west-1"), ZoneApiMock.newBuilder().with(CloudName.from("aws")).withId("prod.aws-us-east-1").build() @@ -896,6 +909,14 @@ public class ControllerTest { "application--tenant.global.vespa.oath.cloud"), tester.configServer().containerEndpoints().get(context.deploymentIdIn(zone))); } + List<String> zoneDnsNames = tester.controller().routing().endpointsOf(context.deploymentIdIn(zone1)) + .scope(Endpoint.Scope.zone) + .mapToList(Endpoint::dnsName); + assertEquals(List.of("application--tenant.us-west-1.vespa.oath.cloud", + "application.tenant.us-west-1.prod.vespa.yahooapis.com", + "application--tenant.us-west-1.prod.vespa.yahooapis.com", + "application.tenant.us-west-1.vespa.oath.cloud"), + zoneDnsNames); } @Test diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeSchedulerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeSchedulerTest.java index 76781d964a1..7d512ba090c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeSchedulerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeSchedulerTest.java @@ -40,14 +40,14 @@ public class OsUpgradeSchedulerTest { tester.controller().upgradeOsIn(cloud, version0, Duration.ofDays(1), false); // Target remains unchanged as it hasn't expired yet - for (var interval : List.of(Duration.ZERO, Duration.ofDays(15))) { + for (var interval : List.of(Duration.ZERO, Duration.ofDays(30))) { tester.clock().advance(interval); scheduler.maintain(); assertEquals(version0, tester.controller().osVersionTarget(cloud).get().osVersion().version()); } - // Just over 30 days pass, and a new target replaces the expired one - Version version1 = Version.fromString("7.0.0.20210215"); + // Just over 45 days pass, and a new target replaces the expired one + Version version1 = Version.fromString("7.0.0.20210302"); tester.clock().advance(Duration.ofDays(15).plus(Duration.ofSeconds(1))); scheduler.maintain(); assertEquals("New target set", version1, tester.controller().osVersionTarget(cloud).get().osVersion().version()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/TenantRoleMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/TenantRoleMaintainerTest.java index c023ec9b43d..050610905f3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/TenantRoleMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/TenantRoleMaintainerTest.java @@ -25,19 +25,22 @@ public class TenantRoleMaintainerTest { @Test public void maintains_iam_roles_for_tenants_in_production() { - var devApp = tester.newDeploymentContext("tenant1", "app1", "default"); - var prodApp = tester.newDeploymentContext("tenant2", "app2", "default"); + var devAppTenant1 = tester.newDeploymentContext("tenant1", "app1", "default"); + var prodAppTenant2 = tester.newDeploymentContext("tenant2", "app2", "default"); + var devAppTenant2 = tester.newDeploymentContext("tenant2","app3","default"); ApplicationPackage appPackage = new ApplicationPackageBuilder() .region("us-west-1") .build(); - // Deploy dev - devApp.runJob(JobType.devUsEast1, appPackage); + // Deploy dev apps + devAppTenant1.runJob(JobType.devUsEast1, appPackage); + devAppTenant2.runJob(JobType.devUsEast1, appPackage); // Deploy prod - prodApp.submit(appPackage).deploy(); - assertEquals(1, permanentDeployments(devApp.instance())); - assertEquals(1, permanentDeployments(prodApp.instance())); + prodAppTenant2.submit(appPackage).deploy(); + assertEquals(1, permanentDeployments(devAppTenant1.instance())); + assertEquals(1, permanentDeployments(devAppTenant2.instance())); + assertEquals(1, permanentDeployments(prodAppTenant2.instance())); var maintainer = new TenantRoleMaintainer(tester.controller(), Duration.ofDays(1)); maintainer.maintain(); @@ -46,7 +49,7 @@ public class TenantRoleMaintainerTest { List<TenantName> tenantNames = ((MockRoleService) roleService).maintainedTenants(); assertEquals(1, tenantNames.size()); - assertEquals(prodApp.application().id().tenant(), tenantNames.get(0)); + assertEquals(prodAppTenant2.application().id().tenant(), tenantNames.get(0)); } private long permanentDeployments(Instance instance) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java index 5bd7d1db769..9f07f784dbf 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java @@ -50,6 +50,19 @@ public class NotificationsDbTest { private final NotificationsDb notificationsDb = new NotificationsDb(clock, curatorDb); @Test + public void clears_old_application_package_notifications() { + List<Notification> allNotifications = new ArrayList<>(notifications); + allNotifications.add(notification(1601, Type.applicationPackage, Level.warning, NotificationSource.from(ApplicationId.from("tenant1", "app1", "instance1")), "msg")); + allNotifications.add(notification(1701, Type.applicationPackage, Level.warning, NotificationSource.from(new DeploymentId(ApplicationId.from("tenant1", "app1", "instance1"), ZoneId.from("dev", "us-south-3"))), "msg")); + curatorDb.writeNotifications(tenant, allNotifications); + + assertEquals(allNotifications, notificationsDb.listNotifications(NotificationSource.from(tenant), false)); + notificationsDb.removeApplicationPackageNotificationsWithInstanceSource(); + allNotifications.remove(6); + assertEquals(allNotifications, notificationsDb.listNotifications(NotificationSource.from(tenant), false)); + } + + @Test public void list_test() { assertEquals(notifications, notificationsDb.listNotifications(NotificationSource.from(tenant), false)); assertEquals(notificationIndices(0, 1, 2, 3), notificationsDb.listNotifications(NotificationSource.from(tenant), true)); diff --git a/dist/vespa.spec b/dist/vespa.spec index ff9f56e0b07..c9c81cf870a 100644 --- a/dist/vespa.spec +++ b/dist/vespa.spec @@ -28,12 +28,12 @@ Source0: vespa-%{version}.tar.gz %if 0%{?centos} BuildRequires: epel-release -%if 0%{?el7} && ! 0%{?amzn:1} +%if 0%{?el7} && ! 0%{?amzn2} BuildRequires: centos-release-scl %endif %endif %if 0%{?el7} -%if 0%{?amzn} +%if 0%{?amzn2} BuildRequires: gcc10-c++ BuildRequires: libatomic10-devel BuildRequires: gcc10-binutils @@ -79,7 +79,7 @@ BuildRequires: vespa-lz4-devel >= 1.9.2-2 BuildRequires: vespa-onnxruntime-devel = 1.7.1 BuildRequires: vespa-openssl-devel >= 1.1.1k-1 %if 0%{?centos} -%if 0%{?amzn} +%if 0%{?amzn2} BuildRequires: vespa-protobuf-devel = 3.7.0-5.amzn2 %else BuildRequires: vespa-protobuf-devel = 3.7.0-4.el7 @@ -140,7 +140,7 @@ BuildRequires: gtest-devel BuildRequires: gmock-devel %endif %endif -%if 0%{?el7} && 0%{?amzn:1} +%if 0%{?el7} && 0%{?amzn2} BuildRequires: vespa-xxhash-devel = 0.8.0 BuildRequires: vespa-openblas-devel = 0.3.12 %else @@ -152,7 +152,7 @@ BuildRequires: re2-devel %if ! 0%{?el7} BuildRequires: libicu-devel %endif -%if 0%{?el7} && 0%{?amzn:1} +%if 0%{?el7} && 0%{?amzn2} BuildRequires: java-11-amazon-corretto %else BuildRequires: java-11-openjdk-devel @@ -188,7 +188,7 @@ Requires: perl-URI %if ! 0%{?el7} Requires: valgrind %endif -%if 0%{?el7} && 0%{?amzn:1} +%if 0%{?el7} && 0%{?amzn2} Requires: vespa-xxhash = 0.8.0 %else Requires: xxhash @@ -197,7 +197,7 @@ Requires: xxhash-libs >= 0.8.0 %if 0%{?el8} Requires: openblas %else -%if 0%{?el7} && 0%{?amzn:1} +%if 0%{?el7} && 0%{?amzn2} Requires: vespa-openblas %else Requires: openblas-serial @@ -222,7 +222,7 @@ Requires: vespa-lz4 >= 1.9.2-2 Requires: vespa-onnxruntime = 1.7.1 Requires: vespa-openssl >= 1.1.1k-1 %if 0%{?centos} -%if 0%{?amzn} +%if 0%{?amzn2} Requires: vespa-protobuf = 3.7.0-5.amzn2 %else Requires: vespa-protobuf = 3.7.0-4.el7 @@ -230,14 +230,14 @@ Requires: vespa-protobuf = 3.7.0-4.el7 %else Requires: vespa-protobuf = 3.7.0-5.el7 %endif -%if ! 0%{?amzn} +%if ! 0%{?amzn2} Requires: vespa-telegraf >= 1.1.1-1 -Requires: vespa-valgrind >= 3.16.0-1 +Requires: vespa-valgrind >= 3.17.0-1 %endif Requires: vespa-zstd >= 1.4.5-2 %define _vespa_llvm_version 7 %define _extra_link_directory /usr/lib64/llvm7.0/lib;%{_vespa_deps_prefix}/lib64 -%if 0%{?amzn} +%if 0%{?amzn2} %define _extra_include_directory /usr/include/llvm7.0;%{_vespa_deps_prefix}/include %else %define _extra_include_directory /usr/include/llvm7.0;%{_vespa_deps_prefix}/include;/usr/include/openblas @@ -302,7 +302,7 @@ Requires: %{name}-tools = %{version}-%{release} # Ugly workaround because vespamalloc/src/vespamalloc/malloc/mmap.cpp uses the private # _dl_sym function. Exclude automated reqires for libraries in /opt/vespa-deps/lib64. -%if 0%{?amzn2:1} +%if 0%{?amzn2} %global __requires_exclude ^lib(c\\.so\\.6\\(GLIBC_PRIVATE\\)|pthread\\.so\\.0\\(GLIBC_PRIVATE\\)|(crypto|icui18n|icuuc|lz4|protobuf|ssl|zstd|onnxruntime|openblas|re2|xxhash)\\.so\\.[0-9.]*\\([A-Z._0-9]*\\))\\(64bit\\)$ %else %global __requires_exclude ^lib(c\\.so\\.6\\(GLIBC_PRIVATE\\)|pthread\\.so\\.0\\(GLIBC_PRIVATE\\)|(crypto|icui18n|icuuc|lz4|protobuf|ssl|zstd|onnxruntime)\\.so\\.[0-9.]*\\([A-Z._0-9]*\\))\\(64bit\\)$ @@ -317,7 +317,7 @@ Vespa - The open big data serving engine Summary: Vespa - The open big data serving engine - base -%if 0%{?el7} && 0%{?amzn:1} +%if 0%{?el7} && 0%{?amzn2} Requires: java-11-amazon-corretto %else Requires: java-11-openjdk-devel @@ -334,7 +334,7 @@ Vespa - The open big data serving engine - base Summary: Vespa - The open big data serving engine - base C++ libs -%if 0%{?el7} && 0%{?amzn:1} +%if 0%{?el7} && 0%{?amzn2} Requires: vespa-xxhash = 0.8.0 %else Requires: xxhash-libs >= 0.8.0 diff --git a/document/src/test/java/com/yahoo/document/serialization/SerializationTestUtils.java b/document/src/test/java/com/yahoo/document/serialization/SerializationTestUtils.java index 25021b0d2f8..951ee802e58 100644 --- a/document/src/test/java/com/yahoo/document/serialization/SerializationTestUtils.java +++ b/document/src/test/java/com/yahoo/document/serialization/SerializationTestUtils.java @@ -12,6 +12,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardOpenOption; +import java.nio.file.StandardCopyOption; import static org.junit.Assert.assertEquals; @@ -50,8 +51,10 @@ public class SerializationTestUtils { public static void assertSerializationMatchesCpp(String binaryFilesFolder, String fileName, Document document, TestDocumentFactory factory) throws IOException { byte[] buf = serializeDocument(document); - Files.write(Paths.get(binaryFilesFolder, fileName + "__java"), buf, - StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING); + Files.write(Paths.get(binaryFilesFolder, fileName + "__java.new"), buf, + StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING); + Files.move(Paths.get(binaryFilesFolder, fileName + "__java.new"), + Paths.get(binaryFilesFolder, fileName + "__java"), StandardCopyOption.ATOMIC_MOVE); assertDeserializeFromFile(Paths.get(binaryFilesFolder, fileName + "__java"), document, factory); assertDeserializeFromFile(Paths.get(binaryFilesFolder, fileName + "__cpp"), document, factory); diff --git a/document/src/test/resources/.gitattributes b/document/src/test/resources/.gitattributes new file mode 100644 index 00000000000..7ab6af7a4f4 --- /dev/null +++ b/document/src/test/resources/.gitattributes @@ -0,0 +1,2 @@ +*__cpp binary +*__java binary diff --git a/document/src/test/resources/reference/reference_with_id__cpp b/document/src/test/resources/reference/reference_with_id__cpp Binary files differindex d2668f8a5b1..6e9158827d7 100644 --- a/document/src/test/resources/reference/reference_with_id__cpp +++ b/document/src/test/resources/reference/reference_with_id__cpp diff --git a/document/src/tests/serialization/vespadocumentserializer_test.cpp b/document/src/tests/serialization/vespadocumentserializer_test.cpp index 0c9dfaf2e56..60ec692e078 100644 --- a/document/src/tests/serialization/vespadocumentserializer_test.cpp +++ b/document/src/tests/serialization/vespadocumentserializer_test.cpp @@ -713,7 +713,9 @@ void checkDeserialization(const string &name, std::unique_ptr<Slime> slime) { const string data_dir = TEST_PATH("../../test/resources/predicates/"); PredicateFieldValue value(std::move(slime)); - serializeToFile(value, data_dir + name + "__cpp"); + serializeToFile(value, data_dir + name + "__cpp.new"); + vespalib::rename(data_dir + name + "__cpp.new", + data_dir + name + "__cpp"); deserializeAndCheck(data_dir + name + "__cpp", value); deserializeAndCheck(data_dir + name + "__java", value); @@ -841,7 +843,10 @@ void checkDeserialization(const string &name, std::unique_ptr<vespalib::eval::Va if (tensor) { value = std::move(tensor); } - serializeToFile(value, data_dir + name + "__cpp"); + serializeToFile(value, data_dir + name + "__cpp.new"); + vespalib::rename(data_dir + name + "__cpp.new", + data_dir + name + "__cpp"); + deserializeAndCheck(data_dir + name + "__cpp", value); deserializeAndCheck(data_dir + name + "__java", value); } @@ -966,8 +971,10 @@ struct RefFixture { const ReferenceFieldValue& value) { const string data_dir = TEST_PATH("../../test/resources/reference/"); const string field_name = "ref_field"; - serializeToFile(value, data_dir + file_base_name + "__cpp", + serializeToFile(value, data_dir + file_base_name + "__cpp.new", ref_doc_type, field_name); + vespalib::rename(data_dir + file_base_name + "__cpp.new", + data_dir + file_base_name + "__cpp"); deserializeAndCheck(data_dir + file_base_name + "__cpp", value, fixed_repo, field_name); 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 f7b8434ca8e..10608b46826 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -225,13 +225,6 @@ public class Flags { "Takes effect at redeployment", APPLICATION_ID); - public static final UnboundBooleanFlag UPGRADE_DELL_SSD_FIRMWARE = defineFeatureFlag( - "upgrade_dell_ssd_firmware", false, - List.of("andreer"), "2021-04-13", "2021-05-13", - "Whether to consider upgrading Dell SSD firmware", - "Takes effect on next host-admin tick", - HOSTNAME); - public static final UnboundIntFlag NUM_DISTRIBUTOR_STRIPES = defineIntFlag( "num-distributor-stripes", 0, List.of("geirst", "vekterli"), "2021-04-20", "2021-07-01", diff --git a/linguistics/abi-spec.json b/linguistics/abi-spec.json index 58b838d7332..b77b03664d4 100644 --- a/linguistics/abi-spec.json +++ b/linguistics/abi-spec.json @@ -427,6 +427,57 @@ ], "fields": [] }, + "com.yahoo.language.process.SpecialTokenRegistry": { + "superClass": "java.lang.Object", + "interfaces": [], + "attributes": [ + "public" + ], + "methods": [ + "public void <init>()", + "public void <init>(com.yahoo.vespa.configdefinition.SpecialtokensConfig)", + "public void <init>(java.util.List)", + "public com.yahoo.language.process.SpecialTokens getSpecialTokens(java.lang.String)" + ], + "fields": [] + }, + "com.yahoo.language.process.SpecialTokens$Token": { + "superClass": "java.lang.Object", + "interfaces": [ + "java.lang.Comparable" + ], + "attributes": [ + "public", + "final" + ], + "methods": [ + "public void <init>(java.lang.String)", + "public void <init>(java.lang.String, java.lang.String)", + "public java.lang.String token()", + "public java.lang.String replacement()", + "public int compareTo(com.yahoo.language.process.SpecialTokens$Token)", + "public boolean equals(java.lang.Object)", + "public int hashCode()", + "public java.lang.String toString()", + "public bridge synthetic int compareTo(java.lang.Object)" + ], + "fields": [] + }, + "com.yahoo.language.process.SpecialTokens": { + "superClass": "java.lang.Object", + "interfaces": [], + "attributes": [ + "public" + ], + "methods": [ + "public void <init>(java.lang.String, java.util.List)", + "public java.lang.String name()", + "public java.util.Map asMap()", + "public com.yahoo.language.process.SpecialTokens$Token tokenize(java.lang.String, boolean)", + "public static com.yahoo.language.process.SpecialTokens empty()" + ], + "fields": [] + }, "com.yahoo.language.process.StemList": { "superClass": "java.util.AbstractList", "interfaces": [], diff --git a/linguistics/src/main/java/com/yahoo/language/opennlp/OpenNlpTokenizer.java b/linguistics/src/main/java/com/yahoo/language/opennlp/OpenNlpTokenizer.java index e1185cb2457..73518876c3f 100644 --- a/linguistics/src/main/java/com/yahoo/language/opennlp/OpenNlpTokenizer.java +++ b/linguistics/src/main/java/com/yahoo/language/opennlp/OpenNlpTokenizer.java @@ -4,6 +4,7 @@ package com.yahoo.language.opennlp; import com.yahoo.language.Language; import com.yahoo.language.LinguisticsCase; import com.yahoo.language.process.Normalizer; +import com.yahoo.language.process.SpecialTokenRegistry; import com.yahoo.language.process.StemMode; import com.yahoo.language.process.Token; import com.yahoo.language.process.TokenType; @@ -32,15 +33,21 @@ public class OpenNlpTokenizer implements Tokenizer { private final Normalizer normalizer; private final Transformer transformer; private final SimpleTokenizer simpleTokenizer; + private final SpecialTokenRegistry specialTokenRegistry; public OpenNlpTokenizer() { this(new SimpleNormalizer(), new SimpleTransformer()); } public OpenNlpTokenizer(Normalizer normalizer, Transformer transformer) { + this(normalizer, transformer, new SpecialTokenRegistry(List.of())); + } + + public OpenNlpTokenizer(Normalizer normalizer, Transformer transformer, SpecialTokenRegistry specialTokenRegistry) { this.normalizer = normalizer; this.transformer = transformer; - simpleTokenizer = new SimpleTokenizer(normalizer, transformer); + this.specialTokenRegistry = specialTokenRegistry; + this.simpleTokenizer = new SimpleTokenizer(normalizer, transformer, specialTokenRegistry); } @Override diff --git a/linguistics/src/main/java/com/yahoo/language/process/SpecialTokenRegistry.java b/linguistics/src/main/java/com/yahoo/language/process/SpecialTokenRegistry.java new file mode 100644 index 00000000000..b6335d67967 --- /dev/null +++ b/linguistics/src/main/java/com/yahoo/language/process/SpecialTokenRegistry.java @@ -0,0 +1,72 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.language.process; + +import com.yahoo.vespa.configdefinition.SpecialtokensConfig; +import com.yahoo.vespa.configdefinition.SpecialtokensConfig.Tokenlist; +import com.yahoo.vespa.configdefinition.SpecialtokensConfig.Tokenlist.Tokens; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +/** + * Immutable named lists of "special tokens" - strings which should override the normal tokenizer semantics + * and be tokenized into a single token. + * + * @author bratseth + */ +public class SpecialTokenRegistry { + + /** + * The current special token lists, indexed on name. + * These lists are unmodifiable and used directly by clients of this + */ + private final Map<String, SpecialTokens> specialTokenMap; + + /** Creates an empty special token registry */ + public SpecialTokenRegistry() { + this(List.of()); + } + + /** Create a special token registry from a configuration object. */ + public SpecialTokenRegistry(SpecialtokensConfig config) { + this(specialTokensFrom(config)); + } + + public SpecialTokenRegistry(List<SpecialTokens> specialTokensList) { + specialTokenMap = specialTokensList.stream().collect(Collectors.toUnmodifiableMap(t -> t.name(), t -> t)); + } + + private static List<SpecialTokens> specialTokensFrom(SpecialtokensConfig config) { + List<SpecialTokens> specialTokensList = new ArrayList<>(); + for (Iterator<Tokenlist> i = config.tokenlist().iterator(); i.hasNext();) { + Tokenlist tokenListConfig = i.next(); + + List<SpecialTokens.Token> tokenList = new ArrayList<>(); + for (Iterator<Tokens> j = tokenListConfig.tokens().iterator(); j.hasNext();) { + Tokens tokenConfig = j.next(); + tokenList.add(new SpecialTokens.Token(tokenConfig.token(), tokenConfig.replace())); + } + specialTokensList.add(new SpecialTokens(tokenListConfig.name(), tokenList)); + } + return specialTokensList; + } + + /** + * Returns the list of special tokens for a given name. + * + * @param name the name of the special tokens to return + * null, the empty string or the string "default" returns + * the default ones + * @return a read-only list of SpecialToken instances, an empty list if this name + * has no special tokens + */ + public SpecialTokens getSpecialTokens(String name) { + if (name == null || name.trim().equals("")) + name = "default"; + return specialTokenMap.getOrDefault(name, SpecialTokens.empty()); + } + +} diff --git a/linguistics/src/main/java/com/yahoo/language/process/SpecialTokens.java b/linguistics/src/main/java/com/yahoo/language/process/SpecialTokens.java new file mode 100644 index 00000000000..465d9b754b3 --- /dev/null +++ b/linguistics/src/main/java/com/yahoo/language/process/SpecialTokens.java @@ -0,0 +1,141 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.language.process; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; + +import static com.yahoo.language.LinguisticsCase.toLowerCase; + +/** + * An immutable list of special tokens - strings which should override the normal tokenizer semantics + * and be tokenized into a single token. Special tokens are case insensitive. + * + * @author bratseth + */ +public class SpecialTokens { + + private static final SpecialTokens empty = new SpecialTokens("(empty)", List.of()); + + private final String name; + private final int maximumLength; + private final List<Token> tokens; + private final Map<String, String> tokenMap; + + public SpecialTokens(String name, List<Token> tokens) { + tokens.stream().peek(token -> token.validate()); + List<Token> mutableTokens = new ArrayList<>(tokens); + Collections.sort(mutableTokens); + this.name = name; + this.maximumLength = tokens.stream().mapToInt(token -> token.token().length()).max().orElse(0); + this.tokens = List.copyOf(mutableTokens); + this.tokenMap = tokens.stream().collect(Collectors.toUnmodifiableMap(t -> t.token(), t -> t.replacement())); + } + + /** Returns the name of this special tokens list */ + public String name() { + return name; + } + + /** + * Returns the tokens of this as an immutable map from token to replacement. + * Tokens which do not have a replacement token maps to themselves. + */ + public Map<String, String> asMap() { return tokenMap; } + + /** + * Returns the special token starting at the start of the given string, or null if no + * special token starts at this string + * + * @param string the string to search for a special token at the start position + * @param substring true to allow the special token to be followed by a character which does not + * mark the end of a token + */ + public Token tokenize(String string, boolean substring) { + // XXX detonator pattern token.length may be != the length of the + // matching data in string, ref caseIndependentLength(String) + String input = toLowerCase(string.substring(0, Math.min(string.length(), maximumLength))); + for (Iterator<Token> i = tokens.iterator(); i.hasNext();) { + Token special = i.next(); + + if (input.startsWith(special.token())) { + if (string.length() == special.token().length() || substring || tokenEndsAt(special.token().length(), string)) + return special; + } + } + return null; + } + + private boolean tokenEndsAt(int position, String string) { + return !Character.isLetterOrDigit(string.charAt(position)); + } + + public static SpecialTokens empty() { return empty; } + + /** An immutable special token */ + public final static class Token implements Comparable<Token> { + + private final String token; + private final String replacement; + + /** Creates a special token */ + public Token(String token) { + this(token, null); + } + + /** Creates a special token which will be represented by the given replacement token */ + public Token(String token, String replacement) { + this.token = toLowerCase(token); + if (replacement == null || replacement.trim().equals("")) + this.replacement = this.token; + else + this.replacement = toLowerCase(replacement); + } + + /** Returns the special token */ + public String token() { return token; } + + /** Returns the token to replace occurrences of this by, which equals token() unless this has a replacement. */ + public String replacement() { return replacement; } + + @Override + public int compareTo(Token other) { + if (this.token().length() < other.token().length()) return 1; + if (this.token().length() == other.token().length()) return 0; + return -1; + } + + @Override + public boolean equals(Object other) { + if (other == this) return true; + if ( ! (other instanceof Token)) return false; + return Objects.equals(this.token, ((Token)other).token); + } + + @Override + public int hashCode() { return token.hashCode(); } + + @Override + public String toString() { + return "token '" + token + "'" + (replacement.equals(token) ? "" : " replacement '" + replacement + "'"); + } + + private void validate() { + // XXX not fool proof length test, should test codepoint by codepoint for mixed case user input? not even that will necessarily be 100% robust... + String asLow = toLowerCase(token); + // TODO: Put along with the global toLowerCase + String asHigh = token.toUpperCase(Locale.ENGLISH); + if (asLow.length() != token.length() || asHigh.length() != token.length()) { + throw new IllegalArgumentException("Special token '" + token + "' has case sensitive length. " + + "Please report this to the Vespa team."); + } + } + + } + +} diff --git a/linguistics/src/main/java/com/yahoo/language/process/TokenType.java b/linguistics/src/main/java/com/yahoo/language/process/TokenType.java index 57a5b6edb68..ad154d1b003 100644 --- a/linguistics/src/main/java/com/yahoo/language/process/TokenType.java +++ b/linguistics/src/main/java/com/yahoo/language/process/TokenType.java @@ -4,7 +4,7 @@ package com.yahoo.language.process; /** * An enumeration of token types. * - * @author <a href="mailto:mathiasm@yahoo-inc.com">Mathias Mølster Lidal</a> + * @author Mathias Mølster Lidal */ public enum TokenType { diff --git a/linguistics/src/main/java/com/yahoo/language/simple/SimpleLinguistics.java b/linguistics/src/main/java/com/yahoo/language/simple/SimpleLinguistics.java index e1a04b2985d..4ffe2a866d8 100644 --- a/linguistics/src/main/java/com/yahoo/language/simple/SimpleLinguistics.java +++ b/linguistics/src/main/java/com/yahoo/language/simple/SimpleLinguistics.java @@ -11,10 +11,14 @@ import com.yahoo.language.process.GramSplitter; import com.yahoo.language.process.Normalizer; import com.yahoo.language.process.Segmenter; import com.yahoo.language.process.SegmenterImpl; +import com.yahoo.language.process.SpecialTokenRegistry; import com.yahoo.language.process.Stemmer; import com.yahoo.language.process.StemmerImpl; import com.yahoo.language.process.Tokenizer; import com.yahoo.language.process.Transformer; +import com.yahoo.vespa.configdefinition.SpecialtokensConfig; + +import java.util.List; /** * Factory of simple linguistic processor implementations. @@ -31,6 +35,7 @@ public class SimpleLinguistics implements Linguistics { private final Detector detector; private final CharacterClasses characterClasses; private final GramSplitter gramSplitter; + private final SpecialTokenRegistry specialTokenRegistry = new SpecialTokenRegistry(List.of()); @Inject public SimpleLinguistics() { @@ -45,7 +50,7 @@ public class SimpleLinguistics implements Linguistics { public Stemmer getStemmer() { return new StemmerImpl(getTokenizer()); } @Override - public Tokenizer getTokenizer() { return new SimpleTokenizer(normalizer, transformer); } + public Tokenizer getTokenizer() { return new SimpleTokenizer(normalizer, transformer, specialTokenRegistry); } @Override public Normalizer getNormalizer() { return normalizer; } diff --git a/linguistics/src/main/java/com/yahoo/language/simple/SimpleTokenizer.java b/linguistics/src/main/java/com/yahoo/language/simple/SimpleTokenizer.java index 7df432f496d..740307c0cca 100644 --- a/linguistics/src/main/java/com/yahoo/language/simple/SimpleTokenizer.java +++ b/linguistics/src/main/java/com/yahoo/language/simple/SimpleTokenizer.java @@ -23,11 +23,13 @@ import java.util.logging.Level; */ public class SimpleTokenizer implements Tokenizer { + private static final Logger log = Logger.getLogger(SimpleTokenizer.class.getName()); private final static int SPACE_CODE = 32; + private final Normalizer normalizer; private final Transformer transformer; private final KStemmer stemmer = new KStemmer(); - private static final Logger log = Logger.getLogger(SimpleTokenizer.class.getName()); + private final SpecialTokenRegistry specialTokenRegistry; public SimpleTokenizer() { this(new SimpleNormalizer(), new SimpleTransformer()); @@ -38,8 +40,13 @@ public class SimpleTokenizer implements Tokenizer { } public SimpleTokenizer(Normalizer normalizer, Transformer transformer) { + this(normalizer, transformer, new SpecialTokenRegistry(List.of())); + } + + public SimpleTokenizer(Normalizer normalizer, Transformer transformer, SpecialTokenRegistry specialTokenRegistry) { this.normalizer = normalizer; this.transformer = transformer; + this.specialTokenRegistry = specialTokenRegistry; } @Override @@ -56,8 +63,8 @@ public class SimpleTokenizer implements Tokenizer { String original = input.substring(prev, next); String token = processToken(original, language, stemMode, removeAccents); tokens.add(new SimpleToken(original).setOffset(prev) - .setType(prevType) - .setTokenString(token)); + .setType(prevType) + .setTokenString(token)); prev = next; prevType = nextType; } @@ -67,20 +74,20 @@ public class SimpleTokenizer implements Tokenizer { } private String processToken(String token, Language language, StemMode stemMode, boolean removeAccents) { - final String original = token; - log.log(Level.FINEST, () -> "processToken '"+original+"'"); + String original = token; + log.log(Level.FINEST, () -> "processToken '" + original + "'"); token = normalizer.normalize(token); token = LinguisticsCase.toLowerCase(token); if (removeAccents) token = transformer.accentDrop(token, language); if (stemMode != StemMode.NONE) { - final String oldToken = token; + String oldToken = token; token = stemmer.stem(token); - final String newToken = token; - log.log(Level.FINEST, () -> "stem '"+oldToken+"' to '"+newToken+"'"); + String newToken = token; + log.log(Level.FINEST, () -> "stem '" + oldToken+"' to '" + newToken+"'"); } - final String result = token; - log.log(Level.FINEST, () -> "processed token is: "+result); + String result = token; + log.log(Level.FINEST, () -> "processed token is: " + result); return result; } diff --git a/linguistics/src/test/java/com/yahoo/language/process/SpecialTokensTestCase.java b/linguistics/src/test/java/com/yahoo/language/process/SpecialTokensTestCase.java new file mode 100644 index 00000000000..47c3ba7933c --- /dev/null +++ b/linguistics/src/test/java/com/yahoo/language/process/SpecialTokensTestCase.java @@ -0,0 +1,40 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.language.process; + +import com.yahoo.vespa.configdefinition.SpecialtokensConfig; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +/** + * @author bratseth + */ +public class SpecialTokensTestCase { + + @Test + public void testSpecialTokensConfig() { + var builder = new SpecialtokensConfig.Builder(); + var tokenBuilder = new SpecialtokensConfig.Tokenlist.Builder(); + tokenBuilder.name("default"); + + var tokenListBuilder1 = new SpecialtokensConfig.Tokenlist.Tokens.Builder(); + tokenListBuilder1.token("c++"); + tokenListBuilder1.replace("cpp"); + tokenBuilder.tokens(tokenListBuilder1); + + var tokenListBuilder2 = new SpecialtokensConfig.Tokenlist.Tokens.Builder(); + tokenListBuilder2.token("..."); + tokenBuilder.tokens(tokenListBuilder2); + + builder.tokenlist(tokenBuilder); + + var registry = new SpecialTokenRegistry(builder.build()); + + var defaultTokens = registry.getSpecialTokens("default"); + assertEquals("default", defaultTokens.name()); + assertEquals(2, defaultTokens.asMap().size()); + assertEquals("cpp", defaultTokens.asMap().get("c++")); + assertEquals("...", defaultTokens.asMap().get("...")); + } + +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index 74ba19a72c5..c23e1899257 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -405,7 +405,7 @@ public class NodeAgentImpl implements NodeAgent { } private boolean noCpuCap(ZoneApi zone) { - return zone.getEnvironment() == Environment.dev || zone.getSystemName().isCd(); + return zone.getEnvironment() == Environment.dev; } private boolean downloadImageIfNeeded(NodeAgentContext context, Optional<Container> container) { diff --git a/parent/pom.xml b/parent/pom.xml index 322548194d9..0d29ad22a59 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -537,6 +537,18 @@ <version>${prometheus.client.version}</version> </dependency> <dependency> + <!-- TODO: Try to remove, as this overlaps with javax.activation. --> + <groupId>jakarta.activation</groupId> + <artifactId>jakarta.activation-api</artifactId> + <version>1.2.1</version> + </dependency> + <dependency> + <!-- TODO: Try to remove, as this conflicts with javax.xml.bind:jaxb-api --> + <groupId>jakarta.xml.bind</groupId> + <artifactId>jakarta.xml.bind-api</artifactId> + <version>2.3.2</version> + </dependency> + <dependency> <groupId>junit</groupId> <artifactId>junit</artifactId> <version>4.12</version> @@ -864,7 +876,7 @@ <maven-javadoc-plugin.version>3.0.1</maven-javadoc-plugin.version> <maven-plugin-tools.version>3.6.0</maven-plugin-tools.version> <maven-resources-plugin.version>2.7</maven-resources-plugin.version> - <maven-shade-plugin.version>3.2.1</maven-shade-plugin.version> + <maven-shade-plugin.version>3.2.4</maven-shade-plugin.version> <maven-site-plugin.version>3.3</maven-site-plugin.version> <maven-source-plugin.version>3.0.1</maven-source-plugin.version> <prometheus.client.version>0.6.0</prometheus.client.version> diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt b/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt index 88bd84cbd16..1aa0b1c585d 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt @@ -5,9 +5,9 @@ vespa_add_library(searchcore_bucketmover_test STATIC bucketmover_common.cpp ) -vespa_add_executable(searchcore_documentbucketmover_v2_test_app TEST +vespa_add_executable(searchcore_documentbucketmover_test_app TEST SOURCES - documentbucketmover_v2_test.cpp + documentbucketmover_test.cpp DEPENDS searchcore_bucketmover_test searchcore_test @@ -15,7 +15,7 @@ vespa_add_executable(searchcore_documentbucketmover_v2_test_app TEST searchcore_feedoperation GTest::GTest ) -vespa_add_test(NAME searchcore_documentbucketmover_v2_test_app COMMAND searchcore_documentbucketmover_v2_test_app) +vespa_add_test(NAME searchcore_documentbucketmover_test_app COMMAND searchcore_documentbucketmover_test_app) vespa_add_executable(searchcore_scaniterator_test_app TEST SOURCES diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_v2_test.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp index 8dcad91f69a..609f2413bdc 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_v2_test.cpp +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp @@ -1,7 +1,7 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "bucketmover_common.h" -#include <vespa/searchcore/proton/server/bucketmovejobv2.h> +#include <vespa/searchcore/proton/server/bucketmovejob.h> #include <vespa/searchcore/proton/server/executor_thread_service.h> #include <vespa/searchcore/proton/server/document_db_maintenance_config.h> #include <vespa/persistence/dummyimpl/dummy_bucket_executor.h> diff --git a/searchcore/src/tests/proton/documentdb/job_tracked_maintenance_job/job_tracked_maintenance_job_test.cpp b/searchcore/src/tests/proton/documentdb/job_tracked_maintenance_job/job_tracked_maintenance_job_test.cpp index 4287a6b262a..8862a6d3bb2 100644 --- a/searchcore/src/tests/proton/documentdb/job_tracked_maintenance_job/job_tracked_maintenance_job_test.cpp +++ b/searchcore/src/tests/proton/documentdb/job_tracked_maintenance_job/job_tracked_maintenance_job_test.cpp @@ -34,13 +34,11 @@ struct MyMaintenanceJob : public IBlockableMaintenanceJob GateVector _runGates; size_t _runIdx; bool _blocked; - bool _stopped; MyMaintenanceJob(size_t numRuns) : IBlockableMaintenanceJob("myjob", 10s, 20s), _runGates(getGateVector(numRuns)), _runIdx(0), - _blocked(false), - _stopped(false) + _blocked(false) {} void block() { setBlocked(BlockedReason::RESOURCE_LIMITS); } void unBlock() { unBlock(BlockedReason::RESOURCE_LIMITS); } @@ -51,7 +49,7 @@ struct MyMaintenanceJob : public IBlockableMaintenanceJob _runGates[_runIdx++]->await(5s); return _runIdx == _runGates.size(); } - void onStop() override { _stopped = true; } + void onStop() override { } }; struct Fixture @@ -65,10 +63,10 @@ struct Fixture size_t _runIdx; ThreadStackExecutor _exec; Fixture(size_t numRuns = 1) - : _tracker(new SimpleJobTracker(1)), - _job(new MyMaintenanceJob(numRuns)), + : _tracker(std::make_shared<SimpleJobTracker>(1)), + _job(std::make_unique<MyMaintenanceJob>(numRuns)), _myJob(static_cast<MyMaintenanceJob *>(_job.get())), - _trackedJob(new JobTrackedMaintenanceJob(_tracker, std::move(_job))), + _trackedJob(std::make_unique<JobTrackedMaintenanceJob>(_tracker, std::move(_job))), _runRetval(false), _runGates(getGateVector(numRuns)), _runIdx(0), @@ -144,9 +142,9 @@ TEST_F("require that block calls are sent to underlying jobs", Fixture) TEST_F("require that stop calls are sent to underlying jobs", Fixture) { - EXPECT_FALSE(f._myJob->_stopped); - f._trackedJob->onStop(); - EXPECT_TRUE(f._myJob->_stopped); + EXPECT_FALSE(f._myJob->stopped()); + f._trackedJob->stop(); + EXPECT_TRUE(f._myJob->stopped()); } TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_jobtest.cpp b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_jobtest.cpp index 73cd6a8e6be..a177f3f763e 100644 --- a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_jobtest.cpp +++ b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_jobtest.cpp @@ -1,7 +1,7 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "lid_space_jobtest.h" -#include <vespa/searchcore/proton/server/lid_space_compaction_job_take2.h> +#include <vespa/searchcore/proton/server/lid_space_compaction_job.h> #include <vespa/searchcore/proton/server/executorthreadingservice.h> #include <vespa/persistence/dummyimpl/dummy_bucket_executor.h> #include <vespa/vespalib/util/threadstackexecutor.h> diff --git a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp index 49236a829f5..e73fa739fbb 100644 --- a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp +++ b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp @@ -290,17 +290,14 @@ struct MySimpleJob : public BlockableMaintenanceJob { vespalib::CountDownLatch _latch; size_t _runCnt; - bool _stopped; MySimpleJob(vespalib::duration delay, vespalib::duration interval, uint32_t finishCount) : BlockableMaintenanceJob("my_job", delay, interval), _latch(finishCount), - _runCnt(0), - _stopped(false) - { - } + _runCnt(0) + { } void block() { setBlocked(BlockedReason::FROZEN_BUCKET); } bool run() override { LOG(info, "MySimpleJob::run()"); @@ -308,10 +305,6 @@ struct MySimpleJob : public BlockableMaintenanceJob ++_runCnt; return true; } - void onStop() override { - BlockableMaintenanceJob::onStop(); - _stopped = true; - } }; struct MySplitJob : public MySimpleJob @@ -333,13 +326,11 @@ struct MySplitJob : public MySimpleJob struct MyLongRunningJob : public BlockableMaintenanceJob { vespalib::Gate _firstRun; - bool _stopped; MyLongRunningJob(vespalib::duration delay, vespalib::duration interval) : BlockableMaintenanceJob("long_running_job", delay, interval), - _firstRun(), - _stopped(false) + _firstRun() { } void block() { setBlocked(BlockedReason::FROZEN_BUCKET); } @@ -348,10 +339,6 @@ struct MyLongRunningJob : public BlockableMaintenanceJob usleep(10000); return false; } - void onStop() override { - BlockableMaintenanceJob::onStop(); - _stopped = true; - } }; using MyAttributeManager = test::MockAttributeManager; diff --git a/searchcore/src/tests/proton/documentdb/move_operation_limiter/move_operation_limiter_test.cpp b/searchcore/src/tests/proton/documentdb/move_operation_limiter/move_operation_limiter_test.cpp index 1f152f4a257..8466a016fc3 100644 --- a/searchcore/src/tests/proton/documentdb/move_operation_limiter/move_operation_limiter_test.cpp +++ b/searchcore/src/tests/proton/documentdb/move_operation_limiter/move_operation_limiter_test.cpp @@ -12,11 +12,9 @@ using namespace proton; struct MyBlockableMaintenanceJob : public IBlockableMaintenanceJob { bool blocked; - bool stopped; MyBlockableMaintenanceJob() : IBlockableMaintenanceJob("my_job", 1s, 1s), - blocked(false), - stopped(false) + blocked(false) {} void setBlocked(BlockedReason reason) override { ASSERT_TRUE(reason == BlockedReason::OUTSTANDING_OPS); @@ -29,7 +27,7 @@ struct MyBlockableMaintenanceJob : public IBlockableMaintenanceJob { blocked = false; } bool run() override { return true; } - void onStop() override { stopped = true; } + void onStop() override { } }; struct Fixture { diff --git a/searchcore/src/vespa/searchcore/proton/server/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/server/CMakeLists.txt index 75d7dc0cb59..7d4d6868d21 100644 --- a/searchcore/src/vespa/searchcore/proton/server/CMakeLists.txt +++ b/searchcore/src/vespa/searchcore/proton/server/CMakeLists.txt @@ -5,7 +5,7 @@ vespa_add_library(searchcore_server STATIC bootstrapconfig.cpp bootstrapconfigmanager.cpp buckethandler.cpp - bucketmovejobv2.cpp + bucketmovejob.cpp clusterstatehandler.cpp combiningfeedview.cpp ddbstate.cpp @@ -57,7 +57,7 @@ vespa_add_library(searchcore_server STATIC ireplayconfig.cpp job_tracked_maintenance_job.cpp lid_space_compaction_handler.cpp - lid_space_compaction_job_take2.cpp + lid_space_compaction_job.cpp maintenance_controller_explorer.cpp maintenance_jobs_injector.cpp maintenancecontroller.cpp diff --git a/searchcore/src/vespa/searchcore/proton/server/bucketmovejobv2.cpp b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp index 9e34462ae21..a43fd55ba02 100644 --- a/searchcore/src/vespa/searchcore/proton/server/bucketmovejobv2.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp @@ -1,6 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include "bucketmovejobv2.h" +#include "bucketmovejob.h" #include "imaintenancejobrunner.h" #include "ibucketstatechangednotifier.h" #include "iclusterstatechangednotifier.h" @@ -87,7 +87,6 @@ BucketMoveJobV2::BucketMoveJobV2(const std::shared_ptr<IBucketStateCalculator> & _movers(), _bucketsInFlight(), _buckets2Move(), - _stopped(false), _bucketsPending(0), _bucketCreateNotifier(bucketCreateNotifier), _clusterStateChangedNotifier(clusterStateChangedNotifier), @@ -206,9 +205,9 @@ private: void BucketMoveJobV2::failOperation(std::shared_ptr<BucketMoveJobV2> job, BucketId bucketId) { auto & master = job->_master; - if (job->_stopped) return; + if (job->stopped()) return; master.execute(makeLambdaTask([job=std::move(job), bucketId]() { - if (job->_stopped.load(std::memory_order_relaxed)) return; + if (job->stopped()) return; job->considerBucket(job->_ready.meta_store()->getBucketDB().takeGuard(), bucketId); })); } @@ -229,12 +228,12 @@ BucketMoveJobV2::startMove(BucketMover & mover, size_t maxDocsToMove) { void BucketMoveJobV2::prepareMove(std::shared_ptr<BucketMoveJobV2> job, BucketMover::MoveKeys keys, IDestructorCallbackSP onDone) { - if (job->_stopped) return; //TODO Remove once lidtracker is no longer in use. + if (job->stopped()) return; //TODO Remove once lidtracker is no longer in use. auto moveOps = keys.createMoveOperations(); auto & master = job->_master; - if (job->_stopped) return; + if (job->stopped()) return; master.execute(makeLambdaTask([job=std::move(job), moveOps=std::move(moveOps), onDone=std::move(onDone)]() mutable { - if (job->_stopped.load(std::memory_order_relaxed)) return; + if (job->stopped()) return; job->completeMove(std::move(moveOps), std::move(onDone)); })); } @@ -449,13 +448,6 @@ BucketMoveJobV2::notifyDiskMemUsage(DiskMemUsageState state) } void -BucketMoveJobV2::onStop() { - // Called by master write thread - BlockableMaintenanceJob::onStop(); - _stopped = true; -} - -void BucketMoveJobV2::updatePending() { _bucketsPending.store(_bucketsInFlight.size() + _buckets2Move.size(), std::memory_order_relaxed); } diff --git a/searchcore/src/vespa/searchcore/proton/server/bucketmovejobv2.h b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h index df75c8c9766..7d5dafb33b3 100644 --- a/searchcore/src/vespa/searchcore/proton/server/bucketmovejobv2.h +++ b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h @@ -72,7 +72,6 @@ private: Bucket2Mover _bucketsInFlight; BucketMoveSet _buckets2Move; - std::atomic<bool> _stopped; std::atomic<size_t> _bucketsPending; bucketdb::IBucketCreateNotifier &_bucketCreateNotifier; @@ -142,7 +141,6 @@ public: void notifyBucketStateChanged(const BucketId &bucketId, ActiveState newState) override; void notifyDiskMemUsage(DiskMemUsageState state) override; void notifyCreateBucket(const bucketdb::Guard & guard, const BucketId &bucket) override; - void onStop() override; void updateMetrics(DocumentDBTaggedMetrics & metrics) const override; }; diff --git a/searchcore/src/vespa/searchcore/proton/server/i_maintenance_job.h b/searchcore/src/vespa/searchcore/proton/server/i_maintenance_job.h index 6bea9855c82..7148576b76f 100644 --- a/searchcore/src/vespa/searchcore/proton/server/i_maintenance_job.h +++ b/searchcore/src/vespa/searchcore/proton/server/i_maintenance_job.h @@ -4,6 +4,7 @@ #include <vespa/vespalib/stllike/string.h> #include <vespa/vespalib/util/time.h> #include <memory> +#include <atomic> namespace proton { @@ -21,7 +22,9 @@ private: const vespalib::string _name; const vespalib::duration _delay; const vespalib::duration _interval; - + std::atomic<bool> _stopped; +protected: + virtual void onStop() = 0; public: using UP = std::unique_ptr<IMaintenanceJob>; using SP = std::shared_ptr<IMaintenanceJob>; @@ -31,7 +34,8 @@ public: vespalib::duration interval) : _name(name), _delay(delay), - _interval(interval) + _interval(interval), + _stopped(false) {} virtual ~IMaintenanceJob() = default; @@ -41,9 +45,12 @@ public: virtual vespalib::duration getInterval() const { return _interval; } virtual bool isBlocked() const { return false; } virtual IBlockableMaintenanceJob *asBlockable() { return nullptr; } - virtual void onStop() = 0; virtual void updateMetrics(DocumentDBTaggedMetrics &) const {} - + void stop() { + _stopped = true; + onStop(); + } + bool stopped() const { return _stopped.load(std::memory_order_relaxed); } /** * Register maintenance job runner, in case event passed to the * job causes it to want to be run again. diff --git a/searchcore/src/vespa/searchcore/proton/server/job_tracked_maintenance_job.h b/searchcore/src/vespa/searchcore/proton/server/job_tracked_maintenance_job.h index 0e1b2b00ce5..ecc592a00a4 100644 --- a/searchcore/src/vespa/searchcore/proton/server/job_tracked_maintenance_job.h +++ b/searchcore/src/vespa/searchcore/proton/server/job_tracked_maintenance_job.h @@ -9,7 +9,7 @@ namespace proton { /** * Class for tracking the start and end of a maintenance job. */ -class JobTrackedMaintenanceJob : public IMaintenanceJob +class JobTrackedMaintenanceJob final : public IMaintenanceJob { private: IJobTracker::SP _tracker; @@ -26,7 +26,7 @@ public: _job->registerRunner(runner); } bool run() override; - void onStop() override { _job->onStop(); } + void onStop() override { _job->stop(); } }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.cpp b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp index f2333f2f2b7..059408f4a5e 100644 --- a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp @@ -1,6 +1,6 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include "lid_space_compaction_job_take2.h" +#include "lid_space_compaction_job.h" #include "i_document_scan_iterator.h" #include "i_lid_space_compaction_handler.h" #include "i_operation_storer.h" @@ -59,7 +59,7 @@ public: void fail(const Bucket & bucket) override { assert(bucket.getBucketId() == _meta.bucketId); auto & master = _job->_master; - if (_job->_stopped) return; + if (_job->stopped()) return; master.execute(makeLambdaTask([job=std::move(_job)] { job->_scanItr.reset(); })); } private: @@ -88,7 +88,7 @@ void CompactionJob::moveDocument(std::shared_ptr<CompactionJob> job, const search::DocumentMetaData & metaThen, std::shared_ptr<IDestructorCallback> context) { - if (job->_stopped) return; //TODO Remove once lidtracker is no longer in use. + if (job->stopped()) return; //TODO Remove once lidtracker is no longer in use. // The real lid must be sampled in the master thread. //TODO remove target lid from createMoveOperation interface auto op = job->_handler->createMoveOperation(metaThen, 0); @@ -97,9 +97,9 @@ CompactionJob::moveDocument(std::shared_ptr<CompactionJob> job, const search::Do if (metaThen.gid != op->getDocument()->getId().getGlobalId()) return; auto & master = job->_master; - if (job->_stopped) return; + if (job->stopped()) return; master.execute(makeLambdaTask([self=std::move(job), meta=metaThen, moveOp=std::move(op), onDone=std::move(context)]() mutable { - if (self->_stopped.load(std::memory_order_relaxed)) return; + if (self->stopped()) return; self->completeMove(meta, std::move(moveOp), std::move(onDone)); })); } @@ -151,8 +151,7 @@ CompactionJob::CompactionJob(const DocumentDBLidSpaceCompactionConfig &config, _master(master), _bucketExecutor(bucketExecutor), _dbRetainer(std::move(dbRetainer)), - _bucketSpace(bucketSpace), - _stopped(false) + _bucketSpace(bucketSpace) { _diskMemUsageNotifier.addDiskMemUsageListener(this); _clusterStateChangedNotifier.addClusterStateChangedHandler(this); @@ -189,12 +188,6 @@ CompactionJob::create(const DocumentDBLidSpaceCompactionConfig &config, }); } -void -CompactionJob::onStop() { - BlockableMaintenanceJob::onStop(); - _stopped = true; -} - DocumentMetaData CompactionJob::getNextDocument(const LidUsageStats &stats, bool retryLastDocument) { diff --git a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.h b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.h index 141f29f4510..725c7387bdc 100644 --- a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.h +++ b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.h @@ -52,7 +52,6 @@ private: BucketExecutor &_bucketExecutor; RetainGuard _dbRetainer; document::BucketSpace _bucketSpace; - std::atomic<bool> _stopped; bool hasTooMuchLidBloat(const search::LidUsageStats &stats) const; bool shouldRestartScanDocuments(const search::LidUsageStats &stats) const; @@ -66,7 +65,6 @@ private: std::shared_ptr<IDestructorCallback> onDone); void completeMove(const search::DocumentMetaData & metaThen, std::unique_ptr<MoveOperation> moveOp, std::shared_ptr<IDestructorCallback> onDone); - void onStop() override; class MoveTask; CompactionJob(const DocumentDBLidSpaceCompactionConfig &config, diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp b/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp index 8f69fe84b3d..e713335be4b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp @@ -1,9 +1,9 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include "bucketmovejobv2.h" +#include "bucketmovejob.h" #include "heart_beat_job.h" #include "job_tracked_maintenance_job.h" -#include "lid_space_compaction_job_take2.h" +#include "lid_space_compaction_job.h" #include "lid_space_compaction_handler.h" #include "maintenance_jobs_injector.h" #include "prune_session_cache_job.h" diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenancejobrunner.cpp b/searchcore/src/vespa/searchcore/proton/server/maintenancejobrunner.cpp index 658fa9f7482..d508c3193fc 100644 --- a/searchcore/src/vespa/searchcore/proton/server/maintenancejobrunner.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/maintenancejobrunner.cpp @@ -25,7 +25,7 @@ MaintenanceJobRunner::stop() { Guard guard(_lock); _stopped = true; } - _job->onStop(); + _job->stop(); } void diff --git a/searchcore/src/vespa/searchcore/proton/server/pruneremoveddocumentsjob_v2.cpp b/searchcore/src/vespa/searchcore/proton/server/pruneremoveddocumentsjob_v2.cpp index f2cb6f3c270..0c5b165611e 100644 --- a/searchcore/src/vespa/searchcore/proton/server/pruneremoveddocumentsjob_v2.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/pruneremoveddocumentsjob_v2.cpp @@ -36,7 +36,6 @@ PruneRemovedDocumentsJobV2(const DocumentDBPruneConfig &config, RetainGuard dbRe _cfgAgeLimit(config.getAge()), _subDbId(subDbId), _bucketSpace(bucketSpace), - _stopped(false), _nextLid(1u) { } @@ -75,7 +74,7 @@ PruneRemovedDocumentsJobV2::PruneTask::run(const Bucket & bucket, IDestructorCal void PruneRemovedDocumentsJobV2::remove(uint32_t lid, const RawDocumentMetaData & oldMeta) { - if (_stopped.load(std::memory_order_relaxed)) return; + if (stopped()) return; if ( ! _metaStore.validLid(lid)) return; const RawDocumentMetaData &meta = _metaStore.getRawMetaData(lid); if (meta.getBucketId() != oldMeta.getBucketId()) return; diff --git a/searchcore/src/vespa/searchcore/proton/server/pruneremoveddocumentsjob_v2.h b/searchcore/src/vespa/searchcore/proton/server/pruneremoveddocumentsjob_v2.h index 98330c75d5c..b0eefedf8e2 100644 --- a/searchcore/src/vespa/searchcore/proton/server/pruneremoveddocumentsjob_v2.h +++ b/searchcore/src/vespa/searchcore/proton/server/pruneremoveddocumentsjob_v2.h @@ -40,7 +40,6 @@ private: const vespalib::duration _cfgAgeLimit; const uint32_t _subDbId; const document::BucketSpace _bucketSpace; - std::atomic<bool> _stopped; DocId _nextLid; @@ -51,7 +50,6 @@ private: IPruneRemovedDocumentsHandler &handler, IThreadService & master, BucketExecutor & bucketExecutor); bool run() override; - void onStop() override { _stopped = true; } public: static std::shared_ptr<PruneRemovedDocumentsJobV2> create(const Config &config, RetainGuard dbRetainer, const IDocumentMetaStore &metaStore, uint32_t subDbId, diff --git a/storage/src/vespa/storage/distributor/CMakeLists.txt b/storage/src/vespa/storage/distributor/CMakeLists.txt index c5c67d3203b..82f5d462dd1 100644 --- a/storage/src/vespa/storage/distributor/CMakeLists.txt +++ b/storage/src/vespa/storage/distributor/CMakeLists.txt @@ -27,7 +27,6 @@ vespa_add_library(storage_distributor ideal_service_layer_nodes_bundle.cpp idealstatemanager.cpp idealstatemetricsset.cpp - legacy_single_stripe_accessor.cpp messagetracker.cpp multi_threaded_stripe_access_guard.cpp nodeinfo.cpp diff --git a/storage/src/vespa/storage/distributor/bucketdbupdater.cpp b/storage/src/vespa/storage/distributor/bucketdbupdater.cpp index ee36259bdd3..d903a818270 100644 --- a/storage/src/vespa/storage/distributor/bucketdbupdater.cpp +++ b/storage/src/vespa/storage/distributor/bucketdbupdater.cpp @@ -33,7 +33,7 @@ BucketDBUpdater::BucketDBUpdater(const DistributorNodeContext& node_ctx, ChainedMessageSender& chained_sender, std::shared_ptr<const lib::Distribution> bootstrap_distribution, StripeAccessor& stripe_accessor) - : framework::StatusReporter("temp_bucketdb", "Bucket DB Updater"), // TODO STRIPE rename once duplication is removed + : framework::StatusReporter("bucketdb", "Bucket DB Updater"), _stripe_accessor(stripe_accessor), _active_state_bundle(lib::ClusterState()), _node_ctx(node_ctx), @@ -483,6 +483,13 @@ BucketDBUpdater::report_xml_status(vespalib::xml::XmlOutputStream& xos, << XmlAttribute("processingtime", i->_processingTime) << XmlEndTag(); } + xos << XmlEndTag() + << XmlTag("single_bucket_requests"); + auto guard = _stripe_accessor.rendezvous_and_hold_all(); + guard->report_single_bucket_requests(xos); + xos << XmlEndTag() + << XmlTag("delayed_single_bucket_requests"); + guard->report_delayed_single_bucket_requests(xos); xos << XmlEndTag() << XmlEndTag(); return ""; } diff --git a/storage/src/vespa/storage/distributor/distributor.cpp b/storage/src/vespa/storage/distributor/distributor.cpp index 6975a2595ad..e5d25681148 100644 --- a/storage/src/vespa/storage/distributor/distributor.cpp +++ b/storage/src/vespa/storage/distributor/distributor.cpp @@ -7,9 +7,11 @@ #include "distributor_bucket_space.h" #include "distributor_status.h" #include "distributor_stripe.h" +#include "distributor_stripe_pool.h" +#include "distributor_stripe_thread.h" #include "distributormetricsset.h" #include "idealstatemetricsset.h" -#include "legacy_single_stripe_accessor.h" +#include "multi_threaded_stripe_access_guard.h" #include "operation_sequencer.h" #include "ownership_transfer_safe_time_point_calculator.h" #include "throttlingoperationstarter.h" @@ -57,7 +59,11 @@ Distributor::Distributor(DistributorComponentRegister& compReg, _use_legacy_mode(num_distributor_stripes == 0), _stripe(std::make_unique<DistributorStripe>(compReg, *_metrics, node_identity, threadPool, doneInitHandler, *this, _use_legacy_mode)), - _stripe_accessor(std::make_unique<LegacySingleStripeAccessor>(*_stripe)), + _stripe_pool(), + _stripes(), + _stripe_accessor(), + _message_queue(), + _fetched_messages(), _component(*this, compReg, "distributor"), _total_config(_component.total_distributor_config_sp()), _bucket_db_updater(), @@ -74,13 +80,18 @@ Distributor::Distributor(DistributorComponentRegister& compReg, _component.registerMetricUpdateHook(_metricUpdateHook, framework::SecondTime(0)); if (!_use_legacy_mode) { LOG(info, "Setting up distributor with %u stripes", num_distributor_stripes); // TODO STRIPE remove once legacy gone + _stripe_pool = std::make_unique<DistributorStripePool>(); + _stripe_accessor = std::make_unique<MultiThreadedStripeAccessor>(*_stripe_pool); _bucket_db_updater = std::make_unique<BucketDBUpdater>(_component, _component, *this, *this, _component.getDistribution(), *_stripe_accessor); + _stripes.emplace_back(std::move(_stripe)); + _distributorStatusDelegate.registerStatusPage(); + _bucket_db_status_delegate = std::make_unique<StatusReporterDelegate>(compReg, *this, *_bucket_db_updater); + _bucket_db_status_delegate->registerStatusPage(); } _hostInfoReporter.enableReporting(config().getEnableHostInfoReporting()); - _distributorStatusDelegate.registerStatusPage(); hostInfoReporterRegistrar.registerReporter(&_hostInfoReporter); propagateDefaultDistribution(_component.getDistribution()); }; @@ -91,6 +102,20 @@ Distributor::~Distributor() closeNextLink(); } +// TODO STRIPE remove +DistributorStripe& +Distributor::first_stripe() noexcept { + assert(_stripes.size() == 1); + return *_stripes[0]; +} + +// TODO STRIPE remove +const DistributorStripe& +Distributor::first_stripe() const noexcept { + assert(_stripes.size() == 1); + return *_stripes[0]; +} + // TODO STRIPE figure out how to handle inspection functions used by tests when legacy mode no longer exists. // All functions below that assert on _use_legacy_mode are only currently used by tests @@ -217,6 +242,10 @@ Distributor::onOpen() if (_component.getDistributorConfig().startDistributorThread) { _threadPool.addThread(*this); _threadPool.start(_component.getThreadPool()); + if (!_use_legacy_mode) { + std::vector<TickableStripe*> pool_stripes({_stripes[0].get()}); + _stripe_pool->start(pool_stripes); + } } else { LOG(warning, "Not starting distributor thread as it's configured to " "run. Unless you are just running a test tool, this is a " @@ -226,8 +255,17 @@ Distributor::onOpen() void Distributor::onClose() { LOG(debug, "Distributor::onClose invoked"); - _stripe->flush_and_close(); - if (_bucket_db_updater) { + if (_use_legacy_mode) { + _stripe->flush_and_close(); + } else { + { + auto guard = _stripe_accessor->rendezvous_and_hold_all(); + guard->flush_and_close(); + } + // TODO STRIPE must ensure no incoming requests can be posted on stripes between close + // and pool stop+join! + _stripe_pool->stop_and_join(); + assert(_bucket_db_updater); _bucket_db_updater->flush(); } } @@ -273,22 +311,31 @@ bool should_be_handled_by_top_level_bucket_db_updater(const api::StorageMessage& bool Distributor::onDown(const std::shared_ptr<api::StorageMessage>& msg) { - // FIXME STRIPE this MUST be in a separate thread to enforce processing in a single thread - // regardless of what RPC thread (comm mgr, FRT...) this is called from! - if (!_use_legacy_mode && should_be_handled_by_top_level_bucket_db_updater(*msg)) { - return msg->callHandler(*_bucket_db_updater, msg); - } // TODO STRIPE can we route both requests and responses that are BucketCommand|Reply based on their bucket alone? // that covers most operations already... - return _stripe->handle_or_enqueue_message(msg); + if (_use_legacy_mode) { + return _stripe->handle_or_enqueue_message(msg); + } else { + if (should_be_handled_by_top_level_bucket_db_updater(*msg)) { + dispatch_to_main_distributor_thread_queue(msg); + return true; + } + assert(_stripes.size() == 1); + assert(_stripe_pool->stripe_count() == 1); + // TODO STRIPE correct routing with multiple stripes + bool handled = first_stripe().handle_or_enqueue_message(msg); + if (handled) { + _stripe_pool->stripe_thread(0).notify_event_has_triggered(); + } + return handled; + } } bool Distributor::handleReply(const std::shared_ptr<api::StorageReply>& reply) { - if (!_use_legacy_mode && should_be_handled_by_top_level_bucket_db_updater(*reply)) { - return reply->callHandler(*_bucket_db_updater, reply); - } + // TODO STRIPE this is used by tests. Do we need to invoke top-level BucketDBUpdater for any of them? + assert(_use_legacy_mode); return _stripe->handleReply(reply); } @@ -375,33 +422,61 @@ Distributor::propagateDefaultDistribution( std::shared_ptr<const lib::Distribution> distribution) { // TODO STRIPE cannot directly access stripe when not in legacy mode! - _stripe->propagateDefaultDistribution(std::move(distribution)); + if (_use_legacy_mode) { + _stripe->propagateDefaultDistribution(std::move(distribution)); + } else { + // Should only be called at ctor time, at which point the pool is not yet running. + assert(_stripe_pool->stripe_count() == 0); + assert(_stripes.size() == 1); // TODO STRIPE all the stripes yes + auto new_configs = BucketSpaceDistributionConfigs::from_default_distribution(std::move(distribution)); + for (auto& stripe : _stripes) { + stripe->update_distribution_config(new_configs); + } + } } std::unordered_map<uint16_t, uint32_t> Distributor::getMinReplica() const { // TODO STRIPE merged snapshot from all stripes - return _stripe->getMinReplica(); + if (_use_legacy_mode) { + return _stripe->getMinReplica(); + } else { + return first_stripe().getMinReplica(); + } } BucketSpacesStatsProvider::PerNodeBucketSpacesStats Distributor::getBucketSpacesStats() const { // TODO STRIPE merged snapshot from all stripes - return _stripe->getBucketSpacesStats(); + if (_use_legacy_mode) { + return _stripe->getBucketSpacesStats(); + } else { + return first_stripe().getBucketSpacesStats(); + } } SimpleMaintenanceScanner::PendingMaintenanceStats Distributor::pending_maintenance_stats() const { // TODO STRIPE merged snapshot from all stripes - return _stripe->pending_maintenance_stats(); + if (_use_legacy_mode) { + return _stripe->pending_maintenance_stats(); + } else { + return first_stripe().pending_maintenance_stats(); + } } void Distributor::propagateInternalScanMetricsToExternal() { - _stripe->propagateInternalScanMetricsToExternal(); + // TODO STRIPE propagate to all stripes + // TODO STRIPE reconsider metric wiring... + if (_use_legacy_mode) { + _stripe->propagateInternalScanMetricsToExternal(); + } else { + first_stripe().propagateInternalScanMetricsToExternal(); + } } void @@ -411,30 +486,70 @@ Distributor::scanAllBuckets() _stripe->scanAllBuckets(); } +void +Distributor::dispatch_to_main_distributor_thread_queue(const std::shared_ptr<api::StorageMessage>& msg) +{ + MBUS_TRACE(msg->getTrace(), 9, "Distributor: Added to main thread message queue"); + framework::TickingLockGuard guard(_threadPool.freezeCriticalTicks()); + _message_queue.emplace_back(msg); + guard.broadcast(); +} + +void +Distributor::fetch_external_messages() +{ + assert(!_use_legacy_mode); + assert(_fetched_messages.empty()); + _fetched_messages.swap(_message_queue); +} + +void +Distributor::process_fetched_external_messages() +{ + assert(!_use_legacy_mode); + for (auto& msg : _fetched_messages) { + MBUS_TRACE(msg->getTrace(), 9, "Distributor: Processing message in main thread"); + if (!msg->callHandler(*_bucket_db_updater, msg)) { + MBUS_TRACE(msg->getTrace(), 9, "Distributor: Not handling it. Sending further down"); + sendDown(msg); + } + } + if (!_fetched_messages.empty()) { + _fetched_messages.clear(); + signal_work_was_done(); + } +} + framework::ThreadWaitInfo Distributor::doCriticalTick(framework::ThreadIndex idx) { _tickResult = framework::ThreadWaitInfo::NO_MORE_CRITICAL_WORK_KNOWN; if (!_use_legacy_mode) { enableNextDistribution(); + fetch_status_requests(); + fetch_external_messages(); } // Propagates any new configs down to stripe(s) enableNextConfig(); - // TODO STRIPE only do in legacy mode, use stripe pool ticking otherwise - _stripe->doCriticalTick(idx); - _tickResult.merge(_stripe->_tickResult); + if (_use_legacy_mode) { + _stripe->doCriticalTick(idx); + _tickResult.merge(_stripe->_tickResult); + } return _tickResult; } framework::ThreadWaitInfo Distributor::doNonCriticalTick(framework::ThreadIndex idx) { - if (!_use_legacy_mode) { + if (_use_legacy_mode) { + _stripe->doNonCriticalTick(idx); + _tickResult = _stripe->_tickResult; + } else { + _tickResult = framework::ThreadWaitInfo::NO_MORE_CRITICAL_WORK_KNOWN; + handle_status_requests(); + process_fetched_external_messages(); _bucket_db_updater->resend_delayed_messages(); } - // TODO STRIPE stripes need their own thread loops! - _stripe->doNonCriticalTick(idx); - _tickResult = _stripe->_tickResult; return _tickResult; } @@ -460,30 +575,112 @@ Distributor::enableNextConfig() // TODO STRIPE rename to enable_next_config_if_c } } +void +Distributor::fetch_status_requests() +{ + if (_fetched_status_requests.empty()) { + _fetched_status_requests.swap(_status_to_do); + } +} + +void +Distributor::handle_status_requests() +{ + for (auto& s : _fetched_status_requests) { + s->getReporter().reportStatus(s->getStream(), s->getPath()); + s->notifyCompleted(); + } + if (!_fetched_status_requests.empty()) { + _fetched_status_requests.clear(); + signal_work_was_done(); + } +} + +void +Distributor::signal_work_was_done() +{ + _tickResult = framework::ThreadWaitInfo::MORE_WORK_ENQUEUED; +} + vespalib::string Distributor::getReportContentType(const framework::HttpUrlPath& path) const { - return _stripe->getReportContentType(path); + assert(!_use_legacy_mode); + if (path.hasAttribute("page")) { + if (path.getAttribute("page") == "buckets") { + return "text/html"; + } else { + return "application/xml"; + } + } else { + return "text/html"; + } } std::string Distributor::getActiveIdealStateOperations() const { - return _stripe->getActiveIdealStateOperations(); + // TODO STRIPE need to aggregate status responses _across_ stripes..! + if (_use_legacy_mode) { + return _stripe->getActiveIdealStateOperations(); + } else { + auto guard = _stripe_accessor->rendezvous_and_hold_all(); + return first_stripe().getActiveIdealStateOperations(); + } } bool Distributor::reportStatus(std::ostream& out, const framework::HttpUrlPath& path) const { - return _stripe->reportStatus(out, path); + assert(!_use_legacy_mode); + if (!path.hasAttribute("page") || path.getAttribute("page") == "buckets") { + framework::PartlyHtmlStatusReporter htmlReporter(*this); + htmlReporter.reportHtmlHeader(out, path); + if (!path.hasAttribute("page")) { + out << "<a href=\"?page=pending\">Count of pending messages to " + << "storage nodes</a><br><a href=\"?page=maintenance&show=50\">" + << "List maintenance queue (adjust show parameter to see more " + << "operations, -1 for all)</a><br>\n<a href=\"?page=buckets\">" + << "List all buckets, highlight non-ideal state</a><br>\n"; + } else { + auto guard = _stripe_accessor->rendezvous_and_hold_all(); + const auto& op_ctx = _component; + for (const auto& space : op_ctx.bucket_space_repo()) { + out << "<h2>" << document::FixedBucketSpaces::to_string(space.first) << " - " << space.first << "</h2>\n"; + guard->report_bucket_db_status(space.first, out); + } + } + htmlReporter.reportHtmlFooter(out, path); + } else { + framework::PartlyXmlStatusReporter xmlReporter(*this, out, path); + using namespace vespalib::xml; + std::string page(path.getAttribute("page")); + + if (page == "pending") { + auto guard = _stripe_accessor->rendezvous_and_hold_all(); + auto stats = guard->pending_operation_stats(); + xmlReporter << XmlTag("pending") + << XmlAttribute("externalload", stats.external_load_operations) + << XmlAttribute("maintenance", stats.maintenance_operations) + << XmlEndTag(); + } + } + return true; } bool Distributor::handleStatusRequest(const DelegatedStatusRequest& request) const { - // TODO STRIPE need to aggregate status responses _across_ stripes..! - return _stripe->handleStatusRequest(request); + assert(!_use_legacy_mode); + auto wrappedRequest = std::make_shared<DistributorStatus>(request); + { + framework::TickingLockGuard guard(_threadPool.freezeCriticalTicks()); + _status_to_do.push_back(wrappedRequest); + guard.broadcast(); + } + wrappedRequest->waitForCompletion(); + return true; } } diff --git a/storage/src/vespa/storage/distributor/distributor.h b/storage/src/vespa/storage/distributor/distributor.h index 0420f1b1f22..4257657816a 100644 --- a/storage/src/vespa/storage/distributor/distributor.h +++ b/storage/src/vespa/storage/distributor/distributor.h @@ -40,8 +40,9 @@ class BucketDBUpdater; class DistributorBucketSpaceRepo; class DistributorStatus; class DistributorStripe; +class DistributorStripePool; +class StripeAccessor; class OperationSequencer; -class LegacySingleStripeAccessor; class OwnershipTransferSafeTimePointCalculator; class SimpleMaintenanceScanner; class ThrottlingOperationStarter; @@ -118,6 +119,10 @@ private: friend class DistributorTestUtil; friend class MetricUpdateHook; + // TODO STRIPE remove + DistributorStripe& first_stripe() noexcept; + const DistributorStripe& first_stripe() const noexcept; + void setNodeStateUp(); bool handleMessage(const std::shared_ptr<api::StorageMessage>& msg); @@ -163,21 +168,37 @@ private: void propagateInternalScanMetricsToExternal(); void scanAllBuckets(); void enableNextConfig(); + void fetch_status_requests(); + void handle_status_requests(); + void signal_work_was_done(); void enableNextDistribution(); void propagateDefaultDistribution(std::shared_ptr<const lib::Distribution>); + void dispatch_to_main_distributor_thread_queue(const std::shared_ptr<api::StorageMessage>& msg); + void fetch_external_messages(); + void process_fetched_external_messages(); + + using MessageQueue = std::vector<std::shared_ptr<api::StorageMessage>>; + DistributorComponentRegister& _comp_reg; std::shared_ptr<DistributorMetricSet> _metrics; ChainedMessageSender* _messageSender; const bool _use_legacy_mode; // TODO STRIPE multiple stripes...! This is for proof of concept of wiring. - std::unique_ptr<DistributorStripe> _stripe; - std::unique_ptr<LegacySingleStripeAccessor> _stripe_accessor; + std::unique_ptr<DistributorStripe> _stripe; + std::unique_ptr<DistributorStripePool> _stripe_pool; + std::vector<std::unique_ptr<DistributorStripe>> _stripes; + std::unique_ptr<StripeAccessor> _stripe_accessor; + MessageQueue _message_queue; // Queue for top-level ops + MessageQueue _fetched_messages; distributor::DistributorComponent _component; std::shared_ptr<const DistributorConfiguration> _total_config; std::unique_ptr<BucketDBUpdater> _bucket_db_updater; StatusReporterDelegate _distributorStatusDelegate; + std::unique_ptr<StatusReporterDelegate> _bucket_db_status_delegate; framework::TickingThreadPool& _threadPool; + mutable std::vector<std::shared_ptr<DistributorStatus>> _status_to_do; + mutable std::vector<std::shared_ptr<DistributorStatus>> _fetched_status_requests; framework::ThreadWaitInfo _tickResult; MetricUpdateHook _metricUpdateHook; DistributorHostInfoReporter _hostInfoReporter; diff --git a/storage/src/vespa/storage/distributor/distributor_stripe.cpp b/storage/src/vespa/storage/distributor/distributor_stripe.cpp index 1f6a5b318fd..cfa30e04642 100644 --- a/storage/src/vespa/storage/distributor/distributor_stripe.cpp +++ b/storage/src/vespa/storage/distributor/distributor_stripe.cpp @@ -60,6 +60,7 @@ DistributorStripe::DistributorStripe(DistributorComponentRegister& compReg, _externalOperationHandler(_component, _component, getMetrics(), getMessageSender(), *_operation_sequencer, *this, _component, _idealStateManager, _operationOwner), + _external_message_mutex(), _threadPool(threadPool), _doneInitializeHandler(doneInitHandler), _doneInitializing(false), @@ -84,7 +85,10 @@ DistributorStripe::DistributorStripe(DistributorComponentRegister& compReg, _must_send_updated_host_info(false), _use_legacy_mode(use_legacy_mode) { - _bucketDBStatusDelegate.registerStatusPage(); + if (use_legacy_mode) { + _distributorStatusDelegate.registerStatusPage(); + _bucketDBStatusDelegate.registerStatusPage(); + } propagateDefaultDistribution(_component.getDistribution()); propagateClusterStates(); }; @@ -168,14 +172,19 @@ DistributorStripe::handle_or_enqueue_message(const std::shared_ptr<api::StorageM if (_externalOperationHandler.try_handle_message_outside_main_thread(msg)) { return true; } - // TODO STRIPE redesign how message queue guarding and wakeup is performed. - // Currently involves a _thread pool global_ lock transitively via tick guard! - framework::TickingLockGuard guard(_threadPool.freezeCriticalTicks()); MBUS_TRACE(msg->getTrace(), 9, "Distributor: Added to message queue. Thread state: " + _threadPool.getStatus()); - _messageQueue.push_back(msg); - guard.broadcast(); + if (_use_legacy_mode) { + // TODO STRIPE remove + framework::TickingLockGuard guard(_threadPool.freezeCriticalTicks()); + _messageQueue.push_back(msg); + guard.broadcast(); + } else { + std::lock_guard lock(_external_message_mutex); + _messageQueue.push_back(msg); + // Caller has the responsibility to wake up correct stripe + } return true; } @@ -727,6 +736,7 @@ DistributorStripe::scanNextBucket() void DistributorStripe::send_updated_host_info_if_required() { if (_must_send_updated_host_info) { + // TODO STRIPE how to handle with multiple stripes? _component.getStateUpdater().immediately_send_get_node_state_replies(); _must_send_updated_host_info = false; } @@ -745,10 +755,9 @@ framework::ThreadWaitInfo DistributorStripe::doCriticalTick(framework::ThreadIndex) { _tickResult = framework::ThreadWaitInfo::NO_MORE_CRITICAL_WORK_KNOWN; - if (_use_legacy_mode) { - enableNextDistribution(); - enableNextConfig(); - } + assert(_use_legacy_mode); + enableNextDistribution(); + enableNextConfig(); fetchStatusRequests(); fetchExternalMessages(); return _tickResult; @@ -758,7 +767,13 @@ framework::ThreadWaitInfo DistributorStripe::doNonCriticalTick(framework::ThreadIndex) { _tickResult = framework::ThreadWaitInfo::NO_MORE_CRITICAL_WORK_KNOWN; - handleStatusRequests(); + if (!_use_legacy_mode) { + std::lock_guard lock(_external_message_mutex); + fetchExternalMessages(); + } + if (_use_legacy_mode) { + handleStatusRequests(); + } startExternalOperations(); if (initializing()) { _bucketDBUpdater.resendDelayedMessages(); @@ -830,6 +845,7 @@ DistributorStripe::propagate_config_snapshot_to_internal_components() void DistributorStripe::fetchStatusRequests() { + assert(_use_legacy_mode); if (_fetchedStatusRequests.empty()) { _fetchedStatusRequests.swap(_statusToDo); } @@ -845,6 +861,7 @@ DistributorStripe::fetchExternalMessages() void DistributorStripe::handleStatusRequests() { + assert(_use_legacy_mode); uint32_t sz = _fetchedStatusRequests.size(); for (uint32_t i = 0; i < sz; ++i) { auto& s = *_fetchedStatusRequests[i]; @@ -860,6 +877,7 @@ DistributorStripe::handleStatusRequests() vespalib::string DistributorStripe::getReportContentType(const framework::HttpUrlPath& path) const { + assert(_use_legacy_mode); if (path.hasAttribute("page")) { if (path.getAttribute("page") == "buckets") { return "text/html"; @@ -883,10 +901,12 @@ DistributorStripe::getActiveOperations() const return _operationOwner.toString(); } +// TODO STRIPE remove this; delegated to top-level Distributor only bool DistributorStripe::reportStatus(std::ostream& out, const framework::HttpUrlPath& path) const { + assert(_use_legacy_mode); if (!path.hasAttribute("page") || path.getAttribute("page") == "buckets") { framework::PartlyHtmlStatusReporter htmlReporter(*this); htmlReporter.reportHtmlHeader(out, path); @@ -909,8 +929,7 @@ DistributorStripe::reportStatus(std::ostream& out, if (page == "pending") { xmlReporter << XmlTag("pending") << XmlAttribute("externalload", _operationOwner.size()) - << XmlAttribute("maintenance", - _maintenanceOperationOwner.size()) + << XmlAttribute("maintenance",_maintenanceOperationOwner.size()) << XmlEndTag(); } else if (page == "maintenance") { // Need new page @@ -920,9 +939,11 @@ DistributorStripe::reportStatus(std::ostream& out, return true; } +// TODO STRIPE remove this; delegated to top-level Distributor only bool DistributorStripe::handleStatusRequest(const DelegatedStatusRequest& request) const { + assert(_use_legacy_mode); auto wrappedRequest = std::make_shared<DistributorStatus>(request); { framework::TickingLockGuard guard(_threadPool.freezeCriticalTicks()); @@ -933,4 +954,10 @@ DistributorStripe::handleStatusRequest(const DelegatedStatusRequest& request) co return true; } +StripeAccessGuard::PendingOperationStats +DistributorStripe::pending_operation_stats() const +{ + return {_operationOwner.size(), _maintenanceOperationOwner.size()}; +} + } diff --git a/storage/src/vespa/storage/distributor/distributor_stripe.h b/storage/src/vespa/storage/distributor/distributor_stripe.h index 7b34367cecb..efded7d29d5 100644 --- a/storage/src/vespa/storage/distributor/distributor_stripe.h +++ b/storage/src/vespa/storage/distributor/distributor_stripe.h @@ -22,6 +22,7 @@ #include <vespa/storageapi/message/state.h> #include <vespa/storageframework/generic/metric/metricupdatehook.h> #include <vespa/storageframework/generic/thread/tickingthread.h> +#include <mutex> #include <queue> #include <unordered_map> @@ -116,6 +117,8 @@ public: bool handleStatusRequest(const DelegatedStatusRequest& request) const override; + StripeAccessGuard::PendingOperationStats pending_operation_stats() const; + std::string getActiveIdealStateOperations() const; std::string getActiveOperations() const; @@ -193,7 +196,6 @@ private: friend class DistributorTestUtil; friend class MetricUpdateHook; friend class Distributor; - friend class LegacySingleStripeAccessGuard; friend class MultiThreadedStripeAccessGuard; bool handleMessage(const std::shared_ptr<api::StorageMessage>& msg); @@ -298,6 +300,7 @@ private: std::vector<std::shared_ptr<api::StorageMessage>>, IndirectHigherPriority >; + mutable std::mutex _external_message_mutex; MessageQueue _messageQueue; ClientRequestPriorityQueue _client_request_priority_queue; MessageQueue _fetchedMessages; diff --git a/storage/src/vespa/storage/distributor/distributor_stripe_pool.h b/storage/src/vespa/storage/distributor/distributor_stripe_pool.h index 9149482cd5d..4ac52b0ede8 100644 --- a/storage/src/vespa/storage/distributor/distributor_stripe_pool.h +++ b/storage/src/vespa/storage/distributor/distributor_stripe_pool.h @@ -62,10 +62,10 @@ public: void park_all_threads() noexcept; void unpark_all_threads() noexcept; - [[nodiscard]] const DistributorStripeThread& stripe(size_t idx) const noexcept { + [[nodiscard]] const DistributorStripeThread& stripe_thread(size_t idx) const noexcept { return *_stripes[idx]; } - [[nodiscard]] DistributorStripeThread& stripe(size_t idx) noexcept { + [[nodiscard]] DistributorStripeThread& stripe_thread(size_t idx) noexcept { return *_stripes[idx]; } [[nodiscard]] size_t stripe_count() const noexcept { return _stripes.size(); } diff --git a/storage/src/vespa/storage/distributor/distributor_stripe_thread.cpp b/storage/src/vespa/storage/distributor/distributor_stripe_thread.cpp index 7359fbe5cf8..372736b8d7d 100644 --- a/storage/src/vespa/storage/distributor/distributor_stripe_thread.cpp +++ b/storage/src/vespa/storage/distributor/distributor_stripe_thread.cpp @@ -70,7 +70,6 @@ void DistributorStripeThread::wait_until_event_notified_or_timed_out() noexcept void DistributorStripeThread::wait_until_unparked() noexcept { std::unique_lock lock(_mutex); - assert(should_park_relaxed()); // _should_park is always written within _mutex, relaxed load is safe. _park_cond.wait(lock, [this]{ return !should_park_relaxed(); }); } diff --git a/storage/src/vespa/storage/distributor/distributor_stripe_thread.h b/storage/src/vespa/storage/distributor/distributor_stripe_thread.h index b02d733895e..60f10889afd 100644 --- a/storage/src/vespa/storage/distributor/distributor_stripe_thread.h +++ b/storage/src/vespa/storage/distributor/distributor_stripe_thread.h @@ -53,6 +53,9 @@ public: TickableStripe* operator->() noexcept { return &_stripe; } const TickableStripe* operator->() const noexcept { return &_stripe; } + + TickableStripe& stripe() noexcept { return _stripe; } + const TickableStripe& stripe() const noexcept { return _stripe; } private: [[nodiscard]] bool should_stop_thread_relaxed() const noexcept { return _should_stop.load(std::memory_order_relaxed); diff --git a/storage/src/vespa/storage/distributor/idealstatemanager.cpp b/storage/src/vespa/storage/distributor/idealstatemanager.cpp index a090f00300b..17f3911c6ee 100644 --- a/storage/src/vespa/storage/distributor/idealstatemanager.cpp +++ b/storage/src/vespa/storage/distributor/idealstatemanager.cpp @@ -288,8 +288,6 @@ IdealStateManager::getBucketStatus( } void IdealStateManager::dump_bucket_space_db_status(document::BucketSpace bucket_space, std::ostream& out) const { - out << "<h2>" << document::FixedBucketSpaces::to_string(bucket_space) << " - " << bucket_space << "</h2>\n"; - StatusBucketVisitor proc(*this, bucket_space, out); auto &distributorBucketSpace(_bucketSpaceRepo.get(bucket_space)); distributorBucketSpace.getBucketDatabase().forEach(proc); @@ -300,6 +298,7 @@ void IdealStateManager::getBucketStatus(std::ostream& out) const { _distributorComponent.getDistributor().getClusterStateBundle().getVersion()); for (auto& space : _bucketSpaceRepo) { + out << "<h2>" << document::FixedBucketSpaces::to_string(space.first) << " - " << space.first << "</h2>\n"; dump_bucket_space_db_status(space.first, out); } } diff --git a/storage/src/vespa/storage/distributor/idealstatemanager.h b/storage/src/vespa/storage/distributor/idealstatemanager.h index ebcaad4cf96..0bffed449ef 100644 --- a/storage/src/vespa/storage/distributor/idealstatemanager.h +++ b/storage/src/vespa/storage/distributor/idealstatemanager.h @@ -68,6 +68,9 @@ public: IdealStateMetricSet& getMetrics() { return *_metrics; } + + void dump_bucket_space_db_status(document::BucketSpace bucket_space, std::ostream& out) const; + void getBucketStatus(std::ostream& out) const; // HtmlStatusReporter @@ -126,7 +129,6 @@ private: void getBucketStatus(document::BucketSpace bucketSpace, const BucketDatabase::ConstEntryRef& entry, NodeMaintenanceStatsTracker& statsTracker, std::ostream& out) const; - void dump_bucket_space_db_status(document::BucketSpace bucket_space, std::ostream& out) const; }; } diff --git a/storage/src/vespa/storage/distributor/legacy_single_stripe_accessor.cpp b/storage/src/vespa/storage/distributor/legacy_single_stripe_accessor.cpp deleted file mode 100644 index 0c6c0206608..00000000000 --- a/storage/src/vespa/storage/distributor/legacy_single_stripe_accessor.cpp +++ /dev/null @@ -1,92 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include "legacy_single_stripe_accessor.h" -#include "distributor_stripe.h" - -namespace storage::distributor { - -LegacySingleStripeAccessGuard::LegacySingleStripeAccessGuard(LegacySingleStripeAccessor& accessor, - DistributorStripe& stripe) - : _accessor(accessor), - _stripe(stripe) -{} - -LegacySingleStripeAccessGuard::~LegacySingleStripeAccessGuard() { - _accessor.mark_guard_released(); -} - -void LegacySingleStripeAccessGuard::update_total_distributor_config(std::shared_ptr<const DistributorConfiguration> config) { - _stripe.update_total_distributor_config(std::move(config)); -} - -void LegacySingleStripeAccessGuard::update_distribution_config(const BucketSpaceDistributionConfigs& new_configs) { - _stripe.update_distribution_config(new_configs); -} - -void LegacySingleStripeAccessGuard::set_pending_cluster_state_bundle(const lib::ClusterStateBundle& pending_state) { - _stripe.getBucketSpaceRepo().set_pending_cluster_state_bundle(pending_state); - // TODO STRIPE also read only repo? -} - -void LegacySingleStripeAccessGuard::clear_pending_cluster_state_bundle() { - _stripe.getBucketSpaceRepo().clear_pending_cluster_state_bundle(); - // TODO STRIPE also read only repo? -} - -void LegacySingleStripeAccessGuard::enable_cluster_state_bundle(const lib::ClusterStateBundle& new_state) { - _stripe.enableClusterStateBundle(new_state); -} - -void LegacySingleStripeAccessGuard::notify_distribution_change_enabled() { - _stripe.notifyDistributionChangeEnabled(); -} - -PotentialDataLossReport -LegacySingleStripeAccessGuard::remove_superfluous_buckets(document::BucketSpace bucket_space, - const lib::ClusterState& new_state, - bool is_distribution_change) -{ - return _stripe.bucket_db_updater().remove_superfluous_buckets(bucket_space, new_state, is_distribution_change); -} - -void -LegacySingleStripeAccessGuard::merge_entries_into_db(document::BucketSpace bucket_space, - api::Timestamp gathered_at_timestamp, - const lib::Distribution& distribution, - const lib::ClusterState& new_state, - const char* storage_up_states, - const std::unordered_set<uint16_t>& outdated_nodes, - const std::vector<dbtransition::Entry>& entries) -{ - _stripe.bucket_db_updater().merge_entries_into_db(bucket_space, gathered_at_timestamp, distribution, - new_state, storage_up_states, outdated_nodes, entries); -} - -void LegacySingleStripeAccessGuard::update_read_snapshot_before_db_pruning() { - _stripe.bucket_db_updater().update_read_snapshot_before_db_pruning(); -} - -void LegacySingleStripeAccessGuard::update_read_snapshot_after_db_pruning(const lib::ClusterStateBundle& new_state) { - _stripe.bucket_db_updater().update_read_snapshot_after_db_pruning(new_state); -} - -void LegacySingleStripeAccessGuard::update_read_snapshot_after_activation(const lib::ClusterStateBundle& activated_state) { - _stripe.bucket_db_updater().update_read_snapshot_after_activation(activated_state); -} - -void LegacySingleStripeAccessGuard::clear_read_only_bucket_repo_databases() { - _stripe.bucket_db_updater().clearReadOnlyBucketRepoDatabases(); -} - -std::unique_ptr<StripeAccessGuard> LegacySingleStripeAccessor::rendezvous_and_hold_all() { - // For sanity checking during development. - assert(!_guard_held); - _guard_held = true; - return std::make_unique<LegacySingleStripeAccessGuard>(*this, _stripe); -} - -void LegacySingleStripeAccessor::mark_guard_released() { - assert(_guard_held); - _guard_held = false; -} - -} diff --git a/storage/src/vespa/storage/distributor/legacy_single_stripe_accessor.h b/storage/src/vespa/storage/distributor/legacy_single_stripe_accessor.h deleted file mode 100644 index caf1e397e5b..00000000000 --- a/storage/src/vespa/storage/distributor/legacy_single_stripe_accessor.h +++ /dev/null @@ -1,69 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#pragma once - -#include "stripe_access_guard.h" - -namespace storage::distributor { - -class DistributorStripe; -class LegacySingleStripeAccessor; - -/** - * Very simple stripe access guard which expects the caller and its single stripe to run in the - * same thread. This means there's no actual striping of operations or any thread synchronization - * performed. Only intended as a stop-gap while we have legacy stripe behavior. - */ -class LegacySingleStripeAccessGuard : public StripeAccessGuard { - LegacySingleStripeAccessor& _accessor; - DistributorStripe& _stripe; -public: - LegacySingleStripeAccessGuard(LegacySingleStripeAccessor& accessor, - DistributorStripe& stripe); - ~LegacySingleStripeAccessGuard() override; - - void update_total_distributor_config(std::shared_ptr<const DistributorConfiguration> config) override; - - void update_distribution_config(const BucketSpaceDistributionConfigs& new_configs) override; - void set_pending_cluster_state_bundle(const lib::ClusterStateBundle& pending_state) override; - void clear_pending_cluster_state_bundle() override; - void enable_cluster_state_bundle(const lib::ClusterStateBundle& new_state) override; - void notify_distribution_change_enabled() override; - - PotentialDataLossReport remove_superfluous_buckets(document::BucketSpace bucket_space, - const lib::ClusterState& new_state, - bool is_distribution_change) override; - void merge_entries_into_db(document::BucketSpace bucket_space, - api::Timestamp gathered_at_timestamp, - const lib::Distribution& distribution, - const lib::ClusterState& new_state, - const char* storage_up_states, - const std::unordered_set<uint16_t>& outdated_nodes, - const std::vector<dbtransition::Entry>& entries) override; - - void update_read_snapshot_before_db_pruning() override; - void update_read_snapshot_after_db_pruning(const lib::ClusterStateBundle& new_state) override; - void update_read_snapshot_after_activation(const lib::ClusterStateBundle& activated_state) override; - void clear_read_only_bucket_repo_databases() override; -}; - -/** - * Impl of StripeAccessor which creates LegacySingleStripeAccessGuards bound to a single stripe. - */ -class LegacySingleStripeAccessor : public StripeAccessor { - DistributorStripe& _stripe; - bool _guard_held; - - friend class LegacySingleStripeAccessGuard; -public: - explicit LegacySingleStripeAccessor(DistributorStripe& stripe) - : _stripe(stripe), - _guard_held(false) - {} - ~LegacySingleStripeAccessor() override = default; - - std::unique_ptr<StripeAccessGuard> rendezvous_and_hold_all() override; -private: - void mark_guard_released(); -}; - -} diff --git a/storage/src/vespa/storage/distributor/multi_threaded_stripe_access_guard.cpp b/storage/src/vespa/storage/distributor/multi_threaded_stripe_access_guard.cpp index 6bc9c03158a..a4a59745e3e 100644 --- a/storage/src/vespa/storage/distributor/multi_threaded_stripe_access_guard.cpp +++ b/storage/src/vespa/storage/distributor/multi_threaded_stripe_access_guard.cpp @@ -21,6 +21,10 @@ MultiThreadedStripeAccessGuard::~MultiThreadedStripeAccessGuard() { _accessor.mark_guard_released(); } +void MultiThreadedStripeAccessGuard::flush_and_close() { + first_stripe().flush_and_close(); +} + void MultiThreadedStripeAccessGuard::update_total_distributor_config(std::shared_ptr<const DistributorConfiguration> config) { // TODO STRIPE multiple stripes first_stripe().update_total_distributor_config(std::move(config)); @@ -94,8 +98,33 @@ void MultiThreadedStripeAccessGuard::clear_read_only_bucket_repo_databases() { first_stripe().bucket_db_updater().clearReadOnlyBucketRepoDatabases(); } +void MultiThreadedStripeAccessGuard::report_bucket_db_status(document::BucketSpace bucket_space, std::ostream& out) const { + // TODO STRIPE multiple stripes + first_stripe().ideal_state_manager().dump_bucket_space_db_status(bucket_space, out); +} + +StripeAccessGuard::PendingOperationStats +MultiThreadedStripeAccessGuard::pending_operation_stats() const { + // TODO STRIPE multiple stripes + return first_stripe().pending_operation_stats(); +} + +void MultiThreadedStripeAccessGuard::report_single_bucket_requests(vespalib::xml::XmlOutputStream& xos) const { + // TODO STRIPE multiple stripes + first_stripe().bucket_db_updater().report_single_bucket_requests(xos); +} + +void MultiThreadedStripeAccessGuard::report_delayed_single_bucket_requests(vespalib::xml::XmlOutputStream& xos) const { + // TODO STRIPE multiple stripes + first_stripe().bucket_db_updater().report_delayed_single_bucket_requests(xos); +} + DistributorStripe& MultiThreadedStripeAccessGuard::first_stripe() noexcept { - return dynamic_cast<DistributorStripe&>(_stripe_pool.stripe(0)); + return dynamic_cast<DistributorStripe&>(_stripe_pool.stripe_thread(0).stripe()); +} + +const DistributorStripe& MultiThreadedStripeAccessGuard::first_stripe() const noexcept { + return dynamic_cast<const DistributorStripe&>(_stripe_pool.stripe_thread(0).stripe()); } std::unique_ptr<StripeAccessGuard> MultiThreadedStripeAccessor::rendezvous_and_hold_all() { diff --git a/storage/src/vespa/storage/distributor/multi_threaded_stripe_access_guard.h b/storage/src/vespa/storage/distributor/multi_threaded_stripe_access_guard.h index 376eccd1c4a..a44f069d615 100644 --- a/storage/src/vespa/storage/distributor/multi_threaded_stripe_access_guard.h +++ b/storage/src/vespa/storage/distributor/multi_threaded_stripe_access_guard.h @@ -27,6 +27,8 @@ public: DistributorStripePool& stripe_pool); ~MultiThreadedStripeAccessGuard() override; + void flush_and_close() override; + void update_total_distributor_config(std::shared_ptr<const DistributorConfiguration> config) override; void update_distribution_config(const BucketSpaceDistributionConfigs& new_configs) override; @@ -50,9 +52,16 @@ public: void update_read_snapshot_after_db_pruning(const lib::ClusterStateBundle& new_state) override; void update_read_snapshot_after_activation(const lib::ClusterStateBundle& activated_state) override; void clear_read_only_bucket_repo_databases() override; + + void report_bucket_db_status(document::BucketSpace bucket_space, std::ostream& out) const override; + PendingOperationStats pending_operation_stats() const override; + void report_single_bucket_requests(vespalib::xml::XmlOutputStream& xos) const override; + void report_delayed_single_bucket_requests(vespalib::xml::XmlOutputStream& xos) const override; + private: // TODO STRIPE remove once multi threaded stripe support is implemented DistributorStripe& first_stripe() noexcept; + const DistributorStripe& first_stripe() const noexcept; }; /** @@ -65,7 +74,7 @@ class MultiThreadedStripeAccessor : public StripeAccessor { friend class MultiThreadedStripeAccessGuard; public: - MultiThreadedStripeAccessor(DistributorStripePool& stripe_pool) + explicit MultiThreadedStripeAccessor(DistributorStripePool& stripe_pool) : _stripe_pool(stripe_pool), _guard_held(false) {} diff --git a/storage/src/vespa/storage/distributor/stripe_access_guard.h b/storage/src/vespa/storage/distributor/stripe_access_guard.h index 69aae755dec..a1779a4eb4f 100644 --- a/storage/src/vespa/storage/distributor/stripe_access_guard.h +++ b/storage/src/vespa/storage/distributor/stripe_access_guard.h @@ -16,6 +16,8 @@ class Distribution; namespace storage { class DistributorConfiguration; } +namespace vespalib::xml { class XmlOutputStream; } + namespace storage::distributor { /** @@ -28,6 +30,8 @@ class StripeAccessGuard { public: virtual ~StripeAccessGuard() = default; + virtual void flush_and_close() = 0; + virtual void update_total_distributor_config(std::shared_ptr<const DistributorConfiguration> config) = 0; virtual void update_distribution_config(const BucketSpaceDistributionConfigs& new_configs) = 0; @@ -52,6 +56,22 @@ public: virtual void update_read_snapshot_after_activation(const lib::ClusterStateBundle& activated_state) = 0; virtual void clear_read_only_bucket_repo_databases() = 0; + // TODO STRIPE: Add merge() function. + struct PendingOperationStats { + size_t external_load_operations; + size_t maintenance_operations; + PendingOperationStats(size_t external_load_operations_in, + size_t maintenance_operations_in) + : external_load_operations(external_load_operations_in), + maintenance_operations(maintenance_operations_in) {} + }; + + // Functions used for state reporting + virtual void report_bucket_db_status(document::BucketSpace bucket_space, std::ostream& out) const = 0; + virtual PendingOperationStats pending_operation_stats() const = 0; + virtual void report_single_bucket_requests(vespalib::xml::XmlOutputStream& xos) const = 0; + virtual void report_delayed_single_bucket_requests(vespalib::xml::XmlOutputStream& xos) const = 0; + }; /** diff --git a/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.cpp b/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.cpp index 8c24effa616..77151da19bc 100644 --- a/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.cpp +++ b/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.cpp @@ -21,9 +21,10 @@ #include <vespa/log/bufferedlogger.h> LOG_SETUP(".distributor.stripe_bucket_db_updater"); +using document::BucketSpace; using storage::lib::Node; using storage::lib::NodeType; -using document::BucketSpace; +using vespalib::xml::XmlAttribute; namespace storage::distributor { @@ -960,20 +961,30 @@ StripeBucketDBUpdater::reportXmlStatus(vespalib::xml::XmlOutputStream& xos, } xos << XmlEndTag() << XmlTag("single_bucket_requests"); - for (const auto & entry : _sentMessages) - { - entry.second.print_xml_tag(xos, XmlAttribute("sendtimestamp", entry.second.timestamp)); - } + report_single_bucket_requests(xos); xos << XmlEndTag() << XmlTag("delayed_single_bucket_requests"); - for (const auto & entry : _delayedRequests) - { - entry.second.print_xml_tag(xos, XmlAttribute("resendtimestamp", entry.first.getTime())); - } + report_delayed_single_bucket_requests(xos); xos << XmlEndTag() << XmlEndTag(); return ""; } +void +StripeBucketDBUpdater::report_single_bucket_requests(vespalib::xml::XmlOutputStream& xos) const +{ + for (const auto& entry : _sentMessages) { + entry.second.print_xml_tag(xos, XmlAttribute("sendtimestamp", entry.second.timestamp)); + } +} + +void +StripeBucketDBUpdater::report_delayed_single_bucket_requests(vespalib::xml::XmlOutputStream& xos) const +{ + for (const auto& entry : _delayedRequests) { + entry.second.print_xml_tag(xos, XmlAttribute("resendtimestamp", entry.first.getTime())); + } +} + StripeBucketDBUpdater::MergingNodeRemover::MergingNodeRemover( const lib::ClusterState& oldState, const lib::ClusterState& s, diff --git a/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h b/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h index 42fe0d5c29a..5f843b8ff33 100644 --- a/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h +++ b/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h @@ -59,6 +59,11 @@ public: vespalib::string reportXmlStatus(vespalib::xml::XmlOutputStream&, const framework::HttpUrlPath&) const; vespalib::string getReportContentType(const framework::HttpUrlPath&) const override; bool reportStatus(std::ostream&, const framework::HttpUrlPath&) const override; + + // Functions used for state reporting when a StripeAccessGuard is held. + void report_single_bucket_requests(vespalib::xml::XmlOutputStream& xos) const; + void report_delayed_single_bucket_requests(vespalib::xml::XmlOutputStream& xos) const; + void print(std::ostream& out, bool verbose, const std::string& indent) const; const DistributorNodeContext& node_context() const { return _node_ctx; } DistributorStripeOperationContext& operation_context() { return _op_ctx; } @@ -140,7 +145,6 @@ private: friend class DistributorTestUtil; // TODO refactor and rewire to avoid needing this direct meddling - friend class LegacySingleStripeAccessGuard; friend class MultiThreadedStripeAccessGuard; // Only to be used by tests that want to ensure both the BucketDBUpdater _and_ the Distributor |