From 69435eba0f94edff7e1f03e3efd0656d00b9f287 Mon Sep 17 00:00:00 2001 From: Geir Storli Date: Fri, 14 May 2021 21:38:55 +0200 Subject: Revert "Revert "Revert "Revert "Revert "Now that everything is controlled bucketwise by the bucket db we shou…""""" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../document_iterator/document_iterator_test.cpp | 56 ++++++++++++++++++++++ .../documentbucketmover/documentmover_test.cpp | 17 +++++++ .../lid_space_handler_test.cpp | 7 +++ .../commit_and_wait_document_retriever.cpp | 5 -- .../proton/server/documentbucketmover.cpp | 2 +- .../proton/server/lid_space_compaction_handler.cpp | 4 +- 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(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(), selectAll(), newestV(), -1, false); + TEST_DO(verifyStrongReadConsistency(itr)); +} + +TEST("require that readconsistency::strong does commit") { + DocumentIterator itr(bucket(5), std::make_shared(), 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(*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(document.bucketId, document.timestamp, std::move(doc), -- cgit v1.2.3