diff options
| author | Henning Baldersheim <balder@yahoo-inc.com> | 2023-12-20 22:27:30 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2023-12-20 22:27:30 +0100 |
| commit | fc4d21db12894fa8056f03c091b7cbc756da3d6f (patch) | |
| tree | e64b166dc7bdec39157d23d5c049b622c63ab92d | |
| parent | 65be04820cda0d06b8d2c7f7212f814a5ef434f3 (diff) | |
| parent | 57595d4ee49e13ad5183361b0c6e4c8a250c6d3f (diff) | |
Merge branch 'master' into balder/gc-unused-feature-flags
93 files changed, 739 insertions, 608 deletions
diff --git a/client/go/internal/cli/cmd/root.go b/client/go/internal/cli/cmd/root.go index 383ce7dd28d..8e0f3de4f72 100644 --- a/client/go/internal/cli/cmd/root.go +++ b/client/go/internal/cli/cmd/root.go @@ -69,6 +69,7 @@ type CLI struct { // the error. type ErrCLI struct { Status int + warn bool quiet bool hints []string error @@ -599,7 +600,11 @@ func (c *CLI) Run(args ...string) error { if err != nil { if cliErr, ok := err.(ErrCLI); ok { if !cliErr.quiet { - c.printErr(cliErr, cliErr.hints...) + if cliErr.warn { + c.printWarning(cliErr, cliErr.hints...) + } else { + c.printErr(cliErr, cliErr.hints...) + } } } else { c.printErr(err) diff --git a/client/go/internal/cli/cmd/status.go b/client/go/internal/cli/cmd/status.go index 29a4a5775db..6056ee439b2 100644 --- a/client/go/internal/cli/cmd/status.go +++ b/client/go/internal/cli/cmd/status.go @@ -176,7 +176,11 @@ $ vespa status deployment -t local [session-id] --wait 600 waiter := cli.waiter(time.Duration(waitSecs) * time.Second) id, err := waiter.Deployment(t, wantedID) if err != nil { - return err + var hints []string + if waiter.Timeout == 0 { + hints = []string{"Consider using the --wait flag to wait for completion"} + } + return ErrCLI{Status: 1, warn: true, hints: hints, error: err} } if t.IsCloud() { log.Printf("Deployment run %s has completed", color.CyanString(strconv.FormatInt(id, 10))) diff --git a/client/go/internal/cli/cmd/status_test.go b/client/go/internal/cli/cmd/status_test.go index 3c6277b9050..5ef96c462d8 100644 --- a/client/go/internal/cli/cmd/status_test.go +++ b/client/go/internal/cli/cmd/status_test.go @@ -85,7 +85,7 @@ func TestStatusError(t *testing.T) { cli.httpClient = client assert.NotNil(t, cli.Run("status", "container")) assert.Equal(t, - "Container default at http://127.0.0.1:8080 is not ready: unhealthy container default: status 500 at http://127.0.0.1:8080/status.html: wait timed out\n", + "Container default at http://127.0.0.1:8080 is not ready: unhealthy container default: status 500 at http://127.0.0.1:8080/status.html: giving up\n", stdout.String()) assert.Equal(t, "Error: services not ready: default\n", @@ -122,13 +122,13 @@ func TestStatusLocalDeployment(t *testing.T) { resp.Body = []byte(`{"currentGeneration": 42, "converged": false}`) client.NextResponse(resp) assert.NotNil(t, cli.Run("status", "deployment")) - assert.Equal(t, "Error: deployment not converged on latest generation: wait timed out\n", stderr.String()) + assert.Equal(t, "Warning: deployment not converged on latest generation: giving up\nHint: Consider using the --wait flag to wait for completion\n", stderr.String()) // Explicit generation stderr.Reset() client.NextResponse(resp) assert.NotNil(t, cli.Run("status", "deployment", "41")) - assert.Equal(t, "Error: deployment not converged on generation 41: wait timed out\n", stderr.String()) + assert.Equal(t, "Warning: deployment not converged on generation 41: giving up\nHint: Consider using the --wait flag to wait for completion\n", stderr.String()) } func TestStatusCloudDeployment(t *testing.T) { @@ -164,7 +164,7 @@ func TestStatusCloudDeployment(t *testing.T) { Body: []byte(`{"active": false, "status": "failure"}`), }) assert.NotNil(t, cli.Run("status", "deployment", "42", "-w", "10")) - assert.Equal(t, "Waiting up to 10s for deployment to converge...\nError: deployment run 42 incomplete after waiting up to 10s: aborting wait: run 42 ended with unsuccessful status: failure\n", stderr.String()) + assert.Equal(t, "Waiting up to 10s for deployment to converge...\nWarning: deployment run 42 incomplete after waiting up to 10s: aborting wait: run 42 ended with unsuccessful status: failure\n", stderr.String()) } func isLocalTarget(args []string) bool { diff --git a/client/go/internal/vespa/target.go b/client/go/internal/vespa/target.go index 543ce2f4a29..90d1e1997da 100644 --- a/client/go/internal/vespa/target.go +++ b/client/go/internal/vespa/target.go @@ -36,7 +36,7 @@ const ( AnyDeployment int64 = -2 ) -var errWaitTimeout = errors.New("wait timed out") +var errWaitTimeout = errors.New("giving up") var errAuth = errors.New("auth failed") // Authenticator authenticates the given HTTP request. @@ -255,7 +255,7 @@ func wait(service *Service, okFn responseFunc, reqFn requestFunc, timeout, retry deadline := time.Now().Add(timeout) loopOnce := timeout == 0 for time.Now().Before(deadline) || loopOnce { - response, err = service.Do(reqFn(), 10*time.Second) + response, err = service.Do(reqFn(), 20*time.Second) if errors.Is(err, errAuth) { return status, fmt.Errorf("aborting wait: %w", err) } else if err == nil { diff --git a/client/js/app/yarn.lock b/client/js/app/yarn.lock index dff60b31ec5..c06265edaa1 100644 --- a/client/js/app/yarn.lock +++ b/client/js/app/yarn.lock @@ -1204,7 +1204,7 @@ "@nodelib/fs.scandir" "2.1.5" fastq "^1.6.0" -"@pkgr/utils@^2.3.1": +"@pkgr/utils@^2.4.2": version "2.4.2" resolved "https://registry.yarnpkg.com/@pkgr/utils/-/utils-2.4.2.tgz#9e638bbe9a6a6f165580dc943f138fd3309a2cbc" integrity sha512-POgTXhjrTfbTV63DiFXav4lBHiICLKKwDeaKn9Nphwj7WH6m0hMMCaJkMyRWjgtPFyRKRVoMXXjczsTQRDEhYw== @@ -1900,9 +1900,9 @@ base@^0.11.1: pascalcase "^0.1.1" big-integer@^1.6.44: - version "1.6.51" - resolved "https://registry.yarnpkg.com/big-integer/-/big-integer-1.6.51.tgz#0df92a5d9880560d3ff2d5fd20245c889d130686" - integrity sha512-GPEid2Y9QU1Exl1rpO9B2IPJGHPSupF5GnVIP0blYvNOMer2bTvSWs1jGOUg04hTmu67nmLsQ9TBo1puaotBHg== + version "1.6.52" + resolved "https://registry.yarnpkg.com/big-integer/-/big-integer-1.6.52.tgz#60a887f3047614a8e1bffe5d7173490a97dc8c85" + integrity sha512-QxD8cf2eVqJOOz63z6JIN9BzvVs/dlySa5HGSBH5xtR8dPteIRQnBxxKqkNTiT6jbDTF6jAfrd4oMcND9RGbQg== bplist-parser@^0.2.0: version "0.2.0" @@ -2564,9 +2564,9 @@ eslint-plugin-import@^2: tsconfig-paths "^3.15.0" eslint-plugin-prettier@^5: - version "5.0.1" - resolved "https://registry.yarnpkg.com/eslint-plugin-prettier/-/eslint-plugin-prettier-5.0.1.tgz#a3b399f04378f79f066379f544e42d6b73f11515" - integrity sha512-m3u5RnR56asrwV/lDC4GHorlW75DsFfmUcjfCYylTUs85dBRnB7VM6xG8eCMJdeDRnppzmxZVf1GEPJvl1JmNg== + version "5.1.0" + resolved "https://registry.yarnpkg.com/eslint-plugin-prettier/-/eslint-plugin-prettier-5.1.0.tgz#f14bb2b18756ad54f1ad3dc4c989cb73dfa326a3" + integrity sha512-hQc+2zbnMeXcIkg+pKZtVa+3Yqx4WY7SMkn1PLZ4VbBEU7jJIpVn9347P8BBhTbz6ne85aXvQf30kvexcqBeWw== dependencies: prettier-linter-helpers "^1.0.0" synckit "^0.8.5" @@ -2842,9 +2842,9 @@ fast-diff@^1.1.2: integrity sha512-VxPP4NqbUjj6MaAOafWeUn2cXWLcCtljklUtZf0Ind4XQ+QPtmA0b18zZy0jIQx+ExRVCR/ZQpBmik5lXshNsw== fast-glob@^3.3.0: - version "3.3.1" - resolved "https://registry.yarnpkg.com/fast-glob/-/fast-glob-3.3.1.tgz#784b4e897340f3dbbef17413b3f11acf03c874c4" - integrity sha512-kNFPyjhh5cKjrUltxs+wFx+ZkbRaxxmZ+X0ZU31SOsxCEtP9VPgtq2teZw1DebupL5GmDaNQ6yKMMVcM41iqDg== + version "3.3.2" + resolved "https://registry.yarnpkg.com/fast-glob/-/fast-glob-3.3.2.tgz#a904501e57cfdd2ffcded45e99a54fef55e46129" + integrity sha512-oX2ruAFQwf/Orj8m737Y5adxDQO0LAB7/S5MnxCdTNDd4p6BsyIVsv9JQsATbTSq8KHRpLwIHbVlUNatxd+1Ow== dependencies: "@nodelib/fs.stat" "^2.0.2" "@nodelib/fs.walk" "^1.2.3" @@ -2863,9 +2863,9 @@ fast-levenshtein@^2.0.6: integrity sha512-DCXu6Ifhqcks7TZKY3Hxp3y6qphY5SJZmrWMDrKcERSOXWQdMhU9Ig/PYrzyw/ul9jOIyh0N4M0tbC5hodg8dw== fastq@^1.6.0: - version "1.15.0" - resolved "https://registry.yarnpkg.com/fastq/-/fastq-1.15.0.tgz#d04d07c6a2a68fe4599fea8d2e103a937fae6b3a" - integrity sha512-wBrocU2LCXXa+lWBt8RoIRD89Fi8OdABODa/kEnyeyjS5aZO5/GNvI5sEINADqP/h8M29UHTHUb53sUu5Ihqdw== + version "1.16.0" + resolved "https://registry.yarnpkg.com/fastq/-/fastq-1.16.0.tgz#83b9a9375692db77a822df081edb6a9cf6839320" + integrity sha512-ifCoaXsDrsdkWTtiNJX5uzHDsrck5TzfKKDcuFFTIrrc/BS076qgEIfoIy1VeZqViznfKiysPYTh/QeHtnIsYA== dependencies: reusify "^1.0.4" @@ -5442,12 +5442,12 @@ supports-preserve-symlinks-flag@^1.0.0: integrity sha512-ot0WnXS9fgdkgIcePe6RHNk1WA8+muPa6cSjeR3V8K27q9BB1rTE3R1p7Hv0z1ZyAc8s6Vvv8DIyWf681MAt0w== synckit@^0.8.5: - version "0.8.5" - resolved "https://registry.yarnpkg.com/synckit/-/synckit-0.8.5.tgz#b7f4358f9bb559437f9f167eb6bc46b3c9818fa3" - integrity sha512-L1dapNV6vu2s/4Sputv8xGsCdAVlb5nRDMFU/E27D44l5U6cw1g0dGd45uLc+OXjNMmF4ntiMdCimzcjFKQI8Q== + version "0.8.6" + resolved "https://registry.yarnpkg.com/synckit/-/synckit-0.8.6.tgz#b69b7fbce3917c2673cbdc0d87fb324db4a5b409" + integrity sha512-laHF2savN6sMeHCjLRkheIU4wo3Zg9Ln5YOjOo7sZ5dVQW8yF5pPE5SIw1dsPhq3TRp1jisKRCdPhfs/1WMqDA== dependencies: - "@pkgr/utils" "^2.3.1" - tslib "^2.5.0" + "@pkgr/utils" "^2.4.2" + tslib "^2.6.2" tabbable@^6.0.1: version "6.2.0" @@ -5525,7 +5525,7 @@ tsconfig-paths@^3.15.0: minimist "^1.2.6" strip-bom "^3.0.0" -tslib@^2.0.0, tslib@^2.5.0, tslib@^2.6.0: +tslib@^2.0.0, tslib@^2.6.0, tslib@^2.6.2: version "2.6.2" resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.6.2.tgz#703ac29425e7b37cd6fd456e92404d46d1f3e4ae" integrity sha512-AEYxH93jGFPn/a2iVAwW87VuUIkR1FVUKB77NwMF7nBTDkDrrT/Hpt/IrCJ0QXhW27jTBDcf5ZY7w6RiqTMw2Q== diff --git a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java index 3df11855f75..ab5645eb50d 100644 --- a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java +++ b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java @@ -27,6 +27,7 @@ import com.yahoo.io.IOUtils; import com.yahoo.io.reader.NamedReader; import com.yahoo.path.Path; import com.yahoo.text.Utf8; +import com.yahoo.text.XML; import com.yahoo.vespa.config.ConfigDefinition; import com.yahoo.vespa.config.ConfigDefinitionBuilder; import com.yahoo.vespa.config.ConfigDefinitionKey; @@ -36,6 +37,7 @@ import org.w3c.dom.Element; import org.w3c.dom.Node; import org.w3c.dom.NodeList; import org.xml.sax.SAXException; + import javax.xml.parsers.ParserConfigurationException; import javax.xml.transform.TransformerException; import javax.xml.transform.TransformerFactory; @@ -166,7 +168,7 @@ public class FilesApplicationPackage extends AbstractApplicationPackage { configDefsDir = applicationFile(appDir, CONFIG_DEFINITIONS_DIR); addUserIncludeDirs(); this.metaData = metaData; - transformerFactory = TransformerFactory.newInstance(); + this.transformerFactory = XML.createTransformerFactory(); } @Override diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index a8aaf0f57ef..10c5662678e 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -1293,7 +1293,8 @@ "public boolean createPostinglistWhenNonStrict()", "public boolean useEstimateForFetchPostings()", "public boolean useThreadBundleForFetchPostings()", - "public boolean restartOnDeployWhenOnnxModelChanges()" + "public boolean restartOnDeployWhenOnnxModelChanges()", + "public boolean sortBlueprintsByCost()" ], "fields" : [ ] }, @@ -1464,7 +1465,8 @@ "public abstract void registerModel(java.net.URI, com.yahoo.config.model.api.OnnxModelOptions)", "public abstract java.util.Map models()", "public abstract void setRestartOnDeploy()", - "public abstract boolean restartOnDeploy()" + "public abstract boolean restartOnDeploy()", + "public abstract void store()" ], "fields" : [ ] }, @@ -1487,7 +1489,8 @@ "public void registerModel(java.net.URI, com.yahoo.config.model.api.OnnxModelOptions)", "public java.util.Map models()", "public void setRestartOnDeploy()", - "public boolean restartOnDeploy()" + "public boolean restartOnDeploy()", + "public void store()" ], "fields" : [ ] }, diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index 852e02dab36..008a661a316 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -120,6 +120,7 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"baldersheim"}) default boolean useEstimateForFetchPostings() { return false; } @ModelFeatureFlag(owners = {"baldersheim"}) default boolean useThreadBundleForFetchPostings() { return false; } @ModelFeatureFlag(owners = {"hmusum"}) default boolean restartOnDeployWhenOnnxModelChanges() { return false; } + @ModelFeatureFlag(owners = {"baldersheim"}) default boolean sortBlueprintsByCost() { return false; } } /** Warning: As elsewhere in this package, do not make backwards incompatible changes that will break old config models! */ diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/OnnxModelCost.java b/config-model-api/src/main/java/com/yahoo/config/model/api/OnnxModelCost.java index c13ce4def09..d70b751eba0 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/OnnxModelCost.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/OnnxModelCost.java @@ -25,6 +25,7 @@ public interface OnnxModelCost { Map<String, ModelInfo> models(); void setRestartOnDeploy(); boolean restartOnDeploy(); + void store(); } record ModelInfo(String modelId, long estimatedCost, long hash, Optional<OnnxModelOptions> onnxModelOptions) {} @@ -41,6 +42,7 @@ public interface OnnxModelCost { @Override public Map<String, ModelInfo> models() { return Map.of(); } @Override public void setRestartOnDeploy() {} @Override public boolean restartOnDeploy() { return false; } + @Override public void store() {} } } diff --git a/config-model/src/main/java/com/yahoo/schema/derived/RawRankProfile.java b/config-model/src/main/java/com/yahoo/schema/derived/RawRankProfile.java index dad39e74c37..a99fa8b7710 100644 --- a/config-model/src/main/java/com/yahoo/schema/derived/RawRankProfile.java +++ b/config-model/src/main/java/com/yahoo/schema/derived/RawRankProfile.java @@ -170,6 +170,7 @@ public class RawRankProfile implements RankProfilesConfig.Producer { private final OptionalDouble approximateThreshold; private final OptionalDouble targetHitsMaxAdjustmentFactor; private final double rankScoreDropLimit; + private final boolean sortBlueprintsByCost; private final boolean alwaysMarkPhraseExpensive; private final boolean createPostinglistWhenNonStrict; private final boolean useEstimateForFetchPostings; @@ -215,6 +216,7 @@ public class RawRankProfile implements RankProfilesConfig.Producer { minHitsPerThread = compiled.getMinHitsPerThread(); numSearchPartitions = compiled.getNumSearchPartitions(); termwiseLimit = compiled.getTermwiseLimit().orElse(deployProperties.featureFlags().defaultTermwiseLimit()); + sortBlueprintsByCost = deployProperties.featureFlags().sortBlueprintsByCost(); alwaysMarkPhraseExpensive = deployProperties.featureFlags().alwaysMarkPhraseExpensive(); createPostinglistWhenNonStrict = deployProperties.featureFlags().createPostinglistWhenNonStrict(); useEstimateForFetchPostings = deployProperties.featureFlags().useEstimateForFetchPostings(); @@ -469,6 +471,9 @@ public class RawRankProfile implements RankProfilesConfig.Producer { if (termwiseLimit < 1.0) { properties.add(new Pair<>("vespa.matching.termwise_limit", termwiseLimit + "")); } + if (sortBlueprintsByCost) { + properties.add(new Pair<>("vespa.matching.sort_blueprints_by_cost", String.valueOf(sortBlueprintsByCost))); + } if (alwaysMarkPhraseExpensive) { properties.add(new Pair<>("vespa.matching.always_mark_phrase_expensive", String.valueOf(alwaysMarkPhraseExpensive))); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidator.java index 1205ae4e939..e118a2940d7 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidator.java @@ -50,7 +50,8 @@ public class RestartOnDeployForOnnxModelChangesValidator implements ChangeValida if (enoughMemoryToAvoidRestart(clusterInCurrentModel, cluster, deployState.getDeployLogger())) continue; - log.log(FINE, "Validating " + cluster + ", current models=" + currentModels + ", next models=" + nextModels); + log.log(FINE, "Validating %s, current Onnx models:%s, next Onnx models:%s" + .formatted(cluster, currentModels, nextModels)); actions.addAll(validateModelChanges(cluster, currentModels, nextModels)); actions.addAll(validateSetOfModels(cluster, currentModels, nextModels)); } @@ -79,7 +80,7 @@ public class RestartOnDeployForOnnxModelChangesValidator implements ChangeValida List<ConfigChangeAction> actions = new ArrayList<>(); Set<String> currentModelIds = currentModels.keySet(); Set<String> nextModelIds = nextModels.keySet(); - log.log(FINE, "Checking if model set has changed (%s) -> (%s)".formatted(currentModelIds, nextModelIds)); + log.log(FINE, "Checking if Onnx model set has changed (%s) -> (%s)".formatted(currentModelIds, nextModelIds)); if (! currentModelIds.equals(nextModelIds)) { String message = "Onnx model set has changed from %s to %s, need to restart services in %s" .formatted(currentModelIds, nextModelIds, cluster); @@ -99,6 +100,7 @@ public class RestartOnDeployForOnnxModelChangesValidator implements ChangeValida private static void setRestartOnDeployAndAddRestartAction(List<ConfigChangeAction> actions, ApplicationContainerCluster cluster, String message) { log.log(INFO, message); cluster.onnxModelCostCalculator().setRestartOnDeploy(); + cluster.onnxModelCostCalculator().store(); actions.add(new VespaRestartAction(cluster.id(), message)); } @@ -109,21 +111,29 @@ public class RestartOnDeployForOnnxModelChangesValidator implements ChangeValida double nextModelCostInGb = onnxModelCostInGb(cluster); double totalMemory = cluster.getContainers().get(0).getHostResource().realResources().memoryGb(); - double availableMemory = Math.max(0, totalMemory - Host.memoryOverheadGb - currentModelCostInGb - nextModelCostInGb); - if (availableMemory <= 0.0) - return false; + double memoryUsedByModels = currentModelCostInGb + nextModelCostInGb; + double availableMemory = Math.max(0, totalMemory - Host.memoryOverheadGb - memoryUsedByModels); var availableMemoryPercentage = cluster.availableMemoryPercentage(); int memoryPercentage = (int) (availableMemory / totalMemory * availableMemoryPercentage); - if (memoryPercentage < percentLimit || availableMemory < gbLimit) { - deployLogger.log(INFO, "Validating %s, not enough memory (%s) to avoid restart (models require %s), consider a flavor with more memory to avoid this" - .formatted(cluster, availableMemory, currentModelCostInGb + nextModelCostInGb)); + var prefix = "Validating Onnx models memory usage for %s".formatted(cluster); + if (memoryPercentage < percentLimit) { + deployLogger.log(INFO, ("%s, percentage of available memory " + + "too low (%d < %d) to avoid restart, consider a flavor with more memory to avoid this") + .formatted(prefix, memoryPercentage, percentLimit)); + return false; + } + + if (availableMemory < gbLimit) { + deployLogger.log(INFO, ("%s, available memory too low " + + "(%.2f Gb < %.2f Gb) to avoid restart, consider a flavor with more memory to avoid this") + .formatted(prefix, availableMemory, gbLimit)); return false; } - log.log(FINE, "Validating " + cluster + ", enough memory (%s) to avoid restart (models require %s)" - .formatted(availableMemory, currentModelCostInGb + nextModelCostInGb)); + log.log(FINE, "%s, enough available memory (%.2f Gb) to avoid restart (models use %.2f Gb)" + .formatted(prefix, availableMemory, memoryUsedByModels)); return true; } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidatorTest.java index 213cf4bdfcf..0b32194e257 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidatorTest.java @@ -124,6 +124,7 @@ class JvmHeapSizeValidatorTest { @Override public Map<String, ModelInfo> models() { return Map.of(); } @Override public void setRestartOnDeploy() {} @Override public boolean restartOnDeploy() { return false;} + @Override public void store() {} @Override public long aggregatedModelCostInBytes() { return totalCost.get(); } @Override public void registerModel(ApplicationFile path) {} @Override public void registerModel(ApplicationFile path, OnnxModelOptions onnxModelOptions) {} diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidatorTest.java index 5873d15bd9a..adcf58785fa 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidatorTest.java @@ -110,6 +110,9 @@ public class RestartOnDeployForOnnxModelChangesValidatorTest { @Override public boolean restartOnDeploy() { return restartOnDeploy; } + + @Override + public void store() {} }; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java index 43594c943e3..0c387e38afb 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java @@ -201,6 +201,7 @@ public class ModelContextImpl implements ModelContext { private final int rpc_events_before_wakeup; private final int heapPercentage; private final String summaryDecodePolicy; + private boolean sortBlueprintsByCost; private final boolean alwaysMarkPhraseExpensive; private final boolean createPostinglistWhenNonStrict; private final boolean useEstimateForFetchPostings; @@ -260,6 +261,7 @@ public class ModelContextImpl implements ModelContext { this.useEstimateForFetchPostings = flagValue(source, appId, version, Flags.USE_ESTIMATE_FOR_FETCH_POSTINGS); this.useThreadBundleForFetchPostings = flagValue(source, appId, version, Flags.USE_THREAD_BUNDLE_FOR_FETCH_POSTINGS); this.restartOnDeployWhenOnnxModelChanges = flagValue(source, appId, version, Flags.RESTART_ON_DEPLOY_WHEN_ONNX_MODEL_CHANGES); + this.sortBlueprintsByCost = flagValue(source, appId, version, Flags.SORT_BLUEPRINTS_BY_COST); } @Override public int heapSizePercentage() { return heapPercentage; } @@ -316,6 +318,7 @@ public class ModelContextImpl implements ModelContext { @Override public long mergingMaxMemoryUsagePerNode() { return mergingMaxMemoryUsagePerNode; } @Override public boolean usePerDocumentThrottledDeleteBucket() { return usePerDocumentThrottledDeleteBucket; } @Override public boolean restartOnDeployWhenOnnxModelChanges() { return restartOnDeployWhenOnnxModelChanges; } + @Override public boolean sortBlueprintsByCost() { return sortBlueprintsByCost; } private static <V> V flagValue(FlagSource source, ApplicationId appId, Version vespaVersion, UnboundFlag<? extends V, ?, ?> flag) { return flag.bindTo(source) diff --git a/dependency-versions/pom.xml b/dependency-versions/pom.xml index 40d1caab492..a4849558253 100644 --- a/dependency-versions/pom.xml +++ b/dependency-versions/pom.xml @@ -91,7 +91,7 @@ <commons-compress.vespa.version>1.25.0</commons-compress.vespa.version> <commons-cli.vespa.version>1.6.0</commons-cli.vespa.version> <curator.vespa.version>5.5.0</curator.vespa.version> - <dropwizard.metrics.vespa.version>4.1.12.1</dropwizard.metrics.vespa.version> <!-- ZK 3.9.1 requires this --> + <dropwizard.metrics.vespa.version>4.2.23</dropwizard.metrics.vespa.version> <!-- ZK 3.9.1 requires this --> <eclipse-collections.vespa.version>11.1.0</eclipse-collections.vespa.version> <eclipse-sisu.vespa.version>0.9.0.M2</eclipse-sisu.vespa.version> <felix.vespa.version>7.0.5</felix.vespa.version> @@ -120,7 +120,7 @@ <mimepull.vespa.version>1.10.0</mimepull.vespa.version> <mockito.vespa.version>5.8.0</mockito.vespa.version> <mojo-executor.vespa.version>2.4.0</mojo-executor.vespa.version> - <netty.vespa.version>4.1.101.Final</netty.vespa.version> + <netty.vespa.version>4.1.104.Final</netty.vespa.version> <netty-tcnative.vespa.version>2.0.62.Final</netty-tcnative.vespa.version> <onnxruntime.vespa.version>1.16.3</onnxruntime.vespa.version> <opennlp.vespa.version>2.3.1</opennlp.vespa.version> @@ -153,7 +153,7 @@ <maven-antrun-plugin.vespa.version>3.1.0</maven-antrun-plugin.vespa.version> <maven-assembly-plugin.vespa.version>3.6.0</maven-assembly-plugin.vespa.version> <maven-bundle-plugin.vespa.version>5.1.9</maven-bundle-plugin.vespa.version> - <maven-compiler-plugin.vespa.version>3.11.0</maven-compiler-plugin.vespa.version> + <maven-compiler-plugin.vespa.version>3.12.0</maven-compiler-plugin.vespa.version> <maven-core.vespa.version>3.9.6</maven-core.vespa.version> <maven-dependency-plugin.vespa.version>3.6.1</maven-dependency-plugin.vespa.version> <maven-deploy-plugin.vespa.version>3.1.1</maven-deploy-plugin.vespa.version> 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 e5227a86cfb..a32ed75d67b 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -325,6 +325,13 @@ public class Flags { "Where specified, CNAME records are used instead of the default ALIAS records, which have a default 60s TTL.", "Takes effect at redeployment from controller"); + public static final UnboundBooleanFlag SORT_BLUEPRINTS_BY_COST = defineFeatureFlag( + "sort-blueprints-by-cost", false, + List.of("baldersheim"), "2023-12-19", "2024-02-29", + "If true blueprints are sorted based on cost estimate, rather that absolute estimated hits", + "Takes effect at redeployment", + INSTANCE_ID); + public static final UnboundBooleanFlag ALWAYS_MARK_PHRASE_EXPENSIVE = defineFeatureFlag( "always-mark-phrase-expensive", false, List.of("baldersheim"), "2023-11-20", "2023-12-31", diff --git a/http-utils/src/main/java/ai/vespa/util/http/hc5/VespaHttpClientBuilder.java b/http-utils/src/main/java/ai/vespa/util/http/hc5/VespaHttpClientBuilder.java index edd10f9297a..4e4e6f2956a 100644 --- a/http-utils/src/main/java/ai/vespa/util/http/hc5/VespaHttpClientBuilder.java +++ b/http-utils/src/main/java/ai/vespa/util/http/hc5/VespaHttpClientBuilder.java @@ -69,16 +69,16 @@ public class VespaHttpClientBuilder { connectionConfigBuilder.setConnectTimeout(connectTimeout); return this; } - public VespaHttpClientBuilder socketTimeout(long connectTimeout, TimeUnit timeUnit) { - connectionConfigBuilder.setConnectTimeout(connectTimeout, timeUnit); + public VespaHttpClientBuilder socketTimeout(int socketTimeout, TimeUnit timeUnit) { + connectionConfigBuilder.setSocketTimeout(socketTimeout, timeUnit); return this; } - public VespaHttpClientBuilder validateAfterInactivity(TimeValue validateAfterInactivity) { - connectionConfigBuilder.setValidateAfterInactivity(validateAfterInactivity); + public VespaHttpClientBuilder socketTimeout(Timeout socketTimeout) { + connectionConfigBuilder.setSocketTimeout(socketTimeout); return this; } - public VespaHttpClientBuilder socketTimeout(Timeout connectTimeout) { - connectionConfigBuilder.setConnectTimeout(connectTimeout); + public VespaHttpClientBuilder validateAfterInactivity(TimeValue validateAfterInactivity) { + connectionConfigBuilder.setValidateAfterInactivity(validateAfterInactivity); return this; } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/HttpMetricFetcher.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/HttpMetricFetcher.java index db53c2db266..ac9063a045f 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/HttpMetricFetcher.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/HttpMetricFetcher.java @@ -82,11 +82,11 @@ public abstract class HttpMetricFetcher { private static CloseableHttpClient createHttpClient() { return VespaHttpClientBuilder.custom() .connectTimeout(Timeout.ofMilliseconds(CONNECTION_TIMEOUT)) - .socketTimeout(Timeout.ofMilliseconds(CONNECTION_TIMEOUT)) + .socketTimeout(Timeout.ofMilliseconds(SOCKET_TIMEOUT)) .apacheBuilder() .setUserAgent("metrics-proxy-http-client") .setDefaultRequestConfig(RequestConfig.custom() - .setConnectionRequestTimeout(Timeout.ofMilliseconds(SOCKET_TIMEOUT)) + .setConnectionRequestTimeout(Timeout.ofMilliseconds(CONNECTION_TIMEOUT)) .setResponseTimeout(Timeout.ofMilliseconds(SOCKET_TIMEOUT)) .build()) .build(); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java index 1e4a11fdea2..986ab830283 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java @@ -233,11 +233,13 @@ public class ClusterModel { double queryCpu = queryCpuPerGroup * groupCount() / groups; double writeCpu = (double)groupSize() / groupSize; return new Load(cpu.queryFraction() * queryCpu + (1 - cpu.queryFraction()) * writeCpu, - (1 - memory.fixedFraction()) * (double)groupSize() / groupSize + memory.fixedFraction() * 1, - (double)groupSize() / groupSize); + (1 - memory.fixedFraction()) * (double) groupSize() / groupSize + memory.fixedFraction() * 1, + (double)groupSize() / groupSize, + 1, + 1); } else { - return new Load((double)nodeCount() / nodes, 1, 1); + return new Load((double) nodeCount() / nodes, 1, 1, 1, 1); } } @@ -246,7 +248,7 @@ public class ClusterModel { * if one of the nodes go down. */ public Load idealLoad() { - var ideal = new Load(cpu.idealLoad(), memory.idealLoad(), disk.idealLoad()).divide(redundancyAdjustment()); + var ideal = new Load(cpu.idealLoad(), memory.idealLoad(), disk.idealLoad(), cpu.idealLoad(), memory.idealLoad()).divide(redundancyAdjustment()); if ( !cluster.bcpGroupInfo().isEmpty() && cluster.bcpGroupInfo().queryRate() > 0) { // Since we have little local information, use information about query cost in other groups Load bcpGroupIdeal = adjustQueryDependentIdealLoadByBcpGroupInfo(ideal); @@ -392,7 +394,7 @@ public class ClusterModel { if (averageQueryRate().isEmpty() || averageQueryRate().getAsDouble() == 0.0) return OptionalDouble.empty(); // TODO: Query rate should generally be sampled at the time where we see the peak resource usage int fanOut = clusterSpec.type().isContainer() ? 1 : groupSize(); - return OptionalDouble.of(peakLoad().cpu() * cpu.queryFraction() * fanOut * nodes.not().retired().first().get().resources().vcpu() + return OptionalDouble.of(peakLoad().cpu() * cpu.queryFraction() * fanOut * nodes.not().retired().first().get().resources().vcpu() / averageQueryRate().getAsDouble() / groupCount()); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterNodesTimeseries.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterNodesTimeseries.java index e1ef21ebd13..6978e269c3d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterNodesTimeseries.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterNodesTimeseries.java @@ -67,7 +67,8 @@ public class ClusterNodesTimeseries { * the average of the highest reading for that dimension on each node. */ public Load peakLoad() { - return new Load(peakLoad(Load.Dimension.cpu), peakLoad(Load.Dimension.memory), peakLoad(Load.Dimension.disk)); + return new Load(peakLoad(Load.Dimension.cpu), peakLoad(Load.Dimension.memory), peakLoad(Load.Dimension.disk), + peakLoad(Load.Dimension.gpu), peakLoad(Load.Dimension.gpuMemory)); } private double peakLoad(Load.Dimension dimension) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Load.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Load.java index 799ed621807..22c13795d18 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Load.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Load.java @@ -3,9 +3,7 @@ package com.yahoo.vespa.hosted.provision.autoscale; import com.yahoo.config.provision.NodeResources; -import java.util.Objects; import java.util.function.DoubleBinaryOperator; -import java.util.function.DoubleFunction; import java.util.function.DoubleUnaryOperator; import java.util.function.Predicate; @@ -14,32 +12,36 @@ import java.util.function.Predicate; * * @author bratseth */ -public class Load { +public record Load(double cpu, double memory, double disk, double gpu, double gpuMemory) { - public enum Dimension { cpu, memory, disk } + public enum Dimension { cpu, memory, disk, gpu, gpuMemory } - private final double cpu, memory, disk; - - public Load(double cpu, double memory, double disk) { + public Load(double cpu, double memory, double disk, double gpu, double gpuMemory) { this.cpu = requireNormalized(cpu, "cpu"); this.memory = requireNormalized(memory, "memory"); this.disk = requireNormalized(disk, "disk"); + this.gpu = requireNormalized(gpu, "gpu"); + this.gpuMemory = requireNormalized(gpuMemory, "gpuMemory"); } public double cpu() { return cpu; } public double memory() { return memory; } public double disk() { return disk; } + public double gpu() { return gpu; } + public double gpuMemory() { return gpuMemory; } - public Load withCpu(double cpu) { return new Load(cpu, memory, disk); } - public Load withMemory(double memory) { return new Load(cpu, memory, disk); } - public Load withDisk(double disk) { return new Load(cpu, memory, disk); } + public Load withCpu(double cpu) { return new Load(cpu, memory, disk, gpu, gpuMemory); } + public Load withMemory(double memory) { return new Load(cpu, memory, disk, gpu, gpuMemory); } + public Load withDisk(double disk) { return new Load(cpu, memory, disk, gpu, gpuMemory); } + public Load withGpu(double gpu) { return new Load(cpu, memory, disk, gpu, gpuMemory); } + public Load withGpuMemory(double gpuMemory) { return new Load(cpu, memory, disk, gpu, gpuMemory); } public Load add(Load other) { return join(other, (a, b) -> a + b); } public Load multiply(NodeResources resources) { - return new Load(cpu * resources.vcpu(), memory * resources.memoryGb(), disk * resources.diskGb()); + return new Load(cpu * resources.vcpu(), memory * resources.memoryGb(), disk * resources.diskGb(), gpu * resources.gpuResources().count(), gpu * resources.gpuResources().memoryGb()); } public Load multiply(double factor) { return map(v -> v * factor); @@ -55,21 +57,25 @@ public class Load { return map(v -> divide(v, divisor)); } public Load divide(NodeResources resources) { - return new Load(divide(cpu, resources.vcpu()), divide(memory, resources.memoryGb()), divide(disk, resources.diskGb())); + return new Load(divide(cpu, resources.vcpu()), divide(memory, resources.memoryGb()), divide(disk, resources.diskGb()), divide(gpu, resources.gpuResources().count()), divide(gpuMemory, resources.gpuResources().memoryGb())); } /** Returns the load where the given function is applied to each dimension of this. */ public Load map(DoubleUnaryOperator f) { return new Load(f.applyAsDouble(cpu), f.applyAsDouble(memory), - f.applyAsDouble(disk)); + f.applyAsDouble(disk), + f.applyAsDouble(gpu), + f.applyAsDouble(gpuMemory)); } /** Returns the load where the given function is applied to each dimension of this and the given load. */ public Load join(Load other, DoubleBinaryOperator f) { return new Load(f.applyAsDouble(this.cpu(), other.cpu()), f.applyAsDouble(this.memory(), other.memory()), - f.applyAsDouble(this.disk(), other.disk())); + f.applyAsDouble(this.disk(), other.disk()), + f.applyAsDouble(this.gpu(), other.gpu()), + f.applyAsDouble(this.gpuMemory(), other.gpuMemory())); } /** Returns true if any dimension matches the predicate. */ @@ -88,6 +94,8 @@ public class Load { case cpu -> cpu(); case memory -> memory(); case disk -> disk(); + case gpu -> gpu(); + case gpuMemory -> gpuMemory(); }; } @@ -95,7 +103,7 @@ public class Load { if (Double.isNaN(value)) throw new IllegalArgumentException(name + " must be a number but is NaN"); if (value < 0) - throw new IllegalArgumentException(name + " must be zero or lager, but is " + value); + throw new IllegalArgumentException(name + " must be zero or larger, but is " + value); return value; } @@ -105,28 +113,19 @@ public class Load { } @Override - public boolean equals(Object o) { - if (o == this) return true; - if ( ! (o instanceof Load other)) return false; - if (other.cpu() != this.cpu()) return false; - if (other.memory() != this.memory()) return false; - if (other.disk() != this.disk()) return false; - return true; - } - - @Override - public int hashCode() { return Objects.hash(cpu, memory, disk); } - - @Override public String toString() { - return "load: " + cpu + " cpu, " + memory + " memory, " + disk + " disk"; + return "load: " + cpu + " cpu, " + memory + " memory, " + disk + " disk," + gpu + " gpu," + gpuMemory + " gpuMemory"; } - public static Load zero() { return new Load(0, 0, 0); } - public static Load one() { return new Load(1, 1, 1); } + public static Load zero() { return new Load(0, 0, 0, 0, 0); } + public static Load one() { return new Load(1, 1, 1, 1, 1); } public static Load byDividing(NodeResources a, NodeResources b) { - return new Load(divide(a.vcpu(), b.vcpu()), divide(a.memoryGb(), b.memoryGb()), divide(a.diskGb(), b.diskGb())); + return new Load(divide(a.vcpu(), b.vcpu()), + divide(a.memoryGb(), b.memoryGb()), + divide(a.diskGb(), b.diskGb()), + divide(a.gpuResources().count(), b.gpuResources().count()), + divide(a.gpuResources().memoryGb(), b.gpuResources().memoryGb())); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsResponse.java index a6882e49efa..f35879d0b24 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsResponse.java @@ -76,8 +76,10 @@ public class MetricsResponse { nodeMetrics.add(new Pair<>(hostname, new NodeMetricSnapshot(at, new Load(Metric.cpu.from(nodeValues), Metric.memory.from(nodeValues), - Metric.disk.from(nodeValues)), - (long)Metric.generation.from(nodeValues), + Metric.disk.from(nodeValues), + Metric.gpu.from(nodeValues), + Metric.gpuMemory.from(nodeValues)), + (long) Metric.generation.from(nodeValues), Metric.inService.from(nodeValues) > 0, clusterIsStable(node.get(), applicationNodes, nodeValues), Metric.queryRate.from(nodeValues)))); @@ -126,6 +128,7 @@ public class MetricsResponse { @Override public List<String> metricResponseNames() { + // TODO(mpolden): Track only CPU util once we support proper GPU scaling return List.of(HostedNodeAdminMetrics.CPU_UTIL.baseName(), HostedNodeAdminMetrics.GPU_UTIL.baseName()); } @@ -139,6 +142,7 @@ public class MetricsResponse { @Override public List<String> metricResponseNames() { + // TODO(mpolden): Track only CPU memory once we support proper GPU scaling return List.of(HostedNodeAdminMetrics.MEM_UTIL.baseName(), SearchNodeMetrics.CONTENT_PROTON_RESOURCE_USAGE_MEMORY.average(), HostedNodeAdminMetrics.GPU_MEM_USED.baseName(), @@ -147,7 +151,7 @@ public class MetricsResponse { @Override double computeFinal(ListMap<String, Double> values) { - return Math.max(gpuMemUtil(values), cpuMemUtil(values)); + return Math.max(cpuMemUtil(values), gpuMemory.computeFinal(values)); } private double cpuMemUtil(ListMap<String, Double> values) { @@ -160,12 +164,6 @@ public class MetricsResponse { return 0; } - private double gpuMemUtil(ListMap<String, Double> values) { - var usedGpuMemory = values.get(HostedNodeAdminMetrics.GPU_MEM_USED.baseName()).stream().mapToDouble(v -> v).sum(); - var totalGpuMemory = values.get(HostedNodeAdminMetrics.GPU_MEM_TOTAL.baseName()).stream().mapToDouble(v -> v).sum(); - return totalGpuMemory > 0 ? usedGpuMemory / totalGpuMemory : 0; - } - }, disk { // a node resource @@ -187,6 +185,35 @@ public class MetricsResponse { } }, + gpu { // a node resource + + @Override + public List<String> metricResponseNames() { + return List.of(HostedNodeAdminMetrics.GPU_UTIL.baseName()); + } + + @Override + double computeFinal(ListMap<String, Double> values) { + return values.values().stream().flatMap(List::stream).mapToDouble(v -> v).max().orElse(0) / 100; // % to ratio + } + + }, + gpuMemory { // a node resource + + @Override + public List<String> metricResponseNames() { + return List.of(HostedNodeAdminMetrics.GPU_MEM_USED.baseName(), + HostedNodeAdminMetrics.GPU_MEM_TOTAL.baseName()); + } + + @Override + double computeFinal(ListMap<String, Double> values) { + var usedGpuMemory = values.get(HostedNodeAdminMetrics.GPU_MEM_USED.baseName()).stream().mapToDouble(v -> v).sum(); + var totalGpuMemory = values.get(HostedNodeAdminMetrics.GPU_MEM_TOTAL.baseName()).stream().mapToDouble(v -> v).sum(); + return totalGpuMemory > 0 ? usedGpuMemory / totalGpuMemory : 0; + } + + }, generation { // application config generation active on the node @Override diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDb.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDb.java index 38127fa3093..4d0bbb4e511 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDb.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDb.java @@ -144,6 +144,8 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb { row.putBool(6, snapshot.getSecond().inService()); row.putBool(7, snapshot.getSecond().stable()); row.putFloat(8, (float) snapshot.getSecond().queryRate()); + row.putFloat(9, (float) snapshot.getSecond().load().gpu()); + row.putFloat(10, (float) snapshot.getSecond().load().gpuMemory()); row.append(); } writer.commit(); @@ -243,6 +245,9 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb { private void ensureNodeTableIsUpdated() { try { // Example: nodeTable.ensureColumnExists("write_rate", "float"); + // TODO(mpolden): Remove after January 2024 + nodeTable.ensureColumnExists("gpu_util", "float"); + nodeTable.ensureColumnExists("gpu_mem_total_util", "float"); } catch (Exception e) { nodeTable.repair(e); } @@ -262,7 +267,9 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb { try { issue("create table " + nodeTable.name + " (hostname string, at timestamp, cpu_util float, mem_total_util float, disk_util float," + - " application_generation long, inService boolean, stable boolean, queries_rate float)" + + " application_generation long, inService boolean, stable boolean, queries_rate float," + + " gpu_util float, gpu_mem_total_util float" + + " )" + " timestamp(at)" + "PARTITION BY DAY;", newContext()); @@ -311,7 +318,9 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb { new NodeMetricSnapshot(Instant.ofEpochMilli(record.getTimestamp(1) / 1000), new Load(record.getFloat(2), record.getFloat(3), - record.getFloat(4)), + record.getFloat(4), + getFloatOrDefault(record, 9, 0), + getFloatOrDefault(record, 10, 0)), record.getLong(5), record.getBool(6), record.getBool(7), @@ -323,6 +332,11 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb { } } + private float getFloatOrDefault(Record record, int col, float defaultValue) { + float value = record.getFloat(col); + return Float.isNaN(value) ? defaultValue : value; + } + private ClusterTimeseries getClusterSnapshots(ApplicationId application, ClusterSpec.Id cluster) throws SqlException { String sql = "select * from " + clusterTable.name; var context = newContext(); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializer.java index c4e7d3b9acc..6f325700401 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializer.java @@ -76,6 +76,8 @@ public class ApplicationSerializer { private static final String cpuKey = "cpu"; private static final String memoryKey = "memory"; private static final String diskKey = "disk"; + private static final String gpuKey = "gpu"; + private static final String gpuMemory = "gpuMemory"; private static final String fromKey = "from"; private static final String toKey = "to"; private static final String generationKey = "generation"; @@ -201,12 +203,16 @@ public class ApplicationSerializer { loadObject.setDouble(cpuKey, load.cpu()); loadObject.setDouble(memoryKey, load.memory()); loadObject.setDouble(diskKey, load.disk()); + loadObject.setDouble(gpuKey, load.gpu()); + loadObject.setDouble(gpuMemory, load.gpuMemory()); } private static Load loadFromSlime(Inspector loadObject) { return new Load(loadObject.field(cpuKey).asDouble(), loadObject.field(memoryKey).asDouble(), - loadObject.field(diskKey).asDouble()); + loadObject.field(diskKey).asDouble(), + loadObject.field(gpuKey).asDouble(), + loadObject.field(gpuMemory).asDouble()); } private static void toSlime(Autoscaling.Metrics metrics, Cursor metricsObject) { 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 b2db5977109..c78ad2b0da6 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 @@ -91,10 +91,7 @@ public class CapacityPolicies { } public NodeResources specifyFully(NodeResources resources, ClusterSpec clusterSpec, ApplicationId applicationId) { - NodeResources amended = resources.withUnspecifiedFieldsFrom(defaultResources(clusterSpec, applicationId).with(DiskSpeed.any)); - // TODO jonmv: remove this after all apps are 8.248.8 or above; architecture for admin nodes was not picked up before this. - if (clusterSpec.vespaVersion().isBefore(Version.fromString("8.248.8"))) amended = amended.with(resources.architecture()); - return amended; + return resources.withUnspecifiedFieldsFrom(defaultResources(clusterSpec, applicationId).with(DiskSpeed.any)); } private NodeResources defaultResources(ClusterSpec clusterSpec, ApplicationId applicationId) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java index 225eb3e4e8d..89853896104 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java @@ -98,6 +98,8 @@ public class ApplicationSerializer { loadObject.setDouble("cpu", load.cpu()); loadObject.setDouble("memory", load.memory()); loadObject.setDouble("disk", load.disk()); + loadObject.setDouble("gpu", load.gpu()); + loadObject.setDouble("gpuMemory", load.gpuMemory()); } private static void toSlime(Autoscaling.Metrics metrics, Cursor metricsObject) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java index 9080030f026..0b157e8635b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java @@ -502,6 +502,8 @@ public class NodesV2ApiHandler extends ThreadedHttpRequestHandler { object.setDouble("cpu", load.cpu()); object.setDouble("memory", load.memory()); object.setDouble("disk", load.disk()); + object.setDouble("gpu", load.gpu()); + object.setDouble("gpuMemory", load.gpuMemory()); } /** Returns a copy of the given URI with the host and port from the given URI and the path set to the given path */ diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java index fe6b204ed31..d3b88997059 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java @@ -40,7 +40,6 @@ import com.yahoo.vespa.hosted.provision.applications.Cluster; import com.yahoo.vespa.hosted.provision.autoscale.Autoscaling; import com.yahoo.vespa.hosted.provision.autoscale.Load; import com.yahoo.vespa.hosted.provision.autoscale.MemoryMetricsDb; -import com.yahoo.vespa.hosted.provision.lb.LoadBalancerService; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.IP; import com.yahoo.vespa.hosted.provision.node.Status; @@ -239,8 +238,8 @@ public class MockNodeRepository extends NodeRepository { Optional.of(new ClusterResources(4, 1, new NodeResources(3, 16, 100, 1))), clock().instant(), - new Load(0.1, 0.2, 0.3), - new Load(0.4, 0.5, 0.6), + new Load(0.1, 0.2, 0.3, 0, 0), + new Load(0.4, 0.5, 0.6, 0, 0), new Autoscaling.Metrics(0.7, 0.8, 0.9))); try (Mutex lock = applications().lock(app1Id)) { applications().put(app1.with(cluster1), lock); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepoStatsTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepoStatsTest.java index 0a26678d37e..b2e04ba2233 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepoStatsTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepoStatsTest.java @@ -15,7 +15,6 @@ import com.yahoo.vespa.hosted.provision.autoscale.NodeMetricSnapshot; import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester; import org.junit.Test; -import java.time.Duration; import java.util.List; import static org.junit.Assert.assertEquals; @@ -99,7 +98,7 @@ public class NodeRepoStatsTest { else { loadFactor = loadApp3; } - var snapshot = new NodeMetricSnapshot(now, new Load(1.0, 0.9, 0.8).multiply(loadFactor), 1, true, true, 1.0 ); + var snapshot = new NodeMetricSnapshot(now, new Load(1.0, 0.9, 0.8, 0, 0).multiply(loadFactor), 1, true, true, 1.0 ); tester.nodeRepository().metricsDb().addNodeMetrics(List.of(new Pair<>(node.hostname(), snapshot))); } @@ -108,8 +107,8 @@ public class NodeRepoStatsTest { assertEquals(26, stats.totalCost(), delta); assertEquals(8.319999999999999, stats.totalAllocatedCost(), delta); - assertLoad(new Load(0.6180,0.5562,0.4944), stats.load()); - assertLoad(new Load(0.4682,0.4214,0.3745), stats.activeLoad()); + assertLoad(new Load(0.6180,0.5562,0.4944, 0, 0), stats.load()); + assertLoad(new Load(0.4682,0.4214,0.3745, 0, 0), stats.activeLoad()); var app1Stats = stats.applicationStats().get(0); var app2Stats = stats.applicationStats().get(2); @@ -119,25 +118,27 @@ public class NodeRepoStatsTest { assertEquals(3.6400, app1Stats.cost(), delta); assertEquals(0.8676, app1Stats.utilizedCost(), delta); assertEquals(2.7724, app1Stats.unutilizedCost(), delta); - assertLoad(new Load(0.2571, 0.2314, 0.2057), app1Stats.load()); + assertLoad(new Load(0.2571, 0.2314, 0.2057, 0, 0), app1Stats.load()); assertEquals(app2, app2Stats.id()); assertEquals(2.0799, app2Stats.cost(), delta); assertEquals(0.7712, app2Stats.utilizedCost(), delta); assertEquals(1.3087, app2Stats.unutilizedCost(), delta); - assertLoad(new Load(.40, 0.36, 0.32), app2Stats.load()); + assertLoad(new Load(.40, 0.36, 0.32, 0, 0), app2Stats.load()); assertEquals(app3, app3Stats.id()); assertEquals(2.6000, app3Stats.cost(), delta); assertEquals(1.2049, app3Stats.utilizedCost(), delta); assertEquals(1.3950, app3Stats.unutilizedCost(), delta); - assertLoad(new Load(0.5, 0.45, 0.40), app3Stats.load()); + assertLoad(new Load(0.5, 0.45, 0.40, 0, 0), app3Stats.load()); } private static void assertLoad(Load expected, Load actual) { - assertEquals("cpu", expected.cpu(), actual.cpu(), delta); - assertEquals("memory", expected.memory(), actual.memory(), delta); - assertEquals("disk", expected.disk(), actual.disk(), delta); + assertEquals("cpu", expected.cpu(), actual.cpu(), delta); + assertEquals("memory", expected.memory(), actual.memory(), delta); + assertEquals("disk", expected.disk(), actual.disk(), delta); + assertEquals("gpu", expected.gpu(), actual.gpu(), delta); + assertEquals("gpuMemory", expected.gpuMemory(), actual.gpuMemory(), delta); } } 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 d4d34ab66e5..4236f7ac968 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 @@ -44,7 +44,7 @@ public class AutoscalingTest { .capacity(Capacity.from(min, max)) .build(); fixture.tester.clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(0.8f, 0.17, 0.12), 1, true, true, 100); + fixture.loader().applyLoad(new Load(0.8f, 0.17, 0.12, 0, 0), 1, true, true, 100); var result = fixture.autoscale(); assertTrue(result.resources().isEmpty()); assertEquals(Autoscaling.Status.insufficient, result.status()); @@ -63,13 +63,13 @@ public class AutoscalingTest { .capacity(Capacity.from(min, max)) .build(); fixture.tester.clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(0.8f, 0.17, 0.12), 1, true, true, 100); + fixture.loader().applyLoad(new Load(0.8f, 0.17, 0.12, 0, 0), 1, true, true, 100); var result = fixture.autoscale(); assertTrue(result.resources().isEmpty()); assertEquals(Autoscaling.Status.insufficient, result.status()); fixture.tester.clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(0.08f, 0.17, 0.12), 1, true, true, 100); + fixture.loader().applyLoad(new Load(0.08f, 0.17, 0.12, 0, 0), 1, true, true, 100); fixture.tester().assertResources("Scaling down", 8, 1, 16, 32, 200, fixture.autoscale()); @@ -128,8 +128,8 @@ public class AutoscalingTest { @Test public void test_autoscaling_up_is_fast() { var fixture = DynamicProvisioningTester.fixture().awsProdSetup(true).build(); - fixture.loader().applyLoad(new Load(0.1, 0.1, 0.1), 3); - fixture.loader().applyLoad(new Load(1.0, 1.0, 1.0), 1); + fixture.loader().applyLoad(new Load(0.1, 0.1, 0.1, 0, 0), 3); + fixture.loader().applyLoad(new Load(1.0, 1.0, 1.0, 0, 0), 1); fixture.tester().assertResources("Scaling up since resource usage is too high", 8, 1, 5.3, 17.0, 75.1, fixture.autoscale()); @@ -148,7 +148,7 @@ public class AutoscalingTest { .build(); fixture.tester().setScalingDuration(fixture.applicationId(), fixture.clusterSpec.id(), Duration.ofMinutes(5)); - fixture.loader().applyLoad(new Load(0.01, 0.38, 0), 5); + fixture.loader().applyLoad(new Load(0.01, 0.38, 0, 0, 0), 5); fixture.tester().assertResources("Scaling down", 2, 1, 4, 8, 50, fixture.autoscale()); @@ -190,7 +190,7 @@ public class AutoscalingTest { public void test_only_autoscaling_up_quickly() { var fixture = DynamicProvisioningTester.fixture().awsProdSetup(true).build(); fixture.setScalingDuration(Duration.ofHours(12)); // Fixture sets last completion to be 1 day into the past - fixture.loader().applyLoad(new Load(1.0, 0.1, 1.0), 10); + fixture.loader().applyLoad(new Load(1.0, 0.1, 1.0, 0, 0), 10); fixture.tester().assertResources("Scaling up (only) since resource usage is too high", 5, 1, 11.7, 14.9, 131.5, fixture.autoscale()); @@ -202,7 +202,7 @@ public class AutoscalingTest { var fixture = DynamicProvisioningTester.fixture().awsProdSetup(true).build(); fixture.setScalingDuration(Duration.ofHours(12)); // Fixture sets last completion to be 1 day into the past fixture.tester.clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(1.0, 0.1, 1.0), 10); + fixture.loader().applyLoad(new Load(1.0, 0.1, 1.0, 0, 0), 10); fixture.tester().assertResources("Scaling cpu and disk up and memory down", 5, 1, 11.7, 4.0, 131.5, fixture.autoscale()); @@ -213,7 +213,7 @@ public class AutoscalingTest { var fixture = DynamicProvisioningTester.fixture().awsProdSetup(false).build(); fixture.setScalingDuration(Duration.ofHours(6)); fixture.tester.clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(1.0, 0.1, 1.0), 10); + fixture.loader().applyLoad(new Load(1.0, 0.1, 1.0, 0, 0), 10); fixture.tester().assertResources("Scaling cpu and disk up, memory follows", 16, 1, 4, 8.0, 28.3, fixture.autoscale()); @@ -264,7 +264,7 @@ public class AutoscalingTest { .clusterType(ClusterSpec.Type.container) .awsProdSetup(false) .build(); - var duration = fixture.loader().addMeasurements(new Load(0.04, 0.39, 0.01), 20); + var duration = fixture.loader().addMeasurements(new Load(0.04, 0.39, 0.01, 0, 0), 20); fixture.tester().clock().advance(duration.negated()); fixture.loader().zeroTraffic(20, 1); fixture.tester().assertResources("Scaled down", @@ -358,7 +358,7 @@ public class AutoscalingTest { fixture.setScalingDuration(Duration.ofHours(6)); fixture.tester().clock().advance(Duration.ofDays(1)); - fixture.loader().applyLoad(new Load(0.25, 0.95, 0.95), 120); + fixture.loader().applyLoad(new Load(0.25, 0.95, 0.95, 0, 0), 120); fixture.tester().assertResources("Scaling up to limit since resource usage is too high", 6, 1, 2.4, 78.0, 79.0, fixture.autoscale()); @@ -372,7 +372,7 @@ public class AutoscalingTest { // deploy fixture.tester().clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(0.05f, 0.05f, 0.05f), 120); + fixture.loader().applyLoad(new Load(0.05f, 0.05f, 0.05f, 0, 0), 120); fixture.tester().assertResources("Scaling down to limit since resource usage is low", 4, 1, 1.8, 7.4, 23.4, fixture.autoscale()); @@ -395,7 +395,7 @@ public class AutoscalingTest { 2, 1, defaultResources, fixture.nodes().toResources()); fixture.tester().clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(0.25, 0.95, 0.95), 120); + fixture.loader().applyLoad(new Load(0.25, 0.95, 0.95, 0, 0), 120); fixture.tester().assertResources("Scaling up", 5, 1, defaultResources.vcpu(), defaultResources.memoryGb(), defaultResources.diskGb(), @@ -461,7 +461,7 @@ public class AutoscalingTest { .build(); fixture.tester().clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(0.01, 0.01, 0.01), 120); + fixture.loader().applyLoad(new Load(0.01, 0.01, 0.01, 0, 0), 120); Autoscaling suggestion = fixture.suggest(); fixture.tester().assertResources("Choosing the remote disk flavor as it has less disk", 2, 1, 3.0, 100.0, 10.0, @@ -498,7 +498,7 @@ public class AutoscalingTest { public void not_using_out_of_service_measurements() { var fixture = DynamicProvisioningTester.fixture().awsProdSetup(true).build(); fixture.tester().clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(0.9, 0.6, 0.7), 1, false, true, 120); + fixture.loader().applyLoad(new Load(0.9, 0.6, 0.7, 0, 0), 1, false, true, 120); assertTrue("Not scaling up since nodes were measured while cluster was out of service", fixture.autoscale().resources().isEmpty()); } @@ -507,7 +507,7 @@ public class AutoscalingTest { public void not_using_unstable_measurements() { var fixture = DynamicProvisioningTester.fixture().awsProdSetup(true).build(); fixture.tester().clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(0.9, 0.6, 0.7), 1, true, false, 120); + fixture.loader().applyLoad(new Load(0.9, 0.6, 0.7, 0, 0), 1, true, false, 120); assertTrue("Not scaling up since nodes were measured while cluster was unstable", fixture.autoscale().resources().isEmpty()); } @@ -536,7 +536,7 @@ public class AutoscalingTest { .build(); fixture.setScalingDuration(Duration.ofHours(6)); fixture.tester().clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(0.5, 0.8, 0.1), 120); + fixture.loader().applyLoad(new Load(0.5, 0.8, 0.1, 0, 0), 120); fixture.tester().assertResources("Suggesting resources where disk is 3x memory (this is a content cluster)", 11, 1, 13.0, 60.0, 179.9, fixture.tester().suggest(fixture.applicationId, fixture.clusterSpec.id(), min, min)); @@ -557,7 +557,7 @@ public class AutoscalingTest { .build(); fixture.setScalingDuration(Duration.ofHours(6)); fixture.tester().clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(0.5, 0.8, 0.1), 120); + fixture.loader().applyLoad(new Load(0.5, 0.8, 0.1, 0, 0), 120); fixture.tester().assertResources("Suggesting resources where disk is 3x memory (this is a content cluster)", 13, 1, 36.0, 72.0, 900.0, fixture.tester().suggest(fixture.applicationId, fixture.clusterSpec.id(), min, min)); @@ -668,7 +668,7 @@ public class AutoscalingTest { .capacity(Capacity.from(min, max)) .build(); fixture.tester().clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(0.16, 0.02, 0.5), 120); + fixture.loader().applyLoad(new Load(0.16, 0.02, 0.5, 0, 0), 120); fixture.tester().assertResources("Scaling down memory", 6, 1, 2.1, 4.0, 96.2, fixture.autoscale()); @@ -826,7 +826,7 @@ public class AutoscalingTest { .zone(new Zone(Environment.dev, RegionName.from("us-east"))) .build(); fixture.tester().clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(1.0, 1.0, 1.0), 200); + fixture.loader().applyLoad(new Load(1.0, 1.0, 1.0, 0, 0), 200); assertTrue("Not attempting to scale up because policies dictate we'll only get one node", fixture.autoscale().resources().isEmpty()); } @@ -842,7 +842,7 @@ public class AutoscalingTest { .capacity(Capacity.from(min, max, IntRange.of(3, 5), false, true, Optional.empty(), ClusterInfo.empty())) .build(); fixture.tester().clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(1.0, 1.0, 1.0), 200); + fixture.loader().applyLoad(new Load(1.0, 1.0, 1.0, 0, 0), 200); assertEquals("Don't autoscale: Autoscaling is disabled in single node clusters", fixture.autoscale().toString()); } @@ -866,7 +866,7 @@ public class AutoscalingTest { .zone(new Zone(Environment.dev, RegionName.from("us-east"))) .build(); fixture.tester().clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(1.0, 1.0, 1.0), 200); + fixture.loader().applyLoad(new Load(1.0, 1.0, 1.0, 0, 0), 200); fixture.tester().assertResources("We scale even in dev because resources are 'required'", 3, 1, 1.0, 13.4, 62.5, fixture.autoscale()); @@ -889,7 +889,7 @@ public class AutoscalingTest { .zone(new Zone(Environment.dev, RegionName.from("us-east"))) .build(); fixture.tester().clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(1.0, 1.0, 1.0), 200); + fixture.loader().applyLoad(new Load(1.0, 1.0, 1.0, 0, 0), 200); fixture.tester().assertResources("We scale even in dev because resources are required", 3, 1, 1.5, 8, 50, fixture.autoscale()); @@ -927,13 +927,13 @@ public class AutoscalingTest { fixture.currentResources().advertisedResources()); fixture.tester().deploy(fixture.applicationId(), clusterSpec(false), fixture.capacity()); - fixture.loader().applyLoad(new Load(0.1, 0.1, 0.1), 5); + fixture.loader().applyLoad(new Load(0.1, 0.1, 0.1, 0, 0), 5); fixture.tester().assertResources("Exclusive nodes makes no difference here", 2, 1, 4, 8, 100.0, fixture.autoscale()); fixture.tester().deploy(fixture.applicationId(), clusterSpec(true), fixture.capacity()); - fixture.loader().applyLoad(new Load(0.1, 0.1, 0.1), 5); + fixture.loader().applyLoad(new Load(0.1, 0.1, 0.1, 0, 0), 5); fixture.tester().assertResources("Reverts to the initial resources", 2, 1, 4, 8, 100, fixture.currentResources().advertisedResources()); @@ -952,7 +952,7 @@ public class AutoscalingTest { .build(); var initialNodes = fixture.nodes().asList(); fixture.tester().clock().advance(Duration.ofDays(2)); - fixture.loader().applyLoad(new Load(0.06, 0.52, 0.27), 100); + fixture.loader().applyLoad(new Load(0.06, 0.52, 0.27, 0, 0), 100); var autoscaling = fixture.autoscale(); fixture.tester().assertResources("Scaling down", 7, 1, 2, 15.8, 384.0, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Loader.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Loader.java index 2e953a0f67c..8dc3945223f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Loader.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Loader.java @@ -49,7 +49,7 @@ public class Loader { var idealLoad = fixture.clusterModel().idealLoad(); NodeList nodes = fixture.nodes(); float oneExtraNodeFactor = (float)(nodes.size() - 1.0) / (nodes.size()); - Load load = new Load(value, idealLoad.memory(), idealLoad.disk()).multiply(oneExtraNodeFactor); + Load load = new Load(value, idealLoad.memory(), idealLoad.disk(), 0, 0).multiply(oneExtraNodeFactor); Instant initialTime = fixture.tester().clock().instant(); for (int i = 0; i < count; i++) { fixture.tester().clock().advance(samplingInterval); @@ -101,7 +101,7 @@ public class Loader { var idealLoad = fixture.clusterModel().idealLoad(); NodeList nodes = fixture.nodes(); float oneExtraNodeFactor = (float)(nodes.size() - 1.0) / (nodes.size()); - Load load = new Load(idealLoad.cpu(), value, idealLoad.disk()).multiply(oneExtraNodeFactor); + Load load = new Load(idealLoad.cpu(), value, idealLoad.disk(), 0, 0).multiply(oneExtraNodeFactor); for (int i = 0; i < count; i++) { fixture.tester().clock().advance(samplingInterval); for (Node node : nodes) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcherTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcherTest.java index 4ec4ecd6d84..a984306b577 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcherTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcherTest.java @@ -99,6 +99,8 @@ public class MetricsV2MetricsFetcherTest { assertEquals("host-3.yahoo.com", values.get(0).getFirst()); assertEquals(0.13, values.get(0).getSecond().load().cpu(), delta); assertEquals(0.9375, values.get(0).getSecond().load().memory(), delta); + assertEquals(0.13, values.get(0).getSecond().load().gpu(), delta); + assertEquals(0.9375, values.get(0).getSecond().load().gpuMemory(), delta); assertFalse("Unstable because buckets are being merged", values.get(0).getSecond().stable()); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDbTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDbTest.java index e8d1368de71..71ed5bafc3d 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDbTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDbTest.java @@ -40,7 +40,7 @@ public class NodeMetricsDbTest { Collection<Pair<String, NodeMetricSnapshot>> values = new ArrayList<>(); for (int i = 0; i < 40; i++) { values.add(new Pair<>(node0, new NodeMetricSnapshot(clock.instant(), - new Load(0.9, 0.6, 0.6), + new Load(0.9, 0.6, 0.6, 0, 0), 0, true, false, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDbTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDbTest.java index d52ec12d486..96588250674 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDbTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDbTest.java @@ -57,6 +57,8 @@ public class QuestMetricsDbTest { assertEquals(0.1, snapshot.load().cpu(), delta); assertEquals(0.2, snapshot.load().memory(), delta); assertEquals(0.4, snapshot.load().disk(), delta); + assertEquals(0.5, snapshot.load().gpu(), delta); + assertEquals(0.6, snapshot.load().gpuMemory(), delta); assertEquals(1, snapshot.generation(), delta); assertEquals(30, snapshot.queryRate(), delta); @@ -230,7 +232,7 @@ public class QuestMetricsDbTest { for (int i = 1; i <= countPerHost; i++) { for (String host : hosts) timeseries.add(new Pair<>(host, new NodeMetricSnapshot(clock.instant(), - new Load(i * 0.1, i * 0.2, i * 0.4), + new Load(i * 0.1, i * 0.2, i * 0.4, i * 0.5, i * 0.6), i % 100, true, true, @@ -244,7 +246,7 @@ public class QuestMetricsDbTest { Collection<Pair<String, NodeMetricSnapshot>> timeseries = new ArrayList<>(); for (int i = 1; i <= countPerHost; i++) { for (String host : hosts) - timeseries.add(new Pair<>(host, new NodeMetricSnapshot(at, new Load(i * 0.1, i * 0.2, i * 0.4), + timeseries.add(new Pair<>(host, new NodeMetricSnapshot(at, new Load(i * 0.1, i * 0.2, i * 0.4, i * 0.5, i * 0.6), i % 100, true, false, 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 e4a712d3898..4f9b2de4da0 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 @@ -77,7 +77,7 @@ public class AutoscalingMaintainerTester { for (Node node : nodes) nodeRepository().metricsDb().addNodeMetrics(List.of(new Pair<>(node.hostname(), new NodeMetricSnapshot(clock().instant(), - new Load(cpu, mem, disk), + new Load(cpu, mem, disk, 0, 0), generation, true, true, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java index d4771594569..152f743900b 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java @@ -270,7 +270,7 @@ public class MetricsReporterTest { Optional.empty(), tester.clock().instant(), Load.zero(), - new Load(0.1, 0.2, 0.3), + new Load(0.1, 0.2, 0.3, 0, 0), Autoscaling.Metrics.zero())); tester.nodeRepository().applications().put(application.with(cluster), tester.nodeRepository().applications().lock(applicationId)); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java index 0a78874405d..f8be27300fe 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java @@ -139,7 +139,7 @@ public class ScalingSuggestionsMaintainerTest { for (Node node : nodes) nodeRepository.metricsDb().addNodeMetrics(List.of(new Pair<>(node.hostname(), new NodeMetricSnapshot(nodeRepository.clock().instant(), - new Load(cpu, memory, disk), + new Load(cpu, memory, disk, 0, 0), generation, true, true, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializerTest.java index 7a00c84faf6..918a9043c93 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializerTest.java @@ -57,8 +57,8 @@ public class ApplicationSerializerTest { Optional.of(new ClusterResources(20, 10, new NodeResources(0.5, 4, 14, 16))), Instant.ofEpochMilli(1234L), - new Load(0.1, 0.2, 0.3), - new Load(0.4, 0.5, 0.6), + new Load(0.1, 0.2, 0.3, 0.4, 0.5), + new Load(0.4, 0.5, 0.6, 0.7, 0.8), new Autoscaling.Metrics(0.7, 0.8, 0.9)), new Autoscaling(Autoscaling.Status.insufficient, "Autoscaling status", diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json index 28bde7bd966..7b2cf1dc8e4 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json @@ -63,12 +63,16 @@ "peak" : { "cpu" : 0.0, "memory" : 0.0, - "disk" : 0.0 + "disk" : 0.0, + "gpu": 0.0, + "gpuMemory": 0.0 }, "ideal" : { "cpu" : 0.0, "memory" : 0.0, - "disk" : 0.0 + "disk" : 0.0, + "gpu": 0.0, + "gpuMemory": 0.0 }, "metrics" : { "queryRate" : 0.0, @@ -96,12 +100,16 @@ "peak" : { "cpu" : 0.1, "memory" : 0.2, - "disk" : 0.3 + "disk" : 0.3, + "gpu": 0.0, + "gpuMemory": 0.0 }, "ideal" : { "cpu" : 0.4, "memory" : 0.5, - "disk" : 0.6 + "disk" : 0.6, + "gpu": 0.0, + "gpuMemory": 0.0 }, "metrics" : { "queryRate" : 0.7, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json index 05a62ff944d..10173089f75 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json @@ -50,12 +50,16 @@ "peak" : { "cpu" : 0.0, "memory" : 0.0, - "disk" : 0.0 + "disk" : 0.0, + "gpu": 0.0, + "gpuMemory": 0.0 }, "ideal" : { "cpu" : 0.0, "memory" : 0.0, - "disk" : 0.0 + "disk" : 0.0, + "gpu": 0.0, + "gpuMemory": 0.0 }, "metrics" : { "queryRate" : 0.0, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/stats.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/stats.json index 788eb6d359f..b031e0deba0 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/stats.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/stats.json @@ -4,12 +4,16 @@ "load": { "cpu": 0.0, "memory": 0.0, - "disk": 0.0 + "disk": 0.0, + "gpu": 0.0, + "gpuMemory": 0.0 }, "activeLoad": { "cpu": 0.0, "memory": 0.0, - "disk": 0.0 + "disk": 0.0, + "gpu": 0.0, + "gpuMemory": 0.0 }, "applications": [ { @@ -17,7 +21,9 @@ "load": { "cpu": 0.0, "memory": 0.0, - "disk": 0.0 + "disk": 0.0, + "gpu": 0.0, + "gpuMemory": 0.0 }, "cost": 0.0, "unutilizedCost": 0.0 @@ -27,7 +33,9 @@ "load": { "cpu": 0.0, "memory": 0.0, - "disk": 0.0 + "disk": 0.0, + "gpu": 0.0, + "gpuMemory": 0.0 }, "cost": 0.0, "unutilizedCost": 0.0 @@ -37,7 +45,9 @@ "load": { "cpu": 0.0, "memory": 0.0, - "disk": 0.0 + "disk": 0.0, + "gpu": 0.0, + "gpuMemory": 0.0 }, "cost": 0.0, "unutilizedCost": 0.0 @@ -47,7 +57,9 @@ "load": { "cpu": 0.0, "memory": 0.0, - "disk": 0.0 + "disk": 0.0, + "gpu": 0.0, + "gpuMemory": 0.0 }, "cost": 0.0, "unutilizedCost": 0.0 diff --git a/parent/pom.xml b/parent/pom.xml index 45ea90fdb41..0506c87493c 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -317,7 +317,7 @@ --> <groupId>org.openrewrite.maven</groupId> <artifactId>rewrite-maven-plugin</artifactId> - <version>5.16.0</version> + <version>5.17.0</version> <configuration> <activeRecipes> <recipe>org.openrewrite.java.testing.junit5.JUnit5BestPractices</recipe> diff --git a/searchcore/src/tests/proton/matching/query_test.cpp b/searchcore/src/tests/proton/matching/query_test.cpp index bf574af725d..e798f87dd12 100644 --- a/searchcore/src/tests/proton/matching/query_test.cpp +++ b/searchcore/src/tests/proton/matching/query_test.cpp @@ -30,11 +30,11 @@ #include <vespa/document/datatype/positiondatatype.h> #include <vespa/vespalib/stllike/asciistream.h> #include <vespa/vespalib/util/thread_bundle.h> - #include <vespa/vespalib/testkit/testapp.h> +#include <vespa/searchlib/query/tree/querytreecreator.h> + #include <vespa/log/log.h> LOG_SETUP("query_test"); -#include <vespa/searchlib/query/tree/querytreecreator.h> using document::PositionDataType; using search::fef::FieldInfo; @@ -80,70 +80,6 @@ using CollectionType = FieldInfo::CollectionType; namespace proton::matching { namespace { -class Test : public vespalib::TestApp { - MatchData::UP _match_data; - Blueprint::UP _blueprint; - FakeRequestContext _requestContext; - - void setUp(); - void tearDown(); - - void requireThatMatchDataIsReserved(); - void requireThatMatchDataIsReservedForEachFieldInAView(); - void requireThatTermsAreLookedUp(); - void requireThatTermsAreLookedUpInMultipleFieldsFromAView(); - void requireThatAttributeTermsAreLookedUpInAttributeSource(); - void requireThatAttributeTermDataHandlesAreAllocated(); - void requireThatTermDataIsFilledIn(); - - SearchIterator::UP getIterator(Node &node, ISearchContext &context); - - void requireThatSingleIndexCanUseBlendingAsBlacklisting(); - void requireThatIteratorsAreBuiltWithBlending(); - void requireThatIteratorsAreBuiltForAllTermNodes(); - void requireThatNearIteratorsCanBeBuilt(); - void requireThatONearIteratorsCanBeBuilt(); - void requireThatPhraseIteratorsCanBeBuilt(); - - void requireThatUnknownFieldActsEmpty(); - void requireThatIllegalFieldsAreIgnored(); - void requireThatQueryGluesEverythingTogether(); - void requireThatLocationIsAddedTheCorrectPlace(); - void requireThatQueryAddsLocation(); - void requireThatQueryAddsLocationCutoff(); - void requireThatFakeFieldSearchDumpsDiffer(); - void requireThatNoDocsGiveZeroDocFrequency(); - void requireThatWeakAndBlueprintsAreCreatedCorrectly(); - void requireThatParallelWandBlueprintsAreCreatedCorrectly(); - void requireThatWhiteListBlueprintCanBeUsed(); - void requireThatRankBlueprintStaysOnTopAfterWhiteListing(); - void requireThatAndNotBlueprintStaysOnTopAfterWhiteListing(); - void requireThatSameElementTermsAreProperlyPrefixed(); - void requireThatSameElementAllocatesMatchData(); - void requireThatSameElementIteratorsCanBeBuilt(); - void requireThatConstBoolBlueprintsAreCreatedCorrectly(); - void global_filter_is_calculated_and_handled(); - -public: - ~Test() override; - int Main() override; -}; - -#define TEST_CALL(func) \ - TEST_DO(setUp()); \ - TEST_DO(func()); \ - TEST_DO(tearDown()) - -void Test::setUp() { - _match_data.reset(); - _blueprint.reset(); -} - -void Test::tearDown() { - _match_data.reset(); - _blueprint.reset(); -} - const string field = "field"; const string loc_field = "location"; const string resolved_field1 = "resolved1"; @@ -167,6 +103,55 @@ fef_test::IndexEnvironment plain_index_env; fef_test::IndexEnvironment resolved_index_env; fef_test::IndexEnvironment attribute_index_env; +void setupIndexEnvironments() +{ + plain_index_env.getFields().emplace_back(FieldType::INDEX, CollectionType::SINGLE, field, field_id); + + resolved_index_env.getFields().emplace_back(FieldType::INDEX, CollectionType::SINGLE, resolved_field1, field_id); + resolved_index_env.getFields().emplace_back(FieldType::INDEX, CollectionType::SINGLE, resolved_field2, field_id + 1); + + attribute_index_env.getFields().emplace_back(FieldType::ATTRIBUTE, CollectionType::SINGLE, field, 0); + FieldInfo loc_field_info = FieldInfo(FieldType::ATTRIBUTE, CollectionType::SINGLE, + PositionDataType::getZCurveFieldName(loc_field), field_id + 1); + plain_index_env.getFields().push_back(loc_field_info); + attribute_index_env.getFields().push_back(loc_field_info); +} +struct InitializeGlobals { + InitializeGlobals() { setupIndexEnvironments(); } +}; + +InitializeGlobals globals; + +struct Fixture { + Fixture(); + SearchIterator::UP getIterator(Node &node, ISearchContext &context); + MatchData::UP _match_data; + Blueprint::UP _blueprint; + FakeRequestContext _requestContext; +}; + +Fixture::Fixture() + : _match_data(), + _blueprint(), + _requestContext() +{ +} + +SearchIterator::UP +Fixture::getIterator(Node &node, ISearchContext &context) { + MatchDataLayout mdl; + MatchDataReserveVisitor mdr_visitor(mdl); + node.accept(mdr_visitor); + _match_data = mdl.createMatchData(); + + _blueprint = BlueprintBuilder::build(_requestContext, node, context); + + _blueprint->fetchPostings(ExecuteInfo::TRUE); + SearchIterator::UP search(_blueprint->createSearch(*_match_data, true)); + search->initFullRange(); + return search; +} + vespalib::ThreadBundle &ttb() { return vespalib::ThreadBundle::trivial(); } vespalib::string @@ -180,23 +165,6 @@ termAsString(const vespalib::string & term) { return term; } -void setupIndexEnvironments() -{ - FieldInfo field_info(FieldType::INDEX, CollectionType::SINGLE, field, field_id); - plain_index_env.getFields().push_back(field_info); - - FieldInfo field_info1(FieldType::INDEX, CollectionType::SINGLE, resolved_field1, field_id); - resolved_index_env.getFields().push_back(field_info1); - FieldInfo field_info2(FieldType::INDEX, CollectionType::SINGLE, resolved_field2, field_id + 1); - resolved_index_env.getFields().push_back(field_info2); - - FieldInfo attr_info(FieldType::ATTRIBUTE, CollectionType::SINGLE, field, 0); - attribute_index_env.getFields().push_back(attr_info); - FieldInfo loc_field_info = FieldInfo(FieldType::ATTRIBUTE, CollectionType::SINGLE, - PositionDataType::getZCurveFieldName(loc_field), field_id + 1); - plain_index_env.getFields().push_back(loc_field_info); - attribute_index_env.getFields().push_back(loc_field_info); -} Node::UP buildQueryTree(const ViewResolver &resolver, const search::fef::IIndexEnvironment &idxEnv) @@ -234,7 +202,7 @@ Node::UP buildSameElementQueryTree(const ViewResolver &resolver, return node; } -void Test::requireThatMatchDataIsReserved() { +TEST("requireThatMatchDataIsReserved") { Node::UP node = buildQueryTree(ViewResolver(), plain_index_env); MatchDataLayout mdl; @@ -252,7 +220,7 @@ ViewResolver getViewResolver() { return resolver; } -void Test::requireThatMatchDataIsReservedForEachFieldInAView() { +TEST("requireThatMatchDataIsReservedForEachFieldInAView") { Node::UP node = buildQueryTree(getViewResolver(), resolved_index_env); MatchDataLayout mdl; @@ -265,8 +233,6 @@ void Test::requireThatMatchDataIsReservedForEachFieldInAView() { class LookupTestCheckerVisitor : public CustomTypeTermVisitor<ProtonNodeTypes> { - int Main() { return 0; } - public: template <class TermType> void checkNode(const TermType &n, int estimatedHitCount, bool empty) { @@ -292,7 +258,7 @@ public: void visit(ProtonInTerm&) override {} }; -void Test::requireThatTermsAreLookedUp() { +TEST("requireThatTermsAreLookedUp") { FakeRequestContext requestContext; Node::UP node = buildQueryTree(ViewResolver(), plain_index_env); @@ -326,7 +292,7 @@ void Test::requireThatTermsAreLookedUp() { TEST_DO(node->accept(checker)); } -void Test::requireThatTermsAreLookedUpInMultipleFieldsFromAView() { +TEST("requireThatTermsAreLookedUpInMultipleFieldsFromAView") { Node::UP node = buildQueryTree(getViewResolver(), resolved_index_env); FakeRequestContext requestContext; @@ -362,7 +328,7 @@ void Test::requireThatTermsAreLookedUpInMultipleFieldsFromAView() { TEST_DO(node->accept(checker)); } -void Test::requireThatAttributeTermsAreLookedUpInAttributeSource() { +TEST("requireThatAttributeTermsAreLookedUpInAttributeSource") { const string term = "bar"; ProtonStringTerm node(term, field, 1, Weight(2)); node.resolve(ViewResolver(), attribute_index_env); @@ -377,12 +343,11 @@ void Test::requireThatAttributeTermsAreLookedUpInAttributeSource() { node.accept(visitor); Blueprint::UP blueprint = BlueprintBuilder::build(requestContext, node, context); - EXPECT_TRUE(!blueprint->getState().estimate().empty); EXPECT_EQUAL(1u, blueprint->getState().estimate().estHits); } -void Test::requireThatAttributeTermDataHandlesAreAllocated() { +TEST("requireThatAttributeTermDataHandlesAreAllocated") { const string term = "bar"; ProtonStringTerm node(term, field, 1, Weight(2)); node.resolve(ViewResolver(), attribute_index_env); @@ -395,9 +360,7 @@ void Test::requireThatAttributeTermDataHandlesAreAllocated() { node.accept(reserve_visitor); Blueprint::UP blueprint = BlueprintBuilder::build(requestContext, node, context); - MatchData::UP match_data = mdl.createMatchData(); - EXPECT_EQUAL(1u, match_data->getNumTermFields()); EXPECT_TRUE(node.field(0).attribute_field); } @@ -406,8 +369,6 @@ void Test::requireThatAttributeTermDataHandlesAreAllocated() { class SetUpTermDataTestCheckerVisitor : public CustomTypeTermVisitor<ProtonNodeTypes> { - int Main() { return 0; } - public: void visit(ProtonNumberTerm &) override {} void visit(ProtonLocationTerm &) override {} @@ -416,8 +377,7 @@ public: void visit(ProtonStringTerm &n) override { const ITermData &term_data = n; - EXPECT_EQUAL(string_weight.percent(), - term_data.getWeight().percent()); + EXPECT_EQUAL(string_weight.percent(), term_data.getWeight().percent()); EXPECT_EQUAL(1u, term_data.getPhraseLength()); EXPECT_EQUAL(string_id, term_data.getUniqueId()); EXPECT_EQUAL(term_data.numFields(), n.numFields()); @@ -445,7 +405,7 @@ public: void visit(ProtonInTerm&) override { } }; -void Test::requireThatTermDataIsFilledIn() { +TEST("requireThatTermDataIsFilledIn") { Node::UP node = buildQueryTree(getViewResolver(), resolved_index_env); FakeRequestContext requestContext; @@ -467,20 +427,6 @@ void Test::requireThatTermDataIsFilledIn() { ); } -SearchIterator::UP Test::getIterator(Node &node, ISearchContext &context) { - MatchDataLayout mdl; - MatchDataReserveVisitor mdr_visitor(mdl); - node.accept(mdr_visitor); - _match_data = mdl.createMatchData(); - - _blueprint = BlueprintBuilder::build(_requestContext, node, context); - - _blueprint->fetchPostings(ExecuteInfo::TRUE); - SearchIterator::UP search(_blueprint->createSearch(*_match_data, true)); - search->initFullRange(); - return search; -} - FakeIndexSearchable getFakeSearchable(const string &term, int doc1, int doc2) { FakeIndexSearchable source; source.getFake().addResult(field, term, @@ -488,31 +434,31 @@ FakeIndexSearchable getFakeSearchable(const string &term, int doc1, int doc2) { return source; } -void Test::requireThatSingleIndexCanUseBlendingAsBlacklisting() { +TEST_F("requireThatSingleIndexCanUseBlendingAsBlacklisting", Fixture) { QueryBuilder<ProtonNodeTypes> builder; builder.addStringTerm(string_term, field, 1, Weight(2)) .resolve(ViewResolver(), plain_index_env); Node::UP node = builder.build(); - ASSERT_TRUE(node.get()); + ASSERT_TRUE(node); FakeSearchContext context; context.addIdx(1).idx(0) = getFakeSearchable(string_term, 2, 5); context.selector().setSource(5, 1); - SearchIterator::UP iterator = getIterator(*node, context); - ASSERT_TRUE(iterator.get()); + SearchIterator::UP iterator = f.getIterator(*node, context); + ASSERT_TRUE(iterator); EXPECT_TRUE(!iterator->seek(1)); EXPECT_TRUE(!iterator->seek(2)); EXPECT_TRUE(iterator->seek(5)); iterator->unpack(5); } -void Test::requireThatIteratorsAreBuiltWithBlending() { +TEST_F("requireThatIteratorsAreBuiltWithBlending", Fixture) { QueryBuilder<ProtonNodeTypes> builder; builder.addStringTerm(string_term, field, 1, Weight(2)) .resolve(ViewResolver(), plain_index_env); Node::UP node = builder.build(); - ASSERT_TRUE(node.get()); + ASSERT_TRUE(node); FakeSearchContext context; context.addIdx(1).idx(0) = getFakeSearchable(string_term, 3, 7); @@ -520,8 +466,8 @@ void Test::requireThatIteratorsAreBuiltWithBlending() { context.selector().setSource(3, 1); context.selector().setSource(7, 1); - SearchIterator::UP iterator = getIterator(*node, context); - ASSERT_TRUE(iterator.get()); + SearchIterator::UP iterator = f.getIterator(*node, context); + ASSERT_TRUE(iterator); EXPECT_TRUE(!iterator->seek(1)); EXPECT_TRUE(iterator->seek(2)); @@ -530,9 +476,9 @@ void Test::requireThatIteratorsAreBuiltWithBlending() { EXPECT_TRUE(iterator->seek(7)); } -void Test::requireThatIteratorsAreBuiltForAllTermNodes() { +TEST_F("requireThatIteratorsAreBuiltForAllTermNodes", Fixture) { Node::UP node = buildQueryTree(ViewResolver(), plain_index_env); - ASSERT_TRUE(node.get()); + ASSERT_TRUE(node); FakeSearchContext context(42); context.addIdx(0).idx(0).getFake() @@ -547,8 +493,8 @@ void Test::requireThatIteratorsAreBuiltForAllTermNodes() { .addResult(field, substring_term, FakeResult().doc(23).pos(2)) .addResult(field, suffix_term, FakeResult().doc(42).pos(2)); - SearchIterator::UP iterator = getIterator(*node, context); - ASSERT_TRUE(iterator.get()); + SearchIterator::UP iterator = f.getIterator(*node, context); + ASSERT_TRUE(iterator); EXPECT_TRUE(!iterator->seek(1)); EXPECT_TRUE(iterator->seek(2)); @@ -560,7 +506,7 @@ void Test::requireThatIteratorsAreBuiltForAllTermNodes() { EXPECT_TRUE(iterator->seek(42)); } -void Test::requireThatNearIteratorsCanBeBuilt() { +TEST_F("requireThatNearIteratorsCanBeBuilt", Fixture) { QueryBuilder<ProtonNodeTypes> builder; builder.addNear(2, 4); builder.addStringTerm(string_term, field, 1, Weight(2)); @@ -569,7 +515,7 @@ void Test::requireThatNearIteratorsCanBeBuilt() { ViewResolver resolver; ResolveViewVisitor visitor(resolver, plain_index_env); node->accept(visitor); - ASSERT_TRUE(node.get()); + ASSERT_TRUE(node); FakeSearchContext context(8); context.addIdx(0).idx(0).getFake() @@ -578,13 +524,13 @@ void Test::requireThatNearIteratorsCanBeBuilt() { .addResult(field, string_term, FakeResult() .doc(4).pos(40).len(50).doc(8).pos(5).len(50)); - SearchIterator::UP iterator = getIterator(*node, context); - ASSERT_TRUE(iterator.get()); + SearchIterator::UP iterator = f.getIterator(*node, context); + ASSERT_TRUE(iterator); EXPECT_TRUE(!iterator->seek(4)); EXPECT_TRUE(iterator->seek(8)); } -void Test::requireThatONearIteratorsCanBeBuilt() { +TEST_F("requireThatONearIteratorsCanBeBuilt", Fixture) { QueryBuilder<ProtonNodeTypes> builder; builder.addONear(2, 4); builder.addStringTerm(string_term, field, 1, Weight(2)); @@ -593,7 +539,7 @@ void Test::requireThatONearIteratorsCanBeBuilt() { ViewResolver resolver; ResolveViewVisitor visitor(resolver, plain_index_env); node->accept(visitor); - ASSERT_TRUE(node.get()); + ASSERT_TRUE(node); FakeSearchContext context(8); context.addIdx(0).idx(0).getFake() @@ -602,13 +548,13 @@ void Test::requireThatONearIteratorsCanBeBuilt() { .addResult(field, prefix_term, FakeResult() .doc(4).pos(2).len(50).doc(8).pos(5).len(50)); - SearchIterator::UP iterator = getIterator(*node, context); - ASSERT_TRUE(iterator.get()); + SearchIterator::UP iterator = f.getIterator(*node, context); + ASSERT_TRUE(iterator); EXPECT_TRUE(!iterator->seek(4)); EXPECT_TRUE(iterator->seek(8)); } -void Test::requireThatPhraseIteratorsCanBeBuilt() { +TEST_F("requireThatPhraseIteratorsCanBeBuilt", Fixture) { QueryBuilder<ProtonNodeTypes> builder; builder.addPhrase(3, field, 0, Weight(42)); builder.addStringTerm(string_term, field, 1, Weight(2)); @@ -618,7 +564,7 @@ void Test::requireThatPhraseIteratorsCanBeBuilt() { ViewResolver resolver; ResolveViewVisitor visitor(resolver, plain_index_env); node->accept(visitor); - ASSERT_TRUE(node.get()); + ASSERT_TRUE(node); FakeSearchContext context(9); context.addIdx(0).idx(0).getFake() @@ -636,8 +582,8 @@ void Test::requireThatPhraseIteratorsCanBeBuilt() { .doc(5).pos(5).len(50) .doc(8).pos(4).len(50)); - SearchIterator::UP iterator = getIterator(*node, context); - ASSERT_TRUE(iterator.get()); + SearchIterator::UP iterator = f.getIterator(*node, context); + ASSERT_TRUE(iterator); EXPECT_TRUE(!iterator->seek(4)); EXPECT_TRUE(!iterator->seek(5)); EXPECT_TRUE(iterator->seek(8)); @@ -645,8 +591,7 @@ void Test::requireThatPhraseIteratorsCanBeBuilt() { EXPECT_TRUE(iterator->isAtEnd()); } -void -Test::requireThatUnknownFieldActsEmpty() +TEST_F("requireThatUnknownFieldActsEmpty", Fixture) { FakeSearchContext context; context.addIdx(0).idx(0).getFake() @@ -654,28 +599,25 @@ Test::requireThatUnknownFieldActsEmpty() .doc(4).pos(3).len(50) .doc(5).pos(2).len(50)); - ProtonNodeTypes::StringTerm - node(string_term, unknown_field, string_id, string_weight); + ProtonNodeTypes::StringTerm node(string_term, unknown_field, string_id, string_weight); node.resolve(ViewResolver(), plain_index_env); std::vector<const ITermData *> terms; TermDataExtractor::extractTerms(node, terms); - SearchIterator::UP iterator = getIterator(node, context); + SearchIterator::UP iterator = f.getIterator(node, context); ASSERT_TRUE(EXPECT_EQUAL(1u, terms.size())); EXPECT_EQUAL(0u, terms[0]->numFields()); - ASSERT_TRUE(iterator.get()); + ASSERT_TRUE(iterator); EXPECT_TRUE(!iterator->seek(1)); EXPECT_TRUE(iterator->isAtEnd()); } -void -Test::requireThatIllegalFieldsAreIgnored() +TEST("requireThatIllegalFieldsAreIgnored") { - ProtonNodeTypes::StringTerm - node(string_term, unknown_field, string_id, string_weight); + ProtonNodeTypes::StringTerm node(string_term, unknown_field, string_id, string_weight); node.resolve(ViewResolver(), plain_index_env); FakeRequestContext requestContext; @@ -686,14 +628,12 @@ Test::requireThatIllegalFieldsAreIgnored() node.accept(reserve_visitor); Blueprint::UP blueprint = BlueprintBuilder::build(requestContext, node, context); - EXPECT_EQUAL(0u, node.numFields()); - MatchData::UP match_data = mdl.createMatchData(); EXPECT_EQUAL(0u, match_data->getNumTermFields()); } -void Test::requireThatQueryGluesEverythingTogether() { +TEST("requireThatQueryGluesEverythingTogether") { QueryBuilder<ProtonNodeTypes> builder; builder.addStringTerm(string_term, field, 1, Weight(2)); string stack_dump = StackDumpCreator::create(*builder.build()); @@ -712,19 +652,18 @@ void Test::requireThatQueryGluesEverythingTogether() { MatchData::UP md = mdl.createMatchData(); EXPECT_EQUAL(1u, md->getNumTermFields()); - query.optimize(); + query.optimize(true); query.fetchPostings(ExecuteInfo::TRUE); SearchIterator::UP search = query.createSearch(*md); - ASSERT_TRUE(search.get()); + ASSERT_TRUE(search); } -void checkQueryAddsLocation(const string &loc_in, const string &loc_out) { +void +checkQueryAddsLocation(const string &loc_in, const string &loc_out) { fef_test::IndexEnvironment index_environment; - FieldInfo field_info(FieldType::INDEX, CollectionType::SINGLE, field, 0); - index_environment.getFields().push_back(field_info); - field_info = FieldInfo(FieldType::ATTRIBUTE, CollectionType::SINGLE, - PositionDataType::getZCurveFieldName(loc_field), 1); - index_environment.getFields().push_back(field_info); + index_environment.getFields().emplace_back(FieldType::INDEX, CollectionType::SINGLE, field, 0); + index_environment.getFields().emplace_back(FieldType::ATTRIBUTE, CollectionType::SINGLE, + PositionDataType::getZCurveFieldName(loc_field), 1); QueryBuilder<ProtonNodeTypes> builder; builder.addStringTerm(string_term, field, 1, Weight(2)); @@ -748,7 +687,7 @@ void checkQueryAddsLocation(const string &loc_in, const string &loc_out) { query.fetchPostings(ExecuteInfo::TRUE); SearchIterator::UP search = query.createSearch(*md); - ASSERT_TRUE(search.get()); + ASSERT_TRUE(search); if (!EXPECT_NOT_EQUAL(string::npos, search->asString().find(loc_out))) { fprintf(stderr, "search (missing loc_out '%s'): %s", loc_out.c_str(), search->asString().c_str()); @@ -794,7 +733,7 @@ void verifyThatRankBlueprintAndAndNotStaysOnTopAfterLocation(QueryBuilder<Proton EXPECT_TRUE(dynamic_cast<const FakeBlueprint *>(&root->getChild(1))); } -void Test::requireThatLocationIsAddedTheCorrectPlace() { +TEST("requireThatLocationIsAddedTheCorrectPlace") { { QueryBuilder<ProtonNodeTypes> builder; builder.addRank(2); @@ -809,20 +748,19 @@ void Test::requireThatLocationIsAddedTheCorrectPlace() { } } -void Test::requireThatQueryAddsLocation() { +TEST("requireThatQueryAddsLocation") { checkQueryAddsLocation("(2,10,10,3,0,1,0,0)", "{p:{x:10,y:10},r:3,b:{x:[7,13],y:[7,13]}}"); checkQueryAddsLocation("{p:{x:10,y:10},r:3}", "{p:{x:10,y:10},r:3,b:{x:[7,13],y:[7,13]}}"); checkQueryAddsLocation("{b:{x:[6,11],y:[8,15]},p:{x:10,y:10},r:3}", "{p:{x:10,y:10},r:3,b:{x:[7,11],y:[8,13]}}"); checkQueryAddsLocation("{a:12345,b:{x:[8,10],y:[8,10]},p:{x:10,y:10},r:3}", "{p:{x:10,y:10},r:3,a:12345,b:{x:[8,10],y:[8,10]}}"); } -void Test::requireThatQueryAddsLocationCutoff() { +TEST("requireThatQueryAddsLocationCutoff") { checkQueryAddsLocation("[2,10,11,23,24]", "{b:{x:[10,23],y:[11,24]}}"); checkQueryAddsLocation("{b:{y:[11,24],x:[10,23]}}", "{b:{x:[10,23],y:[11,24]}}"); } -void -Test::requireThatFakeFieldSearchDumpsDiffer() +TEST("requireThatFakeFieldSearchDumpsDiffer") { FakeRequestContext requestContext; uint32_t fieldId = 0; @@ -865,7 +803,7 @@ Test::requireThatFakeFieldSearchDumpsDiffer() EXPECT_NOT_EQUAL(s1->asString(), s4->asString()); } -void Test::requireThatNoDocsGiveZeroDocFrequency() { +TEST("requireThatNoDocsGiveZeroDocFrequency") { ProtonStringTerm node(string_term, field, string_id, string_weight); node.resolve(ViewResolver(), plain_index_env); FakeSearchContext context; @@ -882,7 +820,7 @@ void Test::requireThatNoDocsGiveZeroDocFrequency() { EXPECT_EQUAL(0.0, node.field(0).getDocFreq()); } -void Test::requireThatWeakAndBlueprintsAreCreatedCorrectly() { +TEST("requireThatWeakAndBlueprintsAreCreatedCorrectly") { using search::queryeval::WeakAndBlueprint; ProtonWeakAnd wand(123, "view"); @@ -915,7 +853,7 @@ void Test::requireThatWeakAndBlueprintsAreCreatedCorrectly() { EXPECT_EQUAL(3u, wbp->getChild(1).getState().estimate().estHits); } -void Test::requireThatParallelWandBlueprintsAreCreatedCorrectly() { +TEST("requireThatParallelWandBlueprintsAreCreatedCorrectly") { using search::queryeval::WeakAndBlueprint; ProtonWandTerm wand(2, field, 42, Weight(100), 123, 9000, 1.25); @@ -945,8 +883,7 @@ void Test::requireThatParallelWandBlueprintsAreCreatedCorrectly() { EXPECT_EQUAL(1000u, wbp->get_docid_limit()); } -void -Test::requireThatWhiteListBlueprintCanBeUsed() +TEST("requireThatWhiteListBlueprintCanBeUsed") { QueryBuilder<ProtonNodeTypes> builder; builder.addStringTerm("foo", field, field_id, string_weight); @@ -960,14 +897,14 @@ Test::requireThatWhiteListBlueprintCanBeUsed() .addResult(field, "foo", FakeResult().doc(1).doc(3).doc(5).doc(7).doc(9).doc(11)); context.setLimit(42); - query.setWhiteListBlueprint(SimpleBlueprint::UP(new SimpleBlueprint(SimpleResult().addHit(1).addHit(2).addHit(4).addHit(5).addHit(6).addHit(7).addHit(8).addHit(10).addHit(11).addHit(12)))); + query.setWhiteListBlueprint(std::make_unique<SimpleBlueprint>(SimpleResult().addHit(1).addHit(2).addHit(4).addHit(5).addHit(6).addHit(7).addHit(8).addHit(10).addHit(11).addHit(12))); FakeRequestContext requestContext; MatchDataLayout mdl; query.reserveHandles(requestContext, context, mdl); MatchData::UP md = mdl.createMatchData(); - query.optimize(); + query.optimize(true); query.fetchPostings(ExecuteInfo::TRUE); SearchIterator::UP search = query.createSearch(*md); SimpleResult exp = SimpleResult().addHit(1).addHit(5).addHit(7).addHit(11); @@ -1009,14 +946,14 @@ void verifyThatRankBlueprintAndAndNotStaysOnTopAfterWhiteListing(QueryBuilder<Pr EXPECT_TRUE(dynamic_cast<const SourceBlenderBlueprint *>(&root->getChild(1))); } -void Test::requireThatRankBlueprintStaysOnTopAfterWhiteListing() { +TEST("requireThatRankBlueprintStaysOnTopAfterWhiteListing") { QueryBuilder<ProtonNodeTypes> builder; builder.addRank(2); builder.addAndNot(2); verifyThatRankBlueprintAndAndNotStaysOnTopAfterWhiteListing<RankBlueprint, AndNotBlueprint>(builder); } -void Test::requireThatAndNotBlueprintStaysOnTopAfterWhiteListing() { +TEST("requireThatAndNotBlueprintStaysOnTopAfterWhiteListing") { QueryBuilder<ProtonNodeTypes> builder; builder.addAndNot(2); builder.addRank(2); @@ -1039,8 +976,7 @@ make_same_element_stack_dump(const vespalib::string &prefix, const vespalib::str return query; } -void -Test::requireThatSameElementTermsAreProperlyPrefixed() +TEST("requireThatSameElementTermsAreProperlyPrefixed") { search::query::Node::UP query = make_same_element_stack_dump("", ""); auto * root = dynamic_cast<search::query::SameElement *>(query.get()); @@ -1071,8 +1007,7 @@ Test::requireThatSameElementTermsAreProperlyPrefixed() EXPECT_EQUAL(dynamic_cast<ProtonStringTerm *>(root->getChildren()[1])->getView(), "abc.abc.f2"); } -void -Test::requireThatSameElementAllocatesMatchData() +TEST("requireThatSameElementAllocatesMatchData") { Node::UP node = buildSameElementQueryTree(ViewResolver(), plain_index_env); MatchDataLayout mdl; @@ -1082,8 +1017,7 @@ Test::requireThatSameElementAllocatesMatchData() EXPECT_EQUAL(1u, match_data->getNumTermFields()); } -void -Test::requireThatSameElementIteratorsCanBeBuilt() { +TEST_F("requireThatSameElementIteratorsCanBeBuilt", Fixture) { Node::UP node = buildSameElementQueryTree(ViewResolver(), plain_index_env); FakeSearchContext context(10); context.addIdx(0).idx(0).getFake() @@ -1091,13 +1025,13 @@ Test::requireThatSameElementIteratorsCanBeBuilt() { .doc(4).elem(1).pos(0).doc(8).elem(1).pos(0)) .addResult(field, prefix_term, FakeResult() .doc(4).elem(2).pos(0).doc(8).elem(1).pos(1)); - SearchIterator::UP iterator = getIterator(*node, context); - ASSERT_TRUE(iterator.get()); + SearchIterator::UP iterator = f.getIterator(*node, context); + ASSERT_TRUE(iterator); EXPECT_TRUE(!iterator->seek(4)); EXPECT_TRUE(iterator->seek(8)); } -void Test::requireThatConstBoolBlueprintsAreCreatedCorrectly() { +TEST("requireThatConstBoolBlueprintsAreCreatedCorrectly") { using search::queryeval::AlwaysTrueBlueprint; using search::queryeval::EmptyBlueprint; @@ -1123,8 +1057,7 @@ class GlobalFilterBlueprint : public SimpleBlueprint { public: std::shared_ptr<const GlobalFilter> filter; double estimated_hit_ratio; - GlobalFilterBlueprint(const SimpleResult& result, - bool want_global_filter) + GlobalFilterBlueprint(const SimpleResult& result, bool want_global_filter) : search::queryeval::SimpleBlueprint(result), filter(), estimated_hit_ratio(-1.0) @@ -1140,8 +1073,7 @@ public: GlobalFilterBlueprint::~GlobalFilterBlueprint() = default; -void -Test::global_filter_is_calculated_and_handled() +TEST("global_filter_is_calculated_and_handled") { // estimated hits = 3, estimated hit ratio = 0.3 auto result = SimpleResult().addHit(3).addHit(5).addHit(7); @@ -1183,52 +1115,7 @@ Test::global_filter_is_calculated_and_handled() } } -Test::~Test() = default; - -int -Test::Main() -{ - setupIndexEnvironments(); - - TEST_INIT("query_test"); - - TEST_CALL(requireThatMatchDataIsReserved); - TEST_CALL(requireThatMatchDataIsReservedForEachFieldInAView); - TEST_CALL(requireThatTermsAreLookedUp); - TEST_CALL(requireThatTermsAreLookedUpInMultipleFieldsFromAView); - TEST_CALL(requireThatAttributeTermsAreLookedUpInAttributeSource); - TEST_CALL(requireThatAttributeTermDataHandlesAreAllocated); - TEST_CALL(requireThatTermDataIsFilledIn); - TEST_CALL(requireThatSingleIndexCanUseBlendingAsBlacklisting); - TEST_CALL(requireThatIteratorsAreBuiltWithBlending); - TEST_CALL(requireThatIteratorsAreBuiltForAllTermNodes); - TEST_CALL(requireThatNearIteratorsCanBeBuilt); - TEST_CALL(requireThatONearIteratorsCanBeBuilt); - TEST_CALL(requireThatPhraseIteratorsCanBeBuilt); - TEST_CALL(requireThatUnknownFieldActsEmpty); - TEST_CALL(requireThatIllegalFieldsAreIgnored); - TEST_CALL(requireThatQueryGluesEverythingTogether); - TEST_CALL(requireThatLocationIsAddedTheCorrectPlace); - TEST_CALL(requireThatQueryAddsLocation); - TEST_CALL(requireThatQueryAddsLocationCutoff); - TEST_CALL(requireThatFakeFieldSearchDumpsDiffer); - TEST_CALL(requireThatNoDocsGiveZeroDocFrequency); - TEST_CALL(requireThatWeakAndBlueprintsAreCreatedCorrectly); - TEST_CALL(requireThatParallelWandBlueprintsAreCreatedCorrectly); - TEST_CALL(requireThatWhiteListBlueprintCanBeUsed); - TEST_CALL(requireThatRankBlueprintStaysOnTopAfterWhiteListing); - TEST_CALL(requireThatAndNotBlueprintStaysOnTopAfterWhiteListing); - TEST_CALL(requireThatSameElementTermsAreProperlyPrefixed); - TEST_CALL(requireThatSameElementAllocatesMatchData); - TEST_CALL(requireThatSameElementIteratorsCanBeBuilt); - TEST_CALL(requireThatConstBoolBlueprintsAreCreatedCorrectly); - TEST_CALL(global_filter_is_calculated_and_handled); - - TEST_DONE(); -} - - } // namespace } // namespace proton::matching -TEST_APPHOOK(proton::matching::Test); +TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp index ddc6441210a..148877ed6cc 100644 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp @@ -62,7 +62,6 @@ BucketDB::checkActiveCount() const { void BucketDB::unloadBucket(BucketId bucket, const BucketState &delta) { - checkActiveCount(); BucketState *state = getBucketStatePtr(bucket); assert(state); *state -= delta; diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp index 3bdf396c700..19ec623635d 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp @@ -203,14 +203,15 @@ MatchToolsFactory(QueryLimiter & queryLimiter, trace.addEvent(5, "Build query execution plan"); _query.reserveHandles(_requestContext, searchContext, _mdl); trace.addEvent(5, "Optimize query execution plan"); - _query.optimize(); + bool sort_by_cost = SortBlueprintsByCost::check(_queryEnv.getProperties(), rankSetup.sort_blueprints_by_cost()); + _query.optimize(sort_by_cost); trace.addEvent(4, "Perform dictionary lookups and posting lists initialization"); double hitRate = std::min(1.0, double(maxNumHits)/double(searchContext.getDocIdLimit())); _query.fetchPostings(ExecuteInfo::create(is_search, hitRate, _requestContext.getDoom(), thread_bundle)); if (is_search) { _query.handle_global_filter(_requestContext, searchContext.getDocIdLimit(), _attribute_blueprint_params.global_filter_lower_limit, - _attribute_blueprint_params.global_filter_upper_limit, trace); + _attribute_blueprint_params.global_filter_upper_limit, trace, sort_by_cost); } _query.freeze(); trace.addEvent(5, "Prepare shared state for multi-threaded rank executors"); diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.cpp b/searchcore/src/vespa/searchcore/proton/matching/query.cpp index 12911520666..a93e8fbbddc 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/query.cpp @@ -198,9 +198,10 @@ Query::reserveHandles(const IRequestContext & requestContext, ISearchContext &co } void -Query::optimize() +Query::optimize(bool sort_by_cost) { - _blueprint = Blueprint::optimize(std::move(_blueprint)); + (void) sort_by_cost; + _blueprint = Blueprint::optimize(std::move(_blueprint), sort_by_cost); LOG(debug, "optimized blueprint:\n%s\n", _blueprint->asString().c_str()); } @@ -213,7 +214,7 @@ Query::fetchPostings(const ExecuteInfo & executeInfo) void Query::handle_global_filter(const IRequestContext & requestContext, uint32_t docid_limit, double global_filter_lower_limit, double global_filter_upper_limit, - search::engine::Trace& trace) + search::engine::Trace& trace, bool sort_by_cost) { if (!handle_global_filter(*_blueprint, docid_limit, global_filter_lower_limit, global_filter_upper_limit, requestContext.thread_bundle(), &trace)) @@ -222,7 +223,7 @@ Query::handle_global_filter(const IRequestContext & requestContext, uint32_t doc } // optimized order may change after accounting for global filter: trace.addEvent(5, "Optimize query execution plan to account for global filter"); - _blueprint = Blueprint::optimize(std::move(_blueprint)); + _blueprint = Blueprint::optimize(std::move(_blueprint), sort_by_cost); LOG(debug, "blueprint after handle_global_filter:\n%s\n", _blueprint->asString().c_str()); // strictness may change if optimized order changed: fetchPostings(ExecuteInfo::create(true, 1.0, requestContext.getDoom(), requestContext.thread_bundle())); diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.h b/searchcore/src/vespa/searchcore/proton/matching/query.h index 39013f99fd4..2fc978ba3f9 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.h +++ b/searchcore/src/vespa/searchcore/proton/matching/query.h @@ -103,12 +103,12 @@ public: * testing becomes harder. Not calling this function enables the * test to verify the original query without optimization. **/ - void optimize(); + void optimize(bool sort_by_cost); void fetchPostings(const ExecuteInfo & executeInfo) ; void handle_global_filter(const IRequestContext & requestContext, uint32_t docid_limit, double global_filter_lower_limit, double global_filter_upper_limit, - search::engine::Trace& trace); + search::engine::Trace& trace, bool sort_by_cost); /** * Calculates and handles the global filter if needed by the blueprint tree. diff --git a/searchlib/CMakeLists.txt b/searchlib/CMakeLists.txt index c9e19b76cef..5628db99171 100644 --- a/searchlib/CMakeLists.txt +++ b/searchlib/CMakeLists.txt @@ -77,7 +77,7 @@ vespa_define_module( src/tests/attribute/compaction src/tests/attribute/dfa_fuzzy_matcher src/tests/attribute/direct_multi_term_blueprint - src/tests/attribute/document_weight_iterator + src/tests/attribute/direct_posting_store src/tests/attribute/enum_attribute_compaction src/tests/attribute/enum_comparator src/tests/attribute/enumeratedsave diff --git a/searchlib/src/main/java/com/yahoo/searchlib/gbdt/XmlHelper.java b/searchlib/src/main/java/com/yahoo/searchlib/gbdt/XmlHelper.java index fce0485f41a..60617687f44 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/gbdt/XmlHelper.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/gbdt/XmlHelper.java @@ -7,6 +7,7 @@ import org.w3c.dom.Node; import org.w3c.dom.NodeList; import org.xml.sax.SAXException; +import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; @@ -15,21 +16,21 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.util.LinkedList; import java.util.List; import java.util.Optional; +import java.util.logging.Level; +import java.util.logging.Logger; /** * @author Simon Thoresen Hult */ abstract class XmlHelper { - - private static final Charset UTF8 = Charset.forName("UTF-8"); - public static Element parseXml(String xml) throws ParserConfigurationException, IOException, SAXException { - return parseXmlStream(new ByteArrayInputStream(xml.getBytes(UTF8))); + return parseXmlStream(new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8))); } public static Element parseXmlFile(String fileName) @@ -41,22 +42,27 @@ abstract class XmlHelper { public static Element parseXmlStream(InputStream in) throws ParserConfigurationException, IOException, SAXException { - DocumentBuilderFactory factory = createDocumentBuilderFactory(); - DocumentBuilder builder = factory.newDocumentBuilder(); + DocumentBuilder builder = createDocumentBuilderFactory().newDocumentBuilder(); Document doc = builder.parse(in); return doc.getDocumentElement(); } - private static DocumentBuilderFactory createDocumentBuilderFactory() throws ParserConfigurationException { - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setNamespaceAware(true); - factory.setXIncludeAware(false); + private static DocumentBuilderFactory createDocumentBuilderFactory() { + try { + DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + factory.setNamespaceAware(true); + factory.setXIncludeAware(false); + factory.setExpandEntityReferences(false); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - // XXE prevention - factory.setFeature("http://xml.org/sax/features/external-general-entities", false); - factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - return factory; + // XXE prevention + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + return factory; + } catch (ParserConfigurationException e) { + throw new RuntimeException("Failed to initialize XML parser", e); + } } public static String getAttributeText(Node node, String name) { diff --git a/searchlib/src/tests/attribute/bitvector/bitvector_test.cpp b/searchlib/src/tests/attribute/bitvector/bitvector_test.cpp index 181c0fdf110..f612bdda87f 100644 --- a/searchlib/src/tests/attribute/bitvector/bitvector_test.cpp +++ b/searchlib/src/tests/attribute/bitvector/bitvector_test.cpp @@ -7,7 +7,7 @@ #include <vespa/searchlib/common/bitvectoriterator.h> #include <vespa/searchlib/fef/termfieldmatchdata.h> #include <vespa/searchlib/parsequery/parse.h> -#include <vespa/searchlib/queryeval/document_weight_search_iterator.h> +#include <vespa/searchlib/queryeval/docid_with_weight_search_iterator.h> #include <vespa/searchlib/queryeval/executeinfo.h> #include <vespa/searchlib/test/searchiteratorverifier.h> #include <vespa/searchlib/util/randomgenerator.h> @@ -432,7 +432,7 @@ BitVectorTest::test(BasicType bt, CollectionType ct, const vespalib::string &pre const auto* dww = v->as_docid_with_weight_posting_store(); if (dww != nullptr) { auto lres = dww->lookup(getSearchStr<VectorType>(), dww->get_dictionary_snapshot()); - using DWSI = search::queryeval::DocumentWeightSearchIterator; + using DWSI = search::queryeval::DocidWithWeightSearchIterator; TermFieldMatchData md; auto dwsi = std::make_unique<DWSI>(md, *dww, lres); if (!filter) { diff --git a/searchlib/src/tests/attribute/direct_posting_store/.gitignore b/searchlib/src/tests/attribute/direct_posting_store/.gitignore new file mode 100644 index 00000000000..5516bc721c7 --- /dev/null +++ b/searchlib/src/tests/attribute/direct_posting_store/.gitignore @@ -0,0 +1 @@ +searchlib_direct_posting_store_test_app diff --git a/searchlib/src/tests/attribute/direct_posting_store/CMakeLists.txt b/searchlib/src/tests/attribute/direct_posting_store/CMakeLists.txt new file mode 100644 index 00000000000..0cb45e6b18f --- /dev/null +++ b/searchlib/src/tests/attribute/direct_posting_store/CMakeLists.txt @@ -0,0 +1,9 @@ +# Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(searchlib_direct_posting_store_test_app TEST + SOURCES + direct_posting_store_test.cpp + DEPENDS + searchlib + searchlib_test +) +vespa_add_test(NAME searchlib_direct_posting_store_test_app COMMAND searchlib_direct_posting_store_test_app) diff --git a/searchlib/src/tests/attribute/document_weight_iterator/document_weight_iterator_test.cpp b/searchlib/src/tests/attribute/direct_posting_store/direct_posting_store_test.cpp index 28416d09d6f..d6b0ff2a4b6 100644 --- a/searchlib/src/tests/attribute/document_weight_iterator/document_weight_iterator_test.cpp +++ b/searchlib/src/tests/attribute/direct_posting_store/direct_posting_store_test.cpp @@ -8,14 +8,14 @@ #include <vespa/searchlib/attribute/attributememorysavetarget.h> #include <vespa/searchlib/attribute/i_docid_with_weight_posting_store.h> #include <vespa/searchlib/index/dummyfileheadercontext.h> -#include <vespa/searchlib/queryeval/document_weight_search_iterator.h> +#include <vespa/searchlib/queryeval/docid_with_weight_search_iterator.h> #include <vespa/searchlib/test/searchiteratorverifier.h> #include <vespa/searchlib/util/randomgenerator.h> #include <vespa/vespalib/test/insertion_operators.h> #include <vespa/vespalib/testkit/test_kit.h> #include <vespa/log/log.h> -LOG_SETUP("document_weight_iterator_test"); +LOG_SETUP("direct_posting_store_test"); using namespace search; using namespace search::attribute; @@ -80,12 +80,12 @@ struct StringFixture { } }; -TEST("require that appropriate attributes support the document weight attribute interface") { +TEST("require that appropriate attributes support the IDocidWithWeightPostingStore interface") { EXPECT_TRUE(make_attribute(BasicType::INT64, CollectionType::WSET, true)->as_docid_with_weight_posting_store() != nullptr); EXPECT_TRUE(make_attribute(BasicType::STRING, CollectionType::WSET, true)->as_docid_with_weight_posting_store() != nullptr); } -TEST("require that inappropriate attributes do not support the document weight attribute interface") { +TEST("require that inappropriate attributes do not support the IDocidWithWeightPostingStore interface") { EXPECT_TRUE(make_attribute(BasicType::INT64, CollectionType::SINGLE, false)->as_docid_with_weight_posting_store() == nullptr); EXPECT_TRUE(make_attribute(BasicType::INT64, CollectionType::ARRAY, false)->as_docid_with_weight_posting_store() == nullptr); EXPECT_TRUE(make_attribute(BasicType::INT64, CollectionType::WSET, false)->as_docid_with_weight_posting_store() == nullptr); @@ -199,7 +199,7 @@ public: ASSERT_TRUE(api != nullptr); auto dict_entry = api->lookup("123", api->get_dictionary_snapshot()); ASSERT_TRUE(dict_entry.posting_idx.valid()); - return std::make_unique<queryeval::DocumentWeightSearchIterator>(_tfmd, *api, dict_entry); + return std::make_unique<queryeval::DocidWithWeightSearchIterator>(_tfmd, *api, dict_entry); } private: mutable fef::TermFieldMatchData _tfmd; diff --git a/searchlib/src/tests/attribute/document_weight_iterator/.gitignore b/searchlib/src/tests/attribute/document_weight_iterator/.gitignore deleted file mode 100644 index 08cae9a48df..00000000000 --- a/searchlib/src/tests/attribute/document_weight_iterator/.gitignore +++ /dev/null @@ -1 +0,0 @@ -searchlib_document_weight_iterator_test_app diff --git a/searchlib/src/tests/attribute/document_weight_iterator/CMakeLists.txt b/searchlib/src/tests/attribute/document_weight_iterator/CMakeLists.txt deleted file mode 100644 index 4cb480068e3..00000000000 --- a/searchlib/src/tests/attribute/document_weight_iterator/CMakeLists.txt +++ /dev/null @@ -1,9 +0,0 @@ -# Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -vespa_add_executable(searchlib_document_weight_iterator_test_app TEST - SOURCES - document_weight_iterator_test.cpp - DEPENDS - searchlib - searchlib_test -) -vespa_add_test(NAME searchlib_document_weight_iterator_test_app COMMAND searchlib_document_weight_iterator_test_app) diff --git a/searchlib/src/tests/attribute/multi_term_or_filter_search/multi_term_or_filter_search_test.cpp b/searchlib/src/tests/attribute/multi_term_or_filter_search/multi_term_or_filter_search_test.cpp index dea2702ef0d..552a128c518 100644 --- a/searchlib/src/tests/attribute/multi_term_or_filter_search/multi_term_or_filter_search_test.cpp +++ b/searchlib/src/tests/attribute/multi_term_or_filter_search/multi_term_or_filter_search_test.cpp @@ -3,6 +3,7 @@ #include <vespa/searchlib/attribute/i_direct_posting_store.h> #include <vespa/searchlib/attribute/multi_term_or_filter_search.h> #include <vespa/searchlib/common/bitvector.h> +#include <vespa/searchlib/fef/termfieldmatchdata.h> #include <vespa/searchlib/queryeval/searchiterator.h> #include <vespa/vespalib/gtest/gtest.h> #define ENABLE_GTEST_MIGRATION @@ -11,13 +12,16 @@ using PostingList = search::attribute::PostingListTraits<int32_t>::PostingStoreBase; using Iterator = search::attribute::PostingListTraits<int32_t>::const_iterator; using KeyData = PostingList::KeyDataType; + using search::BitVector; using search::attribute::MultiTermOrFilterSearch; +using search::fef::TermFieldMatchData; using search::queryeval::SearchIterator; using vespalib::datastore::EntryRef; class MultiTermOrFilterSearchTest : public ::testing::Test { PostingList _postings; + mutable TermFieldMatchData _tfmd; vespalib::GenerationHandler _gens; std::vector<EntryRef> _trees; uint32_t _range_start; @@ -62,7 +66,7 @@ public: for (size_t i = 0; i < num_trees(); ++i) { iterators.emplace_back(get_tree(i)); } - auto result = MultiTermOrFilterSearch::create(std::move(iterators)); + auto result = MultiTermOrFilterSearch::create(std::move(iterators), _tfmd); result->initRange(_range_start, _range_end); return result; }; @@ -73,6 +77,8 @@ public: while (doc_id < _range_end) { if (iterator.seek(doc_id)) { result.emplace_back(doc_id); + iterator.unpack(doc_id); + EXPECT_EQ(doc_id, _tfmd.getDocId()); ++doc_id; } else { doc_id = std::max(doc_id + 1, iterator.getDocId()); diff --git a/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp b/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp index 8831bd1ec75..ecc03ac54c5 100644 --- a/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp +++ b/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp @@ -488,11 +488,11 @@ TEST("require that direct attribute iterators work") { EXPECT_TRUE(result.has_minmax); EXPECT_EQUAL(100, result.min_weight); EXPECT_EQUAL(1000, result.max_weight); - EXPECT_TRUE(result.iterator_dump.find("DocumentWeightSearchIterator") != vespalib::string::npos); + EXPECT_TRUE(result.iterator_dump.find("DocidWithWeightSearchIterator") != vespalib::string::npos); } else { EXPECT_EQUAL(num_docs, result.est_hits); EXPECT_FALSE(result.has_minmax); - EXPECT_TRUE(result.iterator_dump.find("DocumentWeightSearchIterator") == vespalib::string::npos); + EXPECT_TRUE(result.iterator_dump.find("DocidWithWeightSearchIterator") == vespalib::string::npos); } ASSERT_EQUAL(3u, result.hits.size()); EXPECT_FALSE(result.est_empty); @@ -513,7 +513,7 @@ TEST("require that single weighted set turns filter on filter fields") { SimpleStringTerm node("foo", "", 0, Weight(1)); Result result = do_search(attribute_manager, node, strict); EXPECT_EQUAL(3u, result.est_hits); - EXPECT_TRUE(result.iterator_dump.find("DocumentWeightSearchIterator") == vespalib::string::npos); + EXPECT_TRUE(result.iterator_dump.find("DocidWithWeightSearchIterator") == vespalib::string::npos); EXPECT_TRUE(result.iterator_dump.find("FilterAttributePostingListIteratorT") != vespalib::string::npos); ASSERT_EQUAL(3u, result.hits.size()); EXPECT_FALSE(result.est_empty); diff --git a/searchlib/src/tests/queryeval/blueprint/blueprint_test.cpp b/searchlib/src/tests/queryeval/blueprint/blueprint_test.cpp index eefca5c82e9..f800e124bdc 100644 --- a/searchlib/src/tests/queryeval/blueprint/blueprint_test.cpp +++ b/searchlib/src/tests/queryeval/blueprint/blueprint_test.cpp @@ -1,6 +1,7 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "mysearch.h" #include <vespa/vespalib/testkit/testapp.h> +#include <vespa/searchlib/queryeval/flow.h> #include <vespa/searchlib/queryeval/blueprint.h> #include <vespa/searchlib/queryeval/intermediate_blueprints.h> #include <vespa/vespalib/objects/objectdumper.h> @@ -22,8 +23,12 @@ class MyOr : public IntermediateBlueprint { private: public: - double calculate_cost() const final { return 1.0; } - double calculate_relative_estimate() const final { return 0.5; } + double calculate_cost() const final { + return cost_of(get_children(), OrFlow()); + } + double calculate_relative_estimate() const final { + return estimate_of(get_children(), OrFlow()); + } HitEstimate combine(const std::vector<HitEstimate> &data) const override { return max(data); } @@ -32,7 +37,7 @@ public: return mixChildrenFields(); } - void sort(Children &children) const override { + void sort(Children &children, bool) const override { std::sort(children.begin(), children.end(), TieredGreaterEstimate()); } @@ -440,7 +445,8 @@ TEST_F("testChildAndNotCollapsing", Fixture) ) ); TEST_DO(f.check_not_equal(*sorted, *unsorted)); - unsorted = Blueprint::optimize(std::move(unsorted)); + unsorted->setDocIdLimit(1000); + unsorted = Blueprint::optimize(std::move(unsorted), true); TEST_DO(f.check_equal(*sorted, *unsorted)); } @@ -479,7 +485,8 @@ TEST_F("testChildAndCollapsing", Fixture) ); TEST_DO(f.check_not_equal(*sorted, *unsorted)); - unsorted = Blueprint::optimize(std::move(unsorted)); + unsorted->setDocIdLimit(1000); + unsorted = Blueprint::optimize(std::move(unsorted), true); TEST_DO(f.check_equal(*sorted, *unsorted)); } @@ -517,7 +524,8 @@ TEST_F("testChildOrCollapsing", Fixture) .add(MyLeafSpec(1).addField(2, 42).create()) ); TEST_DO(f.check_not_equal(*sorted, *unsorted)); - unsorted = Blueprint::optimize(std::move(unsorted)); + unsorted->setDocIdLimit(1000); + unsorted = Blueprint::optimize(std::move(unsorted), true); TEST_DO(f.check_equal(*sorted, *unsorted)); } @@ -560,7 +568,8 @@ TEST_F("testChildSorting", Fixture) ); TEST_DO(f.check_not_equal(*sorted, *unsorted)); - unsorted = Blueprint::optimize(std::move(unsorted)); + unsorted->setDocIdLimit(1000); + unsorted = Blueprint::optimize(std::move(unsorted), true); TEST_DO(f.check_equal(*sorted, *unsorted)); } diff --git a/searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp b/searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp index 234ff5a9d19..ab1c004c721 100644 --- a/searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp +++ b/searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp @@ -15,6 +15,7 @@ #include <vespa/searchlib/query/tree/simplequery.h> #include <vespa/searchlib/common/bitvectoriterator.h> #include <vespa/vespalib/util/overload.h> +#include <vespa/vespalib/util/approx.h> #include <vespa/vespalib/data/simple_buffer.h> #include <vespa/vespalib/data/slime/slime.h> #include <vespa/vespalib/data/slime/inserter.h> @@ -75,7 +76,8 @@ void check_sort_order(IntermediateBlueprint &self, BlueprintVector children, std for (const auto & child: children) { unordered.push_back(child.get()); } - self.sort(children); + // TODO: sort by cost (requires both setDocIdLimit and optimize to be called) + self.sort(children, false); for (size_t i = 0; i < children.size(); ++i) { EXPECT_EQUAL(children[i].get(), unordered[order[i]]); } @@ -129,7 +131,7 @@ TEST("test AndNot Blueprint") { template <typename BP> void optimize(std::unique_ptr<BP> &ref) { - auto optimized = Blueprint::optimize(std::move(ref)); + auto optimized = Blueprint::optimize(std::move(ref), true); ref.reset(dynamic_cast<BP*>(optimized.get())); ASSERT_TRUE(ref); optimized.release(); @@ -141,8 +143,8 @@ TEST("test And propagates updated histestimate") { bp->addChild(ap(MyLeafSpec(20).create<RememberExecuteInfo>()->setSourceId(2))); bp->addChild(ap(MyLeafSpec(200).create<RememberExecuteInfo>()->setSourceId(2))); bp->addChild(ap(MyLeafSpec(2000).create<RememberExecuteInfo>()->setSourceId(2))); - optimize(bp); bp->setDocIdLimit(5000); + optimize(bp); bp->fetchPostings(ExecuteInfo::TRUE); EXPECT_EQUAL(3u, bp->childCnt()); for (uint32_t i = 0; i < bp->childCnt(); i++) { @@ -161,8 +163,8 @@ TEST("test Or propagates updated histestimate") { bp->addChild(ap(MyLeafSpec(2000).create<RememberExecuteInfo>()->setSourceId(2))); bp->addChild(ap(MyLeafSpec(800).create<RememberExecuteInfo>()->setSourceId(2))); bp->addChild(ap(MyLeafSpec(20).create<RememberExecuteInfo>()->setSourceId(2))); - optimize(bp); bp->setDocIdLimit(5000); + optimize(bp); bp->fetchPostings(ExecuteInfo::TRUE); EXPECT_EQUAL(4u, bp->childCnt()); for (uint32_t i = 0; i < bp->childCnt(); i++) { @@ -514,36 +516,45 @@ vespalib::string to_str(const Inspector &value) { } void compare(const Blueprint &bp1, const Blueprint &bp2, bool expect_eq) { - auto ignore_cost = [expect_eq](const auto &path, const auto &a, const auto &b) { - if (!path.empty() && std::holds_alternative<vespalib::stringref>(path.back())) { - vespalib::stringref field = std::get<vespalib::stringref>(path.back()); - if (field == "cost") { - return true; - } - } - if (expect_eq) { - fprintf(stderr, " mismatch at %s: %s vs %s\n", path_to_str(path).c_str(), - to_str(a).c_str(), to_str(b).c_str()); - } - return false; - }; + auto cmp_hook = [expect_eq](const auto &path, const auto &a, const auto &b) { + if (!path.empty() && std::holds_alternative<vespalib::stringref>(path.back())) { + vespalib::stringref field = std::get<vespalib::stringref>(path.back()); + if (field == "cost") { + return true; + } + if (field == "relative_estimate") { + double a_val = a.asDouble(); + double b_val = b.asDouble(); + if (a_val != 0.0 && b_val != 0.0 && vespalib::approx_equal(a_val, b_val)) { + return true; + } + } + } + if (expect_eq) { + fprintf(stderr, " mismatch at %s: %s vs %s\n", path_to_str(path).c_str(), + to_str(a).c_str(), to_str(b).c_str()); + } + return false; + }; Slime a; Slime b; bp1.asSlime(SlimeInserter(a)); bp2.asSlime(SlimeInserter(b)); if (expect_eq) { - EXPECT_TRUE(vespalib::slime::are_equal(a.get(), b.get(), ignore_cost)); + EXPECT_TRUE(vespalib::slime::are_equal(a.get(), b.get(), cmp_hook)); } else { - EXPECT_FALSE(vespalib::slime::are_equal(a.get(), b.get(), ignore_cost)); + EXPECT_FALSE(vespalib::slime::are_equal(a.get(), b.get(), cmp_hook)); } } void -optimize_and_compare(Blueprint::UP top, Blueprint::UP expect) { +optimize_and_compare(Blueprint::UP top, Blueprint::UP expect, bool sort_by_cost = true) { + top->setDocIdLimit(1000); + expect->setDocIdLimit(1000); TEST_DO(compare(*top, *expect, false)); - top = Blueprint::optimize(std::move(top)); + top = Blueprint::optimize(std::move(top), sort_by_cost); TEST_DO(compare(*top, *expect, true)); - expect = Blueprint::optimize(std::move(expect)); + expect = Blueprint::optimize(std::move(expect), sort_by_cost); TEST_DO(compare(*expect, *top, true)); } @@ -670,11 +681,11 @@ TEST("test empty root node optimization and safeness") { //------------------------------------------------------------------------- auto expect_up = std::make_unique<EmptyBlueprint>(); - EXPECT_EQUAL(expect_up->asString(), Blueprint::optimize(std::move(top1))->asString()); - EXPECT_EQUAL(expect_up->asString(), Blueprint::optimize(std::move(top2))->asString()); - EXPECT_EQUAL(expect_up->asString(), Blueprint::optimize(std::move(top3))->asString()); - EXPECT_EQUAL(expect_up->asString(), Blueprint::optimize(std::move(top4))->asString()); - EXPECT_EQUAL(expect_up->asString(), Blueprint::optimize(std::move(top5))->asString()); + EXPECT_EQUAL(expect_up->asString(), Blueprint::optimize(std::move(top1), true)->asString()); + EXPECT_EQUAL(expect_up->asString(), Blueprint::optimize(std::move(top2), true)->asString()); + EXPECT_EQUAL(expect_up->asString(), Blueprint::optimize(std::move(top3), true)->asString()); + EXPECT_EQUAL(expect_up->asString(), Blueprint::optimize(std::move(top4), true)->asString()); + EXPECT_EQUAL(expect_up->asString(), Blueprint::optimize(std::move(top5), true)->asString()); } TEST("and with one empty child is optimized away") { @@ -682,7 +693,7 @@ TEST("and with one empty child is optimized away") { Blueprint::UP top = ap((new SourceBlenderBlueprint(*selector))-> addChild(ap(MyLeafSpec(10).create())). addChild(addLeafs(std::make_unique<AndBlueprint>(), {{0, true}, 10, 20}))); - top = Blueprint::optimize(std::move(top)); + top = Blueprint::optimize(std::move(top), true); Blueprint::UP expect_up(ap((new SourceBlenderBlueprint(*selector))-> addChild(ap(MyLeafSpec(10).create())). addChild(std::make_unique<EmptyBlueprint>()))); @@ -857,8 +868,8 @@ TEST("require that replaced blueprints retain source id") { addChild(ap(MyLeafSpec(30).create()->setSourceId(55))))); Blueprint::UP expect2_up(ap(MyLeafSpec(30).create()->setSourceId(42))); //------------------------------------------------------------------------- - top1_up = Blueprint::optimize(std::move(top1_up)); - top2_up = Blueprint::optimize(std::move(top2_up)); + top1_up = Blueprint::optimize(std::move(top1_up), true); + top2_up = Blueprint::optimize(std::move(top2_up), true); EXPECT_EQUAL(expect1_up->asString(), top1_up->asString()); EXPECT_EQUAL(expect2_up->asString(), top2_up->asString()); EXPECT_EQUAL(13u, top1_up->getSourceId()); @@ -1177,7 +1188,7 @@ TEST("require that children of near are not optimized") { auto expect_up = ap((new NearBlueprint(10))-> addChild(addLeafs(std::make_unique<OrBlueprint>(), {20, {0, true}})). addChild(addLeafs(std::make_unique<OrBlueprint>(), {{0, true}, 30}))); - top_up = Blueprint::optimize(std::move(top_up)); + top_up = Blueprint::optimize(std::move(top_up), true); TEST_DO(compare(*top_up, *expect_up, true)); } @@ -1188,27 +1199,27 @@ TEST("require that children of onear are not optimized") { auto expect_up = ap((new ONearBlueprint(10))-> addChild(addLeafs(std::make_unique<OrBlueprint>(), {20, {0, true}})). addChild(addLeafs(std::make_unique<OrBlueprint>(), {{0, true}, 30}))); - top_up = Blueprint::optimize(std::move(top_up)); + top_up = Blueprint::optimize(std::move(top_up), true); TEST_DO(compare(*top_up, *expect_up, true)); } TEST("require that ANDNOT without children is optimized to empty search") { Blueprint::UP top_up = std::make_unique<AndNotBlueprint>(); auto expect_up = std::make_unique<EmptyBlueprint>(); - top_up = Blueprint::optimize(std::move(top_up)); + top_up = Blueprint::optimize(std::move(top_up), true); EXPECT_EQUAL(expect_up->asString(), top_up->asString()); } TEST("require that highest cost tier sorts last for OR") { Blueprint::UP top = addLeafsWithCostTier(std::make_unique<OrBlueprint>(), {{50, 1}, {30, 3}, {20, 2}, {10, 1}}); Blueprint::UP expect = addLeafsWithCostTier(std::make_unique<OrBlueprint>(), {{50, 1}, {10, 1}, {20, 2}, {30, 3}}); - optimize_and_compare(std::move(top), std::move(expect)); + optimize_and_compare(std::move(top), std::move(expect), false); } TEST("require that highest cost tier sorts last for AND") { Blueprint::UP top = addLeafsWithCostTier(std::make_unique<AndBlueprint>(), {{10, 1}, {20, 3}, {30, 2}, {50, 1}}); Blueprint::UP expect = addLeafsWithCostTier(std::make_unique<AndBlueprint>(), {{10, 1}, {50, 1}, {30, 2}, {20, 3}}); - optimize_and_compare(std::move(top), std::move(expect)); + optimize_and_compare(std::move(top), std::move(expect), false); } template<typename BP> @@ -1325,7 +1336,7 @@ void verify_cost(make &&mk, double expect) { .cost(1.2).leaf(300) .cost(1.3).leaf(500); bp->setDocIdLimit(1000); - bp = Blueprint::optimize(std::move(bp)); + bp = Blueprint::optimize(std::move(bp), true); EXPECT_EQUAL(bp->cost(), expect); } diff --git a/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp b/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp index f910ff5be1b..1180206279d 100644 --- a/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp +++ b/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp @@ -48,7 +48,7 @@ concept ChildCollector = requires(T a, std::unique_ptr<Blueprint> bp) { // inherit Blueprint to capture the default filter factory struct DefaultBlueprint : Blueprint { double calculate_relative_estimate() const override { abort(); } - void optimize(Blueprint* &, OptimizePass) override { abort(); } + void optimize(Blueprint* &, OptimizePass, bool) override { abort(); } const State &getState() const override { abort(); } void fetchPostings(const ExecuteInfo &) override { abort(); } void freeze() override { abort(); } diff --git a/searchlib/src/tests/queryeval/parallel_weak_and/parallel_weak_and_test.cpp b/searchlib/src/tests/queryeval/parallel_weak_and/parallel_weak_and_test.cpp index fa12b453d8c..0e27c77feae 100644 --- a/searchlib/src/tests/queryeval/parallel_weak_and/parallel_weak_and_test.cpp +++ b/searchlib/src/tests/queryeval/parallel_weak_and/parallel_weak_and_test.cpp @@ -1,6 +1,6 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <vespa/searchlib/query/tree/simplequery.h> -#include <vespa/searchlib/queryeval/document_weight_search_iterator.h> +#include <vespa/searchlib/queryeval/docid_with_weight_search_iterator.h> #include <vespa/searchlib/queryeval/fake_requestcontext.h> #include <vespa/searchlib/queryeval/fake_searchable.h> #include <vespa/searchlib/queryeval/simpleresult.h> @@ -661,7 +661,7 @@ SearchIterator::UP create_wand(bool use_dww, assert(childrenMatchData->getNumTermFields() == dict_entries.size()); wand::Terms terms; for (size_t i = 0; i < dict_entries.size(); ++i) { - terms.push_back(wand::Term(new DocumentWeightSearchIterator(*(childrenMatchData->resolveTermField(handles[i])), attr, dict_entries[i]), + terms.push_back(wand::Term(new DocidWithWeightSearchIterator(*(childrenMatchData->resolveTermField(handles[i])), attr, dict_entries[i]), weights[i], dict_entries[i].posting_size, childrenMatchData->resolveTermField(handles[i]))); diff --git a/searchlib/src/tests/queryeval/same_element/same_element_test.cpp b/searchlib/src/tests/queryeval/same_element/same_element_test.cpp index d05e6c8e4f4..7c535e5d3d5 100644 --- a/searchlib/src/tests/queryeval/same_element/same_element_test.cpp +++ b/searchlib/src/tests/queryeval/same_element/same_element_test.cpp @@ -46,7 +46,7 @@ std::unique_ptr<SameElementBlueprint> make_blueprint(const std::vector<FakeResul } Blueprint::UP finalize(Blueprint::UP bp, bool strict) { - Blueprint::UP result = Blueprint::optimize(std::move(bp)); + Blueprint::UP result = Blueprint::optimize(std::move(bp), true); result->fetchPostings(ExecuteInfo::createForTest(strict)); result->freeze(); return result; diff --git a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp index b9adcf3b093..037285fedf0 100644 --- a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp @@ -17,7 +17,7 @@ #include <vespa/searchlib/query/tree/stackdumpcreator.h> #include <vespa/searchlib/queryeval/andsearchstrict.h> #include <vespa/searchlib/queryeval/create_blueprint_visitor_helper.h> -#include <vespa/searchlib/queryeval/document_weight_search_iterator.h> +#include <vespa/searchlib/queryeval/docid_with_weight_search_iterator.h> #include <vespa/searchlib/queryeval/dot_product_blueprint.h> #include <vespa/searchlib/queryeval/dot_product_search.h> #include <vespa/searchlib/queryeval/emptysearch.h> @@ -527,7 +527,7 @@ public: } } if (_attr.has_btree_iterator(_dict_entry.posting_idx)) { - return std::make_unique<queryeval::DocumentWeightSearchIterator>(*tfmda[0], _attr, _dict_entry); + return std::make_unique<queryeval::DocidWithWeightSearchIterator>(*tfmda[0], _attr, _dict_entry); } else { return _attr.make_bitvector_iterator(_dict_entry.posting_idx, get_docid_limit(), *tfmda[0], strict); } diff --git a/searchlib/src/vespa/searchlib/attribute/multi_term_or_filter_search.cpp b/searchlib/src/vespa/searchlib/attribute/multi_term_or_filter_search.cpp index 19668522e17..214851e9681 100644 --- a/searchlib/src/vespa/searchlib/attribute/multi_term_or_filter_search.cpp +++ b/searchlib/src/vespa/searchlib/attribute/multi_term_or_filter_search.cpp @@ -15,14 +15,19 @@ template<typename IteratorPack> class MultiTermOrFilterSearchImpl : public MultiTermOrFilterSearch { IteratorPack _children; + fef::TermFieldMatchData* _tfmd; void seek_all(uint32_t docId); public: - explicit MultiTermOrFilterSearchImpl(IteratorPack&& children); + explicit MultiTermOrFilterSearchImpl(IteratorPack&& children, fef::TermFieldMatchData* tfmd); ~MultiTermOrFilterSearchImpl() override; void doSeek(uint32_t docId) override; - void doUnpack(uint32_t) override { } + void doUnpack(uint32_t docid) override { + if (_tfmd != nullptr) { + _tfmd->resetOnlyDocId(docid); + } + } void initRange(uint32_t begin, uint32_t end) override { SearchIterator::initRange(begin, end); @@ -46,9 +51,10 @@ public: }; template<typename IteratorPack> -MultiTermOrFilterSearchImpl<IteratorPack>::MultiTermOrFilterSearchImpl(IteratorPack&& children) +MultiTermOrFilterSearchImpl<IteratorPack>::MultiTermOrFilterSearchImpl(IteratorPack&& children, fef::TermFieldMatchData* tfmd) : MultiTermOrFilterSearch(), - _children(std::move(children)) + _children(std::move(children)), + _tfmd(tfmd) { } @@ -89,7 +95,7 @@ namespace { template <typename IteratorType, typename IteratorPackType> std::unique_ptr<queryeval::SearchIterator> -create_helper(std::vector<IteratorType>&& children) +create_helper(std::vector<IteratorType>&& children, fef::TermFieldMatchData* tfmd) { if (children.empty()) { return std::make_unique<queryeval::EmptySearch>(); @@ -97,7 +103,7 @@ create_helper(std::vector<IteratorType>&& children) std::sort(children.begin(), children.end(), [](const auto & a, const auto & b) { return a.size() > b.size(); }); using OrFilter = MultiTermOrFilterSearchImpl<IteratorPackType>; - return std::make_unique<OrFilter>(IteratorPackType(std::move(children))); + return std::make_unique<OrFilter>(IteratorPackType(std::move(children)), tfmd); } } @@ -106,13 +112,25 @@ create_helper(std::vector<IteratorType>&& children) std::unique_ptr<queryeval::SearchIterator> MultiTermOrFilterSearch::create(std::vector<DocidIterator>&& children) { - return create_helper<DocidIterator, DocidIteratorPack>(std::move(children)); + return create_helper<DocidIterator, DocidIteratorPack>(std::move(children), nullptr); +} + +std::unique_ptr<queryeval::SearchIterator> +MultiTermOrFilterSearch::create(std::vector<DocidIterator>&& children, fef::TermFieldMatchData& tfmd) +{ + return create_helper<DocidIterator, DocidIteratorPack>(std::move(children), &tfmd); } std::unique_ptr<queryeval::SearchIterator> MultiTermOrFilterSearch::create(std::vector<DocidWithWeightIterator>&& children) { - return create_helper<DocidWithWeightIterator, DocidWithWeightIteratorPack>(std::move(children)); + return create_helper<DocidWithWeightIterator, DocidWithWeightIteratorPack>(std::move(children), nullptr); +} + +std::unique_ptr<queryeval::SearchIterator> +MultiTermOrFilterSearch::create(std::vector<DocidWithWeightIterator>&& children, fef::TermFieldMatchData& tfmd) +{ + return create_helper<DocidWithWeightIterator, DocidWithWeightIteratorPack>(std::move(children), &tfmd); } std::unique_ptr<queryeval::SearchIterator> @@ -123,7 +141,7 @@ MultiTermOrFilterSearch::create(const std::vector<SearchIterator *>& children, return std::make_unique<queryeval::EmptySearch>(); } else { using OrFilter = MultiTermOrFilterSearchImpl<SearchIteratorPack>; - return std::make_unique<OrFilter>(SearchIteratorPack(children, std::move(md))); + return std::make_unique<OrFilter>(SearchIteratorPack(children, std::move(md)), nullptr); } } diff --git a/searchlib/src/vespa/searchlib/attribute/multi_term_or_filter_search.h b/searchlib/src/vespa/searchlib/attribute/multi_term_or_filter_search.h index 42eb33d2eed..1e8227c3007 100644 --- a/searchlib/src/vespa/searchlib/attribute/multi_term_or_filter_search.h +++ b/searchlib/src/vespa/searchlib/attribute/multi_term_or_filter_search.h @@ -18,7 +18,9 @@ protected: MultiTermOrFilterSearch() = default; public: static std::unique_ptr<SearchIterator> create(std::vector<DocidIterator>&& children); + static std::unique_ptr<SearchIterator> create(std::vector<DocidIterator>&& children, fef::TermFieldMatchData& tfmd); static std::unique_ptr<SearchIterator> create(std::vector<DocidWithWeightIterator>&& children); + static std::unique_ptr<SearchIterator> create(std::vector<DocidWithWeightIterator>&& children, fef::TermFieldMatchData& tfmd); static std::unique_ptr<SearchIterator> create(const std::vector<SearchIterator *>& children, std::unique_ptr<fef::MatchData> md); }; diff --git a/searchlib/src/vespa/searchlib/common/bitvector.cpp b/searchlib/src/vespa/searchlib/common/bitvector.cpp index b79703a8e5c..a75066a67a9 100644 --- a/searchlib/src/vespa/searchlib/common/bitvector.cpp +++ b/searchlib/src/vespa/searchlib/common/bitvector.cpp @@ -39,7 +39,7 @@ BitVector::allocatePaddedAndAligned(Index start, Index end, Index capacity, cons { assert(capacity >= end); uint32_t words = numActiveWords(start, capacity); - words += (-words & 15); // Pad to 64 byte alignment + words += (-words & 15); // Pad to 128 byte alignment const size_t sz(words * sizeof(Word)); Alloc alloc = (init_alloc != nullptr) ? init_alloc->create(sz) : Alloc::alloc(sz, MMAP_LIMIT); assert(alloc.size()/sizeof(Word) >= words); diff --git a/searchlib/src/vespa/searchlib/fef/indexproperties.cpp b/searchlib/src/vespa/searchlib/fef/indexproperties.cpp index 9c986d0bc63..4637ad5a4e8 100644 --- a/searchlib/src/vespa/searchlib/fef/indexproperties.cpp +++ b/searchlib/src/vespa/searchlib/fef/indexproperties.cpp @@ -455,6 +455,12 @@ FuzzyAlgorithm::lookup(const Properties& props, vespalib::FuzzyMatchingAlgorithm return vespalib::fuzzy_matching_algorithm_from_string(value, default_value); } +const vespalib::string SortBlueprintsByCost::NAME("vespa.matching.sort_blueprints_by_cost"); +const bool SortBlueprintsByCost::DEFAULT_VALUE(false); +bool SortBlueprintsByCost::check(const Properties &props, bool fallback) { + return lookupBool(props, NAME, fallback); +} + const vespalib::string AlwaysMarkPhraseExpensive::NAME("vespa.matching.always_mark_phrase_expensive"); const bool AlwaysMarkPhraseExpensive::DEFAULT_VALUE(false); bool AlwaysMarkPhraseExpensive::check(const Properties &props, bool fallback) { diff --git a/searchlib/src/vespa/searchlib/fef/indexproperties.h b/searchlib/src/vespa/searchlib/fef/indexproperties.h index 1921f52276f..db8de8209a9 100644 --- a/searchlib/src/vespa/searchlib/fef/indexproperties.h +++ b/searchlib/src/vespa/searchlib/fef/indexproperties.h @@ -336,6 +336,15 @@ namespace matching { static vespalib::FuzzyMatchingAlgorithm lookup(const Properties& props); static vespalib::FuzzyMatchingAlgorithm lookup(const Properties& props, vespalib::FuzzyMatchingAlgorithm default_value); }; + /** + * Sort blueprints based on relative cost estimate rather than est_hits + **/ + struct SortBlueprintsByCost { + static const vespalib::string NAME; + static const bool DEFAULT_VALUE; + static bool check(const Properties &props) { return check(props, DEFAULT_VALUE); } + static bool check(const Properties &props, bool fallback); + }; /** * When enabled, the unpacking part of the phrase iterator will be tagged as expensive diff --git a/searchlib/src/vespa/searchlib/fef/ranksetup.cpp b/searchlib/src/vespa/searchlib/fef/ranksetup.cpp index d6b0b900516..aadc5300ede 100644 --- a/searchlib/src/vespa/searchlib/fef/ranksetup.cpp +++ b/searchlib/src/vespa/searchlib/fef/ranksetup.cpp @@ -56,6 +56,7 @@ RankSetup::RankSetup(const BlueprintFactory &factory, const IIndexEnvironment &i _dumpFeatures(), _warnings(), _feature_rename_map(), + _sort_blueprints_by_cost(false), _ignoreDefaultRankFeatures(false), _compiled(false), _compileError(false), @@ -134,6 +135,7 @@ RankSetup::configure() _mutateOnSummary._attribute = mutate::on_summary::Attribute::lookup(_indexEnv.getProperties()); _mutateOnSummary._operation = mutate::on_summary::Operation::lookup(_indexEnv.getProperties()); _mutateAllowQueryOverride = mutate::AllowQueryOverride::check(_indexEnv.getProperties()); + _sort_blueprints_by_cost = matching::SortBlueprintsByCost::check(_indexEnv.getProperties()); _always_mark_phrase_expensive = matching::AlwaysMarkPhraseExpensive::check(_indexEnv.getProperties()); } diff --git a/searchlib/src/vespa/searchlib/fef/ranksetup.h b/searchlib/src/vespa/searchlib/fef/ranksetup.h index d744b38cc6e..d8b977a0331 100644 --- a/searchlib/src/vespa/searchlib/fef/ranksetup.h +++ b/searchlib/src/vespa/searchlib/fef/ranksetup.h @@ -65,6 +65,7 @@ private: std::vector<vespalib::string> _dumpFeatures; Warnings _warnings; StringStringMap _feature_rename_map; + bool _sort_blueprints_by_cost; bool _ignoreDefaultRankFeatures; bool _compiled; bool _compileError; @@ -459,6 +460,7 @@ public: const MutateOperation & getMutateOnSummary() const { return _mutateOnSummary; } bool allowMutateQueryOverride() const { return _mutateAllowQueryOverride; } + bool sort_blueprints_by_cost() const noexcept { return _sort_blueprints_by_cost; } }; } diff --git a/searchlib/src/vespa/searchlib/queryeval/CMakeLists.txt b/searchlib/src/vespa/searchlib/queryeval/CMakeLists.txt index 5e6d31d3761..51fe2d12637 100644 --- a/searchlib/src/vespa/searchlib/queryeval/CMakeLists.txt +++ b/searchlib/src/vespa/searchlib/queryeval/CMakeLists.txt @@ -7,7 +7,7 @@ vespa_add_library(searchlib_queryeval OBJECT booleanmatchiteratorwrapper.cpp children_iterators.cpp create_blueprint_visitor_helper.cpp - document_weight_search_iterator.cpp + docid_with_weight_search_iterator.cpp dot_product_blueprint.cpp dot_product_search.cpp elementiterator.cpp diff --git a/searchlib/src/vespa/searchlib/queryeval/blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/blueprint.cpp index d2b4661d9c3..6ca072d6dc7 100644 --- a/searchlib/src/vespa/searchlib/queryeval/blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/blueprint.cpp @@ -130,15 +130,15 @@ Blueprint::Blueprint() noexcept Blueprint::~Blueprint() = default; Blueprint::UP -Blueprint::optimize(Blueprint::UP bp) { +Blueprint::optimize(Blueprint::UP bp, bool sort_by_cost) { Blueprint *root = bp.release(); - root->optimize(root, OptimizePass::FIRST); - root->optimize(root, OptimizePass::LAST); + root->optimize(root, OptimizePass::FIRST, sort_by_cost); + root->optimize(root, OptimizePass::LAST, sort_by_cost); return Blueprint::UP(root); } void -Blueprint::optimize_self(OptimizePass) +Blueprint::optimize_self(OptimizePass, bool) { } @@ -548,19 +548,19 @@ IntermediateBlueprint::should_do_termwise_eval(const UnpackInfo &unpack, double } void -IntermediateBlueprint::optimize(Blueprint* &self, OptimizePass pass) +IntermediateBlueprint::optimize(Blueprint* &self, OptimizePass pass, bool sort_by_cost) { assert(self == this); if (should_optimize_children()) { for (auto &child : _children) { auto *child_ptr = child.release(); - child_ptr->optimize(child_ptr, pass); + child_ptr->optimize(child_ptr, pass, sort_by_cost); child.reset(child_ptr); } } - optimize_self(pass); + optimize_self(pass, sort_by_cost); if (pass == OptimizePass::LAST) { - sort(_children); + sort(_children, sort_by_cost); set_cost(calculate_cost()); } maybe_eliminate_self(self, get_replacement()); @@ -758,10 +758,10 @@ LeafBlueprint::getRange(vespalib::string &, vespalib::string &) const { } void -LeafBlueprint::optimize(Blueprint* &self, OptimizePass pass) +LeafBlueprint::optimize(Blueprint* &self, OptimizePass pass, bool sort_by_cost) { assert(self == this); - optimize_self(pass); + optimize_self(pass, sort_by_cost); maybe_eliminate_self(self, get_replacement()); } diff --git a/searchlib/src/vespa/searchlib/queryeval/blueprint.h b/searchlib/src/vespa/searchlib/queryeval/blueprint.h index 0dbd7b618a7..a78dd092f5a 100644 --- a/searchlib/src/vespa/searchlib/queryeval/blueprint.h +++ b/searchlib/src/vespa/searchlib/queryeval/blueprint.h @@ -172,6 +172,20 @@ public: // lower limit for docid_limit: max child estimate static HitEstimate sat_sum(const std::vector<HitEstimate> &data, uint32_t docid_limit); + // sort children to minimize total cost of OR flow + struct MinimalOrCost { + bool operator () (const auto &a, const auto &b) const noexcept { + return a->estimate() / a->cost() > b->estimate() / b->cost(); + } + }; + + // sort children to minimize total cost of AND flow + struct MinimalAndCost { + bool operator () (const auto &a, const auto &b) const noexcept { + return (1.0 - a->estimate()) / a->cost() > (1.0 - b->estimate()) / b->cost(); + } + }; + // utility to get the greater estimate to sort first, higher tiers last struct TieredGreaterEstimate { bool operator () (const auto &a, const auto &b) const noexcept { @@ -246,9 +260,9 @@ public: virtual void setDocIdLimit(uint32_t limit) noexcept { _docid_limit = limit; } uint32_t get_docid_limit() const noexcept { return _docid_limit; } - static Blueprint::UP optimize(Blueprint::UP bp); - virtual void optimize(Blueprint* &self, OptimizePass pass) = 0; - virtual void optimize_self(OptimizePass pass); + static Blueprint::UP optimize(Blueprint::UP bp, bool sort_by_cost); + virtual void optimize(Blueprint* &self, OptimizePass pass, bool sort_by_cost) = 0; + virtual void optimize_self(OptimizePass pass, bool sort_by_cost); virtual Blueprint::UP get_replacement(); virtual bool should_optimize_children() const { return true; } @@ -376,7 +390,7 @@ public: void setDocIdLimit(uint32_t limit) noexcept final; - void optimize(Blueprint* &self, OptimizePass pass) final; + void optimize(Blueprint* &self, OptimizePass pass, bool sort_by_cost) final; void set_global_filter(const GlobalFilter &global_filter, double estimated_hit_ratio) override; IndexList find(const IPredicate & check) const; @@ -393,7 +407,7 @@ public: virtual double calculate_cost() const = 0; virtual HitEstimate combine(const std::vector<HitEstimate> &data) const = 0; virtual FieldSpecBaseList exposeFields() const = 0; - virtual void sort(Children &children) const = 0; + virtual void sort(Children &children, bool sort_by_cost) const = 0; virtual bool inheritStrict(size_t i) const = 0; virtual SearchIteratorUP createIntermediateSearch(MultiSearch::Children subSearches, @@ -413,7 +427,7 @@ class LeafBlueprint : public Blueprint private: State _state; protected: - void optimize(Blueprint* &self, OptimizePass pass) final; + void optimize(Blueprint* &self, OptimizePass pass, bool sort_by_cost) final; void setEstimate(HitEstimate est) { _state.estimate(est); _state.relative_estimate(calculate_relative_estimate()); diff --git a/searchlib/src/vespa/searchlib/queryeval/document_weight_search_iterator.cpp b/searchlib/src/vespa/searchlib/queryeval/docid_with_weight_search_iterator.cpp index 6b0bd3ec7fc..85bd751df27 100644 --- a/searchlib/src/vespa/searchlib/queryeval/document_weight_search_iterator.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/docid_with_weight_search_iterator.cpp @@ -1,3 +1,3 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include "document_weight_search_iterator.h" +#include "docid_with_weight_search_iterator.h" diff --git a/searchlib/src/vespa/searchlib/queryeval/document_weight_search_iterator.h b/searchlib/src/vespa/searchlib/queryeval/docid_with_weight_search_iterator.h index 448f1c8f2b4..8201c6a78b8 100644 --- a/searchlib/src/vespa/searchlib/queryeval/document_weight_search_iterator.h +++ b/searchlib/src/vespa/searchlib/queryeval/docid_with_weight_search_iterator.h @@ -8,7 +8,12 @@ namespace search::queryeval { -class DocumentWeightSearchIterator : public SearchIterator +/** + * SearchIterator implementation over a low-level posting list with {docid, weight} tuples. + * + * This is used by the parallel weak AND search iterator. + */ +class DocidWithWeightSearchIterator : public SearchIterator { private: fef::TermFieldMatchData &_tfmd; @@ -17,9 +22,9 @@ private: queryeval::MinMaxPostingInfo _postingInfo; public: - DocumentWeightSearchIterator(fef::TermFieldMatchData &tfmd, - const IDocidWithWeightPostingStore &attr, - IDirectPostingStore::LookupResult dict_entry) + DocidWithWeightSearchIterator(fef::TermFieldMatchData &tfmd, + const IDocidWithWeightPostingStore &attr, + IDirectPostingStore::LookupResult dict_entry) : _tfmd(tfmd), _matchPosition(_tfmd.populate_fixed()), _iterator(attr.create(dict_entry.posting_idx)), diff --git a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp index d50b9846f17..bebc1f433f7 100644 --- a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp @@ -33,7 +33,7 @@ size_t lookup_create_source(std::vector<std::unique_ptr<CombineType> > &sources, } template <typename CombineType> -void optimize_source_blenders(IntermediateBlueprint &self, size_t begin_idx) { +void optimize_source_blenders(IntermediateBlueprint &self, size_t begin_idx, bool sort_by_cost) { std::vector<size_t> source_blenders; const SourceBlenderBlueprint * reference = nullptr; for (size_t i = begin_idx; i < self.childCnt(); ++i) { @@ -63,7 +63,7 @@ void optimize_source_blenders(IntermediateBlueprint &self, size_t begin_idx) { top->addChild(std::move(sources.back())); sources.pop_back(); } - blender_up = Blueprint::optimize(std::move(blender_up)); + blender_up = Blueprint::optimize(std::move(blender_up), sort_by_cost); self.addChild(std::move(blender_up)); } } @@ -114,7 +114,7 @@ AndNotBlueprint::exposeFields() const } void -AndNotBlueprint::optimize_self(OptimizePass pass) +AndNotBlueprint::optimize_self(OptimizePass pass, bool sort_by_cost) { if (childCnt() == 0) { return; @@ -152,7 +152,7 @@ AndNotBlueprint::optimize_self(OptimizePass pass) } } if (pass == OptimizePass::LAST) { - optimize_source_blenders<OrBlueprint>(*this, 1); + optimize_source_blenders<OrBlueprint>(*this, 1, sort_by_cost); } } @@ -166,10 +166,14 @@ AndNotBlueprint::get_replacement() } void -AndNotBlueprint::sort(Children &children) const +AndNotBlueprint::sort(Children &children, bool sort_by_cost) const { if (children.size() > 2) { - std::sort(children.begin() + 1, children.end(), TieredGreaterEstimate()); + if (sort_by_cost) { + std::sort(children.begin() + 1, children.end(), MinimalOrCost()); + } else { + std::sort(children.begin() + 1, children.end(), TieredGreaterEstimate()); + } } } @@ -231,7 +235,7 @@ AndBlueprint::exposeFields() const } void -AndBlueprint::optimize_self(OptimizePass pass) +AndBlueprint::optimize_self(OptimizePass pass, bool sort_by_cost) { if (pass == OptimizePass::FIRST) { for (size_t i = 0; i < childCnt(); ++i) { @@ -244,7 +248,7 @@ AndBlueprint::optimize_self(OptimizePass pass) } } if (pass == OptimizePass::LAST) { - optimize_source_blenders<AndBlueprint>(*this, 0); + optimize_source_blenders<AndBlueprint>(*this, 0, sort_by_cost); } } @@ -258,9 +262,13 @@ AndBlueprint::get_replacement() } void -AndBlueprint::sort(Children &children) const +AndBlueprint::sort(Children &children, bool sort_by_cost) const { - std::sort(children.begin(), children.end(), TieredLessEstimate()); + if (sort_by_cost) { + std::sort(children.begin(), children.end(), MinimalAndCost()); + } else { + std::sort(children.begin(), children.end(), TieredLessEstimate()); + } } bool @@ -336,7 +344,7 @@ OrBlueprint::exposeFields() const } void -OrBlueprint::optimize_self(OptimizePass pass) +OrBlueprint::optimize_self(OptimizePass pass, bool sort_by_cost) { if (pass == OptimizePass::FIRST) { for (size_t i = 0; (childCnt() > 1) && (i < childCnt()); ++i) { @@ -351,7 +359,7 @@ OrBlueprint::optimize_self(OptimizePass pass) } } if (pass == OptimizePass::LAST) { - optimize_source_blenders<OrBlueprint>(*this, 0); + optimize_source_blenders<OrBlueprint>(*this, 0, sort_by_cost); } } @@ -365,9 +373,13 @@ OrBlueprint::get_replacement() } void -OrBlueprint::sort(Children &children) const +OrBlueprint::sort(Children &children, bool sort_by_cost) const { - std::sort(children.begin(), children.end(), TieredGreaterEstimate()); + if (sort_by_cost) { + std::sort(children.begin(), children.end(), MinimalOrCost()); + } else { + std::sort(children.begin(), children.end(), TieredGreaterEstimate()); + } } bool @@ -444,7 +456,7 @@ WeakAndBlueprint::exposeFields() const } void -WeakAndBlueprint::sort(Children &) const +WeakAndBlueprint::sort(Children &, bool) const { // order needs to stay the same as _weights } @@ -508,9 +520,13 @@ NearBlueprint::exposeFields() const } void -NearBlueprint::sort(Children &children) const +NearBlueprint::sort(Children &children, bool sort_by_cost) const { - std::sort(children.begin(), children.end(), TieredLessEstimate()); + if (sort_by_cost) { + std::sort(children.begin(), children.end(), MinimalAndCost()); + } else { + std::sort(children.begin(), children.end(), TieredLessEstimate()); + } } bool @@ -571,10 +587,9 @@ ONearBlueprint::exposeFields() const } void -ONearBlueprint::sort(Children &children) const +ONearBlueprint::sort(Children &, bool) const { // ordered near cannot sort children here - (void)children; } bool @@ -640,7 +655,7 @@ RankBlueprint::exposeFields() const } void -RankBlueprint::optimize_self(OptimizePass pass) +RankBlueprint::optimize_self(OptimizePass pass, bool sort_by_cost) { if (pass == OptimizePass::FIRST) { for (size_t i = 1; i < childCnt(); ++i) { @@ -650,7 +665,7 @@ RankBlueprint::optimize_self(OptimizePass pass) } } if (pass == OptimizePass::LAST) { - optimize_source_blenders<OrBlueprint>(*this, 1); + optimize_source_blenders<OrBlueprint>(*this, 1, sort_by_cost); } } @@ -664,9 +679,8 @@ RankBlueprint::get_replacement() } void -RankBlueprint::sort(Children &children) const +RankBlueprint::sort(Children &, bool) const { - (void)children; } bool @@ -743,7 +757,7 @@ SourceBlenderBlueprint::exposeFields() const } void -SourceBlenderBlueprint::sort(Children &) const +SourceBlenderBlueprint::sort(Children &, bool) const { } diff --git a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h index cc3da6ce983..620280e979b 100644 --- a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h +++ b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h @@ -19,10 +19,10 @@ public: double calculate_relative_estimate() const final; HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; - void optimize_self(OptimizePass pass) override; + void optimize_self(OptimizePass pass, bool sort_by_cost) override; AndNotBlueprint * asAndNot() noexcept final { return this; } Blueprint::UP get_replacement() override; - void sort(Children &children) const override; + void sort(Children &children, bool sort_by_cost) const override; bool inheritStrict(size_t i) const override; SearchIterator::UP createIntermediateSearch(MultiSearch::Children subSearches, @@ -47,10 +47,10 @@ public: double calculate_relative_estimate() const final; HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; - void optimize_self(OptimizePass pass) override; + void optimize_self(OptimizePass pass, bool sort_by_cost) override; AndBlueprint * asAnd() noexcept final { return this; } Blueprint::UP get_replacement() override; - void sort(Children &children) const override; + void sort(Children &children, bool sort_by_cost) const override; bool inheritStrict(size_t i) const override; SearchIterator::UP createIntermediateSearch(MultiSearch::Children subSearches, @@ -73,10 +73,10 @@ public: double calculate_relative_estimate() const final; HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; - void optimize_self(OptimizePass pass) override; + void optimize_self(OptimizePass pass, bool sort_by_cost) override; OrBlueprint * asOr() noexcept final { return this; } Blueprint::UP get_replacement() override; - void sort(Children &children) const override; + void sort(Children &children, bool sort_by_cost) const override; bool inheritStrict(size_t i) const override; SearchIterator::UP createIntermediateSearch(MultiSearch::Children subSearches, @@ -101,7 +101,7 @@ public: double calculate_relative_estimate() const final; HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; - void sort(Children &children) const override; + void sort(Children &children, bool sort_by_cost) const override; bool inheritStrict(size_t i) const override; bool always_needs_unpack() const override; WeakAndBlueprint * asWeakAnd() noexcept final { return this; } @@ -133,7 +133,7 @@ public: HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; bool should_optimize_children() const override { return false; } - void sort(Children &children) const override; + void sort(Children &children, bool sort_by_cost) const override; bool inheritStrict(size_t i) const override; SearchIteratorUP createSearch(fef::MatchData &md, bool strict) const override; SearchIterator::UP @@ -157,7 +157,7 @@ public: HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; bool should_optimize_children() const override { return false; } - void sort(Children &children) const override; + void sort(Children &children, bool sort_by_cost) const override; bool inheritStrict(size_t i) const override; SearchIteratorUP createSearch(fef::MatchData &md, bool strict) const override; SearchIterator::UP @@ -177,9 +177,9 @@ public: double calculate_relative_estimate() const final; HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; - void optimize_self(OptimizePass pass) override; + void optimize_self(OptimizePass pass, bool sort_by_cost) override; Blueprint::UP get_replacement() override; - void sort(Children &children) const override; + void sort(Children &children, bool sort_by_cost) const override; bool inheritStrict(size_t i) const override; bool isRank() const noexcept final { return true; } SearchIterator::UP @@ -206,7 +206,7 @@ public: double calculate_relative_estimate() const final; HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; - void sort(Children &children) const override; + void sort(Children &children, bool sort_by_cost) const override; bool inheritStrict(size_t i) const override; SearchIterator::UP createIntermediateSearch(MultiSearch::Children subSearches, diff --git a/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp b/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp index 66f505581c7..e90156868fb 100644 --- a/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp @@ -4,7 +4,6 @@ #include "andsearch.h" #include "andnotsearch.h" #include "sourceblendersearch.h" -#include <vespa/searchlib/common/bitvectoriterator.h> #include <vespa/vespalib/hwaccelrated/iaccelrated.h> namespace search::queryeval { @@ -18,7 +17,7 @@ namespace { struct And { using Word = BitWord::Word; void operator () (const IAccelrated & accel, size_t offset, const std::vector<Meta> & src, void *dest) noexcept { - accel.and64(offset, src, dest); + accel.and128(offset, src, dest); } static constexpr bool isAnd() noexcept { return true; } }; @@ -26,7 +25,7 @@ struct And { struct Or { using Word = BitWord::Word; void operator () (const IAccelrated & accel, size_t offset, const std::vector<Meta> & src, void *dest) noexcept { - accel.or64(offset, src, dest); + accel.or128(offset, src, dest); } static constexpr bool isAnd() noexcept { return false; } }; @@ -56,8 +55,8 @@ MultiBitVector<Update>::MultiBitVector(size_t reserved) _accel(IAccelrated::getAccelerator()), _lastWords() { - static_assert(sizeof(_lastWords) == 64, "Lastwords should have 64 byte size"); - static_assert(NumWordsInBatch == 8, "Batch size should be 8 words."); + static_assert(sizeof(_lastWords) == 128, "Lastwords should have 128 byte size"); + static_assert(NumWordsInBatch == 16, "Batch size should be 16 words."); memset(_lastWords, 0, sizeof(_lastWords)); } diff --git a/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.h b/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.h index 0d9e2c4f25f..0ecf9d85b92 100644 --- a/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.h +++ b/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.h @@ -50,7 +50,7 @@ private: Update _update; const IAccelrated & _accel; - alignas(64) Word _lastWords[8]; + alignas(64) Word _lastWords[16]; static constexpr size_t NumWordsInBatch = sizeof(_lastWords) / sizeof(Word); }; diff --git a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp index 96181377282..500e9fe4dbb 100644 --- a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp @@ -45,7 +45,7 @@ SameElementBlueprint::addTerm(Blueprint::UP term) } void -SameElementBlueprint::optimize_self(OptimizePass pass) +SameElementBlueprint::optimize_self(OptimizePass pass, bool) { if (pass == OptimizePass::LAST) { std::sort(_terms.begin(), _terms.end(), diff --git a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.h b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.h index 6a988e67149..06c20339e81 100644 --- a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.h +++ b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.h @@ -34,7 +34,7 @@ public: // used by create visitor void addTerm(Blueprint::UP term); - void optimize_self(OptimizePass pass) override; + void optimize_self(OptimizePass pass, bool sort_by_cost) override; void fetchPostings(const ExecuteInfo &execInfo) override; std::unique_ptr<SameElementSearch> create_same_element_search(search::fef::TermFieldMatchData& tfmd, bool strict) const; diff --git a/searchlib/src/vespa/searchlib/queryeval/wand/parallel_weak_and_search.cpp b/searchlib/src/vespa/searchlib/queryeval/wand/parallel_weak_and_search.cpp index 828ca4be08d..f3028f5159a 100644 --- a/searchlib/src/vespa/searchlib/queryeval/wand/parallel_weak_and_search.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/wand/parallel_weak_and_search.cpp @@ -1,7 +1,7 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "parallel_weak_and_search.h" -#include <vespa/searchlib/queryeval/document_weight_search_iterator.h> +#include <vespa/searchlib/queryeval/docid_with_weight_search_iterator.h> #include <vespa/searchlib/queryeval/monitoring_dump_iterator.h> #include <vespa/searchlib/fef/matchdatalayout.h> #include <vespa/vespalib/objects/visit.h> @@ -243,7 +243,7 @@ ParallelWeakAndSearch::create(search::fef::TermFieldMatchData &tfmd, assert(childrenMatchData->getNumTermFields() == dict_entries.size()); wand::Terms terms; for (size_t i = 0; i < dict_entries.size(); ++i) { - terms.push_back(wand::Term(new DocumentWeightSearchIterator(*(childrenMatchData->resolveTermField(handles[i])), attr, dict_entries[i]), + terms.push_back(wand::Term(new DocidWithWeightSearchIterator(*(childrenMatchData->resolveTermField(handles[i])), attr, dict_entries[i]), weights[i], dict_entries[i].posting_size, childrenMatchData->resolveTermField(handles[i]))); diff --git a/vespajlib/abi-spec.json b/vespajlib/abi-spec.json index a97950415e4..10b0478b5b0 100644 --- a/vespajlib/abi-spec.json +++ b/vespajlib/abi-spec.json @@ -3420,7 +3420,8 @@ "public static org.w3c.dom.Element getChild(org.w3c.dom.Element, java.lang.String)", "public static java.util.Optional getChildValue(org.w3c.dom.Element, java.lang.String)", "public static java.lang.String getNodePath(org.w3c.dom.Node, java.lang.String)", - "public static boolean isName(java.lang.CharSequence)" + "public static boolean isName(java.lang.CharSequence)", + "public static javax.xml.transform.TransformerFactory createTransformerFactory()" ], "fields" : [ ] }, diff --git a/vespajlib/src/main/java/com/yahoo/text/XML.java b/vespajlib/src/main/java/com/yahoo/text/XML.java index a6e36a0c3e1..72a2dba54e1 100644 --- a/vespajlib/src/main/java/com/yahoo/text/XML.java +++ b/vespajlib/src/main/java/com/yahoo/text/XML.java @@ -9,9 +9,11 @@ import org.xml.sax.InputSource; import org.xml.sax.SAXException; import org.xml.sax.SAXParseException; +import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import javax.xml.transform.TransformerFactory; import java.io.File; import java.io.IOException; import java.io.Reader; @@ -446,6 +448,19 @@ public class XML { } /** + * Creates a new XML TransformerFactory. + * + * @return a TransformerFactory + */ + public static TransformerFactory createTransformerFactory() { + TransformerFactory transformerFactory = TransformerFactory.newInstance(); + transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); + return transformerFactory; + } + + + /** * The point of this weird class and the jumble of abstract methods is * linking the scan for characters that must be quoted into the quoting * table, and making it actual work to make them go out of sync again. diff --git a/vespalib/src/vespa/vespalib/hwaccelrated/avx2.cpp b/vespalib/src/vespa/vespalib/hwaccelrated/avx2.cpp index bbba4109fc2..66441b3c08b 100644 --- a/vespalib/src/vespa/vespalib/hwaccelrated/avx2.cpp +++ b/vespalib/src/vespa/vespalib/hwaccelrated/avx2.cpp @@ -26,13 +26,13 @@ Avx2Accelrator::squaredEuclideanDistance(const double * a, const double * b, siz } void -Avx2Accelrator::and64(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept { - helper::andChunks<32u, 2u>(offset, src, dest); +Avx2Accelrator::and128(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept { + helper::andChunks<32u, 4u>(offset, src, dest); } void -Avx2Accelrator::or64(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept { - helper::orChunks<32u, 2u>(offset, src, dest); +Avx2Accelrator::or128(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept { + helper::orChunks<32u, 4u>(offset, src, dest); } } diff --git a/vespalib/src/vespa/vespalib/hwaccelrated/avx2.h b/vespalib/src/vespa/vespalib/hwaccelrated/avx2.h index 934d815d67b..af46035666c 100644 --- a/vespalib/src/vespa/vespalib/hwaccelrated/avx2.h +++ b/vespalib/src/vespa/vespalib/hwaccelrated/avx2.h @@ -16,8 +16,8 @@ public: double squaredEuclideanDistance(const int8_t * a, const int8_t * b, size_t sz) const noexcept override; double squaredEuclideanDistance(const float * a, const float * b, size_t sz) const noexcept override; double squaredEuclideanDistance(const double * a, const double * b, size_t sz) const noexcept override; - void and64(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept override; - void or64(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept override; + void and128(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept override; + void or128(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept override; }; } diff --git a/vespalib/src/vespa/vespalib/hwaccelrated/avx512.cpp b/vespalib/src/vespa/vespalib/hwaccelrated/avx512.cpp index 035f33cb25e..5f408c05fef 100644 --- a/vespalib/src/vespa/vespalib/hwaccelrated/avx512.cpp +++ b/vespalib/src/vespa/vespalib/hwaccelrated/avx512.cpp @@ -36,13 +36,13 @@ Avx512Accelrator::squaredEuclideanDistance(const double * a, const double * b, s } void -Avx512Accelrator::and64(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept { - helper::andChunks<64, 1>(offset, src, dest); +Avx512Accelrator::and128(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept { + helper::andChunks<64, 2>(offset, src, dest); } void -Avx512Accelrator::or64(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept { - helper::orChunks<64, 1>(offset, src, dest); +Avx512Accelrator::or128(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept { + helper::orChunks<64, 2>(offset, src, dest); } } diff --git a/vespalib/src/vespa/vespalib/hwaccelrated/avx512.h b/vespalib/src/vespa/vespalib/hwaccelrated/avx512.h index 38eab0a2549..a86a2787d5a 100644 --- a/vespalib/src/vespa/vespalib/hwaccelrated/avx512.h +++ b/vespalib/src/vespa/vespalib/hwaccelrated/avx512.h @@ -18,8 +18,8 @@ public: double squaredEuclideanDistance(const int8_t * a, const int8_t * b, size_t sz) const noexcept override; double squaredEuclideanDistance(const float * a, const float * b, size_t sz) const noexcept override; double squaredEuclideanDistance(const double * a, const double * b, size_t sz) const noexcept override; - void and64(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept override; - void or64(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept override; + void and128(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept override; + void or128(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept override; }; } diff --git a/vespalib/src/vespa/vespalib/hwaccelrated/generic.cpp b/vespalib/src/vespa/vespalib/hwaccelrated/generic.cpp index a8e5535cc21..f0112aaddf7 100644 --- a/vespalib/src/vespa/vespalib/hwaccelrated/generic.cpp +++ b/vespalib/src/vespa/vespalib/hwaccelrated/generic.cpp @@ -173,13 +173,13 @@ GenericAccelrator::squaredEuclideanDistance(const double * a, const double * b, } void -GenericAccelrator::and64(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept { - helper::andChunks<16, 4>(offset, src, dest); +GenericAccelrator::and128(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept { + helper::andChunks<16, 8>(offset, src, dest); } void -GenericAccelrator::or64(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept { - helper::orChunks<16,4>(offset, src, dest); +GenericAccelrator::or128(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept { + helper::orChunks<16, 8>(offset, src, dest); } } diff --git a/vespalib/src/vespa/vespalib/hwaccelrated/generic.h b/vespalib/src/vespa/vespalib/hwaccelrated/generic.h index 16c8bab71da..ba986656635 100644 --- a/vespalib/src/vespa/vespalib/hwaccelrated/generic.h +++ b/vespalib/src/vespa/vespalib/hwaccelrated/generic.h @@ -26,8 +26,8 @@ public: double squaredEuclideanDistance(const int8_t * a, const int8_t * b, size_t sz) const noexcept override; double squaredEuclideanDistance(const float * a, const float * b, size_t sz) const noexcept override; double squaredEuclideanDistance(const double * a, const double * b, size_t sz) const noexcept override; - void and64(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept override; - void or64(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept override; + void and128(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept override; + void or128(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept override; }; } diff --git a/vespalib/src/vespa/vespalib/hwaccelrated/iaccelrated.cpp b/vespalib/src/vespa/vespalib/hwaccelrated/iaccelrated.cpp index d707553b504..a02e9545765 100644 --- a/vespalib/src/vespa/vespalib/hwaccelrated/iaccelrated.cpp +++ b/vespalib/src/vespa/vespalib/hwaccelrated/iaccelrated.cpp @@ -153,11 +153,11 @@ verifyOr64(const IAccelrated & accel, const std::vector<std::vector<uint64_t>> & simpleOrWith(expected, optionallyInvert(vRefs[j].second, vectors[j])); } - uint64_t dest[8] __attribute((aligned(64))); - accel.or64(offset*sizeof(uint64_t), vRefs, dest); + uint64_t dest[16] __attribute((aligned(64))); + accel.or128(offset * sizeof(uint64_t), vRefs, dest); int diff = memcmp(&expected[offset], dest, sizeof(dest)); if (diff != 0) { - LOG_ABORT("Accelerator fails to compute correct 64 bytes OR"); + LOG_ABORT("Accelerator fails to compute correct 128 bytes OR"); } } @@ -174,11 +174,11 @@ verifyAnd64(const IAccelrated & accel, const std::vector<std::vector<uint64_t>> simpleAndWith(expected, optionallyInvert(vRefs[j].second, vectors[j])); } - uint64_t dest[8] __attribute((aligned(64))); - accel.and64(offset*sizeof(uint64_t), vRefs, dest); + uint64_t dest[16] __attribute((aligned(64))); + accel.and128(offset * sizeof(uint64_t), vRefs, dest); int diff = memcmp(&expected[offset], dest, sizeof(dest)); if (diff != 0) { - LOG_ABORT("Accelerator fails to compute correct 64 bytes AND"); + LOG_ABORT("Accelerator fails to compute correct 128 bytes AND"); } } @@ -186,9 +186,9 @@ void verifyOr64(const IAccelrated & accel) { std::vector<std::vector<uint64_t>> vectors(3) ; for (auto & v : vectors) { - fill(v, 16); + fill(v, 32); } - for (size_t offset = 0; offset < 8; offset++) { + for (size_t offset = 0; offset < 16; offset++) { for (size_t i = 1; i < vectors.size(); i++) { verifyOr64(accel, vectors, offset, i, false); verifyOr64(accel, vectors, offset, i, true); @@ -200,9 +200,9 @@ void verifyAnd64(const IAccelrated & accel) { std::vector<std::vector<uint64_t>> vectors(3); for (auto & v : vectors) { - fill(v, 16); + fill(v, 32); } - for (size_t offset = 0; offset < 8; offset++) { + for (size_t offset = 0; offset < 16; offset++) { for (size_t i = 1; i < vectors.size(); i++) { verifyAnd64(accel, vectors, offset, i, false); verifyAnd64(accel, vectors, offset, i, true); diff --git a/vespalib/src/vespa/vespalib/hwaccelrated/iaccelrated.h b/vespalib/src/vespa/vespalib/hwaccelrated/iaccelrated.h index 806e77caced..f070f206b7e 100644 --- a/vespalib/src/vespa/vespalib/hwaccelrated/iaccelrated.h +++ b/vespalib/src/vespa/vespalib/hwaccelrated/iaccelrated.h @@ -31,10 +31,10 @@ public: virtual double squaredEuclideanDistance(const int8_t * a, const int8_t * b, size_t sz) const noexcept = 0; virtual double squaredEuclideanDistance(const float * a, const float * b, size_t sz) const noexcept = 0; virtual double squaredEuclideanDistance(const double * a, const double * b, size_t sz) const noexcept = 0; - // AND 64 bytes from multiple, optionally inverted sources - virtual void and64(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept = 0; - // OR 64 bytes from multiple, optionally inverted sources - virtual void or64(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept = 0; + // AND 128 bytes from multiple, optionally inverted sources + virtual void and128(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept = 0; + // OR 128 bytes from multiple, optionally inverted sources + virtual void or128(size_t offset, const std::vector<std::pair<const void *, bool>> &src, void *dest) const noexcept = 0; static const IAccelrated & getAccelerator() __attribute__((noinline)); }; diff --git a/vespalib/src/vespa/vespalib/hwaccelrated/private_helpers.hpp b/vespalib/src/vespa/vespalib/hwaccelrated/private_helpers.hpp index c884f0d7bb9..6731b449462 100644 --- a/vespalib/src/vespa/vespalib/hwaccelrated/private_helpers.hpp +++ b/vespalib/src/vespa/vespalib/hwaccelrated/private_helpers.hpp @@ -43,7 +43,7 @@ void andChunks(size_t offset, const std::vector<std::pair<const void *, bool>> & src, void * dest) { typedef uint64_t Chunk __attribute__ ((vector_size (ChunkSize))); static_assert(sizeof(Chunk) == ChunkSize, "sizeof(Chunk) == ChunkSize"); - static_assert(ChunkSize*Chunks == 64, "ChunkSize*Chunks == 64"); + static_assert(ChunkSize*Chunks == 128, "ChunkSize*Chunks == 128"); Chunk * chunk = static_cast<Chunk *>(dest); const Chunk * tmp = cast<Chunk, ChunkSize>(src[0].first, offset); for (size_t n=0; n < Chunks; n++) { @@ -62,7 +62,7 @@ void orChunks(size_t offset, const std::vector<std::pair<const void *, bool>> & src, void * dest) { typedef uint64_t Chunk __attribute__ ((vector_size (ChunkSize))); static_assert(sizeof(Chunk) == ChunkSize, "sizeof(Chunk) == ChunkSize"); - static_assert(ChunkSize*Chunks == 64, "ChunkSize*Chunks == 64"); + static_assert(ChunkSize*Chunks == 128, "ChunkSize*Chunks == 128"); Chunk * chunk = static_cast<Chunk *>(dest); const Chunk * tmp = cast<Chunk, ChunkSize>(src[0].first, offset); for (size_t n=0; n < Chunks; n++) { |