Revert "Revert "Revert "Revert "Revert "Revert "Revert "Revert "Revert "Now that everything is controlled bucketwise by the bucket db we shou…"""""""""

6 files changed, 84 insertions, 7 deletions

diff --git a/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp b/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp
index 93ad4cc8324..7ee811e66bf 100644
--- a/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp
+++ b/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp
@@ -488,6 +488,62 @@ TEST("require that iterator ignoring maxbytes stops at the end, and does not aut
     TEST_DO(verifyIterateIgnoringStopSignal(itr));
 }
 
+void verifyReadConsistency(DocumentIterator & itr, ILidCommitState & lidCommitState) {
+    IDocumentRetriever::SP retriever = doc("id:ns:document::1", Timestamp(2), bucket(5));
+    auto commitAndWaitRetriever = std::make_shared<CommitAndWaitDocumentRetriever>(retriever, lidCommitState);
+    itr.add(commitAndWaitRetriever);
+
+    IterateResult res = itr.iterate(largeNum);
+    EXPECT_TRUE(res.isCompleted());
+    EXPECT_EQUAL(1u, res.getEntries().size());
+    TEST_DO(checkEntry(res, 0, Document(*DataType::DOCUMENT, DocumentId("id:ns:document::1")), Timestamp(2)));
+}
+
+class ILidCommitStateProxy : public ILidCommitState {
+public:
+    explicit ILidCommitStateProxy(ILidCommitState & lidState)
+        : _waitCompleteCount(0),
+          _lidState(lidState)
+    {}
+private:
+    State waitState(State state, uint32_t lid) const override {
+        assert(state == State::COMPLETED);
+        _lidState.waitComplete(lid);
+        _waitCompleteCount++;
+        return state;
+    }
+
+    State waitState(State state, const LidList &lids) const override {
+        assert(state == State::COMPLETED);
+        _lidState.waitComplete(lids);
+        _waitCompleteCount++;
+        return state;
+    }
+
+public:
+    mutable size_t _waitCompleteCount;
+private:
+    ILidCommitState & _lidState;
+};
+
+void verifyStrongReadConsistency(DocumentIterator & itr) {
+    PendingLidTracker lidTracker;
+
+    ILidCommitStateProxy lidCommitState(lidTracker);
+    TEST_DO(verifyReadConsistency(itr, lidCommitState));
+    EXPECT_EQUAL(1u, lidCommitState._waitCompleteCount);
+}
+
+TEST("require that default readconsistency does commit") {
+    DocumentIterator itr(bucket(5), std::make_shared<document::AllFields>(), selectAll(), newestV(), -1, false);
+    TEST_DO(verifyStrongReadConsistency(itr));
+}
+
+TEST("require that readconsistency::strong does commit") {
+    DocumentIterator itr(bucket(5), std::make_shared<document::AllFields>(), selectAll(), newestV(), -1, false, storage::spi::ReadConsistency::STRONG);
+    TEST_DO(verifyStrongReadConsistency(itr));
+}
+
 TEST("require that docid limit is honoured") {
     IDocumentRetriever::SP retriever = doc("id:ns:document::1", Timestamp(2), bucket(5));
     auto & udr = dynamic_cast<UnitDR &>(*retriever);
diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentmover_test.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentmover_test.cpp
index 475d5064104..143d8f290c6 100644
--- a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentmover_test.cpp
+++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentmover_test.cpp
@@ -90,6 +90,23 @@ TEST_F(DocumentMoverTest, require_that_we_can_move_all_documents)
     }
 }
 
