Merge pull request #3295 from vespa-engine/toregge/keep-better-track-of-referenced-lids

Keep track of mapping from gid to referenced lid in reference attribute

4 files changed, 57 insertions, 31 deletions

diff --git a/searchcore/src/tests/proton/reference/gid_to_lid_change_listener/gid_to_lid_change_listener_test.cpp b/searchcore/src/tests/proton/reference/gid_to_lid_change_listener/gid_to_lid_change_listener_test.cpp
index 08787e41438..4c6f9017000 100644
--- a/searchcore/src/tests/proton/reference/gid_to_lid_change_listener/gid_to_lid_change_listener_test.cpp
+++ b/searchcore/src/tests/proton/reference/gid_to_lid_change_listener/gid_to_lid_change_listener_test.cpp
@@ -7,6 +7,7 @@
 #include <vespa/searchcore/proton/reference/gid_to_lid_change_listener.h>
 #include <vespa/searchlib/common/i_gid_to_lid_mapper_factory.h>
 #include <vespa/searchlib/common/i_gid_to_lid_mapper.h>
+#include <vespa/searchlib/test/mock_gid_to_lid_mapping.h>
 #include <map>
 #include <vespa/log/log.h>
 LOG_SETUP("gid_to_lid_change_listener_test");
@@ -19,6 +20,7 @@ using search::attribute::Config;
 using search::attribute::BasicType;
 using search::attribute::Reference;
 using search::attribute::ReferenceAttribute;
+using search::attribute::test::MockGidToLidMapperFactory;
 
 namespace proton {
 
@@ -32,39 +34,14 @@ vespalib::string doc1("id:test:music::1");
 vespalib::string doc2("id:test:music::2");
 vespalib::string doc3("id:test:music::3");
 
-using MockGidToLidMap = std::map<GlobalId, uint32_t>;
-
-struct MyGidToLidMapper : public search::IGidToLidMapper
+struct MyGidToLidMapperFactory : public MockGidToLidMapperFactory
 {
-    const MockGidToLidMap &_map;
-    MyGidToLidMapper(const MockGidToLidMap &map)
-        : _map(map)
-    {
-    }
-    virtual uint32_t mapGidToLid(const document::GlobalId &gid) const override {
-        auto itr = _map.find(gid);
-        if (itr != _map.end()) {
-            return itr->second;
-        } else {
-            return 0u;
-        }
-    }
-};
-
-struct MyGidToLidMapperFactory : public search::IGidToLidMapperFactory
-{
-    MockGidToLidMap _map;
-
     MyGidToLidMapperFactory()
-        : _map()
+        : MockGidToLidMapperFactory()
     {
         _map.insert({toGid(doc1), 10});
         _map.insert({toGid(doc2), 17});
     }
-
-    virtual std::unique_ptr<search::IGidToLidMapper> getMapper() const override {
-        return std::make_unique<MyGidToLidMapper>(_map);
-    }
 };
 
 }
diff --git a/searchcore/src/tests/proton/reference/gid_to_lid_mapper/gid_to_lid_mapper_test.cpp b/searchcore/src/tests/proton/reference/gid_to_lid_mapper/gid_to_lid_mapper_test.cpp
index c3181d27bef..5445ba9c585 100644
--- a/searchcore/src/tests/proton/reference/gid_to_lid_mapper/gid_to_lid_mapper_test.cpp
+++ b/searchcore/src/tests/proton/reference/gid_to_lid_mapper/gid_to_lid_mapper_test.cpp
@@ -3,6 +3,7 @@
 #include <vespa/searchcore/proton/bucketdb/bucket_db_owner.h>
 #include <vespa/searchcore/proton/documentmetastore/documentmetastore.h>
 #include <vespa/vespalib/stllike/string.h>
+#include <vespa/vespalib/test/insertion_operators.h>
 #include <vespa/document/base/documentid.h>
 #include <vespa/document/bucket/bucketid.h>
 #include <vespa/searchcore/proton/reference/gid_to_lid_mapper.h>
@@ -40,6 +41,31 @@ void assertLid(const std::unique_ptr<search::IGidToLidMapper> &mapper, const ves
     EXPECT_EQUAL(lid, mapper->mapGidToLid(toGid(docId)));
 }
 
