diff options
author | Tor Egge <Tor.Egge@yahoo-inc.com> | 2017-04-04 10:13:15 +0000 |
---|---|---|
committer | Tor Egge <Tor.Egge@yahoo-inc.com> | 2017-04-04 10:13:15 +0000 |
commit | 9efa6f5cb858d3a94932df582d26968ce4eadf16 (patch) | |
tree | cba4f8ab0a6a3385330cbba461eda3b702ba7111 /searchcore | |
parent | 9b43f1a3707664da3ab78e4fb4692ce6e924e2cf (diff) |
Add attribute specs builder, used to create attribute specs vector with
proper annotations (hideFromReading, hideFromWriting) and adjusted
attribute config that should be saved in transaction log.
Diffstat (limited to 'searchcore')
10 files changed, 542 insertions, 0 deletions
diff --git a/searchcore/CMakeLists.txt b/searchcore/CMakeLists.txt index d7edca0ac35..8b2025462ed 100644 --- a/searchcore/CMakeLists.txt +++ b/searchcore/CMakeLists.txt @@ -65,6 +65,7 @@ vespa_define_module( src/tests/proton/attribute/attribute_initializer src/tests/proton/attribute/attribute_manager src/tests/proton/attribute/attribute_populator + src/tests/proton/attribute/attribute_specs_builder src/tests/proton/attribute/attribute_usage_filter src/tests/proton/attribute/attributes_state_explorer src/tests/proton/attribute/document_field_populator diff --git a/searchcore/src/tests/proton/attribute/attribute_specs_builder/CMakeLists.txt b/searchcore/src/tests/proton/attribute/attribute_specs_builder/CMakeLists.txt new file mode 100644 index 00000000000..b676722c504 --- /dev/null +++ b/searchcore/src/tests/proton/attribute/attribute_specs_builder/CMakeLists.txt @@ -0,0 +1,9 @@ +# Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(searchcore_attribute_specs_builder_test_app TEST + SOURCES + attribute_specs_builder_test.cpp + DEPENDS + searchcore_attribute + searchcore_pcommon +) +vespa_add_test(NAME searchcore_attribute_specs_builder_test_app COMMAND searchcore_attribute_specs_builder_test_app) diff --git a/searchcore/src/tests/proton/attribute/attribute_specs_builder/DESC b/searchcore/src/tests/proton/attribute/attribute_specs_builder/DESC new file mode 100644 index 00000000000..5cfe2ba2167 --- /dev/null +++ b/searchcore/src/tests/proton/attribute/attribute_specs_builder/DESC @@ -0,0 +1 @@ +attribute specs builder test. Take a look at attribute_specs_builder_test.cpp for details. diff --git a/searchcore/src/tests/proton/attribute/attribute_specs_builder/FILES b/searchcore/src/tests/proton/attribute/attribute_specs_builder/FILES new file mode 100644 index 00000000000..0c648bd92f6 --- /dev/null +++ b/searchcore/src/tests/proton/attribute/attribute_specs_builder/FILES @@ -0,0 +1 @@ +attribute_specs_builder_test.cpp diff --git a/searchcore/src/tests/proton/attribute/attribute_specs_builder/attribute_specs_builder_test.cpp b/searchcore/src/tests/proton/attribute/attribute_specs_builder/attribute_specs_builder_test.cpp new file mode 100644 index 00000000000..780596db295 --- /dev/null +++ b/searchcore/src/tests/proton/attribute/attribute_specs_builder/attribute_specs_builder_test.cpp @@ -0,0 +1,320 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <vespa/fastos/fastos.h> +#include <vespa/log/log.h> +LOG_SETUP("attribute_specs_builder_test"); +#include <vespa/vespalib/testkit/testapp.h> +#include <vespa/vespalib/stllike/string.h> +#include <vespa/searchcore/proton/test/attribute_utils.h> +#include <vespa/searchcore/proton/attribute/attribute_specs_builder.h> +#include <vespa/searchcore/proton/common/i_document_type_inspector.h> +#include <vespa/vespalib/test/insertion_operators.h> +#include <vespa/config-indexschema.h> +#include <vespa/config-attributes.h> + +using vespa::config::search::AttributesConfig; +using vespa::config::search::AttributesConfigBuilder; +using vespa::config::search::IndexschemaConfig; +using vespa::config::search::IndexschemaConfigBuilder; +using search::attribute::Config; +using search::attribute::BasicType; +using search::attribute::CollectionType; + +namespace { + +const char *boolStr(bool val) { + return val ? "true" : "false"; +} + +} + +namespace std +{ + +ostream &operator<<(ostream &os, const Config &cfg) +{ + os << "{basicType=" << cfg.basicType().asString(); + os << ", collectionType=" << cfg.collectionType().asString(); + os << ", fastAccess=" << boolStr(cfg.fastAccess()); + os << "}"; + return os; +} + +ostream &operator<<(ostream &os, const proton::AttributeSpec &spec) +{ + os << "{name=" << spec.getName(); + os << ", hideFromReading=" << boolStr(spec.getHideFromReading()); + os << ", hideFromWriting=" << boolStr(spec.getHideFromWriting()); + os << ", " << spec.getConfig(); + os << "}"; + return os; +} + +} + +namespace proton +{ + +namespace { + +AttributesConfig::Attribute make_int32_sv_cfg() { + AttributesConfig::Attribute attr; + attr.name = "a"; + attr.datatype = AttributesConfig::Attribute::Datatype::INT32; + attr.collectiontype = AttributesConfig::Attribute::Collectiontype::SINGLE; + return attr; +} + +AttributesConfig::Attribute make_string_sv_cfg() { + AttributesConfig::Attribute attr; + attr.name = "a"; + attr.datatype = AttributesConfig::Attribute::Datatype::STRING; + attr.collectiontype = AttributesConfig::Attribute::Collectiontype::SINGLE; + return attr; +} + +AttributesConfig::Attribute make_predicate_cfg(uint32_t arity) +{ + AttributesConfig::Attribute attr; + attr.name = "a"; + attr.datatype = AttributesConfig::Attribute::Datatype::PREDICATE; + attr.collectiontype = AttributesConfig::Attribute::Collectiontype::SINGLE; + attr.arity = arity; + return attr; +} + +AttributesConfig::Attribute make_tensor_cfg(const vespalib::string &spec) +{ + AttributesConfig::Attribute attr; + attr.name = "a"; + attr.datatype = AttributesConfig::Attribute::Datatype::TENSOR; + attr.collectiontype = AttributesConfig::Attribute::Collectiontype::SINGLE; + attr.tensortype = spec; + return attr; +} + +AttributesConfig::Attribute make_reference_cfg() +{ + AttributesConfig::Attribute attr; + attr.name = "a"; + attr.datatype = AttributesConfig::Attribute::Datatype::REFERENCE; + attr.collectiontype = AttributesConfig::Attribute::Collectiontype::SINGLE; + return attr; +} + +AttributesConfig attrCfg(std::vector<AttributesConfig::Attribute> attributes) +{ + AttributesConfigBuilder result; + result.attribute = attributes; + return result; +} + +AttributesConfig::Attribute make_fa(const AttributesConfig::Attribute &cfg) +{ + AttributesConfig::Attribute attr(cfg); + attr.fastaccess = true; + return attr; +} + +Config make_fa(const Config &cfg) +{ + Config modCfg(cfg); + modCfg.setFastAccess(true); + return modCfg; +} + +const Config int32_sv(BasicType::Type::INT32); +const Config string_sv(BasicType::Type::STRING); + +Config getPredicate(uint32_t arity) +{ + Config ret(BasicType::Type::PREDICATE); + search::attribute::PredicateParams predicateParams; + predicateParams.setArity(arity); + ret.setPredicateParams(predicateParams); + return ret; +} + +class MyInspector : public IDocumentTypeInspector +{ + std::set<vespalib::string> _unchanged; +public: + virtual bool hasUnchangedField(const vespalib::string &name) const override { + return _unchanged.count(name) > 0; + } + MyInspector() + : _unchanged() + { + } + ~MyInspector() { } + void addFields(const std::vector<vespalib::string> &fields) { + for (const auto &field : fields) { + _unchanged.insert(field); + } + } +}; + +} + +class Fixture +{ + MyInspector _inspector; + IndexschemaConfigBuilder _oldIndexSchema; + AttributeSpecsBuilder _builder; + +public: + Fixture() + : _inspector(), + _builder() + { + } + ~Fixture() { } + void addFields(const std::vector<vespalib::string> &fields) { + _inspector.addFields(fields); + } + void addOldIndexField(const vespalib::string &name) { + IndexschemaConfig::Indexfield field; + field.name = name; + _oldIndexSchema.indexfield.emplace_back(field); + } + void setup(const AttributesConfig &newConfig) { + _builder.setup(newConfig); + } + void setup(const AttributesConfig &oldConfig, const AttributesConfig &newConfig) { + _builder.setup(oldConfig, newConfig, _oldIndexSchema, _inspector); + } + void assertSpecs(const std::vector<AttributeSpec> &expSpecs) + { + auto &actSpecs = _builder.getAttributeSpecs(); + EXPECT_EQUAL(expSpecs, actSpecs); + } + void assertConfigs(const std::vector<AttributesConfig::Attribute> &exp) + { + auto actConfig = _builder.getAttributesConfig(); + EXPECT_TRUE(exp == actConfig->attribute); + } +}; + +TEST_F("require that empty specs is OK", Fixture) +{ + f.setup(attrCfg({})); + TEST_DO(f.assertSpecs({})); + TEST_DO(f.assertConfigs({})); +} + +TEST_F("require that simple attribute specs is OK", Fixture) +{ + f.setup(attrCfg({make_int32_sv_cfg()})); + TEST_DO(f.assertSpecs({AttributeSpec("a", int32_sv, false, false)})); + TEST_DO(f.assertConfigs({make_int32_sv_cfg()})); +} + +TEST_F("require that adding attribute aspect is delayed if field type is unchanged", Fixture) +{ + f.addFields({"a"}); + f.setup(attrCfg({}), attrCfg({make_int32_sv_cfg()})); + TEST_DO(f.assertSpecs({AttributeSpec("a", int32_sv, false, true)})); + TEST_DO(f.assertConfigs({})); +} + +TEST_F("require that adding attribute is not delayed if field type changed", Fixture) +{ + f.setup(attrCfg({}), attrCfg({make_int32_sv_cfg()})); + TEST_DO(f.assertSpecs({AttributeSpec("a", int32_sv, false, false)})); + TEST_DO(f.assertConfigs({make_int32_sv_cfg()})); +} + +TEST_F("require that removing attribute aspect is delayed if field type is unchanged", Fixture) +{ + f.addFields({"a"}); + f.setup(attrCfg({make_int32_sv_cfg()}), attrCfg({})); + TEST_DO(f.assertSpecs({AttributeSpec("a", int32_sv, true, false)})); + TEST_DO(f.assertConfigs({make_int32_sv_cfg()})); +} + +TEST_F("require that removing attribute aspect is not delayed if field type changed", Fixture) +{ + f.setup(attrCfg({make_int32_sv_cfg()}), attrCfg({})); + TEST_DO(f.assertSpecs({})); + TEST_DO(f.assertConfigs({})); +} + +TEST_F("require that removing attribute aspect is not delayed if also indexed", Fixture) +{ + f.addFields({"a"}); + f.addOldIndexField("a"); + f.setup(attrCfg({make_string_sv_cfg()}), attrCfg({})); + TEST_DO(f.assertSpecs({})); + TEST_DO(f.assertConfigs({})); +} + +TEST_F("require that removing attribute aspect is not delayed for tensor", Fixture) +{ + f.addFields({"a"}); + f.setup(attrCfg({make_tensor_cfg("tensor(x[10])")}), attrCfg({})); + TEST_DO(f.assertSpecs({})); + TEST_DO(f.assertConfigs({})); +} + +TEST_F("require that removing attribute aspect is not delayed for predicate", Fixture) +{ + f.addFields({"a"}); + f.setup(attrCfg({make_predicate_cfg(4)}), attrCfg({})); + TEST_DO(f.assertSpecs({})); + TEST_DO(f.assertConfigs({})); +} + +TEST_F("require that removing attribute aspect is not delayed for reference", Fixture) +{ + f.addFields({"a"}); + f.setup(attrCfg({make_reference_cfg()}), attrCfg({})); + TEST_DO(f.assertSpecs({})); + TEST_DO(f.assertConfigs({})); +} + +TEST_F("require that fast access flag change is delayed, false->true edge", Fixture) +{ + f.addFields({"a"}); + f.setup(attrCfg({make_int32_sv_cfg()}), attrCfg({make_fa(make_int32_sv_cfg())})); + TEST_DO(f.assertSpecs({AttributeSpec("a", int32_sv)})); + TEST_DO(f.assertConfigs({make_int32_sv_cfg()})); +} + +TEST_F("require that fast access flag change is delayed, true->false edge", Fixture) +{ + f.addFields({"a"}); + f.setup(attrCfg({make_fa(make_int32_sv_cfg())}), attrCfg({make_int32_sv_cfg()})); + TEST_DO(f.assertSpecs({AttributeSpec("a", make_fa(int32_sv))})); + TEST_DO(f.assertConfigs({make_fa(make_int32_sv_cfg())})); +} + +TEST_F("require that fast access flag change is delayed, false->true edge, predicate attr", Fixture) +{ + f.addFields({"a"}); + f.setup(attrCfg({make_predicate_cfg(4)}), attrCfg({make_fa(make_predicate_cfg(4))})); + TEST_DO(f.assertSpecs({AttributeSpec("a", getPredicate(4))})); + TEST_DO(f.assertConfigs({make_predicate_cfg(4)})); +} + +TEST_F("require that fast access flag change is not delayed, true->false edge, predicate attr", Fixture) +{ + f.addFields({"a"}); + f.setup(attrCfg({make_fa(make_predicate_cfg(4))}), attrCfg({make_predicate_cfg(4)})); + TEST_DO(f.assertSpecs({AttributeSpec("a", getPredicate(4))})); + TEST_DO(f.assertConfigs({make_predicate_cfg(4)})); +} + +TEST_F("require that fast access flag change is not delayed, true->false edge, string attribute, indexed field", Fixture) +{ + f.addFields({"a"}); + f.addOldIndexField("a"); + f.setup(attrCfg({make_fa(make_string_sv_cfg())}), attrCfg({make_string_sv_cfg()})); + TEST_DO(f.assertSpecs({AttributeSpec("a", string_sv, false, false)})); + TEST_DO(f.assertConfigs({make_string_sv_cfg()})); +} + +} + +TEST_MAIN() +{ + TEST_RUN_ALL(); +} diff --git a/searchcore/src/vespa/searchcore/proton/attribute/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/attribute/CMakeLists.txt index 75d93d9e003..723189a3544 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/CMakeLists.txt +++ b/searchcore/src/vespa/searchcore/proton/attribute/CMakeLists.txt @@ -11,6 +11,7 @@ vespa_add_library(searchcore_attribute STATIC attribute_manager_initializer.cpp attribute_populator.cpp attribute_spec.cpp + attribute_specs_builder.cpp attribute_usage_filter.cpp attribute_usage_sampler_context.cpp attribute_usage_sampler_functor.cpp diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_spec.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_spec.cpp index 9d1d5105978..6cd76182659 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_spec.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_spec.cpp @@ -33,4 +33,13 @@ AttributeSpec::operator=(AttributeSpec &&) = default; AttributeSpec::~AttributeSpec() { } +bool +AttributeSpec::operator==(const AttributeSpec &rhs) const +{ + return ((_name == rhs._name) && + (_cfg == rhs._cfg) && + (_hideFromReading == rhs._hideFromReading) && + (_hideFromWriting == rhs._hideFromWriting)); +} + } diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_spec.h b/searchcore/src/vespa/searchcore/proton/attribute/attribute_spec.h index 247eabdad0e..f8c8834894a 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_spec.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_spec.h @@ -33,6 +33,7 @@ public: const search::attribute::Config &getConfig() const { return _cfg; } bool getHideFromReading() const { return _hideFromReading; } bool getHideFromWriting() const { return _hideFromWriting; } + bool operator==(const AttributeSpec &rhs) const; }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_specs_builder.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_specs_builder.cpp new file mode 100644 index 00000000000..2b4545fd563 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_specs_builder.cpp @@ -0,0 +1,154 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "attribute_specs_builder.h" +#include <vespa/searchlib/attribute/configconverter.h> +#include <vespa/searchcore/proton/common/i_document_type_inspector.h> +#include <vespa/vespalib/stllike/hash_map.hpp> +#include <vespa/config-indexschema.h> +#include <vespa/config-attributes.h> + +using search::attribute::ConfigConverter; +using vespa::config::search::AttributesConfig; +using vespa::config::search::IndexschemaConfig; +using search::attribute::BasicType; + +namespace proton { + +namespace { + +template <class Elem> +class ConfigHash { + vespalib::hash_map<vespalib::string, const Elem *> _hash; +public: + ConfigHash(const std::vector<Elem> &config) + : _hash() + { + for (const auto &elem : config) { + auto insres = _hash.insert(std::make_pair(elem.name, &elem)); + assert(insres.second); + } + } + ~ConfigHash() { } + const Elem *lookup(const vespalib::string &name) const { + auto itr = _hash.find(name); + return ((itr == _hash.end()) ? nullptr : itr->second); + } +}; + +using AttributesConfigHash = ConfigHash<AttributesConfig::Attribute>; +using IndexConfigHash = ConfigHash<IndexschemaConfig::Indexfield>; + +bool fastPartialUpdateAttribute(const search::attribute::Config &cfg) { + auto basicType = cfg.basicType().type(); + return ((basicType != BasicType::Type::PREDICATE) && + (basicType != BasicType::Type::TENSOR) && + (basicType != BasicType::Type::REFERENCE)); +} + +bool isStringIndex(const IndexConfigHash &indexConfig, const vespalib::string &name) +{ + auto index = indexConfig.lookup(name); + if (index != nullptr) { + if (index->datatype == IndexschemaConfig::Indexfield::Datatype::STRING) { + return true; + } + } + return false; +} + +} + +AttributeSpecsBuilder::AttributeSpecsBuilder() + : _specs(), + _config(std::make_shared<AttributesConfigBuilder>()) +{ +} + +AttributeSpecsBuilder::~AttributeSpecsBuilder() +{ +} + +const AttributeSpecsBuilder::AttributeSpecs & +AttributeSpecsBuilder::getAttributeSpecs() const +{ + return _specs; +} + +std::shared_ptr<AttributeSpecsBuilder::AttributesConfig> +AttributeSpecsBuilder::getAttributesConfig() const +{ + return _config; +} + +void +AttributeSpecsBuilder::setup(const AttributesConfig &newConfig) +{ + for (const auto &attr : newConfig.attribute) { + search::attribute::Config cfg = ConfigConverter::convert(attr); + _specs.emplace_back(attr.name, cfg); + } + _config = std::make_shared<AttributesConfigBuilder>(newConfig); +} + +void +AttributeSpecsBuilder::setup(const AttributesConfig &oldAttributesConfig, + const AttributesConfig &newAttributesConfig, + const IndexschemaConfig &oldIndexschemaConfig, + const IDocumentTypeInspector &inspector) +{ + AttributesConfigHash oldAttrs(oldAttributesConfig.attribute); + AttributesConfigHash newAttrs(newAttributesConfig.attribute); + IndexConfigHash oldIndexes(oldIndexschemaConfig.indexfield); + 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 + _specs.emplace_back(newAttr.name, newCfg); + _config->attribute.emplace_back(newAttr); + } else { + auto oldAttr = oldAttrs.lookup(newAttr.name); + if (oldAttr != nullptr) { + search::attribute::Config oldCfg = ConfigConverter::convert(*oldAttr); + if ((fastPartialUpdateAttribute(oldCfg) && + !isStringIndex(oldIndexes, newAttr.name)) || + !oldAttr->fastaccess) { + // Delay change of fast access flag + newCfg.setFastAccess(oldAttr->fastaccess); + _specs.emplace_back(newAttr.name, newCfg); + auto modNewAttr = newAttr; + modNewAttr.fastaccess = oldAttr->fastaccess; + _config->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. + _specs.emplace_back(newAttr.name, newCfg); + _config->attribute.emplace_back(newAttr); + } + } else { + // Delay addition of attribute aspect + _specs.emplace_back(newAttr.name, newCfg, false, true); + } + } + } + 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 (fastPartialUpdateAttribute(oldCfg) && !isStringIndex(oldIndexes, oldAttr.name)) { + _specs.emplace_back(oldAttr.name, oldCfg, true, false); + _config->attribute.emplace_back(oldAttr); + } + } + } + } +} + +} // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_specs_builder.h b/searchcore/src/vespa/searchcore/proton/attribute/attribute_specs_builder.h new file mode 100644 index 00000000000..5be087a13d6 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_specs_builder.h @@ -0,0 +1,45 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "attribute_spec.h" + +namespace document { class DocumentType; } +namespace vespa { namespace config { namespace search { namespace internal { +class InternalAttributesType; +class InternalIndexschemaType; +} } } } + +namespace proton { + +class IDocumentTypeInspector; + +/* + * Class to build adjusted attribute config and vector of attribute specs. + */ +class AttributeSpecsBuilder +{ + using AttributeSpecs = std::vector<AttributeSpec>; + using AttributesConfigBuilder = vespa::config::search::internal::InternalAttributesType; + using AttributesConfig = const vespa::config::search::internal::InternalAttributesType; + using DocumentType = document::DocumentType; + using IndexschemaConfig = const vespa::config::search::internal::InternalIndexschemaType; + + AttributeSpecs _specs; + std::shared_ptr<AttributesConfigBuilder> _config; + +public: + AttributeSpecsBuilder(); + ~AttributeSpecsBuilder(); + + void setup(const AttributesConfig &newConfig); + void setup(const AttributesConfig &oldAttributesConfig, + const AttributesConfig &newAttributesConfig, + const IndexschemaConfig &oldIndexschemaConfig, + const IDocumentTypeInspector &inspector); + + const AttributeSpecs &getAttributeSpecs() const; + std::shared_ptr<AttributesConfig> getAttributesConfig() const; +}; + +} // namespace proton |