From bb74faa5f2e56c930db1ec06ebf912e5b316c087 Mon Sep 17 00:00:00 2001 From: Tor Brede Vekterli Date: Wed, 29 Jan 2020 11:26:48 +0000 Subject: Add ReadableAttributeVector accessor to IAttributeManager Provides a unified interface for fetching both regular as well as imported attributes. Exposing `ReadableAttributeVector` instead of raw `AttributeVector` instances enforces that all access is done via appropriate acquired read guards. Refactor document selection processing code to use the new interface in order to prepare for imported field support in selections. --- .../attributemanager/attributemanager_test.cpp | 17 ++++++++++++++ .../attribute_searchable_adapter_test.cpp | 9 ++++++++ .../searchable/attributeblueprint_test.cpp | 4 ++++ .../vespa/searchlib/attribute/attributemanager.cpp | 11 ++++++++- .../vespa/searchlib/attribute/attributemanager.h | 2 ++ .../vespa/searchlib/attribute/iattributemanager.h | 26 +++++++++++++++++----- .../attribute/readable_attribute_vector.h | 2 +- .../searchlib/test/mock_attribute_manager.cpp | 5 +++++ .../vespa/searchlib/test/mock_attribute_manager.h | 1 + 9 files changed, 69 insertions(+), 8 deletions(-) (limited to 'searchlib') diff --git a/searchlib/src/tests/attribute/attributemanager/attributemanager_test.cpp b/searchlib/src/tests/attribute/attributemanager/attributemanager_test.cpp index 62cde4d2c9c..7d09b2aa0b8 100644 --- a/searchlib/src/tests/attribute/attributemanager/attributemanager_test.cpp +++ b/searchlib/src/tests/attribute/attributemanager/attributemanager_test.cpp @@ -34,6 +34,7 @@ private: void testGuards(); void testConfigConvert(); void testContext(); + void can_get_readable_attribute_vector_by_name(); bool assertDataType(BT::Type exp, @@ -377,6 +378,21 @@ AttributeManagerTest::testContext() } } +void +AttributeManagerTest::can_get_readable_attribute_vector_by_name() +{ + auto attr = AttributeFactory::createAttribute("cool_attr", Config(BT::INT32, CT::SINGLE)); + // Ensure there's something to actually load, or fetching the attribute will throw. + attr->addDocs(64); + attr->commit(); + AttributeManager manager; + manager.add(attr); + auto av = manager.readable_attribute_vector("cool_attr"); + EXPECT_EQUAL(av.get(), static_cast(attr.get())); + av = manager.readable_attribute_vector("uncool_attr"); + EXPECT_TRUE(av.get() == nullptr); +} + int AttributeManagerTest::Main() { TEST_INIT("attributemanager_test"); @@ -385,6 +401,7 @@ int AttributeManagerTest::Main() testGuards(); testConfigConvert(); testContext(); + can_get_readable_attribute_vector_by_name(); TEST_DONE(); } diff --git a/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp b/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp index 4818287b429..2eafeab20bd 100644 --- a/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp +++ b/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp @@ -123,6 +123,15 @@ public: return IAttributeContext::UP(); } + std::shared_ptr readable_attribute_vector(const string& name) const override { + if (name == field) { + return _attribute_vector; + } else if (name == other) { + return _other; + } + return {}; + } + void asyncForAttribute(const vespalib::string &name, std::unique_ptr func) const override; }; diff --git a/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp b/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp index e9addae07b9..24be21f65ec 100644 --- a/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp +++ b/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp @@ -100,6 +100,10 @@ public: void asyncForAttribute(const vespalib::string &, std::unique_ptr) const override { assert(!"Not implemented"); } + + std::shared_ptr readable_attribute_vector(const string&) const override { + return _attribute_vector; + } }; constexpr uint32_t DOCID_LIMIT = 3; diff --git a/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp b/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp index da280aa7bda..5fa32e8ad75 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp @@ -131,7 +131,7 @@ bool AttributeManager::hasReaders() const const AttributeManager::VectorHolder * AttributeManager::findAndLoadAttribute(const string & name) const { - const VectorHolder * loadedVector(NULL); + const VectorHolder * loadedVector(nullptr); AttributeMap::const_iterator found = _attributes.find(name); if (found != _attributes.end()) { AttributeVector & vec = *found->second; @@ -263,5 +263,14 @@ AttributeManager::asyncForAttribute(const vespalib::string &, std::unique_ptr +AttributeManager::readable_attribute_vector(const string& name) const +{ + const auto* attr = findAndLoadAttribute(name); + if (attr) { + return *attr; + } + return {}; +} } diff --git a/searchlib/src/vespa/searchlib/attribute/attributemanager.h b/searchlib/src/vespa/searchlib/attribute/attributemanager.h index 1c60ab00585..9f30ff87a70 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributemanager.h +++ b/searchlib/src/vespa/searchlib/attribute/attributemanager.h @@ -49,6 +49,8 @@ public: void getAttributeList(AttributeList & list) const override; attribute::IAttributeContext::UP createContext() const override; + std::shared_ptr readable_attribute_vector(const string& name) const override; + const Snapshot & getSnapshot() const { return _snapShot; } const string & getBaseDir() const { return _baseDir; } void setBaseDir(const string & base); diff --git a/searchlib/src/vespa/searchlib/attribute/iattributemanager.h b/searchlib/src/vespa/searchlib/attribute/iattributemanager.h index ef9403e1c4b..51e432f2035 100644 --- a/searchlib/src/vespa/searchlib/attribute/iattributemanager.h +++ b/searchlib/src/vespa/searchlib/attribute/iattributemanager.h @@ -8,7 +8,10 @@ namespace search { -namespace attribute { class AttributeReadGuard; } +namespace attribute { +class AttributeReadGuard; +class ReadableAttributeVector; +} /** * This is an interface used to access all registered attribute vectors. @@ -17,12 +20,17 @@ class IAttributeManager : public attribute::IAttributeExecutor { public: IAttributeManager(const IAttributeManager &) = delete; IAttributeManager & operator = (const IAttributeManager &) = delete; - typedef std::shared_ptr SP; - typedef vespalib::string string; + using SP = std::shared_ptr; + using string = vespalib::string; /** * Returns a view of the attribute vector with the given name. * + * NOTE: this method is deprecated! Prefer using readable_attribute_vector(name) instead, + * as that enforces appropriate guards to be taken before accessing the underlying vector. + * + * TODO remove this when all usages are gone. + * * @param name name of the attribute vector. * @return view of the attribute vector or empty view if the attribute vector does not exists. **/ @@ -52,9 +60,15 @@ public: virtual attribute::IAttributeContext::UP createContext() const = 0; /** - * Virtual destructor to allow safe subclassing. - **/ - virtual ~IAttributeManager() {} + * Looks up and returns a readable attribute vector shared_ptr with the provided name. + * This transparently supports imported attribute vectors. + * + * @param name name of the attribute vector. + * @return The attribute vector, or an empty shared_ptr if no vector was found with the given name. + */ + virtual std::shared_ptr readable_attribute_vector(const string& name) const = 0; + + ~IAttributeManager() override = default; protected: IAttributeManager() = default; }; diff --git a/searchlib/src/vespa/searchlib/attribute/readable_attribute_vector.h b/searchlib/src/vespa/searchlib/attribute/readable_attribute_vector.h index 03058362cb1..0e3954bdcb5 100644 --- a/searchlib/src/vespa/searchlib/attribute/readable_attribute_vector.h +++ b/searchlib/src/vespa/searchlib/attribute/readable_attribute_vector.h @@ -13,7 +13,7 @@ class AttributeReadGuard; */ class ReadableAttributeVector { public: - virtual ~ReadableAttributeVector() {} + virtual ~ReadableAttributeVector() = default; virtual std::unique_ptr makeReadGuard(bool stableEnumGuard) const = 0; }; diff --git a/searchlib/src/vespa/searchlib/test/mock_attribute_manager.cpp b/searchlib/src/vespa/searchlib/test/mock_attribute_manager.cpp index 5ef9b6cb2d4..4c80c6f869c 100644 --- a/searchlib/src/vespa/searchlib/test/mock_attribute_manager.cpp +++ b/searchlib/src/vespa/searchlib/test/mock_attribute_manager.cpp @@ -61,4 +61,9 @@ MockAttributeManager::addAttribute(const AttributeVector::SP &attr) { addAttribute(attr->getName(), attr); } +std::shared_ptr +MockAttributeManager::readable_attribute_vector(const string& name) const { + return findAttribute(name); +} + } diff --git a/searchlib/src/vespa/searchlib/test/mock_attribute_manager.h b/searchlib/src/vespa/searchlib/test/mock_attribute_manager.h index dbf84a40e84..a8ec686433a 100644 --- a/searchlib/src/vespa/searchlib/test/mock_attribute_manager.h +++ b/searchlib/src/vespa/searchlib/test/mock_attribute_manager.h @@ -28,6 +28,7 @@ public: IAttributeContext::UP createContext() const override; void addAttribute(const vespalib::string &name, const AttributeVector::SP &attr); void addAttribute(const AttributeVector::SP &attr); + std::shared_ptr readable_attribute_vector(const string& name) const override; }; } -- cgit v1.2.3