Implement attribute context for imported attributes.

13 files changed, 378 insertions, 2 deletions

diff --git a/searchcore/CMakeLists.txt b/searchcore/CMakeLists.txt
index b45c4b38058..4f6586d7404 100644
--- a/searchcore/CMakeLists.txt
+++ b/searchcore/CMakeLists.txt
@@ -67,6 +67,7 @@ vespa_define_module(
     src/tests/proton/attribute/attributes_state_explorer
     src/tests/proton/attribute/document_field_populator
     src/tests/proton/attribute/exclusive_attribute_read_accessor
+    src/tests/proton/attribute/imported_attributes_context
     src/tests/proton/attribute/imported_attributes_repo
     src/tests/proton/bucketdb/bucketdb
     src/tests/proton/common
diff --git a/searchcore/src/tests/proton/attribute/imported_attributes_context/CMakeLists.txt b/searchcore/src/tests/proton/attribute/imported_attributes_context/CMakeLists.txt
new file mode 100644
index 00000000000..58d0e87f53c
--- /dev/null
+++ b/searchcore/src/tests/proton/attribute/imported_attributes_context/CMakeLists.txt
@@ -0,0 +1,8 @@
+# Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+vespa_add_executable(searchcore_imported_attributes_context_test_app TEST
+    SOURCES
+    imported_attributes_context_test.cpp
+    DEPENDS
+    searchcore_attribute
+)
+vespa_add_test(NAME searchcore_imported_attributes_context_test_app COMMAND searchcore_imported_attributes_context_test_app)
diff --git a/searchcore/src/tests/proton/attribute/imported_attributes_context/DESC b/searchcore/src/tests/proton/attribute/imported_attributes_context/DESC
new file mode 100644
index 00000000000..02050ecaf20
--- /dev/null
+++ b/searchcore/src/tests/proton/attribute/imported_attributes_context/DESC
@@ -0,0 +1 @@
+imported_attributes_context test. Take a look at imported_attributes_context_test.cpp for details.
diff --git a/searchcore/src/tests/proton/attribute/imported_attributes_context/FILES b/searchcore/src/tests/proton/attribute/imported_attributes_context/FILES
new file mode 100644
index 00000000000..6ff2c1b61f6
--- /dev/null
+++ b/searchcore/src/tests/proton/attribute/imported_attributes_context/FILES
@@ -0,0 +1 @@
+imported_attributes_context_test.cpp
diff --git a/searchcore/src/tests/proton/attribute/imported_attributes_context/imported_attributes_context_test.cpp b/searchcore/src/tests/proton/attribute/imported_attributes_context/imported_attributes_context_test.cpp
new file mode 100644
index 00000000000..6f6ef33e4f2
--- /dev/null
+++ b/searchcore/src/tests/proton/attribute/imported_attributes_context/imported_attributes_context_test.cpp
@@ -0,0 +1,159 @@
+// 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("imported_attributes_context_test");
+#include <vespa/vespalib/testkit/testapp.h>
+
+#include <vespa/searchcore/proton/attribute/imported_attributes_context.h>
+#include <vespa/searchcore/proton/attribute/imported_attributes_repo.h>
+#include <vespa/searchlib/attribute/attribute.h>
+#include <vespa/searchlib/attribute/attributefactory.h>
+#include <vespa/searchlib/attribute/imported_attribute_vector.h>
+#include <thread>
+
+using namespace proton;
+using search::AttributeVector;
+using search::attribute::BasicType;
+using search::attribute::Config;
+using search::attribute::IAttributeVector;
+using search::attribute::ImportedAttributeVector;
+using search::attribute::ReferenceAttribute;
+using generation_t = AttributeVector::generation_t;
+
+ReferenceAttribute::SP
+createReferenceAttribute(const vespalib::string &name)
+{
+    return std::make_shared<ReferenceAttribute>(name, Config(BasicType::REFERENCE));
+}
+
+AttributeVector::SP
+createTargetAttribute(const vespalib::string &name)
+{
+    return search::AttributeFactory::createAttribute(name, Config(BasicType::STRING));
+}
+
+void
+addDoc(AttributeVector &attr)
+{
+    attr.addDocs(1);
+    attr.commit();
+}
+
+bool
+hasActiveEnumGuards(AttributeVector &attr)
+{
+    bool result;
+    std::thread thread([&result,&attr]() { result = attr.hasActiveEnumGuards(); });
+    thread.join();
+    return result;
+}
+
+void
+assertGuards(AttributeVector &attr, generation_t expCurrentGeneration, generation_t expFirstUsedGeneration, bool expHasActiveEnumGuards)
+{
+    EXPECT_EQUAL(expCurrentGeneration, attr.getCurrentGeneration());
+    EXPECT_EQUAL(expFirstUsedGeneration, attr.getFirstUsedGeneration());
+    EXPECT_EQUAL(expHasActiveEnumGuards, hasActiveEnumGuards(attr));
+}
+
+void
+addDocAndAssertGuards(AttributeVector &attr, generation_t expCurrentGeneration, generation_t expFirstUsedGeneration, bool expHasActiveEnumGuards)
+{
+    addDoc(attr);
+    assertGuards(attr, expCurrentGeneration, expFirstUsedGeneration, expHasActiveEnumGuards);
+}
+
+struct Fixture {
+    ImportedAttributesRepo repo;
+    std::unique_ptr<ImportedAttributesContext> ctx;
+    Fixture()
+        : repo(),
+          ctx(std::make_unique<ImportedAttributesContext>(repo))
+    {
+    }
+    Fixture &addAttribute(const vespalib::string &name) {
+        auto attr = std::make_shared<ImportedAttributeVector>(name,
+                                                              createReferenceAttribute(name + "_ref"),
+                                                              createTargetAttribute(name + "_target"));
+        repo.add(name, attr);
+        return *this;
+    }
+    AttributeVector::SP getTargetAttribute(const vespalib::string &importedName) const {
+        return repo.get(importedName)->getTargetAttribute();
+    }
+    void clearContext() {
+        ctx.reset();
+    }
+};
+
+TEST_F("require that attributes can be retrieved", Fixture)
+{
+    f.addAttribute("foo").addAttribute("bar");
+    EXPECT_EQUAL("foo", f.ctx->getAttribute("foo")->getName());
+    EXPECT_EQUAL("bar", f.ctx->getAttribute("bar")->getName());
+    EXPECT_EQUAL("bar", f.ctx->getAttribute("bar")->getName());
+    EXPECT_TRUE(f.ctx->getAttribute("not_found") == nullptr);
+}
+
+TEST_F("require that stable enum attributes can be retrieved", Fixture)
+{
+    f.addAttribute("foo").addAttribute("bar");
+    EXPECT_EQUAL("foo", f.ctx->getAttributeStableEnum("foo")->getName());
+    EXPECT_EQUAL("bar", f.ctx->getAttributeStableEnum("bar")->getName());
+    EXPECT_EQUAL("bar", f.ctx->getAttributeStableEnum("bar")->getName());
+    EXPECT_TRUE(f.ctx->getAttributeStableEnum("not_found") == nullptr);
+}
+
+TEST_F("require that all attributes can be retrieved", Fixture)
+{
+    f.addAttribute("foo").addAttribute("bar");
+    std::vector<const IAttributeVector *> list;
+    f.ctx->getAttributeList(list);
+    EXPECT_EQUAL(2u, list.size());
+    EXPECT_EQUAL("bar", list[0]->getName());
+    EXPECT_EQUAL("foo", list[1]->getName());
+}
+
+TEST_F("require that guards are cached", Fixture)
+{
+    f.addAttribute("foo");
+    auto targetAttr = f.getTargetAttribute("foo");
+    TEST_DO(addDocAndAssertGuards(*targetAttr, 2, 2, false));
+
+    f.ctx->getAttribute("foo"); // guard is taken and cached
+    TEST_DO(addDocAndAssertGuards(*targetAttr, 4, 2, false));
+
+    f.clearContext(); // guard is released
+    TEST_DO(addDocAndAssertGuards(*targetAttr, 6, 6, false));
+}
+
+TEST_F("require that stable enum guards are cached", Fixture)
+{
+    f.addAttribute("foo");
+    auto targetAttr = f.getTargetAttribute("foo");
+    TEST_DO(addDocAndAssertGuards(*targetAttr, 2, 2, false));
+
+    f.ctx->getAttributeStableEnum("foo"); // enum guard is taken and cached
+    TEST_DO(addDocAndAssertGuards(*targetAttr, 4, 2, true));
+
+    f.clearContext(); // guard is released
+    TEST_DO(addDocAndAssertGuards(*targetAttr, 6, 6, false));
+}
+
+TEST_F("require that stable enum guards can be released", Fixture)
+{
+    f.addAttribute("foo");
+    auto targetAttr = f.getTargetAttribute("foo");
+    TEST_DO(addDocAndAssertGuards(*targetAttr, 2, 2, false));
+
+    f.ctx->getAttributeStableEnum("foo"); // enum guard is taken and cached
+    TEST_DO(addDocAndAssertGuards(*targetAttr, 4, 2, true));
+
+    f.ctx->releaseEnumGuards();
+    TEST_DO(addDocAndAssertGuards(*targetAttr, 6, 6, false));
+}
+
+TEST_MAIN()
+{
+    TEST_RUN_ALL();
+}
diff --git a/searchcore/src/tests/proton/attribute/imported_attributes_repo/imported_attributes_repo_test.cpp b/searchcore/src/tests/proton/attribute/imported_attributes_repo/imported_attributes_repo_test.cpp
index f37fd2c47cb..d9b4031e22f 100644
--- a/searchcore/src/tests/proton/attribute/imported_attributes_repo/imported_attributes_repo_test.cpp
+++ b/searchcore/src/tests/proton/attribute/imported_attributes_repo/imported_attributes_repo_test.cpp
@@ -63,6 +63,17 @@ TEST_F("require that not-found attribute returns nullptr", Fixture)
     EXPECT_EQUAL(f.get("not_found").get(), notFound);
 }
 
