Delay notifyRemoveDone() until after remove batch has completed.

5 files changed, 78 insertions, 12 deletions

diff --git a/searchcore/src/vespa/searchcore/proton/server/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/server/CMakeLists.txt
index 70adc7e47b6..c5b0cd70527 100644
--- a/searchcore/src/vespa/searchcore/proton/server/CMakeLists.txt
+++ b/searchcore/src/vespa/searchcore/proton/server/CMakeLists.txt
@@ -77,6 +77,7 @@ vespa_add_library(searchcore_server STATIC
     pruneremoveddocumentsjob.cpp
     putdonecontext.cpp
     reconfig_params.cpp
+    remove_batch_done_context.cpp
     removedonecontext.cpp
     removedonetask.cpp
     replaypacketdispatcher.cpp
diff --git a/searchcore/src/vespa/searchcore/proton/server/remove_batch_done_context.cpp b/searchcore/src/vespa/searchcore/proton/server/remove_batch_done_context.cpp
new file mode 100644
index 00000000000..b0ece5f35a1
--- /dev/null
+++ b/searchcore/src/vespa/searchcore/proton/server/remove_batch_done_context.cpp
@@ -0,0 +1,27 @@
+// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include "remove_batch_done_context.h"
+#include <vespa/searchcore/proton/reference/i_gid_to_lid_change_handler.h>
+
+namespace proton {
+
+RemoveBatchDoneContext::RemoveBatchDoneContext(vespalib::Executor &executor,
+                                               vespalib::Executor::Task::UP task,
+                                               IGidToLidChangeHandler &gidToLidChangeHandler,
+                                               std::vector<document::GlobalId> gidsToRemove,
+                                               search::SerialNum serialNum)
+    : search::ScheduleTaskCallback(executor, std::move(task)),
+      _gidToLidChangeHandler(gidToLidChangeHandler),
+      _gidsToRemove(std::move(gidsToRemove)),
+      _serialNum(serialNum)
+{
+}
+
+RemoveBatchDoneContext::~RemoveBatchDoneContext()
+{
+    for (const auto &gid : _gidsToRemove) {
+        _gidToLidChangeHandler.notifyRemoveDone(gid, _serialNum);
+    }
+}
+
+}  // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/server/remove_batch_done_context.h b/searchcore/src/vespa/searchcore/proton/server/remove_batch_done_context.h
new file mode 100644
index 00000000000..2a93239574a
--- /dev/null
+++ b/searchcore/src/vespa/searchcore/proton/server/remove_batch_done_context.h
@@ -0,0 +1,38 @@
+// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#pragma once
+
+#include <vespa/searchlib/common/scheduletaskcallback.h>
+#include <vespa/document/base/globalid.h>
+#include <vespa/searchlib/common/serialnum.h>
+#include <vector>
+
+namespace proton
+{
+
+class IGidToLidChangeHandler;
+
+/**
+ * Context class for document batch remove that notifies gid to lid
+ * change handler about each remove done and schedules a
+ * task when instance is destroyed. Typically a shared pointer to an
+ * instance is passed around to multiple worker threads that performs
+ * portions of a larger task before dropping the shared pointer.
+ */
+class RemoveBatchDoneContext : public search::ScheduleTaskCallback
+{
+    IGidToLidChangeHandler         &_gidToLidChangeHandler;
+    std::vector<document::GlobalId> _gidsToRemove;
+    search::SerialNum               _serialNum;
+
+public:
+    RemoveBatchDoneContext(vespalib::Executor &executor,
+                           vespalib::Executor::Task::UP task,
+                           IGidToLidChangeHandler &gidToLidChangeHandler,
+                           std::vector<document::GlobalId> gidsToRemove,
+                           search::SerialNum serialNum);
+
+    virtual ~RemoveBatchDoneContext();
+};
+
+}  // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp
index c6f888545b1..5f0fc3b8aa6 100644
--- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp
@@ -7,6 +7,7 @@
 #include "removedonecontext.h"
 #include "storeonlyfeedview.h"
 #include "updatedonecontext.h"
+#include "remove_batch_done_context.h"
 #include <vespa/document/datatype/documenttype.h>
 #include <vespa/searchcore/proton/common/commit_time_tracker.h>
 #include <vespa/searchcore/proton/common/feedtoken.h>
@@ -658,25 +659,26 @@ StoreOnlyFeedView::removeDocuments(const RemoveDocumentsOperation &op, bool remo
     const LidVector &lidsToRemove(ctx->getLidVector());
     bool useDMS = useDocumentMetaStore(serialNum);
     bool explicitReuseLids = false;
+    std::vector<document::GlobalId> gidsToRemove;
     if (useDMS) {
-        std::vector<document::GlobalId> gidsToRemove(getGidsToRemove(_metaStore, lidsToRemove));
+        gidsToRemove = getGidsToRemove(_metaStore, lidsToRemove);
         for (const auto &gid : gidsToRemove) {
             _gidToLidChangeHandler.notifyRemove(gid, serialNum);
         }
         _metaStore.removeBatch(lidsToRemove, ctx->getDocIdLimit());
         _metaStore.commit(serialNum, serialNum);
-        for (const auto &gid : gidsToRemove) {
-            _gidToLidChangeHandler.notifyRemoveDone(gid, serialNum);
-        }
         explicitReuseLids = _lidReuseDelayer.delayReuse(lidsToRemove);
     }
     std::shared_ptr<search::IDestructorCallback> onWriteDone;
+    vespalib::Executor::Task::UP removeBatchDoneTask;
+    if (explicitReuseLids) {
+        removeBatchDoneTask = makeLambdaTask([=]() { _metaStore.removeBatchComplete(lidsToRemove); });
+    } else {
+        removeBatchDoneTask = makeLambdaTask([]() {});
+    }
+    onWriteDone = std::make_shared<RemoveBatchDoneContext>(_writeService.master(), std::move(removeBatchDoneTask),
+                                                           _gidToLidChangeHandler, std::move(gidsToRemove), serialNum);
     if (remove_index_and_attributes) {
-        if (explicitReuseLids) {
-            onWriteDone = std::make_shared<search::ScheduleTaskCallback>(
-                    _writeService.master(),
-                    makeLambdaTask([=]() { _metaStore.removeBatchComplete(lidsToRemove); }));
-        }
         removeIndexedFields(serialNum, lidsToRemove, immediateCommit, onWriteDone);
         removeAttributes(serialNum, lidsToRemove, immediateCommit, onWriteDone);
     }
@@ -685,9 +687,6 @@ StoreOnlyFeedView::removeDocuments(const RemoveDocumentsOperation &op, bool remo
             removeSummary(serialNum, lid);
         }
     }
-    if (explicitReuseLids && !onWriteDone) {
-        _metaStore.removeBatchComplete(lidsToRemove);
-    }
     return lidsToRemove.size();
 }
 
diff --git a/searchlib/src/vespa/searchlib/common/scheduletaskcallback.h b/searchlib/src/vespa/searchlib/common/scheduletaskcallback.h
index 4e12fb9d1b8..27bbe751532 100644
--- a/searchlib/src/vespa/searchlib/common/scheduletaskcallback.h
+++ b/searchlib/src/vespa/searchlib/common/scheduletaskcallback.h
@@ -2,6 +2,7 @@
 #pragma once
 
 #include "idestructorcallback.h"
+#include <vespa/vespalib/util/executor.h>
 
 namespace search
 {