diff options
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 ¶ms, 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) { |