+TEST_F("require that all attributes can be retrieved", Fixture)
+{
+    f.add(createAttr("foo"));
+    f.add(createAttr("bar"));
+    std::vector<ImportedAttributeVector::SP> list;
+    f.repo.getAll(list);
+    EXPECT_EQUAL(2u, list.size());
+    EXPECT_EQUAL("bar", list[0]->getName());
+    EXPECT_EQUAL("foo", list[1]->getName());
+}
+
 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 dc6b9c5e8d1..6e85cc79638 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/CMakeLists.txt
+++ b/searchcore/src/vespa/searchcore/proton/attribute/CMakeLists.txt
@@ -23,6 +23,7 @@ vespa_add_library(searchcore_attribute STATIC
     exclusive_attribute_read_accessor.cpp
     filter_attribute_manager.cpp
     flushableattribute.cpp
+    imported_attributes_context.cpp
     imported_attributes_repo.cpp
     initialized_attributes_result.cpp
     sequential_attributes_initializer.cpp
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.cpp b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.cpp
new file mode 100644
index 00000000000..63ae28444d3
--- /dev/null
+++ b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.cpp
@@ -0,0 +1,94 @@
+// 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 "imported_attributes_context.h"
+#include "imported_attributes_repo.h"
+#include <vespa/searchlib/attribute/attributeguard.h>
+#include <vespa/searchlib/attribute/imported_attribute_vector.h>
+#include <vespa/vespalib/stllike/hash_map.hpp>
+
+using search::attribute::IAttributeVector;
+using search::attribute::ImportedAttributeVector;
+using LockGuard = std::lock_guard<std::mutex>;
+
+namespace proton {
+
+ImportedAttributesContext::GuardedAttribute::GuardedAttribute(ImportedAttributeVector::SP attr,
+                                                              bool stableEnumGuard)
+    : _attr(std::move(attr)),
+      _guard(stableEnumGuard ? _attr->acquireEnumGuard() : _attr->acquireGuard())
+{
+}
+
+ImportedAttributesContext::GuardedAttribute::~GuardedAttribute()
+{
+}
+
+const IAttributeVector *
+ImportedAttributesContext::GuardedAttribute::get() const
+{
+    return _attr.get();
+}
+
+const IAttributeVector *
+ImportedAttributesContext::getOrCacheAttribute(const vespalib::string &name,
+                                               AttributeCache &attributes,
+                                               bool stableEnumGuard,
+                                               const LockGuard &) const
+{
+    auto itr = attributes.find(name);
+    if (itr != attributes.end()) {
+        return itr->second.get();
+    }
+    ImportedAttributeVector::SP result = _repo.get(name);
+    if (result.get() != nullptr) {
+        attributes.emplace(name, GuardedAttribute(result, stableEnumGuard));
+        return result.get();
+    } else {
+        return nullptr;
+    }
+}
+
+ImportedAttributesContext::ImportedAttributesContext(const ImportedAttributesRepo &repo)
+    : _repo(repo),
+      _guardedAttributes(),
+      _enumGuardedAttributes(),
+      _cacheMutex()
+{
+}
+
+ImportedAttributesContext::~ImportedAttributesContext()
+{
+}
+
+const IAttributeVector *
+ImportedAttributesContext::getAttribute(const vespalib::string &name) const
+{
+    LockGuard guard(_cacheMutex);
+    return getOrCacheAttribute(name, _guardedAttributes, false, guard);
+}
+
+const IAttributeVector *
+ImportedAttributesContext::getAttributeStableEnum(const vespalib::string &name) const
+{
+    LockGuard guard(_cacheMutex);
+    return getOrCacheAttribute(name, _enumGuardedAttributes, true, guard);
+}
+
+void
+ImportedAttributesContext::getAttributeList(std::vector<const IAttributeVector *> &list) const
+{
+    std::vector<ImportedAttributeVector::SP> attributes;
+    _repo.getAll(attributes);
+    for (const auto &attr : attributes) {
+        list.push_back(getAttribute(attr->getName()));
+    }
+}
+
+void
+ImportedAttributesContext::releaseEnumGuards()
+{
+    LockGuard guard(_cacheMutex);
+    _enumGuardedAttributes.clear();
+}
+    
+}
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.h b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.h
new file mode 100644
index 00000000000..0fae4d015ce
--- /dev/null
+++ b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_context.h
@@ -0,0 +1,69 @@
+// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+#pragma once
+
+#include <vespa/searchcommon/attribute/iattributecontext.h>
+#include <vespa/vespalib/stllike/hash_fun.h>
+#include <vespa/vespalib/stllike/hash_map.h>
+#include <mutex>
+#include <unordered_map>
+
+namespace search {
+class AttributeGuard;
+namespace attribute {
+class IAttributeVector;
+class ImportedAttributeVector;
+}}
+
+namespace proton {
+
+class ImportedAttributesRepo;
+
+/**
+ * Short lived context class that gives access to all imported attributes in a given repo.
+ *
+ * Attribute guards and enum guards are cached in this class and released upon destruction.
+ */
+class ImportedAttributesContext : public search::attribute::IAttributeContext {
+private:
+    using AttributeGuard = search::AttributeGuard;
+    using IAttributeVector = search::attribute::IAttributeVector;
+    using ImportedAttributeVector = search::attribute::ImportedAttributeVector;
+
+    class GuardedAttribute {
+    private:
+        std::shared_ptr<ImportedAttributeVector> _attr;
+        std::unique_ptr<AttributeGuard> _guard;
+
+    public:
+        GuardedAttribute(std::shared_ptr<ImportedAttributeVector> attr, bool stableEnumGuard);
+        ~GuardedAttribute();
+        GuardedAttribute(GuardedAttribute &&rhs) = default;
+        GuardedAttribute &operator=(GuardedAttribute &&rhs) = default;
+        const IAttributeVector *get() const;
+    };
+
+    using AttributeCache = std::unordered_map<vespalib::string, GuardedAttribute, vespalib::hash<vespalib::string>>;
+    using LockGuard = std::lock_guard<std::mutex>;
+
+    const ImportedAttributesRepo &_repo;
+    mutable AttributeCache _guardedAttributes;
+    mutable AttributeCache _enumGuardedAttributes;
+    mutable std::mutex _cacheMutex;
+
+    const IAttributeVector *getOrCacheAttribute(const vespalib::string &name,
+                                                AttributeCache &attributes,
+                                                bool stableEnumGuard,
+                                                const LockGuard &) const;
+
+public:
+    ImportedAttributesContext(const ImportedAttributesRepo &repo);
+    ~ImportedAttributesContext();
+
+    // Implements search::attribute::IAttributeContext
+    virtual const IAttributeVector *getAttribute(const vespalib::string &name) const override;
+    virtual const IAttributeVector *getAttributeStableEnum(const vespalib::string &name) const override;
+    virtual void getAttributeList(std::vector<const IAttributeVector *> &list) const override;
+    virtual void releaseEnumGuards() override;
+};
+
+}
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_repo.cpp b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_repo.cpp
index d7bacd3e429..d7a87609995 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_repo.cpp
+++ b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_repo.cpp
@@ -2,6 +2,7 @@
 #include <vespa/fastos/fastos.h>
 #include "imported_attributes_repo.h"
 #include <vespa/searchlib/attribute/imported_attribute_vector.h>
+#include <vespa/vespalib/stllike/hash_map.hpp>
 
 namespace proton {
 
@@ -33,4 +34,13 @@ ImportedAttributesRepo::get(const vespalib::string &name) const
     return ImportedAttributeVector::SP();
 }
 
+void
+ImportedAttributesRepo::getAll(std::vector<std::shared_ptr<ImportedAttributeVector>> &result) const
+{
+    result.reserve(_repo.size());
+    for (const auto &itr : _repo) {
+        result.push_back(itr.second);
+    }
+}
+
 }
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_repo.h b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_repo.h
index 98983e2e698..497773722e8 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_repo.h
+++ b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_repo.h
@@ -1,8 +1,9 @@
 // Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #pragma once
 
