diff options
57 files changed, 598 insertions, 296 deletions
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/AllocatedHosts.java b/config-provisioning/src/main/java/com/yahoo/config/provision/AllocatedHosts.java index de4f3a555bd..96942c53a12 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/AllocatedHosts.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/AllocatedHosts.java @@ -26,6 +26,13 @@ import java.util.Set; */ public class AllocatedHosts { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String mappingKey = "mapping"; private static final String hostSpecKey = "hostSpec"; private static final String hostSpecHostNameKey = "hostName"; diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ContainerEndpointSerializer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ContainerEndpointSerializer.java index 379af7f71ea..91f9e3c8eed 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ContainerEndpointSerializer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ContainerEndpointSerializer.java @@ -18,6 +18,13 @@ import java.util.List; */ public class ContainerEndpointSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String clusterIdField = "clusterId"; private static final String namesField = "names"; diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index 9474c9f9160..06713d14d88 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -7148,7 +7148,10 @@ "public com.yahoo.data.access.Inspector inspect()", "public java.lang.String toString()", "public java.lang.String toJson()", - "public java.lang.StringBuilder writeJson(java.lang.StringBuilder)" + "public java.lang.StringBuilder writeJson(java.lang.StringBuilder)", + "public java.lang.Double getDouble(java.lang.String)", + "public com.yahoo.tensor.Tensor getTensor(java.lang.String)", + "public java.util.Set featureNames()" ], "fields": [] }, diff --git a/container-search/src/main/java/com/yahoo/data/JsonProducer.java b/container-search/src/main/java/com/yahoo/data/JsonProducer.java index 6d925b41379..c9dc0946a3e 100644 --- a/container-search/src/main/java/com/yahoo/data/JsonProducer.java +++ b/container-search/src/main/java/com/yahoo/data/JsonProducer.java @@ -12,6 +12,7 @@ public interface JsonProducer { * be human-readable and containing embedded newlines; also the * exact indentation etc may change, so use compact=true for a * canonical format. + * * @param target the StringBuilder to append to. * @return the target passed in is also returned (to allow chaining). */ @@ -20,7 +21,8 @@ public interface JsonProducer { /** * Convenience method equivalent to: * writeJson(new StringBuilder()).toString() - * @return String containing JSON representation of this object's data. + * + * @return a String containing JSON representation of this object's data. */ default String toJson() { return writeJson(new StringBuilder()).toString(); diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FeatureDataField.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FeatureDataField.java index 1f60dd3d1cf..b0003f4321e 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FeatureDataField.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FeatureDataField.java @@ -6,9 +6,8 @@ import com.yahoo.data.access.Type; import com.yahoo.search.result.FeatureData; /** - * Class representing a "feature data" field. This was historically - * just a string containing JSON; now it's a structure of - * data (that will be rendered as JSON by default). + * Class representing a "feature data" field: A map of values which are + * either floats or tensors. */ public class FeatureDataField extends LongstringField { @@ -23,12 +22,8 @@ public class FeatureDataField extends LongstringField { @Override public Object convert(Inspector value) { - if (! value.valid()) { - return null; - } - if (value.type() == Type.STRING) { - return value.asString(); - } + if ( ! value.valid()) return null; + if (value.type() == Type.STRING) return value.asString(); return new FeatureData(value); } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/LongstringField.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/LongstringField.java index 2f9c6d5b325..5de38e43c96 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/LongstringField.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/LongstringField.java @@ -5,10 +5,6 @@ */ package com.yahoo.prelude.fastsearch; -import java.nio.ByteBuffer; - -import com.yahoo.io.SlowInflate; -import com.yahoo.text.Utf8; import com.yahoo.data.access.Inspector; /** diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/JSONDebugSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/JSONDebugSearcher.java index 2330ca2382a..5f921b67702 100644 --- a/container-search/src/main/java/com/yahoo/prelude/searcher/JSONDebugSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/searcher/JSONDebugSearcher.java @@ -3,6 +3,7 @@ package com.yahoo.prelude.searcher; import com.yahoo.prelude.fastsearch.FastHit; import com.yahoo.prelude.hitfield.JSONString; +import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.Searcher; import com.yahoo.processing.request.CompoundName; @@ -27,7 +28,7 @@ public class JSONDebugSearcher extends Searcher { private static CompoundName PROPERTYNAME = new CompoundName("dumpjson"); @Override - public Result search(com.yahoo.search.Query query, Execution execution) { + public Result search(Query query, Execution execution) { Result r = execution.search(query); String propertyName = query.properties().getString(PROPERTYNAME); if (propertyName != null) { diff --git a/container-search/src/main/java/com/yahoo/search/result/FeatureData.java b/container-search/src/main/java/com/yahoo/search/result/FeatureData.java index 53e77631ff9..7e5d6b12f30 100644 --- a/container-search/src/main/java/com/yahoo/search/result/FeatureData.java +++ b/container-search/src/main/java/com/yahoo/search/result/FeatureData.java @@ -6,29 +6,42 @@ import com.yahoo.data.access.Inspectable; import com.yahoo.data.access.Type; import com.yahoo.data.JsonProducer; import com.yahoo.data.access.simple.JsonRender; +import com.yahoo.io.GrowableByteBuffer; +import com.yahoo.tensor.Tensor; +import com.yahoo.tensor.serialization.JsonFormat; +import com.yahoo.tensor.serialization.TypedBinaryFormat; + +import java.nio.charset.StandardCharsets; +import java.util.HashSet; +import java.util.Optional; +import java.util.Set; /** - * A wrapper for structured data representing feature values. + * A wrapper for structured data representing feature values: A map of floats and tensors. + * This class is not thread safe even when it is only consumed. */ public class FeatureData implements Inspectable, JsonProducer { private final Inspector value; + private Set<String> featureNames = null; + public FeatureData(Inspector value) { this.value = value; } + /** + * Returns the fields of this as an inspector, where tensors are represented as binary data + * which can be decoded using + * <code>com.yahoo.tensor.serialization.TypedBinaryFormat.decode(Optional.empty(), GrowableByteBuffer.wrap(featureValue.asData()))</code> + */ @Override - public Inspector inspect() { - return value; - } + public Inspector inspect() { return value; } + @Override public String toString() { - if (value.type() == Type.EMPTY) { - return ""; - } else { - return toJson(); - } + if (value.type() == Type.EMPTY) return ""; + return toJson(); } @Override @@ -38,7 +51,64 @@ public class FeatureData implements Inspectable, JsonProducer { @Override public StringBuilder writeJson(StringBuilder target) { - return JsonRender.render(value, target, true); + return JsonRender.render(value, new Encoder(target, true)); + } + + /** + * Returns the value of a scalar feature, or null if it is not present. + * + * @throws IllegalArgumentException if the value exists but isn't a scalar + * (that is, if it is a tensor with nonzero rank) + */ + public Double getDouble(String featureName) { + Inspector featureValue = value.field(featureName); + if ( ! featureValue.valid()) return null; + + switch (featureValue.type()) { + case DOUBLE: return featureValue.asDouble(); + case DATA: throw new IllegalArgumentException("Feature '" + featureName + "' is a tensor, not a double"); + default: throw new IllegalStateException("Unexpected feature value type " + featureValue.type()); + } + } + + /** + * Returns the value of a tensor feature, or null if it is not present. + * This will return any feature value: Scalars are returned as a rank 0 tensor. + */ + public Tensor getTensor(String featureName) { + Inspector featureValue = value.field(featureName); + if ( ! featureValue.valid()) return null; + + switch (featureValue.type()) { + case DOUBLE: return Tensor.from(featureValue.asDouble()); + case DATA: return TypedBinaryFormat.decode(Optional.empty(), GrowableByteBuffer.wrap(featureValue.asData())); + default: throw new IllegalStateException("Unexpected feature value type " + featureValue.type()); + } + } + + /** Returns the names of the features available in this */ + public Set<String> featureNames() { + if (featureNames != null) return featureNames; + + featureNames = new HashSet<>(); + value.fields().forEach(field -> featureNames.add(field.getKey())); + return featureNames; + } + + /** A JSON encoder which encodes DATA as a tensor */ + private static class Encoder extends JsonRender.StringEncoder { + + Encoder(StringBuilder out, boolean compact) { + super(out, compact); + } + + @Override + public void encodeDATA(byte[] value) { + // This could be done more efficiently ... + target().append(new String(JsonFormat.encodeWithType(TypedBinaryFormat.decode(Optional.empty(), GrowableByteBuffer.wrap(value))), + StandardCharsets.UTF_8)); + } + } } diff --git a/container-search/src/main/java/com/yahoo/search/result/PositionsData.java b/container-search/src/main/java/com/yahoo/search/result/PositionsData.java index 483849a5435..203e0206f1e 100644 --- a/container-search/src/main/java/com/yahoo/search/result/PositionsData.java +++ b/container-search/src/main/java/com/yahoo/search/result/PositionsData.java @@ -10,7 +10,7 @@ import com.yahoo.data.access.simple.JsonRender; /** * A wrapper for structured data representing an array of position values. - **/ + */ public class PositionsData implements Inspectable, JsonProducer, XmlProducer { private final Inspector value; diff --git a/container-search/src/test/java/com/yahoo/search/result/FeatureDataTestCase.java b/container-search/src/test/java/com/yahoo/search/result/FeatureDataTestCase.java new file mode 100644 index 00000000000..9cc7cc743fc --- /dev/null +++ b/container-search/src/test/java/com/yahoo/search/result/FeatureDataTestCase.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.search.result; + +import com.yahoo.data.access.slime.SlimeAdapter; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Slime; +import com.yahoo.tensor.Tensor; +import com.yahoo.tensor.serialization.TypedBinaryFormat; +import org.junit.Test; + +import java.util.stream.Collectors; + +import static org.junit.Assert.assertEquals; + +/** + * @author bratseth + */ +public class FeatureDataTestCase { + + private static final double delta = 0.00000001; + + @Test + public void testFeatureData() { + Cursor features = new Slime().setObject(); + features.setDouble("scalar1", 1.5); + features.setDouble("scalar2", 2.5); + Tensor tensor1 = Tensor.from("tensor(x[3]):[1.5, 2, 2.5]"); + features.setData("tensor1", TypedBinaryFormat.encode(tensor1)); + Tensor tensor2 = Tensor.from(0.5); + features.setData("tensor2", TypedBinaryFormat.encode(tensor2)); + + FeatureData featureData = new FeatureData(new SlimeAdapter(features)); + assertEquals("scalar1,scalar2,tensor1,tensor2", + featureData.featureNames().stream().sorted().collect(Collectors.joining(","))); + assertEquals(1.5, featureData.getDouble("scalar1"), delta); + assertEquals(2.5, featureData.getDouble("scalar2"), delta); + assertEquals(Tensor.from(1.5), featureData.getTensor("scalar1")); + assertEquals(Tensor.from(2.5), featureData.getTensor("scalar2")); + assertEquals(tensor1, featureData.getTensor("tensor1")); + assertEquals(tensor2, featureData.getTensor("tensor2")); + + String expectedJson = + "{" + + "\"scalar1\":1.5," + + "\"scalar2\":2.5," + + "\"tensor1\":{\"type\":\"tensor(x[3])\",\"cells\":[{\"address\":{\"x\":\"0\"},\"value\":1.5},{\"address\":{\"x\":\"1\"},\"value\":2.0},{\"address\":{\"x\":\"2\"},\"value\":2.5}]}," + + "\"tensor2\":{\"type\":\"tensor()\",\"cells\":[{\"address\":{},\"value\":0.5}]}" + + "}"; + assertEquals(expectedJson, featureData.toJson()); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java index 159eb234aa7..b8bb9a7ef79 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java @@ -3,11 +3,10 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.component.Version; import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.zone.UpgradePolicy; import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node; -import com.yahoo.config.provision.zone.UpgradePolicy; -import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.yolean.Exceptions; @@ -68,7 +67,9 @@ public abstract class InfrastructureUpgrader extends Maintainer { for (SystemApplication application : applications) { if (convergedOn(target, application.dependencies(), zone)) { boolean currentAppConverged = convergedOn(target, application, zone); - if (!currentAppConverged) { + // In dynamically provisioned zones there may be no tenant hosts at the time of upgrade, so we + // should always set the target version. + if (application == SystemApplication.tenantHost || !currentAppConverged) { upgrade(target, application, zone); } converged &= currentAppConverged; 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 47c7b54264e..1f20bdf5533 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 @@ -54,6 +54,13 @@ import java.util.TreeMap; */ public class ApplicationSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + // Application fields private final String idField = "id"; private final String createdAtField = "createdAt"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/AuditLogSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/AuditLogSerializer.java index 7fee9a7f9b4..d18e561ce5d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/AuditLogSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/AuditLogSerializer.java @@ -19,6 +19,13 @@ import java.util.function.Function; */ public class AuditLogSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String entriesField = "entries"; private static final String atField = "at"; private static final String principalField = "principal"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ConfidenceOverrideSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ConfidenceOverrideSerializer.java index a87875da104..2cb981aac03 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ConfidenceOverrideSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ConfidenceOverrideSerializer.java @@ -18,6 +18,13 @@ import java.util.Map; */ public class ConfidenceOverrideSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private final static String overridesField = "overrides"; public Slime toSlime(Map<Version, Confidence> overrides) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializer.java index 40781ac6e92..418038d4f1e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializer.java @@ -27,6 +27,13 @@ import java.util.stream.Collectors; */ class LogSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String idField = "id"; private static final String typeField = "type"; private static final String timestampField = "at"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NameServiceQueueSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NameServiceQueueSerializer.java index 3dfb5ffe5f8..e3dedd65e68 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NameServiceQueueSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NameServiceQueueSerializer.java @@ -24,6 +24,13 @@ import java.util.ArrayList; */ public class NameServiceQueueSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String requestsField = "requests"; private static final String requestType = "requestType"; private static final String recordsField = "records"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializer.java index 21f8b1bcb80..d68e24a27ea 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializer.java @@ -20,6 +20,13 @@ import java.util.TreeSet; */ public class OsVersionSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String versionsField = "versions"; private static final String versionField = "version"; private static final String cloudField = "cloud"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java index 3e3c0df1673..88805f54d65 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java @@ -26,6 +26,13 @@ import java.util.TreeMap; */ public class OsVersionStatusSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String versionsField = "versions"; private static final String versionField = "version"; private static final String nodesField = "nodes"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java index a9c6c54a44a..9cfce8dc16a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java @@ -24,6 +24,13 @@ import java.util.function.Function; */ public class RoutingPolicySerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String routingPoliciesField = "routingPolicies"; private static final String clusterField = "cluster"; private static final String canonicalNameField = "canonicalName"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java index f29af1055d0..1c95c9766f5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java @@ -56,6 +56,13 @@ import static java.util.Comparator.comparing; */ class RunSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String stepsField = "steps"; private static final String applicationField = "id"; private static final String jobTypeField = "type"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java index 56e80068908..3a4e6c3954c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java @@ -29,6 +29,13 @@ import java.util.Optional; */ public class TenantSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String nameField = "name"; private static final String typeField = "type"; private static final String athenzDomainField = "athenzDomain"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionSerializer.java index 5edae803fdb..e5897963254 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionSerializer.java @@ -13,6 +13,13 @@ import com.yahoo.slime.Slime; */ public class VersionSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String versionField = "version"; public Slime toSlime(Version version) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java index 72d38bbee5f..405a2e452d0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java @@ -27,6 +27,13 @@ import java.util.Set; */ public class VersionStatusSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + // VersionStatus fields private static final String versionsField = "versions"; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java index 0e6000c651b..73c86fc8de1 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java @@ -7,7 +7,6 @@ import com.yahoo.config.provision.HostName; import com.yahoo.vespa.athenz.identity.ServiceIdentitySslSocketFactory; import com.yahoo.vespa.athenz.identity.SiaIdentityProvider; import com.yahoo.vespa.hosted.node.admin.component.ConfigServerInfo; -import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger; import org.apache.http.HttpHeaders; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.CloseableHttpResponse; @@ -38,6 +37,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.logging.Logger; /** * Retries request on config server a few times before giving up. Assumes that all requests should be sent with @@ -47,7 +47,7 @@ import java.util.Optional; * @author bjorncs */ public class ConfigServerApiImpl implements ConfigServerApi { - private static final PrefixLogger NODE_ADMIN_LOGGER = PrefixLogger.getNodeAdminLogger(ConfigServerApiImpl.class); + private static final Logger logger = Logger.getLogger(ConfigServerApiImpl.class.getName()); private final ObjectMapper mapper = new ObjectMapper(); @@ -106,7 +106,7 @@ public class ConfigServerApiImpl implements ConfigServerApi { try { return mapper.readValue(response.getEntity().getContent(), wantedReturnType); } catch (IOException e) { - throw new RuntimeException("Failed parse response from config server", e); + throw new UncheckedIOException("Failed parse response from config server", e); } } catch (HttpException e) { if (!e.isRetryable()) throw e; @@ -117,15 +117,15 @@ public class ConfigServerApiImpl implements ConfigServerApi { // Failure to communicate with a config server is not abnormal during upgrades if (e.getMessage().contains("(Connection refused)")) { - NODE_ADMIN_LOGGER.info("Connection refused to " + configServer + " (upgrading?), will try next"); + logger.info("Connection refused to " + configServer + " (upgrading?), will try next"); } else { - NODE_ADMIN_LOGGER.warning("Failed to communicate with " + configServer + ", will try next: " + e.getMessage()); + logger.warning("Failed to communicate with " + configServer + ", will try next: " + e.getMessage()); } } } - throw new RuntimeException("All requests against the config servers (" - + configServers + ") failed, last as follows:", lastException); + throw HttpException.handleException( + "All requests against the config servers (" + configServers + ") failed, last as follows:", lastException); } @Override diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java index 256fe38ec68..3825107bfa6 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java @@ -1,13 +1,19 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.configserver; +import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException; +import org.apache.http.NoHttpResponseException; + import javax.ws.rs.core.Response; +import java.io.EOFException; +import java.net.SocketException; +import java.net.SocketTimeoutException; /** * @author hakonhall */ @SuppressWarnings("serial") -public class HttpException extends RuntimeException { +public class HttpException extends ConvergenceException { private final boolean isRetryable; @@ -21,7 +27,12 @@ public class HttpException extends RuntimeException { this.isRetryable = isRetryable; } - public boolean isRetryable() { + private HttpException(String message) { + super(message); + this.isRetryable = false; + } + + boolean isRetryable() { return isRetryable; } @@ -55,6 +66,22 @@ public class HttpException extends RuntimeException { throw new HttpException(status, message, true); } + /** + * Returns {@link HttpException} if the given Throwable is of a known and well understood error or + * a RuntimeException with the given exception as cause otherwise. + */ + public static RuntimeException handleException(String prefix, Throwable t) { + for (; t != null; t = t.getCause()) { + if (t instanceof SocketException || + t instanceof SocketTimeoutException || + t instanceof NoHttpResponseException || + t instanceof EOFException) + return new HttpException(prefix + t.getMessage()); + } + + return new RuntimeException(prefix, t); + } + public static class NotFoundException extends HttpException { public NotFoundException(String message) { super(Response.Status.NOT_FOUND, message, false); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeRepositoryException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeRepositoryException.java index 6094518c3fc..10e61d373d2 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeRepositoryException.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeRepositoryException.java @@ -1,7 +1,9 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.configserver.noderepository; -public class NodeRepositoryException extends RuntimeException { +import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException; + +public class NodeRepositoryException extends ConvergenceException { public NodeRepositoryException(String message) { super(message); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java index d402e75ff7b..52d6f16dd78 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java @@ -91,6 +91,13 @@ public class NodeSpec { Set<String> additionalIpAddresses, NodeReports reports, Optional<String> parentHostname) { + if (state == NodeState.active) { + Objects.requireNonNull(wantedVespaVersion, "Unknown vespa version for active node"); + Objects.requireNonNull(wantedDockerImage, "Unknown docker image for active node"); + Objects.requireNonNull(wantedRestartGeneration, "Unknown restartGeneration for active node"); + Objects.requireNonNull(currentRestartGeneration, "Unknown currentRestartGeneration for active node"); + } + this.hostname = Objects.requireNonNull(hostname); this.wantedDockerImage = Objects.requireNonNull(wantedDockerImage); this.currentDockerImage = Objects.requireNonNull(currentDockerImage); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java index ca52eca13d2..fe19b81614d 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java @@ -12,10 +12,8 @@ import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.Ge import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.GetNodesResponse; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.NodeMessageResponse; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.NodeRepositoryNode; -import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger; import java.time.Instant; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Objects; @@ -23,6 +21,7 @@ import java.util.Optional; import java.util.Set; import java.util.TreeMap; import java.util.function.Function; +import java.util.logging.Logger; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -30,7 +29,7 @@ import java.util.stream.Stream; * @author stiankri, dybis */ public class RealNodeRepository implements NodeRepository { - private static final PrefixLogger NODE_ADMIN_LOGGER = PrefixLogger.getNodeAdminLogger(RealNodeRepository.class); + private static final Logger logger = Logger.getLogger(RealNodeRepository.class.getName()); private final ConfigServerApi configServerApi; @@ -46,7 +45,7 @@ public class RealNodeRepository implements NodeRepository { NodeMessageResponse response = configServerApi.post("/nodes/v2/node", nodesToPost, NodeMessageResponse.class); if (Strings.isNullOrEmpty(response.errorCode)) return; - throw new NodeRepositoryException("Failed to add nodes to node-repo: " + response.message + " " + response.errorCode); + throw new NodeRepositoryException("Failed to add nodes: " + response.message + " " + response.errorCode); } @Override @@ -80,43 +79,37 @@ public class RealNodeRepository implements NodeRepository { */ @Override public Map<String, Acl> getAcls(String hostName) { - try { - String path = String.format("/nodes/v2/acl/%s?children=true", hostName); - GetAclResponse response = configServerApi.get(path, GetAclResponse.class); - - // Group ports by container hostname that trusts them - Map<String, Set<Integer>> trustedPorts = response.trustedPorts.stream() - .collect(Collectors.groupingBy( - GetAclResponse.Port::getTrustedBy, - Collectors.mapping(port -> port.port, Collectors.toSet()))); - - // Group node ip-addresses by container hostname that trusts them - Map<String, Set<Acl.Node>> trustedNodes = response.trustedNodes.stream() - .collect(Collectors.groupingBy( - GetAclResponse.Node::getTrustedBy, - Collectors.mapping( - node -> new Acl.Node(node.hostname, node.ipAddress), - Collectors.toSet()))); - - // Group trusted networks by container hostname that trusts them - Map<String, Set<String>> trustedNetworks = response.trustedNetworks.stream() - .collect(Collectors.groupingBy(GetAclResponse.Network::getTrustedBy, - Collectors.mapping(node -> node.network, Collectors.toSet()))); - - - // For each hostname create an ACL - return Stream.of(trustedNodes.keySet(), trustedPorts.keySet(), trustedNetworks.keySet()) - .flatMap(Set::stream) - .distinct() - .collect(Collectors.toMap( - Function.identity(), - hostname -> new Acl(trustedPorts.get(hostname), trustedNodes.get(hostname), - trustedNetworks.get(hostname)))); - } catch (HttpException.NotFoundException e) { - NODE_ADMIN_LOGGER.warning("Failed to fetch ACLs for " + hostName + " No ACL will be applied"); - } - - return Collections.emptyMap(); + String path = String.format("/nodes/v2/acl/%s?children=true", hostName); + GetAclResponse response = configServerApi.get(path, GetAclResponse.class); + + // Group ports by container hostname that trusts them + Map<String, Set<Integer>> trustedPorts = response.trustedPorts.stream() + .collect(Collectors.groupingBy( + GetAclResponse.Port::getTrustedBy, + Collectors.mapping(port -> port.port, Collectors.toSet()))); + + // Group node ip-addresses by container hostname that trusts them + Map<String, Set<Acl.Node>> trustedNodes = response.trustedNodes.stream() + .collect(Collectors.groupingBy( + GetAclResponse.Node::getTrustedBy, + Collectors.mapping( + node -> new Acl.Node(node.hostname, node.ipAddress), + Collectors.toSet()))); + + // Group trusted networks by container hostname that trusts them + Map<String, Set<String>> trustedNetworks = response.trustedNetworks.stream() + .collect(Collectors.groupingBy(GetAclResponse.Network::getTrustedBy, + Collectors.mapping(node -> node.network, Collectors.toSet()))); + + + // For each hostname create an ACL + return Stream.of(trustedNodes.keySet(), trustedPorts.keySet(), trustedNetworks.keySet()) + .flatMap(Set::stream) + .distinct() + .collect(Collectors.toMap( + Function.identity(), + hostname -> new Acl(trustedPorts.get(hostname), trustedNodes.get(hostname), + trustedNetworks.get(hostname)))); } @Override @@ -127,7 +120,7 @@ public class RealNodeRepository implements NodeRepository { NodeMessageResponse.class); if (Strings.isNullOrEmpty(response.errorCode)) return; - throw new NodeRepositoryException("Unexpected message " + response.message + " " + response.errorCode); + throw new NodeRepositoryException("Failed to update node attributes: " + response.message + " " + response.errorCode); } @Override @@ -137,10 +130,10 @@ public class RealNodeRepository implements NodeRepository { "/nodes/v2/state/" + state + "/" + hostName, Optional.empty(), /* body */ NodeMessageResponse.class); - NODE_ADMIN_LOGGER.info(response.message); + logger.info(response.message); if (Strings.isNullOrEmpty(response.errorCode)) return; - throw new NodeRepositoryException("Unexpected message " + response.message + " " + response.errorCode); + throw new NodeRepositoryException("Failed to set node state: " + response.message + " " + response.errorCode); } private static NodeSpec createNodeSpec(NodeRepositoryNode node) { @@ -149,30 +142,13 @@ public class RealNodeRepository implements NodeRepository { Objects.requireNonNull(node.state, "Unknown node state"); NodeState nodeState = NodeState.valueOf(node.state); - if (nodeState == NodeState.active) { - Objects.requireNonNull(node.wantedVespaVersion, "Unknown vespa version for active node"); - Objects.requireNonNull(node.wantedDockerImage, "Unknown docker image for active node"); - Objects.requireNonNull(node.restartGeneration, "Unknown restartGeneration for active node"); - Objects.requireNonNull(node.currentRestartGeneration, "Unknown currentRestartGeneration for active node"); - } - - String hostName = Objects.requireNonNull(node.hostname, "hostname is null"); - - NodeOwner owner = null; - if (node.owner != null) { - owner = new NodeOwner(node.owner.tenant, node.owner.application, node.owner.instance); - } - - NodeMembership membership = null; - if (node.membership != null) { - membership = new NodeMembership(node.membership.clusterType, node.membership.clusterId, - node.membership.group, node.membership.index, node.membership.retired); - } - NodeReports reports = NodeReports.fromMap(node.reports == null ? Collections.emptyMap() : node.reports); + Optional<NodeMembership> membership = Optional.ofNullable(node.membership) + .map(m -> new NodeMembership(m.clusterType, m.clusterId, m.group, m.index, m.retired)); + NodeReports reports = NodeReports.fromMap(Optional.ofNullable(node.reports).orElseGet(Map::of)); return new NodeSpec( - hostName, + node.hostname, Optional.ofNullable(node.wantedDockerImage).map(DockerImage::fromString), Optional.ofNullable(node.currentDockerImage).map(DockerImage::fromString), nodeState, @@ -185,8 +161,8 @@ public class RealNodeRepository implements NodeRepository { Optional.ofNullable(node.currentOsVersion).map(Version::fromString), Optional.ofNullable(node.allowedToBeDown), Optional.ofNullable(node.wantToDeprovision), - Optional.ofNullable(owner), - Optional.ofNullable(membership), + Optional.ofNullable(node.owner).map(o -> new NodeOwner(o.tenant, o.application, o.instance)), + membership, Optional.ofNullable(node.restartGeneration), Optional.ofNullable(node.currentRestartGeneration), node.rebootGeneration, diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorException.java index fe19da0c41c..8575bf7f655 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorException.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorException.java @@ -1,8 +1,10 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.configserver.orchestrator; +import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException; + @SuppressWarnings("serial") -public class OrchestratorException extends RuntimeException { +public class OrchestratorException extends ConvergenceException { public OrchestratorException(String message) { super(message); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorImpl.java index 64a67aa612a..6124e1bdc0e 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorImpl.java @@ -40,8 +40,7 @@ public class OrchestratorImpl implements Orchestrator { } catch (HttpException.NotFoundException n) { throw new OrchestratorNotFoundException("Failed to suspend " + hostName + ", host not found"); } catch (HttpException e) { - throw new OrchestratorException("Failed to suspend " + hostName + ": " + - e.toString()); + throw new OrchestratorException("Failed to suspend " + hostName + ": " + e.toString()); } catch (RuntimeException e) { throw new RuntimeException("Got error on suspend", e); } @@ -60,9 +59,8 @@ public class OrchestratorImpl implements Orchestrator { parentHostName, params); batchOperationResult = configServerApi.put(url, Optional.empty(), BatchOperationResult.class); } catch (HttpException e) { - throw new OrchestratorException("Failed to batch suspend for " + - parentHostName + ": " + e.toString()); - } catch (Exception e) { + throw new OrchestratorException("Failed to batch suspend for " + parentHostName + ": " + e.toString()); + } catch (RuntimeException e) { throw new RuntimeException("Got error on batch suspend for " + parentHostName + ", with nodes " + hostNames, e); } @@ -80,9 +78,8 @@ public class OrchestratorImpl implements Orchestrator { } catch (HttpException.NotFoundException n) { throw new OrchestratorNotFoundException("Failed to resume " + hostName + ", host not found"); } catch (HttpException e) { - throw new OrchestratorException("Failed to suspend " + hostName + ": " + - e.toString()); - } catch (Exception e) { + throw new OrchestratorException("Failed to suspend " + hostName + ": " + e.toString()); + } catch (RuntimeException e) { throw new RuntimeException("Got error on resume", e); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImpl.java index efeb3039379..2fe8d4b4792 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImpl.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.node.admin.configserver.state; import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi; +import com.yahoo.vespa.hosted.node.admin.configserver.HttpException; import com.yahoo.vespa.hosted.node.admin.configserver.state.bindings.HealthResponse; /** @@ -16,26 +17,12 @@ public class StateImpl implements State { @Override public HealthCode getHealth() { - HealthResponse response; try { - response = configServerApi.get("/state/v1/health", HealthResponse.class); - } catch (RuntimeException e) { - if (causedByConnectionRefused(e)) { - return HealthCode.DOWN; - } - - throw e; + HealthResponse response = configServerApi.get("/state/v1/health", HealthResponse.class); + return HealthCode.fromString(response.status.code); + } catch (HttpException e) { + return HealthCode.DOWN; } - return HealthCode.fromString(response.status.code); } - private static boolean causedByConnectionRefused(Throwable throwable) { - for (Throwable cause = throwable; cause != null; cause = cause.getCause()) { - if (cause instanceof java.net.ConnectException) { - return true; - } - } - - return false; - } } 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 8c38b1bbd84..cfdcefec1e3 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 @@ -391,7 +391,7 @@ public class NodeAgentImpl implements NodeAgent { public void converge(NodeAgentContext context) { try { doConverge(context); - } catch (OrchestratorException | ConvergenceException e) { + } catch (ConvergenceException e) { context.log(logger, e.getMessage()); } catch (ContainerNotFoundException e) { containerState = ABSENT; @@ -436,6 +436,7 @@ public class NodeAgentImpl implements NodeAgent { case reserved: case parked: case failed: + case inactive: removeContainerIfNeededUpdateContainerState(context, container); updateNodeRepoWithCurrentAttributes(context); break; @@ -481,10 +482,6 @@ public class NodeAgentImpl implements NodeAgent { context.log(logger, "Call resume against Orchestrator"); orchestrator.resume(context.hostname().value()); break; - case inactive: - removeContainerIfNeededUpdateContainerState(context, container); - updateNodeRepoWithCurrentAttributes(context); - break; case provisioned: nodeRepository.setNodeState(context.hostname().value(), NodeState.dirty); break; @@ -497,7 +494,7 @@ public class NodeAgentImpl implements NodeAgent { nodeRepository.setNodeState(context.hostname().value(), NodeState.ready); break; default: - throw new RuntimeException("UNKNOWN STATE " + node.getState().name()); + throw new ConvergenceException("UNKNOWN STATE " + node.getState().name()); } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/PrefixLogger.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/PrefixLogger.java deleted file mode 100644 index f4d85a19f6d..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/PrefixLogger.java +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.node.admin.util; - -import com.yahoo.log.LogLevel; -import com.yahoo.vespa.hosted.dockerapi.ContainerName; - -import java.util.logging.Level; -import java.util.logging.Logger; - -/** - * @author freva - */ -public class PrefixLogger { - private final String prefix; - private final Logger logger; - - private <T> PrefixLogger(Class<T> clazz, String prefix) { - this.logger = Logger.getLogger(clazz.getName()); - this.prefix = prefix + ": "; - } - - public static <T> PrefixLogger getNodeAdminLogger(Class<T> clazz) { - return new PrefixLogger(clazz, "NodeAdmin"); - } - - public static <T> PrefixLogger getNodeAgentLogger(Class<T> clazz, ContainerName containerName) { - return new PrefixLogger(clazz, "NodeAgent-" + containerName.asString()); - } - - private void log(Level level, String message, Throwable thrown) { - logger.log(level, prefix + message, thrown); - } - - private void log(Level level, String message) { - logger.log(level, prefix + message); - } - - - public void debug(String message) { - log(LogLevel.DEBUG, message); - } - - public void info(String message) { - log(LogLevel.INFO, message); - } - - public void info(String message, Throwable thrown) { - log(LogLevel.INFO, message, thrown); - } - - public void error(String message) { - log(LogLevel.ERROR, message); - } - - public void error(String message, Throwable thrown) { - log(LogLevel.ERROR, message, thrown); - } - - public void warning(String message) { - log(LogLevel.WARNING, message); - } - - public void warning(String message, Throwable thrown) { - log(LogLevel.WARNING, message, thrown); - } -} diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImplTest.java index 2604aa05367..14755ebf9cc 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImplTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.node.admin.configserver.state; import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi; +import com.yahoo.vespa.hosted.node.admin.configserver.HttpException; import com.yahoo.vespa.hosted.node.admin.configserver.state.bindings.HealthResponse; import org.junit.Test; @@ -28,7 +29,7 @@ public class StateImplTest { @Test public void connectException() { - RuntimeException exception = new RuntimeException(new ConnectException("connection refused")); + RuntimeException exception = HttpException.handleException("Error: ", new ConnectException("connection refused")); when(api.get(any(), any())).thenThrow(exception); HealthCode code = state.getHealth(); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java index a4b915a6128..fd2294c1b5d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java @@ -27,6 +27,13 @@ import java.util.function.Function; */ public class LoadBalancerSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String idField = "id"; private static final String hostnameField = "hostname"; private static final String dnsZoneField = "dnsZone"; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java index d38a6e5031c..424889caf72 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java @@ -44,12 +44,12 @@ import java.util.function.UnaryOperator; */ public class NodeSerializer { - // WARNING: Since there are multiple config servers in a cluster and they upgrade one by one - // (and rewrite all nodes on startup), - // changes to the serialized format must be made such that what is serialized on version N+1 - // can be read by version N: + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: // - ADDING FIELDS: Always ok // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. /** The configured node flavors */ private final NodeFlavors flavors; diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp index d37072f5da2..9b535be19b7 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp @@ -1079,8 +1079,12 @@ DocumentMetaStore::foreach(const search::IGidToLidMapperVisitor &visitor) const } // namespace proton -template class search::btree:: -BTreeIterator<proton::DocumentMetaStore::DocId, - search::btree::BTreeNoLeafData, - search::btree::NoAggregated, - const proton::DocumentMetaStore::KeyComp &>; +namespace search::btree { + +template class BTreeIteratorBase<proton::DocumentMetaStore::DocId, BTreeNoLeafData, NoAggregated, BTreeDefaultTraits::INTERNAL_SLOTS, BTreeDefaultTraits::LEAF_SLOTS, BTreeDefaultTraits::PATH_SIZE>; + +template class BTreeConstIterator<proton::DocumentMetaStore::DocId, BTreeNoLeafData, NoAggregated, const proton::DocumentMetaStore::KeyComp &>; + +template class BTreeIterator<proton::DocumentMetaStore::DocId, BTreeNoLeafData, NoAggregated, const proton::DocumentMetaStore::KeyComp &>; + +} diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h index efac1158cfb..27c1c97556c 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h @@ -268,8 +268,12 @@ public: } -extern template class search::btree:: -BTreeIterator<proton::DocumentMetaStore::DocId, - search::btree::BTreeNoLeafData, - search::btree::NoAggregated, - const proton::DocumentMetaStore::KeyComp &>; +namespace search::btree { + +extern template class BTreeIteratorBase<proton::DocumentMetaStore::DocId, BTreeNoLeafData, NoAggregated, BTreeDefaultTraits::INTERNAL_SLOTS, BTreeDefaultTraits::LEAF_SLOTS, BTreeDefaultTraits::PATH_SIZE>; + +extern template class BTreeConstIterator<proton::DocumentMetaStore::DocId, BTreeNoLeafData, NoAggregated, const proton::DocumentMetaStore::KeyComp &>; + +extern template class BTreeIterator<proton::DocumentMetaStore::DocId, BTreeNoLeafData, NoAggregated, const proton::DocumentMetaStore::KeyComp &>; + +} diff --git a/searchlib/src/tests/features/ranking_expression/ranking_expression_test.cpp b/searchlib/src/tests/features/ranking_expression/ranking_expression_test.cpp index c7c3447a4cc..251040ecfa7 100644 --- a/searchlib/src/tests/features/ranking_expression/ranking_expression_test.cpp +++ b/searchlib/src/tests/features/ranking_expression/ranking_expression_test.cpp @@ -26,6 +26,8 @@ struct DummyExpression : IntrinsicExpression { DummyExpression(const FeatureType &type_in) : type(type_in) {} vespalib::string describe_self() const override { return "dummy"; } const FeatureType &result_type() const override { return type; } + void prepare_shared_state(const QueryEnv &, IObjectStore &) const override { + } FeatureExecutor &create_executor(const QueryEnv &, vespalib::Stash &stash) const override { return stash.create<DummyExecutor>(); } @@ -81,7 +83,7 @@ SetupResult::SetupResult(const TypeMap &object_inputs, setup_ok = rank.setup(index_env, {}); EXPECT_TRUE(!deps.accept_type_mismatch); } -SetupResult::~SetupResult() {} +SetupResult::~SetupResult() = default; void verify_output_type(const TypeMap &object_inputs, const vespalib::string &expression, const FeatureType &expect) diff --git a/searchlib/src/tests/memoryindex/field_index/field_index_test.cpp b/searchlib/src/tests/memoryindex/field_index/field_index_test.cpp index 05c905cdc32..0f1c966ad5d 100644 --- a/searchlib/src/tests/memoryindex/field_index/field_index_test.cpp +++ b/searchlib/src/tests/memoryindex/field_index/field_index_test.cpp @@ -171,7 +171,7 @@ assertPostingList(const std::string &exp, uint32_t docId = itr.getKey(); ss << docId; if (store != nullptr) { // consider features as well - EntryRef ref(itr.getData()); + EntryRef ref(itr.getData().get_features()); store->setupForField(0, decoder); store->setupForUnpackFeatures(ref, decoder); decoder.unpackFeatures(matchData, docId); diff --git a/searchlib/src/vespa/searchlib/attribute/enumcomparator.h b/searchlib/src/vespa/searchlib/attribute/enumcomparator.h index 4cd446352d0..255d0bead9f 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumcomparator.h +++ b/searchlib/src/vespa/searchlib/attribute/enumcomparator.h @@ -27,13 +27,18 @@ public: /** * Creates a comparator using the given enum store. **/ - EnumStoreComparatorT(const EnumStoreType & enumStore); + EnumStoreComparatorT(const EnumStoreType & enumStore) + : _enumStore(enumStore), + _value() + {} /** * Creates a comparator using the given enum store and that uses the * given value during compare if the enum index is invalid. **/ - EnumStoreComparatorT(const EnumStoreType & enumStore, - EntryValue value); + EnumStoreComparatorT(const EnumStoreType & enumStore, EntryValue value) + : _enumStore(enumStore), + _value(value) + {} static int compare(EntryValue lhs, EntryValue rhs) { if (lhs < rhs) { @@ -60,7 +65,7 @@ private: typedef typename ParentType::EnumIndex EnumIndex; typedef typename ParentType::EntryValue EntryValue; using ParentType::getValue; - bool _prefix; + bool _prefix; size_t _prefixLen; public: /** @@ -90,22 +95,6 @@ public: } }; - -template <typename EntryType> -EnumStoreComparatorT<EntryType>::EnumStoreComparatorT(const EnumStoreType & enumStore) : - _enumStore(enumStore), - _value() -{ -} - -template <typename EntryType> -EnumStoreComparatorT<EntryType>::EnumStoreComparatorT(const EnumStoreType & enumStore, - EntryValue value) : - _enumStore(enumStore), - _value(value) -{ -} - template <> int EnumStoreComparatorT<NumericEntryType<float> >::compare(EntryValue lhs, EntryValue rhs); diff --git a/searchlib/src/vespa/searchlib/attribute/enumstorebase.cpp b/searchlib/src/vespa/searchlib/attribute/enumstorebase.cpp index 94c431368cb..decb4152d8d 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumstorebase.cpp +++ b/searchlib/src/vespa/searchlib/attribute/enumstorebase.cpp @@ -704,6 +704,10 @@ template class btree::BTreeIteratorBase<EnumStoreBase::Index, datastore::EntryRef, btree::NoAggregated, EnumTreeTraits::INTERNAL_SLOTS, EnumTreeTraits::LEAF_SLOTS, EnumTreeTraits::PATH_SIZE>; +template class btree::BTreeConstIterator<EnumStoreBase::Index, btree::BTreeNoLeafData, btree::NoAggregated, const EnumStoreComparatorWrapper, EnumTreeTraits>; + +template class btree::BTreeConstIterator<EnumStoreBase::Index, datastore::EntryRef, btree::NoAggregated, const EnumStoreComparatorWrapper, EnumTreeTraits>; + template class btree::BTreeIterator<EnumStoreBase::Index, btree::BTreeNoLeafData, btree::NoAggregated, const EnumStoreComparatorWrapper, EnumTreeTraits>; diff --git a/searchlib/src/vespa/searchlib/attribute/enumstorebase.h b/searchlib/src/vespa/searchlib/attribute/enumstorebase.h index 48bf4a56874..d8604a5a85e 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumstorebase.h +++ b/searchlib/src/vespa/searchlib/attribute/enumstorebase.h @@ -467,6 +467,10 @@ extern template class btree::BTreeIteratorBase<EnumStoreBase::Index, datastore::EntryRef, btree::NoAggregated, EnumTreeTraits::INTERNAL_SLOTS, EnumTreeTraits::LEAF_SLOTS, EnumTreeTraits::PATH_SIZE>; +extern template class btree::BTreeConstIterator<EnumStoreBase::Index, btree::BTreeNoLeafData, btree::NoAggregated, const EnumStoreComparatorWrapper, EnumTreeTraits>; + +extern template class btree::BTreeConstIterator<EnumStoreBase::Index, datastore::EntryRef, btree::NoAggregated, const EnumStoreComparatorWrapper, EnumTreeTraits>; + extern template class btree::BTreeIterator<EnumStoreBase::Index, btree::BTreeNoLeafData, btree::NoAggregated, const EnumStoreComparatorWrapper, EnumTreeTraits>; diff --git a/searchlib/src/vespa/searchlib/features/rankingexpression/intrinsic_blueprint_adapter.cpp b/searchlib/src/vespa/searchlib/features/rankingexpression/intrinsic_blueprint_adapter.cpp index 018da0e7bcd..4ff9d2f4e30 100644 --- a/searchlib/src/vespa/searchlib/features/rankingexpression/intrinsic_blueprint_adapter.cpp +++ b/searchlib/src/vespa/searchlib/features/rankingexpression/intrinsic_blueprint_adapter.cpp @@ -29,8 +29,11 @@ struct IntrinsicBlueprint : IntrinsicExpression { : blueprint(std::move(blueprint_in)), type(type_in) {} vespalib::string describe_self() const override { return blueprint->getName(); } const FeatureType &result_type() const override { return type; } - FeatureExecutor &create_executor(const QueryEnv &queryEnv, vespalib::Stash &stash) const override { - return blueprint->createExecutor(queryEnv, stash); + void prepare_shared_state(const QueryEnv & env, fef::IObjectStore & store) const override { + blueprint->prepareSharedState(env, store); + } + FeatureExecutor &create_executor(const QueryEnv &env, vespalib::Stash &stash) const override { + return blueprint->createExecutor(env, stash); } }; diff --git a/searchlib/src/vespa/searchlib/features/rankingexpression/intrinsic_expression.h b/searchlib/src/vespa/searchlib/features/rankingexpression/intrinsic_expression.h index 34c4f34b03f..79cb3f9035b 100644 --- a/searchlib/src/vespa/searchlib/features/rankingexpression/intrinsic_expression.h +++ b/searchlib/src/vespa/searchlib/features/rankingexpression/intrinsic_expression.h @@ -11,6 +11,7 @@ namespace search::fef { class FeatureType; class FeatureExecutor; class IQueryEnvironment; +class IObjectStore; } namespace search::features::rankingexpression { @@ -26,8 +27,8 @@ struct IntrinsicExpression { using UP = std::unique_ptr<IntrinsicExpression>; virtual vespalib::string describe_self() const = 0; virtual const FeatureType &result_type() const = 0; - virtual FeatureExecutor &create_executor(const QueryEnv &queryEnv, - vespalib::Stash &stash) const = 0; + virtual void prepare_shared_state(const QueryEnv & env, fef::IObjectStore & store) const = 0; + virtual FeatureExecutor &create_executor(const QueryEnv &queryEnv, vespalib::Stash &stash) const = 0; virtual ~IntrinsicExpression(); }; diff --git a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp index b2c8c64d55a..2733ec62105 100644 --- a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp +++ b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp @@ -134,10 +134,14 @@ CompiledRankingExpressionExecutor::execute(uint32_t) //----------------------------------------------------------------------------- +namespace { + using Context = fef::FeatureExecutor::Inputs; double resolve_input(void *ctx, size_t idx) { return ((const Context *)(ctx))->get_number(idx); } Context *make_ctx(const Context &inputs) { return const_cast<Context *>(&inputs); } +} + LazyCompiledRankingExpressionExecutor::LazyCompiledRankingExpressionExecutor(const CompiledFunction &compiled_function) : _ranking_function(compiled_function.get_lazy_function()) { @@ -278,6 +282,14 @@ RankingExpressionBlueprint::createInstance() const return std::make_unique<RankingExpressionBlueprint>(_expression_replacer); } +void +RankingExpressionBlueprint::prepareSharedState(const fef::IQueryEnvironment & env, fef::IObjectStore & store) const +{ + if (_intrinsic_expression) { + return _intrinsic_expression->prepare_shared_state(env, store); + } +} + fef::FeatureExecutor & RankingExpressionBlueprint::createExecutor(const fef::IQueryEnvironment &env, vespalib::Stash &stash) const { diff --git a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.h b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.h index 8d5144206ea..104e8d63a70 100644 --- a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.h +++ b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.h @@ -26,7 +26,7 @@ private: public: RankingExpressionBlueprint(); RankingExpressionBlueprint(rankingexpression::ExpressionReplacer::SP replacer); - ~RankingExpressionBlueprint(); + ~RankingExpressionBlueprint() override; void visitDumpFeatures(const fef::IIndexEnvironment &env, fef::IDumpFeatureVisitor &visitor) const override; fef::Blueprint::UP createInstance() const override; @@ -37,6 +37,7 @@ public: } bool setup(const fef::IIndexEnvironment & env, const fef::ParameterList & params) override; + void prepareSharedState(const fef::IQueryEnvironment & queryEnv, fef::IObjectStore & objectStore) const override; fef::FeatureExecutor &createExecutor(const fef::IQueryEnvironment &env, vespalib::Stash &stash) const override; }; diff --git a/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp b/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp index e2e1c99a9b9..8daf87e1899 100644 --- a/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp +++ b/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp @@ -127,36 +127,35 @@ FieldIndex::compactFeatures() const PostingList *tree = _postingListStore.getTreeEntry(pidx); auto pitr = tree->begin(_postingListStore.getAllocator()); for (; pitr.valid(); ++pitr) { - EntryRef oldFeatures(pitr.getData()); + const PostingListEntry &posting_entry(pitr.getData()); // Filter on which buffers to move features from when // performing incremental compaction. - EntryRef newFeatures = _featureStore.moveFeatures(packedIndex, oldFeatures); + EntryRef newFeatures = _featureStore.moveFeatures(packedIndex, posting_entry.get_features()); // Features must be written before reference is updated. std::atomic_thread_fence(std::memory_order_release); - // Ugly, ugly due to const_cast in iterator - pitr.writeData(newFeatures.ref()); + // Reference the moved data + posting_entry.update_features(newFeatures); } } else { const PostingListKeyDataType *shortArray = _postingListStore.getKeyDataEntry(pidx, clusterSize); const PostingListKeyDataType *ite = shortArray + clusterSize; for (const PostingListKeyDataType *it = shortArray; it < ite; ++it) { - EntryRef oldFeatures(it->getData()); + const PostingListEntry &posting_entry(it->getData()); // Filter on which buffers to move features from when // performing incremental compaction. - EntryRef newFeatures = _featureStore.moveFeatures(packedIndex, oldFeatures); + EntryRef newFeatures = _featureStore.moveFeatures(packedIndex, posting_entry.get_features()); // Features must be written before reference is updated. std::atomic_thread_fence(std::memory_order_release); - // Ugly, ugly due to const_cast, but new data is - // semantically equal to old data - const_cast<PostingListKeyDataType *>(it)->setData(newFeatures.ref()); + // Reference the moved data + posting_entry.update_features(newFeatures); } } } @@ -189,7 +188,7 @@ FieldIndex::dump(search::index::IndexBuilder & indexBuilder) assert(pitr.valid()); for (; pitr.valid(); ++pitr) { uint32_t docId = pitr.getKey(); - EntryRef featureRef(pitr.getData()); + EntryRef featureRef(pitr.getData().get_features()); _featureStore.setupForReadFeatures(featureRef, decoder); decoder.readFeatures(features); features.set_doc_id(docId); @@ -202,7 +201,7 @@ FieldIndex::dump(search::index::IndexBuilder & indexBuilder) const PostingListKeyDataType *kde = kd + clusterSize; for (; kd != kde; ++kd) { uint32_t docId = kd->_key; - EntryRef featureRef(kd->getData()); + EntryRef featureRef(kd->getData().get_features()); _featureStore.setupForReadFeatures(featureRef, decoder); decoder.readFeatures(features); features.set_doc_id(docId); diff --git a/searchlib/src/vespa/searchlib/memoryindex/field_index.h b/searchlib/src/vespa/searchlib/memoryindex/field_index.h index dba57f553b5..d5df2fa49c8 100644 --- a/searchlib/src/vespa/searchlib/memoryindex/field_index.h +++ b/searchlib/src/vespa/searchlib/memoryindex/field_index.h @@ -5,6 +5,7 @@ #include "feature_store.h" #include "field_index_remover.h" #include "word_store.h" +#include "posting_list_entry.h" #include <vespa/searchlib/index/docidandfeatures.h> #include <vespa/searchlib/index/field_length_calculator.h> #include <vespa/searchlib/index/indexbuilder.h> @@ -35,8 +36,8 @@ class OrderedFieldIndexInserter; class FieldIndex { public: // Mapping from docid -> feature ref - using PostingList = btree::BTreeRoot<uint32_t, uint32_t, search::btree::NoAggregated>; - using PostingListStore = btree::BTreeStore<uint32_t, uint32_t, + using PostingList = btree::BTreeRoot<uint32_t, PostingListEntry, search::btree::NoAggregated>; + using PostingListStore = btree::BTreeStore<uint32_t, PostingListEntry, search::btree::NoAggregated, std::less<uint32_t>, btree::BTreeDefaultTraits>; diff --git a/searchlib/src/vespa/searchlib/memoryindex/ordered_field_index_inserter.cpp b/searchlib/src/vespa/searchlib/memoryindex/ordered_field_index_inserter.cpp index 637a13d67be..1d38e88b747 100644 --- a/searchlib/src/vespa/searchlib/memoryindex/ordered_field_index_inserter.cpp +++ b/searchlib/src/vespa/searchlib/memoryindex/ordered_field_index_inserter.cpp @@ -111,7 +111,7 @@ OrderedFieldIndexInserter::add(uint32_t docId, assert(_prevDocId == noDocId || _prevDocId < docId || (_prevDocId == docId && !_prevAdd)); datastore::EntryRef featureRef = _fieldIndex.addFeatures(features); - _adds.push_back(PostingListKeyDataType(docId, featureRef.ref())); + _adds.push_back(PostingListKeyDataType(docId, featureRef)); _listener.insert(_dItr.getKey()._wordRef, docId); _prevDocId = docId; _prevAdd = true; diff --git a/searchlib/src/vespa/searchlib/memoryindex/posting_iterator.cpp b/searchlib/src/vespa/searchlib/memoryindex/posting_iterator.cpp index 63040aab66f..290aa16dfe4 100644 --- a/searchlib/src/vespa/searchlib/memoryindex/posting_iterator.cpp +++ b/searchlib/src/vespa/searchlib/memoryindex/posting_iterator.cpp @@ -6,6 +6,7 @@ #include <vespa/vespalib/btree/btreenodeallocator.hpp> #include <vespa/vespalib/btree/btreenodestore.hpp> #include <vespa/vespalib/btree/btreeroot.hpp> +#include <vespa/vespalib/btree/btreestore.hpp> #include <vespa/log/log.h> LOG_SETUP(".searchlib.memoryindex.posting_iterator"); @@ -62,7 +63,7 @@ PostingIterator::doUnpack(uint32_t docId) assert(docId == getDocId()); assert(_itr.valid()); assert(docId == _itr.getKey()); - datastore::EntryRef featureRef(_itr.getData()); + datastore::EntryRef featureRef(_itr.getData().get_features()); _featureStore.setupForUnpackFeatures(featureRef, _featureDecoder); _featureDecoder.unpackFeatures(_matchData, docId); setUnpacked(); @@ -70,3 +71,59 @@ PostingIterator::doUnpack(uint32_t docId) } +namespace search::btree { + +template class BTreeNodeTT<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + BTreeDefaultTraits::INTERNAL_SLOTS>; + +template class BTreeLeafNode<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + BTreeDefaultTraits::LEAF_SLOTS>; + +template class BTreeNodeStore<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + BTreeDefaultTraits::INTERNAL_SLOTS, + BTreeDefaultTraits::LEAF_SLOTS>; + +template class BTreeIteratorBase<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + BTreeDefaultTraits::INTERNAL_SLOTS, + BTreeDefaultTraits::LEAF_SLOTS, + BTreeDefaultTraits::PATH_SIZE>; + +template class BTreeIterator<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + std::less<uint32_t>, + BTreeDefaultTraits>; + +template class BTree<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + std::less<uint32_t>, + BTreeDefaultTraits>; + +template class BTreeRoot<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + std::less<uint32_t>, + BTreeDefaultTraits>; + +template class BTreeRootBase<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + BTreeDefaultTraits::INTERNAL_SLOTS, + BTreeDefaultTraits::LEAF_SLOTS>; + +template class BTreeNodeAllocator<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + BTreeDefaultTraits::INTERNAL_SLOTS, + BTreeDefaultTraits::LEAF_SLOTS>; + +} diff --git a/searchlib/src/vespa/searchlib/memoryindex/posting_list_entry.h b/searchlib/src/vespa/searchlib/memoryindex/posting_list_entry.h new file mode 100644 index 00000000000..b28cd87736c --- /dev/null +++ b/searchlib/src/vespa/searchlib/memoryindex/posting_list_entry.h @@ -0,0 +1,36 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +# pragma once + +#include <vespa/vespalib/datastore/entryref.h> + +namespace search::memoryindex { + +/** + * Entry per document in memory index posting list. + */ +class PostingListEntry { + mutable datastore::EntryRef _features; // reference to compressed features + +public: + PostingListEntry(datastore::EntryRef features) + : _features(features) + { + } + + PostingListEntry() + : _features() + { + } + + datastore::EntryRef get_features() const { return _features; } + + /* + * Reference moved features (used when compacting FeatureStore). + * The moved features must have the same content as the original + * features. + */ + void update_features(datastore::EntryRef features) const { _features = features; } +}; + +} diff --git a/searchlib/src/vespa/searchlib/test/fakedata/fakememtreeocc.cpp b/searchlib/src/vespa/searchlib/test/fakedata/fakememtreeocc.cpp index 54c0aa866b4..f5300430bea 100644 --- a/searchlib/src/vespa/searchlib/test/fakedata/fakememtreeocc.cpp +++ b/searchlib/src/vespa/searchlib/test/fakedata/fakememtreeocc.cpp @@ -261,13 +261,13 @@ FakeMemTreeOccMgr::flush() lastWord = wordIdx; if (i->getRemove()) { if (itr.valid() && itr.getKey() == docId) { - uint64_t bits = _featureStore.bitSize(fw->getPackedIndex(), EntryRef(itr.getData())); + uint64_t bits = _featureStore.bitSize(fw->getPackedIndex(), EntryRef(itr.getData().get_features())); _featureSizes[wordIdx] -= RefType::align((bits + 7) / 8) * 8; tree.remove(itr); } } else { if (!itr.valid() || docId < itr.getKey()) { - tree.insert(itr, docId, i->getFeatureRef().ref()); + tree.insert(itr, docId, i->getFeatureRef()); } } } diff --git a/vespajlib/abi-spec.json b/vespajlib/abi-spec.json index b2b895040bc..3b733105d2e 100644 --- a/vespajlib/abi-spec.json +++ b/vespajlib/abi-spec.json @@ -104,14 +104,22 @@ "com.yahoo.data.access.ObjectTraverser" ], "attributes": [ - "public", - "final" + "public" ], "methods": [ "public void <init>(java.lang.StringBuilder, boolean)", "public void encode(com.yahoo.data.access.Inspector)", + "protected void encodeEMPTY()", + "protected void encodeBOOL(boolean)", + "protected void encodeLONG(long)", + "protected void encodeDOUBLE(double)", + "protected void encodeSTRING(java.lang.String)", + "protected void encodeDATA(byte[])", + "protected void encodeARRAY(com.yahoo.data.access.Inspector)", + "protected void encodeOBJECT(com.yahoo.data.access.Inspector)", "public void entry(int, com.yahoo.data.access.Inspector)", - "public void field(java.lang.String, com.yahoo.data.access.Inspector)" + "public void field(java.lang.String, com.yahoo.data.access.Inspector)", + "public java.lang.StringBuilder target()" ], "fields": [] }, @@ -124,7 +132,8 @@ ], "methods": [ "public void <init>()", - "public static java.lang.StringBuilder render(com.yahoo.data.access.Inspectable, java.lang.StringBuilder, boolean)" + "public static java.lang.StringBuilder render(com.yahoo.data.access.Inspectable, java.lang.StringBuilder, boolean)", + "public static java.lang.StringBuilder render(com.yahoo.data.access.Inspectable, com.yahoo.data.access.simple.JsonRender$StringEncoder)" ], "fields": [] }, diff --git a/vespajlib/src/main/java/com/yahoo/data/access/simple/JsonRender.java b/vespajlib/src/main/java/com/yahoo/data/access/simple/JsonRender.java index 253b0c60927..9f662c77c59 100644 --- a/vespajlib/src/main/java/com/yahoo/data/access/simple/JsonRender.java +++ b/vespajlib/src/main/java/com/yahoo/data/access/simple/JsonRender.java @@ -11,19 +11,25 @@ import com.yahoo.data.access.ObjectTraverser; * * @author arnej27959 */ -public final class JsonRender -{ +public final class JsonRender { + public static StringBuilder render(Inspectable value, StringBuilder target, - boolean compact) - { - StringEncoder enc = new StringEncoder(target, compact); - enc.encode(value.inspect()); - return target; + boolean compact) { + return render(value, new StringEncoder(target, compact)); + } + + /** + * Renders the given value to the target stringbuilder with a given encoder. + * This is useful to use an encoder where rendering of some value types is customized. + */ + public static StringBuilder render(Inspectable value, StringEncoder encoder) { + encoder.encode(value.inspect()); + return encoder.target(); } - public static final class StringEncoder implements ArrayTraverser, ObjectTraverser - { + public static class StringEncoder implements ArrayTraverser, ObjectTraverser { + private final StringBuilder out; private boolean head = true; private boolean compact; @@ -41,21 +47,21 @@ public final class JsonRender } } - private void encodeEMPTY() { + protected void encodeEMPTY() { out.append("null"); } - private void encodeBOOL(boolean value) { + protected void encodeBOOL(boolean value) { out.append(value ? "true" : "false"); } - private void encodeLONG(long value) { - out.append(String.valueOf(value)); + protected void encodeLONG(long value) { + out.append(value); } - private void encodeDOUBLE(double value) { + protected void encodeDOUBLE(double value) { if (Double.isFinite(value)) { - out.append(String.valueOf(value)); + out.append(value); } else { out.append("null"); } @@ -63,7 +69,7 @@ public final class JsonRender static final char[] hex = "0123456789ABCDEF".toCharArray(); - private void encodeSTRING(String value) { + protected void encodeSTRING(String value) { out.append('"'); for (char c : value.toCharArray()) { switch (c) { @@ -89,7 +95,7 @@ public final class JsonRender out.append('"'); } - private void encodeDATA(byte[] value) { + protected void encodeDATA(byte[] value) { out.append('"'); out.append("0x"); for (int pos = 0; pos < value.length; pos++) { @@ -99,14 +105,14 @@ public final class JsonRender out.append('"'); } - private void encodeARRAY(Inspector inspector) { + protected void encodeARRAY(Inspector inspector) { openScope("["); ArrayTraverser at = this; inspector.traverse(at); closeScope("]"); } - private void encodeOBJECT(Inspector inspector) { + protected void encodeOBJECT(Inspector inspector) { openScope("{"); ObjectTraverser ot = this; inspector.traverse(ot); @@ -164,5 +170,10 @@ public final class JsonRender out.append(' '); encodeValue(inspector); } + + /** Returns the target this is encoding values to */ + public StringBuilder target() { return out; } + } + } diff --git a/vespajlib/src/main/java/com/yahoo/tensor/serialization/JsonFormat.java b/vespajlib/src/main/java/com/yahoo/tensor/serialization/JsonFormat.java index 52635905d72..1a210a614cc 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/serialization/JsonFormat.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/serialization/JsonFormat.java @@ -19,21 +19,33 @@ import java.util.Iterator; * A JSON map containing a 'cells' array. * See http://docs.vespa.ai/documentation/reference/document-json-put-format.html#tensor */ -// TODO: We should probably move reading of this format from the document module to here public class JsonFormat { - /** Serializes the given tensor into JSON format */ + /** Serializes the given tensor value into JSON format */ public static byte[] encode(Tensor tensor) { Slime slime = new Slime(); Cursor root = slime.setObject(); - Cursor cellsArray = root.setArray("cells"); + encodeCells(tensor, root); + return com.yahoo.slime.JsonFormat.toJsonBytes(slime); + } + + /** Serializes the given tensor type and value into JSON format */ + public static byte[] encodeWithType(Tensor tensor) { + Slime slime = new Slime(); + Cursor root = slime.setObject(); + root.setString("type", tensor.type().toString()); + encodeCells(tensor, root); + return com.yahoo.slime.JsonFormat.toJsonBytes(slime); + } + + private static void encodeCells(Tensor tensor, Cursor rootObject) { + Cursor cellsArray = rootObject.setArray("cells"); for (Iterator<Tensor.Cell> i = tensor.cellIterator(); i.hasNext(); ) { Tensor.Cell cell = i.next(); Cursor cellObject = cellsArray.addObject(); encodeAddress(tensor.type(), cell.getKey(), cellObject.setObject("address")); cellObject.setDouble("value", cell.getValue()); } - return com.yahoo.slime.JsonFormat.toJsonBytes(slime); } private static void encodeAddress(TensorType type, TensorAddress address, Cursor addressObject) { diff --git a/vespalib/src/vespa/vespalib/btree/btreeiterator.h b/vespalib/src/vespa/vespalib/btree/btreeiterator.h index de9637c00f1..7c247cd01da 100644 --- a/vespalib/src/vespa/vespalib/btree/btreeiterator.h +++ b/vespalib/src/vespa/vespalib/btree/btreeiterator.h @@ -302,8 +302,7 @@ protected: * * @param pathSize New tree height (number of levels of internal nodes) */ - void - clearPath(uint32_t pathSize); + VESPA_DLL_LOCAL void clearPath(uint32_t pathSize); public: bool @@ -396,8 +395,7 @@ public: /** * Setup iterator to be empty and not be associated with any tree. */ - void - setupEmpty(); + VESPA_DLL_LOCAL void setupEmpty(); /** * Move iterator to beyond last element in the current tree. |