From 44c80f8df773c4661fbf08d7d6a896bf52c41f60 Mon Sep 17 00:00:00 2001 From: Tor Egge Date: Sat, 3 Sep 2022 14:41:47 +0200 Subject: Let attribute aspect delayer adjust summary config. --- .../attribute_aspect_delayer_test.cpp | 76 ++++- .../proton/attribute/attribute_aspect_delayer.cpp | 378 +++++++++++++++------ .../proton/attribute/attribute_aspect_delayer.h | 3 + .../searchcore/proton/server/documentdbconfig.cpp | 12 +- 4 files changed, 342 insertions(+), 127 deletions(-) diff --git a/searchcore/src/tests/proton/attribute/attribute_aspect_delayer/attribute_aspect_delayer_test.cpp b/searchcore/src/tests/proton/attribute/attribute_aspect_delayer/attribute_aspect_delayer_test.cpp index 0ce67e8bfcb..4ff459a5a9d 100644 --- a/searchcore/src/tests/proton/attribute/attribute_aspect_delayer/attribute_aspect_delayer_test.cpp +++ b/searchcore/src/tests/proton/attribute/attribute_aspect_delayer/attribute_aspect_delayer_test.cpp @@ -33,6 +33,14 @@ ostream &operator<<(ostream &os, const SummarymapConfig::Override &override) { } +namespace vespa::config::search::internal { + +std::ostream &operator<<(std::ostream &os, const SummaryConfig::Classes::Fields &field) { + return os << "{name=" << field.name << ", type=" << field.type << ", command=" << field.command << ", source=" << field.source << "}"; +} + +} + namespace proton { namespace { @@ -159,6 +167,16 @@ SummaryConfig::Classes::Fields make_summary_field(const vespalib::string &name, return field; } +SummaryConfig::Classes::Fields make_summary_field(const vespalib::string &name, const vespalib::string &type, const vespalib::string& command, const vespalib::string& source) +{ + SummaryConfig::Classes::Fields field; + field.name = name; + field.type = type; + field.command = command; + field.source = source; + return field; +} + SummaryConfig sCfg(std::vector fields) { SummaryConfigBuilder result; @@ -235,6 +253,12 @@ public: auto summarymapConfig = _delayer.getSummarymapConfig(); EXPECT_EQ(exp, summarymapConfig->override); } + void assertSummaryConfig(const std::vector &exp) + { + auto summaryConfig = _delayer.getSummaryConfig(); + ASSERT_EQ(1, summaryConfig->classes.size()); + EXPECT_EQ(exp, summaryConfig->classes[0].fields); + } }; TEST_F(DelayerTest, require_that_empty_config_is_ok) @@ -242,44 +266,50 @@ TEST_F(DelayerTest, require_that_empty_config_is_ok) setup(attrCfg({}), smCfg({}), attrCfg({}), sCfg({}), smCfg({})); assertAttributeConfig({}); assertSummarymapConfig({}); + assertSummaryConfig({}); } TEST_F(DelayerTest, require_that_simple_attribute_config_is_ok) { - setup(attrCfg({make_int32_sv_cfg()}), smCfg({make_attribute_override("a")}), attrCfg({make_int32_sv_cfg()}), sCfg({make_summary_field("a", "integer")}), smCfg({make_attribute_override("a")})); + setup(attrCfg({make_int32_sv_cfg()}), smCfg({make_attribute_override("a")}), attrCfg({make_int32_sv_cfg()}), sCfg({make_summary_field("a", "integer", "attribute", "a")}), smCfg({make_attribute_override("a")})); assertAttributeConfig({make_int32_sv_cfg()}); assertSummarymapConfig({make_attribute_override("a")}); + assertSummaryConfig({make_summary_field("a", "integer", "attribute", "a")}); } TEST_F(DelayerTest, require_that_adding_attribute_aspect_is_delayed_if_field_type_is_unchanged) { addFields({"a"}); - setup(attrCfg({}), smCfg({}), attrCfg({make_int32_sv_cfg()}), sCfg({make_summary_field("a", "integer")}), smCfg({make_attribute_override("a")})); + setup(attrCfg({}), smCfg({}), attrCfg({make_int32_sv_cfg()}), sCfg({make_summary_field("a", "integer", "attribute", "a")}), smCfg({make_attribute_override("a")})); assertAttributeConfig({}); assertSummarymapConfig({}); + assertSummaryConfig({make_summary_field("a", "integer")}); } TEST_F(DelayerTest, require_that_adding_attribute_aspect_is_delayed_if_field_type_is_unchanged_geopos_override) { addFields({"a"}); - setup(attrCfg({}), smCfg({}), attrCfg({make_int32_sv_cfg()}), sCfg({make_summary_field("a", "integer")}), smCfg({make_geopos_override("a")})); + setup(attrCfg({}), smCfg({}), attrCfg({make_int32_sv_cfg()}), sCfg({make_summary_field("a", "integer", "geopos", "a")}), smCfg({make_geopos_override("a")})); assertAttributeConfig({}); assertSummarymapConfig({make_geopos_override("a")}); + assertSummaryConfig({make_summary_field("a", "integer", "geopos", "a")}); } TEST_F(DelayerTest, require_that_adding_attribute_aspect_is_delayed_if_field_type_is_unchanged_mapped_summary) { addFields({"a"}); - setup(attrCfg({}), smCfg({}), attrCfg({make_int32_sv_cfg()}), sCfg({make_summary_field("a_mapped", "integer")}), smCfg({make_attribute_override("a_mapped", "a")})); + setup(attrCfg({}), smCfg({}), attrCfg({make_int32_sv_cfg()}), sCfg({make_summary_field("a_mapped", "integer", "attribute", "a")}), smCfg({make_attribute_override("a_mapped", "a")})); assertAttributeConfig({}); assertSummarymapConfig({}); + assertSummaryConfig({make_summary_field("a_mapped", "integer", "copy", "a")}); } TEST_F(DelayerTest, require_that_adding_attribute_is_not_delayed_if_field_type_changed) { - setup(attrCfg({}), smCfg({}), attrCfg({make_int32_sv_cfg()}), sCfg({make_summary_field("a", "integer")}), smCfg({make_attribute_override("a")})); + setup(attrCfg({}), smCfg({}), attrCfg({make_int32_sv_cfg()}), sCfg({make_summary_field("a", "integer", "attribute", "a")}), smCfg({make_attribute_override("a")})); assertAttributeConfig({make_int32_sv_cfg()}); assertSummarymapConfig({make_attribute_override("a")}); + assertSummaryConfig({make_summary_field("a", "integer", "attribute", "a")}); } TEST_F(DelayerTest, require_that_removing_attribute_aspect_is_delayed_if_field_type_is_unchanged) @@ -288,6 +318,7 @@ TEST_F(DelayerTest, require_that_removing_attribute_aspect_is_delayed_if_field_t setup(attrCfg({make_int32_sv_cfg()}), smCfg({make_attribute_override("a")}), attrCfg({}), sCfg({make_summary_field("a", "integer")}), smCfg({})); assertAttributeConfig({make_int32_sv_cfg()}); assertSummarymapConfig({make_attribute_override("a")}); + assertSummaryConfig({make_summary_field("a", "integer", "attribute", "a")}); } TEST_F(DelayerTest, require_that_summary_map_override_is_removed_when_summary_aspect_is_removed_even_if_removing_attribute_aspect_is_delayed) @@ -296,6 +327,7 @@ TEST_F(DelayerTest, require_that_summary_map_override_is_removed_when_summary_as setup(attrCfg({make_int32_sv_cfg()}), smCfg({make_attribute_override("a")}), attrCfg({}), sCfg({}), smCfg({})); assertAttributeConfig({make_int32_sv_cfg()}); assertSummarymapConfig({}); + assertSummaryConfig({}); } TEST_F(DelayerTest, require_that_removing_attribute_aspect_is_delayed_if_field_type_is_unchanged_gepos_override) @@ -304,6 +336,7 @@ TEST_F(DelayerTest, require_that_removing_attribute_aspect_is_delayed_if_field_t setup(attrCfg({make_int32_sv_cfg()}), smCfg({make_geopos_override("a")}), attrCfg({}), sCfg({}), smCfg({})); assertAttributeConfig({make_int32_sv_cfg()}); assertSummarymapConfig({}); + assertSummaryConfig({}); } TEST_F(DelayerTest, require_that_removing_attribute_aspect_is_not_delayed_if_field_type_changed) @@ -311,6 +344,7 @@ TEST_F(DelayerTest, require_that_removing_attribute_aspect_is_not_delayed_if_fie setup(attrCfg({make_int32_sv_cfg()}), smCfg({make_attribute_override("a")}), attrCfg({}), sCfg({make_summary_field("a", "integer")}), smCfg({})); assertAttributeConfig({}); assertSummarymapConfig({}); + assertSummaryConfig({make_summary_field("a", "integer")}); } TEST_F(DelayerTest, require_that_removing_attribute_aspect_is_not_delayed_if_also_indexed) @@ -320,15 +354,17 @@ TEST_F(DelayerTest, require_that_removing_attribute_aspect_is_not_delayed_if_als setup(attrCfg({make_string_sv_cfg()}), smCfg({make_attribute_override("a")}), attrCfg({}), sCfg({make_summary_field("a", "string")}), smCfg({})); assertAttributeConfig({}); assertSummarymapConfig({}); + assertSummaryConfig({make_summary_field("a", "string")}); } TEST_F(DelayerTest, require_that_adding_attribute_aspect_is_delayed_for_tensor_field) { addFields({"a"}); setup(attrCfg({}), smCfg({}), - attrCfg({make_tensor_cfg("tensor(x[10])")}), sCfg({make_summary_field("a", "tensor")}), smCfg({make_attribute_override("a")})); + attrCfg({make_tensor_cfg("tensor(x[10])")}), sCfg({make_summary_field("a", "tensor", "attribute", "a")}), smCfg({make_attribute_override("a")})); assertAttributeConfig({}); assertSummarymapConfig({}); + assertSummaryConfig({make_summary_field("a", "tensor")}); } TEST_F(DelayerTest, require_that_removing_attribute_aspect_is_delayed_for_tensor_field) @@ -338,6 +374,7 @@ TEST_F(DelayerTest, require_that_removing_attribute_aspect_is_delayed_for_tensor attrCfg({}), sCfg({make_summary_field("a", "tensor")}), smCfg({})); assertAttributeConfig({make_tensor_cfg("tensor(x[10])")}); assertSummarymapConfig({make_attribute_override("a")}); + assertSummaryConfig({make_summary_field("a", "tensor", "attribute", "a")}); } TEST_F(DelayerTest, require_that_removing_attribute_aspect_is_not_delayed_for_predicate) @@ -346,6 +383,7 @@ TEST_F(DelayerTest, require_that_removing_attribute_aspect_is_not_delayed_for_pr setup(attrCfg({make_predicate_cfg(4)}), smCfg({}), attrCfg({}), sCfg({make_summary_field("a", "string")}), smCfg({})); assertAttributeConfig({}); assertSummarymapConfig({}); + assertSummaryConfig({make_summary_field("a", "string")}); } TEST_F(DelayerTest, require_that_removing_attribute_aspect_is_not_delayed_for_reference) @@ -354,64 +392,72 @@ TEST_F(DelayerTest, require_that_removing_attribute_aspect_is_not_delayed_for_re setup(attrCfg({make_reference_cfg()}), smCfg({}), attrCfg({}), sCfg({make_summary_field("a", "longstring")}), smCfg({})); assertAttributeConfig({}); assertSummarymapConfig({}); + assertSummaryConfig({make_summary_field("a", "longstring")}); } TEST_F(DelayerTest, require_that_fast_access_flag_change_is_delayed_false_true_edge) { addFields({"a"}); - setup(attrCfg({make_int32_sv_cfg()}), smCfg({make_attribute_override("a")}), attrCfg({make_fa(make_int32_sv_cfg())}), sCfg({make_summary_field("a", "integer")}), smCfg({make_attribute_override("a")})); + setup(attrCfg({make_int32_sv_cfg()}), smCfg({make_attribute_override("a")}), attrCfg({make_fa(make_int32_sv_cfg())}), sCfg({make_summary_field("a", "integer", "attribute", "a")}), smCfg({make_attribute_override("a")})); assertAttributeConfig({make_int32_sv_cfg()}); assertSummarymapConfig({make_attribute_override("a")}); + assertSummaryConfig({make_summary_field("a", "integer", "attribute", "a")}); } TEST_F(DelayerTest, require_that_fast_access_flag_change_is_delayed_true_false_edge) { addFields({"a"}); - setup(attrCfg({make_fa(make_int32_sv_cfg())}), smCfg({make_attribute_override("a")}), attrCfg({make_int32_sv_cfg()}), sCfg({make_summary_field("a", "integer")}), smCfg({make_attribute_override("a")})); + setup(attrCfg({make_fa(make_int32_sv_cfg())}), smCfg({make_attribute_override("a")}), attrCfg({make_int32_sv_cfg()}), sCfg({make_summary_field("a", "integer", "attribute", "a")}), smCfg({make_attribute_override("a")})); assertAttributeConfig({make_fa(make_int32_sv_cfg())}); assertSummarymapConfig({make_attribute_override("a")}); + assertSummaryConfig({make_summary_field("a", "integer", "attribute", "a")}); } TEST_F(DelayerTest, require_that_fast_access_flag_change_is_delayed_false_true_edge_on_tensor_attribute) { addFields({"a"}); setup(attrCfg({make_tensor_cfg("tensor(x[10])")}), smCfg({make_attribute_override("a")}), - attrCfg({make_fa(make_tensor_cfg("tensor(x[10])"))}), sCfg({make_summary_field("a", "tensor")}), smCfg({make_attribute_override("a")})); + attrCfg({make_fa(make_tensor_cfg("tensor(x[10])"))}), sCfg({make_summary_field("a", "tensor", "attribute", "a")}), smCfg({make_attribute_override("a")})); assertAttributeConfig({make_tensor_cfg("tensor(x[10])")}); assertSummarymapConfig({make_attribute_override("a")}); + assertSummaryConfig({make_summary_field("a", "tensor", "attribute", "a")}); } TEST_F(DelayerTest, require_that_fast_access_flag_change_is_delayed_true_false_edge_on_tensor_attribute) { addFields({"a"}); setup(attrCfg({make_fa(make_tensor_cfg("tensor(x[10])"))}), smCfg({make_attribute_override("a")}), - attrCfg({make_tensor_cfg("tensor(x[10])")}), sCfg({make_summary_field("a", "tensor")}), smCfg({make_attribute_override("a")})); + attrCfg({make_tensor_cfg("tensor(x[10])")}), sCfg({make_summary_field("a", "tensor", "attribute", "a")}), smCfg({make_attribute_override("a")})); assertAttributeConfig({make_fa(make_tensor_cfg("tensor(x[10])"))}); assertSummarymapConfig({make_attribute_override("a")}); + assertSummaryConfig({make_summary_field("a", "tensor", "attribute", "a")}); } TEST_F(DelayerTest, require_that_fast_access_flag_change_is_not_delayed_true_false_edge_on_string_attribute_indexed_field) { addFields({"a"}); addOldIndexField("a"); - setup(attrCfg({make_fa(make_string_sv_cfg())}), smCfg({make_attribute_override("a")}), attrCfg({make_string_sv_cfg()}), sCfg({make_summary_field("a", "tensor")}), smCfg({make_attribute_override("a")})); + setup(attrCfg({make_fa(make_string_sv_cfg())}), smCfg({make_attribute_override("a")}), attrCfg({make_string_sv_cfg()}), sCfg({make_summary_field("a", "string", "attribute", "a")}), smCfg({make_attribute_override("a")})); assertAttributeConfig({make_string_sv_cfg()}); assertSummarymapConfig({make_attribute_override("a")}); + assertSummaryConfig({make_summary_field("a", "string", "attribute", "a")}); } TEST_F(DelayerTest, require_that_adding_attribute_aspect_to_struct_field_is_not_delayed_if_field_type_is_changed) { - setup(attrCfg({}), smCfg({}), attrCfg({make_int32_sv_cfg("array.a")}), sCfg({make_summary_field("array", "jsonstring")}), smCfg({make_attribute_combiner_override("array")})); + setup(attrCfg({}), smCfg({}), attrCfg({make_int32_sv_cfg("array.a")}), sCfg({make_summary_field("array", "jsonstring", "attributecombiner", "array")}), smCfg({make_attribute_combiner_override("array")})); assertAttributeConfig({make_int32_sv_cfg("array.a")}); assertSummarymapConfig({make_attribute_combiner_override("array")}); + assertSummaryConfig({make_summary_field("array", "jsonstring", "attributecombiner", "array")}); } TEST_F(DelayerTest, require_that_adding_attribute_aspect_to_struct_field_is_delayed_if_field_type_is_unchanged) { addFields({"array.a"}); - setup(attrCfg({}), smCfg({}), attrCfg({make_int32_sv_cfg("array.a")}), sCfg({make_summary_field("array", "jsonstring")}), smCfg({make_attribute_combiner_override("array")})); + setup(attrCfg({}), smCfg({}), attrCfg({make_int32_sv_cfg("array.a")}), sCfg({make_summary_field("array", "jsonstring", "attributecombiner", "array")}), smCfg({make_attribute_combiner_override("array")})); assertAttributeConfig({}); assertSummarymapConfig({}); + assertSummaryConfig({make_summary_field("array", "jsonstring")}); } TEST_F(DelayerTest, require_that_removing_attribute_aspect_from_struct_field_is_not_delayed) @@ -420,14 +466,16 @@ TEST_F(DelayerTest, require_that_removing_attribute_aspect_from_struct_field_is_ setup(attrCfg({make_int32_sv_cfg("array.a")}), smCfg({make_attribute_combiner_override("array")}), attrCfg({}), sCfg({make_summary_field("array", "jsonstring")}), smCfg({})); assertAttributeConfig({}); assertSummarymapConfig({}); + assertSummaryConfig({make_summary_field("array", "jsonstring")}); } TEST_F(DelayerTest, require_that_adding_attribute_aspect_to_struct_field_is_delayed_if_field_type_is_unchanged_with_filtering_docsum) { addFields({"array.a"}); - setup(attrCfg({}), smCfg({}), attrCfg({make_int32_sv_cfg("array.a")}), sCfg({make_summary_field("array", "jsonstring")}), smCfg({make_attribute_combiner_override("array"), make_matched_attribute_elements_filter_override("array_filtered", "array")})); + setup(attrCfg({}), smCfg({}), attrCfg({make_int32_sv_cfg("array.a")}), sCfg({make_summary_field("array", "jsonstring", "attributecombiner", "array"), make_summary_field("array_filtered", "jsonstring", "matchedattributeelementsfilter", "array")}), smCfg({make_attribute_combiner_override("array"), make_matched_attribute_elements_filter_override("array_filtered", "array")})); assertAttributeConfig({}); assertSummarymapConfig({make_matched_elements_filter_override("array_filtered", "array")}); + assertSummaryConfig({make_summary_field("array", "jsonstring"), make_summary_field("array_filtered", "jsonstring", "matchedelementsfilter", "array")}); } } diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_aspect_delayer.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_aspect_delayer.cpp index 6e935a577fd..946914fe7bb 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_aspect_delayer.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_aspect_delayer.cpp @@ -18,6 +18,7 @@ using search::attribute::ConfigConverter; using vespa::config::search::AttributesConfig; using vespa::config::search::AttributesConfigBuilder; using vespa::config::search::SummaryConfig; +using vespa::config::search::SummaryConfigBuilder; using vespa::config::search::SummarymapConfig; using vespa::config::search::SummarymapConfigBuilder; @@ -25,6 +26,12 @@ namespace proton { namespace { +vespalib::string attribute_combiner_dfw_string("attributecombiner"); +vespalib::string matched_attribute_elements_filter_dfw_string("matchedattributeelementsfilter"); +vespalib::string matched_elements_filter_dfw_string("matchedelementsfilter"); +vespalib::string copy_dfw_string("copy"); +vespalib::string attribute_dfw_string("attribute"); + using AttributesConfigHash = ConfigHash; bool willTriggerReprocessOnAttributeAspectRemoval(const search::attribute::Config &cfg, @@ -68,144 +75,296 @@ vespalib::string source_field(const SummarymapConfig::Override &override) { } } +vespalib::string +source_field(const SummaryConfig::Classes::Fields& summary_field) +{ + if (summary_field.source == "") { + return summary_field.name; + } else { + return summary_field.source; + } } -AttributeAspectDelayer::AttributeAspectDelayer() - : _attributesConfig(std::make_shared()), - _summarymapConfig(std::make_shared()) +void +remove_docsum_field_rewriter(SummaryConfig::Classes::Fields& summary_field) { + if (source_field(summary_field) != summary_field.name) { + summary_field.command = copy_dfw_string; + } else { + summary_field.command = ""; + summary_field.source = ""; + } } -AttributeAspectDelayer::~AttributeAspectDelayer() +class AttributeAspectConfigRewriter +{ + const AttributesConfig& _old_attributes_config; + const AttributesConfig& _new_attributes_config; + AttributesConfigHash _old_attributes_config_hash; + AttributesConfigHash _new_attributes_config_hash; + const IIndexschemaInspector& _old_index_schema_inspector; + const IDocumentTypeInspector& _inspector; + vespalib::hash_set _delayed_add_attribute_aspect; + vespalib::hash_set _delayed_add_attribute_aspect_struct; + vespalib::hash_set _delayed_remove_attribute_aspect; + + bool has_unchanged_field(const vespalib::string& name) const; + bool should_delay_add_attribute_aspect(const vespalib::string& name) const; + bool should_delay_remove_attribute_aspect(const vespalib::string& name) const; + bool calculate_fast_access(const AttributesConfig::Attribute& new_attribute_config) const; + void mark_delayed_add_attribute_aspect(const vespalib::string& name) { _delayed_add_attribute_aspect.insert(name); } + bool get_delayed_add_attribute_aspect(const vespalib::string& name) const noexcept { return _delayed_add_attribute_aspect.find(name) != _delayed_add_attribute_aspect.end(); } + void mark_delayed_add_attribute_aspect_struct(const vespalib::string& name) { _delayed_add_attribute_aspect_struct.insert(name); } + bool get_delayed_add_attribute_aspect_struct(const vespalib::string& name) const noexcept { return _delayed_add_attribute_aspect_struct.find(name) != _delayed_add_attribute_aspect_struct.end(); } + void mark_delayed_remove_attribute_aspect(const vespalib::string& name) { _delayed_remove_attribute_aspect.insert(name); } + bool get_delayed_remove_attribute_aspect(const vespalib::string& name) const noexcept { return _delayed_remove_attribute_aspect.find(name) != _delayed_remove_attribute_aspect.end(); } +public: + AttributeAspectConfigRewriter(const AttributesConfig& old_attributes_config, + const AttributesConfig& new_attributes_config, + const IIndexschemaInspector& old_index_schema_inspector, + const IDocumentTypeInspector& inspector); + ~AttributeAspectConfigRewriter(); + void calculate_delayed_attribute_aspects(); + void build_attributes_config(AttributesConfigBuilder& attributes_config_builder) const; + void build_summary_map_config(const SummarymapConfig& old_summarymap_config, + const SummarymapConfig& new_summarymap_config, + const SummaryConfig& new_summary_config, + SummarymapConfigBuilder& summary_map_config_builder) const; + void build_summary_config(const SummaryConfig& new_summary_config, + SummaryConfigBuilder& summary_config_builder) const; +}; + +AttributeAspectConfigRewriter::AttributeAspectConfigRewriter(const AttributesConfig& old_attributes_config, + const AttributesConfig& new_attributes_config, + const IIndexschemaInspector& old_index_schema_inspector, + const IDocumentTypeInspector& inspector) + : _old_attributes_config(old_attributes_config), + _new_attributes_config(new_attributes_config), + _old_attributes_config_hash(old_attributes_config.attribute), + _new_attributes_config_hash(new_attributes_config.attribute), + _old_index_schema_inspector(old_index_schema_inspector), + _inspector(inspector), + _delayed_add_attribute_aspect(), + _delayed_add_attribute_aspect_struct(), + _delayed_remove_attribute_aspect() { + calculate_delayed_attribute_aspects(); } -std::shared_ptr -AttributeAspectDelayer::getAttributesConfig() const +AttributeAspectConfigRewriter::~AttributeAspectConfigRewriter() = default; + +bool +AttributeAspectConfigRewriter::has_unchanged_field(const vespalib::string& name) const { - return _attributesConfig; + return _inspector.hasUnchangedField(name); } -std::shared_ptr -AttributeAspectDelayer::getSummarymapConfig() const +bool +AttributeAspectConfigRewriter::should_delay_add_attribute_aspect(const vespalib::string& name) const { - return _summarymapConfig; + if (!has_unchanged_field(name)) { + // No reprocessing due to field type/presence change, just use new config + return false; + } + auto old_attribute_config = _old_attributes_config_hash.lookup(name); + if (old_attribute_config != nullptr) { + return false; // Already added for ready subdb + } + auto new_attribute_config = _new_attributes_config_hash.lookup(name); + if (new_attribute_config == nullptr) { + return false; // Not added for any subdb + } + // Delay addition of attribute aspect since it would trigger reprocessing. + return true; } -namespace { +bool +AttributeAspectConfigRewriter::should_delay_remove_attribute_aspect(const vespalib::string& name) const +{ + if (!has_unchanged_field(name)) { + // No reprocessing due to field type/presence change, just use new config + return false; + } + auto old_attribute_config = _old_attributes_config_hash.lookup(name); + if (old_attribute_config == nullptr) { + return false; // Already removed in all subdbs + } + auto new_attribute_config = _new_attributes_config_hash.lookup(name); + if (new_attribute_config != nullptr) { + return false; // Not removed for ready subdb + } + // Delay removal of attribute aspect if it would trigger reprocessing. + auto old_cfg = ConfigConverter::convert(*old_attribute_config); + return willTriggerReprocessOnAttributeAspectRemoval(old_cfg, _old_index_schema_inspector, name); +} + +bool +AttributeAspectConfigRewriter::calculate_fast_access(const AttributesConfig::Attribute& new_attribute_config) const +{ + auto& name = new_attribute_config.name; + if (!has_unchanged_field(name)) { + // No reprocessing due to field type/presence change, just use new config + return new_attribute_config.fastaccess; + } + auto old_attribute_config = _old_attributes_config_hash.lookup(name); + assert(old_attribute_config != nullptr); + auto old_cfg = ConfigConverter::convert(*old_attribute_config); + if (!old_attribute_config->fastaccess || willTriggerReprocessOnAttributeAspectRemoval(old_cfg, _old_index_schema_inspector, name)) { + // Delay change of fast access flag + return old_attribute_config->fastaccess; + } else { + // Don't delay change of fast access flag from true to + // false when removing attribute aspect in a way that + // doesn't trigger reprocessing. + return new_attribute_config.fastaccess; + } +} void -handleNewAttributes(const AttributesConfig &oldAttributesConfig, - const AttributesConfig &newAttributesConfig, - const SummarymapConfig &newSummarymapConfig, - const IIndexschemaInspector &oldIndexschemaInspector, - const IDocumentTypeInspector &inspector, - AttributesConfigBuilder &attributesConfig, - SummarymapConfigBuilder &summarymapConfig) -{ - vespalib::hash_set delayed; - vespalib::hash_set delayedStruct; - AttributesConfigHash oldAttrs(oldAttributesConfig.attribute); - for (const auto &newAttr : newAttributesConfig.attribute) { - search::attribute::Config newCfg = ConfigConverter::convert(newAttr); - if (!inspector.hasUnchangedField(newAttr.name)) { - // No reprocessing due to field type change, just use new config - attributesConfig.attribute.emplace_back(newAttr); - } else { - auto oldAttr = oldAttrs.lookup(newAttr.name); - if (oldAttr != nullptr) { - search::attribute::Config oldCfg = ConfigConverter::convert(*oldAttr); - if (willTriggerReprocessOnAttributeAspectRemoval(oldCfg, oldIndexschemaInspector, newAttr.name) || !oldAttr->fastaccess) { - // Delay change of fast access flag - newCfg.setFastAccess(oldAttr->fastaccess); - auto modNewAttr = newAttr; - modNewAttr.fastaccess = oldAttr->fastaccess; - attributesConfig.attribute.emplace_back(modNewAttr); - // TODO: Don't delay change of fast access flag if - // attribute type can change without doucment field - // type changing (needs a smarter attribute - // reprocessing initializer). - } else { - // Don't delay change of fast access flag from true to - // false when removing attribute aspect in a way that - // doesn't trigger reprocessing. - attributesConfig.attribute.emplace_back(newAttr); - } - } else { - // Delay addition of attribute aspect - delayed.insert(newAttr.name); - auto pos = newAttr.name.find('.'); - if (pos != vespalib::string::npos) { - delayedStruct.insert(newAttr.name.substr(0, pos)); - } +AttributeAspectConfigRewriter::calculate_delayed_attribute_aspects() +{ + for (const auto &newAttr : _new_attributes_config.attribute) { + if (should_delay_add_attribute_aspect(newAttr.name)) { + mark_delayed_add_attribute_aspect(newAttr.name); + auto pos = newAttr.name.find('.'); + if (pos != vespalib::string::npos) { + mark_delayed_add_attribute_aspect_struct(newAttr.name.substr(0, pos)); } } } - for (const auto &override : newSummarymapConfig.override) { - if (override.command == "attribute") { - auto itr = delayed.find(source_field(override)); - if (itr == delayed.end()) { - summarymapConfig.override.emplace_back(override); + for (const auto &oldAttr : _old_attributes_config.attribute) { + if (should_delay_remove_attribute_aspect(oldAttr.name)) { + mark_delayed_remove_attribute_aspect(oldAttr.name); + } + } +} + +void +AttributeAspectConfigRewriter::build_attributes_config(AttributesConfigBuilder& attributes_config_builder) const +{ + for (const auto &newAttr : _new_attributes_config.attribute) { + if (get_delayed_add_attribute_aspect(newAttr.name)) { + // Delay addition of attribute aspect + } else { + attributes_config_builder.attribute.emplace_back(newAttr); + attributes_config_builder.attribute.back().fastaccess = calculate_fast_access(newAttr); + } + } + for (const auto &oldAttr : _old_attributes_config.attribute) { + if (get_delayed_remove_attribute_aspect(oldAttr.name)) { + // Delay removal of attribute aspect + attributes_config_builder.attribute.emplace_back(oldAttr); + } + } +} + +void +AttributeAspectConfigRewriter::build_summary_map_config(const SummarymapConfig& old_summarymap_config, + const SummarymapConfig& new_summarymap_config, + const SummaryConfig& new_summary_config, + SummarymapConfigBuilder& summarymap_config_builder) const +{ + KnownSummaryFields knownSummaryFields(new_summary_config); + for (const auto &override : new_summarymap_config.override) { + if (override.command == attribute_dfw_string) { + if (!get_delayed_add_attribute_aspect(source_field(override))) { + summarymap_config_builder.override.emplace_back(override); } - } else if (override.command == "attributecombiner") { - auto itr = delayedStruct.find(source_field(override)); - if (itr == delayedStruct.end()) { - summarymapConfig.override.emplace_back(override); + } else if (override.command == attribute_combiner_dfw_string) { + if (!get_delayed_add_attribute_aspect_struct(source_field(override))) { + summarymap_config_builder.override.emplace_back(override); } - } else if (override.command == "matchedattributeelementsfilter") { - auto itr = delayedStruct.find(source_field(override)); - if (itr == delayedStruct.end()) { - summarymapConfig.override.emplace_back(override); + } else if (override.command == matched_attribute_elements_filter_dfw_string) { + if (!get_delayed_add_attribute_aspect_struct(source_field(override))) { + summarymap_config_builder.override.emplace_back(override); } else { SummarymapConfig::Override mutated_override(override); - mutated_override.command = "matchedelementsfilter"; - summarymapConfig.override.emplace_back(mutated_override); + mutated_override.command = matched_elements_filter_dfw_string; + summarymap_config_builder.override.emplace_back(mutated_override); } } else { - summarymapConfig.override.emplace_back(override); + summarymap_config_builder.override.emplace_back(override); + } + } + for (const auto &override : old_summarymap_config.override) { + if (override.command == attribute_dfw_string) { + if (get_delayed_remove_attribute_aspect(source_field(override)) && knownSummaryFields.known(override.field)) { + summarymap_config_builder.override.emplace_back(override); + } } } } void -handleOldAttributes(const AttributesConfig &oldAttributesConfig, - const AttributesConfig &newAttributesConfig, - const SummarymapConfig &oldSummarymapConfig, - const SummaryConfig &newSummaryConfig, - const IIndexschemaInspector &oldIndexschemaInspector, - const IDocumentTypeInspector &inspector, - AttributesConfigBuilder &attributesConfig, - SummarymapConfigBuilder &summarymapConfig) -{ - vespalib::hash_set delayed; - KnownSummaryFields knownSummaryFields(newSummaryConfig); - AttributesConfigHash newAttrs(newAttributesConfig.attribute); - for (const auto &oldAttr : oldAttributesConfig.attribute) { - search::attribute::Config oldCfg = ConfigConverter::convert(oldAttr); - if (inspector.hasUnchangedField(oldAttr.name)) { - auto newAttr = newAttrs.lookup(oldAttr.name); - if (newAttr == nullptr) { - // Delay removal of attribute aspect if it would trigger - // reprocessing. - if (willTriggerReprocessOnAttributeAspectRemoval(oldCfg, oldIndexschemaInspector, oldAttr.name)) { - attributesConfig.attribute.emplace_back(oldAttr); - delayed.insert(oldAttr.name); +AttributeAspectConfigRewriter::build_summary_config(const SummaryConfig& new_summary_config, + SummaryConfigBuilder& summary_config_builder) const +{ + summary_config_builder = new_summary_config; + for (auto &summary_class : summary_config_builder.classes) { + for (auto &summary_field : summary_class.fields) { + if (summary_field.command == attribute_dfw_string) { + if (get_delayed_add_attribute_aspect(source_field(summary_field))) { + remove_docsum_field_rewriter(summary_field); + } + } else if (summary_field.command == attribute_combiner_dfw_string) { + if (get_delayed_add_attribute_aspect_struct(source_field(summary_field))) { + remove_docsum_field_rewriter(summary_field); + } + } else if (summary_field.command == matched_attribute_elements_filter_dfw_string) { + if (get_delayed_add_attribute_aspect_struct(source_field(summary_field)) || + get_delayed_add_attribute_aspect(source_field(summary_field))) { + summary_field.command = matched_elements_filter_dfw_string; + } + } else if (summary_field.command == matched_elements_filter_dfw_string) { + if (get_delayed_remove_attribute_aspect(source_field(summary_field))) { + summary_field.command = matched_attribute_elements_filter_dfw_string; + } + } else if (summary_field.command == "") { + if (get_delayed_remove_attribute_aspect(summary_field.name)) { + summary_field.command = attribute_dfw_string; + summary_field.source = summary_field.name; + } + } else if (summary_field.command == copy_dfw_string) { + if (get_delayed_remove_attribute_aspect(source_field(summary_field))) { + summary_field.command = attribute_dfw_string; + summary_field.source = source_field(summary_field); } } } } - for (const auto &override : oldSummarymapConfig.override) { - if (override.command == "attribute") { - auto itr = delayed.find(source_field(override)); - if (itr != delayed.end() && knownSummaryFields.known(override.field)) { - summarymapConfig.override.emplace_back(override); - } - } - } } } +AttributeAspectDelayer::AttributeAspectDelayer() + : _attributesConfig(std::make_shared()), + _summarymapConfig(std::make_shared()), + _summaryConfig(std::make_shared()) +{ +} + +AttributeAspectDelayer::~AttributeAspectDelayer() +{ +} + +std::shared_ptr +AttributeAspectDelayer::getAttributesConfig() const +{ + return _attributesConfig; +} + +std::shared_ptr +AttributeAspectDelayer::getSummarymapConfig() const +{ + return _summarymapConfig; +} + +std::shared_ptr +AttributeAspectDelayer::getSummaryConfig() const +{ + return _summaryConfig; +} + void AttributeAspectDelayer::setup(const AttributesConfig &oldAttributesConfig, const SummarymapConfig &oldSummarymapConfig, @@ -215,14 +374,17 @@ AttributeAspectDelayer::setup(const AttributesConfig &oldAttributesConfig, const IIndexschemaInspector &oldIndexschemaInspector, const IDocumentTypeInspector &inspector) { - handleNewAttributes(oldAttributesConfig, newAttributesConfig, - newSummarymapConfig, - oldIndexschemaInspector, inspector, - *_attributesConfig, *_summarymapConfig); - handleOldAttributes(oldAttributesConfig, newAttributesConfig, - oldSummarymapConfig, newSummaryConfig, - oldIndexschemaInspector, inspector, - *_attributesConfig, *_summarymapConfig); + AttributeAspectConfigRewriter cfg_rewriter(oldAttributesConfig, + newAttributesConfig, + oldIndexschemaInspector, + inspector); + cfg_rewriter.build_attributes_config(*_attributesConfig); + cfg_rewriter.build_summary_map_config(oldSummarymapConfig, + newSummarymapConfig, + newSummaryConfig, + *_summarymapConfig); + cfg_rewriter.build_summary_config(newSummaryConfig, + *_summaryConfig); } } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_aspect_delayer.h b/searchcore/src/vespa/searchcore/proton/attribute/attribute_aspect_delayer.h index 9ffb23dca8d..9d8338349d1 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_aspect_delayer.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_aspect_delayer.h @@ -29,10 +29,12 @@ class AttributeAspectDelayer using IndexschemaConfig = const vespa::config::search::internal::InternalIndexschemaType; using SummarymapConfigBuilder = vespa::config::search::internal::InternalSummarymapType; using SummarymapConfig = const vespa::config::search::internal::InternalSummarymapType; + using SummaryConfigBuilder = vespa::config::search::internal::InternalSummaryType; using SummaryConfig = const vespa::config::search::internal::InternalSummaryType; std::shared_ptr _attributesConfig; std::shared_ptr _summarymapConfig; + std::shared_ptr _summaryConfig; public: AttributeAspectDelayer(); @@ -52,6 +54,7 @@ public: std::shared_ptr getAttributesConfig() const; std::shared_ptr getSummarymapConfig() const; + std::shared_ptr getSummaryConfig() const; }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdbconfig.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdbconfig.cpp index 8c067cfc3db..53ce693ddb7 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdbconfig.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentdbconfig.cpp @@ -302,8 +302,10 @@ DocumentDBConfig::makeDelayedAttributeAspectConfig(const SP &newCfg, const Docum attributeAspectDelayer.setup(oldCfg.getAttributesConfig(), oldCfg.getSummarymapConfig(), n.getAttributesConfig(), n.getSummaryConfig(), n.getSummarymapConfig(), oldIndexschemaInspector, inspector); - bool delayedAttributeAspects = (n.getAttributesConfig() != *attributeAspectDelayer.getAttributesConfig()) || - (n.getSummarymapConfig() != *attributeAspectDelayer.getSummarymapConfig()); + bool attributes_config_changed = (n.getAttributesConfig() != *attributeAspectDelayer.getAttributesConfig()); + bool summarymap_config_changed = (n.getSummarymapConfig() != *attributeAspectDelayer.getSummarymapConfig()); + bool summary_config_changed = (n.getSummaryConfig() != *attributeAspectDelayer.getSummaryConfig()); + bool delayedAttributeAspects = (attributes_config_changed || summarymap_config_changed || summary_config_changed); if (!delayedAttributeAspects) { return newCfg; } @@ -314,9 +316,9 @@ DocumentDBConfig::makeDelayedAttributeAspectConfig(const SP &newCfg, const Docum n._rankingExpressions, n._onnxModels, n._indexschema, - attributeAspectDelayer.getAttributesConfig(), - n._summary, - attributeAspectDelayer.getSummarymapConfig(), + (attributes_config_changed ? attributeAspectDelayer.getAttributesConfig() : n._attributes), + (summary_config_changed ? attributeAspectDelayer.getSummaryConfig() : n._summary), + (summarymap_config_changed ? attributeAspectDelayer.getSummarymapConfig() : n._summarymap), n._juniperrc, n._documenttypes, n._repo, -- cgit v1.2.3