diff options
74 files changed, 1405 insertions, 472 deletions
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ZooKeeperTestServer.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ZooKeeperTestServer.java index 34c8fafa702..73b4163d542 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ZooKeeperTestServer.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ZooKeeperTestServer.java @@ -8,6 +8,7 @@ import org.apache.zookeeper.server.ZooKeeperServer; import java.io.File; import java.io.IOException; import java.net.InetSocketAddress; +import java.time.Duration; /** * This class sets up a zookeeper server, such that we can test fleetcontroller zookeeper parts without stubbing in the client. @@ -15,7 +16,7 @@ import java.net.InetSocketAddress; public class ZooKeeperTestServer { private File zooKeeperDir; private ZooKeeperServer server; - private static final int tickTime = 100; + private static final Duration tickTime = Duration.ofMillis(2000); private NIOServerCnxnFactory factory; private static final String DIR_PREFIX = "test_fltctrl_zk"; private static final String DIR_POSTFIX = "sdir"; @@ -31,7 +32,7 @@ public class ZooKeeperTestServer { throw new IllegalStateException("Failed to create directory " + zooKeeperDir); } zooKeeperDir.deleteOnExit(); - server = new ZooKeeperServer(zooKeeperDir, zooKeeperDir, tickTime); + server = new ZooKeeperServer(zooKeeperDir, zooKeeperDir, (int)tickTime.toMillis()); final int maxcc = 10000; // max number of connections from the same client factory = new NIOServerCnxnFactory(); factory.configure(new InetSocketAddress(port), maxcc); // Use any port diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/Index.java b/config-model/src/main/java/com/yahoo/searchdefinition/Index.java index 90f061d933d..aba6cf9a233 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/Index.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/Index.java @@ -10,11 +10,11 @@ import java.io.Serializable; import java.util.Collections; import java.util.Iterator; import java.util.LinkedHashSet; +import java.util.Locale; import java.util.Objects; import java.util.Optional; import java.util.Set; - /** * An index definition in a search definition. * Two indices are equal if they have the same name and the same settings, except @@ -24,6 +24,8 @@ import java.util.Set; */ public class Index implements Cloneable, Serializable { + public static enum DistanceMetric { EUCLIDEAN, ANGULAR, GEODEGREES } + public enum Type { VESPA("vespa"); @@ -61,7 +63,9 @@ public class Index implements Cloneable, Serializable { /** The boolean index definition, if set */ private BooleanIndexDefinition boolIndex; - private Optional<HnswIndexParams> hnswIndexParams; + private Optional<HnswIndexParams> hnswIndexParams = Optional.empty(); + + private Optional<DistanceMetric> distanceMetric = Optional.empty(); /** Whether the posting lists of this index field should have interleaved features (num occs, field length) in document id stream. */ private boolean interleavedFeatures = false; @@ -134,12 +138,13 @@ public class Index implements Cloneable, Serializable { stemming == index.stemming && type == index.type && Objects.equals(boolIndex, index.boolIndex) && + Objects.equals(distanceMetric, index.distanceMetric) && Objects.equals(hnswIndexParams, index.hnswIndexParams); } @Override public int hashCode() { - return Objects.hash(name, rankType, prefix, aliases, stemming, normalized, type, boolIndex, hnswIndexParams, interleavedFeatures); + return Objects.hash(name, rankType, prefix, aliases, stemming, normalized, type, boolIndex, distanceMetric, hnswIndexParams, interleavedFeatures); } public String toString() { @@ -187,6 +192,16 @@ public class Index implements Cloneable, Serializable { boolIndex = def; } + public Optional<DistanceMetric> getDistanceMetric() { + return distanceMetric; + } + + public void setDistanceMetric(String value) { + String upper = value.toUpperCase(Locale.ENGLISH); + DistanceMetric dm = DistanceMetric.valueOf(upper); + distanceMetric = Optional.of(dm); + } + public Optional<HnswIndexParams> getHnswIndexParams() { return hnswIndexParams; } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/AttributeFields.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/AttributeFields.java index 5b87fdcf5f6..8b5f7658475 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/AttributeFields.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/AttributeFields.java @@ -240,13 +240,14 @@ public class AttributeFields extends Derived implements AttributesConfig.Produce aaB.tensortype(attribute.tensorType().get().toString()); } aaB.imported(imported); + var dma = attribute.distanceMetric(); if (attribute.hnswIndexParams().isPresent()) { var ib = new AttributesConfig.Attribute.Index.Builder(); var params = attribute.hnswIndexParams().get(); ib.hnsw.enabled(true); ib.hnsw.maxlinkspernode(params.maxLinksPerNode()); ib.hnsw.neighborstoexploreatinsert(params.neighborsToExploreAtInsert()); - var dm = AttributesConfig.Attribute.Index.Hnsw.Distancemetric.Enum.valueOf(params.distanceMetric().toString()); + var dm = AttributesConfig.Attribute.Index.Hnsw.Distancemetric.Enum.valueOf(dma.toString()); ib.hnsw.distancemetric(dm); aaB.index(ib); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/document/Attribute.java b/config-model/src/main/java/com/yahoo/searchdefinition/document/Attribute.java index 9ed5e4ca2de..1661a80f238 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/document/Attribute.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/document/Attribute.java @@ -24,6 +24,7 @@ import com.yahoo.document.datatypes.Float16FieldValue; import com.yahoo.document.datatypes.StringFieldValue; import com.yahoo.document.datatypes.TensorFieldValue; import com.yahoo.tensor.TensorType; +import static com.yahoo.searchdefinition.Index.DistanceMetric; import java.io.Serializable; import java.util.LinkedHashSet; @@ -66,7 +67,9 @@ public final class Attribute implements Cloneable, Serializable { /** This is set if the type of this is REFERENCE */ private final Optional<StructuredDataType> referenceDocumentType; - private Optional<HnswIndexParams> hnswIndexParams; + private Optional<DistanceMetric> distanceMetric = Optional.empty(); + + private Optional<HnswIndexParams> hnswIndexParams = Optional.empty(); private boolean isPosition = false; private final Sorting sorting = new Sorting(); @@ -152,7 +155,6 @@ public final class Attribute implements Cloneable, Serializable { setCollectionType(collectionType); this.tensorType = tensorType; this.referenceDocumentType = referenceDocumentType; - this.hnswIndexParams = Optional.empty(); } public Attribute convertToArray() { @@ -197,6 +199,11 @@ public final class Attribute implements Cloneable, Serializable { public double densePostingListThreshold() { return densePostingListThreshold; } public Optional<TensorType> tensorType() { return tensorType; } public Optional<StructuredDataType> referenceDocumentType() { return referenceDocumentType; } + + public static final DistanceMetric DEFAULT_DISTANCE_METRIC = DistanceMetric.EUCLIDEAN; + public DistanceMetric distanceMetric() { + return distanceMetric.orElse(DEFAULT_DISTANCE_METRIC); + } public Optional<HnswIndexParams> hnswIndexParams() { return hnswIndexParams; } public Sorting getSorting() { return sorting; } @@ -221,6 +228,7 @@ public final class Attribute implements Cloneable, Serializable { public void setUpperBound(long upperBound) { this.upperBound = upperBound; } public void setDensePostingListThreshold(double threshold) { this.densePostingListThreshold = threshold; } public void setTensorType(TensorType tensorType) { this.tensorType = Optional.of(tensorType); } + public void setDistanceMetric(Optional<DistanceMetric> dm) { this.distanceMetric = dm; } public void setHnswIndexParams(HnswIndexParams params) { this.hnswIndexParams = Optional.of(params); } public String getName() { return name; } @@ -354,8 +362,8 @@ public final class Attribute implements Cloneable, Serializable { /** Returns whether these attributes describes the same entity, even if they have different names */ public boolean isCompatible(Attribute other) { - if ( ! this.type.equals(other.type)) return false; - if ( ! this.collectionType.equals(other.collectionType)) return false; + if (! this.type.equals(other.type)) return false; + if (! this.collectionType.equals(other.collectionType)) return false; if (this.isPrefetch() != other.isPrefetch()) return false; if (this.removeIfZero != other.removeIfZero) return false; if (this.createIfNonExistent != other.createIfNonExistent) return false; @@ -364,10 +372,11 @@ public final class Attribute implements Cloneable, Serializable { // if (this.noSearch != other.noSearch) return false; No backend consequences so compatible for now if (this.fastSearch != other.fastSearch) return false; if (this.huge != other.huge) return false; - if ( ! this.sorting.equals(other.sorting)) return false; - if (!this.tensorType.equals(other.tensorType)) return false; - if (!this.referenceDocumentType.equals(other.referenceDocumentType)) return false; - if (!this.hnswIndexParams.equals(other.hnswIndexParams)) return false; + if (! this.sorting.equals(other.sorting)) return false; + if (! Objects.equals(tensorType, other.tensorType)) return false; + if (! Objects.equals(referenceDocumentType, other.referenceDocumentType)) return false; + if (! Objects.equals(distanceMetric, other.distanceMetric)) return false; + if (! Objects.equals(hnswIndexParams, other.hnswIndexParams)) return false; return true; } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/document/HnswIndexParams.java b/config-model/src/main/java/com/yahoo/searchdefinition/document/HnswIndexParams.java index 01434be8785..2f084d3e513 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/document/HnswIndexParams.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/document/HnswIndexParams.java @@ -13,18 +13,13 @@ public class HnswIndexParams { public static final int DEFAULT_MAX_LINKS_PER_NODE = 16; public static final int DEFAULT_NEIGHBORS_TO_EXPLORE_AT_INSERT = 200; - public static final DistanceMetric DEFAULT_DISTANCE_METRIC = DistanceMetric.EUCLIDEAN; private final Optional<Integer> maxLinksPerNode; private final Optional<Integer> neighborsToExploreAtInsert; - private final Optional<DistanceMetric> distanceMetric; - - public static enum DistanceMetric { EUCLIDEAN, ANGULAR, GEODEGREES } public static class Builder { private Optional<Integer> maxLinksPerNode = Optional.empty(); private Optional<Integer> neighborsToExploreAtInsert = Optional.empty(); - private Optional<DistanceMetric> distanceMetric = Optional.empty(); public void setMaxLinksPerNode(int value) { maxLinksPerNode = Optional.of(value); @@ -32,38 +27,31 @@ public class HnswIndexParams { public void setNeighborsToExploreAtInsert(int value) { neighborsToExploreAtInsert = Optional.of(value); } - public void setDistanceMetric(String value) { - String upper = value.toUpperCase(Locale.ENGLISH); - DistanceMetric dm = DistanceMetric.valueOf(upper); - distanceMetric = Optional.of(dm); - } public HnswIndexParams build() { - return new HnswIndexParams(maxLinksPerNode, neighborsToExploreAtInsert, distanceMetric); + return new HnswIndexParams(maxLinksPerNode, neighborsToExploreAtInsert); } } public HnswIndexParams() { this.maxLinksPerNode = Optional.empty(); this.neighborsToExploreAtInsert = Optional.empty(); - this.distanceMetric = Optional.empty(); } public HnswIndexParams(Optional<Integer> maxLinksPerNode, - Optional<Integer> neighborsToExploreAtInsert, - Optional<DistanceMetric> distanceMetric) { + Optional<Integer> neighborsToExploreAtInsert) { this.maxLinksPerNode = maxLinksPerNode; this.neighborsToExploreAtInsert = neighborsToExploreAtInsert; - this.distanceMetric = distanceMetric; } /** * Creates a new instance where values from the given parameter instance are used where they are present, * otherwise we use values from this. */ - public HnswIndexParams overrideFrom(HnswIndexParams rhs) { + public HnswIndexParams overrideFrom(Optional<HnswIndexParams> other) { + if (! other.isPresent()) return this; + HnswIndexParams rhs = other.get(); return new HnswIndexParams(rhs.maxLinksPerNode.or(() -> maxLinksPerNode), - rhs.neighborsToExploreAtInsert.or(() -> neighborsToExploreAtInsert), - rhs.distanceMetric.or(() -> distanceMetric)); + rhs.neighborsToExploreAtInsert.or(() -> neighborsToExploreAtInsert)); } public int maxLinksPerNode() { @@ -73,8 +61,4 @@ public class HnswIndexParams { public int neighborsToExploreAtInsert() { return neighborsToExploreAtInsert.orElse(DEFAULT_NEIGHBORS_TO_EXPLORE_AT_INSERT); } - - public DistanceMetric distanceMetric() { - return distanceMetric.orElse(DEFAULT_DISTANCE_METRIC); - } } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/fieldoperation/IndexOperation.java b/config-model/src/main/java/com/yahoo/searchdefinition/fieldoperation/IndexOperation.java index 7f9da28b9ca..0c1f443dee3 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/fieldoperation/IndexOperation.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/fieldoperation/IndexOperation.java @@ -32,6 +32,7 @@ public class IndexOperation implements FieldOperation { private OptionalDouble densePostingListThreshold = OptionalDouble.empty(); private Optional<Boolean> enableBm25 = Optional.empty(); + private Optional<String> distanceMetric = Optional.empty(); private Optional<HnswIndexParams.Builder> hnswIndexParams = Optional.empty(); public String getIndexName() { @@ -94,6 +95,9 @@ public class IndexOperation implements FieldOperation { if (enableBm25.isPresent()) { index.setInterleavedFeatures(enableBm25.get()); } + if (distanceMetric.isPresent()) { + index.setDistanceMetric(distanceMetric.get()); + } if (hnswIndexParams.isPresent()) { index.setHnswIndexParams(hnswIndexParams.get().build()); } @@ -127,6 +131,10 @@ public class IndexOperation implements FieldOperation { enableBm25 = Optional.of(value); } + public void setDistanceMetric(String value) { + this.distanceMetric = Optional.of(value); + } + public void setHnswIndexParams(HnswIndexParams.Builder params) { this.hnswIndexParams = Optional.of(params); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/TensorFieldProcessor.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/TensorFieldProcessor.java index 2790f2ddf6e..c97ee2bd935 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/TensorFieldProcessor.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/TensorFieldProcessor.java @@ -81,8 +81,9 @@ public class TensorFieldProcessor extends Processor { var index = field.getIndex(field.getName()); // TODO: Calculate default params based on tensor dimension size var params = new HnswIndexParams(); - if (index != null && index.getHnswIndexParams().isPresent()) { - params = params.overrideFrom(index.getHnswIndexParams().get()); + if (index != null) { + params = params.overrideFrom(index.getHnswIndexParams()); + field.getAttribute().setDistanceMetric(index.getDistanceMetric()); } field.getAttribute().setHnswIndexParams(params); } diff --git a/config-model/src/main/javacc/SDParser.jj b/config-model/src/main/javacc/SDParser.jj index cca56c209c8..3560cf2cd84 100644 --- a/config-model/src/main/javacc/SDParser.jj +++ b/config-model/src/main/javacc/SDParser.jj @@ -1816,6 +1816,7 @@ Object indexBody(IndexOperation index) : | <UPPERBOUND> <COLON> num = consumeLong() { index.setUpperBound(num); } | <DENSEPOSTINGLISTTHRESHOLD> <COLON> threshold = consumeFloat() { index.setDensePostingListThreshold(threshold); } | <ENABLE_BM25> { index.setEnableBm25(true); } + | <DISTANCEMETRIC> <COLON> str = identifierWithDash() { index.setDistanceMetric(str); } | hnswIndex(index) { } ) { return null; } @@ -1841,7 +1842,6 @@ void hnswIndexBody(HnswIndexParams.Builder params) : } { ( <MAXLINKSPERNODE> <COLON> num = integer() { params.setMaxLinksPerNode(num); } - | <DISTANCEMETRIC> <COLON> str = identifierWithDash() { params.setDistanceMetric(str); } | <NEIGHBORSTOEXPLOREATINSERT> <COLON> num = integer() { params.setNeighborsToExploreAtInsert(num); } ) } diff --git a/config-model/src/test/derived/hnsw_index/test.sd b/config-model/src/test/derived/hnsw_index/test.sd index 3b954e74fc5..207ed764a87 100644 --- a/config-model/src/test/derived/hnsw_index/test.sd +++ b/config-model/src/test/derived/hnsw_index/test.sd @@ -3,9 +3,9 @@ search test { field t1 type tensor(x[128]) { indexing: attribute | index index { + distance-metric: angular hnsw { max-links-per-node: 32 - distance-metric: angular neighbors-to-explore-at-insert: 300 } } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/NearestNeighborTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/NearestNeighborTestCase.java index ead4e586d9f..9f57b22fd58 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/derived/NearestNeighborTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/NearestNeighborTestCase.java @@ -31,6 +31,9 @@ public class NearestNeighborTestCase extends AbstractExportingTestCase { } catch (QueryException e) { // success assertEquals("Invalid request parameter", e.getMessage()); + } catch (RuntimeException e) { + e.printStackTrace(); + throw e; } } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/document/HnswIndexParamsTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/document/HnswIndexParamsTestCase.java index d687590faf2..e3dcc925e5e 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/document/HnswIndexParamsTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/document/HnswIndexParamsTestCase.java @@ -2,13 +2,13 @@ package com.yahoo.searchdefinition.document; +import java.util.Optional; import org.junit.Test; import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; -import static com.yahoo.searchdefinition.document.HnswIndexParams.DistanceMetric; public class HnswIndexParamsTestCase { @@ -18,35 +18,27 @@ public class HnswIndexParamsTestCase { var builder = new HnswIndexParams.Builder(); builder.setMaxLinksPerNode(7); var one = builder.build(); - builder.setDistanceMetric("angular"); - var two = builder.build(); builder.setNeighborsToExploreAtInsert(42); var three = builder.build(); builder.setMaxLinksPerNode(17); - builder.setDistanceMetric("geodegrees"); builder.setNeighborsToExploreAtInsert(500); var four = builder.build(); assertThat(empty.maxLinksPerNode(), is(16)); - assertThat(empty.distanceMetric(), is(DistanceMetric.EUCLIDEAN)); assertThat(empty.neighborsToExploreAtInsert(), is(200)); assertThat(one.maxLinksPerNode(), is(7)); - assertThat(two.distanceMetric(), is(DistanceMetric.ANGULAR)); assertThat(three.neighborsToExploreAtInsert(), is(42)); assertThat(four.maxLinksPerNode(), is(17)); - assertThat(four.distanceMetric(), is(DistanceMetric.GEODEGREES)); assertThat(four.neighborsToExploreAtInsert(), is(500)); - var five = four.overrideFrom(empty); + var five = four.overrideFrom(Optional.of(empty)); assertThat(five.maxLinksPerNode(), is(17)); - assertThat(five.distanceMetric(), is(DistanceMetric.GEODEGREES)); assertThat(five.neighborsToExploreAtInsert(), is(500)); - var six = four.overrideFrom(two); + var six = four.overrideFrom(Optional.of(one)); assertThat(six.maxLinksPerNode(), is(7)); - assertThat(six.distanceMetric(), is(DistanceMetric.ANGULAR)); assertThat(six.neighborsToExploreAtInsert(), is(500)); } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterResources.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterResources.java index 11873bc908c..a4ed22c5266 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterResources.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterResources.java @@ -44,6 +44,13 @@ public class ClusterResources { return false; } + /** Returns true if this is within the given limits (inclusive) */ + public boolean isWithin(ClusterResources min, ClusterResources max) { + if (this.smallerThan(min)) return false; + if (max.smallerThan(this)) return false; + return true; + } + @Override public boolean equals(Object o) { if (o == this) return true; @@ -52,7 +59,7 @@ public class ClusterResources { ClusterResources other = (ClusterResources)o; if (other.nodes != this.nodes) return false; if (other.groups != this.groups) return false; - if (other.nodeResources != this.nodeResources) return false; + if ( ! other.nodeResources.equals(this.nodeResources)) return false; return true; } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java index 6d7fe752e46..5fc05a87a7d 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java @@ -114,26 +114,32 @@ public class NodeResources { public StorageType storageType() { return storageType; } public NodeResources withVcpu(double vcpu) { + if (vcpu == this.vcpu) return this; return new NodeResources(vcpu, memoryGb, diskGb, bandwidthGbps, diskSpeed, storageType); } public NodeResources withMemoryGb(double memoryGb) { + if (memoryGb == this.memoryGb) return this; return new NodeResources(vcpu, memoryGb, diskGb, bandwidthGbps, diskSpeed, storageType); } public NodeResources withDiskGb(double diskGb) { + if (diskGb == this.diskGb) return this; return new NodeResources(vcpu, memoryGb, diskGb, bandwidthGbps, diskSpeed, storageType); } public NodeResources withBandwidthGbps(double bandwidthGbps) { + if (bandwidthGbps == this.bandwidthGbps) return this; return new NodeResources(vcpu, memoryGb, diskGb, bandwidthGbps, diskSpeed, storageType); } - public NodeResources with(DiskSpeed speed) { - return new NodeResources(vcpu, memoryGb, diskGb, bandwidthGbps, speed, storageType); + public NodeResources with(DiskSpeed diskSpeed) { + if (diskSpeed == this.diskSpeed) return this; + return new NodeResources(vcpu, memoryGb, diskGb, bandwidthGbps, diskSpeed, storageType); } public NodeResources with(StorageType storageType) { + if (storageType == this.storageType) return this; return new NodeResources(vcpu, memoryGb, diskGb, bandwidthGbps, diskSpeed, storageType); } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/TenantId.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/TenantId.java index 4974192e213..3ac24bac7ca 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/TenantId.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/TenantId.java @@ -10,10 +10,6 @@ public class TenantId extends NonDefaultIdentifier { super(id); } - public boolean isUser() { - return id().startsWith("by-"); - } - @Override public void validate() { super.validate(); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/UserId.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/UserId.java index d2effc76827..f1a8e57ab03 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/UserId.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/identifiers/UserId.java @@ -10,8 +10,4 @@ public class UserId extends NonDefaultIdentifier { super(id); } - public TenantId toTenantId() { - return new TenantId("by-" + id().replace('_', '-')); - } - } diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/identifiers/IdentifierTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/identifiers/IdentifierTest.java index 8e278240a02..fdba1ab2680 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/identifiers/IdentifierTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/identifiers/IdentifierTest.java @@ -107,11 +107,6 @@ public class IdentifierTest { } @Test - public void user_tenant_id_does_not_contain_underscore() { - assertEquals("by-under-score-user", new UserId("under_score_user").toTenantId().id()); - } - - @Test public void dns_names_has_no_underscore() { assertEquals("a-b-c", new ApplicationId("a_b_c").toDns()); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index be3f4e50dc7..08f22ac778e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -205,7 +205,6 @@ public class ApplicationApiHandler extends LoggingRequestHandler { private HttpResponse handleGET(Path path, HttpRequest request) { if (path.matches("/application/v4/")) return root(request); - if (path.matches("/application/v4/user")) return authenticatedUser(request); if (path.matches("/application/v4/tenant")) return tenants(request); if (path.matches("/application/v4/tenant/{tenant}")) return tenant(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/cost")) return tenantCost(path.get("tenant"), request); @@ -248,7 +247,6 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } private HttpResponse handlePUT(Path path, HttpRequest request) { - if (path.matches("/application/v4/user")) return new EmptyResponse(); if (path.matches("/application/v4/tenant/{tenant}")) return updateTenant(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/global-rotation/override")) return setGlobalRotationOverride(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), false, request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/global-rotation/override")) return setGlobalRotationOverride(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), false, request); @@ -325,24 +323,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { private HttpResponse root(HttpRequest request) { return recurseOverTenants(request) ? recursiveRoot(request) - : new ResourceResponse(request, "user", "tenant"); - } - - // TODO jonmv: Move to Athenz API. - private HttpResponse authenticatedUser(HttpRequest request) { - Principal user = requireUserPrincipal(request); - - String userName = user instanceof AthenzPrincipal ? ((AthenzPrincipal) user).getIdentity().getName() : user.getName(); - List<Tenant> tenants = controller.tenants().asList(new Credentials(user)); - - Slime slime = new Slime(); - Cursor response = slime.setObject(); - response.setString("user", userName); - Cursor tenantsArray = response.setArray("tenants"); - for (Tenant tenant : tenants) - tenantInTenantsListToSlime(tenant, request.getUri(), tenantsArray.addObject()); - response.setBool("tenantExists", true); - return new SlimeJsonResponse(slime); + : new ResourceResponse(request, "tenant"); } private HttpResponse tenants(HttpRequest request) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index fd0981e8427..2752ba64b61 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -176,24 +176,6 @@ public class ApplicationApiTest extends ControllerContainerTest { .oktaAccessToken(OKTA_AT).oktaIdentityToken(OKTA_IT) .data("{\"athensDomain\":\"domain1\", \"property\":\"property1\"}"), new File("tenant-without-applications.json")); - // GET the authenticated user (with associated tenants) - tester.assertResponse(request("/application/v4/user", GET).userIdentity(USER_ID), - new File("user.json")); - // TODO jonmv: Remove when dashboard is gone. - // PUT a user tenant — does nothing - tester.assertResponse(request("/application/v4/user", PUT).userIdentity(USER_ID), - ""); - - // GET the authenticated user which now exists (with associated tenants) - tester.assertResponse(request("/application/v4/user", GET).userIdentity(USER_ID), - new File("user.json")); - - // DELETE the user — it doesn't exist, so access control fails - tester.assertResponse(request("/application/v4/tenant/by-myuser", DELETE).userIdentity(USER_ID), - "{\n \"code\" : 403,\n \"message\" : \"Access denied\"\n}", 403); - // GET all tenants - tester.assertResponse(request("/application/v4/tenant/", GET).userIdentity(USER_ID), - new File("tenant-list.json")); // GET list of months for a tenant tester.assertResponse(request("/application/v4/tenant/tenant1/cost", GET).userIdentity(USER_ID).oktaAccessToken(OKTA_AT).oktaIdentityToken(OKTA_IT), @@ -783,11 +765,6 @@ public class ApplicationApiTest extends ControllerContainerTest { .userIdentity(USER_ID), "{\"message\":\"Aborting run 2 of staging-test for tenant1.application1.instance1\"}"); - // GET user lists only tenants for the authenticated user - tester.assertResponse(request("/application/v4/user", GET) - .userIdentity(new UserId("other_user")), - "{\"user\":\"other_user\",\"tenants\":[],\"tenantExists\":true}"); - // OPTIONS return 200 OK tester.assertResponse(request("/application/v4/", Request.Method.OPTIONS) .userIdentity(USER_ID), @@ -1108,14 +1085,6 @@ public class ApplicationApiTest extends ControllerContainerTest { "{\"error-code\":\"BAD_REQUEST\",\"message\":\"New tenant or application names must start with a letter, may contain no more than 20 characters, and may only contain lowercase letters, digits or dashes, but no double-dashes.\"}", 400); - // POST (add) an Athenz tenant with by- prefix - tester.assertResponse(request("/application/v4/tenant/by-tenant2", POST) - .userIdentity(USER_ID) - .data("{\"athensDomain\":\"domain1\", \"property\":\"property1\"}") - .oktaAccessToken(OKTA_AT).oktaIdentityToken(OKTA_IT), - "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Athenz tenant name cannot have prefix 'by-'\"}", - 400); - // POST (add) an Athenz tenant with a reserved name tester.assertResponse(request("/application/v4/tenant/hosted-vespa", POST) .userIdentity(USER_ID) @@ -1395,25 +1364,12 @@ public class ApplicationApiTest extends ControllerContainerTest { createAthenzDomainWithAdmin(ATHENZ_TENANT_DOMAIN, tenantAdmin); allowLaunchOfService(new com.yahoo.vespa.athenz.api.AthenzService(ATHENZ_TENANT_DOMAIN, "service")); - // Create tenant - // PUT (create) the authenticated user - tester.assertResponse(request("/application/v4/user?user=new_user&domain=by", PUT) - .userIdentity(userId), // Normalized to by-new-user by API - ""); - ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .athenzIdentity(com.yahoo.config.provision.AthenzDomain.from("domain1"), com.yahoo.config.provision.AthenzService.from("service")) .build(); - // POST (deploy) an application to a dev zone fails because user tenant is used — these do not exist. - MultiPartStreamer entity = createApplicationDeployData(applicationPackage, true); - tester.assertResponse(request("/application/v4/tenant/by-new-user/application/application1/environment/dev/region/us-west-1/instance/default", POST) - .data(entity) - .userIdentity(userId), - "{\n \"code\" : 403,\n \"message\" : \"Access denied\"\n}", - 403); - createTenantAndApplication(); + MultiPartStreamer entity = createApplicationDeployData(applicationPackage, true); // POST (deploy) an application to dev through a deployment job, with user instance and a proper tenant tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/new-user/deploy/dev-us-east-1", POST) .data(entity) @@ -1426,13 +1382,6 @@ public class ApplicationApiTest extends ControllerContainerTest { .domains.get(ATHENZ_TENANT_DOMAIN) .admin(HostedAthenzIdentities.from(userId)); - // POST (deploy) an application to a dev zone fails because user tenant is used — these do not exist. - tester.assertResponse(request("/application/v4/tenant/by-new-user/application/application1/environment/dev/region/us-west-1/instance/default", POST) - .data(entity) - .userIdentity(userId), - "{\n \"code\" : 403,\n \"message\" : \"Access denied\"\n}", - 403); - // POST (deploy) an application to dev through a deployment job tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/new-user/deploy/dev-us-east-1", POST) .data(entity) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/root.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/root.json index 986245decca..d63a7ba7d56 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/root.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/root.json @@ -1,9 +1,6 @@ { "resources":[ { - "url":"http://localhost:8080/application/v4/user/" - }, - { "url":"http://localhost:8080/application/v4/tenant/" } ] diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/user-which-exists.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/user-which-exists.json deleted file mode 100644 index f2703677738..00000000000 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/user-which-exists.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "user": "myuser", - "tenants": @include(tenant-list-with-user.json), - "tenantExists": true -}
\ No newline at end of file diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/user.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/user.json deleted file mode 100644 index 9902267dbb5..00000000000 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/user.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "user": "myuser", - "tenants": @include(tenant-list.json), - "tenantExists": true -}
\ No newline at end of file diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java index 6db5bc9f523..51466e5b1e2 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java @@ -71,11 +71,6 @@ public class UserApiTest extends ControllerContainerCloudTest { .data("{\"token\":\"hello\"}"), new File("tenant-without-applications.json")); - // PUT a tenant is ignored. - tester.assertResponse(request("/application/v4/user/", PUT) - .roles(operator), - "", 200); - // GET at user/v1 root fails as no access control is defined there. tester.assertResponse(request("/user/v1/"), accessDenied, 403); diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 0a47fcf7032..1dfbbb37bb6 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -67,6 +67,12 @@ public class Flags { "Takes effect on next host admin tick.", HOSTNAME); + public static final UnboundBooleanFlag USE_NEW_VESPA_RPMS = defineFeatureFlag( + "use-new-vespa-rpms", false, + "Whether to use the new vespa-rpms YUM repo when upgrading/downgrading.", + "Takes effect when upgrading or downgrading host admin to a different version.", + HOSTNAME, NODE_TYPE); + public static final UnboundListFlag<String> DISABLED_HOST_ADMIN_TASKS = defineListFlag( "disabled-host-admin-tasks", List.of(), String.class, "List of host-admin task names (as they appear in the log, e.g. root>main>UpgradeTask) that should be skipped", 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 01f602d1a57..9c8b0ec2b86 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 @@ -52,7 +52,8 @@ public class NodeAgentImpl implements NodeAgent { private static final long BYTES_IN_GB = 1_000_000_000L; // Container is started with uncapped CPU and is kept that way until the first successful health check + this duration - private static final Duration DEFAULT_WARM_UP_DURATION = Duration.ofMinutes(1); + // Subtract 1 second to avoid warmup coming in lockstep with tick time and always end up using an extra tick when there are just a few ms left + private static final Duration DEFAULT_WARM_UP_DURATION = Duration.ofSeconds(90).minus(Duration.ofSeconds(1)); private static final Logger logger = Logger.getLogger(NodeAgentImpl.class.getName()); @@ -103,7 +104,7 @@ public class NodeAgentImpl implements NodeAgent { FlagSource flagSource, Optional<CredentialsMaintainer> credentialsMaintainer, Optional<AclMaintainer> aclMaintainer, Optional<HealthChecker> healthChecker, Clock clock) { this(contextSupplier, nodeRepository, orchestrator, dockerOperations, storageMaintainer, flagSource, credentialsMaintainer, - aclMaintainer, healthChecker, clock, DEFAULT_WARM_UP_DURATION); + aclMaintainer, healthChecker, clock, DEFAULT_WARM_UP_DURATION); } public NodeAgentImpl(NodeAgentContextSupplier contextSupplier, NodeRepository nodeRepository, @@ -211,7 +212,7 @@ public class NodeAgentImpl implements NodeAgent { private Container startContainer(NodeAgentContext context) { ContainerData containerData = createContainerData(context); - ContainerResources wantedResources = context.nodeType() != NodeType.tenant || warmUpDuration.isNegative() ? + ContainerResources wantedResources = context.nodeType() != NodeType.tenant || warmUpDuration(context.zone()).isNegative() ? getContainerResources(context) : getContainerResources(context).withUnlimitedCpus(); dockerOperations.createContainer(context, containerData, wantedResources); dockerOperations.startContainer(context); @@ -357,7 +358,7 @@ public class NodeAgentImpl implements NodeAgent { ContainerResources wantedContainerResources = getContainerResources(context); if (healthChecker.isPresent() && firstSuccessfulHealthCheckInstant - .map(clock.instant().minus(warmUpDuration)::isBefore) + .map(clock.instant().minus(warmUpDuration(context.zone()))::isBefore) .orElse(true)) return existingContainer; @@ -473,7 +474,7 @@ public class NodeAgentImpl implements NodeAgent { if (firstSuccessfulHealthCheckInstant.isEmpty()) firstSuccessfulHealthCheckInstant = Optional.of(clock.instant()); - Duration timeLeft = Duration.between(clock.instant(), firstSuccessfulHealthCheckInstant.get().plus(warmUpDuration)); + Duration timeLeft = Duration.between(clock.instant(), firstSuccessfulHealthCheckInstant.get().plus(warmUpDuration(context.zone()))); if (!container.get().resources.equalsCpu(getContainerResources(context))) throw new ConvergenceException("Refusing to resume until warm up period ends (" + (timeLeft.isNegative() ? "next tick" : "in " + timeLeft) + ")"); @@ -604,4 +605,10 @@ public class NodeAgentImpl implements NodeAgent { protected Optional<CredentialsMaintainer> credentialsMaintainer() { return credentialsMaintainer; } + + private Duration warmUpDuration(ZoneApi zone) { + return zone.getSystemName().isCd() || zone.getEnvironment().isTest() + ? Duration.ofSeconds(-1) + : warmUpDuration; + } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/DefaultEnvWriter.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/DefaultEnvWriter.java index 83ac3eeeaf4..22cabdc8ed9 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/DefaultEnvWriter.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/DefaultEnvWriter.java @@ -1,6 +1,8 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.task.util; +import com.yahoo.vespa.hosted.node.admin.component.TaskContext; + import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -10,6 +12,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.TreeSet; +import java.util.logging.Logger; import static com.yahoo.vespa.hosted.node.admin.task.util.file.IOExceptionUtil.ifExists; import static com.yahoo.yolean.Exceptions.uncheck; @@ -23,6 +26,8 @@ import static java.util.stream.Collectors.joining; */ public class DefaultEnvWriter { + private static final Logger logger = Logger.getLogger(DefaultEnvWriter.class.getName()); + private final Map<String, Operation> operations = new LinkedHashMap<>(); public DefaultEnvWriter addOverride(String name, String value) { @@ -50,12 +55,13 @@ public class DefaultEnvWriter { * * @return true if the file was modified */ - public boolean updateFile(Path defaultEnvFile) { + public boolean updateFile(TaskContext context, Path defaultEnvFile) { List<String> currentDefaultEnvLines = ifExists(() -> Files.readAllLines(defaultEnvFile)).orElse(List.of()); List<String> newDefaultEnvLines = generateContent(currentDefaultEnvLines); if (currentDefaultEnvLines.equals(newDefaultEnvLines)) { return false; } else { + context.log(logger, "Updating " + defaultEnvFile.toString()); Path tempFile = Paths.get(defaultEnvFile.toString() + ".tmp"); uncheck(() -> Files.write(tempFile, newDefaultEnvLines)); uncheck(() -> Files.move(tempFile, defaultEnvFile, ATOMIC_MOVE)); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/DefaultEnvWriterTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/DefaultEnvWriterTest.java index a81ad8ff2eb..a2457266560 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/DefaultEnvWriterTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/DefaultEnvWriterTest.java @@ -1,6 +1,7 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.task.util; +import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -9,11 +10,16 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.logging.Logger; import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; /** * @author bjorncs @@ -26,6 +32,8 @@ public class DefaultEnvWriterTest { private static final Path EXAMPLE_FILE = Paths.get("src/test/resources/default-env-example.txt"); private static final Path EXPECTED_RESULT_FILE = Paths.get("src/test/resources/default-env-rewritten.txt"); + private final TaskContext context = mock(TaskContext.class); + @Test public void default_env_is_correctly_rewritten() throws IOException { Path tempFile = temporaryFolder.newFile().toPath(); @@ -36,14 +44,16 @@ public class DefaultEnvWriterTest { writer.addFallback("VESPA_CONFIGSERVER", "new-fallback-configserver"); writer.addOverride("VESPA_TLS_CONFIG_FILE", "/override/path/to/config.file"); - boolean modified = writer.updateFile(tempFile); + boolean modified = writer.updateFile(context, tempFile); assertTrue(modified); assertEquals(Files.readString(EXPECTED_RESULT_FILE), Files.readString(tempFile)); + verify(context, times(1)).log(any(Logger.class), any(String.class)); - modified = writer.updateFile(tempFile); + modified = writer.updateFile(context, tempFile); assertFalse(modified); assertEquals(Files.readString(EXPECTED_RESULT_FILE), Files.readString(tempFile)); + verify(context, times(1)).log(any(Logger.class), any(String.class)); } @Test diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java index 3cf2442f6f7..4ba480b73b1 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java @@ -19,6 +19,7 @@ import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.hosted.provision.applications.Applications; import com.yahoo.vespa.hosted.provision.lb.LoadBalancer; import com.yahoo.vespa.hosted.provision.lb.LoadBalancerId; import com.yahoo.vespa.hosted.provision.lb.LoadBalancerInstance; @@ -98,6 +99,7 @@ public class NodeRepository extends AbstractComponent { private final FirmwareChecks firmwareChecks; private final DockerImages dockerImages; private final JobControl jobControl; + private final Applications applications; /** * Creates a node repository from a zookeeper provider. @@ -124,6 +126,7 @@ public class NodeRepository extends AbstractComponent { this.firmwareChecks = new FirmwareChecks(db, clock); this.dockerImages = new DockerImages(db, dockerImage); this.jobControl = new JobControl(db); + this.applications = new Applications(); // read and write all nodes to make sure they are stored in the latest version of the serialized format for (State state : State.values()) @@ -154,6 +157,9 @@ public class NodeRepository extends AbstractComponent { /** Returns the status of maintenance jobs managed by this. */ public JobControl jobControl() { return jobControl; } + /** Returns this node repo's view of the applications deployed to it */ + public Applications applications() { return applications; } + // ---------------- Query API ---------------------------------------------------------------- /** diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Application.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Application.java new file mode 100644 index 00000000000..e56e426b499 --- /dev/null +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Application.java @@ -0,0 +1,59 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.provision.applications; + +import com.yahoo.config.provision.ClusterResources; +import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.transaction.Mutex; + +import java.util.Map; +import java.util.HashMap; +import java.util.Optional; + +/** + * The node repository's view of an application deployment. + * + * This is immutable, and must be locked with the application lock on read-modify-write. + * + * @author bratseth + */ +public class Application { + + private final Map<ClusterSpec.Id, Cluster> clusters; + + public Application() { + this(Map.of()); + } + + private Application(Map<ClusterSpec.Id, Cluster> clusters) { + this.clusters = Map.copyOf(clusters); + } + + /** Returns the cluster with the given id or null if none */ + public Cluster cluster(ClusterSpec.Id id) { return clusters.get(id); } + + public Application with(ClusterSpec.Id id, Cluster cluster) { + Map<ClusterSpec.Id, Cluster> clusters = new HashMap<>(this.clusters); + clusters.put(id, cluster); + return new Application(clusters); + } + + /** + * Returns an application with the given cluster having the min and max resource limits of the given cluster. + * If the cluster has a target which is not inside the new limits, the target is removed. + */ + public Application withClusterLimits(ClusterSpec.Id id, ClusterResources min, ClusterResources max) { + Cluster cluster = clusters.get(id); + return with(id, new Cluster(min, max, cluster == null ? Optional.empty() : cluster.targetResources())); + } + + /** + * Returns an application with the given target for the given cluster, + * if it exists and the target is within the bounds + */ + public Application withClusterTarget(ClusterSpec.Id id, ClusterResources target) { + Cluster cluster = clusters.get(id); + if (cluster == null) return this; + return with(id, cluster.withTarget(target)); + } + +} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Applications.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Applications.java new file mode 100644 index 00000000000..879fcc5f6cb --- /dev/null +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Applications.java @@ -0,0 +1,29 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.provision.applications; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.transaction.Mutex; + +import java.util.concurrent.ConcurrentHashMap; + +/** + * An (in-memory, for now) repository of the node repo's view of applications. + * + * This is multithread safe. + * + * @author bratseth + */ +public class Applications { + + private final ConcurrentHashMap<ApplicationId, Application> applications = new ConcurrentHashMap<>(); + + /** Returns the application with the given id, or null if it does not exist and should not be created */ + public Application get(ApplicationId applicationId, boolean create) { + return applications.computeIfAbsent(applicationId, id -> create ? new Application() : null); + } + + public void set(ApplicationId id, Application application, Mutex applicationLock) { + applications.put(id, application); + } + +} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java new file mode 100644 index 00000000000..6ff7f41be8f --- /dev/null +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java @@ -0,0 +1,66 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.provision.applications; + +import com.yahoo.config.provision.ClusterResources; +import com.yahoo.config.provision.NodeResources; + +import java.util.Objects; +import java.util.Optional; + +/** + * The node repo's view of a cluster in an application deployment. + * + * This is immutable, and must be locked with the application lock on read-modify-write. + * + * @author bratseth + */ +public class Cluster { + + private final ClusterResources min, max; + private final Optional<ClusterResources> target; + + Cluster(ClusterResources minResources, ClusterResources maxResources, Optional<ClusterResources> targetResources) { + this.min = Objects.requireNonNull(minResources); + this.max = Objects.requireNonNull(maxResources); + Objects.requireNonNull(targetResources); + + if (targetResources.isPresent() && ! targetResources.get().isWithin(minResources, maxResources)) + this.target = Optional.empty(); + else + this.target = targetResources; + } + + /** Returns the configured minimal resources in this cluster */ + public ClusterResources minResources() { return min; } + + /** Returns the configured maximal resources in this cluster */ + public ClusterResources maxResources() { return max; } + + /** + * Returns the computed resources (between min and max, inclusive) this cluster should + * have allocated at the moment, or empty if the system currently have no opinion on this. + */ + public Optional<ClusterResources> targetResources() { return target; } + + public Cluster withTarget(ClusterResources target) { + return new Cluster(min, max, Optional.of(target)); + } + + public Cluster withoutTarget() { + return new Cluster(min, max, Optional.empty()); + } + + public NodeResources capAtLimits(NodeResources resources) { + resources = resources.withVcpu(between(min.nodeResources().vcpu(), max.nodeResources().vcpu(), resources.vcpu())); + resources = resources.withMemoryGb(between(min.nodeResources().memoryGb(), max.nodeResources().memoryGb(), resources.memoryGb())); + resources = resources.withDiskGb(between(min.nodeResources().diskGb(), max.nodeResources().diskGb(), resources.diskGb())); + return resources; + } + + private double between(double min, double max, double value) { + value = Math.max(min, value); + value = Math.min(max, value); + return value; + } + +} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java index 7a36103f337..1c3ea55163a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java @@ -26,26 +26,32 @@ public class AllocatableClusterResources { private final ClusterSpec.Type clusterType; + private final double fulfilment; + public AllocatableClusterResources(List<Node> nodes, HostResourcesCalculator calculator) { this.advertisedResources = nodes.get(0).flavor().resources(); this.realResources = calculator.realResourcesOf(nodes.get(0)); this.nodes = nodes.size(); this.groups = (int)nodes.stream().map(node -> node.allocation().get().membership().cluster().group()).distinct().count(); this.clusterType = nodes.get(0).allocation().get().membership().cluster().type(); + this.fulfilment = 1; } public AllocatableClusterResources(ClusterResources realResources, NodeResources advertisedResources, + NodeResources idealResources, ClusterSpec.Type clusterType) { this.realResources = realResources.nodeResources(); this.advertisedResources = advertisedResources; this.nodes = realResources.nodes(); this.groups = realResources.groups(); this.clusterType = clusterType; + this.fulfilment = fulfilment(realResources.nodeResources(), idealResources); } public AllocatableClusterResources(ClusterResources realResources, Flavor flavor, + NodeResources idealResources, ClusterSpec.Type clusterType, HostResourcesCalculator calculator) { this.realResources = realResources.nodeResources(); @@ -53,6 +59,7 @@ public class AllocatableClusterResources { this.nodes = realResources.nodes(); this.groups = realResources.groups(); this.clusterType = clusterType; + this.fulfilment = fulfilment(realResources.nodeResources(), idealResources); } /** @@ -67,15 +74,38 @@ public class AllocatableClusterResources { */ public NodeResources advertisedResources() { return advertisedResources; } - public double cost() { return nodes * Autoscaler.costOf(advertisedResources); } + public ClusterResources toAdvertisedClusterResources() { + return new ClusterResources(nodes, groups, advertisedResources); + } public int nodes() { return nodes; } public int groups() { return groups; } public ClusterSpec.Type clusterType() { return clusterType; } + public double cost() { return nodes * Autoscaler.costOf(advertisedResources); } + + /** + * Returns the fraction measuring how well the real resources fulfils the ideal: 1 means completely fulfiled, + * 0 means we have zero real resources. + * The real may be short of the ideal due to resource limits imposed by the system or application. + */ + public double fulfilment() { return fulfilment; } + + private static double fulfilment(NodeResources realResources, NodeResources idealResources) { + double vcpuFulfilment = Math.min(1, realResources.vcpu() / idealResources.vcpu()); + double memoryGbFulfilment = Math.min(1, realResources.memoryGb() / idealResources.memoryGb()); + double diskGbFulfilment = Math.min(1, realResources.diskGb() / idealResources.diskGb()); + return (vcpuFulfilment + memoryGbFulfilment + diskGbFulfilment) / 3; + } + + public boolean preferableTo(AllocatableClusterResources other) { + if (this.fulfilment > other.fulfilment) return true; // we always want to fulfil as much as possible + return this.cost() < other.cost(); // otherwise, prefer lower cost + } + @Override public String toString() { - return "$" + cost() + ": " + realResources(); + return "$" + cost() + " (fulfilment " + fulfilment + "): " + realResources(); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java index dc9699a7a0b..1040ffdf0e4 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java @@ -9,6 +9,7 @@ import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.host.FlavorOverrides; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; +import com.yahoo.vespa.hosted.provision.applications.Cluster; import com.yahoo.vespa.hosted.provision.provisioning.HostResourcesCalculator; import com.yahoo.vespa.hosted.provision.provisioning.NodeResourceLimits; @@ -66,7 +67,7 @@ public class Autoscaler { * @param clusterNodes the list of all the active nodes in a cluster * @return a new suggested allocation for this cluster, or empty if it should not be rescaled at this time */ - public Optional<AllocatableClusterResources> autoscale(List<Node> clusterNodes) { + public Optional<AllocatableClusterResources> autoscale(Cluster cluster, List<Node> clusterNodes) { if (clusterNodes.stream().anyMatch(node -> node.status().wantToRetire() || node.allocation().get().membership().retired() || node.allocation().get().isRemovable())) { @@ -83,7 +84,8 @@ public class Autoscaler { Optional<AllocatableClusterResources> bestAllocation = findBestAllocation(cpuLoad.get(), memoryLoad.get(), diskLoad.get(), - currentAllocation); + currentAllocation, + cluster); if (bestAllocation.isEmpty()) return Optional.empty(); if (closeToIdeal(Resource.cpu, cpuLoad.get()) && @@ -96,14 +98,15 @@ public class Autoscaler { } private Optional<AllocatableClusterResources> findBestAllocation(double cpuLoad, double memoryLoad, double diskLoad, - AllocatableClusterResources currentAllocation) { + AllocatableClusterResources currentAllocation, + Cluster cluster) { Optional<AllocatableClusterResources> bestAllocation = Optional.empty(); - for (ResourceIterator i = new ResourceIterator(cpuLoad, memoryLoad, diskLoad, currentAllocation); i.hasNext(); ) { - ClusterResources allocation = i.next(); - Optional<AllocatableClusterResources> allocatableResources = toAllocatableResources(allocation, - currentAllocation.clusterType()); + for (ResourceIterator i = new ResourceIterator(cpuLoad, memoryLoad, diskLoad, currentAllocation, cluster); i.hasNext(); ) { + Optional<AllocatableClusterResources> allocatableResources = toAllocatableResources(i.next(), + currentAllocation.clusterType(), + cluster); if (allocatableResources.isEmpty()) continue; - if (bestAllocation.isEmpty() || allocatableResources.get().cost() < bestAllocation.get().cost()) + if (bestAllocation.isEmpty() || allocatableResources.get().preferableTo(bestAllocation.get())) bestAllocation = allocatableResources; } return bestAllocation; @@ -126,14 +129,20 @@ public class Autoscaler { * or empty if none available. */ private Optional<AllocatableClusterResources> toAllocatableResources(ClusterResources resources, - ClusterSpec.Type clusterType) { - NodeResources nodeResources = nodeResourceLimits.enlargeToLegal(resources.nodeResources(), clusterType); + ClusterSpec.Type clusterType, + Cluster cluster) { + NodeResources nodeResources = resources.nodeResources(); + if ( ! cluster.minResources().equals(cluster.maxResources())) // enforce application limits unless suggest mode + nodeResources = cluster.capAtLimits(nodeResources); + nodeResources = nodeResourceLimits.enlargeToLegal(nodeResources, clusterType); // enforce system limits + if (allowsHostSharing(nodeRepository.zone().cloud())) { // return the requested resources, or empty if they cannot fit on existing hosts for (Flavor flavor : nodeRepository.getAvailableFlavors().getFlavors()) { if (flavor.resources().satisfies(nodeResources)) return Optional.of(new AllocatableClusterResources(resources.with(nodeResources), nodeResources, + resources.nodeResources(), clusterType)); } return Optional.empty(); @@ -148,6 +157,7 @@ public class Autoscaler { flavor = flavor.with(FlavorOverrides.ofDisk(nodeResources.diskGb())); var candidate = new AllocatableClusterResources(resources.with(flavor.resources()), flavor, + resources.nodeResources(), clusterType, resourcesCalculator); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Resource.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Resource.java index e84544e7e7b..3d5ce8881e0 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Resource.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Resource.java @@ -12,7 +12,7 @@ public enum Resource { /** Cpu utilization ratio */ cpu { - String metricName() { return "cpu.util"; } + public String metricName() { return "cpu.util"; } double idealAverageLoad() { return 0.2; } double valueFrom(NodeResources resources) { return resources.vcpu(); } double valueFromMetric(double metricValue) { return metricValue / 100; } // % to ratio @@ -20,7 +20,7 @@ public enum Resource { /** Memory utilization ratio */ memory { - String metricName() { return "mem_total.util"; } + public String metricName() { return "mem_total.util"; } double idealAverageLoad() { return 0.7; } double valueFrom(NodeResources resources) { return resources.memoryGb(); } double valueFromMetric(double metricValue) { return metricValue / 100; } // % to ratio @@ -28,13 +28,13 @@ public enum Resource { /** Disk utilization ratio */ disk { - String metricName() { return "disk.util"; } + public String metricName() { return "disk.util"; } double idealAverageLoad() { return 0.6; } double valueFrom(NodeResources resources) { return resources.diskGb(); } double valueFromMetric(double metricValue) { return metricValue / 100; } // % to ratio }; - abstract String metricName(); + public abstract String metricName(); /** The load we should have of this resource on average, when one node in the cluster is down */ abstract double idealAverageLoad(); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceIterator.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceIterator.java index 82c07345c7f..bc14ca1779c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceIterator.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceIterator.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.provision.autoscale; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.NodeResources; +import com.yahoo.vespa.hosted.provision.applications.Cluster; /** * Provides iteration over possible cluster resource allocations given a target total load @@ -10,16 +11,19 @@ import com.yahoo.config.provision.NodeResources; */ public class ResourceIterator { - // Configured min and max nodes TODO: These should come from the application package - private static final int minimumNodesPerCluster = 3; // Since this is with redundancy it cannot be lower than 2 - private static final int maximumNodesPerCluster = 150; + // Configured min and max nodes for suggestions for apps which have not activated autoscaling + private static final int minimumNodes = 3; // Since this is with redundancy it cannot be lower than 2 + private static final int maximumNodes = 150; // When a query is issued on a node the cost is the sum of a fixed cost component and a cost component // proportional to document count. We must account for this when comparing configurations with more or fewer nodes. // TODO: Measure this, and only take it into account with queries private static final double fixedCpuCostFraction = 0.1; - // Describes the observed state + // Prescribed state + private final Cluster cluster; + + // Observed state private final AllocatableClusterResources allocation; private final double cpuLoad; private final double memoryLoad; @@ -33,7 +37,9 @@ public class ResourceIterator { // Iterator state private int currentNodes; - public ResourceIterator(double cpuLoad, double memoryLoad, double diskLoad, AllocatableClusterResources currentAllocation) { + public ResourceIterator(double cpuLoad, double memoryLoad, double diskLoad, + AllocatableClusterResources currentAllocation, + Cluster cluster) { this.cpuLoad = cpuLoad; this.memoryLoad = memoryLoad; this.diskLoad = diskLoad; @@ -42,6 +48,8 @@ public class ResourceIterator { groupSize = (int)Math.ceil((double)currentAllocation.nodes() / currentAllocation.groups()); allocation = currentAllocation; + this.cluster = cluster; + // What number of nodes is it effective to add or remove at the time from this cluster? // This is the group size, since we (for now) assume the group size is decided by someone wiser than us // and we decide the number of groups. @@ -49,30 +57,52 @@ public class ResourceIterator { singleGroupMode = currentAllocation.groups() == 1; nodeIncrement = singleGroupMode ? 1 : groupSize; + // Step down to the right starting point currentNodes = currentAllocation.nodes(); - while (currentNodes - nodeIncrement >= minimumNodesPerCluster - && (singleGroupMode || currentNodes - nodeIncrement > groupSize)) // group level redundancy + while (currentNodes - nodeIncrement >= minNodes() + && ( singleGroupMode || currentNodes - nodeIncrement > groupSize)) // group level redundancy currentNodes -= nodeIncrement; } + /** If autoscaling is not enabled (meaning max and min resources are the same), we want to suggest */ + private boolean suggestMode() { + return cluster.minResources().equals(cluster.maxResources()); + } + public ClusterResources next() { - int nodesWithRedundancy = currentNodes - (singleGroupMode ? 1 : groupSize); - ClusterResources next = new ClusterResources(currentNodes, - singleGroupMode ? 1 : currentNodes / groupSize, - resourcesFor(nodesWithRedundancy)); + ClusterResources next = resourcesWith(currentNodes); currentNodes += nodeIncrement; return next; } public boolean hasNext() { - return currentNodes <= maximumNodesPerCluster; + return currentNodes <= maxNodes(); + } + + private int minNodes() { + if (suggestMode()) return minimumNodes; + if (singleGroupMode) return cluster.minResources().nodes(); + return Math.max(cluster.minResources().nodes(), cluster.minResources().groups() * groupSize ); + } + + private int maxNodes() { + if (suggestMode()) return maximumNodes; + if (singleGroupMode) return cluster.maxResources().nodes(); + return Math.min(cluster.maxResources().nodes(), cluster.maxResources().groups() * groupSize ); + } + + private ClusterResources resourcesWith(int nodes) { + int nodesWithRedundancy = nodes - (singleGroupMode ? 1 : groupSize); + return new ClusterResources(nodes, + singleGroupMode ? 1 : nodes / groupSize, + nodeResourcesWith(nodesWithRedundancy)); } /** * For the observed load this instance is initialized with, returns the resources needed per node to be at * ideal load given a target node count */ - private NodeResources resourcesFor(int nodeCount) { + private NodeResources nodeResourcesWith(int nodeCount) { // Cpu: Scales with cluster size (TODO: Only reads, writes scales with group size) // Memory and disk: Scales with group size @@ -103,6 +133,7 @@ public class ResourceIterator { disk = nodeUsage(Resource.disk, diskLoad) / Resource.disk.idealAverageLoad(); } } + return allocation.realResources().withVcpu(cpu).withMemoryGb(memory).withDiskGb(disk); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java index 3f26725da15..abfe65408b6 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java @@ -8,6 +8,8 @@ import com.yahoo.config.provision.Deployer; import com.yahoo.config.provision.NodeResources; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; +import com.yahoo.vespa.hosted.provision.applications.Application; +import com.yahoo.vespa.hosted.provision.applications.Cluster; import com.yahoo.vespa.hosted.provision.autoscale.AllocatableClusterResources; import com.yahoo.vespa.hosted.provision.autoscale.Autoscaler; import com.yahoo.vespa.hosted.provision.autoscale.NodeMetricsDb; @@ -52,29 +54,60 @@ public class AutoscalingMaintainer extends Maintainer { private void autoscale(ApplicationId application, List<Node> applicationNodes) { try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, nodeRepository())) { if ( ! deployment.isValid()) return; // Another config server will consider this application - nodesByCluster(applicationNodes).forEach((clusterId, clusterNodes) -> autoscale(application, clusterId, clusterNodes)); + nodesByCluster(applicationNodes).forEach((clusterId, clusterNodes) -> autoscale(application, clusterId, clusterNodes, deployment)); } } - private void autoscale(ApplicationId application, ClusterSpec.Id clusterId, List<Node> clusterNodes) { - Optional<AllocatableClusterResources> target = autoscaler.autoscale(clusterNodes); - if (target.isEmpty()) return; + private void autoscale(ApplicationId applicationId, + ClusterSpec.Id clusterId, + List<Node> clusterNodes, + MaintenanceDeployment deployment) { + Application application = nodeRepository().applications().get(applicationId, true); + Cluster cluster = application.cluster(clusterId); + if (cluster == null) return; // no information on limits for this cluster + Optional<AllocatableClusterResources> target = autoscaler.autoscale(cluster, clusterNodes); + if (target.isEmpty()) return; // current resources are fine + if (cluster.minResources().equals(cluster.maxResources())) { // autoscaling is deactivated + logAutoscaling("Scaling suggestion for ", target.get(), applicationId, clusterId, clusterNodes); + } + else { + logAutoscaling("Autoscaling ", target.get(), applicationId, clusterId, clusterNodes); + autoscaleTo(target.get(), applicationId, clusterId, application, deployment); + } + } + + private void autoscaleTo(AllocatableClusterResources target, + ApplicationId applicationId, + ClusterSpec.Id clusterId, + Application application, + MaintenanceDeployment deployment) { + nodeRepository().applications().set(applicationId, + application.withClusterTarget(clusterId, target.toAdvertisedClusterResources()), + deployment.applicationLock().get()); + deployment.activate(); + } + + private void logAutoscaling(String prefix, + AllocatableClusterResources target, + ApplicationId application, + ClusterSpec.Id clusterId, + List<Node> clusterNodes) { Instant lastLogTime = lastLogged.get(new Pair<>(application, clusterId)); if (lastLogTime != null && lastLogTime.isAfter(nodeRepository().clock().instant().minus(Duration.ofHours(1)))) return; - int currentGroups = (int) clusterNodes.stream().map(node -> node.allocation().get().membership().cluster().group()).distinct().count(); + int currentGroups = (int)clusterNodes.stream().map(node -> node.allocation().get().membership().cluster().group()).distinct().count(); ClusterSpec.Type clusterType = clusterNodes.get(0).allocation().get().membership().cluster().type(); - log.info("Autoscale: " + application + " " + clusterType + " " + clusterId + + log.info(prefix + application + " " + clusterType + " " + clusterId + ":" + "\nfrom " + toString(clusterNodes.size(), currentGroups, clusterNodes.get(0).flavor().resources()) + - "\nto " + toString(target.get().nodes(), target.get().groups(), target.get().advertisedResources())); + "\nto " + toString(target.nodes(), target.groups(), target.advertisedResources())); lastLogged.put(new Pair<>(application, clusterId), nodeRepository().clock().instant()); } private String toString(int nodes, int groups, NodeResources resources) { return String.format(nodes + (groups > 1 ? " (in " + groups + " groups)" : "") + - " * [vcpu: %1$.1f, memory: %2$.1f Gb, disk %3$.1f Gb]" + - " (total: [vcpu: %4$.1f, memory: %5$.1f Gb, disk: %6$.1f Gb])," + + " * [vcpu: %0$.1f, memory: %1$.1f Gb, disk %2$.1f Gb]" + + " (total: [vcpu: %3$.1f, memory: %4$.1f Gb, disk: %5$.1f Gb])", resources.vcpu(), resources.memoryGb(), resources.diskGb(), nodes * resources.vcpu(), nodes * resources.memoryGb(), nodes * resources.diskGb()); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java index 856de2609be..d9e06f87db7 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java @@ -36,10 +36,8 @@ class MaintenanceDeployment implements Closeable { public MaintenanceDeployment(ApplicationId application, Deployer deployer, NodeRepository nodeRepository) { this.application = application; Optional<Mutex> lock = tryLock(application, nodeRepository); - try { deployment = tryDeployment(lock, application, deployer, nodeRepository); - this.lock = lock; lock = Optional.empty(); } finally { @@ -52,6 +50,16 @@ class MaintenanceDeployment implements Closeable { return deployment.isPresent(); } + /** + * Returns the application lock held by this, or empty if it is not held. + * + * @throws IllegalStateException id this is called when closed + */ + public Optional<Mutex> applicationLock() { + if (closed) throw new IllegalStateException(this + " is closed"); + return lock; + } + public boolean prepare() { return doStep(() -> deployment.get().prepare()); } @@ -61,7 +69,7 @@ class MaintenanceDeployment implements Closeable { } private boolean doStep(Runnable action) { - if (closed) throw new IllegalStateException("Deployment of '" + application + "' is closed"); + if (closed) throw new IllegalStateException(this + "' is closed"); if ( ! isValid()) return false; try { action.run(); @@ -101,4 +109,9 @@ class MaintenanceDeployment implements Closeable { closed = true; } + @Override + public String toString() { + return "deployment of " + application; + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java index a23c1a932d4..f010e6905e1 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java @@ -31,42 +31,37 @@ public class CapacityPolicies { this.isUsingAdvertisedResources = zone.cloud().value().equals("aws"); } - public int decideSize(Capacity capacity, ClusterSpec cluster, ApplicationId application) { - int requestedNodes = capacity.minResources().nodes(); - + public int decideSize(int requested, Capacity capacity, ClusterSpec cluster, ApplicationId application) { if (application.instance().isTester()) return 1; - ensureRedundancy(requestedNodes, cluster, capacity.canFail()); - - if (capacity.isRequired()) return requestedNodes; - + ensureRedundancy(requested, cluster, capacity.canFail()); + if (capacity.isRequired()) return requested; switch(zone.environment()) { case dev : case test : return 1; - case perf : return Math.min(capacity.minResources().nodes(), 3); - case staging: return requestedNodes <= 1 ? requestedNodes : Math.max(2, requestedNodes / 10); - case prod : return requestedNodes; + case perf : return Math.min(requested, 3); + case staging: return requested <= 1 ? requested : Math.max(2, requested / 10); + case prod : return requested; default : throw new IllegalArgumentException("Unsupported environment " + zone.environment()); } } - public NodeResources decideNodeResources(Capacity capacity, ClusterSpec cluster) { - NodeResources resources = capacity.minResources().nodeResources(); - if (resources == NodeResources.unspecified) - resources = defaultNodeResources(cluster.type()); - ensureSufficientResources(resources, cluster); + public NodeResources decideNodeResources(NodeResources requested, Capacity capacity, ClusterSpec cluster) { + if (requested == NodeResources.unspecified) + requested = defaultNodeResources(cluster.type()); + ensureSufficientResources(requested, cluster); - if (capacity.isRequired()) return resources; + if (capacity.isRequired()) return requested; // Allow slow storage in zones which are not performance sensitive if (zone.system().isCd() || zone.environment() == Environment.dev || zone.environment() == Environment.test) - resources = resources.with(NodeResources.DiskSpeed.any).with(NodeResources.StorageType.any); + requested = requested.with(NodeResources.DiskSpeed.any).with(NodeResources.StorageType.any); // Dev does not cap the cpu of containers since usage is spotty: Allocate just a small amount exclusively // Do not cap in AWS as hosts are allocated on demand and 1-to-1, so the node can use the entire host if (zone.environment() == Environment.dev && !zone.region().value().contains("aws-")) - resources = resources.withVcpu(0.1); + requested = requested.withVcpu(0.1); - return resources; + return requested; } private void ensureSufficientResources(NodeResources resources, ClusterSpec cluster) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java index 2c2c927034b..d03aa0cac91 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.provision.provisioning; import com.google.inject.Inject; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Capacity; +import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Environment; @@ -15,10 +16,14 @@ import com.yahoo.config.provision.ProvisionLogger; import com.yahoo.config.provision.Provisioner; import com.yahoo.config.provision.Zone; import com.yahoo.log.LogLevel; +import com.yahoo.transaction.Mutex; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; +import com.yahoo.vespa.hosted.provision.applications.Application; +import com.yahoo.vespa.hosted.provision.applications.Cluster; import com.yahoo.vespa.hosted.provision.node.Allocation; import com.yahoo.vespa.hosted.provision.node.filter.ApplicationFilter; import com.yahoo.vespa.hosted.provision.node.filter.NodeHostFilter; @@ -87,38 +92,36 @@ public class NodeRepositoryProvisioner implements Provisioner { * The nodes are ordered by increasing index number. */ @Override - public List<HostSpec> prepare(ApplicationId application, ClusterSpec cluster, Capacity requestedCapacity, + public List<HostSpec> prepare(ApplicationId application, ClusterSpec cluster, Capacity requested, ProvisionLogger logger) { - if (cluster.group().isPresent()) throw new IllegalArgumentException("Node requests cannot specify a group"); - log.log(zone.system().isCd() ? Level.INFO : LogLevel.DEBUG, - () -> "Received deploy prepare request for " + requestedCapacity + + () -> "Received deploy prepare request for " + requested + " for application " + application + ", cluster " + cluster); - int effectiveGroups; - NodeSpec requestedNodes; - NodeResources resources = requestedCapacity.minResources().nodeResources(); - if ( requestedCapacity.type() == NodeType.tenant) { - int nodeCount = capacityPolicies.decideSize(requestedCapacity, cluster, application); - if (zone.environment().isManuallyDeployed() && nodeCount < requestedCapacity.minResources().nodes()) - logger.log(Level.INFO, "Requested " + requestedCapacity.minResources().nodes() + " nodes for " + cluster + - ", downscaling to " + nodeCount + " nodes in " + zone.environment()); - resources = capacityPolicies.decideNodeResources(requestedCapacity, cluster); - boolean exclusive = capacityPolicies.decideExclusivity(cluster.isExclusive()); - effectiveGroups = Math.min(requestedCapacity.minResources().groups(), nodeCount); // cannot have more groups than nodes - requestedNodes = NodeSpec.from(nodeCount, resources, exclusive, requestedCapacity.canFail()); + if (cluster.group().isPresent()) throw new IllegalArgumentException("Node requests cannot specify a group"); - if ( ! hasQuota(application, nodeCount)) - throw new IllegalArgumentException(requestedCapacity + " requested for " + cluster + - (requestedCapacity.minResources().nodes() != nodeCount ? " resolved to " + nodeCount + " nodes" : "") + - " exceeds your quota. Resolve this at https://cloud.vespa.ai/quota"); + if ( ! hasQuota(application, requested.maxResources().nodes())) + throw new IllegalArgumentException(requested + " requested for " + cluster + + ". Max value exceeds your quota. Resolve this at https://cloud.vespa.ai/quota"); + + int groups; + NodeResources resources; + NodeSpec nodeSpec; + if ( requested.type() == NodeType.tenant) { + ClusterResources target = decideTargetResources(application, cluster.id(), requested); + int nodeCount = capacityPolicies.decideSize(target.nodes(), requested, cluster, application); + resources = capacityPolicies.decideNodeResources(target.nodeResources(), requested, cluster); + boolean exclusive = capacityPolicies.decideExclusivity(cluster.isExclusive()); + groups = Math.min(target.groups(), nodeCount); // cannot have more groups than nodes + nodeSpec = NodeSpec.from(nodeCount, resources, exclusive, requested.canFail()); + logIfDownscaled(target.nodes(), nodeCount, cluster, logger); } else { - requestedNodes = NodeSpec.from(requestedCapacity.type()); - effectiveGroups = 1; // type request with multiple groups is not supported + groups = 1; // type request with multiple groups is not supported + resources = requested.minResources().nodeResources(); + nodeSpec = NodeSpec.from(requested.type()); } - - return asSortedHosts(preparer.prepare(application, cluster, requestedNodes, effectiveGroups), resources); + return asSortedHosts(preparer.prepare(application, cluster, nodeSpec, groups), resources); } @Override @@ -138,6 +141,40 @@ public class NodeRepositoryProvisioner implements Provisioner { loadBalancerProvisioner.ifPresent(lbProvisioner -> lbProvisioner.deactivate(application, transaction)); } + /** + * Returns the target cluster resources, a value between the min and max in the requested capacity, + * and updates the application store with the received min and max, + */ + private ClusterResources decideTargetResources(ApplicationId applicationId, ClusterSpec.Id clusterId, Capacity requested) { + try (Mutex lock = nodeRepository.lock(applicationId)) { + Application application = nodeRepository.applications().get(applicationId, true); + application = application.withClusterLimits(clusterId, requested.minResources(), requested.maxResources()); + nodeRepository.applications().set(applicationId, application, lock); + return application.cluster(clusterId).targetResources() + .orElseGet(() -> currentResources(applicationId, clusterId, requested) + .orElse(requested.minResources())); + } + } + + /** Returns the current resources of this cluster, if it's already deployed and inside the requested limits */ + private Optional<ClusterResources> currentResources(ApplicationId applicationId, + ClusterSpec.Id clusterId, + Capacity requested) { + List<Node> nodes = NodeList.copyOf(nodeRepository.getNodes(applicationId, Node.State.active)) + .cluster(clusterId).not().retired().asList(); + if (nodes.size() < 1) return Optional.empty(); + long groups = nodes.stream().map(node -> node.allocation().get().membership().cluster().group()).distinct().count(); + var resources = new ClusterResources(nodes.size(), (int)groups, nodes.get(0).flavor().resources()); + if ( ! resources.isWithin(requested.minResources(), requested.maxResources())) return Optional.empty(); + return Optional.of(resources); + } + + private void logIfDownscaled(int targetNodes, int actualNodes, ClusterSpec cluster, ProvisionLogger logger) { + if (zone.environment().isManuallyDeployed() && actualNodes < targetNodes) + logger.log(Level.INFO, "Requested " + targetNodes + " nodes for " + cluster + + ", downscaling to " + actualNodes + " nodes in " + zone.environment()); + } + private boolean hasQuota(ApplicationId application, int requestedNodes) { if ( ! this.zone.system().isPublic()) return true; // no quota management diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingIntegrationTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingIntegrationTest.java index d154af4f025..f02acdc1fca 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingIntegrationTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingIntegrationTest.java @@ -2,12 +2,15 @@ package com.yahoo.vespa.hosted.provision.autoscale; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.NodeResources; import com.yahoo.test.ManualClock; import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.applications.Application; +import com.yahoo.vespa.hosted.provision.applications.Cluster; import com.yahoo.vespa.hosted.provision.provisioning.HostResourcesCalculator; import com.yahoo.vespa.hosted.provision.testutils.OrchestratorMock; import org.junit.Test; @@ -47,7 +50,13 @@ public class AutoscalingIntegrationTest { tester.nodeMetricsDb().gc(tester.clock()); } - var scaledResources = autoscaler.autoscale(tester.nodeRepository().getNodes(application1)); + ClusterResources min = new ClusterResources(2, 1, nodes); + ClusterResources max = new ClusterResources(2, 1, nodes); + + Application application = tester.nodeRepository().applications().get(application1, true).withClusterLimits(cluster1.id(), min, max); + tester.nodeRepository().applications().set(application1, application, tester.nodeRepository().lock(application1)); + var scaledResources = autoscaler.autoscale(application.cluster(cluster1.id()), + tester.nodeRepository().getNodes(application1)); assertTrue(scaledResources.isPresent()); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java index 39259bf44f8..3bb0676da4f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.provision.autoscale; import com.google.common.collect.Sets; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.CloudName; +import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.Flavor; @@ -31,6 +32,8 @@ public class AutoscalingTest { @Test public void testAutoscalingSingleContentGroup() { NodeResources resources = new NodeResources(3, 100, 100, 1); + ClusterResources min = new ClusterResources( 2, 1, new NodeResources(1, 1, 1, 1)); + ClusterResources max = new ClusterResources(20, 1, new NodeResources(100, 1000, 1000, 1)); AutoscalingTester tester = new AutoscalingTester(resources); ApplicationId application1 = tester.applicationId("application1"); @@ -39,37 +42,39 @@ public class AutoscalingTest { // deploy tester.deploy(application1, cluster1, 5, 1, resources); - assertTrue("No measurements -> No change", tester.autoscale(application1).isEmpty()); + assertTrue("No measurements -> No change", tester.autoscale(application1, cluster1.id(), min, max).isEmpty()); tester.addMeasurements(Resource.cpu, 0.25f, 1f, 60, application1); - assertTrue("Too few measurements -> No change", tester.autoscale(application1).isEmpty()); + assertTrue("Too few measurements -> No change", tester.autoscale(application1, cluster1.id(), min, max).isEmpty()); tester.addMeasurements(Resource.cpu, 0.25f, 1f, 60, application1); AllocatableClusterResources scaledResources = tester.assertResources("Scaling up since resource usage is too high", 15, 1, 1.3, 28.6, 28.6, - tester.autoscale(application1)); + tester.autoscale(application1, cluster1.id(), min, max)); tester.deploy(application1, cluster1, scaledResources); - assertTrue("Cluster in flux -> No further change", tester.autoscale(application1).isEmpty()); + assertTrue("Cluster in flux -> No further change", tester.autoscale(application1, cluster1.id(), min, max).isEmpty()); tester.deactivateRetired(application1, cluster1, scaledResources); tester.addMeasurements(Resource.cpu, 0.8f, 1f, 3, application1); assertTrue("Load change is large, but insufficient measurements for new config -> No change", - tester.autoscale(application1).isEmpty()); + tester.autoscale(application1, cluster1.id(), min, max).isEmpty()); tester.addMeasurements(Resource.cpu, 0.19f, 1f, 100, application1); - assertEquals("Load change is small -> No change", Optional.empty(), tester.autoscale(application1)); + assertEquals("Load change is small -> No change", Optional.empty(), tester.autoscale(application1, cluster1.id(), min, max)); tester.addMeasurements(Resource.cpu, 0.1f, 1f, 120, application1); - tester.assertResources("Scaling down since resource usage has gone down significantly", - 26, 1, 0.6, 16.0, 16.0, - tester.autoscale(application1)); + tester.assertResources("Scaling down to minimum since usage has gone down significantly", + 14, 1, 1.0, 30.8, 30.8, + tester.autoscale(application1, cluster1.id(), min, max)); } /** We prefer fewer nodes for container clusters as (we assume) they all use the same disk and memory */ @Test public void testAutoscalingSingleContainerGroup() { NodeResources resources = new NodeResources(3, 100, 100, 1); + ClusterResources min = new ClusterResources( 2, 1, new NodeResources(1, 1, 1, 1)); + ClusterResources max = new ClusterResources(20, 1, new NodeResources(100, 1000, 1000, 1)); AutoscalingTester tester = new AutoscalingTester(resources); ApplicationId application1 = tester.applicationId("application1"); @@ -81,7 +86,7 @@ public class AutoscalingTest { tester.addMeasurements(Resource.cpu, 0.25f, 1f, 120, application1); AllocatableClusterResources scaledResources = tester.assertResources("Scaling up since cpu usage is too high", 7, 1, 2.6, 80.0, 80.0, - tester.autoscale(application1)); + tester.autoscale(application1, cluster1.id(), min, max)); tester.deploy(application1, cluster1, scaledResources); tester.deactivateRetired(application1, cluster1, scaledResources); @@ -89,12 +94,91 @@ public class AutoscalingTest { tester.addMeasurements(Resource.cpu, 0.1f, 1f, 120, application1); tester.assertResources("Scaling down since cpu usage has gone down", 4, 1, 2.4, 68.6, 68.6, - tester.autoscale(application1)); + tester.autoscale(application1, cluster1.id(), min, max)); + } + + @Test + public void testAutoscalingRespectsUpperLimit() { + NodeResources resources = new NodeResources(3, 100, 100, 1); + ClusterResources min = new ClusterResources( 2, 1, new NodeResources(1, 1, 1, 1)); + ClusterResources max = new ClusterResources( 6, 1, new NodeResources(2.4, 78, 79, 1)); + AutoscalingTester tester = new AutoscalingTester(resources); + + ApplicationId application1 = tester.applicationId("application1"); + ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1"); + + // deploy + tester.deploy(application1, cluster1, 5, 1, resources); + tester.addMeasurements(Resource.cpu, 0.25f, 120, application1); + tester.addMeasurements(Resource.memory, 0.95f, 120, application1); + tester.addMeasurements(Resource.disk, 0.95f, 120, application1); + tester.assertResources("Scaling up to limit since resource usage is too high", + 6, 1, 2.4, 78.0, 79.0, + tester.autoscale(application1, cluster1.id(), min, max)); + } + + @Test + public void testAutoscalingRespectsLowerLimit() { + NodeResources resources = new NodeResources(3, 100, 100, 1); + ClusterResources min = new ClusterResources( 4, 1, new NodeResources(1.8, 7.4, 8.5, 1)); + ClusterResources max = new ClusterResources( 6, 1, new NodeResources(2.4, 78, 79, 1)); + AutoscalingTester tester = new AutoscalingTester(resources); + + ApplicationId application1 = tester.applicationId("application1"); + ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1"); + + // deploy + tester.deploy(application1, cluster1, 5, 1, resources); + tester.addMeasurements(Resource.cpu, 0.05f, 120, application1); + tester.addMeasurements(Resource.memory, 0.05f, 120, application1); + tester.addMeasurements(Resource.disk, 0.05f, 120, application1); + tester.assertResources("Scaling down to limit since resource usage is low", + 4, 1, 1.8, 7.4, 8.5, + tester.autoscale(application1, cluster1.id(), min, max)); + } + + @Test + public void testAutoscalingRespectsGroupLimit() { + NodeResources hostResources = new NodeResources(30.0, 100, 100, 1); + ClusterResources min = new ClusterResources( 2, 2, new NodeResources(1, 1, 1, 1)); + ClusterResources max = new ClusterResources(18, 6, new NodeResources(100, 1000, 1000, 1)); + AutoscalingTester tester = new AutoscalingTester(hostResources); + + ApplicationId application1 = tester.applicationId("application1"); + ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1"); + + // deploy + tester.deploy(application1, cluster1, 5, 5, new NodeResources(3.0, 10, 10, 1)); + tester.addMeasurements(Resource.cpu, 0.3f, 1f, 240, application1); + tester.assertResources("Scaling up since resource usage is too high", + 6, 6, 3.6, 8.0, 8.0, + tester.autoscale(application1, cluster1.id(), min, max)); + } + + /** This condition ensures we get recommendation suggestions when deactivated */ + @Test + public void testAutoscalingLimitsAreIgnoredIfMinEqualsMax() { + NodeResources resources = new NodeResources(3, 100, 100, 1); + ClusterResources min = new ClusterResources( 2, 1, new NodeResources(1, 1, 1, 1)); + ClusterResources max = min; + AutoscalingTester tester = new AutoscalingTester(resources); + + ApplicationId application1 = tester.applicationId("application1"); + ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1"); + + // deploy + tester.deploy(application1, cluster1, 5, 1, resources); + tester.addMeasurements(Resource.cpu, 0.25f, 1f, 120, application1); + tester.assertResources("Scaling up since resource usage is too high", + 7, 1, 2.6, 80.0, 80.0, + tester.autoscale(application1, cluster1.id(), min, max)); } @Test public void testAutoscalingGroupSize1() { NodeResources resources = new NodeResources(3, 100, 100, 1); + ClusterResources min = new ClusterResources( 2, 2, new NodeResources(1, 1, 1, 1)); + ClusterResources max = new ClusterResources(20, 20, new NodeResources(100, 1000, 1000, 1)); AutoscalingTester tester = new AutoscalingTester(resources); ApplicationId application1 = tester.applicationId("application1"); @@ -105,12 +189,14 @@ public class AutoscalingTest { tester.addMeasurements(Resource.cpu, 0.25f, 1f, 120, application1); tester.assertResources("Scaling up since resource usage is too high", 7, 7, 2.5, 80.0, 80.0, - tester.autoscale(application1)); + tester.autoscale(application1, cluster1.id(), min, max)); } @Test public void testAutoscalingGroupSize3() { NodeResources resources = new NodeResources(3, 100, 100, 1); + ClusterResources min = new ClusterResources( 3, 1, new NodeResources(1, 1, 1, 1)); + ClusterResources max = new ClusterResources(21, 7, new NodeResources(100, 1000, 1000, 1)); AutoscalingTester tester = new AutoscalingTester(resources); ApplicationId application1 = tester.applicationId("application1"); @@ -121,12 +207,14 @@ public class AutoscalingTest { tester.addMeasurements(Resource.cpu, 0.22f, 1f, 120, application1); tester.assertResources("Scaling up since resource usage is too high", 9, 3, 2.7, 83.3, 83.3, - tester.autoscale(application1)); + tester.autoscale(application1, cluster1.id(), min, max)); } @Test public void testAutoscalingAvoidsIllegalConfigurations() { NodeResources resources = new NodeResources(3, 100, 100, 1); + ClusterResources min = new ClusterResources( 2, 1, new NodeResources(1, 1, 1, 1)); + ClusterResources max = new ClusterResources(20, 1, new NodeResources(100, 1000, 1000, 1)); AutoscalingTester tester = new AutoscalingTester(resources); ApplicationId application1 = tester.applicationId("application1"); @@ -137,11 +225,13 @@ public class AutoscalingTest { tester.addMeasurements(Resource.memory, 0.02f, 1f, 120, application1); tester.assertResources("Scaling down", 6, 1, 3.0, 4.0, 100.0, - tester.autoscale(application1)); + tester.autoscale(application1, cluster1.id(), min, max)); } @Test public void testAutoscalingAws() { + ClusterResources min = new ClusterResources( 2, 1, new NodeResources(1, 1, 1, 1)); + ClusterResources max = new ClusterResources(20, 1, new NodeResources(100, 1000, 1000, 1)); List<Flavor> flavors = new ArrayList<>(); flavors.add(new Flavor("aws-xlarge", new NodeResources(3, 200, 100, 1, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote))); flavors.add(new Flavor("aws-large", new NodeResources(3, 150, 100, 1, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote))); @@ -160,7 +250,7 @@ public class AutoscalingTest { tester.addMeasurements(Resource.memory, 0.9f, 0.6f, 120, application1); AllocatableClusterResources scaledResources = tester.assertResources("Scaling up since resource usage is too high.", 8, 1, 3, 83, 34.3, - tester.autoscale(application1)); + tester.autoscale(application1, cluster1.id(), min, max)); tester.deploy(application1, cluster1, scaledResources); tester.deactivateRetired(application1, cluster1, scaledResources); @@ -168,7 +258,7 @@ public class AutoscalingTest { tester.addMeasurements(Resource.memory, 0.3f, 0.6f, 1000, application1); tester.assertResources("Scaling down since resource usage has gone down", 5, 1, 3, 83, 36, - tester.autoscale(application1)); + tester.autoscale(application1, cluster1.id(), min, max)); } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java index efb97841623..ebc4d158ded 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java @@ -20,6 +20,7 @@ import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; +import com.yahoo.vespa.hosted.provision.applications.Application; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.IP; import com.yahoo.vespa.hosted.provision.provisioning.FatalProvisioningException; @@ -140,8 +141,25 @@ class AutoscalingTester { } } - public Optional<AllocatableClusterResources> autoscale(ApplicationId application) { - return autoscaler.autoscale(nodeRepository().getNodes(application, Node.State.active)); + public void addMeasurements(Resource resource, float value, int count, ApplicationId applicationId) { + List<Node> nodes = nodeRepository().getNodes(applicationId, Node.State.active); + for (int i = 0; i < count; i++) { + clock().advance(Duration.ofMinutes(1)); + for (Node node : nodes) { + db.add(List.of(new NodeMetrics.MetricValue(node.hostname(), + resource.metricName(), + clock().instant().toEpochMilli(), + value * 100))); // the metrics are in % + } + } + } + + public Optional<AllocatableClusterResources> autoscale(ApplicationId applicationId, ClusterSpec.Id clusterId, + ClusterResources min, ClusterResources max) { + Application application = nodeRepository().applications().get(applicationId, true).withClusterLimits(clusterId, min, max); + nodeRepository().applications().set(applicationId, application, nodeRepository().lock(applicationId)); + return autoscaler.autoscale(application.cluster(clusterId), + nodeRepository().getNodes(applicationId, Node.State.active)); } public AllocatableClusterResources assertResources(String message, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java new file mode 100644 index 00000000000..da169cba08f --- /dev/null +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java @@ -0,0 +1,113 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.provision.maintenance; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.Capacity; +import com.yahoo.config.provision.ClusterResources; +import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.Flavor; +import com.yahoo.config.provision.NodeResources; +import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.Zone; +import com.yahoo.config.provisioning.FlavorsConfig; +import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.NodeRepository; +import com.yahoo.vespa.hosted.provision.autoscale.NodeMetrics; +import com.yahoo.vespa.hosted.provision.autoscale.NodeMetricsDb; +import com.yahoo.vespa.hosted.provision.autoscale.Resource; +import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder; +import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester; +import com.yahoo.vespa.hosted.provision.testutils.MockDeployer; +import org.junit.Test; + +import java.time.Duration; +import java.util.List; +import java.util.Map; + +import static org.junit.Assert.assertTrue; + +/** + * Tests the autoscaling maintainer integration. + * The specific recommendations of the autoscaler are not tested here. + * + * @author bratseth + */ +public class AutoscalingMaintainerTest { + + @Test + public void testAutoscalingMaintainer() { + ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east3"))).flavorsConfig(flavorsConfig()).build(); + + ApplicationId app1 = tester.makeApplicationId("app1"); + ClusterSpec cluster1 = tester.clusterSpec(); + + ApplicationId app2 = tester.makeApplicationId("app2"); + ClusterSpec cluster2 = tester.clusterSpec(); + + NodeResources lowResources = new NodeResources(4, 4, 10, 0.1); + NodeResources highResources = new NodeResources(6.5, 9, 20, 0.1); + + Map<ApplicationId, MockDeployer.ApplicationContext> apps = Map.of( + app1, new MockDeployer.ApplicationContext(app1, cluster1, Capacity.from(new ClusterResources(2, 1, lowResources))), + app2, new MockDeployer.ApplicationContext(app2, cluster2, Capacity.from(new ClusterResources(2, 1, highResources)))); + MockDeployer deployer = new MockDeployer(tester.provisioner(), tester.clock(), apps); + + NodeMetricsDb nodeMetricsDb = new NodeMetricsDb(); + AutoscalingMaintainer maintainer = new AutoscalingMaintainer(tester.nodeRepository(), + tester.identityHostResourcesCalculator(), + nodeMetricsDb, + deployer, + Duration.ofMinutes(1)); + maintainer.maintain(); // noop + assertTrue(deployer.lastDeployTime(app1).isEmpty()); + assertTrue(deployer.lastDeployTime(app2).isEmpty()); + + tester.makeReadyNodes(20, "flt", NodeType.host, 8); + tester.deployZoneApp(); + + tester.deploy(app1, cluster1, Capacity.from(new ClusterResources(5, 1, new NodeResources(4, 4, 10, 0.1)), + new ClusterResources(5, 1, new NodeResources(4, 4, 10, 0.1)), + false, true)); + tester.deploy(app2, cluster2, Capacity.from(new ClusterResources(5, 1, new NodeResources(4, 4, 10, 0.1)), + new ClusterResources(10, 1, new NodeResources(6.5, 9, 20, 0.1)), + false, true)); + + maintainer.maintain(); // noop + assertTrue(deployer.lastDeployTime(app1).isEmpty()); + assertTrue(deployer.lastDeployTime(app2).isEmpty()); + + addMeasurements(Resource.cpu, 0.9f, 500, app1, tester.nodeRepository(), nodeMetricsDb); + addMeasurements(Resource.memory, 0.9f, 500, app1, tester.nodeRepository(), nodeMetricsDb); + addMeasurements(Resource.disk, 0.9f, 500, app1, tester.nodeRepository(), nodeMetricsDb); + addMeasurements(Resource.cpu, 0.9f, 500, app2, tester.nodeRepository(), nodeMetricsDb); + addMeasurements(Resource.memory, 0.9f, 500, app2, tester.nodeRepository(), nodeMetricsDb); + addMeasurements(Resource.disk, 0.9f, 500, app2, tester.nodeRepository(), nodeMetricsDb); + + maintainer.maintain(); + assertTrue(deployer.lastDeployTime(app1).isEmpty()); // since autoscaling is off + assertTrue(deployer.lastDeployTime(app2).isPresent()); + } + + public void addMeasurements(Resource resource, float value, int count, ApplicationId applicationId, + NodeRepository nodeRepository, NodeMetricsDb db) { + List<Node> nodes = nodeRepository.getNodes(applicationId, Node.State.active); + for (int i = 0; i < count; i++) { + for (Node node : nodes) + db.add(List.of(new NodeMetrics.MetricValue(node.hostname(), + resource.metricName(), + nodeRepository.clock().instant().toEpochMilli(), + value * 100))); // the metrics are in % + } + } + + private FlavorsConfig flavorsConfig() { + FlavorConfigBuilder b = new FlavorConfigBuilder(); + b.addFlavor("flt", 30, 30, 40, 3, Flavor.Type.BARE_METAL); + b.addFlavor("cpu", 40, 20, 40, 3, Flavor.Type.BARE_METAL); + b.addFlavor("mem", 20, 40, 40, 3, Flavor.Type.BARE_METAL); + return b.build(); + } + +} diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceTester.java index 4344016c6fe..664809dc3ab 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceTester.java @@ -3,7 +3,9 @@ package com.yahoo.vespa.hosted.provision.maintenance; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.NodeFlavors; +import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Zone; @@ -14,6 +16,7 @@ import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder; +import com.yahoo.vespa.hosted.provision.provisioning.HostResourcesCalculator; import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver; import java.time.Instant; diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RebalancerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RebalancerTest.java index 8c9d5cca54b..387f614c5eb 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RebalancerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RebalancerTest.java @@ -55,7 +55,7 @@ public class RebalancerTest { Rebalancer rebalancer = new Rebalancer(deployer, tester.nodeRepository(), - new IdentityHostResourcesCalculator(), + tester.identityHostResourcesCalculator(), Optional.empty(), metric, tester.clock(), @@ -149,18 +149,4 @@ public class RebalancerTest { return b.build(); } - private static class IdentityHostResourcesCalculator implements HostResourcesCalculator { - - @Override - public NodeResources realResourcesOf(Node node) { - return node.flavor().resources(); - } - - @Override - public NodeResources advertisedResourcesOf(Flavor flavor) { - return flavor.resources(); - } - - } - } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java index f88cb839946..76258e86de9 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java @@ -493,7 +493,7 @@ public class ProvisioningTest { fail("Expected exception"); } catch (IllegalArgumentException e) { - assertEquals("6 nodes with [vcpu: 1.0, memory: 4.0 Gb, disk 10.0 Gb, bandwidth: 4.0 Gbps] requested for content cluster 'content0' 6.42 exceeds your quota. Resolve this at https://cloud.vespa.ai/quota", + assertEquals("6 nodes with [vcpu: 1.0, memory: 4.0 Gb, disk 10.0 Gb, bandwidth: 4.0 Gbps] requested for content cluster 'content0' 6.42. Max value exceeds your quota. Resolve this at https://cloud.vespa.ai/quota", e.getMessage()); } } @@ -772,10 +772,10 @@ public class ProvisioningTest { allHosts.addAll(content1); Function<Integer, Capacity> capacity = count -> Capacity.from(new ClusterResources(count, 1, NodeResources.unspecified), required, true); - int expectedContainer0Size = tester.capacityPolicies().decideSize(capacity.apply(container0Size), containerCluster0, application); - int expectedContainer1Size = tester.capacityPolicies().decideSize(capacity.apply(container1Size), containerCluster1, application); - int expectedContent0Size = tester.capacityPolicies().decideSize(capacity.apply(content0Size), contentCluster0, application); - int expectedContent1Size = tester.capacityPolicies().decideSize(capacity.apply(content1Size), contentCluster1, application); + int expectedContainer0Size = tester.capacityPolicies().decideSize(container0Size, capacity.apply(container0Size), containerCluster0, application); + int expectedContainer1Size = tester.capacityPolicies().decideSize(container1Size, capacity.apply(container1Size), containerCluster1, application); + int expectedContent0Size = tester.capacityPolicies().decideSize(content0Size, capacity.apply(content0Size), contentCluster0, application); + int expectedContent1Size = tester.capacityPolicies().decideSize(content1Size, capacity.apply(content1Size), contentCluster1, application); assertEquals("Hosts in each group cluster is disjunct and the total number of unretired nodes is correct", expectedContainer0Size + expectedContainer1Size + expectedContent0Size + expectedContent1Size, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java index 3e7104380a0..a8df47aab1a 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java @@ -233,6 +233,10 @@ public class ProvisioningTester { InstanceName.from(UUID.randomUUID().toString())); } + public ApplicationId makeApplicationId(String applicationName) { + return ApplicationId.from("tenant", applicationName, "default"); + } + public List<Node> makeReadyNodes(int n, String flavor) { return makeReadyNodes(n, flavor, NodeType.tenant); } @@ -418,12 +422,15 @@ public class ProvisioningTester { } public List<Node> deploy(ApplicationId application, Capacity capacity) { - List<HostSpec> prepared = prepare(application, clusterSpec(), capacity); + return deploy(application, clusterSpec(), capacity); + } + + public List<Node> deploy(ApplicationId application, ClusterSpec cluster, Capacity capacity) { + List<HostSpec> prepared = prepare(application, cluster, capacity); activate(application, Set.copyOf(prepared)); return getNodes(application, Node.State.active).asList(); } - /** Returns the hosts from the input list which are not retired */ public List<HostSpec> nonRetired(Collection<HostSpec> hosts) { return hosts.stream().filter(host -> ! host.membership().get().retired()).collect(Collectors.toList()); @@ -522,4 +529,22 @@ public class ProvisioningTester { @Override public void log(Level level, String message) { } } + public IdentityHostResourcesCalculator identityHostResourcesCalculator() { + return new IdentityHostResourcesCalculator(); + } + + private static class IdentityHostResourcesCalculator implements HostResourcesCalculator { + + @Override + public NodeResources realResourcesOf(Node node) { + return node.flavor().resources(); + } + + @Override + public NodeResources advertisedResourcesOf(Flavor flavor) { + return flavor.resources(); + } + + } + } diff --git a/searchlib/CMakeLists.txt b/searchlib/CMakeLists.txt index 055dfc6645e..a76baeced04 100644 --- a/searchlib/CMakeLists.txt +++ b/searchlib/CMakeLists.txt @@ -215,6 +215,7 @@ vespa_define_module( src/tests/tensor/dense_tensor_store src/tests/tensor/distance_functions src/tests/tensor/hnsw_index + src/tests/tensor/hnsw_saver src/tests/transactionlog src/tests/transactionlogstress src/tests/true diff --git a/searchlib/src/tests/attribute/enum_attribute_compaction/enum_attribute_compaction_test.cpp b/searchlib/src/tests/attribute/enum_attribute_compaction/enum_attribute_compaction_test.cpp index 31af5945337..45d432c29be 100644 --- a/searchlib/src/tests/attribute/enum_attribute_compaction/enum_attribute_compaction_test.cpp +++ b/searchlib/src/tests/attribute/enum_attribute_compaction/enum_attribute_compaction_test.cpp @@ -221,7 +221,7 @@ TEST_P(IntegerCompactionTest, compact) test_enum_store_compaction(); } -INSTANTIATE_TEST_CASE_P(IntegerCompactionTestSet, IntegerCompactionTest, ::testing::Values(CollectionType::SINGLE, CollectionType::ARRAY, CollectionType::WSET)); +VESPA_GTEST_INSTANTIATE_TEST_SUITE_P(IntegerCompactionTestSet, IntegerCompactionTest, ::testing::Values(CollectionType::SINGLE, CollectionType::ARRAY, CollectionType::WSET)); using StringCompactionTest = CompactionTest<StringAttribute>; @@ -230,6 +230,6 @@ TEST_P(StringCompactionTest, compact) test_enum_store_compaction(); } -INSTANTIATE_TEST_CASE_P(StringCompactionTestSet, StringCompactionTest, ::testing::Values(CollectionType::SINGLE, CollectionType::ARRAY, CollectionType::WSET)); +VESPA_GTEST_INSTANTIATE_TEST_SUITE_P(StringCompactionTestSet, StringCompactionTest, ::testing::Values(CollectionType::SINGLE, CollectionType::ARRAY, CollectionType::WSET)); GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp b/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp index 3a885dda233..43e694f0bcd 100644 --- a/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp +++ b/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp @@ -72,7 +72,7 @@ public: #endif using FloatEnumStoreTestTypes = ::testing::Types<FloatEnumStore, DoubleEnumStore>; -TYPED_TEST_CASE(FloatEnumStoreTest, FloatEnumStoreTestTypes); +VESPA_GTEST_TYPED_TEST_SUITE(FloatEnumStoreTest, FloatEnumStoreTestTypes); TYPED_TEST(FloatEnumStoreTest, numbers_can_be_inserted_and_retrieved) { @@ -452,7 +452,7 @@ LoaderTest<StringEnumStore>::load_values(enumstore::EnumeratedLoaderBase& loader #endif using LoaderTestTypes = ::testing::Types<NumericEnumStore, FloatEnumStore, StringEnumStore>; -TYPED_TEST_CASE(LoaderTest, LoaderTestTypes); +VESPA_GTEST_TYPED_TEST_SUITE(LoaderTest, LoaderTestTypes); TYPED_TEST(LoaderTest, store_is_instantiated_with_enumerated_loader) { diff --git a/searchlib/src/tests/attribute/tensorattribute/tensorattribute_test.cpp b/searchlib/src/tests/attribute/tensorattribute/tensorattribute_test.cpp index b6bdee4f94d..00450eab21a 100644 --- a/searchlib/src/tests/attribute/tensorattribute/tensorattribute_test.cpp +++ b/searchlib/src/tests/attribute/tensorattribute/tensorattribute_test.cpp @@ -148,7 +148,7 @@ public: std::unique_ptr<NearestNeighborIndexSaver> make_saver() const override { return std::unique_ptr<NearestNeighborIndexSaver>(); } - void load(const search::fileutil::LoadedBuffer&) override {} + bool load(const search::fileutil::LoadedBuffer&) override { return false; } std::vector<Neighbor> find_top_k(uint32_t k, vespalib::tensor::TypedCells vector, uint32_t explore_k) const override { (void) k; (void) vector; 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 c562c0cf29c..7a0c240dea5 100644 --- a/searchlib/src/tests/memoryindex/field_index/field_index_test.cpp +++ b/searchlib/src/tests/memoryindex/field_index/field_index_test.cpp @@ -517,7 +517,7 @@ struct FieldIndexTest : public ::testing::Test { }; using FieldIndexTestTypes = ::testing::Types<FieldIndex<false>, FieldIndex<true>>; -TYPED_TEST_CASE(FieldIndexTest, FieldIndexTestTypes); +VESPA_GTEST_TYPED_TEST_SUITE(FieldIndexTest, FieldIndexTestTypes); // Disable warnings emitted by gtest generated files when using typed tests #pragma GCC diagnostic push diff --git a/searchlib/src/tests/tensor/hnsw_saver/CMakeLists.txt b/searchlib/src/tests/tensor/hnsw_saver/CMakeLists.txt new file mode 100644 index 00000000000..90202e222a7 --- /dev/null +++ b/searchlib/src/tests/tensor/hnsw_saver/CMakeLists.txt @@ -0,0 +1,9 @@ +# Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(searchlib_hnsw_save_load_test_app TEST + SOURCES + hnsw_save_load_test.cpp + DEPENDS + searchlib + gtest +) +vespa_add_test(NAME searchlib_hnsw_save_load_test_app COMMAND searchlib_hnsw_save_load_test_app) diff --git a/searchlib/src/tests/tensor/hnsw_saver/hnsw_save_load_test.cpp b/searchlib/src/tests/tensor/hnsw_saver/hnsw_save_load_test.cpp new file mode 100644 index 00000000000..b9e27d413f3 --- /dev/null +++ b/searchlib/src/tests/tensor/hnsw_saver/hnsw_save_load_test.cpp @@ -0,0 +1,150 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/searchlib/tensor/hnsw_graph.h> +#include <vespa/searchlib/tensor/hnsw_index_saver.h> +#include <vespa/searchlib/tensor/hnsw_index_loader.h> +#include <vespa/vespalib/util/bufferwriter.h> +#include <vespa/searchlib/util/fileutil.h> +#include <vespa/vespalib/gtest/gtest.h> +#include <vector> + +#include <vespa/log/log.h> +LOG_SETUP("hnsw_save_load_test"); + +using namespace search::tensor; +using search::BufferWriter; +using search::fileutil::LoadedBuffer; + +class VectorBufferWriter : public BufferWriter { +private: + char tmp[1024]; +public: + std::vector<char> output; + VectorBufferWriter() { + setup(tmp, 1024); + } + ~VectorBufferWriter() {} + void flush() override { + for (size_t i = 0; i < usedLen(); ++i) { + output.push_back(tmp[i]); + } + rewind(); + } +}; + +using V = std::vector<uint32_t>; + +void populate(HnswGraph &graph) { + // no 0 + graph.make_node_for_document(1, 1); + graph.make_node_for_document(2, 2); + // no 3 + graph.make_node_for_document(4, 2); + graph.make_node_for_document(5, 0); + graph.make_node_for_document(6, 1); + + graph.set_link_array(1, 0, V{2, 4, 6}); + graph.set_link_array(2, 0, V{1, 4, 6}); + graph.set_link_array(4, 0, V{1, 2, 6}); + graph.set_link_array(6, 0, V{1, 2, 4}); + graph.set_link_array(2, 1, V{4}); + graph.set_link_array(4, 1, V{2}); + graph.set_entry_node(2, 1); +} + +void modify(HnswGraph &graph) { + graph.remove_node_for_document(2); + graph.remove_node_for_document(6); + graph.make_node_for_document(7, 2); + + graph.set_link_array(1, 0, V{7, 4}); + graph.set_link_array(4, 0, V{7, 2}); + graph.set_link_array(7, 0, V{4, 2}); + graph.set_link_array(4, 1, V{7}); + graph.set_link_array(7, 1, V{4}); + + graph.set_entry_node(4, 1); +} + + +class CopyGraphTest : public ::testing::Test { +public: + HnswGraph original; + HnswGraph copy; + + void expect_empty_d(uint32_t docid) const { + EXPECT_FALSE(copy.node_refs[docid].load_acquire().valid()); + } + + void expect_level_0(uint32_t docid, const V& exp_links) const { + auto levels = copy.get_level_array(docid); + EXPECT_GE(levels.size(), 1); + auto links = copy.get_link_array(docid, 0); + EXPECT_EQ(exp_links.size(), links.size()); + for (size_t i = 0; i < exp_links.size() && i < links.size(); ++i) { + EXPECT_EQ(exp_links[i], links[i]); + } + } + + void expect_level_1(uint32_t docid, const V& exp_links) const { + auto levels = copy.get_level_array(docid); + EXPECT_EQ(2, levels.size()); + auto links = copy.get_link_array(docid, 1); + EXPECT_EQ(exp_links.size(), links.size()); + for (size_t i = 0; i < exp_links.size() && i < links.size(); ++i) { + EXPECT_EQ(exp_links[i], links[i]); + } + } + + std::vector<char> save_original() const { + HnswIndexSaver saver(original); + VectorBufferWriter vector_writer; + saver.save(vector_writer); + return vector_writer.output; + } + void load_copy(std::vector<char> data) { + HnswIndexLoader loader(copy); + LoadedBuffer buffer(&data[0], data.size()); + loader.load(buffer); + } + + void expect_copy_as_populated() const { + EXPECT_EQ(copy.size(), 7); + EXPECT_EQ(copy.entry_docid, 2); + EXPECT_EQ(copy.entry_level, 1); + + expect_empty_d(0); + expect_empty_d(3); + expect_empty_d(5); + + expect_level_0(1, {2, 4, 6}); + expect_level_0(2, {1, 4, 6}); + expect_level_0(4, {1, 2, 6}); + expect_level_0(6, {1, 2, 4}); + + expect_level_1(2, {4}); + expect_level_1(4, {2}); + } +}; + +TEST_F(CopyGraphTest, reconstructs_graph) +{ + populate(original); + auto data = save_original(); + load_copy(data); + expect_copy_as_populated(); +} + +TEST_F(CopyGraphTest, later_changes_ignored) +{ + populate(original); + HnswIndexSaver saver(original); + modify(original); + VectorBufferWriter vector_writer; + saver.save(vector_writer); + auto data = vector_writer.output; + load_copy(data); + expect_copy_as_populated(); +} + +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/vespa/searchlib/tensor/CMakeLists.txt b/searchlib/src/vespa/searchlib/tensor/CMakeLists.txt index 7090158c773..0f106f693f8 100644 --- a/searchlib/src/vespa/searchlib/tensor/CMakeLists.txt +++ b/searchlib/src/vespa/searchlib/tensor/CMakeLists.txt @@ -9,11 +9,15 @@ vespa_add_library(searchlib_tensor OBJECT generic_tensor_attribute.cpp generic_tensor_attribute_saver.cpp generic_tensor_store.cpp + hnsw_graph.cpp hnsw_index.cpp + hnsw_index_loader.cpp + hnsw_index_saver.cpp imported_tensor_attribute_vector.cpp imported_tensor_attribute_vector_read_guard.cpp inv_log_level_generator.cpp nearest_neighbor_index.cpp + nearest_neighbor_index_saver.cpp tensor_attribute.cpp tensor_store.cpp DEPENDS diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_graph.cpp b/searchlib/src/vespa/searchlib/tensor/hnsw_graph.cpp new file mode 100644 index 00000000000..db8d1067980 --- /dev/null +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_graph.cpp @@ -0,0 +1,53 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "hnsw_graph.h" +#include "hnsw_index.h" +#include <vespa/vespalib/datastore/array_store.hpp> +#include <vespa/vespalib/util/rcuvector.hpp> + +namespace search::tensor { + +HnswGraph::HnswGraph() + : node_refs(), + nodes(HnswIndex::make_default_node_store_config()), + links(HnswIndex::make_default_link_store_config()), + entry_docid(0), // Note that docid 0 is reserved and never used + entry_level(-1) +{} + +HnswGraph::~HnswGraph() {} + +void +HnswGraph::make_node_for_document(uint32_t docid, uint32_t num_levels) +{ + node_refs.ensure_size(docid + 1, AtomicEntryRef()); + // A document cannot be added twice. + assert(!node_refs[docid].load_acquire().valid()); + // Note: The level array instance lives as long as the document is present in the index. + vespalib::Array<AtomicEntryRef> levels(num_levels, AtomicEntryRef()); + auto node_ref = nodes.add(levels); + node_refs[docid].store_release(node_ref); +} + +void +HnswGraph::remove_node_for_document(uint32_t docid) +{ + auto node_ref = node_refs[docid].load_acquire(); + nodes.remove(node_ref); + search::datastore::EntryRef invalid; + node_refs[docid].store_release(invalid); +} + +void +HnswGraph::set_link_array(uint32_t docid, uint32_t level, const LinkArrayRef& new_links) +{ + auto new_links_ref = links.add(new_links); + auto node_ref = node_refs[docid].load_acquire(); + assert(node_ref.valid()); + auto levels = nodes.get_writable(node_ref); + auto old_links_ref = levels[level].load_acquire(); + levels[level].store_release(new_links_ref); + links.remove(old_links_ref); +} + +} // namespace diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_graph.h b/searchlib/src/vespa/searchlib/tensor/hnsw_graph.h new file mode 100644 index 00000000000..64892d06f09 --- /dev/null +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_graph.h @@ -0,0 +1,74 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <vespa/vespalib/datastore/array_store.h> +#include <vespa/vespalib/datastore/atomic_entry_ref.h> +#include <vespa/vespalib/datastore/entryref.h> +#include <vespa/vespalib/util/rcuvector.h> + +namespace search::tensor { + +/** + * Stroage of a hierarchical navigable small world graph (HNSW) + * that is used for approximate K-nearest neighbor search. + */ +struct HnswGraph { + using AtomicEntryRef = search::datastore::AtomicEntryRef; + + // This uses 10 bits for buffer id -> 1024 buffers. + // As we have very short arrays we get less fragmentation with fewer and larger buffers. + using EntryRefType = search::datastore::EntryRefT<22>; + + // Provides mapping from document id -> node reference. + // The reference is used to lookup the node data in NodeStore. + using NodeRefVector = vespalib::RcuVector<AtomicEntryRef>; + + // This stores the level arrays for all nodes. + // Each node consists of an array of levels (from level 0 to n) where each entry is a reference to the link array at that level. + using NodeStore = search::datastore::ArrayStore<AtomicEntryRef, EntryRefType>; + using StoreConfig = search::datastore::ArrayStoreConfig; + using LevelArrayRef = NodeStore::ConstArrayRef; + + // This stores all link arrays. + // A link array consists of the document ids of the nodes a particular node is linked to. + using LinkStore = search::datastore::ArrayStore<uint32_t, EntryRefType>; + using LinkArrayRef = LinkStore::ConstArrayRef; + + NodeRefVector node_refs; + NodeStore nodes; + LinkStore links; + uint32_t entry_docid; + int32_t entry_level; + + HnswGraph(); + + ~HnswGraph(); + + void make_node_for_document(uint32_t docid, uint32_t num_levels); + + void remove_node_for_document(uint32_t docid); + + LevelArrayRef get_level_array(uint32_t docid) const { + auto node_ref = node_refs[docid].load_acquire(); + assert(node_ref.valid()); + return nodes.get(node_ref); + } + + LinkArrayRef get_link_array(uint32_t docid, uint32_t level) const { + auto levels = get_level_array(docid); + assert(level < levels.size()); + return links.get(levels[level].load_acquire()); + } + + void set_link_array(uint32_t docid, uint32_t level, const LinkArrayRef& new_links); + + void set_entry_node(uint32_t docid, int32_t level) { + entry_docid = docid; + entry_level = level; + } + + size_t size() const { return node_refs.size(); } +}; + +} diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp b/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp index 19b02d18893..de6daba650c 100644 --- a/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp @@ -2,7 +2,8 @@ #include "distance_function.h" #include "hnsw_index.h" -#include "nearest_neighbor_index_saver.h" +#include "hnsw_index_loader.h" +#include "hnsw_index_saver.h" #include "random_level_generator.h" #include <vespa/searchlib/util/state_explorer_utils.h> #include <vespa/eval/tensor/dense/typed_cells.h> @@ -67,54 +68,6 @@ HnswIndex::max_links_for_level(uint32_t level) const return (level == 0) ? _cfg.max_links_at_level_0() : _cfg.max_links_on_inserts(); } -void -HnswIndex::make_node_for_document(uint32_t docid, uint32_t num_levels) -{ - _node_refs.ensure_size(docid + 1, AtomicEntryRef()); - // A document cannot be added twice. - assert(!_node_refs[docid].load_acquire().valid()); - - // Note: The level array instance lives as long as the document is present in the index. - LevelArray levels(num_levels, AtomicEntryRef()); - auto node_ref = _nodes.add(levels); - _node_refs[docid].store_release(node_ref); -} - -void -HnswIndex::remove_node_for_document(uint32_t docid) -{ - auto node_ref = _node_refs[docid].load_acquire(); - _nodes.remove(node_ref); - EntryRef invalid; - _node_refs[docid].store_release(invalid); -} - -HnswIndex::LevelArrayRef -HnswIndex::get_level_array(uint32_t docid) const -{ - auto node_ref = _node_refs[docid].load_acquire(); - return _nodes.get(node_ref); -} - -HnswIndex::LinkArrayRef -HnswIndex::get_link_array(uint32_t docid, uint32_t level) const -{ - auto levels = get_level_array(docid); - assert(level < levels.size()); - return _links.get(levels[level].load_acquire()); -} - -void -HnswIndex::set_link_array(uint32_t docid, uint32_t level, const LinkArrayRef& links) -{ - auto new_links_ref = _links.add(links); - auto node_ref = _node_refs[docid].load_acquire(); - auto levels = _nodes.get_writable(node_ref); - auto old_links_ref = levels[level].load_acquire(); - levels[level].store_release(new_links_ref); - _links.remove(old_links_ref); -} - bool HnswIndex::have_closer_distance(HnswCandidate candidate, const LinkArrayRef& result) const { @@ -183,7 +136,7 @@ HnswIndex::select_neighbors(const HnswCandidateVector& neighbors, uint32_t max_l void HnswIndex::shrink_if_needed(uint32_t docid, uint32_t level) { - auto old_links = get_link_array(docid, level); + auto old_links = _graph.get_link_array(docid, level); uint32_t max_links = max_links_for_level(level); if (old_links.size() > max_links) { HnswCandidateVector neighbors; @@ -192,7 +145,7 @@ HnswIndex::shrink_if_needed(uint32_t docid, uint32_t level) neighbors.emplace_back(neighbor_docid, dist); } auto split = select_neighbors(neighbors, max_links); - set_link_array(docid, level, split.used); + _graph.set_link_array(docid, level, split.used); for (uint32_t removed_docid : split.unused) { remove_link_to(removed_docid, docid, level); } @@ -202,9 +155,9 @@ HnswIndex::shrink_if_needed(uint32_t docid, uint32_t level) void HnswIndex::connect_new_node(uint32_t docid, const LinkArrayRef &neighbors, uint32_t level) { - set_link_array(docid, level, neighbors); + _graph.set_link_array(docid, level, neighbors); for (uint32_t neighbor_docid : neighbors) { - auto old_links = get_link_array(neighbor_docid, level); + auto old_links = _graph.get_link_array(neighbor_docid, level); add_link_to(neighbor_docid, level, old_links, docid); } for (uint32_t neighbor_docid : neighbors) { @@ -216,11 +169,11 @@ void HnswIndex::remove_link_to(uint32_t remove_from, uint32_t remove_id, uint32_t level) { LinkArray new_links; - auto old_links = get_link_array(remove_from, level); + auto old_links = _graph.get_link_array(remove_from, level); for (uint32_t id : old_links) { if (id != remove_id) new_links.push_back(id); } - set_link_array(remove_from, level, new_links); + _graph.set_link_array(remove_from, level, new_links); } @@ -245,7 +198,7 @@ HnswIndex::find_nearest_in_layer(const TypedCells& input, const HnswCandidate& e bool keep_searching = true; while (keep_searching) { keep_searching = false; - for (uint32_t neighbor_docid : get_link_array(nearest.docid, level)) { + for (uint32_t neighbor_docid : _graph.get_link_array(nearest.docid, level)) { double dist = calc_distance(input, neighbor_docid); if (dist < nearest.distance) { nearest = HnswCandidate(neighbor_docid, dist); @@ -260,7 +213,7 @@ void HnswIndex::search_layer(const TypedCells& input, uint32_t neighbors_to_find, FurthestPriQ& best_neighbors, uint32_t level) const { NearestPriQ candidates; - uint32_t doc_id_limit = _node_refs.size(); + uint32_t doc_id_limit = _graph.node_refs.size(); auto visited = _visited_set_pool.get(doc_id_limit); for (const auto &entry : best_neighbors.peek()) { assert(entry.docid < doc_id_limit); @@ -275,7 +228,7 @@ HnswIndex::search_layer(const TypedCells& input, uint32_t neighbors_to_find, Fur break; } candidates.pop(); - for (uint32_t neighbor_docid : get_link_array(cand.docid, level)) { + for (uint32_t neighbor_docid : _graph.get_link_array(cand.docid, level)) { if ((neighbor_docid >= doc_id_limit) || visited.is_marked(neighbor_docid)) { continue; } @@ -295,15 +248,12 @@ HnswIndex::search_layer(const TypedCells& input, uint32_t neighbors_to_find, Fur HnswIndex::HnswIndex(const DocVectorAccess& vectors, DistanceFunction::UP distance_func, RandomLevelGenerator::UP level_generator, const Config& cfg) - : _vectors(vectors), + : + _graph(), + _vectors(vectors), _distance_func(std::move(distance_func)), _level_generator(std::move(level_generator)), - _cfg(cfg), - _node_refs(), - _nodes(make_default_node_store_config()), - _links(make_default_link_store_config()), - _entry_docid(0), // Note that docid 0 is reserved and never used - _entry_level(-1) + _cfg(cfg) { } @@ -315,16 +265,16 @@ HnswIndex::add_document(uint32_t docid) auto input = get_vector(docid); // TODO: Add capping on num_levels int level = _level_generator->max_level(); - make_node_for_document(docid, level + 1); - if (_entry_docid == 0) { - _entry_docid = docid; - _entry_level = level; + _graph.make_node_for_document(docid, level + 1); + uint32_t entry_docid = get_entry_docid(); + if (entry_docid == 0) { + _graph.set_entry_node(docid, level); return; } - int search_level = _entry_level; - double entry_dist = calc_distance(input, _entry_docid); - HnswCandidate entry_point(_entry_docid, entry_dist); + int search_level = get_entry_level(); + double entry_dist = calc_distance(input, entry_docid); + HnswCandidate entry_point(entry_docid, entry_dist); while (search_level > level) { entry_point = find_nearest_in_layer(input, entry_point, search_level); --search_level; @@ -332,7 +282,7 @@ HnswIndex::add_document(uint32_t docid) FurthestPriQ best_neighbors; best_neighbors.push(entry_point); - search_level = std::min(level, _entry_level); + search_level = std::min(level, search_level); // Insert the added document in each level it should exist in. while (search_level >= 0) { @@ -342,9 +292,8 @@ HnswIndex::add_document(uint32_t docid) connect_new_node(docid, neighbors.used, search_level); --search_level; } - if (level > _entry_level) { - _entry_docid = docid; - _entry_level = level; + if (level > get_entry_level()) { + _graph.set_entry_node(docid, level); } } @@ -354,7 +303,7 @@ HnswIndex::mutual_reconnect(const LinkArrayRef &cluster, uint32_t level) std::vector<PairDist> pairs; for (uint32_t i = 0; i + 1 < cluster.size(); ++i) { uint32_t n_id_1 = cluster[i]; - LinkArrayRef n_list_1 = get_link_array(n_id_1, level); + LinkArrayRef n_list_1 = _graph.get_link_array(n_id_1, level); for (uint32_t j = i + 1; j < cluster.size(); ++j) { uint32_t n_id_2 = cluster[j]; if (has_link_to(n_list_1, n_id_2)) continue; @@ -363,10 +312,10 @@ HnswIndex::mutual_reconnect(const LinkArrayRef &cluster, uint32_t level) } std::sort(pairs.begin(), pairs.end()); for (const PairDist & pair : pairs) { - LinkArrayRef old_links_1 = get_link_array(pair.id_first, level); + LinkArrayRef old_links_1 = _graph.get_link_array(pair.id_first, level); if (old_links_1.size() >= _cfg.max_links_on_inserts()) continue; - LinkArrayRef old_links_2 = get_link_array(pair.id_second, level); + LinkArrayRef old_links_2 = _graph.get_link_array(pair.id_second, level); if (old_links_2.size() >= _cfg.max_links_on_inserts()) continue; add_link_to(pair.id_first, level, old_links_1, pair.id_second); @@ -377,27 +326,25 @@ HnswIndex::mutual_reconnect(const LinkArrayRef &cluster, uint32_t level) void HnswIndex::remove_document(uint32_t docid) { - bool need_new_entrypoint = (docid == _entry_docid); + bool need_new_entrypoint = (docid == get_entry_docid()); LinkArray empty; - LevelArrayRef node_levels = get_level_array(docid); + LevelArrayRef node_levels = _graph.get_level_array(docid); for (int level = node_levels.size(); level-- > 0; ) { - LinkArrayRef my_links = get_link_array(docid, level); + LinkArrayRef my_links = _graph.get_link_array(docid, level); for (uint32_t neighbor_id : my_links) { if (need_new_entrypoint) { - _entry_docid = neighbor_id; - _entry_level = level; + _graph.set_entry_node(neighbor_id, level); need_new_entrypoint = false; } remove_link_to(neighbor_id, docid, level); } mutual_reconnect(my_links, level); - set_link_array(docid, level, empty); + _graph.set_link_array(docid, level, empty); } if (need_new_entrypoint) { - _entry_docid = 0; - _entry_level = -1; + _graph.set_entry_node(0, -1); } - remove_node_for_document(docid); + _graph.remove_node_for_document(docid); } void @@ -405,26 +352,26 @@ HnswIndex::transfer_hold_lists(generation_t current_gen) { // Note: RcuVector transfers hold lists as part of reallocation based on current generation. // We need to set the next generation here, as it is incremented on a higher level right after this call. - _node_refs.setGeneration(current_gen + 1); - _nodes.transferHoldLists(current_gen); - _links.transferHoldLists(current_gen); + _graph.node_refs.setGeneration(current_gen + 1); + _graph.nodes.transferHoldLists(current_gen); + _graph.links.transferHoldLists(current_gen); } void HnswIndex::trim_hold_lists(generation_t first_used_gen) { - _node_refs.removeOldGenerations(first_used_gen); - _nodes.trimHoldLists(first_used_gen); - _links.trimHoldLists(first_used_gen); + _graph.node_refs.removeOldGenerations(first_used_gen); + _graph.nodes.trimHoldLists(first_used_gen); + _graph.links.trimHoldLists(first_used_gen); } vespalib::MemoryUsage HnswIndex::memory_usage() const { vespalib::MemoryUsage result; - result.merge(_node_refs.getMemoryUsage()); - result.merge(_nodes.getMemoryUsage()); - result.merge(_links.getMemoryUsage()); + result.merge(_graph.node_refs.getMemoryUsage()); + result.merge(_graph.nodes.getMemoryUsage()); + result.merge(_graph.links.getMemoryUsage()); result.merge(_visited_set_pool.memory_usage()); return result; } @@ -439,13 +386,15 @@ HnswIndex::get_state(const vespalib::slime::Inserter& inserter) const std::unique_ptr<NearestNeighborIndexSaver> HnswIndex::make_saver() const { - return std::unique_ptr<NearestNeighborIndexSaver>(); + return std::make_unique<HnswIndexSaver>(_graph); } -void +bool HnswIndex::load(const fileutil::LoadedBuffer& buf) { - (void) buf; + assert(get_entry_docid() == 0); // cannot load after index has data + HnswIndexLoader loader(_graph); + return loader.load(buf); } struct NeighborsByDocId { @@ -476,12 +425,13 @@ FurthestPriQ HnswIndex::top_k_candidates(const TypedCells &vector, uint32_t k) const { FurthestPriQ best_neighbors; - if (_entry_level < 0) { + if (get_entry_level() < 0) { return best_neighbors; } - double entry_dist = calc_distance(vector, _entry_docid); - HnswCandidate entry_point(_entry_docid, entry_dist); - int search_level = _entry_level; + uint32_t entry_docid = get_entry_docid(); + int search_level = get_entry_level(); + double entry_dist = calc_distance(vector, entry_docid); + HnswCandidate entry_point(entry_docid, entry_dist); while (search_level > 0) { entry_point = find_nearest_in_layer(vector, entry_point, search_level); --search_level; @@ -494,14 +444,14 @@ HnswIndex::top_k_candidates(const TypedCells &vector, uint32_t k) const HnswNode HnswIndex::get_node(uint32_t docid) const { - auto node_ref = _node_refs[docid].load_acquire(); + auto node_ref = _graph.node_refs[docid].load_acquire(); if (!node_ref.valid()) { return HnswNode(); } - auto levels = _nodes.get(node_ref); + auto levels = _graph.nodes.get(node_ref); HnswNode::LevelArray result; for (const auto& links_ref : levels) { - auto links = _links.get(links_ref.load_acquire()); + auto links = _graph.links.get(links_ref.load_acquire()); HnswNode::LinkArray result_links(links.begin(), links.end()); std::sort(result_links.begin(), result_links.end()); result.push_back(result_links); @@ -514,14 +464,13 @@ HnswIndex::set_node(uint32_t docid, const HnswNode &node) { size_t num_levels = node.size(); assert(num_levels > 0); - make_node_for_document(docid, num_levels); + _graph.make_node_for_document(docid, num_levels); for (size_t level = 0; level < num_levels; ++level) { connect_new_node(docid, node.level(level), level); } int max_level = num_levels - 1; - if (_entry_level < max_level) { - _entry_docid = docid; - _entry_level = max_level; + if (get_entry_level() < max_level) { + _graph.set_entry_node(docid, max_level); } } @@ -529,15 +478,15 @@ bool HnswIndex::check_link_symmetry() const { bool all_sym = true; - for (size_t docid = 0; docid < _node_refs.size(); ++docid) { - auto node_ref = _node_refs[docid].load_acquire(); + for (size_t docid = 0; docid < _graph.node_refs.size(); ++docid) { + auto node_ref = _graph.node_refs[docid].load_acquire(); if (node_ref.valid()) { - auto levels = _nodes.get(node_ref); + auto levels = _graph.nodes.get(node_ref); uint32_t level = 0; for (const auto& links_ref : levels) { - auto links = _links.get(links_ref.load_acquire()); + auto links = _graph.links.get(links_ref.load_acquire()); for (auto neighbor_docid : links) { - auto neighbor_links = get_link_array(neighbor_docid, level); + auto neighbor_links = _graph.get_link_array(neighbor_docid, level); if (! has_link_to(neighbor_links, docid)) { all_sym = false; } diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index.h b/searchlib/src/vespa/searchlib/tensor/hnsw_index.h index 1185acd9624..b5d57c2ebfd 100644 --- a/searchlib/src/vespa/searchlib/tensor/hnsw_index.h +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index.h @@ -8,6 +8,7 @@ #include "hnsw_node.h" #include "nearest_neighbor_index.h" #include "random_level_generator.h" +#include "hnsw_graph.h" #include <vespa/eval/tensor/dense/typed_cells.h> #include <vespa/searchlib/common/bitvector.h> #include <vespa/vespalib/datastore/array_store.h> @@ -57,54 +58,30 @@ public: }; protected: - using AtomicEntryRef = search::datastore::AtomicEntryRef; + using AtomicEntryRef = HnswGraph::AtomicEntryRef; + using NodeStore = HnswGraph::NodeStore; - // This uses 10 bits for buffer id -> 1024 buffers. - // As we have very short arrays we get less fragmentation with fewer and larger buffers. - using EntryRefType = search::datastore::EntryRefT<22>; - - // Provides mapping from document id -> node reference. - // The reference is used to lookup the node data in NodeStore. - using NodeRefVector = vespalib::RcuVector<AtomicEntryRef>; + using LinkStore = HnswGraph::LinkStore; + using LinkArrayRef = HnswGraph::LinkArrayRef; + using LinkArray = vespalib::Array<uint32_t>; - // This stores the level arrays for all nodes. - // Each node consists of an array of levels (from level 0 to n) where each entry is a reference to the link array at that level. - using NodeStore = search::datastore::ArrayStore<AtomicEntryRef, EntryRefType>; - using LevelArrayRef = NodeStore::ConstArrayRef; + using LevelArrayRef = HnswGraph::LevelArrayRef; using LevelArray = vespalib::Array<AtomicEntryRef>; - // This stores all link arrays. - // A link array consists of the document ids of the nodes a particular node is linked to. - using LinkStore = search::datastore::ArrayStore<uint32_t, EntryRefType>; - using LinkArrayRef = LinkStore::ConstArrayRef; - using LinkArray = vespalib::Array<uint32_t>; - using TypedCells = vespalib::tensor::TypedCells; + HnswGraph _graph; const DocVectorAccess& _vectors; DistanceFunction::UP _distance_func; RandomLevelGenerator::UP _level_generator; Config _cfg; - NodeRefVector _node_refs; - NodeStore _nodes; - LinkStore _links; mutable vespalib::ReusableSetPool _visited_set_pool; - uint32_t _entry_docid; - int _entry_level; - - static search::datastore::ArrayStoreConfig make_default_node_store_config(); - static search::datastore::ArrayStoreConfig make_default_link_store_config(); uint32_t max_links_for_level(uint32_t level) const; - void make_node_for_document(uint32_t docid, uint32_t num_levels); - void remove_node_for_document(uint32_t docid); - LevelArrayRef get_level_array(uint32_t docid) const; - LinkArrayRef get_link_array(uint32_t docid, uint32_t level) const; - void set_link_array(uint32_t docid, uint32_t level, const LinkArrayRef& links); void add_link_to(uint32_t docid, uint32_t level, const LinkArrayRef& old_links, uint32_t new_link) { LinkArray new_links(old_links.begin(), old_links.end()); new_links.push_back(new_link); - set_link_array(docid, level, new_links); + _graph.set_link_array(docid, level, new_links); } /** @@ -156,20 +133,23 @@ public: void get_state(const vespalib::slime::Inserter& inserter) const override; std::unique_ptr<NearestNeighborIndexSaver> make_saver() const override; - void load(const fileutil::LoadedBuffer& buf) override; + bool load(const fileutil::LoadedBuffer& buf) override; std::vector<Neighbor> find_top_k(uint32_t k, TypedCells vector, uint32_t explore_k) const override; const DistanceFunction *distance_function() const override { return _distance_func.get(); } FurthestPriQ top_k_candidates(const TypedCells &vector, uint32_t k) const; - uint32_t get_entry_docid() const { return _entry_docid; } - uint32_t get_entry_level() const { return _entry_level; } + uint32_t get_entry_docid() const { return _graph.entry_docid; } + int32_t get_entry_level() const { return _graph.entry_level; } // Should only be used by unit tests. HnswNode get_node(uint32_t docid) const; void set_node(uint32_t docid, const HnswNode &node); bool check_link_symmetry() const; + + static search::datastore::ArrayStoreConfig make_default_node_store_config(); + static search::datastore::ArrayStoreConfig make_default_link_store_config(); }; } diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index_loader.cpp b/searchlib/src/vespa/searchlib/tensor/hnsw_index_loader.cpp new file mode 100644 index 00000000000..75c62e5202f --- /dev/null +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index_loader.cpp @@ -0,0 +1,47 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "hnsw_index_loader.h" +#include "hnsw_graph.h" +#include <vespa/searchlib/util/fileutil.h> + +namespace search::tensor { + +HnswIndexLoader::~HnswIndexLoader() {} + +HnswIndexLoader::HnswIndexLoader(HnswGraph &graph) + : _graph(graph), _ptr(nullptr), _end(nullptr), _failed(false) +{ +} + +bool +HnswIndexLoader::load(const fileutil::LoadedBuffer& buf) +{ + size_t num_readable = buf.size(sizeof(uint32_t)); + _ptr = static_cast<const uint32_t *>(buf.buffer()); + _end = _ptr + num_readable; + uint32_t entry_docid = nextVal(); + int32_t entry_level = nextVal(); + uint32_t num_nodes = nextVal(); + std::vector<uint32_t> link_array; + for (uint32_t docid = 0; docid < num_nodes; ++docid) { + uint32_t num_levels = nextVal(); + if (num_levels > 0) { + _graph.make_node_for_document(docid, num_levels); + for (uint32_t level = 0; level < num_levels; ++level) { + uint32_t num_links = nextVal(); + link_array.clear(); + while (num_links-- > 0) { + link_array.push_back(nextVal()); + } + _graph.set_link_array(docid, level, link_array); + } + } + } + if (_failed) return false; + _graph.node_refs.ensure_size(num_nodes); + _graph.set_entry_node(entry_docid, entry_level); + return true; +} + + +} diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index_loader.h b/searchlib/src/vespa/searchlib/tensor/hnsw_index_loader.h new file mode 100644 index 00000000000..abc68889a1b --- /dev/null +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index_loader.h @@ -0,0 +1,35 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <cstdint> + +namespace search::fileutil { class LoadedBuffer; } + +namespace search::tensor { + +class HnswGraph; + +/** + * Implements loading of HNSW graph structure from binary format. + **/ +class HnswIndexLoader { +public: + HnswIndexLoader(HnswGraph &graph); + ~HnswIndexLoader(); + bool load(const fileutil::LoadedBuffer& buf); +private: + HnswGraph &_graph; + const uint32_t *_ptr; + const uint32_t *_end; + bool _failed; + uint32_t nextVal() { + if (__builtin_expect((_ptr == _end), false)) { + _failed = true; + return 0; + } + return *_ptr++; + } +}; + +} diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.cpp b/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.cpp new file mode 100644 index 00000000000..acff30f8cbf --- /dev/null +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.cpp @@ -0,0 +1,57 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "hnsw_index_saver.h" +#include "hnsw_graph.h" +#include <vespa/vespalib/util/bufferwriter.h> + +namespace search::tensor { + +HnswIndexSaver::~HnswIndexSaver() {} + +HnswIndexSaver::HnswIndexSaver(const HnswGraph &graph) + : _graph_links(graph.links), _meta_data() +{ + _meta_data.entry_docid = graph.entry_docid; + _meta_data.entry_level = graph.entry_level; + size_t num_nodes = graph.node_refs.size(); + _meta_data.nodes.reserve(num_nodes); + for (size_t i = 0; i < num_nodes; ++i) { + LevelVector node; + auto node_ref = graph.node_refs[i].load_acquire(); + if (node_ref.valid()) { + auto levels = graph.nodes.get(node_ref); + for (const auto& links_ref : levels) { + auto level = links_ref.load_acquire(); + node.push_back(level); + } + } + _meta_data.nodes.emplace_back(std::move(node)); + } +} + +void +HnswIndexSaver::save(BufferWriter& writer) const +{ + writer.write(&_meta_data.entry_docid, sizeof(uint32_t)); + writer.write(&_meta_data.entry_level, sizeof(int32_t)); + uint32_t num_nodes = _meta_data.nodes.size(); + writer.write(&num_nodes, sizeof(uint32_t)); + for (const auto &node : _meta_data.nodes) { + uint32_t num_levels = node.size(); + writer.write(&num_levels, sizeof(uint32_t)); + for (auto links_ref : node) { + if (links_ref.valid()) { + vespalib::ConstArrayRef<uint32_t> link_array = _graph_links.get(links_ref); + uint32_t num_links = link_array.size(); + writer.write(&num_links, sizeof(uint32_t)); + writer.write(link_array.cbegin(), sizeof(uint32_t)*num_links); + } else { + uint32_t num_links = 0; + writer.write(&num_links, sizeof(uint32_t)); + } + } + } + writer.flush(); +} + +} diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.h b/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.h new file mode 100644 index 00000000000..d1d8e0db19d --- /dev/null +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.h @@ -0,0 +1,37 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "nearest_neighbor_index_saver.h" +#include "hnsw_graph.h" +#include <vespa/vespalib/datastore/entryref.h> +#include <vector> + +namespace search::tensor { + +/** + * Implements saving of HNSW graph structure in binary format. + * The constructor takes a snapshot of all meta-data, but + * the links will be fetched from the graph in the save() + * method. + **/ +class HnswIndexSaver : public NearestNeighborIndexSaver { +public: + using LevelVector = std::vector<search::datastore::EntryRef>; + + HnswIndexSaver(const HnswGraph &graph); + ~HnswIndexSaver(); + void save(BufferWriter& writer) const override; + +private: + struct MetaData { + uint32_t entry_docid; + int32_t entry_level; + std::vector<LevelVector> nodes; + MetaData() : entry_docid(0), entry_level(-1), nodes() {} + }; + const HnswGraph::LinkStore &_graph_links; + MetaData _meta_data; +}; + +} diff --git a/searchlib/src/vespa/searchlib/tensor/nearest_neighbor_index.h b/searchlib/src/vespa/searchlib/tensor/nearest_neighbor_index.h index bb6ef012a56..aca2ce2af66 100644 --- a/searchlib/src/vespa/searchlib/tensor/nearest_neighbor_index.h +++ b/searchlib/src/vespa/searchlib/tensor/nearest_neighbor_index.h @@ -47,7 +47,7 @@ public: * and the caller ensures that an attribute read guard is held during the lifetime of the saver. */ virtual std::unique_ptr<NearestNeighborIndexSaver> make_saver() const = 0; - virtual void load(const fileutil::LoadedBuffer& buf) = 0; + virtual bool load(const fileutil::LoadedBuffer& buf) = 0; virtual std::vector<Neighbor> find_top_k(uint32_t k, vespalib::tensor::TypedCells vector, diff --git a/searchlib/src/vespa/searchlib/tensor/nearest_neighbor_index_saver.cpp b/searchlib/src/vespa/searchlib/tensor/nearest_neighbor_index_saver.cpp new file mode 100644 index 00000000000..4b293488737 --- /dev/null +++ b/searchlib/src/vespa/searchlib/tensor/nearest_neighbor_index_saver.cpp @@ -0,0 +1,3 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "nearest_neighbor_index_saver.h" diff --git a/searchlib/src/vespa/searchlib/tensor/nearest_neighbor_index_saver.h b/searchlib/src/vespa/searchlib/tensor/nearest_neighbor_index_saver.h index 7d599ac31c8..cee48d63359 100644 --- a/searchlib/src/vespa/searchlib/tensor/nearest_neighbor_index_saver.h +++ b/searchlib/src/vespa/searchlib/tensor/nearest_neighbor_index_saver.h @@ -21,7 +21,7 @@ namespace search::tensor { class NearestNeighborIndexSaver { public: virtual ~NearestNeighborIndexSaver() {} - virtual void save(BufferWriter& writer) const; + virtual void save(BufferWriter& writer) const = 0; }; } diff --git a/storage/src/tests/distributor/btree_bucket_database_test.cpp b/storage/src/tests/distributor/btree_bucket_database_test.cpp index 43d74ca2fb5..a2518272a7f 100644 --- a/storage/src/tests/distributor/btree_bucket_database_test.cpp +++ b/storage/src/tests/distributor/btree_bucket_database_test.cpp @@ -9,8 +9,8 @@ using namespace ::testing; namespace storage::distributor { -INSTANTIATE_TEST_CASE_P(BTreeDatabase, BucketDatabaseTest, - ::testing::Values(std::make_shared<BTreeBucketDatabase>())); +VESPA_GTEST_INSTANTIATE_TEST_SUITE_P(BTreeDatabase, BucketDatabaseTest, + ::testing::Values(std::make_shared<BTreeBucketDatabase>())); using document::BucketId; diff --git a/storage/src/tests/distributor/mapbucketdatabasetest.cpp b/storage/src/tests/distributor/mapbucketdatabasetest.cpp index 0ae4a49530e..2c000f6b5db 100644 --- a/storage/src/tests/distributor/mapbucketdatabasetest.cpp +++ b/storage/src/tests/distributor/mapbucketdatabasetest.cpp @@ -5,7 +5,7 @@ namespace storage::distributor { -INSTANTIATE_TEST_CASE_P(MapDatabase, BucketDatabaseTest, - ::testing::Values(std::make_shared<MapBucketDatabase>())); +VESPA_GTEST_INSTANTIATE_TEST_SUITE_P(MapDatabase, BucketDatabaseTest, + ::testing::Values(std::make_shared<MapBucketDatabase>())); } diff --git a/storageapi/src/tests/mbusprot/storageprotocoltest.cpp b/storageapi/src/tests/mbusprot/storageprotocoltest.cpp index 2e5eb115844..0f628f59aac 100644 --- a/storageapi/src/tests/mbusprot/storageprotocoltest.cpp +++ b/storageapi/src/tests/mbusprot/storageprotocoltest.cpp @@ -20,7 +20,7 @@ #include <iomanip> #include <sstream> -#include <gtest/gtest.h> +#include <vespa/vespalib/gtest/gtest.h> using namespace ::testing; @@ -105,11 +105,10 @@ std::string version_as_gtest_string(TestParamInfo<vespalib::Version> info) { } -// TODO replace with INSTANTIATE_TEST_SUITE_P on newer gtest versions -INSTANTIATE_TEST_CASE_P(MultiVersionTest, StorageProtocolTest, - Values(vespalib::Version(6, 240, 0), - vespalib::Version(7, 41, 19)), - version_as_gtest_string); +VESPA_GTEST_INSTANTIATE_TEST_SUITE_P(MultiVersionTest, StorageProtocolTest, + Values(vespalib::Version(6, 240, 0), + vespalib::Version(7, 41, 19)), + version_as_gtest_string); namespace { mbus::Message::UP lastCommand; diff --git a/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/FeederParams.java b/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/FeederParams.java index 98394a56694..c1e164f7fe8 100644 --- a/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/FeederParams.java +++ b/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/FeederParams.java @@ -34,7 +34,8 @@ class FeederParams { private boolean benchmarkMode = false; private int numDispatchThreads = 1; private int maxPending = 0; - private int numConnectionsPerTarget = 2; + private int numConnectionsPerTarget = 1; + private long numMessagesToSend = Long.MAX_VALUE; private List<InputStream> inputStreams = new ArrayList<>(); FeederParams() { @@ -84,10 +85,9 @@ class FeederParams { } int getNumConnectionsPerTarget() { return numConnectionsPerTarget; } - FeederParams setNumConnectionsPerTarget(int numConnectionsPerTarget) { - this.numConnectionsPerTarget = numConnectionsPerTarget; - return this; - } + + long getNumMessagesToSend() { return numMessagesToSend; } + boolean isSerialTransferEnabled() { return maxPending == 1; } @@ -116,6 +116,7 @@ class FeederParams { opts.addOption("b", "mode", true, "Mode for benchmarking."); opts.addOption("o", "output", true, "File to write to. Extensions gives format (.xml, .json, .vespa) json will be produced if no extension."); opts.addOption("c", "numconnections", true, "Number of connections per host."); + opts.addOption("l", "nummessages", true, "Number of messages to send (all is default)."); CommandLine cmd = new DefaultParser().parse(opts, args); @@ -142,6 +143,9 @@ class FeederParams { if (cmd.hasOption('s')) { setSerialTransfer(); } + if (cmd.hasOption('l')) { + numMessagesToSend = Long.valueOf(cmd.getOptionValue('l').trim()); + } if ( !cmd.getArgList().isEmpty()) { inputStreams.clear(); diff --git a/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/SimpleFeeder.java b/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/SimpleFeeder.java index 2925ea08de9..556d9bd60c7 100644 --- a/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/SimpleFeeder.java +++ b/vespa_feed_perf/src/main/java/com/yahoo/vespa/feed/perf/SimpleFeeder.java @@ -65,6 +65,7 @@ public class SimpleFeeder implements ReplyHandler { private final RPCMessageBus mbus; private final SourceSession session; private final int numThreads; + private final long numMessagesToSend; private final Destination destination; private final boolean benchmarkMode; private final static long REPORT_INTERVAL = TimeUnit.SECONDS.toMillis(10); @@ -81,18 +82,20 @@ public class SimpleFeeder implements ReplyHandler { private final Destination destination; private final FeedReader reader; private final Executor executor; - AtomicReference<Throwable> failure; + private final long messagesToSend; + private final AtomicReference<Throwable> failure; - Metrics(Destination destination, FeedReader reader, Executor executor, AtomicReference<Throwable> failure) { + Metrics(Destination destination, FeedReader reader, Executor executor, AtomicReference<Throwable> failure, long messagesToSend) { this.destination = destination; this.reader = reader; this.executor = executor; + this.messagesToSend = messagesToSend; this.failure = failure; } long feed() throws Throwable { long numMessages = 0; - while (failure.get() == null) { + while ((failure.get() == null) && (numMessages < messagesToSend)) { FeedOperation op = reader.read(); if (op.getType() == FeedOperation.Type.INVALID) { break; @@ -341,6 +344,7 @@ public class SimpleFeeder implements ReplyHandler { inputStreams = params.getInputStreams(); out = params.getStdOut(); numThreads = params.getNumDispatchThreads(); + numMessagesToSend = params.getNumMessagesToSend(); mbus = newMessageBus(docTypeMgr, params); session = newSession(mbus, this, params.getMaxPending()); docTypeMgr.configure(params.getConfigId()); @@ -380,7 +384,7 @@ public class SimpleFeeder implements ReplyHandler { printHeader(out); long numMessagesSent = 0; for (InputStream in : inputStreams) { - Metrics m = new Metrics(destination, createFeedReader(in), executor, failure); + Metrics m = new Metrics(destination, createFeedReader(in), executor, failure, numMessagesToSend); numMessagesSent += m.feed(); } while (failure.get() == null && numReplies.get() < numMessagesSent) { diff --git a/vespa_feed_perf/src/main/sh/vespa-feed-perf b/vespa_feed_perf/src/main/sh/vespa-feed-perf index 466cd2ee98c..d6ccf0e4fc5 100755 --- a/vespa_feed_perf/src/main/sh/vespa-feed-perf +++ b/vespa_feed_perf/src/main/sh/vespa-feed-perf @@ -74,4 +74,4 @@ findhost # END environment bootstrap section -exec java -jar $VESPA_HOME/lib/jars/vespa_feed_perf-jar-with-dependencies.jar "$@" +exec java -XX:+UseParallelGC -XX:ParallelGCThreads=4 -jar $VESPA_HOME/lib/jars/vespa_feed_perf-jar-with-dependencies.jar "$@" diff --git a/vespa_feed_perf/src/test/java/com/yahoo/vespa/feed/perf/FeederParamsTest.java b/vespa_feed_perf/src/test/java/com/yahoo/vespa/feed/perf/FeederParamsTest.java index 8682adc0935..13e307d9973 100644 --- a/vespa_feed_perf/src/test/java/com/yahoo/vespa/feed/perf/FeederParamsTest.java +++ b/vespa_feed_perf/src/test/java/com/yahoo/vespa/feed/perf/FeederParamsTest.java @@ -88,12 +88,19 @@ public class FeederParamsTest { } @Test public void requireThatNumConnectionsAreParsed() throws ParseException, FileNotFoundException { - assertEquals(2, new FeederParams().getNumConnectionsPerTarget()); - assertEquals(17, new FeederParams().parseArgs("-c 17").getNumConnectionsPerTarget()); + assertEquals(1, new FeederParams().getNumConnectionsPerTarget()); + assertEquals(16, new FeederParams().parseArgs("-c 16").getNumConnectionsPerTarget()); assertEquals(17, new FeederParams().parseArgs("--numconnections", "17").getNumConnectionsPerTarget()); } @Test + public void requireThatNumMessagesToSendAreParsed() throws ParseException, FileNotFoundException { + assertEquals(Long.MAX_VALUE, new FeederParams().getNumMessagesToSend()); + assertEquals(18, new FeederParams().parseArgs("-l 18").getNumMessagesToSend()); + assertEquals(19, new FeederParams().parseArgs("--nummessages", "19").getNumMessagesToSend()); + } + + @Test public void requireThatDumpStreamAreParsed() throws ParseException, IOException { assertNull(new FeederParams().getDumpStream()); diff --git a/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp b/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp index 88a5a05738b..e4631e28625 100644 --- a/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp +++ b/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp @@ -159,7 +159,7 @@ template <> std::vector<double> TestBase<DoubleUniqueStore>::values{ 10.0, 20.0, 30.0, 10.0 }; using UniqueStoreTestTypes = ::testing::Types<NumberUniqueStore, StringUniqueStore, CStringUniqueStore, DoubleUniqueStore>; -TYPED_TEST_CASE(TestBase, UniqueStoreTestTypes); +VESPA_GTEST_TYPED_TEST_SUITE(TestBase, UniqueStoreTestTypes); // Disable warnings emitted by gtest generated files when using typed tests #pragma GCC diagnostic push diff --git a/vespalib/src/vespa/vespalib/gtest/gtest.h b/vespalib/src/vespa/vespalib/gtest/gtest.h index e5bfcf2ae55..87362687103 100644 --- a/vespalib/src/vespa/vespalib/gtest/gtest.h +++ b/vespalib/src/vespa/vespalib/gtest/gtest.h @@ -14,3 +14,15 @@ main(int argc, char* argv[]) \ ::testing::InitGoogleTest(&argc, argv); \ return RUN_ALL_TESTS(); \ } + +#ifdef INSTANTIATE_TEST_SUITE_P +#define VESPA_GTEST_INSTANTIATE_TEST_SUITE_P INSTANTIATE_TEST_SUITE_P +#else +#define VESPA_GTEST_INSTANTIATE_TEST_SUITE_P INSTANTIATE_TEST_CASE_P +#endif + +#ifdef TYPED_TEST_SUITE +#define VESPA_GTEST_TYPED_TEST_SUITE TYPED_TEST_SUITE +#else +#define VESPA_GTEST_TYPED_TEST_SUITE TYPED_TEST_CASE +#endif |