+TEST_F(DocumentMoverTest, require_that_move_is_stalled_if_document_is_pending_commit)
+{
+    setupForBucket(_source.bucket(1), 6, 9);
+    {
+        IPendingLidTracker::Token token = _pendingLidsForCommit.produce(1);
+        EXPECT_FALSE(moveDocuments(5));
+        EXPECT_FALSE(_mover.bucketDone());
+    }
+    EXPECT_TRUE(moveDocuments(5));
+    EXPECT_TRUE(_mover.bucketDone());
+    EXPECT_EQ(5u, _handler._moves.size());
+    EXPECT_EQ(5u, _limiter.beginOpCount);
+    for (size_t i = 0; i < 5u; ++i) {
+        assertEqual(_source.bucket(1), _source.docs(1)[0], 6, 9, _handler._moves[0]);
+    }
+}
+
 TEST_F(DocumentMoverTest, require_that_bucket_is_cached_when_IDocumentMoveHandler_handles_move_operation)
 {
     setupForBucket(_source.bucket(1), 6, 9);
diff --git a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_handler_test.cpp b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_handler_test.cpp
index 369d4f85c73..2ca1101ac3a 100644
--- a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_handler_test.cpp
+++ b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_handler_test.cpp
@@ -38,6 +38,13 @@ 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);
diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/commit_and_wait_document_retriever.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/commit_and_wait_document_retriever.cpp
index 6804d95adcb..aa20627600f 100644
--- a/searchcore/src/vespa/searchcore/proton/persistenceengine/commit_and_wait_document_retriever.cpp
+++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/commit_and_wait_document_retriever.cpp
@@ -31,7 +31,6 @@ CommitAndWaitDocumentRetriever::getDocumentMetaData(const document::DocumentId &
 document::Document::UP
 CommitAndWaitDocumentRetriever::getFullDocument(search::DocumentIdT lid) const {
     // Ensure that attribute vectors are committed
-    assert(_uncommittedLidsTracker.getState(lid) == ILidCommitState::State::COMPLETED);
     _uncommittedLidsTracker.waitComplete(lid);
     return _retriever->getFullDocument(lid);
 }
@@ -40,7 +39,6 @@ document::Document::UP
 CommitAndWaitDocumentRetriever::getPartialDocument(search::DocumentIdT lid, const document::DocumentId & docId,
                                                    const document::FieldSet & fieldSet) const
 {
-    assert(_uncommittedLidsTracker.getState(lid) == ILidCommitState::State::COMPLETED);
     _uncommittedLidsTracker.waitComplete(lid);
     return _retriever->getPartialDocument(lid, docId, fieldSet);
 }
@@ -49,9 +47,6 @@ void
 CommitAndWaitDocumentRetriever::visitDocuments(const LidVector &lids, search::IDocumentVisitor &visitor,
                                                ReadConsistency readConsistency) const
 {
-    for (auto lid : lids) {
-        assert(_uncommittedLidsTracker.getState(lid) == ILidCommitState::State::COMPLETED);
-    }
     _uncommittedLidsTracker.waitComplete(lids);
     _retriever->visitDocuments(lids, visitor, readConsistency);
 }
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp b/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp
index 94fbfbd4e20..a0ce668294f 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp
@@ -22,7 +22,7 @@ typedef IDocumentMetaStore::Iterator Iterator;
 
 MoveOperation::UP
 BucketMover::createMoveOperation(const MoveKey &key) {
-    assert ( ! _source->lidNeedsCommit(key._lid) );
+    if (_source->lidNeedsCommit(key._lid)) return {};
 
     const RawDocumentMetaData &metaNow = _source->meta_store()->getRawMetaData(key._lid);
     if (metaNow.getGid() != key._gid) return {};
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 c73bd56930a..1e1e15eee59 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
@@ -71,7 +71,9 @@ MoveOperation::UP
 LidSpaceCompactionHandler::createMoveOperation(const search::DocumentMetaData &document, uint32_t moveToLid) const
 {
     const uint32_t moveFromLid = document.lid;
-    assert ( ! _subDb.lidNeedsCommit(moveFromLid) );
+    if (_subDb.lidNeedsCommit(moveFromLid)) {
+        return MoveOperation::UP();
+    }
     auto doc = _subDb.retriever()->getFullDocument(moveFromLid);
     auto op = std::make_unique<MoveOperation>(document.bucketId, document.timestamp,
                                               std::move(doc),