Use current metadata when creating move operation.

2 files changed, 36 insertions, 15 deletions

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 bae4833d657..526aedd7b28 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
@@ -33,6 +33,13 @@ private:
     std::atomic<size_t> & _count;
 };
 
+bool
+isSameDocument(const search::DocumentMetaData & a, const search::DocumentMetaData & b) {
+    return (a.lid == b.lid) &&
+           (a.bucketId == b.bucketId) &&
+           (a.gid == b.gid);
+}
+
 }
 
 void
@@ -67,31 +74,42 @@ CompactionJob::scanDocuments(const LidUsageStats &stats)
 }
 
 void
-CompactionJob::moveDocument(const search::DocumentMetaData & meta, std::shared_ptr<IDestructorCallback> context) {
+CompactionJob::moveDocument(const search::DocumentMetaData & metaThen, std::shared_ptr<IDestructorCallback> context) {
     IncOnDestruct countGuard(_executedCount);
     if (_stopped.load(std::memory_order_relaxed)) return;
     // The real lid must be sampled in the master thread.
     //TODO remove target lid from createMoveOperation interface
-    auto op = _handler->createMoveOperation(meta, 0);
+    // Reread meta data as document might have been altered after move was initiated
+    // If so it will fail the timestamp sanity check later on.
+    search::DocumentMetaData metaNow = _handler->getMetaData(metaThen.lid);
+    if ( ! isSameDocument(metaThen, metaNow)) return;
+    auto op = _handler->createMoveOperation(metaNow, 0);
     if (!op || !op->getDocument()) return;
     // Early detection and force md5 calculation outside of master thread
-    if (meta.gid != op->getDocument()->getId().getGlobalId()) return;
+    if (metaThen.gid != op->getDocument()->getId().getGlobalId()) return;
 
-    _master.execute(makeLambdaTask([this, metaThen=meta, moveOp=std::move(op), onDone=std::move(context)]() {
+    _master.execute(makeLambdaTask([this, metaNow, moveOp=std::move(op), onDone=std::move(context)]() mutable {
         if (_stopped.load(std::memory_order_relaxed)) return;
-        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;
-
-        uint32_t lowestLid = _handler->getLidStatus().getLowestFreeLid();
-        if (lowestLid >= metaNow.lid) return;
-        moveOp->setTargetLid(lowestLid);
-        _opStorer.appendOperation(*moveOp, onDone);
-        _handler->handleMove(*moveOp, std::move(onDone));
+        completeMove(metaNow, std::move(moveOp), std::move(onDone));
     }));
 }
 
+void
+CompactionJob::completeMove(const search::DocumentMetaData & metaThen, std::unique_ptr<MoveOperation> moveOp,
+                            std::shared_ptr<IDestructorCallback> onDone)
+{
+    search::DocumentMetaData metaNow = _handler->getMetaData(metaThen.lid);
+    // This should be impossible and should probably be an assert
+    if ( ! isSameDocument(metaThen, metaNow)) return;
+    if (metaNow.gid != moveOp->getDocument()->getId().getGlobalId()) return;
+
+    uint32_t lowestLid = _handler->getLidStatus().getLowestFreeLid();
+    if (lowestLid >= metaNow.lid) return;
+    moveOp->setTargetLid(lowestLid);
+    _opStorer.appendOperation(*moveOp, onDone);
+    _handler->handleMove(*moveOp, std::move(onDone));
+}
+
 CompactionJob::CompactionJob(const DocumentDBLidSpaceCompactionConfig &config,
                              std::shared_ptr<ILidSpaceCompactionHandler> handler,
                              IOperationStorer &opStorer,
diff --git a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.h b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.h
index af9fe50c545..b73f971f8ee 100644
--- a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.h
+++ b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job_take2.h
@@ -12,6 +12,7 @@ namespace vespalib { class IDestructorCallback; }
 namespace proton {
     class IDiskMemUsageNotifier;
     class IClusterStateChangedNotifier;
+    class MoveOperation;
 }
 
 namespace proton::lidspace {
@@ -34,7 +35,9 @@ private:
     std::atomic<size_t>      _executedCount;
 
     bool scanDocuments(const search::LidUsageStats &stats) override;
-    void moveDocument(const search::DocumentMetaData & meta, std::shared_ptr<IDestructorCallback> onDone);
+    void moveDocument(const search::DocumentMetaData & metaThen, std::shared_ptr<IDestructorCallback> onDone);
+    void completeMove(const search::DocumentMetaData & metaThen, std::unique_ptr<MoveOperation> moveOp,
+                      std::shared_ptr<IDestructorCallback> onDone);
     void onStop() override;
     bool inSync() const override;
     void failOperation();