diff options
44 files changed, 390 insertions, 130 deletions
diff --git a/client/go/internal/admin/jvm/mem_options.go b/client/go/internal/admin/jvm/mem_options.go index d3b0d44c677..c78ee80dc80 100644 --- a/client/go/internal/admin/jvm/mem_options.go +++ b/client/go/internal/admin/jvm/mem_options.go @@ -55,8 +55,8 @@ func (opts *Options) MaybeAddHugepages(heapSize AmountOfMemory) { } func adjustAvailableMemory(measured AmountOfMemory) AmountOfMemory { - reserved := 1024 // MB - need_min := 64 // MB + reserved := 700 // MB -- keep in sync with com.yahoo.vespa.model.Host.memoryOverheadGb + need_min := 64 // MB available := measured.ToMB() if available > need_min+2*reserved { return MegaBytesOfMemory(available - reserved) diff --git a/client/go/internal/admin/jvm/mem_options_test.go b/client/go/internal/admin/jvm/mem_options_test.go index 3501e44c723..3db10153086 100644 --- a/client/go/internal/admin/jvm/mem_options_test.go +++ b/client/go/internal/admin/jvm/mem_options_test.go @@ -14,6 +14,6 @@ func TestAdjustment(t *testing.T) { assert.True(t, int(adj) >= lastAdj) lastAdj = int(adj) } - adj := adjustAvailableMemory(MegaBytesOfMemory(31024)).ToMB() + adj := adjustAvailableMemory(MegaBytesOfMemory(30700)).ToMB() assert.Equal(t, 30000, int(adj)) } diff --git a/client/go/internal/admin/jvm/standalone_container.go b/client/go/internal/admin/jvm/standalone_container.go index 20031bc7725..fb615dcf4c2 100644 --- a/client/go/internal/admin/jvm/standalone_container.go +++ b/client/go/internal/admin/jvm/standalone_container.go @@ -37,7 +37,6 @@ func (a *StandaloneContainer) configureOptions() { opts := a.jvmOpts opts.ConfigureCpuCount(0) opts.AddCommonXX() - opts.AddOption("-XX:-OmitStackTraceInFastThrow") opts.AddCommonOpens() opts.AddCommonJdkProperties() a.addJdiscProperties() diff --git a/client/go/internal/admin/jvm/xx_options.go b/client/go/internal/admin/jvm/xx_options.go index 13b69e43dda..abc92f19bf2 100644 --- a/client/go/internal/admin/jvm/xx_options.go +++ b/client/go/internal/admin/jvm/xx_options.go @@ -19,5 +19,5 @@ func (opts *Options) AddCommonXX() { // not common after all: opts.AddOption("-XX:MaxJavaStackTraceDepth=1000000") // Aid debugging for slight cost in performance - opts.AddOption("-XX:+OmitStackTraceInFastThrow") + opts.AddOption("-XX:-OmitStackTraceInFastThrow") } diff --git a/config-model/src/main/java/com/yahoo/schema/Schema.java b/config-model/src/main/java/com/yahoo/schema/Schema.java index 3402ba31be9..127d12594b4 100644 --- a/config-model/src/main/java/com/yahoo/schema/Schema.java +++ b/config-model/src/main/java/com/yahoo/schema/Schema.java @@ -721,7 +721,7 @@ public class Schema implements ImmutableSchema { "', but this schema does not exist"); // Require schema and document type inheritance to be consistent to keep things simple - // And require it to be explicit so we have the option to support other possibilities later + // and require it to be explicit, so we have the option to support other possibilities later var parentDocument = owner.schemas().get(inherited.get()).getDocument(); if ( ! getDocument().inheritedTypes().containsKey(new DataTypeName(parentDocument.getName()))) throw new IllegalArgumentException(this + " inherits '" + inherited.get() + diff --git a/config-model/src/main/java/com/yahoo/vespa/documentmodel/DocumentSummary.java b/config-model/src/main/java/com/yahoo/vespa/documentmodel/DocumentSummary.java index 2f61c1c631d..e48aa0eef7c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/documentmodel/DocumentSummary.java +++ b/config-model/src/main/java/com/yahoo/vespa/documentmodel/DocumentSummary.java @@ -9,7 +9,8 @@ import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.logging.Level; + +import static java.util.logging.Level.WARNING; /** * A document summary definition - a list of summary fields. @@ -121,11 +122,11 @@ public class DocumentSummary extends FieldView { public void validate(DeployLogger logger) { for (var inheritedName : inherited) { var inheritedSummary = owner.getSummary(inheritedName); - if (inheritedSummary == null) { - // TODO Vespa 9: Throw IllegalArgumentException instead - logger.logApplicationPackage(Level.WARNING, - this + " inherits '" + inheritedName + "' but this is not present in " + owner); - } + // TODO: Throw when no one is doing this anymore + if (inheritedName.equals("default")) + logger.logApplicationPackage(WARNING, this + " inherits '" + inheritedName + "', which makes no sense. Remove this inheritance"); + else if (inheritedSummary == null ) + throw new IllegalArgumentException(this + " inherits '" + inheritedName + "', but this is not present in " + owner); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/Host.java b/config-model/src/main/java/com/yahoo/vespa/model/Host.java index a8085919a98..f87f1382ffb 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/Host.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/Host.java @@ -16,7 +16,7 @@ import java.util.Objects; public final class Host extends TreeConfigProducer<AnyConfigProducer> implements SentinelConfig.Producer, Comparable<Host> { // Memory needed for auxiliary processes always running on the node (config-proxy, metrics-proxy). - // Keep in sync with node-repository/ClusterModel. + // Keep in sync with node-repository/ClusterModel and startup scripts (go and bash). public static final double memoryOverheadGb = 0.7; private ConfigSentinel configSentinel = null; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidator.java index 9cf5fe84c21..4900b56801c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidator.java @@ -23,21 +23,22 @@ public class JvmHeapSizeValidator implements Validator { context.model().getContainerClusters().forEach((clusterId, appCluster) -> { var mp = appCluster.getMemoryPercentage().orElse(null); if (mp == null) return; - if (mp.availableMemoryGb().isEmpty()) { + if (mp.asAbsoluteGb().isEmpty()) { context.deployState().getDeployLogger().log(Level.FINE, "Host resources unknown or percentage overridden with 'allocated-memory'"); return; } long jvmModelCost = appCluster.onnxModelCostCalculator().aggregatedModelCostInBytes(); if (jvmModelCost > 0) { - double availableMemoryGb = mp.availableMemoryGb().getAsDouble(); + double availableMemoryGb = mp.asAbsoluteGb().getAsDouble(); + int percentageOfTotal = mp.ofContainerTotal().getAsInt(); double modelCostGb = jvmModelCost / (1024D * 1024 * 1024); context.deployState().getDeployLogger().log(Level.FINE, () -> Text.format("JVM: %d%% (limit: %d%%), %.2fGB (limit: %.2fGB), ONNX: %.2fGB", - mp.percentage(), percentLimit, availableMemoryGb, gbLimit, modelCostGb)); - if (mp.percentage() < percentLimit) { + percentageOfTotal, percentLimit, availableMemoryGb, gbLimit, modelCostGb)); + if (percentageOfTotal < percentLimit) { context.illegal(Text.format("Allocated percentage of memory of JVM in cluster '%s' is too low (%d%% < %d%%). " + "Estimated cost of ONNX models is %.2fGB. Either use a node flavor with more memory or use less expensive models. " + "You may override this validation by specifying 'allocated-memory' (https://docs.vespa.ai/en/performance/container-tuning.html#jvm-heap-size).", - clusterId, mp.percentage(), percentLimit, modelCostGb)); + clusterId, percentageOfTotal, percentLimit, modelCostGb)); } if (availableMemoryGb < gbLimit) { context.illegal( diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidator.java index 008a3fc5547..e57110e44e5 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidator.java @@ -115,7 +115,7 @@ public class RestartOnDeployForOnnxModelChangesValidator implements ChangeValida double memoryUsedByModels = currentModelCostInGb + nextModelCostInGb; double availableMemory = Math.max(0, totalMemory - Host.memoryOverheadGb - memoryUsedByModels); - var availableMemoryPercentage = cluster.availableMemoryPercentage(); + var availableMemoryPercentage = cluster.heapSizePercentageOfAvailable(); int memoryPercentage = (int) (availableMemory / totalMemory * availableMemoryPercentage); var prefix = "Validating Onnx models memory usage for %s".formatted(cluster); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java index ed7646b3066..531dc8f0fcf 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java @@ -209,22 +209,27 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat if (memoryPercentage != null) return Optional.of(JvmMemoryPercentage.of(memoryPercentage)); if (isHostedVespa()) { - int availableMemoryPercentage = availableMemoryPercentage(); - if (getContainers().isEmpty()) return Optional.of(JvmMemoryPercentage.of(availableMemoryPercentage)); // Node memory is not known - - // Node memory is known so convert available memory percentage to node memory percentage - double totalMemory = getContainers().stream().mapToDouble(c -> c.getHostResource().realResources().memoryGb()).min().orElseThrow(); - double jvmHeapDeductionGb = onnxModelCostCalculator.aggregatedModelCostInBytes() / (1024D * 1024 * 1024); - double availableMemory = Math.max(0, totalMemory - Host.memoryOverheadGb - jvmHeapDeductionGb); - int memoryPercentage = (int) (availableMemory / totalMemory * availableMemoryPercentage); - logger.log(FINE, () -> "cluster id '%s': memoryPercentage=%d, availableMemory=%f, totalMemory=%f, availableMemoryPercentage=%d, jvmHeapDeductionGb=%f" - .formatted(id(), memoryPercentage, availableMemory, totalMemory, availableMemoryPercentage, jvmHeapDeductionGb)); - return Optional.of(JvmMemoryPercentage.of(memoryPercentage, availableMemory)); + int heapSizePercentageOfAvailable = heapSizePercentageOfAvailable(); + if (getContainers().isEmpty()) return Optional.of(JvmMemoryPercentage.of(heapSizePercentageOfAvailable)); // Node memory is not known + + // Node memory is known, so compute heap size as a percentage of available memory (excluding overhead, which the startup scripts also account for) + double totalMemoryGb = getContainers().stream().mapToDouble(c -> c.getHostResource().realResources().memoryGb()).min().orElseThrow(); + double totalMemoryMinusOverhead = Math.max(0, totalMemoryGb - Host.memoryOverheadGb); + double onnxModelCostGb = onnxModelCostCalculator.aggregatedModelCostInBytes() / (1024D * 1024 * 1024); + double availableMemoryGb = Math.max(0, totalMemoryMinusOverhead - onnxModelCostGb); + int memoryPercentageOfAvailable = (int) (heapSizePercentageOfAvailable * availableMemoryGb / totalMemoryMinusOverhead); + int memoryPercentageOfTotal = (int) (heapSizePercentageOfAvailable * availableMemoryGb / totalMemoryGb); + logger.log(FINE, () -> ("cluster id '%s': memoryPercentageOfAvailable=%d, memoryPercentageOfTotal=%d, " + + "availableMemoryGb=%f, totalMemoryGb=%f, heapSizePercentageOfAvailable=%d, onnxModelCostGb=%f") + .formatted(id(), memoryPercentageOfAvailable, memoryPercentageOfTotal, + availableMemoryGb, totalMemoryGb, heapSizePercentageOfAvailable, onnxModelCostGb)); + return Optional.of(JvmMemoryPercentage.of(memoryPercentageOfAvailable, memoryPercentageOfTotal, + availableMemoryGb * heapSizePercentageOfAvailable * 1e-2)); } return Optional.empty(); } - public int availableMemoryPercentage() { + public int heapSizePercentageOfAvailable() { return getHostClusterId().isPresent() ? heapSizePercentageOfTotalAvailableMemoryWhenCombinedCluster : heapSizePercentageOfAvailableMemory; @@ -310,14 +315,14 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat public void getConfig(QrStartConfig.Builder builder) { super.getConfig(builder); var memoryPct = getMemoryPercentage().orElse(null); - int heapsize = memoryPct != null && memoryPct.availableMemoryGb().isPresent() - ? (int) (memoryPct.availableMemoryGb().getAsDouble() * 1024) : 1536; + int heapsize = memoryPct != null && memoryPct.asAbsoluteGb().isPresent() + ? (int) (memoryPct.asAbsoluteGb().getAsDouble() * 1024) : 1536; builder.jvm.verbosegc(true) .availableProcessors(0) .compressedClassSpaceSize(0) .minHeapsize(heapsize) .heapsize(heapsize); - if (memoryPct != null) builder.jvm.heapSizeAsPercentageOfPhysicalMemory(memoryPct.percentage()); + if (memoryPct != null) builder.jvm.heapSizeAsPercentageOfPhysicalMemory(memoryPct.ofContainerAvailable()); } @Override diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java index aeb6c030a49..00ab47e8ddb 100755 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java @@ -73,6 +73,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.OptionalDouble; +import java.util.OptionalInt; import java.util.Set; import java.util.TreeSet; @@ -721,10 +722,10 @@ public abstract class ContainerCluster<CONTAINER extends Container> * Returns the percentage of host physical memory this application has specified for nodes in this cluster, * or empty if this is not specified by the application. */ - public record JvmMemoryPercentage(int percentage, OptionalDouble availableMemoryGb) { - static JvmMemoryPercentage of(int percentage) { return new JvmMemoryPercentage(percentage, OptionalDouble.empty()); } - static JvmMemoryPercentage of(int percentage, double availableMemoryGb) { - return new JvmMemoryPercentage(percentage, OptionalDouble.of(availableMemoryGb)); + public record JvmMemoryPercentage(int ofContainerAvailable, OptionalInt ofContainerTotal, OptionalDouble asAbsoluteGb) { // optionalInt pctOfTotal < int pctOfAvailable + static JvmMemoryPercentage of(int percentageOfAvailable) { return new JvmMemoryPercentage(percentageOfAvailable, OptionalInt.empty(), OptionalDouble.empty()); } + static JvmMemoryPercentage of(int percentageOfAvailable, int percentageOfTotal, double absoluteMemoryGb) { + return new JvmMemoryPercentage(percentageOfAvailable, OptionalInt.of(percentageOfTotal), OptionalDouble.of(absoluteMemoryGb)); } } public Optional<JvmMemoryPercentage> getMemoryPercentage() { return Optional.empty(); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index 5bcd21a5b9b..9841ffe760c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -1040,7 +1040,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { } catch (NumberFormatException e) { throw new IllegalArgumentException("The memory percentage given for nodes in " + cluster + - " must be an integer percentage ending by the '%' sign", e); + " must be given as an integer followe by '%'", e); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java index ae890408b99..d37e5d5382a 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java @@ -249,7 +249,7 @@ public class ContentCluster extends TreeConfigProducer<AnyConfigProducer> implem for (ContainerModel containerModel : containers) { Optional<String> hostClusterId = containerModel.getCluster().getHostClusterId(); if (hostClusterId.isPresent() && hostClusterId.get().equals(clusterId) && containerModel.getCluster().getMemoryPercentage().isPresent()) { - return containerModel.getCluster().getMemoryPercentage().get().percentage() * 0.01; + return containerModel.getCluster().getMemoryPercentage().get().ofContainerAvailable() * 0.01; } } return 0.0; diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java index 5ead9812b56..2846925b9c3 100644 --- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java +++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java @@ -151,7 +151,7 @@ public class ModelProvisioningTest { assertEquals("-Xlog:gc", mydisc2.getContainers().get(1).getJvmOptions()); assertEquals("lib/blablamalloc.so", mydisc2.getContainers().get(0).getPreLoad()); assertEquals("lib/blablamalloc.so", mydisc2.getContainers().get(1).getPreLoad()); - assertEquals(45, mydisc2.getMemoryPercentage().get().percentage()); + assertEquals(45, mydisc2.getMemoryPercentage().get().ofContainerAvailable()); assertEquals(Optional.of("-XX:+UseParNewGC"), mydisc2.getJvmGCOptions()); QrStartConfig.Builder qrStartBuilder = new QrStartConfig.Builder(); mydisc2.getConfig(qrStartBuilder); @@ -234,8 +234,8 @@ public class ModelProvisioningTest { assertEquals(2, model.getContentClusters().get("content1").getRootGroup().getNodes().size(), "Nodes in content1"); assertEquals(1, model.getContainerClusters().get("container1").getContainers().size(), "Nodes in container1"); assertEquals(2, model.getContentClusters().get("content").getRootGroup().getNodes().size(), "Nodes in cluster without ID"); - assertEquals(65, physicalMemoryPercentage(model.getContainerClusters().get("container1")), "Heap size for container1"); - assertEquals(84, physicalMemoryPercentage(model.getContainerClusters().get("container2")), "Heap size for container2"); + assertEquals(85, physicalMemoryPercentage(model.getContainerClusters().get("container1")), "Heap size for container1"); + assertEquals(85, physicalMemoryPercentage(model.getContainerClusters().get("container2")), "Heap size for container2"); assertProvisioned(2, ClusterSpec.Id.from("content1"), ClusterSpec.Type.content, model); assertProvisioned(1, ClusterSpec.Id.from("container1"), ClusterSpec.Type.container, model); assertProvisioned(2, ClusterSpec.Id.from("content"), ClusterSpec.Type.content, model); @@ -287,8 +287,8 @@ public class ModelProvisioningTest { VespaModel model = tester.createModel(xmlWithNodes, true, deployStateWithClusterEndpoints("container1").deployLogger(logger)); assertEquals(2, model.getContentClusters().get("content1").getRootGroup().getNodes().size(), "Nodes in content1"); assertEquals(2, model.getContainerClusters().get("container1").getContainers().size(), "Nodes in container1"); - assertEquals(18, physicalMemoryPercentage(model.getContainerClusters().get("container1")), "Heap size is lowered with combined clusters"); - assertEquals(2025077080L, protonMemorySize(model.getContentClusters().get("content1")), "Memory for proton is lowered to account for the jvm heap"); + assertEquals(24, physicalMemoryPercentage(model.getContainerClusters().get("container1")), "Heap size is lowered with combined clusters"); + assertEquals(1876900708, protonMemorySize(model.getContentClusters().get("content1")), "Memory for proton is lowered to account for the jvm heap"); assertProvisioned(0, ClusterSpec.Id.from("container1"), ClusterSpec.Type.container, model); assertProvisioned(2, ClusterSpec.Id.from("content1"), ClusterSpec.Id.from("container1"), ClusterSpec.Type.combined, model); var msgs = logger.msgs().stream().filter(m -> m.level().equals(Level.WARNING)).toList(); @@ -356,7 +356,7 @@ public class ModelProvisioningTest { VespaModel model = tester.createModel(xmlWithNodes, true, deployStateWithClusterEndpoints("container1")); assertEquals(2, model.getContentClusters().get("content1").getRootGroup().getNodes().size(), "Nodes in content1"); assertEquals(2, model.getContainerClusters().get("container1").getContainers().size(), "Nodes in container1"); - assertEquals(65, physicalMemoryPercentage(model.getContainerClusters().get("container1")), "Heap size is normal"); + assertEquals(85, physicalMemoryPercentage(model.getContainerClusters().get("container1")), "Heap size is normal"); assertEquals((long) ((3 - memoryOverheadGb) * (Math.pow(1024, 3))), protonMemorySize(model.getContentClusters().get("content1")), "Memory for proton is normal"); } diff --git a/config-model/src/test/java/com/yahoo/schema/SummaryTestCase.java b/config-model/src/test/java/com/yahoo/schema/SummaryTestCase.java index 8ffbab84fd7..539ec91ee5f 100644 --- a/config-model/src/test/java/com/yahoo/schema/SummaryTestCase.java +++ b/config-model/src/test/java/com/yahoo/schema/SummaryTestCase.java @@ -6,16 +6,13 @@ import com.yahoo.vespa.documentmodel.DocumentSummary; import com.yahoo.vespa.model.test.utils.DeployLoggerStub; import com.yahoo.vespa.objects.FieldBase; import com.yahoo.yolean.Exceptions; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static com.yahoo.config.model.test.TestUtil.joinLines; import java.util.Collection; import java.util.List; -import java.util.Optional; import java.util.logging.Level; -import java.util.stream.Collectors; import static org.junit.jupiter.api.Assertions.*; @@ -233,16 +230,12 @@ public class SummaryTestCase { " document-summary test_summary inherits nonesuch {" + " }" + "}"); - DeployLoggerStub logger = new DeployLoggerStub(); - ApplicationBuilder.createFromStrings(logger, schema); - assertEquals("document-summary 'test_summary' inherits 'nonesuch' but this is not present in schema 'test'", - logger.entries.get(0).message); - // fail("Expected failure"); + ApplicationBuilder.createFromString(schema); + fail("Expected failure"); } catch (IllegalArgumentException e) { - fail(); - // assertEquals("document-summary 'test_summary' inherits nonesuch but this is not present in schema 'test'", - // e.getMessage()); + assertEquals("document-summary 'test_summary' inherits 'nonesuch', but this is not present in schema 'test'", + e.getMessage()); } } diff --git a/config-model/src/test/java/com/yahoo/schema/processing/SummaryConsistencyTestCase.java b/config-model/src/test/java/com/yahoo/schema/processing/SummaryConsistencyTestCase.java index 1184e49b381..5ea097f13fb 100644 --- a/config-model/src/test/java/com/yahoo/schema/processing/SummaryConsistencyTestCase.java +++ b/config-model/src/test/java/com/yahoo/schema/processing/SummaryConsistencyTestCase.java @@ -5,11 +5,11 @@ import com.yahoo.schema.Schema; import com.yahoo.schema.ApplicationBuilder; import com.yahoo.schema.parser.ParseException; import com.yahoo.vespa.documentmodel.SummaryTransform; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static com.yahoo.config.model.test.TestUtil.joinLines; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; public class SummaryConsistencyTestCase { @@ -45,8 +45,7 @@ public class SummaryConsistencyTestCase { } @Test - @Disabled - void testDocumentTypesWithInheritanceOfNonExistingField() throws ParseException { + void testDocumentSummaryWithInheritanceOfNonExistingSummary() { String schemaString = """ schema foo { document foo { @@ -61,8 +60,7 @@ public class SummaryConsistencyTestCase { } } """; - var schema = ApplicationBuilder.createFromString(schemaString).getSchema(); - schema.getSummaryField("foo_summary"); + assertThrows(IllegalArgumentException.class, () -> ApplicationBuilder.createFromString(schemaString).getSchema()); } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidatorTest.java index 45125c8eb68..340968f89d1 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidatorTest.java @@ -45,10 +45,10 @@ class JvmHeapSizeValidatorTest { @Test void fails_on_too_low_heap_size() throws IOException, SAXException { - var deployState = createDeployState(2.2, 1024L * 1024 * 1024); + var deployState = createDeployState(2.3, 1024L * 1024 * 1024); var model = new VespaModel(new NullConfigModelRegistry(), deployState); ValidationTester.expect(new JvmHeapSizeValidator(), model, deployState, - "Allocated memory to JVM in cluster 'container' is too low (0.50GB < 0.60GB). Estimated cost of ONNX models is 1.00GB."); + "Allocated memory to JVM in cluster 'container' is too low (0.51GB < 0.60GB). Estimated cost of ONNX models is 1.00GB."); } @Test diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidatorTest.java index 4c0786ea879..484a938476d 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidatorTest.java @@ -33,7 +33,7 @@ public class RestartOnDeployForOnnxModelChangesValidatorTest { // Must be so large that changing model set or options requires restart (due to using more memory than available), // but not so large that deployment will not work at all with one model - private static final long defaultCost = 723456789; + private static final long defaultCost = 635241309; private static final long defaultHash = 0; @@ -47,8 +47,8 @@ public class RestartOnDeployForOnnxModelChangesValidatorTest { @Test void validate_changed_estimated_cost() { - VespaModel current = createModel(onnxModelCost(70000000, defaultHash)); - VespaModel next = createModel(onnxModelCost(723456789, defaultHash)); + VespaModel current = createModel(onnxModelCost(defaultCost, defaultHash)); + VespaModel next = createModel(onnxModelCost(19 * defaultCost / 20, defaultHash)); List<ConfigChangeAction> result = validateModel(current, next); assertEquals(1, result.size()); assertTrue(result.get(0).validationId().isEmpty()); @@ -58,8 +58,8 @@ public class RestartOnDeployForOnnxModelChangesValidatorTest { @Test void validate_changed_estimated_cost_non_hosted() { boolean hosted = false; - VespaModel current = createModel(onnxModelCost(70000000, defaultHash), hosted); - VespaModel next = createModel(onnxModelCost(723456789, defaultHash), hosted); + VespaModel current = createModel(onnxModelCost(defaultCost, defaultHash), hosted); + VespaModel next = createModel(onnxModelCost(19 * defaultCost / 20, defaultHash), hosted); List<ConfigChangeAction> result = validateModel(current, next, hosted); assertEquals(0, result.size()); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/searchchain/SchemaChainsTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/search/searchchain/SchemaChainsTest.java index ea43f5c8124..b782366655f 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/searchchain/SchemaChainsTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/searchchain/SchemaChainsTest.java @@ -142,7 +142,7 @@ public class SchemaChainsTest extends SchemaChainsTestBase { assertTrue(chain.phases().isEmpty()); assertEquals(1, chain.inherits().size()); assertEquals("native", chain.inherits(0)); - assertEquals(10, chain.components().size()); + assertEquals(11, chain.components().size()); assertEquals("com.yahoo.prelude.querytransform.PhrasingSearcher@vespa", chain.components(0)); assertEquals("com.yahoo.prelude.searcher.FieldCollapsingSearcher@vespa", chain.components(1)); assertEquals("com.yahoo.search.yql.MinimalQueryInserter@vespa", chain.components(2)); @@ -153,13 +153,14 @@ public class SchemaChainsTest extends SchemaChainsTestBase { assertEquals("com.yahoo.prelude.semantics.SemanticSearcher@vespa", chain.components(7)); assertEquals("com.yahoo.search.grouping.GroupingQueryParser@vespa", chain.components(8)); assertEquals("com.yahoo.search.querytransform.WeakAndReplacementSearcher@vespa", chain.components(9)); + assertEquals("com.yahoo.search.searchers.OpportunisticWeakAndSearcher@vespa", chain.components(10)); assertTrue(chain.excludes().isEmpty()); assertEquals(ChainsConfig.Chains.Type.SEARCH, chain.type()); } @Test public void require_all_default_chains_are_correct() { - assertEquals(63, chainsConfig.components().size()); + assertEquals(64, chainsConfig.components().size()); assertEquals(10, chainsConfig.chains().size()); validateVespaPhasesChain(findChain("vespaPhases")); validateNativeChain(findChain("native")); diff --git a/container-disc/src/main/sh/vespa-start-container-daemon.sh b/container-disc/src/main/sh/vespa-start-container-daemon.sh index c4807bb134f..a13485fb6bb 100755 --- a/container-disc/src/main/sh/vespa-start-container-daemon.sh +++ b/container-disc/src/main/sh/vespa-start-container-daemon.sh @@ -151,8 +151,8 @@ configure_memory() { available_cgroup=$((available_cgroup_bytes >> 20)) available=$((available > available_cgroup ? available_cgroup : available)) fi - #Subtract 1G as fixed overhead for an application container. - reserved_mem=1024 + # Subtract 700MB as fixed overhead for an application container -- keep in sync with com.yahoo.vespa.model.Host.memoryOverheadGb + reserved_mem=700 available=$((available > reserved_mem ? available - reserved_mem : available)) jvm_heapsize=$((available * jvm_heapSizeAsPercentageOfPhysicalMemory / 100)) diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index 1c6c773afd9..1bed32c6cdf 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -8917,6 +8917,18 @@ ], "fields" : [ ] }, + "com.yahoo.search.searchers.OpportunisticWeakAndSearcher" : { + "superClass" : "com.yahoo.search.Searcher", + "interfaces" : [ ], + "attributes" : [ + "public" + ], + "methods" : [ + "public void <init>()", + "public com.yahoo.search.Result search(com.yahoo.search.Query, com.yahoo.search.searchchain.Execution)" + ], + "fields" : [ ] + }, "com.yahoo.search.searchers.QueryValidator" : { "superClass" : "com.yahoo.search.Searcher", "interfaces" : [ ], diff --git a/container-search/src/main/java/com/yahoo/search/querytransform/WeakAndReplacementSearcher.java b/container-search/src/main/java/com/yahoo/search/querytransform/WeakAndReplacementSearcher.java index 7536e74042c..72a1a7d3430 100644 --- a/container-search/src/main/java/com/yahoo/search/querytransform/WeakAndReplacementSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/querytransform/WeakAndReplacementSearcher.java @@ -12,6 +12,7 @@ import com.yahoo.search.Searcher; import com.yahoo.search.searchchain.Execution; import com.yahoo.search.yql.MinimalQueryInserter; import com.yahoo.yolean.chain.After; +import com.yahoo.yolean.chain.Provides; /** * Recursively replaces all instances of OrItems with WeakAndItems if the query property weakand.replace is true. @@ -19,10 +20,12 @@ import com.yahoo.yolean.chain.After; * * @author karowan */ +@Provides(WeakAndReplacementSearcher.REPLACE_OR_WITH_WEAKAND) @After(MinimalQueryInserter.EXTERNAL_YQL) public class WeakAndReplacementSearcher extends Searcher { + public static final String REPLACE_OR_WITH_WEAKAND = "replace-or-with-weakand"; static final CompoundName WEAKAND_REPLACE = CompoundName.from("weakAnd.replace"); - static final CompoundName WAND_HITS = CompoundName.from("wand.hits"); + public static final CompoundName WAND_HITS = CompoundName.from("wand.hits"); @Override public Result search(Query query, Execution execution) { if (!query.properties().getBoolean(WEAKAND_REPLACE)) { diff --git a/container-search/src/main/java/com/yahoo/search/searchchain/model/VespaSearchers.java b/container-search/src/main/java/com/yahoo/search/searchchain/model/VespaSearchers.java index 69a1f8ec6cb..c03a74ea2c5 100644 --- a/container-search/src/main/java/com/yahoo/search/searchchain/model/VespaSearchers.java +++ b/container-search/src/main/java/com/yahoo/search/searchchain/model/VespaSearchers.java @@ -34,7 +34,8 @@ public class VespaSearchers { com.yahoo.prelude.searcher.PosSearcher.class, com.yahoo.prelude.semantics.SemanticSearcher.class, com.yahoo.search.grouping.GroupingQueryParser.class, - com.yahoo.search.querytransform.WeakAndReplacementSearcher.class); + com.yahoo.search.querytransform.WeakAndReplacementSearcher.class, + com.yahoo.search.searchers.OpportunisticWeakAndSearcher.class); public static final Collection<ChainedComponentModel> nativeSearcherModels; diff --git a/container-search/src/main/java/com/yahoo/search/searchers/OpportunisticWeakAndSearcher.java b/container-search/src/main/java/com/yahoo/search/searchers/OpportunisticWeakAndSearcher.java new file mode 100644 index 00000000000..d871dda2aa2 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/searchers/OpportunisticWeakAndSearcher.java @@ -0,0 +1,83 @@ +package com.yahoo.search.searchers; + +import com.yahoo.api.annotations.Beta; +import com.yahoo.component.chain.dependencies.After; +import com.yahoo.prelude.query.AndItem; +import com.yahoo.prelude.query.CompositeItem; +import com.yahoo.prelude.query.Item; +import com.yahoo.prelude.query.WeakAndItem; +import com.yahoo.processing.request.CompoundName; +import com.yahoo.search.Query; +import com.yahoo.search.Result; +import com.yahoo.search.Searcher; +import com.yahoo.search.querytransform.WeakAndReplacementSearcher; +import com.yahoo.search.searchchain.Execution; + +import static com.yahoo.search.querytransform.WeakAndReplacementSearcher.WAND_HITS; + +/** + * Will opportunistically replace the WeakAND with an AND as it is faster. + * If enough hits are returned all is good and we return. If not we fall back to the original query. + * + * @author baldersheim + */ +@Beta +@After(WeakAndReplacementSearcher.REPLACE_OR_WITH_WEAKAND) +public class OpportunisticWeakAndSearcher extends Searcher { + static final CompoundName OPPORTUNISTIC_AND = CompoundName.from("weakAnd.opportunistic.and"); + + @Override + public Result search(Query query, Execution execution) { + if (!query.properties().getBoolean(OPPORTUNISTIC_AND)) { + return execution.search(query); + } + + Item originalRoot = query.getModel().getQueryTree().getRoot(); + int targetHits = targetHits(originalRoot); + if (targetHits >= 0) { + query.getModel().getQueryTree().setRoot(weakAnd2AndRecurse(originalRoot.clone())); + query.trace("WeakAND => AND", true, 2); + Result result = execution.search(query); + if (result.getHitCount() >= query.properties().getInteger(WAND_HITS)) { + return result; + } + query.getModel().getQueryTree().setRoot(originalRoot); + return execution.search(query); + } + return execution.search(query); + } + + // returns targetHits for the first WeakAndItem found, -1 if none found. + static int targetHits(Item item) { + if (!(item instanceof CompositeItem compositeItem)) return -1; + if (item instanceof WeakAndItem weakAndItem) return weakAndItem.getN(); + for (int i = 0; i < compositeItem.getItemCount(); i++) { + int targetHits = targetHits(compositeItem.getItem(i)); + if (targetHits >= 0) return targetHits; + } + return -1; + } + + static Item weakAnd2AndRecurse(Item item) { + if (!(item instanceof CompositeItem compositeItem)) return item; + compositeItem = weakAnd2And(compositeItem); + for (int i = 0; i < compositeItem.getItemCount(); i++) { + Item subItem = compositeItem.getItem(i); + Item replacedItem = weakAnd2AndRecurse(subItem); + if (replacedItem != subItem) { + compositeItem.setItem(i, replacedItem); + } + } + return compositeItem; + } + + private static CompositeItem weakAnd2And(CompositeItem item) { + if (item instanceof WeakAndItem weakAndItem) { + AndItem andItem = new AndItem(); + andItem.setWeight(weakAndItem.getWeight()); + item.items().forEach(andItem::addItem); + return andItem; + } + return item; + } +} diff --git a/container-search/src/test/java/com/yahoo/search/querytransform/WeakAndReplacementSearcherTestCase.java b/container-search/src/test/java/com/yahoo/search/querytransform/WeakAndReplacementSearcherTestCase.java index 52f5fd0cafb..7b91a5d3c25 100644 --- a/container-search/src/test/java/com/yahoo/search/querytransform/WeakAndReplacementSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/querytransform/WeakAndReplacementSearcherTestCase.java @@ -23,6 +23,7 @@ import static com.yahoo.search.querytransform.WeakAndReplacementSearcher.WAND_HI 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.assertTrue; public class WeakAndReplacementSearcherTestCase { @@ -57,7 +58,7 @@ public class WeakAndReplacementSearcherTestCase { Result result = buildExec().search(query); Item root = TestUtils.getQueryTreeRoot(result); assertFalse(orItemsExist(root)); - assertTrue(root instanceof WeakAndItem); + assertInstanceOf(WeakAndItem.class, root); assertEquals(N, ((WeakAndItem) root).getN()); } @@ -103,24 +104,22 @@ public class WeakAndReplacementSearcherTestCase { if (item1 != item2) { return false; } - if (!(item1 instanceof CompositeItem)) { + if (!(item1 instanceof CompositeItem compositeItem1)) { return true; } - CompositeItem compositeItem1 = (CompositeItem) item1; CompositeItem compositeItem2 = (CompositeItem) item2; return IntStream.range(0, compositeItem1.getItemCount()) .allMatch(i -> deepEquals(compositeItem1.getItem(i), compositeItem2.getItem(i))); } private boolean orItemsExist(Item item) { - if (!(item instanceof CompositeItem)) { + if (!(item instanceof CompositeItem compositeItem)) { return false; } if (item instanceof OrItem) { return true; } - CompositeItem compositeItem = (CompositeItem) item; return compositeItem.items().stream().anyMatch(this::orItemsExist); } diff --git a/container-search/src/test/java/com/yahoo/search/searchers/OpportunisticWeakAndSearcherTestCase.java b/container-search/src/test/java/com/yahoo/search/searchers/OpportunisticWeakAndSearcherTestCase.java new file mode 100644 index 00000000000..642b9bc7bc4 --- /dev/null +++ b/container-search/src/test/java/com/yahoo/search/searchers/OpportunisticWeakAndSearcherTestCase.java @@ -0,0 +1,40 @@ +package com.yahoo.search.searchers; + +import com.yahoo.prelude.query.AndItem; +import com.yahoo.prelude.query.CompositeItem; +import com.yahoo.prelude.query.Item; +import com.yahoo.prelude.query.OrItem; +import com.yahoo.prelude.query.WeakAndItem; +import com.yahoo.prelude.query.WordItem; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + + +public class OpportunisticWeakAndSearcherTestCase { + private static Item buildQueryItem(CompositeItem root, CompositeItem injectAtLevel2) { + root.addItem(new WordItem("text")); + injectAtLevel2.addItem(new WordItem("a")); + injectAtLevel2.addItem(new WordItem("b")); + root.addItem(injectAtLevel2); + return root; + } + + @Test + public void requireThatWeakAndIsDetected() { + assertEquals(-1, OpportunisticWeakAndSearcher.targetHits(new OrItem())); + assertEquals(33, OpportunisticWeakAndSearcher.targetHits(new WeakAndItem(33))); + assertEquals(77, OpportunisticWeakAndSearcher.targetHits(buildQueryItem(new OrItem(), new WeakAndItem(77)))); + assertEquals(77, OpportunisticWeakAndSearcher.targetHits(buildQueryItem(new AndItem(), new WeakAndItem(77)))); + assertEquals(-1, OpportunisticWeakAndSearcher.targetHits(buildQueryItem(new OrItem(), new AndItem()))); + } + + @Test + public void requireThatWeakAndIsReplacedWithAnd() { + assertEquals(buildQueryItem(new OrItem(), new AndItem()), + OpportunisticWeakAndSearcher.weakAnd2AndRecurse(buildQueryItem(new OrItem(), new WeakAndItem()))); + assertEquals(buildQueryItem(new AndItem(), new AndItem()), + OpportunisticWeakAndSearcher.weakAnd2AndRecurse(buildQueryItem(new AndItem(), new WeakAndItem()))); + } + +} diff --git a/dependency-versions/pom.xml b/dependency-versions/pom.xml index 0e7921f050a..ddcfe103b04 100644 --- a/dependency-versions/pom.xml +++ b/dependency-versions/pom.xml @@ -68,7 +68,7 @@ <assertj.vespa.version>3.26.0</assertj.vespa.version> <!-- Athenz dependencies. Make sure these dependencies match those in Vespa's internal repositories --> - <aws-sdk.vespa.version>1.12.731</aws-sdk.vespa.version> + <aws-sdk.vespa.version>1.12.732</aws-sdk.vespa.version> <athenz.vespa.version>1.11.59</athenz.vespa.version> <!-- Athenz END --> @@ -79,7 +79,7 @@ xargs perl -pi -e 's/major = [0-9]+, minor = [0-9]+, micro = [0-9]+/major = 5, minor = 3, micro = 0/g' --> <bouncycastle.vespa.version>1.78.1</bouncycastle.vespa.version> - <byte-buddy.vespa.version>1.14.16</byte-buddy.vespa.version> + <byte-buddy.vespa.version>1.14.17</byte-buddy.vespa.version> <checker-qual.vespa.version>3.38.0</checker-qual.vespa.version> <commons-beanutils.vespa.version>1.9.4</commons-beanutils.vespa.version> <commons-codec.vespa.version>1.17.0</commons-codec.vespa.version> @@ -102,7 +102,7 @@ <felix.log.vespa.version>1.3.0</felix.log.vespa.version> <findbugs.vespa.version>3.0.2</findbugs.vespa.version> <!-- Should be kept in sync with guava --> <hamcrest.vespa.version>2.2</hamcrest.vespa.version> - <hdrhistogram.vespa.version>2.2.1</hdrhistogram.vespa.version> + <hdrhistogram.vespa.version>2.2.2</hdrhistogram.vespa.version> <huggingface.vespa.version>0.28.0</huggingface.vespa.version> <icu4j.vespa.version>75.1</icu4j.vespa.version> <java-jjwt.vespa.version>0.11.5</java-jjwt.vespa.version> @@ -173,7 +173,7 @@ <maven-core.vespa.version>3.9.7</maven-core.vespa.version> <maven-dependency-plugin.vespa.version>3.6.1</maven-dependency-plugin.vespa.version> <maven-deploy-plugin.vespa.version>3.1.2</maven-deploy-plugin.vespa.version> - <maven-enforcer-plugin.vespa.version>3.4.1</maven-enforcer-plugin.vespa.version> + <maven-enforcer-plugin.vespa.version>3.5.0</maven-enforcer-plugin.vespa.version> <maven-failsafe-plugin.vespa.version>3.2.5</maven-failsafe-plugin.vespa.version> <maven-gpg-plugin.vespa.version>3.2.4</maven-gpg-plugin.vespa.version> <maven-install-plugin.vespa.version>3.1.2</maven-install-plugin.vespa.version> @@ -185,6 +185,7 @@ <maven-resolver.vespa.version>1.9.20</maven-resolver.vespa.version> <maven-shade-plugin.vespa.version>3.5.3</maven-shade-plugin.vespa.version> <maven-shared-utils.vespa.version>3.4.2</maven-shared-utils.vespa.version> + <maven-dependency-tree.vespa.version>3.3.0</maven-dependency-tree.vespa.version> <maven-site-plugin.vespa.version>3.12.1</maven-site-plugin.vespa.version> <maven-source-plugin.vespa.version>3.3.1</maven-source-plugin.vespa.version> <properties-maven-plugin.vespa.version>1.2.1</properties-maven-plugin.vespa.version> 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 f200940e52d..1b1e74c0b1e 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java @@ -196,8 +196,8 @@ public class PermanentFlags { ); public static final UnboundBooleanFlag JVM_OMIT_STACK_TRACE_IN_FAST_THROW = defineFeatureFlag( - "jvm-omit-stack-trace-in-fast-throw", true, - "Controls JVM option OmitStackTraceInFastThrow (default feature flag value is true, which is the default JVM option value as well)", + "jvm-omit-stack-trace-in-fast-throw", false, + "Controls JVM option OmitStackTraceInFastThrow (the default JVM option corresponds to feature flag being false )", "takes effect on JVM restart", CLUSTER_TYPE, INSTANCE_ID); diff --git a/maven-plugins/allowed-maven-dependencies.txt b/maven-plugins/allowed-maven-dependencies.txt index 6245caf1d92..80cd3c47a58 100644 --- a/maven-plugins/allowed-maven-dependencies.txt +++ b/maven-plugins/allowed-maven-dependencies.txt @@ -32,7 +32,7 @@ org.apache.maven.resolver:maven-resolver-impl:${maven-resolver.vespa.version} org.apache.maven.resolver:maven-resolver-named-locks:${maven-resolver.vespa.version} org.apache.maven.resolver:maven-resolver-spi:${maven-resolver.vespa.version} org.apache.maven.resolver:maven-resolver-util:${maven-resolver.vespa.version} -org.apache.maven.shared:maven-dependency-tree:3.2.1 +org.apache.maven.shared:maven-dependency-tree:${maven-dependency-tree.vespa.version} org.apache.maven.shared:maven-shared-utils:${maven-shared-utils.vespa.version} org.apache.maven:maven-api-meta:${maven-xml-impl.vespa.version} org.apache.maven:maven-api-xml:${maven-xml-impl.vespa.version} @@ -59,8 +59,6 @@ org.codehaus.plexus:plexus-sec-dispatcher:2.0 org.codehaus.plexus:plexus-utils:${plexus-utils.vespa.version} org.codehaus.plexus:plexus-xml:${plexus-xml.vespa.version} org.codehaus.woodstox:stax2-api:${stax2-api.vespa.version} -org.eclipse.aether:aether-api:1.0.0.v20140518 -org.eclipse.aether:aether-util:1.0.0.v20140518 org.eclipse.sisu:org.eclipse.sisu.inject:${eclipse-sisu.vespa.version} org.eclipse.sisu:org.eclipse.sisu.plexus:${eclipse-sisu.vespa.version} org.hamcrest:hamcrest-core:${hamcrest.vespa.version} diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/MetricsJsonResponse.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/MetricsJsonResponse.java new file mode 100644 index 00000000000..b927db790b2 --- /dev/null +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/MetricsJsonResponse.java @@ -0,0 +1,31 @@ +package ai.vespa.metricsproxy.http; + +import com.yahoo.container.jdisc.HttpResponse; + +import java.io.IOException; +import java.io.OutputStream; +import java.util.function.Consumer; + +/** + * @author jonmv + */ +public class MetricsJsonResponse extends HttpResponse { + + private final Consumer<OutputStream> modelWriter; + + public MetricsJsonResponse(int status, Consumer<OutputStream> modelWriter) { + super(status); + this.modelWriter = modelWriter; + } + + @Override + public void render(OutputStream outputStream) throws IOException { + modelWriter.accept(outputStream); + } + + @Override + public long maxPendingBytes() { + return 1 << 20; + } + +} diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/PrometheusResponse.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/PrometheusResponse.java new file mode 100644 index 00000000000..e0c74671c9c --- /dev/null +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/PrometheusResponse.java @@ -0,0 +1,35 @@ +package ai.vespa.metricsproxy.http; + +import ai.vespa.metricsproxy.metric.model.prometheus.PrometheusModel; +import com.yahoo.container.jdisc.HttpResponse; + +import java.io.IOException; +import java.io.OutputStream; +import java.io.OutputStreamWriter; +import java.io.Writer; + +/** + * @author jonmv + */ +public class PrometheusResponse extends HttpResponse { + + private final PrometheusModel model; + + public PrometheusResponse(int status, PrometheusModel model) { + super(status); + this.model = model; + } + + @Override + public void render(OutputStream outputStream) throws IOException { + Writer writer = new OutputStreamWriter(outputStream); + model.serialize(writer); + writer.flush(); + } + + @Override + public long maxPendingBytes() { + return 1 << 20; + } + +} diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandler.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandler.java index 58b51020bb9..ace0d0abc65 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandler.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandler.java @@ -3,7 +3,8 @@ package ai.vespa.metricsproxy.http.application; import ai.vespa.metricsproxy.core.MetricsConsumers; -import ai.vespa.metricsproxy.http.TextResponse; +import ai.vespa.metricsproxy.http.MetricsJsonResponse; +import ai.vespa.metricsproxy.http.PrometheusResponse; import ai.vespa.metricsproxy.metric.model.ConsumerId; import ai.vespa.metricsproxy.metric.model.DimensionId; import ai.vespa.metricsproxy.metric.model.MetricsPacket; @@ -62,12 +63,12 @@ public class ApplicationMetricsHandler extends HttpHandlerBase { return Optional.empty(); } - private JsonResponse applicationMetricsResponse(String requestedConsumer) { + private HttpResponse applicationMetricsResponse(String requestedConsumer) { try { ConsumerId consumer = getConsumerOrDefault(requestedConsumer, metricsConsumers); var metricsByNode = metricsRetriever.getMetrics(consumer); - return new JsonResponse(OK, toGenericApplicationModel(metricsByNode).serialize()); + return new MetricsJsonResponse(OK, toGenericApplicationModel(metricsByNode)::serialize); } catch (Exception e) { log.log(Level.WARNING, "Got exception when retrieving metrics:", e); @@ -75,7 +76,7 @@ public class ApplicationMetricsHandler extends HttpHandlerBase { } } - private TextResponse applicationPrometheusResponse(String requestedConsumer) { + private HttpResponse applicationPrometheusResponse(String requestedConsumer) { ConsumerId consumer = getConsumerOrDefault(requestedConsumer, metricsConsumers); var metricsByNode = metricsRetriever.getMetrics(consumer); @@ -87,7 +88,7 @@ public class ApplicationMetricsHandler extends HttpHandlerBase { .map(builder -> builder.putDimension(DimensionId.toDimensionId("hostname"), element.hostname)) .map(MetricsPacket.Builder::build)) .toList(); - return new TextResponse(200, toPrometheusModel(metricsForAllNodes).serialize()); + return new PrometheusResponse(200, toPrometheusModel(metricsForAllNodes)); } } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/metrics/MetricsV1Handler.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/metrics/MetricsV1Handler.java index 3e4565c780b..50c1420edef 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/metrics/MetricsV1Handler.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/metrics/MetricsV1Handler.java @@ -3,6 +3,7 @@ package ai.vespa.metricsproxy.http.metrics; import ai.vespa.metricsproxy.core.MetricsConsumers; import ai.vespa.metricsproxy.core.MetricsManager; +import ai.vespa.metricsproxy.http.MetricsJsonResponse; import ai.vespa.metricsproxy.http.ValuesFetcher; import ai.vespa.metricsproxy.metric.model.MetricsPacket; import ai.vespa.metricsproxy.service.VespaServices; @@ -51,10 +52,10 @@ public class MetricsV1Handler extends HttpHandlerBase { return Optional.empty(); } - private JsonResponse valuesResponse(String consumer) { + private HttpResponse valuesResponse(String consumer) { try { List<MetricsPacket> metrics = valuesFetcher.fetch(consumer); - return new JsonResponse(OK, toGenericJsonModel(metrics).serialize()); + return new MetricsJsonResponse(OK, toGenericJsonModel(metrics)::serialize); } catch (Exception e) { log.log(Level.WARNING, "Got exception when rendering metrics:", e); return new ErrorResponse(INTERNAL_SERVER_ERROR, e.getMessage()); diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/metrics/MetricsV2Handler.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/metrics/MetricsV2Handler.java index 1f6cc17b2e1..7e9bba466df 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/metrics/MetricsV2Handler.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/metrics/MetricsV2Handler.java @@ -3,6 +3,7 @@ package ai.vespa.metricsproxy.http.metrics; import ai.vespa.metricsproxy.core.MetricsConsumers; import ai.vespa.metricsproxy.core.MetricsManager; +import ai.vespa.metricsproxy.http.MetricsJsonResponse; import ai.vespa.metricsproxy.http.ValuesFetcher; import ai.vespa.metricsproxy.http.application.ClusterIdDimensionProcessor; import ai.vespa.metricsproxy.http.application.Node; @@ -62,7 +63,7 @@ public class MetricsV2Handler extends HttpHandlerBase { return Optional.empty(); } - private JsonResponse valuesResponse(String consumer) { + private HttpResponse valuesResponse(String consumer) { try { List<MetricsPacket> metrics = processAndBuild(valuesFetcher.fetchMetricsAsBuilders(consumer), new ServiceIdDimensionProcessor(), @@ -71,7 +72,7 @@ public class MetricsV2Handler extends HttpHandlerBase { Node localNode = new Node(nodeInfoConfig.role(), nodeInfoConfig.hostname(), 0, ""); Map<Node, List<MetricsPacket>> metricsByNode = Map.of(localNode, metrics); - return new JsonResponse(OK, toGenericApplicationModel(metricsByNode).serialize()); + return new MetricsJsonResponse(OK, toGenericApplicationModel(metricsByNode)::serialize); } catch (Exception e) { log.log(Level.WARNING, "Got exception when rendering metrics:", e); return new ErrorResponse(INTERNAL_SERVER_ERROR, e.getMessage()); diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/prometheus/PrometheusHandler.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/prometheus/PrometheusHandler.java index d73561b5eff..e609b54b916 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/prometheus/PrometheusHandler.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/prometheus/PrometheusHandler.java @@ -3,6 +3,7 @@ package ai.vespa.metricsproxy.http.prometheus; import ai.vespa.metricsproxy.core.MetricsConsumers; import ai.vespa.metricsproxy.core.MetricsManager; +import ai.vespa.metricsproxy.http.PrometheusResponse; import ai.vespa.metricsproxy.http.TextResponse; import ai.vespa.metricsproxy.http.ValuesFetcher; import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensions; @@ -56,11 +57,11 @@ public class PrometheusHandler extends HttpHandlerBase { return Optional.empty(); } - private TextResponse valuesResponse(String consumer) { + private HttpResponse valuesResponse(String consumer) { try { List<MetricsPacket> metrics = new ArrayList<>(valuesFetcher.fetch(consumer)); metrics.addAll(nodeMetricGatherer.gatherMetrics()); - return new TextResponse(OK, toPrometheusModel(metrics).serialize()); + return new PrometheusResponse(OK, toPrometheusModel(metrics)); } catch (Exception e) { log.log(Level.WARNING, "Got exception when rendering metrics:", e); return new TextResponse(INTERNAL_SERVER_ERROR, e.getMessage()); diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericApplicationModel.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericApplicationModel.java index 59d1f9e5c83..105724d17f8 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericApplicationModel.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericApplicationModel.java @@ -5,10 +5,13 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.util.List; import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_ABSENT; +import static java.nio.charset.StandardCharsets.UTF_8; /** * @author gjoranv @@ -21,8 +24,14 @@ public class GenericApplicationModel { public List<GenericJsonModel> nodes; public String serialize() { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + serialize(out); + return out.toString(UTF_8); + } + + public void serialize(OutputStream out) { try { - return JacksonUtil.objectMapper().writeValueAsString(this); + JacksonUtil.objectMapper().writeValue(out, this); } catch (IOException e) { throw new JsonRenderingException("Could not render application nodes. Check the log for details.", e); } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java index c4ba0f39a18..54616dff759 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java @@ -6,10 +6,13 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonPropertyOrder; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.util.List; import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_ABSENT; +import static java.nio.charset.StandardCharsets.UTF_8; /** * @author gjoranv @@ -32,8 +35,14 @@ public class GenericJsonModel { public List<GenericService> services; public String serialize() { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + serialize(out); + return out.toString(UTF_8); + } + + public void serialize(OutputStream out) { try { - return JacksonUtil.objectMapper().writeValueAsString(this); + JacksonUtil.objectMapper().writeValue(out, this); } catch (IOException e) { throw new JsonRenderingException("Could not render metrics. Check the log for details.", e); } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/prometheus/PrometheusModel.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/prometheus/PrometheusModel.java index fd00ad67ec0..0f3878821f3 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/prometheus/PrometheusModel.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/prometheus/PrometheusModel.java @@ -11,6 +11,7 @@ import io.prometheus.client.Collector.MetricFamilySamples.Sample; import io.prometheus.client.exporter.common.TextFormat; import java.io.StringWriter; +import java.io.Writer; import java.util.ArrayList; import java.util.Enumeration; import java.util.Iterator; @@ -50,12 +51,16 @@ public class PrometheusModel implements Enumeration<MetricFamilySamples> { public String serialize() { var writer = new StringWriter(); + serialize(writer); + return writer.toString(); + } + + public void serialize(Writer writer) { try { TextFormat.write004(writer, this); } catch (Exception e) { throw new PrometheusRenderingException("Could not render metrics. Check the log for details.", e); } - return writer.toString(); } private MetricFamilySamples createMetricFamily(MetricId metricId) { @@ -70,6 +75,7 @@ public class PrometheusModel implements Enumeration<MetricFamilySamples> { })); return new MetricFamilySamples(metricId.getIdForPrometheus(), Collector.Type.UNKNOWN, "", sampleList); } + private static Sample createSample(ServiceId serviceId, MetricId metricId, Number metric, Long timeStamp, Map<DimensionId, String> dimensions) { diff --git a/metrics/src/main/java/ai/vespa/metrics/StorageMetrics.java b/metrics/src/main/java/ai/vespa/metrics/StorageMetrics.java index 4d91cc1d989..8718dc0076a 100644 --- a/metrics/src/main/java/ai/vespa/metrics/StorageMetrics.java +++ b/metrics/src/main/java/ai/vespa/metrics/StorageMetrics.java @@ -39,8 +39,8 @@ public enum StorageMetrics implements VespaMetrics { VDS_FILESTOR_ALLTHREADS_MERGEBUCKETS_FAILED("vds.filestor.allthreads.mergebuckets.failed", Unit.REQUEST, "Number of failed requests."), VDS_FILESTOR_ALLTHREADS_MERGEBUCKETS_LATENCY("vds.filestor.allthreads.mergebuckets.latency", Unit.MILLISECOND, "Latency of successful requests."), VDS_FILESTOR_ALLTHREADS_MERGELATENCYTOTAL("vds.filestor.allthreads.mergelatencytotal", Unit.MILLISECOND, "Latency of total merge operation, from master node receives it, until merge is complete and master node replies."), - VDS_FILESTOR_ALLTHREADS_MERGE_PUT_LATENCY("vds.filestor.allthreads.put_latency", Unit.MILLISECOND, "Latency of individual puts that are part of merge operations"), // TODO Vespa 9: Update metric name to include 'merge' - VDS_FILESTOR_ALLTHREADS_MERGE_REMOVE_LATENCY("vds.filestor.allthreads.remove_latency", Unit.MILLISECOND, "Latency of individual removes that are part of merge operations"), // TODO Vespa 9: Update metric name to include 'merge' + VDS_FILESTOR_ALLTHREADS_MERGE_PUT_LATENCY("vds.filestor.allthreads.merge_put_latency", Unit.MILLISECOND, "Latency of individual puts that are part of merge operations"), + VDS_FILESTOR_ALLTHREADS_MERGE_REMOVE_LATENCY("vds.filestor.allthreads.merge_remove_latency", Unit.MILLISECOND, "Latency of individual removes that are part of merge operations"), VDS_FILESTOR_ALLSTRIPES_THROTTLED_RPC_DIRECT_DISPATCHES("vds.filestor.allstripes.throttled_rpc_direct_dispatches", Unit.INSTANCE, "Number of times an RPC thread could not directly dispatch an async operation directly to Proton because it was disallowed by the throttle policy"), VDS_FILESTOR_ALLSTRIPES_THROTTLED_PERSISTENCE_THREAD_POLLS("vds.filestor.allstripes.throttled_persistence_thread_polls", Unit.INSTANCE, "Number of times a persistence thread could not immediately dispatch a queued async operation because it was disallowed by the throttle policy"), VDS_FILESTOR_ALLSTRIPES_TIMEOUTS_WAITING_FOR_THROTTLE_TOKEN("vds.filestor.allstripes.timeouts_waiting_for_throttle_token", Unit.INSTANCE, "Number of times a persistence thread timed out waiting for an available throttle policy token"), diff --git a/parent/pom.xml b/parent/pom.xml index eec97060a88..2f7eabab3e4 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -915,7 +915,7 @@ <dependency> <groupId>org.apache.maven.shared</groupId> <artifactId>maven-dependency-tree</artifactId> - <version>3.2.1</version> + <version>${maven-dependency-tree.vespa.version}</version> </dependency> <dependency> <groupId>org.apache.maven.shared</groupId> diff --git a/searchlib/src/vespa/searchlib/queryeval/hitcollector.cpp b/searchlib/src/vespa/searchlib/queryeval/hitcollector.cpp index 3154f95bbe1..c1d59463ad9 100644 --- a/searchlib/src/vespa/searchlib/queryeval/hitcollector.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/hitcollector.cpp @@ -193,6 +193,33 @@ struct NoRescorer }; template <typename Rescorer> +class RerankRescorer { + Rescorer _rescorer; + using HitVector = std::vector<HitCollector::Hit>; + using Iterator = typename HitVector::const_iterator; + Iterator _reranked_cur; + Iterator _reranked_end; +public: + RerankRescorer(const Rescorer& rescorer, + const HitVector& reranked_hits) + : _rescorer(rescorer), + _reranked_cur(reranked_hits.begin()), + _reranked_end(reranked_hits.end()) + { + } + + double rescore(uint32_t docid, double score) noexcept { + if (_reranked_cur != _reranked_end && _reranked_cur->first == docid) { + double result = _reranked_cur->second; + ++_reranked_cur; + return result; + } else { + return _rescorer.rescore(docid, score); + } + } +}; + +template <typename Rescorer> void add_rescored_hits(ResultSet& rs, const std::vector<HitCollector::Hit>& hits, Rescorer rescorer) { @@ -203,6 +230,17 @@ add_rescored_hits(ResultSet& rs, const std::vector<HitCollector::Hit>& hits, Res template <typename Rescorer> void +add_rescored_hits(ResultSet& rs, const std::vector<HitCollector::Hit>& hits, const std::vector<HitCollector::Hit>& reranked_hits, Rescorer rescorer) +{ + if (reranked_hits.empty()) { + add_rescored_hits(rs, hits, rescorer); + } else { + add_rescored_hits(rs, hits, RerankRescorer(rescorer, reranked_hits)); + } +} + +template <typename Rescorer> +void mixin_rescored_hits(ResultSet& rs, const std::vector<HitCollector::Hit>& hits, const std::vector<uint32_t>& docids, double default_value, Rescorer rescorer) { auto hits_cur = hits.begin(); @@ -217,18 +255,14 @@ mixin_rescored_hits(ResultSet& rs, const std::vector<HitCollector::Hit>& hits, c } } +template <typename Rescorer> void -mergeHitsIntoResultSet(const std::vector<HitCollector::Hit> &hits, ResultSet &result) +mixin_rescored_hits(ResultSet& rs, const std::vector<HitCollector::Hit>& hits, const std::vector<uint32_t>& docids, double default_value, const std::vector<HitCollector::Hit>& reranked_hits, Rescorer rescorer) { - uint32_t rhCur(0); - uint32_t rhEnd(result.getArrayUsed()); - for (const auto &hit : hits) { - while (rhCur != rhEnd && result[rhCur].getDocId() != hit.first) { - // just set the iterators right - ++rhCur; - } - assert(rhCur != rhEnd); // the hits should be a subset of the hits in ranked hit array. - result[rhCur]._rankValue = hit.second; + if (reranked_hits.empty()) { + mixin_rescored_hits(rs, hits, docids, default_value, rescorer); + } else { + mixin_rescored_hits(rs, hits, docids, default_value, RerankRescorer(rescorer, reranked_hits)); } } @@ -247,9 +281,9 @@ HitCollector::getResultSet(HitRank default_value) if ( ! _collector->isDocIdCollector() ) { rs->allocArray(_hits.size()); if (needReScore) { - add_rescored_hits(*rs, _hits, rescorer); + add_rescored_hits(*rs, _hits, _reRankedHits, rescorer); } else { - add_rescored_hits(*rs, _hits, NoRescorer()); + add_rescored_hits(*rs, _hits, _reRankedHits, NoRescorer()); } } else { if (_unordered) { @@ -257,16 +291,12 @@ HitCollector::getResultSet(HitRank default_value) } rs->allocArray(_docIdVector.size()); if (needReScore) { - mixin_rescored_hits(*rs, _hits, _docIdVector, default_value, rescorer); + mixin_rescored_hits(*rs, _hits, _docIdVector, default_value, _reRankedHits, rescorer); } else { - mixin_rescored_hits(*rs, _hits, _docIdVector, default_value, NoRescorer()); + mixin_rescored_hits(*rs, _hits, _docIdVector, default_value, _reRankedHits, NoRescorer()); } } - if (!_reRankedHits.empty()) { - mergeHitsIntoResultSet(_reRankedHits, *rs); - } - if (_bitVector) { rs->setBitOverflow(std::move(_bitVector)); } diff --git a/storage/src/vespa/storage/persistence/filestorage/merge_handler_metrics.cpp b/storage/src/vespa/storage/persistence/filestorage/merge_handler_metrics.cpp index c3cb38bd7ac..582c69e943f 100644 --- a/storage/src/vespa/storage/persistence/filestorage/merge_handler_metrics.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/merge_handler_metrics.cpp @@ -20,8 +20,8 @@ MergeHandlerMetrics::MergeHandlerMetrics(metrics::MetricSet* owner) "current node.", owner), mergeAverageDataReceivedNeeded("mergeavgdatareceivedneeded", {}, "Amount of data transferred from previous node " "in chain that we needed to apply locally.", owner), - put_latency("put_latency", {}, "Latency of individual puts that are part of merge operations", owner), - remove_latency("remove_latency", {}, "Latency of individual removes that are part of merge operations", owner) + merge_put_latency("merge_put_latency", {}, "Latency of individual puts that are part of merge operations", owner), + merge_remove_latency("merge_remove_latency", {}, "Latency of individual removes that are part of merge operations", owner) {} MergeHandlerMetrics::~MergeHandlerMetrics() = default; diff --git a/storage/src/vespa/storage/persistence/filestorage/merge_handler_metrics.h b/storage/src/vespa/storage/persistence/filestorage/merge_handler_metrics.h index 44b85570357..a2d68011695 100644 --- a/storage/src/vespa/storage/persistence/filestorage/merge_handler_metrics.h +++ b/storage/src/vespa/storage/persistence/filestorage/merge_handler_metrics.h @@ -21,8 +21,8 @@ struct MergeHandlerMetrics { metrics::DoubleAverageMetric mergeAverageDataReceivedNeeded; // Individual operation metrics. These capture both count and latency sum, so // no need for explicit count metric on the side. - metrics::DoubleAverageMetric put_latency; - metrics::DoubleAverageMetric remove_latency; + metrics::DoubleAverageMetric merge_put_latency; + metrics::DoubleAverageMetric merge_remove_latency; // Iteration over metadata and document payload data is already covered by // the merge[Meta]Data(Read|Write)Latency metrics, so not repeated here. Can be // explicitly added if deemed required. diff --git a/storage/src/vespa/storage/persistence/mergehandler.cpp b/storage/src/vespa/storage/persistence/mergehandler.cpp index f6cb75ae8bb..b3207428f5f 100644 --- a/storage/src/vespa/storage/persistence/mergehandler.cpp +++ b/storage/src/vespa/storage/persistence/mergehandler.cpp @@ -527,14 +527,14 @@ MergeHandler::applyDiffEntry(std::shared_ptr<ApplyBucketDiffState> async_results document::DocumentId docId = doc->getId(); auto complete = std::make_unique<ApplyBucketDiffEntryComplete>(std::move(async_results), std::move(docId), std::move(throttle_token), "put", - _clock, _env._metrics.merge_handler_metrics.put_latency); + _clock, _env._metrics.merge_handler_metrics.merge_put_latency); _spi.putAsync(bucket, timestamp, std::move(doc), std::move(complete)); } else { std::vector<spi::IdAndTimestamp> ids; ids.emplace_back(document::DocumentId(e._docName), timestamp); auto complete = std::make_unique<ApplyBucketDiffEntryComplete>(std::move(async_results), ids[0].id, std::move(throttle_token), "remove", - _clock, _env._metrics.merge_handler_metrics.remove_latency); + _clock, _env._metrics.merge_handler_metrics.merge_remove_latency); _spi.removeAsync(bucket, std::move(ids), std::move(complete)); } } |