diff options
110 files changed, 1640 insertions, 1207 deletions
diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index b67b496a7ce..c7b80dce778 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -287,7 +287,7 @@ "public" ], "methods": [ - "public void <init>(java.util.Optional, com.yahoo.config.application.api.DeploymentSpec$UpgradePolicy, java.util.Optional, java.util.List, java.util.List, java.lang.String, java.util.Optional, java.util.Optional, com.yahoo.config.application.api.Notifications)", + "public void <init>(java.util.Optional, com.yahoo.config.application.api.DeploymentSpec$UpgradePolicy, java.util.Optional, java.util.List, java.util.List, java.lang.String, java.util.Optional, java.util.Optional, com.yahoo.config.application.api.Notifications, java.util.List)", "public java.util.Optional globalServiceId()", "public com.yahoo.config.application.api.DeploymentSpec$UpgradePolicy upgradePolicy()", "public java.util.Optional majorVersion()", @@ -297,6 +297,7 @@ "public java.util.List steps()", "public java.util.List zones()", "public com.yahoo.config.application.api.Notifications notifications()", + "public java.util.List endpoints()", "public java.lang.String xmlForm()", "public boolean includes(com.yahoo.config.provision.Environment, java.util.Optional)", "public static com.yahoo.config.application.api.DeploymentSpec fromXml(java.io.Reader)", @@ -313,6 +314,22 @@ "public static final com.yahoo.config.application.api.DeploymentSpec empty" ] }, + "com.yahoo.config.application.api.Endpoint": { + "superClass": "java.lang.Object", + "interfaces": [], + "attributes": [ + "public" + ], + "methods": [ + "public void <init>(java.util.Optional, java.lang.String, java.util.Set)", + "public java.lang.String endpointId()", + "public java.lang.String containerId()", + "public java.util.Set regions()", + "public boolean equals(java.lang.Object)", + "public int hashCode()" + ], + "fields": [] + }, "com.yahoo.config.application.api.FileRegistry$Entry": { "superClass": "java.lang.Object", "interfaces": [], diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java index 66111b91aa0..ada5ee23a7c 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java @@ -46,7 +46,8 @@ public class DeploymentSpec { "<deployment version='1.0'/>", Optional.empty(), Optional.empty(), - Notifications.none()); + Notifications.none(), + List.of()); private final Optional<String> globalServiceId; private final UpgradePolicy upgradePolicy; @@ -57,10 +58,12 @@ public class DeploymentSpec { private final Optional<AthenzDomain> athenzDomain; private final Optional<AthenzService> athenzService; private final Notifications notifications; + private final List<Endpoint> endpoints; public DeploymentSpec(Optional<String> globalServiceId, UpgradePolicy upgradePolicy, Optional<Integer> majorVersion, List<ChangeBlocker> changeBlockers, List<Step> steps, String xmlForm, - Optional<AthenzDomain> athenzDomain, Optional<AthenzService> athenzService, Notifications notifications) { + Optional<AthenzDomain> athenzDomain, Optional<AthenzService> athenzService, Notifications notifications, + List<Endpoint> endpoints) { validateTotalDelay(steps); this.globalServiceId = globalServiceId; this.upgradePolicy = upgradePolicy; @@ -71,8 +74,10 @@ public class DeploymentSpec { this.athenzDomain = athenzDomain; this.athenzService = athenzService; this.notifications = notifications; + this.endpoints = List.copyOf(Objects.requireNonNull(endpoints, "Missing endpoints parameter")); validateZones(this.steps); validateAthenz(); + validateEndpoints(this.steps, globalServiceId, this.endpoints); } /** Throw an IllegalArgumentException if the total delay exceeds 24 hours */ @@ -94,6 +99,26 @@ public class DeploymentSpec { ensureUnique(zone, zones); } + /** Throw an IllegalArgumentException if an endpoint refers to a region that is not declared in 'prod' */ + private void validateEndpoints(List<Step> steps, Optional<String> globalServiceId, List<Endpoint> endpoints) { + if (globalServiceId.isPresent() && ! endpoints.isEmpty()) { + throw new IllegalArgumentException("Providing both 'endpoints' and 'global-service-id'. Use only 'endpoints'."); + } + + var stepZones = steps.stream() + .flatMap(s -> s.zones().stream()) + .flatMap(z -> z.region.stream()) + .collect(Collectors.toSet()); + + for (var endpoint : endpoints){ + for (var endpointRegion : endpoint.regions()) { + if (! stepZones.contains(endpointRegion)) { + throw new IllegalArgumentException("Region used in endpoint that is not declared in 'prod': " + endpointRegion); + } + } + } + } + /* * Throw an IllegalArgumentException if Athenz configuration violates: * domain not configured -> no zone can configure service @@ -101,16 +126,16 @@ public class DeploymentSpec { */ private void validateAthenz() { // If athenz domain is not set, athenz service cannot be set on any level - if (! athenzDomain.isPresent()) { + if (athenzDomain.isEmpty()) { for (DeclaredZone zone : zones()) { if(zone.athenzService().isPresent()) { throw new IllegalArgumentException("Athenz service configured for zone: " + zone + ", but Athenz domain is not configured"); } } // if athenz domain is not set, athenz service must be set implicitly or directly on all zones. - } else if(! athenzService.isPresent()) { + } else if (athenzService.isEmpty()) { for (DeclaredZone zone : zones()) { - if(! zone.athenzService().isPresent()) { + if (zone.athenzService().isEmpty()) { throw new IllegalArgumentException("Athenz domain is configured, but Athenz service not configured for zone: " + zone); } } @@ -199,6 +224,9 @@ public class DeploymentSpec { /** Returns the notification configuration */ public Notifications notifications() { return notifications; } + /** Returns the rotations configuration */ + public List<Endpoint> endpoints() { return endpoints; } + /** Returns the XML form of this spec, or null if it was not created by fromXml, nor is empty */ public String xmlForm() { return xmlForm; } @@ -369,7 +397,7 @@ public class DeploymentSpec { Optional<AthenzService> athenzService, Optional<String> testerFlavor) { if (environment != Environment.prod && region.isPresent()) throw new IllegalArgumentException("Non-prod environments cannot specify a region"); - if (environment == Environment.prod && ! region.isPresent()) + if (environment == Environment.prod && region.isEmpty()) throw new IllegalArgumentException("Prod environments must be specified with a region"); this.environment = environment; this.region = region; @@ -417,7 +445,7 @@ public class DeploymentSpec { @Override public String toString() { - return environment + ( region.isPresent() ? "." + region.get() : ""); + return environment + (region.map(regionName -> "." + regionName).orElse("")); } } diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java b/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java new file mode 100644 index 00000000000..73328027540 --- /dev/null +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java @@ -0,0 +1,57 @@ +package com.yahoo.config.application.api; + +import com.yahoo.config.provision.RegionName; + +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * Represents a (global) endpoint in 'deployments.xml'. It contains the name of the + * endpoint (endpointId) and the name of the container cluster that the endpoint + * should point to. + * + * If the endpointId is not set, it will default to the same as the containerId. + */ +public class Endpoint { + private final Optional<String> endpointId; + private final String containerId; + private final Set<RegionName> regions; + + public Endpoint(Optional<String> endpointId, String containerId, Set<String> regions) { + this.endpointId = endpointId; + this.containerId = containerId; + this.regions = Set.copyOf( + Objects.requireNonNull( + regions.stream().map(RegionName::from).collect(Collectors.toList()), + "Missing 'regions' parameter")); + } + + public String endpointId() { + return endpointId.orElse(containerId); + } + + public String containerId() { + return containerId; + } + + public Set<RegionName> regions() { + return regions; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Endpoint endpoint = (Endpoint) o; + return Objects.equals(endpointId, endpoint.endpointId) && + Objects.equals(containerId, endpoint.containerId) && + Objects.equals(regions, endpoint.regions); + } + + @Override + public int hashCode() { + return Objects.hash(endpointId, containerId, regions); + } +} diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java index 8e899b28a06..58795f6ea9e 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java @@ -6,6 +6,7 @@ import com.yahoo.config.application.api.DeploymentSpec.DeclaredZone; import com.yahoo.config.application.api.DeploymentSpec.Delay; import com.yahoo.config.application.api.DeploymentSpec.ParallelZones; import com.yahoo.config.application.api.DeploymentSpec.Step; +import com.yahoo.config.application.api.Endpoint; import com.yahoo.config.application.api.Notifications; import com.yahoo.config.application.api.Notifications.Role; import com.yahoo.config.application.api.Notifications.When; @@ -25,6 +26,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -40,6 +42,8 @@ public class DeploymentSpecXmlReader { private static final String stagingTag = "staging"; private static final String blockChangeTag = "block-change"; private static final String prodTag = "prod"; + private static final String endpointsTag = "endpoints"; + private static final String endpointTag = "endpoint"; private final boolean validate; @@ -123,7 +127,8 @@ public class DeploymentSpecXmlReader { xmlForm, athenzDomain, athenzService, - readNotifications(root)); + readNotifications(root), + readEndpoints(root)); } private Notifications readNotifications(Element root) { @@ -152,6 +157,38 @@ public class DeploymentSpecXmlReader { return Notifications.of(emailAddresses, emailRoles); } + private List<Endpoint> readEndpoints(Element root) { + final var endpointsElement = XML.getChild(root, endpointsTag); + if (endpointsElement == null) { return Collections.emptyList(); } + + final var endpoints = new ArrayList<Endpoint>(); + + for (var endpointElement : XML.getChildren(endpointsElement, endpointTag)) { + final Optional<String> rotationId = stringAttribute("id", endpointElement); + final Optional<String> containerId = stringAttribute("container-id", endpointElement); + final var regions = new HashSet<String>(); + + if (containerId.isEmpty()) { + throw new IllegalArgumentException("Missing 'container-id' from 'endpoint' tag."); + } + + for (var regionElement : XML.getChildren(endpointElement, "region")) { + var region = regionElement.getTextContent(); + if (region == null || region.isEmpty() || region.isBlank()) { + throw new IllegalArgumentException("Empty 'region' element in 'endpoint' tag."); + } + if (regions.contains(region)) { + throw new IllegalArgumentException("Duplicate 'region' element in 'endpoint' tag: " + region); + } + regions.add(region); + } + + endpoints.add(new Endpoint(rotationId, containerId.get(), regions)); + } + + return endpoints; + } + /** * Imposes some constraints on tag order which are not expressible in the schema */ diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java index 46e6dc457e9..3049d511bf1 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java @@ -1,9 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.application.api; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.yahoo.config.provision.Deployment; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import org.junit.Test; @@ -11,7 +9,11 @@ import org.junit.Test; import java.io.StringReader; import java.time.Instant; import java.time.ZoneId; +import java.util.Collections; +import java.util.List; import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; import static com.yahoo.config.application.api.Notifications.Role.author; import static com.yahoo.config.application.api.Notifications.When.failing; @@ -452,4 +454,43 @@ public class DeploymentSpecTest { assertEquals(Optional.of("d-2-8-50"), spec.steps().get(2).zones().get(0).testerFlavor()); } + @Test + public void noEndpoints() { + assertEquals(Collections.emptyList(), DeploymentSpec.fromXml("<deployment />").endpoints()); + } + + @Test + public void emptyEndpoints() { + final var spec = DeploymentSpec.fromXml("<deployment><endpoints/></deployment>"); + assertEquals(Collections.emptyList(), spec.endpoints()); + } + + @Test + public void someEndpoints() { + final var spec = DeploymentSpec.fromXml("" + + "<deployment>" + + " <prod>" + + " <region active=\"true\">us-east</region>" + + " </prod>" + + " <endpoints>" + + " <endpoint id=\"foo\" container-id=\"bar\">" + + " <region>us-east</region>" + + " </endpoint>" + + " <endpoint id=\"nalle\" container-id=\"frosk\" />" + + " <endpoint container-id=\"quux\" />" + + " </endpoints>" + + "</deployment>"); + + assertEquals( + List.of("foo", "nalle", "quux"), + spec.endpoints().stream().map(Endpoint::endpointId).collect(Collectors.toList()) + ); + + assertEquals( + List.of("bar", "frosk", "quux"), + spec.endpoints().stream().map(Endpoint::containerId).collect(Collectors.toList()) + ); + + assertEquals(Set.of(RegionName.from("us-east")), spec.endpoints().get(0).regions()); + } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java index dccd19e7c63..47c6b2dbb52 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java @@ -9,6 +9,7 @@ import ai.vespa.metricsproxy.core.MetricsConsumers; import ai.vespa.metricsproxy.core.MetricsManager; import ai.vespa.metricsproxy.core.MonitoringConfig; import ai.vespa.metricsproxy.core.VespaMetrics; +import ai.vespa.metricsproxy.http.GenericMetricsHandler; import ai.vespa.metricsproxy.metric.ExternalMetrics; import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensions; import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensionsConfig; @@ -20,6 +21,7 @@ import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.config.model.producer.AbstractConfigProducerRoot; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Zone; +import com.yahoo.osgi.provider.model.ComponentModel; import com.yahoo.search.config.QrStartConfig; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.admin.Admin; @@ -27,6 +29,7 @@ import com.yahoo.vespa.model.admin.monitoring.MetricSet; import com.yahoo.vespa.model.admin.monitoring.MetricsConsumer; import com.yahoo.vespa.model.admin.monitoring.Monitoring; import com.yahoo.vespa.model.container.ContainerCluster; +import com.yahoo.vespa.model.container.component.Handler; import java.nio.file.Path; import java.nio.file.Paths; @@ -66,6 +69,8 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC static final Path METRICS_PROXY_BUNDLE_FILE = absoluteBundlePath((Paths.get(METRICS_PROXY_NAME + JAR_WITH_DEPS.suffix))); static final String METRICS_PROXY_BUNDLE_NAME = "com.yahoo.vespa." + METRICS_PROXY_NAME; + private static final String METRICS_HANDLER_BINDING = "/metrics/v1/values"; + static final class AppDimensionNames { static final String ZONE = "zone"; static final String APPLICATION_ID = "applicationId"; // tenant.app.instance @@ -90,6 +95,7 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC addPlatformBundle(METRICS_PROXY_BUNDLE_FILE); addClusterComponents(); + addGenericMetricsHandler(); } private void addClusterComponents() { @@ -103,6 +109,14 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC addMetricsProxyComponent(VespaMetrics.class); } + private void addGenericMetricsHandler() { + Handler<AbstractConfigProducer<?>> metricsHandler = new Handler<>( + new ComponentModel(GenericMetricsHandler.class.getName(), null, METRICS_PROXY_BUNDLE_NAME, null)); + metricsHandler.addServerBindings("http://*" + METRICS_HANDLER_BINDING, + "http://*" + METRICS_HANDLER_BINDING + "/*"); + addComponent(metricsHandler); + } + @Override protected void doPrepare(DeployState deployState) { } diff --git a/config-model/src/main/resources/schema/deployment.rnc b/config-model/src/main/resources/schema/deployment.rnc index 15a4e70e33f..7b15a1c062d 100644 --- a/config-model/src/main/resources/schema/deployment.rnc +++ b/config-model/src/main/resources/schema/deployment.rnc @@ -10,6 +10,7 @@ start = element deployment { Upgrade? & BlockChange* & Notifications? & + Endpoints? & Test? & Staging? & Prod* @@ -73,3 +74,17 @@ Delay = element delay { Parallel = element parallel { Region* } + +EndpointRegion = element region { + text +} + +Endpoint = element endpoint { + attribute id { xsd:string }? & + attribute container-id { xsd:string } & + EndpointRegion* +} + +Endpoints = element endpoints { + Endpoint+ +} diff --git a/config-model/src/test/schema-test-files/deployment.xml b/config-model/src/test/schema-test-files/deployment.xml index ea65dbec661..0b30067d78a 100644 --- a/config-model/src/test/schema-test-files/deployment.xml +++ b/config-model/src/test/schema-test-files/deployment.xml @@ -20,4 +20,10 @@ <region active='true'>us-south-2</region> </parallel> </prod> + <endpoints> + <endpoint id="foo" container-id="bar"> + <region>us-east</region> + </endpoint> + <endpoint container-id="bar" /> + </endpoints> </deployment> diff --git a/config-provisioning/abi-spec.json b/config-provisioning/abi-spec.json index 66cc491d471..3cfa301894a 100644 --- a/config-provisioning/abi-spec.json +++ b/config-provisioning/abi-spec.json @@ -783,7 +783,8 @@ "public java.lang.String value()", "public boolean isPublic()", "public boolean isCd()", - "public static java.util.Set all()" + "public static java.util.Set all()", + "public static java.util.Set allOf(java.util.function.Predicate)" ], "fields": [ "public static final enum com.yahoo.config.provision.SystemName cd", diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/SystemName.java b/config-provisioning/src/main/java/com/yahoo/config/provision/SystemName.java index 088d0bdb3c3..ba462b9eb64 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/SystemName.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/SystemName.java @@ -3,6 +3,9 @@ package com.yahoo.config.provision; import java.util.EnumSet; import java.util.Set; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Systems in hosted Vespa @@ -72,4 +75,8 @@ public enum SystemName { public boolean isCd() { return isCd; } public static Set<SystemName> all() { return EnumSet.allOf(SystemName.class); } + + public static Set<SystemName> allOf(Predicate<SystemName> predicate) { + return Stream.of(values()).filter(predicate::test).collect(Collectors.toSet()); + } } diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/SystemNameTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/SystemNameTest.java index 6ffca4918a8..eb066958254 100644 --- a/config-provisioning/src/test/java/com/yahoo/config/provision/SystemNameTest.java +++ b/config-provisioning/src/test/java/com/yahoo/config/provision/SystemNameTest.java @@ -3,6 +3,8 @@ package com.yahoo.config.provision; import org.junit.Test; +import java.util.Set; + import static org.junit.Assert.assertEquals; /** @@ -15,4 +17,10 @@ public class SystemNameTest { assertEquals(name, SystemName.from(name.value())); } } + + @Test + public void allOf() { + assertEquals(Set.of(SystemName.cd, SystemName.PublicCd, SystemName.vaas), SystemName.allOf(SystemName::isCd)); + assertEquals(Set.of(SystemName.PublicCd, SystemName.Public, SystemName.vaas), SystemName.allOf(SystemName::isPublic)); + } }
\ No newline at end of file diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index 3aea22ce8b2..0ceec0ab00b 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -451,12 +451,16 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye } private Application getApplication(ApplicationId applicationId) { + return getApplication(applicationId, Optional.empty()); + } + + private Application getApplication(ApplicationId applicationId, Optional<Version> version) { try { Tenant tenant = tenantRepository.getTenant(applicationId.tenant()); if (tenant == null) throw new IllegalArgumentException("Tenant '" + applicationId.tenant() + "' not found"); long sessionId = getSessionIdForApplication(tenant, applicationId); RemoteSession session = tenant.getRemoteSessionRepo().getSession(sessionId, 0); - return session.ensureApplicationLoaded().getForVersionOrLatest(Optional.empty(), clock.instant()); + return session.ensureApplicationLoaded().getForVersionOrLatest(version, clock.instant()); } catch (Exception e) { log.log(LogLevel.WARNING, "Failed getting application for '" + applicationId + "'", e); throw e; @@ -500,12 +504,14 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye // ---------------- Convergence ---------------------------------------------------------------- - public HttpResponse checkServiceForConfigConvergence(ApplicationId applicationId, String hostAndPort, URI uri, Duration timeout) { - return convergeChecker.checkService(getApplication(applicationId), hostAndPort, uri, timeout); + public HttpResponse checkServiceForConfigConvergence(ApplicationId applicationId, String hostAndPort, URI uri, + Duration timeout, Optional<Version> vespaVersion) { + return convergeChecker.checkService(getApplication(applicationId, vespaVersion), hostAndPort, uri, timeout); } - public HttpResponse servicesToCheckForConfigConvergence(ApplicationId applicationId, URI uri, Duration timeoutPerService) { - return convergeChecker.servicesToCheck(getApplication(applicationId), uri, timeoutPerService); + public HttpResponse servicesToCheckForConfigConvergence(ApplicationId applicationId, URI uri, + Duration timeoutPerService, Optional<Version> vespaVersion) { + return convergeChecker.servicesToCheck(getApplication(applicationId, vespaVersion), uri, timeoutPerService); } // ---------------- Logs ---------------------------------------------------------------- diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ConfigConvergenceChecker.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ConfigConvergenceChecker.java index 4fbda42fdc7..97c1c9c89b0 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ConfigConvergenceChecker.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ConfigConvergenceChecker.java @@ -8,6 +8,7 @@ import com.yahoo.config.model.api.HostInfo; import com.yahoo.config.model.api.PortInfo; import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.log.LogLevel; import com.yahoo.slime.Cursor; import com.yahoo.vespa.config.server.http.JSONResponse; import org.glassfish.jersey.client.ClientProperties; @@ -43,6 +44,7 @@ import static com.yahoo.config.model.api.container.ContainerServiceType.QRSERVER */ public class ConfigConvergenceChecker extends AbstractComponent { + private static final java.util.logging.Logger log = java.util.logging.Logger.getLogger(ConfigConvergenceChecker.class.getName()); private static final ApplicationId routingApplicationId = ApplicationId.from("hosted-vespa", "routing", "default"); private static final String statePath = "/state/v1/"; private static final String configSubPath = "config"; @@ -68,6 +70,7 @@ public class ConfigConvergenceChecker extends AbstractComponent { /** Check all services in given application. Returns the minimum current generation of all services */ public ServiceListResponse servicesToCheck(Application application, URI requestUrl, Duration timeoutPerService) { + log.log(LogLevel.INFO, "Finding services to check config convergence for in '" + application); List<ServiceInfo> servicesToCheck = new ArrayList<>(); application.getModel().getHosts() .forEach(host -> host.getServices().stream() diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java index e04f83cc648..d9592dc9352 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.config.server.http.v2; import com.google.inject.Inject; +import com.yahoo.component.Version; import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationName; @@ -62,7 +63,8 @@ public class ApplicationHandler extends HttpHandler { if (isServiceConvergeRequest(request)) { // Expects both hostname and port in the request (hostname:port) String hostAndPort = getHostNameFromRequest(request); - return applicationRepository.checkServiceForConfigConvergence(applicationId, hostAndPort, request.getUri(), timeout); + return applicationRepository.checkServiceForConfigConvergence(applicationId, hostAndPort, request.getUri(), + timeout, getVespaVersionFromRequest(request)); } if (isClusterControllerStatusRequest(request)) { @@ -89,7 +91,8 @@ public class ApplicationHandler extends HttpHandler { } if (isServiceConvergeListRequest(request)) { - return applicationRepository.servicesToCheckForConfigConvergence(applicationId, request.getUri(), timeout); + return applicationRepository.servicesToCheckForConfigConvergence(applicationId, request.getUri(), timeout, + getVespaVersionFromRequest(request)); } if (isFiledistributionStatusRequest(request)) { @@ -225,6 +228,13 @@ public class ApplicationHandler extends HttpHandler { .build(); } + private static Optional<Version> getVespaVersionFromRequest(HttpRequest request) { + String vespaVersion = request.getProperty("vespaVersion"); + return (vespaVersion == null || vespaVersion.isEmpty()) + ? Optional.empty() + : Optional.of(Version.fromString(vespaVersion)); + } + private static class DeleteApplicationResponse extends JSONResponse { DeleteApplicationResponse(int status, ApplicationId applicationId) { super(status); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/DefaultRpcAuthorizerProvider.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/DefaultRpcAuthorizerProvider.java index ff502ff2cc0..c7bbecc157c 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/DefaultRpcAuthorizerProvider.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/DefaultRpcAuthorizerProvider.java @@ -8,6 +8,8 @@ import com.yahoo.container.di.componentgraph.Provider; import com.yahoo.security.tls.TransportSecurityUtils; import com.yahoo.vespa.config.server.host.HostRegistries; import com.yahoo.vespa.config.server.rpc.RequestHandlerProvider; +import com.yahoo.vespa.flags.FlagSource; +import com.yahoo.vespa.flags.Flags; /** * A provider for {@link RpcAuthorizer}. The instance provided is dependent on the configuration of the configserver. @@ -22,14 +24,29 @@ public class DefaultRpcAuthorizerProvider implements Provider<RpcAuthorizer> { public DefaultRpcAuthorizerProvider(ConfigserverConfig config, NodeIdentifier nodeIdentifier, HostRegistries hostRegistries, - RequestHandlerProvider handlerProvider) { - // TODO Re-enable once performance/stability issues with MultiTenantRpcAuthorizer are fixed + RequestHandlerProvider handlerProvider, + FlagSource flagSource) { + String authorizerMode = Flags.CONFIGSERVER_RPC_AUTHORIZER.bindTo(flagSource).value(); + boolean useMultiTenantAuthorizer = + TransportSecurityUtils.isTransportSecurityEnabled() && config.multitenant() && config.hostedVespa() && !authorizerMode.equals("disable"); this.rpcAuthorizer = - TransportSecurityUtils.isTransportSecurityEnabled() && config.multitenant() && config.hostedVespa() && false - ? new MultiTenantRpcAuthorizer(nodeIdentifier, hostRegistries, handlerProvider) + useMultiTenantAuthorizer + ? new MultiTenantRpcAuthorizer(nodeIdentifier, hostRegistries, handlerProvider, toMultiTenantRpcAuthorizerMode(authorizerMode), getThreadPoolSize(config)) : new NoopRpcAuthorizer(); } + private static MultiTenantRpcAuthorizer.Mode toMultiTenantRpcAuthorizerMode(String authorizerMode) { + switch (authorizerMode) { + case "log-only": return MultiTenantRpcAuthorizer.Mode.LOG_ONLY; + case "enforce": return MultiTenantRpcAuthorizer.Mode.ENFORCE; + default: throw new IllegalArgumentException("Invalid authorizer mode: " + authorizerMode); + } + } + + private static int getThreadPoolSize(ConfigserverConfig config) { + return config.numRpcThreads() != 0 ? config.numRpcThreads() : 8; + } + @Override public RpcAuthorizer get() { return rpcAuthorizer; diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java index 2527569bea4..a63f709a99f 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java @@ -1,7 +1,6 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.rpc.security; -import com.google.inject.Inject; import com.yahoo.concurrent.DaemonThreadFactory; import com.yahoo.config.FileReference; import com.yahoo.config.provision.ApplicationId; @@ -49,15 +48,16 @@ public class MultiTenantRpcAuthorizer implements RpcAuthorizer { private final Executor executor; private final Mode mode; - @Inject public MultiTenantRpcAuthorizer(NodeIdentifier nodeIdentifier, HostRegistries hostRegistries, - RequestHandlerProvider handlerProvider) { + RequestHandlerProvider handlerProvider, + Mode mode, + int threadPoolSize) { this(nodeIdentifier, hostRegistries.getTenantHostRegistry(), handlerProvider, - Executors.newFixedThreadPool(4, new DaemonThreadFactory("RPC-Authorizer-")), - Mode.LOG_ONLY); // TODO Change default mode + Executors.newFixedThreadPool(threadPoolSize, new DaemonThreadFactory("multi-tenant-rpc-authorizer-")), + mode); } MultiTenantRpcAuthorizer(NodeIdentifier nodeIdentifier, @@ -112,10 +112,10 @@ public class MultiTenantRpcAuthorizer implements RpcAuthorizer { } else { String hostname = configRequest.getClientHostName(); Optional<RequestHandler> tenantHandler = - Optional.of(hostRegistry.getKeyForHost(hostname)) + Optional.ofNullable(hostRegistry.getKeyForHost(hostname)) .flatMap(this::getTenantHandler); if (tenantHandler.isEmpty()) { - return; // unknown tenant + return; // unknown host } ApplicationId resolvedApplication = tenantHandler.get().resolveApplicationId(hostname); ApplicationId peerOwner = applicationId(peerIdentity); @@ -158,7 +158,8 @@ public class MultiTenantRpcAuthorizer implements RpcAuthorizer { private void handleAuthorizationFailure(Request request, Throwable throwable) { String errorMessage = String.format("For request '%s' from '%s': %s", request.methodName(), request.target().toString(), throwable.getMessage()); - log.log(LogLevel.WARNING, errorMessage, throwable); + log.log(LogLevel.WARNING, errorMessage); + log.log(LogLevel.DEBUG, throwable, throwable::getMessage); if (mode == Mode.ENFORCE) { JrtErrorCode error = throwable instanceof AuthorizationException ? JrtErrorCode.UNAUTHORIZED : JrtErrorCode.AUTHORIZATION_FAILED; request.setError(error.code, errorMessage); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java index 24eb862c528..a31737b6e13 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java @@ -187,7 +187,7 @@ public class RemoteSessionRepo extends SessionRepo<RemoteSession> { if (applicationRepo.requireActiveSessionOf(applicationId) == session.getSessionId()) { log.log(LogLevel.DEBUG, () -> "Found active application for session " + session.getSessionId() + " , loading it"); reloadHandler.reloadConfig(session.ensureApplicationLoaded()); - log.log(LogLevel.INFO, session.logPre() + "Application activated successfully: " + applicationId); + log.log(LogLevel.INFO, session.logPre() + "Application activated successfully: " + applicationId + " (generation " + session.getSessionId() + ")"); return; } } diff --git a/container-search/src/main/java/com/yahoo/fs4/MapEncoder.java b/container-search/src/main/java/com/yahoo/fs4/MapEncoder.java index 3e45d7e5f53..f7e01bc1254 100644 --- a/container-search/src/main/java/com/yahoo/fs4/MapEncoder.java +++ b/container-search/src/main/java/com/yahoo/fs4/MapEncoder.java @@ -21,7 +21,9 @@ public class MapEncoder { // TODO: Time to refactor private static byte [] getUtf8(Object value) { - if (value instanceof Tensor) { + if (value == null) { + return Utf8.toBytes(""); + } else if (value instanceof Tensor) { return TypedBinaryFormat.encode((Tensor)value); } else { return Utf8.toBytes(value.toString()); @@ -70,11 +72,7 @@ public class MapEncoder { buffer.putInt(utf8.length); buffer.put(utf8); Object value = entry.getValue(); - if (value == null) { - utf8 = Utf8.toBytes(""); - } else { - utf8 = getUtf8(value); - } + utf8 = getUtf8(value); buffer.putInt(utf8.length); buffer.put(utf8); } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java index 39a1587afea..13041aa4035 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java @@ -5,6 +5,8 @@ import ai.vespa.searchlib.searchprotocol.protobuf.SearchProtocol.StringProperty; import ai.vespa.searchlib.searchprotocol.protobuf.SearchProtocol.TensorProperty; import com.google.protobuf.ByteString; import com.google.protobuf.InvalidProtocolBufferException; +import com.yahoo.data.access.simple.Value; +import com.yahoo.data.access.slime.SlimeAdapter; import com.yahoo.document.GlobalId; import com.yahoo.fs4.GetDocSumsPacket; import com.yahoo.io.GrowableByteBuffer; @@ -23,6 +25,7 @@ import com.yahoo.search.query.Sorting.Order; import com.yahoo.search.result.Coverage; import com.yahoo.search.result.Relevance; import com.yahoo.searchlib.aggregation.Grouping; +import com.yahoo.slime.BinaryFormat; import com.yahoo.vespa.objects.BufferSerializer; import java.nio.ByteBuffer; @@ -221,7 +224,12 @@ public class ProtobufSerialization { if(sorting != null) { result.hits().setSorted(true); } - + var slimeTrace = protobuf.getSlimeTrace(); + if (slimeTrace != null && !slimeTrace.isEmpty()) { + var traces = new Value.ArrayValue(); + traces.add(new SlimeAdapter(BinaryFormat.decode(slimeTrace.toByteArray()).get())); + query.trace(traces, query.getTraceLevel()); + } return result; } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java index 43ae29ee922..03b3d586b73 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java @@ -1,6 +1,7 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.api.integration.configserver; +import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions; @@ -72,7 +73,12 @@ public interface ConfigServer { NodeRepository nodeRepository(); /** Get service convergence status for given deployment */ - Optional<ServiceConvergence> serviceConvergence(DeploymentId deployment); + default Optional<ServiceConvergence> serviceConvergence(DeploymentId deployment) { + return serviceConvergence(deployment, Optional.empty()); + } + + /** Get service convergence status for given deployment, using the nodes in the model at the given Vespa version. */ + Optional<ServiceConvergence> serviceConvergence(DeploymentId deployment, Optional<Version> version); /** Get all load balancers in given zone */ List<LoadBalancer> getLoadBalancers(ZoneId zone); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java index 84e15deea4c..d989a9dfbe6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java @@ -18,6 +18,7 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationActivity; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; +import com.yahoo.vespa.hosted.controller.application.Endpoint; import com.yahoo.vespa.hosted.controller.application.EndpointList; import com.yahoo.vespa.hosted.controller.application.RotationStatus; import com.yahoo.vespa.hosted.controller.rotation.RotationId; @@ -56,7 +57,7 @@ public class Application { private final OptionalInt majorVersion; private final ApplicationMetrics metrics; private final Optional<String> pemDeployKey; - private final Optional<RotationId> rotation; + private final List<RotationId> rotations; private final Map<HostName, RotationStatus> rotationStatus; /** Creates an empty application */ @@ -65,7 +66,7 @@ public class Application { new DeploymentJobs(OptionalLong.empty(), Collections.emptyList(), Optional.empty(), false), Change.empty(), Change.empty(), Optional.empty(), Optional.empty(), OptionalInt.empty(), new ApplicationMetrics(0, 0), - Optional.empty(), Optional.empty(), Collections.emptyMap()); + Optional.empty(), Collections.emptyList(), Collections.emptyMap()); } /** Used from persistence layer: Do not use */ @@ -73,18 +74,18 @@ public class Application { List<Deployment> deployments, DeploymentJobs deploymentJobs, Change change, Change outstandingChange, Optional<IssueId> ownershipIssueId, Optional<User> owner, OptionalInt majorVersion, ApplicationMetrics metrics, Optional<String> pemDeployKey, - Optional<RotationId> rotation, Map<HostName, RotationStatus> rotationStatus) { + List<RotationId> rotations, Map<HostName, RotationStatus> rotationStatus) { this(id, createdAt, deploymentSpec, validationOverrides, deployments.stream().collect(Collectors.toMap(Deployment::zone, Function.identity())), deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, pemDeployKey, rotation, rotationStatus); + metrics, pemDeployKey, rotations, rotationStatus); } Application(ApplicationId id, Instant createdAt, DeploymentSpec deploymentSpec, ValidationOverrides validationOverrides, Map<ZoneId, Deployment> deployments, DeploymentJobs deploymentJobs, Change change, Change outstandingChange, Optional<IssueId> ownershipIssueId, Optional<User> owner, OptionalInt majorVersion, ApplicationMetrics metrics, Optional<String> pemDeployKey, - Optional<RotationId> rotation, Map<HostName, RotationStatus> rotationStatus) { + List<RotationId> rotations, Map<HostName, RotationStatus> rotationStatus) { this.id = Objects.requireNonNull(id, "id cannot be null"); this.createdAt = Objects.requireNonNull(createdAt, "instant of creation cannot be null"); this.deploymentSpec = Objects.requireNonNull(deploymentSpec, "deploymentSpec cannot be null"); @@ -98,7 +99,7 @@ public class Application { this.majorVersion = Objects.requireNonNull(majorVersion, "majorVersion cannot be null"); this.metrics = Objects.requireNonNull(metrics, "metrics cannot be null"); this.pemDeployKey = pemDeployKey; - this.rotation = Objects.requireNonNull(rotation, "rotation cannot be null"); + this.rotations = List.copyOf(Objects.requireNonNull(rotations, "rotations cannot be null")); this.rotationStatus = ImmutableMap.copyOf(Objects.requireNonNull(rotationStatus, "rotationStatus cannot be null")); } @@ -195,13 +196,15 @@ public class Application { } /** Returns the global rotation id of this, if present */ - public Optional<RotationId> rotation() { - return rotation; + public List<RotationId> rotations() { + return rotations; } /** Returns the default global endpoints for this in given system */ public EndpointList endpointsIn(SystemName system) { - if (rotation.isEmpty()) return EndpointList.EMPTY; + // TODO: Do we need to change something here? .defaultGlobalId seems like it is + // TODO: making some assumptions on naming. + if (rotations.isEmpty()) return EndpointList.EMPTY; return EndpointList.defaultGlobal(id, system); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 8231de59249..de5d8de1f9e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -327,7 +327,7 @@ public class ApplicationController { // Include global DNS names cnames = app.endpointsIn(controller.system()).asList().stream().map(Endpoint::dnsName).collect(Collectors.toSet()); // Include rotation ID to ensure that deployment can respond to health checks with rotation ID as Host header - app.rotation().map(RotationId::asString).ifPresent(cnames::add); + app.rotations().stream().map(RotationId::asString).forEach(cnames::add); // Update application with information from application package if ( ! preferOldestVersion diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java index 943ede5197b..e8d69f8a577 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java @@ -27,6 +27,7 @@ import com.yahoo.vespa.hosted.controller.rotation.RotationId; import java.time.Instant; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -55,7 +56,7 @@ public class LockedApplication { private final OptionalInt majorVersion; private final ApplicationMetrics metrics; private final Optional<String> pemDeployKey; - private final Optional<RotationId> rotation; + private final List<RotationId> rotations; private final Map<HostName, RotationStatus> rotationStatus; /** @@ -70,7 +71,7 @@ public class LockedApplication { application.deployments(), application.deploymentJobs(), application.change(), application.outstandingChange(), application.ownershipIssueId(), application.owner(), application.majorVersion(), application.metrics(), - application.pemDeployKey(), application.rotation(), application.rotationStatus()); + application.pemDeployKey(), application.rotations(), application.rotationStatus()); } private LockedApplication(Lock lock, ApplicationId id, Instant createdAt, @@ -78,7 +79,7 @@ public class LockedApplication { Map<ZoneId, Deployment> deployments, DeploymentJobs deploymentJobs, Change change, Change outstandingChange, Optional<IssueId> ownershipIssueId, Optional<User> owner, OptionalInt majorVersion, ApplicationMetrics metrics, Optional<String> pemDeployKey, - Optional<RotationId> rotation, Map<HostName, RotationStatus> rotationStatus) { + List<RotationId> rotations, Map<HostName, RotationStatus> rotationStatus) { this.lock = lock; this.id = id; this.createdAt = createdAt; @@ -93,7 +94,7 @@ public class LockedApplication { this.majorVersion = majorVersion; this.metrics = metrics; this.pemDeployKey = pemDeployKey; - this.rotation = rotation; + this.rotations = rotations; this.rotationStatus = rotationStatus; } @@ -101,35 +102,35 @@ public class LockedApplication { public Application get() { return new Application(id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, pemDeployKey, - rotation, rotationStatus); + rotations, rotationStatus); } public LockedApplication withBuiltInternally(boolean builtInternally) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs.withBuiltInternally(builtInternally), change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, pemDeployKey, - rotation, rotationStatus); + rotations, rotationStatus); } public LockedApplication withProjectId(OptionalLong projectId) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs.withProjectId(projectId), change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, pemDeployKey, - rotation, rotationStatus); + rotations, rotationStatus); } public LockedApplication withDeploymentIssueId(IssueId issueId) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs.with(issueId), change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, pemDeployKey, - rotation, rotationStatus); + rotations, rotationStatus); } public LockedApplication withJobPause(JobType jobType, OptionalLong pausedUntil) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs.withPause(jobType, pausedUntil), change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, pemDeployKey, - rotation, rotationStatus); + rotations, rotationStatus); } public LockedApplication withJobCompletion(long projectId, JobType jobType, JobStatus.JobRun completion, @@ -137,14 +138,14 @@ public class LockedApplication { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs.withCompletion(projectId, jobType, completion, jobError), change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, - pemDeployKey, rotation, rotationStatus); + pemDeployKey, rotations, rotationStatus); } public LockedApplication withJobTriggering(JobType jobType, JobStatus.JobRun job) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs.withTriggering(jobType, job), change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, pemDeployKey, - rotation, rotationStatus); + rotations, rotationStatus); } public LockedApplication withNewDeployment(ZoneId zone, ApplicationVersion applicationVersion, Version version, @@ -195,45 +196,45 @@ public class LockedApplication { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs.without(jobType), change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, pemDeployKey, - rotation, rotationStatus); + rotations, rotationStatus); } public LockedApplication with(DeploymentSpec deploymentSpec) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, pemDeployKey, - rotation, rotationStatus); + rotations, rotationStatus); } public LockedApplication with(ValidationOverrides validationOverrides) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, pemDeployKey, rotation, rotationStatus); + metrics, pemDeployKey, rotations, rotationStatus); } public LockedApplication withChange(Change change) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, pemDeployKey, rotation, rotationStatus); + metrics, pemDeployKey, rotations, rotationStatus); } public LockedApplication withOutstandingChange(Change outstandingChange) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, pemDeployKey, rotation, rotationStatus); + metrics, pemDeployKey, rotations, rotationStatus); } public LockedApplication withOwnershipIssueId(IssueId issueId) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, Optional.ofNullable(issueId), owner, - majorVersion, metrics, pemDeployKey, rotation, rotationStatus); + majorVersion, metrics, pemDeployKey, rotations, rotationStatus); } public LockedApplication withOwner(User owner) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, Optional.ofNullable(owner), majorVersion, metrics, pemDeployKey, - rotation, rotationStatus); + rotations, rotationStatus); } /** Set a major version for this, or set to null to remove any major version override */ @@ -241,31 +242,31 @@ public class LockedApplication { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion == null ? OptionalInt.empty() : OptionalInt.of(majorVersion), - metrics, pemDeployKey, rotation, rotationStatus); + metrics, pemDeployKey, rotations, rotationStatus); } public LockedApplication with(MetricsService.ApplicationMetrics metrics) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, pemDeployKey, rotation, rotationStatus); + metrics, pemDeployKey, rotations, rotationStatus); } public LockedApplication withPemDeployKey(String pemDeployKey) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, Optional.ofNullable(pemDeployKey), rotation, rotationStatus); + metrics, Optional.ofNullable(pemDeployKey), rotations, rotationStatus); } public LockedApplication with(RotationId rotation) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, pemDeployKey, Optional.of(rotation), rotationStatus); + metrics, pemDeployKey, List.of(rotation), rotationStatus); } public LockedApplication withRotationStatus(Map<HostName, RotationStatus> rotationStatus) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, pemDeployKey, rotation, rotationStatus); + metrics, pemDeployKey, rotations, rotationStatus); } /** Don't expose non-leaf sub-objects. */ @@ -278,7 +279,7 @@ public class LockedApplication { private LockedApplication with(Map<ZoneId, Deployment> deployments) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, pemDeployKey, rotation, rotationStatus); + metrics, pemDeployKey, rotations, rotationStatus); } @Override diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java index 1b90a43421b..6c9b8dd0784 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java @@ -119,7 +119,7 @@ public class Endpoint { private static String separator(SystemName system, boolean directRouting, boolean tls) { if (!tls) return "."; if (directRouting) return "."; - if (isPublic(system)) return "."; + if (system.isPublic()) return "."; return "--"; } @@ -141,7 +141,7 @@ public class Endpoint { } private static String systemPart(SystemName system, String separator) { - if (system == SystemName.main || isPublic(system)) return ""; + if (!system.isCd()) return ""; return system.value() + separator; } @@ -160,10 +160,6 @@ public class Endpoint { } } - private static boolean isPublic(SystemName system) { // TODO: Remove and inline once we're down to one - return system == SystemName.Public || system == SystemName.vaas; - } - /** An endpoint's scope */ public enum Scope { @@ -277,7 +273,7 @@ public class Endpoint { } else { throw new IllegalArgumentException("Must set either cluster or rotation target"); } - if (isPublic(system) && !directRouting) { + if (system.isPublic() && !directRouting) { throw new IllegalArgumentException("Public system only supports direct routing endpoints"); } if (directRouting && !port.isDefault()) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java index 26233c998f8..3eeaf09c10b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java @@ -1,6 +1,7 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.application; +import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.zone.ZoneId; @@ -9,6 +10,7 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ServiceConvergence; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -61,11 +63,11 @@ public enum SystemApplication { } /** Returns whether config for this application has converged in given zone */ - public boolean configConvergedIn(ZoneId zone, Controller controller) { + public boolean configConvergedIn(ZoneId zone, Controller controller, Optional<Version> version) { if (!hasApplicationPackage()) { return true; } - return controller.configServer().serviceConvergence(new DeploymentId(id(), zone)) + return controller.configServer().serviceConvergence(new DeploymentId(id(), zone), version) .map(ServiceConvergence::converged) .orElse(false); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/auditlog/AuditLog.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/auditlog/AuditLog.java index c467a4a0acd..aefe8ae7b48 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/auditlog/AuditLog.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/auditlog/AuditLog.java @@ -109,6 +109,7 @@ public class AuditLog { public enum Method { POST, PATCH, + PUT, DELETE } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index 7a42456f2ac..68e84c7d289 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -268,7 +268,7 @@ public class InternalStepRunner implements StepRunner { logger.log("Checking installation of " + platform + " and " + application.id() + " ..."); if ( nodesConverged(id.application(), id.type(), platform, logger) - && servicesConverged(id.application(), id.type(), logger)) { + && servicesConverged(id.application(), id.type(), platform, logger)) { if (endpointsAvailable(id.application(), id.type().zone(controller.system()), logger)) { logger.log("Installation succeeded!"); return Optional.of(running); @@ -298,7 +298,7 @@ public class InternalStepRunner implements StepRunner { Version platform = controller.jobController().run(id).get().versions().targetPlatform(); logger.log("Checking installation of tester container ..."); if ( nodesConverged(id.tester().id(), id.type(), platform, logger) - && servicesConverged(id.tester().id(), id.type(), logger)) { + && servicesConverged(id.tester().id(), id.type(), platform, logger)) { if (endpointsAvailable(id.tester().id(), id.type().zone(controller.system()), logger)) { logger.log("Tester container successfully installed!"); return Optional.of(running); @@ -354,9 +354,10 @@ public class InternalStepRunner implements StepRunner { && node.rebootGeneration() >= node.wantedRebootGeneration()); } - private boolean servicesConverged(ApplicationId id, JobType type, DualLogger logger) { - Optional<ServiceConvergence> convergence = controller.configServer().serviceConvergence(new DeploymentId(id, type.zone(controller.system()))); - if ( ! convergence.isPresent()) { + private boolean servicesConverged(ApplicationId id, JobType type, Version platform, DualLogger logger) { + var convergence = controller.configServer().serviceConvergence(new DeploymentId(id, type.zone(controller.system())), + Optional.of(platform)); + if (convergence.isEmpty()) { logger.log("Config status not currently available -- will retry."); return false; } @@ -519,7 +520,7 @@ public class InternalStepRunner implements StepRunner { }); } catch (IllegalStateException e) { - logger.log(INFO, "Job '" + id.type() + "'no longer supposed to run?:", e); + logger.log(INFO, "Job '" + id.type() + "' no longer supposed to run?", e); return Optional.of(error); } return Optional.of(running); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecord.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecord.java index b17ca52d835..3cd3d969731 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecord.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecord.java @@ -43,11 +43,6 @@ public class CreateRecord implements NameServiceRequest { } @Override - public List<Record> affectedRecords() { - return List.of(record); - } - - @Override public String toString() { return "create record " + record; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecords.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecords.java index ec943676962..9dd02735638 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecords.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecords.java @@ -52,11 +52,6 @@ public class CreateRecords implements NameServiceRequest { } @Override - public List<Record> affectedRecords() { - return records(); - } - - @Override public String toString() { return "create records " + records(); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceForwarder.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceForwarder.java index f4bea8b1083..299ea168c7a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceForwarder.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceForwarder.java @@ -39,24 +39,24 @@ public class NameServiceForwarder { } /** Create or update a CNAME record with given name and data */ - public Record createCname(RecordName name, RecordData canonicalName, NameServiceQueue.Priority priority) { - return forward(new CreateRecord(new Record(Record.Type.CNAME, name, canonicalName)), priority).affectedRecords().get(0); + public void createCname(RecordName name, RecordData canonicalName, NameServiceQueue.Priority priority) { + forward(new CreateRecord(new Record(Record.Type.CNAME, name, canonicalName)), priority); } /** Create or update an ALIAS record with given name and targets */ - public List<Record> createAlias(RecordName name, Set<AliasTarget> targets, NameServiceQueue.Priority priority) { + public void createAlias(RecordName name, Set<AliasTarget> targets, NameServiceQueue.Priority priority) { var records = targets.stream().map(AliasTarget::asData) .map(data -> new Record(Record.Type.ALIAS, name, data)) .collect(Collectors.toList()); - return forward(new CreateRecords(records), priority).affectedRecords(); + forward(new CreateRecords(records), priority); } /** Create or update a TXT record with given name and data */ - public List<Record> createTxt(RecordName name, List<RecordData> txtData, NameServiceQueue.Priority priority) { + public void createTxt(RecordName name, List<RecordData> txtData, NameServiceQueue.Priority priority) { var records = txtData.stream() .map(data -> new Record(Record.Type.TXT, name, data)) .collect(Collectors.toList()); - return forward(new CreateRecords(records), priority).affectedRecords(); + forward(new CreateRecords(records), priority); } /** Remove all records of given type and name */ @@ -69,7 +69,7 @@ public class NameServiceForwarder { forward(new RemoveRecords(type, data), priority); } - private NameServiceRequest forward(NameServiceRequest request, NameServiceQueue.Priority priority) { + private void forward(NameServiceRequest request, NameServiceQueue.Priority priority) { try (Lock lock = db.lockNameServiceQueue()) { NameServiceQueue queue = db.readNameServiceQueue(); var queued = queue.requests().size(); @@ -80,7 +80,6 @@ public class NameServiceForwarder { } db.writeNameServiceQueue(queue.with(request, priority).last(maxQueuedRequests)); } - return request; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceQueue.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceQueue.java index cc49e589cbb..684fb091d92 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceQueue.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceQueue.java @@ -72,7 +72,7 @@ public class NameServiceQueue { if (requests.isEmpty()) return this; var queue = new NameServiceQueue(requests); - for (var i = 0; i < n && !queue.requests.isEmpty(); i++) { + for (int i = 0; i < n && !queue.requests.isEmpty(); i++) { var request = queue.requests.peek(); try { request.dispatchTo(nameService); @@ -97,11 +97,13 @@ public class NameServiceQueue { /** Priority of a request added to this */ public enum Priority { + /** Default priority. Request will be delivered in FIFO order */ normal, - /**Request is queued before others. Useful for code that needs to act on effects of a request */ + /** Request is queued first. Useful for code that needs to act on effects of a request */ high + } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceRequest.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceRequest.java index a01719ccc88..65076694160 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceRequest.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceRequest.java @@ -2,9 +2,6 @@ package com.yahoo.vespa.hosted.controller.dns; import com.yahoo.vespa.hosted.controller.api.integration.dns.NameService; -import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; - -import java.util.List; /** * Interface for requests to a {@link NameService}. @@ -16,7 +13,4 @@ public interface NameServiceRequest { /** Send this to given name service */ void dispatchTo(NameService nameService); - /** Returns the records affected by executing this */ - List<Record> affectedRecords(); - } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/RemoveRecords.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/RemoveRecords.java index b721f66e452..ddc4d157afd 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/RemoveRecords.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/RemoveRecords.java @@ -61,11 +61,6 @@ public class RemoveRecords implements NameServiceRequest { } @Override - public List<Record> affectedRecords() { - return List.of(); - } - - @Override public String toString() { return "remove records of type " + type + ", by " + name.map(n -> "name " + n).orElse("") + diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java index 0cf89d798a7..4ad5940f8f2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.HostName; +import com.yahoo.config.provision.SystemName; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.ApplicationController; import com.yahoo.vespa.hosted.controller.Controller; @@ -20,6 +21,7 @@ import java.util.concurrent.ForkJoinPool; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Predicate; import java.util.logging.Level; import java.util.logging.Logger; @@ -39,7 +41,8 @@ public class DeploymentMetricsMaintainer extends Maintainer { private final ApplicationController applications; public DeploymentMetricsMaintainer(Controller controller, Duration duration, JobControl jobControl) { - super(controller, duration, jobControl); + super(controller, duration, jobControl, DeploymentMetricsMaintainer.class.getSimpleName(), + SystemName.allOf(Predicate.not(SystemName::isPublic))); this.applications = controller.applications(); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/NameServiceDispatcher.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/NameServiceDispatcher.java index 8878ac9bd5b..d4dc068c71f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/NameServiceDispatcher.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/NameServiceDispatcher.java @@ -41,6 +41,7 @@ public class NameServiceDispatcher extends Maintainer { try (Lock lock = db.lockNameServiceQueue()) { NameServiceQueue queue = db.readNameServiceQueue(); NameServiceQueue remaining = queue.dispatchTo(nameService, requestCount); + if (queue == remaining) return; // Queue unchanged db.writeNameServiceQueue(remaining); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java index 62d401cf478..3c2455b314a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java @@ -42,7 +42,7 @@ public class SystemUpgrader extends InfrastructureUpgrader { protected boolean convergedOn(Version target, SystemApplication application, ZoneId zone) { return minVersion(zone, application, Node::currentVersion).map(target::equals) .orElse(true) - && application.configConvergedIn(zone, controller()); + && application.configConvergedIn(zone, controller(), Optional.of(target)); } @Override diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java index 3c2cbade606..3dd091919aa 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java @@ -70,7 +70,8 @@ public class ApplicationSerializer { private final String writeQualityField = "writeQuality"; private final String queryQualityField = "queryQuality"; private final String pemDeployKeyField = "pemDeployKey"; - private final String rotationField = "rotation"; + private final String rotationsField = "endpoints"; + private final String deprecatedRotationField = "rotation"; private final String rotationStatusField = "rotationStatus"; // Deployment fields @@ -162,7 +163,8 @@ public class ApplicationSerializer { root.setDouble(queryQualityField, application.metrics().queryServiceQuality()); root.setDouble(writeQualityField, application.metrics().writeServiceQuality()); application.pemDeployKey().ifPresent(pemDeployKey -> root.setString(pemDeployKeyField, pemDeployKey)); - application.rotation().ifPresent(rotation -> root.setString(rotationField, rotation.asString())); + Cursor rotations = root.setArray(rotationsField); + application.rotations().forEach(rotation -> rotations.addString(rotation.asString())); toSlime(application.rotationStatus(), root.setArray(rotationStatusField)); return slime; } @@ -329,12 +331,12 @@ public class ApplicationSerializer { ApplicationMetrics metrics = new ApplicationMetrics(root.field(queryQualityField).asDouble(), root.field(writeQualityField).asDouble()); Optional<String> pemDeployKey = optionalString(root.field(pemDeployKeyField)); - Optional<RotationId> rotation = rotationFromSlime(root.field(rotationField)); + List<RotationId> rotations = rotationsFromSlime(root); Map<HostName, RotationStatus> rotationStatus = rotationStatusFromSlime(root.field(rotationStatusField)); return new Application(id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, deploying, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, - pemDeployKey, rotation, rotationStatus); + pemDeployKey, rotations, rotationStatus); } private List<Deployment> deploymentsFromSlime(Inspector array) { @@ -514,7 +516,26 @@ public class ApplicationSerializer { Instant.ofEpochMilli(object.field(atField).asLong()))); } - private Optional<RotationId> rotationFromSlime(Inspector field) { + private List<RotationId> rotationsFromSlime(Inspector root) { + final var rotations = rotationListFromSlime(root.field(rotationsField)); + final var legacyRotation = legacyRotationFromSlime(root.field(deprecatedRotationField)); + legacyRotation.ifPresent(rotations::add); + return rotations; + } + + private List<RotationId> rotationListFromSlime(Inspector field) { + final var rotations = new ArrayList<RotationId>(); + + field.traverse((ArrayTraverser) (idx, inspector) -> { + final var rotation = new RotationId(inspector.asString()); + rotations.add(rotation); + }); + + return rotations; + } + + // TODO: Remove after June 2019 once the 'rotation' field is gone from storage + private Optional<RotationId> legacyRotationFromSlime(Inspector field) { return field.valid() ? optionalString(field).map(RotationId::new) : Optional.empty(); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/AuditLogSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/AuditLogSerializer.java index 5bcb155efcb..7fee9a7f9b4 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/AuditLogSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/AuditLogSerializer.java @@ -60,6 +60,7 @@ public class AuditLogSerializer { switch (method) { case POST: return "POST"; case PATCH: return "PATCH"; + case PUT: return "PUT"; case DELETE: return "DELETE"; default: throw new IllegalArgumentException("No serialization defined for method " + method); } @@ -69,6 +70,7 @@ public class AuditLogSerializer { switch (field.asString()) { case "POST": return AuditLog.Entry.Method.POST; case "PATCH": return AuditLog.Entry.Method.PATCH; + case "PUT": return AuditLog.Entry.Method.PUT; case "DELETE": return AuditLog.Entry.Method.DELETE; default: throw new IllegalArgumentException("Unknown serialized value '" + field.asString() + "'"); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index d231b034e8a..367c9528e8c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -496,7 +496,8 @@ public class ApplicationApiHandler extends LoggingRequestHandler { .map(URI::toString) .forEach(globalRotationsArray::addString); - application.rotation().ifPresent(rotation -> object.setString("rotationId", rotation.asString())); + + application.rotations().stream().findFirst().ifPresent(rotation -> object.setString("rotationId", rotation.asString())); // Per-cluster rotations Set<RoutingPolicy> routingPolicies = controller.applications().routingPolicies().get(application.id()); @@ -515,7 +516,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { for (Deployment deployment : deployments) { Cursor deploymentObject = instancesArray.addObject(); - if (application.rotation().isPresent() && deployment.zone().environment() == Environment.prod) { + if ((! application.rotations().isEmpty()) && deployment.zone().environment() == Environment.prod) { toSlime(application.rotationStatus(deployment), deploymentObject); } @@ -707,7 +708,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { ApplicationId applicationId = ApplicationId.from(tenantName, applicationName, instanceName); Application application = controller.applications().require(applicationId); ZoneId zone = ZoneId.from(environment, region); - if (!application.rotation().isPresent()) { + if (application.rotations().isEmpty()) { throw new NotExistsException("global rotation does not exist for " + application); } Deployment deployment = application.deployments().get(zone); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java index b3953c47c01..d2b16721503 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java @@ -47,7 +47,7 @@ public class RotationRepository { /** Get rotation for given application */ public Optional<Rotation> getRotation(Application application) { - return application.rotation().map(allRotations::get); + return application.rotations().stream().map(allRotations::get).findFirst(); } /** @@ -60,8 +60,8 @@ public class RotationRepository { * @param lock Lock which must be acquired by the caller */ public Rotation getOrAssignRotation(Application application, RotationLock lock) { - if (application.rotation().isPresent()) { - return allRotations.get(application.rotation().get()); + if (! application.rotations().isEmpty()) { + return allRotations.get(application.rotations().get(0)); } if (application.deploymentSpec().globalServiceId().isEmpty()) { throw new IllegalArgumentException("global-service-id is not set in deployment spec"); @@ -81,8 +81,8 @@ public class RotationRepository { */ public Map<RotationId, Rotation> availableRotations(@SuppressWarnings("unused") RotationLock lock) { List<RotationId> assignedRotations = applications.asList().stream() - .filter(application -> application.rotation().isPresent()) - .map(application -> application.rotation().get()) + .filter(application -> ! application.rotations().isEmpty()) + .flatMap(application -> application.rotations().stream()) .collect(Collectors.toList()); Map<RotationId, Rotation> unassignedRotations = new LinkedHashMap<>(this.allRotations); assignedRotations.forEach(unassignedRotations::remove); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java index e2d4c90f443..879575669cd 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java @@ -157,7 +157,7 @@ public class VersionStatus { ListMap<Version, HostName> versions = new ListMap<>(); for (ZoneId zone : zones) { for (SystemApplication application : SystemApplication.all()) { - boolean configConverged = application.configConvergedIn(zone, controller); + boolean configConverged = application.configConvergedIn(zone, controller, Optional.empty()); if (!configConverged) { log.log(LogLevel.WARNING, "Config for " + application.id() + " in " + zone + " has not converged"); } 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 7dcb4aba138..b89941b5d2d 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 @@ -436,7 +436,7 @@ public class ControllerTest { .build(); tester.deployCompletely(app1, applicationPackage); app1 = tester.applications().require(app1.id()); - assertEquals("rotation-id-02", app1.rotation().get().asString()); + assertEquals("rotation-id-02", app1.rotations().get(0).asString()); // Existing DNS records are updated to point to the newly assigned rotation assertEquals(6, tester.controllerTester().nameService().records().size()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/auditlog/AuditLoggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/auditlog/AuditLoggerTest.java index 6470ce3663f..67979571f73 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/auditlog/AuditLoggerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/auditlog/AuditLoggerTest.java @@ -24,11 +24,10 @@ import static org.junit.Assert.assertTrue; public class AuditLoggerTest { private final ControllerTester tester = new ControllerTester(); + private final Supplier<AuditLog> log = () -> tester.controller().auditLogger().readLog(); @Test public void test_logging() { - Supplier<AuditLog> log = () -> tester.controller().auditLogger().readLog(); - { // GET request is ignored HttpRequest request = testRequest(Method.GET, URI.create("http://localhost:8080/os/v1/"), ""); tester.controller().auditLogger().log(request); @@ -40,11 +39,8 @@ public class AuditLoggerTest { String data = "{\"cloud\":\"cloud9\",\"version\":\"42.0\"}"; HttpRequest request = testRequest(Method.PATCH, url, data); tester.controller().auditLogger().log(request); - - assertEquals(instant(), log.get().entries().get(0).at()); + assertEntry(Entry.Method.PATCH, 1, "/os/v1/?foo=bar"); assertEquals("user", log.get().entries().get(0).principal()); - assertEquals(Entry.Method.PATCH, log.get().entries().get(0).method()); - assertEquals("/os/v1/?foo=bar", log.get().entries().get(0).resource()); assertEquals(data, log.get().entries().get(0).data().get()); } @@ -53,9 +49,31 @@ public class AuditLoggerTest { HttpRequest request = testRequest(Method.PATCH, URI.create("http://localhost:8080/os/v1/"), "{\"cloud\":\"cloud9\",\"version\":\"43.0\"}"); tester.controller().auditLogger().log(request); - assertEquals(2, log.get().entries().size()); - assertEquals(instant(), log.get().entries().get(0).at()); - assertEquals("/os/v1/", log.get().entries().get(0).resource()); + assertEntry(Entry.Method.PATCH, 2, "/os/v1/"); + } + + { // PUT is logged + tester.clock().advance(Duration.ofDays(1)); + HttpRequest request = testRequest(Method.PUT, URI.create("http://localhost:8080/zone/v2/prod/us-north-1/nodes/v2/state/dirty/node1/"), + ""); + tester.controller().auditLogger().log(request); + assertEntry(Entry.Method.PUT, 3, "/zone/v2/prod/us-north-1/nodes/v2/state/dirty/node1/"); + } + + { // DELETE is logged + tester.clock().advance(Duration.ofDays(1)); + HttpRequest request = testRequest(Method.DELETE, URI.create("http://localhost:8080/zone/v2/prod/us-north-1/nodes/v2/node/node1"), + ""); + tester.controller().auditLogger().log(request); + assertEntry(Entry.Method.DELETE, 4, "/zone/v2/prod/us-north-1/nodes/v2/node/node1"); + } + + { // POST is logged + tester.clock().advance(Duration.ofDays(1)); + HttpRequest request = testRequest(Method.POST, URI.create("http://localhost:8080/controller/v1/jobs/upgrader/confidence/6.42"), + "6.42"); + tester.controller().auditLogger().log(request); + assertEntry(Entry.Method.POST, 5, "/controller/v1/jobs/upgrader/confidence/6.42"); } { // 14 days pass and another PATCH request is logged. Older entries are removed due to expiry @@ -63,8 +81,7 @@ public class AuditLoggerTest { HttpRequest request = testRequest(Method.PATCH, URI.create("http://localhost:8080/os/v1/"), "{\"cloud\":\"cloud9\",\"version\":\"44.0\"}"); tester.controller().auditLogger().log(request); - assertEquals(1, log.get().entries().size()); - assertEquals(instant(), log.get().entries().get(0).at()); + assertEntry(Entry.Method.PATCH, 1, "/os/v1/"); } } @@ -72,6 +89,13 @@ public class AuditLoggerTest { return tester.clock().instant().truncatedTo(MILLIS); } + private void assertEntry(Entry.Method method, int logSize, String resource) { + assertEquals(logSize, log.get().entries().size()); + assertEquals(instant(), log.get().entries().get(0).at()); + assertEquals(method, log.get().entries().get(0).method()); + assertEquals(resource, log.get().entries().get(0).resource()); + } + private static HttpRequest testRequest(Method method, URI url, String data) { HttpRequest request = HttpRequest.createTestRequest( url.toString(), diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index 913c9a800c1..31b48bea883 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -195,7 +195,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer } @Override - public Optional<ServiceConvergence> serviceConvergence(DeploymentId deployment) { + public Optional<ServiceConvergence> serviceConvergence(DeploymentId deployment, Optional<Version> version) { return Optional.ofNullable(serviceStatus.get(deployment)); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java index d7c0ac9fe9e..084ac593f4c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java @@ -29,6 +29,7 @@ import com.yahoo.vespa.hosted.controller.application.RotationStatus; import com.yahoo.vespa.hosted.controller.rotation.RotationId; import org.junit.Test; +import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -115,7 +116,7 @@ public class ApplicationSerializerTest { OptionalInt.of(7), new MetricsService.ApplicationMetrics(0.5, 0.9), Optional.of("-----BEGIN PUBLIC KEY-----\n∠( ᐛ 」∠)＿\n-----END PUBLIC KEY-----"), - Optional.of(new RotationId("my-rotation")), + List.of(new RotationId("my-rotation")), rotationStatus); Application serialized = applicationSerializer.fromSlime(applicationSerializer.toSlime(original)); @@ -151,7 +152,7 @@ public class ApplicationSerializerTest { assertEquals(original.change(), serialized.change()); assertEquals(original.pemDeployKey(), serialized.pemDeployKey()); - assertEquals(original.rotation().get(), serialized.rotation().get()); + assertEquals(original.rotations(), serialized.rotations()); assertEquals(original.rotationStatus(), serialized.rotationStatus()); // Test cluster utilization @@ -244,4 +245,32 @@ public class ApplicationSerializerTest { // ok if no error } + /** TODO: Test can be removed after June 2019 - once legacy field for single rotation is retired */ + @Test + public void testParsingLegacyRotationElement() throws IOException { + // Use the 'complete-application.json' as a baseline + final var applicationJson = Files.readAllBytes(testData.resolve("complete-application.json")); + final var slime = SlimeUtils.jsonToSlime(applicationJson); + + // Add the necessary fields to the Slime representation of the application + final var cursor = slime.get(); + + cursor.setString("rotation", "single-rotation"); + + final var rotations = cursor.setArray("endpoints"); + rotations.addString("multiple-rotation-1"); + rotations.addString("multiple-rotation-2"); + + // Parse and test the output from parsing contains both legacy rotation and multiple rotations + final var application = applicationSerializer.fromSlime(slime); + + assertEquals( + List.of( + new RotationId("multiple-rotation-1"), + new RotationId("multiple-rotation-2"), + new RotationId("single-rotation") + ), + application.rotations() + ); + } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepositoryTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepositoryTest.java index 8d1f40260e3..02a82e35f10 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepositoryTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepositoryTest.java @@ -14,10 +14,11 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import java.net.URI; +import java.util.List; import java.util.Optional; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; /** * @author Oyvind Gronnesby @@ -64,7 +65,7 @@ public class RotationRepositoryTest { Rotation expected = new Rotation(new RotationId("foo-1"), "foo-1.com"); application = tester.applications().require(application.id()); - assertEquals(expected.id(), application.rotation().get()); + assertEquals(List.of(expected.id()), application.rotations()); assertEquals(URI.create("https://app1--tenant1.global.vespa.oath.cloud:4443/"), application.endpointsIn(SystemName.main).main().get().url()); try (RotationLock lock = repository.lock()) { @@ -80,7 +81,7 @@ public class RotationRepositoryTest { .searchDefinition("search foo { }") // Update application package so there is something to deploy .build(); tester.deployCompletely(application, applicationPackage, 43); - assertEquals(expected.id(), tester.applications().require(application.id()).rotation().get()); + assertEquals(List.of(expected.id()), tester.applications().require(application.id()).rotations()); } @Test @@ -139,8 +140,7 @@ public class RotationRepositoryTest { .build(); tester.deployCompletely(application, applicationPackage); Application app = tester.applications().require(application.id()); - Optional<RotationId> rotation = app.rotation(); - assertFalse(rotation.isPresent()); + assertTrue(app.rotations().isEmpty()); } @Test @@ -153,7 +153,7 @@ public class RotationRepositoryTest { Application application = tester.createApplication("app2", "tenant2", 22L, 2L); tester.deployCompletely(application, applicationPackage); - assertEquals(new RotationId("foo-1"), tester.applications().require(application.id()).rotation().get()); + assertEquals(List.of(new RotationId("foo-1")), tester.applications().require(application.id()).rotations()); assertEquals("https://cd--app2--tenant2.global.vespa.oath.cloud:4443/", tester.applications().require(application.id()) .endpointsIn(SystemName.cd).main().get().url().toString()); } diff --git a/document/src/tests/serialization/vespadocumentserializer_test.cpp b/document/src/tests/serialization/vespadocumentserializer_test.cpp index c573eef6147..4b5c3e6f195 100644 --- a/document/src/tests/serialization/vespadocumentserializer_test.cpp +++ b/document/src/tests/serialization/vespadocumentserializer_test.cpp @@ -40,8 +40,7 @@ #include <vespa/eval/tensor/types.h> #include <vespa/eval/tensor/tensor_builder.h> #include <vespa/eval/tensor/tensor.h> -#include <vespa/eval/tensor/default_tensor.h> -#include <vespa/eval/tensor/tensor_factory.h> +#include <vespa/eval/tensor/default_tensor_engine.h> #include <vespa/vespalib/io/fileutil.h> #include <vespa/vespalib/objects/nbostream.h> #include <vespa/vespalib/testkit/testapp.h> @@ -53,10 +52,9 @@ using vespalib::Slime; using vespalib::nbostream; using vespalib::nbostream_longlivedbuf; using vespalib::slime::Cursor; +using vespalib::eval::TensorSpec; using vespalib::tensor::Tensor; -using vespalib::tensor::TensorBuilder; -using vespalib::tensor::TensorCells; -using vespalib::tensor::TensorDimensions; +using vespalib::tensor::DefaultTensorEngine; using vespalib::compression::CompressionConfig; using namespace document; using std::string; @@ -828,13 +826,14 @@ TEST("Require that predicate deserialization matches Java") { namespace { -Tensor::UP -createTensor(const TensorCells &cells, const TensorDimensions &dimensions) { - vespalib::tensor::DefaultTensor::builder builder; - return vespalib::tensor::TensorFactory::create(cells, dimensions, builder); +Tensor::UP createTensor(const TensorSpec &spec) { + auto value = DefaultTensorEngine::ref().from_spec(spec); + Tensor *tensor = dynamic_cast<Tensor*>(value.get()); + ASSERT_TRUE(tensor != nullptr); + value.release(); + return Tensor::UP(tensor); } - } TEST("Require that tensors can be serialized") @@ -846,12 +845,12 @@ TEST("Require that tensors can be serialized") nbostream stream; serializeAndDeserialize(noTensorValue, stream); stream.clear(); - emptyTensorValue = createTensor({}, {"x", "y"}); + emptyTensorValue = createTensor(TensorSpec("tensor(x{},y{})")); serializeAndDeserialize(emptyTensorValue, stream); stream.clear(); - twoCellsTwoDimsValue = createTensor({ {{{"y", "3"}}, 3}, - {{{"x", "4"}, {"y", "5"}}, 7} }, - {"x", "y"}); + twoCellsTwoDimsValue = createTensor(TensorSpec("tensor(x{},y{})") + .add({{"x", ""}, {"y", "3"}}, 3) + .add({{"x", "4"}, {"y", "5"}}, 7)); serializeAndDeserialize(twoCellsTwoDimsValue, stream); EXPECT_NOT_EQUAL(noTensorValue, emptyTensorValue); EXPECT_NOT_EQUAL(noTensorValue, twoCellsTwoDimsValue); @@ -859,6 +858,7 @@ TEST("Require that tensors can be serialized") } + const int tensor_doc_type_id = 321; const string tensor_field_name = "my_tensor"; @@ -904,15 +904,15 @@ void checkDeserialization(const string &name, std::unique_ptr<Tensor> tensor) { deserializeAndCheck(data_dir + name + "__java", value); } + TEST("Require that tensor deserialization matches Java") { checkDeserialization("non_existing_tensor", std::unique_ptr<Tensor>()); - checkDeserialization("empty_tensor", createTensor({}, {"dimX", "dimY"})); + checkDeserialization("empty_tensor", createTensor(TensorSpec("tensor(dimX{},dimY{})"))); checkDeserialization("multi_cell_tensor", - createTensor({ {{{"dimX", "a"}, {"dimY", "bb"}}, 2.0 }, - {{{"dimX", "ccc"}, - {"dimY", "dddd"}}, 3.0}, - {{{"dimX", "e"},{"dimY","ff"}}, 5.0} }, - { "dimX", "dimY" })); + createTensor(TensorSpec("tensor(dimX{},dimY{})") + .add({{"dimX", "a"}, {"dimY", "bb"}}, 2.0) + .add({{"dimX", "ccc"}, {"dimY", "dddd"}}, 3.0) + .add({{"dimX", "e"}, {"dimY", "ff"}}, 5.0))); } struct TensorDocFixture { @@ -980,14 +980,14 @@ DeserializedTensorDoc::getTensor() const TEST("Require that wrong tensor type hides tensor") { TensorDocFixture f(tensor_doc_repo, - createTensor({ {{{"dimX", "a"},{"dimY", "bb"}}, 2.0 }, - {{{"dimX", "ccc"},{"dimY", "dddd"}}, 3.0}, - {{{"dimX", "e"},{"dimY","ff"}}, 5.0} }, - { "dimX", "dimY" })); + createTensor(TensorSpec("tensor(dimX{},dimY{})") + .add({{"dimX", "a"}, {"dimY", "bb"}}, 2.0) + .add({{"dimX", "ccc"}, {"dimY", "dddd"}}, 3.0) + .add({{"dimX", "e"}, {"dimY", "ff"}}, 5.0))); TensorDocFixture f1(tensor_doc_repo1, - createTensor({ {{{"dimX", "a"}}, 20.0 }, - {{{"dimX", "ccc"}}, 30.0} }, - { "dimX" })); + createTensor(TensorSpec("tensor(dimX{})") + .add({{"dimX", "a"}}, 20.0) + .add({{"dimX", "ccc"}}, 30.0))); DeserializedTensorDoc doc; doc.setup(tensor_doc_repo, f._blob); EXPECT_TRUE(doc.getTensor() != nullptr); diff --git a/document/src/tests/tensor_fieldvalue/tensor_fieldvalue_test.cpp b/document/src/tests/tensor_fieldvalue/tensor_fieldvalue_test.cpp index dc91df0b5a0..9d2da9c983a 100644 --- a/document/src/tests/tensor_fieldvalue/tensor_fieldvalue_test.cpp +++ b/document/src/tests/tensor_fieldvalue/tensor_fieldvalue_test.cpp @@ -9,8 +9,7 @@ LOG_SETUP("fieldvalue_test"); #include <vespa/document/fieldvalue/tensorfieldvalue.h> #include <vespa/eval/tensor/tensor.h> #include <vespa/eval/tensor/types.h> -#include <vespa/eval/tensor/default_tensor.h> -#include <vespa/eval/tensor/tensor_factory.h> +#include <vespa/eval/tensor/default_tensor_engine.h> #include <vespa/eval/tensor/test/test_utils.h> #include <vespa/vespalib/testkit/testapp.h> @@ -19,6 +18,7 @@ using namespace document; using namespace vespalib::tensor; using vespalib::eval::TensorSpec; using vespalib::eval::ValueType; +using vespalib::tensor::DefaultTensorEngine; using vespalib::tensor::test::makeTensor; namespace @@ -27,10 +27,12 @@ namespace TensorDataType xSparseTensorDataType(ValueType::from_spec("tensor(x{})")); TensorDataType xySparseTensorDataType(ValueType::from_spec("tensor(x{},y{})")); -Tensor::UP -createTensor(const TensorCells &cells, const TensorDimensions &dimensions) { - vespalib::tensor::DefaultTensor::builder builder; - return vespalib::tensor::TensorFactory::create(cells, dimensions, builder); +Tensor::UP createTensor(const TensorSpec &spec) { + auto value = DefaultTensorEngine::ref().from_spec(spec); + Tensor *tensor = dynamic_cast<Tensor*>(value.get()); + ASSERT_TRUE(tensor != nullptr); + value.release(); + return Tensor::UP(tensor); } std::unique_ptr<Tensor> @@ -54,10 +56,10 @@ TEST("require that TensorFieldValue can be assigned tensors and cloned") { TensorFieldValue noTensorValue(xySparseTensorDataType); TensorFieldValue emptyTensorValue(xySparseTensorDataType); TensorFieldValue twoCellsTwoDimsValue(xySparseTensorDataType); - emptyTensorValue = createTensor({}, {"x", "y"}); - twoCellsTwoDimsValue = createTensor({ {{{"y", "3"}}, 3}, - {{{"x", "4"}, {"y", "5"}}, 7} }, - {"x", "y"}); + emptyTensorValue = createTensor(TensorSpec("tensor(x{},y{})")); + twoCellsTwoDimsValue = createTensor(TensorSpec("tensor(x{},y{})") + .add({{"x", ""}, {"y", "3"}}, 3) + .add({{"x", "4"}, {"y", "5"}}, 7)); EXPECT_NOT_EQUAL(noTensorValue, emptyTensorValue); EXPECT_NOT_EQUAL(noTensorValue, twoCellsTwoDimsValue); EXPECT_NOT_EQUAL(emptyTensorValue, noTensorValue); @@ -74,10 +76,9 @@ TEST("require that TensorFieldValue can be assigned tensors and cloned") { EXPECT_NOT_EQUAL(*twoClone, *noneClone); EXPECT_NOT_EQUAL(*twoClone, *emptyClone); TensorFieldValue twoCellsTwoDimsValue2(xySparseTensorDataType); - twoCellsTwoDimsValue2 = - createTensor({ {{{"y", "3"}}, 3}, - {{{"x", "4"}, {"y", "5"}}, 7} }, - {"x", "y"}); + twoCellsTwoDimsValue2 = createTensor(TensorSpec("tensor(x{},y{})") + .add({{"x", ""}, {"y", "3"}}, 3) + .add({{"x", "4"}, {"y", "5"}}, 7)); EXPECT_NOT_EQUAL(*noneClone, twoCellsTwoDimsValue2); EXPECT_NOT_EQUAL(*emptyClone, twoCellsTwoDimsValue2); EXPECT_EQUAL(*twoClone, twoCellsTwoDimsValue2); @@ -87,8 +88,8 @@ TEST("require that TensorFieldValue::toString works") { TensorFieldValue tensorFieldValue(xSparseTensorDataType); EXPECT_EQUAL("{TensorFieldValue: null}", tensorFieldValue.toString()); - tensorFieldValue = createTensor({{{{"x","a"}}, 3}}, {"x"}); - EXPECT_EQUAL("{TensorFieldValue: {\"dimensions\":[\"x\"],\"cells\":[{\"address\":{\"x\":\"a\"},\"value\":3}]}}", tensorFieldValue.toString()); + tensorFieldValue = createTensor(TensorSpec("tensor(x{})").add({{"x", "a"}}, 3)); + EXPECT_EQUAL("{TensorFieldValue: spec(tensor(x{})) {\n [a]: 3\n}}", tensorFieldValue.toString()); } TEST("require that wrong tensor type for special case assign throws exception") diff --git a/document/src/vespa/document/fieldvalue/tensorfieldvalue.cpp b/document/src/vespa/document/fieldvalue/tensorfieldvalue.cpp index f209358c901..9b318a39f0a 100644 --- a/document/src/vespa/document/fieldvalue/tensorfieldvalue.cpp +++ b/document/src/vespa/document/fieldvalue/tensorfieldvalue.cpp @@ -4,16 +4,16 @@ #include <vespa/document/base/exceptions.h> #include <vespa/document/datatype/tensor_data_type.h> #include <vespa/vespalib/util/xmlstream.h> +#include <vespa/eval/eval/tensor_spec.h> #include <vespa/eval/tensor/tensor.h> -#include <vespa/eval/tensor/serialization/slime_binary_format.h> -#include <vespa/vespalib/data/slime/slime.h> +#include <vespa/eval/tensor/default_tensor_engine.h> #include <ostream> #include <cassert> -using vespalib::slime::JsonFormat; using vespalib::tensor::Tensor; -using vespalib::tensor::SlimeBinaryFormat; +using vespalib::eval::TensorSpec; using vespalib::eval::ValueType; +using Engine = vespalib::tensor::DefaultTensorEngine; using namespace vespalib::xml; namespace document { @@ -148,11 +148,7 @@ TensorFieldValue::print(std::ostream& out, bool verbose, (void) indent; out << "{TensorFieldValue: "; if (_tensor) { - auto slime = SlimeBinaryFormat::serialize(*_tensor); - vespalib::SimpleBuffer buf; - JsonFormat::encode(*slime, buf, true); - auto json = buf.get().make_string(); - out << json; + out << Engine::ref().to_spec(*_tensor).to_string(); } else { out << "null"; } diff --git a/eval/CMakeLists.txt b/eval/CMakeLists.txt index d15f1c37d2d..4468ac46f61 100644 --- a/eval/CMakeLists.txt +++ b/eval/CMakeLists.txt @@ -44,7 +44,6 @@ vespa_define_module( src/tests/tensor/tensor_performance src/tests/tensor/tensor_remove_operation src/tests/tensor/tensor_serialization - src/tests/tensor/tensor_slime_serialization src/tests/tensor/typed_cells src/tests/tensor/vector_from_doubles_function diff --git a/eval/src/tests/tensor/tensor_serialization/tensor_serialization_test.cpp b/eval/src/tests/tensor/tensor_serialization/tensor_serialization_test.cpp index b7aa988775d..2c113e814e9 100644 --- a/eval/src/tests/tensor/tensor_serialization/tensor_serialization_test.cpp +++ b/eval/src/tests/tensor/tensor_serialization/tensor_serialization_test.cpp @@ -4,8 +4,7 @@ #include <vespa/eval/tensor/sparse/sparse_tensor.h> #include <vespa/eval/tensor/sparse/sparse_tensor_builder.h> #include <vespa/eval/tensor/types.h> -#include <vespa/eval/tensor/default_tensor.h> -#include <vespa/eval/tensor/tensor_factory.h> +#include <vespa/eval/tensor/default_tensor_engine.h> #include <vespa/eval/tensor/serialization/typed_binary_format.h> #include <vespa/eval/tensor/serialization/sparse_binary_format.h> #include <vespa/vespalib/objects/nbostream.h> @@ -14,6 +13,7 @@ #include <vespa/eval/tensor/dense/dense_tensor_view.h> using namespace vespalib::tensor; +using vespalib::eval::TensorSpec; using vespalib::nbostream; using ExpBuffer = std::vector<uint8_t>; @@ -33,257 +33,181 @@ std::ostream &operator<<(std::ostream &out, const std::vector<uint8_t> &rhs) } -namespace vespalib::tensor { - -static bool operator==(const Tensor &lhs, const Tensor &rhs) -{ - return lhs.equals(rhs); -} - -} - -template <class BuilderType> -void -checkDeserialize(vespalib::nbostream &stream, const Tensor &rhs) -{ - (void) stream; - (void) rhs; -} - -template <> -void -checkDeserialize<DefaultTensor::builder>(nbostream &stream, const Tensor &rhs) -{ - nbostream wrapStream(stream.peek(), stream.size()); - auto chk = TypedBinaryFormat::deserialize(wrapStream); - EXPECT_EQUAL(0u, wrapStream.size()); - EXPECT_EQUAL(*chk, rhs); -} - -template <typename BuilderType> -struct Fixture -{ - BuilderType _builder; - Fixture() : _builder() {} - - Tensor::UP createTensor(const TensorCells &cells) { - return TensorFactory::create(cells, _builder); - } - Tensor::UP createTensor(const TensorCells &cells, const TensorDimensions &dimensions) { - return TensorFactory::create(cells, dimensions, _builder); +//----------------------------------------------------------------------------- + +template <typename T> +void verify_cells_only(const ExpBuffer &exp, const TensorSpec &spec) { + nbostream input(&exp[0], exp.size()); + std::vector<T> cells; + TypedBinaryFormat::deserializeCellsOnlyFromDenseTensors(input, cells); + ASSERT_EQUAL(cells.size(), spec.cells().size()); + size_t i = 0; + for (const auto &cell: spec.cells()) { + EXPECT_EQUAL(cells[i++], cell.second.value); } + ASSERT_EQUAL(i, cells.size()); +} - void serialize(nbostream &stream, const Tensor &tensor) { - TypedBinaryFormat::serialize(stream, tensor); - } - Tensor::UP deserialize(nbostream &stream) { - BuilderType builder; - nbostream wrapStream(stream.peek(), stream.size()); - auto formatId = wrapStream.getInt1_4Bytes(); - ASSERT_EQUAL(formatId, 1u); // sparse format - SparseBinaryFormat::deserialize(wrapStream, builder); - EXPECT_TRUE(wrapStream.empty()); - auto ret = builder.build(); - checkDeserialize<BuilderType>(stream, *ret); - stream.adjustReadPos(stream.size()); - return ret; - } - void assertSerialized(const ExpBuffer &exp, const TensorCells &rhs, - const TensorDimensions &rhsDimensions) { - Tensor::UP rhsTensor(createTensor(rhs, rhsDimensions)); - nbostream rhsStream; - serialize(rhsStream, *rhsTensor); - EXPECT_EQUAL(exp, rhsStream); - auto rhs2 = deserialize(rhsStream); - EXPECT_EQUAL(*rhs2, *rhsTensor); +void verify_serialized(const ExpBuffer &exp, const TensorSpec &spec) { + auto &engine = DefaultTensorEngine::ref(); + auto value = engine.from_spec(spec); + auto value_spec = engine.to_spec(*value); + nbostream actual; + engine.encode(*value, actual); + EXPECT_EQUAL(exp, actual); + auto decoded = engine.decode(actual); + auto decoded_spec = engine.to_spec(*decoded); + EXPECT_EQUAL(0u, actual.size()); + EXPECT_EQUAL(value_spec, decoded_spec); + if (value->type().is_dense()) { + TEST_DO(verify_cells_only<float>(exp, value_spec)); + TEST_DO(verify_cells_only<double>(exp, value_spec)); } -}; - -using SparseFixture = Fixture<SparseTensorBuilder>; - +} -template <typename FixtureType> -void -testTensorSerialization(FixtureType &f) -{ - TEST_DO(f.assertSerialized({ 0x01, 0x00, 0x00 }, {}, {})); - TEST_DO(f.assertSerialized({ 0x01, 0x01, 0x01, 0x78, 0x00 }, - {}, { "x" })); - TEST_DO(f.assertSerialized({ 0x01, 0x02, 0x01, 0x78, 0x01, 0x79, 0x00 }, - {}, { "x", "y" })); - TEST_DO(f.assertSerialized({ 0x01, 0x01, 0x01, 0x78, 0x01, 0x01, 0x31, 0x40, - 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }, - { {{{"x","1"}}, 3} }, { "x" })); - TEST_DO(f.assertSerialized({ 0x01, 0x02, 0x01, 0x78, 0x01, 0x79, 0x01, 0x00, +//----------------------------------------------------------------------------- + +TEST("test tensor serialization for SparseTensor") { + TEST_DO(verify_serialized({ 0x01, 0x01, 0x01, 0x78, 0x00 }, + TensorSpec("tensor(x{})"))); + TEST_DO(verify_serialized({ 0x01, 0x02, 0x01, 0x78, 0x01, 0x79, 0x00 }, + TensorSpec("tensor(x{},y{})"))); + TEST_DO(verify_serialized({ 0x01, 0x01, 0x01, 0x78, 0x01, 0x01, 0x31, 0x40, + 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }, + TensorSpec("tensor(x{})") + .add({{"x", "1"}}, 3))); + TEST_DO(verify_serialized({ 0x01, 0x02, 0x01, 0x78, 0x01, 0x79, 0x01, 0x00, 0x00, 0x40, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }, - { {{}, 3} }, { "x", "y"})); - TEST_DO(f.assertSerialized({ 0x01, 0x02, 0x01, 0x78, 0x01, 0x79, 0x01, 0x01, - 0x31, 0x00, 0x40, 0x08, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00 }, - { {{{"x","1"}}, 3} }, { "x", "y" })); - TEST_DO(f.assertSerialized({ 0x01, 0x02, 0x01, 0x78, 0x01, 0x79, 0x01, 0x00, - 0x01, 0x33, 0x40, 0x08, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00 }, - { {{{"y","3"}}, 3} }, { "x", "y" })); - TEST_DO(f.assertSerialized({ 0x01, 0x02, 0x01, 0x78, 0x01, 0x79, 0x01, 0x01, + TensorSpec("tensor(x{},y{})") + .add({{"x", ""}, {"y", ""}}, 3))); + TEST_DO(verify_serialized({ 0x01, 0x02, 0x01, 0x78, 0x01, 0x79, 0x01, 0x01, + 0x31, 0x00, 0x40, 0x08, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00 }, + TensorSpec("tensor(x{},y{})") + .add({{"x", "1"}, {"y", ""}}, 3))); + TEST_DO(verify_serialized({ 0x01, 0x02, 0x01, 0x78, 0x01, 0x79, 0x01, 0x00, + 0x01, 0x33, 0x40, 0x08, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00 }, + TensorSpec("tensor(x{},y{})") + .add({{"x", ""}, {"y", "3"}}, 3))); + TEST_DO(verify_serialized({ 0x01, 0x02, 0x01, 0x78, 0x01, 0x79, 0x01, 0x01, 0x32, 0x01, 0x34, 0x40, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }, - { {{{"x","2"}, {"y", "4"}}, 3} }, { "x", "y" })); - TEST_DO(f.assertSerialized({ 0x01, 0x02, 0x01, 0x78, 0x01, 0x79, + TensorSpec("tensor(x{},y{})") + .add({{"x", "2"}, {"y", "4"}}, 3))); + TEST_DO(verify_serialized({ 0x01, 0x02, 0x01, 0x78, 0x01, 0x79, 0x01, 0x01, 0x31, 0x00, 0x40, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }, - { {{{"x","1"}}, 3} }, {"x", "y"})); + TensorSpec("tensor(x{},y{})") + .add({{"x", "1"}, {"y", ""}}, 3))); } -TEST_F("test tensor serialization for SparseTensor", SparseFixture) -{ - testTensorSerialization(f); +TEST("test tensor serialization for DenseTensor") { + TEST_DO(verify_serialized({0x02, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00}, + TensorSpec("double"))); + TEST_DO(verify_serialized({0x02, 0x01, 0x01, 0x78, 0x01, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00}, + TensorSpec("tensor(x[1])") + .add({{"x", 0}}, 0))); + TEST_DO(verify_serialized({0x02, 0x02, 0x01, 0x78, 0x01, + 0x01, 0x79, 0x01, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00}, + TensorSpec("tensor(x[1],y[1])") + .add({{"x", 0}, {"y", 0}}, 0))); + TEST_DO(verify_serialized({0x02, 0x01, 0x01, 0x78, 0x02, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x40, 0x08, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00}, + TensorSpec("tensor(x[2])") + .add({{"x", 1}}, 3))); + TEST_DO(verify_serialized({0x02, 0x02, 0x01, 0x78, 0x01, + 0x01, 0x79, 0x01, + 0x40, 0x08, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00}, + TensorSpec("tensor(x[1],y[1])") + .add({{"x", 0}, {"y", 0}}, 3))); + TEST_DO(verify_serialized({0x02, 0x02, 0x01, 0x78, 0x02, + 0x01, 0x79, 0x01, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x40, 0x08, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00}, + TensorSpec("tensor(x[2],y[1])") + .add({{"x", 1}, {"y", 0}}, 3))); + TEST_DO(verify_serialized({0x02, 0x02, 0x01, 0x78, 0x01, + 0x01, 0x79, 0x04, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x40, 0x08, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00}, + TensorSpec("tensor(x[1],y[4])") + .add({{"x", 0}, {"y", 3}}, 3))); + TEST_DO(verify_serialized({0x02, 0x02, 0x01, 0x78, 0x03, + 0x01, 0x79, 0x05, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x40, 0x08, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00}, + TensorSpec("tensor(x[3],y[5])") + .add({{"x", 2}, {"y", 4}}, 3))); } - -struct DenseFixture -{ - Tensor::UP createTensor(const DenseTensorCells &cells) { - return TensorFactory::createDense(cells); - } - - void serialize(nbostream &stream, const Tensor &tensor) { - TypedBinaryFormat::serialize(stream, tensor); - } - - Tensor::UP deserialize(nbostream &stream) { - nbostream wrapStream(stream.peek(), stream.size()); - auto ret = TypedBinaryFormat::deserialize(wrapStream); - EXPECT_TRUE(wrapStream.size() == 0); - stream.adjustReadPos(stream.size()); - return ret; - } - void assertSerialized(const ExpBuffer &exp, const DenseTensorCells &rhs) { - assertSerialized(exp, SerializeFormat::DOUBLE, rhs); - } - template <typename T> - void assertCellsOnly(const ExpBuffer &exp, const DenseTensorView & rhs) { - nbostream a(&exp[0], exp.size()); - std::vector<T> v; - TypedBinaryFormat::deserializeCellsOnlyFromDenseTensors(a, v); - EXPECT_EQUAL(v.size(), rhs.cellsRef().size()); - for (size_t i(0); i < v.size(); i++) { - EXPECT_EQUAL(v[i], rhs.cellsRef()[i]); - } - } - void assertSerialized(const ExpBuffer &exp, SerializeFormat cellType, const DenseTensorCells &rhs) { - Tensor::UP rhsTensor(createTensor(rhs)); - nbostream rhsStream; - TypedBinaryFormat::serialize(rhsStream, *rhsTensor, cellType); - EXPECT_EQUAL(exp, rhsStream); - auto rhs2 = deserialize(rhsStream); - EXPECT_EQUAL(*rhs2, *rhsTensor); - - assertCellsOnly<float>(exp, dynamic_cast<const DenseTensorView &>(*rhs2)); - assertCellsOnly<double>(exp, dynamic_cast<const DenseTensorView &>(*rhs2)); - } -}; - - -TEST_F("test tensor serialization for DenseTensor", DenseFixture) { - TEST_DO(f.assertSerialized({0x02, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00}, - {})); - TEST_DO(f.assertSerialized({0x02, 0x01, 0x01, 0x78, 0x01, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00}, - {{{{"x", 0}}, 0}})); - TEST_DO(f.assertSerialized({0x02, 0x02, 0x01, 0x78, 0x01, - 0x01, 0x79, 0x01, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00}, - {{{{"x", 0}, {"y", 0}}, 0}})); - TEST_DO(f.assertSerialized({0x02, 0x01, 0x01, 0x78, 0x02, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x40, 0x08, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00}, - {{{{"x", 1}}, 3}})); - TEST_DO(f.assertSerialized({0x02, 0x02, 0x01, 0x78, 0x01, - 0x01, 0x79, 0x01, - 0x40, 0x08, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00}, - {{{{"x", 0}, {"y", 0}}, 3}})); - TEST_DO(f.assertSerialized({0x02, 0x02, 0x01, 0x78, 0x02, - 0x01, 0x79, 0x01, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x40, 0x08, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00}, - {{{{"x", 1}, {"y", 0}}, 3}})); - TEST_DO(f.assertSerialized({0x02, 0x02, 0x01, 0x78, 0x01, - 0x01, 0x79, 0x04, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x40, 0x08, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00}, - {{{{"x", 0}, {"y", 3}}, 3}})); - TEST_DO(f.assertSerialized({0x02, 0x02, 0x01, 0x78, 0x03, - 0x01, 0x79, 0x05, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x40, 0x08, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00}, - {{{{"x", 2}, {"y", 4}}, 3}})); -} - -TEST_F("test 'float' cells", DenseFixture) { - TEST_DO(f.assertSerialized({0x06, 0x01, 0x02, 0x01, 0x78, 0x03, - 0x01, 0x79, 0x05, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - 0x40, 0x40, 0x00, 0x00 }, - SerializeFormat::FLOAT, { {{{"x",2}, {"y",4}}, 3} })); +TEST("test 'float' cells") { + TEST_DO(verify_serialized({0x06, 0x01, 0x02, 0x01, 0x78, 0x03, + 0x01, 0x79, 0x05, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x40, 0x40, 0x00, 0x00 }, + TensorSpec("tensor<float>(x[3],y[5])") + .add({{"x", 2}, {"y", 4}}, 3))); } - TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/eval/src/tests/tensor/tensor_slime_serialization/.gitignore b/eval/src/tests/tensor/tensor_slime_serialization/.gitignore deleted file mode 100644 index 9cb3b664d58..00000000000 --- a/eval/src/tests/tensor/tensor_slime_serialization/.gitignore +++ /dev/null @@ -1 +0,0 @@ -vespalib_tensor_slime_serialization_test_app diff --git a/eval/src/tests/tensor/tensor_slime_serialization/CMakeLists.txt b/eval/src/tests/tensor/tensor_slime_serialization/CMakeLists.txt deleted file mode 100644 index 9a9b8b7a436..00000000000 --- a/eval/src/tests/tensor/tensor_slime_serialization/CMakeLists.txt +++ /dev/null @@ -1,8 +0,0 @@ -# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -vespa_add_executable(eval_tensor_slime_serialization_test_app TEST - SOURCES - tensor_slime_serialization_test.cpp - DEPENDS - vespaeval -) -vespa_add_test(NAME eval_tensor_slime_serialization_test_app COMMAND eval_tensor_slime_serialization_test_app) diff --git a/eval/src/tests/tensor/tensor_slime_serialization/tensor_slime_serialization_test.cpp b/eval/src/tests/tensor/tensor_slime_serialization/tensor_slime_serialization_test.cpp deleted file mode 100644 index e6e6c7de686..00000000000 --- a/eval/src/tests/tensor/tensor_slime_serialization/tensor_slime_serialization_test.cpp +++ /dev/null @@ -1,185 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include <vespa/vespalib/testkit/test_kit.h> -#include <vespa/eval/tensor/sparse/sparse_tensor.h> -#include <vespa/eval/tensor/sparse/sparse_tensor_builder.h> -#include <vespa/eval/tensor/types.h> -#include <vespa/eval/tensor/default_tensor.h> -#include <vespa/eval/tensor/tensor_factory.h> -#include <vespa/eval/tensor/serialization/typed_binary_format.h> -#include <vespa/eval/tensor/serialization/slime_binary_format.h> -#include <vespa/vespalib/data/slime/slime.h> -#include <iostream> - -using namespace vespalib::tensor; - -template <typename BuilderType> -struct Fixture -{ - BuilderType _builder; - Fixture() : _builder() {} - - Tensor::UP createTensor(const TensorCells &cells) { - return vespalib::tensor::TensorFactory::create(cells, _builder); - } - Tensor::UP createTensor(const TensorCells &cells, const TensorDimensions &dimensions) { - return TensorFactory::create(cells, dimensions, _builder); - } - - static inline uint32_t getTensorTypeId(); - - void assertSerialized(const vespalib::string &exp, const TensorCells &rhs, - const TensorDimensions &rhsDimensions) { - Tensor::UP rhsTensor(createTensor(rhs, rhsDimensions)); - auto slime = SlimeBinaryFormat::serialize(*rhsTensor); - vespalib::Memory memory_exp(exp); - vespalib::Slime expSlime; - size_t used = vespalib::slime::JsonFormat::decode(memory_exp, expSlime); - EXPECT_TRUE(used > 0); - EXPECT_EQUAL(expSlime, *slime); - } -}; - -template <> -uint32_t -Fixture<SparseTensorBuilder>::getTensorTypeId() { return 2u; } - - -using SparseFixture = Fixture<SparseTensorBuilder>; - - -namespace { -vespalib::string twoCellsJson[3] = -{ - "{ dimensions: [ 'x', 'y' ]," - " cells: [" - "{ address: { y:'3'}, value: 4.0 }," - "{ address: { x:'1'}, value: 3.0 }" - "] }", - "{ dimensions: [ 'x', 'y' ]," - " cells: [" - "{ address: { x:'1'}, value: 3.0 }," - "{ address: { y:'3'}, value: 4.0 }" - "] }", - "{ dimensions: [ 'x', 'y' ]," - " cells: [" - "{ address: { x:'1'}, value: 3.0 }," - "{ address: { y:'3'}, value: 4.0 }" - "] }", -}; -} - - -template <typename FixtureType> -void -testTensorSlimeSerialization(FixtureType &f) -{ - TEST_DO(f.assertSerialized("{ dimensions: [], cells: [] }", {}, {})); - TEST_DO(f.assertSerialized("{ dimensions: [ 'x' ], cells: [] }", - {}, { "x" })); - TEST_DO(f.assertSerialized("{ dimensions: [ 'x', 'y' ], cells: [] }", - {}, { "x", "y" })); - TEST_DO(f.assertSerialized("{ dimensions: [ 'x' ]," - "cells: [" - "{ address: { x: '1' }, value: 3.0 }" - "] }", - { {{{"x","1"}}, 3} }, { "x" })); - TEST_DO(f.assertSerialized("{ dimensions: [ 'x', 'y' ]," - " cells: [" - "{ address: { }, value: 3.0 }" - "] }", - { {{}, 3} }, { "x", "y"})); - TEST_DO(f.assertSerialized("{ dimensions: [ 'x', 'y' ]," - " cells: [" - "{ address: { x: '1' }, value: 3.0 }" - "] }", - { {{{"x","1"}}, 3} }, { "x", "y" })); - TEST_DO(f.assertSerialized("{ dimensions: [ 'x', 'y' ]," - " cells: [" - "{ address: { y: '3' }, value: 3.0 }" - "] }", - { {{{"y","3"}}, 3} }, { "x", "y" })); - TEST_DO(f.assertSerialized("{ dimensions: [ 'x', 'y' ]," - " cells: [" - "{ address: { x:'2', y:'4'}, value: 3.0 }" - "] }", - { {{{"x","2"}, {"y", "4"}}, 3} }, { "x", "y" })); - TEST_DO(f.assertSerialized("{ dimensions: [ 'x', 'y' ]," - " cells: [" - "{ address: { x:'1'}, value: 3.0 }" - "] }", - { {{{"x","1"}}, 3} }, {"x", "y"})); - TEST_DO(f.assertSerialized(twoCellsJson[FixtureType::getTensorTypeId()], - { {{{"x","1"}}, 3}, {{{"y","3"}}, 4} }, - {"x", "y"})); -} - -TEST_F("test tensor slime serialization for SparseTensor", SparseFixture) -{ - testTensorSlimeSerialization(f); -} - - -struct DenseFixture -{ - DenseFixture() {} - - Tensor::UP createTensor(const DenseTensorCells &cells) { - return vespalib::tensor::TensorFactory::createDense(cells); - } - - void assertSerialized(const vespalib::string &exp, - const DenseTensorCells &rhs) { - Tensor::UP rhsTensor(createTensor(rhs)); - auto slime = SlimeBinaryFormat::serialize(*rhsTensor); - vespalib::Memory memory_exp(exp); - vespalib::Slime expSlime; - size_t used = vespalib::slime::JsonFormat::decode(memory_exp, expSlime); - EXPECT_TRUE(used > 0); - EXPECT_EQUAL(expSlime, *slime); - } -}; - - -TEST_F("test tensor slime serialization for DenseTensor", DenseFixture) -{ - TEST_DO(f.assertSerialized("{ dimensions: [], cells: [" - "{ address: { }, value: 0.0 }" - "] }", {})); - TEST_DO(f.assertSerialized("{ dimensions: [ 'x' ], cells: [" - "{ address: { x: '0' }, value: 0.0 }" - "] }", - { {{{"x",0}}, 0} })); - TEST_DO(f.assertSerialized("{ dimensions: [ 'x', 'y' ], cells: [" - "{ address: { x: '0', y: '0' }, value: 0.0 }" - "] }", - { {{{"x",0},{"y",0}}, 0} })); - TEST_DO(f.assertSerialized("{ dimensions: [ 'x' ]," - "cells: [" - "{ address: { x: '0' }, value: 0.0 }," - "{ address: { x: '1' }, value: 3.0 }" - "] }", - { {{{"x",1}}, 3} })); - TEST_DO(f.assertSerialized("{ dimensions: [ 'x', 'y' ]," - " cells: [" - "{ address: { x: '0', y: '0' }, value: 3.0 }" - "] }", - { {{{"x",0},{"y",0}}, 3} })); - TEST_DO(f.assertSerialized("{ dimensions: [ 'x', 'y' ]," - " cells: [" - "{ address: { x: '0', y: '0' }, value: 0.0 }," - "{ address: { x: '1', y: '0' }, value: 3.0 }" - "] }", - { {{{"x",1},{"y", 0}}, 3} })); - TEST_DO(f.assertSerialized("{ dimensions: [ 'x', 'y' ]," - " cells: [" - "{ address: { x: '0', y: '0' }, value: 0.0 }," - "{ address: { x: '0', y: '1' }, value: 0.0 }," - "{ address: { x: '0', y: '2' }, value: 0.0 }," - "{ address: { x: '0', y: '3' }, value: 3.0 }" - "] }", - { {{{"x",0},{"y",3}}, 3} })); -} - - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/eval/src/vespa/eval/tensor/CMakeLists.txt b/eval/src/vespa/eval/tensor/CMakeLists.txt index ca4d275ea44..4e9940d3c0a 100644 --- a/eval/src/vespa/eval/tensor/CMakeLists.txt +++ b/eval/src/vespa/eval/tensor/CMakeLists.txt @@ -5,7 +5,6 @@ vespa_add_library(eval_tensor OBJECT tensor.cpp tensor_address.cpp tensor_apply.cpp - tensor_factory.cpp tensor_mapper.cpp wrapped_simple_tensor.cpp ) diff --git a/eval/src/vespa/eval/tensor/serialization/CMakeLists.txt b/eval/src/vespa/eval/tensor/serialization/CMakeLists.txt index cd3cd455e55..fc9ac64ea68 100644 --- a/eval/src/vespa/eval/tensor/serialization/CMakeLists.txt +++ b/eval/src/vespa/eval/tensor/serialization/CMakeLists.txt @@ -3,6 +3,5 @@ vespa_add_library(eval_tensor_serialization OBJECT SOURCES sparse_binary_format.cpp dense_binary_format.cpp - slime_binary_format.cpp typed_binary_format.cpp ) diff --git a/eval/src/vespa/eval/tensor/serialization/slime_binary_format.cpp b/eval/src/vespa/eval/tensor/serialization/slime_binary_format.cpp deleted file mode 100644 index ece3c2e4a07..00000000000 --- a/eval/src/vespa/eval/tensor/serialization/slime_binary_format.cpp +++ /dev/null @@ -1,100 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "slime_binary_format.h" -#include <vespa/eval/tensor/types.h> -#include <vespa/eval/tensor/tensor.h> -#include <vespa/eval/tensor/tensor_builder.h> -#include <vespa/eval/tensor/tensor_visitor.h> -#include <vespa/vespalib/data/slime/inserter.h> -#include <vespa/vespalib/data/slime/cursor.h> -#include <vespa/vespalib/data/slime/slime.h> -#include <vespa/vespalib/data/memory.h> - -namespace vespalib::tensor { - - -using slime::Inserter; -using slime::SlimeInserter; -using slime::Cursor; -using slime::ObjectInserter; - -namespace { - -Memory memory_address("address"); -Memory memory_cells("cells"); -Memory memory_dimensions("dimensions"); -Memory memory_value("value"); - -void writeTensorAddress(Cursor &cursor, const TensorAddress &value) { - ObjectInserter addressInserter(cursor, memory_address); - Cursor &addressCursor = addressInserter.insertObject(); - for (const auto &elem : value.elements()) { - Memory dimension(elem.dimension()); - Memory label(elem.label()); - addressCursor.setString(dimension, label); - } -} - -} - -class SlimeBinaryFormatSerializer : public TensorVisitor -{ - Cursor &_tensor; // cursor for whole tensor - Cursor &_dimensions; // cursor for dimensions array - Cursor &_cells; // cursor for cells array -public: - SlimeBinaryFormatSerializer(Inserter &inserter); - virtual ~SlimeBinaryFormatSerializer() override; - virtual void visit(const TensorAddress &address, double value) override; - void serialize(const Tensor &tensor); -}; - -SlimeBinaryFormatSerializer::SlimeBinaryFormatSerializer(Inserter &inserter) - : _tensor(inserter.insertObject()), - _dimensions(_tensor.setArray(memory_dimensions)), - _cells(_tensor.setArray(memory_cells)) -{ -} - - -SlimeBinaryFormatSerializer::~SlimeBinaryFormatSerializer() = default; - -void -SlimeBinaryFormatSerializer::visit(const TensorAddress &address, double value) -{ - Cursor &cellCursor = _cells.addObject(); - writeTensorAddress(cellCursor, address); - cellCursor.setDouble(memory_value, value); -} - - -void -SlimeBinaryFormatSerializer::serialize(const Tensor &tensor) -{ - eval::ValueType type(tensor.type()); - for (const auto & dimension : type.dimensions()) { - _dimensions.addString(Memory(dimension.name)); - } - tensor.accept(*this); -} - - -void -SlimeBinaryFormat::serialize(Inserter &inserter, const Tensor &tensor) -{ - SlimeBinaryFormatSerializer serializer(inserter); - serializer.serialize(tensor); -} - - -std::unique_ptr<Slime> -SlimeBinaryFormat::serialize(const Tensor &tensor) -{ - auto slime = std::make_unique<Slime>(); - SlimeInserter inserter(*slime); - serialize(inserter, tensor); - return slime; -} - - -} diff --git a/eval/src/vespa/eval/tensor/serialization/slime_binary_format.h b/eval/src/vespa/eval/tensor/serialization/slime_binary_format.h deleted file mode 100644 index c9e9ff2c3e9..00000000000 --- a/eval/src/vespa/eval/tensor/serialization/slime_binary_format.h +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -#include <memory> - -namespace vespalib { class Slime; } - -namespace vespalib::slime { struct Inserter; } - -namespace vespalib::tensor { - -class Tensor; -class TensorBuilder; - -/** - * Class for serializing a tensor into a slime object. - */ -class SlimeBinaryFormat -{ -public: - static void serialize(slime::Inserter &inserter, const Tensor &tensor); - static std::unique_ptr<Slime> serialize(const Tensor &tensor); -}; - -} diff --git a/eval/src/vespa/eval/tensor/serialization/typed_binary_format.cpp b/eval/src/vespa/eval/tensor/serialization/typed_binary_format.cpp index 4ca037e82a4..7be4db46372 100644 --- a/eval/src/vespa/eval/tensor/serialization/typed_binary_format.cpp +++ b/eval/src/vespa/eval/tensor/serialization/typed_binary_format.cpp @@ -25,9 +25,9 @@ namespace { constexpr uint32_t SPARSE_BINARY_FORMAT_TYPE = 1u; constexpr uint32_t DENSE_BINARY_FORMAT_TYPE = 2u; constexpr uint32_t MIXED_BINARY_FORMAT_TYPE = 3u; -constexpr uint32_t SPARSE_BINARY_FORMAT_WITH_CELLTYPE = 5u; //Future +constexpr uint32_t SPARSE_BINARY_FORMAT_WITH_CELLTYPE = 5u; constexpr uint32_t DENSE_BINARY_FORMAT_WITH_CELLTYPE = 6u; -constexpr uint32_t MIXED_BINARY_FORMAT_WITH_CELLTYPE = 7u; //Future +constexpr uint32_t MIXED_BINARY_FORMAT_WITH_CELLTYPE = 7u; constexpr uint32_t DOUBLE_VALUE_TYPE = 0; constexpr uint32_t FLOAT_VALUE_TYPE = 1; @@ -91,10 +91,11 @@ TypedBinaryFormat::deserialize(nbostream &stream) if (formatId == DENSE_BINARY_FORMAT_TYPE) { return DenseBinaryFormat(SerializeFormat::DOUBLE).deserialize(stream); } - if (formatId == DENSE_BINARY_FORMAT_WITH_CELLTYPE) { - return DenseBinaryFormat(encoding2Format(stream.getInt1_4Bytes())).deserialize(stream); - } - if (formatId == MIXED_BINARY_FORMAT_TYPE) { + if ((formatId == SPARSE_BINARY_FORMAT_WITH_CELLTYPE) || + (formatId == DENSE_BINARY_FORMAT_WITH_CELLTYPE) || + (formatId == MIXED_BINARY_FORMAT_TYPE) || + (formatId == MIXED_BINARY_FORMAT_WITH_CELLTYPE)) + { stream.adjustReadPos(read_pos - stream.rp()); return std::make_unique<WrappedSimpleTensor>(eval::SimpleTensor::decode(stream)); } diff --git a/eval/src/vespa/eval/tensor/tensor_factory.cpp b/eval/src/vespa/eval/tensor/tensor_factory.cpp deleted file mode 100644 index 0b7fa3b9c2e..00000000000 --- a/eval/src/vespa/eval/tensor/tensor_factory.cpp +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "tensor.h" -#include "tensor_factory.h" -#include "tensor_builder.h" -#include <vespa/eval/tensor/dense/dense_tensor_builder.h> - -namespace vespalib::tensor { - -std::unique_ptr<Tensor> -TensorFactory::create(const TensorCells &cells, TensorBuilder &builder) { - for (const auto &cell : cells) { - for (const auto &addressElem : cell.first) { - const auto &dimension = addressElem.first; - builder.define_dimension(dimension); - } - } - for (const auto &cell : cells) { - for (const auto &addressElem : cell.first) { - const auto &dimension = addressElem.first; - const auto &label = addressElem.second; - builder.add_label(builder.define_dimension(dimension), label); - } - builder.add_cell(cell.second); - } - return builder.build(); -} - - -std::unique_ptr<Tensor> -TensorFactory::create(const TensorCells &cells, const TensorDimensions &dimensions, TensorBuilder &builder) { - for (const auto &dimension : dimensions) { - builder.define_dimension(dimension); - } - return create(cells, builder); -} - - -std::unique_ptr<Tensor> -TensorFactory::createDense(const DenseTensorCells &cells) -{ - std::map<std::string, size_t> dimensionSizes; - DenseTensorBuilder builder; - for (const auto &cell : cells) { - for (const auto &addressElem : cell.first) { - dimensionSizes[addressElem.first] = std::max(dimensionSizes[addressElem.first], (addressElem.second + 1)); - } - } - std::map<std::string, typename DenseTensorBuilder::Dimension> dimensionEnums; - for (const auto &dimensionElem : dimensionSizes) { - dimensionEnums[dimensionElem.first] = builder.defineDimension(dimensionElem.first, dimensionElem.second); - } - for (const auto &cell : cells) { - for (const auto &addressElem : cell.first) { - const auto &dimension = addressElem.first; - size_t label = addressElem.second; - builder.addLabel(dimensionEnums[dimension], label); - } - builder.addCell(cell.second); - } - return builder.build(); -} - - -} diff --git a/eval/src/vespa/eval/tensor/tensor_factory.h b/eval/src/vespa/eval/tensor/tensor_factory.h deleted file mode 100644 index 5364c28c8ff..00000000000 --- a/eval/src/vespa/eval/tensor/tensor_factory.h +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -#include "types.h" - -namespace vespalib::tensor { - - -class Tensor; -class TensorBuilder; - -/** - * A factory for creating tensors based on stl structures (TensorCells and - * TensorDimensions) in unit tests. - */ -class TensorFactory { -public: - static std::unique_ptr<Tensor> - create(const TensorCells &cells, TensorBuilder &builder); - static std::unique_ptr<Tensor> - create(const TensorCells &cells, const TensorDimensions &dimensions, TensorBuilder &builder); - static std::unique_ptr<Tensor> - createDense(const DenseTensorCells &cells); -}; - -} 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 c790fc5659e..9cf508444f5 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -158,6 +158,11 @@ public class Flags { "Takes effect only at bootstrap of config server/controller", HOSTNAME); + public static final UnboundStringFlag CONFIGSERVER_RPC_AUTHORIZER = defineStringFlag( + "configserver-rpc-authorizer", "log-only", + "Configserver RPC authorizer. Allowed values: ['disable', 'log-only', 'enforce']", + "Takes effect on restart of configserver"); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, String description, diff --git a/metrics-proxy/pom.xml b/metrics-proxy/pom.xml index 66ca25dadf0..3cfe1d2f568 100644 --- a/metrics-proxy/pom.xml +++ b/metrics-proxy/pom.xml @@ -60,6 +60,12 @@ </dependency> <dependency> <groupId>com.yahoo.vespa</groupId> + <artifactId>container-core</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> <artifactId>container-di</artifactId> <version>${project.version}</version> <scope>provided</scope> @@ -104,6 +110,12 @@ <version>${project.version}</version> </dependency> <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>jdisc_http_service</artifactId> + <version>${project.version}</version> + <scope>test</scope> + </dependency> + <dependency> <groupId>org.apache.httpcomponents</groupId> <artifactId>httpclient</artifactId> </dependency> @@ -120,6 +132,11 @@ <scope>test</scope> </dependency> <dependency> + <groupId>org.eclipse.jetty</groupId> + <artifactId>jetty-http</artifactId> + <scope>test</scope> + </dependency> + <dependency> <groupId>org.hamcrest</groupId> <artifactId>hamcrest-core</artifactId> <scope>test</scope> @@ -158,6 +175,16 @@ <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-compiler-plugin</artifactId> </plugin> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-surefire-plugin</artifactId> + <configuration> + <!-- Illegal reflective access by guice --> + <argLine> + --add-opens=java.base/java.lang=ALL-UNNAMED + </argLine> + </configuration> + </plugin> </plugins> </build> </project> diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/GenericMetricsHandler.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/GenericMetricsHandler.java new file mode 100644 index 00000000000..488f26b7af7 --- /dev/null +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/GenericMetricsHandler.java @@ -0,0 +1,77 @@ +/* + * Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + */ + +package ai.vespa.metricsproxy.http; + +import ai.vespa.metricsproxy.core.MetricsManager; +import ai.vespa.metricsproxy.metric.model.MetricsPacket; +import ai.vespa.metricsproxy.metric.model.json.GenericJsonModel; +import ai.vespa.metricsproxy.metric.model.json.GenericJsonUtil; +import ai.vespa.metricsproxy.service.VespaServices; +import com.google.inject.Inject; +import com.yahoo.container.jdisc.HttpRequest; +import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.container.jdisc.ThreadedHttpRequestHandler; +import com.yahoo.yolean.Exceptions; + +import java.io.IOException; +import java.io.OutputStream; +import java.nio.charset.Charset; +import java.time.Instant; +import java.util.List; +import java.util.concurrent.Executor; + +/** + * Handler exposing the generic metrics format via http. + * + * @author gjoranv + */ +public class GenericMetricsHandler extends ThreadedHttpRequestHandler { + + private final MetricsManager metricsManager; + private final VespaServices vespaServices; + + @Inject + public GenericMetricsHandler(Executor executor, MetricsManager metricsManager, VespaServices vespaServices) { + super(executor); + this.metricsManager = metricsManager; + this.vespaServices = vespaServices; + } + + @Override + public HttpResponse handle(HttpRequest request) { + try { + List<MetricsPacket> metrics = metricsManager.getMetrics(vespaServices.getVespaServices(), Instant.now()); + return new Response(200, GenericJsonUtil.toGenericJsonModel(metrics).serialize()); + } catch (GenericJsonModel.JsonMetricsRenderingException e) { + return new ErrorResponse(500, Exceptions.toMessageString(e)); + } + } + + private static class Response extends HttpResponse { + private final byte[] data; + + Response(int code, String data) { + super(code); + this.data = data.getBytes(Charset.forName(DEFAULT_CHARACTER_ENCODING)); + } + + @Override + public String getContentType() { + return "application/json"; + } + + @Override + public void render(OutputStream outputStream) throws IOException { + outputStream.write(data); + } + } + + private static class ErrorResponse extends Response { + ErrorResponse(int code, String data) { + super(code, "{\"error\":\"" + data + "\"}"); + } + } + +} diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java index dd8858ff99e..14f085ff388 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java @@ -38,8 +38,13 @@ public class GenericJsonModel { return mapper.writeValueAsString(this); } catch (IOException e) { log.log(Level.WARNING, "Got exception when rendering metrics:", e); - throw new RuntimeException("Could not render metrics. Check the log for details."); + throw new JsonMetricsRenderingException("Could not render metrics. Check the log for details."); } } + public static class JsonMetricsRenderingException extends RuntimeException { + JsonMetricsRenderingException(String message) { + super(message); + } + } } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/JacksonUtil.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/JacksonUtil.java index 5c04f933c4a..d9df3e0c0e9 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/JacksonUtil.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/JacksonUtil.java @@ -20,12 +20,12 @@ import java.util.Locale; * @author smorgrav * @author gjoranv */ -class JacksonUtil { +public class JacksonUtil { /** * Returns an object mapper with a custom floating point serializer to avoid scientific notation */ - static ObjectMapper createObjectMapper() { + public static ObjectMapper createObjectMapper() { ObjectMapper mapper = new ObjectMapper(); SimpleModule module = new SimpleModule("DoubleSerializer", new Version(1, 0, 0, "", null, null)); diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/VespaService.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/VespaService.java index d3f674176b3..c59daf65cc2 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/VespaService.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/VespaService.java @@ -33,6 +33,7 @@ public class VespaService implements Comparable<VespaService> { private volatile int pid = -1; private volatile String state = "UNKNOWN"; + private volatile boolean isAlive; // Used to keep the last polled system metrics for service private Metrics systemMetrics; @@ -42,7 +43,6 @@ public class VespaService implements Comparable<VespaService> { private final RemoteHealthMetricFetcher remoteHealthMetricFetcher; private final RemoteMetricsFetcher remoteMetricsFetcher; - private boolean isAlive; // Used to keep track of log level when health or metrics requests fail private AtomicInteger metricsFetchCount = new AtomicInteger(0); diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/TestUtil.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/TestUtil.java index e18bd38f97a..6f86be3aa30 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/TestUtil.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/TestUtil.java @@ -4,12 +4,22 @@ package ai.vespa.metricsproxy; +import ai.vespa.metricsproxy.core.MetricsConsumers; +import ai.vespa.metricsproxy.core.MetricsManager; +import ai.vespa.metricsproxy.core.VespaMetrics; +import ai.vespa.metricsproxy.metric.ExternalMetrics; +import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensions; +import ai.vespa.metricsproxy.metric.dimensions.NodeDimensions; +import ai.vespa.metricsproxy.service.VespaService; +import ai.vespa.metricsproxy.service.VespaServices; + import java.io.BufferedReader; import java.io.File; import java.io.FileReader; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.util.List; import java.util.stream.Collectors; /** @@ -17,6 +27,15 @@ import java.util.stream.Collectors; */ public class TestUtil { + public static MetricsManager createMetricsManager(VespaServices vespaServices, + MetricsConsumers consumers, + ApplicationDimensions applicationDimensions, + NodeDimensions nodeDimensions) { + VespaMetrics metrics = new VespaMetrics(consumers, vespaServices); + return new MetricsManager(vespaServices, metrics, new ExternalMetrics(consumers), + applicationDimensions, nodeDimensions); + } + public static String getFileContents(String filename) { InputStream in = TestUtil.class.getClassLoader().getResourceAsStream(filename); if (in == null) { diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/core/MetricsManagerTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/core/MetricsManagerTest.java index 81634323269..1635ccab197 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/core/MetricsManagerTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/core/MetricsManagerTest.java @@ -4,6 +4,7 @@ package ai.vespa.metricsproxy.core; +import ai.vespa.metricsproxy.TestUtil; import ai.vespa.metricsproxy.core.ConsumersConfig.Consumer; import ai.vespa.metricsproxy.metric.ExternalMetrics; import ai.vespa.metricsproxy.metric.Metric; @@ -56,7 +57,8 @@ public class MetricsManagerTest { @Before public void setupMetricsManager() { - metricsManager = getMetricsManager(); + metricsManager = TestUtil.createMetricsManager(new VespaServices(testServices), getMetricsConsumers(), + getApplicationDimensions(), getNodeDimensions()); } @Test @@ -201,15 +203,6 @@ public class MetricsManagerTest { return MetricsManager.adjustTimestamp(builder, startTime).getTimestamp(); } - private MetricsManager getMetricsManager() { - VespaServices vespaServices = new VespaServices(testServices); - MetricsConsumers consumers = getMetricsConsumers(); - VespaMetrics metrics = new VespaMetrics(consumers, vespaServices); - - return new MetricsManager(vespaServices, metrics, new ExternalMetrics(consumers), - getApplicationDimensions(),getNodeDimensions()); - } - private static MetricsConsumers getMetricsConsumers() { Consumer.Metric.Dimension.Builder metricDimension = new Consumer.Metric.Dimension.Builder() .key("dim0").value("metric-dim"); diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/GenericMetricsHandlerTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/GenericMetricsHandlerTest.java new file mode 100644 index 00000000000..744c96d7744 --- /dev/null +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/GenericMetricsHandlerTest.java @@ -0,0 +1,153 @@ +/* + * Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + */ + +package ai.vespa.metricsproxy.http; + +import ai.vespa.metricsproxy.TestUtil; +import ai.vespa.metricsproxy.core.ConsumersConfig; +import ai.vespa.metricsproxy.core.MetricsConsumers; +import ai.vespa.metricsproxy.core.MetricsManager; +import ai.vespa.metricsproxy.metric.Metric; +import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensions; +import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensionsConfig; +import ai.vespa.metricsproxy.metric.dimensions.NodeDimensions; +import ai.vespa.metricsproxy.metric.dimensions.NodeDimensionsConfig; +import ai.vespa.metricsproxy.metric.model.MetricsPacket; +import ai.vespa.metricsproxy.metric.model.json.GenericJsonModel; +import ai.vespa.metricsproxy.metric.model.json.GenericMetrics; +import ai.vespa.metricsproxy.metric.model.json.GenericService; +import ai.vespa.metricsproxy.service.DummyService; +import ai.vespa.metricsproxy.service.VespaService; +import ai.vespa.metricsproxy.service.VespaServices; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.ImmutableList; +import com.yahoo.container.jdisc.RequestHandlerTestDriver; +import org.junit.BeforeClass; +import org.junit.Ignore; +import org.junit.Test; + +import java.time.Instant; +import java.util.List; +import java.util.concurrent.Executors; + +import static ai.vespa.metricsproxy.core.VespaMetrics.INSTANCE_DIMENSION_ID; +import static ai.vespa.metricsproxy.core.VespaMetrics.VESPA_CONSUMER_ID; +import static ai.vespa.metricsproxy.metric.model.ServiceId.toServiceId; +import static ai.vespa.metricsproxy.metric.model.json.JacksonUtil.createObjectMapper; +import static ai.vespa.metricsproxy.service.DummyService.METRIC_1; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; + +/** + * @author gjoranv + */ +@SuppressWarnings("UnstableApiUsage") +public class GenericMetricsHandlerTest { + + private static final List<VespaService> testServices = ImmutableList.of( + new DummyService(0, "dummy/id/0"), + new DummyService(1, "dummy/id/0")); + + private static final String CPU_METRIC = "cpu"; + + private static final String URI = "http://localhost/metrics/v1/values"; + + private static final VespaServices vespaServices = new VespaServices(testServices); + + private static RequestHandlerTestDriver testDriver; + + @BeforeClass + public static void setupMetricsManager() { + MetricsManager metricsManager = TestUtil.createMetricsManager(vespaServices, getMetricsConsumers(), getApplicationDimensions(), getNodeDimensions()); + metricsManager.setExtraMetrics(ImmutableList.of( + new MetricsPacket.Builder(toServiceId("foo")) + .timestamp(Instant.now().getEpochSecond()) + .putMetrics(ImmutableList.of(new Metric(CPU_METRIC, 12.345))))); + GenericMetricsHandler handler = new GenericMetricsHandler(Executors.newSingleThreadExecutor(), metricsManager, vespaServices); + testDriver = new RequestHandlerTestDriver(handler); + } + + @Ignore + @Test + public void visually_inspect_response() throws Exception{ + String response = testDriver.sendRequest(URI).readAll(); + ObjectMapper mapper = createObjectMapper(); + var jsonModel = mapper.readValue(response, GenericJsonModel.class); + System.out.println(mapper.writerWithDefaultPrettyPrinter().writeValueAsString(jsonModel)); + } + + @Test + public void response_contains_node_metrics() throws Exception { + String response = testDriver.sendRequest(URI).readAll(); + var jsonModel = createObjectMapper().readValue(response, GenericJsonModel.class); + + assertNotNull(jsonModel.node); + assertEquals(1, jsonModel.node.metrics.size()); + assertEquals(12.345, jsonModel.node.metrics.get(0).values.get(CPU_METRIC), 0.0001d); + } + + @Test + public void response_contains_service_metrics() throws Exception { + String response = testDriver.sendRequest(URI).readAll(); + var jsonModel = createObjectMapper().readValue(response, GenericJsonModel.class); + + assertEquals(1, jsonModel.services.size()); + GenericService dummyService = jsonModel.services.get(0); + assertEquals(2, dummyService.metrics.size()); + + GenericMetrics dummy0Metrics = getMetricsForInstance("dummy0", dummyService); + assertEquals(1L, dummy0Metrics.values.get(METRIC_1).longValue()); + assertEquals("metric-dim", dummy0Metrics.dimensions.get("dim0")); + + GenericMetrics dummy1Metrics = getMetricsForInstance("dummy1", dummyService); + assertEquals(6L, dummy1Metrics.values.get(METRIC_1).longValue()); + assertEquals("metric-dim", dummy1Metrics.dimensions.get("dim0")); + } + + @Test + public void all_timestamps_are_equal_and_non_zero() throws Exception { + String response = testDriver.sendRequest(URI).readAll(); + var jsonModel = createObjectMapper().readValue(response, GenericJsonModel.class); + + Long nodeTimestamp = jsonModel.node.timestamp; + assertNotEquals(0L, (long) nodeTimestamp); + for (var service : jsonModel.services) + assertEquals(nodeTimestamp, service.timestamp); + } + + private static GenericMetrics getMetricsForInstance(String instance, GenericService service) { + for (var metrics : service.metrics) { + if (metrics.dimensions.get(INSTANCE_DIMENSION_ID.id).equals(instance)) + return metrics; + } + throw new RuntimeException("Could not find metrics for service instance " + instance); + } + + private static MetricsConsumers getMetricsConsumers() { + ConsumersConfig.Consumer.Metric.Dimension.Builder metricDimension = new ConsumersConfig.Consumer.Metric.Dimension.Builder() + .key("dim0").value("metric-dim"); + + return new MetricsConsumers(new ConsumersConfig.Builder() + .consumer(new ConsumersConfig.Consumer.Builder() + .name(VESPA_CONSUMER_ID.id) + .metric(new ConsumersConfig.Consumer.Metric.Builder() + .name(CPU_METRIC) + .outputname(CPU_METRIC)) + .metric(new ConsumersConfig.Consumer.Metric.Builder() + .name(METRIC_1) + .outputname(METRIC_1) + .dimension(metricDimension))) + .build()); + } + + private static ApplicationDimensions getApplicationDimensions() { + return new ApplicationDimensions(new ApplicationDimensionsConfig.Builder().build()); + } + + private static NodeDimensions getNodeDimensions() { + return new NodeDimensions(new NodeDimensionsConfig.Builder().build()); + } + +} diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModelTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModelTest.java index 44fe383839f..dc3eb12ff2c 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModelTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModelTest.java @@ -64,7 +64,7 @@ public class GenericJsonModelTest { assertNotNull(jsonModel.node); assertEquals(1, jsonModel.node.metrics.size()); GenericMetrics nodeMetrics = jsonModel.node.metrics.get(0); - assertEquals(1.234, jsonModel.node.metrics.get(0).values.get("node-metric"), 0.001d); + assertEquals(1.234, nodeMetrics.values.get("node-metric"), 0.001d); assertEquals("node-dim-value", nodeMetrics.dimensions.get("node-dim")); assertEquals(1, jsonModel.services.size()); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java index 6dfcf3e9aec..2f5d6ae60d8 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java @@ -294,6 +294,7 @@ public class DockerOperationsImpl implements DockerOperations { context.pathInNodeUnderVespaHome("var/jdisc_container"), context.pathInNodeUnderVespaHome("var/jdisc_core"), context.pathInNodeUnderVespaHome("var/maven"), + context.pathInNodeUnderVespaHome("var/mediasearch"), // TODO: Remove when vespa-routing is no more context.pathInNodeUnderVespaHome("var/run"), context.pathInNodeUnderVespaHome("var/scoreboards"), context.pathInNodeUnderVespaHome("var/service"), diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java index 4b086e1ea09..684f6dbcd50 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java @@ -63,15 +63,15 @@ public class FailedExpirer extends Maintainer { this.nodeRepository = nodeRepository; this.zone = zone; this.clock = clock; - if (zone.system() == SystemName.main) { + if (zone.system().isCd()) { + defaultExpiry = containerExpiry = Duration.ofMinutes(30); + } else { if (zone.environment() == Environment.staging || zone.environment() == Environment.test) { defaultExpiry = Duration.ofHours(1); } else { defaultExpiry = Duration.ofDays(4); } containerExpiry = Duration.ofHours(1); - } else { - defaultExpiry = containerExpiry = Duration.ofMinutes(30); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java index 50a371458ce..9ace6ad55ef 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java @@ -184,7 +184,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { hostProvisionerInterval = Duration.ofMinutes(5); hostDeprovisionerInterval = Duration.ofMinutes(5); - if (zone.environment().equals(Environment.prod) && zone.system() != SystemName.cd) { + if (zone.environment().equals(Environment.prod) && ! zone.system().isCd()) { inactiveExpiry = Duration.ofHours(4); // enough time for the application owner to discover and redeploy retiredInterval = Duration.ofMinutes(10); dirtyExpiry = Duration.ofHours(2); // enough time to clean the node diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java index 9d8bc047862..feccfb430e6 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java @@ -70,8 +70,8 @@ public class DockerHostCapacity { .filter(n -> n.type().equals(NodeType.host)) .filter(n -> speed == NodeResources.DiskSpeed.any || n.flavor().resources().diskSpeed() == speed) .map(n -> freeCapacityOf(n, false)) - .reduce(new NodeResources(0, 0, 0, speed), NodeResources::add) - .withDiskSpeed(speed); // Set speed to 'any' if necessary + .map(resources -> resources.withDiskSpeed(speed)) // Set speed to 'any' if necessary + .reduce(new NodeResources(0, 0, 0, speed), NodeResources::add); } /** Return total capacity for a given disk speed (or for any disk speed) */ @@ -80,8 +80,8 @@ public class DockerHostCapacity { .filter(n -> n.type().equals(NodeType.host)) .filter(n -> speed == NodeResources.DiskSpeed.any || n.flavor().resources().diskSpeed() == speed) .map(host -> host.flavor().resources()) - .reduce(new NodeResources(0, 0, 0, speed), NodeResources::add) - .withDiskSpeed(speed); // Set speed to 'any' if necessary + .map(resources -> resources.withDiskSpeed(speed)) // Set speed to 'any' if necessary + .reduce(new NodeResources(0, 0, 0, speed), NodeResources::add); } public int freeCapacityInFlavorEquivalence(Flavor flavor) { diff --git a/searchcore/src/tests/proton/attribute/attribute_test.cpp b/searchcore/src/tests/proton/attribute/attribute_test.cpp index 992a9c56dfb..3734d2fe1dc 100644 --- a/searchcore/src/tests/proton/attribute/attribute_test.cpp +++ b/searchcore/src/tests/proton/attribute/attribute_test.cpp @@ -7,9 +7,8 @@ #include <vespa/document/update/arithmeticvalueupdate.h> #include <vespa/document/update/assignvalueupdate.h> #include <vespa/document/update/documentupdate.h> -#include <vespa/eval/tensor/default_tensor.h> #include <vespa/eval/tensor/tensor.h> -#include <vespa/eval/tensor/tensor_factory.h> +#include <vespa/eval/tensor/default_tensor_engine.h> #include <vespa/eval/tensor/types.h> #include <vespa/fastos/file.h> #include <vespa/searchcommon/attribute/attributecontent.h> @@ -75,9 +74,9 @@ using search::tensor::TensorAttribute; using search::test::DirectoryHandler; using std::string; using vespalib::eval::ValueType; +using vespalib::eval::TensorSpec; using vespalib::tensor::Tensor; -using vespalib::tensor::TensorCells; -using vespalib::tensor::TensorDimensions; +using vespalib::tensor::DefaultTensorEngine; using AVConfig = search::attribute::Config; using AVBasicType = search::attribute::BasicType; @@ -612,13 +611,11 @@ TEST_F("require that filter attribute manager can return flushed serial number", namespace { -Tensor::UP -createTensor(const TensorCells &cells, const TensorDimensions &dimensions) { - vespalib::tensor::DefaultTensor::builder builder; - return vespalib::tensor::TensorFactory::create(cells, dimensions, builder); +Tensor::UP make_tensor(const TensorSpec &spec) { + auto tensor = DefaultTensorEngine::ref().from_spec(spec); + return Tensor::UP(dynamic_cast<Tensor*>(tensor.release())); } - AttributeVector::SP createTensorAttribute(Fixture &f) { AVConfig cfg(AVBasicType::TENSOR); @@ -650,8 +647,8 @@ TEST_F("Test that we can use attribute writer to write to tensor attribute", AttributeVector::SP a1 = createTensorAttribute(f); Schema s = createTensorSchema(); DocBuilder builder(s); - auto tensor = createTensor({ {{{"x", "4"}, {"y", "5"}}, 7} }, - {"x", "y"}); + auto tensor = make_tensor(TensorSpec("tensor(x{},y{})") + .add({{"x", "4"}, {"y", "5"}}, 7)); Document::UP doc = createTensorPutDoc(builder, *tensor); f.put(1, *doc, 1); EXPECT_EQUAL(2u, a1->getNumDocs()); @@ -668,8 +665,8 @@ TEST_F("require that attribute writer handles tensor assign update", Fixture) AttributeVector::SP a1 = createTensorAttribute(f); Schema s = createTensorSchema(); DocBuilder builder(s); - auto tensor = createTensor({ {{{"x", "6"}, {"y", "7"}}, 9} }, - {"x", "y"}); + auto tensor = make_tensor(TensorSpec("tensor(x{},y{})") + .add({{"x", "6"}, {"y", "7"}}, 9)); Document::UP doc = createTensorPutDoc(builder, *tensor); f.put(1, *doc, 1); EXPECT_EQUAL(2u, a1->getNumDocs()); @@ -682,8 +679,8 @@ TEST_F("require that attribute writer handles tensor assign update", Fixture) const document::DocumentType &dt(builder.getDocumentType()); DocumentUpdate upd(*builder.getDocumentTypeRepo(), dt, DocumentId("doc::1")); - auto new_tensor = createTensor({ {{{"x", "8"}, {"y", "9"}}, 11} }, - {"x", "y"}); + auto new_tensor = make_tensor(TensorSpec("tensor(x{},y{})") + .add({{"x", "8"}, {"y", "9"}}, 11)); TensorDataType xySparseTensorDataType(vespalib::eval::ValueType::from_spec("tensor(x{},y{})")); TensorFieldValue new_value(xySparseTensorDataType); new_value = new_tensor->clone(); diff --git a/searchcore/src/tests/proton/docsummary/docsummary.cpp b/searchcore/src/tests/proton/docsummary/docsummary.cpp index 38b85594a62..e8152161faa 100644 --- a/searchcore/src/tests/proton/docsummary/docsummary.cpp +++ b/searchcore/src/tests/proton/docsummary/docsummary.cpp @@ -2,9 +2,9 @@ #include <tests/proton/common/dummydbowner.h> #include <vespa/config/helper/configgetter.hpp> -#include <vespa/eval/tensor/default_tensor.h> +#include <vespa/eval/tensor/tensor.h> +#include <vespa/eval/tensor/default_tensor_engine.h> #include <vespa/eval/tensor/serialization/typed_binary_format.h> -#include <vespa/eval/tensor/tensor_factory.h> #include <vespa/document/repo/documenttyperepo.h> #include <vespa/document/test/make_bucket_space.h> #include <vespa/searchcore/proton/attribute/attribute_writer.h> @@ -55,10 +55,9 @@ using search::index::schema::CollectionType; using storage::spi::Timestamp; using vespa::config::search::core::ProtonConfig; using vespa::config::content::core::BucketspacesConfig; +using vespalib::eval::TensorSpec; using vespalib::tensor::Tensor; -using vespalib::tensor::TensorCells; -using vespalib::tensor::TensorDimensions; -using vespalib::tensor::TensorFactory; +using vespalib::tensor::DefaultTensorEngine; using namespace vespalib::slime; typedef std::unique_ptr<GeneralResult> GeneralResultPtr; @@ -137,10 +136,9 @@ getDocTypeName() return "searchdocument"; } -Tensor::UP createTensor(const TensorCells &cells, - const TensorDimensions &dimensions) { - vespalib::tensor::DefaultTensor::builder builder; - return TensorFactory::create(cells, dimensions, builder); +Tensor::UP make_tensor(const TensorSpec &spec) { + auto tensor = DefaultTensorEngine::ref().from_spec(spec); + return Tensor::UP(dynamic_cast<Tensor*>(tensor.release())); } vespalib::string asVstring(vespalib::Memory str) { @@ -750,7 +748,8 @@ Test::requireThatAttributesAreUsed() endElement(). endField(). startAttributeField("bj"). - addTensor(createTensor({ {{{"x","f"},{"y","g"}}, 3} }, { "x", "y"})). + addTensor(make_tensor(TensorSpec("tensor(x{},y{})") + .add({{"x", "f"}, {"y", "g"}}, 3))). endField(). endDocument(), 2); @@ -776,7 +775,8 @@ Test::requireThatAttributesAreUsed() "bh:[{item:40.4,weight:4},{item:50.5,weight:5}]," "bi:[{item:'quux',weight:7},{item:'qux',weight:6}]," "bj:'0x01020178017901016601674008000000000000'}", *rep, 0, true)); - TEST_DO(assertTensor(createTensor({ {{{"x","f"},{"y","g"}}, 3} }, { "x", "y"}), + TEST_DO(assertTensor(make_tensor(TensorSpec("tensor(x{},y{})") + .add({{"x", "f"}, {"y", "g"}}, 3)), "bj", *rep, 0, rclass)); // empty doc @@ -790,13 +790,15 @@ Test::requireThatAttributesAreUsed() attributeFieldWriter.execute(attributeFieldWriter.getExecutorId(bjAttr->getNamePrefix()), [&]() { - bjTensorAttr->setTensor(3, *createTensor({ {{{"x", "a"},{"y", "b"}}, 4} }, { "x"})); + bjTensorAttr->setTensor(3, *make_tensor(TensorSpec("tensor(x{},y{})") + .add({{"x", "a"}, {"y", "b"}}, 4))); bjTensorAttr->commit(); }); attributeFieldWriter.sync(); DocsumReply::UP rep2 = dc._ddb->getDocsums(req); - TEST_DO(assertTensor(createTensor({ {{{"x","a"},{"y","b"}}, 4} }, { "x", "y"}), + TEST_DO(assertTensor(make_tensor(TensorSpec("tensor(x{},y{})") + .add({{"x", "a"}, {"y", "b"}}, 4)), "bj", *rep2, 1, rclass)); DocsumRequest req3; diff --git a/searchcore/src/tests/proton/docsummary/summaryfieldconverter_test.cpp b/searchcore/src/tests/proton/docsummary/summaryfieldconverter_test.cpp index b82fec85d47..cc6eef14fd6 100644 --- a/searchcore/src/tests/proton/docsummary/summaryfieldconverter_test.cpp +++ b/searchcore/src/tests/proton/docsummary/summaryfieldconverter_test.cpp @@ -47,8 +47,7 @@ #include <vespa/searchlib/util/slime_output_raw_buf_adapter.h> #include <vespa/eval/tensor/tensor.h> #include <vespa/eval/tensor/types.h> -#include <vespa/eval/tensor/default_tensor.h> -#include <vespa/eval/tensor/tensor_factory.h> +#include <vespa/eval/tensor/default_tensor_engine.h> #include <vespa/vespalib/data/slime/slime.h> using document::Annotation; @@ -95,12 +94,12 @@ using vespa::config::search::SummarymapConfig; using vespa::config::search::SummarymapConfigBuilder; using vespalib::Slime; using vespalib::eval::ValueType; +using vespalib::eval::TensorSpec; using vespalib::geo::ZCurve; using vespalib::slime::Cursor; using vespalib::string; using vespalib::tensor::Tensor; -using vespalib::tensor::TensorCells; -using vespalib::tensor::TensorDimensions; +using vespalib::tensor::DefaultTensorEngine; using namespace search::docsummary; @@ -676,11 +675,9 @@ Test::requireThatPredicateIsPrinted() SFC::convertSummaryField(false, *doc.getValue("predicate")).get()); } - -Tensor::UP -createTensor(const TensorCells &cells, const TensorDimensions &dimensions) { - vespalib::tensor::DefaultTensor::builder builder; - return vespalib::tensor::TensorFactory::create(cells, dimensions, builder); +Tensor::UP make_tensor(const TensorSpec &spec) { + auto tensor = DefaultTensorEngine::ref().from_spec(spec); + return Tensor::UP(dynamic_cast<Tensor*>(tensor.release())); } void @@ -688,14 +685,14 @@ Test::requireThatTensorIsNotConverted() { TensorDataType tensorDataType(ValueType::from_spec("tensor(x{},y{})")); TensorFieldValue tensorFieldValue(tensorDataType); - tensorFieldValue = createTensor({ {{{"x", "4"}, {"y", "5"}}, 7} }, - {"x", "y"}); + tensorFieldValue = make_tensor(TensorSpec("tensor(x{},y{})") + .add({{"x", "4"}, {"y", "5"}}, 7)); Document doc(getDocType(), DocumentId("doc:scheme:")); doc.setRepo(*_documentRepo); doc.setValue("tensor", tensorFieldValue); - TEST_CALL(checkTensor(createTensor({ {{{"x", "4"}, {"y", "5"}}, 7} }, - {"x", "y"}), + TEST_CALL(checkTensor(make_tensor(TensorSpec("tensor(x{},y{})") + .add({{"x", "4"}, {"y", "5"}}, 7)), SFC::convertSummaryField(false, *doc.getValue("tensor")).get())); doc.setValue("tensor", TensorFieldValue()); diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp index af6a9d38385..10b1c829674 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp @@ -191,7 +191,7 @@ MySubDb::MySubDb(const std::shared_ptr<const DocumentTypeRepo> &repo, std::share _metaStore(*_metaStoreSP), _realRetriever(std::make_shared<MyDocumentRetriever>(repo)), _retriever(_realRetriever), - _subDb(_metaStoreSP, _retriever, subDbId), _docs(), + _subDb("my_sub_db", subDbId, _metaStoreSP, _retriever, IFeedView::SP()), _docs(), _bucketDBHandler(*bucketDB) { _bucketDBHandler.addDocumentMetaStore(_metaStoreSP.get(), 0); @@ -238,7 +238,11 @@ struct MoveFixture void setupForBucket(const BucketId &bucket, uint32_t sourceSubDbId, uint32_t targetSubDbId) { - _source._subDb._subDbId = sourceSubDbId; + _source._subDb = MaintenanceDocumentSubDB(_source._subDb.name(), + sourceSubDbId, + _source._subDb.meta_store(), + _source._subDb.retriever(), + _source._subDb.feed_view()); _mover.setupForBucket(bucket, &_source._subDb, targetSubDbId, _handler, _bucketDb); } void moveDocuments(size_t maxDocsToMove) { diff --git a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp index 82566025a30..5305ff15003 100644 --- a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp +++ b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp @@ -160,51 +160,59 @@ struct MyFrozenBucketHandler : public IFrozenBucketHandler virtual void removeListener(IBucketFreezeListener *) override { } }; -struct MyFeedView : public test::DummyFeedView -{ - MyFeedView(const std::shared_ptr<const DocumentTypeRepo> &repo) - : test::DummyFeedView(repo) +struct MyFeedView : public test::DummyFeedView { + MyFeedView(std::shared_ptr<const DocumentTypeRepo> repo) + : test::DummyFeedView(std::move(repo)) { } }; -struct MyDocumentStore : public test::DummyDocumentStore -{ +struct MyDocumentStore : public test::DummyDocumentStore { Document::SP _readDoc; mutable uint32_t _readLid; MyDocumentStore() : _readDoc(), _readLid(0) {} - virtual document::Document::UP - read(search::DocumentIdT lid, const document::DocumentTypeRepo &) const override { + ~MyDocumentStore(); + document::Document::UP read(search::DocumentIdT lid, const document::DocumentTypeRepo &) const override { _readLid = lid; return Document::UP(_readDoc->clone()); } }; -struct MySummaryManager : public test::DummySummaryManager -{ - MyDocumentStore _store; - MySummaryManager() : _store() {} - virtual search::IDocumentStore &getBackingStore() override { return _store; } -}; +MyDocumentStore::~MyDocumentStore() = default; -struct MySubDb : public test::DummyDocumentSubDb -{ - std::shared_ptr<const DocumentTypeRepo> _repo; - MySubDb(const std::shared_ptr<const DocumentTypeRepo> &repo, std::shared_ptr<BucketDBOwner> bucketDB); - ~MySubDb(); - virtual IFeedView::SP getFeedView() const override { - return IFeedView::SP(new MyFeedView(_repo)); +struct MyDocumentRetriever : public DocumentRetrieverBaseForTest { + std::shared_ptr<const DocumentTypeRepo> repo; + const MyDocumentStore& store; + MyDocumentRetriever(std::shared_ptr<const DocumentTypeRepo> repo_in, const MyDocumentStore& store_in) + : repo(repo_in), + store(store_in) + { + } + const document::DocumentTypeRepo& getDocumentTypeRepo() const override { return *repo; } + void getBucketMetaData(const storage::spi::Bucket&, DocumentMetaData::Vector& ) const override { abort(); } + DocumentMetaData getDocumentMetaData(const DocumentId& ) const override { abort(); } + Document::UP getDocument(DocumentIdT lid) const override { + return store.read(lid, *repo); } + CachedSelect::SP parseSelect(const vespalib::string &) const override { abort(); } }; +struct MySubDb { + test::DummyDocumentSubDb sub_db; + MaintenanceDocumentSubDB maintenance_sub_db; + MySubDb(std::shared_ptr<BucketDBOwner> bucket_db, const MyDocumentStore& store, std::shared_ptr<const DocumentTypeRepo> repo); + ~MySubDb(); +}; -MySubDb::MySubDb(const std::shared_ptr<const DocumentTypeRepo> &repo, std::shared_ptr<BucketDBOwner> bucketDB) - : test::DummyDocumentSubDb(bucketDB, SUBDB_ID), - _repo(repo) +MySubDb::MySubDb(std::shared_ptr<BucketDBOwner> bucket_db, const MyDocumentStore& store, std::shared_ptr<const DocumentTypeRepo> repo) + : sub_db(std::move(bucket_db), SUBDB_ID), + maintenance_sub_db(sub_db.getName(), sub_db.getSubDbId(), sub_db.getDocumentMetaStoreContext().getSP(), + std::make_shared<MyDocumentRetriever>(repo, store), + std::make_shared<MyFeedView>(repo)) { - _summaryManager.reset(new MySummaryManager()); } -MySubDb::~MySubDb() {} + +MySubDb::~MySubDb() = default; struct MyDirectJobRunner : public IMaintenanceJobRunner { IMaintenanceJob &_job; @@ -350,17 +358,15 @@ struct HandlerFixture { DocBuilder _docBuilder; std::shared_ptr<BucketDBOwner> _bucketDB; + MyDocumentStore _docStore; MySubDb _subDb; - MySummaryManager &_summaryMgr; - MyDocumentStore &_docStore; LidSpaceCompactionHandler _handler; HandlerFixture() : _docBuilder(Schema()), _bucketDB(std::make_shared<BucketDBOwner>()), - _subDb(_docBuilder.getDocumentTypeRepo(), _bucketDB), - _summaryMgr(static_cast<MySummaryManager &>(*_subDb.getSummaryManager())), - _docStore(_summaryMgr._store), - _handler(_subDb, "test") + _docStore(), + _subDb(_bucketDB, _docStore, _docBuilder.getDocumentTypeRepo()), + _handler(_subDb.maintenance_sub_db, "test") { _docStore._readDoc = _docBuilder.startDocument(DOC_ID).endDocument(); } diff --git a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp index c3974368e54..a4f2aeb767d 100644 --- a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp +++ b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp @@ -523,9 +523,10 @@ MyDocumentSubDB::getSubDB() { IDocumentRetriever::SP retriever(new MyDocumentRetriever(*this)); - return MaintenanceDocumentSubDB(_metaStoreSP, + return MaintenanceDocumentSubDB("my_sub_db", _subDBId, + _metaStoreSP, retriever, - _subDBId); + IFeedView::SP()); } diff --git a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp index cb929e0a6c7..4848c5a5d47 100644 --- a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp @@ -88,9 +88,9 @@ BucketMoveJob::checkBucket(const BucketId &bucket, const MaintenanceDocumentSubDB &source(wantReady ? _notReady : _ready); const MaintenanceDocumentSubDB &target(wantReady ? _ready : _notReady); LOG(debug, "checkBucket(): mover.setupForBucket(%s, source:%u, target:%u)", - bucket.toString().c_str(), source._subDbId, target._subDbId); - mover.setupForBucket(bucket, &source, target._subDbId, - _moveHandler, _ready._metaStore->getBucketDB()); + bucket.toString().c_str(), source.sub_db_id(), target.sub_db_id()); + mover.setupForBucket(bucket, &source, target.sub_db_id(), + _moveHandler, _ready.meta_store()->getBucketDB()); } BucketMoveJob::ScanResult @@ -98,7 +98,7 @@ BucketMoveJob::scanBuckets(size_t maxBucketsToScan, IFrozenBucketHandler::Exclus { size_t bucketsScanned = 0; bool passDone = false; - ScanIterator itr(_ready._metaStore->getBucketDB().takeGuard(), + ScanIterator itr(_ready.meta_store()->getBucketDB().takeGuard(), _scanPass, _scanPos._lastBucket, _endPos._lastBucket); BucketId bucket; for (; itr.valid() && @@ -250,7 +250,7 @@ BucketMoveJob::deactivateBucket(BucketId bucket) void BucketMoveJob::activateBucket(BucketId bucket) { - BucketDBOwner::Guard notReadyBdb(_notReady._metaStore->getBucketDB().takeGuard()); + BucketDBOwner::Guard notReadyBdb(_notReady.meta_store()->getBucketDB().takeGuard()); if (notReadyBdb->get(bucket).getDocumentCount() == 0) { return; // notready bucket already empty. This is the normal case. } @@ -291,7 +291,7 @@ BucketMoveJob::scanAndMove(size_t maxBucketsToScan, while (!_delayedBuckets.empty() && _delayedMover.bucketDone()) { const BucketId bucket = *_delayedBuckets.begin(); _delayedBuckets.erase(_delayedBuckets.begin()); - ScanIterator itr(_ready._metaStore->getBucketDB().takeGuard(), bucket); + ScanIterator itr(_ready.meta_store()->getBucketDB().takeGuard(), bucket); if (itr.getBucket() == bucket) { checkBucket(bucket, itr, _delayedMover, bucketGuard); } diff --git a/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp b/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp index 3c73532fbba..d6d8ba03a67 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp @@ -24,12 +24,12 @@ DocumentBucketMover::moveDocument(DocumentIdT lid, const document::GlobalId &gid, Timestamp timestamp) { - Document::SP doc(_source->_retriever->getDocument(lid).release()); + Document::SP doc(_source->retriever()->getDocument(lid).release()); if (!doc || doc->getId().getGlobalId() != gid) return; // Failed to retrieve document, removed or changed identity // TODO(geirst): what if doc is NULL? BucketId bucketId = _bucket.stripUnused(); - MoveOperation op(bucketId, timestamp, doc, DbDocumentId(_source->_subDbId, lid), _targetSubDbId); + MoveOperation op(bucketId, timestamp, doc, DbDocumentId(_source->sub_db_id(), lid), _targetSubDbId); // We cache the bucket for the document we are going to move to avoid getting // inconsistent bucket info (getBucketInfo()) while moving between ready and not-ready @@ -103,16 +103,16 @@ DocumentBucketMover::moveDocuments(size_t maxDocsToMove) if (_bucketDone) { return; } - Iterator itr = (_lastGidValid ? _source->_metaStore->upperBound(_lastGid) - : _source->_metaStore->lowerBound(_bucket)); - const Iterator end = _source->_metaStore->upperBound(_bucket); + Iterator itr = (_lastGidValid ? _source->meta_store()->upperBound(_lastGid) + : _source->meta_store()->lowerBound(_bucket)); + const Iterator end = _source->meta_store()->upperBound(_bucket); size_t docsMoved = 0; size_t docsSkipped = 0; // In absence of a proper cost metric typedef std::vector<MoveKey> MoveVec; MoveVec toMove; for (; itr != end && docsMoved < maxDocsToMove; ++itr) { DocumentIdT lid = itr.getKey(); - const RawDocumentMetaData &metaData = _source->_metaStore->getRawMetaData(lid); + const RawDocumentMetaData &metaData = _source->meta_store()->getRawMetaData(lid); if (metaData.getBucketUsedBits() != _bucket.getUsedBits()) { ++docsSkipped; if (docsSkipped >= 50) { diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp index 16e8d7ae90a..5aacfa2678c 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp @@ -1,5 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include "bootstrapconfig.h" #include "combiningfeedview.h" #include "commit_and_wait_document_retriever.h" #include "document_meta_store_read_guards.h" @@ -7,29 +8,29 @@ #include "documentdb.h" #include "documentdbconfigscout.h" #include "idocumentdbowner.h" +#include "idocumentsubdb.h" #include "lid_space_compaction_handler.h" #include "maintenance_jobs_injector.h" #include "reconfig_params.h" -#include "bootstrapconfig.h" #include <vespa/document/repo/documenttyperepo.h> -#include <vespa/searchcore/proton/metrics/executor_threading_service_stats.h> +#include <vespa/searchcommon/common/schemaconfigurer.h> #include <vespa/searchcore/proton/attribute/attribute_writer.h> #include <vespa/searchcore/proton/attribute/imported_attributes_repo.h> #include <vespa/searchcore/proton/common/eventlogger.h> #include <vespa/searchcore/proton/common/statusreport.h> +#include <vespa/searchcore/proton/docsummary/isummarymanager.h> #include <vespa/searchcore/proton/feedoperation/noopoperation.h> #include <vespa/searchcore/proton/index/index_writer.h> #include <vespa/searchcore/proton/initializer/task_runner.h> +#include <vespa/searchcore/proton/metrics/executor_threading_service_stats.h> #include <vespa/searchcore/proton/metrics/metricswireservice.h> -#include <vespa/searchcore/proton/reference/i_document_db_reference_resolver.h> -#include <vespa/searchcore/proton/reference/i_document_db_reference_registry.h> #include <vespa/searchcore/proton/reference/document_db_reference_resolver.h> -#include <vespa/searchcore/proton/docsummary/isummarymanager.h> +#include <vespa/searchcore/proton/reference/i_document_db_reference_registry.h> +#include <vespa/searchcore/proton/reference/i_document_db_reference_resolver.h> #include <vespa/searchlib/attribute/attributefactory.h> #include <vespa/searchlib/attribute/configconverter.h> #include <vespa/searchlib/engine/docsumreply.h> #include <vespa/searchlib/engine/searchreply.h> -#include <vespa/searchcommon/common/schemaconfigurer.h> #include <vespa/vespalib/io/fileutil.h> #include <vespa/vespalib/util/closuretask.h> #include <vespa/vespalib/util/exceptions.h> @@ -205,9 +206,11 @@ DocumentDB::DocumentDB(const vespalib::string &baseDir, _clusterStateHandler.addClusterStateChangedHandler(this); // Forward changes of cluster state to bucket handler _clusterStateHandler.addClusterStateChangedHandler(&_bucketHandler); - for (auto subDb : _subDBs) { - _lidSpaceCompactionHandlers.push_back(std::make_unique<LidSpaceCompactionHandler>(*subDb, _docTypeName.getName())); - } + + _lidSpaceCompactionHandlers.push_back(std::make_unique<LidSpaceCompactionHandler>(_maintenanceController.getReadySubDB(), _docTypeName.getName())); + _lidSpaceCompactionHandlers.push_back(std::make_unique<LidSpaceCompactionHandler>(_maintenanceController.getRemSubDB(), _docTypeName.getName())); + _lidSpaceCompactionHandlers.push_back(std::make_unique<LidSpaceCompactionHandler>(_maintenanceController.getNotReadySubDB(), _docTypeName.getName())); + _writeFilter.setConfig(loaded_config->getMaintenanceConfigSP()->getAttributeUsageFilterConfig()); fastos::TimeStamp visibilityDelay = loaded_config->getMaintenanceConfigSP()->getVisibilityDelay(); _visibility.setVisibilityDelay(visibilityDelay); diff --git a/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.cpp b/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.cpp index 70e548e8c05..682719436f1 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.cpp @@ -139,18 +139,21 @@ wrapRetriever(const IDocumentRetriever::SP &retriever, ICommitable &commit) void DocumentSubDBCollection::maintenanceSync(MaintenanceController &mc, ICommitable &commit) { RetrieversSP retrievers = getRetrievers(); - MaintenanceDocumentSubDB readySubDB( - getReadySubDB()->getDocumentMetaStoreContext().getSP(), - wrapRetriever((*retrievers)[_readySubDbId], commit), - _readySubDbId); - MaintenanceDocumentSubDB remSubDB( - getRemSubDB()->getDocumentMetaStoreContext().getSP(), - (*retrievers)[_remSubDbId], - _remSubDbId); - MaintenanceDocumentSubDB notReadySubDB( - getNotReadySubDB()->getDocumentMetaStoreContext().getSP(), - wrapRetriever((*retrievers)[_notReadySubDbId], commit), - _notReadySubDbId); + MaintenanceDocumentSubDB readySubDB(getReadySubDB()->getName(), + _readySubDbId, + getReadySubDB()->getDocumentMetaStoreContext().getSP(), + wrapRetriever((*retrievers)[_readySubDbId], commit), + getReadySubDB()->getFeedView()); + MaintenanceDocumentSubDB remSubDB(getRemSubDB()->getName(), + _remSubDbId, + getRemSubDB()->getDocumentMetaStoreContext().getSP(), + (*retrievers)[_remSubDbId], + getRemSubDB()->getFeedView()); + MaintenanceDocumentSubDB notReadySubDB(getNotReadySubDB()->getName(), + _notReadySubDbId, + getNotReadySubDB()->getDocumentMetaStoreContext().getSP(), + wrapRetriever((*retrievers)[_notReadySubDbId], commit), + getNotReadySubDB()->getFeedView()); mc.syncSubDBs(readySubDB, remSubDB, notReadySubDB); } diff --git a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_handler.cpp b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_handler.cpp index a77aa302fea..c4dc26a0875 100644 --- a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_handler.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_handler.cpp @@ -16,8 +16,8 @@ using storage::spi::Timestamp; namespace proton { -LidSpaceCompactionHandler::LidSpaceCompactionHandler(IDocumentSubDB &subDb, - const vespalib::string &docTypeName) +LidSpaceCompactionHandler::LidSpaceCompactionHandler(const MaintenanceDocumentSubDB& subDb, + const vespalib::string& docTypeName) : _subDb(subDb), _docTypeName(docTypeName) { @@ -26,27 +26,24 @@ LidSpaceCompactionHandler::LidSpaceCompactionHandler(IDocumentSubDB &subDb, LidUsageStats LidSpaceCompactionHandler::getLidStatus() const { - return _subDb.getDocumentMetaStoreContext().get().getLidUsageStats(); + return _subDb.meta_store()->getLidUsageStats(); } IDocumentScanIterator::UP LidSpaceCompactionHandler::getIterator() const { - return IDocumentScanIterator::UP(new DocumentScanIterator( - _subDb.getDocumentMetaStoreContext().get())); + return std::make_unique<DocumentScanIterator>(*_subDb.meta_store()); } MoveOperation::UP LidSpaceCompactionHandler::createMoveOperation(const search::DocumentMetaData &document, uint32_t moveToLid) const { - IFeedView::SP feedView = _subDb.getFeedView(); - const ISummaryManager::SP &summaryMan = _subDb.getSummaryManager(); const uint32_t moveFromLid = document.lid; - Document::UP doc = summaryMan->getBackingStore().read(moveFromLid, *feedView->getDocumentTypeRepo()); - MoveOperation::UP op(new MoveOperation(document.bucketId, document.timestamp, - Document::SP(doc.release()), - DbDocumentId(_subDb.getSubDbId(), moveFromLid), - _subDb.getSubDbId())); + auto doc = _subDb.retriever()->getDocument(moveFromLid); + auto op = std::make_unique<MoveOperation>(document.bucketId, document.timestamp, + Document::SP(doc.release()), + DbDocumentId(_subDb.sub_db_id(), moveFromLid), + _subDb.sub_db_id()); op->setTargetLid(moveToLid); return op; } @@ -54,14 +51,14 @@ LidSpaceCompactionHandler::createMoveOperation(const search::DocumentMetaData &d void LidSpaceCompactionHandler::handleMove(const MoveOperation& op, IDestructorCallback::SP doneCtx) { - _subDb.getFeedView()->handleMove(op, std::move(doneCtx)); + _subDb.feed_view()->handleMove(op, std::move(doneCtx)); } void LidSpaceCompactionHandler::handleCompactLidSpace(const CompactLidSpaceOperation &op) { - assert(_subDb.getSubDbId() == op.getSubDbId()); - _subDb.getFeedView()->handleCompactLidSpace(op); + assert(_subDb.sub_db_id() == op.getSubDbId()); + _subDb.feed_view()->handleCompactLidSpace(op); } } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_handler.h b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_handler.h index 0861513a909..21d20001923 100644 --- a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_handler.h +++ b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_handler.h @@ -2,7 +2,7 @@ #pragma once #include "i_lid_space_compaction_handler.h" -#include "idocumentsubdb.h" +#include "maintenancedocumentsubdb.h" namespace proton { @@ -12,18 +12,18 @@ namespace proton { class LidSpaceCompactionHandler : public ILidSpaceCompactionHandler { private: - IDocumentSubDB &_subDb; + const MaintenanceDocumentSubDB& _subDb; vespalib::string _docTypeName; public: - LidSpaceCompactionHandler(IDocumentSubDB &subDb, - const vespalib::string &docTypeName); + LidSpaceCompactionHandler(const MaintenanceDocumentSubDB& subDb, + const vespalib::string& docTypeName); // Implements ILidSpaceCompactionHandler virtual vespalib::string getName() const override { - return _docTypeName + "." + _subDb.getName(); + return _docTypeName + "." + _subDb.name(); } - virtual uint32_t getSubDbId() const override { return _subDb.getSubDbId(); } + virtual uint32_t getSubDbId() const override { return _subDb.sub_db_id(); } virtual search::LidUsageStats getLidStatus() const override; virtual IDocumentScanIterator::UP getIterator() const override; virtual MoveOperation::UP createMoveOperation(const search::DocumentMetaData &document, uint32_t moveToLid) const override; 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 744ce2aa7f4..6b130dfa144 100644 --- a/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp @@ -111,8 +111,8 @@ MaintenanceJobsInjector::injectJobs(MaintenanceController &controller, controller.registerJobInMasterThread(MUP(new DocumentDBCommitJob(commit, config.getVisibilityDelay()))); } const MaintenanceDocumentSubDB &mRemSubDB(controller.getRemSubDB()); - MUP pruneRDjob(new PruneRemovedDocumentsJob(config.getPruneRemovedDocumentsConfig(), *mRemSubDB._metaStore, - mRemSubDB._subDbId, docTypeName, prdHandler, fbHandler)); + MUP pruneRDjob(new PruneRemovedDocumentsJob(config.getPruneRemovedDocumentsConfig(), *mRemSubDB.meta_store(), + mRemSubDB.sub_db_id(), docTypeName, prdHandler, fbHandler)); controller.registerJobInMasterThread( trackJob(jobTrackers.getRemovedDocumentsPrune(), std::move(pruneRDjob))); if (!config.getLidSpaceCompactionConfig().isDisabled()) { diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.cpp b/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.cpp index 79135d27e5c..e6afb8a19b2 100644 --- a/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.cpp @@ -197,6 +197,19 @@ MaintenanceController::newConfig(const DocumentDBMaintenanceConfig::SP &config) restart(); } +namespace { + +void +assert_equal_meta_store_instances(const MaintenanceDocumentSubDB& old_db, + const MaintenanceDocumentSubDB& new_db) +{ + if (old_db.valid() && new_db.valid()) { + assert(old_db.meta_store().get() == new_db.meta_store().get()); + } +} + +} + void MaintenanceController::syncSubDBs(const MaintenanceDocumentSubDB &readySubDB, const MaintenanceDocumentSubDB &remSubDB, @@ -206,11 +219,16 @@ MaintenanceController::syncSubDBs(const MaintenanceDocumentSubDB &readySubDB, bool oldValid = _readySubDB.valid(); assert(readySubDB.valid()); assert(remSubDB.valid()); + // Document meta store instances should not change. Maintenance jobs depend on this fact. + assert_equal_meta_store_instances(_readySubDB, readySubDB); + assert_equal_meta_store_instances(_remSubDB, remSubDB); + assert_equal_meta_store_instances(_notReadySubDB, notReadySubDB); _readySubDB = readySubDB; _remSubDB = remSubDB; _notReadySubDB = notReadySubDB; - if (!oldValid && _started) + if (!oldValid && _started) { restart(); + } } diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenancedocumentsubdb.cpp b/searchcore/src/vespa/searchcore/proton/server/maintenancedocumentsubdb.cpp index ffaea1ff576..6b781eb8e0f 100644 --- a/searchcore/src/vespa/searchcore/proton/server/maintenancedocumentsubdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/maintenancedocumentsubdb.cpp @@ -5,26 +5,37 @@ namespace proton { MaintenanceDocumentSubDB::MaintenanceDocumentSubDB() - : _metaStore(), - _retriever(), - _subDbId(0u) -{ } + : _name(""), + _sub_db_id(0), + _meta_store(nullptr), + _retriever(nullptr), + _feed_view(nullptr) +{ +} -MaintenanceDocumentSubDB::~MaintenanceDocumentSubDB() { } +MaintenanceDocumentSubDB::~MaintenanceDocumentSubDB() = default; -MaintenanceDocumentSubDB::MaintenanceDocumentSubDB(const IDocumentMetaStore::SP & metaStore, - const IDocumentRetriever::SP & retriever, - uint32_t subDbId) - : _metaStore(metaStore), - _retriever(retriever), - _subDbId(subDbId) -{ } +MaintenanceDocumentSubDB::MaintenanceDocumentSubDB(const vespalib::string& name, + uint32_t sub_db_id, + IDocumentMetaStore::SP meta_store, + IDocumentRetriever::SP retriever, + IFeedView::SP feed_view) + : _name(name), + _sub_db_id(sub_db_id), + _meta_store(std::move(meta_store)), + _retriever(std::move(retriever)), + _feed_view(std::move(feed_view)) +{ +} void -MaintenanceDocumentSubDB::clear() { - _metaStore.reset(); +MaintenanceDocumentSubDB::clear() +{ + _name = ""; + _sub_db_id = 0; + _meta_store.reset(); _retriever.reset(); - _subDbId = 0u; -} + _feed_view.reset(); +} -} // namespace proton +} diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenancedocumentsubdb.h b/searchcore/src/vespa/searchcore/proton/server/maintenancedocumentsubdb.h index 41c43d910d7..bfe73f361b6 100644 --- a/searchcore/src/vespa/searchcore/proton/server/maintenancedocumentsubdb.h +++ b/searchcore/src/vespa/searchcore/proton/server/maintenancedocumentsubdb.h @@ -2,6 +2,7 @@ #pragma once +#include "ifeedview.h" #include <vespa/searchcore/proton/documentmetastore/i_document_meta_store.h> #include <vespa/searchcore/proton/persistenceengine/i_document_retriever.h> @@ -13,19 +14,30 @@ namespace proton { */ class MaintenanceDocumentSubDB { -public: - IDocumentMetaStore::SP _metaStore; - IDocumentRetriever::SP _retriever; - uint32_t _subDbId; +private: + vespalib::string _name; + uint32_t _sub_db_id; + IDocumentMetaStore::SP _meta_store; + IDocumentRetriever::SP _retriever; + IFeedView::SP _feed_view; +public: MaintenanceDocumentSubDB(); ~MaintenanceDocumentSubDB(); - MaintenanceDocumentSubDB(const IDocumentMetaStore::SP & metaStore, - const IDocumentRetriever::SP & retriever, - uint32_t subDbId); + MaintenanceDocumentSubDB(const vespalib::string& name, + uint32_t sub_db_id, + IDocumentMetaStore::SP meta_store, + IDocumentRetriever::SP retriever, + IFeedView::SP feed_view); + + const vespalib::string& name() const { return _name; } + uint32_t sub_db_id() const { return _sub_db_id; } + const IDocumentMetaStore::SP& meta_store() const { return _meta_store; } + const IDocumentRetriever::SP& retriever() const { return _retriever; } + const IFeedView::SP& feed_view() const { return _feed_view; } - bool valid() const { return bool(_metaStore); } + bool valid() const { return _meta_store.get() != nullptr; } void clear(); }; diff --git a/searchlib/CMakeLists.txt b/searchlib/CMakeLists.txt index bae47872d6c..f032bbe9c30 100644 --- a/searchlib/CMakeLists.txt +++ b/searchlib/CMakeLists.txt @@ -163,6 +163,7 @@ vespa_define_module( src/tests/hitcollector src/tests/index/docbuilder src/tests/index/doctypebuilder + src/tests/index/field_length_calculator src/tests/indexmetainfo src/tests/ld-library-path src/tests/memoryindex/compact_words_store diff --git a/searchlib/src/tests/attribute/imported_attribute_vector/imported_attribute_vector_test.cpp b/searchlib/src/tests/attribute/imported_attribute_vector/imported_attribute_vector_test.cpp index 1e24a784fd9..cfa05b7a765 100644 --- a/searchlib/src/tests/attribute/imported_attribute_vector/imported_attribute_vector_test.cpp +++ b/searchlib/src/tests/attribute/imported_attribute_vector/imported_attribute_vector_test.cpp @@ -1,8 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/eval/tensor/default_tensor.h> #include <vespa/eval/tensor/tensor.h> -#include <vespa/eval/tensor/tensor_factory.h> +#include <vespa/eval/tensor/default_tensor_engine.h> #include <vespa/searchcommon/attribute/search_context_params.h> #include <vespa/searchlib/fef/termfieldmatchdata.h> #include <vespa/searchlib/tensor/i_tensor_attribute.h> @@ -13,11 +12,17 @@ using search::attribute::IAttributeVector; using search::tensor::ITensorAttribute; using search::tensor::TensorAttribute; using vespalib::eval::ValueType; -using vespalib::tensor::DenseTensorCells; +using vespalib::eval::TensorSpec; using vespalib::tensor::Tensor; -using vespalib::tensor::TensorCells; -using vespalib::tensor::TensorDimensions; -using vespalib::tensor::TensorFactory; +using vespalib::tensor::DefaultTensorEngine; + +Tensor::UP createTensor(const TensorSpec &spec) { + auto value = DefaultTensorEngine::ref().from_spec(spec); + Tensor *tensor = dynamic_cast<Tensor*>(value.get()); + ASSERT_TRUE(tensor != nullptr); + value.release(); + return Tensor::UP(tensor); +} namespace search::attribute { @@ -482,32 +487,23 @@ TEST("onSerializeForDescendingSort() is forwarded with remapped LID to target ve } struct TensorAttrFixture : Fixture { - vespalib::tensor::DefaultTensor::builder builder; std::shared_ptr<Tensor> tensor1; std::shared_ptr<Tensor> tensor2; TensorAttrFixture(bool dense) : Fixture(), - builder(), tensor1(), tensor2() { setup(dense); } - Tensor::UP createTensor(const TensorCells &cells, - const TensorDimensions &dimensions) { - return TensorFactory::create(cells, dimensions, builder); - } - Tensor::UP createDenseTensor(const DenseTensorCells &cells) const { - return TensorFactory::createDense(cells); - } void setup(bool dense) { if (dense) { - tensor1 = createDenseTensor({ {{{"x",1}}, 11} }); - tensor2 = createDenseTensor({ {{{"x",0}}, 12}, {{{"x", 1}}, 0} }); + tensor1 = createTensor(TensorSpec("tensor(x[2])").add({{"x", 1}}, 11)); + tensor2 = createTensor(TensorSpec("tensor(x[2])").add({{"x", 0}}, 12).add({{"x", 1}}, 0)); } else { - tensor1 = createTensor({ {{{"x","1"}}, 11} }, { "x" }); - tensor2 = createTensor({ {{{"x","0"}}, 12} }, { "x" }); + tensor1 = createTensor(TensorSpec("tensor(x{})").add({{"x", "1"}}, 11)); + tensor2 = createTensor(TensorSpec("tensor(x{})").add({{"x", "0"}}, 12)); } const std::vector<ImportedAttributeFixture::LidToLidMapping<std::shared_ptr<Tensor>>> mappings = { {DocId(2), dummy_gid(3), DocId(3), tensor1 }, diff --git a/searchlib/src/tests/attribute/tensorattribute/tensorattribute_test.cpp b/searchlib/src/tests/attribute/tensorattribute/tensorattribute_test.cpp index cbbaa518b16..e0ec4a6fd06 100644 --- a/searchlib/src/tests/attribute/tensorattribute/tensorattribute_test.cpp +++ b/searchlib/src/tests/attribute/tensorattribute/tensorattribute_test.cpp @@ -6,8 +6,9 @@ #include <vespa/searchlib/tensor/generic_tensor_attribute.h> #include <vespa/searchlib/tensor/dense_tensor_attribute.h> #include <vespa/searchlib/attribute/attributeguard.h> -#include <vespa/eval/tensor/tensor_factory.h> -#include <vespa/eval/tensor/default_tensor.h> +#include <vespa/eval/tensor/tensor.h> +#include <vespa/eval/tensor/dense/dense_tensor.h> +#include <vespa/eval/tensor/default_tensor_engine.h> #include <vespa/vespalib/io/fileutil.h> #include <vespa/vespalib/data/fileheader.h> #include <vespa/fastos/file.h> @@ -21,11 +22,10 @@ using search::tensor::GenericTensorAttribute; using search::AttributeGuard; using search::AttributeVector; using vespalib::eval::ValueType; +using vespalib::eval::TensorSpec; using vespalib::tensor::Tensor; -using vespalib::tensor::TensorCells; -using vespalib::tensor::DenseTensorCells; -using vespalib::tensor::TensorDimensions; -using vespalib::tensor::TensorFactory; +using vespalib::tensor::DenseTensor; +using vespalib::tensor::DefaultTensorEngine; namespace vespalib { namespace tensor { @@ -41,6 +41,17 @@ static bool operator==(const Tensor &lhs, const Tensor &rhs) vespalib::string sparseSpec("tensor(x{},y{})"); vespalib::string denseSpec("tensor(x[2],y[3])"); +Tensor::UP createTensor(const TensorSpec &spec) { + auto value = DefaultTensorEngine::ref().from_spec(spec); + if (value->is_double()) { + return Tensor::UP(new DenseTensor(ValueType::double_type(), {value->as_double()})); + } + Tensor *tensor = dynamic_cast<Tensor*>(value.get()); + ASSERT_TRUE(tensor != nullptr); + value.release(); + return Tensor::UP(tensor); +} + struct Fixture { using BasicType = search::attribute::BasicType; @@ -52,7 +63,6 @@ struct Fixture vespalib::string _typeSpec; std::shared_ptr<TensorAttribute> _tensorAttr; std::shared_ptr<AttributeVector> _attr; - vespalib::tensor::DefaultTensor::builder _builder; bool _denseTensors; bool _useDenseTensorAttribute; @@ -63,7 +73,6 @@ struct Fixture _typeSpec(typeSpec), _tensorAttr(), _attr(), - _builder(), _denseTensors(false), _useDenseTensorAttribute(useDenseTensorAttribute) { @@ -85,17 +94,6 @@ struct Fixture } } - Tensor::UP createTensor(const TensorCells &cells) { - return TensorFactory::create(cells, _builder); - } - Tensor::UP createTensor(const TensorCells &cells, - const TensorDimensions &dimensions) { - return TensorFactory::create(cells, dimensions, _builder); - } - Tensor::UP createDenseTensor(const DenseTensorCells &cells) const { - return TensorFactory::createDense(cells); - } - void ensureSpace(uint32_t docId) { while (_attr->getNumDocs() <= docId) { uint32_t newDocId = 0u; @@ -138,19 +136,9 @@ struct Fixture } void - assertGetTensor(const TensorCells &expCells, - const TensorDimensions &expDimensions, - uint32_t docId) - { - Tensor::UP expTensor = createTensor(expCells, expDimensions); - assertGetTensor(*expTensor, docId); - } - - void - assertGetDenseTensor(const DenseTensorCells &expCells, - uint32_t docId) + assertGetTensor(const TensorSpec &expSpec, uint32_t docId) { - Tensor::UP expTensor = createDenseTensor(expCells); + Tensor::UP expTensor = createTensor(expSpec); assertGetTensor(*expTensor, docId); } @@ -166,72 +154,27 @@ struct Fixture EXPECT_TRUE(loadok); } - bool isUnbound(const vespalib::string &dimensionName) const - { - ValueType type = _cfg.tensorType(); - for (const auto &dim : type.dimensions()) { - if (dim.name == dimensionName && !dim.is_bound()) { - return true; - } - } - return false; - } - Tensor::UP expDenseTensor3() const { - if (isUnbound("x")) { - if (isUnbound("y")) { - return createDenseTensor({ {{{"x",0},{"y",1}}, 11} }); - } - return createDenseTensor({ {{{"x",0},{"y",1}}, 11}, - {{{"x",0},{"y",2}}, 0} }); - } else if (isUnbound("y")) { - return createDenseTensor({ {{{"x",0},{"y",1}}, 11}, - {{{"x",1},{"y",0}}, 0} }); - } - return createDenseTensor({ {{{"x",0},{"y",1}}, 11}, - {{{"x",1},{"y",2}}, 0} }); + return createTensor(TensorSpec(denseSpec) + .add({{"x", 0}, {"y", 1}}, 11) + .add({{"x", 1}, {"y", 2}}, 0)); } Tensor::UP expDenseFillTensor() const { - if (isUnbound("x")) { - if (isUnbound("y")) { - return createDenseTensor({ {{{"x",0},{"y",0}}, 5} }); - } - return createDenseTensor({ {{{"x",0},{"y",0}}, 5}, - {{{"x",0},{"y",2}}, 0} }); - } else if (isUnbound("y")) { - return createDenseTensor({ {{{"x",0},{"y",0}}, 5}, - {{{"x",1},{"y",0}}, 0} }); - } - return createDenseTensor({ {{{"x",0},{"y",0}}, 5}, - {{{"x",1},{"y",2}}, 0} }); + return createTensor(TensorSpec(denseSpec) + .add({{"x", 0}, {"y", 0}}, 5) + .add({{"x", 1}, {"y", 2}}, 0)); } Tensor::UP expEmptyDenseTensor() const { - if (isUnbound("x")) { - if (isUnbound("y")) { - return createDenseTensor({ {{{"x",0},{"y",0}}, 0} }); - } - return createDenseTensor({ {{{"x",0},{"y",2}}, 0} }); - } else if (isUnbound("y")) { - return createDenseTensor({ {{{"x",1},{"y",0}}, 0} }); - } - return createDenseTensor({ {{{"x",1},{"y",2}}, 0} }); + return createTensor(TensorSpec(denseSpec)); } vespalib::string expEmptyDenseTensorSpec() const { - if (isUnbound("x")) { - if (isUnbound("y")) { - return "tensor(x[1],y[1])"; - } - return "tensor(x[1],y[3])"; - } else if (isUnbound("y")) { - return "tensor(x[2],y[1])"; - } - return "tensor(x[2],y[3])"; + return denseSpec; } void testEmptyAttribute(); @@ -257,7 +200,7 @@ Fixture::testSetTensorValue() EXPECT_EQUAL(5u, _attr->getNumDocs()); EXPECT_EQUAL(5u, _attr->getCommittedDocIdLimit()); TEST_DO(assertGetNoTensor(4)); - EXPECT_EXCEPTION(setTensor(4, *createTensor({}, {})), + EXPECT_EXCEPTION(setTensor(4, *createTensor(TensorSpec("double"))), WrongTensorTypeException, "but other tensor type is 'double'"); TEST_DO(assertGetNoTensor(4)); @@ -267,9 +210,11 @@ Fixture::testSetTensorValue() setTensor(3, *expDenseTensor3()); TEST_DO(assertGetTensor(*expDenseTensor3(), 3)); } else { - TEST_DO(assertGetTensor({}, {"x", "y"}, 4)); - setTensor(3, *createTensor({ {{}, 11} }, { "x", "y"})); - TEST_DO(assertGetTensor({ {{}, 11} }, { "x", "y"}, 3)); + TEST_DO(assertGetTensor(TensorSpec(sparseSpec), 4)); + setTensor(3, *createTensor(TensorSpec(sparseSpec) + .add({{"x", ""}, {"y", ""}}, 11))); + TEST_DO(assertGetTensor(TensorSpec(sparseSpec) + .add({{"x", ""}, {"y", ""}}, 11), 3)); } TEST_DO(assertGetNoTensor(2)); TEST_DO(clearTensor(3)); @@ -284,7 +229,8 @@ Fixture::testSaveLoad() if (_denseTensors) { setTensor(3, *expDenseTensor3()); } else { - setTensor(3, *createTensor({ {{{"y","1"}}, 11} }, { "x", "y"})); + setTensor(3, *createTensor(TensorSpec(sparseSpec) + .add({{"x", ""}, {"y", "1"}}, 11))); } TEST_DO(save()); TEST_DO(load()); @@ -294,8 +240,9 @@ Fixture::testSaveLoad() TEST_DO(assertGetTensor(*expDenseTensor3(), 3)); TEST_DO(assertGetTensor(*expEmptyDenseTensor(), 4)); } else { - TEST_DO(assertGetTensor({ {{{"y","1"}}, 11} }, { "x", "y"}, 3)); - TEST_DO(assertGetTensor({}, {"x", "y"}, 4)); + TEST_DO(assertGetTensor(TensorSpec(sparseSpec) + .add({{"x", ""}, {"y", "1"}}, 11), 3)); + TEST_DO(assertGetTensor(TensorSpec(sparseSpec), 4)); } TEST_DO(assertGetNoTensor(2)); } @@ -310,9 +257,11 @@ Fixture::testCompaction() } ensureSpace(4); Tensor::UP emptytensor = _tensorAttr->getEmptyTensor(); - Tensor::UP emptyxytensor = createTensor({}, {"x", "y"}); - Tensor::UP simpletensor = createTensor({ {{{"y","1"}}, 11} }, { "x", "y"}); - Tensor::UP filltensor = createTensor({ {{}, 5} }, { "x", "y"}); + Tensor::UP emptyxytensor = createTensor(TensorSpec(sparseSpec)); + Tensor::UP simpletensor = createTensor(TensorSpec(sparseSpec) + .add({{"x", ""}, {"y", "1"}}, 11)); + Tensor::UP filltensor = createTensor(TensorSpec(sparseSpec) + .add({{"x", ""}, {"y", ""}}, 5)); if (_denseTensors) { emptyxytensor = expEmptyDenseTensor(); simpletensor = expDenseTensor3(); diff --git a/searchlib/src/tests/features/constant/constant_test.cpp b/searchlib/src/tests/features/constant/constant_test.cpp index f9d1736ba9e..e558839d2cf 100644 --- a/searchlib/src/tests/features/constant/constant_test.cpp +++ b/searchlib/src/tests/features/constant/constant_test.cpp @@ -7,9 +7,8 @@ #include <vespa/searchlib/fef/test/indexenvironment.h> #include <vespa/eval/eval/function.h> #include <vespa/eval/eval/tensor_spec.h> -#include <vespa/eval/tensor/default_tensor.h> +#include <vespa/eval/tensor/tensor.h> #include <vespa/eval/tensor/default_tensor_engine.h> -#include <vespa/eval/tensor/tensor_factory.h> using search::feature_t; using namespace search::fef; @@ -22,21 +21,11 @@ using vespalib::eval::DoubleValue; using vespalib::eval::TensorSpec; using vespalib::eval::ValueType; using vespalib::tensor::DefaultTensorEngine; -using vespalib::tensor::DenseTensorCells; using vespalib::tensor::Tensor; -using vespalib::tensor::TensorCells; -using vespalib::tensor::TensorDimensions; -using vespalib::tensor::TensorFactory; namespace { -Tensor::UP createTensor(const TensorCells &cells, - const TensorDimensions &dimensions) { - vespalib::tensor::DefaultTensor::builder builder; - return TensorFactory::create(cells, dimensions, builder); -} - Tensor::UP make_tensor(const TensorSpec &spec) { auto tensor = DefaultTensorEngine::ref().from_spec(spec); return Tensor::UP(dynamic_cast<Tensor*>(tensor.release())); @@ -72,10 +61,9 @@ struct ExecFixture return extractDouble(docId); } void addTensor(const vespalib::string &name, - const TensorCells &cells, - const TensorDimensions &dimensions) + const TensorSpec &spec) { - Tensor::UP tensor = createTensor(cells, dimensions); + Tensor::UP tensor = make_tensor(spec); ValueType type(tensor->type()); test.getIndexEnv().addConstantValue(name, std::move(type), @@ -100,10 +88,10 @@ TEST_F("require that existing tensor constant is detected", ExecFixture("constant(foo)")) { f.addTensor("foo", - { {{{"x", "a"}}, 3}, - {{{"x", "b"}}, 5}, - {{{"x", "c"}}, 7} }, - { "x" }); + TensorSpec("tensor(x{})") + .add({{"x","a"}}, 3) + .add({{"x","b"}}, 5) + .add({{"x","c"}}, 7)); EXPECT_TRUE(f.setup()); auto expect = make_tensor(TensorSpec("tensor(x{})") .add({{"x","b"}}, 5) diff --git a/searchlib/src/tests/features/tensor/tensor_test.cpp b/searchlib/src/tests/features/tensor/tensor_test.cpp index abe54906791..3aa2671fd25 100644 --- a/searchlib/src/tests/features/tensor/tensor_test.cpp +++ b/searchlib/src/tests/features/tensor/tensor_test.cpp @@ -15,7 +15,6 @@ #include <vespa/eval/tensor/default_tensor.h> #include <vespa/eval/tensor/default_tensor_engine.h> #include <vespa/eval/tensor/serialization/typed_binary_format.h> -#include <vespa/eval/tensor/tensor_factory.h> #include <vespa/eval/tensor/test/test_utils.h> #include <vespa/vespalib/objects/nbostream.h> @@ -36,7 +35,6 @@ using vespalib::tensor::DenseTensorCells; using vespalib::tensor::Tensor; using vespalib::tensor::TensorCells; using vespalib::tensor::TensorDimensions; -using vespalib::tensor::TensorFactory; using vespalib::tensor::test::makeTensor; using AVC = search::attribute::Config; diff --git a/searchlib/src/tests/index/field_length_calculator/CMakeLists.txt b/searchlib/src/tests/index/field_length_calculator/CMakeLists.txt new file mode 100644 index 00000000000..df09d0abaa7 --- /dev/null +++ b/searchlib/src/tests/index/field_length_calculator/CMakeLists.txt @@ -0,0 +1,9 @@ +# Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(searchlib_field_length_calculator_test_app TEST + SOURCES + field_length_calculator_test.cpp + DEPENDS + searchlib + gtest +) +vespa_add_test(NAME searchlib_field_length_calculator_test_app COMMAND searchlib_field_length_calculator_test_app) diff --git a/searchlib/src/tests/index/field_length_calculator/field_length_calculator_test.cpp b/searchlib/src/tests/index/field_length_calculator/field_length_calculator_test.cpp new file mode 100644 index 00000000000..c99d241cbc0 --- /dev/null +++ b/searchlib/src/tests/index/field_length_calculator/field_length_calculator_test.cpp @@ -0,0 +1,68 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/searchlib/index/field_length_calculator.h> +#include <vespa/vespalib/gtest/gtest.h> + +using search::index::FieldLengthCalculator; + +namespace search::index { + +namespace { + +// Arithmetic average of arithmetic sequence 1, 2, ... , samples +double arith_avg(uint32_t samples) { + return static_cast<double>(samples + 1) / 2; +} + +} + +TEST(FieldLengthCalculatorTest, empty_is_zero) +{ + FieldLengthCalculator calc; + EXPECT_EQ(0.0, calc.get_average_field_length()); + EXPECT_EQ(0, calc.get_num_samples()); +} + +TEST(FieldLengthCalculatorTest, startup_is_average) +{ + FieldLengthCalculator calc; + calc.add_field_length(3); + EXPECT_DOUBLE_EQ(3.0, calc.get_average_field_length()); + EXPECT_EQ(1, calc.get_num_samples()); + calc.add_field_length(4); + EXPECT_DOUBLE_EQ(3.5, calc.get_average_field_length()); + EXPECT_EQ(2, calc.get_num_samples()); + calc.add_field_length(7); + EXPECT_DOUBLE_EQ((3 + 4 + 7)/3.0, calc.get_average_field_length()); + EXPECT_EQ(3, calc.get_num_samples()); + calc.add_field_length(9); + EXPECT_DOUBLE_EQ((3 + 4 + 7 + 9)/4.0, calc.get_average_field_length()); + EXPECT_EQ(4, calc.get_num_samples()); +} + +TEST(FieldLengthCalculatorTest, average_until_max_num_samples) +{ + const uint32_t max_num_samples = 5; + FieldLengthCalculator calc(0.0, 0, max_num_samples); + static constexpr double epsilon = 0.000000001; // Allowed difference + for (uint32_t i = 0; i + 1 < max_num_samples; ++i) { + calc.add_field_length(i + 1); + } + // Arithmetic average + EXPECT_NEAR(arith_avg(max_num_samples - 1), calc.get_average_field_length(), epsilon); + EXPECT_EQ(max_num_samples - 1, calc.get_num_samples()); + calc.add_field_length(max_num_samples); + // Arithmetic average + EXPECT_NEAR(arith_avg(max_num_samples), calc.get_average_field_length(), epsilon); + EXPECT_EQ(max_num_samples, calc.get_num_samples()); + calc.add_field_length(max_num_samples + 1); + // No longer arithmetic average + EXPECT_LT(arith_avg(max_num_samples + 1), calc.get_average_field_length()); + // Switched to exponential decay + EXPECT_NEAR((arith_avg(max_num_samples) * (max_num_samples - 1) + max_num_samples + 1) / max_num_samples, calc.get_average_field_length(), epsilon); + EXPECT_EQ(max_num_samples, calc.get_num_samples()); +} + +} + +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/memoryindex/field_inverter/field_inverter_test.cpp b/searchlib/src/tests/memoryindex/field_inverter/field_inverter_test.cpp index f08e61b0da2..a818bb75bf2 100644 --- a/searchlib/src/tests/memoryindex/field_inverter/field_inverter_test.cpp +++ b/searchlib/src/tests/memoryindex/field_inverter/field_inverter_test.cpp @@ -98,6 +98,16 @@ makeDoc16(DocBuilder &b) return b.endDocument(); } +Document::UP +makeDoc17(DocBuilder &b) +{ + b.startDocument("doc::17"); + b.startIndexField("f1").addStr("foo0").addStr("bar0").endField(); + b.startIndexField("f2").startElement(1).addStr("foo").addStr("bar").endElement().startElement(1).addStr("bar").endElement().endField(); + b.startIndexField("f3").startElement(3).addStr("foo2").addStr("bar2").endElement().startElement(4).addStr("bar2").endElement().endField(); + return b.endDocument(); +} + } struct Fixture @@ -323,6 +333,23 @@ TEST_F("require that multiple words at same position works", Fixture) f._inserter.toStr()); } +TEST_F("require that cheap features are calculated", Fixture) +{ + f.invertDocument(17, *makeDoc17(f._b)); + f._inserter.setVerbose(); + f._inserter.set_show_cheap_features(); + f.pushDocuments(); + EXPECT_EQUAL("f=1," + "w=bar0,a=17(fl=2,occs=1,e=0,w=1,l=2[1])," + "w=foo0,a=17(fl=2,occs=1,e=0,w=1,l=2[0])," + "f=2," + "w=bar,a=17(fl=3,occs=2,e=0,w=1,l=2[1],e=1,w=1,l=1[0])," + "w=foo,a=17(fl=3,occs=1,e=0,w=1,l=2[0])," + "f=3," + "w=bar2,a=17(fl=3,occs=2,e=0,w=3,l=2[1],e=1,w=4,l=1[0])," + "w=foo2,a=17(fl=3,occs=1,e=0,w=3,l=2[0])", + f._inserter.toStr()); +} } // namespace memoryindex } // namespace search diff --git a/searchlib/src/vespa/searchlib/index/field_length_calculator.h b/searchlib/src/vespa/searchlib/index/field_length_calculator.h new file mode 100644 index 00000000000..50d47ced063 --- /dev/null +++ b/searchlib/src/vespa/searchlib/index/field_length_calculator.h @@ -0,0 +1,42 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <atomic> +#include <algorithm> + +namespace search::index { + +/** + * Class used to calculate average field length, with a bias towards + * the latest field lengths when max_num_samples samples have been reached. + */ +class FieldLengthCalculator { + std::atomic<double> _average_field_length; + uint32_t _num_samples; // Capped by _max_num_samples + uint32_t _max_num_samples; + +public: + FieldLengthCalculator() + : FieldLengthCalculator(0.0, 0) + { + } + + FieldLengthCalculator(double average_field_length, uint32_t num_samples, uint32_t max_num_samples = 100000) + : _average_field_length(average_field_length), + _num_samples(std::min(num_samples, max_num_samples)), + _max_num_samples(max_num_samples) + { + } + + double get_average_field_length() const { return _average_field_length.load(std::memory_order_relaxed); } + uint32_t get_num_samples() const { return _num_samples; } + uint32_t get_max_num_samples() { return _max_num_samples; } + + void add_field_length(uint32_t field_length) { + if (_num_samples < _max_num_samples) { + ++_num_samples; + } + _average_field_length.store((_average_field_length.load(std::memory_order_relaxed) * (_num_samples - 1) + field_length) / _num_samples, std::memory_order_relaxed); + } +}; + +} diff --git a/searchlib/src/vespa/searchlib/memoryindex/field_inverter.cpp b/searchlib/src/vespa/searchlib/memoryindex/field_inverter.cpp index d19f05a98ee..bfa0143d395 100644 --- a/searchlib/src/vespa/searchlib/memoryindex/field_inverter.cpp +++ b/searchlib/src/vespa/searchlib/memoryindex/field_inverter.cpp @@ -279,6 +279,29 @@ FieldInverter::remove(const vespalib::stringref word, uint32_t docId) } void +FieldInverter::endDoc() +{ + uint32_t field_length = 0; + if (_elem > 0) { + auto itr = _elems.end() - _elem; + while (itr != _elems.end()) { + field_length += itr->_len; + ++itr; + } + itr = _elems.end() - _elem; + while (itr != _elems.end()) { + itr->set_field_length(field_length); + ++itr; + } + } + uint32_t newPosSize = static_cast<uint32_t>(_positions.size()); + _pendingDocs.insert({ _docId, + { _oldPosSize, newPosSize - _oldPosSize } }); + _docId = 0; + _oldPosSize = newPosSize; +} + +void FieldInverter::processNormalDocTextField(const StringFieldValue &field) { startElement(1); @@ -500,6 +523,7 @@ FieldInverter::pushDocuments(IOrderedFieldIndexInserter &inserter) (void) numWordIds; if (lastWordNum != i._wordNum || lastDocId != i._docId) { if (!emptyFeatures) { + _features.set_num_occs(_features.word_positions().size()); inserter.add(lastDocId, _features); emptyFeatures = true; } @@ -520,6 +544,8 @@ FieldInverter::pushDocuments(IOrderedFieldIndexInserter &inserter) _features.clear(lastDocId); lastElemId = NO_ELEMENT_ID; lastWordPos = NO_WORD_POS; + const ElemInfo &elem = _elems[i._elemRef]; + _features.set_field_length(elem.get_field_length()); } else { continue; // ignore dup remove } @@ -539,6 +565,7 @@ FieldInverter::pushDocuments(IOrderedFieldIndexInserter &inserter) } if (!emptyFeatures) { + _features.set_num_occs(_features.word_positions().size()); inserter.add(lastDocId, _features); } inserter.flush(); diff --git a/searchlib/src/vespa/searchlib/memoryindex/field_inverter.h b/searchlib/src/vespa/searchlib/memoryindex/field_inverter.h index ba6a0e96698..e547dbe98c6 100644 --- a/searchlib/src/vespa/searchlib/memoryindex/field_inverter.h +++ b/searchlib/src/vespa/searchlib/memoryindex/field_inverter.h @@ -99,14 +99,18 @@ private: public: int32_t _weight; uint32_t _len; + uint32_t _field_length; ElemInfo(int32_t weight) : _weight(weight), - _len(0u) + _len(0u), + _field_length(0u) { } void setLen(uint32_t len) { _len = len; } + uint32_t get_field_length() const { return _field_length; } + void set_field_length(uint32_t field_length) { _field_length = field_length; } }; using ElemInfoVec = std::vector<ElemInfo>; @@ -317,13 +321,7 @@ public: _wpos = 0; } - void endDoc() { - uint32_t newPosSize = static_cast<uint32_t>(_positions.size()); - _pendingDocs.insert({ _docId, - { _oldPosSize, newPosSize - _oldPosSize } }); - _docId = 0; - _oldPosSize = newPosSize; - } + void endDoc(); void addWord(const vespalib::stringref word) { uint32_t wordRef = saveWord(word); diff --git a/searchlib/src/vespa/searchlib/test/memoryindex/ordered_field_index_inserter.h b/searchlib/src/vespa/searchlib/test/memoryindex/ordered_field_index_inserter.h index a341e36045e..f984bd8fcbd 100644 --- a/searchlib/src/vespa/searchlib/test/memoryindex/ordered_field_index_inserter.h +++ b/searchlib/src/vespa/searchlib/test/memoryindex/ordered_field_index_inserter.h @@ -11,6 +11,7 @@ class OrderedFieldIndexInserter : public IOrderedFieldIndexInserter { std::stringstream _ss; bool _first; bool _verbose; + bool _show_cheap_features; uint32_t _fieldId; void @@ -27,6 +28,7 @@ public: : _ss(), _first(true), _verbose(false), + _show_cheap_features(false), _fieldId(0) { } @@ -55,6 +57,11 @@ public: _ss << "("; auto wpi = features.word_positions().begin(); bool firstElement = true; + if (_show_cheap_features) { + _ss << "fl=" << features.field_length() << + ",occs=" << features.num_occs(); + firstElement = false; + } for (auto &el : features.elements()) { if (!firstElement) { _ss << ","; @@ -70,6 +77,7 @@ public: } firstWordPos = false; _ss << wpi->getWordPos(); + ++wpi; } _ss << "]"; } @@ -105,6 +113,7 @@ public: } void setVerbose() { _verbose = true; } + void set_show_cheap_features() { _show_cheap_features = true; } }; } diff --git a/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.cpp b/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.cpp index 72cedb05f7c..1cac789ad21 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.cpp @@ -31,7 +31,6 @@ #include <vespa/vespalib/stllike/asciistream.h> #include <vespa/vespalib/util/stringfmt.h> #include <vespa/vespalib/data/slime/slime.h> -#include <vespa/eval/tensor/serialization/slime_binary_format.h> #include <vespa/eval/tensor/serialization/typed_binary_format.h> #include <vespa/vespalib/objects/nbostream.h> #include <vespa/searchlib/util/slime_output_raw_buf_adapter.h> diff --git a/vespajlib/abi-spec.json b/vespajlib/abi-spec.json index c31eed32830..04e68e60178 100644 --- a/vespajlib/abi-spec.json +++ b/vespajlib/abi-spec.json @@ -788,7 +788,11 @@ ], "methods": [ "public static com.yahoo.tensor.IndexedTensor$Builder of(com.yahoo.tensor.TensorType)", + "public static com.yahoo.tensor.IndexedTensor$Builder of(com.yahoo.tensor.TensorType, float[])", + "public static com.yahoo.tensor.IndexedTensor$Builder of(com.yahoo.tensor.TensorType, double[])", "public static com.yahoo.tensor.IndexedTensor$Builder of(com.yahoo.tensor.TensorType, com.yahoo.tensor.DimensionSizes)", + "public static com.yahoo.tensor.IndexedTensor$Builder of(com.yahoo.tensor.TensorType, com.yahoo.tensor.DimensionSizes, float[])", + "public static com.yahoo.tensor.IndexedTensor$Builder of(com.yahoo.tensor.TensorType, com.yahoo.tensor.DimensionSizes, double[])", "public varargs abstract com.yahoo.tensor.IndexedTensor$Builder cell(double, long[])", "public varargs abstract com.yahoo.tensor.IndexedTensor$Builder cell(float, long[])", "public com.yahoo.tensor.TensorType type()", diff --git a/vespajlib/src/main/java/com/yahoo/tensor/IndexedDoubleTensor.java b/vespajlib/src/main/java/com/yahoo/tensor/IndexedDoubleTensor.java index 285837a1bc6..e644244178d 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/IndexedDoubleTensor.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/IndexedDoubleTensor.java @@ -43,8 +43,12 @@ class IndexedDoubleTensor extends IndexedTensor { private double[] values; BoundDoubleBuilder(TensorType type, DimensionSizes sizes) { + this(type, sizes, new double[(int)sizes.totalSize()]); + } + + BoundDoubleBuilder(TensorType type, DimensionSizes sizes, double[] values) { super(type, sizes); - values = new double[(int)sizes.totalSize()]; + this.values = values; } @Override diff --git a/vespajlib/src/main/java/com/yahoo/tensor/IndexedFloatTensor.java b/vespajlib/src/main/java/com/yahoo/tensor/IndexedFloatTensor.java index 8f8c24c8421..30157d9791a 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/IndexedFloatTensor.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/IndexedFloatTensor.java @@ -43,8 +43,16 @@ class IndexedFloatTensor extends IndexedTensor { private float[] values; BoundFloatBuilder(TensorType type, DimensionSizes sizes) { + this(type, sizes, new float[(int)sizes.totalSize()]); + } + + BoundFloatBuilder(TensorType type, DimensionSizes sizes, float[] values) { super(type, sizes); - values = new float[(int)sizes.totalSize()]; + if (sizes.totalSize() != values.length) { + throw new IllegalArgumentException("Invalid size(" + values.length + ") of supplied value vector." + + " Type specifies that size should be " + sizes.totalSize()); + } + this.values = values; } @Override diff --git a/vespajlib/src/main/java/com/yahoo/tensor/IndexedTensor.java b/vespajlib/src/main/java/com/yahoo/tensor/IndexedTensor.java index 19edfc0269e..a03131f3ec9 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/IndexedTensor.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/IndexedTensor.java @@ -236,29 +236,106 @@ public abstract class IndexedTensor implements Tensor { } /** + * Creates a builder initialized with the given values + * + * @param type the type of the tensor to build + * @param values the initial values of the tensor. This <b>transfers ownership</b> of the value array - it + * must not be further mutated by the caller + */ + public static Builder of(TensorType type, float[] values) { + if (type.dimensions().stream().allMatch(d -> d instanceof TensorType.IndexedBoundDimension)) + return of(type, BoundBuilder.dimensionSizesOf(type), values); + else + return new UnboundBuilder(type); + } + + /** + * Creates a builder initialized with the given values + * + * @param type the type of the tensor to build + * @param values the initial values of the tensor. This <b>transfers ownership</b> of the value array - it + * must not be further mutated by the caller + */ + public static Builder of(TensorType type, double[] values) { + if (type.dimensions().stream().allMatch(d -> d instanceof TensorType.IndexedBoundDimension)) + return of(type, BoundBuilder.dimensionSizesOf(type), values); + else + return new UnboundBuilder(type); + } + + /** * Create a builder with dimension size information for this instance. Must be one size entry per dimension, * and, agree with the type size information when specified in the type. * If sizes are completely specified in the type this size information is redundant. */ public static Builder of(TensorType type, DimensionSizes sizes) { + validate(type, sizes); + + if (type.valueType() == TensorType.Value.FLOAT) + return new IndexedFloatTensor.BoundFloatBuilder(type, sizes); + else if (type.valueType() == TensorType.Value.DOUBLE) + return new IndexedDoubleTensor.BoundDoubleBuilder(type, sizes); + else + return new IndexedDoubleTensor.BoundDoubleBuilder(type, sizes); // Default + } + + /** + * Creates a builder initialized with the given values + * + * @param type the type of the tensor to build + * @param values the initial values of the tensor. This <b>transfers ownership</b> of the value array - it + * must not be further mutated by the caller + */ + public static Builder of(TensorType type, DimensionSizes sizes, float[] values) { + validate(type, sizes); + validateSizes(sizes, values.length); + + if (type.valueType() == TensorType.Value.FLOAT) + return new IndexedFloatTensor.BoundFloatBuilder(type, sizes, values); + else if (type.valueType() == TensorType.Value.DOUBLE) + return new IndexedDoubleTensor.BoundDoubleBuilder(type, sizes).fill(values); + else + return new IndexedDoubleTensor.BoundDoubleBuilder(type, sizes).fill(values); // Default + } + + /** + * Creates a builder initialized with the given values + * + * @param type the type of the tensor to build + * @param values the initial values of the tensor. This <b>transfers ownership</b> of the value array - it + * must not be further mutated by the caller + */ + public static Builder of(TensorType type, DimensionSizes sizes, double[] values) { + validate(type, sizes); + validateSizes(sizes, values.length); + + if (type.valueType() == TensorType.Value.FLOAT) + return new IndexedFloatTensor.BoundFloatBuilder(type, sizes).fill(values); + else if (type.valueType() == TensorType.Value.DOUBLE) + return new IndexedDoubleTensor.BoundDoubleBuilder(type, sizes, values); + else + return new IndexedDoubleTensor.BoundDoubleBuilder(type, sizes, values); // Default + } + + private static void validateSizes(DimensionSizes sizes, int length) { + if (sizes.totalSize() != length) { + throw new IllegalArgumentException("Invalid size(" + length + ") of supplied value vector." + + " Type specifies that size should be " + sizes.totalSize()); + } + } + + private static void validate(TensorType type, DimensionSizes sizes) { // validate if (sizes.dimensions() != type.dimensions().size()) throw new IllegalArgumentException(sizes.dimensions() + - " is the wrong number of dimensions for " + type); + " is the wrong number of dimensions for " + type); for (int i = 0; i < sizes.dimensions(); i++ ) { Optional<Long> size = type.dimensions().get(i).size(); if (size.isPresent() && size.get() < sizes.size(i)) throw new IllegalArgumentException("Size of dimension " + type.dimensions().get(i).name() + " is " + - sizes.size(i) + - " but cannot be larger than " + size.get() + " in " + type); + sizes.size(i) + + " but cannot be larger than " + size.get() + " in " + type); } - - if (type.valueType() == TensorType.Value.FLOAT) - return new IndexedFloatTensor.BoundFloatBuilder(type, sizes); - else if (type.valueType() == TensorType.Value.DOUBLE) - return new IndexedDoubleTensor.BoundDoubleBuilder(type, sizes); - else - return new IndexedDoubleTensor.BoundDoubleBuilder(type, sizes); // Default } public abstract Builder cell(double value, long ... indexes); @@ -290,6 +367,20 @@ public abstract class IndexedTensor implements Tensor { throw new IllegalArgumentException("Must have a dimension size entry for each dimension in " + type); this.sizes = sizes; } + BoundBuilder fill(float [] values) { + long index = 0; + for (float value : values) { + cellByDirectIndex(index++, value); + } + return this; + } + BoundBuilder fill(double [] values) { + long index = 0; + for (double value : values) { + cellByDirectIndex(index++, value); + } + return this; + } DimensionSizes sizes() { return sizes; } diff --git a/vespajlib/src/test/java/com/yahoo/tensor/IndexedTensorTestCase.java b/vespajlib/src/test/java/com/yahoo/tensor/IndexedTensorTestCase.java index a5fc3d5a5d8..4bfdb53e321 100644 --- a/vespajlib/src/test/java/com/yahoo/tensor/IndexedTensorTestCase.java +++ b/vespajlib/src/test/java/com/yahoo/tensor/IndexedTensorTestCase.java @@ -10,6 +10,7 @@ import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * @author bratseth @@ -41,6 +42,37 @@ public class IndexedTensorTestCase { assertTrue(singleValueFromString instanceof IndexedTensor); assertEquals(singleValue, singleValueFromString); } + + private void verifyFloat(String spec) { + float [] floats = {1.0f, 2.0f, 3.0f}; + Tensor tensor = IndexedTensor.Builder.of(TensorType.fromSpec(spec), floats).build(); + int index = 0; + for (Double cell : tensor.cells().values()) { + assertEquals(cell, Double.valueOf(floats[index++])); + } + } + private void verifyDouble(String spec) { + double [] values = {1.0, 2.0, 3.0}; + Tensor tensor = IndexedTensor.Builder.of(TensorType.fromSpec(spec), values).build(); + int index = 0; + for (Double cell : tensor.cells().values()) { + assertEquals(cell, Double.valueOf(values[index++])); + } + } + + @Test + public void testBoundHandoverBuilding() { + verifyFloat("tensor<float>(x[3])"); + verifyDouble("tensor<float>(x[3])"); + verifyFloat("tensor<double>(x[3])"); + verifyDouble("tensor<double>(x[3])"); + try { + verifyDouble("tensor<double>(x[4])"); + fail("Expect IllegalArgumentException"); + } catch (IllegalArgumentException e) { + assertEquals("Invalid size(3) of supplied value vector. Type specifies that size should be 4", e.getMessage()); + } + } @Test public void testBoundBuilding() { diff --git a/vespalib/src/tests/util/rcuvector/CMakeLists.txt b/vespalib/src/tests/util/rcuvector/CMakeLists.txt index 77900b8422d..8aa07763bef 100644 --- a/vespalib/src/tests/util/rcuvector/CMakeLists.txt +++ b/vespalib/src/tests/util/rcuvector/CMakeLists.txt @@ -3,6 +3,6 @@ vespa_add_executable(vespalib_rcuvector_test_app TEST SOURCES rcuvector_test.cpp DEPENDS - searchlib + vespalib ) vespa_add_test(NAME vespalib_rcuvector_test_app COMMAND vespalib_rcuvector_test_app) |