+using GidMap = std::map<GlobalId, uint32_t>;
+
+struct GidCollector : public search::IGidToLidMapperVisitor
+{
+    GidMap &_map;
+    GidCollector(GidMap &map)
+        : IGidToLidMapperVisitor(),
+          _map(map)
+    {
+    }
+    virtual void visit(const document::GlobalId &gid, uint32_t lid) const override { _map.insert(std::make_pair(gid, lid)); }
+};
+
+GidMap collectGids(const std::unique_ptr<search::IGidToLidMapper> &mapper)
+{
+    GidMap result;
+    mapper->foreach(GidCollector(result));
+    return result;
+}
+
+void assertGids(const GidMap &expGids, const GidMap &gids)
+{
+    EXPECT_EQUAL(expGids, gids);
+}
+
 }
 
 struct Fixture
@@ -136,6 +162,17 @@ TEST_F("Test that mapper holds read guard", Fixture)
     TEST_DO(f.assertPut(doc3, 2, 10, 7, [&]() -> auto & { return mapper; }));
 }
 
+TEST_F("Test that gid mapper can iterate over known gids", Fixture)
+{
+    auto factory = f.getGidToLidMapperFactory();
+    auto mapper = factory->getMapper();
+    TEST_DO(assertGids({{toGid(doc1), 4}, {toGid(doc2), 7}}, collectGids(mapper)));
+    f.put(doc3);
+    TEST_DO(assertGids({{toGid(doc1), 4}, {toGid(doc2), 7}, {toGid(doc3), 1}}, collectGids(mapper)));
+    f.remove(4);
+    TEST_DO(assertGids({{toGid(doc2), 7}, {toGid(doc3), 1}}, collectGids(mapper)));
+}
+
 }
 
 TEST_MAIN()
diff --git a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_mapper.cpp b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_mapper.cpp
index 7364e6ee6c0..db990dff3bc 100644
--- a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_mapper.cpp
+++ b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_mapper.cpp
@@ -1,11 +1,12 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "gid_to_lid_mapper.h"
+#include <vespa/searchcore/proton/documentmetastore/documentmetastore.h>
 
 namespace proton {
 
 GidToLidMapper::GidToLidMapper(vespalib::GenerationHandler::Guard &&guard,
-                               const search::IDocumentMetaStore &dms)
+                               const DocumentMetaStore &dms)
     : _guard(std::move(guard)),
       _dms(dms)
 {
@@ -26,4 +27,13 @@ GidToLidMapper::mapGidToLid(const document::GlobalId &gid) const
     }
 }
 
+void
+GidToLidMapper::foreach(const search::IGidToLidMapperVisitor &visitor) const
+{
+    const auto &dms = _dms;
+    dms.beginFrozen().foreach_key([&dms,&visitor](uint32_t lid)
+                                  {  visitor.visit(dms.getRawMetaData(lid).getGid(), lid); });
+}
+
+
 } // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_mapper.h b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_mapper.h
index 7c5dc840af5..ed6bf91dbd3 100644
--- a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_mapper.h
+++ b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_mapper.h
@@ -4,10 +4,11 @@
 
 #include <vespa/searchlib/common/i_gid_to_lid_mapper.h>
 #include <vespa/vespalib/util/generationhandler.h>
-#include <vespa/searchlib/common/idocumentmetastore.h>
 
 namespace proton {
 
+class DocumentMetaStore;
+
 /*
  * Class for mapping from gid to lid. Instances should be short lived
  * due to read guards preventing resource reuse.
@@ -15,12 +16,13 @@ namespace proton {
 class GidToLidMapper : public search::IGidToLidMapper
 {
     vespalib::GenerationHandler::Guard _guard;
-    const search::IDocumentMetaStore &_dms;
+    const DocumentMetaStore &_dms;
 public:
     GidToLidMapper(vespalib::GenerationHandler::Guard &&guard,
-                   const search::IDocumentMetaStore &dms);
+                   const DocumentMetaStore &dms);
     virtual ~GidToLidMapper();
     virtual uint32_t mapGidToLid(const document::GlobalId &gid) const override;
+    virtual void foreach(const search::IGidToLidMapperVisitor &visitor) const override;
 };
 
 } // namespace proton