diff options
93 files changed, 2525 insertions, 783 deletions
diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/AssembleContainerPluginMojo.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/AssembleContainerPluginMojo.java index 30ffc54225f..13d73d58a97 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/AssembleContainerPluginMojo.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/AssembleContainerPluginMojo.java @@ -52,7 +52,7 @@ public class AssembleContainerPluginMojo extends AbstractMojo { private boolean useCommonAssemblyIds = false; @Parameter(alias = "AttachBundle", defaultValue = "false") - private boolean attachBundleToArtifact; + private boolean attachBundleArtifact; @Parameter(alias = "BundleClassifier", defaultValue = "bundle") private String bundleClassifierName; @@ -88,7 +88,7 @@ public class AssembleContainerPluginMojo extends AbstractMojo { addDependencies(jarWithDependencies); createArchive(jarFiles.get(Dependencies.WITH), jarWithDependencies); - if (attachBundleToArtifact) { + if (attachBundleArtifact) { projectHelper.attachArtifact(project, project.getArtifact().getType(), bundleClassifierName, diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java index bd98ccea8ea..c7b9c93c67c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java @@ -297,6 +297,12 @@ public class VespaMetricSet { metrics.add(new Metric("content.proton.documentdb.removed.document_store.memory_usage.dead_bytes.average")); metrics.add(new Metric("content.proton.documentdb.removed.document_store.memory_usage.onhold_bytes.average")); + // document store cache + metrics.add(new Metric("content.proton.documentdb.ready.document_store.cache.memory_usage.average")); + metrics.add(new Metric("content.proton.documentdb.ready.document_store.cache.hit_rate.average")); + metrics.add(new Metric("content.proton.documentdb.notready.document_store.cache.memory_usage.average")); + metrics.add(new Metric("content.proton.documentdb.notready.document_store.cache.hit_rate.average")); + // attribute metrics.add(new Metric("content.proton.documentdb.ready.attribute.memory_usage.allocated_bytes.average")); metrics.add(new Metric("content.proton.documentdb.ready.attribute.memory_usage.used_bytes.average")); diff --git a/controller-api/pom.xml b/controller-api/pom.xml index 1607d6bea0e..7d06e2facbd 100644 --- a/controller-api/pom.xml +++ b/controller-api/pom.xml @@ -86,39 +86,15 @@ </configuration> </plugin> <plugin> - <groupId>org.apache.maven.plugins</groupId> - <artifactId>maven-surefire-plugin</artifactId> - </plugin> - <plugin> <groupId>com.yahoo.vespa</groupId> <artifactId>bundle-plugin</artifactId> <extensions>true</extensions> <configuration> + <attachBundleArtifact>true</attachBundleArtifact> + <bundleClassifierName>deploy</bundleClassifierName> <useCommonAssemblyIds>false</useCommonAssemblyIds> </configuration> </plugin> - <plugin> - <groupId>org.codehaus.mojo</groupId> - <artifactId>build-helper-maven-plugin</artifactId> - <executions> - <execution> - <id>attach-artifacts</id> - <phase>package</phase> - <goals> - <goal>attach-artifact</goal> - </goals> - <configuration> - <artifacts> - <artifact> - <file>target/${project.artifactId}-deploy.jar</file> - <type>jar</type> - <classifier>deploy</classifier> - </artifact> - </artifacts> - </configuration> - </execution> - </executions> - </plugin> </plugins> </build> </project> diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/LogEntry.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/LogEntry.java new file mode 100644 index 00000000000..026e9250cef --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/LogEntry.java @@ -0,0 +1,71 @@ +package com.yahoo.vespa.hosted.controller.api.integration; + +import com.yahoo.log.LogLevel; + +import java.util.Objects; +import java.util.logging.Level; +import java.util.logging.LogRecord; + +import static java.util.Objects.requireNonNull; + +/** Immutable, simple log entries. */ +public class LogEntry { + + private final long id; + private final long at; + private final Level level; + private final String message; + + public LogEntry(long id, long at, Level level, String message) { + if (id < 0) + throw new IllegalArgumentException("Id must be non-negative, but was " + id + "."); + + this.id = id; + this.at = at; + this.level = LogLevel.getVespaLogLevel(requireNonNull(level)); + this.message = requireNonNull(message); + } + + public long id() { + return id; + } + + public long at() { + return at; + } + + public Level level() { + return level; + } + + public String message() { + return message; + } + + @Override + public String toString() { + return "LogEntry{" + + "id=" + id + + ", at=" + at + + ", level=" + level + + ", message='" + message + '\'' + + '}'; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof LogEntry)) return false; + LogEntry entry = (LogEntry) o; + return id == entry.id && + at == entry.at && + Objects.equals(level, entry.level) && + Objects.equals(message, entry.message); + } + + @Override + public int hashCode() { + return Objects.hash(id, at, level, message); + } + +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/LogStore.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/LogStore.java deleted file mode 100644 index 23da48b4aad..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/LogStore.java +++ /dev/null @@ -1,22 +0,0 @@ -package com.yahoo.vespa.hosted.controller.api.integration; - -import com.yahoo.vespa.hosted.controller.api.integration.configserver.PrepareResponse; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; - -import java.util.Optional; - -/** - * @author freva - */ -public interface LogStore { - - /** @return the log of the given step of the given deployment job, or an empty byte array if non-existent. */ - byte[] get(RunId id, String step); - - /** Stores the given log for the given step of the given deployment job. */ - void append(RunId id, String step, byte[] log); - - /** Deletes all data associated with the given deployment job */ - void delete(RunId id); - -} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/RunDataStore.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/RunDataStore.java new file mode 100644 index 00000000000..88634fa8587 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/RunDataStore.java @@ -0,0 +1,22 @@ +package com.yahoo.vespa.hosted.controller.api.integration; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; + +import java.util.Optional; + +/** + * @author jonmv + */ +public interface RunDataStore { + + /** Returns the run logs of the given deployment job, if existent. */ + Optional<byte[]> get(RunId id); + + /** Stores the given log for the given deployment job. */ + void put(RunId id, byte[] log); + + /** Deletes all data associated with the given application. */ + void delete(ApplicationId id); + +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/TesterCloud.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/TesterCloud.java index f1d07fc9097..f2dbcda54b6 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/TesterCloud.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/TesterCloud.java @@ -1,6 +1,9 @@ package com.yahoo.vespa.hosted.controller.api.integration.deployment; +import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; + import java.net.URI; +import java.util.List; /** * Allows running some predefined tests -- typically remotely. @@ -12,8 +15,8 @@ public interface TesterCloud { /** Signals the tester to run its tests. */ void startTests(URI testerUrl, Suite suite, byte[] config); - /** Returns the currently stored logs from the tester. */ - byte[] getLogs(URI testerUrl); + /** Returns the log entries from the tester with ids after the given threshold. */ + List<LogEntry> getLog(URI testerUrl, long after); /** Returns the current status of the tester. */ Status getStatus(URI testerUrl); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockLogStore.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockLogStore.java deleted file mode 100644 index 330b967c1b5..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockLogStore.java +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.api.integration.stubs; - -import com.yahoo.vespa.hosted.controller.api.integration.LogStore; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; - -import java.util.Collections; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; - -/** - * @author jonmv - */ -public class MockLogStore implements LogStore { - - private final Map<RunId, Map<String, byte[]>> logs = new ConcurrentHashMap<>(); - - @Override - public byte[] get(RunId id, String step) { - return logs.getOrDefault(id, Collections.emptyMap()).getOrDefault(step, new byte[0]); - } - - @Override - public void append(RunId id, String step, byte[] log) { - logs.putIfAbsent(id, new ConcurrentHashMap<>()); - byte[] old = get(id, step); - byte[] union = new byte[old.length + log.length]; - System.arraycopy(old, 0, union, 0, old.length); - System.arraycopy(log, 0, union, old.length, log.length); - logs.get(id).put(step, union); - } - - @Override - public void delete(RunId id) { - logs.remove(id); - } - -} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockRunDataStore.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockRunDataStore.java new file mode 100644 index 00000000000..0d7d8eca10e --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockRunDataStore.java @@ -0,0 +1,34 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.api.integration.stubs; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.vespa.hosted.controller.api.integration.RunDataStore; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; + +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; + +/** + * @author jonmv + */ +public class MockRunDataStore implements RunDataStore { + + private final Map<RunId, byte[]> logs = new ConcurrentHashMap<>(); + + @Override + public Optional<byte[]> get(RunId id) { + return Optional.ofNullable(logs.get(id)); + } + + @Override + public void put(RunId id, byte[] log) { + logs.put(id, log); + } + + @Override + public void delete(ApplicationId id) { + logs.keySet().removeIf(runId -> runId.application().equals(id)); + } + +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockTesterCloud.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockTesterCloud.java index ff3f168a978..176fa8ae683 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockTesterCloud.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockTesterCloud.java @@ -1,16 +1,21 @@ package com.yahoo.vespa.hosted.controller.api.integration.stubs; +import com.google.common.collect.ImmutableList; +import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud; import java.net.URI; -import java.util.Arrays; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud.Status.NOT_STARTED; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud.Status.RUNNING; public class MockTesterCloud implements TesterCloud { - private byte[] logs = new byte[0]; + private List<LogEntry> log = new ArrayList<>(); private Status status = NOT_STARTED; private byte[] config; private URI testerUrl; @@ -23,8 +28,8 @@ public class MockTesterCloud implements TesterCloud { } @Override - public byte[] getLogs(URI testerUrl) { - return Arrays.copyOf(logs, logs.length); + public List<LogEntry> getLog(URI testerUrl, long after) { + return log.stream().filter(entry -> entry.id() > after).collect(Collectors.toList()); } @Override @@ -32,8 +37,11 @@ public class MockTesterCloud implements TesterCloud { return status; } - public void set(byte[] logs, Status status) { - this.logs = Arrays.copyOf(logs, logs.length); + public void add(LogEntry entry) { + log.add(entry); + } + + public void set(Status status) { this.status = status; } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneFilter.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneFilter.java index 3bcd7298a15..3e33b2e7bf0 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneFilter.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneFilter.java @@ -5,7 +5,7 @@ package com.yahoo.vespa.hosted.controller.api.integration.zone; * A ZoneId list which can be filtered in various ways; elements can be accessed after at least one filter. * * The methods here return instances of {@link ZoneList}, which extends ZoneFilter, but with accessors and additional filters. - * This forces the developer to consider which of the filters in this class to apply, prior to processing any zones. + * This forces the developer to consider which of the filters in this class to apply, prior to accessing any zones. * * @author jonmv */ @@ -14,13 +14,13 @@ public interface ZoneFilter { /** Negates the next filter. */ ZoneFilter not(); - /** All zones from the initial pool. */ - ZoneList all(); - /** Zones which are upgraded by the controller. */ ZoneList controllerUpgraded(); /** Zones where config servers are up and running. */ ZoneList reachable(); + /** All zones from the initial pool. */ + ZoneList all(); + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneRegistry.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneRegistry.java index 419e532c531..e7ef3e52eb5 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneRegistry.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneRegistry.java @@ -9,7 +9,6 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import java.net.URI; import java.time.Duration; -import java.util.Collections; import java.util.List; import java.util.Optional; @@ -53,21 +52,10 @@ public interface ZoneRegistry { /** Returns the Vespa upgrade policy to use for zones in this registry */ UpgradePolicy upgradePolicy(); - /** Returns the OS upgrade policy to use for zones in this registry */ - // TODO: Remove - default UpgradePolicy osUpgradePolicy() { - return upgradePolicy(); - } + /** Returns the OS upgrade policy to use for zones belonging to given cloud, in this registry */ + UpgradePolicy osUpgradePolicy(CloudName cloud); - // TODO: Remove default implementation /** Returns all OS upgrade policies */ - default List<UpgradePolicy> osUpgradePolicies() { - return Collections.singletonList(upgradePolicy()); - } - - /** Returns the OS upgrade policy to use for zones belonging to given cloud, in this registry */ - default UpgradePolicy osUpgradePolicy(CloudName cloud) { - return osUpgradePolicy(); // TODO: Remove default implementation - } + List<UpgradePolicy> osUpgradePolicies(); } diff --git a/controller-server/pom.xml b/controller-server/pom.xml index 66fefd6f2fe..44954e19fea 100644 --- a/controller-server/pom.xml +++ b/controller-server/pom.xml @@ -216,6 +216,8 @@ <groupId>com.yahoo.vespa</groupId> <artifactId>bundle-plugin</artifactId> <configuration> + <attachBundleArtifact>true</attachBundleArtifact> + <bundleClassifierName>deploy</bundleClassifierName> <useCommonAssemblyIds>false</useCommonAssemblyIds> <WebInfUrl>/WEB-INF/web.xml</WebInfUrl> </configuration> @@ -235,33 +237,6 @@ </compilerArgs> </configuration> </plugin> - <plugin> - <groupId>org.apache.maven.plugins</groupId> - <artifactId>maven-surefire-plugin</artifactId> - </plugin> - - <plugin> - <groupId>org.codehaus.mojo</groupId> - <artifactId>build-helper-maven-plugin</artifactId> - <executions> - <execution> - <id>attach-artifacts</id> - <phase>package</phase> - <goals> - <goal>attach-artifact</goal> - </goals> - <configuration> - <artifacts> - <artifact> - <file>target/${project.artifactId}-deploy.jar</file> - <type>jar</type> - <classifier>deploy</classifier> - </artifact> - </artifacts> - </configuration> - </execution> - </executions> - </plugin> </plugins> </build> </project> diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java index 3733022766e..fb299b4563f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java @@ -12,7 +12,7 @@ import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.api.identifiers.Property; import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; import com.yahoo.vespa.hosted.controller.api.integration.BuildService; -import com.yahoo.vespa.hosted.controller.api.integration.LogStore; +import com.yahoo.vespa.hosted.controller.api.integration.RunDataStore; import com.yahoo.vespa.hosted.controller.api.integration.MetricsService; import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory; import com.yahoo.vespa.hosted.controller.api.integration.chef.Chef; @@ -94,12 +94,12 @@ public class Controller extends AbstractComponent { MetricsService metricsService, NameService nameService, RoutingGenerator routingGenerator, Chef chef, AthenzClientFactory athenzClientFactory, ArtifactRepository artifactRepository, ApplicationStore applicationStore, - BuildService buildService, LogStore logStore) { + BuildService buildService, RunDataStore runDataStore) { this(curator, rotationsConfig, gitHub, entityService, organization, globalRoutingService, zoneRegistry, configServer, metricsService, nameService, routingGenerator, chef, Clock.systemUTC(), athenzClientFactory, artifactRepository, applicationStore, buildService, - logStore, com.yahoo.net.HostName::getLocalhost); + runDataStore, com.yahoo.net.HostName::getLocalhost); } public Controller(CuratorDb curator, RotationsConfig rotationsConfig, @@ -110,7 +110,7 @@ public class Controller extends AbstractComponent { RoutingGenerator routingGenerator, Chef chef, Clock clock, AthenzClientFactory athenzClientFactory, ArtifactRepository artifactRepository, ApplicationStore applicationStore, - BuildService buildService, LogStore logStore, Supplier<String> hostnameSupplier) { + BuildService buildService, RunDataStore runDataStore, Supplier<String> hostnameSupplier) { this.hostnameSupplier = Objects.requireNonNull(hostnameSupplier, "HostnameSupplier cannot be null"); this.curator = Objects.requireNonNull(curator, "Curator cannot be null"); @@ -125,7 +125,7 @@ public class Controller extends AbstractComponent { this.clock = Objects.requireNonNull(clock, "Clock cannot be null"); this.athenzClientFactory = Objects.requireNonNull(athenzClientFactory, "AthenzClientFactory cannot be null"); - jobController = new JobController(this, logStore); + jobController = new JobController(this, runDataStore); applicationController = new ApplicationController(this, curator, athenzClientFactory, Objects.requireNonNull(rotationsConfig, "RotationsConfig cannot be null"), Objects.requireNonNull(nameService, "NameService cannot be null"), @@ -232,8 +232,19 @@ public class Controller extends AbstractComponent { /** Set the target OS version for infrastructure on cloud in this system */ public void upgradeOsIn(CloudName cloud, Version version) { + if (version.isEmpty()) { + throw new IllegalArgumentException("Invalid version '" + version.toFullString() + "'"); + } + if (zoneRegistry.zones().all().ids().stream().noneMatch(zone -> cloud.equals(zone.cloud()))) { + throw new IllegalArgumentException("Cloud '" + cloud.value() + "' does not exist in this system"); + } try (Lock lock = curator.lockOsVersions()) { Set<OsVersion> versions = new TreeSet<>(curator.readOsVersions()); + if (versions.stream().anyMatch(osVersion -> osVersion.cloud().equals(cloud) && + osVersion.version().isAfter(version))) { + throw new IllegalArgumentException("Cannot downgrade cloud '" + cloud.value() + "' to version " + + version.toFullString()); + } versions.removeIf(osVersion -> osVersion.cloud().equals(cloud)); // Only allow a single target per cloud versions.add(new OsVersion(version, cloud)); curator.writeOsVersions(versions); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index ead9388fc3b..054eabc0f21 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -32,14 +32,11 @@ import java.io.IOException; import java.io.PrintStream; import java.io.UncheckedIOException; import java.net.URI; -import java.text.SimpleDateFormat; import java.time.Duration; -import java.util.Date; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Optional; -import java.util.TimeZone; import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.LogRecord; @@ -51,7 +48,6 @@ import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Con import static com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException.ErrorCode.APPLICATION_LOCK_FAILURE; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException.ErrorCode.OUT_OF_CAPACITY; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.active; -import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.failed; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.reserved; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.deploymentFailed; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.error; @@ -73,6 +69,8 @@ import static java.util.logging.Level.WARNING; */ public class InternalStepRunner implements StepRunner { + private static final Logger logger = Logger.getLogger(InternalStepRunner.class.getName()); + static final Duration endpointTimeout = Duration.ofMinutes(15); static final Duration installationTimeout = Duration.ofMinutes(150); @@ -93,7 +91,7 @@ public class InternalStepRunner implements StepRunner { @Override public Optional<RunStatus> run(LockedStep step, RunId id) { - ByteArrayLogger logger = ByteArrayLogger.of(id.application(), id.type(), step.get()); + DualLogger logger = new DualLogger(id, step.get()); try { switch (step.get()) { case deployInitialReal: return deployInitialReal(id, logger); @@ -115,19 +113,16 @@ public class InternalStepRunner implements StepRunner { return Optional.empty(); } catch (RuntimeException e) { - logger.log(INFO, "Unexpected exception running " + id, e); + logger.log(WARNING, "Unexpected exception running " + id, e); if (JobProfile.of(id.type()).alwaysRun().contains(step.get())) { logger.log("Will keep trying, as this is a cleanup step."); return Optional.empty(); } return Optional.of(error); } - finally { - controller.jobController().log(id, step.get(), logger.getLog()); - } } - private Optional<RunStatus> deployInitialReal(RunId id, ByteArrayLogger logger) { + private Optional<RunStatus> deployInitialReal(RunId id, DualLogger logger) { Versions versions = controller.jobController().run(id).get().versions(); logger.log("Deploying platform version " + versions.sourcePlatform().orElse(versions.targetPlatform()) + @@ -136,14 +131,14 @@ public class InternalStepRunner implements StepRunner { return deployReal(id, true, logger); } - private Optional<RunStatus> deployReal(RunId id, ByteArrayLogger logger) { + private Optional<RunStatus> deployReal(RunId id, DualLogger logger) { Versions versions = controller.jobController().run(id).get().versions(); logger.log("Deploying platform version " + versions.targetPlatform() + " and application version " + versions.targetApplication().id() + " ..."); return deployReal(id, false, logger); } - private Optional<RunStatus> deployReal(RunId id, boolean setTheStage, ByteArrayLogger logger) { + private Optional<RunStatus> deployReal(RunId id, boolean setTheStage, DualLogger logger) { return deploy(id.application(), id.type(), () -> controller.applications().deploy(id.application(), @@ -156,7 +151,7 @@ public class InternalStepRunner implements StepRunner { logger); } - private Optional<RunStatus> deployTester(RunId id, ByteArrayLogger logger) { + private Optional<RunStatus> deployTester(RunId id, DualLogger logger) { // TODO jvenstad: Consider deploying old version of tester for initial staging feeding? logger.log("Deploying the tester container ..."); return deploy(testerOf(id.application()), @@ -171,7 +166,7 @@ public class InternalStepRunner implements StepRunner { logger); } - private Optional<RunStatus> deploy(ApplicationId id, JobType type, Supplier<ActivateResult> deployment, ByteArrayLogger logger) { + private Optional<RunStatus> deploy(ApplicationId id, JobType type, Supplier<ActivateResult> deployment, DualLogger logger) { try { PrepareResponse prepareResponse = deployment.get().prepareResponse(); if ( ! prepareResponse.configChangeActions.refeedActions.stream().allMatch(action -> action.allowed)) { @@ -219,15 +214,15 @@ public class InternalStepRunner implements StepRunner { } } - private Optional<RunStatus> installInitialReal(RunId id, ByteArrayLogger logger) { + private Optional<RunStatus> installInitialReal(RunId id, DualLogger logger) { return installReal(id, true, logger); } - private Optional<RunStatus> installReal(RunId id, ByteArrayLogger logger) { + private Optional<RunStatus> installReal(RunId id, DualLogger logger) { return installReal(id, false, logger); } - private Optional<RunStatus> installReal(RunId id, boolean setTheStage, ByteArrayLogger logger) { + private Optional<RunStatus> installReal(RunId id, boolean setTheStage, DualLogger logger) { if (expired(id.application(), id.type())) { logger.log(INFO, "Deployment expired before installation was successful."); return Optional.of(installationFailed); @@ -252,7 +247,7 @@ public class InternalStepRunner implements StepRunner { return Optional.empty(); } - private Optional<RunStatus> installTester(RunId id, ByteArrayLogger logger) { + private Optional<RunStatus> installTester(RunId id, DualLogger logger) { logger.log("Checking installation of tester container ..."); if (expired(id.application(), id.type())) { logger.log(INFO, "Deployment expired before tester was installed."); @@ -273,7 +268,7 @@ public class InternalStepRunner implements StepRunner { return Optional.empty(); } - private boolean nodesConverged(ApplicationId id, JobType type, Version target, ByteArrayLogger logger) { + private boolean nodesConverged(ApplicationId id, JobType type, Version target, DualLogger logger) { List<Node> nodes = controller.configServer().nodeRepository().list(type.zone(controller.system()), id, ImmutableSet.of(active, reserved)); for (Node node : nodes) logger.log(String.format("%70s: %-16s%-25s%-32s%s", @@ -297,7 +292,7 @@ public class InternalStepRunner implements StepRunner { .orElse(false); } - private Optional<RunStatus> startTests(RunId id, ByteArrayLogger logger) { + private Optional<RunStatus> startTests(RunId id, DualLogger logger) { logger.log("Attempting to find endpoints ..."); if (expired(id.application(), id.type())) { logger.log(INFO, "Deployment expired before tests could start."); @@ -340,10 +335,13 @@ public class InternalStepRunner implements StepRunner { return Optional.empty(); } - private Optional<RunStatus> endTests(RunId id, ByteArrayLogger logger) { + private Optional<RunStatus> endTests(RunId id, DualLogger logger) { URI testerEndpoint = testerEndpoint(id) .orElseThrow(() -> new NoSuchElementException("Endpoint for tester vanished again before tests were complete!")); + JobController jobs = controller.jobController(); + jobs.logTestEntries(id, testerCloud.getLog(testerEndpoint, jobs.run(id).get().lastTestLogEntry())); + RunStatus status; switch (testerCloud.getStatus(testerEndpoint)) { case NOT_STARTED: @@ -362,17 +360,16 @@ public class InternalStepRunner implements StepRunner { default: throw new AssertionError("Unknown status!"); } - logger.log(new String(testerCloud.getLogs(testerEndpoint))); // TODO jvenstad: Replace with something less hopeless! return Optional.of(status); } - private Optional<RunStatus> deactivateReal(RunId id, ByteArrayLogger logger) { + private Optional<RunStatus> deactivateReal(RunId id, DualLogger logger) { logger.log("Deactivating deployment of " + id.application() + " in " + id.type().zone(controller.system()) + " ..."); controller.applications().deactivate(id.application(), id.type().zone(controller.system())); return Optional.of(running); } - private Optional<RunStatus> deactivateTester(RunId id, ByteArrayLogger logger) { + private Optional<RunStatus> deactivateTester(RunId id, DualLogger logger) { logger.log("Deactivating tester of " + id.application() + " in " + id.type().zone(controller.system()) + " ..."); controller.jobController().deactivateTester(id.application(), id.type()); return Optional.of(running); @@ -516,53 +513,39 @@ public class InternalStepRunner implements StepRunner { } } - /** Logger which logs all records to a private byte array, as well as to its parent. */ - static class ByteArrayLogger extends Logger { - - private static final Logger parent = Logger.getLogger(InternalStepRunner.class.getName()); - private static final SimpleDateFormat timestampFormat = new SimpleDateFormat("[HH:mm:ss.SSS] "); - static { timestampFormat.setTimeZone(TimeZone.getTimeZone("UTC")); } - - private final ByteArrayOutputStream bytes; - private final PrintStream out; + /** Logger which logs to a {@link JobController}, as well as to the parent class' {@link Logger}. */ + private class DualLogger { - private ByteArrayLogger(Logger parent, String suffix) { - super(parent.getName() + suffix, null); - setParent(parent); + private final RunId id; + private final Step step; + private final String prefix; - bytes = new ByteArrayOutputStream(); - out = new PrintStream(bytes); + private DualLogger(RunId id, Step step) { + this.id = id; + this.step = step; + this.prefix = id + " at " + step + ": "; } - static ByteArrayLogger of(ApplicationId id, JobType type, Step step) { - return new ByteArrayLogger(parent, String.format(".%s.%s.%s", id.toString(), type.jobName(), step)); + private void log(String message) { + log(DEBUG, message); } - @Override - public void log(LogRecord record) { - // TODO jvenstad: Store log records in a serialised format. - String timestamp = timestampFormat.format(new Date(record.getMillis())); - for (String line : record.getMessage().split("\n")) - out.println(timestamp + ": " + line); - if (record.getThrown() != null) - record.getThrown().printStackTrace(out); - - record.setSourceClassName(null); // Makes the root logger's ConsoleHandler use the logger name instead, when printing. - getParent().log(record); + private void log(Level level, String message) { + log(level, message, null); } - public void log(String message) { - log(DEBUG, message); - } + private void log(Level level, String message, Throwable thrown) { + LogRecord record = new LogRecord(level, prefix + message); + record.setThrown(thrown); + logger.log(record); - @Override - public boolean isLoggable(Level __) { - return true; - } + if (thrown != null) { + ByteArrayOutputStream traceBuffer = new ByteArrayOutputStream(); + thrown.printStackTrace(new PrintStream(traceBuffer)); + message += "\n" + traceBuffer; + } - public byte[] getLog() { - out.flush(); - return bytes.toByteArray(); + controller.jobController().log(id, step, level, message); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 12d751551f4..ed844b96eda 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -6,7 +6,8 @@ import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; -import com.yahoo.vespa.hosted.controller.api.integration.LogStore; +import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; +import com.yahoo.vespa.hosted.controller.api.integration.RunDataStore; import com.yahoo.vespa.hosted.controller.api.integration.configserver.NoInstanceException; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; @@ -16,9 +17,10 @@ import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.application.SourceRevision; +import com.yahoo.vespa.hosted.controller.persistence.BufferedLogStore; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; -import java.util.HashMap; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -28,11 +30,14 @@ import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.UnaryOperator; +import java.util.logging.Level; +import java.util.logging.LogRecord; import java.util.stream.Stream; import static com.google.common.collect.ImmutableList.copyOf; import static com.yahoo.vespa.hosted.controller.deployment.Step.deactivateTester; import static com.yahoo.vespa.hosted.controller.deployment.InternalStepRunner.testerOf; +import static com.yahoo.vespa.hosted.controller.deployment.Step.endTests; /** * A singleton owned by the controller, which contains the state and methods for controlling deployment jobs. @@ -51,12 +56,12 @@ public class JobController { private final Controller controller; private final CuratorDb curator; - private final LogStore logs; + private final BufferedLogStore logs; - public JobController(Controller controller, LogStore logStore) { + public JobController(Controller controller, RunDataStore runDataStore) { this.controller = controller; this.curator = controller.curator(); - this.logs = logStore; + this.logs = new BufferedLogStore(curator, runDataStore); } @@ -70,28 +75,45 @@ public class JobController { } } - /** Returns the details currently logged for the given run, if known. */ - public Optional<RunDetails> details(RunId id) { - Run run = runs(id.application(), id.type()).get(id); - if (run == null) - return Optional.empty(); - - Map<Step, byte[]> details = new HashMap<>(); - for (Step step : run.steps().keySet()) { - byte[] log = logs.get(id, step.name()); - if (log.length > 0) - details.put(step, log); - } - return Optional.of(new RunDetails(details)); + /** Returns all entries currently logged for the given run. */ + public Optional<RunLog> details(RunId id) { + return details(id, -1); } - /** Appends the given log bytes to the currently stored bytes for the given run and step. */ - public void log(RunId id, Step step, byte[] log) { + /** Returns the logged entries for the given run, which are after the given id threshold. */ + public Optional<RunLog> details(RunId id, long after) { try (Lock __ = curator.lock(id.application(), id.type())) { - logs.append(id, step.name(), log); + Run run = runs(id.application(), id.type()).get(id); + if (run == null) + return Optional.empty(); + + return active(id).isPresent() + ? Optional.of(logs.readActive(id.application(), id.type(), after)) + : logs.readFinished(id, after); } } + /** Stores the given log record for the given run and step. */ + public void log(RunId id, Step step, Level level, String message) { + locked(id, __ -> { + LogEntry entry = new LogEntry(0, controller.clock().millis(), level, message); + logs.append(id.application(), id.type(), step, Collections.singletonList(entry)); + return __; + }); + } + + /** Stores the given test log records, and records the id of the last of these, for continuation. */ + public void logTestEntries(RunId id, List<LogEntry> entries) { + if (entries.isEmpty()) + return; + + locked(id, run -> { + long lastTestRecord = entries.stream().mapToLong(LogEntry::id).max().getAsLong(); + logs.append(id.application(), id.type(), endTests, entries); + return run.with(lastTestRecord); + }); + } + /** Returns a list of all application which have registered. */ public List<ApplicationId> applications() { return copyOf(controller.applications().asList().stream() @@ -153,6 +175,7 @@ public class JobController { locked(id, run -> { // Store the modified run after it has been written to the collection, in case the latter fails. Run finishedRun = run.finished(controller.clock().instant()); locked(id.application(), id.type(), runs -> runs.put(run.id(), finishedRun)); + logs.flush(id); return finishedRun; }); } @@ -238,14 +261,15 @@ public class JobController { locked(id, type, deactivateTester, __ -> { try (Lock ___ = curator.lock(id, type)) { deactivateTester(id, type); - curator.deleteJobData(id, type); + curator.deleteRunData(id, type); + logs.delete(id); } }); } catch (TimeoutException e) { return; // Don't remove the data if we couldn't deactivate all testers. } - curator.deleteJobData(id); + curator.deleteRunData(id); }); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Run.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Run.java index 70670430bf4..6a0da68f3c9 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Run.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Run.java @@ -78,6 +78,13 @@ public class Run { return new Run(id, new EnumMap<>(steps), versions, start, end, aborted, lastTestRecord); } + public Run with(long lastTestRecord) { + if (hasEnded()) + throw new AssertionError("This run ended at " + end.get() + " -- it can't be further modified!"); + + return new Run(id, new EnumMap<>(steps), versions, start, end, status, lastTestRecord); + } + /** Returns the id of this run. */ public RunId id() { return id; @@ -118,7 +125,7 @@ public class Run { } /** Returns the sequence id of the last test record received from the tester, for the test logs of this run. */ - public long lastTestRecord() { + public long lastTestLogEntry() { return lastTestRecord; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunDetails.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunDetails.java deleted file mode 100644 index 60d7a6a8f04..00000000000 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunDetails.java +++ /dev/null @@ -1,26 +0,0 @@ -package com.yahoo.vespa.hosted.controller.deployment; - -import com.google.common.collect.ImmutableMap; - -import java.util.Map; -import java.util.Optional; - -/** - * Contains details about a deployment job run. - * - * @author jonmv - */ -public class RunDetails { - - // TODO jvenstad: Store a serialised structure, rather than a flat text. - private final Map<Step, byte[]> logs; - - public RunDetails(Map<Step, byte[]> logs) { - this.logs = ImmutableMap.copyOf(logs); - } - - public Optional<byte[]> get(Step step) { - return Optional.ofNullable(logs.get(step)); - } - -} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunLog.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunLog.java new file mode 100644 index 00000000000..d310ccc8915 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunLog.java @@ -0,0 +1,56 @@ +package com.yahoo.vespa.hosted.controller.deployment; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; + +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.OptionalLong; + +/** + * Immutable class which contains the log of a deployment job run. + * + * @author jonmv + */ +public class RunLog { + + private static final RunLog empty = RunLog.of(Collections.emptyMap()); + + private final Map<Step, List<LogEntry>> log; + private final OptionalLong lastId; + + private RunLog(OptionalLong lastId, Map<Step, List<LogEntry>> log) { + this.log = log; + this.lastId = lastId; + } + + /** Creates a RunLog which contains a deep copy of the given log. */ + public static RunLog of(Map<Step, List<LogEntry>> log) { + ImmutableMap.Builder<Step, List<LogEntry>> builder = ImmutableMap.builder(); + log.forEach((step, entries) -> builder.put(step, ImmutableList.copyOf(entries))); + OptionalLong lastId = log.values().stream() + .flatMap(List::stream) + .mapToLong(LogEntry::id) + .max(); + return new RunLog(lastId, builder.build()); + } + + /** Returns an empty RunLog. */ + public static RunLog empty() { + return empty; + } + + /** Returns the log entries for the given step, if any are recorded. */ + public Optional<List<LogEntry>> get(Step step) { + return Optional.ofNullable(log.get(step)); + } + + /** Returns the id of the last log entry in this, if it has any. */ + public OptionalLong lastId() { + return lastId; + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java index feec83d226e..258a72c8d01 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.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.hosted.controller.maintenance; +import com.google.common.collect.ImmutableMap; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.SystemName; import com.yahoo.jdisc.Metric; @@ -11,6 +12,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.chef.Chef; import com.yahoo.vespa.hosted.controller.api.integration.chef.rest.PartialNode; import com.yahoo.vespa.hosted.controller.api.integration.chef.rest.PartialNodeResult; import com.yahoo.vespa.hosted.controller.application.ApplicationList; +import com.yahoo.vespa.hosted.controller.application.JobList; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.rotation.RotationLock; @@ -34,6 +36,7 @@ public class MetricsReporter extends Maintainer { public static final String convergeMetric = "seconds.since.last.chef.convergence"; public static final String deploymentFailMetric = "deployment.failurePercentage"; public static final String deploymentAverageDuration = "deployment.averageDuration"; + public static final String deploymentFailingUpgrades = "deployment.failingUpgrades"; public static final String remainingRotations = "remaining_rotations"; private final Metric metric; @@ -113,38 +116,46 @@ public class MetricsReporter extends Maintainer { } private void reportDeploymentMetrics() { - metric.set(deploymentFailMetric, deploymentFailRatio() * 100, metric.createContext(Collections.emptyMap())); - for (Map.Entry<ApplicationId, Duration> entry : averageDeploymentDurations().entrySet()) { - Map<String, String> dimensions = new HashMap<>(); - dimensions.put("tenant", entry.getKey().tenant().value()); - dimensions.put("app", entry.getKey().application().value() + "." + entry.getKey().instance().value()); - metric.set(deploymentAverageDuration, entry.getValue().getSeconds(), metric.createContext(dimensions)); - } + ApplicationList applications = ApplicationList.from(controller().applications().asList()) + .notPullRequest() + .hasProductionDeployment(); + + metric.set(deploymentFailMetric, deploymentFailRatio(applications) * 100, metric.createContext(Collections.emptyMap())); + + averageDeploymentDurations(applications, clock.instant()).forEach((application, duration) -> { + metric.set(deploymentAverageDuration, duration.getSeconds(), metric.createContext(dimensions(application))); + }); + + deploymentsFailingUpgrade(applications).forEach((application, failingJobs) -> { + metric.set(deploymentFailingUpgrades, failingJobs, metric.createContext(dimensions(application))); + }); } - private double deploymentFailRatio() { - List<Application> applications = ApplicationList.from(controller().applications().asList()) - .notPullRequest() - .hasProductionDeployment() - .asList(); + private static double deploymentFailRatio(ApplicationList applicationList) { + List<Application> applications = applicationList.asList(); if (applications.isEmpty()) return 0; return (double) applications.stream().filter(a -> a.deploymentJobs().hasFailures()).count() / (double) applications.size(); } - private Map<ApplicationId, Duration> averageDeploymentDurations() { - Instant now = clock.instant(); - return ApplicationList.from(controller().applications().asList()) - .notPullRequest() - .hasProductionDeployment() - .asList() - .stream() - .collect(Collectors.toMap(Application::id, - application -> averageDeploymentDuration(application, now))); + private static Map<ApplicationId, Duration> averageDeploymentDurations(ApplicationList applications, Instant now) { + return applications.asList().stream() + .collect(Collectors.toMap(Application::id, + application -> averageDeploymentDuration(application, now))); } - private Duration averageDeploymentDuration(Application application, Instant now) { + private static Map<ApplicationId, Integer> deploymentsFailingUpgrade(ApplicationList applications) { + return applications.asList() + .stream() + .collect(Collectors.toMap(Application::id, MetricsReporter::deploymentsFailingUpgrade)); + } + + private static int deploymentsFailingUpgrade(Application application) { + return JobList.from(application).upgrading().failing().size(); + } + + private static Duration averageDeploymentDuration(Application application, Instant now) { List<Duration> jobDurations = application.deploymentJobs().jobStatus().values().stream() .filter(status -> status.lastTriggered().isPresent()) .map(status -> { @@ -162,10 +173,17 @@ public class MetricsReporter extends Maintainer { .orElse(Duration.ZERO); } - private void keepNodesWithSystem(PartialNodeResult nodeResult, SystemName system) { + private static void keepNodesWithSystem(PartialNodeResult nodeResult, SystemName system) { nodeResult.rows.removeIf(node -> !system.name().equals(node.getValue("system").orElse("main"))); } + private static Map<String, String> dimensions(ApplicationId application) { + return ImmutableMap.of( + "tenant", application.tenant().value(), + "app",application.application().value() + "." + application.instance().value() + ); + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStore.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStore.java new file mode 100644 index 00000000000..dce3f52bd35 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStore.java @@ -0,0 +1,97 @@ +package com.yahoo.vespa.hosted.controller.persistence; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.vespa.hosted.controller.api.integration.RunDataStore; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; +import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; +import com.yahoo.vespa.hosted.controller.deployment.RunLog; +import com.yahoo.vespa.hosted.controller.deployment.Step; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; + +/** + * Stores logs in bite-sized chunks using a {@link CuratorDb}, and flushes to a + * {@link com.yahoo.vespa.hosted.controller.api.integration.RunDataStore} when the log is final. + * + * @author jonmv + */ +public class BufferedLogStore { + + static final int chunkSize = 1 << 17; + + private final CuratorDb buffer; + private final RunDataStore store; + private final LogSerializer logSerializer = new LogSerializer(); + + public BufferedLogStore(CuratorDb buffer, RunDataStore store) { + this.buffer = buffer; + this.store = store; + } + + /** Appends to the log of the given, active run, reassigning IDs as counted here, and converting to Vespa log levels. */ + public void append(ApplicationId id, JobType type, Step step, List<LogEntry> entries) { + if (entries.isEmpty()) + return; + + // Start a new chunk if the previous one is full, or if none have been written yet. + // The id of a chunk is the id of the first entry in it. + long lastEntryId = buffer.readLastLogEntryId(id, type).orElse(-1L); + long lastChunkId = buffer.getLogChunkIds(id, type).max().orElse(0); + byte[] emptyChunk = "[]".getBytes(); + byte[] lastChunk = buffer.readLog(id, type, lastChunkId).orElse(emptyChunk); + if (lastChunk.length > chunkSize) { + lastChunkId = lastEntryId + 1; + lastChunk = emptyChunk; + } + Map<Step, List<LogEntry>> log = logSerializer.fromJson(lastChunk, -1); + List<LogEntry> stepEntries = log.computeIfAbsent(step, __ -> new ArrayList<>()); + for (LogEntry entry : entries) + stepEntries.add(new LogEntry(++lastEntryId, entry.at(), entry.level(), entry.message())); + + buffer.writeLog(id, type, lastChunkId, logSerializer.toJson(log)); + buffer.writeLastLogEntryId(id, type, lastEntryId); + } + + /** Reads all log entries after the given threshold, from the buffered log, i.e., for an active run. */ + public RunLog readActive(ApplicationId id, JobType type, long after) { + return buffer.readLastLogEntryId(id, type).orElse(-1L) <= after + ? RunLog.empty() + : RunLog.of(readChunked(id, type, after)); + } + + /** Reads all log entries after the given threshold, from the stored log, i.e., for a finished run. */ + public Optional<RunLog> readFinished(RunId id, long after) { + return store.get(id).map(json -> RunLog.of(logSerializer.fromJson(json, after))); + } + + /** Writes the buffered log of the now finished run to the long-term store, and clears the buffer. */ + public void flush(RunId id) { + store.put(id, logSerializer.toJson(readChunked(id.application(), id.type(), -1))); + buffer.deleteLog(id.application(), id.type()); + } + + /** Deletes all logs for the given application. */ + public void delete(ApplicationId id) { + for (JobType type : JobType.values()) + buffer.deleteLog(id, type); + store.delete(id); + } + + private Map<Step, List<LogEntry>> readChunked(ApplicationId id, JobType type, long after) { + long[] chunkIds = buffer.getLogChunkIds(id, type).toArray(); + int firstChunk = chunkIds.length; + while (firstChunk > 0 && chunkIds[--firstChunk] > after + 1); + return logSerializer.fromJson(Arrays.stream(chunkIds, firstChunk, chunkIds.length) + .mapToObj(chunkId -> buffer.readLog(id, type, chunkId)) + .filter(Optional::isPresent).map(Optional::get) + .collect(Collectors.toList()), + after); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index 2ed69ad9be8..cf244c38565 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -45,6 +45,12 @@ import java.util.function.Predicate; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; +import java.util.stream.LongStream; + +import static java.util.stream.Collectors.collectingAndThen; +import static java.util.stream.Collectors.groupingBy; +import static java.util.stream.Collectors.reducing; +import static java.util.stream.Collectors.toMap; /** * Curator backed database for storing the persistence state of controllers. This maps controller specific operations @@ -52,6 +58,7 @@ import java.util.stream.Collectors; * * @author bratseth * @author mpolden + * @author jonmv */ public class CuratorDb { @@ -296,7 +303,7 @@ public class CuratorDb { .map(this::readTenant) .filter(Optional::isPresent) .map(Optional::get) - .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); + .collect(collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); } public void removeTenant(TenantName name) { @@ -328,7 +335,7 @@ public class CuratorDb { .map(this::readApplication) .filter(Optional::isPresent) .map(Optional::get) - .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); + .collect(collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); } public void removeApplication(ApplicationId application) { @@ -342,7 +349,7 @@ public class CuratorDb { } public void writeHistoricRuns(ApplicationId id, JobType type, Iterable<Run> runs) { - curator.set(jobPath(id, type), asJson(runSerializer.toSlime(runs))); + curator.set(runsPath(id, type), asJson(runSerializer.toSlime(runs))); } public Optional<Run> readLastRun(ApplicationId id, JobType type) { @@ -351,15 +358,15 @@ public class CuratorDb { public Map<RunId, Run> readHistoricRuns(ApplicationId id, JobType type) { // TODO jvenstad: Add, somewhere, a retention filter based on age or count. - return readSlime(jobPath(id, type)).map(runSerializer::runsFromSlime).orElse(new LinkedHashMap<>()); + return readSlime(runsPath(id, type)).map(runSerializer::runsFromSlime).orElse(new LinkedHashMap<>()); } - public void deleteJobData(ApplicationId id, JobType type) { - curator.delete(jobPath(id, type)); + public void deleteRunData(ApplicationId id, JobType type) { + curator.delete(runsPath(id, type)); curator.delete(lastRunPath(id, type)); } - public void deleteJobData(ApplicationId id) { + public void deleteRunData(ApplicationId id) { curator.delete(jobRoot.append(id.serializedForm())); } @@ -369,6 +376,33 @@ public class CuratorDb { .collect(Collectors.toList()); } + + public Optional<byte[]> readLog(ApplicationId id, JobType type, long chunkId) { + return curator.getData(logPath(id, type, chunkId)); + } + + public void writeLog(ApplicationId id, JobType type, long chunkId, byte[] log) { + curator.set(logPath(id, type, chunkId), log); + } + + public void deleteLog(ApplicationId id, JobType type) { + curator.delete(runsPath(id, type).append("logs")); + } + + public Optional<Long> readLastLogEntryId(ApplicationId id, JobType type) { + return curator.getData(lastLogPath(id, type)) + .map(String::new).map(Long::parseLong); + } + + public void writeLastLogEntryId(ApplicationId id, JobType type, long lastId) { + curator.set(lastLogPath(id, type), Long.toString(lastId).getBytes()); + } + + public LongStream getLogChunkIds(ApplicationId id, JobType type) { + return curator.getChildren(runsPath(id, type).append("logs")).stream() + .mapToLong(Long::parseLong); + } + // -------------- Provisioning (called by internal code) ------------------ @SuppressWarnings("unused") @@ -501,12 +535,20 @@ public class CuratorDb { return applicationRoot.append(application.serializedForm()); } - private static Path jobPath(ApplicationId id, JobType type) { + private static Path runsPath(ApplicationId id, JobType type) { return jobRoot.append(id.serializedForm()).append(type.jobName()); } private static Path lastRunPath(ApplicationId id, JobType type) { - return jobPath(id, type).append("last"); + return runsPath(id, type).append("last"); + } + + private static Path logPath(ApplicationId id, JobType type, long first) { + return runsPath(id, type).append("logs").append(Long.toString(first)); + } + + private static Path lastLogPath(ApplicationId id, JobType type) { + return runsPath(id, type).append("logs"); } private static Path controllerPath(String hostname) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogRecordSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogRecordSerializer.java deleted file mode 100644 index f582b240260..00000000000 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogRecordSerializer.java +++ /dev/null @@ -1,64 +0,0 @@ -package com.yahoo.vespa.hosted.controller.persistence; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import com.yahoo.log.LogLevel; -import com.yahoo.slime.ArrayTraverser; -import com.yahoo.slime.Cursor; -import com.yahoo.slime.Inspector; -import com.yahoo.slime.ObjectTraverser; -import com.yahoo.slime.Slime; -import com.yahoo.vespa.hosted.controller.deployment.Step; - -import java.util.List; -import java.util.Map; -import java.util.logging.LogRecord; - -/** - * Serialisation of LogRecord objects. Not all fields are stored! - * - * @author jonmv - */ -class LogRecordSerializer { - - private static final String idField = "id"; - private static final String levelField = "level"; - private static final String timestampField = "at"; - private static final String messageField = "message"; - - Slime recordsToSlime(Map<Step, List<LogRecord>> stepRecords) { - Slime root = new Slime(); - Cursor recordsObject = root.setObject(); - stepRecords.forEach((step, records) -> { - Cursor recordsArray = recordsObject.setArray(RunSerializer.valueOf(step)); - records.forEach(record -> toSlime(record, recordsArray.addObject())); - }); - return root; - } - - void toSlime(LogRecord record, Cursor recordObject) { - recordObject.setLong(idField, record.getSequenceNumber()); - recordObject.setString(levelField, LogLevel.getVespaLogLevel(record.getLevel()).getName()); - recordObject.setLong(timestampField, record.getMillis()); - recordObject.setString(messageField, record.getMessage()); - } - - Map<Step, List<LogRecord>> recordsFromSlime(Slime slime) { - ImmutableMap.Builder<Step, List<LogRecord>> stepRecords = ImmutableMap.builder(); - slime.get().traverse((ObjectTraverser) (step, recordsArray) -> { - ImmutableList.Builder<LogRecord> records = ImmutableList.builder(); - recordsArray.traverse((ArrayTraverser) (__, recordObject) -> records.add(fromSlime(recordObject))); - stepRecords.put(RunSerializer.stepOf(step), records.build()); - }); - return stepRecords.build(); - } - - private LogRecord fromSlime(Inspector recordObject) { - LogRecord record = new LogRecord(LogLevel.parse(recordObject.field(levelField).asString()), - recordObject.field(messageField).asString()); - record.setSequenceNumber(recordObject.field(idField).asLong()); - record.setMillis(recordObject.field(timestampField).asLong()); - return record; - } - -} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializer.java new file mode 100644 index 00000000000..337204a31af --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializer.java @@ -0,0 +1,92 @@ +package com.yahoo.vespa.hosted.controller.persistence; + +import com.yahoo.log.LogLevel; +import com.yahoo.slime.ArrayTraverser; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Inspector; +import com.yahoo.slime.ObjectTraverser; +import com.yahoo.slime.Slime; +import com.yahoo.vespa.config.SlimeUtils; +import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; +import com.yahoo.vespa.hosted.controller.deployment.Step; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +/** + * Serialisation of LogRecord objects. Not all fields are stored! + * + * @author jonmv + */ +class LogSerializer { + + private static final String idField = "id"; + private static final String levelField = "level"; + private static final String timestampField = "at"; + private static final String messageField = "message"; + + byte[] toJson(Map<Step, List<LogEntry>> log) { + try { + return SlimeUtils.toJsonBytes(toSlime(log)); + } + catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + Slime toSlime(Map<Step, List<LogEntry>> log) { + Slime root = new Slime(); + Cursor logObject = root.setObject(); + log.forEach((step, entries) -> { + Cursor recordsArray = logObject.setArray(RunSerializer.valueOf(step)); + entries.forEach(entry -> toSlime(entry, recordsArray.addObject())); + }); + return root; + } + + private void toSlime(LogEntry entry, Cursor entryObject) { + entryObject.setLong(idField, entry.id()); + entryObject.setLong(timestampField, entry.at()); + entryObject.setString(levelField, entry.level().getName()); + entryObject.setString(messageField, entry.message()); + } + + Map<Step, List<LogEntry>> fromJson(byte[] logJson, long after) { + return fromJson(Collections.singletonList(logJson), after); + } + + Map<Step, List<LogEntry>> fromJson(List<byte[]> logJsons, long after) { + return fromSlime(logJsons.stream() + .map(SlimeUtils::jsonToSlime) + .collect(Collectors.toList()), + after); + } + + Map<Step, List<LogEntry>> fromSlime(List<Slime> slimes, long after) { + Map<Step, List<LogEntry>> log = new HashMap<>(); + slimes.forEach(slime -> slime.get().traverse((ObjectTraverser) (stepName, entryArray) -> { + Step step = RunSerializer.stepOf(stepName); + List<LogEntry> entries = log.computeIfAbsent(step, __ -> new ArrayList<>()); + entryArray.traverse((ArrayTraverser) (__, entryObject) -> { + LogEntry entry = fromSlime(entryObject); + if (entry.id() > after) + entries.add(entry); + }); + })); + return log; + } + + private LogEntry fromSlime(Inspector entryObject) { + return new LogEntry(entryObject.field(idField).asLong(), + entryObject.field(timestampField).asLong(), + LogLevel.parse(entryObject.field(levelField).asString()), + entryObject.field(messageField).asString()); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MockCuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MockCuratorDb.java index 57364cee049..cae48eba242 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MockCuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MockCuratorDb.java @@ -1,6 +1,7 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.persistence; +import com.google.inject.Inject; import com.yahoo.vespa.curator.mock.MockCurator; /** @@ -11,6 +12,7 @@ import com.yahoo.vespa.curator.mock.MockCurator; @SuppressWarnings("unused") // injected public class MockCuratorDb extends CuratorDb { + @Inject public MockCuratorDb() { this("test-controller:2222"); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializer.java index aee19644b82..0a4a13d3723 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializer.java @@ -47,16 +47,8 @@ public class OsVersionSerializer { public OsVersion fromSlime(Inspector object) { return new OsVersion( Version.fromString(object.field(versionField).asString()), - cloudFrom(object.field(cloudField)) + CloudName.from(object.field(cloudField).asString()) ); } - // TODO: Simplify and inline after 2018-09-01 - private static CloudName cloudFrom(Inspector field) { - if (!field.valid()) { - return CloudName.defaultName(); - } - return CloudName.from(field.asString()); - } - } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java index 47a690459a8..b4da0a6b033 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java @@ -140,7 +140,7 @@ class RunSerializer { runObject.setLong(startField, run.start().toEpochMilli()); run.end().ifPresent(end -> runObject.setLong(endField, end.toEpochMilli())); runObject.setString(statusField, valueOf(run.status())); - runObject.setLong(lastTestRecordField, run.lastTestRecord()); + runObject.setLong(lastTestRecordField, run.lastTestLogEntry()); Cursor stepsObject = runObject.setObject(stepsField); run.steps().forEach((step, status) -> stepsObject.setString(valueOf(step), valueOf(status))); 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 3391eee4d62..25fb15b283c 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 @@ -174,7 +174,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return application(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job")) return JobControllerApiHandlerHelper.jobTypeResponse(jobTypes(path), latestRuns(path), request.getUri()); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}")) return JobControllerApiHandlerHelper.runResponse(controller.jobController().runs(appIdFromPath(path), jobTypeFromPath(path)), request.getUri()); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}/run/{number}")) return JobControllerApiHandlerHelper.runDetailsResponse(controller.jobController(), runIdFromPath(path)); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}/run/{number}")) return JobControllerApiHandlerHelper.runDetailsResponse(controller.jobController(), runIdFromPath(path), request.getProperty("after")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}")) return deployment(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/service")) return services(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/service/{service}/{*}")) return service(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.getRest(), request); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java index 1304ebf9e1c..68ba8ab83f3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java @@ -10,7 +10,8 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.SourceRevision; import com.yahoo.vespa.hosted.controller.deployment.JobController; -import com.yahoo.vespa.hosted.controller.deployment.RunDetails; +import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; +import com.yahoo.vespa.hosted.controller.deployment.RunLog; import com.yahoo.vespa.hosted.controller.deployment.Run; import com.yahoo.vespa.hosted.controller.deployment.Step; import com.yahoo.vespa.hosted.controller.restapi.SlimeJsonResponse; @@ -101,21 +102,35 @@ class JobControllerApiHandlerHelper { /** * @return Response with logs from a single run */ - static HttpResponse runDetailsResponse(JobController jobController, RunId runId) { + static HttpResponse runDetailsResponse(JobController jobController, RunId runId, String after) { Slime slime = new Slime(); Cursor logsObject = slime.setObject(); - RunDetails runDetails = jobController.details(runId).orElseThrow(() -> - new NotExistsException(String.format( + logsObject.setBool("active", jobController.active(runId).isPresent()); + + RunLog runLog = (after == null ? jobController.details(runId) : jobController.details(runId, Long.parseLong(after))) + .orElseThrow(() -> new NotExistsException(String.format( "No run details exist for application: %s, job type: %s, number: %d", runId.application().toShortString(), runId.type().jobName(), runId.number()))); + for (Step step : Step.values()) { - runDetails.get(step).ifPresent(stepLog -> logsObject.setString(step.name(), stepLog)); + runLog.get(step).ifPresent(entries -> toSlime(logsObject.setArray(step.name()), entries)); } + runLog.lastId().ifPresent(id -> logsObject.setLong("lastId", id)); return new SlimeJsonResponse(slime); } + private static void toSlime(Cursor entryArray, List<LogEntry> entries) { + entries.forEach(entry -> toSlime(entryArray.addObject(), entry)); + } + + private static void toSlime(Cursor entryObject, LogEntry entry) { + entryObject.setLong("at", entry.at()); + entryObject.setString("level", entry.level().getName()); + entryObject.setString("message", entry.message()); + } + /** * Unpack payload and submit to job controller. Defaults instance to 'default' and renders the * application version on success. diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java index 7256c283ab7..a36a8d8384f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java @@ -117,10 +117,11 @@ public class ControllerAuthorizationFilter extends CorsRequestFilterBase { private static boolean isHostedOperatorOperation(Path path, Method method) { if (isWhiteListedOperation(path, method)) return false; return path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying") || - path.matches("/controller/v1/{*}") || - path.matches("/provision/v2/{*}") || - path.matches("/screwdriver/v1/trigger/tenant/{*}") || - path.matches("/zone/v2/{*}"); + path.matches("/controller/v1/{*}") || + path.matches("/provision/v2/{*}") || + path.matches("/screwdriver/v1/trigger/tenant/{*}") || + path.matches("/os/v1/{*}") || + path.matches("/zone/v2/{*}"); } private static boolean isTenantAdminOperation(Path path, Method method) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java new file mode 100644 index 00000000000..3283fd4fcc4 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java @@ -0,0 +1,127 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.restapi.os; + +import com.yahoo.component.Version; +import com.yahoo.container.jdisc.HttpRequest; +import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.container.jdisc.LoggingRequestHandler; +import com.yahoo.io.IOUtils; +import com.yahoo.restapi.Path; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Inspector; +import com.yahoo.slime.Slime; +import com.yahoo.vespa.config.SlimeUtils; +import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.api.integration.zone.CloudName; +import com.yahoo.vespa.hosted.controller.restapi.ErrorResponse; +import com.yahoo.vespa.hosted.controller.restapi.SlimeJsonResponse; +import com.yahoo.vespa.hosted.controller.versions.OsVersion; +import com.yahoo.yolean.Exceptions; + +import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; +import java.util.Set; +import java.util.logging.Level; + +/** + * This implements the /os/v1 API which provides operators with information about, and scheduling of OS upgrades for + * nodes in the system. + * + * @author mpolden + */ +@SuppressWarnings("unused") // Injected +public class OsApiHandler extends LoggingRequestHandler { + + private final Controller controller; + + public OsApiHandler(Context ctx, Controller controller) { + super(ctx); + this.controller = controller; + } + + @Override + public HttpResponse handle(HttpRequest request) { + try { + switch (request.getMethod()) { + case GET: return get(request); + case PATCH: return patch(request); + default: return ErrorResponse.methodNotAllowed("Method '" + request.getMethod() + "' is unsupported"); + } + } catch (IllegalArgumentException e) { + return ErrorResponse.badRequest(Exceptions.toMessageString(e)); + } catch (RuntimeException e) { + log.log(Level.WARNING, "Unexpected error handling '" + request.getUri() + "'", e); + return ErrorResponse.internalServerError(Exceptions.toMessageString(e)); + } + } + + private HttpResponse patch(HttpRequest request) { + Path path = new Path(request.getUri().getPath()); + if (path.matches("/os/v1/")) return new SlimeJsonResponse(setOsVersion(request)); + return ErrorResponse.notFoundError("Nothing at " + path); + } + + private HttpResponse get(HttpRequest request) { + Path path = new Path(request.getUri().getPath()); + if (path.matches("/os/v1/")) return new SlimeJsonResponse(osVersions()); + return ErrorResponse.notFoundError("Nothing at " + path); + } + + private Slime setOsVersion(HttpRequest request) { + Slime requestData = toSlime(request.getData()); + Inspector root = requestData.get(); + Inspector versionField = root.field("version"); + Inspector cloudField = root.field("cloud"); + if (!versionField.valid() || !cloudField.valid()) { + throw new IllegalArgumentException("Fields 'version' and 'cloud' are required"); + } + + CloudName cloud = CloudName.from(cloudField.asString()); + Version target; + try { + target = Version.fromString(versionField.asString()); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException("Invalid version '" + versionField.asString() + "'", e); + } + + controller.upgradeOsIn(cloud, target); + Slime response = new Slime(); + Cursor cursor = response.setObject(); + cursor.setString("message", "Set target OS version for cloud '" + cloud.value() + "' to " + + target.toFullString()); + return response; + } + + private Slime osVersions() { + Slime slime = new Slime(); + Cursor root = slime.setObject(); + Set<OsVersion> osVersions = controller.osVersions(); + + Cursor versions = root.setArray("versions"); + controller.osVersionStatus().versions().forEach((osVersion, nodes) -> { + Cursor currentVersionObject = versions.addObject(); + currentVersionObject.setString("version", osVersion.version().toFullString()); + currentVersionObject.setBool("targetVersion", osVersions.contains(osVersion)); + currentVersionObject.setString("cloud", osVersion.cloud().value()); + Cursor nodesArray = currentVersionObject.setArray("nodes"); + nodes.forEach(node -> { + Cursor nodeObject = nodesArray.addObject(); + nodeObject.setString("hostname", node.hostname().value()); + nodeObject.setString("environment", node.environment().value()); + nodeObject.setString("region", node.region().value()); + }); + }); + + return slime; + } + + private static Slime toSlime(InputStream json) { + try { + return SlimeUtils.jsonToSlime(IOUtils.readBytes(json, 1000 * 1000)); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java index 3d4fe457726..f682224e5e9 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java @@ -26,7 +26,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.organization.MockOrgani import com.yahoo.vespa.hosted.controller.api.integration.routing.MemoryGlobalRoutingService; import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGenerator; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockBuildService; -import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockLogStore; +import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockRunDataStore; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzClientFactoryMock; @@ -35,19 +35,22 @@ import com.yahoo.vespa.hosted.controller.integration.ApplicationStoreMock; import com.yahoo.vespa.hosted.controller.integration.ArtifactRepositoryMock; import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock; import com.yahoo.vespa.hosted.controller.integration.MetricsServiceMock; +import com.yahoo.vespa.hosted.controller.integration.RoutingGeneratorMock; import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock; import com.yahoo.vespa.hosted.controller.persistence.ApplicationSerializer; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; -import com.yahoo.vespa.hosted.controller.integration.RoutingGeneratorMock; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import com.yahoo.vespa.hosted.rotation.config.RotationsConfig; +import java.util.Arrays; import java.util.Optional; import java.util.OptionalLong; +import java.util.function.Consumer; import java.util.function.Supplier; +import java.util.logging.Handler; import java.util.logging.Logger; import static org.junit.Assert.assertNotNull; @@ -128,11 +131,19 @@ public final class ControllerTester { metricsService, routingGenerator); // Make root logger use time from manual clock - Logger.getLogger("").getHandlers()[0].setFilter( + configureDefaultLogHandler(handler -> handler.setFilter( record -> { record.setMillis(clock.millis()); return true; - }); + })); + } + + public void configureDefaultLogHandler(Consumer<Handler> configureFunc) { + Arrays.stream(Logger.getLogger("").getHandlers()) + // Do not mess with log configuration if a custom one has been set + .filter(ignored -> System.getProperty("java.util.logging.config.file") == null) + .findFirst() + .ifPresent(configureFunc); } public static BuildService.BuildJob buildJob(Application application, JobType jobType) { @@ -301,7 +312,7 @@ public final class ControllerTester { artifactRepository, applicationStore, buildService, - new MockLogStore(), + new MockRunDataStore(), () -> "test-controller"); // Calculate initial versions controller.updateVersionStatus(VersionStatus.compute(controller)); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java index ba0607892e6..ce0446e41b6 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java @@ -15,6 +15,7 @@ import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbi import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.RestartAction; import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.ServiceInfo; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; +import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud; @@ -37,18 +38,19 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.time.Duration; +import java.util.Arrays; import java.util.Collections; import java.util.Optional; import java.util.logging.Logger; -import java.util.stream.Collectors; -import java.util.stream.Stream; import static com.yahoo.log.LogLevel.DEBUG; +import static com.yahoo.log.LogLevel.ERROR; import static com.yahoo.vespa.hosted.controller.deployment.InternalStepRunner.testerOf; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.aborted; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.failed; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.succeeded; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.unfinished; +import static java.util.logging.Level.INFO; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -87,7 +89,7 @@ public class InternalStepRunnerTest { // Get deployment job logs to stderr. Logger.getLogger(InternalStepRunner.class.getName()).setLevel(DEBUG); Logger.getLogger("").setLevel(DEBUG); - Logger.getLogger("").getHandlers()[0].setLevel(DEBUG); + tester.controllerTester().configureDefaultLogHandler(handler -> handler.setLevel(DEBUG)); } @Test @@ -210,7 +212,7 @@ public class InternalStepRunnerTest { assertEquals(Step.Status.succeeded, jobs.active(run.id()).get().steps().get(Step.startTests)); assertEquals(unfinished, jobs.active(run.id()).get().steps().get(Step.endTests)); - cloud.set("Awesome!".getBytes(), TesterCloud.Status.SUCCESS); + cloud.set(TesterCloud.Status.SUCCESS); runner.run(); assertTrue(jobs.run(run.id()).get().hasEnded()); assertFalse(jobs.run(run.id()).get().hasFailed()); @@ -319,9 +321,17 @@ public class InternalStepRunnerTest { } @Test - public void testsFailIfEndpointsAreGone() { + public void testsFailIfTesterEndpointsVanish() { RunId id = startSystemTestTests(); - cloud.set(new byte[0], TesterCloud.Status.NOT_STARTED); + routing.removeEndpoints(new DeploymentId(testerOf(appId), JobType.systemTest.zone(tester.controller().system()))); + runner.run(); + assertEquals(failed, jobs.run(id).get().steps().get(Step.endTests)); + } + + @Test + public void testsFailIfTesterRestarts() { + RunId id = startSystemTestTests(); + cloud.set(TesterCloud.Status.NOT_STARTED); runner.run(); assertEquals(failed, jobs.run(id).get().steps().get(Step.endTests)); } @@ -329,19 +339,29 @@ public class InternalStepRunnerTest { @Test public void testsFailIfTestsFailRemotely() { RunId id = startSystemTestTests(); - cloud.set("Failure!".getBytes(), TesterCloud.Status.FAILURE); + cloud.add(new LogEntry(123, 321, ERROR, "Failure!")); + cloud.set(TesterCloud.Status.FAILURE); + + long lastId = jobs.details(id).get().lastId().getAsLong(); runner.run(); + assertTestLogEntries(id, Step.endTests, + new LogEntry(lastId + 1, 321, ERROR, "Failure!"), + new LogEntry(lastId + 2, tester.clock().millis(), DEBUG, "Tests failed.")); assertEquals(failed, jobs.run(id).get().steps().get(Step.endTests)); - assertLogMessages(id, Step.endTests, "Tests failed.", "Failure!"); } @Test public void testsFailIfTestsErr() { RunId id = startSystemTestTests(); - cloud.set("Error!".getBytes(), TesterCloud.Status.ERROR); + cloud.add(new LogEntry(0, 123, ERROR, "Error!")); + cloud.set(TesterCloud.Status.ERROR); + + long lastId = jobs.details(id).get().lastId().getAsLong(); runner.run(); assertEquals(failed, jobs.run(id).get().steps().get(Step.endTests)); - assertLogMessages(id, Step.endTests, "Tester failed running its tests!", "Error!"); + assertTestLogEntries(id, Step.endTests, + new LogEntry(lastId + 1, 123, ERROR, "Error!"), + new LogEntry(lastId + 2, tester.clock().millis(), INFO, "Tester failed running its tests!")); } @Test @@ -360,19 +380,31 @@ public class InternalStepRunnerTest { configObject.field("endpoints").field(JobType.systemTest.zone(tester.controller().system()).value()).traverse((ArrayTraverser) (__, endpoint) -> assertEquals(routing.endpoints(new DeploymentId(appId, JobType.systemTest.zone(tester.controller().system()))).get(0).getEndpoint(), endpoint.asString())); - cloud.set("Success!".getBytes(), TesterCloud.Status.SUCCESS); + long lastId = jobs.details(id).get().lastId().getAsLong(); + cloud.add(new LogEntry(0, 123, INFO, "Ready!")); + runner.run(); + assertTestLogEntries(id, Step.endTests, + new LogEntry(lastId + 1, 123, INFO, "Ready!")); + + cloud.add(new LogEntry(1, 1234, INFO, "Steady!")); + runner.run(); + assertTestLogEntries(id, Step.endTests, + new LogEntry(lastId + 1, 123, INFO, "Ready!"), + new LogEntry(lastId + 2, 1234, INFO, "Steady!")); + + cloud.add(new LogEntry(12, 12345, INFO, "Success!")); + cloud.set(TesterCloud.Status.SUCCESS); runner.run(); + assertTestLogEntries(id, Step.endTests, + new LogEntry(lastId + 1, 123, INFO, "Ready!"), + new LogEntry(lastId + 2, 1234, INFO, "Steady!"), + new LogEntry(lastId + 3, 12345, INFO, "Success!"), + new LogEntry(lastId + 4, tester.clock().millis(), DEBUG, "Tests completed successfully.")); assertEquals(succeeded, jobs.run(id).get().steps().get(Step.endTests)); - assertLogMessages(id, Step.endTests, "Tests completed successfully.", "Success!"); } - private void assertLogMessages(RunId id, Step step, String... messages) { - String pattern = Stream.of(messages) - .map(message -> "\\[[^]]*] : " + message + "\n") - .collect(Collectors.joining()); - String logs = new String(jobs.details(id).get().get(step).get()); - if ( ! logs.matches(pattern)) - throw new AssertionError("Expected a match for\n'''\n" + pattern + "\n'''\nbut got\n'''\n" + logs + "\n'''"); + private void assertTestLogEntries(RunId id, Step step, LogEntry... entries) { + assertEquals(Arrays.asList(entries), jobs.details(id).get().get(step).get()); } private RunId startSystemTestTests() { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java index a13d9f7a19c..cb6d971c2d9 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java @@ -1,11 +1,13 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.integration; +import com.google.common.collect.ImmutableList; import com.google.inject.Inject; import com.yahoo.component.AbstractComponent; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; +import com.yahoo.vespa.hosted.controller.api.integration.zone.CloudName; import com.yahoo.vespa.hosted.controller.api.integration.zone.UpgradePolicy; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; @@ -34,6 +36,7 @@ public class ZoneRegistryMock extends AbstractComponent implements ZoneRegistry private List<ZoneId> zones = new ArrayList<>(); private SystemName system = SystemName.main; private UpgradePolicy upgradePolicy = null; + private Map<CloudName, UpgradePolicy> osUpgradePolicies = new HashMap<>(); @Inject public ZoneRegistryMock() { @@ -73,6 +76,11 @@ public class ZoneRegistryMock extends AbstractComponent implements ZoneRegistry return this; } + public ZoneRegistryMock setOsUpgradePolicy(CloudName cloud, UpgradePolicy upgradePolicy) { + osUpgradePolicies.put(cloud, upgradePolicy); + return this; + } + @Override public SystemName system() { return system; @@ -93,8 +101,13 @@ public class ZoneRegistryMock extends AbstractComponent implements ZoneRegistry } @Override - public UpgradePolicy osUpgradePolicy() { - return upgradePolicy; + public UpgradePolicy osUpgradePolicy(CloudName cloud) { + return osUpgradePolicies.get(cloud); + } + + @Override + public List<UpgradePolicy> osUpgradePolicies() { + return ImmutableList.copyOf(osUpgradePolicies.values()); } @Override diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java index fa6edd939c4..7ab858f3081 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java @@ -3,18 +3,19 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; +import com.yahoo.component.Version; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.SystemName; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.ControllerTester; -import com.yahoo.vespa.hosted.controller.integration.MetricsMock; -import com.yahoo.vespa.hosted.controller.integration.MetricsMock.MapContext; import com.yahoo.vespa.hosted.controller.api.integration.chef.ChefMock; import com.yahoo.vespa.hosted.controller.api.integration.chef.rest.PartialNodeResult; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; +import com.yahoo.vespa.hosted.controller.integration.MetricsMock; +import com.yahoo.vespa.hosted.controller.integration.MetricsMock.MapContext; import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; import org.junit.Before; import org.junit.Test; @@ -43,6 +44,7 @@ import static org.junit.Assert.assertNull; public class MetricsReporterTest { private static final Path testData = Paths.get("src/test/resources/"); + private MetricsMock metrics; @Before @@ -75,7 +77,7 @@ public class MetricsReporterTest { .environment(Environment.prod) .region("us-west-1") .build(); - MetricsReporter metricsReporter = createReporter(tester.controller(), metrics, SystemName.cd); + MetricsReporter metricsReporter = createReporter(tester.controller(), metrics, SystemName.main); metricsReporter.maintain(); assertEquals(0.0, metrics.getMetric(MetricsReporter.deploymentFailMetric)); @@ -102,7 +104,7 @@ public class MetricsReporterTest { } @Test - public void it_omits_zone_when_unknown() { + public void test_chef_metrics_omit_zone_when_unknown() { ControllerTester tester = new ControllerTester(); String hostname = "fake-node2.test"; MapContext metricContext = getMetricContextByHost(tester.controller(), hostname); @@ -117,7 +119,7 @@ public class MetricsReporterTest { .region("us-west-1") .build(); - MetricsReporter reporter = createReporter(tester.controller(), metrics, SystemName.cd); + MetricsReporter reporter = createReporter(tester.controller(), metrics, SystemName.main); Application app = tester.createApplication("app1", "tenant1", 1, 11L); tester.deployCompletely(app, applicationPackage); @@ -136,7 +138,7 @@ public class MetricsReporterTest { tester.deployAndNotify(app, applicationPackage, true, productionUsWest1); reporter.maintain(); - // Average time is 1 hour + // Average time is 1 hour (system-test) + 90 minutes (staging-test runs in parallel with system-test) + 90 minutes (production) / 3 jobs assertEquals(Duration.ofMinutes(80), getAverageDeploymentDuration(app)); // Another deployment starts and stalls for 12 hours @@ -151,11 +153,69 @@ public class MetricsReporterTest { getAverageDeploymentDuration(app)); } + @Test + public void test_deployments_failing_upgrade() { + DeploymentTester tester = new DeploymentTester(); + ApplicationPackage applicationPackage = new ApplicationPackageBuilder() + .environment(Environment.prod) + .region("us-west-1") + .build(); + + MetricsReporter reporter = createReporter(tester.controller(), metrics, SystemName.main); + Application app = tester.createApplication("app1", "tenant1", 1, 11L); + + // Initial deployment without failures + tester.deployCompletely(app, applicationPackage); + reporter.maintain(); + assertEquals(0, getDeploymentsFailingUpgrade(app)); + + // Failing application change is not counted + tester.jobCompletion(component).application(app).nextBuildNumber().uploadArtifact(applicationPackage).submit(); + tester.deployAndNotify(app, applicationPackage, false, systemTest); + reporter.maintain(); + assertEquals(0, getDeploymentsFailingUpgrade(app)); + + // Application change completes + tester.deployAndNotify(app, applicationPackage, true, systemTest); + tester.deployAndNotify(app, applicationPackage, true, stagingTest); + tester.deployAndNotify(app, applicationPackage, true, productionUsWest1); + assertFalse("Change deployed", tester.controller().applications().require(app.id()).change().isPresent()); + + // New versions is released and upgrade fails in test environments + Version version = Version.fromString("7.1"); + tester.upgradeSystem(version); + tester.upgrader().maintain(); + tester.deployAndNotify(app, applicationPackage, false, systemTest); + tester.deployAndNotify(app, applicationPackage, false, stagingTest); + reporter.maintain(); + assertEquals(2, getDeploymentsFailingUpgrade(app)); + + // Test and staging pass and upgrade fails in production + tester.deployAndNotify(app, applicationPackage, true, systemTest); + tester.deployAndNotify(app, applicationPackage, true, stagingTest); + tester.deployAndNotify(app, applicationPackage, false, productionUsWest1); + reporter.maintain(); + assertEquals(1, getDeploymentsFailingUpgrade(app)); + + // Upgrade eventually succeeds + tester.deployAndNotify(app, applicationPackage, true, productionUsWest1); + assertFalse("Upgrade deployed", tester.controller().applications().require(app.id()).change().isPresent()); + reporter.maintain(); + assertEquals(0, getDeploymentsFailingUpgrade(app)); + } + private Duration getAverageDeploymentDuration(Application application) { + return Duration.ofSeconds(getMetric(MetricsReporter.deploymentAverageDuration, application).longValue()); + } + + private int getDeploymentsFailingUpgrade(Application application) { + return getMetric(MetricsReporter.deploymentFailingUpgrades, application).intValue(); + } + + private Number getMetric(String name, Application application) { return metrics.getMetric((dimensions) -> application.id().tenant().value().equals(dimensions.get("tenant")) && appDimension(application).equals(dimensions.get("app")), - MetricsReporter.deploymentAverageDuration) - .map(seconds -> Duration.ofSeconds(seconds.longValue())) + name) .orElseThrow(() -> new RuntimeException("Expected metric to exist for " + application.id())); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java index 44823ab5777..045386dd93a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java @@ -210,7 +210,7 @@ public class OsUpgraderTest { tester.controllerTester().zoneRegistry() .setZones(zone1, zone2, zone3, zone4, zone5) .setSystemName(system) - .setUpgradePolicy(upgradePolicy); + .setOsUpgradePolicy(CloudName.defaultName(), upgradePolicy); return new OsUpgrader(tester.controller(), Duration.ofDays(1), new JobControl(tester.controllerTester().curator()), CloudName.defaultName()); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java index 98ed64ba879..97084ea4ae9 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java @@ -35,7 +35,7 @@ public class OsVersionStatusUpdaterTest { for (ZoneId zone : tester.zoneRegistry().zones().controllerUpgraded().ids()) { upgradePolicy = upgradePolicy.upgrade(zone); } - tester.zoneRegistry().setUpgradePolicy(upgradePolicy); + tester.zoneRegistry().setOsUpgradePolicy(CloudName.defaultName(), upgradePolicy); // Initially empty assertSame(OsVersionStatus.empty, tester.controller().osVersionStatus()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStoreTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStoreTest.java new file mode 100644 index 00000000000..6147c50120e --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStoreTest.java @@ -0,0 +1,84 @@ +package com.yahoo.vespa.hosted.controller.persistence; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.vespa.hosted.controller.api.integration.RunDataStore; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; +import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockRunDataStore; +import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; +import com.yahoo.vespa.hosted.controller.deployment.RunLog; +import com.yahoo.vespa.hosted.controller.deployment.Step; +import org.junit.Test; + +import java.util.Arrays; +import java.util.Collections; +import java.util.Optional; +import java.util.logging.Level; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; + +public class BufferedLogStoreTest { + + @Test + public void chunkingAndFlush() { + CuratorDb buffer = new MockCuratorDb(); + RunDataStore store = new MockRunDataStore(); + BufferedLogStore logs = new BufferedLogStore(buffer, store); + RunId id = new RunId(ApplicationId.from("tenant", "application", "instance"), + JobType.productionUsWest1, + 123); + + byte[] manyBytes = new byte[BufferedLogStore.chunkSize / 2 + 1]; // One fits, and two (over-)fills. + Arrays.fill(manyBytes, (byte) 'O'); + LogEntry entry = new LogEntry(0, 123, Level.WARNING, new String(manyBytes)); + + // Log entries are re-sequenced by the log store, by enumeration. + LogEntry entry0 = new LogEntry(0, entry.at(), entry.level(), entry.message()); + LogEntry entry1 = new LogEntry(1, entry.at(), entry.level(), entry.message()); + LogEntry entry2 = new LogEntry(2, entry.at(), entry.level(), entry.message()); + + assertEquals(Optional.empty(), logs.readFinished(id, -1)); + assertEquals(RunLog.empty(), logs.readActive(id.application(), id.type(), -1)); + + logs.append(id.application(), id.type(), Step.deployReal, Collections.singletonList(entry)); + assertEquals(Arrays.asList(entry0), + logs.readActive(id.application(), id.type(), -1).get(Step.deployReal).get()); + assertEquals(RunLog.empty(), logs.readActive(id.application(), id.type(), 0)); + + logs.append(id.application(), id.type(), Step.deployReal, Collections.singletonList(entry)); + assertEquals(Arrays.asList(entry0, entry1), + logs.readActive(id.application(), id.type(), -1).get(Step.deployReal).get()); + assertEquals(Arrays.asList(entry1), + logs.readActive(id.application(), id.type(), 0).get(Step.deployReal).get()); + assertEquals(RunLog.empty(), logs.readActive(id.application(), id.type(), 1)); + + logs.append(id.application(), id.type(), Step.deployReal, Collections.singletonList(entry)); + assertEquals(Arrays.asList(entry0, entry1, entry2), + logs.readActive(id.application(), id.type(), -1).get(Step.deployReal).get()); + assertEquals(Arrays.asList(entry1, entry2), + logs.readActive(id.application(), id.type(), 0).get(Step.deployReal).get()); + assertEquals(Arrays.asList(entry2), + logs.readActive(id.application(), id.type(), 1).get(Step.deployReal).get()); + assertEquals(RunLog.empty(), logs.readActive(id.application(), id.type(), 2)); + + // We should now have two chunks, with two and one entries. + assertEquals(Optional.of(2L), buffer.readLastLogEntryId(id.application(), id.type())); + assertArrayEquals(new long[]{0, 2}, buffer.getLogChunkIds(id.application(), id.type()).toArray()); + + // Flushing clears the buffer entirely, and stores its aggregated content in the data store. + logs.flush(id); + assertEquals(Optional.empty(), buffer.readLastLogEntryId(id.application(), id.type())); + assertArrayEquals(new long[]{}, buffer.getLogChunkIds(id.application(), id.type()).toArray()); + assertEquals(RunLog.empty(), logs.readActive(id.application(), id.type(), -1)); + + assertEquals(Arrays.asList(entry0, entry1, entry2), + logs.readFinished(id, -1).get().get(Step.deployReal).get()); + assertEquals(Arrays.asList(entry1, entry2), + logs.readFinished(id, 0).get().get(Step.deployReal).get()); + assertEquals(Arrays.asList(entry2), + logs.readFinished(id, 1).get().get(Step.deployReal).get()); + assertEquals(Collections.emptyList(), logs.readFinished(id, 2).get().get(Step.deployReal).get()); + } + +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializerTest.java index 554e30637f9..64aaec97b09 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializerTest.java @@ -1,9 +1,7 @@ package com.yahoo.vespa.hosted.controller.persistence; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.yahoo.log.LogLevel; -import com.yahoo.vespa.config.SlimeUtils; +import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; import com.yahoo.vespa.hosted.controller.deployment.Step; import org.junit.Test; @@ -11,10 +9,11 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.logging.Level; -import java.util.logging.LogRecord; import static com.yahoo.vespa.hosted.controller.deployment.Step.deployReal; import static com.yahoo.vespa.hosted.controller.deployment.Step.deployTester; @@ -25,41 +24,38 @@ import static org.junit.Assert.assertEquals; */ public class LogSerializerTest { - private static final LogRecordSerializer serializer = new LogRecordSerializer(); + private static final LogSerializer serializer = new LogSerializer(); private static final Path logsFile = Paths.get("src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/logs.json"); @Test public void testSerialization() throws IOException { - // Local, because it's not supposed to be used for anything else than verifying equality here! - class EgalitarianLogRecord extends LogRecord { - private EgalitarianLogRecord(Level level, String msg) { - super(level, msg); - } - @Override - public boolean equals(Object o) { - if ( ! (o instanceof LogRecord)) return false; - LogRecord record = (LogRecord) o; - return getSequenceNumber() == record.getSequenceNumber() - && getLevel() == record.getLevel() - && getMillis() == record.getMillis() - && getMessage().equals(record.getMessage()); - } - @Override - public int hashCode() { throw new AssertionError(); } - } + byte[] logJson = Files.readAllBytes(logsFile); - LogRecord first = new EgalitarianLogRecord(LogLevel.INFO, "First"); first.setMillis( 0); first.setSequenceNumber(1); - LogRecord second = new EgalitarianLogRecord(LogLevel.INFO, "Second"); second.setMillis( 0); second.setSequenceNumber(2); - LogRecord third = new EgalitarianLogRecord(LogLevel.DEBUG, "Third"); third.setMillis(1000); third.setSequenceNumber(3); - LogRecord fourth = new EgalitarianLogRecord(LogLevel.WARNING, "Fourth"); fourth.setMillis(2000); fourth.setSequenceNumber(4); + LogEntry first = new LogEntry(0, 0, LogLevel.INFO, "First"); + LogEntry second = new LogEntry(1, 0, LogLevel.INFO, "Second"); + LogEntry third = new LogEntry(2, 1000, LogLevel.DEBUG, "Third"); + LogEntry fourth = new LogEntry(3, 2000, LogLevel.WARNING, "Fourth"); - Map<Step, List<LogRecord>> expected = ImmutableMap.of(deployReal, ImmutableList.of(first, third), - deployTester, ImmutableList.of(second, fourth)); + Map<Step, List<LogEntry>> expected = new HashMap<>(); + expected.put(deployReal, new ArrayList<>()); + expected.get(deployReal).add(third); + expected.put(deployTester, new ArrayList<>()); + expected.get(deployTester).add(fourth); - Map<Step, List<LogRecord>> stepRecords = serializer.recordsFromSlime(SlimeUtils.jsonToSlime(Files.readAllBytes(logsFile))); - assertEquals(expected, stepRecords); + assertEquals(expected, serializer.fromJson(logJson, 1)); - assertEquals(expected, serializer.recordsFromSlime(serializer.recordsToSlime(stepRecords))); + expected.get(deployReal).add(0, first); + expected.get(deployTester).add(0, second); + assertEquals(expected, serializer.fromJson(logJson, -1)); + + assertEquals(expected, serializer.fromJson(serializer.toJson(expected), -1)); + + expected.get(deployReal).add(first); + expected.get(deployReal).add(third); + expected.get(deployTester).add(second); + expected.get(deployTester).add(fourth); + + assertEquals(expected, serializer.fromJson(Arrays.asList(logJson, logJson), -1)); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java index a1fa4c1cd29..82aee3b3550 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java @@ -69,7 +69,7 @@ public class RunSerializerTest { assertEquals(start, run.start()); assertFalse(run.hasEnded()); assertEquals(running, run.status()); - assertEquals(3, run.lastTestRecord()); + assertEquals(3, run.lastTestLogEntry()); assertEquals(new Version(1, 2, 3), run.versions().targetPlatform()); assertEquals(ApplicationVersion.from(new SourceRevision("git@github.com:user/repo.git", "master", @@ -106,7 +106,7 @@ public class RunSerializerTest { assertEquals(run.start(), phoenix.start()); assertEquals(run.end(), phoenix.end()); assertEquals(run.status(), phoenix.status()); - assertEquals(run.lastTestRecord(), phoenix.lastTestRecord()); + assertEquals(run.lastTestLogEntry(), phoenix.lastTestLogEntry()); assertEquals(run.versions(), phoenix.versions()); assertEquals(run.steps(), phoenix.steps()); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/logs.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/logs.json index 1135e63e464..a6a092109a1 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/logs.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/logs.json @@ -2,13 +2,13 @@ "deployReal": [ { - "id": 1, + "id": 0, "level": "info", "at": 0, "message": "First" }, { - "id": 3, + "id": 2, "level": "debug", "at": 1000, "message": "Third" @@ -17,13 +17,13 @@ "deployTester": [ { - "id": 2, + "id": 1, "level": "info", "at": 0, "message": "Second" }, { - "id": 4, + "id": 3, "level": "warning", "at": 2000, "message": "Fourth" diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java index 60a05e4f938..b571e6f1c48 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java @@ -8,7 +8,6 @@ import com.yahoo.config.provision.TenantName; import com.yahoo.vespa.athenz.api.AthenzDomain; import com.yahoo.vespa.athenz.utils.AthenzIdentities; import com.yahoo.vespa.hosted.controller.Application; -import com.yahoo.vespa.hosted.controller.integration.ArtifactRepositoryMock; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.TestIdentities; import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions; @@ -21,18 +20,18 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockBuildService; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; -import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzClientFactoryMock; import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzDbMock; import com.yahoo.vespa.hosted.controller.deployment.BuildJob; +import com.yahoo.vespa.hosted.controller.integration.ArtifactRepositoryMock; import com.yahoo.vespa.hosted.controller.maintenance.JobControl; import com.yahoo.vespa.hosted.controller.maintenance.Upgrader; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; +import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import java.io.File; -import java.io.IOException; import java.time.Duration; import java.util.Optional; @@ -96,11 +95,11 @@ public class ContainerControllerTester { // ---- Delegators: - public void assertResponse(Request request, File expectedResponse) throws IOException { + public void assertResponse(Request request, File expectedResponse) { containerTester.assertResponse(request, expectedResponse); } - public void assertResponse(Request request, String expectedResponse, int expectedStatusCode) throws IOException { + public void assertResponse(Request request, String expectedResponse, int expectedStatusCode) { containerTester.assertResponse(request, expectedResponse, expectedStatusCode); } 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 cc275b0636f..f65cf6af346 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 @@ -110,23 +110,27 @@ public class ContainerTester { assertEquals("Status code", expectedStatusCode, response.getStatus()); } - public void assertResponse(Supplier<Request> request, String expectedResponse) throws IOException { + public void assertResponse(Supplier<Request> request, String expectedResponse) { assertResponse(request.get(), expectedResponse, 200); } - public void assertResponse(Request request, String expectedResponse) throws IOException { + public void assertResponse(Request request, String expectedResponse) { assertResponse(request, expectedResponse, 200); } - public void assertResponse(Supplier<Request> request, String expectedResponse, int expectedStatusCode) throws IOException { + public void assertResponse(Supplier<Request> request, String expectedResponse, int expectedStatusCode) { assertResponse(request.get(), expectedResponse, expectedStatusCode); } - public void assertResponse(Request request, String expectedResponse, int expectedStatusCode) throws IOException { + public void assertResponse(Request request, String expectedResponse, int expectedStatusCode) { FilterResult filterResult = invokeSecurityFilters(request); request = filterResult.request; Response response = filterResult.response != null ? filterResult.response : container.handleRequest(request); - assertEquals(expectedResponse, response.getBodyAsString()); + try { + assertEquals(expectedResponse, response.getBodyAsString()); + } catch (IOException e) { + throw new UncheckedIOException(e); + } assertEquals("Status code", expectedStatusCode, response.getStatus()); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java index 978e8bce2f4..11286acda3a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java @@ -31,7 +31,8 @@ import static org.junit.Assert.assertEquals; */ public class ControllerContainerTest { - public static final AthenzUser USER = AthenzUser.fromUserId("bob"); + private static final AthenzUser defaultUser = AthenzUser.fromUserId("bob"); + protected JDisc container; @Before @@ -68,7 +69,7 @@ public class ControllerContainerTest { " <component id='com.yahoo.vespa.hosted.controller.api.integration.routing.MemoryGlobalRoutingService'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.api.integration.stubs.LoggingDeploymentIssues'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.api.integration.stubs.DummyOwnershipIssues'/>\n" + - " <component id='com.yahoo.vespa.hosted.controller.api.integration.stubs.MockLogStore'/>\n" + + " <component id='com.yahoo.vespa.hosted.controller.api.integration.stubs.MockRunDataStore'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.api.integration.organization.MockOrganization'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.integration.ConfigServerMock'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.integration.NodeRepositoryClientMock'/>\n" + @@ -92,6 +93,9 @@ public class ControllerContainerTest { " <handler id='com.yahoo.vespa.hosted.controller.restapi.controller.ControllerApiHandler'>\n" + " <binding>http://*/controller/v1/*</binding>\n" + " </handler>\n" + + " <handler id='com.yahoo.vespa.hosted.controller.restapi.os.OsApiHandler'>\n" + + " <binding>http://*/os/v1/*</binding>\n" + + " </handler>\n" + " <handler id='com.yahoo.vespa.hosted.controller.restapi.screwdriver.ScrewdriverApiHandler'>\n" + " <binding>http://*/screwdriver/v1/*</binding>\n" + " </handler>\n" + @@ -127,11 +131,11 @@ public class ControllerContainerTest { } protected static Request authenticatedRequest(String uri) { - return addIdentityToRequest(new Request(uri), USER); + return addIdentityToRequest(new Request(uri), defaultUser); } protected static Request authenticatedRequest(String uri, byte[] body, Request.Method method) { - return addIdentityToRequest(new Request(uri, body, method), USER); + return addIdentityToRequest(new Request(uri, body, method), defaultUser); } protected static Request addIdentityToRequest(Request request, AthenzIdentity identity) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java index 9e8dfad82c1..bfa4a694208 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java @@ -7,14 +7,16 @@ import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.vespa.hosted.controller.ControllerTester; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; -import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockLogStore; +import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockRunDataStore; import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.SourceRevision; import com.yahoo.vespa.hosted.controller.deployment.JobController; +import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; import com.yahoo.vespa.hosted.controller.deployment.Run; import com.yahoo.vespa.hosted.controller.deployment.RunStatus; import com.yahoo.vespa.hosted.controller.deployment.Step; import com.yahoo.vespa.hosted.controller.deployment.Versions; +import com.yahoo.vespa.hosted.controller.persistence.BufferedLogStore; import org.json.JSONException; import org.json.JSONObject; import org.junit.Assert; @@ -33,6 +35,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.logging.Level; import static org.junit.Assert.fail; @@ -90,19 +93,21 @@ public class JobControllerApiHandlerHelperTest { @Test public void runDetailsResponse() { ControllerTester tester = new ControllerTester(); - MockLogStore logStore = new MockLogStore(); - JobController jobController = new JobController(tester.controller(), logStore); + MockRunDataStore dataStore = new MockRunDataStore(); + JobController jobController = new JobController(tester.controller(), dataStore); + BufferedLogStore logStore = new BufferedLogStore(tester.curator(), dataStore); RunId runId = new RunId(appId, JobType.systemTest, 42); tester.curator().writeHistoricRuns( runId.application(), runId.type(), Collections.singleton(createRun(JobType.systemTest, 42, 44, lastStep, Optional.of(RunStatus.running)))); - logStore.append(runId, Step.deployTester.name(), "INFO\t1234567890\tSUCCESS".getBytes()); - logStore.append(runId, Step.installTester.name(), "INFO\t1234598760\tSUCCESS".getBytes()); - logStore.append(runId, Step.deactivateTester.name(), "INFO\t1234678901\tERROR: Something went wrong".getBytes()); + logStore.append(appId, JobType.systemTest, Step.deployTester, Collections.singletonList(new LogEntry(0, 1, Level.INFO, "SUCCESS"))); + logStore.append(appId, JobType.systemTest, Step.installTester, Collections.singletonList(new LogEntry(0, 12, Level.FINE, "SUCCESS"))); + logStore.append(appId, JobType.systemTest, Step.deactivateTester, Collections.singletonList(new LogEntry(0, 123, Level.WARNING, "ERROR"))); + logStore.flush(runId); - HttpResponse response = JobControllerApiHandlerHelper.runDetailsResponse(jobController, runId); + HttpResponse response = JobControllerApiHandlerHelper.runDetailsResponse(jobController, runId,"0"); assertFile(response, "job/run-details-response.json"); } @@ -112,7 +117,7 @@ public class JobControllerApiHandlerHelperTest { tester.createTenant("tenant", "domain", 1L); tester.createApplication(TenantName.from("tenant"), "application", "default", 1L); - JobController jobController = new JobController(tester.controller(), new MockLogStore()); + JobController jobController = new JobController(tester.controller(), new MockRunDataStore()); HttpResponse response = JobControllerApiHandlerHelper.submitResponse( jobController, "tenant", "application", new SourceRevision("repository", "branch", "commit"), new byte[0], new byte[0]); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java new file mode 100644 index 00000000000..2d76517bfe7 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java @@ -0,0 +1,157 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.restapi.os; + + +import com.google.common.collect.ImmutableList; +import com.yahoo.application.container.handler.Request; +import com.yahoo.config.provision.SystemName; +import com.yahoo.vespa.athenz.api.AthenzIdentity; +import com.yahoo.vespa.athenz.api.AthenzUser; +import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node; +import com.yahoo.vespa.hosted.controller.api.integration.zone.CloudName; +import com.yahoo.vespa.hosted.controller.api.integration.zone.UpgradePolicy; +import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; +import com.yahoo.vespa.hosted.controller.application.SystemApplication; +import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock; +import com.yahoo.vespa.hosted.controller.integration.NodeRepositoryMock; +import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock; +import com.yahoo.vespa.hosted.controller.maintenance.JobControl; +import com.yahoo.vespa.hosted.controller.maintenance.Maintainer; +import com.yahoo.vespa.hosted.controller.maintenance.OsUpgrader; +import com.yahoo.vespa.hosted.controller.restapi.ContainerControllerTester; +import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; +import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus; +import org.intellij.lang.annotations.Language; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.time.Duration; +import java.util.List; + +/** + * @author mpolden + */ +public class OsApiTest extends ControllerContainerTest { + + private static final String responses = "src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/"; + private static final AthenzIdentity operator = AthenzUser.fromUserId("operatorUser"); + private static final CloudName cloud1 = CloudName.from("cloud1"); + private static final CloudName cloud2 = CloudName.from("cloud2"); + private static final ZoneId zone1 = ZoneId.from("prod", "us-east-3", cloud1.value()); + private static final ZoneId zone2 = ZoneId.from("prod", "us-west-1", cloud1.value()); + private static final ZoneId zone3 = ZoneId.from("prod", "eu-west-1", cloud2.value()); + + private ContainerControllerTester tester; + private List<OsUpgrader> osUpgraders; + + @Before + public void before() { + tester = new ContainerControllerTester(container, responses); + addUserToHostedOperatorRole(operator); + zoneRegistryMock().setSystemName(SystemName.cd) + .setZones(zone1, zone2, zone3) + .setOsUpgradePolicy(cloud1, UpgradePolicy.create().upgrade(zone1).upgrade(zone2)) + .setOsUpgradePolicy(cloud2, UpgradePolicy.create().upgrade(zone3)); + osUpgraders = ImmutableList.of( + new OsUpgrader(tester.controller(), Duration.ofDays(1), + new JobControl(tester.controller().curator()), + cloud1), + new OsUpgrader(tester.controller(), Duration.ofDays(1), + new JobControl(tester.controller().curator()), + cloud2)); + } + + @Test + public void test_api() { + // No versions available yet + assertResponse(new Request("http://localhost:8080/os/v1/"), "{\"versions\":[]}", 200); + + // All nodes are initially on empty version + upgradeAndUpdateStatus(); + assertFile(new Request("http://localhost:8080/os/v1/"), "versions-initial.json"); + + // Upgrade OS to a different version in each cloud + assertResponse(new Request("http://localhost:8080/os/v1/", "{\"version\": \"7.5.2\", \"cloud\": \"cloud1\"}", Request.Method.PATCH), + "{\"message\":\"Set target OS version for cloud 'cloud1' to 7.5.2\"}", 200); + assertResponse(new Request("http://localhost:8080/os/v1/", "{\"version\": \"8.2.1\", \"cloud\": \"cloud2\"}", Request.Method.PATCH), + "{\"message\":\"Set target OS version for cloud 'cloud2' to 8.2.1\"}", 200); + + // Status is updated after some zones are upgraded + upgradeAndUpdateStatus(); + completeUpgrade(zone1); + assertFile(new Request("http://localhost:8080/os/v1/"), "versions-partially-upgraded.json"); + + // All zones are upgraded + upgradeAndUpdateStatus(); + completeUpgrade(zone2, zone3); + assertFile(new Request("http://localhost:8080/os/v1/"), "versions-all-upgraded.json"); + + + // Error: Missing field 'cloud' or 'version' + assertResponse(new Request("http://localhost:8080/os/v1/", "{\"version\": \"7.6\"}", Request.Method.PATCH), + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Fields 'version' and 'cloud' are required\"}", 400); + assertResponse(new Request("http://localhost:8080/os/v1/", "{\"cloud\": \"cloud1\"}", Request.Method.PATCH), + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Fields 'version' and 'cloud' are required\"}", 400); + + // Error: Invalid versions + assertResponse(new Request("http://localhost:8080/os/v1/", "{\"version\": null, \"cloud\": \"cloud1\"}", Request.Method.PATCH), + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Invalid version '0.0.0'\"}", 400); + assertResponse(new Request("http://localhost:8080/os/v1/", "{\"version\": \"foo\", \"cloud\": \"cloud1\"}", Request.Method.PATCH), + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Invalid version 'foo': For input string: \\\"foo\\\"\"}", 400); + + // Error: Invalid cloud + assertResponse(new Request("http://localhost:8080/os/v1/", "{\"version\": \"7.6\", \"cloud\": \"foo\"}", Request.Method.PATCH), + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Cloud 'foo' does not exist in this system\"}", 400); + + // Error: Downgrade OS + assertResponse(new Request("http://localhost:8080/os/v1/", "{\"version\": \"7.4.1\", \"cloud\": \"cloud1\"}", Request.Method.PATCH), + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Cannot downgrade cloud 'cloud1' to version 7.4.1\"}", 400); + + } + + private void upgradeAndUpdateStatus() { + osUpgraders.forEach(Maintainer::run); + updateVersionStatus(); + } + + private void updateVersionStatus() { + tester.controller().updateOsVersionStatus(OsVersionStatus.compute(tester.controller())); + } + + private void completeUpgrade(ZoneId... zones) { + for (ZoneId zone : zones) { + for (SystemApplication application : SystemApplication.all()) { + for (Node node : nodeRepository().list(zone, application.id())) { + nodeRepository().putByHostname(zone, new Node( + node.hostname(), node.state(), node.type(), node.owner(), node.currentVersion(), + node.wantedVersion(), node.wantedOsVersion(), node.wantedOsVersion(), node.serviceState(), + node.restartGeneration(), node.wantedRestartGeneration(), node.rebootGeneration(), + node.wantedRebootGeneration())); + } + } + } + updateVersionStatus(); + } + + private ZoneRegistryMock zoneRegistryMock() { + return (ZoneRegistryMock) tester.containerTester().container().components() + .getComponent(ZoneRegistryMock.class.getName()); + } + + private NodeRepositoryMock nodeRepository() { + return ((ConfigServerMock) tester.containerTester().container().components() + .getComponent(ConfigServerMock.class.getName())).nodeRepository(); + } + + private void assertResponse(Request request, @Language("JSON") String body, int statusCode) { + addIdentityToRequest(request, operator); + tester.assertResponse(request, body, statusCode); + } + + private void assertFile(Request request, String filename) { + addIdentityToRequest(request, operator); + tester.assertResponse(request, new File(filename)); + } + +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-all-upgraded.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-all-upgraded.json new file mode 100644 index 00000000000..e1d81a874dc --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-all-upgraded.json @@ -0,0 +1,108 @@ +{ + "versions": [ + { + "version": "7.5.2", + "targetVersion": true, + "cloud": "cloud1", + "nodes": [ + { + "hostname": "node-3-configserver-host", + "environment": "prod", + "region": "us-east-3" + }, + { + "hostname": "node-2-configserver-host", + "environment": "prod", + "region": "us-east-3" + }, + { + "hostname": "node-1-configserver-host", + "environment": "prod", + "region": "us-east-3" + }, + { + "hostname": "node-3-configserver-host", + "environment": "prod", + "region": "us-west-1" + }, + { + "hostname": "node-2-configserver-host", + "environment": "prod", + "region": "us-west-1" + }, + { + "hostname": "node-1-configserver-host", + "environment": "prod", + "region": "us-west-1" + }, + { + "hostname": "node-2-proxy-host", + "environment": "prod", + "region": "us-east-3" + }, + { + "hostname": "node-1-proxy-host", + "environment": "prod", + "region": "us-east-3" + }, + { + "hostname": "node-3-proxy-host", + "environment": "prod", + "region": "us-east-3" + }, + { + "hostname": "node-2-proxy-host", + "environment": "prod", + "region": "us-west-1" + }, + { + "hostname": "node-1-proxy-host", + "environment": "prod", + "region": "us-west-1" + }, + { + "hostname": "node-3-proxy-host", + "environment": "prod", + "region": "us-west-1" + } + ] + }, + { + "version": "8.2.1", + "targetVersion": true, + "cloud": "cloud2", + "nodes": [ + { + "hostname": "node-3-configserver-host", + "environment": "prod", + "region": "eu-west-1" + }, + { + "hostname": "node-2-configserver-host", + "environment": "prod", + "region": "eu-west-1" + }, + { + "hostname": "node-1-configserver-host", + "environment": "prod", + "region": "eu-west-1" + }, + { + "hostname": "node-2-proxy-host", + "environment": "prod", + "region": "eu-west-1" + }, + { + "hostname": "node-1-proxy-host", + "environment": "prod", + "region": "eu-west-1" + }, + { + "hostname": "node-3-proxy-host", + "environment": "prod", + "region": "eu-west-1" + } + ] + } + ] +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-initial.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-initial.json new file mode 100644 index 00000000000..9c1625fdcd5 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-initial.json @@ -0,0 +1,108 @@ +{ + "versions": [ + { + "version": "0.0.0", + "targetVersion": false, + "cloud": "cloud1", + "nodes": [ + { + "hostname": "node-3-configserver-host", + "environment": "prod", + "region": "us-east-3" + }, + { + "hostname": "node-2-configserver-host", + "environment": "prod", + "region": "us-east-3" + }, + { + "hostname": "node-1-configserver-host", + "environment": "prod", + "region": "us-east-3" + }, + { + "hostname": "node-3-configserver-host", + "environment": "prod", + "region": "us-west-1" + }, + { + "hostname": "node-2-configserver-host", + "environment": "prod", + "region": "us-west-1" + }, + { + "hostname": "node-1-configserver-host", + "environment": "prod", + "region": "us-west-1" + }, + { + "hostname": "node-2-proxy-host", + "environment": "prod", + "region": "us-east-3" + }, + { + "hostname": "node-1-proxy-host", + "environment": "prod", + "region": "us-east-3" + }, + { + "hostname": "node-3-proxy-host", + "environment": "prod", + "region": "us-east-3" + }, + { + "hostname": "node-2-proxy-host", + "environment": "prod", + "region": "us-west-1" + }, + { + "hostname": "node-1-proxy-host", + "environment": "prod", + "region": "us-west-1" + }, + { + "hostname": "node-3-proxy-host", + "environment": "prod", + "region": "us-west-1" + } + ] + }, + { + "version": "0.0.0", + "targetVersion": false, + "cloud": "cloud2", + "nodes": [ + { + "hostname": "node-3-configserver-host", + "environment": "prod", + "region": "eu-west-1" + }, + { + "hostname": "node-2-configserver-host", + "environment": "prod", + "region": "eu-west-1" + }, + { + "hostname": "node-1-configserver-host", + "environment": "prod", + "region": "eu-west-1" + }, + { + "hostname": "node-2-proxy-host", + "environment": "prod", + "region": "eu-west-1" + }, + { + "hostname": "node-1-proxy-host", + "environment": "prod", + "region": "eu-west-1" + }, + { + "hostname": "node-3-proxy-host", + "environment": "prod", + "region": "eu-west-1" + } + ] + } + ] +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-partially-upgraded.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-partially-upgraded.json new file mode 100644 index 00000000000..48a96b70df7 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-partially-upgraded.json @@ -0,0 +1,121 @@ +{ + "versions": [ + { + "version": "0.0.0", + "targetVersion": false, + "cloud": "cloud1", + "nodes": [ + { + "hostname": "node-3-configserver-host", + "environment": "prod", + "region": "us-west-1" + }, + { + "hostname": "node-2-configserver-host", + "environment": "prod", + "region": "us-west-1" + }, + { + "hostname": "node-1-configserver-host", + "environment": "prod", + "region": "us-west-1" + }, + { + "hostname": "node-2-proxy-host", + "environment": "prod", + "region": "us-west-1" + }, + { + "hostname": "node-1-proxy-host", + "environment": "prod", + "region": "us-west-1" + }, + { + "hostname": "node-3-proxy-host", + "environment": "prod", + "region": "us-west-1" + } + ] + }, + { + "version": "7.5.2", + "targetVersion": true, + "cloud": "cloud1", + "nodes": [ + { + "hostname": "node-3-configserver-host", + "environment": "prod", + "region": "us-east-3" + }, + { + "hostname": "node-2-configserver-host", + "environment": "prod", + "region": "us-east-3" + }, + { + "hostname": "node-1-configserver-host", + "environment": "prod", + "region": "us-east-3" + }, + { + "hostname": "node-2-proxy-host", + "environment": "prod", + "region": "us-east-3" + }, + { + "hostname": "node-1-proxy-host", + "environment": "prod", + "region": "us-east-3" + }, + { + "hostname": "node-3-proxy-host", + "environment": "prod", + "region": "us-east-3" + } + ] + }, + { + "version": "0.0.0", + "targetVersion": false, + "cloud": "cloud2", + "nodes": [ + { + "hostname": "node-3-configserver-host", + "environment": "prod", + "region": "eu-west-1" + }, + { + "hostname": "node-2-configserver-host", + "environment": "prod", + "region": "eu-west-1" + }, + { + "hostname": "node-1-configserver-host", + "environment": "prod", + "region": "eu-west-1" + }, + { + "hostname": "node-2-proxy-host", + "environment": "prod", + "region": "eu-west-1" + }, + { + "hostname": "node-1-proxy-host", + "environment": "prod", + "region": "eu-west-1" + }, + { + "hostname": "node-3-proxy-host", + "environment": "prod", + "region": "eu-west-1" + } + ] + }, + { + "version": "8.2.1", + "targetVersion": true, + "cloud": "cloud2", + "nodes": [] + } + ] +} diff --git a/controller-server/src/test/resources/job/run-details-response.json b/controller-server/src/test/resources/job/run-details-response.json index 3ba9bff049e..06f02565a75 100644 --- a/controller-server/src/test/resources/job/run-details-response.json +++ b/controller-server/src/test/resources/job/run-details-response.json @@ -1,5 +1,19 @@ { - "deployTester":"INFO\t1234567890\tSUCCESS", - "installTester":"INFO\t1234598760\tSUCCESS", - "deactivateTester":"INFO\t1234678901\tERROR: Something went wrong" + "active":false, + "lastId":2, + "deployTester":[], + "installTester":[ + { + "at":12, + "level":"DEBUG", + "message":"SUCCESS" + } + ], + "deactivateTester":[ + { + "at":123, + "level":"WARNING", + "message":"ERROR" + } + ] } diff --git a/docker-api/pom.xml b/docker-api/pom.xml index 87761a17f70..64410c32f06 100644 --- a/docker-api/pom.xml +++ b/docker-api/pom.xml @@ -129,7 +129,7 @@ <artifactId>bundle-plugin</artifactId> <extensions>true</extensions> <configuration> - <attachBundleToArtifact>true</attachBundleToArtifact> + <attachBundleArtifact>true</attachBundleArtifact> </configuration> </plugin> <plugin> diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java index 699aff775cf..70d266fdfa5 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java @@ -65,6 +65,7 @@ import java.util.stream.Collectors; * @author bjorncs */ @Beta +// TODO Vespa 7: Remove unused Manhattan metrics public class JettyHttpServer extends AbstractServerProvider { public interface Metrics { diff --git a/node-admin/pom.xml b/node-admin/pom.xml index 2f7b3109d38..7daeacec463 100644 --- a/node-admin/pom.xml +++ b/node-admin/pom.xml @@ -133,10 +133,9 @@ <groupId>com.yahoo.vespa</groupId> <artifactId>bundle-plugin</artifactId> <extensions>true</extensions> - </plugin> - <plugin> - <groupId>org.apache.maven.plugins</groupId> - <artifactId>maven-compiler-plugin</artifactId> + <configuration> + <attachBundleArtifact>true</attachBundleArtifact> + </configuration> </plugin> </plugins> </build> diff --git a/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp b/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp index ce09a9a3742..bd539999b5d 100644 --- a/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp +++ b/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp @@ -8,7 +8,6 @@ #include <vespa/searchcore/proton/attribute/attribute_writer.h> #include <vespa/searchcore/proton/attribute/attributemanager.h> #include <vespa/searchcore/proton/attribute/exclusive_attribute_read_accessor.h> -#include <vespa/searchcore/proton/attribute/i_attribute_functor.h> #include <vespa/searchcore/proton/attribute/imported_attributes_repo.h> #include <vespa/searchcore/proton/attribute/sequential_attributes_initializer.h> #include <vespa/searchcore/proton/flushengine/shrink_lid_space_flush_target.h> @@ -20,6 +19,7 @@ #include <vespa/searchcore/proton/test/attribute_utils.h> #include <vespa/searchcore/proton/test/attribute_vectors.h> #include <vespa/searchlib/attribute/attributefactory.h> +#include <vespa/searchlib/attribute/i_attribute_functor.h> #include <vespa/searchlib/attribute/attributevector.hpp> #include <vespa/searchlib/attribute/attribute_read_guard.h> #include <vespa/searchlib/attribute/imported_attribute_vector.h> @@ -79,7 +79,7 @@ namespace { const uint64_t createSerialNum = 42u; -class MyAttributeFunctor : public proton::IAttributeFunctor +class MyAttributeFunctor : public search::attribute::IAttributeFunctor { std::vector<vespalib::string> _names; diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_functor.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_functor.cpp index b8aceb83dc7..e271f2f0bef 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_functor.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_functor.cpp @@ -14,13 +14,10 @@ AttributeUsageSamplerFunctor::AttributeUsageSamplerFunctor( { } -AttributeUsageSamplerFunctor::~AttributeUsageSamplerFunctor() -{ -} +AttributeUsageSamplerFunctor::~AttributeUsageSamplerFunctor() = default; void -AttributeUsageSamplerFunctor::operator()(const search::AttributeVector & - attributeVector) +AttributeUsageSamplerFunctor::operator()(const search::AttributeVector & attributeVector) { // Executed by attribute writer thread search::AddressSpaceUsage usage = attributeVector.getAddressSpaceUsage(); diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_functor.h b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_functor.h index c2c09e06545..1b06e1dff45 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_functor.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_functor.h @@ -2,7 +2,7 @@ #pragma once -#include "i_attribute_functor.h" +#include <vespa/searchlib/attribute/i_attribute_functor.h> #include <memory> namespace proton { @@ -13,17 +13,15 @@ class AttributeUsageSamplerContext; * Functor for sampling attribute usage and passing it on to sampler * context. */ -class AttributeUsageSamplerFunctor : public IAttributeFunctor +class AttributeUsageSamplerFunctor : public search::attribute::IAttributeFunctor { std::shared_ptr<AttributeUsageSamplerContext> _samplerContext; std::string _subDbName; public: - AttributeUsageSamplerFunctor(std::shared_ptr<AttributeUsageSamplerContext> - samplerContext, + AttributeUsageSamplerFunctor(std::shared_ptr<AttributeUsageSamplerContext> samplerContext, const std::string &subDbname); - ~AttributeUsageSamplerFunctor(); - virtual void - operator()(const search::AttributeVector &attributeVector) override; + ~AttributeUsageSamplerFunctor() override; + void operator()(const search::AttributeVector &attributeVector) override; }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp index 87248e060c9..d0a1e891150 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp @@ -4,7 +4,6 @@ #include "attribute_directory.h" #include "attributedisklayout.h" #include "attributemanager.h" -#include "i_attribute_functor.h" #include "imported_attributes_context.h" #include "imported_attributes_repo.h" #include "sequential_attributes_initializer.h" @@ -12,6 +11,7 @@ #include <vespa/searchcore/proton/flushengine/shrink_lid_space_flush_target.h> #include <vespa/searchlib/attribute/attributecontext.h> #include <vespa/searchlib/attribute/attribute_read_guard.h> +#include <vespa/searchlib/attribute/i_attribute_functor.h> #include <vespa/searchlib/attribute/interlock.h> #include <vespa/searchlib/common/isequencedtaskexecutor.h> #include <vespa/searchlib/common/threaded_compactable_lid_space.h> diff --git a/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.cpp b/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.cpp index 79ef162adce..48332bedbb7 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.cpp @@ -1,10 +1,10 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "filter_attribute_manager.h" -#include "i_attribute_functor.h" #include <vespa/searchlib/common/isequencedtaskexecutor.h> #include <vespa/vespalib/util/exceptions.h> #include <vespa/searchlib/attribute/attributevector.h> +#include <vespa/searchlib/attribute/i_attribute_functor.h> #include <vespa/searchlib/attribute/attribute_read_guard.h> using search::AttributeGuard; diff --git a/searchcore/src/vespa/searchcore/proton/attribute/i_attribute_manager.h b/searchcore/src/vespa/searchcore/proton/attribute/i_attribute_manager.h index ca4a1a28c77..dd8f9a5df90 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/i_attribute_manager.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/i_attribute_manager.h @@ -10,13 +10,13 @@ #include <vespa/searchlib/common/serialnum.h> namespace search { -class IDestructorCallback; -class ISequencedTaskExecutor; + class IDestructorCallback; + class ISequencedTaskExecutor; } +namespace search::attribute { class IAttributeFunctor; } namespace proton { -class IAttributeFunctor; class ImportedAttributesRepo; /** @@ -27,11 +27,9 @@ class ImportedAttributesRepo; */ struct IAttributeManager : public search::IAttributeManager { - typedef std::shared_ptr<IAttributeManager> SP; - + using SP = std::shared_ptr<IAttributeManager>; using OnWriteDoneType = const std::shared_ptr<search::IDestructorCallback> &; - - virtual ~IAttributeManager() {} + using IAttributeFunctor = search::attribute::IAttributeFunctor; /** * Create a new attribute manager based on the content of the current one and diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp index 3c9f57dc02b..9b5f898495c 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp @@ -303,7 +303,7 @@ MatchThread::processResult(const Doom & hardDoom, } if (hardDoom.doom()) return; size_t totalHits = result->getNumHits(); - search::RankedHit *hits = result->getArray(); + const search::RankedHit *hits = result->getArray(); size_t numHits = result->getArrayUsed(); search::BitVector *bits = result->getBitOverflow(); if (bits != nullptr && hits != nullptr) { @@ -316,7 +316,7 @@ MatchThread::processResult(const Doom & hardDoom, } if (hardDoom.doom()) return; size_t sortLimit = hasGrouping ? numHits : context.result->maxSize(); - context.sort->sorter->sortResults(hits, numHits, sortLimit); + result->sort(*context.sort->sorter, sortLimit); if (hardDoom.doom()) return; if (hasGrouping) { search::grouping::GroupingManager man(*context.grouping); diff --git a/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.cpp b/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.cpp index 335b6855d48..661b5e34f34 100644 --- a/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.cpp +++ b/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.cpp @@ -51,12 +51,25 @@ DocumentDBTaggedMetrics::SubDBMetrics::LidSpaceMetrics::LidSpaceMetrics(MetricSe DocumentDBTaggedMetrics::SubDBMetrics::LidSpaceMetrics::~LidSpaceMetrics() { } +DocumentDBTaggedMetrics::SubDBMetrics::DocumentStoreMetrics::CacheMetrics::CacheMetrics(MetricSet *parent) + : MetricSet("cache", "", "Document store cache metrics", parent), + memoryUsage("memory_usage", "", "Memory usage of the cache (in bytes)", this), + elements("elements", "", "Number of elements in the cache", this), + hitRate("hit_rate", "", "Rate of hits in the cache compared to number of lookups", this), + lookups("lookups", "", "Number of lookups in the cache (hits + misses)", this), + invalidations("invalidations", "", "Number of invalidations (erased elements) in the cache. ", this) +{ +} + +DocumentDBTaggedMetrics::SubDBMetrics::DocumentStoreMetrics::CacheMetrics::~CacheMetrics() {} + DocumentDBTaggedMetrics::SubDBMetrics::DocumentStoreMetrics::DocumentStoreMetrics(MetricSet *parent) - : MetricSet("document_store", "", "document store metrics for this document sub DB", parent), + : MetricSet("document_store", "", "Document store metrics for this document sub DB", parent), diskUsage("disk_usage", "", "Disk space usage in bytes", this), diskBloat("disk_bloat", "", "Disk space bloat in bytes", this), maxBucketSpread("max_bucket_spread", "", "Max bucket spread in underlying files (sum(unique buckets in each chunk)/unique buckets in file)", this), - memoryUsage(this) + memoryUsage(this), + cache(this) { } DocumentDBTaggedMetrics::SubDBMetrics::DocumentStoreMetrics::~DocumentStoreMetrics() { } diff --git a/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.h b/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.h index 7a0de216343..952fb0304f5 100644 --- a/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.h +++ b/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.h @@ -49,10 +49,23 @@ struct DocumentDBTaggedMetrics : metrics::MetricSet struct DocumentStoreMetrics : metrics::MetricSet { + struct CacheMetrics : metrics::MetricSet + { + metrics::LongValueMetric memoryUsage; + metrics::LongValueMetric elements; + metrics::LongAverageMetric hitRate; + metrics::LongCountMetric lookups; + metrics::LongCountMetric invalidations; + + CacheMetrics(metrics::MetricSet *parent); + ~CacheMetrics(); + }; + metrics::LongValueMetric diskUsage; metrics::LongValueMetric diskBloat; metrics::DoubleValueMetric maxBucketSpread; MemoryUsageMetrics memoryUsage; + CacheMetrics cache; DocumentStoreMetrics(metrics::MetricSet *parent); ~DocumentStoreMetrics(); diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp index 24d40e15677..da636068deb 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp @@ -1166,6 +1166,26 @@ updateMatchingMetrics(DocumentDBMetricsCollection &metrics, const IDocumentSubDB } void +updateDocumentStoreCacheHitRate(const CacheStats ¤t, const CacheStats &last, + metrics::LongAverageMetric &cacheHitRate) +{ + if (current.lookups() < last.lookups() || current.hits < last.hits) { + LOG(warning, "Not adding document store cache hit rate metrics as values calculated " + "are corrupt. current.lookups=%" PRIu64 ", last.lookups=%" PRIu64 ", current.hits=%" PRIu64 ", last.hits=%" PRIu64 ".", + current.lookups(), last.lookups(), current.hits, last.hits); + } else { + if ((current.lookups() - last.lookups()) > 0xffffffffull + || (current.hits - last.hits) > 0xffffffffull) + { + LOG(warning, "Document store cache hit rate metrics to add are suspiciously high." + " lookups diff=%" PRIu64 ", hits diff=%" PRIu64 ".", + current.lookups() - last.lookups(), current.hits - last.hits); + } + cacheHitRate.addTotalValueWithCount(current.hits - last.hits, current.lookups() - last.lookups()); + } +} + +void updateDocstoreMetrics(LegacyDocumentDBMetrics::DocstoreMetrics &metrics, const DocumentSubDBCollection &sub_dbs, CacheStats &lastCacheStats) @@ -1180,26 +1200,8 @@ updateDocstoreMetrics(LegacyDocumentDBMetrics::DocstoreMetrics &metrics, } } metrics.memoryUsage.set(memoryUsage); - size_t lookups = cache_stats.hits + cache_stats.misses; - metrics.cacheLookups.set(lookups); - size_t last_count = lastCacheStats.hits + lastCacheStats.misses; - // For the above code to add sane values to the metric, the following - // must be true - if (lookups < last_count || cache_stats.hits < lastCacheStats.hits) { - LOG(warning, "Not adding document db metrics as values calculated " - "are corrupt. %" PRIu64 ", %" PRIu64 ", %" PRIu64 ", %" PRIu64 ".", - lookups, last_count, cache_stats.hits, lastCacheStats.hits); - } else { - if (lookups - last_count > 0xffffffffull - || cache_stats.hits - lastCacheStats.hits > 0xffffffffull) - { - LOG(warning, "Document db metrics to add are suspiciously high." - " %" PRIu64 ", %" PRIu64 ".", - lookups - last_count, cache_stats.hits - lastCacheStats.hits); - } - metrics.cacheHitRate.addTotalValueWithCount( - cache_stats.hits - lastCacheStats.hits, lookups - last_count); - } + metrics.cacheLookups.set(cache_stats.lookups()); + updateDocumentStoreCacheHitRate(cache_stats, lastCacheStats, metrics.cacheHitRate); metrics.hits = cache_stats.hits; metrics.cacheElements.set(cache_stats.elements); metrics.cacheMemoryUsed.set(cache_stats.memory_used); @@ -1207,9 +1209,9 @@ updateDocstoreMetrics(LegacyDocumentDBMetrics::DocstoreMetrics &metrics, } void -updateDocumentStoreMetrics(DocumentDBTaggedMetrics::SubDBMetrics:: - DocumentStoreMetrics &metrics, - IDocumentSubDB *subDb) +updateDocumentStoreMetrics(DocumentDBTaggedMetrics::SubDBMetrics::DocumentStoreMetrics &metrics, + IDocumentSubDB *subDb, + CacheStats &lastCacheStats) { const ISummaryManager::SP &summaryMgr = subDb->getSummaryManager(); search::IDocumentStore &backingStore = summaryMgr->getBackingStore(); @@ -1218,6 +1220,14 @@ updateDocumentStoreMetrics(DocumentDBTaggedMetrics::SubDBMetrics:: metrics.diskBloat.set(storageStats.diskBloat()); metrics.maxBucketSpread.set(storageStats.maxBucketSpread()); metrics.memoryUsage.update(backingStore.getMemoryUsage()); + + search::CacheStats cacheStats = backingStore.getCacheStats(); + metrics.cache.memoryUsage.set(cacheStats.memory_used); + metrics.cache.elements.set(cacheStats.elements); + updateDocumentStoreCacheHitRate(cacheStats, lastCacheStats, metrics.cache.hitRate); + metrics.cache.lookups.set(cacheStats.lookups()); + metrics.cache.invalidations.set(cacheStats.invalidations); + lastCacheStats = cacheStats; } template <typename MetricSetType> @@ -1257,7 +1267,7 @@ DocumentDB::updateLegacyMetrics(LegacyDocumentDBMetrics &metrics, const Executor metrics.summaryExecutor.update(threadingServiceStats.getSummaryExecutorStats()); metrics.indexExecutor.update(threadingServiceStats.getIndexExecutorStats()); metrics.sessionManager.update(_sessionManager->getGroupingStats()); - updateDocstoreMetrics(metrics.docstore, _subDBs, _lastDocStoreCacheStats); + updateDocstoreMetrics(metrics.docstore, _subDBs, _lastDocStoreCacheStats.total); metrics.numDocs.set(getNumDocs()); DocumentMetaStoreReadGuards dmss(_subDBs); @@ -1295,9 +1305,9 @@ DocumentDB::updateMetrics(DocumentDBTaggedMetrics &metrics, const ExecutorThread _jobTrackers.updateMetrics(metrics.job); updateMetrics(metrics.attribute); - updateDocumentStoreMetrics(metrics.ready.documentStore, _subDBs.getReadySubDB()); - updateDocumentStoreMetrics(metrics.removed.documentStore, _subDBs.getRemSubDB()); - updateDocumentStoreMetrics(metrics.notReady.documentStore, _subDBs.getNotReadySubDB()); + updateDocumentStoreMetrics(metrics.ready.documentStore, _subDBs.getReadySubDB(), _lastDocStoreCacheStats.readySubDb); + updateDocumentStoreMetrics(metrics.removed.documentStore, _subDBs.getRemSubDB(), _lastDocStoreCacheStats.removedSubDb); + updateDocumentStoreMetrics(metrics.notReady.documentStore, _subDBs.getNotReadySubDB(), _lastDocStoreCacheStats.notReadySubDb); DocumentMetaStoreReadGuards dmss(_subDBs); updateLidSpaceMetrics(metrics.ready.lidSpace, dmss.readydms->get()); updateLidSpaceMetrics(metrics.notReady.lidSpace, dmss.notreadydms->get()); diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.h b/searchcore/src/vespa/searchcore/proton/server/documentdb.h index 1a64a4013de..5c04c4057ae 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdb.h +++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.h @@ -73,6 +73,13 @@ private: DocumentDBMetricsCollection &getMetrics() { return _metrics; } }; + struct DocumentStoreCacheStats { + search::CacheStats total; + search::CacheStats readySubDb; + search::CacheStats notReadySubDb; + search::CacheStats removedSubDb; + DocumentStoreCacheStats() : total(), readySubDb(), notReadySubDb(), removedSubDb() {} + }; using InitializeThreads = std::shared_ptr<vespalib::ThreadStackExecutorBase>; using IFlushTargetList = std::vector<std::shared_ptr<searchcorespi::IFlushTarget>>; @@ -129,8 +136,8 @@ private: ILidSpaceCompactionHandler::Vector _lidSpaceCompactionHandlers; DocumentDBJobTrackers _jobTrackers; - // Last updated cache statistics. Necessary due to metrics implementation is upside down. - search::CacheStats _lastDocStoreCacheStats; + // Last updated document store cache statistics. Necessary due to metrics implementation is upside down. + DocumentStoreCacheStats _lastDocStoreCacheStats; IBucketStateCalculator::SP _calc; void registerReference(); diff --git a/searchlib/CMakeLists.txt b/searchlib/CMakeLists.txt index 6e4a3ef1e1f..1ad8f562384 100644 --- a/searchlib/CMakeLists.txt +++ b/searchlib/CMakeLists.txt @@ -71,6 +71,7 @@ vespa_define_module( src/tests/attribute src/tests/attribute/attributefilewriter src/tests/attribute/attributemanager + src/tests/attribute/attribute_operation src/tests/attribute/benchmark src/tests/attribute/bitvector src/tests/attribute/bitvector_search_cache diff --git a/searchlib/src/testlist.txt b/searchlib/src/testlist.txt index 259bcaa9a4d..ae93fcf517f 100644 --- a/searchlib/src/testlist.txt +++ b/searchlib/src/testlist.txt @@ -5,6 +5,7 @@ tests/alignment tests/attribute tests/attribute/attributefilewriter tests/attribute/attributemanager +tests/attribute/attribute_operation tests/attribute/bitvector tests/attribute/comparator tests/attribute/document_weight_iterator diff --git a/searchlib/src/tests/attribute/attribute_operation/CMakeLists.txt b/searchlib/src/tests/attribute/attribute_operation/CMakeLists.txt new file mode 100644 index 00000000000..2189c50d163 --- /dev/null +++ b/searchlib/src/tests/attribute/attribute_operation/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(searchlib_attribute_operation_test_app TEST + SOURCES + attribute_operation_test.cpp + DEPENDS + searchlib +) +vespa_add_test(NAME searchlib_attribute_operation_test_app COMMAND searchlib_attribute_operation_test_app) diff --git a/searchlib/src/tests/attribute/attribute_operation/attribute_operation_test.cpp b/searchlib/src/tests/attribute/attribute_operation/attribute_operation_test.cpp new file mode 100644 index 00000000000..12bac6c6b20 --- /dev/null +++ b/searchlib/src/tests/attribute/attribute_operation/attribute_operation_test.cpp @@ -0,0 +1,179 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/searchlib/attribute/attribute_operation.h> +#include <vespa/searchlib/attribute/attributefactory.h> +#include <vespa/searchlib/attribute/singlenumericattribute.h> +#include <vespa/searchlib/common/bitvector.h> +#include <vespa/vespalib/testkit/testapp.h> + +#include <vespa/log/log.h> +LOG_SETUP("attribute_operation_test"); + +using search::attribute::AttributeOperation; +using search::attribute::BasicType; +using search::AttributeVector; +using search::AttributeFactory; +using search::attribute::CollectionType; +using search::attribute::Config; + +TEST("test legal operations on integer attribute") { + const std::vector<uint32_t> docs; + for (auto operation : {"++", "--", "+=7", "+= 7", "-=7", "*=8", "/=6", "%=7", "=3", "=-3"}) { + EXPECT_TRUE(AttributeOperation::create(BasicType::INT64, operation, docs)); + } +} + +TEST("test illegal operations on integer attribute") { + const std::vector<uint32_t> docs; + for (auto operation : {"", "-", "+", "+=7.1", "=a", "*=8.z", "=", "=.7", "/=0", "%=0"}) { + EXPECT_FALSE(AttributeOperation::create(BasicType::INT64, operation, docs)); + } +} + +TEST("test legal operations on float attribute") { + const std::vector<uint32_t> docs; + for (auto operation : {"++", "--", "+=7", "+= 7", "-=7", "*=8", "*=8.7", "*=.7", "/=6", "%=7", "=3", "=-3"}) { + EXPECT_TRUE(AttributeOperation::create(BasicType::DOUBLE, operation, docs)); + } +} + +TEST("test illegal operations on float attribute") { + const std::vector<uint32_t> docs; + for (auto operation : {"", "-", "+", "=a", "*=8.z", "=", "/=0", "%=0"}) { + EXPECT_FALSE(AttributeOperation::create(BasicType::DOUBLE, operation, docs)); + } +} + +AttributeVector::SP +createAttribute(BasicType basicType, const vespalib::string &fieldName, bool fastSearch = false) +{ + Config cfg(basicType, CollectionType::SINGLE); + cfg.setFastSearch(fastSearch); + auto av = search::AttributeFactory::createAttribute(fieldName, cfg); + while (20 >= av->getNumDocs()) { + AttributeVector::DocId checkDocId(0u); + ASSERT_TRUE(av->addDoc(checkDocId)); + } + av->commit(); + return av; +} + +template <typename T, typename A, typename R> +void verify(BasicType type, vespalib::stringref operation, AttributeVector & attr, T initial, T expected, std::vector<uint32_t> docs, R result) +{ + auto & attrT = dynamic_cast<A &>(attr); + for (uint32_t docid(0); docid < attr.getNumDocs(); docid++) { + attrT.update(docid, initial); + } + attr.commit(); + auto op = AttributeOperation::create(type, operation, std::move(result)); + EXPECT_TRUE(op); + op->operator()(attr); + for (uint32_t docid(0); docid < attr.getNumDocs(); docid++) { + if (docs.empty() || (docid < docs.front())) { + EXPECT_EQUAL(initial, attrT.get(docid)); + } else { + EXPECT_EQUAL(expected, attrT.get(docid)); + docs.erase(docs.begin()); + } + } +} + +template <typename T, typename R> +void verify2(BasicType typeClaimed, vespalib::stringref operation, AttributeVector & attr, T initial, T expected, std::vector<uint32_t> docs, R result) { + BasicType::Type type = attr.getBasicType(); + if (type == BasicType::INT64) { + verify<int64_t, search::IntegerAttributeTemplate<int64_t>, R>(typeClaimed, operation, attr, initial, expected, docs, std::move(result)); + } else if (type == BasicType::INT32) { + verify<int32_t, search::IntegerAttributeTemplate<int32_t>, R>(typeClaimed, operation, attr, initial, expected, docs, std::move(result)); + } else if (type == BasicType::DOUBLE) { + verify<double , search::FloatingPointAttributeTemplate<double>, R>(typeClaimed, operation, attr, initial, expected, docs, std::move(result)); + } else if (type == BasicType::FLOAT) { + verify<float , search::FloatingPointAttributeTemplate<float>, R>(typeClaimed, operation, attr, initial, expected, docs, std::move(result)); + } else { + ASSERT_TRUE(false); + } +} + +template <typename T> +void verify(BasicType typeClaimed, vespalib::stringref operation, AttributeVector & attr, T initial, T expected) { + std::vector<uint32_t> docs = {1,4,7,9,10,17,19}; + { + verify2<T, std::vector<uint32_t>>(typeClaimed, operation, attr, initial, expected, docs, docs); + } + { + std::vector<AttributeOperation::Hit> hits; + std::for_each(docs.begin(), docs.end(), [&hits](uint32_t docId) { hits.emplace_back(docId, 0.0); }); + verify2<T, std::vector<AttributeOperation::Hit>>(typeClaimed, operation, attr, initial, expected, docs, hits); + } + { + // Only Array hits + AttributeOperation::FullResult hits; + std::for_each(docs.begin(), docs.end(), [&hits](uint32_t docId) { + hits.second.push_back(search::RankedHit(docId, 0.0)); + }); + verify2<T, AttributeOperation::FullResult>(typeClaimed, operation, attr, initial, expected, docs, std::move(hits)); + } + { + // Only BitVector + AttributeOperation::FullResult hits; + hits.first = search::BitVector::create(docs.back() + 1); + std::for_each(docs.begin(), docs.end(), [&hits](uint32_t docId) { + hits.first->setBit(docId); + }); + verify2<T, AttributeOperation::FullResult>(typeClaimed, operation, attr, initial, expected, docs, std::move(hits)); + } + { + // And a nice mix + AttributeOperation::FullResult hits; + hits.first = search::BitVector::create(docs.back() + 1); + std::for_each(docs.begin(), docs.end(), [&hits](uint32_t docId) { + if ((docId%2) == 0) hits.first->setBit(docId); + }); + std::for_each(docs.begin(), docs.end(), [&hits](uint32_t docId) { + if ((docId%2) != 0) hits.second.push_back(search::RankedHit(docId, 0.0)); + }); + verify2<T, AttributeOperation::FullResult>(typeClaimed, operation, attr, initial, expected, docs, std::move(hits)); + } +} + +template <typename T> +void verify(vespalib::stringref operation, AttributeVector & attr, T initial, T expected) { + verify<T>(attr.getBasicType(), operation, attr, initial, expected); +} + +TEST("test all integer operations") { + auto attr = createAttribute(BasicType::INT64, "ai"); + const std::vector<std::pair<const char *, int64_t>> expectedOperation = { + {"++", 8}, {"--", 6}, {"+=7", 14}, {"-=9", -2}, {"*=3", 21}, {"/=3", 2}, {"%=3", 1} + }; + for (auto operation : expectedOperation) { + TEST_DO(verify<int64_t>(operation.first, *attr, 7, operation.second)); + } +} + +TEST("test all float operations") { + auto attr = createAttribute(BasicType::DOUBLE, "af"); + const std::vector<std::pair<const char *, double>> expectedOperation = { + {"++", 8}, {"--", 6}, {"+=7.3", 14.3}, {"-=0.9", 6.1}, {"*=3.1", 21.7}, {"/=2", 3.5}, {"%=3", 7} + }; + for (auto operation : expectedOperation) { + TEST_DO(verify<double>(operation.first, *attr, 7, operation.second)); + } +} + +TEST("test that even slightly mismatching type will fail to update") { + auto attr = createAttribute(BasicType::INT32, "ai"); + for (auto operation : {"++", "--", "+=7", "-=9", "*=3", "/=3", "%=3"}) { + TEST_DO(verify<int64_t>(BasicType::INT64, operation, *attr, 7, 7)); + } +} + +TEST("test that fastsearch attributes will fail to update") { + auto attr = createAttribute(BasicType::INT64, "ai", true); + for (auto operation : {"++", "--", "+=7", "-=9", "*=3", "/=3", "%=3"}) { + TEST_DO(verify<int64_t>(BasicType::INT64, operation, *attr, 7, 7)); + } +} + +TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/common/resultset/resultset_test.cpp b/searchlib/src/tests/common/resultset/resultset_test.cpp index 21a54c96f40..d0513333ba6 100644 --- a/searchlib/src/tests/common/resultset/resultset_test.cpp +++ b/searchlib/src/tests/common/resultset/resultset_test.cpp @@ -14,44 +14,13 @@ using vespalib::arraysize; namespace { -void concatenate(const ResultSet *input_array[], size_t array_size, - ResultSet &output) -{ - size_t hit_count = 0; - for (size_t i = 0; i < array_size; ++i) { - hit_count += input_array[i]->getArrayUsed(); - } - output.allocArray(hit_count); - RankedHit *p = output.getArray(); - for (size_t i = 0; i < array_size; ++i) { - const ResultSet &set = *input_array[i]; - memcpy(p, set.getArray(), set.getArrayUsed() * sizeof(RankedHit)); - p += set.getArrayUsed(); - if (set.getBitOverflow()) { - if (output.getBitOverflow()) { - output.getBitOverflow()->orWith(*set.getBitOverflow()); - } else { - output.setBitOverflow(BitVector::create(*set.getBitOverflow())); - } - } - } - output.setArrayUsed(hit_count); -} - - void addHit(ResultSet &set, unsigned int doc_id, double rank) { - if (set.getArrayAllocated() == 0) { - set.allocArray(10); - } - ASSERT_LESS(set.getArrayUsed(), set.getArrayAllocated()); - RankedHit *hit_array = set.getArray(); - hit_array[set.getArrayUsed()]._docId = doc_id; - hit_array[set.getArrayUsed()]._rankValue = rank; - set.setArrayUsed(set.getArrayUsed() + 1); + set.push_back(RankedHit(doc_id, rank)); } TEST("require that mergeWithOverflow works") { ResultSet set1; + set1.allocArray(10); addHit(set1, 2, 4.2); addHit(set1, 4, 3.2); BitVector::UP bit_vector = BitVector::create(20); @@ -65,44 +34,6 @@ TEST("require that mergeWithOverflow works") { EXPECT_EQUAL(3u, set1.getNumHits()); } -TEST("require that resultsets can be concatenated") { - ResultSet set1; - addHit(set1, 2, 4.2); - addHit(set1, 4, 3.2); - BitVector::UP bit_vector = BitVector::create(20); - bit_vector->setBit(7); - set1.setBitOverflow(std::move(bit_vector)); - - ResultSet set2; - addHit(set2, 12, 4.2); - addHit(set2, 14, 3.2); - bit_vector = BitVector::create(20); - bit_vector->setBit(17); - set2.setBitOverflow(std::move(bit_vector)); - - const ResultSet *sets[] = { &set1, &set2 }; - ResultSet target; - concatenate(sets, arraysize(sets), target); - - EXPECT_EQUAL(4u, target.getArrayAllocated()); - ASSERT_EQUAL(4u, target.getArrayUsed()); - EXPECT_EQUAL(2u, target.getArray()[0]._docId); - EXPECT_EQUAL(4.2, target.getArray()[0]._rankValue); - EXPECT_EQUAL(4u, target.getArray()[1]._docId); - EXPECT_EQUAL(3.2, target.getArray()[1]._rankValue); - EXPECT_EQUAL(12u, target.getArray()[2]._docId); - EXPECT_EQUAL(4.2, target.getArray()[2]._rankValue); - EXPECT_EQUAL(14u, target.getArray()[3]._docId); - EXPECT_EQUAL(3.2, target.getArray()[3]._rankValue); - - BitVector * bv = target.getBitOverflow(); - ASSERT_TRUE(bv); - EXPECT_EQUAL(20u, bv->size()); - EXPECT_EQUAL(7u, bv->getNextTrueBit(0)); - EXPECT_EQUAL(17u, bv->getNextTrueBit(8)); - EXPECT_EQUAL(20u, bv->getNextTrueBit(18)); -} - } // namespace TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/fef/properties/properties_test.cpp b/searchlib/src/tests/fef/properties/properties_test.cpp index f319980a0c8..474764bde0b 100644 --- a/searchlib/src/tests/fef/properties/properties_test.cpp +++ b/searchlib/src/tests/fef/properties/properties_test.cpp @@ -422,6 +422,54 @@ TEST("test stuff") { IsFilterField::set(p, "bar"); EXPECT_TRUE(IsFilterField::check(p, "bar")); } + { + EXPECT_EQUAL(execute::onmatch::Attribute::NAME, vespalib::string("vespa.execute.onmatch.attribute")); + EXPECT_EQUAL(execute::onmatch::Attribute::DEFAULT_VALUE, ""); + Properties p; + EXPECT_EQUAL(execute::onmatch::Attribute::lookup(p), ""); + p.add("vespa.execute.onmatch.attribute", "foobar"); + EXPECT_EQUAL(execute::onmatch::Attribute::lookup(p), "foobar"); + } + { + EXPECT_EQUAL(execute::onmatch::Operation::NAME, vespalib::string("vespa.execute.onmatch.operation")); + EXPECT_EQUAL(execute::onmatch::Operation::DEFAULT_VALUE, ""); + Properties p; + EXPECT_EQUAL(execute::onmatch::Operation::lookup(p), ""); + p.add("vespa.execute.onmatch.operation", "++"); + EXPECT_EQUAL(execute::onmatch::Operation::lookup(p), "++"); + } + { + EXPECT_EQUAL(execute::onrerank::Attribute::NAME, vespalib::string("vespa.execute.onrerank.attribute")); + EXPECT_EQUAL(execute::onrerank::Attribute::DEFAULT_VALUE, ""); + Properties p; + EXPECT_EQUAL(execute::onrerank::Attribute::lookup(p), ""); + p.add("vespa.execute.onrerank.attribute", "foobar"); + EXPECT_EQUAL(execute::onrerank::Attribute::lookup(p), "foobar"); + } + { + EXPECT_EQUAL(execute::onrerank::Operation::NAME, vespalib::string("vespa.execute.onrerank.operation")); + EXPECT_EQUAL(execute::onrerank::Operation::DEFAULT_VALUE, ""); + Properties p; + EXPECT_EQUAL(execute::onrerank::Operation::lookup(p), ""); + p.add("vespa.execute.onrerank.operation", "++"); + EXPECT_EQUAL(execute::onrerank::Operation::lookup(p), "++"); + } + { + EXPECT_EQUAL(execute::onsummary::Attribute::NAME, vespalib::string("vespa.execute.onsummary.attribute")); + EXPECT_EQUAL(execute::onsummary::Attribute::DEFAULT_VALUE, ""); + Properties p; + EXPECT_EQUAL(execute::onsummary::Attribute::lookup(p), ""); + p.add("vespa.execute.onsummary.attribute", "foobar"); + EXPECT_EQUAL(execute::onsummary::Attribute::lookup(p), "foobar"); + } + { + EXPECT_EQUAL(execute::onsummary::Operation::NAME, vespalib::string("vespa.execute.onsummary.operation")); + EXPECT_EQUAL(execute::onsummary::Operation::DEFAULT_VALUE, ""); + Properties p; + EXPECT_EQUAL(execute::onsummary::Operation::lookup(p), ""); + p.add("vespa.execute.onsummary.operation", "++"); + EXPECT_EQUAL(execute::onsummary::Operation::lookup(p), "++"); + } } } diff --git a/searchlib/src/vespa/searchlib/attribute/CMakeLists.txt b/searchlib/src/vespa/searchlib/attribute/CMakeLists.txt index 58218ecfd65..0916ff7186d 100644 --- a/searchlib/src/vespa/searchlib/attribute/CMakeLists.txt +++ b/searchlib/src/vespa/searchlib/attribute/CMakeLists.txt @@ -5,6 +5,7 @@ vespa_add_library(searchlib_attribute OBJECT attribute.cpp attribute_blueprint_factory.cpp attribute_header.cpp + attribute_operation.cpp attribute_read_guard.cpp attribute_weighted_set_blueprint.cpp attributecontext.cpp diff --git a/searchlib/src/vespa/searchlib/attribute/attribute_operation.cpp b/searchlib/src/vespa/searchlib/attribute/attribute_operation.cpp new file mode 100644 index 00000000000..2274eaf1b41 --- /dev/null +++ b/searchlib/src/vespa/searchlib/attribute/attribute_operation.cpp @@ -0,0 +1,343 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "attribute_operation.h" +#include <vespa/searchlib/attribute/singlenumericattribute.h> +#include <vespa/searchlib/common/bitvector.h> +#include <vespa/searchcommon/attribute/basictype.h> +#include <vespa/vespalib/util/exceptions.h> +#include <vespa/vespalib/util/array.hpp> + +#include <vespa/log/log.h> +LOG_SETUP(".proton.matching.attribute_operation"); + +namespace search::attribute { + +namespace { + +template <typename T> +struct Inc { + using V = T; + Inc(T ) {} + T operator()(T oldVal) const { return oldVal + 1; } +}; + +template <typename T> +struct Dec { + using V = T; + Dec(T ) {} + T operator()(T oldVal) const { return oldVal - 1; } +}; + +template <typename T> +struct Add { + using V = T; + Add(T m) : _m(m) {} + T _m; + T operator()(T oldVal) const { return oldVal + _m; } +}; + +template <typename T> +struct Mul { + using V = T; + Mul(T m) : _m(m) {} + T _m; + T operator()(T oldVal) const { return oldVal * _m; } +}; + +template <typename T> +struct Div { + using V = T; + Div(T m) : _m(m) {} + T _m; + T operator()(T oldVal) const { return oldVal / _m; } +}; + +template <typename T> +struct Mod { + using V = T; + Mod(T m) : _m(m) {} + T _m; + T operator()(T oldVal) const { return oldVal % static_cast<int64_t>(_m); } +}; + +template <> +struct Mod<double> { + using V = double; + Mod(double ) {} + double operator()(double oldVal) const { return oldVal; } +}; + +template <> +struct Mod<float> { + using V = float; + Mod(float ) {} + float operator()(float oldVal) const { return oldVal; } +}; + +template <typename T> +struct Set { + using V = T; + Set(T m) : _m(m) {} + T _m; + T operator()(T) const { return _m; } +}; + +template <typename T, typename OP> +struct UpdateFast { + using A = search::SingleValueNumericAttribute<T>; + using F = OP; + A * attr; + F op; + typedef typename T::LoadedValueType ValueType; + UpdateFast(search::attribute::IAttributeVector &attr_in, typename F::V operand) + : attr(dynamic_cast<A *>(&attr_in)), + op(operand) + {} + void operator()(uint32_t docid) { attr->set(docid, op(attr->getFast(docid))); } + bool valid() const { return (attr != nullptr); } +}; + +template <typename OP> +class OperateOverResultSet : public AttributeOperation { +public: + OperateOverResultSet(FullResult && result, typename OP::F::V operand) + : _operand(operand), + _result(std::move(result)) + {} + + void operator()(const search::AttributeVector &attributeVector) override { + OP op(const_cast<search::AttributeVector &>(attributeVector), _operand); + if (op.valid()) { + const search::RankedHit *hits = &_result.second[0]; + size_t numHits = _result.second.size(); + std::for_each(hits, hits+numHits, [&op](search::RankedHit hit) { op(hit.getDocId()); }); + if (_result.first) { + _result.first->foreach_truebit([&op](uint32_t docId) { op(docId); }); + } + } + } +private: + typename OP::F::V _operand; + FullResult _result; +}; + +template<typename OP> +class OperateOverHits : public AttributeOperation { +public: + using Hit = AttributeOperation::Hit; + OperateOverHits(std::vector<Hit> reRanked, typename OP::F::V operand) + : _operand(operand), + _reRanked(std::move(reRanked)) + {} + + void operator()(const search::AttributeVector &attributeVector) override { + OP op(const_cast<search::AttributeVector &>(attributeVector), _operand); + if (op.valid()) { + std::for_each(_reRanked.begin(), _reRanked.end(), + [&op](Hit hit) { op(hit.first); }); + } + } +private: + typename OP::F::V _operand; + std::vector<Hit> _reRanked; +}; + +template<typename OP> +class OperateOverDocIds : public AttributeOperation { +public: + OperateOverDocIds(std::vector<uint32_t> docIds, typename OP::F::V operand) + : _operand(operand), + _docIds(std::move(docIds)) + {} + + void operator()(const search::AttributeVector &attributeVector) override { + OP op(const_cast<search::AttributeVector &>(attributeVector), _operand); + if (op.valid()) { + std::for_each(_docIds.begin(), _docIds.end(), + [&op](uint32_t docId) { op(docId); }); + } + } +private: + typename OP::F::V _operand; + std::vector<uint32_t> _docIds; +}; + +struct Operation { + enum class Type { INC, DEC, ADD, SUB, MUL, DIV, MOD, SET, BAD }; + Operation(Type operation_in, vespalib::stringref operand_in) : operation(operation_in), operand(operand_in) { } + template <typename V> + std::unique_ptr<AttributeOperation> create(search::attribute::BasicType type, V vector) const; + template <typename IT, typename V> + std::unique_ptr<AttributeOperation> create(V vector) const; + bool valid() const { return operation != Type::BAD; } + bool hasArgument() const { return valid() && (operation != Type::INC) && (operation != Type::DEC); } + Type operation; + vespalib::stringref operand; + static Operation create(vespalib::stringref s); +}; + +Operation +Operation::create(vespalib::stringref s) +{ + Type op = Type::BAD; + if (s.size() >= 2) { + if ((s[0] == '+') && (s[1] == '+')) { + op = Type::INC; + } else if ((s[0] == '-') && (s[1] == '-')) { + op = Type::DEC; + } else if ((s[0] == '+') && (s[1] == '=')) { + op = Type::ADD; + } else if ((s[0] == '-') && (s[1] == '=')) { + op = Type::SUB; + } else if ((s[0] == '*') && (s[1] == '=')) { + op = Type::MUL; + } else if ((s[0] == '/') && (s[1] == '=')) { + op = Type::DIV; + } else if ((s[0] == '%') && (s[1] == '=')) { + op = Type::MOD; + } else if (s[0] == '=') { + op = Type::SET; + } + if (op == Type::SET) { + s = s.substr(1); + } else if (op == Type::BAD) { + } else { + s = s.substr(2); + } + } + return Operation(op, s); +} + +template<typename T, typename OP> +std::unique_ptr<AttributeOperation> +createOperation(std::vector<uint32_t> vector, T operand) { + return std::make_unique<OperateOverDocIds<OP>>(std::move(vector), operand); +} + +template<typename T, typename OP> +std::unique_ptr<AttributeOperation> +createOperation(std::vector<AttributeOperation::Hit> vector, T operand) { + return std::make_unique<OperateOverHits<OP>>(std::move(vector), operand); +} + +template<typename T, typename OP> +std::unique_ptr<AttributeOperation> +createOperation(AttributeOperation::FullResult && result, T operand) { + return std::make_unique<OperateOverResultSet<OP>>(std::move(result), operand); +} + +template <typename T_IN, typename V> +std::unique_ptr<AttributeOperation> +Operation::create(V vector) const { + using T = typename T_IN::T; + using A = typename T_IN::A; + T value(0); + Type validOp = operation; + if (hasArgument()) { + vespalib::asciistream is(operand); + try { + is >> value; + if (!is.empty()) { + LOG(warning, "Invalid operand, unable to consume all of (%s). (%s) is unconsumed.", operand.data(), is.c_str()); + validOp = Type::BAD; + } + if (((validOp == Type::DIV) || (validOp == Type::MOD)) && (value == 0)) { + LOG(warning, "Division by zero is not acceptable (%s).", operand.data()); + validOp = Type::BAD; + } + } catch (vespalib::IllegalArgumentException & e) { + LOG(warning, "Invalid operand, ignoring : %s", e.what()); + validOp = Type::BAD; + } + } + switch (validOp) { + case Type::INC: + return createOperation<T, UpdateFast<A, Inc<T>>>(std::move(vector), value); + case Type::DEC: + return createOperation<T, UpdateFast<A, Dec<T>>>(std::move(vector), value); + case Type::ADD: + return createOperation<T, UpdateFast<A, Add<T>>>(std::move(vector), value); + case Type::SUB: + return createOperation<T, UpdateFast<A, Add<T>>>(std::move(vector), -value); + case Type::MUL: + return createOperation<T, UpdateFast<A, Mul<T>>>(std::move(vector), value); + case Type::DIV: + return createOperation<T, UpdateFast<A, Div<T>>>(std::move(vector), value); + case Type::MOD: + return createOperation<T, UpdateFast<A, Mod<T>>>(std::move(vector), value); + case Type::SET: + return createOperation<T, UpdateFast<A, Set<T>>>(std::move(vector), value); + default: + return std::unique_ptr<AttributeOperation>(); + } +} + +struct Int64T { + using T = int64_t; + using A = search::IntegerAttributeTemplate<int64_t>; +}; + +struct Int32T { + using T = int64_t; + using A = search::IntegerAttributeTemplate<int32_t>; +}; + +struct Int8T { + using T = int64_t; + using A = search::IntegerAttributeTemplate<int8_t>; +}; + +struct DoubleT { + using T = double; + using A = search::FloatingPointAttributeTemplate<double>; +}; +struct FloatT { + using T = double; + using A = search::FloatingPointAttributeTemplate<float>; +}; + +template <typename V> +std::unique_ptr<AttributeOperation> +Operation::create(BasicType type, V hits) const { + if ( ! valid()) { + return std::unique_ptr<AttributeOperation>(); + } + switch (type.type()) { + case BasicType::INT64: + return create<Int64T, V>(std::move(hits)); + case BasicType::INT32: + return create<Int32T, V>(std::move(hits)); + case BasicType::INT8: + return create<Int8T, V>(std::move(hits)); + case BasicType::DOUBLE: + return create<DoubleT, V>(std::move(hits)); + case BasicType::FLOAT: + return create<FloatT, V>(std::move(hits)); + default: + return std::unique_ptr<AttributeOperation>(); + } +} + +} + +std::unique_ptr<AttributeOperation> +AttributeOperation::create(BasicType type, const vespalib::string & operation, std::vector<uint32_t> docs) { + Operation op = Operation::create(operation); + return op.create<std::vector<uint32_t>>(type, std::move(docs)); +} + +std::unique_ptr<AttributeOperation> +AttributeOperation::create(BasicType type, const vespalib::string & operation, std::vector<Hit> docs) { + Operation op = Operation::create(operation); + return op.create<std::vector<Hit>>(type, std::move(docs)); +} + +std::unique_ptr<AttributeOperation> +AttributeOperation::create(BasicType type, const vespalib::string & operation, FullResult && docs) { + Operation op = Operation::create(operation); + return op.create<FullResult>(type, std::move(docs)); +} + +} + +template class vespalib::Array<search::RankedHit>;
\ No newline at end of file diff --git a/searchlib/src/vespa/searchlib/attribute/attribute_operation.h b/searchlib/src/vespa/searchlib/attribute/attribute_operation.h new file mode 100644 index 00000000000..3d6afd75640 --- /dev/null +++ b/searchlib/src/vespa/searchlib/attribute/attribute_operation.h @@ -0,0 +1,27 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "i_attribute_functor.h" +#include <vespa/searchlib/common/rankedhit.h> +#include <vespa/searchcommon/attribute/basictype.h> +#include <vespa/vespalib/util/array.h> +#include <vector> + +namespace search { class BitVector; } + +namespace search::attribute { + +class AttributeOperation : public IAttributeFunctor { +public: + using Hit = std::pair<uint32_t, double>; + using FullResult = std::pair<std::unique_ptr<search::BitVector>, vespalib::Array<search::RankedHit>>; + static std::unique_ptr<AttributeOperation> + create(search::attribute::BasicType type, const vespalib::string & operation, std::vector<uint32_t> docIds); + static std::unique_ptr<AttributeOperation> + create(search::attribute::BasicType type, const vespalib::string & operation, std::vector<Hit> hits); + static std::unique_ptr<AttributeOperation> + create(search::attribute::BasicType type, const vespalib::string & operation, FullResult && result); +}; + +} diff --git a/searchlib/src/vespa/searchlib/attribute/floatbase.h b/searchlib/src/vespa/searchlib/attribute/floatbase.h index 955b2b252af..10bd2648aca 100644 --- a/searchlib/src/vespa/searchlib/attribute/floatbase.h +++ b/searchlib/src/vespa/searchlib/attribute/floatbase.h @@ -66,7 +66,8 @@ public: typedef SequentialReadModifyWriteInterface<LoadedNumericValueT> LoadedVector; virtual uint32_t getRawValues(DocId doc, const multivalue::Value<T> * & values) const; virtual uint32_t getRawValues(DocId doc, const multivalue::WeightedValue<T> * & values) const; - + virtual T get(DocId doc) const = 0; + virtual T getFromEnum(EnumHandle e) const = 0; protected: FloatingPointAttributeTemplate(const vespalib::string & name); FloatingPointAttributeTemplate(const vespalib::string & name, const Config & c); @@ -83,8 +84,6 @@ private: bool findEnum(const char *value, EnumHandle &e) const override; std::vector<EnumHandle> findFoldedEnums(const char *value) const override; bool isUndefined(DocId doc) const override; - virtual T get(DocId doc) const = 0; - virtual T getFromEnum(EnumHandle e) const = 0; double getFloatFromEnum(EnumHandle e) const override; long onSerializeForAscendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const override; diff --git a/searchcore/src/vespa/searchcore/proton/attribute/i_attribute_functor.h b/searchlib/src/vespa/searchlib/attribute/i_attribute_functor.h index 26dfebf1e73..574142f888d 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/i_attribute_functor.h +++ b/searchlib/src/vespa/searchlib/attribute/i_attribute_functor.h @@ -4,7 +4,7 @@ namespace search { class AttributeVector; } -namespace proton { +namespace search::attribute { /* * Interface class for access attribute in correct attribute write @@ -18,5 +18,4 @@ public: virtual ~IAttributeFunctor() { } }; -} // namespace proton - +} diff --git a/searchlib/src/vespa/searchlib/attribute/integerbase.h b/searchlib/src/vespa/searchlib/attribute/integerbase.h index 5dec40fd4da..c3299d9fdf7 100644 --- a/searchlib/src/vespa/searchlib/attribute/integerbase.h +++ b/searchlib/src/vespa/searchlib/attribute/integerbase.h @@ -65,7 +65,8 @@ public: typedef SequentialReadModifyWriteInterface<LoadedNumericValueT> LoadedVector; virtual uint32_t getRawValues(DocId doc, const multivalue::Value<T> * & values) const; virtual uint32_t getRawValues(DocId doc, const multivalue::WeightedValue<T> * & values) const; - + virtual T get(DocId doc) const = 0; + virtual T getFromEnum(EnumHandle e) const = 0; protected: IntegerAttributeTemplate(const vespalib::string & name) : IntegerAttribute(name, BasicType::fromType(T())), @@ -100,8 +101,6 @@ private: bool findEnum(const char *value, EnumHandle &e) const override; std::vector<EnumHandle> findFoldedEnums(const char *value) const override; - virtual T get(DocId doc) const = 0; - virtual T getFromEnum(EnumHandle e) const = 0; largeint_t getIntFromEnum(EnumHandle e) const override; long onSerializeForAscendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const override; long onSerializeForDescendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const override; diff --git a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp index 9ac7c060903..c7299cd71d9 100644 --- a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp @@ -137,19 +137,21 @@ template <typename B> void SingleValueEnumAttribute<B>::reEnumerate() { + auto newIndexes = std::make_unique<vespalib::Array<EnumIndex>>(); + newIndexes->reserve(_enumIndices.capacity()); + for (uint32_t i = 0; i < _enumIndices.size(); ++i) { + EnumIndex oldIdx = _enumIndices[i]; + EnumIndex newIdx; + if (oldIdx.valid()) { + this->_enumStore.getCurrentIndex(oldIdx, newIdx); + } + newIndexes->push_back_fast(newIdx); + } this->logEnumStoreEvent("compactfixup", "drain"); { EnumModifier enumGuard(this->getEnumModifier()); this->logEnumStoreEvent("compactfixup", "start"); - for (uint32_t i = 0; i < _enumIndices.size(); ++i) { - EnumIndex oldIdx = _enumIndices[i]; - if (oldIdx.valid()) { - EnumIndex newIdx; - this->_enumStore.getCurrentIndex(oldIdx, newIdx); - std::atomic_thread_fence(std::memory_order_release); - _enumIndices[i] = newIdx; - } - } + _enumIndices.replaceVector(std::move(newIndexes)); } this->logEnumStoreEvent("compactfixup", "complete"); } diff --git a/searchlib/src/vespa/searchlib/common/rcuvector.cpp b/searchlib/src/vespa/searchlib/common/rcuvector.cpp index b0a8aca1979..d5efb661947 100644 --- a/searchlib/src/vespa/searchlib/common/rcuvector.cpp +++ b/searchlib/src/vespa/searchlib/common/rcuvector.cpp @@ -2,8 +2,7 @@ #include "rcuvector.hpp" -namespace search { -namespace attribute { +namespace search::attribute { template class RcuVectorBase<uint8_t>; template class RcuVectorBase<uint16_t>; @@ -36,4 +35,3 @@ template class RcuVectorHeld<float>; template class RcuVectorHeld<double>; } -} diff --git a/searchlib/src/vespa/searchlib/common/rcuvector.h b/searchlib/src/vespa/searchlib/common/rcuvector.h index dba14517701..b62beeee1f1 100644 --- a/searchlib/src/vespa/searchlib/common/rcuvector.h +++ b/searchlib/src/vespa/searchlib/common/rcuvector.h @@ -8,8 +8,7 @@ #include <vespa/vespalib/util/alloc.h> #include <vespa/vespalib/util/array.h> -namespace search { -namespace attribute { +namespace search::attribute { template <typename T> class RcuVectorHeld : public vespalib::GenerationHeldBase @@ -122,6 +121,7 @@ public: void reset(); void shrink(size_t newSize) __attribute__((noinline)); + void replaceVector(std::unique_ptr<Array> replacement); }; template <typename T> @@ -161,4 +161,3 @@ public: }; } -} diff --git a/searchlib/src/vespa/searchlib/common/rcuvector.hpp b/searchlib/src/vespa/searchlib/common/rcuvector.hpp index 2b6d9fdc480..dde96440935 100644 --- a/searchlib/src/vespa/searchlib/common/rcuvector.hpp +++ b/searchlib/src/vespa/searchlib/common/rcuvector.hpp @@ -5,8 +5,7 @@ #include "rcuvector.h" #include <vespa/vespalib/util/array.hpp> -namespace search { -namespace attribute { +namespace search::attribute { template <typename T> RcuVectorHeld<T>::RcuVectorHeld(size_t size, std::unique_ptr<T> data) @@ -15,7 +14,7 @@ RcuVectorHeld<T>::RcuVectorHeld(size_t size, std::unique_ptr<T> data) { } template <typename T> -RcuVectorHeld<T>::~RcuVectorHeld() { } +RcuVectorHeld<T>::~RcuVectorHeld() = default; template <typename T> void @@ -47,7 +46,7 @@ RcuVectorBase<T>::reset() { } template <typename T> -RcuVectorBase<T>::~RcuVectorBase() { } +RcuVectorBase<T>::~RcuVectorBase() = default; template <typename T> void @@ -57,9 +56,15 @@ RcuVectorBase<T>::expand(size_t newCapacity) { for (const T & v : _data) { tmpData->push_back_fast(v); } - tmpData->swap(_data); // atomic switch of underlying data - size_t holdSize = tmpData->capacity() * sizeof(T); - vespalib::GenerationHeldBase::UP hold(new RcuVectorHeld<Array>(holdSize, std::move(tmpData))); + replaceVector(std::move(tmpData)); +} + +template <typename T> +void +RcuVectorBase<T>::replaceVector(std::unique_ptr<Array> replacement) { + replacement->swap(_data); // atomic switch of underlying data + size_t holdSize = replacement->capacity() * sizeof(T); + vespalib::GenerationHeldBase::UP hold(new RcuVectorHeld<Array>(holdSize, std::move(replacement))); _genHolder.hold(std::move(hold)); onReallocation(); } @@ -198,4 +203,3 @@ RcuVector<T>::getMemoryUsage() const } } -} diff --git a/searchlib/src/vespa/searchlib/common/resultset.cpp b/searchlib/src/vespa/searchlib/common/resultset.cpp index cdf20ff3882..f30a11f7c56 100644 --- a/searchlib/src/vespa/searchlib/common/resultset.cpp +++ b/searchlib/src/vespa/searchlib/common/resultset.cpp @@ -2,6 +2,7 @@ #include "resultset.h" #include "bitvector.h" +#include "sortresults.h" #include <cstring> using vespalib::alloc::Alloc; @@ -12,10 +13,8 @@ namespace search { constexpr size_t MMAP_LIMIT = 0x2000000; ResultSet::ResultSet() - : _elemsUsedInRankedHitsArray(0u), - _rankedHitsArrayAllocElements(0u), - _bitOverflow(), - _rankedHitsArray() + : _bitOverflow(), + _rankedHitsArray(Alloc::alloc(0, MMAP_LIMIT)) {} ResultSet::~ResultSet() = default; @@ -25,23 +24,12 @@ void ResultSet::allocArray(unsigned int arrayAllocated) { if (arrayAllocated > 0) { - Alloc::alloc(arrayAllocated * sizeof(RankedHit), MMAP_LIMIT).swap(_rankedHitsArray); + _rankedHitsArray.reserve(arrayAllocated); } else { - Alloc().swap(_rankedHitsArray); + _rankedHitsArray.clear(); } - _rankedHitsArrayAllocElements = arrayAllocated; - _elemsUsedInRankedHitsArray = 0; } - -void -ResultSet::setArrayUsed(unsigned int arrayUsed) -{ - assert(arrayUsed <= _rankedHitsArrayAllocElements); - _elemsUsedInRankedHitsArray = arrayUsed; -} - - void ResultSet::setBitOverflow(BitVector::UP newBitOverflow) { @@ -55,7 +43,7 @@ ResultSet::setBitOverflow(BitVector::UP newBitOverflow) unsigned int ResultSet::getNumHits() const { - return (_bitOverflow) ? _bitOverflow->countTrueBits() : _elemsUsedInRankedHitsArray; + return (_bitOverflow) ? _bitOverflow->countTrueBits() : _rankedHitsArray.size(); } @@ -69,15 +57,12 @@ ResultSet::mergeWithBitOverflow(HitRank default_value) const BitVector *bitVector = _bitOverflow.get(); const RankedHit *oldA = getArray(); - const RankedHit *oldAEnd = oldA + _elemsUsedInRankedHitsArray; + const RankedHit *oldAEnd = oldA + getArrayUsed(); uint32_t bidx = bitVector->getFirstTrueBit(); uint32_t actualHits = getNumHits(); - Alloc newHitsAlloc = Alloc::alloc(actualHits*sizeof(RankedHit), MMAP_LIMIT); - RankedHit *newHitsArray = static_cast<RankedHit *>(newHitsAlloc.get()); - - RankedHit * tgtA = newHitsArray; - RankedHit * tgtAEnd = newHitsArray + actualHits; + vespalib::Array<RankedHit> newHits(Alloc::alloc(0, MMAP_LIMIT)); + newHits.reserve(actualHits); if (oldAEnd > oldA) { // we have array hits uint32_t firstArrayHit = oldA->_docId; @@ -85,38 +70,40 @@ ResultSet::mergeWithBitOverflow(HitRank default_value) // bitvector hits before array hits while (bidx < firstArrayHit) { - tgtA->_docId = bidx; - tgtA->_rankValue = default_value; - tgtA++; + newHits.push_back_fast(RankedHit(bidx, default_value)); bidx = bitVector->getNextTrueBit(bidx + 1); } // merge bitvector and array hits while (bidx <= lastArrayHit) { - tgtA->_docId = bidx; if (bidx == oldA->_docId) { - tgtA->_rankValue = oldA->_rankValue; + newHits.push_back_fast(RankedHit(bidx, oldA->_rankValue)); oldA++; } else { - tgtA->_rankValue = default_value; + newHits.push_back_fast(RankedHit(bidx, default_value)); } - tgtA++; bidx = bitVector->getNextTrueBit(bidx + 1); } } assert(oldA == oldAEnd); // bitvector hits after array hits - while (tgtA < tgtAEnd) { - tgtA->_docId = bidx; - tgtA->_rankValue = default_value; - tgtA++; + while (newHits.size() < actualHits) { + newHits.push_back_fast(RankedHit(bidx, default_value)); bidx = bitVector->getNextTrueBit(bidx + 1); } - _rankedHitsArrayAllocElements = actualHits; - _elemsUsedInRankedHitsArray = actualHits; - _rankedHitsArray.swap(newHitsAlloc); + _rankedHitsArray.swap(newHits); setBitOverflow(nullptr); } +void +ResultSet::sort(FastS_IResultSorter & sorter, unsigned int ntop) { + sorter.sortResults(&_rankedHitsArray[0], _rankedHitsArray.size(), ntop); +} + +std::pair<std::unique_ptr<BitVector>, vespalib::Array<RankedHit>> +ResultSet::stealResult(ResultSet && rhs) { + return std::make_pair(std::move(rhs._bitOverflow), std::move(rhs._rankedHitsArray)); +} + } // namespace search diff --git a/searchlib/src/vespa/searchlib/common/resultset.h b/searchlib/src/vespa/searchlib/common/resultset.h index cdd64b73731..acffc1ff18e 100644 --- a/searchlib/src/vespa/searchlib/common/resultset.h +++ b/searchlib/src/vespa/searchlib/common/resultset.h @@ -3,7 +3,9 @@ #pragma once #include "rankedhit.h" -#include <vespa/vespalib/util/alloc.h> +#include <vespa/vespalib/util/array.h> + +class FastS_IResultSorter; namespace search { @@ -12,10 +14,8 @@ class BitVector; class ResultSet { private: - unsigned int _elemsUsedInRankedHitsArray; - unsigned int _rankedHitsArrayAllocElements; std::unique_ptr<BitVector> _bitOverflow; - vespalib::alloc::Alloc _rankedHitsArray; + vespalib::Array<RankedHit> _rankedHitsArray; public: using UP = std::unique_ptr<ResultSet>; ResultSet& operator=(const ResultSet &) = delete; @@ -25,17 +25,19 @@ public: void allocArray(unsigned int arrayAllocated); - void setArrayUsed(unsigned int arrayUsed); void setBitOverflow(std::unique_ptr<BitVector> newBitOverflow); - const RankedHit * getArray() const { return static_cast<const RankedHit *>(_rankedHitsArray.get()); } - RankedHit * getArray() { return static_cast<RankedHit *>(_rankedHitsArray.get()); } - unsigned int getArrayUsed() const { return _elemsUsedInRankedHitsArray; } - unsigned int getArrayAllocated() const { return _rankedHitsArrayAllocElements; } + const RankedHit * getArray() const { return &_rankedHitsArray[0]; } + RankedHit & operator [](uint32_t i) { return _rankedHitsArray[i]; } + void push_back(RankedHit hit) { _rankedHitsArray.push_back_fast(hit); } + unsigned int getArrayUsed() const { return _rankedHitsArray.size(); } const BitVector * getBitOverflow() const { return _bitOverflow.get(); } BitVector * getBitOverflow() { return _bitOverflow.get(); } unsigned int getNumHits() const; void mergeWithBitOverflow(HitRank default_value = default_rank_value); + void sort(FastS_IResultSorter & sorter, unsigned int ntop); + static std::pair<std::unique_ptr<BitVector>, vespalib::Array<RankedHit>> + stealResult(ResultSet && rhs); }; } // namespace search diff --git a/searchlib/src/vespa/searchlib/common/sortresults.cpp b/searchlib/src/vespa/searchlib/common/sortresults.cpp index e39f11f56b2..757ba9f3f9a 100644 --- a/searchlib/src/vespa/searchlib/common/sortresults.cpp +++ b/searchlib/src/vespa/searchlib/common/sortresults.cpp @@ -166,10 +166,6 @@ FastS_DefaultResultSorter FastS_DefaultResultSorter::__instance; //----------------------------------------------------------------------------- -FastS_DocIdResultSorter FastS_DocIdResultSorter::__instance; - -//----------------------------------------------------------------------------- - bool FastS_SortSpec::Add(IAttributeContext & vecMan, const SortInfo & sInfo) { @@ -374,12 +370,6 @@ FastS_SortSpec::freeSortData() } } -bool -FastS_SortSpec::hasSortData() const -{ - return ! _binarySortData.empty() && ! _sortDataArray.empty(); -} - void FastS_SortSpec::initWithoutSorting(const RankedHit * hits, uint32_t hitCnt) { diff --git a/searchlib/src/vespa/searchlib/common/sortresults.h b/searchlib/src/vespa/searchlib/common/sortresults.h index 26bdc8eba96..6d69a60e967 100644 --- a/searchlib/src/vespa/searchlib/common/sortresults.h +++ b/searchlib/src/vespa/searchlib/common/sortresults.h @@ -9,7 +9,6 @@ #include <vespa/vespalib/util/array.h> #include <vespa/vespalib/util/doom.h> -#define PREFETCH 64 #define INSERT_SORT_LEVEL 80 namespace search { @@ -36,11 +35,6 @@ struct FastS_IResultSorter { virtual ~FastS_IResultSorter() {} /** - * @return should bitvector hits also be sorted? - **/ - virtual bool completeSort() const = 0; - - /** * Sort the given array of results. * * @param a the array of hits @@ -59,7 +53,6 @@ private: public: static FastS_DefaultResultSorter *instance() { return &__instance; } - bool completeSort() const override { return false; } void sortResults(search::RankedHit a[], uint32_t n, uint32_t ntop) override { return FastS_SortResults(a, n, ntop); } @@ -67,21 +60,6 @@ public: //----------------------------------------------------------------------------- -class FastS_DocIdResultSorter : public FastS_IResultSorter -{ -private: - static FastS_DocIdResultSorter __instance; - -public: - static FastS_DocIdResultSorter *Instance() { return &__instance; } - bool completeSort() const override { return true; } - void sortResults(search::RankedHit[], uint32_t, uint32_t) override { - // already sorted on docid - } -}; - -//----------------------------------------------------------------------------- - class FastS_SortSpec : public FastS_IResultSorter { private: @@ -144,12 +122,10 @@ public: _sortDataArray[i]._len); } bool Init(const vespalib::string & sortSpec, search::attribute::IAttributeContext & vecMan); - bool completeSort() const override { return true; } void sortResults(search::RankedHit a[], uint32_t n, uint32_t topn) override; uint32_t getSortDataSize(uint32_t offset, uint32_t n); void copySortData(uint32_t offset, uint32_t n, uint32_t *idx, char *buf); void freeSortData(); - bool hasSortData() const; void initWithoutSorting(const search::RankedHit * hits, uint32_t hitCnt); static int Compare(const FastS_SortSpec *self, const SortData &a, const SortData &b); }; diff --git a/searchlib/src/vespa/searchlib/docstore/cachestats.h b/searchlib/src/vespa/searchlib/docstore/cachestats.h index 37ad4df1442..f20e31e5ae0 100644 --- a/searchlib/src/vespa/searchlib/docstore/cachestats.h +++ b/searchlib/src/vespa/searchlib/docstore/cachestats.h @@ -12,19 +12,22 @@ struct CacheStats { size_t misses; size_t elements; size_t memory_used; + size_t invalidations; CacheStats() : hits(0), misses(0), elements(0), - memory_used(0) + memory_used(0), + invalidations(0) { } - CacheStats(size_t hit, size_t miss, size_t elem, size_t mem) - : hits(hit), - misses(miss), - elements(elem), - memory_used(mem) + CacheStats(size_t hits_, size_t misses_, size_t elements_, size_t memory_used_, size_t invalidations_) + : hits(hits_), + misses(misses_), + elements(elements_), + memory_used(memory_used_), + invalidations(invalidations_) { } CacheStats & @@ -34,9 +37,12 @@ struct CacheStats { misses += rhs.misses; elements += rhs.elements; memory_used += rhs.memory_used; + invalidations += rhs.invalidations; return *this; } + + size_t lookups() const { return hits + misses; } }; -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/docstore/documentstore.cpp b/searchlib/src/vespa/searchlib/docstore/documentstore.cpp index 6e784bf8c30..95b6c3b1584 100644 --- a/searchlib/src/vespa/searchlib/docstore/documentstore.cpp +++ b/searchlib/src/vespa/searchlib/docstore/documentstore.cpp @@ -555,7 +555,7 @@ DocumentStore::getFileChunkStats() const CacheStats DocumentStore::getCacheStats() const { CacheStats visitStats = _visitCache->getCacheStats(); CacheStats singleStats(_cache->getHit(), _cache->getMiss() + _uncached_lookups, - _cache->size(), _cache->sizeBytes()); + _cache->size(), _cache->sizeBytes(), _cache->getInvalidate()); singleStats += visitStats; return singleStats; } diff --git a/searchlib/src/vespa/searchlib/docstore/visitcache.cpp b/searchlib/src/vespa/searchlib/docstore/visitcache.cpp index a3fc171eef2..994df3237f2 100644 --- a/searchlib/src/vespa/searchlib/docstore/visitcache.cpp +++ b/searchlib/src/vespa/searchlib/docstore/visitcache.cpp @@ -221,7 +221,7 @@ VisitCache::remove(uint32_t key) { CacheStats VisitCache::getCacheStats() const { - return CacheStats(_cache->getHit(), _cache->getMiss(), _cache->size(), _cache->sizeBytes()); + return CacheStats(_cache->getHit(), _cache->getMiss(), _cache->size(), _cache->sizeBytes(), _cache->getInvalidate()); } VisitCache::Cache::Cache(BackingStore & b, size_t maxBytes) : diff --git a/searchlib/src/vespa/searchlib/expression/attributenode.cpp b/searchlib/src/vespa/searchlib/expression/attributenode.cpp index 41955c08261..ae6d47fe83d 100644 --- a/searchlib/src/vespa/searchlib/expression/attributenode.cpp +++ b/searchlib/src/vespa/searchlib/expression/attributenode.cpp @@ -35,11 +35,11 @@ private: IMPLEMENT_RESULTNODE(EnumAttributeResult, AttributeResult); -AttributeResult::UP createResult(const IAttributeVector * attribute) +std::unique_ptr<AttributeResult> createResult(const IAttributeVector * attribute) { return (dynamic_cast<const SingleValueEnumAttributeBase *>(attribute) != NULL) - ? AttributeResult::UP(new EnumAttributeResult(attribute, 0)) - : AttributeResult::UP(new AttributeResult(attribute, 0)); + ? std::make_unique<EnumAttributeResult>(attribute, 0) + : std::make_unique<AttributeResult>(attribute, 0); } } @@ -106,69 +106,69 @@ void AttributeNode::onPrepare(bool preserveAccurateTypes) if (preserveAccurateTypes) { switch (basicType) { case BasicType::INT8: - setResultType(std::unique_ptr<ResultNode>(new Int8ResultNodeVector())); + setResultType(std::make_unique<Int8ResultNodeVector>()); break; case BasicType::INT16: - setResultType(std::unique_ptr<ResultNode>(new Int16ResultNodeVector())); + setResultType(std::make_unique<Int16ResultNodeVector>()); break; case BasicType::INT32: - setResultType(std::unique_ptr<ResultNode>(new Int32ResultNodeVector())); + setResultType(std::make_unique<Int32ResultNodeVector>()); break; case BasicType::INT64: - setResultType(std::unique_ptr<ResultNode>(new Int64ResultNodeVector())); + setResultType(std::make_unique<Int64ResultNodeVector>()); break; default: throw std::runtime_error("This is no valid integer attribute " + attribute->getName()); break; } } else { - setResultType(std::unique_ptr<ResultNode>(new IntegerResultNodeVector())); + setResultType(std::make_unique<IntegerResultNodeVector>()); } - _handler.reset(new IntegerHandler(updateResult())); + _handler = std::make_unique<IntegerHandler>(updateResult()); } else { if (preserveAccurateTypes) { switch (basicType) { case BasicType::INT8: - setResultType(std::unique_ptr<ResultNode>(new Int8ResultNode())); + setResultType(std::make_unique<Int8ResultNode>()); break; case BasicType::INT16: - setResultType(std::unique_ptr<ResultNode>(new Int16ResultNode())); + setResultType(std::make_unique<Int16ResultNode>()); break; case BasicType::INT32: - setResultType(std::unique_ptr<ResultNode>(new Int32ResultNode())); + setResultType(std::make_unique<Int32ResultNode>()); break; case BasicType::INT64: - setResultType(std::unique_ptr<ResultNode>(new Int64ResultNode())); + setResultType(std::make_unique<Int64ResultNode>()); break; default: throw std::runtime_error("This is no valid integer attribute " + attribute->getName()); break; } } else { - setResultType(std::unique_ptr<ResultNode>(new Int64ResultNode())); + setResultType(std::make_unique<Int64ResultNode>()); } } } else if (attribute->isFloatingPointType()) { if (_hasMultiValue) { - setResultType(std::unique_ptr<ResultNode>(new FloatResultNodeVector())); - _handler.reset(new FloatHandler(updateResult())); + setResultType(std::make_unique<FloatResultNodeVector>()); + _handler = std::make_unique<FloatHandler>(updateResult()); } else { - setResultType(std::unique_ptr<ResultNode>(new FloatResultNode())); + setResultType(std::make_unique<FloatResultNode>()); } } else if (attribute->isStringType()) { if (_hasMultiValue) { if (_useEnumOptimization) { - setResultType(std::unique_ptr<ResultNode>(new EnumResultNodeVector())); - _handler.reset(new EnumHandler(updateResult())); + setResultType(std::make_unique<EnumResultNodeVector>()); + _handler = std::make_unique<EnumHandler>(updateResult()); } else { - setResultType(std::unique_ptr<ResultNode>(new StringResultNodeVector())); - _handler.reset(new StringHandler(updateResult())); + setResultType(std::make_unique<StringResultNodeVector>()); + _handler = std::make_unique<StringHandler>(updateResult()); } } else { if (_useEnumOptimization) { - setResultType(std::unique_ptr<ResultNode>(new EnumResultNode())); + setResultType(std::make_unique<EnumResultNode>()); } else { - setResultType(std::unique_ptr<ResultNode>(new StringResultNode())); + setResultType(std::make_unique<StringResultNode>()); } } } else { @@ -224,7 +224,7 @@ void AttributeNode::EnumHandler::handle(const AttributeResult & r) bool AttributeNode::onExecute() const { - if (_hasMultiValue) { + if (_handler) { _handler->handle(*_scratchResult); } else { updateResult().set(*_scratchResult); diff --git a/searchlib/src/vespa/searchlib/expression/attributenode.h b/searchlib/src/vespa/searchlib/expression/attributenode.h index 912b0f94c50..e0f9b4402b2 100644 --- a/searchlib/src/vespa/searchlib/expression/attributenode.h +++ b/searchlib/src/vespa/searchlib/expression/attributenode.h @@ -146,9 +146,9 @@ private: mutable std::vector<search::attribute::IAttributeVector::WeightedEnum> _wVector; }; - mutable AttributeResult::UP _scratchResult; - mutable bool _hasMultiValue; - mutable bool _useEnumOptimization; + AttributeResult::UP _scratchResult; + bool _hasMultiValue; + bool _useEnumOptimization; std::unique_ptr<Handler> _handler; vespalib::string _attributeName; }; diff --git a/searchlib/src/vespa/searchlib/expression/expressionnode.h b/searchlib/src/vespa/searchlib/expression/expressionnode.h index 12e0897be08..da8a9e2c3ee 100644 --- a/searchlib/src/vespa/searchlib/expression/expressionnode.h +++ b/searchlib/src/vespa/searchlib/expression/expressionnode.h @@ -5,11 +5,9 @@ #include <vespa/vespalib/objects/identifiable.hpp> #include <vespa/vespalib/objects/visit.h> -namespace search { +namespace search::attribute { class IAttributeContext; } -namespace attribute { class IAttributeContext; } - -namespace expression { +namespace search::expression { typedef uint32_t DocId; @@ -55,5 +53,3 @@ private: typedef ExpressionNode::CP * ExpressionNodeArray; } -} - diff --git a/searchlib/src/vespa/searchlib/fef/indexproperties.cpp b/searchlib/src/vespa/searchlib/fef/indexproperties.cpp index 69504c4cc71..b05d5fb4e54 100644 --- a/searchlib/src/vespa/searchlib/fef/indexproperties.cpp +++ b/searchlib/src/vespa/searchlib/fef/indexproperties.cpp @@ -108,6 +108,63 @@ SecondPhase::lookup(const Properties &props) } // namespace rank +namespace execute::onmatch { + +const vespalib::string Attribute::NAME("vespa.execute.onmatch.attribute"); +const vespalib::string Attribute::DEFAULT_VALUE(""); +const vespalib::string Operation::NAME("vespa.execute.onmatch.operation"); +const vespalib::string Operation::DEFAULT_VALUE(""); + +vespalib::string +Attribute::lookup(const Properties &props, const vespalib::string & defaultValue) { + return lookupString(props, NAME, defaultValue); +} + +vespalib::string +Operation::lookup(const Properties &props, const vespalib::string & defaultValue) { + return lookupString(props, NAME, defaultValue); +} + +} + +namespace execute::onrerank { + +const vespalib::string Attribute::NAME("vespa.execute.onrerank.attribute"); +const vespalib::string Attribute::DEFAULT_VALUE(""); +const vespalib::string Operation::NAME("vespa.execute.onrerank.operation"); +const vespalib::string Operation::DEFAULT_VALUE(""); + +vespalib::string +Attribute::lookup(const Properties &props, const vespalib::string & defaultValue) { + return lookupString(props, NAME, defaultValue); +} + +vespalib::string +Operation::lookup(const Properties &props, const vespalib::string & defaultValue) { + return lookupString(props, NAME, defaultValue); +} + +} + +namespace execute::onsummary { + +const vespalib::string Attribute::NAME("vespa.execute.onsummary.attribute"); +const vespalib::string Attribute::DEFAULT_VALUE(""); +const vespalib::string Operation::NAME("vespa.execute.onsummary.operation"); +const vespalib::string Operation::DEFAULT_VALUE(""); + +vespalib::string +Attribute::lookup(const Properties &props, const vespalib::string & defaultValue) { + return lookupString(props, NAME, defaultValue); +} + +vespalib::string +Operation::lookup(const Properties &props, const vespalib::string & defaultValue) { + return lookupString(props, NAME, defaultValue); +} + +} + namespace summary { const vespalib::string Feature::NAME("vespa.summary.feature"); diff --git a/searchlib/src/vespa/searchlib/fef/indexproperties.h b/searchlib/src/vespa/searchlib/fef/indexproperties.h index 5140e811e1c..68bed502121 100644 --- a/searchlib/src/vespa/searchlib/fef/indexproperties.h +++ b/searchlib/src/vespa/searchlib/fef/indexproperties.h @@ -87,6 +87,51 @@ namespace dump { } // namespace dump +namespace execute::onmatch { + struct Attribute { + static const vespalib::string NAME; + static const vespalib::string DEFAULT_VALUE; + static vespalib::string lookup(const Properties &props) { return lookup(props, DEFAULT_VALUE); } + static vespalib::string lookup(const Properties &props, const vespalib::string & defaultValue); + }; + struct Operation { + static const vespalib::string NAME; + static const vespalib::string DEFAULT_VALUE; + static vespalib::string lookup(const Properties &props) { return lookup(props, DEFAULT_VALUE); } + static vespalib::string lookup(const Properties &props, const vespalib::string & defaultValue); + }; +} + +namespace execute::onrerank { + struct Attribute { + static const vespalib::string NAME; + static const vespalib::string DEFAULT_VALUE; + static vespalib::string lookup(const Properties &props) { return lookup(props, DEFAULT_VALUE); } + static vespalib::string lookup(const Properties &props, const vespalib::string & defaultValue); + }; + struct Operation { + static const vespalib::string NAME; + static const vespalib::string DEFAULT_VALUE; + static vespalib::string lookup(const Properties &props) { return lookup(props, DEFAULT_VALUE); } + static vespalib::string lookup(const Properties &props, const vespalib::string & defaultValue); + }; +} + +namespace execute::onsummary { + struct Attribute { + static const vespalib::string NAME; + static const vespalib::string DEFAULT_VALUE; + static vespalib::string lookup(const Properties &props) { return lookup(props, DEFAULT_VALUE); } + static vespalib::string lookup(const Properties &props, const vespalib::string & defaultValue); + }; + struct Operation { + static const vespalib::string NAME; + static const vespalib::string DEFAULT_VALUE; + static vespalib::string lookup(const Properties &props) { return lookup(props, DEFAULT_VALUE); } + static vespalib::string lookup(const Properties &props, const vespalib::string & defaultValue); + }; +} + namespace matching { /** diff --git a/searchlib/src/vespa/searchlib/queryeval/hitcollector.cpp b/searchlib/src/vespa/searchlib/queryeval/hitcollector.cpp index 5c27b9d2067..7002faa6451 100644 --- a/searchlib/src/vespa/searchlib/queryeval/hitcollector.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/hitcollector.cpp @@ -213,15 +213,15 @@ namespace { void mergeHitsIntoResultSet(const std::vector<HitCollector::Hit> &hits, ResultSet &result) { - RankedHit *rhIter = result.getArray(); - RankedHit *rhEnd = rhIter + result.getArrayUsed(); + uint32_t rhCur(0); + uint32_t rhEnd(result.getArrayUsed()); for (const auto &hit : hits) { - while (rhIter != rhEnd && rhIter->_docId != hit.first) { + while (rhCur != rhEnd && result[rhCur]._docId != hit.first) { // just set the iterators right - ++rhIter; + ++rhCur; } - assert(rhIter != rhEnd); // the hits should be a subset of the hits in ranked hit array. - rhIter->_rankValue = hit.second; + assert(rhCur != rhEnd); // the hits should be a subset of the hits in ranked hit array. + result[rhCur]._rankValue = hit.second; } } @@ -248,23 +248,19 @@ HitCollector::getResultSet(HitRank default_value) // destroys the heap property or score sort order sortHitsByDocId(); - std::unique_ptr<ResultSet> rs(new ResultSet()); + auto rs = std::make_unique<ResultSet>(); if ( ! _collector->isDocIdCollector() ) { unsigned int iSize = _hits.size(); rs->allocArray(iSize); - RankedHit * rh = rs->getArray(); if (_needReScore) { for (uint32_t i = 0; i < iSize; ++i) { - rh[i]._docId = _hits[i].first; - rh[i]._rankValue = getReScore(_hits[i].second); + rs->push_back(RankedHit(_hits[i].first, getReScore(_hits[i].second))); } } else { for (uint32_t i = 0; i < iSize; ++i) { - rh[i]._docId = _hits[i].first; - rh[i]._rankValue = _hits[i].second; + rs->push_back(RankedHit(_hits[i].first, _hits[i].second)); } } - rs->setArrayUsed(iSize); } else { if (_unordered) { std::sort(_docIdVector.begin(), _docIdVector.end()); @@ -272,39 +268,35 @@ HitCollector::getResultSet(HitRank default_value) unsigned int iSize = _hits.size(); unsigned int jSize = _docIdVector.size(); rs->allocArray(jSize); - RankedHit * rh = rs->getArray(); uint32_t i = 0; if (_needReScore) { for (uint32_t j = 0; j < jSize; ++j) { uint32_t docId = _docIdVector[j]; - rh[j]._docId = docId; if (i < iSize && docId == _hits[i].first) { - rh[j]._rankValue = getReScore(_hits[i].second); + rs->push_back(RankedHit(docId, getReScore(_hits[i].second))); ++i; } else { - rh[j]._rankValue = default_value; + rs->push_back(RankedHit(docId, default_value)); } } } else { for (uint32_t j = 0; j < jSize; ++j) { uint32_t docId = _docIdVector[j]; - rh[j]._docId = docId; if (i < iSize && docId == _hits[i].first) { - rh[j]._rankValue = _hits[i].second; + rs->push_back(RankedHit(docId, _hits[i].second)); ++i; } else { - rh[j]._rankValue = default_value; + rs->push_back(RankedHit(docId, default_value)); } } } - rs->setArrayUsed(jSize); } if (_hasReRanked) { - mergeHitsIntoResultSet(_reRankedHits, *rs.get()); + mergeHitsIntoResultSet(_reRankedHits, *rs); } - if (_bitVector != NULL) { + if (_bitVector) { rs->setBitOverflow(std::move(_bitVector)); } |