diff options
83 files changed, 529 insertions, 624 deletions
diff --git a/Gemfile.lock b/Gemfile.lock index b98be0cafcf..01c440e3772 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -79,7 +79,7 @@ GEM jekyll (>= 3.5, < 5.0) jekyll-feed (~> 0.9) jekyll-seo-tag (~> 2.1) - nokogiri (1.16.4-x86_64-linux) + nokogiri (1.16.5-x86_64-linux) racc (~> 1.4) pathutil (0.16.2) forwardable-extended (~> 2.6) diff --git a/build_settings.cmake b/build_settings.cmake index 1549ac83c74..e046d7f71f3 100644 --- a/build_settings.cmake +++ b/build_settings.cmake @@ -65,6 +65,9 @@ endif() set(VESPA_ATOMIC_LIB "atomic") if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang") set(CXX_SPECIFIC_WARN_OPTS "-Wnon-virtual-dtor -Wformat-security -Wno-overloaded-virtual") + if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 18.0) + set(CXX_SPECIFIC_WARN_OPTS "${CXX_SPECIFIC_WARN_OPTS} -Wno-error=vla-cxx-extension") + endif() set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-delete-null-pointer-checks -fsized-deallocation") if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Darwin") set(VESPA_ATOMIC_LIB "") diff --git a/client/README.md b/client/README.md index f51db87d631..e730614230f 100644 --- a/client/README.md +++ b/client/README.md @@ -39,7 +39,7 @@ This is a [work-in-progress javascript app](js/app) for querying a Vespa applica <!-- ToDo: move this / demote this somehow --> -### vespa_query_dsl +### vespa\_query\_dsl This lib is used for composing Vespa [YQL queries](https://docs.vespa.ai/en/reference/query-language-reference.html). diff --git a/client/go/go.mod b/client/go/go.mod index 7117d5ef334..4fe9108efaf 100644 --- a/client/go/go.mod +++ b/client/go/go.mod @@ -5,7 +5,7 @@ go 1.20 require ( github.com/alessio/shellescape v1.4.2 github.com/briandowns/spinner v1.23.0 - github.com/fatih/color v1.16.0 + github.com/fatih/color v1.17.0 // This is the most recent version compatible with Go 1.20. Upgrade when we upgrade our Go version github.com/go-json-experiment/json v0.0.0-20230324203220-04923b7a9528 github.com/klauspost/compress v1.17.8 diff --git a/client/go/go.sum b/client/go/go.sum index e98a14f862a..2af61a8ce98 100644 --- a/client/go/go.sum +++ b/client/go/go.sum @@ -10,6 +10,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM= github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE= +github.com/fatih/color v1.17.0 h1:GlRw1BRJxkpqUCBKzKOw098ed57fEsKeNjpTe3cSjK4= +github.com/fatih/color v1.17.0/go.mod h1:YZ7TlrGPkiz6ku9fK3TLD/pl3CpsiFyu8N92HLgmosI= github.com/go-json-experiment/json v0.0.0-20230324203220-04923b7a9528 h1:hmpF6G+rHcypt8J6jhBH/rDUx+04Th/L61Y8uCKFb7Q= github.com/go-json-experiment/json v0.0.0-20230324203220-04923b7a9528/go.mod h1:AHV+bpNGVGD0DCHMBhhTYtT7yeBYD9Yk92XAjB7vOgo= github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk= diff --git a/client/go/internal/vespa/document/throttler.go b/client/go/internal/vespa/document/throttler.go index 39900156563..3eb0ccd17f6 100644 --- a/client/go/internal/vespa/document/throttler.go +++ b/client/go/internal/vespa/document/throttler.go @@ -37,8 +37,8 @@ type dynamicThrottler struct { func newThrottler(connections int, nowFunc func() time.Time) *dynamicThrottler { var ( - minInflight = 16 * int64(connections) - maxInflight = 256 * minInflight // 4096 max streams per connection on the server side + minInflight = 2 * int64(connections) + maxInflight = 256 * minInflight // 512 max streams per connection on the server side ) t := &dynamicThrottler{ minInflight: minInflight, @@ -49,7 +49,7 @@ func newThrottler(connections int, nowFunc func() time.Time) *dynamicThrottler { start: nowFunc(), now: nowFunc, } - t.targetInflight.Store(8 * minInflight) + t.targetInflight.Store(minInflight) t.targetTimesTen.Store(10 * maxInflight) return t } @@ -57,7 +57,7 @@ func newThrottler(connections int, nowFunc func() time.Time) *dynamicThrottler { func NewThrottler(connections int) Throttler { return newThrottler(connections, time.Now) } func (t *dynamicThrottler) Sent() { - currentInflight := t.targetInflight.Load() + currentInflight := t.TargetInflight() t.sent++ if t.sent*t.sent*t.sent < 100*currentInflight*currentInflight { return @@ -73,8 +73,12 @@ func (t *dynamicThrottler) Sent() { t.throughputs[index] = currentThroughput // Loop over throughput measurements and pick the one which optimises throughput and latency. - choice := float64(currentInflight) + best := float64(currentInflight) maxObjective := float64(-1) + choice := 0 + j := -1 + k := -1 + s := 0.0 for i := len(t.throughputs) - 1; i >= 0; i-- { if t.throughputs[i] == 0 { continue // Skip unknown values @@ -83,10 +87,25 @@ func (t *dynamicThrottler) Sent() { objective := t.throughputs[i] * math.Pow(inflight, throttlerWeight-1) // Optimise throughput (weight), but also latency (1 - weight) if objective > maxObjective { maxObjective = objective - choice = inflight + best = inflight + choice = i } + // Additionally, smooth the throughput values, to reduce the impact of noise, and reduce jumpiness + if j != -1 { + u := t.throughputs[j] + if k != -1 { + t.throughputs[j] = (2*u + t.throughputs[i] + s) / 4 + } + s = u + } + k = j + j = i + } + target := int64((rand.Float64()*0.40+0.84)*best + rand.Float64()*4 - 1) // Random walk, skewed towards increase + // If the best inflight is at the high end of the known, we override the random walk to speed up upwards exploration + if choice == j && choice+1 < len(t.throughputs) { + target = int64(1 + float64(t.minInflight)*math.Pow(256, (float64(choice)+1.5)/float64(len(t.throughputs)))) } - target := int64((rand.Float64()*0.20 + 0.92) * choice) // Random walk, skewed towards increase t.targetInflight.Store(max(t.minInflight, min(t.maxInflight, target))) } diff --git a/client/go/internal/vespa/document/throttler_test.go b/client/go/internal/vespa/document/throttler_test.go index 03f0bc75bdc..b386e0d5105 100644 --- a/client/go/internal/vespa/document/throttler_test.go +++ b/client/go/internal/vespa/document/throttler_test.go @@ -9,14 +9,19 @@ import ( func TestThrottler(t *testing.T) { clock := &manualClock{tick: time.Second} tr := newThrottler(8, clock.now) - for i := 0; i < 100; i++ { + + if got, want := tr.TargetInflight(), int64(16); got != want { + t.Errorf("got TargetInflight() = %d, but want %d", got, want) + } + for i := 0; i < 30; i++ { tr.Sent() + tr.Success() } - if got, want := tr.TargetInflight(), int64(1024); got != want { + if got, want := tr.TargetInflight(), int64(18); got != want { t.Errorf("got TargetInflight() = %d, but want %d", got, want) } - tr.Throttled(5) - if got, want := tr.TargetInflight(), int64(128); got != want { + tr.Throttled(34) + if got, want := tr.TargetInflight(), int64(17); got != want { t.Errorf("got TargetInflight() = %d, but want %d", got, want) } } diff --git a/client/js/app/yarn.lock b/client/js/app/yarn.lock index edeb568b30b..3b6d996ff64 100644 --- a/client/js/app/yarn.lock +++ b/client/js/app/yarn.lock @@ -1315,10 +1315,10 @@ dependencies: "@babel/runtime" "^7.13.10" -"@remix-run/router@1.16.0": - version "1.16.0" - resolved "https://registry.yarnpkg.com/@remix-run/router/-/router-1.16.0.tgz#0e10181e5fec1434eb071a9bc4bdaac843f16dcc" - integrity sha512-Quz1KOffeEf/zwkCBM3kBtH4ZoZ+pT3xIXBG4PPW/XFtDP7EGhtTiC2+gpL9GnR7+Qdet5Oa6cYSvwKYg6kN9Q== +"@remix-run/router@1.16.1": + version "1.16.1" + resolved "https://registry.yarnpkg.com/@remix-run/router/-/router-1.16.1.tgz#73db3c48b975eeb06d0006481bde4f5f2d17d1cd" + integrity sha512-es2g3dq6Nb07iFxGk5GuHN20RwBZOsuDQN7izWIisUcv9r+d2C5jQxqmgkdebXgReWfiyUabcki6Fg77mSNrig== "@rollup/rollup-android-arm-eabi@4.17.2": version "4.17.2" @@ -4718,19 +4718,19 @@ react-refresh@^0.14.0: integrity sha512-wViHqhAd8OHeLS/IRMJjTSDHF3U9eWi62F/MledQGPdJGDhodXJ9PBLNGr6WWL7qlH12Mt3TyTpbS+hGXMjCzQ== react-router-dom@^6: - version "6.23.0" - resolved "https://registry.yarnpkg.com/react-router-dom/-/react-router-dom-6.23.0.tgz#8b80ad92ad28f4dc38972e92d84b4c208150545a" - integrity sha512-Q9YaSYvubwgbal2c9DJKfx6hTNoBp3iJDsl+Duva/DwxoJH+OTXkxGpql4iUK2sla/8z4RpjAm6EWx1qUDuopQ== + version "6.23.1" + resolved "https://registry.yarnpkg.com/react-router-dom/-/react-router-dom-6.23.1.tgz#30cbf266669693e9492aa4fc0dde2541ab02322f" + integrity sha512-utP+K+aSTtEdbWpC+4gxhdlPFwuEfDKq8ZrPFU65bbRJY+l706qjR7yaidBpo3MSeA/fzwbXWbKBI6ftOnP3OQ== dependencies: - "@remix-run/router" "1.16.0" - react-router "6.23.0" + "@remix-run/router" "1.16.1" + react-router "6.23.1" -react-router@6.23.0: - version "6.23.0" - resolved "https://registry.yarnpkg.com/react-router/-/react-router-6.23.0.tgz#2f2d7492c66a6bdf760be4c6bdf9e1d672fa154b" - integrity sha512-wPMZ8S2TuPadH0sF5irFGjkNLIcRvOSaEe7v+JER8508dyJumm6XZB1u5kztlX0RVq6AzRVndzqcUh6sFIauzA== +react-router@6.23.1: + version "6.23.1" + resolved "https://registry.yarnpkg.com/react-router/-/react-router-6.23.1.tgz#d08cbdbd9d6aedc13eea6e94bc6d9b29cb1c4be9" + integrity sha512-fzcOaRF69uvqbbM7OhvQyBTFDVrrGlsFdS3AL+1KfIBtGETibHzi3FkoTRyiDJnWNc2VxrfvR+657ROHjaNjqQ== dependencies: - "@remix-run/router" "1.16.0" + "@remix-run/router" "1.16.1" react-textarea-autosize@8.3.4: version "8.3.4" diff --git a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterController.java b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterController.java index 7e0c6fe3f63..114b88f03a8 100644 --- a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterController.java +++ b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterController.java @@ -11,11 +11,12 @@ import com.yahoo.vespa.clustercontroller.core.RemoteClusterControllerTaskSchedul import com.yahoo.vespa.clustercontroller.core.restapiv2.ClusterControllerStateRestAPI; import com.yahoo.vespa.clustercontroller.core.status.StatusHandler; import com.yahoo.vespa.zookeeper.server.VespaZooKeeperServer; + +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; import java.util.TreeMap; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Logger; /** @@ -27,9 +28,10 @@ public class ClusterController extends AbstractComponent private static final Logger log = Logger.getLogger(ClusterController.class.getName()); private final JDiscMetricWrapper metricWrapper; + private final Object monitor = new Object(); private final Map<String, FleetController> controllers = new TreeMap<>(); private final Map<String, StatusHandler.ContainerStatusPageServer> status = new TreeMap<>(); - private final AtomicInteger referents = new AtomicInteger(); + private final Map<String, Integer> referents = new HashMap<>(); private final AtomicBoolean shutdown = new AtomicBoolean(); /** @@ -44,9 +46,9 @@ public class ClusterController extends AbstractComponent } public void setOptions(FleetControllerOptions options, Metric metricImpl) throws Exception { - referents.incrementAndGet(); metricWrapper.updateMetricImplementation(metricImpl); - synchronized (controllers) { + synchronized (monitor) { + referents.merge(options.clusterName(), 1, Integer::sum); FleetController controller = controllers.get(options.clusterName()); if (controller == null) { controller = FleetController.create(options, metricWrapper); @@ -68,21 +70,34 @@ public class ClusterController extends AbstractComponent * we must also let the last configurer shut down this controller, to ensure this is shut down * before the ZK server it had injected from the configurers. */ - void countdown() { - if (referents.decrementAndGet() == 0) - shutdown(); + void countdown(String clusterName) { + synchronized (monitor) { + referents.compute(clusterName, (__, count) -> { + if (count == null) throw new IllegalStateException("trying to remove unknown cluster: " + clusterName); + if (count == 1) { + shutDownController(controllers.remove(clusterName)); + status.remove(clusterName); + return null; + } + return count - 1; + }); + } + } + + private void shutDownController(FleetController controller) { + if (controller == null) return; + try { + controller.shutdown(); + } catch (Exception e) { + log.warning("Failed to shut down fleet controller: " + e.getMessage()); + } } void shutdown() { if (shutdown.compareAndSet(false, true)) { - synchronized (controllers) { + synchronized (monitor) { for (FleetController controller : controllers.values()) { - try { - shutdownController(controller); - } - catch (Exception e) { - log.warning("Failed to shut down fleet controller: " + e.getMessage()); - } + shutDownController(controller); } } } @@ -90,7 +105,7 @@ public class ClusterController extends AbstractComponent @Override public Map<String, RemoteClusterControllerTaskScheduler> getFleetControllers() { - synchronized (controllers) { + synchronized (monitor) { return new LinkedHashMap<>(controllers); } } @@ -105,8 +120,4 @@ public class ClusterController extends AbstractComponent return status; } - void shutdownController(FleetController controller) throws Exception { - controller.shutdown(); - } - } diff --git a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java index 5a2034f0372..265a99e2f72 100644 --- a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java +++ b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java @@ -45,7 +45,7 @@ public class ClusterControllerClusterConfigurer extends AbstractComponent { @Override public void deconstruct() { - if (controller != null) controller.countdown(); + if (controller != null) controller.countdown(options.clusterName()); } FleetControllerOptions getOptions() { return options; } diff --git a/config-model/src/main/java/com/yahoo/schema/document/SDField.java b/config-model/src/main/java/com/yahoo/schema/document/SDField.java index f165141b16e..2483fa47667 100644 --- a/config-model/src/main/java/com/yahoo/schema/document/SDField.java +++ b/config-model/src/main/java/com/yahoo/schema/document/SDField.java @@ -46,7 +46,7 @@ import java.util.TreeMap; * * @author bratseth */ -public class SDField extends Field implements TypedKey, ImmutableSDField { +public class SDField extends Field implements ImmutableSDField { /** Use this field for modifying index-structure, even if it doesn't have any indexing code */ private boolean indexStructureField = false; @@ -315,7 +315,7 @@ public class SDField extends Field implements TypedKey, ImmutableSDField { supplyStructField.accept(field.getName(), field.getDataType()); } } - if ((subType == null) && (structFields.size() > 0)) { + if ((subType == null) && (!structFields.isEmpty())) { throw new IllegalArgumentException("Cannot find matching (repo=" + sdoc + ") for subfields in " + this + " [" + getDataType() + getDataType().getClass() + "] with " + structFields.size() + " struct fields"); @@ -627,7 +627,7 @@ public class SDField extends Field implements TypedKey, ImmutableSDField { public Attribute addAttribute(Attribute attribute) { String name = attribute.getName(); - if (name == null || "".equals(name)) { + if (name == null || name.isEmpty()) { name = getName(); attribute.setName(name); } diff --git a/config-model/src/main/java/com/yahoo/schema/document/TypedKey.java b/config-model/src/main/java/com/yahoo/schema/document/TypedKey.java deleted file mode 100644 index 652d21d7f7d..00000000000 --- a/config-model/src/main/java/com/yahoo/schema/document/TypedKey.java +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.schema.document; - -import com.yahoo.document.DataType; - -/** - * Common interface for various typed key (or field definitions). - * Used by code which wants to use common algorithms for dealing with typed keys, like the logical mapping - * - * @author bratseth - */ -public interface TypedKey { - - String getName(); - - void setDataType(DataType type); - - DataType getDataType(); - -} diff --git a/config-model/src/main/java/com/yahoo/schema/processing/AttributesImplicitWord.java b/config-model/src/main/java/com/yahoo/schema/processing/AttributesImplicitWord.java index 767593b82d0..769f0c9de92 100644 --- a/config-model/src/main/java/com/yahoo/schema/processing/AttributesImplicitWord.java +++ b/config-model/src/main/java/com/yahoo/schema/processing/AttributesImplicitWord.java @@ -2,6 +2,7 @@ package com.yahoo.schema.processing; import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.document.TensorDataType; import com.yahoo.schema.RankProfileRegistry; import com.yahoo.document.DataType; import com.yahoo.schema.Schema; @@ -45,6 +46,8 @@ public class AttributesImplicitWord extends Processor { private boolean fieldImplicitlyWordMatch(ImmutableSDField field) { // numeric types should not trigger exact-match query parsing if (field.getDataType().getPrimitiveType() instanceof NumericDataType) return false; + // Tensor type should not trigger exact-match query parsing + if (field.getDataType() instanceof TensorDataType) return false; return (! field.hasIndex() && !field.getAttributes().isEmpty() diff --git a/config-model/src/main/java/com/yahoo/vespa/documentmodel/SummaryField.java b/config-model/src/main/java/com/yahoo/vespa/documentmodel/SummaryField.java index d50d5e36134..785b45d8def 100644 --- a/config-model/src/main/java/com/yahoo/vespa/documentmodel/SummaryField.java +++ b/config-model/src/main/java/com/yahoo/vespa/documentmodel/SummaryField.java @@ -3,10 +3,12 @@ package com.yahoo.vespa.documentmodel; import com.yahoo.document.DataType; import com.yahoo.document.Field; -import com.yahoo.schema.document.TypedKey; import java.io.Serializable; -import java.util.*; +import java.util.Collection; +import java.util.Iterator; +import java.util.LinkedHashSet; +import java.util.Set; import java.util.stream.Collectors; import static com.yahoo.text.Lowercase.toLowerCase; @@ -16,7 +18,7 @@ import static com.yahoo.text.Lowercase.toLowerCase; * * @author bratseth */ -public class SummaryField extends Field implements Cloneable, TypedKey { +public class SummaryField extends Field implements Cloneable { /** A source (field name). */ public static class Source implements Serializable { @@ -62,7 +64,7 @@ public class SummaryField extends Field implements Cloneable, TypedKey { */ private Set<Source> sources = new java.util.LinkedHashSet<>(); - private Set<String> destinations =new java.util.LinkedHashSet<>(); + private Set<String> destinations = new java.util.LinkedHashSet<>(); /** True if this field was defined implicitly */ private boolean implicit = false; diff --git a/config-model/src/test/derived/attributes/index-info.cfg b/config-model/src/test/derived/attributes/index-info.cfg index 1d4e8f485b3..245cff48d15 100644 --- a/config-model/src/test/derived/attributes/index-info.cfg +++ b/config-model/src/test/derived/attributes/index-info.cfg @@ -175,8 +175,6 @@ indexinfo[].command[].indexname "a13" indexinfo[].command[].command "attribute" indexinfo[].command[].indexname "a13" indexinfo[].command[].command "type tensor(x{})" -indexinfo[].command[].indexname "a13" -indexinfo[].command[].command "word" indexinfo[].command[].indexname "a7_arr" indexinfo[].command[].command "lowercase" indexinfo[].command[].indexname "a7_arr" diff --git a/config-model/src/test/derived/nearestneighbor_streaming/vsmfields.cfg b/config-model/src/test/derived/nearestneighbor_streaming/vsmfields.cfg index ab9a96f819b..ec06d01f05a 100644 --- a/config-model/src/test/derived/nearestneighbor_streaming/vsmfields.cfg +++ b/config-model/src/test/derived/nearestneighbor_streaming/vsmfields.cfg @@ -3,25 +3,25 @@ searchall 1 fieldspec[].name "vec_a" fieldspec[].searchmethod NEAREST_NEIGHBOR fieldspec[].arg1 "EUCLIDEAN" -fieldspec[].normalize LOWERCASE +fieldspec[].normalize LOWERCASE_AND_FOLD fieldspec[].maxlength 1048576 fieldspec[].fieldtype ATTRIBUTE fieldspec[].name "vec_b" fieldspec[].searchmethod NEAREST_NEIGHBOR fieldspec[].arg1 "ANGULAR" -fieldspec[].normalize LOWERCASE +fieldspec[].normalize LOWERCASE_AND_FOLD fieldspec[].maxlength 1048576 fieldspec[].fieldtype ATTRIBUTE fieldspec[].name "vec_c" fieldspec[].searchmethod NEAREST_NEIGHBOR fieldspec[].arg1 "INNERPRODUCT" -fieldspec[].normalize LOWERCASE +fieldspec[].normalize LOWERCASE_AND_FOLD fieldspec[].maxlength 1048576 fieldspec[].fieldtype ATTRIBUTE fieldspec[].name "vec_d" fieldspec[].searchmethod NONE fieldspec[].arg1 "" -fieldspec[].normalize LOWERCASE +fieldspec[].normalize LOWERCASE_AND_FOLD fieldspec[].maxlength 1048576 fieldspec[].fieldtype ATTRIBUTE documenttype[].name "test" diff --git a/config-model/src/test/derived/tensor/index-info.cfg b/config-model/src/test/derived/tensor/index-info.cfg index c9ce2433e17..2402f074837 100644 --- a/config-model/src/test/derived/tensor/index-info.cfg +++ b/config-model/src/test/derived/tensor/index-info.cfg @@ -9,26 +9,18 @@ indexinfo[].command[].indexname "f2" indexinfo[].command[].command "attribute" indexinfo[].command[].indexname "f2" indexinfo[].command[].command "type tensor<float>(x[2],y[1])" -indexinfo[].command[].indexname "f2" -indexinfo[].command[].command "word" indexinfo[].command[].indexname "f3" indexinfo[].command[].command "attribute" indexinfo[].command[].indexname "f3" indexinfo[].command[].command "type tensor(x{})" -indexinfo[].command[].indexname "f3" -indexinfo[].command[].command "word" indexinfo[].command[].indexname "f4" indexinfo[].command[].command "attribute" indexinfo[].command[].indexname "f4" indexinfo[].command[].command "type tensor(x[10],y[10])" -indexinfo[].command[].indexname "f4" -indexinfo[].command[].command "word" indexinfo[].command[].indexname "f5" indexinfo[].command[].command "attribute" indexinfo[].command[].indexname "f5" indexinfo[].command[].command "type tensor<float>(x[10])" -indexinfo[].command[].indexname "f5" -indexinfo[].command[].command "word" indexinfo[].command[].indexname "f6" indexinfo[].command[].command "attribute" indexinfo[].command[].indexname "f6" @@ -39,5 +31,3 @@ indexinfo[].command[].indexname "f7" indexinfo[].command[].command "attribute" indexinfo[].command[].indexname "f7" indexinfo[].command[].command "type tensor<int8>(p{},x[5])" -indexinfo[].command[].indexname "f7" -indexinfo[].command[].command "word" diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/ContentClusterFixture.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/ContentClusterFixture.java index 8778f0c26c0..0677cabafb0 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/ContentClusterFixture.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/ContentClusterFixture.java @@ -29,11 +29,18 @@ public abstract class ContentClusterFixture { nextCluster = createCluster(nextSd); } + protected ContentClusterFixture(ContentCluster currentCluster, ContentCluster nextCluster) { + this.currentCluster = currentCluster; + this.nextCluster = nextCluster; + } + public ContentClusterFixture(String entireSd) throws Exception { - currentCluster = new ContentClusterBuilder().build( - ContentClusterUtils.createMockRoot(List.of(entireSd))); - nextCluster = new ContentClusterBuilder().build( - ContentClusterUtils.createMockRoot(List.of(entireSd))); + currentCluster = createClusterFromEntireSd(entireSd); + nextCluster = createClusterFromEntireSd(entireSd); + } + + protected static ContentCluster createClusterFromEntireSd(String sdContent) throws Exception { + return new ContentClusterBuilder().build(ContentClusterUtils.createMockRoot(List.of(sdContent))); } private static ContentCluster createCluster(String sdContent) throws Exception { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/IndexingScriptChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/IndexingScriptChangeValidatorTest.java index cd54a20523f..247f01068fa 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/IndexingScriptChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/IndexingScriptChangeValidatorTest.java @@ -39,6 +39,21 @@ public class IndexingScriptChangeValidatorTest { } } + private static class ComplexFixture extends ContentClusterFixture { + IndexingScriptChangeValidator validator; + public ComplexFixture(String currentSd, String nextSd) throws Exception { + super(createClusterFromEntireSd(currentSd), createClusterFromEntireSd(nextSd)); + validator = new IndexingScriptChangeValidator(ClusterSpec.Id.from("test"), + currentDb().getDerivedConfiguration().getSchema(), + nextDb().getDerivedConfiguration().getSchema()); + } + + @Override + public List<VespaConfigChangeAction> validate() { + return validator.validate(); + } + } + private static class ScriptFixture { private final ScriptExpression currentScript; @@ -56,6 +71,9 @@ public class IndexingScriptChangeValidatorTest { private static final String FIELD = "field f1 type string"; private static final String FIELD_F2 = "field f2 type string"; + private static final String TENSOR_FIELD_F1 = "field f1 type tensor(x[2])"; + private static final String TENSOR_FIELD_F2 = "field f2 type tensor(x[2])"; + private static final String TENSOR_FIELD_F3 = "field f3 type tensor(x[2])"; private static VespaConfigChangeAction expectedReindexingAction(String changedMsg, String fromScript, String toScript) { return expectedReindexingAction("f1", changedMsg, fromScript, toScript); @@ -115,6 +133,28 @@ public class IndexingScriptChangeValidatorTest { } @Test + void requireThatAddingIndexAspectForExtraTensorFieldWithChangedInputRequireReindexing() throws Exception { + new ComplexFixture(joinLines("schema test {", + " document test {", + " " + TENSOR_FIELD_F1 + " { }", + " " + TENSOR_FIELD_F2 + " { }", + " }", + " " + TENSOR_FIELD_F3 + " { indexing: input f1 | attribute }", + "}"), + joinLines("schema test {", + " document test {", + " " + TENSOR_FIELD_F1 + " { }", + " " + TENSOR_FIELD_F2 + " { }", + " }", + " " + TENSOR_FIELD_F3 + " { indexing: input f2 | index | attribute }", + "}")). + assertValidation(List.of(expectedReindexingAction("f3", "add index aspect", + "{ input f1 | attribute f3; }", + "{ input f2 | index f3 | attribute f3; }"))); + } + + + @Test void requireThatSettingDynamicSummaryIsOk() throws Exception { new Fixture(FIELD + " { indexing: summary }", FIELD + " { indexing: summary \n summary: dynamic }"). diff --git a/configdefinitions/src/vespa/CMakeLists.txt b/configdefinitions/src/vespa/CMakeLists.txt index 81e587fcace..0ab12932880 100644 --- a/configdefinitions/src/vespa/CMakeLists.txt +++ b/configdefinitions/src/vespa/CMakeLists.txt @@ -6,8 +6,6 @@ vespa_add_library(configdefinitions ) vespa_generate_config(configdefinitions application-id.def) install_config_definition(application-id.def cloud.config.application-id.def) -vespa_generate_config(configdefinitions athenz-provider-service.def) -install_config_definition(athenz-provider-service.def vespa.hosted.athenz.instanceproviderservice.config.athenz-provider-service.def) vespa_generate_config(configdefinitions attributes.def) install_config_definition(attributes.def vespa.config.search.attributes.def) vespa_generate_config(configdefinitions cluster-info.def) diff --git a/configdefinitions/src/vespa/athenz-provider-service.def b/configdefinitions/src/vespa/athenz-provider-service.def deleted file mode 100644 index 5ee9be323e8..00000000000 --- a/configdefinitions/src/vespa/athenz-provider-service.def +++ /dev/null @@ -1,37 +0,0 @@ -# Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -namespace=vespa.hosted.athenz.instanceproviderservice.config - -# Athenz domain -domain string - -# Athenz service name -serviceName string - -# Secret name of private Key -secretName string - -# Secret version -secretVersion int - -# Tempory resources -sisSecretName string default="" -sisSecretVersion int default=0 -sisUrl string default = "" - -# Secret name of CA certificate -caCertSecretName string - -# Certificate DNS suffix -certDnsSuffix string default="" - -# Athenz ZTS server url -ztsUrl string default="" - -# Path to Athenz CA JKS trust store -athenzCaTrustStore string default="" - -# Period between certificate updates -updatePeriodDays int default=1 - -# Tenant Service id -tenantService string default=vespa.vespa.tenant diff --git a/configserver/src/main/resources/configserver-app/services.xml b/configserver/src/main/resources/configserver-app/services.xml index fb6516ce6cf..d71840a95c2 100644 --- a/configserver/src/main/resources/configserver-app/services.xml +++ b/configserver/src/main/resources/configserver-app/services.xml @@ -45,13 +45,11 @@ </components> <preprocess:include file='config-models.xml' required='false' /> - <preprocess:include file='routing-status.xml' required='false' /> <preprocess:include file='model-integration.xml' required='true' /> <component id="com.yahoo.vespa.configserver.flags.ConfigServerFlagSource" bundle="configserver-flags"/> <component id="com.yahoo.vespa.configserver.flags.db.FlagsDbImpl" bundle="configserver-flags"/> - <preprocess:include file='metrics-packets.xml' required='false' /> <component id="com.yahoo.vespa.service.slobrok.SlobrokMonitorManagerImpl" bundle="service-monitor" /> <component id="com.yahoo.vespa.service.health.HealthMonitorManager" bundle="service-monitor" /> <component id="com.yahoo.vespa.service.manager.UnionMonitorManager" bundle="service-monitor" /> @@ -153,13 +151,9 @@ <preprocess:include file='http-server.xml' required='false' /> </http> - <preprocess:include file='athenz-identity-provider.xml' required='false' /> - <preprocess:include file='configserver-config.xml' required='false' /> <preprocess:include file='configserver-components.xml' required='false' /> - <preprocess:include file='zookeeper-server-config.xml' required='false' /> - </container> </services> diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java index ea0cd2312a6..d3e6241a6e5 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java @@ -134,10 +134,8 @@ abstract class SimpleParser extends StructuredParser { if (topLevelItem != null && topLevelItem != not) { // => neutral rank items becomes implicit positives not.addPositiveItem(getItemAsPositiveItem(topLevelItem, not)); - return not; - } else { - return not; } + return not; } if (topLevelItem != null) { return topLevelItem; diff --git a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java index ffa6c82e941..611df6ad284 100644 --- a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java @@ -1532,7 +1532,7 @@ public class JsonRendererTestCase { + "}"; assertEquals( "Unexpected character ('a' (code 97)): was expecting comma to separate Object entries\n" + - " at [Source: (String)\"{ \"root\": { \"invalidvalue\": 1adsf, }}\"; line: 1, column: 41]", + " at [Source: (String)\"{ \"root\": { \"invalidvalue\": 1adsf, }}\"; line: 1, column: 40]", validateJSON(json)); } diff --git a/dependency-versions/pom.xml b/dependency-versions/pom.xml index 73aee9fd093..b61ae50449d 100644 --- a/dependency-versions/pom.xml +++ b/dependency-versions/pom.xml @@ -37,7 +37,7 @@ <guava.vespa.version>33.2.0-jre</guava.vespa.version> <guice.vespa.version>6.0.0</guice.vespa.version> <j2objc-annotations.vespa.version>3.0.0</j2objc-annotations.vespa.version> - <jackson2.vespa.version>2.16.2</jackson2.vespa.version> + <jackson2.vespa.version>2.17.1</jackson2.vespa.version> <jackson-databind.vespa.version>${jackson2.vespa.version}</jackson-databind.vespa.version> <jakarta.inject.vespa.version>2.0.1</jakarta.inject.vespa.version> <javax.activation-api.vespa.version>1.2.0</javax.activation-api.vespa.version> @@ -68,7 +68,7 @@ <assertj.vespa.version>3.25.3</assertj.vespa.version> <!-- Athenz dependencies. Make sure these dependencies match those in Vespa's internal repositories --> - <aws-sdk.vespa.version>1.12.717</aws-sdk.vespa.version> + <aws-sdk.vespa.version>1.12.721</aws-sdk.vespa.version> <athenz.vespa.version>1.11.58</athenz.vespa.version> <!-- Athenz END --> @@ -124,7 +124,7 @@ <maven-wagon.vespa.version>3.5.3</maven-wagon.vespa.version> <maven-xml-impl.vespa.version>4.0.0-alpha-13</maven-xml-impl.vespa.version> <mimepull.vespa.version>1.10.0</mimepull.vespa.version> - <mockito.vespa.version>5.11.0</mockito.vespa.version> + <mockito.vespa.version>5.12.0</mockito.vespa.version> <mojo-executor.vespa.version>2.4.0</mojo-executor.vespa.version> <netty.vespa.version>4.1.109.Final</netty.vespa.version> <netty-tcnative.vespa.version>2.0.65.Final</netty-tcnative.vespa.version> @@ -159,8 +159,8 @@ <!-- CAUTION: upgrading junit for tenants poms may break testing frameworks --> <!-- CAUTION 2: this version must match the exported packages from the tenant-cd-api module --> <!-- CAUTION 3: this is probably not a good idea to change too ofter; consider a major version next time --> - <junit.vespa.tenant.version>5.10.1</junit.vespa.tenant.version> - <junit.platform.vespa.tenant.version>1.10.1</junit.platform.vespa.tenant.version> + <junit.vespa.tenant.version>5.10.2</junit.vespa.tenant.version> + <junit.platform.vespa.tenant.version>1.10.2</junit.platform.vespa.tenant.version> <surefire.vespa.tenant.version>${surefire.vespa.version}</surefire.vespa.tenant.version> <!-- Maven plugins --> 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 df7720f74f1..a577bbe74df 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -53,7 +53,8 @@ public class Flags { List.of("hakonhall"), "2024-05-06", "2024-07-06", "Whether to provision new GCP VM instances with a service account that are independent " + "of the zone, and aligned with the Athenz service names (configserver and tenant-host).", - "Takes effect when provisioning new VM instances"); + "Takes effect when provisioning new VM instances", + APPLICATION, INSTANCE_ID); public static final UnboundDoubleFlag DEFAULT_TERM_WISE_LIMIT = defineDoubleFlag( "default-term-wise-limit", 1.0, @@ -450,6 +451,18 @@ public class Flags { "Whether to encrypt disk when provisioning new hosts", "Will be read only on boot."); + public static UnboundBooleanFlag HUBSPOT_SYNC_TENANTS = defineFeatureFlag( + "hubspot-sync-tenants", false, + List.of("bjorncs"), "2024-05-07", "2025-01-01", + "Whether to sync tenants to HubSpot", + "Takes effect immediately"); + + public static final UnboundBooleanFlag REMOVE_ORPHANED_DNS_RECORDS = defineFeatureFlag( + "remove-orphaned-dns-records", false, + List.of("mpolden"), "2024-05-07", "2024-10-01", + "Whether EndpointDnsMaintainer should remove orphaned records instead of logging them", + "Takes effect on next maintenance run"); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, List<String> owners, String createdAt, String expiresAt, String description, diff --git a/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java b/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java index 3b73d9d6013..2a667930add 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java @@ -18,12 +18,12 @@ import static com.yahoo.vespa.flags.Dimension.ARCHITECTURE; import static com.yahoo.vespa.flags.Dimension.CERTIFICATE_PROVIDER; import static com.yahoo.vespa.flags.Dimension.CLAVE; import static com.yahoo.vespa.flags.Dimension.CLOUD_ACCOUNT; -import static com.yahoo.vespa.flags.Dimension.FLAVOR; -import static com.yahoo.vespa.flags.Dimension.INSTANCE_ID; import static com.yahoo.vespa.flags.Dimension.CLUSTER_ID; import static com.yahoo.vespa.flags.Dimension.CLUSTER_TYPE; import static com.yahoo.vespa.flags.Dimension.CONSOLE_USER_EMAIL; +import static com.yahoo.vespa.flags.Dimension.FLAVOR; import static com.yahoo.vespa.flags.Dimension.HOSTNAME; +import static com.yahoo.vespa.flags.Dimension.INSTANCE_ID; import static com.yahoo.vespa.flags.Dimension.NODE_TYPE; import static com.yahoo.vespa.flags.Dimension.TENANT_ID; import static com.yahoo.vespa.flags.Dimension.VESPA_VERSION; @@ -178,7 +178,7 @@ public class PermanentFlags { HOSTNAME, NODE_TYPE, TENANT_ID, INSTANCE_ID, CLUSTER_TYPE, CLUSTER_ID, VESPA_VERSION); public static final UnboundStringFlag ZOOKEEPER_SERVER_VERSION = defineStringFlag( - "zookeeper-server-version", "3.9.1", + "zookeeper-server-version", "3.9.2", "ZooKeeper server version, a jar file zookeeper-server-<ZOOKEEPER_SERVER_VERSION>-jar-with-dependencies.jar must exist", "Takes effect on restart of Docker container", NODE_TYPE, INSTANCE_ID, HOSTNAME); diff --git a/messagebus/src/main/java/com/yahoo/messagebus/DynamicThrottlePolicy.java b/messagebus/src/main/java/com/yahoo/messagebus/DynamicThrottlePolicy.java index 97f681404e9..1a42b688437 100644 --- a/messagebus/src/main/java/com/yahoo/messagebus/DynamicThrottlePolicy.java +++ b/messagebus/src/main/java/com/yahoo/messagebus/DynamicThrottlePolicy.java @@ -167,10 +167,10 @@ public class DynamicThrottlePolicy extends StaticThrottlePolicy { } /** - * Determines where on each latency level the attractor sits. 2 is at the very end, and makes this to *boom*. + * Determines where on each latency level the attractor sits. 2 is at the very end, and makes this go *boom*. * 0.2 is at the very start, and makes the algorithm more conservative. Probably fine to stay away from this. */ - // Original javadoc is non-sense, but kept for historical reasons. + // Original javadoc is nonsense, but kept for historical reasons. /* * Sets the lower efficiency threshold at which the algorithm should perform window size back off. Efficiency is * the correlation between throughput and window size. The algorithm will increase the window size until efficiency diff --git a/metrics/src/main/java/ai/vespa/metrics/set/Vespa9VespaMetricSet.java b/metrics/src/main/java/ai/vespa/metrics/set/Vespa9VespaMetricSet.java index 5b5126b23f0..957eaf8304f 100644 --- a/metrics/src/main/java/ai/vespa/metrics/set/Vespa9VespaMetricSet.java +++ b/metrics/src/main/java/ai/vespa/metrics/set/Vespa9VespaMetricSet.java @@ -211,6 +211,10 @@ public class Vespa9VespaMetricSet { addMetric(metrics, ContainerMetrics.JDISC_TLS_CAPABILITY_CHECKS_SUCCEEDED.rate()); addMetric(metrics, ContainerMetrics.JDISC_TLS_CAPABILITY_CHECKS_FAILED.rate()); + // Embedders + addMetric(metrics, ContainerMetrics.EMBEDDER_LATENCY, EnumSet.of(max, sum, count)); + addMetric(metrics, ContainerMetrics.EMBEDDER_SEQUENCE_LENGTH, EnumSet.of(max, sum, count)); + return metrics; } diff --git a/metrics/src/main/java/ai/vespa/metrics/set/VespaMetricSet.java b/metrics/src/main/java/ai/vespa/metrics/set/VespaMetricSet.java index e703011ef5f..778d6963e19 100644 --- a/metrics/src/main/java/ai/vespa/metrics/set/VespaMetricSet.java +++ b/metrics/src/main/java/ai/vespa/metrics/set/VespaMetricSet.java @@ -116,10 +116,6 @@ public class VespaMetricSet { // Routing layer metrics addMetric(metrics, RoutingLayerMetrics.WORKER_CONNECTIONS.max()); // Hosted Vespa only (routing layer) - // Embedders - addMetric(metrics, ContainerMetrics.EMBEDDER_LATENCY, EnumSet.of(max, sum, count)); - addMetric(metrics, ContainerMetrics.EMBEDDER_SEQUENCE_LENGTH, EnumSet.of(max, sum, count)); - return metrics; } @@ -237,6 +233,10 @@ public class VespaMetricSet { addMetric(metrics, ContainerMetrics.FEED_LATENCY, EnumSet.of(sum, count, max)); addMetric(metrics, ContainerMetrics.FEED_HTTP_REQUESTS, EnumSet.of(count, rate)); + // Embedders + addMetric(metrics, ContainerMetrics.EMBEDDER_LATENCY, EnumSet.of(max, sum, count)); + addMetric(metrics, ContainerMetrics.EMBEDDER_SEQUENCE_LENGTH, EnumSet.of(max, sum, count)); + // Deprecated metrics. TODO: Remove on Vespa 9. addMetric(metrics, ContainerMetrics.SERVER_REJECTED_REQUESTS, EnumSet.of(rate, count)); addMetric(metrics, ContainerMetrics.SERVER_THREAD_POOL_SIZE, EnumSet.of(max, last)); diff --git a/metrics/src/tests/CMakeLists.txt b/metrics/src/tests/CMakeLists.txt index 043dd7f736d..779b799cc75 100644 --- a/metrics/src/tests/CMakeLists.txt +++ b/metrics/src/tests/CMakeLists.txt @@ -9,7 +9,6 @@ vespa_add_executable(metrics_gtest_runner_app TEST metricsettest.cpp metrictest.cpp snapshottest.cpp - stresstest.cpp summetrictest.cpp valuemetrictest.cpp gtest_runner.cpp diff --git a/metrics/src/tests/stresstest.cpp b/metrics/src/tests/stresstest.cpp deleted file mode 100644 index a5213ba8b2d..00000000000 --- a/metrics/src/tests/stresstest.cpp +++ /dev/null @@ -1,137 +0,0 @@ -// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include <vespa/metrics/metricmanager.h> -#include <vespa/metrics/metrics.h> -#include <vespa/metrics/summetric.hpp> -#include <vespa/vespalib/util/time.h> -#include <vespa/vespalib/util/size_literals.h> -#include <thread> -#include <vespa/vespalib/gtest/gtest.h> - -#include <vespa/log/log.h> -LOG_SETUP(".metrics.test.stress"); - -namespace metrics { - -namespace { -struct InnerMetricSet : public MetricSet { - LongCountMetric _count; - LongAverageMetric _value1; - LongAverageMetric _value2; - SumMetric<LongAverageMetric> _valueSum; - - InnerMetricSet(const char* name, MetricSet* owner = 0); - ~InnerMetricSet(); - - MetricSet* clone(std::vector<Metric::UP> &ownerList, CopyType copyType, - MetricSet* owner, bool includeUnused) const override; -}; - -InnerMetricSet::InnerMetricSet(const char* name, MetricSet* owner) - : MetricSet(name, {}, "", owner), - _count("count", {}, "", this), - _value1("value1", {}, "", this), - _value2("value2", {}, "", this), - _valueSum("valuesum", {}, "", this) -{ - _valueSum.addMetricToSum(_value1); - _valueSum.addMetricToSum(_value2); -} -InnerMetricSet::~InnerMetricSet() = default; - -MetricSet* -InnerMetricSet::clone(std::vector<Metric::UP> &ownerList, CopyType copyType, - MetricSet* owner, bool includeUnused) const -{ - if (copyType != CLONE) { - return MetricSet::clone(ownerList, copyType, owner, includeUnused); -} - InnerMetricSet * myset = new InnerMetricSet(getName().c_str(), owner); - myset->assignValues(*this); - return myset; -} - -struct OuterMetricSet : public MetricSet { - InnerMetricSet _inner1; - InnerMetricSet _inner2; - SumMetric<InnerMetricSet> _innerSum; - InnerMetricSet _tmp; - - OuterMetricSet(MetricSet* owner = 0); - ~OuterMetricSet(); -}; - -OuterMetricSet::OuterMetricSet(MetricSet* owner) - : MetricSet("outer", {}, "", owner), - _inner1("inner1", this), - _inner2("inner2", this), - _innerSum("innersum", {}, "", this), - _tmp("innertmp", 0) -{ - _innerSum.addMetricToSum(_inner1); - _innerSum.addMetricToSum(_inner2); -} - -OuterMetricSet::~OuterMetricSet() = default; - -struct Hammer { - using UP = std::unique_ptr<Hammer>; - - OuterMetricSet& _metrics; - std::atomic<bool> _stop_requested; - std::thread _thread; - - Hammer(OuterMetricSet& metrics) - : _metrics(metrics), - _stop_requested(false), - _thread() - { - _thread = std::thread([this](){run();}); - } - ~Hammer() { - _stop_requested = true; - _thread.join(); - //std::cerr << "Loadgiver thread joined\n"; - } - - void run() { - uint64_t i = 0; - while (!_stop_requested.load(std::memory_order_relaxed)) { - ++i; - setMetrics(i, _metrics._inner1); - setMetrics(i + 3, _metrics._inner2); - } - } - - void setMetrics(uint64_t val, InnerMetricSet& set) { - set._count.inc(val); - set._value1.addValue(val); - set._value2.addValue(val + 10); - } -}; - -} - - -TEST(StressTest, test_stress) -{ - OuterMetricSet metrics; - - LOG(info, "Starting load givers"); - std::vector<Hammer::UP> hammers; - for (uint32_t i=0; i<10; ++i) { - hammers.push_back(std::make_unique<Hammer>(metrics)); - } - LOG(info, "Waiting to let loadgivers hammer a while"); - std::this_thread::sleep_for(5s); - - LOG(info, "Removing loadgivers"); - hammers.clear(); - - LOG(info, "Printing end state"); - std::ostringstream ost; - metrics.print(ost, true, "", 5); - // std::cerr << ost.str() << "\n"; -} - -} diff --git a/parent/pom.xml b/parent/pom.xml index 3299a9fb871..45259e567ca 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.29.0</version> + <version>5.30.0</version> <configuration> <activeRecipes> <recipe>org.openrewrite.java.testing.junit5.JUnit5BestPractices</recipe> @@ -327,7 +327,7 @@ <dependency> <groupId>org.openrewrite.recipe</groupId> <artifactId>rewrite-testing-frameworks</artifactId> - <version>2.7.0</version> + <version>2.8.0</version> </dependency> </dependencies> </plugin> @@ -1199,7 +1199,7 @@ See pluginManagement of rewrite-maven-plugin for more details --> <groupId>org.openrewrite.recipe</groupId> <artifactId>rewrite-recipe-bom</artifactId> - <version>2.10.0</version> + <version>2.11.0</version> <type>pom</type> <scope>import</scope> </dependency> diff --git a/searchlib/src/tests/queryeval/iterator_benchmark/iterator_benchmark_test.cpp b/searchlib/src/tests/queryeval/iterator_benchmark/iterator_benchmark_test.cpp index 96472200952..db0fe76b7af 100644 --- a/searchlib/src/tests/queryeval/iterator_benchmark/iterator_benchmark_test.cpp +++ b/searchlib/src/tests/queryeval/iterator_benchmark/iterator_benchmark_test.cpp @@ -26,6 +26,18 @@ using vespalib::make_string_short::fmt; const vespalib::string field_name = "myfield"; double budget_sec = 1.0; +double estimate_actual_cost(Blueprint &bp, InFlow in_flow) { + if (in_flow.strict()) { + assert(bp.strict()); + return bp.strict_cost(); + } else if (bp.strict()) { + auto stats = FlowStats::from(flow::DefaultAdapter(), &bp); + return flow::forced_strict_cost(stats, in_flow.rate()); + } else { + return bp.cost() * in_flow.rate(); + } +} + enum class PlanningAlgo { Order, Estimate, @@ -236,7 +248,7 @@ strict_search(BenchmarkBlueprintFactory& factory, uint32_t docid_limit, Planning timer.after(); } FlowStats flow(ctx.blueprint->estimate(), ctx.blueprint->cost(), ctx.blueprint->strict_cost()); - double actual_cost = ctx.blueprint->estimate_actual_cost(InFlow(true)); + double actual_cost = estimate_actual_cost(*ctx.blueprint, InFlow(true)); return {timer.min_time() * 1000.0, hits + 1, hits, flow, actual_cost, get_class_name(*ctx.iterator), factory.get_name(*ctx.blueprint)}; } @@ -270,7 +282,7 @@ non_strict_search(BenchmarkBlueprintFactory& factory, uint32_t docid_limit, doub timer.after(); } FlowStats flow(ctx.blueprint->estimate(), ctx.blueprint->cost(), ctx.blueprint->strict_cost()); - double actual_cost = ctx.blueprint->estimate_actual_cost(InFlow(filter_hit_ratio)); + double actual_cost = estimate_actual_cost(*ctx.blueprint, InFlow(filter_hit_ratio)); return {timer.min_time() * 1000.0, seeks, hits, flow, actual_cost, get_class_name(*ctx.iterator), factory.get_name(*ctx.blueprint)}; } diff --git a/searchlib/src/tests/tensor/distance_calculator/distance_calculator_test.cpp b/searchlib/src/tests/tensor/distance_calculator/distance_calculator_test.cpp index 4ffc1fe366e..136878f0ea5 100644 --- a/searchlib/src/tests/tensor/distance_calculator/distance_calculator_test.cpp +++ b/searchlib/src/tests/tensor/distance_calculator/distance_calculator_test.cpp @@ -9,7 +9,6 @@ #include <vespa/searchlib/test/attribute_builder.h> #include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/util/exceptions.h> -#include <iostream> using namespace search::attribute::test; using namespace search::attribute; diff --git a/searchlib/src/tests/tensor/distance_functions/CMakeLists.txt b/searchlib/src/tests/tensor/distance_functions/CMakeLists.txt index e1a54f7883a..92ad9ae2648 100644 --- a/searchlib/src/tests/tensor/distance_functions/CMakeLists.txt +++ b/searchlib/src/tests/tensor/distance_functions/CMakeLists.txt @@ -7,3 +7,10 @@ vespa_add_executable(searchlib_distance_functions_test_app TEST GTest::GTest ) vespa_add_test(NAME searchlib_distance_functions_test_app COMMAND searchlib_distance_functions_test_app) + +vespa_add_executable(searchlib_distance_functions_benchmark_app TEST + SOURCES + distance_functions_benchmark.cpp + DEPENDS + searchlib +) diff --git a/searchlib/src/tests/tensor/distance_functions/distance_functions_benchmark.cpp b/searchlib/src/tests/tensor/distance_functions/distance_functions_benchmark.cpp new file mode 100644 index 00000000000..15d6040a11a --- /dev/null +++ b/searchlib/src/tests/tensor/distance_functions/distance_functions_benchmark.cpp @@ -0,0 +1,129 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/eval/eval/typed_cells.h> +#include <vespa/searchlib/common/geo_gcd.h> +#include <vespa/searchlib/tensor/distance_functions.h> +#include <vespa/searchlib/tensor/distance_function_factory.h> +#include <vespa/searchlib/tensor/mips_distance_transform.h> +#include <vespa/vespalib/util/benchmark_timer.h> +#include <vespa/vespalib/util/classname.h> + +using namespace search::tensor; +using vespalib::eval::Int8Float; +using vespalib::BFloat16; +using vespalib::eval::TypedCells; +using search::attribute::DistanceMetric; + +size_t npos = std::string::npos; + +double run_calc(size_t iterations, TypedCells b, const BoundDistanceFunction & df) __attribute_noinline__; +double run_calc_with_limit(size_t iterations, TypedCells b, const BoundDistanceFunction & df) __attribute_noinline__; + +double +run_calc(size_t iterations, TypedCells b, const BoundDistanceFunction & df) { + vespalib::BenchmarkTimer timer(1.0); + double min_result = std::numeric_limits<double>::max(); + while (timer.has_budget()) { + timer.before(); + for (size_t i(0); i < iterations; i++) { + min_result = std::min(df.calc(b), min_result); + } + timer.after(); + } + printf("%s::calc: Time used = %1.3f, min_result=%3.3f\n", + vespalib::getClassName(df).c_str(), timer.min_time(), min_result); + return min_result; +} + +double +run_calc_with_limit(size_t iterations, TypedCells b, const BoundDistanceFunction & df) { + vespalib::BenchmarkTimer timer(1.0); + double min_result = std::numeric_limits<double>::max(); + while (timer.has_budget()) { + timer.before(); + for (size_t i(0); i < iterations; i++) { + min_result = std::min(df.calc_with_limit(b, std::numeric_limits<double>::max()), min_result); + } + timer.after(); + } + + printf("%s::calc_with_limit: Time used = %1.3f, min_result=%3.3f\n", + vespalib::getClassName(df).c_str(), timer.min_time(), min_result); + return min_result; +} + +template<typename T> +void benchmark(size_t iterations, size_t elems) __attribute_noinline__; + +template<typename T> +void benchmark(size_t iterations, size_t elems, const DistanceFunctionFactory & df) { + std::vector<T> av, bv; + srand(7); + av.reserve(elems); + bv.reserve(elems); + for (size_t i(0); i < elems; i++) { + av.push_back(rand()); + bv.push_back(rand()); + } + TypedCells a_cells(av), b_cells(bv); + + double calc_result = run_calc(iterations, b_cells, *df.for_query_vector(a_cells)); + double calc_with_limit_result = run_calc_with_limit(iterations, b_cells, *df.for_query_vector(a_cells)); + assert(calc_result == calc_with_limit_result); +} + +template<typename T> +void benchmark(size_t iterations, size_t elems, const std::string & dist_functions) { + if (dist_functions.find("euclid") != npos) { + benchmark<T>(iterations, elems, EuclideanDistanceFunctionFactory<T>()); + } + if (dist_functions.find("angular") != npos) { + if (std::is_same<T, double>() || std::is_same<T, float>()) { + benchmark<T>(iterations, elems, AngularDistanceFunctionFactory<T>()); + } + } + if (dist_functions.find("prenorm") != npos) { + if (std::is_same<T, double>() || std::is_same<T, float>()) { + benchmark<T>(iterations, elems, PrenormalizedAngularDistanceFunctionFactory<T>()); + } + } + if (dist_functions.find("mips") != npos) { + if (std::is_same<T, double>() || std::is_same<T, float>() || std::is_same<T, Int8Float>()) { + benchmark<T>(iterations, elems, MipsDistanceFunctionFactory<T>()); + } + } +} + +void +benchmark(size_t iterations, size_t elems, const std::string & dist_functions, const std::string & data_types) { + if (data_types.find("double") != npos) { + benchmark<double>(iterations, elems, dist_functions); + } + if (data_types.find("float32") != npos) { + benchmark<float>(iterations, elems, dist_functions); + } + if (data_types.find("bfloat16") != npos) { + benchmark<BFloat16>(iterations, elems, dist_functions); + } + if (data_types.find("float8") != npos) { + benchmark<Int8Float>(iterations, elems, dist_functions); + } +} + +int +main(int argc, char *argv[]) { + size_t num_iterations = 10000000; + size_t num_elems = 1024; + std::string dist_functions = "angular euclid prenorm mips"; + std::string data_types = "double float32 bfloat16 float8"; + if (argc > 1) { num_iterations = atol(argv[1]); } + if (argc > 2) { num_elems = atol(argv[2]); } + if (argc > 3) { dist_functions = argv[3]; } + if (argc > 4) { data_types = argv[4]; } + + printf("Benchmarking %ld iterations with vector length %ld with distance functions '%s' for data types '%s'\n", + num_iterations, num_elems, dist_functions.c_str(), data_types.c_str()); + benchmark(num_iterations, num_elems, dist_functions, data_types); + + return 0; +} diff --git a/searchlib/src/tests/tensor/hnsw_index/hnsw_index_test.cpp b/searchlib/src/tests/tensor/hnsw_index/hnsw_index_test.cpp index a1cf86c95cc..97b88bc787a 100644 --- a/searchlib/src/tests/tensor/hnsw_index/hnsw_index_test.cpp +++ b/searchlib/src/tests/tensor/hnsw_index/hnsw_index_test.cpp @@ -111,7 +111,7 @@ class MyBoundDistanceFunction : public BoundDistanceFunction { std::unique_ptr<BoundDistanceFunction> _real; public: - MyBoundDistanceFunction(std::unique_ptr<BoundDistanceFunction> real) + explicit MyBoundDistanceFunction(std::unique_ptr<BoundDistanceFunction> real) : _real(std::move(real)) { } @@ -147,19 +147,19 @@ class MyDistanceFunctionFactory : public DistanceFunctionFactory { std::unique_ptr<DistanceFunctionFactory> _real; public: - MyDistanceFunctionFactory(std::unique_ptr<DistanceFunctionFactory> real) + explicit MyDistanceFunctionFactory(std::unique_ptr<DistanceFunctionFactory> real) : _real(std::move(real)) { } ~MyDistanceFunctionFactory() override; - std::unique_ptr<BoundDistanceFunction> for_query_vector(TypedCells lhs) override { + std::unique_ptr<BoundDistanceFunction> for_query_vector(TypedCells lhs) const override { EXPECT_FALSE(lhs.non_existing_attribute_value()); return std::make_unique<MyBoundDistanceFunction>(_real->for_query_vector(lhs)); } - std::unique_ptr<BoundDistanceFunction> for_insertion_vector(TypedCells lhs) override { + std::unique_ptr<BoundDistanceFunction> for_insertion_vector(TypedCells lhs) const override { EXPECT_FALSE(lhs.non_existing_attribute_value()); return std::make_unique<MyBoundDistanceFunction>(_real->for_insertion_vector(lhs)); } diff --git a/searchlib/src/vespa/searchlib/fef/ranksetup.cpp b/searchlib/src/vespa/searchlib/fef/ranksetup.cpp index 25588cf3229..ba5abb35141 100644 --- a/searchlib/src/vespa/searchlib/fef/ranksetup.cpp +++ b/searchlib/src/vespa/searchlib/fef/ranksetup.cpp @@ -195,7 +195,7 @@ RankSetup::compile() _firstPhaseRankFeature = parser.featureName(); _first_phase_resolver->addSeed(_firstPhaseRankFeature); } else { - vespalib::string e = fmt("invalid feature name for initial rank: '%s'", _firstPhaseRankFeature.c_str()); + vespalib::string e = fmt("invalid feature name for first phase rank: '%s'", _firstPhaseRankFeature.c_str()); _warnings.emplace_back(e); _compileError = true; } @@ -206,7 +206,7 @@ RankSetup::compile() _secondPhaseRankFeature = parser.featureName(); _second_phase_resolver->addSeed(_secondPhaseRankFeature); } else { - vespalib::string e = fmt("invalid feature name for final rank: '%s'", _secondPhaseRankFeature.c_str()); + vespalib::string e = fmt("invalid feature name for second phase rank: '%s'", _secondPhaseRankFeature.c_str()); _warnings.emplace_back(e); _compileError = true; } diff --git a/searchlib/src/vespa/searchlib/fef/test/matchdatabuilder.cpp b/searchlib/src/vespa/searchlib/fef/test/matchdatabuilder.cpp index 3645496e4fb..41551ac1062 100644 --- a/searchlib/src/vespa/searchlib/fef/test/matchdatabuilder.cpp +++ b/searchlib/src/vespa/searchlib/fef/test/matchdatabuilder.cpp @@ -10,11 +10,11 @@ LOG_SETUP(".fef.matchdatabuilder"); namespace search::fef::test { -MatchDataBuilder::MatchDataBuilder(QueryEnvironment &queryEnv, MatchData &data) : - _queryEnv(queryEnv), - _data(data), - _index(), - _match() +MatchDataBuilder::MatchDataBuilder(QueryEnvironment &queryEnv, MatchData &data) + : _queryEnv(queryEnv), + _data(data), + _index(), + _match() { // reset all match data objects. for (TermFieldHandle handle = 0; handle < _data.getNumTermFields(); ++handle) { @@ -22,7 +22,7 @@ MatchDataBuilder::MatchDataBuilder(QueryEnvironment &queryEnv, MatchData &data) } } -MatchDataBuilder::~MatchDataBuilder() {} +MatchDataBuilder::~MatchDataBuilder() = default; TermFieldMatchData * MatchDataBuilder::getTermFieldMatchData(uint32_t termId, uint32_t fieldId) @@ -59,7 +59,7 @@ MatchDataBuilder::addElement(const vespalib::string &fieldName, int32_t weight, LOG(error, "Field '%s' does not exist.", fieldName.c_str()); return false; } - _index[info->id()].elements.push_back(MyElement(weight, length)); + _index[info->id()].elements.emplace_back(weight, length); return true; } @@ -77,8 +77,7 @@ MatchDataBuilder::addOccurence(const vespalib::string &fieldName, uint32_t termI } const ITermFieldData *tfd = _queryEnv.getTerm(termId)->lookupField(info->id()); if (tfd == nullptr) { - LOG(error, "Field '%s' is not searched by the given term.", - fieldName.c_str()); + LOG(error, "Field '%s' is not searched by the given term.", fieldName.c_str()); return false; } _match[termId][info->id()].insert(Position(pos, element)); @@ -99,14 +98,13 @@ MatchDataBuilder::setWeight(const vespalib::string &fieldName, uint32_t termId, } const ITermFieldData *tfd = _queryEnv.getTerm(termId)->lookupField(info->id()); if (tfd == nullptr) { - LOG(error, "Field '%s' is not searched by the given term.", - fieldName.c_str()); + LOG(error, "Field '%s' is not searched by the given term.", fieldName.c_str()); return false; } uint32_t eid = _index[info->id()].elements.size(); _match[termId][info->id()].clear(); _match[termId][info->id()].insert(Position(0, eid)); - _index[info->id()].elements.push_back(MyElement(weight, 1)); + _index[info->id()].elements.emplace_back(weight, 1); return true; } @@ -142,19 +140,13 @@ MatchDataBuilder::apply(uint32_t docId) // For each occurence of that term, in that field, do for (const auto& occ : field_elem.second) { // Append a term match position to the term match data. - match->appendPosition(TermFieldMatchDataPosition( - occ.eid, - occ.pos, - field.getWeight(occ.eid), - field.getLength(occ.eid))); - LOG(debug, - "Added occurence of term '%u' in field '%s'" - " at position '%u'.", + match->appendPosition(TermFieldMatchDataPosition(occ.eid, occ.pos, + field.getWeight(occ.eid), + field.getLength(occ.eid))); + LOG(debug, "Added occurence of term '%u' in field '%s' at position '%u'.", termId, name.c_str(), occ.pos); if (occ.pos >= field.getLength(occ.eid)) { - LOG(warning, - "Added occurence of term '%u' in field '%s'" - " at position '%u' >= fieldLen '%u'.", + LOG(warning, "Added occurence of term '%u' in field '%s' at position '%u' >= fieldLen '%u'.", termId, name.c_str(), occ.pos, field.getLength(occ.eid)); } } diff --git a/searchlib/src/vespa/searchlib/fef/test/matchdatabuilder.h b/searchlib/src/vespa/searchlib/fef/test/matchdatabuilder.h index 0e5025efd37..753e1596520 100644 --- a/searchlib/src/vespa/searchlib/fef/test/matchdatabuilder.h +++ b/searchlib/src/vespa/searchlib/fef/test/matchdatabuilder.h @@ -13,7 +13,7 @@ public: struct MyElement { int32_t weight; uint32_t length; - MyElement(int32_t w, uint32_t l) : weight(w), length(l) {} + MyElement(int32_t w, uint32_t l) noexcept : weight(w), length(l) {} }; struct MyField { uint32_t fieldLength; @@ -21,7 +21,7 @@ public: MyField() : fieldLength(0), elements() {} MyElement &getElement(uint32_t eid) { while (elements.size() <= eid) { - elements.push_back(MyElement(0, 0)); + elements.emplace_back(0, 0); } return elements[eid]; } @@ -68,6 +68,8 @@ public: * @param data The match data to build in. */ MatchDataBuilder(QueryEnvironment &queryEnv, MatchData &data); + MatchDataBuilder(const MatchDataBuilder &) = delete; + MatchDataBuilder & operator=(const MatchDataBuilder &) = delete; ~MatchDataBuilder(); /** @@ -133,10 +135,6 @@ public: bool apply(uint32_t docId); private: - MatchDataBuilder(const MatchDataBuilder &); // hide - MatchDataBuilder & operator=(const MatchDataBuilder &); // hide - -private: QueryEnvironment &_queryEnv; MatchData &_data; IndexData _index; diff --git a/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp b/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp index 2bc94073c92..49a0f0621d2 100644 --- a/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp +++ b/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp @@ -213,6 +213,17 @@ FieldIndex<interleaved_features>::getMemoryUsage() const } template <bool interleaved_features> +void +FieldIndex<interleaved_features>::commit() +{ + _remover.flush(); + freeze(); + assign_generation(); + incGeneration(); + reclaim_memory(); +} + +template <bool interleaved_features> queryeval::SearchIterator::UP FieldIndex<interleaved_features>::make_search_iterator(const vespalib::string& term, uint32_t field_id, @@ -248,7 +259,7 @@ public: : SimpleLeafBlueprint(field), _guard(), _field(field), - _posting_itr(posting_itr), + _posting_itr(std::move(posting_itr)), _feature_store(feature_store), _field_id(field_id), _query_term(query_term), @@ -302,7 +313,7 @@ FieldIndex<interleaved_features>::make_term_blueprint(const vespalib::string& te auto posting_itr = findFrozen(term); bool use_bit_vector = field.isFilter(); return std::make_unique<MemoryTermBlueprint<interleaved_features>> - (std::move(guard), posting_itr, getFeatureStore(), field, field_id, term, use_bit_vector); + (std::move(guard), std::move(posting_itr), getFeatureStore(), field, field_id, term, use_bit_vector); } template class FieldIndex<false>; diff --git a/searchlib/src/vespa/searchlib/memoryindex/field_index.h b/searchlib/src/vespa/searchlib/memoryindex/field_index.h index 0b245300a7b..18e60cf2194 100644 --- a/searchlib/src/vespa/searchlib/memoryindex/field_index.h +++ b/searchlib/src/vespa/searchlib/memoryindex/field_index.h @@ -87,13 +87,7 @@ public: vespalib::MemoryUsage getMemoryUsage() const override; PostingListStore &getPostingListStore() { return _postingListStore; } - void commit() override { - _remover.flush(); - freeze(); - assign_generation(); - incGeneration(); - reclaim_memory(); - } + void commit() override; /** * Should only by used by unit tests. diff --git a/searchlib/src/vespa/searchlib/queryeval/blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/blueprint.cpp index cfa165be067..412a5973ad8 100644 --- a/searchlib/src/vespa/searchlib/queryeval/blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/blueprint.cpp @@ -169,31 +169,6 @@ Blueprint::null_plan(InFlow in_flow, uint32_t docid_limit) sort(in_flow); } -double -Blueprint::estimate_actual_cost(InFlow in_flow) const noexcept -{ - double res = estimate_strict_cost_diff(in_flow); - if (in_flow.strict()) { - res += strict_cost(); - } else { - res += in_flow.rate() * cost(); - } - return res; -} - -double -Blueprint::estimate_strict_cost_diff(InFlow &in_flow) const noexcept -{ - if (in_flow.strict()) { - REQUIRE(strict()); - } else if (strict()) { - double rate = in_flow.rate(); - in_flow.force_strict(); - return flow::strict_cost_diff(estimate(), rate); - } - return 0.0; -} - Blueprint::UP Blueprint::optimize(Blueprint::UP bp) { Blueprint *root = bp.release(); @@ -624,24 +599,6 @@ IntermediateBlueprint::should_do_termwise_eval(const UnpackInfo &unpack, double return (count_termwise_nodes(unpack) > 1); } -double -IntermediateBlueprint::estimate_self_cost(InFlow) const noexcept -{ - return 0.0; -} - -double -IntermediateBlueprint::estimate_actual_cost(InFlow in_flow) const noexcept -{ - double res = estimate_strict_cost_diff(in_flow); - auto cost_of = [](const auto &child, InFlow child_flow)noexcept{ - return child->estimate_actual_cost(child_flow); - }; - res += flow::actual_cost_of(flow::DefaultAdapter(), _children, my_flow(in_flow), cost_of); - res += estimate_self_cost(in_flow); - return res; -} - void IntermediateBlueprint::optimize(Blueprint* &self, OptimizePass pass) { diff --git a/searchlib/src/vespa/searchlib/queryeval/blueprint.h b/searchlib/src/vespa/searchlib/queryeval/blueprint.h index a493c725407..a443f34f856 100644 --- a/searchlib/src/vespa/searchlib/queryeval/blueprint.h +++ b/searchlib/src/vespa/searchlib/queryeval/blueprint.h @@ -313,20 +313,6 @@ public: // optimal ordering. Used for testing. void null_plan(InFlow in_flow, uint32_t docid_limit); - // Estimate the actual cost of evaluating the (sub-)query - // represented by this blueprint with the given in-flow. This - // function should be called after query planning has been - // performed. This function could be useful to predict very - // expensive queries, but the initial use-case is to understand - // query cost better in micro-benchmarks to improve low-level cost - // tuning. - virtual double estimate_actual_cost(InFlow in_flow) const noexcept; - // Estimate the change in cost caused by having a strict iterator - // with a non-strict in-flow. Note that this function might force - // the in_flow to be strict in order to align it with the - // strictness of this blueprint. - double estimate_strict_cost_diff(InFlow &in_flow) const noexcept; - static Blueprint::UP optimize(Blueprint::UP bp); virtual void sort(InFlow in_flow) = 0; static Blueprint::UP optimize_and_sort(Blueprint::UP bp, InFlow in_flow, const Options &opts) { @@ -496,9 +482,6 @@ public: void setDocIdLimit(uint32_t limit) noexcept final; void each_node_post_order(const std::function<void(Blueprint&)> &f) override; - // additional cost not attributed to the children flow (heap merge/unpack/etc) - virtual double estimate_self_cost(InFlow in_flow) const noexcept; - double estimate_actual_cost(InFlow in_flow) const noexcept override; void optimize(Blueprint* &self, OptimizePass pass) final; void sort(InFlow in_flow) override; void set_global_filter(const GlobalFilter &global_filter, double estimated_hit_ratio) override; diff --git a/searchlib/src/vespa/searchlib/queryeval/flow.h b/searchlib/src/vespa/searchlib/queryeval/flow.h index be7b9031c00..b7841dc2017 100644 --- a/searchlib/src/vespa/searchlib/queryeval/flow.h +++ b/searchlib/src/vespa/searchlib/queryeval/flow.h @@ -204,16 +204,6 @@ double ordered_cost_of(ADAPTER adapter, const T &children, F flow, bool allow_fo return total_cost; } -static double actual_cost_of(auto adapter, const auto &children, auto flow, auto cost_of) noexcept { - double total_cost = 0.0; - for (const auto &child: children) { - double child_cost = cost_of(child, InFlow(flow.strict(), flow.flow())); - flow.update_cost(total_cost, child_cost); - flow.add(adapter.estimate(child)); - } - return total_cost; -} - auto select_strict_and_child(auto adapter, const auto &children, size_t first, double est, bool native_strict) { double cost = 0.0; size_t best_idx = first; diff --git a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp index b0b3b302e82..5b8fa79b8af 100644 --- a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp @@ -318,11 +318,6 @@ OrBlueprint::calculate_flow_stats(uint32_t) const { OrFlow::cost_of(get_children(), true) + flow::heap_cost(est, get_children().size())}; } -double -OrBlueprint::estimate_self_cost(InFlow in_flow) const noexcept { - return in_flow.strict() ? flow::heap_cost(estimate(), get_children().size()) : 0.0; -} - Blueprint::HitEstimate OrBlueprint::combine(const std::vector<HitEstimate> &data) const { @@ -436,11 +431,6 @@ WeakAndBlueprint::calculate_flow_stats(uint32_t docid_limit) const { OrFlow::cost_of(get_children(), true) + flow::heap_cost(est, get_children().size())}; } -double -WeakAndBlueprint::estimate_self_cost(InFlow in_flow) const noexcept { - return in_flow.strict() ? flow::heap_cost(estimate(), get_children().size()) : 0.0; -} - Blueprint::HitEstimate WeakAndBlueprint::combine(const std::vector<HitEstimate> &data) const { @@ -519,11 +509,6 @@ NearBlueprint::calculate_flow_stats(uint32_t) const { AndFlow::cost_of(get_children(), true) + childCnt() * est}; } -double -NearBlueprint::estimate_self_cost(InFlow) const noexcept { - return childCnt() * estimate(); -} - Blueprint::HitEstimate NearBlueprint::combine(const std::vector<HitEstimate> &data) const { @@ -589,11 +574,6 @@ ONearBlueprint::calculate_flow_stats(uint32_t) const { AndFlow::cost_of(get_children(), true) + childCnt() * est}; } -double -ONearBlueprint::estimate_self_cost(InFlow) const noexcept { - return childCnt() * estimate(); -} - Blueprint::HitEstimate ONearBlueprint::combine(const std::vector<HitEstimate> &data) const { @@ -762,16 +742,6 @@ SourceBlenderBlueprint::calculate_flow_stats(uint32_t) const { return {my_est, my_cost + 1.0, my_strict_cost + my_est}; } -double -SourceBlenderBlueprint::estimate_self_cost(InFlow in_flow) const noexcept -{ - if (in_flow.strict()) { - return estimate(); - } else { - return in_flow.rate(); - } -} - Blueprint::HitEstimate SourceBlenderBlueprint::combine(const std::vector<HitEstimate> &data) const { diff --git a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h index f7eeace3e8b..913370caae1 100644 --- a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h +++ b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h @@ -67,7 +67,6 @@ public: ~OrBlueprint() override; bool supports_termwise_children() const override { return true; } FlowStats calculate_flow_stats(uint32_t docid_limit) const final; - double estimate_self_cost(InFlow in_flow) const noexcept override; HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; void optimize_self(OptimizePass pass) override; @@ -96,7 +95,6 @@ private: AnyFlow my_flow(InFlow in_flow) const override; public: FlowStats calculate_flow_stats(uint32_t docid_limit) const final; - double estimate_self_cost(InFlow in_flow) const noexcept override; HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; Blueprint::UP get_replacement() override; @@ -129,7 +127,6 @@ private: AnyFlow my_flow(InFlow in_flow) const override; public: FlowStats calculate_flow_stats(uint32_t docid_limit) const final; - double estimate_self_cost(InFlow in_flow) const noexcept override; HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; void sort(Children &children, InFlow in_flow) const override; @@ -152,7 +149,6 @@ private: AnyFlow my_flow(InFlow in_flow) const override; public: FlowStats calculate_flow_stats(uint32_t docid_limit) const final; - double estimate_self_cost(InFlow in_flow) const noexcept override; HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; void sort(Children &children, InFlow in_flow) const override; @@ -201,7 +197,6 @@ public: explicit SourceBlenderBlueprint(const ISourceSelector &selector) noexcept; ~SourceBlenderBlueprint() override; FlowStats calculate_flow_stats(uint32_t docid_limit) const final; - double estimate_self_cost(InFlow in_flow) const noexcept override; HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; void sort(Children &children, InFlow in_flow) const override; diff --git a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.cpp b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.cpp index 2b25aa29747..c5435b557b0 100644 --- a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.cpp @@ -191,16 +191,14 @@ SimplePhraseSearch::doSeek(uint32_t doc_id) { void SimplePhraseSearch::doStrictSeek(uint32_t doc_id) { uint32_t next_candidate = doc_id; - while (getDocId() < doc_id || getDocId() == beginId()) { - getChildren()[0]->seek(next_candidate + 1); - next_candidate = getChildren()[0]->getDocId(); + auto &best_child = *getChildren()[_eval_order[0]]; + while (getDocId() < doc_id) { + best_child.seek(next_candidate + 1); + next_candidate = best_child.getDocId(); if (isAtEnd(next_candidate)) { setAtEnd(); return; } - // child must behave as strict. - assert(next_candidate > doc_id && next_candidate != beginId()); - phraseSeek(next_candidate); } } diff --git a/searchlib/src/vespa/searchlib/tensor/angular_distance.cpp b/searchlib/src/vespa/searchlib/tensor/angular_distance.cpp index af99260979d..ef40381c807 100644 --- a/searchlib/src/vespa/searchlib/tensor/angular_distance.cpp +++ b/searchlib/src/vespa/searchlib/tensor/angular_distance.cpp @@ -70,14 +70,14 @@ template class BoundAngularDistance<double>; template <typename FloatType> BoundDistanceFunction::UP -AngularDistanceFunctionFactory<FloatType>::for_query_vector(TypedCells lhs) { +AngularDistanceFunctionFactory<FloatType>::for_query_vector(TypedCells lhs) const { using DFT = BoundAngularDistance<FloatType>; return std::make_unique<DFT>(lhs); } template <typename FloatType> BoundDistanceFunction::UP -AngularDistanceFunctionFactory<FloatType>::for_insertion_vector(TypedCells lhs) { +AngularDistanceFunctionFactory<FloatType>::for_insertion_vector(TypedCells lhs) const { using DFT = BoundAngularDistance<FloatType>; return std::make_unique<DFT>(lhs); } diff --git a/searchlib/src/vespa/searchlib/tensor/angular_distance.h b/searchlib/src/vespa/searchlib/tensor/angular_distance.h index 5e0a060e060..aa51f58b3cd 100644 --- a/searchlib/src/vespa/searchlib/tensor/angular_distance.h +++ b/searchlib/src/vespa/searchlib/tensor/angular_distance.h @@ -15,8 +15,8 @@ template <typename FloatType> class AngularDistanceFunctionFactory : public DistanceFunctionFactory { public: AngularDistanceFunctionFactory() = default; - BoundDistanceFunction::UP for_query_vector(TypedCells lhs) override; - BoundDistanceFunction::UP for_insertion_vector(TypedCells lhs) override; + BoundDistanceFunction::UP for_query_vector(TypedCells lhs) const override; + BoundDistanceFunction::UP for_insertion_vector(TypedCells lhs) const override; }; } diff --git a/searchlib/src/vespa/searchlib/tensor/distance_function_factory.h b/searchlib/src/vespa/searchlib/tensor/distance_function_factory.h index 356366d6a77..3b0a0ac91fd 100644 --- a/searchlib/src/vespa/searchlib/tensor/distance_function_factory.h +++ b/searchlib/src/vespa/searchlib/tensor/distance_function_factory.h @@ -17,8 +17,8 @@ struct DistanceFunctionFactory { using TypedCells = vespalib::eval::TypedCells; DistanceFunctionFactory() noexcept = default; virtual ~DistanceFunctionFactory() = default; - virtual BoundDistanceFunction::UP for_query_vector(TypedCells lhs) = 0; - virtual BoundDistanceFunction::UP for_insertion_vector(TypedCells lhs) = 0; + virtual BoundDistanceFunction::UP for_query_vector(TypedCells lhs) const = 0; + virtual BoundDistanceFunction::UP for_insertion_vector(TypedCells lhs) const = 0; using UP = std::unique_ptr<DistanceFunctionFactory>; }; diff --git a/searchlib/src/vespa/searchlib/tensor/euclidean_distance.cpp b/searchlib/src/vespa/searchlib/tensor/euclidean_distance.cpp index 3ab3a1123eb..355110b2f90 100644 --- a/searchlib/src/vespa/searchlib/tensor/euclidean_distance.cpp +++ b/searchlib/src/vespa/searchlib/tensor/euclidean_distance.cpp @@ -44,16 +44,8 @@ public: double score = 1.0 / (1.0 + d); return score; } - double calc_with_limit(TypedCells rhs, double limit) const noexcept override { - vespalib::ConstArrayRef<AttributeCellType> rhs_vector = rhs.typify<AttributeCellType>(); - double sum = 0.0; - size_t sz = _lhs_vector.size(); - assert(sz == rhs_vector.size()); - for (size_t i = 0; i < sz && sum <= limit; ++i) { - double diff = _lhs_vector[i] - rhs_vector[i]; - sum += diff*diff; - } - return sum; + double calc_with_limit(TypedCells rhs, double) const noexcept override { + return calc(rhs); } }; @@ -64,14 +56,14 @@ template class BoundEuclideanDistance<double>; template <typename FloatType> BoundDistanceFunction::UP -EuclideanDistanceFunctionFactory<FloatType>::for_query_vector(TypedCells lhs) { +EuclideanDistanceFunctionFactory<FloatType>::for_query_vector(TypedCells lhs) const { using DFT = BoundEuclideanDistance<FloatType>; return std::make_unique<DFT>(lhs); } template <typename FloatType> BoundDistanceFunction::UP -EuclideanDistanceFunctionFactory<FloatType>::for_insertion_vector(TypedCells lhs) { +EuclideanDistanceFunctionFactory<FloatType>::for_insertion_vector(TypedCells lhs) const { using DFT = BoundEuclideanDistance<FloatType>; return std::make_unique<DFT>(lhs); } diff --git a/searchlib/src/vespa/searchlib/tensor/euclidean_distance.h b/searchlib/src/vespa/searchlib/tensor/euclidean_distance.h index 8c39a12bf86..78460c93307 100644 --- a/searchlib/src/vespa/searchlib/tensor/euclidean_distance.h +++ b/searchlib/src/vespa/searchlib/tensor/euclidean_distance.h @@ -15,8 +15,8 @@ template <typename FloatType> class EuclideanDistanceFunctionFactory : public DistanceFunctionFactory { public: EuclideanDistanceFunctionFactory() noexcept = default; - BoundDistanceFunction::UP for_query_vector(TypedCells lhs) override; - BoundDistanceFunction::UP for_insertion_vector(TypedCells lhs) override; + BoundDistanceFunction::UP for_query_vector(TypedCells lhs) const override; + BoundDistanceFunction::UP for_insertion_vector(TypedCells lhs) const override; }; } diff --git a/searchlib/src/vespa/searchlib/tensor/geo_degrees_distance.cpp b/searchlib/src/vespa/searchlib/tensor/geo_degrees_distance.cpp index f5484f40271..a8a48ae4116 100644 --- a/searchlib/src/vespa/searchlib/tensor/geo_degrees_distance.cpp +++ b/searchlib/src/vespa/searchlib/tensor/geo_degrees_distance.cpp @@ -82,12 +82,12 @@ public: }; BoundDistanceFunction::UP -GeoDistanceFunctionFactory::for_query_vector(TypedCells lhs) { +GeoDistanceFunctionFactory::for_query_vector(TypedCells lhs) const { return std::make_unique<BoundGeoDistance>(lhs); } BoundDistanceFunction::UP -GeoDistanceFunctionFactory::for_insertion_vector(TypedCells lhs) { +GeoDistanceFunctionFactory::for_insertion_vector(TypedCells lhs) const { return std::make_unique<BoundGeoDistance>(lhs); } diff --git a/searchlib/src/vespa/searchlib/tensor/geo_degrees_distance.h b/searchlib/src/vespa/searchlib/tensor/geo_degrees_distance.h index 1464898421b..a85e31e8ecc 100644 --- a/searchlib/src/vespa/searchlib/tensor/geo_degrees_distance.h +++ b/searchlib/src/vespa/searchlib/tensor/geo_degrees_distance.h @@ -14,8 +14,8 @@ namespace search::tensor { class GeoDistanceFunctionFactory : public DistanceFunctionFactory { public: GeoDistanceFunctionFactory() = default; - BoundDistanceFunction::UP for_query_vector(TypedCells lhs) override; - BoundDistanceFunction::UP for_insertion_vector(TypedCells lhs) override; + BoundDistanceFunction::UP for_query_vector(TypedCells lhs) const override; + BoundDistanceFunction::UP for_insertion_vector(TypedCells lhs) const override; }; } diff --git a/searchlib/src/vespa/searchlib/tensor/hamming_distance.cpp b/searchlib/src/vespa/searchlib/tensor/hamming_distance.cpp index 7f29a100492..7ea2e440a51 100644 --- a/searchlib/src/vespa/searchlib/tensor/hamming_distance.cpp +++ b/searchlib/src/vespa/searchlib/tensor/hamming_distance.cpp @@ -49,14 +49,14 @@ public: template <typename FloatType> BoundDistanceFunction::UP -HammingDistanceFunctionFactory<FloatType>::for_query_vector(TypedCells lhs) { +HammingDistanceFunctionFactory<FloatType>::for_query_vector(TypedCells lhs) const { using DFT = BoundHammingDistance<FloatType>; return std::make_unique<DFT>(lhs); } template <typename FloatType> BoundDistanceFunction::UP -HammingDistanceFunctionFactory<FloatType>::for_insertion_vector(TypedCells lhs) { +HammingDistanceFunctionFactory<FloatType>::for_insertion_vector(TypedCells lhs) const { using DFT = BoundHammingDistance<FloatType>; return std::make_unique<DFT>(lhs); } diff --git a/searchlib/src/vespa/searchlib/tensor/hamming_distance.h b/searchlib/src/vespa/searchlib/tensor/hamming_distance.h index 6e7f96e1e2f..2e3b75cc61f 100644 --- a/searchlib/src/vespa/searchlib/tensor/hamming_distance.h +++ b/searchlib/src/vespa/searchlib/tensor/hamming_distance.h @@ -16,8 +16,8 @@ template <typename FloatType> class HammingDistanceFunctionFactory : public DistanceFunctionFactory { public: HammingDistanceFunctionFactory() = default; - BoundDistanceFunction::UP for_query_vector(TypedCells lhs) override; - BoundDistanceFunction::UP for_insertion_vector(TypedCells lhs) override; + BoundDistanceFunction::UP for_query_vector(TypedCells lhs) const override; + BoundDistanceFunction::UP for_insertion_vector(TypedCells lhs) const override; }; } diff --git a/searchlib/src/vespa/searchlib/tensor/mips_distance_transform.cpp b/searchlib/src/vespa/searchlib/tensor/mips_distance_transform.cpp index c42242d8dc8..fa47187fec9 100644 --- a/searchlib/src/vespa/searchlib/tensor/mips_distance_transform.cpp +++ b/searchlib/src/vespa/searchlib/tensor/mips_distance_transform.cpp @@ -76,13 +76,13 @@ public: template<typename FloatType> BoundDistanceFunction::UP -MipsDistanceFunctionFactory<FloatType>::for_query_vector(TypedCells lhs) { +MipsDistanceFunctionFactory<FloatType>::for_query_vector(TypedCells lhs) const { return std::make_unique<BoundMipsDistanceFunction<FloatType, false>>(lhs, *_sq_norm_store); } template<typename FloatType> BoundDistanceFunction::UP -MipsDistanceFunctionFactory<FloatType>::for_insertion_vector(TypedCells lhs) { +MipsDistanceFunctionFactory<FloatType>::for_insertion_vector(TypedCells lhs) const { return std::make_unique<BoundMipsDistanceFunction<FloatType, true>>(lhs, *_sq_norm_store); }; diff --git a/searchlib/src/vespa/searchlib/tensor/mips_distance_transform.h b/searchlib/src/vespa/searchlib/tensor/mips_distance_transform.h index 67a6eb58de0..336511ab78f 100644 --- a/searchlib/src/vespa/searchlib/tensor/mips_distance_transform.h +++ b/searchlib/src/vespa/searchlib/tensor/mips_distance_transform.h @@ -62,8 +62,8 @@ public: MipsDistanceFunctionFactory() noexcept = default; ~MipsDistanceFunctionFactory() override = default; - BoundDistanceFunction::UP for_query_vector(TypedCells lhs) override; - BoundDistanceFunction::UP for_insertion_vector(TypedCells lhs) override; + BoundDistanceFunction::UP for_query_vector(TypedCells lhs) const override; + BoundDistanceFunction::UP for_insertion_vector(TypedCells lhs) const override; }; } diff --git a/searchlib/src/vespa/searchlib/tensor/prenormalized_angular_distance.cpp b/searchlib/src/vespa/searchlib/tensor/prenormalized_angular_distance.cpp index 4bc90001227..58e92cbe2d4 100644 --- a/searchlib/src/vespa/searchlib/tensor/prenormalized_angular_distance.cpp +++ b/searchlib/src/vespa/searchlib/tensor/prenormalized_angular_distance.cpp @@ -62,14 +62,14 @@ template class BoundPrenormalizedAngularDistance<double>; template <typename FloatType> BoundDistanceFunction::UP -PrenormalizedAngularDistanceFunctionFactory<FloatType>::for_query_vector(TypedCells lhs) { +PrenormalizedAngularDistanceFunctionFactory<FloatType>::for_query_vector(TypedCells lhs) const { using DFT = BoundPrenormalizedAngularDistance<FloatType>; return std::make_unique<DFT>(lhs); } template <typename FloatType> BoundDistanceFunction::UP -PrenormalizedAngularDistanceFunctionFactory<FloatType>::for_insertion_vector(TypedCells lhs) { +PrenormalizedAngularDistanceFunctionFactory<FloatType>::for_insertion_vector(TypedCells lhs) const { using DFT = BoundPrenormalizedAngularDistance<FloatType>; return std::make_unique<DFT>(lhs); } diff --git a/searchlib/src/vespa/searchlib/tensor/prenormalized_angular_distance.h b/searchlib/src/vespa/searchlib/tensor/prenormalized_angular_distance.h index 7e3a8c2c676..6a791e0b6ec 100644 --- a/searchlib/src/vespa/searchlib/tensor/prenormalized_angular_distance.h +++ b/searchlib/src/vespa/searchlib/tensor/prenormalized_angular_distance.h @@ -14,8 +14,8 @@ template <typename FloatType> class PrenormalizedAngularDistanceFunctionFactory : public DistanceFunctionFactory { public: PrenormalizedAngularDistanceFunctionFactory() = default; - BoundDistanceFunction::UP for_query_vector(TypedCells lhs) override; - BoundDistanceFunction::UP for_insertion_vector(TypedCells lhs) override; + BoundDistanceFunction::UP for_query_vector(TypedCells lhs) const override; + BoundDistanceFunction::UP for_insertion_vector(TypedCells lhs) const override; }; } diff --git a/storage/src/tests/bucketdb/bucketmanagertest.cpp b/storage/src/tests/bucketdb/bucketmanagertest.cpp index 92547a83d25..b5501d38241 100644 --- a/storage/src/tests/bucketdb/bucketmanagertest.cpp +++ b/storage/src/tests/bucketdb/bucketmanagertest.cpp @@ -69,8 +69,7 @@ public: ~BucketManagerTest() override; - void setupTestEnvironment(bool fakePersistenceLayer = true, - bool noDelete = false); + void setupTestEnvironment(); void addBucketsToDB(uint32_t count); bool wasBlockedDueToLastModified(api::StorageMessage* msg, uint64_t lastModified); void insertSingleBucket(const document::BucketId& bucket, const api::BucketInfo& info); @@ -131,15 +130,9 @@ std::string getMkDirDisk(const std::string & rootFolder, int disk) { return os.str(); } -void BucketManagerTest::setupTestEnvironment(bool fakePersistenceLayer, bool noDelete) +void BucketManagerTest::setupTestEnvironment() { vdstestlib::DirConfig config(getStandardConfig(true, "bucketmanagertest")); - std::string rootFolder = getRootFolder(config); - if (!noDelete) { - assert(system(("rm -rf " + rootFolder).c_str()) == 0); - } - assert(system(getMkDirDisk(rootFolder, 0).c_str()) == 0); - assert(system(getMkDirDisk(rootFolder, 1).c_str()) == 0); auto repo = std::make_shared<const DocumentTypeRepo>( *ConfigGetter<DocumenttypesConfig>::getConfig("config-doctypes", FileSpec("../config-doctypes.cfg"))); @@ -153,18 +146,10 @@ void BucketManagerTest::setupTestEnvironment(bool fakePersistenceLayer, bool noD auto manager = std::make_unique<BucketManager>(*config_from<StorServerConfig>(config_uri), _node->getComponentRegister()); _manager = manager.get(); _top->push_back(std::move(manager)); - if (fakePersistenceLayer) { - auto bottom = std::make_unique<DummyStorageLink>(); - _bottom = bottom.get(); - _top->push_back(std::move(bottom)); - } else { - using StorFilestorConfig = vespa::config::content::internal::InternalStorFilestorType; - auto bottom = std::make_unique<FileStorManager>(*config_from<StorFilestorConfig>(config_uri), - _node->getPersistenceProvider(), _node->getComponentRegister(), - *_node, _node->get_host_info()); - _top->push_back(std::move(bottom)); - } - // Generate a doc to use for testing.. + auto bottom = std::make_unique<DummyStorageLink>(); + _bottom = bottom.get(); + _top->push_back(std::move(bottom)); + const DocumentType &type(*_node->getTypeRepo()->getDocumentType("text/html")); _document = std::make_shared<document::Document>(*_node->getTypeRepo(), type, document::DocumentId("id:ns:text/html::ntnu")); } diff --git a/storage/src/tests/common/metricstest.cpp b/storage/src/tests/common/metricstest.cpp index 899c1979e86..7216bef03db 100644 --- a/storage/src/tests/common/metricstest.cpp +++ b/storage/src/tests/common/metricstest.cpp @@ -67,16 +67,12 @@ MetricsTest::~MetricsTest() = default; void MetricsTest::SetUp() { _config = std::make_unique<vdstestlib::DirConfig>(getStandardConfig(true, "metricstest")); - std::filesystem::remove_all(std::filesystem::path(getRootFolder(*_config))); - try { - _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), _config->getConfigId()); - _node->setupDummyPersistence(); - _clock = &_node->getClock(); - _clock->setAbsoluteTimeInSeconds(1000000); - _top = std::make_unique<DummyStorageLink>(); - } catch (config::InvalidConfigException& e) { - fprintf(stderr, "%s\n", e.what()); - } + _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), _config->getConfigId()); + _node->setupDummyPersistence(); + _clock = &_node->getClock(); + _clock->setAbsoluteTimeInSeconds(1000000); + _top = std::make_unique<DummyStorageLink>(); + _metricManager = std::make_unique<metrics::MetricManager>(std::make_unique<MetricClock>(*_clock)); _topSet.reset(new metrics::MetricSet("vds", {}, "")); { diff --git a/storage/src/tests/common/testhelper.cpp b/storage/src/tests/common/testhelper.cpp index 4ca935b7904..91758b894b0 100644 --- a/storage/src/tests/common/testhelper.cpp +++ b/storage/src/tests/common/testhelper.cpp @@ -36,18 +36,6 @@ vdstestlib::DirConfig getStandardConfig(bool storagenode, const std::string & ro std::string clusterName("storage"); vdstestlib::DirConfig dc; vdstestlib::DirConfig::Config* config; - config = &dc.addConfig("fleetcontroller"); - config->set("cluster_name", clusterName); - config->set("index", "0"); - config->set("zookeeper_server", "\"\""); - config->set("total_distributor_count", "10"); - config->set("total_storage_count", "10"); - config = &dc.addConfig("upgrading"); - config = &dc.addConfig("load-type"); - config = &dc.addConfig("bucket"); - config = &dc.addConfig("messagebus"); - config = &dc.addConfig("stor-prioritymapping"); - config = &dc.addConfig("stor-bucketdbupdater"); config = &dc.addConfig("metricsmanager"); config->set("consumer[2]"); config->set("consumer[0].name", "\"status\""); @@ -71,16 +59,8 @@ vdstestlib::DirConfig getStandardConfig(bool storagenode, const std::string & ro // Easier to see what goes wrong with only 1 thread per disk. config->set("num_threads", "1"); config->set("num_response_threads", "1"); - config->set("maximum_versions_of_single_document_stored", "0"); - config->set("keep_remove_time_period", "2000000000"); - config->set("revert_time_period", "2000000000"); - // Don't want test to call exit() - config->set("fail_disk_after_error_count", "0"); - config = &dc.addConfig("stor-bouncer"); config = &dc.addConfig("stor-server"); config->set("cluster_name", clusterName); - config->set("enable_dead_lock_detector", "false"); - config->set("enable_dead_lock_detector_warnings", "false"); config->set("max_merges_per_node", "25"); config->set("max_merge_queue_size", "20"); config->set("resource_exhaustion_merge_back_pressure_duration_secs", "15.0"); @@ -88,6 +68,7 @@ vdstestlib::DirConfig getStandardConfig(bool storagenode, const std::string & ro rootFolder += (storagenode ? "vdsroot" : "vdsroot.distributor"); config->set("root_folder", rootFolder); config->set("is_distributor", (storagenode ? "false" : "true")); + config->set("write_pid_file_on_startup", "false"); config = &dc.addConfig("stor-devices"); config->set("root_folder", rootFolder); config = &dc.addConfig("stor-status"); diff --git a/storage/src/tests/common/teststorageapp.cpp b/storage/src/tests/common/teststorageapp.cpp index e2e2de10702..9e421051184 100644 --- a/storage/src/tests/common/teststorageapp.cpp +++ b/storage/src/tests/common/teststorageapp.cpp @@ -35,17 +35,16 @@ TestStorageApp::TestStorageApp(StorageComponentRegisterImpl::UP compReg, _node_identity("test_cluster", type, index), _initialized(false) { - // Use config to adjust values + // Use config to adjust values vespalib::string clusterName = "mycluster"; uint32_t redundancy = 2; uint32_t nodeCount = 10; if (!configId.empty()) { config::ConfigUri uri(configId); - std::unique_ptr<vespa::config::content::core::StorServerConfig> serverConfig = config::ConfigGetter<vespa::config::content::core::StorServerConfig>::getConfig(uri.getConfigId(), uri.getContext()); + auto serverConfig = config::ConfigGetter<vespa::config::content::core::StorServerConfig>::getConfig(uri.getConfigId(), uri.getContext()); clusterName = serverConfig->clusterName; if (index == 0xffff) index = serverConfig->nodeIndex; redundancy = config::ConfigGetter<vespa::config::content::StorDistributionConfig>::getConfig(uri.getConfigId(), uri.getContext())->redundancy; - nodeCount = config::ConfigGetter<vespa::config::content::FleetcontrollerConfig>::getConfig(uri.getConfigId(), uri.getContext())->totalStorageCount; } else { if (index == 0xffff) index = 0; } diff --git a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp index f12b85eb2ea..2680eacf49c 100644 --- a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp +++ b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp @@ -93,8 +93,6 @@ struct FileStorTestBase : Test { enum {LONG_WAITTIME=60}; unique_ptr<TestServiceLayerApp> _node; std::unique_ptr<vdstestlib::DirConfig> config; - std::unique_ptr<vdstestlib::DirConfig> config2; - std::unique_ptr<vdstestlib::DirConfig> smallConfig; const int32_t _waitTime; const document::DocumentType* _testdoctype1; @@ -167,27 +165,8 @@ struct FileStorTestBase : Test { std::string rootOfRoot = "filestormanagertest"; config = std::make_unique<vdstestlib::DirConfig>(getStandardConfig(true, rootOfRoot)); - config2 = std::make_unique<vdstestlib::DirConfig>(*config); - config2->getConfig("stor-server").set("root_folder", rootOfRoot + "-vdsroot.2"); - config2->getConfig("stor-devices").set("root_folder", rootOfRoot + "-vdsroot.2"); - config2->getConfig("stor-server").set("node_index", "1"); - - smallConfig = std::make_unique<vdstestlib::DirConfig>(*config); - vdstestlib::DirConfig::Config& c(smallConfig->getConfig("stor-filestor", true)); - c.set("initial_index_read", "128"); - c.set("use_direct_io", "false"); - c.set("maximum_gap_to_read_through", "64"); - - assert(system(vespalib::make_string("rm -rf %s", getRootFolder(*config).c_str()).c_str()) == 0); - assert(system(vespalib::make_string("rm -rf %s", getRootFolder(*config2).c_str()).c_str()) == 0); - assert(system(vespalib::make_string("mkdir -p %s/disks/d0", getRootFolder(*config).c_str()).c_str()) == 0); - assert(system(vespalib::make_string("mkdir -p %s/disks/d0", getRootFolder(*config2).c_str()).c_str()) == 0); - try { - _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), config->getConfigId()); - _node->setupDummyPersistence(); - } catch (config::InvalidConfigException& e) { - fprintf(stderr, "%s\n", e.what()); - } + _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), config->getConfigId()); + _node->setupDummyPersistence(); _testdoctype1 = _node->getTypeRepo()->getDocumentType("testdoctype1"); } @@ -227,10 +206,10 @@ struct TestFileStorComponents { DummyStorageLink top; FileStorManager* manager; - explicit TestFileStorComponents(FileStorTestBase& test, bool use_small_config = false) + explicit TestFileStorComponents(FileStorTestBase& test) : manager(nullptr) { - auto config_uri = config::ConfigUri((use_small_config ? test.smallConfig : test.config)->getConfigId()); + auto config_uri = config::ConfigUri(test.config->getConfigId()); auto config = config_from<StorFilestorConfig>(config_uri); auto fsm = std::make_unique<FileStorManager>(*config, test._node->getPersistenceProvider(), test._node->getComponentRegister(), *test._node, test._node->get_host_info()); @@ -1255,7 +1234,7 @@ createIterator(DummyStorageLink& link, } TEST_F(FileStorManagerTest, visiting) { - TestFileStorComponents c(*this, true); + TestFileStorComponents c(*this); auto& top = c.top; // Adding documents to two buckets which we are going to visit // We want one bucket in one slotfile, and one bucket with a file split diff --git a/storage/src/tests/persistence/mergehandlertest.cpp b/storage/src/tests/persistence/mergehandlertest.cpp index 4bd0570efa8..e865c87e15e 100644 --- a/storage/src/tests/persistence/mergehandlertest.cpp +++ b/storage/src/tests/persistence/mergehandlertest.cpp @@ -220,11 +220,11 @@ void MergeHandlerTest::setUpChain(ChainPos pos) { _nodes.clear(); if (pos != FRONT) { - _nodes.push_back(api::MergeBucketCommand::Node(2, false)); + _nodes.emplace_back(2, false); } - _nodes.push_back(api::MergeBucketCommand::Node(0, false)); + _nodes.emplace_back(0, false); if (pos != BACK) { - _nodes.push_back(api::MergeBucketCommand::Node(1, false)); + _nodes.emplace_back(1, false); } } diff --git a/storage/src/tests/persistence/persistencetestutils.cpp b/storage/src/tests/persistence/persistencetestutils.cpp index a599b1d380a..4a2ee987362 100644 --- a/storage/src/tests/persistence/persistencetestutils.cpp +++ b/storage/src/tests/persistence/persistencetestutils.cpp @@ -26,11 +26,7 @@ namespace storage { namespace { vdstestlib::DirConfig initialize(const std::string & rootOfRoot) { - vdstestlib::DirConfig config(getStandardConfig(true, rootOfRoot)); - std::string rootFolder = getRootFolder(config); - std::filesystem::remove_all(std::filesystem::path(rootFolder)); - std::filesystem::create_directories(std::filesystem::path(vespalib::make_string("%s/disks/d0", rootFolder.c_str()))); - return config; + return getStandardConfig(true, rootOfRoot); } template<typename T> diff --git a/storage/src/tests/storageserver/statereportertest.cpp b/storage/src/tests/storageserver/statereportertest.cpp index 43eb37afe15..c233c6e9314 100644 --- a/storage/src/tests/storageserver/statereportertest.cpp +++ b/storage/src/tests/storageserver/statereportertest.cpp @@ -69,7 +69,6 @@ StateReporterTest::~StateReporterTest() = default; void StateReporterTest::SetUp() { _config = std::make_unique<vdstestlib::DirConfig>(getStandardConfig(true, "statereportertest")); - assert(system(("rm -rf " + getRootFolder(*_config)).c_str()) == 0); _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), _config->getConfigId()); _node->setupDummyPersistence(); diff --git a/storage/src/tests/visiting/visitortest.cpp b/storage/src/tests/visiting/visitortest.cpp index f83b6c99d64..29a6c1baeb4 100644 --- a/storage/src/tests/visiting/visitortest.cpp +++ b/storage/src/tests/visiting/visitortest.cpp @@ -155,13 +155,6 @@ VisitorTest::initializeTest(const TestParams& params) "visitor_memory_usage_limit", std::to_string(params._maxVisitorMemoryUsage)); - std::string rootFolder = getRootFolder(config); - - ::chmod(rootFolder.c_str(), 0755); - std::filesystem::remove_all(std::filesystem::path(rootFolder)); - std::filesystem::create_directories(std::filesystem::path(vespalib::make_string("%s/disks/d0", rootFolder.c_str()))); - std::filesystem::create_directories(std::filesystem::path(vespalib::make_string("%s/disks/d1", rootFolder.c_str()))); - _messageSessionFactory = std::make_unique<TestVisitorMessageSessionFactory>(); if (params._autoReplyError.getCode() != mbus::ErrorCode::NONE) { _messageSessionFactory->_autoReplyError = params._autoReplyError; @@ -217,14 +210,12 @@ VisitorTest::initializeTest(const TestParams& params) _documents.clear(); for (uint32_t i=0; i<docCount; ++i) { std::ostringstream uri; - uri << "id:test:testdoctype1:n=" << i % 10 << ":http://www.ntnu.no/" - << i << ".html"; + uri << "id:test:testdoctype1:n=" << i % 10 << ":http://www.ntnu.no/" << i << ".html"; _documents.push_back(Document::SP( _node->getTestDocMan().createDocument(content, uri.str()))); const document::DocumentType& type(_documents.back()->getType()); - _documents.back()->setValue(type.getField("headerval"), - document::IntFieldValue(i % 4)); + _documents.back()->setValue(type.getField("headerval"), document::IntFieldValue(i % 4)); } } diff --git a/storage/src/vespa/storage/config/stor-server.def b/storage/src/vespa/storage/config/stor-server.def index 8cd204bcf9f..44e4b14eafc 100644 --- a/storage/src/vespa/storage/config/stor-server.def +++ b/storage/src/vespa/storage/config/stor-server.def @@ -114,3 +114,7 @@ simulated_bucket_request_latency_msec int default=0 ## a disjoint subset of the node's buckets, in order to reduce locking contention. ## Max value is unspecified, but will be clamped internally. content_node_bucket_db_stripe_bits int default=4 restart + +## Iff set, a special `pidfile` file is written under the node's root directory upon +## startup containing the PID of the running process. +write_pid_file_on_startup bool default=true diff --git a/storage/src/vespa/storage/storageserver/storagenode.cpp b/storage/src/vespa/storage/storageserver/storagenode.cpp index f7a426a0527..35b70dd853c 100644 --- a/storage/src/vespa/storage/storageserver/storagenode.cpp +++ b/storage/src/vespa/storage/storageserver/storagenode.cpp @@ -140,7 +140,7 @@ StorageNode::initialize(const NodeStateReporter & nodeStateReporter) // Initializing state manager early, as others use it init time to // update node state according min used bits etc. // Needs node type to be set right away. Needs thread pool, index and - // dead lock detector too, but not before open() + // deadlock detector too, but not before open() _stateManager = std::make_unique<StateManager>( _context.getComponentRegister(), std::move(_hostInfo), @@ -148,10 +148,10 @@ StorageNode::initialize(const NodeStateReporter & nodeStateReporter) _singleThreadedDebugMode); _context.getComponentRegister().setNodeStateUpdater(*_stateManager); - // Create VDS root folder, in case it doesn't already exist. - // Maybe better to rather fail if it doesn't exist, but tests - // might break if we do that. Might alter later. - std::filesystem::create_directories(std::filesystem::path(_rootFolder)); + // Create storage root folder, in case it doesn't already exist. + if (!_rootFolder.empty()) { + std::filesystem::create_directories(std::filesystem::path(_rootFolder)); + } // else: running as part of unit tests initializeNodeSpecific(); @@ -192,13 +192,16 @@ StorageNode::initialize(const NodeStateReporter & nodeStateReporter) initializeStatusWebServer(); + if (server_config().writePidFileOnStartup) { + assert(!_rootFolder.empty()); // Write pid file as the last thing we do. If we fail initialization // due to an exception we won't run shutdown. Thus we won't remove the // pid file if something throws after writing it in initialization. // Initialize _pidfile here, such that we can know that we didn't create // it in shutdown code for shutdown during init. - _pidFile = _rootFolder + "/pidfile"; - writePidFile(_pidFile); + _pidFile = _rootFolder + "/pidfile"; + writePidFile(_pidFile); + } } void diff --git a/storageserver/src/tests/testhelper.cpp b/storageserver/src/tests/testhelper.cpp index 40e263d4e68..e3b047ddfd1 100644 --- a/storageserver/src/tests/testhelper.cpp +++ b/storageserver/src/tests/testhelper.cpp @@ -62,13 +62,11 @@ vdstestlib::DirConfig getStandardConfig(bool storagenode) { config->set("enable_dead_lock_detector_warnings", "false"); config->set("max_merges_per_node", "25"); config->set("max_merge_queue_size", "20"); - config->set("root_folder", - (storagenode ? "vdsroot" : "vdsroot.distributor")); - config->set("is_distributor", - (storagenode ? "false" : "true")); + config->set("root_folder", (storagenode ? "vdsroot" : "vdsroot.distributor")); + config->set("is_distributor", (storagenode ? "false" : "true")); + config->set("write_pid_file_on_startup", "false"); config = &dc.addConfig("stor-devices"); - config->set("root_folder", - (storagenode ? "vdsroot" : "vdsroot.distributor")); + config->set("root_folder", (storagenode ? "vdsroot" : "vdsroot.distributor")); config = &dc.addConfig("stor-status"); config->set("httpport", "0"); config = &dc.addConfig("stor-visitor"); diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/bindings/AccessTokenResponseEntity.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/bindings/AccessTokenResponseEntity.java index a3063524b93..785c215df18 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/bindings/AccessTokenResponseEntity.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/bindings/AccessTokenResponseEntity.java @@ -9,6 +9,7 @@ import com.yahoo.vespa.athenz.api.AthenzRole; import java.time.Instant; import java.util.List; +import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -19,6 +20,7 @@ import java.util.stream.Stream; public class AccessTokenResponseEntity { private final AthenzAccessToken accessToken; private final Instant expiryTime; + // roles can be null (not set in the json response) private final List<AthenzRole> roles; public AccessTokenResponseEntity( @@ -29,7 +31,8 @@ public class AccessTokenResponseEntity { this.accessToken = new AthenzAccessToken(accessToken); // We do not know from when, so best we can do is assume now ... this.expiryTime = Instant.now().plusSeconds(expiresIn); - this.roles = Stream.of(roles.split(" ")) + this.roles = Optional.ofNullable(roles).stream() + .flatMap(r -> Stream.of(r.split(" "))) .map(AthenzResourceName::fromString) .map(AthenzRole::fromResourceName) .toList(); diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/DynamicThrottler.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/DynamicThrottler.java index 951a1776b6f..567788b8501 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/DynamicThrottler.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/DynamicThrottler.java @@ -28,12 +28,12 @@ public class DynamicThrottler extends StaticThrottler { public DynamicThrottler(FeedClientBuilderImpl builder) { super(builder); - targetInflight = new AtomicLong(8 * minInflight); + targetInflight = new AtomicLong(minInflight); } @Override public void sent(long __, CompletableFuture<HttpResponse> ___) { - double currentInflight = targetInflight.get(); + double currentInflight = targetInflight(); if (++sent * sent * sent < 1e2 * currentInflight * currentInflight) return; @@ -43,22 +43,36 @@ public class DynamicThrottler extends StaticThrottler { // Use buckets for throughput over inflight, along the log-scale, in [minInflight, maxInflight). int index = (int) (throughputs.length * log(max(1, min(255, currentInflight / minInflight))) - / log(256)); // 4096 (server max streams per connection) / 16 (our min per connection) + / log(256)); // 512 (server max streams per connection) / 2 (our min per connection) throughputs[index] = currentThroughput; // Loop over throughput measurements and pick the one which optimises throughput and latency. - double choice = currentInflight; + double best = currentInflight; double max = -1; - for (int i = throughputs.length; i-- > 0; ) { + int j = -1, k = -1, choice = 0; + double s = 0; + for (int i = 0; i < throughputs.length; i++) { if (throughputs[i] == 0) continue; // Skip unknown values. double inflight = minInflight * pow(256, (i + 0.5) / throughputs.length); double objective = throughputs[i] * pow(inflight, (weight - 1)); // Optimise throughput (weight), but also latency (1 - weight). if (objective > max) { max = objective; - choice = inflight; + best = inflight; + choice = i; } + // Additionally, smooth the throughput values, to reduce the impact of noise, and reduce jumpiness. + if (j != -1) { + double t = throughputs[j]; + if (k != -1) throughputs[j] = (2 * t + throughputs[i] + s) / 4; + s = t; + } + k = j; + j = i; } - long target = (long) ((random() * 0.20 + 0.92) * choice); // Random walk, skewed towards increase. + long target = (long) ((random() * 0.40 + 0.84) * best + random() * 4 - 1); // Random step, skewed towards increase. + // If the best inflight is at the high end of the known, we override the random walk to speed up upwards exploration. + if (choice == j && choice + 1 < throughputs.length) + target = (long) (1 + minInflight * pow(256, (choice + 1.5) / throughputs.length)); targetInflight.set(max(minInflight, min(maxInflight, target))); } diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/StaticThrottler.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/StaticThrottler.java index 9010b0a7ad8..f0ee434e87c 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/StaticThrottler.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/StaticThrottler.java @@ -22,7 +22,7 @@ public class StaticThrottler implements Throttler { public StaticThrottler(FeedClientBuilderImpl builder) { minInflight = 2L * builder.connectionsPerEndpoint * builder.endpoints.size(); - maxInflight = 256 * minInflight; // 4096 max streams per connection on the server side. + maxInflight = 256 * minInflight; // 512 max streams per connection on the server side. targetX10 = new AtomicLong(10 * maxInflight); // 10x the actual value to allow for smaller updates. } diff --git a/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/DynamicThrottlerTest.java b/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/DynamicThrottlerTest.java new file mode 100644 index 00000000000..7e07fc6e116 --- /dev/null +++ b/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/DynamicThrottlerTest.java @@ -0,0 +1,30 @@ +package ai.vespa.feed.client.impl; + +import org.junit.jupiter.api.Test; + +import java.net.URI; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * @author jonmv + */ +class DynamicThrottlerTest { + + @Test + void testThrottler() { + DynamicThrottler throttler = new DynamicThrottler(new FeedClientBuilderImpl(List.of(URI.create("http://localhost:8080")))); + assertEquals(16, throttler.targetInflight()); + + for (int i = 0; i < 30; i++) { + throttler.sent(1, null); + throttler.success(); + } + assertEquals(18, throttler.targetInflight()); + + throttler.throttled(34); + assertEquals(17, throttler.targetInflight()); + } + +} diff --git a/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/HttpRequestStrategyTest.java b/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/HttpRequestStrategyTest.java index 54fab9b859b..b1a04ac9ed4 100644 --- a/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/HttpRequestStrategyTest.java +++ b/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/HttpRequestStrategyTest.java @@ -33,6 +33,7 @@ import static ai.vespa.feed.client.FeedClient.CircuitBreaker.State.OPEN; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -105,7 +106,7 @@ class HttpRequestStrategyTest { cluster.expect((__, vessel) -> vessel.completeExceptionally(new RuntimeException("boom"))); ExecutionException expected = assertThrows(ExecutionException.class, () -> strategy.enqueue(id1, request).get()); - assertTrue(expected.getCause() instanceof FeedException); + assertInstanceOf(FeedException.class, expected.getCause()); assertEquals("java.lang.RuntimeException: boom", expected.getCause().getMessage()); assertEquals(1, strategy.stats().requests()); @@ -200,7 +201,7 @@ class HttpRequestStrategyTest { @Override public int retries() { return 1; } }) .setCircuitBreaker(breaker) - .setConnectionsPerEndpoint(1), + .setConnectionsPerEndpoint(3), // Must be >= 0.5x text ops. cluster); DocumentId id1 = DocumentId.of("ns", "type", "1"); diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java index f1829a1c42e..6e07661235e 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java @@ -1397,7 +1397,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { Phaser phaser = new Phaser(2); // Synchronize this thread (dispatch) with the visitor callback thread. AtomicReference<String> error = new AtomicReference<>(); // Set if error occurs during processing of visited documents. callback.onStart(response, fullyApplied); - final AtomicLong receivedDocsCount = new AtomicLong(0); + final AtomicLong locallyReceivedDocCount = new AtomicLong(0); VisitorControlHandler controller = new VisitorControlHandler() { final ScheduledFuture<?> abort = streaming ? visitDispatcher.schedule(this::abort, visitTimeout(request), MILLISECONDS) : null; final AtomicReference<VisitorSession> session = new AtomicReference<>(); @@ -1411,7 +1411,10 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { try (response) { callback.onEnd(response); - response.writeDocumentCount(receivedDocsCount.get()); + // Locally tracked document count is only correct if we have a local data handler. + // Otherwise, we have to report the statistics received transitively from the content nodes. + long statsDocCount = (getVisitorStatistics() != null ? getVisitorStatistics().getDocumentsVisited() : 0); + response.writeDocumentCount(parameters.getLocalDataHandler() != null ? locallyReceivedDocCount.get() : statsDocCount); if (session.get() != null) response.writeTrace(session.get().getTrace()); @@ -1457,7 +1460,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { if (m instanceof PutDocumentMessage put) document = put.getDocumentPut().getDocument(); else if (parameters.visitRemoves() && m instanceof RemoveDocumentMessage remove) removeId = remove.getDocumentId(); else throw new UnsupportedOperationException("Got unsupported message type: " + m.getClass().getName()); - receivedDocsCount.getAndAdd(1); + locallyReceivedDocCount.getAndAdd(1); callback.onDocument(response, document, removeId, diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java index 3a8456d213d..b2c0b1b2ce8 100644 --- a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java +++ b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java @@ -441,13 +441,18 @@ public class DocumentV1ApiTest { assertEquals("[Content:cluster=content]", parameters.getRemoteDataHandler()); assertEquals("[document]", parameters.fieldSet()); assertEquals(60_000L, parameters.getSessionTimeoutMs()); + VisitorStatistics statistics = new VisitorStatistics(); + statistics.setBucketsVisited(1); + statistics.setDocumentsVisited(2); + // Visiting with remote data handlers should report the remotely aggregated statistics + parameters.getControlHandler().onVisitorStatistics(statistics); parameters.getControlHandler().onDone(VisitorControlHandler.CompletionCode.SUCCESS, "We made it!"); }); response = driver.sendRequest("http://localhost/document/v1/space/music/docid?destinationCluster=content&selection=true&cluster=content&timeout=60", POST); assertSameJson(""" { "pathId": "/document/v1/space/music/docid", - "documentCount": 0 + "documentCount": 2 }""", response.readAll()); assertEquals(200, response.getStatus()); diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java index 15bec762119..f3f0a0684a0 100644 --- a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java +++ b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java @@ -109,7 +109,7 @@ public abstract class Maintainer implements Runnable { /** Convenience methods to convert attempts and failures into a success factor deviation from the baseline, and return */ protected final double asSuccessFactorDeviation(int attempts, int failures) { double factor = attempts == 0 ? 1.0 : 1 - (double) failures / attempts; - return new BigDecimal(factor - successFactorBaseline).setScale(2, RoundingMode.HALF_UP).doubleValue(); + return new BigDecimal(factor - successFactorBaseline).setScale(5, RoundingMode.HALF_UP).doubleValue(); } /** Returns the interval at which this job is set to run */ diff --git a/vespalib/src/tests/util/hamming/hamming_benchmark.cpp b/vespalib/src/tests/util/hamming/hamming_benchmark.cpp index 347c935f5b7..2600ec217aa 100644 --- a/vespalib/src/tests/util/hamming/hamming_benchmark.cpp +++ b/vespalib/src/tests/util/hamming/hamming_benchmark.cpp @@ -4,7 +4,6 @@ #include <vector> #include <cinttypes> #include <cstdlib> -#include <cstdint> #include <cstdio> using namespace vespalib; |