diff options
45 files changed, 358 insertions, 656 deletions
diff --git a/application/pom.xml b/application/pom.xml index bb1ea4b30ee..4e5e2f6ba9f 100644 --- a/application/pom.xml +++ b/application/pom.xml @@ -87,6 +87,12 @@ <dependency> <groupId>org.apache.felix</groupId> <artifactId>org.apache.felix.log</artifactId> + <exclusions> + <exclusion> + <groupId>org.osgi</groupId> + <artifactId>*</artifactId> + </exclusion> + </exclusions> </dependency> <dependency> <groupId>org.apache.opennlp</groupId> 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/cloud-tenant-base-dependencies-enforcer/pom.xml b/cloud-tenant-base-dependencies-enforcer/pom.xml index 85f0962555d..13ae492250f 100644 --- a/cloud-tenant-base-dependencies-enforcer/pom.xml +++ b/cloud-tenant-base-dependencies-enforcer/pom.xml @@ -21,6 +21,7 @@ <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-enforcer-plugin</artifactId> + <version>${maven-enforcer-plugin.vespa.version}</version> <dependencies> <dependency> <groupId>com.yahoo.vespa</groupId> @@ -198,8 +199,6 @@ <include>org.junit.vintage:junit-vintage-engine:${junit.vespa.version}:test</include> <include>org.lz4:lz4-java:${org.lz4.vespa.version}:test</include> <include>org.opentest4j:opentest4j:${opentest4j.vespa.version}:test</include> - <include>org.osgi:org.osgi.compendium:[4.1.0, 5):test</include> - <include>org.osgi:org.osgi.core:[4.1.0, 5):test</include> <include>xerces:xercesImpl:${xerces.vespa.version}:test</include> </allowed> </enforceDependencies> 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/container-dependencies-enforcer/pom.xml b/container-dependencies-enforcer/pom.xml index 1e91c1534cd..06e49e34da2 100644 --- a/container-dependencies-enforcer/pom.xml +++ b/container-dependencies-enforcer/pom.xml @@ -202,8 +202,6 @@ <include>org.hdrhistogram:HdrHistogram:${hdrhistogram.vespa.version}:test</include> <include>org.json:json:${org.json.vespa.version}:test</include> <!-- TODO: Remove on Vespa 9 --> <include>org.lz4:lz4-java:${org.lz4.vespa.version}:test</include> - <include>org.osgi:org.osgi.compendium:[4.1.0, 5):test</include> - <include>org.osgi:org.osgi.core:[4.1.0, 5):test</include> <include>xerces:xercesImpl:${xerces.vespa.version}:test</include> </allowed> </enforceDependencies> diff --git a/dependency-versions/pom.xml b/dependency-versions/pom.xml index db38fcc82fa..615d3c15ed1 100644 --- a/dependency-versions/pom.xml +++ b/dependency-versions/pom.xml @@ -88,7 +88,7 @@ <dropwizard.metrics.vespa.version>4.2.19</dropwizard.metrics.vespa.version> <eclipse-collections.vespa.version>11.1.0</eclipse-collections.vespa.version> <felix.vespa.version>7.0.5</felix.vespa.version> - <felix.log.vespa.version>1.0.1</felix.log.vespa.version> + <felix.log.vespa.version>1.3.0</felix.log.vespa.version> <findbugs.vespa.version>3.0.2</findbugs.vespa.version> <!-- Should be kept in sync with guava --> <hamcrest.vespa.version>2.2</hamcrest.vespa.version> <hdrhistogram.vespa.version>2.1.12</hdrhistogram.vespa.version> 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/jdisc_core/pom.xml b/jdisc_core/pom.xml index 879a7fd2950..e8529533320 100644 --- a/jdisc_core/pom.xml +++ b/jdisc_core/pom.xml @@ -100,11 +100,7 @@ <exclusions> <exclusion> <groupId>org.osgi</groupId> - <artifactId>org.osgi.compendium</artifactId> - </exclusion> - <exclusion> - <groupId>org.osgi</groupId> - <artifactId>org.osgi.core</artifactId> + <artifactId>*</artifactId> </exclusion> </exclusions> </dependency> diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ConsoleLogFormatter.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ConsoleLogFormatter.java index 080f3d3f74b..f7f53d304df 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ConsoleLogFormatter.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ConsoleLogFormatter.java @@ -4,7 +4,7 @@ package com.yahoo.jdisc.core; import org.osgi.framework.Bundle; import org.osgi.framework.ServiceReference; import org.osgi.service.log.LogEntry; -import org.osgi.service.log.LogService; +import org.osgi.service.log.LogLevel; import java.io.PrintWriter; import java.io.StringWriter; @@ -86,17 +86,17 @@ class ConsoleLogFormatter { } private StringBuilder formatLevel(LogEntry entry, StringBuilder out) { - switch (entry.getLevel()) { - case LogService.LOG_ERROR: + switch (entry.getLogLevel()) { + case ERROR: out.append("error"); break; - case LogService.LOG_WARNING: + case WARN: out.append("warning"); break; - case LogService.LOG_INFO: + case INFO: out.append("info"); break; - case LogService.LOG_DEBUG: + case DEBUG: out.append("debug"); break; default: @@ -117,7 +117,7 @@ class ConsoleLogFormatter { private StringBuilder formatException(LogEntry entry, StringBuilder out) { Throwable t = entry.getException(); if (t != null) { - if (entry.getLevel() == LogService.LOG_INFO) { + if (entry.getLogLevel() == LogLevel.INFO) { out.append(": "); String msg = t.getMessage(); if (msg != null) { diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ConsoleLogListener.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ConsoleLogListener.java index 2cfa604109b..1d872bbcb64 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ConsoleLogListener.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ConsoleLogListener.java @@ -3,62 +3,64 @@ package com.yahoo.jdisc.core; import com.yahoo.net.HostName; import org.osgi.service.log.LogEntry; +import org.osgi.service.log.LogLevel; import org.osgi.service.log.LogListener; import java.io.PrintStream; +import java.util.Optional; /** * @author Vikas Panwar */ class ConsoleLogListener implements LogListener { - public static final int DEFAULT_LOG_LEVEL = Integer.MAX_VALUE; + public static final LogLevel DEFAULT_LOG_LEVEL = LogLevel.TRACE; private final ConsoleLogFormatter formatter; private final PrintStream out; - private final int maxLevel; + private final LogLevel maxLevel; ConsoleLogListener(PrintStream out, String serviceName, String logLevel) { this.out = out; this.formatter = new ConsoleLogFormatter(getHostname(), getProcessId(), serviceName); - this.maxLevel = parseLogLevel(logLevel); + this.maxLevel = parseLogLevel(logLevel).orElse(null); } @Override public void logged(LogEntry entry) { - if (entry.getLevel() > maxLevel) { + if (maxLevel == null || !maxLevel.implies(entry.getLogLevel())) { return; } out.println(formatter.formatEntry(entry)); } - public static int parseLogLevel(String logLevel) { + public static Optional<LogLevel> parseLogLevel(String logLevel) { if (logLevel == null || logLevel.isEmpty()) { - return DEFAULT_LOG_LEVEL; + return Optional.of(DEFAULT_LOG_LEVEL); } if (logLevel.equalsIgnoreCase("OFF")) { - return Integer.MIN_VALUE; + return Optional.empty(); } if (logLevel.equalsIgnoreCase("ERROR")) { - return 1; + return Optional.of(LogLevel.ERROR); } if (logLevel.equalsIgnoreCase("WARNING")) { - return 2; + return Optional.of(LogLevel.WARN); } if (logLevel.equalsIgnoreCase("INFO")) { - return 3; + return Optional.of(LogLevel.INFO); } if (logLevel.equalsIgnoreCase("DEBUG")) { - return 4; + return Optional.of(LogLevel.DEBUG); } if (logLevel.equalsIgnoreCase("ALL")) { - return Integer.MAX_VALUE; + return Optional.of(LogLevel.TRACE); } try { - return Integer.valueOf(logLevel); - } catch (NumberFormatException e) { + return Optional.of(LogLevel.values()[Integer.parseInt(logLevel)]); + } catch (NumberFormatException | IndexOutOfBoundsException e) { // fall through } - return DEFAULT_LOG_LEVEL; + return Optional.of(DEFAULT_LOG_LEVEL); } public static ConsoleLogListener newInstance() { diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/OsgiLogHandler.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/OsgiLogHandler.java index 48fdb2a0293..989bc26dd85 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/OsgiLogHandler.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/OsgiLogHandler.java @@ -4,6 +4,7 @@ package com.yahoo.jdisc.core; import com.google.common.collect.ImmutableMap; import org.osgi.framework.Bundle; import org.osgi.framework.ServiceReference; +import org.osgi.service.log.LogLevel; import org.osgi.service.log.LogService; import java.util.Dictionary; @@ -45,8 +46,9 @@ class OsgiLogHandler extends Handler { } @Override + @SuppressWarnings("deprecation") public void publish(LogRecord record) { - logService.log(new LogRecordReference(record), toServiceLevel(record.getLevel()), record.getMessage(), + logService.log(new LogRecordReference(record), toServiceLevel(record.getLevel()).ordinal(), record.getMessage(), record.getThrown()); } @@ -60,22 +62,22 @@ class OsgiLogHandler extends Handler { // empty } - public static int toServiceLevel(Level level) { + public static LogLevel toServiceLevel(Level level) { int val = level.intValue(); if (val >= Level.SEVERE.intValue()) { - return LogService.LOG_ERROR; + return LogLevel.ERROR; } if (val >= Level.WARNING.intValue()) { - return LogService.LOG_WARNING; + return LogLevel.WARN; } if (val >= Level.INFO.intValue()) { - return LogService.LOG_INFO; + return LogLevel.INFO; } // Level.CONFIG // Level.FINE // Level.FINER // Level.FINEST - return LogService.LOG_DEBUG; + return LogLevel.DEBUG; } private static <T> Map<String, T> createDictionary(T[] in) { diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/OsgiLogManager.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/OsgiLogManager.java deleted file mode 100644 index 0530d63fe7a..00000000000 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/OsgiLogManager.java +++ /dev/null @@ -1,103 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.jdisc.core; - -import org.osgi.framework.BundleContext; -import org.osgi.framework.ServiceReference; -import org.osgi.service.log.LogService; -import org.osgi.util.tracker.ServiceTracker; -import org.osgi.util.tracker.ServiceTrackerCustomizer; - -import java.util.concurrent.CopyOnWriteArrayList; -import java.util.logging.Handler; -import java.util.logging.Level; -import java.util.logging.Logger; - -/** - * TODO: unused, remove (not public api) - * - * @author Simon Thoresen Hult - */ -class OsgiLogManager implements LogService { - - private static final Object globalLock = new Object(); - private final CopyOnWriteArrayList<LogService> services = new CopyOnWriteArrayList<>(); - private final boolean configureLogLevel; - private ServiceTracker<LogService,LogService> tracker; - - OsgiLogManager(boolean configureLogLevel) { - this.configureLogLevel = configureLogLevel; - } - - public void install(BundleContext osgiContext) { - if (tracker != null) { - throw new IllegalStateException("OsgiLogManager already installed."); - } - tracker = new ServiceTracker<>(osgiContext, LogService.class, new ServiceTrackerCustomizer<>() { - - @Override - public LogService addingService(ServiceReference<LogService> reference) { - LogService service = osgiContext.getService(reference); - services.add(service); - return service; - } - - @Override - public void modifiedService(ServiceReference<LogService> reference, LogService service) { - - } - - @Override - public void removedService(ServiceReference<LogService> reference, LogService service) { - services.remove(service); - } - }); - tracker.open(); - synchronized (globalLock) { - Logger root = Logger.getLogger(""); - if (configureLogLevel) { - root.setLevel(Level.ALL); - } - for (Handler handler : root.getHandlers()) { - root.removeHandler(handler); - } - root.addHandler(new OsgiLogHandler(this)); - } - } - - public boolean uninstall() { - if (tracker == null) { - return false; - } - tracker.close(); // implicitly clears the services array - tracker = null; - return true; - } - - @Override - public void log(int level, String message) { - log(null, level, message, null); - } - - @Override - public void log(int level, String message, Throwable throwable) { - log(null, level, message, throwable); - } - - @SuppressWarnings("rawtypes") - @Override - public void log(ServiceReference serviceRef, int level, String message) { - log(serviceRef, level, message, null); - } - - @SuppressWarnings("rawtypes") - @Override - public void log(ServiceReference serviceRef, int level, String message, Throwable throwable) { - for (LogService obj : services) { - obj.log(serviceRef, level, message, throwable); - } - } - - public static OsgiLogManager newInstance() { - return new OsgiLogManager(System.getProperty("java.util.logging.config.file") == null); - } -} diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ConsoleLogFormatterTestCase.java b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ConsoleLogFormatterTestCase.java index 64130ddc125..4376ccb6c7e 100644 --- a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ConsoleLogFormatterTestCase.java +++ b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ConsoleLogFormatterTestCase.java @@ -6,7 +6,7 @@ import org.mockito.Mockito; import org.osgi.framework.Bundle; import org.osgi.framework.ServiceReference; import org.osgi.service.log.LogEntry; -import org.osgi.service.log.LogService; +import org.osgi.service.log.LogLevel; import java.io.PrintWriter; import java.io.StringWriter; @@ -22,14 +22,14 @@ import static org.junit.jupiter.api.Assertions.assertEquals; public class ConsoleLogFormatterTestCase { private static final ConsoleLogFormatter SIMPLE_FORMATTER = new ConsoleLogFormatter(null, null, null); - private static final LogEntry SIMPLE_ENTRY = new MyEntry(0, 0, null); + private static final LogEntry SIMPLE_ENTRY = new MyEntry(0, LogLevel.AUDIT, null); // TODO: Should (at least) use ConsoleLogFormatter.ABSENCE_REPLACEMENT instead of literal '-'. See ticket 7128315. @Test void requireThatMillisecondsArePadded() { for (int i = 0; i < 10000; ++i) { - LogEntry entry = new MyEntry(i, 0, null); + LogEntry entry = new MyEntry(i, LogLevel.AUDIT, null); Instant instant = Instant.ofEpochMilli(i); assertEquals(String.format("%d.%06d\t-\t-\t-\t-\tunknown\t", instant.getEpochSecond(), instant.getNano() / 1000), SIMPLE_FORMATTER.formatEntry(entry)); @@ -70,7 +70,7 @@ public class ConsoleLogFormatterTestCase { @Test void requireThatProcessIdIncludesThreadIdWhenAvailable() { - LogEntry entry = new MyEntry(0, 0, null).putProperty("THREAD_ID", "threadId"); + LogEntry entry = new MyEntry(0, LogLevel.AUDIT, null).putProperty("THREAD_ID", "threadId"); assertEquals("0.000000\t-\tprocessId/threadId\t-\t-\tunknown\t", new ConsoleLogFormatter(null, "processId", null).formatEntry(entry)); } @@ -93,7 +93,7 @@ public class ConsoleLogFormatterTestCase { @Test void requireThatBundleNameIsIncluded() { - LogEntry entry = new MyEntry(0, 0, null).setBundleSymbolicName("bundleName"); + LogEntry entry = new MyEntry(0, LogLevel.AUDIT, null).setBundleSymbolicName("bundleName"); assertEquals("0.000000\t-\t-\t-\tbundleName\tunknown\t", SIMPLE_FORMATTER.formatEntry(entry)); } @@ -106,7 +106,7 @@ public class ConsoleLogFormatterTestCase { @Test void requireThatLoggerNameIsIncluded() { - LogEntry entry = new MyEntry(0, 0, null).putProperty("LOGGER_NAME", "loggerName"); + LogEntry entry = new MyEntry(0, LogLevel.AUDIT, null).putProperty("LOGGER_NAME", "loggerName"); assertEquals("0.000000\t-\t-\t-\t/loggerName\tunknown\t", SIMPLE_FORMATTER.formatEntry(entry)); } @@ -119,7 +119,7 @@ public class ConsoleLogFormatterTestCase { @Test void requireThatBundleAndLoggerNameIsCombined() { - LogEntry entry = new MyEntry(0, 0, null).setBundleSymbolicName("bundleName") + LogEntry entry = new MyEntry(0, LogLevel.AUDIT, null).setBundleSymbolicName("bundleName") .putProperty("LOGGER_NAME", "loggerName"); assertEquals("0.000000\t-\t-\t-\tbundleName/loggerName\tunknown\t", SIMPLE_FORMATTER.formatEntry(entry)); @@ -129,34 +129,32 @@ public class ConsoleLogFormatterTestCase { void requireThatLevelNameIsIncluded() { ConsoleLogFormatter formatter = SIMPLE_FORMATTER; assertEquals("0.000000\t-\t-\t-\t-\terror\t", - formatter.formatEntry(new MyEntry(0, LogService.LOG_ERROR, null))); + formatter.formatEntry(new MyEntry(0, LogLevel.ERROR, null))); assertEquals("0.000000\t-\t-\t-\t-\twarning\t", - formatter.formatEntry(new MyEntry(0, LogService.LOG_WARNING, null))); + formatter.formatEntry(new MyEntry(0, LogLevel.WARN, null))); assertEquals("0.000000\t-\t-\t-\t-\tinfo\t", - formatter.formatEntry(new MyEntry(0, LogService.LOG_INFO, null))); + formatter.formatEntry(new MyEntry(0, LogLevel.INFO, null))); assertEquals("0.000000\t-\t-\t-\t-\tdebug\t", - formatter.formatEntry(new MyEntry(0, LogService.LOG_DEBUG, null))); - assertEquals("0.000000\t-\t-\t-\t-\tunknown\t", - formatter.formatEntry(new MyEntry(0, 69, null))); + formatter.formatEntry(new MyEntry(0, LogLevel.DEBUG, null))); } @Test void requireThatMessageIsIncluded() { - LogEntry entry = new MyEntry(0, 0, "message"); + LogEntry entry = new MyEntry(0, LogLevel.AUDIT, "message"); assertEquals("0.000000\t-\t-\t-\t-\tunknown\tmessage", SIMPLE_FORMATTER.formatEntry(entry)); } @Test void requireThatMessageIsOptional() { - LogEntry entry = new MyEntry(0, 0, null); + LogEntry entry = new MyEntry(0, LogLevel.AUDIT, null); assertEquals("0.000000\t-\t-\t-\t-\tunknown\t", SIMPLE_FORMATTER.formatEntry(entry)); } @Test void requireThatMessageIsEscaped() { - LogEntry entry = new MyEntry(0, 0, "\\\n\r\t"); + LogEntry entry = new MyEntry(0, LogLevel.AUDIT, "\\\n\r\t"); assertEquals("0.000000\t-\t-\t-\t-\tunknown\t\\\\\\n\\r\\t", SIMPLE_FORMATTER.formatEntry(entry)); } @@ -164,7 +162,7 @@ public class ConsoleLogFormatterTestCase { @Test void requireThatExceptionIsIncluded() { Throwable t = new Throwable(); - LogEntry entry = new MyEntry(0, 0, null).setException(t); + LogEntry entry = new MyEntry(0, LogLevel.AUDIT, null).setException(t); assertEquals("0.000000\t-\t-\t-\t-\tunknown\t\\n" + formatThrowable(t), SIMPLE_FORMATTER.formatEntry(entry)); } @@ -172,7 +170,7 @@ public class ConsoleLogFormatterTestCase { @Test void requireThatExceptionIsEscaped() { Throwable t = new Throwable("\\\n\r\t"); - LogEntry entry = new MyEntry(0, 0, null).setException(t); + LogEntry entry = new MyEntry(0, LogLevel.AUDIT, null).setException(t); assertEquals("0.000000\t-\t-\t-\t-\tunknown\t\\n" + formatThrowable(t), SIMPLE_FORMATTER.formatEntry(entry)); } @@ -180,7 +178,7 @@ public class ConsoleLogFormatterTestCase { @Test void requireThatExceptionIsSimplifiedForInfoEntries() { Throwable t = new Throwable("exception"); - LogEntry entry = new MyEntry(0, LogService.LOG_INFO, "entry").setException(t); + LogEntry entry = new MyEntry(0, LogLevel.INFO, "entry").setException(t); assertEquals("0.000000\t-\t-\t-\t-\tinfo\tentry: exception", SIMPLE_FORMATTER.formatEntry(entry)); } @@ -188,7 +186,7 @@ public class ConsoleLogFormatterTestCase { @Test void requireThatSimplifiedExceptionIsEscaped() { Throwable t = new Throwable("\\\n\r\t"); - LogEntry entry = new MyEntry(0, LogService.LOG_INFO, "entry").setException(t); + LogEntry entry = new MyEntry(0, LogLevel.INFO, "entry").setException(t); assertEquals("0.000000\t-\t-\t-\t-\tinfo\tentry: \\\\\\n\\r\\t", SIMPLE_FORMATTER.formatEntry(entry)); } @@ -196,7 +194,7 @@ public class ConsoleLogFormatterTestCase { @Test void requireThatSimplifiedExceptionMessageIsOptional() { Throwable t = new Throwable(); - LogEntry entry = new MyEntry(0, LogService.LOG_INFO, "entry").setException(t); + LogEntry entry = new MyEntry(0, LogLevel.INFO, "entry").setException(t); assertEquals("0.000000\t-\t-\t-\t-\tinfo\tentry: java.lang.Throwable", SIMPLE_FORMATTER.formatEntry(entry)); } @@ -210,13 +208,13 @@ public class ConsoleLogFormatterTestCase { private static class MyEntry implements LogEntry { final String message; - final int level; + final LogLevel level; final long time; Bundle bundle = null; ServiceReference<?> serviceReference = null; Throwable exception; - MyEntry(long time, int level, String message) { + MyEntry(long time, LogLevel level, String message) { this.message = message; this.level = level; this.time = time; @@ -244,9 +242,15 @@ public class ConsoleLogFormatterTestCase { return time; } - @Override + @Override public LogLevel getLogLevel() { return level; } + @Override public String getLoggerName() { return null; } + @Override public long getSequence() { return 0; } + @Override public String getThreadInfo() { return null; } + @Override public StackTraceElement getLocation() { return null; } + + @Override @SuppressWarnings("deprecation") public int getLevel() { - return level; + return level.ordinal(); } @Override diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ConsoleLogListenerTestCase.java b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ConsoleLogListenerTestCase.java index 0efefc21a2f..88d73f32550 100644 --- a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ConsoleLogListenerTestCase.java +++ b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ConsoleLogListenerTestCase.java @@ -5,11 +5,12 @@ import org.junit.jupiter.api.Test; import org.osgi.framework.Bundle; import org.osgi.framework.ServiceReference; import org.osgi.service.log.LogEntry; +import org.osgi.service.log.LogLevel; import org.osgi.service.log.LogListener; -import org.osgi.service.log.LogService; import java.io.ByteArrayOutputStream; import java.io.PrintStream; +import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -23,42 +24,35 @@ public class ConsoleLogListenerTestCase { @Test void requireThatLogLevelParserKnowsOsgiLogLevels() { - assertEquals(LogService.LOG_ERROR, ConsoleLogListener.parseLogLevel("ERROR")); - assertEquals(LogService.LOG_WARNING, ConsoleLogListener.parseLogLevel("WARNING")); - assertEquals(LogService.LOG_INFO, ConsoleLogListener.parseLogLevel("INFO")); - assertEquals(LogService.LOG_DEBUG, ConsoleLogListener.parseLogLevel("DEBUG")); + assertEquals(LogLevel.ERROR, ConsoleLogListener.parseLogLevel("ERROR").orElseThrow()); + assertEquals(LogLevel.WARN, ConsoleLogListener.parseLogLevel("WARNING").orElseThrow()); + assertEquals(LogLevel.INFO, ConsoleLogListener.parseLogLevel("INFO").orElseThrow()); + assertEquals(LogLevel.DEBUG, ConsoleLogListener.parseLogLevel("DEBUG").orElseThrow()); } @Test void requireThatLogLevelParserKnowsOff() { - assertEquals(Integer.MIN_VALUE, ConsoleLogListener.parseLogLevel("OFF")); + assertEquals(Optional.empty(), ConsoleLogListener.parseLogLevel("OFF")); } @Test void requireThatLogLevelParserKnowsAll() { - assertEquals(Integer.MAX_VALUE, ConsoleLogListener.parseLogLevel("ALL")); - } - - @Test - void requireThatLogLevelParserKnowsIntegers() { - for (int i = -69; i < 69; ++i) { - assertEquals(i, ConsoleLogListener.parseLogLevel(String.valueOf(i))); - } + assertEquals(LogLevel.TRACE, ConsoleLogListener.parseLogLevel("ALL").orElseThrow()); } @Test void requireThatLogLevelParserErrorsReturnDefault() { - assertEquals(ConsoleLogListener.DEFAULT_LOG_LEVEL, ConsoleLogListener.parseLogLevel(null)); - assertEquals(ConsoleLogListener.DEFAULT_LOG_LEVEL, ConsoleLogListener.parseLogLevel("")); - assertEquals(ConsoleLogListener.DEFAULT_LOG_LEVEL, ConsoleLogListener.parseLogLevel("foo")); + assertEquals(ConsoleLogListener.DEFAULT_LOG_LEVEL, ConsoleLogListener.parseLogLevel(null).orElseThrow()); + assertEquals(ConsoleLogListener.DEFAULT_LOG_LEVEL, ConsoleLogListener.parseLogLevel("").orElseThrow()); + assertEquals(ConsoleLogListener.DEFAULT_LOG_LEVEL, ConsoleLogListener.parseLogLevel("foo").orElseThrow()); } @Test void requireThatLogEntryWithLevelAboveThresholdIsNotOutput() { ByteArrayOutputStream out = new ByteArrayOutputStream(); LogListener listener = new ConsoleLogListener(new PrintStream(out), null, "5"); - for (int i = 0; i < 10; ++i) { - listener.logged(new MyEntry(0, i, "message")); + for (LogLevel l : LogLevel.values()) { + listener.logged(new MyEntry(0, l, "message")); } // TODO: Should use ConsoleLogFormatter.ABSENCE_REPLACEMENT instead of literal '-'. See ticket 7128315. assertEquals("0.000000\t" + HOSTNAME + "\t" + PROCESS_ID + "\t-\t-\tunknown\tmessage\n" + @@ -73,10 +67,10 @@ public class ConsoleLogListenerTestCase { private static class MyEntry implements LogEntry { final String message; - final int level; + final LogLevel level; final long time; - MyEntry(long time, int level, String message) { + MyEntry(long time, LogLevel level, String message) { this.message = message; this.level = level; this.time = time; @@ -87,9 +81,15 @@ public class ConsoleLogListenerTestCase { return time; } - @Override + @Override public LogLevel getLogLevel() { return level; } + @Override public String getLoggerName() { return null; } + @Override public long getSequence() { return 0; } + @Override public String getThreadInfo() { return null; } + @Override public StackTraceElement getLocation() { return null; } + + @Override @SuppressWarnings("deprecation") public int getLevel() { - return level; + return level.ordinal(); } @Override diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/core/OsgiLogHandlerTestCase.java b/jdisc_core/src/test/java/com/yahoo/jdisc/core/OsgiLogHandlerTestCase.java index 626cae67c41..f5a86b63ae5 100644 --- a/jdisc_core/src/test/java/com/yahoo/jdisc/core/OsgiLogHandlerTestCase.java +++ b/jdisc_core/src/test/java/com/yahoo/jdisc/core/OsgiLogHandlerTestCase.java @@ -2,7 +2,9 @@ package com.yahoo.jdisc.core; import org.junit.jupiter.api.Test; +import org.osgi.framework.Bundle; import org.osgi.framework.ServiceReference; +import org.osgi.service.log.LogLevel; import org.osgi.service.log.LogService; import java.time.Instant; @@ -30,40 +32,40 @@ public class OsgiLogHandlerTestCase { Logger log = newLogger(logService); log.log(Level.INFO, "foo"); - assertEquals(OsgiLogHandler.toServiceLevel(Level.INFO), logService.lastLevel); + assertEquals(OsgiLogHandler.toServiceLevel(Level.INFO), logService.lastLevel()); assertEquals("foo", logService.lastMessage); assertNull(logService.lastThrowable); Throwable t = new Throwable(); log.log(Level.SEVERE, "bar", t); - assertEquals(OsgiLogHandler.toServiceLevel(Level.SEVERE), logService.lastLevel); + assertEquals(OsgiLogHandler.toServiceLevel(Level.SEVERE), logService.lastLevel()); assertEquals("bar", logService.lastMessage); assertEquals(t, logService.lastThrowable); } @Test void requireThatStadardLogLevelsAreConverted() { - assertLogLevel(LogService.LOG_ERROR, Level.SEVERE); - assertLogLevel(LogService.LOG_WARNING, Level.WARNING); - assertLogLevel(LogService.LOG_INFO, Level.INFO); - assertLogLevel(LogService.LOG_DEBUG, Level.CONFIG); - assertLogLevel(LogService.LOG_DEBUG, Level.FINE); - assertLogLevel(LogService.LOG_DEBUG, Level.FINER); - assertLogLevel(LogService.LOG_DEBUG, Level.FINEST); + assertLogLevel(LogLevel.ERROR, Level.SEVERE); + assertLogLevel(LogLevel.WARN, Level.WARNING); + assertLogLevel(LogLevel.INFO, Level.INFO); + assertLogLevel(LogLevel.DEBUG, Level.CONFIG); + assertLogLevel(LogLevel.DEBUG, Level.FINE); + assertLogLevel(LogLevel.DEBUG, Level.FINER); + assertLogLevel(LogLevel.DEBUG, Level.FINEST); } @Test void requireThatCustomLogLevelsAreConverted() { for (int i = Level.ALL.intValue() - 69; i < Level.OFF.intValue() + 69; ++i) { - int expectedLevel; + LogLevel expectedLevel; if (i >= Level.SEVERE.intValue()) { - expectedLevel = LogService.LOG_ERROR; + expectedLevel = LogLevel.ERROR; } else if (i >= Level.WARNING.intValue()) { - expectedLevel = LogService.LOG_WARNING; + expectedLevel = LogLevel.WARN; } else if (i >= Level.INFO.intValue()) { - expectedLevel = LogService.LOG_INFO; + expectedLevel = LogLevel.INFO; } else { - expectedLevel = LogService.LOG_DEBUG; + expectedLevel = LogLevel.DEBUG; } assertLogLevel(expectedLevel, new MyLogLevel(i)); } @@ -120,11 +122,11 @@ public class OsgiLogHandlerTestCase { assertNull(ref.getProperty("unknown")); } - private static void assertLogLevel(int expectedLevel, Level level) { + private static void assertLogLevel(LogLevel expectedLevel, Level level) { MyLogService logService = new MyLogService(); Logger log = newLogger(logService); log.log(level, "message"); - assertEquals(expectedLevel, logService.lastLevel); + assertEquals(expectedLevel, logService.lastLevel()); } @SuppressWarnings("unchecked") @@ -154,28 +156,36 @@ public class OsgiLogHandlerTestCase { String lastMessage; Throwable lastThrowable; - @Override + LogLevel lastLevel() { return LogLevel.values()[lastLevel]; } + + @Override @SuppressWarnings("deprecation") public void log(int level, String message) { log(null, level, message, null); } - @Override + @Override @SuppressWarnings("deprecation") public void log(int level, String message, Throwable throwable) { log(null, level, message, throwable); } - @Override + @Override @SuppressWarnings("deprecation") public void log(ServiceReference serviceReference, int level, String message) { log(serviceReference, level, message, null); } - @Override + @Override @SuppressWarnings("deprecation") public void log(ServiceReference serviceReference, int level, String message, Throwable throwable) { lastServiceReference = serviceReference; lastLevel = level; lastMessage = message; lastThrowable = throwable; } + + @Override public org.osgi.service.log.Logger getLogger(String s) { return null; } + @Override public org.osgi.service.log.Logger getLogger(Class<?> aClass) { return null; } + @Override public <L extends org.osgi.service.log.Logger> L getLogger(String s, Class<L> aClass) { return null; } + @Override public <L extends org.osgi.service.log.Logger> L getLogger(Class<?> aClass, Class<L> aClass1) { return null; } + @Override public <L extends org.osgi.service.log.Logger> L getLogger(Bundle bundle, String s, Class<L> aClass) { return null; } } private static class MyResourceBundle extends ResourceBundle { diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/core/OsgiLogManagerTestCase.java b/jdisc_core/src/test/java/com/yahoo/jdisc/core/OsgiLogManagerTestCase.java deleted file mode 100644 index 7b5af97ca13..00000000000 --- a/jdisc_core/src/test/java/com/yahoo/jdisc/core/OsgiLogManagerTestCase.java +++ /dev/null @@ -1,185 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.jdisc.core; - -import com.yahoo.jdisc.test.TestDriver; -import org.junit.jupiter.api.Test; -import org.mockito.Mockito; -import org.osgi.framework.BundleContext; -import org.osgi.framework.BundleException; -import org.osgi.framework.ServiceReference; -import org.osgi.framework.ServiceRegistration; -import org.osgi.service.log.LogService; - -import java.util.logging.Level; -import java.util.logging.Logger; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertSame; - - -/** - * @author Simon Thoresen Hult - */ -public class OsgiLogManagerTestCase { - - @Test - void requireThatAllLogMethodsAreImplemented() throws BundleException { - FelixFramework felix = TestDriver.newOsgiFramework(); - felix.start(); - - BundleContext ctx = felix.bundleContext(); - OsgiLogManager manager = new OsgiLogManager(true); - manager.install(ctx); - MyLogService service = new MyLogService(); - ctx.registerService(LogService.class.getName(), service, null); - - manager.log(2, "a"); - assertLast(service, null, 2, "a", null); - - Throwable t1 = new Throwable(); - manager.log(4, "b", t1); - assertLast(service, null, 4, "b", t1); - - ServiceReference<?> ref1 = Mockito.mock(ServiceReference.class); - manager.log(ref1, 8, "c"); - assertLast(service, ref1, 8, "c", null); - - ServiceReference<?> ref2 = Mockito.mock(ServiceReference.class); - Throwable t2 = new Throwable(); - manager.log(ref2, 16, "d", t2); - assertLast(service, ref2, 16, "d", t2); - - manager.uninstall(); - felix.stop(); - } - - @Test - void requireThatLogManagerWritesToAllRegisteredLogServices() throws BundleException { - FelixFramework felix = TestDriver.newOsgiFramework(); - felix.start(); - - BundleContext ctx = felix.bundleContext(); - MyLogService foo = new MyLogService(); - ServiceRegistration<LogService> fooReg = ctx.registerService(LogService.class, foo, null); - - OsgiLogManager manager = new OsgiLogManager(true); - manager.install(ctx); - - ServiceReference<?> ref1 = Mockito.mock(ServiceReference.class); - Throwable t1 = new Throwable(); - manager.log(ref1, 2, "a", t1); - assertLast(foo, ref1, 2, "a", t1); - - MyLogService bar = new MyLogService(); - ServiceRegistration<LogService> barReg = ctx.registerService(LogService.class, bar, null); - - ServiceReference<?> ref2 = Mockito.mock(ServiceReference.class); - Throwable t2 = new Throwable(); - manager.log(ref2, 4, "b", t2); - assertLast(foo, ref2, 4, "b", t2); - assertLast(bar, ref2, 4, "b", t2); - - MyLogService baz = new MyLogService(); - ServiceRegistration<LogService> bazReg = ctx.registerService(LogService.class, baz, null); - - ServiceReference<?> ref3 = Mockito.mock(ServiceReference.class); - Throwable t3 = new Throwable(); - manager.log(ref3, 8, "c", t3); - assertLast(foo, ref3, 8, "c", t3); - assertLast(bar, ref3, 8, "c", t3); - assertLast(baz, ref3, 8, "c", t3); - - fooReg.unregister(); - - ServiceReference<?> ref4 = Mockito.mock(ServiceReference.class); - Throwable t4 = new Throwable(); - manager.log(ref4, 16, "d", t4); - assertLast(foo, ref3, 8, "c", t3); - assertLast(bar, ref4, 16, "d", t4); - assertLast(baz, ref4, 16, "d", t4); - - barReg.unregister(); - - ServiceReference<?> ref5 = Mockito.mock(ServiceReference.class); - Throwable t5 = new Throwable(); - manager.log(ref5, 32, "e", t5); - assertLast(foo, ref3, 8, "c", t3); - assertLast(bar, ref4, 16, "d", t4); - assertLast(baz, ref5, 32, "e", t5); - - bazReg.unregister(); - - ServiceReference<?> ref6 = Mockito.mock(ServiceReference.class); - Throwable t6 = new Throwable(); - manager.log(ref6, 64, "f", t6); - assertLast(foo, ref3, 8, "c", t3); - assertLast(bar, ref4, 16, "d", t4); - assertLast(baz, ref5, 32, "e", t5); - - manager.uninstall(); - felix.stop(); - } - - @Test - void requireThatRootLoggerModificationCanBeDisabled() throws BundleException { - Logger logger = Logger.getLogger(""); - logger.setLevel(Level.WARNING); - - new OsgiLogManager(false).install(Mockito.mock(BundleContext.class)); - assertEquals(Level.WARNING, logger.getLevel()); - - new OsgiLogManager(true).install(Mockito.mock(BundleContext.class)); - assertEquals(Level.ALL, logger.getLevel()); - } - - @Test - void requireThatRootLoggerLevelIsModifiedIfNoLoggerConfigIsGiven() { - Logger logger = Logger.getLogger(""); - logger.setLevel(Level.WARNING); - - OsgiLogManager.newInstance().install(Mockito.mock(BundleContext.class)); - - assertNull(System.getProperty("java.util.logging.config.file")); - assertEquals(Level.ALL, logger.getLevel()); - } - - private static void assertLast(MyLogService service, ServiceReference<?> ref, int level, String message, Throwable t) { - assertSame(ref, service.lastServiceReference); - assertEquals(level, service.lastLevel); - assertEquals(message, service.lastMessage); - assertSame(t, service.lastThrowable); - } - - @SuppressWarnings("rawtypes") - private static class MyLogService implements LogService { - - ServiceReference lastServiceReference; - int lastLevel; - String lastMessage; - Throwable lastThrowable; - - @Override - public void log(int level, String message) { - log(null, level, message, null); - } - - @Override - public void log(int level, String message, Throwable throwable) { - log(null, level, message, throwable); - } - - @Override - public void log(ServiceReference serviceReference, int level, String message) { - log(serviceReference, level, message, null); - } - - @Override - public void log(ServiceReference serviceReference, int level, String message, Throwable throwable) { - lastServiceReference = serviceReference; - lastLevel = level; - lastMessage = message; - lastThrowable = throwable; - } - } -} diff --git a/jdisc_core_test/integration_test/src/test/java/com/yahoo/jdisc/core/OsgiLogManagerIntegrationTest.java b/jdisc_core_test/integration_test/src/test/java/com/yahoo/jdisc/core/OsgiLogManagerIntegrationTest.java deleted file mode 100644 index 629bef6ded3..00000000000 --- a/jdisc_core_test/integration_test/src/test/java/com/yahoo/jdisc/core/OsgiLogManagerIntegrationTest.java +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.jdisc.core; - -import org.junit.Test; -import org.mockito.Mockito; -import org.osgi.framework.BundleContext; - -import java.util.logging.Level; -import java.util.logging.Logger; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; - - -/** - * @author Simon Thoresen Hult - */ -public class OsgiLogManagerIntegrationTest { - - @Test - public void requireThatRootLoggerLevelIsNotModifiedIfLoggerConfigIsGiven() { - Logger logger = Logger.getLogger(""); - logger.setLevel(Level.WARNING); - - OsgiLogManager.newInstance().install(Mockito.mock(BundleContext.class)); - - assertNotNull(System.getProperty("java.util.logging.config.file")); - assertEquals(Level.WARNING, logger.getLevel()); - } -} 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/vespa-dependencies-enforcer/allowed-maven-dependencies.txt b/vespa-dependencies-enforcer/allowed-maven-dependencies.txt index 65ef50c49e5..636228a7672 100644 --- a/vespa-dependencies-enforcer/allowed-maven-dependencies.txt +++ b/vespa-dependencies-enforcer/allowed-maven-dependencies.txt @@ -106,7 +106,7 @@ org.apache.curator:curator-client:5.5.0 org.apache.curator:curator-framework:5.5.0 org.apache.curator:curator-recipes:5.5.0 org.apache.felix:org.apache.felix.framework:7.0.5 -org.apache.felix:org.apache.felix.log:1.0.1 +org.apache.felix:org.apache.felix.log:1.3.0 org.apache.httpcomponents:httpclient:4.5.14 org.apache.httpcomponents:httpcore:4.4.16 org.apache.httpcomponents:httpmime:4.5.14 @@ -197,8 +197,6 @@ org.jvnet.mimepull:mimepull:1.10.0 org.kohsuke:libpam4j:1.11 org.lz4:lz4-java:1.8.0 org.opentest4j:opentest4j:1.3.0 -org.osgi:org.osgi.compendium:4.1.0 -org.osgi:org.osgi.core:4.1.0 org.ow2.asm:asm:9.5 org.ow2.asm:asm-analysis:9.5 org.ow2.asm:asm-commons:9.5 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 } } } |