diff options
122 files changed, 1413 insertions, 1109 deletions
diff --git a/README.md b/README.md index 37ceb44ccfd..97c4bf69bc3 100644 --- a/README.md +++ b/README.md @@ -28,14 +28,15 @@ You can also setup CentOS 7 natively and install the following build dependencie ### Build Java modules export MAVEN_OPTS="-Xms128m -Xmx512m" - sh bootstrap.sh java + source /opt/rh/rh-maven33/enable + bash bootstrap.sh java mvn -T <num-threads> install ### Build C++ modules Replace `<build-dir>` with the name of the directory in which you'd like to build Vespa. Replace `<source-dir>` with the directory in which you've cloned/unpacked the source tree. - sh bootstrap-cpp.sh <source-dir> <build-dir> + bash bootstrap-cpp.sh <source-dir> <build-dir> cd <build-dir> make -j <num-threads> ctest3 -j <num-threads> diff --git a/athenz-identity-provider-service/pom.xml b/athenz-identity-provider-service/pom.xml index bb82e40f827..26e24be526c 100644 --- a/athenz-identity-provider-service/pom.xml +++ b/athenz-identity-provider-service/pom.xml @@ -82,6 +82,12 @@ <version>${project.version}</version> <scope>provided</scope> </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>config-model-api</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + </dependency> <!-- TEST --> <dependency> diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzInstanceProviderService.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzInstanceProviderService.java index 74b697fb004..26a88896fb9 100644 --- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzInstanceProviderService.java +++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzInstanceProviderService.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.athenz.instanceproviderservice; import com.google.inject.Inject; import com.yahoo.component.AbstractComponent; +import com.yahoo.config.model.api.SuperModelProvider; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; import com.yahoo.jdisc.http.SecretStore; @@ -48,25 +49,39 @@ public class AthenzInstanceProviderService extends AbstractComponent { private final Server jetty; @Inject - public AthenzInstanceProviderService(AthenzProviderServiceConfig config, NodeRepository nodeRepository, Zone zone, SecretStore secretStore) { + public AthenzInstanceProviderService(AthenzProviderServiceConfig config, SuperModelProvider superModelProvider, + NodeRepository nodeRepository, Zone zone, SecretStore secretStore) { this(config, new SecretStoreKeyProvider(secretStore, getZoneConfig(config, zone).secretName()), Executors.newSingleThreadScheduledExecutor(), - nodeRepository, zone, new AthenzCertificateClient(config, getZoneConfig(config, zone))); + superModelProvider, nodeRepository, zone, new AthenzCertificateClient(config, getZoneConfig(config, zone)), createSslContextFactory()); + } + + private AthenzInstanceProviderService(AthenzProviderServiceConfig config, + KeyProvider keyProvider, + ScheduledExecutorService scheduler, + SuperModelProvider superModelProvider, + NodeRepository nodeRepository, + Zone zone, + CertificateClient certificateClient, + SslContextFactory sslContextFactory) { + this(config, scheduler, zone, sslContextFactory, + new InstanceValidator(keyProvider, superModelProvider), + new IdentityDocumentGenerator(config, getZoneConfig(config, zone), nodeRepository, zone, keyProvider), + new AthenzCertificateUpdater( + certificateClient, sslContextFactory, keyProvider, config, getZoneConfig(config, zone))); } AthenzInstanceProviderService(AthenzProviderServiceConfig config, - KeyProvider keyProvider, ScheduledExecutorService scheduler, - NodeRepository nodeRepository, Zone zone, - CertificateClient certificateClient) { + SslContextFactory sslContextFactory, + InstanceValidator instanceValidator, + IdentityDocumentGenerator identityDocumentGenerator, + AthenzCertificateUpdater reloader) { // TODO: Enable for all systems. Currently enabled for CD system only if (SystemName.cd.equals(zone.system())) { this.scheduler = scheduler; - SslContextFactory sslContextFactory = createSslContextFactory(); - this.jetty = createJettyServer( - config, keyProvider, sslContextFactory, nodeRepository, zone); - AthenzCertificateUpdater reloader = - new AthenzCertificateUpdater(certificateClient, sslContextFactory, keyProvider, config, getZoneConfig(config, zone)); + this.jetty = createJettyServer(config, sslContextFactory, instanceValidator, identityDocumentGenerator); + // TODO Configurable update frequency scheduler.scheduleAtFixedRate(reloader, 0, 1, TimeUnit.DAYS); try { @@ -81,22 +96,19 @@ public class AthenzInstanceProviderService extends AbstractComponent { } private static Server createJettyServer(AthenzProviderServiceConfig config, - KeyProvider keyProvider, SslContextFactory sslContextFactory, - NodeRepository nodeRepository, - Zone zone) { + InstanceValidator instanceValidator, + IdentityDocumentGenerator identityDocumentGenerator) { Server server = new Server(); ServerConnector connector = new ServerConnector(server, sslContextFactory); connector.setPort(config.port()); server.addConnector(connector); ServletHandler handler = new ServletHandler(); - InstanceConfirmationServlet instanceConfirmationServlet = - new InstanceConfirmationServlet(new InstanceValidator(keyProvider)); + InstanceConfirmationServlet instanceConfirmationServlet = new InstanceConfirmationServlet(instanceValidator); handler.addServletWithMapping(new ServletHolder(instanceConfirmationServlet), config.apiPath() + "/instance"); - IdentityDocumentServlet identityDocumentServlet = - new IdentityDocumentServlet(new IdentityDocumentGenerator(config, getZoneConfig(config, zone), nodeRepository, zone, keyProvider)); + IdentityDocumentServlet identityDocumentServlet = new IdentityDocumentServlet(identityDocumentGenerator); handler.addServletWithMapping(new ServletHolder(identityDocumentServlet), config.apiPath() + "/identity-document"); handler.addServletWithMapping(StatusServlet.class, "/status.html"); @@ -110,7 +122,7 @@ public class AthenzInstanceProviderService extends AbstractComponent { return config.zones(key); } - private static SslContextFactory createSslContextFactory() { + static SslContextFactory createSslContextFactory() { try { SslContextFactory sslContextFactory = new SslContextFactory(); sslContextFactory.setWantClientAuth(true); @@ -122,7 +134,7 @@ public class AthenzInstanceProviderService extends AbstractComponent { } } - private static class AthenzCertificateUpdater implements Runnable { + static class AthenzCertificateUpdater implements Runnable { // TODO Make expiry a configuration parameter private static final TemporalAmount EXPIRY_TIME = Duration.ofDays(30); @@ -134,11 +146,11 @@ public class AthenzInstanceProviderService extends AbstractComponent { private final AthenzProviderServiceConfig config; private final AthenzProviderServiceConfig.Zones zoneConfig; - private AthenzCertificateUpdater(CertificateClient certificateClient, - SslContextFactory sslContextFactory, - KeyProvider keyProvider, - AthenzProviderServiceConfig config, - AthenzProviderServiceConfig.Zones zoneConfig) { + AthenzCertificateUpdater(CertificateClient certificateClient, + SslContextFactory sslContextFactory, + KeyProvider keyProvider, + AthenzProviderServiceConfig config, + AthenzProviderServiceConfig.Zones zoneConfig) { this.certificateClient = certificateClient; this.sslContextFactory = sslContextFactory; this.keyProvider = keyProvider; @@ -179,7 +191,7 @@ public class AthenzInstanceProviderService extends AbstractComponent { log.log(LogLevel.INFO, "Deconstructing Athenz provider service"); if(scheduler != null) scheduler.shutdown(); - if(jetty !=null) + if(jetty != null) jetty.stop(); if (scheduler != null && !scheduler.awaitTermination(1, TimeUnit.MINUTES)) { log.log(LogLevel.ERROR, "Failed to stop certificate updater"); diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/impl/InstanceValidator.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/impl/InstanceValidator.java index 8d76300c2bb..427f35c41d8 100644 --- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/impl/InstanceValidator.java +++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/impl/InstanceValidator.java @@ -1,6 +1,10 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl; +import com.yahoo.config.model.api.ApplicationInfo; +import com.yahoo.config.model.api.ServiceInfo; +import com.yahoo.config.model.api.SuperModelProvider; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.log.LogLevel; import com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.model.InstanceConfirmation; import com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.model.ProviderUniqueId; @@ -12,6 +16,7 @@ import java.security.PublicKey; import java.security.Signature; import java.security.SignatureException; import java.util.Base64; +import java.util.Optional; import java.util.logging.Logger; /** @@ -22,19 +27,29 @@ import java.util.logging.Logger; public class InstanceValidator { private static final Logger log = Logger.getLogger(InstanceValidator.class.getName()); + static final String SERVICE_PROPERTIES_DOMAIN_KEY = "identity.domain"; + static final String SERVICE_PROPERTIES_SERVICE_KEY = "identity.service"; private final KeyProvider keyProvider; + private final SuperModelProvider superModelProvider; - public InstanceValidator(KeyProvider keyProvider) { + public InstanceValidator(KeyProvider keyProvider, SuperModelProvider superModelProvider) { this.keyProvider = keyProvider; + this.superModelProvider = superModelProvider; } public boolean isValidInstance(InstanceConfirmation instanceConfirmation) { SignedIdentityDocument signedIdentityDocument = instanceConfirmation.signedIdentityDocument; ProviderUniqueId providerUniqueId = signedIdentityDocument.identityDocument.providerUniqueId; + ApplicationId applicationId = ApplicationId.from( + providerUniqueId.tenant, providerUniqueId.application, providerUniqueId.instance); + + if (! isSameIdentityAsInServicesXml(applicationId, instanceConfirmation.domain, instanceConfirmation.service)) { + return false; + } + log.log(LogLevel.INFO, () -> String.format("Validating instance %s.", providerUniqueId)); - PublicKey publicKey = keyProvider.getPublicKey(signedIdentityDocument.signingKeyVersion); - if (isSignatureValid(publicKey, signedIdentityDocument.rawIdentityDocument, signedIdentityDocument.signature)) { + if (isInstanceSignatureValid(instanceConfirmation)) { log.log(LogLevel.INFO, () -> String.format("Instance %s is valid.", providerUniqueId)); return true; } @@ -42,7 +57,14 @@ public class InstanceValidator { return false; } - public static boolean isSignatureValid(PublicKey publicKey, String rawIdentityDocument, String signature) { + boolean isInstanceSignatureValid(InstanceConfirmation instanceConfirmation) { + SignedIdentityDocument signedIdentityDocument = instanceConfirmation.signedIdentityDocument; + + PublicKey publicKey = keyProvider.getPublicKey(signedIdentityDocument.signingKeyVersion); + return isSignatureValid(publicKey, signedIdentityDocument.rawIdentityDocument, signedIdentityDocument.signature); + } + + static boolean isSignatureValid(PublicKey publicKey, String rawIdentityDocument, String signature) { try { Signature signatureVerifier = Signature.getInstance("SHA512withRSA"); signatureVerifier.initVerify(publicKey); @@ -52,4 +74,38 @@ public class InstanceValidator { throw new RuntimeException(e); } } + + // If/when we dont care about logging exactly whats wrong, this can be simplified + boolean isSameIdentityAsInServicesXml(ApplicationId applicationId, String domain, String service) { + Optional<ApplicationInfo> applicationInfo = superModelProvider.getSuperModel().getApplicationInfo(applicationId); + + if (!applicationInfo.isPresent()) { + log.info(String.format("Could not find application info for %s", applicationId.serializedForm())); + return false; + } + + Optional<ServiceInfo> matchingServiceInfo = applicationInfo.get() + .getModel() + .getHosts() + .stream() + .flatMap(hostInfo -> hostInfo.getServices().stream()) + .filter(serviceInfo -> serviceInfo.getProperty(SERVICE_PROPERTIES_DOMAIN_KEY).isPresent()) + .filter(serviceInfo -> serviceInfo.getProperty(SERVICE_PROPERTIES_SERVICE_KEY).isPresent()) + .findFirst(); + + if (!matchingServiceInfo.isPresent()) { + log.info(String.format("Application %s has not specified domain/service", applicationId.serializedForm())); + return false; + } + + String domainInConfig = matchingServiceInfo.get().getProperty(SERVICE_PROPERTIES_DOMAIN_KEY).get(); + String serviceInConfig = matchingServiceInfo.get().getProperty(SERVICE_PROPERTIES_SERVICE_KEY).get(); + if (!domainInConfig.equals(domain) || !serviceInConfig.equals(service)) { + log.warning(String.format("domain '%s' or service '%s' does not match the one in config for application %s", + domain, service, applicationId.serializedForm())); + return false; + } + + return true; + } } diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzInstanceProviderServiceTest.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzInstanceProviderServiceTest.java index 20a56359eff..bf0746aee7e 100644 --- a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzInstanceProviderServiceTest.java +++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzInstanceProviderServiceTest.java @@ -1,22 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.athenz.instanceproviderservice; -import com.fasterxml.jackson.core.JsonProcessingException; +import athenz.shade.zts.jersey.repackaged.com.google.common.collect.ImmutableMap; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; -import com.yahoo.component.Version; -import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.ApplicationName; -import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.InstanceName; -import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; -import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.Zone; import com.yahoo.log.LogLevel; +import com.yahoo.vespa.hosted.athenz.instanceproviderservice.AthenzInstanceProviderService.AthenzCertificateUpdater; import com.yahoo.vespa.hosted.athenz.instanceproviderservice.config.AthenzProviderServiceConfig; import com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.CertificateClient; import com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.IdentityDocumentGenerator; @@ -27,11 +19,6 @@ import com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.model.Identity import com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.model.InstanceConfirmation; import com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.model.ProviderUniqueId; import com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.model.SignedIdentityDocument; -import com.yahoo.vespa.hosted.provision.Node; -import com.yahoo.vespa.hosted.provision.NodeRepository; -import com.yahoo.vespa.hosted.provision.node.Allocation; -import com.yahoo.vespa.hosted.provision.node.Generation; -import com.yahoo.vespa.hosted.provision.testutils.MockNodeFlavors; import org.apache.http.HttpEntity; import org.apache.http.HttpResponse; import org.apache.http.HttpStatus; @@ -53,13 +40,12 @@ import org.bouncycastle.jce.provider.BouncyCastleProvider; import org.bouncycastle.operator.ContentSigner; import org.bouncycastle.operator.OperatorCreationException; import org.bouncycastle.operator.jcajce.JcaContentSignerBuilder; +import org.eclipse.jetty.util.ssl.SslContextFactory; import org.junit.Test; import javax.net.ssl.SSLContext; import java.io.IOException; -import java.io.UnsupportedEncodingException; import java.math.BigInteger; -import java.security.InvalidKeyException; import java.security.KeyManagementException; import java.security.KeyPair; import java.security.KeyPairGenerator; @@ -68,7 +54,6 @@ import java.security.NoSuchAlgorithmException; import java.security.PrivateKey; import java.security.PublicKey; import java.security.Signature; -import java.security.SignatureException; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.time.Instant; @@ -76,16 +61,15 @@ import java.time.temporal.TemporalAmount; import java.util.Base64; import java.util.Calendar; import java.util.Date; -import java.util.HashSet; -import java.util.Optional; +import java.util.concurrent.ScheduledExecutorService; import java.util.logging.Logger; import static org.hamcrest.CoreMatchers.equalTo; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyLong; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -102,28 +86,35 @@ public class AthenzInstanceProviderServiceTest { public void provider_service_hosts_endpoint_secured_with_tls() throws Exception { String domain = "domain"; String service = "service"; + AutoGeneratedKeyProvider keyProvider = new AutoGeneratedKeyProvider(); PrivateKey privateKey = keyProvider.getPrivateKey(0); AthenzProviderServiceConfig config = getAthenzProviderConfig(domain, service, "vespa.dns.suffix", ZONE); - ScheduledExecutorServiceMock executor = new ScheduledExecutorServiceMock(); + SslContextFactory sslContextFactory = AthenzInstanceProviderService.createSslContextFactory(); + AthenzCertificateUpdater certificateUpdater = new AthenzCertificateUpdater( + new SelfSignedCertificateClient(keyProvider.getKeyPair(), config, getZoneConfig(config, ZONE)), + sslContextFactory, + keyProvider, + config, + getZoneConfig(config, ZONE)); + + ScheduledExecutorService executor = mock(ScheduledExecutorService.class); + when(executor.awaitTermination(anyLong(), any())).thenReturn(true); + + InstanceValidator instanceValidator = mock(InstanceValidator.class); + when(instanceValidator.isValidInstance(any())).thenReturn(true); - AthenzInstanceProviderService athenzInstanceProviderService = - new AthenzInstanceProviderService(config, - keyProvider, - executor, - mock(NodeRepository.class), - ZONE, - new SelfSignedCertificateClient(keyProvider.getKeyPair(), config, - getZoneConfig(config, ZONE))); + IdentityDocumentGenerator identityDocumentGenerator = mock(IdentityDocumentGenerator.class); + + AthenzInstanceProviderService athenzInstanceProviderService = new AthenzInstanceProviderService( + config, executor, ZONE, sslContextFactory, instanceValidator, identityDocumentGenerator, certificateUpdater); try (CloseableHttpClient client = createHttpClient(domain, service)) { - Runnable certificateRefreshCommand = executor.getCommand() - .orElseThrow(() -> new AssertionError("Command not present")); assertFalse(getStatus(client)); - certificateRefreshCommand.run(); + certificateUpdater.run(); assertTrue(getStatus(client)); assertInstanceConfirmationSucceeds(client, privateKey); - certificateRefreshCommand.run(); + certificateUpdater.run(); assertTrue(getStatus(client)); assertInstanceConfirmationSucceeds(client, privateKey); } finally { @@ -131,62 +122,7 @@ public class AthenzInstanceProviderServiceTest { } } - @Test - public void generates_valid_identity_document() throws Exception { - String hostname = "x.y.com"; - - ApplicationId appid = ApplicationId.from( - TenantName.from("tenant"), ApplicationName.from("application"), InstanceName.from("default")); - Allocation allocation = new Allocation(appid, - ClusterMembership.from("container/default/0/0", Version.fromString("1.2.3")), - Generation.inital(), - false); - Node n = Node.create("ostkid", - ImmutableSet.of("127.0.0.1"), - new HashSet<>(), - hostname, - Optional.empty(), - new MockNodeFlavors().getFlavorOrThrow("default"), - NodeType.tenant) - .with(allocation); - - NodeRepository nodeRepository = mock(NodeRepository.class); - when(nodeRepository.getNode(eq(hostname))).thenReturn(Optional.of(n)); - AutoGeneratedKeyProvider keyProvider = new AutoGeneratedKeyProvider(); - - String dnsSuffix = "vespa.dns.suffix"; - AthenzProviderServiceConfig athenzProviderConfig = getAthenzProviderConfig("domain", "service", dnsSuffix, ZONE); - IdentityDocumentGenerator identityDocumentGenerator = new IdentityDocumentGenerator( - athenzProviderConfig, - getZoneConfig(athenzProviderConfig, ZONE), - nodeRepository, - ZONE, - keyProvider); - String rawSignedIdentityDocument = identityDocumentGenerator.generateSignedIdentityDocument(hostname); - - - SignedIdentityDocument signedIdentityDocument = - Utils.getMapper().readValue(rawSignedIdentityDocument, SignedIdentityDocument.class); - - // Verify attributes - assertEquals(hostname, signedIdentityDocument.identityDocument.instanceHostname); - - String environment = "dev"; - String region = "us-north-1"; - String expectedZoneDnsSuffix = environment + "-" + region + "." + dnsSuffix; - assertEquals(expectedZoneDnsSuffix, signedIdentityDocument.dnsSuffix); - - ProviderUniqueId expectedProviderUniqueId = - new ProviderUniqueId("tenant", "application", environment, region, "default", "default", 0); - assertEquals(expectedProviderUniqueId, signedIdentityDocument.identityDocument.providerUniqueId); - - // Validate signature - assertTrue("Message", InstanceValidator.isSignatureValid(keyProvider.getPublicKey(0), - signedIdentityDocument.rawIdentityDocument, - signedIdentityDocument.signature)); - } - - private static AthenzProviderServiceConfig getAthenzProviderConfig(String domain, String service, String dnsSuffix, Zone zone) { + public static AthenzProviderServiceConfig getAthenzProviderConfig(String domain, String service, String dnsSuffix, Zone zone) { AthenzProviderServiceConfig.Zones.Builder zoneConfig = new AthenzProviderServiceConfig.Zones.Builder() .serviceName(service) @@ -205,7 +141,7 @@ public class AthenzInstanceProviderServiceTest { } - private AthenzProviderServiceConfig.Zones getZoneConfig(AthenzProviderServiceConfig config, Zone zone) { + public static AthenzProviderServiceConfig.Zones getZoneConfig(AthenzProviderServiceConfig config, Zone zone) { return config.zones(zone.environment().value() + "." + zone.region().value()); } @@ -264,23 +200,23 @@ public class AthenzInstanceProviderServiceTest { "localhost/zts", 1)); return new StringEntity(mapper.writeValueAsString(instanceConfirmation)); - } catch (JsonProcessingException - | NoSuchAlgorithmException - | UnsupportedEncodingException - | SignatureException - | InvalidKeyException e) { + } catch (Exception e) { throw new RuntimeException(e); } } - private static class AutoGeneratedKeyProvider implements KeyProvider { + public static class AutoGeneratedKeyProvider implements KeyProvider { private final KeyPair keyPair; - public AutoGeneratedKeyProvider() throws IOException, NoSuchAlgorithmException { - KeyPairGenerator rsa = KeyPairGenerator.getInstance("RSA"); - rsa.initialize(2048); - keyPair = rsa.genKeyPair(); + public AutoGeneratedKeyProvider() { + try { + KeyPairGenerator rsa = KeyPairGenerator.getInstance("RSA"); + rsa.initialize(2048); + keyPair = rsa.genKeyPair(); + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException(e); + } } @Override diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ScheduledExecutorServiceMock.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ScheduledExecutorServiceMock.java deleted file mode 100644 index 45cb82a0c0a..00000000000 --- a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ScheduledExecutorServiceMock.java +++ /dev/null @@ -1,115 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.athenz.instanceproviderservice; - -import java.util.Collection; -import java.util.List; -import java.util.Objects; -import java.util.Optional; -import java.util.concurrent.Callable; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.Future; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; - -/** - * @author bjorncs - */ -public class ScheduledExecutorServiceMock implements ScheduledExecutorService { - - private Runnable runnable; - - public Optional<Runnable> getCommand() { - return Optional.ofNullable(runnable); - } - - @Override - public ScheduledFuture<?> schedule(Runnable command, long delay, TimeUnit unit) { - throw new UnsupportedOperationException(); - } - - @Override - public <V> ScheduledFuture<V> schedule(Callable<V> callable, long delay, TimeUnit unit) { - throw new UnsupportedOperationException(); - } - - @Override - public ScheduledFuture<?> scheduleAtFixedRate(Runnable command, long initialDelay, long period, TimeUnit unit) { - if (runnable != null) { - throw new IllegalStateException("Can only register single command"); - } - runnable = Objects.requireNonNull(command); - return null; - } - - @Override - public ScheduledFuture<?> scheduleWithFixedDelay(Runnable command, long initialDelay, long delay, TimeUnit unit) { - throw new UnsupportedOperationException(); - } - - @Override - public void shutdown() { - // do nothing - } - - @Override - public List<Runnable> shutdownNow() { - throw new UnsupportedOperationException(); - } - - @Override - public boolean isShutdown() { - throw new UnsupportedOperationException(); - } - - @Override - public boolean isTerminated() { - throw new UnsupportedOperationException(); - } - - @Override - public boolean awaitTermination(long timeout, TimeUnit unit) throws InterruptedException { - return true; - } - - @Override - public <T> Future<T> submit(Callable<T> task) { - throw new UnsupportedOperationException(); - } - - @Override - public <T> Future<T> submit(Runnable task, T result) { - throw new UnsupportedOperationException(); - } - - @Override - public Future<?> submit(Runnable task) { - throw new UnsupportedOperationException(); - } - - @Override - public <T> List<Future<T>> invokeAll(Collection<? extends Callable<T>> tasks) throws InterruptedException { - throw new UnsupportedOperationException(); - } - - @Override - public <T> List<Future<T>> invokeAll(Collection<? extends Callable<T>> tasks, long timeout, TimeUnit unit) throws InterruptedException { - throw new UnsupportedOperationException(); - } - - @Override - public <T> T invokeAny(Collection<? extends Callable<T>> tasks) throws InterruptedException, ExecutionException { - throw new UnsupportedOperationException(); - } - - @Override - public <T> T invokeAny(Collection<? extends Callable<T>> tasks, long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException { - throw new UnsupportedOperationException(); - } - - @Override - public void execute(Runnable command) { - throw new UnsupportedOperationException(); - } -} diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/impl/IdentityDocumentGeneratorTest.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/impl/IdentityDocumentGeneratorTest.java new file mode 100644 index 00000000000..d77757374ce --- /dev/null +++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/impl/IdentityDocumentGeneratorTest.java @@ -0,0 +1,98 @@ +package com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl; + + +import com.google.common.collect.ImmutableSet; +import com.yahoo.component.Version; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ApplicationName; +import com.yahoo.config.provision.ClusterMembership; +import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.InstanceName; +import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.TenantName; +import com.yahoo.config.provision.Zone; +import com.yahoo.vespa.hosted.athenz.instanceproviderservice.AthenzInstanceProviderServiceTest.AutoGeneratedKeyProvider; +import com.yahoo.vespa.hosted.athenz.instanceproviderservice.config.AthenzProviderServiceConfig; +import com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.model.ProviderUniqueId; +import com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.model.SignedIdentityDocument; +import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.NodeRepository; +import com.yahoo.vespa.hosted.provision.node.Allocation; +import com.yahoo.vespa.hosted.provision.node.Generation; +import com.yahoo.vespa.hosted.provision.testutils.MockNodeFlavors; +import org.junit.Test; + +import java.util.HashSet; +import java.util.Optional; + +import static com.yahoo.vespa.hosted.athenz.instanceproviderservice.AthenzInstanceProviderServiceTest.getAthenzProviderConfig; +import static com.yahoo.vespa.hosted.athenz.instanceproviderservice.AthenzInstanceProviderServiceTest.getZoneConfig; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * @author valerijf + */ +public class IdentityDocumentGeneratorTest { + private static final Zone ZONE = new Zone(SystemName.cd, Environment.dev, RegionName.from("us-north-1")); + + @Test + public void generates_valid_identity_document() throws Exception { + String hostname = "x.y.com"; + + ApplicationId appid = ApplicationId.from( + TenantName.from("tenant"), ApplicationName.from("application"), InstanceName.from("default")); + Allocation allocation = new Allocation(appid, + ClusterMembership.from("container/default/0/0", Version.fromString("1.2.3")), + Generation.inital(), + false); + Node n = Node.create("ostkid", + ImmutableSet.of("127.0.0.1"), + new HashSet<>(), + hostname, + Optional.empty(), + new MockNodeFlavors().getFlavorOrThrow("default"), + NodeType.tenant) + .with(allocation); + + NodeRepository nodeRepository = mock(NodeRepository.class); + when(nodeRepository.getNode(eq(hostname))).thenReturn(Optional.of(n)); + AutoGeneratedKeyProvider keyProvider = new AutoGeneratedKeyProvider(); + + String dnsSuffix = "vespa.dns.suffix"; + AthenzProviderServiceConfig config = getAthenzProviderConfig("domain", "service", dnsSuffix, ZONE); + IdentityDocumentGenerator identityDocumentGenerator = new IdentityDocumentGenerator( + config, + getZoneConfig(config, ZONE), + nodeRepository, + ZONE, + keyProvider); + String rawSignedIdentityDocument = identityDocumentGenerator.generateSignedIdentityDocument(hostname); + + + SignedIdentityDocument signedIdentityDocument = + Utils.getMapper().readValue(rawSignedIdentityDocument, SignedIdentityDocument.class); + + // Verify attributes + assertEquals(hostname, signedIdentityDocument.identityDocument.instanceHostname); + + String environment = "dev"; + String region = "us-north-1"; + String expectedZoneDnsSuffix = environment + "-" + region + "." + dnsSuffix; + assertEquals(expectedZoneDnsSuffix, signedIdentityDocument.dnsSuffix); + + ProviderUniqueId expectedProviderUniqueId = + new ProviderUniqueId("tenant", "application", environment, region, "default", "default", 0); + assertEquals(expectedProviderUniqueId, signedIdentityDocument.identityDocument.providerUniqueId); + + // Validate signature + assertTrue("Message", InstanceValidator.isSignatureValid(keyProvider.getPublicKey(0), + signedIdentityDocument.rawIdentityDocument, + signedIdentityDocument.signature)); + } +}
\ No newline at end of file diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/impl/InstanceValidatorTest.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/impl/InstanceValidatorTest.java new file mode 100644 index 00000000000..c1fab319ebf --- /dev/null +++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/impl/InstanceValidatorTest.java @@ -0,0 +1,171 @@ +package com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.yahoo.config.model.api.ApplicationInfo; +import com.yahoo.config.model.api.HostInfo; +import com.yahoo.config.model.api.Model; +import com.yahoo.config.model.api.ServiceInfo; +import com.yahoo.config.model.api.SuperModel; +import com.yahoo.config.model.api.SuperModelProvider; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.vespa.hosted.athenz.instanceproviderservice.AthenzInstanceProviderServiceTest.AutoGeneratedKeyProvider; +import com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.model.IdentityDocument; +import com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.model.InstanceConfirmation; +import com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.model.ProviderUniqueId; +import com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.model.SignedIdentityDocument; +import org.junit.Test; + +import java.security.PrivateKey; +import java.security.Signature; +import java.time.Instant; +import java.util.Base64; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import java.util.stream.Stream; + +import static com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.InstanceValidator.SERVICE_PROPERTIES_DOMAIN_KEY; +import static com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.InstanceValidator.SERVICE_PROPERTIES_SERVICE_KEY; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * @author valerijf + */ +public class InstanceValidatorTest { + + private final ApplicationId applicationId = ApplicationId.from("tenant", "application", "instance"); + private final String domain = "domain"; + private final String service = "service"; + + @Test + public void valid_signature() throws Exception { + KeyProvider keyProvider = new AutoGeneratedKeyProvider(); + InstanceValidator instanceValidator = new InstanceValidator(keyProvider, null); + InstanceConfirmation instanceConfirmation = createInstanceConfirmation( + keyProvider.getPrivateKey(0), applicationId, domain, service); + + assertTrue(instanceValidator.isInstanceSignatureValid(instanceConfirmation)); + } + + @Test + public void invalid_signature() throws Exception { + KeyProvider keyProvider = new AutoGeneratedKeyProvider(); + InstanceValidator instanceValidator = new InstanceValidator(keyProvider, null); + + KeyProvider fakeKeyProvider = new AutoGeneratedKeyProvider(); + InstanceConfirmation instanceConfirmation = createInstanceConfirmation( + fakeKeyProvider.getPrivateKey(0), applicationId, domain, service); + + assertFalse(instanceValidator.isInstanceSignatureValid(instanceConfirmation)); + } + + @Test + public void application_does_not_exist() { + SuperModelProvider superModelProvider = mockSuperModelProvider(); + InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider); + + assertFalse(instanceValidator.isSameIdentityAsInServicesXml(applicationId, domain, service)); + } + + @Test + public void application_does_not_have_domain_set() { + SuperModelProvider superModelProvider = mockSuperModelProvider( + mockApplicationInfo(applicationId, 5, Collections.emptyList())); + InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider); + + assertFalse(instanceValidator.isSameIdentityAsInServicesXml(applicationId, domain, service)); + } + + @Test + public void application_has_wrong_domain() { + ServiceInfo serviceInfo = new ServiceInfo("serviceName", "type", Collections.emptyList(), + Collections.singletonMap(SERVICE_PROPERTIES_DOMAIN_KEY, "not-domain"), "confId", "hostName"); + + SuperModelProvider superModelProvider = mockSuperModelProvider( + mockApplicationInfo(applicationId, 5, Collections.singletonList(serviceInfo))); + InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider); + + assertFalse(instanceValidator.isSameIdentityAsInServicesXml(applicationId, domain, service)); + } + + @Test + public void application_has_same_domain_and_service() { + Map<String, String> properties = new HashMap<>(); + properties.put(SERVICE_PROPERTIES_DOMAIN_KEY, domain); + properties.put(SERVICE_PROPERTIES_SERVICE_KEY, service); + + ServiceInfo serviceInfo = new ServiceInfo("serviceName", "type", Collections.emptyList(), + properties, "confId", "hostName"); + + SuperModelProvider superModelProvider = mockSuperModelProvider( + mockApplicationInfo(applicationId, 5, Collections.singletonList(serviceInfo))); + InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider); + + assertTrue(instanceValidator.isSameIdentityAsInServicesXml(applicationId, domain, service)); + } + + private static InstanceConfirmation createInstanceConfirmation(PrivateKey privateKey, ApplicationId applicationId, + String domain, String service) { + IdentityDocument identityDocument = new IdentityDocument( + new ProviderUniqueId(applicationId.tenant().value(), applicationId.application().value(), + "environment", "region", applicationId.instance().value(), "cluster-id", 0), + "hostname", + "instance-hostname", + Instant.now()); + + try { + ObjectMapper mapper = Utils.getMapper(); + String encodedIdentityDocument = + Base64.getEncoder().encodeToString(mapper.writeValueAsString(identityDocument).getBytes()); + Signature sigGenerator = Signature.getInstance("SHA512withRSA"); + sigGenerator.initSign(privateKey); + sigGenerator.update(encodedIdentityDocument.getBytes()); + + return new InstanceConfirmation( + "provider", domain, service, + new SignedIdentityDocument(encodedIdentityDocument, + Base64.getEncoder().encodeToString(sigGenerator.sign()), + 0, + identityDocument.providerUniqueId.asString(), + "dnssuffix", + "service", + "localhost/zts", + 1)); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + private SuperModelProvider mockSuperModelProvider(ApplicationInfo... appInfos) { + SuperModel superModel = new SuperModel(Stream.of(appInfos) + .collect(Collectors.groupingBy( + appInfo -> appInfo.getApplicationId().tenant(), + Collectors.toMap( + ApplicationInfo::getApplicationId, + Function.identity() + ) + ))); + + SuperModelProvider superModelProvider = mock(SuperModelProvider.class); + when(superModelProvider.getSuperModel()).thenReturn(superModel); + return superModelProvider; + } + + private ApplicationInfo mockApplicationInfo(ApplicationId appId, int numHosts, List<ServiceInfo> serviceInfo) { + List<HostInfo> hosts = IntStream.range(0, numHosts) + .mapToObj(i -> new HostInfo("host-" + i + "." + appId.toShortString() + ".yahoo.com", serviceInfo)) + .collect(Collectors.toList()); + + Model model = mock(Model.class); + when(model.getHosts()).thenReturn(hosts); + + return new ApplicationInfo(appId, 0, model); + } +}
\ No newline at end of file diff --git a/bootstrap-cpp.sh b/bootstrap-cpp.sh index 0b1d5751e96..47d2a82622a 100755 --- a/bootstrap-cpp.sh +++ b/bootstrap-cpp.sh @@ -5,9 +5,10 @@ usage() { echo "Usage: $0 <source-dir> <build-dir>" >&2 } +# Parse arguments if [ $# -eq 2 ]; then - SOURCE_DIR=$(realpath $1) - BUILD_DIR=$(realpath $2) + SOURCE_DIR="$1" + BUILD_DIR="$2" elif [[ $# -eq 1 && ( "$1" = "-h" || "$1" = "--help" )]]; then usage exit 0 @@ -17,10 +18,23 @@ else exit 1 fi -mkdir -p "${BUILD_DIR}" +# Check the source directory +if [ ! -d "$SOURCE_DIR" ] ; then + echo "Source dir $SOURCE_DIR not found" >&2 + exit 1 +fi +SOURCE_DIR=$(realpath "${SOURCE_DIR}") + +# Check (and possibly create) the build directory +mkdir -p "${BUILD_DIR}" || { + echo "Failed to create build directory" >&2 + exit 1 +} +BUILD_DIR=$(realpath "${BUILD_DIR}") +# Build it source /opt/rh/devtoolset-6/enable || true cd "${SOURCE_DIR}" -sh ./bootstrap.sh full +bash ./bootstrap.sh full cd "${BUILD_DIR}" -sh ${SOURCE_DIR}/bootstrap-cmake.sh ${SOURCE_DIR} +bash ${SOURCE_DIR}/bootstrap-cmake.sh "${SOURCE_DIR}" diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java index f5e3e64bac3..2dcf874a66e 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java @@ -214,6 +214,9 @@ public class VespaMetricSet { metrics.add(new Metric("content.proton.documentdb.ready.lid_space.lid_fragmentation_factor.average")); metrics.add(new Metric("content.proton.documentdb.notready.lid_space.lid_fragmentation_factor.average")); metrics.add(new Metric("content.proton.documentdb.removed.lid_space.lid_fragmentation_factor.average")); + metrics.add(new Metric("content.proton.documentdb.ready.lid_space.lid_limit.last")); + metrics.add(new Metric("content.proton.documentdb.notready.lid_space.lid_limit.last")); + metrics.add(new Metric("content.proton.documentdb.removed.lid_space.lid_limit.last")); // resource usage metrics.add(new Metric("content.proton.resource_usage.disk.average")); @@ -303,11 +306,13 @@ public class VespaMetricSet { metrics.add(new Metric("vds.filestor.spi.put.success.average")); metrics.add(new Metric("vds.filestor.spi.remove.success.average")); metrics.add(new Metric("vds.filestor.spi.update.success.average")); + metrics.add(new Metric("vds.filestor.spi.deleteBucket.success.average")); metrics.add(new Metric("vds.filestor.spi.get.success.average")); metrics.add(new Metric("vds.filestor.spi.iterate.success.average")); metrics.add(new Metric("vds.filestor.spi.put.success.rate")); metrics.add(new Metric("vds.filestor.spi.remove.success.rate")); metrics.add(new Metric("vds.filestor.spi.update.success.rate")); + metrics.add(new Metric("vds.filestor.spi.deleteBucket.success.rate")); metrics.add(new Metric("vds.filestor.spi.get.success.rate")); metrics.add(new Metric("vds.filestor.spi.iterate.success.rate")); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index fb7ad137c22..ce9d0ed27f1 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -162,6 +162,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { addLegacyFilters(spec, cluster); // TODO: Remove for Vespa 7 // Athenz copper argos + // NOTE: Must be done after addNodes() addIdentity(spec, cluster, context.getDeployState().getProperties().configServerSpecs()); //TODO: overview handler, see DomQrserverClusterBuilder @@ -703,7 +704,10 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { Identity identity = new Identity(domain.trim(), service.trim(), cfgHostName); cluster.addComponent(identity); - + cluster.getContainers().forEach(container -> { + container.setProp("identity.domain", domain); + container.setProp("identity.service", service); + }); } } diff --git a/container-search/src/main/java/com/yahoo/prelude/statistics/StatisticsSearcher.java b/container-search/src/main/java/com/yahoo/prelude/statistics/StatisticsSearcher.java index f9da5b18fe3..c436ce09d33 100644 --- a/container-search/src/main/java/com/yahoo/prelude/statistics/StatisticsSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/statistics/StatisticsSearcher.java @@ -66,14 +66,14 @@ public class StatisticsSearcher extends Searcher { private Value peakQPS; // peak 1s QPS private Counter emptyResults; // number of results containing no concrete hits private Value hitsPerQuery; // mean number of hits per query - private long prevMaxQPSTime; // previous measurement time of QPS - private double queriesForQPS = 0.0; - private final Object peakQpsLock = new Object(); + + private final PeakQpsReporter peakQpsReporter; + private Metric metric; private Map<String, Metric.Context> chainContexts = new CopyOnWriteHashMap<>(); private Map<String, Metric.Context> statePageOnlyContexts = new CopyOnWriteHashMap<>(); - + private java.util.Timer scheduler = new java.util.Timer(true); private void initEvents(com.yahoo.statistics.Statistics manager, MetricReceiver metricReceiver) { queries = new Counter(QUERIES_METRIC, manager, false); @@ -108,37 +108,63 @@ public class StatisticsSearcher extends Searcher { metric.set(ACTIVE_QUERIES_METRIC, searchQueriesInFlight, null); } } + private class PeakQpsReporter extends java.util.TimerTask { + private long prevMaxQPSTime = System.currentTimeMillis(); + private long queriesForQPS = 0; + private Metric.Context metricContext = null; + public void setContext(Metric.Context metricContext) { + if (this.metricContext == null) { + synchronized(this) { + this.metricContext = metricContext; + } + } + } + @Override + public void run() { + long now = System.currentTimeMillis(); + synchronized (this) { + if (metricContext == null) return; + flushPeakQps(now); + } + } + private void flushPeakQps(long now) { + double ms = (double) (now - prevMaxQPSTime); + final double value = ((double)queriesForQPS) / (ms / 1000.0); + peakQPS.put(value); + metric.set(PEAK_QPS_METRIC, value, metricContext); + prevMaxQPSTime = now; + queriesForQPS = 0; + } + public void countQuery() { + synchronized (this) { + ++queriesForQPS; + } + } + } StatisticsSearcher(Metric metric) { this(com.yahoo.statistics.Statistics.nullImplementation, metric, MetricReceiver.nullImplementation); } public StatisticsSearcher(com.yahoo.statistics.Statistics manager, Metric metric, MetricReceiver metricReceiver) { + this.peakQpsReporter = new PeakQpsReporter(); this.metric = metric; initEvents(manager, metricReceiver); + scheduler.schedule(peakQpsReporter, 1000, 1000); + } + + @Override + public void deconstruct() { + scheduler.cancel(); } public String getMyID() { return (getId().stringValue()); } - private void qps(long now, Metric.Context metricContext) { - // We can either have peakQpsLock _or_ have prevMaxQpsTime as a volatile - // and queriesForQPS as an AtomicInteger. That would lead no locking, - // but two memory barriers in the common case. Don't change till we know - // that is actually better. - synchronized (peakQpsLock) { - if ((now - prevMaxQPSTime) >= (1000)) { - double ms = (double) (now - prevMaxQPSTime); - final double peakQPS = queriesForQPS / (ms / 1000); - this.peakQPS.put(peakQPS); - metric.set(PEAK_QPS_METRIC, peakQPS, metricContext); - queriesForQPS = 1.0d; - prevMaxQPSTime = now; - } else { - queriesForQPS += 1.0d; - } - } + private void qps(Metric.Context metricContext) { + peakQpsReporter.setContext(metricContext); + peakQpsReporter.countQuery(); } private Metric.Context getChainMetricContext(String chainName) { @@ -164,7 +190,7 @@ public class StatisticsSearcher extends Searcher { incrQueryCount(metricContext); logQuery(query); long start = System.currentTimeMillis(); // Start time, in millisecs. - qps(start, metricContext); + qps(metricContext); Result result; //handle exceptions thrown below in searchers try { @@ -174,7 +200,6 @@ public class StatisticsSearcher extends Searcher { throw e; } - long end = System.currentTimeMillis(); // Start time, in millisecs. long latency = end - start; if (latency >= 0) { diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java index f578322ac76..815963efb18 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java @@ -29,6 +29,7 @@ public class ConfigServerException extends RuntimeException { public enum ErrorCode { APPLICATION_LOCK_FAILURE, BAD_REQUEST, + ACTIVATION_CONFLICT, INTERNAL_SERVER_ERROR, INVALID_APPLICATION_PACKAGE, METHOD_NOT_ALLOWED, diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/security/KeyServiceMock.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/security/KeyServiceMock.java new file mode 100644 index 00000000000..46fa2a593c5 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/security/KeyServiceMock.java @@ -0,0 +1,13 @@ +package com.yahoo.vespa.hosted.controller.api.integration.security; + +/** + * @author mpolden + */ +public class KeyServiceMock implements KeyService { + + @Override + public String getSecret(String key) { + return "fake-secret-for-" + key; + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/mock/AthenzClientFactoryMock.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/mock/AthenzClientFactoryMock.java index 0cd55fc685f..d4a2d77c115 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/mock/AthenzClientFactoryMock.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/mock/AthenzClientFactoryMock.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.athenz.mock; +import com.google.inject.Inject; import com.yahoo.component.AbstractComponent; import com.yahoo.vespa.hosted.controller.athenz.NToken; import com.yahoo.vespa.hosted.controller.athenz.ZmsClient; @@ -19,6 +20,7 @@ public class AthenzClientFactoryMock extends AbstractComponent implements Athenz private final AthenzDbMock athenz; + @Inject public AthenzClientFactoryMock() { this(new AthenzDbMock()); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java index 0a3e6a1beb8..e5c7b7f1c2f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java @@ -127,17 +127,33 @@ public class DeploymentTrigger { for (Application application : applications.asList()) { try (Lock lock = applications().lock(application.id())) { Optional<LockedApplication> lockedApplication = controller.applications().get(application.id(), lock); - if (!lockedApplication.isPresent()) continue; // application removed + if ( ! lockedApplication.isPresent()) continue; // application removed triggerReadyJobs(lockedApplication.get()); } } } - + + /** Find the next step to trigger if any, and triggers it */ private void triggerReadyJobs(LockedApplication application) { if ( ! application.deploying().isPresent()) return; - for (JobType jobType : order.jobsFrom(application.deploymentSpec())) { + List<JobType> jobs = order.jobsFrom(application.deploymentSpec()); + + // Should the first step be triggered? + if ( ! jobs.isEmpty() && jobs.get(0).equals(JobType.systemTest) && + application.deploying().get() instanceof Change.VersionChange) { + Version target = ((Change.VersionChange)application.deploying().get()).version(); + JobStatus jobStatus = application.deploymentJobs().jobStatus().get(JobType.systemTest); + if (jobStatus == null || ! jobStatus.lastTriggered().isPresent() + || ! jobStatus.lastTriggered().get().version().equals(target)) { + application = trigger(JobType.systemTest, application, false, "Upgrade to " + target); + controller.applications().store(application); + } + } + + // Find next steps to trigger based on the state of the previous step + for (JobType jobType : jobs) { JobStatus jobStatus = application.deploymentJobs().jobStatus().get(jobType); - if (jobStatus == null) continue; // never run + if (jobStatus == null) continue; // job has never run if (jobStatus.isRunning(jobTimeoutLimit())) continue; // Collect the subset of next jobs which have not run with the last changes diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ControllerCuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ControllerCuratorDb.java new file mode 100644 index 00000000000..53c152308d9 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ControllerCuratorDb.java @@ -0,0 +1,69 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.persistence; + +import com.google.inject.Inject; +import com.yahoo.cloud.config.ClusterInfoConfig; +import com.yahoo.cloud.config.ZookeeperServerConfig; +import com.yahoo.net.HostName; +import com.yahoo.vespa.curator.Curator; +import com.yahoo.vespa.zookeeper.ZooKeeperServer; + +import java.util.stream.Collectors; + +/** + * A CuratorDb that configures its own ZooKeeper cluster + * + * @author bratseth + */ +// TODO: Remove when multi controller is enabled +@Deprecated +public class ControllerCuratorDb extends CuratorDb { + + /** Use a nonstandard zk port to avoid interfering with connection to the config server zk cluster */ + private static final int zooKeeperPort = 2281; + + @SuppressWarnings("unused") // This server is used (only) from the curator instance of this over the network */ + private final ZooKeeperServer zooKeeperServer; + + /** Create a curator db which also set up a ZooKeeper server (such that this instance is both client and server) */ + @Inject + public ControllerCuratorDb(ClusterInfoConfig clusterInfo) { + super(new Curator(toConnectionSpec(clusterInfo))); + this.zooKeeperServer = new ZooKeeperServer(toZookeeperServerConfig(clusterInfo)); + } + + private static ZookeeperServerConfig toZookeeperServerConfig(ClusterInfoConfig clusterInfo) { + ZookeeperServerConfig.Builder b = new ZookeeperServerConfig.Builder(); + b.zooKeeperConfigFile("conf/zookeeper/controller-zookeeper.cfg"); + b.dataDir("var/controller-zookeeper"); + b.clientPort(zooKeeperPort); + b.myidFile("var/controller-zookeeper/myid"); + b.myid(myIndex(clusterInfo)); + + for (ClusterInfoConfig.Services clusterMember : clusterInfo.services()) { + ZookeeperServerConfig.Server.Builder server = new ZookeeperServerConfig.Server.Builder(); + server.id(clusterMember.index()); + server.hostname(clusterMember.hostname()); + server.quorumPort(zooKeeperPort + 1); + server.electionPort(zooKeeperPort + 2); + b.server(server); + } + return new ZookeeperServerConfig(b); + } + + private static Integer myIndex(ClusterInfoConfig clusterInfo) { + String hostname = HostName.getLocalhost(); + return clusterInfo.services().stream() + .filter(service -> service.hostname().equals(hostname)) + .map(ClusterInfoConfig.Services::index) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Unable to find index for this node by hostname '" + + hostname + "'")); + } + + private static String toConnectionSpec(ClusterInfoConfig clusterInfo) { + return clusterInfo.services().stream() + .map(member -> member.hostname() + ":" + zooKeeperPort) + .collect(Collectors.joining(",")); + } +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index 68df16504a8..554c496bc71 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -2,10 +2,7 @@ package com.yahoo.vespa.hosted.controller.persistence; import com.google.inject.Inject; -import com.yahoo.cloud.config.ClusterInfoConfig; -import com.yahoo.cloud.config.ZookeeperServerConfig; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.net.HostName; import com.yahoo.path.Path; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.config.SlimeUtils; @@ -14,7 +11,6 @@ import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; -import com.yahoo.vespa.zookeeper.ZooKeeperServer; import java.io.IOException; import java.io.UncheckedIOException; @@ -30,7 +26,6 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.stream.Collectors; /** * Curator backed database for storing working state shared between controller servers. @@ -40,9 +35,6 @@ import java.util.stream.Collectors; */ public class CuratorDb { - /** Use a nonstandard zk port to avoid interfering with connection to the config server zk cluster */ - private static final int zooKeeperPort = 2281; - private static final Logger log = Logger.getLogger(CuratorDb.class.getName()); private static final Path root = Path.fromString("/controller/v1"); @@ -52,9 +44,6 @@ public class CuratorDb { private final StringSetSerializer stringSetSerializer = new StringSetSerializer(); private final JobQueueSerializer jobQueueSerializer = new JobQueueSerializer(); - @SuppressWarnings("unused") // This server is used (only) from the curator instance of this over the network */ - private final ZooKeeperServer zooKeeperServer; - private final Curator curator; /** @@ -63,54 +52,11 @@ public class CuratorDb { */ private final ConcurrentHashMap<Path, Lock> locks = new ConcurrentHashMap<>(); - /** Create a curator db which also set up a ZooKeeper server (such that this instance is both client and server) */ @Inject - public CuratorDb(ClusterInfoConfig clusterInfo) { - this.zooKeeperServer = new ZooKeeperServer(toZookeeperServerConfig(clusterInfo)); - this.curator = new Curator(toConnectionSpec(clusterInfo)); - } - - /** Create a curator db which does not set up a server, using the given Curator instance */ - protected CuratorDb(Curator curator) { - this.zooKeeperServer = null; + public CuratorDb(Curator curator) { this.curator = curator; } - private static ZookeeperServerConfig toZookeeperServerConfig(ClusterInfoConfig clusterInfo) { - ZookeeperServerConfig.Builder b = new ZookeeperServerConfig.Builder(); - b.zooKeeperConfigFile("conf/zookeeper/controller-zookeeper.cfg"); - b.dataDir("var/controller-zookeeper"); - b.clientPort(zooKeeperPort); - b.myidFile("var/controller-zookeeper/myid"); - b.myid(myIndex(clusterInfo)); - - for (ClusterInfoConfig.Services clusterMember : clusterInfo.services()) { - ZookeeperServerConfig.Server.Builder server = new ZookeeperServerConfig.Server.Builder(); - server.id(clusterMember.index()); - server.hostname(clusterMember.hostname()); - server.quorumPort(zooKeeperPort + 1); - server.electionPort(zooKeeperPort + 2); - b.server(server); - } - return new ZookeeperServerConfig(b); - } - - private static Integer myIndex(ClusterInfoConfig clusterInfo) { - String hostname = HostName.getLocalhost(); - return clusterInfo.services().stream() - .filter(service -> service.hostname().equals(hostname)) - .map(ClusterInfoConfig.Services::index) - .findFirst() - .orElseThrow(() -> new IllegalStateException("Unable to find index for this node by hostname '" + - hostname + "'")); - } - - private static String toConnectionSpec(ClusterInfoConfig clusterInfo) { - return clusterInfo.services().stream() - .map(member -> member.hostname() + ":" + zooKeeperPort) - .collect(Collectors.joining(",")); - } - // -------------- Locks -------------------------------------------------- public Lock lock(TenantId id, Duration timeout) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/ErrorResponse.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/ErrorResponse.java index 4af833baa98..deee3357771 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/ErrorResponse.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/ErrorResponse.java @@ -7,6 +7,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServ import com.yahoo.yolean.Exceptions; import static com.yahoo.jdisc.Response.Status.BAD_REQUEST; +import static com.yahoo.jdisc.Response.Status.CONFLICT; import static com.yahoo.jdisc.Response.Status.FORBIDDEN; import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR; import static com.yahoo.jdisc.Response.Status.METHOD_NOT_ALLOWED; @@ -66,7 +67,16 @@ public class ErrorResponse extends SlimeJsonResponse { } public static ErrorResponse from(ConfigServerException e) { - return new ErrorResponse(BAD_REQUEST, e.getErrorCode().name(), Exceptions.toMessageString(e)); + switch (e.getErrorCode()) { + case NOT_FOUND: + return new ErrorResponse(NOT_FOUND, e.getErrorCode().name(), Exceptions.toMessageString(e)); + case ACTIVATION_CONFLICT: + return new ErrorResponse(CONFLICT, e.getErrorCode().name(), Exceptions.toMessageString(e)); + case INTERNAL_SERVER_ERROR: + return new ErrorResponse(INTERNAL_SERVER_ERROR, e.getErrorCode().name(), Exceptions.toMessageString(e)); + default: + return new ErrorResponse(BAD_REQUEST, e.getErrorCode().name(), Exceptions.toMessageString(e)); + } } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java index f73373ac718..022fa705def 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java @@ -6,9 +6,11 @@ import com.yahoo.config.provision.Environment; import com.yahoo.test.ManualClock; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.ControllerTester; +import com.yahoo.vespa.hosted.controller.LockedApplication; import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; import com.yahoo.vespa.hosted.controller.api.integration.BuildService; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; +import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType; import com.yahoo.vespa.hosted.controller.maintenance.BlockedChangeDeployer; @@ -17,6 +19,7 @@ import org.junit.Test; import java.time.Duration; import java.time.Instant; +import java.util.Optional; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -310,6 +313,21 @@ public class DeploymentTriggerTest { BuildService.BuildJob productionJob = tester.buildSystem().takeJobsToRun().get(0); assertEquals("production-us-west-1", productionJob.jobName()); } + + @Test + public void testUpgradingButNoJobStarted() { + DeploymentTester tester = new DeploymentTester(); + BlockedChangeDeployer blockedChangeDeployer = new BlockedChangeDeployer(tester.controller(), + Duration.ofHours(1), + new JobControl(tester.controllerTester().curator())); + LockedApplication app = (LockedApplication)tester.createAndDeploy("default0", 3, "default"); + // Store that we are upgrading but don't start the system-tests job + tester.controller().applications().store(app.withDeploying(Optional.of(new Change.VersionChange(Version.fromString("6.2"))))); + assertEquals(0, tester.buildSystem().jobs().size()); + blockedChangeDeployer.run(); + assertEquals(1, tester.buildSystem().jobs().size()); + assertEquals("system-test", tester.buildSystem().jobs().get(0).jobName()); + } @Test public void testHandleMultipleNotificationsFromLastJob() { @@ -336,4 +354,5 @@ public class DeploymentTriggerTest { tester.applications().require(application.id()).deploying().isPresent()); assertTrue("All jobs consumed", buildSystem.jobs().isEmpty()); } + } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index 278df2f9b1e..e3443d6c014 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -486,6 +486,22 @@ public class ApplicationApiTest extends ControllerContainerTest { athenzUserDomain, "mytenant"), new File("deploy-out-of-capacity.json"), 400); + // POST (deploy) an application where activation fails + configServer.throwOnNextPrepare(new ConfigServerException(new URI("server-url"), "Failed to activate application", ConfigServerException.ErrorCode.ACTIVATION_CONFLICT, null)); + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-west-1/instance/default/deploy", + entity, + Request.Method.POST, + athenzUserDomain, "mytenant"), + new File("deploy-activation-conflict.json"), 409); + + // POST (deploy) an application where we get an internal server error + configServer.throwOnNextPrepare(new ConfigServerException(new URI("server-url"), "Internal server error", ConfigServerException.ErrorCode.INTERNAL_SERVER_ERROR, null)); + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-west-1/instance/default/deploy", + entity, + Request.Method.POST, + athenzUserDomain, "mytenant"), + new File("deploy-internal-server-error.json"), 500); + // DELETE tenant which has an application tester.assertResponse(request("/application/v4/tenant/tenant1", "", Request.Method.DELETE), "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Could not delete tenant 'tenant1': This tenant has active applications\"}", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-activation-conflict.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-activation-conflict.json new file mode 100644 index 00000000000..c7c242d86a9 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-activation-conflict.json @@ -0,0 +1,4 @@ +{ + "error-code":"ACTIVATION_CONFLICT", + "message":"Failed to activate application" +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-internal-server-error.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-internal-server-error.json new file mode 100644 index 00000000000..aa0f5a34dd2 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-internal-server-error.json @@ -0,0 +1,4 @@ +{ + "error-code":"INTERNAL_SERVER_ERROR", + "message":"Internal server error" +} diff --git a/eval/src/apps/tensor_conformance/generate.cpp b/eval/src/apps/tensor_conformance/generate.cpp index 1f621e33d26..993d226c3c6 100644 --- a/eval/src/apps/tensor_conformance/generate.cpp +++ b/eval/src/apps/tensor_conformance/generate.cpp @@ -93,6 +93,7 @@ void generate_tensor_map(TestBuilder &dst) { generate_op1_map("isNan(a)", operation::IsNan::f, Mask2Seq(SkipNth(3), 1.0, my_nan), dst); generate_op1_map("relu(a)", operation::Relu::f, Sub2(Div10(N())), dst); generate_op1_map("sigmoid(a)", operation::Sigmoid::f, Sub2(Div10(N())), dst); + generate_op1_map("a in [1,5,7,13,42]", MyIn::f, N(), dst); generate_map_expr("map(a,f(a)((a+1)*2))", MyOp::f, Div10(N()), dst); } diff --git a/eval/src/apps/tensor_conformance/test_spec.json b/eval/src/apps/tensor_conformance/test_spec.json index 286cc428cd9..95439b0f104 100644 --- a/eval/src/apps/tensor_conformance/test_spec.json +++ b/eval/src/apps/tensor_conformance/test_spec.json @@ -532,6 +532,24 @@ {"expression":"map(a,f(a)(sigmoid(a)))","inputs":{"a":"0x010301780179017A180161036261720169BFF8000000000000016103626172016ABFF6666666666666016103626172016BBFF4CCCCCCCCCCCD016103626172016CBFF3333333333333016103666F6F0169BFFE666666666666016103666F6F016ABFFCCCCCCCCCCCCD016103666F6F016BBFFB333333333333016103666F6F016CBFF999999999999A0162036261720169BFE6666666666666016203626172016ABFE3333333333334016203626172016BBFE0000000000000016203626172016CBFD9999999999998016203666F6F0169BFF199999999999A016203666F6F016ABFF0000000000000016203666F6F016BBFECCCCCCCCCCCCC016203666F6F016CBFE999999999999A01630362617201693FB99999999999A0016303626172016A3FC99999999999A0016303626172016B3FD3333333333330016303626172016C3FD9999999999998016303666F6F0169BFD3333333333334016303666F6F016ABFC9999999999998016303666F6F016BBFB99999999999A0016303666F6F016C0000000000000000"},"result":{"expect":"0x010301780179017A1801610362617201693FC759B8355A1BB0016103626172016A3FC95209D0A1A2DE016103626172016B3FCB69C25FE3C688016103626172016C3FCDA0FADA5A6609016103666F6F01693FC0A764FD2927E7016103666F6F016A3FC2282CFA533F46016103666F6F016B3FC3C5848EF36C9E016103666F6F016C3FC5806BEB16EB7F01620362617201693FD53C695ABCD715016203626172016A3FD6AD912C137583016203626172016B3FD829A0565978DE016203626172016C3FD9AF19F3D3169C016203666F6F01693FCFF77A137CDBF9016203666F6F016A3FD136561454BA86016203666F6F016B3FD27FCDA8478FA3016203666F6F016C3FD3D775461EDE9501630362617201693FE0CCA12729AFB8016303626172016A3FE1983D7795F414016303626172016B3FE261D545E46A8A016303626172016C3FE32873061674B2016303666F6F01693FDB3C5574372AEB016303666F6F016A3FDCCF8510D417DA016303666F6F016B3FDE66BDB1ACA090016303666F6F016C3FE0000000000000"}} {"expression":"map(a,f(a)(sigmoid(a)))","inputs":{"a":"0x0301017902017803017A070203626172BFF3333333333333BFF199999999999ABFF0000000000000BFECCCCCCCCCCCCCBFE999999999999ABFE6666666666666BFE33333333333343FC99999999999A03FD33333333333303FD99999999999983FE00000000000003FE33333333333343FE66666666666683FE99999999999983FF999999999999A3FFB3333333333343FFCCCCCCCCCCCCC3FFE66666666666640000000000000004000CCCCCCCCCCCC400199999999999A03666F6FBFFE666666666666BFFCCCCCCCCCCCCDBFFB333333333333BFF999999999999ABFF8000000000000BFF6666666666666BFF4CCCCCCCCCCCDBFE0000000000000BFD9999999999998BFD3333333333334BFC9999999999998BFB99999999999A000000000000000003FB99999999999A03FECCCCCCCCCCCCC3FF00000000000003FF199999999999A3FF33333333333343FF4CCCCCCCCCCCC3FF66666666666663FF8000000000000"},"result":{"expect":"0x0301017902017803017A0702036261723FCDA0FADA5A66093FCFF77A137CDBF93FD136561454BA863FD27FCDA8478FA33FD3D775461EDE953FD53C695ABCD7153FD6AD912C1375833FE1983D7795F4143FE261D545E46A8A3FE32873061674B23FE3EB2FD4D343913FE4A93769F6453E3FE561CB52A194763FE614455CF090B63FEA9FE5053A45203FEB0E9EDC4324D83FEB75F4C16B302F3FEBD626C0B5B6063FEC2F7D5A8A79C93FEC8247621BC0C83FECCED80FEF120203666F6F3FC0A764FD2927E73FC2282CFA533F463FC3C5848EF36C9E3FC5806BEB16EB7F3FC759B8355A1BB03FC95209D0A1A2DE3FCB69C25FE3C6883FD829A0565978DE3FD9AF19F3D3169C3FDB3C5574372AEB3FDCCF8510D417DA3FDE66BDB1ACA0903FE00000000000003FE0CCA12729AFB83FE6C0192BDC382E3FE764D4F5D5A2BD3FE802217B20C9023FE897C14969667F3FE9258F68070E5E3FE9AB7D8BD797483FEA2991F2A97914"}} {"expression":"map(a,f(a)(sigmoid(a)))","inputs":{"a":"0x03020178017A010179050C01610169BFFE666666666666BFF8000000000000BFF199999999999ABFE6666666666666BFD33333333333340161016ABFFCCCCCCCCCCCCDBFF6666666666666BFF0000000000000BFE3333333333334BFC99999999999980161016BBFFB333333333333BFF4CCCCCCCCCCCDBFECCCCCCCCCCCCCBFE0000000000000BFB99999999999A00161016CBFF999999999999ABFF3333333333333BFE999999999999ABFD99999999999980000000000000000016201693FB99999999999A03FE00000000000003FECCCCCCCCCCCCC3FF4CCCCCCCCCCCC3FFB3333333333340162016A3FC99999999999A03FE33333333333343FF00000000000003FF66666666666663FFCCCCCCCCCCCCC0162016B3FD33333333333303FE66666666666683FF199999999999A3FF80000000000003FFE6666666666660162016C3FD99999999999983FE99999999999983FF33333333333343FF999999999999A4000000000000000016301694000CCCCCCCCCCCC40040000000000004007333333333334400A666666666666400D99999999999A0163016A400199999999999A4004CCCCCCCCCCCC4008000000000000400B333333333334400E6666666666660163016B4002666666666666400599999999999A4008CCCCCCCCCCCC400C000000000000400F3333333333340163016C40033333333333344006666666666666400999999999999A400CCCCCCCCCCCCC4010000000000000"},"result":{"expect":"0x03020178017A010179050C016101693FC0A764FD2927E73FC759B8355A1BB03FCFF77A137CDBF93FD53C695ABCD7153FDB3C5574372AEB0161016A3FC2282CFA533F463FC95209D0A1A2DE3FD136561454BA863FD6AD912C1375833FDCCF8510D417DA0161016B3FC3C5848EF36C9E3FCB69C25FE3C6883FD27FCDA8478FA33FD829A0565978DE3FDE66BDB1ACA0900161016C3FC5806BEB16EB7F3FCDA0FADA5A66093FD3D775461EDE953FD9AF19F3D3169C3FE0000000000000016201693FE0CCA12729AFB83FE3EB2FD4D343913FE6C0192BDC382E3FE9258F68070E5E3FEB0E9EDC4324D80162016A3FE1983D7795F4143FE4A93769F6453E3FE764D4F5D5A2BD3FE9AB7D8BD797483FEB75F4C16B302F0162016B3FE261D545E46A8A3FE561CB52A194763FE802217B20C9023FEA2991F2A979143FEBD626C0B5B6060162016C3FE32873061674B23FE614455CF090B63FE897C14969667F3FEA9FE5053A45203FEC2F7D5A8A79C9016301693FEC8247621BC0C83FED9291DDB596F83FEE54C20D06AA953FEEDC99CF2C9D4D3FEF3A59F801F5820163016A3FECCED80FEF12023FEDC99E39374D9C3FEE7B7CBC36FABD3FEEF76F8069F3FB3FEF4CBFA61DE6A30163016B3FED15854CD0D92B3FEDFC1F4CE6E8223FEE9EDD88B9D8AF3FEF0FDFCBF19A933FEF5D77DCF758080163016C3FED56A636946E583FEE2A667D67D08C3FEEBF2786AED6983FEF261E0FCD4B463FEF6CA82F0DE1EA"}} +{"expression":"a in [1,5,7,13,42]","inputs":{"a":"0x02003FF0000000000000"},"result":{"expect":"0x02003FF0000000000000"}} +{"expression":"a in [1,5,7,13,42]","inputs":{"a":"0x02010178033FF000000000000040000000000000004008000000000000"},"result":{"expect":"0x02010178033FF000000000000000000000000000000000000000000000"}} +{"expression":"a in [1,5,7,13,42]","inputs":{"a":"0x02020178030179053FF000000000000040000000000000004008000000000000401000000000000040140000000000004018000000000000401C00000000000040200000000000004022000000000000402400000000000040260000000000004028000000000000402A000000000000402C000000000000402E000000000000"},"result":{"expect":"0x02020178030179053FF00000000000000000000000000000000000000000000000000000000000003FF000000000000000000000000000003FF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003FF000000000000000000000000000000000000000000000"}} +{"expression":"a in [1,5,7,13,42]","inputs":{"a":"0x0203017803017905017A073FF000000000000040000000000000004008000000000000401000000000000040140000000000004018000000000000401C00000000000040200000000000004022000000000000402400000000000040260000000000004028000000000000402A000000000000402C000000000000402E0000000000004030000000000000403100000000000040320000000000004033000000000000403400000000000040350000000000004036000000000000403700000000000040380000000000004039000000000000403A000000000000403B000000000000403C000000000000403D000000000000403E000000000000403F00000000000040400000000000004040800000000000404100000000000040418000000000004042000000000000404280000000000040430000000000004043800000000000404400000000000040448000000000004045000000000000404580000000000040460000000000004046800000000000404700000000000040478000000000004048000000000000404880000000000040490000000000004049800000000000404A000000000000404A800000000000404B000000000000404B800000000000404C000000000000404C800000000000404D000000000000404D800000000000404E000000000000404E800000000000404F000000000000404F8000000000004050000000000000405040000000000040508000000000004050C000000000004051000000000000405140000000000040518000000000004051C000000000004052000000000000405240000000000040528000000000004052C000000000004053000000000000405340000000000040538000000000004053C000000000004054000000000000405440000000000040548000000000004054C000000000004055000000000000405540000000000040558000000000004055C000000000004056000000000000405640000000000040568000000000004056C000000000004057000000000000405740000000000040578000000000004057C000000000004058000000000000405840000000000040588000000000004058C000000000004059000000000000405940000000000040598000000000004059C00000000000405A000000000000405A400000000000"},"result":{"expect":"0x0203017803017905017A073FF00000000000000000000000000000000000000000000000000000000000003FF000000000000000000000000000003FF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003FF000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003FF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"}} +{"expression":"a in [1,5,7,13,42]","inputs":{"a":"0x010101780301613FF00000000000000162400000000000000001634008000000000000"},"result":{"expect":"0x010101780301613FF00000000000000162000000000000000001630000000000000000"}} +{"expression":"a in [1,5,7,13,42]","inputs":{"a":"0x010201780179060161036261724000000000000000016103666F6F3FF00000000000000162036261724010000000000000016203666F6F40080000000000000163036261724018000000000000016303666F6F4014000000000000"},"result":{"expect":"0x010201780179060161036261720000000000000000016103666F6F3FF00000000000000162036261720000000000000000016203666F6F00000000000000000163036261720000000000000000016303666F6F3FF0000000000000"}} +{"expression":"a in [1,5,7,13,42]","inputs":{"a":"0x010301780179017A1801610362617201694014000000000000016103626172016A4018000000000000016103626172016B401C000000000000016103626172016C4020000000000000016103666F6F01693FF0000000000000016103666F6F016A4000000000000000016103666F6F016B4008000000000000016103666F6F016C40100000000000000162036261720169402A000000000000016203626172016A402C000000000000016203626172016B402E000000000000016203626172016C4030000000000000016203666F6F01694022000000000000016203666F6F016A4024000000000000016203666F6F016B4026000000000000016203666F6F016C402800000000000001630362617201694035000000000000016303626172016A4036000000000000016303626172016B4037000000000000016303626172016C4038000000000000016303666F6F01694031000000000000016303666F6F016A4032000000000000016303666F6F016B4033000000000000016303666F6F016C4034000000000000"},"result":{"expect":"0x010301780179017A1801610362617201693FF0000000000000016103626172016A0000000000000000016103626172016B3FF0000000000000016103626172016C0000000000000000016103666F6F01693FF0000000000000016103666F6F016A0000000000000000016103666F6F016B0000000000000000016103666F6F016C000000000000000001620362617201693FF0000000000000016203626172016A0000000000000000016203626172016B0000000000000000016203626172016C0000000000000000016203666F6F01690000000000000000016203666F6F016A0000000000000000016203666F6F016B0000000000000000016203666F6F016C000000000000000001630362617201690000000000000000016303626172016A0000000000000000016303626172016B0000000000000000016303626172016C0000000000000000016303666F6F01690000000000000000016303666F6F016A0000000000000000016303666F6F016B0000000000000000016303666F6F016C0000000000000000"}} +{"expression":"a in [1,5,7,13,42]","inputs":{"a":"0x0301017902017803017A07020362617240200000000000004022000000000000402400000000000040260000000000004028000000000000402A000000000000402C0000000000004036000000000000403700000000000040380000000000004039000000000000403A000000000000403B000000000000403C000000000000404200000000000040428000000000004043000000000000404380000000000040440000000000004044800000000000404500000000000003666F6F3FF000000000000040000000000000004008000000000000401000000000000040140000000000004018000000000000401C000000000000402E000000000000403000000000000040310000000000004032000000000000403300000000000040340000000000004035000000000000403D000000000000403E000000000000403F0000000000004040000000000000404080000000000040410000000000004041800000000000"},"result":{"expect":"0x0301017902017803017A070203626172000000000000000000000000000000000000000000000000000000000000000000000000000000003FF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003FF000000000000003666F6F3FF00000000000000000000000000000000000000000000000000000000000003FF000000000000000000000000000003FF000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"}} +{"expression":"a in [1,5,7,13,42]","inputs":{"a":"0x03020178017A010179050C016101693FF000000000000040140000000000004022000000000000402A00000000000040310000000000000161016A400000000000000040180000000000004024000000000000402C00000000000040320000000000000161016B4008000000000000401C0000000000004026000000000000402E00000000000040330000000000000161016C401000000000000040200000000000004028000000000000403000000000000040340000000000000162016940350000000000004039000000000000403D000000000000404080000000000040428000000000000162016A4036000000000000403A000000000000403E000000000000404100000000000040430000000000000162016B4037000000000000403B000000000000403F000000000000404180000000000040438000000000000162016C4038000000000000403C00000000000040400000000000004042000000000000404400000000000001630169404480000000000040468000000000004048800000000000404A800000000000404C8000000000000163016A404500000000000040470000000000004049000000000000404B000000000000404D0000000000000163016B404580000000000040478000000000004049800000000000404B800000000000404D8000000000000163016C40460000000000004048000000000000404A000000000000404C000000000000404E000000000000"},"result":{"expect":"0x03020178017A010179050C016101693FF00000000000003FF000000000000000000000000000003FF000000000000000000000000000000161016A000000000000000000000000000000000000000000000000000000000000000000000000000000000161016B00000000000000003FF00000000000000000000000000000000000000000000000000000000000000161016C0000000000000000000000000000000000000000000000000000000000000000000000000000000001620169000000000000000000000000000000000000000000000000000000000000000000000000000000000162016A000000000000000000000000000000000000000000000000000000000000000000000000000000000162016B000000000000000000000000000000000000000000000000000000000000000000000000000000000162016C0000000000000000000000000000000000000000000000000000000000000000000000000000000001630169000000000000000000000000000000000000000000000000000000000000000000000000000000000163016A3FF000000000000000000000000000000000000000000000000000000000000000000000000000000163016B000000000000000000000000000000000000000000000000000000000000000000000000000000000163016C00000000000000000000000000000000000000000000000000000000000000000000000000000000"}} +{"expression":"map(a,f(a)(a in [1,5,7,13,42]))","inputs":{"a":"0x02003FF0000000000000"},"result":{"expect":"0x02003FF0000000000000"}} +{"expression":"map(a,f(a)(a in [1,5,7,13,42]))","inputs":{"a":"0x02010178033FF000000000000040000000000000004008000000000000"},"result":{"expect":"0x02010178033FF000000000000000000000000000000000000000000000"}} +{"expression":"map(a,f(a)(a in [1,5,7,13,42]))","inputs":{"a":"0x02020178030179053FF000000000000040000000000000004008000000000000401000000000000040140000000000004018000000000000401C00000000000040200000000000004022000000000000402400000000000040260000000000004028000000000000402A000000000000402C000000000000402E000000000000"},"result":{"expect":"0x02020178030179053FF00000000000000000000000000000000000000000000000000000000000003FF000000000000000000000000000003FF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003FF000000000000000000000000000000000000000000000"}} +{"expression":"map(a,f(a)(a in [1,5,7,13,42]))","inputs":{"a":"0x0203017803017905017A073FF000000000000040000000000000004008000000000000401000000000000040140000000000004018000000000000401C00000000000040200000000000004022000000000000402400000000000040260000000000004028000000000000402A000000000000402C000000000000402E0000000000004030000000000000403100000000000040320000000000004033000000000000403400000000000040350000000000004036000000000000403700000000000040380000000000004039000000000000403A000000000000403B000000000000403C000000000000403D000000000000403E000000000000403F00000000000040400000000000004040800000000000404100000000000040418000000000004042000000000000404280000000000040430000000000004043800000000000404400000000000040448000000000004045000000000000404580000000000040460000000000004046800000000000404700000000000040478000000000004048000000000000404880000000000040490000000000004049800000000000404A000000000000404A800000000000404B000000000000404B800000000000404C000000000000404C800000000000404D000000000000404D800000000000404E000000000000404E800000000000404F000000000000404F8000000000004050000000000000405040000000000040508000000000004050C000000000004051000000000000405140000000000040518000000000004051C000000000004052000000000000405240000000000040528000000000004052C000000000004053000000000000405340000000000040538000000000004053C000000000004054000000000000405440000000000040548000000000004054C000000000004055000000000000405540000000000040558000000000004055C000000000004056000000000000405640000000000040568000000000004056C000000000004057000000000000405740000000000040578000000000004057C000000000004058000000000000405840000000000040588000000000004058C000000000004059000000000000405940000000000040598000000000004059C00000000000405A000000000000405A400000000000"},"result":{"expect":"0x0203017803017905017A073FF00000000000000000000000000000000000000000000000000000000000003FF000000000000000000000000000003FF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003FF000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003FF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"}} +{"expression":"map(a,f(a)(a in [1,5,7,13,42]))","inputs":{"a":"0x010101780301613FF00000000000000162400000000000000001634008000000000000"},"result":{"expect":"0x010101780301613FF00000000000000162000000000000000001630000000000000000"}} +{"expression":"map(a,f(a)(a in [1,5,7,13,42]))","inputs":{"a":"0x010201780179060161036261724000000000000000016103666F6F3FF00000000000000162036261724010000000000000016203666F6F40080000000000000163036261724018000000000000016303666F6F4014000000000000"},"result":{"expect":"0x010201780179060161036261720000000000000000016103666F6F3FF00000000000000162036261720000000000000000016203666F6F00000000000000000163036261720000000000000000016303666F6F3FF0000000000000"}} +{"expression":"map(a,f(a)(a in [1,5,7,13,42]))","inputs":{"a":"0x010301780179017A1801610362617201694014000000000000016103626172016A4018000000000000016103626172016B401C000000000000016103626172016C4020000000000000016103666F6F01693FF0000000000000016103666F6F016A4000000000000000016103666F6F016B4008000000000000016103666F6F016C40100000000000000162036261720169402A000000000000016203626172016A402C000000000000016203626172016B402E000000000000016203626172016C4030000000000000016203666F6F01694022000000000000016203666F6F016A4024000000000000016203666F6F016B4026000000000000016203666F6F016C402800000000000001630362617201694035000000000000016303626172016A4036000000000000016303626172016B4037000000000000016303626172016C4038000000000000016303666F6F01694031000000000000016303666F6F016A4032000000000000016303666F6F016B4033000000000000016303666F6F016C4034000000000000"},"result":{"expect":"0x010301780179017A1801610362617201693FF0000000000000016103626172016A0000000000000000016103626172016B3FF0000000000000016103626172016C0000000000000000016103666F6F01693FF0000000000000016103666F6F016A0000000000000000016103666F6F016B0000000000000000016103666F6F016C000000000000000001620362617201693FF0000000000000016203626172016A0000000000000000016203626172016B0000000000000000016203626172016C0000000000000000016203666F6F01690000000000000000016203666F6F016A0000000000000000016203666F6F016B0000000000000000016203666F6F016C000000000000000001630362617201690000000000000000016303626172016A0000000000000000016303626172016B0000000000000000016303626172016C0000000000000000016303666F6F01690000000000000000016303666F6F016A0000000000000000016303666F6F016B0000000000000000016303666F6F016C0000000000000000"}} +{"expression":"map(a,f(a)(a in [1,5,7,13,42]))","inputs":{"a":"0x0301017902017803017A07020362617240200000000000004022000000000000402400000000000040260000000000004028000000000000402A000000000000402C0000000000004036000000000000403700000000000040380000000000004039000000000000403A000000000000403B000000000000403C000000000000404200000000000040428000000000004043000000000000404380000000000040440000000000004044800000000000404500000000000003666F6F3FF000000000000040000000000000004008000000000000401000000000000040140000000000004018000000000000401C000000000000402E000000000000403000000000000040310000000000004032000000000000403300000000000040340000000000004035000000000000403D000000000000403E000000000000403F0000000000004040000000000000404080000000000040410000000000004041800000000000"},"result":{"expect":"0x0301017902017803017A070203626172000000000000000000000000000000000000000000000000000000000000000000000000000000003FF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003FF000000000000003666F6F3FF00000000000000000000000000000000000000000000000000000000000003FF000000000000000000000000000003FF000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"}} +{"expression":"map(a,f(a)(a in [1,5,7,13,42]))","inputs":{"a":"0x03020178017A010179050C016101693FF000000000000040140000000000004022000000000000402A00000000000040310000000000000161016A400000000000000040180000000000004024000000000000402C00000000000040320000000000000161016B4008000000000000401C0000000000004026000000000000402E00000000000040330000000000000161016C401000000000000040200000000000004028000000000000403000000000000040340000000000000162016940350000000000004039000000000000403D000000000000404080000000000040428000000000000162016A4036000000000000403A000000000000403E000000000000404100000000000040430000000000000162016B4037000000000000403B000000000000403F000000000000404180000000000040438000000000000162016C4038000000000000403C00000000000040400000000000004042000000000000404400000000000001630169404480000000000040468000000000004048800000000000404A800000000000404C8000000000000163016A404500000000000040470000000000004049000000000000404B000000000000404D0000000000000163016B404580000000000040478000000000004049800000000000404B800000000000404D8000000000000163016C40460000000000004048000000000000404A000000000000404C000000000000404E000000000000"},"result":{"expect":"0x03020178017A010179050C016101693FF00000000000003FF000000000000000000000000000003FF000000000000000000000000000000161016A000000000000000000000000000000000000000000000000000000000000000000000000000000000161016B00000000000000003FF00000000000000000000000000000000000000000000000000000000000000161016C0000000000000000000000000000000000000000000000000000000000000000000000000000000001620169000000000000000000000000000000000000000000000000000000000000000000000000000000000162016A000000000000000000000000000000000000000000000000000000000000000000000000000000000162016B000000000000000000000000000000000000000000000000000000000000000000000000000000000162016C0000000000000000000000000000000000000000000000000000000000000000000000000000000001630169000000000000000000000000000000000000000000000000000000000000000000000000000000000163016A3FF000000000000000000000000000000000000000000000000000000000000000000000000000000163016B000000000000000000000000000000000000000000000000000000000000000000000000000000000163016C00000000000000000000000000000000000000000000000000000000000000000000000000000000"}} {"expression":"map(a,f(a)((a+1)*2))","inputs":{"a":"0x02003FB999999999999A"},"result":{"expect":"0x0200400199999999999A"}} {"expression":"map(a,f(a)((a+1)*2))","inputs":{"a":"0x02010178033FB999999999999A3FC999999999999A3FD3333333333333"},"result":{"expect":"0x0201017803400199999999999A40033333333333334004CCCCCCCCCCCD"}} {"expression":"map(a,f(a)((a+1)*2))","inputs":{"a":"0x02020178030179053FB999999999999A3FC999999999999A3FD33333333333333FD999999999999A3FE00000000000003FE33333333333333FE66666666666663FE999999999999A3FECCCCCCCCCCCCD3FF00000000000003FF199999999999A3FF33333333333333FF4CCCCCCCCCCCD3FF66666666666663FF8000000000000"},"result":{"expect":"0x0202017803017905400199999999999A40033333333333334004CCCCCCCCCCCD40066666666666664008000000000000400999999999999A400B333333333333400CCCCCCCCCCCCD400E66666666666640100000000000004010CCCCCCCCCCCD401199999999999A401266666666666640133333333333334014000000000000"}} @@ -1206,4 +1224,4 @@ {"expression":"tensor(x[10])(x+1)","inputs":{},"result":{"expect":"0x020101780A3FF000000000000040000000000000004008000000000000401000000000000040140000000000004018000000000000401C000000000000402000000000000040220000000000004024000000000000"}} {"expression":"tensor(x[5],y[4])(x*4+(y+1))","inputs":{},"result":{"expect":"0x02020178050179043FF000000000000040000000000000004008000000000000401000000000000040140000000000004018000000000000401C00000000000040200000000000004022000000000000402400000000000040260000000000004028000000000000402A000000000000402C000000000000402E00000000000040300000000000004031000000000000403200000000000040330000000000004034000000000000"}} {"expression":"tensor(x[5],y[4])(x==y)","inputs":{},"result":{"expect":"0x02020178050179043FF000000000000000000000000000000000000000000000000000000000000000000000000000003FF000000000000000000000000000000000000000000000000000000000000000000000000000003FF000000000000000000000000000000000000000000000000000000000000000000000000000003FF00000000000000000000000000000000000000000000000000000000000000000000000000000"}} -{"num_tests":1208} +{"num_tests":1226} diff --git a/eval/src/tests/eval/compiled_function/compiled_function_test.cpp b/eval/src/tests/eval/compiled_function/compiled_function_test.cpp index b887c6e45f9..0e9806d5381 100644 --- a/eval/src/tests/eval/compiled_function/compiled_function_test.cpp +++ b/eval/src/tests/eval/compiled_function/compiled_function_test.cpp @@ -128,16 +128,13 @@ TEST_FF("require that compiled evaluation passes all conformance tests", MyEvalT //----------------------------------------------------------------------------- TEST("require that large (plugin) set membership checks work") { - nodes::Array my_set; + auto my_in = std::make_unique<nodes::In>(std::make_unique<nodes::Symbol>(0)); for(size_t i = 1; i <= 100; ++i) { - my_set.add(nodes::Node_UP(new nodes::Number(i))); + my_in->add_entry(std::make_unique<nodes::Number>(i)); } - nodes::DumpContext dump_ctx({}); - vespalib::string expr = vespalib::make_string("if(a in %s,1,0)", - my_set.dump(dump_ctx).c_str()); - // fprintf(stderr, "expression: %s\n", expr.c_str()); - CompiledFunction cf(Function::parse(expr), PassParams::SEPARATE); - CompiledFunction arr_cf(Function::parse(expr), PassParams::ARRAY); + Function my_fun(std::move(my_in), {"a"}); + CompiledFunction cf(my_fun, PassParams::SEPARATE); + CompiledFunction arr_cf(my_fun, PassParams::ARRAY); auto fun = cf.get_function<1>(); auto arr_fun = arr_cf.get_function(); for (double value = 0.5; value <= 100.5; value += 0.5) { @@ -146,7 +143,7 @@ TEST("require that large (plugin) set membership checks work") { EXPECT_EQUAL(1.0, arr_fun(&value)); } else { EXPECT_EQUAL(0.0, fun(value)); - EXPECT_EQUAL(0.0, arr_fun(&value)); + EXPECT_EQUAL(0.0, arr_fun(&value)); } } } diff --git a/eval/src/tests/eval/function/function_test.cpp b/eval/src/tests/eval/function/function_test.cpp index 75a6df41b50..6c3839b6cc9 100644 --- a/eval/src/tests/eval/function/function_test.cpp +++ b/eval/src/tests/eval/function/function_test.cpp @@ -81,6 +81,11 @@ bool verify_string(const vespalib::string &str, const vespalib::string &expr) { return ok; } +void verify_error(const vespalib::string &expr, const vespalib::string &expected_error) { + Function function = Function::parse(params, expr); + EXPECT_TRUE(function.has_error()); + EXPECT_EQUAL(expected_error, function.get_error()); +} TEST("require that scientific numbers can be parsed") { EXPECT_EQUAL(1.0, as_number(Function::parse(params, "1"))); @@ -163,18 +168,16 @@ TEST("require that strings are parsed and dumped correctly") { } } -TEST("require that arrays can be parsed") { - EXPECT_EQUAL("[]", Function::parse(params, "[]").dump()); - EXPECT_EQUAL("[1,2,3]", Function::parse(params, "[1,2,3]").dump()); - EXPECT_EQUAL("[1,2,3]", Function::parse(params, "[ 1 , 2 , 3 ]").dump()); - EXPECT_EQUAL("[[x],[x,y],[1,2,[z,w]]]", Function::parse(params, "[[x],[x,y],[1,2,[z,w]]]").dump()); - EXPECT_EQUAL("[(x+1),(y-[3,7]),z,[]]", Function::parse(params, "[x+1,y-[3,7],z,[]]").dump()); +TEST("require that free arrays cannot be parsed") { + verify_error("[1,2,3]", "[]...[missing value]...[[1,2,3]]"); } TEST("require that negative values can be parsed") { - EXPECT_EQUAL("(-1)", Function::parse(params, "-1").dump()); - EXPECT_EQUAL("(-2.5)", Function::parse(params, "-2.5").dump()); - EXPECT_EQUAL("(-100)", Function::parse(params, "-100").dump()); + EXPECT_EQUAL("-1", Function::parse(params, "-1").dump()); + EXPECT_EQUAL("1", Function::parse(params, "--1").dump()); + EXPECT_EQUAL("-1", Function::parse(params, " ( - ( - ( - ( (1) ) ) ) )").dump()); + EXPECT_EQUAL("-2.5", Function::parse(params, "-2.5").dump()); + EXPECT_EQUAL("-100", Function::parse(params, "-100").dump()); } TEST("require that negative symbols can be parsed") { @@ -206,7 +209,7 @@ TEST("require that operators have appropriate binding order") { verify_operator_binding_order({ { Operator::Order::RIGHT, { "^" } }, { Operator::Order::LEFT, { "*", "/", "%" } }, { Operator::Order::LEFT, { "+", "-" } }, - { Operator::Order::LEFT, { "==", "!=", "~=", "<", "<=", ">", ">=", "in" } }, + { Operator::Order::LEFT, { "==", "!=", "~=", "<", "<=", ">", ">=" } }, { Operator::Order::LEFT, { "&&" } }, { Operator::Order::LEFT, { "||" } } }); } @@ -248,10 +251,31 @@ TEST("require that operators can not bind out of parenthesis") { } TEST("require that set membership constructs can be parsed") { - EXPECT_EQUAL("(x in [y,z,w])", Function::parse(params, "x in [y,z,w]").dump()); - EXPECT_EQUAL("(x in [y,z,w])", Function::parse(params, "x in[y,z,w]").dump()); - EXPECT_EQUAL("(x in [y,z,w])", Function::parse(params, "(x)in[y,z,w]").dump()); - EXPECT_EQUAL("((x+1) in [y,z,(w-1)])", Function::parse(params, "(x+1)in[y,z,(w-1)]").dump()); + EXPECT_EQUAL("(x in [1,2,3])", Function::parse(params, "x in [1,2,3]").dump()); + EXPECT_EQUAL("(x in [1,2,3])", Function::parse(params, "x in [ 1 , 2 , 3 ] ").dump()); + EXPECT_EQUAL("(x in [-1,-2,-3])", Function::parse(params, "x in [-1,-2,-3]").dump()); + EXPECT_EQUAL("(x in [-1,-2,-3])", Function::parse(params, "x in [ - 1 , - 2 , - 3 ]").dump()); + EXPECT_EQUAL("(x in [1,2,3])", Function::parse(params, "x in[1,2,3]").dump()); + EXPECT_EQUAL("(x in [1,2,3])", Function::parse(params, "(x)in[1,2,3]").dump()); + EXPECT_EQUAL("(x in [\"a\",2,\"c\"])", Function::parse(params, "x in [\"a\",2,\"c\"]").dump()); +} + +TEST("require that set membership entries must be array of strings/numbers") { + verify_error("x in 1", "[x in ]...[expected '[', but got '1']...[1]"); + verify_error("x in ([1])", "[x in ]...[expected '[', but got '(']...[([1])]"); + verify_error("x in [y]", "[x in [y]...[invalid entry for 'in' operator]...[]]"); + verify_error("x in [!1]", "[x in [!1]...[invalid entry for 'in' operator]...[]]"); + verify_error("x in [1+2]", "[x in [1]...[expected ',', but got '+']...[+2]]"); + verify_error("x in [-\"foo\"]", "[x in [-\"foo\"]...[invalid entry for 'in' operator]...[]]"); +} + +TEST("require that set membership binds to the next value") { + EXPECT_EQUAL("((x in [1,2,3])^2)", Function::parse(params, "x in [1,2,3]^2").dump()); +} + +TEST("require that set membership binds to the left with appropriate precedence") { + EXPECT_EQUAL("((x<y) in [1,2,3])", Function::parse(params, "x < y in [1,2,3]").dump()); + EXPECT_EQUAL("(x&&(y in [1,2,3]))", Function::parse(params, "x && y in [1,2,3]").dump()); } TEST("require that function calls can be parsed") { @@ -309,22 +333,12 @@ TEST("require that leaf nodes have no children") { EXPECT_EQUAL(0u, Function::parse("\"abc\"").root().num_children()); } -TEST("require that Array children can be accessed") { - Function f = Function::parse("[1,2,3]"); - const Node &root = f.root(); - EXPECT_TRUE(!root.is_leaf()); - ASSERT_EQUAL(3u, root.num_children()); - EXPECT_EQUAL(1.0, root.get_child(0).get_const_value()); - EXPECT_EQUAL(2.0, root.get_child(1).get_const_value()); - EXPECT_EQUAL(3.0, root.get_child(2).get_const_value()); -} - TEST("require that Neg child can be accessed") { - Function f = Function::parse("-1"); + Function f = Function::parse("-x"); const Node &root = f.root(); EXPECT_TRUE(!root.is_leaf()); ASSERT_EQUAL(1u, root.num_children()); - EXPECT_EQUAL(1.0, root.get_child(0).get_const_value()); + EXPECT_TRUE(root.get_child(0).is_param()); } TEST("require that Not child can be accessed") { @@ -386,7 +400,7 @@ TEST("require that children can be detached") { EXPECT_EQUAL(1u, detach_from_root("-a")); EXPECT_EQUAL(1u, detach_from_root("!a")); EXPECT_EQUAL(3u, detach_from_root("if(1,2,3)")); - EXPECT_EQUAL(5u, detach_from_root("[1,2,3,4,5]")); + EXPECT_EQUAL(1u, detach_from_root("a in [1,2,3,4,5]")); EXPECT_EQUAL(2u, detach_from_root("a+b")); EXPECT_EQUAL(1u, detach_from_root("isNan(a)")); EXPECT_EQUAL(2u, detach_from_root("max(a,b)")); @@ -456,7 +470,7 @@ TEST("require that traversal works as expected") { EXPECT_TRUE(verify_expression_traversal("1")); EXPECT_TRUE(verify_expression_traversal("1+2")); EXPECT_TRUE(verify_expression_traversal("1+2*3-4/5")); - EXPECT_TRUE(verify_expression_traversal("if(x,1+2*3,[a,b,c]/5)")); + EXPECT_TRUE(verify_expression_traversal("if(x,1+2*3,if(a,b,c)/5)")); } //----------------------------------------------------------------------------- @@ -492,14 +506,6 @@ TEST("require that string is const") { EXPECT_TRUE(Function::parse("\"x\"").root().is_const()); } -TEST("require that array is const if all elements are const") { - EXPECT_TRUE(Function::parse("[1,2,3]").root().is_const()); - EXPECT_TRUE(!Function::parse("[x,2,3]").root().is_const()); - EXPECT_TRUE(!Function::parse("[1,y,3]").root().is_const()); - EXPECT_TRUE(!Function::parse("[1,2,z]").root().is_const()); - EXPECT_TRUE(!Function::parse("[x,y,z]").root().is_const()); -} - TEST("require that neg is const if sub-expression is const") { EXPECT_TRUE(Function::parse("-123").root().is_const()); EXPECT_TRUE(!Function::parse("-x").root().is_const()); @@ -517,11 +523,11 @@ TEST("require that operators are cost if both children are const") { EXPECT_TRUE(Function::parse("1+2").root().is_const()); } -TEST("require that set membership is const only if array elements are const") { +TEST("require that set membership is never tagged as const (NB: avoids jit recursion)") { EXPECT_TRUE(!Function::parse("x in [x,y,z]").root().is_const()); EXPECT_TRUE(!Function::parse("1 in [x,y,z]").root().is_const()); EXPECT_TRUE(!Function::parse("1 in [1,y,z]").root().is_const()); - EXPECT_TRUE(Function::parse("1 in [1,2,3]").root().is_const()); + EXPECT_TRUE(!Function::parse("1 in [1,2,3]").root().is_const()); } TEST("require that calls are cost if all parameters are const") { @@ -554,10 +560,8 @@ TEST("require that feature in set of constants is tree if children are trees or EXPECT_TRUE(Function::parse("if (foo in [1, 2], if(bar < 3, 4, 5), 6)").root().is_tree()); EXPECT_TRUE(Function::parse("if (foo in [1, 2], if(bar < 3, 4, 5), if(baz < 6, 7, 8))").root().is_tree()); EXPECT_TRUE(Function::parse("if (foo in [1, 2], 3, if(baz < 4, 5, 6))").root().is_tree()); - EXPECT_TRUE(Function::parse("if (foo in [min(1,2), max(1,2)], 3, 4)").root().is_tree()); + EXPECT_TRUE(Function::parse("if (foo in [1, 2], min(1,3), max(1,4))").root().is_tree()); EXPECT_TRUE(!Function::parse("if (1 in [1, 2], 3, 4)").root().is_tree()); - EXPECT_TRUE(!Function::parse("if (1 in [foo, 2], 3, 4)").root().is_tree()); - EXPECT_TRUE(!Function::parse("if (foo in [bar, 2], 3, 4)").root().is_tree()); } TEST("require that sums of trees and forests are forests") { @@ -671,14 +675,17 @@ TEST("require that unknown function that is valid parameter works as expected wi EXPECT_EQUAL("[z(x)]...[unknown symbol: 'z(x)']...[+y]", Function::parse(params, "z(x)+y", MySymbolExtractor({'(', ')'})).dump()); } -//----------------------------------------------------------------------------- - -void verify_error(const vespalib::string &expr, const vespalib::string &expected_error) { - Function function = Function::parse(params, expr); - EXPECT_TRUE(function.has_error()); - EXPECT_EQUAL(expected_error, function.get_error()); +TEST("require that custom symbol extractor is not invoked for known function call") { + MySymbolExtractor extractor; + EXPECT_EQUAL(extractor.invoke_count, 0u); + EXPECT_EQUAL("[bogus]...[unknown symbol: 'bogus']...[(1,2)]", Function::parse(params, "bogus(1,2)", extractor).dump()); + EXPECT_EQUAL(extractor.invoke_count, 1u); + EXPECT_EQUAL("max(1,2)", Function::parse(params, "max(1,2)", extractor).dump()); + EXPECT_EQUAL(extractor.invoke_count, 1u); } +//----------------------------------------------------------------------------- + TEST("require that valid function does not report parse error") { Function function = Function::parse(params, "x + y"); EXPECT_TRUE(!function.has_error()); diff --git a/eval/src/tests/eval/gbdt/gbdt_test.cpp b/eval/src/tests/eval/gbdt/gbdt_test.cpp index f4f5970cabe..20969a1e3b4 100644 --- a/eval/src/tests/eval/gbdt/gbdt_test.cpp +++ b/eval/src/tests/eval/gbdt/gbdt_test.cpp @@ -31,14 +31,14 @@ TEST("require that tree stats can be calculated") { EXPECT_EQUAL(tree_size, TreeStats(Function::parse(Model().make_tree(tree_size)).root()).size); } - TreeStats stats1(Function::parse("if((a<1),1.0,if((b in [1,2,3]),if((c in 1),2.0,3.0),4.0))").root()); + TreeStats stats1(Function::parse("if((a<1),1.0,if((b in [1,2,3]),if((c in [1]),2.0,3.0),4.0))").root()); EXPECT_EQUAL(3u, stats1.num_params); EXPECT_EQUAL(4u, stats1.size); EXPECT_EQUAL(1u, stats1.num_less_checks); EXPECT_EQUAL(2u, stats1.num_in_checks); EXPECT_EQUAL(3u, stats1.max_set_size); - TreeStats stats2(Function::parse("if((d in 1),10.0,if((e<1),20.0,30.0))").root()); + TreeStats stats2(Function::parse("if((d in [1]),10.0,if((e<1),20.0,30.0))").root()); EXPECT_EQUAL(2u, stats2.num_params); EXPECT_EQUAL(3u, stats2.size); EXPECT_EQUAL(1u, stats2.num_less_checks); @@ -61,9 +61,9 @@ TEST("require that trees can be extracted from forest") { } TEST("require that forest stats can be calculated") { - Function function = Function::parse("if((a<1),1.0,if((b in [1,2,3]),if((c in 1),2.0,3.0),4.0))+" - "if((d in 1),10.0,if((e<1),20.0,30.0))+" - "if((d in 1),10.0,if((e<1),20.0,30.0))"); + Function function = Function::parse("if((a<1),1.0,if((b in [1,2,3]),if((c in [1]),2.0,3.0),4.0))+" + "if((d in [1]),10.0,if((e<1),20.0,30.0))+" + "if((d in [1]),10.0,if((e<1),20.0,30.0))"); std::vector<const Node *> trees = extract_trees(function.root()); ForestStats stats(trees); EXPECT_EQUAL(5u, stats.num_params); @@ -261,8 +261,8 @@ TEST("require that models with in checks are rejected by less only vm optimizer" } TEST("require that general VM tree optimizer works") { - Function function = Function::parse("if((a<1),1.0,if((b in [1,2,3]),if((c in 1),2.0,3.0),4.0))+" - "if((d in 1),10.0,if((e<1),if((f<1),20.0,30.0),40.0))"); + Function function = Function::parse("if((a<1),1.0,if((b in [1,2,3]),if((c in [1]),2.0,3.0),4.0))+" + "if((d in [1]),10.0,if((e<1),if((f<1),20.0,30.0),40.0))"); CompiledFunction compiled_function(function, PassParams::ARRAY, general_vm_chain); EXPECT_EQUAL(1u, compiled_function.get_forests().size()); auto f = compiled_function.get_function(); @@ -324,17 +324,17 @@ TEST("require that forests evaluate to approximately the same for all evaluation //----------------------------------------------------------------------------- TEST("require that GDBT expressions can be detected") { - Function function = Function::parse("if((a<1),1.0,if((b in [1,2,3]),if((c in 1),2.0,3.0),4.0))+" - "if((d in 1),10.0,if((e<1),20.0,30.0))+" - "if((d in 1),10.0,if((e<1),20.0,30.0))"); + Function function = Function::parse("if((a<1),1.0,if((b in [1,2,3]),if((c in [1]),2.0,3.0),4.0))+" + "if((d in [1]),10.0,if((e<1),20.0,30.0))+" + "if((d in [1]),10.0,if((e<1),20.0,30.0))"); EXPECT_TRUE(contains_gbdt(function.root(), 9)); EXPECT_TRUE(!contains_gbdt(function.root(), 10)); } TEST("require that wrapped GDBT expressions can be detected") { - Function function = Function::parse("10*(if((a<1),1.0,if((b in [1,2,3]),if((c in 1),2.0,3.0),4.0))+" - "if((d in 1),10.0,if((e<1),20.0,30.0))+" - "if((d in 1),10.0,if((e<1),20.0,30.0)))"); + Function function = Function::parse("10*(if((a<1),1.0,if((b in [1,2,3]),if((c in [1]),2.0,3.0),4.0))+" + "if((d in [1]),10.0,if((e<1),20.0,30.0))+" + "if((d in [1]),10.0,if((e<1),20.0,30.0)))"); EXPECT_TRUE(contains_gbdt(function.root(), 9)); EXPECT_TRUE(!contains_gbdt(function.root(), 10)); } @@ -345,9 +345,9 @@ TEST("require that lazy parameters are not suggested for GBDT models") { } TEST("require that lazy parameters can be suggested for small GBDT models") { - Function function = Function::parse("if((a<1),1.0,if((b in [1,2,3]),if((c in 1),2.0,3.0),4.0))+" - "if((d in 1),10.0,if((e<1),20.0,30.0))+" - "if((d in 1),10.0,if((e<1),20.0,30.0))"); + Function function = Function::parse("if((a<1),1.0,if((b in [1,2,3]),if((c in [1]),2.0,3.0),4.0))+" + "if((d in [1]),10.0,if((e<1),20.0,30.0))+" + "if((d in [1]),10.0,if((e<1),20.0,30.0))"); EXPECT_TRUE(CompiledFunction::should_use_lazy_params(function)); } diff --git a/eval/src/tests/eval/node_types/node_types_test.cpp b/eval/src/tests/eval/node_types/node_types_test.cpp index e8009447793..0b6ea9e4d35 100644 --- a/eval/src/tests/eval/node_types/node_types_test.cpp +++ b/eval/src/tests/eval/node_types/node_types_test.cpp @@ -81,12 +81,6 @@ TEST("require that input parameters preserve their type") { TEST_DO(verify("tensor(x{},y[10],z[])", "tensor(x{},y[10],z[])")); } -TEST("require that arrays are double (size) unless they contain an error") { - TEST_DO(verify("[1,2,3]", "double")); - TEST_DO(verify("[any,tensor,double]", "double")); - TEST_DO(verify("[1,error,3]", "error")); -} - TEST("require that if resolves to the appropriate type") { TEST_DO(verify("if(error,1,2)", "error")); TEST_DO(verify("if(1,error,2)", "error")); @@ -108,17 +102,6 @@ TEST("require that if resolves to the appropriate type") { TEST_DO(verify("if(double,any,double)", "any")); } -TEST("require that set membership resolves to double unless error") { - TEST_DO(verify("1 in [1,2,3]", "double")); - TEST_DO(verify("1 in [tensor,tensor,tensor]", "double")); - TEST_DO(verify("1 in tensor", "double")); - TEST_DO(verify("tensor in 1", "double")); - TEST_DO(verify("tensor in [1,2,any]", "double")); - TEST_DO(verify("any in [1,tensor,any]", "double")); - TEST_DO(verify("error in [1,tensor,any]", "error")); - TEST_DO(verify("any in [tensor,error,any]", "error")); -} - TEST("require that reduce resolves correct type") { TEST_DO(verify("reduce(error,sum)", "error")); TEST_DO(verify("reduce(tensor,sum)", "double")); @@ -244,6 +227,10 @@ TEST("require that map resolves correct type") { TEST_DO(verify_op1("map(%s,f(x)(sin(x)))")); } +TEST("require that set membership resolves correct type") { + TEST_DO(verify_op1("%s in [1,2,3]")); +} + TEST("require that join resolves correct type") { TEST_DO(verify_op2("join(%s,%s,f(x,y)(x+y))")); } diff --git a/eval/src/tests/eval/simple_tensor/simple_tensor_test.cpp b/eval/src/tests/eval/simple_tensor/simple_tensor_test.cpp index beec4ed928b..150b86f27ce 100644 --- a/eval/src/tests/eval/simple_tensor/simple_tensor_test.cpp +++ b/eval/src/tests/eval/simple_tensor/simple_tensor_test.cpp @@ -13,8 +13,7 @@ using Cells = SimpleTensor::Cells; using Address = SimpleTensor::Address; using Stash = vespalib::Stash; -// need to specify numbers explicitly as size_t to avoid ambiguous behavior for 0 -constexpr size_t operator "" _z (unsigned long long int n) { return n; } +TensorSpec to_spec(const Tensor &a) { return a.engine().to_spec(a); } const Tensor &unwrap(const Value &value) { ASSERT_TRUE(value.is_tensor()); @@ -56,28 +55,8 @@ TEST("require that simple tensors can be built using tensor spec") { .add({{"w", "yyy"}, {"x", 1}, {"y", "yyy"}, {"z", 0}}, 0.0) .add({{"w", "yyy"}, {"x", 1}, {"y", "yyy"}, {"z", 1}}, 4.0); auto full_tensor = SimpleTensorEngine::ref().create(full_spec); - SimpleTensor expect_tensor(ValueType::from_spec("tensor(w{},x[2],y{},z[2])"), - CellBuilder() - .add({{"xxx"}, {0_z}, {"xxx"}, {0_z}}, 1.0) - .add({{"xxx"}, {0_z}, {"xxx"}, {1_z}}, 0.0) - .add({{"xxx"}, {0_z}, {"yyy"}, {0_z}}, 0.0) - .add({{"xxx"}, {0_z}, {"yyy"}, {1_z}}, 2.0) - .add({{"xxx"}, {1_z}, {"xxx"}, {0_z}}, 0.0) - .add({{"xxx"}, {1_z}, {"xxx"}, {1_z}}, 0.0) - .add({{"xxx"}, {1_z}, {"yyy"}, {0_z}}, 0.0) - .add({{"xxx"}, {1_z}, {"yyy"}, {1_z}}, 0.0) - .add({{"yyy"}, {0_z}, {"xxx"}, {0_z}}, 0.0) - .add({{"yyy"}, {0_z}, {"xxx"}, {1_z}}, 0.0) - .add({{"yyy"}, {0_z}, {"yyy"}, {0_z}}, 0.0) - .add({{"yyy"}, {0_z}, {"yyy"}, {1_z}}, 0.0) - .add({{"yyy"}, {1_z}, {"xxx"}, {0_z}}, 3.0) - .add({{"yyy"}, {1_z}, {"xxx"}, {1_z}}, 0.0) - .add({{"yyy"}, {1_z}, {"yyy"}, {0_z}}, 0.0) - .add({{"yyy"}, {1_z}, {"yyy"}, {1_z}}, 4.0) - .build()); - EXPECT_EQUAL(expect_tensor, *tensor); - EXPECT_EQUAL(expect_tensor, *full_tensor); - EXPECT_EQUAL(full_spec, tensor->engine().to_spec(*tensor)); + EXPECT_EQUAL(full_spec, to_spec(*tensor)); + EXPECT_EQUAL(full_spec, to_spec(*full_tensor)); }; TEST("require that simple tensors can have their values negated") { @@ -92,10 +71,10 @@ TEST("require that simple tensors can have their values negated") { .add({{"x","2"},{"y","1"}}, 3) .add({{"x","1"},{"y","2"}}, -5)); auto result = tensor->map([](double a){ return -a; }); - EXPECT_EQUAL(*expect, *result); + EXPECT_EQUAL(to_spec(*expect), to_spec(*result)); Stash stash; const Value &result2 = SimpleTensorEngine::ref().map(TensorValue(*tensor), operation::Neg::f, stash); - EXPECT_EQUAL(*expect, unwrap(result2)); + EXPECT_EQUAL(to_spec(*expect), to_spec(unwrap(result2))); } TEST("require that simple tensors can be multiplied with each other") { @@ -117,10 +96,10 @@ TEST("require that simple tensors can be multiplied with each other") { .add({{"x","2"},{"y","1"},{"z","2"}}, 39) .add({{"x","1"},{"y","2"},{"z","1"}}, 55)); auto result = SimpleTensor::join(*lhs, *rhs, [](double a, double b){ return (a * b); }); - EXPECT_EQUAL(*expect, *result); + EXPECT_EQUAL(to_spec(*expect), to_spec(*result)); Stash stash; const Value &result2 = SimpleTensorEngine::ref().join(TensorValue(*lhs), TensorValue(*rhs), operation::Mul::f, stash); - EXPECT_EQUAL(*expect, unwrap(result2)); + EXPECT_EQUAL(to_spec(*expect), to_spec(unwrap(result2))); } TEST("require that simple tensors support dimension reduction") { @@ -147,21 +126,21 @@ TEST("require that simple tensors support dimension reduction") { auto result_sum_y = tensor->reduce(aggr_sum, {"y"}); auto result_sum_x = tensor->reduce(aggr_sum, {"x"}); auto result_sum_all = tensor->reduce(aggr_sum, {"x", "y"}); - EXPECT_EQUAL(*expect_sum_y, *result_sum_y); - EXPECT_EQUAL(*expect_sum_x, *result_sum_x); - EXPECT_EQUAL(*expect_sum_all, *result_sum_all); + EXPECT_EQUAL(to_spec(*expect_sum_y), to_spec(*result_sum_y)); + EXPECT_EQUAL(to_spec(*expect_sum_x), to_spec(*result_sum_x)); + EXPECT_EQUAL(to_spec(*expect_sum_all), to_spec(*result_sum_all)); const Value &result_sum_y_2 = SimpleTensorEngine::ref().reduce(TensorValue(*tensor), Aggr::SUM, {"y"}, stash); const Value &result_sum_x_2 = SimpleTensorEngine::ref().reduce(TensorValue(*tensor), Aggr::SUM, {"x"}, stash); const Value &result_sum_all_2 = SimpleTensorEngine::ref().reduce(TensorValue(*tensor), Aggr::SUM, {"x", "y"}, stash); const Value &result_sum_all_3 = SimpleTensorEngine::ref().reduce(TensorValue(*tensor), Aggr::SUM, {}, stash); - EXPECT_EQUAL(*expect_sum_y, unwrap(result_sum_y_2)); - EXPECT_EQUAL(*expect_sum_x, unwrap(result_sum_x_2)); + EXPECT_EQUAL(to_spec(*expect_sum_y), to_spec(unwrap(result_sum_y_2))); + EXPECT_EQUAL(to_spec(*expect_sum_x), to_spec(unwrap(result_sum_x_2))); EXPECT_TRUE(result_sum_all_2.is_double()); EXPECT_TRUE(result_sum_all_3.is_double()); EXPECT_EQUAL(21, result_sum_all_2.as_double()); EXPECT_EQUAL(21, result_sum_all_3.as_double()); - EXPECT_EQUAL(*result_sum_y, *result_sum_y); - EXPECT_NOT_EQUAL(*result_sum_y, *result_sum_x); + EXPECT_EQUAL(to_spec(*result_sum_y), to_spec(*result_sum_y)); + EXPECT_NOT_EQUAL(to_spec(*result_sum_y), to_spec(*result_sum_x)); } TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/eval/src/tests/eval/tensor_function/tensor_function_test.cpp b/eval/src/tests/eval/tensor_function/tensor_function_test.cpp index 5b2d0848f64..8bd86621bf6 100644 --- a/eval/src/tests/eval/tensor_function/tensor_function_test.cpp +++ b/eval/src/tests/eval/tensor_function/tensor_function_test.cpp @@ -102,7 +102,9 @@ void verify_equal(const Tensor &expect, const Value &value) { const Tensor *tensor = value.as_tensor(); ASSERT_TRUE(tensor != nullptr); ASSERT_EQUAL(&expect.engine(), &tensor->engine()); - EXPECT_TRUE(expect.engine().equal(expect, *tensor)); + auto expect_spec = expect.engine().to_spec(expect); + auto value_spec = tensor->engine().to_spec(*tensor); + EXPECT_EQUAL(expect_spec, value_spec); } TEST("require that tensor injection works") { diff --git a/eval/src/tests/eval/value_cache/tensor_loader_test.cpp b/eval/src/tests/eval/value_cache/tensor_loader_test.cpp index 20a77eb9fe3..ee8e502815f 100644 --- a/eval/src/tests/eval/value_cache/tensor_loader_test.cpp +++ b/eval/src/tests/eval/value_cache/tensor_loader_test.cpp @@ -42,8 +42,8 @@ std::unique_ptr<Tensor> make_mixed_tensor() { void verify_tensor(std::unique_ptr<Tensor> expect, ConstantValue::UP actual) { const auto &engine = expect->engine(); ASSERT_EQUAL(engine.type_of(*expect), actual->type()); - EXPECT_TRUE(&engine == &actual->value().as_tensor()->engine()); - EXPECT_TRUE(engine.equal(*expect, *actual->value().as_tensor())); + ASSERT_TRUE(&engine == &actual->value().as_tensor()->engine()); + EXPECT_EQUAL(engine.to_spec(*expect), engine.to_spec(*actual->value().as_tensor())); } TEST_F("require that invalid types loads an empty double", ConstantTensorLoader(SimpleTensorEngine::ref())) { diff --git a/eval/src/vespa/eval/eval/basic_nodes.cpp b/eval/src/vespa/eval/eval/basic_nodes.cpp index 50db7370a66..a96d634e07a 100644 --- a/eval/src/vespa/eval/eval/basic_nodes.cpp +++ b/eval/src/vespa/eval/eval/basic_nodes.cpp @@ -60,7 +60,7 @@ Node::traverse(NodeTraverser &traverser) const void Number::accept(NodeVisitor &visitor) const { visitor.visit(*this); } void Symbol::accept(NodeVisitor &visitor) const { visitor.visit(*this); } void String::accept(NodeVisitor &visitor) const { visitor.visit(*this); } -void Array ::accept(NodeVisitor &visitor) const { visitor.visit(*this); } +void In ::accept(NodeVisitor &visitor) const { visitor.visit(*this); } void Neg ::accept(NodeVisitor &visitor) const { visitor.visit(*this); } void Not ::accept(NodeVisitor &visitor) const { visitor.visit(*this); } void If ::accept(NodeVisitor &visitor) const { visitor.visit(*this); } @@ -124,7 +124,7 @@ If::If(Node_UP cond_in, Node_UP true_expr_in, Node_UP false_expr_in, double p_tr if (less) { _is_tree = (less->lhs().is_param() && less->rhs().is_const()); } else if (in) { - _is_tree = (in->lhs().is_param() && in->rhs().is_const()); + _is_tree = in->child().is_param(); } } } diff --git a/eval/src/vespa/eval/eval/basic_nodes.h b/eval/src/vespa/eval/eval/basic_nodes.h index 3f4c21be810..ebf65178b99 100644 --- a/eval/src/vespa/eval/eval/basic_nodes.h +++ b/eval/src/vespa/eval/eval/basic_nodes.h @@ -142,40 +142,39 @@ public: void accept(NodeVisitor &visitor) const override; }; -class Array : public Node { +class In : public Node { private: - std::vector<Node_UP> _nodes; - bool _is_const; + Node_UP _child; + std::vector<Node_UP> _entries; public: - Array() : _nodes(), _is_const(false) {} - bool is_const() const override { return _is_const; } - size_t size() const { return _nodes.size(); } - const Node &get(size_t i) const { return *_nodes[i]; } - size_t num_children() const override { return size(); } - const Node &get_child(size_t idx) const override { return get(idx); } - void detach_children(NodeHandler &handler) override { - for (size_t i = 0; i < _nodes.size(); ++i) { - handler.handle(std::move(_nodes[i])); - } - _nodes.clear(); + In(Node_UP child) : _child(std::move(child)), _entries() {} + void add_entry(Node_UP entry) { + assert(entry->is_const()); + _entries.push_back(std::move(entry)); } - void add(Node_UP node) { - if (_nodes.empty()) { - _is_const = node->is_const(); - } else { - _is_const = (_is_const && node->is_const()); - } - _nodes.push_back(std::move(node)); + size_t num_entries() const { return _entries.size(); } + const Node &get_entry(size_t idx) const { return *_entries[idx]; } + const Node &child() const { return *_child; } + size_t num_children() const override { return _child ? 1 : 0; } + const Node &get_child(size_t idx) const override { + (void) idx; + assert(idx == 0); + return child(); + } + void detach_children(NodeHandler &handler) override { + handler.handle(std::move(_child)); } vespalib::string dump(DumpContext &ctx) const override { vespalib::string str; - str += "["; + str += "("; + str += _child->dump(ctx); + str += " in ["; CommaTracker node_list; - for (const auto &node: _nodes) { + for (const auto &node: _entries) { node_list.maybe_comma(str); str += node->dump(ctx); } - str += "]"; + str += "])"; return str; } void accept(NodeVisitor &visitor) const override; diff --git a/eval/src/vespa/eval/eval/function.cpp b/eval/src/vespa/eval/eval/function.cpp index 0843baa700c..c4f91067260 100644 --- a/eval/src/vespa/eval/eval/function.cpp +++ b/eval/src/vespa/eval/eval/function.cpp @@ -281,12 +281,15 @@ public: size_t operator_mark() const { return _operator_mark; } void operator_mark(size_t mark) { _operator_mark = mark; } - void push_operator(Operator_UP node) { + void apply_until(const nodes::Operator &op) { while ((_operator_stack.size() > _operator_mark) && - (_operator_stack.back()->do_before(*node))) + (_operator_stack.back()->do_before(op))) { apply_operator(); } + } + void push_operator(Operator_UP node) { + apply_until(*node); _operator_stack.push_back(std::move(node)); } Operator_UP pop_operator() { @@ -299,6 +302,7 @@ public: //----------------------------------------------------------------------------- +void parse_value(ParseContext &ctx); void parse_expression(ParseContext &ctx); int unhex(char c) { @@ -642,8 +646,11 @@ void parse_symbol_or_call(ParseContext &ctx) { } } -void parse_array(ParseContext &ctx) { - std::unique_ptr<nodes::Array> array(new nodes::Array()); +void parse_in(ParseContext &ctx) +{ + ctx.apply_until(nodes::Less()); + auto in = std::make_unique<nodes::In>(ctx.pop_expression()); + ctx.skip_spaces(); ctx.eat('['); ctx.skip_spaces(); size_t size = 0; @@ -651,11 +658,19 @@ void parse_array(ParseContext &ctx) { if (++size > 1) { ctx.eat(','); } - parse_expression(ctx); - array->add(ctx.pop_expression()); + parse_value(ctx); + ctx.skip_spaces(); + auto entry = ctx.pop_expression(); + auto num = nodes::as<nodes::Number>(*entry); + auto str = nodes::as<nodes::String>(*entry); + if (num || str) { + in->add_entry(std::move(entry)); + } else { + ctx.fail("invalid entry for 'in' operator"); + } } ctx.eat(']'); - ctx.push_expression(std::move(array)); + ctx.push_expression(std::move(in)); } void parse_value(ParseContext &ctx) { @@ -663,7 +678,13 @@ void parse_value(ParseContext &ctx) { if (ctx.get() == '-') { ctx.next(); parse_value(ctx); - ctx.push_expression(Node_UP(new nodes::Neg(ctx.pop_expression()))); + auto entry = ctx.pop_expression(); + auto num = nodes::as<nodes::Number>(*entry); + if (num) { + ctx.push_expression(std::make_unique<nodes::Number>(-num->value())); + } else { + ctx.push_expression(std::make_unique<nodes::Neg>(std::move(entry))); + } } else if (ctx.get() == '!') { ctx.next(); parse_value(ctx); @@ -672,8 +693,6 @@ void parse_value(ParseContext &ctx) { ctx.next(); parse_expression(ctx); ctx.eat(')'); - } else if (ctx.get() == '[') { - parse_array(ctx); } else if (ctx.get() == '"') { parse_string(ctx); } else if (isdigit(ctx.get())) { @@ -683,7 +702,8 @@ void parse_value(ParseContext &ctx) { } } -void parse_operator(ParseContext &ctx) { +bool parse_operator(ParseContext &ctx) { + bool expect_value = true; ctx.skip_spaces(); vespalib::string &str = ctx.peek(ctx.scratch(), nodes::OperatorRepo::instance().max_size()); Operator_UP op = nodes::OperatorRepo::instance().create(str); @@ -691,24 +711,38 @@ void parse_operator(ParseContext &ctx) { ctx.push_operator(std::move(op)); ctx.skip(str.size()); } else { - ctx.fail(make_string("invalid operator: '%c'", ctx.get())); + vespalib::string ident = get_ident(ctx, true); + if (ident == "in") { + parse_in(ctx); + expect_value = false; + } else { + if (ident.empty()) { + ctx.fail(make_string("invalid operator: '%c'", ctx.get())); + } else { + ctx.fail(make_string("invalid operator: '%s'", ident.c_str())); + } + } } + return expect_value; } void parse_expression(ParseContext &ctx) { size_t old_mark = ctx.operator_mark(); ctx.operator_mark(ctx.num_operators()); + bool expect_value = true; for (;;) { - parse_value(ctx); + if (expect_value) { + parse_value(ctx); + } ctx.skip_spaces(); - if (ctx.eos() || ctx.get() == ')' || ctx.get() == ',' || ctx.get() == ']') { + if (ctx.eos() || ctx.get() == ')' || ctx.get() == ',') { while (ctx.num_operators() > ctx.operator_mark()) { ctx.apply_operator(); } ctx.operator_mark(old_mark); return; } - parse_operator(ctx); + expect_value = parse_operator(ctx); } } diff --git a/eval/src/vespa/eval/eval/gbdt.cpp b/eval/src/vespa/eval/eval/gbdt.cpp index cffd8bdb0c3..0edc42070ba 100644 --- a/eval/src/vespa/eval/eval/gbdt.cpp +++ b/eval/src/vespa/eval/eval/gbdt.cpp @@ -70,13 +70,11 @@ TreeStats::traverse(const nodes::Node &node, size_t depth, size_t &sum_path) { ++num_less_checks; } else { assert(in); - auto symbol = nodes::as<nodes::Symbol>(in->lhs()); + auto symbol = nodes::as<nodes::Symbol>(in->child()); assert(symbol); num_params = std::max(num_params, size_t(symbol->id() + 1)); ++num_in_checks; - auto array = nodes::as<nodes::Array>(in->rhs()); - size_t array_size = (array) ? array->size() : 1; - max_set_size = std::max(max_set_size, array_size); + max_set_size = std::max(max_set_size, in->num_entries()); } return 1.0 + (p_true * true_path) + ((1.0 - p_true) * false_path); } else { diff --git a/eval/src/vespa/eval/eval/interpreted_function.cpp b/eval/src/vespa/eval/eval/interpreted_function.cpp index caf71fcb68b..f99c4ace2dd 100644 --- a/eval/src/vespa/eval/eval/interpreted_function.cpp +++ b/eval/src/vespa/eval/eval/interpreted_function.cpp @@ -65,24 +65,6 @@ void op_skip_if_false(State &state, uint64_t param) { //----------------------------------------------------------------------------- -// compare lhs with a set member, short-circuit if found -void op_check_member(State &state, uint64_t param) { - if (state.peek(1).equal(state.peek(0))) { - state.replace(2, state.stash.create<DoubleValue>(1.0)); - state.program_offset += param; - } else { - state.stack.pop_back(); - } -} - -// set member not found, replace lhs with false -void op_not_member(State &state, uint64_t) { - state.stack.pop_back(); - state.stack.push_back(state.stash.create<DoubleValue>(0.0)); -} - -//----------------------------------------------------------------------------- - void op_double_map(State &state, uint64_t param) { state.replace(1, state.stash.create<DoubleValue>(to_map_fun(param)(state.peek(0).as_double()))); } @@ -252,8 +234,14 @@ struct ProgramBuilder : public NodeVisitor, public NodeTraverser { void visit(const String &node) override { make_const_op(node, stash.create<DoubleValue>(node.hash())); } - void visit(const Array &node) override { - make_const_op(node, stash.create<DoubleValue>(node.size())); + void visit(const In &node) override { + auto my_in = std::make_unique<In>(std::make_unique<Symbol>(0)); + for (size_t i = 0; i < node.num_entries(); ++i) { + my_in->add_entry(std::make_unique<Number>(node.get_entry(i).get_const_value())); + } + Function my_fun(std::move(my_in), {"x"}); + const auto &token = stash.create<CompileCache::Token::UP>(CompileCache::compile(my_fun, PassParams::SEPARATE)); + make_map_op(node, token.get()->get().get_function<1>()); } void visit(const Neg &node) override { make_map_op(node, operation::Neg::f); @@ -367,26 +355,6 @@ struct ProgramBuilder : public NodeVisitor, public NodeTraverser { void visit(const GreaterEqual &node) override { make_join_op(node, operation::GreaterEqual::f); } - void visit(const In &node) override { - std::vector<size_t> checks; - node.lhs().traverse(*this); - auto array = as<Array>(node.rhs()); - if (array) { - for (size_t i = 0; i < array->size(); ++i) { - array->get(i).traverse(*this); - checks.push_back(program.size()); - program.emplace_back(op_check_member); - } - } else { - node.rhs().traverse(*this); - checks.push_back(program.size()); - program.emplace_back(op_check_member); - } - for (size_t i = 0; i < checks.size(); ++i) { - program[checks[i]].update_param(program.size() - checks[i]); - } - program.emplace_back(op_not_member); - } void visit(const And &node) override { make_join_op(node, operation::And::f); } @@ -472,7 +440,7 @@ struct ProgramBuilder : public NodeVisitor, public NodeTraverser { //------------------------------------------------------------------------- bool open(const Node &node) override { - if (check_type<Array, If, In>(node)) { + if (check_type<If>(node)) { node.accept(*this); return false; } diff --git a/eval/src/vespa/eval/eval/key_gen.cpp b/eval/src/vespa/eval/eval/key_gen.cpp index a745023fcf4..e0494e1fe11 100644 --- a/eval/src/vespa/eval/eval/key_gen.cpp +++ b/eval/src/vespa/eval/eval/key_gen.cpp @@ -25,7 +25,12 @@ struct KeyGen : public NodeVisitor, public NodeTraverser { void visit(const Number &node) override { add_byte( 1); add_double(node.value()); } void visit(const Symbol &node) override { add_byte( 2); add_int(node.id()); } void visit(const String &node) override { add_byte( 3); add_hash(node.hash()); } - void visit(const Array &node) override { add_byte( 4); add_size(node.size()); } + void visit(const In &node) override { add_byte( 4); + add_size(node.num_entries()); + for (size_t i = 0; i < node.num_entries(); ++i) { + add_double(node.get_entry(i).get_const_value()); + } + } void visit(const Neg &) override { add_byte( 5); } void visit(const Not &) override { add_byte( 6); } void visit(const If &node) override { add_byte( 7); add_double(node.p_true()); } @@ -49,7 +54,6 @@ struct KeyGen : public NodeVisitor, public NodeTraverser { void visit(const LessEqual &) override { add_byte(30); } void visit(const Greater &) override { add_byte(31); } void visit(const GreaterEqual &) override { add_byte(32); } - void visit(const In &) override { add_byte(33); } void visit(const And &) override { add_byte(34); } void visit(const Or &) override { add_byte(35); } void visit(const Cos &) override { add_byte(36); } diff --git a/eval/src/vespa/eval/eval/llvm/llvm_wrapper.cpp b/eval/src/vespa/eval/eval/llvm/llvm_wrapper.cpp index 303833e8e6c..9355cf7a4e4 100644 --- a/eval/src/vespa/eval/eval/llvm/llvm_wrapper.cpp +++ b/eval/src/vespa/eval/eval/llvm/llvm_wrapper.cpp @@ -56,9 +56,9 @@ namespace { struct SetMemberHash : PluginState { vespalib::hash_set<double> members; - explicit SetMemberHash(const Array &array) : members(array.size() * 3) { - for (size_t i = 0; i < array.size(); ++i) { - members.insert(array.get(i).get_const_value()); + explicit SetMemberHash(const In &in) : members(in.num_entries() * 3) { + for (size_t i = 0; i < in.num_entries(); ++i) { + members.insert(in.get_entry(i).get_const_value()); } } static bool check_membership(const PluginState *state, double value) { @@ -252,7 +252,7 @@ struct FunctionBuilder : public NodeVisitor, public NodeTraverser { inside_forest = true; forest_end = &node; } - if (check_type<Array, If, In>(node)) { + if (check_type<If>(node)) { node.accept(*this); return false; } @@ -355,9 +355,27 @@ struct FunctionBuilder : public NodeVisitor, public NodeTraverser { void visit(const String &item) override { push_double(item.hash()); } - void visit(const Array &item) override { - // NB: visit not open - push_double(item.size()); + void visit(const In &item) override { + llvm::Value *lhs = pop_double(); + if (item.num_entries() > 8) { + // build call to hash lookup + plugin_state.emplace_back(new SetMemberHash(item)); + void *call_ptr = (void *) SetMemberHash::check_membership; + PluginState *state = plugin_state.back().get(); + llvm::PointerType *funptr_t = make_check_membership_funptr_t(); + llvm::Value *call_fun = builder.CreateIntToPtr(builder.getInt64((uint64_t)call_ptr), funptr_t, "inject_call_addr"); + llvm::Value *ctx = builder.CreateIntToPtr(builder.getInt64((uint64_t)state), builder.getVoidTy()->getPointerTo(), "inject_ctx"); + push(builder.CreateCall(call_fun, {ctx, lhs}, "call_check_membership")); + } else { + // build explicit code to check all set members + llvm::Value *found = builder.getFalse(); + for (size_t i = 0; i < item.num_entries(); ++i) { + llvm::Value *elem = llvm::ConstantFP::get(builder.getDoubleTy(), item.get_entry(i).get_const_value()); + llvm::Value *elem_eq = builder.CreateFCmpOEQ(lhs, elem, "elem_eq"); + found = builder.CreateOr(found, elem_eq, "found"); + } + push(found); + } } void visit(const Neg &) override { llvm::Value *child = pop_double(); @@ -480,38 +498,6 @@ struct FunctionBuilder : public NodeVisitor, public NodeTraverser { llvm::Value *a = pop_double(); push(builder.CreateFCmpOGE(a, b, "cmp_ge_res")); } - void visit(const In &item) override { - // NB: visit not open - item.lhs().traverse(*this); // NB: recursion - llvm::Value *lhs = pop_double(); - auto array = as<Array>(item.rhs()); - if (array) { - if (array->is_const() && array->size() > 8) { - // build call to hash lookup - plugin_state.emplace_back(new SetMemberHash(*array)); - void *call_ptr = (void *) SetMemberHash::check_membership; - PluginState *state = plugin_state.back().get(); - llvm::PointerType *funptr_t = make_check_membership_funptr_t(); - llvm::Value *call_fun = builder.CreateIntToPtr(builder.getInt64((uint64_t)call_ptr), funptr_t, "inject_call_addr"); - llvm::Value *ctx = builder.CreateIntToPtr(builder.getInt64((uint64_t)state), builder.getVoidTy()->getPointerTo(), "inject_ctx"); - push(builder.CreateCall(call_fun, {ctx, lhs}, "call_check_membership")); - } else { - // build explicit code to check all set members - llvm::Value *found = builder.getFalse(); - for (size_t i = 0; i < array->size(); ++i) { - array->get(i).traverse(*this); // NB: recursion - llvm::Value *elem = pop_double(); - llvm::Value *elem_eq = builder.CreateFCmpOEQ(lhs, elem, "elem_eq"); - found = builder.CreateOr(found, elem_eq, "found"); - } - push(found); - } - } else { - item.rhs().traverse(*this); // NB: recursion - llvm::Value *rhs = pop_double(); - push(builder.CreateFCmpOEQ(lhs, rhs, "rhs_eq")); - } - } void visit(const And &) override { llvm::Value *b = pop_bool(); llvm::Value *a = pop_bool(); diff --git a/eval/src/vespa/eval/eval/node_types.cpp b/eval/src/vespa/eval/eval/node_types.cpp index d995c5281c4..f86c3e1a84a 100644 --- a/eval/src/vespa/eval/eval/node_types.cpp +++ b/eval/src/vespa/eval/eval/node_types.cpp @@ -90,9 +90,7 @@ struct TypeResolver : public NodeVisitor, public NodeTraverser { void visit(const String &node) override { bind_type(ValueType::double_type(), node); } - void visit(const Array &node) override { - bind_type(ValueType::double_type(), node); - } + void visit(const In &node) override { resolve_op1(node); } void visit(const Neg &node) override { resolve_op1(node); } void visit(const Not &node) override { resolve_op1(node); } void visit(const If &node) override { @@ -139,9 +137,6 @@ struct TypeResolver : public NodeVisitor, public NodeTraverser { void visit(const LessEqual &node) override { resolve_op2(node); } void visit(const Greater &node) override { resolve_op2(node); } void visit(const GreaterEqual &node) override { resolve_op2(node); } - void visit(const In &node) override { - bind_type(ValueType::double_type(), node); - } void visit(const And &node) override { resolve_op2(node); } void visit(const Or &node) override { resolve_op2(node); } void visit(const Cos &node) override { resolve_op1(node); } diff --git a/eval/src/vespa/eval/eval/node_visitor.h b/eval/src/vespa/eval/eval/node_visitor.h index 91d65ceb7ec..c5a6fd51373 100644 --- a/eval/src/vespa/eval/eval/node_visitor.h +++ b/eval/src/vespa/eval/eval/node_visitor.h @@ -22,7 +22,7 @@ struct NodeVisitor { virtual void visit(const nodes::Number &) = 0; virtual void visit(const nodes::Symbol &) = 0; virtual void visit(const nodes::String &) = 0; - virtual void visit(const nodes::Array &) = 0; + virtual void visit(const nodes::In &) = 0; virtual void visit(const nodes::Neg &) = 0; virtual void visit(const nodes::Not &) = 0; virtual void visit(const nodes::If &) = 0; @@ -50,7 +50,6 @@ struct NodeVisitor { virtual void visit(const nodes::LessEqual &) = 0; virtual void visit(const nodes::Greater &) = 0; virtual void visit(const nodes::GreaterEqual &) = 0; - virtual void visit(const nodes::In &) = 0; virtual void visit(const nodes::And &) = 0; virtual void visit(const nodes::Or &) = 0; @@ -92,7 +91,7 @@ struct EmptyNodeVisitor : NodeVisitor { void visit(const nodes::Number &) override {} void visit(const nodes::Symbol &) override {} void visit(const nodes::String &) override {} - void visit(const nodes::Array &) override {} + void visit(const nodes::In &) override {} void visit(const nodes::Neg &) override {} void visit(const nodes::Not &) override {} void visit(const nodes::If &) override {} @@ -116,7 +115,6 @@ struct EmptyNodeVisitor : NodeVisitor { void visit(const nodes::LessEqual &) override {} void visit(const nodes::Greater &) override {} void visit(const nodes::GreaterEqual &) override {} - void visit(const nodes::In &) override {} void visit(const nodes::And &) override {} void visit(const nodes::Or &) override {} void visit(const nodes::Cos &) override {} diff --git a/eval/src/vespa/eval/eval/operator_nodes.cpp b/eval/src/vespa/eval/eval/operator_nodes.cpp index 11817630da4..4c66268dfa2 100644 --- a/eval/src/vespa/eval/eval/operator_nodes.cpp +++ b/eval/src/vespa/eval/eval/operator_nodes.cpp @@ -37,23 +37,10 @@ OperatorRepo::OperatorRepo() : _map(), _max_size(0) { add(nodes::LessEqual()); add(nodes::Greater()); add(nodes::GreaterEqual()); - add(nodes::In()); add(nodes::And()); add(nodes::Or()); } -vespalib::string -In::dump(DumpContext &ctx) const -{ - vespalib::string str; - str += "("; - str += lhs().dump(ctx); - str += " in "; - str += rhs().dump(ctx); - str += ")"; - return str; -} - } // namespace vespalib::eval::nodes } // namespace vespalib::eval } // namespace vespalib diff --git a/eval/src/vespa/eval/eval/operator_nodes.h b/eval/src/vespa/eval/eval/operator_nodes.h index e4dda484d68..eafd817d42c 100644 --- a/eval/src/vespa/eval/eval/operator_nodes.h +++ b/eval/src/vespa/eval/eval/operator_nodes.h @@ -166,9 +166,6 @@ struct Less : OperatorHelper<Less> { Less() : Helper("<" struct LessEqual : OperatorHelper<LessEqual> { LessEqual() : Helper("<=", 10, LEFT) {}}; struct Greater : OperatorHelper<Greater> { Greater() : Helper(">", 10, LEFT) {}}; struct GreaterEqual : OperatorHelper<GreaterEqual> { GreaterEqual() : Helper(">=", 10, LEFT) {}}; -struct In : OperatorHelper<In> { In() : Helper("in", 10, LEFT) {} - virtual vespalib::string dump(DumpContext &ctx) const override; -}; struct And : OperatorHelper<And> { And() : Helper("&&", 2, LEFT) {}}; struct Or : OperatorHelper<Or> { Or() : Helper("||", 1, LEFT) {}}; diff --git a/eval/src/vespa/eval/eval/simple_tensor.cpp b/eval/src/vespa/eval/eval/simple_tensor.cpp index 75c170d48ba..e39e926708d 100644 --- a/eval/src/vespa/eval/eval/simple_tensor.cpp +++ b/eval/src/vespa/eval/eval/simple_tensor.cpp @@ -611,33 +611,6 @@ SimpleTensor::create(const TensorSpec &spec) return builder.build(); } -bool -SimpleTensor::equal(const SimpleTensor &a, const SimpleTensor &b) -{ - if (a.type() != b.type()) { - return false; - } - TypeAnalyzer type_info(a.type(), b.type()); - View view_a(a, type_info.overlap_a); - View view_b(b, type_info.overlap_b); - const CellRef *pos_a = view_a.refs_begin(); - const CellRef *end_a = view_a.refs_end(); - const CellRef *pos_b = view_b.refs_begin(); - const CellRef *end_b = view_b.refs_end(); - ViewMatcher::CrossCompare cmp(view_a.selector(), view_b.selector()); - while ((pos_a != end_a) && (pos_b != end_b)) { - if (cmp.compare(pos_a->get(), pos_b->get()) != ViewMatcher::CrossCompare::Result::EQUAL) { - return false; - } - if (pos_a->get().value != pos_b->get().value) { - return false; - } - ++pos_a; - ++pos_b; - } - return ((pos_a == end_a) && (pos_b == end_b)); -} - std::unique_ptr<SimpleTensor> SimpleTensor::join(const SimpleTensor &a, const SimpleTensor &b, join_fun_t function) { diff --git a/eval/src/vespa/eval/eval/simple_tensor.h b/eval/src/vespa/eval/eval/simple_tensor.h index ec154ff969a..366796f00d8 100644 --- a/eval/src/vespa/eval/eval/simple_tensor.h +++ b/eval/src/vespa/eval/eval/simple_tensor.h @@ -88,7 +88,6 @@ public: std::unique_ptr<SimpleTensor> reduce(Aggregator &aggr, const std::vector<vespalib::string> &dimensions) const; std::unique_ptr<SimpleTensor> rename(const std::vector<vespalib::string> &from, const std::vector<vespalib::string> &to) const; static std::unique_ptr<SimpleTensor> create(const TensorSpec &spec); - static bool equal(const SimpleTensor &a, const SimpleTensor &b); static std::unique_ptr<SimpleTensor> join(const SimpleTensor &a, const SimpleTensor &b, join_fun_t function); static std::unique_ptr<SimpleTensor> concat(const SimpleTensor &a, const SimpleTensor &b, const vespalib::string &dimension); static void encode(const SimpleTensor &tensor, nbostream &output); diff --git a/eval/src/vespa/eval/eval/simple_tensor_engine.cpp b/eval/src/vespa/eval/eval/simple_tensor_engine.cpp index d69715cab22..21498ca2ff1 100644 --- a/eval/src/vespa/eval/eval/simple_tensor_engine.cpp +++ b/eval/src/vespa/eval/eval/simple_tensor_engine.cpp @@ -47,12 +47,6 @@ SimpleTensorEngine::type_of(const Tensor &tensor) const return to_simple(tensor).type(); } -bool -SimpleTensorEngine::equal(const Tensor &a, const Tensor &b) const -{ - return SimpleTensor::equal(to_simple(a), to_simple(b)); -} - vespalib::string SimpleTensorEngine::to_string(const Tensor &tensor) const { diff --git a/eval/src/vespa/eval/eval/simple_tensor_engine.h b/eval/src/vespa/eval/eval/simple_tensor_engine.h index bc6d0166bd1..c751f2f6b49 100644 --- a/eval/src/vespa/eval/eval/simple_tensor_engine.h +++ b/eval/src/vespa/eval/eval/simple_tensor_engine.h @@ -20,7 +20,6 @@ public: static const TensorEngine &ref() { return _engine; }; ValueType type_of(const Tensor &tensor) const override; - bool equal(const Tensor &a, const Tensor &b) const override; vespalib::string to_string(const Tensor &tensor) const override; TensorSpec to_spec(const Tensor &tensor) const override; diff --git a/eval/src/vespa/eval/eval/tensor.cpp b/eval/src/vespa/eval/eval/tensor.cpp index ed50d33de9b..926606f8e26 100644 --- a/eval/src/vespa/eval/eval/tensor.cpp +++ b/eval/src/vespa/eval/eval/tensor.cpp @@ -2,6 +2,7 @@ #include "tensor.h" #include "tensor_engine.h" +#include "tensor_spec.h" namespace vespalib { namespace eval { @@ -9,7 +10,9 @@ namespace eval { bool operator==(const Tensor &lhs, const Tensor &rhs) { - return ((&lhs.engine() == &rhs.engine()) && lhs.engine().equal(lhs, rhs)); + auto lhs_spec = lhs.engine().to_spec(lhs); + auto rhs_spec = rhs.engine().to_spec(rhs); + return (lhs_spec == rhs_spec); } std::ostream & diff --git a/eval/src/vespa/eval/eval/tensor_engine.h b/eval/src/vespa/eval/eval/tensor_engine.h index d33c1ba0ed2..00927f0c1b1 100644 --- a/eval/src/vespa/eval/eval/tensor_engine.h +++ b/eval/src/vespa/eval/eval/tensor_engine.h @@ -41,7 +41,6 @@ struct TensorEngine using Aggr = eval::Aggr; virtual ValueType type_of(const Tensor &tensor) const = 0; - virtual bool equal(const Tensor &a, const Tensor &b) const = 0; virtual vespalib::string to_string(const Tensor &tensor) const = 0; virtual TensorSpec to_spec(const Tensor &tensor) const = 0; diff --git a/eval/src/vespa/eval/eval/test/eval_spec.cpp b/eval/src/vespa/eval/eval/test/eval_spec.cpp index c7c0d754976..d214486cf21 100644 --- a/eval/src/vespa/eval/eval/test/eval_spec.cpp +++ b/eval/src/vespa/eval/eval/test/eval_spec.cpp @@ -104,12 +104,6 @@ EvalSpec::add_terminal_cases() { add_expression({}, "10").add_case({}, 10.0); add_expression({}, "100").add_case({}, 100.0); add_rule({"a", -5.0, 5.0}, "a", [](double a){ return a; }); - add_expression({}, "[]").add_case({}, 0.0); - add_expression({}, "[1]").add_case({}, 1.0); - add_expression({}, "[1,2]").add_case({}, 2.0); - add_expression({}, "[1,2,3]").add_case({}, 3.0); - add_expression({}, "[3,2,1]").add_case({}, 3.0); - add_expression({}, "[1,1,1,1,1]").add_case({}, 5.0); add_expression({}, "\"\"").add_case({}, vespalib::hash_code("")); add_expression({}, "\"foo\"").add_case({}, vespalib::hash_code("foo")); add_expression({}, "\"foo bar baz\"").add_case({}, vespalib::hash_code("foo bar baz")); @@ -277,52 +271,27 @@ EvalSpec::add_set_membership_cases() { add_expression({"a"}, "(a in [])") .add_case({0.0}, 0.0) - .add_case({1.0}, 0.0) - .add_case({2.0}, 0.0); - - add_expression({"a"}, "(a in [[]])") - .add_case({0.0}, 1.0) - .add_case({1.0}, 0.0) - .add_case({2.0}, 0.0); - - add_expression({"a"}, "(a in [[[]]])") - .add_case({0.0}, 0.0) - .add_case({1.0}, 1.0) - .add_case({2.0}, 0.0); - - add_expression({"a", "b"}, "(a in b)") - .add_case({my_nan, 2.0}, 0.0) - .add_case({2.0, my_nan}, 0.0) - .add_case({my_nan, my_nan}, 0.0) - .add_case({1.0, 2.0}, 0.0) - .add_case({2.0 - 1e-10, 2.0}, 0.0) - .add_case({2.0, 2.0}, 1.0) - .add_case({2.0 + 1e-10, 2.0}, 0.0) - .add_case({3.0, 2.0}, 0.0); - - add_expression({"a", "b"}, "(a in [b])") - .add_case({my_nan, 2.0}, 0.0) - .add_case({2.0, my_nan}, 0.0) - .add_case({my_nan, my_nan}, 0.0) - .add_case({1.0, 2.0}, 0.0) - .add_case({2.0 - 1e-10, 2.0}, 0.0) - .add_case({2.0, 2.0}, 1.0) - .add_case({2.0 + 1e-10, 2.0}, 0.0) - .add_case({3.0, 2.0}, 0.0); - - add_expression({"a", "b"}, "(a in [[b]])") - .add_case({1.0, 2.0}, 1.0) - .add_case({2.0, 2.0}, 0.0); - - add_expression({"a", "b", "c", "d"}, "(a in [b,c,d])") - .add_case({0.0, 10.0, 20.0, 30.0}, 0.0) - .add_case({3.0, 10.0, 20.0, 30.0}, 0.0) - .add_case({10.0, 10.0, 20.0, 30.0}, 1.0) - .add_case({20.0, 10.0, 20.0, 30.0}, 1.0) - .add_case({30.0, 10.0, 20.0, 30.0}, 1.0) - .add_case({10.0, 30.0, 20.0, 10.0}, 1.0) - .add_case({20.0, 30.0, 20.0, 10.0}, 1.0) - .add_case({30.0, 30.0, 20.0, 10.0}, 1.0); + .add_case({1.0}, 0.0); + + add_expression({"a"}, "(a in [2.0])") + .add_case({my_nan}, 0.0) + .add_case({1.0}, 0.0) + .add_case({2.0 - 1e-10}, 0.0) + .add_case({2.0}, 1.0) + .add_case({2.0 + 1e-10}, 0.0) + .add_case({3.0}, 0.0); + + add_expression({"a"}, "(a in [10,20,30])") + .add_case({0.0}, 0.0) + .add_case({3.0}, 0.0) + .add_case({10.0}, 1.0) + .add_case({20.0}, 1.0) + .add_case({30.0}, 1.0); + + add_expression({"a"}, "(a in [30,20,10])") + .add_case({10.0}, 1.0) + .add_case({20.0}, 1.0) + .add_case({30.0}, 1.0); } void diff --git a/eval/src/vespa/eval/eval/test/tensor_conformance.cpp b/eval/src/vespa/eval/eval/test/tensor_conformance.cpp index 617aa75c945..e0a9f731804 100644 --- a/eval/src/vespa/eval/eval/test/tensor_conformance.cpp +++ b/eval/src/vespa/eval/eval/test/tensor_conformance.cpp @@ -395,63 +395,6 @@ struct TestContext { //------------------------------------------------------------------------- - void verify_equal(const TensorSpec &a, const TensorSpec &b) { - auto ta = tensor(a); - auto tb = tensor(b); - EXPECT_EQUAL(a, b); - EXPECT_EQUAL(*ta, *tb); - TensorSpec spec = engine.to_spec(*ta); - TensorSpec ref_spec = ref_engine.to_spec(*ref_engine.create(a)); - EXPECT_EQUAL(spec, ref_spec); - } - - void test_tensor_equality() { - TEST_DO(verify_equal(spec(), spec())); - TEST_DO(verify_equal(spec(10.0), spec(10.0))); - TEST_DO(verify_equal(spec(x()), spec(x()))); - TEST_DO(verify_equal(spec(x({"a"}), Seq({1})), spec(x({"a"}), Seq({1})))); - TEST_DO(verify_equal(spec({x({"a"}),y({"a"})}, Seq({1})), spec({y({"a"}),x({"a"})}, Seq({1})))); - TEST_DO(verify_equal(spec(x(3)), spec(x(3)))); - TEST_DO(verify_equal(spec({x(1),y(1)}, Seq({1})), spec({y(1),x(1)}, Seq({1})))); - TEST_DO(verify_equal(spec({x({"a"}),y(1)}, Seq({1})), spec({y(1),x({"a"})}, Seq({1})))); - TEST_DO(verify_equal(spec({y({"a"}),x(1)}, Seq({1})), spec({x(1),y({"a"})}, Seq({1})))); - } - - //------------------------------------------------------------------------- - - void verify_not_equal(const TensorSpec &a, const TensorSpec &b) { - auto ta = tensor(a); - auto tb = tensor(b); - EXPECT_NOT_EQUAL(a, b); - EXPECT_NOT_EQUAL(b, a); - EXPECT_NOT_EQUAL(*ta, *tb); - EXPECT_NOT_EQUAL(*tb, *ta); - } - - void test_tensor_inequality() { - TEST_DO(verify_not_equal(spec(1.0), spec(2.0))); - TEST_DO(verify_not_equal(spec(), spec(x()))); - TEST_DO(verify_not_equal(spec(), spec(x(1)))); - TEST_DO(verify_not_equal(spec(x()), spec(x(1)))); - TEST_DO(verify_not_equal(spec(x()), spec(y()))); - TEST_DO(verify_not_equal(spec(x(1)), spec(x(2)))); - TEST_DO(verify_not_equal(spec(x(1)), spec(y(1)))); - TEST_DO(verify_not_equal(spec(x({"a"}), Seq({1})), spec(x({"a"}), Seq({2})))); - TEST_DO(verify_not_equal(spec(x({"a"}), Seq({1})), spec(x({"b"}), Seq({1})))); - TEST_DO(verify_not_equal(spec(x({"a"}), Seq({1})), spec({x({"a"}),y({"a"})}, Seq({1})))); - TEST_DO(verify_not_equal(spec(x(1), Seq({1})), spec(x(1), Seq({2})))); - TEST_DO(verify_not_equal(spec(x(1), Seq({1})), spec(x(2), Seq({1}), Bits({1,0})))); - TEST_DO(verify_not_equal(spec(x(2), Seq({1,1}), Bits({1,0})), - spec(x(2), Seq({1,1}), Bits({0,1})))); - TEST_DO(verify_not_equal(spec(x(1), Seq({1})), spec({x(1),y(1)}, Seq({1})))); - TEST_DO(verify_not_equal(spec({x({"a"}),y(1)}, Seq({1})), spec({x({"a"}),y(1)}, Seq({2})))); - TEST_DO(verify_not_equal(spec({x({"a"}),y(1)}, Seq({1})), spec({x({"b"}),y(1)}, Seq({1})))); - TEST_DO(verify_not_equal(spec({x(2),y({"a"})}, Seq({1}), Bits({1,0})), - spec({x(2),y({"a"})}, Seq({X,1}), Bits({0,1})))); - } - - //------------------------------------------------------------------------- - void verify_reduce_result(const Eval &eval, const TensorSpec &a, const Eval::Result &expect) { TEST_DO(verify_result(eval.eval(engine, a), expect)); } @@ -548,6 +491,7 @@ struct TestContext { TEST_DO(test_map_op("isNan(a)", operation::IsNan::f, Mask2Seq(SkipNth(3), 1.0, my_nan))); TEST_DO(test_map_op("relu(a)", operation::Relu::f, Sub2(Div10(N())))); TEST_DO(test_map_op("sigmoid(a)", operation::Sigmoid::f, Sub2(Div10(N())))); + TEST_DO(test_map_op("a in [1,5,7,13,42]", MyIn::f, N())); TEST_DO(test_map_op("(a+1)*2", MyOp::f, Div10(N()))); } @@ -989,8 +933,6 @@ struct TestContext { void run_tests() { TEST_DO(test_tensor_create_type()); - TEST_DO(test_tensor_equality()); - TEST_DO(test_tensor_inequality()); TEST_DO(test_tensor_reduce()); TEST_DO(test_tensor_map()); TEST_DO(test_tensor_apply()); diff --git a/eval/src/vespa/eval/eval/test/tensor_model.hpp b/eval/src/vespa/eval/eval/test/tensor_model.hpp index 761d7c751aa..50a7b6a639a 100644 --- a/eval/src/vespa/eval/eval/test/tensor_model.hpp +++ b/eval/src/vespa/eval/eval/test/tensor_model.hpp @@ -115,6 +115,22 @@ struct MyOp { } }; +// 'a in [1,5,7,13,42]' +struct MyIn { + static double f(double a) { + if ((a == 1) || + (a == 5) || + (a == 7) || + (a == 13) || + (a == 42)) + { + return 1.0; + } else { + return 0.0; + } + } +}; + // A collection of labels for a single dimension struct Domain { vespalib::string dimension; diff --git a/eval/src/vespa/eval/eval/value.cpp b/eval/src/vespa/eval/eval/value.cpp index 0118d95e5cb..456d80c0ff0 100644 --- a/eval/src/vespa/eval/eval/value.cpp +++ b/eval/src/vespa/eval/eval/value.cpp @@ -14,12 +14,6 @@ TensorValue::as_double() const return _tensor->as_double(); } -bool -TensorValue::equal(const Value &rhs) const -{ - return (rhs.is_tensor() && _tensor->engine().equal(*_tensor, *rhs.as_tensor())); -} - ValueType TensorValue::type() const { diff --git a/eval/src/vespa/eval/eval/value.h b/eval/src/vespa/eval/eval/value.h index 0d727db6b91..8826faed140 100644 --- a/eval/src/vespa/eval/eval/value.h +++ b/eval/src/vespa/eval/eval/value.h @@ -27,7 +27,6 @@ struct Value { virtual double as_double() const { return 0.0; } virtual bool as_bool() const { return false; } virtual const Tensor *as_tensor() const { return nullptr; } - virtual bool equal(const Value &rhs) const = 0; virtual ValueType type() const = 0; virtual ~Value() {} }; @@ -36,7 +35,6 @@ struct ErrorValue : public Value { static ErrorValue instance; bool is_error() const override { return true; } double as_double() const override { return error_value; } - bool equal(const Value &) const override { return false; } ValueType type() const override { return ValueType::error_type(); } }; @@ -49,9 +47,6 @@ public: bool is_double() const override { return true; } double as_double() const override { return _value; } bool as_bool() const override { return (_value != 0.0); } - bool equal(const Value &rhs) const override { - return (rhs.is_double() && (_value == rhs.as_double())); - } ValueType type() const override { return ValueType::double_type(); } }; @@ -66,7 +61,6 @@ public: bool is_tensor() const override { return true; } double as_double() const override; const Tensor *as_tensor() const override { return _tensor; } - bool equal(const Value &rhs) const override; ValueType type() const override; }; diff --git a/eval/src/vespa/eval/eval/vm_forest.cpp b/eval/src/vespa/eval/eval/vm_forest.cpp index 4a73394e354..c456c660af7 100644 --- a/eval/src/vespa/eval/eval/vm_forest.cpp +++ b/eval/src/vespa/eval/eval/vm_forest.cpp @@ -128,20 +128,13 @@ void encode_in(const nodes::In &in, std::vector<uint32_t> &model_out) { size_t meta_idx = model_out.size(); - auto symbol = nodes::as<nodes::Symbol>(in.lhs()); + auto symbol = nodes::as<nodes::Symbol>(in.child()); assert(symbol); model_out.push_back(uint32_t(symbol->id()) << 12); - assert(in.rhs().is_const()); - auto array = nodes::as<nodes::Array>(in.rhs()); size_t set_size_idx = model_out.size(); - if (array) { - model_out.push_back(array->size()); - for (size_t i = 0; i < array->size(); ++i) { - encode_const(array->get(i).get_const_value(), model_out); - } - } else { - model_out.push_back(1); - encode_const(in.rhs().get_const_value(), model_out); + model_out.push_back(in.num_entries()); + for (size_t i = 0; i < in.num_entries(); ++i) { + encode_const(in.get_entry(i).get_const_value(), model_out); } size_t left_idx = model_out.size(); uint32_t left_type = encode_node(left_child, model_out); diff --git a/eval/src/vespa/eval/tensor/default_tensor_engine.cpp b/eval/src/vespa/eval/tensor/default_tensor_engine.cpp index 2082b7efd25..7adb95f69ca 100644 --- a/eval/src/vespa/eval/tensor/default_tensor_engine.cpp +++ b/eval/src/vespa/eval/tensor/default_tensor_engine.cpp @@ -97,16 +97,6 @@ DefaultTensorEngine::type_of(const Tensor &tensor) const return my_tensor.getType(); } -bool -DefaultTensorEngine::equal(const Tensor &a, const Tensor &b) const -{ - assert(&a.engine() == this); - assert(&b.engine() == this); - const tensor::Tensor &my_a = static_cast<const tensor::Tensor &>(a); - const tensor::Tensor &my_b = static_cast<const tensor::Tensor &>(b); - return my_a.equals(my_b); -} - vespalib::string DefaultTensorEngine::to_string(const Tensor &tensor) const { diff --git a/eval/src/vespa/eval/tensor/default_tensor_engine.h b/eval/src/vespa/eval/tensor/default_tensor_engine.h index abdce6edb62..bbb03aceb1f 100644 --- a/eval/src/vespa/eval/tensor/default_tensor_engine.h +++ b/eval/src/vespa/eval/tensor/default_tensor_engine.h @@ -20,7 +20,6 @@ public: static const TensorEngine &ref() { return _engine; }; ValueType type_of(const Tensor &tensor) const override; - bool equal(const Tensor &a, const Tensor &b) const override; vespalib::string to_string(const Tensor &tensor) const override; TensorSpec to_spec(const Tensor &tensor) const override; diff --git a/eval/src/vespa/eval/tensor/wrapped_simple_tensor.cpp b/eval/src/vespa/eval/tensor/wrapped_simple_tensor.cpp index a407a46610b..534854732c7 100644 --- a/eval/src/vespa/eval/tensor/wrapped_simple_tensor.cpp +++ b/eval/src/vespa/eval/tensor/wrapped_simple_tensor.cpp @@ -4,6 +4,7 @@ #include "tensor_address_builder.h" #include "tensor_visitor.h" #include <vespa/eval/eval/simple_tensor_engine.h> +#include <vespa/eval/eval/tensor_spec.h> #include <vespa/vespalib/util/stringfmt.h> namespace vespalib::tensor { @@ -11,10 +12,9 @@ namespace vespalib::tensor { bool WrappedSimpleTensor::equals(const Tensor &arg) const { - if (auto other = dynamic_cast<const WrappedSimpleTensor *>(&arg)) { - return eval::SimpleTensor::equal(_tensor, other->_tensor); - } - return false; + auto lhs_spec = _tensor.engine().to_spec(_tensor); + auto rhs_spec = arg.engine().to_spec(arg); + return (lhs_spec == rhs_spec); } vespalib::string diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp index 906f71fecb4..7413d0f369f 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp @@ -143,7 +143,15 @@ applyCompactLidSpace(uint32_t wantedLidLimit, SerialNum serialNum, AttributeVector &attr) { if (attr.getStatus().getLastSyncToken() < serialNum) { - attr.compactLidSpace(wantedLidLimit); + /* + * If the attribute is an empty placeholder attribute due to + * later config changes removing the attribute then it might + * be smaller than expected during transaction log replay. + */ + attr.commit(); + if (wantedLidLimit <= attr.getCommittedDocIdLimit()) { + attr.compactLidSpace(wantedLidLimit); + } attr.commit(serialNum, serialNum); } } diff --git a/searchcore/src/vespa/searchcore/proton/server/proton.cpp b/searchcore/src/vespa/searchcore/proton/server/proton.cpp index aa1c8483c8d..cd016e5cfc4 100644 --- a/searchcore/src/vespa/searchcore/proton/server/proton.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/proton.cpp @@ -311,14 +311,14 @@ Proton::init(const BootstrapConfig::SP & configSnapshot) RPCHooks::Params rpcParams(*this, protonConfig.rpcport, _configUri.getConfigId()); rpcParams.slobrok_config = _configUri.createWithNewId(protonConfig.slobrokconfigid); _rpcHooks.reset(new RPCHooks(rpcParams)); - + + waitForInitDone(); + _metricsEngine->start(_configUri); _stateServer.reset(new vespalib::StateServer(protonConfig.httpport, _healthAdapter, _metricsEngine->metrics_producer(), *this)); _customComponentBindToken = _stateServer->repo().bind(CUSTOM_COMPONENT_API_PATH, _genericStateHandler); _customComponentRootToken = _stateServer->repo().add_root_resource(CUSTOM_COMPONENT_API_PATH); - waitForInitDone(); - _executor.sync(); waitForOnlineState(); _isReplayDone = true; diff --git a/searchlib/src/apps/vespa-ranking-expression-analyzer/vespa-ranking-expression-analyzer.cpp b/searchlib/src/apps/vespa-ranking-expression-analyzer/vespa-ranking-expression-analyzer.cpp index 550cd651c2b..7e8b5a85448 100644 --- a/searchlib/src/apps/vespa-ranking-expression-analyzer/vespa-ranking-expression-analyzer.cpp +++ b/searchlib/src/apps/vespa-ranking-expression-analyzer/vespa-ranking-expression-analyzer.cpp @@ -105,21 +105,11 @@ struct FunctionInfo { void check_in(const In *node) { if (node) { - auto lhs_symbol = as<Symbol>(node->lhs()); - auto rhs_symbol = as<Symbol>(node->rhs()); - if (lhs_symbol && node->rhs().is_const()) { - auto array = as<Array>(node->rhs()); - if (array) { - for (size_t i = 0; i < array->size(); ++i) { - inputs[lhs_symbol->id()].cmp_with.push_back(array->get(i).get_const_value()); - } - } else { - inputs[lhs_symbol->id()].cmp_with.push_back(node->rhs().get_const_value()); + if (auto symbol = as<Symbol>(node->child())) { + for (size_t i = 0; i < node->num_entries(); ++i) { + inputs[symbol->id()].cmp_with.push_back(node->get_entry(i).get_const_value()); } } - if (node->lhs().is_const() && rhs_symbol) { - inputs[rhs_symbol->id()].cmp_with.push_back(node->lhs().get_const_value()); - } } } diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/internal/SuperModelListenerImpl.java b/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/internal/SuperModelListenerImpl.java index b44d73125bd..82d55cd05d7 100644 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/internal/SuperModelListenerImpl.java +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/internal/SuperModelListenerImpl.java @@ -44,10 +44,9 @@ public class SuperModelListenerImpl implements SuperModelListener, Supplier<Serv // This snapshot() call needs to be within the synchronized block, // since applicationActivated()/applicationRemoved() may be called // asynchronously even before snapshot() returns. - SuperModel snapshot = superModelProvider.snapshot(this); - - snapshot.getAllApplicationInfos().stream().forEach(application -> - applicationActivated(snapshot, application)); + this.superModel = superModelProvider.snapshot(this); + superModel.getAllApplicationInfos().stream().forEach(application -> + slobrokMonitorManager.applicationActivated(superModel, application)); } } @@ -85,4 +84,4 @@ public class SuperModelListenerImpl implements SuperModelListener, Supplier<Serv } private void dummy(LatencyMeasurement measurement) {} -}
\ No newline at end of file +} diff --git a/storage/src/tests/distributor/bucketdbupdatertest.cpp b/storage/src/tests/distributor/bucketdbupdatertest.cpp index f9578fc47a4..d1a54c04359 100644 --- a/storage/src/tests/distributor/bucketdbupdatertest.cpp +++ b/storage/src/tests/distributor/bucketdbupdatertest.cpp @@ -176,7 +176,7 @@ public: for (int i=0; i<bucketCount + invalidBucketCount; i++) { if (!getBucketDBUpdater().getDistributorComponent() - .ownsBucketInState(state, document::BucketId(16, i))) { + .ownsBucketInState(state, makeDocumentBucket(document::BucketId(16, i)))) { continue; } @@ -1963,7 +1963,7 @@ BucketDBUpdaterTest::testNoDbResurrectionForBucketNotOwnedInCurrentState() setAndEnableClusterState(stateAfter, expectedMsgs, dummyBucketsToReturn); } CPPUNIT_ASSERT(!getBucketDBUpdater().getDistributorComponent() - .ownsBucketInCurrentState(bucket)); + .ownsBucketInCurrentState(makeDocumentBucket(bucket))); sendFakeReplyForSingleBucketRequest(*rbi); @@ -1992,7 +1992,7 @@ BucketDBUpdaterTest::testNoDbResurrectionForBucketNotOwnedInPendingState() // Set, but _don't_ enable cluster state. We want it to be pending. setSystemState(stateAfter); CPPUNIT_ASSERT(getBucketDBUpdater().getDistributorComponent() - .ownsBucketInCurrentState(bucket)); + .ownsBucketInCurrentState(makeDocumentBucket(bucket))); CPPUNIT_ASSERT(!getBucketDBUpdater() .checkOwnershipInPendingState(bucket).isOwned()); @@ -2125,7 +2125,7 @@ BucketDBUpdaterTest::testNewerMutationsNotOverwrittenByEarlierBucketFetch() constexpr uint64_t insertionTimestamp = 1001ULL * 1000000; api::BucketInfo wantedInfo(5, 6, 7); getBucketDBUpdater().getDistributorComponent().updateBucketDatabase( - bucket, + makeDocumentBucket(bucket), BucketCopy(insertionTimestamp, 0, wantedInfo), DatabaseUpdate::CREATE_IF_NONEXISTING); diff --git a/storage/src/tests/distributor/distributortest.cpp b/storage/src/tests/distributor/distributortest.cpp index 4be9735c0f7..cbc78157911 100644 --- a/storage/src/tests/distributor/distributortest.cpp +++ b/storage/src/tests/distributor/distributortest.cpp @@ -170,12 +170,12 @@ private: } } - getExternalOperationHandler().removeNodesFromDB(document::BucketId(16, 1), removedNodes); + getExternalOperationHandler().removeNodesFromDB(makeDocumentBucket(document::BucketId(16, 1)), removedNodes); uint32_t flags(DatabaseUpdate::CREATE_IF_NONEXISTING | (resetTrusted ? DatabaseUpdate::RESET_TRUSTED : 0)); - getExternalOperationHandler().updateBucketDatabase(document::BucketId(16, 1), + getExternalOperationHandler().updateBucketDatabase(makeDocumentBucket(document::BucketId(16, 1)), changedNodes, flags); } @@ -558,12 +558,12 @@ Distributor_Test::testNoDbResurrectionForBucketNotOwnedInPendingState() CPPUNIT_ASSERT(!getBucketDBUpdater() .checkOwnershipInPendingState(nonOwnedBucket).isOwned()); CPPUNIT_ASSERT(!getBucketDBUpdater().getDistributorComponent() - .checkOwnershipInPendingAndCurrentState(nonOwnedBucket) + .checkOwnershipInPendingAndCurrentState(makeDocumentBucket(nonOwnedBucket)) .isOwned()); std::vector<BucketCopy> copies; copies.emplace_back(1234, 0, api::BucketInfo(0x567, 1, 2)); - getExternalOperationHandler().updateBucketDatabase(nonOwnedBucket, copies, + getExternalOperationHandler().updateBucketDatabase(makeDocumentBucket(nonOwnedBucket), copies, DatabaseUpdate::CREATE_IF_NONEXISTING); CPPUNIT_ASSERT_EQUAL(std::string("NONEXISTING"), @@ -579,7 +579,7 @@ Distributor_Test::testAddedDbBucketsWithoutGcTimestampImplicitlyGetCurrentTime() std::vector<BucketCopy> copies; copies.emplace_back(1234, 0, api::BucketInfo(0x567, 1, 2)); - getExternalOperationHandler().updateBucketDatabase(bucket, copies, + getExternalOperationHandler().updateBucketDatabase(makeDocumentBucket(bucket), copies, DatabaseUpdate::CREATE_IF_NONEXISTING); BucketDatabase::Entry e(getBucket(bucket)); CPPUNIT_ASSERT_EQUAL(uint32_t(101234), e->getLastGarbageCollectionTime()); diff --git a/storage/src/tests/distributor/distributortestutil.cpp b/storage/src/tests/distributor/distributortestutil.cpp index 2a44ad6d52b..5deb31f8579 100644 --- a/storage/src/tests/distributor/distributortestutil.cpp +++ b/storage/src/tests/distributor/distributortestutil.cpp @@ -3,6 +3,9 @@ #include <vespa/storage/distributor/distributor.h> #include <vespa/config-stor-distribution.h> #include <vespa/vespalib/text/stringtokenizer.h> +#include <vespa/document/test/make_document_bucket.h> + +using document::test::makeDocumentBucket; namespace storage::distributor { @@ -127,7 +130,7 @@ DistributorTestUtil::getNodes(document::BucketId id) std::string DistributorTestUtil::getIdealStr(document::BucketId id, const lib::ClusterState& state) { - if (!getExternalOperationHandler().ownsBucketInState(state, id)) { + if (!getExternalOperationHandler().ownsBucketInState(state, makeDocumentBucket(id))) { return id.toString(); } @@ -340,6 +343,12 @@ DistributorTestUtil::getConfig() { return const_cast<DistributorConfiguration&>(_distributor->getConfig()); } +DistributorBucketSpace & +DistributorTestUtil::getDistributorBucketSpace() +{ + return _distributor->getDefaultBucketSpace(); +} + BucketDatabase& DistributorTestUtil::getBucketDatabase() { return _distributor->getDefaultBucketSpace().getBucketDatabase(); diff --git a/storage/src/tests/distributor/distributortestutil.h b/storage/src/tests/distributor/distributortestutil.h index 4de84e47bfe..4f09c11ac03 100644 --- a/storage/src/tests/distributor/distributortestutil.h +++ b/storage/src/tests/distributor/distributortestutil.h @@ -19,6 +19,7 @@ namespace distributor { class BucketDBUpdater; class Distributor; +class DistributorBucketSpace; class IdealStateManager; class ExternalOperationHandler; class Operation; @@ -121,6 +122,7 @@ public: } // TODO explicit notion of bucket spaces for tests + DistributorBucketSpace &getDistributorBucketSpace(); BucketDatabase& getBucketDatabase(); const BucketDatabase& getBucketDatabase() const; diff --git a/storage/src/tests/distributor/externaloperationhandlertest.cpp b/storage/src/tests/distributor/externaloperationhandlertest.cpp index 6cf8d068231..683352e6b09 100644 --- a/storage/src/tests/distributor/externaloperationhandlertest.cpp +++ b/storage/src/tests/distributor/externaloperationhandlertest.cpp @@ -153,7 +153,7 @@ ExternalOperationHandlerTest::findNonOwnedUserBucketInState( lib::ClusterState state(statestr); for (uint64_t i = 1; i < 1000; ++i) { document::BucketId bucket(32, i); - if (!getExternalOperationHandler().ownsBucketInState(state, bucket)) { + if (!getExternalOperationHandler().ownsBucketInState(state, makeDocumentBucket(bucket))) { return bucket; } } @@ -169,8 +169,8 @@ ExternalOperationHandlerTest::findOwned1stNotOwned2ndInStates( lib::ClusterState state2(statestr2); for (uint64_t i = 1; i < 1000; ++i) { document::BucketId bucket(32, i); - if (getExternalOperationHandler().ownsBucketInState(state1, bucket) - && !getExternalOperationHandler().ownsBucketInState(state2, bucket)) + if (getExternalOperationHandler().ownsBucketInState(state1, makeDocumentBucket(bucket)) + && !getExternalOperationHandler().ownsBucketInState(state2, makeDocumentBucket(bucket))) { return bucket; } diff --git a/storage/src/tests/distributor/getoperationtest.cpp b/storage/src/tests/distributor/getoperationtest.cpp index ee1ea70163f..8bb8e24c17a 100644 --- a/storage/src/tests/distributor/getoperationtest.cpp +++ b/storage/src/tests/distributor/getoperationtest.cpp @@ -75,6 +75,7 @@ public: new api::GetCommand(makeDocumentBucket(document::BucketId(0)), docId, "[all]")); op.reset(new GetOperation(getExternalOperationHandler(), + getDistributorBucketSpace(), msg, getDistributor().getMetrics(). gets[msg->getLoadType()])); diff --git a/storage/src/tests/distributor/putoperationtest.cpp b/storage/src/tests/distributor/putoperationtest.cpp index 5cc9a26a9ea..7f54e163006 100644 --- a/storage/src/tests/distributor/putoperationtest.cpp +++ b/storage/src/tests/distributor/putoperationtest.cpp @@ -131,6 +131,7 @@ public: void sendPut(std::shared_ptr<api::PutCommand> msg) { op.reset(new PutOperation(getExternalOperationHandler(), + getDistributorBucketSpace(), msg, getDistributor().getMetrics(). puts[msg->getLoadType()])); @@ -273,7 +274,7 @@ PutOperationTest::testNodeRemovedOnReply() "doc:test:test, timestamp 100, size 33) => 0"), _sender.getCommands(true, true)); - getExternalOperationHandler().removeNodeFromDB(document::BucketId(16, 0x8b13), 0); + getExternalOperationHandler().removeNodeFromDB(makeDocumentBucket(document::BucketId(16, 0x8b13)), 0); sendReply(0); sendReply(1); @@ -281,7 +282,7 @@ PutOperationTest::testNodeRemovedOnReply() CPPUNIT_ASSERT_EQUAL(std::string( "PutReply(doc:test:test, BucketId(0x0000000000000000), " "timestamp 100) ReturnCode(BUCKET_DELETED, " - "BucketId(0x4000000000008b13) was deleted from nodes [0] " + "Bucket(BucketSpace(0x0000000000000000), BucketId(0x4000000000008b13)) was deleted from nodes [0] " "after message was sent but before it was done. " "Sent to [1,0])"), _sender.getLastReply()); @@ -596,7 +597,7 @@ PutOperationTest::getNodes(const std::string& infoString) { std::vector<uint16_t> targetNodes; std::vector<uint16_t> createNodes; - PutOperation::getTargetNodes(getExternalOperationHandler().getIdealNodes(bid), + PutOperation::getTargetNodes(getExternalOperationHandler().getIdealNodes(makeDocumentBucket(bid)), targetNodes, createNodes, entry, 2); ost << "target( "; diff --git a/storage/src/tests/distributor/removelocationtest.cpp b/storage/src/tests/distributor/removelocationtest.cpp index 8c55b7e715c..52612048daa 100644 --- a/storage/src/tests/distributor/removelocationtest.cpp +++ b/storage/src/tests/distributor/removelocationtest.cpp @@ -40,6 +40,7 @@ public: new api::RemoveLocationCommand(selection, makeDocumentBucket(document::BucketId(0)))); op.reset(new RemoveLocationOperation(getExternalOperationHandler(), + getDistributorBucketSpace(), msg, getDistributor().getMetrics(). removelocations[msg->getLoadType()])); diff --git a/storage/src/tests/distributor/removeoperationtest.cpp b/storage/src/tests/distributor/removeoperationtest.cpp index 4c2f1bba00d..423e0816c13 100644 --- a/storage/src/tests/distributor/removeoperationtest.cpp +++ b/storage/src/tests/distributor/removeoperationtest.cpp @@ -58,6 +58,7 @@ public: new api::RemoveCommand(makeDocumentBucket(document::BucketId(0)), dId, 100)); op.reset(new RemoveOperation(getExternalOperationHandler(), + getDistributorBucketSpace(), msg, getDistributor().getMetrics(). removes[msg->getLoadType()])); diff --git a/storage/src/tests/distributor/statecheckerstest.cpp b/storage/src/tests/distributor/statecheckerstest.cpp index 29c922248e7..e71525d5dd6 100644 --- a/storage/src/tests/distributor/statecheckerstest.cpp +++ b/storage/src/tests/distributor/statecheckerstest.cpp @@ -13,6 +13,7 @@ #include <vespa/storage/distributor/operations/idealstate/setbucketstateoperation.h> #include <vespa/storage/distributor/operations/idealstate/splitoperation.h> #include <vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.h> +#include <vespa/storage/distributor/distributor_bucket_space_repo.h> #include <vespa/storageapi/message/stat.h> #include <vespa/storage/storageutil/utils.h> #include <tests/distributor/distributortestutil.h> @@ -20,8 +21,12 @@ #include <vespa/storageapi/message/state.h> #include <vespa/config-stor-distribution.h> #include <vespa/storage/distributor/distributor.h> +#include <vespa/document/test/make_bucket_space.h> +#include <vespa/document/test/make_document_bucket.h> using namespace std::literals::string_literals; +using document::test::makeBucketSpace; +using document::test::makeDocumentBucket; namespace storage { namespace distributor { @@ -105,8 +110,9 @@ struct StateCheckersTest : public CppUnit::TestFixture, void assertCurrentIdealState(const document::BucketId& bucket, const std::vector<uint16_t> expected) { + auto &distributorBucketSpace(getIdealStateManager().getBucketSpaceRepo().get(makeBucketSpace())); std::vector<uint16_t> idealNodes( - getIdealStateManager().getDistributorComponent() + distributorBucketSpace .getDistribution().getIdealStorageNodes( getIdealStateManager().getDistributorComponent() .getClusterState(), @@ -128,17 +134,17 @@ struct StateCheckersTest : public CppUnit::TestFixture, std::ostringstream ost; c.siblingBucket = getIdealStateManager().getDistributorComponent() - .getSibling(c.bucketId); + .getSibling(c.getBucketId()); std::vector<BucketDatabase::Entry> entries; - getBucketDatabase().getAll(c.bucketId, entries); + getBucketDatabase().getAll(c.getBucketId(), entries); c.siblingEntry = getBucketDatabase().get(c.siblingBucket); c.entries = entries; for (uint32_t j = 0; j < entries.size(); ++j) { // Run checking only on this bucketid, but include all buckets // owned by it or owners of it, so we can detect inconsistent split. - if (entries[j].getBucketId() == c.bucketId) { + if (entries[j].getBucketId() == c.getBucketId()) { c.entry = entries[j]; StateChecker::Result result(checker.check(c)); @@ -263,7 +269,7 @@ struct StateCheckersTest : public CppUnit::TestFixture, lib::ClusterState(params._clusterState)); NodeMaintenanceStatsTracker statsTracker; StateChecker::Context c( - getExternalOperationHandler(), statsTracker, bid); + getExternalOperationHandler(), getDistributorBucketSpace(), statsTracker, makeDocumentBucket(bid)); std::string result = testStateChecker( checker, c, false, *params._blockerMessage, params._includeMessagePriority, @@ -361,7 +367,7 @@ std::string StateCheckersTest::testSplit(uint32_t splitCount, SplitBucketStateChecker checker; NodeMaintenanceStatsTracker statsTracker; - StateChecker::Context c(getExternalOperationHandler(), statsTracker, bid); + StateChecker::Context c(getExternalOperationHandler(), getDistributorBucketSpace(), statsTracker, makeDocumentBucket(bid)); getConfig().setSplitSize(splitSize); getConfig().setSplitCount(splitCount); getConfig().setMinimalBucketSplit(minSplitBits); @@ -465,7 +471,7 @@ StateCheckersTest::testInconsistentSplit(const document::BucketId& bid, { SplitInconsistentStateChecker checker; NodeMaintenanceStatsTracker statsTracker; - StateChecker::Context c(getExternalOperationHandler(), statsTracker, bid); + StateChecker::Context c(getExternalOperationHandler(), getDistributorBucketSpace(), statsTracker, makeDocumentBucket(bid)); return testStateChecker(checker, c, true, PendingMessage(), includePriority); } @@ -533,7 +539,7 @@ StateCheckersTest::testJoin(uint32_t joinCount, getConfig().setMinimalBucketSplit(minSplitBits); NodeMaintenanceStatsTracker statsTracker; - StateChecker::Context c(getExternalOperationHandler(), statsTracker, bid); + StateChecker::Context c(getExternalOperationHandler(), getDistributorBucketSpace(), statsTracker, makeDocumentBucket(bid)); return testStateChecker(checker, c, true, blocker, includePriority); } @@ -789,7 +795,7 @@ StateCheckersTest::testSynchronizeAndMove(const std::string& bucketInfo, _distributor->enableClusterState(lib::ClusterState(clusterState)); NodeMaintenanceStatsTracker statsTracker; - StateChecker::Context c(getExternalOperationHandler(), statsTracker, bid); + StateChecker::Context c(getExternalOperationHandler(), getDistributorBucketSpace(), statsTracker, makeDocumentBucket(bid)); return testStateChecker(checker, c, false, blocker, includePriority); } @@ -984,7 +990,7 @@ StateCheckersTest::testDeleteExtraCopies( } DeleteExtraCopiesStateChecker checker; NodeMaintenanceStatsTracker statsTracker; - StateChecker::Context c(getExternalOperationHandler(), statsTracker, bid); + StateChecker::Context c(getExternalOperationHandler(), getDistributorBucketSpace(), statsTracker, makeDocumentBucket(bid)); return testStateChecker(checker, c, false, blocker, includePriority); } @@ -995,8 +1001,9 @@ StateCheckersTest::testDeleteExtraCopies() setupDistributor(2, 100, "distributor:1 storage:4"); { + auto &distributorBucketSpace(getIdealStateManager().getBucketSpaceRepo().get(makeBucketSpace())); std::vector<uint16_t> idealNodes( - getIdealStateManager().getDistributorComponent() + distributorBucketSpace .getDistribution().getIdealStorageNodes( getIdealStateManager().getDistributorComponent().getClusterState(), document::BucketId(17, 0), @@ -1133,7 +1140,7 @@ std::string StateCheckersTest::testBucketState( BucketStateStateChecker checker; NodeMaintenanceStatsTracker statsTracker; - StateChecker::Context c(getExternalOperationHandler(), statsTracker, bid); + StateChecker::Context c(getExternalOperationHandler(), getDistributorBucketSpace(), statsTracker, makeDocumentBucket(bid)); return testStateChecker(checker, c, false, PendingMessage(), includePriority); } @@ -1332,7 +1339,7 @@ std::string StateCheckersTest::testBucketStatePerGroup( BucketStateStateChecker checker; NodeMaintenanceStatsTracker statsTracker; - StateChecker::Context c(getExternalOperationHandler(), statsTracker, bid); + StateChecker::Context c(getExternalOperationHandler(), getDistributorBucketSpace(), statsTracker, makeDocumentBucket(bid)); return testStateChecker(checker, c, false, PendingMessage(), includePriority); } @@ -1474,8 +1481,8 @@ std::string StateCheckersTest::testGarbageCollection( getConfig().setGarbageCollection("music", checkInterval); getConfig().setLastGarbageCollectionChangeTime(lastChangeTime); NodeMaintenanceStatsTracker statsTracker; - StateChecker::Context c(getExternalOperationHandler(), statsTracker, - e.getBucketId()); + StateChecker::Context c(getExternalOperationHandler(), getDistributorBucketSpace(), statsTracker, + makeDocumentBucket(e.getBucketId())); getClock().setAbsoluteTimeInSeconds(nowTimestamp); return testStateChecker(checker, c, false, PendingMessage(), includePriority, includeSchedulingPri); @@ -1561,8 +1568,8 @@ StateCheckersTest::gcInhibitedWhenIdealNodeInMaintenance() getConfig().setGarbageCollection("music", 3600); getConfig().setLastGarbageCollectionChangeTime(0); NodeMaintenanceStatsTracker statsTracker; - StateChecker::Context c(getExternalOperationHandler(), statsTracker, - bucket); + StateChecker::Context c(getExternalOperationHandler(), getDistributorBucketSpace(), statsTracker, + makeDocumentBucket(bucket)); getClock().setAbsoluteTimeInSeconds(4000); // Would normally (in a non-maintenance case) trigger GC due to having // overshot the GC check cycle. @@ -1727,7 +1734,7 @@ StateCheckersTest::contextPopulatesIdealStateContainers() setupDistributor(2, 100, "distributor:1 storage:4"); NodeMaintenanceStatsTracker statsTracker; - StateChecker::Context c(getExternalOperationHandler(), statsTracker, {17, 0}); + StateChecker::Context c(getExternalOperationHandler(), getDistributorBucketSpace(), statsTracker, makeDocumentBucket({17, 0})); CPPUNIT_ASSERT_EQUAL((std::vector<uint16_t>{1, 3}), c.idealState); CPPUNIT_ASSERT_EQUAL(size_t(2), c.unorderedIdealState.size()); @@ -1772,7 +1779,7 @@ public: // NOTE: resets the bucket database! void runFor(const document::BucketId& bid) { Checker checker; - StateChecker::Context c(_fixture.getExternalOperationHandler(), _statsTracker, bid); + StateChecker::Context c(_fixture.getExternalOperationHandler(), _fixture.getDistributorBucketSpace(), _statsTracker, makeDocumentBucket(bid)); _result = _fixture.testStateChecker( checker, c, false, StateCheckersTest::PendingMessage(), false); } diff --git a/storage/src/tests/distributor/statoperationtest.cpp b/storage/src/tests/distributor/statoperationtest.cpp index 442d74503ab..9ae8fc2fa4a 100644 --- a/storage/src/tests/distributor/statoperationtest.cpp +++ b/storage/src/tests/distributor/statoperationtest.cpp @@ -46,6 +46,7 @@ StatOperationTest::testBucketInfo() StatBucketOperation op( getExternalOperationHandler(), + getDistributorBucketSpace(), std::shared_ptr<api::StatBucketCommand>( new api::StatBucketCommand(makeDocumentBucket(document::BucketId(16, 5)), ""))); @@ -90,7 +91,7 @@ StatOperationTest::testBucketList() { new api::GetBucketListCommand(makeDocumentBucket(document::BucketId(16, 5)))); StatBucketListOperation op( - getExternalOperationHandler().getBucketDatabase(), + getDistributorBucketSpace().getBucketDatabase(), getIdealStateManager(), getExternalOperationHandler().getIndex(), msg); diff --git a/storage/src/tests/distributor/twophaseupdateoperationtest.cpp b/storage/src/tests/distributor/twophaseupdateoperationtest.cpp index 9f6a7010179..a7418629f81 100644 --- a/storage/src/tests/distributor/twophaseupdateoperationtest.cpp +++ b/storage/src/tests/distributor/twophaseupdateoperationtest.cpp @@ -337,7 +337,7 @@ TwoPhaseUpdateOperationTest::sendUpdate(const std::string& bucketState, ExternalOperationHandler& handler = getExternalOperationHandler(); return std::make_shared<TwoPhaseUpdateOperation>( - handler, msg, getDistributor().getMetrics()); + handler, getDistributorBucketSpace(), msg, getDistributor().getMetrics()); } diff --git a/storage/src/tests/distributor/updateoperationtest.cpp b/storage/src/tests/distributor/updateoperationtest.cpp index 62590eabdad..c15a2b4057c 100644 --- a/storage/src/tests/distributor/updateoperationtest.cpp +++ b/storage/src/tests/distributor/updateoperationtest.cpp @@ -93,6 +93,7 @@ UpdateOperation_Test::sendUpdate(const std::string& bucketState) ExternalOperationHandler& handler = getExternalOperationHandler(); return std::shared_ptr<UpdateOperation>( new UpdateOperation(handler, + getDistributorBucketSpace(), msg, getDistributor().getMetrics().updates[msg->getLoadType()])); } diff --git a/storage/src/tests/distributor/visitoroperationtest.cpp b/storage/src/tests/distributor/visitoroperationtest.cpp index f12e1fa4e33..26f4fb3e784 100644 --- a/storage/src/tests/distributor/visitoroperationtest.cpp +++ b/storage/src/tests/distributor/visitoroperationtest.cpp @@ -202,6 +202,7 @@ private: { return std::make_unique<VisitorOperation>( getExternalOperationHandler(), + getDistributorBucketSpace(), msg, config, getDistributor().getMetrics().visits[msg->getLoadType()]); diff --git a/storage/src/vespa/storage/bucketdb/bucketmanager.cpp b/storage/src/vespa/storage/bucketdb/bucketmanager.cpp index 65731ed64da..5e7cf4af046 100644 --- a/storage/src/vespa/storage/bucketdb/bucketmanager.cpp +++ b/storage/src/vespa/storage/bucketdb/bucketmanager.cpp @@ -227,7 +227,7 @@ BucketManager::updateMetrics(bool updateDocCount) uint32_t diskCount = _component.getDiskCount(); if (!updateDocCount || _doneInitialized) { MetricsUpdater m(diskCount); - _component.getBucketSpaceRepo().forEachBucket( + _component.getBucketSpaceRepo().forEachBucketChunked( m, "BucketManager::updateMetrics"); if (updateDocCount) { for (uint16_t i = 0; i< diskCount; i++) { @@ -244,7 +244,7 @@ BucketManager::updateMetrics(bool updateDocCount) void BucketManager::updateMinUsedBits() { MetricsUpdater m(_component.getDiskCount()); - _component.getBucketSpaceRepo().forEachBucket( + _component.getBucketSpaceRepo().forEachBucketChunked( m, "BucketManager::updateMetrics"); // When going through to get sizes, we also record min bits MinimumUsedBitsTracker& bitTracker(_component.getMinUsedBitsTracker()); @@ -266,20 +266,20 @@ void BucketManager::run(framework::ThreadHandle& thread) framework::MilliSecTime timeToCheckMinUsedBits(0); while (!thread.interrupted()) { bool didWork = false; - BIList infoReqs; + BucketInfoRequestMap infoReqs; { vespalib::MonitorGuard monitor(_workerMonitor); infoReqs.swap(_bucketInfoRequests); } - didWork |= processRequestBucketInfoCommands(infoReqs); + for (auto &req : infoReqs) { + didWork |= processRequestBucketInfoCommands(req.first, req.second); + } { vespalib::MonitorGuard monitor(_workerMonitor); - if (!infoReqs.empty()) { - infoReqs.insert(infoReqs.end(), - _bucketInfoRequests.begin(), _bucketInfoRequests.end()); - _bucketInfoRequests.swap(infoReqs); + for (const auto &req : infoReqs) { + assert(req.second.empty()); } if (!didWork) { monitor.wait(1000); @@ -343,7 +343,7 @@ BucketManager::reportStatus(std::ostream& out, framework::PartlyXmlStatusReporter xmlReporter(*this, out, path); xmlReporter << vespalib::xml::XmlTag("buckets"); BucketDBDumper dumper(xmlReporter.getStream()); - _component.getBucketSpaceRepo().forEachBucket( + _component.getBucketSpaceRepo().forEachBucketChunked( dumper, "BucketManager::reportStatus"); xmlReporter << vespalib::xml::XmlEndTag(); } else { @@ -362,7 +362,7 @@ BucketManager::dump(std::ostream& out) const { vespalib::XmlOutputStream xos(out); BucketDBDumper dumper(xos); - _component.getBucketSpaceRepo().forEachBucket(dumper, "BucketManager::dump"); + _component.getBucketSpaceRepo().forEachBucketChunked(dumper, "BucketManager::dump"); } @@ -394,7 +394,7 @@ bool BucketManager::onRequestBucketInfo( if (cmd->getBuckets().size() == 0 && cmd->hasSystemState()) { vespalib::MonitorGuard monitor(_workerMonitor); - _bucketInfoRequests.push_back(cmd); + _bucketInfoRequests[cmd->getBucketSpace()].push_back(cmd); monitor.signal(); LOG(spam, "Scheduled request bucket info request for retrieval"); return true; @@ -498,7 +498,8 @@ BucketManager::leaveQueueProtectedSection(ScopedQueueDispatchGuard& queueGuard) } bool -BucketManager::processRequestBucketInfoCommands(BIList& reqs) +BucketManager::processRequestBucketInfoCommands(document::BucketSpace bucketSpace, + BucketInfoRequestList &reqs) { if (reqs.empty()) return false; @@ -529,7 +530,7 @@ BucketManager::processRequestBucketInfoCommands(BIList& reqs) our_hash.c_str()); vespalib::LockGuard lock(_clusterStateLock); - for (BIList::reverse_iterator it = reqs.rbegin(); it != reqs.rend(); ++it) { + for (auto it = reqs.rbegin(); it != reqs.rend(); ++it) { // Currently small requests should not be forwarded to worker thread assert((*it)->hasSystemState()); const auto their_hash = normalizer.normalize( @@ -602,12 +603,12 @@ BucketManager::processRequestBucketInfoCommands(BIList& reqs) if (LOG_WOULD_LOG(spam)) { DistributorInfoGatherer<true> builder( *clusterState, result, idFac, distribution); - _component.getBucketDatabase(BucketSpace::placeHolder()).chunkedAll(builder, + _component.getBucketDatabase(bucketSpace).chunkedAll(builder, "BucketManager::processRequestBucketInfoCommands-1"); } else { DistributorInfoGatherer<false> builder( *clusterState, result, idFac, distribution); - _component.getBucketDatabase(BucketSpace::placeHolder()).chunkedAll(builder, + _component.getBucketDatabase(bucketSpace).chunkedAll(builder, "BucketManager::processRequestBucketInfoCommands-2"); } _metrics->fullBucketInfoLatency.addValue( diff --git a/storage/src/vespa/storage/bucketdb/bucketmanager.h b/storage/src/vespa/storage/bucketdb/bucketmanager.h index c680ff7ed6c..3b71230a8ed 100644 --- a/storage/src/vespa/storage/bucketdb/bucketmanager.h +++ b/storage/src/vespa/storage/bucketdb/bucketmanager.h @@ -12,20 +12,21 @@ #pragma once -#include <vespa/storage/bucketdb/config-stor-bucketdb.h> -#include "storbucketdb.h" #include "bucketmanagermetrics.h" +#include "storbucketdb.h" +#include <vespa/config/subscription/configuri.h> +#include <vespa/storage/bucketdb/config-stor-bucketdb.h> #include <vespa/storage/common/bucketmessages.h> #include <vespa/storage/common/servicelayercomponent.h> #include <vespa/storage/common/storagelinkqueued.h> +#include <vespa/storageapi/message/bucket.h> #include <vespa/storageframework/generic/memory/memorymanagerinterface.h> -#include <vespa/storageframework/generic/status/statusreporter.h> #include <vespa/storageframework/generic/metric/metricupdatehook.h> +#include <vespa/storageframework/generic/status/statusreporter.h> -#include <vespa/storageapi/message/bucket.h> -#include <vespa/config/subscription/configuri.h> -#include <unordered_set> #include <list> +#include <unordered_map> +#include <unordered_set> namespace storage { @@ -34,16 +35,19 @@ class BucketManager : public StorageLinkQueued, private framework::Runnable, private framework::MetricUpdateHook { +public: /** Type used for message queues */ - typedef std::list<std::shared_ptr<api::StorageCommand> > CommandList; - typedef std::list<std::shared_ptr<api::RequestBucketInfoCommand> > BIList; + using CommandList = std::list<std::shared_ptr<api::StorageCommand>>; + using BucketInfoRequestList = std::list<std::shared_ptr<api::RequestBucketInfoCommand>>; + using BucketInfoRequestMap = std::unordered_map<document::BucketSpace, BucketInfoRequestList, document::BucketSpace::hash>; +private: config::ConfigUri _configUri; uint32_t _chunkLevel; mutable vespalib::Lock _stateAccess; framework::MemoryToken::UP _bucketDBMemoryToken; - BIList _bucketInfoRequests; + BucketInfoRequestMap _bucketInfoRequests; /** * We have our own thread running, which we use to send messages down. @@ -128,7 +132,8 @@ private: void updateMinUsedBits(); bool onRequestBucketInfo(const std::shared_ptr<api::RequestBucketInfoCommand>&) override; - bool processRequestBucketInfoCommands(BIList&); + bool processRequestBucketInfoCommands(document::BucketSpace bucketSpace, + BucketInfoRequestList &reqs); /** * Enqueue reply and add its bucket to the set of conflicting buckets iff diff --git a/storage/src/vespa/storage/common/content_bucket_space_repo.h b/storage/src/vespa/storage/common/content_bucket_space_repo.h index f6afa6002eb..390cfc15f5d 100644 --- a/storage/src/vespa/storage/common/content_bucket_space_repo.h +++ b/storage/src/vespa/storage/common/content_bucket_space_repo.h @@ -29,6 +29,14 @@ public: void forEachBucket(Functor &functor, const char *clientId) const { for (const auto &elem : _map) { + elem.second->bucketDatabase().all(functor, clientId); + } + } + + template <typename Functor> + void forEachBucketChunked(Functor &functor, + const char *clientId) const { + for (const auto &elem : _map) { elem.second->bucketDatabase().chunkedAll(functor, clientId); } } diff --git a/storage/src/vespa/storage/distributor/bucketdbupdater.cpp b/storage/src/vespa/storage/distributor/bucketdbupdater.cpp index 97c652c8a89..569136b8b10 100644 --- a/storage/src/vespa/storage/distributor/bucketdbupdater.cpp +++ b/storage/src/vespa/storage/distributor/bucketdbupdater.cpp @@ -18,10 +18,13 @@ using document::BucketSpace; namespace storage::distributor { -BucketDBUpdater::BucketDBUpdater(Distributor& owner, DistributorBucketSpace& bucketSpace, - DistributorMessageSender& sender, DistributorComponentRegister& compReg) +BucketDBUpdater::BucketDBUpdater(Distributor& owner, + DistributorBucketSpaceRepo &bucketSpaceRepo, + DistributorBucketSpace& bucketSpace, + DistributorMessageSender& sender, + DistributorComponentRegister& compReg) : framework::StatusReporter("bucketdb", "Bucket DB Updater"), - _bucketSpaceComponent(owner, bucketSpace, compReg, "Bucket DB Updater"), + _bucketSpaceComponent(owner, bucketSpaceRepo, bucketSpace, compReg, "Bucket DB Updater"), _sender(sender), _transitionTimer(_bucketSpaceComponent.getClock()) { @@ -61,7 +64,8 @@ BucketDBUpdater::checkOwnershipInPendingState(const document::BucketId& b) const if (hasPendingClusterState()) { const lib::ClusterState& state(_pendingClusterState->getNewClusterState()); const lib::Distribution& distribution(_pendingClusterState->getDistribution()); - if (!_bucketSpaceComponent.ownsBucketInState(distribution, state, b)) { + document::Bucket bucket(BucketSpace::placeHolder(), b); + if (!_bucketSpaceComponent.ownsBucketInState(distribution, state, bucket)) { return BucketOwnership::createNotOwnedInState(state); } } @@ -459,13 +463,15 @@ BucketDBUpdater::findRelatedBucketsInDatabase(uint16_t node, const document::Buc void BucketDBUpdater::updateDatabase(uint16_t node, BucketListMerger& merger) { - for (const document::BucketId & bucket : merger.getRemovedEntries()) { + for (const document::BucketId & bucketId : merger.getRemovedEntries()) { + document::Bucket bucket(BucketSpace::placeHolder(), bucketId); _bucketSpaceComponent.removeNodeFromDB(bucket, node); } for (const BucketListMerger::BucketEntry& entry : merger.getAddedEntries()) { + document::Bucket bucket(BucketSpace::placeHolder(), entry.first); _bucketSpaceComponent.updateBucketDatabase( - entry.first, + bucket, BucketCopy(merger.getTimestamp(), node, entry.second), DatabaseUpdate::CREATE_IF_NONEXISTING); } diff --git a/storage/src/vespa/storage/distributor/bucketdbupdater.h b/storage/src/vespa/storage/distributor/bucketdbupdater.h index 2428b3bd355..994e207f200 100644 --- a/storage/src/vespa/storage/distributor/bucketdbupdater.h +++ b/storage/src/vespa/storage/distributor/bucketdbupdater.h @@ -27,9 +27,8 @@ class BucketDBUpdater : public framework::StatusReporter, public api::MessageHandler { public: - // TODO take in BucketSpaceRepo instead, this class needs access to all - // bucket spaces. BucketDBUpdater(Distributor& owner, + DistributorBucketSpaceRepo &bucketSpaceRepo, DistributorBucketSpace& bucketSpace, DistributorMessageSender& sender, DistributorComponentRegister& compReg); diff --git a/storage/src/vespa/storage/distributor/distributor.cpp b/storage/src/vespa/storage/distributor/distributor.cpp index 05145d21410..11c029c5c94 100644 --- a/storage/src/vespa/storage/distributor/distributor.cpp +++ b/storage/src/vespa/storage/distributor/distributor.cpp @@ -72,12 +72,12 @@ Distributor::Distributor(DistributorComponentRegister& compReg, _operationOwner(*this, _component.getClock()), _maintenanceOperationOwner(*this, _component.getClock()), _pendingMessageTracker(compReg), - _bucketDBUpdater(*this, getDefaultBucketSpace(), *this, compReg), + _bucketDBUpdater(*this, *_bucketSpaceRepo, getDefaultBucketSpace(), *this, compReg), _distributorStatusDelegate(compReg, *this, *this), _bucketDBStatusDelegate(compReg, *this, _bucketDBUpdater), - _idealStateManager(*this, getDefaultBucketSpace(), compReg, + _idealStateManager(*this, *_bucketSpaceRepo, getDefaultBucketSpace(), compReg, manageActiveBucketCopies), - _externalOperationHandler(*this, getDefaultBucketSpace(), + _externalOperationHandler(*this, *_bucketSpaceRepo, getDefaultBucketSpace(), _idealStateManager, compReg), _threadPool(threadPool), _initializingIsUp(true), @@ -152,9 +152,9 @@ const DistributorBucketSpace& Distributor::getDefaultBucketSpace() const noexcep } BucketOwnership -Distributor::checkOwnershipInPendingState(const document::BucketId& b) const +Distributor::checkOwnershipInPendingState(const document::Bucket &b) const { - return _bucketDBUpdater.checkOwnershipInPendingState(b); + return _bucketDBUpdater.checkOwnershipInPendingState(b.getBucketId()); } void @@ -506,7 +506,8 @@ public: } void -Distributor::checkBucketForSplit(const BucketDatabase::Entry& e, +Distributor::checkBucketForSplit(document::BucketSpace bucketSpace, + const BucketDatabase::Entry& e, uint8_t priority) { if (!getConfig().doInlineSplit()) { @@ -518,7 +519,7 @@ Distributor::checkBucketForSplit(const BucketDatabase::Entry& e, SplitChecker checker(priority); for (uint32_t i = 0; i < e->getNodeCount(); ++i) { _pendingMessageTracker.checkPendingMessages(e->getNodeRef(i).getNode(), - document::Bucket(document::BucketSpace::placeHolder(), e.getBucketId()), + document::Bucket(bucketSpace, e.getBucketId()), checker); if (checker.found) { return; @@ -526,7 +527,7 @@ Distributor::checkBucketForSplit(const BucketDatabase::Entry& e, } Operation::SP operation = - _idealStateManager.generateInterceptingSplit(e, priority); + _idealStateManager.generateInterceptingSplit(bucketSpace, e, priority); if (operation.get()) { _maintenanceOperationOwner.start(operation, priority); diff --git a/storage/src/vespa/storage/distributor/distributor.h b/storage/src/vespa/storage/distributor/distributor.h index 3b74676f937..f59b47574ba 100644 --- a/storage/src/vespa/storage/distributor/distributor.h +++ b/storage/src/vespa/storage/distributor/distributor.h @@ -72,7 +72,7 @@ public: return _pendingMessageTracker; } - BucketOwnership checkOwnershipInPendingState(const document::BucketId&) const override; + BucketOwnership checkOwnershipInPendingState(const document::Bucket &bucket) const override; /** * Enables a new cluster state. Called after the bucket db updater has @@ -112,7 +112,7 @@ public: * Checks whether a bucket needs to be split, and sends a split * if so. */ - void checkBucketForSplit(const BucketDatabase::Entry& e, uint8_t priority) override; + void checkBucketForSplit(document::BucketSpace bucketSpace, const BucketDatabase::Entry& e, uint8_t priority) override; const lib::Distribution& getDistribution() const override; diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space_component.cpp b/storage/src/vespa/storage/distributor/distributor_bucket_space_component.cpp index 22571fb3d5c..4616179ae82 100644 --- a/storage/src/vespa/storage/distributor/distributor_bucket_space_component.cpp +++ b/storage/src/vespa/storage/distributor/distributor_bucket_space_component.cpp @@ -6,10 +6,11 @@ namespace storage::distributor { DistributorBucketSpaceComponent::DistributorBucketSpaceComponent( DistributorInterface& distributor, + DistributorBucketSpaceRepo &bucketSpaceRepo, DistributorBucketSpace& bucketSpace, DistributorComponentRegister& compReg, const std::string& name) - : DistributorComponent(distributor, compReg, name), + : DistributorComponent(distributor, bucketSpaceRepo, compReg, name), _bucketSpace(bucketSpace) { } diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space_component.h b/storage/src/vespa/storage/distributor/distributor_bucket_space_component.h index 9a000d28117..9c04cb6b67f 100644 --- a/storage/src/vespa/storage/distributor/distributor_bucket_space_component.h +++ b/storage/src/vespa/storage/distributor/distributor_bucket_space_component.h @@ -15,9 +15,10 @@ class DistributorBucketSpaceComponent : public DistributorComponent { DistributorBucketSpace& _bucketSpace; public: DistributorBucketSpaceComponent(DistributorInterface& distributor, - DistributorBucketSpace& bucketSpace, - DistributorComponentRegister& compReg, - const std::string& name); + DistributorBucketSpaceRepo &bucketSpaceRepo, + DistributorBucketSpace& bucketSpace, + DistributorComponentRegister& compReg, + const std::string& name); BucketDatabase& getBucketDatabase() override { return _bucketSpace.getBucketDatabase(); diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.cpp b/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.cpp index b6dc1e856d7..67ca2397b11 100644 --- a/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.cpp +++ b/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.cpp @@ -2,10 +2,13 @@ #include "distributor_bucket_space_repo.h" #include <vespa/vdslib/distribution/distribution.h> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP(".distributor.managed_bucket_space_repo"); +using document::BucketSpace; + namespace storage { namespace distributor { @@ -23,5 +26,19 @@ void DistributorBucketSpaceRepo::setDefaultDistribution( _defaultSpace.setDistribution(std::move(distr)); } +DistributorBucketSpace & +DistributorBucketSpaceRepo::get(BucketSpace bucketSpace) +{ + assert(bucketSpace == BucketSpace::placeHolder()); + return _defaultSpace; +} + +const DistributorBucketSpace & +DistributorBucketSpaceRepo::get(BucketSpace bucketSpace) const +{ + assert(bucketSpace == BucketSpace::placeHolder()); + return _defaultSpace; +} + } } diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.h b/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.h index 1e0fc375eca..41eebf4bc4b 100644 --- a/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.h +++ b/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.h @@ -2,6 +2,7 @@ #pragma once #include "distributor_bucket_space.h" +#include <vespa/document/bucket/bucketspace.h> #include <memory> namespace storage { @@ -24,6 +25,8 @@ public: const DistributorBucketSpace& getDefaultSpace() const noexcept { return _defaultSpace; } + DistributorBucketSpace &get(document::BucketSpace bucketSpace); + const DistributorBucketSpace &get(document::BucketSpace bucketSpace) const; void setDefaultDistribution(std::shared_ptr<lib::Distribution> distr); }; diff --git a/storage/src/vespa/storage/distributor/distributorcomponent.cpp b/storage/src/vespa/storage/distributor/distributorcomponent.cpp index f8150ba0535..f8a0a5504ec 100644 --- a/storage/src/vespa/storage/distributor/distributorcomponent.cpp +++ b/storage/src/vespa/storage/distributor/distributorcomponent.cpp @@ -3,20 +3,25 @@ #include <vespa/storage/common/bucketoperationlogger.h> #include <vespa/storageapi/messageapi/storagereply.h> #include <vespa/vdslib/distribution/distribution.h> +#include <vespa/storage/distributor/distributor_bucket_space_repo.h> #include <vespa/log/log.h> LOG_SETUP(".distributorstoragelink"); +using document::BucketSpace; + namespace storage { namespace distributor { DistributorComponent::DistributorComponent( DistributorInterface& distributor, + DistributorBucketSpaceRepo &bucketSpaceRepo, DistributorComponentRegister& compReg, const std::string& name) : storage::DistributorComponent(compReg, name), - _distributor(distributor) + _distributor(distributor), + _bucketSpaceRepo(bucketSpaceRepo) { } @@ -41,11 +46,12 @@ DistributorComponent::getClusterState() const }; std::vector<uint16_t> -DistributorComponent::getIdealNodes(const document::BucketId& bid) const +DistributorComponent::getIdealNodes(const document::Bucket &bucket) const { - return getDistribution().getIdealStorageNodes( + auto &bucketSpace(_bucketSpaceRepo.get(bucket.getBucketSpace())); + return bucketSpace.getDistribution().getIdealStorageNodes( getClusterState(), - bid, + bucket.getBucketId(), _distributor.getStorageNodeUpStates()); } @@ -53,7 +59,7 @@ BucketOwnership DistributorComponent::checkOwnershipInPendingAndGivenState( const lib::Distribution& distribution, const lib::ClusterState& clusterState, - const document::BucketId& bucket) const + const document::Bucket &bucket) const { try { BucketOwnership pendingRes( @@ -62,7 +68,7 @@ DistributorComponent::checkOwnershipInPendingAndGivenState( return pendingRes; } uint16_t distributor = distribution.getIdealDistributorNode( - clusterState, bucket); + clusterState, bucket.getBucketId()); if (getIndex() == distributor) { return BucketOwnership::createOwned(); @@ -78,24 +84,25 @@ DistributorComponent::checkOwnershipInPendingAndGivenState( BucketOwnership DistributorComponent::checkOwnershipInPendingAndCurrentState( - const document::BucketId& bucket) const + const document::Bucket &bucket) const { + auto &bucketSpace(_bucketSpaceRepo.get(bucket.getBucketSpace())); return checkOwnershipInPendingAndGivenState( - getDistribution(), getClusterState(), bucket); + bucketSpace.getDistribution(), getClusterState(), bucket); } bool DistributorComponent::ownsBucketInState( const lib::Distribution& distribution, const lib::ClusterState& clusterState, - const document::BucketId& bucket) const + const document::Bucket &bucket) const { LOG(spam, "checking bucket %s in state %s with distr %s", bucket.toString().c_str(), clusterState.toString().c_str(), distribution.getNodeGraph().getDistributionConfigHash().c_str()); try { uint16_t distributor = distribution.getIdealDistributorNode( - clusterState, bucket); + clusterState, bucket.getBucketId()); return (getIndex() == distributor); } catch (lib::TooFewBucketBitsInUseException& e) { @@ -108,16 +115,17 @@ DistributorComponent::ownsBucketInState( bool DistributorComponent::ownsBucketInState( const lib::ClusterState& clusterState, - const document::BucketId& bucket) const + const document::Bucket &bucket) const { - return ownsBucketInState(getDistribution(), clusterState, bucket); + auto &bucketSpace(_bucketSpaceRepo.get(bucket.getBucketSpace())); + return ownsBucketInState(bucketSpace.getDistribution(), clusterState, bucket); } bool -DistributorComponent::ownsBucketInCurrentState( - const document::BucketId& bucket) const +DistributorComponent::ownsBucketInCurrentState(const document::Bucket &bucket) const { - return ownsBucketInState(getDistribution(), getClusterState(), bucket); + auto &bucketSpace(_bucketSpaceRepo.get(bucket.getBucketSpace())); + return ownsBucketInState(bucketSpace.getDistribution(), getClusterState(), bucket); } api::StorageMessageAddress @@ -137,15 +145,15 @@ DistributorComponent::getRedundancy() const { bool DistributorComponent::checkDistribution( api::StorageCommand &cmd, - const document::BucketId& bid) + const document::Bucket &bucket) { - BucketOwnership bo(checkOwnershipInPendingAndCurrentState(bid)); + BucketOwnership bo(checkOwnershipInPendingAndCurrentState(bucket)); if (!bo.isOwned()) { std::string systemStateStr = bo.getNonOwnedState().toString(); LOG(debug, "Got message with wrong distribution, " - "bucketid %s sending back state '%s'", - bid.toString().c_str(), + "bucket %s sending back state '%s'", + bucket.toString().c_str(), systemStateStr.c_str()); api::StorageReply::UP reply(cmd.makeReply()); @@ -160,10 +168,11 @@ DistributorComponent::checkDistribution( } void -DistributorComponent::removeNodesFromDB(const document::BucketId& bucketId, +DistributorComponent::removeNodesFromDB(const document::Bucket &bucket, const std::vector<uint16_t>& nodes) { - BucketDatabase::Entry dbentry = getBucketDatabase().get(bucketId); + auto &bucketSpace(_bucketSpaceRepo.get(bucket.getBucketSpace())); + BucketDatabase::Entry dbentry = bucketSpace.getBucketDatabase().get(bucket.getBucketId()); if (dbentry.valid()) { for (uint32_t i = 0; i < nodes.size(); ++i) { @@ -171,20 +180,20 @@ DistributorComponent::removeNodesFromDB(const document::BucketId& bucketId, LOG(debug, "Removed node %d from bucket %s. %u copies remaining", nodes[i], - bucketId.toString().c_str(), + bucket.toString().c_str(), dbentry->getNodeCount()); } } if (dbentry->getNodeCount() != 0) { - getBucketDatabase().update(dbentry); + bucketSpace.getBucketDatabase().update(dbentry); } else { LOG(debug, "After update, bucket %s now has no copies. " "Removing from database.", - bucketId.toString().c_str()); + bucket.toString().c_str()); - getBucketDatabase().remove(bucketId); + bucketSpace.getBucketDatabase().remove(bucket.getBucketId()); } } } @@ -192,7 +201,7 @@ DistributorComponent::removeNodesFromDB(const document::BucketId& bucketId, std::vector<uint16_t> DistributorComponent::enumerateDownNodes( const lib::ClusterState& s, - const document::BucketId& bucket, + const document::Bucket &bucket, const std::vector<BucketCopy>& candidates) const { std::vector<uint16_t> downNodes; @@ -216,19 +225,20 @@ DistributorComponent::enumerateDownNodes( void DistributorComponent::updateBucketDatabase( - const document::BucketId& bucketId, + const document::Bucket &bucket, const std::vector<BucketCopy>& changedNodes, uint32_t updateFlags) { - assert(!(bucketId == document::BucketId())); - BucketDatabase::Entry dbentry = getBucketDatabase().get(bucketId); + auto &bucketSpace(_bucketSpaceRepo.get(bucket.getBucketSpace())); + assert(!(bucket.getBucketId() == document::BucketId())); + BucketDatabase::Entry dbentry = bucketSpace.getBucketDatabase().get(bucket.getBucketId()); - BucketOwnership ownership(checkOwnershipInPendingAndCurrentState(bucketId)); + BucketOwnership ownership(checkOwnershipInPendingAndCurrentState(bucket)); if (!ownership.isOwned()) { LOG(debug, "Trying to add %s to database that we do not own according to " "cluster state '%s' - ignoring!", - bucketId.toString().c_str(), + bucket.toString().c_str(), ownership.getNonOwnedState().toString().c_str()); LOG_BUCKET_OPERATION_NO_LOCK(bucketId, "Ignoring database insert since " "we do not own the bucket"); @@ -237,7 +247,7 @@ DistributorComponent::updateBucketDatabase( if (!dbentry.valid()) { if (updateFlags & DatabaseUpdate::CREATE_IF_NONEXISTING) { - dbentry = BucketDatabase::Entry(bucketId, BucketInfo()); + dbentry = BucketDatabase::Entry(bucket.getBucketId(), BucketInfo()); } else { return; } @@ -254,11 +264,11 @@ DistributorComponent::updateBucketDatabase( // Ensure that we're not trying to bring any zombie copies into the // bucket database (i.e. copies on nodes that are actually down). std::vector<uint16_t> downNodes( - enumerateDownNodes(getClusterState(), bucketId, changedNodes)); + enumerateDownNodes(getClusterState(), bucket, changedNodes)); // Optimize for common case where we don't have to create a new // bucket copy vector if (downNodes.empty()) { - dbentry->addNodes(changedNodes, getIdealNodes(bucketId)); + dbentry->addNodes(changedNodes, getIdealNodes(bucket)); } else { std::vector<BucketCopy> upNodes; for (uint32_t i = 0; i < changedNodes.size(); ++i) { @@ -270,12 +280,12 @@ DistributorComponent::updateBucketDatabase( upNodes.push_back(copy); } } - dbentry->addNodes(upNodes, getIdealNodes(bucketId)); + dbentry->addNodes(upNodes, getIdealNodes(bucket)); } if (updateFlags & DatabaseUpdate::RESET_TRUSTED) { dbentry->resetTrusted(); } - getBucketDatabase().update(dbentry); + bucketSpace.getBucketDatabase().update(dbentry); } void @@ -330,19 +340,12 @@ DistributorComponent::getSibling(const document::BucketId& bid) const { }; BucketDatabase::Entry -DistributorComponent::createAppropriateBucket(const document::BucketId& bid) -{ - return getBucketDatabase().createAppropriateBucket( - _distributor.getConfig().getMinimalBucketSplit(), - bid); -} - -document::BucketId -DistributorComponent::getAppropriateBucket(const document::BucketId& bid) +DistributorComponent::createAppropriateBucket(const document::Bucket &bucket) { - return getBucketDatabase().getAppropriateBucket( + auto &bucketSpace(_bucketSpaceRepo.get(bucket.getBucketSpace())); + return bucketSpace.getBucketDatabase().createAppropriateBucket( _distributor.getConfig().getMinimalBucketSplit(), - bid); + bucket.getBucketId()); } bool diff --git a/storage/src/vespa/storage/distributor/distributorcomponent.h b/storage/src/vespa/storage/distributor/distributorcomponent.h index 742d88dec51..307ddc20299 100644 --- a/storage/src/vespa/storage/distributor/distributorcomponent.h +++ b/storage/src/vespa/storage/distributor/distributorcomponent.h @@ -14,6 +14,8 @@ namespace storage { namespace distributor { +class DistributorBucketSpaceRepo; + struct DatabaseUpdate { enum UpdateFlags { CREATE_IF_NONEXISTING = 1, @@ -29,6 +31,7 @@ class DistributorComponent : public storage::DistributorComponent { public: DistributorComponent(DistributorInterface& distributor, + DistributorBucketSpaceRepo &bucketSpaceRepo, DistributorComponentRegister& compReg, const std::string& name); @@ -42,27 +45,27 @@ public: BucketOwnership checkOwnershipInPendingAndGivenState( const lib::Distribution& distribution, const lib::ClusterState& clusterState, - const document::BucketId& bucket) const; + const document::Bucket &bucket) const; BucketOwnership checkOwnershipInPendingAndCurrentState( - const document::BucketId& bucket) const; + const document::Bucket &bucket) const; bool ownsBucketInState(const lib::Distribution& distribution, const lib::ClusterState& clusterState, - const document::BucketId& bucket) const; + const document::Bucket &bucket) const; /** * Returns true if this distributor owns the given bucket in the * given cluster and current distribution config. */ bool ownsBucketInState(const lib::ClusterState& clusterState, - const document::BucketId& bucket) const; + const document::Bucket &bucket) const; /** * Returns true if this distributor owns the given bucket with the current * cluster state and distribution config. */ - bool ownsBucketInCurrentState(const document::BucketId&) const; + bool ownsBucketInCurrentState(const document::Bucket &bucket) const; /** * Returns a reference to the current system state. Valid until the next @@ -73,7 +76,7 @@ public: /** * Returns the ideal nodes for the given bucket. */ - std::vector<uint16_t> getIdealNodes(const document::BucketId& bucketId) const; + std::vector<uint16_t> getIdealNodes(const document::Bucket &bucket) const; /** * Returns the slobrok address of the given storage node. @@ -94,16 +97,14 @@ public: * Verifies that the given command has been received at the * correct distributor based on the current system state. */ - bool checkDistribution( - api::StorageCommand& cmd, - const document::BucketId& bid); + bool checkDistribution(api::StorageCommand& cmd, const document::Bucket &bucket); /** * Removes the given bucket copies from the bucket database. * If the resulting bucket is empty afterwards, removes the entire * bucket entry from the bucket database. */ - void removeNodesFromDB(const document::BucketId& id, + void removeNodesFromDB(const document::Bucket &bucket, const std::vector<uint16_t>& nodes); /** @@ -111,15 +112,15 @@ public: * If the resulting bucket is empty afterwards, removes the entire * bucket entry from the bucket database. */ - void removeNodeFromDB(const document::BucketId& id, uint16_t node) { - removeNodesFromDB(id, toVector<uint16_t>(node)); + void removeNodeFromDB(const document::Bucket &bucket, uint16_t node) { + removeNodesFromDB(bucket, toVector<uint16_t>(node)); } /** * Adds the given copies to the bucket database. */ void updateBucketDatabase( - const document::BucketId& bid, + const document::Bucket &bucket, const std::vector<BucketCopy>& changedNodes, uint32_t updateFlags = 0); @@ -127,11 +128,11 @@ public: * Simple API for the common case of modifying a single node. */ void updateBucketDatabase( - const document::BucketId& bid, + const document::Bucket &bucket, const BucketCopy& changedNode, uint32_t updateFlags = 0) { - updateBucketDatabase(bid, + updateBucketDatabase(bucket, toVector<BucketCopy>(changedNode), updateFlags); } @@ -162,6 +163,9 @@ public: // even has a different signature altogether...! virtual const lib::Distribution& getDistribution() const = 0; + DistributorBucketSpaceRepo &getBucketSpaceRepo() { return _bucketSpaceRepo; } + const DistributorBucketSpaceRepo &getBucketSpaceRepo() const { return _bucketSpaceRepo; } + /** * Finds a bucket that has the same direct parent as the given bucket * (i.e. split one bit less), but different bit in the most used bit. @@ -169,13 +173,10 @@ public: document::BucketId getSibling(const document::BucketId& bid) const; /** - * Gets a bucket that is split correctly according to other buckets that - * are in the bucket database. For instance, if you have a sibling bucket of - * the bucket, a similarly split bucket should be created. + * Create a bucket that is split correctly according to other buckets that + * are in the bucket database. */ - document::BucketId getAppropriateBucket(const document::BucketId& bid); - - BucketDatabase::Entry createAppropriateBucket(const document::BucketId& bid); + BucketDatabase::Entry createAppropriateBucket(const document::Bucket &bucket); /** * Returns true if the node is currently initializing. @@ -185,12 +186,13 @@ public: private: std::vector<uint16_t> enumerateDownNodes( const lib::ClusterState& s, - const document::BucketId& bucket, + const document::Bucket &bucket, const std::vector<BucketCopy>& candidates) const; DistributorInterface& _distributor; protected: + DistributorBucketSpaceRepo &_bucketSpaceRepo; vespalib::Lock _sync; }; diff --git a/storage/src/vespa/storage/distributor/distributorinterface.h b/storage/src/vespa/storage/distributor/distributorinterface.h index 913e83dd70e..cd51387964a 100644 --- a/storage/src/vespa/storage/distributor/distributorinterface.h +++ b/storage/src/vespa/storage/distributor/distributorinterface.h @@ -27,7 +27,7 @@ public: virtual void enableClusterState(const lib::ClusterState& state) = 0; - virtual BucketOwnership checkOwnershipInPendingState(const document::BucketId&) const = 0; + virtual BucketOwnership checkOwnershipInPendingState(const document::Bucket &bucket) const = 0; virtual void notifyDistributionChangeEnabled() = 0; @@ -46,7 +46,7 @@ public: * @param e The bucket to check. * @param pri The priority the split should be sent at. */ - virtual void checkBucketForSplit(const BucketDatabase::Entry& e, uint8_t pri) = 0; + virtual void checkBucketForSplit(document::BucketSpace bucketSpace, const BucketDatabase::Entry& e, uint8_t pri) = 0; /** * @return Returns the current cluster state. diff --git a/storage/src/vespa/storage/distributor/externaloperationhandler.cpp b/storage/src/vespa/storage/distributor/externaloperationhandler.cpp index 5bc2658e242..77a86a3756d 100644 --- a/storage/src/vespa/storage/distributor/externaloperationhandler.cpp +++ b/storage/src/vespa/storage/distributor/externaloperationhandler.cpp @@ -19,6 +19,7 @@ #include <vespa/storageapi/message/removelocation.h> #include <vespa/storageapi/message/batch.h> #include <vespa/storageapi/message/stat.h> +#include "distributor_bucket_space_repo.h" #include <vespa/log/log.h> LOG_SETUP(".distributor.manager"); @@ -27,10 +28,11 @@ namespace storage::distributor { ExternalOperationHandler::ExternalOperationHandler( Distributor& owner, + DistributorBucketSpaceRepo& bucketSpaceRepo, DistributorBucketSpace& bucketSpace, const MaintenanceOperationGenerator& gen, DistributorComponentRegister& compReg) - : DistributorBucketSpaceComponent(owner, bucketSpace, compReg, "External operation handler"), + : DistributorBucketSpaceComponent(owner, bucketSpaceRepo, bucketSpace, compReg, "External operation handler"), _operationGenerator(gen), _rejectFeedBeforeTimeReached() // At epoch { } @@ -79,9 +81,10 @@ ExternalOperationHandler::checkSafeTimeReached(api::StorageCommand& cmd) bool ExternalOperationHandler::checkTimestampMutationPreconditions( api::StorageCommand& cmd, - const document::BucketId& bucket, + const document::BucketId &bucketId, PersistenceOperationMetricSet& persistenceMetrics) { + document::Bucket bucket(cmd.getBucket().getBucketSpace(), bucketId); if (!checkDistribution(cmd, bucket)) { LOG(debug, "Distributor manager received %s, bucket %s with wrong " @@ -135,7 +138,9 @@ IMPL_MSG_COMMAND_H(ExternalOperationHandler, Put) auto handle = _mutationSequencer.try_acquire(cmd->getDocumentId()); if (allowMutation(handle)) { - _op = std::make_shared<PutOperation>(*this, cmd, getMetrics().puts[cmd->getLoadType()], std::move(handle)); + _op = std::make_shared<PutOperation>(*this, + _bucketSpaceRepo.get(cmd->getBucket().getBucketSpace()), + cmd, getMetrics().puts[cmd->getLoadType()], std::move(handle)); } else { sendUp(makeConcurrentMutationRejectionReply(*cmd, cmd->getDocumentId())); } @@ -158,7 +163,9 @@ IMPL_MSG_COMMAND_H(ExternalOperationHandler, Update) } auto handle = _mutationSequencer.try_acquire(cmd->getDocumentId()); if (allowMutation(handle)) { - _op = std::make_shared<TwoPhaseUpdateOperation>(*this, cmd, getMetrics(), std::move(handle)); + _op = std::make_shared<TwoPhaseUpdateOperation>(*this, + _bucketSpaceRepo.get(cmd->getBucket().getBucketSpace()), + cmd, getMetrics(), std::move(handle)); } else { sendUp(makeConcurrentMutationRejectionReply(*cmd, cmd->getDocumentId())); } @@ -181,8 +188,10 @@ IMPL_MSG_COMMAND_H(ExternalOperationHandler, Remove) } auto handle = _mutationSequencer.try_acquire(cmd->getDocumentId()); if (allowMutation(handle)) { + auto &distributorBucketSpace(_bucketSpaceRepo.get(cmd->getBucket().getBucketSpace())); _op = std::make_shared<RemoveOperation>( *this, + distributorBucketSpace, cmd, getMetrics().removes[cmd->getLoadType()], std::move(handle)); @@ -197,8 +206,9 @@ IMPL_MSG_COMMAND_H(ExternalOperationHandler, RemoveLocation) { document::BucketId bid; RemoveLocationOperation::getBucketId(*this, *cmd, bid); + document::Bucket bucket(cmd->getBucket().getBucketSpace(), bid); - if (!checkDistribution(*cmd, bid)) { + if (!checkDistribution(*cmd, bucket)) { LOG(debug, "Distributor manager received %s with wrong distribution", cmd->toString().c_str()); @@ -210,6 +220,7 @@ IMPL_MSG_COMMAND_H(ExternalOperationHandler, RemoveLocation) _op = Operation::SP(new RemoveLocationOperation( *this, + _bucketSpaceRepo.get(cmd->getBucket().getBucketSpace()), cmd, getMetrics().removelocations[cmd->getLoadType()])); return true; @@ -217,12 +228,13 @@ IMPL_MSG_COMMAND_H(ExternalOperationHandler, RemoveLocation) IMPL_MSG_COMMAND_H(ExternalOperationHandler, Get) { - if (!checkDistribution(*cmd, getBucketId(cmd->getDocumentId()))) { + document::Bucket bucket(cmd->getBucket().getBucketSpace(), getBucketId(cmd->getDocumentId())); + if (!checkDistribution(*cmd, bucket)) { LOG(debug, "Distributor manager received get for %s, " "bucket %s with wrong distribution", cmd->getDocumentId().toString().c_str(), - getBucketId(cmd->getDocumentId()).toString().c_str()); + bucket.toString().c_str()); getMetrics().gets[cmd->getLoadType()].failures.wrongdistributor++; return true; @@ -230,6 +242,7 @@ IMPL_MSG_COMMAND_H(ExternalOperationHandler, Get) _op = Operation::SP(new GetOperation( *this, + _bucketSpaceRepo.get(cmd->getBucket().getBucketSpace()), cmd, getMetrics().gets[cmd->getLoadType()])); return true; @@ -237,16 +250,17 @@ IMPL_MSG_COMMAND_H(ExternalOperationHandler, Get) IMPL_MSG_COMMAND_H(ExternalOperationHandler, MultiOperation) { - if (!checkDistribution(*cmd, cmd->getBucketId())) { + if (!checkDistribution(*cmd, cmd->getBucket())) { LOG(debug, "Distributor manager received multi-operation message, " "bucket %s with wrong distribution", - cmd->getBucketId().toString().c_str()); + cmd->getBucket().toString().c_str()); return true; } _op = Operation::SP(new MultiOperationOperation( *this, + _bucketSpaceRepo.get(cmd->getBucket().getBucketSpace()), cmd, getMetrics().multioperations[cmd->getLoadType()])); return true; @@ -254,21 +268,24 @@ IMPL_MSG_COMMAND_H(ExternalOperationHandler, MultiOperation) IMPL_MSG_COMMAND_H(ExternalOperationHandler, StatBucket) { - if (!checkDistribution(*cmd, cmd->getBucketId())) { + if (!checkDistribution(*cmd, cmd->getBucket())) { return true; } - - _op = Operation::SP(new StatBucketOperation(*this, cmd)); + auto &distributorBucketSpace(_bucketSpaceRepo.get(cmd->getBucket().getBucketSpace())); + _op = Operation::SP(new StatBucketOperation(*this, distributorBucketSpace, cmd)); return true; } IMPL_MSG_COMMAND_H(ExternalOperationHandler, GetBucketList) { - if (!checkDistribution(*cmd, cmd->getBucketId())) { + if (!checkDistribution(*cmd, cmd->getBucket())) { return true; } + auto bucketSpace(cmd->getBucket().getBucketSpace()); + auto &distributorBucketSpace(_bucketSpaceRepo.get(bucketSpace)); + auto &bucketDatabase(distributorBucketSpace.getBucketDatabase()); _op = Operation::SP(new StatBucketListOperation( - getBucketDatabase(), _operationGenerator, getIndex(), cmd)); + bucketDatabase, _operationGenerator, getIndex(), cmd)); return true; } @@ -277,7 +294,8 @@ IMPL_MSG_COMMAND_H(ExternalOperationHandler, CreateVisitor) const DistributorConfiguration& config(getDistributor().getConfig()); VisitorOperation::Config visitorConfig(config.getMinBucketsPerVisitor(), config.getMaxVisitorsPerNodePerClientVisitor()); - _op = Operation::SP(new VisitorOperation(*this, cmd, visitorConfig, getMetrics().visits[cmd->getLoadType()])); + auto &distributorBucketSpace(_bucketSpaceRepo.get(cmd->getBucket().getBucketSpace())); + _op = Operation::SP(new VisitorOperation(*this, distributorBucketSpace, cmd, visitorConfig, getMetrics().visits[cmd->getLoadType()])); return true; } diff --git a/storage/src/vespa/storage/distributor/externaloperationhandler.h b/storage/src/vespa/storage/distributor/externaloperationhandler.h index 6d7078d0405..763796767cf 100644 --- a/storage/src/vespa/storage/distributor/externaloperationhandler.h +++ b/storage/src/vespa/storage/distributor/externaloperationhandler.h @@ -38,6 +38,7 @@ public: DEF_MSG_COMMAND_H(GetBucketList); ExternalOperationHandler(Distributor& owner, + DistributorBucketSpaceRepo& bucketSpaceRepo, DistributorBucketSpace& bucketSpace, const MaintenanceOperationGenerator&, DistributorComponentRegister& compReg); @@ -61,7 +62,7 @@ private: api::ReturnCode makeSafeTimeRejectionResult(TimePoint unsafeTime); bool checkTimestampMutationPreconditions( api::StorageCommand& cmd, - const document::BucketId& bucket, + const document::BucketId &bucketId, PersistenceOperationMetricSet& persistenceMetrics); std::shared_ptr<api::StorageMessage> makeConcurrentMutationRejectionReply( api::StorageCommand& cmd, diff --git a/storage/src/vespa/storage/distributor/idealstatemanager.cpp b/storage/src/vespa/storage/distributor/idealstatemanager.cpp index bd9e9ca471d..a33d8376e4d 100644 --- a/storage/src/vespa/storage/distributor/idealstatemanager.cpp +++ b/storage/src/vespa/storage/distributor/idealstatemanager.cpp @@ -10,10 +10,12 @@ #include <vespa/storageapi/message/multioperation.h> #include <vespa/storage/common/bucketmessages.h> #include <vespa/vespalib/stllike/hash_map.hpp> +#include "distributor_bucket_space_repo.h" #include <vespa/log/log.h> LOG_SETUP(".distributor.operation.queue"); +using document::BucketSpace; using storage::lib::Node; using storage::lib::NodeType; @@ -22,12 +24,14 @@ namespace distributor { IdealStateManager::IdealStateManager( Distributor& owner, + DistributorBucketSpaceRepo& bucketSpaceRepo, DistributorBucketSpace& bucketSpace, DistributorComponentRegister& compReg, bool manageActiveBucketCopies) : HtmlStatusReporter("idealstateman", "Ideal state manager"), _metrics(new IdealStateMetricSet), - _distributorComponent(owner, bucketSpace, compReg, "Ideal state manager") + _distributorComponent(owner, bucketSpaceRepo, bucketSpace, compReg, "Ideal state manager"), + _bucketSpaceRepo(bucketSpaceRepo) { _distributorComponent.registerStatusPage(*this); _distributorComponent.registerMetric(*_metrics); @@ -73,17 +77,17 @@ IdealStateManager::iAmUp() const void IdealStateManager::fillParentAndChildBuckets(StateChecker::Context& c) const { - _distributorComponent.getBucketDatabase().getAll(c.bucketId, c.entries); + c.db.getAll(c.getBucketId(), c.entries); if (c.entries.empty()) { LOG(spam, "Did not find bucket %s in bucket database", - c.bucketId.toString().c_str()); + c.bucket.toString().c_str()); } } void IdealStateManager::fillSiblingBucket(StateChecker::Context& c) const { - c.siblingEntry = _distributorComponent.getBucketDatabase().get(c.siblingBucket); + c.siblingEntry = c.db.get(c.siblingBucket); } BucketDatabase::Entry* @@ -91,7 +95,7 @@ IdealStateManager::getEntryForPrimaryBucket(StateChecker::Context& c) const { for (uint32_t j = 0; j < c.entries.size(); ++j) { BucketDatabase::Entry& e = c.entries[j]; - if (e.getBucketId() == c.bucketId) { + if (e.getBucketId() == c.getBucketId()) { return &e; } } @@ -142,7 +146,8 @@ IdealStateManager::generateHighestPriority( const document::Bucket &bucket, NodeMaintenanceStatsTracker& statsTracker) const { - StateChecker::Context c(_distributorComponent, statsTracker, bucket.getBucketId()); + auto &distributorBucketSpace(_bucketSpaceRepo.get(bucket.getBucketSpace())); + StateChecker::Context c(_distributorComponent, distributorBucketSpace, statsTracker, bucket); fillParentAndChildBuckets(c); fillSiblingBucket(c); @@ -171,11 +176,14 @@ IdealStateManager::prioritize( } IdealStateOperation::SP -IdealStateManager::generateInterceptingSplit(const BucketDatabase::Entry& e, +IdealStateManager::generateInterceptingSplit(BucketSpace bucketSpace, + const BucketDatabase::Entry& e, api::StorageMessage::Priority pri) { NodeMaintenanceStatsTracker statsTracker; - StateChecker::Context c(_distributorComponent, statsTracker, e.getBucketId()); + document::Bucket bucket(bucketSpace, e.getBucketId()); + auto &distributorBucketSpace(_bucketSpaceRepo.get(bucket.getBucketSpace())); + StateChecker::Context c(_distributorComponent, distributorBucketSpace, statsTracker, bucket); if (e.valid()) { c.entry = e; @@ -209,7 +217,8 @@ std::vector<MaintenanceOperation::SP> IdealStateManager::generateAll(const document::Bucket &bucket, NodeMaintenanceStatsTracker& statsTracker) const { - StateChecker::Context c(_distributorComponent, statsTracker, bucket.getBucketId()); + auto &distributorBucketSpace(_bucketSpaceRepo.get(bucket.getBucketSpace())); + StateChecker::Context c(_distributorComponent, distributorBucketSpace, statsTracker, bucket); fillParentAndChildBuckets(c); fillSiblingBucket(c); BucketDatabase::Entry* e(getEntryForPrimaryBucket(c)); @@ -232,7 +241,7 @@ IdealStateManager::generateAll(const document::Bucket &bucket, void IdealStateManager::getBucketStatus( - document::BucketSpace bucketSpace, + BucketSpace bucketSpace, const BucketDatabase::Entry& entry, NodeMaintenanceStatsTracker& statsTracker, std::ostream& out) const @@ -264,8 +273,10 @@ IdealStateManager::getBucketStatus( void IdealStateManager::getBucketStatus(std::ostream& out) const { - StatusBucketVisitor proc(*this, document::BucketSpace::placeHolder(), out); - _distributorComponent.getBucketDatabase().forEach(proc); + BucketSpace bucketSpace(BucketSpace::placeHolder()); + StatusBucketVisitor proc(*this, bucketSpace, out); + auto &distributorBucketSpace(_bucketSpaceRepo.get(bucketSpace)); + distributorBucketSpace.getBucketDatabase().forEach(proc); } } // distributor diff --git a/storage/src/vespa/storage/distributor/idealstatemanager.h b/storage/src/vespa/storage/distributor/idealstatemanager.h index 567efb9f347..ef2bb983aee 100644 --- a/storage/src/vespa/storage/distributor/idealstatemanager.h +++ b/storage/src/vespa/storage/distributor/idealstatemanager.h @@ -40,6 +40,7 @@ class IdealStateManager : public framework::HtmlStatusReporter, public: IdealStateManager(Distributor& owner, + DistributorBucketSpaceRepo& bucketSpaceRepo, DistributorBucketSpace& bucketSpace, DistributorComponentRegister& compReg, bool manageActiveBucketCopies); @@ -67,6 +68,7 @@ public: * with higher priority than the given one. */ IdealStateOperation::SP generateInterceptingSplit( + document::BucketSpace bucketSpace, const BucketDatabase::Entry& e, api::StorageMessage::Priority pri); @@ -84,6 +86,8 @@ public: return _distributorComponent; } StorageComponent::LoadTypeSetSP getLoadTypes() { return _distributorComponent.getLoadTypes(); } + DistributorBucketSpaceRepo &getBucketSpaceRepo() { return _bucketSpaceRepo; } + const DistributorBucketSpaceRepo &getBucketSpaceRepo() const { return _bucketSpaceRepo; } private: void fillParentAndChildBuckets(StateChecker::Context& c) const; @@ -111,6 +115,7 @@ private: SplitBucketStateChecker* _splitBucketStateChecker; DistributorBucketSpaceComponent _distributorComponent; + DistributorBucketSpaceRepo &_bucketSpaceRepo; std::vector<IdealStateOperation::SP> generateOperationsForBucket( StateChecker::Context& c) const; @@ -139,7 +144,6 @@ private: const BucketDatabase::Entry& entry, NodeMaintenanceStatsTracker& statsTracker, std::ostream& out) const; - }; } // distributor diff --git a/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp index b71e2728f5b..246020c191c 100644 --- a/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp @@ -5,6 +5,7 @@ #include <vespa/storageapi/message/persistence.h> #include <vespa/vdslib/state/nodestate.h> #include <vespa/document/fieldvalue/document.h> +#include <vespa/storage/distributor/distributor_bucket_space.h> #include <vespa/log/log.h> LOG_SETUP(".distributor.callback.doc.get"); @@ -46,10 +47,12 @@ GetOperation::GroupId::operator==(const GroupId& other) const } GetOperation::GetOperation(DistributorComponent& manager, + DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::GetCommand> & msg, PersistenceOperationMetricSet& metric) : Operation(), _manager(manager), + _bucketSpace(bucketSpace), _msg(msg), _returnCode(api::ReturnCode::OK), _doc((document::Document*)NULL), @@ -239,7 +242,7 @@ GetOperation::assignTargetNodeGroups() document::BucketId bid = bucketIdFactory.getBucketId(_msg->getDocumentId()); std::vector<BucketDatabase::Entry> entries; - _manager.getBucketDatabase().getParents(bid, entries); + _bucketSpace.getBucketDatabase().getParents(bid, entries); for (uint32_t j = 0; j < entries.size(); ++j) { const BucketDatabase::Entry& e = entries[j]; diff --git a/storage/src/vespa/storage/distributor/operations/external/getoperation.h b/storage/src/vespa/storage/distributor/operations/external/getoperation.h index 12748c38b90..03279a87152 100644 --- a/storage/src/vespa/storage/distributor/operations/external/getoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/getoperation.h @@ -18,11 +18,13 @@ class PersistenceOperationMetricSet; namespace distributor { class DistributorComponent; +class DistributorBucketSpace; class GetOperation : public Operation { public: GetOperation(DistributorComponent& manager, + DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::GetCommand> & msg, PersistenceOperationMetricSet& metric); @@ -71,6 +73,7 @@ private: std::map<GroupId, GroupVector> _responses; DistributorComponent& _manager; + DistributorBucketSpace &_bucketSpace; std::shared_ptr<api::GetCommand> _msg; diff --git a/storage/src/vespa/storage/distributor/operations/external/multioperationoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/multioperationoperation.cpp index 19c693e2a7f..5d93d3e3a5a 100644 --- a/storage/src/vespa/storage/distributor/operations/external/multioperationoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/multioperationoperation.cpp @@ -4,6 +4,7 @@ #include "putoperation.h" #include <vespa/storageapi/message/multioperation.h> #include <vespa/storageapi/message/persistence.h> +#include <vespa/storage/distributor/distributor_bucket_space.h> #include <vespa/log/log.h> LOG_SETUP(".distributor.callback.doc.multioperation"); @@ -14,6 +15,7 @@ namespace storage::distributor { MultiOperationOperation::MultiOperationOperation( DistributorComponent& manager, + DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::MultiOperationCommand> & msg, PersistenceOperationMetricSet& metric) : Operation(), @@ -22,6 +24,7 @@ MultiOperationOperation::MultiOperationOperation( _tracker(_trackerInstance), _msg(msg), _manager(manager), + _bucketSpace(bucketSpace), _minUseBits(manager.getDistributor().getConfig().getMinimalBucketSplit()) { } @@ -36,14 +39,14 @@ MultiOperationOperation::sendToBucket( std::vector<uint16_t> targetNodes; std::vector<MessageTracker::ToSend> createBucketBatch; - if (PutOperation::checkCreateBucket(_manager.getDistribution(), + if (PutOperation::checkCreateBucket(_bucketSpace.getDistribution(), _manager.getClusterState(), e, targetNodes, createBucketBatch, *moCommand)) { - _manager.getBucketDatabase().update(e); + _bucketSpace.getBucketDatabase().update(e); } if (createBucketBatch.size()) { @@ -143,18 +146,18 @@ MultiOperationOperation::onStart(DistributorMessageSender& sender) { if (operationIt->valid()) { document::DocumentId docId = operationIt->getDocumentId(); - document::BucketId bucketId( - _manager.getBucketIdFactory().getBucketId(docId)); + document::Bucket bucket(_msg->getBucket().getBucketSpace(), + _manager.getBucketIdFactory().getBucketId(docId)); - LOG(debug, "Operation with documentid %s mapped to bucketid %s", docId.toString().c_str(), bucketId.toString().c_str()); + LOG(debug, "Operation with documentid %s mapped to bucket %s", docId.toString().c_str(), bucket.toString().c_str()); // OK, we have a bucket ID, must now know which buckets this belongs // to std::vector<BucketDatabase::Entry> entries; - _manager.getBucketDatabase().getParents(bucketId, entries); + _bucketSpace.getBucketDatabase().getParents(bucket.getBucketId(), entries); if (entries.empty()) { - entries.push_back(_manager.createAppropriateBucket(bucketId)); + entries.push_back(_manager.createAppropriateBucket(bucket)); } for (uint32_t i = 0; i < entries.size(); ++i) { diff --git a/storage/src/vespa/storage/distributor/operations/external/multioperationoperation.h b/storage/src/vespa/storage/distributor/operations/external/multioperationoperation.h index a967c7f076f..f63fbbc5458 100644 --- a/storage/src/vespa/storage/distributor/operations/external/multioperationoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/multioperationoperation.h @@ -17,10 +17,13 @@ namespace api { namespace distributor { +class DistributorBucketSpace; + class MultiOperationOperation : public Operation { public: MultiOperationOperation(DistributorComponent& manager, + DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::MultiOperationCommand> & msg, PersistenceOperationMetricSet& metric); ~MultiOperationOperation(); @@ -36,6 +39,7 @@ private: PersistenceMessageTracker& _tracker; std::shared_ptr<api::MultiOperationCommand> _msg; DistributorComponent& _manager; + DistributorBucketSpace &_bucketSpace; uint32_t _minUseBits; uint32_t getMinimumUsedBits(const vdslib::DocumentList& opList) const; diff --git a/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp index 3faf979cc9c..7ef03cb696a 100644 --- a/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp @@ -12,6 +12,7 @@ #include <vespa/storageapi/message/bucket.h> #include <vespa/storageapi/message/persistence.h> #include <vespa/vdslib/distribution/idealnodecalculatorimpl.h> +#include <vespa/storage/distributor/distributor_bucket_space.h> LOG_SETUP(".distributor.callback.doc.put"); @@ -21,6 +22,7 @@ using namespace storage; using document::BucketSpace; PutOperation::PutOperation(DistributorComponent& manager, + DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::PutCommand> & msg, PersistenceOperationMetricSet& metric, SequencingHandle sequencingHandle) @@ -31,7 +33,8 @@ PutOperation::PutOperation(DistributorComponent& manager, msg->getTimestamp()), _tracker(_trackerInstance), _msg(msg), - _manager(manager) + _manager(manager), + _bucketSpace(bucketSpace) { }; @@ -182,7 +185,7 @@ PutOperation::insertDatabaseEntryAndScheduleCreateBucket( // subsequently arriving from the storage node will always overwrite it. BucketCopy copy(BucketCopy::recentlyCreatedCopy( 0, copies[i].getNode().getIndex())); - _manager.updateBucketDatabase(lastBucket, copy, + _manager.updateBucketDatabase(document::Bucket(originalCommand.getBucket().getBucketSpace(), lastBucket), copy, DatabaseUpdate::CREATE_IF_NONEXISTING); } ActiveList active; @@ -190,11 +193,11 @@ PutOperation::insertDatabaseEntryAndScheduleCreateBucket( assert(!multipleBuckets); (void) multipleBuckets; BucketDatabase::Entry entry( - _manager.getBucketDatabase().get(lastBucket)); + _bucketSpace.getBucketDatabase().get(lastBucket)); std::vector<uint16_t> idealState( - _manager.getDistribution().getIdealStorageNodes( + _bucketSpace.getDistribution().getIdealStorageNodes( _manager.getClusterState(), lastBucket, "ui")); - active = ActiveCopy::calculate(idealState, _manager.getDistribution(), + active = ActiveCopy::calculate(idealState, _bucketSpace.getDistribution(), entry); LOG(debug, "Active copies for bucket %s: %s", entry.getBucketId().toString().c_str(), active.toString().c_str()); @@ -203,7 +206,7 @@ PutOperation::insertDatabaseEntryAndScheduleCreateBucket( copy.setActive(true); entry->updateNode(copy); } - _manager.getBucketDatabase().update(entry); + _bucketSpace.getBucketDatabase().update(entry); } for (uint32_t i=0, n=copies.size(); i<n; ++i) { if (!copies[i].isNewCopy()) continue; @@ -274,13 +277,13 @@ PutOperation::onStart(DistributorMessageSender& sender) std::vector<document::BucketId> bucketsToCheckForSplit; lib::IdealNodeCalculatorImpl idealNodeCalculator; - idealNodeCalculator.setDistribution(_manager.getDistribution()); + idealNodeCalculator.setDistribution(_bucketSpace.getDistribution()); idealNodeCalculator.setClusterState(_manager.getClusterState()); OperationTargetResolverImpl targetResolver( - _manager.getBucketDatabase(), + _bucketSpace.getBucketDatabase(), idealNodeCalculator, _manager.getDistributor().getConfig().getMinimalBucketSplit(), - _manager.getDistribution().getRedundancy()); + _bucketSpace.getDistribution().getRedundancy()); OperationTargetList targets(targetResolver.getTargets( OperationTargetResolver::PUT, bid)); @@ -299,7 +302,7 @@ PutOperation::onStart(DistributorMessageSender& sender) // Mark any entries we're not feeding to as not trusted. std::vector<BucketDatabase::Entry> entries; - _manager.getBucketDatabase().getParents(bid, entries); + _bucketSpace.getBucketDatabase().getParents(bid, entries); std::vector<PersistenceMessageTracker::ToSend> createBucketBatch; if (targets.hasAnyNewCopies()) { @@ -338,6 +341,7 @@ PutOperation::onStart(DistributorMessageSender& sender) // TODO(vekterli): only check entries for sendToExisting? for (uint32_t i = 0; i < entries.size(); ++i) { _manager.getDistributor().checkBucketForSplit( + _msg->getBucket().getBucketSpace(), entries[i], _msg->getPriority()); } diff --git a/storage/src/vespa/storage/distributor/operations/external/putoperation.h b/storage/src/vespa/storage/distributor/operations/external/putoperation.h index 191a0116031..8beffe8b2c3 100644 --- a/storage/src/vespa/storage/distributor/operations/external/putoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/putoperation.h @@ -20,10 +20,13 @@ namespace api { } namespace distributor { +class DistributorBucketSpace; + class PutOperation : public SequencedOperation { public: PutOperation(DistributorComponent& manager, + DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::PutCommand> & msg, PersistenceOperationMetricSet& metric, SequencingHandle sequencingHandle = SequencingHandle()); @@ -72,6 +75,7 @@ private: std::shared_ptr<api::PutCommand> _msg; DistributorComponent& _manager; + DistributorBucketSpace &_bucketSpace; }; } // distributor diff --git a/storage/src/vespa/storage/distributor/operations/external/removelocationoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/removelocationoperation.cpp index fdf7cf4860f..2584244023b 100644 --- a/storage/src/vespa/storage/distributor/operations/external/removelocationoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/removelocationoperation.cpp @@ -6,6 +6,7 @@ #include <vespa/document/fieldvalue/document.h> #include <vespa/document/repo/documenttyperepo.h> #include <vespa/document/select/parser.h> +#include <vespa/storage/distributor/distributor_bucket_space.h> #include <vespa/log/log.h> LOG_SETUP(".distributor.callback.doc.removelocation"); @@ -17,6 +18,7 @@ using document::BucketSpace; RemoveLocationOperation::RemoveLocationOperation( DistributorComponent& manager, + DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::RemoveLocationCommand> & msg, PersistenceOperationMetricSet& metric) : Operation(), @@ -26,7 +28,8 @@ RemoveLocationOperation::RemoveLocationOperation( 0), _tracker(_trackerInstance), _msg(msg), - _manager(manager) + _manager(manager), + _bucketSpace(bucketSpace) {} RemoveLocationOperation::~RemoveLocationOperation() {} @@ -68,7 +71,7 @@ RemoveLocationOperation::onStart(DistributorMessageSender& sender) } std::vector<BucketDatabase::Entry> entries; - _manager.getBucketDatabase().getAll(bid, entries); + _bucketSpace.getBucketDatabase().getAll(bid, entries); bool sent = false; for (uint32_t j = 0; j < entries.size(); ++j) { diff --git a/storage/src/vespa/storage/distributor/operations/external/removelocationoperation.h b/storage/src/vespa/storage/distributor/operations/external/removelocationoperation.h index 5feff9ba642..64aeb19bae9 100644 --- a/storage/src/vespa/storage/distributor/operations/external/removelocationoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/removelocationoperation.h @@ -10,10 +10,13 @@ namespace api { class RemoveLocationCommand; } namespace distributor { +class DistributorBucketSpace; + class RemoveLocationOperation : public Operation { public: RemoveLocationOperation(DistributorComponent& manager, + DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::RemoveLocationCommand> & msg, PersistenceOperationMetricSet& metric); ~RemoveLocationOperation(); @@ -34,6 +37,7 @@ private: std::shared_ptr<api::RemoveLocationCommand> _msg; DistributorComponent& _manager; + DistributorBucketSpace &_bucketSpace; }; } diff --git a/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp index d2af1f6f9c5..4b9a7b3f173 100644 --- a/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "removeoperation.h" #include <vespa/storageapi/message/persistence.h> +#include <vespa/storage/distributor/distributor_bucket_space.h> #include <vespa/log/log.h> LOG_SETUP(".distributor.operation.external.remove"); @@ -11,6 +12,7 @@ using namespace storage; using document::BucketSpace; RemoveOperation::RemoveOperation(DistributorComponent& manager, + DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::RemoveCommand> & msg, PersistenceOperationMetricSet& metric, SequencingHandle sequencingHandle) @@ -20,7 +22,8 @@ RemoveOperation::RemoveOperation(DistributorComponent& manager, manager, msg->getTimestamp()), _tracker(_trackerInstance), _msg(msg), - _manager(manager) + _manager(manager), + _bucketSpace(bucketSpace) { } @@ -36,7 +39,7 @@ RemoveOperation::onStart(DistributorMessageSender& sender) _msg->getDocumentId())); std::vector<BucketDatabase::Entry> entries; - _manager.getBucketDatabase().getParents(bucketId, entries); + _bucketSpace.getBucketDatabase().getParents(bucketId, entries); bool sent = false; diff --git a/storage/src/vespa/storage/distributor/operations/external/removeoperation.h b/storage/src/vespa/storage/distributor/operations/external/removeoperation.h index f48193ee2bf..7794be73ac8 100644 --- a/storage/src/vespa/storage/distributor/operations/external/removeoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/removeoperation.h @@ -10,10 +10,13 @@ namespace api { class RemoveCommand; } namespace distributor { +class DistributorBucketSpace; + class RemoveOperation : public SequencedOperation { public: RemoveOperation(DistributorComponent& manager, + DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::RemoveCommand> & msg, PersistenceOperationMetricSet& metric, SequencingHandle sequencingHandle = SequencingHandle()); @@ -33,6 +36,7 @@ private: std::shared_ptr<api::RemoveCommand> _msg; DistributorComponent& _manager; + DistributorBucketSpace &_bucketSpace; }; } diff --git a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp index cb395d42c9a..4e2f8a3169a 100644 --- a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp @@ -3,6 +3,7 @@ #include <vespa/storage/distributor/distributorcomponent.h> #include <vespa/storageapi/message/persistence.h> #include <vespa/storageapi/message/stat.h> +#include <vespa/storage/distributor/distributor_bucket_space.h> #include <vespa/log/log.h> LOG_SETUP(".distributor.callback.statbucket"); @@ -12,9 +13,11 @@ namespace distributor { StatBucketOperation::StatBucketOperation( DistributorComponent& manager, + DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::StatBucketCommand> & cmd) : Operation(), _manager(manager), + _bucketSpace(bucketSpace), _command(cmd) { } @@ -35,7 +38,7 @@ StatBucketOperation::onStart(DistributorMessageSender& sender) std::vector<uint16_t> nodes; BucketDatabase::Entry entry( - _manager.getBucketDatabase().get(_command->getBucketId())); + _bucketSpace.getBucketDatabase().get(_command->getBucketId())); if (entry.valid()) { nodes = entry->getNodes(); diff --git a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.h b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.h index 79fb2b4e642..af448c2dd55 100644 --- a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.h @@ -16,11 +16,13 @@ namespace api { class StatBucketCommand; } namespace distributor { class DistributorComponent; +class DistributorBucketSpace; class StatBucketOperation : public Operation { public: StatBucketOperation(DistributorComponent& manager, + DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::StatBucketCommand> & cmd); ~StatBucketOperation(); @@ -31,6 +33,7 @@ public: void onReceive(DistributorMessageSender& sender, const std::shared_ptr<api::StorageReply> & msg) override; private: DistributorComponent& _manager; + DistributorBucketSpace &_bucketSpace; std::shared_ptr<api::StatBucketCommand> _command; diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp index b586d5ffed9..79ffee7430c 100644 --- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp @@ -10,6 +10,7 @@ #include <vespa/storageapi/message/persistence.h> #include <vespa/storageapi/message/batch.h> #include <vespa/vespalib/stllike/hash_map.hpp> +#include <vespa/storage/distributor/distributor_bucket_space.h> #include <vespa/log/log.h> LOG_SETUP(".distributor.callback.twophaseupdate"); @@ -22,6 +23,7 @@ namespace distributor { TwoPhaseUpdateOperation::TwoPhaseUpdateOperation( DistributorComponent& manager, + DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::UpdateCommand>& msg, DistributorMetricSet& metrics, SequencingHandle sequencingHandle) @@ -32,6 +34,7 @@ TwoPhaseUpdateOperation::TwoPhaseUpdateOperation( _updateCmd(msg), _updateReply(), _manager(manager), + _bucketSpace(bucketSpace), _sendState(SendState::NONE_SENT), _mode(Mode::FAST_PATH), _replySent(false) @@ -148,7 +151,7 @@ TwoPhaseUpdateOperation::isFastPathPossible() const { // Fast path iff bucket exists AND is consistent (split and copies). std::vector<BucketDatabase::Entry> entries; - _manager.getBucketDatabase().getParents(_updateDocBucketId, entries); + _bucketSpace.getBucketDatabase().getParents(_updateDocBucketId, entries); if (entries.size() != 1) { return false; @@ -161,7 +164,7 @@ TwoPhaseUpdateOperation::startFastPathUpdate(DistributorMessageSender& sender) { _mode = Mode::FAST_PATH; std::shared_ptr<UpdateOperation> updateOperation( - new UpdateOperation(_manager, _updateCmd, _updateMetric)); + new UpdateOperation(_manager, _bucketSpace, _updateCmd, _updateMetric)); IntermediateMessageSender intermediate( _sentMessageMap, updateOperation, sender); @@ -189,7 +192,7 @@ TwoPhaseUpdateOperation::startSafePathUpdate(DistributorMessageSender& sender) "[all]")); copyMessageSettings(*_updateCmd, *get); std::shared_ptr<GetOperation> getOperation( - std::make_shared<GetOperation>(_manager, get, _getMetric)); + std::make_shared<GetOperation>(_manager, _bucketSpace, get, _getMetric)); IntermediateMessageSender intermediate( _sentMessageMap, getOperation, sender); @@ -224,8 +227,9 @@ TwoPhaseUpdateOperation::onStart(DistributorMessageSender& sender) { bool TwoPhaseUpdateOperation::lostBucketOwnershipBetweenPhases() const { + document::Bucket updateDocBucket(_updateCmd->getBucket().getBucketSpace(), _updateDocBucketId); BucketOwnership bo(_manager.checkOwnershipInPendingAndCurrentState( - _updateDocBucketId)); + updateDocBucket)); return !bo.isOwned(); } @@ -256,7 +260,7 @@ TwoPhaseUpdateOperation::schedulePutsWithUpdatedDocument( new api::PutCommand(bucket, doc, putTimestamp)); copyMessageSettings(*_updateCmd, *put); std::shared_ptr<PutOperation> putOperation( - new PutOperation(_manager, put, _putMetric)); + new PutOperation(_manager, _bucketSpace, put, _putMetric)); IntermediateMessageSender intermediate( _sentMessageMap, putOperation, sender); diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h index 820dce051eb..e3fb6c93a3a 100644 --- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h @@ -21,6 +21,8 @@ class CreateBucketReply; namespace distributor { +class DistributorBucketSpace; + /* * General functional outline: * @@ -49,6 +51,7 @@ class TwoPhaseUpdateOperation : public SequencedOperation { public: TwoPhaseUpdateOperation(DistributorComponent& manager, + DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::UpdateCommand> & msg, DistributorMetricSet& metrics, SequencingHandle sequencingHandle = SequencingHandle()); @@ -124,6 +127,7 @@ private: std::shared_ptr<api::UpdateCommand> _updateCmd; std::shared_ptr<api::StorageReply> _updateReply; DistributorComponent& _manager; + DistributorBucketSpace &_bucketSpace; SentMessageMap _sentMessageMap; SendState _sendState; Mode _mode; diff --git a/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp index 89bff0d1382..d622c42b321 100644 --- a/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp @@ -5,6 +5,7 @@ #include <vespa/storageapi/message/bucket.h> #include <vespa/storageapi/message/persistence.h> #include <vespa/document/fieldvalue/document.h> +#include <vespa/storage/distributor/distributor_bucket_space.h> #include <vespa/log/log.h> LOG_SETUP(".distributor.callback.doc.update"); @@ -15,6 +16,7 @@ using namespace storage; using document::BucketSpace; UpdateOperation::UpdateOperation(DistributorComponent& manager, + DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::UpdateCommand> & msg, PersistenceOperationMetricSet& metric) : Operation(), @@ -24,7 +26,8 @@ UpdateOperation::UpdateOperation(DistributorComponent& manager, msg->getTimestamp()), _tracker(_trackerInstance), _msg(msg), - _manager(manager) + _manager(manager), + _bucketSpace(bucketSpace) { } @@ -69,7 +72,7 @@ UpdateOperation::onStart(DistributorMessageSender& sender) _msg->getDocumentId())); std::vector<BucketDatabase::Entry> entries; - _manager.getBucketDatabase().getParents(bucketId, entries); + _bucketSpace.getBucketDatabase().getParents(bucketId, entries); if (entries.empty()) { _tracker.fail(sender, diff --git a/storage/src/vespa/storage/distributor/operations/external/updateoperation.h b/storage/src/vespa/storage/distributor/operations/external/updateoperation.h index 802927918ca..a23fd2ab876 100644 --- a/storage/src/vespa/storage/distributor/operations/external/updateoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/updateoperation.h @@ -17,10 +17,13 @@ class CreateBucketReply; namespace distributor { +class DistributorBucketSpace; + class UpdateOperation : public Operation { public: UpdateOperation(DistributorComponent& manager, + DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::UpdateCommand> & msg, PersistenceOperationMetricSet& metric); @@ -40,6 +43,7 @@ private: std::shared_ptr<api::UpdateCommand> _msg; DistributorComponent& _manager; + DistributorBucketSpace &_bucketSpace; std::pair<document::BucketId, uint16_t> _newestTimestampLocation; bool anyStorageNodesAvailable() const; diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp index 7ef00cad480..a4cebcc7c3e 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp @@ -45,11 +45,13 @@ VisitorOperation::BucketInfo::toString() const VisitorOperation::VisitorOperation( DistributorComponent& owner, + DistributorBucketSpace &bucketSpace, const api::CreateVisitorCommand::SP& m, const Config& config, VisitorMetricSet& metrics) : Operation(), _owner(owner), + _bucketSpace(bucketSpace), _msg(m), _sentReply(false), _config(config), @@ -275,7 +277,8 @@ VisitorOperation::verifyDistributorIsNotDown(const lib::ClusterState& state) void VisitorOperation::verifyDistributorOwnsBucket(const document::BucketId& bid) { - BucketOwnership bo(_owner.checkOwnershipInPendingAndCurrentState(bid)); + document::Bucket bucket(_msg->getBucketSpace(), bid); + BucketOwnership bo(_owner.checkOwnershipInPendingAndCurrentState(bucket)); if (!bo.isOwned()) { verifyDistributorIsNotDown(bo.getNonOwnedState()); std::string systemStateStr = bo.getNonOwnedState().toString(); @@ -457,7 +460,7 @@ bool VisitorOperation::expandBucketAll() { std::vector<BucketDatabase::Entry> entries; - _owner.getBucketDatabase().getAll(_superBucket.bid, entries); + _bucketSpace.getBucketDatabase().getAll(_superBucket.bid, entries); return pickBucketsToVisit(entries); } @@ -465,7 +468,7 @@ bool VisitorOperation::expandBucketContaining() { std::vector<BucketDatabase::Entry> entries; - _owner.getBucketDatabase().getParents(_superBucket.bid, entries); + _bucketSpace.getBucketDatabase().getParents(_superBucket.bid, entries); return pickBucketsToVisit(entries); } @@ -518,7 +521,7 @@ VisitorOperation::expandBucketContained() uint32_t maxBuckets = _msg->getMaxBucketsPerVisitor(); std::unique_ptr<document::BucketId> bid = getBucketIdAndLast( - _owner.getBucketDatabase(), + _bucketSpace.getBucketDatabase(), _superBucket.bid, _lastBucket); @@ -535,7 +538,7 @@ VisitorOperation::expandBucketContained() _superBucket.subBucketsVisitOrder.push_back(*bid); _superBucket.subBuckets[*bid] = BucketInfo(); - bid = getBucketIdAndLast(_owner.getBucketDatabase(), + bid = getBucketIdAndLast(_bucketSpace.getBucketDatabase(), _superBucket.bid, *bid); } @@ -843,7 +846,7 @@ VisitorOperation::assignBucketsToNodes(NodeToBucketsMap& nodeToBucketsMap) continue; } - BucketDatabase::Entry entry(_owner.getBucketDatabase().get(subBucket)); + BucketDatabase::Entry entry(_bucketSpace.getBucketDatabase().get(subBucket)); if (!bucketIsValidAndConsistent(entry)) { return false; } diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h index 006e2916335..f35a9dcb3ec 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h @@ -18,6 +18,7 @@ class VisitorMetricSet; namespace distributor { class DistributorComponent; +class DistributorBucketSpace; class VisitorOperation : public Operation { @@ -33,6 +34,7 @@ public: }; VisitorOperation(DistributorComponent& manager, + DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::CreateVisitorCommand> & msg, const Config& config, VisitorMetricSet& metrics); @@ -147,6 +149,7 @@ private: std::unique_ptr<document::OrderingSpecification> _ordering; DistributorComponent& _owner; + DistributorBucketSpace &_bucketSpace; SentMessagesMap _sentMessages; api::CreateVisitorCommand::SP _msg; diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.cpp index c1d95ac1c92..d78262709e3 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.cpp @@ -52,7 +52,7 @@ GarbageCollectionOperation::onReceive(DistributorMessageSender&, if (!rep->getResult().failed()) { _manager->getDistributorComponent().updateBucketDatabase( - getBucketId(), + getBucket(), BucketCopy(_manager->getDistributorComponent().getUniqueTimestamp(), node, rep->getBucketInfo())); diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/joinoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/joinoperation.cpp index 14c86b2ee7b..77135f56399 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/joinoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/joinoperation.cpp @@ -98,7 +98,8 @@ JoinOperation::onReceive(DistributorMessageSender&, const api::StorageReply::SP& const std::vector<document::BucketId>& sourceBuckets( rep.getSourceBuckets()); for (uint32_t i = 0; i < sourceBuckets.size(); i++) { - _manager->getDistributorComponent().removeNodeFromDB(sourceBuckets[i], node); + document::Bucket sourceBucket(msg->getBucket().getBucketSpace(), sourceBuckets[i]); + _manager->getDistributorComponent().removeNodeFromDB(sourceBucket, node); } // Add new buckets. @@ -107,7 +108,7 @@ JoinOperation::onReceive(DistributorMessageSender&, const api::StorageReply::SP& getBucketId().toString().c_str()); } else { _manager->getDistributorComponent().updateBucketDatabase( - getBucketId(), + getBucket(), BucketCopy(_manager->getDistributorComponent().getUniqueTimestamp(), node, rep.getBucketInfo()), diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.cpp index e3b19848a89..6c0245cb590 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.cpp @@ -38,7 +38,7 @@ RemoveBucketOperation::onStartInternal(DistributorMessageSender& sender) _ok = true; if (!getNodes().empty()) { - _manager->getDistributorComponent().removeNodesFromDB(getBucketId(), getNodes()); + _manager->getDistributorComponent().removeNodesFromDB(getBucket(), getNodes()); for (uint32_t i = 0; i < msgs.size(); ++i) { _tracker.queueCommand(msgs[i].second, msgs[i].first); } @@ -81,7 +81,7 @@ RemoveBucketOperation::onReceiveInternal(const std::shared_ptr<api::StorageReply rep->getBucketInfo().toString().c_str()); _manager->getDistributorComponent().updateBucketDatabase( - getBucketId(), + getBucket(), BucketCopy(_manager->getDistributorComponent().getUniqueTimestamp(), node, rep->getBucketInfo()), diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.cpp index 0dc3f5ed72e..a8f547afe45 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.cpp @@ -103,7 +103,7 @@ SplitOperation::onReceive(DistributorMessageSender&, const api::StorageReply::SP // copies would be arbitrarily determined by which copy managed // to finish its split first. _manager->getDistributorComponent().updateBucketDatabase( - sinfo.first, copy, + document::Bucket(msg->getBucket().getBucketSpace(), sinfo.first), copy, (DatabaseUpdate::CREATE_IF_NONEXISTING | DatabaseUpdate::RESET_TRUSTED)); diff --git a/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp b/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp index 446a80f85ab..a1b43149963 100644 --- a/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp +++ b/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp @@ -83,7 +83,7 @@ PersistenceMessageTrackerImpl::receiveReply( void PersistenceMessageTrackerImpl::revert( MessageSender& sender, - const std::vector<std::pair<document::BucketId, uint16_t> > revertNodes) + const std::vector<BucketNodePair> revertNodes) { if (_revertTimestamp != 0) { // Since we're reverting, all received bucket info is voided. @@ -93,9 +93,8 @@ PersistenceMessageTrackerImpl::revert( reverts.push_back(_revertTimestamp); for (uint32_t i = 0; i < revertNodes.size(); i++) { - document::Bucket bucket(document::BucketSpace::placeHolder(), revertNodes[i].first); std::shared_ptr<api::RevertCommand> toRevert( - new api::RevertCommand(bucket, reverts)); + new api::RevertCommand(revertNodes[i].first, reverts)); toRevert->setPriority(_priority); queueCommand(toRevert, revertNodes[i].second); } @@ -169,7 +168,7 @@ PersistenceMessageTrackerImpl::checkCopiesDeleted() iter++) { BucketDatabase::Entry dbentry = - _manager.getBucketDatabase().get(iter->first); + _manager.getBucketDatabase().get(iter->first.getBucketId()); if (!dbentry.valid()) { continue; @@ -188,7 +187,7 @@ PersistenceMessageTrackerImpl::checkCopiesDeleted() if (!missing.empty()) { std::ostringstream msg; - msg << iter->first << " was deleted from nodes [" + msg << iter->first.toString() << " was deleted from nodes [" << commaSeparated(missing) << "] after message was sent but before it was done. Sent to [" << commaSeparated(total) @@ -207,7 +206,7 @@ PersistenceMessageTrackerImpl::addBucketInfoFromReply( uint16_t node, const api::BucketInfoReply& reply) { - const document::BucketId& bucket(reply.getBucketId()); + document::Bucket bucket(reply.getBucket()); const api::BucketInfo& bucketInfo(reply.getBucketInfo()); if (reply.hasBeenRemapped()) { @@ -295,7 +294,7 @@ PersistenceMessageTrackerImpl::handleCreateBucketReply( && reply.getResult().getResult() != api::ReturnCode::EXISTS) { LOG(spam, "Create bucket reply failed, so deleting it from bucket db"); - _manager.removeNodeFromDB(reply.getBucketId(), node); + _manager.removeNodeFromDB(reply.getBucket(), node); LOG_BUCKET_OPERATION_NO_LOCK( reply.getBucketId(), vespalib::make_string( @@ -314,8 +313,7 @@ PersistenceMessageTrackerImpl::handlePersistenceReply( } if (reply.getResult().success()) { logSuccessfulReply(node, reply); - _revertNodes.push_back(std::pair<document::BucketId, uint16_t>( - reply.getBucketId(), node)); + _revertNodes.emplace_back(reply.getBucket(), node); } else if (!hasSentReply()) { updateFailureResult(reply); } diff --git a/storage/src/vespa/storage/distributor/persistencemessagetracker.h b/storage/src/vespa/storage/distributor/persistencemessagetracker.h index 617dd5f1211..96200342e08 100644 --- a/storage/src/vespa/storage/distributor/persistencemessagetracker.h +++ b/storage/src/vespa/storage/distributor/persistencemessagetracker.h @@ -29,7 +29,7 @@ class PersistenceMessageTrackerImpl : public PersistenceMessageTracker, public MessageTracker { private: - typedef std::map<document::BucketId, std::vector<BucketCopy> > BucketInfoMap; + typedef std::map<document::Bucket, std::vector<BucketCopy> > BucketInfoMap; BucketInfoMap _remapBucketInfo; BucketInfoMap _bucketInfo; @@ -52,7 +52,7 @@ public: void updateFromReply(MessageSender& sender, api::BucketInfoReply& reply, uint16_t node) override; std::shared_ptr<api::BucketInfoReply>& getReply() override { return _reply; } - typedef std::pair<document::BucketId, uint16_t> BucketNodePair; + typedef std::pair<document::Bucket, uint16_t> BucketNodePair; void revert(MessageSender& sender, const std::vector<BucketNodePair> revertNodes); @@ -72,7 +72,7 @@ private: std::shared_ptr<api::BucketInfoReply> _reply; DistributorComponent& _manager; api::Timestamp _revertTimestamp; - std::vector<std::pair<document::BucketId, uint16_t> > _revertNodes; + std::vector<BucketNodePair> _revertNodes; mbus::TraceNode _trace; framework::MilliSecTimer _requestTimer; uint8_t _priority; diff --git a/storage/src/vespa/storage/distributor/statechecker.cpp b/storage/src/vespa/storage/distributor/statechecker.cpp index 0107430bb96..f959e5a80fb 100644 --- a/storage/src/vespa/storage/distributor/statechecker.cpp +++ b/storage/src/vespa/storage/distributor/statechecker.cpp @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "statechecker.h" #include "distributorcomponent.h" +#include "distributor_bucket_space.h" #include <vespa/log/log.h> LOG_SETUP(".distributor.statechecker"); @@ -59,22 +60,23 @@ StateChecker::Result::createStoredResult( } StateChecker::Context::Context(const DistributorComponent& c, + const DistributorBucketSpace &distributorBucketSpace, NodeMaintenanceStatsTracker& statsTracker, - const document::BucketId& bid) - : bucketId(bid), - siblingBucket(c.getSibling(bid)), + const document::Bucket &bucket_) + : bucket(bucket_), + siblingBucket(c.getSibling(bucket.getBucketId())), systemState(c.getClusterState()), distributorConfig(c.getDistributor().getConfig()), - distribution(c.getDistribution()), + distribution(distributorBucketSpace.getDistribution()), gcTimeCalculator(c.getDistributor().getBucketIdHasher(), std::chrono::seconds(distributorConfig .getGarbageCollectionInterval())), component(c), - db(c.getBucketDatabase()), + db(distributorBucketSpace.getBucketDatabase()), stats(statsTracker) { idealState = - distribution.getIdealStorageNodes(systemState, bucketId); + distribution.getIdealStorageNodes(systemState, bucket.getBucketId()); unorderedIdealState.insert(idealState.begin(), idealState.end()); } diff --git a/storage/src/vespa/storage/distributor/statechecker.h b/storage/src/vespa/storage/distributor/statechecker.h index fbadd5642d4..9f9f57dd3bf 100644 --- a/storage/src/vespa/storage/distributor/statechecker.h +++ b/storage/src/vespa/storage/distributor/statechecker.h @@ -19,6 +19,7 @@ class DistributorConfiguration; namespace distributor { class DistributorComponent; +class DistributorBucketSpace; class NodeMaintenanceStatsTracker; /** @@ -45,15 +46,16 @@ public: struct Context { Context(const DistributorComponent&, + const DistributorBucketSpace &distributorBucketSpace, NodeMaintenanceStatsTracker&, - const document::BucketId& bid); + const document::Bucket &bucket_); ~Context(); Context(const Context &) = delete; Context & operator =(const Context &) = delete; // Per bucket - document::BucketId bucketId; + document::Bucket bucket; document::BucketId siblingBucket; BucketDatabase::Entry entry; @@ -82,7 +84,8 @@ public: return siblingEntry; } - document::Bucket getBucket() const { return document::Bucket(document::BucketSpace::placeHolder(), bucketId); } + document::Bucket getBucket() const { return bucket; } + document::BucketId getBucketId() const { return bucket.getBucketId(); } std::string toString() const; }; diff --git a/storage/src/vespa/storage/distributor/statecheckers.cpp b/storage/src/vespa/storage/distributor/statecheckers.cpp index 35d111a8c38..4c5301dd8c2 100644 --- a/storage/src/vespa/storage/distributor/statecheckers.cpp +++ b/storage/src/vespa/storage/distributor/statecheckers.cpp @@ -27,12 +27,12 @@ SplitBucketStateChecker::validForSplit(StateChecker::Context& c) if (c.entry->getNodeCount() == 0) { LOG(spam, "Can't split bucket %s, since it has no copies", - c.bucketId.toString().c_str()); + c.bucket.toString().c_str()); return false; } // Can't split anymore if we already used 58 bits. - if (c.bucketId.getUsedBits() >= 58) { + if (c.getBucketId().getUsedBits() >= 58) { return false; } @@ -145,7 +145,7 @@ SplitBucketStateChecker::check(StateChecker::Context& c) { } // Always split it if it has less used bits than the minimum. - if (c.bucketId.getUsedBits() < c.distributorConfig.getMinimalBucketSplit()) { + if (c.getBucketId().getUsedBits() < c.distributorConfig.getMinimalBucketSplit()) { return generateMinimumBucketSplitOperation(c); } return Result::noMaintenanceNeeded(); @@ -217,7 +217,7 @@ JoinBucketsStateChecker::siblingsAreInSync(const Context& context) const LOG(spam, "Not joining bucket %s because sibling bucket %s had different " "node count", - context.bucketId.toString().c_str(), + context.bucket.toString().c_str(), context.siblingBucket.toString().c_str()); return false; } @@ -238,7 +238,7 @@ JoinBucketsStateChecker::siblingsAreInSync(const Context& context) const "does not have the same node set, or inconsistent joins cannot be " "performed either due to config or because replicas were not in " "their ideal location", - context.bucketId.toString().c_str(), + context.bucket.toString().c_str(), context.siblingBucket.toString().c_str()); return false; } @@ -247,7 +247,7 @@ JoinBucketsStateChecker::siblingsAreInSync(const Context& context) const LOG(spam, "Not joining bucket %s because it or %s is out of sync " "and syncing it may cause it to become too large", - context.bucketId.toString().c_str(), + context.bucket.toString().c_str(), context.siblingBucket.toString().c_str()); return false; } @@ -258,8 +258,8 @@ JoinBucketsStateChecker::siblingsAreInSync(const Context& context) const bool JoinBucketsStateChecker::singleBucketJoinIsConsistent(const Context& c) const { - document::BucketId joinTarget(c.bucketId.getUsedBits() - 1, - c.bucketId.getRawId()); + document::BucketId joinTarget(c.getBucketId().getUsedBits() - 1, + c.getBucketId().getRawId()); // If there are 2 children under the potential join target bucket, joining // would cause the bucket tree to become inconsistent. The reason for this // being that "moving" a bucket one bit up in the tree (and into @@ -305,30 +305,30 @@ JoinBucketsStateChecker::shouldJoin(const Context& c) const { if (c.entry->getNodeCount() == 0) { LOG(spam, "Not joining bucket %s because it has no nodes", - c.bucketId.toString().c_str()); + c.bucket.toString().c_str()); return false; } if (contextBucketHasTooManyReplicas(c)) { LOG(spam, "Not joining %s because it has too high replication level", - c.bucketId.toString().c_str()); + c.bucket.toString().c_str()); return false; } if (c.distributorConfig.getJoinSize() == 0 && c.distributorConfig.getJoinCount() == 0) { LOG(spam, "Not joining bucket %s because join is disabled", - c.bucketId.toString().c_str()); + c.bucket.toString().c_str()); return false; } - if (bucketAtDistributionBitLimit(c.bucketId, c)) { + if (bucketAtDistributionBitLimit(c.getBucketId(), c)) { LOG(spam, "Not joining bucket %s because it is below the min split " "count (config: %u, cluster state: %u, bucket has: %u)", - c.bucketId.toString().c_str(), + c.bucket.toString().c_str(), c.distributorConfig.getMinimalBucketSplit(), c.systemState.getDistributionBitCount(), - c.bucketId.getUsedBits()); + c.getBucketId().getUsedBits()); return false; } @@ -337,11 +337,11 @@ JoinBucketsStateChecker::shouldJoin(const Context& c) const } if (c.getSiblingEntry().valid()) { - if (!isFirstSibling(c.bucketId)) { + if (!isFirstSibling(c.getBucketId())) { LOG(spam, "Not joining bucket %s because it is the second sibling of " "%s and not the first", - c.bucketId.toString().c_str(), + c.bucket.toString().c_str(), c.siblingBucket.toString().c_str()); return false; } @@ -427,8 +427,8 @@ JoinBucketsStateChecker::computeJoinBucket(const Context& c) const { // Always decrease by at least 1 bit, as we could not get here unless this // were a valid outcome. - unsigned int level = c.bucketId.getUsedBits() - 1; - document::BucketId target(level, c.bucketId.getRawId()); + unsigned int level = c.getBucketId().getUsedBits() - 1; + document::BucketId target(level, c.getBucketId().getRawId()); // Push bucket up the tree as long as it gets no siblings. This means // joins involving 2 source buckets will currently only be decreased by 1 @@ -436,7 +436,7 @@ JoinBucketsStateChecker::computeJoinBucket(const Context& c) const // be decreased by multiple bits. We may want to optimize joins for cases // with 2 source buckets in the future. while (true) { - document::BucketId candidate(level, c.bucketId.getRawId()); + document::BucketId candidate(level, c.getBucketId().getRawId()); if (bucketHasMultipleChildren(candidate, c) || !legalBucketSplitLevel(candidate, c)) { @@ -458,15 +458,15 @@ JoinBucketsStateChecker::check(StateChecker::Context& c) } document::Bucket joinedBucket(computeJoinBucket(c)); - assert(joinedBucket.getBucketId().getUsedBits() < c.bucketId.getUsedBits()); + assert(joinedBucket.getBucketId().getUsedBits() < c.getBucketId().getUsedBits()); std::vector<document::BucketId> sourceBuckets; if (c.getSiblingEntry().valid()) { sourceBuckets.push_back(c.siblingBucket); } else { - sourceBuckets.push_back(c.bucketId); + sourceBuckets.push_back(c.getBucketId()); } - sourceBuckets.push_back(c.bucketId); + sourceBuckets.push_back(c.getBucketId()); IdealStateOperation::UP op(new JoinOperation( c.component.getClusterName(), BucketAndNodes(joinedBucket, c.entry->getNodes()), @@ -568,7 +568,7 @@ SplitInconsistentStateChecker::check(StateChecker::Context& c) return Result::noMaintenanceNeeded(); } - if (!isLeastSplitBucket(c.bucketId, c.entries)) { + if (!isLeastSplitBucket(c.getBucketId(), c.entries)) { return Result::noMaintenanceNeeded(); } @@ -581,7 +581,7 @@ SplitInconsistentStateChecker::check(StateChecker::Context& c) op->setPriority(c.distributorConfig.getMaintenancePriorities() .splitInconsistentBucket); - op->setDetailedReason(getReason(c.bucketId, c.entries)); + op->setDetailedReason(getReason(c.getBucketId(), c.entries)); return Result::createStoredResult(std::move(op), MaintenancePriority::HIGH); } @@ -856,7 +856,7 @@ SynchronizeAndMoveStateChecker::check(StateChecker::Context& c) } else { LOG(spam, "Bucket %s: No need for merge, as bucket is in consistent state " "(or inconsistent buckets are empty) %s", - c.bucketId.toString().c_str(), + c.bucket.toString().c_str(), c.entry->toString().c_str()); return Result::noMaintenanceNeeded(); } @@ -1119,7 +1119,7 @@ GarbageCollectionStateChecker::needsGarbageCollection(const Context& c) const std::chrono::seconds currentTime( c.component.getClock().getTimeInSeconds().getTime()); - return c.gcTimeCalculator.shouldGc(c.bucketId, currentTime, lastRunAt); + return c.gcTimeCalculator.shouldGc(c.getBucketId(), currentTime, lastRunAt); } StateChecker::Result diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp index f1daa42ca39..88a7343f8c8 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp @@ -2,20 +2,21 @@ #include "filestormanager.h" -#include <vespa/storageapi/message/bucketsplitting.h> -#include <vespa/storageapi/message/multioperation.h> -#include <vespa/storageapi/message/persistence.h> -#include <vespa/storageapi/message/removelocation.h> -#include <vespa/storageapi/message/state.h> +#include <vespa/storage/bucketdb/lockablemap.hpp> #include <vespa/storage/common/bucketmessages.h> +#include <vespa/storage/common/bucketoperationlogger.h> +#include <vespa/storage/common/content_bucket_space_repo.h> +#include <vespa/storage/common/messagebucket.h> #include <vespa/storage/config/config-stor-server.h> +#include <vespa/storage/persistence/bucketownershipnotifier.h> #include <vespa/storage/persistence/persistencethread.h> #include <vespa/storage/storageutil/log.h> -#include <vespa/storage/common/messagebucket.h> -#include <vespa/storage/persistence/bucketownershipnotifier.h> #include <vespa/storageapi/message/batch.h> -#include <vespa/storage/common/bucketoperationlogger.h> -#include <vespa/storage/bucketdb/lockablemap.hpp> +#include <vespa/storageapi/message/bucketsplitting.h> +#include <vespa/storageapi/message/multioperation.h> +#include <vespa/storageapi/message/persistence.h> +#include <vespa/storageapi/message/removelocation.h> +#include <vespa/storageapi/message/state.h> #include <vespa/vespalib/stllike/hash_map.hpp> #include <vespa/vespalib/util/stringfmt.h> @@ -974,7 +975,7 @@ FileStorManager::updateState() if (_nodeUpInLastNodeStateSeenByProvider && !nodeUp) { LOG(debug, "Received cluster state where this node is down; de-activating all buckets in database"); Deactivator deactivator; - _component.getBucketDatabase(BucketSpace::placeHolder()).all(deactivator, "FileStorManager::updateState"); + _component.getBucketSpaceRepo().forEachBucket(deactivator, "FileStorManager::updateState"); } _provider->setClusterState(spiState); _nodeUpInLastNodeStateSeenByProvider = nodeUp; |