Recover from temporary glitch in reverse mapping.

2 files changed, 41 insertions, 1 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 73e27ca4f7d..337cdcc30e8 100644
--- a/searchlib/src/tests/attribute/reference_attribute/reference_attribute_test.cpp
+++ b/searchlib/src/tests/attribute/reference_attribute/reference_attribute_test.cpp
@@ -41,6 +41,17 @@ struct MyGidToLidMapperFactory : public search::attribute::test::MockGidToLidMap
         _map.insert({toGid(doc1), 10});
         _map.insert({toGid(doc2), 17});
     }
+
+    void add(vespalib::stringref docId, uint32_t lid) {
+        auto insres = _map.insert({ toGid(docId), lid });
+        if (!insres.second) {
+            insres.first->second = lid;
+        }
+    }
+
+    void remove(vespalib::stringref docId) {
+        _map.erase(toGid(docId));
+    }
 };
 
 class LidCollector
@@ -371,4 +382,33 @@ TEST_F("Require that notifyGidToLidChange changes reverse mapping", Fixture)
     TEST_DO(f.assertLids(11, { }));
 }
 
+TEST_F("Require that reverse mapping recovers from temporary out of order glitch", Fixture)
+{
+    auto factory = std::make_shared<MyGidToLidMapperFactory>();
+    f._attr->setGidToLidMapperFactory(factory);
+    f.ensureDocIdLimit(4);
+    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));
+    TEST_DO(f.assertRefLid(1, 10));
+    TEST_DO(f.assertRefLid(2, 17));
+    TEST_DO(f.assertRefLid(3, 10));
+    // Notify reference attribute about gid to lid mapping changes
+    f.notifyGidToLidChange(toGid(doc1), 0);
+    f.notifyGidToLidChange(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 }));
+}
+
 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 17afa6f9a2d..5a088fd1d64 100644
--- a/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp
@@ -366,8 +366,8 @@ ReferenceAttribute::notifyGidToLidChange(const GlobalId &gid, DocId referencedLi
                 _reverseMappingIndices[oldReferencedLid] = EntryRef();
             }
             entry.setLid(referencedLid);
-            syncReverseMappingIndices(entry);
         }
+        syncReverseMappingIndices(entry);
         commit();
     }
 }