diff options
77 files changed, 1633 insertions, 1453 deletions
diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ConfigserverSslContextFactoryProvider.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ConfigserverSslContextFactoryProvider.java index 61ac8f7a7e2..bb3216ba3ba 100644 --- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ConfigserverSslContextFactoryProvider.java +++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ConfigserverSslContextFactoryProvider.java @@ -109,6 +109,9 @@ public class ConfigserverSslContextFactoryProvider extends AbstractComponent imp AthenzService configserverIdentity, ZtsClient ztsClient, AthenzProviderServiceConfig.Zones zoneConfig) { + + // TODO Use DefaultTlsContext to configure SslContextFactory (ensure that cipher/protocol configuration is same across all TLS endpoints) + SslContextFactory factory = new SslContextFactory(); factory.setWantClientAuth(true); @@ -124,6 +127,7 @@ public class ConfigserverSslContextFactoryProvider extends AbstractComponent imp .orElseGet(() -> updateKeystore(configserverIdentity, generateKeystorePassword(), keyProvider, ztsClient, zoneConfig)); factory.setKeyStore(keyStore); factory.setKeyStorePassword(""); + factory.setExcludeProtocols("TLSv1.3"); // TLSv1.3 is broken is multiple OpenJDK 11 versions factory.setEndpointIdentificationAlgorithm(null); // disable https hostname verification of clients (must be disabled when using Athenz x509 certificates) return factory; } diff --git a/component/src/main/java/com/yahoo/component/Version.java b/component/src/main/java/com/yahoo/component/Version.java index 8ce4fe050f7..e0ed3f03f7e 100644 --- a/component/src/main/java/com/yahoo/component/Version.java +++ b/component/src/main/java/com/yahoo/component/Version.java @@ -119,7 +119,7 @@ public final class Version implements Comparable<Version> { */ public Version(String versionString) { if (! "".equals(versionString)) { - String[] components=versionString.split("\\x2e"); // Split on dot + String[] components=versionString.split("\\."); // Split on dot if (components.length > 0) major = Integer.parseInt(components[0]); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/Search.java b/config-model/src/main/java/com/yahoo/searchdefinition/Search.java index a99300cfd32..d5a8f6419fb 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/Search.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/Search.java @@ -5,7 +5,6 @@ import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.document.Field; import com.yahoo.searchdefinition.derived.SummaryClass; import com.yahoo.searchdefinition.document.Attribute; -import com.yahoo.searchdefinition.document.ImmutableImportedSDField; import com.yahoo.searchdefinition.document.ImmutableSDField; import com.yahoo.searchdefinition.document.ImportedFields; import com.yahoo.searchdefinition.document.SDDocumentType; @@ -22,7 +21,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; @@ -33,9 +31,9 @@ import java.util.logging.Logger; import java.util.stream.Stream; /** - * <p>A search definition describes (or uses) some document types, defines how these are turned into a relevancy tuned - * index through indexing and how data from documents should be served at search time.</p> <p>The identity of this - * class is its name.</p> + * A search definition describes (or uses) some document types, defines how these are turned into a relevancy tuned + * index through indexing and how data from documents should be served at search time. The identity of this + * class is its name. * * @author bratseth */ diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/SummaryClass.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/SummaryClass.java index 56cfb2a595c..f88da34428d 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/SummaryClass.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/SummaryClass.java @@ -27,12 +27,12 @@ public class SummaryClass extends Derived { public static final String DOCUMENT_ID_FIELD = "documentid"; /** True if this summary class needs to access summary information on disk */ - private boolean accessingDiskSummary=false; + private boolean accessingDiskSummary = false; /** The summary fields of this indexed by name */ private Map<String,SummaryClassField> fields = new java.util.LinkedHashMap<>(); - private DeployLogger deployLogger = new BaseDeployLogger(); + private DeployLogger deployLogger; private final Random random = new Random(7); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/NetworkPortRequestor.java b/config-model/src/main/java/com/yahoo/vespa/model/NetworkPortRequestor.java index 4d64ece1c77..96870ee30d8 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/NetworkPortRequestor.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/NetworkPortRequestor.java @@ -26,7 +26,11 @@ public interface NetworkPortRequestor { */ default int getWantedPort() { return 0; } - /** Returns the number of ports needed by this service. */ + /** + * Returns the number of ports needed by this service. + * User-defined ports for container http servers should not be counted, as those + * ports are required to be outside Vespa's port range. + */ int getPortCount(); /** diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java index 3559d401346..60f13ab6012 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java @@ -60,7 +60,7 @@ public class MetricsProxyContainer extends Container implements } int metricsRpcPortOffset() { - return numHttpServerPorts; + return numHttpServerPorts + numMessageBusPorts() + numRpcPorts(); } @Override @@ -92,7 +92,7 @@ public class MetricsProxyContainer extends Container implements @Override protected void tagServers() { super.tagServers(); - portsMeta.on(numHttpServerPorts).tag("rpc").tag("metrics"); + portsMeta.on(metricsRpcPortOffset()).tag("rpc").tag("metrics"); } @Override diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java index 1d195b3bc0a..558e0cdb3c2 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java @@ -4,17 +4,17 @@ package com.yahoo.vespa.model.admin.metricsproxy; -import ai.vespa.metricsproxy.core.MetricsConsumers; import ai.vespa.metricsproxy.core.ConsumersConfig; +import ai.vespa.metricsproxy.core.MetricsConsumers; import ai.vespa.metricsproxy.core.MetricsManager; -import ai.vespa.metricsproxy.metric.ExternalMetrics; +import ai.vespa.metricsproxy.core.MonitoringConfig; import ai.vespa.metricsproxy.core.VespaMetrics; +import ai.vespa.metricsproxy.metric.ExternalMetrics; import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensions; import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensionsConfig; import ai.vespa.metricsproxy.rpc.RpcServer; -import ai.vespa.metricsproxy.core.MonitoringConfig; -import ai.vespa.metricsproxy.service.SystemPollerProvider; import ai.vespa.metricsproxy.service.ConfigSentinelClient; +import ai.vespa.metricsproxy.service.SystemPollerProvider; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.config.model.producer.AbstractConfigProducerRoot; @@ -25,7 +25,6 @@ import com.yahoo.vespa.model.admin.Admin; import com.yahoo.vespa.model.admin.monitoring.MetricSet; import com.yahoo.vespa.model.admin.monitoring.MetricsConsumer; import com.yahoo.vespa.model.admin.monitoring.Monitoring; -import com.yahoo.vespa.model.admin.monitoring.builder.Metrics; import com.yahoo.vespa.model.container.ContainerCluster; import java.nio.file.Path; @@ -83,7 +82,8 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC this.parent = parent; applicationId = deployState.getProperties().applicationId(); - setRpcServerEnabled(false); + setMessageBusEnabled(false); + setRpcServerEnabled(true); addDefaultHandlersExceptStatus(); addPlatformBundle(METRICS_PROXY_BUNDLE_FILE); @@ -129,7 +129,7 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC .orElse(emptyMetricSet()); } - // Returns the metricConsumers from services.xml + // Returns the metrics consumers from services.xml private Map<String, MetricsConsumer> getUserMetricsConsumers() { return getAdmin() .map(admin -> admin.getUserMetrics().getConsumers()) diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java b/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java index f2bd67cdd81..52abe8e8fde 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java @@ -75,7 +75,6 @@ public abstract class Container extends AbstractService implements private final JettyHttpServer defaultHttpServer = new JettyHttpServer(new ComponentId("DefaultHttpServer")); protected final int numHttpServerPorts; - private static final int numRpcServerPorts = 2; protected Container(AbstractConfigProducer parent, String name, int index) { this(parent, name, false, index); @@ -89,6 +88,7 @@ public abstract class Container extends AbstractService implements this.index = index; if (getHttp() == null) { + // TODO Vespa 8: set to 1. The second (health) port has not been used since Vespa 6 or earlier. numHttpServerPorts = 2; addChild(defaultHttpServer); } else if (getHttp().getHttpServer() == null) { @@ -165,16 +165,19 @@ public abstract class Container extends AbstractService implements } protected void tagServers() { + int offset = 0; if (numHttpServerPorts > 0) { - portsMeta.on(0).tag("http").tag("query").tag("external").tag("state"); + portsMeta.on(offset++).tag("http").tag("query").tag("external").tag("state"); } for (int i = 1; i < numHttpServerPorts; i++) - portsMeta.on(i).tag("http").tag("external"); + portsMeta.on(offset++).tag("http").tag("external"); + if (messageBusEnabled()) { + portsMeta.on(offset++).tag("rpc").tag("messaging"); + } if (rpcServerEnabled()) { - portsMeta.on(numHttpServerPorts + 0).tag("rpc").tag("messaging"); - portsMeta.on(numHttpServerPorts + 1).tag("rpc").tag("admin"); + portsMeta.on(offset++).tag("rpc").tag("admin"); } } @@ -235,12 +238,12 @@ public abstract class Container extends AbstractService implements } /** - * @return the number of ports needed by the Container - those reserved manually(reservePortPrepended) + * @return the number of ports needed by the Container except those reserved manually(reservePortPrepended) */ public int getPortCount() { - int httpPorts = (getHttp() != null) ? 0 : numHttpServerPorts + 2; // TODO remove +2, only here to keep irrelevant unit tests from failing. - int rpcPorts = (rpcServerEnabled()) ? numRpcServerPorts : 0; - return httpPorts + rpcPorts; + // TODO Vespa 8: remove +2, only here for historical reasons + int httpPorts = (getHttp() != null) ? 0 : numHttpServerPorts + 2; + return httpPorts + numMessageBusPorts() + numRpcPorts(); } @Override @@ -256,12 +259,11 @@ public abstract class Container extends AbstractService implements for (int i = 1; i < httpPorts; i++) { suffixes[off++] = "http/" + i; } - int rpcPorts = (rpcServerEnabled()) ? numRpcServerPorts : 0; - if (rpcPorts > 0) { + if (messageBusEnabled()) { suffixes[off++] = "messaging"; } - if (rpcPorts > 1) { - suffixes[off++] = "rpc"; + if (rpcServerEnabled()) { + suffixes[off++] = "rpc/admin"; } while (off < n) { suffixes[off] = "unused/" + off; @@ -283,13 +285,18 @@ public abstract class Container extends AbstractService implements } private int getRpcPort() { - return rpcServerEnabled() ? getRelativePort(numHttpServerPorts + 1) : 0; + return rpcServerEnabled() ? getRelativePort(numHttpServerPorts + numMessageBusPorts()) : 0; } + protected int numRpcPorts() { return rpcServerEnabled() ? 1 : 0; } + + private int getMessagingPort() { - return getRelativePort(numHttpServerPorts); + return messageBusEnabled() ? getRelativePort(numHttpServerPorts) : 0; } + protected int numMessageBusPorts() { return messageBusEnabled() ? 1 : 0; } + @Override public int getHealthPort() { final Http http = getHttp(); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java index e002e722a5c..04533836739 100755 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java @@ -126,8 +126,6 @@ public abstract class ContainerCluster<CONTAINER extends Container> public static final String ROOT_HANDLER_BINDING = "*://*/"; - private static final boolean messageBusEnabled = true; - private final String name; protected List<CONTAINER> containers = new ArrayList<>(); @@ -139,7 +137,10 @@ public abstract class ContainerCluster<CONTAINER extends Container> private ContainerDocumentApi containerDocumentApi; private SecretStore secretStore; + // TODO: move all message-bus related fields/methods to ApplicationContainerCluster. No need for mbus for other clusters. private MbusParams mbusParams; + private boolean messageBusEnabled = true; + private boolean rpcServerEnabled = true; private boolean httpServerEnabled = true; @@ -663,6 +664,8 @@ public abstract class ContainerCluster<CONTAINER extends Container> */ public Optional<Integer> getMemoryPercentage() { return Optional.ofNullable(memoryPercentage); } + public final void setMessageBusEnabled(boolean messageBusEnabled) { this.messageBusEnabled = messageBusEnabled; } + boolean messageBusEnabled() { return messageBusEnabled; } public final void setRpcServerEnabled(boolean rpcServerEnabled) { this.rpcServerEnabled = rpcServerEnabled; } 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 83cf8c880d4..394ff2a1c5d 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 @@ -137,6 +137,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { ApplicationContainerCluster cluster = createContainerCluster(spec, modelContext); addClusterContent(cluster, spec, modelContext); addBundlesForPlatformComponents(cluster); + cluster.setMessageBusEnabled(rpcServerEnabled); cluster.setRpcServerEnabled(rpcServerEnabled); cluster.setHttpServerEnabled(httpServerEnabled); model.setCluster(cluster); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java index ff486c6a437..a1b77332bd2 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java @@ -60,19 +60,32 @@ public class MetricsProxyContainerTest { } @Test - public void rpc_server_is_running_on_expected_port() { + public void metrics_rpc_server_is_running_on_expected_port() { VespaModel model = getModel(servicesWithContent()); - MetricsProxyContainer container = (MetricsProxyContainer)model.id2producer().get(CONTAINER_CONFIG_ID); - int rpcPort = container.metricsRpcPortOffset(); - assertTrue(container.getPortsMeta().getTagsAt(rpcPort).contains("rpc")); - assertTrue(container.getPortsMeta().getTagsAt(rpcPort).contains("metrics")); + int offset = container.metricsRpcPortOffset(); + assertEquals(2, container.getPortsMeta().getTagsAt(offset).size()); + assertTrue(container.getPortsMeta().getTagsAt(offset).contains("rpc")); + assertTrue(container.getPortsMeta().getTagsAt(offset).contains("metrics")); - assertEquals("rpc/metrics", container.getPortSuffixes()[rpcPort]); + assertEquals("rpc/metrics", container.getPortSuffixes()[offset]); RpcConnectorConfig config = getRpcConnectorConfig(model); - assertEquals(19094, config.port()); + assertEquals(19095, config.port()); + } + + @Test + public void admin_rpc_server_is_running() { + VespaModel model = getModel(servicesWithContent()); + MetricsProxyContainer container = (MetricsProxyContainer)model.id2producer().get(CONTAINER_CONFIG_ID); + + int offset = container.metricsRpcPortOffset() - 1; + assertEquals(2, container.getPortsMeta().getTagsAt(offset).size()); + assertTrue(container.getPortsMeta().getTagsAt(offset).contains("rpc")); + assertTrue(container.getPortsMeta().getTagsAt(offset).contains("admin")); + + assertEquals("rpc/admin", container.getPortSuffixes()[offset]); } @Test diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index 9d04f14b4aa..3aea22ce8b2 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -103,7 +103,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye private final ConfigserverConfig configserverConfig; private final FileDistributionStatus fileDistributionStatus; private final Orchestrator orchestrator; - private final LogRetriever logRetriever = new LogRetriever(); + private final LogRetriever logRetriever; @Inject public ApplicationRepository(TenantRepository tenantRepository, @@ -113,9 +113,16 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye HttpProxy httpProxy, ConfigserverConfig configserverConfig, Orchestrator orchestrator) { - this(tenantRepository, hostProvisionerProvider.getHostProvisioner(), infraDeployerProvider.getInfraDeployer(), - configConvergenceChecker, httpProxy, configserverConfig, orchestrator, - Clock.systemUTC(), new FileDistributionStatus()); + this(tenantRepository, + hostProvisionerProvider.getHostProvisioner(), + infraDeployerProvider.getInfraDeployer(), + configConvergenceChecker, + httpProxy, + configserverConfig, + orchestrator, + new LogRetriever(), + new FileDistributionStatus(), + Clock.systemUTC()); } // For testing @@ -123,17 +130,45 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye Provisioner hostProvisioner, Orchestrator orchestrator, Clock clock) { - this(tenantRepository, hostProvisioner, orchestrator, clock, new ConfigserverConfig(new ConfigserverConfig.Builder())); + this(tenantRepository, + hostProvisioner, + orchestrator, + new ConfigserverConfig(new ConfigserverConfig.Builder()), + new LogRetriever(), + clock); } // For testing public ApplicationRepository(TenantRepository tenantRepository, Provisioner hostProvisioner, Orchestrator orchestrator, - Clock clock, - ConfigserverConfig configserverConfig) { - this(tenantRepository, Optional.of(hostProvisioner), Optional.empty(), new ConfigConvergenceChecker(), new HttpProxy(new SimpleHttpFetcher()), - configserverConfig, orchestrator, clock, new FileDistributionStatus()); + LogRetriever logRetriever, + Clock clock) { + this(tenantRepository, + hostProvisioner, + orchestrator, + new ConfigserverConfig(new ConfigserverConfig.Builder()), + logRetriever, + clock); + } + + // For testing + public ApplicationRepository(TenantRepository tenantRepository, + Provisioner hostProvisioner, + Orchestrator orchestrator, + ConfigserverConfig configserverConfig, + LogRetriever logRetriever, + Clock clock) { + this(tenantRepository, + Optional.of(hostProvisioner), + Optional.empty(), + new ConfigConvergenceChecker(), + new HttpProxy(new SimpleHttpFetcher()), + configserverConfig, + orchestrator, + logRetriever, + new FileDistributionStatus(), + clock); } private ApplicationRepository(TenantRepository tenantRepository, @@ -143,17 +178,19 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye HttpProxy httpProxy, ConfigserverConfig configserverConfig, Orchestrator orchestrator, - Clock clock, - FileDistributionStatus fileDistributionStatus) { + LogRetriever logRetriever, + FileDistributionStatus fileDistributionStatus, + Clock clock) { this.tenantRepository = tenantRepository; this.hostProvisioner = hostProvisioner; this.infraDeployer = infraDeployer; this.convergeChecker = configConvergenceChecker; this.httpProxy = httpProxy; - this.clock = clock; this.configserverConfig = configserverConfig; this.orchestrator = orchestrator; + this.logRetriever = logRetriever; this.fileDistributionStatus = fileDistributionStatus; + this.clock = clock; } // ---------------- Deploying ---------------------------------------------------------------- diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/RequestHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/RequestHandler.java index 185958510b6..c3d9fb15212 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/RequestHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/RequestHandler.java @@ -1,15 +1,16 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server; -import java.util.Optional; -import java.util.Set; - import com.yahoo.component.Version; +import com.yahoo.config.FileReference; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.config.ConfigKey; import com.yahoo.vespa.config.GetConfigRequest; -import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.config.protocol.ConfigResponse; +import java.util.Optional; +import java.util.Set; + /** * Instances of this can serve misc config related requests * @@ -83,4 +84,12 @@ public interface RequestHandler { * @return an {@link ApplicationId} instance. */ ApplicationId resolveApplicationId(String hostName); + + /** + * Returns the set of file references from the application's Vespa models, aggregated across all application versions. + * + * @param applicationId application id to use + * @return set of file references that is owned by the application + */ + Set<FileReference> listFileReferences(ApplicationId applicationId); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelRequestHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelRequestHandler.java index d77ca396974..520972f2fcf 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelRequestHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelRequestHandler.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.config.server; import com.google.inject.Inject; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.config.ConfigInstance; +import com.yahoo.config.FileReference; import com.yahoo.config.model.api.ConfigDefinitionRepo; import com.yahoo.component.Version; import com.yahoo.log.LogLevel; @@ -117,6 +118,11 @@ public class SuperModelRequestHandler implements RequestHandler { return ApplicationId.global(); } + @Override + public Set<FileReference> listFileReferences(ApplicationId applicationId) { + throw new UnsupportedOperationException(); + } + public void enable() { enabled = true; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ApplicationMapper.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ApplicationMapper.java index 4fe2a018766..5ce9ebca69d 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ApplicationMapper.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ApplicationMapper.java @@ -1,18 +1,18 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.application; -import com.yahoo.config.provision.ApplicationId; import com.yahoo.component.Version; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.vespa.config.server.NotFoundException; import java.time.Instant; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import com.yahoo.vespa.config.server.NotFoundException; - /** * Used during config request handling to route to the right config model * based on application id and version. @@ -81,4 +81,8 @@ public final class ApplicationMapper { return Collections.unmodifiableSet(requestHandlers.keySet()); } + public List<Application> listApplications(ApplicationId applicationId) { + return requestHandlers.get(applicationId).getAllApplications(); + } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ApplicationSet.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ApplicationSet.java index 3444ffc2865..41119077b28 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ApplicationSet.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ApplicationSet.java @@ -92,4 +92,8 @@ public final class ApplicationSet { return generation; } + List<Application> getAllApplications() { + return new ArrayList<>(applications.values()); + } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/LogRetriever.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/LogRetriever.java index 3f7c870210f..791d596d398 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/LogRetriever.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/LogRetriever.java @@ -12,6 +12,9 @@ import java.io.OutputStream; import java.io.UncheckedIOException; import java.util.Optional; +/** + * @author olaaun + */ public class LogRetriever { private final HttpClient httpClient = HttpClientBuilder.create().build(); @@ -26,6 +29,7 @@ public class LogRetriever { } private static class ProxyResponse extends HttpResponse { + private final org.apache.http.HttpResponse clientResponse; private ProxyResponse(org.apache.http.HttpResponse clientResponse) { @@ -45,4 +49,5 @@ public class LogRetriever { clientResponse.getEntity().writeTo(outputStream); } } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/AuthorizationException.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/AuthorizationException.java new file mode 100644 index 00000000000..20435d96068 --- /dev/null +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/AuthorizationException.java @@ -0,0 +1,17 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.config.server.rpc.security; + +/** + * @author bjorncs + */ +class AuthorizationException extends RuntimeException { + + AuthorizationException(String message) { + super(message); + } + + AuthorizationException(String message, Throwable cause) { + super(message, cause); + } +} + diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/security/DummyNodeIdentifierProvider.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/DummyNodeIdentifierProvider.java index 0ff0bcc26bb..8666a47f09f 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/security/DummyNodeIdentifierProvider.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/DummyNodeIdentifierProvider.java @@ -1,8 +1,10 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.config.provision.security; +package com.yahoo.vespa.config.server.rpc.security; import com.google.inject.Inject; -import com.google.inject.Provider; +import com.yahoo.config.provision.security.NodeIdentifier; +import com.yahoo.config.provision.security.NodeIdentity; +import com.yahoo.container.di.componentgraph.Provider; import java.security.cert.X509Certificate; import java.util.List; @@ -22,6 +24,9 @@ public class DummyNodeIdentifierProvider implements Provider<NodeIdentifier> { return instance; } + @Override + public void deconstruct() {} + private static class ThrowingNodeIdentifier implements NodeIdentifier { @Override public NodeIdentity identifyNode(List<X509Certificate> peerCertificateChain) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/GlobalConfigAuthorizationPolicy.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/GlobalConfigAuthorizationPolicy.java new file mode 100644 index 00000000000..cc1125b6cc6 --- /dev/null +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/GlobalConfigAuthorizationPolicy.java @@ -0,0 +1,52 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.config.server.rpc.security; + +import com.yahoo.cloud.config.LbServicesConfig; +import com.yahoo.cloud.config.RoutingConfig; +import com.yahoo.config.ConfigInstance; +import com.yahoo.config.provision.NodeType; +import com.yahoo.vespa.config.ConfigKey; + +import java.util.Arrays; +import java.util.EnumSet; +import java.util.List; + +/** + * Specifies which node type that are allowed to access global configuration + * + * @author bjorncs + */ +enum GlobalConfigAuthorizationPolicy { + + LB_SERVICES(new LbServicesConfig.Builder(), NodeType.proxy), + ROUTING(new RoutingConfig.Builder(), NodeType.tenant); // TODO Remove handling of RoutingConfig when YCA filter is removed + + final String namespace; + final String name; + final EnumSet<NodeType> allowedToAccess; + + GlobalConfigAuthorizationPolicy(ConfigInstance.Builder builder, NodeType... allowedToAccess) { + this.namespace = builder.getDefNamespace(); + this.name = builder.getDefName(); + this.allowedToAccess = EnumSet.copyOf(List.of(allowedToAccess)); + } + + static void verifyAccessAllowed(ConfigKey<?> configKey, NodeType nodeType) { + GlobalConfigAuthorizationPolicy policy = findPolicyFromConfigKey(configKey); + if (!policy.allowedToAccess.contains(nodeType)) { + String message = String.format( + "Node with type '%s' is not allowed to access global config [%s]", + nodeType, configKey); + throw new AuthorizationException(message); + } + } + + private static GlobalConfigAuthorizationPolicy findPolicyFromConfigKey(ConfigKey<?> configKey) { + return Arrays.stream(values()) + .filter(policy -> policy.namespace.equals(configKey.getNamespace()) && policy.name.equals(configKey.getName())) + .findAny() + .orElseThrow(() -> new AuthorizationException(String.format("No policy defined for global config [%s]", configKey))); + } + +} + diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java new file mode 100644 index 00000000000..64987b8efc9 --- /dev/null +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java @@ -0,0 +1,219 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.config.server.rpc.security; + +import com.google.inject.Inject; +import com.yahoo.concurrent.DaemonThreadFactory; +import com.yahoo.config.FileReference; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.TenantName; +import com.yahoo.config.provision.security.NodeIdentifier; +import com.yahoo.config.provision.security.NodeIdentifierException; +import com.yahoo.config.provision.security.NodeIdentity; +import com.yahoo.jrt.Request; +import com.yahoo.jrt.SecurityContext; +import com.yahoo.log.LogLevel; +import com.yahoo.security.tls.MixedMode; +import com.yahoo.security.tls.TransportSecurityUtils; +import com.yahoo.vespa.config.ConfigKey; +import com.yahoo.vespa.config.protocol.JRTServerConfigRequestV3; +import com.yahoo.vespa.config.server.RequestHandler; +import com.yahoo.vespa.config.server.host.HostRegistries; +import com.yahoo.vespa.config.server.host.HostRegistry; +import com.yahoo.vespa.config.server.tenant.Tenant; +import com.yahoo.vespa.config.server.tenant.TenantRepository; + +import java.security.cert.X509Certificate; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; +import java.util.function.BiConsumer; +import java.util.logging.Logger; + + +/** + * A {@link RpcAuthorizer} that perform access control for configserver RPC methods when TLS and multi-tenant mode are enabled. + * + * @author bjorncs + */ +public class MultiTenantRpcAuthorizer implements RpcAuthorizer { + + public enum Mode { LOG_ONLY, ENFORCE } + + private static final Logger log = Logger.getLogger(MultiTenantRpcAuthorizer.class.getName()); + + private final NodeIdentifier nodeIdentifier; + private final HostRegistry<TenantName> hostRegistry; + private final TenantRepository tenantRepository; + private final Executor executor; + private final Mode mode; + + @Inject + public MultiTenantRpcAuthorizer(NodeIdentifier nodeIdentifier, + HostRegistries hostRegistries, + TenantRepository tenantRepository) { + this(nodeIdentifier, + hostRegistries.getTenantHostRegistry(), + tenantRepository, + Executors.newFixedThreadPool(4, new DaemonThreadFactory("RPC-Authorizer-")), + Mode.LOG_ONLY); // TODO Change default mode + } + + MultiTenantRpcAuthorizer(NodeIdentifier nodeIdentifier, + HostRegistry<TenantName> hostRegistry, + TenantRepository tenantRepository, + Executor executor, + Mode mode) { + this.nodeIdentifier = nodeIdentifier; + this.hostRegistry = hostRegistry; + this.tenantRepository = tenantRepository; + this.executor = executor; + this.mode = mode; + } + + @Override + public CompletableFuture<Void> authorizeConfigRequest(Request request) { + return doAsyncAuthorization(request, this::doConfigRequestAuthorization); + } + + @Override + public CompletableFuture<Void> authorizeFileRequest(Request request) { + return doAsyncAuthorization(request, this::doFileRequestAuthorization); + } + + private CompletableFuture<Void> doAsyncAuthorization(Request request, BiConsumer<Request, NodeIdentity> authorizer) { + return CompletableFuture.runAsync( + () -> getPeerIdentity(request) + .ifPresent(peerIdentity -> { + try { + authorizer.accept(request, peerIdentity); + } catch (Throwable t) { + handleAuthorizationFailure(request, t); + } + }), + executor); + } + + private void doConfigRequestAuthorization(Request request, NodeIdentity peerIdentity) { + switch (peerIdentity.nodeType()) { + case config: + return; // configserver is allowed to access all config + case proxy: + case tenant: + case host: + JRTServerConfigRequestV3 configRequest = JRTServerConfigRequestV3.createFromRequest(request); + ConfigKey<?> configKey = configRequest.getConfigKey(); + if (isConfigKeyForGlobalConfig(configKey)) { + GlobalConfigAuthorizationPolicy.verifyAccessAllowed(configKey, peerIdentity.nodeType()); + return; // global config access ok + } else { + String hostname = configRequest.getClientHostName(); + Optional<RequestHandler> tenantHandler = + Optional.of(hostRegistry.getKeyForHost(hostname)) + .flatMap(this::getTenantHandler); + if (tenantHandler.isEmpty()) { + return; // unknown tenant + } + ApplicationId resolvedApplication = tenantHandler.get().resolveApplicationId(hostname); + ApplicationId peerOwner = applicationId(peerIdentity); + if (peerOwner.equals(resolvedApplication)) { + return; // allowed to access + } + throw new AuthorizationException( + String.format( + "Peer is not allowed to access config for owned by %s. Peer is owned by %s", + resolvedApplication.toShortString(), peerOwner.toShortString())); + } + default: + throw new AuthorizationException(String.format("'%s' nodes are not allowed to access config", peerIdentity.nodeType())); + } + } + + private void doFileRequestAuthorization(Request request, NodeIdentity peerIdentity) { + switch (peerIdentity.nodeType()) { + case config: + return; // configserver is allowed to access all files + case proxy: + case tenant: + case host: + ApplicationId peerOwner = applicationId(peerIdentity); + FileReference requestedFile = new FileReference(request.parameters().get(0).asString()); + RequestHandler tenantHandler = getTenantHandler(peerOwner.tenant()) + .orElseThrow(() -> new AuthorizationException( + String.format( + "Application '%s' does not exist - unable to verify file ownership for '%s'", + peerOwner.toShortString(), requestedFile.value()))); + Set<FileReference> filesOwnedByApplication = tenantHandler.listFileReferences(peerOwner); + if (filesOwnedByApplication.contains(requestedFile)) { + return; // allowed to access + } + throw new AuthorizationException("Peer is not allowed to access file " + requestedFile.value()); + default: + throw new AuthorizationException(String.format("'%s' nodes are not allowed to access files", peerIdentity.nodeType())); + } + } + + private void handleAuthorizationFailure(Request request, Throwable throwable) { + String errorMessage = String.format("For request from '%s': %s", request.target().toString(), throwable.getMessage()); + log.log(LogLevel.WARNING, errorMessage, throwable); + if (mode == Mode.ENFORCE) { + JrtErrorCode error = throwable instanceof AuthorizationException ? JrtErrorCode.UNAUTHORIZED : JrtErrorCode.AUTHORIZATION_FAILED; + request.setError(error.code, errorMessage); + request.returnRequest(); + throwUnchecked(throwable); // rethrow exception to ensure that subsequent completion stages are not executed (don't execute implementation of rpc method). + } + } + + // TODO Make peer identity mandatory once TLS mixed mode is removed + private Optional<NodeIdentity> getPeerIdentity(Request request) { + Optional<SecurityContext> securityContext = request.target().getSecurityContext(); + if (securityContext.isEmpty()) { + if (TransportSecurityUtils.getInsecureMixedMode() == MixedMode.DISABLED) { + throw new IllegalStateException("Security context missing"); // security context should always be present + } + return Optional.empty(); // client choose to communicate over insecure channel + } + List<X509Certificate> certChain = securityContext.get().peerCertificateChain(); + if (certChain.isEmpty()) { + throw new IllegalStateException("Client authentication is not enforced!"); // clients should be required to authenticate when TLS is enabled + } + try { + return Optional.of(nodeIdentifier.identifyNode(certChain)); + } catch (NodeIdentifierException e) { + throw new AuthorizationException("Failed to identity peer: " + e.getMessage(), e); + } + } + + private static boolean isConfigKeyForGlobalConfig(ConfigKey<?> configKey) { + return "*".equals(configKey.getConfigId()); + } + + private static ApplicationId applicationId(NodeIdentity peerIdentity) { + return peerIdentity.applicationId() + .orElseThrow(() -> new AuthorizationException("Peer node is not associated with an application")); + } + + private Optional<RequestHandler> getTenantHandler(TenantName tenantName) { + return Optional.ofNullable(tenantRepository.getTenant(tenantName)) + .map(Tenant::getRequestHandler); + } + + @SuppressWarnings("unchecked") + private static <T extends Throwable> void throwUnchecked(Throwable t) throws T { + throw (T)t; + } + + private enum JrtErrorCode { + UNAUTHORIZED(1), + AUTHORIZATION_FAILED(2); + + final int code; + + JrtErrorCode(int errorOffset) { + this.code = 0x20000 + errorOffset; + } + } + +} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/RpcAuthorizer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/RpcAuthorizer.java new file mode 100644 index 00000000000..ccda24530e4 --- /dev/null +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/RpcAuthorizer.java @@ -0,0 +1,19 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.config.server.rpc.security; + +import com.yahoo.jrt.Request; + +import java.util.concurrent.CompletableFuture; + +/** + * Authorization logic for configserver's RPC method + * + * @author bjorncs + */ +public interface RpcAuthorizer { + + CompletableFuture<Void> authorizeConfigRequest(Request request); + + CompletableFuture<Void> authorizeFileRequest(Request request); + +} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandler.java index 1430475e486..65b92558216 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandler.java @@ -1,39 +1,41 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.tenant; -import java.time.Clock; -import java.util.Collection; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Optional; -import java.util.OptionalLong; -import java.util.Set; - import com.yahoo.component.Version; +import com.yahoo.config.FileReference; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.TenantName; import com.yahoo.log.LogLevel; import com.yahoo.vespa.config.ConfigKey; import com.yahoo.vespa.config.GetConfigRequest; import com.yahoo.vespa.config.protocol.ConfigResponse; import com.yahoo.vespa.config.server.NotFoundException; +import com.yahoo.vespa.config.server.ReloadHandler; +import com.yahoo.vespa.config.server.ReloadListener; +import com.yahoo.vespa.config.server.RequestHandler; +import com.yahoo.vespa.config.server.application.Application; import com.yahoo.vespa.config.server.application.ApplicationMapper; import com.yahoo.vespa.config.server.application.ApplicationSet; import com.yahoo.vespa.config.server.application.TenantApplications; -import com.yahoo.vespa.config.server.rpc.ConfigResponseFactory; +import com.yahoo.vespa.config.server.application.VersionDoesNotExistException; import com.yahoo.vespa.config.server.host.HostRegistries; import com.yahoo.vespa.config.server.host.HostRegistry; import com.yahoo.vespa.config.server.host.HostValidator; -import com.yahoo.vespa.config.server.ReloadHandler; -import com.yahoo.vespa.config.server.ReloadListener; -import com.yahoo.vespa.config.server.RequestHandler; -import com.yahoo.vespa.config.server.application.VersionDoesNotExistException; -import com.yahoo.vespa.config.server.application.Application; -import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.TenantName; import com.yahoo.vespa.config.server.monitoring.MetricUpdater; import com.yahoo.vespa.config.server.monitoring.Metrics; +import com.yahoo.vespa.config.server.rpc.ConfigResponseFactory; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.Lock; +import java.time.Clock; +import java.util.Collection; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; + +import static java.util.stream.Collectors.toSet; + /** * A per tenant request handler, for handling reload (activate application) and getConfig requests for * a set of applications belonging to a tenant. @@ -248,7 +250,14 @@ public class TenantRequestHandler implements RequestHandler, ReloadHandler, Host } return applicationId; } - + + @Override + public Set<FileReference> listFileReferences(ApplicationId applicationId) { + return applicationMapper.listApplications(applicationId).stream() + .flatMap(app -> app.getModel().fileReferences().stream()) + .collect(toSet()); + } + @Override public void verifyHosts(ApplicationId key, Collection<String> newHosts) { hostRegistry.verifyHosts(key, newHosts); diff --git a/configserver/src/main/resources/configserver-app/services.xml b/configserver/src/main/resources/configserver-app/services.xml index a7fd0f696b3..cf79ae81312 100644 --- a/configserver/src/main/resources/configserver-app/services.xml +++ b/configserver/src/main/resources/configserver-app/services.xml @@ -38,7 +38,7 @@ <component id="com.yahoo.vespa.config.server.application.HttpProxy" bundle="configserver" /> <component id="com.yahoo.vespa.config.server.filedistribution.FileServer" bundle="configserver" /> <component id="com.yahoo.vespa.config.server.maintenance.ConfigServerMaintenance" bundle="configserver" /> - <component id="com.yahoo.config.provision.security.DummyNodeIdentifierProvider" bundle="config-provisioning" /> + <component id="com.yahoo.vespa.config.server.rpc.security.DummyNodeIdentifierProvider" bundle="configserver" /> <component id="com.yahoo.vespa.serviceview.ConfigServerLocation" bundle="configserver" /> diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java index e0fa760b35d..3eff6c935a7 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java @@ -1,7 +1,6 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server; -import com.github.tomakehurst.wiremock.junit.WireMockRule; import com.google.common.io.Files; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.component.Version; @@ -21,6 +20,7 @@ import com.yahoo.test.ManualClock; import com.yahoo.text.Utf8; import com.yahoo.vespa.config.server.application.OrchestratorMock; import com.yahoo.vespa.config.server.deploy.DeployTester; +import com.yahoo.vespa.config.server.http.LogRetriever; import com.yahoo.vespa.config.server.http.SessionHandlerTest; import com.yahoo.vespa.config.server.http.v2.PrepareResult; import com.yahoo.vespa.config.server.session.LocalSession; @@ -37,6 +37,8 @@ import org.junit.rules.TemporaryFolder; import java.io.File; import java.io.IOException; +import java.io.OutputStream; +import java.nio.charset.StandardCharsets; import java.time.Clock; import java.time.Duration; import java.time.Instant; @@ -46,17 +48,11 @@ import java.util.Collections; import java.util.Optional; import java.util.Set; -import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; -import static com.github.tomakehurst.wiremock.client.WireMock.get; -import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; -import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options; -import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; /** @@ -83,9 +79,6 @@ public class ApplicationRepositoryTest { @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); - @Rule - public final WireMockRule wireMock = new WireMockRule(options().port(8080), true); - @Before public void setup() { Curator curator = new MockCurator(); @@ -97,7 +90,10 @@ public class ApplicationRepositoryTest { tenantRepository.addTenant(tenant3); orchestrator = new OrchestratorMock(); provisioner = new SessionHandlerTest.MockProvisioner(); - applicationRepository = new ApplicationRepository(tenantRepository, provisioner, orchestrator, clock); + applicationRepository = new ApplicationRepository(tenantRepository, + provisioner, + orchestrator, + clock); timeoutBudget = new TimeoutBudget(clock, Duration.ofSeconds(60)); } @@ -151,7 +147,11 @@ public class ApplicationRepositoryTest { @Test public void getLogs() { - wireMock.stubFor(get(urlEqualTo("/logs")).willReturn(aResponse().withStatus(200))); + applicationRepository = new ApplicationRepository(tenantRepository, + provisioner, + orchestrator, + new MockLogRetriever(), + clock); deployApp(testAppLogServerWithContainer); HttpResponse response = applicationRepository.getLogs(applicationId(), Optional.empty(), ""); assertEquals(200, response.getStatus()); @@ -159,7 +159,11 @@ public class ApplicationRepositoryTest { @Test public void getLogsForHostname() { - wireMock.stubFor(get(urlEqualTo("/logs")).willReturn(aResponse().withStatus(200))); + applicationRepository = new ApplicationRepository(tenantRepository, + provisioner, + orchestrator, + new MockLogRetriever(), + clock); deployApp(testAppLogServerWithContainer); HttpResponse response = applicationRepository.getLogs(applicationId(), Optional.of("localhost"), ""); assertEquals(200, response.getStatus()); @@ -167,6 +171,11 @@ public class ApplicationRepositoryTest { @Test(expected = IllegalArgumentException.class) public void refuseToGetLogsFromHostnameNotInApplication() { + applicationRepository = new ApplicationRepository(tenantRepository, + provisioner, + orchestrator, + new MockLogRetriever(), + clock); deployApp(testAppLogServerWithContainer); HttpResponse response = applicationRepository.getLogs(applicationId(), Optional.of("host123.fake.yahoo.com"), ""); assertEquals(200, response.getStatus()); @@ -273,8 +282,8 @@ public class ApplicationRepositoryTest { assertNull(tenant.getLocalSessionRepo().getSession(sessionId)); assertNull(tenant.getRemoteSessionRepo().getSession(sessionId)); assertTrue(provisioner.removed); - assertThat(provisioner.lastApplicationId.tenant(), is(tenant.getName())); - assertThat(provisioner.lastApplicationId, is(applicationId())); + assertEquals(tenant.getName(), provisioner.lastApplicationId.tenant()); + assertEquals(applicationId(), provisioner.lastApplicationId); assertFalse(applicationRepository.delete(applicationId())); } @@ -292,7 +301,7 @@ public class ApplicationRepositoryTest { // Delete app with id fooId, should not affect original app assertTrue(applicationRepository.delete(fooId)); - assertThat(provisioner.lastApplicationId, is(fooId)); + assertEquals(fooId, provisioner.lastApplicationId); assertNotNull(applicationRepository.getActiveSession(applicationId())); assertTrue(applicationRepository.delete(applicationId())); @@ -332,7 +341,7 @@ public class ApplicationRepositoryTest { // All sessions except 3 should be removed after the call to deleteExpiredLocalSessions tester.applicationRepository().deleteExpiredLocalSessions(); - final Collection<LocalSession> sessions = tester.tenant().getLocalSessionRepo().listSessions(); + Collection<LocalSession> sessions = tester.tenant().getLocalSessionRepo().listSessions(); assertEquals(1, sessions.size()); assertEquals(3, new ArrayList<>(sessions).get(0).getSessionId()); @@ -372,4 +381,28 @@ public class ApplicationRepositoryTest { Tenant tenant = tenantRepository.getTenant(applicationId.tenant()); return applicationRepository.getMetadataFromSession(tenant, sessionId); } + + private static class MockLogRetriever extends LogRetriever { + + @Override + public HttpResponse getLogs(String logServerHostname) { + return new MockHttpResponse(); + } + + private static class MockHttpResponse extends HttpResponse { + + private MockHttpResponse() { + super(200); + } + + @Override + public void render(OutputStream outputStream) throws IOException { + outputStream.write("log line".getBytes(StandardCharsets.UTF_8)); + } + + } + + + } + } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java index 6b67dcc4e9a..f619bd92bef 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java @@ -30,6 +30,7 @@ import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.config.server.TestComponentRegistry; import com.yahoo.vespa.config.server.TimeoutBudget; import com.yahoo.vespa.config.server.application.OrchestratorMock; +import com.yahoo.vespa.config.server.http.LogRetriever; import com.yahoo.vespa.config.server.http.v2.PrepareResult; import com.yahoo.vespa.config.server.modelfactory.ModelFactoryRegistry; import com.yahoo.vespa.config.server.monitoring.Metrics; @@ -126,8 +127,9 @@ public class DeployTester { applicationRepository = new ApplicationRepository(tenantRepository, new ProvisionerAdapter(provisioner), new OrchestratorMock(), - clock, - configserverConfig); + configserverConfig, + new LogRetriever(), + clock); } public Tenant tenant() { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRequestHandler.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRequestHandler.java index bfc06a58b16..0f6cd10d564 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRequestHandler.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRequestHandler.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.config.server.rpc; +import com.yahoo.config.FileReference; import com.yahoo.config.provision.ApplicationId; import com.yahoo.component.Version; import com.yahoo.vespa.config.ConfigKey; @@ -91,6 +92,11 @@ public class MockRequestHandler implements RequestHandler, ReloadHandler, Tenant } @Override + public Set<FileReference> listFileReferences(ApplicationId applicationId) { + return Set.of(); + } + + @Override public RequestHandler getRequestHandler() { return this; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizerTest.java new file mode 100644 index 00000000000..cb8219aaf8d --- /dev/null +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizerTest.java @@ -0,0 +1,269 @@ +package com.yahoo.vespa.config.server.rpc.security;// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +import com.yahoo.cloud.config.LbServicesConfig; +import com.yahoo.cloud.config.RoutingConfig; +import com.yahoo.config.FileReference; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.HostName; +import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.TenantName; +import com.yahoo.config.provision.security.NodeIdentifier; +import com.yahoo.config.provision.security.NodeIdentifierException; +import com.yahoo.config.provision.security.NodeIdentity; +import com.yahoo.jrt.Request; +import com.yahoo.jrt.SecurityContext; +import com.yahoo.jrt.StringValue; +import com.yahoo.jrt.Target; +import com.yahoo.jrt.Values; +import com.yahoo.security.KeyAlgorithm; +import com.yahoo.security.KeyUtils; +import com.yahoo.security.SignatureAlgorithm; +import com.yahoo.security.X509CertificateBuilder; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.JsonFormat; +import com.yahoo.slime.Slime; +import com.yahoo.vespa.config.ConfigKey; +import com.yahoo.vespa.config.server.RequestHandler; +import com.yahoo.vespa.config.server.host.HostRegistry; +import com.yahoo.vespa.config.server.tenant.Tenant; +import com.yahoo.vespa.config.server.tenant.TenantRepository; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import javax.security.auth.x500.X500Principal; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.math.BigInteger; +import java.security.cert.X509Certificate; +import java.time.Instant; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executor; + +import static com.yahoo.vespa.config.server.rpc.security.MultiTenantRpcAuthorizer.Mode.ENFORCE; +import static java.time.temporal.ChronoUnit.DAYS; +import static org.hamcrest.core.IsInstanceOf.instanceOf; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * @author bjorncs + */ +public class MultiTenantRpcAuthorizerTest { + + private static final List<X509Certificate> PEER_CERTIFICATE_CHAIN = List.of(createDummyCertificate()); + private static final ApplicationId APPLICATION_ID = ApplicationId.from("mytenant", "myapplication", "default"); + private static final ApplicationId EVIL_APP_ID = ApplicationId.from("malice", "malice-app", "default"); + private static final HostName HOSTNAME = HostName.from("myhostname"); + private static final FileReference FILE_REFERENCE = new FileReference("myfilereference"); + + @Rule + public ExpectedException exceptionRule = ExpectedException.none(); + + @Test + public void configserver_can_access_files_and_config() throws InterruptedException, ExecutionException { + RpcAuthorizer authorizer = createAuthorizer(new NodeIdentity.Builder(NodeType.config).build(), + new HostRegistry<>()); + + Request configRequest = createConfigRequest(new ConfigKey<>("name", "configid", "namespace"), HOSTNAME); + authorizer.authorizeConfigRequest(configRequest) + .get(); + + Request fileRequest = createFileRequest(FILE_REFERENCE); + authorizer.authorizeFileRequest(fileRequest) + .get(); + } + + @Test + public void tenant_node_can_access_its_own_files_and_config() throws ExecutionException, InterruptedException { + NodeIdentity identity = new NodeIdentity.Builder(NodeType.tenant) + .applicationId(APPLICATION_ID) + .build(); + + HostRegistry<TenantName> hostRegistry = new HostRegistry<>(); + hostRegistry.update(APPLICATION_ID.tenant(), List.of(HOSTNAME.value())); + + RpcAuthorizer authorizer = createAuthorizer(identity, hostRegistry); + + Request configRequest = createConfigRequest(new ConfigKey<>("name", "configid", "namespace"), HOSTNAME); + authorizer.authorizeConfigRequest(configRequest) + .get(); + + Request fileRequest = createFileRequest(FILE_REFERENCE); + authorizer.authorizeFileRequest(fileRequest) + .get(); + } + + @Test + public void proxy_node_can_access_lbservice_config() throws ExecutionException, InterruptedException { + RpcAuthorizer authorizer = createAuthorizer(new NodeIdentity.Builder(NodeType.proxy).build(), new HostRegistry<>()); + + Request configRequest = createConfigRequest( + new ConfigKey<>(LbServicesConfig.CONFIG_DEF_NAME, "*", LbServicesConfig.CONFIG_DEF_NAMESPACE), + HOSTNAME); + authorizer.authorizeConfigRequest(configRequest) + .get(); + } + + @Test + public void tenant_node_can_access_routing_config() throws ExecutionException, InterruptedException { + RpcAuthorizer authorizer = createAuthorizer(new NodeIdentity.Builder(NodeType.tenant).build(), new HostRegistry<>()); + + Request configRequest = createConfigRequest( + new ConfigKey<>(RoutingConfig.CONFIG_DEF_NAME, "*", RoutingConfig.CONFIG_DEF_NAMESPACE), + HOSTNAME); + authorizer.authorizeConfigRequest(configRequest) + .get(); + } + + @Test + public void tenant_node_cannot_access_lbservice_config() throws ExecutionException, InterruptedException { + RpcAuthorizer authorizer = createAuthorizer(new NodeIdentity.Builder(NodeType.tenant).build(), new HostRegistry<>()); + + Request configRequest = createConfigRequest( + new ConfigKey<>(LbServicesConfig.CONFIG_DEF_NAME, "*", LbServicesConfig.CONFIG_DEF_NAMESPACE), + HOSTNAME); + + exceptionRule.expectMessage("Node with type 'tenant' is not allowed to access global config [name=lb-services,namespace=cloud.config,configId=*]"); + exceptionRule.expectCause(instanceOf(AuthorizationException.class)); + + authorizer.authorizeConfigRequest(configRequest) + .get(); + } + + @Test + public void tenant_node_cannot_access_other_files() throws ExecutionException, InterruptedException { + NodeIdentity identity = new NodeIdentity.Builder(NodeType.tenant) + .applicationId(APPLICATION_ID) + .build(); + + HostRegistry<TenantName> hostRegistry = new HostRegistry<>(); + hostRegistry.update(APPLICATION_ID.tenant(), List.of(HOSTNAME.value())); + + RpcAuthorizer authorizer = createAuthorizer(identity, hostRegistry); + + Request fileRequest = createFileRequest(new FileReference("other-file-reference")); + + exceptionRule.expectMessage("Peer is not allowed to access file other-file-reference"); + exceptionRule.expectCause(instanceOf(AuthorizationException.class)); + + authorizer.authorizeFileRequest(fileRequest) + .get(); + } + + @Test + public void tenant_node_cannot_access_other_config() throws ExecutionException, InterruptedException { + NodeIdentity identity = new NodeIdentity.Builder(NodeType.tenant) + .applicationId(EVIL_APP_ID) + .build(); + + HostRegistry<TenantName> hostRegistry = new HostRegistry<>(); + hostRegistry.update(APPLICATION_ID.tenant(), List.of(HOSTNAME.value())); + + RpcAuthorizer authorizer = createAuthorizer(identity, hostRegistry); + + Request configRequest = createConfigRequest(new ConfigKey<>("name", "configid", "namespace"), HOSTNAME); + + exceptionRule.expectMessage("Peer is not allowed to access config for owned by mytenant.myapplication. Peer is owned by malice.malice-app"); + exceptionRule.expectCause(instanceOf(AuthorizationException.class)); + + authorizer.authorizeConfigRequest(configRequest) + .get(); + } + + private static RpcAuthorizer createAuthorizer(NodeIdentity identity, HostRegistry<TenantName> hostRegistry) { + return new MultiTenantRpcAuthorizer( + new StaticNodeIdentifier(identity), + hostRegistry, + createTenantRepositoryMock(), + new DirectExecutor(), + ENFORCE); + } + + private static Request createConfigRequest(ConfigKey<?> configKey, HostName hostName) { + return mockJrtRpcRequest(createConfigPayload(configKey, hostName.value())); + } + + private static Request createFileRequest(FileReference fileReference) { + return mockJrtRpcRequest(fileReference.value()); + } + + private static TenantRepository createTenantRepositoryMock() { + RequestHandler requestHandler = mock(RequestHandler.class); + when(requestHandler.hasApplication(APPLICATION_ID, Optional.empty())).thenReturn(true); + when(requestHandler.resolveApplicationId(HOSTNAME.value())).thenReturn(APPLICATION_ID); + when(requestHandler.listFileReferences(APPLICATION_ID)).thenReturn(Set.of(FILE_REFERENCE)); + Tenant tenant = mock(Tenant.class); + when(tenant.getRequestHandler()).thenReturn(requestHandler); + + TenantRepository tenantRepository = mock(TenantRepository.class); + when(tenantRepository.getTenant(APPLICATION_ID.tenant())).thenReturn(tenant); + return tenantRepository; + } + + private static Request mockJrtRpcRequest(String payload) { + SecurityContext securityContext = mock(SecurityContext.class); + when(securityContext.peerCertificateChain()).thenReturn(PEER_CERTIFICATE_CHAIN); + Target target = mock(Target.class); + when(target.getSecurityContext()).thenReturn(Optional.of(securityContext)); + Request request = mock(Request.class); + when(request.target()).thenReturn(target); + Values values = new Values(); + values.add(new StringValue(payload)); + when(request.parameters()).thenReturn(values); + return request; + } + + private static String createConfigPayload(ConfigKey<?> configKey, String hostname) { + Slime data = new Slime(); + Cursor request = data.setObject(); + request.setString("defName", configKey.getName()); + request.setString("defNamespace", configKey.getNamespace()); + request.setString("defMD5", configKey.getMd5()); + request.setString("configId", configKey.getConfigId()); + request.setString("clientHostname", hostname); + try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { + new JsonFormat(false).encode(out, data); + return new String(out.toByteArray()); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + private static X509Certificate createDummyCertificate() { + return X509CertificateBuilder.fromKeypair( + KeyUtils.generateKeypair(KeyAlgorithm.EC), + new X500Principal("CN=" + HOSTNAME), + Instant.EPOCH, + Instant.EPOCH.plus(1, DAYS), + SignatureAlgorithm.SHA256_WITH_ECDSA, + BigInteger.ONE) + .build(); + } + + private static class DirectExecutor implements Executor { + + @Override + public void execute(Runnable command) { + command.run(); + } + } + + private static class StaticNodeIdentifier implements NodeIdentifier { + final NodeIdentity identity; + + StaticNodeIdentifier(NodeIdentity identity) { + this.identity = identity; + } + + @Override + public NodeIdentity identifyNode(List<X509Certificate> peerCertificateChain) throws NodeIdentifierException { + return identity; + } + } + +}
\ No newline at end of file diff --git a/container-core/src/main/java/com/yahoo/language/provider/DefaultLinguisticsProvider.java b/container-core/src/main/java/com/yahoo/language/provider/DefaultLinguisticsProvider.java index 8f08e33a777..d9af9bf88b2 100644 --- a/container-core/src/main/java/com/yahoo/language/provider/DefaultLinguisticsProvider.java +++ b/container-core/src/main/java/com/yahoo/language/provider/DefaultLinguisticsProvider.java @@ -1,10 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.language.provider; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.inject.Inject; -import com.yahoo.language.opennlp.OpenNlpLinguistics; import com.yahoo.container.di.componentgraph.Provider; import com.yahoo.language.Linguistics; +import com.yahoo.language.opennlp.OpenNlpLinguistics; /** * Provides the default linguistics implementation if no linguistics component has been explicitly configured @@ -14,14 +16,15 @@ import com.yahoo.language.Linguistics; */ public class DefaultLinguisticsProvider implements Provider<Linguistics> { - private final Linguistics linguistics; + // Use lazy initialization to avoid expensive (memory-wise) instantiation f + private volatile Supplier<Linguistics> linguisticsSupplier = Suppliers.memoize(OpenNlpLinguistics::new); @SuppressWarnings("deprecation") @Inject - public DefaultLinguisticsProvider() { linguistics = new OpenNlpLinguistics(); } + public DefaultLinguisticsProvider() { } @Override - public Linguistics get() { return linguistics; } + public Linguistics get() { return linguisticsSupplier.get(); } @Override public void deconstruct() {} diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java index 04b1d526c67..3b30e0df9ad 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java @@ -30,6 +30,7 @@ import com.yahoo.processing.execution.Execution.Trace; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.Searcher; +import com.yahoo.search.dispatch.rpc.MockRpcResourcePoolBuilder; import com.yahoo.search.dispatch.searchcluster.Node; import com.yahoo.search.grouping.GroupingRequest; import com.yahoo.search.grouping.request.AllOperation; @@ -66,7 +67,6 @@ import static org.junit.Assert.assertTrue; * * @author bratseth */ -@SuppressWarnings({ "rawtypes", "unchecked", "deprecation" }) public class FastSearcherTestCase { private final static DocumentdbInfoConfig documentdbInfoConfig = new DocumentdbInfoConfig(new DocumentdbInfoConfig.Builder()); @@ -77,7 +77,7 @@ public class FastSearcherTestCase { Logger.getLogger(FastSearcher.class.getName()).setLevel(Level.ALL); FastSearcher fastSearcher = new FastSearcher(new MockBackend(), new FS4ResourcePool("container.0", 1), - new MockDispatcher("a", Collections.emptyList()), + MockDispatcher.create(Collections.emptyList()), new SummaryParameters(null), new ClusterParams("testhittype"), documentdbInfoConfig); @@ -94,7 +94,7 @@ public class FastSearcherTestCase { Logger.getLogger(FastSearcher.class.getName()).setLevel(Level.ALL); FastSearcher fastSearcher = new FastSearcher(new MockBackend(), new FS4ResourcePool("container.0", 1), - new MockDispatcher("a", Collections.emptyList()), + MockDispatcher.create(Collections.emptyList()), new SummaryParameters(null), new ClusterParams("testhittype"), documentdbInfoConfig); @@ -121,12 +121,14 @@ public class FastSearcherTestCase { List<Node> nodes = new ArrayList<>(); nodes.add(new Node(0, "host1", 5000, 0)); - nodes.add(new Node(2, "host2", 5000, 0)); + nodes.add(new Node(1, "host2", 5000, 0)); + + var mockFs4ResourcePool = new MockFS4ResourcePool(); + var mockRpcResourcePool = new MockRpcResourcePoolBuilder().connection(0).connection(1).build(); - MockFS4ResourcePool mockFs4ResourcePool = new MockFS4ResourcePool(); FastSearcher fastSearcher = new FastSearcher(new MockBackend(), mockFs4ResourcePool, - new MockDispatcher("a", nodes, mockFs4ResourcePool, 1, new VipStatus()), + MockDispatcher.create(nodes, mockFs4ResourcePool, mockRpcResourcePool, 1, new VipStatus()), new SummaryParameters(null), new ClusterParams("testhittype"), documentdbConfigWithOneDb); @@ -145,8 +147,7 @@ public class FastSearcherTestCase { doFill(fastSearcher, result); ErrorMessage error = result.hits().getError(); assertEquals("Since we don't actually run summary backends we get this error when the Dispatcher is used", - "Error response from rpc node connection to hostX:0: Connection error", - error.getDetailedMessage().replaceAll("host[12]", "hostX")); + "getDocsums(..) attempted for node X", error.getDetailedMessage().replaceAll("\\d", "X")); } { // direct.summaries due to no summary features @@ -155,8 +156,7 @@ public class FastSearcherTestCase { doFill(fastSearcher, result); ErrorMessage error = result.hits().getError(); assertEquals("Since we don't actually run summary backends we get this error when the Dispatcher is used", - "Error response from rpc node connection to hostX:0: Connection error", - error.getDetailedMessage().replaceAll("host[12]", "hostX")); + "getDocsums(..) attempted for node X", error.getDetailedMessage().replaceAll("\\d", "X")); } } @@ -167,7 +167,7 @@ public class FastSearcherTestCase { new DocumentdbInfoConfig(new DocumentdbInfoConfig.Builder().documentdb(new DocumentdbInfoConfig.Documentdb.Builder().name("testDb"))); FastSearcher fastSearcher = new FastSearcher(mockBackend, new FS4ResourcePool("container.0", 1), - new MockDispatcher("a", Collections.emptyList()), + MockDispatcher.create(Collections.emptyList()), new SummaryParameters(null), new ClusterParams("testhittype"), documentdbConfigWithOneDb); @@ -335,7 +335,7 @@ public class FastSearcherTestCase { Logger.getLogger(FastSearcher.class.getName()).setLevel(Level.ALL); return new FastSearcher(mockBackend, new FS4ResourcePool("container.0", 1), - new MockDispatcher("a", Collections.emptyList()), + MockDispatcher.create(Collections.emptyList()), new SummaryParameters(null), new ClusterParams("testhittype"), config); @@ -345,7 +345,7 @@ public class FastSearcherTestCase { public void testSinglePassGroupingIsForcedWithSingleNodeGroups() { FastSearcher fastSearcher = new FastSearcher(new MockBackend(), new FS4ResourcePool("container.0", 1), - new MockDispatcher(new Node(0, "host0", 123, 0)), + MockDispatcher.create(Collections.singletonList(new Node(0, "host0", 123, 0))), new SummaryParameters(null), new ClusterParams("testhittype"), documentdbInfoConfig); @@ -366,9 +366,7 @@ public class FastSearcherTestCase { @Test public void testSinglePassGroupingIsNotForcedWithSingleNodeGroups() { - MockDispatcher dispatcher = - new MockDispatcher("a", ImmutableList.of(new Node(0, "host0", 123, 0), - new Node(2, "host1", 123, 0))); + MockDispatcher dispatcher = MockDispatcher.create(ImmutableList.of(new Node(0, "host0", 123, 0), new Node(2, "host1", 123, 0))); FastSearcher fastSearcher = new FastSearcher(new MockBackend(), new FS4ResourcePool("container.0", 1), @@ -411,7 +409,7 @@ public class FastSearcherTestCase { Backend backend = listeners.getBackend(server.host.getHostString(),server.host.getPort()); FastSearcher fastSearcher = new FastSearcher(backend, new FS4ResourcePool("container.0", 1), - new MockDispatcher("a", Collections.emptyList()), + MockDispatcher.create(Collections.emptyList()), new SummaryParameters(null), new ClusterParams("testhittype"), documentdbInfoConfig); diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTester.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTester.java index f01286d0c3a..6eab16045c2 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTester.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTester.java @@ -13,6 +13,8 @@ import com.yahoo.prelude.fastsearch.test.fs4mock.MockBackend; import com.yahoo.prelude.fastsearch.test.fs4mock.MockFS4ResourcePool; import com.yahoo.search.Query; import com.yahoo.search.Result; +import com.yahoo.search.dispatch.rpc.MockRpcResourcePoolBuilder; +import com.yahoo.search.dispatch.rpc.RpcResourcePool; import com.yahoo.search.dispatch.searchcluster.Node; import com.yahoo.search.searchchain.Execution; @@ -31,6 +33,7 @@ class FastSearcherTester { public static final String selfHostname = HostName.getLocalhost(); private final MockFS4ResourcePool mockFS4ResourcePool; + private final RpcResourcePool mockRpcResourcePool; private final FastSearcher fastSearcher; private final MockDispatcher mockDispatcher; private final VipStatus vipStatus; @@ -53,7 +56,10 @@ class FastSearcherTester { vipStatus = new VipStatus(b.build()); mockFS4ResourcePool = new MockFS4ResourcePool(); - mockDispatcher = new MockDispatcher(clusterId, searchNodes, mockFS4ResourcePool, containerClusterSize, vipStatus); + var builder = new MockRpcResourcePoolBuilder(); + searchNodes.forEach(node -> builder.connection(node.key())); + mockRpcResourcePool = builder.build(); + mockDispatcher = MockDispatcher.create(searchNodes, mockFS4ResourcePool, mockRpcResourcePool, containerClusterSize, vipStatus); fastSearcher = new FastSearcher(new MockBackend(selfHostname, 0L, true), mockFS4ResourcePool, mockDispatcher, diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/MockDispatcher.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/MockDispatcher.java index 6be895f33d2..e78fd920adc 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/MockDispatcher.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/MockDispatcher.java @@ -2,28 +2,37 @@ package com.yahoo.prelude.fastsearch.test; import com.yahoo.container.handler.VipStatus; +import com.yahoo.prelude.fastsearch.FS4InvokerFactory; import com.yahoo.prelude.fastsearch.FS4ResourcePool; import com.yahoo.search.Result; import com.yahoo.search.dispatch.Dispatcher; +import com.yahoo.search.dispatch.rpc.RpcInvokerFactory; +import com.yahoo.search.dispatch.rpc.RpcResourcePool; import com.yahoo.search.dispatch.searchcluster.Node; +import com.yahoo.search.dispatch.searchcluster.SearchCluster; import com.yahoo.vespa.config.search.DispatchConfig; -import java.util.Collections; import java.util.List; class MockDispatcher extends Dispatcher { + public static MockDispatcher create(List<Node> nodes) { + var fs4ResourcePool = new FS4ResourcePool("container.0", 1); + var rpcResourcePool = new RpcResourcePool(toDispatchConfig(nodes)); - public MockDispatcher(Node node) { - this(node.hostname(), Collections.singletonList(node)); + return create(nodes, fs4ResourcePool, rpcResourcePool, 1, new VipStatus()); } - public MockDispatcher(String clusterId, List<Node> nodes) { - this(clusterId, nodes, new FS4ResourcePool("container.0", 1), 1, new VipStatus()); + public static MockDispatcher create(List<Node> nodes, FS4ResourcePool fs4ResourcePool, RpcResourcePool rpcResourcePool, + int containerClusterSize, VipStatus vipStatus) { + var dispatchConfig = toDispatchConfig(nodes); + var searchCluster = new SearchCluster("a", dispatchConfig, containerClusterSize, vipStatus); + return new MockDispatcher(searchCluster, dispatchConfig, fs4ResourcePool, rpcResourcePool); } - public MockDispatcher(String clusterId, List<Node> nodes, FS4ResourcePool fs4ResourcePool, - int containerClusterSize, VipStatus vipStatus) { - super(clusterId, toDispatchConfig(nodes), fs4ResourcePool, containerClusterSize, vipStatus, new MockMetric()); + private MockDispatcher(SearchCluster searchCluster, DispatchConfig dispatchConfig, FS4ResourcePool fs4ResourcePool, + RpcResourcePool rpcResourcePool) { + super(searchCluster, dispatchConfig, new FS4InvokerFactory(fs4ResourcePool, searchCluster), + new RpcInvokerFactory(rpcResourcePool, searchCluster, dispatchConfig.dispatchWithProtobuf()), new MockMetric()); } private static DispatchConfig toDispatchConfig(List<Node> nodes) { @@ -43,4 +52,5 @@ class MockDispatcher extends Dispatcher { public void fill(Result result, String summaryClass) { } + } diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/rpc/MockRpcResourcePoolBuilder.java b/container-search/src/test/java/com/yahoo/search/dispatch/rpc/MockRpcResourcePoolBuilder.java new file mode 100644 index 00000000000..dbef9d819e8 --- /dev/null +++ b/container-search/src/test/java/com/yahoo/search/dispatch/rpc/MockRpcResourcePoolBuilder.java @@ -0,0 +1,54 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.dispatch.rpc; + +import com.yahoo.compress.CompressionType; +import com.yahoo.prelude.fastsearch.FastHit; +import com.yahoo.search.dispatch.rpc.Client.NodeConnection; +import com.yahoo.search.dispatch.rpc.Client.ResponseReceiver; +import com.yahoo.search.dispatch.rpc.RpcFillInvoker.GetDocsumsResponseReceiver; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** + * @author ovirtanen + */ +public class MockRpcResourcePoolBuilder { + + private Map<Integer, NodeConnection> nodeConnections = new HashMap<>(); + + public MockRpcResourcePoolBuilder connection(int distKey) { + nodeConnections.put(distKey, new MockNodeConnection(distKey)); + return this; + } + + public RpcResourcePool build() { + return new RpcResourcePool(nodeConnections); + } + + private static class MockNodeConnection implements NodeConnection { + private final int key; + + public MockNodeConnection(int key) { + this.key = key; + } + + @Override + public void getDocsums(List<FastHit> hits, CompressionType compression, int uncompressedLength, byte[] compressedSlime, + GetDocsumsResponseReceiver responseReceiver, double timeoutSeconds) { + responseReceiver.receive(Client.ResponseOrError.fromError("getDocsums(..) attempted for node " + key)); + } + + @Override + public void request(String rpcMethod, CompressionType compression, int uncompressedLength, byte[] compressedPayload, + ResponseReceiver responseReceiver, double timeoutSeconds) { + responseReceiver.receive(Client.ResponseOrError.fromError("request('"+rpcMethod+"', ..) attempted for node " + key)); + } + + @Override + public void close() { + } + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index 78e1d4756fb..83a347819f2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -388,41 +388,35 @@ public class InternalStepRunner implements StepRunner { logger.log("Attempting to find endpoints ..."); Map<ZoneId, List<URI>> endpoints = deploymentEndpoints(id.application(), zones); + if ( ! endpoints.containsKey(id.type().zone(controller.system())) && timedOut(deployment.get(), endpointTimeout)) { + logger.log(WARNING, "Endpoints for the deployment to test vanished again, while it was still active!"); + return Optional.of(error); + } List<String> messages = new ArrayList<>(); - messages.add("Found endpoints"); + messages.add("Found endpoints:"); endpoints.forEach((zone, uris) -> { messages.add("- " + zone); uris.forEach(uri -> messages.add(" |-- " + uri)); }); logger.log(messages); - if ( ! endpoints.containsKey(id.type().zone(controller.system()))) { - if (timedOut(deployment.get(), endpointTimeout)) { - logger.log(WARNING, "Endpoints failed to show up within " + endpointTimeout.toMinutes() + " minutes!"); - return Optional.of(error); - } - logger.log("Endpoints for the deployment to test are not yet ready."); - return Optional.empty(); + Optional<URI> testerEndpoint = controller.jobController().testerEndpoint(id); + if (testerEndpoint.isEmpty() && timedOut(deployment.get(), endpointTimeout)) { + logger.log(WARNING, "Endpoints for the tester container vanished again, while it was still active!"); + return Optional.of(error); } - Map<ZoneId, List<String>> clusters = listClusters(id.application(), zones); - - Optional<URI> testerEndpoint = controller.jobController().testerEndpoint(id); - if (testerEndpoint.isPresent() && controller.jobController().cloud().ready(testerEndpoint.get())) { + if (controller.jobController().cloud().ready(testerEndpoint.get())) { logger.log("Starting tests ..."); controller.jobController().cloud().startTests(testerEndpoint.get(), TesterCloud.Suite.of(id.type()), testConfig(id.application(), id.type().zone(controller.system()), - controller.system(), endpoints, clusters)); + controller.system(), endpoints, + listClusters(id.application(), zones))); return Optional.of(running); } - if (timedOut(deployment.get(), endpointTimeout)) { - logger.log(WARNING, "Endpoint for tester failed to show up within " + endpointTimeout.toMinutes() + " minutes of real deployment!"); - return Optional.of(error); - } - - logger.log("Endpoints of tester container not yet available."); + logger.log("Tester container not yet ready."); return Optional.empty(); } @@ -462,7 +456,7 @@ public class InternalStepRunner implements StepRunner { private Optional<RunStatus> copyVespaLogs(RunId id, DualLogger logger) { ZoneId zone = id.type().zone(controller.system()); - if (controller.applications().require(id.application()).deployments().containsKey(zone)) + if (deployment(id.application(), id.type()).isPresent()) try { logger.log("Copying Vespa log from nodes of " + id.application() + " in " + zone + " ..."); List<LogEntry> entries = new ArrayList<>(); @@ -572,6 +566,7 @@ public class InternalStepRunner implements StepRunner { /** Returns the real application with the given id. */ private Application application(ApplicationId id) { + controller.applications().lockOrThrow(id, __ -> { }); // Memory fence. return controller.applications().require(id); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tls/ControllerSslContextFactoryProvider.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tls/ControllerSslContextFactoryProvider.java index dcc61b13bab..84d43314412 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tls/ControllerSslContextFactoryProvider.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tls/ControllerSslContextFactoryProvider.java @@ -56,6 +56,8 @@ public class ControllerSslContextFactoryProvider extends AbstractComponent imple /** Create a SslContextFactory backed by an in-memory key and trust store */ private SslContextFactory createSslContextFactory(int port) { + // TODO Use DefaultTlsContext to configure SslContextFactory (ensure that cipher/protocol configuration is same across all TLS endpoints). + SslContextFactory factory = new SslContextFactory(); if (port != 443) { factory.setWantClientAuth(true); @@ -63,6 +65,7 @@ public class ControllerSslContextFactoryProvider extends AbstractComponent imple factory.setTrustStore(truststore); factory.setKeyStore(keystore); factory.setKeyStorePassword(""); + factory.setExcludeProtocols("TLSv1.3"); // TLSv1.3 is broken is multiple OpenJDK 11 versions factory.setEndpointIdentificationAlgorithm(null); // disable https hostname verification of clients (must be disabled when using Athenz x509 certificates) return factory; } diff --git a/document/src/vespa/document/select/operator.cpp b/document/src/vespa/document/select/operator.cpp index 36113844d88..85dbef5ad9b 100644 --- a/document/src/vespa/document/select/operator.cpp +++ b/document/src/vespa/document/select/operator.cpp @@ -128,7 +128,7 @@ RegexOperator::match(const vespalib::string& val, vespalib::stringref expr) cons // Should we catch this in parsing? if (expr.size() == 0) return ResultList(Result::True); try { - std::basic_regex<char> expression(expr.data(), expr.size()); + std::regex expression(expr.data(), expr.size()); return ResultList(Result::get(std::regex_search(val.c_str(), val.c_str() + val.size(), expression))); } catch (std::regex_error &) { return ResultList(Result::False); diff --git a/jrt/src/com/yahoo/jrt/Spec.java b/jrt/src/com/yahoo/jrt/Spec.java index f245d110c7f..7e4f6d987fa 100644 --- a/jrt/src/com/yahoo/jrt/Spec.java +++ b/jrt/src/com/yahoo/jrt/Spec.java @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jrt; - import java.net.InetSocketAddress; import java.net.SocketAddress; @@ -12,11 +11,23 @@ import java.net.SocketAddress; */ public class Spec { - private SocketAddress address; - private String host; - private int port; - private boolean malformed; + private final SocketAddress address; + private final String host; + private final int port; + private final boolean malformed; + private final String asString; + + private static SocketAddress createAddress(String host, int port) { + return (host == null) + ? new InetSocketAddress(port) + : new InetSocketAddress(host, port); + } + private static String createString(String host, int port) { + return (host == null) + ? "tcp/" + port + : "tcp/" + host + ":" + port; + } /** * Create a Spec from a string. The form of the input string is * 'tcp/host:port' or 'tcp/port' where 'host' is the host name and @@ -29,21 +40,31 @@ public class Spec { if (spec.startsWith("tcp/")) { int sep = spec.indexOf(':'); String portStr; + String hostStr = null; if (sep == -1) { portStr = spec.substring(4); } else { - host = spec.substring(4, sep); + hostStr = spec.substring(4, sep); portStr = spec.substring(sep + 1); } + boolean correct = true; + int portNum = 0; try { - port = Integer.parseInt(portStr); + portNum = Integer.parseInt(portStr); } catch (NumberFormatException e) { - host = null; - port = 0; - malformed = true; + correct = false; } + port = portNum; + malformed = ! correct; + host = correct ? hostStr : null; + address = correct ? createAddress(host, port) : null; + asString = correct ? createString(host, port) : "MALFORMED"; } else { malformed = true; + port = 0; + host = null; + address = null; + asString = "MALFORMED"; } } @@ -56,6 +77,9 @@ public class Spec { public Spec(String host, int port) { this.host = host; this.port = port; + malformed = false; + asString = createString(host, port); + address = createAddress(host, port); } /** @@ -68,7 +92,7 @@ public class Spec { * @param port port number */ public Spec(int port) { - this.port = port; + this(null, port); } /** @@ -106,16 +130,6 @@ public class Spec { * @return socket address */ SocketAddress address() { - if (malformed) { - return null; - } - if (address == null) { - if (host == null) { - address = new InetSocketAddress(port); - } else { - address = new InetSocketAddress(host, port); - } - } return address; } @@ -126,13 +140,7 @@ public class Spec { * @return string representation of this address */ public String toString() { - if (malformed) { - return "MALFORMED"; - } - if (host == null) { - return "tcp/" + port; - } - return "tcp/" + host + ":" + port; + return asString; } } diff --git a/logd/src/tests/watcher/watcher_test.cpp b/logd/src/tests/watcher/watcher_test.cpp index 9f7c476e101..7e7585f9209 100644 --- a/logd/src/tests/watcher/watcher_test.cpp +++ b/logd/src/tests/watcher/watcher_test.cpp @@ -22,7 +22,7 @@ using vespalib::ThreadStackExecutor; using vespalib::makeLambdaTask; using namespace std::chrono_literals; -std::basic_regex<char> rotated_log(R"(vespa.log-[0-9]*-[0-9]*-[0-9]*\.[0-9]*-[0-9]*-[0-9]*)"); +std::regex rotated_log(R"(vespa.log-[0-9]*-[0-9]*-[0-9]*\.[0-9]*-[0-9]*-[0-9]*)"); namespace logdemon { diff --git a/messagebus/src/main/java/com/yahoo/messagebus/network/rpc/RPCTargetPool.java b/messagebus/src/main/java/com/yahoo/messagebus/network/rpc/RPCTargetPool.java index e681f0834a8..04d9f4ade68 100755 --- a/messagebus/src/main/java/com/yahoo/messagebus/network/rpc/RPCTargetPool.java +++ b/messagebus/src/main/java/com/yahoo/messagebus/network/rpc/RPCTargetPool.java @@ -95,17 +95,25 @@ public class RPCTargetPool { if (target != null) { return target; } - entry.close(); - targets.remove(key); + dropTarget(entry, key); } - RPCTarget [] tmpTargets = new RPCTarget[numTargetsPerSpec]; - for (int i=0; i < tmpTargets.length; i++) { - tmpTargets[i] = new RPCTarget(spec, orb); - } - entry = new Entry(tmpTargets, now); - targets.put(key, entry); - return entry.getTarget(now); + return createAndAddTarget(orb, spec, key, now); + } + } + + private void dropTarget(Entry entry, String key) { + entry.close(); + targets.remove(key); + } + + private RPCTarget createAndAddTarget(Supervisor orb, Spec spec, String key, long now) { + RPCTarget [] tmpTargets = new RPCTarget[numTargetsPerSpec]; + for (int i=0; i < tmpTargets.length; i++) { + tmpTargets[i] = new RPCTarget(spec, orb); } + Entry entry = new Entry(tmpTargets, now); + targets.put(key, entry); + return entry.getTarget(now); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java index a1c53c3d931..0a58ddc93f1 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java @@ -274,6 +274,9 @@ public class DockerOperationsImpl implements DockerOperations { List<Path> paths = new ArrayList<>(List.of( Paths.get("/etc/vespa/flags"), Paths.get("/etc/yamas-agent"), + Paths.get("/opt/splunkforwarder/var/log"), + Paths.get("/var/log"), + Paths.get("/var/spool/postfix/maildrop"), context.pathInNodeUnderVespaHome("logs/daemontools_y"), context.pathInNodeUnderVespaHome("logs/jdisc_core"), context.pathInNodeUnderVespaHome("logs/langdetect"), diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index 50bf57c78ff..80d924153c8 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -608,7 +608,7 @@ public class NodeAgentImpl implements NodeAgent { // TODO: Clean up and inline method when old metrics proxy has been discontinued. private void runPushMetricsCommand(NodeAgentContext context, String wrappedMetrics, boolean newMetricsProxy) { - int port = newMetricsProxy ? 19094 : 19091; + int port = newMetricsProxy ? 19095 : 19091; String[] command = {"vespa-rpc-invoke", "-t", "2", "tcp/localhost:" + port, "setExtraMetrics", wrappedMetrics}; try { dockerOperations.executeCommandInContainerAsRoot(context, 5L, command); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java index 416c70fbf10..7bd0cb00c82 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java @@ -680,7 +680,7 @@ public class NodeAgentImplTest { .replaceAll("\\s", "") .replaceAll("\\n", ""); - String[] expectedCommand = {"vespa-rpc-invoke", "-t", "2", "tcp/localhost:19094", "setExtraMetrics", expectedMetrics}; + String[] expectedCommand = {"vespa-rpc-invoke", "-t", "2", "tcp/localhost:19095", "setExtraMetrics", expectedMetrics}; doAnswer(invocation -> { NodeAgentContext calledContainerName = (NodeAgentContext) invocation.getArguments()[0]; long calledTimeout = (long) invocation.getArguments()[1]; @@ -691,7 +691,7 @@ public class NodeAgentImplTest { .replaceAll("([0-9]+\\.[0-9]{1,3})([0-9]*)", "$1"); // Only keep the first 3 decimals // TODO: Remove when old metrics proxy is discontinued. - calledCommand[3] = calledCommand[3].replaceFirst("19091", "19094"); + calledCommand[3] = calledCommand[3].replaceFirst("19091", "19095"); assertEquals(context, calledContainerName); assertEquals(5L, calledTimeout); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java index 349be09c4a5..d36638a1a66 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java @@ -42,10 +42,14 @@ public class CapacityPolicies { } public NodeResources decideNodeResources(Capacity requestedCapacity, ClusterSpec cluster) { - Optional<NodeResources> requestedResources = requestedCapacity.nodeResources(); + NodeResources resources = decideNodeResources(requestedCapacity.nodeResources(), cluster); + if (zone.system() == SystemName.cd) + return resources.withDiskSpeed(NodeResources.DiskSpeed.any); + else + return resources; + } - if (zone.system() == SystemName.cd && requestedResources.isPresent()) - requestedResources = Optional.of(requestedResources.get().withDiskSpeed(NodeResources.DiskSpeed.any)); + private NodeResources decideNodeResources(Optional<NodeResources> requestedResources, ClusterSpec cluster) { if (requestedResources.isPresent() && ! requestedResources.get().allocateByLegacyName()) return requestedResources.get(); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java index 5a06eb32505..242aeefd340 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java @@ -96,11 +96,11 @@ public class NodeRepositoryProvisioner implements Provisioner { if (zone.environment().isManuallyDeployed() && nodeCount < requestedCapacity.nodeCount()) logger.log(Level.INFO, "Requested " + requestedCapacity.nodeCount() + " nodes for " + cluster + ", downscaling to " + nodeCount + " nodes in " + zone.environment()); - NodeResources flavor = capacityPolicies.decideNodeResources(requestedCapacity, cluster); - log.log(LogLevel.DEBUG, () -> "Decided flavor for requested tenant nodes: " + flavor); + NodeResources resources = capacityPolicies.decideNodeResources(requestedCapacity, cluster); + log.log(LogLevel.DEBUG, () -> "Decided node resources for requested tenant nodes: " + resources); boolean exclusive = capacityPolicies.decideExclusivity(cluster.isExclusive()); effectiveGroups = wantedGroups > nodeCount ? nodeCount : wantedGroups; // cannot have more groups than nodes - requestedNodes = NodeSpec.from(nodeCount, flavor, exclusive, requestedCapacity.canFail()); + requestedNodes = NodeSpec.from(nodeCount, resources, exclusive, requestedCapacity.canFail()); } else { requestedNodes = NodeSpec.from(requestedCapacity.type()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java index 4141161c20e..148d474e080 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java @@ -14,6 +14,7 @@ import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.OutOfCapacityException; import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; import com.yahoo.config.provisioning.FlavorsConfig; import com.yahoo.transaction.NestedTransaction; @@ -251,7 +252,27 @@ public class DynamicDockerAllocationTest { tester.activate(application1, ImmutableSet.copyOf(hosts)); List<Node> initialSpareCapacity = findSpareCapacity(tester); - assertThat(initialSpareCapacity.size(), is(0)); + assertEquals(0, initialSpareCapacity.size()); + } + + @Test + public void cd_uses_slow_disk_nodes_for_docker_hosts() { + ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(SystemName.cd, Environment.test, RegionName.from("us-east"))).flavorsConfig(flavorsConfig()).build(); + tester.makeReadyNodes(4, new Flavor(new NodeResources(1, 2, 3, NodeResources.DiskSpeed.slow)), NodeType.host, 10, true); + deployZoneApp(tester); + ApplicationId application1 = tester.makeApplicationId(); + List<HostSpec> hosts = tester.prepare(application1, clusterSpec("myContent.t1.a1"), 3, 1, new NodeResources(1, 1, 1)); + tester.activate(application1, ImmutableSet.copyOf(hosts)); + } + + @Test + public void cd_uses_slow_disk_nodes_for_docker_hosts_with_default_flavor() { + ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(SystemName.cd, Environment.test, RegionName.from("us-east"))).flavorsConfig(flavorsConfig()).build(); + tester.makeReadyNodes(4, new Flavor(new NodeResources(1, 2, 3, NodeResources.DiskSpeed.slow)), NodeType.host, 10, true); + deployZoneApp(tester); + ApplicationId application1 = tester.makeApplicationId(); + List<HostSpec> hosts = tester.prepare(application1, clusterSpec("myContent.t1.a1"), Capacity.fromCount(3, Optional.empty(), false, true), 1); + tester.activate(application1, ImmutableSet.copyOf(hosts)); } @Test(expected = OutOfCapacityException.class) diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java index d63da52eb49..e1ba3291bd3 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java @@ -130,12 +130,12 @@ public class ProvisioningTester { public void patchNode(Node node) { nodeRepository.write(node, () -> {}); } - public List<HostSpec> prepare(ApplicationId application, ClusterSpec cluster, int nodeCount, int groups, NodeResources flavor) { - return prepare(application, cluster, nodeCount, groups, false, flavor); + public List<HostSpec> prepare(ApplicationId application, ClusterSpec cluster, int nodeCount, int groups, NodeResources resources) { + return prepare(application, cluster, nodeCount, groups, false, resources); } - public List<HostSpec> prepare(ApplicationId application, ClusterSpec cluster, int nodeCount, int groups, boolean required, NodeResources flavor) { - return prepare(application, cluster, Capacity.fromCount(nodeCount, Optional.ofNullable(flavor), required, true), groups); + public List<HostSpec> prepare(ApplicationId application, ClusterSpec cluster, int nodeCount, int groups, boolean required, NodeResources resources) { + return prepare(application, cluster, Capacity.fromCount(nodeCount, Optional.ofNullable(resources), required, true), groups); } public List<HostSpec> prepare(ApplicationId application, ClusterSpec cluster, Capacity capacity, int groups) { diff --git a/searchcore/src/tests/proton/docsummary/docsummary.cpp b/searchcore/src/tests/proton/docsummary/docsummary.cpp index a3acbfdbfe0..38b85594a62 100644 --- a/searchcore/src/tests/proton/docsummary/docsummary.cpp +++ b/searchcore/src/tests/proton/docsummary/docsummary.cpp @@ -657,7 +657,7 @@ Test::requireThatSummariesTimeout() vespalib::Slime summary = getSlime(*rep, 0, false); JsonFormat::encode(summary, buf, false); auto bufstring = buf.get().make_stringref(); - EXPECT_TRUE(std::regex_search(bufstring.data(), bufstring.data() + bufstring.size(), std::basic_regex<char>("Timed out with -[0-9]+us left."))); + EXPECT_TRUE(std::regex_search(bufstring.data(), bufstring.data() + bufstring.size(), std::regex("Timed out with -[0-9]+us left."))); } void diff --git a/searchlib/src/tests/features/imported_dot_product/imported_dot_product_test.cpp b/searchlib/src/tests/features/imported_dot_product/imported_dot_product_test.cpp index 54c77fb25a7..a8352acc952 100644 --- a/searchlib/src/tests/features/imported_dot_product/imported_dot_product_test.cpp +++ b/searchlib/src/tests/features/imported_dot_product/imported_dot_product_test.cpp @@ -48,7 +48,7 @@ struct FixtureBase : ImportedAttributeFixture { feature.getQueryEnv().getProperties().add("dotProduct.vector", vector); if (pre_parsed) { - feature.getQueryEnv().getObjectStore().add("dotProduct.vector.object", std::move(pre_parsed)); + feature.getQueryEnv().getObjectStore().add("dotProduct.vector", std::make_unique<DotProductBlueprint::SharedState>(nullptr, std::move(pre_parsed))); } auto readGuard = imported_attr->makeReadGuard(false); const IAttributeVector *attr = readGuard->attribute(); @@ -143,9 +143,11 @@ struct ArrayFixture : FixtureBase { auto& obj_store = feature.getQueryEnv().getObjectStore(); bp.prepareSharedState(feature.getQueryEnv(), obj_store); // Resulting name is very implementation defined. But at least the tests will break if it changes. - const auto* parsed = obj_store.get("dotProduct.fancyvector.object"); - ASSERT_TRUE(parsed != nullptr); - const auto* as_object = dynamic_cast<const ExpectedType*>(parsed); + const auto* anything = obj_store.get("dotProduct.fancyvector"); + ASSERT_TRUE(anything != nullptr); + const auto* state = dynamic_cast<const DotProductBlueprint::SharedState*>(anything); + ASSERT_TRUE(state != nullptr); + const auto* as_object = dynamic_cast<const ExpectedType*>(state->_arguments.get()); ASSERT_TRUE(as_object != nullptr); verify(expected, *as_object); } diff --git a/searchlib/src/vespa/searchlib/diskindex/fusion.cpp b/searchlib/src/vespa/searchlib/diskindex/fusion.cpp index 5bf540fdaaf..35204303bc5 100644 --- a/searchlib/src/vespa/searchlib/diskindex/fusion.cpp +++ b/searchlib/src/vespa/searchlib/diskindex/fusion.cpp @@ -323,7 +323,8 @@ Fusion::openFieldWriter(const SchemaUtil::IndexIterator &index, FieldWriter &wri { vespalib::string dir = _outDir + "/" + index.getName(); - if (!writer.open(dir + "/", 64, 262144, _dynamicKPosIndexFormat, false, index.getSchema(), + if (!writer.open(dir + "/", 64, 262144, _dynamicKPosIndexFormat, + index.use_experimental_posting_list_format(), index.getSchema(), index.getIndex(), _tuneFileIndexing._write, _fileHeaderContext)) { throw IllegalArgumentException(make_string("Could not open output posocc + dictionary in %s", dir.c_str())); } diff --git a/searchlib/src/vespa/searchlib/diskindex/indexbuilder.cpp b/searchlib/src/vespa/searchlib/diskindex/indexbuilder.cpp index 7a6e24f2529..f9620c35908 100644 --- a/searchlib/src/vespa/searchlib/diskindex/indexbuilder.cpp +++ b/searchlib/src/vespa/searchlib/diskindex/indexbuilder.cpp @@ -97,7 +97,8 @@ FileHandle::open(vespalib::stringref dir, _fieldWriter = std::make_shared<FieldWriter>(docIdLimit, numWordIds); - if (!_fieldWriter->open(dir + "/", 64, 262144u, false, false, + if (!_fieldWriter->open(dir + "/", 64, 262144u, false, + index.use_experimental_posting_list_format(), index.getSchema(), index.getIndex(), tuneFileWrite, fileHeaderContext)) { LOG(error, "Could not open term writer %s for write (%s)", diff --git a/searchlib/src/vespa/searchlib/diskindex/zcposocciterators.cpp b/searchlib/src/vespa/searchlib/diskindex/zcposocciterators.cpp index 3e0a36bcccd..5ba78a110b0 100644 --- a/searchlib/src/vespa/searchlib/diskindex/zcposocciterators.cpp +++ b/searchlib/src/vespa/searchlib/diskindex/zcposocciterators.cpp @@ -14,10 +14,12 @@ using search::index::PostingListCounts; template <bool bigEndian, bool dynamic_k> ZcRareWordPosOccIterator<bigEndian, dynamic_k>:: -ZcRareWordPosOccIterator(Position start, uint64_t bitLength, uint32_t docIdLimit, bool decode_cheap_features, - const PosOccFieldsParams *fieldsParams, - const TermFieldMatchDataArray &matchData) - : ZcRareWordPostingIterator<bigEndian, dynamic_k>(matchData, start, docIdLimit, decode_cheap_features), +ZcRareWordPosOccIterator(Position start, uint64_t bitLength, uint32_t docIdLimit, + bool decode_normal_features, bool decode_cheap_features, + const PosOccFieldsParams *fieldsParams, + const TermFieldMatchDataArray &matchData) + : ZcRareWordPostingIterator<bigEndian, dynamic_k>(matchData, start, docIdLimit, + decode_normal_features, decode_cheap_features), _decodeContextReal(start.getOccurences(), start.getBitOffset(), bitLength, fieldsParams) { assert(!matchData.valid() || (fieldsParams->getNumFields() == matchData.size())); @@ -26,11 +28,13 @@ ZcRareWordPosOccIterator(Position start, uint64_t bitLength, uint32_t docIdLimit template <bool bigEndian, bool dynamic_k> ZcPosOccIterator<bigEndian, dynamic_k>:: -ZcPosOccIterator(Position start, uint64_t bitLength, uint32_t docIdLimit, bool decode_cheap_features, +ZcPosOccIterator(Position start, uint64_t bitLength, uint32_t docIdLimit, + bool decode_normal_features, bool decode_cheap_features, uint32_t minChunkDocs, const PostingListCounts &counts, const PosOccFieldsParams *fieldsParams, const TermFieldMatchDataArray &matchData) - : ZcPostingIterator<bigEndian>(minChunkDocs, dynamic_k, counts, matchData, start, docIdLimit, decode_cheap_features), + : ZcPostingIterator<bigEndian>(minChunkDocs, dynamic_k, counts, matchData, start, docIdLimit, + decode_normal_features, decode_cheap_features), _decodeContextReal(start.getOccurences(), start.getBitOffset(), bitLength, fieldsParams) { assert(!matchData.valid() || (fieldsParams->getNumFields() == matchData.size())); @@ -51,15 +55,15 @@ create_zc_posocc_iterator(const PostingListCounts &counts, bitcompression::Posit assert((num_docs == counts._numDocs) || ((num_docs == posting_params._min_chunk_docs) && (num_docs < counts._numDocs))); if (num_docs < posting_params._min_skip_docs) { if (posting_params._dynamic_k) { - return std::make_unique<ZcRareWordPosOccIterator<bigEndian, true>>(start, bit_length, posting_params._doc_id_limit, posting_params._encode_cheap_features, &fields_params, match_data); + return std::make_unique<ZcRareWordPosOccIterator<bigEndian, true>>(start, bit_length, posting_params._doc_id_limit, posting_params._encode_features, posting_params._encode_cheap_features, &fields_params, match_data); } else { - return std::make_unique<ZcRareWordPosOccIterator<bigEndian, false>>(start, bit_length, posting_params._doc_id_limit, posting_params._encode_cheap_features, &fields_params, match_data); + return std::make_unique<ZcRareWordPosOccIterator<bigEndian, false>>(start, bit_length, posting_params._doc_id_limit, posting_params._encode_features, posting_params._encode_cheap_features, &fields_params, match_data); } } else { if (posting_params._dynamic_k) { - return std::make_unique<ZcPosOccIterator<bigEndian, true>>(start, bit_length, posting_params._doc_id_limit, posting_params._encode_cheap_features, posting_params._min_chunk_docs, counts, &fields_params, match_data); + return std::make_unique<ZcPosOccIterator<bigEndian, true>>(start, bit_length, posting_params._doc_id_limit, posting_params._encode_features, posting_params._encode_cheap_features, posting_params._min_chunk_docs, counts, &fields_params, match_data); } else { - return std::make_unique<ZcPosOccIterator<bigEndian, false>>(start, bit_length, posting_params._doc_id_limit, posting_params._encode_cheap_features, posting_params._min_chunk_docs, counts, &fields_params, match_data); + return std::make_unique<ZcPosOccIterator<bigEndian, false>>(start, bit_length, posting_params._doc_id_limit, posting_params._encode_features, posting_params._encode_cheap_features, posting_params._min_chunk_docs, counts, &fields_params, match_data); } } } diff --git a/searchlib/src/vespa/searchlib/diskindex/zcposocciterators.h b/searchlib/src/vespa/searchlib/diskindex/zcposocciterators.h index 107636df0bf..46cbd7a49bb 100644 --- a/searchlib/src/vespa/searchlib/diskindex/zcposocciterators.h +++ b/searchlib/src/vespa/searchlib/diskindex/zcposocciterators.h @@ -19,9 +19,10 @@ private: using DecodeContextReal = std::conditional_t<dynamic_k, bitcompression::EGPosOccDecodeContextCooked<bigEndian>, bitcompression::EG2PosOccDecodeContextCooked<bigEndian>>; DecodeContextReal _decodeContextReal; public: - ZcRareWordPosOccIterator(Position start, uint64_t bitLength, uint32_t docIdLimit, bool decode_cheap_features, - const bitcompression::PosOccFieldsParams *fieldsParams, - const fef::TermFieldMatchDataArray &matchData); + ZcRareWordPosOccIterator(Position start, uint64_t bitLength, uint32_t docIdLimit, + bool decode_normal_features, bool decode_cheap_features, + const bitcompression::PosOccFieldsParams *fieldsParams, + const fef::TermFieldMatchDataArray &matchData); }; @@ -35,10 +36,11 @@ private: using DecodeContext = std::conditional_t<dynamic_k, bitcompression::EGPosOccDecodeContextCooked<bigEndian>, bitcompression::EG2PosOccDecodeContextCooked<bigEndian>>; DecodeContext _decodeContextReal; public: - ZcPosOccIterator(Position start, uint64_t bitLength, uint32_t docIdLimit, bool decode_cheap_features, - uint32_t minChunkDocs, const index::PostingListCounts &counts, - const bitcompression::PosOccFieldsParams *fieldsParams, - const fef::TermFieldMatchDataArray &matchData); + ZcPosOccIterator(Position start, uint64_t bitLength, uint32_t docIdLimit, + bool decode_normal_features, bool decode_cheap_features, + uint32_t minChunkDocs, const index::PostingListCounts &counts, + const bitcompression::PosOccFieldsParams *fieldsParams, + const fef::TermFieldMatchDataArray &matchData); }; std::unique_ptr<search::queryeval::SearchIterator> diff --git a/searchlib/src/vespa/searchlib/diskindex/zcpostingiterators.cpp b/searchlib/src/vespa/searchlib/diskindex/zcpostingiterators.cpp index 53edae444af..6f0caf333d3 100644 --- a/searchlib/src/vespa/searchlib/diskindex/zcpostingiterators.cpp +++ b/searchlib/src/vespa/searchlib/diskindex/zcpostingiterators.cpp @@ -37,12 +37,13 @@ ZcIteratorBase::initRange(uint32_t beginid, uint32_t endid) template <bool bigEndian> ZcRareWordPostingIteratorBase<bigEndian>:: -ZcRareWordPostingIteratorBase(const TermFieldMatchDataArray &matchData, Position start, uint32_t docIdLimit, bool decode_cheap_features) +ZcRareWordPostingIteratorBase(const TermFieldMatchDataArray &matchData, Position start, uint32_t docIdLimit, bool decode_normal_features, bool decode_cheap_features) : ZcIteratorBase(matchData, start, docIdLimit), _decodeContext(nullptr), _residue(0), _prevDocId(0), _numDocs(0), + _decode_normal_features(decode_normal_features), _decode_cheap_features(decode_cheap_features), _field_length(0), _num_occs(0) @@ -51,8 +52,8 @@ ZcRareWordPostingIteratorBase(const TermFieldMatchDataArray &matchData, Position template <bool bigEndian, bool dynamic_k> ZcRareWordPostingIterator<bigEndian, dynamic_k>:: -ZcRareWordPostingIterator(const TermFieldMatchDataArray &matchData, Position start, uint32_t docIdLimit, bool decode_cheap_features) - : ZcRareWordPostingIteratorBase<bigEndian>(matchData, start, docIdLimit, decode_cheap_features), +ZcRareWordPostingIterator(const TermFieldMatchDataArray &matchData, Position start, uint32_t docIdLimit, bool decode_normal_features, bool decode_cheap_features) + : ZcRareWordPostingIteratorBase<bigEndian>(matchData, start, docIdLimit, decode_normal_features, decode_cheap_features), _doc_id_k_param() { } @@ -87,11 +88,13 @@ ZcRareWordPostingIterator<bigEndian, dynamic_k>::doSeek(uint32_t docId) } } while (__builtin_expect(oDocId < docId, true)) { - UC64_DECODECONTEXT_STORE(o, _decodeContext->_); - _decodeContext->skipFeatures(1); - UC64_DECODECONTEXT_LOAD(o, _decodeContext->_); - if (__builtin_expect(--_residue == 0, false)) { - goto atbreak; + if (_decode_normal_features) { + UC64_DECODECONTEXT_STORE(o, _decodeContext->_); + _decodeContext->skipFeatures(1); + UC64_DECODECONTEXT_LOAD(o, _decodeContext->_); + if (__builtin_expect(--_residue == 0, false)) { + goto atbreak; + } } UC64_DECODEEXPGOLOMB_NS(o, _doc_id_k_param.get_doc_id_k(), EC); oDocId += 1 + static_cast<uint32_t>(val64); @@ -123,7 +126,11 @@ ZcRareWordPostingIteratorBase<bigEndian>::doUnpack(uint32_t docId) return; } assert(docId == getDocId()); - _decodeContext->unpackFeatures(_matchData, docId); + if (_decode_normal_features) { + _decodeContext->unpackFeatures(_matchData, docId); + } else { + _matchData[0]->reset(docId); + } if (_decode_cheap_features) { TermFieldMatchData *tfmd = _matchData[0]; tfmd->setFieldLength(_field_length); @@ -167,7 +174,7 @@ ZcRareWordPostingIterator<bigEndian, dynamic_k>::readWordStart(uint32_t docIdLim clearUnpacked(); } -ZcPostingIteratorBase::ZcPostingIteratorBase(const TermFieldMatchDataArray &matchData, Position start, uint32_t docIdLimit, bool decode_cheap_features) +ZcPostingIteratorBase::ZcPostingIteratorBase(const TermFieldMatchDataArray &matchData, Position start, uint32_t docIdLimit, bool decode_normal_features, bool decode_cheap_features) : ZcIteratorBase(matchData, start, docIdLimit), _valI(nullptr), _valIBase(nullptr), @@ -179,6 +186,7 @@ ZcPostingIteratorBase::ZcPostingIteratorBase(const TermFieldMatchDataArray &matc _chunk(), _featuresSize(0), _hasMore(false), + _decode_normal_features(decode_normal_features), _decode_cheap_features(decode_cheap_features), _chunkNo(0), _field_length(0), @@ -192,8 +200,10 @@ ZcPostingIterator(uint32_t minChunkDocs, bool dynamicK, const PostingListCounts &counts, const search::fef::TermFieldMatchDataArray &matchData, - Position start, uint32_t docIdLimit, bool decode_cheap_features) - : ZcPostingIteratorBase(matchData, start, docIdLimit, decode_cheap_features), + Position start, uint32_t docIdLimit, + bool decode_normal_features, bool decode_cheap_features) + : ZcPostingIteratorBase(matchData, start, docIdLimit, + decode_normal_features, decode_cheap_features), _decodeContext(nullptr), _minChunkDocs(minChunkDocs), _docIdK(0), @@ -254,8 +264,10 @@ ZcPostingIterator<bigEndian>::readWordStart(uint32_t docIdLimit) UC64_DECODEEXPGOLOMB_NS(o, K_VALUE_ZCPOSTING_L4SKIPSIZE, EC); l4SkipSize = val64; } - UC64_DECODEEXPGOLOMB_NS(o, K_VALUE_ZCPOSTING_FEATURESSIZE, EC); - _featuresSize = val64; + if (_decode_normal_features) { + UC64_DECODEEXPGOLOMB_NS(o, K_VALUE_ZCPOSTING_FEATURESSIZE, EC); + _featuresSize = val64; + } if (_dynamicK) { UC64_DECODEEXPGOLOMB_NS(o, _docIdK, EC); } else { @@ -332,7 +344,7 @@ ZcPostingIteratorBase::doL4SkipSeek(uint32_t docId) } do { lastL4SkipDocId = _l4._skipDocId; - _l4.decodeSkipEntry(); + _l4.decodeSkipEntry(_decode_normal_features); _l4.nextDocId(); #if DEBUG_ZCPOSTING_PRINTF printf("L4Decode docId %d, docIdPos %d," @@ -385,7 +397,7 @@ ZcPostingIteratorBase::doL3SkipSeek(uint32_t docId) } do { lastL3SkipDocId = _l3._skipDocId; - _l3.decodeSkipEntry(); + _l3.decodeSkipEntry(_decode_normal_features); _l3.nextDocId(); #if DEBUG_ZCPOSTING_PRINTF printf("L3Decode docId %d, docIdPos %d," @@ -432,7 +444,7 @@ ZcPostingIteratorBase::doL2SkipSeek(uint32_t docId) } do { lastL2SkipDocId = _l2._skipDocId; - _l2.decodeSkipEntry(); + _l2.decodeSkipEntry(_decode_normal_features); _l2.nextDocId(); #if DEBUG_ZCPOSTING_PRINTF printf("L2Decode docId %d, docIdPos %d, l1SkipPos %d, nextDocId %d\n", @@ -472,7 +484,7 @@ ZcPostingIteratorBase::doL1SkipSeek(uint32_t docId) } do { lastL1SkipDocId = _l1._skipDocId; - _l1.decodeSkipEntry(); + _l1.decodeSkipEntry(_decode_normal_features); _l1.nextDocId(); #if DEBUG_ZCPOSTING_PRINTF printf("L1Decode docId %d, docIdPos %d, L1SkipPos %d, nextDocId %d\n", @@ -556,11 +568,15 @@ ZcPostingIterator<bigEndian>::doUnpack(uint32_t docId) _featureSeekPos = 0; } assert(docId == getDocId()); - uint32_t needUnpack = getNeedUnpack(); - if (needUnpack > 1) { - _decodeContext->skipFeatures(needUnpack - 1); + if (_decode_normal_features) { + uint32_t needUnpack = getNeedUnpack(); + if (needUnpack > 1) { + _decodeContext->skipFeatures(needUnpack - 1); + } + _decodeContext->unpackFeatures(_matchData, docId); + } else { + _matchData[0]->reset(docId); } - _decodeContext->unpackFeatures(_matchData, docId); if (_decode_cheap_features) { TermFieldMatchData *tfmd = _matchData[0]; tfmd->setFieldLength(_field_length); diff --git a/searchlib/src/vespa/searchlib/diskindex/zcpostingiterators.h b/searchlib/src/vespa/searchlib/diskindex/zcpostingiterators.h index a66c47f49fd..81e8d3a5671 100644 --- a/searchlib/src/vespa/searchlib/diskindex/zcpostingiterators.h +++ b/searchlib/src/vespa/searchlib/diskindex/zcpostingiterators.h @@ -68,11 +68,12 @@ public: unsigned int _residue; uint32_t _prevDocId; // Previous document id uint32_t _numDocs; // Documents in chunk or word + bool _decode_normal_features; bool _decode_cheap_features; uint32_t _field_length; uint32_t _num_occs; - ZcRareWordPostingIteratorBase(const fef::TermFieldMatchDataArray &matchData, Position start, uint32_t docIdLimit, bool decode_cheap_features); + ZcRareWordPostingIteratorBase(const fef::TermFieldMatchDataArray &matchData, Position start, uint32_t docIdLimit, bool decode_normal_features, bool decode_cheap_features); void doUnpack(uint32_t docId) override; void rewind(Position start) override; @@ -114,13 +115,14 @@ class ZcRareWordPostingIterator : public ZcRareWordPostingIteratorBase<bigEndian using ParentClass::setDocId; using ParentClass::setAtEnd; using ParentClass::_numDocs; + using ParentClass::_decode_normal_features; using ParentClass::_decode_cheap_features; using ParentClass::_field_length; using ParentClass::_num_occs; ZcPostingDocIdKParam<dynamic_k> _doc_id_k_param; public: using ParentClass::_decodeContext; - ZcRareWordPostingIterator(const fef::TermFieldMatchDataArray &matchData, Position start, uint32_t docIdLimit, bool decode_cheap_features); + ZcRareWordPostingIterator(const fef::TermFieldMatchDataArray &matchData, Position start, uint32_t docIdLimit, bool decode_normal_features, bool decode_cheap_features); void doSeek(uint32_t docId) override; void readWordStart(uint32_t docIdLimit) override; }; @@ -166,9 +168,11 @@ protected: void postSetup(const ZcPostingIteratorBase &l0) { _docIdPos = l0._valIBase; } - void decodeSkipEntry() { + void decodeSkipEntry(bool decode_normal_features) { ZCDECODE(_valI, _docIdPos += 1 +); - ZCDECODE(_valI, _skipFeaturePos += 1 +); + if (decode_normal_features) { + ZCDECODE(_valI, _skipFeaturePos += 1 +); + } } void nextDocId() { ZCDECODE(_valI, _skipDocId += 1 +); @@ -191,8 +195,8 @@ protected: _docIdPos = l1._docIdPos; _l1Pos = l1._valIBase; } - void decodeSkipEntry() { - L1Skip::decodeSkipEntry(); + void decodeSkipEntry(bool decode_normal_features) { + L1Skip::decodeSkipEntry(decode_normal_features); ZCDECODE(_valI, _l1Pos += 1 + ); } }; @@ -214,8 +218,8 @@ protected: _l1Pos = l2._l1Pos; _l2Pos = l2._valIBase; } - void decodeSkipEntry() { - L2Skip::decodeSkipEntry(); + void decodeSkipEntry(bool decode_normal_features) { + L2Skip::decodeSkipEntry(decode_normal_features); ZCDECODE(_valI, _l2Pos += 1 + ); } }; @@ -239,8 +243,8 @@ protected: _l3Pos = l3._valIBase; } - void decodeSkipEntry() { - L3Skip::decodeSkipEntry(); + void decodeSkipEntry(bool decode_normal_features) { + L3Skip::decodeSkipEntry(decode_normal_features); ZCDECODE(_valI, _l3Pos += 1 + ); } }; @@ -263,6 +267,7 @@ protected: ChunkSkip _chunk; uint64_t _featuresSize; bool _hasMore; + bool _decode_normal_features; bool _decode_cheap_features; uint32_t _chunkNo; uint32_t _field_length; @@ -285,7 +290,7 @@ protected: VESPA_DLL_LOCAL void doL1SkipSeek(uint32_t docId); void doSeek(uint32_t docId) override; public: - ZcPostingIteratorBase(const fef::TermFieldMatchDataArray &matchData, Position start, uint32_t docIdLimit, bool decode_cheap_features); + ZcPostingIteratorBase(const fef::TermFieldMatchDataArray &matchData, Position start, uint32_t docIdLimit, bool decode_normal_features, bool decode_cheap_features); }; template <bool bigEndian> @@ -312,7 +317,7 @@ public: const PostingListCounts &_counts; ZcPostingIterator(uint32_t minChunkDocs, bool dynamicK, const PostingListCounts &counts, - const search::fef::TermFieldMatchDataArray &matchData, Position start, uint32_t docIdLimit, bool decode_cheap_features); + const search::fef::TermFieldMatchDataArray &matchData, Position start, uint32_t docIdLimit, bool decode_normal_features, bool decode_cheap_features); void doUnpack(uint32_t docId) override; diff --git a/searchlib/src/vespa/searchlib/features/dotproductfeature.cpp b/searchlib/src/vespa/searchlib/features/dotproductfeature.cpp index fc71100dea6..c26694abcf6 100644 --- a/searchlib/src/vespa/searchlib/features/dotproductfeature.cpp +++ b/searchlib/src/vespa/searchlib/features/dotproductfeature.cpp @@ -354,8 +354,7 @@ size_t SparseDotProductByContentFillExecutor<BaseType>::getAttributeValues(uint3 DotProductBlueprint::DotProductBlueprint() : Blueprint("dotProduct"), _defaultAttribute(), - _queryVector(), - _attribute(nullptr) + _queryVector() { } DotProductBlueprint::~DotProductBlueprint() = default; @@ -572,8 +571,6 @@ createForDirectArray(const IAttributeVector * attribute, return createForDirectArrayImpl<A>(attribute, arguments.values, arguments.indexes, stash); } -const char * OBJECT = "object"; - FeatureExecutor & createFromObject(const IAttributeVector * attribute, const fef::Anything & object, vespalib::Stash &stash) { @@ -760,8 +757,23 @@ attemptParseArrayQueryVector(const IAttributeVector & attribute, const Property return std::unique_ptr<fef::Anything>(); } +vespalib::string +make_key(const vespalib::string & base, const vespalib::string & queryVector) { + vespalib::string key(base); + key.append('.'); + key.append(queryVector); + return key; +} + } // anon ns +DotProductBlueprint::SharedState::SharedState(const IAttributeVector * attribute, fef::Anything::UP arguments) + : _attribute(attribute), + _arguments(std::move(arguments)) +{} + +DotProductBlueprint::SharedState::~SharedState() = default; + const IAttributeVector * DotProductBlueprint::upgradeIfNecessary(const IAttributeVector * attribute, const IQueryEnvironment & env) const { if ((attribute->getCollectionType() == attribute::CollectionType::WSET) && @@ -776,8 +788,7 @@ DotProductBlueprint::upgradeIfNecessary(const IAttributeVector * attribute, cons void DotProductBlueprint::prepareSharedState(const IQueryEnvironment & env, IObjectStore & store) const { - _attribute = env.getAttributeContext().getAttribute(getAttribute(env)); - const IAttributeVector * attribute = _attribute; + const IAttributeVector * attribute = env.getAttributeContext().getAttribute(getAttribute(env)); if (attribute == nullptr) return; attribute = upgradeIfNecessary(attribute, env); @@ -816,24 +827,30 @@ DotProductBlueprint::prepareSharedState(const IQueryEnvironment & env, IObjectSt // TODO actually use the parsed output for wset operations! } } - if (arguments) { - store.add(getBaseName() + "." + _queryVector + "." + OBJECT, std::move(arguments)); - } + store.add(make_key(getBaseName(), _queryVector), std::make_unique<SharedState>(attribute, std::move(arguments))); } FeatureExecutor & DotProductBlueprint::createExecutor(const IQueryEnvironment & env, vespalib::Stash &stash) const { - const IAttributeVector * attribute = (_attribute != nullptr) ? _attribute : env.getAttributeContext().getAttribute(getAttribute(env)); + const IAttributeVector * attribute = nullptr; + const SharedState * sharedState = nullptr; + const fef::Anything * argument = env.getObjectStore().get(make_key(getBaseName(), _queryVector)); + if (argument != nullptr) { + sharedState = static_cast<const SharedState *>(argument); + attribute = sharedState->_attribute; + } + if (attribute == nullptr) { + attribute = env.getAttributeContext().getAttribute(getAttribute(env)); + } if (attribute == nullptr) { LOG(warning, "The attribute vector '%s' was not found in the attribute manager, returning executor with default value.", getAttribute(env).c_str()); return stash.create<SingleZeroValueExecutor>(); } attribute = upgradeIfNecessary(attribute, env); - const fef::Anything * argument = env.getObjectStore().get(getBaseName() + "." + _queryVector + "." + OBJECT); - if (argument != nullptr) { - return createFromObject(attribute, *argument, stash); + if ((sharedState != nullptr) && sharedState->_arguments) { + return createFromObject(attribute, *sharedState->_arguments, stash); } else { Property prop = env.getProperties().lookup(getBaseName(), _queryVector); if (prop.found() && !prop.get().empty()) { diff --git a/searchlib/src/vespa/searchlib/features/dotproductfeature.h b/searchlib/src/vespa/searchlib/features/dotproductfeature.h index e5c4073bd8f..77a78b131b0 100644 --- a/searchlib/src/vespa/searchlib/features/dotproductfeature.h +++ b/searchlib/src/vespa/searchlib/features/dotproductfeature.h @@ -304,12 +304,17 @@ private: vespalib::string _defaultAttribute; vespalib::string _queryVector; - mutable const IAttributeVector * _attribute; - vespalib::string getAttribute(const fef::IQueryEnvironment & env) const; const IAttributeVector * upgradeIfNecessary(const IAttributeVector * attribute, const fef::IQueryEnvironment & env) const; public: + class SharedState : public fef::Anything { + public: + SharedState(const IAttributeVector * attribute, fef::Anything::UP arguments); + ~SharedState() override; + const IAttributeVector * _attribute; + fef::Anything::UP _arguments; + }; DotProductBlueprint(); ~DotProductBlueprint() override; void visitDumpFeatures(const fef::IIndexEnvironment & env, fef::IDumpFeatureVisitor & visitor) const override; diff --git a/searchlib/src/vespa/searchlib/index/schemautil.h b/searchlib/src/vespa/searchlib/index/schemautil.h index c8fe8e4fe32..69b79ecfedd 100644 --- a/searchlib/src/vespa/searchlib/index/schemautil.h +++ b/searchlib/src/vespa/searchlib/index/schemautil.h @@ -83,6 +83,10 @@ public: return _schema.getIndexField(_index).getName(); } + bool use_experimental_posting_list_format() const { + return _schema.getIndexField(_index).use_experimental_posting_list_format(); + } + IndexIterator &operator++() { if (_index < _schema.getNumIndexFields()) { ++_index; diff --git a/searchlib/src/vespa/searchlib/test/fakedata/fakezcfilterocc.cpp b/searchlib/src/vespa/searchlib/test/fakedata/fakezcfilterocc.cpp index 31e2a323781..7b2b277d5e4 100644 --- a/searchlib/src/vespa/searchlib/test/fakedata/fakezcfilterocc.cpp +++ b/searchlib/src/vespa/searchlib/test/fakedata/fakezcfilterocc.cpp @@ -520,9 +520,9 @@ createIterator(const TermFieldMatchDataArray &matchData) const return new FakeFilterOccZCArrayIterator(_compressed.first, 0, _posting_params._doc_id_limit, matchData); } -template <bool doSkip> class FakeZcSkipFilterOcc : public FakeZcFilterOcc { + search::index::PostingListCounts _counts; public: FakeZcSkipFilterOcc(const FakeWord &fw); @@ -531,580 +531,31 @@ public: }; static FPFactoryInit -initNoSkip(std::make_pair("ZcNoSkipFilterOcc", - makeFPFactory<FPFactoryT<FakeZcSkipFilterOcc<false> > >)); - - -static FPFactoryInit initSkip(std::make_pair("ZcSkipFilterOcc", - makeFPFactory<FPFactoryT<FakeZcSkipFilterOcc<true> > >)); + makeFPFactory<FPFactoryT<FakeZcSkipFilterOcc>>)); -template<> -FakeZcSkipFilterOcc<false>::FakeZcSkipFilterOcc(const FakeWord &fw) - : FakeZcFilterOcc(fw, true, Zc4PostingParams(force_skip, disable_chunking, fw._docIdLimit, true, false, false), ".zc5noskipfilterocc") -{ - setup(fw); -} - - -template<> -FakeZcSkipFilterOcc<true>::FakeZcSkipFilterOcc(const FakeWord &fw) +FakeZcSkipFilterOcc::FakeZcSkipFilterOcc(const FakeWord &fw) : FakeZcFilterOcc(fw, true, Zc4PostingParams(force_skip, disable_chunking, fw._docIdLimit, true, false, false), ".zc5skipfilterocc") { setup(fw); + _counts._bitLength = _compressedBits; + _counts._numDocs = _hitDocs; } -template <bool doSkip> -FakeZcSkipFilterOcc<doSkip>::~FakeZcSkipFilterOcc() -{ -} - - -template <bool doSkip> -class FakeFilterOccZCSkipArrayIterator - : public queryeval::RankedSearchIteratorBase -{ -private: - - FakeFilterOccZCSkipArrayIterator(const FakeFilterOccZCSkipArrayIterator &other); - - FakeFilterOccZCSkipArrayIterator& - operator=(const FakeFilterOccZCSkipArrayIterator &other); - -public: - // Pointer to compressed data - const uint8_t *_valI; - uint32_t _lastDocId; - uint32_t _l1SkipDocId; - uint32_t _l2SkipDocId; - uint32_t _l3SkipDocId; - uint32_t _l4SkipDocId; - const uint8_t *_l1SkipDocIdPos; - const uint8_t *_l1SkipValI; - const uint8_t *_valIBase; - const uint8_t *_l1SkipValIBase; - const uint8_t *_l2SkipDocIdPos; - const uint8_t *_l2SkipValI; - const uint8_t *_l2SkipL1SkipPos; - const uint8_t *_l2SkipValIBase; - const uint8_t *_l3SkipDocIdPos; - const uint8_t *_l3SkipValI; - const uint8_t *_l3SkipL1SkipPos; - const uint8_t *_l3SkipL2SkipPos; - const uint8_t *_l3SkipValIBase; - const uint8_t *_l4SkipDocIdPos; - const uint8_t *_l4SkipValI; - const uint8_t *_l4SkipL1SkipPos; - const uint8_t *_l4SkipL2SkipPos; - const uint8_t *_l4SkipL3SkipPos; - - typedef search::bitcompression::FeatureDecodeContextBE DecodeContext; - typedef search::bitcompression::FeatureEncodeContextBE EncodeContext; - DecodeContext _decodeContext; - uint32_t _docIdLimit; - - FakeFilterOccZCSkipArrayIterator(const uint64_t *compressed, - int bitOffset, - uint32_t docIdLimit, - const TermFieldMatchDataArray &matchData); - - ~FakeFilterOccZCSkipArrayIterator() override; - - void doL4SkipSeek(uint32_t docId); - void doL3SkipSeek(uint32_t docId); - void doL2SkipSeek(uint32_t docId); - void doL1SkipSeek(uint32_t docId); - - void doUnpack(uint32_t docId) override; - void doSeek(uint32_t docId) override; - void initRange(uint32_t begin, uint32_t end) override; - Trinary is_strict() const override { return Trinary::True; } -}; - - -template <bool doSkip> -FakeFilterOccZCSkipArrayIterator<doSkip>:: -FakeFilterOccZCSkipArrayIterator(const uint64_t *compressed, - int bitOffset, - uint32_t docIdLimit, - const fef::TermFieldMatchDataArray &matchData) - : queryeval::RankedSearchIteratorBase(matchData), - _valI(NULL), - _lastDocId(0), - _l1SkipDocId(0), - _l2SkipDocId(0), - _l3SkipDocId(0), - _l4SkipDocId(0), - _l1SkipDocIdPos(NULL), - _l1SkipValI(NULL), - _valIBase(NULL), - _l1SkipValIBase(NULL), - _l2SkipDocIdPos(NULL), - _l2SkipValI(NULL), - _l2SkipL1SkipPos(NULL), - _l2SkipValIBase(NULL), - _l3SkipDocIdPos(NULL), - _l3SkipValI(NULL), - _l3SkipL1SkipPos(NULL), - _l3SkipL2SkipPos(NULL), - _l3SkipValIBase(NULL), - _l4SkipDocIdPos(NULL), - _l4SkipValI(NULL), - _l4SkipL1SkipPos(NULL), - _l4SkipL2SkipPos(NULL), - _l4SkipL3SkipPos(NULL), - _decodeContext(compressed, bitOffset), - _docIdLimit(docIdLimit) -{ -} - -template <bool doSkip> -void -FakeFilterOccZCSkipArrayIterator<doSkip>:: -initRange(uint32_t begin, uint32_t end) -{ - queryeval::RankedSearchIteratorBase::initRange(begin, end); - DecodeContext &d = _decodeContext; - Zc4PostingParams params(force_skip, disable_chunking, _docIdLimit, true, false, false); - Zc4PostingHeader header; - header.read(d, params); - _lastDocId = header._last_doc_id; - assert((d.getBitOffset() & 7) == 0); - const uint8_t *bcompr = d.getByteCompr(); - _valIBase = _valI = bcompr; - _l1SkipDocIdPos = _l2SkipDocIdPos = bcompr; - _l3SkipDocIdPos = _l4SkipDocIdPos = bcompr; - bcompr += header._doc_ids_size; - if (header._l1_skip_size != 0) { - _l1SkipValIBase = _l1SkipValI = bcompr; - _l2SkipL1SkipPos = _l3SkipL1SkipPos = _l4SkipL1SkipPos = bcompr; - bcompr += header._l1_skip_size; - } else { - _l1SkipValIBase = _l1SkipValI = NULL; - _l2SkipL1SkipPos = _l3SkipL1SkipPos = _l4SkipL1SkipPos = NULL; - } - if (header._l2_skip_size != 0) { - _l2SkipValIBase = _l2SkipValI = bcompr; - _l3SkipL2SkipPos = _l4SkipL2SkipPos = bcompr; - bcompr += header._l2_skip_size; - } else { - _l2SkipValIBase = _l2SkipValI = NULL; - _l3SkipL2SkipPos = _l4SkipL2SkipPos = NULL; - } - if (header._l3_skip_size != 0) { - _l3SkipValIBase = _l3SkipValI = bcompr; - _l4SkipL3SkipPos = bcompr; - bcompr += header._l3_skip_size; - } else { - _l3SkipValIBase = _l3SkipValI = NULL; - _l4SkipL3SkipPos = NULL; - } - if (header._l4_skip_size != 0) { - _l4SkipValI = bcompr; - bcompr += header._l4_skip_size; - } else { - _l4SkipValI = NULL; - } - d.setByteCompr(bcompr); - uint32_t oDocId; - ZCDECODE(_valI, oDocId = 1 +); -#if DEBUG_ZCFILTEROCC_PRINTF - printf("DecodeInit docId=%d\n", - oDocId); -#endif - setDocId(oDocId); - if (_l1SkipValI != NULL) { - ZCDECODE(_l1SkipValI, _l1SkipDocId = 1 +); - } else - _l1SkipDocId = _lastDocId; -#if DEBUG_ZCFILTEROCC_PRINTF - printf("L1DecodeInit docId=%d\n", - _l1SkipDocId); -#endif - if (_l2SkipValI != NULL) { - ZCDECODE(_l2SkipValI, _l2SkipDocId = 1 +); - } else - _l2SkipDocId = _lastDocId; -#if DEBUG_ZCFILTEROCC_PRINTF - printf("L2DecodeInit docId=%d\n", - _l2SkipDocId); -#endif - if (_l3SkipValI != NULL) { - ZCDECODE(_l3SkipValI, _l3SkipDocId = 1 +); - } else - _l3SkipDocId = _lastDocId; -#if DEBUG_ZCFILTEROCC_PRINTF - printf("L3DecodeInit docId=%d\n", - _l3SkipDocId); -#endif - if (_l4SkipValI != NULL) { - ZCDECODE(_l4SkipValI, _l4SkipDocId = 1 +); - } else - _l4SkipDocId = _lastDocId; -#if DEBUG_ZCFILTEROCC_PRINTF - printf("L4DecodeInit docId=%d\n", - _l4SkipDocId); -#endif - clearUnpacked(); -} - - -template <bool doSkip> -FakeFilterOccZCSkipArrayIterator<doSkip>:: -~FakeFilterOccZCSkipArrayIterator() -{ -} - - -template <> -void -FakeFilterOccZCSkipArrayIterator<true>::doL4SkipSeek(uint32_t docId) -{ - uint32_t lastL4SkipDocId; - - if (__builtin_expect(docId > _lastDocId, false)) { - _l4SkipDocId = _l3SkipDocId = _l2SkipDocId = _l1SkipDocId = search::endDocId; - setAtEnd(); - return; - } - do { - lastL4SkipDocId = _l4SkipDocId; - ZCDECODE(_l4SkipValI, _l4SkipDocIdPos += 1 +); - ZCDECODE(_l4SkipValI, _l4SkipL1SkipPos += 1 + ); - ZCDECODE(_l4SkipValI, _l4SkipL2SkipPos += 1 + ); - ZCDECODE(_l4SkipValI, _l4SkipL3SkipPos += 1 + ); - ZCDECODE(_l4SkipValI, _l4SkipDocId += 1 + ); -#if DEBUG_ZCFILTEROCC_PRINTF - printf("L4Decode docId %d, docIdPos %d," - " l1SkipPos %d, l2SkipPos %d, l3SkipPos %d, nextDocId %d\n", - lastL4SkipDocId, - (int) (_l4SkipDocIdPos - _valIBase), - (int) (_l4SkipL1SkipPos - _l1SkipValIBase), - (int) (_l4SkipL2SkipPos - _l2SkipValIBase), - (int) (_l4SkipL3SkipPos - _l3SkipValIBase), - _l4SkipDocId); -#endif - } while (docId > _l4SkipDocId); - _valI = _l1SkipDocIdPos = _l2SkipDocIdPos = _l3SkipDocIdPos = - _l4SkipDocIdPos; - _l1SkipDocId = _l2SkipDocId = _l3SkipDocId = lastL4SkipDocId; - _l1SkipValI = _l2SkipL1SkipPos = _l3SkipL1SkipPos = _l4SkipL1SkipPos; - _l2SkipValI = _l3SkipL2SkipPos = _l4SkipL2SkipPos; - _l3SkipValI = _l4SkipL3SkipPos; - ZCDECODE(_valI, lastL4SkipDocId += 1 +); - ZCDECODE(_l1SkipValI, _l1SkipDocId += 1 +); - ZCDECODE(_l2SkipValI, _l2SkipDocId += 1 +); - ZCDECODE(_l3SkipValI, _l3SkipDocId += 1 +); -#if DEBUG_ZCFILTEROCC_PRINTF - printf("L4Seek, docId %d docIdPos %d" - " L1SkipPos %d L2SkipPos %d L3SkipPos %d, nextDocId %d\n", - lastL4SkipDocId, - (int) (_l4SkipDocIdPos - _valIBase), - (int) (_l4SkipL1SkipPos - _l1SkipValIBase), - (int) (_l4SkipL2SkipPos - _l2SkipValIBase), - (int) (_l4SkipL3SkipPos - _l3SkipValIBase), - _l4SkipDocId); -#endif - setDocId(lastL4SkipDocId); -} - - -template <> -void -FakeFilterOccZCSkipArrayIterator<true>::doL3SkipSeek(uint32_t docId) -{ - uint32_t lastL3SkipDocId; - - if (__builtin_expect(docId > _l4SkipDocId, false)) { - doL4SkipSeek(docId); - if (docId <= _l3SkipDocId) { - return; - } - } - do { - lastL3SkipDocId = _l3SkipDocId; - ZCDECODE(_l3SkipValI, _l3SkipDocIdPos += 1 +); - ZCDECODE(_l3SkipValI, _l3SkipL1SkipPos += 1 + ); - ZCDECODE(_l3SkipValI, _l3SkipL2SkipPos += 1 + ); - ZCDECODE(_l3SkipValI, _l3SkipDocId += 1 + ); -#if DEBUG_ZCFILTEROCC_PRINTF - printf("L3Decode docId %d, docIdPos %d," - " l1SkipPos %d, l2SkipPos %d, nextDocId %d\n", - lastL3SkipDocId, - (int) (_l3SkipDocIdPos - _valIBase), - (int) (_l3SkipL1SkipPos - _l1SkipValIBase), - (int) (_l3SkipL2SkipPos - _l2SkipValIBase), - _l3SkipDocId); -#endif - } while (docId > _l3SkipDocId); - _valI = _l1SkipDocIdPos = _l2SkipDocIdPos = _l3SkipDocIdPos; - _l1SkipDocId = _l2SkipDocId = lastL3SkipDocId; - _l1SkipValI = _l2SkipL1SkipPos = _l3SkipL1SkipPos; - _l2SkipValI = _l3SkipL2SkipPos; - ZCDECODE(_valI, lastL3SkipDocId += 1 +); - ZCDECODE(_l1SkipValI, _l1SkipDocId += 1 +); - ZCDECODE(_l2SkipValI, _l2SkipDocId += 1 +); -#if DEBUG_ZCFILTEROCC_PRINTF - printf("L3Seek, docId %d docIdPos %d" - " L1SkipPos %d L2SkipPos %d, nextDocId %d\n", - lastL3SkipDocId, - (int) (_l3SkipDocIdPos - _valIBase), - (int) (_l3SkipL1SkipPos - _l1SkipValIBase), - (int) (_l3SkipL2SkipPos - _l2SkipValIBase), - _l3SkipDocId); -#endif - setDocId(lastL3SkipDocId); -} - - -template <> -void -FakeFilterOccZCSkipArrayIterator<true>::doL2SkipSeek(uint32_t docId) -{ - uint32_t lastL2SkipDocId; - - if (__builtin_expect(docId > _l3SkipDocId, false)) { - doL3SkipSeek(docId); - if (docId <= _l2SkipDocId) { - return; - } - } - do { - lastL2SkipDocId = _l2SkipDocId; - ZCDECODE(_l2SkipValI, _l2SkipDocIdPos += 1 +); - ZCDECODE(_l2SkipValI, _l2SkipL1SkipPos += 1 + ); - ZCDECODE(_l2SkipValI, _l2SkipDocId += 1 + ); -#if DEBUG_ZCFILTEROCC_PRINTF - printf("L2Decode docId %d, docIdPos %d, l1SkipPos %d, nextDocId %d\n", - lastL2SkipDocId, - (int) (_l2SkipDocIdPos - _valIBase), - (int) (_l2SkipL1SkipPos - _l1SkipValIBase), - _l2SkipDocId); -#endif - } while (docId > _l2SkipDocId); - _valI = _l1SkipDocIdPos = _l2SkipDocIdPos; - _l1SkipDocId = lastL2SkipDocId; - _l1SkipValI = _l2SkipL1SkipPos; - ZCDECODE(_valI, lastL2SkipDocId += 1 +); - ZCDECODE(_l1SkipValI, _l1SkipDocId += 1 +); -#if DEBUG_ZCFILTEROCC_PRINTF - printf("L2Seek, docId %d docIdPos %d L1SkipPos %d, nextDocId %d\n", - lastL2SkipDocId, - (int) (_l2SkipDocIdPos - _valIBase), - (int) (_l2SkipL1SkipPos - _l1SkipValIBase), - _l2SkipDocId); -#endif - setDocId(lastL2SkipDocId); -} - - -template <> -void -FakeFilterOccZCSkipArrayIterator<false>::doL1SkipSeek(uint32_t docId) -{ - (void) docId; -} - - -template <> -void -FakeFilterOccZCSkipArrayIterator<true>::doL1SkipSeek(uint32_t docId) -{ - uint32_t lastL1SkipDocId; - if (__builtin_expect(docId > _l2SkipDocId, false)) { - doL2SkipSeek(docId); - if (docId <= _l1SkipDocId) { - return; - } - } - do { - lastL1SkipDocId = _l1SkipDocId; - ZCDECODE(_l1SkipValI, _l1SkipDocIdPos += 1 +); - ZCDECODE(_l1SkipValI, _l1SkipDocId += 1 +); -#if DEBUG_ZCFILTEROCC_PRINTF - printf("L1Decode docId %d, docIdPos %d, L1SkipPos %d, nextDocId %d\n", - lastL1SkipDocId, - (int) (_l1SkipDocIdPos - _valIBase), - (int) (_l1SkipValI - _l1SkipValIBase), - _l1SkipDocId); -#endif - } while (docId > _l1SkipDocId); - _valI = _l1SkipDocIdPos; - ZCDECODE(_valI, lastL1SkipDocId += 1 +); - setDocId(lastL1SkipDocId); -#if DEBUG_ZCFILTEROCC_PRINTF - printf("L1SkipSeek, docId %d docIdPos %d, nextDocId %d\n", - lastL1SkipDocId, - (int) (_l1SkipDocIdPos - _valIBase), - _l1SkipDocId); -#endif -} - - -template <bool doSkip> -void -FakeFilterOccZCSkipArrayIterator<doSkip>::doSeek(uint32_t docId) -{ - if (getUnpacked()) { - clearUnpacked(); - } - if (doSkip && docId > _l1SkipDocId) { - doL1SkipSeek(docId); - } - uint32_t oDocId = getDocId(); - if (doSkip) { -#if DEBUG_ZCFILTEROCC_ASSERT - assert(oDocId <= _l1SkipDocId); - assert(docId <= _l1SkipDocId); - assert(oDocId <= _l2SkipDocId); - assert(docId <= _l2SkipDocId); - assert(oDocId <= _l3SkipDocId); - assert(docId <= _l3SkipDocId); - assert(oDocId <= _l4SkipDocId); - assert(docId <= _l4SkipDocId); -#endif - } - const uint8_t *oCompr = _valI; - while (__builtin_expect(oDocId < docId, true)) { - if (!doSkip) { - if (__builtin_expect(oDocId >= _lastDocId, false)) { -#if DEBUG_ZCFILTEROCC_ASSERT - assert(_l1SkipDocId == _lastDocId); - assert(_l2SkipDocId == _lastDocId); - assert(_l3SkipDocId == _lastDocId); - assert(_l4SkipDocId == _lastDocId); -#endif - oDocId = _l1SkipDocId = _l2SkipDocId = _l3SkipDocId = - _l4SkipDocId = search::endDocId; - break; - } - } - if (doSkip) { -#if DEBUG_ZCFILTEROCC_ASSERT - assert(oDocId <= _l1SkipDocId); - assert(oDocId <= _l2SkipDocId); - assert(oDocId <= _l3SkipDocId); - assert(oDocId <= _l4SkipDocId); -#endif - } else if (__builtin_expect(oDocId >= _l1SkipDocId, false)) { - // Validate L1 Skip information - assert(oDocId == _l1SkipDocId); - ZCDECODE(_l1SkipValI, _l1SkipDocIdPos += 1 +); - assert(oCompr == _l1SkipDocIdPos); - if (__builtin_expect(oDocId >= _l2SkipDocId, false)) { - // Validate L2 Skip information - assert(oDocId == _l2SkipDocId); - ZCDECODE(_l2SkipValI, _l2SkipDocIdPos += 1 +); - ZCDECODE(_l2SkipValI, _l2SkipL1SkipPos += 1 +); - assert(oCompr = _l2SkipDocIdPos); - assert(_l1SkipValI == _l2SkipL1SkipPos); - if (__builtin_expect(oDocId >= _l3SkipDocId, false)) { - // Validate L3 Skip information - assert(oDocId == _l3SkipDocId); - ZCDECODE(_l3SkipValI, _l3SkipDocIdPos += 1 +); - ZCDECODE(_l3SkipValI, _l3SkipL1SkipPos += 1 +); - ZCDECODE(_l3SkipValI, _l3SkipL2SkipPos += 1 +); - assert(oCompr = _l3SkipDocIdPos); - assert(_l1SkipValI == _l3SkipL1SkipPos); - assert(_l2SkipValI == _l3SkipL2SkipPos); - if (__builtin_expect(oDocId >= _l4SkipDocId, false)) { - // Validate L4 Skip information - assert(oDocId == _l4SkipDocId); - ZCDECODE(_l4SkipValI, _l4SkipDocIdPos += 1 +); - ZCDECODE(_l4SkipValI, _l4SkipL1SkipPos += 1 +); - ZCDECODE(_l4SkipValI, _l4SkipL2SkipPos += 1 +); - ZCDECODE(_l4SkipValI, _l4SkipL3SkipPos += 1 +); - assert(oCompr = _l4SkipDocIdPos); - assert(_l1SkipValI == _l4SkipL1SkipPos); - assert(_l2SkipValI == _l4SkipL2SkipPos); - assert(_l3SkipValI == _l4SkipL3SkipPos); - ZCDECODE(_l4SkipValI, _l4SkipDocId += 1 +); - assert(_l4SkipDocId <= _lastDocId); -#if DEBUG_ZCFILTEROCC_PRINTF - printf("L4DecodeV docId=%d docIdPos=%d" - " L1SkipPos=%d L2SkipPos %d L3SkipPos %d\n", - _l4SkipDocId, - (int) (_l4SkipDocIdPos - _valIBase), - (int) (_l4SkipL1SkipPos - _l1SkipValIBase), - (int) (_l4SkipL2SkipPos - _l2SkipValIBase), - (int) (_l4SkipL3SkipPos - _l3SkipValIBase)); -#endif - } - ZCDECODE(_l3SkipValI, _l3SkipDocId += 1 +); - assert(_l3SkipDocId <= _lastDocId); - assert(_l3SkipDocId <= _l4SkipDocId); -#if DEBUG_ZCFILTEROCC_PRINTF - printf("L3DecodeV docId=%d docIdPos=%d" - " L1SkipPos=%d L2SkipPos %d\n", - _l3SkipDocId, - (int) (_l3SkipDocIdPos - _valIBase), - (int) (_l3SkipL1SkipPos - _l1SkipValIBase), - (int) (_l3SkipL2SkipPos - _l2SkipValIBase)); -#endif - } - ZCDECODE(_l2SkipValI, _l2SkipDocId += 1 +); - assert(_l2SkipDocId <= _lastDocId); - assert(_l2SkipDocId <= _l4SkipDocId); - assert(_l2SkipDocId <= _l3SkipDocId); -#if DEBUG_ZCFILTEROCC_PRINTF - printf("L2DecodeV docId=%d docIdPos=%d L1SkipPos=%d\n", - _l2SkipDocId, - (int) (_l2SkipDocIdPos - _valIBase), - (int) (_l2SkipL1SkipPos - _l1SkipValIBase)); -#endif - } - ZCDECODE(_l1SkipValI, _l1SkipDocId += 1 +); - assert(_l1SkipDocId <= _lastDocId); - assert(_l1SkipDocId <= _l4SkipDocId); - assert(_l1SkipDocId <= _l3SkipDocId); - assert(_l1SkipDocId <= _l2SkipDocId); -#if DEBUG_ZCFILTEROCC_PRINTF - printf("L1DecodeV docId=%d, docIdPos=%d\n", - _l1SkipDocId, - (int) (_l1SkipDocIdPos - _valIBase)); -#endif - } - ZCDECODE(oCompr, oDocId += 1 +); -#if DEBUG_ZCFILTEROCC_PRINTF - printf("Decode docId=%d\n", - oDocId); -#endif - } - _valI = oCompr; - setDocId(oDocId); - return; -} - - -template <bool doSkip> -void -FakeFilterOccZCSkipArrayIterator<doSkip>::doUnpack(uint32_t docId) -{ - if (_matchData.size() != 1 || getUnpacked()) { - return; - } - assert(docId == getDocId()); - _matchData[0]->reset(docId); - setUnpacked(); -} - +FakeZcSkipFilterOcc::~FakeZcSkipFilterOcc() = default; -template <bool doSkip> SearchIterator * -FakeZcSkipFilterOcc<doSkip>:: -createIterator(const TermFieldMatchDataArray &matchData) const +FakeZcSkipFilterOcc::createIterator(const TermFieldMatchDataArray &matchData) const { - return new FakeFilterOccZCSkipArrayIterator<doSkip>(_compressed.first, - 0, - _posting_params._doc_id_limit, - matchData); + return create_zc_posocc_iterator(true, _counts, Position(_compressed.first, 0), _compressedBits, _posting_params, _fieldsParams, matchData).release(); } template <bool bigEndian> class FakeEGCompr64PosOcc : public FakeZcFilterOcc { + search::index::PostingListCounts _counts; public: FakeEGCompr64PosOcc(const FakeWord &fw); ~FakeEGCompr64PosOcc() override; @@ -1120,6 +571,8 @@ FakeEGCompr64PosOcc<bigEndian>::FakeEGCompr64PosOcc(const FakeWord &fw) bigEndian ? ".zcposoccbe" : ".zcposoccle") { setup(fw); + _counts._bitLength = _compressedBits; + _counts._numDocs = _hitDocs; } @@ -1149,14 +602,14 @@ SearchIterator * FakeEGCompr64PosOcc<bigEndian>:: createIterator(const TermFieldMatchDataArray &matchData) const { - return new ZcRareWordPosOccIterator<bigEndian, true>(Position(_compressed.first, 0), - _compressedBits, _posting_params._doc_id_limit, false, &_fieldsParams, matchData); + return create_zc_posocc_iterator(bigEndian, _counts, Position(_compressed.first, 0), _compressedBits, _posting_params, _fieldsParams, matchData).release(); } template <bool bigEndian> class FakeEG2Compr64PosOcc : public FakeZcFilterOcc { + search::index::PostingListCounts _counts; public: FakeEG2Compr64PosOcc(const FakeWord &fw); ~FakeEG2Compr64PosOcc() override; @@ -1172,6 +625,8 @@ FakeEG2Compr64PosOcc<bigEndian>::FakeEG2Compr64PosOcc(const FakeWord &fw) bigEndian ? ".zc4posoccbe" : ".zc4posoccle") { setup(fw); + _counts._bitLength = _compressedBits; + _counts._numDocs = _hitDocs; } @@ -1202,8 +657,7 @@ SearchIterator * FakeEG2Compr64PosOcc<bigEndian>:: createIterator(const TermFieldMatchDataArray &matchData) const { - return new ZcRareWordPosOccIterator<bigEndian, false>(Position(_compressed.first, 0), - _compressedBits, _posting_params._doc_id_limit, false, &_fieldsParams, matchData); + return create_zc_posocc_iterator(bigEndian, _counts, Position(_compressed.first, 0), _compressedBits, _posting_params, _fieldsParams, matchData).release(); } @@ -1260,11 +714,7 @@ SearchIterator * FakeZcSkipPosOcc<bigEndian>:: createIterator(const TermFieldMatchDataArray &matchData) const { - return new ZcPosOccIterator<bigEndian, true>(Position(_compressed.first, 0), _compressedBits, _posting_params._doc_id_limit, false, - static_cast<uint32_t>(-1), - _counts, - &_fieldsParams, - matchData); + return create_zc_posocc_iterator(bigEndian, _counts, Position(_compressed.first, 0), _compressedBits, _posting_params, _fieldsParams, matchData).release(); } @@ -1355,7 +805,7 @@ class FakeZc5NoSkipPosOccCf : public FakeZc4SkipPosOcc<bigEndian> public: FakeZc5NoSkipPosOccCf(const FakeWord &fw) : FakeZc4SkipPosOcc<bigEndian>(fw, Zc4PostingParams(disable_skip, disable_chunking, fw._docIdLimit, true, true, true), - (bigEndian ? ".zc5noskipposoccbe.cf" : "zc5noskipposoccle.cf")) + (bigEndian ? ".zc5noskipposoccbe.cf" : ".zc5noskipposoccle.cf")) { } }; diff --git a/storage/src/tests/distributor/CMakeLists.txt b/storage/src/tests/distributor/CMakeLists.txt index fdccf9b1394..245d54e8e69 100644 --- a/storage/src/tests/distributor/CMakeLists.txt +++ b/storage/src/tests/distributor/CMakeLists.txt @@ -24,7 +24,6 @@ vespa_add_library(storage_testdistributor TEST ownership_transfer_safe_time_point_calculator_test.cpp pendingmessagetrackertest.cpp persistence_metrics_set_test.cpp - putoperationtest.cpp removebucketoperationtest.cpp removelocationtest.cpp removeoperationtest.cpp @@ -50,6 +49,7 @@ vespa_add_library(storage_gtestdistributor TEST bucketdatabasetest.cpp bucketdbupdatertest.cpp mapbucketdatabasetest.cpp + putoperationtest.cpp # Fixture etc. dupes with non-gtest runner : distributortestutil.cpp bucket_db_prune_elision_test.cpp diff --git a/storage/src/tests/distributor/bucketdbupdatertest.cpp b/storage/src/tests/distributor/bucketdbupdatertest.cpp index 9d063c82c69..1cfc1692edb 100644 --- a/storage/src/tests/distributor/bucketdbupdatertest.cpp +++ b/storage/src/tests/distributor/bucketdbupdatertest.cpp @@ -2596,7 +2596,7 @@ TEST_F(BucketDBUpdaterTest, DISABLED_benchmark_bulk_loading_into_empty_db) { ASSERT_EQ(_bucketSpaces.size(), _sender.commands.size()); for (uint32_t bsi = 0; bsi < _bucketSpaces.size(); ++bsi) { ASSERT_EQ(_sender.commands[bsi]->getType(), MessageType::REQUESTBUCKETINFO); - const auto& req = dynamic_cast<const RequestBucketInfoCommand &>(*_sender.commands[bsi]); + const auto& req = dynamic_cast<const RequestBucketInfoCommand&>(*_sender.commands[bsi]); auto sreply = std::make_shared<RequestBucketInfoReply>(req); sreply->setAddress(storageAddress(0)); @@ -2605,7 +2605,7 @@ TEST_F(BucketDBUpdaterTest, DISABLED_benchmark_bulk_loading_into_empty_db) { for (uint32_t sb = 0; sb < superbuckets; ++sb) { for (uint64_t i = 0; i < sub_buckets; ++i) { document::BucketId bucket(48, (i << 32ULL) | sb); - vec.push_back(api::RequestBucketInfoReply::Entry(bucket, api::BucketInfo(10,1,1))); + vec.push_back(api::RequestBucketInfoReply::Entry(bucket, api::BucketInfo(10, 1, 1))); } } } @@ -2626,4 +2626,29 @@ TEST_F(BucketDBUpdaterTest, DISABLED_benchmark_bulk_loading_into_empty_db) { EXPECT_EQ(size_t(0), mutable_global_db().size()); } +TEST_F(BucketDBUpdaterTest, pending_cluster_state_getter_is_non_null_only_when_state_is_pending) { + auto initial_baseline = std::make_shared<lib::ClusterState>("distributor:1 storage:2 .0.s:d"); + auto initial_default = std::make_shared<lib::ClusterState>("distributor:1 storage:2 .0.s:m"); + + lib::ClusterStateBundle initial_bundle(*initial_baseline, {{FixedBucketSpaces::default_space(), initial_default}, + {FixedBucketSpaces::global_space(), initial_baseline}}); + set_cluster_state_bundle(initial_bundle); + + auto* state = getBucketDBUpdater().pendingClusterStateOrNull(FixedBucketSpaces::default_space()); + ASSERT_TRUE(state != nullptr); + EXPECT_EQ(*initial_default, *state); + + state = getBucketDBUpdater().pendingClusterStateOrNull(FixedBucketSpaces::global_space()); + ASSERT_TRUE(state != nullptr); + EXPECT_EQ(*initial_baseline, *state); + + ASSERT_NO_FATAL_FAILURE(completeBucketInfoGathering(*initial_baseline, messageCount(1), 0)); + + state = getBucketDBUpdater().pendingClusterStateOrNull(FixedBucketSpaces::default_space()); + EXPECT_TRUE(state == nullptr); + + state = getBucketDBUpdater().pendingClusterStateOrNull(FixedBucketSpaces::global_space()); + EXPECT_TRUE(state == nullptr); +} + } diff --git a/storage/src/tests/distributor/putoperationtest.cpp b/storage/src/tests/distributor/putoperationtest.cpp index aa5fd80df91..66ef13310a4 100644 --- a/storage/src/tests/distributor/putoperationtest.cpp +++ b/storage/src/tests/distributor/putoperationtest.cpp @@ -1,7 +1,5 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/config/helper/configgetter.h> -#include <vespa/document/config/config-documenttypes.h> #include <vespa/document/repo/documenttyperepo.h> #include <vespa/storage/distributor/operations/external/putoperation.h> #include <vespa/storage/distributor/distributor.h> @@ -12,9 +10,8 @@ #include <tests/common/dummystoragelink.h> #include <vespa/document/test/make_document_bucket.h> #include <vespa/vdstestlib/cppunit/macros.h> +#include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/text/stringtokenizer.h> -#include <vespa/vespalib/testkit/test_kit.h> -#include <vespa/config/helper/configgetter.hpp> #include <iomanip> using std::shared_ptr; @@ -29,79 +26,26 @@ using namespace storage::lib; using namespace std::literals::string_literals; using document::test::makeDocumentBucket; -namespace storage { - -namespace distributor { - -class PutOperationTest : public CppUnit::TestFixture, - public DistributorTestUtil { - CPPUNIT_TEST_SUITE(PutOperationTest); - CPPUNIT_TEST(testSimple); - CPPUNIT_TEST(testBucketDatabaseGetsSpecialEntryWhenCreateBucketSent); - CPPUNIT_TEST(testSendInlineSplitBeforePutIfBucketTooLarge); - CPPUNIT_TEST(testDoNotSendInlineSplitIfNotConfigured); - CPPUNIT_TEST(testNodeRemovedOnReply); - CPPUNIT_TEST(testDoNotSendCreateBucketIfAlreadyPending); - CPPUNIT_TEST(testMultipleCopies); - CPPUNIT_TEST(testMultipleCopiesEarlyReturnPrimaryNotRequired); - CPPUNIT_TEST(testMultipleCopiesEarlyReturnPrimaryRequired); - CPPUNIT_TEST(testMultipleCopiesEarlyReturnPrimaryRequiredNotDone); - CPPUNIT_TEST_IGNORED(testDoNotRevertOnFailureAfterEarlyReturn); - CPPUNIT_TEST(testStorageFailed); - CPPUNIT_TEST(testRevertSuccessfulCopiesWhenOneFails); - CPPUNIT_TEST(testNoRevertIfRevertDisabled); - CPPUNIT_TEST(testNoStorageNodes); - CPPUNIT_TEST(testUpdateCorrectBucketOnRemappedPut); - CPPUNIT_TEST(testTargetNodes); - CPPUNIT_TEST(testDoNotResurrectDownedNodesInBucketDB); - CPPUNIT_TEST(sendToRetiredNodesIfNoUpNodesAvailable); - CPPUNIT_TEST(replicaImplicitlyActivatedWhenActivationIsNotDisabled); - CPPUNIT_TEST(replicaNotImplicitlyActivatedWhenActivationIsDisabled); - CPPUNIT_TEST_SUITE_END(); - - std::shared_ptr<const DocumentTypeRepo> _repo; - const DocumentType* _html_type; - std::unique_ptr<Operation> op; +using namespace ::testing; -protected: - void testSimple(); - void testBucketDatabaseGetsSpecialEntryWhenCreateBucketSent(); - void testSendInlineSplitBeforePutIfBucketTooLarge(); - void testDoNotSendInlineSplitIfNotConfigured(); - void testNodeRemovedOnReply(); - void testDoNotSendCreateBucketIfAlreadyPending(); - void testStorageFailed(); - void testNoReply(); - void testMultipleCopies(); - void testRevertSuccessfulCopiesWhenOneFails(); - void testNoRevertIfRevertDisabled(); - void testInconsistentChecksum(); - void testNoStorageNodes(); - void testMultipleCopiesEarlyReturnPrimaryNotRequired(); - void testMultipleCopiesEarlyReturnPrimaryRequired(); - void testMultipleCopiesEarlyReturnPrimaryRequiredNotDone(); - void testDoNotRevertOnFailureAfterEarlyReturn(); - void testUpdateCorrectBucketOnRemappedPut(); - void testBucketNotFound(); - void testTargetNodes(); - void testDoNotResurrectDownedNodesInBucketDB(); - void sendToRetiredNodesIfNoUpNodesAvailable(); - void replicaImplicitlyActivatedWhenActivationIsNotDisabled(); - void replicaNotImplicitlyActivatedWhenActivationIsDisabled(); - - void doTestCreationWithBucketActivationDisabled(bool disabled); +namespace storage::distributor { +class PutOperationTest : public Test, + public DistributorTestUtil +{ public: - void setUp() override { - _repo.reset( - new DocumentTypeRepo(*ConfigGetter<DocumenttypesConfig> - ::getConfig("config-doctypes", - FileSpec(TEST_PATH("config-doctypes.cfg"))))); - _html_type = _repo->getDocumentType("text/html"); + document::TestDocMan _testDocMan; + std::unique_ptr<Operation> op; + + ~PutOperationTest() override; + + void do_test_creation_with_bucket_activation_disabled(bool disabled); + + void SetUp() override { createLinks(); }; - void tearDown() override { + void TearDown() override { close(); } @@ -113,7 +57,7 @@ public: = api::ReturnCode::OK, api::BucketInfo info = api::BucketInfo(1,2,3,4,5)) { - CPPUNIT_ASSERT(!_sender.commands.empty()); + ASSERT_FALSE(_sender.commands.empty()); if (idx == -1) { idx = _sender.commands.size() - 1; } else if (static_cast<size_t>(idx) >= _sender.commands.size()) { @@ -138,30 +82,25 @@ public: op->start(_sender, framework::MilliSecTime(0)); } - Document::SP createDummyDocument(const char* ns, - const char* id) const - { - return Document::SP( - new Document(*_html_type, - DocumentId(DocIdString(ns, id)))); + const document::DocumentType& doc_type() const { + return *_testDocMan.getTypeRepo().getDocumentType("testdoctype1"); + } + Document::SP createDummyDocument(const char* ns, const char* id) const { + return std::make_shared<Document>(doc_type(), DocumentId(DocIdString(ns, id))); } - std::shared_ptr<api::PutCommand> createPut( - const Document::SP doc) const - { - return std::shared_ptr<api::PutCommand>( - new api::PutCommand(makeDocumentBucket(document::BucketId(0)), doc, 100)); + std::shared_ptr<api::PutCommand> createPut(Document::SP doc) const { + return std::make_shared<api::PutCommand>(makeDocumentBucket(document::BucketId(0)), std::move(doc), 100); } }; -CPPUNIT_TEST_SUITE_REGISTRATION(PutOperationTest); +PutOperationTest::~PutOperationTest() = default; document::BucketId PutOperationTest::createAndSendSampleDocument(uint32_t timeout) { Document::SP - doc(new Document(*_html_type, - DocumentId(DocIdString("test", "test")))); + doc(new Document(doc_type(), DocumentId(DocIdString("test", "test")))); document::BucketId id = getExternalOperationHandler().getBucketId(doc->getId()); addIdealNodes(id); @@ -186,44 +125,36 @@ typedef bool RequirePrimaryWritten; } -void -PutOperationTest::testSimple() -{ +TEST_F(PutOperationTest, simple) { setupDistributor(1, 1, "storage:1 distributor:1"); createAndSendSampleDocument(180); - CPPUNIT_ASSERT_EQUAL(std::string("Put(BucketId(0x4000000000008b13), " - "doc:test:test, timestamp 100, size 33) => 0"), - _sender.getCommands(true, true)); + ASSERT_EQ("Put(BucketId(0x4000000000008b13), " + "doc:test:test, timestamp 100, size 36) => 0", + _sender.getCommands(true, true)); sendReply(); - CPPUNIT_ASSERT_EQUAL(std::string("PutReply(doc:test:test, BucketId(0x0000000000000000), " - "timestamp 100) ReturnCode(NONE)"), - _sender.getLastReply()); + ASSERT_EQ("PutReply(doc:test:test, BucketId(0x0000000000000000), " + "timestamp 100) ReturnCode(NONE)", + _sender.getLastReply()); } -void -PutOperationTest::testBucketDatabaseGetsSpecialEntryWhenCreateBucketSent() -{ +TEST_F(PutOperationTest, bucket_database_gets_special_entry_when_CreateBucket_sent) { setupDistributor(2, 1, "storage:1 distributor:1"); Document::SP doc(createDummyDocument("test", "test")); sendPut(createPut(doc)); // Database updated before CreateBucket is sent - CPPUNIT_ASSERT_EQUAL( - std::string("BucketId(0x4000000000008b13) : " - "node(idx=0,crc=0x1,docs=0/0,bytes=0/0,trusted=true,active=true,ready=false)"), - dumpBucket(getExternalOperationHandler().getBucketId(doc->getId()))); + ASSERT_EQ("BucketId(0x4000000000008b13) : " + "node(idx=0,crc=0x1,docs=0/0,bytes=0/0,trusted=true,active=true,ready=false)", + dumpBucket(getExternalOperationHandler().getBucketId(doc->getId()))); - CPPUNIT_ASSERT_EQUAL(std::string("Create bucket => 0,Put => 0"), - _sender.getCommands(true)); + ASSERT_EQ("Create bucket => 0,Put => 0", _sender.getCommands(true)); } -void -PutOperationTest::testSendInlineSplitBeforePutIfBucketTooLarge() -{ +TEST_F(PutOperationTest, send_inline_split_before_put_if_bucket_too_large) { setupDistributor(1, 1, "storage:1 distributor:1"); getConfig().setSplitCount(1024); getConfig().setSplitSize(1000000); @@ -232,19 +163,16 @@ PutOperationTest::testSendInlineSplitBeforePutIfBucketTooLarge() sendPut(createPut(createDummyDocument("test", "uri"))); - CPPUNIT_ASSERT_EQUAL( - std::string("SplitBucketCommand(BucketId(0x4000000000002a52)Max doc count: " - "1024, Max total doc size: 1000000) Reasons to start: " - "[Splitting bucket because its maximum size (10000 b, 10000 docs, 10000 meta, 10000 b total) is " - "higher than the configured limit of (1000000, 1024)] => 0," - "Put(BucketId(0x4000000000002a52), doc:test:uri, timestamp 100, " - "size 32) => 0"), - _sender.getCommands(true, true)); + ASSERT_EQ("SplitBucketCommand(BucketId(0x4000000000002a52)Max doc count: " + "1024, Max total doc size: 1000000) Reasons to start: " + "[Splitting bucket because its maximum size (10000 b, 10000 docs, 10000 meta, 10000 b total) is " + "higher than the configured limit of (1000000, 1024)] => 0," + "Put(BucketId(0x4000000000002a52), doc:test:uri, timestamp 100, " + "size 35) => 0", + _sender.getCommands(true, true)); } -void -PutOperationTest::testDoNotSendInlineSplitIfNotConfigured() -{ +TEST_F(PutOperationTest, do_not_send_inline_split_if_not_configured) { setupDistributor(1, 1, "storage:1 distributor:1"); getConfig().setSplitCount(1024); getConfig().setDoInlineSplit(false); @@ -253,94 +181,79 @@ PutOperationTest::testDoNotSendInlineSplitIfNotConfigured() sendPut(createPut(createDummyDocument("test", "uri"))); - CPPUNIT_ASSERT_EQUAL( - std::string( - "Put(BucketId(0x4000000000002a52), doc:test:uri, timestamp 100, " - "size 32) => 0"), - _sender.getCommands(true, true)); + ASSERT_EQ("Put(BucketId(0x4000000000002a52), doc:test:uri, timestamp 100, " + "size 35) => 0", + _sender.getCommands(true, true)); } -void -PutOperationTest::testNodeRemovedOnReply() -{ +TEST_F(PutOperationTest, node_removed_on_reply) { setupDistributor(2, 2, "storage:2 distributor:1"); createAndSendSampleDocument(180); - CPPUNIT_ASSERT_EQUAL( - std::string("Put(BucketId(0x4000000000008b13), " - "doc:test:test, timestamp 100, size 33) => 1," - "Put(BucketId(0x4000000000008b13), " - "doc:test:test, timestamp 100, size 33) => 0"), - _sender.getCommands(true, true)); + ASSERT_EQ("Put(BucketId(0x4000000000008b13), " + "doc:test:test, timestamp 100, size 36) => 1," + "Put(BucketId(0x4000000000008b13), " + "doc:test:test, timestamp 100, size 36) => 0", + _sender.getCommands(true, true)); getExternalOperationHandler().removeNodeFromDB(makeDocumentBucket(document::BucketId(16, 0x8b13)), 0); sendReply(0); sendReply(1); - CPPUNIT_ASSERT_EQUAL(std::string( - "PutReply(doc:test:test, BucketId(0x0000000000000000), " - "timestamp 100) ReturnCode(BUCKET_DELETED, " - "Bucket(BucketSpace(0x0000000000000001), BucketId(0x4000000000008b13)) was deleted from nodes [0] " - "after message was sent but before it was done. " - "Sent to [1,0])"), - _sender.getLastReply()); + ASSERT_EQ("PutReply(doc:test:test, BucketId(0x0000000000000000), " + "timestamp 100) ReturnCode(BUCKET_DELETED, " + "Bucket(BucketSpace(0x0000000000000001), BucketId(0x4000000000008b13)) was deleted from nodes [0] " + "after message was sent but before it was done. " + "Sent to [1,0])", + _sender.getLastReply()); } -void -PutOperationTest::testStorageFailed() -{ +TEST_F(PutOperationTest, storage_failed) { setupDistributor(2, 1, "storage:1 distributor:1"); createAndSendSampleDocument(180); sendReply(-1, api::ReturnCode::INTERNAL_FAILURE); - CPPUNIT_ASSERT_EQUAL(std::string("PutReply(doc:test:test, BucketId(0x0000000000000000), " - "timestamp 100) ReturnCode(INTERNAL_FAILURE)"), - _sender.getLastReply(true)); + ASSERT_EQ("PutReply(doc:test:test, BucketId(0x0000000000000000), " + "timestamp 100) ReturnCode(INTERNAL_FAILURE)", + _sender.getLastReply(true)); } -void -PutOperationTest::testMultipleCopies() -{ +TEST_F(PutOperationTest, multiple_copies) { setupDistributor(3, 4, "storage:4 distributor:1"); Document::SP doc(createDummyDocument("test", "test")); sendPut(createPut(doc)); - CPPUNIT_ASSERT_EQUAL(std::string("Create bucket => 3,Create bucket => 1," - "Create bucket => 0,Put => 3,Put => 1,Put => 0"), - _sender.getCommands(true)); + ASSERT_EQ("Create bucket => 3,Create bucket => 1," + "Create bucket => 0,Put => 3,Put => 1,Put => 0", + _sender.getCommands(true)); for (uint32_t i = 0; i < 6; i++) { sendReply(i); } - CPPUNIT_ASSERT_EQUAL( - std::string("PutReply(doc:test:test, BucketId(0x0000000000000000), " - "timestamp 100) ReturnCode(NONE)"), - _sender.getLastReply(true)); - - CPPUNIT_ASSERT_EQUAL( - std::string("BucketId(0x4000000000008b13) : " - "node(idx=3,crc=0x1,docs=2/4,bytes=3/5,trusted=true,active=false,ready=false), " - "node(idx=1,crc=0x1,docs=2/4,bytes=3/5,trusted=true,active=false,ready=false), " - "node(idx=0,crc=0x1,docs=2/4,bytes=3/5,trusted=true,active=false,ready=false)"), - dumpBucket(getExternalOperationHandler().getBucketId(doc->getId()))); -} + ASSERT_EQ("PutReply(doc:test:test, BucketId(0x0000000000000000), " + "timestamp 100) ReturnCode(NONE)", + _sender.getLastReply(true)); + ASSERT_EQ("BucketId(0x4000000000008b13) : " + "node(idx=3,crc=0x1,docs=2/4,bytes=3/5,trusted=true,active=false,ready=false), " + "node(idx=1,crc=0x1,docs=2/4,bytes=3/5,trusted=true,active=false,ready=false), " + "node(idx=0,crc=0x1,docs=2/4,bytes=3/5,trusted=true,active=false,ready=false)", + dumpBucket(getExternalOperationHandler().getBucketId(doc->getId()))); +} -void -PutOperationTest::testMultipleCopiesEarlyReturnPrimaryRequired() -{ +TEST_F(PutOperationTest, multiple_copies_early_return_primary_required) { setupDistributor(3, 4, "storage:4 distributor:1", 2, true); sendPut(createPut(createDummyDocument("test", "test"))); - CPPUNIT_ASSERT_EQUAL(std::string("Create bucket => 3,Create bucket => 1," - "Create bucket => 0,Put => 3,Put => 1,Put => 0"), - _sender.getCommands(true)); + ASSERT_EQ("Create bucket => 3,Create bucket => 1," + "Create bucket => 0,Put => 3,Put => 1,Put => 0", + _sender.getCommands(true)); // Reply to 2 CreateBucket, including primary for (uint32_t i = 0; i < 2; i++) { @@ -351,23 +264,19 @@ PutOperationTest::testMultipleCopiesEarlyReturnPrimaryRequired() sendReply(3 + i); } - CPPUNIT_ASSERT_EQUAL( - std::string( - "PutReply(doc:test:test, BucketId(0x0000000000000000), " - "timestamp 100) ReturnCode(NONE)"), - _sender.getLastReply()); + ASSERT_EQ("PutReply(doc:test:test, BucketId(0x0000000000000000), " + "timestamp 100) ReturnCode(NONE)", + _sender.getLastReply()); } -void -PutOperationTest::testMultipleCopiesEarlyReturnPrimaryNotRequired() -{ +TEST_F(PutOperationTest, multiple_copies_early_return_primary_not_required) { setupDistributor(3, 4, "storage:4 distributor:1", 2, false); sendPut(createPut(createDummyDocument("test", "test"))); - CPPUNIT_ASSERT_EQUAL(std::string("Create bucket => 3,Create bucket => 1," - "Create bucket => 0,Put => 3,Put => 1,Put => 0"), - _sender.getCommands(true)); + ASSERT_EQ("Create bucket => 3,Create bucket => 1," + "Create bucket => 0,Put => 3,Put => 1,Put => 0", + _sender.getCommands(true)); // Reply only to 2 nodes (but not the primary) for (uint32_t i = 1; i < 3; i++) { @@ -377,22 +286,19 @@ PutOperationTest::testMultipleCopiesEarlyReturnPrimaryNotRequired() sendReply(3 + i); // Put } - CPPUNIT_ASSERT_EQUAL( - std::string("PutReply(doc:test:test, BucketId(0x0000000000000000), " - "timestamp 100) ReturnCode(NONE)"), - _sender.getLastReply()); + ASSERT_EQ("PutReply(doc:test:test, BucketId(0x0000000000000000), " + "timestamp 100) ReturnCode(NONE)", + _sender.getLastReply()); } -void -PutOperationTest::testMultipleCopiesEarlyReturnPrimaryRequiredNotDone() -{ +TEST_F(PutOperationTest, multiple_copies_early_return_primary_required_not_done) { setupDistributor(3, 4, "storage:4 distributor:1", 2, true); sendPut(createPut(createDummyDocument("test", "test"))); - CPPUNIT_ASSERT_EQUAL(std::string("Create bucket => 3,Create bucket => 1," - "Create bucket => 0,Put => 3,Put => 1,Put => 0"), - _sender.getCommands(true)); + ASSERT_EQ("Create bucket => 3,Create bucket => 1," + "Create bucket => 0,Put => 3,Put => 1,Put => 0", + _sender.getCommands(true)); // Reply only to 2 nodes (but not the primary) sendReply(1); @@ -400,20 +306,18 @@ PutOperationTest::testMultipleCopiesEarlyReturnPrimaryRequiredNotDone() sendReply(4); sendReply(5); - CPPUNIT_ASSERT_EQUAL(0, (int)_sender.replies.size()); + ASSERT_EQ(0, _sender.replies.size()); } -void -PutOperationTest::testDoNotRevertOnFailureAfterEarlyReturn() -{ +TEST_F(PutOperationTest, do_not_revert_on_failure_after_early_return) { setupDistributor(Redundancy(3),NodeCount(4), "storage:4 distributor:1", ReturnAfter(2), RequirePrimaryWritten(false)); sendPut(createPut(createDummyDocument("test", "test"))); - CPPUNIT_ASSERT_EQUAL(std::string("Create bucket => 3,Create bucket => 1," - "Create bucket => 0,Put => 3,Put => 1,Put => 0"), - _sender.getCommands(true)); + ASSERT_EQ("Create bucket => 3,Create bucket => 1," + "Create bucket => 0,Put => 3,Put => 1,Put => 0", + _sender.getCommands(true)); for (uint32_t i = 0; i < 3; i++) { sendReply(i); // CreateBucket @@ -422,28 +326,23 @@ PutOperationTest::testDoNotRevertOnFailureAfterEarlyReturn() sendReply(3 + i); // Put } - CPPUNIT_ASSERT_EQUAL( - std::string( - "PutReply(doc:test:test, BucketId(0x0000000000000000), " - "timestamp 100) ReturnCode(NONE)"), - _sender.getLastReply()); + ASSERT_EQ("PutReply(doc:test:test, BucketId(0x0000000000000000), " + "timestamp 100) ReturnCode(NONE)", + _sender.getLastReply()); sendReply(5, api::ReturnCode::INTERNAL_FAILURE); // Should not be any revert commands sent - CPPUNIT_ASSERT_EQUAL(std::string("Create bucket => 3,Create bucket => 1," - "Create bucket => 0,Put => 3,Put => 1,Put => 0"), - _sender.getCommands(true)); + ASSERT_EQ("Create bucket => 3,Create bucket => 1," + "Create bucket => 0,Put => 3,Put => 1,Put => 0", + _sender.getCommands(true)); } -void -PutOperationTest::testRevertSuccessfulCopiesWhenOneFails() -{ +TEST_F(PutOperationTest, revert_successful_copies_when_one_fails) { setupDistributor(3, 4, "storage:4 distributor:1"); createAndSendSampleDocument(180); - CPPUNIT_ASSERT_EQUAL(std::string("Put => 3,Put => 1,Put => 0"), - _sender.getCommands(true)); + ASSERT_EQ("Put => 3,Put => 1,Put => 0", _sender.getCommands(true)); for (uint32_t i = 0; i < 2; i++) { sendReply(i); @@ -451,28 +350,24 @@ PutOperationTest::testRevertSuccessfulCopiesWhenOneFails() sendReply(2, api::ReturnCode::INTERNAL_FAILURE); - CPPUNIT_ASSERT_EQUAL(std::string("PutReply(doc:test:test, " - "BucketId(0x0000000000000000), timestamp 100) " - "ReturnCode(INTERNAL_FAILURE)"), - _sender.getLastReply(true)); + ASSERT_EQ("PutReply(doc:test:test, " + "BucketId(0x0000000000000000), timestamp 100) " + "ReturnCode(INTERNAL_FAILURE)", + _sender.getLastReply(true)); - CPPUNIT_ASSERT_EQUAL(std::string("Revert => 3,Revert => 1"), - _sender.getCommands(true, false, 3)); + ASSERT_EQ("Revert => 3,Revert => 1", _sender.getCommands(true, false, 3)); } -void -PutOperationTest::testNoRevertIfRevertDisabled() -{ +TEST_F(PutOperationTest, no_revert_if_revert_disabled) { close(); getDirConfig().getConfig("stor-distributormanager") .set("enable_revert", "false"); - setUp(); + SetUp(); setupDistributor(3, 4, "storage:4 distributor:1"); createAndSendSampleDocument(180); - CPPUNIT_ASSERT_EQUAL(std::string("Put => 3,Put => 1,Put => 0"), - _sender.getCommands(true)); + ASSERT_EQ("Put => 3,Put => 1,Put => 0", _sender.getCommands(true)); for (uint32_t i = 0; i < 2; i++) { sendReply(i); @@ -480,26 +375,23 @@ PutOperationTest::testNoRevertIfRevertDisabled() sendReply(2, api::ReturnCode::INTERNAL_FAILURE); - CPPUNIT_ASSERT_EQUAL(std::string("PutReply(doc:test:test, " - "BucketId(0x0000000000000000), timestamp 100) " - "ReturnCode(INTERNAL_FAILURE)"), - _sender.getLastReply(true)); + ASSERT_EQ("PutReply(doc:test:test, " + "BucketId(0x0000000000000000), timestamp 100) " + "ReturnCode(INTERNAL_FAILURE)", + _sender.getLastReply(true)); - CPPUNIT_ASSERT_EQUAL(std::string(""), - _sender.getCommands(true, false, 3)); + ASSERT_EQ("", _sender.getCommands(true, false, 3)); } -void -PutOperationTest::testDoNotSendCreateBucketIfAlreadyPending() -{ +TEST_F(PutOperationTest, do_not_send_CreateBucket_if_already_pending) { setupDistributor(2, 2, "storage:2 distributor:1"); Document::SP doc(createDummyDocument("test", "uri")); sendPut(createPut(doc)); - CPPUNIT_ASSERT_EQUAL(std::string("Create bucket => 1,Create bucket => 0," - "Put => 1,Put => 0"), - _sender.getCommands(true)); + ASSERT_EQ("Create bucket => 1,Create bucket => 0," + "Put => 1,Put => 0", + _sender.getCommands(true)); // Manually shove sent messages into pending message tracker, since // this isn't done automatically. @@ -510,37 +402,32 @@ PutOperationTest::testDoNotSendCreateBucketIfAlreadyPending() sendPut(createPut(doc)); - CPPUNIT_ASSERT_EQUAL(std::string("Create bucket => 1,Create bucket => 0," - "Put => 1,Put => 0," - "Put => 1,Put => 0"), - _sender.getCommands(true)); + ASSERT_EQ("Create bucket => 1,Create bucket => 0," + "Put => 1,Put => 0," + "Put => 1,Put => 0", + _sender.getCommands(true)); } -void -PutOperationTest::testNoStorageNodes() -{ +TEST_F(PutOperationTest, no_storage_nodes) { setupDistributor(2, 1, "storage:0 distributor:1"); createAndSendSampleDocument(180); - CPPUNIT_ASSERT_EQUAL(std::string("PutReply(doc:test:test, BucketId(0x0000000000000000), " - "timestamp 100) ReturnCode(NOT_CONNECTED, " - "Can't store document: No storage nodes available)"), - _sender.getLastReply(true)); + ASSERT_EQ("PutReply(doc:test:test, BucketId(0x0000000000000000), " + "timestamp 100) ReturnCode(NOT_CONNECTED, " + "Can't store document: No storage nodes available)", + _sender.getLastReply(true)); } -void -PutOperationTest::testUpdateCorrectBucketOnRemappedPut() -{ +TEST_F(PutOperationTest, update_correct_bucket_on_remapped_put) { setupDistributor(2, 2, "storage:2 distributor:1"); - Document::SP doc(new Document(*_html_type, DocumentId( + Document::SP doc(new Document(doc_type(), DocumentId( UserDocIdString("userdoc:test:13:uri")))); addNodesToBucketDB(document::BucketId(16,13), "0=0,1=0"); sendPut(createPut(doc)); - CPPUNIT_ASSERT_EQUAL(std::string("Put => 0,Put => 1"), - _sender.getCommands(true)); + ASSERT_EQ("Put => 0,Put => 1", _sender.getCommands(true)); { std::shared_ptr<api::StorageCommand> msg2 = _sender.commands[0]; @@ -553,15 +440,14 @@ PutOperationTest::testUpdateCorrectBucketOnRemappedPut() sendReply(1); - CPPUNIT_ASSERT_EQUAL(std::string("PutReply(userdoc:test:13:uri, " - "BucketId(0x0000000000000000), " - "timestamp 100) ReturnCode(NONE)"), - _sender.getLastReply()); + ASSERT_EQ("PutReply(userdoc:test:13:uri, " + "BucketId(0x0000000000000000), " + "timestamp 100) ReturnCode(NONE)", + _sender.getLastReply()); - CPPUNIT_ASSERT_EQUAL( - std::string("BucketId(0x440000000000000d) : " - "node(idx=0,crc=0x1,docs=2/4,bytes=3/5,trusted=true,active=false,ready=false)"), - dumpBucket(document::BucketId(17, 13))); + ASSERT_EQ("BucketId(0x440000000000000d) : " + "node(idx=0,crc=0x1,docs=2/4,bytes=3/5,trusted=true,active=false,ready=false)", + dumpBucket(document::BucketId(17, 13))); } BucketInfo @@ -612,52 +498,78 @@ PutOperationTest::getNodes(const std::string& infoString) { return ost.str(); } -void -PutOperationTest::testTargetNodes() -{ +TEST_F(PutOperationTest, target_nodes) { setupDistributor(2, 6, "storage:6 distributor:1"); // Ideal state of bucket is 1,3. - CPPUNIT_ASSERT_EQUAL(std::string("target( 1 3 ) create( 1 3 )"), getNodes("")); - CPPUNIT_ASSERT_EQUAL(std::string("target( 1 3 ) create( 3 )"), getNodes("1-1-true")); - CPPUNIT_ASSERT_EQUAL(std::string("target( 1 3 ) create( 3 )"), getNodes("1-1-false")); - CPPUNIT_ASSERT_EQUAL(std::string("target( 3 4 5 ) create( )"), getNodes("3-1-true,4-1-true,5-1-true")); - CPPUNIT_ASSERT_EQUAL(std::string("target( 3 4 ) create( )"), getNodes("3-2-true,4-2-true,5-1-false")); - CPPUNIT_ASSERT_EQUAL(std::string("target( 1 3 4 ) create( )"), getNodes("3-2-true,4-2-true,1-1-false")); - CPPUNIT_ASSERT_EQUAL(std::string("target( 4 5 ) create( )"), getNodes("4-2-false,5-1-false")); - CPPUNIT_ASSERT_EQUAL(std::string("target( 1 4 ) create( 1 )"), getNodes("4-1-true")); + ASSERT_EQ("target( 1 3 ) create( 1 3 )", getNodes("")); + ASSERT_EQ("target( 1 3 ) create( 3 )", getNodes("1-1-true")); + ASSERT_EQ("target( 1 3 ) create( 3 )", getNodes("1-1-false")); + ASSERT_EQ("target( 3 4 5 ) create( )", getNodes("3-1-true,4-1-true,5-1-true")); + ASSERT_EQ("target( 3 4 ) create( )", getNodes("3-2-true,4-2-true,5-1-false")); + ASSERT_EQ("target( 1 3 4 ) create( )", getNodes("3-2-true,4-2-true,1-1-false")); + ASSERT_EQ("target( 4 5 ) create( )", getNodes("4-2-false,5-1-false")); + ASSERT_EQ("target( 1 4 ) create( 1 )", getNodes("4-1-true")); } -void -PutOperationTest::testDoNotResurrectDownedNodesInBucketDB() -{ - setupDistributor(2, 2, "storage:2 distributor:1"); +TEST_F(PutOperationTest, replica_not_resurrected_in_db_when_node_down_in_active_state) { + setupDistributor(Redundancy(3), NodeCount(3), "distributor:1 storage:3"); Document::SP doc(createDummyDocument("test", "uri")); document::BucketId bId = getExternalOperationHandler().getBucketId(doc->getId()); - addNodesToBucketDB(bId, "0=1/2/3/t,1=1/2/3/t"); + addNodesToBucketDB(bId, "0=1/2/3/t,1=1/2/3/t,2=1/2/3/t"); sendPut(createPut(doc)); - CPPUNIT_ASSERT_EQUAL(std::string("Put => 1,Put => 0"), - _sender.getCommands(true)); + ASSERT_EQ("Put => 1,Put => 0,Put => 2", _sender.getCommands(true)); - enableDistributorClusterState("distributor:1 storage:2 .1.s:d"); + enableDistributorClusterState("distributor:1 storage:3 .1.s:d .2.s:m"); addNodesToBucketDB(bId, "0=1/2/3/t"); // This will actually remove node #1. - sendReply(0, api::ReturnCode::OK, api::BucketInfo(9,9,9)); - sendReply(1, api::ReturnCode::OK, api::BucketInfo(5,6,7)); + sendReply(0, api::ReturnCode::OK, api::BucketInfo(9, 9, 9)); + sendReply(1, api::ReturnCode::OK, api::BucketInfo(5, 6, 7)); + sendReply(2, api::ReturnCode::OK, api::BucketInfo(7, 8, 9)); - CPPUNIT_ASSERT_EQUAL( - std::string("BucketId(0x4000000000002a52) : " - "node(idx=0,crc=0x5,docs=6/6,bytes=7/7,trusted=true,active=false,ready=false)"), - dumpBucket(getExternalOperationHandler().getBucketId(doc->getId()))); + ASSERT_EQ("BucketId(0x4000000000002a52) : " + "node(idx=0,crc=0x5,docs=6/6,bytes=7/7,trusted=true,active=false,ready=false)", + dumpBucket(getExternalOperationHandler().getBucketId(doc->getId()))); } -void -PutOperationTest::sendToRetiredNodesIfNoUpNodesAvailable() -{ +TEST_F(PutOperationTest, replica_not_resurrected_in_db_when_node_down_in_pending_state) { + setupDistributor(Redundancy(3), NodeCount(4), "version:1 distributor:1 storage:3"); + + auto doc = createDummyDocument("test", "uri"); + auto bucket = getExternalOperationHandler().getBucketId(doc->getId()); + addNodesToBucketDB(bucket, "0=1/2/3/t,1=1/2/3/t,2=1/2/3/t"); + sendPut(createPut(doc)); + + ASSERT_EQ("Put => 1,Put => 0,Put => 2", _sender.getCommands(true)); + // Trigger a pending (but not completed) cluster state transition where content + // node 0 is down. This will prune its replica from the DB. We assume that the + // downed node managed to send off a reply to the Put before it went down, and + // this reply must not recreate the replica in the bucket database. This is because + // we have an invariant that the DB shall not contain replicas on nodes that are + // not available. + // We also let node 2 be in maintenance to ensure we also consider this an unavailable state. + // Note that we have to explicitly trigger a transition that requires an async bucket + // fetch here; if we just set a node down the cluster state would be immediately applied + // and the distributor's "clear pending messages for downed nodes" logic would kick in + // and hide the problem. + getBucketDBUpdater().onSetSystemState( + std::make_shared<api::SetSystemStateCommand>( + lib::ClusterState("version:2 distributor:1 storage:4 .0.s:d .2.s:m"))); + + sendReply(0, api::ReturnCode::OK, api::BucketInfo(5, 6, 7)); + sendReply(1, api::ReturnCode::OK, api::BucketInfo(6, 7, 8)); + sendReply(2, api::ReturnCode::OK, api::BucketInfo(9, 8, 7)); + + ASSERT_EQ("BucketId(0x4000000000002a52) : " + "node(idx=1,crc=0x5,docs=6/6,bytes=7/7,trusted=true,active=false,ready=false)", + dumpBucket(bucket)); +} + +TEST_F(PutOperationTest, send_to_retired_nodes_if_no_up_nodes_available) { setupDistributor(Redundancy(2), NodeCount(2), "distributor:1 storage:2 .0.s:r .1.s:r"); Document::SP doc(createDummyDocument("test", "uri")); @@ -667,41 +579,31 @@ PutOperationTest::sendToRetiredNodesIfNoUpNodesAvailable() sendPut(createPut(doc)); - CPPUNIT_ASSERT_EQUAL("Put => 0,Put => 1"s, - _sender.getCommands(true)); + ASSERT_EQ("Put => 0,Put => 1", _sender.getCommands(true)); } -void -PutOperationTest::doTestCreationWithBucketActivationDisabled(bool disabled) -{ +void PutOperationTest::do_test_creation_with_bucket_activation_disabled(bool disabled) { setupDistributor(Redundancy(2), NodeCount(2), "distributor:1 storage:1"); disableBucketActivationInConfig(disabled); Document::SP doc(createDummyDocument("test", "uri")); sendPut(createPut(doc)); - CPPUNIT_ASSERT_EQUAL(std::string("Create bucket => 0,Put => 0"), - _sender.getCommands(true)); + ASSERT_EQ("Create bucket => 0,Put => 0", _sender.getCommands(true)); auto cmd = _sender.commands[0]; auto createCmd = std::dynamic_pointer_cast<api::CreateBucketCommand>(cmd); - CPPUNIT_ASSERT(createCmd.get() != nullptr); + ASSERT_TRUE(createCmd.get() != nullptr); // There's only 1 content node, so if activation were not disabled, it // should always be activated. - CPPUNIT_ASSERT_EQUAL(!disabled, createCmd->getActive()); + ASSERT_EQ(!disabled, createCmd->getActive()); } -void -PutOperationTest::replicaImplicitlyActivatedWhenActivationIsNotDisabled() -{ - doTestCreationWithBucketActivationDisabled(false); -} - -void -PutOperationTest::replicaNotImplicitlyActivatedWhenActivationIsDisabled() -{ - doTestCreationWithBucketActivationDisabled(true); +TEST_F(PutOperationTest, replica_implicitly_activated_when_activation_is_not_disabled) { + do_test_creation_with_bucket_activation_disabled(false); } +TEST_F(PutOperationTest, replica_not_implicitly_activated_when_activation_is_disabled) { + do_test_creation_with_bucket_activation_disabled(true); } } diff --git a/storage/src/vespa/storage/distributor/bucketdbupdater.cpp b/storage/src/vespa/storage/distributor/bucketdbupdater.cpp index 5482696b945..33fa80ab484 100644 --- a/storage/src/vespa/storage/distributor/bucketdbupdater.cpp +++ b/storage/src/vespa/storage/distributor/bucketdbupdater.cpp @@ -71,7 +71,7 @@ BucketOwnership BucketDBUpdater::checkOwnershipInPendingState(const document::Bucket& b) const { if (hasPendingClusterState()) { - const lib::ClusterState& state(*_pendingClusterState->getNewClusterStateBundle().getDerivedClusterState(b.getBucketSpace())); + const auto& state(*_pendingClusterState->getNewClusterStateBundle().getDerivedClusterState(b.getBucketSpace())); if (!_distributorComponent.ownsBucketInState(state, b)) { return BucketOwnership::createNotOwnedInState(state); } @@ -79,6 +79,13 @@ BucketDBUpdater::checkOwnershipInPendingState(const document::Bucket& b) const return BucketOwnership::createOwned(); } +const lib::ClusterState* +BucketDBUpdater::pendingClusterStateOrNull(const document::BucketSpace& space) const { + return (hasPendingClusterState() + ? _pendingClusterState->getNewClusterStateBundle().getDerivedClusterState(space).get() + : nullptr); +} + void BucketDBUpdater::sendRequestBucketInfo( uint16_t node, @@ -121,7 +128,7 @@ BucketDBUpdater::recheckBucketInfo(uint32_t nodeIdx, void BucketDBUpdater::removeSuperfluousBuckets( const lib::ClusterStateBundle& newState, - [[maybe_unused]] bool is_distribution_config_change) + bool is_distribution_config_change) { const bool move_to_read_only_db = shouldDeferStateEnabling(); const char* up_states = _distributorComponent.getDistributor().getStorageNodeUpStates(); @@ -130,6 +137,16 @@ BucketDBUpdater::removeSuperfluousBuckets( const auto& oldClusterState(elem.second->getClusterState()); const auto& new_cluster_state = newState.getDerivedClusterState(elem.first); + // Running a full DB sweep is expensive, so if the cluster state transition does + // not actually indicate that buckets should possibly be removed, we elide it entirely. + if (!is_distribution_config_change + && db_pruning_may_be_elided(oldClusterState, *new_cluster_state, up_states)) + { + LOG(debug, "Eliding DB pruning for state transition '%s' -> '%s'", + oldClusterState.toString().c_str(), new_cluster_state->toString().c_str()); + continue; + } + auto& bucketDb(elem.second->getBucketDatabase()); auto& readOnlyDb(_distributorComponent.getReadOnlyBucketSpaceRepo().get(elem.first).getBucketDatabase()); @@ -850,6 +867,7 @@ BucketDBUpdater::NodeRemover::process(BucketDatabase::Entry& e) for (uint16_t i = 0; i < e->getNodeCount(); i++) { Node n(NodeType::STORAGE, e->getNodeRef(i).getNode()); + // TODO replace with intersection hash set of config and cluster state if (_state.getNodeState(n).getState().oneOf(_upStates)) { remainingCopies.push_back(e->getNodeRef(i)); } diff --git a/storage/src/vespa/storage/distributor/bucketdbupdater.h b/storage/src/vespa/storage/distributor/bucketdbupdater.h index e9877d37e67..dbcf6699bdc 100644 --- a/storage/src/vespa/storage/distributor/bucketdbupdater.h +++ b/storage/src/vespa/storage/distributor/bucketdbupdater.h @@ -37,10 +37,13 @@ public: DistributorBucketSpaceRepo& readOnlyBucketSpaceRepo, DistributorMessageSender& sender, DistributorComponentRegister& compReg); - ~BucketDBUpdater(); + ~BucketDBUpdater() override; void flush(); + // If there is a pending state, returns ownership state of bucket in it. + // Otherwise always returns "is owned", i.e. it must also be checked in the current state. BucketOwnership checkOwnershipInPendingState(const document::Bucket&) const; + const lib::ClusterState* pendingClusterStateOrNull(const document::BucketSpace&) const; void recheckBucketInfo(uint32_t nodeIdx, const document::Bucket& bucket); bool onSetSystemState(const std::shared_ptr<api::SetSystemStateCommand>& cmd) override; diff --git a/storage/src/vespa/storage/distributor/distributor.cpp b/storage/src/vespa/storage/distributor/distributor.cpp index c92dfbdc14e..771baa04fca 100644 --- a/storage/src/vespa/storage/distributor/distributor.cpp +++ b/storage/src/vespa/storage/distributor/distributor.cpp @@ -143,6 +143,11 @@ Distributor::checkOwnershipInPendingState(const document::Bucket &b) const return _bucketDBUpdater.checkOwnershipInPendingState(b); } +const lib::ClusterState* +Distributor::pendingClusterStateOrNull(const document::BucketSpace& space) const { + return _bucketDBUpdater.pendingClusterStateOrNull(space); +} + void Distributor::sendCommand(const std::shared_ptr<api::StorageCommand>& cmd) { diff --git a/storage/src/vespa/storage/distributor/distributor.h b/storage/src/vespa/storage/distributor/distributor.h index cd24b91eba2..3237b4b4d71 100644 --- a/storage/src/vespa/storage/distributor/distributor.h +++ b/storage/src/vespa/storage/distributor/distributor.h @@ -74,6 +74,8 @@ public: BucketOwnership checkOwnershipInPendingState(const document::Bucket &bucket) const override; + const lib::ClusterState* pendingClusterStateOrNull(const document::BucketSpace&) const override; + /** * Enables a new cluster state. Called after the bucket db updater has * retrieved all bucket info related to the change. diff --git a/storage/src/vespa/storage/distributor/distributorcomponent.cpp b/storage/src/vespa/storage/distributor/distributorcomponent.cpp index 9bd215b9644..1c84376dea6 100644 --- a/storage/src/vespa/storage/distributor/distributorcomponent.cpp +++ b/storage/src/vespa/storage/distributor/distributorcomponent.cpp @@ -194,29 +194,28 @@ DistributorComponent::removeNodesFromDB(const document::Bucket &bucket, } } -std::vector<uint16_t> -DistributorComponent::enumerateDownNodes( +void +DistributorComponent::enumerateUnavailableNodes( + std::vector<uint16_t>& unavailableNodes, const lib::ClusterState& s, - const document::Bucket &bucket, + const document::Bucket& bucket, const std::vector<BucketCopy>& candidates) const { - std::vector<uint16_t> downNodes; + const auto* up_states = _distributor.getStorageNodeUpStates(); for (uint32_t i = 0; i < candidates.size(); ++i) { const BucketCopy& copy(candidates[i]); const lib::NodeState& ns( - s.getNodeState(lib::Node(lib::NodeType::STORAGE, - copy.getNode()))); - if (ns.getState() == lib::State::DOWN) { + s.getNodeState(lib::Node(lib::NodeType::STORAGE, copy.getNode()))); + if (!ns.getState().oneOf(up_states)) { LOG(debug, "Trying to add a bucket copy to %s whose node is marked as " "down in the cluster state: %s. Ignoring it since no zombies " "are allowed!", bucket.toString().c_str(), copy.toString().c_str()); - downNodes.push_back(copy.getNode()); + unavailableNodes.emplace_back(copy.getNode()); } } - return downNodes; } void @@ -236,8 +235,6 @@ DistributorComponent::updateBucketDatabase( "cluster state '%s' - ignoring!", 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"); return; } @@ -258,22 +255,24 @@ 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(bucketSpace.getClusterState(), bucket, changedNodes)); + // bucket database (i.e. copies on nodes that are actually unavailable). + std::vector<uint16_t> unavailableNodes; + enumerateUnavailableNodes(unavailableNodes, bucketSpace.getClusterState(), bucket, changedNodes); + if (auto* pending_state = _distributor.pendingClusterStateOrNull(bucket.getBucketSpace())) { + enumerateUnavailableNodes(unavailableNodes, *pending_state, bucket, changedNodes); + } // Optimize for common case where we don't have to create a new // bucket copy vector - if (downNodes.empty()) { + if (unavailableNodes.empty()) { dbentry->addNodes(changedNodes, getIdealNodes(bucket)); } else { std::vector<BucketCopy> upNodes; for (uint32_t i = 0; i < changedNodes.size(); ++i) { const BucketCopy& copy(changedNodes[i]); - if (std::find(downNodes.begin(), downNodes.end(), - copy.getNode()) - == downNodes.end()) + if (std::find(unavailableNodes.begin(), unavailableNodes.end(), copy.getNode()) + == unavailableNodes.end()) { - upNodes.push_back(copy); + upNodes.emplace_back(copy); } } dbentry->addNodes(upNodes, getIdealNodes(bucket)); diff --git a/storage/src/vespa/storage/distributor/distributorcomponent.h b/storage/src/vespa/storage/distributor/distributorcomponent.h index f2aea89d47c..df25efb3b00 100644 --- a/storage/src/vespa/storage/distributor/distributorcomponent.h +++ b/storage/src/vespa/storage/distributor/distributorcomponent.h @@ -175,9 +175,10 @@ public: bool initializing() const; private: - std::vector<uint16_t> enumerateDownNodes( + void enumerateUnavailableNodes( + std::vector<uint16_t>& unavailableNodes, const lib::ClusterState& s, - const document::Bucket &bucket, + const document::Bucket& bucket, const std::vector<BucketCopy>& candidates) const; DistributorInterface& _distributor; diff --git a/storage/src/vespa/storage/distributor/distributorinterface.h b/storage/src/vespa/storage/distributor/distributorinterface.h index 17c300fa0a9..d9f037bb8f1 100644 --- a/storage/src/vespa/storage/distributor/distributorinterface.h +++ b/storage/src/vespa/storage/distributor/distributorinterface.h @@ -24,6 +24,7 @@ public: virtual DistributorMetricSet& getMetrics() = 0; virtual void enableClusterStateBundle(const lib::ClusterStateBundle& state) = 0; virtual BucketOwnership checkOwnershipInPendingState(const document::Bucket &bucket) const = 0; + virtual const lib::ClusterState* pendingClusterStateOrNull(const document::BucketSpace&) const = 0; virtual void notifyDistributionChangeEnabled() = 0; /** diff --git a/vdslib/src/tests/CMakeLists.txt b/vdslib/src/tests/CMakeLists.txt index 0c9ae7e0048..084320ee059 100644 --- a/vdslib/src/tests/CMakeLists.txt +++ b/vdslib/src/tests/CMakeLists.txt @@ -8,6 +8,8 @@ vespa_add_executable(vdslib_gtest_runner_app TEST DEPENDS vdslib_bucketdistributiontest vdslib_containertest + vdslib_teststate + vdslib_testthread gtest ) @@ -22,8 +24,6 @@ vespa_add_executable(vdslib_testrunner_app TEST testrunner.cpp DEPENDS vdslib_testdistribution - vdslib_teststate - vdslib_testthread ) # TODO: Test with a larger chunk size to parallelize test suite runs diff --git a/vdslib/src/tests/state/CMakeLists.txt b/vdslib/src/tests/state/CMakeLists.txt index 8b1957a91bb..6308e06e3f1 100644 --- a/vdslib/src/tests/state/CMakeLists.txt +++ b/vdslib/src/tests/state/CMakeLists.txt @@ -6,4 +6,5 @@ vespa_add_library(vdslib_teststate nodestatetest.cpp DEPENDS vdslib + gtest ) diff --git a/vdslib/src/tests/state/cluster_state_bundle_test.cpp b/vdslib/src/tests/state/cluster_state_bundle_test.cpp index 4798fe505f8..bfd83673442 100644 --- a/vdslib/src/tests/state/cluster_state_bundle_test.cpp +++ b/vdslib/src/tests/state/cluster_state_bundle_test.cpp @@ -2,8 +2,7 @@ #include <vespa/vdslib/state/cluster_state_bundle.h> #include <vespa/vdslib/state/clusterstate.h> - -#include <cppunit/extensions/HelperMacros.h> +#include <vespa/vespalib/gtest/gtest.h> using document::BucketSpace; @@ -11,20 +10,6 @@ namespace storage::lib { using ClusterStatePtr = std::shared_ptr<const ClusterState>; -struct ClusterStateBundleTest : public CppUnit::TestFixture { - CPPUNIT_TEST_SUITE(ClusterStateBundleTest); - CPPUNIT_TEST(derived_state_is_returned_if_bucket_space_is_found); - CPPUNIT_TEST(baseline_state_is_returned_if_bucket_space_is_not_found); - CPPUNIT_TEST(verify_equality_operator); - CPPUNIT_TEST_SUITE_END(); - - void derived_state_is_returned_if_bucket_space_is_found(); - void baseline_state_is_returned_if_bucket_space_is_not_found(); - void verify_equality_operator(); -}; - -CPPUNIT_TEST_SUITE_REGISTRATION(ClusterStateBundleTest); - struct Fixture { ClusterState baselineState; ClusterStatePtr derivedState; @@ -37,18 +22,16 @@ struct Fixture { ~Fixture() {} }; -void -ClusterStateBundleTest::derived_state_is_returned_if_bucket_space_is_found() +TEST(ClusterStateBundleTest, derived_state_is_returned_if_bucket_space_is_found) { Fixture f; - CPPUNIT_ASSERT_EQUAL(*f.derivedState, *f.bundle.getDerivedClusterState(BucketSpace(1))); + EXPECT_EQ(*f.derivedState, *f.bundle.getDerivedClusterState(BucketSpace(1))); } -void -ClusterStateBundleTest::baseline_state_is_returned_if_bucket_space_is_not_found() +TEST(ClusterStateBundleTest, baseline_state_is_returned_if_bucket_space_is_not_found) { Fixture f; - CPPUNIT_ASSERT_EQUAL(f.baselineState, *f.bundle.getDerivedClusterState(BucketSpace(2))); + EXPECT_EQ(f.baselineState, *f.bundle.getDerivedClusterState(BucketSpace(2))); } ClusterStateBundle @@ -61,16 +44,15 @@ makeBundle(const vespalib::string &baselineState, const std::map<BucketSpace, ve return ClusterStateBundle(ClusterState(baselineState), std::move(derivedBucketSpaceStates)); } -void -ClusterStateBundleTest::verify_equality_operator() +TEST(ClusterStateBundleTest, verify_equality_operator) { Fixture f; - CPPUNIT_ASSERT(f.bundle != makeBundle("storage:3", {{BucketSpace(1), "storage:2 .1.s:m"}})); - CPPUNIT_ASSERT(f.bundle != makeBundle("storage:2", {})); - CPPUNIT_ASSERT(f.bundle != makeBundle("storage:2", {{BucketSpace(1), "storage:2 .0.s:m"}})); - CPPUNIT_ASSERT(f.bundle != makeBundle("storage:2", {{BucketSpace(2), "storage:2 .1.s:m"}})); + EXPECT_NE(f.bundle, makeBundle("storage:3", {{BucketSpace(1), "storage:2 .1.s:m"}})); + EXPECT_NE(f.bundle, makeBundle("storage:2", {})); + EXPECT_NE(f.bundle, makeBundle("storage:2", {{BucketSpace(1), "storage:2 .0.s:m"}})); + EXPECT_NE(f.bundle, makeBundle("storage:2", {{BucketSpace(2), "storage:2 .1.s:m"}})); - CPPUNIT_ASSERT_EQUAL(f.bundle, makeBundle("storage:2", {{BucketSpace(1), "storage:2 .1.s:m"}})); + EXPECT_EQ(f.bundle, makeBundle("storage:2", {{BucketSpace(1), "storage:2 .1.s:m"}})); } } diff --git a/vdslib/src/tests/state/clusterstatetest.cpp b/vdslib/src/tests/state/clusterstatetest.cpp index 8cdde1bbc00..143f3aed0e9 100644 --- a/vdslib/src/tests/state/clusterstatetest.cpp +++ b/vdslib/src/tests/state/clusterstatetest.cpp @@ -1,79 +1,37 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <vespa/document/bucket/bucketidfactory.h> #include <vespa/vdslib/state/clusterstate.h> -#include <vespa/vespalib/util/exception.h> +#include <vespa/vdslib/state/random.h> +#include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/stllike/asciistream.h> -#include <vespa/document/bucket/bucketidfactory.h> +#include <vespa/vespalib/util/exception.h> #include <cmath> -#include <vespa/vdslib/state/random.h> -#include <vespa/vdstestlib/cppunit/macros.h> +#include <gmock/gmock.h> using vespalib::string; +using ::testing::ContainsRegex; -namespace storage { -namespace lib { - -struct ClusterStateTest : public CppUnit::TestFixture { - - void testBasicFunctionality(); - void testErrorBehaviour(); - void testBackwardsCompability(); - void testDetailed(); - void testParseFailure(); - void testParseFailureGroups(); - - void testDiff(); - - CPPUNIT_TEST_SUITE(ClusterStateTest); - CPPUNIT_TEST(testBasicFunctionality); - CPPUNIT_TEST(testErrorBehaviour); - CPPUNIT_TEST(testBackwardsCompability); - CPPUNIT_TEST(testDetailed); - - // Ideal state tests. - CPPUNIT_TEST(testParseFailure); - CPPUNIT_TEST(testParseFailureGroups); - CPPUNIT_TEST(testDiff); - CPPUNIT_TEST_SUITE_END(); -}; - -CPPUNIT_TEST_SUITE_REGISTRATION(ClusterStateTest); - -void -ClusterStateTest::testDiff() { - ClusterState state1("distributor:9 storage:4"); - ClusterState state2("distributor:7 storage:6"); - ClusterState state3("distributor:9 storage:2"); - CPPUNIT_ASSERT_EQUAL( - std::string("storage [4: d to u, 5: d to u] " - "distributor [7: u to d, 8: u to d]"), - state1.getTextualDifference(state2)); - CPPUNIT_ASSERT_EQUAL( - std::string("storage [2: u to d, 3: u to d, 4: u to d, 5: u to d] " - "distributor [7: d to u, 8: d to u]"), - state2.getTextualDifference(state3)); -} - +namespace storage::lib { #define VERIFY3(source, result, type, typestr) { \ vespalib::asciistream ost; \ - try{ \ + try { \ state->serialize(ost, type); \ } catch (std::exception& e) { \ - CPPUNIT_FAIL("Failed to serialize system state " \ + FAIL() << ("Failed to serialize system state " \ + state->toString(true) + " in " + std::string(typestr) \ + " format: " + std::string(e.what())); \ } \ - CPPUNIT_ASSERT_EQUAL_MSG(vespalib::string(state->toString(true)), \ - vespalib::string(typestr) + " \"" + vespalib::string(result) + "\"", \ - vespalib::string(typestr) + " \"" + ost.str() + "\""); \ + EXPECT_EQ(vespalib::string(typestr) + " \"" + vespalib::string(result) + "\"", \ + vespalib::string(typestr) + " \"" + ost.str() + "\"") << state->toString(true); \ } #define VERIFY2(serialized, result, testOld, testNew) { \ std::unique_ptr<ClusterState> state; \ - try{ \ + try { \ state.reset(new ClusterState(serialized)); \ } catch (std::exception& e) { \ - CPPUNIT_FAIL("Failed to parse '" + std::string(serialized) \ + FAIL() << ("Failed to parse '" + std::string(serialized) \ + "': " + e.what()); \ } \ if (testOld) VERIFY3(serialized, result, true, "Old") \ @@ -90,15 +48,14 @@ ClusterStateTest::testDiff() { #define VERIFY_FAIL(serialized, error) { \ try{ \ ClusterState state(serialized); \ - CPPUNIT_FAIL("Parsing the state '" + std::string(serialized) \ + FAIL() << ("Parsing the state '" + std::string(serialized) \ + "' is supposed to fail."); \ } catch (vespalib::Exception& e) { \ - CPPUNIT_ASSERT_MATCH_REGEX(error, e.getMessage()); \ + EXPECT_THAT(e.getMessage(), ContainsRegex(error)); \ } \ } -void -ClusterStateTest::testBasicFunctionality() +TEST(ClusterStateTest, test_basic_functionality) { // Version is default and should not be written VERIFYNEW("version:0", ""); @@ -144,31 +101,30 @@ ClusterStateTest::testBasicFunctionality() { ClusterState state("storage:5 .4.s:d .4.m:Foo\\x20bar"); const NodeState& ns(state.getNodeState(Node(NodeType::STORAGE, 4))); - CPPUNIT_ASSERT_EQUAL(string("Foo bar"), ns.getDescription()); + EXPECT_EQ(string("Foo bar"), ns.getDescription()); } ClusterState state; state.setClusterState(State::UP); state.setNodeState(Node(NodeType::DISTRIBUTOR, 3), NodeState(NodeType::DISTRIBUTOR, State::UP)); - CPPUNIT_ASSERT_EQUAL(std::string("distributor:4 .0.s:d .1.s:d .2.s:d"), - state.toString(false)); + EXPECT_EQ(std::string("distributor:4 .0.s:d .1.s:d .2.s:d"), + state.toString(false)); state.setNodeState(Node(NodeType::DISTRIBUTOR, 1), NodeState(NodeType::DISTRIBUTOR, State::UP)); - CPPUNIT_ASSERT_EQUAL(std::string("distributor:4 .0.s:d .2.s:d"), - state.toString(false)); + EXPECT_EQ(std::string("distributor:4 .0.s:d .2.s:d"), + state.toString(false)); state.setNodeState(Node(NodeType::DISTRIBUTOR, 3), NodeState(NodeType::DISTRIBUTOR, State::DOWN)); - CPPUNIT_ASSERT_EQUAL(std::string("distributor:2 .0.s:d"), - state.toString(false)); + EXPECT_EQ(std::string("distributor:2 .0.s:d"), + state.toString(false)); state.setNodeState(Node(NodeType::DISTRIBUTOR, 4), NodeState(NodeType::DISTRIBUTOR, State::UP)); - CPPUNIT_ASSERT_EQUAL(std::string("distributor:5 .0.s:d .2.s:d .3.s:d"), - state.toString(false)); + EXPECT_EQ(std::string("distributor:5 .0.s:d .2.s:d .3.s:d"), + state.toString(false)); } -void -ClusterStateTest::testErrorBehaviour() +TEST(ClusterStateTest, test_error_behaviour) { // Keys with invalid values @@ -215,8 +171,7 @@ ClusterStateTest::testErrorBehaviour() "distributor:4 .2.s:s storage:10 .3.s:s .3.d:4"); } -void -ClusterStateTest::testBackwardsCompability() +TEST(ClusterStateTest, test_backwards_compability) { // 4.1 and older nodes do not support some features, and the java parser // do not allow unknown elements as it was supposed to do, thus we should @@ -243,8 +198,7 @@ ClusterStateTest::testBackwardsCompability() } -void -ClusterStateTest::testDetailed() +TEST(ClusterStateTest, test_detailed) { ClusterState state( "version:314 cluster:i " @@ -252,35 +206,35 @@ ClusterStateTest::testDetailed() "storage:10 .2.d:16 .2.d.3:d .4.s:d .5.c:1.3 .5.r:4" " .6.m:bar\\tfoo .7.s:m .8.d:10 .8.d.4.c:0.6 .8.d.4.m:small" ); - CPPUNIT_ASSERT_EQUAL(314u, state.getVersion()); - CPPUNIT_ASSERT_EQUAL(State::INITIALIZING, state.getClusterState()); - CPPUNIT_ASSERT_EQUAL(uint16_t(8),state.getNodeCount(NodeType::DISTRIBUTOR)); - CPPUNIT_ASSERT_EQUAL(uint16_t(10),state.getNodeCount(NodeType::STORAGE)); + EXPECT_EQ(314u, state.getVersion()); + EXPECT_EQ(State::INITIALIZING, state.getClusterState()); + EXPECT_EQ(uint16_t(8), state.getNodeCount(NodeType::DISTRIBUTOR)); + EXPECT_EQ(uint16_t(10), state.getNodeCount(NodeType::STORAGE)); // Testing distributor node states for (uint16_t i = 0; i <= 20; ++i) { const NodeState& ns(state.getNodeState(Node(NodeType::DISTRIBUTOR, i))); // Test node states if (i == 1 || i == 3) { - CPPUNIT_ASSERT_EQUAL(State::INITIALIZING, ns.getState()); + EXPECT_EQ(State::INITIALIZING, ns.getState()); } else if (i == 5 || i >= 8) { - CPPUNIT_ASSERT_EQUAL(State::DOWN, ns.getState()); + EXPECT_EQ(State::DOWN, ns.getState()); } else { - CPPUNIT_ASSERT_EQUAL(State::UP, ns.getState()); + EXPECT_EQ(State::UP, ns.getState()); } // Test initialize progress if (i == 1) { - CPPUNIT_ASSERT_EQUAL(vespalib::Double(0.0), ns.getInitProgress()); + EXPECT_EQ(vespalib::Double(0.0), ns.getInitProgress()); } else if (i == 3) { - CPPUNIT_ASSERT_EQUAL(vespalib::Double(0.5), ns.getInitProgress()); + EXPECT_EQ(vespalib::Double(0.5), ns.getInitProgress()); } else { - CPPUNIT_ASSERT_EQUAL(vespalib::Double(0.0), ns.getInitProgress()); + EXPECT_EQ(vespalib::Double(0.0), ns.getInitProgress()); } // Test message if (i == 7) { - CPPUNIT_ASSERT_EQUAL(string("foo bar"), ns.getDescription()); + EXPECT_EQ(string("foo bar"), ns.getDescription()); } else { - CPPUNIT_ASSERT_EQUAL(string(""), ns.getDescription()); + EXPECT_EQ(string(""), ns.getDescription()); } } @@ -289,42 +243,41 @@ ClusterStateTest::testDetailed() const NodeState& ns(state.getNodeState(Node(NodeType::STORAGE, i))); // Test node states if (i == 4 || i >= 10) { - CPPUNIT_ASSERT_EQUAL(State::DOWN, ns.getState()); + EXPECT_EQ(State::DOWN, ns.getState()); } else if (i == 7) { - CPPUNIT_ASSERT_EQUAL(State::MAINTENANCE, ns.getState()); + EXPECT_EQ(State::MAINTENANCE, ns.getState()); } else { - CPPUNIT_ASSERT_EQUAL(State::UP, ns.getState()); + EXPECT_EQ(State::UP, ns.getState()); } // Test disk states if (i == 2) { - CPPUNIT_ASSERT_EQUAL(uint16_t(16), ns.getDiskCount()); + EXPECT_EQ(uint16_t(16), ns.getDiskCount()); } else if (i == 8) { - CPPUNIT_ASSERT_EQUAL(uint16_t(10), ns.getDiskCount()); + EXPECT_EQ(uint16_t(10), ns.getDiskCount()); } else { - CPPUNIT_ASSERT_EQUAL(uint16_t(0), ns.getDiskCount()); + EXPECT_EQ(uint16_t(0), ns.getDiskCount()); } if (i == 2) { for (uint16_t j = 0; j < 16; ++j) { if (j == 3) { - CPPUNIT_ASSERT_EQUAL(State::DOWN, - ns.getDiskState(j).getState()); + EXPECT_EQ(State::DOWN, + ns.getDiskState(j).getState()); } else { - CPPUNIT_ASSERT_EQUAL(State::UP, - ns.getDiskState(j).getState()); + EXPECT_EQ(State::UP, + ns.getDiskState(j).getState()); } } } else if (i == 8) { for (uint16_t j = 0; j < 10; ++j) { if (j == 4) { - CPPUNIT_ASSERT_DOUBLES_EQUAL( - 0.6, ns.getDiskState(j).getCapacity().getValue(), 0.0001); - CPPUNIT_ASSERT_EQUAL( + EXPECT_DOUBLE_EQ(0.6, ns.getDiskState(j).getCapacity().getValue()); + EXPECT_EQ( string("small"), ns.getDiskState(j).getDescription()); } else { - CPPUNIT_ASSERT_DOUBLES_EQUAL( - 1.0, ns.getDiskState(j).getCapacity().getValue(), 0.0001); - CPPUNIT_ASSERT_EQUAL( + EXPECT_DOUBLE_EQ( + 1.0, ns.getDiskState(j).getCapacity().getValue()); + EXPECT_EQ( string(""), ns.getDiskState(j).getDescription()); } @@ -332,57 +285,36 @@ ClusterStateTest::testDetailed() } // Test message if (i == 6) { - CPPUNIT_ASSERT_EQUAL(string("bar\tfoo"), ns.getDescription()); + EXPECT_EQ(string("bar\tfoo"), ns.getDescription()); } else { - CPPUNIT_ASSERT_EQUAL(string(""), ns.getDescription()); + EXPECT_EQ(string(""), ns.getDescription()); } // Test reliability if (i == 5) { - CPPUNIT_ASSERT_EQUAL(uint16_t(4), ns.getReliability()); + EXPECT_EQ(uint16_t(4), ns.getReliability()); } else { - CPPUNIT_ASSERT_EQUAL(uint16_t(1), ns.getReliability()); + EXPECT_EQ(uint16_t(1), ns.getReliability()); } // Test capacity if (i == 5) { - CPPUNIT_ASSERT_EQUAL(vespalib::Double(1.3), ns.getCapacity()); + EXPECT_EQ(vespalib::Double(1.3), ns.getCapacity()); } else { - CPPUNIT_ASSERT_EQUAL(vespalib::Double(1.0), ns.getCapacity()); + EXPECT_EQ(vespalib::Double(1.0), ns.getCapacity()); } } } -void -ClusterStateTest::testParseFailure() +TEST(ClusterStateTest, test_parse_failure) { - try { - ClusterState state("storage"); - CPPUNIT_ASSERT(false); - } catch (vespalib::Exception& e) { - } - - try { - ClusterState state(""); - } catch (vespalib::Exception& e) { - CPPUNIT_ASSERT(false); - } - - try { - ClusterState state(".her:tull"); - CPPUNIT_ASSERT(false); - } catch (vespalib::Exception& e) { - } + EXPECT_THROW(ClusterState state("storage"), vespalib::Exception); + EXPECT_NO_THROW(ClusterState state("")); + EXPECT_THROW(ClusterState state(".her:tull"), vespalib::Exception); } -void -ClusterStateTest::testParseFailureGroups() +TEST(ClusterStateTest, test_parse_failure_groups) { - try { - ClusterState state(")"); - CPPUNIT_ASSERT(false); - } catch (vespalib::Exception& e) { - } + EXPECT_THROW(ClusterState state(")"), vespalib::Exception); } -} // lib -} // storage +} diff --git a/vdslib/src/tests/state/nodestatetest.cpp b/vdslib/src/tests/state/nodestatetest.cpp index 67ea7759654..da6faa49779 100644 --- a/vdslib/src/tests/state/nodestatetest.cpp +++ b/vdslib/src/tests/state/nodestatetest.cpp @@ -1,109 +1,88 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <vespa/vdslib/state/nodestate.h> -#include <cppunit/extensions/HelperMacros.h> +#include <vespa/vespalib/gtest/gtest.h> -namespace storage { -namespace lib { +namespace storage::lib { -class NodeStateTest : public CppUnit::TestFixture { - CPPUNIT_TEST_SUITE(NodeStateTest); - CPPUNIT_TEST(testParsing); - CPPUNIT_TEST(testExponential); - CPPUNIT_TEST(stateInstancesProvideDescriptiveNames); - CPPUNIT_TEST_SUITE_END(); - -public: -protected: - void testParsing(); - void testExponential(); // Test exponential notation. - void stateInstancesProvideDescriptiveNames(); -}; - -CPPUNIT_TEST_SUITE_REGISTRATION( NodeStateTest ); - -void -NodeStateTest::testParsing() +TEST(NodeStateTest, test_parsing) { { NodeState ns = NodeState("s:u"); - CPPUNIT_ASSERT_EQUAL(std::string("s:u"), ns.toString()); - CPPUNIT_ASSERT_EQUAL(vespalib::Double(1.0), ns.getCapacity()); - CPPUNIT_ASSERT_EQUAL(uint16_t(1), ns.getReliability()); + EXPECT_EQ(std::string("s:u"), ns.toString()); + EXPECT_EQ(vespalib::Double(1.0), ns.getCapacity()); + EXPECT_EQ(uint16_t(1), ns.getReliability()); } { NodeState ns = NodeState("s:m"); - CPPUNIT_ASSERT_EQUAL(std::string("s:m"), ns.toString()); - CPPUNIT_ASSERT_EQUAL(vespalib::Double(1.0), ns.getCapacity()); - CPPUNIT_ASSERT_EQUAL(uint16_t(1), ns.getReliability()); + EXPECT_EQ(std::string("s:m"), ns.toString()); + EXPECT_EQ(vespalib::Double(1.0), ns.getCapacity()); + EXPECT_EQ(uint16_t(1), ns.getReliability()); } { NodeState ns = NodeState("t:4"); - CPPUNIT_ASSERT_EQUAL(std::string("s:u t:4"), ns.toString()); - CPPUNIT_ASSERT_EQUAL(uint64_t(4), ns.getStartTimestamp()); + EXPECT_EQ(std::string("s:u t:4"), ns.toString()); + EXPECT_EQ(uint64_t(4), ns.getStartTimestamp()); } { NodeState ns = NodeState("s:u c:2.4 r:3 b:12"); - CPPUNIT_ASSERT_EQUAL(std::string("s:u c:2.4 r:3 b:12"), ns.toString()); - CPPUNIT_ASSERT_EQUAL(vespalib::Double(2.4), ns.getCapacity()); - CPPUNIT_ASSERT_EQUAL(uint16_t(3), ns.getReliability()); - CPPUNIT_ASSERT_EQUAL(12, (int)ns.getMinUsedBits()); + EXPECT_EQ(std::string("s:u c:2.4 r:3 b:12"), ns.toString()); + EXPECT_EQ(vespalib::Double(2.4), ns.getCapacity()); + EXPECT_EQ(uint16_t(3), ns.getReliability()); + EXPECT_EQ(12, (int)ns.getMinUsedBits()); - CPPUNIT_ASSERT(!(NodeState("s:u b:12") == NodeState("s:u b:13"))); + EXPECT_NE(NodeState("s:u b:12"), NodeState("s:u b:13")); } { NodeState ns = NodeState("c:2.4\ns:u\nr:5"); - CPPUNIT_ASSERT_EQUAL(std::string("s:u c:2.4 r:5"), ns.toString()); - CPPUNIT_ASSERT_EQUAL(vespalib::Double(2.4), ns.getCapacity()); - CPPUNIT_ASSERT_EQUAL(uint16_t(5), ns.getReliability()); + EXPECT_EQ(std::string("s:u c:2.4 r:5"), ns.toString()); + EXPECT_EQ(vespalib::Double(2.4), ns.getCapacity()); + EXPECT_EQ(uint16_t(5), ns.getReliability()); } { NodeState ns = NodeState("c:2.4 r:1"); - CPPUNIT_ASSERT_EQUAL(std::string("s:u c:2.4"), ns.toString()); - CPPUNIT_ASSERT_EQUAL(vespalib::Double(2.4), ns.getCapacity()); - CPPUNIT_ASSERT_EQUAL(uint16_t(1), ns.getReliability()); + EXPECT_EQ(std::string("s:u c:2.4"), ns.toString()); + EXPECT_EQ(vespalib::Double(2.4), ns.getCapacity()); + EXPECT_EQ(uint16_t(1), ns.getReliability()); } { NodeState ns = NodeState("c:2.4 k:2.6"); - CPPUNIT_ASSERT_EQUAL(std::string("s:u c:2.4"), ns.toString()); - CPPUNIT_ASSERT_EQUAL(vespalib::Double(2.4), ns.getCapacity()); - CPPUNIT_ASSERT_EQUAL(uint16_t(1), ns.getReliability()); + EXPECT_EQ(std::string("s:u c:2.4"), ns.toString()); + EXPECT_EQ(vespalib::Double(2.4), ns.getCapacity()); + EXPECT_EQ(uint16_t(1), ns.getReliability()); } } -void -NodeStateTest::testExponential() +TEST(NodeStateTest, test_exponential) { { NodeState ns = NodeState("c:3E-8"); - CPPUNIT_ASSERT_EQUAL( std::string("s:u c:3e-08"), ns.toString() ); - CPPUNIT_ASSERT_EQUAL(vespalib::Double(3E-8), ns.getCapacity()); + EXPECT_EQ(std::string("s:u c:3e-08"), ns.toString() ); + EXPECT_EQ(vespalib::Double(3E-8), ns.getCapacity()); } { NodeState ns = NodeState("c:3e-08"); - CPPUNIT_ASSERT_EQUAL( std::string("s:u c:3e-08"), ns.toString() ); - CPPUNIT_ASSERT_EQUAL(vespalib::Double(3e-08), ns.getCapacity()); + EXPECT_EQ(std::string("s:u c:3e-08"), ns.toString() ); + EXPECT_EQ(vespalib::Double(3e-08), ns.getCapacity()); } } -void -NodeStateTest::stateInstancesProvideDescriptiveNames() +TEST(NodeStateTest, state_instances_provide_descriptive_names) { - CPPUNIT_ASSERT_EQUAL(vespalib::string("Unknown"), - State::UNKNOWN.getName()); - CPPUNIT_ASSERT_EQUAL(vespalib::string("Maintenance"), - State::MAINTENANCE.getName()); - CPPUNIT_ASSERT_EQUAL(vespalib::string("Down"), - State::DOWN.getName()); - CPPUNIT_ASSERT_EQUAL(vespalib::string("Stopping"), - State::STOPPING.getName()); - CPPUNIT_ASSERT_EQUAL(vespalib::string("Initializing"), - State::INITIALIZING.getName()); - CPPUNIT_ASSERT_EQUAL(vespalib::string("Retired"), - State::RETIRED.getName()); - CPPUNIT_ASSERT_EQUAL(vespalib::string("Up"), - State::UP.getName()); + EXPECT_EQ(vespalib::string("Unknown"), + State::UNKNOWN.getName()); + EXPECT_EQ(vespalib::string("Maintenance"), + State::MAINTENANCE.getName()); + EXPECT_EQ(vespalib::string("Down"), + State::DOWN.getName()); + EXPECT_EQ(vespalib::string("Stopping"), + State::STOPPING.getName()); + EXPECT_EQ(vespalib::string("Initializing"), + State::INITIALIZING.getName()); + EXPECT_EQ(vespalib::string("Retired"), + State::RETIRED.getName()); + EXPECT_EQ(vespalib::string("Up"), + State::UP.getName()); } -} // lib -} // storage +} diff --git a/vdslib/src/tests/thread/CMakeLists.txt b/vdslib/src/tests/thread/CMakeLists.txt index 21bcb06b43c..4d1e753a8f6 100644 --- a/vdslib/src/tests/thread/CMakeLists.txt +++ b/vdslib/src/tests/thread/CMakeLists.txt @@ -4,4 +4,5 @@ vespa_add_library(vdslib_testthread taskschedulertest.cpp DEPENDS vdslib + gtest ) diff --git a/vdslib/src/tests/thread/taskschedulertest.cpp b/vdslib/src/tests/thread/taskschedulertest.cpp index 3c056cbb363..540de722137 100644 --- a/vdslib/src/tests/thread/taskschedulertest.cpp +++ b/vdslib/src/tests/thread/taskschedulertest.cpp @@ -1,24 +1,10 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <vespa/vdslib/thread/taskscheduler.h> -#include <vespa/vdstestlib/cppunit/macros.h> +#include <vespa/vespalib/gtest/gtest.h> namespace vdslib { -struct TaskSchedulerTest : public CppUnit::TestFixture { - void testSimple(); - void testMultipleTasksAtSameTime(); - void testRemoveTask(); - - CPPUNIT_TEST_SUITE(TaskSchedulerTest); - CPPUNIT_TEST(testSimple); - CPPUNIT_TEST(testMultipleTasksAtSameTime); - CPPUNIT_TEST(testRemoveTask); - CPPUNIT_TEST_SUITE_END(); -}; - -CPPUNIT_TEST_SUITE_REGISTRATION(TaskSchedulerTest); - namespace { struct TestWatch : public TaskScheduler::Watch { @@ -106,10 +92,9 @@ std::string join(std::vector<std::string>& v) { return ost.str(); } -} // End of anonymous namespace +} -void -TaskSchedulerTest::testSimple() +TEST(TaskSchedulerTest, test_simple) { FastOS_ThreadPool threadPool(128 * 1024); TestWatch watch(0); @@ -127,7 +112,7 @@ TaskSchedulerTest::testSimple() task->registerCallsWithName("", calls); scheduler.add(TestTask::UP(task)); scheduler.waitForTaskCounterOfAtLeast(counter + 1); - CPPUNIT_ASSERT_EQUAL(std::string("0"), join(calls)); + EXPECT_EQ(std::string("0"), join(calls)); scheduler.waitUntilNoTasksRemaining(); // Ensure task is complete } // Test that task is repeated at intervals if wanted. @@ -142,8 +127,8 @@ TaskSchedulerTest::testSimple() scheduler.waitForTaskCounterOfAtLeast(counter + i); watch.increment(100); } - CPPUNIT_ASSERT_EQUAL(std::string("0,110,220,330,440"), - join(calls)); + EXPECT_EQ(std::string("0,110,220,330,440"), + join(calls)); scheduler.waitUntilNoTasksRemaining(); // Ensure task is complete } // Test that task scheduled at specific time works, and that if @@ -158,23 +143,22 @@ TaskSchedulerTest::testSimple() watch.increment(49); // Not yet time to run FastOS_Thread::Sleep(5); // Check that it has not run yet.. - CPPUNIT_ASSERT_EQUAL(counter, scheduler.getTaskCounter()); + EXPECT_EQ(counter, scheduler.getTaskCounter()); watch.increment(10); // Now time is enough for it to run scheduler.waitForTaskCounterOfAtLeast(counter + 1); watch.increment(10); FastOS_Thread::Sleep(5); // Check that it has not run yet.. - CPPUNIT_ASSERT_EQUAL(counter + 1, scheduler.getTaskCounter()); + EXPECT_EQ(counter + 1, scheduler.getTaskCounter()); watch.increment(50); scheduler.waitForTaskCounterOfAtLeast(counter + 2); - CPPUNIT_ASSERT_EQUAL(std::string("59,129,129,129"), - join(calls)); + EXPECT_EQ(std::string("59,129,129,129"), + join(calls)); scheduler.waitUntilNoTasksRemaining(); // Ensure task is complete } } -void -TaskSchedulerTest::testMultipleTasksAtSameTime() +TEST(TaskSchedulerTest, test_multiple_tasks_at_same_time) { FastOS_ThreadPool threadPool(128 * 1024); TestWatch watch(0); @@ -200,19 +184,18 @@ TaskSchedulerTest::testMultipleTasksAtSameTime() std::ostringstream ost; for (size_t i=0; i<calls.size(); ++i) ost << calls[i] << "\n"; - CPPUNIT_ASSERT_EQUAL(std::string( - "10 task1\n" - "10 task2\n" - "10 task1\n" - "10 task2\n" - "10 task1\n" - "10 task2\n" - ), ost.str()); + EXPECT_EQ(std::string( + "10 task1\n" + "10 task2\n" + "10 task1\n" + "10 task2\n" + "10 task1\n" + "10 task2\n" + ), ost.str()); } } -void -TaskSchedulerTest::testRemoveTask() +TEST(TaskSchedulerTest, test_remove_task) { FastOS_ThreadPool threadPool(128 * 1024); TestWatch watch(0); @@ -236,7 +219,7 @@ TaskSchedulerTest::testRemoveTask() scheduler.remove(task); delete task; // Time should not be advanced as task didn't get to run - CPPUNIT_ASSERT_EQUAL(0, (int) watch.getTime()); + EXPECT_EQ(0, (int) watch.getTime()); } } diff --git a/vespalib/src/vespa/vespalib/testkit/test_hook.cpp b/vespalib/src/vespa/vespalib/testkit/test_hook.cpp index 0b627f8d0bf..b6020dcb879 100644 --- a/vespalib/src/vespa/vespalib/testkit/test_hook.cpp +++ b/vespalib/src/vespa/vespalib/testkit/test_hook.cpp @@ -98,7 +98,7 @@ TestHook::runAll() FastOSTestThreadFactory threadFactory; TestThreadFactory::factory = &threadFactory; std::string name = TestMaster::master.getName(); - std::basic_regex<char> pattern(lookup_subset_pattern(name), std::regex::extended); + std::regex pattern(lookup_subset_pattern(name), std::regex::extended); size_t testsPassed = 0; size_t testsFailed = 0; size_t testsIgnored = 0; diff --git a/vsm/src/vespa/vsm/vsm/fieldsearchspec.cpp b/vsm/src/vespa/vsm/vsm/fieldsearchspec.cpp index b12282910bf..bb30cdd89e9 100644 --- a/vsm/src/vespa/vsm/vsm/fieldsearchspec.cpp +++ b/vsm/src/vespa/vsm/vsm/fieldsearchspec.cpp @@ -156,9 +156,9 @@ FieldSearchSpecMap::~FieldSearchSpecMap() {} namespace { const std::string _G_empty(""); const std::string _G_value(".value"); - const std::basic_regex<char> _G_map1("\\{[a-zA-Z0-9]+\\}"); - const std::basic_regex<char> _G_map2("\\{\".*\"\\}"); - const std::basic_regex<char> _G_array("\\[[0-9]+\\]"); + const std::regex _G_map1("\\{[a-zA-Z0-9]+\\}"); + const std::regex _G_map2("\\{\".*\"\\}"); + const std::regex _G_array("\\[[0-9]+\\]"); } vespalib::string FieldSearchSpecMap::stripNonFields(const vespalib::string & rawIndex) |