diff options
107 files changed, 2025 insertions, 1440 deletions
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/CasWriteFailed.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/CasWriteFailed.java new file mode 100644 index 00000000000..c62d8d4bcd5 --- /dev/null +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/CasWriteFailed.java @@ -0,0 +1,23 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.clustercontroller.core.database; + +/** + * Exception used to signal that a write intended to overwrite a value previously + * read encountered an underlying version conflict during an atomic compare-and-swap + * operation. This generally means that another node has written to the value since + * we last read it, and that the information we hold may be stale. + * + * Upon receiving such an exception, the caller should no longer assume it holds + * up-to-date information and should drop and roles that build on top of such an + * assumption (such as leadership sessions). + */ +public class CasWriteFailed extends RuntimeException { + + public CasWriteFailed(String message) { + super(message); + } + + public CasWriteFailed(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/Database.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/Database.java index c98e362c2d2..4b0461200a4 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/Database.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/Database.java @@ -46,7 +46,12 @@ public abstract class Database { * store the version in the database, such that if another fleetcontroller takes over as master it will use a * higher version system state. * + * Precondition: retrieveLatestSystemStateVersion() MUST have been called at least once prior to calling + * this method. + * * @return True if request succeeded. False if not. + * @throws CasWriteFailed if the expected version of the znode did not match what was actually stored in the DB. + * In this case, the write has NOT been applied. */ public abstract boolean storeLatestSystemStateVersion(int version) throws InterruptedException; @@ -84,6 +89,17 @@ public abstract class Database { */ public abstract Map<Node, Long> retrieveStartTimestamps() throws InterruptedException; + /** + * Stores the last published cluster state bundle synchronously into ZooKeeper. + * + * Precondition: retrieveLastPublishedStateBundle() MUST have been called at least once prior to calling + * this method. + * + * @return true if the write is known to have been successful, false otherwise. If false is returned, the + * write may or may not have taken place. + * @throws CasWriteFailed if the expected version of the znode did not match what was actually stored in the DB. + * In this case, the write has NOT been applied. + */ public abstract boolean storeLastPublishedStateBundle(ClusterStateBundle stateBundle) throws InterruptedException; public abstract ClusterStateBundle retrieveLastPublishedStateBundle() throws InterruptedException; diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/DatabaseHandler.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/DatabaseHandler.java index f30b86130c2..b9d02f16090 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/DatabaseHandler.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/DatabaseHandler.java @@ -105,8 +105,7 @@ public class DatabaseHandler { } public void shutdown(FleetController fleetController) { - reset(); - fleetController.lostDatabaseConnection(); + relinquishDatabaseConnectivity(fleetController); } public boolean isClosed() { return database == null || database.isClosed(); } @@ -232,68 +231,85 @@ public class DatabaseHandler { didWork = true; connect(context.getCluster(), currentTime); } - synchronized (databaseMonitor) { - if (database == null || database.isClosed()) { - return didWork; - } - if (pendingStore.masterVote != null) { - didWork = true; - log.log(LogLevel.DEBUG, "Fleetcontroller " + nodeIndex + ": Attempting to store master vote " - + pendingStore.masterVote + " into zookeeper."); - if (database.storeMasterVote(pendingStore.masterVote)) { - log.log(LogLevel.DEBUG, "Fleetcontroller " + nodeIndex + ": Managed to store master vote " - + pendingStore.masterVote + " into zookeeper."); - currentlyStored.masterVote = pendingStore.masterVote; - pendingStore.masterVote = null; - } else { - log.log(LogLevel.WARNING, "Fleetcontroller " + nodeIndex + ": Failed to store master vote"); + try { + synchronized (databaseMonitor) { + if (database == null || database.isClosed()) { return didWork; } + didWork |= performZooKeeperWrites(); } - if (pendingStore.lastSystemStateVersion != null) { - didWork = true; - log.log(LogLevel.DEBUG, "Fleetcontroller " + nodeIndex - + ": Attempting to store last system state version " + pendingStore.lastSystemStateVersion - + " into zookeeper."); - // TODO guard version write with a CaS predicated on the version we last read/wrote. - // TODO Drop leadership status if there is a mismatch, as it implies we're racing with another leader. - if (database.storeLatestSystemStateVersion(pendingStore.lastSystemStateVersion)) { - currentlyStored.lastSystemStateVersion = pendingStore.lastSystemStateVersion; - pendingStore.lastSystemStateVersion = null; - } else { - return didWork; - } + } catch (CasWriteFailed e) { + log.log(LogLevel.WARNING, String.format("CaS write to ZooKeeper failed, another controller " + + "has likely taken over ownership: %s", e.getMessage())); + // Clear DB and master election state. This shall trigger a full re-fetch of all + // version and election-related metadata. + relinquishDatabaseConnectivity(context.getFleetController()); + } + return didWork; + } + + private void relinquishDatabaseConnectivity(FleetController fleetController) { + reset(); + fleetController.lostDatabaseConnection(); + } + + private boolean performZooKeeperWrites() throws InterruptedException { + boolean didWork = false; + if (pendingStore.masterVote != null) { + didWork = true; + log.log(LogLevel.DEBUG, "Fleetcontroller " + nodeIndex + ": Attempting to store master vote " + + pendingStore.masterVote + " into zookeeper."); + if (database.storeMasterVote(pendingStore.masterVote)) { + log.log(LogLevel.DEBUG, "Fleetcontroller " + nodeIndex + ": Managed to store master vote " + + pendingStore.masterVote + " into zookeeper."); + currentlyStored.masterVote = pendingStore.masterVote; + pendingStore.masterVote = null; + } else { + log.log(LogLevel.WARNING, "Fleetcontroller " + nodeIndex + ": Failed to store master vote"); + return true; } - if (pendingStore.startTimestamps != null) { - didWork = true; - log.log(LogLevel.DEBUG, "Fleetcontroller " + nodeIndex + ": Attempting to store " - + pendingStore.startTimestamps.size() + " start timestamps into zookeeper."); - if (database.storeStartTimestamps(pendingStore.startTimestamps)) { - currentlyStored.startTimestamps = pendingStore.startTimestamps; - pendingStore.startTimestamps = null; - } else { - return didWork; - } + } + if (pendingStore.lastSystemStateVersion != null) { + didWork = true; + log.log(LogLevel.DEBUG, "Fleetcontroller " + nodeIndex + + ": Attempting to store last system state version " + pendingStore.lastSystemStateVersion + + " into zookeeper."); + if (database.storeLatestSystemStateVersion(pendingStore.lastSystemStateVersion)) { + currentlyStored.lastSystemStateVersion = pendingStore.lastSystemStateVersion; + pendingStore.lastSystemStateVersion = null; + } else { + return true; } - if (pendingStore.wantedStates != null) { - didWork = true; - log.log(LogLevel.DEBUG, "Fleetcontroller " + nodeIndex + ": Attempting to store " - + pendingStore.wantedStates.size() + " wanted states into zookeeper."); - if (database.storeWantedStates(pendingStore.wantedStates)) { - currentlyStored.wantedStates = pendingStore.wantedStates; - pendingStore.wantedStates = null; - } else { - return didWork; - } + } + if (pendingStore.startTimestamps != null) { + didWork = true; + log.log(LogLevel.DEBUG, "Fleetcontroller " + nodeIndex + ": Attempting to store " + + pendingStore.startTimestamps.size() + " start timestamps into zookeeper."); + if (database.storeStartTimestamps(pendingStore.startTimestamps)) { + currentlyStored.startTimestamps = pendingStore.startTimestamps; + pendingStore.startTimestamps = null; + } else { + return true; } - if (pendingStore.clusterStateBundle != null) { - didWork = true; - if (database.storeLastPublishedStateBundle(pendingStore.clusterStateBundle)) { - currentlyStored.clusterStateBundle = pendingStore.clusterStateBundle; - pendingStore.clusterStateBundle = null; - } else { - return true; - } + } + if (pendingStore.wantedStates != null) { + didWork = true; + log.log(LogLevel.DEBUG, "Fleetcontroller " + nodeIndex + ": Attempting to store " + + pendingStore.wantedStates.size() + " wanted states into zookeeper."); + if (database.storeWantedStates(pendingStore.wantedStates)) { + currentlyStored.wantedStates = pendingStore.wantedStates; + pendingStore.wantedStates = null; + } else { + return true; + } + } + if (pendingStore.clusterStateBundle != null) { + didWork = true; + if (database.storeLastPublishedStateBundle(pendingStore.clusterStateBundle)) { + currentlyStored.clusterStateBundle = pendingStore.clusterStateBundle; + pendingStore.clusterStateBundle = null; + } else { + return true; } } return didWork; diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/ZooKeeperDatabase.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/ZooKeeperDatabase.java index 77ca7e12711..880f2cdaf19 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/ZooKeeperDatabase.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/ZooKeeperDatabase.java @@ -35,6 +35,10 @@ public class ZooKeeperDatabase extends Database { private final int nodeIndex; private final MasterDataGatherer masterDataGatherer; private boolean reportErrors = true; + // Expected ZK znode versions. Note: these are _not_ -1 as that would match anything. + // We expect the caller to invoke the load methods prior to calling any store methods. + private int lastKnownStateBundleZNodeVersion = -2; + private int lastKnownStateVersionZNodeVersion = -2; public void stopErrorReporting() { reportErrors = false; @@ -196,10 +200,14 @@ public class ZooKeeperDatabase extends Database { byte data[] = Integer.toString(version).getBytes(utf8); try{ log.log(LogLevel.INFO, String.format("Fleetcontroller %d: Storing new cluster state version in ZooKeeper: %d", nodeIndex, version)); - session.setData(zooKeeperRoot + "latestversion", data, -1); + var stat = session.setData(zooKeeperRoot + "latestversion", data, lastKnownStateVersionZNodeVersion); + lastKnownStateVersionZNodeVersion = stat.getVersion(); return true; } catch (InterruptedException e) { throw (InterruptedException) new InterruptedException("Interrupted").initCause(e); + } catch (KeeperException.BadVersionException e) { + throw new CasWriteFailed(String.format("version mismatch in cluster state version znode (expected %d): %s", + lastKnownStateVersionZNodeVersion, e.getMessage()), e); } catch (Exception e) { maybeLogExceptionWarning(e, "Failed to store latest system state version used " + version); return false; @@ -209,10 +217,13 @@ public class ZooKeeperDatabase extends Database { public Integer retrieveLatestSystemStateVersion() throws InterruptedException { Stat stat = new Stat(); try{ - log.log(LogLevel.DEBUG, "Fleetcontroller " + nodeIndex + ": Fetching latest cluster state at '" + zooKeeperRoot + "latestversion'"); + log.log(LogLevel.DEBUG, () -> String.format("Fleetcontroller %d: Fetching latest cluster state at '%slatestversion'", + nodeIndex, zooKeeperRoot)); byte[] data = session.getData(zooKeeperRoot + "latestversion", false, stat); + lastKnownStateVersionZNodeVersion = stat.getVersion(); final Integer versionNumber = Integer.valueOf(new String(data, utf8)); - log.log(LogLevel.INFO, String.format("Fleetcontroller %d: Read cluster state version %d from ZooKeeper", nodeIndex, versionNumber)); + log.log(LogLevel.INFO, String.format("Fleetcontroller %d: Read cluster state version %d from ZooKeeper " + + "(znode version %d)", nodeIndex, versionNumber, stat.getVersion())); return versionNumber; } catch (InterruptedException e) { throw (InterruptedException) new InterruptedException("Interrupted").initCause(e); @@ -336,12 +347,16 @@ public class ZooKeeperDatabase extends Database { EnvelopedClusterStateBundleCodec envelopedBundleCodec = new SlimeClusterStateBundleCodec(); byte[] encodedBundle = envelopedBundleCodec.encodeWithEnvelope(stateBundle); try{ - log.log(LogLevel.DEBUG, () -> String.format("Fleetcontroller %d: Storing published state bundle %s at '%spublished_state_bundle'", - nodeIndex, stateBundle, zooKeeperRoot)); - // TODO CAS on expected zknode version - session.setData(zooKeeperRoot + "published_state_bundle", encodedBundle, -1); + log.log(LogLevel.DEBUG, () -> String.format("Fleetcontroller %d: Storing published state bundle %s at " + + "'%spublished_state_bundle' with expected znode version %d", + nodeIndex, stateBundle, zooKeeperRoot, lastKnownStateBundleZNodeVersion)); + var stat = session.setData(zooKeeperRoot + "published_state_bundle", encodedBundle, lastKnownStateBundleZNodeVersion); + lastKnownStateBundleZNodeVersion = stat.getVersion(); } catch (InterruptedException e) { throw (InterruptedException) new InterruptedException("Interrupted").initCause(e); + } catch (KeeperException.BadVersionException e) { + throw new CasWriteFailed(String.format("version mismatch in cluster state bundle znode (expected %d): %s", + lastKnownStateBundleZNodeVersion, e.getMessage()), e); } catch (Exception e) { maybeLogExceptionWarning(e, "Failed to store last published cluster state bundle in ZooKeeper"); return false; @@ -354,6 +369,7 @@ public class ZooKeeperDatabase extends Database { Stat stat = new Stat(); try { byte[] data = session.getData(zooKeeperRoot + "published_state_bundle", false, stat); + lastKnownStateBundleZNodeVersion = stat.getVersion(); if (data != null && data.length != 0) { EnvelopedClusterStateBundleCodec envelopedBundleCodec = new SlimeClusterStateBundleCodec(); return envelopedBundleCodec.decodeWithEnvelope(data); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ZooKeeperDatabaseTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ZooKeeperDatabaseTest.java index e5200b63a7e..6062f45c4de 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ZooKeeperDatabaseTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ZooKeeperDatabaseTest.java @@ -1,9 +1,12 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.core; +import com.yahoo.vespa.clustercontroller.core.database.CasWriteFailed; import com.yahoo.vespa.clustercontroller.core.database.Database; import com.yahoo.vespa.clustercontroller.core.database.ZooKeeperDatabase; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import java.io.IOException; import java.time.Duration; @@ -14,6 +17,9 @@ import static org.mockito.Mockito.mock; public class ZooKeeperDatabaseTest { + @Rule + public ExpectedException expectedException = ExpectedException.none(); + private static class Fixture implements AutoCloseable { final ZooKeeperTestServer zkServer; ClusterFixture clusterFixture; @@ -53,9 +59,8 @@ public class ZooKeeperDatabaseTest { public void can_store_and_load_cluster_state_bundle_from_database() throws Exception { try (Fixture f = new Fixture()) { f.createDatabase(); - ClusterStateBundle bundleToStore = ClusterStateBundleUtil.makeBundle("distributor:2 storage:2", - StateMapping.of("default", "distributor:2 storage:2 .0.s:d"), - StateMapping.of("upsidedown", "distributor:2 .0.s:d storage:2")); + f.db().retrieveLastPublishedStateBundle(); // Must be called once prior to prime last known znode version + ClusterStateBundle bundleToStore = dummyBundle(); f.db().storeLastPublishedStateBundle(bundleToStore); ClusterStateBundle bundleReceived = f.db().retrieveLastPublishedStateBundle(); @@ -63,6 +68,32 @@ public class ZooKeeperDatabaseTest { } } + private static ClusterStateBundle dummyBundle() { + return ClusterStateBundleUtil.makeBundle("distributor:2 storage:2", + StateMapping.of("default", "distributor:2 storage:2 .0.s:d"), + StateMapping.of("upsidedown", "distributor:2 .0.s:d storage:2")); + } + + @Test + public void storing_cluster_state_bundle_with_mismatching_expected_znode_version_throws_exception() throws Exception { + expectedException.expect(CasWriteFailed.class); + expectedException.expectMessage("version mismatch in cluster state bundle znode (expected -2)"); + try (Fixture f = new Fixture()) { + f.createDatabase(); + f.db().storeLastPublishedStateBundle(dummyBundle()); + } + } + + @Test + public void storing_cluster_state_version_with_mismatching_expected_znode_version_throws_exception() throws Exception { + expectedException.expect(CasWriteFailed.class); + expectedException.expectMessage("version mismatch in cluster state version znode (expected -2)"); + try (Fixture f = new Fixture()) { + f.createDatabase(); + f.db().storeLatestSystemStateVersion(12345); + } + } + @Test public void empty_state_bundle_is_returned_if_no_bundle_already_stored_in_database() throws Exception { try (Fixture f = new Fixture()) { 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/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java index 5ffc7293742..e815fd9a5f9 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java @@ -133,7 +133,6 @@ public class ConfigProxyRpcServer implements Runnable, TargetWatcher, RpcServer dispatchRpcRequest(req, () -> { JRTServerConfigRequest request = JRTServerConfigRequestV3.createFromRequest(req); if (isProtocolVersionSupported(request)) { - proxyServer.getStatistics().incRpcRequests(); req.target().addWatcher(this); getConfigImpl(request); return; diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyStatistics.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyStatistics.java deleted file mode 100644 index 314a4b0cb11..00000000000 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyStatistics.java +++ /dev/null @@ -1,104 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.proxy; - -import com.yahoo.log.LogLevel; -import com.yahoo.log.event.Event; - -/** - * Statistics/metrics for config proxy. - * //TODO Use metrics framework - * - * @author hmusum - */ -class ConfigProxyStatistics implements Runnable { - static final long defaultEventInterval = 5 * 60; // in seconds - - private final long eventInterval; // in seconds - private boolean stopped; - private long lastRun = System.currentTimeMillis(); - - /* Number of RPC getConfig requests */ - private long rpcRequests = 0; - private long processedRequests = 0; - private long errors = 0; - private long delayedResponses = 0; - - ConfigProxyStatistics() { - this(defaultEventInterval); - } - - ConfigProxyStatistics(long eventInterval) { - this.eventInterval = eventInterval; - } - - // Send events every eventInterval seconds - public void run() { - while (true) { - try { - Thread.sleep(1000); - } catch (InterruptedException e) { - ProxyServer.log.log(LogLevel.WARNING, e.getMessage()); - } - if (stopped) { - return; - } - ProxyServer.log.log(LogLevel.SPAM, "Running ConfigProxyStatistics"); - // Only send events every eventInterval seconds - if ((System.currentTimeMillis() - lastRun) > eventInterval * 1000) { - lastRun = System.currentTimeMillis(); - sendEvents(); - } - } - } - - private void sendEvents() { - Event.count("rpc_requests", rpcRequests()); - Event.count("processed_messages", processedRequests()); - Event.count("errors", errors()); - Event.value("delayed_responses", delayedResponses()); - } - - void stop() { - stopped = true; - } - - Long getEventInterval() { - return eventInterval; - } - - void incRpcRequests() { - rpcRequests++; - } - - void incProcessedRequests() { - processedRequests++; - } - - void incErrorCount() { - errors++; - } - - long processedRequests() { - return processedRequests; - } - - long rpcRequests() { - return rpcRequests; - } - - long errors() { - return errors; - } - - long delayedResponses() { - return delayedResponses; - } - - void delayedResponses(long count) { - delayedResponses = count; - } - - void decDelayedResponses() { - delayedResponses--; - } -} diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/Mode.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/Mode.java index da6e7d975cb..fdc2b886701 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/Mode.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/Mode.java @@ -31,9 +31,6 @@ class Mode { Mode(String modeString) { switch (modeString.toLowerCase()) { - case "" : - mode = ModeName.DEFAULT; - break; case "default" : mode = ModeName.DEFAULT; break; @@ -41,7 +38,7 @@ class Mode { mode = ModeName.MEMORYCACHE; break; default: - throw new IllegalArgumentException("Unrecognized mode'" + modeString + "' supplied"); + throw new IllegalArgumentException("Unrecognized mode '" + modeString + "' supplied"); } } diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java index 4bf3bd5a786..f4f2308f261 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java @@ -28,7 +28,7 @@ import static java.util.concurrent.TimeUnit.SECONDS; /** * A proxy server that handles RPC config requests. The proxy can run in two modes: * 'default' and 'memorycache', where the last one will not get config from an upstream - * config source, but will serve config only from memory cache. + * config source, but will serve config from memory cache only. * * @author hmusum */ @@ -38,7 +38,7 @@ public class ProxyServer implements Runnable { private static final int JRT_TRANSPORT_THREADS = 4; static final String DEFAULT_PROXY_CONFIG_SOURCES = "tcp/localhost:19070"; - final static Logger log = Logger.getLogger(ProxyServer.class.getName()); + private final static Logger log = Logger.getLogger(ProxyServer.class.getName()); private final AtomicBoolean signalCaught = new AtomicBoolean(false); // Scheduled executor that periodically checks for requests that have timed out and response should be returned to clients @@ -52,7 +52,6 @@ public class ProxyServer implements Runnable { private volatile ConfigSourceClient configClient; - private final ConfigProxyStatistics statistics; private final TimingValues timingValues; private final MemoryCache memoryCache; private static final double timingValuesRatio = 0.8; @@ -72,33 +71,28 @@ public class ProxyServer implements Runnable { defaultTimingValues = tv; } - private ProxyServer(Spec spec, DelayedResponses delayedResponses, ConfigSourceSet source, - ConfigProxyStatistics statistics, TimingValues timingValues, - boolean delayedResponseHandling, MemoryCache memoryCache, - ConfigSourceClient configClient) { - this.delayedResponses = delayedResponses; + private ProxyServer(Spec spec, ConfigSourceSet source, TimingValues timingValues, + boolean delayedResponseHandling, MemoryCache memoryCache, ConfigSourceClient configClient) { + this.delayedResponses = new DelayedResponses(); this.configSource = source; log.log(LogLevel.DEBUG, "Using config source '" + source); - this.statistics = statistics; this.timingValues = timingValues; this.delayedResponseHandling = delayedResponseHandling; this.memoryCache = memoryCache; this.rpcServer = createRpcServer(spec); - this.configClient = createClient(rpcServer, statistics, delayedResponses, source, timingValues, memoryCache, configClient); + this.configClient = createClient(rpcServer, delayedResponses, source, timingValues, memoryCache, configClient); this.fileDistributionAndUrlDownload = new FileDistributionAndUrlDownload(supervisor, source); } static ProxyServer createTestServer(ConfigSourceSet source) { - return createTestServer(source, null, new MemoryCache(), new ConfigProxyStatistics()); + return createTestServer(source, null, new MemoryCache()); } static ProxyServer createTestServer(ConfigSourceSet source, ConfigSourceClient configSourceClient, - MemoryCache memoryCache, - ConfigProxyStatistics statistics) { + MemoryCache memoryCache) { final boolean delayedResponseHandling = false; - return new ProxyServer(null, new DelayedResponses(), - source, statistics, defaultTimingValues(), delayedResponseHandling, + return new ProxyServer(null, source, defaultTimingValues(), delayedResponseHandling, memoryCache, configSourceClient); } @@ -120,7 +114,6 @@ public class ProxyServer implements Runnable { } RawConfig resolveConfig(JRTServerConfigRequest req) { - statistics.incProcessedRequests(); // Calling getConfig() will either return with an answer immediately or // create a background thread that retrieves config from the server and // calls updateSubscribers when new config is returned from the config source. @@ -155,12 +148,11 @@ public class ProxyServer implements Runnable { } } - private ConfigSourceClient createClient(RpcServer rpcServer, ConfigProxyStatistics statistics, - DelayedResponses delayedResponses, + private ConfigSourceClient createClient(RpcServer rpcServer, DelayedResponses delayedResponses, ConfigSourceSet source, TimingValues timingValues, MemoryCache memoryCache, ConfigSourceClient client) { return (client == null) - ? new RpcConfigSourceClient(rpcServer, source, statistics, memoryCache, timingValues, delayedResponses) + ? new RpcConfigSourceClient(rpcServer, source, memoryCache, timingValues, delayedResponses) : client; } @@ -169,7 +161,7 @@ public class ProxyServer implements Runnable { } private RpcConfigSourceClient createRpcClient() { - return new RpcConfigSourceClient(rpcServer, configSource, statistics, memoryCache, timingValues, delayedResponses); + return new RpcConfigSourceClient(rpcServer, configSource, memoryCache, timingValues, delayedResponses); } private void setupSignalHandler() { @@ -202,15 +194,9 @@ public class ProxyServer implements Runnable { port = Integer.parseInt(args[0]); } Event.started("configproxy"); - ConfigProxyStatistics statistics = new ConfigProxyStatistics(properties.eventInterval); - Thread t = new Thread(statistics); - t.setName("Metrics generator"); - t.setDaemon(true); - t.start(); ConfigSourceSet configSources = new ConfigSourceSet(properties.configSources); - DelayedResponses delayedResponses = new DelayedResponses(); - ProxyServer proxyServer = new ProxyServer(new Spec(null, port), delayedResponses, configSources, statistics, + ProxyServer proxyServer = new ProxyServer(new Spec(null, port), configSources, defaultTimingValues(), true, new MemoryCache(), null); // catch termination and interrupt signal proxyServer.setupSignalHandler(); @@ -221,18 +207,14 @@ public class ProxyServer implements Runnable { } static Properties getSystemProperties() { - // Read system properties - long eventInterval = Long.getLong("eventinterval", ConfigProxyStatistics.defaultEventInterval); final String[] inputConfigSources = System.getProperty("proxyconfigsources", DEFAULT_PROXY_CONFIG_SOURCES).split(","); - return new Properties(eventInterval, inputConfigSources); + return new Properties(inputConfigSources); } static class Properties { - final long eventInterval; final String[] configSources; - Properties(long eventInterval, String[] configSources) { - this.eventInterval = eventInterval; + Properties(String[] configSources) { this.configSources = configSources; } } @@ -245,10 +227,6 @@ public class ProxyServer implements Runnable { return timingValues; } - ConfigProxyStatistics getStatistics() { - return statistics; - } - // Cancels all config instances and flushes the cache. When this method returns, // the cache will not be updated again before someone calls getConfig(). private synchronized void flush() { @@ -261,9 +239,6 @@ public class ProxyServer implements Runnable { if (rpcServer != null) rpcServer.shutdown(); if (delayedResponseScheduler != null) delayedResponseScheduler.cancel(true); flush(); - if (statistics != null) { - statistics.stop(); - } fileDistributionAndUrlDownload.close(); } diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java index c9f43ac48e2..d809a3c97ed 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java @@ -38,7 +38,6 @@ class RpcConfigSourceClient implements ConfigSourceClient { private final HashMap<ConfigCacheKey, Subscriber> activeSubscribers = new HashMap<>(); private final Object activeSubscribersLock = new Object(); private final MemoryCache memoryCache; - private final ConfigProxyStatistics statistics; private final DelayedResponses delayedResponses; private final TimingValues timingValues; @@ -48,13 +47,11 @@ class RpcConfigSourceClient implements ConfigSourceClient { RpcConfigSourceClient(RpcServer rpcServer, ConfigSourceSet configSourceSet, - ConfigProxyStatistics statistics, MemoryCache memoryCache, TimingValues timingValues, DelayedResponses delayedResponses) { this.rpcServer = rpcServer; this.configSourceSet = configSourceSet; - this.statistics = statistics; this.memoryCache = memoryCache; this.delayedResponses = delayedResponses; this.timingValues = timingValues; @@ -122,7 +119,6 @@ class RpcConfigSourceClient implements ConfigSourceClient { // happens at the same time DelayedResponse delayedResponse = new DelayedResponse(request); delayedResponses.add(delayedResponse); - statistics.delayedResponses(delayedResponses.size()); final ConfigCacheKey configCacheKey = new ConfigCacheKey(input.getKey(), input.getDefMd5()); RawConfig cachedConfig = memoryCache.get(configCacheKey); @@ -139,7 +135,6 @@ class RpcConfigSourceClient implements ConfigSourceClient { // unless another thread already did it ret = cachedConfig; } - statistics.decDelayedResponses(); } if (!cachedConfig.isError() && cachedConfig.getGeneration() > 0) { needToGetConfig = false; @@ -220,7 +215,6 @@ class RpcConfigSourceClient implements ConfigSourceClient { */ public void updateSubscribers(RawConfig config) { log.log(LogLevel.DEBUG, () -> "Config updated for " + config.getKey() + "," + config.getGeneration()); - if (config.isError()) { statistics.incErrorCount(); } DelayQueue<DelayedResponse> responseDelayQueue = delayedResponses.responses(); log.log(LogLevel.SPAM, () -> "Delayed response queue: " + responseDelayQueue); if (responseDelayQueue.size() == 0) { diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ModeTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ModeTest.java deleted file mode 100644 index 1a6bbd11b59..00000000000 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ModeTest.java +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.proxy; - -import org.junit.Test; - -import java.util.HashSet; -import java.util.Set; - -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; - -/** - * @author hmusum - */ -public class ModeTest { - - @Test - public void basic() { - Mode mode = new Mode(); - assertModeName(Mode.ModeName.DEFAULT, mode); - assertTrue(mode.isDefault()); - - mode = new Mode(""); - assertModeName(Mode.ModeName.DEFAULT, mode); - assertTrue(mode.isDefault()); - - mode = new Mode(Mode.ModeName.DEFAULT.name()); - assertModeName(Mode.ModeName.DEFAULT, mode); - assertTrue(mode.isDefault()); - - mode = new Mode(Mode.ModeName.MEMORYCACHE.name()); - assertModeName(Mode.ModeName.MEMORYCACHE, mode); - assertTrue(mode.isMemoryCache()); - - assertTrue(new Mode(Mode.ModeName.DEFAULT.name()).requiresConfigSource()); - - assertFalse(new Mode(Mode.ModeName.MEMORYCACHE.name()).requiresConfigSource()); - - Set<String> modes = new HashSet<>(); - for (Mode.ModeName modeName : Mode.ModeName.values()) { - modes.add(modeName.name().toLowerCase()); - } - - assertThat(Mode.modes(), is(modes)); - - assertFalse(Mode.validModeName("foo")); - - assertThat(mode.toString(), is(Mode.ModeName.MEMORYCACHE.name().toLowerCase())); - } - - @Test(expected = IllegalArgumentException.class) - public void failWhenInvalidMode() { - new Mode("invalid_mode"); - } - - private void assertModeName(Mode.ModeName expected, Mode actual) { - assertThat(actual.name(), is(expected.name().toLowerCase())); - } -} diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java index 9d6d0ca2a39..1c64631d205 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java @@ -23,7 +23,6 @@ public class ProxyServerTest { private final MemoryCache memoryCache = new MemoryCache(); private final MockConfigSource source = new MockConfigSource(); private MockConfigSourceClient client = new MockConfigSourceClient(source, memoryCache); - private final ConfigProxyStatistics statistics = new ConfigProxyStatistics(); private ProxyServer proxy; static final RawConfig fooConfig = ConfigTester.fooConfig; @@ -42,7 +41,7 @@ public class ProxyServerTest { source.clear(); source.put(fooConfig.getKey(), createConfigWithNextConfigGeneration(fooConfig, 0)); source.put(errorConfigKey, createConfigWithNextConfigGeneration(fooConfig, ErrorCode.UNKNOWN_DEFINITION)); - proxy = ProxyServer.createTestServer(source, client, memoryCache, statistics); + proxy = ProxyServer.createTestServer(source, client, memoryCache); } @After @@ -64,27 +63,6 @@ public class ProxyServerTest { assertThat(res.getPayload().toString(), is(ConfigTester.fooPayload.toString())); assertEquals(1, memoryCache.size()); assertThat(memoryCache.get(new ConfigCacheKey(fooConfig.getKey(), fooConfig.getDefMd5())), is(res)); - - - assertEquals(1, statistics.processedRequests()); - assertEquals(0, statistics.rpcRequests()); - assertEquals(0, statistics.errors()); - assertEquals(0, statistics.delayedResponses()); - - statistics.incProcessedRequests(); - statistics.incRpcRequests(); - statistics.incErrorCount(); - statistics.delayedResponses(1); - - assertEquals(2, statistics.processedRequests()); - assertEquals(1, statistics.rpcRequests()); - assertEquals(1, statistics.errors()); - assertEquals(1, statistics.delayedResponses()); - - statistics.decDelayedResponses(); - assertEquals(0, statistics.delayedResponses()); - - assertEquals(ConfigProxyStatistics.defaultEventInterval, statistics.getEventInterval().longValue()); } /** @@ -100,6 +78,14 @@ public class ProxyServerTest { assertThat(proxy.getMode().name(), is(mode)); } + // Try setting an invalid mode + try { + proxy.setMode("invalid"); + assert (false); + } catch (IllegalArgumentException e) { + assertEquals("Unrecognized mode 'invalid' supplied", e.getMessage()); + } + // Also switch to DEFAULT mode, as that is not covered above proxy.setMode("default"); assertTrue(proxy.getMode().isDefault()); @@ -228,7 +214,6 @@ public class ProxyServerTest { @Test public void testReadingSystemProperties() { ProxyServer.Properties properties = ProxyServer.getSystemProperties(); - assertThat(properties.eventInterval, is(ConfigProxyStatistics.defaultEventInterval)); assertThat(properties.configSources.length, is(1)); assertThat(properties.configSources[0], is(ProxyServer.DEFAULT_PROXY_CONFIG_SOURCES)); } diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClientTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClientTest.java index 7f762955b92..35f1dd8fcd8 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClientTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClientTest.java @@ -18,7 +18,6 @@ import static org.junit.Assert.assertEquals; public class RpcConfigSourceClientTest { private MockRpcServer rpcServer; - private ConfigProxyStatistics statistics; private DelayedResponses delayedResponses; private RpcConfigSourceClient rpcConfigSourceClient; @@ -29,10 +28,9 @@ public class RpcConfigSourceClientTest { @Before public void setup() { rpcServer = new MockRpcServer(); - statistics = new ConfigProxyStatistics(); delayedResponses = new DelayedResponses(); rpcConfigSourceClient = - new RpcConfigSourceClient(rpcServer, new MockConfigSource(), statistics, + new RpcConfigSourceClient(rpcServer, new MockConfigSource(), new MemoryCache(), ProxyServer.defaultTimingValues(), delayedResponses); } @@ -57,7 +55,6 @@ public class RpcConfigSourceClientTest { public void errorResponse() { configUpdatedSendResponse(ProxyServerTest.errorConfig); assertSentResponses(0); - assertEquals(1, statistics.errors()); } @Test 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/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlag.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/DefinedFlag.java index 92397fc84a7..c706a2b1e51 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlag.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/DefinedFlag.java @@ -1,12 +1,11 @@ // Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.http.flags; +package com.yahoo.vespa.configserver.flags.http; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.jdisc.Response; -import com.yahoo.vespa.config.server.http.HttpConfigResponse; import com.yahoo.vespa.flags.FlagDefinition; import com.yahoo.vespa.flags.json.DimensionHelper; @@ -42,6 +41,7 @@ public class DefinedFlag extends HttpResponse { @Override public String getContentType() { - return HttpConfigResponse.JSON_CONTENT_TYPE; + return "application/json"; } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlags.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/DefinedFlags.java index 9604c51ee4b..26d590593c0 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlags.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/DefinedFlags.java @@ -1,11 +1,10 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.http.flags; +package com.yahoo.vespa.configserver.flags.http; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.jdisc.Response; -import com.yahoo.vespa.config.server.http.HttpConfigResponse; import com.yahoo.vespa.flags.FlagDefinition; import java.io.IOException; @@ -18,8 +17,7 @@ import java.util.List; */ public class DefinedFlags extends HttpResponse { private static ObjectMapper mapper = new ObjectMapper(); - private static final Comparator<FlagDefinition> sortByFlagId = - (left, right) -> left.getUnboundFlag().id().compareTo(right.getUnboundFlag().id()); + private static final Comparator<FlagDefinition> sortByFlagId = Comparator.comparing(flagDefinition -> flagDefinition.getUnboundFlag().id()); private final List<FlagDefinition> flags; @@ -40,6 +38,6 @@ public class DefinedFlags extends HttpResponse { @Override public String getContentType() { - return HttpConfigResponse.JSON_CONTENT_TYPE; + return "application/json"; } } diff --git a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/ErrorResponse.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/ErrorResponse.java new file mode 100644 index 00000000000..b9e5c75fe22 --- /dev/null +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/ErrorResponse.java @@ -0,0 +1,66 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.configserver.flags.http; + +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Slime; + +import static com.yahoo.jdisc.Response.Status.BAD_REQUEST; +import static com.yahoo.jdisc.Response.Status.FORBIDDEN; +import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR; +import static com.yahoo.jdisc.Response.Status.METHOD_NOT_ALLOWED; +import static com.yahoo.jdisc.Response.Status.NOT_FOUND; +import static com.yahoo.jdisc.Response.Status.UNAUTHORIZED; + +/** + * A HTTP JSON response containing an error code and a message + * + * @author bratseth + */ +public class ErrorResponse extends SlimeJsonResponse { + + public enum errorCodes { + NOT_FOUND, + BAD_REQUEST, + FORBIDDEN, + METHOD_NOT_ALLOWED, + INTERNAL_SERVER_ERROR, + UNAUTHORIZED + } + + public ErrorResponse(int statusCode, String errorType, String message) { + super(statusCode, asSlimeMessage(errorType, message)); + } + + private static Slime asSlimeMessage(String errorType, String message) { + Slime slime = new Slime(); + Cursor root = slime.setObject(); + root.setString("error-code", errorType); + root.setString("message", message); + return slime; + } + + public static ErrorResponse notFoundError(String message) { + return new ErrorResponse(NOT_FOUND, errorCodes.NOT_FOUND.name(), message); + } + + public static ErrorResponse internalServerError(String message) { + return new ErrorResponse(INTERNAL_SERVER_ERROR, errorCodes.INTERNAL_SERVER_ERROR.name(), message); + } + + public static ErrorResponse badRequest(String message) { + return new ErrorResponse(BAD_REQUEST, errorCodes.BAD_REQUEST.name(), message); + } + + public static ErrorResponse forbidden(String message) { + return new ErrorResponse(FORBIDDEN, errorCodes.FORBIDDEN.name(), message); + } + + public static ErrorResponse unauthorized(String message) { + return new ErrorResponse(UNAUTHORIZED, errorCodes.UNAUTHORIZED.name(), message); + } + + public static ErrorResponse methodNotAllowed(String message) { + return new ErrorResponse(METHOD_NOT_ALLOWED, errorCodes.METHOD_NOT_ALLOWED.name(), message); + } + +} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataListResponse.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/FlagDataListResponse.java index b33fc7c2b04..efc78cb7930 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataListResponse.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/FlagDataListResponse.java @@ -1,12 +1,11 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.http.flags; +package com.yahoo.vespa.configserver.flags.http; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.jdisc.Response; -import com.yahoo.vespa.config.server.http.HttpConfigResponse; import com.yahoo.vespa.flags.FlagId; import com.yahoo.vespa.flags.json.FlagData; import com.yahoo.vespa.flags.json.wire.WireFlagDataList; @@ -54,6 +53,6 @@ public class FlagDataListResponse extends HttpResponse { @Override public String getContentType() { - return HttpConfigResponse.JSON_CONTENT_TYPE; + return "application/json"; } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataResponse.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/FlagDataResponse.java index 054b218ff2d..8ff4085df8d 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataResponse.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/FlagDataResponse.java @@ -1,9 +1,8 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.http.flags; +package com.yahoo.vespa.configserver.flags.http; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.jdisc.Response; -import com.yahoo.vespa.config.server.http.HttpConfigResponse; import com.yahoo.vespa.flags.json.FlagData; import java.io.OutputStream; @@ -26,6 +25,6 @@ public class FlagDataResponse extends HttpResponse { @Override public String getContentType() { - return HttpConfigResponse.JSON_CONTENT_TYPE; + return "application/json"; } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagsHandler.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/FlagsHandler.java index 00f3d457d3d..40bb69111e0 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagsHandler.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/FlagsHandler.java @@ -1,14 +1,12 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.http.flags; +package com.yahoo.vespa.configserver.flags.http; import com.google.inject.Inject; import com.yahoo.container.jdisc.HttpRequest; 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.config.server.http.HttpErrorResponse; -import com.yahoo.vespa.config.server.http.HttpHandler; -import com.yahoo.vespa.config.server.http.NotFoundException; import com.yahoo.vespa.configserver.flags.FlagsDb; import com.yahoo.vespa.flags.FlagDefinition; import com.yahoo.vespa.flags.FlagId; @@ -25,7 +23,8 @@ import java.util.Objects; * * @author hakonhall */ -public class FlagsHandler extends HttpHandler { +public class FlagsHandler extends LoggingRequestHandler { + private final FlagsDb flagsDb; @Inject @@ -35,28 +34,44 @@ public class FlagsHandler extends HttpHandler { } @Override - protected HttpResponse handleGET(HttpRequest request) { + public HttpResponse handle(HttpRequest request) { + try { + switch (request.getMethod()) { + case GET: return handleGET(request); + case DELETE: return handleDELETE(request); + case PUT: return handlePUT(request); + default: return ErrorResponse.methodNotAllowed("Method '" + request.getMethod() + "' is not supported"); + } + } + catch (IllegalArgumentException e) { + return ErrorResponse.badRequest(Exceptions.toMessageString(e)); + } + catch (RuntimeException e) { + log.log(LogLevel.WARNING, "Unexpected error handling '" + request.getUri() + "'", e); + return ErrorResponse.internalServerError(Exceptions.toMessageString(e)); + } + } + + private HttpResponse handleGET(HttpRequest request) { Path path = new Path(request.getUri()); if (path.matches("/flags/v1")) return new V1Response(flagsV1Uri(request), "data", "defined"); if (path.matches("/flags/v1/data")) return getFlagDataList(request); if (path.matches("/flags/v1/data/{flagId}")) return getFlagData(findFlagId(request, path)); if (path.matches("/flags/v1/defined")) return new DefinedFlags(Flags.getAllFlags()); if (path.matches("/flags/v1/defined/{flagId}")) return getDefinedFlag(findFlagId(request, path)); - throw new NotFoundException("Nothing at path '" + path + "'"); + return ErrorResponse.notFoundError("Nothing at path '" + path + "'"); } - @Override - protected HttpResponse handlePUT(HttpRequest request) { + private HttpResponse handlePUT(HttpRequest request) { Path path = new Path(request.getUri()); if (path.matches("/flags/v1/data/{flagId}")) return putFlagData(request, findFlagId(request, path)); - throw new NotFoundException("Nothing at path '" + path + "'"); + return ErrorResponse.notFoundError("Nothing at path '" + path + "'"); } - @Override - protected HttpResponse handleDELETE(HttpRequest request) { + private HttpResponse handleDELETE(HttpRequest request) { Path path = new Path(request.getUri()); if (path.matches("/flags/v1/data/{flagId}")) return deleteFlagData(findFlagId(request, path)); - throw new NotFoundException("Nothing at path '" + path + "'"); + return ErrorResponse.notFoundError("Nothing at path '" + path + "'"); } private String flagsV1Uri(HttpRequest request) { @@ -66,19 +81,24 @@ public class FlagsHandler extends HttpHandler { } private HttpResponse getDefinedFlag(FlagId flagId) { - FlagDefinition definition = Flags.getFlag(flagId) - .orElseThrow(() -> new NotFoundException("Flag " + flagId + " not defined")); - return new DefinedFlag(definition); + var definedFlag = Flags.getFlag(flagId).map(DefinedFlag::new); + if (definedFlag.isPresent()) { + return definedFlag.get(); + } + return ErrorResponse.notFoundError("Flag " + flagId + " not defined"); } 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) { - FlagData data = flagsDb.getValue(flagId).orElseThrow(() -> new NotFoundException("Flag " + flagId + " not set")); - return new FlagDataResponse(data); + var data = flagsDb.getValue(flagId).map(FlagDataResponse::new); + if (data.isPresent()) { + return data.get(); + } + return ErrorResponse.notFoundError("Flag " + flagId + " not set"); } private HttpResponse putFlagData(HttpRequest request, FlagId flagId) { @@ -86,7 +106,7 @@ public class FlagsHandler extends HttpHandler { try { data = FlagData.deserialize(request.getData()); } catch (UncheckedIOException e) { - return HttpErrorResponse.badRequest("Failed to deserialize request data: " + Exceptions.toMessageString(e)); + return ErrorResponse.badRequest("Failed to deserialize request data: " + Exceptions.toMessageString(e)); } if (!isForce(request)) { @@ -105,16 +125,14 @@ public class FlagsHandler extends HttpHandler { private FlagId findFlagId(HttpRequest request, Path path) { FlagId flagId = new FlagId(path.get("flagId")); - - if (!isForce(request)) { - Flags.getFlag(flagId).orElseThrow(() -> - new NotFoundException("There is no flag '" + flagId + "' (use ?force=true to override)")); + if (!isForce(request) && Flags.getFlag(flagId).isEmpty()) { + throw new IllegalArgumentException("There is no flag '" + flagId + "' (use ?force=true to override)"); } - return flagId; } private boolean isForce(HttpRequest request) { return Objects.equals(request.getProperty("force"), "true"); } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/OKResponse.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/OKResponse.java index 87c02ae56f1..f41940f692b 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/OKResponse.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/OKResponse.java @@ -1,9 +1,8 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.http.flags; +package com.yahoo.vespa.configserver.flags.http; import com.yahoo.container.jdisc.EmptyResponse; import com.yahoo.jdisc.Response; -import com.yahoo.vespa.config.server.http.HttpConfigResponse; /** * @author hakonhall @@ -15,6 +14,6 @@ public class OKResponse extends EmptyResponse { @Override public String getContentType() { - return HttpConfigResponse.JSON_CONTENT_TYPE; + return "application/json"; } } diff --git a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/SlimeJsonResponse.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/SlimeJsonResponse.java new file mode 100644 index 00000000000..e5568514894 --- /dev/null +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/SlimeJsonResponse.java @@ -0,0 +1,38 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.configserver.flags.http; + +import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.slime.JsonFormat; +import com.yahoo.slime.Slime; + +import java.io.IOException; +import java.io.OutputStream; + +/** + * A generic Json response using Slime for JSON encoding + * + * @author bratseth + */ +public class SlimeJsonResponse extends HttpResponse { + + private final Slime slime; + + public SlimeJsonResponse(Slime slime) { + super(200); + this.slime = slime; + } + + public SlimeJsonResponse(int statusCode, Slime slime) { + super(statusCode); + this.slime = slime; + } + + @Override + public void render(OutputStream stream) throws IOException { + new JsonFormat(true).encode(stream, slime); + } + + @Override + public String getContentType() { return "application/json"; } + +} diff --git a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/V1Response.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/V1Response.java new file mode 100644 index 00000000000..ac1e9514700 --- /dev/null +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/V1Response.java @@ -0,0 +1,46 @@ +// 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.http; + +import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.jdisc.Response; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.JsonFormat; +import com.yahoo.slime.Slime; + +import java.io.IOException; +import java.io.OutputStream; +import java.util.List; + +/** + * @author hakonhall + */ +public class V1Response extends HttpResponse { + + private final Slime slime; + + public V1Response(String flagsV1Uri, String... names) { + super(Response.Status.OK); + this.slime = generateBody(flagsV1Uri, List.of(names)); + } + + @Override + public void render(OutputStream stream) throws IOException { + new JsonFormat(true).encode(stream, slime); + } + + @Override + public String getContentType() { + return "application/json"; + } + + private static Slime generateBody(String flagsV1Uri, List<String> names) { + Slime slime = new Slime(); + Cursor root = slime.setObject(); + names.forEach(name -> { + Cursor data = root.setObject(name); + data.setString("url", flagsV1Uri + "/" + name); + }); + return slime; + } + +} diff --git a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/package-info.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/http/package-info.java new file mode 100644 index 00000000000..87b63114b73 --- /dev/null +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/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 mpolden + */ +@ExportPackage +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 d0d1d61628c..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 @@ -103,4 +103,4 @@ public class ConfigServerFlagSourceTest { assertFalse(rawFlag2.isPresent()); verify(flagsDb, times(1)).getValue(flagId2); } -}
\ No newline at end of file +} diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/flags/FlagsHandlerTest.java b/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/http/FlagsHandlerTest.java index 5ae6ce9820b..cbd37c8a5cf 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/flags/FlagsHandlerTest.java +++ b/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/http/FlagsHandlerTest.java @@ -1,25 +1,27 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.http.flags; +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.config.server.http.SessionHandlerTest; +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.yolean.Exceptions; import org.junit.Test; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.util.stream.Collectors; import java.util.stream.Stream; -import static com.yahoo.yolean.Exceptions.uncheck; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; @@ -37,7 +39,7 @@ public class FlagsHandlerTest { private static final String FLAGS_V1_URL = "https://foo.com:4443/flags/v1"; - private final FlagsDbImpl flagsDb = new FlagsDbImpl(new MockCurator()); + private final FlagsDb flagsDb = new FlagsDbImpl(new MockCurator()); private final FlagsHandler handler = new FlagsHandler(FlagsHandler.testOnlyContext(), flagsDb); @Test @@ -161,7 +163,7 @@ public class FlagsHandlerTest { @Test public void testForcing() { - assertThat(handle(Method.PUT, "/data/" + new FlagId("undef"), "", 404), + assertThat(handle(Method.PUT, "/data/" + new FlagId("undef"), "", 400), containsString("There is no flag 'undef'")); assertThat(handle(Method.PUT, "/data/" + new FlagId("undef") + "?force=true", "", 400), @@ -191,10 +193,12 @@ public class FlagsHandlerTest { HttpResponse response = handler.handle(request); assertEquals(expectedStatus, response.getStatus()); assertEquals("application/json", response.getContentType()); - return uncheck(() -> SessionHandlerTest.getRenderedString(response)); + var outputStream = new ByteArrayOutputStream(); + Exceptions.uncheck(() -> response.render(outputStream)); + return outputStream.toString(StandardCharsets.UTF_8); } private InputStream makeInputStream(String content) { return new ByteArrayInputStream(Utf8.toBytes(content)); } -}
\ No newline at end of file +} 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 035a102e784..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 @@ -469,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()); 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/flags/V1Response.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/V1Response.java deleted file mode 100644 index 3594c801ca8..00000000000 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/V1Response.java +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.http.flags; - -import com.yahoo.jdisc.Response; -import com.yahoo.slime.Cursor; -import com.yahoo.slime.Slime; -import com.yahoo.text.Utf8; -import com.yahoo.vespa.config.SlimeUtils; -import com.yahoo.vespa.config.server.http.HttpConfigResponse; -import com.yahoo.vespa.config.server.http.StaticResponse; - -import java.util.Arrays; -import java.util.List; - -import static com.yahoo.yolean.Exceptions.uncheck; - -/** - * @author hakonhall - */ -public class V1Response extends StaticResponse { - public V1Response(String flagsV1Uri, String... names) { - super(Response.Status.OK, HttpConfigResponse.JSON_CONTENT_TYPE, generateBody(flagsV1Uri, Arrays.asList(names))); - } - - private static String generateBody(String flagsV1Uri, List<String> names) { - Slime slime = new Slime(); - Cursor root = slime.setObject(); - names.forEach(name -> { - Cursor data = root.setObject(name); - data.setString("url", flagsV1Uri + "/" + name); - }); - return Utf8.toString(uncheck(() -> SlimeUtils.toJsonBytes(slime))); - } -} 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 41dd19aedc4..97b2156e8ca 100644 --- a/configserver/src/main/resources/configserver-app/services.xml +++ b/configserver/src/main/resources/configserver-app/services.xml @@ -92,10 +92,6 @@ <handler id='com.yahoo.vespa.config.server.http.status.StatusHandler' bundle='configserver'> <binding>http://*/status</binding> </handler> - <handler id='com.yahoo.vespa.config.server.http.flags.FlagsHandler' bundle='configserver'> - <binding>http://*/flags/v1</binding> - <binding>http://*/flags/v1/*</binding> - </handler> <handler id='com.yahoo.vespa.config.server.http.v2.TenantHandler' bundle='configserver'> <binding>http://*/application/v2/tenant/</binding> <binding>http://*/application/v2/tenant/*</binding> diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeHistory.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeHistory.java index 42e2c97e515..a4691001adc 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeHistory.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeHistory.java @@ -32,6 +32,6 @@ public class NodeHistory { return event; } - public enum Agent { system, application, operator, NodeRetirer, NodeFailer } + public enum Agent { system, application, operator, NodeFailer } } 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/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/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 26dcd7949a1..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 @@ -555,27 +555,6 @@ public class ApplicationSerializer { private List<AssignedRotation> assignedRotationsFromSlime(DeploymentSpec deploymentSpec, Inspector root) { final var assignedRotations = new LinkedHashMap<EndpointId, AssignedRotation>(); - // Add the legacy rotation field to the set - this needs to be first - // TODO: Remove when we retire the rotations field - final var legacyRotation = legacyRotationFromSlime(root.field(deprecatedRotationField)); - if (legacyRotation.isPresent() && deploymentSpec.globalServiceId().isPresent()) { - final var clusterId = new ClusterSpec.Id(deploymentSpec.globalServiceId().get()); - final var regions = deploymentSpec.zones().stream().flatMap(zone -> zone.region().stream()).collect(Collectors.toSet()); - assignedRotations.putIfAbsent(EndpointId.default_(), new AssignedRotation(clusterId, EndpointId.default_(), legacyRotation.get(), regions)); - } - - // Now add the same entries from "stupid" list of rotations - // TODO: Remove when we retire the rotations field - final var rotations = rotationListFromSlime(root.field(rotationsField)); - for (var rotation : rotations) { - final var regions = deploymentSpec.zones().stream().flatMap(zone -> zone.region().stream()).collect(Collectors.toSet()); - if (deploymentSpec.globalServiceId().isPresent()) { - final var clusterId = new ClusterSpec.Id(deploymentSpec.globalServiceId().get()); - assignedRotations.putIfAbsent(EndpointId.default_(), new AssignedRotation(clusterId, EndpointId.default_(), rotation, regions)); - } - } - - // Last - add the actual entries we want. Do _not_ remove this during clean-up root.field(assignedRotationsField).traverse((ArrayTraverser) (idx, inspector) -> { final var clusterId = new ClusterSpec.Id(inspector.field(assignedRotationClusterField).asString()); final var endpointId = EndpointId.of(inspector.field(assignedRotationEndpointField).asString()); @@ -590,22 +569,6 @@ public class ApplicationSerializer { return List.copyOf(assignedRotations.values()); } - private List<RotationId> rotationListFromSlime(Inspector field) { - final var rotations = new ArrayList<RotationId>(); - - field.traverse((ArrayTraverser) (idx, inspector) -> { - final var rotation = new RotationId(inspector.asString()); - rotations.add(rotation); - }); - - return rotations; - } - - // TODO: Remove after June 2019 once the 'rotation' field is gone from storage - private Optional<RotationId> legacyRotationFromSlime(Inspector field) { - return field.valid() ? optionalString(field).map(RotationId::new) : Optional.empty(); - } - private OptionalLong optionalLong(Inspector field) { return field.valid() ? OptionalLong.of(field.asLong()) : OptionalLong.empty(); } 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 new file mode 100644 index 00000000000..31058a71816 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/flags/AuditedFlagsHandler.java @@ -0,0 +1,30 @@ +// 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.restapi.flags; + +import com.yahoo.container.jdisc.HttpRequest; +import com.yahoo.container.jdisc.HttpResponse; +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; + +/** + * An extension of {@link FlagsHandler} which logs requests to the audit log. + * + * @author mpolden + */ +public class AuditedFlagsHandler extends FlagsHandler { + + private final AuditLogger auditLogger; + + public AuditedFlagsHandler(Context context, Controller controller, FlagsDb flagsDb) { + super(context, flagsDb); + auditLogger = controller.auditLogger(); + } + + @Override + public HttpResponse handle(HttpRequest request) { + return super.handle(auditLogger.log(request)); + } + +} 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/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/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java index 7b39b0d53a4..aca4f750649 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java @@ -253,9 +253,8 @@ public class ApplicationSerializerTest { // ok if no error } - /** TODO: Test can be removed after June 2019 - once legacy field for single rotation is retired */ @Test - public void testParsingLegacyRotationElement() throws IOException { + public void testParsingAssignedRotations() throws IOException { // Use the 'complete-application.json' as a baseline final var applicationJson = Files.readAllBytes(testData.resolve("complete-application.json")); final var slime = SlimeUtils.jsonToSlime(applicationJson); @@ -267,12 +266,6 @@ public class ApplicationSerializerTest { // Add the necessary fields to the Slime representation of the application final var cursor = slime.get(); - cursor.setString("rotation", "single-rotation"); - - final var rotations = cursor.setArray("endpoints"); - rotations.addString("multiple-rotation-1"); - rotations.addString("multiple-rotation-2"); - final var assignedRotations = cursor.setArray("assignedRotations"); final var assignedRotation = assignedRotations.addObject(); assignedRotation.setString("clusterId", "foobar"); @@ -282,51 +275,23 @@ public class ApplicationSerializerTest { // Parse and test the output from parsing contains both legacy rotation and multiple rotations final var application = applicationSerializer.fromSlime(slime); - // Since only one AssignedEndpoint can be "default", we make sure that we are ignoring the - // multiple-rotation entries as the globalServiceId will override them assertEquals( List.of( - new RotationId("single-rotation"), new RotationId("assigned-rotation") ), application.rotations() ); assertEquals( - Optional.of(new RotationId("single-rotation")), application.legacyRotation() + Optional.of(new RotationId("assigned-rotation")), application.legacyRotation() ); - // The same goes here for AssignedRotations with "default" EndpointId as in the .rotations() test above. - // Note that we are only using Set.of() on "assigned-rotation" because in this test we do not have access - // to a deployment.xml that describes the zones a rotation should map to. assertEquals( List.of( - new AssignedRotation(new ClusterSpec.Id("foo"), EndpointId.of("default"), new RotationId("single-rotation"), regions), new AssignedRotation(new ClusterSpec.Id("foobar"), EndpointId.of("nice-endpoint"), new RotationId("assigned-rotation"), Set.of()) ), application.assignedRotations() ); } - @Test - public void testParsingOnlyLegacyRotationElement() throws IOException { - // Use the 'complete-application.json' as a baseline - final var applicationJson = Files.readAllBytes(testData.resolve("complete-application.json")); - final var slime = SlimeUtils.jsonToSlime(applicationJson); - - // Add the necessary fields to the Slime representation of the application - final var cursor = slime.get(); - - cursor.setString("rotation", "single-rotation"); - - // Parse and test the output from parsing contains both legacy rotation and multiple rotations - final var application = applicationSerializer.fromSlime(slime); - - assertEquals( - List.of( - new RotationId("single-rotation") - ), - application.rotations() - ); - } } 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/ContainerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java index c7be543dd00..b32cbbcb926 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java @@ -7,7 +7,6 @@ import com.yahoo.application.container.handler.Response; import com.yahoo.component.ComponentSpecification; import com.yahoo.component.Version; import com.yahoo.config.provision.zone.ZoneApi; -import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.container.http.filter.FilterChainRepository; import com.yahoo.jdisc.http.filter.SecurityRequestFilter; import com.yahoo.jdisc.http.filter.SecurityRequestFilterChain; 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 11aa132b478..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,6 +60,8 @@ public class ControllerContainerTest { " </rotations>\n" + " </config>\n" + " <component id='com.yahoo.vespa.flags.InMemoryFlagSource'/>\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" + " <component id='com.yahoo.vespa.hosted.controller.api.integration.dns.MemoryNameService'/>\n" + @@ -112,6 +114,10 @@ public class ControllerContainerTest { " <binding>http://*/zone/v2</binding>\n" + " <binding>http://*/zone/v2/*</binding>\n" + " </handler>\n" + + " <handler id='com.yahoo.vespa.hosted.controller.restapi.flags.AuditedFlagsHandler'>\n" + + " <binding>http://*/flags/v1</binding>\n" + + " <binding>http://*/flags/v1/*</binding>\n" + + " </handler>\n" + variablePartXml() + "</container>"; } 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/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/flags/AuditedFlagsApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/flags/AuditedFlagsApiTest.java new file mode 100644 index 00000000000..b4ef98cc7f6 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/flags/AuditedFlagsApiTest.java @@ -0,0 +1,57 @@ +// 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.restapi.flags; + +import com.yahoo.application.container.handler.Request; +import com.yahoo.vespa.athenz.api.AthenzIdentity; +import com.yahoo.vespa.athenz.api.AthenzUser; +import com.yahoo.vespa.hosted.controller.auditlog.AuditLog; +import com.yahoo.vespa.hosted.controller.restapi.ContainerControllerTester; +import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +/** + * @author mpolden + */ +public class AuditedFlagsApiTest extends ControllerContainerTest { + + private static final String responses = "src/test/java/com/yahoo/vespa/hosted/controller/restapi/flags/responses/"; + private static final AthenzIdentity operator = AthenzUser.fromUserId("operatorUser"); + + private ContainerControllerTester tester; + + @Before + public void before() { + addUserToHostedOperatorRole(operator); + tester = new ContainerControllerTester(container, responses); + } + + @Test + public void test_audit_logging() { + var body = "{\n" + + " \"id\": \"id1\",\n" + + " \"rules\": [\n" + + " {\n" + + " \"value\": true\n" + + " }\n" + + " ]\n" + + "}"; + assertResponse(new Request("http://localhost:8080/flags/v1/data/id1?force=true", body, Request.Method.PUT), + "", 200); + var log = tester.controller().auditLogger().readLog(); + assertEquals(1, log.entries().size()); + var entry = log.entries().get(0); + assertEquals(operator.getFullName(), entry.principal()); + assertEquals(AuditLog.Entry.Method.PUT, entry.method()); + assertEquals("/flags/v1/data/id1?force=true", entry.resource()); + assertEquals(body, log.entries().get(0).data().get()); + } + + private void assertResponse(Request request, String body, int statusCode) { + addIdentityToRequest(request, operator); + tester.assertResponse(request, body, statusCode); + } + +} diff --git a/defaults/src/main/java/com/yahoo/vespa/defaults/Defaults.java b/defaults/src/main/java/com/yahoo/vespa/defaults/Defaults.java index f1b7e38986f..6fb6e4f0860 100644 --- a/defaults/src/main/java/com/yahoo/vespa/defaults/Defaults.java +++ b/defaults/src/main/java/com/yahoo/vespa/defaults/Defaults.java @@ -114,11 +114,11 @@ public class Defaults { public String vespaHostname() { return vespaHost; } /** - * Returns the path where a Vespa application can store arbitrary files. This should only be used for temporary - * files as there are no availability guarantees for files stored here. The application must be able to recreate + * Returns the path where a Vespa application can store arbitrary files on the node. This path + * is persistent during the lifetime of this node. The application must be able to recreate * required files on its own (e.g. by downloading them from a remote source) if missing. * - * @return the temporary storage path + * @return the local application storage path */ public String temporaryApplicationStorage() { return temporaryApplicationStorage; } 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/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 0981d7b84e2..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"); @@ -151,13 +151,6 @@ public class Flags { "Takes effect on deployment through controller", APPLICATION_ID); - public static final UnboundBooleanFlag DISABLE_CHEF = defineFeatureFlag( - "disable-chef", false, - "Stops and disables chef-client", - "Takes effect on next host-admin tick", - HOSTNAME, NODE_TYPE); - - /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, String description, String modificationEffect, FetchVector.Dimension... dimensions) { 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/node/Agent.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Agent.java index 812c370df5f..f46e2f501bc 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Agent.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Agent.java @@ -7,5 +7,5 @@ package com.yahoo.vespa.hosted.provision.node; * @author bratseth */ public enum Agent { - system, application, operator, NodeRetirer, NodeFailer + system, application, operator, NodeFailer } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java index cf6531c0748..6198183be89 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java @@ -385,7 +385,6 @@ public class NodeSerializer { case "application" : return Agent.application; case "system" : return Agent.system; case "operator" : return Agent.operator; - case "NodeRetirer" : return Agent.system; // TODO: Remove after 7.67 case "NodeFailer" : return Agent.NodeFailer; } throw new IllegalArgumentException("Unknown node event agent '" + eventAgentField.asString() + "'"); @@ -395,7 +394,6 @@ public class NodeSerializer { case application : return "application"; case system : return "system"; case operator : return "operator"; - case NodeRetirer : return "system"; // TODO: Remove after 7.67 case NodeFailer : return "NodeFailer"; } throw new IllegalArgumentException("Serialized form of '" + agent + "' not defined"); 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 |