diff options
30 files changed, 234 insertions, 229 deletions
diff --git a/client/go/internal/cli/cmd/api_key.go b/client/go/internal/cli/cmd/api_key.go index 8b3780ab82b..7c187aa5da7 100644 --- a/client/go/internal/cli/cmd/api_key.go +++ b/client/go/internal/cli/cmd/api_key.go @@ -54,11 +54,11 @@ Read more in https://cloud.vespa.ai/en/security/guide`, } func doApiKey(cli *CLI, overwriteKey bool, args []string) error { - app, err := cli.config.application() + targetType, err := cli.targetType(true) if err != nil { return err } - targetType, err := cli.targetType() + app, err := cli.config.application() if err != nil { return err } diff --git a/client/go/internal/cli/cmd/cert.go b/client/go/internal/cli/cmd/cert.go index 7fbb357d1db..5c1ed04ab4e 100644 --- a/client/go/internal/cli/cmd/cert.go +++ b/client/go/internal/cli/cmd/cert.go @@ -95,11 +95,11 @@ $ vespa auth cert add -a my-tenant.my-app.my-instance path/to/application/packag } func doCert(cli *CLI, overwriteCertificate, skipApplicationPackage bool, args []string) error { - app, err := cli.config.application() + targetType, err := cli.targetType(true) if err != nil { return err } - targetType, err := cli.targetType() + app, err := cli.config.application() if err != nil { return err } @@ -141,11 +141,11 @@ func doCert(cli *CLI, overwriteCertificate, skipApplicationPackage bool, args [] } func doCertAdd(cli *CLI, overwriteCertificate bool, args []string) error { - pkg, err := cli.applicationPackageFrom(args, false) + target, err := cli.target(targetOptions{cloudExclusive: true}) if err != nil { return err } - target, err := cli.target(targetOptions{}) + pkg, err := cli.applicationPackageFrom(args, false) if err != nil { return err } diff --git a/client/go/internal/cli/cmd/config_test.go b/client/go/internal/cli/cmd/config_test.go index b00be38d021..7a4035f54a3 100644 --- a/client/go/internal/cli/cmd/config_test.go +++ b/client/go/internal/cli/cmd/config_test.go @@ -272,7 +272,7 @@ func TestConfigTargetResolving(t *testing.T) { } func assertTargetType(t *testing.T, expected string, cli *CLI) { - targetType, err := cli.targetType() + targetType, err := cli.targetType(false) require.Nil(t, err) assert.Equal(t, expected, targetType.name) } diff --git a/client/go/internal/cli/cmd/destroy.go b/client/go/internal/cli/cmd/destroy.go index ca69f21a9b4..38d93f49675 100644 --- a/client/go/internal/cli/cmd/destroy.go +++ b/client/go/internal/cli/cmd/destroy.go @@ -36,18 +36,14 @@ $ vespa destroy --force`, DisableAutoGenTag: true, SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { - target, err := cli.target(targetOptions{}) + target, err := cli.target(targetOptions{cloudExclusive: true}) if err != nil { return err } description := target.Deployment().String() - if !target.IsCloud() { - return errHint(fmt.Errorf("cannot remove deployment, only supported for Vespa Cloud")) - } else { - env := target.Deployment().Zone.Environment - if env != "dev" && env != "perf" { - return errHint(fmt.Errorf("cannot remove production %s", description), "See https://cloud.vespa.ai/en/deleting-applications") - } + env := target.Deployment().Zone.Environment + if env != "dev" && env != "perf" { + return errHint(fmt.Errorf("cannot remove production %s", description), "See https://cloud.vespa.ai/en/deleting-applications") } ok := force if !ok { diff --git a/client/go/internal/cli/cmd/destroy_test.go b/client/go/internal/cli/cmd/destroy_test.go index b23e524e0ab..44610576d7e 100644 --- a/client/go/internal/cli/cmd/destroy_test.go +++ b/client/go/internal/cli/cmd/destroy_test.go @@ -56,5 +56,5 @@ func TestDestroy(t *testing.T) { require.Nil(t, cli.Run("config", "set", "target", "local")) require.Nil(t, cli.Run("config", "set", "application", "foo.bar.baz")) require.NotNil(t, cli.Run("destroy", "-z", "prod.aws-us-east-1c")) - assert.Equal(t, "Error: cannot remove deployment, only supported for Vespa Cloud\n", stderr.String()) + assert.Equal(t, "Error: unsupported target local: this command only supports targets cloud and hosted\n", stderr.String()) } diff --git a/client/go/internal/cli/cmd/log.go b/client/go/internal/cli/cmd/log.go index fa07e33538c..8d3f3f4f384 100644 --- a/client/go/internal/cli/cmd/log.go +++ b/client/go/internal/cli/cmd/log.go @@ -34,7 +34,7 @@ $ vespa log --follow`, SilenceUsage: true, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - target, err := cli.target(targetOptions{logLevel: levelArg}) + target, err := cli.target(targetOptions{logLevel: levelArg, cloudExclusive: true}) if err != nil { return err } diff --git a/client/go/internal/cli/cmd/login.go b/client/go/internal/cli/cmd/login.go index baf35ce7954..d6eb8207b7f 100644 --- a/client/go/internal/cli/cmd/login.go +++ b/client/go/internal/cli/cmd/login.go @@ -20,13 +20,13 @@ func newLoginCmd(cli *CLI) *cobra.Command { return &cobra.Command{ Use: "login", Args: cobra.NoArgs, - Short: "Authenticate the Vespa CLI", + Short: "Authenticate Vespa CLI with Vespa Cloud", Example: "$ vespa auth login", DisableAutoGenTag: true, SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - targetType, err := cli.targetType() + targetType, err := cli.targetType(true) if err != nil { return err } diff --git a/client/go/internal/cli/cmd/logout.go b/client/go/internal/cli/cmd/logout.go index 93f7cb6270f..204513145aa 100644 --- a/client/go/internal/cli/cmd/logout.go +++ b/client/go/internal/cli/cmd/logout.go @@ -9,12 +9,12 @@ func newLogoutCmd(cli *CLI) *cobra.Command { return &cobra.Command{ Use: "logout", Args: cobra.NoArgs, - Short: "Log out of Vespa Cli", + Short: "Sign out of Vespa Cloud", Example: "$ vespa auth logout", DisableAutoGenTag: true, SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { - targetType, err := cli.targetType() + targetType, err := cli.targetType(true) if err != nil { return err } diff --git a/client/go/internal/cli/cmd/prod.go b/client/go/internal/cli/cmd/prod.go index 79a6907eef2..1a2f88311b6 100644 --- a/client/go/internal/cli/cmd/prod.go +++ b/client/go/internal/cli/cmd/prod.go @@ -53,6 +53,10 @@ https://cloud.vespa.ai/en/reference/deployment`, DisableAutoGenTag: true, SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { + target, err := cli.target(targetOptions{noCertificate: true, cloudExclusive: true}) + if err != nil { + return err + } pkg, err := cli.applicationPackageFrom(args, false) if err != nil { return err @@ -70,10 +74,6 @@ https://cloud.vespa.ai/en/reference/deployment`, if err != nil { return fmt.Errorf("a services.xml declaring your cluster(s) must exist: %w", err) } - target, err := cli.target(targetOptions{noCertificate: true}) - if err != nil { - return err - } fmt.Fprint(cli.Stdout, "This will modify any existing ", color.YellowString("deployment.xml"), " and ", color.YellowString("services.xml"), "!\nBefore modification a backup of the original file will be created.\n\n") @@ -135,7 +135,7 @@ https://cloud.vespa.ai/en/reference/vespa-cloud-api#submission-properties Example: `$ mvn package # when adding custom Java components $ vespa prod deploy`, RunE: func(cmd *cobra.Command, args []string) error { - target, err := cli.target(targetOptions{noCertificate: true}) + target, err := cli.target(targetOptions{noCertificate: true, cloudExclusive: true}) if err != nil { return err } diff --git a/client/go/internal/cli/cmd/prod_test.go b/client/go/internal/cli/cmd/prod_test.go index a01056b7178..944f09b3d42 100644 --- a/client/go/internal/cli/cmd/prod_test.go +++ b/client/go/internal/cli/cmd/prod_test.go @@ -44,6 +44,9 @@ func TestProdInit(t *testing.T) { cli, _, _ := newTestCLI(t) cli.Stdin = &buf + assert.Nil(t, cli.Run("config", "set", "target", "cloud")) + assert.Nil(t, cli.Run("config", "set", "application", "foo.bar")) + assert.Nil(t, cli.Run("auth", "api-key")) assert.Nil(t, cli.Run("prod", "init", pkgDir)) // Verify contents diff --git a/client/go/internal/cli/cmd/root.go b/client/go/internal/cli/cmd/root.go index 69fd88c1b2b..c3a3db0df57 100644 --- a/client/go/internal/cli/cmd/root.go +++ b/client/go/internal/cli/cmd/root.go @@ -74,6 +74,8 @@ type targetOptions struct { logLevel string // noCertificate declares that no client certificate should be required when using this target. noCertificate bool + // cloudExclusive specifies whether to only allow Vespa Cloud and Hosted Vespa targets + cloudExclusive bool } type targetType struct { @@ -349,7 +351,7 @@ func (c *CLI) waiter(once bool, timeout time.Duration) *Waiter { // target creates a target according the configuration of this CLI and given opts. func (c *CLI) target(opts targetOptions) (vespa.Target, error) { - targetType, err := c.targetType() + targetType, err := c.targetType(opts.cloudExclusive) if err != nil { return nil, err } @@ -374,7 +376,7 @@ func (c *CLI) target(opts targetOptions) (vespa.Target, error) { } // targetType resolves the real target type and its custom URL (if any) -func (c *CLI) targetType() (targetType, error) { +func (c *CLI) targetType(cloud bool) (targetType, error) { v, err := c.config.targetOrURL() if err != nil { return targetType{}, err @@ -387,6 +389,9 @@ func (c *CLI) targetType() (targetType, error) { return targetType{}, err } } + if cloud && tt.name != vespa.TargetCloud && tt.name != vespa.TargetHosted { + return targetType{}, fmt.Errorf("unsupported target %s: this command only supports targets %s and %s", tt.name, vespa.TargetCloud, vespa.TargetHosted) + } return tt, nil } diff --git a/client/go/internal/cli/cmd/waiter.go b/client/go/internal/cli/cmd/waiter.go index 40d1d76518e..34a10ccce33 100644 --- a/client/go/internal/cli/cmd/waiter.go +++ b/client/go/internal/cli/cmd/waiter.go @@ -35,7 +35,7 @@ func (w *Waiter) DeployService(target vespa.Target) (*vespa.Service, error) { // Service returns the service identified by cluster ID, available on target. func (w *Waiter) Service(target vespa.Target, cluster string) (*vespa.Service, error) { - targetType, err := w.cli.targetType() + targetType, err := w.cli.targetType(false) if err != nil { return nil, err } diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ApplicationClusterEndpoint.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ApplicationClusterEndpoint.java index e70b0f1a599..b7969267328 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ApplicationClusterEndpoint.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ApplicationClusterEndpoint.java @@ -18,6 +18,21 @@ import java.util.stream.Stream; * @author mortent */ public class ApplicationClusterEndpoint { + @Override + public String toString() { + return "ApplicationClusterEndpoint{" + + "dnsName=" + dnsName + + ", scope=" + scope + + ", routingMethod=" + routingMethod + + ", weight=" + weight + + ", hostNames=" + hostNames + + ", clusterId='" + clusterId + "'" + + '}'; + } + + public enum Scope {application, global, zone} + + public enum RoutingMethod {shared, sharedLayer4, exclusive} private final DnsName dnsName; private final Scope scope; @@ -25,16 +40,14 @@ public class ApplicationClusterEndpoint { private final int weight; private final List<String> hostNames; private final String clusterId; - private final AuthMethod authMethod; - private ApplicationClusterEndpoint(DnsName dnsName, Scope scope, RoutingMethod routingMethod, int weight, List<String> hostNames, String clusterId, AuthMethod authMethod) { - this.dnsName = Objects.requireNonNull(dnsName); - this.scope = Objects.requireNonNull(scope); - this.routingMethod = Objects.requireNonNull(routingMethod); + private ApplicationClusterEndpoint(DnsName dnsName, Scope scope, RoutingMethod routingMethod, int weight, List<String> hostNames, String clusterId) { + this.dnsName = dnsName; + this.scope = scope; + this.routingMethod = routingMethod; this.weight = weight; - this.hostNames = List.copyOf(Objects.requireNonNull(hostNames)); - this.clusterId = Objects.requireNonNull(clusterId); - this.authMethod = Objects.requireNonNull(authMethod); + this.hostNames = List.copyOf(hostNames); + this.clusterId = clusterId; } public DnsName dnsName() { @@ -61,33 +74,10 @@ public class ApplicationClusterEndpoint { return clusterId; } - public AuthMethod authMethod() { - return authMethod; - } - - @Override - public String toString() { - return "ApplicationClusterEndpoint{" + - "dnsName=" + dnsName + - ", scope=" + scope + - ", routingMethod=" + routingMethod + - ", weight=" + weight + - ", hostNames=" + hostNames + - ", clusterId='" + clusterId + '\'' + - ", authMethod=" + authMethod + - '}'; - } - public static Builder builder() { return new Builder(); } - public enum Scope { application, global, zone } - - public enum RoutingMethod { shared, sharedLayer4, exclusive } - - public enum AuthMethod { mtls, token } - public static class Builder { private DnsName dnsName; @@ -96,7 +86,6 @@ public class ApplicationClusterEndpoint { private int weigth = 1; private List<String> hosts; private String clusterId; - private AuthMethod authMethod; public Builder dnsName(DnsName name) { this.dnsName = name; @@ -143,15 +132,9 @@ public class ApplicationClusterEndpoint { return this; } - public Builder authMethod(AuthMethod authMethod) { - this.authMethod = authMethod; - return this; - } - public ApplicationClusterEndpoint build() { - return new ApplicationClusterEndpoint(dnsName, scope, routingMethod, weigth, hosts, clusterId, authMethod); + return new ApplicationClusterEndpoint(dnsName, scope, routingMethod, weigth, hosts, clusterId); } - } public static class DnsName implements Comparable<DnsName> { diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ContainerEndpoint.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ContainerEndpoint.java index de06ddd549a..78da750fb5b 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ContainerEndpoint.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ContainerEndpoint.java @@ -3,7 +3,9 @@ package com.yahoo.config.model.api; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.OptionalInt; +import java.util.OptionalLong; /** * ContainerEndpoint tracks the service names that a Container Cluster should be @@ -19,7 +21,6 @@ public class ContainerEndpoint { private final List<String> names; private final OptionalInt weight; private final ApplicationClusterEndpoint.RoutingMethod routingMethod; - private final ApplicationClusterEndpoint.AuthMethod authMethod; public ContainerEndpoint(String clusterId, ApplicationClusterEndpoint.Scope scope, List<String> names) { this(clusterId, scope, names, OptionalInt.empty()); @@ -30,16 +31,11 @@ public class ContainerEndpoint { } public ContainerEndpoint(String clusterId, ApplicationClusterEndpoint.Scope scope, List<String> names, OptionalInt weight, ApplicationClusterEndpoint.RoutingMethod routingMethod) { - this(clusterId, scope, names, weight, routingMethod, ApplicationClusterEndpoint.AuthMethod.mtls); - } - - public ContainerEndpoint(String clusterId, ApplicationClusterEndpoint.Scope scope, List<String> names, OptionalInt weight, ApplicationClusterEndpoint.RoutingMethod routingMethod, ApplicationClusterEndpoint.AuthMethod authMethod) { this.clusterId = Objects.requireNonNull(clusterId); this.scope = Objects.requireNonNull(scope); this.names = List.copyOf(Objects.requireNonNull(names)); - this.weight = Objects.requireNonNull(weight); - this.routingMethod = Objects.requireNonNull(routingMethod); - this.authMethod = Objects.requireNonNull(authMethod); + this.weight = weight; + this.routingMethod = routingMethod; } public String clusterId() { @@ -62,10 +58,6 @@ public class ContainerEndpoint { return routingMethod; } - public ApplicationClusterEndpoint.AuthMethod authMethod() { - return authMethod; - } - @Override public boolean equals(Object o) { if (this == o) return true; @@ -75,17 +67,16 @@ public class ContainerEndpoint { Objects.equals(scope, that.scope) && Objects.equals(names, that.names) && Objects.equals(weight, that.weight) && - Objects.equals(routingMethod, that.routingMethod) && - Objects.equals(authMethod, that.authMethod); + Objects.equals(routingMethod, that.routingMethod); } @Override public int hashCode() { - return Objects.hash(clusterId, names, scope, weight, routingMethod, authMethod); + return Objects.hash(clusterId, names, scope, weight, routingMethod); } @Override public String toString() { - return String.format("container endpoint %s -> %s [scope=%s, weight=%s, routingMethod=%s, authMethod=%s]", clusterId, names, scope, weight, routingMethod, authMethod); + return String.format("container endpoint %s -> %s [scope=%s, weight=%s, routingMetod=%s]", clusterId, names, scope, weight, routingMethod); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java index 07983c7c85a..584207caeac 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java @@ -39,7 +39,6 @@ import com.yahoo.vespa.model.container.component.Handler; import com.yahoo.vespa.model.container.component.SystemBindingPattern; import com.yahoo.vespa.model.container.configserver.ConfigserverCluster; import com.yahoo.vespa.model.utils.FileSender; - import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -49,6 +48,7 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; +import static com.yahoo.config.model.api.ApplicationClusterEndpoint.RoutingMethod.sharedLayer4; import static com.yahoo.vespa.model.container.docproc.DocprocChains.DOCUMENT_TYPE_MANAGER_CLASS; /** @@ -98,7 +98,7 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat private Integer memoryPercentage = null; - private List<ApplicationClusterEndpoint> endpoints = List.of(); + private List<ApplicationClusterEndpoint> endpointList = List.of(); public ApplicationContainerCluster(TreeConfigProducer<?> parent, String configSubId, String clusterId, DeployState deployState) { super(parent, configSubId, clusterId, deployState, true, 10); @@ -132,7 +132,7 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat super.doPrepare(deployState); addAndSendApplicationBundles(deployState); sendUserConfiguredFiles(deployState); - createEndpoints(deployState); + createEndpointList(deployState); } private void addAndSendApplicationBundles(DeployState deployState) { @@ -198,7 +198,7 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat } /** Create list of endpoints, these will be consumed later by LbServicesProducer */ - private void createEndpoints(DeployState deployState) { + private void createEndpointList(DeployState deployState) { if (!deployState.isHosted()) return; if (deployState.getProperties().applicationId().instance().isTester()) return; List<ApplicationClusterEndpoint> endpoints = new ArrayList<>(); @@ -224,26 +224,25 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat .dnsName(l4Name) .hosts(hosts) .clusterId(getName()) - .authMethod(ApplicationClusterEndpoint.AuthMethod.mtls) .build()); } } // Include all endpoints provided by controller endpointsFromController.stream() - .filter(ce -> ce.clusterId().equals(getName())) - .forEach(ce -> ce.names().forEach( - name -> endpoints.add(ApplicationClusterEndpoint.builder() - .scope(ce.scope()) - .weight(ce.weight().orElse(1)) // Default to weight=1 if not set - .routingMethod(ce.routingMethod()) - .dnsName(ApplicationClusterEndpoint.DnsName.from(name)) - .hosts(hosts) - .clusterId(getName()) - .authMethod(ce.authMethod()) - .build()) - )); - this.endpoints = List.copyOf(endpoints); + .filter(ce -> ce.clusterId().equals(getName())) + .filter(ce -> ce.routingMethod() == sharedLayer4) + .forEach(ce -> ce.names().forEach( + name -> endpoints.add(ApplicationClusterEndpoint.builder() + .scope(ce.scope()) + .weight(Long.valueOf(ce.weight().orElse(1)).intValue()) // Default to weight=1 if not set + .routingMethod(ce.routingMethod()) + .dnsName(ApplicationClusterEndpoint.DnsName.from(name)) + .hosts(hosts) + .clusterId(getName()) + .build()) + )); + endpointList = List.copyOf(endpoints); } @Override @@ -365,7 +364,7 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat @Override public List<ApplicationClusterEndpoint> endpoints() { - return endpoints; + return endpointList; } @Override diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java index 894fc55c014..2562e1e3124 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java @@ -439,6 +439,7 @@ public class ContainerClusterTest { cluster.doPrepare(state); List<ApplicationClusterEndpoint> endpoints = cluster.endpoints(); + assertNames(List.of(), endpoints.stream().filter(e -> e.routingMethod() == shared).toList()); assertNames(expectedSharedL4Names, endpoints.stream().filter(e -> e.routingMethod() == sharedLayer4).toList()); List<ContainerEndpoint> endpointsWithWeight = diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ContainerEndpointSerializer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ContainerEndpointSerializer.java index 33f47bbc154..b813d56b345 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ContainerEndpointSerializer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ContainerEndpointSerializer.java @@ -11,7 +11,6 @@ import com.yahoo.slime.SlimeUtils; import java.util.ArrayList; import java.util.List; -import java.util.OptionalInt; /** * Contains all methods for de-/serializing ContainerEndpoints to/from JSON. @@ -33,48 +32,48 @@ public class ContainerEndpointSerializer { private static final String namesField = "names"; private static final String weightField = "weight"; private static final String routingMethodField = "routingMethod"; - private static final String authMethodField = "authMethod"; private ContainerEndpointSerializer() {} public static ContainerEndpoint endpointFromSlime(Inspector inspector) { - String clusterId = inspector.field(clusterIdField).asString(); - String scope = inspector.field(scopeField).asString(); - Inspector namesInspector = inspector.field(namesField); - OptionalInt weight = SlimeUtils.optionalInteger(inspector.field(weightField)); + final var clusterId = inspector.field(clusterIdField).asString(); + final var scope = inspector.field(scopeField).asString(); + final var namesInspector = inspector.field(namesField); + final var weight = SlimeUtils.optionalInteger(inspector.field(weightField)); // assign default routingmethod. Remove when 7.507 is latest version - // Cannot be used before all endpoints are assigned explicit routing method (from controller) - ApplicationClusterEndpoint.RoutingMethod routingMethod = SlimeUtils.optionalString(inspector.field(routingMethodField)) - .map(ContainerEndpointSerializer::routingMethodFrom) - .orElse(ApplicationClusterEndpoint.RoutingMethod.sharedLayer4); - ApplicationClusterEndpoint.AuthMethod authMethod = SlimeUtils.optionalString(inspector.field(authMethodField)) - .map(ContainerEndpointSerializer::authMethodFrom) - .orElse(ApplicationClusterEndpoint.AuthMethod.mtls); + // Cannot be used before all endpoints are assigned explicit routingmethod (from controller) + final var routingMethod = SlimeUtils.optionalString(inspector.field(routingMethodField)).orElse(ApplicationClusterEndpoint.RoutingMethod.sharedLayer4.name()); if (clusterId.isEmpty()) { throw new IllegalStateException("'clusterId' missing on serialized ContainerEndpoint"); } + if (scope.isEmpty()) { throw new IllegalStateException("'scope' missing on serialized ContainerEndpoint"); } - if (!namesInspector.valid()) { + + if (! namesInspector.valid()) { throw new IllegalStateException("'names' missing on serialized ContainerEndpoint"); } - List<String> names = new ArrayList<>(); + if(routingMethod.isEmpty()) { + throw new IllegalStateException("'routingMethod' missing on serialized ContainerEndpoint"); + } + + final var names = new ArrayList<String>(); namesInspector.traverse((ArrayTraverser) (idx, nameInspector) -> { final var containerName = nameInspector.asString(); names.add(containerName); }); - return new ContainerEndpoint(clusterId, scopeFrom(scope), names, weight, routingMethod, authMethod); + return new ContainerEndpoint(clusterId, ApplicationClusterEndpoint.Scope.valueOf(scope), names, weight, + ApplicationClusterEndpoint.RoutingMethod.valueOf(routingMethod)); } public static List<ContainerEndpoint> endpointListFromSlime(Slime slime) { final var inspector = slime.get(); return endpointListFromSlime(inspector); } - public static List<ContainerEndpoint> endpointListFromSlime(Inspector inspector) { final var endpoints = new ArrayList<ContainerEndpoint>(); @@ -89,12 +88,11 @@ public class ContainerEndpointSerializer { public static void endpointToSlime(Cursor cursor, ContainerEndpoint endpoint) { cursor.setString(clusterIdField, endpoint.clusterId()); - cursor.setString(scopeField, asString(endpoint.scope())); + cursor.setString(scopeField, endpoint.scope().name()); endpoint.weight().ifPresent(w -> cursor.setLong(weightField, w)); final var namesInspector = cursor.setArray(namesField); endpoint.names().forEach(namesInspector::addString); - cursor.setString(routingMethodField, asString(endpoint.routingMethod())); - cursor.setString(authMethodField, asString(endpoint.authMethod())); + cursor.setString(routingMethodField, endpoint.routingMethod().name()); } public static Slime endpointListToSlime(List<ContainerEndpoint> endpoints) { @@ -109,53 +107,4 @@ public class ContainerEndpointSerializer { return slime; } - private static ApplicationClusterEndpoint.RoutingMethod routingMethodFrom(String s) { - return switch (s) { - case "shared" -> ApplicationClusterEndpoint.RoutingMethod.shared; - case "sharedLayer4" -> ApplicationClusterEndpoint.RoutingMethod.sharedLayer4; - case "exclusive" -> ApplicationClusterEndpoint.RoutingMethod.exclusive; - default -> throw new IllegalArgumentException("Unknown routing method '" + s + "'"); - }; - } - - private static ApplicationClusterEndpoint.AuthMethod authMethodFrom(String s) { - return switch (s) { - case "mtls" -> ApplicationClusterEndpoint.AuthMethod.mtls; - case "token" -> ApplicationClusterEndpoint.AuthMethod.token; - default -> throw new IllegalArgumentException("Unknown auth method '" + s + "'"); - }; - } - - private static ApplicationClusterEndpoint.Scope scopeFrom(String s) { - return switch (s) { - case "global" -> ApplicationClusterEndpoint.Scope.global; - case "application" -> ApplicationClusterEndpoint.Scope.application; - case "zone" -> ApplicationClusterEndpoint.Scope.zone; - default -> throw new IllegalArgumentException("Unknown scope '" + s + "'"); - }; - } - - private static String asString(ApplicationClusterEndpoint.RoutingMethod routingMethod) { - return switch (routingMethod) { - case shared -> "shared"; - case sharedLayer4 -> "sharedLayer4"; - case exclusive -> "exclusive"; - }; - } - - private static String asString(ApplicationClusterEndpoint.AuthMethod authMethod) { - return switch (authMethod) { - case mtls -> "mtls"; - case token -> "token"; - }; - } - - private static String asString(ApplicationClusterEndpoint.Scope scope) { - return switch (scope) { - case global -> "global"; - case application -> "application"; - case zone -> "zone"; - }; - } - } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/ContainerEndpointSerializerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/ContainerEndpointSerializerTest.java index 5e7fe64f998..c8f31697c5e 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/ContainerEndpointSerializerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/ContainerEndpointSerializerTest.java @@ -8,6 +8,7 @@ import org.junit.Test; import java.util.List; import java.util.OptionalInt; +import java.util.OptionalLong; import static org.junit.Assert.assertEquals; @@ -45,9 +46,11 @@ public class ContainerEndpointSerializerTest { @Test public void writeReadEndpoints() { - List<ContainerEndpoint> endpoints = List.of(new ContainerEndpoint("foo", ApplicationClusterEndpoint.Scope.global, List.of("a", "b"), OptionalInt.of(3), - ApplicationClusterEndpoint.RoutingMethod.shared, ApplicationClusterEndpoint.AuthMethod.token)); - assertEquals(endpoints, ContainerEndpointSerializer.endpointListFromSlime(ContainerEndpointSerializer.endpointListToSlime(endpoints))); + final var endpoints = List.of(new ContainerEndpoint("foo", ApplicationClusterEndpoint.Scope.global, List.of("a", "b"), OptionalInt.of(3), ApplicationClusterEndpoint.RoutingMethod.shared)); + final var serialized = ContainerEndpointSerializer.endpointListToSlime(endpoints); + final var deserialized = ContainerEndpointSerializer.endpointListFromSlime(serialized); + + assertEquals(endpoints, deserialized); } } diff --git a/eval/src/apps/analyze_onnx_model/analyze_onnx_model.cpp b/eval/src/apps/analyze_onnx_model/analyze_onnx_model.cpp index a3aa7cbb32f..03db333d582 100644 --- a/eval/src/apps/analyze_onnx_model/analyze_onnx_model.cpp +++ b/eval/src/apps/analyze_onnx_model/analyze_onnx_model.cpp @@ -9,6 +9,7 @@ #include <vespa/vespalib/util/require.h> #include <vespa/vespalib/util/guard.h> #include <vespa/vespalib/util/stringfmt.h> +#include <charconv> using vespalib::make_string_short::fmt; @@ -20,7 +21,12 @@ using vespalib::FilePointer; using namespace vespalib::eval; using namespace vespalib::eval::test; -struct MyError { +struct MyError : public std::exception { + explicit MyError(vespalib::stringref m) : + std::exception(), + msg(m) + {} + const char * what() const noexcept override { return msg.c_str(); } vespalib::string msg; }; @@ -47,17 +53,42 @@ void extract(const vespalib::string &str, const vespalib::string &prefix, vespal dst = str.substr(pos); } } +struct MemoryUsage { + size_t size; + size_t rss; +}; -void report_memory_usage(const vespalib::string &desc) { - vespalib::string vm_size = "unknown"; - vespalib::string vm_rss = "unknown"; - vespalib::string line; +static const vespalib::string UNKNOWN = "unknown"; + +size_t convert(const vespalib::string & s) { + if (s == UNKNOWN) return 0; + size_t v(0); + size_t end = s.find("kB"); + auto [ptr,ec] = std::from_chars(s.data(), s.data()+std::min(s.size(), end), v, 10); + if (ec != std::errc()) { + throw std::runtime_error(fmt("Bad format : '%s' at '%s'", s.c_str(), ptr)); + } + if (end == vespalib::string::npos) { + throw std::runtime_error(fmt("Bad format : %s", s.c_str())); + } + return v * 1024; +} + +MemoryUsage extract_memory_usage() { + vespalib::string vm_size = UNKNOWN; + vespalib::string vm_rss = UNKNOWN; FilePointer file(fopen("/proc/self/status", "r")); + vespalib::string line; while (read_line(file, line)) { extract(line, "VmSize:", vm_size); extract(line, "VmRSS:", vm_rss); } - fprintf(stderr, "vm_size: %s, vm_rss: %s (%s)\n", vm_size.c_str(), vm_rss.c_str(), desc.c_str()); + return {convert(vm_size), convert(vm_rss)}; +} + +void report_memory_usage(const vespalib::string &desc) { + MemoryUsage vm = extract_memory_usage(); + fprintf(stderr, "vm_size: %zu kB, vm_rss: %zu kB (%s)\n", vm.size/1024, vm.rss/1024, desc.c_str()); } struct Options { @@ -118,7 +149,7 @@ void dump_wire_info(const Onnx::WireInfo &wire) { struct MakeInputType { Options &opts; std::map<vespalib::string,int> symbolic_sizes; - MakeInputType(Options &opts_in) : opts(opts_in), symbolic_sizes() {} + explicit MakeInputType(Options &opts_in) : opts(opts_in), symbolic_sizes() {} ValueType operator()(const Onnx::TensorInfo &info) { int d = 0; std::vector<ValueType::Dimension> dim_list; @@ -229,30 +260,32 @@ int probe_types() { if (!JsonFormat::decode(std_in, params)) { throw MyError{"invalid json"}; } + MemoryUsage vm_before = extract_memory_usage(); Slime result; auto &root = result.setObject(); auto &types = root.setObject("outputs"); - Onnx model(params["model"].asString().make_string(), Onnx::Optimize::DISABLE); + Onnx model(params["model"].asString().make_string(), Onnx::Optimize::ENABLE); Onnx::WirePlanner planner; - for (size_t i = 0; i < model.inputs().size(); ++i) { - auto spec = params["inputs"][model.inputs()[i].name].asString().make_string(); + for (const auto & input : model.inputs()) { + auto spec = params["inputs"][input.name].asString().make_string(); auto input_type = ValueType::from_spec(spec); if (input_type.is_error()) { - if (!params["inputs"][model.inputs()[i].name].valid()) { - throw MyError{fmt("missing type for model input '%s'", - model.inputs()[i].name.c_str())}; + if (!params["inputs"][input.name].valid()) { + throw MyError(fmt("missing type for model input '%s'", input.name.c_str())); } else { - throw MyError{fmt("invalid type for model input '%s': '%s'", - model.inputs()[i].name.c_str(), spec.c_str())}; + throw MyError(fmt("invalid type for model input '%s': '%s'",input.name.c_str(), spec.c_str())); } } - bind_input(planner, model.inputs()[i], input_type); + bind_input(planner, input, input_type); } planner.prepare_output_types(model); for (const auto &output: model.outputs()) { auto output_type = make_output(planner, output); types.setString(output.name, output_type.to_spec()); } + MemoryUsage vm_after = extract_memory_usage(); + root.setLong("vm_size", vm_after.size - vm_before.size); + root.setLong("vm_rss", vm_after.rss - vm_before.rss); write_compact(result, std_out); return 0; } diff --git a/eval/src/tests/instruction/sparse_join_reduce_plan/sparse_join_reduce_plan_test.cpp b/eval/src/tests/instruction/sparse_join_reduce_plan/sparse_join_reduce_plan_test.cpp index e101487ff59..cfc2277278f 100644 --- a/eval/src/tests/instruction/sparse_join_reduce_plan/sparse_join_reduce_plan_test.cpp +++ b/eval/src/tests/instruction/sparse_join_reduce_plan/sparse_join_reduce_plan_test.cpp @@ -40,7 +40,9 @@ struct Event { res_addr.push_back(make_handle(label)); } } - auto operator<=>(const Event &rhs) const = default; + bool operator==(const Event& rhs) const noexcept { + return lhs_idx == rhs.lhs_idx && rhs_idx == rhs.rhs_idx && res_addr == rhs.res_addr; + } }; struct Trace { @@ -55,7 +57,9 @@ struct Trace { events.emplace_back(lhs_idx, rhs_idx, res_addr); return *this; } - auto operator<=>(const Trace &rhs) const = default; + bool operator==(const Trace& rhs) const noexcept { + return estimate == rhs.estimate && events == rhs.events; + } }; std::ostream & diff --git a/eval/src/tests/instruction/universal_dot_product/universal_dot_product_test.cpp b/eval/src/tests/instruction/universal_dot_product/universal_dot_product_test.cpp index 95eb7b406e6..e3393dc2de7 100644 --- a/eval/src/tests/instruction/universal_dot_product/universal_dot_product_test.cpp +++ b/eval/src/tests/instruction/universal_dot_product/universal_dot_product_test.cpp @@ -206,7 +206,7 @@ void benchmark(const vespalib::string &desc, const vespalib::string &expr, std:: for (size_t i = 0; i < ctf_meta.steps.size(); ++i) { auto name = strip_ns(ctf_meta.steps[i].class_name); if (name.find("Inject") > name.size() && name.find("ConstValue") > name.size()) { - fprintf(stderr, " %s: %zu ns\n", name.c_str(), count_ns(min_time[i])); + fprintf(stderr, " %s: %zu ns\n", name.c_str(), (size_t)count_ns(min_time[i])); fprintf(stderr, " +-- %s\n", strip_ns(ctf_meta.steps[i].symbol_name).c_str()); } } diff --git a/eval/src/vespa/eval/onnx/onnx_wrapper.cpp b/eval/src/vespa/eval/onnx/onnx_wrapper.cpp index 8f9450c2660..89d88dcc32c 100644 --- a/eval/src/vespa/eval/onnx/onnx_wrapper.cpp +++ b/eval/src/vespa/eval/onnx/onnx_wrapper.cpp @@ -8,10 +8,6 @@ #include <vespa/vespalib/util/stringfmt.h> #include <vespa/vespalib/util/typify.h> #include <vespa/vespalib/util/classname.h> -#include <assert.h> -#include <cmath> -#include <stdlib.h> -#include <stdio.h> #include <type_traits> #include <vespa/log/log.h> @@ -171,15 +167,15 @@ private: public: OnnxString(const OnnxString &rhs) = delete; OnnxString &operator=(const OnnxString &rhs) = delete; - OnnxString(OnnxString &&rhs) = default; - OnnxString &operator=(OnnxString &&rhs) = default; + OnnxString(OnnxString &&rhs) noexcept = default; + OnnxString &operator=(OnnxString &&rhs) noexcept = default; const char *get() const { return _str.get(); } ~OnnxString() = default; static OnnxString get_input_name(const Ort::Session &session, size_t idx) { - return OnnxString(session.GetInputNameAllocated(idx, _alloc)); + return {session.GetInputNameAllocated(idx, _alloc)}; } static OnnxString get_output_name(const Ort::Session &session, size_t idx) { - return OnnxString(session.GetOutputNameAllocated(idx, _alloc)); + return {session.GetOutputNameAllocated(idx, _alloc)}; } }; Ort::AllocatorWithDefaultOptions OnnxString::_alloc; @@ -216,7 +212,7 @@ Onnx::TensorType get_type_of(const Ort::Value &value) { throw Ort::Exception("[onnx wrapper] actual value has unknown dimension size", ORT_FAIL); } } - return Onnx::TensorType(make_element_type(element_type), shape); + return {make_element_type(element_type), shape}; } std::vector<int64_t> extract_sizes(const ValueType &type) { @@ -306,7 +302,7 @@ Onnx::WirePlanner::do_model_probe(const Onnx &model) result_values.emplace_back(nullptr); } Ort::RunOptions run_opts(nullptr); - Ort::Session &session = const_cast<Ort::Session&>(model._session); + auto &session = const_cast<Ort::Session&>(model._session); session.Run(run_opts, model._input_name_refs.data(), param_values.data(), param_values.size(), model._output_name_refs.data(), result_values.data(), result_values.size()); @@ -554,7 +550,7 @@ Onnx::EvalContext::EvalContext(const Onnx &model, const WireInfo &wire_info) const auto &vespa = _wire_info.vespa_inputs[i]; const auto &onnx = _wire_info.onnx_inputs[i]; if (is_same_type(vespa.cell_type(), onnx.elements)) { - _param_values.push_back(Ort::Value(nullptr)); + _param_values.emplace_back(nullptr); _param_binders.push_back(SelectAdaptParam()(vespa.cell_type())); } else { _param_values.push_back(CreateOnnxTensor()(onnx, _alloc)); @@ -587,7 +583,7 @@ Onnx::EvalContext::bind_param(size_t i, const Value ¶m) void Onnx::EvalContext::eval() { - Ort::Session &session = const_cast<Ort::Session&>(_model._session); + auto &session = const_cast<Ort::Session&>(_model._session); Ort::RunOptions run_opts(nullptr); session.Run(run_opts, _model._input_name_refs.data(), _param_values.data(), _param_values.size(), diff --git a/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp b/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp index d80604919de..28899b40408 100644 --- a/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp +++ b/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp @@ -164,8 +164,8 @@ VerifyRankSetup::~VerifyRankSetup() = default; bool VerifyRankSetup::verify(const search::index::Schema &schema, - const search::fef::Properties &props, - const IRankingAssetsRepo &repo) + const search::fef::Properties &props, + const IRankingAssetsRepo &repo) { proton::matching::IndexEnvironment indexEnv(0, schema, props, repo); search::fef::BlueprintFactory factory; @@ -195,12 +195,12 @@ VerifyRankSetup::verify(const search::index::Schema &schema, bool VerifyRankSetup::verifyConfig(const VerifyRanksetupConfig &myCfg, - const RankProfilesConfig &rankCfg, - const IndexschemaConfig &schemaCfg, - const AttributesConfig &attributeCfg, - const RankingConstantsConfig &constantsCfg, - const RankingExpressionsConfig &expressionsCfg, - const OnnxModelsConfig &modelsCfg) + const RankProfilesConfig &rankCfg, + const IndexschemaConfig &schemaCfg, + const AttributesConfig &attributeCfg, + const RankingConstantsConfig &constantsCfg, + const RankingExpressionsConfig &expressionsCfg, + const OnnxModelsConfig &modelsCfg) { bool ok = true; search::index::Schema schema; diff --git a/searchlib/src/vespa/searchlib/features/onnx_feature.cpp b/searchlib/src/vespa/searchlib/features/onnx_feature.cpp index cdeb0515659..a330a4ff325 100644 --- a/searchlib/src/vespa/searchlib/features/onnx_feature.cpp +++ b/searchlib/src/vespa/searchlib/features/onnx_feature.cpp @@ -132,8 +132,7 @@ OnnxBlueprint::setup(const IIndexEnvironment &env, return fail("model setup failed: %s", ex.what()); } Onnx::WirePlanner planner; - for (size_t i = 0; i < _model->inputs().size(); ++i) { - const auto &model_input = _model->inputs()[i]; + for (const auto & model_input : _model->inputs()) { auto input_feature = model_cfg->input_feature(model_input.name); if (!input_feature.has_value()) { input_feature = fmt("rankingExpression(\"%s\")", normalize_name(model_input.name, "input").c_str()); @@ -151,8 +150,7 @@ OnnxBlueprint::setup(const IIndexEnvironment &env, } } planner.prepare_output_types(*_model); - for (size_t i = 0; i < _model->outputs().size(); ++i) { - const auto &model_output = _model->outputs()[i]; + for (const auto & model_output : _model->outputs()) { auto output_name = model_cfg->output_name(model_output.name); if (!output_name.has_value()) { output_name = normalize_name(model_output.name, "output"); diff --git a/searchlib/src/vespa/searchlib/util/fileutil.cpp b/searchlib/src/vespa/searchlib/util/fileutil.cpp index c2f86224312..f602c66b544 100644 --- a/searchlib/src/vespa/searchlib/util/fileutil.cpp +++ b/searchlib/src/vespa/searchlib/util/fileutil.cpp @@ -36,7 +36,9 @@ LoadedMmap::LoadedMmap(const vespalib::string &fileName) if (sz) { void *tmpBuffer = mmap(nullptr, sz, PROT_READ, MAP_PRIVATE, fd.fd(), 0); if (tmpBuffer != MAP_FAILED) { +#ifdef __linux__ madvise(tmpBuffer, sz, MADV_DONTDUMP); +#endif _mapSize = sz; _mapBuffer = tmpBuffer; uint32_t hl = GenericHeader::getMinSize(); diff --git a/storage/src/tests/distributor/top_level_bucket_db_updater_test.cpp b/storage/src/tests/distributor/top_level_bucket_db_updater_test.cpp index 7eb9dfe6269..074b1492a6e 100644 --- a/storage/src/tests/distributor/top_level_bucket_db_updater_test.cpp +++ b/storage/src/tests/distributor/top_level_bucket_db_updater_test.cpp @@ -829,6 +829,36 @@ TEST_F(TopLevelBucketDBUpdaterTest, storage_node_in_maintenance_clears_buckets_f EXPECT_FALSE(bucket_exists_that_has_node(100, 1)); } +TEST_F(TopLevelBucketDBUpdaterTest, node_removed_from_distribution_config_clears_buckets_for_node) { + ASSERT_NO_FATAL_FAILURE(set_storage_nodes(3)); + enable_distributor_cluster_state("distributor:1 storage:3"); + + for (int i = 1; i < 100; ++i) { + add_ideal_nodes(document::BucketId(16, i)); + } + + EXPECT_TRUE(bucket_exists_that_has_node(100, 1)); + + // Node 1 is removed, 0 and 2 remain + auto distribution_config = "redundancy 2\n" + "group[2]\n" + "group[0].name \"invalid\"\n" + "group[0].index \"invalid\"\n" + "group[0].partitions 1|*\n" + "group[0].nodes[0]\n" + "group[1].name coolnodes\n" + "group[1].index 0\n" + "group[1].nodes[2]\n" + "group[1].nodes[0].index 0\n" + "group[1].nodes[2].index 2\n"; + + set_distribution(distribution_config); + + EXPECT_TRUE( bucket_exists_that_has_node(100, 0)); + EXPECT_FALSE(bucket_exists_that_has_node(100, 1)); + EXPECT_TRUE( bucket_exists_that_has_node(100, 2)); +} + TEST_F(TopLevelBucketDBUpdaterTest, node_down_copies_get_in_sync) { ASSERT_NO_FATAL_FAILURE(set_storage_nodes(3)); document::BucketId bid(16, 1); diff --git a/storage/src/vespa/storage/distributor/activecopy.cpp b/storage/src/vespa/storage/distributor/activecopy.cpp index 4c35d42a0e7..e9d6d8cca30 100644 --- a/storage/src/vespa/storage/distributor/activecopy.cpp +++ b/storage/src/vespa/storage/distributor/activecopy.cpp @@ -132,7 +132,7 @@ ActiveCopy::calculate(const Node2Index & idealState, const lib::Distribution& di { IndexList validNodesWithCopy = buildValidNodeIndexList(e); if (validNodesWithCopy.empty()) { - return ActiveList(); + return {}; } std::vector<IndexList> groups; if (distribution.activePerGroup()) { @@ -162,7 +162,7 @@ ActiveCopy::calculate(const Node2Index & idealState, const lib::Distribution& di } result.emplace_back(*best); } - return ActiveList(std::move(result)); + return {std::move(result)}; } void @@ -170,8 +170,8 @@ ActiveList::print(std::ostream& out, bool verbose, const std::string& indent) co { out << "["; if (verbose) { - for (size_t i=0; i<_v.size(); ++i) { - out << "\n" << indent << " " << _v[i].nodeIndex() << " " << _v[i].getReason(); + for (const auto & copy : _v) { + out << "\n" << indent << " " << copy.nodeIndex() << " " << copy.getReason(); } if (!_v.empty()) { out << "\n" << indent; diff --git a/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.cpp b/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.cpp index 4c8e51908b0..fd747484ccf 100644 --- a/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.cpp +++ b/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.cpp @@ -659,11 +659,14 @@ StripeBucketDBUpdater::MergingNodeRemover::MergingNodeRemover( _cachedDecisionSuperbucket(UINT64_MAX), _cachedOwned(false) { - // TODO intersection of cluster state and distribution config const uint16_t storage_count = s.getNodeCount(lib::NodeType::STORAGE); _available_nodes.resize(storage_count); for (uint16_t i = 0; i < storage_count; ++i) { - if (s.getNodeState(lib::Node(lib::NodeType::STORAGE, i)).getState().oneOf(_upStates)) { + // To be considered available, a given node index must both be marked as available in the + // cluster state AND be present (have a valid node -> group mapping) in the distribution config. + if (s.getNodeState(lib::Node(lib::NodeType::STORAGE, i)).getState().oneOf(_upStates) + && node_is_present_in_config(i)) + { _available_nodes[i] = true; } } @@ -791,6 +794,12 @@ StripeBucketDBUpdater::MergingNodeRemover::storage_node_is_available(uint16_t in return ((index < _available_nodes.size()) && _available_nodes[index]); } +bool +StripeBucketDBUpdater::MergingNodeRemover::node_is_present_in_config(uint16_t node_index) const noexcept +{ + return (_distribution.getNodeGraph().getGroupForNode(node_index) != nullptr); +} + StripeBucketDBUpdater::MergingNodeRemover::~MergingNodeRemover() = default; namespace { 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 b8b729edbeb..2e4ef2a7543 100644 --- a/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h +++ b/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h @@ -199,18 +199,19 @@ private: Result merge(BucketDatabase::Merger&) override; static void logRemove(const document::BucketId& bucketId, const char* msg) ; - bool distributorOwnsBucket(const document::BucketId&) const; + [[nodiscard]] bool distributorOwnsBucket(const document::BucketId&) const; const std::vector<BucketDatabase::Entry>& getNonOwnedEntries() const noexcept { return _nonOwnedBuckets; } - size_t removed_buckets() const noexcept { return _removed_buckets; } - size_t removed_documents() const noexcept { return _removed_documents; } + [[nodiscard]] size_t removed_buckets() const noexcept { return _removed_buckets; } + [[nodiscard]] size_t removed_documents() const noexcept { return _removed_documents; } private: void setCopiesInEntry(BucketDatabase::Entry& e, const std::vector<BucketCopy>& copies) const; - bool has_unavailable_nodes(const BucketDatabase::Entry&) const; - bool storage_node_is_available(uint16_t index) const noexcept; + [[nodiscard]] bool has_unavailable_nodes(const BucketDatabase::Entry&) const; + [[nodiscard]] bool storage_node_is_available(uint16_t index) const noexcept; + [[nodiscard]] bool node_is_present_in_config(uint16_t node_index) const noexcept; const lib::ClusterState _state; std::vector<bool> _available_nodes; diff --git a/vespalib/src/vespa/vespalib/io/mapped_file_input.cpp b/vespalib/src/vespa/vespalib/io/mapped_file_input.cpp index 7f1f0d003b7..95e4a1b496f 100644 --- a/vespalib/src/vespa/vespalib/io/mapped_file_input.cpp +++ b/vespalib/src/vespa/vespalib/io/mapped_file_input.cpp @@ -20,7 +20,9 @@ MappedFileInput::MappedFileInput(const vespalib::string &file_name) if (_data != MAP_FAILED) { _size = info.st_size; madvise(_data, _size, MADV_SEQUENTIAL); +#ifdef __linux__ madvise(_data, _size, MADV_DONTDUMP); +#endif } } } |