Check gid and bucketid inside the correct master thread to ensure you do the right thing.

3 files changed, 39 insertions, 19 deletions

diff --git a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_common.cpp b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_common.cpp
index 5cc1041efba..b9bb6a7c801 100644
--- a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_common.cpp
+++ b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_common.cpp
@@ -2,11 +2,13 @@
 
 #include "lid_space_common.h"
 
-MyScanIterator::MyScanIterator(const LidVector &lids, bool bucketIdEqualLid)
-    : _lids(lids),
+using vespalib::make_string_short::fmt;
+
+MyScanIterator::MyScanIterator(const MyHandler & handler, const LidVector &lids)
+    : _handler(handler),
+      _lids(lids),
       _itr(_lids.begin()),
-      _validItr(true),
-      _bucketIdEqualLid(bucketIdEqualLid)
+      _validItr(true)
 {}
 MyScanIterator::~MyScanIterator() = default;
 
@@ -23,7 +25,7 @@ search::DocumentMetaData MyScanIterator::next(uint32_t compactLidLimit, bool ret
     if (_itr != _lids.end()) {
         uint32_t lid = *_itr;
         if (lid > compactLidLimit) {
-            return search::DocumentMetaData(lid, TIMESTAMP_1, createBucketId(lid), GID_1);
+            return _handler.getMetaData(lid);
         }
     } else {
         _validItr = false;
@@ -32,7 +34,7 @@ search::DocumentMetaData MyScanIterator::next(uint32_t compactLidLimit, bool ret
 }
 
 document::BucketId
-MyScanIterator::createBucketId(uint32_t lid) const {
+MyHandler::createBucketId(uint32_t lid) const {
     return _bucketIdEqualLid ? document::BucketId(lid) : BUCKET_ID_1;
 }
 
@@ -86,19 +88,24 @@ MyHandler::getLidStatus() const {
 IDocumentScanIterator::UP
 MyHandler::getIterator() const {
     assert(_iteratorCnt < _lids.size());
-    return std::make_unique<MyScanIterator>(_lids[_iteratorCnt++], _bucketIdEqualLid);
+    return std::make_unique<MyScanIterator>(*this, _lids[_iteratorCnt++]);
 }
 
 search::DocumentMetaData
 MyHandler::getMetaData(uint32_t lid) const {
-    return search::DocumentMetaData(lid, TIMESTAMP_1, document::BucketId(lid), GID_1);
+    if (lid < _docs.size()) {
+        return _docs[lid].first;
+    }
+    return search::DocumentMetaData();
 }
 
 MoveOperation::UP
 MyHandler::createMoveOperation(const search::DocumentMetaData &document, uint32_t moveToLid) const {
     assert(document.lid > moveToLid);
     _moveFromLid = document.lid;
-    auto op = std::make_unique<MoveOperation>();
+    const auto & entry = _docs[document.lid];
+    auto op = std::make_unique<MoveOperation>(entry.first.bucketId, entry.first.timestamp, entry.second,
+                                              DbDocumentId(document.lid), 0);
     op->setTargetLid(moveToLid);
     return op;
 }
@@ -128,12 +135,18 @@ MyHandler::MyHandler(bool storeMoveDoneContexts, bool bucketIdEqualLid)
       _bucketIdEqualLid(bucketIdEqualLid),
       _moveDoneContexts(),
       _op_listener(),
-      _rm_listener()
-{}
+      _rm_listener(),
+      _docs()
+{
+    DocBuilder builder = DocBuilder(Schema());
+    for (uint32_t i(0); i < 10; i++) {
+        auto doc = builder.startDocument(fmt("%s%d", DOC_ID.c_str(), i)).endDocument();
+        _docs.emplace_back(DocumentMetaData(i, TIMESTAMP_1, createBucketId(i), doc->getId().getGlobalId()), std::move(doc));
+    }
+}
 
 MyHandler::~MyHandler() = default;
 
-
 void
 MyStorer::appendOperation(const FeedOperation &op, DoneCallback) {
     if (op.getType() == FeedOperation::MOVE) {
diff --git a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_common.h b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_common.h
index fb644d2bf02..ec34c0fd5a7 100644
--- a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_common.h
+++ b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_common.h
@@ -35,7 +35,7 @@ constexpr double REMOVE_BATCH_BLOCK_RATE = 1.0 / 21.0;
 constexpr double REMOVE_BLOCK_RATE = 1.0 / 20.0;
 constexpr double RESOURCE_LIMIT_FACTOR = 1.0;
 constexpr uint32_t MAX_OUTSTANDING_MOVE_OPS = 10;
-const vespalib::string DOC_ID = "id:test:searchdocument::0";
+const vespalib::string DOC_ID = "id:test:searchdocument::";
 const BucketId BUCKET_ID_1(1);
 const BucketId BUCKET_ID_2(2);
 const Timestamp TIMESTAMP_1(1);
@@ -44,18 +44,17 @@ const GlobalId GID_1;
 using LidVector = std::vector<uint32_t>;
 using LidPair = std::pair<uint32_t, uint32_t>;
 using LidPairVector = std::vector<LidPair>;
+struct MyHandler;
 
 struct MyScanIterator : public IDocumentScanIterator {
+    const MyHandler & _handler;
     LidVector _lids;
     LidVector::const_iterator _itr;
     bool _validItr;
-    bool _bucketIdEqualLid;
-    explicit MyScanIterator(const LidVector &lids, bool bucketIdEqualLid);
+    explicit MyScanIterator(const MyHandler & handler, const LidVector &lids);
     ~MyScanIterator() override;
     bool valid() const override;
     search::DocumentMetaData next(uint32_t compactLidLimit, bool retry) override;
-
-    document::BucketId createBucketId(uint32_t lid) const;
 };
 
 struct MyHandler : public ILidSpaceCompactionHandler {
@@ -71,6 +70,7 @@ struct MyHandler : public ILidSpaceCompactionHandler {
     std::vector<IDestructorCallback::SP> _moveDoneContexts;
     documentmetastore::OperationListener::SP _op_listener;
     RemoveOperationsRateTracker* _rm_listener;
+    std::vector<std::pair<search::DocumentMetaData, std::shared_ptr<Document>>> _docs;
 
     explicit MyHandler(bool storeMoveDoneContexts, bool _bucketIdEqualLid);
     ~MyHandler() override;
@@ -87,6 +87,7 @@ struct MyHandler : public ILidSpaceCompactionHandler {
                                           uint32_t moveToLid) const override;
     void handleMove(const MoveOperation &, IDestructorCallback::SP moveDoneCtx) override;
     void handleCompactLidSpace(const CompactLidSpaceOperation &op, std::shared_ptr<IDestructorCallback>) override;
+    document::BucketId createBucketId(uint32_t lid) const;
 };
 
 struct MyStorer : public IOperationStorer {
diff --git a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.cpp b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.cpp
index 634662f7e2d..f5441f86835 100644
--- a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.cpp
@@ -7,6 +7,7 @@
 #include <vespa/searchcore/proton/feedoperation/moveoperation.h>
 #include <vespa/searchcorespi/index/i_thread_service.h>
 #include <vespa/persistence/spi/bucket_tasks.h>
+#include <vespa/document/fieldvalue/document.h>
 #include <vespa/vespalib/util/destructor_callbacks.h>
 #include <vespa/vespalib/util/lambdatask.h>
 #include <cassert>
@@ -46,14 +47,19 @@ CompactionJob::moveDocument(const search::DocumentMetaData & meta, std::shared_p
     // The real lid must be sampled in the master thread.
     //TODO remove target lid from createMoveOperation interface
     auto op = _handler.createMoveOperation(meta, 0);
-    if (!op) return;
+    if (!op || !op->getDocument()) return;
+    // Early detection and force md5 calculation outside of master thread
+    if (meta.gid != op->getDocument()->getId().getGlobalId()) return;
 
     _master.execute(makeLambdaTask([this, metaThen=meta, moveOp=std::move(op), onDone=std::move(context)]() {
         search::DocumentMetaData metaNow = _handler.getMetaData(metaThen.lid);
         if (metaNow.lid != metaThen.lid) return;
         if (metaNow.bucketId != metaThen.bucketId) return;
+        if (metaNow.gid != moveOp->getDocument()->getId().getGlobalId()) return;
 
-        moveOp->setTargetLid(_handler.getLidStatus().getLowestFreeLid());
+        uint32_t lowestLid = _handler.getLidStatus().getLowestFreeLid();
+        if (lowestLid >= metaNow.lid) return;
+        moveOp->setTargetLid(lowestLid);
         _opStorer.appendOperation(*moveOp, onDone);
         _handler.handleMove(*moveOp, std::move(onDone));
     }));