From afd12bb1a8235af33187cfe63944d8154575919a Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 25 Jan 2021 18:12:16 +0000 Subject: Check gid and bucketid inside the correct master thread to ensure you do the right thing. --- .../lid_space_compaction/lid_space_common.cpp | 37 +++++++++++++++------- .../lid_space_compaction/lid_space_common.h | 11 ++++--- .../server/lid_space_compaction_job_take2.cpp | 10 ++++-- 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(_lids[_iteratorCnt++], _bucketIdEqualLid); + return std::make_unique(*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(); + const auto & entry = _docs[document.lid]; + auto op = std::make_unique(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; using LidPair = std::pair; using LidPairVector = std::vector; +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 _moveDoneContexts; documentmetastore::OperationListener::SP _op_listener; RemoveOperationsRateTracker* _rm_listener; + std::vector>> _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) 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 #include #include +#include #include #include #include @@ -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)); })); -- cgit v1.2.3