Retry docsum fetching if there has been lidspace compaction and your request might have been affected.

6 files changed, 116 insertions, 152 deletions

diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/document_meta_store_adapter.h b/searchcore/src/vespa/searchcore/proton/documentmetastore/document_meta_store_adapter.h
index 974e9111306..6f133e1a14f 100644
--- a/searchcore/src/vespa/searchcore/proton/documentmetastore/document_meta_store_adapter.h
+++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/document_meta_store_adapter.h
@@ -13,22 +13,24 @@ namespace proton {
 class DocumentMetaStoreAdapter : public IDocumentMetaStore
 {
 protected:
-    virtual void doCommit(search::SerialNum firstSerialNum,
-                          search::SerialNum lastSerialNum) = 0;
+    using SerialNum = search::SerialNum;
+    virtual void doCommit(SerialNum firstSerialNum, SerialNum lastSerialNum) = 0;
     virtual DocId doGetCommittedDocIdLimit() const = 0;
     virtual void doRemoveAllOldGenerations() = 0;
-
+    virtual uint64_t doGetCurrentGeneration() const = 0;
 public:
-    virtual void commit(search::SerialNum firstSerialNum,
-                        search::SerialNum lastSerialNum) override {
+    void commit(SerialNum firstSerialNum, SerialNum lastSerialNum) override {
         doCommit(firstSerialNum, lastSerialNum);
     }
-    virtual DocId getCommittedDocIdLimit() const override {
+    DocId getCommittedDocIdLimit() const override {
         return doGetCommittedDocIdLimit();
     }
-    virtual void removeAllOldGenerations() override {
+    void removeAllOldGenerations() override {
         doRemoveAllOldGenerations();
     }
+    uint64_t getCurrentGeneration() const override {
+        return doGetCurrentGeneration();
+    }
 };
 
 } // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h
index 5b0f2c1fe91..78fdbc2ad0e 100644
--- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h
+++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h
@@ -57,6 +57,7 @@ public:
     using DocumentMetaStoreAttribute::commit;
     using DocumentMetaStoreAttribute::getCommittedDocIdLimit;
     using DocumentMetaStoreAttribute::removeAllOldGenerations;
+    using DocumentMetaStoreAttribute::getCurrentGeneration;
 
 private:
     // maps from lid -> meta data
@@ -87,19 +88,15 @@ private:
     VESPA_DLL_LOCAL void ensureSpace(DocId lid);
     bool insert(DocId lid, const RawDocumentMetaData &metaData);
 
-    const GlobalId &
-    getRawGid(DocId lid) const
-    {
-        return getRawMetaData(lid).getGid();
-    }
+    const GlobalId & getRawGid(DocId lid) const { return getRawMetaData(lid).getGid(); }
 
-    virtual void onUpdateStat() override;
+    void onUpdateStat() override;
 
     // Implements AttributeVector
-    virtual void onGenerationChange(generation_t generation) override;
-    virtual void removeOldGenerations(generation_t firstUsed) override;
-    virtual std::unique_ptr<search::AttributeSaver> onInitSave() override;
-    virtual bool onLoad() override;
+    void onGenerationChange(generation_t generation) override;
+    void removeOldGenerations(generation_t firstUsed) override;
+    std::unique_ptr<search::AttributeSaver> onInitSave() override;
+    bool onLoad() override;
 
     bool
     checkBuckets(const GlobalId &gid,
@@ -122,23 +119,23 @@ private:
                                    const RawDocumentMetaData &newMetaData);
 
     void unload();
-
-    virtual void
-    updateActiveLids(const BucketId &bucketId, bool active) override;
+    void updateActiveLids(const BucketId &bucketId, bool active) override;
 
     /**
      * Implements DocumentMetaStoreAdapter
      */
-    virtual void doCommit(search::SerialNum firstSerialNum,
-                          search::SerialNum lastSerialNum) override {
+    void doCommit(SerialNum firstSerialNum, SerialNum lastSerialNum) override {
         commit(firstSerialNum, lastSerialNum);
     }
-    virtual DocId doGetCommittedDocIdLimit() const override {
+    DocId doGetCommittedDocIdLimit() const override {
         return getCommittedDocIdLimit();
     }
-    virtual void doRemoveAllOldGenerations() override {
+    void doRemoveAllOldGenerations() override {
         removeAllOldGenerations();
     }
+    uint64_t doGetCurrentGeneration() const override {
+        return getCurrentGeneration();
+    }
 
     VESPA_DLL_LOCAL DocId readNextDoc(documentmetastore::Reader & reader, TreeType::Builder & treeBuilder);
 
@@ -161,8 +158,8 @@ public:
     /**
      * Implements documentmetastore::IStore.
      */
-    virtual Result inspectExisting(const GlobalId &gid) const override;
-    virtual Result inspect(const GlobalId &gid) override;
+    Result inspectExisting(const GlobalId &gid) const override;
+    Result inspect(const GlobalId &gid) override;
     /**
      * Puts the given <lid, meta data> pair to this store.
      * This function should only be called before constructFreeList()
@@ -171,17 +168,17 @@ public:
      * map is then re-built the same way it was originally where add()
      * was used to create the <lid, gid> pairs.
      **/
-    virtual Result put(const GlobalId &gid,
-                       const BucketId &bucketId,
-                       const Timestamp &timestamp,
-                       DocId lid) override;
-    virtual bool updateMetaData(DocId lid,
-                                const BucketId &bucketId,
-                                const Timestamp &timestamp) override;
-    virtual bool remove(DocId lid) override;
-
-    virtual BucketId getBucketOf(const vespalib::GenerationHandler::Guard & guard, uint32_t lid) const override;
-    virtual vespalib::GenerationHandler::Guard getGuard() const override;
+    Result put(const GlobalId &gid,
+               const BucketId &bucketId,
+               const Timestamp &timestamp,
+               DocId lid) override;
+    bool updateMetaData(DocId lid,
+                        const BucketId &bucketId,
+                        const Timestamp &timestamp) override;
+    bool remove(DocId lid) override;
+
+    BucketId getBucketOf(const vespalib::GenerationHandler::Guard & guard, uint32_t lid) const override;
+    vespalib::GenerationHandler::Guard getGuard() const override;
 
     /**
      * Put lid on a hold list, for later reuse.  Typically called
@@ -189,45 +186,28 @@ public:
      * document has been torn down (memory index, attribute vectors,
      * document store).
      */
-    virtual void removeComplete(DocId lid) override;
-    virtual void move(DocId fromLid, DocId toLid) override;
+    void removeComplete(DocId lid) override;
+    void move(DocId fromLid, DocId toLid) override;
     bool validLidFast(DocId lid) const { return _lidAlloc.validLid(lid); }
-    virtual bool validLid(DocId lid) const override {
-        return validLidFast(lid);
-    }
-    virtual void removeBatch(const std::vector<DocId> &lidsToRemove,
-                             const DocId docIdLimit) override;
+    bool validLid(DocId lid) const override { return validLidFast(lid); }
+    void removeBatch(const std::vector<DocId> &lidsToRemove, const DocId docIdLimit) override;
     /**
      * Put lids on a hold list, for laster reuse.
      */
-    virtual void
-    removeBatchComplete(const std::vector<DocId> &lidsToRemove) override;
-    virtual const RawDocumentMetaData &
-    getRawMetaData(DocId lid) const override { return _metaDataStore[lid]; }
-
-
+    void removeBatchComplete(const std::vector<DocId> &lidsToRemove) override;
+    const RawDocumentMetaData & getRawMetaData(DocId lid) const override { return _metaDataStore[lid]; }
 
     /**
      * Implements search::IDocumentMetaStore
      **/
-    virtual bool getGid(DocId lid, GlobalId &gid) const override;
-    virtual bool getLid(const GlobalId & gid, DocId &lid) const override;
-    virtual search::DocumentMetaData
-    getMetaData(const GlobalId &gid) const override;
-    virtual void
-    getMetaData(const BucketId &bucketId,
-                search::DocumentMetaData::Vector &result) const override;
-    virtual DocId getNumUsedLids() const override {
-        return _lidAlloc.getNumUsedLids();
-    }
-    virtual DocId getNumActiveLids() const override {
-        return _lidAlloc.getNumActiveLids();
-    }
-    virtual search::LidUsageStats getLidUsageStats() const override;
-    virtual search::queryeval::Blueprint::UP
-    createBlackListBlueprint() const override;
-
-
+    bool getGid(DocId lid, GlobalId &gid) const override;
+    bool getLid(const GlobalId & gid, DocId &lid) const override;
+    search::DocumentMetaData getMetaData(const GlobalId &gid) const override;
+    void getMetaData(const BucketId &bucketId, search::DocumentMetaData::Vector &result) const override;
+    DocId   getNumUsedLids() const override { return _lidAlloc.getNumUsedLids(); }
+    DocId getNumActiveLids() const override { return _lidAlloc.getNumActiveLids(); }
+    search::LidUsageStats getLidUsageStats() const override;
+    search::queryeval::Blueprint::UP createBlackListBlueprint() const override;
 
     /**
      * Implements search::AttributeVector
@@ -237,69 +217,44 @@ public:
               const search::AttributeVector::SearchContext::Params & params)
         const override;
 
-
-
     /**
      * Implements proton::IDocumentMetaStore
      */
-    virtual void constructFreeList() override;
-
-    virtual Iterator begin() const override;
-
-    virtual Iterator lowerBound(const BucketId &bucketId) const override;
-
-    virtual Iterator upperBound(const BucketId &bucketId) const override;
-
-    virtual Iterator lowerBound(const GlobalId &gid) const override;
-
-    virtual Iterator upperBound(const GlobalId &gid) const override;
+    void constructFreeList() override;
+    Iterator begin() const override;
+    Iterator lowerBound(const BucketId &bucketId) const override;
+    Iterator upperBound(const BucketId &bucketId) const override;
+    Iterator lowerBound(const GlobalId &gid) const override;
+    Iterator upperBound(const GlobalId &gid) const override;
 
-    virtual void
-    getLids(const BucketId &bucketId, std::vector<DocId> &lids) override;
+    void getLids(const BucketId &bucketId, std::vector<DocId> &lids) override;
 
-    virtual search::AttributeGuard getActiveLidsGuard() const override {
+    search::AttributeGuard getActiveLidsGuard() const override {
         return _lidAlloc.getActiveLidsGuard();
     }
 
-    virtual bool getFreeListActive() const override {
+    bool getFreeListActive() const override {
         return _lidAlloc.isFreeListConstructed();
     }
 
-    virtual void compactLidSpace(DocId wantedLidLimit) override;
+    void compactLidSpace(DocId wantedLidLimit) override;
+    void holdUnblockShrinkLidSpace() override;
+    bool canShrinkLidSpace() const override;
 
-    virtual void holdUnblockShrinkLidSpace() override;
-
-    virtual bool canShrinkLidSpace() const override;
-
-    virtual search::SerialNum getLastSerialNum() const override {
+    SerialNum getLastSerialNum() const override {
         return getStatus().getLastSyncToken();
     }
 
-
-
     /**
      * Implements documentmetastore::IBucketHandler.
      */
-    virtual BucketDBOwner &getBucketDB() const override {
-        return *_bucketDB;
-    }
-
-    virtual bucketdb::BucketDeltaPair
-    handleSplit(const bucketdb::SplitBucketSession &session) override;
-
-    virtual bucketdb::BucketDeltaPair
-    handleJoin(const bucketdb::JoinBucketsSession &session) override;
-
-    virtual void
-    setBucketState(const BucketId &bucketId, bool active) override;
+    BucketDBOwner &getBucketDB() const override { return *_bucketDB; }
 
-    virtual void
-    populateActiveBuckets(const BucketId::List &buckets) override;
-
-
-
-    ConstIterator
-    beginFrozen() const;
+    bucketdb::BucketDeltaPair handleSplit(const bucketdb::SplitBucketSession &session) override;
+    bucketdb::BucketDeltaPair handleJoin(const bucketdb::JoinBucketsSession &session) override;
+    void setBucketState(const BucketId &bucketId, bool active) override;
+    void populateActiveBuckets(const BucketId::List &buckets) override;
+    ConstIterator beginFrozen() const;
 
     const vespalib::GenerationHandler & getGenerationHandler() const {
         return AttributeVector::getGenerationHandler();
@@ -310,22 +265,16 @@ public:
     }
 
     const BitAttribute &getActiveLids() const { return _lidAlloc.getActiveLids(); }
-
-    virtual void
-    clearDocs(DocId lidLow, DocId lidLimit) override;
+    void clearDocs(DocId lidLow, DocId lidLimit) override;
 
     /*
      * Called by document db executor to unblock shrinking of lid
      * space after all lids held by holdLid() operations have been
      * unheld.
      */
-    void
-    unblockShrinkLidSpace();
-
-    virtual void
-    onShrinkLidSpace() override;
-
-    virtual uint64_t getEstimatedSaveByteSize() const override;
+    void unblockShrinkLidSpace();
+    void onShrinkLidSpace() override;
+    uint64_t getEstimatedSaveByteSize() const override;
 };
 
 }
@@ -335,5 +284,3 @@ BTreeIterator<proton::DocumentMetaStore::DocId,
               search::btree::BTreeNoLeafData,
               search::btree::NoAggregated,
               const proton::DocumentMetaStore::KeyComp &>;
-
-
diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp
index aa02d32ac5d..00d74196d18 100644
--- a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp
+++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp
@@ -45,6 +45,7 @@ struct StupidMetaStore : IDocumentMetaStore {
     DocId getCommittedDocIdLimit() const override { return 1; }
     DocId getNumUsedLids() const override { return 0; }
     DocId getNumActiveLids() const override { return 0; }
+    uint64_t getCurrentGeneration() const override { return 0; }
     LidUsageStats getLidUsageStats() const override { return LidUsageStats(); }
     Blueprint::UP createBlackListBlueprint() const override {
         return Blueprint::UP();
diff --git a/searchcore/src/vespa/searchcore/proton/server/searchview.cpp b/searchcore/src/vespa/searchcore/proton/server/searchview.cpp
index 15ee799405b..103de39eaf7 100644
--- a/searchcore/src/vespa/searchcore/proton/server/searchview.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/searchview.cpp
@@ -49,6 +49,17 @@ convertGidsToLids(const DocsumRequest & request,
     }
 }
 
+bool
+requestHasLidAbove(const DocsumRequest & request, uint32_t docIdLimit)
+{
+    for (const DocsumRequest::Hit & h : request.hits) {
+        if (h.docid >= docIdLimit) {
+            return true;
+        }
+    }
+    return false;
+}
+
 /**
  * Maps the lids in the reply to gids using the original request.
  **/
@@ -80,15 +91,12 @@ createEmptyReply(const DocsumRequest & request)
 
 }
 
-
 SearchView::SearchView(const ISummaryManager::ISummarySetup::SP & summarySetup,
                        const MatchView::SP & matchView)
     : ISearchHandler(),
       _summarySetup(summarySetup),
       _matchView(matchView)
-{
-}
-
+{ }
 
 DocsumReply::UP
 SearchView::getDocsums(const DocsumRequest & req)
@@ -99,21 +107,24 @@ SearchView::getDocsums(const DocsumRequest & req)
                      req.resultClassName.c_str(), req.hits.size());
         return createEmptyReply(req);
     }
-    IDocumentMetaStoreContext::IReadGuard::UP readGuard = _matchView->getDocumentMetaStore()->getReadGuard();
-    DocsumReply::UP reply = getDocsumsInternal(req, readGuard->get());
-    if (false) {
-        LOG(info, "Must refetch docsums since the lids have moved.");
-        reply = getDocsumsInternal(req, readGuard->get());
+    SearchView::InternalDocsumReply reply = getDocsumsInternal(req);
+    while ( ! reply.second ) {
+        reply = getDocsumsInternal(req);
     }
     if ( ! req.useRootSlime()) {
-        convertLidsToGids(*reply, req);
+        LOG(info, "Must refetch docsums since the lids have moved.");
+        convertLidsToGids(*reply.first, req);
     }
-    return reply;
+    return std::move(reply.first);
 }
 
-DocsumReply::UP
-SearchView::getDocsumsInternal(const DocsumRequest & req, const search::IDocumentMetaStore & metaStore)
+SearchView::InternalDocsumReply
+SearchView::getDocsumsInternal(const DocsumRequest & req)
 {
+    IDocumentMetaStoreContext::IReadGuard::UP readGuard = _matchView->getDocumentMetaStore()->getReadGuard();
+    const search::IDocumentMetaStore & metaStore = readGuard->get();
+    uint64_t startGeneration = readGuard->get().getCurrentGeneration();
+
     convertGidsToLids(req, metaStore, _matchView->getDocIdLimit().get());
     IDocsumStore::UP store(_summarySetup->createDocsumStore(req.resultClassName));
     Matcher::SP matcher = _matchView->getMatcher(req.ranking);
@@ -121,7 +132,14 @@ SearchView::getDocsumsInternal(const DocsumRequest & req, const search::IDocumen
     DocsumContext::UP ctx(new DocsumContext(req, _summarySetup->getDocsumWriter(), *store, matcher,
                                             mctx->getSearchContext(), mctx->getAttributeContext(),
                                             *_summarySetup->getAttributeManager(), *getSessionManager()));
-    return ctx->getDocsums();
+    SearchView::InternalDocsumReply reply(ctx->getDocsums(), true);
+    uint64_t endGeneration = readGuard->get().getCurrentGeneration();
+    if (startGeneration != endGeneration) {
+        if (requestHasLidAbove(req, metaStore.getNumUsedLids())) {
+            reply.second = false;
+        }
+    }
+    return reply;
 }
 
 SearchReply::UP
diff --git a/searchcore/src/vespa/searchcore/proton/server/searchview.h b/searchcore/src/vespa/searchcore/proton/server/searchview.h
index 573a4157fd2..b17cd61bc66 100644
--- a/searchcore/src/vespa/searchcore/proton/server/searchview.h
+++ b/searchcore/src/vespa/searchcore/proton/server/searchview.h
@@ -10,13 +10,9 @@ namespace proton {
 
 class SearchView : public ISearchHandler
 {
-private:
-    ISummaryManager::ISummarySetup::SP _summarySetup;
-    MatchView::SP                      _matchView;
-
-    DocsumReply::UP getDocsumsInternal(const DocsumRequest & req, const search::IDocumentMetaStore & metaStore);
-    using IndexSearchable = searchcorespi::IndexSearchable;
 public:
+    using IndexSearchable = searchcorespi::IndexSearchable;
+    using InternalDocsumReply = std::pair<DocsumReply::UP, bool>;
     typedef std::shared_ptr<SearchView> SP;
 
     SearchView(const ISummaryManager::ISummarySetup::SP &summarySetup, const MatchView::SP &matchView);
@@ -29,17 +25,14 @@ public:
     const matching::SessionManager::SP  & getSessionManager()    const { return _matchView->getSessionManager(); }
     const IDocumentMetaStoreContext::SP & getDocumentMetaStore() const { return _matchView->getDocumentMetaStore(); }
     DocIdLimit &getDocIdLimit() const { return _matchView->getDocIdLimit(); }
-
     matching::MatchingStats getMatcherStats(const vespalib::string &rankProfile) const { return _matchView->getMatcherStats(rankProfile); }
 
-    /**
-     * Implements ISearchHandler
-     */
     DocsumReply::UP getDocsums(const DocsumRequest & req) override;
-
-    SearchReply::UP match(const ISearchHandler::SP &self,
-                          const SearchRequest &req,
-                          vespalib::ThreadBundle &threadBundle) const override;
+    SearchReply::UP match(const ISearchHandler::SP &self, const SearchRequest &req, vespalib::ThreadBundle &threadBundle) const override;
+private:
+    InternalDocsumReply getDocsumsInternal(const DocsumRequest & req);
+    ISummaryManager::ISummarySetup::SP _summarySetup;
+    MatchView::SP                      _matchView;
 };
 
 } // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/test/document_meta_store_observer.h b/searchcore/src/vespa/searchcore/proton/test/document_meta_store_observer.h
index c2a461eb90b..88e658dd841 100644
--- a/searchcore/src/vespa/searchcore/proton/test/document_meta_store_observer.h
+++ b/searchcore/src/vespa/searchcore/proton/test/document_meta_store_observer.h
@@ -46,6 +46,9 @@ struct DocumentMetaStoreObserver : public IDocumentMetaStore
     virtual search::queryeval::Blueprint::UP createBlackListBlueprint() const override {
         return _store.createBlackListBlueprint();
     }
+    uint64_t getCurrentGeneration() const override {
+        return _store.getCurrentGeneration();
+    }
 
 
     /**