Add teste for move when pending.

Implement backoff for lidspace when poending.

5 files changed, 43 insertions, 10 deletions

diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp
index 7df8641cc30..cad56a3a8c2 100644
--- a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp
+++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp
@@ -223,6 +223,7 @@ struct MoveFixture
     MySubDbTwoBuckets          _source;
     BucketDBOwner              _bucketDb;
     MyMoveHandler              _handler;
+    PendingLidTracker          _pendingLidsForCommit;
     MoveFixture()
         : _builder(),
           _bucketDB(std::make_shared<BucketDBOwner>()),
@@ -241,11 +242,11 @@ struct MoveFixture
                                                   _source._subDb.meta_store(),
                                                   _source._subDb.retriever(),
                                                   _source._subDb.feed_view(),
-                                                  nullptr);
+                                                  &_pendingLidsForCommit);
         _mover.setupForBucket(bucket, &_source._subDb, targetSubDbId, _handler, _bucketDb);
     }
-    void moveDocuments(size_t maxDocsToMove) {
-        _mover.moveDocuments(maxDocsToMove);
+    bool moveDocuments(size_t maxDocsToMove) {
+        return _mover.moveDocuments(maxDocsToMove);
     }
 };
 
@@ -275,7 +276,24 @@ assertEqual(const BucketId &bucket, const test::Document &doc,
 TEST_F("require that we can move all documents", MoveFixture)
 {
     f.setupForBucket(f._source.bucket(1), 6, 9);
-    f.moveDocuments(5);
+    EXPECT_TRUE(f.moveDocuments(5));
+    EXPECT_TRUE(f._mover.bucketDone());
+    EXPECT_EQUAL(5u, f._handler._moves.size());
+    EXPECT_EQUAL(5u, f._limiter.beginOpCount);
+    for (size_t i = 0; i < 5u; ++i) {
+        assertEqual(f._source.bucket(1), f._source.docs(1)[0], 6, 9, f._handler._moves[0]);
+    }
+}
+
+TEST_F("require that move is stalled if document is pending commit", MoveFixture)
+{
+    f.setupForBucket(f._source.bucket(1), 6, 9);
+    {
+        IPendingLidTracker::Token token = f._pendingLidsForCommit.produce(1);
+        EXPECT_FALSE(f.moveDocuments(5));
+        EXPECT_FALSE(f._mover.bucketDone());
+    }
+    EXPECT_TRUE(f.moveDocuments(5));
     EXPECT_TRUE(f._mover.bucketDone());
     EXPECT_EQUAL(5u, f._handler._moves.size());
     EXPECT_EQUAL(5u, f._limiter.beginOpCount);
@@ -287,7 +305,7 @@ TEST_F("require that we can move all documents", MoveFixture)
 TEST_F("require that bucket is cached when IDocumentMoveHandler handles move operation", MoveFixture)
 {
     f.setupForBucket(f._source.bucket(1), 6, 9);
-    f.moveDocuments(5);
+    EXPECT_TRUE(f.moveDocuments(5));
     EXPECT_TRUE(f._mover.bucketDone());
     EXPECT_EQUAL(5u, f._handler._moves.size());
     EXPECT_EQUAL(5u, f._handler._numCachedBuckets);
@@ -302,16 +320,16 @@ TEST_F("require that we can move documents in several steps", MoveFixture)
     EXPECT_EQUAL(2u, f._handler._moves.size());
     assertEqual(f._source.bucket(1), f._source.docs(1)[0], 6, 9, f._handler._moves[0]);
     assertEqual(f._source.bucket(1), f._source.docs(1)[1], 6, 9, f._handler._moves[1]);
-    f.moveDocuments(2);
+    EXPECT_TRUE(f.moveDocuments(2));
     EXPECT_FALSE(f._mover.bucketDone());
     EXPECT_EQUAL(4u, f._handler._moves.size());
     assertEqual(f._source.bucket(1), f._source.docs(1)[2], 6, 9, f._handler._moves[2]);
     assertEqual(f._source.bucket(1), f._source.docs(1)[3], 6, 9, f._handler._moves[3]);
-    f.moveDocuments(2);
+    EXPECT_TRUE(f.moveDocuments(2));
     EXPECT_TRUE(f._mover.bucketDone());
     EXPECT_EQUAL(5u, f._handler._moves.size());
     assertEqual(f._source.bucket(1), f._source.docs(1)[4], 6, 9, f._handler._moves[4]);
-    f.moveDocuments(2);
+    EXPECT_TRUE(f.moveDocuments(2));
     EXPECT_TRUE(f._mover.bucketDone());
     EXPECT_EQUAL(5u, f._handler._moves.size());
 }
diff --git a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp
index a0f4ccf4f3c..a9058a83af8 100644
--- a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp
+++ b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp
@@ -534,7 +534,16 @@ TEST_F(HandlerTest, createMoveOperation_works_as_expected)
     const BucketId bucketId(100);
     const Timestamp timestamp(200);
     DocumentMetaData document(moveFromLid, timestamp, bucketId, GlobalId());
+    {
+        EXPECT_FALSE(_subDb.maintenance_sub_db.lidNeedsCommit(moveFromLid));
+        IPendingLidTracker::Token token = _subDb._pendingLidsForCommit.produce(moveFromLid);
+        EXPECT_TRUE(_subDb.maintenance_sub_db.lidNeedsCommit(moveFromLid));
+        MoveOperation::UP op = _handler.createMoveOperation(document, moveToLid);
+        ASSERT_FALSE(op);
+    }
+    EXPECT_FALSE(_subDb.maintenance_sub_db.lidNeedsCommit(moveFromLid));
     MoveOperation::UP op = _handler.createMoveOperation(document, moveToLid);
+    ASSERT_TRUE(op);
     EXPECT_EQ(10u, _docStore._readLid);
     EXPECT_EQ(DbDocumentId(SUBDB_ID, moveFromLid).toString(),
               op->getPrevDbDocumentId().toString()); // source
diff --git a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_handler.cpp b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_handler.cpp
index d55e90b7446..b7158afd221 100644
--- a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_handler.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_handler.cpp
@@ -46,9 +46,12 @@ MoveOperation::UP
 LidSpaceCompactionHandler::createMoveOperation(const search::DocumentMetaData &document, uint32_t moveToLid) const
 {
     const uint32_t moveFromLid = document.lid;
+    if (_subDb.lidNeedsCommit(moveFromLid)) {
+        return MoveOperation::UP();
+    }
     auto doc = _subDb.retriever()->getFullDocument(moveFromLid);
     auto op = std::make_unique<MoveOperation>(document.bucketId, document.timestamp,
-                                              Document::SP(doc.release()),
+                                              std::move(doc),
                                               DbDocumentId(_subDb.sub_db_id(), moveFromLid),
                                               _subDb.sub_db_id());
     op->setTargetLid(moveToLid);
diff --git a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp
index a056e7b4d15..46d4a3a5974 100644
--- a/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/lid_space_compaction_job.cpp
@@ -56,6 +56,9 @@ LidSpaceCompactionJob::scanDocuments(const LidUsageStats &stats)
                 return true;
             } else {
                 MoveOperation::UP op = _handler.createMoveOperation(document, stats.getLowestFreeLid());
+                if ( ! op ) {
+                    return false;
+                }
                 search::IDestructorCallback::SP context = _moveOpsLimiter->beginOperation();
                 _opStorer.appendOperation(*op, context);
                 _handler.handleMove(*op, std::move(context));
diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenancedocumentsubdb.cpp b/searchcore/src/vespa/searchcore/proton/server/maintenancedocumentsubdb.cpp
index b866319a09f..f91ed2bf608 100644
--- a/searchcore/src/vespa/searchcore/proton/server/maintenancedocumentsubdb.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/maintenancedocumentsubdb.cpp
@@ -44,7 +44,7 @@ MaintenanceDocumentSubDB::clear()
 bool
 MaintenanceDocumentSubDB::lidNeedsCommit(search::DocumentIdT lid) const {
     return ((_pendingLidsForCommit != nullptr) &&
-            (_pendingLidsForCommit->getState(lid) == ILidCommitState::State::NEED_COMMIT));
+            (_pendingLidsForCommit->getState(lid) != ILidCommitState::State::COMPLETED));
 }
 
 }