-#include <map>
+#include <vespa/vespalib/stllike/hash_map.h>
 #include <vespa/vespalib/stllike/string.h>
+#include <vector>
 
 namespace search { namespace attribute { class ImportedAttributeVector; } }
 
@@ -14,7 +15,7 @@ namespace proton {
 class ImportedAttributesRepo {
 private:
     using ImportedAttributeVector = search::attribute::ImportedAttributeVector;
-    using Repo = std::map<vespalib::string, std::shared_ptr<ImportedAttributeVector>>;
+    using Repo = vespalib::hash_map<vespalib::string, std::shared_ptr<ImportedAttributeVector>>;
 
     Repo _repo;
 
@@ -24,6 +25,7 @@ public:
     ~ImportedAttributesRepo();
     void add(const vespalib::string &name, std::shared_ptr<ImportedAttributeVector> attr);
     std::shared_ptr<ImportedAttributeVector> get(const vespalib::string &name) const;
+    void getAll(std::vector<std::shared_ptr<ImportedAttributeVector>> &result) const;
     size_t size() const { return _repo.size(); }
 };
 
diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.cpp b/searchlib/src/vespa/searchlib/attribute/attributevector.cpp
index 9e75ae89e2f..318eab5d96e 100644
--- a/searchlib/src/vespa/searchlib/attribute/attributevector.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/attributevector.cpp
@@ -791,6 +791,20 @@ AttributeVector::onInitSave()
     return std::unique_ptr<AttributeSaver>();
 }
 
+bool
+AttributeVector::hasActiveEnumGuards()
+{
+    std::unique_lock<std::shared_timed_mutex> lock(_enumLock, std::defer_lock);
+    for (size_t i = 0; i < 1000; ++i) {
+        // Note: Need to run this in loop as try_lock() is allowed to fail spuriously and return false
+        // even if the mutex is not currently locked by any other thread.
+        if (lock.try_lock()) {
+            return false;
+        }
+    }
+    return true;
+}
+
 IExtendAttribute *AttributeVector::getExtendInterface() { return nullptr; }
 
 uint64_t
diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.h b/searchlib/src/vespa/searchlib/attribute/attributevector.h
index e942fc39539..b01c06e7c75 100644
--- a/searchlib/src/vespa/searchlib/attribute/attributevector.h
+++ b/searchlib/src/vespa/searchlib/attribute/attributevector.h
@@ -289,6 +289,11 @@ public:
         return _genHandler.getCurrentGeneration();
     }
 
+    /**
+     * Used for unit testing. Must not be called from the thread owning the enum guard(s).
+     */
+    bool hasActiveEnumGuards();
+
     virtual IExtendAttribute * getExtendInterface();
 
 protected: