diff options
62 files changed, 1371 insertions, 531 deletions
diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzSslKeyStoreConfigurator.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzSslKeyStoreConfigurator.java index beff50b52c6..da16bfe3c24 100644 --- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzSslKeyStoreConfigurator.java +++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/AthenzSslKeyStoreConfigurator.java @@ -2,28 +2,38 @@ package com.yahoo.vespa.hosted.athenz.instanceproviderservice; import com.google.inject.Inject; +import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.component.AbstractComponent; import com.yahoo.config.provision.Zone; import com.yahoo.jdisc.http.ssl.SslKeyStoreConfigurator; import com.yahoo.jdisc.http.ssl.SslKeyStoreContext; import com.yahoo.log.LogLevel; +import com.yahoo.vespa.defaults.Defaults; import com.yahoo.vespa.hosted.athenz.instanceproviderservice.config.AthenzProviderServiceConfig; import com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.AthenzCertificateClient; +import java.io.BufferedInputStream; +import java.io.BufferedOutputStream; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.security.GeneralSecurityException; import java.security.KeyStore; import java.security.KeyStoreException; -import java.security.NoSuchAlgorithmException; import java.security.PrivateKey; import java.security.cert.Certificate; -import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.time.Duration; import java.time.Instant; +import java.util.Optional; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Logger; import static com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.Utils.getZoneConfig; @@ -31,44 +41,77 @@ import static com.yahoo.vespa.hosted.athenz.instanceproviderservice.impl.Utils.g /** * @author bjorncs */ -// TODO Cache certificate on disk @SuppressWarnings("unused") // Component injected into Jetty connector factory public class AthenzSslKeyStoreConfigurator extends AbstractComponent implements SslKeyStoreConfigurator { private static final Logger log = Logger.getLogger(AthenzSslKeyStoreConfigurator.class.getName()); - // TODO Make expiry and update frequency configurable parameters - private static final Duration CERTIFICATE_EXPIRY_TIME = Duration.ofDays(30); - private static final Duration CERTIFICATE_UPDATE_PERIOD = Duration.ofDays(7); private static final String CERTIFICATE_ALIAS = "athenz"; private static final String CERTIFICATE_PASSWORD = "athenz"; + private static final Duration EXPIRATION_MARGIN = Duration.ofHours(6); private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); private final AthenzCertificateClient certificateClient; private final KeyProvider keyProvider; private final AthenzProviderServiceConfig.Zones zoneConfig; - private final AtomicBoolean alreadyConfigured = new AtomicBoolean(); + private final Duration updatePeriod; + private final Path keystoreCachePath; private volatile KeyStore currentKeyStore; @Inject public AthenzSslKeyStoreConfigurator(KeyProvider keyProvider, AthenzProviderServiceConfig config, - Zone zone) { + Zone zone, + ConfigserverConfig configserverConfig) { AthenzProviderServiceConfig.Zones zoneConfig = getZoneConfig(config, zone); - this.certificateClient = new AthenzCertificateClient(config, zoneConfig); + Path keystoreCachePath = createKeystoreCachePath(configserverConfig); + AthenzCertificateClient certificateClient = new AthenzCertificateClient(config, zoneConfig); + Duration updatePeriod = Duration.ofDays(config.updatePeriodDays()); + this.certificateClient = certificateClient; this.keyProvider = keyProvider; this.zoneConfig = zoneConfig; - this.currentKeyStore = downloadCertificate(keyProvider, certificateClient, zoneConfig); + this.currentKeyStore = initializeKeystore(keyProvider, certificateClient, zoneConfig, keystoreCachePath, updatePeriod); + this.updatePeriod = updatePeriod; + this.keystoreCachePath = keystoreCachePath; + } + + private static KeyStore initializeKeystore(KeyProvider keyProvider, + AthenzCertificateClient certificateClient, + AthenzProviderServiceConfig.Zones zoneConfig, + Path keystoreCachePath, + Duration updatePeriod) { + return tryReadKeystoreFile(keystoreCachePath.toFile(), updatePeriod) + .orElseGet(() -> downloadCertificate(keyProvider, certificateClient, zoneConfig, keystoreCachePath)); + } + + private static Optional<KeyStore> tryReadKeystoreFile(File certificateFile, Duration updatePeriod) { + try { + if (!certificateFile.exists()) return Optional.empty(); + KeyStore keyStore = KeyStore.getInstance("JKS"); + try (InputStream in = new BufferedInputStream(new FileInputStream(certificateFile))) { + keyStore.load(in, new char[0]); + } + Instant minimumExpiration = Instant.now().plus(updatePeriod).plus(EXPIRATION_MARGIN); + boolean isExpired = getCertificateExpiry(keyStore).isBefore(minimumExpiration); + if (isExpired) return Optional.empty(); + return Optional.of(keyStore); + } catch (IOException | GeneralSecurityException e) { + log.log(LogLevel.ERROR, "Failed to read keystore from disk: " + e.getMessage(), e); + return Optional.empty(); + } + } + + private static Path createKeystoreCachePath(ConfigserverConfig configserverConfig) { + return Paths.get( + Defaults.getDefaults().underVespaHome(configserverConfig.configServerDBDir()), + "server-x509-athenz-cert.jks"); } @Override public void configure(SslKeyStoreContext sslKeyStoreContext) { - if (alreadyConfigured.getAndSet(true)) { // For debugging purpose of SslKeyStoreConfigurator interface - throw new IllegalStateException("Already configured. configure() can only be called once."); - } sslKeyStoreContext.updateKeyStore(currentKeyStore, CERTIFICATE_PASSWORD); scheduler.scheduleAtFixedRate(new AthenzCertificateUpdater(sslKeyStoreContext), - CERTIFICATE_UPDATE_PERIOD.toMinutes()/*initial delay*/, - CERTIFICATE_UPDATE_PERIOD.toMinutes(), - TimeUnit.MINUTES); + updatePeriod.toDays()/*initial delay*/, + updatePeriod.toDays(), + TimeUnit.DAYS); } @Override @@ -81,36 +124,42 @@ public class AthenzSslKeyStoreConfigurator extends AbstractComponent implements } } - Instant getKeyStoreExpiry() throws KeyStoreException { - X509Certificate certificate = (X509Certificate) currentKeyStore.getCertificate(CERTIFICATE_ALIAS); - return certificate.getNotAfter().toInstant(); + Instant getCertificateExpiry() throws KeyStoreException { + return getCertificateExpiry(currentKeyStore); } + private static Instant getCertificateExpiry(KeyStore keyStore) throws KeyStoreException { + X509Certificate certificate = (X509Certificate) keyStore.getCertificate(CERTIFICATE_ALIAS); + return certificate.getNotAfter().toInstant(); + } private static KeyStore downloadCertificate(KeyProvider keyProvider, AthenzCertificateClient certificateClient, - AthenzProviderServiceConfig.Zones zoneConfig) { + AthenzProviderServiceConfig.Zones zoneConfig, + Path keystoreCachePath) { try { PrivateKey privateKey = keyProvider.getPrivateKey(zoneConfig.secretVersion()); - X509Certificate certificate = certificateClient.updateCertificate(privateKey, CERTIFICATE_EXPIRY_TIME); - verifyActualExpiry(certificate); + X509Certificate certificate = certificateClient.updateCertificate(privateKey); + Instant expirationTime = certificate.getNotAfter().toInstant(); + Duration expiry = Duration.between(certificate.getNotBefore().toInstant(), expirationTime); + log.log(LogLevel.INFO, String.format("Got Athenz x509 certificate with expiry %s (expires %s)", expiry, expirationTime)); KeyStore keyStore = KeyStore.getInstance("JKS"); keyStore.load(null); keyStore.setKeyEntry( CERTIFICATE_ALIAS, privateKey, CERTIFICATE_PASSWORD.toCharArray(), new Certificate[]{certificate}); + tryWriteKeystore(keyStore, keystoreCachePath); return keyStore; - } catch (IOException | NoSuchAlgorithmException | CertificateException | KeyStoreException e) { + } catch (IOException | GeneralSecurityException e) { throw new RuntimeException(e); } } - private static void verifyActualExpiry(X509Certificate certificate) { - Duration actualExpiry = - Duration.between(certificate.getNotBefore().toInstant(), certificate.getNotAfter().toInstant()); - if (CERTIFICATE_EXPIRY_TIME.compareTo(actualExpiry) > 0) { - log.log(LogLevel.WARNING, - String.format("Expected expiry %s, got %s", CERTIFICATE_EXPIRY_TIME, actualExpiry)); + private static void tryWriteKeystore(KeyStore keyStore, Path keystoreCachePath) { + try (OutputStream out = new BufferedOutputStream(new FileOutputStream(keystoreCachePath.toFile()))) { + keyStore.store(out, new char[0]); + } catch (IOException | GeneralSecurityException e) { + log.log(LogLevel.ERROR, "Failed to write keystore to disk: " + e.getMessage(), e); } } @@ -126,7 +175,7 @@ public class AthenzSslKeyStoreConfigurator extends AbstractComponent implements public void run() { try { log.log(LogLevel.INFO, "Updating Athenz certificate from ZTS"); - currentKeyStore = downloadCertificate(keyProvider, certificateClient, zoneConfig); + currentKeyStore = downloadCertificate(keyProvider, certificateClient, zoneConfig, keystoreCachePath); sslKeyStoreContext.updateKeyStore(currentKeyStore, CERTIFICATE_PASSWORD); log.log(LogLevel.INFO, "Athenz certificate reload successfully completed"); } catch (Throwable e) { diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/CertificateExpiryMetricUpdater.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/CertificateExpiryMetricUpdater.java index cf734facf34..2d80b15c7ec 100644 --- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/CertificateExpiryMetricUpdater.java +++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/CertificateExpiryMetricUpdater.java @@ -59,7 +59,7 @@ public class CertificateExpiryMetricUpdater extends AbstractComponent { Instant now = Instant.now(); try { - Duration keyStoreExpiry = Duration.between(now, keyStoreConfigurator.getKeyStoreExpiry()); + Duration keyStoreExpiry = Duration.between(now, keyStoreConfigurator.getCertificateExpiry()); metric.set(ATHENZ_CONFIGSERVER_CERT_METRIC_NAME, keyStoreExpiry.getSeconds(), null); } catch (KeyStoreException e) { logger.log(Level.WARNING, "Failed to update key store expiry metric", e); diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/impl/AthenzCertificateClient.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/impl/AthenzCertificateClient.java index c6aee673f9c..c849de481dc 100644 --- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/impl/AthenzCertificateClient.java +++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/impl/AthenzCertificateClient.java @@ -29,7 +29,7 @@ public class AthenzCertificateClient { this.zoneConfig = zoneConfig; } - public X509Certificate updateCertificate(PrivateKey privateKey, TemporalAmount expiryTime) { + public X509Certificate updateCertificate(PrivateKey privateKey) { SimpleServiceIdentityProvider identityProvider = new SimpleServiceIdentityProvider( authority, zoneConfig.domain(), zoneConfig.serviceName(), privateKey, Integer.toString(zoneConfig.secretVersion()), TimeUnit.MINUTES.toSeconds(10)); @@ -38,7 +38,7 @@ public class AthenzCertificateClient { InstanceRefreshRequest req = ZTSClient.generateInstanceRefreshRequest( zoneConfig.domain(), zoneConfig.serviceName(), privateKey, - config.certDnsSuffix(), (int)expiryTime.get(ChronoUnit.SECONDS)); + config.certDnsSuffix(), /*expiryTime*/0); String pemEncoded = ztsClient.postInstanceRefreshRequest(zoneConfig.domain(), zoneConfig.serviceName(), req) .getCertificate(); return Crypto.loadX509Certificate(pemEncoded); diff --git a/athenz-identity-provider-service/src/main/resources/configdefinitions/athenz-provider-service.def b/athenz-identity-provider-service/src/main/resources/configdefinitions/athenz-provider-service.def index 21f2aea6ab0..d3f758a2240 100644 --- a/athenz-identity-provider-service/src/main/resources/configdefinitions/athenz-provider-service.def +++ b/athenz-identity-provider-service/src/main/resources/configdefinitions/athenz-provider-service.def @@ -24,3 +24,6 @@ certDnsSuffix string # Path to Athenz CA JKS trust store athenzCaTrustStore string + +# Period between certificate updates +updatePeriodDays int default=1 diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregator.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregator.java index 3b8cb14e977..ed2ca32dab0 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregator.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregator.java @@ -27,6 +27,7 @@ import java.util.*; public class ClusterStatsAggregator { private final Set<Integer> distributors; + private final Set<Integer> storageNodes; private final Set<Integer> nonUpdatedDistributors; // Maps the distributor node index to a map of content node index to the @@ -40,14 +41,26 @@ public class ClusterStatsAggregator { ClusterStatsAggregator(Set<Integer> distributors, Set<Integer> storageNodes) { this.distributors = distributors; + this.storageNodes = storageNodes; nonUpdatedDistributors = new HashSet<>(distributors); aggregatedStats = new ContentClusterStats(storageNodes); } - ContentClusterStats getAggregatedStats() { + public ContentClusterStats getAggregatedStats() { return aggregatedStats; } + public ContentNodeStats getAggregatedStatsForDistributor(int distributorIndex) { + ContentNodeStats result = new ContentNodeStats(distributorIndex); + ContentClusterStats distributorStats = distributorToStats.get(distributorIndex); + if (distributorStats != null) { + for (Iterator<ContentNodeStats> itr = distributorStats.iterator(); itr.hasNext(); ) { + result.add(itr.next()); + } + } + return result; + } + boolean hasUpdatesFromAllDistributors() { return nonUpdatedDistributors.isEmpty(); } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java index 74ddd941afb..27612bb8b7d 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java @@ -46,6 +46,7 @@ public class ContentCluster { final StringBuilder sb, final Timer timer, final ClusterState state, + final ClusterStatsAggregator statsAggregator, final Distribution distribution, final FleetControllerOptions options, final EventLog eventLog) { @@ -71,6 +72,7 @@ public class ContentCluster { distributorNodeInfoByIndex, timer, state, + statsAggregator, options.maxPrematureCrashes, eventLog, clusterName, diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStats.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStats.java index 2698b079073..bc476923b23 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStats.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStats.java @@ -29,11 +29,11 @@ public class ContentClusterStats implements Iterable<ContentNodeStats> { return mapToNodeStats.values().iterator(); } - ContentNodeStats getContentNode(Integer index) { + public ContentNodeStats getContentNode(Integer index) { return mapToNodeStats.get(index); } - int size() { + public int size() { return mapToNodeStats.size(); } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/StateVersionTracker.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/StateVersionTracker.java index 2063bdb24c9..1a57975c4bc 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/StateVersionTracker.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/StateVersionTracker.java @@ -78,6 +78,10 @@ public class StateVersionTracker { return currentClusterState.getBaselineClusterState(); } + public ClusterStatsAggregator getAggregatedClusterStats() { + return clusterStateView.getStatsAggregator(); + } + public ClusterStateBundle getVersionedClusterStateBundle() { return currentClusterState; } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/LegacyIndexPageRequestHandler.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/LegacyIndexPageRequestHandler.java index 2ced8648ced..c7f83743708 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/LegacyIndexPageRequestHandler.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/LegacyIndexPageRequestHandler.java @@ -65,6 +65,7 @@ public class LegacyIndexPageRequestHandler implements StatusPageServer.RequestHa content, timer, stateVersionTracker.getVersionedClusterState(), + stateVersionTracker.getAggregatedClusterStats(), data.getOptions().storageDistribution, data.getOptions(), eventLog diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/statuspage/VdsClusterHtmlRendrer.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/statuspage/VdsClusterHtmlRendrer.java index 2ed98855121..eb377718614 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/statuspage/VdsClusterHtmlRendrer.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/statuspage/VdsClusterHtmlRendrer.java @@ -1,14 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.core.status.statuspage; +import com.yahoo.document.FixedBucketSpaces; import com.yahoo.vdslib.state.ClusterState; import com.yahoo.vdslib.state.NodeState; import com.yahoo.vdslib.state.NodeType; import com.yahoo.vdslib.state.State; -import com.yahoo.vespa.clustercontroller.core.EventLog; -import com.yahoo.vespa.clustercontroller.core.NodeInfo; -import com.yahoo.vespa.clustercontroller.core.RealTimer; -import com.yahoo.vespa.clustercontroller.core.Timer; +import com.yahoo.vespa.clustercontroller.core.*; import java.util.ArrayList; import java.util.HashMap; @@ -30,6 +28,9 @@ public class VdsClusterHtmlRendrer { private final HtmlTable.CellProperties headerProperties; private final StringBuilder contentBuilder = new StringBuilder(); private final static String TAG_NOT_SET = "not set"; + private final static HtmlTable.CellProperties WARNING_PROPERTY = new HtmlTable.CellProperties().setBackgroundColor(0xffffc0); + private final static HtmlTable.CellProperties ERROR_PROPERTY = new HtmlTable.CellProperties().setBackgroundColor(0xffc0c0); + private final static HtmlTable.CellProperties CENTERED_PROPERTY = new HtmlTable.CellProperties().align(HtmlTable.Orientation.CENTER); Table(final String clusterName, final int slobrokGenerationCount) { table.getTableProperties().align(HtmlTable.Orientation.RIGHT).setBackgroundColor(0xc0ffc0); @@ -38,8 +39,8 @@ public class VdsClusterHtmlRendrer { table.getColProperties(2).align(HtmlTable.Orientation.LEFT); table.getColProperties(3).align(HtmlTable.Orientation.LEFT); table.getColProperties(7).align(HtmlTable.Orientation.LEFT); - table.getColProperties(12).align(HtmlTable.Orientation.LEFT); - for (int i = 4; i < 13; ++i) table.getColProperties(i).allowLineBreaks(false); + table.getColProperties(14).align(HtmlTable.Orientation.LEFT); + for (int i = 4; i < 15; ++i) table.getColProperties(i).allowLineBreaks(false); headerProperties = new HtmlTable.CellProperties() .setBackgroundColor(0xffffff) .align(HtmlTable.Orientation.CENTER); @@ -64,6 +65,7 @@ public class VdsClusterHtmlRendrer { final TreeMap<Integer, NodeInfo> distributorNodeInfos, final Timer timer, final ClusterState state, + final ClusterStatsAggregator statsAggregator, final int maxPrematureCrashes, final EventLog eventLog, final String pathPrefix, @@ -75,6 +77,7 @@ public class VdsClusterHtmlRendrer { NodeType.STORAGE, timer, state, + statsAggregator, maxPrematureCrashes, eventLog, pathPrefix, @@ -84,6 +87,7 @@ public class VdsClusterHtmlRendrer { NodeType.DISTRIBUTOR, timer, state, + statsAggregator, maxPrematureCrashes, eventLog, pathPrefix, @@ -134,12 +138,16 @@ public class VdsClusterHtmlRendrer { .addCell(new HtmlTable.Cell("SSV<sup>4)</sup>")) .addCell(new HtmlTable.Cell("PC<sup>5)</sup>")) .addCell(new HtmlTable.Cell("ELW<sup>6)</sup>")) + .addCell(new HtmlTable.Cell("Buckets pending") + .addProperties(new HtmlTable.CellProperties().setColSpan(2).setRowSpan(1))) .addCell(new HtmlTable.Cell("Start Time")) .addCell(new HtmlTable.Cell("RPC Address"))); table.addRow(new HtmlTable.Row().setHeaderRow().addProperties(headerProperties) .addCell(new HtmlTable.Cell("Reported")) .addCell(new HtmlTable.Cell("Wanted")) - .addCell(new HtmlTable.Cell("System"))); + .addCell(new HtmlTable.Cell("System")) + .addCell(new HtmlTable.Cell(FixedBucketSpaces.defaultSpace())) + .addCell(new HtmlTable.Cell(FixedBucketSpaces.globalSpace()))); } private void renderNodesOneType( @@ -147,6 +155,7 @@ public class VdsClusterHtmlRendrer { final NodeType nodeType, final Timer timer, final ClusterState state, + final ClusterStatsAggregator statsAggregator, final int maxPrematureCrashes, final EventLog eventLog, final String pathPrefix, @@ -156,9 +165,6 @@ public class VdsClusterHtmlRendrer { addTableHeader(name, nodeType); for (final NodeInfo nodeInfo : nodeInfos.values()) { HtmlTable.Row row = new HtmlTable.Row(); - HtmlTable.CellProperties warning = new HtmlTable.CellProperties().setBackgroundColor(0xffffc0); - HtmlTable.CellProperties error = new HtmlTable.CellProperties().setBackgroundColor(0xffc0c0); - HtmlTable.CellProperties centered = new HtmlTable.CellProperties().align(HtmlTable.Orientation.CENTER); // Add node index row.addCell(new HtmlTable.Cell("<a href=\"" + pathPrefix + "/node=" + nodeInfo.getNode() @@ -168,18 +174,18 @@ public class VdsClusterHtmlRendrer { NodeState reportedState = nodeInfo.getReportedState().clone().setStartTimestamp(0); row.addCell(new HtmlTable.Cell(HtmlTable.escape(reportedState.toString(true)))); if (!nodeInfo.getReportedState().getState().equals(State.UP)) { - row.getLastCell().addProperties(warning); + row.getLastCell().addProperties(WARNING_PROPERTY); } // Add wanted state if (nodeInfo.getWantedState() == null || nodeInfo.getWantedState().getState().equals(State.UP)) { - row.addCell(new HtmlTable.Cell("-").addProperties(centered)); + row.addCell(new HtmlTable.Cell("-").addProperties(CENTERED_PROPERTY)); } else { row.addCell(new HtmlTable.Cell(HtmlTable.escape(nodeInfo.getWantedState().toString(true)))); if (nodeInfo.getWantedState().toString(true).indexOf("Disabled by fleet controller") != -1) { - row.getLastCell().addProperties(error); + row.getLastCell().addProperties(ERROR_PROPERTY); } else { - row.getLastCell().addProperties(warning); + row.getLastCell().addProperties(WARNING_PROPERTY); } } @@ -188,13 +194,13 @@ public class VdsClusterHtmlRendrer { if (state.getClusterState().oneOf("uir")) { row.addCell(new HtmlTable.Cell(HtmlTable.escape(ns.toString(true)))); if (ns.getState().equals(State.DOWN)) { - row.getLastCell().addProperties(error); + row.getLastCell().addProperties(ERROR_PROPERTY); } else if (ns.getState().oneOf("mi")) { - row.getLastCell().addProperties(warning); + row.getLastCell().addProperties(WARNING_PROPERTY); } } else { row.addCell(new HtmlTable.Cell("Cluster " + - state.getClusterState().name().toLowerCase()).addProperties(error)); + state.getClusterState().name().toLowerCase()).addProperties(ERROR_PROPERTY)); } // Add build tag version. @@ -204,7 +210,7 @@ public class VdsClusterHtmlRendrer { : TAG_NOT_SET; row.addCell(new HtmlTable.Cell(buildTagText)); if (! dominantVtag.equals(nodeInfo.getVtag())) { - row.getLastCell().addProperties(warning); + row.getLastCell().addProperties(WARNING_PROPERTY); } // Add failed connection attempt count @@ -212,22 +218,22 @@ public class VdsClusterHtmlRendrer { long timeSinceContact = nodeInfo.getTimeOfFirstFailingConnectionAttempt() == 0 ? 0 : currentTime - nodeInfo.getTimeOfFirstFailingConnectionAttempt(); if (timeSinceContact > 60 * 1000) { - row.getLastCell().addProperties(error); + row.getLastCell().addProperties(ERROR_PROPERTY); } else if (nodeInfo.getConnectionAttemptCount() > 0) { - row.getLastCell().addProperties(warning); + row.getLastCell().addProperties(WARNING_PROPERTY); } // Add time since first failing row.addCell(new HtmlTable.Cell((timeSinceContact / 1000) + " s")); if (timeSinceContact > 60 * 1000) { - row.getLastCell().addProperties(error); + row.getLastCell().addProperties(ERROR_PROPERTY); } else if (nodeInfo.getConnectionAttemptCount() > 0) { - row.getLastCell().addProperties(warning); + row.getLastCell().addProperties(WARNING_PROPERTY); } // State pending time if (nodeInfo.getLatestNodeStateRequestTime() == null) { - row.addCell(new HtmlTable.Cell("-").addProperties(centered)); + row.addCell(new HtmlTable.Cell("-").addProperties(CENTERED_PROPERTY)); } else { row.addCell(new HtmlTable.Cell(HtmlTable.escape(RealTimer.printDuration( currentTime - nodeInfo.getLatestNodeStateRequestTime())))); @@ -236,17 +242,17 @@ public class VdsClusterHtmlRendrer { // System state version row.addCell(new HtmlTable.Cell("" + nodeInfo.getSystemStateVersionAcknowledged())); if (nodeInfo.getSystemStateVersionAcknowledged() < state.getVersion() - 2) { - row.getLastCell().addProperties(error); + row.getLastCell().addProperties(ERROR_PROPERTY); } else if (nodeInfo.getSystemStateVersionAcknowledged() < state.getVersion()) { - row.getLastCell().addProperties(warning); + row.getLastCell().addProperties(WARNING_PROPERTY); } // Premature crashes row.addCell(new HtmlTable.Cell("" + nodeInfo.getPrematureCrashCount())); if (nodeInfo.getPrematureCrashCount() >= maxPrematureCrashes) { - row.getLastCell().addProperties(error); + row.getLastCell().addProperties(ERROR_PROPERTY); } else if (nodeInfo.getPrematureCrashCount() > 0) { - row.getLastCell().addProperties(warning); + row.getLastCell().addProperties(WARNING_PROPERTY); } // Events last week @@ -254,14 +260,23 @@ public class VdsClusterHtmlRendrer { currentTime - eventLog.getRecentTimePeriod()); row.addCell(new HtmlTable.Cell("" + nodeEvents)); if (nodeEvents > 20) { - row.getLastCell().addProperties(error); + row.getLastCell().addProperties(ERROR_PROPERTY); } else if (nodeEvents > 3) { - row.getLastCell().addProperties(warning); + row.getLastCell().addProperties(WARNING_PROPERTY); + } + + // Buckets pending ('default' and 'global' spaces) + if (nodeType.equals(NodeType.STORAGE)) { + addBucketsPending(row, getStatsForContentNode(statsAggregator, nodeInfo, FixedBucketSpaces.defaultSpace())); + addBucketsPending(row, getStatsForContentNode(statsAggregator, nodeInfo, FixedBucketSpaces.globalSpace())); + } else { + addBucketsPending(row, getStatsForDistributorNode(statsAggregator, nodeInfo, FixedBucketSpaces.defaultSpace())); + addBucketsPending(row, getStatsForDistributorNode(statsAggregator, nodeInfo, FixedBucketSpaces.globalSpace())); } // Start time if (nodeInfo.getStartTimestamp() == 0) { - row.addCell(new HtmlTable.Cell("-").addProperties(error).addProperties(centered)); + row.addCell(new HtmlTable.Cell("-").addProperties(ERROR_PROPERTY).addProperties(CENTERED_PROPERTY)); } else { String startTime = RealTimer.printDateNoMilliSeconds( 1000 * nodeInfo.getStartTimestamp(), utcTimeZone); @@ -270,16 +285,50 @@ public class VdsClusterHtmlRendrer { // RPC address if (nodeInfo.getRpcAddress() == null) { - row.addCell(new HtmlTable.Cell("-").addProperties(error)); + row.addCell(new HtmlTable.Cell("-").addProperties(ERROR_PROPERTY)); } else { row.addCell(new HtmlTable.Cell(HtmlTable.escape(nodeInfo.getRpcAddress()))); if (nodeInfo.isRpcAddressOutdated()) { - row.getLastCell().addProperties(warning); + row.getLastCell().addProperties(WARNING_PROPERTY); } } table.addRow(row); } } + + private static ContentNodeStats.BucketSpaceStats getStatsForContentNode(ClusterStatsAggregator statsAggregator, + NodeInfo nodeInfo, + String bucketSpace) { + ContentNodeStats nodeStats = statsAggregator.getAggregatedStats().getContentNode(nodeInfo.getNodeIndex()); + if (nodeStats != null) { + return nodeStats.getBucketSpace(bucketSpace); + } + return null; + } + + private static ContentNodeStats.BucketSpaceStats getStatsForDistributorNode(ClusterStatsAggregator statsAggregator, + NodeInfo nodeInfo, + String bucketSpace) { + ContentNodeStats nodeStats = statsAggregator.getAggregatedStatsForDistributor(nodeInfo.getNodeIndex()); + return nodeStats.getBucketSpace(bucketSpace); + } + + private static void addBucketsPending(HtmlTable.Row row, ContentNodeStats.BucketSpaceStats bucketSpaceStats) { + if (bucketSpaceStats != null) { + long bucketsPending = bucketSpaceStats.getBucketsPending(); + String cellValue = String.valueOf(bucketsPending); + if (!bucketSpaceStats.valid()) { + cellValue += "?"; + } + row.addCell(new HtmlTable.Cell(cellValue)); + if (bucketsPending > 0 || !bucketSpaceStats.valid()) { + row.getLastCell().addProperties(WARNING_PROPERTY); + } + } else { + row.addCell(new HtmlTable.Cell("-").addProperties(CENTERED_PROPERTY)); + } + } + private void addFooter(final StringBuilder contentBuilder, final long stableStateTimePeriode) { contentBuilder.append("<font size=\"-1\">\n") .append("1) FC - Failed connections - We have tried to connect to the nodes this many times " + diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregatorTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregatorTest.java index ac6417d0077..90c9700e001 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregatorTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregatorTest.java @@ -18,24 +18,50 @@ public class ClusterStatsAggregatorTest { private static class Fixture { private ClusterStatsAggregator aggregator; + public Fixture(Set<Integer> distributorNodes, Set<Integer> contentNodes) { aggregator = new ClusterStatsAggregator(distributorNodes, contentNodes); } + public void update(int distributorIndex, ContentClusterStatsBuilder clusterStats) { aggregator.updateForDistributor(distributorIndex, clusterStats.build()); } + public void verify(ContentClusterStatsBuilder expectedStats) { assertEquals(expectedStats.build(), aggregator.getAggregatedStats()); } + + public void verify(int distributorIndex, ContentNodeStatsBuilder expectedStats) { + assertEquals(expectedStats.build(), aggregator.getAggregatedStatsForDistributor(distributorIndex)); + } + public boolean hasUpdatesFromAllDistributors() { return aggregator.hasUpdatesFromAllDistributors(); } + public boolean mayHaveBucketsPendingInGlobalSpace() { return aggregator.mayHaveBucketsPendingInGlobalSpace(); } } + private static class FourNodesFixture extends Fixture { + public FourNodesFixture() { + super(distributorNodes(1, 2), contentNodes(3, 4)); + + update(1, new ContentClusterStatsBuilder() + .add(3, "default", 10, 1) + .add(3, "global", 11, 2) + .add(4, "default", 12, 3) + .add(4, "global", 13, 4)); + update(2, new ContentClusterStatsBuilder() + .add(3, "default", 14, 5) + .add(3, "global", 15, 6) + .add(4, "default", 16, 7) + .add(4, "global", 17, 8)); + } + } + private static Set<Integer> distributorNodes(Integer... indices) { return Sets.newHashSet(indices); } @@ -56,18 +82,8 @@ public class ClusterStatsAggregatorTest { @Test public void aggregator_handles_updates_to_multiple_distributors_and_content_nodes() { - Fixture f = new Fixture(distributorNodes(1, 2), contentNodes(3, 4)); + Fixture f = new FourNodesFixture(); - f.update(1, new ContentClusterStatsBuilder() - .add(3, "default", 10, 1) - .add(3, "global", 11, 2) - .add(4, "default", 12, 3) - .add(4, "global", 13, 4)); - f.update(2, new ContentClusterStatsBuilder() - .add(3, "default", 14, 5) - .add(3, "global", 15, 6) - .add(4, "default", 16, 7) - .add(4, "global", 17, 8)); f.verify(new ContentClusterStatsBuilder() .add(3, "default", 10 + 14, 1 + 5) .add(3, "global", 11 + 15, 2 + 6) @@ -154,4 +170,17 @@ public class ClusterStatsAggregatorTest { assertFalse(f.mayHaveBucketsPendingInGlobalSpace()); } + @Test + public void aggregator_can_provide_aggregated_stats_per_distributor() { + Fixture f = new FourNodesFixture(); + + f.verify(1, ContentNodeStatsBuilder.forNode(1) + .add("default", 10 + 12, 1 + 3) + .add("global", 11 + 13, 2 + 4)); + + f.verify(2, ContentNodeStatsBuilder.forNode(2) + .add("default", 14 + 16, 5 + 7) + .add("global", 15 + 17, 6 + 8)); + } + } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterHtmlRendrerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterHtmlRendrerTest.java index 24d387f65de..6dc260d6bf3 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterHtmlRendrerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterHtmlRendrerTest.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.clustercontroller.core; +import com.google.common.collect.Sets; import com.yahoo.vdslib.state.ClusterState; import com.yahoo.vespa.clustercontroller.core.hostinfo.HostInfo; import com.yahoo.vespa.clustercontroller.core.status.statuspage.VdsClusterHtmlRendrer; @@ -48,12 +49,14 @@ public class ContentClusterHtmlRendrerTest { distributorNodeInfoByIndex.put(x, nodeInfo); } storageNodeInfoByIndex.put(2, new StorageNodeInfo(contentCluster, 2, false, "storage" + 2, null)); + ClusterStatsAggregator statsAggregator = new ClusterStatsAggregator(Sets.newHashSet(2), Sets.newHashSet(2)); table.renderNodes( storageNodeInfoByIndex, distributorNodeInfoByIndex, new FakeTimer(), state, + statsAggregator, 10, eventLog, "pathPrefix", diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStatsBuilder.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStatsBuilder.java index 16767cafa8f..dd0205658cb 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStatsBuilder.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStatsBuilder.java @@ -9,7 +9,7 @@ import java.util.Map; */ public class ContentClusterStatsBuilder { - private final Map<Integer, Map<String, ContentNodeStats.BucketSpaceStats>> stats = new HashMap<>(); + private final Map<Integer, ContentNodeStatsBuilder> stats = new HashMap<>(); public ContentClusterStatsBuilder add(int nodeIndex, String bucketSpace, long bucketsTotal, long bucketsPending) { return add(nodeIndex, bucketSpace, ContentNodeStats.BucketSpaceStats.of(bucketsTotal, bucketsPending)); @@ -24,24 +24,24 @@ public class ContentClusterStatsBuilder { } public ContentClusterStatsBuilder add(int nodeIndex, String bucketSpace, ContentNodeStats.BucketSpaceStats bucketSpaceStats) { - Map<String, ContentNodeStats.BucketSpaceStats> contentNodeStats = stats.get(nodeIndex); - if (contentNodeStats == null) { - contentNodeStats = new HashMap<>(); - stats.put(nodeIndex, contentNodeStats); + ContentNodeStatsBuilder nodeStatsBuilder = stats.get(nodeIndex); + if (nodeStatsBuilder == null) { + nodeStatsBuilder = ContentNodeStatsBuilder.forNode(nodeIndex); + stats.put(nodeIndex, nodeStatsBuilder); } - contentNodeStats.put(bucketSpace, bucketSpaceStats); + nodeStatsBuilder.add(bucketSpace, bucketSpaceStats); return this; } public ContentClusterStatsBuilder add(int nodeIndex) { - stats.put(nodeIndex, new HashMap<>()); + stats.put(nodeIndex, ContentNodeStatsBuilder.forNode(nodeIndex)); return this; } public ContentClusterStats build() { Map<Integer, ContentNodeStats> nodeToStatsMap = new HashMap<>(); - stats.forEach((nodeIndex, bucketSpaces) -> - nodeToStatsMap.put(nodeIndex, new ContentNodeStats(nodeIndex, bucketSpaces))); + stats.forEach((nodeIndex, nodeStatsBuilder) -> + nodeToStatsMap.put(nodeIndex, nodeStatsBuilder.build())); return new ContentClusterStats(nodeToStatsMap); } } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStatsBuilder.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStatsBuilder.java new file mode 100644 index 00000000000..9fb020260e9 --- /dev/null +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStatsBuilder.java @@ -0,0 +1,31 @@ +package com.yahoo.vespa.clustercontroller.core; + +import java.util.HashMap; +import java.util.Map; + +public class ContentNodeStatsBuilder { + + private final int nodeIndex; + private final Map<String, ContentNodeStats.BucketSpaceStats> stats = new HashMap<>(); + + private ContentNodeStatsBuilder(int nodeIndex) { + this.nodeIndex = nodeIndex; + } + + public static ContentNodeStatsBuilder forNode(int nodeIndex) { + return new ContentNodeStatsBuilder(nodeIndex); + } + + public ContentNodeStatsBuilder add(String bucketSpace, long bucketsTotal, long bucketsPending) { + return add(bucketSpace, ContentNodeStats.BucketSpaceStats.of(bucketsTotal, bucketsPending)); + } + + public ContentNodeStatsBuilder add(String bucketSpace, ContentNodeStats.BucketSpaceStats bucketSpaceStats) { + stats.put(bucketSpace, bucketSpaceStats); + return this; + } + + public ContentNodeStats build() { + return new ContentNodeStats(nodeIndex, stats); + } +} diff --git a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/async/AsyncTest.java b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/async/AsyncTest.java index 9d8bb6d10e4..2817c83a041 100644 --- a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/async/AsyncTest.java +++ b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/async/AsyncTest.java @@ -1,14 +1,15 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.utils.communication.async; -import junit.framework.TestCase; -import org.junit.Ignore; import org.junit.Test; import java.util.LinkedList; -public class AsyncTest extends TestCase { +import static org.junit.Assert.*; +public class AsyncTest { + + @Test public void testListeners() { AsyncOperationImpl<String> op = new AsyncOperationImpl<>("test"); class Listener implements AsyncCallback<String> { @@ -28,16 +29,17 @@ public class AsyncTest extends TestCase { op.unregister(l1); op.setResult("foo"); op.register(l4); - // Listener that is unregistered is not called + // Listener that is unregistered is not called assertEquals(false, l1.called); - // Listener that is registered is called + // Listener that is registered is called assertEquals(true, l2.called); - // Multiple listeners supported + // Multiple listeners supported assertEquals(true, l3.called); - // Listener called directly when registered after result is set + // Listener called directly when registered after result is set assertEquals(true, l4.called); } + @Test public void testMultipleResultSetters() { { AsyncOperationImpl<String> op = new AsyncOperationImpl<>("test"); @@ -62,6 +64,7 @@ public class AsyncTest extends TestCase { } } + @Test public void testPartialResultOnFailure() { AsyncOperationImpl<String> op = new AsyncOperationImpl<>("test"); op.setFailure(new Exception("bar"), "foo"); @@ -70,6 +73,7 @@ public class AsyncTest extends TestCase { assertEquals("bar", op.getCause().getMessage()); } + @Test public void testListenImpl() { class ListenImpl extends AsyncOperationListenImpl<String> { public ListenImpl(AsyncOperation<String> op) { @@ -93,6 +97,7 @@ public class AsyncTest extends TestCase { assertEquals(1, l1.calls); } + @Test public void testRedirectedOperation() { { final AsyncOperationImpl<String> op = new AsyncOperationImpl<>("test", "desc"); @@ -141,6 +146,7 @@ public class AsyncTest extends TestCase { } } + @Test public void testRedirectOnSuccessOperation() { { final AsyncOperationImpl<Integer> target = new AsyncOperationImpl<>("foo"); @@ -236,6 +242,7 @@ public class AsyncTest extends TestCase { } } + @Test public void testStressCompletionAndRegisterToDetectRace() throws Exception { int iterations = 1000; Object monitor = new Object(); @@ -247,7 +254,7 @@ public class AsyncTest extends TestCase { t1.start(); t2.start(); for (int i=0; i<iterations; ++i) { - final AsyncOperationImpl<String> op = new AsyncOperationImpl<>("test"); + AsyncOperationImpl<String> op = new AsyncOperationImpl<>("test"); synchronized (monitor) { completer.op = op; listener.op = op; @@ -263,23 +270,8 @@ public class AsyncTest extends TestCase { t1.join(); t2.join(); } - /* - System.out.println("Done with " + iterations + " iterations. " - + "Registered prior " + listener.priorReg + " times. " - + "Unset " + listener.unset + " times. "); - // */ assertEquals(0, listener.unset); assertEquals(iterations, listener.counter); } - public void ignoreTestExceptionOnCallback() throws Exception { - AsyncOperationImpl<String> impl = new AsyncOperationImpl<>("foo"); - impl.register(new AsyncCallback<String>() { - @Override - public void done(AsyncOperation<String> op) { - throw new RuntimeException("Foo"); - } - }); - impl.setResult(null); - } } diff --git a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/AsyncHttpClientWithBaseTest.java b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/AsyncHttpClientWithBaseTest.java index e04eb1c043d..685ad492052 100644 --- a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/AsyncHttpClientWithBaseTest.java +++ b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/AsyncHttpClientWithBaseTest.java @@ -3,10 +3,15 @@ package com.yahoo.vespa.clustercontroller.utils.communication.http; import com.yahoo.vespa.clustercontroller.utils.communication.async.AsyncOperation; import com.yahoo.vespa.clustercontroller.utils.communication.async.AsyncOperationImpl; -import junit.framework.TestCase; +import org.junit.Test; -public class AsyncHttpClientWithBaseTest extends TestCase { +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertTrue; +public class AsyncHttpClientWithBaseTest { + + @Test public void testOverride() { class HttpClient implements AsyncHttpClient<HttpResult> { HttpRequest lastRequest; @@ -39,11 +44,13 @@ public class AsyncHttpClientWithBaseTest extends TestCase { base.close(); } + @Test public void testClientMustBeSet() { - try{ - new AsyncHttpClientWithBase<HttpResult>(null); + try { + new AsyncHttpClientWithBase<>(null); assertTrue(false); } catch (IllegalArgumentException e) { } } + } diff --git a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/HttpRequestTest.java b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/HttpRequestTest.java index 9d51954e567..7baff0453b0 100644 --- a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/HttpRequestTest.java +++ b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/HttpRequestTest.java @@ -1,9 +1,13 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.utils.communication.http; -import junit.framework.TestCase; +import org.junit.Test; -public class HttpRequestTest extends TestCase { +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertTrue; + +public class HttpRequestTest { private HttpRequest createRequest() { return new HttpRequest() @@ -16,6 +20,7 @@ public class HttpRequestTest extends TestCase { .setTimeout(25); } + @Test public void testEquality() { assertEquals(createRequest(), createRequest()); assertNotSame(createRequest(), createRequest().setHost("localhost")); @@ -25,6 +30,7 @@ public class HttpRequestTest extends TestCase { assertNotSame(createRequest(), createRequest().setHttpOperation(HttpRequest.HttpOp.DELETE)); } + @Test public void testVerifyComplete() { // To be a complete request, an HTTP request must include: // - A path @@ -42,6 +48,7 @@ public class HttpRequestTest extends TestCase { new HttpRequest().setHttpOperation(HttpRequest.HttpOp.GET).setPath("/bar").verifyComplete(); } + @Test public void testMerge() { { HttpRequest base = new HttpRequest() @@ -84,16 +91,19 @@ public class HttpRequestTest extends TestCase { } } + @Test public void testNonExistingHeader() { assertEquals("foo", new HttpRequest().getHeader("asd", "foo")); assertEquals("foo", new HttpRequest().addHttpHeader("bar", "foo").getHeader("asd", "foo")); } + @Test public void testOption() { assertEquals("bar", new HttpRequest().addUrlOption("foo", "bar").getOption("foo", "foo")); assertEquals("foo", new HttpRequest().getOption("asd", "foo")); } + @Test public void testToString() { assertEquals("GET? http://localhost:8080/", new HttpRequest() @@ -113,9 +123,11 @@ public class HttpRequestTest extends TestCase { .toString(true)); } + @Test public void testNothingButGetCoverage() { assertEquals(false, new HttpRequest().equals(new Object())); new HttpRequest().getHeaders(); new HttpRequest().setUrlOptions(new HttpRequest().getUrlOptions()); } + } diff --git a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/HttpResultTest.java b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/HttpResultTest.java index 6f165e8d35c..f137e9b9580 100644 --- a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/HttpResultTest.java +++ b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/HttpResultTest.java @@ -1,10 +1,13 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.utils.communication.http; -import junit.framework.TestCase; +import org.junit.Test; -public class HttpResultTest extends TestCase { +import static org.junit.Assert.assertEquals; +public class HttpResultTest { + + @Test public void testSuccess() { assertEquals(false, new HttpResult().setHttpCode(199, "foo").isSuccess()); assertEquals(true, new HttpResult().setHttpCode(200, "foo").isSuccess()); @@ -12,6 +15,7 @@ public class HttpResultTest extends TestCase { assertEquals(false, new HttpResult().setHttpCode(300, "foo").isSuccess()); } + @Test public void testToString() { assertEquals("HTTP 200/OK", new HttpResult().setContent("Foo").toString()); assertEquals("HTTP 200/OK\n\nFoo", new HttpResult().setContent("Foo").toString(true)); @@ -19,7 +23,9 @@ public class HttpResultTest extends TestCase { assertEquals("HTTP 200/OK", new HttpResult().setContent("").toString(true)); } + @Test public void testNothingButGetCoverage() { new HttpResult().getHeaders(); } + } diff --git a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/JsonAsyncHttpClientTest.java b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/JsonAsyncHttpClientTest.java index 5fa8ac87da6..3d3cd517020 100644 --- a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/JsonAsyncHttpClientTest.java +++ b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/JsonAsyncHttpClientTest.java @@ -3,11 +3,16 @@ package com.yahoo.vespa.clustercontroller.utils.communication.http; import com.yahoo.vespa.clustercontroller.utils.communication.async.AsyncOperation; import com.yahoo.vespa.clustercontroller.utils.communication.async.AsyncOperationImpl; -import junit.framework.TestCase; import org.codehaus.jettison.json.JSONObject; +import org.junit.Test; -public class JsonAsyncHttpClientTest extends TestCase { +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +public class JsonAsyncHttpClientTest { + + @Test public void testJSONInJSONOut() throws Exception { DummyAsyncHttpClient dummy = new DummyAsyncHttpClient( new HttpResult().setContent(new JSONObject().put("bar", 42))); @@ -27,6 +32,7 @@ public class JsonAsyncHttpClientTest extends TestCase { client.close(); } + @Test public void testStringInJSONOut() throws Exception { DummyAsyncHttpClient dummy = new DummyAsyncHttpClient( new HttpResult().setContent(new JSONObject().put("bar", 42).toString())); @@ -40,6 +46,7 @@ public class JsonAsyncHttpClientTest extends TestCase { assertEquals(new JSONObject().put("bar", 42).toString(), result.getResult().getJson().toString()); } + @Test public void testIllegalJsonIn() throws Exception { DummyAsyncHttpClient dummy = new DummyAsyncHttpClient( new HttpResult().setContent(new JSONObject().put("bar", 42))); @@ -56,6 +63,7 @@ public class JsonAsyncHttpClientTest extends TestCase { } } + @Test public void testIllegalJSONOut() throws Exception { DummyAsyncHttpClient dummy = new DummyAsyncHttpClient( new HttpResult().setContent("my illegal json")); @@ -69,7 +77,8 @@ public class JsonAsyncHttpClientTest extends TestCase { assertEquals("{\"error\":\"Invalid JSON in output: A JSONObject text must begin with '{' at character 1 of my illegal json\",\"output\":\"my illegal json\"}", result.getResult().getJson().toString()); } - public void testEmptyReply() throws Exception { + @Test + public void testEmptyReply() { class Client implements AsyncHttpClient<HttpResult> { AsyncOperationImpl<HttpResult> lastOp; @Override @@ -87,6 +96,7 @@ public class JsonAsyncHttpClientTest extends TestCase { assertNull(op.getResult()); } + @Test public void testNotVerifyingJson() throws Exception { DummyAsyncHttpClient dummy = new DummyAsyncHttpClient( new HttpResult().setContent(new JSONObject().put("bar", 42))); @@ -105,4 +115,5 @@ public class JsonAsyncHttpClientTest extends TestCase { result.toString(); client.close(); } + } diff --git a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/JsonHttpResultTest.java b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/JsonHttpResultTest.java index 07b11214301..8265247b124 100644 --- a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/JsonHttpResultTest.java +++ b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/JsonHttpResultTest.java @@ -1,16 +1,20 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.utils.communication.http; -import junit.framework.TestCase; import org.codehaus.jettison.json.JSONException; import org.codehaus.jettison.json.JSONObject; +import org.junit.Test; -public class JsonHttpResultTest extends TestCase { +import static org.junit.Assert.assertEquals; +public class JsonHttpResultTest { + + @Test public void testCopyConstructor() { assertEquals("{}", new JsonHttpResult(new HttpResult()).getJson().toString()); } + @Test public void testOutput() { assertEquals("HTTP 200/OK\n" + "\n" @@ -22,6 +26,7 @@ public class JsonHttpResultTest extends TestCase { new JsonHttpResult(new HttpResult().setContent("{ \"foo\" : }")).toString(true)); } + @Test public void testNonJsonOutput() { JsonHttpResult result = new JsonHttpResult(); result.setContent("Foo"); @@ -30,6 +35,7 @@ public class JsonHttpResultTest extends TestCase { assertEquals("Foo", sb.toString()); } + @Test public void testInvalidJsonOutput() { JsonHttpResult result = new JsonHttpResult(); result.setJson(new JSONObject() { @@ -42,4 +48,5 @@ public class JsonHttpResultTest extends TestCase { result.printContent(sb); assertEquals("JSON: {}", sb.toString()); } + } diff --git a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/LoggingAsyncHttpClientTest.java b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/LoggingAsyncHttpClientTest.java index 0191c29dd66..37841f7ca29 100644 --- a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/LoggingAsyncHttpClientTest.java +++ b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/LoggingAsyncHttpClientTest.java @@ -3,12 +3,15 @@ package com.yahoo.vespa.clustercontroller.utils.communication.http; import com.yahoo.vespa.clustercontroller.utils.communication.async.AsyncOperation; import com.yahoo.vespa.clustercontroller.utils.communication.async.AsyncOperationImpl; -import junit.framework.TestCase; +import org.junit.Test; import java.util.logging.Level; import java.util.logging.Logger; -public class LoggingAsyncHttpClientTest extends TestCase { +import static org.junit.Assert.assertEquals; + +public class LoggingAsyncHttpClientTest { + class HttpClient implements AsyncHttpClient<HttpResult> { AsyncOperationImpl<HttpResult> lastOp; @Override @@ -20,11 +23,13 @@ public class LoggingAsyncHttpClientTest extends TestCase { } } - public void testWithoutDebugLog() throws Exception { + @Test + public void testWithoutDebugLog() { doRequests(); } - public void testWithDebugLog() throws Exception { + @Test + public void testWithDebugLog() { Logger log = Logger.getLogger(LoggingAsyncHttpClient.class.getName()); log.setLevel(Level.FINE); doRequests(); @@ -45,6 +50,6 @@ public class LoggingAsyncHttpClientTest extends TestCase { client.lastOp.setFailure(new Exception("foo")); assertEquals("foo", op.getCause().getMessage()); } - } + } diff --git a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/ProxyAsyncHttpClientTest.java b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/ProxyAsyncHttpClientTest.java index d231dadc9b1..062fd4aaa32 100644 --- a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/ProxyAsyncHttpClientTest.java +++ b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/ProxyAsyncHttpClientTest.java @@ -1,11 +1,15 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.utils.communication.http; -import junit.framework.TestCase; import org.codehaus.jettison.json.JSONObject; +import org.junit.Test; -public class ProxyAsyncHttpClientTest extends TestCase { +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +public class ProxyAsyncHttpClientTest { + + @Test public void testSimple() throws Exception { // Can't really test much here, but verifies that the code runs. DummyAsyncHttpClient dummy = new DummyAsyncHttpClient( @@ -28,6 +32,7 @@ public class ProxyAsyncHttpClientTest extends TestCase { dummy.lastRequest); } + @Test public void testNoAndEmptyPath() throws Exception { DummyAsyncHttpClient dummy = new DummyAsyncHttpClient( new HttpResult().setContent(new JSONObject().put("bar", 42))); @@ -40,4 +45,5 @@ public class ProxyAsyncHttpClientTest extends TestCase { } client.execute(new HttpRequest().setHost("local").setPath("")); } + } diff --git a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/RequestQueueTest.java b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/RequestQueueTest.java index e3b6f7f7528..230920df53f 100644 --- a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/RequestQueueTest.java +++ b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/RequestQueueTest.java @@ -4,11 +4,14 @@ package com.yahoo.vespa.clustercontroller.utils.communication.http; import com.yahoo.vespa.clustercontroller.utils.communication.async.AsyncCallback; import com.yahoo.vespa.clustercontroller.utils.communication.async.AsyncOperation; import com.yahoo.vespa.clustercontroller.utils.communication.async.AsyncOperationImpl; -import junit.framework.TestCase; +import org.junit.Test; import java.util.LinkedList; -public class RequestQueueTest extends TestCase { +import static org.junit.Assert.assertEquals; + +public class RequestQueueTest { + public static class Request { public final HttpRequest request; public final AsyncOperationImpl<HttpResult> result; @@ -33,6 +36,7 @@ public class RequestQueueTest extends TestCase { public void close() {} }; + @Test public void testNormalUsage() { TestClient client = new TestClient(); RequestQueue<HttpResult> queue = new RequestQueue<>(client, 4); @@ -82,6 +86,7 @@ public class RequestQueueTest extends TestCase { } } + @Test public void testWaitUntilEmpty() throws Exception { TestClient client = new TestClient(); RequestQueue<HttpResult> queue = new RequestQueue<>(client, 4); @@ -105,4 +110,5 @@ public class RequestQueueTest extends TestCase { } assertEquals(1, result.size()); } + } diff --git a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/TimeoutHandlerTest.java b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/TimeoutHandlerTest.java index 8c567a25b3a..72a2a4eab8a 100644 --- a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/TimeoutHandlerTest.java +++ b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/TimeoutHandlerTest.java @@ -5,14 +5,19 @@ import com.yahoo.vespa.clustercontroller.utils.communication.async.AsyncOperatio import com.yahoo.vespa.clustercontroller.utils.communication.async.AsyncOperationImpl; import com.yahoo.vespa.clustercontroller.utils.communication.async.AsyncUtils; import com.yahoo.vespa.clustercontroller.utils.test.FakeClock; -import junit.framework.TestCase; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; import java.util.concurrent.ArrayBlockingQueue; -import java.util.concurrent.Executor; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; -public class TimeoutHandlerTest extends TestCase { +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class TimeoutHandlerTest { public class TestClient implements AsyncHttpClient<HttpResult> { AsyncOperationImpl<HttpResult> lastOp; @@ -29,6 +34,7 @@ public class TimeoutHandlerTest extends TestCase { private FakeClock clock; private TimeoutHandler<HttpResult> handler; + @Before public void setUp() { executor = new ThreadPoolExecutor(10, 100, 100, TimeUnit.SECONDS, new ArrayBlockingQueue<Runnable>(1000)); clock = new FakeClock(); @@ -36,11 +42,13 @@ public class TimeoutHandlerTest extends TestCase { handler = new TimeoutHandler<>(executor, clock, client); } + @After public void tearDown() { handler.close(); executor.shutdown(); } + @Test public void testTimeout() { AsyncOperation<HttpResult> op = handler.execute(new HttpRequest().setTimeout(1000)); assertFalse(op.isDone()); @@ -59,6 +67,7 @@ public class TimeoutHandlerTest extends TestCase { assertTrue(op.getCause().getMessage(), op.getCause().getMessage().contains("Operation timeout")); } + @Test public void testNoTimeout() { AsyncOperation<HttpResult> op = handler.execute(new HttpRequest().setTimeout(1000)); clock.adjust(999); @@ -70,6 +79,7 @@ public class TimeoutHandlerTest extends TestCase { assertEquals("foo", op.getResult().getContent()); } + @Test public void testNoTimeoutFailing() { AsyncOperation<HttpResult> op = handler.execute(new HttpRequest().setTimeout(1000)); clock.adjust(999); @@ -81,6 +91,7 @@ public class TimeoutHandlerTest extends TestCase { assertEquals("foo", op.getCause().getMessage()); } + @Test public void testProvokeCompletedOpPurgeInTimeoutList() { AsyncOperation<HttpResult> op1 = handler.execute(new HttpRequest().setTimeout(1000)); AsyncOperationImpl<HttpResult> op1Internal = client.lastOp; @@ -97,6 +108,7 @@ public class TimeoutHandlerTest extends TestCase { assertEquals(false, op2.isSuccess()); } + @Test public void testNothingButGetCoverage() { AsyncOperation<HttpResult> op = handler.execute(new HttpRequest().setTimeout(1000)); op.getProgress(); @@ -111,4 +123,5 @@ public class TimeoutHandlerTest extends TestCase { client.lastOp.setResult(new HttpResult().setContent("foo")); AsyncUtils.waitFor(op); } + } diff --git a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/writer/HttpWriterTest.java b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/writer/HttpWriterTest.java index b358c3cbbb5..5ef6239894c 100644 --- a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/writer/HttpWriterTest.java +++ b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/communication/http/writer/HttpWriterTest.java @@ -1,9 +1,13 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.utils.communication.http.writer; -import junit.framework.TestCase; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +public class HttpWriterTest { -public class HttpWriterTest extends TestCase { private static String defaultTitle = "My Title"; private static String defaultHeader = "<html>\n" + " <head>\n" @@ -14,16 +18,20 @@ public class HttpWriterTest extends TestCase { private static String defaultFooter = " </body>\n" + "</html>\n"; - + @Test public void testStructure() { HttpWriter writer = new HttpWriter(); String header = defaultHeader.replace(defaultTitle, "Untitled page"); assertEquals(header + defaultFooter, writer.toString()); } + + @Test public void testTitle() { HttpWriter writer = new HttpWriter().addTitle(defaultTitle); assertEquals(defaultHeader + defaultFooter, writer.toString()); } + + @Test public void testParagraph() { String paragraph = "This is a paragraph"; String paragraph2 = "More text"; @@ -36,6 +44,8 @@ public class HttpWriterTest extends TestCase { + " </p>\n"; assertEquals(defaultHeader + content + defaultFooter, writer.toString()); } + + @Test public void testLink() { String name = "My link"; String link = "/foo/bar?hmm"; @@ -43,6 +53,8 @@ public class HttpWriterTest extends TestCase { String content = " <a href=\"" + link + "\">" + name + "</a>\n"; assertEquals(defaultHeader + content + defaultFooter, writer.toString()); } + + @Test public void testErrors() { try{ HttpWriter writer = new HttpWriter().addTitle(defaultTitle); @@ -57,4 +69,5 @@ public class HttpWriterTest extends TestCase { } catch (IllegalStateException e) { } } + } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java index f16697b5ba6..864cd823728 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java @@ -23,10 +23,18 @@ import com.yahoo.searchlib.rankingexpression.rule.Arguments; import com.yahoo.searchlib.rankingexpression.rule.CompositeNode; import com.yahoo.searchlib.rankingexpression.rule.ConstantNode; import com.yahoo.searchlib.rankingexpression.rule.ExpressionNode; +import com.yahoo.searchlib.rankingexpression.rule.GeneratorLambdaFunctionNode; import com.yahoo.searchlib.rankingexpression.rule.ReferenceNode; +import com.yahoo.searchlib.rankingexpression.rule.TensorFunctionNode; import com.yahoo.searchlib.rankingexpression.transform.ExpressionTransformer; import com.yahoo.tensor.Tensor; import com.yahoo.tensor.TensorType; +import com.yahoo.tensor.functions.Generate; +import com.yahoo.tensor.functions.Join; +import com.yahoo.tensor.functions.Reduce; +import com.yahoo.tensor.functions.Rename; +import com.yahoo.tensor.functions.ScalarFunctions; +import com.yahoo.tensor.functions.TensorFunction; import com.yahoo.tensor.serialization.TypedBinaryFormat; import java.io.BufferedReader; @@ -37,9 +45,11 @@ import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.logging.Logger; /** @@ -197,7 +207,7 @@ public class TensorFlowFeatureConverter extends ExpressionTransformer<RankProfil */ private void verifyRequiredMacros(RankingExpression expression, Map<String, TensorType> requiredMacros, RankProfile profile, QueryProfileRegistry queryProfiles) { - List<String> macroNames = new ArrayList<>(); + Set<String> macroNames = new HashSet<>(); addMacroNamesIn(expression.getRoot(), macroNames); for (String macroName : macroNames) { TensorType requiredType = requiredMacros.get(macroName); @@ -223,10 +233,84 @@ public class TensorFlowFeatureConverter extends ExpressionTransformer<RankProfil "' of type " + requiredType + " which must be produced by a macro in the rank profile, but " + "this macro produces type " + actualType); + + // Check if batch dimensions can be reduced out. + reduceBatchDimensions(expression, macro, actualType); + } + } + + /** + * If the macro specifies that a single exemplar should be + * evaluated, we can reduce the batch dimension out. + */ + private void reduceBatchDimensions(RankingExpression expression, RankProfile.Macro macro, TensorType type) { + if (type.dimensions().size() > 1) { + List<String> reduceDimensions = new ArrayList<>(); + for (TensorType.Dimension dimension : type.dimensions()) { + if (dimension.size().orElse(-1L) == 1) { + reduceDimensions.add(dimension.name()); + } + } + if (reduceDimensions.size() > 0) { + ExpressionNode root = expression.getRoot(); + root = reduceBatchDimensionsAtInput(root, macro, reduceDimensions); + root = expandBatchDimensionsAtOutput(root, reduceDimensions); // todo: determine when we can skip this + expression.setRoot(root); + } + } + } + + private ExpressionNode reduceBatchDimensionsAtInput(ExpressionNode node, + RankProfile.Macro macro, + List<String> reduceDimensions) { + if (node instanceof TensorFunctionNode) { + TensorFunction tensorFunction = ((TensorFunctionNode) node).function(); + if (tensorFunction instanceof Rename) { + List<ExpressionNode> children = ((TensorFunctionNode)node).children(); + if (children.size() == 1 && children.get(0) instanceof ReferenceNode) { + ReferenceNode referenceNode = (ReferenceNode) children.get(0); + if (referenceNode.getName().equals(macro.getName())) { + return reduceBatchDimensionExpression(tensorFunction, reduceDimensions); + } + } + } + } + if (node instanceof ReferenceNode) { + ReferenceNode referenceNode = (ReferenceNode) node; + if (referenceNode.getName().equals(macro.getName())) { + return reduceBatchDimensionExpression(TensorFunctionNode.wrapArgument(node), reduceDimensions); + } + } + if (node instanceof CompositeNode) { + List<ExpressionNode> children = ((CompositeNode)node).children(); + List<ExpressionNode> transformedChildren = new ArrayList<>(children.size()); + for (ExpressionNode child : children) { + transformedChildren.add(reduceBatchDimensionsAtInput(child, macro, reduceDimensions)); + } + return ((CompositeNode)node).setChildren(transformedChildren); + } + return node; + } + + private ExpressionNode reduceBatchDimensionExpression(TensorFunction function, List<String> reduceDimensions) { + return new TensorFunctionNode(new Reduce(function, Reduce.Aggregator.sum, reduceDimensions)); + } + + private ExpressionNode expandBatchDimensionsAtOutput(ExpressionNode node, + List<String> reduceDimensions) { + TensorType.Builder typeBuilder = new TensorType.Builder(); + for (String name : reduceDimensions) { + typeBuilder.indexed(name, 1); } + TensorType generatedType = typeBuilder.build(); + ExpressionNode generatedExpression = new ConstantNode(new DoubleValue(1)); + Generate generatedFunction = new Generate(generatedType, + new GeneratorLambdaFunctionNode(generatedType, generatedExpression).asLongListToDoubleOperator()); + Join expand = new Join(TensorFunctionNode.wrapArgument(node), generatedFunction, ScalarFunctions.multiply()); + return new TensorFunctionNode(expand); } - private void addMacroNamesIn(ExpressionNode node, List<String> names) { + private void addMacroNamesIn(ExpressionNode node, Set<String> names) { if (node instanceof ReferenceNode) { ReferenceNode referenceNode = (ReferenceNode)node; if (referenceNode.getOutput() == null) // macro references cannot specify outputs diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java index cc9957144f3..44de53991f4 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java @@ -2,22 +2,23 @@ package com.yahoo.vespa.model.builder.xml.dom; import com.yahoo.component.Version; +import com.yahoo.config.model.ConfigModelContext; import com.yahoo.config.model.api.ConfigServerSpec; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.vespa.model.HostResource; import com.yahoo.vespa.model.HostSystem; -import com.yahoo.vespa.model.admin.*; -import com.yahoo.config.model.ConfigModelContext; import com.yahoo.vespa.model.admin.Admin; +import com.yahoo.vespa.model.admin.Logserver; +import com.yahoo.vespa.model.admin.Slobrok; import com.yahoo.vespa.model.container.Container; import com.yahoo.vespa.model.container.ContainerModel; - import org.w3c.dom.Element; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; @@ -27,6 +28,7 @@ import java.util.stream.Collectors; * @author bratseth */ public class DomAdminV4Builder extends DomAdminBuilderBase { + private ApplicationId ZONE_APPLICATION_ID = ApplicationId.from("hosted-vespa", "routing", "default"); private final Collection<ContainerModel> containerModels; private final ConfigModelContext context; @@ -63,7 +65,7 @@ public class DomAdminV4Builder extends DomAdminBuilderBase { createSlobroks(admin, allocateHosts(admin.getHostSystem(), "slobroks", nodesSpecification)); } else { - createSlobroks(admin, pickContainerHosts(nodesSpecification.count(), 2)); + createSlobroks(admin, pickContainerHostsForSlobrok(nodesSpecification.count(), 2)); } } @@ -94,17 +96,36 @@ public class DomAdminV4Builder extends DomAdminBuilderBase { * on topology changes, and less nodes may be returned if fewer are available * @param minHostsPerContainerCluster the desired number of hosts per cluster */ - private List<HostResource> pickContainerHosts(int count, int minHostsPerContainerCluster) { + private List<HostResource> pickContainerHostsForSlobrok(int count, int minHostsPerContainerCluster) { + Collection<ContainerModel> containerModelsWithSlobrok = containerModels.stream() + .filter(this::shouldHaveSlobrok) + .collect(Collectors.toList()); + int hostsPerCluster = (int) Math.max( + minHostsPerContainerCluster, + Math.ceil((double) count / containerModelsWithSlobrok.size())); + // Pick from all container clusters to make sure we don't lose all nodes at once if some clusters are removed. // This will overshoot the desired size (due to ceil and picking at least one node per cluster). List<HostResource> picked = new ArrayList<>(); - for (ContainerModel containerModel : containerModels) - picked.addAll(pickContainerHostsFrom(containerModel, - (int) Math.max(minHostsPerContainerCluster, - Math.ceil((double) count / containerModels.size())))); + for (ContainerModel containerModel : containerModelsWithSlobrok) + picked.addAll(pickContainerHostsFrom(containerModel, hostsPerCluster)); return picked; } + private boolean shouldHaveSlobrok(ContainerModel containerModel) { + // Avoid Slobroks on node-admin container cluster, as node-admin is migrating + // TODO: Remove this hack once node-admin has migrated out the zone app + + ApplicationId applicationId = context.getDeployState().getProperties().applicationId(); + if (!applicationId.equals(ZONE_APPLICATION_ID)) { + return true; + } + + // aka clustername, aka application-model's ClusterId + String clustername = containerModel.getCluster().getName(); + return !Objects.equals(clustername, "node-admin"); + } + private List<HostResource> pickContainerHostsFrom(ContainerModel model, int count) { boolean retired = true; List<HostResource> picked = sortedContainerHostsFrom(model, count, !retired); diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java index 91be617d0ea..d4ce445d50c 100644 --- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java +++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java @@ -1,46 +1,57 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.model.provision; -import static com.yahoo.config.model.test.TestUtil.joinLines; -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.*; - -import java.io.StringReader; -import java.util.Collection; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.Set; -import java.util.stream.Collectors; - import com.yahoo.config.application.api.ApplicationPackage; +import com.yahoo.config.model.api.HostInfo; import com.yahoo.config.model.deploy.DeployProperties; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.Flavor; import com.yahoo.config.provisioning.FlavorsConfig; import com.yahoo.search.config.QrStartConfig; +import com.yahoo.searchdefinition.parser.ParseException; import com.yahoo.vespa.config.search.core.ProtonConfig; -import static com.yahoo.vespa.defaults.Defaults.getDefaults; +import com.yahoo.vespa.model.AbstractService; import com.yahoo.vespa.model.HostResource; import com.yahoo.vespa.model.HostSystem; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.admin.Admin; import com.yahoo.vespa.model.admin.Slobrok; import com.yahoo.vespa.model.container.Container; import com.yahoo.vespa.model.container.ContainerCluster; import com.yahoo.vespa.model.content.ContentSearchCluster; +import com.yahoo.vespa.model.content.StorageNode; +import com.yahoo.vespa.model.content.cluster.ContentCluster; import com.yahoo.vespa.model.search.Dispatch; import com.yahoo.vespa.model.search.SearchNode; import com.yahoo.vespa.model.test.VespaModelTester; +import com.yahoo.vespa.model.test.utils.ApplicationPackageUtils; +import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg; import org.junit.Ignore; import org.junit.Test; -import com.yahoo.config.model.deploy.DeployState; -import com.yahoo.searchdefinition.parser.ParseException; -import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.content.StorageNode; -import com.yahoo.vespa.model.content.cluster.ContentCluster; -import com.yahoo.vespa.model.test.utils.ApplicationPackageUtils; -import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg; +import java.io.StringReader; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +import static com.yahoo.config.model.test.TestUtil.joinLines; +import static com.yahoo.vespa.defaults.Defaults.getDefaults; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.collection.IsIn.isIn; +import static org.hamcrest.core.Every.everyItem; +import static org.hamcrest.core.IsNot.not; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * Test cases for provisioning nodes to entire vespamodels @@ -736,6 +747,52 @@ public class ModelProvisioningTest { } @Test + public void testSlobroksAreSpreadOverAllContainerClustersExceptNodeAdmin() { + String services = + "<?xml version='1.0' encoding='utf-8' ?>\n" + + "<services>" + + " <admin version='4.0'/>" + + " <container version='1.0' id='routing'>" + + " <nodes count='10'/>" + + " </container>" + + " <container version='1.0' id='node-admin'>" + + " <nodes count='3'/>" + + " </container>" + + "</services>"; + + int numberOfHosts = 13; + VespaModelTester tester = new VespaModelTester(); + tester.addHosts(numberOfHosts); + tester.setApplicationId("hosted-vespa", "routing", "default"); + VespaModel model = tester.createModel(services, true); + assertThat(model.getRoot().getHostSystem().getHosts().size(), is(numberOfHosts)); + + Set<String> routingHosts = getClusterHostnames(model, "routing"); + assertEquals(10, routingHosts.size()); + + Set<String> nodeAdminHosts = getClusterHostnames(model, "node-admin"); + assertEquals(3, nodeAdminHosts.size()); + + Set<String> slobrokHosts = model.getAdmin().getSlobroks().stream() + .map(AbstractService::getHostName) + .collect(Collectors.toSet()); + assertEquals(3, slobrokHosts.size()); + + assertThat(slobrokHosts, everyItem(isIn(routingHosts))); + assertThat(slobrokHosts, everyItem(not(isIn(nodeAdminHosts)))); + } + + private Set<String> getClusterHostnames(VespaModel model, String clusterId) { + return model.getHosts().stream() + .filter(host -> host.getServices().stream() + .anyMatch(serviceInfo -> Objects.equals( + serviceInfo.getProperty("clustername"), + Optional.of(clusterId)))) + .map(HostInfo::getHostname) + .collect(Collectors.toSet()); + } + + @Test public void test2ContentNodesProduces1ClusterController() { String services = "<?xml version='1.0' encoding='utf-8' ?>\n" + diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java index 19d37c5fb44..beba8ade1d8 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java @@ -252,10 +252,20 @@ public class RankingExpressionWithTensorFlowTestCase { } @Test + public void testTensorFlowReduceBatchDimension() { + final String expression = "join(join(reduce(join(reduce(rename(Placeholder, (d0, d1), (d0, d2)), sum, d0), constant(\"layer_Variable_read\"), f(a,b)(a * b)), sum, d2), constant(\"layer_Variable_1_read\"), f(a,b)(a + b)), tensor(d0[1])(1.0), f(a,b)(a * b))"; + RankProfileSearchFixture search = fixtureWith("tensor(d0[1],d1[784])(0.0)", + "tensorflow('mnist_softmax/saved')"); + search.assertFirstPhaseExpression(expression, "my_profile"); + assertLargeConstant("layer_Variable_1_read", search, Optional.of(10L)); + assertLargeConstant("layer_Variable_read", search, Optional.of(7840L)); + } + + @Test public void testImportingFromStoredExpressionsWithSmallConstants() throws IOException { - final String expression = "join(reduce(join(join(join(constant(\"dnn_hidden2_Const\"), join(reduce(join(join(join(0.009999999776482582, join(reduce(join(rename(input, (d0, d1), (d0, d4)), constant(\"dnn_hidden1_weights_read\"), f(a,b)(a * b)), sum, d4), constant(\"dnn_hidden1_bias_read\"), f(a,b)(a + b)), f(a,b)(a * b)), join(reduce(join(rename(input, (d0, d1), (d0, d4)), constant(\"dnn_hidden1_weights_read\"), f(a,b)(a * b)), sum, d4), constant(\"dnn_hidden1_bias_read\"), f(a,b)(a + b)), f(a,b)(max(a,b))), constant(\"dnn_hidden2_weights_read\"), f(a,b)(a * b)), sum, d3), constant(\"dnn_hidden2_bias_read\"), f(a,b)(a + b)), f(a,b)(a * b)), join(reduce(join(join(join(0.009999999776482582, join(reduce(join(rename(input, (d0, d1), (d0, d4)), constant(\"dnn_hidden1_weights_read\"), f(a,b)(a * b)), sum, d4), constant(\"dnn_hidden1_bias_read\"), f(a,b)(a + b)), f(a,b)(a * b)), join(reduce(join(rename(input, (d0, d1), (d0, d4)), constant(\"dnn_hidden1_weights_read\"), f(a,b)(a * b)), sum, d4), constant(\"dnn_hidden1_bias_read\"), f(a,b)(a + b)), f(a,b)(max(a,b))), constant(\"dnn_hidden2_weights_read\"), f(a,b)(a * b)), sum, d3), constant(\"dnn_hidden2_bias_read\"), f(a,b)(a + b)), f(a,b)(max(a,b))), constant(\"dnn_outputs_weights_read\"), f(a,b)(a * b)), sum, d2), constant(\"dnn_outputs_bias_read\"), f(a,b)(a + b))"; + final String expression = "join(join(reduce(join(join(join(constant(\"dnn_hidden2_Const\"), join(reduce(join(join(join(0.009999999776482582, join(reduce(join(reduce(rename(input, (d0, d1), (d0, d4)), sum, d0), constant(\"dnn_hidden1_weights_read\"), f(a,b)(a * b)), sum, d4), constant(\"dnn_hidden1_bias_read\"), f(a,b)(a + b)), f(a,b)(a * b)), join(reduce(join(reduce(rename(input, (d0, d1), (d0, d4)), sum, d0), constant(\"dnn_hidden1_weights_read\"), f(a,b)(a * b)), sum, d4), constant(\"dnn_hidden1_bias_read\"), f(a,b)(a + b)), f(a,b)(max(a,b))), constant(\"dnn_hidden2_weights_read\"), f(a,b)(a * b)), sum, d3), constant(\"dnn_hidden2_bias_read\"), f(a,b)(a + b)), f(a,b)(a * b)), join(reduce(join(join(join(0.009999999776482582, join(reduce(join(reduce(rename(input, (d0, d1), (d0, d4)), sum, d0), constant(\"dnn_hidden1_weights_read\"), f(a,b)(a * b)), sum, d4), constant(\"dnn_hidden1_bias_read\"), f(a,b)(a + b)), f(a,b)(a * b)), join(reduce(join(reduce(rename(input, (d0, d1), (d0, d4)), sum, d0), constant(\"dnn_hidden1_weights_read\"), f(a,b)(a * b)), sum, d4), constant(\"dnn_hidden1_bias_read\"), f(a,b)(a + b)), f(a,b)(max(a,b))), constant(\"dnn_hidden2_weights_read\"), f(a,b)(a * b)), sum, d3), constant(\"dnn_hidden2_bias_read\"), f(a,b)(a + b)), f(a,b)(max(a,b))), constant(\"dnn_outputs_weights_read\"), f(a,b)(a * b)), sum, d2), constant(\"dnn_outputs_bias_read\"), f(a,b)(a + b)), tensor(d0[1])(1.0), f(a,b)(a * b))"; StoringApplicationPackage application = new StoringApplicationPackage(applicationDir); - RankProfileSearchFixture search = fixtureWith("tensor(d0[2],d1[784])(0.0)", + RankProfileSearchFixture search = fixtureWith("tensor(d0[1],d1[784])(0.0)", "tensorflow('mnist/saved')", null, null, diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java index 37ddf458121..715b84c7093 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java @@ -12,6 +12,7 @@ import com.yahoo.config.model.provision.Host; import com.yahoo.config.model.provision.Hosts; import com.yahoo.config.model.provision.InMemoryProvisioner; import com.yahoo.config.model.provision.SingleNodeProvisioner; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Flavor; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.test.utils.ApplicationPackageUtils; @@ -43,6 +44,7 @@ public class VespaModelTester { private boolean hosted = true; private Map<String, Collection<Host>> hostsByFlavor = new HashMap<>(); + private ApplicationId applicationId = ApplicationId.defaultId(); public VespaModelTester() { this(new NullConfigModelRegistry()); @@ -84,6 +86,11 @@ public class VespaModelTester { /** Sets whether this sets up a model for a hosted system. Default: true */ public void setHosted(boolean hosted) { this.hosted = hosted; } + /** Sets the tenant, application name, and instance name of the model being built. */ + public void setApplicationId(String tenant, String applicationName, String instanceName) { + applicationId = ApplicationId.from(tenant, applicationName, instanceName); + } + /** Creates a model which uses 0 as start index and fails on out of capacity */ public VespaModel createModel(String services, String ... retiredHostNames) { return createModel(services, true, retiredHostNames); @@ -108,11 +115,16 @@ public class VespaModelTester { new InMemoryProvisioner(hostsByFlavor, failOnOutOfCapacity, startIndexForClusters, retiredHostNames) : new SingleNodeProvisioner(); + DeployProperties properties = new DeployProperties.Builder() + .hostedVespa(hosted) + .applicationId(applicationId) + .build(); + DeployState deployState = new DeployState.Builder() .applicationPackage(appPkg) .modelHostProvisioner(provisioner) - .properties((new DeployProperties.Builder()).hostedVespa(hosted).build()).build(true); + .properties(properties) + .build(true); return modelCreatorWithMockPkg.create(false, deployState, configModelRegistry); } - } diff --git a/configgen/src/main/scala/com/yahoo/config/codegen/ConfigGenerator.scala b/configgen/src/main/scala/com/yahoo/config/codegen/ConfigGenerator.scala index 90e23f24cf3..38306a03575 100644 --- a/configgen/src/main/scala/com/yahoo/config/codegen/ConfigGenerator.scala +++ b/configgen/src/main/scala/com/yahoo/config/codegen/ConfigGenerator.scala @@ -179,36 +179,8 @@ object ConfigGenerator { } } - // TODO: The default ctor can be removed if the config library uses builders to set values from payload, but ... - // a default ctor is also needed for all innerArrays, because of InnerNodeVector.createNew() - def defaultConstructor = { - // TODO @link gives javadoc warnings, although the syntax seems to be valid - //def link = "{@link " + {nodeClass(inner)} + "#" + {nodeClass(inner)} + "(Builder)}" - def link = {nodeClass(inner)} + "(Builder)" - - def ctor = - <code> - |/** - | * @deprecated Not for public use. - | * Does not check for uninitialized fields. - | * Replaced by {link} - | */ - |@Deprecated - |private {nodeClass(inner)}() {{ - | this(new Builder(), false); - |}} - </code>.text.stripMargin.trim - - inner match { - case array: InnerCNode if inner.isArray => ctor - case _ => "" - } - } - // TODO: merge these two constructors into one when the config library uses builders to set values from payload. <code> - |{defaultConstructor} - | |public {nodeClass(inner)}(Builder builder) {{ | this(builder, true); |}} @@ -319,7 +291,7 @@ object ConfigGenerator { | for (Builder b : builders) { | elems.add(new %s(b)); | } - | return new InnerNodeVector<%s>(elems, new %s()); + | return new InnerNodeVector<%s>(elems); |} """.stripMargin.format(List.fill(5)(nodeClass(inner)): _*).trim } diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/component/Deconstructor.java b/container-disc/src/main/java/com/yahoo/container/jdisc/component/Deconstructor.java index a986fbc794b..f13c078c41d 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/component/Deconstructor.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/component/Deconstructor.java @@ -6,7 +6,9 @@ import com.yahoo.concurrent.ThreadFactoryFactory; import com.yahoo.container.di.ComponentDeconstructor; import com.yahoo.container.di.componentgraph.Provider; import com.yahoo.jdisc.SharedResource; +import com.yahoo.log.LogLevel; +import java.security.SecureRandom; import java.util.Random; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -20,6 +22,7 @@ import static java.util.logging.Level.WARNING; * @author gv */ public class Deconstructor implements ComponentDeconstructor { + private static final Logger log = Logger.getLogger(Deconstructor.class.getName()); private final ScheduledExecutorService executor = @@ -52,27 +55,44 @@ public class Deconstructor implements ComponentDeconstructor { } private static class DestructComponentTask implements Runnable { + private final AbstractComponent component; DestructComponentTask(AbstractComponent component) { this.component = component; } + /** Returns a random value which will be different across identical containers invoking this at the same time */ + private long random() { + return new SecureRandom().nextLong(); + } + public void run() { log.info("Starting deconstruction of " + component); try { component.deconstruct(); log.info("Finished deconstructing " + component); - } catch (Error e) { - try { - Thread.sleep((long) (new Random(System.nanoTime()).nextDouble() * 180 * 1000)); - } catch (InterruptedException exception) { } - com.yahoo.protect.Process.logAndDie("Error when deconstructing " + component, e); - } catch (Exception e) { + } + catch (Exception | NoClassDefFoundError e) { // May get class not found due to it being already unloaded log.log(WARNING, "Exception thrown when deconstructing " + component, e); - } catch (Throwable t) { - log.log(WARNING, "Unexpected Throwable thrown when deconstructing " + component, t); + } + catch (Error e) { + try { + // Randomize restart over 10 minutes to avoid simultaneous cluster restarts + long randomSleepSeconds = random() * 60 * 10; + log.log(LogLevel.FATAL, "Error when deconstructing " + component + ". Will sleep for " + + randomSleepSeconds + " seconds then restart", e); + Thread.sleep(randomSleepSeconds * 1000); + } + catch (InterruptedException exception) { + log.log(WARNING, "Randomized wait before dying disrupted. Dying now."); + } + com.yahoo.protect.Process.logAndDie("Shutting down due to error when deconstructing " + component); + } + catch (Throwable e) { + log.log(WARNING, "Non-error not exception throwable thrown when deconstructing " + component, e); } } } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentJobExecutor.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentJobExecutor.java index fde5b311cd9..b781b231e48 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentJobExecutor.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentJobExecutor.java @@ -42,6 +42,7 @@ public class DeploymentJobExecutor extends Maintainer { protected void maintain() { controller().applications().deploymentTrigger().deploymentQueue().takeJobsToRun() .forEach(buildJob -> executor.execute(() -> { + log.log(Level.INFO, "Attempting to trigger " + buildJob + " in Screwdriver."); for (int i = 0; i < triggeringRetries; i++) if (buildService.trigger(buildJob)) return; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index d3edd1c0ca5..368fd323fb0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -89,6 +89,7 @@ import java.io.InputStream; import java.net.URI; import java.net.URISyntaxException; import java.security.Principal; +import java.time.DayOfWeek; import java.time.Duration; import java.util.Collections; import java.util.List; @@ -375,6 +376,19 @@ public class ApplicationApiHandler extends LoggingRequestHandler { job.lastSuccess().ifPresent(jobRun -> toSlime(jobRun, jobObject.setObject("lastSuccess"))); } + // Change blockers + Cursor changeBlockers = object.setArray("changeBlockers"); + application.deploymentSpec().changeBlocker().forEach(changeBlocker -> { + Cursor changeBlockerObject = changeBlockers.addObject(); + changeBlockerObject.setBool("versions", changeBlocker.blocksVersions()); + changeBlockerObject.setBool("revisions", changeBlocker.blocksRevisions()); + changeBlockerObject.setString("timeZone", changeBlocker.window().zone().getId()); + Cursor days = changeBlockerObject.setArray("days"); + changeBlocker.window().days().stream().map(DayOfWeek::getValue).forEach(days::addLong); + Cursor hours = changeBlockerObject.setArray("hours"); + changeBlocker.window().hours().forEach(hours::addLong); + }); + // Compile version. The version that should be used when building an application object.setString("compileVersion", application.oldestDeployedVersion().orElse(controller.systemVersion()).toFullString()); 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 be987e84cd8..c3dc80c65df 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 @@ -14,7 +14,6 @@ import com.yahoo.jdisc.http.filter.SecurityRequestFilterChain; import com.yahoo.slime.ArrayTraverser; import com.yahoo.slime.Inspector; import com.yahoo.slime.Slime; -import com.yahoo.slime.Type; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; @@ -152,12 +151,19 @@ public class ContainerTester { if ( ! fieldsToCensor.contains(fieldName)) return; String fromString; - if ( fieldValue.type().equals(Type.STRING)) - fromString = "\"" + fieldName + "\":\"" + fieldValue.asString() + "\""; - else if ( fieldValue.type().equals(Type.LONG)) - fromString = "\"" + fieldName + "\":" + fieldValue.asLong(); - else - throw new IllegalArgumentException("Can only censor strings and longs"); + switch (fieldValue.type()) { + case STRING: + fromString = "\"" + fieldName + "\":\"" + fieldValue.asString() + "\""; + break; + case LONG: + fromString = "\"" + fieldName + "\":" + fieldValue.asLong(); + break; + case BOOL: + fromString = "\"" + fieldName + "\":" + fieldValue.asBool(); + break; + default: + throw new IllegalArgumentException("Can only censor strings, longs and booleans"); + } String toString = "\"" + fieldName + "\":\"(ignore)\""; replaceStrings.add(new Pair<>(fromString, toString)); } 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 4a263bf1fbd..4b59e57fd5a 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 @@ -87,6 +87,7 @@ public class ApplicationApiTest extends ControllerContainerTest { .region("corp-us-east-1") .region("us-east-3") .region("us-west-1") + .blockChange(false, true, "mon-fri", "0-8", "UTC") .build(); private static final AthenzDomain ATHENZ_TENANT_DOMAIN = new AthenzDomain("domain1"); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json index d3b765551f0..9cd9329e36f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json @@ -219,6 +219,7 @@ } } ], + "changeBlockers": [], "compileVersion": "(ignore)", "globalRotations": [ "http://application1.tenant1.global.vespa.yahooapis.com:4080/", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json index 155166dea4c..29d23034053 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json @@ -183,6 +183,31 @@ } } ], + "changeBlockers": [ + { + "versions": true, + "revisions": false, + "timeZone": "UTC", + "days": [ + 1, + 2, + 3, + 4, + 5 + ], + "hours": [ + 0, + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8 + ] + } + ], "compileVersion": "(ignore)", "globalRotations": [ "http://application1.tenant1.global.vespa.yahooapis.com:4080/", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json index 35124adbf68..a1414fa3511 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json @@ -183,6 +183,31 @@ } } ], + "changeBlockers": [ + { + "versions": true, + "revisions": false, + "timeZone": "UTC", + "days": [ + 1, + 2, + 3, + 4, + 5 + ], + "hours": [ + 0, + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8 + ] + } + ], "compileVersion": "(ignore)", "globalRotations": [ "http://application1.tenant1.global.vespa.yahooapis.com:4080/", diff --git a/eval/CMakeLists.txt b/eval/CMakeLists.txt index 38abfec3c8c..ba42ddecef9 100644 --- a/eval/CMakeLists.txt +++ b/eval/CMakeLists.txt @@ -26,10 +26,10 @@ vespa_define_module( src/tests/gp/ponder_nov2017 src/tests/tensor/dense_dot_product_function src/tests/tensor/dense_fast_rename_function + src/tests/tensor/dense_inplace_map_function src/tests/tensor/dense_tensor_address_combiner src/tests/tensor/dense_tensor_builder src/tests/tensor/dense_xw_product_function - src/tests/tensor/vector_from_doubles_function src/tests/tensor/sparse_tensor_builder src/tests/tensor/tensor_address src/tests/tensor/tensor_conformance @@ -37,6 +37,7 @@ vespa_define_module( src/tests/tensor/tensor_performance src/tests/tensor/tensor_serialization src/tests/tensor/tensor_slime_serialization + src/tests/tensor/vector_from_doubles_function LIBS src/vespa/eval diff --git a/eval/src/tests/tensor/dense_inplace_map_function/CMakeLists.txt b/eval/src/tests/tensor/dense_inplace_map_function/CMakeLists.txt new file mode 100644 index 00000000000..3bd1dbaf271 --- /dev/null +++ b/eval/src/tests/tensor/dense_inplace_map_function/CMakeLists.txt @@ -0,0 +1,8 @@ +# Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(eval_dense_inplace_map_function_test_app TEST + SOURCES + dense_inplace_map_function_test.cpp + DEPENDS + vespaeval +) +vespa_add_test(NAME eval_dense_inplace_map_function_test_app COMMAND eval_dense_inplace_map_function_test_app) 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 new file mode 100644 index 00000000000..77af747a066 --- /dev/null +++ b/eval/src/tests/tensor/dense_inplace_map_function/dense_inplace_map_function_test.cpp @@ -0,0 +1,79 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/vespalib/testkit/test_kit.h> +#include <vespa/eval/eval/tensor_function.h> +#include <vespa/eval/eval/simple_tensor.h> +#include <vespa/eval/eval/simple_tensor_engine.h> +#include <vespa/eval/tensor/default_tensor_engine.h> +#include <vespa/eval/tensor/dense/dense_inplace_map_function.h> +#include <vespa/eval/tensor/dense/dense_tensor.h> +#include <vespa/eval/eval/test/tensor_model.hpp> +#include <vespa/eval/eval/test/eval_fixture.h> + +#include <vespa/vespalib/util/stringfmt.h> +#include <vespa/vespalib/util/stash.h> + +using namespace vespalib; +using namespace vespalib::eval; +using namespace vespalib::eval::test; +using namespace vespalib::tensor; +using namespace vespalib::eval::tensor_function; + +const TensorEngine &prod_engine = DefaultTensorEngine::ref(); + +EvalFixture::ParamRepo make_params() { + return EvalFixture::ParamRepo() + .add("x5", spec({x(5)}, N())) + .add_mutable("_d", spec(5.0)) + .add_mutable("_x5", spec({x(5)}, N())) + .add_mutable("_x5y3", spec({x(5),y(3)}, N())) + .add_mutable("_x5_u", spec({x(5)}, N()), "tensor(x[])") + .add_mutable("_x_m", spec({x({"a", "b", "c"})}, N())); +} +EvalFixture::ParamRepo param_repo = make_params(); + +void verify_optimized(const vespalib::string &expr, size_t cnt) { + EvalFixture fixture(prod_engine, expr, param_repo, true, true); + EXPECT_EQUAL(fixture.result(), EvalFixture::ref(expr, param_repo)); + EXPECT_EQUAL(fixture.get_param(0), fixture.result()); + auto info = fixture.find_all<DenseInplaceMapFunction>(); + ASSERT_EQUAL(info.size(), cnt); + for (size_t i = 0; i < cnt; ++i) { + EXPECT_TRUE(info[i]->result_is_mutable()); + } +} + +void verify_not_optimized(const vespalib::string &expr) { + EvalFixture fixture(prod_engine, expr, param_repo, true, true); + EXPECT_EQUAL(fixture.result(), EvalFixture::ref(expr, param_repo)); + EXPECT_NOT_EQUAL(fixture.get_param(0), fixture.result()); + auto info = fixture.find_all<DenseInplaceMapFunction>(); + EXPECT_TRUE(info.empty()); +} + +TEST("require that mutable dense concrete tensors are optimized") { + TEST_DO(verify_optimized("map(_x5,f(x)(x+10))", 1)); + TEST_DO(verify_optimized("map(_x5y3,f(x)(x+10))", 1)); +} + +TEST("require that inplace map operations can be chained") { + TEST_DO(verify_optimized("map(map(_x5,f(x)(x+10)),f(x)(x-5))", 2)); +} + +TEST("require that abstract tensors are not optimized") { + TEST_DO(verify_not_optimized("map(_x5_u,f(x)(x+10))")); +} + +TEST("require that non-mutable tensors are not optimized") { + TEST_DO(verify_not_optimized("map(x5,f(x)(x+10))")); +} + +TEST("require that scalar values are not optimized") { + TEST_DO(verify_not_optimized("map(_d,f(x)(x+10))")); +} + +TEST("require that mapped tensors are not optimized") { + TEST_DO(verify_not_optimized("map(_x_m,f(x)(x+10))")); +} + +TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/eval/src/vespa/eval/eval/tensor_function.h b/eval/src/vespa/eval/eval/tensor_function.h index 442e082d9d9..2c3e9d21e0b 100644 --- a/eval/src/vespa/eval/eval/tensor_function.h +++ b/eval/src/vespa/eval/eval/tensor_function.h @@ -26,8 +26,8 @@ class Tensor; /** * Interface used to describe a tensor function as a tree of nodes - * with information about operation sequencing and intermediate result - * types. Each node in the tree describes a single tensor + * with information about operation sequencing and intermediate + * results. Each node in the tree describes a single tensor * operation. This is the intermediate representation of a tensor * function. * @@ -38,10 +38,14 @@ class Tensor; * * The generic tree will then be optimized (in-place, bottom-up) where * sub-expressions may be replaced with optimized - * implementation-specific alternatives. + * implementation-specific alternatives. Note that multiple nodes in + * the original representation can be replaced with a single + * specialized node in the optimized tree. * * This leaves us with a mixed-mode tree with some generic and some - * specialized nodes, that may be evaluated recursively. + * specialized nodes. This tree will then be compiled into a sequence + * of instructions (each node will map to a single instruction) and + * evaluated in the context of an interpreted function. **/ struct TensorFunction { @@ -65,6 +69,7 @@ struct TensorFunction void set(const TensorFunction &child) const { ptr = &child; } }; virtual const ValueType &result_type() const = 0; + virtual bool result_is_mutable() const { return false; } /** * Push references to all children (NB: implementation must use diff --git a/eval/src/vespa/eval/eval/test/eval_fixture.cpp b/eval/src/vespa/eval/eval/test/eval_fixture.cpp index b7e3764f6ce..2593b74e300 100644 --- a/eval/src/vespa/eval/eval/test/eval_fixture.cpp +++ b/eval/src/vespa/eval/eval/test/eval_fixture.cpp @@ -20,11 +20,44 @@ NodeTypes get_types(const Function &function, const ParamRepo ¶m_repo) { return NodeTypes(function, param_types); } -const TensorFunction &make_tfun(bool optimized, const TensorEngine &engine, const Function &function, - const NodeTypes &node_types, Stash &stash) -{ - const TensorFunction &plain_fun = make_tensor_function(engine, function.root(), node_types, stash); - return optimized ? engine.optimize(plain_fun, stash) : plain_fun; +std::set<size_t> get_mutable(const Function &function, const ParamRepo ¶m_repo) { + std::set<size_t> mutable_set; + for (size_t i = 0; i < function.num_params(); ++i) { + auto pos = param_repo.map.find(function.param_name(i)); + ASSERT_TRUE(pos != param_repo.map.end()); + if (pos->second.is_mutable) { + mutable_set.insert(i); + } + } + return mutable_set; +} + +struct MyMutableInject : public tensor_function::Inject { + MyMutableInject(const ValueType &result_type_in, size_t param_idx_in) + : Inject(result_type_in, param_idx_in) {} + bool result_is_mutable() const override { return true; } +}; + +const TensorFunction &maybe_patch(bool allow_mutable, const TensorFunction &plain_fun, const std::set<size_t> &mutable_set, Stash &stash) { + using Child = TensorFunction::Child; + if (!allow_mutable) { + return plain_fun; + } + Child root(plain_fun); + std::vector<Child::CREF> nodes({root}); + for (size_t i = 0; i < nodes.size(); ++i) { + nodes[i].get().get().push_children(nodes); + } + while (!nodes.empty()) { + const Child &child = nodes.back(); + if (auto inject = as<tensor_function::Inject>(child.get())) { + if (mutable_set.count(inject->param_idx()) > 0) { + child.set(stash.create<MyMutableInject>(inject->result_type(), inject->param_idx())); + } + } + nodes.pop_back(); + } + return root.get(); } std::vector<Value::UP> make_params(const TensorEngine &engine, const Function &function, @@ -53,12 +86,16 @@ std::vector<Value::CREF> get_refs(const std::vector<Value::UP> &values) { EvalFixture::EvalFixture(const TensorEngine &engine, const vespalib::string &expr, const ParamRepo ¶m_repo, - bool optimized) + bool optimized, + bool allow_mutable) : _engine(engine), _stash(), _function(Function::parse(expr)), _node_types(get_types(_function, param_repo)), - _tensor_function(make_tfun(optimized, _engine, _function, _node_types, _stash)), + _mutable_set(get_mutable(_function, param_repo)), + _plain_tensor_function(make_tensor_function(_engine, _function.root(), _node_types, _stash)), + _patched_tensor_function(maybe_patch(allow_mutable, _plain_tensor_function, _mutable_set, _stash)), + _tensor_function(optimized ? _engine.optimize(_patched_tensor_function, _stash) : _patched_tensor_function), _ifun(_engine, _tensor_function), _ictx(_ifun), _param_values(make_params(_engine, _function, param_repo)), @@ -69,4 +106,11 @@ EvalFixture::EvalFixture(const TensorEngine &engine, ASSERT_TRUE(!result_type.is_error()); } +const TensorSpec +EvalFixture::get_param(size_t idx) const +{ + ASSERT_LESS(idx, _param_values.size()); + return _engine.to_spec(*(_param_values[idx])); +} + } // namespace vespalib::eval::test diff --git a/eval/src/vespa/eval/eval/test/eval_fixture.h b/eval/src/vespa/eval/eval/test/eval_fixture.h index 1f864e980cc..7d3b95ad144 100644 --- a/eval/src/vespa/eval/eval/test/eval_fixture.h +++ b/eval/src/vespa/eval/eval/test/eval_fixture.h @@ -9,6 +9,7 @@ #include <vespa/eval/eval/simple_tensor_engine.h> #include <vespa/eval/tensor/default_tensor_engine.h> #include <vespa/vespalib/util/stash.h> +#include <set> namespace vespalib::eval::test { @@ -18,23 +19,30 @@ public: struct Param { TensorSpec value; // actual parameter value vespalib::string type; // pre-defined type (could be abstract) - Param(TensorSpec value_in) - : value(std::move(value_in)), type(value.type()) {} - Param(TensorSpec value_in, const vespalib::string &type_in) - : value(std::move(value_in)), type(type_in) {} + bool is_mutable; // input will be mutable (if allow_mutable is true) + Param(TensorSpec value_in, const vespalib::string &type_in, bool is_mutable_in) + : value(std::move(value_in)), type(type_in), is_mutable(is_mutable_in) {} ~Param() {} }; struct ParamRepo { std::map<vespalib::string,Param> map; ParamRepo() : map() {} - ParamRepo &add(const vespalib::string &name, TensorSpec value_in) { - map.insert_or_assign(name, Param(std::move(value_in))); + ParamRepo &add(const vespalib::string &name, TensorSpec value_in, const vespalib::string &type_in, bool is_mutable_in) { + map.insert_or_assign(name, Param(std::move(value_in), type_in, is_mutable_in)); return *this; } - ParamRepo &add(const vespalib::string &name, TensorSpec value_in, const vespalib::string &type_in) { - map.insert_or_assign(name, Param(std::move(value_in), type_in)); - return *this; + ParamRepo &add(const vespalib::string &name, TensorSpec value, const vespalib::string &type) { + return add(name, value, type, false); + } + ParamRepo &add_mutable(const vespalib::string &name, TensorSpec value, const vespalib::string &type) { + return add(name, value, type, true); + } + ParamRepo &add(const vespalib::string &name, const TensorSpec &value) { + return add(name, value, value.type(), false); + } + ParamRepo &add_mutable(const vespalib::string &name, const TensorSpec &value) { + return add(name, value, value.type(), true); } ~ParamRepo() {} }; @@ -44,6 +52,9 @@ private: Stash _stash; Function _function; NodeTypes _node_types; + std::set<size_t> _mutable_set; + const TensorFunction &_plain_tensor_function; + const TensorFunction &_patched_tensor_function; const TensorFunction &_tensor_function; InterpretedFunction _ifun; InterpretedFunction::Context _ictx; @@ -64,7 +75,8 @@ private: } public: - EvalFixture(const TensorEngine &engine, const vespalib::string &expr, const ParamRepo ¶m_repo, bool optimized); + EvalFixture(const TensorEngine &engine, const vespalib::string &expr, const ParamRepo ¶m_repo, + bool optimized = true, bool allow_mutable = false); ~EvalFixture() {} template <typename T> std::vector<const T *> find_all() { @@ -73,11 +85,12 @@ public: return list; } const TensorSpec &result() const { return _result; } + const TensorSpec get_param(size_t idx) const; static TensorSpec ref(const vespalib::string &expr, const ParamRepo ¶m_repo) { - return EvalFixture(SimpleTensorEngine::ref(), expr, param_repo, false).result(); + return EvalFixture(SimpleTensorEngine::ref(), expr, param_repo, false, false).result(); } static TensorSpec prod(const vespalib::string &expr, const ParamRepo ¶m_repo) { - return EvalFixture(tensor::DefaultTensorEngine::ref(), expr, param_repo, true).result(); + return EvalFixture(tensor::DefaultTensorEngine::ref(), expr, param_repo, true, false).result(); } }; diff --git a/eval/src/vespa/eval/tensor/default_tensor_engine.cpp b/eval/src/vespa/eval/tensor/default_tensor_engine.cpp index 7b4a502dd4d..dead8ee4870 100644 --- a/eval/src/vespa/eval/tensor/default_tensor_engine.cpp +++ b/eval/src/vespa/eval/tensor/default_tensor_engine.cpp @@ -10,6 +10,7 @@ #include "dense/dense_dot_product_function.h" #include "dense/dense_xw_product_function.h" #include "dense/dense_fast_rename_function.h" +#include "dense/dense_inplace_map_function.h" #include "dense/vector_from_doubles_function.h" #include <vespa/eval/eval/value.h> #include <vespa/eval/eval/tensor_spec.h> @@ -223,6 +224,7 @@ DefaultTensorEngine::optimize(const TensorFunction &expr, Stash &stash) const child.set(DenseDotProductFunction::optimize(child.get(), stash)); child.set(DenseXWProductFunction::optimize(child.get(), stash)); child.set(DenseFastRenameFunction::optimize(child.get(), stash)); + child.set(DenseInplaceMapFunction::optimize(child.get(), stash)); nodes.pop_back(); } return root.get(); diff --git a/eval/src/vespa/eval/tensor/dense/CMakeLists.txt b/eval/src/vespa/eval/tensor/dense/CMakeLists.txt index 73315b7a120..cf6d3a3431c 100644 --- a/eval/src/vespa/eval/tensor/dense/CMakeLists.txt +++ b/eval/src/vespa/eval/tensor/dense/CMakeLists.txt @@ -3,6 +3,7 @@ vespa_add_library(eval_tensor_dense OBJECT SOURCES dense_dot_product_function.cpp dense_fast_rename_function.cpp + dense_inplace_map_function.cpp dense_tensor.cpp dense_tensor_address_combiner.cpp dense_tensor_builder.cpp 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 new file mode 100644 index 00000000000..162bdb2ebfe --- /dev/null +++ b/eval/src/vespa/eval/tensor/dense/dense_inplace_map_function.cpp @@ -0,0 +1,67 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "dense_inplace_map_function.h" +#include "dense_tensor.h" +#include "dense_tensor_view.h" +#include <vespa/eval/eval/value.h> +#include <vespa/eval/tensor/tensor.h> + +namespace vespalib::tensor { + +using CellsRef = DenseTensorView::CellsRef; +using eval::Value; +using eval::ValueType; +using eval::TensorFunction; +using eval::as; +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()); +} + +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))) { + cell = function(cell); + } +} + +bool isConcreteDenseTensor(const ValueType &type) { + return (type.is_dense() && !type.is_abstract()); +} + +} // namespace vespalib::tensor::<unnamed> + +DenseInplaceMapFunction::DenseInplaceMapFunction(const eval::ValueType &result_type, + const eval::TensorFunction &child, + map_fun_t function_in) + : eval::tensor_function::Op1(result_type, child), + _function(function_in) +{ +} + +DenseInplaceMapFunction::~DenseInplaceMapFunction() +{ +} + +eval::InterpretedFunction::Instruction +DenseInplaceMapFunction::compile_self(Stash &) const +{ + return eval::InterpretedFunction::Instruction(my_inplace_map_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())) { + return stash.create<DenseInplaceMapFunction>(map->result_type(), map->child(), map->function()); + } + } + return expr; +} + +} // namespace vespalib::tensor diff --git a/eval/src/vespa/eval/tensor/dense/dense_inplace_map_function.h b/eval/src/vespa/eval/tensor/dense/dense_inplace_map_function.h new file mode 100644 index 00000000000..f02f83edae1 --- /dev/null +++ b/eval/src/vespa/eval/tensor/dense/dense_inplace_map_function.h @@ -0,0 +1,29 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <vespa/eval/eval/tensor_function.h> + +namespace vespalib::tensor { + +/** + * Tensor function for inplace map operation on mutable dense tensors. + **/ +class DenseInplaceMapFunction : public eval::tensor_function::Op1 +{ +public: + using map_fun_t = ::vespalib::eval::tensor_function::map_fun_t; +private: + map_fun_t _function; +public: + DenseInplaceMapFunction(const eval::ValueType &result_type, + const eval::TensorFunction &child, + map_fun_t function_in); + ~DenseInplaceMapFunction(); + map_fun_t function() const { return _function; } + bool result_is_mutable() const override { return true; } + eval::InterpretedFunction::Instruction compile_self(Stash &stash) const override; + static const eval::TensorFunction &optimize(const eval::TensorFunction &expr, Stash &stash); +}; + +} // namespace vespalib::tensor diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/containerdata/ConfigServerContainerData.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/containerdata/ConfigServerContainerData.java new file mode 100644 index 00000000000..2ddfebea36f --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/containerdata/ConfigServerContainerData.java @@ -0,0 +1,61 @@ +package com.yahoo.vespa.hosted.node.admin.containerdata; + +import com.yahoo.vespa.hosted.dockerapi.ContainerName; +import com.yahoo.vespa.hosted.node.admin.component.Environment; + +import java.nio.file.Path; +import java.nio.file.Paths; + +public class ConfigServerContainerData { + + public static final Path configServerAppDir = Paths.get("home/y/conf/configserver-app/"); + + private final Environment environment; + private final String configServerNodeHostName; + + public ConfigServerContainerData(Environment environment, String configServerNodeHostName) { + this.environment = environment; + this.configServerNodeHostName = configServerNodeHostName; + } + + public void create() { + ContainerData containerData = ContainerData.createCleanContainerData( + environment, ContainerName.fromHostname(configServerNodeHostName)); + containerData.addFile(getPath("configserver-config.xml"), createConfigServerConfigXml()); + containerData.addFile(getPath("node-repository-config.xml"), createNodeRepoConfigXml()); + } + + private Path getPath(String fileName) { + return configServerAppDir.resolve(fileName); + } + + private String createConfigServerConfigXml() { + return "<config name=\"cloud.config.configserver\">\n" + + " <system>" + environment.getSystem() + "</system>\n" + + " <environment>" + environment.getEnvironment() + "</environment>\n" + + " <region>" + environment.getRegion() + "</region>\n" + + " <hostedVespa>true</hostedVespa>\n" + + " <defaultFlavor>t2.xlarge</defaultFlavor>\n" + // TODO: Avoid hardcoding + createZookeeperServers() + + " <serverId>" + configServerNodeHostName + "</serverId>\n" + + " <nodeAdminInContainer>false</nodeAdminInContainer>\n" + + "</config>\n"; + } + + private String createZookeeperServers() { + StringBuilder zookeeperServers = new StringBuilder(" <zookeeperserver>\n"); + environment.getConfigServerHostNames().forEach(hostname -> zookeeperServers.append(" <item>\n" + + " <hostname>" + hostname + "</hostname>\n" + + " </item>\n")); + zookeeperServers.append(" </zookeeperserver>\n"); + return zookeeperServers.toString(); + } + + // TODO: Avoid hardcoded Docker registry + private String createNodeRepoConfigXml() { + return "<config name=\"config.provisioning.node-repository\">\n" + + " <dockerImage>658543512185.dkr.ecr.us-east-2.amazonaws.com:4443/vespa/aws</dockerImage>\n" + + "</config>\n"; + } + +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/containerdata/ContainerData.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/containerdata/ContainerData.java new file mode 100644 index 00000000000..ebfb433de4a --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/containerdata/ContainerData.java @@ -0,0 +1,94 @@ +package com.yahoo.vespa.hosted.node.admin.containerdata; + +import com.yahoo.io.IOUtils; +import com.yahoo.log.LogLevel; +import com.yahoo.text.Utf8; +import com.yahoo.vespa.hosted.dockerapi.ContainerName; +import com.yahoo.vespa.hosted.node.admin.component.Environment; +import com.yahoo.vespa.hosted.node.admin.task.util.file.IOExceptionUtil; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.NoSuchFileException; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Collections; +import java.util.List; +import java.util.logging.Logger; +import java.util.stream.Collectors; + +/** + * This class can be used for adding files that should be accessible in a container's file system after + * the container has been started. + * <p> + * Files that are added will be copied into the destination path on the host. + * The entry point for a Docker image will take care of copying + * everything into it's right place (ATM only done for config server nodes) + * Note: Creating a new instance of this will cleanup all old data in the destination path + */ +public class ContainerData { + + private static final Logger log = Logger.getLogger(ContainerData.class.getName()); + public static final Path containerDataPath = Paths.get("/home/y/var/container-data"); + + private final Path destinationPathOnHost; + + private ContainerData(Environment environment, ContainerName containerName) { + this.destinationPathOnHost = environment.pathInHostFromPathInNode(containerName, ContainerData.containerDataPath); + } + + public static ContainerData createCleanContainerData(Environment environment, ContainerName containerName) { + ContainerData containerData = new ContainerData(environment, containerName); + IOExceptionUtil.uncheck(containerData::cleanup); + return containerData; + } + + + public void addFile(Path relativePathInContainer, String data) { + if (relativePathInContainer.isAbsolute()) + throw new IllegalArgumentException("Path must be relative to root: " + relativePathInContainer); + + Path path = destinationPathOnHost.resolve(relativePathInContainer); + if (!path.toFile().exists()) { + IOExceptionUtil.uncheck(() -> Files.createDirectories(path.getParent())); + } + IOUtils.writeFile(path.toFile(), Utf8.toBytes(data)); + } + + private void cleanup() throws IOException { + log.log(LogLevel.INFO, "Cleaning up " + destinationPathOnHost.toAbsolutePath()); + recursiveDelete(destinationPathOnHost); + } + + + /* The below is copied from FileHelper in node-maintainer. Use methods in that class + instead when we start depending on node-maintainer + */ + + /** + * Similar to rm -rf file: + * - It's not an error if file doesn't exist + * - If file is a directory, it and all content is removed + * - For symlinks: Only the symlink is removed, not what the symlink points to + */ + private static void recursiveDelete(Path basePath) throws IOException { + if (Files.isDirectory(basePath)) { + for (Path path : listContentsOfDirectory(basePath)) { + recursiveDelete(path); + } + } + + Files.deleteIfExists(basePath); + } + + private static List<Path> listContentsOfDirectory(Path basePath) { + try { + return Files.list(basePath).collect(Collectors.toList()); + } catch (NoSuchFileException ignored) { + return Collections.emptyList(); + } catch (IOException e) { + throw new RuntimeException("Failed to list contents of directory " + basePath.toAbsolutePath(), e); + } + } + +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index 54871e773bd..0dc03b13fc7 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.node.admin.nodeagent; import com.fasterxml.jackson.core.JsonProcessingException; import com.yahoo.concurrent.ThreadFactoryFactory; +import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.dockerapi.Container; import com.yahoo.vespa.hosted.dockerapi.ContainerName; import com.yahoo.vespa.hosted.dockerapi.ContainerResources; @@ -17,6 +18,7 @@ import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer; +import com.yahoo.vespa.hosted.node.admin.containerdata.ConfigServerContainerData; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.OrchestratorException; @@ -256,6 +258,7 @@ public class NodeAgentImpl implements NodeAgent { } private void startContainer(ContainerNodeSpec nodeSpec) { + createContainerData(nodeSpec); dockerOperations.createContainer(containerName, nodeSpec); dockerOperations.startContainer(containerName, nodeSpec); aclMaintainer.run(); @@ -677,4 +680,11 @@ public class NodeAgentImpl implements NodeAgent { logger.info("Ask Orchestrator for permission to suspend node " + hostname); orchestrator.suspend(hostname); } + + private void createContainerData(ContainerNodeSpec nodeSpec) { + if (nodeSpec.nodeType.equals(NodeType.config.name())) { + logger.info("Creating files needed by config server"); + new ConfigServerContainerData(environment, nodeSpec.hostname).create(); + } + } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java index 1aa3cc2c082..8c6657c147a 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java @@ -12,6 +12,8 @@ import com.yahoo.vespa.hosted.dockerapi.Docker; import com.yahoo.vespa.hosted.dockerapi.DockerImage; import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec; +import com.yahoo.vespa.hosted.node.admin.containerdata.ConfigServerContainerData; +import com.yahoo.vespa.hosted.node.admin.containerdata.ContainerData; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer; import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer; @@ -55,7 +57,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; /** - * @author bakksjo + * @author Øyvind Bakksjø */ public class NodeAgentImplTest { private static final Duration NODE_AGENT_SCAN_INTERVAL = Duration.ofSeconds(30); @@ -624,6 +626,59 @@ public class NodeAgentImplTest { assertEquals(Collections.emptySet(), actualMetrics); } + @Test + public void testRunningConfigServer() throws IOException { + final long rebootGeneration = 0; + final ContainerNodeSpec nodeSpec = nodeSpecBuilder + .nodeType("config") + .wantedDockerImage(dockerImage) + .nodeState(Node.State.active) + .wantedVespaVersion(vespaVersion) + .build(); + + System.out.println(nodeSpec); + + NodeAgentImpl nodeAgent = makeNodeAgent(null, false); + + when(nodeRepository.getContainerNodeSpec(hostName)).thenReturn(Optional.of(nodeSpec)); + Path tempDirectory = Files.createTempDirectory("foo"); + when(pathResolver.getApplicationStoragePathForHost()).thenReturn(tempDirectory); + when(dockerOperations.pullImageAsyncIfNeeded(eq(dockerImage))).thenReturn(false); + when(storageMaintainer.getDiskUsageFor(eq(containerName))).thenReturn(Optional.of(201326592000L)); + + nodeAgent.converge(); + + verify(dockerOperations, never()).removeContainer(any(), any()); + verify(orchestrator, never()).suspend(any(String.class)); + + final InOrder inOrder = inOrder(dockerOperations, orchestrator, nodeRepository, aclMaintainer); + inOrder.verify(dockerOperations, times(1)).pullImageAsyncIfNeeded(eq(dockerImage)); + inOrder.verify(dockerOperations, times(1)).createContainer(eq(containerName), eq(nodeSpec)); + inOrder.verify(dockerOperations, times(1)).startContainer(eq(containerName), eq(nodeSpec)); + inOrder.verify(aclMaintainer, times(1)).run(); + inOrder.verify(dockerOperations, times(1)).resumeNode(eq(containerName)); + inOrder.verify(nodeRepository).updateNodeAttributes( + hostName, new NodeAttributes() + .withRebootGeneration(rebootGeneration) + .withDockerImage(dockerImage) + .withVespaVersion(vespaVersion)); + inOrder.verify(orchestrator).resume(hostName); + + // Files written in createContainerData() + assertFileExists(containerName, tempDirectory, "node-repository-config.xml"); + assertFileExists(containerName, tempDirectory, "configserver-config.xml"); + } + + private void assertFileExists(ContainerName containerName, Path tempDirectory, String filename) { + File file = tempDirectory + .resolve(containerName.asString()) + .resolve(Paths.get("/").relativize(ContainerData.containerDataPath)) + .resolve(ConfigServerContainerData.configServerAppDir) + .resolve(filename) + .toAbsolutePath() + .toFile(); + assertTrue("File " + file + " does not exist", file.exists()); + } private NodeAgentImpl makeNodeAgent(DockerImage dockerImage, boolean isRunning) { Optional<Container> container = dockerImage != null ? diff --git a/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostApi.java b/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostApi.java index ad0f3e094eb..1c4d138acef 100644 --- a/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostApi.java +++ b/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostApi.java @@ -14,9 +14,7 @@ import javax.ws.rs.PUT; import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.Produces; -import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; -import javax.ws.rs.core.UriInfo; /** * Definition of Orchestrator's REST API for hosts. diff --git a/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostSuspensionApi.java b/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostSuspensionApi.java index a5ca15d2d15..a9846134eff 100644 --- a/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostSuspensionApi.java +++ b/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostSuspensionApi.java @@ -6,9 +6,16 @@ import com.yahoo.vespa.orchestrator.restapi.wire.BatchOperationResult; import javax.ws.rs.Consumes; import javax.ws.rs.PUT; +import javax.ws.rs.Path; +import javax.ws.rs.PathParam; import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; import javax.ws.rs.core.MediaType; +import java.util.List; +/** + * @author hakonhall + */ public interface HostSuspensionApi { /** * Path prefix for this api. Resources implementing this API should use this with a @Path annotation. @@ -27,5 +34,13 @@ public interface HostSuspensionApi { @PUT @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) + @Deprecated // TODO: Remove after 2018-04-01 BatchOperationResult suspendAll(BatchHostSuspendRequest request); + + @PUT + @Path("/{hostname}") + @Produces(MediaType.APPLICATION_JSON) + @Consumes(MediaType.APPLICATION_JSON) + BatchOperationResult suspendAll(@PathParam("hostname") String parentHostname, + @QueryParam("hostname") List<String> hostnames); } diff --git a/orchestrator/pom.xml b/orchestrator/pom.xml index ae22b6718f9..3559e4282c3 100644 --- a/orchestrator/pom.xml +++ b/orchestrator/pom.xml @@ -131,6 +131,16 @@ <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-compiler-plugin</artifactId> + <configuration> + <compilerArgs> + <arg>-Xlint:all</arg> + <arg>-Xlint:-deprecation</arg> + <arg>-Xlint:-serial</arg> + <arg>-Xlint:-varargs</arg> + <arg>-Xlint:-try</arg> + <arg>-Werror</arg> + </compilerArgs> + </configuration> </plugin> </plugins> </build> diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostSuspensionResource.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostSuspensionResource.java index 85166ecc741..c1e312ddee6 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostSuspensionResource.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostSuspensionResource.java @@ -21,6 +21,9 @@ import java.util.List; import java.util.logging.Logger; import java.util.stream.Collectors; +/** + * @author hakonhall + */ @Path(HostSuspensionApi.PATH_PREFIX) public class HostSuspensionResource implements HostSuspensionApi { @@ -47,26 +50,28 @@ public class HostSuspensionResource implements HostSuspensionApi { log.log(LogLevel.DEBUG, message); throw createWebApplicationException(message, Response.Status.BAD_REQUEST); } + return suspendAll(parentHostnameString, hostnamesAsStrings); + } + @Override + public BatchOperationResult suspendAll(String parentHostnameString, List<String> hostnamesAsStrings) { HostName parentHostname = new HostName(parentHostnameString); - List<HostName> hostNames = hostnamesAsStrings.stream().map(HostName::new).collect(Collectors.toList()); - + List<HostName> hostnames = hostnamesAsStrings.stream().map(HostName::new).collect(Collectors.toList()); try { - orchestrator.suspendAll(parentHostname, hostNames); + orchestrator.suspendAll(parentHostname, hostnames); } catch (BatchHostStateChangeDeniedException e) { - log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostNames + " with parent host " + parentHostname, e); + log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostnames + " with parent host " + parentHostname, e); throw createWebApplicationException(e.getMessage(), Response.Status.CONFLICT); } catch (BatchHostNameNotFoundException e) { - log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostNames + " with parent host " + parentHostname, e); + log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostnames + " with parent host " + parentHostname, e); // Note that we're returning BAD_REQUEST instead of NOT_FOUND because the resource identified // by the URL path was found. It's one of the hostnames in the request it failed to find. throw createWebApplicationException(e.getMessage(), Response.Status.BAD_REQUEST); } catch (BatchInternalErrorException e) { - log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostNames + " with parent host " + parentHostname, e); + log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostnames + " with parent host " + parentHostname, e); throw createWebApplicationException(e.getMessage(), Response.Status.INTERNAL_SERVER_ERROR); } - - log.log(LogLevel.DEBUG, "Suspended " + hostNames + " with parent " + parentHostname); + log.log(LogLevel.DEBUG, "Suspended " + hostnames + " with parent " + parentHostname); return BatchOperationResult.successResult(); } diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java index 65309440aee..2c7db25ae30 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java @@ -21,7 +21,6 @@ import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactoryMoc import com.yahoo.vespa.orchestrator.model.ApplicationApi; import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException; import com.yahoo.vespa.orchestrator.policy.Policy; -import com.yahoo.vespa.orchestrator.restapi.wire.BatchHostSuspendRequest; import com.yahoo.vespa.orchestrator.restapi.wire.BatchOperationResult; import com.yahoo.vespa.orchestrator.restapi.wire.GetHostResponse; import com.yahoo.vespa.orchestrator.restapi.wire.PatchHostRequest; @@ -56,7 +55,11 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +/** + * @author hakonhall + */ public class HostResourceTest { + private static final int SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS = 0; private static final TenantId TENANT_ID = new TenantId("tenantId"); private static final ApplicationInstanceId APPLICATION_INSTANCE_ID = new ApplicationInstanceId("applicationId"); @@ -110,20 +113,20 @@ public class HostResourceTest { public void grantSuspensionRequest( ApplicationInstance applicationInstance, HostName hostName, - MutableStatusRegistry hostStatusService) throws HostStateChangeDeniedException { + MutableStatusRegistry hostStatusService) { } @Override - public void grantSuspensionRequest(ApplicationApi applicationApi) throws HostStateChangeDeniedException { + public void grantSuspensionRequest(ApplicationApi applicationApi) { } @Override - public void releaseSuspensionGrant(ApplicationApi application) throws HostStateChangeDeniedException { + public void releaseSuspensionGrant(ApplicationApi application) { } @Override - public void acquirePermissionToRemove(ApplicationApi applicationApi) throws HostStateChangeDeniedException { + public void acquirePermissionToRemove(ApplicationApi applicationApi) { } @Override @@ -151,7 +154,7 @@ public class HostResourceTest { private final UriInfo uriInfo = mock(UriInfo.class); @Test - public void returns_200_on_success() throws Exception { + public void returns_200_on_success() { HostResource hostResource = new HostResource(alwaysAllowOrchestrator, uriInfo); @@ -163,25 +166,23 @@ public class HostResourceTest { } @Test - public void returns_200_on_success_batch() throws Exception { + public void returns_200_on_success_batch() { HostSuspensionResource hostSuspensionResource = new HostSuspensionResource(alwaysAllowOrchestrator); - BatchHostSuspendRequest request = - new BatchHostSuspendRequest("parentHostname", Arrays.asList("hostname1", "hostname2")); - BatchOperationResult response = hostSuspensionResource.suspendAll(request); + BatchOperationResult response = hostSuspensionResource.suspendAll("parentHostname", + Arrays.asList("hostname1", "hostname2")); assertThat(response.success()); } @Test - public void returns_200_empty_batch() throws Exception { + public void returns_200_empty_batch() { HostSuspensionResource hostSuspensionResource = new HostSuspensionResource(alwaysAllowOrchestrator); - BatchHostSuspendRequest request = - new BatchHostSuspendRequest("parentHostname", Collections.emptyList()); - BatchOperationResult response = hostSuspensionResource.suspendAll(request); + BatchOperationResult response = hostSuspensionResource.suspendAll("parentHostname", + Collections.emptyList());; assertThat(response.success()); } @Test - public void throws_404_when_host_unknown() throws Exception { + public void throws_404_when_host_unknown() { try { HostResource hostResource = new HostResource(hostNotFoundOrchestrator, uriInfo); @@ -196,12 +197,10 @@ public class HostResourceTest { // This is so because the hostname is part of the URL path for single-host, while the // hostnames are part of the request body for multi-host. @Test - public void throws_400_when_host_unknown_for_batch() throws Exception { + public void throws_400_when_host_unknown_for_batch() { try { HostSuspensionResource hostSuspensionResource = new HostSuspensionResource(hostNotFoundOrchestrator); - BatchHostSuspendRequest request = - new BatchHostSuspendRequest("parentHostname", Arrays.asList("hostname1", "hostname2")); - hostSuspensionResource.suspendAll(request); + hostSuspensionResource.suspendAll("parentHostname", Arrays.asList("hostname1", "hostname2")); fail(); } catch (WebApplicationException w) { assertThat(w.getResponse().getStatus()).isEqualTo(400); @@ -249,7 +248,7 @@ public class HostResourceTest { } @Test - public void throws_409_when_request_rejected_by_policies() throws Exception { + public void throws_409_when_request_rejected_by_policies() { final OrchestratorImpl alwaysRejectResolver = new OrchestratorImpl( new AlwaysFailPolicy(), new ClusterControllerClientFactoryMock(), @@ -266,7 +265,7 @@ public class HostResourceTest { } @Test - public void throws_409_when_request_rejected_by_policies_for_batch() throws Exception { + public void throws_409_when_request_rejected_by_policies_for_batch() { final OrchestratorImpl alwaysRejectResolver = new OrchestratorImpl( new AlwaysFailPolicy(), new ClusterControllerClientFactoryMock(), @@ -276,9 +275,7 @@ public class HostResourceTest { try { HostSuspensionResource hostSuspensionResource = new HostSuspensionResource(alwaysRejectResolver); - BatchHostSuspendRequest request = - new BatchHostSuspendRequest("parentHostname", Arrays.asList("hostname1", "hostname2")); - hostSuspensionResource.suspendAll(request); + hostSuspensionResource.suspendAll("parentHostname", Arrays.asList("hostname1", "hostname2")); fail(); } catch (WebApplicationException w) { assertThat(w.getResponse().getStatus()).isEqualTo(409); diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/AthenzCredentials.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/AthenzCredentials.java index c5dce1c5b1d..8127ac9feb3 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/AthenzCredentials.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/AthenzCredentials.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.athenz.identityprovider; import java.security.KeyPair; import java.security.cert.X509Certificate; -import java.time.Instant; /** * @author bjorncs @@ -14,18 +13,15 @@ class AthenzCredentials { private final X509Certificate certificate; private final KeyPair keyPair; private final SignedIdentityDocument identityDocument; - private final Instant createdAt; AthenzCredentials(String nToken, X509Certificate certificate, KeyPair keyPair, - SignedIdentityDocument identityDocument, - Instant createdAt) { + SignedIdentityDocument identityDocument) { this.nToken = nToken; this.certificate = certificate; this.keyPair = keyPair; this.identityDocument = identityDocument; - this.createdAt = createdAt; } String getNToken() { @@ -44,8 +40,5 @@ class AthenzCredentials { return identityDocument; } - Instant getCreatedAt() { - return createdAt; - } } diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/AthenzCredentialsService.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/AthenzCredentialsService.java index dd816929bfb..b9fb7e94782 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/AthenzCredentialsService.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/AthenzCredentialsService.java @@ -79,7 +79,7 @@ class AthenzCredentialsService { SignedIdentityDocument identityDocument) { X509Certificate certificate = instanceIdentity.getX509Certificate(); String serviceToken = instanceIdentity.getServiceToken(); - return new AthenzCredentials(serviceToken, certificate, keyPair, identityDocument, clock.instant()); + return new AthenzCredentials(serviceToken, certificate, keyPair, identityDocument); } private static SignedIdentityDocument parseSignedIdentityDocument(String rawDocument) { diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/AthenzIdentityProviderImpl.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/AthenzIdentityProviderImpl.java index 95113e1b0b1..78ad95f84f3 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/AthenzIdentityProviderImpl.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/AthenzIdentityProviderImpl.java @@ -19,6 +19,7 @@ import java.time.Duration; import java.time.Instant; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Logger; @@ -33,35 +34,19 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen // TODO Make some of these values configurable through config. Match requested expiration of register/update requests. // TODO These should match the requested expiration - static final Duration EXPIRES_AFTER = Duration.ofDays(1); - static final Duration EXPIRATION_MARGIN = Duration.ofMinutes(30); - static final Duration INITIAL_WAIT_NTOKEN = Duration.ofMinutes(5); - static final Duration UPDATE_PERIOD = EXPIRES_AFTER.dividedBy(3); - static final Duration REDUCED_UPDATE_PERIOD = Duration.ofMinutes(30); - static final Duration INITIAL_BACKOFF_DELAY = Duration.ofMinutes(4); - static final Duration MAX_REGISTER_BACKOFF_DELAY = Duration.ofHours(1); - static final int BACKOFF_DELAY_MULTIPLIER = 2; + static final Duration UPDATE_PERIOD = Duration.ofDays(1); static final Duration AWAIT_TERMINTATION_TIMEOUT = Duration.ofSeconds(90); - private static final Duration CERTIFICATE_EXPIRY_METRIC_UPDATE_PERIOD = Duration.ofMinutes(5); - private static final String CERTIFICATE_EXPIRY_METRIC_NAME = "athenz-tenant-cert.expiry.seconds"; - - static final String REGISTER_INSTANCE_TAG = "register-instance"; - static final String UPDATE_CREDENTIALS_TAG = "update-credentials"; - static final String TIMEOUT_INITIAL_WAIT_TAG = "timeout-initial-wait"; - static final String METRICS_UPDATER_TAG = "metrics-updater"; - + public static final String CERTIFICATE_EXPIRY_METRIC_NAME = "athenz-tenant-cert.expiry.seconds"; private volatile AthenzCredentials credentials; - private final AtomicReference<Throwable> lastThrowable = new AtomicReference<>(); + private final Metric metric; private final AthenzCredentialsService athenzCredentialsService; - private final Scheduler scheduler; + private final ScheduledExecutorService scheduler; private final Clock clock; private final String domain; private final String service; - private final CertificateExpiryMetricUpdater metricUpdater; - @Inject public AthenzIdentityProviderImpl(IdentityConfig config, Metric metric) { this(config, @@ -70,7 +55,7 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen new IdentityDocumentService(config.loadBalancerAddress()), new AthenzService(), Clock.systemUTC()), - new ThreadPoolScheduler(), + new ScheduledThreadPoolExecutor(1), Clock.systemUTC()); } @@ -78,22 +63,22 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen AthenzIdentityProviderImpl(IdentityConfig config, Metric metric, AthenzCredentialsService athenzCredentialsService, - Scheduler scheduler, + ScheduledExecutorService scheduler, Clock clock) { + this.metric = metric; this.athenzCredentialsService = athenzCredentialsService; this.scheduler = scheduler; this.clock = clock; this.domain = config.domain(); this.service = config.service(); - metricUpdater = new CertificateExpiryMetricUpdater(metric); registerInstance(); } private void registerInstance() { try { credentials = athenzCredentialsService.registerInstance(); - scheduler.schedule(new UpdateCredentialsTask(), UPDATE_PERIOD); - scheduler.submit(metricUpdater); + scheduler.scheduleAtFixedRate(this::refreshCertificate, 0, 5, TimeUnit.MINUTES); + scheduler.scheduleAtFixedRate(this::reportMetrics, UPDATE_PERIOD.toMinutes(), UPDATE_PERIOD.toMinutes(), TimeUnit.MINUTES); } catch (Throwable t) { throw new AthenzIdentityProviderException("Could not retrieve Athenz credentials", t); } @@ -121,7 +106,12 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen @Override public void deconstruct() { - scheduler.shutdown(AWAIT_TERMINTATION_TIMEOUT); + try { + scheduler.shutdownNow(); + scheduler.awaitTermination(AWAIT_TERMINTATION_TIMEOUT.getSeconds(), TimeUnit.SECONDS); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } } private boolean isExpired(AthenzCredentials credentials) { @@ -129,96 +119,28 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen } private static Instant getExpirationTime(AthenzCredentials credentials) { - return credentials.getCreatedAt().plus(EXPIRES_AFTER).minus(EXPIRATION_MARGIN); + return credentials.getCertificate().getNotAfter().toInstant(); } - private class UpdateCredentialsTask implements RunnableWithTag { - @Override - public void run() { - try { - AthenzCredentials newCredentials = isExpired(credentials) - ? athenzCredentialsService.registerInstance() - : athenzCredentialsService.updateCredentials(credentials); - credentials = newCredentials; - scheduler.schedule(new UpdateCredentialsTask(), UPDATE_PERIOD); - } catch (Throwable t) { - log.log(LogLevel.WARNING, "Failed to update credentials: " + t.getMessage(), t); - lastThrowable.set(t); - Duration timeToExpiration = Duration.between(clock.instant(), getExpirationTime(credentials)); - // NOTE: Update period might be after timeToExpiration, still we do not want to DDoS Athenz. - Duration updatePeriod = - timeToExpiration.compareTo(UPDATE_PERIOD) > 0 ? UPDATE_PERIOD : REDUCED_UPDATE_PERIOD; - scheduler.schedule(new UpdateCredentialsTask(), updatePeriod); - } - } - - @Override - public String tag() { - return UPDATE_CREDENTIALS_TAG; + void refreshCertificate() { + try { + AthenzCredentials newCredentials = isExpired(credentials) + ? athenzCredentialsService.registerInstance() + : athenzCredentialsService.updateCredentials(credentials); + credentials = newCredentials; + } catch (Throwable t) { + log.log(LogLevel.WARNING, "Failed to update credentials: " + t.getMessage(), t); } } - private class CertificateExpiryMetricUpdater implements RunnableWithTag { - private final Metric metric; - - private CertificateExpiryMetricUpdater(Metric metric) { - this.metric = metric; - } - - @Override - public void run() { + void reportMetrics() { + try { Instant expirationTime = getExpirationTime(credentials); Duration remainingLifetime = Duration.between(clock.instant(), expirationTime); metric.set(CERTIFICATE_EXPIRY_METRIC_NAME, remainingLifetime.getSeconds(), null); - scheduler.schedule(this, CERTIFICATE_EXPIRY_METRIC_UPDATE_PERIOD); - } - - @Override - public String tag() { - return METRICS_UPDATER_TAG; - } - } - - private static class ThreadPoolScheduler implements Scheduler { - - private static final Logger log = Logger.getLogger(ThreadPoolScheduler.class.getName()); - - private final ScheduledExecutorService executor = Executors.newScheduledThreadPool(0); - - @Override - public void schedule(RunnableWithTag runnable, Duration delay) { - log.log(LogLevel.FINE, String.format("Scheduling task '%s' in '%s'", runnable.tag(), delay)); - executor.schedule(runnable, delay.getSeconds(), TimeUnit.SECONDS); - } - - @Override - public void submit(RunnableWithTag runnable) { - log.log(LogLevel.FINE, String.format("Scheduling task '%s' now", runnable.tag())); - executor.submit(runnable); - } - - @Override - public void shutdown(Duration timeout) { - try { - executor.shutdownNow(); - executor.awaitTermination(AWAIT_TERMINTATION_TIMEOUT.getSeconds(), TimeUnit.SECONDS); - } catch (InterruptedException e) { - throw new RuntimeException(e); - } + } catch (Throwable t) { + log.log(LogLevel.WARNING, "Failed to update metrics: " + t.getMessage(), t); } - - } - - public interface Scheduler { - void schedule(RunnableWithTag runnable, Duration delay); - default void submit(RunnableWithTag runnable) { schedule(runnable, Duration.ZERO); } - default void shutdown(Duration timeout) {} } - - public interface RunnableWithTag extends Runnable { - - String tag(); - } - } diff --git a/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/AthenzIdentityProviderImplTest.java b/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/AthenzIdentityProviderImplTest.java index d9dbd73a94e..3bb074cc1dc 100644 --- a/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/AthenzIdentityProviderImplTest.java +++ b/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/AthenzIdentityProviderImplTest.java @@ -2,34 +2,25 @@ package com.yahoo.vespa.athenz.identityprovider; import com.yahoo.container.core.identity.IdentityConfig; -import com.yahoo.container.jdisc.athenz.AthenzIdentityProvider; import com.yahoo.container.jdisc.athenz.AthenzIdentityProviderException; import com.yahoo.jdisc.Metric; import com.yahoo.test.ManualClock; -import com.yahoo.vespa.athenz.identityprovider.AthenzIdentityProviderImpl.RunnableWithTag; -import com.yahoo.vespa.athenz.identityprovider.AthenzIdentityProviderImpl.Scheduler; import org.junit.Test; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; import java.security.cert.X509Certificate; import java.time.Duration; import java.time.Instant; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.PriorityQueue; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.Predicate; - -import static com.yahoo.vespa.athenz.identityprovider.AthenzIdentityProviderImpl.METRICS_UPDATER_TAG; -import static com.yahoo.vespa.athenz.identityprovider.AthenzIdentityProviderImpl.REDUCED_UPDATE_PERIOD; -import static com.yahoo.vespa.athenz.identityprovider.AthenzIdentityProviderImpl.UPDATE_CREDENTIALS_TAG; -import static com.yahoo.vespa.athenz.identityprovider.AthenzIdentityProviderImpl.UPDATE_PERIOD; -import static org.junit.Assert.assertEquals; +import java.util.Date; +import java.util.concurrent.ScheduledExecutorService; +import java.util.function.Supplier; + import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; /** @@ -38,72 +29,79 @@ import static org.mockito.Mockito.when; */ public class AthenzIdentityProviderImplTest { - private static final Metric DUMMY_METRIC = new Metric() { - @Override - public void set(String s, Number number, Context context) { - } - - @Override - public void add(String s, Number number, Context context) { - } - - @Override - public Context createContext(Map<String, ?> stringMap) { - return null; - } - }; + public static final Duration certificateValidity = Duration.ofDays(30); private static final IdentityConfig IDENTITY_CONFIG = new IdentityConfig(new IdentityConfig.Builder() .service("tenantService").domain("tenantDomain").loadBalancerAddress("cfg")); - @Test (expected = AthenzIdentityProviderException.class) + @Test(expected = AthenzIdentityProviderException.class) public void component_creation_fails_when_credentials_not_found() { AthenzCredentialsService credentialService = mock(AthenzCredentialsService.class); when(credentialService.registerInstance()) .thenThrow(new RuntimeException("athenz unavailable")); - ManualClock clock = new ManualClock(Instant.EPOCH); - MockScheduler scheduler = new MockScheduler(clock); - AthenzIdentityProvider identityProvider = - new AthenzIdentityProviderImpl(IDENTITY_CONFIG, DUMMY_METRIC, credentialService, scheduler, clock); + new AthenzIdentityProviderImpl(IDENTITY_CONFIG, mock(Metric.class), credentialService, mock(ScheduledExecutorService.class), new ManualClock(Instant.EPOCH)); } @Test - public void failed_credentials_updates_will_schedule_retries() { + public void metrics_updated_on_refresh() { IdentityDocumentService identityDocumentService = mock(IdentityDocumentService.class); AthenzService athenzService = mock(AthenzService.class); ManualClock clock = new ManualClock(Instant.EPOCH); - MockScheduler scheduler = new MockScheduler(clock); - X509Certificate x509Certificate = mock(X509Certificate.class); + Metric metric = mock(Metric.class); when(identityDocumentService.getSignedIdentityDocument()).thenReturn(getIdentityDocument()); - when(athenzService.sendInstanceRegisterRequest(any(), any())).thenReturn( - new InstanceIdentity(null, "TOKEN")); + when(athenzService.sendInstanceRegisterRequest(any(), any())).then(new Answer<InstanceIdentity>() { + @Override + public InstanceIdentity answer(InvocationOnMock invocationOnMock) throws Throwable { + return new InstanceIdentity(getCertificate(getExpirationSupplier(clock)), "TOKEN"); + } + }); + when(athenzService.sendInstanceRefreshRequest(anyString(), anyString(), anyString(), anyString(), any(), any(), any(), any())) .thenThrow(new RuntimeException("#1")) .thenThrow(new RuntimeException("#2")) - .thenThrow(new RuntimeException("#3")) - .thenReturn(new InstanceIdentity(null, "TOKEN")); + .thenReturn(new InstanceIdentity(getCertificate(getExpirationSupplier(clock)), "TOKEN")); + AthenzCredentialsService credentialService = new AthenzCredentialsService(IDENTITY_CONFIG, identityDocumentService, athenzService, clock); - AthenzIdentityProvider identityProvider = - new AthenzIdentityProviderImpl(IDENTITY_CONFIG, DUMMY_METRIC, credentialService, scheduler, clock); - - List<MockScheduler.CompletedTask> expectedTasks = - Arrays.asList( - new MockScheduler.CompletedTask(UPDATE_CREDENTIALS_TAG, UPDATE_PERIOD), - new MockScheduler.CompletedTask(UPDATE_CREDENTIALS_TAG, UPDATE_PERIOD), - new MockScheduler.CompletedTask(UPDATE_CREDENTIALS_TAG, REDUCED_UPDATE_PERIOD), - new MockScheduler.CompletedTask(UPDATE_CREDENTIALS_TAG, REDUCED_UPDATE_PERIOD), - new MockScheduler.CompletedTask(UPDATE_CREDENTIALS_TAG, UPDATE_PERIOD)); - AtomicInteger counter = new AtomicInteger(0); - List<MockScheduler.CompletedTask> completedTasks = - scheduler.runAllTasks(task -> !task.tag().equals(METRICS_UPDATER_TAG) && - counter.getAndIncrement() < expectedTasks.size()); - assertEquals(expectedTasks, completedTasks); + AthenzIdentityProviderImpl identityProvider = + new AthenzIdentityProviderImpl(IDENTITY_CONFIG, metric, credentialService, mock(ScheduledExecutorService.class), clock); + + identityProvider.reportMetrics(); + verify(metric).set(eq(AthenzIdentityProviderImpl.CERTIFICATE_EXPIRY_METRIC_NAME), eq(certificateValidity.getSeconds()), any()); + + // Advance 1 day, refresh fails, cert is 1 day old + clock.advance(Duration.ofDays(1)); + identityProvider.refreshCertificate(); + identityProvider.reportMetrics(); + verify(metric).set(eq(AthenzIdentityProviderImpl.CERTIFICATE_EXPIRY_METRIC_NAME), eq(certificateValidity.minus(Duration.ofDays(1)).getSeconds()), any()); + + // Advance 1 more day, refresh fails, cert is 2 days old + clock.advance(Duration.ofDays(1)); + identityProvider.refreshCertificate(); + identityProvider.reportMetrics(); + verify(metric).set(eq(AthenzIdentityProviderImpl.CERTIFICATE_EXPIRY_METRIC_NAME), eq(certificateValidity.minus(Duration.ofDays(2)).getSeconds()), any()); + + // Advance 1 more day, refresh succeds, cert is new + clock.advance(Duration.ofDays(1)); + identityProvider.refreshCertificate(); + identityProvider.reportMetrics(); + verify(metric).set(eq(AthenzIdentityProviderImpl.CERTIFICATE_EXPIRY_METRIC_NAME), eq(certificateValidity.getSeconds()), any()); + + } + + private Supplier<Date> getExpirationSupplier(ManualClock clock) { + return () -> new Date(clock.instant().plus(certificateValidity).toEpochMilli()); + } + + private X509Certificate getCertificate(Supplier<Date> expiry) { + X509Certificate x509Certificate = mock(X509Certificate.class); + when(x509Certificate.getNotAfter()).thenReturn(expiry.get()); + return x509Certificate; } private static String getIdentityDocument() { @@ -119,82 +117,4 @@ public class AthenzIdentityProviderImplTest { "}"; } - - private static class MockScheduler implements Scheduler { - - private final PriorityQueue<DelayedTask> tasks = new PriorityQueue<>(); - private final ManualClock clock; - - MockScheduler(ManualClock clock) { - this.clock = clock; - } - - @Override - public void schedule(RunnableWithTag task, Duration delay) { - tasks.offer(new DelayedTask(task, delay, clock.instant().plus(delay))); - } - - List<CompletedTask> runAllTasks(Predicate<RunnableWithTag> filter) { - List<CompletedTask> completedTasks = new ArrayList<>(); - while (!tasks.isEmpty()) { - DelayedTask task = tasks.poll(); - RunnableWithTag runnable = task.runnableWithTag; - if (filter.test(runnable)) { - clock.setInstant(task.startTime); - runnable.run(); - completedTasks.add(new CompletedTask(runnable.tag(), task.delay)); - } - } - return completedTasks; - } - - private static class DelayedTask implements Comparable<DelayedTask> { - final RunnableWithTag runnableWithTag; - final Duration delay; - final Instant startTime; - - DelayedTask(RunnableWithTag runnableWithTag, Duration delay, Instant startTime) { - this.runnableWithTag = runnableWithTag; - this.delay = delay; - this.startTime = startTime; - } - - @Override - public int compareTo(DelayedTask other) { - return this.startTime.compareTo(other.startTime); - } - } - - private static class CompletedTask { - final String tag; - final Duration delay; - - CompletedTask(String tag, Duration delay) { - this.tag = tag; - this.delay = delay; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - CompletedTask that = (CompletedTask) o; - return Objects.equals(tag, that.tag) && - Objects.equals(delay, that.delay); - } - - @Override - public int hashCode() { - return Objects.hash(tag, delay); - } - - @Override - public String toString() { - return "CompletedTask{" + - "tag='" + tag + '\'' + - ", delay=" + delay + - '}'; - } - } - } } |