diff options
105 files changed, 2115 insertions, 1015 deletions
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/document/SDField.java b/config-model/src/main/java/com/yahoo/searchdefinition/document/SDField.java index c4e0f4cafef..8b523211471 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/document/SDField.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/document/SDField.java @@ -463,33 +463,12 @@ public class SDField extends Field implements TypedKey, FieldOperationContainer, } } }.visit(indexingScript); - } else { - if (!getDataType().equals(PositionDataType.INSTANCE) && - !getDataType().equals(DataType.getArray(PositionDataType.INSTANCE)) && - hasAttributeExpression(exp)) - { - throw new IllegalArgumentException("For field '" + getName() + "': Setting attribute on a field that has struct or map sub-type(s) is not supported"); - } } for (SDField structField : getStructFields()) { structField.setIndexingScript(exp); } } - private static boolean hasAttributeExpression(ScriptExpression exp) { - var visitor = new ExpressionVisitor() { - boolean result = false; - @Override - protected void doVisit(Expression exp) { - if (exp instanceof AttributeExpression) { - result = true; - } - } - }; - visitor.visit(exp); - return visitor.result; - } - @Override public ScriptExpression getIndexingScript() { return indexingScript; diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java index a0c05193661..3ba3745f46e 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java @@ -49,36 +49,6 @@ public class ComplexAttributeFieldsValidatorTestCase { } @Test - public void throws_when_attribute_is_set_on_a_field_with_struct_sub_type() throws IOException, SAXException { - exceptionRule.expect(IllegalArgumentException.class); - exceptionRule.expectMessage("For field 'struct_array.f2': Setting attribute on a field that has struct or map sub-type(s) is not supported"); - createModelAndValidate(joinLines(createSearchDefintionWithInvalidStructFieldAttribute("array<s1>"))); - } - - @Test - public void throws_when_attribute_is_set_on_a_field_with_map_sub_type() throws IOException, SAXException { - exceptionRule.expect(IllegalArgumentException.class); - exceptionRule.expectMessage("For field 'struct_array.f2': Setting attribute on a field that has struct or map sub-type(s) is not supported"); - createModelAndValidate(joinLines(createSearchDefintionWithInvalidStructFieldAttribute("map<string, int>"))); - } - - private String createSearchDefintionWithInvalidStructFieldAttribute(String invalidFieldType) { - return joinLines("search test {", - " document test {", - " struct s1 {", - " field f1 type int {}", - " }", - " struct s2 {", - " field f2 type " + invalidFieldType + " {}", - " }", - " field struct_array type array<s2> {", - " struct-field f2 { indexing: attribute }", - " }", - " }", - "}"); - } - - @Test public void validation_passes_when_only_supported_struct_field_attributes_are_used() throws IOException, SAXException { createModelAndValidate(joinLines("search test {", " document test {", diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/RotationName.java b/config-provisioning/src/main/java/com/yahoo/config/provision/RotationName.java index 5d9ac3699b3..fb6d9dc09e6 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/RotationName.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/RotationName.java @@ -8,6 +8,7 @@ import java.util.Objects; * * @author mpolden */ +// TODO(mpolden): Remove this once all usages have been replaced public class RotationName implements Comparable<RotationName> { private final String name; diff --git a/configserver-flags/pom.xml b/configserver-flags/pom.xml index 8c96512c4c0..11ef9b6c950 100644 --- a/configserver-flags/pom.xml +++ b/configserver-flags/pom.xml @@ -20,6 +20,12 @@ <!-- provided --> <dependency> <groupId>com.yahoo.vespa</groupId> + <artifactId>container-dev</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> <artifactId>zkfacade</artifactId> <version>${project.version}</version> <scope>provided</scope> diff --git a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSource.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSource.java index 90709951dec..b1ffc05e70c 100644 --- a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSource.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSource.java @@ -5,7 +5,6 @@ import com.google.inject.Inject; import com.yahoo.vespa.configserver.flags.db.BootstrapFlagSource; import com.yahoo.vespa.configserver.flags.db.ZooKeeperFlagSource; import com.yahoo.vespa.flags.OrderedFlagSource; -import com.yahoo.vespa.flags.persistence.FlagsDb; import java.nio.file.FileSystem; import java.nio.file.FileSystems; diff --git a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/FlagsDb.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/FlagsDb.java new file mode 100644 index 00000000000..2c29ae0b818 --- /dev/null +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/FlagsDb.java @@ -0,0 +1,25 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.configserver.flags; + +import com.yahoo.vespa.flags.FlagId; +import com.yahoo.vespa.flags.json.FlagData; + +import java.util.Map; +import java.util.Optional; + +/** + * @author hakonhall + */ +public interface FlagsDb { + /** Get the String value of the flag. */ + Optional<FlagData> getValue(FlagId flagId); + + /** Set the String value of the flag. */ + void setValue(FlagId flagId, FlagData data); + + /** Remove the flag value if it exists. */ + void removeValue(FlagId flagId); + + /** Get all flags that have been set. */ + Map<FlagId, FlagData> getAllFlags(); +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/persistence/FlagsDb.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImpl.java index 2ed762f2895..5058358ba03 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/persistence/FlagsDb.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImpl.java @@ -1,8 +1,9 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.flags.persistence; +package com.yahoo.vespa.configserver.flags.db; import com.google.inject.Inject; import com.yahoo.path.Path; +import com.yahoo.vespa.configserver.flags.FlagsDb; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.flags.FlagId; import com.yahoo.vespa.flags.json.FlagData; @@ -19,15 +20,14 @@ import java.util.stream.Collectors; /** * @author hakonhall */ -public class FlagsDb { - +public class FlagsDbImpl implements FlagsDb { private static final Path ROOT_PATH = Path.fromString("/flags/v1"); private final Curator curator; private final Curator.DirectoryCache cache; @Inject - public FlagsDb(Curator curator) { + public FlagsDbImpl(Curator curator) { this.curator = curator; curator.create(ROOT_PATH); ExecutorService executorService = Executors.newFixedThreadPool(1); @@ -35,28 +35,28 @@ public class FlagsDb { cache.start(); } - /** Get the String value of the flag. */ + @Override public Optional<FlagData> getValue(FlagId flagId) { return Optional.ofNullable(cache.getCurrentData(getZkPathFor(flagId))) - .map(ChildData::getData) - .map(FlagData::deserializeUtf8Json); + .map(ChildData::getData) + .map(FlagData::deserializeUtf8Json); } - /** Set the String value of the flag. */ + @Override public void setValue(FlagId flagId, FlagData data) { curator.set(getZkPathFor(flagId), data.serializeToUtf8Json()); } - /** Get all flags that have been set. */ + @Override public Map<FlagId, FlagData> getAllFlags() { List<ChildData> dataList = cache.getCurrentData(); return dataList.stream() - .map(ChildData::getData) - .map(FlagData::deserializeUtf8Json) - .collect(Collectors.toMap(FlagData::id, Function.identity())); + .map(ChildData::getData) + .map(FlagData::deserializeUtf8Json) + .collect(Collectors.toMap(FlagData::id, Function.identity())); } - /** Remove the flag value if it exists. */ + @Override public void removeValue(FlagId flagId) { curator.delete(getZkPathFor(flagId)); } @@ -64,5 +64,4 @@ public class FlagsDb { private static Path getZkPathFor(FlagId flagId) { return ROOT_PATH.append(flagId.toString()); } - } diff --git a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/ZooKeeperFlagSource.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/ZooKeeperFlagSource.java index e6ab3f5b387..4a9d604b4bd 100644 --- a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/ZooKeeperFlagSource.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/ZooKeeperFlagSource.java @@ -1,7 +1,7 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.configserver.flags.db; -import com.yahoo.vespa.flags.persistence.FlagsDb; +import com.yahoo.vespa.configserver.flags.FlagsDb; import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagId; import com.yahoo.vespa.flags.FlagSource; diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/DefinedFlag.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/DefinedFlag.java index 8234e9df725..c706a2b1e51 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/http/DefinedFlag.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/DefinedFlag.java @@ -1,5 +1,5 @@ // Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.flags.http; +package com.yahoo.vespa.configserver.flags.http; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ArrayNode; diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/DefinedFlags.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/DefinedFlags.java index e1db7dda6e0..26d590593c0 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/http/DefinedFlags.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/DefinedFlags.java @@ -1,5 +1,5 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.flags.http; +package com.yahoo.vespa.configserver.flags.http; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/ErrorResponse.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/ErrorResponse.java index 969903093a4..b9e5c75fe22 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/http/ErrorResponse.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/ErrorResponse.java @@ -1,5 +1,5 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.flags.http; +package com.yahoo.vespa.configserver.flags.http; import com.yahoo.slime.Cursor; import com.yahoo.slime.Slime; diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/FlagDataListResponse.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/FlagDataListResponse.java index 5af97007997..efc78cb7930 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/http/FlagDataListResponse.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/FlagDataListResponse.java @@ -1,5 +1,5 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.flags.http; +package com.yahoo.vespa.configserver.flags.http; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ArrayNode; diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/FlagDataResponse.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/FlagDataResponse.java index f6e81e030c7..8ff4085df8d 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/http/FlagDataResponse.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/FlagDataResponse.java @@ -1,5 +1,5 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.flags.http; +package com.yahoo.vespa.configserver.flags.http; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.jdisc.Response; diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/FlagsHandler.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/FlagsHandler.java index 76f74cbe931..40bb69111e0 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/http/FlagsHandler.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/FlagsHandler.java @@ -1,5 +1,5 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.flags.http; +package com.yahoo.vespa.configserver.flags.http; import com.google.inject.Inject; import com.yahoo.container.jdisc.HttpRequest; @@ -7,11 +7,11 @@ import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.LoggingRequestHandler; import com.yahoo.log.LogLevel; import com.yahoo.restapi.Path; +import com.yahoo.vespa.configserver.flags.FlagsDb; import com.yahoo.vespa.flags.FlagDefinition; import com.yahoo.vespa.flags.FlagId; import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.json.FlagData; -import com.yahoo.vespa.flags.persistence.FlagsDb; import com.yahoo.yolean.Exceptions; import java.io.UncheckedIOException; @@ -90,7 +90,7 @@ public class FlagsHandler extends LoggingRequestHandler { private HttpResponse getFlagDataList(HttpRequest request) { return new FlagDataListResponse(flagsV1Uri(request), flagsDb.getAllFlags(), - Objects.equals(request.getProperty("recursive"), "true")); + Objects.equals(request.getProperty("recursive"), "true")); } private HttpResponse getFlagData(FlagId flagId) { diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/OKResponse.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/OKResponse.java index d094e2d5734..f41940f692b 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/http/OKResponse.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/OKResponse.java @@ -1,5 +1,5 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.flags.http; +package com.yahoo.vespa.configserver.flags.http; import com.yahoo.container.jdisc.EmptyResponse; import com.yahoo.jdisc.Response; diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/SlimeJsonResponse.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/SlimeJsonResponse.java index dd71795ae43..e5568514894 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/http/SlimeJsonResponse.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/SlimeJsonResponse.java @@ -1,5 +1,5 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.flags.http; +package com.yahoo.vespa.configserver.flags.http; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.slime.JsonFormat; diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/V1Response.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/V1Response.java index e8ff0bd99a4..ac1e9514700 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/http/V1Response.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/V1Response.java @@ -1,5 +1,5 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.flags.http; +package com.yahoo.vespa.configserver.flags.http; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.jdisc.Response; diff --git a/flags/src/main/java/com/yahoo/vespa/flags/persistence/package-info.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/package-info.java index d4753ed1756..87b63114b73 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/persistence/package-info.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/package-info.java @@ -3,6 +3,6 @@ * @author mpolden */ @ExportPackage -package com.yahoo.vespa.flags.persistence; +package com.yahoo.vespa.configserver.flags.http; import com.yahoo.osgi.annotation.ExportPackage; diff --git a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/package-info.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/package-info.java index 97e66d95715..d6f078326a3 100644 --- a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/package-info.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/package-info.java @@ -3,5 +3,3 @@ package com.yahoo.vespa.configserver.flags; import com.yahoo.osgi.annotation.ExportPackage; - -/** The node repository controls and allocates the nodes available in a hosted Vespa zone */ diff --git a/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSourceTest.java b/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSourceTest.java index b2f891326fc..c46677bfc10 100644 --- a/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSourceTest.java +++ b/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSourceTest.java @@ -7,7 +7,6 @@ import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagId; import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.RawFlag; -import com.yahoo.vespa.flags.persistence.FlagsDb; import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.After; import org.junit.Before; diff --git a/flags/src/test/java/com/yahoo/vespa/flags/persistence/FlagsDbTest.java b/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImplTest.java index 5102305af90..ecc9bacb081 100644 --- a/flags/src/test/java/com/yahoo/vespa/flags/persistence/FlagsDbTest.java +++ b/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImplTest.java @@ -1,5 +1,5 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.flags.persistence; +package com.yahoo.vespa.configserver.flags.db; import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.flags.FetchVector; @@ -23,11 +23,11 @@ import static org.junit.Assert.assertTrue; /** * @author hakonhall */ -public class FlagsDbTest { +public class FlagsDbImplTest { @Test public void test() { MockCurator curator = new MockCurator(); - FlagsDb db = new FlagsDb(curator); + FlagsDbImpl db = new FlagsDbImpl(curator); Condition condition1 = new Condition(Condition.Type.WHITELIST, FetchVector.Dimension.HOSTNAME, "host1"); Rule rule1 = new Rule(Optional.of(JsonNodeRawFlag.fromJson("13")), condition1); @@ -39,8 +39,8 @@ public class FlagsDbTest { assertTrue(dataCopy.isPresent()); assertEquals("{\"id\":\"id\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\"," + - "\"values\":[\"host1\"]}],\"value\":13}],\"attributes\":{\"zone\":\"zone-a\"}}", - dataCopy.get().serializeToJson()); + "\"values\":[\"host1\"]}],\"value\":13}],\"attributes\":{\"zone\":\"zone-a\"}}", + dataCopy.get().serializeToJson()); FlagId flagId2 = new FlagId("id2"); FlagData data2 = new FlagData(flagId2, new FetchVector().with(FetchVector.Dimension.ZONE_ID, "zone-a"), rule1); @@ -53,4 +53,4 @@ public class FlagsDbTest { db.removeValue(flagId2); assertFalse(db.getValue(flagId2).isPresent()); } -} +}
\ No newline at end of file diff --git a/flags/src/test/java/com/yahoo/vespa/flags/http/FlagsHandlerTest.java b/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/http/FlagsHandlerTest.java index 8ae1008ba22..cbd37c8a5cf 100644 --- a/flags/src/test/java/com/yahoo/vespa/flags/http/FlagsHandlerTest.java +++ b/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/http/FlagsHandlerTest.java @@ -1,16 +1,17 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.flags.http; +package com.yahoo.vespa.configserver.flags.http; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.jdisc.http.HttpRequest.Method; import com.yahoo.text.Utf8; +import com.yahoo.vespa.configserver.flags.FlagsDb; +import com.yahoo.vespa.configserver.flags.db.FlagsDbImpl; import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagId; import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.UnboundBooleanFlag; -import com.yahoo.vespa.flags.persistence.FlagsDb; import com.yahoo.yolean.Exceptions; import org.junit.Test; @@ -38,7 +39,7 @@ public class FlagsHandlerTest { private static final String FLAGS_V1_URL = "https://foo.com:4443/flags/v1"; - private final FlagsDb flagsDb = new FlagsDb(new MockCurator()); + private final FlagsDb flagsDb = new FlagsDbImpl(new MockCurator()); private final FlagsHandler handler = new FlagsHandler(FlagsHandler.testOnlyContext(), flagsDb); @Test diff --git a/configserver/pom.xml b/configserver/pom.xml index f346cde63a3..fd33950a546 100644 --- a/configserver/pom.xml +++ b/configserver/pom.xml @@ -185,6 +185,12 @@ <scope>compile</scope> </dependency> <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>jaxrs_client_utils</artifactId> + <version>${project.version}</version> + <scope>compile</scope> <!-- TODO Should ideally be provided, but this bundle is not installed as part of configserver. Orchestrator bundle also includes jaxrs_client_utils in compile scope --> + </dependency> + <dependency> <groupId>com.fasterxml.jackson.core</groupId> <artifactId>jackson-databind</artifactId> <scope>provided</scope> diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index 192488c11ab..f65eaaf3fa3 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -42,7 +42,11 @@ import com.yahoo.vespa.config.server.deploy.Deployment; import com.yahoo.vespa.config.server.deploy.InfraDeployerProvider; import com.yahoo.vespa.config.server.http.LogRetriever; import com.yahoo.vespa.config.server.http.SimpleHttpFetcher; +import com.yahoo.vespa.config.server.http.v2.MetricsResponse; import com.yahoo.vespa.config.server.http.v2.PrepareResult; +import com.yahoo.vespa.config.server.metrics.ClusterInfo; +import com.yahoo.vespa.config.server.metrics.MetricsAggregator; +import com.yahoo.vespa.config.server.metrics.MetricsRetriever; import com.yahoo.vespa.config.server.provision.HostProvisionerProvider; import com.yahoo.vespa.config.server.session.LocalSession; import com.yahoo.vespa.config.server.session.LocalSessionRepo; @@ -67,10 +71,14 @@ import java.nio.file.attribute.BasicFileAttributes; import java.time.Clock; import java.time.Duration; import java.time.Instant; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.logging.Level; @@ -80,6 +88,7 @@ import java.util.stream.Collectors; import static com.yahoo.config.model.api.container.ContainerServiceType.CLUSTERCONTROLLER_CONTAINER; import static com.yahoo.config.model.api.container.ContainerServiceType.CONTAINER; import static com.yahoo.config.model.api.container.ContainerServiceType.LOGSERVER_CONTAINER; +import static com.yahoo.config.model.api.container.ContainerServiceType.METRICS_PROXY_CONTAINER; import static com.yahoo.vespa.config.server.tenant.TenantRepository.HOSTED_VESPA_TENANT; import static java.nio.file.Files.readAttributes; @@ -460,7 +469,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye Tenant tenant = tenantRepository.getTenant(applicationId.tenant()); if (tenant == null) throw new NotFoundException("Tenant '" + applicationId.tenant() + "' not found"); long sessionId = getSessionIdForApplication(tenant, applicationId); - RemoteSession session = tenant.getRemoteSessionRepo().getSession(sessionId, 0); + RemoteSession session = tenant.getRemoteSessionRepo().getSession(sessionId); return session.ensureApplicationLoaded().getForVersionOrLatest(version, clock.instant()); } catch (NotFoundException e) { log.log(LogLevel.WARNING, "Failed getting application for '" + applicationId + "': " + e.getMessage()); @@ -633,6 +642,21 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye return tenantRepository.getTenant(tenantName).getApplicationRepo().activeApplications(); } + // ---------------- Metrics ------------------------------------------------------------------------ + + public MetricsResponse getMetrics(ApplicationId applicationId) { + var metricsRetriever = new MetricsRetriever(); + var clusters = getClustersOfApplication(applicationId); + var clusterMetrics = new LinkedHashMap<ClusterInfo, MetricsAggregator>(); + + clusters.forEach(cluster -> { + var metrics = metricsRetriever.requestMetricsForCluster(cluster); + clusterMetrics.put(cluster, metrics); + }); + + return new MetricsResponse(200, applicationId, clusterMetrics); + } + // ---------------- Misc operations ---------------------------------------------------------------- public ApplicationMetaData getMetadataFromSession(Tenant tenant, long sessionId) { @@ -750,18 +774,41 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye .anyMatch(serviceInfo -> serviceInfo.getServiceType().equalsIgnoreCase("logserver"))) .findFirst().orElseThrow(() -> new IllegalArgumentException("Could not find HostInfo for LogServer")); - ServiceInfo containerServiceInfo = logServerHostInfo.getServices().stream() - .filter(service -> List.of(LOGSERVER_CONTAINER.serviceName, CONTAINER.serviceName).contains(service.getServiceType())) + ServiceInfo serviceInfo = logServerHostInfo.getServices().stream().filter(service -> List.of(LOGSERVER_CONTAINER.serviceName, CONTAINER.serviceName).contains(service.getServiceType())) .findFirst().orElseThrow(() -> new IllegalArgumentException("No container running on logserver host")); + int port = servicePort(serviceInfo); + return "http://" + logServerHostInfo.getHostname() + ":" + port + "/logs"; + } - int port = containerServiceInfo.getPorts().stream() + private int servicePort(ServiceInfo serviceInfo) { + int port = serviceInfo.getPorts().stream() .filter(portInfo -> portInfo.getTags().stream().anyMatch(tag -> tag.equalsIgnoreCase("http"))) .findFirst().orElseThrow(() -> new IllegalArgumentException("Could not find HTTP port")) .getPort(); - - return "http://" + logServerHostInfo.getHostname() + ":" + port + "/logs"; + return port; } + /** Finds the hosts of an application, grouped by cluster name */ + private Collection<ClusterInfo> getClustersOfApplication(ApplicationId applicationId) { + Application application = getApplication(applicationId); + Map<String, List<URI>> clusterHosts = new HashMap<>(); + Map<String, ClusterInfo> clusters = new HashMap<>(); + application.getModel().getHosts().stream() + .filter(host -> host.getServices().stream().noneMatch(serviceInfo -> serviceInfo.getServiceType().equalsIgnoreCase("logserver"))) + .forEach(hostInfo -> { + ServiceInfo serviceInfo = hostInfo.getServices().stream().filter(service -> METRICS_PROXY_CONTAINER.serviceName.equals(service.getServiceType())) + .findFirst().orElseThrow(() -> new IllegalArgumentException("Unable to find services " + METRICS_PROXY_CONTAINER.serviceName.toString())); + String clusterName = serviceInfo.getProperty("clusterid").orElse(""); + String clusterTypeString = serviceInfo.getProperty("clustertype").orElse(""); + ClusterInfo.ClusterType clusterType = ClusterInfo.ClusterType.valueOf(clusterTypeString); + URI host = URI.create("http://" + hostInfo.getHostname() + ":" + servicePort(serviceInfo) + "/metrics/v1/values"); + clusterHosts.computeIfAbsent(clusterName, l -> new ArrayList<URI>()).add(host); + clusters.computeIfAbsent(clusterName, c -> new ClusterInfo(clusterName, clusterType)).addHost(host); + } + ); + return clusters.values(); + + } /** Returns version to use when deploying application in given environment */ static Version decideVersion(ApplicationId application, Environment environment, Version sessionVersion, boolean bootstrap) { if ( environment.isManuallyDeployed() diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ConfigConvergenceChecker.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ConfigConvergenceChecker.java index d55e07540d6..d0f8005ace1 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ConfigConvergenceChecker.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ConfigConvergenceChecker.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.application; +import ai.vespa.util.http.VespaClientBuilderFactory; import com.fasterxml.jackson.databind.JsonNode; import com.google.inject.Inject; import com.yahoo.component.AbstractComponent; @@ -17,8 +18,9 @@ import javax.ws.rs.GET; import javax.ws.rs.Path; import javax.ws.rs.ProcessingException; import javax.ws.rs.client.Client; -import javax.ws.rs.client.ClientBuilder; +import javax.ws.rs.client.ClientRequestFilter; import javax.ws.rs.client.WebTarget; +import javax.ws.rs.core.HttpHeaders; import java.net.URI; import java.time.Duration; import java.util.ArrayList; @@ -55,6 +57,7 @@ public class ConfigConvergenceChecker extends AbstractComponent { ); private final StateApiFactory stateApiFactory; + private final VespaClientBuilderFactory clientBuilderFactory = new VespaClientBuilderFactory(); @Inject public ConfigConvergenceChecker() { @@ -97,6 +100,11 @@ public class ConfigConvergenceChecker extends AbstractComponent { } } + @Override + public void deconstruct() { + clientBuilderFactory.close(); + } + @Path(statePath) public interface StateApi { @Path(configSubPath) @@ -152,8 +160,11 @@ public class ConfigConvergenceChecker extends AbstractComponent { return false; } - private static Client createClient(Duration timeout) { - return ClientBuilder.newBuilder() + private Client createClient(Duration timeout) { + return clientBuilderFactory.newBuilder() + .register( + (ClientRequestFilter) ctx -> + ctx.getHeaders().put(HttpHeaders.USER_AGENT, List.of("config-convergence-checker"))) .property(ClientProperties.CONNECT_TIMEOUT, (int) timeout.toMillis()) .property(ClientProperties.READ_TIMEOUT, (int) timeout.toMillis()) .build(); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java index 865805d1258..e18c6ad6c56 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java @@ -104,6 +104,10 @@ public class ApplicationHandler extends HttpHandler { return applicationRepository.getLogs(applicationId, hostname, apiParams); } + if (isMetricsRequest(request)) { + return applicationRepository.getMetrics(applicationId); + } + if (isIsSuspendedRequest(request)) { return new ApplicationSuspendedResponse(applicationRepository.isSuspended(applicationId)); } @@ -144,6 +148,7 @@ public class ApplicationHandler extends HttpHandler { "http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/serviceconverge", "http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/serviceconverge/*", "http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/clustercontroller/*/status/*", + "http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/metrics", "http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*", "http://*/application/v2/tenant/*/application/*/logs", "http://*/application/v2/tenant/*/application/*"); @@ -154,6 +159,11 @@ public class ApplicationHandler extends HttpHandler { request.getUri().getPath().endsWith("/suspended"); } + private static boolean isMetricsRequest(HttpRequest request) { + return getBindingMatch(request).groupCount() == 7 && + request.getUri().getPath().endsWith("/metrics"); + } + private static boolean isLogRequest(HttpRequest request) { return getBindingMatch(request).groupCount() == 4 && request.getUri().getPath().endsWith("/logs"); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/MetricsResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/MetricsResponse.java new file mode 100644 index 00000000000..88971433a01 --- /dev/null +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/MetricsResponse.java @@ -0,0 +1,56 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.config.server.http.v2; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.JsonFormat; +import com.yahoo.slime.Slime; +import com.yahoo.vespa.config.server.http.HttpConfigResponse; +import com.yahoo.vespa.config.server.metrics.ClusterInfo; +import com.yahoo.vespa.config.server.metrics.MetricsAggregator; + +import java.io.IOException; +import java.io.OutputStream; +import java.util.Map; + +/** + * @author olaa + */ +public class MetricsResponse extends HttpResponse { + + private final Slime slime = new Slime(); + + public MetricsResponse(int status, ApplicationId applicationId, Map<ClusterInfo, MetricsAggregator> aggregatedMetrics) { + super(status); + + Cursor application = slime.setObject(); + application.setString("applicationId", applicationId.serializedForm()); + + Cursor clusters = application.setArray("clusters"); + + for (var entry : aggregatedMetrics.entrySet()) { + Cursor cluster = clusters.addObject(); + cluster.setString("clusterId", entry.getKey().getClusterId()); + cluster.setString("clusterType", entry.getKey().getClusterType().name()); + + MetricsAggregator aggregator = entry.getValue(); + Cursor metrics = cluster.setObject("metrics"); + aggregator.aggregateQueryRate().ifPresent(queryrate -> metrics.setDouble("queriesPerSecond", queryrate)); + aggregator.aggregateFeedRate().ifPresent(feedRate -> metrics.setDouble("feedPerSecond", feedRate)); + aggregator.aggregateDocumentCount().ifPresent(documentCount -> metrics.setDouble("documentCount", documentCount)); + aggregator.aggregateQueryLatency().ifPresent(queryLatency -> metrics.setDouble("queryLatency",queryLatency)); + aggregator.aggregateFeedLatency().ifPresent(feedLatency -> metrics.setDouble("feedLatency", feedLatency)); + } + } + + @Override + public void render(OutputStream outputStream) throws IOException { + new JsonFormat(false).encode(outputStream, slime); + } + + @Override + public String getContentType() { + return HttpConfigResponse.JSON_CONTENT_TYPE; + } +} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterInfo.java b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterInfo.java new file mode 100644 index 00000000000..ef9a73fedd4 --- /dev/null +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterInfo.java @@ -0,0 +1,44 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.config.server.metrics; + +import java.net.URI; +import java.util.ArrayList; +import java.util.List; + +/** + * @author olaa + */ +public class ClusterInfo { + + private final String clusterId; + private final ClusterType clusterType; + private final List<URI> hostnames; + + public ClusterInfo(String clusterId, ClusterType clusterType) { + this(clusterId, clusterType, new ArrayList<>()); + } + + public ClusterInfo(String clusterId, ClusterType clusterType, List<URI> hostnames) { + this.clusterId = clusterId; + this.clusterType = clusterType; + this.hostnames = hostnames; + } + + public String getClusterId() { + return clusterId; + } + + public ClusterType getClusterType() { + return clusterType; + } + + public List<URI> getHostnames() { + return hostnames; + } + + public void addHost(URI host) { + hostnames.add(host); + } + + public enum ClusterType {content, container}; +} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsAggregator.java b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsAggregator.java new file mode 100644 index 00000000000..c6b2131863d --- /dev/null +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsAggregator.java @@ -0,0 +1,86 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.config.server.metrics; + +import java.time.Instant; +import java.util.Optional; + +/** + * @author olaa + * @author ogronnesby + */ +public class MetricsAggregator { + + private LatencyMetrics feed; + private LatencyMetrics qr; + private LatencyMetrics container; + private Double documentCount; + private Instant timestamp; + + public MetricsAggregator addFeedLatency(double sum, double count) { + this.feed = combineLatency(this.feed, sum, count); + return this; + } + + public MetricsAggregator addQrLatency(double sum, double count) { + this.qr = combineLatency(this.qr, sum, count); + return this; + } + + public MetricsAggregator addContainerLatency(double sum, double count) { + this.container = combineLatency(this.container, sum, count); + return this; + } + + public MetricsAggregator addDocumentCount(double count) { + this.documentCount = (this.documentCount == null ? 0.0 : this.documentCount) + count; + return this; + } + + public MetricsAggregator setTimestamp(Instant timestamp) { + this.timestamp = timestamp; + return this; + } + + public Optional<Double> aggregateFeedLatency() { + return Optional.ofNullable(feed).map(m -> m.latencySum / m.latencyCount); + + } + + public Optional<Double> aggregateFeedRate() { + return Optional.ofNullable(feed).map(m -> m.latencyCount / 60); + } + + public Optional<Double> aggregateQueryLatency() { + if (container == null && qr == null) return Optional.empty(); + var c = Optional.ofNullable(container).orElseGet(LatencyMetrics::new); + var q = Optional.ofNullable(qr).orElseGet(LatencyMetrics::new); + return Optional.of((c.latencySum + q.latencySum) / (c.latencyCount + q.latencyCount)); + } + + public Optional<Double> aggregateQueryRate() { + if (container == null && qr == null) return Optional.empty(); + var c = Optional.ofNullable(container).orElseGet(LatencyMetrics::new); + var q = Optional.ofNullable(qr).orElseGet(LatencyMetrics::new); + return Optional.of((c.latencyCount + q.latencyCount) / 60); + } + + public Optional<Double> aggregateDocumentCount() { + return Optional.ofNullable(documentCount); + } + + public Instant getTimestamp() { + return timestamp; + } + + private LatencyMetrics combineLatency(LatencyMetrics metricsOrNull, double sum, double count) { + var metrics = Optional.ofNullable(metricsOrNull).orElseGet(LatencyMetrics::new); + metrics.latencyCount += count; + metrics.latencySum += sum; + return metrics; + } + + private static class LatencyMetrics { + double latencySum; + double latencyCount; + } +} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsRetriever.java b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsRetriever.java new file mode 100644 index 00000000000..0881d32b21e --- /dev/null +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsRetriever.java @@ -0,0 +1,89 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.config.server.metrics; + +import com.yahoo.slime.ArrayTraverser; +import com.yahoo.slime.Inspector; +import com.yahoo.slime.Slime; +import com.yahoo.vespa.config.SlimeUtils; +import org.apache.http.HttpResponse; +import org.apache.http.client.HttpClient; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.impl.client.HttpClientBuilder; + +import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; +import java.net.URI; +import java.time.Instant; + + +/** + * Client for reaching out to nodes in an application instance and get their + * metrics. + * + * @author olaa + * @author ogronnesby + */ +public class MetricsRetriever { + private final HttpClient httpClient = HttpClientBuilder.create().build(); + + /** + * Call the metrics API on each host in the cluster and aggregate the metrics + * into a single value. + */ + public MetricsAggregator requestMetricsForCluster(ClusterInfo clusterInfo) { + var aggregator = new MetricsAggregator(); + clusterInfo.getHostnames().forEach(host -> getHostMetrics(host, aggregator)); + return aggregator; + } + + private void getHostMetrics(URI hostURI, MetricsAggregator metrics) { + Slime responseBody = doMetricsRequest(hostURI); + Inspector services = responseBody.get().field("services"); + services.traverse((ArrayTraverser) (i, servicesInspector) -> { + parseService(servicesInspector, metrics); + }); + } + + private Slime doMetricsRequest(URI hostURI) { + HttpGet get = new HttpGet(hostURI); + try { + HttpResponse response = httpClient.execute(get); + InputStream is = response.getEntity().getContent(); + Slime slime = SlimeUtils.jsonToSlime(is.readAllBytes()); + is.close(); + return slime; + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + private void parseService(Inspector service, MetricsAggregator metrics) { + String serviceName = service.field("name").asString(); + Instant timestamp = Instant.ofEpochSecond(service.field("timestamp").asLong()); + metrics.setTimestamp(timestamp); + service.field("metrics").traverse((ArrayTraverser) (i, m) -> { + Inspector values = m.field("values"); + switch (serviceName) { + case "container": + metrics.addContainerLatency( + values.field("query_latency.sum").asDouble(), + values.field("query_latency.count").asDouble()); + metrics.addFeedLatency( + values.field("feed_latency.sum").asDouble(), + values.field("feed_latency.count").asDouble()); + break; + case "qrserver": + metrics.addQrLatency( + values.field("query_latency.sum").asDouble(), + values.field("query_latency.count").asDouble()); + break; + case "distributor": + metrics.addDocumentCount(values.field("vds.distributor.docsstored.average").asDouble()); + break; + } + }); + + } + +} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepo.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepo.java index 415ff268309..3400504fb58 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepo.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepo.java @@ -1,14 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.session; -import com.yahoo.transaction.AbstractTransaction; -import com.yahoo.transaction.NestedTransaction; -import com.yahoo.transaction.Transaction; -import com.yahoo.vespa.config.server.TimeoutBudget; -import com.yahoo.vespa.config.server.NotFoundException; - -import java.time.Clock; -import java.time.Duration; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -29,16 +21,10 @@ public class SessionRepo<SESSIONTYPE extends Session> { sessions.put(session.getSessionId(), session); } - public synchronized SESSIONTYPE removeSession(long id) { + synchronized void removeSession(long id) { if ( ! sessions.containsKey(id)) throw new IllegalArgumentException("No session with id '" + id + "' exists"); - return sessions.remove(id); - } - - public void removeSession(long id, NestedTransaction nestedTransaction) { - SessionRepoTransaction transaction = new SessionRepoTransaction(); - transaction.addRemoveOperation(id); - nestedTransaction.add(transaction); + sessions.remove(id); } /** @@ -51,90 +37,8 @@ public class SessionRepo<SESSIONTYPE extends Session> { return sessions.get(id); } - /** - * Gets a Session with a timeout - * - * @param id session id - * @param timeoutInMillis timeout for getting session (loops and wait for session to show up if not found) - * @return a session belonging to the id supplied, or null if no session with the id was found - */ - public synchronized SESSIONTYPE getSession(long id, long timeoutInMillis) { - try { - return internalGetSession(id, timeoutInMillis); - } catch (InterruptedException e) { - throw new RuntimeException("Interrupted while retrieving session with id " + id); - } - } - - private synchronized SESSIONTYPE internalGetSession(long id, long timeoutInMillis) throws InterruptedException { - TimeoutBudget timeoutBudget = new TimeoutBudget(Clock.systemUTC(), Duration.ofMillis(timeoutInMillis)); - do { - SESSIONTYPE session = getSession(id); - if (session != null) { - return session; - } - wait(100); - } while (timeoutBudget.hasTimeLeft()); - throw new NotFoundException("Unable to retrieve session with id " + id + " before timeout was reached"); - } - public synchronized Collection<SESSIONTYPE> listSessions() { return new ArrayList<>(sessions.values()); } - public class SessionRepoTransaction extends AbstractTransaction { - - void addRemoveOperation(long sessionIdToRemove) { - add(new RemoveOperation(sessionIdToRemove)); - } - - @Override - public void prepare() { } - - @Override - @SuppressWarnings("unchecked") - public void commit() { - for (Operation operation : operations()) - ((SessionOperation)operation).commit(); - } - - @Override - @SuppressWarnings("unchecked") - public void rollbackOrLog() { - for (Operation operation : operations()) - ((SessionOperation)operation).rollback(); - } - - abstract class SessionOperation implements Transaction.Operation { - - abstract void commit(); - - abstract void rollback(); - - } - - public class RemoveOperation extends SessionOperation { - - private final long sessionIdToRemove; - private SESSIONTYPE removed = null; - - RemoveOperation(long sessionIdToRemove) { - this.sessionIdToRemove = sessionIdToRemove; - } - - @Override - public void commit() { - removed = removeSession(sessionIdToRemove); - } - - @Override - public void rollback() { - if (removed != null) - addSession(removed); - } - - } - - } - } diff --git a/configserver/src/main/java/com/yahoo/vespa/serviceview/ConfigServerLocation.java b/configserver/src/main/java/com/yahoo/vespa/serviceview/ConfigServerLocation.java index 5dcdfcdaf37..cc452421d2d 100644 --- a/configserver/src/main/java/com/yahoo/vespa/serviceview/ConfigServerLocation.java +++ b/configserver/src/main/java/com/yahoo/vespa/serviceview/ConfigServerLocation.java @@ -1,20 +1,28 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.serviceview; +import ai.vespa.util.http.VespaClientBuilderFactory; +import com.google.inject.Inject; import com.yahoo.cloud.config.ConfigserverConfig; +import com.yahoo.component.AbstractComponent; /** * Wrapper for settings from the cloud.config.configserver config. * - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen */ -public class ConfigServerLocation { - public final int restApiPort; +public class ConfigServerLocation extends AbstractComponent { + final int restApiPort; + // The client factory must be owned by a component as StateResource is instantiated per request + final VespaClientBuilderFactory clientBuilderFactory = new VespaClientBuilderFactory(); + + @Inject public ConfigServerLocation(ConfigserverConfig configServer) { restApiPort = configServer.httpport(); } + @Override public String toString() { StringBuilder builder = new StringBuilder(); @@ -22,4 +30,8 @@ public class ConfigServerLocation { return builder.toString(); } + @Override + public void deconstruct() { + clientBuilderFactory.close(); + } } diff --git a/configserver/src/main/java/com/yahoo/vespa/serviceview/StateResource.java b/configserver/src/main/java/com/yahoo/vespa/serviceview/StateResource.java index a6d4c229500..c58f3659ca5 100644 --- a/configserver/src/main/java/com/yahoo/vespa/serviceview/StateResource.java +++ b/configserver/src/main/java/com/yahoo/vespa/serviceview/StateResource.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.serviceview; +import ai.vespa.util.http.VespaClientBuilderFactory; import com.yahoo.container.jaxrs.annotation.Component; import com.yahoo.vespa.serviceview.bindings.ApplicationView; import com.yahoo.vespa.serviceview.bindings.ConfigClient; @@ -14,7 +15,6 @@ import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.Produces; import javax.ws.rs.client.Client; -import javax.ws.rs.client.ClientBuilder; import javax.ws.rs.client.ClientRequestFilter; import javax.ws.rs.client.WebTarget; import javax.ws.rs.core.Context; @@ -28,8 +28,6 @@ import java.util.List; import java.util.ListIterator; import java.util.Map; -import static java.util.Collections.singletonList; - /** * A web service to discover and proxy Vespa service state info. @@ -42,6 +40,7 @@ public class StateResource implements StateClient { private static final String USER_AGENT = "service-view-config-server-client"; private static final String SINGLE_API_LINK = "url"; + private final VespaClientBuilderFactory clientBuilderFactory; private final int restApiPort; private final String host; private final UriInfo uriInfo; @@ -58,6 +57,7 @@ public class StateResource implements StateClient { } public StateResource(@Component ConfigServerLocation configServer, @Context UriInfo ui) { + this.clientBuilderFactory = configServer.clientBuilderFactory; this.restApiPort = configServer.restApiPort; this.host = "localhost"; this.uriInfo = ui; @@ -278,11 +278,9 @@ public class StateResource implements StateClient { newUri.append(link.getRawPath()); } - private static Client client() { - return ClientBuilder.newBuilder() - .register((ClientRequestFilter) ctx -> ctx.getHeaders().put(HttpHeaders.USER_AGENT, - singletonList(USER_AGENT))) + private Client client() { + return clientBuilderFactory.newBuilder() + .register((ClientRequestFilter) ctx -> ctx.getHeaders().put(HttpHeaders.USER_AGENT, List.of(USER_AGENT))) .build(); } - } diff --git a/configserver/src/main/resources/configserver-app/services.xml b/configserver/src/main/resources/configserver-app/services.xml index b429e220c33..97b2156e8ca 100644 --- a/configserver/src/main/resources/configserver-app/services.xml +++ b/configserver/src/main/resources/configserver-app/services.xml @@ -54,7 +54,7 @@ <preprocess:include file='model-integration.xml' required='true' /> <component id="com.yahoo.vespa.configserver.flags.ConfigServerFlagSource" bundle="configserver-flags"/> - <component id="com.yahoo.vespa.flags.persistence.FlagsDb" bundle="flags"/> + <component id="com.yahoo.vespa.configserver.flags.db.FlagsDbImpl" bundle="configserver-flags"/> <preprocess:include file='metrics-packets.xml' required='false' /> <preprocess:include file='container.xml' required='false' /> @@ -125,6 +125,7 @@ <binding>http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/serviceconverge</binding> <binding>http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/serviceconverge/*</binding> <binding>http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/clustercontroller/*/status/*</binding> + <binding>http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/metrics</binding> <binding>http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*</binding> <binding>http://*/application/v2/tenant/*/application/*</binding> <binding>http://*/application/v2/tenant/*/application/*/logs</binding> diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/MetricsRetrieverTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/MetricsRetrieverTest.java new file mode 100644 index 00000000000..1b878a432c9 --- /dev/null +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/MetricsRetrieverTest.java @@ -0,0 +1,99 @@ +package com.yahoo.vespa.config.server.metrics; + +import com.github.tomakehurst.wiremock.junit.WireMockRule; +import junit.framework.AssertionFailedError; +import org.junit.Rule; +import org.junit.Test; + +import java.io.IOException; +import java.net.URI; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Optional; +import java.util.function.BiConsumer; + +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +import static com.github.tomakehurst.wiremock.client.WireMock.get; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; +import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options; +import static org.junit.Assert.*; + + +/** + * @author olaa + */ +public class MetricsRetrieverTest { + + @Rule + public final WireMockRule wireMock = new WireMockRule(options().port(8080), true); + + @Test + public void testMetricAggregation() throws IOException { + var metricsRetriever = new MetricsRetriever(); + + var clusters = List.of( + new ClusterInfo("cluster1", ClusterInfo.ClusterType.content, List.of(URI.create("http://localhost:8080/1"), URI.create("http://localhost:8080/2"))), + new ClusterInfo("cluster2", ClusterInfo.ClusterType.container, List.of(URI.create("http://localhost:8080/3"))) + ); + + stubFor(get(urlEqualTo("/1")) + .willReturn(aResponse() + .withStatus(200) + .withBody(contentMetrics()))); + + stubFor(get(urlEqualTo("/2")) + .willReturn(aResponse() + .withStatus(200) + .withBody(contentMetrics()))); + + stubFor(get(urlEqualTo("/3")) + .willReturn(aResponse() + .withStatus(200) + .withBody(containerMetrics()))); + + compareAggregators( + new MetricsAggregator().addDocumentCount(6000.0), + metricsRetriever.requestMetricsForCluster(clusters.get(0)) + ); + + compareAggregators( + new MetricsAggregator() + .addContainerLatency(3000, 43) + .addContainerLatency(2000, 0) + .addQrLatency(3000, 43) + .addFeedLatency(3000, 43), + metricsRetriever.requestMetricsForCluster(clusters.get(1)) + ); + + wireMock.stop(); + } + + private String containerMetrics() throws IOException { + return Files.readString(Path.of("src/test/resources/metrics/container_metrics")); + } + + private String contentMetrics() throws IOException { + return Files.readString(Path.of("src/test/resources/metrics/content_metrics")); + } + + // Same tolerance value as used internally in MetricsAggregator.isZero + private static final double metricsTolerance = 0.001; + + private void compareAggregators(MetricsAggregator expected, MetricsAggregator actual) { + BiConsumer<Double, Double> assertDoubles = (a, b) -> assertEquals(a.doubleValue(), b.doubleValue(), metricsTolerance); + + compareOptionals(expected.aggregateDocumentCount(), actual.aggregateDocumentCount(), assertDoubles); + compareOptionals(expected.aggregateQueryRate(), actual.aggregateQueryRate(), assertDoubles); + compareOptionals(expected.aggregateFeedRate(), actual.aggregateFeedRate(), assertDoubles); + compareOptionals(expected.aggregateQueryLatency(), actual.aggregateQueryLatency(), assertDoubles); + compareOptionals(expected.aggregateFeedLatency(), actual.aggregateFeedLatency(), assertDoubles); + } + + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") + private static <T> void compareOptionals(Optional<T> a, Optional<T> b, BiConsumer<T, T> comparer) { + if (a.isPresent() != b.isPresent()) throw new AssertionFailedError("Both optionals are not present: " + a + ", " + b); + a.ifPresent(x -> b.ifPresent(y -> comparer.accept(x, y))); + } +}
\ No newline at end of file diff --git a/configserver/src/test/resources/metrics/container_metrics b/configserver/src/test/resources/metrics/container_metrics new file mode 100644 index 00000000000..09232fe0f93 --- /dev/null +++ b/configserver/src/test/resources/metrics/container_metrics @@ -0,0 +1,41 @@ +{ + "services": [ + { + "name":"container", + "timestamp": 1557306075, + "metrics": [ + { + "values": { + "queries.rate": 23.0, + "query_latency.sum": 2000, + "document.count": 300000, + "feed.rate": 23.0, + "write_latency.sum": 2000 + } + }, + { + "values": { + "query_latency.count": 43.0, + "query_latency.sum": 3000, + "feed_latency.count": 43.0, + "feed_latency.sum": 3000 + + } + } + ] + }, + + { + "name":"qrserver", + "timestamp": 1557306075, + "metrics": [ + { + "values": { + "query_latency.count": 43.0, + "query_latency.sum": 3000 + } + } + ] + } + ] +}
\ No newline at end of file diff --git a/configserver/src/test/resources/metrics/content_metrics b/configserver/src/test/resources/metrics/content_metrics new file mode 100644 index 00000000000..a239aeea5ca --- /dev/null +++ b/configserver/src/test/resources/metrics/content_metrics @@ -0,0 +1,16 @@ +{ + "services": [ + { + "name":"distributor", + "timestamp": 1557306075, + "metrics": [ + { + "values": { + "vds.distributor.docsstored.average": 3000 + + } + } + ] + } + ] +}
\ No newline at end of file diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/ClusterMetrics.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/ClusterMetrics.java new file mode 100644 index 00000000000..1377a333335 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/ClusterMetrics.java @@ -0,0 +1,40 @@ +// 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.controller.api.application.v4.model; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +/** + * @author olaa + */ +public class ClusterMetrics { + + private final String clusterId; + private final ClusterType clusterType; + private final Map<String, Double> metrics; + + public ClusterMetrics(String clusterId, ClusterType clusterType) { + this.clusterId = clusterId; + this.clusterType = clusterType; + this.metrics = new HashMap<>(); + } + + public String getClusterId() { + return clusterId; + } + + public ClusterType getClusterType() { + return clusterType; + } + + public Map<String, Double> getMetrics() { + return Collections.unmodifiableMap(metrics); + } + + public void addMetric(String name, double value) { + metrics.put(name, value); + } + + public enum ClusterType {content, container}; +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java index 9eae2965c45..688cf275892 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java @@ -6,6 +6,7 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions; import com.yahoo.vespa.hosted.controller.api.application.v4.model.EndpointStatus; +import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.identifiers.Hostname; import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; @@ -44,6 +45,8 @@ public interface ConfigServer { InputStream getLogs(DeploymentId deployment, Map<String, String> queryParameters); + List<ClusterMetrics> getMetrics(DeploymentId deployment); + List<String> getContentClusters(DeploymentId deployment); /** diff --git a/controller-server/pom.xml b/controller-server/pom.xml index c6c6acafe15..ae756eae1fb 100644 --- a/controller-server/pom.xml +++ b/controller-server/pom.xml @@ -107,6 +107,13 @@ <scope>provided</scope> </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>configserver-flags</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + </dependency> + <!-- compile --> <dependency> diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 60fd095eb04..7406795d0e3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -279,7 +279,7 @@ public class ApplicationController { /** Deploys an application. If the application does not exist it is created. */ // TODO: Get rid of the options arg - // TODO jvenstad: Split this, and choose between deployDirectly and deploy in handler, excluding internally built from the latter. + // TODO(jvenstad): Split this, and choose between deployDirectly and deploy in handler, excluding internally built from the latter. public ActivateResult deploy(ApplicationId applicationId, ZoneId zone, Optional<ApplicationPackage> applicationPackageFromDeployer, Optional<ApplicationVersion> applicationVersionFromDeployer, @@ -333,11 +333,12 @@ public class ApplicationController { validateRun(application.get(), zone, platformVersion, applicationVersion); } - // TODO: Remove this when all packages are validated upon submission, as in ApplicationApiHandler.submit(...). + // TODO(jvenstad): Remove this when all packages are validated upon submission, as in ApplicationApiHandler.submit(...). verifyApplicationIdentityConfiguration(applicationId.tenant(), applicationPackage, deployingIdentity); - // Assign global rotation + // TODO(ogronnesby): Remove feature flag and replace calls to withRotationLegacy with withRotation + // TODO(mpolden): Remove all handling of legacy endpoints once withRotationLegacy disappears if (useMultipleEndpoints.with(FetchVector.Dimension.APPLICATION_ID, application.get().id().serializedForm()).value()) { application = withRotation(application, zone); @@ -373,7 +374,7 @@ public class ApplicationController { if ( ! preferOldestVersion && ! application.get().deploymentJobs().deployedInternally() && ! zone.environment().isManuallyDeployed()) - // TODO jvenstad: Store only on submissions + // TODO(jvenstad): Store only on submissions storeWithUpdatedConfig(application, applicationPackage); } // Release application lock while doing the deployment, which is a lengthy task. @@ -495,21 +496,16 @@ public class ApplicationController { } private List<AssignedRotation> createDefaultGlobalIdRotation(Application application, Rotation rotation) { - // This is guaranteed by .withRotationLegacy, but add this to make inspections accept the use of .get() below - assert application.deploymentSpec().globalServiceId().isPresent(); - - final Set<RegionName> regions = application.deploymentSpec().zones().stream() - .filter(zone -> zone.environment().isProduction()) - .flatMap(zone -> zone.region().stream()) - .collect(Collectors.toSet()); - - final var assignment = new AssignedRotation( + Set<RegionName> regions = application.deploymentSpec().zones().stream() + .filter(zone -> zone.environment().isProduction()) + .flatMap(zone -> zone.region().stream()) + .collect(Collectors.toSet()); + var assignment = new AssignedRotation( ClusterSpec.Id.from(application.deploymentSpec().globalServiceId().get()), EndpointId.default_(), rotation.id(), regions ); - return List.of(assignment); } @@ -517,7 +513,7 @@ public class ApplicationController { private LockedApplication withRotation(LockedApplication application, ZoneId zone) { if (zone.environment() == Environment.prod) { try (RotationLock rotationLock = rotationRepository.lock()) { - final var rotations = rotationRepository.getOrAssignRotations(application.get(), rotationLock); + var rotations = rotationRepository.getOrAssignRotations(application.get(), rotationLock); application = application.with(rotations); store(application); // store assigned rotation even if deployment fails registerAssignedRotationCnames(application.get()); @@ -528,16 +524,12 @@ public class ApplicationController { private void registerAssignedRotationCnames(Application application) { application.assignedRotations().forEach(assignedRotation -> { - final var endpoints = application - .endpointsIn(controller.system(), assignedRotation.endpointId()) - .scope(Endpoint.Scope.global); - - final var maybeRotation = rotationRepository.getRotation(assignedRotation.rotationId()); - + var endpoints = application.endpointsIn(controller.system(), assignedRotation.endpointId()) + .scope(Endpoint.Scope.global); + var maybeRotation = rotationRepository.getRotation(assignedRotation.rotationId()); maybeRotation.ifPresent(rotation -> { - endpoints.main().ifPresent(mainEndpoint -> { - registerCname(mainEndpoint.dnsName(), rotation.name()); - }); + // For rotations assigned using <endpoints/> syntax, we only register the non-legacy name in DNS. + endpoints.main().ifPresent(mainEndpoint -> registerCname(mainEndpoint.dnsName(), rotation.name())); }); }); } @@ -545,7 +537,7 @@ public class ApplicationController { private LockedApplication withApplicationCertificate(LockedApplication application) { ApplicationId applicationId = application.get().id(); - // TODO: Verify that the application is deploying to a zone where certificate provisioning is enabled + // TODO(tokle): Verify that the application is deploying to a zone where certificate provisioning is enabled boolean provisionCertificate = provisionApplicationCertificate.with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); if (provisionCertificate) { application = application.withApplicationCertificate( @@ -640,7 +632,7 @@ public class ApplicationController { .orElse(id.applicationId().instance().isTester())) throw new NotExistsException("Deployment", id.toString()); - // TODO jvenstad: Swap to use routingPolicies first, when this is ready. + // TODO(jvenstad): Swap to use routingPolicies first, when this is ready. try { var endpoints = routingGenerator.clusterEndpoints(id); if ( ! endpoints.isEmpty()) @@ -699,12 +691,12 @@ public class ApplicationController { applicationStore.removeAll(TesterId.of(id)); application.get().assignedRotations().forEach(assignedRotation -> { - final var endpoints = application.get().endpointsIn(controller.system(), assignedRotation.endpointId()); + var endpoints = application.get().endpointsIn(controller.system(), assignedRotation.endpointId()); endpoints.asList().stream() - .map(Endpoint::dnsName) - .forEach(name -> { - controller.nameServiceForwarder().removeRecords(Record.Type.CNAME, RecordName.from(name), Priority.normal); - }); + .map(Endpoint::dnsName) + .forEach(name -> { + controller.nameServiceForwarder().removeRecords(Record.Type.CNAME, RecordName.from(name), Priority.normal); + }); }); log.info("Deleted " + application); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java index 294dc10d0bd..8722c3defeb 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java @@ -25,7 +25,6 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.application.RotationStatus; -import com.yahoo.vespa.hosted.controller.rotation.RotationId; import java.time.Instant; import java.util.LinkedHashMap; @@ -279,7 +278,6 @@ public class LockedApplication { metrics, pemDeployKey, rotations, rotationStatus, applicationCertificate); } - /** Don't expose non-leaf sub-objects. */ private LockedApplication with(Deployment deployment) { Map<ZoneId, Deployment> deployments = new LinkedHashMap<>(this.deployments); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/AssignedRotation.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/AssignedRotation.java index ec13066d069..e23230b8503 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/AssignedRotation.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/AssignedRotation.java @@ -72,7 +72,7 @@ public class AssignedRotation { public static AssignedRotation fromStrings(String clusterId, String endpointId, String rotationId, Collection<String> regions) { return new AssignedRotation( new ClusterSpec.Id(clusterId), - new EndpointId(endpointId), + EndpointId.of(endpointId), new RotationId(rotationId), regions.stream().map(RegionName::from).collect(Collectors.toSet()) ); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java index 5dccd5c8120..4041c955cc4 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.controller.application; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.config.provision.RotationName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.ZoneId; @@ -216,7 +215,6 @@ public class Endpoint { private ZoneId zone; private ClusterSpec.Id cluster; - private RotationName rotation; private EndpointId endpointId; private Port port; private boolean legacy = false; @@ -228,7 +226,7 @@ public class Endpoint { /** Sets the cluster and zone target of this */ public EndpointBuilder target(ClusterSpec.Id cluster, ZoneId zone) { - if (rotation != null || endpointId != null) { + if (endpointId != null) { throw new IllegalArgumentException("Cannot set multiple target types"); } this.cluster = cluster; @@ -236,18 +234,9 @@ public class Endpoint { return this; } - /** Sets the rotation target of this */ - public EndpointBuilder target(RotationName rotation) { - if ((cluster != null && zone != null) || endpointId != null) { - throw new IllegalArgumentException("Cannot set multiple target types"); - } - this.rotation = rotation; - return this; - } - /** Sets the endpoint ID as defines in deployments.xml */ public EndpointBuilder named(EndpointId endpointId) { - if (rotation != null || cluster != null || zone != null) { + if (cluster != null || zone != null) { throw new IllegalArgumentException("Cannot set multiple target types"); } this.endpointId = endpointId; @@ -277,8 +266,6 @@ public class Endpoint { String name; if (cluster != null && zone != null) { name = cluster.value(); - } else if (rotation != null) { - name = rotation.value(); } else if (endpointId != null) { name = endpointId.id(); } else { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointId.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointId.java index 13c242c7b5f..7c88b94a2ae 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointId.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointId.java @@ -1,5 +1,7 @@ package com.yahoo.vespa.hosted.controller.application; +import org.jetbrains.annotations.NotNull; + import java.util.Objects; /** @@ -8,12 +10,13 @@ import java.util.Objects; * * @author ogronnesby */ -public class EndpointId { +public class EndpointId implements Comparable<EndpointId> { + private static final EndpointId DEFAULT = new EndpointId("default"); private final String id; - public EndpointId(String id) { + private EndpointId(String id) { this.id = requireNotEmpty(id); } @@ -50,4 +53,10 @@ public class EndpointId { public static EndpointId default_() { return DEFAULT; } public static EndpointId of(String id) { return new EndpointId(id); } + + @Override + public int compareTo(@NotNull EndpointId o) { + return id.compareTo(o.id); + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointList.java index d9aea783880..c4613db27d1 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointList.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.controller.application; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.RotationName; import com.yahoo.config.provision.SystemName; import com.yahoo.vespa.hosted.controller.application.Endpoint.Port; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingId.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingId.java index c9378e27b61..7b0ec3d27ba 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingId.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingId.java @@ -2,31 +2,30 @@ package com.yahoo.vespa.hosted.controller.application; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.RotationName; import java.util.Objects; /** - * Unique identifier for a global routing table entry (application x rotation name). + * Unique identifier for a global routing table entry (application x endpoint ID). * * @author mpolden */ public class RoutingId { private final ApplicationId application; - private final RotationName rotation; + private final EndpointId endpointId; - public RoutingId(ApplicationId application, RotationName rotation) { + public RoutingId(ApplicationId application, EndpointId endpointId) { this.application = Objects.requireNonNull(application, "application must be non-null"); - this.rotation = Objects.requireNonNull(rotation, "rotation must be non-null"); + this.endpointId = Objects.requireNonNull(endpointId, "endpointId must be non-null"); } public ApplicationId application() { return application; } - public RotationName rotation() { - return rotation; + public EndpointId endpointId() { + return endpointId; } @Override @@ -35,12 +34,12 @@ public class RoutingId { if (o == null || getClass() != o.getClass()) return false; RoutingId that = (RoutingId) o; return application.equals(that.application) && - rotation.equals(that.rotation); + endpointId.equals(that.endpointId); } @Override public int hashCode() { - return Objects.hash(application, rotation); + return Objects.hash(application, endpointId); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java index e0145e6b94c..a86bbaa317e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java @@ -5,7 +5,6 @@ import com.google.common.collect.ImmutableSortedSet; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.RotationName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.Endpoint.Port; @@ -27,17 +26,17 @@ public class RoutingPolicy { private final ZoneId zone; private final HostName canonicalName; private final Optional<String> dnsZone; - private final Set<RotationName> rotations; + private final Set<EndpointId> endpoints; /** DO NOT USE. Public for serialization purposes */ public RoutingPolicy(ApplicationId owner, ClusterSpec.Id cluster, ZoneId zone, HostName canonicalName, - Optional<String> dnsZone, Set<RotationName> rotations) { + Optional<String> dnsZone, Set<EndpointId> endpoints) { this.owner = Objects.requireNonNull(owner, "owner must be non-null"); this.cluster = Objects.requireNonNull(cluster, "cluster must be non-null"); this.zone = Objects.requireNonNull(zone, "zone must be non-null"); this.canonicalName = Objects.requireNonNull(canonicalName, "canonicalName must be non-null"); this.dnsZone = Objects.requireNonNull(dnsZone, "dnsZone must be non-null"); - this.rotations = ImmutableSortedSet.copyOf(Objects.requireNonNull(rotations, "rotations must be non-null")); + this.endpoints = ImmutableSortedSet.copyOf(Objects.requireNonNull(endpoints, "endpoints must be non-null")); } /** The application owning this */ @@ -65,9 +64,9 @@ public class RoutingPolicy { return dnsZone; } - /** The rotations in this policy */ - public Set<RotationName> rotations() { - return rotations; + /** The endpoints of this policy */ + public Set<EndpointId> endpoints() { + return endpoints; } /** Returns the endpoint of this */ @@ -77,7 +76,7 @@ public class RoutingPolicy { /** Returns rotation endpoints of this */ public EndpointList rotationEndpointsIn(SystemName system) { - return EndpointList.of(rotations.stream().map(rotation -> endpointOf(owner, rotation, system))); + return EndpointList.of(endpoints.stream().map(endpointId -> endpointOf(owner, endpointId, system))); } @Override @@ -95,14 +94,14 @@ public class RoutingPolicy { @Override public String toString() { - return String.format("%s [rotations: %s%s], %s owned by %s, in %s", canonicalName, rotations, + return String.format("%s [rotations: %s%s], %s owned by %s, in %s", canonicalName, endpoints, dnsZone.map(z -> ", DNS zone: " + z).orElse(""), cluster, owner.toShortString(), zone.value()); } /** Returns the endpoint of given rotation */ - public static Endpoint endpointOf(ApplicationId application, RotationName rotation, SystemName system) { - return Endpoint.of(application).target(rotation).on(Port.tls()).directRouting().in(system); + public static Endpoint endpointOf(ApplicationId application, EndpointId endpointId, SystemName system) { + return Endpoint.of(application).named(endpointId).on(Port.tls()).directRouting().in(system); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java index bef61dda875..068a41ed92c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java @@ -131,18 +131,21 @@ public class Versions { return change.platform().get(); return max(change.platform(), deployment.map(Deployment::version)) - .orElse(application.oldestDeployedPlatform() - .orElse(defaultVersion)); + .orElseGet(() -> application.oldestDeployedPlatform().orElse(defaultVersion)); } private static ApplicationVersion targetApplication(Application application, Change change, Optional<Deployment> deployment) { return max(change.application(), deployment.map(Deployment::applicationVersion)) - .orElse(application.oldestDeployedApplication() - .orElse(application.deploymentJobs().jobStatus().get(JobType.component) - .lastSuccess() - .get() - .application())); + .orElseGet(() -> defaultApplicationVersion(application)); + } + + private static ApplicationVersion defaultApplicationVersion(Application application) { + return application.oldestDeployedApplication() + .orElseGet(() -> Optional.ofNullable(application.deploymentJobs().jobStatus().get(JobType.component)) + .flatMap(JobStatus::lastSuccess) + .map(JobStatus.JobRun::application) + .orElse(ApplicationVersion.unknown)); } private static <T extends Comparable<T>> Optional<T> max(Optional<T> o1, Optional<T> o2) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java index 4ad5940f8f2..ab587fc4078 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java @@ -41,8 +41,7 @@ public class DeploymentMetricsMaintainer extends Maintainer { private final ApplicationController applications; public DeploymentMetricsMaintainer(Controller controller, Duration duration, JobControl jobControl) { - super(controller, duration, jobControl, DeploymentMetricsMaintainer.class.getSimpleName(), - SystemName.allOf(Predicate.not(SystemName::isPublic))); + super(controller, duration, jobControl, DeploymentMetricsMaintainer.class.getSimpleName(), SystemName.all()); this.applications = controller.applications(); } @@ -122,5 +121,4 @@ public class DeploymentMetricsMaintainer extends Maintainer { default: throw new IllegalArgumentException("Unknown API value for rotation status: " + status); } } - } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicies.java index 4a98cb49227..d23ae913889 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicies.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicies.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.RotationName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Controller; @@ -12,6 +13,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordData; import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName; import com.yahoo.vespa.hosted.controller.application.Endpoint; +import com.yahoo.vespa.hosted.controller.application.EndpointId; import com.yahoo.vespa.hosted.controller.application.RoutingId; import com.yahoo.vespa.hosted.controller.application.RoutingPolicy; import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue.Priority; @@ -89,7 +91,7 @@ public class RoutingPolicies { // Create DNS record for each routing ID for (Map.Entry<RoutingId, List<RoutingPolicy>> routeEntry : routingTable.entrySet()) { - Endpoint endpoint = RoutingPolicy.endpointOf(routeEntry.getKey().application(), routeEntry.getKey().rotation(), + Endpoint endpoint = RoutingPolicy.endpointOf(routeEntry.getKey().application(), routeEntry.getKey().endpointId(), controller.system()); Set<AliasTarget> targets = routeEntry.getValue() .stream() @@ -117,9 +119,14 @@ public class RoutingPolicies { /** Create a policy for given load balancer and register a CNAME for it */ private RoutingPolicy createPolicy(ApplicationId application, ZoneId zone, LoadBalancer loadBalancer) { + // TODO(mpolden): Remove rotations from LoadBalancer. Use endpoints from deployment spec instead + Set<EndpointId> endpoints = loadBalancer.rotations().stream() + .map(RotationName::value) + .map(EndpointId::of) + .collect(Collectors.toSet()); RoutingPolicy routingPolicy = new RoutingPolicy(application, loadBalancer.cluster(), zone, loadBalancer.hostname(), loadBalancer.dnsZone(), - loadBalancer.rotations()); + endpoints); RecordName name = RecordName.from(routingPolicy.endpointIn(controller.system()).dnsName()); RecordData data = RecordData.fqdn(loadBalancer.hostname().value()); controller.nameServiceForwarder().createCname(name, data, Priority.normal); @@ -151,7 +158,7 @@ public class RoutingPolicies { var activeRoutingIds = routingIdsFrom(loadBalancers.list); removalCandidates.removeAll(activeRoutingIds); for (var id : removalCandidates) { - Endpoint endpoint = RoutingPolicy.endpointOf(id.application(), id.rotation(), controller.system()); + Endpoint endpoint = RoutingPolicy.endpointOf(id.application(), id.endpointId(), controller.system()); controller.nameServiceForwarder().removeRecords(Record.Type.ALIAS, RecordName.from(endpoint.dnsName()), Priority.normal); } } @@ -161,7 +168,7 @@ public class RoutingPolicies { Set<RoutingId> routingIds = new LinkedHashSet<>(); for (var loadBalancer : loadBalancers) { for (var rotation : loadBalancer.rotations()) { - routingIds.add(new RoutingId(loadBalancer.application(), rotation)); + routingIds.add(new RoutingId(loadBalancer.application(), EndpointId.of(rotation.value()))); } } return Collections.unmodifiableSet(routingIds); @@ -171,7 +178,7 @@ public class RoutingPolicies { private static Map<RoutingId, List<RoutingPolicy>> routingTableFrom(Set<RoutingPolicy> routingPolicies) { var routingTable = new LinkedHashMap<RoutingId, List<RoutingPolicy>>(); for (var policy : routingPolicies) { - for (var rotation : policy.rotations()) { + for (var rotation : policy.endpoints()) { var id = new RoutingId(policy.owner(), rotation); routingTable.putIfAbsent(id, new ArrayList<>()); routingTable.get(id).add(policy); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java index d49587963dc..c1962e8d17c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java @@ -109,6 +109,7 @@ public class ApplicationSerializer { private final String lastWrittenField = "lastWritten"; private final String lastQueriesPerSecondField = "lastQueriesPerSecond"; private final String lastWritesPerSecondField = "lastWritesPerSecond"; + private final String clusterMetricsField = "clusterMetrics"; // DeploymentJobs fields private final String projectIdField = "projectId"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index d704d701cf0..b7b64b9cda2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -106,10 +106,6 @@ public class CuratorDb { CuratorDb(Curator curator, Duration tryLockTimeout) { this.curator = curator; this.tryLockTimeout = tryLockTimeout; - - // TODO: Remove after 7.60 - curator.delete(root.append("openStackServerPool")); - curator.delete(root.append("vespaServerPool")); } /** Returns all hosts configured to be part of this ZooKeeper cluster */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java index 9cfce8dc16a..80858e713c2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java @@ -4,11 +4,10 @@ package com.yahoo.vespa.hosted.controller.persistence; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.RotationName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.slime.ArrayTraverser; -import com.yahoo.slime.Cursor; import com.yahoo.slime.Slime; +import com.yahoo.vespa.hosted.controller.application.EndpointId; import com.yahoo.vespa.hosted.controller.application.RoutingPolicy; import java.util.Collections; @@ -39,36 +38,36 @@ public class RoutingPolicySerializer { private static final String rotationsField = "rotations"; public Slime toSlime(Set<RoutingPolicy> routingPolicies) { - Slime slime = new Slime(); - Cursor root = slime.setObject(); - Cursor policyArray = root.setArray(routingPoliciesField); + var slime = new Slime(); + var root = slime.setObject(); + var policyArray = root.setArray(routingPoliciesField); routingPolicies.forEach(policy -> { - Cursor policyObject = policyArray.addObject(); + var policyObject = policyArray.addObject(); policyObject.setString(clusterField, policy.cluster().value()); policyObject.setString(zoneField, policy.zone().value()); policyObject.setString(canonicalNameField, policy.canonicalName().value()); policy.dnsZone().ifPresent(dnsZone -> policyObject.setString(dnsZoneField, dnsZone)); - Cursor rotationArray = policyObject.setArray(rotationsField); - policy.rotations().forEach(rotation -> { - rotationArray.addString(rotation.value()); + var rotationArray = policyObject.setArray(rotationsField); + policy.endpoints().forEach(endpointId -> { + rotationArray.addString(endpointId.id()); }); }); return slime; } public Set<RoutingPolicy> fromSlime(ApplicationId owner, Slime slime) { - Set<RoutingPolicy> policies = new LinkedHashSet<>(); - Cursor root = slime.get(); - Cursor field = root.field(routingPoliciesField); + var policies = new LinkedHashSet<RoutingPolicy>(); + var root = slime.get(); + var field = root.field(routingPoliciesField); field.traverse((ArrayTraverser) (i, inspect) -> { - Set<RotationName> rotations = new LinkedHashSet<>(); - inspect.field(rotationsField).traverse((ArrayTraverser) (j, rotation) -> rotations.add(RotationName.from(rotation.asString()))); + var endpointIds = new LinkedHashSet<EndpointId>(); + inspect.field(rotationsField).traverse((ArrayTraverser) (j, endpointId) -> endpointIds.add(EndpointId.of(endpointId.asString()))); policies.add(new RoutingPolicy(owner, ClusterSpec.Id.from(inspect.field(clusterField).asString()), ZoneId.from(inspect.field(zoneField).asString()), HostName.from(inspect.field(canonicalNameField).asString()), Serializers.optionalField(inspect.field(dnsZoneField), Function.identity()), - rotations)); + endpointIds)); }); return Collections.unmodifiableSet(policies); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/flags/AuditedFlagsHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/flags/AuditedFlagsHandler.java index 12b59db756f..31058a71816 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/flags/AuditedFlagsHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/flags/AuditedFlagsHandler.java @@ -3,8 +3,8 @@ package com.yahoo.vespa.hosted.controller.restapi.flags; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.vespa.flags.http.FlagsHandler; -import com.yahoo.vespa.flags.persistence.FlagsDb; +import com.yahoo.vespa.configserver.flags.FlagsDb; +import com.yahoo.vespa.configserver.flags.http.FlagsHandler; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.auditlog.AuditLogger; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java index f5047a82e2f..bf798d2f004 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.controller.application; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.config.provision.RotationName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.Endpoint.Port; @@ -23,51 +22,51 @@ public class EndpointTest { @Test public void test_global_endpoints() { - RotationName rotation = RotationName.from("default"); // Always default for non-direct routing + EndpointId endpointId = EndpointId.default_(); Map<String, Endpoint> tests = Map.of( // Legacy endpoint "http://a1.t1.global.vespa.yahooapis.com:4080/", - Endpoint.of(app1).target(rotation).on(Port.plain(4080)).legacy().in(SystemName.main), + Endpoint.of(app1).named(endpointId).on(Port.plain(4080)).legacy().in(SystemName.main), // Legacy endpoint with TLS "https://a1--t1.global.vespa.yahooapis.com:4443/", - Endpoint.of(app1).target(rotation).on(Port.tls(4443)).legacy().in(SystemName.main), + Endpoint.of(app1).named(endpointId).on(Port.tls(4443)).legacy().in(SystemName.main), // Main endpoint "https://a1--t1.global.vespa.oath.cloud:4443/", - Endpoint.of(app1).target(rotation).on(Port.tls(4443)).in(SystemName.main), + Endpoint.of(app1).named(endpointId).on(Port.tls(4443)).in(SystemName.main), // Main endpoint in CD "https://cd--a1--t1.global.vespa.oath.cloud:4443/", - Endpoint.of(app1).target(rotation).on(Port.tls(4443)).in(SystemName.cd), + Endpoint.of(app1).named(endpointId).on(Port.tls(4443)).in(SystemName.cd), // Main endpoint with direct routing and default TLS port "https://a1.t1.global.vespa.oath.cloud/", - Endpoint.of(app1).target(rotation).on(Port.tls()).directRouting().in(SystemName.main), + Endpoint.of(app1).named(endpointId).on(Port.tls()).directRouting().in(SystemName.main), // Main endpoint with custom rotation name "https://r1.a1.t1.global.vespa.oath.cloud/", - Endpoint.of(app1).target(RotationName.from("r1")).on(Port.tls()).directRouting().in(SystemName.main), + Endpoint.of(app1).named(EndpointId.of("r1")).on(Port.tls()).directRouting().in(SystemName.main), // Main endpoint for custom instance in default rotation "https://a2.t2.global.vespa.oath.cloud/", - Endpoint.of(app2).target(rotation).on(Port.tls()).directRouting().in(SystemName.main), + Endpoint.of(app2).named(endpointId).on(Port.tls()).directRouting().in(SystemName.main), // Main endpoint for custom instance with custom rotation name "https://r2.a2.t2.global.vespa.oath.cloud/", - Endpoint.of(app2).target(RotationName.from("r2")).on(Port.tls()).directRouting().in(SystemName.main), + Endpoint.of(app2).named(EndpointId.of("r2")).on(Port.tls()).directRouting().in(SystemName.main), // Main endpoint in public system "https://a1.t1.global.public.vespa.oath.cloud/", - Endpoint.of(app1).target(rotation).on(Port.tls()).directRouting().in(SystemName.Public) + Endpoint.of(app1).named(endpointId).on(Port.tls()).directRouting().in(SystemName.Public) ); tests.forEach((expected, endpoint) -> assertEquals(expected, endpoint.url().toString())); } @Test public void test_global_endpoints_with_endpoint_id() { - final var endpointId = EndpointId.default_(); + var endpointId = EndpointId.default_(); Map<String, Endpoint> tests = Map.of( // Legacy endpoint @@ -111,9 +110,9 @@ public class EndpointTest { @Test public void test_zone_endpoints() { - ClusterSpec.Id cluster = ClusterSpec.Id.from("default"); // Always default for non-direct routing - ZoneId prodZone = ZoneId.from("prod", "us-north-1"); - ZoneId testZone = ZoneId.from("test", "us-north-2"); + var cluster = ClusterSpec.Id.from("default"); // Always default for non-direct routing + var prodZone = ZoneId.from("prod", "us-north-1"); + var testZone = ZoneId.from("test", "us-north-2"); Map<String, Endpoint> tests = Map.of( // Legacy endpoint (always contains environment) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java index a992ce1e3de..6725e05dd6d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java @@ -34,6 +34,7 @@ import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.aborted; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.unfinished; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertTrue; public class InternalDeploymentTester { @@ -171,7 +172,7 @@ public class InternalDeploymentTester { .findAny() .orElseThrow(() -> new AssertionError(type + " is not among the active: " + jobs.active())); assertFalse(run.hasFailed()); - assertFalse(run.status() == aborted); + assertNotSame(aborted, run.status()); ZoneId zone = type.zone(tester.controller().system()); DeploymentId deployment = new DeploymentId(appId, zone); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index a89c5988396..fbc7bf20a24 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -11,6 +11,7 @@ import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions; import com.yahoo.vespa.hosted.controller.api.application.v4.model.EndpointStatus; import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.ConfigChangeActions; +import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.identifiers.Hostname; import com.yahoo.vespa.hosted.controller.api.identifiers.Identifier; @@ -330,6 +331,11 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer return applicationView; } + @Override + public List<ClusterMetrics> getMetrics(DeploymentId deployment) { + return List.of(); + } + // Returns a canned example response @Override public Map<?,?> getServiceApiResponse(String tenantName, String applicationName, String instanceName, diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java index f0344cb8d12..600fca4f45e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java @@ -105,7 +105,7 @@ public class RoutingPoliciesTest { Set<RoutingPolicy> policies = tester.controller().curator().readRoutingPolicies(app1.id()); assertEquals(clustersPerZone * numberOfDeployments, policies.size()); assertTrue("Rotation membership is removed from all policies", - policies.stream().allMatch(policy -> policy.rotations().isEmpty())); + policies.stream().allMatch(policy -> policy.endpoints().isEmpty())); assertEquals("Rotations for " + app2 + " are not removed", 2, records3.get().size()); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializerTest.java index a0e95bd0393..e99cc302ffe 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializerTest.java @@ -5,8 +5,8 @@ import com.google.common.collect.ImmutableSet; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.RotationName; import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.vespa.hosted.controller.application.EndpointId; import com.yahoo.vespa.hosted.controller.application.RoutingPolicy; import org.junit.Test; @@ -26,19 +26,19 @@ public class RoutingPolicySerializerTest { @Test public void test_serialization() { var owner = ApplicationId.defaultId(); - var rotations = Set.of(RotationName.from("r1"), RotationName.from("r2")); + var endpoints = Set.of(EndpointId.of("r1"), EndpointId.of("r2")); var policies = ImmutableSet.of(new RoutingPolicy(owner, ClusterSpec.Id.from("my-cluster1"), ZoneId.from("prod", "us-north-1"), HostName.from("long-and-ugly-name"), Optional.of("zone1"), - rotations), + endpoints), new RoutingPolicy(owner, ClusterSpec.Id.from("my-cluster2"), ZoneId.from("prod", "us-north-2"), HostName.from("long-and-ugly-name-2"), Optional.empty(), - rotations)); + endpoints)); var serialized = serializer.fromSlime(owner, serializer.toSlime(policies)); assertEquals(policies.size(), serialized.size()); for (Iterator<RoutingPolicy> it1 = policies.iterator(), it2 = serialized.iterator(); it1.hasNext();) { @@ -49,7 +49,7 @@ public class RoutingPolicySerializerTest { assertEquals(expected.zone(), actual.zone()); assertEquals(expected.canonicalName(), actual.canonicalName()); assertEquals(expected.dnsZone(), actual.dnsZone()); - assertEquals(expected.rotations(), actual.rotations()); + assertEquals(expected.endpoints(), actual.endpoints()); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java index eaafb08f0de..83a43287880 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java @@ -60,7 +60,7 @@ public class ControllerContainerTest { " </rotations>\n" + " </config>\n" + " <component id='com.yahoo.vespa.flags.InMemoryFlagSource'/>\n" + - " <component id='com.yahoo.vespa.flags.persistence.FlagsDb'/>\n" + + " <component id='com.yahoo.vespa.configserver.flags.db.FlagsDbImpl'/>\n" + " <component id='com.yahoo.vespa.curator.mock.MockCurator'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.athenz.mock.AthenzClientFactoryMock'/>\n" + 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 577b8491bd2..41d8edbabc0 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 @@ -11,7 +11,6 @@ import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.RegionName; -import com.yahoo.config.provision.RotationName; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.slime.Cursor; @@ -44,6 +43,7 @@ import com.yahoo.vespa.hosted.controller.application.ClusterUtilization; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; +import com.yahoo.vespa.hosted.controller.application.EndpointId; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.application.RotationStatus; import com.yahoo.vespa.hosted.controller.application.RoutingPolicy; @@ -1380,7 +1380,7 @@ public class ApplicationApiTest extends ControllerContainerTest { ClusterSpec.Id.from("default"), ZoneId.from(Environment.prod, RegionName.from("us-west-1")), HostName.from("lb-0-canonical-name"), - Optional.of("dns-zone-1"), Set.of(RotationName.from("c0"))); + Optional.of("dns-zone-1"), Set.of(EndpointId.of("c0"))); tester.controller().curator().writeRoutingPolicies(app.id(), Set.of(policy)); // GET application diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java index 616db640132..614df953ca9 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java @@ -2,11 +2,13 @@ package com.yahoo.vespa.hosted.controller.restapi.application; import com.yahoo.component.Version; +import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException; -import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; +import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.InternalDeploymentTester; import org.json.JSONException; import org.json.JSONObject; @@ -134,12 +136,24 @@ public class JobControllerApiHandlerHelperTest { assertResponse(JobControllerApiHandlerHelper.jobTypeResponse(tester.tester().controller(), appId, URI.create("https://some.url:43/root")), "dev-overview.json"); } + @Test + public void testResponsesWithDirectDeployment() { + var tester = new InternalDeploymentTester(); + tester.clock().setInstant(Instant.EPOCH); + var region = "us-west-1"; + var applicationPackage = new ApplicationPackageBuilder().region(region).build(); + // Deploy directly to production zone, like integration tests. + tester.tester().controller().applications().deploy(tester.app().id(), ZoneId.from("prod", region), + Optional.of(applicationPackage), + new DeployOptions(true, Optional.empty(), + false, false)); + assertResponse(JobControllerApiHandlerHelper.jobTypeResponse(tester.tester().controller(), appId, URI.create("https://some.url:43/root/")), + "jobs-direct-deployment.json"); + } + private void compare(HttpResponse response, String expected) throws JSONException, IOException { ByteArrayOutputStream baos = new ByteArrayOutputStream(); response.render(baos); - - System.err.println(baos); - JSONObject actualJSON = new JSONObject(new String(baos.toByteArray())); JSONObject expectedJSON = new JSONObject(expected); assertEquals(expectedJSON.toString(), actualJSON.toString()); @@ -148,7 +162,7 @@ public class JobControllerApiHandlerHelperTest { private void assertResponse(HttpResponse response, String fileName) { try { Path path = Paths.get("src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/").resolve(fileName); - String expected = new String(Files.readAllBytes(path)); + String expected = Files.readString(path); compare(response, expected); } catch (Exception e) { throw new RuntimeException(e); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs-direct-deployment.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs-direct-deployment.json new file mode 100644 index 00000000000..5535e286dcd --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs-direct-deployment.json @@ -0,0 +1,79 @@ +{ + "devJobs": {}, + "deployments": [ + { + "us-west-1": { + "at": 0, + "application": { + "hash": "unknown" + }, + "verified": false, + "platform": "6.1" + } + } + ], + "lastVersions": {}, + "deploying": {}, + "jobs": { + "staging-test": { + "runs": [ + { + "reason": "Testing for productionUsWest1", + "wantedPlatform": "6.1", + "currentPlatform": "6.1", + "wantedApplication": { + "hash": "unknown" + }, + "currentApplication": { + "hash": "unknown" + }, + "tasks": { + "capacity": "running" + }, + "status": "pending" + } + ], + "url": "https://some.url:43/root/staging-test" + }, + "system-test": { + "runs": [ + { + "reason": "Testing for productionUsWest1", + "wantedPlatform": "6.1", + "currentPlatform": "6.1", + "wantedApplication": { + "hash": "unknown" + }, + "currentApplication": { + "hash": "unknown" + }, + "tasks": { + "capacity": "running" + }, + "status": "pending" + } + ], + "url": "https://some.url:43/root/system-test" + }, + "us-west-1": { + "runs": [ + { + "wantedPlatform": "6.1", + "currentPlatform": "6.1", + "wantedApplication": { + "hash": "unknown" + }, + "currentApplication": { + "hash": "unknown" + }, + "tasks": { + "staging-test": "pending", + "system-test": "pending" + }, + "status": "pending" + } + ], + "url": "https://some.url:43/root/production-us-west-1" + } + } +} diff --git a/dist/vespa.spec b/dist/vespa.spec index 3426eff459d..ba1dbc32831 100644 --- a/dist/vespa.spec +++ b/dist/vespa.spec @@ -90,6 +90,7 @@ Requires: perl-Getopt-Long Requires: perl-IO-Socket-IP Requires: perl-JSON Requires: perl-libwww-perl +Requires: perl-LWP-Protocol-https Requires: perl-Net-INET6Glue Requires: perl-Pod-Usage Requires: perl-URI diff --git a/eval/src/tests/tensor/dense_add_dimension_optimizer/dense_add_dimension_optimizer_test.cpp b/eval/src/tests/tensor/dense_add_dimension_optimizer/dense_add_dimension_optimizer_test.cpp index eaf4623afea..274117ea693 100644 --- a/eval/src/tests/tensor/dense_add_dimension_optimizer/dense_add_dimension_optimizer_test.cpp +++ b/eval/src/tests/tensor/dense_add_dimension_optimizer/dense_add_dimension_optimizer_test.cpp @@ -25,6 +25,7 @@ const TensorEngine &prod_engine = DefaultTensorEngine::ref(); EvalFixture::ParamRepo make_params() { return EvalFixture::ParamRepo() .add("x5", spec({x(5)}, N())) + .add("x5f", spec(float_cells({x(5)}), N())) .add("x5y1", spec({x(5),y(1)}, N())) .add("y1z1", spec({y(1),z(1)}, N())) .add("x_m", spec({x({"a"})}, N())); @@ -78,9 +79,9 @@ TEST("require that non-canonical dimension addition is not optimized") { TEST_DO(verify_not_optimized("tensor(y[1])(1)/x5")); } -TEST("require that dimension addition with overlapping dimensions is not optimized") { - TEST_DO(verify_not_optimized("x5y1*tensor(y[1],z[1])(1)")); - TEST_DO(verify_not_optimized("tensor(y[1],z[1])(1)*x5y1")); +TEST("require that dimension addition with overlapping dimensions is optimized") { + TEST_DO(verify_optimized("x5y1*tensor(y[1],z[1])(1)")); + TEST_DO(verify_optimized("tensor(y[1],z[1])(1)*x5y1")); } TEST("require that dimension addition with inappropriate dimensions is not optimized") { @@ -99,8 +100,13 @@ TEST("require that dimension addition optimization requires unit constant tensor TEST_DO(verify_not_optimized("tensor(x[2])(1)*tensor(y[2])(1)")); } -TEST("require that optimization is disabled for tensors with non-double cells") { - TEST_DO(verify_not_optimized("x5*tensor<float>(a[1],b[1],c[1])(1)")); +TEST("require that optimization also works for float cells") { + TEST_DO(verify_optimized("x5*tensor<float>(a[1],b[1],c[1])(1)")); + TEST_DO(verify_optimized("x5f*tensor<float>(a[1],b[1],c[1])(1)")); +} + +TEST("require that optimization is disabled if unit vector would promote tensor cell types") { + TEST_DO(verify_not_optimized("x5f*tensor(a[1],b[1],c[1])(1)")); } TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/eval/src/tests/tensor/dense_fast_rename_optimizer/dense_fast_rename_optimizer_test.cpp b/eval/src/tests/tensor/dense_fast_rename_optimizer/dense_fast_rename_optimizer_test.cpp index 4995ea89735..55a9414f82b 100644 --- a/eval/src/tests/tensor/dense_fast_rename_optimizer/dense_fast_rename_optimizer_test.cpp +++ b/eval/src/tests/tensor/dense_fast_rename_optimizer/dense_fast_rename_optimizer_test.cpp @@ -72,8 +72,8 @@ TEST("require that chained optimized renames are compacted into a single operati TEST_DO(verify_optimized("rename(rename(x5,x,y),y,z)")); } -TEST("require that optimization is disabled for tensors with non-double cells") { - TEST_DO(verify_not_optimized("rename(x5f,x,y)")); +TEST("require that optimization works for float cells") { + TEST_DO(verify_optimized("rename(x5f,x,y)")); } TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/eval/src/tests/tensor/dense_inplace_join_function/dense_inplace_join_function_test.cpp b/eval/src/tests/tensor/dense_inplace_join_function/dense_inplace_join_function_test.cpp index 083ed1c7071..80321ac3d22 100644 --- a/eval/src/tests/tensor/dense_inplace_join_function/dense_inplace_join_function_test.cpp +++ b/eval/src/tests/tensor/dense_inplace_join_function/dense_inplace_join_function_test.cpp @@ -144,10 +144,15 @@ TEST("require that inplace join can be debug dumped") { fprintf(stderr, "%s\n", info[0]->as_string().c_str()); } -TEST("require that optimization is disabled for tensors with non-double cells") { - TEST_DO(verify_not_optimized("mut_x5_A-mut_x5f_D")); - TEST_DO(verify_not_optimized("mut_x5f_D-mut_x5_A")); - TEST_DO(verify_not_optimized("mut_x5f_D-mut_x5f_E")); +TEST("require that optimization works with float cells") { + TEST_DO(verify_p0_optimized("mut_x5f_D-mut_x5f_E", 1)); +} + +TEST("require that overwritten value must have same cell type as result") { + TEST_DO(verify_p0_optimized("mut_x5_A-mut_x5f_D", 1)); + TEST_DO(verify_p1_optimized("mut_x5f_D-mut_x5_A", 1)); + TEST_DO(verify_not_optimized("con_x5_A-mut_x5f_D")); + TEST_DO(verify_not_optimized("mut_x5f_D-con_x5_A")); } TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/eval/src/tests/tensor/dense_inplace_map_function/dense_inplace_map_function_test.cpp b/eval/src/tests/tensor/dense_inplace_map_function/dense_inplace_map_function_test.cpp index 314d3a6186c..f85742b4e0f 100644 --- a/eval/src/tests/tensor/dense_inplace_map_function/dense_inplace_map_function_test.cpp +++ b/eval/src/tests/tensor/dense_inplace_map_function/dense_inplace_map_function_test.cpp @@ -72,8 +72,8 @@ TEST("require that mapped tensors are not optimized") { TEST_DO(verify_not_optimized("map(_x_m,f(x)(x+10))")); } -TEST("require that optimization is disabled for tensors with non-double cells") { - TEST_DO(verify_not_optimized("map(_x5f,f(x)(x+10))")); +TEST("require that optimization works for float cells") { + TEST_DO(verify_optimized("map(_x5f,f(x)(x+10))", 1)); } TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/eval/src/tests/tensor/dense_remove_dimension_optimizer/dense_remove_dimension_optimizer_test.cpp b/eval/src/tests/tensor/dense_remove_dimension_optimizer/dense_remove_dimension_optimizer_test.cpp index 7856775ae30..179fdd3eff4 100644 --- a/eval/src/tests/tensor/dense_remove_dimension_optimizer/dense_remove_dimension_optimizer_test.cpp +++ b/eval/src/tests/tensor/dense_remove_dimension_optimizer/dense_remove_dimension_optimizer_test.cpp @@ -78,8 +78,8 @@ TEST("require that inappropriate tensor types cannot be optimized") { TEST_DO(verify_not_optimized("reduce(x1y5z_m,sum,z)")); } -TEST("require that optimization is disabled for tensors with non-double cells") { - TEST_DO(verify_not_optimized("reduce(x1y5z1f,avg,x)")); +TEST("require that optimization works for float cells") { + TEST_DO(verify_optimized("reduce(x1y5z1f,avg,x)")); } TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/eval/src/tests/tensor/dense_xw_product_function/dense_xw_product_function_test.cpp b/eval/src/tests/tensor/dense_xw_product_function/dense_xw_product_function_test.cpp index 335aa4791a4..426281686d7 100644 --- a/eval/src/tests/tensor/dense_xw_product_function/dense_xw_product_function_test.cpp +++ b/eval/src/tests/tensor/dense_xw_product_function/dense_xw_product_function_test.cpp @@ -45,6 +45,7 @@ EvalFixture::ParamRepo make_params() { .add("y1z1", spec({y(1),z(1)}, MyMatSeq())) .add("x2y3", spec({x(2),y(3)}, MyMatSeq())) .add("x2y3f", spec(float_cells({x(2),y(3)}), MyMatSeq())) + .add("y3z2f", spec(float_cells({y(3),z(2)}), MyMatSeq())) .add("x2z3", spec({x(2),z(3)}, MyMatSeq())) .add("y3z2", spec({y(3),z(2)}, MyMatSeq())) .add("x8y5", spec({x(8),y(5)}, MyMatSeq())) @@ -118,10 +119,16 @@ TEST("require that xw product can be debug dumped") { fprintf(stderr, "%s\n", info[0]->as_string().c_str()); } -TEST("require that optimization is disabled for tensors with non-double cells") { - TEST_DO(verify_not_optimized("reduce(y3f*x2y3,sum,y)")); - TEST_DO(verify_not_optimized("reduce(y3*x2y3f,sum,y)")); - TEST_DO(verify_not_optimized("reduce(y3f*x2y3f,sum,y)")); +TEST("require that optimization works for float cells") { + TEST_DO(verify_optimized("reduce(y3f*x2y3,sum,y)", 3, 2, true)); + TEST_DO(verify_optimized("reduce(y3*x2y3f,sum,y)", 3, 2, true)); + TEST_DO(verify_optimized("reduce(y3f*x2y3f,sum,y)", 3, 2, true)); +} + +TEST("require that optimization works for float cells with inconvenient dimension nesting") { + TEST_DO(verify_optimized("reduce(y3f*y3z2,sum,y)", 3, 2, false)); + TEST_DO(verify_optimized("reduce(y3*y3z2f,sum,y)", 3, 2, false)); + TEST_DO(verify_optimized("reduce(y3f*y3z2f,sum,y)", 3, 2, false)); } TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/eval/src/vespa/eval/eval/value_type.cpp b/eval/src/vespa/eval/eval/value_type.cpp index fc0f3cc5414..d6ba8e83855 100644 --- a/eval/src/vespa/eval/eval/value_type.cpp +++ b/eval/src/vespa/eval/eval/value_type.cpp @@ -12,21 +12,27 @@ using CellType = ValueType::CellType; using Dimension = ValueType::Dimension; using DimensionList = std::vector<Dimension>; -CellType unify(CellType a, CellType b) { - if (a == b) { - return a; - } else { - return CellType::DOUBLE; +template <typename A, typename B> +CellType unify() { + using type = typename UnifyCellTypes<A,B>::type; + return get_cell_type<type>(); +} + +template <typename A> +CellType unify(CellType b) { + switch (b) { + case CellType::DOUBLE: return unify<A,double>(); + case CellType::FLOAT: return unify<A,float>(); } + abort(); } -CellType unify_cell_type(const ValueType &a, const ValueType &b) { - if (a.is_double()) { - return b.cell_type(); - } else if (b.is_double()) { - return a.cell_type(); +CellType unify(CellType a, CellType b) { + switch (a) { + case CellType::DOUBLE: return unify<double>(b); + case CellType::FLOAT: return unify<float>(b); } - return unify(a.cell_type(), b.cell_type()); + abort(); } size_t my_dimension_index(const std::vector<Dimension> &list, const vespalib::string &name) { @@ -265,6 +271,16 @@ ValueType::join(const ValueType &lhs, const ValueType &rhs) return tensor_type(std::move(result.dimensions), unify(lhs._cell_type, rhs._cell_type)); } +CellType +ValueType::unify_cell_types(const ValueType &a, const ValueType &b) { + if (a.is_double()) { + return b.cell_type(); + } else if (b.is_double()) { + return a.cell_type(); + } + return unify(a.cell_type(), b.cell_type()); +} + ValueType ValueType::concat(const ValueType &lhs, const ValueType &rhs, const vespalib::string &dimension) { @@ -278,7 +294,7 @@ ValueType::concat(const ValueType &lhs, const ValueType &rhs, const vespalib::st if (!find_dimension(result.dimensions, dimension)) { result.dimensions.emplace_back(dimension, 2); } - return tensor_type(std::move(result.dimensions), unify_cell_type(lhs, rhs)); + return tensor_type(std::move(result.dimensions), unify_cell_types(lhs, rhs)); } ValueType diff --git a/eval/src/vespa/eval/eval/value_type.h b/eval/src/vespa/eval/eval/value_type.h index 0eb3e1ca28e..64003e2636e 100644 --- a/eval/src/vespa/eval/eval/value_type.h +++ b/eval/src/vespa/eval/eval/value_type.h @@ -78,15 +78,27 @@ public: static ValueType from_spec(const vespalib::string &spec); vespalib::string to_spec() const; static ValueType join(const ValueType &lhs, const ValueType &rhs); + static CellType unify_cell_types(const ValueType &a, const ValueType &b); static ValueType concat(const ValueType &lhs, const ValueType &rhs, const vespalib::string &dimension); static ValueType either(const ValueType &one, const ValueType &other); }; std::ostream &operator<<(std::ostream &os, const ValueType &type); -// utility template -template <typename T> inline bool check_cell_type(ValueType::CellType type); +// utility templates + +template <typename CT> inline bool check_cell_type(ValueType::CellType type); template <> inline bool check_cell_type<double>(ValueType::CellType type) { return (type == ValueType::CellType::DOUBLE); } template <> inline bool check_cell_type<float>(ValueType::CellType type) { return (type == ValueType::CellType::FLOAT); } +template <typename LCT, typename RCT> struct UnifyCellTypes{}; +template <> struct UnifyCellTypes<double, double> { using type = double; }; +template <> struct UnifyCellTypes<double, float> { using type = double; }; +template <> struct UnifyCellTypes<float, double> { using type = double; }; +template <> struct UnifyCellTypes<float, float> { using type = float; }; + +template <typename CT> inline ValueType::CellType get_cell_type(); +template <> inline ValueType::CellType get_cell_type<double>() { return ValueType::CellType::DOUBLE; } +template <> inline ValueType::CellType get_cell_type<float>() { return ValueType::CellType::FLOAT; } + } // namespace diff --git a/eval/src/vespa/eval/tensor/default_tensor_engine.cpp b/eval/src/vespa/eval/tensor/default_tensor_engine.cpp index 58db90f5557..f1eb9ff1523 100644 --- a/eval/src/vespa/eval/tensor/default_tensor_engine.cpp +++ b/eval/src/vespa/eval/tensor/default_tensor_engine.cpp @@ -37,6 +37,7 @@ using eval::TensorFunction; using eval::TensorSpec; using eval::Value; using eval::ValueType; +using CellType = eval::ValueType::CellType; using vespalib::IllegalArgumentException; using vespalib::make_string; @@ -355,8 +356,7 @@ DefaultTensorEngine::reduce(const Value &a, Aggr aggr, const std::vector<vespali size_t vector_size(const ValueType &type, const vespalib::string &dimension) { if (type.is_double()) { return 1; - } else if ((type.cell_type() == ValueType::CellType::DOUBLE) && - (type.dimensions().size() == 1) && + } else if ((type.dimensions().size() == 1) && (type.dimensions()[0].is_indexed()) && (type.dimensions()[0].name == dimension)) { @@ -366,40 +366,50 @@ size_t vector_size(const ValueType &type, const vespalib::string &dimension) { } } +template <typename OCT> struct CallAppendVector { template <typename CT> - static void call(const ConstArrayRef<CT> &arr, double *&pos) { - for (CT cell : arr) { *pos++ = cell; } + static void call(const ConstArrayRef<CT> &arr, OCT *&pos) { + for (CT cell: arr) { *pos++ = cell; } } }; -void append_vector(double *&pos, const Value &value) { +template <typename OCT> +void append_vector(OCT *&pos, const Value &value) { if (auto tensor = value.as_tensor()) { const DenseTensorView *view = static_cast<const DenseTensorView *>(tensor); - TypedCells cellsRef = view->cellsRef(); - dispatch_1<CallAppendVector>(cellsRef, pos); + dispatch_1<CallAppendVector<OCT> >(view->cellsRef(), pos); } else { *pos++ = value.as_double(); } } +template <typename OCT> const Value &concat_vectors(const Value &a, const Value &b, const vespalib::string &dimension, size_t vector_size, Stash &stash) { - ArrayRef<double> cells = stash.create_array<double>(vector_size); - double *pos = cells.begin(); - append_vector(pos, a); - append_vector(pos, b); + ArrayRef<OCT> cells = stash.create_array<OCT>(vector_size); + OCT *pos = cells.begin(); + append_vector<OCT>(pos, a); + append_vector<OCT>(pos, b); assert(pos == cells.end()); - const ValueType &type = stash.create<ValueType>(ValueType::tensor_type({ValueType::Dimension(dimension, vector_size)})); + const ValueType &type = stash.create<ValueType>(ValueType::tensor_type({ValueType::Dimension(dimension, vector_size)}, ValueType::unify_cell_types(a.type(), b.type()))); return stash.create<DenseTensorView>(type, TypedCells(cells)); } +struct CallConcatVectors { + template <typename OCT> + static const Value &call(const Value &a, const Value &b, const vespalib::string &dimension, size_t vector_size, Stash &stash) { + return concat_vectors<OCT>(a, b, dimension, vector_size, stash); + } +}; + const Value & DefaultTensorEngine::concat(const Value &a, const Value &b, const vespalib::string &dimension, Stash &stash) const { size_t a_size = vector_size(a.type(), dimension); size_t b_size = vector_size(b.type(), dimension); if ((a_size > 0) && (b_size > 0)) { - return concat_vectors(a, b, dimension, a_size + b_size, stash); + CellType result_cell_type = ValueType::unify_cell_types(a.type(), b.type()); + return dispatch_0<CallConcatVectors>(result_cell_type, a, b, dimension, (a_size + b_size), stash); } return to_default(simple_engine().concat(to_simple(a, stash), to_simple(b, stash), dimension, stash), stash); } diff --git a/eval/src/vespa/eval/tensor/dense/dense_add_dimension_optimizer.cpp b/eval/src/vespa/eval/tensor/dense/dense_add_dimension_optimizer.cpp index 842e064de43..a4331b6b251 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_add_dimension_optimizer.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_add_dimension_optimizer.cpp @@ -19,21 +19,8 @@ using namespace eval::operation; namespace { -bool is_concrete_dense_tensor(const ValueType &type) { - if (type.cell_type() != ValueType::CellType::DOUBLE) { - return false; // non-double cell types not supported - } - return type.is_dense(); -} - -bool not_overlapping(const ValueType &a, const ValueType &b) { - size_t npos = ValueType::Dimension::npos; - for (const auto &dim: b.dimensions()) { - if (a.dimension_index(dim.name) != npos) { - return false; - } - } - return true; +bool same_cell_type(const TensorFunction &a, const TensorFunction &b) { + return (a.result_type().cell_type() == b.result_type().cell_type()); } bool is_unit_constant(const TensorFunction &node) { @@ -57,15 +44,14 @@ DenseAddDimensionOptimizer::optimize(const eval::TensorFunction &expr, Stash &st const TensorFunction &lhs = join->lhs(); const TensorFunction &rhs = join->rhs(); if ((join->function() == Mul::f) && - is_concrete_dense_tensor(lhs.result_type()) && - is_concrete_dense_tensor(rhs.result_type()) && - not_overlapping(lhs.result_type(), rhs.result_type())) + lhs.result_type().is_dense() && + rhs.result_type().is_dense()) { - if (is_unit_constant(lhs)) { + if (is_unit_constant(lhs) && same_cell_type(rhs, expr)) { return DenseReplaceTypeFunction::create_compact(expr.result_type(), rhs, stash); } - if (is_unit_constant(rhs)) { - return DenseReplaceTypeFunction::create_compact(expr.result_type(), lhs, stash); + if (is_unit_constant(rhs) && same_cell_type(lhs, expr)) { + return DenseReplaceTypeFunction::create_compact(expr.result_type(), lhs, stash); } } } diff --git a/eval/src/vespa/eval/tensor/dense/dense_dot_product_function.cpp b/eval/src/vespa/eval/tensor/dense/dense_dot_product_function.cpp index 9b839e1b12f..8bcaddba3b4 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_dot_product_function.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_dot_product_function.cpp @@ -18,12 +18,6 @@ using namespace eval::operation; namespace { -template <typename T> -ConstArrayRef<T> getCellsRef(const eval::Value &value) { - const DenseTensorView &denseTensor = static_cast<const DenseTensorView &>(value); - return denseTensor.cellsRef().typify<T>(); -} - template <typename LCT, typename RCT> struct HWSupport { static double call(hwaccelrated::IAccelrated *, const ConstArrayRef<LCT> &lhs, const ConstArrayRef<RCT> &rhs) { @@ -48,8 +42,8 @@ template <> struct HWSupport<double, double> { template <typename LCT, typename RCT> void my_dot_product_op(eval::InterpretedFunction::State &state, uint64_t param) { auto *hw = (hwaccelrated::IAccelrated *)(param); - auto lhs = getCellsRef<LCT>(state.peek(1)); - auto rhs = getCellsRef<RCT>(state.peek(0)); + auto lhs = DenseTensorView::typify_cells<LCT>(state.peek(1)); + auto rhs = DenseTensorView::typify_cells<RCT>(state.peek(0)); double result = HWSupport<LCT,RCT>::call(hw, lhs, rhs); state.pop_pop_push(state.stash.create<eval::DoubleValue>(result)); } diff --git a/eval/src/vespa/eval/tensor/dense/dense_fast_rename_optimizer.cpp b/eval/src/vespa/eval/tensor/dense/dense_fast_rename_optimizer.cpp index d8e1876ac64..ac8442477e4 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_fast_rename_optimizer.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_fast_rename_optimizer.cpp @@ -17,15 +17,10 @@ using namespace eval::tensor_function; namespace { -bool is_concrete_dense_stable_rename(const ValueType &from_type, const ValueType &to_type, - const std::vector<vespalib::string> &from, - const std::vector<vespalib::string> &to) +bool is_dense_stable_rename(const ValueType &from_type, const ValueType &to_type, + const std::vector<vespalib::string> &from, + const std::vector<vespalib::string> &to) { - if (from_type.cell_type() != ValueType::CellType::DOUBLE || - to_type.cell_type() != ValueType::CellType::DOUBLE) - { - return false; // non-double cell types not supported - } if (!from_type.is_dense() || !to_type.is_dense() || (from.size() != to.size())) @@ -51,7 +46,8 @@ DenseFastRenameOptimizer::optimize(const eval::TensorFunction &expr, Stash &stas if (auto rename = as<Rename>(expr)) { const ValueType &from_type = rename->child().result_type(); const ValueType &to_type = expr.result_type(); - if (is_concrete_dense_stable_rename(from_type, to_type, rename->from(), rename->to())) { + if (is_dense_stable_rename(from_type, to_type, rename->from(), rename->to())) { + assert(to_type.cell_type() == from_type.cell_type()); return DenseReplaceTypeFunction::create_compact(to_type, rename->child(), stash); } } diff --git a/eval/src/vespa/eval/tensor/dense/dense_generic_join.hpp b/eval/src/vespa/eval/tensor/dense/dense_generic_join.hpp index aa08e6982bb..cdc89b30fff 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_generic_join.hpp +++ b/eval/src/vespa/eval/tensor/dense/dense_generic_join.hpp @@ -43,7 +43,7 @@ struct CallGenericJoin { DenseDimensionCombiner & combiner, Function &&func) { - using OCT = typename OutputCellType<LCT, RCT>::output_type; + using OCT = typename eval::UnifyCellTypes<LCT, RCT>::type; TypedDenseTensorBuilder<OCT> builder(combiner.result_type); return generic_join(combiner, builder, lhsArr, rhsArr, std::move(func)); } diff --git a/eval/src/vespa/eval/tensor/dense/dense_inplace_join_function.cpp b/eval/src/vespa/eval/tensor/dense/dense_inplace_join_function.cpp index 5fdfdbc4e9f..0b5bba88d37 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_inplace_join_function.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_inplace_join_function.cpp @@ -17,35 +17,45 @@ using namespace eval::tensor_function; namespace { -TypedCells getCellsRef(const eval::Value &value) { - const DenseTensorView &denseTensor = static_cast<const DenseTensorView &>(value); - return denseTensor.cellsRef(); +template <typename LCT, typename RCT> +void my_inplace_join_left_op(eval::InterpretedFunction::State &state, uint64_t param) { + join_fun_t function = (join_fun_t)param; + auto lhs_cells = unconstify(DenseTensorView::typify_cells<LCT>(state.peek(1))); + auto rhs_cells = DenseTensorView::typify_cells<RCT>(state.peek(0)); + for (size_t i = 0; i < lhs_cells.size(); ++i) { + lhs_cells[i] = function(lhs_cells[i], rhs_cells[i]); + } + state.stack.pop_back(); } -template <bool write_left> -void my_inplace_join_op(eval::InterpretedFunction::State &state, uint64_t param) { +template <typename LCT, typename RCT> +void my_inplace_join_right_op(eval::InterpretedFunction::State &state, uint64_t param) { join_fun_t function = (join_fun_t)param; - ConstArrayRef<double> lhs_cells = getCellsRef(state.peek(1)).typify<double>(); - ConstArrayRef<double> rhs_cells = getCellsRef(state.peek(0)).typify<double>(); - auto dst_cells = unconstify(write_left ? lhs_cells : rhs_cells); - for (size_t i = 0; i < dst_cells.size(); ++i) { - dst_cells[i] = function(lhs_cells[i], rhs_cells[i]); - } - if (write_left) { - state.stack.pop_back(); - } else { - const Value &result = state.stack.back(); - state.pop_pop_push(result); + auto lhs_cells = DenseTensorView::typify_cells<LCT>(state.peek(1)); + auto rhs_cells = unconstify(DenseTensorView::typify_cells<RCT>(state.peek(0))); + for (size_t i = 0; i < rhs_cells.size(); ++i) { + rhs_cells[i] = function(lhs_cells[i], rhs_cells[i]); } + const Value &result = state.stack.back(); + state.pop_pop_push(result); } -bool sameShapeConcreteDenseTensors(const ValueType &a, const ValueType &b) { - if (a.cell_type() != ValueType::CellType::DOUBLE || - b.cell_type() != ValueType::CellType::DOUBLE) - { - return false; // non-double cell types not supported +struct MyInplaceJoinLeftOp { + template <typename LCT, typename RCT> + static auto get_fun() { return my_inplace_join_left_op<LCT,RCT>; } +}; + +struct MyInplaceJoinRightOp { + template <typename LCT, typename RCT> + static auto get_fun() { return my_inplace_join_right_op<LCT,RCT>; } +}; + +eval::InterpretedFunction::op_function my_select(CellType lct, CellType rct, bool write_left) { + if (write_left) { + return select_2<MyInplaceJoinLeftOp>(lct, rct); + } else { + return select_2<MyInplaceJoinRightOp>(lct, rct); } - return (a.is_dense() && (a == b)); } } // namespace vespalib::tensor::<unnamed> @@ -68,7 +78,8 @@ DenseInplaceJoinFunction::~DenseInplaceJoinFunction() eval::InterpretedFunction::Instruction DenseInplaceJoinFunction::compile_self(Stash &) const { - auto op = _write_left ? my_inplace_join_op<true> : my_inplace_join_op<false>; + auto op = my_select(lhs().result_type().cell_type(), + rhs().result_type().cell_type(), _write_left); return eval::InterpretedFunction::Instruction(op, (uint64_t)function()); } @@ -85,11 +96,17 @@ DenseInplaceJoinFunction::optimize(const eval::TensorFunction &expr, Stash &stas if (auto join = as<Join>(expr)) { const TensorFunction &lhs = join->lhs(); const TensorFunction &rhs = join->rhs(); - if ((lhs.result_is_mutable() || rhs.result_is_mutable()) && - sameShapeConcreteDenseTensors(lhs.result_type(), rhs.result_type())) + if (lhs.result_type().is_dense() && + (lhs.result_type().dimensions() == rhs.result_type().dimensions())) { - return stash.create<DenseInplaceJoinFunction>(join->result_type(), lhs, rhs, - join->function(), lhs.result_is_mutable()); + if (lhs.result_is_mutable() && (lhs.result_type() == expr.result_type())) { + return stash.create<DenseInplaceJoinFunction>(join->result_type(), lhs, rhs, + join->function(), /* write left: */ true); + } + if (rhs.result_is_mutable() && (rhs.result_type() == expr.result_type())) { + return stash.create<DenseInplaceJoinFunction>(join->result_type(), lhs, rhs, + join->function(), /* write left: */ false); + } } } return expr; diff --git a/eval/src/vespa/eval/tensor/dense/dense_inplace_map_function.cpp b/eval/src/vespa/eval/tensor/dense/dense_inplace_map_function.cpp index b38a6b175dc..c82cda34a28 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_inplace_map_function.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_inplace_map_function.cpp @@ -16,24 +16,19 @@ using namespace eval::tensor_function; namespace { -ArrayRef<double> getMutableCells(const eval::Value &value) { - const DenseTensorView &denseTensor = static_cast<const DenseTensorView &>(value); - return unconstify(denseTensor.cellsRef().typify<double>()); -} - +template <typename CT> void my_inplace_map_op(eval::InterpretedFunction::State &state, uint64_t param) { map_fun_t function = (map_fun_t)param; - for (double &cell: getMutableCells(state.peek(0))) { + ArrayRef<CT> cells = unconstify(DenseTensorView::typify_cells<CT>(state.peek(0))); + for (CT &cell: cells) { cell = function(cell); } } -bool isConcreteDenseTensor(const ValueType &type) { - if (type.cell_type() != ValueType::CellType::DOUBLE) { - return false; // non-double cell types not supported - } - return type.is_dense(); -} +struct MyInplaceMapOp { + template <typename CT> + static auto get_fun() { return my_inplace_map_op<CT>; } +}; } // namespace vespalib::tensor::<unnamed> @@ -51,14 +46,16 @@ DenseInplaceMapFunction::~DenseInplaceMapFunction() eval::InterpretedFunction::Instruction DenseInplaceMapFunction::compile_self(Stash &) const { - return eval::InterpretedFunction::Instruction(my_inplace_map_op, (uint64_t)function()); + auto op = select_1<MyInplaceMapOp>(result_type().cell_type()); + return eval::InterpretedFunction::Instruction(op, (uint64_t)function()); } const TensorFunction & DenseInplaceMapFunction::optimize(const eval::TensorFunction &expr, Stash &stash) { if (auto map = as<Map>(expr)) { - if (map->child().result_is_mutable() && isConcreteDenseTensor(map->result_type())) { + if (map->child().result_is_mutable() && map->result_type().is_dense()) { + assert(map->result_type().cell_type() == map->child().result_type().cell_type()); return stash.create<DenseInplaceMapFunction>(map->result_type(), map->child(), map->function()); } } diff --git a/eval/src/vespa/eval/tensor/dense/dense_remove_dimension_optimizer.cpp b/eval/src/vespa/eval/tensor/dense/dense_remove_dimension_optimizer.cpp index 3c58320a6e6..a64d5edbb37 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_remove_dimension_optimizer.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_remove_dimension_optimizer.cpp @@ -14,13 +14,6 @@ using namespace eval::tensor_function; namespace { -bool is_concrete_dense_tensor(const ValueType &type) { - if (type.cell_type() != ValueType::CellType::DOUBLE) { - return false; // non-double cell types not supported - } - return type.is_dense(); -} - bool is_ident_aggr(Aggr aggr) { return ((aggr == Aggr::AVG) || (aggr == Aggr::PROD) || @@ -47,11 +40,12 @@ DenseRemoveDimensionOptimizer::optimize(const eval::TensorFunction &expr, Stash { if (auto reduce = as<Reduce>(expr)) { const TensorFunction &child = reduce->child(); - if (is_concrete_dense_tensor(expr.result_type()) && - is_concrete_dense_tensor(child.result_type()) && + if (expr.result_type().is_dense() && + child.result_type().is_dense() && is_ident_aggr(reduce->aggr()) && is_trivial_dim_list(child.result_type(), reduce->dimensions())) { + assert(expr.result_type().cell_type() == child.result_type().cell_type()); return DenseReplaceTypeFunction::create_compact(expr.result_type(), child, stash); } } diff --git a/eval/src/vespa/eval/tensor/dense/dense_tensor_view.cpp b/eval/src/vespa/eval/tensor/dense/dense_tensor_view.cpp index d98cf52d279..3fed84323ca 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_tensor_view.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_tensor_view.cpp @@ -95,8 +95,7 @@ sameShapeJoin(const ConstArrayRef<LCT> &lhs, const ConstArrayRef<RCT> &rhs, { size_t sz = lhs.size(); assert(sz == rhs.size()); - using OutputSelector = OutputCellType<LCT, RCT>; - using OCT = typename OutputSelector::output_type; + using OCT = typename eval::UnifyCellTypes<LCT,RCT>::type; std::vector<OCT> newCells; newCells.reserve(sz); auto rhsCellItr = rhs.cbegin(); @@ -107,7 +106,7 @@ sameShapeJoin(const ConstArrayRef<LCT> &lhs, const ConstArrayRef<RCT> &rhs, } assert(rhsCellItr == rhs.cend()); assert(newCells.size() == sz); - auto newType = eval::ValueType::tensor_type(lhs_type.dimensions(), OutputSelector::output_cell_type()); + auto newType = eval::ValueType::tensor_type(lhs_type.dimensions(), eval::get_cell_type<OCT>()); return std::make_unique<DenseTensor<OCT>>(std::move(newType), std::move(newCells)); } diff --git a/eval/src/vespa/eval/tensor/dense/dense_tensor_view.h b/eval/src/vespa/eval/tensor/dense/dense_tensor_view.h index 1ec4daf40fd..778f2aa2871 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_tensor_view.h +++ b/eval/src/vespa/eval/tensor/dense/dense_tensor_view.h @@ -42,6 +42,13 @@ public: Tensor::UP clone() const override; eval::TensorSpec toSpec() const override; void accept(TensorVisitor &visitor) const override; + + template <typename T> static ConstArrayRef<T> typify_cells(const eval::Value &self) { + return static_cast<const DenseTensorView &>(self).cellsRef().typify<T>(); + } + template <typename T> static ConstArrayRef<T> unsafe_typify_cells(const eval::Value &self) { + return static_cast<const DenseTensorView &>(self).cellsRef().unsafe_typify<T>(); + } protected: explicit DenseTensorView(const eval::ValueType &type_in) : _typeRef(type_in), diff --git a/eval/src/vespa/eval/tensor/dense/dense_xw_product_function.cpp b/eval/src/vespa/eval/tensor/dense/dense_xw_product_function.cpp index b6ac87ce012..2db5b4e8f92 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_xw_product_function.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_xw_product_function.cpp @@ -21,21 +21,36 @@ using namespace eval::operation; namespace { -XWInput getCellsRef(const eval::Value &value) { - const DenseTensorView &denseTensor = static_cast<const DenseTensorView &>(value); - TypedCells ref = denseTensor.cellsRef(); - assert(ref.type == CellType::DOUBLE); - return ref.typify<double>(); -} +template <typename LCT, typename RCT> +struct HWSupport { + static double call(hwaccelrated::IAccelrated *, const LCT *lhs, const RCT *rhs, size_t len) { + double result = 0.0; + for (size_t i = 0; i < len; ++i) { + result += (lhs[i] * rhs[i]); + } + return result; + } +}; +template <> struct HWSupport<float, float> { + static double call(hwaccelrated::IAccelrated *hw, const float *lhs, const float *rhs, size_t len) { + return hw->dotProduct(lhs, rhs, len); + } +}; +template <> struct HWSupport<double, double> { + static double call(hwaccelrated::IAccelrated *hw, const double *lhs, const double *rhs, size_t len) { + return hw->dotProduct(lhs, rhs, len); + } +}; +template <typename LCT, typename RCT, typename OCT> void multiDotProduct(const DenseXWProductFunction::Self &self, - const XWInput &vectorCells, const XWInput &matrixCells, XWOutput &result) + const ConstArrayRef<LCT> &vectorCells, const ConstArrayRef<RCT> &matrixCells, ArrayRef<OCT> &result) { - double *out = result.begin(); - const double *matrixP = matrixCells.cbegin(); - const double * const vectorP = vectorCells.cbegin(); + OCT *out = result.begin(); + const RCT *matrixP = matrixCells.cbegin(); + const LCT * const vectorP = vectorCells.cbegin(); for (size_t row = 0; row < self._resultSize; ++row) { - double cell = self._hwAccelerator->dotProduct(vectorP, matrixP, self._vectorSize); + double cell = HWSupport<LCT,RCT>::call(self._hwAccelerator.get(), vectorP, matrixP, self._vectorSize); *out++ = cell; matrixP += self._vectorSize; } @@ -43,12 +58,13 @@ void multiDotProduct(const DenseXWProductFunction::Self &self, assert(matrixP == matrixCells.cend()); } +template <typename LCT, typename RCT, typename OCT> void transposedProduct(const DenseXWProductFunction::Self &self, - const XWInput &vectorCells, const XWInput &matrixCells, XWOutput &result) + const ConstArrayRef<LCT> &vectorCells, const ConstArrayRef<RCT> &matrixCells, ArrayRef<OCT> &result) { - double *out = result.begin(); - const double * const matrixP = matrixCells.cbegin(); - const double * const vectorP = vectorCells.cbegin(); + OCT *out = result.begin(); + const RCT * const matrixP = matrixCells.cbegin(); + const LCT * const vectorP = vectorCells.cbegin(); for (size_t row = 0; row < self._resultSize; ++row) { double cell = 0; for (size_t col = 0; col < self._vectorSize; ++col) { @@ -59,41 +75,54 @@ void transposedProduct(const DenseXWProductFunction::Self &self, assert(out == result.end()); } -template <bool commonDimensionInnermost> +template <typename LCT, typename RCT, bool commonDimensionInnermost> void my_xw_product_op(eval::InterpretedFunction::State &state, uint64_t param) { DenseXWProductFunction::Self *self = (DenseXWProductFunction::Self *)(param); - XWInput vectorCells = getCellsRef(state.peek(1)); - XWInput matrixCells = getCellsRef(state.peek(0)); - - ArrayRef<double> outputCells = state.stash.create_array<double>(self->_resultSize); + using OCT = typename eval::UnifyCellTypes<LCT,RCT>::type; + auto vectorCells = DenseTensorView::typify_cells<LCT>(state.peek(1)); + auto matrixCells = DenseTensorView::typify_cells<RCT>(state.peek(0)); + auto outputCells = state.stash.create_array<OCT>(self->_resultSize); if (commonDimensionInnermost) { multiDotProduct(*self, vectorCells, matrixCells, outputCells); } else { transposedProduct(*self, vectorCells, matrixCells, outputCells); } + state.pop_pop_push(state.stash.create<DenseTensorView>(self->_resultType, TypedCells(outputCells))); } -bool isConcreteDenseTensor(const ValueType &type, size_t d) { - if (type.cell_type() != ValueType::CellType::DOUBLE) { - return false; // non-double cell types not supported +template <bool common_inner> +struct MyXWProductOp { + template <typename LCT, typename RCT> + static auto get_fun() { return my_xw_product_op<LCT,RCT,common_inner>; } +}; + +eval::InterpretedFunction::op_function my_select(CellType lct, CellType rct, bool common_innermost) { + if (common_innermost) { + return select_2<MyXWProductOp<true> >(lct, rct); + } else { + return select_2<MyXWProductOp<false> >(lct, rct); } +} + +bool isDenseTensor(const ValueType &type, size_t d) { return (type.is_dense() && (type.dimensions().size() == d)); } bool isDenseXWProduct(const ValueType &res, const ValueType &vec, const ValueType &mat) { - if (isConcreteDenseTensor(res, 1) && - isConcreteDenseTensor(vec, 1) && - isConcreteDenseTensor(mat, 2)) + if (isDenseTensor(res, 1) && + isDenseTensor(vec, 1) && + isDenseTensor(mat, 2)) { size_t res_idx = mat.dimension_index(res.dimensions()[0].name); size_t vec_idx = mat.dimension_index(vec.dimensions()[0].name); size_t npos = ValueType::Dimension::npos; if ((res_idx != npos) && (vec_idx != npos) && (res_idx != vec_idx)) { - return ((mat.dimensions()[res_idx].size == res.dimensions()[0].size) && - (mat.dimensions()[vec_idx].size == vec.dimensions()[0].size)); + assert(mat.dimensions()[res_idx].size == res.dimensions()[0].size); + assert(mat.dimensions()[vec_idx].size == vec.dimensions()[0].size); + return true; } } return false; @@ -134,7 +163,8 @@ eval::InterpretedFunction::Instruction DenseXWProductFunction::compile_self(Stash &stash) const { Self &self = stash.create<Self>(result_type(), _vectorSize, _resultSize); - auto op = _commonDimensionInnermost ? my_xw_product_op<true> : my_xw_product_op<false>; + auto op = my_select(lhs().result_type().cell_type(), + rhs().result_type().cell_type(), _commonDimensionInnermost); return eval::InterpretedFunction::Instruction(op, (uint64_t)(&self)); } @@ -150,22 +180,22 @@ DenseXWProductFunction::visit_self(vespalib::ObjectVisitor &visitor) const const TensorFunction & DenseXWProductFunction::optimize(const eval::TensorFunction &expr, Stash &stash) { - const Reduce *reduce = as<Reduce>(expr); - if (reduce && (reduce->aggr() == Aggr::SUM)) { - const ValueType &result_type = reduce->result_type(); - const Join *join = as<Join>(reduce->child()); - if (join && (join->function() == Mul::f)) { - const TensorFunction &lhs = join->lhs(); - const TensorFunction &rhs = join->rhs(); - if (isDenseXWProduct(result_type, lhs.result_type(), rhs.result_type())) { - return createDenseXWProduct(result_type, lhs, rhs, stash); - } - if (isDenseXWProduct(result_type, rhs.result_type(), lhs.result_type())) { - return createDenseXWProduct(result_type, rhs, lhs, stash); - } + const Reduce *reduce = as<Reduce>(expr); + if (reduce && (reduce->aggr() == Aggr::SUM)) { + const ValueType &result_type = reduce->result_type(); + const Join *join = as<Join>(reduce->child()); + if (join && (join->function() == Mul::f)) { + const TensorFunction &lhs = join->lhs(); + const TensorFunction &rhs = join->rhs(); + if (isDenseXWProduct(result_type, lhs.result_type(), rhs.result_type())) { + return createDenseXWProduct(result_type, lhs, rhs, stash); + } + if (isDenseXWProduct(result_type, rhs.result_type(), lhs.result_type())) { + return createDenseXWProduct(result_type, rhs, lhs, stash); } } - return expr; + } + return expr; } } // namespace vespalib::tensor diff --git a/eval/src/vespa/eval/tensor/dense/dense_xw_product_function.h b/eval/src/vespa/eval/tensor/dense/dense_xw_product_function.h index 9f1bc12b110..f2f4d67c0f0 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_xw_product_function.h +++ b/eval/src/vespa/eval/tensor/dense/dense_xw_product_function.h @@ -8,9 +8,6 @@ namespace vespalib::tensor { -using XWInput = ConstArrayRef<double>; -using XWOutput = ArrayRef<double>; - /** * Tensor function for product of one 1-dimensional and one 2-dimensional dense tensor. */ diff --git a/eval/src/vespa/eval/tensor/dense/typed_cells.h b/eval/src/vespa/eval/tensor/dense/typed_cells.h index 98f95d54d9b..0f22c85735e 100644 --- a/eval/src/vespa/eval/tensor/dense/typed_cells.h +++ b/eval/src/vespa/eval/tensor/dense/typed_cells.h @@ -12,25 +12,6 @@ namespace vespalib::tensor { using CellType = vespalib::eval::ValueType::CellType; - -template<typename LCT, typename RCT> struct OutputCellType; -template<> struct OutputCellType<double, double> { - typedef double output_type; - static constexpr CellType output_cell_type() { return CellType::DOUBLE; }; -}; -template<> struct OutputCellType<float, double> { - typedef double output_type; - static constexpr CellType output_cell_type() { return CellType::DOUBLE; }; -}; -template<> struct OutputCellType<double, float> { - typedef double output_type; - static constexpr CellType output_cell_type() { return CellType::DOUBLE; }; -}; -template<> struct OutputCellType<float, float> { - typedef float output_type; - static constexpr CellType output_cell_type() { return CellType::FLOAT; }; -}; - struct TypedCells { const void *data; CellType type; @@ -67,7 +48,7 @@ struct TypedCells { }; template <typename TGT, typename... Args> -auto dispatch_0(CellType ct, Args &&...args) { +decltype(auto) dispatch_0(CellType ct, Args &&...args) { switch (ct) { case CellType::DOUBLE: return TGT::template call<double>(std::forward<Args>(args)...); case CellType::FLOAT: return TGT::template call<float>(std::forward<Args>(args)...); @@ -76,7 +57,7 @@ auto dispatch_0(CellType ct, Args &&...args) { } template <typename TGT, typename... Args> -auto dispatch_1(const TypedCells &a, Args &&...args) { +decltype(auto) dispatch_1(const TypedCells &a, Args &&...args) { switch (a.type) { case CellType::DOUBLE: return TGT::call(a.unsafe_typify<double>(), std::forward<Args>(args)...); case CellType::FLOAT: return TGT::call(a.unsafe_typify<float>(), std::forward<Args>(args)...); @@ -85,7 +66,7 @@ auto dispatch_1(const TypedCells &a, Args &&...args) { } template <typename TGT, typename A1, typename... Args> -auto dispatch_2(A1 &&a, const TypedCells &b, Args &&...args) { +decltype(auto) dispatch_2(A1 &&a, const TypedCells &b, Args &&...args) { switch (b.type) { case CellType::DOUBLE: return dispatch_1<TGT>(std::forward<A1>(a), b.unsafe_typify<double>(), std::forward<Args>(args)...); case CellType::FLOAT: return dispatch_1<TGT>(std::forward<A1>(a), b.unsafe_typify<float>(), std::forward<Args>(args)...); @@ -94,7 +75,7 @@ auto dispatch_2(A1 &&a, const TypedCells &b, Args &&...args) { } template <typename T, typename... Args> -auto select_1(CellType a_type) { +decltype(auto) select_1(CellType a_type) { switch(a_type) { case CellType::DOUBLE: return T::template get_fun<double, Args...>(); case CellType::FLOAT: return T::template get_fun<float, Args...>(); @@ -103,7 +84,7 @@ auto select_1(CellType a_type) { } template <typename T> -auto select_2(CellType a_type, CellType b_type) { +decltype(auto) select_2(CellType a_type, CellType b_type) { switch(b_type) { case CellType::DOUBLE: return select_1<T, double>(a_type); case CellType::FLOAT: return select_1<T, float>(a_type); diff --git a/fbench/src/fbench/fbench.cpp b/fbench/src/fbench/fbench.cpp index 205dc867950..723980cd1c7 100644 --- a/fbench/src/fbench/fbench.cpp +++ b/fbench/src/fbench/fbench.cpp @@ -63,13 +63,18 @@ FBench::~FBench() bool FBench::init_crypto_engine(const std::string &ca_certs_file_name, const std::string &cert_chain_file_name, - const std::string &private_key_file_name) + const std::string &private_key_file_name, + bool allow_default_tls) { if (ca_certs_file_name.empty() && cert_chain_file_name.empty() && private_key_file_name.empty()) { - _crypto_engine = std::make_shared<vespalib::NullCryptoEngine>(); + if (allow_default_tls) { + _crypto_engine = vespalib::CryptoEngine::get_default(); + } else { + _crypto_engine = std::make_shared<vespalib::NullCryptoEngine>(); + } return true; } if (ca_certs_file_name.empty()) { @@ -297,7 +302,8 @@ FBench::Usage() printf(" -z : use single query file to be distributed between clients.\n"); printf(" -T <str> : CA certificate file to verify peer against.\n"); printf(" -C <str> : client certificate file name.\n"); - printf(" -K <str> : client private key file name.\n\n"); + printf(" -K <str> : client private key file name.\n"); + printf(" -D : use TLS configuration from environment if T/C/K is not used\n\n"); printf(" <hostname> : the host you want to benchmark.\n"); printf(" <port> : the port to use when contacting the host.\n\n"); printf("Several hostnames and ports can be listed\n"); @@ -332,6 +338,7 @@ FBench::Main(int argc, char *argv[]) std::string ca_certs_file_name; // -T std::string cert_chain_file_name; // -C std::string private_key_file_name; // -K + bool allow_default_tls = false; // -D int restartLimit = -1; bool keepAlive = true; @@ -351,7 +358,7 @@ FBench::Main(int argc, char *argv[]) idx = 1; optError = false; - while((opt = GetOpt(argc, argv, "H:A:T:C:K:a:n:c:l:i:s:q:o:r:m:p:kxyzP", arg, idx)) != -1) { + while((opt = GetOpt(argc, argv, "H:A:T:C:K:Da:n:c:l:i:s:q:o:r:m:p:kxyzP", arg, idx)) != -1) { switch(opt) { case 'A': authority = arg; @@ -372,6 +379,9 @@ FBench::Main(int argc, char *argv[]) case 'K': private_key_file_name = std::string(arg); break; + case 'D': + allow_default_tls = true; + break; case 'a': queryStringToAppend = std::string(arg); break; @@ -443,7 +453,7 @@ FBench::Main(int argc, char *argv[]) return -1; } - if (!init_crypto_engine(ca_certs_file_name, cert_chain_file_name, private_key_file_name)) { + if (!init_crypto_engine(ca_certs_file_name, cert_chain_file_name, private_key_file_name, allow_default_tls)) { fprintf(stderr, "failed to initialize crypto engine\n"); return -1; } diff --git a/fbench/src/fbench/fbench.h b/fbench/src/fbench/fbench.h index 8cbab2e6d6c..e4a8e4e0b27 100644 --- a/fbench/src/fbench/fbench.h +++ b/fbench/src/fbench/fbench.h @@ -35,7 +35,8 @@ private: bool init_crypto_engine(const std::string &ca_certs_file_name, const std::string &cert_chain_file_name, - const std::string &private_key_file_name); + const std::string &private_key_file_name, + bool allow_default_tls); void InitBenchmark(int numClients, int ignoreCount, int cycle, const char *filenamePattern, const char *outputPattern, diff --git a/flags/pom.xml b/flags/pom.xml index 7ef082cc1bc..c1e9eca20ab 100644 --- a/flags/pom.xml +++ b/flags/pom.xml @@ -59,18 +59,7 @@ <classifier>no_aop</classifier> <scope>provided</scope> </dependency> - <dependency> - <groupId>com.yahoo.vespa</groupId> - <artifactId>container-dev</artifactId> - <version>${project.version}</version> - <scope>provided</scope> - </dependency> - <dependency> - <groupId>com.yahoo.vespa</groupId> - <artifactId>zkfacade</artifactId> - <version>${project.version}</version> - <scope>provided</scope> - </dependency> + <dependency> <groupId>junit</groupId> <artifactId>junit</artifactId> 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 c160bf4dc4e..e1111822b90 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -135,7 +135,7 @@ public class Flags { HOSTNAME); public static final UnboundStringFlag CONFIGSERVER_RPC_AUTHORIZER = defineStringFlag( - "configserver-rpc-authorizer", "log-only", + "configserver-rpc-authorizer", "enforce", "Configserver RPC authorizer. Allowed values: ['disable', 'log-only', 'enforce']", "Takes effect on restart of configserver"); diff --git a/jaxrs_client_utils/pom.xml b/jaxrs_client_utils/pom.xml index 636fbab7bb0..d32d4c5eccc 100644 --- a/jaxrs_client_utils/pom.xml +++ b/jaxrs_client_utils/pom.xml @@ -16,6 +16,7 @@ <packaging>container-plugin</packaging> <name>${project.artifactId}</name> <dependencies> + <!-- provided --> <dependency> <groupId>com.yahoo.vespa</groupId> <artifactId>vespajlib</artifactId> @@ -29,6 +30,12 @@ <scope>provided</scope> </dependency> <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>security-utils</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + </dependency> + <dependency> <groupId>javax.ws.rs</groupId> <artifactId>javax.ws.rs-api</artifactId> <version>2.0</version> @@ -44,6 +51,8 @@ <artifactId>jersey-proxy-client</artifactId> <scope>provided</scope> </dependency> + + <!-- test --> <dependency> <groupId>com.yahoo.vespa</groupId> <artifactId>jaxrs_utils</artifactId> diff --git a/jaxrs_client_utils/src/main/java/ai/vespa/util/http/VespaClientBuilderFactory.java b/jaxrs_client_utils/src/main/java/ai/vespa/util/http/VespaClientBuilderFactory.java new file mode 100644 index 00000000000..d55128069c4 --- /dev/null +++ b/jaxrs_client_utils/src/main/java/ai/vespa/util/http/VespaClientBuilderFactory.java @@ -0,0 +1,72 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package ai.vespa.util.http; + +import com.yahoo.security.tls.MixedMode; +import com.yahoo.security.tls.TlsContext; +import com.yahoo.security.tls.TransportSecurityUtils; + +import javax.ws.rs.client.ClientBuilder; +import javax.ws.rs.client.ClientRequestContext; +import javax.ws.rs.client.ClientRequestFilter; +import javax.ws.rs.core.UriBuilder; +import java.net.URI; +import java.util.logging.Level; +import java.util.logging.Logger; + +/** + * Factory for JAX-RS http client builder for internal Vespa communications over http/https. + * + * Notes: + * - hostname verification is not enabled - CN/SAN verification is assumed to be handled by the underlying x509 trust manager. + * - ssl context or hostname verifier must not be overriden by the caller + * + * @author bjorncs + */ +public class VespaClientBuilderFactory implements AutoCloseable { + + private static final Logger log = Logger.getLogger(VespaClientBuilderFactory.class.getName()); + + private final TlsContext tlsContext = TransportSecurityUtils.createTlsContext().orElse(null); + private final MixedMode mixedMode = TransportSecurityUtils.getInsecureMixedMode(); + + public ClientBuilder newBuilder() { + ClientBuilder builder = ClientBuilder.newBuilder(); + setSslConfiguration(builder); + return builder; + } + + private void setSslConfiguration(ClientBuilder builder) { + if (tlsContext != null) { + builder.sslContext(tlsContext.context()); + builder.hostnameVerifier((hostname, sslSession) -> true); // disable hostname verification + if (mixedMode != MixedMode.PLAINTEXT_CLIENT_MIXED_SERVER) { + builder.register(new UriRewritingRequestFilter()); + } + } + } + + @Override + public void close() { + if (tlsContext != null) { + tlsContext.close(); + } + } + + static class UriRewritingRequestFilter implements ClientRequestFilter { + @Override + public void filter(ClientRequestContext requestContext) { + requestContext.setUri(rewriteUri(requestContext.getUri())); + } + + private static URI rewriteUri(URI originalUri) { + if (!originalUri.getScheme().equals("http")) { + return originalUri; + } + int port = originalUri.getPort(); + int rewrittenPort = port != -1 ? port : 80; + URI rewrittenUri = UriBuilder.fromUri(originalUri).scheme("https").port(rewrittenPort).build(); + log.log(Level.FINE, () -> String.format("Uri rewritten from '%s' to '%s'", originalUri, rewrittenUri)); + return rewrittenUri; + } + } +} diff --git a/jaxrs_client_utils/src/main/java/ai/vespa/util/http/package-info.java b/jaxrs_client_utils/src/main/java/ai/vespa/util/http/package-info.java new file mode 100644 index 00000000000..8ee304d6de8 --- /dev/null +++ b/jaxrs_client_utils/src/main/java/ai/vespa/util/http/package-info.java @@ -0,0 +1,8 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +/** + * @author bjorncs + */ +@ExportPackage +package ai.vespa.util.http; + +import com.yahoo.osgi.annotation.ExportPackage;
\ No newline at end of file diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java index 4abc4182dd5..aa537d4f69a 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java @@ -109,6 +109,15 @@ public class DockerOperationsImpl implements DockerOperations { addMounts(context, command); + // TODO: Enforce disk constraints + long minMainMemoryAvailableMb = (long) (context.node().memoryGb() * 1024); + if (minMainMemoryAvailableMb > 0) { + // VESPA_TOTAL_MEMORY_MB is used to make any jdisc container think the machine + // only has this much physical memory (overrides total memory reported by `free -m`). + // TODO: Remove after all tenants are running > 7.67 + command.withEnvironment("VESPA_TOTAL_MEMORY_MB", Long.toString(minMainMemoryAvailableMb)); + } + logger.info("Creating new container with args: " + command); command.create(); } @@ -276,6 +285,7 @@ public class DockerOperationsImpl implements DockerOperations { context.pathInNodeUnderVespaHome("var/db/vespa"), context.pathInNodeUnderVespaHome("var/jdisc_container"), context.pathInNodeUnderVespaHome("var/mediasearch"), // TODO: Remove when Vespa 6 is gone + context.pathInNodeUnderVespaHome("var/run"), // TODO: Remove - contains .pid files context.pathInNodeUnderVespaHome("var/vespa"), context.pathInNodeUnderVespaHome("var/yca"), context.pathInNodeUnderVespaHome("var/zookeeper") // Tenant content nodes, config server and controller diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java new file mode 100644 index 00000000000..48f846d5e7f --- /dev/null +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java @@ -0,0 +1,526 @@ +package com.yahoo.vespa.hosted.provision.maintenance; + +import com.yahoo.config.provision.NodeResources; +import com.yahoo.config.provision.NodeType; +import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.NodeRepository; +import com.yahoo.vespa.hosted.provision.node.Allocation; + +import java.util.*; +import java.util.function.Function; +import java.util.stream.Collectors; + +public class CapacityChecker { + private List<Node> hosts; + Map<String, Node> nodeMap; + private Map<Node, List<Node>> nodeChildren; + private Map<Node, AllocationResources> availableResources; + + public AllocationHistory allocationHistory = null; + + public CapacityChecker(NodeRepository nodeRepository) { + this.hosts = getHosts(nodeRepository); + List<Node> tenants = getTenants(nodeRepository, hosts); + nodeMap = constructHostnameToNodeMap(hosts); + this.nodeChildren = constructNodeChildrenMap(tenants, hosts, nodeMap); + this.availableResources = constructAvailableResourcesMap(hosts, nodeChildren); + } + + public List<Node> getHosts() { + return hosts; + } + + public Optional<HostFailurePath> worstCaseHostLossLeadingToFailure() { + Map<Node, Integer> timesNodeCanBeRemoved = computeMaximalRepeatedRemovals(hosts, nodeChildren, availableResources); + return greedyHeuristicFindFailurePath(timesNodeCanBeRemoved, hosts, nodeChildren, availableResources); + } + + protected List<Node> findOvercommittedHosts() { + return findOvercommittedNodes(availableResources); + } + + public List<Node> nodesFromHostnames(List<String> hostnames) { + List<Node> nodes = hostnames.stream() + .filter(h -> nodeMap.containsKey(h)) + .map(h -> nodeMap.get(h)) + .collect(Collectors.toList()); + if (nodes.size() != hostnames.size()) { + Set<String> notFoundNodes = new HashSet<>(hostnames); + notFoundNodes.removeAll(nodes.stream().map(Node::hostname).collect(Collectors.toList())); + throw new IllegalArgumentException(String.format("Host(s) not found: [ %s ]", + String.join(", ", notFoundNodes))); + } + + return nodes; + } + + public Optional<HostFailurePath> findHostRemovalFailure(List<Node> hostsToRemove) { + var removal = findHostRemovalFailure(hostsToRemove, hosts, nodeChildren, availableResources); + if (removal.isEmpty()) return Optional.empty(); + HostFailurePath failurePath = new HostFailurePath(); + failurePath.hostsCausingFailure = hostsToRemove; + failurePath.failureReason = removal.get(); + return Optional.of(failurePath); + } + + // We only care about nodes in one of these states. + private static Node.State[] relevantNodeStates = { + Node.State.active, + Node.State.inactive, + Node.State.dirty, + Node.State.provisioned, + Node.State.ready, + Node.State.reserved + }; + + private List<Node> getHosts(NodeRepository nodeRepository) { + return nodeRepository.getNodes(NodeType.host, relevantNodeStates); + } + + private List<Node> getTenants(NodeRepository nodeRepository, List<Node> hosts) { + var parentNames = hosts.stream().map(Node::hostname).collect(Collectors.toSet()); + return nodeRepository.getNodes(NodeType.tenant, relevantNodeStates).stream() + .filter(t -> parentNames.contains(t.parentHostname().orElse(""))) + .collect(Collectors.toList()); + } + + private Optional<HostFailurePath> greedyHeuristicFindFailurePath(Map<Node, Integer> heuristic, List<Node> hosts, + Map<Node, List<Node>> nodeChildren, + Map<Node, AllocationResources> availableResources) { + if (hosts.size() == 0) return Optional.empty(); + + List<Node> parentRemovalPriorityList = heuristic.entrySet().stream() + .sorted(Comparator.comparingInt(Map.Entry::getValue)) + .map(Map.Entry::getKey) + .collect(Collectors.toList()); + + for (int i = 1; i <= parentRemovalPriorityList.size(); i++) { + List<Node> hostsToRemove = parentRemovalPriorityList.subList(0, i); + var hostRemovalFailure = findHostRemovalFailure(hostsToRemove, hosts, nodeChildren, availableResources); + if (hostRemovalFailure.isPresent()) { + HostFailurePath failurePath = new HostFailurePath(); + failurePath.hostsCausingFailure = hostsToRemove; + failurePath.failureReason = hostRemovalFailure.get(); + return Optional.of(failurePath); + } + } + + throw new IllegalStateException("No path to failure found. This should be impossible!"); + } + + private Map<String, Node> constructHostnameToNodeMap(List<Node> nodes) { + return nodes.stream().collect(Collectors.toMap(Node::hostname, n -> n)); + } + + private Map<Node, List<Node>> constructNodeChildrenMap(List<Node> tenants, List<Node> hosts, Map<String, Node> hostnameToNode) { + Map<Node, List<Node>> nodeChildren = tenants.stream() + .filter(n -> n.parentHostname().isPresent()) + .filter(n -> hostnameToNode.containsKey(n.parentHostname().get())) + .collect(Collectors.groupingBy( + n -> hostnameToNode.get(n.parentHostname().orElseThrow()))); + + for (var host : hosts) nodeChildren.putIfAbsent(host, List.of()); + + return nodeChildren; + } + + private Map<Node, AllocationResources> constructAvailableResourcesMap(List<Node> hosts, Map<Node, List<Node>> nodeChildren) { + Map<Node, AllocationResources> availableResources = new HashMap<>(); + for (var host : hosts) { + NodeResources hostResources = host.flavor().resources(); + int occupiedIps = 0; + Set<String> ipPool = host.ipAddressPool().asSet(); + for (var child : nodeChildren.get(host)) { + hostResources = hostResources.subtract(child.flavor().resources().withDiskSpeed(NodeResources.DiskSpeed.any)); + occupiedIps += child.ipAddresses().stream().filter(ipPool::contains).count(); + } + availableResources.put(host, new AllocationResources(hostResources, host.ipAddressPool().asSet().size() - occupiedIps)); + } + + return availableResources; + } + + /** + * Computes a heuristic for each host, with a lower score indicating a higher perceived likelihood that removing + * the host causes an unrecoverable state + */ + private Map<Node, Integer> computeMaximalRepeatedRemovals(List<Node> hosts, Map<Node, List<Node>> nodeChildren, + Map<Node, AllocationResources> availableResources) { + Map<Node, Integer> timesNodeCanBeRemoved = hosts.stream().collect(Collectors.toMap( + Function.identity(), + _x -> Integer.MAX_VALUE + )); + for (Node host : hosts) { + List<Node> children = nodeChildren.get(host); + if (children.size() == 0) continue; + Map<Node, AllocationResources> resourceMap = new HashMap<>(availableResources); + Map<Node, List<Allocation>> containedAllocations = collateAllocations(nodeChildren); + + int timesHostCanBeRemoved = 0; + Optional<Node> unallocatedNode; + while (timesHostCanBeRemoved < 1000) { // Arbritrary upper bound + unallocatedNode = tryAllocateNodes(nodeChildren.get(host), hosts, resourceMap, containedAllocations); + if (unallocatedNode.isEmpty()) { + timesHostCanBeRemoved++; + } else break; + } + timesNodeCanBeRemoved.put(host, timesHostCanBeRemoved); + } + + return timesNodeCanBeRemoved; + } + + private List<Node> findOvercommittedNodes(Map<Node, AllocationResources> availableResources) { + List<Node> overcommittedNodes = new ArrayList<>(); + for (var entry : availableResources.entrySet()) { + var resources = entry.getValue().nodeResources; + if (resources.vcpu() < 0 || resources.memoryGb() < 0 || resources.diskGb() < 0) { + overcommittedNodes.add(entry.getKey()); + } + } + return overcommittedNodes; + } + + private Map<Node, List<Allocation>> collateAllocations(Map<Node, List<Node>> nodeChildren) { + return nodeChildren.entrySet().stream().collect(Collectors.toMap( + Map.Entry::getKey, + e -> e.getValue().stream() + .map(Node::allocation).flatMap(Optional::stream) + .collect(Collectors.toList()) + )); + } + + /** + * Tests whether it's possible to remove the provided hosts. + * Does not mutate any input variable. + * @return Empty optional if removal is possible, information on what caused the failure otherwise + */ + private Optional<HostRemovalFailure> findHostRemovalFailure(List<Node> hostsToRemove, List<Node> allHosts, + Map<Node, List<Node>> nodechildren, + Map<Node, AllocationResources> availableResources) { + var containedAllocations = collateAllocations(nodechildren); + var resourceMap = new HashMap<>(availableResources); + List<Node> validAllocationTargets = allHosts.stream() + .filter(h -> !hostsToRemove.contains(h)) + .collect(Collectors.toList()); + if (validAllocationTargets.size() == 0) { + return Optional.of(HostRemovalFailure.none()); + } + + allocationHistory = new AllocationHistory(); + for (var host : hostsToRemove) { + Optional<Node> unallocatedNode = tryAllocateNodes(nodechildren.get(host), + validAllocationTargets, resourceMap, containedAllocations, true); + + if (unallocatedNode.isPresent()) { + AllocationFailureReasonList failures = collateAllocationFailures(unallocatedNode.get(), + validAllocationTargets, resourceMap, containedAllocations); + return Optional.of(HostRemovalFailure.create(host, unallocatedNode.get(), failures)); + } + } + return Optional.empty(); + } + + /** + * Attempts to allocate the listed nodes to a new host, mutating availableResources and containedAllocations, + * optionally returning the first node to fail, if one does. + * */ + private Optional<Node> tryAllocateNodes(List<Node> nodes, List<Node> hosts, + Map<Node, AllocationResources> availableResources, + Map<Node, List<Allocation>> containedAllocations) { + return tryAllocateNodes(nodes, hosts, availableResources, containedAllocations, false); + } + private Optional<Node> tryAllocateNodes(List<Node> nodes, List<Node> hosts, + Map<Node, AllocationResources> availableResources, + Map<Node, List<Allocation>> containedAllocations, boolean withHistory) { + for (var node : nodes) { + var newParent = tryAllocateNode(node, hosts, availableResources, containedAllocations); + if (newParent.isEmpty()) { + if (withHistory) allocationHistory.addEntry(node, null, 0); + return Optional.of(node); + } + if (withHistory) { + long eligibleParents = + hosts.stream().filter(h -> + !violatesParentHostPolicy(node, h, containedAllocations) + && availableResources.get(h).satisfies(AllocationResources.from(node.flavor().resources()))).count(); + allocationHistory.addEntry(node, newParent.get(), eligibleParents + 1); + } + } + return Optional.empty(); + } + + /** + * @return The parent to which the node was allocated, if it was successfully allocated. + */ + private Optional<Node> tryAllocateNode(Node node, List<Node> hosts, + Map<Node, AllocationResources> availableResources, + Map<Node, List<Allocation>> containedAllocations) { + AllocationResources requiredNodeResources = AllocationResources.from(node.flavor().resources()); + for (var host : hosts) { + var availableHostResources = availableResources.get(host); + if (violatesParentHostPolicy(node, host, containedAllocations)) { + continue; + } + if (availableHostResources.satisfies(requiredNodeResources)) { + availableResources.put(host, availableHostResources.subtract(requiredNodeResources)); + if (node.allocation().isPresent()) { + containedAllocations.get(host).add(node.allocation().get()); + } + return Optional.of(host); + } + } + + return Optional.empty(); + } + + private static boolean violatesParentHostPolicy(Node node, Node host, Map<Node, List<Allocation>> containedAllocations) { + if (node.allocation().isEmpty()) return false; + Allocation nodeAllocation = node.allocation().get(); + for (var allocation : containedAllocations.get(host)) { + if (allocation.membership().cluster().equalsIgnoringGroupAndVespaVersion(nodeAllocation.membership().cluster()) + && allocation.owner().equals(nodeAllocation.owner())) { + return true; + } + } + return false; + } + + private AllocationFailureReasonList collateAllocationFailures(Node node, List<Node> hosts, + Map<Node, AllocationResources> availableResources, + Map<Node, List<Allocation>> containedAllocations) { + List<AllocationFailureReason> allocationFailureReasons = new ArrayList<>(); + for (var host : hosts) { + AllocationFailureReason reason = new AllocationFailureReason(host); + var availableHostResources = availableResources.get(host); + reason.violatesParentHostPolicy = violatesParentHostPolicy(node, host, containedAllocations); + + NodeResources l = availableHostResources.nodeResources; + NodeResources r = node.flavor().resources(); + if (l.vcpu() < r.vcpu()) { reason.insufficientVcpu = true; } + if (l.memoryGb() < r.memoryGb()) { reason.insufficientMemoryGb = true; } + if (l.diskGb() < r.diskGb()) { reason.insufficientDiskGb = true; } + if (r.diskSpeed() != NodeResources.DiskSpeed.any && r.diskSpeed() != l.diskSpeed()) + { reason.incompatibleDiskSpeed = true; } + if (availableHostResources.availableIPs < 1) { reason.insufficientAvailableIPs = true; } + + allocationFailureReasons.add(reason); + } + + return new AllocationFailureReasonList(allocationFailureReasons); + } + + /** + * Contains the list of hosts that, upon being removed, caused an unrecoverable state, + * as well as the specific host and tenant which caused it. + */ + public static class HostFailurePath { + public List<Node> hostsCausingFailure; + public HostRemovalFailure failureReason; + } + + /** + * Data class used for detailing why removing the given tenant from the given host was unsuccessful. + * A failure might not be caused by failing to allocate a specific tenant, in which case the fields + * will be empty. + */ + public static class HostRemovalFailure { + public Optional<Node> host; + public Optional<Node> tenant; + public AllocationFailureReasonList allocationFailures; + + public static HostRemovalFailure none() { + return new HostRemovalFailure( + Optional.empty(), + Optional.empty(), + new AllocationFailureReasonList(List.of())); + } + + public static HostRemovalFailure create(Node host, Node tenant, AllocationFailureReasonList failureReasons) { + return new HostRemovalFailure( + Optional.of(host), + Optional.of(tenant), + failureReasons); + } + + private HostRemovalFailure(Optional<Node> host, Optional<Node> tenant, AllocationFailureReasonList allocationFailures) { + this.host = host; + this.tenant = tenant; + this.allocationFailures = allocationFailures; + } + + @Override + public String toString() { + if (host.isEmpty() || tenant.isEmpty()) return "No removal candidates exists."; + return String.format( + "Failure to remove host %s" + + "\n\tNo new host found for tenant %s:" + + "\n\t\tSingular Reasons: %s" + + "\n\t\tTotal Reasons: %s", + this.host.get().hostname(), + this.tenant.get().hostname(), + this.allocationFailures.singularReasonFailures().toString(), + this.allocationFailures.toString() + ); + } + } + + /** + * Used to describe the resources required for a tenant, and available to a host. + */ + private static class AllocationResources { + NodeResources nodeResources; + int availableIPs; + + public static AllocationResources from(NodeResources nodeResources) { + return new AllocationResources(nodeResources, 1); + } + + public AllocationResources(NodeResources nodeResources, int availableIPs) { + this.nodeResources = nodeResources; + this.availableIPs = availableIPs; + } + + public boolean satisfies(AllocationResources other) { + if (!this.nodeResources.satisfies(other.nodeResources)) return false; + return this.availableIPs >= other.availableIPs; + } + + public AllocationResources subtract(AllocationResources other) { + return new AllocationResources(this.nodeResources.subtract(other.nodeResources), this.availableIPs - other.availableIPs); + } + } + + /** + * Keeps track of the reason why a host rejected an allocation. + */ + private static class AllocationFailureReason { + Node host; + public AllocationFailureReason (Node host) { + this.host = host; + } + public boolean insufficientVcpu = false; + public boolean insufficientMemoryGb = false; + public boolean insufficientDiskGb = false; + public boolean incompatibleDiskSpeed = false; + public boolean insufficientAvailableIPs = false; + public boolean violatesParentHostPolicy = false; + + public int numberOfReasons() { + int n = 0; + if (insufficientVcpu) n++; + if (insufficientMemoryGb) n++; + if (insufficientDiskGb) n++; + if (incompatibleDiskSpeed) n++; + if (insufficientAvailableIPs) n++; + if (violatesParentHostPolicy) n++; + return n; + } + + @Override + public String toString() { + List<String> reasons = new ArrayList<>(); + if (insufficientVcpu) reasons.add("insufficientVcpu"); + if (insufficientMemoryGb) reasons.add("insufficientMemoryGb"); + if (insufficientDiskGb) reasons.add("insufficientDiskGb"); + if (incompatibleDiskSpeed) reasons.add("incompatibleDiskSpeed"); + if (insufficientAvailableIPs) reasons.add("insufficientAvailableIPs"); + if (violatesParentHostPolicy) reasons.add("violatesParentHostPolicy"); + + return String.format("[%s]", String.join(", ", reasons)); + } + } + + /** + * Provides convenient methods for tallying failures. + */ + public static class AllocationFailureReasonList { + private List<AllocationFailureReason> allocationFailureReasons; + public AllocationFailureReasonList(List<AllocationFailureReason> allocationFailureReasons) { + this.allocationFailureReasons = allocationFailureReasons; + } + + public long insufficientVcpu() { return allocationFailureReasons.stream().filter(r -> r.insufficientVcpu).count(); } + public long insufficientMemoryGb() { return allocationFailureReasons.stream().filter(r -> r.insufficientMemoryGb).count(); } + public long insufficientDiskGb() { return allocationFailureReasons.stream().filter(r -> r.insufficientDiskGb).count(); } + public long incompatibleDiskSpeed() { return allocationFailureReasons.stream().filter(r -> r.incompatibleDiskSpeed).count(); } + public long insufficientAvailableIps() { return allocationFailureReasons.stream().filter(r -> r.insufficientAvailableIPs).count(); } + public long violatesParentHostPolicy() { return allocationFailureReasons.stream().filter(r -> r.violatesParentHostPolicy).count(); } + + public AllocationFailureReasonList singularReasonFailures() { + return new AllocationFailureReasonList(allocationFailureReasons.stream() + .filter(reason -> reason.numberOfReasons() == 1).collect(Collectors.toList())); + } + public AllocationFailureReasonList multipleReasonFailures() { + return new AllocationFailureReasonList(allocationFailureReasons.stream() + .filter(reason -> reason.numberOfReasons() > 1).collect(Collectors.toList())); + } + public long size() { + return allocationFailureReasons.size(); + } + @Override + public String toString() { + return String.format("CPU (%3d), Memory (%3d), Disk size (%3d), Disk speed (%3d), IP (%3d), Parent-Host Policy (%3d)", + insufficientVcpu(), insufficientMemoryGb(), insufficientDiskGb(), + incompatibleDiskSpeed(), insufficientAvailableIps(), violatesParentHostPolicy()); + } + } + + public static class AllocationHistory { + public static class Entry { + public Node tenant; + public Node newParent; + public long eligibleParents; + + public Entry(Node tenant, Node newParent, long eligibleParents) { + this.tenant = tenant; + this.newParent = newParent; + this.eligibleParents = eligibleParents; + } + + @Override + public String toString() { + return String.format("%-20s %-65s -> %15s [%3d valid]", + tenant.hostname().replaceFirst("\\..+", ""), + tenant.flavor().resources(), + newParent == null ? "x" : newParent.hostname().replaceFirst("\\..+", ""), + this.eligibleParents + ); + } + } + + public List<Entry> historyEntries; + + public AllocationHistory() { + this.historyEntries = new ArrayList<>(); + } + + public void addEntry(Node tenant, Node newParent, long eligibleParents) { + this.historyEntries.add(new Entry(tenant, newParent, eligibleParents)); + } + + public Set<String> oldParents() { + Set<String> oldParents = new HashSet<>(); + for (var entry : historyEntries) + entry.tenant.parentHostname().ifPresent(oldParents::add); + return oldParents; + } + + @Override + public String toString() { + StringBuilder out = new StringBuilder(); + + String currentParent = ""; + for (var entry : historyEntries) { + String parentName = entry.tenant.parentHostname().orElseThrow(); + if (!parentName.equals(currentParent)) { + currentParent = parentName; + out.append(parentName).append("\n"); + } + out.append(entry.toString()).append("\n"); + } + + return out.toString(); + } + } +} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityReportMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityReportMaintainer.java index 44d43081ef2..3c47e418b94 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityReportMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityReportMaintainer.java @@ -1,23 +1,15 @@ package com.yahoo.vespa.hosted.provision.maintenance; -import com.yahoo.config.provision.NodeResources; -import com.yahoo.config.provision.NodeType; import com.yahoo.jdisc.Metric; import com.yahoo.log.LogLevel; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; import java.time.Duration; -import java.util.HashMap; -import java.util.List; -import java.util.Map; import java.util.logging.Logger; import java.util.stream.Collectors; -import com.yahoo.vespa.hosted.provision.node.Allocation; - import java.util.*; -import java.util.function.Function; /** * Performs analysis on the node repository to produce metrics that pertain to the capacity of the node repository. @@ -29,7 +21,6 @@ import java.util.function.Function; * @author mgimle */ public class CapacityReportMaintainer extends Maintainer { - private final Metric metric; private final NodeRepository nodeRepository; private static final Logger log = Logger.getLogger(CapacityReportMaintainer.class.getName()); @@ -44,403 +35,20 @@ public class CapacityReportMaintainer extends Maintainer { @Override protected void maintain() { - metric.set("overcommittedHosts", countOvercommittedHosts(), null); - - Optional<HostFailurePath> failurePath = worstCaseHostLossLeadingToFailure(); - if (failurePath.isPresent()) { - int worstCaseHostLoss = failurePath.get().hostsCausingFailure.size(); - metric.set("spareHostCapacity", worstCaseHostLoss - 1, null); - } - } - - protected Optional<HostFailurePath> worstCaseHostLossLeadingToFailure() { - List<Node> hosts = getHosts(); - List<Node> tenants = getTenants(hosts); - Map<String, Node> nodeMap = constructHostnameToNodeMap(hosts); - Map<Node, List<Node>> nodeChildren = constructNodeChildrenMap(tenants, hosts, nodeMap); - Map<Node, AllocationResources> availableResources = constructAvailableResourcesMap(hosts, nodeChildren); - - Map<Node, Integer> timesNodeCanBeRemoved = computeMaximalRepeatedRemovals(hosts, nodeChildren, availableResources); - return greedyHeuristicFindFailurePath(timesNodeCanBeRemoved, hosts, nodeChildren, availableResources); - } - - // We only care about nodes in one of these states. - private Node.State[] relevantNodeStates = { - Node.State.active, - Node.State.inactive, - Node.State.dirty, - Node.State.provisioned, - Node.State.ready, - Node.State.reserved - }; - - private List<Node> getHosts() { - return nodeRepository.getNodes(NodeType.host, relevantNodeStates); - } - - private List<Node> getTenants(List<Node> hosts) { - var parentNames = hosts.stream().map(Node::hostname).collect(Collectors.toSet()); - return nodeRepository.getNodes(NodeType.tenant, relevantNodeStates).stream() - .filter(t -> parentNames.contains(t.parentHostname().orElse(""))) - .collect(Collectors.toList()); - } - - private Optional<HostFailurePath> greedyHeuristicFindFailurePath(Map<Node, Integer> heuristic, List<Node> hosts, - Map<Node, List<Node>> nodeChildren, - Map<Node, AllocationResources> availableResources) { - if (hosts.size() == 0) return Optional.empty(); - List<Node> parentRemovalPriorityList = heuristic.entrySet().stream() - .sorted(Comparator.comparingInt(Map.Entry::getValue)) - .map(Map.Entry::getKey) - .collect(Collectors.toList()); - for (int i = 1; i <= parentRemovalPriorityList.size(); i++) { - List<Node> hostsToRemove = parentRemovalPriorityList.subList(0, i); - var hostRemovalFailure = findHostRemovalFailure(hostsToRemove, hosts, nodeChildren, availableResources); - if (hostRemovalFailure.isPresent()) { - HostFailurePath failurePath = new HostFailurePath(); - failurePath.hostsCausingFailure = hostsToRemove; - failurePath.failureReason = hostRemovalFailure.get(); - return Optional.of(failurePath); + if (!nodeRepository.zone().cloud().value().equals("aws")) { + CapacityChecker capacityChecker = new CapacityChecker(this.nodeRepository); + List<Node> overcommittedHosts = capacityChecker.findOvercommittedHosts(); + if (overcommittedHosts.size() != 0) { + log.log(LogLevel.WARNING, String.format("%d nodes are overcommitted! [ %s ]", overcommittedHosts.size(), + overcommittedHosts.stream().map(Node::hostname).collect(Collectors.joining(", ")))); } - } - - throw new IllegalStateException("No path to failure found. This should be impossible!"); - } - - protected int countOvercommittedHosts() { - List<Node> hosts = getHosts(); - List<Node> tenants = getTenants(hosts); - var nodeMap = constructHostnameToNodeMap(hosts); - var nodeChildren = constructNodeChildrenMap(tenants, hosts, nodeMap); - var availableResources = constructAvailableResourcesMap(hosts, nodeChildren); - - List<Node> overcommittedNodes = findOvercommittedNodes(availableResources); - if (overcommittedNodes.size() != 0) { - log.log(LogLevel.WARNING, String.format("%d nodes are overcommitted! [ %s ]", overcommittedNodes.size(), - overcommittedNodes.stream().map(Node::hostname).collect(Collectors.joining(", ")))); - } - return overcommittedNodes.size(); - } - - private Map<String, Node> constructHostnameToNodeMap(List<Node> nodes) { - return nodes.stream().collect(Collectors.toMap(Node::hostname, n -> n)); - } - - private Map<Node, List<Node>> constructNodeChildrenMap(List<Node> tenants, List<Node> hosts, Map<String, Node> hostnameToNode) { - Map<Node, List<Node>> nodeChildren = tenants.stream() - .filter(n -> n.parentHostname().isPresent()) - .filter(n -> hostnameToNode.containsKey(n.parentHostname().get())) - .collect(Collectors.groupingBy( - n -> hostnameToNode.get(n.parentHostname().orElseThrow()))); - - for (var host : hosts) nodeChildren.putIfAbsent(host, List.of()); - - return nodeChildren; - } - - private Map<Node, AllocationResources> constructAvailableResourcesMap(List<Node> hosts, Map<Node, List<Node>> nodeChildren) { - Map<Node, AllocationResources> availableResources = new HashMap<>(); - for (var host : hosts) { - NodeResources hostResources = host.flavor().resources(); - int occupiedIps = 0; - Set<String> ipPool = host.ipAddressPool().asSet(); - for (var child : nodeChildren.get(host)) { - hostResources = hostResources.subtract(child.flavor().resources()); - occupiedIps += child.ipAddresses().stream().filter(ipPool::contains).count(); - } - availableResources.put(host, new AllocationResources(hostResources, host.ipAddressPool().asSet().size() - occupiedIps)); - } - - return availableResources; - } - - /** - * Computes a heuristic for each host, with a lower score indicating a higher perceived likelihood that removing - * the host causes an unrecoverable state - */ - private Map<Node, Integer> computeMaximalRepeatedRemovals(List<Node> hosts, Map<Node, List<Node>> nodeChildren, - Map<Node, AllocationResources> availableResources) { - Map<Node, Integer> timesNodeCanBeRemoved = hosts.stream().collect(Collectors.toMap( - Function.identity(), - _x -> Integer.MAX_VALUE - )); - for (Node host : hosts) { - List<Node> children = nodeChildren.get(host); - if (children.size() == 0) continue; - Map<Node, AllocationResources> resourceMap = new HashMap<>(availableResources); - Map<Node, List<Allocation>> containedAllocations = collateAllocations(nodeChildren); - - int timesHostCanBeRemoved = 0; - Optional<Node> unallocatedTenant; - while (timesHostCanBeRemoved < 1000) { // Arbritrary upper bound - unallocatedTenant = tryAllocateNodes(nodeChildren.get(host), hosts, resourceMap, containedAllocations); - if (unallocatedTenant.isEmpty()) { - timesHostCanBeRemoved++; - } else break; - } - timesNodeCanBeRemoved.put(host, timesHostCanBeRemoved); - } - - return timesNodeCanBeRemoved; - } - - private List<Node> findOvercommittedNodes(Map<Node, AllocationResources> availableResources) { - List<Node> overcommittedNodes = new ArrayList<>(); - for (var entry : availableResources.entrySet()) { - var resources = entry.getValue().nodeResources; - if (resources.vcpu() < 0 || resources.memoryGb() < 0 || resources.diskGb() < 0) { - overcommittedNodes.add(entry.getKey()); - } - } - return overcommittedNodes; - } - - private Map<Node, List<Allocation>> collateAllocations(Map<Node, List<Node>> nodeChildren) { - return nodeChildren.entrySet().stream().collect(Collectors.toMap( - Map.Entry::getKey, - e -> e.getValue().stream() - .map(Node::allocation).flatMap(Optional::stream) - .collect(Collectors.toList()) - )); - } - - /** - * Tests whether it's possible to remove the provided hosts. - * Does not mutate any input variable. - * @return Empty optional if removal is possible, information on what caused the failure otherwise - */ - private Optional<HostRemovalFailure> findHostRemovalFailure(List<Node> hostsToRemove, List<Node> allHosts, - Map<Node, List<Node>> nodechildren, - Map<Node, AllocationResources> availableResources) { - var containedAllocations = collateAllocations(nodechildren); - var resourceMap = new HashMap<>(availableResources); - List<Node> validAllocationTargets = allHosts.stream() - .filter(h -> !hostsToRemove.contains(h)) - .collect(Collectors.toList()); - if (validAllocationTargets.size() == 0) { - return Optional.of(HostRemovalFailure.none()); - } - - for (var host : hostsToRemove) { - Optional<Node> unallocatedNode = tryAllocateNodes(nodechildren.get(host), - validAllocationTargets, resourceMap, containedAllocations); - - if (unallocatedNode.isPresent()) { - AllocationFailureReasonList failures = collateAllocationFailures(unallocatedNode.get(), - validAllocationTargets, resourceMap, containedAllocations); - return Optional.of(HostRemovalFailure.create(host, unallocatedNode.get(), failures)); - } - } - return Optional.empty(); - } + metric.set("overcommittedHosts", overcommittedHosts.size(), null); - /** - * Attempts to allocate the listed nodes to a new host, mutating availableResources and containedAllocations, - * optionally returning the first node to fail, if one does. - * */ - private Optional<Node> tryAllocateNodes(List<Node> nodes, List<Node> hosts, - Map<Node, AllocationResources> availableResources, - Map<Node, List<Allocation>> containedAllocations) { - for (var node : nodes) { - if (!tryAllocateNode(node, hosts, availableResources, containedAllocations)) { - return Optional.of(node); + Optional<CapacityChecker.HostFailurePath> failurePath = capacityChecker.worstCaseHostLossLeadingToFailure(); + if (failurePath.isPresent()) { + int worstCaseHostLoss = failurePath.get().hostsCausingFailure.size(); + metric.set("spareHostCapacity", worstCaseHostLoss - 1, null); } } - return Optional.empty(); - } - - private boolean tryAllocateNode(Node node, List<Node> hosts, - Map<Node, AllocationResources> availableResources, - Map<Node, List<Allocation>> containedAllocations) { - AllocationResources requiredNodeResources = AllocationResources.from(node.flavor().resources()); - for (var host : hosts) { - var availableHostResources = availableResources.get(host); - if (violatesParentHostPolicy(node, host, containedAllocations)) { - continue; - } - if (availableHostResources.satisfies(requiredNodeResources)) { - availableResources.put(host, availableHostResources.subtract(requiredNodeResources)); - if (node.allocation().isPresent()) { - containedAllocations.get(host).add(node.allocation().get()); - } - return true; - } - } - - return false; - } - - private boolean violatesParentHostPolicy(Node node, Node host, Map<Node, List<Allocation>> containedAllocations) { - if (node.allocation().isEmpty()) return false; - Allocation nodeAllocation = node.allocation().get(); - for (var allocation : containedAllocations.get(host)) { - if (allocation.membership().cluster().equalsIgnoringGroupAndVespaVersion(nodeAllocation.membership().cluster()) - && allocation.owner().equals(nodeAllocation.owner())) { - return true; - } - } - return false; - } - - private AllocationFailureReasonList collateAllocationFailures(Node node, List<Node> hosts, - Map<Node, AllocationResources> availableResources, - Map<Node, List<Allocation>> containedAllocations) { - List<AllocationFailureReason> allocationFailureReasons = new ArrayList<>(); - for (var host : hosts) { - AllocationFailureReason reason = new AllocationFailureReason(host); - var availableHostResources = availableResources.get(host); - reason.violatesParentHostPolicy = violatesParentHostPolicy(node, host, containedAllocations); - - NodeResources l = availableHostResources.nodeResources; - NodeResources r = node.flavor().resources(); - if (l.vcpu() < r.vcpu()) { reason.insufficientVcpu = true; } - if (l.memoryGb() < r.memoryGb()) { reason.insufficientMemoryGb = true; } - if (l.diskGb() < r.diskGb()) { reason.insufficientDiskGb = true; } - if (r.diskSpeed() != NodeResources.DiskSpeed.any && r.diskSpeed() != l.diskSpeed()) - { reason.incompatibleDiskSpeed = true; } - if (availableHostResources.availableIPs < 1) { reason.insufficientAvailableIPs = true; } - - allocationFailureReasons.add(reason); - } - - return new AllocationFailureReasonList(allocationFailureReasons); - } - - /** - * Contains the list of hosts that, upon being removed, caused an unrecoverable state, - * as well as the specific host and tenant which caused it. - */ - public static class HostFailurePath { - List<Node> hostsCausingFailure; - HostRemovalFailure failureReason; - } - - /** - * Data class used for detailing why removing the given tenant from the given host was unsuccessful. - * A failure might not be caused by failing to allocate a specific tenant, in which case the fields - * will be empty. - */ - public static class HostRemovalFailure { - Optional<Node> host; - Optional<Node> tenant; - AllocationFailureReasonList failureReasons; - public static HostRemovalFailure none() { - return new HostRemovalFailure( - Optional.empty(), - Optional.empty(), - new AllocationFailureReasonList(List.of())); - } - public static HostRemovalFailure create(Node host, Node tenant, AllocationFailureReasonList failureReasons) { - return new HostRemovalFailure( - Optional.of(host), - Optional.of(tenant), - failureReasons); - } - private HostRemovalFailure(Optional<Node> host, Optional<Node> tenant, AllocationFailureReasonList failureReasons) { - this.host = host; - this.tenant = tenant; - this.failureReasons = failureReasons; - } - } - - /** - * Used to describe the resources required for a tenant, and available to a host. - */ - private static class AllocationResources { - NodeResources nodeResources; - int availableIPs; - - public static AllocationResources from(NodeResources nodeResources) { - return new AllocationResources(nodeResources, 1); - } - - public AllocationResources(NodeResources nodeResources, int availableIPs) { - this.nodeResources = nodeResources; - this.availableIPs = availableIPs; - } - - public boolean satisfies(AllocationResources other) { - if (!this.nodeResources.satisfies(other.nodeResources)) return false; - return this.availableIPs >= other.availableIPs; - } - - public AllocationResources subtract(AllocationResources other) { - return new AllocationResources(this.nodeResources.subtract(other.nodeResources), this.availableIPs - other.availableIPs); - } - } - - /** - * Keeps track of the reason why a host rejected an allocation. - */ - private class AllocationFailureReason { - Node host; - public AllocationFailureReason (Node host) { - this.host = host; - } - public boolean insufficientVcpu = false; - public boolean insufficientMemoryGb = false; - public boolean insufficientDiskGb = false; - public boolean incompatibleDiskSpeed = false; - public boolean insufficientAvailableIPs = false; - public boolean violatesParentHostPolicy = false; - - public int numberOfReasons() { - int n = 0; - if (insufficientVcpu) n++; - if (insufficientMemoryGb) n++; - if (insufficientDiskGb) n++; - if (incompatibleDiskSpeed) n++; - if (insufficientAvailableIPs) n++; - if (violatesParentHostPolicy) n++; - return n; - } - - @Override - public String toString() { - List<String> reasons = new ArrayList<>(); - if (insufficientVcpu) reasons.add("insufficientVcpu"); - if (insufficientMemoryGb) reasons.add("insufficientMemoryGb"); - if (insufficientDiskGb) reasons.add("insufficientDiskGb"); - if (incompatibleDiskSpeed) reasons.add("incompatibleDiskSpeed"); - if (insufficientAvailableIPs) reasons.add("insufficientAvailableIPs"); - if (violatesParentHostPolicy) reasons.add("violatesParentHostPolicy"); - - return String.format("[%s]", String.join(", ", reasons)); - } - } - - /** - * Provides convenient methods for tallying failures. - */ - public static class AllocationFailureReasonList { - private List<AllocationFailureReason> allocationFailureReasons; - public AllocationFailureReasonList(List<AllocationFailureReason> allocationFailureReasons) { - this.allocationFailureReasons = allocationFailureReasons; - } - - long insufficientVcpu() { return allocationFailureReasons.stream().filter(r -> r.insufficientVcpu).count(); } - long insufficientMemoryGb() { return allocationFailureReasons.stream().filter(r -> r.insufficientMemoryGb).count(); } - long insufficientDiskGb() { return allocationFailureReasons.stream().filter(r -> r.insufficientDiskGb).count(); } - long incompatibleDiskSpeed() { return allocationFailureReasons.stream().filter(r -> r.incompatibleDiskSpeed).count(); } - long insufficientAvailableIps() { return allocationFailureReasons.stream().filter(r -> r.insufficientAvailableIPs).count(); } - long violatesParentHostPolicy() { return allocationFailureReasons.stream().filter(r -> r.violatesParentHostPolicy).count(); } - - public AllocationFailureReasonList singularReasonFailures() { - return new AllocationFailureReasonList(allocationFailureReasons.stream() - .filter(reason -> reason.numberOfReasons() == 1).collect(Collectors.toList())); - } - public AllocationFailureReasonList multipleReasonFailures() { - return new AllocationFailureReasonList(allocationFailureReasons.stream() - .filter(reason -> reason.numberOfReasons() > 1).collect(Collectors.toList())); - } - public long size() { - return allocationFailureReasons.size(); - } - @Override - public String toString() { - return String.format("CPU (%3d), Memory (%3d), Disk size (%3d), Disk speed (%3d), IP (%3d), Parent-Host Policy (%3d)", - insufficientVcpu(), insufficientMemoryGb(), insufficientDiskGb(), - incompatibleDiskSpeed(), insufficientAvailableIps(), violatesParentHostPolicy()); - } } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java index f661977d933..bb1ff637f08 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java @@ -82,7 +82,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { new HostProvisionMaintainer(nodeRepository, durationFromEnv("host_provisioner_interval").orElse(defaults.hostProvisionerInterval), hostProvisioner, flagSource)); hostDeprovisionMaintainer = provisionServiceProvider.getHostProvisioner().map(hostProvisioner -> new HostDeprovisionMaintainer(nodeRepository, durationFromEnv("host_deprovisioner_interval").orElse(defaults.hostDeprovisionerInterval), hostProvisioner, flagSource)); - capacityReportMaintainer = new CapacityReportMaintainer(nodeRepository, metric, durationFromEnv("alert_interval").orElse(defaults.nodeAlerterInterval)); + capacityReportMaintainer = new CapacityReportMaintainer(nodeRepository, metric, durationFromEnv("capacity_report_interval").orElse(defaults.capacityReportInterval)); // The DuperModel is filled with infrastructure applications by the infrastructure provisioner, so explicitly run that now infrastructureProvisioner.maintain(); @@ -143,7 +143,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { private final Duration dirtyExpiry; private final Duration provisionedExpiry; private final Duration rebootInterval; - private final Duration nodeAlerterInterval; + private final Duration capacityReportInterval; private final Duration metricsInterval; private final Duration retiredInterval; private final Duration infrastructureProvisionInterval; @@ -162,7 +162,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { failedExpirerInterval = Duration.ofMinutes(10); provisionedExpiry = Duration.ofHours(4); rebootInterval = Duration.ofDays(30); - nodeAlerterInterval = Duration.ofHours(1); + capacityReportInterval = Duration.ofHours(1); metricsInterval = Duration.ofMinutes(1); infrastructureProvisionInterval = Duration.ofMinutes(1); throttlePolicy = NodeFailer.ThrottlePolicy.hosted; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/HostCapacityResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/HostCapacityResponse.java new file mode 100644 index 00000000000..9f5af52cc08 --- /dev/null +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/HostCapacityResponse.java @@ -0,0 +1,168 @@ +package com.yahoo.vespa.hosted.provision.restapi.v2; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.yahoo.container.jdisc.HttpRequest; +import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.JsonFormat; +import com.yahoo.slime.Slime; +import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.NodeRepository; +import com.yahoo.vespa.hosted.provision.maintenance.CapacityChecker; + +import java.io.IOException; +import java.io.OutputStream; +import java.util.Arrays; +import java.util.List; +import java.util.Optional; + +public class HostCapacityResponse extends HttpResponse { + private final StringBuilder text; + private final Slime slime; + private final CapacityChecker capacityChecker; + private final boolean json; + + public HostCapacityResponse(NodeRepository nodeRepository, HttpRequest request) { + super(200); + capacityChecker = new CapacityChecker(nodeRepository); + + json = request.getBooleanProperty("json"); + String hostsJson = request.getProperty("hosts"); + + text = new StringBuilder(); + slime = new Slime(); + Cursor root = slime.setObject(); + + if (hostsJson != null) { + List<Node> hosts = parseHostList(hostsJson); + hostRemovalResponse(root, hosts); + } else { + zoneFailureReponse(root); + } + } + + private List<Node> parseHostList(String hosts) { + ObjectMapper om = new ObjectMapper(); + String[] hostsArray; + try { + hostsArray = om.readValue(hosts, String[].class); + } catch (Exception e) { + throw new IllegalArgumentException(e.getMessage()); + } + List<String> hostNames = Arrays.asList(hostsArray); + try { + return capacityChecker.nodesFromHostnames(hostNames); + } catch (IllegalArgumentException e) { + throw new NotFoundException(e.getMessage()); + } + } + + private void hostRemovalResponse(Cursor root, List<Node> hosts) { + var failure = capacityChecker.findHostRemovalFailure(hosts); + if (failure.isPresent() && failure.get().failureReason.allocationFailures.size() == 0) { + root.setBool("removalPossible", false); + error(root, "Removing all hosts is trivially impossible."); + } else { + if (json) hostLossPossibleToSlime(root, failure, hosts); + else hostLossPossibleToText(failure, hosts); + } + } + + private void zoneFailureReponse(Cursor root) { + var failurePath = capacityChecker.worstCaseHostLossLeadingToFailure(); + if (failurePath.isPresent()) { + if (json) zoneFailurePathToSlime(root, failurePath.get()); + else zoneFailurePathToText(failurePath.get()); + } else { + error(root, "Node repository contained no hosts."); + } + } + + private void error(Cursor root, String errorMessage) { + if (json) root.setString("error", errorMessage); + else text.append(errorMessage); + } + + private void hostLossPossibleToText(Optional<CapacityChecker.HostFailurePath> failure, List<Node> hostsToRemove) { + text.append(String.format("Attempting to remove %d hosts: ", hostsToRemove.size())); + CapacityChecker.AllocationHistory history = capacityChecker.allocationHistory; + if (failure.isEmpty()) { + text.append("OK\n\n"); + text.append(history); + if (history.oldParents().size() != hostsToRemove.size()) { + long emptyHostCount = hostsToRemove.size() - history.oldParents().size(); + text.append(String.format("\nTrivially removed %d empty host%s.", emptyHostCount, emptyHostCount > 1 ? "s" : "")); + } + } else { + text.append("FAILURE\n\n"); + text.append(history).append("\n"); + text.append(failure.get().failureReason).append("\n\n"); + } + } + + private void zoneFailurePathToText(CapacityChecker.HostFailurePath failurePath) { + text.append(String.format("Found %d hosts. Failure upon trying to remove %d hosts:\n\n", + capacityChecker.getHosts().size(), + failurePath.hostsCausingFailure.size())); + text.append(capacityChecker.allocationHistory).append("\n"); + text.append(failurePath.failureReason); + } + + private void hostLossPossibleToSlime(Cursor root, Optional<CapacityChecker.HostFailurePath> failure, List<Node> hostsToRemove) { + var hosts = root.setArray("hostsToRemove"); + hostsToRemove.forEach(h -> hosts.addString(h.hostname())); + CapacityChecker.AllocationHistory history = capacityChecker.allocationHistory; + root.setBool("removalPossible", failure.isEmpty()); + var arr = root.setArray("history"); + for (var entry : history.historyEntries) { + var object = arr.addObject(); + object.setString("tenant", entry.tenant.hostname()); + if (entry.newParent != null) { + object.setString("newParent", entry.newParent.hostname()); + } + object.setLong("eligibleParents", entry.eligibleParents); + } + } + + private void zoneFailurePathToSlime(Cursor object, CapacityChecker.HostFailurePath failurePath) { + object.setLong("totalHosts", capacityChecker.getHosts().size()); + object.setLong("couldLoseHosts", failurePath.hostsCausingFailure.size()); + failurePath.failureReason.host.ifPresent(host -> + object.setString("failedTenantParent", host.hostname()) + ); + failurePath.failureReason.tenant.ifPresent(tenant -> { + object.setString("failedTenant", tenant.hostname()); + object.setString("failedTenantResources", tenant.flavor().resources().toString()); + tenant.allocation().ifPresent(allocation -> + object.setString("failedTenantAllocation", allocation.toString()) + ); + var explanation = object.setObject("hostCandidateRejectionReasons"); + allocationFailureReasonListToSlime(explanation.setObject("singularReasonFailures"), + failurePath.failureReason.allocationFailures.singularReasonFailures()); + allocationFailureReasonListToSlime(explanation.setObject("totalFailures"), + failurePath.failureReason.allocationFailures); + }); + var details = object.setObject("details"); + hostLossPossibleToSlime(details, Optional.of(failurePath), failurePath.hostsCausingFailure); + } + + private void allocationFailureReasonListToSlime(Cursor root, CapacityChecker.AllocationFailureReasonList allocationFailureReasonList) { + root.setLong("insufficientVcpu", allocationFailureReasonList.insufficientVcpu()); + root.setLong("insufficientMemoryGb", allocationFailureReasonList.insufficientMemoryGb()); + root.setLong("insufficientDiskGb", allocationFailureReasonList.insufficientDiskGb()); + root.setLong("incompatibleDiskSpeed", allocationFailureReasonList.incompatibleDiskSpeed()); + root.setLong("insufficientAvailableIps", allocationFailureReasonList.insufficientAvailableIps()); + root.setLong("violatesParentHostPolicy", allocationFailureReasonList.violatesParentHostPolicy()); + } + + @Override + public void render(OutputStream stream) throws IOException { + if (json) new JsonFormat(true).encode(stream, slime); + else stream.write(text.toString().getBytes()); + } + + @Override + public String getContentType() { + return json ? "application/json" : "text/plain"; + } +} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java index 22318f1ddb4..e036124e489 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java @@ -102,6 +102,7 @@ public class NodesApiHandler extends LoggingRequestHandler { if (path.equals( "/nodes/v2/command/")) return ResourcesResponse.fromStrings(request.getUri(), "restart", "reboot"); if (path.equals( "/nodes/v2/maintenance/")) return new JobsResponse(nodeRepository.jobControl()); if (path.equals( "/nodes/v2/upgrade/")) return new UpgradeResponse(nodeRepository.infrastructureVersions(), nodeRepository.osVersions(), nodeRepository.dockerImages()); + if (path.startsWith("/nodes/v2/capacity/")) return new HostCapacityResponse(nodeRepository, request); throw new NotFoundException("Nothing at path '" + path + "'"); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityReportMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTest.java index a486f8619c5..1f2112673d1 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityReportMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTest.java @@ -8,20 +8,19 @@ import org.junit.Test; import java.io.IOException; import java.nio.file.Paths; import java.util.*; + import static org.junit.Assert.fail; import static org.junit.Assert.*; /** * @author mgimle */ -public class CapacityReportMaintainerTest { - private CapacityReportMaintainerTester tester; - private CapacityReportMaintainer capacityReporter; +public class CapacityCheckerTest { + private CapacityCheckerTester tester; @Before public void setup() { - tester = new CapacityReportMaintainerTester(); - capacityReporter = tester.makeCapacityReportMaintainer(); + tester = new CapacityCheckerTester(); } @Test @@ -30,10 +29,9 @@ public class CapacityReportMaintainerTest { tester.cleanRepository(); tester.restoreNodeRepositoryFromJsonFile(Paths.get(path)); - var failurePath = capacityReporter.worstCaseHostLossLeadingToFailure(); - if (failurePath.isPresent()) { - assertTrue(tester.nodeRepository.getNodes(NodeType.host).containsAll(failurePath.get().hostsCausingFailure)); - } else fail(); + var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); + assertTrue(failurePath.isPresent()); + assertTrue(tester.nodeRepository.getNodes(NodeType.host).containsAll(failurePath.get().hostsCausingFailure)); } @Test @@ -41,7 +39,7 @@ public class CapacityReportMaintainerTest { tester.createNodes(7, 4, 10, new NodeResources(-1, 10, 100), 10, 0, new NodeResources(1, 10, 100), 10); - int overcommittedHosts = capacityReporter.countOvercommittedHosts(); + int overcommittedHosts = tester.capacityChecker.findOvercommittedHosts().size(); assertEquals(tester.nodeRepository.getNodes(NodeType.host).size(), overcommittedHosts); } @@ -50,14 +48,14 @@ public class CapacityReportMaintainerTest { tester.createNodes(1, 1, 0, new NodeResources(1, 10, 100), 10, 0, new NodeResources(1, 10, 100), 10); - var failurePath = capacityReporter.worstCaseHostLossLeadingToFailure(); + var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); assertFalse("Computing worst case host loss with no hosts should return an empty optional.", failurePath.isPresent()); // Odd edge case that should never be able to occur in prod tester.createNodes(1, 10, 10, new NodeResources(10, 1000, 10000), 100, 1, new NodeResources(10, 1000, 10000), 100); - failurePath = capacityReporter.worstCaseHostLossLeadingToFailure(); + failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); assertTrue(failurePath.isPresent()); assertTrue("Computing worst case host loss if all hosts have to be removed should result in an non-empty failureReason with empty nodes.", failurePath.get().failureReason.tenant.isEmpty() && failurePath.get().failureReason.host.isEmpty()); @@ -66,10 +64,10 @@ public class CapacityReportMaintainerTest { tester.createNodes(3, 30, 10, new NodeResources(0, 0, 10000), 1000, 0, new NodeResources(0, 0, 0), 0); - failurePath = capacityReporter.worstCaseHostLossLeadingToFailure(); + failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); assertTrue(failurePath.isPresent()); if (failurePath.get().failureReason.tenant.isPresent()) { - var failureReasons = failurePath.get().failureReason.failureReasons; + var failureReasons = failurePath.get().failureReason.allocationFailures; assertEquals("When there are multiple lacking resources, all failures are multipleReasonFailures", failureReasons.size(), failureReasons.multipleReasonFailures().size()); assertEquals(0, failureReasons.singularReasonFailures().size()); @@ -81,10 +79,10 @@ public class CapacityReportMaintainerTest { tester.createNodes(1, 10, 10, new NodeResources(10, 1000, 10000), 1, 10, new NodeResources(10, 1000, 10000), 1); - var failurePath = capacityReporter.worstCaseHostLossLeadingToFailure(); + var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); assertTrue(failurePath.isPresent()); if (failurePath.get().failureReason.tenant.isPresent()) { - var failureReasons = failurePath.get().failureReason.failureReasons; + var failureReasons = failurePath.get().failureReason.allocationFailures; assertEquals("All failures should be due to hosts having a lack of available ip addresses.", failureReasons.singularReasonFailures().insufficientAvailableIps(), failureReasons.size()); } else fail(); @@ -96,10 +94,10 @@ public class CapacityReportMaintainerTest { tester.createNodes(1, 10, 10, new NodeResources(1, 100, 1000), 100, 10, new NodeResources(0, 100, 1000), 100); - var failurePath = capacityReporter.worstCaseHostLossLeadingToFailure(); + var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); assertTrue(failurePath.isPresent()); if (failurePath.get().failureReason.tenant.isPresent()) { - var failureReasons = failurePath.get().failureReason.failureReasons; + var failureReasons = failurePath.get().failureReason.allocationFailures; assertEquals("All failures should be due to hosts lacking cpu cores.", failureReasons.singularReasonFailures().insufficientVcpu(), failureReasons.size()); } else fail(); @@ -107,10 +105,10 @@ public class CapacityReportMaintainerTest { tester.createNodes(1, 10, 10, new NodeResources(10, 1, 1000), 100, 10, new NodeResources(10, 0, 1000), 100); - failurePath = capacityReporter.worstCaseHostLossLeadingToFailure(); + failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); assertTrue(failurePath.isPresent()); if (failurePath.get().failureReason.tenant.isPresent()) { - var failureReasons = failurePath.get().failureReason.failureReasons; + var failureReasons = failurePath.get().failureReason.allocationFailures; assertEquals("All failures should be due to hosts lacking memory.", failureReasons.singularReasonFailures().insufficientMemoryGb(), failureReasons.size()); } else fail(); @@ -118,10 +116,10 @@ public class CapacityReportMaintainerTest { tester.createNodes(1, 10, 10, new NodeResources(10, 100, 10), 100, 10, new NodeResources(10, 100, 0), 100); - failurePath = capacityReporter.worstCaseHostLossLeadingToFailure(); + failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); assertTrue(failurePath.isPresent()); if (failurePath.get().failureReason.tenant.isPresent()) { - var failureReasons = failurePath.get().failureReason.failureReasons; + var failureReasons = failurePath.get().failureReason.allocationFailures; assertEquals("All failures should be due to hosts lacking disk space.", failureReasons.singularReasonFailures().insufficientDiskGb(), failureReasons.size()); } else fail(); @@ -130,10 +128,10 @@ public class CapacityReportMaintainerTest { tester.createNodes(1, 10, List.of(new NodeResources(1, 10, 100)), 10, new NodeResources(0, 0, 0), 100, 10, new NodeResources(10, 1000, 10000, NodeResources.DiskSpeed.slow), 100); - failurePath = capacityReporter.worstCaseHostLossLeadingToFailure(); + failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); assertTrue(failurePath.isPresent()); if (failurePath.get().failureReason.tenant.isPresent()) { - var failureReasons = failurePath.get().failureReason.failureReasons; + var failureReasons = failurePath.get().failureReason.allocationFailures; assertEquals("All empty hosts should be invalid due to having incompatible disk speed.", failureReasons.singularReasonFailures().incompatibleDiskSpeed(), emptyHostsWithSlowDisk); } else fail(); @@ -146,10 +144,10 @@ public class CapacityReportMaintainerTest { tester.createNodes(1, 1, 10, new NodeResources(1, 100, 1000), 100, 10, new NodeResources(10, 1000, 10000), 100); - var failurePath = capacityReporter.worstCaseHostLossLeadingToFailure(); + var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); assertTrue(failurePath.isPresent()); if (failurePath.get().failureReason.tenant.isPresent()) { - var failureReasons = failurePath.get().failureReason.failureReasons; + var failureReasons = failurePath.get().failureReason.allocationFailures; assertEquals("With only one type of tenant, all failures should be due to violation of the parent host policy.", failureReasons.singularReasonFailures().violatesParentHostPolicy(), failureReasons.size()); } else fail(); @@ -157,10 +155,10 @@ public class CapacityReportMaintainerTest { tester.createNodes(1, 2, 10, new NodeResources(10, 100, 1000), 1, 0, new NodeResources(0, 0, 0), 0); - failurePath = capacityReporter.worstCaseHostLossLeadingToFailure(); + failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); assertTrue(failurePath.isPresent()); if (failurePath.get().failureReason.tenant.isPresent()) { - var failureReasons = failurePath.get().failureReason.failureReasons; + var failureReasons = failurePath.get().failureReason.allocationFailures; assertNotEquals("Fewer distinct children than hosts should result in some parent host policy violations.", failureReasons.size(), failureReasons.singularReasonFailures().violatesParentHostPolicy()); assertNotEquals(0, failureReasons.singularReasonFailures().violatesParentHostPolicy()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityReportMaintainerTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java index ccea4691f10..f5fd0e0526d 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityReportMaintainerTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java @@ -20,7 +20,6 @@ import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.time.Duration; import java.time.Instant; import java.util.*; import java.util.stream.Collectors; @@ -29,22 +28,23 @@ import java.util.stream.IntStream; /** * @author mgimle */ -public class CapacityReportMaintainerTester { +public class CapacityCheckerTester { public static final Zone zone = new Zone(Environment.prod, RegionName.from("us-east")); // Components with state public final ManualClock clock = new ManualClock(); public final NodeRepository nodeRepository; + public CapacityChecker capacityChecker; - CapacityReportMaintainerTester() { + CapacityCheckerTester() { Curator curator = new MockCurator(); NodeFlavors f = new NodeFlavors(new FlavorConfigBuilder().build()); nodeRepository = new NodeRepository(f, curator, clock, zone, new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), true); } - CapacityReportMaintainer makeCapacityReportMaintainer() { - return new CapacityReportMaintainer(nodeRepository, new MetricsReporterTest.TestMetric(), Duration.ofDays(1)); + private void updateCapacityChecker() { + this.capacityChecker = new CapacityChecker(this.nodeRepository); } List<NodeModel> createDistinctChildren(int amount, List<NodeResources> childResources) { @@ -167,9 +167,9 @@ public class CapacityReportMaintainerTester { nodes.addAll(createEmptyHosts(numHosts, numEmptyHosts, emptyHostExcessCapacity, emptyHostExcessIps)); nodeRepository.addNodes(nodes); + updateCapacityChecker(); } - NodeResources containingNodeResources(List<NodeResources> resources, NodeResources excessCapacity) { NodeResources usedByChildren = resources.stream() .reduce(new NodeResources(0, 0, 0), NodeResources::add); @@ -278,6 +278,7 @@ public class CapacityReportMaintainerTester { } nodeRepository.addNodes(nodes); + updateCapacityChecker(); } void cleanRepository() { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java index bfb24d30284..35fa5adaeff 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.nio.charset.CharacterCodingException; import java.nio.charset.StandardCharsets; import java.util.Arrays; +import java.util.List; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -819,6 +820,30 @@ public class RestApiTest { "{\"message\":\"Cancelled outstanding requests for firmware checks\"}"); } + @Test + public void test_capacity() throws Exception { + assertFile(new Request("http://localhost:8080/nodes/v2/capacity/?json=true"), "capacity-zone.json"); + + List<String> hostsToRemove = List.of( + "%22dockerhost1.yahoo.com%22", + "%22dockerhost2.yahoo.com%22", + "%22dockerhost3.yahoo.com%22", + "%22dockerhost4.yahoo.com%22" + ); + String requestUriTemplate = + "http://localhost:8080/nodes/v2/capacity/?json=true&hosts=[%s]" + .replaceAll("\\[", "%%5B") + .replaceAll("]", "%%5D"); + + assertFile(new Request(String.format(requestUriTemplate, + String.join(",", hostsToRemove.subList(0, 3)))), + "capacity-hostremoval-possible.json"); + assertFile(new Request(String.format(requestUriTemplate, + String.join(",", hostsToRemove))), + "capacity-hostremoval-impossible.json"); + } + + /** Tests the rendering of each node separately to make it easier to find errors */ @Test public void test_single_node_rendering() throws Exception { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/capacity-hostremoval-impossible.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/capacity-hostremoval-impossible.json new file mode 100644 index 00000000000..f3c73e61c91 --- /dev/null +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/capacity-hostremoval-impossible.json @@ -0,0 +1,20 @@ +{ + "hostsToRemove": [ + "dockerhost1.yahoo.com", + "dockerhost2.yahoo.com", + "dockerhost3.yahoo.com", + "dockerhost4.yahoo.com" + ], + "removalPossible": false, + "history": [ + { + "tenant": "host4.yahoo.com", + "newParent": "dockerhost5.yahoo.com", + "eligibleParents": 1 + }, + { + "tenant": "test-node-pool-101-2", + "eligibleParents": 0 + } + ] +}
\ No newline at end of file diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/capacity-hostremoval-possible.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/capacity-hostremoval-possible.json new file mode 100644 index 00000000000..b896fd9d63a --- /dev/null +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/capacity-hostremoval-possible.json @@ -0,0 +1,20 @@ +{ + "hostsToRemove": [ + "dockerhost1.yahoo.com", + "dockerhost2.yahoo.com", + "dockerhost3.yahoo.com" + ], + "removalPossible": true, + "history": [ + { + "tenant": "host4.yahoo.com", + "newParent": "dockerhost4.yahoo.com", + "eligibleParents": 2 + }, + { + "tenant": "test-node-pool-101-2", + "newParent": "dockerhost5.yahoo.com", + "eligibleParents": 1 + } + ] +}
\ No newline at end of file diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/capacity-zone.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/capacity-zone.json new file mode 100644 index 00000000000..9895948e69d --- /dev/null +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/capacity-zone.json @@ -0,0 +1,46 @@ +{ + "totalHosts": 5, + "couldLoseHosts": 4, + "failedTenantParent": "dockerhost1.yahoo.com", + "failedTenant": "host4.yahoo.com", + "failedTenantResources": "[vcpu: 1.0, memory: 1.0 Gb, disk 100.0 Gb]", + "failedTenantAllocation": "allocated to tenant3.application3.instance3 as 'content/id3/0/0'", + "hostCandidateRejectionReasons": { + "singularReasonFailures": { + "insufficientVcpu": 0, + "insufficientMemoryGb": 0, + "insufficientDiskGb": 0, + "incompatibleDiskSpeed": 0, + "insufficientAvailableIps": 0, + "violatesParentHostPolicy": 1 + }, + "totalFailures": { + "insufficientVcpu": 0, + "insufficientMemoryGb": 0, + "insufficientDiskGb": 0, + "incompatibleDiskSpeed": 0, + "insufficientAvailableIps": 0, + "violatesParentHostPolicy": 1 + } + }, + "details": { + "hostsToRemove": [ + "dockerhost2.yahoo.com", + "dockerhost1.yahoo.com", + "dockerhost4.yahoo.com", + "dockerhost3.yahoo.com" + ], + "removalPossible": false, + "history": [ + { + "tenant": "test-node-pool-101-2", + "newParent": "dockerhost5.yahoo.com", + "eligibleParents": 1 + }, + { + "tenant": "host4.yahoo.com", + "eligibleParents": 0 + } + ] + } +}
\ No newline at end of file |