diff options
91 files changed, 1296 insertions, 972 deletions
diff --git a/config-model/src/main/java/com/yahoo/schema/derived/RawRankProfile.java b/config-model/src/main/java/com/yahoo/schema/derived/RawRankProfile.java index 68fa0fe6de9..87b79ddcdc3 100644 --- a/config-model/src/main/java/com/yahoo/schema/derived/RawRankProfile.java +++ b/config-model/src/main/java/com/yahoo/schema/derived/RawRankProfile.java @@ -301,28 +301,41 @@ public class RawRankProfile implements RankProfilesConfig.Producer { private void replaceFunctionFeatures(Set<ReferenceNode> features, SerializationContext context) { if (features == null) return; - Map<String, ReferenceNode> functionFeatures = new LinkedHashMap<>(); + Set<ReferenceNode> functionFeatures = new LinkedHashSet<>(); for (Iterator<ReferenceNode> i = features.iterator(); i.hasNext(); ) { ReferenceNode referenceNode = i.next(); // Is the feature a function? ExpressionFunction function = context.getFunction(referenceNode.getName()); if (function != null) { - String propertyName = RankingExpression.propertyName(referenceNode.getName()); - String expressionString = function.getBody().getRoot().toString(context).toString(); + if (referenceNode.getOutput() != null) { + throw new IllegalArgumentException("function " + referenceNode.getName() + + " cannot provide output " + referenceNode.getOutput() + + " demanded by feature " + referenceNode); + } + int needArgs = function.arguments().size(); + var useArgs = referenceNode.getArguments(); + if (needArgs != useArgs.size()) { + throw new IllegalArgumentException("function " + referenceNode.getName() + + " needs " + needArgs + + " arguments but gets " + useArgs.size() + + " from feature " + referenceNode); + } + var instance = function.expand(context, useArgs.expressions(), new java.util.ArrayDeque<>()); + String propertyName = RankingExpression.propertyName(instance.getName()); + String expressionString = instance.getExpressionString(); context.addFunctionSerialization(propertyName, expressionString); - function.returnType().ifPresent(t -> context.addFunctionTypeSerialization(referenceNode.getName(), t)); - var backendReferenceNode = new ReferenceNode(wrapInRankingExpression(referenceNode.getName()), - referenceNode.getArguments().expressions(), - referenceNode.getOutput()); + function.returnType().ifPresent(t -> context.addFunctionTypeSerialization(instance.getName(), t)); + String backendReference = wrapInRankingExpression(instance.getName()); + var backendReferenceNode = new ReferenceNode(backendReference, List.of(), null); // tell backend to map back to the name the user expects: - featureRenames.put(backendReferenceNode.toString(), referenceNode.toString()); - functionFeatures.put(referenceNode.getName(), backendReferenceNode); + featureRenames.put(backendReference, referenceNode.toString()); + functionFeatures.add(backendReferenceNode); i.remove(); // Will add the expanded one in next block } } // Then, replace the features that were functions - for (Map.Entry<String, ReferenceNode> e : functionFeatures.entrySet()) { - features.add(e.getValue()); + for (ReferenceNode mappedFun : functionFeatures) { + features.add(mappedFun); } } diff --git a/config-model/src/test/derived/rankingexpression/rank-profiles.cfg b/config-model/src/test/derived/rankingexpression/rank-profiles.cfg index b3257c962dd..87882eef273 100644 --- a/config-model/src/test/derived/rankingexpression/rank-profiles.cfg +++ b/config-model/src/test/derived/rankingexpression/rank-profiles.cfg @@ -582,3 +582,91 @@ rankprofile[].normalizer[].name "normalize@3221316369@rrank" rankprofile[].normalizer[].input "nativeRank" rankprofile[].normalizer[].algo RRANK rankprofile[].normalizer[].kparam 60.0 +rankprofile[].name "function-with-arg-as-summary-feature" +rankprofile[].fef.property[].name "vespa.type.feature.attribute(t1)" +rankprofile[].fef.property[].value "tensor(m{},v[3])" +rankprofile[].fef.property[].name "rankingExpression(plusOne@478d886d33f680d).rankingScript" +rankprofile[].fef.property[].value "41 + 1" +rankprofile[].fef.property[].name "rankingExpression(useAttr@93d0729be0db6c70.30effc5f9cc0df93).rankingScript" +rankprofile[].fef.property[].value "attribute(foo1) * 17" +rankprofile[].fef.property[].name "rankingExpression(plusOne).rankingScript" +rankprofile[].fef.property[].value "x + 1" +rankprofile[].fef.property[].name "rankingExpression(useAttr).rankingScript" +rankprofile[].fef.property[].value "attribute(name) * weight" +rankprofile[].fef.property[].name "vespa.rank.firstphase" +rankprofile[].fef.property[].value "nativeRank" +rankprofile[].fef.property[].name "vespa.summary.feature" +rankprofile[].fef.property[].value "attribute(t1)" +rankprofile[].fef.property[].name "vespa.summary.feature" +rankprofile[].fef.property[].value "rankingExpression(plusOne@478d886d33f680d)" +rankprofile[].fef.property[].name "vespa.summary.feature" +rankprofile[].fef.property[].value "rankingExpression(useAttr@93d0729be0db6c70.30effc5f9cc0df93)" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "rankingExpression(plusOne@478d886d33f680d)" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "plusOne(41)" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "rankingExpression(useAttr@93d0729be0db6c70.30effc5f9cc0df93)" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "useAttr(foo1,17)" +rankprofile[].fef.property[].name "vespa.type.attribute.t1" +rankprofile[].fef.property[].value "tensor(m{},v[3])" +rankprofile[].name "function-with-arg-in-global-phase" +rankprofile[].fef.property[].name "rankingExpression(useAttr@6598f1aecaec0a2d.40876484d21a389).rankingScript" +rankprofile[].fef.property[].value "attribute(t1) * 42" +rankprofile[].fef.property[].name "rankingExpression(plusOne@31852fecfab75f29).rankingScript" +rankprofile[].fef.property[].value "2 + 1" +rankprofile[].fef.property[].name "rankingExpression(useAttr@93d0729be0db6c70.fe12ed266262cc16).rankingScript" +rankprofile[].fef.property[].value "attribute(foo1) * 1.25" +rankprofile[].fef.property[].name "rankingExpression(withIndirect@93d0729be0db6c70).rankingScript" +rankprofile[].fef.property[].value "rankingExpression(useAttr@93d0729be0db6c70.fe12ed266262cc16)" +rankprofile[].fef.property[].name "rankingExpression(plusOne@4a2b16f9107d7185).rankingScript" +rankprofile[].fef.property[].value "attribute(foo2) + 1" +rankprofile[].fef.property[].name "vespa.type.feature.useAttr(t1,42)" +rankprofile[].fef.property[].value "tensor(m{},v[3])" +rankprofile[].fef.property[].name "rankingExpression(plusOne).rankingScript" +rankprofile[].fef.property[].value "x + 1" +rankprofile[].fef.property[].name "rankingExpression(useAttr).rankingScript" +rankprofile[].fef.property[].value "attribute(name) * weight" +rankprofile[].fef.property[].name "rankingExpression(useAttr@2e0b6bb9bf541103.fe12ed266262cc16).rankingScript" +rankprofile[].fef.property[].value "attribute(name) * 1.25" +rankprofile[].fef.property[].name "rankingExpression(withIndirect).rankingScript" +rankprofile[].fef.property[].value "rankingExpression(useAttr@2e0b6bb9bf541103.fe12ed266262cc16)" +rankprofile[].fef.property[].name "vespa.rank.firstphase" +rankprofile[].fef.property[].value "nativeRank" +rankprofile[].fef.property[].name "vespa.rank.globalphase" +rankprofile[].fef.property[].value "rankingExpression(globalphase)" +rankprofile[].fef.property[].name "rankingExpression(globalphase).rankingScript" +rankprofile[].fef.property[].value "reduce(rankingExpression(useAttr@6598f1aecaec0a2d.40876484d21a389) + rankingExpression(plusOne@31852fecfab75f29) + rankingExpression(withIndirect@93d0729be0db6c70) + rankingExpression(plusOne@4a2b16f9107d7185), sum)" +rankprofile[].fef.property[].name "vespa.match.feature" +rankprofile[].fef.property[].value "rankingExpression(plusOne@4a2b16f9107d7185)" +rankprofile[].fef.property[].name "vespa.match.feature" +rankprofile[].fef.property[].value "rankingExpression(plusOne@31852fecfab75f29)" +rankprofile[].fef.property[].name "vespa.match.feature" +rankprofile[].fef.property[].value "rankingExpression(withIndirect@93d0729be0db6c70)" +rankprofile[].fef.property[].name "vespa.match.feature" +rankprofile[].fef.property[].value "rankingExpression(useAttr@6598f1aecaec0a2d.40876484d21a389)" +rankprofile[].fef.property[].name "vespa.hidden.matchfeature" +rankprofile[].fef.property[].value "plusOne(2)" +rankprofile[].fef.property[].name "vespa.hidden.matchfeature" +rankprofile[].fef.property[].value "withIndirect(foo1)" +rankprofile[].fef.property[].name "vespa.hidden.matchfeature" +rankprofile[].fef.property[].value "useAttr(t1,42)" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "rankingExpression(plusOne@4a2b16f9107d7185)" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "plusOne(attribute(foo2))" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "rankingExpression(plusOne@31852fecfab75f29)" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "plusOne(2)" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "rankingExpression(withIndirect@93d0729be0db6c70)" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "withIndirect(foo1)" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "rankingExpression(useAttr@6598f1aecaec0a2d.40876484d21a389)" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "useAttr(t1,42)" +rankprofile[].fef.property[].name "vespa.type.attribute.t1" +rankprofile[].fef.property[].value "tensor(m{},v[3])" diff --git a/config-model/src/test/derived/rankingexpression/rankexpression.sd b/config-model/src/test/derived/rankingexpression/rankexpression.sd index 15537f1f9d0..1ccf74bfe17 100644 --- a/config-model/src/test/derived/rankingexpression/rankexpression.sd +++ b/config-model/src/test/derived/rankingexpression/rankexpression.sd @@ -469,4 +469,42 @@ schema rankexpression { match-features: nativeRank } + rank-profile function-with-arg-as-summary-feature { + function plusOne(x) { + expression: x + 1 + } + function useAttr(name, weight) { + expression: attribute(name) * weight + } + first-phase { + expression: nativeRank + } + summary-features { + attribute(t1) + plusOne(41) + useAttr(foo1, 17) + } + } + + rank-profile function-with-arg-in-global-phase { + function plusOne(x) { + expression: x + 1 + } + function useAttr(name, weight) { + expression: attribute(name) * weight + } + function withIndirect(name) { + expression: useAttr(name, 1.25) + } + first-phase { + expression: nativeRank + } + global-phase { + expression: sum(useAttr(t1, 42) + plusOne(2) + withIndirect(foo1) + plusOne(attribute(foo2))) + } + match-features { + plusOne(attribute(foo2)) + } + } + } diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java index 13584082ab8..b94fadec213 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java @@ -28,7 +28,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; @@ -108,7 +107,7 @@ public class MetricsPacketsHandler extends AbstractRequestHandler { return getMetricsArray(metricSetId); } if ("prometheus".equals(format)) { - return buildPrometheusOutput(); + return buildPrometheusOutput(metricSetId); } String output = getAllMetricsPackets(metricSetId) + "\n"; @@ -133,8 +132,9 @@ public class MetricsPacketsHandler extends AbstractRequestHandler { /** * Returns metrics in Prometheus format */ - private byte[] buildPrometheusOutput() throws IOException { - return PrometheusHelper.buildPrometheusOutput(getSnapshot(), applicationName, timer.currentTimeMillis()); + private byte[] buildPrometheusOutput(String metricSetId) throws IOException { + var metrics = getPacketsForSnapshot(getSnapshot(), metricSetId, applicationName, timer.currentTimeMillis()); + return PrometheusHelper.buildPrometheusOutput(metrics, timer.currentTimeMillis()); } private static String jsonToString(JsonNode jsonObject) throws JsonProcessingException { diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/PrometheusHelper.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/PrometheusHelper.java index 11ca9e228ec..b38bcfebb48 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/PrometheusHelper.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/state/PrometheusHelper.java @@ -1,82 +1,56 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.jdisc.state; +import com.fasterxml.jackson.databind.JsonNode; + import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.io.OutputStream; -import java.util.Map; +import java.util.List; -import static com.yahoo.container.jdisc.state.JsonUtil.sanitizeDouble; /** * @author olaa */ public class PrometheusHelper { - private static final String HELP_LINE = "# HELP %s \n# TYPE %s untyped\n"; + private static final String HELP_LINE = "# HELP %s\n# TYPE %s untyped\n"; private static final String METRIC_LINE = "%s{%s} %s %d\n"; + private static final String DIMENSION_KEY = "dimensions"; + private static final String METRIC_KEY = "metrics"; + private static final String APPLICATION_KEY = "application"; - protected static byte[] buildPrometheusOutput(MetricSnapshot metricSnapshot, String application, long timestamp) throws IOException { + protected static byte[] buildPrometheusOutput(List<JsonNode> metrics, long timestamp) throws IOException { var outputStream = new ByteArrayOutputStream(); - for (Map.Entry<MetricDimensions, MetricSet> snapshotEntry : metricSnapshot) { - var metricDimensions = snapshotEntry.getKey(); - var metricSet = snapshotEntry.getValue(); - + for (var metric : metrics) { + var metricDimensions = metric.get(DIMENSION_KEY); var dimensionBuilder = new StringBuilder(); - for (var dimension : metricDimensions) { + for (var it = metricDimensions.fieldNames(); it.hasNext(); ) { + var dimension = it.next(); dimensionBuilder - .append(sanitize(dimension.getKey())) + .append(sanitize(dimension)) .append("=\"") - .append(dimension.getValue()) + .append(metricDimensions.get(dimension).asText()) .append("\","); } + var application = metric.get(APPLICATION_KEY).asText(); dimensionBuilder.append("vespa_service=\"").append(application).append("\","); var dimensions = dimensionBuilder.toString(); - - for (var metric : metricSet) { - var metricName = metric.getKey(); - var metricValue = metric.getValue(); - - if (metricValue instanceof CountMetric) { - var sanitizedMetricName = getSanitizedMetricName(metricName, "count"); - var value = ((CountMetric) metricValue).getCount(); - outputStream.write(getMetricLines(sanitizedMetricName, dimensions, value, timestamp)); - } else if (metricValue instanceof GaugeMetric) { - var gauge = (GaugeMetric) metricValue; - writeGaugeMetrics(outputStream, metricName, gauge, dimensions, timestamp); - } + var metricValues = metric.get(METRIC_KEY); + for (var it = metricValues.fieldNames(); it.hasNext(); ) { + var metricName = it.next(); + var metricVal = metricValues.get(metricName).numberValue(); + outputStream.write(getMetricLines(sanitize(metricName), dimensions, metricVal, timestamp)); } } return outputStream.toByteArray(); } - private static void writeGaugeMetrics(OutputStream outputStream, String metricName, GaugeMetric gaugeMetric, String dimensions, long timestamp) throws IOException { - var sanitizedMetricName = getSanitizedMetricName(metricName, "last"); - var value = sanitizeDouble(gaugeMetric.getLast()); - outputStream.write(getMetricLines(sanitizedMetricName, dimensions, value, timestamp)); - - /* - For now - only push "last" value - to limit metric volume - sanitizedMetricName = getSanitizedMetricName(metricName, "average"); - value = sanitizeDouble(gaugeMetric.getAverage()); - outputStream.write(getMetricLines(sanitizedMetricName, dimensions, value, timestamp)); - - sanitizedMetricName = getSanitizedMetricName(metricName, "max"); - value = sanitizeDouble(gaugeMetric.getMax()); - outputStream.write(getMetricLines(sanitizedMetricName, dimensions, value, timestamp)); - */ - } - private static byte[] getMetricLines(String metricName, String dimensions, Number value, long timestamp) { return (String.format(HELP_LINE, metricName, metricName) + String.format(METRIC_LINE, metricName, dimensions, value, timestamp)).getBytes(); } - private static String getSanitizedMetricName(String metricName, String suffix) { - return sanitize(metricName) + "_" + suffix; - } - private static String sanitize(String name) { return name.replaceAll("([-.])", "_"); } diff --git a/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java b/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java index 807e58918da..3eeb8cb7e80 100644 --- a/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java +++ b/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java @@ -7,6 +7,8 @@ import com.yahoo.container.jdisc.RequestHandlerTestDriver; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -146,22 +148,23 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { } @Test - public void prometheus_metrics() { + public void prometheus_metrics() throws Exception { var context = StateMetricContext.newInstance(Map.of("dim-1", "value1")); var snapshot = new MetricSnapshot(); snapshot.set(context, "gauge.metric", 0.2); snapshot.add(context, "counter.metric", 5); + snapshot.add(context, "configserver.requests", 120); + // Infrastructure set only contains max and average + snapshot.set(context, "lockAttempt.lockedLoad", 500); snapshotProvider.setSnapshot(snapshot); + var response = requestAsString("http://localhost/metrics-packets?format=prometheus"); - var expectedResponse = """ - # HELP gauge_metric_last\s - # TYPE gauge_metric_last untyped - gauge_metric_last{dim_1="value1",vespa_service="state-handler-test-base",} 0.2 0 - # HELP counter_metric_count\s - # TYPE counter_metric_count untyped - counter_metric_count{dim_1="value1",vespa_service="state-handler-test-base",} 5 0 - """; + var expectedResponse = readFile("prometheus-unfiltered"); assertEquals(expectedResponse, response); + + response = requestAsString("http://localhost/metrics-packets?format=prometheus&metric-set=infrastructure"); + expectedResponse = readFile("prometheus-filtered"); + assertTrue(response.startsWith(expectedResponse)); } @Test @@ -262,4 +265,8 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { snapshotProvider.setSnapshot(snapshot); } + private String readFile(String fileName) throws Exception { + return Files.readString(Path.of("src/test/resources/metrics-packets-handler-responses/" + fileName + ".txt")); + } + } diff --git a/container-core/src/test/resources/metrics-packets-handler-responses/prometheus-filtered.txt b/container-core/src/test/resources/metrics-packets-handler-responses/prometheus-filtered.txt new file mode 100644 index 00000000000..12e5e863e8a --- /dev/null +++ b/container-core/src/test/resources/metrics-packets-handler-responses/prometheus-filtered.txt @@ -0,0 +1,11 @@ +# HELP configserver_requests_count +# TYPE configserver_requests_count untyped +configserver_requests_count{dim_1="value1",host="some-hostname",vespa_service="state-handler-test-base",} 120 0 +# HELP lockAttempt_lockedLoad_average +# TYPE lockAttempt_lockedLoad_average untyped +lockAttempt_lockedLoad_average{dim_1="value1",host="some-hostname",vespa_service="state-handler-test-base",} 500.0 0 +# HELP lockAttempt_lockedLoad_max +# TYPE lockAttempt_lockedLoad_max untyped +lockAttempt_lockedLoad_max{dim_1="value1",host="some-hostname",vespa_service="state-handler-test-base",} 500.0 0 +# HELP alive +# TYPE alive untyped diff --git a/container-core/src/test/resources/metrics-packets-handler-responses/prometheus-unfiltered.txt b/container-core/src/test/resources/metrics-packets-handler-responses/prometheus-unfiltered.txt new file mode 100644 index 00000000000..1fa14284bf5 --- /dev/null +++ b/container-core/src/test/resources/metrics-packets-handler-responses/prometheus-unfiltered.txt @@ -0,0 +1,42 @@ +# HELP gauge_metric_average +# TYPE gauge_metric_average untyped +gauge_metric_average{dim_1="value1",host="some-hostname",vespa_service="state-handler-test-base",} 0.2 0 +# HELP gauge_metric_last +# TYPE gauge_metric_last untyped +gauge_metric_last{dim_1="value1",host="some-hostname",vespa_service="state-handler-test-base",} 0.2 0 +# HELP gauge_metric_max +# TYPE gauge_metric_max untyped +gauge_metric_max{dim_1="value1",host="some-hostname",vespa_service="state-handler-test-base",} 0.2 0 +# HELP gauge_metric_min +# TYPE gauge_metric_min untyped +gauge_metric_min{dim_1="value1",host="some-hostname",vespa_service="state-handler-test-base",} 0.2 0 +# HELP gauge_metric_sum +# TYPE gauge_metric_sum untyped +gauge_metric_sum{dim_1="value1",host="some-hostname",vespa_service="state-handler-test-base",} 0.2 0 +# HELP gauge_metric_count +# TYPE gauge_metric_count untyped +gauge_metric_count{dim_1="value1",host="some-hostname",vespa_service="state-handler-test-base",} 1 0 +# HELP configserver_requests_count +# TYPE configserver_requests_count untyped +configserver_requests_count{dim_1="value1",host="some-hostname",vespa_service="state-handler-test-base",} 120 0 +# HELP lockAttempt_lockedLoad_average +# TYPE lockAttempt_lockedLoad_average untyped +lockAttempt_lockedLoad_average{dim_1="value1",host="some-hostname",vespa_service="state-handler-test-base",} 500.0 0 +# HELP lockAttempt_lockedLoad_last +# TYPE lockAttempt_lockedLoad_last untyped +lockAttempt_lockedLoad_last{dim_1="value1",host="some-hostname",vespa_service="state-handler-test-base",} 500.0 0 +# HELP lockAttempt_lockedLoad_max +# TYPE lockAttempt_lockedLoad_max untyped +lockAttempt_lockedLoad_max{dim_1="value1",host="some-hostname",vespa_service="state-handler-test-base",} 500.0 0 +# HELP lockAttempt_lockedLoad_min +# TYPE lockAttempt_lockedLoad_min untyped +lockAttempt_lockedLoad_min{dim_1="value1",host="some-hostname",vespa_service="state-handler-test-base",} 500.0 0 +# HELP lockAttempt_lockedLoad_sum +# TYPE lockAttempt_lockedLoad_sum untyped +lockAttempt_lockedLoad_sum{dim_1="value1",host="some-hostname",vespa_service="state-handler-test-base",} 500.0 0 +# HELP lockAttempt_lockedLoad_count +# TYPE lockAttempt_lockedLoad_count untyped +lockAttempt_lockedLoad_count{dim_1="value1",host="some-hostname",vespa_service="state-handler-test-base",} 1 0 +# HELP counter_metric_count +# TYPE counter_metric_count untyped +counter_metric_count{dim_1="value1",host="some-hostname",vespa_service="state-handler-test-base",} 5 0 diff --git a/container-search/src/main/java/com/yahoo/search/ranking/FunEvalSpec.java b/container-search/src/main/java/com/yahoo/search/ranking/FunEvalSpec.java index df9c509dd82..ac1b7c8e218 100644 --- a/container-search/src/main/java/com/yahoo/search/ranking/FunEvalSpec.java +++ b/container-search/src/main/java/com/yahoo/search/ranking/FunEvalSpec.java @@ -4,4 +4,7 @@ package com.yahoo.search.ranking; import java.util.List; import java.util.function.Supplier; -record FunEvalSpec(Supplier<Evaluator> evalSource, List<String> fromQuery, List<String> fromMF) {} +record FunEvalSpec(Supplier<Evaluator> evalSource, + List<String> fromQuery, + List<MatchFeatureInput> fromMF) +{} diff --git a/container-search/src/main/java/com/yahoo/search/ranking/GlobalPhaseSetup.java b/container-search/src/main/java/com/yahoo/search/ranking/GlobalPhaseSetup.java index 7340e9e2a5e..7783eabcdcc 100644 --- a/container-search/src/main/java/com/yahoo/search/ranking/GlobalPhaseSetup.java +++ b/container-search/src/main/java/com/yahoo/search/ranking/GlobalPhaseSetup.java @@ -96,6 +96,41 @@ class GlobalPhaseSetup { return defaultValues; } + static class InputResolver { + final List<String> usedNormalizers = new ArrayList<>(); + final List<String> fromQuery = new ArrayList<>(); + final List<MatchFeatureInput> fromMF = new ArrayList<>(); + private final Set<String> availableMatchFeatures; + private final Map<String, String> renamedFeatures; + private final Set<String> availableNormalizers; + + InputResolver(Set<String> availableMatchFeatures, + Map<String, String> renamedFeatures, + Set<String> availableNormalizers) + { + this.availableMatchFeatures = availableMatchFeatures; + this.renamedFeatures = renamedFeatures; + this.availableNormalizers = availableNormalizers; + } + void resolve(Collection<String> allInputs) { + for (var input : allInputs) { + String queryFeatureName = asQueryFeature(input); + if (queryFeatureName != null) { + fromQuery.add(queryFeatureName); + } else if (availableNormalizers.contains(input)) { + usedNormalizers.add(input); + } else if (availableMatchFeatures.contains(input)) { + String mfName = renamedFeatures.getOrDefault(input, input); + fromMF.add(new MatchFeatureInput(input, mfName)); + } else if (renamedFeatures.values().contains(input)) { + fromMF.add(new MatchFeatureInput(input, input)); + } else { + throw new IllegalArgumentException("Bad config, missing global-phase input: " + input); + } + } + } + } + static GlobalPhaseSetup maybeMakeSetup(RankProfilesConfig.Rankprofile rp, RankProfilesEvaluator modelEvaluator) { var model = modelEvaluator.modelForRankProfile(rp.name()); Map<String, RankProfilesConfig.Rankprofile.Normalizer> availableNormalizers = new HashMap<>(); @@ -130,47 +165,31 @@ class GlobalPhaseSetup { } } } - for (var entry : renameFeatures.entrySet()) { - String old = entry.getKey(); - if (matchFeatures.contains(old)) { - matchFeatures.remove(old); - matchFeatures.add(entry.getValue()); - } - } if (rerankCount < 0) { rerankCount = 100; } if (functionEvaluatorSource != null) { + var mainResolver = new InputResolver(matchFeatures, renameFeatures, availableNormalizers.keySet()); var evaluator = functionEvaluatorSource.get(); var allInputs = List.copyOf(evaluator.function().arguments()); - List<String> fromMF = new ArrayList<>(); - List<String> fromQuery = new ArrayList<>(); + mainResolver.resolve(allInputs); List<NormalizerSetup> normalizers = new ArrayList<>(); - for (var input : allInputs) { - String queryFeatureName = asQueryFeature(input); - if (queryFeatureName != null) { - fromQuery.add(queryFeatureName); - } else if (availableNormalizers.containsKey(input)) { - var cfg = availableNormalizers.get(input); - String normInput = cfg.input(); - if (matchFeatures.contains(normInput)) { - Supplier<Evaluator> normSource = () -> new DummyEvaluator(normInput); - normalizers.add(makeNormalizerSetup(cfg, matchFeatures, normSource, List.of(normInput), rerankCount)); - } else { - Supplier<FunctionEvaluator> normSource = () -> model.evaluatorOf(normInput); - var normInputs = List.copyOf(normSource.get().function().arguments()); - var normSupplier = SimpleEvaluator.wrap(normSource); - normalizers.add(makeNormalizerSetup(cfg, matchFeatures, normSupplier, normInputs, rerankCount)); - } - } else if (matchFeatures.contains(input) || matchFeatures.contains(WrappedHit.alternate(input))) { - fromMF.add(input); + for (var input : mainResolver.usedNormalizers) { + var cfg = availableNormalizers.get(input); + String normInput = cfg.input(); + if (matchFeatures.contains(normInput) || renameFeatures.values().contains(normInput)) { + Supplier<Evaluator> normSource = () -> new DummyEvaluator(normInput); + normalizers.add(makeNormalizerSetup(cfg, matchFeatures, renameFeatures, normSource, List.of(normInput), rerankCount)); } else { - throw new IllegalArgumentException("Bad config, missing global-phase input: " + input); + Supplier<FunctionEvaluator> normSource = () -> model.evaluatorOf(normInput); + var normInputs = List.copyOf(normSource.get().function().arguments()); + var normSupplier = SimpleEvaluator.wrap(normSource); + normalizers.add(makeNormalizerSetup(cfg, matchFeatures, renameFeatures, normSupplier, normInputs, rerankCount)); } } Supplier<Evaluator> supplier = SimpleEvaluator.wrap(functionEvaluatorSource); - var gfun = new FunEvalSpec(supplier, fromQuery, fromMF); - var defaultValues = extraDefaultQueryFeatureValues(rp, fromQuery, normalizers); + var gfun = new FunEvalSpec(supplier, mainResolver.fromQuery, mainResolver.fromMF); + var defaultValues = extraDefaultQueryFeatureValues(rp, mainResolver.fromQuery, normalizers); return new GlobalPhaseSetup(gfun, rerankCount, namesToHide, normalizers, defaultValues); } return null; @@ -178,23 +197,14 @@ class GlobalPhaseSetup { private static NormalizerSetup makeNormalizerSetup(RankProfilesConfig.Rankprofile.Normalizer cfg, Set<String> matchFeatures, + Map<String, String> renamedFeatures, Supplier<Evaluator> evalSupplier, List<String> normInputs, int rerankCount) { - List<String> fromQuery = new ArrayList<>(); - List<String> fromMF = new ArrayList<>(); - for (var input : normInputs) { - String queryFeatureName = asQueryFeature(input); - if (queryFeatureName != null) { - fromQuery.add(queryFeatureName); - } else if (matchFeatures.contains(input) || matchFeatures.contains(WrappedHit.alternate(input))) { - fromMF.add(input); - } else { - throw new IllegalArgumentException("Bad config, missing normalizer input: " + input); - } - } - var fun = new FunEvalSpec(evalSupplier, fromQuery, fromMF); + var normResolver = new InputResolver(matchFeatures, renamedFeatures, Set.of()); + normResolver.resolve(normInputs); + var fun = new FunEvalSpec(evalSupplier, normResolver.fromQuery, normResolver.fromMF); return new NormalizerSetup(cfg.name(), makeNormalizerSupplier(cfg, rerankCount), fun); } diff --git a/container-search/src/main/java/com/yahoo/search/ranking/HitRescorer.java b/container-search/src/main/java/com/yahoo/search/ranking/HitRescorer.java index fee4f5b4160..32eaa4a29c4 100644 --- a/container-search/src/main/java/com/yahoo/search/ranking/HitRescorer.java +++ b/container-search/src/main/java/com/yahoo/search/ranking/HitRescorer.java @@ -14,10 +14,12 @@ class HitRescorer { private static final Logger logger = Logger.getLogger(HitRescorer.class.getName()); private final Supplier<Evaluator> mainEvalSrc; - private final List<String> mainFromMF; + private final List<MatchFeatureInput> mainFromMF; private final List<NormalizerContext> normalizers; - public HitRescorer(Supplier<Evaluator> mainEvalSrc, List<String> mainFromMF, List<NormalizerContext> normalizers) { + public HitRescorer(Supplier<Evaluator> mainEvalSrc, + List<MatchFeatureInput> mainFromMF, + List<NormalizerContext> normalizers) { this.mainEvalSrc = mainEvalSrc; this.mainFromMF = mainFromMF; this.normalizers = normalizers; @@ -48,13 +50,13 @@ class HitRescorer { return newScore; } - private static double evalScorer(WrappedHit wrapped, Evaluator scorer, List<String> fromMF) { - for (String argName : fromMF) { - var asTensor = wrapped.getTensor(argName); + private static double evalScorer(WrappedHit wrapped, Evaluator scorer, List<MatchFeatureInput> fromMF) { + for (var argSpec : fromMF) { + var asTensor = wrapped.getTensor(argSpec.matchFeatureName()); if (asTensor != null) { - scorer.bind(argName, asTensor); + scorer.bind(argSpec.inputName(), asTensor); } else { - logger.warning("Missing match-feature for Evaluator argument: " + argName); + logger.warning("Missing match-feature for Evaluator argument: " + argSpec.inputName()); return 0.0; } } diff --git a/container-search/src/main/java/com/yahoo/search/ranking/MatchFeatureInput.java b/container-search/src/main/java/com/yahoo/search/ranking/MatchFeatureInput.java new file mode 100644 index 00000000000..f80f29b3668 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/ranking/MatchFeatureInput.java @@ -0,0 +1,4 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.ranking; + +record MatchFeatureInput(String inputName, String matchFeatureName) {} diff --git a/container-search/src/main/java/com/yahoo/search/ranking/NormalizerContext.java b/container-search/src/main/java/com/yahoo/search/ranking/NormalizerContext.java index 9438b5ea824..ceac202db47 100644 --- a/container-search/src/main/java/com/yahoo/search/ranking/NormalizerContext.java +++ b/container-search/src/main/java/com/yahoo/search/ranking/NormalizerContext.java @@ -4,4 +4,8 @@ package com.yahoo.search.ranking; import java.util.List; import java.util.function.Supplier; -record NormalizerContext(String name, Normalizer normalizer, Supplier<Evaluator> evalSource, List<String> fromMF) {} +record NormalizerContext(String name, + Normalizer normalizer, + Supplier<Evaluator> evalSource, + List<MatchFeatureInput> fromMF) +{} diff --git a/container-search/src/test/java/com/yahoo/search/ranking/GlobalPhaseRerankHitsImplTest.java b/container-search/src/test/java/com/yahoo/search/ranking/GlobalPhaseRerankHitsImplTest.java index f55130c0c93..39b202daf1e 100644 --- a/container-search/src/test/java/com/yahoo/search/ranking/GlobalPhaseRerankHitsImplTest.java +++ b/container-search/src/test/java/com/yahoo/search/ranking/GlobalPhaseRerankHitsImplTest.java @@ -38,7 +38,11 @@ public class GlobalPhaseRerankHitsImplTest { return new FunEvalSpec(() -> new EvalSum(constValue), Collections.emptyList(), Collections.emptyList()); } static FunEvalSpec makeSumSpec(List<String> fromQuery, List<String> fromMF) { - return new FunEvalSpec(() -> new EvalSum(0.0), fromQuery, fromMF); + List<MatchFeatureInput> mfList = new ArrayList<>(); + for (String mf : fromMF) { + mfList.add(new MatchFeatureInput(mf, mf)); + } + return new FunEvalSpec(() -> new EvalSum(0.0), fromQuery, mfList); } static class ExpectingNormalizer extends Normalizer { List<Double> expected; diff --git a/container-search/src/test/java/com/yahoo/search/ranking/GlobalPhaseSetupTest.java b/container-search/src/test/java/com/yahoo/search/ranking/GlobalPhaseSetupTest.java index 082531a97dd..4db73c5467e 100644 --- a/container-search/src/test/java/com/yahoo/search/ranking/GlobalPhaseSetupTest.java +++ b/container-search/src/test/java/com/yahoo/search/ranking/GlobalPhaseSetupTest.java @@ -32,7 +32,13 @@ public class GlobalPhaseSetupTest { assertEquals(0, setup.normalizers.size()); assertEquals(9, setup.matchFeaturesToHide.size()); assertEquals(1, setup.globalPhaseEvalSpec.fromQuery().size()); - assertEquals(9, setup.globalPhaseEvalSpec.fromMF().size()); + var wantMF = setup.globalPhaseEvalSpec.fromMF(); + assertEquals(8, wantMF.size()); + wantMF.sort((a, b) -> a.matchFeatureName().compareTo(b.matchFeatureName())); + assertEquals("attribute(t1)", wantMF.get(0).matchFeatureName()); + assertEquals("attribute(t1)", wantMF.get(0).inputName()); + assertEquals("myplus", wantMF.get(5).matchFeatureName()); + assertEquals("rankingExpression(myplus)", wantMF.get(5).inputName()); } @Test void queryFeaturesWithDefaults() { @@ -67,28 +73,28 @@ public class GlobalPhaseSetupTest { assertEquals("normalize@2974853441@linear", n.name()); assertEquals(0, n.inputEvalSpec().fromQuery().size()); assertEquals(1, n.inputEvalSpec().fromMF().size()); - assertEquals("funmf", n.inputEvalSpec().fromMF().get(0)); + assertEquals("funmf", n.inputEvalSpec().fromMF().get(0).matchFeatureName()); assertEquals("linear", n.supplier().get().normalizing()); n = nList.get(1); assertEquals("normalize@3414032797@rrank", n.name()); assertEquals(0, n.inputEvalSpec().fromQuery().size()); assertEquals(1, n.inputEvalSpec().fromMF().size()); - assertEquals("attribute(year)", n.inputEvalSpec().fromMF().get(0)); + assertEquals("attribute(year)", n.inputEvalSpec().fromMF().get(0).inputName()); assertEquals("reciprocal-rank{k:60.0}", n.supplier().get().normalizing()); n = nList.get(2); assertEquals("normalize@3551296680@linear", n.name()); assertEquals(0, n.inputEvalSpec().fromQuery().size()); assertEquals(1, n.inputEvalSpec().fromMF().size()); - assertEquals("nativeRank", n.inputEvalSpec().fromMF().get(0)); + assertEquals("nativeRank", n.inputEvalSpec().fromMF().get(0).inputName()); assertEquals("linear", n.supplier().get().normalizing()); n = nList.get(3); assertEquals("normalize@4280591309@rrank", n.name()); assertEquals(0, n.inputEvalSpec().fromQuery().size()); assertEquals(1, n.inputEvalSpec().fromMF().size()); - assertEquals("bm25(myabstract)", n.inputEvalSpec().fromMF().get(0)); + assertEquals("bm25(myabstract)", n.inputEvalSpec().fromMF().get(0).inputName()); assertEquals("reciprocal-rank{k:42.0}", n.supplier().get().normalizing()); n = nList.get(4); @@ -96,24 +102,42 @@ public class GlobalPhaseSetupTest { assertEquals(1, n.inputEvalSpec().fromQuery().size()); assertEquals("myweight", n.inputEvalSpec().fromQuery().get(0)); assertEquals(1, n.inputEvalSpec().fromMF().size()); - assertEquals("attribute(foo1)", n.inputEvalSpec().fromMF().get(0)); + assertEquals("attribute(foo1)", n.inputEvalSpec().fromMF().get(0).inputName()); assertEquals("linear", n.supplier().get().normalizing()); n = nList.get(5); assertEquals("normalize@4640646880@linear", n.name()); assertEquals(0, n.inputEvalSpec().fromQuery().size()); assertEquals(1, n.inputEvalSpec().fromMF().size()); - assertEquals("attribute(foo1)", n.inputEvalSpec().fromMF().get(0)); + assertEquals("attribute(foo1)", n.inputEvalSpec().fromMF().get(0).inputName()); assertEquals("linear", n.supplier().get().normalizing()); n = nList.get(6); assertEquals("normalize@6283155534@linear", n.name()); assertEquals(0, n.inputEvalSpec().fromQuery().size()); assertEquals(1, n.inputEvalSpec().fromMF().size()); - assertEquals("bm25(mytitle)", n.inputEvalSpec().fromMF().get(0)); + assertEquals("bm25(mytitle)", n.inputEvalSpec().fromMF().get(0).inputName()); assertEquals("linear", n.supplier().get().normalizing()); } + @Test void funcWithArgsSetup() { + RankProfilesConfig rpCfg = readConfig("with_mf_funargs"); + assertEquals(1, rpCfg.rankprofile().size()); + RankProfilesEvaluator rpEvaluator = createEvaluator(rpCfg); + var setup = GlobalPhaseSetup.maybeMakeSetup(rpCfg.rankprofile().get(0), rpEvaluator); + assertNotNull(setup); + assertEquals(0, setup.normalizers.size()); + assertEquals(3, setup.matchFeaturesToHide.size()); + assertEquals(0, setup.globalPhaseEvalSpec.fromQuery().size()); + var wantMF = setup.globalPhaseEvalSpec.fromMF(); + assertEquals(4, wantMF.size()); + wantMF.sort((a, b) -> a.matchFeatureName().compareTo(b.matchFeatureName())); + assertEquals("plusOne(2)", wantMF.get(0).matchFeatureName()); + assertEquals("plusOne(attribute(foo2))", wantMF.get(1).matchFeatureName()); + assertEquals("useAttr(t1,42)", wantMF.get(2).matchFeatureName()); + assertEquals("withIndirect(foo1)", wantMF.get(3).matchFeatureName()); + } + private RankProfilesEvaluator createEvaluator(RankProfilesConfig config) { RankingConstantsConfig constantsConfig = new RankingConstantsConfig.Builder().build(); RankingExpressionsConfig expressionsConfig = new RankingExpressionsConfig.Builder().build(); diff --git a/container-search/src/test/resources/config/medium/rank-profiles.cfg b/container-search/src/test/resources/config/medium/rank-profiles.cfg index 5a609f70cef..528946cb4cd 100644 --- a/container-search/src/test/resources/config/medium/rank-profiles.cfg +++ b/container-search/src/test/resources/config/medium/rank-profiles.cfg @@ -26,7 +26,7 @@ rankprofile[0].fef.property[11].value "firstPhase" rankprofile[0].fef.property[12].name "vespa.match.feature" rankprofile[0].fef.property[12].value "attribute(t1)" rankprofile[0].fef.property[13].name "vespa.match.feature" -rankprofile[0].fef.property[13].value "attribute(foo1)" +rankprofile[0].fef.property[13].value "rankingExpression(myplus)" rankprofile[0].fef.property[14].name "vespa.match.feature" rankprofile[0].fef.property[14].value "fieldTermMatch(title,0).occurrences" rankprofile[0].fef.property[15].name "vespa.match.feature" @@ -44,7 +44,7 @@ rankprofile[0].fef.property[20].value "firstPhase" rankprofile[0].fef.property[21].name "vespa.hidden.matchfeature" rankprofile[0].fef.property[21].value "attribute(t1)" rankprofile[0].fef.property[22].name "vespa.hidden.matchfeature" -rankprofile[0].fef.property[22].value "attribute(foo1)" +rankprofile[0].fef.property[22].value "rankingExpression(myplus)" rankprofile[0].fef.property[23].name "vespa.hidden.matchfeature" rankprofile[0].fef.property[23].value "fieldTermMatch(title,0).occurrences" rankprofile[0].fef.property[24].name "vespa.hidden.matchfeature" @@ -53,3 +53,7 @@ rankprofile[0].fef.property[25].name "vespa.globalphase.rerankcount" rankprofile[0].fef.property[25].value "42" rankprofile[0].fef.property[26].name "vespa.type.attribute.t1" rankprofile[0].fef.property[26].value "tensor(m{},v[3])" +rankprofile[0].fef.property[27].name "vespa.feature.rename" +rankprofile[0].fef.property[27].value "rankingExpression(myplus)" +rankprofile[0].fef.property[28].name "vespa.feature.rename" +rankprofile[0].fef.property[28].value "myplus" diff --git a/container-search/src/test/resources/config/with_mf_funargs/rank-profiles.cfg b/container-search/src/test/resources/config/with_mf_funargs/rank-profiles.cfg new file mode 100644 index 00000000000..9acf22f76e5 --- /dev/null +++ b/container-search/src/test/resources/config/with_mf_funargs/rank-profiles.cfg @@ -0,0 +1,59 @@ +rankprofile[0].name "function-with-arg-in-global-phase" +rankprofile[0].fef.property[0].name "rankingExpression(useAttr@6598f1aecaec0a2d.40876484d21a389).rankingScript" +rankprofile[0].fef.property[0].value "attribute(t1) * 42" +rankprofile[0].fef.property[1].name "rankingExpression(plusOne@31852fecfab75f29).rankingScript" +rankprofile[0].fef.property[1].value "2 + 1" +rankprofile[0].fef.property[2].name "rankingExpression(useAttr@93d0729be0db6c70.fe12ed266262cc16).rankingScript" +rankprofile[0].fef.property[2].value "attribute(foo1) * 1.25" +rankprofile[0].fef.property[3].name "rankingExpression(withIndirect@93d0729be0db6c70).rankingScript" +rankprofile[0].fef.property[3].value "rankingExpression(useAttr@93d0729be0db6c70.fe12ed266262cc16)" +rankprofile[0].fef.property[4].name "rankingExpression(plusOne@4a2b16f9107d7185).rankingScript" +rankprofile[0].fef.property[4].value "attribute(foo2) + 1" +rankprofile[0].fef.property[5].name "vespa.type.feature.useAttr(t1,42)" +rankprofile[0].fef.property[5].value "tensor(m{},v[3])" +rankprofile[0].fef.property[6].name "rankingExpression(plusOne).rankingScript" +rankprofile[0].fef.property[6].value "x + 1" +rankprofile[0].fef.property[7].name "rankingExpression(useAttr).rankingScript" +rankprofile[0].fef.property[7].value "attribute(name) * weight" +rankprofile[0].fef.property[8].name "rankingExpression(useAttr@2e0b6bb9bf541103.fe12ed266262cc16).rankingScript" +rankprofile[0].fef.property[8].value "attribute(name) * 1.25" +rankprofile[0].fef.property[9].name "rankingExpression(withIndirect).rankingScript" +rankprofile[0].fef.property[9].value "rankingExpression(useAttr@2e0b6bb9bf541103.fe12ed266262cc16)" +rankprofile[0].fef.property[10].name "vespa.rank.firstphase" +rankprofile[0].fef.property[10].value "nativeRank" +rankprofile[0].fef.property[11].name "vespa.rank.globalphase" +rankprofile[0].fef.property[11].value "rankingExpression(globalphase)" +rankprofile[0].fef.property[12].name "rankingExpression(globalphase).rankingScript" +rankprofile[0].fef.property[12].value "reduce(rankingExpression(useAttr@6598f1aecaec0a2d.40876484d21a389) + rankingExpression(plusOne@31852fecfab75f29) + rankingExpression(withIndirect@93d0729be0db6c70) + rankingExpression(plusOne@4a2b16f9107d7185), sum)" +rankprofile[0].fef.property[13].name "vespa.match.feature" +rankprofile[0].fef.property[13].value "rankingExpression(plusOne@4a2b16f9107d7185)" +rankprofile[0].fef.property[14].name "vespa.match.feature" +rankprofile[0].fef.property[14].value "rankingExpression(plusOne@31852fecfab75f29)" +rankprofile[0].fef.property[15].name "vespa.match.feature" +rankprofile[0].fef.property[15].value "rankingExpression(withIndirect@93d0729be0db6c70)" +rankprofile[0].fef.property[16].name "vespa.match.feature" +rankprofile[0].fef.property[16].value "rankingExpression(useAttr@6598f1aecaec0a2d.40876484d21a389)" +rankprofile[0].fef.property[17].name "vespa.hidden.matchfeature" +rankprofile[0].fef.property[17].value "plusOne(2)" +rankprofile[0].fef.property[18].name "vespa.hidden.matchfeature" +rankprofile[0].fef.property[18].value "withIndirect(foo1)" +rankprofile[0].fef.property[19].name "vespa.hidden.matchfeature" +rankprofile[0].fef.property[19].value "useAttr(t1,42)" +rankprofile[0].fef.property[20].name "vespa.feature.rename" +rankprofile[0].fef.property[20].value "rankingExpression(plusOne@4a2b16f9107d7185)" +rankprofile[0].fef.property[21].name "vespa.feature.rename" +rankprofile[0].fef.property[21].value "plusOne(attribute(foo2))" +rankprofile[0].fef.property[22].name "vespa.feature.rename" +rankprofile[0].fef.property[22].value "rankingExpression(plusOne@31852fecfab75f29)" +rankprofile[0].fef.property[23].name "vespa.feature.rename" +rankprofile[0].fef.property[23].value "plusOne(2)" +rankprofile[0].fef.property[24].name "vespa.feature.rename" +rankprofile[0].fef.property[24].value "rankingExpression(withIndirect@93d0729be0db6c70)" +rankprofile[0].fef.property[25].name "vespa.feature.rename" +rankprofile[0].fef.property[25].value "withIndirect(foo1)" +rankprofile[0].fef.property[26].name "vespa.feature.rename" +rankprofile[0].fef.property[26].value "rankingExpression(useAttr@6598f1aecaec0a2d.40876484d21a389)" +rankprofile[0].fef.property[27].name "vespa.feature.rename" +rankprofile[0].fef.property[27].value "useAttr(t1,42)" +rankprofile[0].fef.property[28].name "vespa.type.attribute.t1" +rankprofile[0].fef.property[28].value "tensor(m{},v[3])" diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/AcceptedCountries.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/AcceptedCountries.java index c665b4fb7c2..082eaac7315 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/AcceptedCountries.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/AcceptedCountries.java @@ -13,7 +13,7 @@ public record AcceptedCountries(List<Country> countries) { countries = List.copyOf(countries); } - public record Country(String code, String displayName, List<TaxType> taxTypes) { + public record Country(String code, String displayName, boolean taxIdMandatory, List<TaxType> taxTypes) { public Country { taxTypes = List.copyOf(taxTypes); } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingReporter.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingReporter.java index 676c29cec5d..4aab4a47fc6 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingReporter.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingReporter.java @@ -7,7 +7,7 @@ import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; public interface BillingReporter { BillingReference maintainTenant(CloudTenant tenant); - InvoiceUpdate maintainInvoice(Bill bill); + InvoiceUpdate maintainInvoice(CloudTenant teannt, Bill bill); /** Export a bill to a payment service. Returns the invoice ID in the external system. */ default String exportBill(Bill bill, String exportMethod, CloudTenant tenant) { diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingReporterMock.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingReporterMock.java index 689ecc356dc..899b31da361 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingReporterMock.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingReporterMock.java @@ -7,6 +7,8 @@ import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; import java.math.BigDecimal; import java.time.Clock; import java.time.ZonedDateTime; +import java.util.HashMap; +import java.util.Map; import java.util.Optional; import java.util.UUID; @@ -14,6 +16,8 @@ public class BillingReporterMock implements BillingReporter { private final Clock clock; private final BillingDatabaseClient dbClient; + private final Map<Bill.Id, String> exportedBills = new HashMap<>(); + public BillingReporterMock(Clock clock, BillingDatabaseClient dbClient) { this.clock = clock; this.dbClient = dbClient; @@ -25,19 +29,29 @@ public class BillingReporterMock implements BillingReporter { } @Override - public InvoiceUpdate maintainInvoice(Bill bill) { - dbClient.addLineItem(bill.tenant(), maintainedMarkerItem(), Optional.of(bill.id())); - return new InvoiceUpdate(1,0,0); + public InvoiceUpdate maintainInvoice(CloudTenant tenant, Bill bill) { + if (exportedBills.containsKey(bill.id())) { + dbClient.addLineItem(bill.tenant(), maintainedMarkerItem(), Optional.of(bill.id())); + return ModifiableInvoiceUpdate.of(bill.id(), 1, 0, 0); + } else { + return FailedInvoiceUpdate.removed(bill.id()); + } } @Override public String exportBill(Bill bill, String exportMethod, CloudTenant tenant) { // Replace bill with a copy with exportedId set - var exportedId = "EXT-ID-123"; + var exportedId = "EXPORTED-" + bill.id().value(); + exportedBills.put(bill.id(), exportedId); dbClient.setExportedInvoiceId(bill.id(), exportedId); return exportedId; } + // Emulates deleting a bill in the external system. + public void deleteExportedBill(Bill.Id billId) { + exportedBills.remove(billId); + } + private static Bill.LineItem maintainedMarkerItem() { return new Bill.LineItem("maintained", "", BigDecimal.valueOf(0.0), "", "", ZonedDateTime.now()); } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/FailedInvoiceUpdate.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/FailedInvoiceUpdate.java new file mode 100644 index 00000000000..9a93c7b05ec --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/FailedInvoiceUpdate.java @@ -0,0 +1,28 @@ +package com.yahoo.vespa.hosted.controller.api.integration.billing; + +/** + * @author gjoranv + */ +public class FailedInvoiceUpdate extends InvoiceUpdate { + + public enum Reason { + UNMODIFIABLE, + REMOVED + } + + public final Reason reason; + + public FailedInvoiceUpdate(Bill.Id billId, Reason reason) { + super(billId, ItemsUpdate.empty()); + this.reason = reason; + } + + public static FailedInvoiceUpdate unmodifiable(Bill.Id billId) { + return new FailedInvoiceUpdate(billId, Reason.UNMODIFIABLE); + } + + public static FailedInvoiceUpdate removed(Bill.Id billId) { + return new FailedInvoiceUpdate(billId, Reason.REMOVED); + } + +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/InvoiceUpdate.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/InvoiceUpdate.java index 6ca3cf6ebb1..bb76834a483 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/InvoiceUpdate.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/InvoiceUpdate.java @@ -1,44 +1,69 @@ package com.yahoo.vespa.hosted.controller.api.integration.billing; +import java.util.Objects; + /** * Helper to track changes to an invoice. * * @author gjoranv */ -public record InvoiceUpdate(int itemsAdded, int itemsRemoved, int itemsModified) { - public boolean isEmpty() { - return itemsAdded == 0 && itemsRemoved == 0 && itemsModified == 0; +public abstract class InvoiceUpdate { + + final Bill.Id billId; + final ItemsUpdate itemsUpdate; + + InvoiceUpdate(Bill.Id billId, ItemsUpdate itemsUpdate) { + this.billId = billId; + this.itemsUpdate = itemsUpdate; } - public static InvoiceUpdate empty() { - return new InvoiceUpdate(0, 0, 0); + public Bill.Id billId() { + return billId; } - public static class Counter { - private int itemsAdded = 0; - private int itemsRemoved = 0; - private int itemsModified = 0; + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + InvoiceUpdate that = (InvoiceUpdate) o; + return Objects.equals(billId, that.billId) && Objects.equals(itemsUpdate, that.itemsUpdate); + } - public void addedItem() { - itemsAdded++; - } + @Override + public int hashCode() { + return Objects.hash(billId, itemsUpdate); + } - public void removedItem() { - itemsRemoved++; - } + public record ItemsUpdate(int itemsAdded, int itemsRemoved, int itemsModified) { - public void modifiedItem() { - itemsModified++; + public boolean isEmpty() { + return itemsAdded == 0 && itemsRemoved == 0 && itemsModified == 0; } - public void add(InvoiceUpdate other) { - itemsAdded += other.itemsAdded; - itemsRemoved += other.itemsRemoved; - itemsModified += other.itemsModified; + public static ItemsUpdate empty() { + return new ItemsUpdate(0, 0, 0); } - public InvoiceUpdate finish() { - return new InvoiceUpdate(itemsAdded, itemsRemoved, itemsModified); + public static class Counter { + private int itemsAdded = 0; + private int itemsRemoved = 0; + private int itemsModified = 0; + + public void addedItem() { + itemsAdded++; + } + + public void removedItem() { + itemsRemoved++; + } + + public void modifiedItem() { + itemsModified++; + } + + public ItemsUpdate finish() { + return new ItemsUpdate(itemsAdded, itemsRemoved, itemsModified); + } } } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/MockBillingController.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/MockBillingController.java index 18dd339b4a1..9012b45748c 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/MockBillingController.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/MockBillingController.java @@ -210,10 +210,10 @@ public class MockBillingController implements BillingController { public AcceptedCountries getAcceptedCountries() { return new AcceptedCountries(List.of( new AcceptedCountries.Country( - "NO", "Norway", + "NO", "Norway", true, List.of(new AcceptedCountries.TaxType("no_vat", "Norwegian VAT number", "[0-9]{9}MVA", "123456789MVA"))), new AcceptedCountries.Country( - "CA", "Canada", + "CA", "Canada", true, List.of(new AcceptedCountries.TaxType("ca_gst_hst", "Canadian GST/HST number", "([0-9]{9}) ?RT ?([0-9]{4})", "123456789RT0002"), new AcceptedCountries.TaxType("ca_pst_bc", "Canadian PST number (British Columbia)", "PST-?([0-9]{4})-?([0-9]{4})", "PST-1234-5678"))) )); @@ -221,7 +221,7 @@ public class MockBillingController implements BillingController { @Override public void validateTaxId(TaxId id) throws IllegalArgumentException { - if (id.isEmpty() || id.isLegacy()) return; + if (id.isEmpty() || id.isLegacy() || id.country().isEmpty()) return; if (!List.of("eu_vat", "no_vat").contains(id.type().value())) throw new IllegalArgumentException("Unknown tax id type '%s'".formatted(id.type().value())); if (!id.code().value().matches("\\w+")) diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/ModifiableInvoiceUpdate.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/ModifiableInvoiceUpdate.java new file mode 100644 index 00000000000..75cce564fc7 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/ModifiableInvoiceUpdate.java @@ -0,0 +1,28 @@ +package com.yahoo.vespa.hosted.controller.api.integration.billing; + +/** + * @author gjoranv + */ +public class ModifiableInvoiceUpdate extends InvoiceUpdate { + + public ModifiableInvoiceUpdate(Bill.Id billId, ItemsUpdate itemsUpdate) { + super(billId, itemsUpdate); + } + + public ItemsUpdate itemsUpdate() { + return itemsUpdate; + } + + public boolean isEmpty() { + return itemsUpdate.isEmpty(); + } + + public static ModifiableInvoiceUpdate of(Bill.Id billId, int itemsAdded, int itemsRemoved, int itemsModified) { + return new ModifiableInvoiceUpdate(billId, new ItemsUpdate(itemsAdded, itemsRemoved, itemsModified)); + } + + @Override + public boolean equals(Object o) { + return super.equals(o); + } +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/PurchaseOrder.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/PurchaseOrder.java index d222864a388..04bf6301e81 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/PurchaseOrder.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/PurchaseOrder.java @@ -18,4 +18,5 @@ public class PurchaseOrder extends StringWrapper<PurchaseOrder> { public static PurchaseOrder empty() { return new PurchaseOrder(""); } + public boolean isEmpty() { return value().isEmpty(); } } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/TaxId.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/TaxId.java index 99c2400c58c..172ad257f6a 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/TaxId.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/TaxId.java @@ -8,17 +8,27 @@ import static ai.vespa.validation.Validation.requireLength; /** * @author olaa */ -public record TaxId(Type type, Code code) { +public record TaxId(Country country, Type type, Code code) { - public TaxId(String type, String code) { this(new Type(type), new Code(code)); } + public TaxId(String country, String type, String code) { this(new Country(country), new Type(type), new Code(code)); } - public static TaxId empty() { return new TaxId(Type.empty(), Code.empty()); } - public boolean isEmpty() { return type.isEmpty() && code.isEmpty(); } + public static TaxId empty() { return new TaxId(Country.empty(), Type.empty(), Code.empty()); } + public boolean isEmpty() { return country.isEmpty() && type.isEmpty() && code.isEmpty(); } // TODO(bjorncs) Remove legacy once no longer present in ZK - public static TaxId legacy(String code) { return new TaxId(Type.empty(), new Code(code)); } + public static TaxId legacy(String code) { return new TaxId(Country.empty(), Type.empty(), new Code(code)); } public boolean isLegacy() { return type.isEmpty() && !code.isEmpty(); } + public static class Country extends StringWrapper<Country> { + public Country(String value) { + super(value); + requireLength(value, "tax code country length", 0, 2); + } + + public static Country empty() { return new Country(""); } + public boolean isEmpty() { return value().isEmpty(); } + } + public static class Type extends StringWrapper<Type> { public Type(String value) { super(value); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/BillingReportMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/BillingReportMaintainer.java index 7868c3fe611..5c37e0e4d0b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/BillingReportMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/BillingReportMaintainer.java @@ -5,17 +5,21 @@ import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.LockedTenant; +import com.yahoo.vespa.hosted.controller.api.integration.billing.Bill; import com.yahoo.vespa.hosted.controller.api.integration.billing.BillStatus; import com.yahoo.vespa.hosted.controller.api.integration.billing.BillingController; import com.yahoo.vespa.hosted.controller.api.integration.billing.BillingDatabaseClient; import com.yahoo.vespa.hosted.controller.api.integration.billing.BillingReporter; +import com.yahoo.vespa.hosted.controller.api.integration.billing.FailedInvoiceUpdate; import com.yahoo.vespa.hosted.controller.api.integration.billing.InvoiceUpdate; +import com.yahoo.vespa.hosted.controller.api.integration.billing.ModifiableInvoiceUpdate; import com.yahoo.vespa.hosted.controller.api.integration.billing.Plan; import com.yahoo.vespa.hosted.controller.api.integration.billing.PlanRegistry; import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; import com.yahoo.vespa.hosted.controller.tenant.Tenant; import java.time.Duration; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Set; @@ -63,17 +67,32 @@ public class BillingReportMaintainer extends ControllerMaintainer { }); } - InvoiceUpdate maintainInvoices() { + List<InvoiceUpdate> maintainInvoices() { + var updates = new ArrayList<InvoiceUpdate>(); + + var tenants = cloudTenants(); var billsNeedingMaintenance = databaseClient.readBills().stream() .filter(bill -> bill.getExportedId().isPresent()) .filter(exported -> exported.status() == BillStatus.OPEN) .toList(); - var updates = new InvoiceUpdate.Counter(); for (var bill : billsNeedingMaintenance) { - updates.add(reporter.maintainInvoice(bill)); + var exportedId = bill.getExportedId().orElseThrow(); + var update = reporter.maintainInvoice(tenants.get(bill.tenant()), bill); + if (update instanceof ModifiableInvoiceUpdate modifiable && ! modifiable.isEmpty()) { + log.fine(invoiceMessage(bill.id(), exportedId) + " was updated with " + modifiable.itemsUpdate()); + } else if (update instanceof FailedInvoiceUpdate failed && failed.reason == FailedInvoiceUpdate.Reason.REMOVED) { + log.fine(invoiceMessage(bill.id(), exportedId) + " has been deleted in the external system"); + // Reset the exportedId to null, so that we don't maintain it again + databaseClient.setExportedInvoiceId(bill.id(), null); + } + updates.add(update); } - return updates.finish(); + return updates; + } + + private String invoiceMessage(Bill.Id billId, String invoiceId) { + return "Invoice '" + invoiceId + "' for bill '" + billId.value() + "'"; } private Map<TenantName, CloudTenant> cloudTenants() { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudTrialExpirer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudTrialExpirer.java index 55428e80493..1e261f78db3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudTrialExpirer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudTrialExpirer.java @@ -30,7 +30,6 @@ import java.util.stream.Collectors; import static com.yahoo.vespa.hosted.controller.persistence.TrialNotifications.State.EXPIRED; import static com.yahoo.vespa.hosted.controller.persistence.TrialNotifications.State.EXPIRES_IMMEDIATELY; -import static com.yahoo.vespa.hosted.controller.persistence.TrialNotifications.State.EXPIRES_SOON; import static com.yahoo.vespa.hosted.controller.persistence.TrialNotifications.State.MID_CHECK_IN; import static com.yahoo.vespa.hosted.controller.persistence.TrialNotifications.State.SIGNED_UP; import static com.yahoo.vespa.hosted.controller.persistence.TrialNotifications.State.UNKNOWN; @@ -95,6 +94,14 @@ public class CloudTrialExpirer extends ControllerMaintainer { return tombstoneTenants(idleOldPlanTenants); } + + /* + * Trial plan notification states. Transition to a new state triggers a notification/email + * - SIGNED_UP: Tenant has signed up for trial + * - MID_CHECK_IN: Tenant is halfway through trial (7 days) + * - EXPIRES_IMMEDIATELY: Tenant has 1 day left of trial + * - EXPIRED: Tenant has expired + */ private boolean notifyTenants() { try { var currentStatus = controller().curator().readTrialNotifications() @@ -129,12 +136,8 @@ public class CloudTrialExpirer extends ControllerMaintainer { && !List.of(EXPIRES_IMMEDIATELY, EXPIRED).contains(state)) { updatedStatus.add(updatedStatus(tenant, now, EXPIRES_IMMEDIATELY)); notifyExpiresImmediately(tenant); - } else if ("trial".equals(plan) && ageInDays >= 12 - && !List.of(EXPIRES_SOON, EXPIRES_IMMEDIATELY, EXPIRED).contains(state)) { - updatedStatus.add(updatedStatus(tenant, now, EXPIRES_SOON)); - notifyExpiresSoon(tenant); } else if ("trial".equals(plan) && ageInDays >= 7 - && !List.of(MID_CHECK_IN, EXPIRES_SOON, EXPIRES_IMMEDIATELY, EXPIRED).contains(state)) { + && !List.of(MID_CHECK_IN, EXPIRES_IMMEDIATELY, EXPIRED).contains(state)) { updatedStatus.add(updatedStatus(tenant, now, MID_CHECK_IN)); notifyMidCheckIn(tenant); } else { @@ -152,44 +155,28 @@ public class CloudTrialExpirer extends ControllerMaintainer { private void notifySignup(Tenant tenant) { var consoleMsg = "Welcome to Vespa Cloud trial! [Manage plan](%s)".formatted(billingUrl(tenant)); - queueNotification(tenant, consoleMsg, "Welcome to Vespa Cloud", - "Welcome to Vespa Cloud! We hope you will enjoy your trial. " + - "Please reach out to us if you have any questions or feedback."); + queueNotification(tenant, consoleMsg, "Welcome to Vespa Cloud", MailTemplating.Template.TRIAL_SIGNED_UP); } private void notifyMidCheckIn(Tenant tenant) { var consoleMsg = "You're halfway through the **14 day** trial period. [Manage plan](%s)".formatted(billingUrl(tenant)); - queueNotification(tenant, consoleMsg, "How is your Vespa Cloud trial going?", - "How is your Vespa Cloud trial going? " + - "Please reach out to us if you have any questions or feedback."); - } - - private void notifyExpiresSoon(Tenant tenant) { - var consoleMsg = "Your Vespa Cloud trial expires in **2** days. [Manage plan](%s)".formatted(billingUrl(tenant)); - queueNotification(tenant, consoleMsg, "Your Vespa Cloud trial expires in 2 days", - "Your Vespa Cloud trial expires in 2 days. " + - "Please reach out to us if you have any questions or feedback."); + queueNotification(tenant, consoleMsg, "How is your Vespa Cloud trial going?", MailTemplating.Template.TRIAL_MIDWAY_CHECKIN); } private void notifyExpiresImmediately(Tenant tenant) { var consoleMsg = "Your Vespa Cloud trial expires **tomorrow**. [Manage plan](%s)".formatted(billingUrl(tenant)); - queueNotification(tenant, consoleMsg, "Your Vespa Cloud trial expires tomorrow", - "Your Vespa Cloud trial expires tomorrow. " + - "Please reach out to us if you have any questions or feedback."); + queueNotification(tenant, consoleMsg, "Your Vespa Cloud trial expires tomorrow", MailTemplating.Template.TRIAL_EXPIRES_IMMEDIATELY); } private void notifyExpired(Tenant tenant) { var consoleMsg = "Your Vespa Cloud trial has expired. [Upgrade plan](%s)".formatted(billingUrl(tenant)); - queueNotification(tenant, consoleMsg, "Your Vespa Cloud trial has expired", - "Your Vespa Cloud trial has expired. " + - "Please reach out to us if you have any questions or feedback."); + queueNotification(tenant, consoleMsg, "Your Vespa Cloud trial has expired", MailTemplating.Template.TRIAL_EXPIRED); } - private void queueNotification(Tenant tenant, String consoleMsg, String emailSubject, String emailMsg) { + private void queueNotification(Tenant tenant, String consoleMsg, String emailSubject, MailTemplating.Template template) { var mail = Optional.of(Notification.MailContent.fromTemplate(MailTemplating.Template.DEFAULT_MAIL_CONTENT) .subject(emailSubject) - .with("mailMessageTemplate", "cloud-trial-notification") - .with("cloudTrialMessage", emailMsg) + .with("mailMessageTemplate", template.getId()) .with("mailTitle", emailSubject) .with("consoleLink", controller().serviceRegistry().consoleUrls().tenantOverview(tenant.name())) .build()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/MailTemplating.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/MailTemplating.java index 1c05330702e..642840bf2b3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/MailTemplating.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/MailTemplating.java @@ -25,7 +25,9 @@ public class MailTemplating { public enum Template { MAIL("mail"), DEFAULT_MAIL_CONTENT("default-mail-content"), NOTIFICATION_MESSAGE("notification-message"), - CLOUD_TRIAL_NOTIFICATION("cloud-trial-notification"), MAIL_VERIFICATION("mail-verification"); + MAIL_VERIFICATION("mail-verification"), TRIAL_SIGNED_UP("trial-signed-up"), TRIAL_MIDWAY_CHECKIN("trial-midway-checkin"), + TRIAL_EXPIRES_IMMEDIATELY("trial-expires-immediately"), TRIAL_EXPIRED("trial-expired") + ; public static Optional<Template> fromId(String id) { return Arrays.stream(values()).filter(t -> t.id.equals(id)).findAny(); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java index 85b7acd603a..7801efe504b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java @@ -96,6 +96,7 @@ public class TenantSerializer { private static final String accountField = "account"; private static final String templateVersionField = "templateVersion"; private static final String taxIdField = "taxId"; + private static final String taxIdCountryField = "country"; private static final String taxIdTypeField = "type"; private static final String taxIdCodeField = "code"; private static final String purchaseOrderField = "purchaseOrder"; @@ -293,9 +294,10 @@ public class TenantSerializer { var taxId = switch (taxIdInspector.type()) { case STRING -> TaxId.legacy(taxIdInspector.asString()); case OBJECT -> { + var taxIdCountry = taxIdInspector.field(taxIdCountryField).asString(); var taxIdType = taxIdInspector.field(taxIdTypeField).asString(); var taxIdCode = taxIdInspector.field(taxIdCodeField).asString(); - yield new TaxId(new TaxId.Type(taxIdType), new TaxId.Code(taxIdCode)); + yield new TaxId(new TaxId.Country(taxIdCountry), new TaxId.Type(taxIdType), new TaxId.Code(taxIdCode)); } case NIX -> TaxId.empty(); default -> throw new IllegalStateException(taxIdInspector.type().name()); @@ -374,6 +376,7 @@ public class TenantSerializer { billingCursor.setBool("emailVerified", billingContact.contact().email().isVerified()); billingCursor.setString("phone", billingContact.contact().phone()); var taxIdCursor = billingCursor.setObject(taxIdField); + taxIdCursor.setString(taxIdCountryField, billingContact.getTaxId().country().value()); taxIdCursor.setString(taxIdTypeField, billingContact.getTaxId().type().value()); taxIdCursor.setString(taxIdCodeField, billingContact.getTaxId().code().value()); billingCursor.setString(purchaseOrderField, billingContact.getPurchaseOrder().value()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TrialNotifications.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TrialNotifications.java index a205e6c4173..cf6923b1e2c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TrialNotifications.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TrialNotifications.java @@ -21,7 +21,7 @@ public record TrialNotifications(List<Status> tenants) { public TrialNotifications { tenants = List.copyOf(tenants); } public record Status(TenantName tenant, State state, Instant lastUpdate) {} - public enum State { SIGNED_UP, MID_CHECK_IN, EXPIRES_SOON, EXPIRES_IMMEDIATELY, EXPIRED, UNKNOWN } + public enum State { SIGNED_UP, MID_CHECK_IN, EXPIRES_IMMEDIATELY, EXPIRED, UNKNOWN } public Slime toSlime() { var slime = new Slime(); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 078c27f5d00..1fd8e7c8f3b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -697,6 +697,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { contact.setBool("emailVerified", billingContact.contact().email().isVerified()); contact.setString("phone", billingContact.contact().phone()); var taxIdCursor = root.setObject("taxId"); + taxIdCursor.setString("country", billingContact.getTaxId().country().value()); taxIdCursor.setString("type", billingContact.getTaxId().type().value()); taxIdCursor.setString("code", billingContact.getTaxId().code().value()); root.setString("purchaseOrder", billingContact.getPurchaseOrder().value()); @@ -720,10 +721,6 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { var mergedPurchaseOrder = optional("purchaseOrder", inspector).map(PurchaseOrder::new).orElse(billing.getPurchaseOrder()); var mergedInvoiceEmail = optional("invoiceEmail", inspector).map(mail -> new Email(mail, false)).orElse(billing.getInvoiceEmail()); - if (!inspector.field("taxId").valid() && inspector.field("address").valid()) { - throw new IllegalArgumentException("Tax ID information is mandatory for setting up billing"); - } - var mergedBilling = info.billingContact() .withContact(mergedContact) .withAddress(mergedAddress) @@ -809,6 +806,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { billingCursor.setBool("emailVerified", billingContact.contact().email().isVerified()); billingCursor.setString("phone", billingContact.contact().phone()); var taxIdCursor = billingCursor.setObject("taxId"); + taxIdCursor.setString("country", billingContact.getTaxId().country().value()); taxIdCursor.setString("type", billingContact.getTaxId().type().value()); taxIdCursor.setString("code", billingContact.getTaxId().code().value()); billingCursor.setString("purchaseOrder", billingContact.getPurchaseOrder().value()); @@ -924,8 +922,10 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { private TaxId updateAndValidateTaxId(Inspector insp, TaxId old) { if (!insp.valid()) return old; - var taxId = new TaxId(getString(insp.field("type"), old.type().value()), - getString(insp.field("code"), old.code().value())); + var taxId = new TaxId( + getString(insp.field("country"), old.country().value()), + getString(insp.field("type"), old.type().value()), + getString(insp.field("code"), old.code().value())); controller.serviceRegistry().billingController().validateTaxId(taxId); return taxId; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandlerV2.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandlerV2.java index b4237a46c5a..93c13e6ed4c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandlerV2.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandlerV2.java @@ -131,6 +131,7 @@ public class BillingApiHandlerV2 extends RestApiRequestHandler<BillingApiHandler var countryCursor = countries.addObject(); countryCursor.setString("code", country.code()); countryCursor.setString("name", country.displayName()); + countryCursor.setBool("taxIdMandatory", country.taxIdMandatory()); var taxTypesCursors = countryCursor.setArray("taxTypes"); country.taxTypes().forEach(taxType -> { var taxTypeCursor = taxTypesCursors.addObject(); diff --git a/controller-server/src/main/resources/mail/cloud-trial-notification.vm b/controller-server/src/main/resources/mail/cloud-trial-notification.vm deleted file mode 100644 index c1ba394bf8e..00000000000 --- a/controller-server/src/main/resources/mail/cloud-trial-notification.vm +++ /dev/null @@ -1,3 +0,0 @@ -<p> - $esc.html($cloudTrialMessage) -</p>
\ No newline at end of file diff --git a/controller-server/src/main/resources/mail/trial-expired.vm b/controller-server/src/main/resources/mail/trial-expired.vm new file mode 100644 index 00000000000..02d2aacd117 --- /dev/null +++ b/controller-server/src/main/resources/mail/trial-expired.vm @@ -0,0 +1,3 @@ +<p> + Your Vespa Cloud trial has expired. Please reach out to us if you have any questions or feedback. +</p>
\ No newline at end of file diff --git a/controller-server/src/main/resources/mail/trial-expires-immediately.vm b/controller-server/src/main/resources/mail/trial-expires-immediately.vm new file mode 100644 index 00000000000..79604cae2e5 --- /dev/null +++ b/controller-server/src/main/resources/mail/trial-expires-immediately.vm @@ -0,0 +1,3 @@ +<p> + Your Vespa Cloud trial expires tomorrow. Please reach out to us if you have any questions or feedback. +</p>
\ No newline at end of file diff --git a/controller-server/src/main/resources/mail/trial-midway-checkin.vm b/controller-server/src/main/resources/mail/trial-midway-checkin.vm new file mode 100644 index 00000000000..c29c2763ef1 --- /dev/null +++ b/controller-server/src/main/resources/mail/trial-midway-checkin.vm @@ -0,0 +1,3 @@ +<p> + How is your Vespa Cloud trial going? Please reach out to us if you have any questions or feedback. +</p>
\ No newline at end of file diff --git a/controller-server/src/main/resources/mail/trial-signed-up.vm b/controller-server/src/main/resources/mail/trial-signed-up.vm new file mode 100644 index 00000000000..20f1867b7bc --- /dev/null +++ b/controller-server/src/main/resources/mail/trial-signed-up.vm @@ -0,0 +1,3 @@ +<p> + Welcome to Vespa Cloud! We hope you will enjoy your trial. Please reach out to us if you have any questions or feedback. +</p>
\ No newline at end of file diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java index 39d867d813d..2d1f06f3602 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java @@ -100,6 +100,7 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg private final PlanRegistry planRegistry = new PlanRegistryMock(); private final ResourceDatabaseClient resourceDb = new ResourceDatabaseClientMock(planRegistry); private final BillingDatabaseClient billingDb = new BillingDatabaseClientMock(clock, planRegistry); + private final BillingReporterMock billingReporter = new BillingReporterMock(clock, billingDb); private final MockBillingController billingController = new MockBillingController(clock, billingDb); private final RoleMaintainerMock roleMaintainer = new RoleMaintainerMock(); @@ -325,7 +326,7 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg @Override public BillingReporter billingReporter() { - return new BillingReporterMock(clock(), billingDb); + return billingReporter; } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/BillingReportMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/BillingReportMaintainerTest.java index 5cb46664a75..8d1848539f0 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/BillingReportMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/BillingReportMaintainerTest.java @@ -5,7 +5,9 @@ import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; import com.yahoo.vespa.hosted.controller.ControllerTester; import com.yahoo.vespa.hosted.controller.api.integration.billing.BillStatus; -import com.yahoo.vespa.hosted.controller.api.integration.billing.InvoiceUpdate; +import com.yahoo.vespa.hosted.controller.api.integration.billing.BillingReporterMock; +import com.yahoo.vespa.hosted.controller.api.integration.billing.FailedInvoiceUpdate; +import com.yahoo.vespa.hosted.controller.api.integration.billing.ModifiableInvoiceUpdate; import com.yahoo.vespa.hosted.controller.api.integration.billing.PlanRegistryMock; import com.yahoo.vespa.hosted.controller.tenant.BillingReference; import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; @@ -46,7 +48,6 @@ public class BillingReportMaintainerTest { @Test void only_open_bills_with_exported_id_are_maintained() { var t1 = tester.createTenant("t1"); - var billingController = tester.controller().serviceRegistry().billingController(); var billingDb = tester.controller().serviceRegistry().billingDatabase(); var start = LocalDate.of(2020, 5, 23).atStartOfDay(ZoneOffset.UTC); @@ -57,25 +58,88 @@ public class BillingReportMaintainerTest { var bill3 = billingDb.createBill(t1, start, end, "exported-and-frozen"); billingDb.setStatus(bill3, "foo", BillStatus.FROZEN); - billingController.setPlan(t1, PlanRegistryMock.paidPlan.id(), false, true); - - tester.controller().serviceRegistry().billingReporter().exportBill(billingDb.readBill(bill2).get(), "FOO", cloudTenant(t1)); - tester.controller().serviceRegistry().billingReporter().exportBill(billingDb.readBill(bill3).get(), "FOO", cloudTenant(t1)); + var reporter = tester.controller().serviceRegistry().billingReporter(); + reporter.exportBill(billingDb.readBill(bill2).get(), "FOO", cloudTenant(t1)); + reporter.exportBill(billingDb.readBill(bill3).get(), "FOO", cloudTenant(t1)); var updates = maintainer.maintainInvoices(); - assertEquals(new InvoiceUpdate(1, 0, 0), updates); assertTrue(billingDb.readBill(bill1).get().getExportedId().isEmpty()); + // Only the exported open bill is maintained + assertEquals(1, updates.size()); + assertEquals(bill2, updates.get(0).billId()); + assertEquals(ModifiableInvoiceUpdate.class, updates.get(0).getClass()); var exportedBill = billingDb.readBill(bill2).get(); - assertEquals("EXT-ID-123", exportedBill.getExportedId().get()); + assertEquals("EXPORTED-" + exportedBill.id().value(), exportedBill.getExportedId().get()); + // Verify that the bill has been updated with a marker line item by the mock var lineItems = exportedBill.lineItems(); assertEquals(1, lineItems.size()); assertEquals("maintained", lineItems.get(0).id()); + // The frozen bill is untouched by the maintainer var frozenBill = billingDb.readBill(bill3).get(); - assertEquals("EXT-ID-123", frozenBill.getExportedId().get()); + assertEquals("EXPORTED-" + frozenBill.id().value(), frozenBill.getExportedId().get()); assertEquals(0, frozenBill.lineItems().size()); + } + + @Test + void bills_whose_invoice_has_been_deleted_in_the_external_system_are_no_longer_maintained() { + var t1 = tester.createTenant("t1"); + var billingDb = tester.controller().serviceRegistry().billingDatabase(); + + var start = LocalDate.of(2020, 5, 23).atStartOfDay(ZoneOffset.UTC); + var end = start.toLocalDate().plusDays(6).atStartOfDay(ZoneOffset.UTC); + + var bill1 = billingDb.createBill(t1, start, end, "exported-then-deleted"); + + var reporter = (BillingReporterMock)tester.controller().serviceRegistry().billingReporter(); + reporter.exportBill(billingDb.readBill(bill1).get(), "FOO", cloudTenant(t1)); + + var updates = maintainer.maintainInvoices(); + assertEquals(1, updates.size()); + assertEquals(ModifiableInvoiceUpdate.class, updates.get(0).getClass()); + + // Delete invoice from the external system + reporter.deleteExportedBill(bill1); + + // Maintainer should report that the invoice has been removed + updates = maintainer.maintainInvoices(); + assertEquals(1, updates.size()); + assertEquals(FailedInvoiceUpdate.class, updates.get(0).getClass()); + assertEquals(FailedInvoiceUpdate.Reason.REMOVED, ((FailedInvoiceUpdate)updates.get(0)).reason); + + // The bill should no longer be maintained + updates = maintainer.maintainInvoices(); + assertEquals(0, updates.size()); + } + + @Test + void it_is_allowed_to_re_export_bills_whose_invoice_has_been_deleted_in_the_external_system() { + var t1 = tester.createTenant("t1"); + var billingDb = tester.controller().serviceRegistry().billingDatabase(); + + var start = LocalDate.of(2020, 5, 23).atStartOfDay(ZoneOffset.UTC); + var end = start.toLocalDate().plusDays(6).atStartOfDay(ZoneOffset.UTC); + + var bill1 = billingDb.createBill(t1, start, end, "exported-then-deleted"); + + var reporter = (BillingReporterMock)tester.controller().serviceRegistry().billingReporter(); + + // Export the bill, then delete it in the external system + reporter.exportBill(billingDb.readBill(bill1).get(), "FOO", cloudTenant(t1)); + maintainer.maintainInvoices(); + reporter.deleteExportedBill(bill1); + maintainer.maintainInvoices(); + + // Ensure it is currently ignored by the maintainer + var updates = maintainer.maintainInvoices(); + assertEquals(0, updates.size()); + // Re-export the bill and verify that it is maintained again + reporter.exportBill(billingDb.readBill(bill1).get(), "FOO", cloudTenant(t1)); + updates = maintainer.maintainInvoices(); + assertEquals(1, updates.size()); + assertEquals(ModifiableInvoiceUpdate.class, updates.get(0).getClass()); } private CloudTenant cloudTenant(TenantName tenantName) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CloudTrialExpirerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CloudTrialExpirerTest.java index 4056459c532..02d0a020cd2 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CloudTrialExpirerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CloudTrialExpirerTest.java @@ -110,40 +110,41 @@ public class CloudTrialExpirerTest { .withBooleanFlag(Flags.CLOUD_TRIAL_NOTIFICATIONS.id(), true); registerTenant(tenant.value(), "trial", Duration.ZERO); assertEquals(0.0, expirer.maintain()); - var expected = "Welcome to Vespa Cloud trial! [Manage plan](https://console.tld/tenant/trial-tenant/account/billing)"; - assertEquals(expected, lastAccountLevelNotificationTitle(tenant)); - assertLastEmailEquals(mailer, "welcome.html"); - - expected = "You're halfway through the **14 day** trial period. [Manage plan](https://console.tld/tenant/trial-tenant/account/billing)"; + var expectedConsoleNotification = + "Welcome to Vespa Cloud trial! [Manage plan](https://console.tld/tenant/trial-tenant/account/billing)"; + var notification = lastAccountLevelNotification(tenant); + assertEquals(expectedConsoleNotification, notification.title()); + assertLastEmail(mailer, notification); + + expectedConsoleNotification = + "You're halfway through the **14 day** trial period. [Manage plan](https://console.tld/tenant/trial-tenant/account/billing)"; clock.advance(Duration.ofDays(7)); assertEquals(0.0, expirer.maintain()); - assertEquals(expected, lastAccountLevelNotificationTitle(tenant)); - assertLastEmailEquals(mailer, "trial-reminder.html"); - - expected = "Your Vespa Cloud trial expires in **2** days. [Manage plan](https://console.tld/tenant/trial-tenant/account/billing)"; - clock.advance(Duration.ofDays(5)); - assertEquals(0.0, expirer.maintain()); - assertEquals(expected, lastAccountLevelNotificationTitle(tenant)); - assertLastEmailEquals(mailer, "trial-expiring-soon.html"); + notification = lastAccountLevelNotification(tenant); + assertEquals(expectedConsoleNotification, notification.title()); + assertLastEmail(mailer, notification); - expected = "Your Vespa Cloud trial expires **tomorrow**. [Manage plan](https://console.tld/tenant/trial-tenant/account/billing)"; - clock.advance(Duration.ofDays(1)); + expectedConsoleNotification = "Your Vespa Cloud trial expires **tomorrow**. [Manage plan](https://console.tld/tenant/trial-tenant/account/billing)"; + clock.advance(Duration.ofDays(6)); assertEquals(0.0, expirer.maintain()); - assertEquals(expected, lastAccountLevelNotificationTitle(tenant)); - assertLastEmailEquals(mailer, "trial-expiring-immediately.html"); + notification = lastAccountLevelNotification(tenant); + assertEquals(expectedConsoleNotification, notification.title()); + assertLastEmail(mailer, notification); - expected = "Your Vespa Cloud trial has expired. [Upgrade plan](https://console.tld/tenant/trial-tenant/account/billing)"; + expectedConsoleNotification = "Your Vespa Cloud trial has expired. [Upgrade plan](https://console.tld/tenant/trial-tenant/account/billing)"; clock.advance(Duration.ofDays(2)); assertEquals(0.0, expirer.maintain()); - assertEquals(expected, lastAccountLevelNotificationTitle(tenant)); - assertLastEmailEquals(mailer, "trial-expired.html"); + notification = lastAccountLevelNotification(tenant); + assertEquals(expectedConsoleNotification, notification.title()); + assertLastEmail(mailer, notification); } - private void assertLastEmailEquals(MockMailer mailer, String expectedContentFile) throws IOException { + private void assertLastEmail(MockMailer mailer, Notification notification) throws IOException { var mails = mailer.inbox("dev-trial-tenant"); assertFalse(mails.isEmpty()); var content = mails.get(mails.size() - 1).htmlMessage().orElseThrow(); - var path = Paths.get("src/test/resources/mail/" + expectedContentFile); + var templateName = notification.mailContent().orElseThrow().values().get("mailMessageTemplate"); + var path = Paths.get("src/test/resources/mail/%s.html".formatted(templateName)); if (OVERWRITE_TEST_FILES) { Files.write(path, content.getBytes(), StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.WRITE); @@ -175,11 +176,10 @@ public class CloudTrialExpirerTest { assertEquals(planId, tester.serviceRegistry().billingController().getPlan(TenantName.from(tenant)).value()); } - private String lastAccountLevelNotificationTitle(TenantName tenant) { + private Notification lastAccountLevelNotification(TenantName tenant) { return tester.controller().notificationsDb() .listNotifications(NotificationSource.from(tenant), false).stream() - .filter(n -> n.type() == Notification.Type.account).map(Notification::title) + .filter(n -> n.type() == Notification.Type.account) .findFirst().orElseThrow(); } - } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java index cfc3320b083..f2fc43933df 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java @@ -238,7 +238,7 @@ public class TenantSerializerTest { .withAddress("Central Station") .withRegion("Irish Sea")) .withPurchaseOrder(new PurchaseOrder("PO42")) - .withTaxId(new TaxId("no_vat", "123456789MVA")) + .withTaxId(new TaxId("NO", "no_vat", "123456789MVA")) .withInvoiceEmail(new Email("billing@mycomp.any", false)) ); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java index 1ad88675169..f8ae7c8ea50 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java @@ -107,6 +107,7 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { "phone":"" }, "taxId": { + "country": "", "type": "", "code": "" }, @@ -125,7 +126,7 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { "email":"foo@example", "phone":"phone" }, - "taxId":{"type": "no_vat", "code": "123456789MVA"}, + "taxId":{"country": "NO", "type": "no_vat", "code": "123456789MVA"}, "purchaseOrder":"PO9001", "invoiceEmail":"billing@mycomp.any", "address": { @@ -151,6 +152,7 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { "phone":"phone" }, "taxId": { + "country": "NO", "type": "no_vat", "code": "123456789MVA" }, @@ -238,6 +240,7 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { "email":"","emailVerified":false, "phone":"", "taxId": { + "country": "", "type": "", "code": "" }, @@ -272,6 +275,7 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { "emailVerified":false, "phone":"phone", "taxId": { + "country": "", "type": "", "code": "" }, diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/accepted-countries.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/accepted-countries.json index 2714de1e64d..5247b84a900 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/accepted-countries.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/accepted-countries.json @@ -3,6 +3,7 @@ { "code": "NO", "name": "Norway", + "taxIdMandatory": true, "taxTypes": [ { "id": "no_vat", @@ -15,6 +16,7 @@ { "code": "CA", "name": "Canada", + "taxIdMandatory": true, "taxTypes": [ { "id": "ca_gst_hst", diff --git a/controller-server/src/test/resources/mail/trial-expiring-immediately.html b/controller-server/src/test/resources/mail/trial-expires-immediately.html index db89eca195a..db89eca195a 100644 --- a/controller-server/src/test/resources/mail/trial-expiring-immediately.html +++ b/controller-server/src/test/resources/mail/trial-expires-immediately.html diff --git a/controller-server/src/test/resources/mail/trial-expiring-soon.html b/controller-server/src/test/resources/mail/trial-expiring-soon.html deleted file mode 100644 index 17c59240cc4..00000000000 --- a/controller-server/src/test/resources/mail/trial-expiring-soon.html +++ /dev/null @@ -1,646 +0,0 @@ -<!DOCTYPE html> -<html - xmlns="http://www.w3.org/1999/xhtml" - xmlns:v="urn:schemas-microsoft-com:vml" - xmlns:o="urn:schemas-microsoft-com:office:office" -> - <head> - <title></title> - <!--[if !mso]><!--> - <meta http-equiv="X-UA-Compatible" content="IE=edge" /> - <!--<![endif]--> - <meta http-equiv="Content-Type" content="text/html; charset=UTF-8" /> - <meta name="viewport" content="width=device-width,initial-scale=1" /> - <style type="text/css"> - #outlook a { - padding: 0; - } - - body { - margin: 0; - padding: 0; - -webkit-text-size-adjust: 100%; - -ms-text-size-adjust: 100%; - } - - table, - td { - border-collapse: collapse; - mso-table-lspace: 0pt; - mso-table-rspace: 0pt; - } - - img { - border: 0; - height: auto; - line-height: 100%; - outline: none; - text-decoration: none; - -ms-interpolation-mode: bicubic; - } - - p { - display: block; - margin: 13px 0; - } - </style> - <!--[if mso]> - <noscript> - <xml> - <o:OfficeDocumentSettings> - <o:AllowPNG /> - <o:PixelsPerInch>96</o:PixelsPerInch> - </o:OfficeDocumentSettings> - </xml> - </noscript> - <![endif]--> - <!--[if lte mso 11]> - <style type="text/css"> - .mj-outlook-group-fix { - width: 100% !important; - } - </style> - <![endif]--> - <!--[if !mso]><!--> - <link - href="https://fonts.googleapis.com/css?family=Open Sans" - rel="stylesheet" - type="text/css" - /> - <style type="text/css"> - @import url(https://fonts.googleapis.com/css?family=Open Sans); - </style> - <!--<![endif]--> - <style type="text/css"> - @media only screen and (min-width: 480px) { - .mj-column-per-100 { - width: 100% !important; - max-width: 100%; - } - } - </style> - <style media="screen and (min-width:480px)"> - .moz-text-html .mj-column-per-100 { - width: 100% !important; - max-width: 100%; - } - </style> - <style type="text/css"> - [owa] .mj-column-per-100 { - width: 100% !important; - max-width: 100%; - } - </style> - <style type="text/css"> - @media only screen and (max-width: 480px) { - table.mj-full-width-mobile { - width: 100% !important; - } - - td.mj-full-width-mobile { - width: auto !important; - } - } - </style> - </head> - - <body style="word-spacing: normal; background-color: #f2f7fa"> - <div style="background-color: #f2f7fa"> - <!--[if mso | IE]><table align="center" border="0" cellpadding="0" cellspacing="0" class="" role="presentation" style="width:600px;" width="600" ><tr><td style="line-height:0px;font-size:0px;mso-line-height-rule:exactly;"><![endif]--> - <div style="margin: 0px auto; max-width: 600px"> - <table - align="center" - border="0" - cellpadding="0" - cellspacing="0" - role="presentation" - style="width: 100%" - > - <tbody> - <tr> - <td - style=" - direction: ltr; - font-size: 0px; - padding: 20px 0px 20px 0px; - padding-bottom: 0px; - padding-top: 0px; - text-align: center; - " - > - <!--[if mso | IE]><table role="presentation" border="0" cellpadding="0" cellspacing="0"><tr><td class="" style="vertical-align:top;width:600px;" ><![endif]--> - <div - class="mj-column-per-100 mj-outlook-group-fix" - style=" - font-size: 0px; - text-align: left; - direction: ltr; - display: inline-block; - vertical-align: top; - width: 100%; - " - > - <table - border="0" - cellpadding="0" - cellspacing="0" - role="presentation" - style="vertical-align: top" - width="100%" - > - <tbody> - <tr> - <td - align="left" - style=" - font-size: 0px; - padding: 0px 0px 0px 25px; - padding-top: 0px; - padding-bottom: 0px; - word-break: break-word; - " - > - <div - style=" - font-family: Open Sans, Helvetica, Arial, - sans-serif; - font-size: 11px; - line-height: 22px; - text-align: left; - color: #797e82; - " - > - <br /> - </div> - </td> - </tr> - </tbody> - </table> - </div> - <!--[if mso | IE]></td></tr></table><![endif]--> - </td> - </tr> - </tbody> - </table> - </div> - <!--[if mso | IE]></td></tr></table><table align="center" border="0" cellpadding="0" cellspacing="0" class="" role="presentation" style="width:600px;" width="600" bgcolor="#ffffff" ><tr><td style="line-height:0px;font-size:0px;mso-line-height-rule:exactly;"><![endif]--> - <div - style=" - background: #ffffff; - background-color: #ffffff; - margin: 0px auto; - max-width: 600px; - " - > - <table - align="center" - border="0" - cellpadding="0" - cellspacing="0" - role="presentation" - style="background: #ffffff; background-color: #ffffff; width: 100%" - > - <tbody> - <tr> - <td - style=" - direction: ltr; - font-size: 0px; - padding: 20px 0; - padding-bottom: 0px; - padding-left: 0px; - padding-right: 0px; - padding-top: 0px; - text-align: center; - " - > - <!--[if mso | IE]><table role="presentation" border="0" cellpadding="0" cellspacing="0"><tr><td class="" style="vertical-align:top;width:600px;" ><![endif]--> - <div - class="mj-column-per-100 mj-outlook-group-fix" - style=" - font-size: 0px; - text-align: left; - direction: ltr; - display: inline-block; - vertical-align: top; - width: 100%; - " - > - <table - border="0" - cellpadding="0" - cellspacing="0" - role="presentation" - style="vertical-align: top" - width="100%" - > - <tbody> - <tr> - <td - align="center" - style=" - font-size: 0px; - padding: 10px 25px; - padding-top: 0px; - padding-right: 0px; - padding-bottom: 40px; - padding-left: 0px; - word-break: break-word; - " - > - <p - style=" - border-top: solid 8px #005a8e; - font-size: 1px; - margin: 0px auto; - width: 100%; - " - ></p> - <!--[if mso | IE - ]><table - align="center" - border="0" - cellpadding="0" - cellspacing="0" - style=" - border-top: solid 8px #005a8e; - font-size: 1px; - margin: 0px auto; - width: 600px; - " - role="presentation" - width="600px" - > - <tr> - <td style="height: 0; line-height: 0"> - - </td> - </tr> - </table><! - [endif]--> - </td> - </tr> - <tr> - <td - align="center" - style=" - font-size: 0px; - padding: 10px 25px; - padding-top: 0px; - padding-bottom: 0px; - word-break: break-word; - " - > - <table - border="0" - cellpadding="0" - cellspacing="0" - role="presentation" - style=" - border-collapse: collapse; - border-spacing: 0px; - " - > - <tbody> - <tr> - <td style="width: 121px"> - <img - alt="" - height="auto" - src="https://data.vespa.oath.cloud/assets/vespa-cloud-logo.png" - style=" - border: none; - display: block; - outline: none; - text-decoration: none; - height: auto; - width: 100%; - font-size: 13px; - " - width="121" - /> - </td> - </tr> - </tbody> - </table> - </td> - </tr> - </tbody> - </table> - </div> - <!--[if mso | IE]></td></tr></table><![endif]--> - </td> - </tr> - </tbody> - </table> - </div> - <!--[if mso | IE]></td></tr></table><table align="center" border="0" cellpadding="0" cellspacing="0" class="" role="presentation" style="width:600px;" width="600" bgcolor="#ffffff" ><tr><td style="line-height:0px;font-size:0px;mso-line-height-rule:exactly;"><![endif]--> - <div - style=" - background: #ffffff; - background-color: #ffffff; - margin: 0px auto; - max-width: 600px; - " - > - <table - align="center" - border="0" - cellpadding="0" - cellspacing="0" - role="presentation" - style="background: #ffffff; background-color: #ffffff; width: 100%" - > - <tbody> - <tr> - <td - style=" - direction: ltr; - font-size: 0px; - padding: 20px 0px 20px 0px; - padding-bottom: 70px; - padding-top: 30px; - text-align: center; - " - > - <!--[if mso | IE]><table role="presentation" border="0" cellpadding="0" cellspacing="0"><tr><td class="" style="vertical-align:top;width:600px;" ><![endif]--> - <div - class="mj-column-per-100 mj-outlook-group-fix" - style=" - font-size: 0px; - text-align: left; - direction: ltr; - display: inline-block; - vertical-align: top; - width: 100%; - " - > - <table - border="0" - cellpadding="0" - cellspacing="0" - role="presentation" - style="vertical-align: top" - width="100%" - > - -<tbody> -<tr> - <td - align="left" - style=" - font-size: 0px; - padding: 0px 25px 0px 25px; - padding-top: 0px; - padding-right: 50px; - padding-bottom: 0px; - padding-left: 50px; - word-break: break-word; - " - > - <div - style=" - font-family: Open Sans, Helvetica, Arial, - sans-serif; - font-size: 13px; - line-height: 22px; - text-align: left; - color: #797e82; - " - > - <h1 - style=" - text-align: center; - color: #000000; - line-height: 32px; - " - > - Your Vespa Cloud trial expires in 2 days - </h1> - </div> - </td> -</tr> -<tr> - <td - align="left" - style=" - font-size: 0px; - padding: 0px 25px 0px 25px; - padding-top: 0px; - padding-right: 50px; - padding-bottom: 0px; - padding-left: 50px; - word-break: break-word; - " - > - <div - style=" - font-family: Open Sans, Helvetica, Arial, - sans-serif; - font-size: 13px; - line-height: 22px; - text-align: left; - color: #797e82; - " - > - -<p> - Your Vespa Cloud trial expires in 2 days. Please reach out to us if you have any questions or feedback. -</p> - </div> - </td> -</tr> -<tr> - <td - align="center" - vertical-align="middle" - style=" - font-size: 0px; - padding: 10px 25px; - padding-top: 20px; - padding-bottom: 20px; - word-break: break-word; - " - > - <table - border="0" - cellpadding="0" - cellspacing="0" - role="presentation" - style="border-collapse: separate; line-height: 100%" - > - <tbody> - <tr> - <td - align="center" - bgcolor="#005A8E" - role="presentation" - style=" - border: none; - border-radius: 100px; - cursor: auto; - mso-padding-alt: 15px 25px 15px 25px; - background: #005a8e; - " - valign="middle" - > - <a - href="https://console.tld/tenant/trial-tenant" - style=" - display: inline-block; - background: #005a8e; - color: #ffffff; - font-family: Open Sans, Helvetica, Arial, - sans-serif; - font-size: 13px; - font-weight: normal; - line-height: 120%; - margin: 0; - text-decoration: none; - text-transform: none; - padding: 15px 25px 15px 25px; - mso-padding-alt: 0px; - border-radius: 100px; - " - target="_blank" - ><b style="font-weight: 700" - ><b style="font-weight: 700" - >Go to Console</b - ></b - ></a - > - </td> - </tr> - </tbody> - </table> - </td> -</tr> -</tbody> - </table> - </div> - <!--[if mso | IE]></td></tr></table><![endif]--> - </td> - </tr> - </tbody> - </table> - </div> - <!--[if mso | IE]></td></tr></table><table align="center" border="0" cellpadding="0" cellspacing="0" class="" role="presentation" style="width:600px;" width="600" ><tr><td style="line-height:0px;font-size:0px;mso-line-height-rule:exactly;"><![endif]--> - <div style="margin: 0px auto; max-width: 600px"> - <table - align="center" - border="0" - cellpadding="0" - cellspacing="0" - role="presentation" - style="width: 100%" - > - <tbody> - <tr> - <td - style=" - direction: ltr; - font-size: 0px; - padding: 20px 0px 20px 0px; - padding-bottom: 0px; - padding-top: 20px; - text-align: center; - " - > - <!--[if mso | IE]><table role="presentation" border="0" cellpadding="0" cellspacing="0"><tr><td class="" style="vertical-align:top;width:600px;" ><![endif]--> - <div - class="mj-column-per-100 mj-outlook-group-fix" - style=" - font-size: 0px; - text-align: left; - direction: ltr; - display: inline-block; - vertical-align: top; - width: 100%; - " - > - <table - border="0" - cellpadding="0" - cellspacing="0" - role="presentation" - style="vertical-align: top" - width="100%" - > - <tbody> - <tr> - <td - align="center" - style=" - font-size: 0px; - padding: 0px 20px 0px 20px; - padding-top: 0px; - padding-bottom: 0px; - word-break: break-word; - " - > - <div - style=" - font-family: Open Sans, Helvetica, Arial, - sans-serif; - font-size: 11px; - line-height: 22px; - text-align: center; - color: #797e82; - " - > - <p style="margin: 10px 0"> - <a - target="_blank" - rel="noopener noreferrer" - style="color: #005a8e" - href="https://legal.yahoo.com/xw/en/yahoo/privacy/topic/b2bprivacypolicy/index.html" - ><span style="color: #005a8e" - >Yahoo Privacy Policy</span - ></a - ><span style="color: #797e82" - > | </span - ><a - target="_blank" - rel="noopener noreferrer" - style="color: #005a8e" - href="https://console.tld/terms-of-service-trial.html" - ><span style="color: #005a8e" - >Terms of Service</span - ></a - ><span style="color: #797e82" - > | </span - ><a - target="_blank" - rel="noopener noreferrer" - style="color: #005a8e" - href="https://console.tld/support" - ><span style="color: #005a8e">Support</span></a - > - </p> - <p style="margin: 10px 0"> - <a - target="_blank" - rel="noopener noreferrer" - style="color: inherit; text-decoration: none" - href="https://console.tld/tenant/trial-tenant/account/notifications" - >Click - <span style="color: #005a8e"><u>here</u></span> - to manage your notifications setting.</a - ><br /> - </p> - </div> - </td> - </tr> - </tbody> - </table> - </div> - <!--[if mso | IE]></td></tr></table><![endif]--> - </td> - </tr> - </tbody> - </table> - </div> - <!--[if mso | IE]></td></tr></table><![endif]--> - </div> - </body> -</html> diff --git a/controller-server/src/test/resources/mail/trial-reminder.html b/controller-server/src/test/resources/mail/trial-midway-checkin.html index fbe0d573538..fbe0d573538 100644 --- a/controller-server/src/test/resources/mail/trial-reminder.html +++ b/controller-server/src/test/resources/mail/trial-midway-checkin.html diff --git a/controller-server/src/test/resources/mail/welcome.html b/controller-server/src/test/resources/mail/trial-signed-up.html index 2e652532db8..2e652532db8 100644 --- a/controller-server/src/test/resources/mail/welcome.html +++ b/controller-server/src/test/resources/mail/trial-signed-up.html diff --git a/dependency-versions/pom.xml b/dependency-versions/pom.xml index c688c5018c6..acf39dafe66 100644 --- a/dependency-versions/pom.xml +++ b/dependency-versions/pom.xml @@ -102,7 +102,7 @@ <java-jjwt.vespa.version>0.11.5</java-jjwt.vespa.version> <java-jwt.vespa.version>4.4.0</java-jwt.vespa.version> <jaxb.runtime.vespa.version>4.0.4</jaxb.runtime.vespa.version> - <jetty.vespa.version>11.0.17</jetty.vespa.version> + <jetty.vespa.version>11.0.18</jetty.vespa.version> <jetty-servlet-api.vespa.version>5.0.2</jetty-servlet-api.vespa.version> <jimfs.vespa.version>1.3.0</jimfs.vespa.version> <jna.vespa.version>5.13.0</jna.vespa.version> @@ -110,7 +110,7 @@ <junit.vespa.version>5.10.0</junit.vespa.version> <junit.platform.vespa.version>1.10.0</junit.platform.vespa.version> <junit4.vespa.version>4.13.2</junit4.vespa.version> - <luben.zstd.vespa.version>1.5.5-6</luben.zstd.vespa.version> + <luben.zstd.vespa.version>1.5.5-7</luben.zstd.vespa.version> <lucene.vespa.version>9.8.0</lucene.vespa.version> <maven-archiver.vespa.version>3.6.1</maven-archiver.vespa.version> <maven-wagon.vespa.version>3.5.3</maven-wagon.vespa.version> @@ -144,7 +144,7 @@ <surefire.vespa.tenant.version>${surefire.vespa.version}</surefire.vespa.tenant.version> <!-- Maven plugins --> - <clover-maven-plugin.vespa.version>4.5.0</clover-maven-plugin.vespa.version> + <clover-maven-plugin.vespa.version>4.5.1</clover-maven-plugin.vespa.version> <maven-antrun-plugin.vespa.version>3.1.0</maven-antrun-plugin.vespa.version> <maven-assembly-plugin.vespa.version>3.6.0</maven-assembly-plugin.vespa.version> <maven-bundle-plugin.vespa.version>5.1.9</maven-bundle-plugin.vespa.version> 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 0f1815e44b6..73ae75136b3 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -195,7 +195,7 @@ public class Flags { // TODO: Move to a permanent flag public static final UnboundListFlag<String> ALLOWED_ATHENZ_PROXY_IDENTITIES = defineListFlag( "allowed-athenz-proxy-identities", List.of(), String.class, - List.of("bjorncs", "tokle"), "2021-02-10", "2023-11-01", + List.of("bjorncs", "tokle"), "2021-02-10", "2024-02-01", "Allowed Athenz proxy identities", "takes effect at redeployment"); @@ -256,7 +256,7 @@ public class Flags { public static final UnboundBooleanFlag ENABLE_PROXY_PROTOCOL_MIXED_MODE = defineFeatureFlag( "enable-proxy-protocol-mixed-mode", true, - List.of("tokle"), "2022-05-09", "2023-11-01", + List.of("tokle"), "2022-05-09", "2023-12-01", "Enable or disable proxy protocol mixed mode", "Takes effect on redeployment", INSTANCE_ID); @@ -310,7 +310,7 @@ public class Flags { HOSTNAME); public static final UnboundBooleanFlag ENABLE_THE_ONE_THAT_SHOULD_NOT_BE_NAMED = defineFeatureFlag( - "enable-the-one-that-should-not-be-named", false, List.of("hmusum"), "2023-05-08", "2023-11-01", + "enable-the-one-that-should-not-be-named", false, List.of("hmusum"), "2023-05-08", "2023-12-01", "Whether to enable the one program that should not be named", "Takes effect at next host-admin tick"); @@ -350,13 +350,13 @@ public class Flags { public static final UnboundBooleanFlag WRITE_CONFIG_SERVER_SESSION_DATA_AS_ONE_BLOB = defineFeatureFlag( "write-config-server-session-data-as-blob", false, - List.of("hmusum"), "2023-07-19", "2023-11-01", + List.of("hmusum"), "2023-07-19", "2024-01-01", "Whether to write config server session data in one blob or as individual paths", "Takes effect immediately"); public static final UnboundBooleanFlag READ_CONFIG_SERVER_SESSION_DATA_AS_ONE_BLOB = defineFeatureFlag( "read-config-server-session-data-as-blob", false, - List.of("hmusum"), "2023-07-19", "2023-11-01", + List.of("hmusum"), "2023-07-19", "2024-01-01", "Whether to read config server session data from session data blob or from individual paths", "Takes effect immediately"); @@ -405,7 +405,7 @@ public class Flags { public static final UnboundStringFlag UNKNOWN_CONFIG_DEFINITION = defineStringFlag( "unknown-config-definition", "warn", - List.of("hmusum"), "2023-09-25", "2023-11-15", + List.of("hmusum"), "2023-09-25", "2024-01-01", "How to handle user config referencing unknown config definitions. Valid values are 'warn' and 'fail'", "Takes effect at redeployment", INSTANCE_ID); diff --git a/screwdriver.yaml b/screwdriver.yaml index eaed5fce7ba..b5d45f9ce04 100644 --- a/screwdriver.yaml +++ b/screwdriver.yaml @@ -126,7 +126,6 @@ jobs: - checkout: | WORKDIR=$(pwd)/.. cd $WORKDIR - git clone -q https://github.com/vespa-engine/system-test git clone -q --depth 1 https://github.com/vespa-engine/sample-apps diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.cpp index 0359c13a5f7..5c4cfb393a4 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.cpp +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.cpp @@ -21,15 +21,21 @@ MyBucketModifiedHandler::notifyBucketModified(const BucketId &bucket) { MyMoveHandler::MyMoveHandler(bucketdb::BucketDBOwner &bucketDb, bool storeMoveDoneContext) : _bucketDb(bucketDb), _moves(), - _numCachedBuckets(), + _lids2Fail(), + _numFailedMoves(0), + _numCachedBuckets(0), _storeMoveDoneContexts(storeMoveDoneContext), _moveDoneContexts() {} MyMoveHandler::~MyMoveHandler() = default; -void +IDocumentMoveHandler::MoveResult MyMoveHandler::handleMove(MoveOperation &op, IDestructorCallback::SP moveDoneCtx) { + if (_lids2Fail.contains(op.getPrevLid())) { + _numFailedMoves++; + return MoveResult::FAILURE; + } _moves.push_back(op); if (_bucketDb.takeGuard()->isCachedBucket(op.getBucketId())) { ++_numCachedBuckets; @@ -37,6 +43,7 @@ MyMoveHandler::handleMove(MoveOperation &op, IDestructorCallback::SP moveDoneCtx if (_storeMoveDoneContexts) { _moveDoneContexts.push_back(std::move(moveDoneCtx)); } + return MoveResult::SUCCESS; } MySubDb::MySubDb(const std::shared_ptr<const DocumentTypeRepo> &repo, std::shared_ptr<bucketdb::BucketDBOwner> bucketDB, @@ -70,6 +77,13 @@ MySubDb::insertDocs(const UserDocuments &docs_) { } bool +MySubDb::remove(uint32_t subDbId, uint32_t lid) { + if (_subDb.sub_db_id() != subDbId) return false; + if (!_metaStore.validLid(lid)) return false; + return _metaStore.remove(lid, 0u); +} + +bool assertEqual(const document::BucketId &bucket, const proton::test::Document &doc, uint32_t sourceSubDbId, uint32_t targetSubDbId, const MoveOperation &op) { if (!EXPECT_EQUAL(bucket, op.getBucketId())) return false; diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.h b/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.h index 9d5c4e0e4ec..e21f084ff6e 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.h +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.h @@ -37,13 +37,24 @@ struct MyMoveHandler : public IDocumentMoveHandler { using MoveOperationVector = std::vector<MoveOperation>; bucketdb::BucketDBOwner &_bucketDb; MoveOperationVector _moves; + std::set<uint32_t> _lids2Fail; + size_t _numFailedMoves; size_t _numCachedBuckets; - bool _storeMoveDoneContexts; + bool _storeMoveDoneContexts; + std::vector<vespalib::IDestructorCallback::SP> _moveDoneContexts; - MyMoveHandler(bucketdb::BucketDBOwner &bucketDb, bool storeMoveDoneContext = false); + explicit MyMoveHandler(bucketdb::BucketDBOwner &bucketDb) : MyMoveHandler(bucketDb, false){} + MyMoveHandler(bucketdb::BucketDBOwner &bucketDb, bool storeMoveDoneContext); ~MyMoveHandler() override; - void handleMove(MoveOperation &op, vespalib::IDestructorCallback::SP moveDoneCtx) override; + MoveResult handleMove(MoveOperation &op, vespalib::IDestructorCallback::SP moveDoneCtx) override; + + void addLid2Fail(uint32_t lid) { + _lids2Fail.insert(lid); + } + void removeLids2Fail(uint32_t lid) { + _lids2Fail.erase(lid); + } void reset() { _moves.clear(); @@ -66,7 +77,7 @@ struct MyDocumentRetriever : public DocumentRetrieverBaseForTest { DocumentVector _docs; uint32_t _lid2Fail; - MyDocumentRetriever(std::shared_ptr<const DocumentTypeRepo> repo) + explicit MyDocumentRetriever(std::shared_ptr<const DocumentTypeRepo> repo) : _repo(std::move(repo)), _docs(), _lid2Fail(0) @@ -78,7 +89,7 @@ struct MyDocumentRetriever : public DocumentRetrieverBaseForTest { void getBucketMetaData(const storage::spi::Bucket &, DocumentMetaData::Vector &) const override {} - DocumentMetaData getDocumentMetaData(const DocumentId &) const override { return DocumentMetaData(); } + DocumentMetaData getDocumentMetaData(const DocumentId &) const override { return {}; } Document::UP getFullDocument(DocumentIdT lid) const override { return (lid != _lid2Fail) ? Document::UP(_docs[lid]->clone()) : Document::UP(); @@ -130,11 +141,13 @@ struct MySubDb { return _docs.getBucket(userId); } - DocumentVector docs(uint32_t userId) { + DocumentVector docs(uint32_t userId) const { return _docs.getGidOrderDocs(userId); } void setBucketState(const BucketId &bucketId, bool active); + // Will remove from metastore so it is invisible. + bool remove(uint32_t subDbId, uint32_t lid); }; struct MyCountJobRunner : public IMaintenanceJobRunner { diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp index 3f4fd0c1a8f..e9407d5445e 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp @@ -75,6 +75,9 @@ struct ControllerFixtureBase : public ::testing::Test _bucketHandler.notifyBucketStateChanged(bucket, BucketInfo::ActiveState::NOT_ACTIVE); return *this; } + bool removeFromSubDB(uint32_t subdbId, uint32_t lid) { + return _ready.remove(subdbId, lid) || _notReady.remove(subdbId, lid); + } void failRetrieveForLid(uint32_t lid) { _ready.failRetrieveForLid(lid); _notReady.failRetrieveForLid(lid); @@ -237,6 +240,7 @@ TEST_F(ControllerFixture, require_that_bucket_is_moved_even_with_error) }); sync(); EXPECT_FALSE(_bmj->done()); + EXPECT_TRUE(docsMoved().empty()); fixRetriever(); masterExecute([this]() { EXPECT_TRUE(_bmj->scanAndMove(4, 3)); @@ -250,6 +254,38 @@ TEST_F(ControllerFixture, require_that_bucket_is_moved_even_with_error) EXPECT_EQ(_ready.bucket(2), bucketsModified()[0]); } +TEST_F(ControllerFixture, require_that_bucket_is_moved_even_with_handler_error) +{ + // bucket 2 should be moved + addReady(_ready.bucket(1)); + _bmj->recompute(); + _moveHandler.addLid2Fail(5); + masterExecute([this]() { + EXPECT_FALSE(_bmj->done()); + EXPECT_TRUE(_bmj->scanAndMove(4, 3)); + EXPECT_TRUE(_bmj->done()); + }); + sync(); + EXPECT_FALSE(_bmj->done()); + EXPECT_EQ(1u, _moveHandler._numFailedMoves); + EXPECT_EQ(1u, docsMoved().size()); + assertEqual(_ready.bucket(2), _ready.docs(2)[1], 1, 2, docsMoved()[0]); + const auto & doc = docsMoved()[0]; + EXPECT_TRUE(removeFromSubDB(doc.getPrevSubDbId(), doc.getPrevLid())); // Explicit remove as movehandler only observes move attempt. + _moveHandler.removeLids2Fail(5); + masterExecute([this]() { + EXPECT_TRUE(_bmj->scanAndMove(4, 3)); + EXPECT_TRUE(_bmj->done()); + }); + sync(); + EXPECT_EQ(2u, docsMoved().size()); + // First document is from previous move round. Handler just appends all operations. + assertEqual(_ready.bucket(2), _ready.docs(2)[1], 1, 2, docsMoved()[0]); + assertEqual(_ready.bucket(2), _ready.docs(2)[0], 1, 2, docsMoved()[1]); + EXPECT_EQ(1u, bucketsModified().size()); + EXPECT_EQ(_ready.bucket(2), bucketsModified()[0]); +} + TEST_F(ControllerFixture, require_that_we_move_buckets_in_several_steps) { diff --git a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp index 2a074a7404c..509210679da 100644 --- a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp +++ b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp @@ -40,7 +40,6 @@ #include <vespa/vespalib/util/gate.h> #include <vespa/vespalib/util/lambdatask.h> #include <vespa/vespalib/util/monitored_refcount.h> -#include <vespa/vespalib/util/size_literals.h> #include <vespa/vespalib/util/threadstackexecutor.h> #include <unistd.h> #include <thread> @@ -216,7 +215,8 @@ public: ~MyFeedHandler() override; bool isExecutorThread() const; - void handleMove(MoveOperation &op, IDestructorCallback::SP moveDoneCtx) override; + + MoveResult handleMove(MoveOperation &op, IDestructorCallback::SP moveDoneCtx) override; void performPruneRemovedDocuments(PruneRemovedDocumentsOperation &op) override; void heartBeat() override; @@ -624,7 +624,7 @@ MyFeedHandler::isExecutorThread() const } -void +IDocumentMoveHandler::MoveResult MyFeedHandler::handleMove(MoveOperation &op, IDestructorCallback::SP moveDoneCtx) { assert(isExecutorThread()); @@ -640,6 +640,7 @@ MyFeedHandler::handleMove(MoveOperation &op, IDestructorCallback::SP moveDoneCtx appendOperation(op, std::move(moveDoneCtx)); _subDBs[op.getSubDbId()]->handleMove(op); _subDBs[op.getPrevSubDbId()]->handleMove(op); + return MoveResult::SUCCESS; } diff --git a/searchcore/src/vespa/searchcore/proton/server/combiningfeedview.cpp b/searchcore/src/vespa/searchcore/proton/server/combiningfeedview.cpp index eadeb6b1e75..612b31b0123 100644 --- a/searchcore/src/vespa/searchcore/proton/server/combiningfeedview.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/combiningfeedview.cpp @@ -193,6 +193,13 @@ CombiningFeedView::handleDeleteBucket(const DeleteBucketOperation &delOp, DoneCa } } +bool +CombiningFeedView::isMoveStillValid(const MoveOperation & moveOp) const { + uint32_t subDbId = moveOp.getPrevSubDbId(); + assert(subDbId < _views.size()); + return _views[subDbId]->isMoveStillValid(moveOp); +} + void CombiningFeedView::prepareMove(MoveOperation &moveOp) { diff --git a/searchcore/src/vespa/searchcore/proton/server/combiningfeedview.h b/searchcore/src/vespa/searchcore/proton/server/combiningfeedview.h index 0b6f6039e36..2890aa7ac0f 100644 --- a/searchcore/src/vespa/searchcore/proton/server/combiningfeedview.h +++ b/searchcore/src/vespa/searchcore/proton/server/combiningfeedview.h @@ -72,6 +72,7 @@ public: void prepareRemove(RemoveOperation &rmOp) override; void handleRemove(FeedToken token, const RemoveOperation &rmOp) override; void prepareDeleteBucket(DeleteBucketOperation &delOp) override; + bool isMoveStillValid(const MoveOperation & moveOp) const override; void prepareMove(MoveOperation &putOp) override; void handleDeleteBucket(const DeleteBucketOperation &delOp, DoneCallback onDone) override; void handleMove(const MoveOperation &moveOp, DoneCallback onDone) override; diff --git a/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp b/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp index 4d410282b42..bc282786b04 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp @@ -38,7 +38,10 @@ BucketMover::createMoveOperation(const MoveKey &key) { void BucketMover::moveDocument(MoveOperationUP moveOp, IDestructorCallbackSP onDone) { - _handler->handleMove(*moveOp, std::move(onDone)); + auto result = _handler->handleMove(*moveOp, std::move(onDone)); + if (result == IDocumentMoveHandler::MoveResult::FAILURE) { + enableReschedule(); + } } BucketMover::MoveKey::MoveKey(uint32_t lid, const document::GlobalId &gid, Timestamp timestamp, MoveGuard guard) noexcept @@ -125,7 +128,7 @@ BucketMover::createMoveOperations(MoveKeys toMove) { } } if ( ! moveOps.failed().empty()) { - _needReschedule.store(true, std::memory_order_relaxed); + enableReschedule(); } return moveOps; } @@ -141,7 +144,7 @@ void BucketMover::cancel() { _cancelled = true; setAllScheduled(); - _needReschedule.store(true, std::memory_order_relaxed); + enableReschedule(); } } diff --git a/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.h b/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.h index 1900f96d6da..0437a258c3b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.h +++ b/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.h @@ -156,6 +156,9 @@ private: size_t pending() const { return _started.load(std::memory_order_relaxed) - _completed.load(std::memory_order_relaxed); } + void enableReschedule() { + _needReschedule.store(true, std::memory_order_relaxed); + } }; } diff --git a/searchcore/src/vespa/searchcore/proton/server/feedhandler.cpp b/searchcore/src/vespa/searchcore/proton/server/feedhandler.cpp index f5774734949..3c957fc3cf2 100644 --- a/searchcore/src/vespa/searchcore/proton/server/feedhandler.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/feedhandler.cpp @@ -756,10 +756,12 @@ FeedHandler::handleOperation(FeedToken token, FeedOperation::UP op) })); } -void +IDocumentMoveHandler::MoveResult FeedHandler::handleMove(MoveOperation &op, vespalib::IDestructorCallback::SP moveDoneCtx) { assert(_writeService.master().isCurrentThread()); + if ( ! _activeFeedView->isMoveStillValid(op)) return MoveResult::FAILURE; + op.set_prepare_serial_num(inc_prepare_serial_num()); _activeFeedView->prepareMove(op); assert(op.getValidDbdId()); @@ -767,6 +769,7 @@ FeedHandler::handleMove(MoveOperation &op, vespalib::IDestructorCallback::SP mov assert(op.getSubDbId() != op.getPrevSubDbId()); appendOperation(op, moveDoneCtx); _activeFeedView->handleMove(op, std::move(moveDoneCtx)); + return MoveResult::SUCCESS; } void diff --git a/searchcore/src/vespa/searchcore/proton/server/feedhandler.h b/searchcore/src/vespa/searchcore/proton/server/feedhandler.h index dddc032ba04..06fca28a55c 100644 --- a/searchcore/src/vespa/searchcore/proton/server/feedhandler.h +++ b/searchcore/src/vespa/searchcore/proton/server/feedhandler.h @@ -239,7 +239,7 @@ public: void performOperation(FeedToken token, FeedOperationUP op); void handleOperation(FeedToken token, FeedOperationUP op); - void handleMove(MoveOperation &op, std::shared_ptr<vespalib::IDestructorCallback> moveDoneCtx) override; + MoveResult handleMove(MoveOperation &op, std::shared_ptr<vespalib::IDestructorCallback> moveDoneCtx) override; void heartBeat() override; RPC::Result receive(const Packet &packet) override; diff --git a/searchcore/src/vespa/searchcore/proton/server/idocumentmovehandler.h b/searchcore/src/vespa/searchcore/proton/server/idocumentmovehandler.h index f5ad1cd596f..98bcc7c1265 100644 --- a/searchcore/src/vespa/searchcore/proton/server/idocumentmovehandler.h +++ b/searchcore/src/vespa/searchcore/proton/server/idocumentmovehandler.h @@ -15,7 +15,8 @@ class MoveOperation; */ struct IDocumentMoveHandler { - virtual void handleMove(MoveOperation &op, std::shared_ptr<vespalib::IDestructorCallback> moveDoneCtx) = 0; + enum class MoveResult { SUCCESS, FAILURE}; + [[nodiscard]] virtual MoveResult handleMove(MoveOperation &op, std::shared_ptr<vespalib::IDestructorCallback> moveDoneCtx) = 0; virtual ~IDocumentMoveHandler() = default; }; diff --git a/searchcore/src/vespa/searchcore/proton/server/ifeedview.h b/searchcore/src/vespa/searchcore/proton/server/ifeedview.h index 946d3a9f2f1..d3a43b535da 100644 --- a/searchcore/src/vespa/searchcore/proton/server/ifeedview.h +++ b/searchcore/src/vespa/searchcore/proton/server/ifeedview.h @@ -56,6 +56,7 @@ public: virtual void prepareRemove(RemoveOperation &rmOp) = 0; virtual void handleRemove(FeedToken token, const RemoveOperation &rmOp) = 0; virtual void prepareDeleteBucket(DeleteBucketOperation &delOp) = 0; + [[nodiscard]] virtual bool isMoveStillValid(const MoveOperation & moveOp) const = 0; virtual void prepareMove(MoveOperation &putOp) = 0; virtual void handleDeleteBucket(const DeleteBucketOperation &delOp, DoneCallback onDone) = 0; virtual void handleMove(const MoveOperation &putOp, DoneCallback onDone) = 0; diff --git a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp index 4608f9c4959..7345a07cd27 100644 --- a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp @@ -34,11 +34,11 @@ namespace { bool isSameDocument(const search::DocumentMetaData &a, const search::DocumentMetaData &b) { + //TODO Timestamp check can be removed once logic has proved itself in large scale. return (a.lid == b.lid) && (a.bucketId == b.bucketId) && (a.gid == b.gid) && - (a.timestamp == - b.timestamp); // Timestamp check can be removed once logic has proved itself in large scale. + (a.timestamp == b.timestamp); } } @@ -111,7 +111,6 @@ CompactionJob::completeMove(const search::DocumentMetaData & metaThen, std::uniq // Reread meta data as document might have been altered after move was initiated // If so it will fail the timestamp sanity check later on. search::DocumentMetaData metaNow = _handler->getMetaData(metaThen.lid); - // This should be impossible and should probably be an assert if ( ! isSameDocument(metaThen, metaNow)) return; if (metaNow.gid != moveOp->getDocument()->getId().getGlobalId()) return; diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp index 4c43c11691c..27c5b835edb 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp @@ -2,13 +2,11 @@ #include "storeonlyfeedview.h" #include "forcecommitcontext.h" -#include "ireplayconfig.h" #include "operationdonecontext.h" #include "putdonecontext.h" #include "removedonecontext.h" #include "updatedonecontext.h" #include <vespa/searchcore/proton/attribute/ifieldupdatecallback.h> -#include <vespa/searchcore/proton/common/feedtoken.h> #include <vespa/searchcore/proton/feedoperation/operations.h> #include <vespa/searchcore/proton/reference/i_gid_to_lid_change_handler.h> #include <vespa/searchcore/proton/reference/i_pending_gid_to_lid_changes.h> @@ -712,6 +710,15 @@ StoreOnlyFeedView::internalDeleteBucket(const DeleteBucketOperation &delOp, Done _params._docTypeName.toString().c_str(), delOp.getBucketId().toString().c_str(), rm_count); } +bool +StoreOnlyFeedView::isMoveStillValid(const MoveOperation & moveOp) const { + uint32_t lid = moveOp.getPrevLid(); + if ( ! _metaStore.validLid(lid)) return false; + const RawDocumentMetaData & meta = _metaStore.getRawMetaData(lid); + return (meta.getTimestamp() == moveOp.getTimestamp()) && + (meta.getGid() == moveOp.getDocument()->getId().getGlobalId()); +} + // CombiningFeedView calls this only for the subdb we're moving to. void StoreOnlyFeedView::prepareMove(MoveOperation &moveOp) diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h index 5e9ebf4d3a6..0e1a4e09482 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h @@ -215,6 +215,7 @@ public: void handleRemove(FeedToken token, const RemoveOperation &rmOp) override; void prepareDeleteBucket(DeleteBucketOperation &delOp) override; void handleDeleteBucket(const DeleteBucketOperation &delOp, DoneCallback onDone) override; + bool isMoveStillValid(const MoveOperation & moveOp) const override; void prepareMove(MoveOperation &putOp) override; void handleMove(const MoveOperation &putOp, DoneCallback doneCtx) override; void heartBeat(search::SerialNum serialNum, DoneCallback onDone) override; diff --git a/searchcore/src/vespa/searchcore/proton/test/buckethandler.cpp b/searchcore/src/vespa/searchcore/proton/test/buckethandler.cpp index 815c9741c07..52acb059a97 100644 --- a/searchcore/src/vespa/searchcore/proton/test/buckethandler.cpp +++ b/searchcore/src/vespa/searchcore/proton/test/buckethandler.cpp @@ -23,16 +23,14 @@ BucketHandler::addBucketStateChangedHandler(IBucketStateChangedHandler *handler) } void -BucketHandler::removeBucketStateChangedHandler(IBucketStateChangedHandler * - handler) +BucketHandler::removeBucketStateChangedHandler(IBucketStateChangedHandler * handler) { _handlers.erase(handler); } void BucketHandler::notifyBucketStateChanged(const document::BucketId &bucketId, - storage::spi::BucketInfo::ActiveState - newState) + storage::spi::BucketInfo::ActiveState newState) { for (auto &handler : _handlers) { handler->notifyBucketStateChanged(bucketId, newState); diff --git a/searchcore/src/vespa/searchcore/proton/test/buckethandler.h b/searchcore/src/vespa/searchcore/proton/test/buckethandler.h index 47dc3145f72..54f721fad86 100644 --- a/searchcore/src/vespa/searchcore/proton/test/buckethandler.h +++ b/searchcore/src/vespa/searchcore/proton/test/buckethandler.h @@ -6,11 +6,7 @@ #include <vespa/searchcore/proton/server/ibucketstatechangedhandler.h> #include <set> -namespace proton -{ - -namespace test -{ +namespace proton::test { /** * Test bucket handler that forwards bucket state change notifications @@ -21,22 +17,12 @@ class BucketHandler : public IBucketStateChangedNotifier std::set<IBucketStateChangedHandler *> _handlers; public: BucketHandler(); + ~BucketHandler() override; + void addBucketStateChangedHandler(IBucketStateChangedHandler *handler) override; + void removeBucketStateChangedHandler(IBucketStateChangedHandler *handler) override; - virtual - ~BucketHandler(); - - virtual void - addBucketStateChangedHandler(IBucketStateChangedHandler *handler) override; - - virtual void - removeBucketStateChangedHandler(IBucketStateChangedHandler *handler) override; - - void - notifyBucketStateChanged(const document::BucketId &bucketId, - storage::spi::BucketInfo::ActiveState newState); + void notifyBucketStateChanged(const document::BucketId &bucketId, + storage::spi::BucketInfo::ActiveState newState); }; - -} // namespace test - -} // namespace proton +} diff --git a/searchcore/src/vespa/searchcore/proton/test/dummy_feed_view.h b/searchcore/src/vespa/searchcore/proton/test/dummy_feed_view.h index f8713b3b691..751d4464ca7 100644 --- a/searchcore/src/vespa/searchcore/proton/test/dummy_feed_view.h +++ b/searchcore/src/vespa/searchcore/proton/test/dummy_feed_view.h @@ -26,6 +26,7 @@ struct DummyFeedView : public IFeedView void handleRemove(FeedToken, const RemoveOperation &) override {} void prepareDeleteBucket(DeleteBucketOperation &) override {} void handleDeleteBucket(const DeleteBucketOperation &, DoneCallback) override {} + bool isMoveStillValid(const MoveOperation &) const override { return true; } void prepareMove(MoveOperation &) override {} void handleMove(const MoveOperation &, DoneCallback) override {} void heartBeat(search::SerialNum, DoneCallback) override {} diff --git a/searchlib/src/vespa/searchcommon/attribute/iattributevector.h b/searchlib/src/vespa/searchcommon/attribute/iattributevector.h index d613bf61a16..b44b6a4baf2 100644 --- a/searchlib/src/vespa/searchcommon/attribute/iattributevector.h +++ b/searchlib/src/vespa/searchcommon/attribute/iattributevector.h @@ -419,6 +419,13 @@ public: */ virtual bool isImported() const = 0; + /* + * Returns whether the match setting for the attribute is uncased. + * This is only relevant for string attributes (i.e. when isStringType() + * returns true). The default for string attributes is uncased matching. + */ + virtual bool has_uncased_matching() const noexcept { return true; } + /** * Will serialize the values for the documentid in ascending order. The serialized form can be used by memcmp and * sortorder will be preserved. diff --git a/searchlib/src/vespa/searchlib/attribute/stringbase.cpp b/searchlib/src/vespa/searchlib/attribute/stringbase.cpp index ac8178f8afa..0e261059777 100644 --- a/searchlib/src/vespa/searchlib/attribute/stringbase.cpp +++ b/searchlib/src/vespa/searchlib/attribute/stringbase.cpp @@ -283,6 +283,12 @@ StringAttribute::get_match_is_cased() const noexcept { return getConfig().get_match() == attribute::Config::Match::CASED; } +bool +StringAttribute::has_uncased_matching() const noexcept +{ + return !get_match_is_cased(); +} + template bool AttributeVector::clearDoc(StringAttribute::ChangeVector& changes, DocId doc); template bool AttributeVector::update(StringAttribute::ChangeVector& changes, DocId doc, const StringChangeData& v); template bool AttributeVector::append(StringAttribute::ChangeVector& changes, DocId doc, const StringChangeData& v, int32_t w, bool doCount); diff --git a/searchlib/src/vespa/searchlib/attribute/stringbase.h b/searchlib/src/vespa/searchlib/attribute/stringbase.h index 1440b945428..e65b181f37d 100644 --- a/searchlib/src/vespa/searchlib/attribute/stringbase.h +++ b/searchlib/src/vespa/searchlib/attribute/stringbase.h @@ -72,6 +72,7 @@ protected: vespalib::MemoryUsage getChangeVectorMemoryUsage() const override; bool get_match_is_cased() const noexcept; + bool has_uncased_matching() const noexcept override; long onSerializeForAscendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const override; long onSerializeForDescendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const override; private: diff --git a/searchsummary/CMakeLists.txt b/searchsummary/CMakeLists.txt index f771a8e4494..a5d65721aeb 100644 --- a/searchsummary/CMakeLists.txt +++ b/searchsummary/CMakeLists.txt @@ -19,6 +19,7 @@ vespa_define_module( src/tests/docsummary/annotation_converter src/tests/docsummary/attribute_combiner src/tests/docsummary/attributedfw + src/tests/docsummary/attribute_tokens_dfw src/tests/docsummary/document_id_dfw src/tests/docsummary/tokens_converter src/tests/docsummary/matched_elements_filter diff --git a/searchsummary/src/tests/docsummary/attribute_tokens_dfw/CMakeLists.txt b/searchsummary/src/tests/docsummary/attribute_tokens_dfw/CMakeLists.txt new file mode 100644 index 00000000000..adcb18585d0 --- /dev/null +++ b/searchsummary/src/tests/docsummary/attribute_tokens_dfw/CMakeLists.txt @@ -0,0 +1,10 @@ +# Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(searchsummary_attribute_tokens_dfw_test_app TEST + SOURCES + attribute_tokens_dfw_test.cpp + DEPENDS + searchsummary + searchsummary_test + GTest::GTest +) +vespa_add_test(NAME searchsummary_attribute_tokens_dfw_test_app COMMAND searchsummary_attribute_tokens_dfw_test_app) diff --git a/searchsummary/src/tests/docsummary/attribute_tokens_dfw/attribute_tokens_dfw_test.cpp b/searchsummary/src/tests/docsummary/attribute_tokens_dfw/attribute_tokens_dfw_test.cpp new file mode 100644 index 00000000000..bac817077c4 --- /dev/null +++ b/searchsummary/src/tests/docsummary/attribute_tokens_dfw/attribute_tokens_dfw_test.cpp @@ -0,0 +1,97 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/searchsummary/docsummary/attribute_tokens_dfw.h> +#include <vespa/searchsummary/test/mock_attribute_manager.h> +#include <vespa/searchsummary/test/mock_state_callback.h> +#include <vespa/searchsummary/test/slime_value.h> +#include <vespa/vespalib/gtest/gtest.h> + +#include <vespa/log/log.h> +LOG_SETUP("attribute_tokens_dfw_test"); + +using search::attribute::CollectionType; +using search::docsummary::AttributeTokensDFW; +using search::docsummary::GetDocsumsState; +using search::docsummary::DocsumFieldWriter; +using search::docsummary::test::MockAttributeManager; +using search::docsummary::test::MockStateCallback; +using search::docsummary::test::SlimeValue; + +class AttributeTokensDFWTest : public ::testing::Test { +protected: + MockAttributeManager _attrs; + std::unique_ptr<DocsumFieldWriter> _writer; + MockStateCallback _callback; + GetDocsumsState _state; + std::shared_ptr<search::MatchingElementsFields> _matching_elems_fields; + vespalib::string _field_name; + +public: + AttributeTokensDFWTest() + : _attrs(), + _writer(), + _callback(), + _state(_callback), + _matching_elems_fields(), + _field_name() + { + _attrs.build_string_attribute("array_str", { {"This", "is", "A TEST"}, {} }); + _attrs.build_string_attribute("cased_array_str", { {"CASING", "Matters here" }, {} }, CollectionType::ARRAY, false); + _attrs.build_string_attribute("wset_str", { {"This is", "b", "C"}, {} }, CollectionType::WSET); + _attrs.build_string_attribute("single_str", { {"Hello World"}, {} }, CollectionType::SINGLE); + _state._attrCtx = _attrs.mgr().createContext(); + } + ~AttributeTokensDFWTest() {} + + void setup(const vespalib::string& field_name) { + _writer = std::make_unique<AttributeTokensDFW>(field_name); + _writer->setIndex(0); + auto attr = _state._attrCtx->getAttribute(field_name); + EXPECT_TRUE(_writer->setFieldWriterStateIndex(0)); + _state._fieldWriterStates.resize(1); + _field_name = field_name; + _state._attributes.resize(1); + _state._attributes[0] = attr; + } + + void expect_field(const vespalib::string& exp_slime_as_json, uint32_t docid) { + vespalib::Slime act; + vespalib::slime::SlimeInserter inserter(act); + if (!_writer->isDefaultValue(docid, _state)) { + _writer->insertField(docid, nullptr, _state, inserter); + } + + SlimeValue exp(exp_slime_as_json); + EXPECT_EQ(exp.slime, act); + } +}; + +TEST_F(AttributeTokensDFWTest, outputs_slime_for_array_of_string) +{ + setup("array_str"); + expect_field("[ ['this' ], [ 'is' ], [ 'a test' ] ]", 1); + expect_field("null", 2); +} + +TEST_F(AttributeTokensDFWTest, outputs_slime_for_cased_array_of_string) +{ + setup("cased_array_str"); + expect_field("[ ['CASING' ], [ 'Matters here' ] ]", 1); + expect_field("null", 2); +} + +TEST_F(AttributeTokensDFWTest, outputs_slime_for_wset_of_string) +{ + setup("wset_str"); + expect_field("[ ['this is'], [ 'b' ], [ 'c' ] ]", 1); + expect_field("null", 2); +} + +TEST_F(AttributeTokensDFWTest, single_string) +{ + setup("single_str"); + expect_field("[ 'hello world' ]", 1); + expect_field("[ '' ]", 2); +} + +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchsummary/src/vespa/searchsummary/docsummary/CMakeLists.txt b/searchsummary/src/vespa/searchsummary/docsummary/CMakeLists.txt index 0287517f830..84ceb18ef6f 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/CMakeLists.txt +++ b/searchsummary/src/vespa/searchsummary/docsummary/CMakeLists.txt @@ -5,6 +5,7 @@ vespa_add_library(searchsummary_docsummary OBJECT array_attribute_combiner_dfw.cpp attribute_combiner_dfw.cpp attribute_field_writer.cpp + attribute_tokens_dfw.cpp attributedfw.cpp check_undefined_value_visitor.cpp copy_dfw.cpp @@ -17,6 +18,7 @@ vespa_add_library(searchsummary_docsummary OBJECT document_id_dfw.cpp dynamicteaserdfw.cpp empty_dfw.cpp + empty_docsum_field_writer_state.cpp geoposdfw.cpp getdocsumargs.cpp juniper_dfw_query_item.cpp diff --git a/searchsummary/src/vespa/searchsummary/docsummary/attribute_tokens_dfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/attribute_tokens_dfw.cpp new file mode 100644 index 00000000000..9e0dafc5e91 --- /dev/null +++ b/searchsummary/src/vespa/searchsummary/docsummary/attribute_tokens_dfw.cpp @@ -0,0 +1,179 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "attribute_tokens_dfw.h" +#include "docsumstate.h" +#include "empty_docsum_field_writer_state.h" +#include <vespa/searchcommon/attribute/iattributevector.h> +#include <vespa/searchcommon/attribute/i_multi_value_attribute.h> +#include <vespa/vespalib/data/slime/slime.h> +#include <vespa/vespalib/text/lowercase.h> +#include <vespa/vespalib/text/utf8.h> +#include <vespa/vespalib/util/stash.h> + +using search::attribute::IAttributeVector; +using search::attribute::BasicType; +using search::attribute::IMultiValueAttribute; +using search::attribute::IMultiValueReadView; +using vespalib::LowerCase; +using vespalib::Utf8Reader; +using vespalib::Utf8Writer; +using vespalib::slime::ArrayInserter; +using vespalib::slime::Cursor; +using vespalib::slime::Inserter; + +namespace search::docsummary { + +namespace { + +const IMultiValueReadView<const char*>* +make_read_view(const IAttributeVector& attribute, vespalib::Stash& stash) +{ + auto multi_value_attribute = attribute.as_multi_value_attribute(); + if (multi_value_attribute != nullptr) { + return multi_value_attribute->make_read_view(IMultiValueAttribute::MultiValueTag<const char*>(), stash); + } + return nullptr; +} + +void +insert_value(vespalib::stringref value, Inserter& inserter, vespalib::string& scratch, bool lowercase) +{ + Cursor& arr = inserter.insertArray(1); + ArrayInserter ai(arr); + if (lowercase) { + scratch.clear(); + Utf8Reader r(value); + Utf8Writer w(scratch); + while (r.hasMore()) { + w.putChar(LowerCase::convert(r.getChar())); + } + ai.insertString(scratch); + } else { + ai.insertString(value); + } +} + +} + +class MultiAttributeTokensDFWState : public DocsumFieldWriterState +{ + const IMultiValueReadView<const char*>* _read_view; + vespalib::string _lowercase_scratch; + bool _lowercase; +public: + MultiAttributeTokensDFWState(const IAttributeVector& attr, vespalib::Stash& stash); + ~MultiAttributeTokensDFWState() override; + void insertField(uint32_t docid, Inserter& target) override; +}; + +MultiAttributeTokensDFWState::MultiAttributeTokensDFWState(const IAttributeVector& attr, vespalib::Stash& stash) + : DocsumFieldWriterState(), + _read_view(make_read_view(attr, stash)), + _lowercase_scratch(), + _lowercase(attr.has_uncased_matching()) +{ +} + +MultiAttributeTokensDFWState::~MultiAttributeTokensDFWState() = default; + +void +MultiAttributeTokensDFWState::insertField(uint32_t docid, Inserter& target) +{ + if (!_read_view) { + return; + } + auto elements = _read_view->get_values(docid); + if (elements.empty()) { + return; + } + Cursor &arr = target.insertArray(elements.size()); + ArrayInserter ai(arr); + for (const auto & element : elements) { + insert_value(element, ai, _lowercase_scratch, _lowercase); + } +} + +class SingleAttributeTokensDFWState : public DocsumFieldWriterState +{ + const IAttributeVector& _attr; + vespalib::string _lowercase_scratch; + bool _lowercase; +public: + SingleAttributeTokensDFWState(const IAttributeVector& attr); + ~SingleAttributeTokensDFWState() override; + void insertField(uint32_t docid, Inserter& target) override; +}; + +SingleAttributeTokensDFWState::SingleAttributeTokensDFWState(const IAttributeVector& attr) + : DocsumFieldWriterState(), + _attr(attr), + _lowercase_scratch(), + _lowercase(attr.has_uncased_matching()) +{ +} + +SingleAttributeTokensDFWState::~SingleAttributeTokensDFWState() = default; + +void +SingleAttributeTokensDFWState::insertField(uint32_t docid, Inserter& target) +{ + auto s = _attr.get_raw(docid); + insert_value(vespalib::stringref(s.data(), s.size()), target, _lowercase_scratch, _lowercase); +} + +DocsumFieldWriterState* +make_field_writer_state(const IAttributeVector& attr, vespalib::Stash& stash) +{ + auto type = attr.getBasicType(); + switch (type) { + case BasicType::Type::STRING: + if (attr.hasMultiValue()) { + return &stash.create<MultiAttributeTokensDFWState>(attr, stash); + } else { + return &stash.create<SingleAttributeTokensDFWState>(attr); + } + default: + ; + } + return &stash.create<EmptyDocsumFieldWriterState>(); +} + +AttributeTokensDFW::AttributeTokensDFW(const vespalib::string& input_field_name) + : DocsumFieldWriter(), + _input_field_name(input_field_name) +{ +} + +AttributeTokensDFW::~AttributeTokensDFW() = default; + +const vespalib::string& +AttributeTokensDFW::getAttributeName() const +{ + return _input_field_name; +} + +bool +AttributeTokensDFW::isGenerated() const +{ + return true; +} + +bool +AttributeTokensDFW::setFieldWriterStateIndex(uint32_t fieldWriterStateIndex) +{ + _state_index = fieldWriterStateIndex; + return true; +} + +void +AttributeTokensDFW::insertField(uint32_t docid, const IDocsumStoreDocument*, GetDocsumsState& state, vespalib::slime::Inserter& target) const +{ + auto& field_writer_state = state._fieldWriterStates[_state_index]; + if (!field_writer_state) { + const auto& attr = *state.getAttribute(getIndex()); + field_writer_state = make_field_writer_state(attr, state.get_stash()); + } + field_writer_state->insertField(docid, target); +} + +} diff --git a/searchsummary/src/vespa/searchsummary/docsummary/attribute_tokens_dfw.h b/searchsummary/src/vespa/searchsummary/docsummary/attribute_tokens_dfw.h new file mode 100644 index 00000000000..53bb3d73290 --- /dev/null +++ b/searchsummary/src/vespa/searchsummary/docsummary/attribute_tokens_dfw.h @@ -0,0 +1,30 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "docsum_field_writer.h" + +namespace search::docsummary { + +/* + * Class for writing values from a string attribute vector as arrays + * containing the tokens. The string values are not split but they are + * lowercased if the string attribute vector uses uncased matching. + */ +class AttributeTokensDFW : public DocsumFieldWriter +{ +private: + vespalib::string _input_field_name; + uint32_t _state_index; // index into _fieldWriterStates in GetDocsumsState + +protected: + const vespalib::string & getAttributeName() const override; +public: + AttributeTokensDFW(const vespalib::string& input_field_name); + ~AttributeTokensDFW() override; + bool isGenerated() const override; + bool setFieldWriterStateIndex(uint32_t fieldWriterStateIndex) override; + void insertField(uint32_t docid, const IDocsumStoreDocument* doc, GetDocsumsState& state, vespalib::slime::Inserter& target) const override; +}; + +} diff --git a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp index 4ec406b7cf0..0fc26387c00 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp @@ -4,6 +4,7 @@ #include "docsumwriter.h" #include "docsumstate.h" #include "docsum_field_writer_state.h" +#include "empty_docsum_field_writer_state.h" #include <vespa/eval/eval/value.h> #include <vespa/eval/eval/value_codec.h> #include <vespa/searchcommon/attribute/i_multi_value_attribute.h> @@ -129,14 +130,6 @@ make_read_view(const IAttributeVector& attribute, vespalib::Stash& stash) return nullptr; } -class EmptyWriterState : public DocsumFieldWriterState -{ -public: - EmptyWriterState() = default; - ~EmptyWriterState() = default; - void insertField(uint32_t, Inserter&) override { } -}; - template <typename MultiValueType> class MultiAttrDFWState : public DocsumFieldWriterState { @@ -302,7 +295,7 @@ make_field_writer_state(const vespalib::string& field_name, const IAttributeVect default: ; } - return &stash.create<EmptyWriterState>(); + return &stash.create<EmptyDocsumFieldWriterState>(); } void diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_commands.cpp b/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_commands.cpp index 2ac5d1babbf..d3fc71b3173 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_commands.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_commands.cpp @@ -7,6 +7,7 @@ namespace search::docsummary::command { const vespalib::string abs_distance("absdist"); const vespalib::string attribute("attribute"); const vespalib::string attribute_combiner("attributecombiner"); +const vespalib::string attribute_tokens("attribute-tokens"); const vespalib::string copy("copy"); const vespalib::string documentid("documentid"); const vespalib::string dynamic_teaser("dynamicteaser"); diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_commands.h b/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_commands.h index d53351d8b04..d77416f2df5 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_commands.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_commands.h @@ -13,6 +13,7 @@ namespace search::docsummary::command { extern const vespalib::string abs_distance; extern const vespalib::string attribute; extern const vespalib::string attribute_combiner; +extern const vespalib::string attribute_tokens; extern const vespalib::string copy; extern const vespalib::string documentid; extern const vespalib::string dynamic_teaser; diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_factory.cpp b/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_factory.cpp index 2f7d9acdb65..28a2c34ca87 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_factory.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_factory.cpp @@ -1,6 +1,7 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "attribute_combiner_dfw.h" +#include "attribute_tokens_dfw.h" #include "copy_dfw.h" #include "docsum_field_writer_commands.h" #include "docsum_field_writer_factory.h" @@ -16,8 +17,10 @@ #include "tokens_dfw.h" #include <vespa/searchlib/common/matching_elements_fields.h> #include <vespa/vespalib/util/exceptions.h> +#include <vespa/vespalib/util/issue.h> using vespalib::IllegalArgumentException; +using vespalib::Issue; namespace search::docsummary { @@ -91,6 +94,21 @@ DocsumFieldWriterFactory::create_docsum_field_writer(const vespalib::string& fie } else { throw_missing_source(command); } + } else if (command == command::attribute_tokens) { + if (!source.empty()) { + if (has_attribute_manager()) { + auto ctx = getEnvironment().getAttributeManager()->createContext(); + const auto* attr = ctx->getAttribute(source); + if (attr == nullptr) { + Issue::report("No valid attribute vector found: field='%s', command='%s', source='%s'", + field_name.c_str(), command.c_str(), source.c_str()); + } else { + fieldWriter = std::make_unique<AttributeTokensDFW>(source); + } + } + } else { + throw_missing_source(command); + } } else if (command == command::abs_distance) { if (has_attribute_manager()) { fieldWriter = AbsDistanceDFW::create(source.c_str(), getEnvironment().getAttributeManager()); diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_state.h b/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_state.h index efacd0b1a49..ce151d82fc0 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_state.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer_state.h @@ -2,6 +2,8 @@ #pragma once +#include <cstdint> + namespace vespalib::slime { struct Inserter; } namespace search::docsummary { diff --git a/searchsummary/src/vespa/searchsummary/docsummary/empty_docsum_field_writer_state.cpp b/searchsummary/src/vespa/searchsummary/docsummary/empty_docsum_field_writer_state.cpp new file mode 100644 index 00000000000..b279430a367 --- /dev/null +++ b/searchsummary/src/vespa/searchsummary/docsummary/empty_docsum_field_writer_state.cpp @@ -0,0 +1,18 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "empty_docsum_field_writer_state.h" + +using vespalib::slime::Inserter; + +namespace search::docsummary { + +EmptyDocsumFieldWriterState::EmptyDocsumFieldWriterState() = default; + +EmptyDocsumFieldWriterState::~EmptyDocsumFieldWriterState() = default; + +void +EmptyDocsumFieldWriterState::insertField(uint32_t, Inserter&) +{ +} + +} diff --git a/searchsummary/src/vespa/searchsummary/docsummary/empty_docsum_field_writer_state.h b/searchsummary/src/vespa/searchsummary/docsummary/empty_docsum_field_writer_state.h new file mode 100644 index 00000000000..ff8319b52d1 --- /dev/null +++ b/searchsummary/src/vespa/searchsummary/docsummary/empty_docsum_field_writer_state.h @@ -0,0 +1,21 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "docsum_field_writer_state.h" + +namespace search::docsummary { + +/* + * Class used as fallback when no suitable field writer state could be + * instantiated. insertField() is a noop. + */ +class EmptyDocsumFieldWriterState : public DocsumFieldWriterState +{ +public: + EmptyDocsumFieldWriterState(); + ~EmptyDocsumFieldWriterState() override; + void insertField(uint32_t, vespalib::slime::Inserter&) override; +}; + +} diff --git a/searchsummary/src/vespa/searchsummary/test/mock_attribute_manager.cpp b/searchsummary/src/vespa/searchsummary/test/mock_attribute_manager.cpp index 557ba8f4cde..4d73420b523 100644 --- a/searchsummary/src/vespa/searchsummary/test/mock_attribute_manager.cpp +++ b/searchsummary/src/vespa/searchsummary/test/mock_attribute_manager.cpp @@ -20,9 +20,13 @@ template <typename AttributeType, typename ValueType> void MockAttributeManager::build_attribute(const vespalib::string& name, BasicType type, CollectionType col_type, - const std::vector<std::vector<ValueType>>& values) + const std::vector<std::vector<ValueType>>& values, + std::optional<bool> uncased) { Config cfg(type, col_type); + if (uncased.has_value()) { + cfg.set_match(uncased.value() ? Config::Match::UNCASED : Config::Match::CASED); + } auto attr_base = AttributeFactory::createAttribute(name, cfg); assert(attr_base); auto attr = std::dynamic_pointer_cast<AttributeType>(attr_base); @@ -55,9 +59,10 @@ MockAttributeManager::~MockAttributeManager() = default; void MockAttributeManager::build_string_attribute(const vespalib::string& name, const std::vector<std::vector<vespalib::string>>& values, - CollectionType col_type) + CollectionType col_type, + std::optional<bool> uncased) { - build_attribute<StringAttribute, vespalib::string>(name, BasicType::Type::STRING, col_type, values); + build_attribute<StringAttribute, vespalib::string>(name, BasicType::Type::STRING, col_type, values, uncased); } void @@ -65,7 +70,7 @@ MockAttributeManager::build_float_attribute(const vespalib::string& name, const std::vector<std::vector<double>>& values, CollectionType col_type) { - build_attribute<FloatingPointAttribute, double>(name, BasicType::Type::DOUBLE, col_type, values); + build_attribute<FloatingPointAttribute, double>(name, BasicType::Type::DOUBLE, col_type, values, std::nullopt); } void @@ -73,14 +78,14 @@ MockAttributeManager::build_int_attribute(const vespalib::string& name, BasicTyp const std::vector<std::vector<int64_t>>& values, CollectionType col_type) { - build_attribute<IntegerAttribute, int64_t>(name, type, col_type, values); + build_attribute<IntegerAttribute, int64_t>(name, type, col_type, values, std::nullopt); } void MockAttributeManager::build_raw_attribute(const vespalib::string& name, const std::vector<std::vector<std::vector<char>>>& values) { - build_attribute<SingleRawAttribute, std::vector<char>>(name, BasicType::Type::RAW, CollectionType::SINGLE, values); + build_attribute<SingleRawAttribute, std::vector<char>>(name, BasicType::Type::RAW, CollectionType::SINGLE, values, std::nullopt); } } diff --git a/searchsummary/src/vespa/searchsummary/test/mock_attribute_manager.h b/searchsummary/src/vespa/searchsummary/test/mock_attribute_manager.h index 69762a37f95..ae105025826 100644 --- a/searchsummary/src/vespa/searchsummary/test/mock_attribute_manager.h +++ b/searchsummary/src/vespa/searchsummary/test/mock_attribute_manager.h @@ -2,6 +2,7 @@ #include <vespa/searchcommon/attribute/basictype.h> #include <vespa/searchlib/attribute/attributemanager.h> +#include <optional> namespace search::docsummary::test { @@ -15,7 +16,8 @@ private: template <typename AttributeType, typename ValueType> void build_attribute(const vespalib::string& name, search::attribute::BasicType type, search::attribute::CollectionType col_type, - const std::vector<std::vector<ValueType>>& values); + const std::vector<std::vector<ValueType>>& values, + std::optional<bool> uncased); public: MockAttributeManager(); @@ -24,7 +26,8 @@ public: void build_string_attribute(const vespalib::string& name, const std::vector<std::vector<vespalib::string>>& values, - search::attribute::CollectionType col_type = search::attribute::CollectionType::ARRAY); + search::attribute::CollectionType col_type = search::attribute::CollectionType::ARRAY, + std::optional<bool> uncased = std::nullopt); void build_float_attribute(const vespalib::string& name, const std::vector<std::vector<double>>& values, search::attribute::CollectionType col_type = search::attribute::CollectionType::ARRAY); diff --git a/vespa-feed-client-api/abi-spec.json b/vespa-feed-client-api/abi-spec.json index e8c2b4a3c9e..907d974ff09 100644 --- a/vespa-feed-client-api/abi-spec.json +++ b/vespa-feed-client-api/abi-spec.json @@ -144,6 +144,7 @@ "public static void setFeedClientBuilderSupplier(java.util.function.Supplier)", "public abstract ai.vespa.feed.client.FeedClientBuilder setConnectionsPerEndpoint(int)", "public abstract ai.vespa.feed.client.FeedClientBuilder setMaxStreamPerConnection(int)", + "public abstract ai.vespa.feed.client.FeedClientBuilder setConnectionTimeToLive(java.time.Duration)", "public abstract ai.vespa.feed.client.FeedClientBuilder setSslContext(javax.net.ssl.SSLContext)", "public abstract ai.vespa.feed.client.FeedClientBuilder setHostnameVerifier(javax.net.ssl.HostnameVerifier)", "public abstract ai.vespa.feed.client.FeedClientBuilder setProxyHostnameVerifier(javax.net.ssl.HostnameVerifier)", diff --git a/vespa-feed-client-api/src/main/java/ai/vespa/feed/client/FeedClientBuilder.java b/vespa-feed-client-api/src/main/java/ai/vespa/feed/client/FeedClientBuilder.java index 270ecad6af8..7101b8452ed 100644 --- a/vespa-feed-client-api/src/main/java/ai/vespa/feed/client/FeedClientBuilder.java +++ b/vespa-feed-client-api/src/main/java/ai/vespa/feed/client/FeedClientBuilder.java @@ -7,6 +7,7 @@ import java.net.URI; import java.nio.file.Path; import java.security.PrivateKey; import java.security.cert.X509Certificate; +import java.time.Duration; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -63,6 +64,9 @@ public interface FeedClientBuilder { */ FeedClientBuilder setMaxStreamPerConnection(int max); + /** Sets a duration after which this client will recycle active connections. This is off ({@code Duration.ZERO}) by default. */ + FeedClientBuilder setConnectionTimeToLive(Duration ttl); + /** Sets {@link SSLContext} instance. */ FeedClientBuilder setSslContext(SSLContext context); diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/FeedClientBuilderImpl.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/FeedClientBuilderImpl.java index cacaeac30ae..6b4372fd11a 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/FeedClientBuilderImpl.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/FeedClientBuilderImpl.java @@ -60,6 +60,7 @@ public class FeedClientBuilderImpl implements FeedClientBuilder { boolean speedTest = false; Compression compression = auto; URI proxy; + Duration connectionTtl = Duration.ZERO; public FeedClientBuilderImpl() { @@ -95,6 +96,13 @@ public class FeedClientBuilderImpl implements FeedClientBuilder { return this; } + @Override + public FeedClientBuilder setConnectionTimeToLive(Duration ttl) { + if (ttl.isNegative()) throw new IllegalArgumentException("Connection TTL cannot be negative, but was " + ttl); + this.connectionTtl = ttl; + return this; + } + /** Sets {@link SSLContext} instance. */ @Override public FeedClientBuilderImpl setSslContext(SSLContext context) { diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/JettyCluster.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/JettyCluster.java index 1edcc6d6cba..5454249d52e 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/JettyCluster.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/JettyCluster.java @@ -159,7 +159,10 @@ class JettyCluster implements Cluster { dest, Pool.StrategyType.RANDOM, connectionsPerEndpoint, false, dest, Integer.MAX_VALUE); pool.preCreateConnections(connectionsPerEndpoint); if (secureProxy) pool.setMaxDuration(Duration.ofMinutes(1).toMillis()); - else pool.setMaximizeConnections(true); + else { + pool.setMaximizeConnections(true); + pool.setMaxDuration(b.connectionTtl.toMillis()); + } return pool; }); HttpClient httpClient = new HttpClient(transport); |