Change API for IGidToLidChangeHandler:

Replace notifyGidToLidChanged() with notifyPut(), notifyRemove() and notifyRemoveDone().

Intended usage:

  Call rotifyRemove() at start of remove operation then call notifyRemoveDone()
  at completion of remove operation.

  Call notifyPut() at end of put operation.

For now, call them at start of put/remove operations to get old behavior.

Track pending remove operations.
Ignore put for gid while remove is pending.
Merge pending remove operations.

10 files changed, 160 insertions, 21 deletions

diff --git a/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp b/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp
index 4970ad867ce..2ed60f3c078 100644
--- a/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp
+++ b/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp
@@ -198,13 +198,23 @@ public:
     {
     }
 
-    virtual void notifyGidToLidChange(document::GlobalId gid, uint32_t lid)  override {
+    virtual void notifyPut(document::GlobalId gid, uint32_t lid, SerialNum)  override {
         _changeGid = gid;
         _changeLid = lid;
         _gidToLid[gid] = lid;
         ++_changes;
     }
 
+    virtual void notifyRemove(document::GlobalId gid, SerialNum)  override {
+        _changeGid = gid;
+        _changeLid = 0;
+        _gidToLid[gid] = 0;
+        ++_changes;
+    }
+
+    virtual void notifyRemoveDone(document::GlobalId, SerialNum)  override {
+    }
+
     void assertChanges(document::GlobalId expGid, uint32_t expLid, uint32_t expChanges) {
         EXPECT_EQUAL(expGid, _changeGid);
         EXPECT_EQUAL(expLid, _changeLid);
diff --git a/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/gid_to_lid_change_handler_test.cpp b/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/gid_to_lid_change_handler_test.cpp
index 2528834e898..c9fe214947c 100644
--- a/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/gid_to_lid_change_handler_test.cpp
+++ b/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/gid_to_lid_change_handler_test.cpp
@@ -14,6 +14,7 @@ LOG_SETUP("gid_to_lid_change_handler_test");
 using document::GlobalId;
 using document::DocumentId;
 using search::makeLambdaTask;
+using search::SerialNum;
 
 namespace proton {
 
@@ -127,8 +128,16 @@ struct Fixture
         _handler->addListener(std::move(listener));
     }
 
-    void notifyGidToLidChange(GlobalId gid, uint32_t lid) {
-        _handler->notifyGidToLidChange(gid, lid);
+    void notifyPut(GlobalId gid, uint32_t lid, SerialNum serialNum) {
+        _handler->notifyPut(gid, lid, serialNum);
+    }
+
+    void notifyRemove(GlobalId gid, SerialNum serialNum) {
+        _handler->notifyRemove(gid, serialNum);
+    }
+
+    void notifyRemoveDone(GlobalId gid, SerialNum serialNum) {
+        _handler->notifyRemoveDone(gid, serialNum);
     }
 
     void removeListeners(const vespalib::string &docTypeName,
@@ -145,7 +154,7 @@ TEST_F("Test that we can register a listener", Fixture)
     TEST_DO(stats.assertCounts(1, 0, 0, 0));
     f.addListener(std::move(listener));
     TEST_DO(stats.assertCounts(1, 1, 0, 0));
-    f.notifyGidToLidChange(toGid(doc1), 10);
+    f.notifyPut(toGid(doc1), 10, 10);
     TEST_DO(stats.assertCounts(1, 1, 0, 1));
     f.removeListeners("testdoc", {});
     TEST_DO(stats.assertCounts(1, 1, 1, 1));
@@ -168,7 +177,7 @@ TEST_F("Test that we can register multiple listeners", Fixture)
     TEST_DO(stats1.assertCounts(1, 1, 0, 0));
     TEST_DO(stats2.assertCounts(1, 1, 0, 0));
     TEST_DO(stats3.assertCounts(1, 1, 0, 0));
-    f.notifyGidToLidChange(toGid(doc1), 10);
+    f.notifyPut(toGid(doc1), 10, 10);
     TEST_DO(stats1.assertCounts(1, 1, 0, 1));
     TEST_DO(stats2.assertCounts(1, 1, 0, 1));
     TEST_DO(stats3.assertCounts(1, 1, 0, 1));
@@ -203,6 +212,50 @@ TEST_F("Test that we keep old listener when registering duplicate", Fixture)
     TEST_DO(stats.assertCounts(2, 1, 1, 0));
 }
 
+TEST_F("Test that put is ignored if we have a pending remove", Fixture)
+{
+    auto &stats = f.addStats();
+    auto listener = std::make_unique<MyListener>(stats, "test", "testdoc");
+    TEST_DO(stats.assertCounts(1, 0, 0, 0));
+    f.addListener(std::move(listener));
+    TEST_DO(stats.assertCounts(1, 1, 0, 0));
+    f.notifyRemove(toGid(doc1), 20);
+    TEST_DO(stats.assertCounts(1, 1, 0, 1));
+    f.notifyPut(toGid(doc1), 10, 10);
+    TEST_DO(stats.assertCounts(1, 1, 0, 1));
+    f.notifyRemoveDone(toGid(doc1), 20);
+    TEST_DO(stats.assertCounts(1, 1, 0, 1));
+    f.notifyPut(toGid(doc1), 11, 30);
+    TEST_DO(stats.assertCounts(1, 1, 0, 2));
+    f.removeListeners("testdoc", {});
+    TEST_DO(stats.assertCounts(1, 1, 1, 2));
+}
+
+TEST_F("Test that pending removes are merged", Fixture)
+{
+    auto &stats = f.addStats();
+    auto listener = std::make_unique<MyListener>(stats, "test", "testdoc");
+    TEST_DO(stats.assertCounts(1, 0, 0, 0));
+    f.addListener(std::move(listener));
+    TEST_DO(stats.assertCounts(1, 1, 0, 0));
+    f.notifyRemove(toGid(doc1), 20);
+    TEST_DO(stats.assertCounts(1, 1, 0, 1));
+    f.notifyRemove(toGid(doc1), 40);
+    TEST_DO(stats.assertCounts(1, 1, 0, 1));
+    f.notifyPut(toGid(doc1), 10, 10);
+    TEST_DO(stats.assertCounts(1, 1, 0, 1));
+    f.notifyRemoveDone(toGid(doc1), 20);
+    TEST_DO(stats.assertCounts(1, 1, 0, 1));
+    f.notifyPut(toGid(doc1), 11, 30);
+    TEST_DO(stats.assertCounts(1, 1, 0, 1));
+    f.notifyRemoveDone(toGid(doc1), 40);
+    TEST_DO(stats.assertCounts(1, 1, 0, 1));
+    f.notifyPut(toGid(doc1), 12, 50);
+    TEST_DO(stats.assertCounts(1, 1, 0, 2));
+    f.removeListeners("testdoc", {});
+    TEST_DO(stats.assertCounts(1, 1, 1, 2));
+}
+
 }
 
 TEST_MAIN()
diff --git a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_handler.cpp b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_handler.cpp
index 5992999bd69..0c0b1258027 100644
--- a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_handler.cpp
+++ b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_handler.cpp
@@ -6,6 +6,7 @@
 #include <vespa/searchcorespi/index/i_thread_service.h>
 #include <vespa/document/base/globalid.h>
 #include <cassert>
+#include <vespa/vespalib/stllike/hash_map.hpp>
 
 using search::makeLambdaTask;
 
@@ -15,7 +16,9 @@ namespace proton {
 GidToLidChangeHandler::GidToLidChangeHandler()
     : _lock(),
       _listeners(),
-      _closed(false)
+      _closed(false),
+      _pendingRemove()
+
 {
 }
 
@@ -24,18 +27,54 @@ GidToLidChangeHandler::~GidToLidChangeHandler()
 {
     assert(_closed);
     assert(_listeners.empty());
+    assert(_pendingRemove.empty());
 }
 
 void
-GidToLidChangeHandler::notifyGidToLidChange(document::GlobalId gid, uint32_t lid)
+GidToLidChangeHandler::notifyGidToLidChange(GlobalId gid, uint32_t lid)
 {
-    lock_guard guard(_lock);
     for (const auto &listener : _listeners) {
         listener->notifyGidToLidChange(gid, lid);
     }
 }
 
 void
+GidToLidChangeHandler::notifyPut(GlobalId gid, uint32_t lid, SerialNum serialNum)
+{
+    lock_guard guard(_lock);
+    auto itr = _pendingRemove.find(gid);
+    if (itr != _pendingRemove.end()) {
+        assert(itr->second > serialNum);
+        return; // Document has already been removed later on
+    }
+    notifyGidToLidChange(gid, lid);
+}
+
+void
+GidToLidChangeHandler::notifyRemove(GlobalId gid, SerialNum serialNum)
+{
+    lock_guard guard(_lock);
+    auto insRes = _pendingRemove.insert(std::make_pair(gid, serialNum));
+    if (!insRes.second) {
+        assert(insRes.first->second < serialNum);
+        insRes.first->second = serialNum;
+    } else {
+        notifyGidToLidChange(gid, 0);
+    }
+}
+
+void
+GidToLidChangeHandler::notifyRemoveDone(GlobalId gid, SerialNum serialNum)
+{
+    lock_guard guard(_lock);
+    auto itr = _pendingRemove.find(gid);
+    assert(itr != _pendingRemove.end() && itr->second >= serialNum);
+    if (itr->second == serialNum) {
+        _pendingRemove.erase(itr);
+    }
+}
+
+void
 GidToLidChangeHandler::close()
 {
     Listeners deferredDelete;
diff --git a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_handler.h b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_handler.h
index 8a569ed1c95..34172683a58 100644
--- a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_handler.h
+++ b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_handler.h
@@ -5,6 +5,8 @@
 #include "i_gid_to_lid_change_handler.h"
 #include <vector>
 #include <mutex>
+#include <vespa/vespalib/stllike/hash_map.h>
+#include <vespa/document/base/globalid.h>
 
 namespace searchcorespi { namespace index { class IThreadService; } }
 
@@ -23,7 +25,9 @@ class GidToLidChangeHandler : public std::enable_shared_from_this<GidToLidChange
     std::mutex _lock;
     Listeners _listeners;
     bool _closed;
+    vespalib::hash_map<GlobalId, SerialNum, GlobalId::hash> _pendingRemove;
 
+    void notifyGidToLidChange(GlobalId gid, uint32_t lid);
 public:
     GidToLidChangeHandler();
     virtual ~GidToLidChangeHandler();
@@ -31,7 +35,9 @@ public:
     /**
      * Notify gid to lid mapping change.
      */
-    virtual void notifyGidToLidChange(document::GlobalId gid, uint32_t lid) override;
+    virtual void notifyPut(GlobalId gid, uint32_t lid, SerialNum serialNum) override;
+    virtual void notifyRemove(GlobalId gid, SerialNum serialNum) override;
+    virtual void notifyRemoveDone(GlobalId gid, SerialNum serialNum) override;
 
     /**
      * Close handler, further notifications are blocked.
diff --git a/searchcore/src/vespa/searchcore/proton/reference/i_gid_to_lid_change_handler.h b/searchcore/src/vespa/searchcore/proton/reference/i_gid_to_lid_change_handler.h
index 73ff140e2c6..a3b1db59abd 100644
--- a/searchcore/src/vespa/searchcore/proton/reference/i_gid_to_lid_change_handler.h
+++ b/searchcore/src/vespa/searchcore/proton/reference/i_gid_to_lid_change_handler.h
@@ -5,6 +5,7 @@
 #include <set>
 #include <memory>
 #include <vespa/vespalib/stllike/string.h>
+#include <vespa/searchlib/common/serialnum.h>
 
 namespace document { class GlobalId; }
 
@@ -19,14 +20,18 @@ class IGidToLidChangeListener;
 class IGidToLidChangeHandler
 {
 public:
+    using SerialNum = search::SerialNum;
+    using GlobalId = document::GlobalId;
+
     virtual ~IGidToLidChangeHandler() { }
 
     virtual void addListener(std::unique_ptr<IGidToLidChangeListener> listener) = 0;
 
     virtual void removeListeners(const vespalib::string &docTypeName,
                                  const std::set<vespalib::string> &keepNames) = 0;
-    virtual void notifyGidToLidChange(document::GlobalId gid, uint32_t lid)  = 0;
-
+    virtual void notifyPut(GlobalId gid, uint32_t lid, SerialNum serialNum) = 0;
+    virtual void notifyRemove(GlobalId gid, SerialNum serialNum) = 0;
+    virtual void notifyRemoveDone(GlobalId gid, SerialNum serialNum) = 0;
 };
 
 } // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp b/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp
index 3a3264336c8..913956c509f 100644
--- a/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp
@@ -262,9 +262,21 @@ SearchableFeedView::forceCommit(SerialNum serialNum, OnForceCommitDoneType onCom
 }
 
 void
-SearchableFeedView::notifyGidToLidChange(const document::GlobalId &gid, uint32_t lid)
+SearchableFeedView::notifyPutGidToLidChange(const document::GlobalId &gid, uint32_t lid, SerialNum serialNum)
 {
-    _gidToLidChangeHandler->notifyGidToLidChange(gid, lid);
+    _gidToLidChangeHandler->notifyPut(gid, lid, serialNum);
+}
+
+void
+SearchableFeedView::notifyRemoveGidToLidChange(const document::GlobalId &gid, SerialNum serialNum)
+{
+    _gidToLidChangeHandler->notifyRemove(gid, serialNum);
+}
+
+void
+SearchableFeedView::notifyRemoveDoneGidToLidChange(const document::GlobalId &gid, SerialNum serialNum)
+{
+    _gidToLidChangeHandler->notifyRemoveDone(gid, serialNum);
 }
 
 } // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.h b/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.h
index c0d9bfcfbc6..4b536d0adde 100644
--- a/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.h
+++ b/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.h
@@ -86,7 +86,9 @@ private:
     void performIndexForceCommit(SerialNum serialNum, OnForceCommitDoneType onCommitDone);
     void forceCommit(SerialNum serialNum, OnForceCommitDoneType onCommitDone) override;
 
-    virtual void notifyGidToLidChange(const document::GlobalId &gid, uint32_t lid) override;
+    virtual void notifyPutGidToLidChange(const document::GlobalId &gid, uint32_t lid, SerialNum serialNum) override;
+    virtual void notifyRemoveGidToLidChange(const document::GlobalId &gid, SerialNum serialNum) override;
+    virtual void notifyRemoveDoneGidToLidChange(const document::GlobalId &gid, SerialNum serialNum) override;
 
 public:
     SearchableFeedView(const StoreOnlyFeedView::Context &storeOnlyCtx, const PersistentParams &params,
diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp
index da3e09bea41..3e7e3d6ddfa 100644
--- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp
@@ -611,16 +611,17 @@ StoreOnlyFeedView::adjustMetaStore(const DocumentOperation &op, const DocumentId
                 op.getLid() != op.getPrevLid())
             {
                 moveMetaData(_metaStore, docId, op);
-                notifyGidToLidChange(docId.getGlobalId(), op.getLid());
+                notifyPutGidToLidChange(docId.getGlobalId(), op.getLid(), serialNum);
             } else {
                 putMetaData(_metaStore, docId, op, _params._subDbType == SubDbType::REMOVED);
                 if (op.getDbDocumentId() != op.getPrevDbDocumentId()) {
-                    notifyGidToLidChange(docId.getGlobalId(), op.getLid());
+                    notifyPutGidToLidChange(docId.getGlobalId(), op.getLid(), serialNum);
                 }
             }
         } else if (op.getValidPrevDbdId(_params._subDbId)) {
             removeMetaData(_metaStore, docId, op, _params._subDbType == SubDbType::REMOVED);
-            notifyGidToLidChange(docId.getGlobalId(), 0u);
+            notifyRemoveGidToLidChange(docId.getGlobalId(), serialNum);
+            notifyRemoveDoneGidToLidChange(docId.getGlobalId(), serialNum);
         }
         _metaStore.commit(serialNum, serialNum);
     }
@@ -651,7 +652,8 @@ StoreOnlyFeedView::removeDocuments(const RemoveDocumentsOperation &op, bool remo
         std::vector<document::GlobalId> gidsToRemove(getGidsToRemove(_metaStore, lidsToRemove));
         _metaStore.removeBatch(lidsToRemove, ctx->getDocIdLimit());
         for (const auto &gid : gidsToRemove) {
-            notifyGidToLidChange(gid, 0u);
+            notifyRemoveGidToLidChange(gid, serialNum);
+            notifyRemoveDoneGidToLidChange(gid, serialNum);
         }
         _metaStore.commit(serialNum, serialNum);
         explicitReuseLids = _lidReuseDelayer.delayReuse(lidsToRemove);
@@ -806,6 +808,12 @@ StoreOnlyFeedView::getDocumentMetaStorePtr() const
 }
 
 void
-StoreOnlyFeedView::notifyGidToLidChange(const document::GlobalId &, uint32_t ) {}
+StoreOnlyFeedView::notifyPutGidToLidChange(const document::GlobalId &, uint32_t, SerialNum) {}
+
+void
+StoreOnlyFeedView::notifyRemoveGidToLidChange(const document::GlobalId &, SerialNum) {}
+
+void
+StoreOnlyFeedView::notifyRemoveDoneGidToLidChange(const document::GlobalId &, SerialNum) {}
 
 } // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h
index 982acf200a7..ec3003fb5d6 100644
--- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h
+++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h
@@ -179,7 +179,9 @@ private:
     // Ack token early if visibility delay is nonzero
     void considerEarlyAck(FeedTokenUP &token, FeedOperation::Type opType);
 
-    virtual void notifyGidToLidChange(const document::GlobalId &gid, uint32_t lid);
+    virtual void notifyPutGidToLidChange(const document::GlobalId &gid, uint32_t lid, SerialNum serialNum);
+    virtual void notifyRemoveGidToLidChange(const document::GlobalId &gid, SerialNum serialNum);
+    virtual void notifyRemoveDoneGidToLidChange(const document::GlobalId &gid, SerialNum serialNum);
 
     void makeUpdatedDocument(SerialNum serialNum, Lid lid, DocumentUpdate::SP upd,
             OnOperationDoneType onWriteDone,PromisedDoc promisedDoc, PromisedStream promisedStream);
diff --git a/searchcore/src/vespa/searchcore/proton/test/mock_gid_to_lid_change_handler.h b/searchcore/src/vespa/searchcore/proton/test/mock_gid_to_lid_change_handler.h
index 2aa613461ef..41efb55e61c 100644
--- a/searchcore/src/vespa/searchcore/proton/test/mock_gid_to_lid_change_handler.h
+++ b/searchcore/src/vespa/searchcore/proton/test/mock_gid_to_lid_change_handler.h
@@ -44,7 +44,9 @@ public:
         _removes.emplace_back(docTypeName, keepNames);
     }
 
-    virtual void notifyGidToLidChange(document::GlobalId, uint32_t)  override { }
+    virtual void notifyPut(document::GlobalId, uint32_t, SerialNum)  override { }
+    virtual void notifyRemove(document::GlobalId, SerialNum)  override { }
+    virtual void notifyRemoveDone(document::GlobalId, SerialNum)  override { }
 
     void assertAdds(const std::vector<AddEntry> &expAdds)
     {