diff options
148 files changed, 790 insertions, 927 deletions
diff --git a/client/go/jvm/application_container.go b/client/go/jvm/application_container.go index b7c9a88b5fd..bb2e34b6c76 100644 --- a/client/go/jvm/application_container.go +++ b/client/go/jvm/application_container.go @@ -146,14 +146,14 @@ func (a *ApplicationContainer) configureOptions() { opts.AddJvmArgsFromString(env) } qrStartCfg := a.getQrStartCfg() - a.configureCPU(qrStartCfg) - a.configureMemory(qrStartCfg) - a.configureGC(qrStartCfg) - a.configureClasspath(qrStartCfg) opts.AddOption("-Djdisc.export.packages=" + qrStartCfg.Jdisc.ExportPackages) opts.AddCommonXX() opts.AddCommonOpens() opts.AddCommonJdkProperties() + a.configureCPU(qrStartCfg) + a.configureMemory(qrStartCfg) + a.configureGC(qrStartCfg) + a.configureClasspath(qrStartCfg) a.addJdiscProperties() svcName := a.ServiceName() if svcName == "container" || svcName == "container-clustercontroller" { diff --git a/config-provisioning/src/main/resources/configdefinitions/config.provisioning.node-repository.def b/config-provisioning/src/main/resources/configdefinitions/config.provisioning.node-repository.def index b054f434322..6a9c388f8b0 100644 --- a/config-provisioning/src/main/resources/configdefinitions/config.provisioning.node-repository.def +++ b/config-provisioning/src/main/resources/configdefinitions/config.provisioning.node-repository.def @@ -7,6 +7,9 @@ containerImage string default="registry.example.com:9999/myorg/vespa" # Default container image to use for tenant nodes. If this is unset (empty), it defaults to containerImage. tenantContainerImage string default="" +# Default container image to use for tenant nodes with GPU resources. If this is unset (empty), starting nodes with GPUs will fail +tenantGpuContainerImage string default="" + # Whether to cache data read from ZooKeeper in-memory. useCuratorClientCache bool default=false diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java index 7530d651f19..28ad003a2d6 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java @@ -127,13 +127,13 @@ public class FileDirectory extends AbstractComponent { return addFile(source, fileReference, hash); } - public void delete(FileReference fileReference, Function<FileReference, Boolean> canBeDeleted) { + public void delete(FileReference fileReference, Function<FileReference, Boolean> isInUse) { if (useLock.value()) try (Lock lock = locks.lock(fileReference)) { - if (canBeDeleted.apply(fileReference)) - deleteDirRecursively(destinationDir(fileReference)); - else + if (isInUse.apply(fileReference)) log.log(Level.FINE, "Unable to delete file reference '" + fileReference.value() + "' since it is still in use"); + else + deleteDirRecursively(destinationDir(fileReference)); } else deleteDirRecursively(destinationDir(fileReference)); 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 727f2f58c90..1bcef125e8c 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 @@ -428,14 +428,6 @@ public class ApplicationController { (wantedMajor.isPresent() ? " for specified major: " + wantedMajor.getAsInt() : "")); } - private OptionalInt firstNonEmpty(OptionalInt... choices) { - for (OptionalInt choice : choices) - if (choice.isPresent()) - return choice; - - return OptionalInt.empty(); - } - /** * Creates a new application for an existing tenant. * @@ -729,10 +721,12 @@ public class ApplicationController { // Remove the instance as well, if it is no longer referenced, and contains only production deployments that are removed now. boolean removeInstance = ! deploymentSpec.instanceNames().contains(instance) && application.get().require(instance).deployments().size() == deploymentsToRemove.size(); - for (ZoneId zone : deploymentsToRemove) - application = deactivate(application, instance, zone); - if (removeInstance) + for (ZoneId zone : deploymentsToRemove) { + application = deactivate(application.get().id().instance(instance), zone, Optional.of(application)).get(); + } + if (removeInstance) { application = application.without(instance); + } return application; } @@ -871,10 +865,13 @@ public class ApplicationController { configServer.setSuspension(deploymentId, suspend); } - /** Deactivate application in the given zone */ - public void deactivate(ApplicationId id, ZoneId zone) { - lockApplicationOrThrow(TenantAndApplicationId.from(id), - application -> store(deactivate(application, id.instance(), zone))); + /** Deactivate application in the given zone. Even if the application itself does not exist, deactivation of the deployment will still be attempted */ + public void deactivate(ApplicationId instanceId, ZoneId zone) { + TenantAndApplicationId applicationId = TenantAndApplicationId.from(instanceId); + try (Mutex lock = lock(applicationId)) { + Optional<LockedApplication> application = getApplication(applicationId).map(app -> new LockedApplication(app, lock)); + deactivate(instanceId, zone, application).ifPresent(this::store); + } } /** @@ -882,18 +879,18 @@ public class ApplicationController { * * @return the application with the deployment in the given zone removed */ - private LockedApplication deactivate(LockedApplication application, InstanceName instanceName, ZoneId zone) { - DeploymentId id = new DeploymentId(application.get().id().instance(instanceName), zone); + private Optional<LockedApplication> deactivate(ApplicationId instanceId, ZoneId zone, Optional<LockedApplication> application) { + DeploymentId id = new DeploymentId(instanceId, zone); try { configServer.deactivate(id); } finally { - controller.routing().of(id).configure(application.get().deploymentSpec()); - if (zone.environment().isManuallyDeployed()) + application.ifPresent(app -> controller.routing().of(id).configure(app.get().deploymentSpec())); + if (id.zoneId().environment().isManuallyDeployed()) applicationStore.putMetaTombstone(id, clock.instant()); - if (!zone.environment().isTest()) + if (!id.zoneId().environment().isTest()) controller.notificationsDb().removeNotifications(NotificationSource.from(id)); } - return application.with(instanceName, instance -> instance.withoutDeploymentIn(zone)); + return application.map(app -> app.with(instanceId.instance(), instance -> instance.withoutDeploymentIn(id.zoneId()))); } public DeploymentTrigger deploymentTrigger() { return deploymentTrigger; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java index 9a7fffd2a9e..c8ec38ec73b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java @@ -17,6 +17,7 @@ import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.hosted.controller.api.integration.ServiceRegistry; import com.yahoo.vespa.hosted.controller.api.integration.maven.MavenRepository; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry; +import com.yahoo.vespa.hosted.controller.application.MailVerifier; import com.yahoo.vespa.hosted.controller.archive.CuratorArchiveBucketDb; import com.yahoo.vespa.hosted.controller.auditlog.AuditLogger; import com.yahoo.vespa.hosted.controller.config.ControllerConfig; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java index f8135e27d74..bdf3d438bd7 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java @@ -51,12 +51,11 @@ public abstract class LockedTenant { } static LockedTenant of(Tenant tenant, Mutex lock) { - switch (tenant.type()) { - case athenz: return new Athenz((AthenzTenant) tenant); - case cloud: return new Cloud((CloudTenant) tenant); - case deleted: return new Deleted((DeletedTenant) tenant); - default: throw new IllegalArgumentException("Unexpected tenant type '" + tenant.getClass().getName() + "'."); - } + return switch (tenant.type()) { + case athenz -> new Athenz((AthenzTenant) tenant); + case cloud -> new Cloud((CloudTenant) tenant); + case deleted -> new Deleted((DeletedTenant) tenant); + }; } /** Returns a read-only copy of this */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java index 411c2c133f5..2d3c060c7b5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java @@ -2,17 +2,12 @@ package com.yahoo.vespa.hosted.controller.application; import com.yahoo.collections.AbstractFilteringList; -import com.yahoo.component.Version; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.ApplicationController; -import java.time.Instant; import java.util.Collection; -import java.util.Comparator; -import java.util.List; -import java.util.stream.Collectors; /** * A list of applications which can be filtered in various ways. @@ -36,7 +31,7 @@ public class ApplicationList extends AbstractFilteringList<Application, Applicat .map(TenantAndApplicationId::from) .distinct() .map(applications::requireApplication) - .collect(Collectors.toUnmodifiableList())); + .toList()); } // ----------------------------------- Filters diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/AssignedRotation.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/AssignedRotation.java index ab9304e75f3..eeeb822ecf5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/AssignedRotation.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/AssignedRotation.java @@ -5,10 +5,8 @@ import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.RegionName; import com.yahoo.vespa.hosted.controller.routing.rotation.RotationId; -import java.util.Collection; import java.util.Objects; import java.util.Set; -import java.util.stream.Collectors; /** * Contains the tuple of [clusterId, endpointId, rotationId, regions[]], to keep track @@ -16,11 +14,7 @@ import java.util.stream.Collectors; * * @author ogronnesby */ -public class AssignedRotation { - private final ClusterSpec.Id clusterId; - private final EndpointId endpointId; - private final RotationId rotationId; - private final Set<RegionName> regions; +public record AssignedRotation(ClusterSpec.Id clusterId, EndpointId endpointId, RotationId rotationId, Set<RegionName> regions) { public AssignedRotation(ClusterSpec.Id clusterId, EndpointId endpointId, RotationId rotationId, Set<RegionName> regions) { this.clusterId = requireNonEmpty(clusterId, clusterId.value(), "clusterId"); @@ -29,35 +23,14 @@ public class AssignedRotation { this.regions = Set.copyOf(Objects.requireNonNull(regions)); } - public ClusterSpec.Id clusterId() { return clusterId; } - public EndpointId endpointId() { return endpointId; } - public RotationId rotationId() { return rotationId; } - public Set<RegionName> regions() { return regions; } - @Override public String toString() { return "AssignedRotation{" + - "clusterId=" + clusterId + - ", endpointId='" + endpointId + '\'' + - ", rotationId=" + rotationId + - ", regions=" + regions + - '}'; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - AssignedRotation that = (AssignedRotation) o; - return clusterId.equals(that.clusterId) && - endpointId.equals(that.endpointId) && - rotationId.equals(that.rotationId) && - regions.equals(that.regions); - } - - @Override - public int hashCode() { - return Objects.hash(clusterId, endpointId, rotationId, regions); + "clusterId=" + clusterId + + ", endpointId='" + endpointId + '\'' + + ", rotationId=" + rotationId + + ", regions=" + regions + + '}'; } private static <T> T requireNonEmpty(T object, String value, String field) { @@ -69,13 +42,4 @@ public class AssignedRotation { return object; } - /** Convenience method intended for tests */ - public static AssignedRotation fromStrings(String clusterId, String endpointId, String rotationId, Collection<String> regions) { - return new AssignedRotation( - new ClusterSpec.Id(clusterId), - EndpointId.of(endpointId), - new RotationId(rotationId), - regions.stream().map(RegionName::from).collect(Collectors.toSet()) - ); - } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java index cbac700a9a0..1eb68c14353 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java @@ -31,7 +31,6 @@ import static java.util.Comparator.comparing; */ public class Endpoint { - private static final String YAHOO_DNS_SUFFIX = ".vespa.yahooapis.com"; private static final String OATH_DNS_SUFFIX = ".vespa.oath.cloud"; private static final String PUBLIC_DNS_SUFFIX = ".vespa-app.cloud"; private static final String PUBLIC_CD_DNS_SUFFIX = ".cd.vespa-app.cloud"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/MailVerifier.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/MailVerifier.java index 902343d5acf..afb0b61c23a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/MailVerifier.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/MailVerifier.java @@ -1,7 +1,9 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller; +package com.yahoo.vespa.hosted.controller.application; import com.yahoo.config.provision.TenantName; +import com.yahoo.vespa.hosted.controller.LockedTenant; +import com.yahoo.vespa.hosted.controller.TenantController; import com.yahoo.vespa.hosted.controller.api.integration.organization.Mail; import com.yahoo.vespa.hosted.controller.api.integration.organization.Mailer; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; @@ -26,13 +28,13 @@ import static com.yahoo.yolean.Exceptions.uncheck; */ public class MailVerifier { + private static final Duration VERIFICATION_DEADLINE = Duration.ofDays(7); + private final TenantController tenantController; private final Mailer mailer; private final CuratorDb curatorDb; private final Clock clock; - private final URI dashboardUri; - private static final Duration VERIFICATION_DEADLINE = Duration.ofDays(7); - + private final URI dashboardUri; public MailVerifier(URI dashboardUri, TenantController tenantController, Mailer mailer, CuratorDb curatorDb, Clock clock) { this.tenantController = tenantController; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializer.java index 6567b0076e7..9f648675cd0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializer.java @@ -18,7 +18,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; /** * Serialisation of {@link LogEntry} objects. Not all fields are stored! @@ -98,25 +97,24 @@ class LogSerializer { } static String valueOf(Type type) { - switch (type) { - case debug: return "debug"; - case info: return "info"; - case warning: return "warning"; - case error: return "error"; - case html: return "html"; - default: throw new AssertionError("Unexpected log entry type '" + type + "'!"); - } + return switch (type) { + case debug -> "debug"; + case info -> "info"; + case warning -> "warning"; + case error -> "error"; + case html -> "html"; + }; } static Type typeOf(String type) { - switch (type) { - case "debug": return Type.debug; - case "info": return Type.info; - case "warning": return Type.warning; - case "error": return Type.error; - case "html": return Type.html; - default: throw new IllegalArgumentException("Unknown log entry type '" + type + "'!"); - } + return switch (type) { + case "debug" -> Type.debug; + case "info" -> Type.info; + case "warning" -> Type.warning; + case "error" -> Type.error; + case "html" -> Type.html; + default -> throw new IllegalArgumentException("Unknown log entry type '" + type + "'!"); + }; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MailVerificationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MailVerificationSerializer.java index 6910d5c21c0..e5ee695e4e8 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MailVerificationSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MailVerificationSerializer.java @@ -15,12 +15,10 @@ import java.time.Instant; public class MailVerificationSerializer { private static final String tenantField = "tenant"; - private static final String audiencesField = "audiences"; private static final String emailField = "email"; private static final String emailTypeField = "emailType"; private static final String emailVerificationCodeField = "emailVerificationCode"; private static final String emailVerificationDeadlineField = "emailVerificationDeadline"; - private static final String rolesField = "roles"; public static Slime toSlime(PendingMailVerification pendingMailVerification) { var slime = new Slime(); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NotificationsSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NotificationsSerializer.java index beda8942fc2..3d28f35fc26 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NotificationsSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NotificationsSerializer.java @@ -4,7 +4,6 @@ package com.yahoo.vespa.hosted.controller.persistence; import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.InstanceName; -import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.slime.Cursor; @@ -16,7 +15,6 @@ import com.yahoo.vespa.hosted.controller.notification.Notification; import com.yahoo.vespa.hosted.controller.notification.NotificationSource; import java.util.List; -import java.util.stream.Collectors; /** * (de)serializes notifications for a tenant @@ -105,7 +103,6 @@ public class NotificationsSerializer { case deployment -> "deployment"; case feedBlock -> "feedBlock"; case reindex -> "reindex"; - default -> throw new IllegalArgumentException("No serialization defined for notification type " + type); }; } @@ -126,7 +123,6 @@ public class NotificationsSerializer { case info -> "info"; case warning -> "warning"; case error -> "error"; - default -> throw new IllegalArgumentException("No serialization defined for notification level " + level); }; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/certificate/EndpointCertificatesHandler.java index dc59f513509..a00e80b17d4 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/certificate/EndpointCertificatesHandler.java @@ -1,4 +1,4 @@ -package com.yahoo.vespa.hosted.controller.certificate; +package com.yahoo.vespa.hosted.controller.restapi.certificate; import com.yahoo.config.provision.ApplicationId; import com.yahoo.container.jdisc.HttpRequest; @@ -74,4 +74,4 @@ public class EndpointCertificatesHandler extends ThreadedHttpRequestHandler { return new StringResponse(EndpointCertificateMetadataSerializer.toSlime(reRequestedMetadata).toString()); } } -}
\ No newline at end of file +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index 978587d9c5c..8fdff787420 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -15,12 +15,15 @@ import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.Tags; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.zone.RoutingMethod; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.path.Path; import com.yahoo.vespa.flags.PermanentFlags; +import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeploymentData; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; +import com.yahoo.vespa.hosted.controller.api.integration.billing.Quota; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ContainerEndpoint; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; @@ -38,7 +41,6 @@ import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; -import com.yahoo.vespa.hosted.controller.deployment.RunStatus; import com.yahoo.vespa.hosted.controller.deployment.Submission; import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock; import com.yahoo.vespa.hosted.controller.notification.Notification; @@ -54,6 +56,7 @@ import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence; import com.yahoo.vespa.hosted.rotation.config.RotationsConfig; import org.junit.jupiter.api.Test; +import java.io.InputStream; import java.time.Duration; import java.time.Instant; import java.util.Collection; @@ -1399,4 +1402,17 @@ public class ControllerTest { } } + @Test + void testDeactivateDeploymentUnknownByController() { + DeploymentContext context = tester.newDeploymentContext(); + DeploymentId deployment = context.deploymentIdIn(ZoneId.from("prod", "us-west-1")); + DeploymentData deploymentData = new DeploymentData(deployment.applicationId(), Tags.empty(), deployment.zoneId(), InputStream::nullInputStream, Version.fromString("6.1"), + Set.of(), Optional::empty, Optional.empty(), Optional.empty(), + Quota::unlimited, List.of(), List.of(), Optional::empty, false); + tester.configServer().deploy(deploymentData); + assertTrue(tester.configServer().application(deployment.applicationId(), deployment.zoneId()).isPresent()); + tester.controller().applications().deactivate(deployment.applicationId(), deployment.zoneId()); + assertFalse(tester.configServer().application(deployment.applicationId(), deployment.zoneId()).isPresent()); + } + } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MailVerifierTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MailVerifierTest.java index edea07e205c..77145be4197 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MailVerifierTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MailVerifierTest.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMailer; +import com.yahoo.vespa.hosted.controller.application.MailVerifier; import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; import com.yahoo.vespa.hosted.controller.tenant.Email; import com.yahoo.vespa.hosted.controller.tenant.PendingMailVerification; @@ -99,4 +100,4 @@ class MailVerifierTest { assertTrue(tester.curator().getPendingMailVerification(resentVerification.get().getVerificationCode()).isPresent()); } -}
\ No newline at end of file +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java index 40217890351..04a623f819b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java @@ -5,7 +5,8 @@ import com.yahoo.component.Version; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Tags; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.security.KeyUtils; @@ -23,6 +24,7 @@ import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentActivity; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; +import com.yahoo.vespa.hosted.controller.application.EndpointId; import com.yahoo.vespa.hosted.controller.application.QuotaUsage; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext; @@ -40,6 +42,7 @@ import java.security.PublicKey; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Optional; @@ -47,6 +50,7 @@ import java.util.OptionalDouble; import java.util.OptionalInt; import java.util.OptionalLong; import java.util.Set; +import java.util.stream.Collectors; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -69,7 +73,6 @@ public class ApplicationSerializerTest { "pDhJeqczkyFcT2PysJ5sZwm7rKPEeXDOhzTPCyRvbUqc2SGdWbKUGGa/Yw==\n" + "-----END PUBLIC KEY-----\n"); - @Test void testSerialization() throws Exception { DeploymentSpec deploymentSpec = DeploymentSpec.fromXml("<deployment version='1.0'>\n" + @@ -133,7 +136,7 @@ public class ApplicationSerializerTest { Tags.fromString("tag1 tag2"), deployments, Map.of(DeploymentContext.systemTest, Instant.ofEpochMilli(333)), - List.of(AssignedRotation.fromStrings("foo", "default", "my-rotation", Set.of("us-west-1"))), + List.of(rotation("foo", "default", "my-rotation", Set.of("us-west-1"))), rotationStatus, Change.of(new Version("6.1"))), new Instance(id3, @@ -236,4 +239,13 @@ public class ApplicationSerializerTest { // ok if no error } + private static AssignedRotation rotation(String clusterId, String endpointId, String rotationId, Collection<String> regions) { + return new AssignedRotation( + new ClusterSpec.Id(clusterId), + EndpointId.of(endpointId), + new RotationId(rotationId), + regions.stream().map(RegionName::from).collect(Collectors.toSet()) + ); + } + } diff --git a/document/src/vespa/document/bucket/bucketid.h b/document/src/vespa/document/bucket/bucketid.h index 370948c1acc..e83418fb07e 100644 --- a/document/src/vespa/document/bucket/bucketid.h +++ b/document/src/vespa/document/bucket/bucketid.h @@ -49,7 +49,7 @@ public: /** Create an initially unset bucket id. */ constexpr BucketId() noexcept : _id(0) {} /** Create a bucket id with the given raw unchecked content. */ - explicit BucketId(Type id) noexcept : _id(id) {} + constexpr explicit BucketId(Type id) noexcept : _id(id) {} /** Create a bucket id using a set of bits from a raw unchecked value. */ BucketId(uint32_t useBits, Type id) noexcept : _id(createUsedBits(useBits, id)) { } diff --git a/fsa/src/vespa/fsa/automaton.cpp b/fsa/src/vespa/fsa/automaton.cpp index 88777f4e198..ee80fee0db1 100644 --- a/fsa/src/vespa/fsa/automaton.cpp +++ b/fsa/src/vespa/fsa/automaton.cpp @@ -341,6 +341,10 @@ void Automaton::PackedAutomaton::finalize() _packed_idx[i] = _pack_map[_packed_ptr[i]]; } } + if (_blob_used == 0) { + _packable = false; + return; + } // compact blobs if the size is constant std::map<uint32_t,uint32_t> bcomp; diff --git a/model-evaluation/src/main/java/ai/vespa/models/handler/ModelsEvaluationHandler.java b/model-evaluation/src/main/java/ai/vespa/models/handler/ModelsEvaluationHandler.java index 78addf0328a..ef04b6641e5 100644 --- a/model-evaluation/src/main/java/ai/vespa/models/handler/ModelsEvaluationHandler.java +++ b/model-evaluation/src/main/java/ai/vespa/models/handler/ModelsEvaluationHandler.java @@ -20,7 +20,10 @@ import java.io.OutputStream; import java.net.URI; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.concurrent.Executor; @@ -134,10 +137,13 @@ public class ModelsEvaluationHandler extends ThreadedHttpRequestHandler { cursor.setString("info", baseUrl(request) + model.name() + "/" + compactedFunction); cursor.setString("eval", baseUrl(request) + model.name() + "/" + compactedFunction + "/" + EVALUATE); Cursor bindings = cursor.setArray("arguments"); - for (Map.Entry<String, TensorType> argument : evaluator.function().argumentTypes().entrySet()) { + var argTypes = evaluator.function().argumentTypes(); + List<String> argNames = new ArrayList<>(argTypes.keySet()); + Collections.sort(argNames); + for (String name : argNames) { Cursor binding = bindings.addObject(); - binding.setString("name", argument.getKey()); - binding.setString("type", argument.getValue().toString()); + binding.setString("name", name); + binding.setString("type", argTypes.get(name).toString()); } } diff --git a/model-evaluation/src/test/java/ai/vespa/models/handler/HandlerTester.java b/model-evaluation/src/test/java/ai/vespa/models/handler/HandlerTester.java index 3b16be311a0..00531e373ee 100644 --- a/model-evaluation/src/test/java/ai/vespa/models/handler/HandlerTester.java +++ b/model-evaluation/src/test/java/ai/vespa/models/handler/HandlerTester.java @@ -12,25 +12,53 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.Map; import java.util.concurrent.Executors; +import java.util.function.Predicate; import static org.junit.Assert.assertEquals; +import static com.yahoo.slime.SlimeUtils.jsonToSlime; class HandlerTester { private final ModelsEvaluationHandler handler; + private static Predicate<String> nop() { + return s -> true; + } + private static Predicate<String> matchString(String expected) { + return s -> expected.equals(s); + } + public static Predicate<String> matchJson(String... expectedJson) { + var jExp = String.join("\n", expectedJson).replaceAll("'", "\""); + var expected = jsonToSlime(jExp); + return s -> { + var got = jsonToSlime(s); + boolean result = got.equalTo(expected); + if (!result) { + System.err.println("got:"); + System.err.println(got); + System.err.println("expected:"); + System.err.println(expected); + } + return result; + }; + } + HandlerTester(ModelsEvaluator models) { this.handler = new ModelsEvaluationHandler(models, Executors.newSingleThreadExecutor()); } void assertResponse(String url, int expectedCode) { - assertResponse(url, Map.of(), expectedCode, (String)null); + checkResponse(url, expectedCode, nop()); } void assertResponse(String url, int expectedCode, String expectedResult) { assertResponse(url, Map.of(), expectedCode, expectedResult); } + void checkResponse(String url, int expectedCode, Predicate<String> check) { + checkResponse(url, Map.of(), expectedCode, check, Map.of()); + } + void assertResponse(String url, int expectedCode, String expectedResult, Map<String, String> headers) { assertResponse(url, Map.of(), expectedCode, expectedResult, headers); } @@ -40,14 +68,18 @@ class HandlerTester { } void assertResponse(String url, Map<String, String> properties, int expectedCode, String expectedResult, Map<String, String> headers) { + checkResponse(url, properties, expectedCode, matchString(expectedResult), headers); + } + + void checkResponse(String url, Map<String, String> properties, int expectedCode, Predicate<String> check, Map<String, String> headers) { HttpRequest getRequest = HttpRequest.createTestRequest(url, com.yahoo.jdisc.http.HttpRequest.Method.GET, null, properties); HttpRequest postRequest = HttpRequest.createTestRequest(url, com.yahoo.jdisc.http.HttpRequest.Method.POST, null, properties); if (headers.size() > 0) { headers.forEach((k,v) -> getRequest.getJDiscRequest().headers().add(k, v)); headers.forEach((k,v) -> postRequest.getJDiscRequest().headers().add(k, v)); } - assertResponse(getRequest, expectedCode, expectedResult); - assertResponse(postRequest, expectedCode, expectedResult); + checkResponse(getRequest, expectedCode, check); + checkResponse(postRequest, expectedCode, check); } void assertResponse(String url, Map<String, String> properties, int expectedCode, Tensor expectedResult) { @@ -56,12 +88,14 @@ class HandlerTester { } void assertResponse(HttpRequest request, int expectedCode, String expectedResult) { + checkResponse(request, expectedCode, matchString(expectedResult)); + } + + void checkResponse(HttpRequest request, int expectedCode, Predicate<String> check) { HttpResponse response = handler.handle(request); assertEquals("application/json", response.getContentType()); assertEquals(expectedCode, response.getStatus()); - if (expectedResult != null) { - assertEquals(expectedResult, getContents(response)); - } + assertEquals(true, check.test(getContents(response))); } void assertResponse(HttpRequest request, int expectedCode, Tensor expectedResult) { diff --git a/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java b/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java index c52bf66626a..c0e5dd9ccda 100644 --- a/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java +++ b/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java @@ -14,7 +14,6 @@ import com.yahoo.vespa.config.search.core.OnnxModelsConfig; import com.yahoo.vespa.config.search.core.RankingConstantsConfig; import com.yahoo.vespa.config.search.core.RankingExpressionsConfig; import org.junit.BeforeClass; -import org.junit.Ignore; import org.junit.Test; import java.util.HashMap; @@ -262,7 +261,6 @@ public class ModelsEvaluationHandlerTest { "tensor(a[2],b[2],c{},d[2]):{a:[[[1.0, 2.0], [3.0, 4.0]], [[5.0, 6.0], [7.0, 8.0]]], b:[[[1.0, 2.0], [3.0, 4.0]], [[5.0, 6.0], [7.0, 8.0]]]}"); } - @Ignore @Test public void testMnistSavedEvaluateSpecificFunction() { assumeTrue(OnnxEvaluator.isRuntimeAvailable()); @@ -270,7 +268,17 @@ public class ModelsEvaluationHandlerTest { properties.put("input", inputTensor()); properties.put("format.tensors", "long"); String url = "http://localhost/model-evaluation/v1/mnist_saved/serving_default.y/eval"; - String expected = "{\"cells\":[{\"address\":{\"d0\":\"0\",\"d1\":\"0\"},\"value\":-0.6319251673007533},{\"address\":{\"d0\":\"0\",\"d1\":\"1\"},\"value\":-7.577770600619843E-4},{\"address\":{\"d0\":\"0\",\"d1\":\"2\"},\"value\":-0.010707969042025622},{\"address\":{\"d0\":\"0\",\"d1\":\"3\"},\"value\":-0.6344759233540788},{\"address\":{\"d0\":\"0\",\"d1\":\"4\"},\"value\":-0.17529455385847528},{\"address\":{\"d0\":\"0\",\"d1\":\"5\"},\"value\":0.7490809723192187},{\"address\":{\"d0\":\"0\",\"d1\":\"6\"},\"value\":-0.022790284182901716},{\"address\":{\"d0\":\"0\",\"d1\":\"7\"},\"value\":0.26799240657608936},{\"address\":{\"d0\":\"0\",\"d1\":\"8\"},\"value\":-0.3152438845465862},{\"address\":{\"d0\":\"0\",\"d1\":\"9\"},\"value\":0.05949304847735276}]}"; + Tensor expected = Tensor.from("tensor(d0[1],d1[10]):{"+ + "{d0:0,d1:0}:-0.6319251673007533,"+ + "{d0:0,d1:1}:-0.0007577770600619843,"+ + "{d0:0,d1:2}:-0.010707969042025622,"+ + "{d0:0,d1:3}:-0.6344759233540788,"+ + "{d0:0,d1:4}:-0.17529455385847528,"+ + "{d0:0,d1:5}:0.7490809723192187,"+ + "{d0:0,d1:6}:-0.022790284182901716,"+ + "{d0:0,d1:7}:0.26799240657608936,"+ + "{d0:0,d1:8}:-0.3152438845465862,"+ + "{d0:0,d1:9}:0.05949304847735276}"); handler.assertResponse(url, properties, 200, expected); } diff --git a/model-evaluation/src/test/java/ai/vespa/models/handler/OnnxEvaluationHandlerTest.java b/model-evaluation/src/test/java/ai/vespa/models/handler/OnnxEvaluationHandlerTest.java index cc3f2863068..29795fbcd95 100644 --- a/model-evaluation/src/test/java/ai/vespa/models/handler/OnnxEvaluationHandlerTest.java +++ b/model-evaluation/src/test/java/ai/vespa/models/handler/OnnxEvaluationHandlerTest.java @@ -12,7 +12,6 @@ import com.yahoo.vespa.config.search.core.OnnxModelsConfig; import com.yahoo.vespa.config.search.core.RankingConstantsConfig; import com.yahoo.vespa.config.search.core.RankingExpressionsConfig; import org.junit.BeforeClass; -import org.junit.Ignore; import org.junit.Test; import java.io.File; @@ -32,36 +31,35 @@ public class OnnxEvaluationHandlerTest { handler = new HandlerTester(createModels()); } - @Ignore @Test public void testListModels() { String url = "http://localhost/model-evaluation/v1"; String expected = "{\"one_layer\":\"http://localhost/model-evaluation/v1/one_layer\"," + "\"add_mul\":\"http://localhost/model-evaluation/v1/add_mul\"," + "\"no_model\":\"http://localhost/model-evaluation/v1/no_model\"}"; - handler.assertResponse(url, 200, expected); + handler.checkResponse(url, 200, HandlerTester.matchJson(expected)); } - @Ignore @Test public void testModelInfo() { String url = "http://localhost/model-evaluation/v1/add_mul"; - String expected = "{\"model\":\"add_mul\",\"functions\":[" + - "{\"function\":\"output1\"," + - "\"info\":\"http://localhost/model-evaluation/v1/add_mul/output1\"," + - "\"eval\":\"http://localhost/model-evaluation/v1/add_mul/output1/eval\"," + - "\"arguments\":[" + - "{\"name\":\"input1\",\"type\":\"tensor<float>(d0[1])\"}," + - "{\"name\":\"input2\",\"type\":\"tensor<float>(d0[1])\"}" + - "]}," + - "{\"function\":\"output2\"," + - "\"info\":\"http://localhost/model-evaluation/v1/add_mul/output2\"," + - "\"eval\":\"http://localhost/model-evaluation/v1/add_mul/output2/eval\"," + - "\"arguments\":[" + - "{\"name\":\"input1\",\"type\":\"tensor<float>(d0[1])\"}," + - "{\"name\":\"input2\",\"type\":\"tensor<float>(d0[1])\"}" + - "]}]}"; - handler.assertResponse(url, 200, expected); + var check = HandlerTester.matchJson( + "{'model':'add_mul','functions':[", + " {'function':'output1',", + " 'info':'http://localhost/model-evaluation/v1/add_mul/output1',", + " 'eval':'http://localhost/model-evaluation/v1/add_mul/output1/eval',", + " 'arguments':[", + " {'name':'input1','type':'tensor<float>(d0[1])'},", + " {'name':'input2','type':'tensor<float>(d0[1])'}", + " ]},", + " {'function':'output2',", + " 'info':'http://localhost/model-evaluation/v1/add_mul/output2',", + " 'eval':'http://localhost/model-evaluation/v1/add_mul/output2/eval',", + " 'arguments':[", + " {'name':'input1','type':'tensor<float>(d0[1])'},", + " {'name':'input2','type':'tensor<float>(d0[1])'}", + " ]}]}"); + handler.checkResponse(url, 200, check); } @Test diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java index fb21b009a30..c490c50c940 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java @@ -7,7 +7,6 @@ import com.yahoo.concurrent.maintenance.JobControl; import com.yahoo.config.provision.ApplicationTransaction; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.DockerImage; -import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.NodeFlavors; import com.yahoo.config.provision.Zone; import com.yahoo.config.provisioning.NodeRepositoryConfig; @@ -88,7 +87,8 @@ public class NodeRepository extends AbstractComponent { zone, new DnsNameResolver(), DockerImage.fromString(config.containerImage()), - Optional.of(config.tenantContainerImage()).filter(s -> !s.isEmpty()).map(DockerImage::fromString), + optionalImage(config.tenantContainerImage()), + optionalImage(config.tenantGpuContainerImage()), flagSource, metricsDb, orchestrator, @@ -109,6 +109,7 @@ public class NodeRepository extends AbstractComponent { NameResolver nameResolver, DockerImage containerImage, Optional<DockerImage> tenantContainerImage, + Optional<DockerImage> tenantGpuContainerImage, FlagSource flagSource, MetricsDb metricsDb, Orchestrator orchestrator, @@ -132,7 +133,7 @@ public class NodeRepository extends AbstractComponent { this.osVersions = new OsVersions(this); this.infrastructureVersions = new InfrastructureVersions(db); this.firmwareChecks = new FirmwareChecks(db, clock); - this.containerImages = new ContainerImages(containerImage, tenantContainerImage); + this.containerImages = new ContainerImages(containerImage, tenantContainerImage, tenantGpuContainerImage); this.archiveUris = new ArchiveUris(db); this.jobControl = new JobControl(new JobControlFlags(db, flagSource)); this.loadBalancers = new LoadBalancers(db); @@ -231,4 +232,8 @@ public class NodeRepository extends AbstractComponent { applications.remove(transaction); } + private static Optional<DockerImage> optionalImage(String image) { + return Optional.of(image).filter(s -> !s.isEmpty()).map(DockerImage::fromString); + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ContainerImages.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ContainerImages.java index f1358788c17..59dbb0b3241 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ContainerImages.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ContainerImages.java @@ -12,7 +12,7 @@ import java.util.Optional; * This class decides the container image to use for a given node. Two sources are considered, in the following order: * * 1. Requested image (from node allocation, this is set by either a feature flag or through services.xml) - * 2. Default image, specified in the node repository config file + * 2. Default image for the node type/configuration, specified in the node repository config file. * * Independent of source, the registry part of the image is rewritten to match the one set in the node repository config * file. @@ -24,10 +24,12 @@ public class ContainerImages { private final DockerImage defaultImage; private final Optional<DockerImage> tenantImage; + private final Optional<DockerImage> tenantGpuImage; - public ContainerImages(DockerImage defaultImage, Optional<DockerImage> tenantContainerImage) { + public ContainerImages(DockerImage defaultImage, Optional<DockerImage> tenantContainerImage, Optional<DockerImage> tenantGpuImage) { this.defaultImage = Objects.requireNonNull(defaultImage); this.tenantImage = Objects.requireNonNull(tenantContainerImage); + this.tenantGpuImage = Objects.requireNonNull(tenantGpuImage); } /** Returns the container image to use for given node */ @@ -39,7 +41,11 @@ public class ContainerImages { if (requestedImage.isPresent()) { image = requestedImage.get(); } else if (nodeType == NodeType.tenant) { - image = tenantImage.orElse(defaultImage); + if (!node.resources().gpuResources().isZero()) { + image = tenantGpuImage.orElseThrow(() -> new IllegalArgumentException(node + " has GPU resources, but there is no GPU container image available")); + } else { + image = tenantImage.orElse(defaultImage); + } } else { image = defaultImage; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java index 93e33051616..5bd53a2f8af 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java @@ -81,6 +81,7 @@ public class MockNodeRepository extends NodeRepository { new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), Optional.empty(), + Optional.empty(), new InMemoryFlagSource(), new MemoryMetricsDb(Clock.fixed(Instant.ofEpochMilli(123), ZoneId.of("Z"))), new OrchestratorMock(), diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java index cd73914850d..b964bf871c1 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java @@ -45,6 +45,7 @@ public class NodeRepositoryTester { new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), Optional.empty(), + Optional.empty(), new InMemoryFlagSource(), new MemoryMetricsDb(clock), new OrchestratorMock(), diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java index d9eef310c20..606bc55fdd2 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java @@ -72,6 +72,7 @@ public class CapacityCheckerTester { new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), Optional.empty(), + Optional.empty(), new InMemoryFlagSource(), new MemoryMetricsDb(clock), new OrchestratorMock(), diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java index 00fff017836..c9421f098e7 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java @@ -266,6 +266,7 @@ public class SpareCapacityMaintainerTest { new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), Optional.empty(), + Optional.empty(), new InMemoryFlagSource(), new MemoryMetricsDb(clock), new OrchestratorMock(), diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ContainerImagesTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ContainerImagesTest.java index 217ead40b81..bb7ea52ca0e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ContainerImagesTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ContainerImagesTest.java @@ -5,14 +5,11 @@ import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.DockerImage; -import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.node.Allocation; import com.yahoo.vespa.hosted.provision.node.Generation; -import com.yahoo.vespa.hosted.provision.node.IP; -import com.yahoo.vespa.hosted.provision.testutils.MockNodeFlavors; import org.junit.Test; import java.util.Optional; @@ -29,7 +26,8 @@ public class ContainerImagesTest { public void image_selection() { DockerImage defaultImage = DockerImage.fromString("registry.example.com/vespa/default"); DockerImage tenantImage = DockerImage.fromString("registry.example.com/vespa/tenant"); - ContainerImages images = new ContainerImages(defaultImage, Optional.of(tenantImage)); + DockerImage gpuImage = DockerImage.fromString("registry.example.com/vespa/tenant-gpu"); + ContainerImages images = new ContainerImages(defaultImage, Optional.of(tenantImage), Optional.of(gpuImage)); assertEquals(defaultImage, images.get(node(NodeType.confighost))); // For preload purposes assertEquals(defaultImage, images.get(node(NodeType.config))); @@ -40,32 +38,40 @@ public class ContainerImagesTest { assertEquals(defaultImage, images.get(node(NodeType.proxyhost))); // For preload purposes assertEquals(defaultImage, images.get(node(NodeType.proxy))); + // Choose GPU when node has GPU resources + assertEquals(gpuImage, images.get(node(NodeType.tenant, null, true))); + // Tenant node requesting a special image DockerImage requested = DockerImage.fromString("registry.example.com/vespa/special"); assertEquals(requested, images.get(node(NodeType.tenant, requested))); // When there is no custom tenant image, the default one is used - images = new ContainerImages(defaultImage, Optional.empty()); + images = new ContainerImages(defaultImage, Optional.empty(), Optional.of(gpuImage)); assertEquals(defaultImage, images.get(node(NodeType.host))); assertEquals(defaultImage, images.get(node(NodeType.tenant))); } private static Node node(NodeType type) { - return node(type, null); + return node(type, null, false); } private static Node node(NodeType type, DockerImage requested) { - Flavor flavor = new MockNodeFlavors().getFlavorOrThrow("default"); - Node.Builder b = Node.create(type + "1", new IP.Config(Set.of(), Set.of()), type + "1.example.com", flavor, type); - if (requested != null) { - b.allocation(new Allocation(ApplicationId.defaultId(), - ClusterMembership.from("container/id1/4/37", - Version.fromString("1.2.3"), - Optional.of(requested)), - NodeResources.unspecified(), - Generation.initial(), - false)); + return node(type, requested, false); + } + + private static Node node(NodeType type, DockerImage requested, boolean gpu) { + NodeResources resources = new NodeResources(4, 8, 100, 0.3); + if (gpu) { + resources = resources.with(new NodeResources.GpuResources(1, 16)); } + Node.Builder b = Node.reserve(Set.of("::1"), type + "1", "parent1", resources, type); + b.allocation(new Allocation(ApplicationId.defaultId(), + ClusterMembership.from("container/id1/4/37", + Version.fromString("1.2.3"), + Optional.ofNullable(requested)), + resources, + Generation.initial(), + false)); return b.build(); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java index 405d9578c95..110569a371a 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java @@ -113,6 +113,7 @@ public class ProvisioningTester { nameResolver, containerImage, Optional.empty(), + Optional.empty(), flagSource, new MemoryMetricsDb(clock), orchestrator, diff --git a/screwdriver.yaml b/screwdriver.yaml index f1a0a89263f..9f9a388477a 100644 --- a/screwdriver.yaml +++ b/screwdriver.yaml @@ -221,12 +221,12 @@ jobs: dnf config-manager --add-repo https://download.docker.com/linux/centos/docker-ce.repo dnf install -y docker-ce docker-ce-cli containerd.io docker system info + - release-java-artifacts: | + screwdriver/release-java-artifacts.sh $VESPA_VERSION $VESPA_REF - release-rpms: | screwdriver/release-rpms.sh $VESPA_VERSION $VESPA_REF - release-container-image: | screwdriver/release-container-image.sh $VESPA_VERSION - - release-java-artifacts: | - screwdriver/release-java-artifacts.sh $VESPA_VERSION $VESPA_REF - update-sample-apps: | screwdriver/update-vespa-version-in-sample-apps.sh $VESPA_VERSION diff --git a/searchcore/src/tests/proton/common/timer/timer_test.cpp b/searchcore/src/tests/proton/common/timer/timer_test.cpp index 23180fc1aba..18699554266 100644 --- a/searchcore/src/tests/proton/common/timer/timer_test.cpp +++ b/searchcore/src/tests/proton/common/timer/timer_test.cpp @@ -9,6 +9,7 @@ #include <vespa/vespalib/util/size_literals.h> #include <vespa/vespalib/util/threadstackexecutor.h> #include <vespa/vespalib/util/lambdatask.h> +#include <thread> using vespalib::Executor; using namespace proton; diff --git a/searchcore/src/tests/proton/server/disk_mem_usage_filter/disk_mem_usage_filter_test.cpp b/searchcore/src/tests/proton/server/disk_mem_usage_filter/disk_mem_usage_filter_test.cpp index dd38765c4f0..ccee7caa917 100644 --- a/searchcore/src/tests/proton/server/disk_mem_usage_filter/disk_mem_usage_filter_test.cpp +++ b/searchcore/src/tests/proton/server/disk_mem_usage_filter/disk_mem_usage_filter_test.cpp @@ -65,9 +65,17 @@ assertResourceUsage(double usage, double limit, double utilization, const Resour EXPECT_DOUBLE_EQ(utilization, state.utilization()); } +TEST_F(DiskMemUsageFilterTest, reconfig_with_identical_config_is_noop) +{ + EXPECT_TRUE(_filter.setConfig(Config(1.0, 0.8))); + assertResourceUsage(0.2, 0.8, 0.25, _filter.usageState().diskState()); + EXPECT_FALSE(_filter.setConfig(Config(1.0, 0.8))); + assertResourceUsage(0.2, 0.8, 0.25, _filter.usageState().diskState()); +} + TEST_F(DiskMemUsageFilterTest, disk_limit_can_be_reached) { - _filter.setConfig(Config(1.0, 0.8)); + EXPECT_TRUE(_filter.setConfig(Config(1.0, 0.8))); assertResourceUsage(0.2, 0.8, 0.25, _filter.usageState().diskState()); triggerDiskLimit(); testWrite("diskLimitReached: { " @@ -80,7 +88,7 @@ TEST_F(DiskMemUsageFilterTest, disk_limit_can_be_reached) TEST_F(DiskMemUsageFilterTest, memory_limit_can_be_reached) { - _filter.setConfig(Config(0.8, 1.0)); + EXPECT_TRUE(_filter.setConfig(Config(0.8, 1.0))); assertResourceUsage(0.3, 0.8, 0.375, _filter.usageState().memoryState()); triggerMemoryLimit(); testWrite("memoryLimitReached: { " @@ -95,7 +103,7 @@ TEST_F(DiskMemUsageFilterTest, memory_limit_can_be_reached) TEST_F(DiskMemUsageFilterTest, both_disk_limit_and_memory_limit_can_be_reached) { - _filter.setConfig(Config(0.8, 0.8)); + EXPECT_TRUE(_filter.setConfig(Config(0.8, 0.8))); triggerMemoryLimit(); triggerDiskLimit(); testWrite("memoryLimitReached: { " diff --git a/searchcore/src/vespa/searchcore/proton/common/scheduled_forward_executor.cpp b/searchcore/src/vespa/searchcore/proton/common/scheduled_forward_executor.cpp index 08712a1094c..b38de7a2456 100644 --- a/searchcore/src/vespa/searchcore/proton/common/scheduled_forward_executor.cpp +++ b/searchcore/src/vespa/searchcore/proton/common/scheduled_forward_executor.cpp @@ -102,7 +102,7 @@ ScheduledForwardExecutor::scheduleAtFixedRate(Executor::Task::UP task, auto handle = _scheduler.scheduleAtFixedRate(makeLambdaTask([&, my_task = std::move(my_task), my_state=state.get()]() { bool start_allowed = my_state->start(); if (start_allowed) { - _executor.execute(makeLambdaTask([&, my_task]() { + _executor.execute(makeLambdaTask([my_state, my_task]() { my_task->run(); my_state->complete(); })); diff --git a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.cpp b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.cpp index 54eea6565cb..fc1d23741c2 100644 --- a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.cpp @@ -188,12 +188,14 @@ DiskMemUsageFilter::set_resource_usage(const TransientResourceUsage& transient_u recalcState(guard); } -void +bool DiskMemUsageFilter::setConfig(Config config_in) { Guard guard(_lock); + if (_config == config_in) return false; _config = config_in; recalcState(guard); + return true; } vespalib::ProcessMemoryStats diff --git a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.h b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.h index 16e4d1d6869..cc901fa72cf 100644 --- a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.h +++ b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.h @@ -24,7 +24,6 @@ namespace proton { class DiskMemUsageFilter : public IResourceWriteFilter, public IDiskMemUsageNotifier { public: - using space_info = std::filesystem::space_info; using Mutex = std::mutex; using Guard = std::lock_guard<Mutex>; @@ -39,6 +38,12 @@ public: : _memoryLimit(memoryLimit_in), _diskLimit(diskLimit_in) { } + bool operator == (const Config & rhs) const noexcept { + return (_memoryLimit == rhs._memoryLimit) && (_diskLimit == rhs._diskLimit); + } + bool operator != (const Config & rhs) const noexcept { + return ! (*this == rhs); + } }; private: @@ -66,7 +71,7 @@ public: DiskMemUsageFilter(const HwInfo &hwInfo); ~DiskMemUsageFilter() override; void set_resource_usage(const TransientResourceUsage& transient_usage, vespalib::ProcessMemoryStats memoryStats, uint64_t diskUsedSizeBytes); - void setConfig(Config config); + [[nodiscard]] bool setConfig(Config config); vespalib::ProcessMemoryStats getMemoryStats() const; uint64_t getDiskUsedSize() const; TransientResourceUsage get_transient_resource_usage() const; diff --git a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.cpp b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.cpp index 102d947e812..9e09e968b49 100644 --- a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.cpp @@ -29,21 +29,26 @@ DiskMemUsageSampler::close() { _periodicHandle.reset(); } +bool +DiskMemUsageSampler::timeToSampleAgain() const noexcept { + return vespalib::steady_clock::now() >= (_lastSampleTime + _sampleInterval); +} + void DiskMemUsageSampler::setConfig(const Config &config, IScheduledExecutor & executor) { - _periodicHandle.reset(); - _filter.setConfig(config.filterConfig); + bool wasChanged = _filter.setConfig(config.filterConfig); + if (_periodicHandle && (_sampleInterval == config.sampleInterval) && !wasChanged) { + return; + } _sampleInterval = config.sampleInterval; + _periodicHandle.reset(); sampleAndReportUsage(); - _lastSampleTime = vespalib::steady_clock::now(); vespalib::duration maxInterval = std::min(vespalib::duration(1s), _sampleInterval); _periodicHandle = executor.scheduleAtFixedRate(makeLambdaTask([this]() { - if (_filter.acceptWriteOperation() && (vespalib::steady_clock::now() < (_lastSampleTime + _sampleInterval))) { - return; + if (!_filter.acceptWriteOperation() || timeToSampleAgain()) { + sampleAndReportUsage(); } - sampleAndReportUsage(); - _lastSampleTime = vespalib::steady_clock::now(); }), maxInterval, maxInterval); } @@ -60,6 +65,7 @@ DiskMemUsageSampler::sampleAndReportUsage() vespalib::ProcessMemoryStats memoryStats = sampleMemoryUsage(); uint64_t diskUsage = sampleDiskUsage(); _filter.set_resource_usage(transientUsage, memoryStats, diskUsage); + _lastSampleTime = vespalib::steady_clock::now(); } namespace { diff --git a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.h b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.h index 16c89a253fd..d434c529836 100644 --- a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.h +++ b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.h @@ -27,6 +27,7 @@ class DiskMemUsageSampler { uint64_t sampleDiskUsage(); vespalib::ProcessMemoryStats sampleMemoryUsage(); TransientResourceUsage sample_transient_resource_usage(); + [[nodiscard]] bool timeToSampleAgain() const noexcept; public: struct Config { DiskMemUsageFilter::Config filterConfig; diff --git a/searchlib/src/vespa/searchlib/fef/blueprintresolver.cpp b/searchlib/src/vespa/searchlib/fef/blueprintresolver.cpp index 0cc70d7fa6a..7492cd45857 100644 --- a/searchlib/src/vespa/searchlib/fef/blueprintresolver.cpp +++ b/searchlib/src/vespa/searchlib/fef/blueprintresolver.cpp @@ -297,21 +297,17 @@ BlueprintResolver::compile() { assert(_executorSpecs.empty()); // only one compilation allowed Compiler compiler(_factory, _indexEnv, _executorSpecs, _featureMap); - auto compile_task = makeLambdaTask([&]() { - compiler.probe_stack(); - for (const auto &seed: _seeds) { - auto ref = compiler.resolve_feature(seed, Blueprint::AcceptInput::ANY); - if (compiler.failed()) { - _warnings = std::move(compiler.errors); - return; - } - _seedMap.emplace(FeatureNameParser(seed).featureName(), ref); - } - }); - ThreadStackExecutor executor(1, 8_Mi); - executor.execute(std::move(compile_task)); - executor.sync(); - executor.shutdown(); + + compiler.probe_stack(); + for (const auto &seed: _seeds) { + auto ref = compiler.resolve_feature(seed, Blueprint::AcceptInput::ANY); + if (compiler.failed()) { + _warnings = std::move(compiler.errors); + break; + } + _seedMap.emplace(FeatureNameParser(seed).featureName(), ref); + } + size_t stack_usage = compiler.stack_usage(); if (stack_usage > (128_Ki)) { _warnings.emplace_back(fmt("high stack usage: %zu bytes", stack_usage)); diff --git a/storage/src/tests/distributor/bucketgctimecalculatortest.cpp b/storage/src/tests/distributor/bucketgctimecalculatortest.cpp index 33cd7bd637a..aaa228758b0 100644 --- a/storage/src/tests/distributor/bucketgctimecalculatortest.cpp +++ b/storage/src/tests/distributor/bucketgctimecalculatortest.cpp @@ -12,7 +12,7 @@ struct MockBucketIdHasher : public BucketGcTimeCalculator::BucketIdHasher { size_t nextGeneratedHash {0}; - size_t doHash(const document::BucketId&) const override { + size_t doHash(const document::BucketId&) const noexcept override { return nextGeneratedHash; } }; diff --git a/storage/src/tests/distributor/maintenancemocks.h b/storage/src/tests/distributor/maintenancemocks.h index 2809a81f79b..5bcd8f962a6 100644 --- a/storage/src/tests/distributor/maintenancemocks.h +++ b/storage/src/tests/distributor/maintenancemocks.h @@ -50,7 +50,7 @@ public: } void onClose(DistributorStripeMessageSender&) override {} - const char* getName() const override { return "MockOperation"; } + const char* getName() const noexcept override { return "MockOperation"; } const std::string& getDetailedReason() const override { return _reason; } diff --git a/storage/src/vespa/storage/bucketdb/bucketmanager.cpp b/storage/src/vespa/storage/bucketdb/bucketmanager.cpp index 9b3ad5c8e22..64c66401992 100644 --- a/storage/src/vespa/storage/bucketdb/bucketmanager.cpp +++ b/storage/src/vespa/storage/bucketdb/bucketmanager.cpp @@ -97,11 +97,12 @@ namespace { template<bool log> class DistributorInfoGatherer { - typedef api::RequestBucketInfoReply::EntryVector ResultArray; - DistributorStateCache _state; + using ResultArray = api::RequestBucketInfoReply::EntryVector; + + DistributorStateCache _state; std::unordered_map<uint16_t, ResultArray>& _result; - const document::BucketIdFactory& _factory; - std::shared_ptr<const lib::Distribution> _storageDistribution; + const document::BucketIdFactory& _factory; + std::shared_ptr<const lib::Distribution> _storageDistribution; public: DistributorInfoGatherer( @@ -123,9 +124,7 @@ namespace { try{ uint16_t i = _state.getOwner(b); auto it = _result.find(i); - // Template parameter. This block should not be included - // in version not logging. - if (log) { + if constexpr (log) { LOG(spam, "Bucket %s (reverse %" PRIu64 "), should be handled" " by distributor %u which we are %sgenerating " "state for.", @@ -164,13 +163,13 @@ namespace { uint64_t active; uint64_t ready; - Count() noexcept : docs(0), bytes(0), buckets(0), active(0), ready(0) {} + constexpr Count() noexcept : docs(0), bytes(0), buckets(0), active(0), ready(0) {} }; Count count; uint32_t lowestUsedBit; - MetricsUpdater() noexcept + constexpr MetricsUpdater() noexcept : count(), lowestUsedBit(58) {} void operator()(document::BucketId::Type bucketId, diff --git a/storage/src/vespa/storage/common/CMakeLists.txt b/storage/src/vespa/storage/common/CMakeLists.txt index 33efbb571fd..4a712719d53 100644 --- a/storage/src/vespa/storage/common/CMakeLists.txt +++ b/storage/src/vespa/storage/common/CMakeLists.txt @@ -8,6 +8,7 @@ vespa_add_library(storage_common OBJECT distributorcomponent.cpp global_bucket_space_distribution_converter.cpp messagebucket.cpp + message_guard.cpp messagesender.cpp node_identity.cpp reindexing_constants.cpp diff --git a/storage/src/vespa/storage/common/bucket_resolver.h b/storage/src/vespa/storage/common/bucket_resolver.h index 0b8e8b18430..ea261200b7b 100644 --- a/storage/src/vespa/storage/common/bucket_resolver.h +++ b/storage/src/vespa/storage/common/bucket_resolver.h @@ -12,7 +12,7 @@ namespace storage { * Interface for resolving which bucket a given a document id belongs to. */ struct BucketResolver { - virtual ~BucketResolver() {} + virtual ~BucketResolver() = default; virtual document::Bucket bucketFromId(const document::DocumentId &documentId) const = 0; virtual document::BucketSpace bucketSpaceFromName(const vespalib::string &bucketSpace) const = 0; virtual vespalib::string nameFromBucketSpace(const document::BucketSpace &bucketSpace) const = 0; diff --git a/storage/src/vespa/storage/common/bucketmessages.h b/storage/src/vespa/storage/common/bucketmessages.h index ca3b28188ec..ccee12938b2 100644 --- a/storage/src/vespa/storage/common/bucketmessages.h +++ b/storage/src/vespa/storage/common/bucketmessages.h @@ -17,8 +17,8 @@ class ReadBucketList : public api::InternalCommand { document::BucketSpace _bucketSpace; public: - typedef std::unique_ptr<ReadBucketList> UP; - static const uint32_t ID = 2003; + using UP = std::unique_ptr<ReadBucketList>; + static constexpr uint32_t ID = 2003; ReadBucketList(document::BucketSpace bucketSpace); ~ReadBucketList(); @@ -40,9 +40,9 @@ class ReadBucketListReply : public api::InternalReply { spi::BucketIdListResult::List _buckets; public: - typedef std::unique_ptr<ReadBucketListReply> UP; - typedef std::shared_ptr<ReadBucketListReply> SP; - static const uint32_t ID = 2004; + using UP = std::unique_ptr<ReadBucketListReply>; + using SP = std::shared_ptr<ReadBucketListReply>; + static constexpr uint32_t ID = 2004; ReadBucketListReply(const ReadBucketList& cmd); ~ReadBucketListReply(); @@ -72,7 +72,7 @@ class ReadBucketInfo : public api::InternalCommand { document::Bucket _bucket; public: - static const uint32_t ID = 2005; + static constexpr uint32_t ID = 2005; ReadBucketInfo(const document::Bucket &bucket); ~ReadBucketInfo(); @@ -95,7 +95,7 @@ class ReadBucketInfoReply : public api::InternalReply { document::Bucket _bucket; public: - static const uint32_t ID = 2006; + static constexpr uint32_t ID = 2006; ReadBucketInfoReply(const ReadBucketInfo& cmd); ~ReadBucketInfoReply(); diff --git a/storage/src/vespa/storage/common/doneinitializehandler.h b/storage/src/vespa/storage/common/doneinitializehandler.h index 41566f493b1..9aaa4e04dc7 100644 --- a/storage/src/vespa/storage/common/doneinitializehandler.h +++ b/storage/src/vespa/storage/common/doneinitializehandler.h @@ -13,7 +13,7 @@ namespace storage { struct DoneInitializeHandler { - virtual ~DoneInitializeHandler() {} + virtual ~DoneInitializeHandler() = default; virtual void notifyDoneInitializing() = 0; }; diff --git a/storage/src/vespa/storage/common/hostreporter/CMakeLists.txt b/storage/src/vespa/storage/common/hostreporter/CMakeLists.txt index 04006b3994b..60919a80bab 100644 --- a/storage/src/vespa/storage/common/hostreporter/CMakeLists.txt +++ b/storage/src/vespa/storage/common/hostreporter/CMakeLists.txt @@ -2,7 +2,6 @@ vespa_add_library(storage_hostreporter OBJECT SOURCES hostinfo.cpp - kernelmetrictool.cpp versionreporter.cpp DEPENDS ) diff --git a/storage/src/vespa/storage/common/hostreporter/kernelmetrictool.cpp b/storage/src/vespa/storage/common/hostreporter/kernelmetrictool.cpp deleted file mode 100644 index d831a404654..00000000000 --- a/storage/src/vespa/storage/common/hostreporter/kernelmetrictool.cpp +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "kernelmetrictool.h" -#include <vespa/vespalib/io/fileutil.h> -#include <vespa/vespalib/text/stringtokenizer.h> -#include <vespa/vespalib/stllike/asciistream.h> -#include <vespa/vespalib/util/exceptions.h> -#include <cctype> - -namespace storage { -namespace kernelmetrictool { - - -vespalib::string readFile(const char* fileName) { - return vespalib::asciistream::createFromDevice(fileName).str(); -} - -vespalib::string stripWhitespace(const vespalib::string& s) { - vespalib::string::size_type start(0); - vespalib::string::size_type stop(s.size() - 1); - while (true) { - if (start == s.size()) return vespalib::string(""); - if (!std::isspace(s[start])) break; - ++start; - } - while (true) { - if (!std::isspace(s[stop])) break; - --stop; - } - return s.substr(start, stop - start + 1); -} - -vespalib::string getLine(vespalib::stringref key, - vespalib::stringref content) -{ - vespalib::string::size_type start(0); - vespalib::string::size_type stop(content.find('\n')); - while (true) { - bool last = (stop == vespalib::string::npos); - vespalib::stringref line(content.substr(start, stop - start)); - for (uint32_t i=0, n=line.size(); i<n; ++i) { - if (std::isspace(line[i])) { - vespalib::stringref s(line.substr(0, i)); - if (s == key) return line; - } - } - if (last) break; - start = stop + 1; - stop = content.find('\n', start); - } - return ""; -} - -vespalib::string getToken(uint32_t index, const vespalib::string& line) { - vespalib::StringTokenizer st(line, " \t\n", ""); - st.removeEmptyTokens(); - return (index >= st.size() ? "" : st[index]); -} - -uint32_t getTokenCount(const vespalib::string& line) { - vespalib::StringTokenizer st(line, " \t\n", ""); - st.removeEmptyTokens(); - return st.size(); -} - -uint64_t toLong(vespalib::stringref s, int base) { - char* endptr; - // FIXME C++17 range-safe from_chars() instead of strtoull() - uint64_t result(strtoull(s.data(), &endptr, base)); - if ((s.data() + s.size()) != endptr) { - throw vespalib::IllegalArgumentException("Parsing '" + s + "' as a long."); - } - return result; -} -} -} /* namespace storage */ diff --git a/storage/src/vespa/storage/common/hostreporter/kernelmetrictool.h b/storage/src/vespa/storage/common/hostreporter/kernelmetrictool.h deleted file mode 100644 index df961c882e1..00000000000 --- a/storage/src/vespa/storage/common/hostreporter/kernelmetrictool.h +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -/* - * This file contains various tools for use by reporters when fetching os information. - */ - -#ifndef STORAGE_SRC_CPP_STORAGE_COMMON_HOSTREPORTER_KERNELMETRICTOOL_H_ -#define STORAGE_SRC_CPP_STORAGE_COMMON_HOSTREPORTER_KERNELMETRICTOOL_H_ - -#include <vespa/vespalib/stllike/string.h> - -namespace storage { -namespace kernelmetrictool { - -vespalib::string readFile(const char* fileName); - -vespalib::string stripWhitespace(const vespalib::string& s); - -vespalib::string getLine(vespalib::stringref key, - vespalib::stringref content); - -vespalib::string getToken(uint32_t index, const vespalib::string& line); - -uint32_t getTokenCount(const vespalib::string& line); - -uint64_t toLong(vespalib::stringref s, int base = 0) ; - -} /* namespace kernelmetrictool */ -} /* namespace storage */ - -#endif /* STORAGE_SRC_CPP_STORAGE_COMMON_HOSTREPORTER_KERNELMETRICTOOL_H_ */ diff --git a/storage/src/vespa/storage/common/message_guard.cpp b/storage/src/vespa/storage/common/message_guard.cpp new file mode 100644 index 00000000000..335b2c3d4d7 --- /dev/null +++ b/storage/src/vespa/storage/common/message_guard.cpp @@ -0,0 +1,16 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include "message_guard.h" + +namespace storage { + +MessageGuard::~MessageGuard() { + _lock.unlock(); + for (uint32_t i = 0; i < messagesUp.size(); i++) { + _messageSender.sendUp(messagesUp[i]); + } + for (uint32_t i = 0; i < messagesDown.size(); i++) { + _messageSender.sendDown(messagesDown[i]); + } +} + +} diff --git a/storage/src/vespa/storage/common/message_guard.h b/storage/src/vespa/storage/common/message_guard.h new file mode 100644 index 00000000000..682d7a3dc99 --- /dev/null +++ b/storage/src/vespa/storage/common/message_guard.h @@ -0,0 +1,39 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#pragma once + +#include "messagesender.h" +#include <memory> +#include <mutex> +#include <vector> + +namespace storage { + +class MessageGuard { + std::vector<std::shared_ptr<api::StorageMessage>> messagesUp; + std::vector<std::shared_ptr<api::StorageMessage>> messagesDown; + + std::unique_lock<std::mutex> _lock; + ChainedMessageSender& _messageSender; + +public: + MessageGuard(std::mutex& lock, ChainedMessageSender& messageSender) + : _lock(lock), + _messageSender(messageSender) + {} + ~MessageGuard(); + + void send(const std::shared_ptr<api::StorageMessage>& message) { + sendUp(message); + } + + void sendUp(const std::shared_ptr<api::StorageMessage>& message) { + messagesUp.push_back(message); + } + + void sendDown(const std::shared_ptr<api::StorageMessage>& message) { + messagesDown.push_back(message); + } +}; + +} + diff --git a/storage/src/vespa/storage/common/messagebucket.h b/storage/src/vespa/storage/common/messagebucket.h index b84c36bf08c..c8805cad1b3 100644 --- a/storage/src/vespa/storage/common/messagebucket.h +++ b/storage/src/vespa/storage/common/messagebucket.h @@ -14,8 +14,7 @@ class StorageMessage; * @throws vespalib::IllegalArgumentException if msg does not * have a bucket id. */ -document::Bucket getStorageMessageBucket( - const api::StorageMessage& msg); +document::Bucket getStorageMessageBucket(const api::StorageMessage& msg); } diff --git a/storage/src/vespa/storage/common/nodestateupdater.h b/storage/src/vespa/storage/common/nodestateupdater.h index 8d378e1ecd5..842828a1b89 100644 --- a/storage/src/vespa/storage/common/nodestateupdater.h +++ b/storage/src/vespa/storage/common/nodestateupdater.h @@ -36,7 +36,7 @@ struct StateListener { }; struct NodeStateUpdater { - typedef std::unique_ptr<NodeStateUpdater> UP; + using UP = std::unique_ptr<NodeStateUpdater>; virtual ~NodeStateUpdater() = default; @@ -53,8 +53,8 @@ struct NodeStateUpdater { * before altering the state. */ struct Lock { - typedef std::shared_ptr<Lock> SP; - virtual ~Lock() {} + using SP = std::shared_ptr<Lock>; + virtual ~Lock() = default; }; virtual Lock::SP grabStateChangeLock() = 0; diff --git a/storage/src/vespa/storage/common/servicelayercomponent.h b/storage/src/vespa/storage/common/servicelayercomponent.h index 3eaec863aa5..2ab8d6a8dfb 100644 --- a/storage/src/vespa/storage/common/servicelayercomponent.h +++ b/storage/src/vespa/storage/common/servicelayercomponent.h @@ -59,7 +59,7 @@ class ServiceLayerComponent : public StorageComponent, _minUsedBitsTracker = &tracker; } public: - typedef std::unique_ptr<ServiceLayerComponent> UP; + using UP = std::unique_ptr<ServiceLayerComponent>; ServiceLayerComponent(ServiceLayerComponentRegister& compReg, vespalib::stringref name) diff --git a/storage/src/vespa/storage/common/statusmessages.h b/storage/src/vespa/storage/common/statusmessages.h index 336df137e41..12432bfe095 100644 --- a/storage/src/vespa/storage/common/statusmessages.h +++ b/storage/src/vespa/storage/common/statusmessages.h @@ -23,7 +23,7 @@ class RequestStatusPage : public api::InternalCommand { // in which results should be sorted on status page. // (Used by filestor threads) public: - static const uint32_t ID = 2100; + static constexpr uint32_t ID = 2100; RequestStatusPage(const framework::HttpUrlPath& path); ~RequestStatusPage(); @@ -46,7 +46,7 @@ class RequestStatusPageReply : public api::InternalReply { std::string _status; std::string _sortToken; public: - static const uint32_t ID = 2101; + static constexpr uint32_t ID = 2101; RequestStatusPageReply(const RequestStatusPage& cmd, const std::string& status); ~RequestStatusPageReply(); diff --git a/storage/src/vespa/storage/common/storagelink.h b/storage/src/vespa/storage/common/storagelink.h index 2111dbadbc4..4961bd96741 100644 --- a/storage/src/vespa/storage/common/storagelink.h +++ b/storage/src/vespa/storage/common/storagelink.h @@ -37,7 +37,7 @@ class StorageLink : public document::Printable, protected api::MessageHandler { public: - typedef std::unique_ptr<StorageLink> UP; + using UP = std::unique_ptr<StorageLink>; enum State { CREATED, OPENED, CLOSING, FLUSHINGDOWN, FLUSHINGUP, CLOSED }; diff --git a/storage/src/vespa/storage/common/vectorprinter.h b/storage/src/vespa/storage/common/vectorprinter.h deleted file mode 100644 index 62daba9bd70..00000000000 --- a/storage/src/vespa/storage/common/vectorprinter.h +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#pragma once - -#include <vector> -#include <ostream> - -namespace storage { - -template <typename T> -class VectorPrinter -{ -public: - VectorPrinter(const std::vector<T>& vec, - const char* separator) - : _vec(&vec), - _separator(separator) - {} - - void print(std::ostream& os) const { - for (uint32_t i = 0; i < _vec->size(); ++i) { - if (i != 0) { - os << _separator; - } - os << (*_vec)[i]; - } - } -private: - const std::vector<T>* _vec; - const char* _separator; -}; - -template <typename T> -inline std::ostream& -operator<<(std::ostream& os, const VectorPrinter<T>& printer) -{ - printer.print(os); - return os; -} - -template <typename T> -inline VectorPrinter<T> -commaSeparated(const std::vector<T>& vec) -{ - return VectorPrinter<T>(vec, ","); -} - -} - diff --git a/storage/src/vespa/storage/common/visitorfactory.h b/storage/src/vespa/storage/common/visitorfactory.h index 48b9dd6d15f..a60ffadbdb0 100644 --- a/storage/src/vespa/storage/common/visitorfactory.h +++ b/storage/src/vespa/storage/common/visitorfactory.h @@ -16,17 +16,17 @@ class Visitor; class VisitorEnvironment { public: - typedef std::unique_ptr<VisitorEnvironment> UP; - VisitorEnvironment() {} - virtual ~VisitorEnvironment() {} + using UP = std::unique_ptr<VisitorEnvironment>; + VisitorEnvironment() = default; + virtual ~VisitorEnvironment() = default; }; class VisitorFactory { public: - typedef std::shared_ptr<VisitorFactory> SP; - typedef std::map<std::string, std::shared_ptr<VisitorFactory> > Map; + using SP = std::shared_ptr<VisitorFactory>; + using Map = std::map<std::string, std::shared_ptr<VisitorFactory>>; - virtual ~VisitorFactory() {}; + virtual ~VisitorFactory() = default; virtual VisitorEnvironment::UP makeVisitorEnvironment(StorageComponent&) = 0; diff --git a/storage/src/vespa/storage/distributor/activecopy.cpp b/storage/src/vespa/storage/distributor/activecopy.cpp index 71fcc77bd0b..a4ee4a51135 100644 --- a/storage/src/vespa/storage/distributor/activecopy.cpp +++ b/storage/src/vespa/storage/distributor/activecopy.cpp @@ -15,7 +15,9 @@ namespace std { for (uint32_t i=0; i<v.size(); ++i) { out << "\n " << v[i]; } - if (!v.empty()) out << "\n"; + if (!v.empty()) { + out << "\n"; + } return out << "]"; } } @@ -113,7 +115,9 @@ namespace { result.reserve(e->getNodeCount()); for (uint32_t i=0, n=e->getNodeCount(); i < n; ++i) { const BucketCopy& cp = e->getNodeRef(i); - if (!cp.valid()) continue; + if (!cp.valid()) { + continue; + } result.push_back(cp.getNode()); } return result; @@ -185,9 +189,13 @@ ActiveList::print(std::ostream& out, bool verbose, out << "\n" << indent << " " << _v[i]._nodeIndex << " " << _v[i].getReason(); } - if (!_v.empty()) out << "\n" << indent; + if (!_v.empty()) { + out << "\n" << indent; + } } else { - if (!_v.empty()) out << _v[0]._nodeIndex; + if (!_v.empty()) { + out << _v[0]._nodeIndex; + } for (size_t i=1; i<_v.size(); ++i) { out << " " << _v[i]._nodeIndex; } @@ -196,10 +204,12 @@ ActiveList::print(std::ostream& out, bool verbose, } bool -ActiveList::contains(uint16_t node) const +ActiveList::contains(uint16_t node) const noexcept { - for (const auto & candadate : _v) { - if (node == candadate._nodeIndex) return true; + for (const auto& candidate : _v) { + if (node == candidate._nodeIndex) { + return true; + } } return false; } diff --git a/storage/src/vespa/storage/distributor/activecopy.h b/storage/src/vespa/storage/distributor/activecopy.h index ee679519a19..258fe3cdf16 100644 --- a/storage/src/vespa/storage/distributor/activecopy.h +++ b/storage/src/vespa/storage/distributor/activecopy.h @@ -10,7 +10,7 @@ namespace storage::distributor { class ActiveList; struct ActiveCopy { - ActiveCopy() : _nodeIndex(-1), _ideal(-1), _doc_count(0), _ready(false), _active(false) { } + constexpr ActiveCopy() noexcept : _nodeIndex(-1), _ideal(-1), _doc_count(0), _ready(false), _active(false) { } ActiveCopy(uint16_t node, const BucketDatabase::Entry& e, const std::vector<uint16_t>& idealState); vespalib::string getReason() const; @@ -35,11 +35,11 @@ public: ActiveList() {} ActiveList(std::vector<ActiveCopy>&& v) : _v(std::move(v)) { } - ActiveCopy& operator[](size_t i) { return _v[i]; } - const ActiveCopy& operator[](size_t i) const { return _v[i]; } - bool contains(uint16_t) const; - bool empty() const { return _v.empty(); } - size_t size() const { return _v.size(); } + ActiveCopy& operator[](size_t i) noexcept { return _v[i]; } + const ActiveCopy& operator[](size_t i) const noexcept { return _v[i]; } + [[nodiscard]] bool contains(uint16_t) const noexcept; + [[nodiscard]] bool empty() const noexcept { return _v.empty(); } + size_t size() const noexcept { return _v.size(); } void print(std::ostream&, bool verbose, const std::string& indent) const override; }; diff --git a/storage/src/vespa/storage/distributor/bucket_ownership_flags.h b/storage/src/vespa/storage/distributor/bucket_ownership_flags.h index a72aa33265e..e40af18d9df 100644 --- a/storage/src/vespa/storage/distributor/bucket_ownership_flags.h +++ b/storage/src/vespa/storage/distributor/bucket_ownership_flags.h @@ -17,14 +17,14 @@ class BucketOwnershipFlags { static constexpr uint8_t owned_in_pending_state_flag = 0x2; public: - BucketOwnershipFlags() noexcept + constexpr BucketOwnershipFlags() noexcept : _flags(0) { } - bool owned_in_current_state() const noexcept { return ((_flags & owned_in_current_state_flag) != 0); } - bool owned_in_pending_state() const noexcept { return ((_flags & owned_in_pending_state_flag) != 0); } - void set_owned_in_current_state() noexcept { _flags |= owned_in_current_state_flag; } - void set_owned_in_pending_state() noexcept { _flags |= owned_in_pending_state_flag; } + constexpr bool owned_in_current_state() const noexcept { return ((_flags & owned_in_current_state_flag) != 0); } + constexpr bool owned_in_pending_state() const noexcept { return ((_flags & owned_in_pending_state_flag) != 0); } + constexpr void set_owned_in_current_state() noexcept { _flags |= owned_in_current_state_flag; } + constexpr void set_owned_in_pending_state() noexcept { _flags |= owned_in_pending_state_flag; } }; } diff --git a/storage/src/vespa/storage/distributor/bucket_spaces_stats_provider.h b/storage/src/vespa/storage/distributor/bucket_spaces_stats_provider.h index eee56d92576..18dc3c93cd8 100644 --- a/storage/src/vespa/storage/distributor/bucket_spaces_stats_provider.h +++ b/storage/src/vespa/storage/distributor/bucket_spaces_stats_provider.h @@ -17,30 +17,30 @@ private: size_t _bucketsTotal; size_t _bucketsPending; public: - BucketSpaceStats(size_t bucketsTotal_, size_t bucketsPending_) + constexpr BucketSpaceStats(size_t bucketsTotal_, size_t bucketsPending_) noexcept : _valid(true), _bucketsTotal(bucketsTotal_), _bucketsPending(bucketsPending_) {} - BucketSpaceStats() + constexpr BucketSpaceStats() noexcept : _valid(false), _bucketsTotal(0), _bucketsPending(0) {} - static BucketSpaceStats make_invalid() noexcept { return {}; } + static constexpr BucketSpaceStats make_invalid() noexcept { return {}; } - bool valid() const noexcept { return _valid; } + [[nodiscard]] bool valid() const noexcept { return _valid; } size_t bucketsTotal() const noexcept { return _bucketsTotal; } size_t bucketsPending() const noexcept { return _bucketsPending; } - bool operator==(const BucketSpaceStats& rhs) const { + bool operator==(const BucketSpaceStats& rhs) const noexcept { return (_valid == rhs._valid) && (_bucketsTotal == rhs._bucketsTotal) && (_bucketsPending == rhs._bucketsPending); } - void merge(const BucketSpaceStats& rhs) { + void merge(const BucketSpaceStats& rhs) noexcept { _valid = _valid && rhs._valid; _bucketsTotal += rhs._bucketsTotal; _bucketsPending += rhs._bucketsPending; diff --git a/storage/src/vespa/storage/distributor/bucketgctimecalculator.h b/storage/src/vespa/storage/distributor/bucketgctimecalculator.h index 201173c661a..f8d30b6e526 100644 --- a/storage/src/vespa/storage/distributor/bucketgctimecalculator.h +++ b/storage/src/vespa/storage/distributor/bucketgctimecalculator.h @@ -22,14 +22,14 @@ class BucketGcTimeCalculator { public: class BucketIdHasher { - virtual size_t doHash(const document::BucketId&) const = 0; + virtual size_t doHash(const document::BucketId&) const noexcept = 0; public: - virtual ~BucketIdHasher() {} - size_t hash(const document::BucketId& b) const { return doHash(b); } + virtual ~BucketIdHasher() = default; + size_t hash(const document::BucketId& b) const noexcept { return doHash(b); } }; class BucketIdIdentityHasher : public BucketIdHasher { - size_t doHash(const document::BucketId& b) const override { + size_t doHash(const document::BucketId& b) const noexcept override { return b.getId(); } }; diff --git a/storage/src/vespa/storage/distributor/bucketownership.h b/storage/src/vespa/storage/distributor/bucketownership.h index 46b38b46bf7..89678e42375 100644 --- a/storage/src/vespa/storage/distributor/bucketownership.h +++ b/storage/src/vespa/storage/distributor/bucketownership.h @@ -12,15 +12,15 @@ class BucketOwnership const lib::ClusterState* _checkedState; bool _owned; - BucketOwnership(const lib::ClusterState& checkedState) + BucketOwnership(const lib::ClusterState& checkedState) noexcept : _checkedState(&checkedState), _owned(false) { } public: - BucketOwnership() : _checkedState(nullptr), _owned(true) {} + constexpr BucketOwnership() noexcept : _checkedState(nullptr), _owned(true) {} - bool isOwned() const { return _owned; } + [[nodiscard]] bool isOwned() const noexcept { return _owned; } /** * Cluster state in which the ownership check failed. Lifetime of returned * reference depends on when the active or pending cluster state of the @@ -30,16 +30,16 @@ public: * * Precondition: isOwned() == false */ - const lib::ClusterState& getNonOwnedState() { + const lib::ClusterState& getNonOwnedState() noexcept { assert(!isOwned()); return *_checkedState; } - static BucketOwnership createOwned() { + static constexpr BucketOwnership createOwned() noexcept { return BucketOwnership(); } - static BucketOwnership createNotOwnedInState(const lib::ClusterState& s) { + static BucketOwnership createNotOwnedInState(const lib::ClusterState& s) noexcept { return BucketOwnership(s); } }; diff --git a/storage/src/vespa/storage/distributor/common/.gitignore b/storage/src/vespa/storage/distributor/common/.gitignore deleted file mode 100644 index e69de29bb2d..00000000000 --- a/storage/src/vespa/storage/distributor/common/.gitignore +++ /dev/null diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space.h b/storage/src/vespa/storage/distributor/distributor_bucket_space.h index 3dfd1e1ce30..f38556a664c 100644 --- a/storage/src/vespa/storage/distributor/distributor_bucket_space.h +++ b/storage/src/vespa/storage/distributor/distributor_bucket_space.h @@ -32,7 +32,7 @@ namespace storage::distributor { * bucket spaces. */ class DistributorBucketSpace { - std::unique_ptr<BucketDatabase> _bucketDatabase; + std::unique_ptr<BucketDatabase> _bucketDatabase; std::shared_ptr<const lib::ClusterState> _clusterState; std::shared_ptr<const lib::Distribution> _distribution; uint16_t _node_index; diff --git a/storage/src/vespa/storage/distributor/distributor_host_info_reporter.h b/storage/src/vespa/storage/distributor/distributor_host_info_reporter.h index 6680059b3d6..412867b537f 100644 --- a/storage/src/vespa/storage/distributor/distributor_host_info_reporter.h +++ b/storage/src/vespa/storage/distributor/distributor_host_info_reporter.h @@ -4,8 +4,7 @@ #include <vespa/storage/common/hostreporter/hostreporter.h> #include <atomic> -namespace storage { -namespace distributor { +namespace storage::distributor { class BucketSpacesStatsProvider; class MinReplicaProvider; @@ -46,6 +45,4 @@ private: std::atomic<bool> _enabled; }; -} // distributor -} // storage - +} // storage::distributor diff --git a/storage/src/vespa/storage/distributor/distributor_interface.h b/storage/src/vespa/storage/distributor/distributor_interface.h index 083c49f4ae3..b66d3ea198f 100644 --- a/storage/src/vespa/storage/distributor/distributor_interface.h +++ b/storage/src/vespa/storage/distributor/distributor_interface.h @@ -14,7 +14,7 @@ class DistributorMetricSet; */ class DistributorInterface : public DistributorMessageSender { public: - virtual ~DistributorInterface() {} + virtual ~DistributorInterface() = default; virtual DistributorMetricSet& metrics() = 0; virtual const DistributorConfiguration& config() const = 0; }; diff --git a/storage/src/vespa/storage/distributor/distributor_node_context.h b/storage/src/vespa/storage/distributor/distributor_node_context.h index ad082a5a9e5..4e25a479456 100644 --- a/storage/src/vespa/storage/distributor/distributor_node_context.h +++ b/storage/src/vespa/storage/distributor/distributor_node_context.h @@ -17,7 +17,7 @@ namespace storage::distributor { */ class DistributorNodeContext : public ClusterContext { public: - virtual ~DistributorNodeContext() {} + virtual ~DistributorNodeContext() = default; virtual const framework::Clock& clock() const noexcept = 0; virtual const document::BucketIdFactory& bucket_id_factory() const noexcept = 0; virtual uint16_t node_index() const noexcept = 0; diff --git a/storage/src/vespa/storage/distributor/document_selection_parser.h b/storage/src/vespa/storage/distributor/document_selection_parser.h index 3e7e0209429..45ca2bf1cde 100644 --- a/storage/src/vespa/storage/distributor/document_selection_parser.h +++ b/storage/src/vespa/storage/distributor/document_selection_parser.h @@ -14,7 +14,7 @@ namespace storage::distributor { */ class DocumentSelectionParser { public: - virtual ~DocumentSelectionParser() {} + virtual ~DocumentSelectionParser() = default; virtual std::unique_ptr<document::select::Node> parse_selection(const vespalib::string& str) const = 0; }; diff --git a/storage/src/vespa/storage/distributor/externaloperationhandler.h b/storage/src/vespa/storage/distributor/externaloperationhandler.h index 076b182abdd..50a2019a2ae 100644 --- a/storage/src/vespa/storage/distributor/externaloperationhandler.h +++ b/storage/src/vespa/storage/distributor/externaloperationhandler.h @@ -11,17 +11,17 @@ namespace documentapi { class TestAndSetCondition; } namespace storage::lib { class ClusterState; } -namespace storage { class PersistenceOperationMetricSet; } namespace storage::distributor { class DistributorMetricSet; -class TopLevelDistributor; -class MaintenanceOperationGenerator; class DirectDispatchSender; -class SequencingHandle; +class MaintenanceOperationGenerator; class OperationSequencer; class OperationOwner; +class PersistenceOperationMetricSet; +class SequencingHandle; +class TopLevelDistributor; class UuidGenerator; class ExternalOperationHandler : public api::MessageHandler diff --git a/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.h b/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.h index 2d77cf0c996..f98c4699eef 100644 --- a/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.h +++ b/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.h @@ -17,12 +17,20 @@ public: IdealServiceLayerNodesBundle() noexcept; ~IdealServiceLayerNodesBundle(); - void set_available_nodes(std::vector<uint16_t> available_nodes) { _available_nodes = std::move(available_nodes); } - void set_available_nonretired_nodes(std::vector<uint16_t> available_nonretired_nodes) { _available_nonretired_nodes = std::move(available_nonretired_nodes); } - void set_available_nonretired_or_maintenance_nodes(std::vector<uint16_t> available_nonretired_or_maintenance_nodes) { _available_nonretired_or_maintenance_nodes = std::move(available_nonretired_or_maintenance_nodes); } + void set_available_nodes(std::vector<uint16_t> available_nodes) { + _available_nodes = std::move(available_nodes); + } + void set_available_nonretired_nodes(std::vector<uint16_t> available_nonretired_nodes) { + _available_nonretired_nodes = std::move(available_nonretired_nodes); + } + void set_available_nonretired_or_maintenance_nodes(std::vector<uint16_t> available_nonretired_or_maintenance_nodes) { + _available_nonretired_or_maintenance_nodes = std::move(available_nonretired_or_maintenance_nodes); + } std::vector<uint16_t> get_available_nodes() const { return _available_nodes; } std::vector<uint16_t> get_available_nonretired_nodes() const { return _available_nonretired_nodes; } - std::vector<uint16_t> get_available_nonretired_or_maintenance_nodes() const { return _available_nonretired_or_maintenance_nodes; } + std::vector<uint16_t> get_available_nonretired_or_maintenance_nodes() const { + return _available_nonretired_or_maintenance_nodes; + } }; } diff --git a/storage/src/vespa/storage/distributor/idealstatemanager.cpp b/storage/src/vespa/storage/distributor/idealstatemanager.cpp index 728040da50e..cf255b5ec18 100644 --- a/storage/src/vespa/storage/distributor/idealstatemanager.cpp +++ b/storage/src/vespa/storage/distributor/idealstatemanager.cpp @@ -27,16 +27,15 @@ IdealStateManager::IdealStateManager( DistributorStripeOperationContext& op_ctx, IdealStateMetricSet& metrics) : _metrics(metrics), + _stateCheckers(), + _splitBucketStateChecker(nullptr), _node_ctx(node_ctx), _op_ctx(op_ctx), _has_logged_phantom_replica_warning(false) { - LOG(debug, "Adding BucketStateStateChecker to state checkers"); _stateCheckers.emplace_back(std::make_shared<BucketStateStateChecker>()); - _stateCheckers.emplace_back(std::make_shared<SplitBucketStateChecker>()); _splitBucketStateChecker = dynamic_cast<SplitBucketStateChecker *>(_stateCheckers.back().get()); - _stateCheckers.emplace_back(std::make_shared<SplitInconsistentStateChecker>()); _stateCheckers.emplace_back(std::make_shared<SynchronizeAndMoveStateChecker>()); _stateCheckers.emplace_back(std::make_shared<JoinBucketsStateChecker>()); @@ -141,7 +140,7 @@ void IdealStateManager::verify_only_live_nodes_in_context(const StateChecker::Co StateChecker::Result IdealStateManager::generateHighestPriority( - const document::Bucket &bucket, + const document::Bucket& bucket, NodeMaintenanceStatsTracker& statsTracker) const { auto& distributorBucketSpace = _op_ctx.bucket_space_repo().get(bucket.getBucketSpace()); @@ -162,7 +161,7 @@ IdealStateManager::generateHighestPriority( MaintenancePriorityAndType IdealStateManager::prioritize( - const document::Bucket &bucket, + const document::Bucket& bucket, NodeMaintenanceStatsTracker& statsTracker) const { StateChecker::Result generated(generateHighestPriority(bucket, statsTracker)); @@ -198,7 +197,7 @@ IdealStateManager::generateInterceptingSplit(BucketSpace bucketSpace, } MaintenanceOperation::SP -IdealStateManager::generate(const document::Bucket &bucket) const +IdealStateManager::generate(const document::Bucket& bucket) const { NodeMaintenanceStatsTracker statsTracker; IdealStateOperation::SP op( diff --git a/storage/src/vespa/storage/distributor/idealstatemanager.h b/storage/src/vespa/storage/distributor/idealstatemanager.h index 32349541de2..0c9e3ffa1c6 100644 --- a/storage/src/vespa/storage/distributor/idealstatemanager.h +++ b/storage/src/vespa/storage/distributor/idealstatemanager.h @@ -42,15 +42,15 @@ public: // MaintenancePriorityGenerator interface MaintenancePriorityAndType prioritize( - const document::Bucket &bucket, + const document::Bucket& bucket, NodeMaintenanceStatsTracker& statsTracker) const override; // MaintenanceOperationGenerator - MaintenanceOperation::SP generate(const document::Bucket &bucket) const override; + MaintenanceOperation::SP generate(const document::Bucket& bucket) const override; // MaintenanceOperationGenerator std::vector<MaintenanceOperation::SP> generateAll( - const document::Bucket &bucket, + const document::Bucket& bucket, NodeMaintenanceStatsTracker& statsTracker) const override; /** @@ -62,49 +62,43 @@ public: const BucketDatabase::Entry& e, api::StorageMessage::Priority pri); - IdealStateMetricSet& getMetrics() { return _metrics; } - + IdealStateMetricSet& getMetrics() noexcept { return _metrics; } void dump_bucket_space_db_status(document::BucketSpace bucket_space, std::ostream& out) const; void getBucketStatus(std::ostream& out) const; - const DistributorNodeContext& node_context() const { return _node_ctx; } - DistributorStripeOperationContext& operation_context() { return _op_ctx; } - const DistributorStripeOperationContext& operation_context() const { return _op_ctx; } - DistributorBucketSpaceRepo &getBucketSpaceRepo() { return _op_ctx.bucket_space_repo(); } - const DistributorBucketSpaceRepo &getBucketSpaceRepo() const { return _op_ctx.bucket_space_repo(); } + const DistributorNodeContext& node_context() const noexcept { return _node_ctx; } + DistributorStripeOperationContext& operation_context() noexcept { return _op_ctx; } + const DistributorStripeOperationContext& operation_context() const noexcept { return _op_ctx; } + DistributorBucketSpaceRepo &getBucketSpaceRepo() noexcept { return _op_ctx.bucket_space_repo(); } + const DistributorBucketSpaceRepo &getBucketSpaceRepo() const noexcept { return _op_ctx.bucket_space_repo(); } private: void verify_only_live_nodes_in_context(const StateChecker::Context& c) const; static void fillParentAndChildBuckets(StateChecker::Context& c); static void fillSiblingBucket(StateChecker::Context& c); StateChecker::Result generateHighestPriority( - const document::Bucket &bucket, + const document::Bucket& bucket, NodeMaintenanceStatsTracker& statsTracker) const; StateChecker::Result runStateCheckers(StateChecker::Context& c) const; static BucketDatabase::Entry* getEntryForPrimaryBucket(StateChecker::Context& c); - IdealStateMetricSet& _metrics; - document::BucketId _lastPrioritizedBucket; - - // Prioritized of state checkers that generate operations - // for idealstatemanager. - std::vector<StateChecker::SP> _stateCheckers; - SplitBucketStateChecker* _splitBucketStateChecker; - - const DistributorNodeContext& _node_ctx; + IdealStateMetricSet& _metrics; + std::vector<StateChecker::SP> _stateCheckers; + SplitBucketStateChecker* _splitBucketStateChecker; + const DistributorNodeContext& _node_ctx; DistributorStripeOperationContext& _op_ctx; - mutable bool _has_logged_phantom_replica_warning; + mutable bool _has_logged_phantom_replica_warning; class StatusBucketVisitor : public BucketDatabase::EntryProcessor { // Stats tracker to use for all generateAll() calls to avoid having // to create a new hash map for each single bucket processed. NodeMaintenanceStatsTracker _statsTracker; - const IdealStateManager & _ism; + const IdealStateManager& _ism; document::BucketSpace _bucketSpace; - std::ostream & _out; + std::ostream& _out; public: StatusBucketVisitor(const IdealStateManager& ism, document::BucketSpace bucketSpace, std::ostream& out) : _statsTracker(), _ism(ism), _bucketSpace(bucketSpace), _out(out) {} diff --git a/storage/src/vespa/storage/distributor/idealstatemetricsset.h b/storage/src/vespa/storage/distributor/idealstatemetricsset.h index 8062a65a01a..6528ad7dc72 100644 --- a/storage/src/vespa/storage/distributor/idealstatemetricsset.h +++ b/storage/src/vespa/storage/distributor/idealstatemetricsset.h @@ -6,9 +6,7 @@ #include <vespa/metrics/countmetric.h> #include <vespa/storage/distributor/operations/idealstate/idealstateoperation.h> -namespace storage { - -namespace distributor { +namespace storage::distributor { class OperationMetricSet : public metrics::MetricSet { @@ -50,7 +48,7 @@ public: metrics::LongValueMetric buckets_toomanycopies; metrics::LongValueMetric buckets; metrics::LongValueMetric buckets_notrusted; - metrics::LongValueMetric buckets_rechecking; // TODO remove, not used (but exposed by VespaMetricSet) + metrics::LongValueMetric buckets_rechecking; // TODO Vespa 9: remove, not used (but exposed by VespaMetricSet) metrics::LongValueMetric buckets_replicas_moving_out; metrics::LongValueMetric buckets_replicas_copying_in; metrics::LongValueMetric buckets_replicas_copying_out; @@ -66,7 +64,4 @@ public: void setPendingOperations(const std::vector<uint64_t>& newMetrics); }; -} - -} - +} // storage::distributor diff --git a/storage/src/vespa/storage/distributor/maintenance/bucketprioritydatabase.h b/storage/src/vespa/storage/distributor/maintenance/bucketprioritydatabase.h index 4c193492f43..f2f4b7cc275 100644 --- a/storage/src/vespa/storage/distributor/maintenance/bucketprioritydatabase.h +++ b/storage/src/vespa/storage/distributor/maintenance/bucketprioritydatabase.h @@ -14,9 +14,9 @@ protected: { public: virtual ~ConstIteratorImpl() = default; - virtual void increment() = 0; - virtual bool equal(const ConstIteratorImpl& other) const = 0; - virtual PrioritizedBucket dereference() const = 0; + virtual void increment() noexcept = 0; + virtual bool equal(const ConstIteratorImpl& other) const noexcept = 0; + virtual PrioritizedBucket dereference() const noexcept = 0; }; using ConstIteratorImplPtr = std::unique_ptr<ConstIteratorImpl>; @@ -31,25 +31,25 @@ public: { ConstIteratorImplPtr _impl; public: - explicit ConstIterator(ConstIteratorImplPtr impl) + explicit ConstIterator(ConstIteratorImplPtr impl) noexcept : _impl(std::move(impl)) {} ConstIterator(const ConstIterator &) = delete; - ConstIterator(ConstIterator &&) = default; + ConstIterator(ConstIterator &&) noexcept = default; virtual ~ConstIterator() = default; private: friend class boost::iterator_core_access; - void increment() { + void increment() noexcept { _impl->increment(); } - bool equal(const ConstIterator& other) const { + [[nodiscard]] bool equal(const ConstIterator& other) const noexcept { return _impl->equal(*other._impl); } - PrioritizedBucket dereference() const { + PrioritizedBucket dereference() const noexcept { return _impl->dereference(); } }; @@ -59,7 +59,7 @@ public: virtual ~BucketPriorityDatabase() = default; virtual const_iterator begin() const = 0; - virtual const_iterator end() const = 0; + virtual const_iterator end() const = 0; virtual void setPriority(const PrioritizedBucket&) = 0; }; diff --git a/storage/src/vespa/storage/distributor/maintenance/maintenancepriority.h b/storage/src/vespa/storage/distributor/maintenance/maintenancepriority.h index aa4f7b715f9..bb7ec904c16 100644 --- a/storage/src/vespa/storage/distributor/maintenance/maintenancepriority.h +++ b/storage/src/vespa/storage/distributor/maintenance/maintenancepriority.h @@ -20,33 +20,33 @@ public: static constexpr const char* toString(Priority pri) noexcept { switch (pri) { case NO_MAINTENANCE_NEEDED: return "NO_MAINTENANCE_NEEDED"; - case VERY_LOW: return "VERY_LOW"; - case LOW: return "LOW"; - case MEDIUM: return "MEDIUM"; - case HIGH: return "HIGH"; - case VERY_HIGH: return "VERY_HIGH"; - case HIGHEST: return "HIGHEST"; - default: return "INVALID"; + case VERY_LOW: return "VERY_LOW"; + case LOW: return "LOW"; + case MEDIUM: return "MEDIUM"; + case HIGH: return "HIGH"; + case VERY_HIGH: return "VERY_HIGH"; + case HIGHEST: return "HIGHEST"; + default: return "INVALID"; } } - MaintenancePriority() + constexpr MaintenancePriority() noexcept : _priority(NO_MAINTENANCE_NEEDED) {} - explicit MaintenancePriority(Priority priority) + constexpr explicit MaintenancePriority(Priority priority) noexcept : _priority(priority) {} - Priority getPriority() const { + constexpr Priority getPriority() const noexcept { return _priority; } - bool requiresMaintenance() const { + constexpr bool requiresMaintenance() const noexcept { return _priority != NO_MAINTENANCE_NEEDED; } - static MaintenancePriority noMaintenanceNeeded() { + static constexpr MaintenancePriority noMaintenanceNeeded() noexcept { return MaintenancePriority(NO_MAINTENANCE_NEEDED); } diff --git a/storage/src/vespa/storage/distributor/maintenance/maintenancepriorityandtype.h b/storage/src/vespa/storage/distributor/maintenance/maintenancepriorityandtype.h index 68602958815..3be7a4a18a7 100644 --- a/storage/src/vespa/storage/distributor/maintenance/maintenancepriorityandtype.h +++ b/storage/src/vespa/storage/distributor/maintenance/maintenancepriorityandtype.h @@ -11,21 +11,21 @@ class MaintenancePriorityAndType MaintenancePriority _priority; MaintenanceOperation::Type _type; public: - MaintenancePriorityAndType(MaintenancePriority pri, - MaintenanceOperation::Type type) + constexpr MaintenancePriorityAndType(MaintenancePriority pri, + MaintenanceOperation::Type type) noexcept : _priority(pri), _type(type) {} - const MaintenancePriority& getPriority() const { + constexpr MaintenancePriority getPriority() const noexcept { return _priority; } - MaintenanceOperation::Type getType() const { + constexpr MaintenanceOperation::Type getType() const noexcept { return _type; } - bool requiresMaintenance() const { + constexpr bool requiresMaintenance() const noexcept { return (_priority.getPriority() != MaintenancePriority::NO_MAINTENANCE_NEEDED); } diff --git a/storage/src/vespa/storage/distributor/maintenance/maintenancescanner.h b/storage/src/vespa/storage/distributor/maintenance/maintenancescanner.h index 3d5280e3b5a..4fec2e57cbc 100644 --- a/storage/src/vespa/storage/distributor/maintenance/maintenancescanner.h +++ b/storage/src/vespa/storage/distributor/maintenance/maintenancescanner.h @@ -17,9 +17,9 @@ public: BucketDatabase::Entry _entry; public: - bool isDone() const { return _done; } - document::BucketSpace getBucketSpace() const { return _bucketSpace; } - const BucketDatabase::Entry& getEntry() const { return _entry; } + [[nodiscard]] bool isDone() const noexcept { return _done; } + document::BucketSpace getBucketSpace() const noexcept { return _bucketSpace; } + const BucketDatabase::Entry& getEntry() const noexcept { return _entry; } static ScanResult createDone() { return ScanResult(true); } static ScanResult createNotDone(document::BucketSpace bucketSpace, BucketDatabase::Entry entry) { @@ -27,7 +27,7 @@ public: } private: - ScanResult(bool done) : _done(done), _bucketSpace(document::BucketSpace::invalid()), _entry() {} + explicit ScanResult(bool done) : _done(done), _bucketSpace(document::BucketSpace::invalid()), _entry() {} ScanResult(document::BucketSpace bucketSpace, const BucketDatabase::Entry& e) : _done(false), _bucketSpace(bucketSpace), _entry(e) {} }; diff --git a/storage/src/vespa/storage/distributor/maintenance/prioritizedbucket.h b/storage/src/vespa/storage/distributor/maintenance/prioritizedbucket.h index 77f117803ec..e1660df7cdb 100644 --- a/storage/src/vespa/storage/distributor/maintenance/prioritizedbucket.h +++ b/storage/src/vespa/storage/distributor/maintenance/prioritizedbucket.h @@ -14,24 +14,24 @@ public: static const PrioritizedBucket INVALID; - PrioritizedBucket() + PrioritizedBucket() noexcept : _bucket(), _priority(MaintenancePriority::NO_MAINTENANCE_NEEDED) {} - PrioritizedBucket(const document::Bucket &bucket, Priority pri) + PrioritizedBucket(const document::Bucket &bucket, Priority pri) noexcept : _bucket(bucket), _priority(pri) { } - document::Bucket getBucket() const { return _bucket; } + document::Bucket getBucket() const noexcept { return _bucket; } - Priority getPriority() const { + Priority getPriority() const noexcept { return _priority; } - bool valid() const { + [[nodiscard]] bool valid() const noexcept { return _bucket.getBucketId().getRawId() != 0; } @@ -41,19 +41,19 @@ public: MaintenancePriority::toString(_priority)); } - bool operator==(const PrioritizedBucket& other) const { + bool operator==(const PrioritizedBucket& other) const noexcept { return _bucket == other._bucket && _priority == other._priority; } - bool requiresMaintenance() const { + [[nodiscard]] bool requiresMaintenance() const noexcept { return _priority != MaintenancePriority::NO_MAINTENANCE_NEEDED; } - bool moreImportantThan(const PrioritizedBucket& other) const { + [[nodiscard]] bool moreImportantThan(const PrioritizedBucket& other) const noexcept { return _priority > other._priority; } - bool moreImportantThan(Priority otherPri) const { + [[nodiscard]] bool moreImportantThan(Priority otherPri) const noexcept { return _priority > otherPri; } diff --git a/storage/src/vespa/storage/distributor/maintenance/simplebucketprioritydatabase.cpp b/storage/src/vespa/storage/distributor/maintenance/simplebucketprioritydatabase.cpp index e267626fd7f..d5104aefea7 100644 --- a/storage/src/vespa/storage/distributor/maintenance/simplebucketprioritydatabase.cpp +++ b/storage/src/vespa/storage/distributor/maintenance/simplebucketprioritydatabase.cpp @@ -42,7 +42,7 @@ SimpleBucketPriorityDatabase::setPriority(const PrioritizedBucket& bucket) } void -SimpleBucketPriorityDatabase::PriFifoMappingConstIteratorImpl::increment() +SimpleBucketPriorityDatabase::PriFifoMappingConstIteratorImpl::increment() noexcept { if (_pri_fifo_iter != _pri_fifo_end) { ++_pri_fifo_iter; @@ -50,14 +50,14 @@ SimpleBucketPriorityDatabase::PriFifoMappingConstIteratorImpl::increment() } bool -SimpleBucketPriorityDatabase::PriFifoMappingConstIteratorImpl::equal(const ConstIteratorImpl& other) const +SimpleBucketPriorityDatabase::PriFifoMappingConstIteratorImpl::equal(const ConstIteratorImpl& other) const noexcept { auto& typed_other = dynamic_cast<const PriFifoMappingConstIteratorImpl&>(other); return (_pri_fifo_iter == typed_other._pri_fifo_iter); } PrioritizedBucket -SimpleBucketPriorityDatabase::PriFifoMappingConstIteratorImpl::dereference() const +SimpleBucketPriorityDatabase::PriFifoMappingConstIteratorImpl::dereference() const noexcept { assert(_pri_fifo_iter != _pri_fifo_end); return {_pri_fifo_iter->second, _pri_fifo_iter->first._pri}; diff --git a/storage/src/vespa/storage/distributor/maintenance/simplebucketprioritydatabase.h b/storage/src/vespa/storage/distributor/maintenance/simplebucketprioritydatabase.h index aa2605e94ca..50bcadf4e18 100644 --- a/storage/src/vespa/storage/distributor/maintenance/simplebucketprioritydatabase.h +++ b/storage/src/vespa/storage/distributor/maintenance/simplebucketprioritydatabase.h @@ -50,15 +50,15 @@ private: PriFifoBucketMap::const_iterator _pri_fifo_end; public: PriFifoMappingConstIteratorImpl(PriFifoBucketMap::const_iterator pri_fifo_iter, - PriFifoBucketMap::const_iterator pri_fifo_end) + PriFifoBucketMap::const_iterator pri_fifo_end) noexcept : _pri_fifo_iter(pri_fifo_iter), _pri_fifo_end(pri_fifo_end) {} ~PriFifoMappingConstIteratorImpl() override = default; - void increment() override; - bool equal(const ConstIteratorImpl& other) const override; - PrioritizedBucket dereference() const override; + void increment() noexcept override; + bool equal(const ConstIteratorImpl& other) const noexcept override; + PrioritizedBucket dereference() const noexcept override; }; void clearAllEntriesForBucket(const document::Bucket &bucket); diff --git a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h index a66cae11477..a867d4a5267 100644 --- a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h +++ b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h @@ -33,12 +33,12 @@ public: void merge(const PendingMaintenanceStats& rhs); }; private: - BucketPriorityDatabase& _bucketPriorityDb; - const MaintenancePriorityGenerator& _priorityGenerator; - const DistributorBucketSpaceRepo& _bucketSpaceRepo; + BucketPriorityDatabase& _bucketPriorityDb; + const MaintenancePriorityGenerator& _priorityGenerator; + const DistributorBucketSpaceRepo& _bucketSpaceRepo; DistributorBucketSpaceRepo::BucketSpaceMap::const_iterator _bucketSpaceItr; - document::BucketId _bucketCursor; - PendingMaintenanceStats _pendingMaintenance; + document::BucketId _bucketCursor; + PendingMaintenanceStats _pendingMaintenance; void countBucket(document::BucketSpace bucketSpace, const BucketInfo &info); public: @@ -55,7 +55,7 @@ public: // TODO: move out into own interface! void prioritizeBucket(const document::Bucket &id); - const PendingMaintenanceStats& getPendingMaintenanceStats() const { + const PendingMaintenanceStats& getPendingMaintenanceStats() const noexcept { return _pendingMaintenance; } }; diff --git a/storage/src/vespa/storage/distributor/messageguard.h b/storage/src/vespa/storage/distributor/messageguard.h deleted file mode 100644 index 2ed75d22577..00000000000 --- a/storage/src/vespa/storage/distributor/messageguard.h +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#pragma once - -#include "pendingclusterstate.h" -#include <vespa/storage/common/messagesender.h> - -namespace storage { - -class MessageGuard { - std::vector<std::shared_ptr<api::StorageMessage> > messagesUp; - std::vector<std::shared_ptr<api::StorageMessage> > messagesDown; - - std::unique_lock<std::mutex> _lock; - ChainedMessageSender& _messageSender; - -public: - MessageGuard(std::mutex & lock, ChainedMessageSender& messageSender) - : _lock(lock), - _messageSender(messageSender) {} - - void send(const std::shared_ptr<api::StorageMessage>& message) { - sendUp(message); - } - - void sendUp(const std::shared_ptr<api::StorageMessage>& message) { - messagesUp.push_back(message); - } - - void sendDown(const std::shared_ptr<api::StorageMessage>& message) { - messagesDown.push_back(message); - } - - ~MessageGuard() { - _lock.unlock(); - for (uint32_t i = 0; i < messagesUp.size(); i++) { - _messageSender.sendUp(messagesUp[i]); - } - for (uint32_t i = 0; i < messagesDown.size(); i++) { - _messageSender.sendDown(messagesDown[i]); - } - } -}; - -} - diff --git a/storage/src/vespa/storage/distributor/messagetracker.h b/storage/src/vespa/storage/distributor/messagetracker.h index d3d6899a854..73e2461eb7a 100644 --- a/storage/src/vespa/storage/distributor/messagetracker.h +++ b/storage/src/vespa/storage/distributor/messagetracker.h @@ -25,11 +25,11 @@ public: uint16_t _target; }; - MessageTracker(const ClusterContext &cluster_context); - MessageTracker(MessageTracker &&) = default; - MessageTracker & operator = (MessageTracker &&) = delete; + MessageTracker(const ClusterContext& cluster_context); + MessageTracker(MessageTracker&&) = default; + MessageTracker& operator=(MessageTracker&&) = delete; MessageTracker(const MessageTracker &) = delete; - MessageTracker & operator = (const MessageTracker &) = delete; + MessageTracker& operator=(const MessageTracker&) = delete; ~MessageTracker(); void queueCommand(std::shared_ptr<api::BucketCommand> msg, uint16_t target) { @@ -49,11 +49,10 @@ public: bool finished(); protected: - std::vector<ToSend> _commandQueue; - + std::vector<ToSend> _commandQueue; // Keeps track of which node a message was sent to. std::map<uint64_t, uint16_t> _sentMessages; - const ClusterContext &_cluster_ctx; + const ClusterContext& _cluster_ctx; }; } diff --git a/storage/src/vespa/storage/distributor/operationowner.cpp b/storage/src/vespa/storage/distributor/operationowner.cpp index c118209353b..81512393c5b 100644 --- a/storage/src/vespa/storage/distributor/operationowner.cpp +++ b/storage/src/vespa/storage/distributor/operationowner.cpp @@ -11,9 +11,7 @@ LOG_SETUP(".operationowner"); namespace storage::distributor { -OperationOwner::~OperationOwner() -{ -} +OperationOwner::~OperationOwner() = default; void OperationOwner::Sender::sendCommand(const std::shared_ptr<api::StorageCommand> & msg) diff --git a/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp index da53d48d461..41260115297 100644 --- a/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp @@ -18,7 +18,7 @@ using document::BucketSpace; namespace storage::distributor { -GetOperation::GroupId::GroupId(const document::BucketId& id, uint32_t checksum, int node) +GetOperation::GroupId::GroupId(const document::BucketId& id, uint32_t checksum, int node) noexcept : _id(id), _checksum(checksum), _node(node) @@ -26,7 +26,7 @@ GetOperation::GroupId::GroupId(const document::BucketId& id, uint32_t checksum, } bool -GetOperation::GroupId::operator<(const GroupId& other) const +GetOperation::GroupId::operator<(const GroupId& other) const noexcept { if (_id.getRawId() != other._id.getRawId()) { return (_id.getRawId() < other._id.getRawId()); @@ -41,7 +41,7 @@ GetOperation::GroupId::operator<(const GroupId& other) const } bool -GetOperation::GroupId::operator==(const GroupId& other) const +GetOperation::GroupId::operator==(const GroupId& other) const noexcept { return (_id == other._id && _checksum == other._checksum @@ -279,7 +279,7 @@ GetOperation::assignTargetNodeGroups(const BucketDatabase::ReadGuard& read_guard } bool -GetOperation::all_bucket_metadata_initially_consistent() const +GetOperation::all_bucket_metadata_initially_consistent() const noexcept { // TODO rename, calling this "responses" is confusing as it's populated before sending anything. return _responses.size() == 1; diff --git a/storage/src/vespa/storage/distributor/operations/external/getoperation.h b/storage/src/vespa/storage/distributor/operations/external/getoperation.h index 61443100695..96b5d970904 100644 --- a/storage/src/vespa/storage/distributor/operations/external/getoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/getoperation.h @@ -12,16 +12,13 @@ namespace document { class Document; } -namespace storage { +namespace storage::api { class GetCommand; } -namespace api { class GetCommand; } - -class PersistenceOperationMetricSet; - -namespace distributor { +namespace storage::distributor { class DistributorNodeContext; class DistributorBucketSpace; +class PersistenceOperationMetricSet; class GetOperation : public Operation { @@ -36,11 +33,11 @@ public: void onClose(DistributorStripeMessageSender& sender) override; void onStart(DistributorStripeMessageSender& sender) override; void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> & msg) override; - const char* getName() const override { return "get"; } + const char* getName() const noexcept override { return "get"; } std::string getStatus() const override { return ""; } - bool all_bucket_metadata_initially_consistent() const; - bool any_replicas_failed() const noexcept { + [[nodiscard]] bool all_bucket_metadata_initially_consistent() const noexcept; + [[nodiscard]] bool any_replicas_failed() const noexcept { return _any_replicas_failed; } @@ -66,12 +63,12 @@ private: class GroupId { public: // Node should be set only if bucket is incomplete - GroupId(const document::BucketId& id, uint32_t checksum, int node); + GroupId(const document::BucketId& id, uint32_t checksum, int node) noexcept; - bool operator<(const GroupId& other) const; - bool operator==(const GroupId& other) const; - const document::BucketId& getBucketId() const { return _id; } - int getNode() const { return _node; } + bool operator<(const GroupId& other) const noexcept; + bool operator==(const GroupId& other) const noexcept; + const document::BucketId& getBucketId() const noexcept { return _id; } + int getNode() const noexcept { return _node; } private: document::BucketId _id; uint32_t _checksum; @@ -129,4 +126,3 @@ private: }; } -} diff --git a/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp index 74ff8371fbe..dea215f47dc 100644 --- a/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp @@ -24,7 +24,7 @@ using document::BucketSpace; PutOperation::PutOperation(const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, - DistributorBucketSpace &bucketSpace, + DistributorBucketSpace& bucketSpace, std::shared_ptr<api::PutCommand> msg, PersistenceOperationMetricSet& metric, SequencingHandle sequencingHandle) : SequencedOperation(std::move(sequencingHandle)), @@ -222,7 +222,7 @@ PutOperation::shouldImplicitlyActivateReplica(const OperationTargetList& targets } void -PutOperation::onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> & msg) +PutOperation::onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply>& msg) { LOG(debug, "Received %s", msg->toString(true).c_str()); _tracker.receiveReply(sender, static_cast<api::BucketInfoReply&>(*msg)); diff --git a/storage/src/vespa/storage/distributor/operations/external/putoperation.h b/storage/src/vespa/storage/distributor/operations/external/putoperation.h index 7ec81d6570d..9801fed0c99 100644 --- a/storage/src/vespa/storage/distributor/operations/external/putoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/putoperation.h @@ -5,16 +5,15 @@ #include <vespa/storage/distributor/operations/sequenced_operation.h> #include <vespa/storage/distributor/persistencemessagetracker.h> -namespace document { - class Document; -} -namespace storage::lib { - class Distribution; -} +namespace document { class Document; } + +namespace storage::lib { class Distribution; } + namespace storage::api { - class CreateBucketReply; - class PutCommand; +class CreateBucketReply; +class PutCommand; } + namespace storage::distributor { class DistributorBucketSpace; @@ -25,16 +24,16 @@ class PutOperation : public SequencedOperation public: PutOperation(const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, - DistributorBucketSpace &bucketSpace, + DistributorBucketSpace& bucketSpace, std::shared_ptr<api::PutCommand> msg, PersistenceOperationMetricSet& metric, SequencingHandle sequencingHandle = SequencingHandle()); ~PutOperation() override; void onStart(DistributorStripeMessageSender& sender) override; - const char* getName() const override { return "put"; }; + const char* getName() const noexcept override { return "put"; }; std::string getStatus() const override { return ""; }; - void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> &) override; + void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply>&) override; void onClose(DistributorStripeMessageSender& sender) override; private: diff --git a/storage/src/vespa/storage/distributor/operations/external/read_for_write_visitor_operation.h b/storage/src/vespa/storage/distributor/operations/external/read_for_write_visitor_operation.h index dac9ed49bd3..f53342a9286 100644 --- a/storage/src/vespa/storage/distributor/operations/external/read_for_write_visitor_operation.h +++ b/storage/src/vespa/storage/distributor/operations/external/read_for_write_visitor_operation.h @@ -41,7 +41,7 @@ public: UuidGenerator& uuid_generator); ~ReadForWriteVisitorOperationStarter() override; - const char* getName() const override { return "ReadForWriteVisitorOperationStarter"; } + const char* getName() const noexcept override { return "ReadForWriteVisitorOperationStarter"; } void onClose(DistributorStripeMessageSender& sender) override; void onStart(DistributorStripeMessageSender& sender) override; void onReceive(DistributorStripeMessageSender& sender, diff --git a/storage/src/vespa/storage/distributor/operations/external/removelocationoperation.h b/storage/src/vespa/storage/distributor/operations/external/removelocationoperation.h index 1f951904ea1..d177676ff03 100644 --- a/storage/src/vespa/storage/distributor/operations/external/removelocationoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/removelocationoperation.h @@ -4,11 +4,9 @@ #include <vespa/storage/distributor/operations/operation.h> #include <vespa/storage/distributor/persistencemessagetracker.h> -namespace storage { +namespace storage::api { class RemoveLocationCommand; } -namespace api { class RemoveLocationCommand; } - -namespace distributor { +namespace storage::distributor { class DistributorBucketSpace; @@ -18,7 +16,7 @@ public: RemoveLocationOperation(const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, const DocumentSelectionParser& parser, - DistributorBucketSpace &bucketSpace, + DistributorBucketSpace& bucketSpace, std::shared_ptr<api::RemoveLocationCommand> msg, PersistenceOperationMetricSet& metric); ~RemoveLocationOperation() override; @@ -29,9 +27,9 @@ public: const api::RemoveLocationCommand& cmd, document::BucketId& id); void onStart(DistributorStripeMessageSender& sender) override; - const char* getName() const override { return "removelocation"; }; + const char* getName() const noexcept override { return "removelocation"; }; std::string getStatus() const override { return ""; }; - void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> &) override; + void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply>&) override; void onClose(DistributorStripeMessageSender& sender) override; private: PersistenceMessageTrackerImpl _trackerInstance; @@ -45,5 +43,3 @@ private: }; } - -} diff --git a/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp index 7beb95451e9..584ad9de2ce 100644 --- a/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp @@ -14,7 +14,7 @@ using document::BucketSpace; RemoveOperation::RemoveOperation(const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, - DistributorBucketSpace &bucketSpace, + DistributorBucketSpace& bucketSpace, std::shared_ptr<api::RemoveCommand> msg, PersistenceOperationMetricSet& metric, SequencingHandle sequencingHandle) @@ -81,7 +81,7 @@ RemoveOperation::onStart(DistributorStripeMessageSender& sender) void RemoveOperation::onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> & msg) { - api::RemoveReply& reply(static_cast<api::RemoveReply&>(*msg)); + auto& reply = static_cast<api::RemoveReply&>(*msg); if (_tracker.getReply().get()) { api::RemoveReply& replyToSend = diff --git a/storage/src/vespa/storage/distributor/operations/external/removeoperation.h b/storage/src/vespa/storage/distributor/operations/external/removeoperation.h index 53de79ac9b3..360ab332f96 100644 --- a/storage/src/vespa/storage/distributor/operations/external/removeoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/removeoperation.h @@ -4,42 +4,36 @@ #include <vespa/storage/distributor/operations/sequenced_operation.h> #include <vespa/storage/distributor/persistencemessagetracker.h> -namespace storage { +namespace storage::api { class RemoveCommand; } -namespace api { class RemoveCommand; } - -namespace distributor { +namespace storage::distributor { class DistributorBucketSpace; -class RemoveOperation : public SequencedOperation +class RemoveOperation : public SequencedOperation { public: RemoveOperation(const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, - DistributorBucketSpace &bucketSpace, + DistributorBucketSpace& bucketSpace, std::shared_ptr<api::RemoveCommand> msg, PersistenceOperationMetricSet& metric, SequencingHandle sequencingHandle = SequencingHandle()); ~RemoveOperation() override; void onStart(DistributorStripeMessageSender& sender) override; - const char* getName() const override { return "remove"; }; + const char* getName() const noexcept override { return "remove"; }; std::string getStatus() const override { return ""; }; void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> &) override; void onClose(DistributorStripeMessageSender& sender) override; private: - PersistenceMessageTrackerImpl _trackerInstance; - PersistenceMessageTracker& _tracker; - + PersistenceMessageTrackerImpl _trackerInstance; + PersistenceMessageTracker& _tracker; std::shared_ptr<api::RemoveCommand> _msg; - - const DistributorNodeContext& _node_ctx; - DistributorBucketSpace &_bucketSpace; + const DistributorNodeContext& _node_ctx; + DistributorBucketSpace& _bucketSpace; }; } - -} diff --git a/storage/src/vespa/storage/distributor/operations/external/statbucketlistoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/statbucketlistoperation.cpp index 8dd291c19bb..6b2550ab547 100644 --- a/storage/src/vespa/storage/distributor/operations/external/statbucketlistoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/statbucketlistoperation.cpp @@ -19,6 +19,8 @@ StatBucketListOperation::StatBucketListOperation( { } +StatBucketListOperation::~StatBucketListOperation() = default; + void StatBucketListOperation::getBucketStatus(const BucketDatabase::Entry& entry, std::ostream& ost) const @@ -42,20 +44,18 @@ StatBucketListOperation::getBucketStatus(const BucketDatabase::Entry& entry, void StatBucketListOperation::onStart(DistributorStripeMessageSender& sender) { - api::GetBucketListReply::SP reply(new api::GetBucketListReply(*_command)); + auto reply = std::make_shared<api::GetBucketListReply>(*_command); std::vector<BucketDatabase::Entry> entries; _bucketDb.getAll(_command->getBucketId(), entries); - for (uint32_t i = 0; i < entries.size(); i++) { + for (const auto& entry : entries) { std::ostringstream ost; ost << "[distributor:" << _distributorIndex << "] "; - getBucketStatus(entries[i], ost); + getBucketStatus(entry, ost); - reply->getBuckets().push_back(api::GetBucketListReply::BucketInfo( - entries[i].getBucketId(), - ost.str())); + reply->getBuckets().emplace_back(entry.getBucketId(), ost.str()); } sender.sendReply(reply); } diff --git a/storage/src/vespa/storage/distributor/operations/external/statbucketlistoperation.h b/storage/src/vespa/storage/distributor/operations/external/statbucketlistoperation.h index 9dcab1ee0f6..fe88ec50749 100644 --- a/storage/src/vespa/storage/distributor/operations/external/statbucketlistoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/statbucketlistoperation.h @@ -5,11 +5,9 @@ #include <vespa/storage/distributor/operations/operation.h> #include <vespa/storage/bucketdb/bucketdatabase.h> -namespace storage { +namespace storage::api { class GetBucketListCommand; } -namespace api { class GetBucketListCommand; } - -namespace distributor { +namespace storage::distributor { class MaintenanceOperationGenerator; @@ -21,9 +19,9 @@ public: const MaintenanceOperationGenerator& generator, uint16_t distributorIndex, const std::shared_ptr<api::GetBucketListCommand>& cmd); - ~StatBucketListOperation() {} + ~StatBucketListOperation() override; - const char* getName() const override { return "statBucketList"; } + const char* getName() const noexcept override { return "statBucketList"; } std::string getStatus() const override { return ""; } void onStart(DistributorStripeMessageSender& sender) override; @@ -43,5 +41,4 @@ private: std::shared_ptr<api::GetBucketListCommand> _command; }; -} // distributor -} // storage +} // storage::distributor diff --git a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp index 20cd8c67c29..85f36e7a963 100644 --- a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp @@ -42,27 +42,19 @@ StatBucketOperation::onStart(DistributorStripeMessageSender& sender) // If no entries exist, give empty reply if (nodes.size() == 0) { - api::StatBucketReply::SP reply(new api::StatBucketReply(*_command, "Bucket was not stored on any nodes.")); + auto reply = std::make_shared<api::StatBucketReply>(*_command, "Bucket was not stored on any nodes."); reply->setResult(api::ReturnCode(api::ReturnCode::OK)); sender.sendReply(reply); } else { - std::vector<std::shared_ptr<api::StorageCommand> > messages; - for (uint32_t i = 0; i < nodes.size(); i++) { - std::shared_ptr<api::StatBucketCommand> cmd( - new api::StatBucketCommand( - _command->getBucket(), - _command->getDocumentSelection())); - - messages.push_back(cmd); - _sent[cmd->getMsgId()] = nodes[i]; + std::vector<std::shared_ptr<api::StorageCommand>> messages; + for (uint16_t node : nodes) { + auto cmd = std::make_shared<api::StatBucketCommand>(_command->getBucket(), _command->getDocumentSelection()); + _sent[cmd->getMsgId()] = node; + messages.emplace_back(std::move(cmd)); } for (uint32_t i = 0; i < nodes.size(); i++) { - sender.sendToNode( - lib::NodeType::STORAGE, - nodes[i], - messages[i], - true); + sender.sendToNode(lib::NodeType::STORAGE, nodes[i], messages[i], true); } } }; @@ -71,9 +63,8 @@ void StatBucketOperation::onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> & msg) { assert(msg->getType() == api::MessageType::STATBUCKET_REPLY); - api::StatBucketReply& myreply(dynamic_cast<api::StatBucketReply&>(*msg)); - - std::map<uint64_t, uint16_t>::iterator found = _sent.find(msg->getMsgId()); + auto& myreply = dynamic_cast<api::StatBucketReply&>(*msg); + auto found = _sent.find(msg->getMsgId()); if (found != _sent.end()) { std::ostringstream ost; @@ -83,19 +74,15 @@ StatBucketOperation::onReceive(DistributorStripeMessageSender& sender, const std ost << "\tBucket information retrieval failed on node " << found->second << ": " << myreply.getResult() << "\n\n"; } _results[found->second] = ost.str(); - _sent.erase(found); } if (_sent.empty()) { std::ostringstream ost; - for (std::map<uint16_t, std::string>::iterator iter = _results.begin(); - iter != _results.end(); - iter++) { - ost << iter->second; + for (const auto& result : _results) { + ost << result.second; } - - api::StatBucketReply::SP reply(new api::StatBucketReply(*_command, ost.str())); + auto reply = std::make_shared<api::StatBucketReply>(*_command, ost.str()); sender.sendReply(reply); } } diff --git a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.h b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.h index 44215d2c68f..13850e642ad 100644 --- a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.h @@ -21,20 +21,18 @@ class StatBucketOperation : public Operation public: StatBucketOperation(DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::StatBucketCommand> & cmd); - ~StatBucketOperation(); + ~StatBucketOperation() override; - const char* getName() const override { return "statBucket"; } + const char* getName() const noexcept override { return "statBucket"; } std::string getStatus() const override { return ""; } void onClose(DistributorStripeMessageSender& sender) override; void onStart(DistributorStripeMessageSender& sender) override; void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> & msg) override; private: - DistributorBucketSpace &_bucketSpace; - + DistributorBucketSpace& _bucketSpace; std::shared_ptr<api::StatBucketCommand> _command; - - std::map<uint64_t, uint16_t> _sent; - std::map<uint16_t, std::string> _results; + std::map<uint64_t, uint16_t> _sent; + std::map<uint16_t, std::string> _results; }; } diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp index 59f52465b93..2acd6068e1a 100644 --- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp @@ -22,13 +22,11 @@ using document::BucketSpace; namespace storage::distributor { - - TwoPhaseUpdateOperation::TwoPhaseUpdateOperation( const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, const DocumentSelectionParser& parser, - DistributorBucketSpace &bucketSpace, + DistributorBucketSpace& bucketSpace, std::shared_ptr<api::UpdateCommand> msg, DistributorMetricSet& metrics, SequencingHandle sequencingHandle) @@ -110,12 +108,13 @@ IntermediateMessageSender::IntermediateMessageSender(SentMessageMap& mm, callback(std::move(cb)), forward(fwd) { } + IntermediateMessageSender::~IntermediateMessageSender() = default; } const char* -TwoPhaseUpdateOperation::stateToString(SendState state) +TwoPhaseUpdateOperation::stateToString(SendState state) noexcept { switch (state) { case SendState::NONE_SENT: return "NONE_SENT"; diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h index 45dec86268f..7c1b6b3eed6 100644 --- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h @@ -8,25 +8,20 @@ #include <set> #include <optional> -namespace document { -class Document; -} - -namespace storage { +namespace document { class Document; } -namespace api { +namespace storage::api { class UpdateCommand; class UpdateReply; class CreateBucketReply; class ReturnCode; } -class UpdateMetricSet; - -namespace distributor { +namespace storage::distributor { class DistributorBucketSpace; class GetOperation; +class UpdateMetricSet; /* * General functional outline: @@ -59,7 +54,7 @@ public: TwoPhaseUpdateOperation(const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, const DocumentSelectionParser& parser, - DistributorBucketSpace &bucketSpace, + DistributorBucketSpace& bucketSpace, std::shared_ptr<api::UpdateCommand> msg, DistributorMetricSet& metrics, SequencingHandle sequencingHandle = SequencingHandle()); @@ -67,7 +62,7 @@ public: void onStart(DistributorStripeMessageSender& sender) override; - const char* getName() const override { return "twophaseupdate"; } + const char* getName() const noexcept override { return "twophaseupdate"; } std::string getStatus() const override { return ""; } @@ -92,7 +87,7 @@ private: }; void transitionTo(SendState newState); - static const char* stateToString(SendState); + static const char* stateToString(SendState) noexcept; void sendReply(DistributorStripeMessageSender&, std::shared_ptr<api::UpdateReply>); @@ -163,7 +158,3 @@ private: }; } - -} - - diff --git a/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp index 5e3fe161f92..f3ce71b08d6 100644 --- a/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp @@ -41,6 +41,8 @@ UpdateOperation::UpdateOperation(const DistributorNodeContext& node_ctx, { } +UpdateOperation::~UpdateOperation() = default; + bool UpdateOperation::anyStorageNodesAvailable() const { diff --git a/storage/src/vespa/storage/distributor/operations/external/updateoperation.h b/storage/src/vespa/storage/distributor/operations/external/updateoperation.h index 7f3fef1260a..96fd878a324 100644 --- a/storage/src/vespa/storage/distributor/operations/external/updateoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/updateoperation.h @@ -3,22 +3,17 @@ #include <vespa/storage/distributor/persistencemessagetracker.h> -namespace document { -class Document; -} - -namespace storage { +namespace document { class Document; } -namespace api { +namespace storage::api { class UpdateCommand; class CreateBucketReply; } -class UpdateMetricSet; - -namespace distributor { +namespace storage::distributor { class DistributorBucketSpace; +class UpdateMetricSet; class UpdateOperation : public Operation { @@ -29,9 +24,10 @@ public: const std::shared_ptr<api::UpdateCommand>& msg, std::vector<BucketDatabase::Entry> entries, UpdateMetricSet& metric); + ~UpdateOperation() override; void onStart(DistributorStripeMessageSender& sender) override; - const char* getName() const override { return "update"; }; + const char* getName() const noexcept override { return "update"; }; std::string getStatus() const override { return ""; }; void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> & msg) override; void onClose(DistributorStripeMessageSender& sender) override; @@ -77,5 +73,3 @@ private: }; } - -} diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp index 5abaad6ef9f..4d40e93477f 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp @@ -35,6 +35,8 @@ VisitorOperation::BucketInfo::print(vespalib::asciistream & out) const out << ")"; } +VisitorOperation::BucketInfo::~BucketInfo() = default; + vespalib::string VisitorOperation::BucketInfo::toString() const { diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h index ce10ea87c11..7f278c0383f 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h @@ -13,7 +13,6 @@ namespace document { class Document; } -namespace storage { struct VisitorMetricSet; } namespace storage::lib { class ClusterState; } namespace storage::distributor { @@ -21,6 +20,7 @@ namespace storage::distributor { class DistributorBucketSpace; class DistributorNodeContext; class DistributorStripeOperationContext; +struct VisitorMetricSet; class VisitorOperation : public Operation { @@ -37,7 +37,7 @@ public: VisitorOperation(const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, - DistributorBucketSpace &bucketSpace, + DistributorBucketSpace& bucketSpace, const std::shared_ptr<api::CreateVisitorCommand>& msg, const Config& config, VisitorMetricSet& metrics); @@ -47,7 +47,7 @@ public: void onClose(DistributorStripeMessageSender& sender) override; void onStart(DistributorStripeMessageSender& sender) override; void onReceive(DistributorStripeMessageSender& sender, - const std::shared_ptr<api::StorageReply> & msg) override; + const std::shared_ptr<api::StorageReply>& msg) override; // Only valid to call if is_read_for_write() == true void fail_with_bucket_already_locked(DistributorStripeMessageSender& sender); @@ -55,7 +55,7 @@ public: [[nodiscard]] bool verify_command_and_expand_buckets(DistributorStripeMessageSender& sender); - const char* getName() const override { return "visit"; } + const char* getName() const noexcept override { return "visit"; } std::string getStatus() const override { return ""; } [[nodiscard]] bool has_sent_reply() const noexcept { return _sentReply; } @@ -72,6 +72,7 @@ private: std::vector<uint16_t> triedNodes; BucketInfo() : done(false), activeNode(-1), failedCount(0), triedNodes() { } + ~BucketInfo(); void print(vespalib::asciistream & out) const; vespalib::string toString() const; diff --git a/storage/src/vespa/storage/distributor/operations/external/visitororder.h b/storage/src/vespa/storage/distributor/operations/external/visitororder.h index 21dc8b1c6c6..90d6b2eaf07 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitororder.h +++ b/storage/src/vespa/storage/distributor/operations/external/visitororder.h @@ -7,9 +7,9 @@ namespace storage::distributor { struct VisitorOrder { - VisitorOrder() { } + VisitorOrder() = default; - bool operator()(const document::BucketId& a, const document::BucketId& b) { + bool operator()(const document::BucketId& a, const document::BucketId& b) noexcept { if (a == document::BucketId(INT_MAX) || b == document::BucketId(0, 0)) { return false; // All before max, non before null diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.h b/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.h index 25308b0fb4b..27dc519dcc2 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.h @@ -22,8 +22,8 @@ public: void onStart(DistributorStripeMessageSender& sender) override; void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> &) override; - const char* getName() const override { return "garbagecollection"; }; - Type getType() const override { return GARBAGE_COLLECTION; } + const char* getName() const noexcept override { return "garbagecollection"; }; + Type getType() const noexcept override { return GARBAGE_COLLECTION; } bool shouldBlockThisOperation(uint32_t, uint16_t, uint8_t) const override; bool is_two_phase() const noexcept { return ((_phase == Phase::ReadMetadataPhase) || (_phase == Phase::WriteRemovesPhase)); diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.cpp index 744b24b593e..a69b7739e07 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.cpp @@ -28,8 +28,8 @@ IdealStateOperation::IdealStateOperation(const BucketAndNodes& bucketAndNodes) : _manager(nullptr), _bucketSpace(nullptr), _bucketAndNodes(bucketAndNodes), - _ok(true), - _priority(255) + _priority(255), + _ok(true) { } @@ -111,7 +111,7 @@ IdealStateOperation::on_throttled() } uint32_t -IdealStateOperation::memorySize() const +IdealStateOperation::memorySize() const noexcept { return sizeof(*this) + _detailedReason.size(); } diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.h b/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.h index 7e62506b77f..6c52bdb738d 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.h @@ -55,23 +55,23 @@ public: @return Returns the target bucket. */ - document::BucketId getBucketId() const { return _bucket.getBucketId(); } + document::BucketId getBucketId() const noexcept { return _bucket.getBucketId(); } - document::Bucket getBucket() const { return _bucket; } + document::Bucket getBucket() const noexcept { return _bucket; } /** Returns the target nodes @return the target nodes */ - std::vector<uint16_t>& getNodes() { return _nodes; } + std::vector<uint16_t>& getNodes() noexcept { return _nodes; } /** Returns the target nodes @return the target nodes */ - const std::vector<uint16_t>& getNodes() const { return _nodes; } + const std::vector<uint16_t>& getNodes() const noexcept { return _nodes; } /** Returns a string representation of this object. @@ -105,14 +105,14 @@ class IdealStateOperation : public MaintenanceOperation public: static const uint32_t MAINTENANCE_MESSAGE_TYPES[]; - typedef std::shared_ptr<IdealStateOperation> SP; - typedef std::unique_ptr<IdealStateOperation> UP; - typedef std::vector<SP> Vector; - typedef std::map<document::BucketId, SP> Map; + using SP = std::shared_ptr<IdealStateOperation>; + using UP = std::unique_ptr<IdealStateOperation>; + using Vector = std::vector<SP>; + using Map = std::map<document::BucketId, SP>; IdealStateOperation(const BucketAndNodes& bucketAndNodes); - virtual ~IdealStateOperation(); + ~IdealStateOperation() override; void onClose(DistributorStripeMessageSender&) override {} @@ -121,37 +121,37 @@ public: @return Returns the status of the operation. */ - virtual bool ok() { return _ok; } + [[nodiscard]] bool ok() const noexcept { return _ok; } /** Returns the target nodes of the operation. @return The target nodes */ - std::vector<uint16_t>& getNodes() { return _bucketAndNodes.getNodes(); } + std::vector<uint16_t>& getNodes() noexcept { return _bucketAndNodes.getNodes(); } /** Returns the target nodes of the operation. @return The target nodes */ - const std::vector<uint16_t>& getNodes() const { return _bucketAndNodes.getNodes(); } + const std::vector<uint16_t>& getNodes() const noexcept { return _bucketAndNodes.getNodes(); } /** Returns the target bucket of the operation. @return The target bucket. */ - document::BucketId getBucketId() const { return _bucketAndNodes.getBucketId(); } + document::BucketId getBucketId() const noexcept { return _bucketAndNodes.getBucketId(); } - document::Bucket getBucket() const { return _bucketAndNodes.getBucket(); } + document::Bucket getBucket() const noexcept { return _bucketAndNodes.getBucket(); } /** Returns the target of the operation. @return The target bucket and nodes */ - const BucketAndNodes& getBucketAndNodes() const { return _bucketAndNodes; } + const BucketAndNodes& getBucketAndNodes() const noexcept { return _bucketAndNodes; } /** Called by the operation when it is finished. Must be called, otherwise the active @@ -174,7 +174,7 @@ public: /** Returns the type of operation this is. */ - virtual Type getType() const = 0; + virtual Type getType() const noexcept = 0; /** Set the priority we should send messages from this operation with. @@ -192,7 +192,7 @@ public: /** Returns the priority we should send messages with. */ - api::StorageMessage::Priority getPriority() { return _priority; } + api::StorageMessage::Priority getPriority() const noexcept { return _priority; } void setDetailedReason(const std::string& detailedReason) { _detailedReason = detailedReason; @@ -205,7 +205,7 @@ public: return _detailedReason; } - uint32_t memorySize() const; + uint32_t memorySize() const noexcept; /** * Sets the various metadata for the given command that @@ -224,13 +224,12 @@ protected: friend struct IdealStateManagerTest; friend class IdealStateManager; - IdealStateManager* _manager; - DistributorBucketSpace *_bucketSpace; - BucketAndNodes _bucketAndNodes; - std::string _detailedReason; - - bool _ok; + IdealStateManager* _manager; + DistributorBucketSpace* _bucketSpace; + BucketAndNodes _bucketAndNodes; + std::string _detailedReason; api::StorageMessage::Priority _priority; + bool _ok; /** * Checks if the given bucket is blocked by any pending messages to any diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/joinoperation.h b/storage/src/vespa/storage/distributor/operations/idealstate/joinoperation.h index d8f77d89b8a..88fb3010654 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/joinoperation.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/joinoperation.h @@ -27,11 +27,11 @@ public: void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply>&) override; - const char* getName() const override { + const char* getName() const noexcept override { return "join"; } - Type getType() const override { + Type getType() const noexcept override { return JOIN_BUCKET; } diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/mergelimiter.h b/storage/src/vespa/storage/distributor/operations/idealstate/mergelimiter.h index 6558aaf7f04..c139109ff0c 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/mergelimiter.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/mergelimiter.h @@ -11,9 +11,9 @@ class MergeLimiter { uint16_t _maxNodes; public: - typedef std::vector<MergeMetaData> NodeArray; + using NodeArray = std::vector<MergeMetaData>; - MergeLimiter(uint16_t maxNodes); + explicit MergeLimiter(uint16_t maxNodes); void limitMergeToMaxNodes(NodeArray&); }; diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.cpp index c5487511296..09fec1a88fd 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.cpp @@ -3,14 +3,11 @@ #include "mergemetadata.h" #include <vespa/vespalib/stllike/asciistream.h> -namespace storage { -namespace distributor { +namespace storage::distributor { vespalib::asciistream& operator<<(vespalib::asciistream& out, const MergeMetaData& e) { return out << "MergeMetaData(" << e._nodeIndex << ")"; } -} // distributor -} // storage - +} // storage::distributor diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.h b/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.h index 446ab0407ca..07d1df4e81c 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.h @@ -17,13 +17,13 @@ struct MergeMetaData { MergeMetaData(uint16_t nodeIndex, const BucketCopy& copy) noexcept : _nodeIndex(nodeIndex), _sourceOnly(false), _copy(©) {} - bool trusted() const { + [[nodiscard]] bool trusted() const noexcept { return _copy->trusted(); } - uint32_t checksum() const { + [[nodiscard]] uint32_t checksum() const noexcept { return _copy->getChecksum(); } - bool source_only() const noexcept { return _sourceOnly; } + [[nodiscard]] bool source_only() const noexcept { return _sourceOnly; } }; vespalib::asciistream& operator<<(vespalib::asciistream& out, const MergeMetaData& e); diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.h b/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.h index 014bae842fa..5416df3a43d 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.h @@ -10,6 +10,7 @@ #include <vespa/storageapi/message/bucket.h> namespace storage::lib { class Distribution; } + namespace storage::distributor { class MergeBucketMetricSet; @@ -26,7 +27,6 @@ protected: MergeLimiter _limiter; public: - static const int LOAD = 10; MergeOperation(const BucketAndNodes& nodes, uint16_t maxNodes = 16) : IdealStateOperation(nodes), @@ -34,13 +34,13 @@ public: _limiter(maxNodes) {} - ~MergeOperation(); + ~MergeOperation() override; void onStart(DistributorStripeMessageSender& sender) override; void onReceive(DistributorStripeMessageSender& sender, const api::StorageReply::SP&) override; - const char* getName() const override { return "merge"; }; + const char* getName() const noexcept override { return "merge"; }; std::string getStatus() const override; - Type getType() const override { return MERGE_BUCKET; } + Type getType() const noexcept override { return MERGE_BUCKET; } /** Generates ordered list of nodes that should be included in the merge */ static void generateSortedNodeList( diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.cpp index 25cae5b9979..e46ccebffba 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.cpp @@ -9,7 +9,9 @@ LOG_SETUP(".distributor.operation.idealstate.remove"); -using namespace storage::distributor; +namespace storage::distributor { + +RemoveBucketOperation::~RemoveBucketOperation() = default; bool RemoveBucketOperation::onStartInternal(DistributorStripeMessageSender& sender) @@ -123,3 +125,4 @@ RemoveBucketOperation::shouldBlockThisOperation(uint32_t, uint16_t target_node, return false; } +} // storage::distributor diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.h b/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.h index 5e0922d5685..db6f5b997cc 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.h @@ -14,6 +14,7 @@ public: RemoveBucketOperation(const ClusterContext &cluster_context, const BucketAndNodes& nodes) : IdealStateOperation(nodes), _tracker(cluster_context) {} + ~RemoveBucketOperation() override; /** Sends messages, returns true if we are done (sent nothing). @@ -28,8 +29,8 @@ public: bool onReceiveInternal(const std::shared_ptr<api::StorageReply> &); void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> &) override; - const char* getName() const override { return "remove"; }; - Type getType() const override { return DELETE_BUCKET; } + const char* getName() const noexcept override { return "remove"; }; + Type getType() const noexcept override { return DELETE_BUCKET; } bool shouldBlockThisOperation(uint32_t, uint16_t, uint8_t) const override; protected: MessageTracker _tracker; diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/setbucketstateoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/setbucketstateoperation.cpp index 76fb6829609..00906d22ea4 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/setbucketstateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/setbucketstateoperation.cpp @@ -10,7 +10,7 @@ LOG_SETUP(".distributor.operation.idealstate.setactive"); namespace storage::distributor { -SetBucketStateOperation::SetBucketStateOperation(const ClusterContext &cluster_ctx, +SetBucketStateOperation::SetBucketStateOperation(const ClusterContext& cluster_ctx, const BucketAndNodes& nodes, const std::vector<uint16_t>& wantedActiveNodes) : IdealStateOperation(nodes), @@ -37,7 +37,9 @@ bool SetBucketStateOperation::shouldBeActive(uint16_t node) const { for (uint32_t i=0, n=_wantedActiveNodes.size(); i<n; ++i) { - if (_wantedActiveNodes[i] == node) return true; + if (_wantedActiveNodes[i] == node) { + return true; + } } return false; } @@ -73,8 +75,7 @@ void SetBucketStateOperation::onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply>& reply) { - api::SetBucketStateReply& rep( - static_cast<api::SetBucketStateReply&>(*reply)); + auto& rep = static_cast<api::SetBucketStateReply&>(*reply); const uint16_t node = _tracker.handleReply(rep); LOG(debug, "Got %s from node %u", reply->toString(true).c_str(), node); diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/setbucketstateoperation.h b/storage/src/vespa/storage/distributor/operations/idealstate/setbucketstateoperation.h index 18160ba4d84..76c9c704653 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/setbucketstateoperation.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/setbucketstateoperation.h @@ -16,8 +16,8 @@ public: void onStart(DistributorStripeMessageSender&) override; void onReceive(DistributorStripeMessageSender&, const std::shared_ptr<api::StorageReply>&) override; - const char* getName() const override { return "setbucketstate"; } - Type getType() const override { return SET_BUCKET_STATE; } + const char* getName() const noexcept override { return "setbucketstate"; } + Type getType() const noexcept override { return SET_BUCKET_STATE; } protected: MessageTracker _tracker; std::vector<uint16_t> _wantedActiveNodes; diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.cpp index 6f3924535ef..97d86528ea0 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.cpp @@ -11,7 +11,7 @@ LOG_SETUP(".distributor.operation.idealstate.split"); using namespace storage::distributor; -SplitOperation::SplitOperation(const ClusterContext &cluster_ctx, const BucketAndNodes& nodes, +SplitOperation::SplitOperation(const ClusterContext& cluster_ctx, const BucketAndNodes& nodes, uint32_t maxBits, uint32_t splitCount, uint32_t splitSize) : IdealStateOperation(nodes), _tracker(cluster_ctx), @@ -19,6 +19,7 @@ SplitOperation::SplitOperation(const ClusterContext &cluster_ctx, const BucketAn _splitCount(splitCount), _splitSize(splitSize) {} + SplitOperation::~SplitOperation() = default; void diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.h b/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.h index 6a268155fc8..604b29e296c 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.h @@ -12,14 +12,14 @@ public: SplitOperation(const ClusterContext& cluster_ctx, const BucketAndNodes& nodes, uint32_t maxBits, uint32_t splitCount, uint32_t splitSize); - SplitOperation(const SplitOperation &) = delete; - SplitOperation & operator = (const SplitOperation &) = delete; + SplitOperation(const SplitOperation&) = delete; + SplitOperation& operator=(const SplitOperation&) = delete; ~SplitOperation(); void onStart(DistributorStripeMessageSender& sender) override; - void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> &) override; - const char* getName() const override { return "split"; }; - Type getType() const override { return SPLIT_BUCKET; } + void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply>&) override; + const char* getName() const noexcept override { return "split"; }; + Type getType() const noexcept override { return SPLIT_BUCKET; } bool isBlocked(const DistributorStripeOperationContext&, const OperationSequencer&) const override; bool shouldBlockThisOperation(uint32_t, uint16_t, uint8_t) const override; protected: diff --git a/storage/src/vespa/storage/distributor/operations/operation.h b/storage/src/vespa/storage/distributor/operations/operation.h index fff0450cc04..e24aa976221 100644 --- a/storage/src/vespa/storage/distributor/operations/operation.h +++ b/storage/src/vespa/storage/distributor/operations/operation.h @@ -23,7 +23,7 @@ class OperationSequencer; class Operation { public: - typedef std::shared_ptr<Operation> SP; + using SP = std::shared_ptr<Operation>; Operation(); @@ -45,7 +45,7 @@ public: onReceive(sender, msg); } - virtual const char* getName() const = 0; + virtual const char* getName() const noexcept = 0; virtual std::string getStatus() const; diff --git a/storage/src/vespa/storage/distributor/operationtargetresolver.cpp b/storage/src/vespa/storage/distributor/operationtargetresolver.cpp index 6362e6fa2ed..62be9e47125 100644 --- a/storage/src/vespa/storage/distributor/operationtargetresolver.cpp +++ b/storage/src/vespa/storage/distributor/operationtargetresolver.cpp @@ -3,8 +3,7 @@ #include "operationtargetresolver.h" #include <vespa/vespalib/stllike/asciistream.h> -namespace storage { -namespace distributor { +namespace storage::distributor { void OperationTarget::print(vespalib::asciistream& out, const PrintProperties&) const { @@ -13,5 +12,3 @@ OperationTarget::print(vespalib::asciistream& out, const PrintProperties&) const } } -} - diff --git a/storage/src/vespa/storage/distributor/operationtargetresolver.h b/storage/src/vespa/storage/distributor/operationtargetresolver.h index ea8c901fbeb..5e3c4a73f66 100644 --- a/storage/src/vespa/storage/distributor/operationtargetresolver.h +++ b/storage/src/vespa/storage/distributor/operationtargetresolver.h @@ -56,7 +56,7 @@ public: class OperationTargetResolver { public: - virtual ~OperationTargetResolver() {} + virtual ~OperationTargetResolver() = default; // Sadly all operations but put currently implement this by themselves. enum OperationType { diff --git a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp index 86b469fe471..6a9d7e0e6da 100644 --- a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp +++ b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp @@ -82,11 +82,10 @@ BucketInstanceList::populate(const document::BucketId& specificId, const Distrib void BucketInstanceList::removeNodeDuplicates() { - // Normally few entries in list. Probably heaper to just go through entries - // to detect whether it pre exist rather than creating a set or similar. + // Normally few entries in list. Probably cheaper to just go through entries + // to detect whether it preexists rather than creating a set or similar. BucketInstanceList other; - for (uint32_t i=0; i<_instances.size(); ++i) { - BucketInstance& instance(_instances[i]); + for (const auto& instance : _instances) { if (!other.contains(instance._node)) { other.add(instance); } @@ -97,7 +96,9 @@ BucketInstanceList::removeNodeDuplicates() void BucketInstanceList::limitToRedundancyCopies(uint16_t redundancy) { - while (_instances.size() > redundancy) _instances.pop_back(); + while (_instances.size() > redundancy) { + _instances.pop_back(); + } } document::BucketId @@ -143,8 +144,7 @@ OperationTargetList BucketInstanceList::createTargets(document::BucketSpace bucketSpace) { OperationTargetList result; - for (uint32_t i=0; i<_instances.size(); ++i) { - BucketInstance& bi(_instances[i]); + for (const auto& bi : _instances) { result.push_back(OperationTarget(document::Bucket(bucketSpace, bi._bucket), bi._node, !bi._exist)); } return result; diff --git a/storage/src/vespa/storage/distributor/ownership_transfer_safe_time_point_calculator.cpp b/storage/src/vespa/storage/distributor/ownership_transfer_safe_time_point_calculator.cpp index 6a03b2aa9bd..818c812ab4f 100644 --- a/storage/src/vespa/storage/distributor/ownership_transfer_safe_time_point_calculator.cpp +++ b/storage/src/vespa/storage/distributor/ownership_transfer_safe_time_point_calculator.cpp @@ -3,8 +3,7 @@ #include "ownership_transfer_safe_time_point_calculator.h" #include <thread> -namespace storage { -namespace distributor { +namespace storage::distributor { OwnershipTransferSafeTimePointCalculator::TimePoint OwnershipTransferSafeTimePointCalculator::safeTimePoint(TimePoint now) const @@ -31,4 +30,3 @@ OwnershipTransferSafeTimePointCalculator::safeTimePoint(TimePoint now) const } } -} diff --git a/storage/src/vespa/storage/distributor/pendingclusterstate.cpp b/storage/src/vespa/storage/distributor/pendingclusterstate.cpp index cf32d21eb82..c03b211d1aa 100644 --- a/storage/src/vespa/storage/distributor/pendingclusterstate.cpp +++ b/storage/src/vespa/storage/distributor/pendingclusterstate.cpp @@ -29,7 +29,7 @@ PendingClusterState::PendingClusterState( DistributorMessageSender& sender, const BucketSpaceStateMap& bucket_space_states, const std::shared_ptr<api::SetSystemStateCommand>& newStateCmd, - const OutdatedNodesMap &outdatedNodesMap, + const OutdatedNodesMap& outdatedNodesMap, api::Timestamp creationTimestamp) : _cmd(newStateCmd), _sentMessages(), @@ -79,7 +79,7 @@ PendingClusterState::PendingClusterState( PendingClusterState::~PendingClusterState() = default; void -PendingClusterState::initializeBucketSpaceTransitions(bool distributionChanged, const OutdatedNodesMap &outdatedNodesMap) +PendingClusterState::initializeBucketSpaceTransitions(bool distributionChanged, const OutdatedNodesMap& outdatedNodesMap) { OutdatedNodes emptyOutdatedNodes; for (const auto &elem : _bucket_space_states) { @@ -129,12 +129,6 @@ PendingClusterState::getOutdatedNodesMap() const return outdatedNodesMap; } -uint16_t -PendingClusterState::newStateStorageNodeCount() const -{ - return _newClusterStateBundle.getBaselineClusterState()->getNodeCount(lib::NodeType::STORAGE); -} - bool PendingClusterState::shouldRequestBucketInfo() const { @@ -318,7 +312,7 @@ PendingClusterState::getSummary() const (_clock.getTimeInMicros().getTime() - _creationTimestamp)); } -PendingBucketSpaceDbTransition & +PendingBucketSpaceDbTransition& PendingClusterState::getPendingBucketSpaceDbTransition(document::BucketSpace bucketSpace) { auto transitionIter = _pendingTransitions.find(bucketSpace); diff --git a/storage/src/vespa/storage/distributor/pendingclusterstate.h b/storage/src/vespa/storage/distributor/pendingclusterstate.h index 1a2f8901b47..cbc526d75d3 100644 --- a/storage/src/vespa/storage/distributor/pendingclusterstate.h +++ b/storage/src/vespa/storage/distributor/pendingclusterstate.h @@ -49,7 +49,7 @@ public: DistributorMessageSender& sender, const BucketSpaceStateMap& bucket_space_states, const std::shared_ptr<api::SetSystemStateCommand>& newStateCmd, - const OutdatedNodesMap &outdatedNodesMap, + const OutdatedNodesMap& outdatedNodesMap, api::Timestamp creationTimestamp) { // Naked new due to private constructor @@ -99,7 +99,7 @@ public: * Returns true if all the nodes we requested have replied to * the request bucket info commands. */ - bool done() { + [[nodiscard]] bool done() const noexcept { return _sentMessages.empty() && _delayedRequests.empty(); } @@ -111,7 +111,7 @@ public: return (_cmd.get() != nullptr); } - std::shared_ptr<api::SetSystemStateCommand> getCommand() { + std::shared_ptr<api::SetSystemStateCommand> getCommand() const noexcept { return _cmd; } @@ -128,7 +128,7 @@ public: && _newClusterStateBundle.deferredActivation()); } - void clearCommand() { + void clearCommand() noexcept { _cmd.reset(); } @@ -151,7 +151,7 @@ public: void merge_into_bucket_databases(StripeAccessGuard& guard); // Get pending transition for a specific bucket space. Only used by unit test. - PendingBucketSpaceDbTransition &getPendingBucketSpaceDbTransition(document::BucketSpace bucketSpace); + PendingBucketSpaceDbTransition& getPendingBucketSpaceDbTransition(document::BucketSpace bucketSpace); // May be a subset of the nodes in the cluster, depending on how many nodes were consulted // as part of the pending cluster state. Caller must take care to aggregate features. @@ -202,16 +202,12 @@ private: } }; - void initializeBucketSpaceTransitions(bool distributionChanged, const OutdatedNodesMap &outdatedNodesMap); + void initializeBucketSpaceTransitions(bool distributionChanged, const OutdatedNodesMap& outdatedNodesMap); void logConstructionInformation() const; void requestNode(BucketSpaceAndNode bucketSpaceAndNode); void requestNodes(); void requestBucketInfoFromStorageNodesWithChangedState(); - /** - * Number of nodes with node type 'storage' in _newClusterState. - */ - uint16_t newStateStorageNodeCount() const; bool shouldRequestBucketInfo() const; bool clusterIsDown() const; bool iAmDown() const; @@ -222,26 +218,30 @@ private: void update_reply_failure_statistics(const api::ReturnCode& result, const BucketSpaceAndNode& source); void update_node_supported_features_from_reply(uint16_t node, const api::RequestBucketInfoReply& reply); - std::shared_ptr<api::SetSystemStateCommand> _cmd; + using SentMessages = std::map<uint64_t, BucketSpaceAndNode>; + using DelayedRequests = std::deque<std::pair<framework::MilliSecTime, BucketSpaceAndNode>>; + using PendingTransitions = std::unordered_map<document::BucketSpace, std::unique_ptr<PendingBucketSpaceDbTransition>, document::BucketSpace::hash>; + using NodeFeatures = vespalib::hash_map<uint16_t, NodeSupportedFeatures>; - std::map<uint64_t, BucketSpaceAndNode> _sentMessages; - std::vector<bool> _requestedNodes; - std::deque<std::pair<framework::MilliSecTime, BucketSpaceAndNode> > _delayedRequests; + std::shared_ptr<api::SetSystemStateCommand> _cmd; - lib::ClusterStateBundle _prevClusterStateBundle; - lib::ClusterStateBundle _newClusterStateBundle; + SentMessages _sentMessages; + std::vector<bool> _requestedNodes; + DelayedRequests _delayedRequests; - const framework::Clock& _clock; - ClusterInformation::CSP _clusterInfo; - api::Timestamp _creationTimestamp; + lib::ClusterStateBundle _prevClusterStateBundle; + lib::ClusterStateBundle _newClusterStateBundle; - DistributorMessageSender& _sender; + const framework::Clock& _clock; + ClusterInformation::CSP _clusterInfo; + api::Timestamp _creationTimestamp; + DistributorMessageSender& _sender; const BucketSpaceStateMap& _bucket_space_states; - uint32_t _clusterStateVersion; - bool _isVersionedTransition; - bool _bucketOwnershipTransfer; - std::unordered_map<document::BucketSpace, std::unique_ptr<PendingBucketSpaceDbTransition>, document::BucketSpace::hash> _pendingTransitions; - vespalib::hash_map<uint16_t, NodeSupportedFeatures> _node_features; + uint32_t _clusterStateVersion; + bool _isVersionedTransition; + bool _bucketOwnershipTransfer; + PendingTransitions _pendingTransitions; + NodeFeatures _node_features; }; } diff --git a/storage/src/vespa/storage/distributor/pendingmessagetracker.h b/storage/src/vespa/storage/distributor/pendingmessagetracker.h index 8f5f05d8a1a..b7fa666fe42 100644 --- a/storage/src/vespa/storage/distributor/pendingmessagetracker.h +++ b/storage/src/vespa/storage/distributor/pendingmessagetracker.h @@ -77,7 +77,7 @@ public: */ using TimePoint = std::chrono::milliseconds; - explicit PendingMessageTracker(framework::ComponentRegister&, uint32_t stripe_index); + PendingMessageTracker(framework::ComponentRegister&, uint32_t stripe_index); ~PendingMessageTracker() override; void insert(const std::shared_ptr<api::StorageMessage>&); @@ -111,8 +111,8 @@ public: * The vector might be smaller than a given node index. In that case, that storage * node has never had any pending messages. */ - const NodeInfo& getNodeInfo() const { return _nodeInfo; } - NodeInfo& getNodeInfo() { return _nodeInfo; } + const NodeInfo& getNodeInfo() const noexcept { return _nodeInfo; } + NodeInfo& getNodeInfo() noexcept { return _nodeInfo; } /** * Clears all pending messages for the given node, and returns diff --git a/storage/src/vespa/storage/distributor/persistence_operation_metric_set.cpp b/storage/src/vespa/storage/distributor/persistence_operation_metric_set.cpp index 0e42a505567..944b4bafa0a 100644 --- a/storage/src/vespa/storage/distributor/persistence_operation_metric_set.cpp +++ b/storage/src/vespa/storage/distributor/persistence_operation_metric_set.cpp @@ -4,7 +4,7 @@ #include <vespa/storageapi/messageapi/returncode.h> #include <vespa/metrics/summetric.hpp> -namespace storage { +namespace storage::distributor { using metrics::MetricSet; @@ -107,5 +107,4 @@ PersistenceOperationMetricSet::updateFromResult(const api::ReturnCode& result) } } -} // storage - +} // storage::distributor diff --git a/storage/src/vespa/storage/distributor/persistence_operation_metric_set.h b/storage/src/vespa/storage/distributor/persistence_operation_metric_set.h index d42e6fa308c..b818d1bdd9f 100644 --- a/storage/src/vespa/storage/distributor/persistence_operation_metric_set.h +++ b/storage/src/vespa/storage/distributor/persistence_operation_metric_set.h @@ -8,11 +8,9 @@ #include <vespa/metrics/summetric.h> #include <mutex> -namespace storage { +namespace storage::api { class ReturnCode; } -namespace api { -class ReturnCode; -} +namespace storage::distributor { class PersistenceFailuresMetricSet : public metrics::MetricSet { diff --git a/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp b/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp index 45129f7be04..08b99cf89a8 100644 --- a/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp +++ b/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp @@ -4,7 +4,6 @@ #include "distributor_bucket_space_repo.h" #include "distributor_bucket_space.h" #include <vespa/vdslib/distribution/distribution.h> -#include <vespa/storage/common/vectorprinter.h> #include <vespa/storageapi/message/persistence.h> #include <vespa/log/log.h> @@ -41,7 +40,7 @@ PersistenceMessageTrackerImpl::updateDB() _op_ctx.update_bucket_database(entry.first, entry.second); } - for (const auto & entry : _remapBucketInfo){ + for (const auto & entry : _remapBucketInfo){ _op_ctx.update_bucket_database(entry.first, entry.second, DatabaseUpdate::CREATE_IF_NONEXISTING); } } diff --git a/storage/src/vespa/storage/distributor/persistencemessagetracker.h b/storage/src/vespa/storage/distributor/persistencemessagetracker.h index e65325cff20..ca330859259 100644 --- a/storage/src/vespa/storage/distributor/persistencemessagetracker.h +++ b/storage/src/vespa/storage/distributor/persistencemessagetracker.h @@ -43,7 +43,7 @@ public: void updateDB(); void updateMetrics(); - bool success() const { return _success; } + [[nodiscard]] bool success() const noexcept { return _success; } void fail(MessageSender& sender, const api::ReturnCode& result) override; /** @@ -67,19 +67,19 @@ public: private: using MessageBatch = std::vector<uint64_t>; - std::vector<MessageBatch> _messageBatches; - PersistenceOperationMetricSet& _metric; + std::vector<MessageBatch> _messageBatches; + PersistenceOperationMetricSet& _metric; std::shared_ptr<api::BucketInfoReply> _reply; - DistributorStripeOperationContext& _op_ctx; - api::Timestamp _revertTimestamp; - std::vector<BucketNodePair> _revertNodes; - mbus::Trace _trace; - framework::MilliSecTimer _requestTimer; - uint32_t _n_persistence_replies_total; - uint32_t _n_successful_persistence_replies; - uint8_t _priority; - bool _success; + DistributorStripeOperationContext& _op_ctx; + api::Timestamp _revertTimestamp; + std::vector<BucketNodePair> _revertNodes; + mbus::Trace _trace; + framework::MilliSecTimer _requestTimer; + uint32_t _n_persistence_replies_total; + uint32_t _n_successful_persistence_replies; + uint8_t _priority; + bool _success; bool canSendReplyEarly() const; void addBucketInfoFromReply(uint16_t node, const api::BucketInfoReply& reply); diff --git a/storage/src/vespa/storage/distributor/statechecker.h b/storage/src/vespa/storage/distributor/statechecker.h index 11c83fdde07..348d90bc712 100644 --- a/storage/src/vespa/storage/distributor/statechecker.h +++ b/storage/src/vespa/storage/distributor/statechecker.h @@ -37,7 +37,7 @@ class NodeMaintenanceStatsTracker; */ class StateChecker { public: - typedef std::shared_ptr<StateChecker> SP; + using SP = std::shared_ptr<StateChecker>; /** * Context object used when generating operations and metrics for a @@ -49,48 +49,46 @@ public: const DistributorStripeOperationContext& op_ctx_in, const DistributorBucketSpace &distributorBucketSpace, NodeMaintenanceStatsTracker&, - const document::Bucket &bucket_); + const document::Bucket& bucket_); ~Context(); - Context(const Context &) = delete; - Context & operator =(const Context &) = delete; + Context(const Context&) = delete; + Context& operator=(const Context&) = delete; // Per bucket - document::Bucket bucket; - document::BucketId siblingBucket; - - BucketDatabase::Entry entry; - BucketDatabase::Entry siblingEntry; + document::Bucket bucket; + document::BucketId siblingBucket; + BucketDatabase::Entry entry; + BucketDatabase::Entry siblingEntry; std::vector<BucketDatabase::Entry> entries; // Common - const lib::ClusterState& systemState; - const lib::ClusterState* pending_cluster_state; // nullptr if no state is pending. + const lib::ClusterState& systemState; + const lib::ClusterState* pending_cluster_state; // nullptr if no state is pending. const DistributorConfiguration& distributorConfig; - const lib::Distribution& distribution; - - BucketGcTimeCalculator gcTimeCalculator; + const lib::Distribution& distribution; + BucketGcTimeCalculator gcTimeCalculator; // Separate ideal state into ordered sequence and unordered set, as we // need to both know the actual order (activation prioritization etc) as // well as have the ability to quickly check if a node is in an ideal // location. - std::vector<uint16_t> idealState; + std::vector<uint16_t> idealState; std::unordered_set<uint16_t> unorderedIdealState; - const DistributorNodeContext& node_ctx; + const DistributorNodeContext& node_ctx; const DistributorStripeOperationContext& op_ctx; - const BucketDatabase& db; - NodeMaintenanceStatsTracker& stats; - const bool merges_inhibited_in_bucket_space; + const BucketDatabase& db; + NodeMaintenanceStatsTracker& stats; + const bool merges_inhibited_in_bucket_space; - const BucketDatabase::Entry& getSiblingEntry() const { + const BucketDatabase::Entry& getSiblingEntry() const noexcept { return siblingEntry; } - document::Bucket getBucket() const { return bucket; } - document::BucketId getBucketId() const { return bucket.getBucketId(); } - document::BucketSpace getBucketSpace() const { return bucket.getBucketSpace(); } + document::Bucket getBucket() const noexcept { return bucket; } + document::BucketId getBucketId() const noexcept { return bucket.getBucketId(); } + document::BucketSpace getBucketSpace() const noexcept { return bucket.getBucketSpace(); } std::string toString() const; }; @@ -151,7 +149,7 @@ public: /** * Returns the name of this state checker. */ - virtual const char* getName() const = 0; + virtual const char* getName() const noexcept = 0; }; } diff --git a/storage/src/vespa/storage/distributor/statecheckers.h b/storage/src/vespa/storage/distributor/statecheckers.h index f998dbeacfd..c1579cb5eda 100644 --- a/storage/src/vespa/storage/distributor/statecheckers.h +++ b/storage/src/vespa/storage/distributor/statecheckers.h @@ -9,14 +9,14 @@ class SynchronizeAndMoveStateChecker : public StateChecker { public: Result check(Context& c) const override; - const char* getName() const override { return "SynchronizeAndMove"; } + const char* getName() const noexcept override { return "SynchronizeAndMove"; } }; class DeleteExtraCopiesStateChecker : public StateChecker { public: Result check(Context& c) const override; - const char* getName() const override { return "DeleteExtraCopies"; } + const char* getName() const noexcept override { return "DeleteExtraCopies"; } private: static bool bucketHasNoData(const Context& c); @@ -33,7 +33,7 @@ class JoinBucketsStateChecker : public StateChecker { public: Result check(Context& c) const override; - const char* getName() const override { return "JoinBuckets"; } + const char* getName() const noexcept override { return "JoinBuckets"; } private: static uint64_t getTotalUsedFileSize(const Context& c); static uint64_t getTotalMetaCount(const Context& c); @@ -50,7 +50,7 @@ class SplitBucketStateChecker : public StateChecker { public: Result check(Context& c) const override; - const char* getName() const override { return "SplitBucket"; } + const char* getName() const noexcept override { return "SplitBucket"; } private: static Result generateMinimumBucketSplitOperation(Context& c); static Result generateMaxSizeExceededSplitOperation(Context& c); @@ -63,14 +63,12 @@ class SplitInconsistentStateChecker : public StateChecker { public: Result check(Context& c) const override; - const char* getName() const override { return "SplitInconsistentBuckets"; } + const char* getName() const noexcept override { return "SplitInconsistentBuckets"; } private: - typedef std::pair<document::BucketId, uint16_t> BucketAndNode; static bool isLeastSplitBucket(const document::BucketId& bucket,const std::vector<BucketDatabase::Entry>& entries); static uint32_t getHighestUsedBits(const std::vector<BucketDatabase::Entry>& entries); static vespalib::string getReason(const document::BucketId& bucketId, const std::vector<BucketDatabase::Entry>& entries); - static bool isLeastSplit(Context& c, std::vector<BucketAndNode>& others); }; class ActiveList; @@ -80,14 +78,14 @@ class BucketStateStateChecker : public StateChecker static bool shouldSkipActivationDueToMaintenance(const ActiveList& activeList, const Context& c); public: Result check(Context& c) const override; - const char* getName() const override { return "SetBucketState"; } + const char* getName() const noexcept override { return "SetBucketState"; } }; class GarbageCollectionStateChecker : public StateChecker { public: Result check(Context& c) const override; - const char* getName() const override { return "GarbageCollection"; } + const char* getName() const noexcept override { return "GarbageCollection"; } private: static bool garbage_collection_disabled(const Context& c) noexcept; static bool needs_garbage_collection(const Context& c, std::chrono::seconds time_since_epoch); diff --git a/storage/src/vespa/storage/distributor/statusdelegator.h b/storage/src/vespa/storage/distributor/statusdelegator.h index 0a4f3e63472..3001f135964 100644 --- a/storage/src/vespa/storage/distributor/statusdelegator.h +++ b/storage/src/vespa/storage/distributor/statusdelegator.h @@ -1,18 +1,16 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -namespace storage { -namespace distributor { +namespace storage::distributor { struct DelegatedStatusRequest; class StatusDelegator { public: - virtual ~StatusDelegator() {} + virtual ~StatusDelegator() = default; virtual bool handleStatusRequest(const DelegatedStatusRequest& request) const = 0; }; -} // distributor -} // storage +} // storage::distributor diff --git a/storage/src/vespa/storage/distributor/statusreporterdelegate.cpp b/storage/src/vespa/storage/distributor/statusreporterdelegate.cpp index 41fc85baca5..a441ed9504c 100644 --- a/storage/src/vespa/storage/distributor/statusreporterdelegate.cpp +++ b/storage/src/vespa/storage/distributor/statusreporterdelegate.cpp @@ -15,6 +15,8 @@ StatusReporterDelegate::StatusReporterDelegate( { } +StatusReporterDelegate::~StatusReporterDelegate() = default; + vespalib::string StatusReporterDelegate::getReportContentType(const framework::HttpUrlPath& path) const { diff --git a/storage/src/vespa/storage/distributor/statusreporterdelegate.h b/storage/src/vespa/storage/distributor/statusreporterdelegate.h index 0acf9b16edb..fb13812beb0 100644 --- a/storage/src/vespa/storage/distributor/statusreporterdelegate.h +++ b/storage/src/vespa/storage/distributor/statusreporterdelegate.h @@ -5,9 +5,7 @@ #include "statusdelegator.h" #include <vespa/storageframework/generic/component/component.h> - -namespace storage { -namespace distributor { +namespace storage::distributor { class StatusReporterDelegate : public framework::StatusReporter @@ -19,11 +17,11 @@ public: StatusReporterDelegate(framework::ComponentRegister& compReg, const StatusDelegator& delegator, const framework::StatusReporter& target); + ~StatusReporterDelegate() override; void registerStatusPage(); vespalib::string getReportContentType(const framework::HttpUrlPath&) const override; bool reportStatus(std::ostream&, const framework::HttpUrlPath&) const override; }; -} // distributor -} // storage +} // storage::distributor diff --git a/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h b/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h index 7a34ed03a3c..b31ffe53451 100644 --- a/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h +++ b/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h @@ -4,12 +4,12 @@ #include "bucketlistmerger.h" #include "distributor_stripe_component.h" #include "distributormessagesender.h" -#include "messageguard.h" #include "operation_routing_snapshot.h" #include "outdated_nodes_map.h" #include "pendingclusterstate.h" #include "potential_data_loss_report.h" #include <vespa/document/bucket/bucket.h> +#include <vespa/storage/common/message_guard.h> #include <vespa/storage/common/storagelink.h> #include <vespa/storageapi/message/bucket.h> #include <vespa/storageapi/messageapi/messagehandler.h> diff --git a/storage/src/vespa/storage/distributor/throttlingoperationstarter.h b/storage/src/vespa/storage/distributor/throttlingoperationstarter.h index 00d1237b2f7..a0613c60fa4 100644 --- a/storage/src/vespa/storage/distributor/throttlingoperationstarter.h +++ b/storage/src/vespa/storage/distributor/throttlingoperationstarter.h @@ -30,7 +30,7 @@ class ThrottlingOperationStarter : public OperationStarter, public PendingWindow void onClose(DistributorStripeMessageSender& sender) override { _operation->onClose(sender); } - const char* getName() const override { + const char* getName() const noexcept override { return _operation->getName(); } std::string getStatus() const override { diff --git a/storage/src/vespa/storage/distributor/top_level_bucket_db_updater.h b/storage/src/vespa/storage/distributor/top_level_bucket_db_updater.h index d8e49d5c383..921ebf65338 100644 --- a/storage/src/vespa/storage/distributor/top_level_bucket_db_updater.h +++ b/storage/src/vespa/storage/distributor/top_level_bucket_db_updater.h @@ -4,11 +4,11 @@ #include "bucketlistmerger.h" #include "distributor_component.h" #include "distributormessagesender.h" -#include "messageguard.h" #include "operation_routing_snapshot.h" #include "outdated_nodes_map.h" #include "pendingclusterstate.h" #include <vespa/document/bucket/bucket.h> +#include <vespa/storage/common/message_guard.h> #include <vespa/storage/common/storagelink.h> #include <vespa/storageapi/message/bucket.h> #include <vespa/storageapi/messageapi/messagehandler.h> diff --git a/storage/src/vespa/storage/distributor/update_metric_set.cpp b/storage/src/vespa/storage/distributor/update_metric_set.cpp index 82f55d3e819..fafce3dae5a 100644 --- a/storage/src/vespa/storage/distributor/update_metric_set.cpp +++ b/storage/src/vespa/storage/distributor/update_metric_set.cpp @@ -2,7 +2,7 @@ #include "update_metric_set.h" -namespace storage { +namespace storage::distributor { using metrics::MetricSet; diff --git a/storage/src/vespa/storage/distributor/update_metric_set.h b/storage/src/vespa/storage/distributor/update_metric_set.h index 4171a8c32da..ad39f8e93cc 100644 --- a/storage/src/vespa/storage/distributor/update_metric_set.h +++ b/storage/src/vespa/storage/distributor/update_metric_set.h @@ -3,7 +3,7 @@ #include "persistence_operation_metric_set.h" -namespace storage { +namespace storage::distributor { class UpdateMetricSet : public PersistenceOperationMetricSet { public: diff --git a/storage/src/vespa/storage/distributor/visitormetricsset.cpp b/storage/src/vespa/storage/distributor/visitormetricsset.cpp index c94dc025fa1..cbc2f0e25d3 100644 --- a/storage/src/vespa/storage/distributor/visitormetricsset.cpp +++ b/storage/src/vespa/storage/distributor/visitormetricsset.cpp @@ -2,7 +2,7 @@ #include "visitormetricsset.h" -namespace storage { +namespace storage::distributor { using metrics::MetricSet; diff --git a/storage/src/vespa/storage/distributor/visitormetricsset.h b/storage/src/vespa/storage/distributor/visitormetricsset.h index 35e03faf279..7751e0805f2 100644 --- a/storage/src/vespa/storage/distributor/visitormetricsset.h +++ b/storage/src/vespa/storage/distributor/visitormetricsset.h @@ -3,7 +3,7 @@ #include "persistence_operation_metric_set.h" -namespace storage { +namespace storage::distributor { struct VisitorMetricSet : public PersistenceOperationMetricSet { metrics::LongAverageMetric buckets_per_visitor; @@ -11,11 +11,10 @@ struct VisitorMetricSet : public PersistenceOperationMetricSet { metrics::LongAverageMetric bytes_per_visitor; VisitorMetricSet(MetricSet* owner = nullptr); - ~VisitorMetricSet(); + ~VisitorMetricSet() override; MetricSet * clone(std::vector<Metric::UP>& ownerList, CopyType copyType, MetricSet* owner, bool includeUnused) const override; }; -} // storage - +} // storage::distributor diff --git a/storage/src/vespa/storage/storageserver/mergethrottler.cpp b/storage/src/vespa/storage/storageserver/mergethrottler.cpp index 681d97299fa..2dea5681b85 100644 --- a/storage/src/vespa/storage/storageserver/mergethrottler.cpp +++ b/storage/src/vespa/storage/storageserver/mergethrottler.cpp @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "mergethrottler.h" +#include <vespa/storageapi/message/state.h> #include <vespa/storage/common/nodestateupdater.h> #include <vespa/storage/common/dummy_mbus_messages.h> #include <vespa/storage/persistence/messages.h> diff --git a/storage/src/vespa/storage/storageserver/mergethrottler.h b/storage/src/vespa/storage/storageserver/mergethrottler.h index e501e0edd39..76a25c0cf22 100644 --- a/storage/src/vespa/storage/storageserver/mergethrottler.h +++ b/storage/src/vespa/storage/storageserver/mergethrottler.h @@ -8,9 +8,9 @@ #pragma once #include <vespa/storage/config/config-stor-server.h> +#include <vespa/storage/common/message_guard.h> #include <vespa/storage/common/storagelink.h> #include <vespa/storage/common/storagecomponent.h> -#include <vespa/storage/distributor/messageguard.h> #include <vespa/storageframework/generic/status/htmlstatusreporter.h> #include <vespa/storageapi/message/bucket.h> #include <vespa/document/bucket/bucket.h> diff --git a/storage/src/vespa/storageapi/defs.h b/storage/src/vespa/storageapi/defs.h index 9ee9fdf2218..bf802a20a5c 100644 --- a/storage/src/vespa/storageapi/defs.h +++ b/storage/src/vespa/storageapi/defs.h @@ -10,9 +10,9 @@ namespace storage:: api { -typedef uint64_t Timestamp; -typedef uint32_t VisitorId; +using Timestamp = uint64_t; +using VisitorId = uint32_t; -const Timestamp MAX_TIMESTAMP = (Timestamp)-1ll; +constexpr Timestamp MAX_TIMESTAMP = (Timestamp)-1LL; } diff --git a/storage/src/vespa/storageapi/message/stat.h b/storage/src/vespa/storageapi/message/stat.h index 0797ae43799..875580ca064 100644 --- a/storage/src/vespa/storageapi/message/stat.h +++ b/storage/src/vespa/storageapi/message/stat.h @@ -58,15 +58,15 @@ class GetBucketListReply : public BucketReply { public: struct BucketInfo { document::BucketId _bucket; - vespalib::string _bucketInformation; + vespalib::string _bucketInformation; BucketInfo(const document::BucketId& id, - vespalib::stringref bucketInformation) + vespalib::stringref bucketInformation) noexcept : _bucket(id), _bucketInformation(bucketInformation) {} - bool operator==(const BucketInfo& other) const { + bool operator==(const BucketInfo& other) const noexcept { return (_bucket == other._bucket && _bucketInformation == other._bucketInformation); } |