diff options
29 files changed, 567 insertions, 146 deletions
diff --git a/client/go/jvm/container.go b/client/go/jvm/container.go index fd65602e573..c755672da86 100644 --- a/client/go/jvm/container.go +++ b/client/go/jvm/container.go @@ -60,11 +60,8 @@ func readableEnv(env map[string]string) string { } func (cb *containerBase) Exec() { - argv := make([]string, 0, 100) - argv = append(argv, "java") - for _, x := range cb.JvmOptions().Args() { - argv = append(argv, x) - } + argv := util.ArrayListOf(cb.JvmOptions().Args()) + argv.Insert(0, "java") p := prog.NewSpec(argv) p.ConfigureNumaCtl() cb.JvmOptions().exportEnvSettings(p) diff --git a/client/go/jvm/options.go b/client/go/jvm/options.go index deb842936ba..55749e86709 100644 --- a/client/go/jvm/options.go +++ b/client/go/jvm/options.go @@ -17,7 +17,7 @@ import ( type Options struct { container Container classPath []string - jvmArgs []string + jvmArgs util.ArrayList[string] mainClass string fixSpec util.FixSpec } @@ -40,10 +40,8 @@ func NewOptions(c Container) *Options { } func (opts *Options) AddOption(arg string) { - for _, old := range opts.jvmArgs { - if arg == old { - return - } + if opts.jvmArgs.Contains(arg) { + return } opts.AppendOption(arg) } diff --git a/client/go/script-utils/configserver/runserver.go b/client/go/script-utils/configserver/runserver.go index 3f49285d036..9afa154cc06 100644 --- a/client/go/script-utils/configserver/runserver.go +++ b/client/go/script-utils/configserver/runserver.go @@ -45,7 +45,7 @@ func (rs *RunServer) WouldRun() bool { } func (rs *RunServer) Exec(prog string) { - argv := []string{ + argv := util.ArrayList[string]{ PROG_NAME, "-s", rs.ServiceName, "-r", "30", @@ -53,9 +53,7 @@ func (rs *RunServer) Exec(prog string) { "--", prog, } - for _, arg := range rs.Args { - argv = append(argv, arg) - } + argv.AppendAll(rs.Args...) err := util.Execvp(rs.ProgPath(), argv) util.JustExitWith(err) } diff --git a/client/go/script-utils/startcbinary/numactl.go b/client/go/script-utils/startcbinary/numactl.go index 89d9c144649..1585d8ddf81 100644 --- a/client/go/script-utils/startcbinary/numactl.go +++ b/client/go/script-utils/startcbinary/numactl.go @@ -58,17 +58,15 @@ func (p *ProgSpec) numaCtlBinary() string { } func (p *ProgSpec) prependNumaCtl(args []string) []string { - result := make([]string, 0, 5+len(args)) - result = append(result, "numactl") + v := util.NewArrayList[string](5 + len(args)) + v.Append("numactl") if p.numaSocket >= 0 { - result = append(result, fmt.Sprintf("--cpunodebind=%d", p.numaSocket)) - result = append(result, fmt.Sprintf("--membind=%d", p.numaSocket)) + v.Append(fmt.Sprintf("--cpunodebind=%d", p.numaSocket)) + v.Append(fmt.Sprintf("--membind=%d", p.numaSocket)) } else { - result = append(result, "--interleave") - result = append(result, "all") + v.Append("--interleave") + v.Append("all") } - for _, arg := range args { - result = append(result, arg) - } - return result + v.AppendAll(args...) + return v } diff --git a/client/go/script-utils/startcbinary/valgrind.go b/client/go/script-utils/startcbinary/valgrind.go index ecbca36e823..ffbbd8cca8d 100644 --- a/client/go/script-utils/startcbinary/valgrind.go +++ b/client/go/script-utils/startcbinary/valgrind.go @@ -73,14 +73,10 @@ func (p *ProgSpec) valgrindLogOption() string { } func (p *ProgSpec) prependValgrind(args []string) []string { - result := make([]string, 0, 15+len(args)) - result = append(result, p.valgrindBinary()) - for _, arg := range p.valgrindOptions() { - result = append(result, arg) - } - result = append(result, p.valgrindLogOption()) - for _, arg := range args { - result = append(result, arg) - } - return result + v := util.NewArrayList[string](15 + len(args)) + v.Append(p.valgrindBinary()) + v.AppendAll(p.valgrindOptions()...) + v.Append(p.valgrindLogOption()) + v.AppendAll(args...) + return v } diff --git a/client/go/util/array_list.go b/client/go/util/array_list.go new file mode 100644 index 00000000000..2e74d30fcec --- /dev/null +++ b/client/go/util/array_list.go @@ -0,0 +1,81 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Author: arnej + +// generic utilities +package util + +type ArrayList[E comparable] []E + +func NewArrayList[E comparable](initialCapacity int) ArrayList[E] { + return make([]E, 0, initialCapacity) +} + +func ArrayListOf[E comparable](elems []E) ArrayList[E] { + return ArrayList[E](elems) +} + +func (arrayP *ArrayList[E]) Append(elem E) { + *arrayP = append(*arrayP, elem) +} + +func (arrayP *ArrayList[E]) AppendAll(elemsToAppend ...E) { + firstLen := len(*arrayP) + secondLen := len(elemsToAppend) + totLen := firstLen + secondLen + if totLen > cap(*arrayP) { + res := make([]E, totLen, cap(*arrayP)+cap(elemsToAppend)) + copy(res, *arrayP) + copy(res[firstLen:], elemsToAppend) + *arrayP = res + } else { + res := (*arrayP)[0:totLen] + copy(res[firstLen:], elemsToAppend) + *arrayP = res + } +} + +func (arrayP *ArrayList[E]) Insert(index int, elem E) { + cur := *arrayP + oldLen := len(cur) + result := append(cur, elem) + if index != oldLen { + copy(result[index+1:], cur[index:]) + result[index] = elem + } + *arrayP = result +} + +func (arrayP *ArrayList[E]) InsertAll(index int, elemsToInsert ...E) { + firstLen := len(*arrayP) + secondLen := len(elemsToInsert) + totLen := firstLen + secondLen + var res []E + if totLen > cap(*arrayP) { + res = make([]E, totLen, cap(*arrayP)+cap(elemsToInsert)) + firstPart := (*arrayP)[:index] + copy(res, firstPart) + } else { + res = (*arrayP)[0:totLen] + } + thirdPart := (*arrayP)[index:] + dst := res[index+secondLen:] + copy(dst, thirdPart) + dst = res[index:] + copy(dst, elemsToInsert) + *arrayP = res +} + +func (arrayP *ArrayList[E]) Contains(elem E) bool { + for _, old := range *arrayP { + if elem == old { + return true + } + } + return false +} + +func (arrayP *ArrayList[E]) Each(f func(E)) { + for _, elem := range *arrayP { + f(elem) + } +} diff --git a/client/go/util/array_list_test.go b/client/go/util/array_list_test.go new file mode 100644 index 00000000000..79eab4f8ef2 --- /dev/null +++ b/client/go/util/array_list_test.go @@ -0,0 +1,110 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package util + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestArrayApi1(t *testing.T) { + v := ArrayList[string]{"x", "y", "z"} + assert.Equal(t, 3, len(v)) + v.Append("a") + assert.Equal(t, 4, len(v)) + v.Insert(2, "b") + assert.Equal(t, 5, len(v)) + v.Insert(0, "c") + assert.Equal(t, 6, len(v)) + assert.Equal(t, "c", v[0]) + assert.Equal(t, "x", v[1]) + assert.Equal(t, "y", v[2]) + assert.Equal(t, "b", v[3]) + assert.Equal(t, "z", v[4]) + assert.Equal(t, "a", v[5]) +} + +func TestArrayApi2(t *testing.T) { + tmp := []string{"i", "j", "k"} + v := NewArrayList[string](10) + assert.Equal(t, 0, len(v)) + assert.Equal(t, 10, cap(v)) + v.AppendAll(tmp...) + assert.Equal(t, 3, len(v)) + assert.Equal(t, 10, cap(v)) + v.AppendAll(tmp...) + assert.Equal(t, 6, len(v)) + assert.Equal(t, 10, cap(v)) + v.AppendAll(tmp...) + assert.Equal(t, 9, len(v)) + assert.Equal(t, 10, cap(v)) + v.AppendAll(tmp...) + assert.Equal(t, 12, len(v)) + assert.Less(t, 11, cap(v)) +} + +func TestArrayApi3(t *testing.T) { + tmp := []string{"i", "j", "k"} + v := ArrayList[string]{ + "foo", "bar", + "baz", "qux", + } + assert.Equal(t, 4, len(v)) + v.InsertAll(0, "a", "b") + assert.Equal(t, 6, len(v)) + v.AppendAll(tmp...) + assert.Equal(t, 9, len(v)) + v.AppendAll("x", "y") + assert.Equal(t, 11, len(v)) + v.InsertAll(4, "foobar", "barfoo") + assert.Equal(t, 13, len(v)) + assert.Equal(t, "a", v[0]) + assert.Equal(t, "b", v[1]) + assert.Equal(t, "foo", v[2]) + assert.Equal(t, "bar", v[3]) + assert.Equal(t, "foobar", v[4]) + assert.Equal(t, "barfoo", v[5]) + assert.Equal(t, "baz", v[6]) + assert.Equal(t, "qux", v[7]) + assert.Equal(t, "i", v[8]) + assert.Equal(t, "j", v[9]) + assert.Equal(t, "k", v[10]) + assert.Equal(t, "x", v[11]) + assert.Equal(t, "y", v[12]) +} + +func TestArrayApi4(t *testing.T) { + v := NewArrayList[string](12) + arr := v[0:10] + v.InsertAll(0, "a", "b", "e") + v.InsertAll(3, "f", "g", "o") + v.InsertAll(2, "c", "d") + v.InsertAll(7, "h", "i", "j", "k", "l", "m", "n") + assert.Equal(t, 15, len(v)) + assert.Equal(t, "a", v[0]) + assert.Equal(t, "b", v[1]) + assert.Equal(t, "c", v[2]) + assert.Equal(t, "d", v[3]) + assert.Equal(t, "e", v[4]) + assert.Equal(t, "f", v[5]) + assert.Equal(t, "g", v[6]) + assert.Equal(t, "h", v[7]) + assert.Equal(t, "i", v[8]) + assert.Equal(t, "j", v[9]) + assert.Equal(t, "k", v[10]) + assert.Equal(t, "l", v[11]) + assert.Equal(t, "m", v[12]) + assert.Equal(t, "n", v[13]) + assert.Equal(t, "o", v[14]) + assert.Equal(t, 10, len(arr)) + assert.Equal(t, "a", arr[0]) + assert.Equal(t, "b", arr[1]) + assert.Equal(t, "c", arr[2]) + assert.Equal(t, "d", arr[3]) + assert.Equal(t, "e", arr[4]) + assert.Equal(t, "f", arr[5]) + assert.Equal(t, "g", arr[6]) + assert.Equal(t, "o", arr[7]) + assert.Equal(t, "", arr[8]) + assert.Equal(t, "", arr[9]) +} diff --git a/config-provisioning/src/main/resources/configdefinitions/config.provisioning.cloud.def b/config-provisioning/src/main/resources/configdefinitions/config.provisioning.cloud.def index 05ccee34d59..3aeb5bc527e 100644 --- a/config-provisioning/src/main/resources/configdefinitions/config.provisioning.cloud.def +++ b/config-provisioning/src/main/resources/configdefinitions/config.provisioning.cloud.def @@ -14,3 +14,6 @@ requireAccessControl bool default=false # The default account used to provision hosts and load balancers in this zone. account string default="" + +# The cloud-specific region for this zone (as opposed to the Vespa region). +region string default="" diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStore.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStore.java index a3ec30bb122..21255ae83bf 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStore.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStore.java @@ -16,6 +16,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.function.Predicate; import java.util.stream.Collectors; import static java.nio.charset.StandardCharsets.UTF_8; @@ -62,7 +63,7 @@ public class BufferedLogStore { return; // Max size exceeded — store no more. byte[] emptyChunk = "[]".getBytes(); - byte[] lastChunk = buffer.readLog(id, type, lastChunkId).orElse(emptyChunk); + byte[] lastChunk = buffer.readLog(id, type, lastChunkId).filter(chunk -> chunk.length > 0).orElse(emptyChunk); long sizeLowerBound = lastChunk.length; Map<Step, List<LogEntry>> log = logSerializer.fromJson(lastChunk, -1); diff --git a/document/src/vespa/document/base/testdocrepo.cpp b/document/src/vespa/document/base/testdocrepo.cpp index 8dcb2d66410..d67f44ee731 100644 --- a/document/src/vespa/document/base/testdocrepo.cpp +++ b/document/src/vespa/document/base/testdocrepo.cpp @@ -58,6 +58,7 @@ DocumenttypesConfig TestDocRepo::getDefaultConfig() { .addTensorField("sparse_xy_tensor", "tensor(x{},y{})") .addTensorField("sparse_float_tensor", "tensor<float>(x{})") .addTensorField("dense_tensor", "tensor(x[2])")) + .imported_field("my_imported_field") .doc_type.fieldsets["[document]"].fields.swap(documentfields); builder.document(type2_id, "testdoctype2", diff --git a/flags/src/main/java/com/yahoo/vespa/flags/custom/ClusterCapacity.java b/flags/src/main/java/com/yahoo/vespa/flags/custom/ClusterCapacity.java index b24d5f62174..dcef85f9a0d 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/custom/ClusterCapacity.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/custom/ClusterCapacity.java @@ -7,10 +7,15 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; - import java.util.Objects; import java.util.OptionalDouble; +import static com.yahoo.vespa.flags.custom.Validation.requireNonNegative; +import static com.yahoo.vespa.flags.custom.Validation.validArchitectures; +import static com.yahoo.vespa.flags.custom.Validation.validDiskSpeeds; +import static com.yahoo.vespa.flags.custom.Validation.validStorageTypes; +import static com.yahoo.vespa.flags.custom.Validation.validateEnum; + /** * @author freva */ @@ -23,23 +28,33 @@ public class ClusterCapacity { private final OptionalDouble memoryGb; private final OptionalDouble diskGb; private final OptionalDouble bandwidthGbps; + private final String diskSpeed; + private final String storageType; + private final String architecture; @JsonCreator public ClusterCapacity(@JsonProperty("count") int count, @JsonProperty("vcpu") Double vcpu, @JsonProperty("memoryGb") Double memoryGb, @JsonProperty("diskGb") Double diskGb, - @JsonProperty("bandwidthGbps") Double bandwidthGbps) { + @JsonProperty("bandwidthGbps") Double bandwidthGbps, + @JsonProperty("diskSpeed") String diskSpeed, + @JsonProperty("storageType") String storageType, + @JsonProperty("architecture") String architecture) { this.count = (int) requireNonNegative("count", count); this.vcpu = vcpu == null ? OptionalDouble.empty() : OptionalDouble.of(requireNonNegative("vcpu", vcpu)); this.memoryGb = memoryGb == null ? OptionalDouble.empty() : OptionalDouble.of(requireNonNegative("memoryGb", memoryGb)); this.diskGb = diskGb == null ? OptionalDouble.empty() : OptionalDouble.of(requireNonNegative("diskGb", diskGb)); this.bandwidthGbps = bandwidthGbps == null ? OptionalDouble.empty() : OptionalDouble.of(bandwidthGbps); + this.diskSpeed = validateEnum("diskSpeed", validDiskSpeeds, diskSpeed == null ? "fast" : diskSpeed); + this.storageType = validateEnum("storageType", validStorageTypes, storageType == null ? "any" : storageType); + this.architecture = validateEnum("architecture", validArchitectures, architecture == null ? "x86_64" : architecture); } /** Returns a new ClusterCapacity equal to {@code this}, but with the given count. */ public ClusterCapacity withCount(int count) { - return new ClusterCapacity(count, vcpuOrNull(), memoryGbOrNull(), diskGbOrNull(), bandwidthGbpsOrNull()); + return new ClusterCapacity(count, vcpuOrNull(), memoryGbOrNull(), diskGbOrNull(), bandwidthGbpsOrNull(), + diskSpeed, storageType, architecture); } @JsonGetter("count") public int count() { return count; } @@ -55,13 +70,14 @@ public class ClusterCapacity { @JsonGetter("bandwidthGbps") public Double bandwidthGbpsOrNull() { return bandwidthGbps.isPresent() ? bandwidthGbps.getAsDouble() : null; } + @JsonGetter("diskSpeed") public String diskSpeed() { return diskSpeed; } + @JsonGetter("storageType") public String storageType() { return storageType; } + @JsonGetter("architecture") public String architecture() { return architecture; } @JsonIgnore public Double vcpu() { return vcpu.orElse(0.0); } @JsonIgnore public Double memoryGb() { return memoryGb.orElse(0.0); } @JsonIgnore public Double diskGb() { return diskGb.orElse(0.0); } - - @JsonIgnore - public double bandwidthGbps() { return bandwidthGbps.orElse(1.0); } + @JsonIgnore public double bandwidthGbps() { return bandwidthGbps.orElse(1.0); } @Override public String toString() { @@ -71,6 +87,9 @@ public class ClusterCapacity { ", memoryGb=" + memoryGb + ", diskGb=" + diskGb + ", bandwidthGbps=" + bandwidthGbps + + ", diskSpeed=" + diskSpeed + + ", storageType=" + storageType + + ", architecture=" + architecture + '}'; } @@ -83,18 +102,15 @@ public class ClusterCapacity { vcpu.equals(that.vcpu) && memoryGb.equals(that.memoryGb) && diskGb.equals(that.diskGb) && - bandwidthGbps.equals(that.bandwidthGbps); + bandwidthGbps.equals(that.bandwidthGbps) && + diskSpeed.equals(that.diskSpeed) && + storageType.equals(that.storageType) && + architecture.equals(that.architecture); } @Override public int hashCode() { - return Objects.hash(count, vcpu, memoryGb, diskGb, bandwidthGbps); - } - - private static double requireNonNegative(String name, double value) { - if (value < 0) - throw new IllegalArgumentException("'" + name + "' must be positive, was " + value); - return value; + return Objects.hash(count, vcpu, memoryGb, diskGb, bandwidthGbps, diskSpeed, storageType, architecture); } } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/custom/HostResources.java b/flags/src/main/java/com/yahoo/vespa/flags/custom/HostResources.java index 6b2cac39065..b7a1fd0be32 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/custom/HostResources.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/custom/HostResources.java @@ -5,10 +5,15 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; - import java.util.Objects; import java.util.Optional; -import java.util.Set; + +import static com.yahoo.vespa.flags.custom.Validation.requirePositive; +import static com.yahoo.vespa.flags.custom.Validation.validArchitectures; +import static com.yahoo.vespa.flags.custom.Validation.validClusterTypes; +import static com.yahoo.vespa.flags.custom.Validation.validDiskSpeeds; +import static com.yahoo.vespa.flags.custom.Validation.validStorageTypes; +import static com.yahoo.vespa.flags.custom.Validation.validateEnum; /** * The advertised node resources of a host, similar to config-provision's NodeResources, @@ -18,10 +23,6 @@ import java.util.Set; */ @JsonIgnoreProperties(ignoreUnknown = true) public class HostResources { - private static final Set<String> validDiskSpeeds = Set.of("slow", "fast"); - private static final Set<String> validStorageTypes = Set.of("remote", "local"); - private static final Set<String> validClusterTypes = Set.of("container", "content", "combined", "admin"); - private static final Set<String> validArchitectures = Set.of("arm64", "x86_64"); private final double vcpu; private final double memoryGb; @@ -91,32 +92,6 @@ public class HostResources { return this.clusterType.map(clusterType::equalsIgnoreCase).orElse(true); } - private static double requirePositive(String name, Double value) { - requireNonNull(name, value); - if (value <= 0) - throw new IllegalArgumentException("'" + name + "' must be positive, was " + value); - return value; - } - - private static int requirePositive(String name, Integer value) { - requireNonNull(name, value); - if (value <= 0) - throw new IllegalArgumentException("'" + name + "' must be positive, was " + value); - return value; - } - - private static String validateEnum(String name, Set<String> validValues, String value) { - requireNonNull(name, value); - if (!validValues.contains(value)) - throw new IllegalArgumentException("Invalid " + name + ", valid values are: " + - validValues + ", got: " + value); - return value; - } - - private static <T> T requireNonNull(String name, T value) { - return Objects.requireNonNull(value, () -> "'" + name + "' has not been specified"); - } - @Override public String toString() { return "HostResources{" + diff --git a/flags/src/main/java/com/yahoo/vespa/flags/custom/Validation.java b/flags/src/main/java/com/yahoo/vespa/flags/custom/Validation.java new file mode 100644 index 00000000000..7e831aa5046 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/custom/Validation.java @@ -0,0 +1,45 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags.custom; + +import java.util.Objects; +import java.util.Set; + +public class Validation { + static final Set<String> validDiskSpeeds = Set.of("slow", "fast", "any"); + static final Set<String> validStorageTypes = Set.of("remote", "local", "any"); + static final Set<String> validClusterTypes = Set.of("container", "content", "combined", "admin"); + static final Set<String> validArchitectures = Set.of("arm64", "x86_64", "any"); + + static double requirePositive(String name, Double value) { + requireNonNull(name, value); + if (value <= 0) + throw new IllegalArgumentException("'" + name + "' must be positive, was " + value); + return value; + } + + static int requirePositive(String name, Integer value) { + requireNonNull(name, value); + if (value <= 0) + throw new IllegalArgumentException("'" + name + "' must be positive, was " + value); + return value; + } + + static double requireNonNegative(String name, double value) { + if (value < 0) + throw new IllegalArgumentException("'" + name + "' must be positive, was " + value); + return value; + } + + static String validateEnum(String name, Set<String> validValues, String value) { + requireNonNull(name, value); + if (!validValues.contains(value)) + throw new IllegalArgumentException("Invalid " + name + ", valid values are: " + + validValues + ", got: " + value); + return value; + } + + private static <T> T requireNonNull(String name, T value) { + return Objects.requireNonNull(value, () -> "'" + name + "' has not been specified"); + } + +} diff --git a/flags/src/test/java/com/yahoo/vespa/flags/custom/ClusterCapacityTest.java b/flags/src/test/java/com/yahoo/vespa/flags/custom/ClusterCapacityTest.java index 796cdbf30d1..23ab3a48ffa 100644 --- a/flags/src/test/java/com/yahoo/vespa/flags/custom/ClusterCapacityTest.java +++ b/flags/src/test/java/com/yahoo/vespa/flags/custom/ClusterCapacityTest.java @@ -12,47 +12,69 @@ public class ClusterCapacityTest { @Test void serialization() throws IOException { - ClusterCapacity clusterCapacity = new ClusterCapacity(7, 1.2, 3.4, 5.6, null); + ClusterCapacity clusterCapacity = new ClusterCapacity(7, 1.2, 3.4, 5.6, null, "fast", "local", "x86_64"); ObjectMapper mapper = new ObjectMapper(); String json = mapper.writeValueAsString(clusterCapacity); - assertEquals("{\"count\":7,\"vcpu\":1.2,\"memoryGb\":3.4,\"diskGb\":5.6}", json); + assertEquals("{\"count\":7,\"vcpu\":1.2,\"memoryGb\":3.4,\"diskGb\":5.6,\"diskSpeed\":\"fast\",\"storageType\":\"local\",\"architecture\":\"x86_64\"}", json); ClusterCapacity deserialized = mapper.readValue(json, ClusterCapacity.class); + assertEquals(7, deserialized.count()); assertEquals(1.2, deserialized.vcpu(), 0.0001); assertEquals(3.4, deserialized.memoryGb(), 0.0001); assertEquals(5.6, deserialized.diskGb(), 0.0001); assertEquals(1.0, deserialized.bandwidthGbps(), 0.0001); - assertEquals(7, deserialized.count()); + assertEquals("fast", deserialized.diskSpeed()); + assertEquals("local", deserialized.storageType()); + assertEquals("x86_64", deserialized.architecture()); } @Test void serialization2() throws IOException { - ClusterCapacity clusterCapacity = new ClusterCapacity(7, 1.2, 3.4, 5.6, 2.3); + ClusterCapacity clusterCapacity = new ClusterCapacity(7, 1.2, 3.4, 5.6, 2.3, "any", "remote", "arm64"); ObjectMapper mapper = new ObjectMapper(); String json = mapper.writeValueAsString(clusterCapacity); - assertEquals("{\"count\":7,\"vcpu\":1.2,\"memoryGb\":3.4,\"diskGb\":5.6,\"bandwidthGbps\":2.3}", json); + assertEquals("{\"count\":7,\"vcpu\":1.2,\"memoryGb\":3.4,\"diskGb\":5.6,\"bandwidthGbps\":2.3,\"diskSpeed\":\"any\",\"storageType\":\"remote\",\"architecture\":\"arm64\"}", json); ClusterCapacity deserialized = mapper.readValue(json, ClusterCapacity.class); + assertEquals(7, deserialized.count()); assertEquals(1.2, deserialized.vcpu(), 0.0001); assertEquals(3.4, deserialized.memoryGb(), 0.0001); assertEquals(5.6, deserialized.diskGb(), 0.0001); assertEquals(2.3, deserialized.bandwidthGbps(), 0.0001); - assertEquals(7, deserialized.count()); + assertEquals("any", deserialized.diskSpeed()); + assertEquals("remote", deserialized.storageType()); + assertEquals("arm64", deserialized.architecture()); } @Test void serializationWithNoNodeResources() throws IOException { - ClusterCapacity clusterCapacity = new ClusterCapacity(7, null, null, null, null); + ClusterCapacity clusterCapacity = new ClusterCapacity(7, null, null, null, null, null, null, null); ObjectMapper mapper = new ObjectMapper(); String json = mapper.writeValueAsString(clusterCapacity); - assertEquals("{\"count\":7}", json); + assertEquals("{\"count\":7,\"diskSpeed\":\"fast\",\"storageType\":\"any\",\"architecture\":\"x86_64\"}", json); ClusterCapacity deserialized = mapper.readValue(json, ClusterCapacity.class); + assertEquals(7, deserialized.count()); assertEquals(0.0, deserialized.vcpu(), 0.0001); assertEquals(0.0, deserialized.memoryGb(), 0.0001); assertEquals(0.0, deserialized.diskGb(), 0.0001); assertEquals(1.0, deserialized.bandwidthGbps(), 0.0001); // 1.0 is used as fallback + assertEquals("fast", deserialized.diskSpeed()); + assertEquals("any", deserialized.storageType()); + assertEquals("x86_64", deserialized.architecture()); + + + // Test that using no values for diskSpeed, storageType and architecture will give expected values (the default values) + var input = "{\"count\":7}"; + deserialized = mapper.readValue(input, ClusterCapacity.class); assertEquals(7, deserialized.count()); + assertEquals(0.0, deserialized.vcpu(), 0.0001); + assertEquals(0.0, deserialized.memoryGb(), 0.0001); + assertEquals(0.0, deserialized.diskGb(), 0.0001); + assertEquals(1.0, deserialized.bandwidthGbps(), 0.0001); // 1.0 is used as fallback + assertEquals("fast", deserialized.diskSpeed()); + assertEquals("any", deserialized.storageType()); + assertEquals("x86_64", deserialized.architecture()); } }
\ No newline at end of file diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java index 345dccf5faf..7dfeefb4e88 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java @@ -198,6 +198,7 @@ public class Nodes { if (node.status().wantToDeprovision() || node.status().wantToRebuild()) return park(node.hostname(), false, agent, reason); + node = node.withWantToRetire(false, false, false, agent, clock.instant()); return db.writeTo(Node.State.ready, node, agent, Optional.of(reason)); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java index 375517278b2..28530ddd39a 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java @@ -82,7 +82,7 @@ public class HostCapacityMaintainerTest { @Test public void does_not_deprovision_when_preprovisioning_enabled() { var tester = new DynamicProvisioningTester().addInitialNodes(); - tester.flagSource.withListFlag(PermanentFlags.PREPROVISION_CAPACITY.id(), List.of(new ClusterCapacity(1, 1.0, 3.0, 2.0, 1.0)), ClusterCapacity.class); + tester.flagSource.withListFlag(PermanentFlags.PREPROVISION_CAPACITY.id(), List.of(new ClusterCapacity(1, 1.0, 3.0, 2.0, 1.0, "fast", "local", "x86_64")), ClusterCapacity.class); Optional<Node> failedHost = tester.nodeRepository.nodes().node("host2"); assertTrue(failedHost.isPresent()); @@ -95,8 +95,8 @@ public class HostCapacityMaintainerTest { public void provision_deficit_and_deprovision_excess() { var tester = new DynamicProvisioningTester().addInitialNodes(); tester.flagSource.withListFlag(PermanentFlags.PREPROVISION_CAPACITY.id(), - List.of(new ClusterCapacity(2, 48.0, 128.0, 1000.0, 10.0), - new ClusterCapacity(1, 16.0, 24.0, 100.0, 1.0)), + List.of(new ClusterCapacity(2, 48.0, 128.0, 1000.0, 10.0, "fast", "local", "x86_64"), + new ClusterCapacity(1, 16.0, 24.0, 100.0, 1.0, "fast", "local", "x86_64")), ClusterCapacity.class); assertEquals(0, tester.hostProvisioner.provisionedHosts().size()); @@ -125,9 +125,9 @@ public class HostCapacityMaintainerTest { var tester = new DynamicProvisioningTester().addInitialNodes(); // Makes provisioned hosts 48-128-1000-10 tester.hostProvisioner.overrideHostFlavor("host4"); - + var clusterCapacity = new ClusterCapacity(2, 1.0, 30.0, 20.0, 3.0, "fast", "local", "x86_64"); tester.flagSource.withListFlag(PermanentFlags.PREPROVISION_CAPACITY.id(), - List.of(new ClusterCapacity(2, 1.0, 30.0, 20.0, 3.0)), + List.of(clusterCapacity), ClusterCapacity.class); assertEquals(0, tester.hostProvisioner.provisionedHosts().size()); @@ -147,10 +147,9 @@ public class HostCapacityMaintainerTest { verifyFirstMaintain(tester); // Add a second cluster equal to the first. It should fit on existing host3 and host100. - tester.flagSource.withListFlag(PermanentFlags.PREPROVISION_CAPACITY.id(), - List.of(new ClusterCapacity(2, 1.0, 30.0, 20.0, 3.0), - new ClusterCapacity(2, 1.0, 30.0, 20.0, 3.0)), + List.of(clusterCapacity, + clusterCapacity), ClusterCapacity.class); tester.maintain(); @@ -163,8 +162,8 @@ public class HostCapacityMaintainerTest { // host3 is a 24-64-100-10 while host100 is 48-128-1000-10. tester.flagSource.withListFlag(PermanentFlags.PREPROVISION_CAPACITY.id(), - List.of(new ClusterCapacity(2, 1.0, 30.0, 20.0, 3.0), - new ClusterCapacity(2, 24.0, 64.0, 100.0, 1.0)), + List.of(clusterCapacity, + new ClusterCapacity(2, 24.0, 64.0, 100.0, 1.0, "fast", "local", "x86_64")), ClusterCapacity.class); tester.maintain(); @@ -179,7 +178,7 @@ public class HostCapacityMaintainerTest { // If the preprovision capacity is reduced, we should see shared hosts deprovisioned. tester.flagSource.withListFlag(PermanentFlags.PREPROVISION_CAPACITY.id(), - List.of(new ClusterCapacity(1, 1.0, 30.0, 20.0, 3.0)), + List.of(new ClusterCapacity(1, 1.0, 30.0, 20.0, 3.0, "fast", "local", "x86_64")), ClusterCapacity.class); tester.maintain(); @@ -267,7 +266,9 @@ public class HostCapacityMaintainerTest { var tester = new DynamicProvisioningTester(); NodeResources resources1 = new NodeResources(24, 64, 100, 10); tester.flagSource.withListFlag(PermanentFlags.PREPROVISION_CAPACITY.id(), - List.of(new ClusterCapacity(2, resources1.vcpu(), resources1.memoryGb(), resources1.diskGb(), resources1.bandwidthGbps())), + List.of(new ClusterCapacity(2, resources1.vcpu(), resources1.memoryGb(), resources1.diskGb(), + resources1.bandwidthGbps(), resources1.diskSpeed().name(), + resources1.storageType().name(), resources1.architecture().name())), ClusterCapacity.class); tester.maintain(); @@ -287,7 +288,7 @@ public class HostCapacityMaintainerTest { // Must be able to allocate 2 nodes with "no resource requirement" tester.flagSource.withListFlag(PermanentFlags.PREPROVISION_CAPACITY.id(), - List.of(new ClusterCapacity(2, 0.0, 0.0, 0.0, 0.0)), + List.of(new ClusterCapacity(2, 0.0, 0.0, 0.0, 0.0, null, null, null)), ClusterCapacity.class); // Next maintenance run does nothing @@ -312,7 +313,7 @@ public class HostCapacityMaintainerTest { // Increasing the capacity provisions additional hosts tester.flagSource.withListFlag(PermanentFlags.PREPROVISION_CAPACITY.id(), - List.of(new ClusterCapacity(3, 0.0, 0.0, 0.0, 0.0)), + List.of(new ClusterCapacity(3, 0.0, 0.0, 0.0, 0.0, null, null, null)), ClusterCapacity.class); assertEquals(0, tester.provisionedHostsMatching(sharedHostNodeResources)); assertTrue(tester.nodeRepository.nodes().node("host102").isEmpty()); @@ -329,7 +330,10 @@ public class HostCapacityMaintainerTest { resources1.vcpu() - applicationNodeResources.vcpu(), resources1.memoryGb() - applicationNodeResources.memoryGb(), resources1.diskGb() - applicationNodeResources.diskGb(), - resources1.bandwidthGbps() - applicationNodeResources.bandwidthGbps())), + resources1.bandwidthGbps() - applicationNodeResources.bandwidthGbps(), + resources1.diskSpeed().name(), + resources1.storageType().name(), + resources1.architecture().name())), ClusterCapacity.class); tester.assertNodesUnchanged(); @@ -339,7 +343,10 @@ public class HostCapacityMaintainerTest { resources1.vcpu() - applicationNodeResources.vcpu() + 1, resources1.memoryGb() - applicationNodeResources.memoryGb() + 1, resources1.diskGb() - applicationNodeResources.diskGb() + 1, - resources1.bandwidthGbps())), + resources1.bandwidthGbps(), + resources1.diskSpeed().name(), + resources1.storageType().name(), + resources1.architecture().name())), ClusterCapacity.class); assertEquals(1, tester.provisionedHostsMatching(sharedHostNodeResources)); diff --git a/searchcore/src/tests/proton/common/timer/CMakeLists.txt b/searchcore/src/tests/proton/common/timer/CMakeLists.txt index 89b9ecc688a..54d31e7e888 100644 --- a/searchcore/src/tests/proton/common/timer/CMakeLists.txt +++ b/searchcore/src/tests/proton/common/timer/CMakeLists.txt @@ -4,5 +4,6 @@ vespa_add_executable(searchcore_common_timer_test_app TEST timer_test.cpp DEPENDS searchcore_pcommon + GTest::GTest ) vespa_add_test(NAME searchcore_common_timer_test_app COMMAND searchcore_common_timer_test_app) diff --git a/searchcore/src/tests/proton/common/timer/timer_test.cpp b/searchcore/src/tests/proton/common/timer/timer_test.cpp index 9eea67623b6..a0ad3378b09 100644 --- a/searchcore/src/tests/proton/common/timer/timer_test.cpp +++ b/searchcore/src/tests/proton/common/timer/timer_test.cpp @@ -1,13 +1,17 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/vespalib/testkit/testapp.h> +#include <vespa/fastos/thread.h> +#include <vespa/fnet/transport.h> +#include <vespa/searchcore/proton/common/scheduled_forward_executor.h> #include <vespa/searchcore/proton/common/scheduledexecutor.h> +#include <vespa/vespalib/gtest/gtest.h> +#include <vespa/vespalib/util/count_down_latch.h> #include <vespa/vespalib/util/size_literals.h> -#include <vespa/fnet/transport.h> -#include <vespa/fastos/thread.h> +#include <vespa/vespalib/util/threadstackexecutor.h> using vespalib::Executor; -typedef Executor::Task Task; +using namespace proton; +using Task = Executor::Task; namespace { @@ -21,34 +25,63 @@ public: } -TEST("testScheduling") { +template <typename T> +std::unique_ptr<T> make_scheduled_executor(FNET_Transport& transport, vespalib::Executor& executor); + +template <> +std::unique_ptr<ScheduledExecutor> +make_scheduled_executor<ScheduledExecutor>(FNET_Transport& transport, vespalib::Executor&) { + return std::make_unique<ScheduledExecutor>(transport); +} + +template <> +std::unique_ptr<ScheduledForwardExecutor> +make_scheduled_executor<ScheduledForwardExecutor>(FNET_Transport& transport, vespalib::Executor& executor) { + return std::make_unique<ScheduledForwardExecutor>(transport, executor); +} + +template <typename ScheduledT> +class ScheduledExecutorTest : public testing::Test { +public: + FastOS_ThreadPool threadPool; + FNET_Transport transport; + vespalib::ThreadStackExecutor executor; + std::unique_ptr<ScheduledT> timer; + + ScheduledExecutorTest() + : threadPool(64_Ki), + transport(), + executor(1, 64_Ki) + { + transport.Start(&threadPool); + timer = make_scheduled_executor<ScheduledT>(transport, executor); + } + ~ScheduledExecutorTest() { + timer->reset(); + transport.ShutDown(true); + } +}; + +using ScheduledTypes = ::testing::Types<ScheduledExecutor, ScheduledForwardExecutor>; + +TYPED_TEST_SUITE(ScheduledExecutorTest, ScheduledTypes); + +TYPED_TEST(ScheduledExecutorTest, test_scheduling) { vespalib::CountDownLatch latch1(3); vespalib::CountDownLatch latch2(2); - FastOS_ThreadPool threadPool(64_Ki); - FNET_Transport transport; - transport.Start(&threadPool); - proton::ScheduledExecutor timer(transport); - timer.scheduleAtFixedRate(std::make_unique<TestTask>(latch1), 100ms, 200ms); - timer.scheduleAtFixedRate(std::make_unique<TestTask>(latch2), 500ms, 500ms); + this->timer->scheduleAtFixedRate(std::make_unique<TestTask>(latch1), 100ms, 200ms); + this->timer->scheduleAtFixedRate(std::make_unique<TestTask>(latch2), 500ms, 500ms); EXPECT_TRUE(latch1.await(60s)); EXPECT_TRUE(latch2.await(60s)); - timer.reset(); - transport.ShutDown(true); } -TEST("testReset") { +TYPED_TEST(ScheduledExecutorTest, test_reset) { vespalib::CountDownLatch latch1(2); - FastOS_ThreadPool threadPool(64_Ki); - FNET_Transport transport; - transport.Start(&threadPool); - proton::ScheduledExecutor timer(transport); - timer.scheduleAtFixedRate(std::make_unique<TestTask>(latch1), 2s, 3s); - timer.reset(); + this->timer->scheduleAtFixedRate(std::make_unique<TestTask>(latch1), 2s, 3s); + this->timer->reset(); EXPECT_TRUE(!latch1.await(3s)); - timer.scheduleAtFixedRate(std::make_unique<TestTask>(latch1), 200ms, 300ms); + this->timer->scheduleAtFixedRate(std::make_unique<TestTask>(latch1), 200ms, 300ms); EXPECT_TRUE(latch1.await(60s)); - timer.reset(); - transport.ShutDown(true); } -TEST_MAIN() { TEST_RUN_ALL(); } +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchcore/src/vespa/searchcore/proton/common/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/common/CMakeLists.txt index fffbd12764b..d42f764fa77 100644 --- a/searchcore/src/vespa/searchcore/proton/common/CMakeLists.txt +++ b/searchcore/src/vespa/searchcore/proton/common/CMakeLists.txt @@ -20,12 +20,13 @@ vespa_add_library(searchcore_pcommon STATIC pendinglidtracker.cpp replay_feed_token_factory.cpp replay_feedtoken_state.cpp + scheduled_forward_executor.cpp + scheduledexecutor.cpp select_utils.cpp selectcontext.cpp selectpruner.cpp state_reporter_utils.cpp statusreport.cpp - scheduledexecutor.cpp DEPENDS searchcore_proton_metrics searchcore_fconfig diff --git a/searchcore/src/vespa/searchcore/proton/common/i_scheduled_executor.h b/searchcore/src/vespa/searchcore/proton/common/i_scheduled_executor.h new file mode 100644 index 00000000000..b95e65da045 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/common/i_scheduled_executor.h @@ -0,0 +1,28 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#pragma once + +#include <vespa/vespalib/util/executor.h> +#include <vespa/vespalib/util/time.h> +#include <memory> + +namespace proton { + +/** + * Interface used to run Tasks at a regular interval. + */ +class IScheduledExecutor { +public: + virtual ~IScheduledExecutor() = default; + + /** + * Schedule a new task to be executed at specified intervals. + * + * @param task The task to schedule. + * @param delay The delay to wait before first execution. + * @param interval The interval between the task is executed. + */ + virtual void scheduleAtFixedRate(std::unique_ptr<vespalib::Executor::Task> task, + vespalib::duration delay, vespalib::duration interval) = 0; +}; + +} diff --git a/searchcore/src/vespa/searchcore/proton/common/scheduled_forward_executor.cpp b/searchcore/src/vespa/searchcore/proton/common/scheduled_forward_executor.cpp new file mode 100644 index 00000000000..acb94c020f6 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/common/scheduled_forward_executor.cpp @@ -0,0 +1,35 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "scheduled_forward_executor.h" +#include <vespa/vespalib/util/lambdatask.h> + +using vespalib::Executor; +using vespalib::makeLambdaTask; + +namespace proton { + +ScheduledForwardExecutor::ScheduledForwardExecutor(FNET_Transport& transport, + vespalib::Executor& executor) + : _scheduler(transport), + _executor(executor) +{ +} + +void +ScheduledForwardExecutor::reset() +{ + _scheduler.reset(); +} + +void +ScheduledForwardExecutor::scheduleAtFixedRate(vespalib::Executor::Task::UP task, + vespalib::duration delay, vespalib::duration interval) +{ + _scheduler.scheduleAtFixedRate(makeLambdaTask([&, my_task = std::move(task)]() { + _executor.execute(makeLambdaTask([&]() { + my_task->run(); + })); + }), delay, interval); +} + +} diff --git a/searchcore/src/vespa/searchcore/proton/common/scheduled_forward_executor.h b/searchcore/src/vespa/searchcore/proton/common/scheduled_forward_executor.h new file mode 100644 index 00000000000..eb7120527d7 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/common/scheduled_forward_executor.h @@ -0,0 +1,29 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#pragma once + +#include "i_scheduled_executor.h" +#include "scheduledexecutor.h" + +class FNET_Transport; + +namespace proton { + +/** + * This class posts Tasks at a regular interval to another executor which runs them. + */ +class ScheduledForwardExecutor : public IScheduledExecutor { +private: + ScheduledExecutor _scheduler; + vespalib::Executor& _executor; + +public: + ScheduledForwardExecutor(FNET_Transport& transport, vespalib::Executor& executor); + void reset(); + + void scheduleAtFixedRate(std::unique_ptr<vespalib::Executor::Task> task, + vespalib::duration delay, vespalib::duration interval) override; + +}; + +} + diff --git a/searchcore/src/vespa/searchcore/proton/common/scheduledexecutor.h b/searchcore/src/vespa/searchcore/proton/common/scheduledexecutor.h index 80c8b7edd15..dfb16c94d7d 100644 --- a/searchcore/src/vespa/searchcore/proton/common/scheduledexecutor.h +++ b/searchcore/src/vespa/searchcore/proton/common/scheduledexecutor.h @@ -1,8 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -#include <vespa/vespalib/util/executor.h> -#include <vespa/vespalib/util/time.h> +#include "i_scheduled_executor.h" #include <mutex> #include <vector> @@ -17,7 +16,7 @@ class TimerTask; * interval. The timer can be reset to clear all tasks currently being * scheduled. */ -class ScheduledExecutor +class ScheduledExecutor : public IScheduledExecutor { private: using TaskList = std::vector<std::unique_ptr<TimerTask>>; @@ -37,16 +36,9 @@ public: * Destroys this timer, finishing the current task executing and then * finishing. */ - ~ScheduledExecutor(); + ~ScheduledExecutor() override; - /** - * Schedule new task to be executed at specified intervals. - * - * @param task The task to schedule. - * @param delay The delay to wait before first execution. - * @param interval The interval in seconds. - */ - void scheduleAtFixedRate(std::unique_ptr<Executor::Task> task, duration delay, duration interval); + void scheduleAtFixedRate(std::unique_ptr<Executor::Task> task, duration delay, duration interval) override; /** * Reset timer, clearing the list of task to execute. diff --git a/searchlib/src/vespa/searchlib/queryeval/predicate_search.cpp b/searchlib/src/vespa/searchlib/queryeval/predicate_search.cpp index dc769d2a03d..b61f6912907 100644 --- a/searchlib/src/vespa/searchlib/queryeval/predicate_search.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/predicate_search.cpp @@ -234,9 +234,6 @@ uint32_t addInterval(uint32_t interval, uint64_t subquery, return end; } } -void restoreSortedOrder(size_t first, size_t last, - vector<uint16_t> &indexes, - const vector<uint32_t> &intervals) __attribute__((noinline)); // One step of insertion sort: First element is moved to correct position. void restoreSortedOrder(size_t first, size_t last, diff --git a/storage/src/tests/persistence/testandsettest.cpp b/storage/src/tests/persistence/testandsettest.cpp index 48e0e838add..5be1c7cd92a 100644 --- a/storage/src/tests/persistence/testandsettest.cpp +++ b/storage/src/tests/persistence/testandsettest.cpp @@ -222,6 +222,18 @@ TEST_F(TestAndSetTest, invalid_document_selection_should_fail) { EXPECT_EQ("", dumpBucket(BUCKET_ID)); } +TEST_F(TestAndSetTest, document_selection_with_imported_field_should_fail_with_illegal_parameters) { + api::Timestamp timestamp = 0; + auto put = std::make_shared<api::PutCommand>(BUCKET, testDoc, timestamp); + put->setCondition(documentapi::TestAndSetCondition("testdoctype1.my_imported_field == null")); + + ASSERT_EQ(fetchResult(asyncHandler->handlePut(*put, createTracker(put, BUCKET))), + api::ReturnCode(api::ReturnCode::Result::ILLEGAL_PARAMETERS, + "Condition field 'my_imported_field' could not be found, or is an imported field. " + "Imported fields are not supported in conditional mutations.")); + EXPECT_EQ("", dumpBucket(BUCKET_ID)); +} + TEST_F(TestAndSetTest, conditional_put_to_non_existing_document_should_fail) { // Conditionally replace nonexisting document // Fail since no document exists to match with test and set diff --git a/storage/src/vespa/storage/persistence/testandsethelper.cpp b/storage/src/vespa/storage/persistence/testandsethelper.cpp index 9396e95c152..393dac09f72 100644 --- a/storage/src/vespa/storage/persistence/testandsethelper.cpp +++ b/storage/src/vespa/storage/persistence/testandsethelper.cpp @@ -5,6 +5,7 @@ #include "persistenceutil.h" #include "fieldvisitor.h" #include <vespa/persistence/spi/persistenceprovider.h> +#include <vespa/document/base/exceptions.h> #include <vespa/document/select/parser.h> #include <vespa/document/repo/documenttyperepo.h> #include <vespa/vespalib/util/stringfmt.h> @@ -61,7 +62,14 @@ api::ReturnCode TestAndSetHelper::retrieveAndMatch(spi::Context & context) { // Walk document selection tree to build a minimal field set FieldVisitor fieldVisitor(*_docTypePtr); - _docSelectionUp->visit(fieldVisitor); + try { + _docSelectionUp->visit(fieldVisitor); + } catch (const document::FieldNotFoundException& e) { + return api::ReturnCode(api::ReturnCode::ILLEGAL_PARAMETERS, + vespalib::make_string("Condition field '%s' could not be found, or is an imported field. " + "Imported fields are not supported in conditional mutations.", + e.getFieldName().c_str())); + } // Retrieve document auto result = retrieveDocument(fieldVisitor.getFieldSet(), context); diff --git a/storage/src/vespa/storage/storageserver/statemanager.cpp b/storage/src/vespa/storage/storageserver/statemanager.cpp index b75400e1308..81961370ed3 100644 --- a/storage/src/vespa/storage/storageserver/statemanager.cpp +++ b/storage/src/vespa/storage/storageserver/statemanager.cpp @@ -6,6 +6,9 @@ #include <vespa/document/bucket/fixed_bucket_spaces.h> #include <vespa/metrics/jsonwriter.h> #include <vespa/metrics/metricmanager.h> +#include <vespa/metrics/metricset.h> +#include <vespa/metrics/metrictimer.h> +#include <vespa/metrics/valuemetric.h> #include <vespa/storageapi/messageapi/storagemessage.h> #include <vespa/vdslib/state/cluster_state_bundle.h> #include <vespa/vdslib/state/clusterstate.h> @@ -22,6 +25,20 @@ LOG_SETUP(".state.manager"); namespace storage { +struct StateManager::StateManagerMetrics : metrics::MetricSet { + metrics::DoubleAverageMetric invoke_state_listeners_latency; + + explicit StateManagerMetrics(metrics::MetricSet* owner = nullptr) + : metrics::MetricSet("state_manager", {}, "", owner), + invoke_state_listeners_latency("invoke_state_listeners_latency", {}, + "Time spent (in ms) propagating state changes to internal state listeners", this) + {} + + ~StateManagerMetrics() override; +}; + +StateManager::StateManagerMetrics::~StateManagerMetrics() = default; + using lib::ClusterStateBundle; StateManager::StateManager(StorageComponentRegister& compReg, @@ -32,6 +49,7 @@ StateManager::StateManager(StorageComponentRegister& compReg, framework::HtmlStatusReporter("systemstate", "Node and system state"), _component(compReg, "statemanager"), _metricManager(metricManager), + _metrics(std::make_unique<StateManagerMetrics>()), _stateLock(), _stateCond(), _listenerLock(), @@ -55,6 +73,7 @@ StateManager::StateManager(StorageComponentRegister& compReg, _nodeState->setMinUsedBits(58); _nodeState->setStartTimestamp(_component.getClock().getTimeInSeconds().getTime()); _component.registerStatusPage(*this); + _component.registerMetric(*_metrics); } StateManager::~StateManager() @@ -246,6 +265,7 @@ StateManager::notifyStateListeners() } _stateCond.notify_all(); } + metrics::MetricTimer handler_latency_timer; for (auto* listener : _stateListeners) { listener->handleNewState(); // If one of them actually altered the state again, abort @@ -255,6 +275,7 @@ StateManager::notifyStateListeners() break; } } + handler_latency_timer.stop(_metrics->invoke_state_listeners_latency); } if (newState) { sendGetNodeStateReplies(); diff --git a/storage/src/vespa/storage/storageserver/statemanager.h b/storage/src/vespa/storage/storageserver/statemanager.h index 0df07d048eb..74b59875ff8 100644 --- a/storage/src/vespa/storage/storageserver/statemanager.h +++ b/storage/src/vespa/storage/storageserver/statemanager.h @@ -45,8 +45,11 @@ class StateManager : public NodeStateUpdater, using TimeStateCmdPair = std::pair<framework::MilliSecTime, api::GetNodeStateCommand::SP>; using TimeSysStatePair = std::pair<framework::MilliSecTime, std::shared_ptr<const ClusterStateBundle>>; + struct StateManagerMetrics; + StorageComponent _component; metrics::MetricManager& _metricManager; + std::unique_ptr<StateManagerMetrics> _metrics; mutable std::mutex _stateLock; std::condition_variable _stateCond; std::mutex _listenerLock; diff --git a/vespa-feed-client-cli/vespa-feed-client-dev.sh b/vespa-feed-client-cli/vespa-feed-client-dev.sh new file mode 100755 index 00000000000..1da93a73f91 --- /dev/null +++ b/vespa-feed-client-cli/vespa-feed-client-dev.sh @@ -0,0 +1,12 @@ +#!/usr/bin/env sh +# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +# Resolve symlink (if any) and normalize path +program=$(readlink -f "$0") +program_dir=$(dirname "$program") + +exec java \ +-Djava.awt.headless=true \ +-Xms128m -Xmx2048m \ +-Djava.util.logging.config.file=$program_dir/src/main/resources/logging.properties \ +-cp $program_dir/target/vespa-feed-client-cli-jar-with-dependencies.jar ai.vespa.feed.client.impl.CliClient "$@" |