diff options
38 files changed, 497 insertions, 1352 deletions
diff --git a/client/go/internal/vespa/target.go b/client/go/internal/vespa/target.go index 5b29990fe4e..8c3f5c9b7c3 100644 --- a/client/go/internal/vespa/target.go +++ b/client/go/internal/vespa/target.go @@ -109,9 +109,7 @@ type LogOptions struct { // Do sends request to this service. Authentication of the request happens automatically. func (s *Service) Do(request *http.Request, timeout time.Duration) (*http.Response, error) { - s.once.Do(func() { - util.ConfigureTLS(s.httpClient, s.TLSOptions.KeyPair, s.TLSOptions.CACertificate, s.TLSOptions.TrustAll) - }) + util.ConfigureTLS(s.httpClient, s.TLSOptions.KeyPair, s.TLSOptions.CACertificate, s.TLSOptions.TrustAll) if s.auth != nil { if err := s.auth.Authenticate(request); err != nil { return nil, fmt.Errorf("%w: %s", errAuth, err) diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index 5d9ae6bc26e..ce73f8300db 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -1471,7 +1471,6 @@ ], "methods" : [ "public void <init>()", - "public com.yahoo.config.model.api.OnnxModelCost$Calculator newCalculator(com.yahoo.config.application.api.ApplicationPackage, com.yahoo.config.application.api.DeployLogger)", "public com.yahoo.config.model.api.OnnxModelCost$Calculator newCalculator(com.yahoo.config.application.api.ApplicationPackage, com.yahoo.config.provision.ApplicationId)", "public long aggregatedModelCostInBytes()", "public void registerModel(com.yahoo.config.application.api.ApplicationFile)", @@ -1488,7 +1487,7 @@ "abstract" ], "methods" : [ - "public abstract com.yahoo.config.model.api.OnnxModelCost$Calculator newCalculator(com.yahoo.config.application.api.ApplicationPackage, com.yahoo.config.application.api.DeployLogger)", + "public com.yahoo.config.model.api.OnnxModelCost$Calculator newCalculator(com.yahoo.config.application.api.ApplicationPackage, com.yahoo.config.application.api.DeployLogger)", "public abstract com.yahoo.config.model.api.OnnxModelCost$Calculator newCalculator(com.yahoo.config.application.api.ApplicationPackage, com.yahoo.config.provision.ApplicationId)", "public static com.yahoo.config.model.api.OnnxModelCost disabled()" ], diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/OnnxModelCost.java b/config-model-api/src/main/java/com/yahoo/config/model/api/OnnxModelCost.java index 02a32852e08..abfddfe40be 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/OnnxModelCost.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/OnnxModelCost.java @@ -14,7 +14,11 @@ import java.net.URI; */ public interface OnnxModelCost { - Calculator newCalculator(ApplicationPackage appPkg, DeployLogger deployLogger); // TODO: Remove when 8.249 is oldest model in use + // TODO: Remove when 8.250 is oldest model in use + default Calculator newCalculator(ApplicationPackage appPkg, DeployLogger deployLogger) { + return newCalculator(appPkg, ApplicationId.defaultId()); + } + Calculator newCalculator(ApplicationPackage appPkg, ApplicationId applicationId); interface Calculator { @@ -26,7 +30,6 @@ public interface OnnxModelCost { static OnnxModelCost disabled() { return new DisabledOnnxModelCost(); } class DisabledOnnxModelCost implements OnnxModelCost, Calculator { - @Override public Calculator newCalculator(ApplicationPackage appPkg, DeployLogger deployLogger) { return this; } @Override public Calculator newCalculator(ApplicationPackage appPkg, ApplicationId applicationId) { return this; } @Override public long aggregatedModelCostInBytes() {return 0;} @Override public void registerModel(ApplicationFile path) {} diff --git a/config-model/src/main/java/com/yahoo/schema/RankProfile.java b/config-model/src/main/java/com/yahoo/schema/RankProfile.java index e2577f4f834..1ff85c9c89f 100644 --- a/config-model/src/main/java/com/yahoo/schema/RankProfile.java +++ b/config-model/src/main/java/com/yahoo/schema/RankProfile.java @@ -111,7 +111,7 @@ public class RankProfile implements Cloneable { private String inheritedSummaryFeaturesProfileName; private Set<ReferenceNode> matchFeatures; - private Set<String> hiddenMatchFeatures; + private Set<ReferenceNode> hiddenMatchFeatures; private String inheritedMatchFeaturesProfileName; private Set<ReferenceNode> rankFeatures; @@ -609,7 +609,7 @@ public class RankProfile implements Cloneable { .orElse(Set.of()); } - public Set<String> getHiddenMatchFeatures() { + public Set<ReferenceNode> getHiddenMatchFeatures() { if (hiddenMatchFeatures != null) return Collections.unmodifiableSet(hiddenMatchFeatures); return uniquelyInherited(p -> p.getHiddenMatchFeatures(), f -> ! f.isEmpty(), "hidden match features") .orElse(Set.of()); @@ -628,15 +628,13 @@ public class RankProfile implements Cloneable { } private void addImplicitMatchFeatures(List<FeatureList> list) { - if (matchFeatures == null) - matchFeatures = new LinkedHashSet<>(); if (hiddenMatchFeatures == null) hiddenMatchFeatures = new LinkedHashSet<>(); + var current = getMatchFeatures(); for (var features : list) { for (ReferenceNode feature : features) { - if (! matchFeatures.contains(feature)) { - matchFeatures.add(feature); - hiddenMatchFeatures.add(feature.toString()); + if (! current.contains(feature)) { + hiddenMatchFeatures.add(feature); } } } 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 eb9f7d44c91..68fa0fe6de9 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 @@ -149,7 +149,7 @@ public class RawRankProfile implements RankProfilesConfig.Producer { private final Map<String, FieldRankSettings> fieldRankSettings = new java.util.LinkedHashMap<>(); private final Set<ReferenceNode> summaryFeatures; private final Set<ReferenceNode> matchFeatures; - private final Collection<String> hiddenMatchFeatures; + private final Set<ReferenceNode> hiddenMatchFeatures; private final Set<ReferenceNode> rankFeatures; private final Map<String, String> featureRenames = new java.util.LinkedHashMap<>(); private final List<RankProfile.RankProperty> rankProperties; @@ -203,6 +203,7 @@ public class RawRankProfile implements RankProfilesConfig.Producer { summaryFeatures = new LinkedHashSet<>(compiled.getSummaryFeatures()); matchFeatures = new LinkedHashSet<>(compiled.getMatchFeatures()); hiddenMatchFeatures = compiled.getHiddenMatchFeatures(); + matchFeatures.addAll(hiddenMatchFeatures); rankFeatures = compiled.getRankFeatures(); rerankCount = compiled.getRerankCount(); globalPhaseRerankCount = compiled.getGlobalPhaseRerankCount(); @@ -429,8 +430,8 @@ public class RawRankProfile implements RankProfilesConfig.Producer { for (ReferenceNode feature : matchFeatures) { properties.add(new Pair<>("vespa.match.feature", feature.toString())); } - for (String feature : hiddenMatchFeatures) { - properties.add(new Pair<>("vespa.hidden.matchfeature", feature)); + for (ReferenceNode feature : hiddenMatchFeatures) { + properties.add(new Pair<>("vespa.hidden.matchfeature", feature.toString())); } for (ReferenceNode feature : rankFeatures) { properties.add(new Pair<>("vespa.dump.feature", feature.toString())); diff --git a/config-model/src/main/java/com/yahoo/schema/processing/AdjustSummaryTransforms.java b/config-model/src/main/java/com/yahoo/schema/processing/AdjustSummaryTransforms.java new file mode 100644 index 00000000000..dd6f118d113 --- /dev/null +++ b/config-model/src/main/java/com/yahoo/schema/processing/AdjustSummaryTransforms.java @@ -0,0 +1,82 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.schema.processing; + +import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.schema.RankProfileRegistry; +import com.yahoo.schema.Schema; +import com.yahoo.schema.derived.SummaryClass; +import com.yahoo.schema.document.Attribute; +import com.yahoo.schema.document.ImmutableSDField; +import com.yahoo.vespa.documentmodel.SummaryField; +import com.yahoo.vespa.documentmodel.SummaryTransform; +import com.yahoo.vespa.model.container.search.QueryProfiles; + +import static com.yahoo.schema.document.ComplexAttributeFieldUtils.isComplexFieldWithOnlyStructFieldAttributes; + +/** + * Adds the corresponding summary transform for all "documentid" summary fields. + * For summary fields without an existing transform: + * - Adds the attribute transforms where the source field has an attribute vector. + * - Adds the attribute combiner transform where the source field is a struct field where all subfields have attribute + * vector. + * - Add the copy transform where the source field is a struct or map field with a different name. + * + * @author geirst + */ +public class AdjustSummaryTransforms extends Processor { + + public AdjustSummaryTransforms(Schema schema, DeployLogger deployLogger, RankProfileRegistry rankProfileRegistry, QueryProfiles queryProfiles) { + super(schema, deployLogger, rankProfileRegistry, queryProfiles); + } + + @Override + public void process(boolean validate, boolean documentsOnly) { + for (var summary : schema.getSummaries().values()) { + for (var summaryField : summary.getSummaryFields().values()) { + makeDocumentIdTransformIfAppropriate(summaryField); + makeAttributeTransformIfAppropriate(summaryField, schema); + makeAttributeCombinerTransformIfAppropriate(summaryField, schema); + makeCopyTransformIfAppropriate(summaryField, schema); + } + } + } + + private void makeDocumentIdTransformIfAppropriate(SummaryField summaryField) + { + if (summaryField.getName().equals(SummaryClass.DOCUMENT_ID_FIELD)) { + summaryField.setTransform(SummaryTransform.DOCUMENT_ID); + } + } + + /** If the source is an attribute, make this use the attribute transform */ + private void makeAttributeTransformIfAppropriate(SummaryField summaryField, Schema schema) { + if (summaryField.getTransform() != SummaryTransform.NONE) return; + Attribute attribute = schema.getAttribute(summaryField.getSingleSource()); + if (attribute == null) return; + summaryField.setTransform(SummaryTransform.ATTRIBUTE); + } + + /** If the source is a complex field with only struct field attributes then make this use the attribute combiner transform */ + private void makeAttributeCombinerTransformIfAppropriate(SummaryField summaryField, Schema schema) { + if (summaryField.getTransform() == SummaryTransform.NONE) { + String sourceFieldName = summaryField.getSingleSource(); + ImmutableSDField source = schema.getField(sourceFieldName); + if (source != null && isComplexFieldWithOnlyStructFieldAttributes(source)) { + summaryField.setTransform(SummaryTransform.ATTRIBUTECOMBINER); + } + } + } + + /* + * This function must be called after makeAttributeCombinerTransformIfAppropriate(). + */ + private void makeCopyTransformIfAppropriate(SummaryField summaryField, Schema schema) { + if (summaryField.getTransform() == SummaryTransform.NONE) { + String sourceFieldName = summaryField.getSingleSource(); + ImmutableSDField source = schema.getField(sourceFieldName); + if (source != null && source.usesStructOrMap() && summaryField.hasExplicitSingleSource()) { + summaryField.setTransform(SummaryTransform.COPY); + } + } + } +} diff --git a/config-model/src/main/java/com/yahoo/schema/processing/Processing.java b/config-model/src/main/java/com/yahoo/schema/processing/Processing.java index d9b90ad661d..2d4b4824310 100644 --- a/config-model/src/main/java/com/yahoo/schema/processing/Processing.java +++ b/config-model/src/main/java/com/yahoo/schema/processing/Processing.java @@ -53,8 +53,8 @@ public class Processing { ImplicitSummaries::new, ImplicitSummaryFields::new, AdjustPositionSummaryFields::new, - SummaryTransformForDocumentId::new, SummaryConsistency::new, + AdjustSummaryTransforms::new, SummaryNamesFieldCollisions::new, SummaryFieldsMustHaveValidSource::new, MatchedElementsOnlyResolver::new, diff --git a/config-model/src/main/java/com/yahoo/schema/processing/ReservedFunctionNames.java b/config-model/src/main/java/com/yahoo/schema/processing/ReservedFunctionNames.java index f4d2faf9444..0987521831b 100644 --- a/config-model/src/main/java/com/yahoo/schema/processing/ReservedFunctionNames.java +++ b/config-model/src/main/java/com/yahoo/schema/processing/ReservedFunctionNames.java @@ -9,6 +9,7 @@ import com.yahoo.searchlib.rankingexpression.parser.RankingExpressionParserConst import com.yahoo.vespa.model.container.search.QueryProfiles; import java.util.Arrays; +import java.util.HashSet; import java.util.Set; import java.util.logging.Level; import java.util.stream.Collectors; @@ -46,8 +47,28 @@ public class ReservedFunctionNames extends Processor { } private static Set<String> getReservedNames() { - return Arrays.stream(RankingExpressionParserConstants.tokenImage) - .map(token -> token.substring(1, token.length()-1)).collect(Collectors.toUnmodifiableSet()); + Set<String> temp = new HashSet<>(); + Arrays.stream(RankingExpressionParserConstants.tokenImage) + .map(token -> token.substring(1, token.length()-1)).forEach(name -> temp.add(name)); + temp.add("attribute"); + temp.add("constant"); + temp.add("customTokenInputIds"); + temp.add("firstphase"); + temp.add("globalphase"); + temp.add("normalize_linear"); + temp.add("onnx"); + temp.add("onnx_vespa"); + temp.add("query"); + temp.add("reciprocal_rank"); + temp.add("reciprocal_rank_fusion"); + temp.add("secondphase"); + temp.add("tensor"); + temp.add("tokenAttentionMask"); + temp.add("tokenInputIds"); + temp.add("tokenTypeIds"); + temp.add("value"); + temp.add("xgboost"); + return Set.copyOf(temp); } } diff --git a/config-model/src/main/java/com/yahoo/schema/processing/SummaryConsistency.java b/config-model/src/main/java/com/yahoo/schema/processing/SummaryConsistency.java index a7ff50ffcca..4b214e00d65 100644 --- a/config-model/src/main/java/com/yahoo/schema/processing/SummaryConsistency.java +++ b/config-model/src/main/java/com/yahoo/schema/processing/SummaryConsistency.java @@ -8,14 +8,11 @@ import com.yahoo.schema.RankProfileRegistry; import com.yahoo.schema.Schema; import com.yahoo.schema.document.Attribute; import com.yahoo.document.WeightedSetDataType; -import com.yahoo.schema.document.ImmutableSDField; import com.yahoo.vespa.documentmodel.DocumentSummary; import com.yahoo.vespa.documentmodel.SummaryField; import com.yahoo.vespa.documentmodel.SummaryTransform; import com.yahoo.vespa.model.container.search.QueryProfiles; -import static com.yahoo.schema.document.ComplexAttributeFieldUtils.isComplexFieldWithOnlyStructFieldAttributes; - /** * Ensure that summary field transforms for fields having the same name * are consistent across summary classes @@ -35,9 +32,6 @@ public class SummaryConsistency extends Processor { for (SummaryField summaryField : summary.getSummaryFields().values()) { assertConsistency(summaryField, schema, validate); - makeAttributeTransformIfAppropriate(summaryField, schema); - makeAttributeCombinerTransformIfAppropriate(summaryField, schema); - makeCopyTransformIfAppropriate(summaryField, schema); } } } @@ -60,38 +54,6 @@ public class SummaryConsistency extends Processor { } } - /** If the source is an attribute, make this use the attribute transform */ - private void makeAttributeTransformIfAppropriate(SummaryField summaryField, Schema schema) { - if (summaryField.getTransform() != SummaryTransform.NONE) return; - Attribute attribute = schema.getAttribute(summaryField.getSingleSource()); - if (attribute == null) return; - summaryField.setTransform(SummaryTransform.ATTRIBUTE); - } - - /** If the source is a complex field with only struct field attributes then make this use the attribute combiner transform */ - private void makeAttributeCombinerTransformIfAppropriate(SummaryField summaryField, Schema schema) { - if (summaryField.getTransform() == SummaryTransform.NONE) { - String sourceFieldName = summaryField.getSingleSource(); - ImmutableSDField source = schema.getField(sourceFieldName); - if (source != null && isComplexFieldWithOnlyStructFieldAttributes(source)) { - summaryField.setTransform(SummaryTransform.ATTRIBUTECOMBINER); - } - } - } - - /* - * This function must be called after makeAttributeCombinerTransformIfAppropriate(). - */ - private void makeCopyTransformIfAppropriate(SummaryField summaryField, Schema schema) { - if (summaryField.getTransform() == SummaryTransform.NONE) { - String sourceFieldName = summaryField.getSingleSource(); - ImmutableSDField source = schema.getField(sourceFieldName); - if (source != null && source.usesStructOrMap() && summaryField.hasExplicitSingleSource()) { - summaryField.setTransform(SummaryTransform.COPY); - } - } - } - private void assertConsistentTypes(SummaryField existing, SummaryField seen) { if (existing.getDataType() instanceof WeightedSetDataType && seen.getDataType() instanceof WeightedSetDataType && ((WeightedSetDataType)existing.getDataType()).getNestedType().equals(((WeightedSetDataType)seen.getDataType()).getNestedType())) diff --git a/config-model/src/main/java/com/yahoo/schema/processing/SummaryTransformForDocumentId.java b/config-model/src/main/java/com/yahoo/schema/processing/SummaryTransformForDocumentId.java deleted file mode 100644 index 388aa93e81c..00000000000 --- a/config-model/src/main/java/com/yahoo/schema/processing/SummaryTransformForDocumentId.java +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.schema.processing; - -import com.yahoo.config.application.api.DeployLogger; -import com.yahoo.schema.RankProfileRegistry; -import com.yahoo.schema.Schema; -import com.yahoo.schema.derived.SummaryClass; -import com.yahoo.vespa.documentmodel.SummaryTransform; -import com.yahoo.vespa.model.container.search.QueryProfiles; - -/** - * Adds the corresponding summary transform for all "documentid" summary fields. - * - * @author geirst - */ -public class SummaryTransformForDocumentId extends Processor { - - public SummaryTransformForDocumentId(Schema schema, DeployLogger deployLogger, RankProfileRegistry rankProfileRegistry, QueryProfiles queryProfiles) { - super(schema, deployLogger, rankProfileRegistry, queryProfiles); - } - - @Override - public void process(boolean validate, boolean documentsOnly) { - for (var summary : schema.getSummaries().values()) { - for (var summaryField : summary.getSummaryFields().values()) { - if (summaryField.getName().equals(SummaryClass.DOCUMENT_ID_FIELD)) { - summaryField.setTransform(SummaryTransform.DOCUMENT_ID); - } - } - } - } -} diff --git a/config-model/src/test/derived/globalphase_token_functions/rank-profiles.cfg b/config-model/src/test/derived/globalphase_token_functions/rank-profiles.cfg index 37d84c1a2d9..d0336e31744 100644 --- a/config-model/src/test/derived/globalphase_token_functions/rank-profiles.cfg +++ b/config-model/src/test/derived/globalphase_token_functions/rank-profiles.cfg @@ -49,3 +49,46 @@ rankprofile[].fef.property[].name "vespa.type.attribute.tokens" rankprofile[].fef.property[].value "tensor(d0[128])" rankprofile[].fef.property[].name "vespa.type.query.input" rankprofile[].fef.property[].value "tensor(d0[32])" +rankprofile[].name "with-fun" +rankprofile[].fef.property[].name "rankingExpression(use_model).rankingScript" +rankprofile[].fef.property[].value "attribute(outputidx) + 1.0" +rankprofile[].fef.property[].name "vespa.rank.globalphase" +rankprofile[].fef.property[].value "rankingExpression(use_model)" +rankprofile[].fef.property[].name "vespa.match.feature" +rankprofile[].fef.property[].value "attribute(outputidx)" +rankprofile[].fef.property[].name "vespa.hidden.matchfeature" +rankprofile[].fef.property[].value "attribute(outputidx)" +rankprofile[].fef.property[].name "vespa.type.attribute.tokens" +rankprofile[].fef.property[].value "tensor(d0[128])" +rankprofile[].name "with-fun-mf" +rankprofile[].fef.property[].name "rankingExpression(use_model).rankingScript" +rankprofile[].fef.property[].value "attribute(outputidx) + 1.0" +rankprofile[].fef.property[].name "vespa.rank.firstphase" +rankprofile[].fef.property[].value "nativeRank" +rankprofile[].fef.property[].name "vespa.rank.globalphase" +rankprofile[].fef.property[].value "rankingExpression(use_model)" +rankprofile[].fef.property[].name "vespa.match.feature" +rankprofile[].fef.property[].value "rankingExpression(use_model)" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "rankingExpression(use_model)" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "use_model" +rankprofile[].fef.property[].name "vespa.type.attribute.tokens" +rankprofile[].fef.property[].value "tensor(d0[128])" +rankprofile[].name "fun-mf-child" +rankprofile[].fef.property[].name "rankingExpression(use_model).rankingScript" +rankprofile[].fef.property[].value "attribute(outputidx) + 1.0" +rankprofile[].fef.property[].name "vespa.rank.firstphase" +rankprofile[].fef.property[].value "rankingExpression(firstphase)" +rankprofile[].fef.property[].name "rankingExpression(firstphase).rankingScript" +rankprofile[].fef.property[].value "42 * attribute(outputidx)" +rankprofile[].fef.property[].name "vespa.rank.globalphase" +rankprofile[].fef.property[].value "rankingExpression(use_model)" +rankprofile[].fef.property[].name "vespa.match.feature" +rankprofile[].fef.property[].value "rankingExpression(use_model)" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "rankingExpression(use_model)" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "use_model" +rankprofile[].fef.property[].name "vespa.type.attribute.tokens" +rankprofile[].fef.property[].value "tensor(d0[128])" diff --git a/config-model/src/test/derived/globalphase_token_functions/test.sd b/config-model/src/test/derived/globalphase_token_functions/test.sd index 5e849772249..511e09948b4 100644 --- a/config-model/src/test/derived/globalphase_token_functions/test.sd +++ b/config-model/src/test/derived/globalphase_token_functions/test.sd @@ -42,4 +42,23 @@ schema test { } } + rank-profile with-fun { + function use_model() { + expression: attribute(outputidx) + 1.0 + } + global-phase { + expression: use_model + } + } + rank-profile with-fun-mf inherits with-fun { + first-phase { + expression: nativeRank + } + match-features: use_model + } + rank-profile fun-mf-child inherits with-fun-mf { + first-phase { + expression: 42 * attribute(outputidx) + } + } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidatorTest.java index fd8e93d73ed..8531aff3b1a 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidatorTest.java @@ -120,9 +120,6 @@ class JvmHeapSizeValidatorTest { ModelCostDummy(long modelCost) { this.modelCost = modelCost; } - @Override - public Calculator newCalculator(ApplicationPackage appPkg, DeployLogger deployLogger) { return this; } - @Override public Calculator newCalculator(ApplicationPackage appPkg, ApplicationId applicationId) { return this; } @Override public long aggregatedModelCostInBytes() { return totalCost.get(); } @Override public void registerModel(ApplicationFile path) {} diff --git a/container-core/src/main/java/com/yahoo/restapi/SlimeJsonResponse.java b/container-core/src/main/java/com/yahoo/restapi/SlimeJsonResponse.java index 252fc99a273..d6720a8797e 100644 --- a/container-core/src/main/java/com/yahoo/restapi/SlimeJsonResponse.java +++ b/container-core/src/main/java/com/yahoo/restapi/SlimeJsonResponse.java @@ -16,24 +16,27 @@ import java.io.OutputStream; public class SlimeJsonResponse extends HttpResponse { protected final Slime slime; + private final boolean compact; public SlimeJsonResponse() { this(new Slime()); } - public SlimeJsonResponse(Slime slime) { - super(200); - this.slime = slime; - } + public SlimeJsonResponse(Slime slime) { this(200, slime, true); } + + public SlimeJsonResponse(Slime slime, boolean compact) { this(200, slime, compact); } + + public SlimeJsonResponse(int statusCode, Slime slime) { this(statusCode, slime, true); } - public SlimeJsonResponse(int statusCode, Slime slime) { + public SlimeJsonResponse(int statusCode, Slime slime, boolean compact) { super(statusCode); this.slime = slime; + this.compact = compact; } @Override public void render(OutputStream stream) throws IOException { - new JsonFormat(true).encode(stream, slime); + new JsonFormat(compact).encode(stream, slime); } @Override 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 new file mode 100644 index 00000000000..c665b4fb7c2 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/AcceptedCountries.java @@ -0,0 +1,23 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +package com.yahoo.vespa.hosted.controller.api.integration.billing; + +import java.util.List; + +/** + * @author bjorncs + */ +public record AcceptedCountries(List<Country> countries) { + + public AcceptedCountries { + countries = List.copyOf(countries); + } + + public record Country(String code, String displayName, List<TaxType> taxTypes) { + public Country { + taxTypes = List.copyOf(taxTypes); + } + } + + public record TaxType(String id, String description, String pattern, String example) {} +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingController.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingController.java index 0aae466feb2..e3b303c38d8 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingController.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingController.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.controller.api.integration.billing; import com.yahoo.config.provision.TenantName; -import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; import java.math.BigDecimal; import java.time.LocalDate; @@ -130,4 +129,7 @@ public interface BillingController { default void updateCache(List<TenantName> tenants) {} + /** Get the list of countries that are accepted */ + AcceptedCountries getAcceptedCountries(); + } 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 f8e83b44510..af33992a093 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 @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.controller.api.integration.billing; import com.yahoo.config.provision.TenantName; -import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; import java.math.BigDecimal; import java.time.Clock; @@ -206,6 +205,19 @@ public class MockBillingController implements BillingController { return count < limit; } + @Override + public AcceptedCountries getAcceptedCountries() { + return new AcceptedCountries(List.of( + new AcceptedCountries.Country( + "NO", "Norway", + List.of(new AcceptedCountries.TaxType("no_vat", "Norwegian VAT number", "[0-9]{9}MVA", "123456789MVA"))), + new AcceptedCountries.Country( + "CA", "Canada", + 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"))) + )); + } + public void setTenants(List<TenantName> tenants) { this.tenants = tenants; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java index 52900f83203..54f53d64f76 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java @@ -72,25 +72,11 @@ enum PathGroup { "/application/v4/tenant/{tenant}/archive-access/aws", "/application/v4/tenant/{tenant}/archive-access/gcp"), - - billingToken(Matcher.tenant, - "/billing/v1/tenant/{tenant}/token"), - - billingInstrument(Matcher.tenant, - "/billing/v1/tenant/{tenant}/instrument/{*}"), - - billingPlan(Matcher.tenant, - "/billing/v1/tenant/{tenant}/plan/{*}"), - - billingCollection(Matcher.tenant, - "/billing/v1/tenant/{tenant}/collection/{*}"), - - billingList(Matcher.tenant, - "/billing/v1/tenant/{tenant}/billing/{*}"), - billing(Matcher.tenant, "/billing/v2/tenant/{tenant}/{*}"), + billingAux("/billing/v2/countries"), + accountant("/billing/v2/accountant/{*}"), userSearch("/user/v1/find"), @@ -247,11 +233,6 @@ enum PathGroup { /** Paths used for receiving payment callbacks */ paymentProcessor("/payment/notification"), - /** Paths used for invoice management */ - hostedAccountant("/billing/v1/invoice/{*}", - "/billing/v1/billing", - "/billing/v1/plans"), - /** Path used for listing endpoint certificate request and re-requesting endpoint certificates */ endpointCertificates("/endpointcertificates/"), @@ -322,20 +303,12 @@ enum PathGroup { static Set<PathGroup> operatorRestrictedPaths() { var paths = billingPathsNoToken(); - paths.add(PathGroup.billingToken); paths.add(accessRequestApproval); return paths; } static Set<PathGroup> billingPathsNoToken() { - return EnumSet.of( - PathGroup.billingCollection, - PathGroup.billingInstrument, - PathGroup.billingList, - PathGroup.billingPlan, - PathGroup.billing, - PathGroup.hostedAccountant - ); + return EnumSet.of(PathGroup.billing, PathGroup.billingAux); } /** Returns whether this group matches path in given context */ diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java index 6b5130cf2e5..d1a8b2ef0c3 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java @@ -26,10 +26,7 @@ enum Policy { .in(SystemName.all()), Privilege.grant(Action.read) .on(PathGroup.billingPathsNoToken()) - .in(SystemName.all()), - Privilege.grant(Action.read) - .on(PathGroup.billingToken) - .in(SystemName.PublicCd)), + .in(SystemName.all())), /** Full access to everything. */ supporter(Privilege.grant(Action.read) @@ -155,40 +152,14 @@ enum Policy { .on(PathGroup.paymentProcessor) .in(SystemName.PublicCd)), - /** Read your own instrument information */ - paymentInstrumentRead(Privilege.grant(Action.read) - .on(PathGroup.billingInstrument) - .in(SystemName.PublicCd, SystemName.Public)), - - /** Ability to update tenant payment instrument */ - paymentInstrumentUpdate(Privilege.grant(Action.update) - .on(PathGroup.billingInstrument) - .in(SystemName.PublicCd, SystemName.Public)), - - /** Ability to remove your own payment instrument */ - paymentInstrumentDelete(Privilege.grant(Action.delete) - .on(PathGroup.billingInstrument) - .in(SystemName.PublicCd, SystemName.Public)), - - /** Get the token to view instrument form */ - paymentInstrumentCreate(Privilege.grant(Action.read) - .on(PathGroup.billingToken) - .in(SystemName.PublicCd, SystemName.Public)), - /** Ability to update tenant payment instrument */ planUpdate(Privilege.grant(Action.update) - .on(PathGroup.billingPlan, PathGroup.billing) - .in(SystemName.PublicCd, SystemName.Public)), - - /** Ability to update tenant collection method */ - collectionMethodUpdate(Privilege.grant(Action.update) - .on(PathGroup.billingCollection) + .on(PathGroup.billing) .in(SystemName.PublicCd, SystemName.Public)), - /** Read the generated bills */ billingInformationRead(Privilege.grant(Action.read) - .on(PathGroup.billingList, PathGroup.billing) + .on(PathGroup.billing, PathGroup.billingAux) .in(SystemName.PublicCd, SystemName.Public)), accessRequests(Privilege.grant(Action.all()) @@ -197,7 +168,7 @@ enum Policy { /** Invoice management */ hostedAccountant(Privilege.grant(Action.all()) - .on(PathGroup.hostedAccountant, PathGroup.accountant, PathGroup.userSearch) + .on(PathGroup.accountant, PathGroup.userSearch) .in(SystemName.PublicCd, SystemName.Public)), /** Listing endpoint certificates and re-requesting certificates */ diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java index d57e38df239..31c8560c908 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java @@ -43,8 +43,6 @@ public enum RoleDefinition { Policy.applicationRead, Policy.deploymentRead, Policy.publicRead, - Policy.paymentInstrumentRead, - Policy.paymentInstrumentDelete, Policy.billingInformationRead, Policy.horizonProxyOperations), @@ -56,8 +54,6 @@ public enum RoleDefinition { Policy.developmentDeployment, Policy.keyManagement, Policy.submission, - Policy.paymentInstrumentRead, - Policy.paymentInstrumentDelete, Policy.billingInformationRead, Policy.secretStoreOperations, Policy.dataplaneToken), @@ -72,7 +68,6 @@ public enum RoleDefinition { Policy.tenantArchiveAccessManagement, Policy.applicationManager, Policy.keyRevokal, - Policy.paymentInstrumentRead, Policy.billingInformationRead, Policy.accessRequests ), @@ -99,7 +94,6 @@ public enum RoleDefinition { paymentProcessor(Policy.paymentProcessor), hostedAccountant(Policy.hostedAccountant, - Policy.collectionMethodUpdate, Policy.planUpdate, Policy.tenantUpdate); diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java index 24539d7c158..c8020666906 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java @@ -8,7 +8,6 @@ import org.junit.jupiter.api.Test; import java.net.URI; import java.util.List; -import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -146,139 +145,6 @@ public class RoleTest { } } - @Test - void payment_instrument() { - URI paymentInstrumentUri = URI.create("/billing/v1/tenant/t1/instrument/foobar"); - URI tenantPaymentInstrumentUri = URI.create("/billing/v1/tenant/t1/instrument"); - URI tokenUri = URI.create("/billing/v1/tenant/t1/token"); - - Role user = Role.reader(TenantName.from("t1")); - assertTrue(publicCdEnforcer.allows(user, Action.read, paymentInstrumentUri)); - assertTrue(publicCdEnforcer.allows(user, Action.delete, paymentInstrumentUri)); - assertFalse(publicCdEnforcer.allows(user, Action.update, tenantPaymentInstrumentUri)); - assertFalse(publicCdEnforcer.allows(user, Action.read, tokenUri)); - - Role developer = Role.developer(TenantName.from("t1")); - assertTrue(publicCdEnforcer.allows(developer, Action.read, paymentInstrumentUri)); - assertTrue(publicCdEnforcer.allows(developer, Action.delete, paymentInstrumentUri)); - assertFalse(publicCdEnforcer.allows(developer, Action.update, tenantPaymentInstrumentUri)); - assertFalse(publicCdEnforcer.allows(developer, Action.read, tokenUri)); - - Role admin = Role.administrator(TenantName.from("t1")); - assertTrue(publicCdEnforcer.allows(admin, Action.read, paymentInstrumentUri)); - assertFalse(publicCdEnforcer.allows(admin, Action.delete, paymentInstrumentUri)); - assertFalse(publicCdEnforcer.allows(admin, Action.update, tenantPaymentInstrumentUri)); - assertFalse(publicCdEnforcer.allows(admin, Action.read, tokenUri)); - } - - @Test - void billing_tenant() { - URI billing = URI.create("/billing/v1/tenant/t1/billing"); - - Role user = Role.reader(TenantName.from("t1")); - Role developer = Role.developer(TenantName.from("t1")); - Role admin = Role.administrator(TenantName.from("t1")); - - Stream.of(user, developer, admin).forEach(role -> { - assertTrue(publicCdEnforcer.allows(role, Action.read, billing)); - assertFalse(publicCdEnforcer.allows(role, Action.update, billing)); - assertFalse(publicCdEnforcer.allows(role, Action.delete, billing)); - assertFalse(publicCdEnforcer.allows(role, Action.create, billing)); - }); - - } - - @Test - void billing_test() { - var tester = new EnforcerTester(publicEnforcer); - - var accountant = Role.hostedAccountant(); - var operator = Role.hostedOperator(); - var reader = Role.reader(TenantName.from("t1")); - var developer = Role.developer(TenantName.from("t1")); - var admin = Role.administrator(TenantName.from("t1")); - var otherAdmin = Role.administrator(TenantName.from("t2")); - - tester.on("/billing/v1/tenant/t1/token") - .assertAction(accountant) - .assertAction(operator) - .assertAction(reader) - .assertAction(developer) - .assertAction(otherAdmin); - - tester.on("/billing/v1/tenant/t1/instrument") - .assertAction(accountant) - .assertAction(operator, Action.read) - .assertAction(reader, Action.read, Action.delete) - .assertAction(developer, Action.read, Action.delete) - .assertAction(admin, Action.read) - .assertAction(otherAdmin); - - tester.on("/billing/v1/tenant/t1/instrument/i1") - .assertAction(accountant) - .assertAction(operator, Action.read) - .assertAction(reader, Action.read, Action.delete) - .assertAction(developer, Action.read, Action.delete) - .assertAction(admin, Action.read) - .assertAction(otherAdmin); - - tester.on("/billing/v1/tenant/t1/billing") - .assertAction(accountant) - .assertAction(operator, Action.read) - .assertAction(reader, Action.read) - .assertAction(developer, Action.read) - .assertAction(admin, Action.read) - .assertAction(otherAdmin); - - tester.on("/billing/v1/tenant/t1/plan") - .assertAction(accountant, Action.update) - .assertAction(operator, Action.read) - .assertAction(reader) - .assertAction(developer) - .assertAction(admin) - .assertAction(otherAdmin); - - tester.on("/billing/v1/tenant/t1/collection") - .assertAction(accountant, Action.update) - .assertAction(operator, Action.read) - .assertAction(reader) - .assertAction(developer) - .assertAction(admin) - .assertAction(otherAdmin); - - tester.on("/billing/v1/billing") - .assertAction(accountant, Action.create, Action.read, Action.update, Action.delete) - .assertAction(operator, Action.read) - .assertAction(reader) - .assertAction(developer) - .assertAction(admin) - .assertAction(otherAdmin); - - tester.on("/billing/v1/invoice/tenant/t1/line-item") - .assertAction(accountant, Action.create, Action.read, Action.update, Action.delete) - .assertAction(operator, Action.read) - .assertAction(reader) - .assertAction(developer) - .assertAction(admin) - .assertAction(otherAdmin); - - tester.on("/billing/v1/invoice") - .assertAction(accountant, Action.create, Action.read, Action.update, Action.delete) - .assertAction(operator, Action.read) - .assertAction(reader) - .assertAction(developer) - .assertAction(admin) - .assertAction(otherAdmin); - - tester.on("/billing/v1/invoice/i1/status") - .assertAction(accountant, Action.create, Action.read, Action.update, Action.delete) - .assertAction(operator, Action.read) - .assertAction(reader) - .assertAction(developer) - .assertAction(admin) - .assertAction(otherAdmin); - } - private static class EnforcerTester { private final Enforcer enforcer; private final URI resource; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandler.java deleted file mode 100644 index 6dc29ebe08c..00000000000 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandler.java +++ /dev/null @@ -1,517 +0,0 @@ -// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.restapi.billing; - -import com.yahoo.config.provision.TenantName; -import com.yahoo.container.jdisc.HttpRequest; -import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.container.jdisc.ThreadedHttpRequestHandler; -import com.yahoo.restapi.ErrorResponse; -import com.yahoo.restapi.JacksonJsonResponse; -import com.yahoo.restapi.MessageResponse; -import com.yahoo.restapi.Path; -import com.yahoo.restapi.SlimeJsonResponse; -import com.yahoo.restapi.StringResponse; -import com.yahoo.slime.Cursor; -import com.yahoo.slime.Inspector; -import com.yahoo.slime.Slime; -import com.yahoo.slime.SlimeUtils; -import com.yahoo.vespa.hosted.controller.ApplicationController; -import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.TenantController; -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.CollectionMethod; -import com.yahoo.vespa.hosted.controller.api.integration.billing.InstrumentOwner; -import com.yahoo.vespa.hosted.controller.api.integration.billing.PaymentInstrument; -import com.yahoo.vespa.hosted.controller.api.integration.billing.PlanId; -import com.yahoo.vespa.hosted.controller.api.integration.billing.PlanRegistry; -import com.yahoo.vespa.hosted.controller.api.integration.billing.StatusHistory; -import com.yahoo.vespa.hosted.controller.api.role.Role; -import com.yahoo.vespa.hosted.controller.api.role.SecurityContext; -import com.yahoo.vespa.hosted.controller.restapi.ErrorResponses; -import com.yahoo.vespa.hosted.controller.tenant.Tenant; -import com.yahoo.yolean.Exceptions; - -import java.io.IOException; -import java.math.BigDecimal; -import java.security.Principal; -import java.time.LocalDate; -import java.time.format.DateTimeFormatter; -import java.time.format.DateTimeParseException; -import java.util.Comparator; -import java.util.List; -import java.util.Optional; -import java.util.Set; -import java.util.concurrent.Executor; - -/** - * @author andreer - * @author olaa - */ -public class BillingApiHandler extends ThreadedHttpRequestHandler { - - private static final DateTimeFormatter DATE_TIME_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd"); - - private final BillingController billingController; - private final ApplicationController applicationController; - private final TenantController tenantController; - private final PlanRegistry planRegistry; - - public BillingApiHandler(Executor executor, - Controller controller) { - super(executor); - this.billingController = controller.serviceRegistry().billingController(); - this.planRegistry = controller.serviceRegistry().planRegistry(); - this.applicationController = controller.applications(); - this.tenantController = controller.tenants(); - } - - @Override - public HttpResponse handle(HttpRequest request) { - try { - Optional<String> userId = Optional.ofNullable(request.getJDiscRequest().getUserPrincipal()).map(Principal::getName); - if (userId.isEmpty()) - return ErrorResponse.unauthorized("Must be authenticated to use this API"); - - Path path = new Path(request.getUri()); - return switch (request.getMethod()) { - case GET -> handleGET(request, path, userId.get()); - case PATCH -> handlePATCH(request, path, userId.get()); - case DELETE -> handleDELETE(path, userId.get()); - case POST -> handlePOST(path, request, userId.get()); - default -> ErrorResponse.methodNotAllowed("Method '" + request.getMethod() + "' is not supported"); - }; - } - catch (IllegalArgumentException e) { - return ErrorResponse.badRequest(Exceptions.toMessageString(e)); - } catch (Exception e) { - return ErrorResponses.logThrowing(request, log, e); - } - } - - private HttpResponse handleGET(HttpRequest request, Path path, String userId) { - if (path.matches("/billing/v1/tenant/{tenant}/token")) return getToken(path.get("tenant"), userId); - if (path.matches("/billing/v1/tenant/{tenant}/instrument")) return getInstruments(path.get("tenant"), userId); - if (path.matches("/billing/v1/tenant/{tenant}/billing")) return getBilling(path.get("tenant"), request.getProperty("until")); - if (path.matches("/billing/v1/tenant/{tenant}/plan")) return getPlan(path.get("tenant")); - if (path.matches("/billing/v1/billing")) return getBillingAllTenants(request.getProperty("until")); - if (path.matches("/billing/v1/invoice/export")) return getAllBills(); - if (path.matches("/billing/v1/invoice/tenant/{tenant}/line-item")) return getLineItems(path.get("tenant")); - if (path.matches("/billing/v1/plans")) return getPlans(); - return ErrorResponse.notFoundError("Nothing at " + path); - } - - private HttpResponse getAllBills() { - var bills = billingController.getBills(); - var headers = new String[]{ "ID", "Tenant", "From", "To", "CpuHours", "MemoryHours", "DiskHours", "Cpu", "Memory", "Disk", "Additional" }; - var rows = bills.stream() - .map(bill -> { - return new Object[] { - bill.id().value(), bill.tenant().value(), - bill.getStartDate().format(DateTimeFormatter.ISO_LOCAL_DATE), - bill.getEndDate().format(DateTimeFormatter.ISO_LOCAL_DATE), - bill.sumCpuHours(), bill.sumMemoryHours(), bill.sumDiskHours(), - bill.sumCpuCost(), bill.sumMemoryCost(), bill.sumDiskCost(), - bill.sumAdditionalCost() - }; - }) - .toList(); - return new CsvResponse(headers, rows); - } - - private HttpResponse handlePATCH(HttpRequest request, Path path, String userId) { - if (path.matches("/billing/v1/tenant/{tenant}/instrument")) return patchActiveInstrument(request, path.get("tenant"), userId); - if (path.matches("/billing/v1/tenant/{tenant}/plan")) return patchPlan(request, path.get("tenant")); - if (path.matches("/billing/v1/tenant/{tenant}/collection")) return patchCollectionMethod(request, path.get("tenant")); - return ErrorResponse.notFoundError("Nothing at " + path); - - } - - private HttpResponse handleDELETE(Path path, String userId) { - if (path.matches("/billing/v1/tenant/{tenant}/instrument/{instrument}")) return deleteInstrument(path.get("tenant"), userId, path.get("instrument")); - if (path.matches("/billing/v1/invoice/line-item/{line-item-id}")) return deleteLineItem(path.get("line-item-id")); - return ErrorResponse.notFoundError("Nothing at " + path); - - } - - private HttpResponse handlePOST(Path path, HttpRequest request, String userId) { - if (path.matches("/billing/v1/invoice")) return createBill(request, userId); - if (path.matches("/billing/v1/invoice/{invoice-id}/status")) return setBillStatus(request, path.get("invoice-id"), userId); - if (path.matches("/billing/v1/invoice/tenant/{tenant}/line-item")) return addLineItem(request, path.get("tenant"), userId); - return ErrorResponse.notFoundError("Nothing at " + path); - - } - - private HttpResponse getPlan(String tenant) { - var plan = billingController.getPlan(TenantName.from(tenant)); - var slime = new Slime(); - var root = slime.setObject(); - root.setString("tenant", tenant); - root.setString("plan", plan.value()); - return new SlimeJsonResponse(slime); - } - - private HttpResponse patchPlan(HttpRequest request, String tenant) { - var tenantName = TenantName.from(tenant); - var slime = inspectorOrThrow(request); - var planId = PlanId.from(slime.field("plan").asString()); - var roles = requestRoles(request); - var isAccountant = roles.contains(Role.hostedAccountant()); - - var hasDeployments = hasDeployments(tenantName); - var result = billingController.setPlan(tenantName, planId, hasDeployments, isAccountant); - - if (result.isSuccess()) - return new StringResponse("Plan: " + planId.value()); - - return ErrorResponse.forbidden(result.getErrorMessage().orElse("Invalid plan change")); - } - - private HttpResponse patchCollectionMethod(HttpRequest request, String tenant) { - var tenantName = TenantName.from(tenant); - var slime = inspectorOrThrow(request); - var newMethod = slime.field("collection").valid() ? - slime.field("collection").asString().toUpperCase() : - slime.field("collectionMethod").asString().toUpperCase(); - if (newMethod.isEmpty()) return ErrorResponse.badRequest("No collection method specified"); - - try { - var result = billingController.setCollectionMethod(tenantName, CollectionMethod.valueOf(newMethod)); - if (result.isSuccess()) - return new StringResponse("Collection method updated to " + newMethod); - - return ErrorResponse.forbidden(result.getErrorMessage().orElse("Invalid collection method change")); - } catch (IllegalArgumentException iea){ - return ErrorResponse.badRequest("Invalid collection method: " + newMethod); - } - } - - private HttpResponse getBillingAllTenants(String until) { - try { - var untilDate = untilParameter(until); - var uncommittedBills = billingController.createUncommittedBills(untilDate); - - var slime = new Slime(); - var root = slime.setObject(); - root.setString("until", untilDate.format(DateTimeFormatter.ISO_DATE)); - var tenants = root.setArray("tenants"); - - tenantController.asList().stream().sorted(Comparator.comparing(Tenant::name)).forEach(tenant -> { - var bill = uncommittedBills.get(tenant.name()); - var tc = tenants.addObject(); - tc.setString("tenant", tenant.name().value()); - getPlanForTenant(tc, tenant.name()); - getCollectionForTenant(tc, tenant.name()); - renderCurrentUsage(tc.setObject("current"), bill); - renderAdditionalItems(tc.setObject("additional").setArray("items"), billingController.getUnusedLineItems(tenant.name())); - - billingController.getDefaultInstrument(tenant.name()).ifPresent(card -> - renderInstrument(tc.setObject("payment"), card) - ); - }); - - return new SlimeJsonResponse(slime); - } catch (DateTimeParseException e) { - return ErrorResponse.badRequest("Could not parse date: " + until); - } - } - - private void getCollectionForTenant(Cursor tc, TenantName tenant) { - var collection = billingController.getCollectionMethod(tenant); - tc.setString("collection", collection.name()); - } - - private HttpResponse addLineItem(HttpRequest request, String tenant, String userId) { - Inspector inspector = inspectorOrThrow(request); - - Optional<Bill.Id> billId = SlimeUtils.optionalString(inspector.field("billId")).map(Bill.Id::of); - - billingController.addLineItem( - TenantName.from(tenant), - getInspectorFieldOrThrow(inspector, "description"), - new BigDecimal(getInspectorFieldOrThrow(inspector, "amount")), - billId, - userId); - - return new MessageResponse("Added line item for tenant " + tenant); - } - - private HttpResponse setBillStatus(HttpRequest request, String billId, String userId) { - Inspector inspector = inspectorOrThrow(request); - String status = getInspectorFieldOrThrow(inspector, "status"); - billingController.updateBillStatus(Bill.Id.of(billId), userId, BillStatus.from(status)); - return new MessageResponse("Updated status of invoice " + billId); - } - - private HttpResponse createBill(HttpRequest request, String userId) { - Inspector inspector = inspectorOrThrow(request); - TenantName tenantName = TenantName.from(getInspectorFieldOrThrow(inspector, "tenant")); - - LocalDate startDate = LocalDate.parse(getInspectorFieldOrThrow(inspector, "startTime")); - LocalDate endDate = LocalDate.parse(getInspectorFieldOrThrow(inspector, "endTime")); - - var billId = billingController.createBillForPeriod(tenantName, startDate, endDate, userId); - - Slime slime = new Slime(); - Cursor root = slime.setObject(); - root.setString("message", "Created invoice with ID " + billId.value()); - root.setString("id", billId.value()); - return new SlimeJsonResponse(slime); - } - - private HttpResponse getInstruments(String tenant, String userId) { - var instrumentListResponse = billingController.listInstruments(TenantName.from(tenant), userId); - return new JacksonJsonResponse<>(200, instrumentListResponse); - } - - private HttpResponse getToken(String tenant, String userId) { - return new StringResponse(billingController.createClientToken(tenant, userId)); - } - - private HttpResponse getBilling(String tenant, String until) { - try { - var untilDate = untilParameter(until); - var tenantId = TenantName.from(tenant); - var slimeResponse = new Slime(); - var root = slimeResponse.setObject(); - - root.setString("until", untilDate.format(DateTimeFormatter.ISO_DATE)); - - getPlanForTenant(root, tenantId); - renderCurrentUsage(root.setObject("current"), getCurrentUsageForTenant(tenantId, untilDate)); - renderAdditionalItems(root.setObject("additional").setArray("items"), billingController.getUnusedLineItems(tenantId)); - renderBills(root.setArray("bills"), getBillsForTenant(tenantId)); - - billingController.getDefaultInstrument(tenantId).ifPresent( card -> - renderInstrument(root.setObject("payment"), card) - ); - - root.setString("collection", billingController.getCollectionMethod(tenantId).name()); - return new SlimeJsonResponse(slimeResponse); - } catch (DateTimeParseException e) { - return ErrorResponse.badRequest("Could not parse date: " + until); - } - } - - private HttpResponse getPlans() { - var slime = new Slime(); - var root = slime.setObject(); - var plans = root.setArray("plans"); - for (var plan : planRegistry.all()) { - var p = plans.addObject(); - p.setString("id", plan.id().value()); - p.setString("name", plan.displayName()); - } - return new SlimeJsonResponse(slime); - } - - private HttpResponse getLineItems(String tenant) { - var slimeResponse = new Slime(); - var root = slimeResponse.setObject(); - var lineItems = root.setArray("lineItems"); - - billingController.getUnusedLineItems(TenantName.from(tenant)) - .forEach(lineItem -> { - var itemCursor = lineItems.addObject(); - renderLineItemToCursor(itemCursor, lineItem); - }); - - return new SlimeJsonResponse(slimeResponse); - } - - private void getPlanForTenant(Cursor cursor, TenantName tenant) { - PlanId plan = billingController.getPlan(tenant); - cursor.setString("plan", plan.value()); - cursor.setString("planName", billingController.getPlanDisplayName(plan)); - } - - private void renderInstrument(Cursor cursor, PaymentInstrument instrument) { - cursor.setString("pi-id", instrument.getId()); - cursor.setString("type", instrument.getType()); - cursor.setString("brand", instrument.getBrand()); - cursor.setString("endingWith", instrument.getEndingWith()); - cursor.setString("expiryDate", instrument.getExpiryDate()); - cursor.setString("displayText", instrument.getDisplayText()); - cursor.setString("nameOnCard", instrument.getNameOnCard()); - cursor.setString("addressLine1", instrument.getAddressLine1()); - cursor.setString("addressLine2", instrument.getAddressLine2()); - cursor.setString("zip", instrument.getZip()); - cursor.setString("city", instrument.getCity()); - cursor.setString("state", instrument.getState()); - cursor.setString("country", instrument.getCountry()); - - } - - private void renderCurrentUsage(Cursor cursor, Bill currentUsage) { - if (currentUsage == null) return; - cursor.setString("amount", currentUsage.sum().toPlainString()); - cursor.setString("status", "accrued"); - cursor.setString("from", currentUsage.getStartDate().format(DATE_TIME_FORMATTER)); - var itemsCursor = cursor.setArray("items"); - currentUsage.lineItems().forEach(lineItem -> { - var itemCursor = itemsCursor.addObject(); - renderLineItemToCursor(itemCursor, lineItem); - }); - } - - private void renderAdditionalItems(Cursor cursor, List<Bill.LineItem> items) { - items.forEach(item -> { - renderLineItemToCursor(cursor.addObject(), item); - }); - } - - private Bill getCurrentUsageForTenant(TenantName tenant, LocalDate until) { - return billingController.createUncommittedBill(tenant, until); - } - - private List<Bill> getBillsForTenant(TenantName tenant) { - return billingController.getBillsForTenant(tenant); - } - - private void renderBills(Cursor cursor, List<Bill> bills) { - bills.forEach(bill -> { - var billCursor = cursor.addObject(); - renderBillToCursor(billCursor, bill); - }); - } - - private void renderBillToCursor(Cursor billCursor, Bill bill) { - billCursor.setString("id", bill.id().value()); - billCursor.setString("from", bill.getStartDate().format(DATE_TIME_FORMATTER)); - billCursor.setString("to", bill.getEndDate().format(DATE_TIME_FORMATTER)); - - billCursor.setString("amount", bill.sum().toString()); - billCursor.setString("status", bill.status().value()); - var statusCursor = billCursor.setArray("statusHistory"); - renderStatusHistory(statusCursor, bill.statusHistory()); - - - var lineItemsCursor = billCursor.setArray("items"); - bill.lineItems().forEach(lineItem -> { - var itemCursor = lineItemsCursor.addObject(); - renderLineItemToCursor(itemCursor, lineItem); - }); - } - - private void renderStatusHistory(Cursor cursor, StatusHistory statusHistory) { - statusHistory.getHistory() - .entrySet() - .stream() - .forEach(entry -> { - var c = cursor.addObject(); - c.setString("at", entry.getKey().format(DATE_TIME_FORMATTER)); - c.setString("status", entry.getValue().value()); - }); - } - - private void renderLineItemToCursor(Cursor cursor, Bill.LineItem lineItem) { - cursor.setString("id", lineItem.id()); - cursor.setString("description", lineItem.description()); - cursor.setString("amount", lineItem.amount().toString()); - cursor.setString("plan", lineItem.plan()); - cursor.setString("planName", billingController.getPlanDisplayName(PlanId.from(lineItem.plan()))); - - lineItem.applicationId().ifPresent(appId -> { - cursor.setString("application", appId.application().value()); - cursor.setString("instance", appId.instance().value()); - }); - lineItem.zoneId().ifPresent(zoneId -> - cursor.setString("zone", zoneId.value()) - ); - - lineItem.getArchitecture().ifPresent(architecture -> { - cursor.setString("architecture", architecture.name()); - }); - - cursor.setLong("majorVersion", lineItem.getMajorVersion()); - - if (! lineItem.getCloudAccount().isUnspecified()) - cursor.setString("cloudAccount", lineItem.getCloudAccount().value()); - - lineItem.getCpuHours().ifPresent(cpuHours -> - cursor.setString("cpuHours", cpuHours.toString()) - ); - lineItem.getMemoryHours().ifPresent(memoryHours -> - cursor.setString("memoryHours", memoryHours.toString()) - ); - lineItem.getDiskHours().ifPresent(diskHours -> - cursor.setString("diskHours", diskHours.toString()) - ); - lineItem.getGpuHours().ifPresent(gpuHours -> - cursor.setString("gpuHours", gpuHours.toString()) - ); - lineItem.getCpuCost().ifPresent(cpuCost -> - cursor.setString("cpuCost", cpuCost.toString()) - ); - lineItem.getMemoryCost().ifPresent(memoryCost -> - cursor.setString("memoryCost", memoryCost.toString()) - ); - lineItem.getDiskCost().ifPresent(diskCost -> - cursor.setString("diskCost", diskCost.toString()) - ); - lineItem.getGpuCost().ifPresent(gpuCost -> - cursor.setString("gpuCost", gpuCost.toString()) - ); - } - - private HttpResponse deleteInstrument(String tenant, String userId, String instrument) { - if (billingController.deleteInstrument(TenantName.from(tenant), userId, instrument)) { - return new StringResponse("OK"); - } else { - return ErrorResponse.forbidden("Cannot delete payment instrument you don't own"); - } - } - - private HttpResponse deleteLineItem(String lineItemId) { - billingController.deleteLineItem(lineItemId); - return new MessageResponse("Succesfully deleted line item " + lineItemId); - } - - private HttpResponse patchActiveInstrument(HttpRequest request, String tenant, String userId) { - var inspector = inspectorOrThrow(request); - String instrumentId = getInspectorFieldOrThrow(inspector, "active"); - InstrumentOwner paymentInstrument = new InstrumentOwner(TenantName.from(tenant), userId, instrumentId, true); - boolean success = billingController.setActivePaymentInstrument(paymentInstrument); - return success ? new StringResponse("OK") : ErrorResponse.internalServerError("Failed to patch active instrument"); - } - - private Inspector inspectorOrThrow(HttpRequest request) { - try { - return SlimeUtils.jsonToSlime(request.getData().readAllBytes()).get(); - } catch (IOException e) { - throw new IllegalArgumentException("Failed to parse request body"); - } - } - - private static String getInspectorFieldOrThrow(Inspector inspector, String field) { - if (!inspector.field(field).valid()) - throw new IllegalArgumentException("Field " + field + " cannot be null"); - return inspector.field(field).asString(); - } - - private LocalDate untilParameter(String until) { - if (until == null || until.isEmpty() || until.isBlank()) - return LocalDate.now(); - return LocalDate.parse(until); - } - - private boolean hasDeployments(TenantName tenantName) { - return applicationController.asList(tenantName) - .stream() - .flatMap(app -> app.instances().values() - .stream() - .flatMap(instance -> instance.deployments().values().stream()) - ) - .count() > 0; - } - - private static Set<Role> requestRoles(HttpRequest request) { - return Optional.ofNullable(request.getJDiscRequest().context().get(SecurityContext.ATTRIBUTE_NAME)) - .filter(SecurityContext.class::isInstance) - .map(SecurityContext.class::cast) - .map(SecurityContext::roles) - .orElseThrow(() -> new IllegalArgumentException("Attribute '" + SecurityContext.ATTRIBUTE_NAME + "' was not set on request")); - } - -} 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 edec869f559..b4237a46c5a 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 @@ -74,6 +74,12 @@ public class BillingApiHandlerV2 extends RestApiRequestHandler<BillingApiHandler private static RestApi createRestApi(BillingApiHandlerV2 self) { return RestApi.builder() /* + * This is the API that is tenant agnostic + */ + .addRoute(RestApi.route("/billing/v2/countries") + .get(self::acceptedCountries)) + + /* * This is the API that is available to tenants to view their status */ .addRoute(RestApi.route("/billing/v2/tenant/{tenant}") @@ -92,9 +98,6 @@ public class BillingApiHandlerV2 extends RestApiRequestHandler<BillingApiHandler .get(self::accountant)) .addRoute(RestApi.route("/billing/v2/accountant/preview") .get(self::accountantPreview)) - .addRoute(RestApi.route("/billing/v2/accountant/preview/tenant/{tenant}") - .get(self::previewBill) - .post(Slime.class, self::createBill)) .addRoute(RestApi.route("/billing/v2/accountant/tenant/{tenant}") .get(self::accountantTenant)) .addRoute(RestApi.route("/billing/v2/accountant/tenant/{tenant}/preview") @@ -119,6 +122,27 @@ public class BillingApiHandlerV2 extends RestApiRequestHandler<BillingApiHandler .build(); } + // ---------- AUX API ------------- + + private SlimeJsonResponse acceptedCountries(RestApi.RequestContext ctx) { + var response = new Slime(); + var countries = response.setObject().setArray("countries"); + billing.getAcceptedCountries().countries().forEach(country -> { + var countryCursor = countries.addObject(); + countryCursor.setString("code", country.code()); + countryCursor.setString("name", country.displayName()); + var taxTypesCursors = countryCursor.setArray("taxTypes"); + country.taxTypes().forEach(taxType -> { + var taxTypeCursor = taxTypesCursors.addObject(); + taxTypeCursor.setString("id", taxType.id()); + taxTypeCursor.setString("description", taxType.description()); + taxTypeCursor.setString("pattern", taxType.pattern()); + taxTypeCursor.setString("example", taxType.example()); + }); + }); + return new SlimeJsonResponse(response, /*compact*/false); + } + // ---------- TENANT API ---------- private Slime tenant(RestApi.RequestContext requestContext) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandlerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandlerTest.java deleted file mode 100644 index f3147d2adde..00000000000 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandlerTest.java +++ /dev/null @@ -1,236 +0,0 @@ -// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.restapi.billing; - -import com.yahoo.config.provision.SystemName; -import com.yahoo.config.provision.TenantName; -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.CollectionMethod; -import com.yahoo.vespa.hosted.controller.api.integration.billing.MockBillingController; -import com.yahoo.vespa.hosted.controller.api.integration.billing.PlanId; -import com.yahoo.vespa.hosted.controller.api.integration.billing.StatusHistory; -import com.yahoo.vespa.hosted.controller.api.role.Role; -import com.yahoo.vespa.hosted.controller.restapi.ContainerTester; -import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerCloudTest; -import com.yahoo.vespa.hosted.controller.security.Auth0Credentials; -import com.yahoo.vespa.hosted.controller.security.CloudTenantSpec; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -import java.io.File; -import java.math.BigDecimal; -import java.time.LocalDate; -import java.time.ZoneOffset; -import java.time.ZonedDateTime; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.Set; -import java.util.TreeMap; - -import static com.yahoo.application.container.handler.Request.Method.GET; -import static com.yahoo.application.container.handler.Request.Method.PATCH; -import static com.yahoo.application.container.handler.Request.Method.POST; -import static org.junit.jupiter.api.Assertions.assertEquals; - -/** - * @author olaa - */ -public class BillingApiHandlerTest extends ControllerContainerCloudTest { - - private static final String responseFiles = "src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/"; - private static final TenantName tenant = TenantName.from("tenant1"); - private static final TenantName tenant2 = TenantName.from("tenant2"); - private static final Set<Role> tenantRole = Set.of(Role.administrator(tenant)); - private static final Set<Role> financeAdmin = Set.of(Role.hostedAccountant()); - private MockBillingController billingController; - - private ContainerTester tester; - - @BeforeEach - public void setup() { - tester = new ContainerTester(container, responseFiles); - billingController = (MockBillingController) tester.serviceRegistry().billingController(); - } - - @Override - protected SystemName system() { - return SystemName.PublicCd; - } - - @Override - protected String variablePartXml() { - return " <component id='com.yahoo.vespa.hosted.controller.security.CloudAccessControlRequests'/>\n" + - " <component id='com.yahoo.vespa.hosted.controller.security.CloudAccessControl'/>\n" + - - " <handler id='com.yahoo.vespa.hosted.controller.restapi.billing.BillingApiHandler'>\n" + - " <binding>http://*/billing/v1/*</binding>\n" + - " </handler>\n" + - - " <http>\n" + - " <server id='default' port='8080' />\n" + - " <filtering>\n" + - " <request-chain id='default'>\n" + - " <filter id='com.yahoo.vespa.hosted.controller.restapi.filter.ControllerAuthorizationFilter'/>\n" + - " <binding>http://*/*</binding>\n" + - " </request-chain>\n" + - " </filtering>\n" + - " </http>\n"; - } - - @Test - void list_plans() { - var listPlansRequest = request("/billing/v1/plans", GET) - .roles(Role.hostedAccountant()); - tester.assertResponse(listPlansRequest, "{\"plans\":[{\"id\":\"trial\",\"name\":\"Free Trial - for testing purposes\"},{\"id\":\"paid\",\"name\":\"Paid Plan - for testing purposes\"},{\"id\":\"none\",\"name\":\"None Plan - for testing purposes\"}]}"); - } - - @Test - void response_list_bills() { - var bill = createBill(); - - billingController.addBill(tenant, bill, true); - billingController.addBill(tenant, bill, false); - billingController.setPlan(tenant, PlanId.from("some-plan"), true, false); - - var request = request("/billing/v1/tenant/tenant1/billing?until=2020-05-28").roles(tenantRole); - tester.assertResponse(request, new File("tenant-billing-view.json")); - - } - - @Test - void test_bill_creation() { - var bills = billingController.getBillsForTenant(tenant); - assertEquals(0, bills.size()); - - String requestBody = "{\"tenant\":\"tenant1\", \"startTime\":\"2020-04-20\", \"endTime\":\"2020-05-20\"}"; - var request = request("/billing/v1/invoice", POST) - .data(requestBody) - .roles(tenantRole); - - tester.assertResponse(request, accessDenied, 403); - request.roles(financeAdmin); - tester.assertResponse(request, new File("invoice-creation-response.json")); - - bills = billingController.getBillsForTenant(tenant); - assertEquals(1, bills.size()); - Bill bill = bills.get(0); - assertEquals("2020-04-20T00:00Z", bill.getStartTime().toString()); - assertEquals("2020-05-21T00:00Z", bill.getEndTime().toString()); - - assertEquals("2020-04-20", bill.getStartDate().toString()); - assertEquals("2020-05-20", bill.getEndDate().toString()); - } - - @Test - void adding_and_listing_line_item() { - - var requestBody = "{" + - "\"description\":\"some description\"," + - "\"amount\":\"123.45\" " + - "}"; - - var request = request("/billing/v1/invoice/tenant/tenant1/line-item", POST) - .data(requestBody) - .roles(financeAdmin); - - tester.assertResponse(request, "{\"message\":\"Added line item for tenant tenant1\"}"); - - var lineItems = billingController.getUnusedLineItems(tenant); - assertEquals(1, lineItems.size()); - Bill.LineItem lineItem = lineItems.get(0); - assertEquals("some description", lineItem.description()); - assertEquals(new BigDecimal("123.45"), lineItem.amount()); - - request = request("/billing/v1/invoice/tenant/tenant1/line-item") - .roles(financeAdmin); - - tester.assertResponse(request, new File("line-item-list.json")); - } - - @Test - void adding_new_status() { - billingController.addBill(tenant, createBill(), true); - - var requestBody = "{\"status\":\"CLOSED\"}"; - var request = request("/billing/v1/invoice/id-1/status", POST) - .data(requestBody) - .roles(financeAdmin); - tester.assertResponse(request, "{\"message\":\"Updated status of invoice id-1\"}"); - - var bill = billingController.getBillsForTenant(tenant).get(0); - assertEquals(BillStatus.CLOSED, bill.status()); - } - - @Test - void list_all_unbilled_items() { - tester.controller().tenants().create(new CloudTenantSpec(tenant, ""), new Auth0Credentials(() -> "foo", Set.of(Role.hostedOperator()))); - tester.controller().tenants().create(new CloudTenantSpec(tenant2, ""), new Auth0Credentials(() -> "foo", Set.of(Role.hostedOperator()))); - - var bill = createBill(); - billingController.setPlan(tenant, PlanId.from("some-plan"), true, false); - billingController.setPlan(tenant2, PlanId.from("some-plan"), true, false); - billingController.addBill(tenant, bill, false); - billingController.addLineItem(tenant, "support", new BigDecimal("42"), Optional.empty(), "Smith"); - billingController.addBill(tenant2, bill, false); - - var request = request("/billing/v1/billing?until=2020-05-28").roles(financeAdmin); - - tester.assertResponse(request, new File("billing-all-tenants.json")); - } - - @Test - void csv_export() { - var bill = createBill(); - billingController.addBill(tenant, bill, true); - var csvRequest = request("/billing/v1/invoice/export", GET).roles(financeAdmin); - tester.assertResponse(csvRequest.get(), new File("billing-all-invoices"), 200, false); - } - - @Test - void patch_collection_method() { - test_patch_collection_with_field_name("collectionMethod"); - test_patch_collection_with_field_name("collection"); - } - - private void test_patch_collection_with_field_name(String fieldName) { - var planRequest = request("/billing/v1/tenant/tenant1/collection", PATCH) - .data("{\"" + fieldName + "\": \"invoice\"}") - .roles(financeAdmin); - tester.assertResponse(planRequest, "Collection method updated to INVOICE"); - assertEquals(CollectionMethod.INVOICE, billingController.getCollectionMethod(tenant)); - - // Test that not event tenant administrators can do this - planRequest = request("/billing/v1/tenant/tenant1/collection", PATCH) - .data("{\"collectionMethod\": \"epay\"}") - .roles(tenantRole); - tester.assertResponse(planRequest, accessDenied, 403); - assertEquals(CollectionMethod.INVOICE, billingController.getCollectionMethod(tenant)); - } - - static Bill createBill() { - var start = LocalDate.of(2020, 5, 23).atStartOfDay(ZoneOffset.UTC); - var end = start.toLocalDate().plusDays(6).atStartOfDay(ZoneOffset.UTC); - var statusHistory = new StatusHistory(new TreeMap<>(Map.of(start, BillStatus.OPEN))); - return new Bill( - Bill.Id.of("id-1"), - TenantName.defaultName(), - statusHistory, - List.of(createLineItem(start)), - start, - end - ); - } - - static Bill.LineItem createLineItem(ZonedDateTime addedAt) { - return new Bill.LineItem( - "some-id", - "description", - new BigDecimal("123.00"), - "paid", - "Smith", - addedAt - ); - } - -} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandlerV2Test.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandlerV2Test.java index 4d3297ddb0c..356076a8d00 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandlerV2Test.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandlerV2Test.java @@ -4,7 +4,10 @@ package com.yahoo.vespa.hosted.controller.restapi.billing; import com.yahoo.application.container.handler.Request; import com.yahoo.config.provision.TenantName; import com.yahoo.test.ManualClock; +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.MockBillingController; +import com.yahoo.vespa.hosted.controller.api.integration.billing.StatusHistory; import com.yahoo.vespa.hosted.controller.api.role.Role; import com.yahoo.vespa.hosted.controller.restapi.ContainerTester; import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerCloudTest; @@ -13,8 +16,16 @@ import com.yahoo.vespa.hosted.controller.security.CloudTenantSpec; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.math.BigDecimal; +import java.io.File; import java.time.Instant; +import java.time.LocalDate; +import java.time.ZoneOffset; +import java.time.ZonedDateTime; +import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.TreeMap; /** * @author ogronnesby @@ -29,11 +40,6 @@ public class BillingApiHandlerV2Test extends ControllerContainerCloudTest { private static final Set<Role> tenantAdmin = Set.of(Role.administrator(tenant)); private static final Set<Role> financeAdmin = Set.of(Role.hostedAccountant()); - private static final String ACCESS_DENIED = "{\n" + - " \"code\" : 403,\n" + - " \"message\" : \"Access denied\"\n" + - "}"; - private MockBillingController billingController; private ContainerTester tester; @@ -44,7 +50,7 @@ public class BillingApiHandlerV2Test extends ControllerContainerCloudTest { var clock = (ManualClock) tester.controller().serviceRegistry().clock(); clock.setInstant(Instant.parse("2021-04-13T00:00:00Z")); billingController = (MockBillingController) tester.serviceRegistry().billingController(); - billingController.addBill(tenant, BillingApiHandlerTest.createBill(), true); + billingController.addBill(tenant, createBill(), true); } @Override @@ -122,7 +128,7 @@ public class BillingApiHandlerV2Test extends ControllerContainerCloudTest { @Test void require_accountant_preview() { var accountantRequest = request("/billing/v2/accountant/preview").roles(Role.hostedAccountant()); - billingController.uncommittedBills.put(tenant, BillingApiHandlerTest.createBill()); + billingController.uncommittedBills.put(tenant, createBill()); tester.assertResponse(accountantRequest, """ {"tenants":[{"tenant":"tenant1","plan":{"id":"trial","name":"Free Trial - for testing purposes"},"quota":{"budget":-1.0},"collection":"AUTO","lastBill":"2020-05-23","unbilled":"123.00"}]}"""); @@ -245,4 +251,35 @@ public class BillingApiHandlerV2Test extends ControllerContainerCloudTest { tester.assertResponse(accountantRequest, """ {"tenant":"tenant1","plan":{"id":"trial","name":"Free Trial - for testing purposes","billed":false,"supported":false},"billing":{},"collection":"AUTO"}"""); } + + @Test + void lists_accepted_countries() { + var req = request("/billing/v2/countries").roles(tenantReader); + tester.assertJsonResponse(req, new File("accepted-countries.json")); + } + + private static Bill createBill() { + var start = LocalDate.of(2020, 5, 23).atStartOfDay(ZoneOffset.UTC); + var end = start.toLocalDate().plusDays(6).atStartOfDay(ZoneOffset.UTC); + var statusHistory = new StatusHistory(new TreeMap<>(Map.of(start, BillStatus.OPEN))); + return new Bill( + Bill.Id.of("id-1"), + TenantName.defaultName(), + statusHistory, + List.of(createLineItem(start)), + start, + end + ); + } + + static Bill.LineItem createLineItem(ZonedDateTime addedAt) { + return new Bill.LineItem( + "some-id", + "description", + new BigDecimal("123.00"), + "paid", + "Smith", + addedAt + ); + } } 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 new file mode 100644 index 00000000000..2714de1e64d --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/accepted-countries.json @@ -0,0 +1,34 @@ +{ + "countries": [ + { + "code": "NO", + "name": "Norway", + "taxTypes": [ + { + "id": "no_vat", + "description": "Norwegian VAT number", + "pattern": "[0-9]{9}MVA", + "example": "123456789MVA" + } + ] + }, + { + "code": "CA", + "name": "Canada", + "taxTypes": [ + { + "id": "ca_gst_hst", + "description": "Canadian GST/HST number", + "pattern": "([0-9]{9}) ?RT ?([0-9]{4})", + "example": "123456789RT0002" + }, + { + "id": "ca_pst_bc", + "description": "Canadian PST number (British Columbia)", + "pattern": "PST-?([0-9]{4})-?([0-9]{4})", + "example": "PST-1234-5678" + } + ] + } + ] +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/billing-all-invoices b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/billing-all-invoices deleted file mode 100644 index 957ed858951..00000000000 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/billing-all-invoices +++ /dev/null @@ -1,2 +0,0 @@ -ID,Tenant,From,To,CpuHours,MemoryHours,DiskHours,Cpu,Memory,Disk,Additional -id-1,default,2020-05-23,2020-05-28,0.00,0.00,0.00,0.00,0.00,0.00,123.00 diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/billing-all-tenants.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/billing-all-tenants.json deleted file mode 100644 index 85c34edf7db..00000000000 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/billing-all-tenants.json +++ /dev/null @@ -1,62 +0,0 @@ -{ - "until": "2020-05-28", - "tenants": [ - { - "tenant": "tenant1", - "plan": "some-plan", - "planName": "Plan with id: some-plan", - "collection": "AUTO", - "current": { - "amount": "123.00", - "status": "accrued", - "from": "2020-05-23", - "items": [ - { - "id": "some-id", - "description": "description", - "amount": "123.00", - "plan": "paid", - "planName": "Plan with id: paid", - "majorVersion": 0 - } - ] - }, - "additional": { - "items": [ - { - "id": "line-item-id", - "description": "support", - "amount": "42.00", - "plan": "paid", - "planName": "Plan with id: paid", - "majorVersion": 0 - } - ] - } - }, - { - "tenant": "tenant2", - "plan": "some-plan", - "planName": "Plan with id: some-plan", - "collection": "AUTO", - "current": { - "amount": "123.00", - "status": "accrued", - "from": "2020-05-23", - "items": [ - { - "id": "some-id", - "description": "description", - "amount": "123.00", - "plan": "paid", - "planName": "Plan with id: paid", - "majorVersion": 0 - } - ] - }, - "additional": { - "items": [ ] - } - } - ] -} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/invoice-creation-response.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/invoice-creation-response.json deleted file mode 100644 index 49fde010c58..00000000000 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/invoice-creation-response.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "message": "Created invoice with ID id-123", - "id": "id-123" -} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/line-item-list.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/line-item-list.json deleted file mode 100644 index 8b69ea78754..00000000000 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/line-item-list.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "lineItems": [ - { - "id": "line-item-id", - "description": "some description", - "amount": "123.45", - "plan": "paid", - "planName": "Plan with id: paid", - "majorVersion": 0 - } - ] -} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/tenant-billing-view.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/tenant-billing-view.json deleted file mode 100644 index 4e255205e19..00000000000 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/billing/responses/tenant-billing-view.json +++ /dev/null @@ -1,49 +0,0 @@ -{ - "until": "2020-05-28", - "plan": "some-plan", - "planName": "Plan with id: some-plan", - "current": { - "amount": "123.00", - "status": "accrued", - "from": "2020-05-23", - "items": [ - { - "id": "some-id", - "description": "description", - "amount": "123.00", - "plan": "paid", - "planName": "Plan with id: paid", - "majorVersion": 0 - } - ] - }, - "additional": { - "items": [ ] - }, - "bills": [ - { - "id": "id-1", - "from": "2020-05-23", - "to": "2020-05-28", - "amount": "123.00", - "status": "OPEN", - "statusHistory": [ - { - "at": "2020-05-23", - "status": "OPEN" - } - ], - "items": [ - { - "id": "some-id", - "description": "description", - "amount": "123.00", - "plan": "paid", - "planName": "Plan with id: paid", - "majorVersion": 0 - } - ] - } - ], - "collection": "AUTO" -} diff --git a/dependency-versions/pom.xml b/dependency-versions/pom.xml index f404cc529de..d151c59ceea 100644 --- a/dependency-versions/pom.xml +++ b/dependency-versions/pom.xml @@ -84,7 +84,7 @@ <commons-csv.vespa.version>1.10.0</commons-csv.vespa.version> <commons-digester.vespa.version>3.2</commons-digester.vespa.version> <commons-exec.vespa.version>1.3</commons-exec.vespa.version> - <commons-io.vespa.version>2.14.0</commons-io.vespa.version> + <commons-io.vespa.version>2.15.0</commons-io.vespa.version> <commons-lang3.vespa.version>3.13.0</commons-lang3.vespa.version> <commons.math3.vespa.version>3.6.1</commons.math3.vespa.version> <commons-compress.vespa.version>1.24.0</commons-compress.vespa.version> diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java index 0f1d3e93cc3..c3fea72fab9 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java @@ -99,7 +99,8 @@ public class FailedExpirer extends NodeRepositoryMaintainer { .map(Node::hostname) .toList(); if (unparkedChildren.isEmpty()) { - return Optional.of(nodeRepository.nodes().park(node.hostname(), true, Agent.FailedExpirer, + // Only forcing de-provisioning of off premises nodes + return Optional.of(nodeRepository.nodes().park(node.hostname(), nodeRepository.zone().cloud().dynamicProvisioning(), Agent.FailedExpirer, "Parked by FailedExpirer due to " + reason.get())); } else { log.info(String.format("Expired failed node %s was not parked because of unparked children: %s", diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.cpp b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.cpp index 5f9a44f691c..c0d7cb207bf 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.cpp +++ b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.cpp @@ -24,11 +24,9 @@ PostingListSearchContext(const IEnumStoreDictionary& dictionary, bool has_btree_ _dictSize(_frozenDictionary.size()), _pidx(), _frozenRoot(), - _FSTC(0.0), - _PLSTC(0.0), _hasWeight(hasWeight), _useBitVector(useBitVector), - _counted_hits() + _estimated_hits() { } @@ -72,6 +70,17 @@ PostingListSearchContext::lookupSingle() } } +size_t +PostingListSearchContext::estimated_hits_in_range() const +{ + if (_estimated_hits.has_value()) { + return _estimated_hits.value(); + } + size_t result = calc_estimated_hits_in_range(); + _estimated_hits = result; + return result; +} + template class PostingListSearchContextT<vespalib::btree::BTreeNoLeafData>; template class PostingListSearchContextT<int32_t>; template class PostingListFoldedSearchContextT<vespalib::btree::BTreeNoLeafData>; diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.h b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.h index 91383bfe5f9..562c15e94d5 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.h +++ b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.h @@ -35,10 +35,6 @@ protected: using EntryRef = vespalib::datastore::EntryRef; using EnumIndex = IEnumStore::Index; - static constexpr long MIN_UNIQUE_VALUES_BEFORE_APPROXIMATION = 100; - static constexpr long MIN_UNIQUE_VALUES_TO_NUMDOCS_RATIO_BEFORE_APPROXIMATION = 20; - static constexpr long MIN_APPROXHITS_TO_NUMDOCS_RATIO_BEFORE_APPROXIMATION = 10; - const IEnumStoreDictionary& _dictionary; const ISearchContext& _baseSearchCtx; const BitVector* _bv; // bitvector if _useBitVector has been set @@ -51,11 +47,9 @@ protected: uint32_t _dictSize; EntryRef _pidx; EntryRef _frozenRoot; // Posting list in tree form - float _FSTC; // Filtering Search Time Constant - float _PLSTC; // Posting List Search Time Constant bool _hasWeight; bool _useBitVector; - mutable std::optional<size_t> _counted_hits; // Snapshot of size of posting lists in range + mutable std::optional<size_t> _estimated_hits; // Snapshot of size of posting lists in range PostingListSearchContext(const IEnumStoreDictionary& dictionary, bool has_btree_dictionary, uint32_t docIdLimit, uint64_t numValues, bool hasWeight, bool useBitVector, const ISearchContext &baseSearchCtx); @@ -65,48 +59,18 @@ protected: void lookupTerm(const vespalib::datastore::EntryComparator &comp); void lookupRange(const vespalib::datastore::EntryComparator &low, const vespalib::datastore::EntryComparator &high); void lookupSingle(); + size_t estimated_hits_in_range() const; virtual bool use_dictionary_entry(DictionaryConstIterator& it) const { (void) it; return true; } + virtual bool use_posting_lists_when_non_strict(const queryeval::ExecuteInfo& info) const = 0; - float calculateFilteringCost() const { - // filtering search time (ms) ~ FSTC * numValues; (FSTC = - // Filtering Search Time Constant) - return _FSTC * _numValues; - } - - float calculatePostingListCost(uint32_t approxNumHits) const { - // search time (ms) ~ PLSTC * numHits * log(numHits); (PLSTC = - // Posting List Search Time Constant) - return _PLSTC * approxNumHits; - } - - uint32_t calculateApproxNumHits() const { - float docsPerUniqueValue = static_cast<float>(_docIdLimit) / - static_cast<float>(_dictSize); - return static_cast<uint32_t>(docsPerUniqueValue * _uniqueValues); - } - - virtual bool fallbackToFiltering() const { - if (_uniqueValues >= 2 && !_dictionary.get_has_btree_dictionary()) { - return true; // force filtering for range search - } - uint32_t numHits = calculateApproxNumHits(); - // numHits > 1000: make sure that posting lists are unit tested. - return (numHits > 1000) && - (calculateFilteringCost() < calculatePostingListCost(numHits)); - } - virtual bool use_posting_list_when_non_strict(const queryeval::ExecuteInfo&) const { - return false; - } - virtual bool fallback_to_approx_num_hits() const { - return ((_uniqueValues > MIN_UNIQUE_VALUES_BEFORE_APPROXIMATION) && - ((_uniqueValues * MIN_UNIQUE_VALUES_TO_NUMDOCS_RATIO_BEFORE_APPROXIMATION > static_cast<int>(_docIdLimit)) || - (calculateApproxNumHits() * MIN_APPROXHITS_TO_NUMDOCS_RATIO_BEFORE_APPROXIMATION > _docIdLimit) || - (_uniqueValues > MIN_UNIQUE_VALUES_BEFORE_APPROXIMATION*10))); - } - virtual size_t countHits() const = 0; + /** + * Calculates the estimated number of hits when _uniqueValues >= 2, + * by looking at the posting lists in the range [lower, upper>. + */ + virtual size_t calc_estimated_hits_in_range() const = 0; virtual void fillArray() = 0; virtual void fillBitVector() = 0; }; @@ -136,7 +100,6 @@ protected: ~PostingListSearchContextT() override; void lookupSingle(); - size_t countHits() const override; void fillArray() override; void fillBitVector() override; @@ -166,7 +129,6 @@ protected: using DictionaryConstIterator = Dictionary::ConstIterator; using EntryRef = vespalib::datastore::EntryRef; using PostingList = typename Parent::PostingList; - using Parent::_counted_hits; using Parent::_docIdLimit; using Parent::_lowerDictItr; using Parent::_merger; @@ -184,8 +146,7 @@ protected: bool useBitVector, const ISearchContext &baseSearchCtx); ~PostingListFoldedSearchContextT() override; - bool fallback_to_approx_num_hits() const override; - size_t countHits() const override; + size_t calc_estimated_hits_in_range() const override; template <bool fill_array> void fill_array_or_bitvector_helper(EntryRef pidx); template <bool fill_array> @@ -217,13 +178,13 @@ private: using Parent = PostingSearchContext<BaseSC, PostingListFoldedSearchContextT<DataT>, AttrT>; using RegexpUtil = vespalib::RegexpUtil; using Parent::_enumStore; - // Note: steps iterator one ore more steps when not using dictionary entry + // Note: Steps iterator one or more steps when not using dictionary entry bool use_dictionary_entry(PostingListSearchContext::DictionaryConstIterator& it) const override; // Note: Uses copy of dictionary iterator to avoid stepping original. bool use_single_dictionary_entry(PostingListSearchContext::DictionaryConstIterator it) const { return use_dictionary_entry(it); } - bool use_posting_list_when_non_strict(const queryeval::ExecuteInfo&) const override; + bool use_posting_lists_when_non_strict(const queryeval::ExecuteInfo& info) const override; public: StringPostingSearchContext(BaseSC&& base_sc, bool useBitVector, const AttrT &toBeSearched); }; @@ -245,11 +206,6 @@ private: void getIterators(bool shouldApplyRangeLimit); bool valid() const override { return this->isValid(); } - bool fallbackToFiltering() const override { - return (this->getRangeLimit() != 0) - ? (this->_uniqueValues >= 2 && !this->_dictionary.get_has_btree_dictionary()) - : Parent::fallbackToFiltering(); - } unsigned int approximateHits() const override { const unsigned int estimate = PostingListSearchContextT<DataT>::approximateHits(); const unsigned int limit = std::abs(this->getRangeLimit()); @@ -269,6 +225,9 @@ private: } } + bool use_posting_lists_when_non_strict(const queryeval::ExecuteInfo& info) const override; + size_t calc_estimated_hits_in_range() const override; + public: NumericPostingSearchContext(BaseSC&& base_sc, const Params & params, const AttrT &toBeSearched); const Params ¶ms() const { return _params; } @@ -301,14 +260,6 @@ StringPostingSearchContext<BaseSC, AttrT, DataT>:: StringPostingSearchContext(BaseSC&& base_sc, bool useBitVector, const AttrT &toBeSearched) : Parent(std::move(base_sc), useBitVector, toBeSearched) { - // after benchmarking prefix search performance on single, array, and weighted set fast-aggregate string attributes - // with 1M values the following constant has been derived: - this->_FSTC = 0.000028; - - // after benchmarking prefix search performance on single, array, and weighted set fast-search string attributes - // with 1M values the following constant has been derived: - this->_PLSTC = 0.000000; - if (this->valid()) { if (this->isPrefix()) { auto comp = _enumStore.make_folded_comparator_prefix(this->queryTerm()->getTerm()); @@ -363,11 +314,11 @@ StringPostingSearchContext<BaseSC, AttrT, DataT>::use_dictionary_entry(PostingLi template <typename BaseSC, typename AttrT, typename DataT> bool -StringPostingSearchContext<BaseSC, AttrT, DataT>::use_posting_list_when_non_strict(const queryeval::ExecuteInfo& info) const +StringPostingSearchContext<BaseSC, AttrT, DataT>::use_posting_lists_when_non_strict(const queryeval::ExecuteInfo& info) const { if (this->isFuzzy()) { uint32_t exp_doc_hits = this->_docIdLimit * info.hitRate(); - constexpr uint32_t fuzzy_use_posting_list_doc_limit = 10000; + constexpr uint32_t fuzzy_use_posting_lists_doc_limit = 10000; /** * The above constant was derived after a query latency experiment with fuzzy matching * on 2M documents with a dictionary size of 292070. @@ -390,7 +341,7 @@ StringPostingSearchContext<BaseSC, AttrT, DataT>::use_posting_list_when_non_stri * is already performed at this point. * The only work remaining if returning true is merging the posting lists. */ - if (exp_doc_hits > fuzzy_use_posting_list_doc_limit) { + if (exp_doc_hits > fuzzy_use_posting_lists_doc_limit) { return true; } } @@ -403,11 +354,6 @@ NumericPostingSearchContext(BaseSC&& base_sc, const Params & params_in, const At : Parent(std::move(base_sc), params_in.useBitVector(), toBeSearched), _params(params_in) { - // after simplyfying the formula and simple benchmarking and thumbs in the air - // a ratio of 8 between numvalues and estimated number of hits has been found. - this->_FSTC = 1; - - this->_PLSTC = 8; if (valid()) { if (_low == _high) { auto comp = _enumStore.make_comparator(_low); @@ -455,7 +401,68 @@ getIterators(bool shouldApplyRangeLimit) } } +template <typename BaseSC, typename AttrT, typename DataT> +bool +NumericPostingSearchContext<BaseSC, AttrT, DataT>::use_posting_lists_when_non_strict(const queryeval::ExecuteInfo& info) const +{ + // The following initial constants are derived after running parts of + // the range search performance test with 10M documents on an Apple M1 Pro with 32 GB memory. + // This code was compiled with two different behaviors: + // 1) 'filter matching' (never use posting lists). + // 2) 'posting list matching' (always use posting lists). + // https://github.com/vespa-engine/system-test/tree/master/tests/performance/range_search + // + // The following test case was used to establish the baseline cost of producing different number of hits as cheap as possible: + // range_hits_ratio=[1, 10, 50, 100, 200, 500], values_in_range=1, fast_search=true, filter_hits_ratio=0. + // The 6 range queries end up using a single posting list that produces the following number of hits: [10k, 100k, 500k, 1M, 2M, 5M] + // Avg query latency (ms) results: [5.43, 8.56, 11.68, 14.68, 22.77, 42.88] + // + // Then the following test case was executed for both 1) 'filter matching' and 2) 'posting list matching': + // range_hits_ratio=[1, 10, 50, 100, 200, 500], values_in_range=100, fast_search=true, filter_hits_ratio=0. + // Avg query latency (ms) results: + // 1) 'filter matching': [47.52, 51.06, 59.68, 79.3, 118.7, 145.26] + // 2) 'posting list matching': [4.79, 11.6, 13.54, 20.24, 32.65, 67.28] + // + // For 1) 'filter matching' we use the result from range_hits_ratio=1 (10k hits) compared to the baseline + // to calculate the cost per document (in ns) to do filter matching: 1M*(47.52-5.43)/10M = 4.2 + // + // For 2) 'posting list matching' we use the results from range_hits_ratio=[50, 100, 200, 500] compared to the baseline + // to calculate the average cost per hit (in ns) when merging the 100 posting lists: + // 1M*(13.54-11.68)/500k = 3.7 + // 1M*(20.24-14.68)/1M = 5.6 + // 1M*(32.65-22.77)/2M = 4.9 + // 1M*(67.28-42.88)/5M = 4.9 + // + // The average is 4.8. + + constexpr float filtering_match_constant = 4.2; + constexpr float posting_list_merge_constant = 4.8; + + uint32_t exp_doc_hits = this->_docIdLimit * info.hitRate(); + float avg_values_per_document = static_cast<float>(this->_numValues) / static_cast<float>(this->_docIdLimit); + float filtering_cost = exp_doc_hits * avg_values_per_document * filtering_match_constant; + float posting_list_cost = this->estimated_hits_in_range() * posting_list_merge_constant; + return posting_list_cost < filtering_cost; +} +template <typename BaseSC, typename AttrT, typename DataT> +size_t +NumericPostingSearchContext<BaseSC, AttrT, DataT>::calc_estimated_hits_in_range() const +{ + size_t exact_sum = 0; + size_t estimated_sum = 0; + constexpr uint32_t max_posting_lists_to_count = 1000; + auto it = this->_lowerDictItr; + for (uint32_t count = 0; (it != this->_upperDictItr) && (count < max_posting_lists_to_count); ++it, ++count) { + exact_sum += this->_postingList.frozenSize(it.getData().load_acquire()); + } + if (it != this->_upperDictItr) { + uint32_t remaining_posting_lists = this->_upperDictItr - it; + float hits_per_posting_list = static_cast<float>(exact_sum) / static_cast<float>(max_posting_lists_to_count); + estimated_sum = remaining_posting_lists * hits_per_posting_list; + } + return exact_sum + estimated_sum; +} extern template class PostingListSearchContextT<vespalib::btree::BTreeNoLeafData>; extern template class PostingListSearchContextT<int32_t>; diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp index 7c7b9117a30..964101ed3a6 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp +++ b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp @@ -58,21 +58,6 @@ PostingListSearchContextT<DataT>::lookupSingle() } template <typename DataT> -size_t -PostingListSearchContextT<DataT>::countHits() const -{ - if (_counted_hits.has_value()) { - return _counted_hits.value(); - } - size_t sum(0); - for (auto it(_lowerDictItr); it != _upperDictItr; ++it) { - sum += _postingList.frozenSize(it.getData().load_acquire()); - } - _counted_hits = sum; - return sum; -} - -template <typename DataT> void PostingListSearchContextT<DataT>::fillArray() { @@ -97,9 +82,9 @@ template <typename DataT> void PostingListSearchContextT<DataT>::fetchPostings(const queryeval::ExecuteInfo & execInfo) { - if (!_merger.merge_done() && _uniqueValues >= 2u) { - if ((execInfo.isStrict() || use_posting_list_when_non_strict(execInfo)) && !fallbackToFiltering()) { - size_t sum(countHits()); + if (!_merger.merge_done() && _uniqueValues >= 2u && this->_dictionary.get_has_btree_dictionary()) { + if (execInfo.isStrict() || use_posting_lists_when_non_strict(execInfo)) { + size_t sum = estimated_hits_in_range(); if (sum < _docIdLimit / 64) { _merger.reserveArray(_uniqueValues, sum); fillArray(); @@ -221,19 +206,14 @@ PostingListSearchContextT<DataT>::approximateHits() const if (_uniqueValues == 0u) { } else if (_uniqueValues == 1u) { numHits = singleHits(); + } else if (_dictionary.get_has_btree_dictionary()) { + numHits = estimated_hits_in_range(); } else { - if (this->fallbackToFiltering()) { - numHits = _docIdLimit; - } else if (this->fallback_to_approx_num_hits()) { - numHits = this->calculateApproxNumHits(); - } else { - numHits = countHits(); - } + numHits = _docIdLimit; } return std::min(numHits, size_t(std::numeric_limits<uint32_t>::max())); } - template <typename DataT> void PostingListSearchContextT<DataT>::applyRangeLimit(int rangeLimit) @@ -273,20 +253,10 @@ template <typename DataT> PostingListFoldedSearchContextT<DataT>::~PostingListFoldedSearchContextT() = default; template <typename DataT> -bool -PostingListFoldedSearchContextT<DataT>::fallback_to_approx_num_hits() const -{ - return false; -} - -template <typename DataT> size_t -PostingListFoldedSearchContextT<DataT>::countHits() const +PostingListFoldedSearchContextT<DataT>::calc_estimated_hits_in_range() const { - if (_counted_hits.has_value()) { - return _counted_hits.value(); - } - size_t sum(0); + size_t sum = 0; bool overflow = false; for (auto it(_lowerDictItr); it != _upperDictItr;) { if (use_dictionary_entry(it)) { @@ -305,7 +275,6 @@ PostingListFoldedSearchContextT<DataT>::countHits() const ++it; } } - _counted_hits = sum; return sum; } diff --git a/storage/src/tests/distributor/mergelimitertest.cpp b/storage/src/tests/distributor/mergelimitertest.cpp index fc115b28c9a..7313c464a37 100644 --- a/storage/src/tests/distributor/mergelimitertest.cpp +++ b/storage/src/tests/distributor/mergelimitertest.cpp @@ -42,24 +42,30 @@ struct NodeFactory { operator const MergeLimiter::NodeArray&() const { return _nodes; } }; -#define ASSERT_LIMIT(maxNodes, nodes, result) \ -{ \ - MergeLimiter limiter(maxNodes); \ - auto nodesCopy = nodes; \ - limiter.limitMergeToMaxNodes(nodesCopy); \ - std::ostringstream actual; \ - for (uint32_t i = 0; i < nodesCopy.size(); ++i) { \ - if (i != 0) actual << ","; \ - actual << nodesCopy[i]._nodeIndex; \ - if (nodesCopy[i]._sourceOnly) actual << 's'; \ - } \ - ASSERT_EQ(result, actual.str()); \ } -} +struct MergeLimiterTest : Test { + + static std::string limit(uint32_t max_nodes, std::vector<MergeMetaData> nodes) { + MergeLimiter limiter(max_nodes); + limiter.limitMergeToMaxNodes(nodes); + std::ostringstream actual; + for (uint32_t i = 0; i < nodes.size(); ++i) { + if (i != 0) { + actual << ","; + } + actual << nodes[i]._nodeIndex; + if (nodes[i]._sourceOnly) { + actual << 's'; + } + } + return actual.str(); + } + +}; // If there is <= max nodes, then none should be removed. -TEST(MergeLimiterTest, keeps_all_below_limit) { +TEST_F(MergeLimiterTest, keeps_all_below_limit) { MergeLimiter::NodeArray nodes(NodeFactory() .addTrusted(3, 0x4) .addTrusted(5, 0x4) @@ -67,22 +73,22 @@ TEST(MergeLimiterTest, keeps_all_below_limit) { .add(2, 0x6) .add(4, 0x5)); - ASSERT_LIMIT(8, nodes, "3,5,9,2,4"); + ASSERT_EQ(limit(8, nodes), "3,5,9,2,4"); } // If less than max nodes is untrusted, merge all untrusted copies with a // trusted one. (Optionally with extra trusted copies if there is space) -TEST(MergeLimiterTest, less_than_max_untrusted) { +TEST_F(MergeLimiterTest, less_than_max_untrusted) { MergeLimiter::NodeArray nodes(NodeFactory() .addTrusted(3, 0x4) .addTrusted(5, 0x4) .add(9, 0x6) .add(2, 0x6) .add(4, 0x5)); - ASSERT_LIMIT(4, nodes, "2,4,9,5"); + ASSERT_EQ(limit(4, nodes), "2,4,9,5"); } // With more than max untrusted, just merge one trusted with as many untrusted // that fits. -TEST(MergeLimiterTest, more_than_max_untrusted) { +TEST_F(MergeLimiterTest, more_than_max_untrusted) { MergeLimiter::NodeArray nodes(NodeFactory() .addTrusted(3, 0x4) .addTrusted(5, 0x4) @@ -91,12 +97,12 @@ TEST(MergeLimiterTest, more_than_max_untrusted) { .add(13, 0x9) .add(1, 0x7) .add(4, 0x5)); - ASSERT_LIMIT(4, nodes, "2,13,1,5"); + ASSERT_EQ(limit(4, nodes), "2,13,1,5"); } // With nothing trusted. If there is <= max different variants (checksums), // merge one of each variant. After this merge, all these nodes can be set // trusted. (Except for any source only ones) -TEST(MergeLimiterTest, all_untrusted_less_than_max_variants) { +TEST_F(MergeLimiterTest, all_untrusted_less_than_max_variants) { MergeLimiter::NodeArray nodes(NodeFactory() .add(3, 0x4) .add(5, 0x4) @@ -105,11 +111,11 @@ TEST(MergeLimiterTest, all_untrusted_less_than_max_variants) { .add(13, 0x3) .add(1, 0x3) .add(4, 0x3)); - ASSERT_LIMIT(4, nodes, "5,2,4,3"); + ASSERT_EQ(limit(4, nodes), "5,2,4,3"); } // With nothing trusted and more than max variants, we just have to merge one // of each variant until we end up with less than max variants. -TEST(MergeLimiterTest, all_untrusted_more_than_max_variants) { +TEST_F(MergeLimiterTest, all_untrusted_more_than_max_variants) { MergeLimiter::NodeArray nodes(NodeFactory() .add(3, 0x4) .add(5, 0x5) @@ -118,12 +124,12 @@ TEST(MergeLimiterTest, all_untrusted_more_than_max_variants) { .add(13, 0x3) .add(1, 0x9) .add(4, 0x8)); - ASSERT_LIMIT(4, nodes, "3,5,2,13"); + ASSERT_EQ(limit(4, nodes), "3,5,2,13"); } // With more than max untrusted, just merge one trusted with as many untrusted // that fits. -TEST(MergeLimiterTest, source_only_last) { +TEST_F(MergeLimiterTest, source_only_last) { MergeLimiter::NodeArray nodes(NodeFactory() .addTrusted(3, 0x4) .addTrusted(5, 0x4).setSourceOnly() @@ -132,20 +138,20 @@ TEST(MergeLimiterTest, source_only_last) { .add(13, 0x9) .add(1, 0x7) .add(4, 0x5)); - ASSERT_LIMIT(4, nodes, "9,3,5s,2s"); + ASSERT_EQ(limit(4, nodes), "9,3,5s,2s"); } -TEST(MergeLimiterTest, limited_set_cannot_be_just_source_only) { +TEST_F(MergeLimiterTest, limited_set_cannot_be_just_source_only) { MergeLimiter::NodeArray nodes(NodeFactory() .addTrusted(9, 0x6) .addTrusted(2, 0x6) .addTrusted(13, 0x6).setSourceOnly() .add(1, 0x7).setSourceOnly()); - ASSERT_LIMIT(2, nodes, "2,13s"); - ASSERT_LIMIT(3, nodes, "2,13s,1s"); + ASSERT_EQ(limit(2, nodes), "2,13s"); + ASSERT_EQ(limit(3, nodes), "2,13s,1s"); } -TEST(MergeLimiterTest, non_source_only_replica_chosen_from_in_sync_group) { +TEST_F(MergeLimiterTest, non_source_only_replica_chosen_from_in_sync_group) { // nodes 9, 2, 13 are all in sync. Merge limiter will currently by default // pop the _last_ node of an in-sync replica "group" when outputting a limited // set. Unless we special-case source-only replicas here, we'd end up with an @@ -155,38 +161,38 @@ TEST(MergeLimiterTest, non_source_only_replica_chosen_from_in_sync_group) { .add(2, 0x6) .add(13, 0x6).setSourceOnly() .add(1, 0x7).setSourceOnly()); - ASSERT_LIMIT(2, nodes, "2,13s"); - ASSERT_LIMIT(3, nodes, "2,13s,1s"); + ASSERT_EQ(limit(2, nodes), "2,13s"); + ASSERT_EQ(limit(3, nodes), "2,13s,1s"); } -TEST(MergeLimiterTest, non_source_only_replicas_preferred_when_replicas_not_in_sync) { +TEST_F(MergeLimiterTest, non_source_only_replicas_preferred_when_replicas_not_in_sync) { MergeLimiter::NodeArray nodes(NodeFactory() .add(9, 0x4) .add(2, 0x5) .add(13, 0x6).setSourceOnly() .add(1, 0x7).setSourceOnly()); - ASSERT_LIMIT(2, nodes, "9,2"); - ASSERT_LIMIT(3, nodes, "9,2,13s"); + ASSERT_EQ(limit(2, nodes), "9,2"); + ASSERT_EQ(limit(3, nodes), "9,2,13s"); } -TEST(MergeLimiterTest, at_least_one_non_source_only_replica_chosen_when_all_trusted) { +TEST_F(MergeLimiterTest, at_least_one_non_source_only_replica_chosen_when_all_trusted) { MergeLimiter::NodeArray nodes(NodeFactory() .addTrusted(9, 0x6) .addTrusted(2, 0x6) .addTrusted(13, 0x6).setSourceOnly() .addTrusted(1, 0x6).setSourceOnly()); - ASSERT_LIMIT(2, nodes, "2,13s"); - ASSERT_LIMIT(3, nodes, "2,13s,1s"); + ASSERT_EQ(limit(2, nodes), "2,13s"); + ASSERT_EQ(limit(3, nodes), "2,13s,1s"); } -TEST(MergeLimiterTest, missing_replica_distinct_from_empty_replica) { +TEST_F(MergeLimiterTest, missing_replica_distinct_from_empty_replica) { MergeLimiter::NodeArray nodes(NodeFactory() .addEmpty(3) .addEmpty(5) .addMissing(1) .addMissing(2)); - ASSERT_LIMIT(2, nodes, "5,2"); - ASSERT_LIMIT(3, nodes, "5,2,3"); + ASSERT_EQ(limit(2, nodes), "5,2"); + ASSERT_EQ(limit(3, nodes), "5,2,3"); } } // storage::distributor diff --git a/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp b/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp index a96bee2d11a..1984d44652a 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp @@ -1127,7 +1127,7 @@ FileStorHandlerImpl::Stripe::flush() namespace { bool -message_type_is_merge_related(api::MessageType::Id msg_type_id) { +message_type_is_merge_related(api::MessageType::Id msg_type_id) noexcept { switch (msg_type_id) { case api::MessageType::MERGEBUCKET_ID: case api::MessageType::MERGEBUCKET_REPLY_ID: @@ -1135,6 +1135,11 @@ message_type_is_merge_related(api::MessageType::Id msg_type_id) { case api::MessageType::GETBUCKETDIFF_REPLY_ID: case api::MessageType::APPLYBUCKETDIFF_ID: case api::MessageType::APPLYBUCKETDIFF_REPLY_ID: + // DeleteBucket is usually (but not necessarily) executed in the context of a higher-level + // merge operation, but we include it here since we want to enforce that not all threads + // in a stripe can dispatch a bucket delete at the same time. This also provides a strict + // upper bound on the number of in-flight bucket deletes in the persistence core. + case api::MessageType::DELETEBUCKET_ID: return true; default: return false; } |