diff options
17 files changed, 412 insertions, 69 deletions
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 1d56e2db08b..30f16acf77d 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 @@ -19,7 +19,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.api.integration.deployment.TestReport; import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud; import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterId; @@ -40,9 +39,10 @@ import com.yahoo.vespa.hosted.controller.versions.VespaVersion; import java.security.cert.X509Certificate; import java.time.Duration; import java.time.Instant; -import java.util.ArrayList; +import java.util.ArrayDeque; import java.util.Collections; import java.util.Comparator; +import java.util.Deque; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -55,7 +55,6 @@ import java.util.TreeMap; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; -import java.util.function.Predicate; import java.util.function.UnaryOperator; import java.util.logging.Level; import java.util.logging.Logger; @@ -390,12 +389,13 @@ public class JobController { * Throws TimeoutException if some step in this job is still being run. */ public void finish(RunId id) throws TimeoutException { - List<Mutex> locks = new ArrayList<>(); + Deque<Mutex> locks = new ArrayDeque<>(); try { // Ensure no step is still running before we finish the run — report depends transitively on all the other steps. Run unlockedRun = run(id).get(); + locks.push(curator.lock(id.application(), id.type(), report)); for (Step step : report.allPrerequisites(unlockedRun.steps().keySet())) - locks.add(curator.lock(id.application(), id.type(), step)); + locks.push(curator.lock(id.application(), id.type(), step)); locked(id, run -> { // If run should be reset, just return here. diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Step.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Step.java index 82d154dcf03..379cc9c4f0a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Step.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Step.java @@ -5,6 +5,7 @@ import java.util.Collection; import java.util.List; import java.util.stream.Stream; +import static java.util.Comparator.reverseOrder; import static java.util.stream.Collectors.toList; /** @@ -87,7 +88,7 @@ public enum Step { .filter(among::contains) .flatMap(pre -> Stream.concat(Stream.of(pre), pre.allPrerequisites(among).stream())) - .sorted() + .sorted(reverseOrder()) .distinct() .collect(toList()); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/FormattedNotification.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/FormattedNotification.java new file mode 100644 index 00000000000..8a6243a7224 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/FormattedNotification.java @@ -0,0 +1,40 @@ +package com.yahoo.vespa.hosted.controller.notification; + +import java.net.URI; +import java.util.Objects; + +/** + * Contains formatted text that can be displayed to a user to give extra information and pointers for a given + * Notification. + * + * @author enygaard + */ +public class FormattedNotification { + private final String prettyType; + private final String messagePrefix; + private final URI uri; + private final Notification notification; + + public FormattedNotification(Notification notification, String prettyType, String messagePrefix, URI uri) { + this.prettyType = Objects.requireNonNull(prettyType); + this.messagePrefix = Objects.requireNonNull(messagePrefix); + this.uri = Objects.requireNonNull(uri); + this.notification = Objects.requireNonNull(notification); + } + + public String prettyType() { + return prettyType; + } + + public String messagePrefix() { + return messagePrefix; + } + + public URI uri() { + return uri; + } + + public Notification notification() { + return notification; + } +}
\ No newline at end of file diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/MissingOptionalException.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/MissingOptionalException.java new file mode 100644 index 00000000000..1379ab4654f --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/MissingOptionalException.java @@ -0,0 +1,18 @@ +package com.yahoo.vespa.hosted.controller.notification; + +/** + * Used to signal that an expected value was not present when creating NotificationContent + * + * @author enygaard + */ +class MissingOptionalException extends RuntimeException { + private final String field; + public MissingOptionalException(String field) { + super(field + " was expected but not present"); + this.field = field; + } + + public String field() { + return field; + } +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationFormatter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationFormatter.java new file mode 100644 index 00000000000..d2b12ab6edc --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationFormatter.java @@ -0,0 +1,190 @@ +package com.yahoo.vespa.hosted.controller.notification; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.Environment; +import com.yahoo.text.Text; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; +import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry; +import org.apache.http.client.utils.URIBuilder; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.Objects; +import java.util.Optional; +import java.util.function.Function; + +/** + * Created a NotificationContent for a given Notification. + * + * The formatter will create specific summary, message start and URI for a given Notification. + * + * @author enygaard + */ +public class NotificationFormatter { + private final ZoneRegistry zoneRegistry; + + public NotificationFormatter(ZoneRegistry zoneRegistry) { + this.zoneRegistry = Objects.requireNonNull(zoneRegistry); + } + + public FormattedNotification format(Notification n) { + switch (n.type()) { + case applicationPackage: + case submission: + return applicationPackage(n); + case deployment: + return deployment(n); + case testPackage: + return testPackage(n); + case reindex: + return reindex(n); + case feedBlock: + return feedBlock(n); + default: + return new FormattedNotification(n, n.type().name(), "", zoneRegistry.dashboardUrl(n.source().tenant())); + } + } + + private FormattedNotification applicationPackage(Notification n) { + var source = n.source(); + var application = requirePresent(source.application(), "application"); + var instance = requirePresent(source.instance(), "instance"); + var message = Text.format("Application package for %s.%s has %s", + application, + instance, + levelText(n.level(), n.messages().size())); + var uri = zoneRegistry.dashboardUrl(ApplicationId.from(source.tenant(), application, instance)); + return new FormattedNotification(n, "Application package", message, uri); + } + + private FormattedNotification deployment(Notification n) { + var source = n.source(); + var message = Text.format("%s for %s.%s has %s", + jobText(source), + requirePresent(source.application(), "application"), + requirePresent(source.instance(), "instance"), + levelText(n.level(), n.messages().size())); + return new FormattedNotification(n,"Deployment", message, jobLink(n.source())); + } + + private FormattedNotification testPackage(Notification n) { + var source = n.source(); + var application = requirePresent(source.application(), "application"); + var message = Text.format("There %s with tests for %s%s", + n.messages().size() > 1 ? "are problems" : "is a problem", + application, + source.instance().map(i -> "."+i).orElse("")); + var uri = zoneRegistry.dashboardUrl(source.tenant(), application); + return new FormattedNotification(n, "Test package", message, uri); + } + + private FormattedNotification reindex(Notification n) { + var message = Text.format("%s is reindexing", clusterInfo(n.source())); + var source = n.source(); + var application = requirePresent(source.application(), "application"); + var instance = requirePresent(source.instance(), "instance"); + var clusterId = requirePresent(source.clusterId(), "clusterId"); + var zone = requirePresent(source.zoneId(), "zoneId"); + var instanceURI = zoneRegistry.dashboardUrl(ApplicationId.from(source.tenant(), application, instance)); + try { + var uri = new URIBuilder(instanceURI) + .setParameter( + String.format("%s.%s.%s", instance, zone.environment(), zone.region()), + String.format("clusters,%s=status", clusterId.value())) + .build(); + return new FormattedNotification(n, "Reindex", message, uri); + } catch (URISyntaxException e) { + throw new IllegalArgumentException(e); + } + } + + private FormattedNotification feedBlock(Notification n) { + String type; + if (n.level() == Notification.Level.warning) { + type = "Nearly feed blocked"; + } else { + type = "Feed blocked"; + } + var message = Text.format("%s is %s", clusterInfo(n.source()), type.toLowerCase()); + var source = n.source(); + var application = requirePresent(source.application(), "application"); + var instance = requirePresent(source.instance(), "instance"); + var clusterId = requirePresent(source.clusterId(), "clusterId"); + var zone = requirePresent(source.zoneId(), "zoneId"); + var instanceURI = zoneRegistry.dashboardUrl(ApplicationId.from(source.tenant(), application, instance)); + try { + var uri = new URIBuilder(instanceURI) + .setParameter( + String.format("%s.%s.%s", instance, zone.environment(), zone.region()), + String.format("clusters,%s", clusterId.value())) + .build(); + return new FormattedNotification(n, type, message, uri); + } catch (URISyntaxException e) { + throw new IllegalArgumentException(e); + } + } + + private URI jobLink(NotificationSource source) { + var application = requirePresent(source.application(), "application"); + var instance = requirePresent(source.instance(), "instance"); + var jobType = requirePresent(source.jobType(), "jobType"); + var runNumber = source.runNumber().orElseThrow(() -> new MissingOptionalException("runNumber")); + var applicationId = ApplicationId.from(source.tenant(), application, instance); + Function<Environment, URI> link = (Environment env) -> zoneRegistry.dashboardUrl(new RunId(applicationId, jobType, runNumber)); + var environment = jobType.zone().environment(); + switch (environment) { + case dev: + case perf: + return link.apply(environment); + default: + return link.apply(Environment.prod); + } + } + + private String jobText(NotificationSource source) { + var jobType = requirePresent(source.jobType(), "jobType"); + var zone = jobType.zone(); + var runNumber = source.runNumber().orElseThrow(() -> new MissingOptionalException("runNumber")); + switch (zone.environment().value()) { + case "production": + return Text.format("Deployment job #%d to %s", runNumber, zone.region()); + case "test": + return Text.format("Test job #%d to %s", runNumber, zone.region()); + case "dev": + case "perf": + return Text.format("Deployment job #%d to %s.%s", runNumber, zone.environment().value(), zone.region().value()); + } + switch (jobType.jobName()) { + case "system-test": + case "staging-test": + } + return Text.format("%s #%d", jobType.jobName(), runNumber); + } + + private String levelText(Notification.Level level, int count) { + switch (level) { + case error: + return "failed"; + case warning: + return count > 1 ? Text.format("%d warnings", count) : "a warning"; + default: + return count > 1 ? Text.format("%d messages", count) : "a message"; + } + } + + private String clusterInfo(NotificationSource source) { + var application = requirePresent(source.application(), "application"); + var instance = requirePresent(source.instance(), "instance"); + var zone = requirePresent(source.zoneId(), "zoneId"); + var clusterId = requirePresent(source.clusterId(), "clusterId"); + return Text.format("Cluster %s in %s.%s for %s.%s", + clusterId.value(), + zone.environment(), zone.region(), + application, instance); + } + + + private static <T> T requirePresent(Optional<T> optional, String field) { + return optional.orElseThrow(() -> new MissingOptionalException(field)); + } +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notifier.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notifier.java index b098b779dbd..5a5188da37f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notifier.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notifier.java @@ -1,13 +1,11 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.notification; -import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Environment; import com.yahoo.text.Text; import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.api.integration.organization.Mail; import com.yahoo.vespa.hosted.controller.api.integration.organization.Mailer; import com.yahoo.vespa.hosted.controller.api.integration.organization.MailerException; @@ -16,7 +14,6 @@ import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; import com.yahoo.vespa.hosted.controller.tenant.TenantContacts; -import java.net.URI; import java.util.Collection; import java.util.List; import java.util.Objects; @@ -31,17 +28,17 @@ import java.util.stream.Collectors; */ public class Notifier { private final CuratorDb curatorDb; - private final ZoneRegistry zoneRegistry; private final Mailer mailer; private final FlagSource flagSource; + private final NotificationFormatter formatter; private static final Logger log = Logger.getLogger(Notifier.class.getName()); public Notifier(CuratorDb curatorDb, ZoneRegistry zoneRegistry, Mailer mailer, FlagSource flagSource) { this.curatorDb = Objects.requireNonNull(curatorDb); - this.zoneRegistry = Objects.requireNonNull(zoneRegistry); this.mailer = Objects.requireNonNull(mailer); this.flagSource = Objects.requireNonNull(flagSource); + this.formatter = new NotificationFormatter(zoneRegistry); } public void dispatch(List<Notification> notifications, NotificationSource source) { @@ -64,6 +61,10 @@ public class Notifier { }); } + public void dispatch(Notification notification) { + dispatch(List.of(notification), notification.source()); + } + private boolean dispatchEnabled(NotificationSource source) { return Flags.NOTIFICATION_DISPATCH_FLAG.bindTo(flagSource) .with(FetchVector.Dimension.TENANT_ID, source.tenant().value()) @@ -80,10 +81,6 @@ public class Notifier { return false; } - public void dispatch(Notification notification) { - dispatch(List.of(notification), notification.source()); - } - private void dispatch(Notification notification, TenantContacts.Type type, Collection<? extends TenantContacts.Contact> contacts) { switch (type) { case EMAIL: @@ -96,21 +93,24 @@ public class Notifier { private void dispatch(Notification notification, Collection<TenantContacts.EmailContact> contacts) { try { - mailer.send(mailOf(notification, contacts.stream().map(c -> c.email()).collect(Collectors.toList()))); + var content = formatter.format(notification); + mailer.send(mailOf(content, contacts.stream().map(c -> c.email()).collect(Collectors.toList()))); } catch (MailerException e) { log.log(Level.SEVERE, "Failed sending email", e); + } catch (MissingOptionalException e) { + log.log(Level.WARNING, "Missing value in required field '" + e.field() + "' for notification type: " + notification.type(), e); } } - private Mail mailOf(Notification n, Collection<String> recipients) { - var source = n.source(); - var subject = Text.format("[%s] %s Vespa Notification for %s", n.level().toString().toUpperCase(), n.type().name(), applicationIdSource(source)); + private Mail mailOf(FormattedNotification content, Collection<String> recipients) { + var notification = content.notification(); + var subject = Text.format("[%s] %s Vespa Notification for %s", notification.level().toString().toUpperCase(), content.prettyType(), applicationIdSource(notification.source())); var body = new StringBuilder(); - body.append("Source: ").append(n.source().toString()).append("\n") - .append("\n") - .append(String.join("\n", n.messages())) + body.append(content.messagePrefix()).append("\n\n") + .append(notification.messages().stream().map(m -> " * " + m).collect(Collectors.joining("\n"))).append("\n") .append("\n") - .append(url(source).toString()); + .append("Vespa Console link:\n") + .append(content.uri().toString()); return new Mail(recipients, subject, body.toString()); } @@ -122,22 +122,5 @@ public class Notifier { return sb.toString(); } - private URI url(NotificationSource source) { - if (source.application().isPresent()) { - if (source.instance().isPresent()) { - if (source.jobType().isPresent() && source.runNumber().isPresent()) { - return zoneRegistry.dashboardUrl( - new RunId(ApplicationId.from(source.tenant(), - source.application().get(), - source.instance().get()), - source.jobType().get(), - source.runNumber().getAsLong())); - } - return zoneRegistry.dashboardUrl(ApplicationId.from(source.tenant(), source.application().get(), source.instance().get())); - } - return zoneRegistry.dashboardUrl(source.tenant(), source.application().get()); - } - return zoneRegistry.dashboardUrl(source.tenant()); - } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationFormatterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationFormatterTest.java new file mode 100644 index 00000000000..c643a612f00 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationFormatterTest.java @@ -0,0 +1,92 @@ +package com.yahoo.vespa.hosted.controller.notification; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ApplicationName; +import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.InstanceName; +import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.TenantName; +import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; +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.application.TenantAndApplicationId; +import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock; +import org.junit.Test; + +import java.time.Instant; +import java.util.List; + +import static org.junit.Assert.assertEquals; + +/** + * @author enygaard + */ +public class NotificationFormatterTest { + private final TenantName tenant = TenantName.from("scoober"); + private final ApplicationName application = ApplicationName.from("myapp"); + private final InstanceName instance = InstanceName.from("beta"); + private final ApplicationId applicationId = ApplicationId.from(tenant, application, instance); + private final DeploymentId deploymentId = new DeploymentId(applicationId, ZoneId.defaultId()); + private final ClusterSpec.Id cluster = new ClusterSpec.Id("content"); + private final ZoneRegistryMock zoneRegistry = new ZoneRegistryMock(SystemName.Public); + + private final NotificationFormatter formatter = new NotificationFormatter(zoneRegistry); + + @Test + public void applicationPackage() { + var notification = new Notification(Instant.now(), Notification.Type.applicationPackage, Notification.Level.warning, NotificationSource.from(applicationId), List.of("1", "2")); + var content = formatter.format(notification); + assertEquals("Application package", content.prettyType()); + assertEquals("Application package for myapp.beta has 2 warnings", content.messagePrefix()); + assertEquals("https://dashboard.tld/scoober.myapp.beta", content.uri().toString()); + } + + @Test + public void deployment() { + var runId = new RunId(applicationId, JobType.prod(RegionName.defaultName()), 1001); + var notification = new Notification(Instant.now(), Notification.Type.deployment, Notification.Level.warning, NotificationSource.from(runId), List.of("1")); + var content = formatter.format(notification); + assertEquals("Deployment", content.prettyType()); + assertEquals("production-default #1001 for myapp.beta has a warning", content.messagePrefix()); + assertEquals("https://dashboard.tld/scoober.myapp.beta/production-default/1001", content.uri().toString()); + } + + @Test + public void deploymentError() { + var runId = new RunId(applicationId, JobType.prod(RegionName.defaultName()), 1001); + var notification = new Notification(Instant.now(), Notification.Type.deployment, Notification.Level.error, NotificationSource.from(runId), List.of("1")); + var content = formatter.format(notification); + assertEquals("Deployment", content.prettyType()); + assertEquals("production-default #1001 for myapp.beta has failed", content.messagePrefix()); + assertEquals("https://dashboard.tld/scoober.myapp.beta/production-default/1001", content.uri().toString()); + } + + @Test + public void testPackage() { + var notification = new Notification(Instant.now(), Notification.Type.testPackage, Notification.Level.warning, NotificationSource.from(TenantAndApplicationId.from(applicationId)), List.of("1")); + var content = formatter.format(notification); + assertEquals("Test package", content.prettyType()); + assertEquals("There is a problem with tests for myapp", content.messagePrefix()); + assertEquals("https://dashboard.tld/scoober/myapp", content.uri().toString()); + } + + @Test + public void reindex() { + var notification = new Notification(Instant.now(), Notification.Type.reindex, Notification.Level.info, NotificationSource.from(deploymentId, cluster), List.of("1")); + var content = formatter.format(notification); + assertEquals("Reindex", content.prettyType()); + assertEquals("Cluster content in prod.default for myapp.beta is reindexing", content.messagePrefix()); + assertEquals("https://dashboard.tld/scoober.myapp.beta?beta.prod.default=clusters%2Ccontent%3Dstatus", content.uri().toString()); + } + + @Test + public void feedBlock() { + var notification = new Notification(Instant.now(), Notification.Type.feedBlock, Notification.Level.warning, NotificationSource.from(deploymentId, cluster), List.of("1")); + var content = formatter.format(notification); + assertEquals("Nearly feed blocked", content.prettyType()); + assertEquals("Cluster content in prod.default for myapp.beta is nearly feed blocked", content.messagePrefix()); + assertEquals("https://dashboard.tld/scoober.myapp.beta?beta.prod.default=clusters%2Ccontent", content.uri().toString()); + } +}
\ No newline at end of file diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java index edbee6e3900..75dbebe96ff 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java @@ -104,9 +104,9 @@ public class NotificationsDbTest { @Test public void notifier_test() { Notification notification1 = notification(12345, Type.deployment, Level.warning, NotificationSource.from(ApplicationId.from(tenant.value(), "app2", "instance2")), "instance msg #2"); - Notification notification2 = notification(12345, Type.deployment, Level.error, NotificationSource.from(ApplicationId.from(tenant.value(), "app3", "instance2")), "instance msg #3"); - Notification notification3 = notification(12345, Type.reindex, Level.warning, NotificationSource.from(ApplicationId.from(tenant.value(), "app2", "instance2")), "instance msg #2"); - + Notification notification2 = notification(12345, Type.applicationPackage, Level.error, NotificationSource.from(ApplicationId.from(tenant.value(), "app3", "instance2")), "instance msg #3"); + Notification notification3 = notification(12345, Type.reindex, Level.warning, NotificationSource.from(new DeploymentId(ApplicationId.from(tenant.value(), "app2", "instance2"), ZoneId.defaultId()), new ClusterSpec.Id("content")), "instance msg #2"); +; var a = notifications.get(0); notificationsDb.setNotification(a.source(), a.type(), a.level(), a.messages()); assertEquals(0, mailer.inbox(email).size()); diff --git a/fnet/src/vespa/fnet/databuffer.cpp b/fnet/src/vespa/fnet/databuffer.cpp index 7820ae2dfcf..ecc352388a6 100644 --- a/fnet/src/vespa/fnet/databuffer.cpp +++ b/fnet/src/vespa/fnet/databuffer.cpp @@ -65,15 +65,18 @@ FNET_DataBuffer::DataToFree(uint32_t len) bool FNET_DataBuffer::Shrink(uint32_t newsize) { - if (GetBufSize() <= newsize || GetDataLen() > newsize) { + const auto data_len = GetDataLen(); + if (GetBufSize() <= newsize || data_len > newsize) { return false; } Alloc newBuf(Alloc::alloc(newsize)); - memcpy(newBuf.get(), _datapt, GetDataLen()); + if (data_len > 0) [[likely]] { + memcpy(newBuf.get(), _datapt, data_len); + } _ownedBuf.swap(newBuf); _bufstart = static_cast<char *>(_ownedBuf.get()); - _freept = _bufstart + GetDataLen(); + _freept = _bufstart + data_len; _datapt = _bufstart; _bufend = _bufstart + newsize; return true; @@ -94,7 +97,7 @@ FNET_DataBuffer::Pack(uint32_t needbytes) bufsize *= 2; Alloc newBuf(Alloc::alloc(bufsize)); - if (_datapt != nullptr) { + if (_datapt != nullptr) [[likely]] { memcpy(newBuf.get(), _datapt, GetDataLen()); } _ownedBuf.swap(newBuf); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index 61e777a9576..dbab7270f08 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -466,6 +466,7 @@ public class NodeAgentImpl implements NodeAgent { case failed: case inactive: case parked: + storageMaintainer.syncLogs(context, true); removeContainerIfNeededUpdateContainerState(context, container); updateNodeRepoWithCurrentAttributes(context, Optional.empty()); stopServicesIfNeeded(context); diff --git a/searchcore/src/vespa/searchcore/proton/server/tls_replay_progress.h b/searchcore/src/vespa/searchcore/proton/server/tls_replay_progress.h index ce0195ad120..5bd1b6382f4 100644 --- a/searchcore/src/vespa/searchcore/proton/server/tls_replay_progress.h +++ b/searchcore/src/vespa/searchcore/proton/server/tls_replay_progress.h @@ -4,6 +4,7 @@ #include <vespa/searchlib/common/serialnum.h> #include <vespa/vespalib/stllike/string.h> +#include <atomic> namespace proton { @@ -13,7 +14,7 @@ private: const vespalib::string _domainName; const search::SerialNum _first; const search::SerialNum _last; - search::SerialNum _current; + std::atomic<search::SerialNum> _current; public: typedef std::unique_ptr<TlsReplayProgress> UP; @@ -27,18 +28,18 @@ public: _current(first) { } - const vespalib::string &getDomainName() const { return _domainName; } - search::SerialNum getFirst() const { return _first; } - search::SerialNum getLast() const { return _last; } - search::SerialNum getCurrent() const { return _current; } - float getProgress() const { + const vespalib::string &getDomainName() const noexcept { return _domainName; } + search::SerialNum getFirst() const noexcept { return _first; } + search::SerialNum getLast() const noexcept { return _last; } + search::SerialNum getCurrent() const noexcept { return _current.load(std::memory_order_relaxed); } + float getProgress() const noexcept { if (_first == _last) { return 1.0; } else { - return ((float)(_current - _first)/float(_last - _first)); + return ((float)(getCurrent() - _first)/float(_last - _first)); } } - void updateCurrent(search::SerialNum current) { _current = current; } + void updateCurrent(search::SerialNum current) noexcept { _current.store(current, std::memory_order_relaxed); } }; } // namespace proton diff --git a/searchcorespi/src/vespa/searchcorespi/index/indexmaintainer.cpp b/searchcorespi/src/vespa/searchcorespi/index/indexmaintainer.cpp index e5bcf5291e5..af273bc5e45 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/indexmaintainer.cpp +++ b/searchcorespi/src/vespa/searchcorespi/index/indexmaintainer.cpp @@ -655,7 +655,7 @@ IndexMaintainer::doneFlush(FlushArgs *args, IDiskIndex::SP *disk_index) { if (args->_prunedSchema != memoryIndex.getPrunedSchema()) { return false; // Must retry operation } - _flush_serial_num = std::max(_flush_serial_num, args->flush_serial_num); + set_flush_serial_num(std::max(flush_serial_num(), args->flush_serial_num)); vespalib::system_time timeStamp = search::FileKit::getModificationTime((*disk_index)->getIndexDir()); _lastFlushTime = timeStamp > _lastFlushTime ? timeStamp : _lastFlushTime; const uint32_t old_id = args->old_absolute_id - _last_fusion_id; @@ -919,13 +919,13 @@ IndexMaintainer::IndexMaintainer(const IndexMaintainerConfig &config, ? getFusionDir(_next_id - 1) : getFlushDir(_next_id - 1); - _flush_serial_num = IndexReadUtilities::readSerialNum(latest_index_dir); + set_flush_serial_num(IndexReadUtilities::readSerialNum(latest_index_dir)); _lastFlushTime = search::FileKit::getModificationTime(latest_index_dir); - set_current_serial_num(_flush_serial_num); + set_current_serial_num(flush_serial_num()); const string selector = IndexDiskLayout::getSelectorFileName(latest_index_dir); _selector = FixedSourceSelector::load(selector, _next_id - 1); } else { - _flush_serial_num = 0; + set_flush_serial_num(0); _selector = std::make_shared<FixedSourceSelector>(0, "sourceselector", 1); } uint32_t baseId(_selector->getBaseId()); @@ -942,7 +942,7 @@ IndexMaintainer::IndexMaintainer(const IndexMaintainerConfig &config, _selector->setDefaultSource(_current_index_id); auto sourceList = loadDiskIndexes(spec, std::make_unique<IndexCollection>(_selector)); _current_index = operations.createMemoryIndex(_schema, *sourceList, current_serial_num()); - LOG(debug, "Index manager created with flushed serial num %" PRIu64, _flush_serial_num); + LOG(debug, "Index manager created with flushed serial num %" PRIu64, flush_serial_num()); sourceList->append(_current_index_id, _current_index); sourceList->setCurrentIndex(_current_index_id); _source_list = std::move(sourceList); @@ -984,11 +984,11 @@ IndexMaintainer::initFlush(SerialNum serialNum, searchcorespi::FlushStats * stat if (args._skippedEmptyLast && args._extraIndexes.empty()) { // No memory index to flush, it was empty LockGuard lock(_state_lock); - _flush_serial_num = current_serial_num(); + set_flush_serial_num(current_serial_num()); _lastFlushTime = vespalib::system_clock::now(); LOG(debug, "No memory index to flush. Update serial number and flush time to current: " "flushSerialNum(%" PRIu64 "), lastFlushTime(%f)", - _flush_serial_num, vespalib::to_s(_lastFlushTime.time_since_epoch())); + flush_serial_num(), vespalib::to_s(_lastFlushTime.time_since_epoch())); return FlushTask::UP(); } SerialNum realSerialNum = args.flush_serial_num; diff --git a/searchcorespi/src/vespa/searchcorespi/index/indexmaintainer.h b/searchcorespi/src/vespa/searchcorespi/index/indexmaintainer.h index dfae5b4d643..b3fb14e1c2e 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/indexmaintainer.h +++ b/searchcorespi/src/vespa/searchcorespi/index/indexmaintainer.h @@ -93,8 +93,8 @@ class IndexMaintainer : public IIndexManager, uint32_t _current_index_id; // Protected by SL + IUL IMemoryIndex::SP _current_index; // Protected by SL + IUL bool _flush_empty_current_index; - std::atomic<SerialNum> _current_serial_num;// Protected by IUL - SerialNum _flush_serial_num; // Protected by SL + std::atomic<SerialNum> _current_serial_num;// Writes protected by IUL + std::atomic<SerialNum> _flush_serial_num; // Writes protected by SL vespalib::system_time _lastFlushTime; // Protected by SL // Extra frozen memory indexes. This list is empty unless new // memory index has been added by force (due to config change or @@ -270,6 +270,12 @@ class IndexMaintainer : public IIndexManager, void set_current_serial_num(SerialNum new_serial_num) noexcept { _current_serial_num.store(new_serial_num, std::memory_order_relaxed); } + [[nodiscard]] SerialNum flush_serial_num() const noexcept { + return _flush_serial_num.load(std::memory_order_relaxed); + } + void set_flush_serial_num(SerialNum new_serial_num) noexcept { + _flush_serial_num.store(new_serial_num, std::memory_order_relaxed); + } public: IndexMaintainer(const IndexMaintainer &) = delete; @@ -340,11 +346,11 @@ public: void compactLidSpace(uint32_t lidLimit, SerialNum serialNum) override; SerialNum getCurrentSerialNum() const override { - return _current_serial_num.load(std::memory_order_relaxed); + return current_serial_num(); } SerialNum getFlushedSerialNum() const override { - return _flush_serial_num; + return flush_serial_num(); } IIndexCollection::SP getSourceCollection() const { diff --git a/vespa-documentgen-plugin/pom.xml b/vespa-documentgen-plugin/pom.xml index 46704533bd1..f3d94fa809d 100644 --- a/vespa-documentgen-plugin/pom.xml +++ b/vespa-documentgen-plugin/pom.xml @@ -58,6 +58,7 @@ <dependency> <groupId>org.apache.maven</groupId> <artifactId>maven-project</artifactId> + <scope>provided</scope> </dependency> <dependency> <groupId>junit</groupId> diff --git a/vespa-documentgen-plugin/src/main/java/com/yahoo/vespa/DocumentGenMojo.java b/vespa-documentgen-plugin/src/main/java/com/yahoo/vespa/DocumentGenMojo.java index 7ffd93f55b0..0b0e7c5b647 100644 --- a/vespa-documentgen-plugin/src/main/java/com/yahoo/vespa/DocumentGenMojo.java +++ b/vespa-documentgen-plugin/src/main/java/com/yahoo/vespa/DocumentGenMojo.java @@ -18,12 +18,11 @@ import com.yahoo.documentmodel.NewDocumentReferenceDataType; import com.yahoo.documentmodel.NewDocumentType; import com.yahoo.documentmodel.OwnedStructDataType; import com.yahoo.documentmodel.VespaDocumentType; -import com.yahoo.searchdefinition.Schema; import com.yahoo.searchdefinition.ApplicationBuilder; +import com.yahoo.searchdefinition.Schema; import com.yahoo.searchdefinition.document.FieldSet; import com.yahoo.searchdefinition.parser.ParseException; import org.apache.maven.plugin.AbstractMojo; -import org.apache.maven.plugins.annotations.Component; import org.apache.maven.plugins.annotations.LifecyclePhase; import org.apache.maven.plugins.annotations.Mojo; import org.apache.maven.plugins.annotations.Parameter; @@ -55,7 +54,7 @@ public class DocumentGenMojo extends AbstractMojo { private static final int STD_INDENT = 4; - @Component + @Parameter( defaultValue = "${project}", readonly = true ) private MavenProject project; /** diff --git a/vespa-maven-plugin/pom.xml b/vespa-maven-plugin/pom.xml index 427c98f26d2..5984c6a1324 100644 --- a/vespa-maven-plugin/pom.xml +++ b/vespa-maven-plugin/pom.xml @@ -63,22 +63,27 @@ <dependency> <groupId>org.apache.maven</groupId> <artifactId>maven-plugin-api</artifactId> + <scope>provided</scope> </dependency> <dependency> <groupId>org.apache.maven.plugin-tools</groupId> <artifactId>maven-plugin-annotations</artifactId> + <scope>provided</scope> </dependency> <dependency> <groupId>org.apache.maven</groupId> <artifactId>maven-model</artifactId> + <scope>provided</scope> </dependency> <dependency> <groupId>org.apache.maven</groupId> <artifactId>maven-artifact</artifactId> + <scope>provided</scope> </dependency> <dependency> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-jar-plugin</artifactId> + <scope>provided</scope> </dependency> <dependency> diff --git a/vespalib/src/vespa/vespalib/stllike/asciistream.cpp b/vespalib/src/vespa/vespalib/stllike/asciistream.cpp index eb127c7051a..6b673363d2d 100644 --- a/vespalib/src/vespa/vespalib/stllike/asciistream.cpp +++ b/vespalib/src/vespa/vespalib/stllike/asciistream.cpp @@ -628,6 +628,9 @@ asciistream::createFromFile(stringref fileName) if (sz < 0) { throw IoException("Failed getting size of file " + fileName + " : Error=" + file.getLastErrorString(), IoException::UNSPECIFIED, VESPA_STRLOC); } + if (sz == 0) { + return is; + } alloc::Alloc buf = alloc::Alloc::alloc(sz); ssize_t actual = file.Read(buf.get(), sz); if (actual != sz) { |