diff options
50 files changed, 520 insertions, 659 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/DeployHandlerLogger.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/DeployHandlerLogger.java index d7977477f30..154d2d0f2f0 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/DeployHandlerLogger.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/DeployHandlerLogger.java @@ -15,7 +15,7 @@ import java.util.logging.Level; import java.util.logging.Logger; /** - * A {@link DeployLogger} which persists messages as a {@link Slime} tree, and holds a tenant and application name. + * A {@link DeployLogger} which stores messages in a {@link Slime} tree, and holds a tenant and application name. * * @author Ulf Lilleengen */ diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareAndActivateResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareAndActivateResponse.java index 617df3868ab..2f88319ba8f 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareAndActivateResponse.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareAndActivateResponse.java @@ -11,7 +11,7 @@ import com.yahoo.vespa.config.server.configchange.ConfigChangeActionsSlimeConver import com.yahoo.vespa.config.server.http.v2.PrepareResult; /** - * Creates a response for SessionPrepareHandler. + * Creates a response for ApplicationApiHandler. * * @author hmusum */ diff --git a/configserver/src/main/sh/start-configserver b/configserver/src/main/sh/start-configserver index 8e7a9d7839a..f223c0a8fb9 100755 --- a/configserver/src/main/sh/start-configserver +++ b/configserver/src/main/sh/start-configserver @@ -78,7 +78,7 @@ cd ${VESPA_HOME} || { echo "Cannot cd to ${VESPA_HOME}" 1>&2; exit 1; } fixfile () { if [ -f $1 ]; then - if [ "${VESPA_USER}" ] && [ "$(id -u)" -eq 0 ]; then + if [ "${VESPA_USER}" ] && [ "${VESPA_UNPRIVILEGED}" != yes ]; then chown ${VESPA_USER} $1 fi chmod 644 $1 @@ -90,8 +90,8 @@ fixddir () { echo "Creating data directory $1" mkdir -p $1 || exit 1 fi - if [ "${VESPA_USER}" ] && [ "$(id -u)" -eq 0 ]; then - chown ${VESPA_USER} $1 + if [ "${VESPA_USER}" ] && [ "${VESPA_UNPRIVILEGED}" != yes ]; then + chown ${VESPA_USER} $1 fi chmod 755 $1 } diff --git a/container-core/src/test/java/com/yahoo/container/messagebus/cfg-disabled/.gitignore b/container-core/src/test/java/com/yahoo/container/messagebus/cfg-disabled/.gitignore deleted file mode 100644 index e69de29bb2d..00000000000 --- a/container-core/src/test/java/com/yahoo/container/messagebus/cfg-disabled/.gitignore +++ /dev/null diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/DeployResult.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/DeployResult.java deleted file mode 100644 index 50074329b49..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/DeployResult.java +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.api.application.v4.model; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonProperty; -import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.ConfigChangeActions; -import com.yahoo.vespa.hosted.controller.api.identifiers.RevisionId; - -import java.util.List; - -/** - * @author gjoranv - */ -@JsonIgnoreProperties(ignoreUnknown = true) -public class DeployResult { - - public final RevisionId revisionId; - public final Long applicationZipSize; - public final List<LogEntry> prepareMessages; - public final ConfigChangeActions configChangeActions; - - @JsonCreator - public DeployResult(@JsonProperty("revisionId") RevisionId revisionId, - @JsonProperty("applicationZipSize") Long applicationZipSize, - @JsonProperty("prepareMessages") List<LogEntry> prepareMessages, - @JsonProperty("configChangeActions") ConfigChangeActions configChangeActions) { - this.revisionId = revisionId; - this.applicationZipSize = applicationZipSize; - this.prepareMessages = prepareMessages; - this.configChangeActions = configChangeActions; - } - - @Override - public String toString() { - return "DeployResult{" + - "revisionId=" + revisionId.id() + - ", applicationZipSize=" + applicationZipSize + - ", prepareMessages=" + prepareMessages + - ", configChangeActions=" + configChangeActions + - '}'; - } -} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/ConfigChangeActions.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/ConfigChangeActions.java deleted file mode 100644 index fd740f19b69..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/ConfigChangeActions.java +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonProperty; - -import java.util.List; - -/** - * @author bjorncs - */ -@JsonIgnoreProperties(ignoreUnknown = true) -public class ConfigChangeActions { - @JsonProperty("restart") public final List<RestartAction> restartActions; - @JsonProperty("refeed") public final List<RefeedAction> refeedActions; - @JsonProperty("reindex") public final List<ReindexAction> reindexActions; - - @JsonCreator - public ConfigChangeActions(@JsonProperty("restart") List<RestartAction> restartActions, - @JsonProperty("refeed") List<RefeedAction> refeedActions, - @JsonProperty("reindex") List<ReindexAction> reindexActions) { - this.restartActions = restartActions; - this.refeedActions = refeedActions; - this.reindexActions = reindexActions; - } - - @Override - public String toString() { - return "ConfigChangeActions{" + - "restartActions=" + restartActions + - ", refeedActions=" + refeedActions + - ", reindexActions=" + reindexActions + - '}'; - } -} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/RefeedAction.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/RefeedAction.java deleted file mode 100644 index bab6ecb0253..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/RefeedAction.java +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonProperty; - -import java.util.List; - -/** - * @author bjorncs - */ -@JsonIgnoreProperties(ignoreUnknown = true) -public class RefeedAction { - - public final String name; - public final String documentType; - public final String clusterName; - public final List<ServiceInfo> services; - public final List<String> messages; - - @JsonCreator - public RefeedAction(@JsonProperty("name") String name, - @JsonProperty("documentType") String documentType, - @JsonProperty("clusterName") String clusterName, - @JsonProperty("services") List<ServiceInfo> services, - @JsonProperty("messages") List<String> messages) { - this.name = name; - this.documentType = documentType; - this.clusterName = clusterName; - this.services = services; - this.messages = messages; - } - - @Override - public String toString() { - return "RefeedAction{" + - "name='" + name + '\'' + - ", documentType='" + documentType + '\'' + - ", clusterName='" + clusterName + '\'' + - ", services=" + services + - ", messages=" + messages + - '}'; - } - -} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/ReindexAction.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/ReindexAction.java deleted file mode 100644 index 36909415888..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/ReindexAction.java +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonProperty; - -import java.util.List; - -/** - * @author jonmv - */ -@JsonIgnoreProperties(ignoreUnknown = true) -public class ReindexAction { - - public final String name; - public final String documentType; - public final String clusterName; - public final List<ServiceInfo> services; - public final List<String> messages; - - @JsonCreator - public ReindexAction(@JsonProperty("name") String name, - @JsonProperty("documentType") String documentType, - @JsonProperty("clusterName") String clusterName, - @JsonProperty("services") List<ServiceInfo> services, - @JsonProperty("messages") List<String> messages) { - this.name = name; - this.documentType = documentType; - this.clusterName = clusterName; - this.services = services; - this.messages = messages; - } - - @Override - public String toString() { - return "ReindexAction{" + - "name='" + name + '\'' + - ", documentType='" + documentType + '\'' + - ", clusterName='" + clusterName + '\'' + - ", services=" + services + - ", messages=" + messages + - '}'; - } - -} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/RestartAction.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/RestartAction.java deleted file mode 100644 index 696e5c049f8..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/RestartAction.java +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonProperty; - -import java.util.List; - -/** - * @author bjorncs - */ -@JsonIgnoreProperties(ignoreUnknown = true) -public class RestartAction { - public final String clusterName; - public final String clusterType; - public final String serviceType; - public final List<ServiceInfo> services; - public final List<String> messages; - - @JsonCreator - public RestartAction(@JsonProperty("clusterName") String clusterName, - @JsonProperty("clusterType") String clusterType, - @JsonProperty("serviceType") String serviceType, - @JsonProperty("services") List<ServiceInfo> services, - @JsonProperty("messages") List<String> messages) { - this.clusterName = clusterName; - this.clusterType = clusterType; - this.serviceType = serviceType; - this.services = services; - this.messages = messages; - } - - @Override - public String toString() { - return "RestartAction{" + - "clusterName='" + clusterName + '\'' + - ", clusterType='" + clusterType + '\'' + - ", serviceType='" + serviceType + '\'' + - ", services=" + services + - ", messages=" + messages + - '}'; - } -} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/ServiceInfo.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/ServiceInfo.java deleted file mode 100644 index bb7e63c2ec4..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/ServiceInfo.java +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonProperty; - -/** - * @author bjorncs - */ -@JsonIgnoreProperties(ignoreUnknown = true) -public class ServiceInfo { - public final String serviceName; - public final String serviceType; - public final String configId; - public final String hostName; - - @JsonCreator - public ServiceInfo(@JsonProperty("serviceName") String serviceName, - @JsonProperty("serviceType") String serviceType, - @JsonProperty("configId") String configId, - @JsonProperty("hostName")String hostName) { - this.serviceName = serviceName; - this.serviceType = serviceType; - this.configId = configId; - this.hostName = hostName; - } - - @Override - public String toString() { - return "ServiceInfo{" + - "serviceName='" + serviceName + '\'' + - ", serviceType='" + serviceType + '\'' + - ", configId='" + configId + '\'' + - ", hostName='" + hostName + '\'' + - '}'; - } -} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/package-info.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/package-info.java deleted file mode 100644 index 75fa8b2b2f1..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/configserverbindings/package-info.java +++ /dev/null @@ -1,5 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -@ExportPackage -package com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings; - -import com.yahoo.osgi.annotation.ExportPackage; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java index 97128d4c980..119e6794e6c 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java @@ -33,7 +33,7 @@ import java.util.Optional; public interface ConfigServer { interface PreparedApplication { - PrepareResponse prepareResponse(); + DeploymentResult deploymentResult(); } PreparedApplication deploy(DeploymentData deployment); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/DeploymentResult.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/DeploymentResult.java new file mode 100644 index 00000000000..4d49e2c3d19 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/DeploymentResult.java @@ -0,0 +1,29 @@ +package com.yahoo.vespa.hosted.controller.api.integration.configserver; + +import java.util.List; +import java.util.logging.Level; + +import static java.util.Objects.requireNonNull; + +/** + * The result of a deployment, carried out against a {@link ConfigServer}. + * + * @author jonmv + */ +public record DeploymentResult(String message, List<LogEntry> log) { + + public DeploymentResult { + requireNonNull(message); + requireNonNull(log); + } + + public record LogEntry(long epochMillis, String message, Level level, boolean concernsPackage) { + + public LogEntry { + requireNonNull(message); + requireNonNull(level); + } + + } + +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/PrepareResponse.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/PrepareResponse.java index 22716c9796f..606acd2cb50 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/PrepareResponse.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/PrepareResponse.java @@ -2,11 +2,7 @@ package com.yahoo.vespa.hosted.controller.api.integration.configserver; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonProperty; -import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.ConfigChangeActions; -import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; -import java.net.URI; import java.util.List; /** @@ -14,8 +10,8 @@ import java.util.List; */ @JsonIgnoreProperties(ignoreUnknown = true) public class PrepareResponse { - public TenantId tenant; + public String message; public List<Log> log; - public ConfigChangeActions configChangeActions; + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/QuotaUsage.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/QuotaUsage.java index 2e4f3b96abe..86177376941 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/QuotaUsage.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/QuotaUsage.java @@ -1,14 +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.api.integration.configserver; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; - /** - * @author ogronnesby + * @author jonmv */ -@JsonIgnoreProperties(ignoreUnknown = true) -public class QuotaUsage { - public static final QuotaUsage zero = new QuotaUsage() {{ this.rate = 0; }}; +public record QuotaUsage(double rate) { + + public static final QuotaUsage zero = new QuotaUsage(0); - public double rate; } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/QuotaUsageResponse.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/QuotaUsageResponse.java new file mode 100644 index 00000000000..1b1c750a0f9 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/QuotaUsageResponse.java @@ -0,0 +1,12 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.api.integration.configserver; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; + +/** + * @author ogronnesby + */ +@JsonIgnoreProperties(ignoreUnknown = true) +public class QuotaUsageResponse { + public double rate; +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index d8234e4f269..7e284a8aef5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -37,6 +37,8 @@ import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCe import com.yahoo.vespa.hosted.controller.api.integration.configserver.ApplicationReindexing; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServer; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ContainerEndpoint; +import com.yahoo.vespa.hosted.controller.api.integration.configserver.DeploymentResult; +import com.yahoo.vespa.hosted.controller.api.integration.configserver.DeploymentResult.LogEntry; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node; import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeFilter; @@ -48,9 +50,9 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterId; import com.yahoo.vespa.hosted.controller.api.integration.noderepository.RestartFilter; import com.yahoo.vespa.hosted.controller.api.integration.secrets.TenantSecretStore; -import com.yahoo.vespa.hosted.controller.application.ActivateResult; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; +import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics.Warning; import com.yahoo.vespa.hosted.controller.application.DeploymentQuotaCalculator; import com.yahoo.vespa.hosted.controller.application.QuotaUsage; import com.yahoo.vespa.hosted.controller.application.SystemApplication; @@ -108,6 +110,9 @@ import static com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence import static com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence.low; import static com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence.normal; import static java.util.Comparator.naturalOrder; +import static java.util.stream.Collectors.collectingAndThen; +import static java.util.stream.Collectors.counting; +import static java.util.stream.Collectors.groupingBy; import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toMap; @@ -475,7 +480,7 @@ public class ApplicationController { } /** Deploys an application package for an existing application instance. */ - public ActivateResult deploy(JobId job, boolean deploySourceVersions, Consumer<String> deployLogger) { + public DeploymentResult deploy(JobId job, boolean deploySourceVersions, Consumer<String> deployLogger) { if (job.application().instance().isTester()) throw new IllegalArgumentException("'" + job.application() + "' is a tester application!"); @@ -515,8 +520,8 @@ public class ApplicationController { } // Release application lock while doing the deployment, which is a lengthy task. // Carry out deployment without holding the application lock. - ActivateResult result = deploy(job.application(), instance.tags(), applicationPackage, zone, platform, containerEndpoints, - endpointCertificateMetadata, run.isDryRun()); + DeploymentResult result = deploy(job.application(), instance.tags(), applicationPackage, zone, platform, containerEndpoints, + endpointCertificateMetadata, run.isDryRun()); endpointCertificateMetadata.ifPresent(e -> deployLogger.accept("Using CA signed certificate version %s".formatted(e.version()))); @@ -530,12 +535,11 @@ public class ApplicationController { ? NotificationSource.from(deployment) : revision.equals(lastRevision.get()) ? NotificationSource.from(applicationId) : null; if (source != null) { - @SuppressWarnings("deprecation") - List<String> warnings = Optional.ofNullable(result.prepareResponse().log) + List<String> warnings = Optional.ofNullable(result.log()) .map(logs -> logs.stream() - .filter(log -> log.applicationPackage) - .filter(log -> LogLevel.parse(log.level).intValue() >= Level.WARNING.intValue()) - .map(log -> log.message) + .filter(LogEntry::concernsPackage) + .filter(log -> log.level().intValue() >= Level.WARNING.intValue()) + .map(LogEntry::message) .sorted() .distinct() .collect(Collectors.toList())) @@ -547,7 +551,7 @@ public class ApplicationController { lockApplicationOrThrow(applicationId, application -> store(application.with(job.application().instance(), i -> i.withNewDeployment(zone, revision, platform, - clock.instant(), warningsFrom(result), + clock.instant(), warningsFrom(result.log()), quotaUsage)))); return result; } @@ -600,7 +604,7 @@ public class ApplicationController { } /** Deploy a system application to given zone */ - public ActivateResult deploySystemApplicationPackage(SystemApplication application, ZoneId zone, Version version) { + public DeploymentResult deploySystemApplicationPackage(SystemApplication application, ZoneId zone, Version version) { if (application.hasApplicationPackage()) { ApplicationPackage applicationPackage = new ApplicationPackage( artifactRepository.getSystemApplicationPackage(application.id(), zone, version) @@ -612,14 +616,14 @@ public class ApplicationController { } /** Deploys the given tester application to the given zone. */ - public ActivateResult deployTester(TesterId tester, ApplicationPackage applicationPackage, ZoneId zone, Version platform) { + public DeploymentResult deployTester(TesterId tester, ApplicationPackage applicationPackage, ZoneId zone, Version platform) { return deploy(tester.id(), Tags.empty(), applicationPackage, zone, platform, Set.of(), /* No application cert for tester*/ Optional.empty(), false); } - private ActivateResult deploy(ApplicationId application, Tags tags, ApplicationPackage applicationPackage, - ZoneId zone, Version platform, Set<ContainerEndpoint> endpoints, - Optional<EndpointCertificateMetadata> endpointCertificateMetadata, - boolean dryRun) { + private DeploymentResult deploy(ApplicationId application, Tags tags, ApplicationPackage applicationPackage, + ZoneId zone, Version platform, Set<ContainerEndpoint> endpoints, + Optional<EndpointCertificateMetadata> endpointCertificateMetadata, + boolean dryRun) { DeploymentId deployment = new DeploymentId(application, zone); try { Optional<DockerImage> dockerImageRepo = Optional.ofNullable( @@ -657,8 +661,7 @@ public class ApplicationController { deploymentQuota, tenantSecretStores, operatorCertificates, cloudAccount, dryRun)); - return new ActivateResult(new com.yahoo.vespa.hosted.controller.api.identifiers.RevisionId(applicationPackage.hash()), preparedApplication.prepareResponse(), - applicationPackage.zippedContent().length); + return preparedApplication.deploymentResult(); } finally { // Even if prepare fails, routing configuration may need to be updated if ( ! application.instance().isTester()) { @@ -1020,16 +1023,13 @@ public class ApplicationController { } /** Extract deployment warnings metric from deployment result */ - private static Map<DeploymentMetrics.Warning, Integer> warningsFrom(ActivateResult result) { - if (result.prepareResponse().log == null) return Map.of(); - Map<DeploymentMetrics.Warning, Integer> warnings = new HashMap<>(); - for (Log log : result.prepareResponse().log) { - // TODO: Categorize warnings. Response from config server should be updated to include the appropriate - // category and typed log level - if (!"warn".equalsIgnoreCase(log.level) && !"warning".equalsIgnoreCase(log.level)) continue; - warnings.merge(DeploymentMetrics.Warning.all, 1, Integer::sum); - } - return Map.copyOf(warnings); + private static Map<DeploymentMetrics.Warning, Integer> warningsFrom(List<DeploymentResult.LogEntry> log) { + return log.stream() + .filter(entry -> entry.level().intValue() >= Level.WARNING.intValue()) + // TODO: Categorize warnings. Response from config server should be updated to include the appropriate + // category and typed log level + .collect(groupingBy(__ -> Warning.all, + collectingAndThen(counting(), Long::intValue))); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ActivateResult.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ActivateResult.java deleted file mode 100644 index 989a0de0f83..00000000000 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ActivateResult.java +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.application; - -import com.yahoo.vespa.hosted.controller.api.identifiers.RevisionId; -import com.yahoo.vespa.hosted.controller.api.integration.configserver.PrepareResponse; - -/** - * @author Oyvind Gronnesby - */ -public class ActivateResult { - // TODO jvenstad: Replace this class with just the PrepareResponse. - - private final RevisionId revisionId; - private final PrepareResponse prepareResponse; - private final long applicationZipSizeBytes; - - public ActivateResult(RevisionId revisionId, PrepareResponse prepareResponse, long applicationZipSizeBytes) { - this.revisionId = revisionId; - this.prepareResponse = prepareResponse; - this.applicationZipSizeBytes = applicationZipSizeBytes; - } - - public long applicationZipSizeBytes() { - return applicationZipSizeBytes; - } - - public RevisionId revisionId() { - return revisionId; - } - - public PrepareResponse prepareResponse() { - return prepareResponse; - } -} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java index c2c95a0c4bf..b99d825a779 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.application.pkg; import com.google.common.hash.Funnel; +import com.google.common.hash.Hasher; import com.google.common.hash.Hashing; import com.yahoo.component.Version; import com.yahoo.compress.ArchiveStreamReader; @@ -71,8 +72,8 @@ public class ApplicationPackage { static final String deploymentFile = "deployment.xml"; private static final String validationOverridesFile = "validation-overrides.xml"; static final String servicesFile = "services.xml"; + private static Hasher hasher() { return Hashing.murmur3_128().newHasher(); } - private final String contentHash; private final String bundleHash; private final byte[] zippedContent; private final DeploymentSpec deploymentSpec; @@ -98,10 +99,8 @@ public class ApplicationPackage { * it must not be further changed by the caller. * If 'requireFiles' is true, files needed by deployment orchestration must be present. */ - @SuppressWarnings("deprecation") // for Hashing.sha1() public ApplicationPackage(byte[] zippedContent, boolean requireFiles) { this.zippedContent = Objects.requireNonNull(zippedContent, "The application package content cannot be null"); - this.contentHash = Hashing.sha1().hashBytes(zippedContent).toString(); this.files = new ZipArchiveCache(zippedContent, Set.of(deploymentFile, validationOverridesFile, servicesFile, buildMetaFile, trustedCertificatesFile)); Optional<DeploymentSpec> deploymentSpec = files.get(deploymentFile).map(bytes -> new String(bytes, UTF_8)).map(DeploymentSpec::fromXml); @@ -134,9 +133,6 @@ public class ApplicationPackage { return new ApplicationPackage(modified.toByteArray()); } - /** Returns a hash of the content of this package */ - public String hash() { return contentHash; } - /** Hash of all files and settings that influence what is deployed to config servers. */ public String bundleHash() { return bundleHash; @@ -242,7 +238,6 @@ public class ApplicationPackage { } // Hashes all files and settings that require a deployment to be forwarded to configservers - @SuppressWarnings("deprecation") // for Hashing.sha1() private String calculateBundleHash(byte[] zippedContent) { Predicate<String> entryMatcher = name -> ! name.endsWith(deploymentFile) && ! name.endsWith(buildMetaFile); SortedMap<String, Long> crcByEntry = new TreeMap<>(); @@ -258,17 +253,14 @@ public class ApplicationPackage { into.putBytes(key.getBytes()); into.putLong(value); }); - return Hashing.sha1().newHasher() - .putObject(crcByEntry, funnel) - .putInt(deploymentSpec.deployableHashCode()) - .hash().toString(); + return hasher().putObject(crcByEntry, funnel) + .putInt(deploymentSpec.deployableHashCode()) + .hash().toString(); } - @SuppressWarnings("deprecation") // for Hashing.sha1() public static String calculateHash(byte[] bytes) { - return Hashing.sha1().newHasher() - .putBytes(bytes) - .hash().toString(); + return hasher().putBytes(bytes) + .hash().toString(); } 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 dcdfea6e594..7a459e4f29f 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 @@ -13,7 +13,6 @@ import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.RoutingMethod; import com.yahoo.config.provision.zone.ZoneId; -import com.yahoo.log.LogLevel; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.Instance; @@ -21,9 +20,9 @@ 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.certificates.EndpointCertificateException; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException; +import com.yahoo.vespa.hosted.controller.api.integration.configserver.DeploymentResult; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node; import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeFilter; -import com.yahoo.vespa.hosted.controller.api.integration.configserver.PrepareResponse; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ServiceConvergence; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId; @@ -31,7 +30,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud; import com.yahoo.vespa.hosted.controller.api.integration.organization.DeploymentFailureMails; import com.yahoo.vespa.hosted.controller.api.integration.organization.Mail; -import com.yahoo.vespa.hosted.controller.application.ActivateResult; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.Endpoint; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; @@ -205,21 +203,18 @@ public class InternalStepRunner implements StepRunner { logger); } - @SuppressWarnings("deprecation") - private Optional<RunStatus> deploy(Supplier<ActivateResult> deployment, Instant startTime, DualLogger logger) { + private Optional<RunStatus> deploy(Supplier<DeploymentResult> deployment, Instant startTime, DualLogger logger) { try { - PrepareResponse prepareResponse = deployment.get().prepareResponse(); - if (prepareResponse.log != null) - logger.logAll(prepareResponse.log.stream() - .map(entry -> new LogEntry(0, // Sequenced by BufferedLogStore. - Instant.ofEpochMilli(entry.time), - LogEntry.typeOf(LogLevel.parse(entry.level)), - entry.message)) - .collect(toList())); + DeploymentResult result = deployment.get(); + logger.logAll(result.log().stream() + .map(entry -> new LogEntry(0, // Sequenced by BufferedLogStore. + Instant.ofEpochMilli(entry.epochMillis()), + LogEntry.typeOf(entry.level()), + entry.message())) + .collect(toList())); logger.log("Deployment successful."); - if (prepareResponse.message != null) - logger.log(prepareResponse.message); + logger.log(result.message()); return Optional.of(running); } @@ -231,38 +226,42 @@ public class InternalStepRunner implements StepRunner { logger.log(WARNING, "Deployment failed for one hour; giving up now!"); switch (e.code()) { - case CERTIFICATE_NOT_READY: + case CERTIFICATE_NOT_READY -> { logger.log("No valid CA signed certificate for app available to config server"); if (startTime.plus(timeouts.endpointCertificate()).isBefore(controller.clock().instant())) { logger.log(WARNING, "CA signed certificate for app not available to config server within " + timeouts.endpointCertificate()); return Optional.of(RunStatus.endpointCertificateTimeout); } return result; - case ACTIVATION_CONFLICT: - case APPLICATION_LOCK_FAILURE: - case CONFIG_NOT_CONVERGED: + } + case ACTIVATION_CONFLICT, APPLICATION_LOCK_FAILURE, CONFIG_NOT_CONVERGED -> { logger.log("Deployment failed with possibly transient error " + e.code() + ", will retry: " + e.getMessage()); return result; - case INTERNAL_SERVER_ERROR: + } + case INTERNAL_SERVER_ERROR -> { // Log only error code, to avoid exposing internal data in error message logger.log("Deployment failed with possibly transient error " + e.code() + ", will retry"); return result; - case LOAD_BALANCER_NOT_READY: - case PARENT_HOST_NOT_READY: + } + case LOAD_BALANCER_NOT_READY, PARENT_HOST_NOT_READY -> { logger.log(e.message()); // Consider splitting these messages in summary and details, on config server. return result; - case NODE_ALLOCATION_FAILURE: + } + case NODE_ALLOCATION_FAILURE -> { logger.log(e.message()); return controller.system().isCd() && startTime.plus(timeouts.capacity()).isAfter(controller.clock().instant()) ? result : Optional.of(nodeAllocationFailure); - case INVALID_APPLICATION_PACKAGE: + } + case INVALID_APPLICATION_PACKAGE -> { logger.log(WARNING, e.getMessage()); return Optional.of(invalidApplication); - case BAD_REQUEST: + } + case BAD_REQUEST -> { logger.log(WARNING, e.getMessage()); return Optional.of(deploymentFailed); + } } throw e; 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 81fb72e19fd..35483309cae 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 @@ -54,16 +54,14 @@ import com.yahoo.vespa.hosted.controller.LockedTenant; import com.yahoo.vespa.hosted.controller.NotExistsException; import com.yahoo.vespa.hosted.controller.api.application.v4.EnvironmentResource; import com.yahoo.vespa.hosted.controller.api.application.v4.model.ProtonMetrics; -import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.RefeedAction; -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.identifiers.TenantId; import com.yahoo.vespa.hosted.controller.api.integration.aws.TenantRoles; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ApplicationReindexing; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Cluster; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException; -import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log; +import com.yahoo.vespa.hosted.controller.api.integration.configserver.DeploymentResult; +import com.yahoo.vespa.hosted.controller.api.integration.configserver.DeploymentResult.LogEntry; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node; import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeFilter; import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeRepository; @@ -75,11 +73,9 @@ 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.noderepository.RestartFilter; import com.yahoo.vespa.hosted.controller.api.integration.secrets.TenantSecretStore; -import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry; import com.yahoo.vespa.hosted.controller.api.role.Role; import com.yahoo.vespa.hosted.controller.api.role.RoleDefinition; import com.yahoo.vespa.hosted.controller.api.role.SecurityContext; -import com.yahoo.vespa.hosted.controller.application.ActivateResult; import com.yahoo.vespa.hosted.controller.application.AssignedRotation; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; @@ -331,16 +327,16 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/submit")) return submit(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}")) return trigger(appIdFromPath(path), jobTypeFromPath(path), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}/pause")) return pause(appIdFromPath(path), jobTypeFromPath(path)); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}")) return deploy(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}/instance/{instance}/environment/{environment}/region/{region}/deploy")) return deploy(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); // legacy synonym of the above + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}")) return deploySystemApplication(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}/instance/{instance}/environment/{environment}/region/{region}/deploy")) return deploySystemApplication(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); // legacy synonym of the above if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/reindex")) return reindex(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}/instance/{instance}/environment/{environment}/region/{region}/reindexing")) return enableReindexing(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}/instance/{instance}/environment/{environment}/region/{region}/restart")) return restart(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}/instance/{instance}/environment/{environment}/region/{region}/suspend")) return suspend(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), true); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/access/support")) return allowSupportAccess(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}/instance/{instance}/environment/{environment}/region/{region}/node/{node}/service-dump")) return requestServiceDump(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("node"), request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}")) return deploy(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}/deploy")) return deploy(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); // legacy synonym of the above + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}")) return deploySystemApplication(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}/deploy")) return deploySystemApplication(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); // legacy synonym of the above if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/restart")) return restart(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); return ErrorResponse.notFoundError("Nothing at " + path); } @@ -2283,7 +2279,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { return new SlimeJsonResponse(slime); } - private HttpResponse deploy(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) { + private HttpResponse deploySystemApplication(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) { ApplicationId applicationId = ApplicationId.from(tenantName, applicationName, instanceName); ZoneId zone = requireZone(environment, region); @@ -2295,18 +2291,18 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { // Resolve system application Optional<SystemApplication> systemApplication = SystemApplication.matching(applicationId); - if (systemApplication.isEmpty() || !systemApplication.get().hasApplicationPackage()) { + if (systemApplication.isEmpty() || ! systemApplication.get().hasApplicationPackage()) { return ErrorResponse.badRequest("Deployment of " + applicationId + " is not supported through this API"); } // Make it explicit that version is not yet supported here String vespaVersion = deployOptions.field("vespaVersion").asString(); - if (!vespaVersion.isEmpty() && !vespaVersion.equals("null")) { + if ( ! vespaVersion.isEmpty()) { return ErrorResponse.badRequest("Specifying version for " + applicationId + " is not permitted"); } - // To avoid second guessing the orchestrated upgrades of system applications - // we don't allow to deploy these during an system upgrade (i.e when new vespa is being rolled out) + // To avoid second guessing the orchestrated upgrades of system applications we don't allow + // deploying these during a system upgrade, i.e., when a new Vespa version is being rolled out VersionStatus versionStatus = controller.readVersionStatus(); if (versionStatus.isUpgrading()) { throw new IllegalArgumentException("Deployment of system applications during a system upgrade is not allowed"); @@ -2315,14 +2311,26 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if (systemVersion.isEmpty()) { throw new IllegalArgumentException("Deployment of system applications is not permitted until system version is determined"); } - ActivateResult result = controller.applications() - .deploySystemApplicationPackage(systemApplication.get(), zone, systemVersion.get().versionNumber()); - return new SlimeJsonResponse(toSlime(result)); + DeploymentResult result = controller.applications() + .deploySystemApplicationPackage(systemApplication.get(), zone, systemVersion.get().versionNumber()); + Slime slime = new Slime(); + Cursor root = slime.setObject(); + root.setString("message", "Deployed " + systemApplication.get() + " in " + zone + " on " + systemVersion.get().versionNumber()); + + Cursor logArray = root.setArray("prepareMessages"); + for (LogEntry logMessage : result.log()) { + Cursor logObject = logArray.addObject(); + logObject.setLong("time", logMessage.epochMillis()); + logObject.setString("level", logMessage.level().getName()); + logObject.setString("message", logMessage.message()); + } + + return new SlimeJsonResponse(slime); } private HttpResponse deleteTenant(String tenantName, HttpRequest request) { boolean forget = request.getBooleanProperty("forget"); - if (forget && !isOperator(request)) + if (forget && ! isOperator(request)) return ErrorResponse.forbidden("Only operators can forget a tenant"); controller.tenants().delete(TenantName.from(tenantName), @@ -2777,55 +2785,6 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { request.getUri()).toString()); } - private Slime toSlime(ActivateResult result) { - Slime slime = new Slime(); - Cursor object = slime.setObject(); - object.setString("revisionId", result.revisionId().id()); - object.setLong("applicationZipSize", result.applicationZipSizeBytes()); - Cursor logArray = object.setArray("prepareMessages"); - if (result.prepareResponse().log != null) { - for (Log logMessage : result.prepareResponse().log) { - Cursor logObject = logArray.addObject(); - logObject.setLong("time", logMessage.time); - logObject.setString("level", logMessage.level); - logObject.setString("message", logMessage.message); - } - } - - Cursor changeObject = object.setObject("configChangeActions"); - - Cursor restartActionsArray = changeObject.setArray("restart"); - for (RestartAction restartAction : result.prepareResponse().configChangeActions.restartActions) { - Cursor restartActionObject = restartActionsArray.addObject(); - restartActionObject.setString("clusterName", restartAction.clusterName); - restartActionObject.setString("clusterType", restartAction.clusterType); - restartActionObject.setString("serviceType", restartAction.serviceType); - serviceInfosToSlime(restartAction.services, restartActionObject.setArray("services")); - stringsToSlime(restartAction.messages, restartActionObject.setArray("messages")); - } - - Cursor refeedActionsArray = changeObject.setArray("refeed"); - for (RefeedAction refeedAction : result.prepareResponse().configChangeActions.refeedActions) { - Cursor refeedActionObject = refeedActionsArray.addObject(); - refeedActionObject.setString("name", refeedAction.name); - refeedActionObject.setString("documentType", refeedAction.documentType); - refeedActionObject.setString("clusterName", refeedAction.clusterName); - serviceInfosToSlime(refeedAction.services, refeedActionObject.setArray("services")); - stringsToSlime(refeedAction.messages, refeedActionObject.setArray("messages")); - } - return slime; - } - - private void serviceInfosToSlime(List<ServiceInfo> serviceInfoList, Cursor array) { - for (ServiceInfo serviceInfo : serviceInfoList) { - Cursor serviceInfoObject = array.addObject(); - serviceInfoObject.setString("serviceName", serviceInfo.serviceName); - serviceInfoObject.setString("serviceType", serviceInfo.serviceType); - serviceInfoObject.setString("configId", serviceInfo.configId); - serviceInfoObject.setString("hostName", serviceInfo.hostName); - } - } - private void stringsToSlime(List<String> strings, Cursor array) { for (String string : strings) array.addString(string); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java index ce7fa5784e8..8cf861ff963 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java @@ -124,15 +124,12 @@ public class ApplicationPackageTest { // services.xml is changed -> different bundle hash assertNotEquals(originalPackage.bundleHash(), changedServices.bundleHash()); - assertNotEquals(originalPackage.hash(), changedServices.hash()); // deployment.xml is changed, with real changes -> different bundle hash assertNotEquals(originalPackage.bundleHash(), changedDeploymentXml.bundleHash()); - assertNotEquals(originalPackage.hash(), changedDeploymentXml.hash()); // deployment.xml is changed, but only deployment orchestration settings -> same bundle hash assertEquals(originalPackage.bundleHash(), similarDeploymentXml.bundleHash()); - assertNotEquals(originalPackage.hash(), similarDeploymentXml.hash()); } private static Map<String, String> unzip(byte[] zip) { 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 8810e09dd40..7687a1561a4 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 @@ -412,8 +412,6 @@ public class InternalStepRunnerTest { tester.runner().run(); // Job run order determined by JobType enum order per application. tester.configServer().convergeServices(app.instanceId(), zone); assertEquals(unfinished, tester.jobs().run(id).stepStatuses().get(Step.installReal)); - assertEquals(applicationPackage().hash(), tester.configServer().application(app.instanceId(), zone).get().applicationPackage().hash()); - assertEquals(otherPackage.hash(), tester.configServer().application(app.instanceId(), DeploymentContext.perfUsEast3.zone()).get().applicationPackage().hash()); tester.configServer().setVersion(version, app.instanceId(), zone); tester.runner().run(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index 5a8d6a45e43..5b141716eaa 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -22,20 +22,17 @@ import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeploymentData; import com.yahoo.vespa.hosted.controller.api.application.v4.model.EndpointStatus; import com.yahoo.vespa.hosted.controller.api.application.v4.model.ProtonMetrics; -import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.ConfigChangeActions; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; -import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ApplicationReindexing; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ApplicationReindexing.Status; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Cluster; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServer; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ContainerEndpoint; +import com.yahoo.vespa.hosted.controller.api.integration.configserver.DeploymentResult; import com.yahoo.vespa.hosted.controller.api.integration.configserver.LoadBalancer; -import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node; import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeFilter; -import com.yahoo.vespa.hosted.controller.api.integration.configserver.PrepareResponse; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ProxyResponse; import com.yahoo.vespa.hosted.controller.api.integration.configserver.QuotaUsage; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ServiceConvergence; @@ -51,7 +48,6 @@ import java.io.InputStream; import java.net.URI; import java.time.Duration; import java.time.Instant; -import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -67,6 +63,7 @@ import java.util.function.Consumer; import java.util.function.Supplier; import java.util.logging.Level; import java.util.stream.Collectors; +import java.util.stream.IntStream; import static com.yahoo.config.provision.NodeResources.DiskSpeed.slow; import static com.yahoo.config.provision.NodeResources.StorageType.remote; @@ -89,7 +86,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer private final Set<DeploymentId> suspendedApplications = new HashSet<>(); private final Map<ZoneId, Set<LoadBalancer>> loadBalancers = new HashMap<>(); private final Set<Environment> deferLoadBalancerProvisioning = new HashSet<>(); - private final Map<DeploymentId, List<Log>> warnings = new HashMap<>(); + private final Map<DeploymentId, List<DeploymentResult.LogEntry>> warnings = new HashMap<>(); private final Map<DeploymentId, Set<ContainerEndpoint>> containerEndpoints = new HashMap<>(); private final Map<DeploymentId, List<ClusterMetrics>> clusterMetrics = new HashMap<>(); private final Map<DeploymentId, TestReport> testReport = new HashMap<>(); @@ -266,15 +263,13 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer } public void generateWarnings(DeploymentId deployment, int count) { - List<Log> logs = new ArrayList<>(count); - for (int i = 0; i < count; i++) { - Log log = new Log(); - log.time = Instant.now().toEpochMilli(); - log.level = Level.WARNING.getName(); - log.message = "log message " + (count + 1) + " generated by unit test"; - logs.add(log); - } - warnings.put(deployment, List.copyOf(logs)); + warnings.put(deployment, + IntStream.rangeClosed(1, count) + .mapToObj(i -> new DeploymentResult.LogEntry(Instant.now().toEpochMilli(), + "log message " + i + " generated by unit test", + Level.WARNING, + false)) + .toList()); } public Map<DeploymentId, Set<ContainerEndpoint>> containerEndpoints() { @@ -407,6 +402,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer Optional.of("dns-zone-1")))); } + // TODO jonmv: compute on deploy, not when getting the result. return () -> { Application application = applications.get(id); application.activate(); @@ -428,12 +424,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer 1)) .collect(Collectors.toList()))); - PrepareResponse prepareResponse = new PrepareResponse(); - prepareResponse.message = "foo"; - prepareResponse.configChangeActions = new ConfigChangeActions(List.of(), List.of(), List.of()); - prepareResponse.tenant = new TenantId("tenant"); - prepareResponse.log = warnings.getOrDefault(id, Collections.emptyList()); - return prepareResponse; + return new DeploymentResult("foo", warnings.getOrDefault(id, List.of())); }; } @@ -549,9 +540,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer @Override public QuotaUsage getQuotaUsage(DeploymentId deploymentId) { - var q = new QuotaUsage(); - q.rate = 42.42; - return q; + return new QuotaUsage(42); } @Override diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-result.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-result.json index f2736102968..5a037ad8201 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-result.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-result.json @@ -1,9 +1,4 @@ { - "revisionId": "da39a3ee5e6b4b0d3255bfef95601890afd80709", - "applicationZipSize": 0, - "prepareMessages": [ ], - "configChangeActions": { - "restart": [ ], - "refeed": [ ] - } + "message": "Deployed system application hosted-vespa.routing of type proxy in prod.us-central-1 on 6.2", + "prepareMessages": [ ] } diff --git a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/misc/SecurityHeadersResponseFilter.java b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/misc/SecurityHeadersResponseFilter.java index 520e22de136..0059fcf1d25 100644 --- a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/misc/SecurityHeadersResponseFilter.java +++ b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/misc/SecurityHeadersResponseFilter.java @@ -20,5 +20,6 @@ public class SecurityHeadersResponseFilter implements SecurityResponseFilter { response.setHeader("X-Content-Type-Options", "nosniff"); response.setHeader("X-Frame-Options", "DENY"); response.setHeader("Referrer-Policy", "strict-origin-when-cross-origin"); + response.setHeader("Vary", "*"); } } diff --git a/metrics-proxy/src/main/sh/start-telegraf.sh b/metrics-proxy/src/main/sh/start-telegraf.sh index 0cdfd784367..ca6549de5f8 100644 --- a/metrics-proxy/src/main/sh/start-telegraf.sh +++ b/metrics-proxy/src/main/sh/start-telegraf.sh @@ -79,7 +79,7 @@ fixddir () { echo "Creating data directory $1" mkdir -p $1 || exit 1 fi - if [ "${VESPA_USER}" ] && [ "$(id -u)" -eq 0 ]; then + if [ "${VESPA_USER}" ] && [ "${VESPA_UNPRIVILEGED}" != yes ]; then chown ${VESPA_USER} $1 fi chmod 755 $1 diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java index 87b5719cc19..61589078b69 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java @@ -284,13 +284,13 @@ public class DynamicProvisioningMaintainer extends NodeRepositoryMaintainer { try { Version osVersion = nodeRepository().osVersions().targetFor(NodeType.host).orElse(Version.emptyVersion); List<Integer> provisionIndices = nodeRepository().database().readProvisionIndices(count); - List<Node> hosts = hostProvisioner.provisionHosts(provisionIndices, NodeType.host, nodeResources, - ApplicationId.defaultId(), osVersion, HostSharing.shared, - Optional.empty(), Optional.empty()) - .stream() - .map(ProvisionedHost::generateHost) - .collect(Collectors.toList()); - nodeRepository().nodes().addNodes(hosts, Agent.DynamicProvisioningMaintainer); + List<Node> hosts = new ArrayList<>(); + hostProvisioner.provisionHosts(provisionIndices, NodeType.host, nodeResources, ApplicationId.defaultId(), + osVersion, HostSharing.shared, Optional.empty(), Optional.empty(), + provisionedHosts -> { + hosts.addAll(provisionedHosts.stream().map(ProvisionedHost::generateHost).toList()); + nodeRepository().nodes().addNodes(hosts, Agent.DynamicProvisioningMaintainer); + }); return hosts; } catch (NodeAllocationException | IllegalArgumentException | IllegalStateException e) { throw new NodeAllocationException("Failed to provision " + count + " " + nodeResources + ": " + e.getMessage(), diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/History.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/History.java index 4aca1cbd056..7da10ce085a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/History.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/History.java @@ -46,10 +46,8 @@ public class History { private static ImmutableMap<Event.Type, Event> toImmutableMap(Collection<Event> events) { ImmutableMap.Builder<Event.Type, Event> builder = new ImmutableMap.Builder<>(); - for (Event event : events) { - if (event.type() == Event.Type.requested) continue; // TODO (freva): Remove requested event after 8.70 + for (Event event : events) builder.put(event.type(), event); - } return builder.build(); } @@ -182,8 +180,6 @@ public class History { down, // The active node came up according to the service monitor up, - // The node made a config request, indicating it is live - requested, // The node resources/flavor were changed resized(false), // The node was rebooted diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java index 24c52f7b2e0..c0275de5798 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java @@ -423,7 +423,6 @@ public class NodeSerializer { case "deallocated" : return History.Event.Type.deallocated; case "down" : return History.Event.Type.down; case "up" : return History.Event.Type.up; - case "requested" : return History.Event.Type.requested; case "resized" : return History.Event.Type.resized; case "rebooted" : return History.Event.Type.rebooted; case "osUpgraded" : return History.Event.Type.osUpgraded; @@ -450,7 +449,6 @@ public class NodeSerializer { case deallocated : return "deallocated"; case down : return "down"; case up : return "up"; - case requested: return "requested"; case resized: return "resized"; case rebooted: return "rebooted"; case osUpgraded: return "osUpgraded"; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java index 35f04683157..e1657ec3358 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java @@ -15,8 +15,10 @@ import com.yahoo.vespa.hosted.provision.NodesAndHosts; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner.HostSharing; +import java.util.ArrayList; import java.util.List; import java.util.Optional; +import java.util.function.Consumer; import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; @@ -102,33 +104,35 @@ public class GroupPreparer { NodeAllocation allocation = prepareAllocation(application, cluster, requestedNodes, surplusActiveNodes, indices::next, wantedGroups, allNodesAndHosts); NodeType hostType = allocation.nodeType().hostType(); - if (canProvisionDynamically(hostType)) { + if (canProvisionDynamically(hostType) && allocation.hostDeficit().isPresent()) { HostSharing sharing = hostSharing(requestedNodes, hostType); Version osVersion = nodeRepository.osVersions().targetFor(hostType).orElse(Version.emptyVersion); - List<ProvisionedHost> provisionedHosts = allocation.hostDeficit() - .map(deficit -> hostProvisioner.get().provisionHosts(allocation.provisionIndices(deficit.count()), - hostType, - deficit.resources(), - application, - osVersion, - sharing, - Optional.of(cluster.type()), - requestedNodes.cloudAccount())) - .orElseGet(List::of); - - // At this point we have started provisioning of the hosts, the first priority is to make sure that - // the returned hosts are added to the node-repo so that they are tracked by the provision maintainers - List<Node> hosts = provisionedHosts.stream() - .map(ProvisionedHost::generateHost) - .collect(Collectors.toList()); - nodeRepository.nodes().addNodes(hosts, Agent.application); - - // Offer the nodes on the newly provisioned hosts, this should be enough to cover the deficit - List<NodeCandidate> candidates = provisionedHosts.stream() - .map(host -> NodeCandidate.createNewExclusiveChild(host.generateNode(), - host.generateHost())) - .collect(Collectors.toList()); - allocation.offer(candidates); + NodeAllocation.HostDeficit deficit = allocation.hostDeficit().get(); + + List<Node> hosts = new ArrayList<>(); + Consumer<List<ProvisionedHost>> provisionedHostsConsumer = provisionedHosts -> { + hosts.addAll(provisionedHosts.stream().map(ProvisionedHost::generateHost).toList()); + nodeRepository.nodes().addNodes(hosts, Agent.application); + + // Offer the nodes on the newly provisioned hosts, this should be enough to cover the deficit + List<NodeCandidate> candidates = provisionedHosts.stream() + .map(host -> NodeCandidate.createNewExclusiveChild(host.generateNode(), + host.generateHost())) + .collect(Collectors.toList()); + allocation.offer(candidates); + }; + + try { + hostProvisioner.get().provisionHosts( + allocation.provisionIndices(deficit.count()), hostType, deficit.resources(), application, + osVersion, sharing, Optional.of(cluster.type()), requestedNodes.cloudAccount(), provisionedHostsConsumer); + } catch (NodeAllocationException e) { + // Mark the nodes that were written to ZK in the consumer for deprovisioning. While these hosts do + // not exist, we cannot remove them from ZK here because other nodes may already have been + // allocated on them, so let DynamicProvisioningMaintainer deal with it + hosts.forEach(host -> nodeRepository.nodes().deprovision(host.hostname(), Agent.system, nodeRepository.clock().instant())); + throw e; + } } if (! allocation.fulfilled() && requestedNodes.canFail()) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java index 9b765adca89..1d95cdc09df 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java @@ -6,6 +6,7 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostEvent; +import com.yahoo.config.provision.NodeAllocationException; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.provision.Node; @@ -13,6 +14,7 @@ import com.yahoo.vespa.hosted.provision.Node; import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.function.Consumer; /** * A service which supports provisioning container hosts dynamically. @@ -45,16 +47,21 @@ public interface HostProvisioner { * @param sharing puts requirements on sharing or exclusivity of the host to be provisioned. * @param clusterType provision host exclusively for this cluster type * @param cloudAccount the cloud account to use - * @return list of {@link ProvisionedHost} describing the provisioned nodes + * @param provisionedHostConsumer consumer of {@link ProvisionedHost}s describing the provisioned nodes, + * the {@link Node} returned from {@link ProvisionedHost#generateHost()} must be + * written to ZK immediately in case the config server goes down while waiting + * for the provisioning to finish. + * @throws NodeAllocationException if the cloud provider cannot satisfy the request */ - List<ProvisionedHost> provisionHosts(List<Integer> provisionIndices, - NodeType hostType, - NodeResources resources, - ApplicationId applicationId, - Version osVersion, - HostSharing sharing, - Optional<ClusterSpec.Type> clusterType, - Optional<CloudAccount> cloudAccount); + void provisionHosts(List<Integer> provisionIndices, + NodeType hostType, + NodeResources resources, + ApplicationId applicationId, + Version osVersion, + HostSharing sharing, + Optional<ClusterSpec.Type> clusterType, + Optional<CloudAccount> cloudAccount, + Consumer<List<ProvisionedHost>> provisionedHostConsumer) throws NodeAllocationException; /** * Continue provisioning of given list of Nodes. diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java index 3ebaf764115..f9af176fc00 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java @@ -26,6 +26,7 @@ import java.util.HashSet; import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.function.Consumer; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -60,10 +61,10 @@ public class MockHostProvisioner implements HostProvisioner { } @Override - public List<ProvisionedHost> provisionHosts(List<Integer> provisionIndices, NodeType hostType, NodeResources resources, - ApplicationId applicationId, Version osVersion, HostSharing sharing, - Optional<ClusterSpec.Type> clusterType, - Optional<CloudAccount> cloudAccount) { + public void provisionHosts(List<Integer> provisionIndices, NodeType hostType, NodeResources resources, + ApplicationId applicationId, Version osVersion, HostSharing sharing, + Optional<ClusterSpec.Type> clusterType, Optional<CloudAccount> cloudAccount, + Consumer<List<ProvisionedHost>> provisionedHostsConsumer) { Flavor hostFlavor = this.hostFlavor.orElseGet(() -> flavors.stream().filter(f -> compatible(f, resources)) .findFirst() .orElseThrow(() -> new NodeAllocationException("No host flavor matches " + resources, true))); @@ -82,7 +83,7 @@ public class MockHostProvisioner implements HostProvisioner { cloudAccount)); } provisionedHosts.addAll(hosts); - return hosts; + provisionedHostsConsumer.accept(hosts); } @Override diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java index 436cf880f1c..3570e56e196 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java @@ -30,6 +30,7 @@ import java.time.Instant; import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.function.Consumer; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -41,6 +42,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -72,8 +74,8 @@ public class DynamicProvisioningTest { mockHostProvisioner(hostProvisioner, "large", 3, null); // Provision shared hosts prepareAndActivate(application1, clusterSpec("mycluster"), 4, 1, resources); - verify(hostProvisioner).provisionHosts(List.of(100, 101, 102, 103), NodeType.host, resources, application1, - Version.emptyVersion, HostSharing.any, Optional.of(ClusterSpec.Type.content), Optional.empty()); + verify(hostProvisioner).provisionHosts(eq(List.of(100, 101, 102, 103)), eq(NodeType.host), eq(resources), eq(application1), + eq(Version.emptyVersion), eq(HostSharing.any), eq(Optional.of(ClusterSpec.Type.content)), eq(Optional.empty()), any()); // Total of 8 nodes should now be in node-repo, 4 active hosts and 4 active nodes assertEquals(8, tester.nodeRepository().nodes().list().size()); @@ -96,8 +98,8 @@ public class DynamicProvisioningTest { ApplicationId application3 = ProvisioningTester.applicationId(); mockHostProvisioner(hostProvisioner, "large", 3, application3); prepareAndActivate(application3, clusterSpec("mycluster", true), 4, 1, resources); - verify(hostProvisioner).provisionHosts(List.of(104, 105, 106, 107), NodeType.host, resources, application3, - Version.emptyVersion, HostSharing.exclusive, Optional.of(ClusterSpec.Type.content), Optional.empty()); + verify(hostProvisioner).provisionHosts(eq(List.of(104, 105, 106, 107)), eq(NodeType.host), eq(resources), eq(application3), + eq(Version.emptyVersion), eq(HostSharing.exclusive), eq(Optional.of(ClusterSpec.Type.content)), eq(Optional.empty()), any()); // Total of 20 nodes should now be in node-repo, 8 active hosts and 12 active nodes assertEquals(20, tester.nodeRepository().nodes().list().size()); @@ -450,14 +452,14 @@ public class DynamicProvisioningTest { return new ClusterResources(nodes, groups, new NodeResources(vcpu, memory, disk, 0.1, diskSpeed, storageType)); } - @SuppressWarnings("unchecked") private void mockHostProvisioner(HostProvisioner hostProvisioner, String hostFlavorName, int numIps, ApplicationId exclusiveTo) { doAnswer(invocation -> { Flavor hostFlavor = tester.nodeRepository().flavors().getFlavorOrThrow(hostFlavorName); - List<Integer> provisionIndexes = (List<Integer>) invocation.getArguments()[0]; - NodeResources nodeResources = (NodeResources) invocation.getArguments()[2]; + List<Integer> provisionIndexes = invocation.getArgument(0); + NodeResources nodeResources = invocation.getArgument(2); + Consumer<List<ProvisionedHost>> provisionedHostConsumer = invocation.getArgument(8); - return provisionIndexes.stream() + List<ProvisionedHost> provisionedHosts = provisionIndexes.stream() .map(hostIndex -> { String hostHostname = "host-" + hostIndex; String hostIp = "::" + hostIndex + ":0"; @@ -475,9 +477,10 @@ public class DynamicProvisioningTest { when(provisionedHost.generateHost()).thenReturn(parent); when(provisionedHost.generateNode()).thenReturn(child); return provisionedHost; - }) - .collect(Collectors.toList()); - }).when(hostProvisioner).provisionHosts(any(), any(), any(), any(), any(), any(), any(), any()); + }).toList(); + provisionedHostConsumer.accept(provisionedHosts); + return null; + }).when(hostProvisioner).provisionHosts(any(), any(), any(), any(), any(), any(), any(), any(), any()); } } diff --git a/searchlib/CMakeLists.txt b/searchlib/CMakeLists.txt index 30d00410758..20df8cd3683 100644 --- a/searchlib/CMakeLists.txt +++ b/searchlib/CMakeLists.txt @@ -190,6 +190,7 @@ vespa_define_module( src/tests/queryeval/dot_product src/tests/queryeval/equiv src/tests/queryeval/fake_searchable + src/tests/queryeval/filter_search src/tests/queryeval/getnodeweight src/tests/queryeval/global_filter src/tests/queryeval/matching_elements_search diff --git a/searchlib/src/tests/queryeval/filter_search/CMakeLists.txt b/searchlib/src/tests/queryeval/filter_search/CMakeLists.txt new file mode 100644 index 00000000000..ad2da1f6eed --- /dev/null +++ b/searchlib/src/tests/queryeval/filter_search/CMakeLists.txt @@ -0,0 +1,10 @@ +# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(searchlib_filter_search_test_app TEST + SOURCES + filter_search_test.cpp + DEPENDS + searchlib + searchlib_test + GTest::GTest +) +vespa_add_test(NAME searchlib_filter_search_test_app COMMAND searchlib_filter_search_test_app) diff --git a/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp b/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp new file mode 100644 index 00000000000..fe9f6424b9c --- /dev/null +++ b/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp @@ -0,0 +1,157 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/searchlib/queryeval/simpleresult.h> +#include <vespa/searchlib/queryeval/blueprint.h> +#include <vespa/searchlib/queryeval/leaf_blueprints.h> +#include <vespa/vespalib/util/trinary.h> +#include <vespa/vespalib/util/require.h> +#include <vespa/vespalib/gtest/gtest.h> +#include <functional> + +using search::queryeval::AlwaysTrueBlueprint; +using search::queryeval::Blueprint; +using search::queryeval::EmptyBlueprint; +using search::queryeval::SimpleBlueprint; +using search::queryeval::SimpleResult; +using search::queryeval::SearchIterator; +using vespalib::Trinary; + +using Constraint = Blueprint::FilterConstraint; +constexpr auto lower_bound = Constraint::LOWER_BOUND; +constexpr auto upper_bound = Constraint::UPPER_BOUND; + +const uint32_t docid_limit = 100; + +template <typename T> +concept FilterFactory = requires(const T &a, bool strict, Constraint upper_or_lower) { + { a.createFilterSearch(strict, upper_or_lower) } -> std::same_as<std::unique_ptr<SearchIterator>>; +}; + +using factory_fun = std::function<std::unique_ptr<SearchIterator>(const std::vector<Blueprint*> &, bool, Constraint)>; + +// Combine children blueprints using a shared filter creation +// algorithm. Satisfies the FilterFactory concept. +struct Combine { + factory_fun fun; + std::vector<Blueprint*> list; + Combine(factory_fun fun_in) : fun(fun_in), list() {} + Combine &&add(std::unique_ptr<Blueprint> child) && { + list.push_back(child.release()); + return std::move(*this); + } + auto createFilterSearch(bool strict, Constraint upper_or_lower) const { + return fun(list, strict, upper_or_lower); + } + ~Combine() { + for (auto *ptr: list) { + delete ptr; + } + } +}; + +// create a leaf blueprint that matches no documents +std::unique_ptr<Blueprint> empty() { + return std::make_unique<EmptyBlueprint>(); +} + +// create a leaf blueprint that matches all documents +std::unique_ptr<Blueprint> full() { + return std::make_unique<AlwaysTrueBlueprint>(); +} + +// make a simple result containing the given documents +SimpleResult make_result(const std::vector<uint32_t> &docs) { + SimpleResult result; + for (uint32_t doc: docs) { + result.addHit(doc); + } + return result; +} + +// make a simple result containing all the documents +SimpleResult make_full_result() { + SimpleResult result; + for (uint32_t docid = 1; docid < docid_limit; ++docid) { + result.addHit(docid); + } + return result; +} + +// make a simple result containing none of the documents +SimpleResult make_empty_result() { + return SimpleResult(); +} + +// create a leaf blueprint with the specified hits +std::unique_ptr<Blueprint> leaf(const std::vector<uint32_t> &docs) { + return std::make_unique<SimpleBlueprint>(make_result(docs)); +} + +// what kind of results are we expecting from a filter search? +struct Expect { + Trinary matches_any; + SimpleResult hits; + Expect(const std::vector<uint32_t> &hits_in) + : matches_any(Trinary::Undefined), hits(make_result(hits_in)) {} + Expect(Trinary matches_any_in) : matches_any(matches_any_in), hits() { + REQUIRE(matches_any != Trinary::Undefined); + if (matches_any == Trinary::True) { + hits = make_full_result(); + } else { + hits = make_empty_result(); + } + } +}; + +template <FilterFactory Blueprint> +void verify(const Blueprint &blueprint, const Expect &upper, const Expect &lower) { + for (auto constraint: {lower_bound, upper_bound}) { + const Expect &expect = (constraint == upper_bound) ? upper : lower; + for (bool strict: {false, true}) { + auto filter = blueprint.createFilterSearch(strict, constraint); + EXPECT_EQ(filter->matches_any(), expect.matches_any); + SimpleResult actual; + if (strict) { + actual.searchStrict(*filter, docid_limit); + } else { + actual.search(*filter, docid_limit); + } + EXPECT_EQ(actual, expect.hits); + } + } +} + +template <FilterFactory Blueprint> +void verify(const Blueprint &blueprint, const Expect &upper_and_lower) { + verify(blueprint, upper_and_lower, upper_and_lower); +} + +TEST(FilterSearchTest, empty_leaf) { + verify(*empty(), Expect(Trinary::False)); +} + +TEST(FilterSearchTest, full_leaf) { + verify(*full(), Expect(Trinary::True)); +} + +TEST(FilterSearchTest, custom_leaf) { + verify(*leaf({5,10,20}), Expect({5,10,20})); +} + +TEST(FilterSearchTest, simple_or) { + verify(Combine(Blueprint::create_or_filter) + .add(leaf({5, 10})) + .add(leaf({7})) + .add(leaf({3, 11})), + Expect({3, 5, 7, 10, 11})); +} + +TEST(FilterSearchTest, simple_and) { + verify(Combine(Blueprint::create_and_filter) + .add(leaf({1, 2, 3, 4, 5, 6})) + .add(leaf({2, 4, 6, 7})) + .add(leaf({1, 4, 6, 7, 10})), + Expect({4, 6})); +} + +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/vdslib/src/vespa/vdslib/state/CMakeLists.txt b/vdslib/src/vespa/vdslib/state/CMakeLists.txt index 8d4e994da93..49f1c942724 100644 --- a/vdslib/src/vespa/vdslib/state/CMakeLists.txt +++ b/vdslib/src/vespa/vdslib/state/CMakeLists.txt @@ -1,6 +1,7 @@ # Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. vespa_add_library(vdslib_state OBJECT SOURCES + globals.cpp nodetype.cpp node.cpp state.cpp diff --git a/vdslib/src/vespa/vdslib/state/clusterstate.cpp b/vdslib/src/vespa/vdslib/state/clusterstate.cpp index e58389e1475..e9159eef631 100644 --- a/vdslib/src/vespa/vdslib/state/clusterstate.cpp +++ b/vdslib/src/vespa/vdslib/state/clusterstate.cpp @@ -1,5 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "clusterstate.h" +#include "globals.h" #include <vespa/vespalib/text/stringtokenizer.h> #include <vespa/document/util/stringutil.h> @@ -13,6 +14,11 @@ LOG_SETUP(".vdslib.state.cluster"); using vespalib::IllegalArgumentException; +using storage::lib::clusterstate::_G_defaultSDState; +using storage::lib::clusterstate::_G_defaultDDState; +using storage::lib::clusterstate::_G_defaultSUState; +using storage::lib::clusterstate::_G_defaultDUState; + namespace storage::lib { ClusterState::ClusterState() @@ -266,10 +272,6 @@ ClusterState::getNodeCount(const NodeType& type) const } namespace { - NodeState _G_defaultSDState(NodeType::STORAGE, State::DOWN); - NodeState _G_defaultDDState(NodeType::DISTRIBUTOR, State::DOWN); - NodeState _G_defaultSUState(NodeType::STORAGE, State::UP); - NodeState _G_defaultDUState(NodeType::DISTRIBUTOR, State::UP); [[noreturn]] void throwUnknownType(const Node & node) __attribute__((noinline)); void throwUnknownType(const Node & node) { throw vespalib::IllegalStateException("Unknown node type " + node.getType().toString(), VESPA_STRLOC); diff --git a/vdslib/src/vespa/vdslib/state/globals.cpp b/vdslib/src/vespa/vdslib/state/globals.cpp new file mode 100644 index 00000000000..08c314e70f9 --- /dev/null +++ b/vdslib/src/vespa/vdslib/state/globals.cpp @@ -0,0 +1,24 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "globals.h" + +namespace storage::lib { + +const State State::UNKNOWN("Unknown", "-", 0, true, true, false, false, false); +const State State::MAINTENANCE("Maintenance", "m", 1, false, false, true, true, false); +const State State::DOWN("Down", "d", 2, false, false, true, true, true); +const State State::STOPPING("Stopping", "s", 3, true, true, false, false, true); +const State State::INITIALIZING("Initializing", "i", 4, true, true, false, false, true); +const State State::RETIRED("Retired", "r", 5, false, false, true, true, false); +const State State::UP("Up", "u", 6, true, true, true, true, true); + +} + +namespace storage::lib::clusterstate { + +NodeState _G_defaultSDState(NodeType::STORAGE, State::DOWN); +NodeState _G_defaultDDState(NodeType::DISTRIBUTOR, State::DOWN); +NodeState _G_defaultSUState(NodeType::STORAGE, State::UP); +NodeState _G_defaultDUState(NodeType::DISTRIBUTOR, State::UP); + +} diff --git a/vdslib/src/vespa/vdslib/state/globals.h b/vdslib/src/vespa/vdslib/state/globals.h new file mode 100644 index 00000000000..e3a88e6abfa --- /dev/null +++ b/vdslib/src/vespa/vdslib/state/globals.h @@ -0,0 +1,13 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "state.h" +#include "nodestate.h" + +namespace storage::lib::clusterstate { + +extern NodeState _G_defaultSDState; +extern NodeState _G_defaultDDState; +extern NodeState _G_defaultSUState; +extern NodeState _G_defaultDUState; + +} diff --git a/vdslib/src/vespa/vdslib/state/state.cpp b/vdslib/src/vespa/vdslib/state/state.cpp index e8f243b1a1e..ffa25c5a543 100644 --- a/vdslib/src/vespa/vdslib/state/state.cpp +++ b/vdslib/src/vespa/vdslib/state/state.cpp @@ -6,14 +6,6 @@ namespace storage::lib { -const State State::UNKNOWN("Unknown", "-", 0, true, true, false, false, false); -const State State::MAINTENANCE("Maintenance", "m", 1, false, false, true, true, false); -const State State::DOWN("Down", "d", 2, false, false, true, true, true); -const State State::STOPPING("Stopping", "s", 3, true, true, false, false, true); -const State State::INITIALIZING("Initializing", "i", 4, true, true, false, false, true); -const State State::RETIRED("Retired", "r", 5, false, false, true, true, false); -const State State::UP("Up", "u", 6, true, true, true, true, true); - const State& State::get(vespalib::stringref serialized) { diff --git a/vespabase/src/common-env.sh b/vespabase/src/common-env.sh index 00bc6652699..628ebe6b074 100755 --- a/vespabase/src/common-env.sh +++ b/vespabase/src/common-env.sh @@ -207,45 +207,26 @@ consider_fallback VESPA_USE_NO_VESPAMALLOC "vespa-rpc-invoke vespa-get-config v fixlimits () { - max_processes_limit=409600 - if ! varhasvalue file_descriptor_limit; then - file_descriptor_limit=262144 + # Cannot bump limits when not root (for testing) + if [ "${VESPA_UNPRIVILEGED}" = yes ]; then + return 0 + fi + # number of open files: + if varhasvalue file_descriptor_limit; then + ulimit -n ${file_descriptor_limit} || exit 1 + elif [ `ulimit -n` -lt 262144 ]; then + ulimit -n 262144 || exit 1 fi - max_processes=$(ulimit -u) - core_size=$(ulimit -c) - file_descriptor=$(ulimit -n) - # Warn if we Cannot bump limits when not root - if [ "$(id -u)" -ne 0 ]; then - # number of open files: - if [ $file_descriptor -lt $file_descriptor_limit ]; then - echo "Expected file descriptor limit to be at least $file_descriptor_limit, was $file_descriptor" - fi - - # core file size - if [ "$core_size" != "unlimited" ]; then - echo "Expected core file size to be unlimited, was $core_size" - fi - - # number of processes/threads - if [ "$max_processes" != "unlimited" ] && [ "$max_processes" -lt "$max_processes_limit" ]; then - echo "Expected max processes to be at least $max_processes_limit, was $max_processes" - fi - else - # number of open files: - if [ $file_descriptor -lt $file_descriptor_limit ]; then - ulimit -n "$file_descriptor_limit" || exit 1 - fi - - # core file size - if [ "$core_size" != "unlimited" ]; then - ulimit -c unlimited - fi + # core file size + if [ `ulimit -c` != "unlimited" ]; then + ulimit -c unlimited + fi - # number of processes/threads - if [ "$max_processes" != "unlimited" ] && [ "$max_processes" -lt "$max_processes_limit" ]; then - ulimit -u "$max_processes_limit" - fi + # number of processes/threads + max_processes=`ulimit -u` + if [ "$max_processes" != "unlimited" ] && [ "$max_processes" -lt 409600 ]; then + ulimit -u 409600 fi } diff --git a/vespabase/src/rhel-prestart.sh b/vespabase/src/rhel-prestart.sh index 0aedfb4622d..79a8e61848c 100755 --- a/vespabase/src/rhel-prestart.sh +++ b/vespabase/src/rhel-prestart.sh @@ -85,7 +85,6 @@ fi if [ "$VESPA_GROUP" = "" ]; then VESPA_GROUP=$(id -rgn) fi -IS_ROOT=$([ "$(id -ru)" == "0" ] && echo true || echo false) cd $VESPA_HOME || { echo "Cannot cd to $VESPA_HOME" 1>&2; exit 1; } @@ -95,21 +94,9 @@ fixdir () { exit 1 fi mkdir -p "$4" - if ! $IS_ROOT; then - local stat="$(stat -c "%U %G" $4)" - local user=${stat% *} - local group=${stat#* } - if [ "$1" != "$user" ]; then - echo "Wrong owner for $VESPA_HOME/$4, expected $1, was $user" - exit 1 - fi - if [ "$2" != "$group" ]; then - echo "Wrong group for $VESPA_HOME/$4, expected $2, was $group" - exit 1 - fi - else - chown $1 "$4" - chgrp $2 "$4" + if [ "${VESPA_UNPRIVILEGED}" != yes ]; then + chown $1 "$4" + chgrp $2 "$4" fi chmod $3 "$4" } @@ -143,9 +130,9 @@ fixdir ${VESPA_USER} ${VESPA_GROUP} 755 var/vespa/bundlecache fixdir ${VESPA_USER} ${VESPA_GROUP} 755 var/vespa/bundlecache/configserver fixdir ${VESPA_USER} ${VESPA_GROUP} 755 var/vespa/cache/config -if [ "$(id -u)" -eq 0 ]; then - chown -hR ${VESPA_USER} logs/vespa - chown -hR ${VESPA_USER} var/db/vespa +if [ "${VESPA_UNPRIVILEGED}" != yes ]; then + chown -hR ${VESPA_USER} logs/vespa + chown -hR ${VESPA_USER} var/db/vespa fi # END directory fixups diff --git a/vespalog/src/test/bufferedlogskiptest.cpp b/vespalog/src/test/bufferedlogskiptest.cpp index 56f59541eaf..ac0bb585be9 100644 --- a/vespalog/src/test/bufferedlogskiptest.cpp +++ b/vespalog/src/test/bufferedlogskiptest.cpp @@ -61,9 +61,9 @@ void testSkipBufferOnDebug(const std::string& file, uint64_t & timer) void reset(uint64_t & timer) { timer = 0; - ns_log::BufferedLogger::logger.setMaxCacheSize(10); - ns_log::BufferedLogger::logger.setMaxEntryAge(300); - ns_log::BufferedLogger::logger.setCountFactor(5); + ns_log::BufferedLogger::instance().setMaxCacheSize(10); + ns_log::BufferedLogger::instance().setMaxEntryAge(300); + ns_log::BufferedLogger::instance().setCountFactor(5); } int @@ -76,7 +76,7 @@ main(int argc, char **argv) ns_log::Logger::fakePid = true; uint64_t timer; logger.setTimer(std::unique_ptr<ns_log::Timer>(new ns_log::TestTimer(timer))); - ns_log::BufferedLogger::logger.setTimer(std::unique_ptr<ns_log::Timer>(new ns_log::TestTimer(timer))); + ns_log::BufferedLogger::instance().setTimer(std::unique_ptr<ns_log::Timer>(new ns_log::TestTimer(timer))); reset(timer); testSkipBufferOnDebug(argv[1], timer); diff --git a/vespalog/src/test/bufferedlogtest.cpp b/vespalog/src/test/bufferedlogtest.cpp index 39990a26b2a..8399fa81dfa 100644 --- a/vespalog/src/test/bufferedlogtest.cpp +++ b/vespalog/src/test/bufferedlogtest.cpp @@ -112,9 +112,9 @@ void testThatEntriesWithHighCountsAreEventuallyRemoved( // Should eventually throw out the entries with high count timer = 10 * 1000000 + 4; // Make sure we don't remove due to age. - ns_log::BufferedLogger::logger.setMaxEntryAge(1000000); + ns_log::BufferedLogger::instance().setMaxEntryAge(1000000); // Let each count, count for 5 seconds. - ns_log::BufferedLogger::logger.setCountFactor(5); + ns_log::BufferedLogger::instance().setCountFactor(5); LOGBM(info, "Starting up, using logfile %s", file.c_str()); timer = 100 * 1000000 + 4; @@ -147,9 +147,9 @@ void testThatEntriesExpire( // Test that we don't keep entries longer than max age timer = 10 * 1000000 + 4; // Time out after 120 seconds - ns_log::BufferedLogger::logger.setMaxEntryAge(120); + ns_log::BufferedLogger::instance().setMaxEntryAge(120); // Let counts count much, so they expire due to time instead - ns_log::BufferedLogger::logger.setCountFactor(100000); + ns_log::BufferedLogger::instance().setCountFactor(100000); LOGBM(info, "Starting up, using logfile %s", file.c_str()); timer = 100 * 1000000 + 4; @@ -217,9 +217,9 @@ void testThatHighCountEntriesDontStarveOthers( std::cerr << "testThatHighCountEntriesDontStarveOthers ...\n"; timer = 10 * 1000000 + 4; // Long time out, we don't want to rely on timeout to prevent starvation - ns_log::BufferedLogger::logger.setMaxEntryAge(12000000); + ns_log::BufferedLogger::instance().setMaxEntryAge(12000000); // Let counts count much, so they score high - ns_log::BufferedLogger::logger.setCountFactor(100000); + ns_log::BufferedLogger::instance().setCountFactor(100000); LOGBM(info, "Starting up, using logfile %s", file.c_str()); timer = 100 * 1000000; @@ -372,8 +372,8 @@ void testNonBufferedLoggerTriggersBufferedLogTrim(const std::string& file, void reset(uint64_t& timer) { timer = 0; - ns_log::BufferedLogger::logger.setMaxEntryAge(300); - ns_log::BufferedLogger::logger.setCountFactor(5); + ns_log::BufferedLogger::instance().setMaxEntryAge(300); + ns_log::BufferedLogger::instance().setCountFactor(5); } int @@ -384,10 +384,10 @@ main(int argc, char **argv) return EXIT_FAILURE; } ns_log::Logger::fakePid = true; - ns_log::BufferedLogger::logger.setMaxCacheSize(10); + ns_log::BufferedLogger::instance().setMaxCacheSize(10); uint64_t timer; logger.setTimer(std::unique_ptr<ns_log::Timer>(new ns_log::TestTimer(timer))); - ns_log::BufferedLogger::logger.setTimer(std::unique_ptr<ns_log::Timer>(new ns_log::TestTimer(timer))); + ns_log::BufferedLogger::instance().setTimer(std::unique_ptr<ns_log::Timer>(new ns_log::TestTimer(timer))); reset(timer); testThatEntriesWithHighCountIsKept(argv[1], timer); diff --git a/vespalog/src/vespa/log/bufferedlogger.cpp b/vespalog/src/vespa/log/bufferedlogger.cpp index 607c8f06766..33ff3da7366 100644 --- a/vespalog/src/vespa/log/bufferedlogger.cpp +++ b/vespalog/src/vespa/log/bufferedlogger.cpp @@ -181,8 +181,6 @@ BackingBuffer::Entry::getAgeFactor() const return _timestamp + _countFactor * _count; } -BufferedLogger BufferedLogger::logger; - BackingBuffer::BackingBuffer() : _timer(new Timer), _mutex(), @@ -383,4 +381,11 @@ BufferedLogger::setTimer(std::unique_ptr<Timer> timer) _backing->_timer = std::move(timer); } +BufferedLogger& +BufferedLogger::instance() +{ + static BufferedLogger logger; + return logger; +} + } // ns_log diff --git a/vespalog/src/vespa/log/bufferedlogger.h b/vespalog/src/vespa/log/bufferedlogger.h index d31425921f4..373f81b5160 100644 --- a/vespalog/src/vespa/log/bufferedlogger.h +++ b/vespalog/src/vespa/log/bufferedlogger.h @@ -95,9 +95,9 @@ if (logger.wants(ns_log::Logger::debug)) { \ logger.doLog(ns_log::Logger::level, \ __FILE__, __LINE__, __VA_ARGS__); \ - ns_log::BufferedLogger::logger.trimCache(); \ + ns_log::BufferedLogger::instance().trimCache(); \ } else { \ - ns_log::BufferedLogger::logger.doLog(logger, \ + ns_log::BufferedLogger::instance().doLog(logger, \ ns_log::Logger::level, __FILE__, __LINE__, \ "", __VA_ARGS__); \ } \ @@ -108,15 +108,15 @@ // Define LOGBM macro for logging buffered, using the message itself as a // token. This is the same as LOG defined above if // VESPA_LOG_USELOGBUFFERFORREGULARLOG is defined. -#define LOGBM(level, ...) \ - do { \ +#define LOGBM(level, ...) \ + do { \ if (logger.wants(ns_log::Logger::level)) { \ if (logger.wants(ns_log::Logger::debug)) { \ logger.doLog(ns_log::Logger::level, \ __FILE__, __LINE__, __VA_ARGS__); \ - ns_log::BufferedLogger::logger.trimCache(); \ + ns_log::BufferedLogger::instance().trimCache(); \ } else { \ - ns_log::BufferedLogger::logger.doLog(logger, \ + ns_log::BufferedLogger::instance().doLog(logger, \ ns_log::Logger::level, __FILE__, __LINE__, \ "", __VA_ARGS__); \ } \ @@ -131,11 +131,11 @@ if (logger.wants(ns_log::Logger::debug)) { \ logger.doLog(ns_log::Logger::level, \ __FILE__, __LINE__, ##ARGS); \ - ns_log::BufferedLogger::logger.trimCache(); \ + ns_log::BufferedLogger::instance().trimCache(); \ } else { \ std::ostringstream ost123; \ ost123 << __FILE__ << ":" << __LINE__; \ - ns_log::BufferedLogger::logger.doLog(logger, \ + ns_log::BufferedLogger::instance().doLog(logger, \ ns_log::Logger::level, \ __FILE__, __LINE__, ost123.str(), ##ARGS); \ } \ @@ -149,9 +149,9 @@ if (logger.wants(ns_log::Logger::debug)) { \ logger.doLog(ns_log::Logger::level, \ __FILE__, __LINE__, __VA_ARGS__); \ - ns_log::BufferedLogger::logger.trimCache(); \ + ns_log::BufferedLogger::instance().trimCache(); \ } else { \ - ns_log::BufferedLogger::logger.doLog(logger, \ + ns_log::BufferedLogger::instance().doLog(logger, \ ns_log::Logger::level, \ __FILE__, __LINE__, token, __VA_ARGS__); \ } \ @@ -159,7 +159,7 @@ } while (false) #define LOGB_FLUSH() \ - ns_log::BufferedLogger::logger.flush() + ns_log::BufferedLogger::instance().flush() namespace ns_log { @@ -171,8 +171,6 @@ class BufferedLogger { BufferedLogger & operator = (const BufferedLogger & buf); public: - static BufferedLogger logger; - BufferedLogger(); ~BufferedLogger(); @@ -198,6 +196,8 @@ public: /** Trim the buffer. Removing old messages if wanted. */ void trimCache(); + + static BufferedLogger& instance(); }; } // ns_log diff --git a/vespalog/src/vespa/log/log.cpp b/vespalog/src/vespa/log/log.cpp index a7394b5848b..7f2668a97ce 100644 --- a/vespalog/src/vespa/log/log.cpp +++ b/vespalog/src/vespa/log/log.cpp @@ -239,7 +239,7 @@ Logger::doLog(LogLevel level, const char *file, int line, const char *fmt, ...) actualSize = tryLog(sizeofPayload, level, file, line, fmt, args); va_end(args); } while (sizeofPayload < actualSize); - ns_log::BufferedLogger::logger.trimCache(); + ns_log::BufferedLogger::instance().trimCache(); } void Logger::doLogCore(uint64_t timestamp, LogLevel level, diff --git a/vespamalloc/src/tests/overwrite/overwrite.cpp b/vespamalloc/src/tests/overwrite/overwrite.cpp index aae5cccb696..b98bd3831d3 100644 --- a/vespamalloc/src/tests/overwrite/overwrite.cpp +++ b/vespamalloc/src/tests/overwrite/overwrite.cpp @@ -17,6 +17,13 @@ void overwrite_memory_real(char *ptr, int offset) void (*overwrite_memory)(char *ptr, int offset) = overwrite_memory_real; +char *new_vec_real(size_t size) +{ + return new char[size]; +} + +char *(*new_vec)(size_t size) = new_vec_real; + void delete_vec_real(char *ptr) { delete [] ptr; @@ -47,7 +54,7 @@ void Test::testFillValue(char *a) // Make sure that enough blocks of memory is allocated and freed. for (size_t i(0); i < 100; i++) { - char *d = new char[256]; + char *d = new_vec(256); memset(d, 0x77, 256); check_ptr(d); delete_vec(d); @@ -59,13 +66,13 @@ void Test::testFillValue(char *a) // Make sure we trigger vespamallocd detection of memory written after delete. char *aa[1024]; for (size_t i(0); i < sizeof(aa)/sizeof(aa[0]); i++) { - aa[i] = new char[256]; + aa[i] = new_vec(256); } // Verify overwrite detection in place after cleaning up. for (size_t i(0); i < sizeof(aa)/sizeof(aa[0]); i++) { check_ptr(aa[i]); - delete [] aa[i]; + delete_vec(aa[i]); EXPECT_EQUAL((int)a[0], 0x66); EXPECT_EQUAL((int)a[1], 0x66); EXPECT_EQUAL((int)a[255], 0x66); @@ -74,28 +81,28 @@ void Test::testFillValue(char *a) void Test::verifyPreWriteDetection() { - char * a = new char[8]; + char * a = new_vec(8); overwrite_memory(a, -1); - delete [] a; + delete_vec(a); } void Test::verifyPostWriteDetection() { - char * a = new char[8]; + char * a = new_vec(8); overwrite_memory(a, 8); - delete [] a; + delete_vec(a); } void Test::verifyWriteAfterFreeDetection() { // Make sure that enough blocks of memory is allocated and freed. - char * a = new char[256]; + char * a = new_vec(256); check_ptr(a); delete_vec(a); for (size_t i(0); i < 100; i++) { - char *d = new char[256]; + char *d = new_vec(256); check_ptr(d); - delete [] d; + delete_vec(d); } // Write freed memory. a[0] = 0; @@ -103,13 +110,13 @@ void Test::verifyWriteAfterFreeDetection() // Make sure we trigger vespamallocd detection of memory written after delete. char *aa[1024]; for (size_t i(0); i < sizeof(aa)/sizeof(aa[0]); i++) { - aa[i] = new char[256]; + aa[i] = new_vec(256); } // Clean up. for (size_t i(0); i < sizeof(aa)/sizeof(aa[0]); i++) { check_ptr(aa[i]); - delete [] aa[i]; + delete_vec(aa[i]); } } @@ -117,7 +124,7 @@ int Test::Main() { TEST_INIT("overwrite_test"); - char * a = new char[256]; + char * a = new_vec(256); memset(a, 0x77, 256); a[0] = 0; EXPECT_EQUAL((int)a[0], 0); @@ -126,7 +133,7 @@ int Test::Main() char * b = a; EXPECT_EQUAL(a, b); check_ptr(a); - delete [] a; + delete_vec(a); EXPECT_EQUAL(a, b); if (_argc > 1) { |