diff options
113 files changed, 761 insertions, 510 deletions
diff --git a/annotations/src/main/java/com/yahoo/api/annotations/Beta.java b/annotations/src/main/java/com/yahoo/api/annotations/Beta.java new file mode 100644 index 00000000000..c555fbbcb57 --- /dev/null +++ b/annotations/src/main/java/com/yahoo/api/annotations/Beta.java @@ -0,0 +1,23 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.api.annotations; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Signifies that the annotated Java type/method/constructor is under development and may still change before they stabilize. + * Should only be used on a type that part of a package annotated with {@link PublicApi}. + * + * @see <a href="https://docs.vespa.ai/en/vespa-versions.html">https://docs.vespa.ai/en/vespa-versions.html</a> + * + * @author bjorncs + */ +@Retention(RetentionPolicy.CLASS) +@Target({ + ElementType.CONSTRUCTOR, + ElementType.METHOD, + ElementType.TYPE +}) +public @interface Beta {} diff --git a/application/src/main/java/com/yahoo/application/Application.java b/application/src/main/java/com/yahoo/application/Application.java index c3ae7e3770c..1b81897b230 100644 --- a/application/src/main/java/com/yahoo/application/Application.java +++ b/application/src/main/java/com/yahoo/application/Application.java @@ -7,7 +7,7 @@ import ai.vespa.rankingexpression.importer.onnx.OnnxImporter; import ai.vespa.rankingexpression.importer.tensorflow.TensorFlowImporter; import ai.vespa.rankingexpression.importer.vespa.VespaImporter; import ai.vespa.rankingexpression.importer.xgboost.XGBoostImporter; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.application.container.JDisc; import com.yahoo.application.container.impl.StandaloneContainerRunner; import com.yahoo.application.content.ContentCluster; diff --git a/application/src/main/java/com/yahoo/application/ApplicationBuilder.java b/application/src/main/java/com/yahoo/application/ApplicationBuilder.java index 639d546a442..0bfeaea475c 100644 --- a/application/src/main/java/com/yahoo/application/ApplicationBuilder.java +++ b/application/src/main/java/com/yahoo/application/ApplicationBuilder.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.application; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.text.StringUtilities; import com.yahoo.text.Utf8; diff --git a/application/src/main/java/com/yahoo/application/container/DocumentProcessing.java b/application/src/main/java/com/yahoo/application/container/DocumentProcessing.java index 1dcf138f9a9..f86c11d431c 100644 --- a/application/src/main/java/com/yahoo/application/container/DocumentProcessing.java +++ b/application/src/main/java/com/yahoo/application/container/DocumentProcessing.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.application.container; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.component.ComponentSpecification; import com.yahoo.docproc.DocprocExecutor; import com.yahoo.docproc.DocprocService; diff --git a/application/src/main/java/com/yahoo/application/container/JDisc.java b/application/src/main/java/com/yahoo/application/container/JDisc.java index 1b04445d6d5..9370a8f7ee0 100644 --- a/application/src/main/java/com/yahoo/application/container/JDisc.java +++ b/application/src/main/java/com/yahoo/application/container/JDisc.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.application.container; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.google.inject.AbstractModule; import com.google.inject.Module; import com.google.inject.name.Names; diff --git a/application/src/main/java/com/yahoo/application/container/Processing.java b/application/src/main/java/com/yahoo/application/container/Processing.java index 49432f2706a..1f96fe2294b 100644 --- a/application/src/main/java/com/yahoo/application/container/Processing.java +++ b/application/src/main/java/com/yahoo/application/container/Processing.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.application.container; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.google.common.util.concurrent.ListenableFuture; import com.yahoo.component.ComponentSpecification; import com.yahoo.component.chain.Chain; diff --git a/application/src/main/java/com/yahoo/application/container/ProcessingBase.java b/application/src/main/java/com/yahoo/application/container/ProcessingBase.java index 5ee791ca3d1..2b4ea822d03 100644 --- a/application/src/main/java/com/yahoo/application/container/ProcessingBase.java +++ b/application/src/main/java/com/yahoo/application/container/ProcessingBase.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.application.container; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.google.common.util.concurrent.ListenableFuture; import com.yahoo.component.ComponentSpecification; import com.yahoo.component.chain.Chain; diff --git a/application/src/main/java/com/yahoo/application/container/Search.java b/application/src/main/java/com/yahoo/application/container/Search.java index 34e6e30e47a..3535b660b78 100644 --- a/application/src/main/java/com/yahoo/application/container/Search.java +++ b/application/src/main/java/com/yahoo/application/container/Search.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.application.container; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.google.common.util.concurrent.ListenableFuture; import com.yahoo.component.ComponentSpecification; import com.yahoo.component.chain.Chain; diff --git a/application/src/main/java/com/yahoo/application/container/SynchronousRequestResponseHandler.java b/application/src/main/java/com/yahoo/application/container/SynchronousRequestResponseHandler.java index a53a4bdf97c..87d1eff05ab 100644 --- a/application/src/main/java/com/yahoo/application/container/SynchronousRequestResponseHandler.java +++ b/application/src/main/java/com/yahoo/application/container/SynchronousRequestResponseHandler.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.application.container; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.application.container.handler.Request; import com.yahoo.application.container.handler.Response; import com.yahoo.jdisc.handler.CompletionHandler; diff --git a/application/src/main/java/com/yahoo/application/container/handler/Headers.java b/application/src/main/java/com/yahoo/application/container/handler/Headers.java index e5264d08dd5..3e0c148bcc7 100644 --- a/application/src/main/java/com/yahoo/application/container/handler/Headers.java +++ b/application/src/main/java/com/yahoo/application/container/handler/Headers.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.application.container.handler; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.jdisc.HeaderFields; import javax.annotation.concurrent.NotThreadSafe; diff --git a/application/src/main/java/com/yahoo/application/container/handler/Request.java b/application/src/main/java/com/yahoo/application/container/handler/Request.java index 620d6080037..1606af498fa 100644 --- a/application/src/main/java/com/yahoo/application/container/handler/Request.java +++ b/application/src/main/java/com/yahoo/application/container/handler/Request.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.application.container.handler; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import net.jcip.annotations.Immutable; import java.nio.charset.StandardCharsets; diff --git a/application/src/main/java/com/yahoo/application/container/handler/Response.java b/application/src/main/java/com/yahoo/application/container/handler/Response.java index 0fc1261197c..91632068328 100644 --- a/application/src/main/java/com/yahoo/application/container/handler/Response.java +++ b/application/src/main/java/com/yahoo/application/container/handler/Response.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.application.container.handler; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.jdisc.http.HttpHeaders; import com.yahoo.text.Utf8; import net.jcip.annotations.Immutable; diff --git a/application/src/main/java/com/yahoo/application/content/ContentCluster.java b/application/src/main/java/com/yahoo/application/content/ContentCluster.java index ca26971ef93..ee29ee64a9d 100644 --- a/application/src/main/java/com/yahoo/application/content/ContentCluster.java +++ b/application/src/main/java/com/yahoo/application/content/ContentCluster.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.application.content; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import java.nio.file.Path; import java.util.Collections; diff --git a/application/src/test/java/com/yahoo/application/ApplicationFacade.java b/application/src/test/java/com/yahoo/application/ApplicationFacade.java index c84f7fca3f2..aaca14d510b 100644 --- a/application/src/test/java/com/yahoo/application/ApplicationFacade.java +++ b/application/src/test/java/com/yahoo/application/ApplicationFacade.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.application; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.application.container.handler.Request; import com.yahoo.application.container.handler.Response; import com.yahoo.component.Component; diff --git a/client/go/cmd/prod.go b/client/go/cmd/prod.go index 89dc4cb6094..4ca4d20d105 100644 --- a/client/go/cmd/prod.go +++ b/client/go/cmd/prod.go @@ -142,11 +142,10 @@ $ vespa prod submit`, if pkg.TestPath == "" { fatalErrHint(fmt.Errorf("No tests found"), "The application must be a Java maven project, or include basic HTTP tests under src/test/application/", - "See https://cloud.vespa.ai/en/reference/getting-to-production") + "See https://cloud.vespa.ai/en/getting-to-production") return - } else { - verifyTests(pkg.TestPath, target) } + verifyTests(pkg.TestPath, target) isCI := os.Getenv("CI") != "" if !isCI { fmt.Fprintln(stderr, color.Yellow("Warning:"), "We recommend doing this only from a CD job") @@ -352,10 +351,26 @@ func prompt(r *bufio.Reader, question, defaultAnswer string, validator func(inpu } func verifyTests(testsParent string, target vespa.Target) { - runTests(filepath.Join(testsParent, "tests", "system-test"), target, true) - runTests(filepath.Join(testsParent, "tests", "staging-setup"), target, true) - runTests(filepath.Join(testsParent, "tests", "staging-test"), target, true) - if util.PathExists(filepath.Join(testsParent, "tests", "production-test")) { - runTests(filepath.Join(testsParent, "tests", "production-test"), target, true) + verifyTest(testsParent, "system-test", target, true) + verifyTest(testsParent, "staging-setup", target, true) + verifyTest(testsParent, "staging-test", target, true) + verifyTest(testsParent, "production-test", target, false) +} + +func verifyTest(testsParent string, suite string, target vespa.Target, required bool) { + testDirectory := filepath.Join(testsParent, "tests", suite) + _, err := os.Stat(testDirectory) + if err != nil { + if required { + if errors.Is(err, os.ErrNotExist) { + fatalErrHint(fmt.Errorf("No %s tests found", suite), + fmt.Sprintf("No such directory: %s", testDirectory), + "See https://cloud.vespa.ai/en/reference/testing") + } + fatalErrHint(err, "See https://cloud.vespa.ai/en/reference/testing") + } + return } + + runTests(testDirectory, target, true) } diff --git a/client/go/cmd/test.go b/client/go/cmd/test.go index 7ba7a19b235..b8e028ee763 100644 --- a/client/go/cmd/test.go +++ b/client/go/cmd/test.go @@ -17,7 +17,6 @@ import ( "net/http" "net/url" "os" - "path" "path/filepath" "strings" "time" @@ -29,24 +28,20 @@ func init() { } var testCmd = &cobra.Command{ - Use: "test [tests directory or test file]", + Use: "test <tests directory or test file>", Short: "Run a test suite, or a single test", Long: `Run a test suite, or a single test -Runs all JSON test files in the specified directory (the working -directory by default), or the single JSON test file specified. +Runs all JSON test files in the specified directory, or the single JSON test file specified. See https://cloud.vespa.ai/en/reference/testing.html for details.`, Example: `$ vespa test src/test/application/tests/system-test $ vespa test src/test/application/tests/system-test/feed-and-query.json`, - Args: cobra.MaximumNArgs(1), + Args: cobra.ExactArgs(1), DisableAutoGenTag: true, Run: func(cmd *cobra.Command, args []string) { target := getTarget() - testPath := "." - if len(args) > 0 { - testPath = args[0] - } + testPath := args[0] if count, failed := runTests(testPath, target, false); len(failed) != 0 { plural := "s" if count == 1 { @@ -77,10 +72,11 @@ func runTests(rootPath string, target vespa.Target, dryRun bool) (int, []string) if err != nil { fatalErrHint(err, "See https://cloud.vespa.ai/en/reference/testing") } + previousFailed := false for _, test := range tests { if !test.IsDir() && filepath.Ext(test.Name()) == ".json" { - testPath := path.Join(rootPath, test.Name()) + testPath := filepath.Join(rootPath, test.Name()) if previousFailed { fmt.Fprintln(stdout, "") previousFailed = false @@ -122,10 +118,10 @@ func runTest(testPath string, target vespa.Target, dryRun bool) string { testName = filepath.Base(testPath) } if !dryRun { - fmt.Fprintf(stdout, "Running %s:", color.Cyan(testName)) + fmt.Fprintf(stdout, "%s:", testName) } - defaultParameters, err := getParameters(test.Defaults.ParametersRaw, path.Dir(testPath)) + defaultParameters, err := getParameters(test.Defaults.ParametersRaw, filepath.Dir(testPath)) if err != nil { fmt.Fprintln(stderr) fatalErrHint(err, fmt.Sprintf("Invalid default parameters for %s", testName), "See https://cloud.vespa.ai/en/reference/testing") @@ -136,24 +132,24 @@ func runTest(testPath string, target vespa.Target, dryRun bool) string { fatalErrHint(fmt.Errorf("a test must have at least one step, but none were found in %s", testPath), "See https://cloud.vespa.ai/en/reference/testing") } for i, step := range test.Steps { - stepName := step.Name - if stepName == "" { - stepName = fmt.Sprintf("step %d", i+1) + stepName := fmt.Sprintf("Step %d", i+1) + if step.Name != "" { + stepName += ": " + step.Name } - failure, longFailure, err := verify(step, path.Dir(testPath), test.Defaults.Cluster, defaultParameters, target, dryRun) + failure, longFailure, err := verify(step, filepath.Dir(testPath), test.Defaults.Cluster, defaultParameters, target, dryRun) if err != nil { fmt.Fprintln(stderr) fatalErrHint(err, fmt.Sprintf("Error in %s", stepName), "See https://cloud.vespa.ai/en/reference/testing") } if !dryRun { if failure != "" { - fmt.Fprintf(stdout, " %s %s:\n%s\n", color.Red("Failed"), color.Cyan(stepName), longFailure) + fmt.Fprintf(stdout, " %s\n%s:\n%s\n", color.Red("failed"), stepName, longFailure) return fmt.Sprintf("%s: %s: %s", testName, stepName, failure) } if i == 0 { fmt.Fprintf(stdout, " ") } - fmt.Fprint(stdout, color.Green(".")) + fmt.Fprint(stdout, ".") } } if !dryRun { @@ -207,7 +203,7 @@ func verify(step step, testsPath string, defaultCluster string, defaultParameter } externalEndpoint := requestUrl.IsAbs() if !externalEndpoint { - baseURL := "http://dummy/" + baseURL := "" if service != nil { baseURL = service.BaseURL } @@ -267,8 +263,13 @@ func verify(step step, testsPath string, defaultCluster string, defaultParameter defer response.Body.Close() if statusCode != response.StatusCode { - failure := fmt.Sprintf("Unexpected %s: %s", "status code", color.Red(response.StatusCode)) - return failure, fmt.Sprintf("%s\nExpected: %s\nActual response:\n%s", failure, color.Cyan(statusCode), util.ReaderToJSON(response.Body)), nil + return fmt.Sprintf("Unexpected status code: %d", color.Red(response.StatusCode)), + fmt.Sprintf("Unexpected status code\nExpected: %d\nActual: %d\nRequested: %s at %s\nResponse:\n%s", + color.Cyan(statusCode), + color.Red(response.StatusCode), + color.Cyan(method), + color.Cyan(requestUrl), + util.ReaderToJSON(response.Body)), nil } if responseBodySpec == nil { @@ -285,20 +286,24 @@ func verify(step step, testsPath string, defaultCluster string, defaultParameter return "", "", fmt.Errorf("got non-JSON response; %w:\n%s", err, string(responseBodyBytes)) } - failure, expected, err := compare(responseBodySpec, responseBody, "") + failure, expected, actual, err := compare(responseBodySpec, responseBody, "") if failure != "" { responsePretty, _ := json.MarshalIndent(responseBody, "", " ") longFailure := failure if expected != "" { - longFailure += "\n" + expected + longFailure += "\nExpected: " + expected + } + if actual != "" { + failure += ": " + actual + longFailure += "\nActual: " + actual } - longFailure += "\nActual response:\n" + string(responsePretty) + longFailure += fmt.Sprintf("\nRequested: %s at %s\nResponse:\n%s", color.Cyan(method), color.Cyan(requestUrl), string(responsePretty)) return failure, longFailure, err } return "", "", err } -func compare(expected interface{}, actual interface{}, path string) (string, string, error) { +func compare(expected interface{}, actual interface{}, path string) (string, string, string, error) { typeMatch := false valueMatch := false switch u := expected.(type) { @@ -323,18 +328,15 @@ func compare(expected interface{}, actual interface{}, path string) (string, str if ok { if len(u) == len(v) { for i, e := range u { - failure, expected, err := compare(e, v[i], fmt.Sprintf("%s/%d", path, i)) - if failure != "" || err != nil { - return failure, expected, err + if failure, expected, actual, err := compare(e, v[i], fmt.Sprintf("%s/%d", path, i)); failure != "" || err != nil { + return failure, expected, actual, err } } valueMatch = true } else { - return fmt.Sprintf("Unexpected %s at %s: %d", - "number of elements", - color.Cyan(path), - color.Red(len(v))), - fmt.Sprintf("Expected: %d", color.Cyan(len(u))), + return fmt.Sprintf("Unexpected number of elements at %s", color.Cyan(path)), + fmt.Sprintf("%d", color.Cyan(len(u))), + fmt.Sprintf("%d", color.Red(len(v))), nil } } @@ -346,17 +348,16 @@ func compare(expected interface{}, actual interface{}, path string) (string, str childPath := fmt.Sprintf("%s/%s", path, strings.ReplaceAll(strings.ReplaceAll(n, "~", "~0"), "/", "~1")) f, ok := v[n] if !ok { - return fmt.Sprintf("Missing expected field at %s", color.Red(childPath)), "", nil + return fmt.Sprintf("Missing expected field at %s", color.Red(childPath)), "", "", nil } - failure, expected, err := compare(e, f, childPath) - if failure != "" || err != nil { - return failure, expected, err + if failure, expected, actual, err := compare(e, f, childPath); failure != "" || err != nil { + return failure, expected, actual, err } } valueMatch = true } default: - return "", "", fmt.Errorf("unexpected JSON type for value '%v'", expected) + return "", "", "", fmt.Errorf("unexpected JSON type for value '%v'", expected) } if !valueMatch { @@ -369,21 +370,22 @@ func compare(expected interface{}, actual interface{}, path string) (string, str } expectedJson, _ := json.Marshal(expected) actualJson, _ := json.Marshal(actual) - return fmt.Sprintf("Unexpected %s at %s: %s", - mismatched, - color.Cyan(path), - color.Red(actualJson)), - fmt.Sprintf("Expected: %s", color.Cyan(expectedJson)), + return fmt.Sprintf("Unexpected %s at %s", mismatched, color.Cyan(path)), + fmt.Sprintf("%s", color.Cyan(expectedJson)), + fmt.Sprintf("%s", color.Red(actualJson)), nil } - return "", "", nil + return "", "", "", nil } func getParameters(parametersRaw []byte, testsPath string) (map[string]string, error) { if parametersRaw != nil { var parametersPath string if err := json.Unmarshal(parametersRaw, ¶metersPath); err == nil { - resolvedParametersPath := path.Join(testsPath, parametersPath) + if err = validateRelativePath(parametersPath); err != nil { + return nil, err + } + resolvedParametersPath := filepath.Join(testsPath, parametersPath) parametersRaw, err = ioutil.ReadFile(resolvedParametersPath) if err != nil { return nil, fmt.Errorf("failed to read request parameters at %s: %w", resolvedParametersPath, err) @@ -401,7 +403,10 @@ func getParameters(parametersRaw []byte, testsPath string) (map[string]string, e func getBody(bodyRaw []byte, testsPath string) ([]byte, error) { var bodyPath string if err := json.Unmarshal(bodyRaw, &bodyPath); err == nil { - resolvedBodyPath := path.Join(testsPath, bodyPath) + if err = validateRelativePath(bodyPath); err != nil { + return nil, err + } + resolvedBodyPath := filepath.Join(testsPath, bodyPath) bodyRaw, err = ioutil.ReadFile(resolvedBodyPath) if err != nil { return nil, fmt.Errorf("failed to read body file at %s: %w", resolvedBodyPath, err) @@ -410,6 +415,18 @@ func getBody(bodyRaw []byte, testsPath string) ([]byte, error) { return bodyRaw, nil } +func validateRelativePath(relPath string) error { + if filepath.IsAbs(relPath) { + return fmt.Errorf("path must be relative, but was '%s'", relPath) + } + cleanPath := filepath.Clean(relPath) + fmt.Println(cleanPath) + if strings.HasPrefix(cleanPath, "../../../") { + return fmt.Errorf("path may not point outside src/test/application, but '%s' does", relPath) + } + return nil +} + type test struct { Name string `json:"name"` Defaults defaults `json:"defaults"` diff --git a/client/go/cmd/test_test.go b/client/go/cmd/test_test.go index 4c5e4c3f1e5..6649353df77 100644 --- a/client/go/cmd/test_test.go +++ b/client/go/cmd/test_test.go @@ -5,6 +5,7 @@ package cmd import ( + "fmt" "github.com/vespa-engine/vespa/client/go/util" "github.com/vespa-engine/vespa/client/go/vespa" "io/ioutil" @@ -23,29 +24,41 @@ func TestSuite(t *testing.T) { searchResponse, _ := ioutil.ReadFile("testdata/tests/response.json") client.NextStatus(200) client.NextStatus(200) - for i := 0; i < 10; i++ { + for i := 0; i < 11; i++ { client.NextResponse(200, string(searchResponse)) } expectedBytes, _ := ioutil.ReadFile("testdata/tests/expected-suite.out") outBytes, errBytes := execute(command{args: []string{"test", "testdata/tests/system-test"}}, t, client) - assert.Equal(t, string(expectedBytes), outBytes) - assert.Equal(t, "", errBytes) baseUrl := "http://127.0.0.1:8080" urlWithQuery := baseUrl + "/search/?presentation.timing=true&query=artist%3A+foo&timeout=3.4s" requests := []*http.Request{createFeedRequest(baseUrl), createFeedRequest(baseUrl), createSearchRequest(urlWithQuery), createSearchRequest(urlWithQuery)} - for i := 0; i < 8; i++ { + requests = append(requests, createSearchRequest(baseUrl+"/search/")) + requests = append(requests, createSearchRequest(baseUrl+"/search/?foo=%2F")) + for i := 0; i < 7; i++ { requests = append(requests, createSearchRequest(baseUrl+"/search/")) } assertRequests(requests, client, t) + fmt.Println(outBytes) + assert.Equal(t, string(expectedBytes), outBytes) + assert.Equal(t, "", errBytes) +} + +func TestIllegalFileReference(t *testing.T) { + client := &mockHttpClient{} + client.NextStatus(200) + client.NextStatus(200) + _, errBytes := execute(command{args: []string{"test", "testdata/tests/production-test/illegal-reference.json"}}, t, client) + assertRequests([]*http.Request{createRequest("GET", "http://127.0.0.1:8080/search/", "{}")}, client, t) + assert.Equal(t, "\nError: path may not point outside src/test/application, but 'foo/../../../../this-is-not-ok.json' does\nHint: Error in Step 2\nHint: See https://cloud.vespa.ai/en/reference/testing\n", errBytes) } func TestProductionTest(t *testing.T) { client := &mockHttpClient{} client.NextStatus(200) outBytes, errBytes := execute(command{args: []string{"test", "testdata/tests/production-test/external.json"}}, t, client) - assert.Equal(t, "Running external.json: . OK\n\nSuccess: 1 test OK\n", outBytes) + assert.Equal(t, "external.json: . OK\n\nSuccess: 1 test OK\n", outBytes) assert.Equal(t, "", errBytes) assertRequests([]*http.Request{createRequest("GET", "https://my.service:123/path?query=wohoo", "")}, client, t) } diff --git a/client/go/cmd/testdata/empty.json b/client/go/cmd/testdata/empty.json new file mode 100644 index 00000000000..9e26dfeeb6e --- /dev/null +++ b/client/go/cmd/testdata/empty.json @@ -0,0 +1 @@ +{}
\ No newline at end of file diff --git a/client/go/cmd/testdata/tests/expected-suite.out b/client/go/cmd/testdata/tests/expected-suite.out index 963889b8019..df916f50a95 100644 --- a/client/go/cmd/testdata/tests/expected-suite.out +++ b/client/go/cmd/testdata/tests/expected-suite.out @@ -1,8 +1,11 @@ -Running my test: .... OK -Running wrong-bool-value.json: Failed step 1: -Unexpected value at /root/coverage/full: true +My test: .... OK +wrong-bool-value.json: failed +Step 1: +Unexpected value at /root/coverage/full Expected: false -Actual response: +Actual: true +Requested: GET at http://127.0.0.1:8080/search/ +Response: { "root": { "children": [ @@ -38,10 +41,55 @@ Actual response: } } -Running wrong-element-count.json: Failed step 1: -Unexpected number of elements at /root/children: 1 +wrong-code.json: failed +Step 1: +Unexpected status code +Expected: 123 +Actual: 200 +Requested: GET at http://127.0.0.1:8080/search/?foo=%2F +Response: +{ + "root": { + "children": [ + { + "fields": { + "artist": "Foo Fighters", + "documentid": "id:test:music::doc", + "sddocname": "music" + }, + "id": "id:test:music::doc", + "relevance": 0.38186238359951247, + "source": "music" + } + ], + "coverage": { + "coverage": 100, + "documents": 1, + "full": true, + "nodes": 1, + "results": 1, + "resultsFull": 1 + }, + "fields": { + "totalCount": 1 + }, + "id": "toplevel", + "relevance": 1 + }, + "timing": { + "querytime": 0.003, + "searchtime": 0.004, + "summaryfetchtime": 0 + } +} + +wrong-element-count.json: failed +Step 1: +Unexpected number of elements at /root/children Expected: 0 -Actual response: +Actual: 1 +Requested: GET at http://127.0.0.1:8080/search/ +Response: { "root": { "children": [ @@ -77,9 +125,11 @@ Actual response: } } -Running wrong-field-name.json: Failed step 1: +wrong-field-name.json: failed +Step 1: Missing expected field at /root/fields/totalCountDracula -Actual response: +Requested: GET at http://127.0.0.1:8080/search/ +Response: { "root": { "children": [ @@ -115,10 +165,13 @@ Actual response: } } -Running wrong-float-value.json: Failed step 1: -Unexpected value at /root/children/0/relevance: 0.38186238359951247 +wrong-float-value.json: failed +Step 1: +Unexpected value at /root/children/0/relevance Expected: 0.381862373599 -Actual response: +Actual: 0.38186238359951247 +Requested: GET at http://127.0.0.1:8080/search/ +Response: { "root": { "children": [ @@ -154,10 +207,13 @@ Actual response: } } -Running wrong-int-value.json: Failed step 1: -Unexpected value at /root/fields/totalCount: 1 +wrong-int-value.json: failed +Step 1: +Unexpected value at /root/fields/totalCount Expected: 2 -Actual response: +Actual: 1 +Requested: GET at http://127.0.0.1:8080/search/ +Response: { "root": { "children": [ @@ -193,9 +249,11 @@ Actual response: } } -Running wrong-null-value.json: Failed step 1: +wrong-null-value.json: failed +Step 1: Missing expected field at /boot -Actual response: +Requested: GET at http://127.0.0.1:8080/search/ +Response: { "root": { "children": [ @@ -231,10 +289,13 @@ Actual response: } } -Running wrong-string-value.json: Failed step 1: -Unexpected value at /root/children/0/fields/artist: "Foo Fighters" +wrong-string-value.json: failed +Step 1: +Unexpected value at /root/children/0/fields/artist Expected: "Boo Fighters" -Actual response: +Actual: "Foo Fighters" +Requested: GET at http://127.0.0.1:8080/search/ +Response: { "root": { "children": [ @@ -270,10 +331,13 @@ Actual response: } } -Running wrong-type.json: Failed step 1: -Unexpected type at /root/fields/totalCount: 1 +wrong-type.json: failed +Step 1: +Unexpected type at /root/fields/totalCount Expected: "1" -Actual response: +Actual: 1 +Requested: GET at http://127.0.0.1:8080/search/ +Response: { "root": { "children": [ @@ -309,12 +373,13 @@ Actual response: } } -Failure: 8 of 9 tests failed: -wrong-bool-value.json: step 1: Unexpected value at /root/coverage/full: true -wrong-element-count.json: step 1: Unexpected number of elements at /root/children: 1 -wrong-field-name.json: step 1: Missing expected field at /root/fields/totalCountDracula -wrong-float-value.json: step 1: Unexpected value at /root/children/0/relevance: 0.38186238359951247 -wrong-int-value.json: step 1: Unexpected value at /root/fields/totalCount: 1 -wrong-null-value.json: step 1: Missing expected field at /boot -wrong-string-value.json: step 1: Unexpected value at /root/children/0/fields/artist: "Foo Fighters" -wrong-type.json: step 1: Unexpected type at /root/fields/totalCount: 1 +Failure: 9 of 10 tests failed: +wrong-bool-value.json: Step 1: Unexpected value at /root/coverage/full: true +wrong-code.json: Step 1: Unexpected status code: 200 +wrong-element-count.json: Step 1: Unexpected number of elements at /root/children: 1 +wrong-field-name.json: Step 1: Missing expected field at /root/fields/totalCountDracula +wrong-float-value.json: Step 1: Unexpected value at /root/children/0/relevance: 0.38186238359951247 +wrong-int-value.json: Step 1: Unexpected value at /root/fields/totalCount: 1 +wrong-null-value.json: Step 1: Missing expected field at /boot +wrong-string-value.json: Step 1: Unexpected value at /root/children/0/fields/artist: "Foo Fighters" +wrong-type.json: Step 1: Unexpected type at /root/fields/totalCount: 1 diff --git a/client/go/cmd/testdata/tests/expected.out b/client/go/cmd/testdata/tests/expected.out index 084fb10f72a..2ca35fe6a37 100644 --- a/client/go/cmd/testdata/tests/expected.out +++ b/client/go/cmd/testdata/tests/expected.out @@ -1,3 +1,3 @@ -Running my test: .... OK +My test: .... OK Success: 1 test OK diff --git a/client/go/cmd/testdata/tests/production-test/illegal-reference.json b/client/go/cmd/testdata/tests/production-test/illegal-reference.json new file mode 100644 index 00000000000..edd8a2fafeb --- /dev/null +++ b/client/go/cmd/testdata/tests/production-test/illegal-reference.json @@ -0,0 +1,14 @@ +{ + "steps": [ + { + "request": { + "body": "foo/../../../empty.json" + } + }, + { + "request": { + "body": "foo/../../../../this-is-not-ok.json" + } + } + ] +}
\ No newline at end of file diff --git a/client/go/cmd/testdata/tests/system-test/test.json b/client/go/cmd/testdata/tests/system-test/test.json index f53df929dbd..2e327b5e5df 100644 --- a/client/go/cmd/testdata/tests/system-test/test.json +++ b/client/go/cmd/testdata/tests/system-test/test.json @@ -1,5 +1,5 @@ { - "name": "my test", + "name": "My test", "defaults": { "cluster": "container", "parameters": { diff --git a/client/go/cmd/testdata/tests/system-test/wrong-code.json b/client/go/cmd/testdata/tests/system-test/wrong-code.json new file mode 100644 index 00000000000..c325054faa1 --- /dev/null +++ b/client/go/cmd/testdata/tests/system-test/wrong-code.json @@ -0,0 +1,14 @@ +{ + "steps": [ + { + "request": { + "parameters": { + "foo": "/" + } + }, + "response": { + "code": 123 + } + } + ] +} diff --git a/config-model/src/main/java/com/yahoo/config/model/producer/AbstractConfigProducer.java b/config-model/src/main/java/com/yahoo/config/model/producer/AbstractConfigProducer.java index 7c9c6292999..47a5fb24a43 100644 --- a/config-model/src/main/java/com/yahoo/config/model/producer/AbstractConfigProducer.java +++ b/config-model/src/main/java/com/yahoo/config/model/producer/AbstractConfigProducer.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.model.producer; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.config.ConfigInstance; import com.yahoo.config.model.ApplicationConfigProducerRoot; import com.yahoo.config.model.deploy.DeployState; diff --git a/config-model/src/main/java/com/yahoo/config/model/test/TestDriver.java b/config-model/src/main/java/com/yahoo/config/model/test/TestDriver.java index 6fe56337a0d..fd98d21dcd5 100644 --- a/config-model/src/main/java/com/yahoo/config/model/test/TestDriver.java +++ b/config-model/src/main/java/com/yahoo/config/model/test/TestDriver.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.model.test; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.component.Version; import com.yahoo.config.model.MapConfigModelRegistry; import com.yahoo.config.application.api.ApplicationPackage; diff --git a/config-model/src/main/java/com/yahoo/config/model/test/TestRoot.java b/config-model/src/main/java/com/yahoo/config/model/test/TestRoot.java index 06e483c102b..c1fd8e4646d 100644 --- a/config-model/src/main/java/com/yahoo/config/model/test/TestRoot.java +++ b/config-model/src/main/java/com/yahoo/config/model/test/TestRoot.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.model.test; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.config.ConfigInstance; import com.yahoo.config.model.ConfigModel; import com.yahoo.vespa.model.HostResource; diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java index ec446e27670..08ae3d838ec 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java @@ -6,7 +6,7 @@ import com.yahoo.searchdefinition.document.SDDocumentType; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; @@ -22,18 +22,15 @@ import java.util.Set; * * @author Ulf Lilleengen */ +// TODO: These should be stored in each schema as everything else public class RankProfileRegistry { private final Map<String, Map<String, RankProfile>> rankProfiles = new LinkedHashMap<>(); - private static final String MAGIC_GLOBAL_RANKPROFILES = "[MAGIC_GLOBAL_RANKPROFILES]"; + private static final String globalRankProfilesKey = "[global]"; /* These rank profiles can be overridden: 'default' rank profile, as that is documented to work. And 'unranked'. */ static final Set<String> overridableRankProfileNames = new HashSet<>(Arrays.asList("default", "unranked")); - public RankProfileRegistry() { - - } - public static RankProfileRegistry createRankProfileRegistryWithBuiltinRankProfiles(Schema schema) { RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); rankProfileRegistry.add(new DefaultRankProfile(schema, rankProfileRegistry, schema.rankingConstants())); @@ -42,14 +39,10 @@ public class RankProfileRegistry { } private String extractName(ImmutableSchema search) { - return search != null ? search.getName() : MAGIC_GLOBAL_RANKPROFILES; + return search != null ? search.getName() : globalRankProfilesKey; } - /** - * Adds a rank profile to this registry - * - * @param rankProfile the rank profile to add - */ + /** Adds a rank profile to this registry */ public void add(RankProfile rankProfile) { String searchName = extractName(rankProfile.getSearch()); if ( ! rankProfiles.containsKey(searchName)) { @@ -91,7 +84,7 @@ public class RankProfileRegistry { } public RankProfile getGlobal(String name) { - Map<String, RankProfile> profiles = rankProfiles.get(MAGIC_GLOBAL_RANKPROFILES); + Map<String, RankProfile> profiles = rankProfiles.get(globalRankProfilesKey); if (profiles == null) return null; return profiles.get(name); } @@ -103,12 +96,13 @@ public class RankProfileRegistry { RankProfile parentProfile = resolve(parent, name); if (parentProfile != null) return parentProfile; } - return get(MAGIC_GLOBAL_RANKPROFILES, name); + return get(globalRankProfilesKey, name); } /** * Rank profiles that are collected across clusters. - * @return A set of global {@link RankProfile} instances. + * + * @return a set of global {@link RankProfile} instances */ public Collection<RankProfile> all() { List<RankProfile> all = new ArrayList<>(); @@ -119,26 +113,28 @@ public class RankProfileRegistry { } /** - * Returns the rank profiles of a given search definition. + * Retrieve all rank profiles for a schema * - * @param search the searchdefinition to get rank profiles for + * @param schema the schema to fetch rank profiles for, or null for the global ones * @return a collection of {@link RankProfile} instances */ - public Collection<RankProfile> rankProfilesOf(String search) { - Map<String, RankProfile> mapping = rankProfiles.get(search); - if (mapping == null) { - return Collections.emptyList(); + public Collection<RankProfile> rankProfilesOf(ImmutableSchema schema) { + String key = schema == null ? globalRankProfilesKey : schema.getName(); + + if ( ! rankProfiles.containsKey(key)) return List.of(); + + var profiles = new LinkedHashMap<>(rankProfiles.get(key)); + // Add all profiles in inherited schemas, unless they are already present (overridden) + while (schema != null && schema.inherited().isPresent()) { + schema = schema.inherited().get(); + var inheritedProfiles = rankProfiles.get(schema.getName()); + if (inheritedProfiles != null) { + for (Map.Entry<String, RankProfile> inheritedProfile : inheritedProfiles.entrySet()) { + profiles.putIfAbsent(inheritedProfile.getKey(), inheritedProfile.getValue()); + } + } } - return mapping.values(); - } - - /** - * Retrieve all rank profiles for a search definition - * @param search search definition to fetch rank profiles for, or null for the global ones - * @return Collection of RankProfiles - */ - public Collection<RankProfile> rankProfilesOf(ImmutableSchema search) { - return rankProfilesOf(extractName(search)); + return profiles.values(); } } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java b/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java index c7a7ecd1d08..ddad67324ba 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java @@ -344,11 +344,12 @@ public class Schema implements ImmutableSchema { } /** - * Returns a field defined in one of the documents of this search definition. This does <b>not</b> include the extra - * fields defined outside of a document (those accessible through the getExtraField() method). + * Returns a field defined in one of the documents of this search definition. + * This does not include the extra fields defined outside the document + * (those accessible through the getExtraField() method). * - * @param name The name of the field to return. - * @return The named field, or null if not found. + * @param name the name of the field to return + * @return the named field, or null if not found */ public SDField getDocumentField(String name) { return (SDField) documentType.getField(name); @@ -458,7 +459,7 @@ public class Schema implements ImmutableSchema { /** * Consolidates a set of index settings for the same index into one * - * @param indices The list of indexes to consolidate + * @param indices the list of indexes to consolidate * @return the consolidated index */ private Index consolidateIndices(List<Index> indices) { @@ -477,13 +478,10 @@ public class Schema implements ImmutableSchema { if (consolidated.getRankType() == null) { consolidated.setRankType(current.getRankType()); } else { - if (current.getRankType() != null && - !consolidated.getRankType().equals(current.getRankType())) - { + if (current.getRankType() != null && consolidated.getRankType() != current.getRankType()) deployLogger.logApplicationPackage(Level.WARNING, "Conflicting rank type settings for " + first.getName() + " in " + this + ", using " + consolidated.getRankType()); - } } for (Iterator<String> j = current.aliasIterator(); j.hasNext();) { @@ -505,11 +503,8 @@ public class Schema implements ImmutableSchema { } } - for (ImmutableSDField field : allConcreteFields()) { - for (Index index : field.getIndices().values()) { - allIndices.add(index); - } - } + for (ImmutableSDField field : allConcreteFields()) + allIndices.addAll(field.getIndices().values()); return Collections.unmodifiableList(allIndices); } @@ -618,17 +613,7 @@ public class Schema implements ImmutableSchema { return summaryFields; } - @Override - public int hashCode() { - return name.hashCode(); - } - - /** - * Returns the first occurrence of an attribute having this name, or null if none - * - * @param name Name of attribute - * @return The Attribute with given name. - */ + /** Returns the first occurrence of an attribute having this name, or null if none */ public Attribute getAttribute(String name) { for (ImmutableSDField field : allConcreteFields()) { Attribute attribute = field.getAttributes().get(name); @@ -650,33 +635,29 @@ public class Schema implements ImmutableSchema { } @Override + public int hashCode() { + return name.hashCode(); + } + + @Override public String toString() { return "schema '" + getName() + "'"; } public boolean isAccessingDiskSummary(SummaryField field) { - if (!field.getTransform().isInMemory()) { - return true; - } - if (field.getSources().size() == 0) { - return isAccessingDiskSummary(getName()); - } + if (!field.getTransform().isInMemory()) return true; + if (field.getSources().size() == 0) return isAccessingDiskSummary(getName()); for (SummaryField.Source source : field.getSources()) { - if (isAccessingDiskSummary(source.getName())) { + if (isAccessingDiskSummary(source.getName())) return true; - } } return false; } private boolean isAccessingDiskSummary(String source) { SDField field = getConcreteField(source); - if (field == null) { - return false; - } - if (field.doesSummarying() && !field.doesAttributing()) { - return true; - } + if (field == null) return false; + if (field.doesSummarying() && !field.doesAttributing()) return true; return false; } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/DiversitySettingsValidator.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/DiversitySettingsValidator.java index 40de25dbc76..5643bb660f1 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/DiversitySettingsValidator.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/DiversitySettingsValidator.java @@ -22,7 +22,7 @@ public class DiversitySettingsValidator extends Processor { if ( ! validate) return; if (documentsOnly) return; - for (RankProfile rankProfile : rankProfileRegistry.rankProfilesOf(schema.getName())) { + for (RankProfile rankProfile : rankProfileRegistry.rankProfilesOf(schema)) { if (rankProfile.getMatchPhaseSettings() != null && rankProfile.getMatchPhaseSettings().getDiversity() != null) { validate(rankProfile, rankProfile.getMatchPhaseSettings().getDiversity()); } diff --git a/config-model/src/test/derived/schemainheritance/rank-profiles.cfg b/config-model/src/test/derived/schemainheritance/rank-profiles.cfg index 9e68045fab0..1e2ed46e696 100644 --- a/config-model/src/test/derived/schemainheritance/rank-profiles.cfg +++ b/config-model/src/test/derived/schemainheritance/rank-profiles.cfg @@ -9,3 +9,4 @@ rankprofile[].fef.property[].value "0" rankprofile[].fef.property[].name "vespa.dump.ignoredefaultfeatures" rankprofile[].fef.property[].value "true" rankprofile[].name "child_profile" +rankprofile[].name "parent_profile" diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java index 958a37e1432..182b924e877 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java @@ -58,8 +58,8 @@ public final class Capacity { */ public NodeType type() { return type; } - public Capacity withLimits(ClusterResources min, ClusterResources max) { - return new Capacity(min, max, required, canFail, type); + public Capacity withGroups(int groups) { + return new Capacity(min.withGroups(groups), max.withGroups(groups), required, canFail, type); } @Override diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Deployer.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Deployer.java index e7d28a3f65b..209f339f51f 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Deployer.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Deployer.java @@ -16,9 +16,7 @@ public interface Deployer { * Creates a new deployment from the active application, if available. Will use the default timeout for deployment. * * @param application the active application to be redeployed - * @return a new deployment from the local active, or empty if a local active application - * was not present for this id (meaning it either is not active or deployed at another - * node in the config server cluster) + * @return a new deployment from the active application, or empty if application does not exist */ default Optional<Deployment> deployFromLocalActive(ApplicationId application) { return deployFromLocalActive(application, false); @@ -29,9 +27,7 @@ public interface Deployer { * * @param application the active application to be redeployed * @param bootstrap the deployment is done when bootstrapping - * @return a new deployment from the local active, or empty if a local active application - * was not present for this id (meaning it either is not active or deployed at another - * node in the config server cluster) + * @return a new deployment from the active application, or empty if application does not exist */ Optional<Deployment> deployFromLocalActive(ApplicationId application, boolean bootstrap); @@ -41,9 +37,7 @@ public interface Deployer { * * @param application the active application to be redeployed * @param timeout the timeout to use for each individual deployment operation - * @return a new deployment from the local active, or empty if a local active application - * was not present for this id (meaning it either is not active or active on another - * node in the config server cluster) + * @return a new deployment from the active application, or empty if application does not exist */ default Optional<Deployment> deployFromLocalActive(ApplicationId application, Duration timeout) { return deployFromLocalActive(application, timeout, false); @@ -56,9 +50,7 @@ public interface Deployer { * @param application the active application to be redeployed * @param timeout the timeout to use for each individual deployment operation * @param bootstrap the deployment is done when bootstrapping - * @return a new deployment from the local active, or empty if a local active application - * was not present for this id (meaning it either is not active or active on another - * node in the config server cluster) + * @return a new deployment from the active application, or empty if application does not exist */ Optional<Deployment> deployFromLocalActive(ApplicationId application, Duration timeout, boolean bootstrap); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java index 79337f3d32b..5f3999a9fdf 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java @@ -131,6 +131,7 @@ public class FileServer { } catch (IOException e) { errorDescription = "For" + reference.value() + ": failed reading file '" + file.getAbsolutePath() + "'"; log.warning(errorDescription + " for sending to '" + target.toString() + "'. " + e.toString()); + fileData.close(); } try { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v1/RoutingStatusApiHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v1/RoutingStatusApiHandler.java index 957868f2abb..48f23a1f7bd 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v1/RoutingStatusApiHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v1/RoutingStatusApiHandler.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.config.server.http.v1; import com.google.inject.Inject; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Deployer; +import com.yahoo.config.provision.Deployment; import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.path.Path; import com.yahoo.restapi.RestApi; @@ -112,16 +113,24 @@ public class RoutingStatusApiHandler extends RestApiRequestHandler<RoutingStatus RestApi.RequestContext.RequestContent requestContent = context.requestContentOrThrow(); Slime requestBody = Exceptions.uncheck(() -> SlimeUtils.jsonToSlime(requestContent.content().readAllBytes())); DeploymentRoutingStatus wantedStatus = deploymentRoutingStatusFromSlime(requestBody, clock.instant()); - DeploymentRoutingStatus currentStatus = deploymentStatus(upstreamNames.iterator().next()); - + List<DeploymentRoutingStatus> currentStatuses = upstreamNames.stream() + .map(this::deploymentStatus) + .collect(Collectors.toList()); + DeploymentRoutingStatus currentStatus = currentStatuses.get(0); // Redeploy application so that a new LbServicesConfig containing the updated status is generated and consumed - // by routing layer. This is required to update weights for application endpoints when routing status for a - // deployment is changed + // by routing layer. This is required to update status of upstreams in application endpoints + log.log(Level.INFO, "Changing routing status of " + instance + " from " + + currentStatus.status() + " to " + wantedStatus.status()); + boolean needsChange = currentStatuses.stream().anyMatch(status -> status.status() != wantedStatus.status()); + if (!needsChange) { + return new SlimeJsonResponse(toSlime(wantedStatus)); + } changeStatus(upstreamNames, wantedStatus); try { - deployer.deployFromLocalActive(instance, Duration.ofMinutes(1)); + Optional<Deployment> deployment = deployer.deployFromLocalActive(instance, Duration.ofMinutes(1)); + if (deployment.isEmpty()) throw new IllegalArgumentException("No deployment of " + instance + " found"); + deployment.get().activate(); } catch (Exception e) { - log.log(Level.SEVERE, "Failed to redeploy " + instance + ". Reverting routing status to " + currentStatus.status(), e); changeStatus(upstreamNames, currentStatus); @@ -138,6 +147,8 @@ public class RoutingStatusApiHandler extends RestApiRequestHandler<RoutingStatus /** Change routing status of a zone */ private SlimeJsonResponse changeZoneStatus(RestApi.RequestContext context) { boolean in = context.request().getMethod() == HttpRequest.Method.DELETE; + log.log(Level.INFO, "Changing routing status of zone from " + zoneStatus() + " to " + + (in ? RoutingStatus.in : RoutingStatus.out)); if (in) { curator.delete(ZONE_STATUS); return new SlimeJsonResponse(toSlime(RoutingStatus.in)); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v1/RoutingStatusApiHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v1/RoutingStatusApiHandlerTest.java index d16030767d5..8dd7cf4d6fc 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v1/RoutingStatusApiHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v1/RoutingStatusApiHandlerTest.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.config.server.http.v1; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Deployer; import com.yahoo.config.provision.Deployment; +import com.yahoo.config.provision.HostFilter; import com.yahoo.container.jdisc.HttpRequestBuilder; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.jdisc.http.HttpRequest.Method; @@ -197,8 +198,19 @@ public class RoutingStatusApiHandlerTest { if (failNextDeployment) { throw new RuntimeException("Deployment failed"); } - lastDeployed.put(application, clock.instant()); - return Optional.empty(); + return Optional.of(new Deployment() { + @Override + public void prepare() {} + + @Override + public long activate() { + lastDeployed.put(application, clock.instant()); + return 1L; + } + + @Override + public void restart(HostFilter filter) {} + }); } @Override diff --git a/container-core/src/main/java/com/yahoo/container/handler/Coverage.java b/container-core/src/main/java/com/yahoo/container/handler/Coverage.java index ef088a1685a..00c3a1d1aae 100644 --- a/container-core/src/main/java/com/yahoo/container/handler/Coverage.java +++ b/container-core/src/main/java/com/yahoo/container/handler/Coverage.java @@ -2,7 +2,7 @@ package com.yahoo.container.handler; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; /** * The coverage report for a result set. diff --git a/container-core/src/main/java/com/yahoo/container/handler/test/MockService.java b/container-core/src/main/java/com/yahoo/container/handler/test/MockService.java index 8f24e2e64a2..b8175802ff7 100644 --- a/container-core/src/main/java/com/yahoo/container/handler/test/MockService.java +++ b/container-core/src/main/java/com/yahoo/container/handler/test/MockService.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.handler.test; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.LoggingRequestHandler; diff --git a/container-core/src/main/java/com/yahoo/container/handler/test/MockServiceHandler.java b/container-core/src/main/java/com/yahoo/container/handler/test/MockServiceHandler.java index c417eb6516e..a06422a1bf4 100644 --- a/container-core/src/main/java/com/yahoo/container/handler/test/MockServiceHandler.java +++ b/container-core/src/main/java/com/yahoo/container/handler/test/MockServiceHandler.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.handler.test; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.container.jdisc.HttpRequest; /** diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/RequestHandlerTestDriver.java b/container-core/src/main/java/com/yahoo/container/jdisc/RequestHandlerTestDriver.java index de877bc413d..04780c81be4 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/RequestHandlerTestDriver.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/RequestHandlerTestDriver.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.jdisc; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.jdisc.Request; import com.yahoo.jdisc.Response; import com.yahoo.jdisc.application.ContainerBuilder; diff --git a/container-core/src/main/java/com/yahoo/metrics/simple/Counter.java b/container-core/src/main/java/com/yahoo/metrics/simple/Counter.java index d726486c195..a119902aac7 100644 --- a/container-core/src/main/java/com/yahoo/metrics/simple/Counter.java +++ b/container-core/src/main/java/com/yahoo/metrics/simple/Counter.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.metrics.simple; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.metrics.simple.UntypedMetric.AssumedType; /** diff --git a/container-core/src/main/java/com/yahoo/metrics/simple/Gauge.java b/container-core/src/main/java/com/yahoo/metrics/simple/Gauge.java index ab250526bd5..684bf4b5db1 100644 --- a/container-core/src/main/java/com/yahoo/metrics/simple/Gauge.java +++ b/container-core/src/main/java/com/yahoo/metrics/simple/Gauge.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.metrics.simple; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.metrics.simple.UntypedMetric.AssumedType; /** diff --git a/container-core/src/main/java/com/yahoo/metrics/simple/MetricReceiver.java b/container-core/src/main/java/com/yahoo/metrics/simple/MetricReceiver.java index 94be443d9bb..c2ef8afd279 100644 --- a/container-core/src/main/java/com/yahoo/metrics/simple/MetricReceiver.java +++ b/container-core/src/main/java/com/yahoo/metrics/simple/MetricReceiver.java @@ -6,7 +6,7 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.atomic.AtomicReference; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.google.common.collect.ImmutableMap; import com.yahoo.concurrent.ThreadLocalDirectory; diff --git a/container-core/src/main/java/com/yahoo/metrics/simple/MetricSettings.java b/container-core/src/main/java/com/yahoo/metrics/simple/MetricSettings.java index 39403703e71..5f4b66275a9 100644 --- a/container-core/src/main/java/com/yahoo/metrics/simple/MetricSettings.java +++ b/container-core/src/main/java/com/yahoo/metrics/simple/MetricSettings.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.metrics.simple; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; /** * All information needed for creating any extra data structures associated with diff --git a/container-core/src/main/java/com/yahoo/metrics/simple/Point.java b/container-core/src/main/java/com/yahoo/metrics/simple/Point.java index af0a1207072..f9ea3849ddc 100644 --- a/container-core/src/main/java/com/yahoo/metrics/simple/Point.java +++ b/container-core/src/main/java/com/yahoo/metrics/simple/Point.java @@ -5,7 +5,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.google.common.collect.ImmutableList; import com.yahoo.collections.Tuple2; import com.yahoo.jdisc.Metric.Context; diff --git a/container-core/src/main/java/com/yahoo/metrics/simple/PointBuilder.java b/container-core/src/main/java/com/yahoo/metrics/simple/PointBuilder.java index 9ca1198e8ea..47061eba10b 100644 --- a/container-core/src/main/java/com/yahoo/metrics/simple/PointBuilder.java +++ b/container-core/src/main/java/com/yahoo/metrics/simple/PointBuilder.java @@ -4,7 +4,7 @@ package com.yahoo.metrics.simple; import java.util.ArrayList; import java.util.Collections; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; /** * Single-use builder for the immutable Point instances used to set dimensions diff --git a/container-core/src/main/java/com/yahoo/processing/handler/ProcessingTestDriver.java b/container-core/src/main/java/com/yahoo/processing/handler/ProcessingTestDriver.java index 12a226dd50e..99675e3fef5 100644 --- a/container-core/src/main/java/com/yahoo/processing/handler/ProcessingTestDriver.java +++ b/container-core/src/main/java/com/yahoo/processing/handler/ProcessingTestDriver.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.processing.handler; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.component.ComponentId; import com.yahoo.component.chain.Chain; import com.yahoo.component.provider.ComponentRegistry; diff --git a/container-search/src/main/java/com/yahoo/prelude/query/ToolBox.java b/container-search/src/main/java/com/yahoo/prelude/query/ToolBox.java index 03c3f7e8b2c..7f37b77919b 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/ToolBox.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/ToolBox.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude.query; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; /** * Query tree helper methods and factories. diff --git a/container-search/src/main/java/com/yahoo/search/grouping/request/ArrayAtLookup.java b/container-search/src/main/java/com/yahoo/search/grouping/request/ArrayAtLookup.java index 43ca1df70d8..d3f239ac924 100644 --- a/container-search/src/main/java/com/yahoo/search/grouping/request/ArrayAtLookup.java +++ b/container-search/src/main/java/com/yahoo/search/grouping/request/ArrayAtLookup.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.grouping.request; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; /** * Represents access of array element in a document attribute in a {@link GroupingExpression}. diff --git a/container-search/src/main/java/com/yahoo/search/grouping/request/InterpolatedLookup.java b/container-search/src/main/java/com/yahoo/search/grouping/request/InterpolatedLookup.java index ffe835946f8..519f1c99d92 100644 --- a/container-search/src/main/java/com/yahoo/search/grouping/request/InterpolatedLookup.java +++ b/container-search/src/main/java/com/yahoo/search/grouping/request/InterpolatedLookup.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.grouping.request; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; /** * This class represents a lookup in a multivalue document diff --git a/container-search/src/main/java/com/yahoo/search/result/Coverage.java b/container-search/src/main/java/com/yahoo/search/result/Coverage.java index dd01494879d..5074a520a4e 100644 --- a/container-search/src/main/java/com/yahoo/search/result/Coverage.java +++ b/container-search/src/main/java/com/yahoo/search/result/Coverage.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.result; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; /** * The coverage report for a result set. diff --git a/container-search/src/main/java/com/yahoo/search/searchers/ValidateNearestNeighborSearcher.java b/container-search/src/main/java/com/yahoo/search/searchers/ValidateNearestNeighborSearcher.java index 27f34914ee9..f81083221a8 100644 --- a/container-search/src/main/java/com/yahoo/search/searchers/ValidateNearestNeighborSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/searchers/ValidateNearestNeighborSearcher.java @@ -2,7 +2,7 @@ package com.yahoo.search.searchers; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.prelude.query.Item; import com.yahoo.prelude.query.NearestNeighborItem; diff --git a/container-search/src/main/java/com/yahoo/search/yql/FieldFiller.java b/container-search/src/main/java/com/yahoo/search/yql/FieldFiller.java index 343f1b06e84..13a9f9510cd 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/FieldFiller.java +++ b/container-search/src/main/java/com/yahoo/search/yql/FieldFiller.java @@ -6,7 +6,7 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.component.chain.dependencies.After; import com.yahoo.prelude.fastsearch.DocumentdbInfoConfig; import com.yahoo.prelude.fastsearch.DocumentdbInfoConfig.Documentdb; diff --git a/container-search/src/main/java/com/yahoo/search/yql/FieldFilter.java b/container-search/src/main/java/com/yahoo/search/yql/FieldFilter.java index c4ab612651d..d5603ff0171 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/FieldFilter.java +++ b/container-search/src/main/java/com/yahoo/search/yql/FieldFilter.java @@ -5,7 +5,7 @@ import java.util.Iterator; import java.util.Map.Entry; import java.util.Set; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.component.chain.dependencies.After; import com.yahoo.component.chain.dependencies.Before; import com.yahoo.prelude.fastsearch.FastHit; diff --git a/container-search/src/main/java/com/yahoo/search/yql/MinimalQueryInserter.java b/container-search/src/main/java/com/yahoo/search/yql/MinimalQueryInserter.java index 8223cb2cba2..649d678db55 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/MinimalQueryInserter.java +++ b/container-search/src/main/java/com/yahoo/search/yql/MinimalQueryInserter.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.yql; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.google.inject.Inject; import com.yahoo.language.Linguistics; import com.yahoo.language.simple.SimpleLinguistics; diff --git a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java index c688f61add0..8334775b8e2 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java +++ b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java @@ -15,7 +15,7 @@ import java.util.Set; import java.util.StringTokenizer; import java.util.function.Supplier; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.google.common.base.Preconditions; import com.yahoo.collections.LazyMap; import com.yahoo.collections.LazySet; diff --git a/document/src/main/java/com/yahoo/document/idstring/IdString.java b/document/src/main/java/com/yahoo/document/idstring/IdString.java index 62fd7a2df99..55beff9eef9 100644 --- a/document/src/main/java/com/yahoo/document/idstring/IdString.java +++ b/document/src/main/java/com/yahoo/document/idstring/IdString.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.document.idstring; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.text.Text; import com.yahoo.text.Utf8String; diff --git a/fbench/src/fbench/fbench.cpp b/fbench/src/fbench/fbench.cpp index 92facaa3cae..0cd9498258e 100644 --- a/fbench/src/fbench/fbench.cpp +++ b/fbench/src/fbench/fbench.cpp @@ -443,7 +443,7 @@ FBench::Main(int argc, char *argv[]) keepAlive = false; break; case 'd': - base64Decode = false; + base64Decode = true; break; case 'x': // consuming x for backwards compability. This turned on header benchmark data diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/LazyTemporaryStorageFileReferenceData.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/LazyTemporaryStorageFileReferenceData.java index 0d4f207b48e..974d5ff1489 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/LazyTemporaryStorageFileReferenceData.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/LazyTemporaryStorageFileReferenceData.java @@ -19,7 +19,7 @@ public class LazyTemporaryStorageFileReferenceData extends LazyFileReferenceData public void close() { try { super.close(); - Files.delete(file.toPath()); + Files.deleteIfExists(file.toPath()); } catch (IOException e) { throw new RuntimeException(e); } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 85cee0d151b..0dc06270031 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -359,7 +359,7 @@ public class Flags { ZONE_ID, APPLICATION_ID); public static final UnboundBooleanFlag USE_FILE_DISTRIBUTION_CONNECTION_POOL = defineFeatureFlag( - "use-file-distribution-connection-pool", false, + "use-file-distribution-connection-pool", true, List.of("hmusum"), "2021-11-16", "2021-12-16", "Whether to use FileDistributionConnectionPool instead of JRTConnectionPool for file downloads", "Takes effect on config server restart", diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/test/ServerProviderConformanceTest.java b/jdisc_core/src/main/java/com/yahoo/jdisc/test/ServerProviderConformanceTest.java index 82929013dda..38d4c70646c 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/test/ServerProviderConformanceTest.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/test/ServerProviderConformanceTest.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.test; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.google.inject.AbstractModule; import com.google.inject.Key; import com.google.inject.Module; diff --git a/linguistics-components/src/main/java/com/yahoo/language/sentencepiece/SentencePieceEmbedder.java b/linguistics-components/src/main/java/com/yahoo/language/sentencepiece/SentencePieceEmbedder.java index 3f4e8ee3462..3afc85300d4 100644 --- a/linguistics-components/src/main/java/com/yahoo/language/sentencepiece/SentencePieceEmbedder.java +++ b/linguistics-components/src/main/java/com/yahoo/language/sentencepiece/SentencePieceEmbedder.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.language.sentencepiece; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.google.inject.Inject; import com.yahoo.language.Language; import com.yahoo.language.process.Embedder; diff --git a/model-evaluation/src/main/java/ai/vespa/models/evaluation/Model.java b/model-evaluation/src/main/java/ai/vespa/models/evaluation/Model.java index 8af5f7bc499..8e7918bae9b 100644 --- a/model-evaluation/src/main/java/ai/vespa/models/evaluation/Model.java +++ b/model-evaluation/src/main/java/ai/vespa/models/evaluation/Model.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.models.evaluation; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.yahoo.searchlib.rankingexpression.ExpressionFunction; diff --git a/model-evaluation/src/main/java/ai/vespa/models/evaluation/ModelsEvaluator.java b/model-evaluation/src/main/java/ai/vespa/models/evaluation/ModelsEvaluator.java index 01427ca811a..1554f11195f 100644 --- a/model-evaluation/src/main/java/ai/vespa/models/evaluation/ModelsEvaluator.java +++ b/model-evaluation/src/main/java/ai/vespa/models/evaluation/ModelsEvaluator.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.models.evaluation; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.google.common.collect.ImmutableMap; import com.google.inject.Inject; import com.yahoo.component.AbstractComponent; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java index ad20f68ca33..e0ccbe10b10 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java @@ -26,7 +26,7 @@ public class Cluster { private final ClusterSpec.Id id; private final boolean exclusive; private final ClusterResources min, max; - private final boolean required; + private boolean required; private final Optional<Suggestion> suggested; private final Optional<ClusterResources> target; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java index 2755030e2b3..078b0621a99 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java @@ -139,20 +139,23 @@ public class AllocatableClusterResources { public static Optional<AllocatableClusterResources> from(ClusterResources wantedResources, ClusterSpec clusterSpec, Limits applicationLimits, + boolean required, NodeList hosts, NodeRepository nodeRepository) { var capacityPolicies = new CapacityPolicies(nodeRepository); var systemLimits = new NodeResourceLimits(nodeRepository); boolean exclusive = clusterSpec.isExclusive(); + int actualNodes = capacityPolicies.decideSize(wantedResources.nodes(), required, true, false, clusterSpec); if ( !clusterSpec.isExclusive() && !nodeRepository.zone().getCloud().dynamicProvisioning()) { // We decide resources: Add overhead to what we'll request (advertised) to make sure real becomes (at least) cappedNodeResources var advertisedResources = nodeRepository.resourcesCalculator().realToRequest(wantedResources.nodeResources(), exclusive); advertisedResources = systemLimits.enlargeToLegal(advertisedResources, clusterSpec.type(), exclusive); // Ask for something legal advertisedResources = applicationLimits.cap(advertisedResources); // Overrides other conditions, even if it will then fail + advertisedResources = capacityPolicies.decideNodeResources(advertisedResources, required, clusterSpec); // Adjust to what we can request var realResources = nodeRepository.resourcesCalculator().requestToReal(advertisedResources, exclusive); // What we'll really get if ( ! systemLimits.isWithinRealLimits(realResources, clusterSpec.type())) return Optional.empty(); if (matchesAny(hosts, advertisedResources)) - return Optional.of(new AllocatableClusterResources(wantedResources.with(realResources), + return Optional.of(new AllocatableClusterResources(wantedResources.withNodes(actualNodes).with(realResources), advertisedResources, wantedResources, clusterSpec)); @@ -165,6 +168,7 @@ public class AllocatableClusterResources { for (Flavor flavor : nodeRepository.flavors().getFlavors()) { // Flavor decide resources: Real resources are the worst case real resources we'll get if we ask for these advertised resources NodeResources advertisedResources = nodeRepository.resourcesCalculator().advertisedResourcesOf(flavor); + advertisedResources = capacityPolicies.decideNodeResources(advertisedResources, required, clusterSpec); // Adjust to what we can get NodeResources realResources = nodeRepository.resourcesCalculator().requestToReal(advertisedResources, exclusive); // Adjust where we don't need exact match to the flavor @@ -180,7 +184,7 @@ public class AllocatableClusterResources { if ( ! between(applicationLimits.min().nodeResources(), applicationLimits.max().nodeResources(), advertisedResources)) continue; if ( ! systemLimits.isWithinRealLimits(realResources, clusterSpec.type())) continue; - var candidate = new AllocatableClusterResources(wantedResources.with(realResources), + var candidate = new AllocatableClusterResources(wantedResources.withNodes(actualNodes).with(realResources), advertisedResources, wantedResources, clusterSpec); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java index d727757b07e..b8a80a9bd2b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java @@ -66,6 +66,7 @@ public class AllocationOptimizer { groupsAdjustedForRedundancy, limits, target, current, clusterModel)); var allocatableResources = AllocatableClusterResources.from(next, current.clusterSpec(), limits, + clusterModel.cluster().required(), hosts, nodeRepository); if (allocatableResources.isEmpty()) continue; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java index 2b9c5396724..0c2c3c48df1 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.provision.provisioning; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Capacity; -import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.NodeResources; @@ -30,21 +29,10 @@ public class CapacityPolicies { this.sharedHosts = type -> PermanentFlags.SHARED_HOST.bindTo(nodeRepository.flagSource()).value().isEnabled(type.name()); } - public Capacity applyOn(Capacity capacity, ApplicationId application) { - return capacity.withLimits(applyOn(capacity.minResources(), capacity, application), - applyOn(capacity.maxResources(), capacity, application)); - } - - private ClusterResources applyOn(ClusterResources resources, Capacity capacity, ApplicationId application) { - int nodes = decideSize(resources.nodes(), capacity.isRequired(), application.instance().isTester()); - int groups = Math.min(resources.groups(), nodes); // cannot have more groups than nodes - var nodeResources = decideNodeResources(resources.nodeResources(), capacity.isRequired()); - return new ClusterResources(nodes, groups, nodeResources); - } - - private int decideSize(int requested, boolean required, boolean isTester) { + public int decideSize(int requested, boolean required, boolean canFail, boolean isTester, ClusterSpec cluster) { if (isTester) return 1; + ensureRedundancy(requested, cluster, canFail); if (required) return requested; switch(zone.environment()) { case dev : case test : return 1; @@ -55,7 +43,10 @@ public class CapacityPolicies { } } - private NodeResources decideNodeResources(NodeResources target, boolean required) { + public NodeResources decideNodeResources(NodeResources target, boolean required, ClusterSpec cluster) { + if (target.isUnspecified()) + target = defaultNodeResources(cluster.type()); + if (required) return target; // Dev does not cap the cpu or network of containers since usage is spotty: Allocate just a small amount exclusively @@ -86,11 +77,28 @@ public class CapacityPolicies { } /** - * Returns whether the nodes requested can share physical host with other applications. + * Whether or not the nodes requested can share physical host with other applications. * A security feature which only makes sense for prod. */ public boolean decideExclusivity(Capacity capacity, boolean requestedExclusivity) { return requestedExclusivity && (capacity.isRequired() || zone.environment() == Environment.prod); } + /** + * Throw if the node count is 1 for container and content clusters and we're in a production zone + * + * @throws IllegalArgumentException if only one node is requested and we can fail + */ + private void ensureRedundancy(int nodeCount, ClusterSpec cluster, boolean canFail) { + if (canFail && + nodeCount == 1 && + requiresRedundancy(cluster.type()) && + zone.environment().isProduction()) + throw new IllegalArgumentException("Deployments to prod require at least 2 nodes per cluster for redundancy. Not fulfilled for " + cluster); + } + + private static boolean requiresRedundancy(ClusterSpec.Type clusterType) { + return clusterType.isContent() || clusterType.isContainer(); + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java index 5dac2004931..b35b0a5e301 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java @@ -84,8 +84,8 @@ public class NodeRepositoryProvisioner implements Provisioner { @Override public List<HostSpec> prepare(ApplicationId application, ClusterSpec cluster, Capacity requested, ProvisionLogger logger) { - log.log(Level.FINE, "Received deploy prepare request for " + requested + - " for application " + application + ", cluster " + cluster); + log.log(Level.FINE, () -> "Received deploy prepare request for " + requested + + " for application " + application + ", cluster " + cluster); if (cluster.group().isPresent()) throw new IllegalArgumentException("Node requests cannot specify a group"); @@ -96,16 +96,17 @@ public class NodeRepositoryProvisioner implements Provisioner { NodeResources resources; NodeSpec nodeSpec; if (requested.type() == NodeType.tenant) { - var actual = capacityPolicies.applyOn(requested, application); - ClusterResources target = decideTargetResources(application, cluster, actual); - boolean exclusive = capacityPolicies.decideExclusivity(actual, cluster.isExclusive()); - ensureRedundancy(target.nodes(), cluster, actual.canFail(), application); - logIfDownscaled(requested.minResources().nodes(), actual.minResources().nodes(), cluster, logger); - - groups = target.groups(); - resources = target.nodeResources().isUnspecified() ? capacityPolicies.defaultNodeResources(cluster.type()) - : target.nodeResources(); - nodeSpec = NodeSpec.from(target.nodes(), resources, exclusive, actual.canFail()); + ClusterResources target = decideTargetResources(application, cluster, requested); + int nodeCount = capacityPolicies.decideSize(target.nodes(), + requested.isRequired(), + requested.canFail(), + application.instance().isTester(), + cluster); + groups = Math.min(target.groups(), nodeCount); // cannot have more groups than nodes + resources = capacityPolicies.decideNodeResources(target.nodeResources(), requested.isRequired(), cluster); + boolean exclusive = capacityPolicies.decideExclusivity(requested, cluster.isExclusive()); + nodeSpec = NodeSpec.from(nodeCount, resources, exclusive, requested.canFail()); + logIfDownscaled(target.nodes(), nodeCount, cluster, logger); } else { groups = 1; // type request with multiple groups is not supported @@ -189,28 +190,10 @@ public class NodeRepositoryProvisioner implements Provisioner { .advertisedResources(); } - /** - * Throw if the node count is 1 for container and content clusters and we're in a production zone - * - * @throws IllegalArgumentException if only one node is requested and we can fail - */ - private void ensureRedundancy(int nodeCount, ClusterSpec cluster, boolean canFail, ApplicationId application) { - if (! application.instance().isTester() && - canFail && - nodeCount == 1 && - requiresRedundancy(cluster.type()) && - zone.environment().isProduction()) - throw new IllegalArgumentException("Deployments to prod require at least 2 nodes per cluster for redundancy. Not fulfilled for " + cluster); - } - - private static boolean requiresRedundancy(ClusterSpec.Type clusterType) { - return clusterType.isContent() || clusterType.isContainer(); - } - - private void logIfDownscaled(int requestedMinNodes, int actualMinNodes, ClusterSpec cluster, ProvisionLogger logger) { - if (zone.environment().isManuallyDeployed() && actualMinNodes < requestedMinNodes) - logger.log(Level.INFO, "Requested " + requestedMinNodes + " nodes for " + cluster + - ", downscaling to " + actualMinNodes + " nodes in " + zone.environment()); + private void logIfDownscaled(int targetNodes, int actualNodes, ClusterSpec cluster, ProvisionLogger logger) { + if (zone.environment().isManuallyDeployed() && actualNodes < targetNodes) + logger.log(Level.INFO, "Requested " + targetNodes + " nodes for " + cluster + + ", downscaling to " + actualNodes + " nodes in " + zone.environment()); } private List<HostSpec> asSortedHosts(List<Node> nodes, NodeResources requestedResources) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java index 16017036e87..230278878b4 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java @@ -51,10 +51,10 @@ public class AutoscalingTest { tester.deploy(application1, cluster1, 5, 1, hostResources); tester.clock().advance(Duration.ofDays(1)); - assertTrue("No measurements -> No change", tester.autoscale(application1, cluster1, capacity).isEmpty()); + assertTrue("No measurements -> No change", tester.autoscale(application1, cluster1.id(), capacity).isEmpty()); tester.addCpuMeasurements(0.25f, 1f, 59, application1); - assertTrue("Too few measurements -> No change", tester.autoscale(application1, cluster1, capacity).isEmpty()); + assertTrue("Too few measurements -> No change", tester.autoscale(application1, cluster1.id(), capacity).isEmpty()); tester.clock().advance(Duration.ofDays(1)); tester.addCpuMeasurements(0.25f, 1f, 120, application1); @@ -62,10 +62,10 @@ public class AutoscalingTest { tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only ClusterResources scaledResources = tester.assertResources("Scaling up since resource usage is too high", 15, 1, 1.2, 28.6, 28.6, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); tester.deploy(application1, cluster1, scaledResources); - assertTrue("Cluster in flux -> No further change", tester.autoscale(application1, cluster1, capacity).isEmpty()); + assertTrue("Cluster in flux -> No further change", tester.autoscale(application1, cluster1.id(), capacity).isEmpty()); tester.deactivateRetired(application1, cluster1, scaledResources); @@ -74,19 +74,19 @@ public class AutoscalingTest { tester.clock().advance(Duration.ofMinutes(-10 * 5)); tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only assertTrue("Load change is large, but insufficient measurements for new config -> No change", - tester.autoscale(application1, cluster1, capacity).isEmpty()); + tester.autoscale(application1, cluster1.id(), capacity).isEmpty()); tester.addCpuMeasurements(0.19f, 1f, 100, application1); tester.clock().advance(Duration.ofMinutes(-10 * 5)); tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only - assertEquals("Load change is small -> No change", Optional.empty(), tester.autoscale(application1, cluster1, capacity).target()); + assertEquals("Load change is small -> No change", Optional.empty(), tester.autoscale(application1, cluster1.id(), capacity).target()); tester.addCpuMeasurements(0.1f, 1f, 120, application1); tester.clock().advance(Duration.ofMinutes(-10 * 5)); tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.assertResources("Scaling down to minimum since usage has gone down significantly", 7, 1, 1.0, 66.7, 66.7, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); var events = tester.nodeRepository().applications().get(application1).get().cluster(cluster1.id()).get().scalingEvents(); } @@ -110,7 +110,7 @@ public class AutoscalingTest { tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only ClusterResources scaledResources = tester.assertResources("Scaling up since cpu usage is too high", 7, 1, 2.5, 80.0, 80.0, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); tester.deploy(application1, cluster1, scaledResources); tester.deactivateRetired(application1, cluster1, scaledResources); @@ -120,7 +120,7 @@ public class AutoscalingTest { tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.assertResources("Scaling down since cpu usage has gone down", 4, 1, 2.5, 68.6, 68.6, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); } @Test @@ -147,7 +147,7 @@ public class AutoscalingTest { var capacity = Capacity.from(min, max); ClusterResources scaledResources = tester.assertResources("Scaling up since resource usage is too high", 14, 1, 1.4, 30.8, 30.8, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); assertEquals("Disk speed from min/max is used", NodeResources.DiskSpeed.any, scaledResources.nodeResources().diskSpeed()); tester.deploy(application1, cluster1, scaledResources); @@ -180,7 +180,7 @@ public class AutoscalingTest { // Autoscaling: Uses disk-speed any as well tester.clock().advance(Duration.ofDays(2)); tester.addCpuMeasurements(0.8f, 1f, 120, application1); - Autoscaler.Advice advice = tester.autoscale(application1, cluster1, capacity); + Autoscaler.Advice advice = tester.autoscale(application1, cluster1.id(), capacity); assertEquals(NodeResources.DiskSpeed.any, advice.target().get().nodeResources().diskSpeed()); @@ -205,7 +205,7 @@ public class AutoscalingTest { tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.assertResources("Scaling up to limit since resource usage is too high", 6, 1, 2.4, 78.0, 79.0, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); } @Test @@ -224,7 +224,7 @@ public class AutoscalingTest { tester.addMeasurements(0.05f, 0.05f, 0.05f, 0, 120, application1); tester.assertResources("Scaling down to limit since resource usage is low", 4, 1, 1.8, 7.7, 10.0, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); } @Test @@ -252,7 +252,7 @@ public class AutoscalingTest { tester.assertResources("Scaling up to limit since resource usage is too high", 4, 1, defaultResources.vcpu(), defaultResources.memoryGb(), defaultResources.diskGb(), - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); } @Test @@ -273,7 +273,7 @@ public class AutoscalingTest { tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.assertResources("Scaling up since resource usage is too high", 6, 6, 3.6, 8.0, 10.0, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); } @Test @@ -291,7 +291,7 @@ public class AutoscalingTest { tester.deploy(application1, cluster1, 5, 1, resources); tester.clock().advance(Duration.ofDays(1)); tester.addCpuMeasurements(0.25f, 1f, 120, application1); - assertTrue(tester.autoscale(application1, cluster1, capacity).isEmpty()); + assertTrue(tester.autoscale(application1, cluster1.id(), capacity).isEmpty()); } @Test @@ -361,7 +361,7 @@ public class AutoscalingTest { tester.deploy(application1, cluster1, 2, 1, resources); tester.addMeasurements(0.5f, 0.6f, 0.7f, 1, false, true, 120, application1); assertTrue("Not scaling up since nodes were measured while cluster was unstable", - tester.autoscale(application1, cluster1, capacity).isEmpty()); + tester.autoscale(application1, cluster1.id(), capacity).isEmpty()); } @Test @@ -379,7 +379,7 @@ public class AutoscalingTest { tester.deploy(application1, cluster1, 2, 1, resources); tester.addMeasurements(0.5f, 0.6f, 0.7f, 1, true, false, 120, application1); assertTrue("Not scaling up since nodes were measured while cluster was unstable", - tester.autoscale(application1, cluster1, capacity).isEmpty()); + tester.autoscale(application1, cluster1.id(), capacity).isEmpty()); } @Test @@ -400,7 +400,7 @@ public class AutoscalingTest { tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.assertResources("Scaling up since resource usage is too high", 7, 7, 2.5, 80.0, 80.0, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); } @Test @@ -423,7 +423,7 @@ public class AutoscalingTest { t -> 1.0); tester.assertResources("Scaling up since resource usage is too high, changing to 1 group is cheaper", 8, 1, 2.6, 83.3, 83.3, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); } /** Same as above but mostly write traffic, which favors smaller groups */ @@ -447,7 +447,7 @@ public class AutoscalingTest { t -> 100.0); tester.assertResources("Scaling down since resource usage is too high, changing to 1 group is cheaper", 4, 1, 2.1, 83.3, 83.3, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); } @Test @@ -469,7 +469,7 @@ public class AutoscalingTest { tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.assertResources("Increase group size to reduce memory load", 8, 2, 12.4, 96.2, 62.5, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); } @Test @@ -490,7 +490,7 @@ public class AutoscalingTest { tester.addMemMeasurements(0.02f, 0.95f, 120, application1); tester.assertResources("Scaling down", 6, 1, 2.9, 4.0, 95.0, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); } @Test @@ -510,7 +510,7 @@ public class AutoscalingTest { tester.addMemMeasurements(0.02f, 0.95f, 120, application1); tester.clock().advance(Duration.ofMinutes(-10 * 5)); tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only - assertTrue(tester.autoscale(application1, cluster1, capacity).target().isEmpty()); + assertTrue(tester.autoscale(application1, cluster1.id(), capacity).target().isEmpty()); // Trying the same later causes autoscaling tester.clock().advance(Duration.ofDays(2)); @@ -519,7 +519,7 @@ public class AutoscalingTest { tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.assertResources("Scaling down", 6, 1, 1.4, 4.0, 95.0, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); } @Test @@ -530,8 +530,7 @@ public class AutoscalingTest { var capacity = Capacity.from(min, max); { // No memory tax - AutoscalingTester tester = new AutoscalingTester(new Zone(Environment.prod, RegionName.from("us-east")), - hostResources, + AutoscalingTester tester = new AutoscalingTester(Environment.prod, hostResources, new OnlySubtractingWhenForecastingCalculator(0)); ApplicationId application1 = tester.applicationId("app1"); @@ -543,12 +542,11 @@ public class AutoscalingTest { tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.assertResources("Scaling up", 4, 1, 6.7, 20.5, 200, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); } { // 15 Gb memory tax - AutoscalingTester tester = new AutoscalingTester(new Zone(Environment.prod, RegionName.from("us-east")), - hostResources, + AutoscalingTester tester = new AutoscalingTester(Environment.prod, hostResources, new OnlySubtractingWhenForecastingCalculator(15)); ApplicationId application1 = tester.applicationId("app1"); @@ -560,7 +558,7 @@ public class AutoscalingTest { tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.assertResources("Scaling up", 4, 1, 6.7, 35.5, 200, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); } } @@ -591,7 +589,7 @@ public class AutoscalingTest { tester.addMemMeasurements(0.9f, 0.6f, 120, application1); ClusterResources scaledResources = tester.assertResources("Scaling up since resource usage is too high.", 8, 1, 3, 83, 34.3, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); tester.deploy(application1, cluster1, scaledResources); tester.deactivateRetired(application1, cluster1, scaledResources); @@ -602,7 +600,7 @@ public class AutoscalingTest { tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.assertResources("Scaling down since resource usage has gone down", 5, 1, 3, 83, 36.0, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); } @Test @@ -624,18 +622,17 @@ public class AutoscalingTest { // (no read share stored) tester.assertResources("Advice to scale up since we set aside for bcp by default", 7, 1, 3, 100, 100, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); tester.storeReadShare(0.25, 0.5, application1); tester.assertResources("Half of global share is the same as the default assumption used above", 7, 1, 3, 100, 100, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); tester.storeReadShare(0.5, 0.5, application1); tester.assertResources("Advice to scale down since we don't need room for bcp", 4, 1, 3, 100, 100, - tester.autoscale(application1, cluster1, capacity)); - + tester.autoscale(application1, cluster1.id(), capacity)); } @Test @@ -659,7 +656,7 @@ public class AutoscalingTest { // (no query rate data) tester.assertResources("Scale up since we assume we need 2x cpu for growth when no data scaling time data", 5, 1, 6.3, 100, 100, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); tester.setScalingDuration(application1, cluster1.id(), Duration.ofMinutes(5)); timeAdded = tester.addQueryRateMeasurements(application1, cluster1.id(), @@ -669,7 +666,7 @@ public class AutoscalingTest { tester.addCpuMeasurements(0.25f, 1f, 200, application1); tester.assertResources("Scale down since observed growth is slower than scaling time", 5, 1, 3.4, 100, 100, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); tester.clearQueryRateMeasurements(application1, cluster1.id()); @@ -681,7 +678,7 @@ public class AutoscalingTest { tester.addCpuMeasurements(0.25f, 1f, 200, application1); tester.assertResources("Scale up since observed growth is faster than scaling time", 5, 1, 10.0, 100, 100, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); } @Test @@ -708,35 +705,55 @@ public class AutoscalingTest { tester.addLoadMeasurements(application1, cluster1.id(), 100, t -> t == 0 ? 20.0 : 10.0, t -> 10.0); tester.assertResources("Query and write load is equal -> scale up somewhat", 5, 1, 7.3, 100, 100, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); tester.addCpuMeasurements(0.4f, 1f, 100, application1); tester.clock().advance(Duration.ofMinutes(-100 * 5)); tester.addLoadMeasurements(application1, cluster1.id(), 100, t -> t == 0 ? 80.0 : 40.0, t -> 10.0); tester.assertResources("Query load is 4x write load -> scale up more", 5, 1, 9.5, 100, 100, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); tester.addCpuMeasurements(0.3f, 1f, 100, application1); tester.clock().advance(Duration.ofMinutes(-100 * 5)); tester.addLoadMeasurements(application1, cluster1.id(), 100, t -> t == 0 ? 20.0 : 10.0, t -> 100.0); tester.assertResources("Write load is 10x query load -> scale down", 5, 1, 2.9, 100, 100, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); tester.addCpuMeasurements(0.4f, 1f, 100, application1); tester.clock().advance(Duration.ofMinutes(-100 * 5)); tester.addLoadMeasurements(application1, cluster1.id(), 100, t -> t == 0 ? 20.0 : 10.0, t-> 0.0); tester.assertResources("Query only -> largest possible", 5, 1, 10.0, 100, 100, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); tester.addCpuMeasurements(0.4f, 1f, 100, application1); tester.clock().advance(Duration.ofMinutes(-100 * 5)); tester.addLoadMeasurements(application1, cluster1.id(), 100, t -> 0.0, t -> 10.0); tester.assertResources("Write only -> smallest possible", 5, 1, 2.1, 100, 100, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); + } + + @Test + public void test_cd_autoscaling_test() { + NodeResources resources = new NodeResources(1, 4, 50, 1); + ClusterResources min = new ClusterResources( 2, 1, resources); + ClusterResources max = new ClusterResources(3, 1, resources); + var capacity = Capacity.from(min, max); + AutoscalingTester tester = new AutoscalingTester(resources.withVcpu(resources.vcpu() * 2)); + ApplicationId application1 = tester.applicationId("application1"); + ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1"); + tester.deploy(application1, cluster1, 2, 1, resources); + + tester.addQueryRateMeasurements(application1, cluster1.id(), + 500, t -> 0.0); + tester.addCpuMeasurements(0.5f, 1f, 10, application1); + + tester.assertResources("Advice to scale up since observed growth is much faster than scaling time", + 3, 1, 1, 4, 50, + tester.autoscale(application1, cluster1.id(), capacity)); } @Test @@ -755,7 +772,7 @@ public class AutoscalingTest { 500, t -> 100.0); tester.addCpuMeasurements(1.0f, 1f, 10, application1); assertTrue("Not attempting to scale up because policies dictate we'll only get one node", - tester.autoscale(application1, cluster1, capacity).target().isEmpty()); + tester.autoscale(application1, cluster1.id(), capacity).target().isEmpty()); } /** Same setup as test_autoscaling_in_dev(), just with required = true */ @@ -776,7 +793,7 @@ public class AutoscalingTest { tester.addCpuMeasurements(1.0f, 1f, 10, application1); tester.assertResources("We scale up even in dev because resources are required", 3, 1, 1.0, 4, 50, - tester.autoscale(application1, cluster1, capacity)); + tester.autoscale(application1, cluster1.id(), capacity)); } /** diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java index 03a29b6613c..998a1e86c3e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java @@ -24,7 +24,6 @@ import com.yahoo.vespa.hosted.provision.applications.Cluster; import com.yahoo.vespa.hosted.provision.applications.ScalingEvent; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.IP; -import com.yahoo.vespa.hosted.provision.provisioning.CapacityPolicies; import com.yahoo.vespa.hosted.provision.provisioning.HostResourcesCalculator; import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester; @@ -47,7 +46,6 @@ class AutoscalingTester { private final ProvisioningTester provisioningTester; private final Autoscaler autoscaler; private final MockHostResourcesCalculator hostResourcesCalculator; - private final CapacityPolicies capacityPolicies; /** Creates an autoscaling tester with a single host type ready */ public AutoscalingTester(NodeResources hostResources) { @@ -55,15 +53,11 @@ class AutoscalingTester { } public AutoscalingTester(Environment environment, NodeResources hostResources) { - this(new Zone(environment, RegionName.from("us-east")), hostResources, null); + this(environment, hostResources, null); } - public AutoscalingTester(Zone zone, NodeResources hostResources) { - this(zone, hostResources, null); - } - - public AutoscalingTester(Zone zone, NodeResources hostResources, HostResourcesCalculator resourcesCalculator) { - this(zone, List.of(new Flavor("hostFlavor", hostResources)), resourcesCalculator); + public AutoscalingTester(Environment environment, NodeResources hostResources, HostResourcesCalculator resourcesCalculator) { + this(new Zone(environment, RegionName.from("us-east")), List.of(new Flavor("hostFlavor", hostResources)), resourcesCalculator); provisioningTester.makeReadyNodes(20, "hostFlavor", NodeType.host, 8); provisioningTester.activateTenantHosts(); } @@ -82,7 +76,6 @@ class AutoscalingTester { hostResourcesCalculator = new MockHostResourcesCalculator(zone); autoscaler = new Autoscaler(nodeRepository()); - capacityPolicies = new CapacityPolicies(provisioningTester.nodeRepository()); } public ProvisioningTester provisioning() { return provisioningTester; } @@ -313,14 +306,13 @@ class AutoscalingTester { ((MemoryMetricsDb)nodeMetricsDb()).clearClusterMetrics(application, cluster); } - public Autoscaler.Advice autoscale(ApplicationId applicationId, ClusterSpec cluster, Capacity capacity) { - capacity = capacityPolicies.applyOn(capacity, applicationId); + public Autoscaler.Advice autoscale(ApplicationId applicationId, ClusterSpec.Id clusterId, Capacity capacity) { Application application = nodeRepository().applications().get(applicationId).orElse(Application.empty(applicationId)) - .withCluster(cluster.id(), false, capacity); + .withCluster(clusterId, false, capacity); try (Mutex lock = nodeRepository().nodes().lock(applicationId)) { nodeRepository().applications().put(application, lock); } - return autoscaler.autoscale(application, application.clusters().get(cluster.id()), + return autoscaler.autoscale(application, application.clusters().get(clusterId), nodeRepository().nodes().list(Node.State.active).owner(applicationId)); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java index 4c3274d763f..6a5b45db8ff 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java @@ -5,13 +5,8 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.NodeResources; -import com.yahoo.config.provision.RegionName; -import com.yahoo.config.provision.SystemName; -import com.yahoo.config.provision.Zone; import com.yahoo.test.ManualClock; -import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.applications.Cluster; import com.yahoo.vespa.hosted.provision.applications.ScalingEvent; import com.yahoo.vespa.hosted.provision.testutils.MockDeployer; @@ -191,6 +186,7 @@ public class AutoscalingMaintainerTest { var tester = new AutoscalingMaintainerTester(new MockDeployer.ApplicationContext(app1, cluster1, app1Capacity)); ManualClock clock = tester.clock(); + // deploy tester.deploy(app1, cluster1, app1Capacity); autoscale(false, Duration.ofMinutes( 1), Duration.ofMinutes( 5), clock, app1, cluster1, tester); @@ -226,33 +222,6 @@ public class AutoscalingMaintainerTest { tester.cluster(app1, cluster1).lastScalingEvent().get().generation()); } - @Test - public void test_cd_autoscaling_test() { - ApplicationId app1 = AutoscalingMaintainerTester.makeApplicationId("app1"); - ClusterSpec cluster1 = AutoscalingMaintainerTester.containerClusterSpec(); - NodeResources resources = new NodeResources(1, 4, 50, 1); - ClusterResources min = new ClusterResources( 2, 1, resources); - ClusterResources max = new ClusterResources(3, 1, resources); - var capacity = Capacity.from(min, max); - var tester = new AutoscalingMaintainerTester(new Zone(SystemName.cd, Environment.prod, RegionName.from("us-east3")), - new MockDeployer.ApplicationContext(app1, cluster1, capacity)); - ManualClock clock = tester.clock(); - - tester.deploy(app1, cluster1, capacity); - assertEquals(2, - tester.nodeRepository().nodes().list(Node.State.active) - .owner(app1) - .cluster(cluster1.id()) - .size()); - - autoscale(false, Duration.ofMinutes( 1), Duration.ofMinutes( 5), clock, app1, cluster1, tester); - assertEquals(3, - tester.nodeRepository().nodes().list(Node.State.active) - .owner(app1) - .cluster(cluster1.id()) - .size()); - } - private void autoscale(boolean down, Duration completionTime, Duration expectedWindow, ManualClock clock, ApplicationId application, ClusterSpec cluster, AutoscalingMaintainerTester tester) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTester.java index e1a1a2af5fb..a47fb983d21 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTester.java @@ -42,11 +42,9 @@ public class AutoscalingMaintainerTester { private final MockDeployer deployer; public AutoscalingMaintainerTester(MockDeployer.ApplicationContext ... appContexts) { - this(new Zone(Environment.prod, RegionName.from("us-east3")), appContexts); - } - - public AutoscalingMaintainerTester(Zone zone, MockDeployer.ApplicationContext ... appContexts) { - provisioningTester = new ProvisioningTester.Builder().zone(zone).flavorsConfig(flavorsConfig()).build(); + provisioningTester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east3"))) + .flavorsConfig(flavorsConfig()) + .build(); provisioningTester.clock().setInstant(Instant.ofEpochMilli(0)); Map<ApplicationId, MockDeployer.ApplicationContext> apps = Arrays.stream(appContexts) .collect(Collectors.toMap(c -> c.id(), c -> c)); @@ -107,7 +105,7 @@ public class AutoscalingMaintainerTester { private FlavorsConfig flavorsConfig() { FlavorConfigBuilder b = new FlavorConfigBuilder(); - b.addFlavor("flt", 30, 30, 50, 3, Flavor.Type.BARE_METAL); + b.addFlavor("flt", 30, 30, 40, 3, Flavor.Type.BARE_METAL); b.addFlavor("cpu", 40, 20, 40, 3, Flavor.Type.BARE_METAL); b.addFlavor("mem", 20, 40, 40, 3, Flavor.Type.BARE_METAL); return b.build(); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java index 95f25612dd7..db165aae919 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java @@ -523,7 +523,7 @@ public class ProvisioningTest { ApplicationId application = ProvisioningTester.applicationId(); tester.makeReadyHosts(10, defaultResources).activateTenantHosts(); - prepare(application, 1, 1, 1, 1, defaultResources, tester); + prepare(application, 1, 2, 3, 3, defaultResources, tester); } @Test @@ -1015,10 +1015,10 @@ public class ProvisioningTest { allHosts.addAll(content1); Function<Integer, Capacity> capacity = count -> Capacity.from(new ClusterResources(count, 1, NodeResources.unspecified()), required, true); - int expectedContainer0Size = tester.decideSize(capacity.apply(container0Size), application); - int expectedContainer1Size = tester.decideSize(capacity.apply(container1Size), application); - int expectedContent0Size = tester.decideSize(capacity.apply(content0Size), application); - int expectedContent1Size = tester.decideSize(capacity.apply(content1Size), application); + int expectedContainer0Size = tester.decideSize(container0Size, capacity.apply(container0Size), containerCluster0, application); + int expectedContainer1Size = tester.decideSize(container1Size, capacity.apply(container1Size), containerCluster1, application); + int expectedContent0Size = tester.decideSize(content0Size, capacity.apply(content0Size), contentCluster0, application); + int expectedContent1Size = tester.decideSize(content1Size, capacity.apply(content1Size), contentCluster1, application); assertEquals("Hosts in each group cluster is disjunct and the total number of unretired nodes is correct", expectedContainer0Size + expectedContainer1Size + expectedContent0Size + expectedContent1Size, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java index d1ec1018023..6ca93671087 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java @@ -152,8 +152,8 @@ public class ProvisioningTester { public NodeList getNodes(ApplicationId id, Node.State ... inState) { return nodeRepository.nodes().list(inState).owner(id); } public InMemoryFlagSource flagSource() { return (InMemoryFlagSource) nodeRepository.flagSource(); } - public int decideSize(Capacity capacity, ApplicationId application) { - return capacityPolicies.applyOn(capacity, application).minResources().nodes(); + public int decideSize(int size, Capacity capacity, ClusterSpec cluster, ApplicationId application) { + return capacityPolicies.decideSize(size, capacity.isRequired(), capacity.canFail(), application.instance().isTester(), cluster); } public Node patchNode(Node node, UnaryOperator<Node> patcher) { diff --git a/predicate-search/src/main/java/com/yahoo/search/predicate/Config.java b/predicate-search/src/main/java/com/yahoo/search/predicate/Config.java index 9b347ebe652..6b731b6221d 100644 --- a/predicate-search/src/main/java/com/yahoo/search/predicate/Config.java +++ b/predicate-search/src/main/java/com/yahoo/search/predicate/Config.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.predicate; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import java.io.DataInputStream; import java.io.DataOutputStream; diff --git a/predicate-search/src/main/java/com/yahoo/search/predicate/Hit.java b/predicate-search/src/main/java/com/yahoo/search/predicate/Hit.java index 5314e8b8f5e..cb2cc538dc7 100644 --- a/predicate-search/src/main/java/com/yahoo/search/predicate/Hit.java +++ b/predicate-search/src/main/java/com/yahoo/search/predicate/Hit.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.predicate; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; /** * Represents a hit from the predicate search algorithm. diff --git a/predicate-search/src/main/java/com/yahoo/search/predicate/PredicateIndex.java b/predicate-search/src/main/java/com/yahoo/search/predicate/PredicateIndex.java index f718ca4aba5..38cb56c2e2b 100644 --- a/predicate-search/src/main/java/com/yahoo/search/predicate/PredicateIndex.java +++ b/predicate-search/src/main/java/com/yahoo/search/predicate/PredicateIndex.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.predicate; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.document.predicate.Predicate; import com.yahoo.search.predicate.index.*; import com.yahoo.search.predicate.index.conjunction.ConjunctionHit; diff --git a/predicate-search/src/main/java/com/yahoo/search/predicate/PredicateIndexBuilder.java b/predicate-search/src/main/java/com/yahoo/search/predicate/PredicateIndexBuilder.java index c1500299d02..d9153b24a87 100644 --- a/predicate-search/src/main/java/com/yahoo/search/predicate/PredicateIndexBuilder.java +++ b/predicate-search/src/main/java/com/yahoo/search/predicate/PredicateIndexBuilder.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.predicate; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.google.common.base.Preconditions; import com.google.common.primitives.Bytes; import com.google.common.primitives.Ints; diff --git a/predicate-search/src/main/java/com/yahoo/search/predicate/PredicateQuery.java b/predicate-search/src/main/java/com/yahoo/search/predicate/PredicateQuery.java index bb0b33e8522..13f0c268f15 100644 --- a/predicate-search/src/main/java/com/yahoo/search/predicate/PredicateQuery.java +++ b/predicate-search/src/main/java/com/yahoo/search/predicate/PredicateQuery.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.predicate; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import java.util.ArrayList; import java.util.List; diff --git a/searchcore/src/vespa/searchcore/config/proton.def b/searchcore/src/vespa/searchcore/config/proton.def index 5857fdd4f8d..87ee83945cb 100644 --- a/searchcore/src/vespa/searchcore/config/proton.def +++ b/searchcore/src/vespa/searchcore/config/proton.def @@ -144,7 +144,7 @@ indexing.semiunboundtasklimit int default = 1000 indexing.kind_of_watermark int default = 0 restart ## Controls minimum reaction time in seconds if using THROUGHPUT -indexing.reactiontime double default = 0.005 restart +indexing.reactiontime double default = 0.002 restart ## How long a freshly loaded index shall be warmed up diff --git a/searchcore/src/vespa/searchcore/proton/docsummary/summarycompacttarget.cpp b/searchcore/src/vespa/searchcore/proton/docsummary/summarycompacttarget.cpp index 6d71b81cb8b..4e0cf3f9059 100644 --- a/searchcore/src/vespa/searchcore/proton/docsummary/summarycompacttarget.cpp +++ b/searchcore/src/vespa/searchcore/proton/docsummary/summarycompacttarget.cpp @@ -56,8 +56,8 @@ SummaryCompactTarget::getApproxMemoryGain() const IFlushTarget::DiskGain SummaryCompactTarget::getApproxDiskGain() const { - uint64_t total(_docStore.getDiskFootprint()); - return DiskGain(total, total - std::min(total, static_cast<uint64_t>(_docStore.getMaxCompactGain()))); + size_t total(_docStore.getDiskFootprint()); + return DiskGain(total, total - std::min(total, _docStore.getMaxCompactGain())); } IFlushTarget::Time diff --git a/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/ElementCompleteness.java b/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/ElementCompleteness.java index b86cc62d3d1..af26a423906 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/ElementCompleteness.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/ElementCompleteness.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.ranking.features; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.searchlib.rankingexpression.evaluation.DoubleValue; import com.yahoo.searchlib.rankingexpression.evaluation.Value; diff --git a/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/Features.java b/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/Features.java index e004564d385..ee8f22b2e36 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/Features.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/Features.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.ranking.features; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.searchlib.rankingexpression.evaluation.Value; import java.util.Collections; diff --git a/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/FieldTermMatch.java b/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/FieldTermMatch.java index 9f86dadc378..752ef137ead 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/FieldTermMatch.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/FieldTermMatch.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.ranking.features; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.searchlib.rankingexpression.evaluation.DoubleValue; import com.yahoo.searchlib.rankingexpression.evaluation.Value; diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/FeatureList.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/FeatureList.java index 6e752e4a168..ad24e89c1f7 100755 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/FeatureList.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/FeatureList.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.rankingexpression; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.searchlib.rankingexpression.parser.ParseException; import com.yahoo.searchlib.rankingexpression.parser.RankingExpressionParser; import com.yahoo.searchlib.rankingexpression.parser.TokenMgrException; diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/TensorValue.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/TensorValue.java index fb195d14040..b37bbb543eb 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/TensorValue.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/TensorValue.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.rankingexpression.evaluation; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.searchlib.rankingexpression.rule.Function; import com.yahoo.searchlib.rankingexpression.rule.TruthOperator; import com.yahoo.tensor.Tensor; diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/TensorFunctionNode.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/TensorFunctionNode.java index 2d06e171097..ba5a243464e 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/TensorFunctionNode.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/TensorFunctionNode.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.rankingexpression.rule; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.google.common.collect.ImmutableMap; import com.yahoo.searchlib.rankingexpression.ExpressionFunction; import com.yahoo.searchlib.rankingexpression.Reference; diff --git a/searchlib/src/tests/transactionlog/CMakeLists.txt b/searchlib/src/tests/transactionlog/CMakeLists.txt index b09271eefe2..0904dc3ee36 100644 --- a/searchlib/src/tests/transactionlog/CMakeLists.txt +++ b/searchlib/src/tests/transactionlog/CMakeLists.txt @@ -5,8 +5,7 @@ vespa_add_executable(searchlib_translogclient_test_app TEST DEPENDS searchlib ) -vespa_add_test(NAME searchlib_translogclient_test_app COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/translogclient_test.sh - DEPENDS searchlib_translogclient_test_app COST 100) +vespa_add_test(NAME searchlib_translogclient_test_app COMMAND searchlib_translogclient_test_app) vespa_add_executable(searchlib_translog_chunks_test_app TEST SOURCES diff --git a/searchlib/src/tests/transactionlog/translogclient_test.cpp b/searchlib/src/tests/transactionlog/translogclient_test.cpp index ab5d432ddfb..5740eeb610d 100644 --- a/searchlib/src/tests/transactionlog/translogclient_test.cpp +++ b/searchlib/src/tests/transactionlog/translogclient_test.cpp @@ -1,10 +1,12 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <vespa/searchlib/transactionlog/translogclient.h> #include <vespa/searchlib/transactionlog/translogserver.h> +#include <vespa/searchlib/test/directory_handler.h> #include <vespa/vespalib/testkit/testapp.h> #include <vespa/vespalib/objects/identifiable.h> #include <vespa/searchlib/index/dummyfileheadercontext.h> #include <vespa/document/util/bytebuffer.h> +#include <vespa/vespalib/util/exceptions.h> #include <vespa/fastos/file.h> #include <thread> @@ -33,8 +35,8 @@ void fillDomainTest(Session * s1, size_t numPackets, size_t numEntries, size_t e uint32_t countFiles(const vespalib::string &dir); void checkFilledDomainTest(Session &s1, size_t numEntries); bool visitDomainTest(TransLogClient & tls, Session * s1, const vespalib::string & name); -void createAndFillDomain(const vespalib::string & name, Encoding encoding, size_t preExistingDomains); -void verifyDomain(const vespalib::string & name); +void createAndFillDomain(const vespalib::string & dir, const vespalib::string & name, Encoding encoding, size_t preExistingDomains); +void verifyDomain(const vespalib::string & dir, const vespalib::string & name); vespalib::string myhex(const void * b, size_t sz) @@ -50,6 +52,12 @@ myhex(const void * b, size_t sz) return s; } +DomainConfig +createDomainConfig(uint32_t partSizeLimit) { + return DomainConfig().setPartSizeLimit(partSizeLimit) + .setEncoding(Encoding(Encoding::xxh64, Encoding::none_multi)); +} + class CallBackTest : public Callback { private: @@ -350,7 +358,7 @@ void fillDomainTest(Session * s1, size_t numPackets, size_t numEntries, size_t entrySize) { size_t value(0); - std::vector<char> entryBuffer(entrySize); + std::vector<char> entryBuffer(entrySize); for(size_t i=0; i < numPackets; i++) { std::unique_ptr<Packet> p(new Packet(DEFAULT_PACKET_SIZE)); for(size_t j=0; j < numEntries; j++, value++) { @@ -457,11 +465,12 @@ getMaxSessionRunTime(TransLogServer &tls, const vespalib::string &domain) return tls.getDomainStats()[domain].maxSessionRunTime.count(); } -void createAndFillDomain(const vespalib::string & name, Encoding encoding, size_t preExistingDomains) +void +createAndFillDomain(const vespalib::string & dir, const vespalib::string & name, Encoding encoding, size_t preExistingDomains) { DummyFileHeaderContext fileHeaderContext; - TransLogServer tlss("test13", 18377, ".", fileHeaderContext, - DomainConfig().setPartSizeLimit(0x1000000).setEncoding(encoding), 4); + TransLogServer tlss(dir, 18377, ".", fileHeaderContext, + createDomainConfig(0x1000000).setEncoding(encoding), 4); TransLogClient tls("tcp/localhost:18377"); createDomainTest(tls, name, preExistingDomains); @@ -469,19 +478,21 @@ void createAndFillDomain(const vespalib::string & name, Encoding encoding, size_ fillDomainTest(s1.get(), name); } -void verifyDomain(const vespalib::string & name) { +void +verifyDomain(const vespalib::string & dir, const vespalib::string & name) { DummyFileHeaderContext fileHeaderContext; - TransLogServer tlss("test13", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x1000000)); + TransLogServer tlss(dir, 18377, ".", fileHeaderContext, createDomainConfig(0x1000000)); TransLogClient tls("tcp/localhost:18377"); auto s1 = openDomainTest(tls, name); visitDomainTest(tls, s1.get(), name); } -} -TEST("testVisitOverGeneratedDomain") { + +void +testVisitOverGeneratedDomain(const vespalib::string & testDir) { DummyFileHeaderContext fileHeaderContext; - TransLogServer tlss("test7", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x10000)); + TransLogServer tlss(testDir, 18377, ".", fileHeaderContext, createDomainConfig(0x10000)); TransLogClient tls("tcp/localhost:18377"); vespalib::string name("test1"); @@ -495,10 +506,11 @@ TEST("testVisitOverGeneratedDomain") { EXPECT_GREATER(maxSessionRunTime, 0); } -TEST("testVisitOverPreExistingDomain") { +void +testVisitOverPreExistingDomain(const vespalib::string & testDir) { // Depends on Test::testVisitOverGeneratedDomain() DummyFileHeaderContext fileHeaderContext; - TransLogServer tlss("test7", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x10000)); + TransLogServer tlss(testDir, 18377, ".", fileHeaderContext, createDomainConfig(0x10000)); TransLogClient tls("tcp/localhost:18377"); vespalib::string name("test1"); @@ -506,9 +518,10 @@ TEST("testVisitOverPreExistingDomain") { visitDomainTest(tls, s1.get(), name); } -TEST("partialUpdateTest") { +void +partialUpdateTest(const vespalib::string & testDir) { DummyFileHeaderContext fileHeaderContext; - TransLogServer tlss("test7", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x10000)); + TransLogServer tlss(testDir, 18377, ".", fileHeaderContext, createDomainConfig(0x10000)); TransLogClient tls("tcp/localhost:18377"); auto s1 = openDomainTest(tls, "test1"); @@ -561,17 +574,33 @@ TEST("partialUpdateTest") { ASSERT_TRUE( ca3.hasSerial(7) ); } +} + +TEST("testVisitAndUpdates") { + test::DirectoryHandler testDir("test7"); + testVisitOverGeneratedDomain(testDir.getDir()); + testVisitOverPreExistingDomain(testDir.getDir()); + partialUpdateTest(testDir.getDir()); +} + + TEST("testCrcVersions") { - createAndFillDomain("ccitt_crc32", Encoding(Encoding::Crc::ccitt_crc32, Encoding::Compression::none), 0); - createAndFillDomain("xxh64", Encoding(Encoding::Crc::xxh64, Encoding::Compression::none), 1); + test::DirectoryHandler testDir("test13"); + try { + createAndFillDomain(testDir.getDir(),"ccitt_crc32", Encoding(Encoding::Crc::ccitt_crc32, Encoding::Compression::none), 0); + ASSERT_TRUE(false); + } catch (vespalib::IllegalArgumentException & e) { + EXPECT_TRUE(e.getMessage().find("Compression:none is not allowed for the tls") != vespalib::string::npos); + } + createAndFillDomain(testDir.getDir(), "xxh64", Encoding(Encoding::Crc::xxh64, Encoding::Compression::zstd), 0); - verifyDomain("ccitt_crc32"); - verifyDomain("xxh64"); + verifyDomain(testDir.getDir(), "xxh64"); } TEST("testRemove") { + test::DirectoryHandler testDir("testremove"); DummyFileHeaderContext fileHeaderContext; - TransLogServer tlss("testremove", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x10000)); + TransLogServer tlss(testDir.getDir(), 18377, ".", fileHeaderContext, createDomainConfig(0x10000)); TransLogClient tls("tcp/localhost:18377"); vespalib::string name("test-delete"); @@ -618,14 +647,15 @@ assertStatus(Session &s, SerialNum expFirstSerial, SerialNum expLastSerial, uint } -TEST("test sending a lot of data") { +void + testSendingAlotOfDataSync(const vespalib::string & testDir) { const unsigned int NUM_PACKETS = 1000; const unsigned int NUM_ENTRIES = 100; const unsigned int TOTAL_NUM_ENTRIES = NUM_PACKETS * NUM_ENTRIES; const vespalib::string MANY("many"); { DummyFileHeaderContext fileHeaderContext; - TransLogServer tlss("test8", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x80000)); + TransLogServer tlss(testDir, 18377, ".", fileHeaderContext, createDomainConfig(0x80000)); TransLogClient tls("tcp/localhost:18377"); createDomainTest(tls, MANY, 0); @@ -648,7 +678,7 @@ TEST("test sending a lot of data") { } { DummyFileHeaderContext fileHeaderContext; - TransLogServer tlss("test8", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x1000000)); + TransLogServer tlss(testDir, 18377, ".", fileHeaderContext, createDomainConfig(0x1000000)); TransLogClient tls("tcp/localhost:18377"); auto s1 = openDomainTest(tls, "many"); @@ -669,7 +699,7 @@ TEST("test sending a lot of data") { } { DummyFileHeaderContext fileHeaderContext; - TransLogServer tlss("test8", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x1000000)); + TransLogServer tlss(testDir, 18377, ".", fileHeaderContext, createDomainConfig(0x1000000)); TransLogClient tls("tcp/localhost:18377"); auto s1 = openDomainTest(tls, MANY); @@ -690,14 +720,14 @@ TEST("test sending a lot of data") { } } -TEST("test sending a lot of data async") { +void testSendingAlotOfDataAsync(const vespalib::string & testDir) { const unsigned int NUM_PACKETS = 1000; const unsigned int NUM_ENTRIES = 100; const unsigned int TOTAL_NUM_ENTRIES = NUM_PACKETS * NUM_ENTRIES; const vespalib::string MANY("many-async"); { DummyFileHeaderContext fileHeaderContext; - TransLogServer tlss("test8", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x80000)); + TransLogServer tlss(testDir, 18377, ".", fileHeaderContext, createDomainConfig(0x80000)); TransLogClient tls("tcp/localhost:18377"); createDomainTest(tls, MANY, 1); auto s1 = openDomainTest(tls, MANY); @@ -719,7 +749,7 @@ TEST("test sending a lot of data async") { } { DummyFileHeaderContext fileHeaderContext; - TransLogServer tlss("test8", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x1000000)); + TransLogServer tlss(testDir, 18377, ".", fileHeaderContext, createDomainConfig(0x1000000)); TransLogClient tls("tcp/localhost:18377"); auto s1 = openDomainTest(tls, MANY); @@ -740,16 +770,21 @@ TEST("test sending a lot of data async") { } } - +TEST("test sending a lot of data both sync and async") { + test::DirectoryHandler testDir("test8"); + testSendingAlotOfDataSync(testDir.getDir()); + testSendingAlotOfDataAsync(testDir.getDir()); +} TEST("testErase") { const unsigned int NUM_PACKETS = 1000; const unsigned int NUM_ENTRIES = 100; const unsigned int TOTAL_NUM_ENTRIES = NUM_PACKETS * NUM_ENTRIES; + test::DirectoryHandler testDir("test12"); { DummyFileHeaderContext fileHeaderContext; - TransLogServer tlss("test12", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x80000)); + TransLogServer tlss(testDir.getDir(), 18377, ".", fileHeaderContext, createDomainConfig(0x80000)); TransLogClient tls("tcp/localhost:18377"); createDomainTest(tls, "erase", 0); @@ -758,7 +793,7 @@ TEST("testErase") { } { DummyFileHeaderContext fileHeaderContext; - TransLogServer tlss("test12", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x1000000)); + TransLogServer tlss(testDir.getDir(), 18377, ".", fileHeaderContext, createDomainConfig(0x1000000)); TransLogClient tls("tcp/localhost:18377"); auto s1 = openDomainTest(tls, "erase"); @@ -845,7 +880,8 @@ TEST("testSync") { const unsigned int TOTAL_NUM_ENTRIES = NUM_PACKETS * NUM_ENTRIES; DummyFileHeaderContext fileHeaderContext; - TransLogServer tlss("test9", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x1000000)); + test::DirectoryHandler testDir("test9"); + TransLogServer tlss(testDir.getDir(), 18377, ".", fileHeaderContext, createDomainConfig(0x1000000)); TransLogClient tls("tcp/localhost:18377"); createDomainTest(tls, "sync", 0); @@ -866,8 +902,9 @@ TEST("test truncate on version mismatch") { uint64_t fromOld(0), toOld(0); size_t countOld(0); DummyFileHeaderContext fileHeaderContext; + test::DirectoryHandler testDir("test11"); { - TransLogServer tlss("test11", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x1000000)); + TransLogServer tlss(testDir.getDir(), 18377, ".", fileHeaderContext, createDomainConfig(0x1000000)); TransLogClient tls("tcp/localhost:18377"); createDomainTest(tls, "sync", 0); @@ -879,7 +916,7 @@ TEST("test truncate on version mismatch") { EXPECT_TRUE(s1->sync(2, syncedTo)); EXPECT_EQUAL(syncedTo, TOTAL_NUM_ENTRIES); } - FastOS_File f("test11/sync/sync-0000000000000000"); + FastOS_File f((testDir.getDir() + "/sync/sync-0000000000000000").c_str()); EXPECT_TRUE(f.OpenWriteOnlyExisting()); EXPECT_TRUE(f.SetPosition(f.GetSize())); @@ -888,7 +925,7 @@ TEST("test truncate on version mismatch") { EXPECT_EQUAL(static_cast<ssize_t>(sizeof(tmp)), f.Write2(tmp, sizeof(tmp))); EXPECT_TRUE(f.Close()); { - TransLogServer tlss("test11", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x10000)); + TransLogServer tlss(testDir.getDir(), 18377, ".", fileHeaderContext, createDomainConfig(0x10000)); TransLogClient tls("tcp/localhost:18377"); auto s1 = openDomainTest(tls, "sync"); uint64_t from(0), to(0); @@ -905,14 +942,16 @@ TEST("test truncation after short read") { const unsigned int NUM_ENTRIES = 1; const unsigned int TOTAL_NUM_ENTRIES = NUM_PACKETS * NUM_ENTRIES; const unsigned int ENTRYSIZE = 4080; - vespalib::string topdir("test10"); + test::DirectoryHandler topdir("test10"); vespalib::string domain("truncate"); - vespalib::string dir(topdir + "/" + domain); + vespalib::string dir(topdir.getDir() + "/" + domain); vespalib::string tlsspec("tcp/localhost:18377"); + + DomainConfig domainConfig = createDomainConfig(0x10000); DummyFileHeaderContext fileHeaderContext; { - TransLogServer tlss(topdir, 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x10000)); + TransLogServer tlss(topdir.getDir(), 18377, ".", fileHeaderContext, domainConfig); TransLogClient tls(tlsspec); createDomainTest(tls, domain, 0); @@ -924,18 +963,14 @@ TEST("test truncation after short read") { EXPECT_TRUE(s1->sync(TOTAL_NUM_ENTRIES, syncedTo)); EXPECT_EQUAL(syncedTo, TOTAL_NUM_ENTRIES); } + EXPECT_EQUAL(2u, countFiles(dir)); { - EXPECT_EQUAL(2u, countFiles(dir)); - } - { - TransLogServer tlss(topdir, 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x10000)); + TransLogServer tlss(topdir.getDir(), 18377, ".", fileHeaderContext, domainConfig); TransLogClient tls(tlsspec); auto s1 = openDomainTest(tls, domain); checkFilledDomainTest(*s1, TOTAL_NUM_ENTRIES); } - { - EXPECT_EQUAL(2u, countFiles(dir)); - } + EXPECT_EQUAL(2u, countFiles(dir)); { vespalib::string filename(dir + "/truncate-0000000000000017"); FastOS_File trfile(filename.c_str()); @@ -944,14 +979,12 @@ TEST("test truncation after short read") { trfile.Close(); } { - TransLogServer tlss(topdir, 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x10000)); + TransLogServer tlss(topdir.getDir(), 18377, ".", fileHeaderContext, domainConfig); TransLogClient tls(tlsspec); auto s1 = openDomainTest(tls, domain); checkFilledDomainTest(*s1, TOTAL_NUM_ENTRIES - 1); } - { - EXPECT_EQUAL(2u, countFiles(dir)); - } + EXPECT_EQUAL(2u, countFiles(dir)); } TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/transactionlog/translogclient_test.sh b/searchlib/src/tests/transactionlog/translogclient_test.sh deleted file mode 100755 index 50d7c73fd6a..00000000000 --- a/searchlib/src/tests/transactionlog/translogclient_test.sh +++ /dev/null @@ -1,6 +0,0 @@ -#!/bin/bash -# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -set -e -rm -rf test7 test8 test9 test10 test11 test12 test13 testremove -$VALGRIND ./searchlib_translogclient_test_app -rm -rf test7 test8 test9 test10 test11 test12 test13 testremove diff --git a/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp b/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp index 4da09868216..8c76a4477f5 100644 --- a/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp +++ b/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp @@ -483,12 +483,20 @@ WriteableFileChunk::writeData(const ProcessedChunkQ & chunks, size_t sz) void WriteableFileChunk::updateChunkInfo(const ProcessedChunkQ & chunks, const ChunkMetaV & cmetaV, size_t sz) { + uint32_t maxChunkId(0); + for (const auto & chunk : chunks) { + maxChunkId = std::max(chunk->getChunkId(), maxChunkId); + } std::lock_guard guard(_lock); + if (maxChunkId >= _chunkInfo.size()) { + _chunkInfo.reserve(vespalib::roundUp2inN(maxChunkId+1)); + } size_t nettoSz(sz); for (size_t i(0); i < chunks.size(); i++) { const ProcessedChunk & chunk = *chunks[i]; assert(_chunkMap.find(chunk.getChunkId()) == _chunkMap.begin()); const Chunk & active = *_chunkMap.begin()->second; + assert(active.getId() == chunk.getChunkId()); if (active.getId() >= _chunkInfo.size()) { _chunkInfo.resize(active.getId()+1); } diff --git a/searchlib/src/vespa/searchlib/transactionlog/chunks.cpp b/searchlib/src/vespa/searchlib/transactionlog/chunks.cpp index ac17c47ef38..cdd58adf005 100644 --- a/searchlib/src/vespa/searchlib/transactionlog/chunks.cpp +++ b/searchlib/src/vespa/searchlib/transactionlog/chunks.cpp @@ -43,9 +43,8 @@ toCompression(CompressionConfig::Type type) { case CompressionConfig::LZ4: return Encoding::Compression::lz4; case CompressionConfig::NONE_MULTI: - return Encoding::Compression::none_multi; case CompressionConfig::NONE: - return Encoding::Compression::none; + return Encoding::Compression::none_multi; default: abort(); } @@ -114,9 +113,6 @@ XXH64CompressedChunk::compress(nbostream & os, Encoding::Crc crc) const { CompressionConfig cfg(_type, _level, 80, 200); ConstBufferRef uncompressed(org.data(), org.size()); Encoding::Compression actual = toCompression(::compress(cfg, uncompressed, compressed, false)); - if (actual == Encoding::Compression::none) { - actual = Encoding::Compression::none_multi; - } os << uint32_t(uncompressed.size()); size_t start = os.wp(); os.write(compressed.getData(), compressed.getDataLen()); diff --git a/searchlib/src/vespa/searchlib/transactionlog/domain.cpp b/searchlib/src/vespa/searchlib/transactionlog/domain.cpp index 0e6cdffa91c..24943b53e6d 100644 --- a/searchlib/src/vespa/searchlib/transactionlog/domain.cpp +++ b/searchlib/src/vespa/searchlib/transactionlog/domain.cpp @@ -403,12 +403,10 @@ void Domain::doCommit(std::unique_ptr<CommitChunk> chunk) { const Packet & packet = chunk->getPacket(); if (packet.empty()) return; - - vespalib::nbostream_longlivedbuf is(packet.getHandle().data(), packet.getHandle().size()); - Packet::Entry entry; - entry.deserialize(is); - DomainPart::SP dp = optionallyRotateFile(entry.serial()); - dp->commit(entry.serial(), packet); + + SerialNum firstSerial = packet.range().from(); + DomainPart::SP dp = optionallyRotateFile(firstSerial); + dp->commit(firstSerial, packet); if (_config.getFSyncOnCommit()) { dp->sync(); } diff --git a/searchlib/src/vespa/searchlib/transactionlog/domainconfig.cpp b/searchlib/src/vespa/searchlib/transactionlog/domainconfig.cpp index b4ab8a1c791..1f414eaa6d3 100644 --- a/searchlib/src/vespa/searchlib/transactionlog/domainconfig.cpp +++ b/searchlib/src/vespa/searchlib/transactionlog/domainconfig.cpp @@ -1,15 +1,25 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "domainconfig.h" +#include <vespa/vespalib/util/exceptions.h> namespace search::transactionlog { DomainConfig::DomainConfig() - : _encoding(Encoding::Crc::xxh64, Encoding::Compression::none), + : _encoding(Encoding::Crc::xxh64, Encoding::Compression::zstd), _compressionLevel(9), _fSyncOnCommit(false), _partSizeLimit(0x10000000), // 256M _chunkSizeLimit(0x40000) // 256k { } +DomainConfig & +DomainConfig::setEncoding(Encoding v) { + if (v.getCompression() == Encoding::none) { + throw vespalib::IllegalArgumentException("Compression:none is not allowed for the tls", VESPA_STRLOC); + } + _encoding = v; + return *this; +} + } diff --git a/searchlib/src/vespa/searchlib/transactionlog/domainconfig.h b/searchlib/src/vespa/searchlib/transactionlog/domainconfig.h index 186227ae958..7701896fa92 100644 --- a/searchlib/src/vespa/searchlib/transactionlog/domainconfig.h +++ b/searchlib/src/vespa/searchlib/transactionlog/domainconfig.h @@ -11,7 +11,7 @@ class DomainConfig { public: using duration = vespalib::duration; DomainConfig(); - DomainConfig & setEncoding(Encoding v) { _encoding = v; return *this; } + DomainConfig & setEncoding(Encoding v); DomainConfig & setPartSizeLimit(size_t v) { _partSizeLimit = v; return *this; } DomainConfig & setChunkSizeLimit(size_t v) { _chunkSizeLimit = v; return *this; } DomainConfig & setCompressionLevel(uint8_t v) { _compressionLevel = v; return *this; } diff --git a/searchlib/src/vespa/searchlib/transactionlog/domainpart.cpp b/searchlib/src/vespa/searchlib/transactionlog/domainpart.cpp index ee575820cce..3dad67df177 100644 --- a/searchlib/src/vespa/searchlib/transactionlog/domainpart.cpp +++ b/searchlib/src/vespa/searchlib/transactionlog/domainpart.cpp @@ -395,10 +395,7 @@ DomainPart::commit(SerialNum firstSerial, const Packet &packet) entry.deserialize(h); if (_range.to() < entry.serial()) { chunk->add(entry); - if (_encoding.getCompression() == Encoding::Compression::none) { - write(*_transLog, *chunk); - chunk = IChunk::create(_encoding, _compressionLevel); - } + assert(_encoding.getCompression() != Encoding::Compression::none); _sz++; _range.to(entry.serial()); } else { diff --git a/searchlib/src/vespa/searchlib/transactionlog/translogserver.cpp b/searchlib/src/vespa/searchlib/transactionlog/translogserver.cpp index 08ee944e749..ce190d2c093 100644 --- a/searchlib/src/vespa/searchlib/transactionlog/translogserver.cpp +++ b/searchlib/src/vespa/searchlib/transactionlog/translogserver.cpp @@ -82,7 +82,7 @@ VESPA_THREAD_STACK_TAG(tls_executor); TransLogServer::TransLogServer(const vespalib::string &name, int listenPort, const vespalib::string &baseDir, const FileHeaderContext &fileHeaderContext) : TransLogServer(name, listenPort, baseDir, fileHeaderContext, - DomainConfig().setEncoding(Encoding(Encoding::xxh64, Encoding::Compression::none)) + DomainConfig().setEncoding(Encoding(Encoding::xxh64, Encoding::Compression::zstd)) .setPartSizeLimit(0x10000000).setChunkSizeLimit(0x40000)) {} diff --git a/searchlib/src/vespa/searchlib/transactionlog/translogserverapp.cpp b/searchlib/src/vespa/searchlib/transactionlog/translogserverapp.cpp index 2d2863af874..9ca3b678054 100644 --- a/searchlib/src/vespa/searchlib/transactionlog/translogserverapp.cpp +++ b/searchlib/src/vespa/searchlib/transactionlog/translogserverapp.cpp @@ -42,7 +42,6 @@ getCompression(searchlib::TranslogserverConfig::Compression::Type type) { switch (type) { case searchlib::TranslogserverConfig::Compression::Type::NONE: - return Encoding::Compression::none; case searchlib::TranslogserverConfig::Compression::Type::NONE_MULTI: return Encoding::Compression::none_multi; case searchlib::TranslogserverConfig::Compression::Type::LZ4: diff --git a/staging_vespalib/src/vespa/vespalib/util/singleexecutor.cpp b/staging_vespalib/src/vespa/vespalib/util/singleexecutor.cpp index b25bc1a6377..a2962c6ea84 100644 --- a/staging_vespalib/src/vespa/vespalib/util/singleexecutor.cpp +++ b/staging_vespalib/src/vespa/vespalib/util/singleexecutor.cpp @@ -89,7 +89,9 @@ SingleExecutor::drain(Lock & lock) { void SingleExecutor::wakeup() { - _consumerCondition.notify_one(); + if (numTasks() > 0) { + _consumerCondition.notify_one(); + } } SingleExecutor & diff --git a/storage/src/tests/persistence/persistencetestutils.h b/storage/src/tests/persistence/persistencetestutils.h index fc986c3c6f2..4bbff9bb2ca 100644 --- a/storage/src/tests/persistence/persistencetestutils.h +++ b/storage/src/tests/persistence/persistencetestutils.h @@ -50,6 +50,8 @@ public: api::LockingRequirements lockingRequirements() const noexcept override { return api::LockingRequirements::Shared; } + void signal_operation_sync_phase_done() noexcept override {} + bool wants_sync_phase_done_notification() const noexcept override { return false; } static std::shared_ptr<NoBucketLock> make(document::Bucket bucket) { return std::make_shared<NoBucketLock>(bucket); } @@ -78,6 +80,8 @@ public: api::LockingRequirements lockingRequirements() const noexcept override { return api::LockingRequirements::Exclusive; } + void signal_operation_sync_phase_done() noexcept override {} + bool wants_sync_phase_done_notification() const noexcept override { return false; } static std::shared_ptr<MockBucketLock> make(document::Bucket bucket, MockBucketLocks& locks) { return std::make_shared<MockBucketLock>(bucket, locks); } diff --git a/storage/src/vespa/storage/persistence/filestorage/filestorhandler.h b/storage/src/vespa/storage/persistence/filestorage/filestorhandler.h index 0d05fd21ce2..a980b5aa2e1 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestorhandler.h +++ b/storage/src/vespa/storage/persistence/filestorage/filestorhandler.h @@ -49,14 +49,29 @@ public: {} }; - class BucketLockInterface { + // Interface that is used for "early ACKing" a potentially longer-running async + // operation when the persistence thread processing the operation has completed + // the synchronous aspects of the operation (such as dispatching one or more + // async operations over the SPI). + class OperationSyncPhaseDoneNotifier { public: - using SP = std::shared_ptr<BucketLockInterface>; + virtual ~OperationSyncPhaseDoneNotifier() = default; + + // Informs the caller if the operation wants to know when the persistence thread is + // done with the synchronous aspects of the operation. Returning false allows the caller + // to optimize for the case where this does _not_ need to happen. + [[nodiscard]] virtual bool wants_sync_phase_done_notification() const noexcept = 0; + // Invoked at most once at the point where the persistence thread is done handling the synchronous + // aspects of the operation iff wants_sync_phase_done_notification() was initially true. + virtual void signal_operation_sync_phase_done() noexcept = 0; + }; - virtual const document::Bucket &getBucket() const = 0; - virtual api::LockingRequirements lockingRequirements() const noexcept = 0; + class BucketLockInterface : public OperationSyncPhaseDoneNotifier { + public: + using SP = std::shared_ptr<BucketLockInterface>; - virtual ~BucketLockInterface() = default; + [[nodiscard]] virtual const document::Bucket &getBucket() const = 0; + [[nodiscard]] virtual api::LockingRequirements lockingRequirements() const noexcept = 0; }; using LockedMessage = std::pair<BucketLockInterface::SP, api::StorageMessage::SP>; diff --git a/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp b/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp index f76d2693309..c6991803b4d 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp @@ -1054,7 +1054,8 @@ message_type_is_merge_related(api::MessageType::Id msg_type_id) { void FileStorHandlerImpl::Stripe::release(const document::Bucket & bucket, api::LockingRequirements reqOfReleasedLock, - api::StorageMessage::Id lockMsgId) + api::StorageMessage::Id lockMsgId, + bool was_active_merge) { std::unique_lock guard(*_lock); auto iter = _lockedBuckets.find(bucket); @@ -1065,7 +1066,7 @@ FileStorHandlerImpl::Stripe::release(const document::Bucket & bucket, if (reqOfReleasedLock == api::LockingRequirements::Exclusive) { assert(entry._exclusiveLock); assert(entry._exclusiveLock->msgId == lockMsgId); - if (message_type_is_merge_related(entry._exclusiveLock->msgType)) { + if (was_active_merge) { assert(_active_merges > 0); --_active_merges; } @@ -1089,13 +1090,27 @@ FileStorHandlerImpl::Stripe::release(const document::Bucket & bucket, } void +FileStorHandlerImpl::Stripe::decrease_active_sync_merges_counter() noexcept +{ + std::unique_lock guard(*_lock); + assert(_active_merges > 0); + const bool may_have_blocked_merge = (_active_merges == _owner._max_active_merges_per_stripe); + --_active_merges; + if (may_have_blocked_merge) { + guard.unlock(); + _cond->notify_all(); + } +} + +void FileStorHandlerImpl::Stripe::lock(const monitor_guard &, const document::Bucket & bucket, - api::LockingRequirements lockReq, const LockEntry & lockEntry) { + api::LockingRequirements lockReq, bool count_as_active_merge, + const LockEntry & lockEntry) { auto& entry = _lockedBuckets[bucket]; assert(!entry._exclusiveLock); if (lockReq == api::LockingRequirements::Exclusive) { assert(entry._sharedLocks.empty()); - if (message_type_is_merge_related(lockEntry.msgType)) { + if (count_as_active_merge) { ++_active_merges; } entry._exclusiveLock = lockEntry; @@ -1151,28 +1166,43 @@ FileStorHandlerImpl::Stripe::get_active_operations_stats(bool reset_min_max) con return result; } -FileStorHandlerImpl::BucketLock::BucketLock(const monitor_guard & guard, Stripe& stripe, - const document::Bucket &bucket, uint8_t priority, +FileStorHandlerImpl::BucketLock::BucketLock(const monitor_guard& guard, Stripe& stripe, + const document::Bucket& bucket, uint8_t priority, api::MessageType::Id msgType, api::StorageMessage::Id msgId, api::LockingRequirements lockReq) : _stripe(stripe), _bucket(bucket), _uniqueMsgId(msgId), - _lockReq(lockReq) + _lockReq(lockReq), + _counts_towards_merge_limit(false) { if (_bucket.getBucketId().getRawId() != 0) { - _stripe.lock(guard, _bucket, lockReq, Stripe::LockEntry(priority, msgType, msgId)); + _counts_towards_merge_limit = message_type_is_merge_related(msgType); + _stripe.lock(guard, _bucket, lockReq, _counts_towards_merge_limit, Stripe::LockEntry(priority, msgType, msgId)); LOG(spam, "Locked bucket %s for message %" PRIu64 " with priority %u in mode %s", - bucket.getBucketId().toString().c_str(), msgId, priority, api::to_string(lockReq)); + bucket.toString().c_str(), msgId, priority, api::to_string(lockReq)); } } FileStorHandlerImpl::BucketLock::~BucketLock() { if (_bucket.getBucketId().getRawId() != 0) { - _stripe.release(_bucket, _lockReq, _uniqueMsgId); + _stripe.release(_bucket, _lockReq, _uniqueMsgId, _counts_towards_merge_limit); LOG(spam, "Unlocked bucket %s for message %" PRIu64 " in mode %s", - _bucket.getBucketId().toString().c_str(), _uniqueMsgId, api::to_string(_lockReq)); + _bucket.toString().c_str(), _uniqueMsgId, api::to_string(_lockReq)); + } +} + +void +FileStorHandlerImpl::BucketLock::signal_operation_sync_phase_done() noexcept +{ + // Not atomic, only destructor can read/write this other than this function, and since + // a strong ref must already be held to this object by the caller, we cannot race with it. + if (_counts_towards_merge_limit){ + LOG(spam, "Synchronous phase for bucket %s is done; reducing active count proactively", + _bucket.toString().c_str()); + _stripe.decrease_active_sync_merges_counter(); + _counts_towards_merge_limit = false; } } @@ -1278,10 +1308,10 @@ FileStorHandlerImpl::getStatus(std::ostream& out, const framework::HttpUrlPath& } std::lock_guard mergeGuard(_mergeStatesLock); - out << "<tr><td>Active merge operations</td><td>" << _mergeStates.size() << "</td></tr>\n"; + out << "<p>Active merge operations: " << _mergeStates.size() << "</p>\n"; if (verbose) { out << "<h4>Active merges</h4>\n"; - if (_mergeStates.size() == 0) { + if (_mergeStates.empty()) { out << "None\n"; } for (auto & entry : _mergeStates) { diff --git a/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.h b/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.h index 3a89ff74f07..5d68be8a800 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.h +++ b/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.h @@ -115,7 +115,8 @@ public: return _queue->size(); } void release(const document::Bucket & bucket, api::LockingRequirements reqOfReleasedLock, - api::StorageMessage::Id lockMsgId); + api::StorageMessage::Id lockMsgId, bool was_active_merge); + void decrease_active_sync_merges_counter() noexcept; // Subsumes isLocked bool operationIsInhibited(const monitor_guard &, const document::Bucket&, @@ -124,7 +125,8 @@ public: api::LockingRequirements lockReq) const noexcept; void lock(const monitor_guard &, const document::Bucket & bucket, - api::LockingRequirements lockReq, const LockEntry & lockEntry); + api::LockingRequirements lockReq, bool count_as_active_merge, + const LockEntry & lockEntry); std::shared_ptr<FileStorHandler::BucketLockInterface> lock(const document::Bucket & bucket, api::LockingRequirements lockReq); void failOperations(const document::Bucket & bucket, const api::ReturnCode & code); @@ -168,12 +170,17 @@ public: const document::Bucket &getBucket() const override { return _bucket; } api::LockingRequirements lockingRequirements() const noexcept override { return _lockReq; } + void signal_operation_sync_phase_done() noexcept override; + bool wants_sync_phase_done_notification() const noexcept override { + return _counts_towards_merge_limit; + } private: Stripe & _stripe; - document::Bucket _bucket; + const document::Bucket _bucket; api::StorageMessage::Id _uniqueMsgId; api::LockingRequirements _lockReq; + bool _counts_towards_merge_limit; }; diff --git a/storage/src/vespa/storage/persistence/persistencehandler.cpp b/storage/src/vespa/storage/persistence/persistencehandler.cpp index 3c981e193f2..8b546771b71 100644 --- a/storage/src/vespa/storage/persistence/persistencehandler.cpp +++ b/storage/src/vespa/storage/persistence/persistencehandler.cpp @@ -29,9 +29,29 @@ PersistenceHandler::PersistenceHandler(vespalib::ISequencedTaskExecutor & sequen PersistenceHandler::~PersistenceHandler() = default; +// Guard that allows an operation that may be executed in an async fashion to +// be explicitly notified when the sync phase of the operation is done, i.e. +// when the persistence thread is no longer working on it. An operation that +// does not care about such notifications can safely return a nullptr notifier, +// in which case the guard is a no-op. +class OperationSyncPhaseTrackingGuard { + std::shared_ptr<FileStorHandler::OperationSyncPhaseDoneNotifier> _maybe_notifier; +public: + explicit OperationSyncPhaseTrackingGuard(const MessageTracker& tracker) + : _maybe_notifier(tracker.sync_phase_done_notifier_or_nullptr()) + {} + + ~OperationSyncPhaseTrackingGuard() { + if (_maybe_notifier) { + _maybe_notifier->signal_operation_sync_phase_done(); + } + } +}; + MessageTracker::UP PersistenceHandler::handleCommandSplitByType(api::StorageCommand& msg, MessageTracker::UP tracker) const { + OperationSyncPhaseTrackingGuard sync_guard(*tracker); switch (msg.getType().getId()) { case api::MessageType::GET_ID: return _simpleHandler.handleGet(static_cast<api::GetCommand&>(msg), std::move(tracker)); diff --git a/storage/src/vespa/storage/persistence/persistenceutil.cpp b/storage/src/vespa/storage/persistence/persistenceutil.cpp index 3f3d59c11aa..cbfc9463a8c 100644 --- a/storage/src/vespa/storage/persistence/persistenceutil.cpp +++ b/storage/src/vespa/storage/persistence/persistenceutil.cpp @@ -155,6 +155,15 @@ MessageTracker::generateReply(api::StorageCommand& cmd) } } +std::shared_ptr<FileStorHandler::OperationSyncPhaseDoneNotifier> +MessageTracker::sync_phase_done_notifier_or_nullptr() const +{ + if (_bucketLock->wants_sync_phase_done_notification()) { + return _bucketLock; + } + return {}; +} + PersistenceUtil::PersistenceUtil(const ServiceLayerComponent& component, FileStorHandler& fileStorHandler, FileStorThreadMetrics& metrics, spi::PersistenceProvider& provider) : _component(component), diff --git a/storage/src/vespa/storage/persistence/persistenceutil.h b/storage/src/vespa/storage/persistence/persistenceutil.h index 4cc2657ea56..4fd0e60c730 100644 --- a/storage/src/vespa/storage/persistence/persistenceutil.h +++ b/storage/src/vespa/storage/persistence/persistenceutil.h @@ -27,7 +27,7 @@ class PersistenceUtil; class MessageTracker : protected Types { public: - typedef std::unique_ptr<MessageTracker> UP; + using UP = std::unique_ptr<MessageTracker>; MessageTracker(const framework::MilliSecTimer & timer, const PersistenceUtil & env, MessageSender & replySender, FileStorHandler::BucketLockInterface::SP bucketLock, std::shared_ptr<api::StorageMessage> msg); @@ -81,6 +81,10 @@ public: bool checkForError(const spi::Result& response); + // Returns a non-nullptr notifier instance iff the underlying operation wants to be notified + // when the sync phase is complete. Otherwise returns a nullptr shared_ptr. + std::shared_ptr<FileStorHandler::OperationSyncPhaseDoneNotifier> sync_phase_done_notifier_or_nullptr() const; + static MessageTracker::UP createForTesting(const framework::MilliSecTimer & timer, PersistenceUtil & env, MessageSender & replySender, FileStorHandler::BucketLockInterface::SP bucketLock, std::shared_ptr<api::StorageMessage> msg); diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java index ce12637ccb0..289a63a1128 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java @@ -300,7 +300,7 @@ public class DefaultZmsClient extends ClientBase implements ZmsClient { .filter(re -> AthenzIdentities.USER_PRINCIPAL_DOMAIN.equals(AthenzIdentities.from(re.memberName()).getDomain())) .collect(Collectors.toUnmodifiableMap( m -> (AthenzUser) AthenzIdentities.from(m.memberName()), - RoleEntity.Member::auditRef)); + m -> m.auditRef() != null ? m.auditRef() : "<no reason provided>")); } @Override diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/FeedParams.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/FeedParams.java index 19a28bbacaf..01f314a7e36 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/FeedParams.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/FeedParams.java @@ -1,8 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.http.client.config; -import com.google.common.annotations.Beta; - import java.util.concurrent.TimeUnit; /** @@ -54,7 +52,6 @@ public final class FeedParams { * @param silentUpgrade true for reducing "false" 4xx/5xx. * @return this, for chaining */ - @Beta public Builder setSilentUpgrade(boolean silentUpgrade) { this.silentUpgrade = silentUpgrade; return this; @@ -184,7 +181,6 @@ public final class FeedParams { /** * Set what frequency to poll for async responses. Default is 10hz (every 0.1s), but 1000hz when using SyncFeedClient */ - @Beta public Builder setIdlePollFrequency(Double idlePollFrequency) { this.idlePollFrequency = idlePollFrequency; return this; diff --git a/vespajlib/src/main/java/com/yahoo/geo/DistanceParser.java b/vespajlib/src/main/java/com/yahoo/geo/DistanceParser.java index 13bff614d83..acb7ed95597 100644 --- a/vespajlib/src/main/java/com/yahoo/geo/DistanceParser.java +++ b/vespajlib/src/main/java/com/yahoo/geo/DistanceParser.java @@ -2,7 +2,7 @@ package com.yahoo.geo; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; /** * Utility for parsing a geographical distance with unit. diff --git a/vespajlib/src/main/java/com/yahoo/io/reader/NamedReader.java b/vespajlib/src/main/java/com/yahoo/io/reader/NamedReader.java index 3129ddba638..f54caa225b0 100644 --- a/vespajlib/src/main/java/com/yahoo/io/reader/NamedReader.java +++ b/vespajlib/src/main/java/com/yahoo/io/reader/NamedReader.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.io.reader; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import java.io.IOException; import java.io.Reader; diff --git a/vespajlib/src/main/java/com/yahoo/path/Path.java b/vespajlib/src/main/java/com/yahoo/path/Path.java index 3254c081f31..12f93d15737 100644 --- a/vespajlib/src/main/java/com/yahoo/path/Path.java +++ b/vespajlib/src/main/java/com/yahoo/path/Path.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.path; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.google.common.collect.ImmutableList; import java.io.File; diff --git a/vespajlib/src/main/java/com/yahoo/tensor/functions/Slice.java b/vespajlib/src/main/java/com/yahoo/tensor/functions/Slice.java index 714e8deb0bb..a6f71dacf30 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/functions/Slice.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/functions/Slice.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.tensor.functions; -import com.google.common.annotations.Beta; +import com.yahoo.api.annotations.Beta; import com.yahoo.tensor.PartialAddress; import com.yahoo.tensor.Tensor; import com.yahoo.tensor.TensorAddress; diff --git a/vespalib/src/vespa/vespalib/btree/btreeiterator.hpp b/vespalib/src/vespa/vespalib/btree/btreeiterator.hpp index 2348e770e9d..042779f1b1b 100644 --- a/vespalib/src/vespa/vespalib/btree/btreeiterator.hpp +++ b/vespalib/src/vespa/vespalib/btree/btreeiterator.hpp @@ -983,6 +983,7 @@ moveFirstLeafNode(BTreeNode::Ref rootRef) InternalNodeTypeRefPair iPair = allocator.moveInternalNode(node); nodeRef = iPair.ref; node = iPair.data; + std::atomic_thread_fence(std::memory_order_release); pnode->setChild(0, nodeRef); moved = true; } @@ -994,6 +995,7 @@ moveFirstLeafNode(BTreeNode::Ref rootRef) LeafNodeTypeRefPair lPair(allocator.moveLeafNode(_leaf.getNode())); _leaf.setNode(lPair.data); + std::atomic_thread_fence(std::memory_order_release); node->setChild(0, lPair.ref); moved = true; } |