diff options
45 files changed, 417 insertions, 464 deletions
diff --git a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/StateRestApiV2Handler.java b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/StateRestApiV2Handler.java index 431fc797df6..eea085bd103 100644 --- a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/StateRestApiV2Handler.java +++ b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/StateRestApiV2Handler.java @@ -3,8 +3,6 @@ package com.yahoo.vespa.clustercontroller.apps.clustercontroller; import com.google.inject.Inject; import com.yahoo.cloud.config.ClusterInfoConfig; -import com.yahoo.cloud.config.ModelConfig; -import com.yahoo.container.jdisc.config.HttpServerConfig; import com.yahoo.container.logging.AccessLog; import com.yahoo.log.LogLevel; import com.yahoo.vespa.clustercontroller.apputil.communication.http.JDiscHttpRequestHandler; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java b/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java index 6d4945f23ad..b4b89278c51 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java @@ -8,7 +8,15 @@ import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.vespa.defaults.Defaults; import com.yahoo.vespa.filedistribution.PathDoesNotExistException; -import java.util.*; +import java.util.Collection; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import java.util.Set; import static com.yahoo.text.Lowercase.toLowerCase; @@ -66,7 +74,7 @@ public abstract class AbstractService extends AbstractConfigProducer<AbstractCon * more key/value pairs to the service-list dump. * Supported key datatypes are String, and values may be String or Integer. */ - private HashMap<String, Object> serviceProperties = new LinkedHashMap<>(); + private Map<String, Object> serviceProperties = new LinkedHashMap<>(); /** The affinity properties of this service. */ private Optional<Affinity> affinity = Optional.empty(); @@ -522,15 +530,13 @@ public abstract class AbstractService extends AbstractConfigProducer<AbstractCon * The service HTTP port for health status * @return portnumber */ - public int getHealthPort() {return -1;} + public int getHealthPort() { return -1;} /** * Overridden by subclasses. List of default dimensions to be added to this services metrics * @return The default dimensions for this service */ - public HashMap<String, String> getDefaultMetricDimensions(){ - return new LinkedHashMap<>(); - } + public HashMap<String, String> getDefaultMetricDimensions(){ return new LinkedHashMap<>(); } // For testing public int getNumPortsAllocated() { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java index b54978f52d3..c4cfd6d185b 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java @@ -204,7 +204,7 @@ public class ContentSearchCluster extends AbstractConfigProducer implements Prot return hasIndexedCluster() ? getIndexed().getSearchNodes() : nonIndexed; } - public SearchNode addSearchNode(ContentNode node, StorageGroup parentGroup, ModelElement element) { + public void addSearchNode(ContentNode node, StorageGroup parentGroup, ModelElement element) { AbstractConfigProducer parent = hasIndexedCluster() ? getIndexed() : this; NodeSpec spec = getNextSearchNodeSpec(parentGroup); @@ -229,7 +229,6 @@ public class ContentSearchCluster extends AbstractConfigProducer implements Prot } else { nonIndexed.add(snode); } - return snode; } /** Translates group ids to continuous 0-base "row" id integers */ diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/EngineFactoryBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/EngineFactoryBuilder.java index 95193b03764..04c5fd4fd72 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/EngineFactoryBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/EngineFactoryBuilder.java @@ -22,8 +22,6 @@ public class EngineFactoryBuilder { return new ProtonEngine.Factory(c.getSearch()); } else if (persistence.getChild("dummy") != null) { return new com.yahoo.vespa.model.content.engines.DummyPersistence.Factory(); - } else if (persistence.getChild("rpc") != null) { - return new RPCEngine.Factory(); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/engines/ProtonEngine.java b/config-model/src/main/java/com/yahoo/vespa/model/content/engines/ProtonEngine.java index 9c655e62d32..df8cfc6f9bd 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/engines/ProtonEngine.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/engines/ProtonEngine.java @@ -22,8 +22,8 @@ public class ProtonEngine { @Override public PersistenceEngine create(StorageNode storageNode, StorageGroup parentGroup, ModelElement storageNodeElement) { - SearchNode searchNode = search.addSearchNode(storageNode, parentGroup, storageNodeElement); - return new ProtonProvider(storageNode, searchNode); + search.addSearchNode(storageNode, parentGroup, storageNodeElement); + return new ProtonProvider(storageNode); } @Override diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/engines/ProtonProvider.java b/config-model/src/main/java/com/yahoo/vespa/model/content/engines/ProtonProvider.java index 9d6bccee7e4..ff3b4891146 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/engines/ProtonProvider.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/engines/ProtonProvider.java @@ -1,14 +1,23 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.content.engines; +import com.yahoo.vespa.config.content.core.StorServerConfig; import com.yahoo.vespa.model.content.StorageNode; import com.yahoo.vespa.model.search.SearchNode; /** * @author baldersheim */ -public class ProtonProvider extends RPCEngine { - public ProtonProvider(StorageNode parent, SearchNode searchNode) { - super(parent, searchNode); +public class ProtonProvider extends PersistenceEngine { + + public ProtonProvider(StorageNode parent) { + super(parent, "provider"); + } + + @Override + public void getConfig(StorServerConfig.Builder builder) { + StorServerConfig.Persistence_provider.Builder provider = new StorServerConfig.Persistence_provider.Builder(); + provider.type(StorServerConfig.Persistence_provider.Type.Enum.RPC); + builder.persistence_provider(provider); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/engines/RPCEngine.java b/config-model/src/main/java/com/yahoo/vespa/model/content/engines/RPCEngine.java deleted file mode 100644 index 8be2c0c4dd6..00000000000 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/engines/RPCEngine.java +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.model.content.engines; - -import com.yahoo.vespa.config.content.core.StorServerConfig; -import com.yahoo.vespa.model.builder.xml.dom.ModelElement; -import com.yahoo.vespa.model.content.StorageGroup; -import com.yahoo.vespa.model.content.StorageNode; -import com.yahoo.vespa.model.content.cluster.ContentCluster; -import com.yahoo.vespa.model.search.SearchNode; - -public class RPCEngine extends PersistenceEngine { - - private SearchNode searchNode; - public RPCEngine(StorageNode parent) { - super(parent, "provider"); - } - - public RPCEngine(StorageNode parent, SearchNode searchNode) { - super(parent, "provider"); - this.searchNode = searchNode; - } - - @Override - public void getConfig(StorServerConfig.Builder builder) { - StorServerConfig.Persistence_provider.Builder provider = - new StorServerConfig.Persistence_provider.Builder(); - provider.type(StorServerConfig.Persistence_provider.Type.Enum.RPC); - - if (searchNode != null) { - provider.rpc(new StorServerConfig.Persistence_provider.Rpc.Builder().connectspec("tcp/localhost:" + searchNode.getPersistenceProviderRpcPort())); - } - - builder.persistence_provider(provider); - } - - public static class Factory implements PersistenceFactory { - @Override - public PersistenceEngine create(StorageNode storageNode, StorageGroup parentGroup, ModelElement storageNodeElement) { - return new RPCEngine(storageNode); - } - - @Override - public boolean supportRevert() { - return false; - } - - @Override - public boolean enableMultiLevelSplitting() { - return false; - } - - @Override - public ContentCluster.DistributionMode getDefaultDistributionMode() { - return ContentCluster.DistributionMode.LOOSE; - } - } -} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java b/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java index 66abf3e23cf..9021120d570 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java @@ -56,6 +56,9 @@ public class SearchNode extends AbstractService implements private TransactionLogServer tls; private AbstractService serviceLayerService; private final Optional<Tuning> tuning; + private static final int RPC_PORT = 0; + private static final int FS4_PORT = 1; + private static final int HEALTH_PORT = 2; public static class Builder extends VespaDomBuilder.DomConfigProducerBuilder<SearchNode> { @@ -101,11 +104,9 @@ public class SearchNode extends AbstractService implements this.nodeSpec = nodeSpec; this.clusterName = clusterName; this.flushOnShutdown = flushOnShutdown; - portsMeta.on(0).tag("rpc").tag("rtc").tag("admin").tag("status"); - portsMeta.on(1).tag("fs4"); - portsMeta.on(2).tag("srmp").tag("hack").tag("test"); - portsMeta.on(3).tag("rpc").tag("engines-provider"); - portsMeta.on(4).tag("http").tag("json").tag("health").tag("state"); + portsMeta.on(RPC_PORT).tag("rpc").tag("rtc").tag("admin").tag("status"); + portsMeta.on(FS4_PORT).tag("fs4"); + portsMeta.on(HEALTH_PORT).tag("http").tag("json").tag("health").tag("state"); // Properties are set in DomSearchBuilder monitorService(); this.tuning = tuning; @@ -139,15 +140,6 @@ public class SearchNode extends AbstractService implements } /** - * Returns the connection spec string that resolves to this search node. - * - * @return The connection string. - */ - public String getConnectSpec() { - return "tcp/" + getHost().getHostName() + ":" + getRpcPort(); - } - - /** * Returns the number of ports needed by this service. * * @return The number of ports. @@ -163,20 +155,7 @@ public class SearchNode extends AbstractService implements * @return The port. */ public int getRpcPort() { - return getRelativePort(0); - } - - protected int getSlimeMessagingPort() { - return getRelativePort(2); - } - - /* - * Returns the rpc port used for the engines provider interface. - * @return The port - */ - - public int getPersistenceProviderRpcPort() { - return getRelativePort(3); + return getRelativePort(RPC_PORT); } @Override @@ -204,11 +183,11 @@ public class SearchNode extends AbstractService implements } public int getDispatchPort() { - return getRelativePort(1); + return getRelativePort(FS4_PORT); } public int getHttpPort() { - return getRelativePort(4); + return getRelativePort(HEALTH_PORT); } @Override @@ -258,8 +237,6 @@ public class SearchNode extends AbstractService implements builder. ptport(getDispatchPort()). rpcport(getRpcPort()). - slime_messaging_port(getSlimeMessagingPort()). - rtcspec(getConnectSpec()). httpport(getHttpPort()). partition(getNodeSpec().partitionId()). clustername(getClusterName()). diff --git a/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java index e1818008462..043f961f98e 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java @@ -318,8 +318,6 @@ public class ContentBuilderTest extends DomBuilderTest { assertTrue(partitionsConfig.dataset(0).engine(0).name_and_port().startsWith("tcp/node0:191")); IndexedSearchCluster sc = m.getContentClusters().get("clu").getSearch().getIndexed(); assertEquals(2, sc.getSearchNodeCount()); - assertTrue(sc.getSearchNode(0).getPersistenceProviderRpcPort() >= 19100); - assertTrue(sc.getSearchNode(0).getPersistenceProviderRpcPort() != sc.getSearchNode(1).getPersistenceProviderRpcPort()); } @Test diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ClusterTest.java index 5ba0c43fcee..2a3dbe002e6 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/ClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ClusterTest.java @@ -506,7 +506,6 @@ public class ClusterTest extends ContentBaseTest { cluster.getStorageNodes().getConfig(builder); cluster.getStorageNodes().getChildren().get("0").getConfig(builder); StorServerConfig config = new StorServerConfig(builder); - assertEquals("tcp/localhost:19106", config.persistence_provider().rpc().connectspec()); } { @@ -514,7 +513,6 @@ public class ClusterTest extends ContentBaseTest { cluster.getStorageNodes().getConfig(builder); cluster.getStorageNodes().getChildren().get("1").getConfig(builder); StorServerConfig config = new StorServerConfig(builder); - assertEquals("tcp/localhost:19118", config.persistence_provider().rpc().connectspec()); } } @@ -602,7 +600,6 @@ public class ClusterTest extends ContentBaseTest { @Test public void testProviders() { testProvider("proton", StorServerConfig.Persistence_provider.Type.RPC); - testProvider("rpc", StorServerConfig.Persistence_provider.Type.RPC); testProvider("vds", StorServerConfig.Persistence_provider.Type.STORAGE); testProvider("dummy", StorServerConfig.Persistence_provider.Type.DUMMY); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/IndexedTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/IndexedTest.java index c1ed602f791..14e3bd72dc7 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/IndexedTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/IndexedTest.java @@ -198,7 +198,6 @@ public class IndexedTest extends ContentBaseTest { StorServerConfig.Builder builder = new StorServerConfig.Builder(); s.getStorageNodes().getConfig(builder); s.getStorageNodes().getChildren().get("3").getConfig(builder); - assertTrue(new StorServerConfig(builder).persistence_provider().rpc().connectspec().startsWith("tcp/localhost:191")); } @Test diff --git a/config/src/vespa/config/frt/frtconfigresponsev3.cpp b/config/src/vespa/config/frt/frtconfigresponsev3.cpp index 3791d3c55b7..379ebbd1803 100644 --- a/config/src/vespa/config/frt/frtconfigresponsev3.cpp +++ b/config/src/vespa/config/frt/frtconfigresponsev3.cpp @@ -58,11 +58,13 @@ FRTConfigResponseV3::readConfigValue() const Slime * rawData = new Slime(); SlimePtr payloadData(rawData); DecompressedData data(decompress(((*_returnValues)[1]._data._buf), ((*_returnValues)[1]._data._len), info.compressionType, info.uncompressedSize)); - size_t consumedSize = JsonFormat::decode(data.memRef, *rawData); - if (consumedSize != data.size) { - std::string json(make_json(*payloadData, true)); - LOG(error, "Error decoding JSON. Consumed size: %lu, uncompressed size: %u, compression type: %s, assumed uncompressed size(%u), compressed size: %u, slime(%s)", consumedSize, data.size, compressionTypeToString(info.compressionType).c_str(), info.uncompressedSize, ((*_returnValues)[1]._data._len), json.c_str()); - assert(false); + if (data.memRef.size > 0) { + size_t consumedSize = JsonFormat::decode(data.memRef, *rawData); + if (consumedSize == 0) { + std::string json(make_json(*payloadData, true)); + LOG(error, "Error decoding JSON. Consumed size: %lu, uncompressed size: %u, compression type: %s, assumed uncompressed size(%u), compressed size: %u, slime(%s)", consumedSize, data.size, compressionTypeToString(info.compressionType).c_str(), info.uncompressedSize, ((*_returnValues)[1]._data._len), json.c_str()); + assert(false); + } } if (LOG_WOULD_LOG(spam)) { LOG(spam, "read config value md5(%s), payload size: %lu", md5.c_str(), data.memRef.size); diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java b/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java index e59f012856a..fa2ee8e89a9 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java @@ -42,7 +42,6 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Level; import java.util.logging.Logger; @@ -80,7 +79,7 @@ public final class ConfiguredApplication implements Application { new ComponentRegistry<>(), new ComponentRegistry<>()); private final OsgiFramework restrictedOsgiFramework; - private final AtomicInteger applicationSerialNo = new AtomicInteger(0); + private volatile int applicationSerialNo = 0; private HandlersConfigurerDi configurer; private ScheduledThreadPoolExecutor shutdownDeadlineExecutor; private Thread reconfigurerThread; @@ -173,12 +172,10 @@ public final class ConfiguredApplication implements Application { startAndStopServers(); log.info("Switching to the latest deployed set of configurations and components. " + - "Application switch number: " + applicationSerialNo.getAndIncrement()); + "Application switch number: " + (applicationSerialNo++)); } private ContainerBuilder createBuilderWithGuiceBindings() { - log.info("Initializing new set of configurations and components. " + - "Application switch number: " + applicationSerialNo.get()); ContainerBuilder builder = activator.newContainerBuilder(); setupGuiceBindings(builder.guiceModules()); return builder; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java index 5cc4c441c3b..f68ce3ebfa5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java @@ -16,6 +16,7 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobReport; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType; import java.time.Instant; +import java.util.Collections; import java.util.Comparator; import java.util.LinkedHashMap; import java.util.List; @@ -43,8 +44,9 @@ public class Application { /** Creates an empty application */ public Application(ApplicationId id) { - this(id, DeploymentSpec.empty, ValidationOverrides.empty, ImmutableMap.of(), new DeploymentJobs(0L), - Optional.empty(), false); // TODO: Get rid of the 0 + this(id, DeploymentSpec.empty, ValidationOverrides.empty, ImmutableMap.of(), + new DeploymentJobs(Optional.empty(), Collections.emptyList(), Optional.empty()), + Optional.empty(), false); } /** Used from persistence layer: Do not use */ @@ -52,7 +54,7 @@ public class Application { List<Deployment> deployments, DeploymentJobs deploymentJobs, Optional<Change> deploying, boolean outstandingChange) { this(id, deploymentSpec, validationOverrides, - deployments.stream().collect(Collectors.toMap(d -> d.zone(), d -> d)), + deployments.stream().collect(Collectors.toMap(Deployment::zone, d -> d)), deploymentJobs, deploying, outstandingChange); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java index af8617cbf05..26bef06adcf 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java @@ -32,24 +32,20 @@ public class DeploymentJobs { private final ImmutableMap<JobType, JobStatus> status; private final Optional<String> jiraIssueId; - /** Creates an empty set of deployment jobs */ - public DeploymentJobs(long projectId) { - this(Optional.of(projectId), ImmutableMap.of(), Optional.empty()); - } - - public DeploymentJobs(Optional<Long> projectId, Collection<JobStatus> jobStatusEntries, Optional<String> jiraIssueId) { + public DeploymentJobs(Optional<Long> projectId, Collection<JobStatus> jobStatusEntries, + Optional<String> jiraIssueId) { this(projectId, asMap(jobStatusEntries), jiraIssueId); } - + private DeploymentJobs(Optional<Long> projectId, Map<JobType, JobStatus> status, Optional<String> jiraIssueId) { - Objects.requireNonNull(projectId, "projectId cannot be null"); + requireId(projectId, "projectId cannot be null or <= 0"); Objects.requireNonNull(status, "status cannot be null"); Objects.requireNonNull(jiraIssueId, "jiraIssueId cannot be null"); this.projectId = projectId; this.status = ImmutableMap.copyOf(status); this.jiraIssueId = jiraIssueId; } - + private static Map<JobType, JobStatus> asMap(Collection<JobStatus> jobStatusEntries) { ImmutableMap.Builder<JobType, JobStatus> b = new ImmutableMap.Builder<>(); for (JobStatus jobStatusEntry : jobStatusEntries) @@ -306,4 +302,15 @@ public class DeploymentJobs { } } + private static Optional<Long> requireId(Optional<Long> id, String message) { + Objects.requireNonNull(id, message); + if (!id.isPresent()) { + return id; + } + if (id.get() <= 0) { + throw new IllegalArgumentException(message); + } + return id; + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java index 14fce0987b7..81b3cb635ef 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java @@ -121,7 +121,9 @@ public class ApplicationSerializer { } private void toSlime(DeploymentJobs deploymentJobs, Cursor cursor) { - deploymentJobs.projectId().ifPresent(projectId -> cursor.setLong(projectIdField, projectId)); + deploymentJobs.projectId() + .filter(id -> id > 0) // TODO: Discards invalid data. Remove filter after October 2017 + .ifPresent(projectId -> cursor.setLong(projectIdField, projectId)); jobStatusToSlime(deploymentJobs.jobStatus().values(), cursor.setArray(jobStatusField)); deploymentJobs.jiraIssueId().ifPresent(jiraIssueId -> cursor.setString(jiraIssueIdField, jiraIssueId)); } @@ -213,7 +215,8 @@ public class ApplicationSerializer { } private DeploymentJobs deploymentJobsFromSlime(Inspector object) { - Optional<Long> projectId = optionalLong(object.field(projectIdField)); + Optional<Long> projectId = optionalLong(object.field(projectIdField)) + .filter(id -> id > 0); // TODO: Discards invalid data. Remove filter after October 2017 List<JobStatus> jobStatusList = jobStatusListFromSlime(object.field(jobStatusField)); Optional<String> jiraIssueKey = optionalString(object.field(jiraIssueIdField)); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java index 5be030b4fd9..bc7b017dd7d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java @@ -39,12 +39,12 @@ public class UpgraderTest { assertEquals("No applications: Nothing to do", 0, tester.buildSystem().jobs().size()); // Setup applications - Application canary0 = tester.createAndDeploy("canary0", 0, "canary"); - Application canary1 = tester.createAndDeploy("canary1", 1, "canary"); - Application default0 = tester.createAndDeploy("default0", 2, "default"); - Application default1 = tester.createAndDeploy("default1", 3, "default"); - Application default2 = tester.createAndDeploy("default2", 4, "default"); - Application conservative0 = tester.createAndDeploy("conservative0", 5, "conservative"); + Application canary0 = tester.createAndDeploy("canary0", 1, "canary"); + Application canary1 = tester.createAndDeploy("canary1", 2, "canary"); + Application default0 = tester.createAndDeploy("default0", 3, "default"); + Application default1 = tester.createAndDeploy("default1", 4, "default"); + Application default2 = tester.createAndDeploy("default2", 5, "default"); + Application conservative0 = tester.createAndDeploy("conservative0", 6, "conservative"); tester.upgrader().maintain(); assertEquals("All already on the right version: Nothing to do", 0, tester.buildSystem().jobs().size()); @@ -163,18 +163,18 @@ public class UpgraderTest { assertEquals("No applications: Nothing to do", 0, tester.buildSystem().jobs().size()); // Setup applications - Application canary0 = tester.createAndDeploy("canary0", 0, "canary"); - Application canary1 = tester.createAndDeploy("canary1", 1, "canary"); - Application default0 = tester.createAndDeploy("default0", 2, "default"); - Application default1 = tester.createAndDeploy("default1", 3, "default"); - Application default2 = tester.createAndDeploy("default2", 4, "default"); - Application default3 = tester.createAndDeploy("default3", 5, "default"); - Application default4 = tester.createAndDeploy("default4", 6, "default"); - Application default5 = tester.createAndDeploy("default5", 7, "default"); - Application default6 = tester.createAndDeploy("default6", 8, "default"); - Application default7 = tester.createAndDeploy("default7", 9, "default"); - Application default8 = tester.createAndDeploy("default8", 10, "default"); - Application default9 = tester.createAndDeploy("default9", 11, "default"); + Application canary0 = tester.createAndDeploy("canary0", 1, "canary"); + Application canary1 = tester.createAndDeploy("canary1", 2, "canary"); + Application default0 = tester.createAndDeploy("default0", 3, "default"); + Application default1 = tester.createAndDeploy("default1", 4, "default"); + Application default2 = tester.createAndDeploy("default2", 5, "default"); + Application default3 = tester.createAndDeploy("default3", 6, "default"); + Application default4 = tester.createAndDeploy("default4", 7, "default"); + Application default5 = tester.createAndDeploy("default5", 8, "default"); + Application default6 = tester.createAndDeploy("default6", 9, "default"); + Application default7 = tester.createAndDeploy("default7", 10, "default"); + Application default8 = tester.createAndDeploy("default8", 11, "default"); + Application default9 = tester.createAndDeploy("default9", 12, "default"); tester.upgrader().maintain(); assertEquals("All already on the right version: Nothing to do", 0, tester.buildSystem().jobs().size()); @@ -275,13 +275,13 @@ public class UpgraderTest { tester.updateVersionStatus(version); // Setup applications - Application canary0 = tester.createAndDeploy("canary0", 0, "canary"); - Application canary1 = tester.createAndDeploy("canary1", 1, "canary"); - Application default0 = tester.createAndDeploy("default0", 2, "default"); - Application default1 = tester.createAndDeploy("default1", 3, "default"); - Application default2 = tester.createAndDeploy("default2", 4, "default"); - Application default3 = tester.createAndDeploy("default3", 5, "default"); - Application default4 = tester.createAndDeploy("default4", 6, "default"); + Application canary0 = tester.createAndDeploy("canary0", 1, "canary"); + Application canary1 = tester.createAndDeploy("canary1", 2, "canary"); + Application default0 = tester.createAndDeploy("default0", 3, "default"); + Application default1 = tester.createAndDeploy("default1", 4, "default"); + Application default2 = tester.createAndDeploy("default2", 5, "default"); + Application default3 = tester.createAndDeploy("default3", 6, "default"); + Application default4 = tester.createAndDeploy("default4", 7, "default"); // New version is released version = Version.fromString("5.1"); @@ -321,13 +321,13 @@ public class UpgraderTest { tester.updateVersionStatus(version); // Setup applications - Application canary0 = tester.createAndDeploy("canary0", 0, "canary"); - Application canary1 = tester.createAndDeploy("canary1", 1, "canary"); - Application default0 = tester.createAndDeploy("default0", 2, "default"); - Application default1 = tester.createAndDeploy("default1", 3, "default"); - Application default2 = tester.createAndDeploy("default2", 4, "default"); - Application default3 = tester.createAndDeploy("default3", 5, "default"); - Application default4 = tester.createAndDeploy("default4", 5, "default"); + Application canary0 = tester.createAndDeploy("canary0", 1, "canary"); + Application canary1 = tester.createAndDeploy("canary1", 2, "canary"); + Application default0 = tester.createAndDeploy("default0", 3, "default"); + Application default1 = tester.createAndDeploy("default1", 4, "default"); + Application default2 = tester.createAndDeploy("default2", 5, "default"); + Application default3 = tester.createAndDeploy("default3", 6, "default"); + Application default4 = tester.createAndDeploy("default4", 7, "default"); // New version is released version = Version.fromString("5.1"); @@ -412,13 +412,13 @@ public class UpgraderTest { .build(); // Setup applications - Application canary0 = tester.createAndDeploy("canary0", 0, "canary"); - Application canary1 = tester.createAndDeploy("canary1", 1, "canary"); - Application default0 = tester.createAndDeploy("default0", 2, "default"); - Application default1 = tester.createAndDeploy("default1", 3, "default"); - Application default2 = tester.createAndDeploy("default2", 4, "default"); - Application default3 = tester.createAndDeploy("default3", 5, "default"); - Application default4 = tester.createAndDeploy("default4", 6, "default"); + Application canary0 = tester.createAndDeploy("canary0", 1, "canary"); + Application canary1 = tester.createAndDeploy("canary1", 2, "canary"); + Application default0 = tester.createAndDeploy("default0", 3, "default"); + Application default1 = tester.createAndDeploy("default1", 4, "default"); + Application default2 = tester.createAndDeploy("default2", 5, "default"); + Application default3 = tester.createAndDeploy("default3", 6, "default"); + Application default4 = tester.createAndDeploy("default4", 7, "default"); // New version is released version = Version.fromString("5.1"); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java index f0a2019d41e..4df7f343522 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java @@ -138,17 +138,30 @@ public class ApplicationSerializerTest { assertFalse(application.deploymentJobs().jobStatus().get(DeploymentJobs.JobType.systemTest).lastCompleted().get().upgrade()); } + // TODO: Remove after October 2017 + @Test + public void testLegacySerializationWithZeroProjectId() { + Application original = applicationSerializer.fromSlime(applicationSlime(0, false)); + assertFalse(original.deploymentJobs().projectId().isPresent()); + Application serialized = applicationSerializer.fromSlime(applicationSerializer.toSlime(original)); + assertFalse(serialized.deploymentJobs().projectId().isPresent()); + } + private Slime applicationSlime(boolean error) { - return SlimeUtils.jsonToSlime(applicationJson(error).getBytes(StandardCharsets.UTF_8)); + return applicationSlime(123, error); + } + + private Slime applicationSlime(long projectId, boolean error) { + return SlimeUtils.jsonToSlime(applicationJson(projectId, error).getBytes(StandardCharsets.UTF_8)); } - private String applicationJson(boolean error) { + private String applicationJson(long projectId, boolean error) { return "{\n" + " \"id\": \"t1:a1:i1\",\n" + " \"deploymentSpecField\": \"<deployment version='1.0'/>\",\n" + " \"deploymentJobs\": {\n" + - " \"projectId\": 123,\n" + + " \"projectId\": " + projectId + ",\n" + " \"jobStatus\": [\n" + " {\n" + " \"jobType\": \"system-test\",\n" + diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java index 4fc6e91039c..b55ee9a195f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java @@ -78,8 +78,8 @@ public class ContainerTester { public void assertResponse(Request request, String expectedResponse, int expectedStatusCode) throws IOException { Response response = container.handleRequest(request); - assertEquals("Status code", expectedStatusCode, response.getStatus()); assertEquals(expectedResponse, response.getBodyAsString()); + assertEquals("Status code", expectedStatusCode, response.getStatus()); } private Set<String> fieldsToCensor(String fieldNameOrNull, Inspector value, Set<String> fieldsToCensor) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index 50ad84028f1..18073ef06f3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -131,27 +131,27 @@ public class ApplicationApiTest extends ControllerContainerTest { new File("application-deployment.json")); // ... systemtest - tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/test/region/test-region/instance/default/", + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/test/region/us-east-1/instance/default/", createApplicationDeployData(applicationPackage, Optional.of(screwdriverProjectId)), Request.Method.POST, athensScrewdriverDomain, "screwdriveruser1"), new File("deploy-result.json")); - tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/test/region/test-region/instance/default", + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/test/region/us-east-1/instance/default", "", Request.Method.DELETE), - "Deactivated tenant/tenant1/application/application1/environment/test/region/test-region/instance/default"); + "Deactivated tenant/tenant1/application/application1/environment/test/region/us-east-1/instance/default"); controllerTester.notifyJobCompletion(id, screwdriverProjectId, true, DeploymentJobs.JobType.systemTest); // Called through the separate screwdriver/v1 API // ... staging - tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/staging/region/staging-region/instance/default/", + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/staging/region/us-east-3/instance/default/", createApplicationDeployData(applicationPackage, Optional.of(screwdriverProjectId)), Request.Method.POST, athensScrewdriverDomain, "screwdriveruser1"), new File("deploy-result.json")); - tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/staging/region/staging-region/instance/default", + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/staging/region/us-east-3/instance/default", "", Request.Method.DELETE), - "Deactivated tenant/tenant1/application/application1/environment/staging/region/staging-region/instance/default"); + "Deactivated tenant/tenant1/application/application1/environment/staging/region/us-east-3/instance/default"); controllerTester.notifyJobCompletion(id, screwdriverProjectId, true, DeploymentJobs.JobType.stagingTest); // ... prod zone diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json index 5df690f5bc7..cc17e76642f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json @@ -8,71 +8,119 @@ "success": true, "lastTriggered": { "version": "(ignore)", + "revision": { + "hash": "(ignore)", + "source": { + "gitRepository": "repository1", + "gitBranch": "master", + "gitCommit": "commit1" + } + }, "at": "(ignore)" }, "lastCompleted": { "version": "(ignore)", + "revision": { + "hash": "(ignore)", + "source": { + "gitRepository": "repository1", + "gitBranch": "master", + "gitCommit": "commit1" + } + }, "at": "(ignore)" }, "lastSuccess": { "version": "(ignore)", + "revision": { + "hash": "(ignore)", + "source": { + "gitRepository": "repository1", + "gitBranch": "master", + "gitCommit": "commit1" + } + }, "at": "(ignore)" } }, { - "type":"staging-test", - "success":true, - "lastTriggered":{ - "version":"(ignore)", - "at":"(ignore)" + "type": "staging-test", + "success": true, + "lastTriggered": { + "version": "(ignore)", + "revision": { + "hash": "(ignore)", + "source": { + "gitRepository": "repository1", + "gitBranch": "master", + "gitCommit": "commit1" + } + }, + "at": "(ignore)" }, - "lastCompleted":{ - "version":"(ignore)", - "at":"(ignore)" + "lastCompleted": { + "version": "(ignore)", + "revision": { + "hash": "(ignore)", + "source": { + "gitRepository": "repository1", + "gitBranch": "master", + "gitCommit": "commit1" + } + }, + "at": "(ignore)" }, - "lastSuccess":{ - "version":"(ignore)", - "at":"(ignore)" + "lastSuccess": { + "version": "(ignore)", + "revision": { + "hash": "(ignore)", + "source": { + "gitRepository": "repository1", + "gitBranch": "master", + "gitCommit": "commit1" + } + }, + "at": "(ignore)" } }, { - "type":"production-corp-us-east-1", - "success":false, - "lastTriggered":{ - "version":"(ignore)", - "revision":{ - "hash":"(ignore)", - "source":{ - "gitRepository":"repository1", - "gitBranch":"master", - "gitCommit":"commit1" + "type": "production-corp-us-east-1", + "success": false, + "lastTriggered": { + "version": "(ignore)", + "revision": { + "hash": "(ignore)", + "source": { + "gitRepository": "repository1", + "gitBranch": "master", + "gitCommit": "commit1" } }, - "at":"(ignore)" + "at": "(ignore)" }, - "lastCompleted":{ - "version":"(ignore)", - "revision":{ - "hash":"(ignore)", - "source":{ - "gitRepository":"repository1", - "gitBranch":"master", - "gitCommit":"commit1" + "lastCompleted": { + "version": "(ignore)", + "revision": { + "hash": "(ignore)", + "source": { + "gitRepository": "repository1", + "gitBranch": "master", + "gitCommit": "commit1" } }, - "at":"(ignore)" + "at": "(ignore)" }, - "firstFailing":{ - "version":"(ignore)", - "revision":{ - "hash":"(ignore)", - "source":{ - "gitRepository":"repository1", - "gitBranch":"master", - "gitCommit":"commit1" + "firstFailing": { + "version": "(ignore)", + "revision": { + "hash": "(ignore)", + "source": { + "gitRepository": "repository1", + "gitBranch": "master", + "gitCommit": "commit1" } }, - "at":"(ignore)" + "at": "(ignore)" } } ], @@ -91,14 +139,14 @@ "environment": "prod", "region": "corp-us-east-1", "instance": "default", - "bcpStatus": {"rotationStatus":"UNKNOWN"}, + "bcpStatus": { + "rotationStatus": "UNKNOWN" + }, "url": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default" } - ] - , - "metrics": - { - "queryServiceQuality":0.5, - "writeServiceQuality":0.7 - } + ], + "metrics": { + "queryServiceQuality": 0.5, + "writeServiceQuality": 0.7 + } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java index 6071776edf5..7bbbf8f0499 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java @@ -131,20 +131,20 @@ public class VersionStatusTest { tester.upgradeSystem(version0); // Setup applications - Application canary0 = tester.createAndDeploy("canary0", 0, "canary"); - Application canary1 = tester.createAndDeploy("canary1", 1, "canary"); - Application canary2 = tester.createAndDeploy("canary2", 2, "canary"); - Application default0 = tester.createAndDeploy("default0", 00, "default"); - Application default1 = tester.createAndDeploy("default1", 11, "default"); - Application default2 = tester.createAndDeploy("default2", 22, "default"); - Application default3 = tester.createAndDeploy("default3", 33, "default"); - Application default4 = tester.createAndDeploy("default4", 44, "default"); - Application default5 = tester.createAndDeploy("default5", 55, "default"); - Application default6 = tester.createAndDeploy("default6", 66, "default"); - Application default7 = tester.createAndDeploy("default7", 77, "default"); - Application default8 = tester.createAndDeploy("default8", 88, "default"); - Application default9 = tester.createAndDeploy("default9", 99, "default"); - Application conservative0 = tester.createAndDeploy("conservative1", 000, "conservative"); + Application canary0 = tester.createAndDeploy("canary0", 1, "canary"); + Application canary1 = tester.createAndDeploy("canary1", 2, "canary"); + Application canary2 = tester.createAndDeploy("canary2", 3, "canary"); + Application default0 = tester.createAndDeploy("default0", 4, "default"); + Application default1 = tester.createAndDeploy("default1", 5, "default"); + Application default2 = tester.createAndDeploy("default2", 6, "default"); + Application default3 = tester.createAndDeploy("default3", 7, "default"); + Application default4 = tester.createAndDeploy("default4", 8, "default"); + Application default5 = tester.createAndDeploy("default5", 9, "default"); + Application default6 = tester.createAndDeploy("default6", 10, "default"); + Application default7 = tester.createAndDeploy("default7", 11, "default"); + Application default8 = tester.createAndDeploy("default8", 12, "default"); + Application default9 = tester.createAndDeploy("default9", 13, "default"); + Application conservative0 = tester.createAndDeploy("conservative1", 14, "conservative"); // The following applications should not affect confidence calculation: diff --git a/dist/release-vespa-rpm.sh b/dist/release-vespa-rpm.sh index ba9195dff05..eaf91468c38 100755 --- a/dist/release-vespa-rpm.sh +++ b/dist/release-vespa-rpm.sh @@ -34,6 +34,11 @@ mv pom.xml pom.xml.hide # Run tito to update spec file and tag tito tag --use-version=$VERSION --no-auto-changelog +# Remove '-1' suffix from tag generated by tito +readonly TITO_GIT_TAG="vespa-${VERSION}-1" +git tag "vespa-${VERSION}" ${TITO_GIT_TAG} +git tag -d ${TITO_GIT_TAG} + # Push changes and tag to branc git push -u origin --follow-tags $RPM_BRANCH diff --git a/eval/src/tests/tensor/tensor_slime_serialization/tensor_slime_serialization_test.cpp b/eval/src/tests/tensor/tensor_slime_serialization/tensor_slime_serialization_test.cpp index 916b6ad4462..e6e6c7de686 100644 --- a/eval/src/tests/tensor/tensor_slime_serialization/tensor_slime_serialization_test.cpp +++ b/eval/src/tests/tensor/tensor_slime_serialization/tensor_slime_serialization_test.cpp @@ -35,7 +35,7 @@ struct Fixture vespalib::Memory memory_exp(exp); vespalib::Slime expSlime; size_t used = vespalib::slime::JsonFormat::decode(memory_exp, expSlime); - EXPECT_EQUAL(used, memory_exp.size); + EXPECT_TRUE(used > 0); EXPECT_EQUAL(expSlime, *slime); } }; @@ -135,7 +135,7 @@ struct DenseFixture vespalib::Memory memory_exp(exp); vespalib::Slime expSlime; size_t used = vespalib::slime::JsonFormat::decode(memory_exp, expSlime); - EXPECT_EQUAL(used, memory_exp.size); + EXPECT_TRUE(used > 0); EXPECT_EQUAL(expSlime, *slime); } }; diff --git a/eval/src/vespa/eval/eval/test/tensor_conformance.cpp b/eval/src/vespa/eval/eval/test/tensor_conformance.cpp index 4e321083252..bd8bdf8cd11 100644 --- a/eval/src/vespa/eval/eval/test/tensor_conformance.cpp +++ b/eval/src/vespa/eval/eval/test/tensor_conformance.cpp @@ -1267,7 +1267,7 @@ struct TestContext { MappedFileInput file(path); Slime slime; EXPECT_TRUE(file.valid()); - EXPECT_EQUAL(JsonFormat::decode(file, slime), file.get().size); + EXPECT_TRUE(JsonFormat::decode(file, slime) > 0); int64_t num_tests = slime.get()["num_tests"].asLong(); Cursor &tests = slime.get()["tests"]; EXPECT_GREATER(num_tests, 0u); diff --git a/metrics/src/tests/metricmanagertest.cpp b/metrics/src/tests/metricmanagertest.cpp index a086cf70ca9..e9934b6dbbb 100644 --- a/metrics/src/tests/metricmanagertest.cpp +++ b/metrics/src/tests/metricmanagertest.cpp @@ -779,7 +779,7 @@ void MetricManagerTest::testJsonOutput() using namespace vespalib::slime; vespalib::Slime slime; size_t parsed = JsonFormat::decode(vespalib::Memory(jsonData), slime); - if (jsonData.size() != parsed) { + if (parsed == 0) { vespalib::SimpleBuffer buffer; JsonFormat::encode(slime, buffer, false); std::ostringstream ost; diff --git a/searchcore/src/tests/proton/docsummary/docsummary.cpp b/searchcore/src/tests/proton/docsummary/docsummary.cpp index 97a96c4bac6..aff27959ec9 100644 --- a/searchcore/src/tests/proton/docsummary/docsummary.cpp +++ b/searchcore/src/tests/proton/docsummary/docsummary.cpp @@ -443,12 +443,12 @@ Test::assertSlime(const std::string &exp, const DocsumReply &reply, uint32_t id, vespalib::slime::JsonFormat::encode(slime, buf, false); vespalib::Slime tmpSlime; size_t used = vespalib::slime::JsonFormat::decode(buf.get(), tmpSlime); - EXPECT_EQUAL(buf.get().size, used); + EXPECT_TRUE(used > 0); slime = std::move(tmpSlime); } vespalib::Slime expSlime; size_t used = vespalib::slime::JsonFormat::decode(exp, expSlime); - EXPECT_EQUAL(exp.size(), used); + EXPECT_TRUE(used > 0); return EXPECT_EQUAL(expSlime, slime); } diff --git a/searchcore/src/tests/proton/docsummary/summaryfieldconverter_test.cpp b/searchcore/src/tests/proton/docsummary/summaryfieldconverter_test.cpp index 17759e353e7..2d0ff39efa4 100644 --- a/searchcore/src/tests/proton/docsummary/summaryfieldconverter_test.cpp +++ b/searchcore/src/tests/proton/docsummary/summaryfieldconverter_test.cpp @@ -119,7 +119,7 @@ FieldBlock::FieldBlock(const vespalib::string &jsonInput) : input(jsonInput), slime(), binary(1024), json() { size_t used = vespalib::slime::JsonFormat::decode(jsonInput, slime); - EXPECT_EQUAL(jsonInput.size(), used); + EXPECT_TRUE(used > 0); { search::SlimeOutputRawBufAdapter adapter(binary); vespalib::slime::JsonFormat::encode(slime, adapter, true); diff --git a/searchcore/src/tests/proton/summaryengine/summaryengine.cpp b/searchcore/src/tests/proton/summaryengine/summaryengine.cpp index 355151dd88c..db707e4aa97 100644 --- a/searchcore/src/tests/proton/summaryengine/summaryengine.cpp +++ b/searchcore/src/tests/proton/summaryengine/summaryengine.cpp @@ -213,12 +213,12 @@ verify(vespalib::stringref exp, const Slime &slime) { Memory expMemory(exp); vespalib::Slime expSlime; size_t used = vespalib::slime::JsonFormat::decode(expMemory, expSlime); - EXPECT_EQUAL(used, expMemory.size); + EXPECT_TRUE(used > 0); vespalib::SimpleBuffer output; vespalib::slime::JsonFormat::encode(slime, output, true); Slime reSlimed; used = vespalib::slime::JsonFormat::decode(output.get(), reSlimed); - EXPECT_EQUAL(used, output.get().size); + EXPECT_TRUE(used > 0); EXPECT_EQUAL(expSlime, reSlimed); } diff --git a/searchcore/src/vespa/searchcore/config/proton.def b/searchcore/src/vespa/searchcore/config/proton.def index 58428a480bc..5471d94c541 100644 --- a/searchcore/src/vespa/searchcore/config/proton.def +++ b/searchcore/src/vespa/searchcore/config/proton.def @@ -11,12 +11,6 @@ ptport int default=8003 restart ## Port to use for the rpcserver. rpcport int default=8004 restart -## Port used for Slime Message Passing (srmp protocol) -slime_messaging_port int default=8005 restart - -## Connect spec for rtc. -rtcspec string default="tcp/localhost:8004" restart - ## Port to use for the web server httpport int default=0 restart diff --git a/searchcore/src/vespa/searchcore/proton/server/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/server/CMakeLists.txt index c5b0cd70527..1716441c30b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/CMakeLists.txt +++ b/searchcore/src/vespa/searchcore/proton/server/CMakeLists.txt @@ -68,7 +68,6 @@ vespa_add_library(searchcore_server STATIC operationdonecontext.cpp pendinglidtracker.cpp persistencehandlerproxy.cpp - persistenceproviderproxy.cpp proton.cpp proton_config_fetcher.cpp proton_config_snapshot.cpp diff --git a/searchcore/src/vespa/searchcore/proton/server/persistenceproviderproxy.cpp b/searchcore/src/vespa/searchcore/proton/server/persistenceproviderproxy.cpp deleted file mode 100644 index 997dcc8f7a0..00000000000 --- a/searchcore/src/vespa/searchcore/proton/server/persistenceproviderproxy.cpp +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "persistenceproviderproxy.h" - -using storage::spi::PersistenceProvider; - -namespace proton { - -PersistenceProviderProxy::PersistenceProviderProxy(PersistenceProvider &pp) - : _pp(pp) -{ -} - -} // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/persistenceproviderproxy.h b/searchcore/src/vespa/searchcore/proton/server/persistenceproviderproxy.h deleted file mode 100644 index 41319111277..00000000000 --- a/searchcore/src/vespa/searchcore/proton/server/persistenceproviderproxy.h +++ /dev/null @@ -1,165 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#pragma once - -#include <vespa/persistence/spi/persistenceprovider.h> - -namespace proton { - -class PersistenceProviderProxy : public storage::spi::PersistenceProvider -{ -private: - using Bucket = storage::spi::Bucket; - using BucketIdListResult = storage::spi::BucketIdListResult; - using BucketInfoResult = storage::spi::BucketInfoResult; - using ClusterState = storage::spi::ClusterState; - using Context = storage::spi::Context; - using CreateIteratorResult = storage::spi::CreateIteratorResult; - using GetResult = storage::spi::GetResult; - using IncludedVersions = storage::spi::IncludedVersions; - using IterateResult = storage::spi::IterateResult; - using IteratorId = storage::spi::IteratorId; - using PartitionId = storage::spi::PartitionId; - using PartitionStateListResult = storage::spi::PartitionStateListResult; - using RemoveResult = storage::spi::RemoveResult; - using Result = storage::spi::Result; - using Selection = storage::spi::Selection; - using Timestamp = storage::spi::Timestamp; - using UpdateResult = storage::spi::UpdateResult; - - storage::spi::PersistenceProvider &_pp; - -public: - PersistenceProviderProxy(storage::spi::PersistenceProvider &pp); - - virtual ~PersistenceProviderProxy() {} - - virtual Result initialize() override { - return _pp.initialize(); - } - - // Implements PersistenceProvider - virtual PartitionStateListResult getPartitionStates() const override { - return _pp.getPartitionStates(); - } - - virtual BucketIdListResult listBuckets(PartitionId partId) const override { - return _pp.listBuckets(partId); - } - - virtual Result setClusterState(const ClusterState &state) override { - return _pp.setClusterState(state); - } - - virtual Result setActiveState(const Bucket &bucket, - storage::spi::BucketInfo::ActiveState newState) override { - return _pp.setActiveState(bucket, newState); - } - - virtual BucketInfoResult getBucketInfo(const Bucket &bucket) const override { - return _pp.getBucketInfo(bucket); - } - - virtual Result put(const Bucket &bucket, - Timestamp timestamp, - const storage::spi::DocumentSP& doc, - Context& context) override { - return _pp.put(bucket, timestamp, doc, context); - } - - virtual RemoveResult remove(const Bucket &bucket, - Timestamp timestamp, - const document::DocumentId &docId, - Context& context) override { - return _pp.remove(bucket, timestamp, docId, context); - } - - virtual RemoveResult removeIfFound(const Bucket &bucket, - Timestamp timestamp, - const document::DocumentId &docId, - Context& context) override { - return _pp.removeIfFound(bucket, timestamp, docId, context); - } - - virtual UpdateResult update(const Bucket &bucket, - Timestamp timestamp, - const storage::spi::DocumentUpdateSP& docUpd, - Context& context) override { - return _pp.update(bucket, timestamp, docUpd, context); - } - - virtual Result flush(const Bucket &bucket, Context& context) override { - return _pp.flush(bucket, context); - } - - virtual GetResult get(const Bucket &bucket, - const document::FieldSet& fieldSet, - const document::DocumentId &docId, - Context& context) const override { - return _pp.get(bucket, fieldSet, docId, context); - } - - virtual CreateIteratorResult createIterator(const Bucket &bucket, - const document::FieldSet& fieldSet, - const Selection &selection, - IncludedVersions versions, - Context& context) override { - return _pp.createIterator(bucket, fieldSet, selection, versions, - context); - } - - virtual IterateResult iterate(IteratorId itrId, - uint64_t maxByteSize, - Context& context) const override { - return _pp.iterate(itrId, maxByteSize, context); - } - - virtual Result destroyIterator(IteratorId itrId, Context& context) override { - return _pp.destroyIterator(itrId, context); - } - - virtual Result createBucket(const Bucket &bucket, Context& context) override { - return _pp.createBucket(bucket, context); - } - - virtual Result deleteBucket(const Bucket &bucket, Context& context) override { - return _pp.deleteBucket(bucket, context); - } - - virtual BucketIdListResult getModifiedBuckets() const override { - return _pp.getModifiedBuckets(); - } - - virtual Result maintain(const Bucket &bucket, - storage::spi::MaintenanceLevel level) override { - return _pp.maintain(bucket, level); - } - - virtual Result split(const Bucket &source, - const Bucket &target1, - const Bucket &target2, - Context& context) override { - return _pp.split(source, target1, target2, context); - } - - virtual Result join(const Bucket &source1, - const Bucket &source2, - const Bucket &target, - Context& context) override { - return _pp.join(source1, source2, target, context); - } - - virtual Result move(const Bucket &source, - storage::spi::PartitionId target, - Context& context) override { - return _pp.move(source, target, context); - } - - virtual Result removeEntry(const Bucket &bucket, - Timestamp timestamp, - Context& context) override { - return _pp.removeEntry(bucket, timestamp, context); - } -}; - -} // namespace proton - diff --git a/searchsummary/src/tests/docsummary/slime_summary/slime_summary_test.cpp b/searchsummary/src/tests/docsummary/slime_summary/slime_summary_test.cpp index 3df589b0491..b2e324eb42e 100644 --- a/searchsummary/src/tests/docsummary/slime_summary/slime_summary_test.cpp +++ b/searchsummary/src/tests/docsummary/slime_summary/slime_summary_test.cpp @@ -24,7 +24,7 @@ struct FieldBlock { : slime(), binary(1024) { size_t used = vespalib::slime::JsonFormat::decode(jsonInput, slime); - EXPECT_EQUAL(jsonInput.size(), used); + EXPECT_TRUE(used > 0); search::SlimeOutputRawBufAdapter adapter(binary); vespalib::slime::BinaryFormat::encode(slime, adapter); } diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/ModelGenerator.java b/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/ModelGenerator.java index ef418f99d47..bee34524b20 100644 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/ModelGenerator.java +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/ModelGenerator.java @@ -31,6 +31,11 @@ import java.util.stream.Collectors; public class ModelGenerator { public static final String CLUSTER_ID_PROPERTY_NAME = "clustername"; + /** + * Create service model based primarily on super model. + * + * If the configServerhosts is non-empty, a config server application is added. + */ ServiceModel toServiceModel( SuperModel superModel, Zone zone, @@ -47,10 +52,12 @@ public class ModelGenerator { } // The config server is part of the service model (but not super model) - ConfigServerApplication configServerApplication = new ConfigServerApplication(); - ApplicationInstance<ServiceMonitorStatus> configServerApplicationInstance = - configServerApplication.toApplicationInstance(configServerHosts); - applicationInstances.put(configServerApplicationInstance.reference(), configServerApplicationInstance); + if (!configServerHosts.isEmpty()) { + ConfigServerApplication configServerApplication = new ConfigServerApplication(); + ApplicationInstance<ServiceMonitorStatus> configServerApplicationInstance = + configServerApplication.toApplicationInstance(configServerHosts); + applicationInstances.put(configServerApplicationInstance.reference(), configServerApplicationInstance); + } return new ServiceModel(applicationInstances); } diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/ServiceMonitorImpl.java b/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/ServiceMonitorImpl.java index 3468644169d..c3ce129e89a 100644 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/ServiceMonitorImpl.java +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/ServiceMonitorImpl.java @@ -8,6 +8,7 @@ import com.yahoo.config.provision.Zone; import com.yahoo.vespa.applicationmodel.ApplicationInstance; import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.logging.Logger; @@ -26,12 +27,20 @@ public class ServiceMonitorImpl implements ServiceMonitor { public ServiceMonitorImpl(SuperModelProvider superModelProvider, ConfigserverConfig configserverConfig) { this.zone = superModelProvider.getZone(); - this.configServerHostnames = configserverConfig.zookeeperserver().stream() - .map(server -> server.hostname()) - .collect(Collectors.toList()); + this.configServerHostnames = toConfigServerList(configserverConfig); superModelListener.start(superModelProvider); } + private List<String> toConfigServerList(ConfigserverConfig configserverConfig) { + if (configserverConfig.multitenant()) { + return configserverConfig.zookeeperserver().stream() + .map(server -> server.hostname()) + .collect(Collectors.toList()); + } + + return Collections.emptyList(); + } + @Override public Map<ApplicationInstanceReference, ApplicationInstance<ServiceMonitorStatus>> queryStatusOfAllApplicationInstances() { diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/ModelGeneratorTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/ModelGeneratorTest.java index e54fdc51a3a..87cbb7950b6 100644 --- a/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/ModelGeneratorTest.java +++ b/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/ModelGeneratorTest.java @@ -11,6 +11,7 @@ import com.yahoo.vespa.applicationmodel.ServiceCluster; import com.yahoo.vespa.applicationmodel.ServiceInstance; import org.junit.Test; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -30,7 +31,7 @@ public class ModelGeneratorTest { private final int PORT = 2; @Test - public void toApplicationModel() throws Exception { + public void toApplicationModelWithConfigServerApplication() throws Exception { SuperModel superModel = ExampleModel.createExampleSuperModelWithOneRpcPort(HOSTNAME, PORT); ModelGenerator modelGenerator = new ModelGenerator(); @@ -73,6 +74,34 @@ public class ModelGeneratorTest { } } + @Test + public void toApplicationModel() throws Exception { + SuperModel superModel = + ExampleModel.createExampleSuperModelWithOneRpcPort(HOSTNAME, PORT); + ModelGenerator modelGenerator = new ModelGenerator(); + + Zone zone = new Zone(Environment.from(ENVIRONMENT), RegionName.from(REGION)); + + List<String> configServerHosts = Collections.emptyList(); + + SlobrokMonitor2 slobrokMonitor = mock(SlobrokMonitor2.class); + when(slobrokMonitor.getStatus(any(), any())).thenReturn(ServiceMonitorStatus.UP); + + ServiceModel serviceModel = + modelGenerator.toServiceModel( + superModel, + zone, + configServerHosts, + slobrokMonitor); + + Map<ApplicationInstanceReference, + ApplicationInstance<ServiceMonitorStatus>> applicationInstances = + serviceModel.getAllApplicationInstances(); + + assertEquals(1, applicationInstances.size()); + verifyOtherApplication(applicationInstances.values().iterator().next()); + } + private void verifyOtherApplication(ApplicationInstance<ServiceMonitorStatus> applicationInstance) { assertEquals(String.format("%s:%s:%s:%s:%s", ExampleModel.TENANT, diff --git a/storage/src/tests/common/hostreporter/util.cpp b/storage/src/tests/common/hostreporter/util.cpp index a9578e8d8cf..e0563a431e6 100644 --- a/storage/src/tests/common/hostreporter/util.cpp +++ b/storage/src/tests/common/hostreporter/util.cpp @@ -24,10 +24,10 @@ reporterToSlime(HostReporter &hostReporter, vespalib::Slime &slime) { hostReporter.report(stream); stream << End(); std::string jsonData = json.str(); - size_t parsedSize = JsonFormat::decode(Memory(jsonData), slime); + size_t parsed = JsonFormat::decode(Memory(jsonData), slime); - if (jsonData.size() != parsedSize) { - CPPUNIT_FAIL("Sizes of jsonData mismatched, probably not json:\n" + jsonData); + if (parsed == 0) { + CPPUNIT_FAIL("jsonData is not json:\n" + jsonData); } } } diff --git a/storage/src/tests/storageserver/statereportertest.cpp b/storage/src/tests/storageserver/statereportertest.cpp index 8622f241a18..3a71444e74c 100644 --- a/storage/src/tests/storageserver/statereportertest.cpp +++ b/storage/src/tests/storageserver/statereportertest.cpp @@ -130,7 +130,7 @@ vespalib::Slime slime; \ size_t parsed = JsonFormat::decode(vespalib::Memory(jsonData), slime); \ vespalib::SimpleBuffer buffer; \ JsonFormat::encode(slime, buffer, false); \ - if (jsonData.size() != parsed) { \ + if (parsed == 0) { \ std::ostringstream error; \ error << "Failed to parse JSON: '\n" \ << jsonData << "'\n:" << buffer.get().make_string() << "\n"; \ diff --git a/vespalib/src/tests/data/input_reader/input_reader_test.cpp b/vespalib/src/tests/data/input_reader/input_reader_test.cpp index e8098b7e3ea..535c188d01e 100644 --- a/vespalib/src/tests/data/input_reader/input_reader_test.cpp +++ b/vespalib/src/tests/data/input_reader/input_reader_test.cpp @@ -112,4 +112,47 @@ TEST("expect that obtain does not set failure state on input reader") { } } +TEST("require that bytes can be unread when appropriate") { + const char *data = "12345"; + MemoryInput memory_input(data); + ChunkedInput input(memory_input, 3); + InputReader src(input); + EXPECT_TRUE(!src.try_unread()); + EXPECT_EQUAL(src.read(), '1'); + EXPECT_EQUAL(src.read(), '2'); + EXPECT_EQUAL(src.read(), '3'); + EXPECT_TRUE(src.try_unread()); + EXPECT_TRUE(src.try_unread()); + EXPECT_TRUE(src.try_unread()); + EXPECT_TRUE(!src.try_unread()); + EXPECT_EQUAL(src.read(), '1'); + EXPECT_EQUAL(src.read(), '2'); + EXPECT_EQUAL(src.read(), '3'); + EXPECT_EQUAL(src.read(), '4'); + EXPECT_TRUE(src.try_unread()); + EXPECT_TRUE(!src.try_unread()); + EXPECT_EQUAL(src.read(), '4'); + EXPECT_EQUAL(src.read(), '5'); + EXPECT_EQUAL(src.obtain(), 0u); + EXPECT_TRUE(!src.try_unread()); + EXPECT_TRUE(!src.failed()); +} + +TEST("require that try read finds eof without failing the reader") { + const char *data = "12345"; + MemoryInput memory_input(data); + ChunkedInput input(memory_input, 3); + InputReader src(input); + EXPECT_EQUAL(src.try_read(), '1'); + EXPECT_EQUAL(src.try_read(), '2'); + EXPECT_EQUAL(src.try_read(), '3'); + EXPECT_EQUAL(src.try_read(), '4'); + EXPECT_EQUAL(src.try_read(), '5'); + EXPECT_TRUE(src.try_unread()); + EXPECT_EQUAL(src.try_read(), '5'); + EXPECT_EQUAL(src.try_read(), '\0'); + EXPECT_TRUE(!src.try_unread()); + EXPECT_TRUE(!src.failed()); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/vespalib/src/tests/slime/slime_binary_format_test.cpp b/vespalib/src/tests/slime/slime_binary_format_test.cpp index 371a843a445..e6661cbf554 100644 --- a/vespalib/src/tests/slime/slime_binary_format_test.cpp +++ b/vespalib/src/tests/slime/slime_binary_format_test.cpp @@ -632,8 +632,7 @@ TEST("testOptionalDecodeOrder") { Slime from_json(const vespalib::string &json) { Slime slime; - size_t size = vespalib::slime::JsonFormat::decode(json, slime); - EXPECT_EQUAL(size, json.size()); + EXPECT_TRUE(vespalib::slime::JsonFormat::decode(json, slime) > 0); return slime; } diff --git a/vespalib/src/tests/slime/slime_json_format_test.cpp b/vespalib/src/tests/slime/slime_json_format_test.cpp index 52e293f4a5e..d1f77f09af1 100644 --- a/vespalib/src/tests/slime/slime_json_format_test.cpp +++ b/vespalib/src/tests/slime/slime_json_format_test.cpp @@ -1,10 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <vespa/vespalib/testkit/test_kit.h> #include <vespa/vespalib/data/slime/slime.h> +#include <vespa/vespalib/data/input.h> +#include <vespa/vespalib/data/memory_input.h> #include <iostream> #include <fstream> using namespace vespalib::slime::convenience; +using vespalib::Input; +using vespalib::MemoryInput; std::string make_json(const Slime &slime, bool compact) { vespalib::SimpleBuffer buf; @@ -60,7 +64,13 @@ std::string json_string(const std::string &str) { std::string normalize(const std::string &json) { Slime slime; - EXPECT_GREATER(vespalib::slime::JsonFormat::decode(json, slime), 0u); + EXPECT_TRUE(vespalib::slime::JsonFormat::decode(json, slime) > 0); + return make_json(slime, true); +} + +std::string normalize(Input &input) { + Slime slime; + EXPECT_TRUE(vespalib::slime::JsonFormat::decode(input, slime) > 0); return make_json(slime, true); } @@ -359,4 +369,19 @@ TEST_F("decode bytes not null-terminated", Slime) { EXPECT_TRUE(parse_json_bytes(mem, f)); } +TEST("require that multiple adjacent values can be decoded from a single input") { + vespalib::string data("true{}false[]null\"foo\"'bar'1.5null"); + MemoryInput input(data); + EXPECT_EQUAL(std::string("true"), normalize(input)); + EXPECT_EQUAL(std::string("{}"), normalize(input)); + EXPECT_EQUAL(std::string("false"), normalize(input)); + EXPECT_EQUAL(std::string("[]"), normalize(input)); + EXPECT_EQUAL(std::string("null"), normalize(input)); + EXPECT_EQUAL(std::string("\"foo\""), normalize(input)); + EXPECT_EQUAL(std::string("\"bar\""), normalize(input)); + EXPECT_EQUAL(std::string("1.5"), normalize(input)); + EXPECT_EQUAL(std::string("null"), normalize(input)); + EXPECT_EQUAL(input.obtain().size, 0u); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/vespalib/src/vespa/vespalib/data/input_reader.h b/vespalib/src/vespa/vespalib/data/input_reader.h index b3dfa0d4cd7..f92ced7e508 100644 --- a/vespalib/src/vespa/vespalib/data/input_reader.h +++ b/vespalib/src/vespa/vespalib/data/input_reader.h @@ -74,6 +74,35 @@ public: } /** + * Try to read a single byte. This function will not fail the + * reader with buffer underflow if eof is reached. + * + * @return the next input byte, or 0 if eof is reached + **/ + char try_read() { + if (__builtin_expect(obtain() > 0, true)) { + return _data.data[_pos++]; + } + return 0; + } + + /** + * Try to unread a single byte. This will work for data that is + * read, but not yet evicted. Note that after eof is found (the + * obtain function returns 0), unreading will not be possible. + * + * @return whether unreading could be performed + **/ + bool try_unread() { + if (__builtin_expect(_pos > 0, true)) { + --_pos; + return true; + } else { + return false; + } + } + + /** * Read a continous sequence of bytes. Bytes within an input chunk * will be referenced directly. Reads crossing chunk boundries * will result in a gathering copy into a temporary buffer owned diff --git a/vespalib/src/vespa/vespalib/data/slime/json_format.cpp b/vespalib/src/vespa/vespalib/data/slime/json_format.cpp index d6126667d68..18bf5289f0d 100644 --- a/vespalib/src/vespa/vespalib/data/slime/json_format.cpp +++ b/vespalib/src/vespa/vespalib/data/slime/json_format.cpp @@ -180,11 +180,7 @@ struct JsonDecoder { JsonDecoder(InputReader &reader) : in(reader), c(in.read()), key(), value() {} void next() { - if (in.obtain() > 0) { - c = in.read(); - } else { - c = 0; - } + c = in.try_read(); } bool skip(char x) { @@ -489,6 +485,7 @@ JsonFormat::decode(Input &input, Slime &slime) InputReader reader(input); JsonDecoder decoder(reader); decoder.decodeValue(slime); + reader.try_unread(); if (reader.failed()) { slime.wrap("partial_result"); slime.get().setLong("offending_offset", reader.get_offset()); diff --git a/vsm/src/tests/docsum/docsum.cpp b/vsm/src/tests/docsum/docsum.cpp index 43cf1c5309c..4409c5f6215 100644 --- a/vsm/src/tests/docsum/docsum.cpp +++ b/vsm/src/tests/docsum/docsum.cpp @@ -116,7 +116,7 @@ DocsumTest::assertSlimeFieldWriter(SlimeFieldWriter & sfw, const FieldValue & fv vespalib::Slime expSlime; size_t used = vespalib::slime::JsonFormat::decode(exp, expSlime); - EXPECT_EQUAL(exp.size(), used); + EXPECT_TRUE(used > 0); EXPECT_EQUAL(expSlime, gotSlime); } |