diff options
Diffstat (limited to 'searchlib')
8 files changed, 99 insertions, 51 deletions
diff --git a/searchlib/src/tests/attribute/reference_attribute/reference_attribute_test.cpp b/searchlib/src/tests/attribute/reference_attribute/reference_attribute_test.cpp index 1b1bc1f8796..07fb14c809b 100644 --- a/searchlib/src/tests/attribute/reference_attribute/reference_attribute_test.cpp +++ b/searchlib/src/tests/attribute/reference_attribute/reference_attribute_test.cpp @@ -188,9 +188,13 @@ struct Fixture void notifyReferencedRemove(const GlobalId &gid) { _attr->notifyReferencedRemove(gid); } - void populateReferencedLids() { + void setGidToLidMapperFactory(std::shared_ptr<MyGidToLidMapperFactory> factory) { + _attr->setGidToLidMapperFactory(factory); _attr->populateReferencedLids(); } + uint32_t getUniqueGids() { + return getStatus().getNumUniqueValues(); + } }; TEST_F("require that we can instantiate reference attribute", Fixture) @@ -289,8 +293,8 @@ TEST_F("require that we can save and load attribute", Fixture) TEST_F("require that update() uses gid-mapper to set referenced lid", Fixture) { f.ensureDocIdLimit(6); - std::shared_ptr<search::IGidToLidMapperFactory> factory = std::make_shared<MyGidToLidMapperFactory>(); - f._attr->setGidToLidMapperFactory(factory); + auto factory = std::make_shared<MyGidToLidMapperFactory>(); + f.setGidToLidMapperFactory(factory); f.set(1, toGid(doc1)); f.set(2, toGid(doc2)); f.set(4, toGid(doc1)); @@ -340,9 +344,8 @@ void preparePopulateReferencedLids(Fixture &f) void checkPopulateReferencedLids(Fixture &f) { - std::shared_ptr<search::IGidToLidMapperFactory> factory = std::make_shared<MyGidToLidMapperFactory>(); - f._attr->setGidToLidMapperFactory(factory); - f.populateReferencedLids(); + auto factory = std::make_shared<MyGidToLidMapperFactory>(); + f.setGidToLidMapperFactory(factory); TEST_DO(f.assertRefLid(1, 10)); TEST_DO(f.assertRefLid(2, 17)); TEST_DO(f.assertRefLid(3, 10)); @@ -388,33 +391,35 @@ TEST_F("Require that notifyReferencedPut and notifyReferencedRemove changes reve TEST_DO(f.assertLids(11, { })); } -TEST_F("Require that reverse mapping recovers from temporary out of order glitch", Fixture) +TEST_F("Require that we track unique gids", Fixture) { - auto factory = std::make_shared<MyGidToLidMapperFactory>(); - f._attr->setGidToLidMapperFactory(factory); - f.ensureDocIdLimit(4); + EXPECT_EQUAL(0u, f.getUniqueGids()); + f.notifyReferencedPut(toGid(doc1), 10); + EXPECT_EQUAL(1u, f.getUniqueGids()); + f.ensureDocIdLimit(3); f.set(1, toGid(doc1)); - f.set(2, toGid(doc2)); - /* - * Changes in gid to lid mapping can be visible via gid to lid - * mapper before notifications arrive. If a lid is reused in the - * referenced document meta store then multiple entries in - * reference store might temporarily map to the same referenced - * lid. - */ - factory->remove(doc1); // remove referenced document - factory->add(doc3, 10); // reuse lid for new referenced document - f.set(3, toGid(doc3)); + f.commit(); + EXPECT_EQUAL(1u, f.getUniqueGids()); TEST_DO(f.assertRefLid(1, 10)); + TEST_DO(f.assertLids(10, { 1 })); + f.set(2, toGid(doc2)); + f.commit(); + EXPECT_EQUAL(2u, f.getUniqueGids()); + TEST_DO(f.assertRefLid(2, 0)); + f.notifyReferencedPut(toGid(doc2), 17); + EXPECT_EQUAL(2u, f.getUniqueGids()); TEST_DO(f.assertRefLid(2, 17)); - TEST_DO(f.assertRefLid(3, 10)); - // Notify reference attribute about gid to lid mapping changes + TEST_DO(f.assertLids(17, { 2 })); + f.clear(1); + f.notifyReferencedRemove(toGid(doc2)); + EXPECT_EQUAL(2u, f.getUniqueGids()); + TEST_DO(f.assertNoRefLid(1)); + TEST_DO(f.assertRefLid(2, 0)); + TEST_DO(f.assertLids(10, { })); + TEST_DO(f.assertLids(17, { })); + f.clear(2); f.notifyReferencedRemove(toGid(doc1)); - f.notifyReferencedPut(toGid(doc3), 10); - TEST_DO(f.assertRefLid(1, 0)); - TEST_DO(f.assertRefLid(2, 17)); - TEST_DO(f.assertRefLid(3, 10)); - TEST_DO(f.assertLids(10, { 3 })); + EXPECT_EQUAL(0u, f.getUniqueGids()); } TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp b/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp index 2cb05b14503..8666f0bdced 100644 --- a/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp @@ -247,9 +247,6 @@ ReferenceAttribute::update(DocId doc, const GlobalId &gid) assert(doc < _indices.size()); EntryRef oldRef = _indices[doc]; Reference refToAdd(gid); - if (_gidToLidMapperFactory) { - refToAdd.setLid(_gidToLidMapperFactory->getMapper()->mapGidToLid(gid)); - } EntryRef newRef = _store.add(refToAdd).ref(); std::atomic_thread_fence(std::memory_order_release); _indices[doc] = newRef; @@ -320,14 +317,23 @@ ReferenceAttribute::setGidToLidMapperFactory(std::shared_ptr<IGidToLidMapperFact } void -ReferenceAttribute::notifyReferencedPut(const GlobalId &gid, DocId referencedLid) +ReferenceAttribute::notifyReferencedPutNoCommit(const GlobalId &gid, DocId referencedLid) { + assert(referencedLid != 0); EntryRef ref = _store.find(gid); - if (ref.valid()) { - const auto &entry = _store.get(ref); - _referenceMappings.notifyReferencedPut(entry, referencedLid); - commit(); + if (!ref.valid() || _store.get(ref).lid() == 0) { + Reference refToAdd(gid); + ref = _store.add(refToAdd).ref(); } + const auto &entry = _store.get(ref); + _referenceMappings.notifyReferencedPut(entry, referencedLid); +} + +void +ReferenceAttribute::notifyReferencedPut(const GlobalId &gid, DocId referencedLid) +{ + notifyReferencedPutNoCommit(gid, referencedLid); + commit(); } void @@ -336,24 +342,42 @@ ReferenceAttribute::notifyReferencedRemove(const GlobalId &gid) EntryRef ref = _store.find(gid); if (ref.valid()) { const auto &entry = _store.get(ref); + uint32_t oldReferencedLid = entry.lid(); _referenceMappings.notifyReferencedRemove(entry); + if (oldReferencedLid != 0) { + _store.remove(ref); + } commit(); } } +namespace { + +class ReferencedLidPopulator : public IGidToLidMapperVisitor +{ + ReferenceAttribute &_attr; +public: + ReferencedLidPopulator(ReferenceAttribute &attr) + : IGidToLidMapperVisitor(), + _attr(attr) + { + } + virtual ~ReferencedLidPopulator() override { } + virtual void visit(const document::GlobalId &gid, uint32_t lid) const override { + _attr.notifyReferencedPutNoCommit(gid, lid); + } +}; + +} + void ReferenceAttribute::populateReferencedLids() { if (_gidToLidMapperFactory) { std::unique_ptr<IGidToLidMapper> mapperUP = _gidToLidMapperFactory->getMapper(); const IGidToLidMapper &mapper = *mapperUP; - const auto &store = _store; - const auto saver = _store.getSaver(); - saver.foreach_key([&store,&mapper,this](EntryRef ref) - { const Reference &entry = store.get(ref); - entry.setLid(mapper.mapGidToLid(entry.gid())); - _referenceMappings.syncMappings(entry); - }); + ReferencedLidPopulator populator(*this); + mapper.foreach(populator); } commit(); } diff --git a/searchlib/src/vespa/searchlib/attribute/reference_attribute.h b/searchlib/src/vespa/searchlib/attribute/reference_attribute.h index ce7c908db99..7387c13fc50 100644 --- a/searchlib/src/vespa/searchlib/attribute/reference_attribute.h +++ b/searchlib/src/vespa/searchlib/attribute/reference_attribute.h @@ -78,6 +78,7 @@ public: ReverseMappingRefs getReverseMappingRefs() const { return _referenceMappings.getReverseMappingRefs(); } const ReverseMapping &getReverseMapping() const { return _referenceMappings.getReverseMapping(); } + void notifyReferencedPutNoCommit(const GlobalId &gid, DocId referencedLid); void notifyReferencedPut(const GlobalId &gid, DocId referencedLid); void notifyReferencedRemove(const GlobalId &gid); void populateReferencedLids(); diff --git a/searchlib/src/vespa/searchlib/attribute/reference_mappings.cpp b/searchlib/src/vespa/searchlib/attribute/reference_mappings.cpp index f2462cdc40d..ae93dfba225 100644 --- a/searchlib/src/vespa/searchlib/attribute/reference_mappings.cpp +++ b/searchlib/src/vespa/searchlib/attribute/reference_mappings.cpp @@ -54,13 +54,6 @@ ReferenceMappings::syncReverseMappingIndices(const Reference &entry) } void -ReferenceMappings::syncMappings(const Reference &entry) -{ - syncReverseMappingIndices(entry); - syncForwardMapping(entry); -} - -void ReferenceMappings::removeReverseMapping(const Reference &entry, uint32_t lid) { EntryRef revMapIdx = entry.revMapIdx(); diff --git a/searchlib/src/vespa/searchlib/attribute/reference_mappings.h b/searchlib/src/vespa/searchlib/attribute/reference_mappings.h index 73754d9cb13..43ae8a5bdb8 100644 --- a/searchlib/src/vespa/searchlib/attribute/reference_mappings.h +++ b/searchlib/src/vespa/searchlib/attribute/reference_mappings.h @@ -61,7 +61,6 @@ public: void notifyReferencedRemove(const Reference &entry); void removeReverseMapping(const Reference &entry, uint32_t lid); void addReverseMapping(const Reference &entry, uint32_t lid); - void syncMappings(const Reference &entry); // Maintain size of mapping from lid to referenced lid void onAddDocs(uint32_t docIdLimit); diff --git a/searchlib/src/vespa/searchlib/common/i_gid_to_lid_mapper.h b/searchlib/src/vespa/searchlib/common/i_gid_to_lid_mapper.h index f5317035a21..8a9e2dbd84f 100644 --- a/searchlib/src/vespa/searchlib/common/i_gid_to_lid_mapper.h +++ b/searchlib/src/vespa/searchlib/common/i_gid_to_lid_mapper.h @@ -3,12 +3,24 @@ #pragma once #include <cstdint> +#include <functional> namespace document { class GlobalId; } namespace search { /* + * Interface for gid to lid mapper visitor. + */ +class IGidToLidMapperVisitor +{ +public: + virtual ~IGidToLidMapperVisitor() { } + virtual void visit(const document::GlobalId &gid, uint32_t lid) const = 0; +}; + + +/* * Interface class for mapping from gid to lid. Instances should be short * lived due to implementations containing read guards preventing resource * reuse. @@ -18,6 +30,7 @@ class IGidToLidMapper public: virtual ~IGidToLidMapper() { } virtual uint32_t mapGidToLid(const document::GlobalId &gid) const = 0; + virtual void foreach(const IGidToLidMapperVisitor &visitor) const = 0; }; } // namespace search diff --git a/searchlib/src/vespa/searchlib/test/imported_attribute_fixture.h b/searchlib/src/vespa/searchlib/test/imported_attribute_fixture.h index edb1a789284..85a2b140039 100644 --- a/searchlib/src/vespa/searchlib/test/imported_attribute_fixture.h +++ b/searchlib/src/vespa/searchlib/test/imported_attribute_fixture.h @@ -101,6 +101,11 @@ struct ImportedAttributeFixture { void map_reference(DocId from_lid, GlobalId via_gid, DocId to_lid) { assert(from_lid < reference_attr->getNumDocs()); mapper_factory->_map[via_gid] = to_lid; + if (to_lid != 0) { + reference_attr->notifyReferencedPut(via_gid, to_lid); + } else { + reference_attr->notifyReferencedRemove(via_gid); + } reference_attr->update(from_lid, via_gid); reference_attr->commit(); } diff --git a/searchlib/src/vespa/searchlib/test/mock_gid_to_lid_mapping.h b/searchlib/src/vespa/searchlib/test/mock_gid_to_lid_mapping.h index c2ffc2c4dfd..86203c19970 100644 --- a/searchlib/src/vespa/searchlib/test/mock_gid_to_lid_mapping.h +++ b/searchlib/src/vespa/searchlib/test/mock_gid_to_lid_mapping.h @@ -29,6 +29,14 @@ struct MockGidToLidMapper : public search::IGidToLidMapper { return 0u; } } + + void foreach(const search::IGidToLidMapperVisitor &visitor) const override { + for (const auto &kv : _map) { + if (kv.second != 0) { + visitor.visit(kv.first, kv.second); + } + } + } }; struct MockGidToLidMapperFactory : public search::IGidToLidMapperFactory { |