diff options
115 files changed, 1266 insertions, 680 deletions
diff --git a/application-model/src/main/java/com/yahoo/vespa/applicationmodel/ApplicationInstanceId.java b/application-model/src/main/java/com/yahoo/vespa/applicationmodel/ApplicationInstanceId.java index df19f6cf275..581e6806a23 100644 --- a/application-model/src/main/java/com/yahoo/vespa/applicationmodel/ApplicationInstanceId.java +++ b/application-model/src/main/java/com/yahoo/vespa/applicationmodel/ApplicationInstanceId.java @@ -10,6 +10,7 @@ import java.util.Objects; */ // TODO: Remove this and use ApplicationName/InstanceName instead (if you need it for the JSON stuff move it to that layer and don't let it leak) public class ApplicationInstanceId { + public static final ApplicationInstanceId CONFIG_SERVER = new ApplicationInstanceId("zone-config-servers"); private final String id; diff --git a/application-model/src/main/java/com/yahoo/vespa/applicationmodel/ClusterId.java b/application-model/src/main/java/com/yahoo/vespa/applicationmodel/ClusterId.java index 28ee5076191..603cfef5708 100644 --- a/application-model/src/main/java/com/yahoo/vespa/applicationmodel/ClusterId.java +++ b/application-model/src/main/java/com/yahoo/vespa/applicationmodel/ClusterId.java @@ -9,6 +9,8 @@ import java.util.Objects; * @author bjorncs */ public class ClusterId { + public static final ClusterId CONFIG_SERVER = new ClusterId("zone-config-servers"); + private final String id; public ClusterId(String id) { diff --git a/application-model/src/main/java/com/yahoo/vespa/applicationmodel/ServiceCluster.java b/application-model/src/main/java/com/yahoo/vespa/applicationmodel/ServiceCluster.java index 9c3e568642b..5d8286bd727 100644 --- a/application-model/src/main/java/com/yahoo/vespa/applicationmodel/ServiceCluster.java +++ b/application-model/src/main/java/com/yahoo/vespa/applicationmodel/ServiceCluster.java @@ -51,6 +51,13 @@ public class ServiceCluster { return applicationInstance.get(); } + public boolean isConfigServerCluster() { + return Objects.equals(applicationInstance.map(ApplicationInstance::tenantId), Optional.of(TenantId.HOSTED_VESPA)) && + Objects.equals(applicationInstance.map(ApplicationInstance::applicationInstanceId), Optional.of(ApplicationInstanceId.CONFIG_SERVER)) && + Objects.equals(clusterId, ClusterId.CONFIG_SERVER) && + Objects.equals(serviceType, ServiceType.CONFIG_SERVER); + } + @Override public String toString() { return "ServiceCluster{" + diff --git a/application-model/src/main/java/com/yahoo/vespa/applicationmodel/TenantId.java b/application-model/src/main/java/com/yahoo/vespa/applicationmodel/TenantId.java index ba9d6251569..0fb3db28448 100644 --- a/application-model/src/main/java/com/yahoo/vespa/applicationmodel/TenantId.java +++ b/application-model/src/main/java/com/yahoo/vespa/applicationmodel/TenantId.java @@ -10,6 +10,7 @@ import java.util.Objects; */ // TODO: Remove this and use TenantName instead (if you need it for the JSON stuff move it to that layer and don't let it leak) public class TenantId { + public static final TenantId HOSTED_VESPA = new TenantId("hosted-vespa"); private final String id; diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java index 46e380e1ebd..e8e12888768 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java @@ -419,15 +419,17 @@ public class DeployState implements ConfigDefinitionStore { String searchName = builder.importReader(reader, readerName, logger); String sdName = stripSuffix(readerName, ApplicationPackage.SD_NAME_SUFFIX); names.put(searchName, sdName); - if (!sdName.equals(searchName)) { + if ( ! sdName.equals(searchName)) { throw new IllegalArgumentException("Search definition file name ('" + sdName + "') and name of " + "search element ('" + searchName + "') are not equal for file '" + readerName + "'"); } } catch (ParseException e) { - throw new IllegalArgumentException("Could not parse search definition file '" + getSearchDefinitionRelativePath(reader.getName()) + "': " + e.getMessage(), e); + throw new IllegalArgumentException("Could not parse search definition file '" + + getSearchDefinitionRelativePath(reader.getName()) + "': " + e.getMessage(), e); } catch (IOException e) { - throw new IllegalArgumentException("Could not read search definition file '" + getSearchDefinitionRelativePath(reader.getName()) + "': " + e.getMessage(), e); + throw new IllegalArgumentException("Could not read search definition file '" + + getSearchDefinitionRelativePath(reader.getName()) + "': " + e.getMessage(), e); } finally { closeIgnoreException(reader.getReader()); } diff --git a/config-model/src/main/javacc/SDParser.jj b/config-model/src/main/javacc/SDParser.jj index 3141f7f7164..db22e73268c 100644 --- a/config-model/src/main/javacc/SDParser.jj +++ b/config-model/src/main/javacc/SDParser.jj @@ -2405,7 +2405,7 @@ TensorType tensorType(String errorMessage) : String tensorTypeString; } { - <TENSOR_TYPE> { tensorTypeString = token.image; } + ( <TENSOR_TYPE> ) { tensorTypeString = token.image; } { TensorType tensorType; try { diff --git a/config-model/src/test/cfg/admin/metricconfig/searchdefinitions/music.sd b/config-model/src/test/cfg/admin/metricconfig/searchdefinitions/music.sd index f7b182f66ea..32e8451d8e2 100644 --- a/config-model/src/test/cfg/admin/metricconfig/searchdefinitions/music.sd +++ b/config-model/src/test/cfg/admin/metricconfig/searchdefinitions/music.sd @@ -4,12 +4,10 @@ search music { field f1 type string { indexing: summary | index # index-to: f1, all - header } field f2 type string { indexing: summary | index # index-to: f2, all - body } } } diff --git a/config-model/src/test/cfg/application/app1/searchdefinitions/music.sd b/config-model/src/test/cfg/application/app1/searchdefinitions/music.sd index dda729d6a35..693afbd308d 100644 --- a/config-model/src/test/cfg/application/app1/searchdefinitions/music.sd +++ b/config-model/src/test/cfg/application/app1/searchdefinitions/music.sd @@ -9,35 +9,28 @@ search music { indexing: summary | index # How this field should be indexed # index-to: title, default # Create two indexes rank-type: about # Type of ranking settings to apply - header } field artist type string { indexing: summary | attribute | index # index-to: artist, default rank-type:about - header } field year type int { indexing: summary | attribute - header } # Increase rank score of popular documents regardless of query field popularity type int { indexing: summary | attribute - body } field url type uri { indexing: summary | index - header } - field cover type raw { - body - } + field cover type raw {} } diff --git a/config-model/src/test/cfg/application/app_genericservices/searchdefinitions/music.sd b/config-model/src/test/cfg/application/app_genericservices/searchdefinitions/music.sd index dda729d6a35..693afbd308d 100644 --- a/config-model/src/test/cfg/application/app_genericservices/searchdefinitions/music.sd +++ b/config-model/src/test/cfg/application/app_genericservices/searchdefinitions/music.sd @@ -9,35 +9,28 @@ search music { indexing: summary | index # How this field should be indexed # index-to: title, default # Create two indexes rank-type: about # Type of ranking settings to apply - header } field artist type string { indexing: summary | attribute | index # index-to: artist, default rank-type:about - header } field year type int { indexing: summary | attribute - header } # Increase rank score of popular documents regardless of query field popularity type int { indexing: summary | attribute - body } field url type uri { indexing: summary | index - header } - field cover type raw { - body - } + field cover type raw {} } diff --git a/config-model/src/test/cfg/application/ml_models/searchdefinitions/test.sd b/config-model/src/test/cfg/application/ml_models/searchdefinitions/test.sd index d7b3cc76339..ab5e42f983d 100644 --- a/config-model/src/test/cfg/application/ml_models/searchdefinitions/test.sd +++ b/config-model/src/test/cfg/application/ml_models/searchdefinitions/test.sd @@ -2,7 +2,7 @@ search test { document test { - field argument type tensor(d0[],d1[784]) { + field argument type tensor<float>(d0[],d1[784]) { indexing: attribute } } diff --git a/config-model/src/test/cfg/routing/content_two_clusters/searchdefinitions/mobile.sd b/config-model/src/test/cfg/routing/content_two_clusters/searchdefinitions/mobile.sd index feae86fe966..3abc9238e8a 100644 --- a/config-model/src/test/cfg/routing/content_two_clusters/searchdefinitions/mobile.sd +++ b/config-model/src/test/cfg/routing/content_two_clusters/searchdefinitions/mobile.sd @@ -4,12 +4,10 @@ search mobile { field f1 type string { indexing: summary | index # index-to: f1, all - header } field f2 type string { indexing: summary | index # index-to: f2, all - body } } } diff --git a/config-model/src/test/cfg/search/data/travel/searchdefinitions/TTEdge.sd b/config-model/src/test/cfg/search/data/travel/searchdefinitions/TTEdge.sd index 8cc6d05584c..e6707345235 100644 --- a/config-model/src/test/cfg/search/data/travel/searchdefinitions/TTEdge.sd +++ b/config-model/src/test/cfg/search/data/travel/searchdefinitions/TTEdge.sd @@ -4,7 +4,6 @@ document TTEdge { # This field will contain a colon separate map for travel times per transport mode field TransportMode type array<string> { indexing: summary | index - header } } diff --git a/config-model/src/test/cfg/search/data/travel/searchdefinitions/TTPOI.sd b/config-model/src/test/cfg/search/data/travel/searchdefinitions/TTPOI.sd index 0cb0596b31c..c39ef03add5 100644 --- a/config-model/src/test/cfg/search/data/travel/searchdefinitions/TTPOI.sd +++ b/config-model/src/test/cfg/search/data/travel/searchdefinitions/TTPOI.sd @@ -5,14 +5,12 @@ document TTPOI { field Categories type array<string> { indexing: summary | index # index-to: Categories - header } # sub catagories associated with the POI field SubCategories type array<string> { indexing: summary | index # index-to: SubCategories - header } } diff --git a/config-model/src/test/cfg/search/data/v2/inherited_rankprofiles/searchdefinitions/music.sd b/config-model/src/test/cfg/search/data/v2/inherited_rankprofiles/searchdefinitions/music.sd index 0d3859b65a0..da9e2d14c55 100644 --- a/config-model/src/test/cfg/search/data/v2/inherited_rankprofiles/searchdefinitions/music.sd +++ b/config-model/src/test/cfg/search/data/v2/inherited_rankprofiles/searchdefinitions/music.sd @@ -4,12 +4,10 @@ search music { field f1 type string { indexing: summary | index # index-to: f1, all - header } field f2 type string { indexing: summary | index # index-to: f2, all - body } } } diff --git a/config-model/src/test/cfg/storage/app_index_higher_than_num_nodes/searchdefinitions/music.sd b/config-model/src/test/cfg/storage/app_index_higher_than_num_nodes/searchdefinitions/music.sd index 0d3859b65a0..da9e2d14c55 100644 --- a/config-model/src/test/cfg/storage/app_index_higher_than_num_nodes/searchdefinitions/music.sd +++ b/config-model/src/test/cfg/storage/app_index_higher_than_num_nodes/searchdefinitions/music.sd @@ -4,12 +4,10 @@ search music { field f1 type string { indexing: summary | index # index-to: f1, all - header } field f2 type string { indexing: summary | index # index-to: f2, all - body } } } diff --git a/config-model/src/test/cfg/storage/clustercontroller_advanced/searchdefinitions/music.sd b/config-model/src/test/cfg/storage/clustercontroller_advanced/searchdefinitions/music.sd index 0d3859b65a0..da9e2d14c55 100644 --- a/config-model/src/test/cfg/storage/clustercontroller_advanced/searchdefinitions/music.sd +++ b/config-model/src/test/cfg/storage/clustercontroller_advanced/searchdefinitions/music.sd @@ -4,12 +4,10 @@ search music { field f1 type string { indexing: summary | index # index-to: f1, all - header } field f2 type string { indexing: summary | index # index-to: f2, all - body } } } diff --git a/config-model/src/test/configmodel/types/types.sd b/config-model/src/test/configmodel/types/types.sd index f34a6776b11..9bd9602008c 100644 --- a/config-model/src/test/configmodel/types/types.sd +++ b/config-model/src/test/configmodel/types/types.sd @@ -90,9 +90,9 @@ search types { #field wildcardfield2 type map<?,?> { #} - field arrarr type array<array<array<string>>> {header} - field maparr type array<map<string, string>> {header} - field complexarray type array< map<int, array<array<string>>> > {body} + field arrarr type array<array<array<string>>> {} + field maparr type array<map<string, string>> {} + field complexarray type array< map<int, array<array<string>>> > {} struct mystruct { field bytearr type array<byte>{} @@ -101,9 +101,9 @@ search types { field structfield type string {} } - field mystructfield type mystruct {header} - field mystructmap type map<int, mystruct> {header} - field mystructarr type array<mystruct> {header} + field mystructfield type mystruct {} + field mystructmap type map<int, mystruct> {} + field mystructarr type array<mystruct> {} struct folder { field Version type int {} @@ -130,7 +130,6 @@ search types { create-if-nonexistent remove-if-zero } - header } # Field defined same way as tag @@ -140,7 +139,6 @@ search types { create-if-nonexistent remove-if-zero } - header } } diff --git a/config-model/src/test/derived/inheritstruct/child.sd b/config-model/src/test/derived/inheritstruct/child.sd index cd3d4f51458..5ac69c429e1 100644 --- a/config-model/src/test/derived/inheritstruct/child.sd +++ b/config-model/src/test/derived/inheritstruct/child.sd @@ -3,7 +3,6 @@ search child { document child inherits parent { field child_struct_field type my_struct { indexing: summary | index - header match: prefix } } diff --git a/config-model/src/test/derived/mail/mail.sd b/config-model/src/test/derived/mail/mail.sd index 6d30891f307..6c2c51eaa8a 100644 --- a/config-model/src/test/derived/mail/mail.sd +++ b/config-model/src/test/derived/mail/mail.sd @@ -54,39 +54,31 @@ search mail { field body type string { indexing: summary | index match: substring - body } field attachmentcount type int { indexing: summary | index - body } field attachmentnames type string { indexing: index - body } field attachmenttypes type string { indexing: index - body } field attachmentlanguages type string { indexing: index match: prefix - body } field attachmentcontent type string { indexing: summary | index match: prefix - body } - field attachments type raw[] { - body - } + field attachments type raw[] {} } diff --git a/config-model/src/test/derived/music3/music3.sd b/config-model/src/test/derived/music3/music3.sd index 8aeed27d29f..a9da946f8c1 100644 --- a/config-model/src/test/derived/music3/music3.sd +++ b/config-model/src/test/derived/music3/music3.sd @@ -7,35 +7,25 @@ search music3 { indexing: summary | index # index-to: title, default rank-type: about - - header } field artist type string { indexing: summary | attribute | index # index-to: artist, default rank-type:about - - header } field year type int { indexing: summary | attribute - - header } # Increase rank score of popular documents regardless of query field popularity type int { indexing: summary | attribute - - header } field url type uri { indexing: summary | index - - header } } diff --git a/config-model/src/test/derived/streamingjuniper/streamingjuniper.sd b/config-model/src/test/derived/streamingjuniper/streamingjuniper.sd index 0d81ecd21f3..92b833ffd5d 100644 --- a/config-model/src/test/derived/streamingjuniper/streamingjuniper.sd +++ b/config-model/src/test/derived/streamingjuniper/streamingjuniper.sd @@ -3,12 +3,10 @@ search streamingjuniper { document streamingjuniper { field f1 type string { indexing: index | summary - header bolding: on } field f2 type string { indexing: index | summary - header summary: dynamic } } diff --git a/config-model/src/test/derived/streamingstructdefault/streamingstructdefault.sd b/config-model/src/test/derived/streamingstructdefault/streamingstructdefault.sd index 6d16a1b3808..46112db7454 100644 --- a/config-model/src/test/derived/streamingstructdefault/streamingstructdefault.sd +++ b/config-model/src/test/derived/streamingstructdefault/streamingstructdefault.sd @@ -8,11 +8,9 @@ search streamingstructdefault { field f1 type array<string> { indexing: index | summary summary-to: default - header } field f2 type array<sct> { indexing: index | summary - header } } document-summary default { diff --git a/config-model/src/test/derived/tensor/attributes.cfg b/config-model/src/test/derived/tensor/attributes.cfg index f1c95da7084..0c556aad868 100644 --- a/config-model/src/test/derived/tensor/attributes.cfg +++ b/config-model/src/test/derived/tensor/attributes.cfg @@ -17,7 +17,7 @@ attribute[].arity 8 attribute[].lowerbound -9223372036854775808 attribute[].upperbound 9223372036854775807 attribute[].densepostinglistthreshold 0.4 -attribute[].tensortype "tensor(x[2],y[])" +attribute[].tensortype "tensor<float>(x[2],y[1])" attribute[].imported false attribute[].name "f3" attribute[].datatype TENSOR diff --git a/config-model/src/test/derived/tensor/documenttypes.cfg b/config-model/src/test/derived/tensor/documenttypes.cfg index 0b75644f4ae..4e04ad4c309 100644 --- a/config-model/src/test/derived/tensor/documenttypes.cfg +++ b/config-model/src/test/derived/tensor/documenttypes.cfg @@ -29,7 +29,7 @@ documenttype[].datatype[].sstruct.field[].name "f2" documenttype[].datatype[].sstruct.field[].id 2080644671 documenttype[].datatype[].sstruct.field[].id_v6 1424572148 documenttype[].datatype[].sstruct.field[].datatype 21 -documenttype[].datatype[].sstruct.field[].detailedtype "tensor(x[2],y[])" +documenttype[].datatype[].sstruct.field[].detailedtype "tensor<float>(x[2],y[1])" documenttype[].datatype[].sstruct.field[].name "f3" documenttype[].datatype[].sstruct.field[].id 1295091863 documenttype[].datatype[].sstruct.field[].id_v6 1444109654 diff --git a/config-model/src/test/derived/tensor/rank-profiles.cfg b/config-model/src/test/derived/tensor/rank-profiles.cfg index 87c0b6fab42..1ce9227d323 100644 --- a/config-model/src/test/derived/tensor/rank-profiles.cfg +++ b/config-model/src/test/derived/tensor/rank-profiles.cfg @@ -1,6 +1,6 @@ rankprofile[].name "default" rankprofile[].fef.property[].name "vespa.type.attribute.f2" -rankprofile[].fef.property[].value "tensor(x[2],y[])" +rankprofile[].fef.property[].value "tensor<float>(x[2],y[1])" rankprofile[].fef.property[].name "vespa.type.attribute.f3" rankprofile[].fef.property[].value "tensor(x{})" rankprofile[].fef.property[].name "vespa.type.attribute.f4" @@ -17,7 +17,7 @@ rankprofile[].fef.property[].value "0" rankprofile[].fef.property[].name "vespa.dump.ignoredefaultfeatures" rankprofile[].fef.property[].value "true" rankprofile[].fef.property[].name "vespa.type.attribute.f2" -rankprofile[].fef.property[].value "tensor(x[2],y[])" +rankprofile[].fef.property[].value "tensor<float>(x[2],y[1])" rankprofile[].fef.property[].name "vespa.type.attribute.f3" rankprofile[].fef.property[].value "tensor(x{})" rankprofile[].fef.property[].name "vespa.type.attribute.f4" @@ -30,7 +30,7 @@ rankprofile[].fef.property[].value "rankingExpression(firstphase)" rankprofile[].fef.property[].name "rankingExpression(firstphase).rankingScript" rankprofile[].fef.property[].value "reduce(map(attribute(f4), f(x)(x * x)) + reduce(tensor(x[2],y[3])(random), count) * rename(attribute(f4), (x, y), (y, x)), sum)" rankprofile[].fef.property[].name "vespa.type.attribute.f2" -rankprofile[].fef.property[].value "tensor(x[2],y[])" +rankprofile[].fef.property[].value "tensor<float>(x[2],y[1])" rankprofile[].fef.property[].name "vespa.type.attribute.f3" rankprofile[].fef.property[].value "tensor(x{})" rankprofile[].fef.property[].name "vespa.type.attribute.f4" @@ -43,7 +43,7 @@ rankprofile[].fef.property[].value "rankingExpression(firstphase)" rankprofile[].fef.property[].name "rankingExpression(firstphase).rankingScript" rankprofile[].fef.property[].value "reduce(reduce(join(attribute(f4), tensor(x[2],y[2],z[3])((x==y)*(y==z)), f(a,b)(a * b)), sum, x), sum)" rankprofile[].fef.property[].name "vespa.type.attribute.f2" -rankprofile[].fef.property[].value "tensor(x[2],y[])" +rankprofile[].fef.property[].value "tensor<float>(x[2],y[1])" rankprofile[].fef.property[].name "vespa.type.attribute.f3" rankprofile[].fef.property[].value "tensor(x{})" rankprofile[].fef.property[].name "vespa.type.attribute.f4" @@ -60,7 +60,7 @@ rankprofile[].fef.property[].value "rankingExpression(firstphase)" rankprofile[].fef.property[].name "rankingExpression(firstphase).rankingScript" rankprofile[].fef.property[].value "reduce(rankingExpression(joinedtensors), sum)" rankprofile[].fef.property[].name "vespa.type.attribute.f2" -rankprofile[].fef.property[].value "tensor(x[2],y[])" +rankprofile[].fef.property[].value "tensor<float>(x[2],y[1])" rankprofile[].fef.property[].name "vespa.type.attribute.f3" rankprofile[].fef.property[].value "tensor(x{})" rankprofile[].fef.property[].name "vespa.type.attribute.f4" @@ -73,7 +73,7 @@ rankprofile[].fef.property[].value "rankingExpression(firstphase)" rankprofile[].fef.property[].name "rankingExpression(firstphase).rankingScript" rankprofile[].fef.property[].value "reduce(attribute(f5), sum)" rankprofile[].fef.property[].name "vespa.type.attribute.f2" -rankprofile[].fef.property[].value "tensor(x[2],y[])" +rankprofile[].fef.property[].value "tensor<float>(x[2],y[1])" rankprofile[].fef.property[].name "vespa.type.attribute.f3" rankprofile[].fef.property[].value "tensor(x{})" rankprofile[].fef.property[].name "vespa.type.attribute.f4" diff --git a/config-model/src/test/derived/tensor/tensor.sd b/config-model/src/test/derived/tensor/tensor.sd index 622be033229..b31352a2105 100644 --- a/config-model/src/test/derived/tensor/tensor.sd +++ b/config-model/src/test/derived/tensor/tensor.sd @@ -5,7 +5,7 @@ search tensor { field f1 type tensor(x[]) { indexing: summary } - field f2 type tensor(x[2],y[]) { + field f2 type tensor<float>(x[2],y[1]) { indexing: attribute } field f3 type tensor<double>(x{}) { diff --git a/config-model/src/test/derived/types/types.sd b/config-model/src/test/derived/types/types.sd index 839cb08dbd6..37ac6e7ee11 100644 --- a/config-model/src/test/derived/types/types.sd +++ b/config-model/src/test/derived/types/types.sd @@ -93,9 +93,9 @@ search types { #field wildcardfield2 type map<?,?> { #} - field arrarr type array<array<array<string>>> {header} - field maparr type array<map<string, string>> {header} - field complexarray type array< map<int, array<array<string>>> > {body} + field arrarr type array<array<array<string>>> {} + field maparr type array<map<string, string>> {} + field complexarray type array< map<int, array<array<string>>> > {} struct mystruct { field bytearr type array<byte>{} @@ -104,9 +104,9 @@ search types { field structfield type string {} } - field mystructfield type mystruct {header} - field mystructmap type map<int, mystruct> {header} - field mystructarr type array<mystruct> {header} + field mystructfield type mystruct {} + field mystructmap type map<int, mystruct> {} + field mystructarr type array<mystruct> {} struct folder { field Version type int {} @@ -133,7 +133,6 @@ search types { create-if-nonexistent remove-if-zero } - header } # Field defined same way as tag @@ -143,14 +142,11 @@ search types { create-if-nonexistent remove-if-zero } - header } } - field pst_sta_boldingoff_nomatch_tag_01 type tag { - body - } + field pst_sta_boldingoff_nomatch_tag_01 type tag {} field other type long { indexing: input along | attribute diff --git a/config-model/src/test/examples/header_body.sd b/config-model/src/test/examples/header_body.sd new file mode 100644 index 00000000000..561ec47b067 --- /dev/null +++ b/config-model/src/test/examples/header_body.sd @@ -0,0 +1,18 @@ +# Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +# Search definition with (ignored) header and body statements +# TODO: Remove in Vespa 8 +search header_body { + + document header_body { + + field title type string { + indexing: summary | index + header + } + + field description type string { + indexing: summary | index + body + } + } +} diff --git a/config-model/src/test/examples/rankpropvars.sd b/config-model/src/test/examples/rankpropvars.sd index 28959edbc09..bac02ea8316 100644 --- a/config-model/src/test/examples/rankpropvars.sd +++ b/config-model/src/test/examples/rankpropvars.sd @@ -52,28 +52,22 @@ document music { field title type string { indexing: index | summary - body } field artist type string { ## index-to: a indexing: index | summary - body } field year type int { indexing: attribute | summary ## index-to: y - body } - field url type uri { - body - } + field url type uri {} field Popularity type string { indexing: attribute | summary - body } } diff --git a/config-model/src/test/examples/simple.sd b/config-model/src/test/examples/simple.sd index 0435ea439df..8a97db6afa2 100644 --- a/config-model/src/test/examples/simple.sd +++ b/config-model/src/test/examples/simple.sd @@ -47,7 +47,6 @@ search simple { rank-type: tags stemming: none normalizing: none - header } field popularity type int { @@ -81,7 +80,6 @@ search simple { field categories type string { indexing: input categories_src | lowercase | normalize | index - body } field categoriesagain type string { diff --git a/config-model/src/test/examples/structoutsideofdocument.sd b/config-model/src/test/examples/structoutsideofdocument.sd index 4c52a248bf5..5c062ef19a6 100644 --- a/config-model/src/test/examples/structoutsideofdocument.sd +++ b/config-model/src/test/examples/structoutsideofdocument.sd @@ -11,6 +11,5 @@ search structoutsideofdocument { field nallestruct type array<nalle> { indexing: summary - body } } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/SearchDefinitionsParsingTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/SearchDefinitionsParsingTestCase.java index fda15528eda..fd4bb393c49 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/SearchDefinitionsParsingTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/SearchDefinitionsParsingTestCase.java @@ -80,4 +80,10 @@ public class SearchDefinitionsParsingTestCase extends SearchDefinitionTestCase { } } + // TODO: Remove in Vespa 8 + @Test + public void requireThatParserHandlesHeadAndBody() throws IOException, ParseException { + assertNotNull(SearchBuilder.buildFromFile("src/test/examples/header_body.sd")); + } + } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java index 8944409e1e9..2a4292f70fc 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java @@ -63,7 +63,7 @@ public class RankingExpressionWithOnnxTestCase { public void testOnnxReferenceWithConstantFeature() { RankProfileSearchFixture search = fixtureWith("constant(mytensor)", "onnx('mnist_softmax.onnx')", - "constant mytensor { file: ignored\ntype: tensor(d0[7],d1[784]) }", + "constant mytensor { file: ignored\ntype: tensor<float>(d0[7],d1[784]) }", null); search.assertFirstPhaseExpression(vespaExpression, "my_profile"); } @@ -73,7 +73,7 @@ public class RankingExpressionWithOnnxTestCase { String queryProfile = "<query-profile id='default' type='root'/>"; String queryProfileType = "<query-profile-type id='root'>" + - " <field name='query(mytensor)' type='tensor(d0[3],d1[784])'/>" + + " <field name='query(mytensor)' type='tensor<float>(d0[3],d1[784])'/>" + "</query-profile-type>"; StoringApplicationPackage application = new StoringApplicationPackage(applicationDir, queryProfile, @@ -93,7 +93,7 @@ public class RankingExpressionWithOnnxTestCase { RankProfileSearchFixture search = fixtureWith("attribute(mytensor)", "onnx('mnist_softmax.onnx')", null, - "field mytensor type tensor(d0[],d1[784]) { indexing: attribute }", + "field mytensor type tensor<float>(d0[],d1[784]) { indexing: attribute }", "Placeholder", application); search.assertFirstPhaseExpression(vespaExpression, "my_profile"); @@ -105,15 +105,15 @@ public class RankingExpressionWithOnnxTestCase { String queryProfile = "<query-profile id='default' type='root'/>"; String queryProfileType = "<query-profile-type id='root'>" + - " <field name='query(mytensor)' type='tensor(d0[3],d1[784],d2[10])'/>" + + " <field name='query(mytensor)' type='tensor<float>(d0[3],d1[784],d2[10])'/>" + "</query-profile-type>"; StoringApplicationPackage application = new StoringApplicationPackage(applicationDir, queryProfile, queryProfileType); RankProfileSearchFixture search = fixtureWith("sum(query(mytensor) * attribute(mytensor) * constant(mytensor),d2)", "onnx('mnist_softmax.onnx')", - "constant mytensor { file: ignored\ntype: tensor(d0[7],d1[784]) }", - "field mytensor type tensor(d0[],d1[784]) { indexing: attribute }", + "constant mytensor { file: ignored\ntype: tensor<float>(d0[7],d1[784]) }", + "field mytensor type tensor<float>(d0[],d1[784]) { indexing: attribute }", "Placeholder", application); search.assertFirstPhaseExpression(vespaExpression, "my_profile"); @@ -122,7 +122,7 @@ public class RankingExpressionWithOnnxTestCase { @Test public void testNestedOnnxReference() { - RankProfileSearchFixture search = fixtureWith("tensor(d0[2],d1[784])(0.0)", + RankProfileSearchFixture search = fixtureWith("tensor<float>(d0[2],d1[784])(0.0)", "5 + sum(onnx('mnist_softmax.onnx'))"); search.assertFirstPhaseExpression("5 + reduce(" + vespaExpression + ", sum)", "my_profile"); } @@ -186,7 +186,7 @@ public class RankingExpressionWithOnnxTestCase { @Test public void testImportingFromStoredExpressions() throws IOException { - RankProfileSearchFixture search = fixtureWith("tensor(d0[2],d1[784])(0.0)", + RankProfileSearchFixture search = fixtureWith("tensor<float>(d0[2],d1[784])(0.0)", "onnx('mnist_softmax.onnx')"); search.assertFirstPhaseExpression(vespaExpression, "my_profile"); @@ -197,7 +197,7 @@ public class RankingExpressionWithOnnxTestCase { IOUtils.copyDirectory(applicationDir.append(ApplicationPackage.MODELS_GENERATED_DIR).toFile(), storedApplicationDirectory.append(ApplicationPackage.MODELS_GENERATED_DIR).toFile()); StoringApplicationPackage storedApplication = new StoringApplicationPackage(storedApplicationDirectory); - RankProfileSearchFixture searchFromStored = fixtureWith("tensor(d0[2],d1[784])(0.0)", + RankProfileSearchFixture searchFromStored = fixtureWith("tensor<float>(d0[2],d1[784])(0.0)", "onnx('mnist_softmax.onnx')", null, null, diff --git a/config-model/src/test/java/com/yahoo/vespa/model/ml/MlModelsTest.java b/config-model/src/test/java/com/yahoo/vespa/model/ml/MlModelsTest.java index c19f1a769de..d66f376ed6a 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/ml/MlModelsTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/ml/MlModelsTest.java @@ -63,9 +63,9 @@ public class MlModelsTest { private final String testProfile = "rankingExpression(input).rankingScript: attribute(argument)\n" + - "rankingExpression(input).type: tensor(d0[],d1[784])\n" + + "rankingExpression(input).type: tensor<float>(d0[],d1[784])\n" + "rankingExpression(Placeholder).rankingScript: attribute(argument)\n" + - "rankingExpression(Placeholder).type: tensor(d0[],d1[784])\n" + + "rankingExpression(Placeholder).type: tensor<float>(d0[],d1[784])\n" + "rankingExpression(imported_ml_function_mnist_saved_dnn_hidden1_add).rankingScript: join(reduce(join(rename(rankingExpression(input), (d0, d1), (d0, d4)), constant(mnist_saved_dnn_hidden1_weights_read), f(a,b)(a * b)), sum, d4), constant(mnist_saved_dnn_hidden1_bias_read), f(a,b)(a + b))\n" + "rankingExpression(mnist_tensorflow).rankingScript: join(reduce(join(map(join(reduce(join(join(join(rankingExpression(imported_ml_function_mnist_saved_dnn_hidden1_add), 0.009999999776482582, f(a,b)(a * b)), rankingExpression(imported_ml_function_mnist_saved_dnn_hidden1_add), f(a,b)(max(a,b))), constant(mnist_saved_dnn_hidden2_weights_read), f(a,b)(a * b)), sum, d3), constant(mnist_saved_dnn_hidden2_bias_read), f(a,b)(a + b)), f(a)(1.0507009873554805 * if (a >= 0, a, 1.6732632423543772 * (exp(a) - 1)))), constant(mnist_saved_dnn_outputs_weights_read), f(a,b)(a * b)), sum, d2), constant(mnist_saved_dnn_outputs_bias_read), f(a,b)(a + b))\n" + "rankingExpression(mnist_softmax_tensorflow).rankingScript: join(reduce(join(rename(rankingExpression(Placeholder), (d0, d1), (d0, d2)), constant(mnist_softmax_saved_layer_Variable_read), f(a,b)(a * b)), sum, d2), constant(mnist_softmax_saved_layer_Variable_1_read), f(a,b)(a + b))\n" + @@ -73,6 +73,6 @@ public class MlModelsTest { "rankingExpression(my_xgboost).rankingScript: if (f29 < -0.1234567, if (f56 < -0.242398, 1.71218, -1.70044), if (f109 < 0.8723473, -1.94071, 1.85965)) + if (f60 < -0.482947, if (f29 < -4.2387498, 0.784718, -0.96853), -6.23624)\n" + "vespa.rank.firstphase: rankingExpression(firstphase)\n" + "rankingExpression(firstphase).rankingScript: rankingExpression(mnist_tensorflow) + rankingExpression(mnist_softmax_tensorflow) + rankingExpression(mnist_softmax_onnx) + rankingExpression(my_xgboost)\n" + - "vespa.type.attribute.argument: tensor(d0[],d1[784])\n"; + "vespa.type.attribute.argument: tensor<float>(d0[],d1[784])\n"; } diff --git a/config-provisioning/abi-spec.json b/config-provisioning/abi-spec.json index 33c02811318..c758629e99d 100644 --- a/config-provisioning/abi-spec.json +++ b/config-provisioning/abi-spec.json @@ -621,6 +621,7 @@ "public int hashCode()", "public java.lang.String toString()", "public boolean satisfies(com.yahoo.config.provision.NodeResources)", + "public boolean compatibleWith(com.yahoo.config.provision.NodeResources)", "public static com.yahoo.config.provision.NodeResources fromLegacyName(java.lang.String)" ], "fields": [] diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java index 165b497252d..7e90767c9c5 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java @@ -149,7 +149,7 @@ public class NodeResources { if (this.allocateByLegacyName || other.allocateByLegacyName) // resources are not available return Objects.equals(this.legacyName, other.legacyName); - if (this.vcpu < other.vcpu()) return false; + if (this.vcpu < other.vcpu) return false; if (this.memoryGb < other.memoryGb) return false; if (this.diskGb < other.diskGb) return false; @@ -161,6 +161,19 @@ public class NodeResources { return true; } + /** Returns true if all the resources of this are the same as or compatible with the given resources */ + public boolean compatibleWith(NodeResources other) { + if (this.allocateByLegacyName || other.allocateByLegacyName) // resources are not available + return Objects.equals(this.legacyName, other.legacyName); + + if (this.vcpu != other.vcpu) return false; + if (this.memoryGb != other.memoryGb) return false; + if (this.diskGb != other.diskGb) return false; + if (other.diskSpeed != DiskSpeed.any && other.diskSpeed != this.diskSpeed) return false; + + return true; + } + /** * Create this from serial form. * diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneApi.java b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneApi.java index fd76dc10bdb..08f9b81fec7 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneApi.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneApi.java @@ -17,4 +17,7 @@ public interface ZoneApi { default RegionName getRegionName() { return getId().region(); } CloudName getCloudName(); + + /** Returns the region name within the cloud, e.g. 'us-east-1' in AWS */ + String getCloudNativeRegionName(); } diff --git a/configdefinitions/src/vespa/configserver.def b/configdefinitions/src/vespa/configserver.def index af1ced533ad..e7e626f3d22 100644 --- a/configdefinitions/src/vespa/configserver.def +++ b/configdefinitions/src/vespa/configserver.def @@ -63,5 +63,6 @@ sleepTimeWhenRedeployingFails long default=30 # Features (to be overridden in configserver-config.xml if needed) buildMinimalSetOfConfigModels bool default=true throwIfBootstrappingTenantRepoFails bool default=true +throwIfActiveSessionCannotBeLoaded bool default=true canReturnEmptySentinelConfig bool default=false serverNodeType enum {config, controller} default=config diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java index c37f4aa8401..936b9bdefda 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java @@ -16,8 +16,6 @@ import com.yahoo.vespa.config.server.monitoring.Metrics; import com.yahoo.vespa.config.server.tenant.TenantRepository; import com.yahoo.vespa.config.server.zookeeper.ConfigCurator; import com.yahoo.vespa.curator.Curator; -import com.yahoo.vespa.flags.FlagSource; -import com.yahoo.vespa.flags.Flags; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.recipes.cache.ChildData; import org.apache.curator.framework.recipes.cache.PathChildrenCacheEvent; @@ -46,6 +44,7 @@ public class RemoteSessionRepo extends SessionRepo<RemoteSession> { private static final Logger log = Logger.getLogger(RemoteSessionRepo.class.getName()); + private final GlobalComponentRegistry componentRegistry; private final Curator curator; private final Path sessionsPath; private final RemoteSessionFactory remoteSessionFactory; @@ -53,28 +52,27 @@ public class RemoteSessionRepo extends SessionRepo<RemoteSession> { private final ReloadHandler reloadHandler; private final TenantName tenantName; private final MetricUpdater metrics; - private final FlagSource flagSource; private final Curator.DirectoryCache directoryCache; private final TenantApplications applicationRepo; private final Executor zkWatcherExecutor; - public RemoteSessionRepo(GlobalComponentRegistry registry, + public RemoteSessionRepo(GlobalComponentRegistry componentRegistry, RemoteSessionFactory remoteSessionFactory, ReloadHandler reloadHandler, TenantName tenantName, TenantApplications applicationRepo) { - this.curator = registry.getCurator(); + this.componentRegistry = componentRegistry; + this.curator = componentRegistry.getCurator(); this.sessionsPath = TenantRepository.getSessionsPath(tenantName); this.applicationRepo = applicationRepo; this.remoteSessionFactory = remoteSessionFactory; this.reloadHandler = reloadHandler; this.tenantName = tenantName; - this.metrics = registry.getMetrics().getOrCreateMetricUpdater(Metrics.createDimensions(tenantName)); - this.flagSource = registry.getFlagSource(); - StripedExecutor<TenantName> zkWatcherExecutor = registry.getZkWatcherExecutor(); + this.metrics = componentRegistry.getMetrics().getOrCreateMetricUpdater(Metrics.createDimensions(tenantName)); + StripedExecutor<TenantName> zkWatcherExecutor = componentRegistry.getZkWatcherExecutor(); this.zkWatcherExecutor = command -> zkWatcherExecutor.execute(tenantName, command); initializeSessions(); - this.directoryCache = curator.createDirectoryCache(sessionsPath.getAbsolute(), false, false, registry.getZkCacheExecutor()); + this.directoryCache = curator.createDirectoryCache(sessionsPath.getAbsolute(), false, false, componentRegistry.getZkCacheExecutor()); this.directoryCache.addListener(this::childEvent); this.directoryCache.start(); } @@ -140,8 +138,8 @@ public class RemoteSessionRepo extends SessionRepo<RemoteSession> { * @param sessionId session id for the new session */ private void sessionAdded(long sessionId) { + log.log(LogLevel.DEBUG, () -> "Adding session to RemoteSessionRepo: " + sessionId); try { - log.log(LogLevel.DEBUG, () -> "Adding session to RemoteSessionRepo: " + sessionId); RemoteSession session = remoteSessionFactory.createSession(sessionId); Path sessionPath = sessionsPath.append(String.valueOf(sessionId)); Curator.FileCache fileCache = curator.createFileCache(sessionPath.append(ConfigCurator.SESSIONSTATE_ZK_SUBPATH).getAbsolute(), false); @@ -151,7 +149,7 @@ public class RemoteSessionRepo extends SessionRepo<RemoteSession> { addSession(session); metrics.incAddedSessions(); } catch (Exception e) { - if (Flags.CONFIG_SERVER_FAIL_IF_ACTIVE_SESSION_CANNOT_BE_LOADED.bindTo(flagSource).value()) throw e; + if (componentRegistry.getConfigserverConfig().throwIfActiveSessionCannotBeLoaded()) throw e; log.log(Level.WARNING, "Failed loading session " + sessionId + ": No config for this session can be served", e); } } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4FillInvoker.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4FillInvoker.java index 469ef4864bf..0d7aeb67451 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4FillInvoker.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4FillInvoker.java @@ -152,7 +152,7 @@ public class FS4FillInvoker extends FillInvoker { boolean summaryNeedsQuery = searcher.summaryNeedsQuery(result.getQuery()); if (result.getQuery().getTraceLevel() >= 3) - result.getQuery().trace((summaryNeedsQuery ? "Resending " : "Not resending ") + "query during document summary fetching", 3); + result.getQuery().trace((summaryNeedsQuery ? "FS4: Resending " : "Not resending ") + "query during document summary fetching", 3); GetDocSumsPacket docsumsPacket = GetDocSumsPacket.create(result, summaryClass, summaryNeedsQuery); int compressionLimit = result.getQuery().properties().getInteger(FS4SearchInvoker.PACKET_COMPRESSION_LIMIT, 0); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/SameElementItem.java b/container-search/src/main/java/com/yahoo/prelude/query/SameElementItem.java index d2c19339298..4a13d24d953 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/SameElementItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/SameElementItem.java @@ -50,6 +50,7 @@ public class SameElementItem extends NonReducibleCompositeItem { @Override protected void adding(Item item) { super.adding(item); + //TODO See if we can require only SimpleIndexedItem instead of TermItem Validator.ensureInstanceOf("Child item", item, TermItem.class); TermItem asTerm = (TermItem) item; Validator.ensureNonEmpty("Struct fieldname", asTerm.getIndexName()); @@ -59,7 +60,7 @@ public class SameElementItem extends NonReducibleCompositeItem { @Override public Optional<Item> extractSingleChild() { if (getItemCount() == 1) { - WordItem child = (WordItem) getItem(0); + SimpleIndexedItem child = (SimpleIndexedItem)getItem(0); child.setIndexName(getFieldName() + "." + child.getIndexName()); return Optional.of(child); } diff --git a/container-search/src/main/java/com/yahoo/search/Query.java b/container-search/src/main/java/com/yahoo/search/Query.java index 7105ba8c7ad..8354ed12fb3 100644 --- a/container-search/src/main/java/com/yahoo/search/Query.java +++ b/container-search/src/main/java/com/yahoo/search/Query.java @@ -13,6 +13,8 @@ import com.yahoo.prelude.query.Highlight; import com.yahoo.prelude.query.QueryException; import com.yahoo.prelude.query.textualrepresentation.TextualQueryRepresentation; import com.yahoo.processing.request.CompoundName; +import com.yahoo.search.dispatch.Dispatcher; +import com.yahoo.search.dispatch.rpc.ProtobufSerialization; import com.yahoo.search.federation.FederationSearcher; import com.yahoo.search.query.Model; import com.yahoo.search.query.ParameterParser; @@ -102,7 +104,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { WEB(4,"web"), PROGRAMMATIC(5, "prog"), YQL(6, "yql"), - SELECT(7, "select");; + SELECT(7, "select"); private final int intValue; private final String stringValue; @@ -219,6 +221,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { argumentType.addField(new FieldDescription(Ranking.RANKING, new QueryProfileFieldType(Ranking.getArgumentType()))); argumentType.addField(new FieldDescription(Model.MODEL, new QueryProfileFieldType(Model.getArgumentType()))); argumentType.addField(new FieldDescription(Select.SELECT, new QueryProfileFieldType(Select.getArgumentType()))); + argumentType.addField(new FieldDescription(Dispatcher.DISPATCH, new QueryProfileFieldType(Dispatcher.getArgumentType()))); argumentType.freeze(); } public static QueryProfileType getArgumentType() { return argumentType; } @@ -495,7 +498,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { private void appendQueryProfileProperties(CompiledQueryProfile profile,Set<String> mentioned,StringBuilder b) { for (Map.Entry<String,Object> property : profile.listValues("", requestProperties()).entrySet()) { if ( ! mentioned.contains(property.getKey())) - b.append(property.getKey() + "=" + property.getValue() + " (value from query profile)<br/>\n"); + b.append(property.getKey()).append("=").append(property.getValue()).append(" (value from query profile)<br/>\n"); } } @@ -741,7 +744,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { StringBuilder concatenated = new StringBuilder(); for (Object message : messages) - concatenated.append(String.valueOf(message)); + concatenated.append(message); trace(concatenated.toString(), includeQuery, traceLevel); } @@ -1090,25 +1093,14 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { return Collections.singletonMap("grouping", true); if (ranking.getQueryCache()) return Collections.singletonMap("query", true); - return Collections.<String,Boolean>emptyMap(); - } - - private int computeTraceLevelForBackend() { - int traceLevel = getTraceLevel(); - if (model.getExecution().trace().getForceTimestamps()) { - traceLevel = Math.max(traceLevel, 5); // Backend produces timing information on level 4 and 5 - } - if (getExplainLevel() > 0) { - traceLevel = Math.max(traceLevel, getExplainLevel() + 5); - } - return traceLevel; + return Collections.emptyMap(); } private Map<String, String> createModelMap() { Map<String, String> m = new HashMap<>(); if (model.getSearchPath() != null) m.put("searchpath", model.getSearchPath()); - int traceLevel = computeTraceLevelForBackend(); + int traceLevel = ProtobufSerialization.getTraceLevelForBackend(this); if (traceLevel > 0) m.put("tracelevel", String.valueOf(traceLevel)); return m; diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java index fb587e394d9..a5c6f3650e0 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java @@ -17,6 +17,9 @@ import com.yahoo.search.dispatch.searchcluster.Group; import com.yahoo.search.dispatch.searchcluster.Node; import com.yahoo.search.dispatch.searchcluster.PingFactory; import com.yahoo.search.dispatch.searchcluster.SearchCluster; +import com.yahoo.search.query.profile.types.FieldDescription; +import com.yahoo.search.query.profile.types.FieldType; +import com.yahoo.search.query.profile.types.QueryProfileType; import com.yahoo.search.result.ErrorMessage; import com.yahoo.vespa.config.search.DispatchConfig; @@ -40,16 +43,21 @@ import java.util.Set; * @author ollvir */ public class Dispatcher extends AbstractComponent { + + public static final String DISPATCH = "dispatch"; + private static final String INTERNAL = "internal"; + private static final String PROTOBUF = "protobuf"; + private static final String FDISPATCH_METRIC = "dispatch_fdispatch"; private static final String INTERNAL_METRIC = "dispatch_internal"; private static final int MAX_GROUP_SELECTION_ATTEMPTS = 3; /** If enabled, this internal dispatcher will be preferred over fdispatch whenever possible */ - public static final CompoundName dispatchInternal = new CompoundName("dispatch.internal"); + public static final CompoundName dispatchInternal = CompoundName.fromComponents(DISPATCH, INTERNAL); /** If enabled, search queries will use protobuf rpc */ - public static final CompoundName dispatchProtobuf = new CompoundName("dispatch.protobuf"); + public static final CompoundName dispatchProtobuf = CompoundName.fromComponents(DISPATCH, PROTOBUF); /** A model of the search cluster this dispatches to */ private final SearchCluster searchCluster; @@ -63,8 +71,25 @@ public class Dispatcher extends AbstractComponent { private final Metric metric; private final Metric.Context metricContext; - public static Dispatcher create(String clusterId, DispatchConfig dispatchConfig, FS4ResourcePool fs4ResourcePool, int containerClusterSize, - VipStatus vipStatus, Metric metric) { + private static final QueryProfileType argumentType; + + static { + argumentType = new QueryProfileType(DISPATCH); + argumentType.setStrict(true); + argumentType.setBuiltin(true); + argumentType.addField(new FieldDescription(INTERNAL, FieldType.booleanType)); + argumentType.addField(new FieldDescription(PROTOBUF, FieldType.booleanType)); + argumentType.freeze(); + } + + public static QueryProfileType getArgumentType() { return argumentType; } + + public static Dispatcher create(String clusterId, + DispatchConfig dispatchConfig, + FS4ResourcePool fs4ResourcePool, + int containerClusterSize, + VipStatus vipStatus, + Metric metric) { var searchCluster = new SearchCluster(clusterId, dispatchConfig, containerClusterSize, vipStatus); var rpcFactory = new RpcInvokerFactory(new RpcResourcePool(dispatchConfig), searchCluster, !dispatchConfig.useFdispatchByDefault()); var pingFactory = dispatchConfig.useFdispatchByDefault()? new FS4PingFactory(fs4ResourcePool) : rpcFactory; @@ -72,11 +97,14 @@ public class Dispatcher extends AbstractComponent { return new Dispatcher(searchCluster, dispatchConfig, rpcFactory, pingFactory, metric); } - public Dispatcher(SearchCluster searchCluster, DispatchConfig dispatchConfig, InvokerFactory invokerFactory, PingFactory pingFactory, - Metric metric) { + public Dispatcher(SearchCluster searchCluster, + DispatchConfig dispatchConfig, + InvokerFactory invokerFactory, + PingFactory pingFactory, + Metric metric) { this.searchCluster = searchCluster; this.loadBalancer = new LoadBalancer(searchCluster, - dispatchConfig.distributionPolicy() == DispatchConfig.DistributionPolicy.ROUNDROBIN); + dispatchConfig.distributionPolicy() == DispatchConfig.DistributionPolicy.ROUNDROBIN); this.multilevelDispatch = dispatchConfig.useMultilevelDispatch(); this.internalDispatchByDefault = !dispatchConfig.useFdispatchByDefault(); this.invokerFactory = invokerFactory; @@ -105,14 +133,14 @@ public class Dispatcher extends AbstractComponent { } public Optional<SearchInvoker> getSearchInvoker(Query query, VespaBackEndSearcher searcher) { - if (multilevelDispatch || !query.properties().getBoolean(dispatchInternal, internalDispatchByDefault)) { + if (multilevelDispatch || ! query.properties().getBoolean(dispatchInternal, internalDispatchByDefault)) { emitDispatchMetric(Optional.empty()); return Optional.empty(); } Optional<SearchInvoker> invoker = getSearchPathInvoker(query, searcher); - if (!invoker.isPresent()) { + if (invoker.isEmpty()) { invoker = getInternalInvoker(query, searcher); } if (invoker.isPresent() && query.properties().getBoolean(com.yahoo.search.query.Model.ESTIMATE)) { @@ -128,17 +156,14 @@ public class Dispatcher extends AbstractComponent { // build invoker based on searchpath private Optional<SearchInvoker> getSearchPathInvoker(Query query, VespaBackEndSearcher searcher) { String searchPath = query.getModel().getSearchPath(); - if (searchPath == null) { - return Optional.empty(); - } + if (searchPath == null) return Optional.empty(); + try { List<Node> nodes = SearchPath.selectNodes(searchPath, searchCluster); - if (nodes.isEmpty()) { - return Optional.empty(); - } else { - query.trace(false, 2, "Dispatching internally with search path ", searchPath); - return invokerFactory.createSearchInvoker(searcher, query, OptionalInt.empty(), nodes, true); - } + if (nodes.isEmpty()) return Optional.empty(); + + query.trace(false, 2, "Dispatching internally with search path ", searchPath); + return invokerFactory.createSearchInvoker(searcher, query, OptionalInt.empty(), nodes, true); } catch (InvalidSearchPathException e) { return Optional.of(new SearchErrorInvoker(ErrorMessage.createIllegalQuery(e.getMessage()))); } @@ -158,14 +183,15 @@ public class Dispatcher extends AbstractComponent { Set<Integer> rejected = null; for (int i = 0; i < max; i++) { Optional<Group> groupInCluster = loadBalancer.takeGroup(rejected); - if (!groupInCluster.isPresent()) { - // No groups available - break; - } + if (groupInCluster.isEmpty()) break; // No groups available + Group group = groupInCluster.get(); boolean acceptIncompleteCoverage = (i == max - 1); - Optional<SearchInvoker> invoker = invokerFactory.createSearchInvoker(searcher, query, OptionalInt.of(group.id()), group.nodes(), - acceptIncompleteCoverage); + Optional<SearchInvoker> invoker = invokerFactory.createSearchInvoker(searcher, + query, + OptionalInt.of(group.id()), + group.nodes(), + acceptIncompleteCoverage); if (invoker.isPresent()) { query.trace(false, 2, "Dispatching internally to search group ", group.id()); query.getModel().setSearchPath("/" + group.id()); diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java index 3acbd29f906..0e1565a108b 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java @@ -72,11 +72,24 @@ public class ProtobufSerialization { builder.setCacheGrouping(true); } + builder.setTraceLevel(getTraceLevelForBackend(query)); + mergeToSearchRequestFromRanking(query.getRanking(), builder); return builder.build(); } + public static int getTraceLevelForBackend(Query query) { + int traceLevel = query.getTraceLevel(); + if (query.getModel().getExecution().trace().getForceTimestamps()) { + traceLevel = Math.max(traceLevel, 5); // Backend produces timing information on level 4 and 5 + } + if (query.getExplainLevel() > 0) { + traceLevel = Math.max(traceLevel, query.getExplainLevel() + 5); + } + return traceLevel; + } + private static void mergeToSearchRequestFromRanking(Ranking ranking, SearchProtocol.SearchRequest.Builder builder) { builder.setRankProfile(ranking.getProfile()); @@ -132,6 +145,9 @@ public class ProtobufSerialization { if (includeQueryData) { mergeQueryDataToDocsumRequest(query, builder); } + if (query.getTraceLevel() >= 3) { + query.trace((includeQueryData ? "ProtoBuf: Resending " : "Not resending ") + "query during document summary fetching", 3); + } return builder; } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcFillInvoker.java b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcFillInvoker.java index aa72823c809..0d3f2e7d0a1 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcFillInvoker.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcFillInvoker.java @@ -42,10 +42,8 @@ public class RpcFillInvoker extends FillInvoker { private final DocumentDatabase documentDb; private final RpcResourcePool resourcePool; - private GetDocsumsResponseReceiver responseReceiver; - RpcFillInvoker(RpcResourcePool resourcePool, DocumentDatabase documentDb) { this.documentDb = documentDb; this.resourcePool = resourcePool; @@ -54,12 +52,15 @@ public class RpcFillInvoker extends FillInvoker { @Override protected void sendFillRequest(Result result, String summaryClass) { ListMap<Integer, FastHit> hitsByNode = hitsByNode(result); + Query query = result.getQuery(); CompressionType compression = CompressionType - .valueOf(result.getQuery().properties().getString(RpcResourcePool.dispatchCompression, "LZ4").toUpperCase()); + .valueOf(query.properties().getString(RpcResourcePool.dispatchCompression, "LZ4").toUpperCase()); - if (result.getQuery().getTraceLevel() >= 3) - result.getQuery().trace("Sending " + hitsByNode.size() + " summary fetch RPC requests", 3); + if (query.getTraceLevel() >= 3) { + query.trace("Sending " + hitsByNode.size() + " summary fetch RPC requests", 3); + query.trace("RpcSlime: Not resending query during document summary fetching", 3); + } responseReceiver = new GetDocsumsResponseReceiver(hitsByNode.size(), resourcePool.compressor(), result); for (Map.Entry<Integer, List<FastHit>> nodeHits : hitsByNode.entrySet()) { diff --git a/container-search/src/main/java/com/yahoo/search/query/Model.java b/container-search/src/main/java/com/yahoo/search/query/Model.java index 5ed58a74627..f06aab09a3d 100644 --- a/container-search/src/main/java/com/yahoo/search/query/Model.java +++ b/container-search/src/main/java/com/yahoo/search/query/Model.java @@ -34,6 +34,7 @@ import static com.yahoo.text.Lowercase.toLowerCase; * @author bratseth */ public class Model implements Cloneable { + /** The type representing the property arguments consumed by this */ private static final QueryProfileType argumentType; private static final CompoundName argumentTypeName; @@ -67,7 +68,7 @@ public class Model implements Cloneable { argumentType.addField(new FieldDescription(SEARCH_PATH, "string", "searchpath")); argumentType.addField(new FieldDescription(RESTRICT, "string", "restrict")); argumentType.freeze(); - argumentTypeName=new CompoundName(argumentType.getId().getName()); + argumentTypeName = new CompoundName(argumentType.getId().getName()); } public static QueryProfileType getArgumentType() { return argumentType; } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/types/FieldDescription.java b/container-search/src/main/java/com/yahoo/search/query/profile/types/FieldDescription.java index e8d8ecb02bb..2f4804f0c48 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/types/FieldDescription.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/types/FieldDescription.java @@ -41,7 +41,7 @@ public class FieldDescription implements Comparable<FieldDescription> { } public FieldDescription(String name, String type, String aliases) { - this(name,type,aliases,false,true); + this(name, type, aliases, false, true); } public FieldDescription(String name, FieldType type, String aliases) { diff --git a/container-search/src/test/java/com/yahoo/prelude/query/test/SameElementItemTestCase.java b/container-search/src/test/java/com/yahoo/prelude/query/test/SameElementItemTestCase.java index ae9960f6c7c..1382c106ae3 100644 --- a/container-search/src/test/java/com/yahoo/prelude/query/test/SameElementItemTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/query/test/SameElementItemTestCase.java @@ -2,11 +2,17 @@ package com.yahoo.prelude.query.test; import com.yahoo.prelude.query.AndItem; +import com.yahoo.prelude.query.IntItem; +import com.yahoo.prelude.query.Item; import com.yahoo.prelude.query.SameElementItem; +import com.yahoo.prelude.query.TermItem; import com.yahoo.prelude.query.WordItem; import org.junit.Test; +import java.util.Optional; + import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; public class SameElementItemTestCase { @@ -85,4 +91,27 @@ public class SameElementItemTestCase { } } + private void verifyExtractSingle(TermItem term) { + String subFieldName = term.getIndexName(); + SameElementItem s = new SameElementItem("structa"); + s.addItem(term); + Optional<Item> single =s.extractSingleChild(); + assertTrue(single.isPresent()); + assertEquals(((TermItem)single.get()).getIndexName(), s.getFieldName() + "." + subFieldName); + } + + @Test + public void requireExtractSingleItemToExtractSingles() { + verifyExtractSingle(new WordItem("b", "f1")); + verifyExtractSingle(new IntItem("7", "f1")); + } + + @Test + public void requireExtractSingleItemToExtractSinglesOnly() { + SameElementItem s = new SameElementItem("structa"); + s.addItem(new WordItem("b", "f1")); + s.addItem(new WordItem("c", "f2")); + assertTrue(s.extractSingleChild().isEmpty()); + } + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/AwsEventFetcher.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/AwsEventFetcher.java index d17f046c5ca..05dea1f8567 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/AwsEventFetcher.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/AwsEventFetcher.java @@ -1,11 +1,10 @@ package com.yahoo.vespa.hosted.controller.api.integration.aws; -import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.api.integration.organization.Issue; import java.util.List; public interface AwsEventFetcher { - List<CloudEvent> getEvents(ZoneId zoneId); + List<CloudEvent> getEvents(String awsRegionName); Issue createIssue(CloudEvent event); } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/AwsLimitsFetcher.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/AwsLimitsFetcher.java new file mode 100644 index 00000000000..4e76f67e7cf --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/AwsLimitsFetcher.java @@ -0,0 +1,14 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.api.integration.aws; + +/** + * @author freva + */ +public interface AwsLimitsFetcher { + + /** Returns the AWS EC2 instance limits in the given AWS region */ + Ec2InstanceCounts getEc2InstanceLimits(String awsRegion); + + /** Returns the current usage of AWS EC2 instances in the given AWS region */ + Ec2InstanceCounts getEc2InstanceUsage(String awsRegion); +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/Ec2InstanceCounts.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/Ec2InstanceCounts.java new file mode 100644 index 00000000000..d5e65cfa2bd --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/Ec2InstanceCounts.java @@ -0,0 +1,49 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.api.integration.aws; + +import java.util.Map; +import java.util.Objects; + +/** + * @author freva + */ +public class Ec2InstanceCounts { + private final int totalCount; + private final Map<String, Integer> instanceCounts; + + private Ec2InstanceCounts(int totalCount, Map<String, Integer> instanceCounts) { + this.totalCount = totalCount; + this.instanceCounts = Map.copyOf(instanceCounts); + } + + public int getTotalCount() { + return totalCount; + } + + /** Returns map of counts by instance type, e.g. 'r5.2xlarge' */ + public Map<String, Integer> getInstanceCounts() { + return instanceCounts; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Ec2InstanceCounts that = (Ec2InstanceCounts) o; + return totalCount == that.totalCount && + instanceCounts.equals(that.instanceCounts); + } + + @Override + public int hashCode() { + return Objects.hash(totalCount, instanceCounts); + } + + @Override + public String toString() { + return "Ec2InstanceLimits{" + + "totalLimit=" + totalCount + + ", instanceCounts=" + instanceCounts + + '}'; + } +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/MockAwsEventFetcher.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/MockAwsEventFetcher.java index 73b1942de44..b0b06fc6c83 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/MockAwsEventFetcher.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/MockAwsEventFetcher.java @@ -1,6 +1,5 @@ package com.yahoo.vespa.hosted.controller.api.integration.aws; -import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.api.integration.organization.Issue; import java.util.List; @@ -8,7 +7,7 @@ import java.util.Optional; public class MockAwsEventFetcher implements AwsEventFetcher { @Override - public List<CloudEvent> getEvents(ZoneId zoneId) { + public List<CloudEvent> getEvents(String awsRegionName) { return List.of(); } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/ApplicationCertificate.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/ApplicationCertificate.java index dbcb44d1711..41f5b65d263 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/ApplicationCertificate.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/ApplicationCertificate.java @@ -14,9 +14,10 @@ public class ApplicationCertificate { private final String secretsKeyNamePrefix; public ApplicationCertificate(String secretsKeyNamePrefix) { - this.secretsKeyNamePrefix = secretsKeyNamePrefix; + this.secretsKeyNamePrefix = Objects.requireNonNull(secretsKeyNamePrefix, "secretsKeyNamePrefix must be non-null"); } + /** The prefix of keys identifying this certificate and its private key in a key store */ public String secretsKeyNamePrefix() { return secretsKeyNamePrefix; } @@ -33,4 +34,10 @@ public class ApplicationCertificate { public int hashCode() { return Objects.hash(secretsKeyNamePrefix); } + + @Override + public String toString() { + return "application certificate '" + secretsKeyNamePrefix + "'"; + } + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/ApplicationCertificateProvider.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/ApplicationCertificateProvider.java index fa489a6b754..b3fdee1415c 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/ApplicationCertificateProvider.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/ApplicationCertificateProvider.java @@ -8,5 +8,7 @@ import com.yahoo.config.provision.ApplicationId; * @author andreer */ public interface ApplicationCertificateProvider { + ApplicationCertificate requestCaSignedCertificate(ApplicationId applicationId); + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java index 688cf275892..88ce8c41561 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java @@ -4,15 +4,14 @@ package com.yahoo.vespa.hosted.controller.api.integration.configserver; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics; import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions; import com.yahoo.vespa.hosted.controller.api.application.v4.model.EndpointStatus; -import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.identifiers.Hostname; import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; import com.yahoo.vespa.serviceview.bindings.ApplicationView; -import java.io.IOException; import java.io.InputStream; import java.util.List; import java.util.Map; @@ -55,10 +54,8 @@ public interface ConfigServer { * @param deployment The application/zone pair * @param endpoint The endpoint to modify * @param status The new status with metadata - * @throws IOException If trouble contacting the server */ - // TODO: Remove checked exception from signature - void setGlobalRotationStatus(DeploymentId deployment, String endpoint, EndpointStatus status) throws IOException; + void setGlobalRotationStatus(DeploymentId deployment, String endpoint, EndpointStatus status); /** * Get the endpoint status for an app in one zone @@ -66,25 +63,15 @@ public interface ConfigServer { * @param deployment The application/zone pair * @param endpoint The endpoint to modify * @return The endpoint status with metadata - * @throws IOException If trouble contacting the server */ - // TODO: Remove checked exception from signature - EndpointStatus getGlobalRotationStatus(DeploymentId deployment, String endpoint) throws IOException; + EndpointStatus getGlobalRotationStatus(DeploymentId deployment, String endpoint); /** The node repository on this config server */ NodeRepository nodeRepository(); - /** Get service convergence status for given deployment */ - default Optional<ServiceConvergence> serviceConvergence(DeploymentId deployment) { - return serviceConvergence(deployment, Optional.empty()); - } - /** Get service convergence status for given deployment, using the nodes in the model at the given Vespa version. */ Optional<ServiceConvergence> serviceConvergence(DeploymentId deployment, Optional<Version> version); - /** Get all load balancers in given zone */ - List<LoadBalancer> getLoadBalancers(ZoneId zone); - /** Get all load balancers for application in given zone */ List<LoadBalancer> getLoadBalancers(ApplicationId application, ZoneId zone); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java index ad29c4b2a0e..e006a35a8f0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java @@ -200,13 +200,6 @@ public class Application { .min(Comparator.naturalOrder()); } - /** Returns the global rotation id of this, if present */ - public Optional<RotationId> legacyRotation() { - return rotations.stream() - .map(AssignedRotation::rotationId) - .findFirst(); - } - /** Returns all rotations for this application */ public List<RotationId> rotations() { return rotations.stream() diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index c7f42a4aff2..4ad35f20c39 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -78,8 +78,6 @@ import com.yahoo.vespa.hosted.controller.versions.VespaVersion; import com.yahoo.vespa.hosted.rotation.config.RotationsConfig; import com.yahoo.yolean.Exceptions; -import java.io.IOException; -import java.io.UncheckedIOException; import java.net.URI; import java.security.Principal; import java.time.Clock; @@ -214,8 +212,8 @@ public class ApplicationController { try { configServer.setGlobalRotationStatus(deployment, endpoint.upstreamName(), status); return endpoint; - } catch (IOException e) { - throw new UncheckedIOException("Failed to set rotation status of " + deployment, e); + } catch (Exception e) { + throw new RuntimeException("Failed to set rotation status of " + deployment, e); } }).orElseThrow(() -> new IllegalArgumentException("No global endpoint exists for " + deployment)); } @@ -226,8 +224,8 @@ public class ApplicationController { try { EndpointStatus status = configServer.getGlobalRotationStatus(deployment, endpoint.upstreamName()); return Map.of(endpoint, status); - } catch (IOException e) { - throw new UncheckedIOException("Failed to get rotation status of " + deployment, e); + } catch (Exception e) { + throw new RuntimeException("Failed to get rotation status of " + deployment, e); } }).orElseGet(Collections::emptyMap); } @@ -300,7 +298,7 @@ public class ApplicationController { ApplicationPackage applicationPackage; Set<String> legacyRotations = new LinkedHashSet<>(); Set<ContainerEndpoint> endpoints = new LinkedHashSet<>(); - ApplicationCertificate applicationCertificate; + Optional<ApplicationCertificate> applicationCertificate; try (Lock lock = lock(applicationId)) { LockedApplication application = new LockedApplication(require(applicationId), lock); @@ -367,10 +365,8 @@ public class ApplicationController { app.rotations().stream().map(RotationId::asString).forEach(legacyRotations::add); } - // Get application certificate (provisions a new certificate if missing) - application = withApplicationCertificate(application); - applicationCertificate = application.get().applicationCertificate().orElse(null); + applicationCertificate = getApplicationCertificate(application.get()); // Update application with information from application package if ( ! preferOldestVersion @@ -382,11 +378,13 @@ public class ApplicationController { // Carry out deployment without holding the application lock. options = withVersion(platformVersion, options); - ActivateResult result = deploy(applicationId, applicationPackage, zone, options, legacyRotations, endpoints, applicationCertificate); + ActivateResult result = deploy(applicationId, applicationPackage, zone, options, legacyRotations, endpoints, + applicationCertificate.orElse(null)); lockOrThrow(applicationId, application -> store(application.withNewDeployment(zone, applicationVersion, platformVersion, clock.instant(), - warningsFrom(result)))); + warningsFrom(result)) + .withApplicationCertificate(applicationCertificate))); return result; } } @@ -536,16 +534,18 @@ public class ApplicationController { }); } - private LockedApplication withApplicationCertificate(LockedApplication application) { - ApplicationId applicationId = application.get().id(); - + private Optional<ApplicationCertificate> getApplicationCertificate(Application application) { + // Re-use certificate if already provisioned + if (application.applicationCertificate().isPresent()) { + return application.applicationCertificate(); + } // TODO(tokle): Verify that the application is deploying to a zone where certificate provisioning is enabled - boolean provisionCertificate = provisionApplicationCertificate.with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); - if (provisionCertificate) { - application = application.withApplicationCertificate( - Optional.of(applicationCertificateProvider.requestCaSignedCertificate(applicationId))); + boolean provisionCertificate = provisionApplicationCertificate.with(FetchVector.Dimension.APPLICATION_ID, + application.id().serializedForm()).value(); + if (!provisionCertificate) { + return Optional.empty(); } - return application; + return Optional.of(applicationCertificateProvider.requestCaSignedCertificate(application.id())); } private ActivateResult unexpectedDeployment(ApplicationId application, ZoneId zone) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/AwsEventReporterMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/AwsEventReporterMaintainer.java index 14a39109c61..12bee2a7954 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/AwsEventReporterMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/AwsEventReporterMaintainer.java @@ -1,7 +1,7 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.CloudName; -import com.yahoo.config.provision.zone.ZoneList; +import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.integration.aws.AwsEventFetcher; import com.yahoo.vespa.hosted.controller.api.integration.aws.CloudEvent; @@ -10,8 +10,10 @@ import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueHandl import java.time.Duration; import java.util.List; +import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.stream.Collectors; /** * @author mgimle @@ -22,27 +24,26 @@ public class AwsEventReporterMaintainer extends Maintainer { private final IssueHandler issueHandler; private final AwsEventFetcher eventFetcher; - private final ZoneList cloudZones; + private final Set<String> awsRegions; AwsEventReporterMaintainer(Controller controller, Duration interval, JobControl jobControl, IssueHandler issueHandler, AwsEventFetcher eventFetcher) { super(controller, interval, jobControl); - this.cloudZones = awsZones(controller); this.issueHandler = issueHandler; this.eventFetcher = eventFetcher; - } - - private ZoneList awsZones(Controller controller) { - return controller.zoneRegistry().zones() + this.awsRegions = controller.zoneRegistry().zones() .ofCloud(CloudName.from("aws")) - .reachable(); + .reachable() + .zones().stream() + .map(ZoneApi::getCloudNativeRegionName) + .collect(Collectors.toSet()); } @Override protected void maintain() { log.log(Level.INFO, "Fetching events for cloud hosts."); - for (var cloudZoneId : cloudZones.ids()) { - List<CloudEvent> events = eventFetcher.getEvents(cloudZoneId); + for (var awsRegion : awsRegions) { + List<CloudEvent> events = eventFetcher.getEvents(awsRegion); for (var event : events) { Issue issue = eventFetcher.createIssue(event); if (!issueHandler.issueExists(issue)) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java index b9581d63583..28474b1181a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java @@ -86,8 +86,6 @@ public class ApplicationSerializer { private final String assignedRotationEndpointField = "endpointId"; private final String assignedRotationClusterField = "clusterId"; private final String assignedRotationRotationField = "rotationId"; - private final String rotationsField = "endpoints"; - private final String deprecatedRotationField = "rotation"; private final String rotationStatusField = "rotationStatus"; private final String applicationCertificateField = "applicationCertificate"; @@ -109,7 +107,6 @@ public class ApplicationSerializer { private final String lastWrittenField = "lastWritten"; private final String lastQueriesPerSecondField = "lastQueriesPerSecond"; private final String lastWritesPerSecondField = "lastWritesPerSecond"; - private final String clusterMetricsField = "clusterMetrics"; // DeploymentJobs fields private final String projectIdField = "projectId"; @@ -181,8 +178,6 @@ public class ApplicationSerializer { root.setDouble(queryQualityField, application.metrics().queryServiceQuality()); root.setDouble(writeQualityField, application.metrics().writeServiceQuality()); application.pemDeployKey().ifPresent(pemDeployKey -> root.setString(pemDeployKeyField, pemDeployKey)); - application.legacyRotation().ifPresent(rotation -> root.setString(deprecatedRotationField, rotation.asString())); - rotationsToSlime(application.assignedRotations(), root, rotationsField); assignedRotationsToSlime(application.assignedRotations(), root, assignedRotationsField); toSlime(application.rotationStatus(), root.setArray(rotationStatusField)); application.applicationCertificate().ifPresent(cert -> root.setString(applicationCertificateField, cert.secretsKeyNamePrefix())); @@ -333,14 +328,14 @@ public class ApplicationSerializer { } private void rotationsToSlime(List<AssignedRotation> rotations, Cursor parent, String fieldName) { - final var rotationsArray = parent.setArray(fieldName); + var rotationsArray = parent.setArray(fieldName); rotations.forEach(rot -> rotationsArray.addString(rot.rotationId().asString())); } private void assignedRotationsToSlime(List<AssignedRotation> rotations, Cursor parent, String fieldName) { - final var rotationsArray = parent.setArray(fieldName); + var rotationsArray = parent.setArray(fieldName); for (var rotation : rotations) { - final var object = rotationsArray.addObject(); + var object = rotationsArray.addObject(); object.setString(assignedRotationEndpointField, rotation.endpointId().id()); object.setString(assignedRotationRotationField, rotation.rotationId().asString()); object.setString(assignedRotationClusterField, rotation.clusterId().value()); @@ -470,17 +465,17 @@ public class ApplicationSerializer { if ( ! object.valid()) return ApplicationVersion.unknown; OptionalLong applicationBuildNumber = optionalLong(object.field(applicationBuildNumberField)); Optional<SourceRevision> sourceRevision = sourceRevisionFromSlime(object.field(sourceRevisionField)); - if ( ! sourceRevision.isPresent() || ! applicationBuildNumber.isPresent()) { + if (sourceRevision.isEmpty() || applicationBuildNumber.isEmpty()) { return ApplicationVersion.unknown; } Optional<String> authorEmail = optionalString(object.field(authorEmailField)); Optional<Version> compileVersion = optionalString(object.field(compileVersionField)).map(Version::fromString); Optional<Instant> buildTime = optionalInstant(object.field(buildTimeField)); - if ( ! authorEmail.isPresent()) + if (authorEmail.isEmpty()) return ApplicationVersion.from(sourceRevision.get(), applicationBuildNumber.getAsLong()); - if ( ! compileVersion.isPresent() || ! buildTime.isPresent()) + if (compileVersion.isEmpty() || buildTime.isEmpty()) return ApplicationVersion.from(sourceRevision.get(), applicationBuildNumber.getAsLong(), authorEmail.get()); return ApplicationVersion.from(sourceRevision.get(), applicationBuildNumber.getAsLong(), authorEmail.get(), @@ -526,7 +521,7 @@ public class ApplicationSerializer { // if the job type has since been removed, ignore it Optional<JobType> jobType = JobType.fromOptionalJobName(object.field(jobTypeField).asString()); - if (! jobType.isPresent()) return Optional.empty(); + if (jobType.isEmpty()) return Optional.empty(); Optional<JobError> jobError = Optional.empty(); if (object.field(errorField).valid()) @@ -553,16 +548,16 @@ public class ApplicationSerializer { } private List<AssignedRotation> assignedRotationsFromSlime(DeploymentSpec deploymentSpec, Inspector root) { - final var assignedRotations = new LinkedHashMap<EndpointId, AssignedRotation>(); + var assignedRotations = new LinkedHashMap<EndpointId, AssignedRotation>(); root.field(assignedRotationsField).traverse((ArrayTraverser) (idx, inspector) -> { - final var clusterId = new ClusterSpec.Id(inspector.field(assignedRotationClusterField).asString()); - final var endpointId = EndpointId.of(inspector.field(assignedRotationEndpointField).asString()); - final var rotationId = new RotationId(inspector.field(assignedRotationRotationField).asString()); - final var regions = deploymentSpec.endpoints().stream() - .filter(endpoint -> endpoint.endpointId().equals(endpointId.id())) - .flatMap(endpoint -> endpoint.regions().stream()) - .collect(Collectors.toSet()); + var clusterId = new ClusterSpec.Id(inspector.field(assignedRotationClusterField).asString()); + var endpointId = EndpointId.of(inspector.field(assignedRotationEndpointField).asString()); + var rotationId = new RotationId(inspector.field(assignedRotationRotationField).asString()); + var regions = deploymentSpec.endpoints().stream() + .filter(endpoint -> endpoint.endpointId().equals(endpointId.id())) + .flatMap(endpoint -> endpoint.regions().stream()) + .collect(Collectors.toSet()); assignedRotations.putIfAbsent(endpointId, new AssignedRotation(clusterId, endpointId, rotationId, regions)); }); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index f6135cd246c..f226fe9e4e3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -19,6 +19,7 @@ import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions; import com.yahoo.vespa.hosted.controller.api.application.v4.model.EndpointStatus; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; @@ -703,6 +704,36 @@ public class ControllerTest { .metrics().warnings().get(DeploymentMetrics.Warning.all).intValue()); } + @Test + public void testDeployProvisionsCertificate() { + ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.PROVISION_APPLICATION_CERTIFICATE.id(), true); + Function<Application, Optional<ApplicationCertificate>> certificate = (application) -> tester.application(application.id()).applicationCertificate(); + + // Create app1 + var app1 = tester.createApplication("app1", "tenant1", 1, 2L); + var applicationPackage = new ApplicationPackageBuilder().environment(Environment.prod) + .region("us-west-1") + .build(); + // Deploy app1 in production + tester.deployCompletely(app1, applicationPackage); + var cert = certificate.apply(app1); + assertTrue("Provisions certificate in " + Environment.prod, cert.isPresent()); + + // Next deployment reuses certificate + tester.deployCompletely(app1, applicationPackage, BuildJob.defaultBuildNumber + 1); + assertEquals(cert, certificate.apply(app1)); + + // Create app2 + var app2 = tester.createApplication("app2", "tenant2", 3, 4L); + ZoneId zone = ZoneId.from("dev", "us-east-1"); + + // Deploy app2 in dev + tester.controller().applications().deploy(app2.id(), zone, Optional.of(applicationPackage), DeployOptions.none()); + assertTrue("Application deployed and activated", + tester.controllerTester().configServer().application(app2.id()).get().activated()); + assertTrue("Provisions certificate in " + Environment.dev, certificate.apply(app2).isPresent()); + } + private void runUpgrade(DeploymentTester tester, ApplicationId application, ApplicationVersion version) { Version next = Version.fromString("6.2"); tester.upgradeSystem(next); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationCertificateMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationCertificateMock.java index 3246a260217..f3bee70db4c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationCertificateMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationCertificateMock.java @@ -5,10 +5,18 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificateProvider; +import java.util.UUID; + +/** + * @author tokle + */ public class ApplicationCertificateMock implements ApplicationCertificateProvider { @Override public ApplicationCertificate requestCaSignedCertificate(ApplicationId applicationId) { - return new ApplicationCertificate(String.format("vespa.tls.%s.%s", applicationId.tenant(),applicationId.application())); + return new ApplicationCertificate(String.format("vespa.tls.%s.%s@%s", applicationId.tenant(), + applicationId.application(), + UUID.randomUUID().toString())); } + } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index fbc7bf20a24..b692500e40f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -205,8 +205,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer disallowConvergenceCheckApplications.add(applicationId); } - @Override - public List<LoadBalancer> getLoadBalancers(ZoneId zone) { + private List<LoadBalancer> getLoadBalancers(ZoneId zone) { return loadBalancers.getOrDefault(zone, Collections.emptyList()); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneApiMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneApiMock.java index 4705982e1f2..269bdcc5dca 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneApiMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneApiMock.java @@ -1,15 +1,12 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.integration; -import com.yahoo.cloud.config.SentinelConfig; -import com.yahoo.config.model.graph.ModelGraphBuilder; import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.config.provision.zone.ZoneId; -import com.yahoo.messagebus.MessagebusConfig; /** * @author hakonhall @@ -18,13 +15,15 @@ public class ZoneApiMock implements ZoneApi { private final SystemName systemName; private final ZoneId id; private final CloudName cloudName; + private final String cloudNativeRegionName; public static Builder newBuilder() { return new Builder(); } - private ZoneApiMock(SystemName systemName, ZoneId id, CloudName cloudName) { + private ZoneApiMock(SystemName systemName, ZoneId id, CloudName cloudName, String cloudNativeRegionName) { this.systemName = systemName; this.id = id; this.cloudName = cloudName; + this.cloudNativeRegionName = cloudNativeRegionName; } public static ZoneApiMock fromId(String id) { @@ -44,10 +43,14 @@ public class ZoneApiMock implements ZoneApi { @Override public CloudName getCloudName() { return cloudName; } + @Override + public String getCloudNativeRegionName() { return cloudNativeRegionName; } + public static class Builder { private SystemName systemName = SystemName.defaultSystem(); private ZoneId id = ZoneId.defaultId(); private CloudName cloudName = CloudName.defaultName(); + private String cloudNativeRegionName = id.region().value(); public Builder with(ZoneId id) { this.id = id; @@ -63,8 +66,13 @@ public class ZoneApiMock implements ZoneApi { public Builder withCloud(String cloud) { return with(CloudName.from(cloud)); } + public Builder withCloudNativeRegionName(String cloudRegionName) { + this.cloudNativeRegionName = cloudRegionName; + return this; + } + public ZoneApiMock build() { - return new ZoneApiMock(systemName, id, cloudName); + return new ZoneApiMock(systemName, id, cloudName, cloudNativeRegionName); } } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java index fb701746d77..ac7dba2b110 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java @@ -282,9 +282,7 @@ public class ApplicationSerializerTest { application.rotations() ); - assertEquals( - Optional.of(new RotationId("assigned-rotation")), application.legacyRotation() - ); + assertEquals(new RotationId("assigned-rotation"), application.rotations().get(0)); assertEquals( List.of( diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index f2955e60f48..a665c6d8a20 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -82,7 +82,7 @@ public class Flags { NODE_TYPE, APPLICATION_ID, HOSTNAME); public static final UnboundBooleanFlag USE_FDISPATCH_BY_DEFAULT = defineFeatureFlag( - "use-fdispatch-by-default", false, + "use-fdispatch-by-default", true, "Should fdispatch be used as the default instead of the java dispatcher", "Takes effect at redeployment", APPLICATION_ID); @@ -94,7 +94,7 @@ public class Flags { APPLICATION_ID); public static final UnboundBooleanFlag DISPATCH_WITH_PROTOBUF = defineFeatureFlag( - "dispatch-with-protobuf", true, + "dispatch-with-protobuf", false, "Should the java dispatcher use protobuf/jrt as the default", "Takes effect at redeployment", APPLICATION_ID); @@ -128,12 +128,6 @@ public class Flags { "Takes effect at redeployment", APPLICATION_ID); - public static final UnboundBooleanFlag CONFIG_SERVER_FAIL_IF_ACTIVE_SESSION_CANNOT_BE_LOADED = defineFeatureFlag( - "config-server-fail-if-active-session-cannot-be-loaded", false, - "Whether to fail or just log if loading an active session fails at startup of config server", - "Takes effect only at bootstrap of config server/controller", - HOSTNAME); - public static final UnboundStringFlag CONFIGSERVER_RPC_AUTHORIZER = defineStringFlag( "configserver-rpc-authorizer", "enforce", "Configserver RPC authorizer. Allowed values: ['disable', 'log-only', 'enforce']", diff --git a/jdisc_http_service/abi-spec.json b/jdisc_http_service/abi-spec.json index b06250c4593..6e04ba741e6 100644 --- a/jdisc_http_service/abi-spec.json +++ b/jdisc_http_service/abi-spec.json @@ -39,6 +39,7 @@ "public com.yahoo.jdisc.http.ConnectorConfig$Builder tcpNoDelay(boolean)", "public com.yahoo.jdisc.http.ConnectorConfig$Builder throttling(com.yahoo.jdisc.http.ConnectorConfig$Throttling$Builder)", "public com.yahoo.jdisc.http.ConnectorConfig$Builder ssl(com.yahoo.jdisc.http.ConnectorConfig$Ssl$Builder)", + "public com.yahoo.jdisc.http.ConnectorConfig$Builder tlsClientAuthEnforcer(com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder)", "public final boolean dispatchGetConfig(com.yahoo.config.ConfigInstance$Producer)", "public final java.lang.String getDefMd5()", "public final java.lang.String getDefName()", @@ -47,7 +48,8 @@ ], "fields": [ "public com.yahoo.jdisc.http.ConnectorConfig$Throttling$Builder throttling", - "public com.yahoo.jdisc.http.ConnectorConfig$Ssl$Builder ssl" + "public com.yahoo.jdisc.http.ConnectorConfig$Ssl$Builder ssl", + "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder tlsClientAuthEnforcer" ] }, "com.yahoo.jdisc.http.ConnectorConfig$Producer": { @@ -180,6 +182,41 @@ ], "fields": [] }, + "com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder": { + "superClass": "java.lang.Object", + "interfaces": [ + "com.yahoo.config.ConfigBuilder" + ], + "attributes": [ + "public" + ], + "methods": [ + "public void <init>()", + "public void <init>(com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer)", + "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder enable(boolean)", + "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder pathWhitelist(java.lang.String)", + "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder pathWhitelist(java.util.Collection)", + "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer build()" + ], + "fields": [ + "public java.util.List pathWhitelist" + ] + }, + "com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer": { + "superClass": "com.yahoo.config.InnerNode", + "interfaces": [], + "attributes": [ + "public", + "final" + ], + "methods": [ + "public void <init>(com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder)", + "public boolean enable()", + "public java.util.List pathWhitelist()", + "public java.lang.String pathWhitelist(int)" + ], + "fields": [] + }, "com.yahoo.jdisc.http.ConnectorConfig": { "superClass": "com.yahoo.config.ConfigInstance", "interfaces": [], @@ -206,7 +243,8 @@ "public boolean tcpKeepAliveEnabled()", "public boolean tcpNoDelay()", "public com.yahoo.jdisc.http.ConnectorConfig$Throttling throttling()", - "public com.yahoo.jdisc.http.ConnectorConfig$Ssl ssl()" + "public com.yahoo.jdisc.http.ConnectorConfig$Ssl ssl()", + "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer tlsClientAuthEnforcer()" ], "fields": [ "public static final java.lang.String CONFIG_DEF_MD5", diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java index 30a1b1d885c..2b9cb426dda 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java @@ -9,6 +9,7 @@ import com.yahoo.component.provider.ComponentRegistry; import com.yahoo.container.logging.AccessLog; import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.application.OsgiFramework; +import com.yahoo.jdisc.http.ConnectorConfig; import com.yahoo.jdisc.http.ServerConfig; import com.yahoo.jdisc.http.ServletPathsConfig; import com.yahoo.jdisc.http.server.FilterBindings; @@ -145,10 +146,13 @@ public class JettyHttpServer extends AbstractServerProvider { setupJmx(server, serverConfig); ((QueuedThreadPool)server.getThreadPool()).setMaxThreads(serverConfig.maxWorkerThreads()); + List<ConnectorConfig> connectorConfigs = new ArrayList<>(); for (ConnectorFactory connectorFactory : connectorFactories.allComponents()) { - ServerSocketChannel preBoundChannel = getChannelFromServiceLayer(connectorFactory.getConnectorConfig().listenPort(), osgiFramework.bundleContext()); + ConnectorConfig connectorConfig = connectorFactory.getConnectorConfig(); + connectorConfigs.add(connectorConfig); + ServerSocketChannel preBoundChannel = getChannelFromServiceLayer(connectorConfig.listenPort(), osgiFramework.bundleContext()); server.addConnector(connectorFactory.createConnector(metric, server, preBoundChannel)); - listenedPorts.add(connectorFactory.getConnectorConfig().listenPort()); + listenedPorts.add(connectorConfig.listenPort()); } janitor = newJanitor(threadFactory); @@ -168,6 +172,7 @@ public class JettyHttpServer extends AbstractServerProvider { getHandlerCollection( serverConfig, servletPathsConfig, + connectorConfigs, jdiscServlet, servletHolders, jDiscFilterInvokerFilter)); @@ -217,6 +222,7 @@ public class JettyHttpServer extends AbstractServerProvider { private HandlerCollection getHandlerCollection( ServerConfig serverConfig, ServletPathsConfig servletPathsConfig, + List<ConnectorConfig> connectorConfigs, ServletHolder jdiscServlet, ComponentRegistry<ServletHolder> servletHolders, FilterHolder jDiscFilterInvokerFilter) { @@ -231,8 +237,11 @@ public class JettyHttpServer extends AbstractServerProvider { servletContextHandler.addServlet(jdiscServlet, "/*"); + var authEnforcer = new TlsClientAuthenticationEnforcer(connectorConfigs); + authEnforcer.setHandler(servletContextHandler); + GzipHandler gzipHandler = newGzipHandler(serverConfig); - gzipHandler.setHandler(servletContextHandler); + gzipHandler.setHandler(authEnforcer); HttpResponseStatisticsCollector statisticsCollector = new HttpResponseStatisticsCollector(); statisticsCollector.setHandler(gzipHandler); diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/TlsClientAuthenticationEnforcer.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/TlsClientAuthenticationEnforcer.java new file mode 100644 index 00000000000..546741b3322 --- /dev/null +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/TlsClientAuthenticationEnforcer.java @@ -0,0 +1,78 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.http.server.jetty; + +import com.yahoo.jdisc.Response; +import com.yahoo.jdisc.http.ConnectorConfig; +import com.yahoo.jdisc.http.servlet.ServletRequest; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.handler.HandlerWrapper; + +import javax.servlet.DispatcherType; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** + * A Jetty handler that enforces TLS client authentication with configurable white list. + * + * @author bjorncs + */ +class TlsClientAuthenticationEnforcer extends HandlerWrapper { + + private final Map<Integer, List<String>> portToWhitelistedPathsMapping; + + TlsClientAuthenticationEnforcer(List<ConnectorConfig> connectorConfigs) { + portToWhitelistedPathsMapping = createWhitelistMapping(connectorConfigs); + } + + @Override + public void handle(String target, Request request, HttpServletRequest servletRequest, HttpServletResponse servletResponse) throws IOException, ServletException { + if (isHttpsRequest(request) + && !isRequestToWhitelistedBinding(servletRequest) + && !isClientAuthenticated(servletRequest)) { + servletResponse.sendError(Response.Status.UNAUTHORIZED, "Client did not present a x509 certificate."); + } else { + _handler.handle(target, request, servletRequest, servletResponse); + } + } + + private static Map<Integer, List<String>> createWhitelistMapping(List<ConnectorConfig> connectorConfigs) { + var mapping = new HashMap<Integer, List<String>>(); + for (ConnectorConfig connectorConfig : connectorConfigs) { + var enforcerConfig = connectorConfig.tlsClientAuthEnforcer(); + if (enforcerConfig.enable()) { + mapping.put(connectorConfig.listenPort(), enforcerConfig.pathWhitelist()); + } + } + return mapping; + } + + private boolean isHttpsRequest(Request request) { + return request.getDispatcherType() == DispatcherType.REQUEST && request.getScheme().equalsIgnoreCase("https"); + } + + private boolean isRequestToWhitelistedBinding(HttpServletRequest servletRequest) { + int localPort = servletRequest.getLocalPort(); + List<String> whiteListedPaths = getWhitelistedPathsForPort(localPort); + if (whiteListedPaths == null) { + return true; // enforcer not enabled + } + // Note: Same path definition as HttpRequestFactory.getUri() + return whiteListedPaths.contains(servletRequest.getRequestURI()); + } + + private List<String> getWhitelistedPathsForPort(int localPort) { + if (portToWhitelistedPathsMapping.containsKey(0) && portToWhitelistedPathsMapping.size() == 1) { + return portToWhitelistedPathsMapping.get(0); // for unit tests which uses 0 for listen port + } + return portToWhitelistedPathsMapping.get(localPort); + } + + private boolean isClientAuthenticated(HttpServletRequest servletRequest) { + return servletRequest.getAttribute(ServletRequest.SERVLET_REQUEST_X509CERT) != null; + } +} diff --git a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def index 9b6fb5401e2..f02a0d7b4a3 100644 --- a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def +++ b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def @@ -80,3 +80,11 @@ ssl.caCertificate string default="" # Client authentication mode. See SSLEngine.getNeedClientAuth()/getWantClientAuth() for details. ssl.clientAuth enum { DISABLED, WANT_AUTH, NEED_AUTH } default=DISABLED + +# Enforce TLS client authentication for https requests at the http layer. +# Intended to be used with connectors with optional client authentication enabled. +# 401 status code is returned for requests from non-authenticated clients. +tlsClientAuthEnforcer.enable bool default=false + +# Paths where client authentication should not be enforced. To be used in combination with WANT_AUTH. Typically used for health checks. +tlsClientAuthEnforcer.pathWhitelist[] string diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java index ec9c90ffa50..31ecf3ca2fc 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java @@ -23,6 +23,7 @@ import com.yahoo.jdisc.http.HttpResponse; import com.yahoo.jdisc.http.ServerConfig; import com.yahoo.jdisc.service.BindingSetNotFoundException; import com.yahoo.security.KeyUtils; +import com.yahoo.security.SslContextBuilder; import com.yahoo.security.X509CertificateBuilder; import com.yahoo.security.X509CertificateUtils; import org.apache.http.entity.ContentType; @@ -32,7 +33,9 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import javax.net.ssl.SSLContext; import javax.security.auth.x500.X500Principal; +import java.io.IOException; import java.math.BigInteger; import java.net.BindException; import java.net.URI; @@ -58,6 +61,7 @@ import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR; import static com.yahoo.jdisc.Response.Status.NOT_FOUND; import static com.yahoo.jdisc.Response.Status.OK; import static com.yahoo.jdisc.Response.Status.REQUEST_URI_TOO_LONG; +import static com.yahoo.jdisc.Response.Status.UNAUTHORIZED; import static com.yahoo.jdisc.Response.Status.UNSUPPORTED_MEDIA_TYPE; import static com.yahoo.jdisc.http.HttpHeaders.Names.CONNECTION; import static com.yahoo.jdisc.http.HttpHeaders.Names.CONTENT_TYPE; @@ -470,16 +474,9 @@ public class HttpServerTest { @Test public void requireThatServerCanRespondToSslRequest() throws Exception { - KeyPair keyPair = KeyUtils.generateKeypair(RSA, 2048); Path privateKeyFile = tmpFolder.newFile().toPath(); - Files.writeString(privateKeyFile, KeyUtils.toPem(keyPair.getPrivate())); - - X509Certificate certificate = X509CertificateBuilder - .fromKeypair( - keyPair, new X500Principal("CN=localhost"), Instant.EPOCH, Instant.EPOCH.plus(100_000, ChronoUnit.DAYS), SHA256_WITH_RSA, BigInteger.ONE) - .build(); Path certificateFile = tmpFolder.newFile().toPath(); - Files.writeString(certificateFile, X509CertificateUtils.toPem(certificate)); + generatePrivateKeyAndCertificate(privateKeyFile, certificateFile); final TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile); driver.client().get("/status.html") @@ -488,6 +485,42 @@ public class HttpServerTest { } @Test + public void requireThatTlsClientAuthenticationEnforcerRejectsRequestsForNonWhitelistedPaths() throws IOException { + Path privateKeyFile = tmpFolder.newFile().toPath(); + Path certificateFile = tmpFolder.newFile().toPath(); + generatePrivateKeyAndCertificate(privateKeyFile, certificateFile); + TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile); + + SSLContext trustStoreOnlyCtx = new SslContextBuilder() + .withTrustStore(certificateFile) + .build(); + + new SimpleHttpClient(trustStoreOnlyCtx, driver.server().getListenPort(), false) + .get("/dummy.html") + .expectStatusCode(is(UNAUTHORIZED)); + + assertThat(driver.close(), is(true)); + } + + @Test + public void requireThatTlsClientAuthenticationEnforcerAllowsRequestForWhitelistedPaths() throws IOException { + Path privateKeyFile = tmpFolder.newFile().toPath(); + Path certificateFile = tmpFolder.newFile().toPath(); + generatePrivateKeyAndCertificate(privateKeyFile, certificateFile); + TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile); + + SSLContext trustStoreOnlyCtx = new SslContextBuilder() + .withTrustStore(certificateFile) + .build(); + + new SimpleHttpClient(trustStoreOnlyCtx, driver.server().getListenPort(), false) + .get("/status.html") + .expectStatusCode(is(OK)); + + assertThat(driver.close(), is(true)); + } + + @Test public void requireThatConnectedAtReturnsNonZero() throws Exception { final TestDriver driver = TestDrivers.newInstance(new ConnectedAtRequestHandler()); driver.client().get("/status.html") @@ -526,6 +559,17 @@ public class HttpServerTest { assertThat(driver.close(), is(true)); } + private static void generatePrivateKeyAndCertificate(Path privateKeyFile, Path certificateFile) throws IOException { + KeyPair keyPair = KeyUtils.generateKeypair(RSA, 2048); + Files.writeString(privateKeyFile, KeyUtils.toPem(keyPair.getPrivate())); + + X509Certificate certificate = X509CertificateBuilder + .fromKeypair( + keyPair, new X500Principal("CN=localhost"), Instant.EPOCH, Instant.EPOCH.plus(100_000, ChronoUnit.DAYS), SHA256_WITH_RSA, BigInteger.ONE) + .build(); + Files.writeString(certificateFile, X509CertificateUtils.toPem(certificate)); + } + private static RequestHandler mockRequestHandler() { final RequestHandler mockRequestHandler = mock(RequestHandler.class); when(mockRequestHandler.refer()).thenReturn(References.NOOP_REFERENCE); diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java index 10fe0f1328f..e0933ac485e 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java @@ -55,8 +55,13 @@ public class TestDrivers { newConfigModule( new ServerConfig.Builder(), new ConnectorConfig.Builder() + .tlsClientAuthEnforcer( + new ConnectorConfig.TlsClientAuthEnforcer.Builder() + .enable(true) + .pathWhitelist("/status.html")) .ssl(new ConnectorConfig.Ssl.Builder() .enabled(true) + .clientAuth(ConnectorConfig.Ssl.ClientAuth.Enum.WANT_AUTH) .privateKeyFile(privateKeyFile.toString()) .certificateFile(certificateFile.toString()) .caCertificateFile(certificateFile.toString())), diff --git a/model-integration/src/test/java/ai/vespa/rankingexpression/importer/onnx/OnnxMnistSoftmaxImportTestCase.java b/model-integration/src/test/java/ai/vespa/rankingexpression/importer/onnx/OnnxMnistSoftmaxImportTestCase.java index 424e4d6c57c..68df59bf93f 100644 --- a/model-integration/src/test/java/ai/vespa/rankingexpression/importer/onnx/OnnxMnistSoftmaxImportTestCase.java +++ b/model-integration/src/test/java/ai/vespa/rankingexpression/importer/onnx/OnnxMnistSoftmaxImportTestCase.java @@ -31,13 +31,13 @@ public class OnnxMnistSoftmaxImportTestCase { Tensor constant0 = Tensor.from(model.largeConstants().get("test_Variable")); assertNotNull(constant0); - assertEquals(new TensorType.Builder().indexed("d2", 784).indexed("d1", 10).build(), + assertEquals(new TensorType.Builder(TensorType.Value.DOUBLE).indexed("d2", 784).indexed("d1", 10).build(), constant0.type()); assertEquals(7840, constant0.size()); Tensor constant1 = Tensor.from(model.largeConstants().get("test_Variable_1")); assertNotNull(constant1); - assertEquals(new TensorType.Builder().indexed("d1", 10).build(), constant1.type()); + assertEquals(new TensorType.Builder(TensorType.Value.DOUBLE).indexed("d1", 10).build(), constant1.type()); assertEquals(10, constant1.size()); // Check inputs diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java index e20480e14ef..b2769ee40c4 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java @@ -254,6 +254,11 @@ public class NodeAgentContextImpl implements NodeAgentContext { public CloudName getCloudName() { return CloudName.defaultName(); } + + @Override + public String getCloudNativeRegionName() { + return getId().region().value(); + } }), Optional.ofNullable(pathToContainerStorage).orElseGet(() -> Paths.get("/home/docker")), Optional.ofNullable(pathToVespaHome).orElseGet(() -> Paths.get("/opt/vespa")), diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java index ab7a565688e..0a8556530e0 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java @@ -10,9 +10,11 @@ import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.History; import java.time.Duration; +import java.time.Instant; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -40,11 +42,7 @@ public class OperatorChangeApplicationMaintainer extends ApplicationMaintainer { .collect(Collectors.groupingBy(node -> node.allocation().get().owner(), Collectors.toList())); return nodesByApplication.entrySet().stream() - .filter(entry -> entry.getValue().stream() - .flatMap(node -> node.history().events().stream()) - .filter(event -> event.agent() == Agent.operator) - .map(History.Event::at) - .anyMatch(getLastDeployTime(entry.getKey())::isBefore)) + .filter(entry -> hasNodesWithChanges(entry.getKey(), entry.getValue())) .map(Map.Entry::getKey) .collect(Collectors.toCollection(LinkedHashSet::new)); } @@ -60,4 +58,15 @@ public class OperatorChangeApplicationMaintainer extends ApplicationMaintainer { " as a manual change was made to its nodes"); } + private boolean hasNodesWithChanges(ApplicationId applicationId, List<Node> nodes) { + Optional<Instant> lastDeployTime = deployer().lastDeployTime(applicationId); + if (lastDeployTime.isEmpty()) return false; + + return nodes.stream() + .flatMap(node -> node.history().events().stream()) + .filter(event -> event.agent() == Agent.operator) + .map(History.Event::at) + .anyMatch(e -> lastDeployTime.get().isBefore(e)); + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java index 5c2c78cef5d..66a8f2f8f6d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java @@ -1,10 +1,10 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.provisioning; -import com.yahoo.config.provision.NodeResources; +import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.NodeFlavors; +import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; -import com.yahoo.config.provision.Flavor; import com.yahoo.vespa.hosted.provision.Node; import java.util.Objects; @@ -98,7 +98,7 @@ public interface NodeSpec { } else { if (flavor.isDocker()) { // Docker nodes can satisfy a request for parts of their resources - if (flavor.resources().satisfies(requestedNodeResources)) + if (flavor.resources().compatibleWith(requestedNodeResources)) return true; } else { // Other nodes must be matched exactly diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveAndFailedExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveAndFailedExpirerTest.java index ae886e0babc..8692c551a2c 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveAndFailedExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveAndFailedExpirerTest.java @@ -139,7 +139,7 @@ public class InactiveAndFailedExpirerTest { { Node toRetire = tester.getNodes(applicationId, Node.State.active).asList().get(0); tester.patchNode(toRetire.with(toRetire.status().withWantToRetire(true))); - List<HostSpec> hostSpecs = tester.prepare(applicationId, cluster, Capacity.fromNodeCount(2), 1); + List<HostSpec> hostSpecs = tester.prepare(applicationId, cluster, Capacity.fromCount(2, nodeResources), 1); tester.activate(applicationId, new HashSet<>(hostSpecs)); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java index 59e95cc1908..4e82bdbfafe 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java @@ -90,16 +90,17 @@ public class NodeFailTester { // Create applications ClusterSpec clusterApp1 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("test"), Version.fromString("6.42"), false); ClusterSpec clusterApp2 = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("test"), Version.fromString("6.42"), false); - int wantedNodesApp1 = 5; - int wantedNodesApp2 = 7; - tester.activate(app1, clusterApp1, wantedNodesApp1); - tester.activate(app2, clusterApp2, wantedNodesApp2); - assertEquals(wantedNodesApp1, tester.nodeRepository.getNodes(app1, Node.State.active).size()); - assertEquals(wantedNodesApp2, tester.nodeRepository.getNodes(app2, Node.State.active).size()); + Capacity capacity1 = Capacity.fromCount(5, nodeResources, false, true); + Capacity capacity2 = Capacity.fromCount(7, nodeResources, false, true); + + tester.activate(app1, clusterApp1, capacity1); + tester.activate(app2, clusterApp2, capacity2); + assertEquals(capacity1.nodeCount(), tester.nodeRepository.getNodes(app1, Node.State.active).size()); + assertEquals(capacity2.nodeCount(), tester.nodeRepository.getNodes(app2, Node.State.active).size()); Map<ApplicationId, MockDeployer.ApplicationContext> apps = Map.of( - app1, new MockDeployer.ApplicationContext(app1, clusterApp1, Capacity.fromCount(wantedNodesApp1, nodeResources, false, true), 1), - app2, new MockDeployer.ApplicationContext(app2, clusterApp2, Capacity.fromCount(wantedNodesApp2, nodeResources, false, true), 1)); + app1, new MockDeployer.ApplicationContext(app1, clusterApp1, capacity1, 1), + app2, new MockDeployer.ApplicationContext(app2, clusterApp2, capacity2, 1)); tester.deployer = new MockDeployer(tester.provisioner, tester.clock(), apps); tester.serviceMonitor = new ServiceMonitorStub(apps, tester.nodeRepository); tester.metric = new MetricsReporterTest.TestMetric(); @@ -254,10 +255,6 @@ public class NodeFailTester { return nodeRepository.setReady(nodes, Agent.system, getClass().getSimpleName()); } - private void activate(ApplicationId applicationId, ClusterSpec cluster, int nodeCount) { - activate(applicationId, cluster, Capacity.fromNodeCount(nodeCount)); - } - private void activate(ApplicationId applicationId, ClusterSpec cluster, Capacity capacity) { List<HostSpec> hosts = provisioner.prepare(applicationId, cluster, capacity, 1, null); NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator)); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java index 9b051184885..0ecbf389825 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java @@ -7,9 +7,9 @@ import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.InstanceName; +import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.ParentHostUnavailableException; import com.yahoo.config.provision.RegionName; @@ -74,7 +74,7 @@ public class DockerProvisioningTest { ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).build(); ApplicationId zoneApplication = tester.makeApplicationId(); - List<Node> parents = tester.makeDockerHosts(10, new NodeResources(2, 2, 2)); + List<Node> parents = tester.makeReadyNodes(10, new NodeResources(2, 2, 2), NodeType.host, 1); for (Node parent : parents) tester.makeReadyVirtualDockerNodes(1, dockerFlavor, parent.hostname()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java index aa1763db487..1f474a8e07e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java @@ -280,7 +280,7 @@ public class DynamicDockerAllocationTest { tester.activate(application, hosts); List<Node> activeNodes = tester.nodeRepository().getNodes(application); - assertEquals(ImmutableSet.of("127.0.127.12", "::12"), activeNodes.get(0).ipAddresses()); + assertEquals(ImmutableSet.of("127.0.127.13", "::13"), activeNodes.get(0).ipAddresses()); assertEquals(ImmutableSet.of("127.0.127.2", "::2"), activeNodes.get(1).ipAddresses()); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java index e87a41455c1..7ad8dbbf7bb 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java @@ -9,10 +9,10 @@ import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.Flavor; -import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.HostFilter; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.InstanceName; +import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.OutOfCapacityException; import com.yahoo.config.provision.RegionName; @@ -143,11 +143,11 @@ public class ProvisioningTest { public void nodeVersionIsReturnedIfSet() { ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.dev, RegionName.from("us-east"))).build(); - ApplicationId application1 = tester.makeApplicationId(); - - tester.makeReadyNodes(4, "d-1-1-1"); + tester.makeReadyNodes(4, new NodeResources(1, 1, 1), NodeType.host, 1); + tester.prepareAndActivateInfraApplication(tester.makeApplicationId(), NodeType.host); // deploy + ApplicationId application1 = tester.makeApplicationId(); SystemState state1 = prepare(application1, 1, 1, 1, 1, new NodeResources(1, 1, 1), tester); tester.activate(application1, state1.allHosts); @@ -366,11 +366,13 @@ public class ProvisioningTest { } @Test - public void dev_deployment_size() { + public void dev_deployment_node_size() { ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.dev, RegionName.from("us-east"))).build(); + tester.makeReadyNodes(4, new NodeResources(1, 1, 1), NodeType.host, 1); + tester.prepareAndActivateInfraApplication(tester.makeApplicationId(), NodeType.host); + ApplicationId application = tester.makeApplicationId(); - tester.makeReadyNodes(4, "d-1-1-1"); SystemState state = prepare(application, 2, 2, 3, 3, new NodeResources(1, 1, 1), tester); assertEquals(4, state.allHosts.size()); @@ -381,8 +383,10 @@ public class ProvisioningTest { public void deploy_specific_vespa_version() { ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.dev, RegionName.from("us-east"))).build(); + tester.makeReadyNodes(4, new NodeResources(1, 1, 1), NodeType.host, 1); + tester.prepareAndActivateInfraApplication(tester.makeApplicationId(), NodeType.host); + ApplicationId application = tester.makeApplicationId(); - tester.makeReadyNodes(4, "d-1-1-1"); SystemState state = prepare(application, 2, 2, 3, 3, new NodeResources(1, 1, 1), Version.fromString("6.91"), tester); assertEquals(4, state.allHosts.size()); tester.activate(application, state.allHosts); @@ -415,8 +419,10 @@ public class ProvisioningTest { public void dev_deployment_flavor() { ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.dev, RegionName.from("us-east"))).build(); + tester.makeReadyNodes(4, new NodeResources(2, 2, 2), NodeType.host, 1); + tester.prepareAndActivateInfraApplication(tester.makeApplicationId(), NodeType.host); + ApplicationId application = tester.makeApplicationId(); - tester.makeReadyNodes(4, "d-2-2-2"); SystemState state = prepare(application, 2, 2, 3, 3, new NodeResources(2, 2, 2), tester); assertEquals(4, state.allHosts.size()); @@ -819,13 +825,13 @@ public class ProvisioningTest { } private SystemState prepare(ApplicationId application, int container0Size, int container1Size, int content0Size, - int content1Size, NodeResources flavor, Version wantedVersion, ProvisioningTester tester) { - return prepare(application, container0Size, container1Size, content0Size, content1Size, false, flavor, + int content1Size, NodeResources nodeResources, Version wantedVersion, ProvisioningTester tester) { + return prepare(application, container0Size, container1Size, content0Size, content1Size, false, nodeResources, wantedVersion, tester); } private SystemState prepare(ApplicationId application, int container0Size, int container1Size, int content0Size, - int content1Size, boolean required, NodeResources flavor, Version wantedVersion, + int content1Size, boolean required, NodeResources nodeResources, Version wantedVersion, ProvisioningTester tester) { // "deploy prepare" with a two container clusters and a storage cluster having of two groups ClusterSpec containerCluster0 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("container0"), wantedVersion, false); @@ -833,10 +839,10 @@ public class ProvisioningTest { ClusterSpec contentCluster0 = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("content0"), wantedVersion, false); ClusterSpec contentCluster1 = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("content1"), wantedVersion, false); - Set<HostSpec> container0 = prepare(application, containerCluster0, container0Size, 1, required, flavor, tester); - Set<HostSpec> container1 = prepare(application, containerCluster1, container1Size, 1, required, flavor, tester); - Set<HostSpec> content0 = prepare(application, contentCluster0, content0Size, 1, required, flavor, tester); - Set<HostSpec> content1 = prepare(application, contentCluster1, content1Size, 1, required, flavor, tester); + Set<HostSpec> container0 = prepare(application, containerCluster0, container0Size, 1, required, nodeResources, tester); + Set<HostSpec> container1 = prepare(application, containerCluster1, container1Size, 1, required, nodeResources, tester); + Set<HostSpec> content0 = prepare(application, contentCluster0, content0Size, 1, required, nodeResources, tester); + Set<HostSpec> content1 = prepare(application, contentCluster1, content1Size, 1, required, nodeResources, tester); Set<HostSpec> allHosts = new HashSet<>(); allHosts.addAll(container0); @@ -868,9 +874,9 @@ public class ProvisioningTest { } private Set<HostSpec> prepare(ApplicationId application, ClusterSpec cluster, int nodeCount, int groups, - boolean required, NodeResources flavor, ProvisioningTester tester) { + boolean required, NodeResources nodeResources, ProvisioningTester tester) { if (nodeCount == 0) return Collections.emptySet(); // this is a shady practice - return new HashSet<>(tester.prepare(application, cluster, nodeCount, groups, required, flavor)); + return new HashSet<>(tester.prepare(application, cluster, nodeCount, groups, required, nodeResources)); } private static class SystemState { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java index 9428af306a1..9f0b4faff01 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java @@ -164,6 +164,13 @@ public class ProvisioningTester { assertEquals(toHostNames(hosts), toHostNames(nodeRepository.getNodes(application, Node.State.active))); } + public void prepareAndActivateInfraApplication(ApplicationId application, NodeType nodeType) { + ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from(nodeType.toString()), Version.fromString("6.42"), false); + Capacity capacity = Capacity.fromRequiredNodeType(nodeType); + List<HostSpec> hostSpecs = prepare(application, cluster, capacity, 1, true); + activate(application, hostSpecs); + } + public void deactivate(ApplicationId applicationId) { NestedTransaction deactivateTransaction = new NestedTransaction(); nodeRepository.deactivate(applicationId, deactivateTransaction); @@ -233,17 +240,14 @@ public class ProvisioningTester { } public List<Node> makeReadyNodes(int n, NodeResources resources) { - return makeReadyNodes(n, resources, NodeType.tenant); + return makeReadyNodes(n, resources, NodeType.tenant, 0); } public List<Node> makeReadyNodes(int n, String flavor, NodeType type) { - return makeReadyNodes(n, asFlavor(flavor, type), type); + return makeReadyNodes(n, asFlavor(flavor, type), type, 0); } - public List<Node> makeReadyNodes(int n, NodeResources resources, NodeType type) { - return makeReadyNodes(n, new Flavor(resources), type, 0); - } - public List<Node> makeReadyNodes(int n, Flavor flavor, NodeType type) { - return makeReadyNodes(n, flavor, type, 0); + public List<Node> makeReadyNodes(int n, NodeResources resources, NodeType type, int ipAddressPoolSize) { + return makeReadyNodes(n, new Flavor(resources), type, ipAddressPoolSize); } public List<Node> makeProvisionedNodes(int count, String flavor, NodeType type, int ipAddressPoolSize) { @@ -279,7 +283,7 @@ public class ProvisioningTester { hostIps.add(ipv6); Set<String> ipAddressPool = new LinkedHashSet<>(); - for (int poolIp = 1; poolIp < ipAddressPoolSize; poolIp++) { + for (int poolIp = 1; poolIp <= ipAddressPoolSize; poolIp++) { nextIP++; String ipv6Addr = String.format("::%d", nextIP); ipAddressPool.add(ipv6Addr); @@ -363,41 +367,32 @@ public class ProvisioningTester { return flavor.get(); } - /** Creates a set of virtual docker hosts */ - public List<Node> makeDockerHosts(int n, NodeResources resources) { - return makeDockerHosts(n, resources, "dockerHost"); - } - - public List<Node> makeDockerHosts(int n, NodeResources resources, String namePrefix) { - return makeReadyVirtualNodes(n, 1, resources, Optional.empty(), i -> namePrefix + i, NodeType.host); - } - /** Creates a set of virtual docker nodes on a single docker host starting with index 1 and increasing */ public List<Node> makeReadyVirtualDockerNodes(int n, NodeResources resources, String dockerHostId) { return makeReadyVirtualNodes(n, 1, resources, Optional.of(dockerHostId), - i -> String.format("%s-%03d", dockerHostId, i), NodeType.tenant); + i -> String.format("%s-%03d", dockerHostId, i)); } /** Creates a single of virtual docker node on a single parent host */ public List<Node> makeReadyVirtualDockerNode(int index, NodeResources resources, String dockerHostId) { return makeReadyVirtualNodes(1, index, resources, Optional.of(dockerHostId), - i -> String.format("%s-%03d", dockerHostId, i), NodeType.tenant); + i -> String.format("%s-%03d", dockerHostId, i)); } /** Creates a set of virtual nodes without a parent host */ public List<Node> makeReadyVirtualNodes(int n, NodeResources resources) { return makeReadyVirtualNodes(n, 0, resources, Optional.empty(), - i -> UUID.randomUUID().toString(), NodeType.tenant); + i -> UUID.randomUUID().toString()); } /** Creates a set of virtual nodes on a single parent host */ private List<Node> makeReadyVirtualNodes(int count, int startIndex, NodeResources flavor, Optional<String> parentHostId, - Function<Integer, String> nodeNamer, NodeType nodeType) { + Function<Integer, String> nodeNamer) { List<Node> nodes = new ArrayList<>(count); for (int i = startIndex; i < count + startIndex; i++) { String hostname = nodeNamer.apply(i); nodes.add(nodeRepository.createNode("openstack-id", hostname, parentHostId, - new Flavor(flavor), nodeType)); + new Flavor(flavor), NodeType.tenant)); } nodes = nodeRepository.addNodes(nodes); nodes = nodeRepository.setDirty(nodes, Agent.system, getClass().getSimpleName()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java index 57a9e3c0047..5badf9246ab 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java @@ -5,8 +5,9 @@ import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.HostSpec; +import com.yahoo.config.provision.NodeResources; +import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.OutOfCapacityException; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; @@ -83,7 +84,8 @@ public class VirtualNodeProvisioningTest { { NodeResources flavor = new NodeResources(1, 1, 1); tester = new ProvisioningTester.Builder().zone(new Zone(Environment.dev, RegionName.from("us-east"))).build(); - tester.makeReadyVirtualDockerNodes(4, flavor, "parentHost1"); + tester.makeReadyNodes(4, flavor, NodeType.host, 1); + tester.prepareAndActivateInfraApplication(tester.makeApplicationId(), NodeType.host); List<HostSpec> containerHosts = prepare(containerClusterSpec, containerNodeCount, groups, flavor); List<HostSpec> contentHosts = prepare(contentClusterSpec, contentNodeCount, groups, flavor); @@ -96,7 +98,8 @@ public class VirtualNodeProvisioningTest { // Allowed to use same parent host for several nodes in same cluster in CD (even if prod env) { tester = new ProvisioningTester.Builder().zone(new Zone(SystemName.cd, Environment.prod, RegionName.from("us-east"))).build(); - tester.makeReadyVirtualDockerNodes(4, flavor, "parentHost1"); + tester.makeReadyNodes(4, flavor, NodeType.host, 1); + tester.prepareAndActivateInfraApplication(tester.makeApplicationId(), NodeType.host); List<HostSpec> containerHosts = prepare(containerClusterSpec, containerNodeCount, groups); List<HostSpec> contentHosts = prepare(contentClusterSpec, contentNodeCount, groups); diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ClusterApiImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ClusterApiImpl.java index d0217710bdb..eb69f1a94a9 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ClusterApiImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ClusterApiImpl.java @@ -1,12 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.orchestrator.model; +import com.yahoo.vespa.applicationmodel.ApplicationInstanceId; import com.yahoo.vespa.applicationmodel.ClusterId; import com.yahoo.vespa.applicationmodel.HostName; import com.yahoo.vespa.applicationmodel.ServiceCluster; import com.yahoo.vespa.applicationmodel.ServiceInstance; import com.yahoo.vespa.applicationmodel.ServiceStatus; import com.yahoo.vespa.applicationmodel.ServiceType; +import com.yahoo.vespa.applicationmodel.TenantId; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory; import com.yahoo.vespa.orchestrator.status.HostStatus; @@ -17,6 +19,7 @@ import java.util.Optional; import java.util.Set; import java.util.function.Predicate; import java.util.stream.Collectors; +import java.util.stream.Stream; class ClusterApiImpl implements ClusterApi { private final ApplicationApi applicationApi; @@ -29,6 +32,15 @@ class ClusterApiImpl implements ClusterApi { private final Set<ServiceInstance> servicesNotInGroup; private final Set<ServiceInstance> servicesDownAndNotInGroup; + /** + * There are supposed to be (at least) 3 config servers in a production-like environment. + * However the number of config servers in the zone-config-servers application/cluster may only be 2, + * if only 2 have been provisioned so far, or 1 is being reprovisioned. In these cases it is + * important for the Orchestrator to count that third config server as down. + */ + private final int missingServices; + private final String descriptionOfMissingServices; + public ClusterApiImpl(ApplicationApi applicationApi, ServiceCluster serviceCluster, NodeGroup nodeGroup, @@ -51,6 +63,15 @@ class ClusterApiImpl implements ClusterApi { servicesDownInGroup = servicesInGroup.stream().filter(this::serviceEffectivelyDown).collect(Collectors.toSet()); servicesDownAndNotInGroup = servicesNotInGroup.stream().filter(this::serviceEffectivelyDown).collect(Collectors.toSet()); + + int serviceInstances = serviceCluster.serviceInstances().size(); + if (serviceCluster.isConfigServerCluster() && serviceInstances < 3) { + missingServices = 3 - serviceInstances; + descriptionOfMissingServices = missingServices + " missing config server" + (missingServices > 1 ? "s" : ""); + } else { + missingServices = 0; + descriptionOfMissingServices = "NA"; + } } @Override @@ -83,29 +104,31 @@ class ClusterApiImpl implements ClusterApi { return servicesDownInGroup.size() == servicesInGroup.size(); } + int missingServices() { return missingServices; } + @Override public boolean noServicesOutsideGroupIsDown() { - return servicesDownAndNotInGroup.size() == 0; + return servicesDownAndNotInGroup.size() + missingServices == 0; } @Override public int percentageOfServicesDown() { - int numberOfServicesDown = servicesDownAndNotInGroup.size() + servicesDownInGroup.size(); - return numberOfServicesDown * 100 / serviceCluster.serviceInstances().size(); + int numberOfServicesDown = servicesDownAndNotInGroup.size() + missingServices + servicesDownInGroup.size(); + return numberOfServicesDown * 100 / (serviceCluster.serviceInstances().size() + missingServices); } @Override public int percentageOfServicesDownIfGroupIsAllowedToBeDown() { - int numberOfServicesDown = servicesDownAndNotInGroup.size() + servicesInGroup.size(); - return numberOfServicesDown * 100 / serviceCluster.serviceInstances().size(); + int numberOfServicesDown = servicesDownAndNotInGroup.size() + missingServices + servicesInGroup.size(); + return numberOfServicesDown * 100 / (serviceCluster.serviceInstances().size() + missingServices); } @Override public String servicesDownAndNotInGroupDescription() { // Sort these for readability and testing stability - return servicesDownAndNotInGroup.stream() - .map(service -> service.toString()) - .sorted() + return Stream + .concat(servicesDownAndNotInGroup.stream().map(ServiceInstance::toString).sorted(), + missingServices > 0 ? Stream.of(descriptionOfMissingServices) : Stream.of()) .collect(Collectors.toList()) .toString(); } diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java index 0a53972b30c..c17e885df02 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java @@ -1,10 +1,17 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.orchestrator.model; +import com.yahoo.vespa.applicationmodel.ApplicationInstance; +import com.yahoo.vespa.applicationmodel.ApplicationInstanceId; +import com.yahoo.vespa.applicationmodel.ClusterId; import com.yahoo.vespa.applicationmodel.HostName; import com.yahoo.vespa.applicationmodel.ServiceCluster; import com.yahoo.vespa.applicationmodel.ServiceStatus; import com.yahoo.vespa.applicationmodel.ServiceType; +import com.yahoo.vespa.applicationmodel.TenantId; +import com.yahoo.vespa.orchestrator.OrchestratorUtil; +import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException; +import com.yahoo.vespa.orchestrator.policy.HostedVespaClusterPolicy; import com.yahoo.vespa.orchestrator.status.HostStatus; import org.junit.Test; @@ -12,12 +19,18 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Optional; +import java.util.Set; +import static org.hamcrest.core.StringContains.containsString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class ClusterApiImplTest { final ApplicationApi applicationApi = mock(ApplicationApi.class); @@ -72,6 +85,69 @@ public class ClusterApiImplTest { assertEquals(80, clusterApi.percentageOfServicesDownIfGroupIsAllowedToBeDown()); } + /** Make a ClusterApiImpl for the cfg1 config server, with cfg3 missing from the cluster (not provisioned). */ + private ClusterApiImpl makeCfg1ClusterApi(ServiceStatus cfg1ServiceStatus, ServiceStatus cfg2ServiceStatus) { + HostName cfg1Hostname = new HostName("cfg1"); + HostName cfg2Hostname = new HostName("cfg2"); + + ServiceCluster serviceCluster = modelUtils.createServiceCluster( + ClusterId.CONFIG_SERVER.s(), + ServiceType.CONFIG_SERVER, + Arrays.asList( + modelUtils.createServiceInstance("cs1", cfg1Hostname, cfg1ServiceStatus), + modelUtils.createServiceInstance("cs2", cfg2Hostname, cfg2ServiceStatus)) + ); + + Set<ServiceCluster> serviceClusterSet = new HashSet<>(Set.of(serviceCluster)); + + ApplicationInstance application = new ApplicationInstance( + TenantId.HOSTED_VESPA, + ApplicationInstanceId.CONFIG_SERVER, + serviceClusterSet); + + serviceCluster.setApplicationInstance(application); + + when(applicationApi.applicationId()).thenReturn(OrchestratorUtil.toApplicationId(application.reference())); + + ClusterApiImpl clusterApi = new ClusterApiImpl( + applicationApi, + serviceCluster, + new NodeGroup(application, cfg1Hostname), + modelUtils.getHostStatusMap(), + modelUtils.getClusterControllerClientFactory()); + + assertEquals(1, clusterApi.missingServices()); + assertFalse(clusterApi.noServicesOutsideGroupIsDown()); + + return clusterApi; + } + + @Test + public void testCfg1SuspensionFailsWithMissingCfg3() { + ClusterApiImpl clusterApi = makeCfg1ClusterApi(ServiceStatus.UP, ServiceStatus.UP); + + HostedVespaClusterPolicy policy = new HostedVespaClusterPolicy(); + + try { + policy.verifyGroupGoingDownIsFine(clusterApi); + fail(); + } catch (HostStateChangeDeniedException e) { + assertThat(e.getMessage(), + containsString("Changing the state of cfg1 would violate enough-services-up: Suspension percentage " + + "for service type configserver would increase from 33% to 66%, over the limit of 10%. " + + "These instances may be down: [1 missing config server] and these hosts are allowed to be down: []")); + } + } + + @Test + public void testCfg1SuspendsIfDownWithMissingCfg3() throws HostStateChangeDeniedException { + ClusterApiImpl clusterApi = makeCfg1ClusterApi(ServiceStatus.DOWN, ServiceStatus.UP); + + HostedVespaClusterPolicy policy = new HostedVespaClusterPolicy(); + + policy.verifyGroupGoingDownIsFine(clusterApi); + } + @Test public void testNoServices() { HostName hostName1 = new HostName("host1"); @@ -141,10 +217,14 @@ public class ClusterApiImplTest { ) ); + + ApplicationInstance applicationInstance = modelUtils.createApplicationInstance(new ArrayList<>()); + serviceCluster.setApplicationInstance(applicationInstance); + ClusterApiImpl clusterApi = new ClusterApiImpl( applicationApi, serviceCluster, - new NodeGroup(modelUtils.createApplicationInstance(new ArrayList<>()), hostName1, hostName3), + new NodeGroup(applicationInstance, hostName1, hostName3), new HashMap<>(), modelUtils.getClusterControllerClientFactory()); diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ModelTestUtils.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ModelTestUtils.java index 78f50dbfc3f..0def668e147 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ModelTestUtils.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ModelTestUtils.java @@ -86,6 +86,9 @@ class ModelTestUtils { new ApplicationInstanceId("application-name:foo:bar:default"), serviceClusterSet); applications.put(application.reference(), application); + + serviceClusters.forEach(cluster -> cluster.setApplicationInstance(application)); + return application; } diff --git a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp index 4843778d0c8..93f3299e121 100644 --- a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp +++ b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp @@ -23,11 +23,13 @@ using namespace vespalib; using search::IDestructorCallback; using storage::spi::Timestamp; using BlockedReason = IBlockableMaintenanceJob::BlockedReason; +using TimePoint = LidUsageStats::TimePoint; constexpr uint32_t SUBDB_ID = 2; constexpr double JOB_DELAY = 1.0; constexpr uint32_t ALLOWED_LID_BLOAT = 1; constexpr double ALLOWED_LID_BLOAT_FACTOR = 0.3; +constexpr double REMOVE_BATCH_BLOCK_DELAY = 20.0; constexpr uint32_t MAX_DOCS_TO_SCAN = 100; constexpr double RESOURCE_LIMIT_FACTOR = 1.0; constexpr uint32_t MAX_OUTSTANDING_MOVE_OPS = 10; @@ -83,6 +85,12 @@ struct MyHandler : public ILidSpaceCompactionHandler { MyHandler(bool storeMoveDoneContexts = false); ~MyHandler(); void clearMoveDoneContexts() { _moveDoneContexts.clear(); } + void set_last_remove_batch(TimePoint last_remove_batch) { + for (auto& s : _stats) { + s = LidUsageStats(s.getLidLimit(), s.getUsedLids(), + s.getLowestFreeLid(), s.getHighestUsedLid(), last_remove_batch); + } + } virtual vespalib::string getName() const override { return "myhandler"; } @@ -255,36 +263,40 @@ struct JobTestBase : public ::testing::Test { { _handler = std::make_unique<MyHandler>(maxOutstandingMoveOps != MAX_OUTSTANDING_MOVE_OPS); _job = std::make_unique<LidSpaceCompactionJob>(DocumentDBLidSpaceCompactionConfig(interval, allowedLidBloat, - allowedLidBloatFactor, false, maxDocsToScan), + allowedLidBloatFactor, + REMOVE_BATCH_BLOCK_DELAY, + false, maxDocsToScan), *_handler, _storer, _frozenHandler, _diskMemUsageNotifier, BlockableMaintenanceJobConfig(resourceLimitFactor, maxOutstandingMoveOps), _clusterStateHandler, nodeRetired); } ~JobTestBase(); JobTestBase &addStats(uint32_t docIdLimit, - const LidVector &usedLids, - const LidPairVector &usedFreePairs) { - return addMultiStats(docIdLimit, {usedLids}, usedFreePairs); + const LidVector &usedLids, + const LidPairVector &usedFreePairs, + TimePoint last_remove_batch = TimePoint()) { + return addMultiStats(docIdLimit, {usedLids}, usedFreePairs, last_remove_batch); } JobTestBase &addMultiStats(uint32_t docIdLimit, const std::vector<LidVector> &usedLidsVector, - const LidPairVector &usedFreePairs) { + const LidPairVector &usedFreePairs, + TimePoint last_remove_batch = TimePoint()) { uint32_t usedLids = usedLidsVector[0].size(); for (auto pair : usedFreePairs) { uint32_t highestUsedLid = pair.first; uint32_t lowestFreeLid = pair.second; _handler->_stats.push_back(LidUsageStats - (docIdLimit, usedLids, lowestFreeLid, highestUsedLid)); + (docIdLimit, usedLids, lowestFreeLid, highestUsedLid, last_remove_batch)); } _handler->_lids = usedLidsVector; return *this; } JobTestBase &addStats(uint32_t docIdLimit, - uint32_t numDocs, - uint32_t lowestFreeLid, - uint32_t highestUsedLid) { + uint32_t numDocs, + uint32_t lowestFreeLid, + uint32_t highestUsedLid) { _handler->_stats.push_back(LidUsageStats - (docIdLimit, numDocs, lowestFreeLid, highestUsedLid)); + (docIdLimit, numDocs, lowestFreeLid, highestUsedLid, TimePoint())); return *this; } bool run() { @@ -319,10 +331,11 @@ struct JobTestBase : public ::testing::Test { void assertNoWorkDone() { assertJobContext(0, 0, 0, 0, 0); } - JobTestBase &setupOneDocumentToCompact() { + JobTestBase &setupOneDocumentToCompact(TimePoint last_remove_batch = TimePoint()) { addStats(10, {1,3,4,5,6,9}, {{9,2}, // 30% bloat: move 9 -> 2 - {6,7}}); // no documents to move + {6,7}}, // no documents to move + last_remove_batch); return *this; } void assertOneDocumentCompacted() { @@ -606,6 +619,41 @@ TEST_F(JobTest, job_is_re_enabled_when_node_is_no_longer_retired) assertOneDocumentCompacted(); } +TEST_F(JobTest, job_is_disabled_while_remove_batch_is_ongoing) +{ + TimePoint last_remove_batch = std::chrono::steady_clock::now(); + setupOneDocumentToCompact(last_remove_batch); + EXPECT_TRUE(run()); // job is disabled + assertNoWorkDone(); +} + +TEST_F(JobTest, job_becomes_disabled_if_remove_batch_starts) +{ + setupThreeDocumentsToCompact(); + EXPECT_FALSE(run()); // job executed as normal (with more work to do) + assertJobContext(2, 9, 1, 0, 0); + + _handler->set_last_remove_batch(std::chrono::steady_clock::now()); + EXPECT_TRUE(run()); // job is disabled + assertJobContext(2, 9, 1, 0, 0); +} + +TEST_F(JobTest, job_is_re_enabled_when_remove_batch_is_no_longer_ongoing) +{ + setupThreeDocumentsToCompact(); + EXPECT_FALSE(run()); // job executed as normal (with more work to do) + assertJobContext(2, 9, 1, 0, 0); + + TimePoint last_remove_batch = std::chrono::steady_clock::now(); + _handler->set_last_remove_batch(last_remove_batch); + EXPECT_TRUE(run()); // job is disabled + assertJobContext(2, 9, 1, 0, 0); + + _handler->set_last_remove_batch(last_remove_batch - std::chrono::seconds(static_cast<long>(REMOVE_BATCH_BLOCK_DELAY))); + EXPECT_FALSE(run()); // job executed as normal (with more work to do) + assertJobContext(3, 8, 2, 0, 0); +} + struct MaxOutstandingJobTest : public JobTest { std::unique_ptr<MyCountJobRunner> runner; MaxOutstandingJobTest() diff --git a/searchcore/src/tests/proton/documentmetastore/documentmetastore_test.cpp b/searchcore/src/tests/proton/documentmetastore/documentmetastore_test.cpp index 99b425b9fd7..f6f0c2b0806 100644 --- a/searchcore/src/tests/proton/documentmetastore/documentmetastore_test.cpp +++ b/searchcore/src/tests/proton/documentmetastore/documentmetastore_test.cpp @@ -1782,7 +1782,7 @@ TEST(DocumentMetaStoreTest, get_lid_usage_stats_works) void assertLidBloat(uint32_t expBloat, uint32_t lidLimit, uint32_t usedLids) { - LidUsageStats stats(lidLimit, usedLids, 0, 0); + LidUsageStats stats(lidLimit, usedLids, 0, 0, LidUsageStats::TimePoint()); EXPECT_EQ(expBloat, stats.getLidBloat()); } @@ -2084,6 +2084,23 @@ TEST(DocumentMetaStoreTest, multiple_lids_can_be_removed_with_removeBatch) assertLidGidFound(4, dms); } +TEST(DocumentMetaStoreTest, tracks_time_of_last_call_to_remove_batch) +{ + DocumentMetaStore dms(createBucketDB()); + dms.constructFreeList(); + addLid(dms, 1); + + LidUsageStats::TimePoint before = std::chrono::steady_clock::now(); + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + dms.removeBatch({1}, 5); + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + LidUsageStats::TimePoint after = std::chrono::steady_clock::now(); + + auto stats = dms.getLidUsageStats(); + EXPECT_LT(before, stats.get_last_remove_batch()); + EXPECT_GT(after, stats.get_last_remove_batch()); +} + } GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchcore/src/vespa/searchcore/config/proton.def b/searchcore/src/vespa/searchcore/config/proton.def index a88239a41f6..fd50dd2094a 100644 --- a/searchcore/src/vespa/searchcore/config/proton.def +++ b/searchcore/src/vespa/searchcore/config/proton.def @@ -356,6 +356,16 @@ lidspacecompaction.allowedlidbloat int default=1000 ## The lid bloat factor must be >= allowedlidbloatfactor before considering compaction. lidspacecompaction.allowedlidbloatfactor double default=0.01 +## The delay (in seconds) for when the last remove batch operation would be considered to block lid space compaction. +## +## When considering compaction, if the document meta store has received a remove batch operation in the last delay seconds, +## the lid space compaction job is blocked. It is considered again at the next regular interval (see above). +## +## Remove batch operations are used when deleting buckets on a content node. +## This functionality ensures that during massive deleting of buckets (e.g. as part of redistribution of data to a new node), +## lid space compaction do not interfere, but instead is applied after deleting of buckets is complete. +lidspacecompaction.removebatchblockdelay double default=5.0 + ## This is the maximum value visibilitydelay you can have. ## A to higher value here will cost more memory while not improving too much. maxvisibilitydelay double default=1.0 diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp index 9b535be19b7..e6f16004bad 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp @@ -450,7 +450,8 @@ DocumentMetaStore::DocumentMetaStore(BucketDBOwner::SP bucketDB, _bucketDB(bucketDB), _shrinkLidSpaceBlockers(0), _subDbType(subDbType), - _trackDocumentSizes(true) + _trackDocumentSizes(true), + _last_remove_batch() { ensureSpace(0); // lid 0 is reserved setCommittedDocIdLimit(1u); // lid 0 is reserved @@ -665,6 +666,7 @@ DocumentMetaStore::removeBatch(const std::vector<DocId> &lidsToRemove, const uin (void) removed; } incGeneration(); + _last_remove_batch = std::chrono::steady_clock::now(); } void @@ -772,7 +774,8 @@ DocumentMetaStore::getLidUsageStats() const return LidUsageStats(docIdLimit, numDocs, lowestFreeLid, - highestUsedLid); + highestUsedLid, + _last_remove_batch); } Blueprint::UP diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h index 27c1c97556c..3bd9795cfd5 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h @@ -71,6 +71,7 @@ private: uint32_t _shrinkLidSpaceBlockers; const SubDbType _subDbType; bool _trackDocumentSizes; + search::LidUsageStats::TimePoint _last_remove_batch; DocId getFreeLid(); DocId peekFreeLid(); diff --git a/searchcore/src/vespa/searchcore/proton/server/document_db_maintenance_config.cpp b/searchcore/src/vespa/searchcore/proton/server/document_db_maintenance_config.cpp index 848b1f27574..b470a390b50 100644 --- a/searchcore/src/vespa/searchcore/proton/server/document_db_maintenance_config.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/document_db_maintenance_config.cpp @@ -54,6 +54,7 @@ DocumentDBLidSpaceCompactionConfig::DocumentDBLidSpaceCompactionConfig() _interval(3600), _allowedLidBloat(1000000000), _allowedLidBloatFactor(1.0), + _remove_batch_block_delay(5.0), _disabled(false), _maxDocsToScan(10000) { @@ -62,12 +63,14 @@ DocumentDBLidSpaceCompactionConfig::DocumentDBLidSpaceCompactionConfig() DocumentDBLidSpaceCompactionConfig::DocumentDBLidSpaceCompactionConfig(double interval, uint32_t allowedLidBloat, double allowedLidBloatFactor, + double remove_batch_block_delay, bool disabled, uint32_t maxDocsToScan) : _delay(std::min(MAX_DELAY_SEC, interval)), _interval(interval), _allowedLidBloat(allowedLidBloat), _allowedLidBloatFactor(allowedLidBloatFactor), + _remove_batch_block_delay(remove_batch_block_delay), _disabled(disabled), _maxDocsToScan(maxDocsToScan) { diff --git a/searchcore/src/vespa/searchcore/proton/server/document_db_maintenance_config.h b/searchcore/src/vespa/searchcore/proton/server/document_db_maintenance_config.h index acbbc442c7a..4b458765f3c 100644 --- a/searchcore/src/vespa/searchcore/proton/server/document_db_maintenance_config.h +++ b/searchcore/src/vespa/searchcore/proton/server/document_db_maintenance_config.h @@ -47,6 +47,7 @@ private: double _interval; uint32_t _allowedLidBloat; double _allowedLidBloatFactor; + double _remove_batch_block_delay; bool _disabled; uint32_t _maxDocsToScan; @@ -55,7 +56,8 @@ public: DocumentDBLidSpaceCompactionConfig(double interval, uint32_t allowedLidBloat, double allowwedLidBloatFactor, - bool disabled = false, + double remove_batch_block_delay, + bool disabled, uint32_t maxDocsToScan = 10000); static DocumentDBLidSpaceCompactionConfig createDisabled(); @@ -64,6 +66,7 @@ public: double getInterval() const { return _interval; } uint32_t getAllowedLidBloat() const { return _allowedLidBloat; } double getAllowedLidBloatFactor() const { return _allowedLidBloatFactor; } + double get_remove_batch_block_delay() const { return _remove_batch_block_delay; } bool isDisabled() const { return _disabled; } uint32_t getMaxDocsToScan() const { return _maxDocsToScan; } }; diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.cpp index a562408b64d..ef31da34683 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.cpp @@ -135,6 +135,7 @@ buildMaintenanceConfig(const BootstrapConfig::SP &bootstrapConfig, proton.lidspacecompaction.interval, proton.lidspacecompaction.allowedlidbloat, proton.lidspacecompaction.allowedlidbloatfactor, + proton.lidspacecompaction.removebatchblockdelay, isDocumentTypeGlobal), AttributeUsageFilterConfig( proton.writefilter.attribute.enumstorelimit, diff --git a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp index fad00fa00e6..c2d655538f5 100644 --- a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp @@ -89,6 +89,13 @@ LidSpaceCompactionJob::compactLidSpace(const LidUsageStats &stats) _shouldCompactLidSpace = false; } +bool +LidSpaceCompactionJob::remove_batch_is_ongoing(const LidUsageStats& stats) const +{ + LidUsageStats::TimePoint now = std::chrono::steady_clock::now(); + return (now - stats.get_last_remove_batch()) < std::chrono::duration<double>(_cfg.get_remove_batch_block_delay()); +} + LidSpaceCompactionJob::LidSpaceCompactionJob(const DocumentDBLidSpaceCompactionConfig &config, ILidSpaceCompactionHandler &handler, IOperationStorer &opStorer, @@ -129,6 +136,11 @@ LidSpaceCompactionJob::run() return true; // indicate work is done since no work can be done } LidUsageStats stats = _handler.getLidStatus(); + if (remove_batch_is_ongoing(stats)) { + // Note that we don't set the job as blocked as the decision to un-block it is not driven externally. + LOG(info, "run(): Lid space compaction is disabled while remove batch (delete buckets) is ongoing"); + return true; + } if (_scanItr) { return scanDocuments(stats); } else if (_shouldCompactLidSpace) { diff --git a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.h b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.h index 0732576cc70..2f242e5a33a 100644 --- a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.h +++ b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.h @@ -45,6 +45,7 @@ private: void compactLidSpace(const search::LidUsageStats &stats); void refreshRunnable(); void refreshAndConsiderRunnable(); + bool remove_batch_is_ongoing(const search::LidUsageStats& stats) const; public: LidSpaceCompactionJob(const DocumentDBLidSpaceCompactionConfig &config, diff --git a/searchlib/src/main/javacc/RankingExpressionParser.jj b/searchlib/src/main/javacc/RankingExpressionParser.jj index c83de4ced0a..ea65a508047 100755 --- a/searchlib/src/main/javacc/RankingExpressionParser.jj +++ b/searchlib/src/main/javacc/RankingExpressionParser.jj @@ -617,7 +617,7 @@ TensorType.Value optionalTensorValueTypeParameter() : } { ( <LT> valueType = identifier() <GT> )? - { return TensorTypeParser.toValueType(valueType); } + { return TensorType.Value.fromId(valueType); } } // NOTE: Only indexed bound dimensions are parsed currently, as that is what we need diff --git a/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp b/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp index 82539214ea9..05ea57cf43a 100644 --- a/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp @@ -476,8 +476,9 @@ namespace search::datastore { using Reference = attribute::Reference; +template class UniqueStoreAllocator<Reference, EntryRefT<22>>; template class UniqueStore<Reference, EntryRefT<22>>; -template class UniqueStoreBuilder<Reference, EntryRefT<22>>; +template class UniqueStoreBuilder<UniqueStoreAllocator<Reference, EntryRefT<22>>>; template class UniqueStoreSaver<Reference, EntryRefT<22>>; } diff --git a/searchlib/src/vespa/searchlib/common/lid_usage_stats.h b/searchlib/src/vespa/searchlib/common/lid_usage_stats.h index 98d2cfc7e5c..b54a3ce8a6a 100644 --- a/searchlib/src/vespa/searchlib/common/lid_usage_stats.h +++ b/searchlib/src/vespa/searchlib/common/lid_usage_stats.h @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once +#include <chrono> #include <stdint.h> namespace search { @@ -8,36 +9,43 @@ namespace search { /** * Stats on the usage and availability of lids in a document meta store. */ -class LidUsageStats -{ +class LidUsageStats { +public: + using TimePoint = std::chrono::time_point<std::chrono::steady_clock>; + private: uint32_t _lidLimit; uint32_t _usedLids; uint32_t _lowestFreeLid; uint32_t _highestUsedLid; + TimePoint _last_remove_batch; public: LidUsageStats() : _lidLimit(0), _usedLids(0), _lowestFreeLid(0), - _highestUsedLid(0) + _highestUsedLid(0), + _last_remove_batch() { } LidUsageStats(uint32_t lidLimit, uint32_t usedLids, uint32_t lowestFreeLid, - uint32_t highestUsedLid) + uint32_t highestUsedLid, + TimePoint last_remove_batch) : _lidLimit(lidLimit), _usedLids(usedLids), _lowestFreeLid(lowestFreeLid), - _highestUsedLid(highestUsedLid) + _highestUsedLid(highestUsedLid), + _last_remove_batch(last_remove_batch) { } uint32_t getLidLimit() const { return _lidLimit; } uint32_t getUsedLids() const { return _usedLids; } uint32_t getLowestFreeLid() const { return _lowestFreeLid; } uint32_t getHighestUsedLid() const { return _highestUsedLid; } + const TimePoint& get_last_remove_batch() const { return _last_remove_batch; } uint32_t getLidBloat() const { // Account for reserved lid 0 int32_t lidBloat = getLidLimit() - getUsedLids() - 1; @@ -61,5 +69,5 @@ public: } }; -} // namespace search +} diff --git a/vespajlib/abi-spec.json b/vespajlib/abi-spec.json index 6f37b9edea4..9c425570a7e 100644 --- a/vespajlib/abi-spec.json +++ b/vespajlib/abi-spec.json @@ -1364,9 +1364,12 @@ "methods": [ "public static com.yahoo.tensor.TensorType$Value[] values()", "public static com.yahoo.tensor.TensorType$Value valueOf(java.lang.String)", + "public java.lang.String id()", + "public boolean isEqualOrLargerThan(com.yahoo.tensor.TensorType$Value)", "public static com.yahoo.tensor.TensorType$Value largestOf(java.util.List)", "public static com.yahoo.tensor.TensorType$Value largestOf(com.yahoo.tensor.TensorType$Value, com.yahoo.tensor.TensorType$Value)", - "public java.lang.String toString()" + "public java.lang.String toString()", + "public static com.yahoo.tensor.TensorType$Value fromId(java.lang.String)" ], "fields": [ "public static final enum com.yahoo.tensor.TensorType$Value DOUBLE", @@ -1409,8 +1412,7 @@ ], "methods": [ "public void <init>()", - "public static com.yahoo.tensor.TensorType fromSpec(java.lang.String)", - "public static com.yahoo.tensor.TensorType$Value toValueType(java.lang.String)" + "public static com.yahoo.tensor.TensorType fromSpec(java.lang.String)" ], "fields": [] }, diff --git a/vespajlib/src/main/java/com/yahoo/tensor/TensorType.java b/vespajlib/src/main/java/com/yahoo/tensor/TensorType.java index 9869f1e908c..319947607d2 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/TensorType.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/TensorType.java @@ -29,7 +29,17 @@ public class TensorType { public enum Value { // Types added must also be added to TensorTypeParser.parseValueTypeSpec, serialization, and largestOf below - DOUBLE, FLOAT; + DOUBLE("double"), FLOAT("float"); + + private final String id; + + Value(String id) { this.id = id; } + + public String id() { return id; } + + public boolean isEqualOrLargerThan(TensorType.Value other) { + return this == other || largestOf(this, other) == this; + } public static Value largestOf(List<Value> values) { if (values.isEmpty()) return Value.DOUBLE; // Default @@ -51,6 +61,15 @@ public class TensorType { @Override public String toString() { return name().toLowerCase(); } + public static Value fromId(String valueTypeString) { + switch (valueTypeString) { + case "double" : return Value.DOUBLE; + case "float" : return Value.FLOAT; + default : throw new IllegalArgumentException("Value type must be either 'double' or 'float'" + + " but was '" + valueTypeString + "'"); + } + } + }; /** The empty tensor type - which is the same as a double */ @@ -146,7 +165,7 @@ public class TensorType { } private boolean isConvertibleOrAssignableTo(TensorType generalization, boolean convertible, boolean considerName) { - if ( this.valueType() != generalization.valueType()) return false; // TODO: This can be relaxed + if ( ! generalization.valueType().isEqualOrLargerThan(this.valueType) ) return false; if (generalization.dimensions().size() != this.dimensions().size()) return false; for (int i = 0; i < generalization.dimensions().size(); i++) { Dimension thisDimension = this.dimensions().get(i); @@ -168,11 +187,9 @@ public class TensorType { @Override public String toString() { - if ((rank() == 0) || (valueType == Value.DOUBLE)) { - return "tensor(" + dimensions.stream().map(Dimension::toString).collect(Collectors.joining(",")) + ")"; - } else { - return "tensor<" + valueType + ">(" + dimensions.stream().map(Dimension::toString).collect(Collectors.joining(",")) + ")"; - } + return "tensor" + + (valueType == Value.DOUBLE ? "" : "<" + valueType.id() + ">") + + "(" + dimensions.stream().map(Dimension::toString).collect(Collectors.joining(",")) + ")"; } @Override @@ -238,7 +255,7 @@ public class TensorType { @Override public int hashCode() { - return dimensions.hashCode(); + return Objects.hash(dimensions, valueType); } /** diff --git a/vespajlib/src/main/java/com/yahoo/tensor/TensorTypeParser.java b/vespajlib/src/main/java/com/yahoo/tensor/TensorTypeParser.java index 1f426942c5f..def3ab6b4ec 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/TensorTypeParser.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/TensorTypeParser.java @@ -56,21 +56,12 @@ public class TensorTypeParser { return new TensorType.Builder(valueType, dimensions).build(); } - public static TensorType.Value toValueType(String valueTypeString) { - switch (valueTypeString) { - case "double" : return TensorType.Value.DOUBLE; - case "float" : return TensorType.Value.FLOAT; - default : throw new IllegalArgumentException("Value type must be either 'double' or 'float'" + - " but was '" + valueTypeString + "'"); - } - } - private static TensorType.Value parseValueTypeSpec(String valueTypeSpec, String fullSpecString) { if ( ! valueTypeSpec.startsWith("<") || ! valueTypeSpec.endsWith(">")) throw formatException(fullSpecString, Optional.of("Value type spec must be enclosed in <>")); try { - return toValueType(valueTypeSpec.substring(1, valueTypeSpec.length() - 1)); + return TensorType.Value.fromId(valueTypeSpec.substring(1, valueTypeSpec.length() - 1)); } catch (IllegalArgumentException e) { throw formatException(fullSpecString, e.getMessage()); diff --git a/vespajlib/src/test/java/com/yahoo/tensor/TensorTypeTestCase.java b/vespajlib/src/test/java/com/yahoo/tensor/TensorTypeTestCase.java index d3bb702175a..a547f941d8e 100644 --- a/vespajlib/src/test/java/com/yahoo/tensor/TensorTypeTestCase.java +++ b/vespajlib/src/test/java/com/yahoo/tensor/TensorTypeTestCase.java @@ -96,6 +96,8 @@ public class TensorTypeTestCase { assertValueType(TensorType.Value.DOUBLE, "tensor(x[])"); assertValueType(TensorType.Value.DOUBLE, "tensor<double>(x[])"); assertValueType(TensorType.Value.FLOAT, "tensor<float>(x[])"); + assertEquals("tensor(x[])", TensorType.fromSpec("tensor<double>(x[])").toString()); + assertEquals("tensor<float>(x[])", TensorType.fromSpec("tensor<float>(x[])").toString()); } private static void assertTensorType(String typeSpec) { diff --git a/vespalib/src/tests/datastore/unique_store/CMakeLists.txt b/vespalib/src/tests/datastore/unique_store/CMakeLists.txt index dd200018448..d72e8c10ad5 100644 --- a/vespalib/src/tests/datastore/unique_store/CMakeLists.txt +++ b/vespalib/src/tests/datastore/unique_store/CMakeLists.txt @@ -4,5 +4,6 @@ vespa_add_executable(vespalib_unique_store_test_app TEST unique_store_test.cpp DEPENDS vespalib + gtest ) vespa_add_test(NAME vespalib_unique_store_test_app COMMAND vespalib_unique_store_test_app) diff --git a/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp b/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp index 2ed7bfd52ed..a585186aa3e 100644 --- a/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp +++ b/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp @@ -1,13 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/log/log.h> -LOG_SETUP("unique_store_test"); #include <vespa/vespalib/datastore/unique_store.hpp> -#include <vespa/vespalib/testkit/testapp.h> +#include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/test/datastore/memstats.h> #include <vespa/vespalib/test/insertion_operators.h> #include <vespa/vespalib/util/traits.h> #include <vector> +#include <vespa/log/log.h> +LOG_SETUP("unique_store_test"); + using namespace search::datastore; using vespalib::MemoryUsage; using vespalib::ArrayRef; @@ -15,8 +16,7 @@ using generation_t = vespalib::GenerationHandler::generation_t; using MemStats = search::datastore::test::MemStats; template <typename EntryT, typename RefT = EntryRefT<22> > -struct Fixture -{ +struct TestBase : public ::testing::Test { using EntryRefType = RefT; using UniqueStoreType = UniqueStore<EntryT, RefT>; using value_type = EntryT; @@ -25,7 +25,7 @@ struct Fixture UniqueStoreType store; ReferenceStore refStore; generation_t generation; - Fixture() + TestBase() : store(), refStore(), generation(1) @@ -38,7 +38,7 @@ struct Fixture UniqueStoreAddResult addResult = store.add(input); EntryRef result = addResult.ref(); auto insres = refStore.insert(std::make_pair(result, std::make_pair(input, 1u))); - EXPECT_EQUAL(insres.second, addResult.inserted()); + EXPECT_EQ(insres.second, addResult.inserted()); if (!insres.second) { ++insres.first->second.second; } @@ -56,10 +56,10 @@ struct Fixture } void assertGet(EntryRef ref, const EntryT &exp) const { EntryT act = store.get(ref); - EXPECT_EQUAL(exp, act); + EXPECT_EQ(exp, act); } void remove(EntryRef ref) { - ASSERT_EQUAL(1u, refStore.count(ref)); + ASSERT_EQ(1u, refStore.count(ref)); store.remove(ref); if (refStore[ref].second > 1) { --refStore[ref].second; @@ -74,19 +74,19 @@ struct Fixture return EntryRefType(ref).bufferId(); } void assertBufferState(EntryRef ref, const MemStats expStats) const { - EXPECT_EQUAL(expStats._used, store.bufferState(ref).size()); - EXPECT_EQUAL(expStats._hold, store.bufferState(ref).getHoldElems()); - EXPECT_EQUAL(expStats._dead, store.bufferState(ref).getDeadElems()); + EXPECT_EQ(expStats._used, store.bufferState(ref).size()); + EXPECT_EQ(expStats._hold, store.bufferState(ref).getHoldElems()); + EXPECT_EQ(expStats._dead, store.bufferState(ref).getDeadElems()); } void assertMemoryUsage(const MemStats expStats) const { MemoryUsage act = store.getMemoryUsage(); - EXPECT_EQUAL(expStats._used, act.usedBytes()); - EXPECT_EQUAL(expStats._hold, act.allocatedBytesOnHold()); - EXPECT_EQUAL(expStats._dead, act.deadBytes()); + EXPECT_EQ(expStats._used, act.usedBytes()); + EXPECT_EQ(expStats._hold, act.allocatedBytesOnHold()); + EXPECT_EQ(expStats._dead, act.deadBytes()); } void assertStoreContent() const { for (const auto &elem : refStore) { - TEST_DO(assertGet(elem.first, elem.second.first)); + assertGet(elem.first, elem.second.first); } } EntryRef getEntryRef(const EntryT &input) { @@ -115,8 +115,8 @@ struct Fixture refs.pop_back(); ReferenceStore compactedRefStore; for (size_t i = 0; i < refs.size(); ++i) { - ASSERT_EQUAL(0u, compactedRefStore.count(compactedRefs[i])); - ASSERT_EQUAL(1u, refStore.count(refs[i])); + ASSERT_EQ(0u, compactedRefStore.count(compactedRefs[i])); + ASSERT_EQ(1u, refStore.count(refs[i])); compactedRefStore.insert(std::make_pair(compactedRefs[i], refStore[refs[i]])); } refStore = compactedRefStore; @@ -126,141 +126,141 @@ struct Fixture auto getSaver() { return store.getSaver(); } }; -using NumberFixture = Fixture<uint32_t>; -using StringFixture = Fixture<std::string>; -using SmallOffsetNumberFixture = Fixture<uint32_t, EntryRefT<10>>; +using NumberTest = TestBase<uint32_t>; +using StringTest = TestBase<std::string>; +using SmallOffsetNumberTest = TestBase<uint32_t, EntryRefT<10>>; -TEST("require that we test with trivial and non-trivial types") +TEST(UniqueStoreTest, trivial_and_non_trivial_types_are_tested) { - EXPECT_TRUE(vespalib::can_skip_destruction<NumberFixture::value_type>::value); - EXPECT_FALSE(vespalib::can_skip_destruction<StringFixture::value_type>::value); + EXPECT_TRUE(vespalib::can_skip_destruction<NumberTest::value_type>::value); + EXPECT_FALSE(vespalib::can_skip_destruction<StringTest::value_type>::value); } -TEST_F("require that we can add and get values of trivial type", NumberFixture) +TEST_F(NumberTest, can_add_and_get_values_of_trivial_type) { - TEST_DO(f.assertAdd(1)); - TEST_DO(f.assertAdd(2)); - TEST_DO(f.assertAdd(3)); - TEST_DO(f.assertAdd(1)); + assertAdd(1); + assertAdd(2); + assertAdd(3); + assertAdd(1); } -TEST_F("require that we can add and get values of non-trivial type", StringFixture) +TEST_F(StringTest, can_add_and_get_values_of_non_trivial_type) { - TEST_DO(f.assertAdd("aa")); - TEST_DO(f.assertAdd("bbb")); - TEST_DO(f.assertAdd("ccc")); - TEST_DO(f.assertAdd("aa")); + assertAdd("aa"); + assertAdd("bbb"); + assertAdd("ccc"); + assertAdd("aa"); } -TEST_F("require that elements are put on hold when value is removed", NumberFixture) +TEST_F(NumberTest, elements_are_put_on_hold_when_value_is_removed) { - EntryRef ref = f.add(1); + EntryRef ref = add(1); // Note: The first buffer have the first element reserved -> we expect 2 elements used here. - TEST_DO(f.assertBufferState(ref, MemStats().used(2).hold(0).dead(1))); - f.store.remove(ref); - TEST_DO(f.assertBufferState(ref, MemStats().used(2).hold(1).dead(1))); + assertBufferState(ref, MemStats().used(2).hold(0).dead(1)); + store.remove(ref); + assertBufferState(ref, MemStats().used(2).hold(1).dead(1)); } -TEST_F("require that elements are reference counted", NumberFixture) +TEST_F(NumberTest, elements_are_reference_counted) { - EntryRef ref = f.add(1); - EntryRef ref2 = f.add(1); - EXPECT_EQUAL(ref.ref(), ref2.ref()); + EntryRef ref = add(1); + EntryRef ref2 = add(1); + EXPECT_EQ(ref.ref(), ref2.ref()); // Note: The first buffer have the first element reserved -> we expect 2 elements used here. - TEST_DO(f.assertBufferState(ref, MemStats().used(2).hold(0).dead(1))); - f.store.remove(ref); - TEST_DO(f.assertBufferState(ref, MemStats().used(2).hold(0).dead(1))); - f.store.remove(ref); - TEST_DO(f.assertBufferState(ref, MemStats().used(2).hold(1).dead(1))); + assertBufferState(ref, MemStats().used(2).hold(0).dead(1)); + store.remove(ref); + assertBufferState(ref, MemStats().used(2).hold(0).dead(1)); + store.remove(ref); + assertBufferState(ref, MemStats().used(2).hold(1).dead(1)); } -TEST_F("require that new underlying buffer is allocated when current is full", SmallOffsetNumberFixture) +TEST_F(SmallOffsetNumberTest, new_underlying_buffer_is_allocated_when_current_is_full) { - uint32_t firstBufferId = f.getBufferId(f.add(1)); - for (uint32_t i = 0; i < (F1::EntryRefType::offsetSize() - 2); ++i) { - uint32_t bufferId = f.getBufferId(f.add(i + 2)); - EXPECT_EQUAL(firstBufferId, bufferId); + uint32_t firstBufferId = getBufferId(add(1)); + for (uint32_t i = 0; i < (SmallOffsetNumberTest::EntryRefType::offsetSize() - 2); ++i) { + uint32_t bufferId = getBufferId(add(i + 2)); + EXPECT_EQ(firstBufferId, bufferId); } - TEST_DO(f.assertStoreContent()); + assertStoreContent(); - uint32_t bias = F1::EntryRefType::offsetSize(); - uint32_t secondBufferId = f.getBufferId(f.add(bias + 1)); - EXPECT_NOT_EQUAL(firstBufferId, secondBufferId); + uint32_t bias = SmallOffsetNumberTest::EntryRefType::offsetSize(); + uint32_t secondBufferId = getBufferId(add(bias + 1)); + EXPECT_NE(firstBufferId, secondBufferId); for (uint32_t i = 0; i < 10u; ++i) { - uint32_t bufferId = f.getBufferId(f.add(bias + i + 2)); - EXPECT_EQUAL(secondBufferId, bufferId); + uint32_t bufferId = getBufferId(add(bias + i + 2)); + EXPECT_EQ(secondBufferId, bufferId); } - TEST_DO(f.assertStoreContent()); + assertStoreContent(); } -TEST_F("require that compaction works", NumberFixture) +TEST_F(NumberTest, store_can_be_compacted) { - EntryRef val1Ref = f.add(1); - EntryRef val2Ref = f.add(2); - f.remove(f.add(4)); - f.trimHoldLists(); - TEST_DO(f.assertBufferState(val1Ref, MemStats().used(4).dead(2))); // Note: First element is reserved - uint32_t val1BufferId = f.getBufferId(val1Ref); + EntryRef val1Ref = add(1); + EntryRef val2Ref = add(2); + remove(add(4)); + trimHoldLists(); + assertBufferState(val1Ref, MemStats().used(4).dead(2)); // Note: First element is reserved + uint32_t val1BufferId = getBufferId(val1Ref); - EXPECT_EQUAL(2u, f.refStore.size()); - f.compactWorst(); - EXPECT_EQUAL(2u, f.refStore.size()); - TEST_DO(f.assertStoreContent()); + EXPECT_EQ(2u, refStore.size()); + compactWorst(); + EXPECT_EQ(2u, refStore.size()); + assertStoreContent(); // Buffer has been compacted - EXPECT_NOT_EQUAL(val1BufferId, f.getBufferId(f.getEntryRef(1))); + EXPECT_NE(val1BufferId, getBufferId(getEntryRef(1))); // Old ref should still point to data. - f.assertGet(val1Ref, 1); - f.assertGet(val2Ref, 2); - EXPECT_TRUE(f.store.bufferState(val1Ref).isOnHold()); - f.trimHoldLists(); - EXPECT_TRUE(f.store.bufferState(val1Ref).isFree()); - TEST_DO(f.assertStoreContent()); + assertGet(val1Ref, 1); + assertGet(val2Ref, 2); + EXPECT_TRUE(store.bufferState(val1Ref).isOnHold()); + trimHoldLists(); + EXPECT_TRUE(store.bufferState(val1Ref).isFree()); + assertStoreContent(); } -TEST_F("require that builder works", NumberFixture) +TEST_F(NumberTest, store_can_be_instantiated_with_builder) { - auto builder = f.getBuilder(2); + auto builder = getBuilder(2); builder.add(10); builder.add(20); builder.setupRefCounts(); EntryRef val10Ref = builder.mapEnumValueToEntryRef(1); EntryRef val20Ref = builder.mapEnumValueToEntryRef(2); - TEST_DO(f.assertBufferState(val10Ref, MemStats().used(3).dead(1))); // Note: First element is reserved + assertBufferState(val10Ref, MemStats().used(3).dead(1)); // Note: First element is reserved EXPECT_TRUE(val10Ref.valid()); EXPECT_TRUE(val20Ref.valid()); - EXPECT_NOT_EQUAL(val10Ref.ref(), val20Ref.ref()); - f.assertGet(val10Ref, 10); - f.assertGet(val20Ref, 20); + EXPECT_NE(val10Ref.ref(), val20Ref.ref()); + assertGet(val10Ref, 10); + assertGet(val20Ref, 20); builder.makeDictionary(); // Align refstore with the two entries added by builder. - f.alignRefStore(val10Ref, 10, 1); - f.alignRefStore(val20Ref, 20, 1); - EXPECT_EQUAL(val10Ref.ref(), f.add(10).ref()); - EXPECT_EQUAL(val20Ref.ref(), f.add(20).ref()); + alignRefStore(val10Ref, 10, 1); + alignRefStore(val20Ref, 20, 1); + EXPECT_EQ(val10Ref.ref(), add(10).ref()); + EXPECT_EQ(val20Ref.ref(), add(20).ref()); } -TEST_F("require that saver works", NumberFixture) +TEST_F(NumberTest, store_can_be_saved) { - EntryRef val10Ref = f.add(10); - EntryRef val20Ref = f.add(20); - f.remove(f.add(40)); - f.trimHoldLists(); + EntryRef val10Ref = add(10); + EntryRef val20Ref = add(20); + remove(add(40)); + trimHoldLists(); - auto saver = f.getSaver(); + auto saver = getSaver(); std::vector<uint32_t> refs; saver.foreach_key([&](EntryRef ref) { refs.push_back(ref.ref()); }); std::vector<uint32_t> expRefs; expRefs.push_back(val10Ref.ref()); expRefs.push_back(val20Ref.ref()); - EXPECT_EQUAL(expRefs, refs); + EXPECT_EQ(expRefs, refs); saver.enumerateValues(); uint32_t invalidEnum = saver.mapEntryRefToEnumValue(EntryRef()); uint32_t enumValue10 = saver.mapEntryRefToEnumValue(val10Ref); uint32_t enumValue20 = saver.mapEntryRefToEnumValue(val20Ref); - EXPECT_EQUAL(0u, invalidEnum); - EXPECT_EQUAL(1u, enumValue10); - EXPECT_EQUAL(2u, enumValue20); + EXPECT_EQ(0u, invalidEnum); + EXPECT_EQ(1u, enumValue10); + EXPECT_EQ(2u, enumValue20); } -TEST_MAIN() { TEST_RUN_ALL(); } +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/vespalib/src/vespa/vespalib/datastore/CMakeLists.txt b/vespalib/src/vespa/vespalib/datastore/CMakeLists.txt index 39306636b00..836fb33cdab 100644 --- a/vespalib/src/vespa/vespalib/datastore/CMakeLists.txt +++ b/vespalib/src/vespa/vespalib/datastore/CMakeLists.txt @@ -7,7 +7,6 @@ vespa_add_library(vespalib_vespalib_datastore OBJECT datastore.cpp datastorebase.cpp entryref.cpp - unique_store.cpp unique_store_dictionary.cpp DEPENDS ) diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store.cpp b/vespalib/src/vespa/vespalib/datastore/unique_store.cpp deleted file mode 100644 index 5c402933ae1..00000000000 --- a/vespalib/src/vespa/vespalib/datastore/unique_store.cpp +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "unique_store.h" -#include "datastore.hpp" -#include <vespa/vespalib/btree/btree.hpp> -#include <vespa/vespalib/btree/btreebuilder.hpp> -#include <vespa/vespalib/btree/btreeroot.hpp> -#include <vespa/vespalib/btree/btreenodeallocator.hpp> -#include <vespa/vespalib/btree/btreeiterator.hpp> -#include <vespa/vespalib/btree/btreenode.hpp> - -namespace search::btree { - -// Instantiate classes related to unique store dictionary btree - -using EntryRef = datastore::EntryRef; -using EntryComparatorWrapper = datastore::EntryComparatorWrapper; -using DictionaryTraits = BTreeTraits<32, 32, 7, true>; - -template class BTreeNodeDataWrap<uint32_t, DictionaryTraits::LEAF_SLOTS>; -template class BTreeNodeT<EntryRef, DictionaryTraits::INTERNAL_SLOTS>; -template class BTreeNodeTT<EntryRef, uint32_t, NoAggregated, DictionaryTraits::LEAF_SLOTS>; -template class BTreeNodeTT<EntryRef, EntryRef, NoAggregated, DictionaryTraits::INTERNAL_SLOTS>; -template class BTreeInternalNode<EntryRef, NoAggregated, DictionaryTraits::INTERNAL_SLOTS>; -template class BTreeLeafNode<EntryRef, uint32_t, NoAggregated, DictionaryTraits::LEAF_SLOTS>; -template class BTreeLeafNodeTemp<EntryRef, uint32_t, NoAggregated, DictionaryTraits::LEAF_SLOTS>; -template class BTreeRootBase<EntryRef, uint32_t, NoAggregated, DictionaryTraits::INTERNAL_SLOTS, DictionaryTraits::LEAF_SLOTS>; -template class BTreeRootT<EntryRef, uint32_t, NoAggregated, EntryComparatorWrapper, DictionaryTraits>; -template class BTreeRoot<EntryRef, uint32_t, NoAggregated, EntryComparatorWrapper, DictionaryTraits>; -template class BTree<EntryRef, uint32_t, NoAggregated, EntryComparatorWrapper, DictionaryTraits>; -template class BTreeBuilder<EntryRef, uint32_t, NoAggregated, DictionaryTraits::INTERNAL_SLOTS, DictionaryTraits::LEAF_SLOTS>; -template class BTreeNodeAllocator<EntryRef, uint32_t, NoAggregated, DictionaryTraits::INTERNAL_SLOTS, DictionaryTraits::LEAF_SLOTS>; -template class BTreeNodeStore<EntryRef, uint32_t, NoAggregated, DictionaryTraits::INTERNAL_SLOTS, DictionaryTraits::LEAF_SLOTS>; -template class BTreeIteratorBase<EntryRef, uint32_t, NoAggregated, DictionaryTraits::INTERNAL_SLOTS, DictionaryTraits::LEAF_SLOTS, DictionaryTraits::PATH_SIZE>; -template class BTreeConstIterator<EntryRef, uint32_t, NoAggregated, EntryComparatorWrapper, DictionaryTraits>; -template class BTreeIterator<EntryRef, uint32_t, NoAggregated, EntryComparatorWrapper, DictionaryTraits>; - -} diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store.h b/vespalib/src/vespa/vespalib/datastore/unique_store.h index e3b8da14141..8a7f0e50845 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store.h +++ b/vespalib/src/vespa/vespalib/datastore/unique_store.h @@ -9,16 +9,14 @@ #include "entry_comparator_wrapper.h" #include "unique_store_add_result.h" #include "unique_store_entry.h" +#include "unique_store_allocator.h" #include "unique_store_comparator.h" #include "unique_store_dictionary_base.h" #include "i_compaction_context.h" -#include "i_compactable.h" -#include <vespa/vespalib/util/array.h> -#include <vespa/vespalib/btree/btree.h> namespace search::datastore { -template <typename EntryT, typename RefT> +template <typename Allocator> class UniqueStoreBuilder; template <typename EntryT, typename RefT> @@ -28,43 +26,27 @@ class UniqueStoreSaver; * Datastore for unique values of type EntryT that is accessed via a * 32-bit EntryRef. */ -template <typename EntryT, typename RefT = EntryRefT<22> > -class UniqueStore : public ICompactable +template <typename EntryT, typename RefT = EntryRefT<22>, typename Compare = UniqueStoreComparator<EntryT, RefT>, typename Allocator = UniqueStoreAllocator<EntryT, RefT> > +class UniqueStore { public: using DataStoreType = DataStoreT<RefT>; using EntryType = EntryT; - using WrappedEntryType = UniqueStoreEntry<EntryType>; using RefType = RefT; using Saver = UniqueStoreSaver<EntryT, RefT>; - using Builder = UniqueStoreBuilder<EntryT, RefT>; - using Compare = UniqueStoreComparator<EntryType, RefType>; - using UniqueStoreBufferType = BufferType<WrappedEntryType>; - template <typename, typename> friend class UniqueStoreBuilder; + using Builder = UniqueStoreBuilder<Allocator>; private: - DataStoreType _store; - UniqueStoreBufferType _typeHandler; - uint32_t _typeId; + Allocator _allocator; + DataStoreType &_store; std::unique_ptr<UniqueStoreDictionaryBase> _dict; using generation_t = vespalib::GenerationHandler::generation_t; - EntryRef allocate(const EntryType &value); - void hold(EntryRef ref); public: UniqueStore(); ~UniqueStore(); - EntryRef move(EntryRef ref) override; UniqueStoreAddResult add(const EntryType &value); EntryRef find(const EntryType &value); - const WrappedEntryType &getWrapped(EntryRef ref) const - { - RefType iRef(ref); - return *_store.template getEntry<WrappedEntryType>(iRef); - } - const EntryType &get(EntryRef ref) const - { - return getWrapped(ref).value(); - } + const EntryType &get(EntryRef ref) const { return _allocator.get(ref); } void remove(EntryRef ref); ICompactionContext::UP compactWorst(); vespalib::MemoryUsage getMemoryUsage() const; diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store.hpp b/vespalib/src/vespa/vespalib/datastore/unique_store.hpp index db64fb406d7..b8092e25255 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store.hpp +++ b/vespalib/src/vespa/vespalib/datastore/unique_store.hpp @@ -6,6 +6,7 @@ #include "unique_store_dictionary.h" #include "datastore.hpp" #include <vespa/vespalib/util/bufferwriter.h> +#include "unique_store_allocator.hpp" #include "unique_store_builder.hpp" #include "unique_store_saver.hpp" #include <atomic> @@ -13,74 +14,48 @@ namespace search::datastore { -constexpr size_t NUM_ARRAYS_FOR_NEW_UNIQUESTORE_BUFFER = 1024u; -constexpr float ALLOC_GROW_FACTOR = 0.2; - -template <typename EntryT, typename RefT> -UniqueStore<EntryT, RefT>::UniqueStore() - : ICompactable(), - _store(), - _typeHandler(1, 2u, RefT::offsetSize(), NUM_ARRAYS_FOR_NEW_UNIQUESTORE_BUFFER, ALLOC_GROW_FACTOR), - _typeId(0), +template <typename EntryT, typename RefT, typename Compare, typename Allocator> +UniqueStore<EntryT, RefT, Compare, Allocator>::UniqueStore() + : _allocator(), + _store(_allocator.getDataStore()), _dict(std::make_unique<UniqueStoreDictionary>()) { - _typeId = _store.addType(&_typeHandler); - assert(_typeId == 0u); - _store.initActiveBuffers(); } -template <typename EntryT, typename RefT> -UniqueStore<EntryT, RefT>::~UniqueStore() -{ - _store.clearHoldLists(); - _store.dropBuffers(); -} +template <typename EntryT, typename RefT, typename Compare, typename Allocator> +UniqueStore<EntryT, RefT, Compare, Allocator>::~UniqueStore() = default; -template <typename EntryT, typename RefT> -EntryRef -UniqueStore<EntryT, RefT>::allocate(const EntryType &value) -{ - return _store.template allocator<WrappedEntryType>(_typeId).alloc(value).ref; -} - -template <typename EntryT, typename RefT> -void -UniqueStore<EntryT, RefT>::hold(EntryRef ref) -{ - _store.holdElem(ref, 1); -} - -template <typename EntryT, typename RefT> +template <typename EntryT, typename RefT, typename Compare, typename Allocator> UniqueStoreAddResult -UniqueStore<EntryT, RefT>::add(const EntryType &value) +UniqueStore<EntryT, RefT, Compare, Allocator>::add(const EntryType &value) { Compare comp(_store, value); - return _dict->add(comp, [this, &value]() -> EntryRef { return allocate(value); }); + UniqueStoreAddResult result = _dict->add(comp, [this, &value]() -> EntryRef { return _allocator.allocate(value); }); + _allocator.getWrapped(result.ref()).inc_ref_count(); + return result; } -template <typename EntryT, typename RefT> +template <typename EntryT, typename RefT, typename Compare, typename Allocator> EntryRef -UniqueStore<EntryT, RefT>::find(const EntryType &value) +UniqueStore<EntryT, RefT, Compare, Allocator>::find(const EntryType &value) { Compare comp(_store, value); return _dict->find(comp); } -template <typename EntryT, typename RefT> -EntryRef -UniqueStore<EntryT, RefT>::move(EntryRef ref) -{ - return _store.template allocator<WrappedEntryType>(_typeId).alloc(getWrapped(ref)).ref; -} - -template <typename EntryT, typename RefT> +template <typename EntryT, typename RefT, typename Compare, typename Allocator> void -UniqueStore<EntryT, RefT>::remove(EntryRef ref) -{ - EntryType unused{}; - Compare comp(_store, unused); - if (_dict->remove(comp, ref)) { - hold(ref); +UniqueStore<EntryT, RefT, Compare, Allocator>::remove(EntryRef ref) +{ + auto &wrapped_entry = _allocator.getWrapped(ref); + if (wrapped_entry.get_ref_count() > 1u) { + wrapped_entry.dec_ref_count(); + } else { + EntryType unused{}; + Compare comp(_store, unused); + if (_dict->remove(comp, ref)) { + _allocator.hold(ref); + } } } @@ -172,73 +147,73 @@ public: } -template <typename EntryT, typename RefT> +template <typename EntryT, typename RefT, typename Compare, typename Allocator> ICompactionContext::UP -UniqueStore<EntryT, RefT>::compactWorst() +UniqueStore<EntryT, RefT, Compare, Allocator>::compactWorst() { std::vector<uint32_t> bufferIdsToCompact = _store.startCompactWorstBuffers(true, true); return std::make_unique<uniquestore::CompactionContext<RefT>> - (_store, *_dict, *this, std::move(bufferIdsToCompact)); + (_store, *_dict, _allocator, std::move(bufferIdsToCompact)); } -template <typename EntryT, typename RefT> +template <typename EntryT, typename RefT, typename Compare, typename Allocator> vespalib::MemoryUsage -UniqueStore<EntryT, RefT>::getMemoryUsage() const +UniqueStore<EntryT, RefT, Compare, Allocator>::getMemoryUsage() const { vespalib::MemoryUsage usage = _store.getMemoryUsage(); usage.merge(_dict->get_memory_usage()); return usage; } -template <typename EntryT, typename RefT> +template <typename EntryT, typename RefT, typename Compare, typename Allocator> const BufferState & -UniqueStore<EntryT, RefT>::bufferState(EntryRef ref) const +UniqueStore<EntryT, RefT, Compare, Allocator>::bufferState(EntryRef ref) const { RefT internalRef(ref); return _store.getBufferState(internalRef.bufferId()); } -template <typename EntryT, typename RefT> +template <typename EntryT, typename RefT, typename Compare, typename Allocator> void -UniqueStore<EntryT, RefT>::transferHoldLists(generation_t generation) +UniqueStore<EntryT, RefT, Compare, Allocator>::transferHoldLists(generation_t generation) { _dict->transfer_hold_lists(generation); _store.transferHoldLists(generation); } -template <typename EntryT, typename RefT> +template <typename EntryT, typename RefT, typename Compare, typename Allocator> void -UniqueStore<EntryT, RefT>::trimHoldLists(generation_t firstUsed) +UniqueStore<EntryT, RefT, Compare, Allocator>::trimHoldLists(generation_t firstUsed) { _dict->trim_hold_lists(firstUsed); _store.trimHoldLists(firstUsed); } -template <typename EntryT, typename RefT> +template <typename EntryT, typename RefT, typename Compare, typename Allocator> void -UniqueStore<EntryT, RefT>::freeze() +UniqueStore<EntryT, RefT, Compare, Allocator>::freeze() { _dict->freeze(); } -template <typename EntryT, typename RefT> -typename UniqueStore<EntryT, RefT>::Builder -UniqueStore<EntryT, RefT>::getBuilder(uint32_t uniqueValuesHint) +template <typename EntryT, typename RefT, typename Compare, typename Allocator> +typename UniqueStore<EntryT, RefT, Compare, Allocator>::Builder +UniqueStore<EntryT, RefT, Compare, Allocator>::getBuilder(uint32_t uniqueValuesHint) { - return Builder(*this, *_dict, uniqueValuesHint); + return Builder(_allocator, *_dict, uniqueValuesHint); } -template <typename EntryT, typename RefT> -typename UniqueStore<EntryT, RefT>::Saver -UniqueStore<EntryT, RefT>::getSaver() const +template <typename EntryT, typename RefT, typename Compare, typename Allocator> +typename UniqueStore<EntryT, RefT, Compare, Allocator>::Saver +UniqueStore<EntryT, RefT, Compare, Allocator>::getSaver() const { return Saver(*_dict, _store); } -template <typename EntryT, typename RefT> +template <typename EntryT, typename RefT, typename Compare, typename Allocator> uint32_t -UniqueStore<EntryT, RefT>::getNumUniques() const +UniqueStore<EntryT, RefT, Compare, Allocator>::getNumUniques() const { return _dict->get_num_uniques(); } diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_allocator.h b/vespalib/src/vespa/vespalib/datastore/unique_store_allocator.h new file mode 100644 index 00000000000..53f76d17569 --- /dev/null +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_allocator.h @@ -0,0 +1,48 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "datastore.h" +#include "entryref.h" +#include "unique_store_add_result.h" +#include "unique_store_entry.h" +#include "i_compactable.h" + +namespace search::datastore { + +/** + * Allocator for unique values of type EntryT that is accessed via a + * 32-bit EntryRef. + */ +template <typename EntryT, typename RefT = EntryRefT<22> > +class UniqueStoreAllocator : public ICompactable +{ +public: + using DataStoreType = DataStoreT<RefT>; + using EntryType = EntryT; + using WrappedEntryType = UniqueStoreEntry<EntryType>; + using RefType = RefT; + using UniqueStoreBufferType = BufferType<WrappedEntryType>; +private: + DataStoreType _store; + UniqueStoreBufferType _typeHandler; + +public: + UniqueStoreAllocator(); + ~UniqueStoreAllocator() override; + EntryRef allocate(const EntryType& value); + void hold(EntryRef ref); + EntryRef move(EntryRef ref) override; + const WrappedEntryType& getWrapped(EntryRef ref) const + { + RefType iRef(ref); + return *_store.template getEntry<WrappedEntryType>(iRef); + } + const EntryType& get(EntryRef ref) const + { + return getWrapped(ref).value(); + } + DataStoreType& getDataStore() { return _store; } +}; + +} diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_allocator.hpp b/vespalib/src/vespa/vespalib/datastore/unique_store_allocator.hpp new file mode 100644 index 00000000000..0849fc5129b --- /dev/null +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_allocator.hpp @@ -0,0 +1,52 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "unique_store_allocator.h" +#include "datastore.hpp" + +namespace search::datastore { + +constexpr size_t NUM_ARRAYS_FOR_NEW_UNIQUESTORE_BUFFER = 1024u; +constexpr float ALLOC_GROW_FACTOR = 0.2; + +template <typename EntryT, typename RefT> +UniqueStoreAllocator<EntryT, RefT>::UniqueStoreAllocator() + : ICompactable(), + _store(), + _typeHandler(1, 2u, RefT::offsetSize(), NUM_ARRAYS_FOR_NEW_UNIQUESTORE_BUFFER, ALLOC_GROW_FACTOR) +{ + auto typeId = _store.addType(&_typeHandler); + assert(typeId == 0u); + _store.initActiveBuffers(); +} + +template <typename EntryT, typename RefT> +UniqueStoreAllocator<EntryT, RefT>::~UniqueStoreAllocator() +{ + _store.clearHoldLists(); + _store.dropBuffers(); +} + +template <typename EntryT, typename RefT> +EntryRef +UniqueStoreAllocator<EntryT, RefT>::allocate(const EntryType& value) +{ + return _store.template allocator<WrappedEntryType>(0).alloc(value).ref; +} + +template <typename EntryT, typename RefT> +void +UniqueStoreAllocator<EntryT, RefT>::hold(EntryRef ref) +{ + _store.holdElem(ref, 1); +} + +template <typename EntryT, typename RefT> +EntryRef +UniqueStoreAllocator<EntryT, RefT>::move(EntryRef ref) +{ + return _store.template allocator<WrappedEntryType>(0).alloc(getWrapped(ref)).ref; +} + +} diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_builder.h b/vespalib/src/vespa/vespalib/datastore/unique_store_builder.h index 0e3416c14aa..58ecf61eb82 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store_builder.h +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_builder.h @@ -2,10 +2,12 @@ #pragma once -#include "unique_store.h" +#include "unique_store_allocator.h" namespace search::datastore { +class UniqueStoreDictionaryBase; + /** * Builder for related UniqueStore class. * @@ -13,23 +15,21 @@ namespace search::datastore { * from enum value to EntryRef value. New unique values must be added * in sorted order. */ -template <typename EntryT, typename RefT> +template <typename Allocator> class UniqueStoreBuilder { - using UniqueStoreType = UniqueStore<EntryT, RefT>; - using EntryType = EntryT; + using EntryType = typename Allocator::EntryType; - UniqueStoreType &_store; + Allocator &_allocator; UniqueStoreDictionaryBase &_dict; std::vector<EntryRef> _refs; std::vector<uint32_t> _refCounts; public: - UniqueStoreBuilder(UniqueStoreType &store, - UniqueStoreDictionaryBase &dict, uint32_t uniqueValuesHint); + UniqueStoreBuilder(Allocator& allocator, UniqueStoreDictionaryBase& dict, uint32_t uniqueValuesHint); ~UniqueStoreBuilder(); void setupRefCounts(); void makeDictionary(); - void add(const EntryType &value) { - EntryRef newRef = _store.allocate(value); + void add(const EntryType& value) { + EntryRef newRef = _allocator.allocate(value); _refs.push_back(newRef); } EntryRef mapEnumValueToEntryRef(uint32_t enumValue) { diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_builder.hpp b/vespalib/src/vespa/vespalib/datastore/unique_store_builder.hpp index a0f1256a23a..e63da8d80c6 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store_builder.hpp +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_builder.hpp @@ -3,19 +3,14 @@ #pragma once #include "unique_store_builder.h" +#include "unique_store_dictionary_base.h" #include "datastore.hpp" -#include <vespa/vespalib/btree/btree.h> -#include <vespa/vespalib/btree/btreebuilder.h> -#include <vespa/vespalib/btree/btreeroot.h> -#include <vespa/vespalib/btree/btreenodeallocator.h> -#include <vespa/vespalib/btree/btreeiterator.h> -#include <vespa/vespalib/btree/btreenode.h> namespace search::datastore { -template <typename EntryT, typename RefT> -UniqueStoreBuilder<EntryT, RefT>::UniqueStoreBuilder(UniqueStoreType &store, UniqueStoreDictionaryBase &dict, uint32_t uniqueValuesHint) - : _store(store), +template <typename Allocator> +UniqueStoreBuilder<Allocator>::UniqueStoreBuilder(Allocator& allocator, UniqueStoreDictionaryBase& dict, uint32_t uniqueValuesHint) + : _allocator(allocator), _dict(dict), _refs(), _refCounts() @@ -24,24 +19,29 @@ UniqueStoreBuilder<EntryT, RefT>::UniqueStoreBuilder(UniqueStoreType &store, Uni _refs.push_back(EntryRef()); } -template <typename EntryT, typename RefT> -UniqueStoreBuilder<EntryT, RefT>::~UniqueStoreBuilder() +template <typename Allocator> +UniqueStoreBuilder<Allocator>::~UniqueStoreBuilder() { } -template <typename EntryT, typename RefT> +template <typename Allocator> void -UniqueStoreBuilder<EntryT, RefT>::setupRefCounts() +UniqueStoreBuilder<Allocator>::setupRefCounts() { _refCounts.resize(_refs.size()); } - -template <typename EntryT, typename RefT> +template <typename Allocator> void -UniqueStoreBuilder<EntryT, RefT>::makeDictionary() +UniqueStoreBuilder<Allocator>::makeDictionary() { - _dict.build(_refs, _refCounts, [this](EntryRef ref) { _store.hold(ref); }); + auto ref_count_itr = _refCounts.cbegin(); + for (auto ref : _refs) { + auto& wrapped_entry = _allocator.getWrapped(ref); + wrapped_entry.set_ref_count(*ref_count_itr); + ++ref_count_itr; + } + _dict.build(_refs, _refCounts, [this](EntryRef ref) { _allocator.hold(ref); }); } } diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.cpp b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.cpp index c562dce090e..b9af208263c 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.cpp +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.cpp @@ -4,7 +4,13 @@ #include "unique_store_add_result.h" #include "entry_comparator_wrapper.h" #include "i_compactable.h" -#include <vespa/vespalib/btree/btreebuilder.h> +#include "datastore.hpp" +#include <vespa/vespalib/btree/btree.hpp> +#include <vespa/vespalib/btree/btreebuilder.hpp> +#include <vespa/vespalib/btree/btreeroot.hpp> +#include <vespa/vespalib/btree/btreenodeallocator.hpp> +#include <vespa/vespalib/btree/btreeiterator.hpp> +#include <vespa/vespalib/btree/btreenode.hpp> namespace search::datastore { @@ -40,14 +46,11 @@ UniqueStoreDictionary::add(const EntryComparator &comp, { auto itr = _dict.lowerBound(EntryRef(), comp); if (itr.valid() && !comp(EntryRef(), itr.getKey())) { - uint32_t refCount = itr.getData(); - assert(refCount != std::numeric_limits<uint32_t>::max()); - itr.writeData(refCount + 1); return UniqueStoreAddResult(itr.getKey(), false); } else { EntryRef newRef = insertEntry(); - _dict.insert(itr, newRef, 1u); + _dict.insert(itr, newRef, btree::BTreeNoLeafData()); return UniqueStoreAddResult(newRef, true); } } @@ -69,14 +72,8 @@ UniqueStoreDictionary::remove(const EntryComparator &comp, EntryRef ref) assert(ref.valid()); auto itr = _dict.lowerBound(ref, comp); if (itr.valid() && itr.getKey() == ref) { - uint32_t refCount = itr.getData(); - if (refCount > 1) { - itr.writeData(refCount - 1); - return false; - } else { - _dict.remove(itr); - return true; - } + _dict.remove(itr); + return true; } return false; } @@ -116,7 +113,7 @@ UniqueStoreDictionary::build(const std::vector<EntryRef> &refs, const std::vecto typename Dictionary::Builder builder(_dict.getAllocator()); for (size_t i = 1; i < refs.size(); ++i) { if (ref_counts[i] != 0u) { - builder.insert(refs[i], ref_counts[i]); + builder.insert(refs[i], btree::BTreeNoLeafData()); } else { hold(refs[i]); } diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h index 576554e842e..4fa71c3f9a9 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h @@ -16,7 +16,7 @@ class UniqueStoreDictionary : public UniqueStoreDictionaryBase { public: using DictionaryTraits = btree::BTreeTraits<32, 32, 7, true>; - using Dictionary = btree::BTree<EntryRef, uint32_t, + using Dictionary = btree::BTree<EntryRef, btree::BTreeNoLeafData, btree::NoAggregated, EntryComparatorWrapper, DictionaryTraits>; diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_entry_base.h b/vespalib/src/vespa/vespalib/datastore/unique_store_entry_base.h index 49962ffc5c0..a048abb9126 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store_entry_base.h +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_entry_base.h @@ -10,9 +10,18 @@ namespace search::datastore { * Class containing common metadata for entries in unique store. */ class UniqueStoreEntryBase { + mutable uint32_t _ref_count; protected: - UniqueStoreEntryBase() {} + UniqueStoreEntryBase() + : _ref_count(0u) + { + } ~UniqueStoreEntryBase() = default; +public: + uint32_t get_ref_count() const { return _ref_count; } + void set_ref_count(uint32_t ref_count) const { _ref_count = ref_count; } + void inc_ref_count() const { ++_ref_count; } + void dec_ref_count() const { --_ref_count; } }; } |