diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2022-03-09 12:13:43 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2022-03-09 21:35:47 +0000 |
commit | 9e6251c1e2246da4f490ae1013fc3972504c42ab (patch) | |
tree | 201d9a0fc85e8ec1fbba9833fea58a8f81b041b7 | |
parent | 1fa852eadd7b09b98ce6dd8bdd7b0446d07a37d6 (diff) |
Move BucketIdListResult
29 files changed, 136 insertions, 115 deletions
diff --git a/document/src/vespa/document/bucket/bucketid.cpp b/document/src/vespa/document/bucket/bucketid.cpp index f78fdbe89dc..f01bfb67674 100644 --- a/document/src/vespa/document/bucket/bucketid.cpp +++ b/document/src/vespa/document/bucket/bucketid.cpp @@ -151,5 +151,4 @@ operator>>(nbostream &is, BucketId &bucketId) } // document -template class vespalib::Array<document::BucketId>; VESPALIB_HASH_SET_INSTANTIATE_H(document::BucketId, document::BucketId::hash); diff --git a/document/src/vespa/document/bucket/bucketidlist.cpp b/document/src/vespa/document/bucket/bucketidlist.cpp index 26f2062e1c4..0f5d18f7b3a 100644 --- a/document/src/vespa/document/bucket/bucketidlist.cpp +++ b/document/src/vespa/document/bucket/bucketidlist.cpp @@ -4,9 +4,8 @@ namespace document::bucket { -BucketIdList::BucketIdList() { } BucketIdList::BucketIdList(const BucketIdList & rhs) = default; BucketIdList & BucketIdList::operator = (const BucketIdList &) = default; -BucketIdList::~BucketIdList() { } +BucketIdList::~BucketIdList() = default; } diff --git a/document/src/vespa/document/bucket/bucketidlist.h b/document/src/vespa/document/bucket/bucketidlist.h index c21879ff375..b6fdb51740a 100644 --- a/document/src/vespa/document/bucket/bucketidlist.h +++ b/document/src/vespa/document/bucket/bucketidlist.h @@ -3,17 +3,18 @@ #pragma once #include "bucketid.h" -#include <vespa/vespalib/util/array.h> +#include <vespa/vespalib/stllike/allocator.h> +#include <vector> namespace document::bucket { -using BucketIdListT = vespalib::Array<BucketId>; +using BucketIdListT = std::vector<BucketId, vespalib::allocator_large<BucketId>>; class BucketIdList : public BucketIdListT { public: - BucketIdList(); - BucketIdList(BucketIdList && rhs) = default; - BucketIdList & operator = (BucketIdList &&) = default; + using BucketIdListT::BucketIdListT; + BucketIdList(BucketIdList && rhs) noexcept = default; + BucketIdList & operator = (BucketIdList &&) noexcept = default; BucketIdList(const BucketIdList & rhs); BucketIdList & operator = (const BucketIdList &); ~BucketIdList(); diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp index d947ca51f49..33770004aba 100644 --- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp +++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp @@ -318,14 +318,14 @@ DummyPersistence::listBuckets(BucketSpace bucketSpace) const list.push_back(entry.first); } } - return BucketIdListResult(list); + return BucketIdListResult(std::move(list)); } void -DummyPersistence::setModifiedBuckets(const BucketIdListResult::List& buckets) +DummyPersistence::setModifiedBuckets(BucketIdListResult::List buckets) { std::lock_guard lock(_monitor); - _modifiedBuckets = buckets; + _modifiedBuckets = std::move(buckets); } void DummyPersistence::set_fake_bucket_set(const std::vector<std::pair<Bucket, BucketInfo>>& fake_info) { @@ -348,10 +348,10 @@ DummyPersistence::getModifiedBuckets(BucketSpace bucketSpace) const { std::lock_guard lock(_monitor); if (bucketSpace == FixedBucketSpaces::default_space()) { - return BucketIdListResult(_modifiedBuckets); + return BucketIdListResult(std::move(_modifiedBuckets)); } else { BucketIdListResult::List emptyList; - return BucketIdListResult(emptyList); + return BucketIdListResult(BucketIdListResult::List()); } } diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h index 3b6fc9f1449..683a738255f 100644 --- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h +++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h @@ -138,12 +138,12 @@ class DummyPersistence : public AbstractPersistenceProvider { public: DummyPersistence(const std::shared_ptr<const document::DocumentTypeRepo>& repo); - ~DummyPersistence(); + ~DummyPersistence() override; Result initialize() override; BucketIdListResult listBuckets(BucketSpace bucketSpace) const override; - void setModifiedBuckets(const BucketIdListResult::List& result); + void setModifiedBuckets(BucketIdListResult::List result); // Important: any subsequent mutations to the bucket set in fake_info will reset // the bucket info due to implicit recalculation of bucket info. diff --git a/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.cpp b/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.cpp index 35654240ec7..6ee99e49798 100644 --- a/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.cpp +++ b/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.cpp @@ -18,8 +18,7 @@ AbstractPersistenceProvider::removeIfFoundAsync(const Bucket& b, Timestamp times BucketIdListResult AbstractPersistenceProvider::getModifiedBuckets(BucketSpace) const { - BucketIdListResult::List list; - return BucketIdListResult(list); + return BucketIdListResult(BucketIdListResult::List()); } } diff --git a/persistence/src/vespa/persistence/spi/result.h b/persistence/src/vespa/persistence/spi/result.h index 10c589307ba..5046a380c86 100644 --- a/persistence/src/vespa/persistence/spi/result.h +++ b/persistence/src/vespa/persistence/spi/result.h @@ -71,7 +71,7 @@ std::ostream & operator << (std::ostream & os, const Result & r); std::ostream & operator << (std::ostream & os, const Result::ErrorType &errorCode); -class BucketInfoResult : public Result { +class BucketInfoResult final : public Result { public: /** * Constructor to use for a result where an error has been detected. @@ -95,7 +95,7 @@ private: BucketInfo _info; }; -class UpdateResult : public Result +class UpdateResult final : public Result { public: /** @@ -151,7 +151,7 @@ private: uint32_t _numRemoved; }; -class GetResult : public Result { +class GetResult final : public Result { public: /** * Constructor to use when there was an error retrieving the document. @@ -173,6 +173,8 @@ public: _is_tombstone(false) { } + GetResult(GetResult &&) noexcept = default; + GetResult & operator=(GetResult &&) noexcept = default; /** * Constructor to use when we found the document asked for. @@ -225,7 +227,7 @@ private: bool _is_tombstone; }; -class BucketIdListResult : public Result { +class BucketIdListResult final : public Result { public: using List = document::bucket::BucketIdList; @@ -241,12 +243,16 @@ public: * @param list The list of bucket ids this partition has. Is swapped with * the list internal to this object. */ - BucketIdListResult(List& list) - : Result() - { - _info.swap(list); - } - + BucketIdListResult(List list) + : Result(), + _info(std::move(list)) + { } + BucketIdListResult() + : Result(), + _info() + { } + BucketIdListResult(BucketIdListResult &&) noexcept = default; + BucketIdListResult & operator =(BucketIdListResult &&) noexcept = default; ~BucketIdListResult(); const List& getList() const { return _info; } @@ -278,7 +284,7 @@ private: IteratorId _iterator; }; -class IterateResult : public Result { +class IterateResult final : public Result { public: using List = std::vector<std::unique_ptr<DocEntry>>; diff --git a/searchcore/src/tests/proton/persistenceengine/persistence_handler_map/persistence_handler_map_test.cpp b/searchcore/src/tests/proton/persistenceengine/persistence_handler_map/persistence_handler_map_test.cpp index e8cc1b54235..ff65de90c4a 100644 --- a/searchcore/src/tests/proton/persistenceengine/persistence_handler_map/persistence_handler_map_test.cpp +++ b/searchcore/src/tests/proton/persistenceengine/persistence_handler_map/persistence_handler_map_test.cpp @@ -30,7 +30,7 @@ struct DummyPersistenceHandler : public IPersistenceHandler { RetrieversSP getDocumentRetrievers(storage::spi::ReadConsistency) override { return RetrieversSP(); } void handleListActiveBuckets(IBucketIdListResultHandler &) override {} - void handlePopulateActiveBuckets(document::BucketId::List &, IGenericResultHandler &) override {} + void handlePopulateActiveBuckets(document::BucketId::List, IGenericResultHandler &) override {} }; BucketSpace space_1(1); diff --git a/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp b/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp index ce0b09a48b8..6e7e50ee4f9 100644 --- a/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp +++ b/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp @@ -217,7 +217,7 @@ struct MyHandler : public IPersistenceHandler, IBucketFreezer { } void handleListBuckets(IBucketIdListResultHandler &resultHandler) override { - resultHandler.handle(BucketIdListResult(bucketList)); + resultHandler.handle(BucketIdListResult(BucketId::List(bucketList.begin(), bucketList.end()))); } void handleSetClusterState(const ClusterState &calc, IGenericResultHandler &resultHandler) override { @@ -245,7 +245,7 @@ struct MyHandler : public IPersistenceHandler, IBucketFreezer { } void handleGetModifiedBuckets(IBucketIdListResultHandler &resultHandler) override { - resultHandler.handle(BucketIdListResult(modBucketList)); + resultHandler.handle(BucketIdListResult(std::move(modBucketList))); } void handleSplit(FeedToken token, const storage::spi::Bucket &, const storage::spi::Bucket &, @@ -262,17 +262,16 @@ struct MyHandler : public IPersistenceHandler, IBucketFreezer { RetrieversSP getDocumentRetrievers(storage::spi::ReadConsistency) override { RetrieversSP ret(new std::vector<IDocumentRetriever::SP>); - ret->push_back(IDocumentRetriever::SP(new MyDocumentRetriever(nullptr, Timestamp(), lastDocId))); - ret->push_back(IDocumentRetriever::SP(new MyDocumentRetriever(document, existingTimestamp, lastDocId))); + ret->push_back(std::make_unique<MyDocumentRetriever>(nullptr, Timestamp(), lastDocId)); + ret->push_back(std::make_unique<MyDocumentRetriever>(document, existingTimestamp, lastDocId)); return ret; } void handleListActiveBuckets(IBucketIdListResultHandler &resultHandler) override { - BucketIdListResult::List list; - resultHandler.handle(BucketIdListResult(list)); + resultHandler.handle(BucketIdListResult()); } - void handlePopulateActiveBuckets(document::BucketId::List &buckets, IGenericResultHandler &resultHandler) override { + void handlePopulateActiveBuckets(document::BucketId::List buckets, IGenericResultHandler &resultHandler) override { (void) buckets; resultHandler.handle(Result()); } diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp index c7a3103af22..37d04f6bcb9 100644 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp @@ -225,25 +225,26 @@ BucketDB::deleteEmptyBucket(const BucketId &bucketId) } } -void -BucketDB::getActiveBuckets(BucketId::List &buckets) const +document::BucketId::List +BucketDB::getActiveBuckets() const { + BucketId::List buckets; for (const auto & entry : _map) { if (entry.second.isActive()) { buckets.push_back(entry.first); } } + return buckets; } -void -BucketDB::populateActiveBuckets(const BucketId::List &buckets, BucketId::List &fixupBuckets) +document::BucketId::List +BucketDB::populateActiveBuckets(BucketId::List buckets) { - typedef BucketId::List BIV; - BIV sorted(buckets); - BIV toAdd; - std::sort(sorted.begin(), sorted.end()); - auto si = sorted.begin(); - auto se = sorted.end(); + BucketId::List toAdd; + BucketId::List fixupBuckets; + std::sort(buckets.begin(), buckets.end()); + auto si = buckets.begin(); + auto se = buckets.end(); for (const auto & entry : _map) { for (; si != se && !(entry.first < *si); ++si) { if (*si < entry.first) { @@ -263,6 +264,7 @@ BucketDB::populateActiveBuckets(const BucketId::List &buckets, BucketId::List &f InsertResult ins(_map.emplace(bucketId, activeState)); assert(ins.second); } + return fixupBuckets; } } diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h index 2ea7594bde1..6fe46d97f46 100644 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h +++ b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h @@ -63,8 +63,8 @@ public: void setBucketState(const BucketId &bucketId, bool active); void createBucket(const BucketId &bucketId); void deleteEmptyBucket(const BucketId &bucketId); - void getActiveBuckets(BucketId::List &buckets) const; - void populateActiveBuckets(const BucketId::List &buckets, BucketId::List &fixupBuckets); + BucketId::List getActiveBuckets() const; + BucketId::List populateActiveBuckets(BucketId::List buckets); ConstMapIterator begin() const { return _map.begin(); } ConstMapIterator end() const { return _map.end(); } diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp index 28234730f7b..2e6a7087b69 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp @@ -982,12 +982,9 @@ DocumentMetaStore::updateActiveLids(const BucketId &bucketId, bool active) } void -DocumentMetaStore::populateActiveBuckets(const BucketId::List &buckets) +DocumentMetaStore::populateActiveBuckets(BucketId::List buckets) { - typedef BucketId::List BIV; - BIV fixupBuckets; - - _bucketDB->takeGuard()->populateActiveBuckets(buckets, fixupBuckets); + BucketId::List fixupBuckets = _bucketDB->takeGuard()->populateActiveBuckets(std::move(buckets)); for (const auto &bucketId : fixupBuckets) { updateActiveLids(bucketId, true); diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h index 9e4977c65e1..5cb0b1469fd 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h @@ -233,7 +233,7 @@ public: bucketdb::BucketDeltaPair handleSplit(const bucketdb::SplitBucketSession &session) override; bucketdb::BucketDeltaPair handleJoin(const bucketdb::JoinBucketsSession &session) override; void setBucketState(const BucketId &bucketId, bool active) override; - void populateActiveBuckets(const BucketId::List &buckets) override; + void populateActiveBuckets(BucketId::List buckets) override; ConstIterator beginFrozen() const; const vespalib::GenerationHandler & getGenerationHandler() const { diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/i_bucket_handler.h b/searchcore/src/vespa/searchcore/proton/documentmetastore/i_bucket_handler.h index bb8c860b56e..1a0288fea84 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/i_bucket_handler.h +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/i_bucket_handler.h @@ -52,7 +52,7 @@ struct IBucketHandler * Sets the bucket state to active, used when adding document db as * part of live reconfig. **/ - virtual void populateActiveBuckets(const BucketId::List &buckets) = 0; + virtual void populateActiveBuckets(BucketId::List buckets) = 0; }; diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/ipersistencehandler.h b/searchcore/src/vespa/searchcore/proton/persistenceengine/ipersistencehandler.h index b393a85f632..1edecc59136 100644 --- a/searchcore/src/vespa/searchcore/proton/persistenceengine/ipersistencehandler.h +++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/ipersistencehandler.h @@ -70,7 +70,7 @@ public: virtual void handleListActiveBuckets(IBucketIdListResultHandler &resultHandler) = 0; - virtual void handlePopulateActiveBuckets(document::BucketId::List &buckets, IGenericResultHandler &resultHandler) = 0; + virtual void handlePopulateActiveBuckets(document::BucketId::List buckets, IGenericResultHandler &resultHandler) = 0; }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp index 7e21e619419..94882bfadf5 100644 --- a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp +++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp @@ -107,19 +107,21 @@ public: : _bucketSet() { } ~BucketIdListResultHandler() override; - void handle(const BucketIdListResult &result) override { + void + handle(BucketIdListResult result) override { const BucketIdListResult::List &buckets = result.getList(); for (size_t i = 0; i < buckets.size(); ++i) { _bucketSet.insert(buckets[i]); } } - std::unique_ptr<BucketIdListResult> getResult() const { + std::unique_ptr<BucketIdListResult> + getResult() const { BucketIdListResult::List buckets; buckets.reserve(_bucketSet.size()); for (document::BucketId bucketId : _bucketSet) { buckets.push_back(bucketId); } - return std::make_unique<BucketIdListResult>(buckets); + return std::make_unique<BucketIdListResult>(std::move(buckets)); } }; @@ -139,10 +141,10 @@ public: } } ~SynchronizedBucketIdListResultHandler() override; - void handle(const BucketIdListResult &result) override { + void handle(BucketIdListResult result) override { { std::lock_guard<std::mutex> guard(_lock); - BucketIdListResultHandler::handle(result); + BucketIdListResultHandler::handle(std::move(result)); } countDown(); } @@ -283,7 +285,7 @@ PersistenceEngine::listBuckets(BucketSpace bucketSpace) const IPersistenceHandler *handler = snap.handlers().get(); handler->handleListBuckets(resultHandler); } - return *resultHandler.getResult(); + return std::move(*resultHandler.getResult()); } @@ -641,10 +643,10 @@ PersistenceEngine::getModifiedBuckets(BucketSpace bucketSpace) const IPersistenceHandler *handler = snap.handlers().get(); handler->handleGetModifiedBuckets(resultHandler); } - for (const auto & item : extraModifiedBuckets) { - resultHandler.handle(*item); + for (auto & item : extraModifiedBuckets) { + resultHandler.handle(std::move(*item)); } - return dynamic_cast<BucketIdListResult &>(*futureResult.get()); + return std::move(dynamic_cast<BucketIdListResult &>(*futureResult.get())); } @@ -763,7 +765,7 @@ private: public: ActiveBucketIdListResultHandler() : _bucketMap() { } - void handle(const BucketIdListResult &result) override { + void handle(BucketIdListResult result) override { const BucketIdListResult::List &buckets = result.getList(); for (size_t i = 0; i < buckets.size(); ++i) { IR ir(_bucketMap.insert(std::make_pair(buckets[i], 1u))); @@ -805,7 +807,7 @@ PersistenceEngine::populateInitialBucketDB(const WriteGuard & guard, BucketSpace auto catchResult = std::make_unique<storage::spi::CatchResult>(); auto futureResult = catchResult->future_result(); GenericResultHandler trHandler(1, std::move(catchResult)); - targetHandler.handlePopulateActiveBuckets(buckets, trHandler); + targetHandler.handlePopulateActiveBuckets(std::move(buckets), trHandler); futureResult.get(); } diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/resulthandler.h b/searchcore/src/vespa/searchcore/proton/persistenceengine/resulthandler.h index 27b1fc8bdd0..897ff75c7b3 100644 --- a/searchcore/src/vespa/searchcore/proton/persistenceengine/resulthandler.h +++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/resulthandler.h @@ -9,12 +9,12 @@ template <typename ResultType> class IResultHandler { public: virtual ~IResultHandler() = default; - virtual void handle(const ResultType &result) = 0; + virtual void handle(ResultType result) = 0; }; using IBucketIdListResultHandler = IResultHandler<storage::spi::BucketIdListResult>; -using IBucketInfoResultHandler = IResultHandler<storage::spi::BucketInfoResult>; -using IGenericResultHandler = IResultHandler<storage::spi::Result>; +using IBucketInfoResultHandler = IResultHandler<const storage::spi::BucketInfoResult &>; +using IGenericResultHandler = IResultHandler<const storage::spi::Result &>; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/buckethandler.cpp b/searchcore/src/vespa/searchcore/proton/server/buckethandler.cpp index 0d9b2f6aaf6..d9201ce5b9e 100644 --- a/searchcore/src/vespa/searchcore/proton/server/buckethandler.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/buckethandler.cpp @@ -46,15 +46,14 @@ void BucketHandler::performPopulateActiveBuckets(document::BucketId::List buckets, IGenericResultHandler *resultHandler) { - _ready->populateActiveBuckets(buckets); + _ready->populateActiveBuckets(std::move(buckets)); resultHandler->handle(Result()); } void BucketHandler::deactivateAllActiveBuckets() { - BucketId::List buckets; - _ready->getBucketDB().takeGuard()->getActiveBuckets(buckets); + BucketId::List buckets = _ready->getBucketDB().takeGuard()->getActiveBuckets(); for (auto bucketId : buckets) { _ready->setBucketState(bucketId, storage::spi::BucketInfo::NOT_ACTIVE); // Don't notify bucket state changed, node is marked down so @@ -93,7 +92,7 @@ BucketHandler::handleListBuckets(IBucketIdListResultHandler &resultHandler) // master write thread in document database. BucketIdListResult::List buckets; _ready->getBucketDB().takeGuard()->getBuckets(buckets); - resultHandler.handle(BucketIdListResult(buckets)); + resultHandler.handle(BucketIdListResult(std::move(buckets))); } void @@ -130,17 +129,16 @@ BucketHandler::handleListActiveBuckets(IBucketIdListResultHandler &resultHandler // Called by SPI thread. // BucketDBOwner ensures synchronization between SPI thread and // master write thread in document database. - BucketIdListResult::List buckets; - _ready->getBucketDB().takeGuard()->getActiveBuckets(buckets); - resultHandler.handle(BucketIdListResult(buckets)); + + resultHandler.handle(BucketIdListResult(_ready->getBucketDB().takeGuard()->getActiveBuckets())); } void -BucketHandler::handlePopulateActiveBuckets(document::BucketId::List &buckets, +BucketHandler::handlePopulateActiveBuckets(document::BucketId::List buckets, IGenericResultHandler &resultHandler) { - _executor.execute(makeLambdaTask([this, buckets, &resultHandler]() { - performPopulateActiveBuckets(std::move(buckets), &resultHandler); + _executor.execute(makeLambdaTask([this, moved_buckets=std::move(buckets), &resultHandler]() mutable { + performPopulateActiveBuckets(std::move(moved_buckets), &resultHandler); })); } diff --git a/searchcore/src/vespa/searchcore/proton/server/buckethandler.h b/searchcore/src/vespa/searchcore/proton/server/buckethandler.h index 2344e080450..79c6e929a51 100644 --- a/searchcore/src/vespa/searchcore/proton/server/buckethandler.h +++ b/searchcore/src/vespa/searchcore/proton/server/buckethandler.h @@ -60,7 +60,7 @@ public: void handleGetBucketInfo(const storage::spi::Bucket &bucket, IBucketInfoResultHandler &resultHandler); void handleListActiveBuckets(IBucketIdListResultHandler &resultHandler); - void handlePopulateActiveBuckets(document::BucketId::List &buckets, + void handlePopulateActiveBuckets(document::BucketId::List buckets, IGenericResultHandler &resultHandler); bool hasBucket(const storage::spi::Bucket &bucket); diff --git a/searchcore/src/vespa/searchcore/proton/server/clusterstatehandler.cpp b/searchcore/src/vespa/searchcore/proton/server/clusterstatehandler.cpp index 20cd4ced6b2..e7142fb5bde 100644 --- a/searchcore/src/vespa/searchcore/proton/server/clusterstatehandler.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/clusterstatehandler.cpp @@ -75,10 +75,7 @@ ClusterStateHandler::performSetClusterState(const ClusterState *calc, IGenericRe void ClusterStateHandler::performGetModifiedBuckets(IBucketIdListResultHandler *resultHandler) { - storage::spi::BucketIdListResult::List modifiedBuckets; - modifiedBuckets.resize(_modifiedBuckets.size()); - std::copy(_modifiedBuckets.begin(), _modifiedBuckets.end(), - modifiedBuckets.begin()); + storage::spi::BucketIdListResult::List modifiedBuckets(_modifiedBuckets.begin(), _modifiedBuckets.end()); if (LOG_WOULD_LOG(debug) && !modifiedBuckets.empty()) { std::ostringstream oss; @@ -91,7 +88,7 @@ ClusterStateHandler::performGetModifiedBuckets(IBucketIdListResultHandler *resul LOG(debug, "performGetModifiedBuckets(): modifiedBuckets(%zu): %s", modifiedBuckets.size(), oss.str().c_str()); } - resultHandler->handle(BucketIdListResult(modifiedBuckets)); + resultHandler->handle(BucketIdListResult(std::move(modifiedBuckets))); _modifiedBuckets.clear(); } diff --git a/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.cpp b/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.cpp index bec9197501b..021460e6ecc 100644 --- a/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.cpp @@ -133,9 +133,9 @@ PersistenceHandlerProxy::handleListActiveBuckets(IBucketIdListResultHandler &res } void -PersistenceHandlerProxy::handlePopulateActiveBuckets(document::BucketId::List &buckets, IGenericResultHandler &resultHandler) +PersistenceHandlerProxy::handlePopulateActiveBuckets(document::BucketId::List buckets, IGenericResultHandler &resultHandler) { - _bucketHandler.handlePopulateActiveBuckets(buckets, resultHandler); + _bucketHandler.handlePopulateActiveBuckets(std::move(buckets), resultHandler); } } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.h b/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.h index 5c35434aae8..c438a777f1b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.h +++ b/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.h @@ -57,7 +57,7 @@ public: void handleListActiveBuckets(IBucketIdListResultHandler &resultHandler) override; - void handlePopulateActiveBuckets(document::BucketId::List &buckets, IGenericResultHandler &resultHandler) override; + void handlePopulateActiveBuckets(document::BucketId::List buckets, IGenericResultHandler &resultHandler) override; }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/test/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/test/CMakeLists.txt index abc8c8450e4..3a2e443bfef 100644 --- a/searchcore/src/vespa/searchcore/proton/test/CMakeLists.txt +++ b/searchcore/src/vespa/searchcore/proton/test/CMakeLists.txt @@ -11,6 +11,7 @@ vespa_add_library(searchcore_test STATIC mock_index_manager.cpp mock_shared_threading_service.cpp userdocumentsbuilder.cpp + resulthandler.cpp threading_service_observer.cpp transport_helper.cpp DEPENDS diff --git a/searchcore/src/vespa/searchcore/proton/test/bucketstatecalculator.h b/searchcore/src/vespa/searchcore/proton/test/bucketstatecalculator.h index d8803fd9c27..c773ae32b3f 100644 --- a/searchcore/src/vespa/searchcore/proton/test/bucketstatecalculator.h +++ b/searchcore/src/vespa/searchcore/proton/test/bucketstatecalculator.h @@ -5,6 +5,7 @@ #include <vespa/document/bucket/bucketidlist.h> #include <vespa/document/bucket/bucket.h> #include <set> +#include <memory> namespace proton::test { @@ -23,8 +24,8 @@ private: bool _nodeMaintenance; public: - typedef std::shared_ptr<BucketStateCalculator> SP; - BucketStateCalculator() : + using SP = std::shared_ptr<BucketStateCalculator>; + BucketStateCalculator() noexcept : _ready(), _asked(), _clusterUp(true), @@ -33,6 +34,8 @@ public: _nodeMaintenance(false) { } + BucketStateCalculator(BucketStateCalculator &&) noexcept = default; + BucketStateCalculator & operator =(BucketStateCalculator &&) noexcept = default; ~BucketStateCalculator() override; BucketStateCalculator &addReady(const document::BucketId &bucket) { _ready.insert(bucket); diff --git a/searchcore/src/vespa/searchcore/proton/test/document_meta_store_observer.h b/searchcore/src/vespa/searchcore/proton/test/document_meta_store_observer.h index 0386cb141ee..b9dacf61af8 100644 --- a/searchcore/src/vespa/searchcore/proton/test/document_meta_store_observer.h +++ b/searchcore/src/vespa/searchcore/proton/test/document_meta_store_observer.h @@ -115,8 +115,8 @@ struct DocumentMetaStoreObserver : public IDocumentMetaStore void setBucketState(const BucketId &bucketId, bool active) override { _store.setBucketState(bucketId, active); } - void populateActiveBuckets(const document::BucketId::List &buckets) override { - _store.populateActiveBuckets(buckets); + void populateActiveBuckets(document::BucketId::List buckets) override { + _store.populateActiveBuckets(std::move(buckets)); } diff --git a/searchcore/src/vespa/searchcore/proton/test/resulthandler.cpp b/searchcore/src/vespa/searchcore/proton/test/resulthandler.cpp new file mode 100644 index 00000000000..00102aeab25 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/test/resulthandler.cpp @@ -0,0 +1,27 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "resulthandler.h" + +namespace proton::test { + +GenericResultHandler::~GenericResultHandler() = default; + +void +GenericResultHandler::handle(const storage::spi::Result &result) { + _result = std::make_unique<storage::spi::Result>(result); +} + +BucketInfoResultHandler::~BucketInfoResultHandler() = default; +void +BucketInfoResultHandler::handle(const storage::spi::BucketInfoResult &result) { + _result = std::make_unique<storage::spi::BucketInfoResult>(result); +} + +BucketIdListResultHandler::~BucketIdListResultHandler() = default; + +void +BucketIdListResultHandler::handle(storage::spi::BucketIdListResult result) { + _result = std::make_unique<storage::spi::BucketIdListResult>(std::move(result)); +} + +} diff --git a/searchcore/src/vespa/searchcore/proton/test/resulthandler.h b/searchcore/src/vespa/searchcore/proton/test/resulthandler.h index 00896cb84fe..d5954e30678 100644 --- a/searchcore/src/vespa/searchcore/proton/test/resulthandler.h +++ b/searchcore/src/vespa/searchcore/proton/test/resulthandler.h @@ -3,19 +3,16 @@ #include <vespa/searchcore/proton/persistenceengine/resulthandler.h> -namespace proton { - -namespace test { +namespace proton::test { class GenericResultHandler : public IGenericResultHandler { private: std::unique_ptr<storage::spi::Result> _result; public: - void handle(const storage::spi::Result &result) override { - _result.reset(new storage::spi::Result(result)); - } - bool valid() const { return _result.get() != NULL; } + ~GenericResultHandler() override; + void handle(const storage::spi::Result &result) override; + bool valid() const { return static_cast<bool>(_result); } const storage::spi::Result &getResult() const { return *_result; } }; @@ -25,10 +22,9 @@ class BucketInfoResultHandler : public IBucketInfoResultHandler private: std::unique_ptr<storage::spi::BucketInfoResult> _result; public: - void handle(const storage::spi::BucketInfoResult &result) override { - _result.reset(new storage::spi::BucketInfoResult(result)); - } - bool valid() const { return _result.get() != NULL; } + ~BucketInfoResultHandler() override; + void handle(const storage::spi::BucketInfoResult &result) override; + bool valid() const { return static_cast<bool>(_result); } const storage::spi::BucketInfoResult &getResult() const { return *_result; } const storage::spi::BucketInfo &getInfo() const { return getResult().getBucketInfo(); } }; @@ -39,16 +35,11 @@ class BucketIdListResultHandler : public IBucketIdListResultHandler private: std::unique_ptr<storage::spi::BucketIdListResult> _result; public: - void handle(const storage::spi::BucketIdListResult &result) override { - _result.reset(new storage::spi::BucketIdListResult(result)); - } - bool valid() const { return _result.get() != NULL; } + ~BucketIdListResultHandler() override; + void handle(storage::spi::BucketIdListResult result) override; + bool valid() const { return static_cast<bool>(_result); } const storage::spi::BucketIdListResult &getResult() const { return *_result; } const storage::spi::BucketIdListResult::List &getList() const { return getResult().getList(); } }; - -} // namespace test - -} // namespace proton - +} diff --git a/storage/src/tests/persistence/filestorage/filestormodifiedbucketstest.cpp b/storage/src/tests/persistence/filestorage/filestormodifiedbucketstest.cpp index 3a1f41cb82c..8ce1ce4ea03 100644 --- a/storage/src/tests/persistence/filestorage/filestormodifiedbucketstest.cpp +++ b/storage/src/tests/persistence/filestorage/filestormodifiedbucketstest.cpp @@ -67,7 +67,7 @@ FileStorModifiedBucketsTest::modifyBuckets(uint32_t first, uint32_t count) spi::BucketInfo::ACTIVE); } - getDummyPersistence().setModifiedBuckets(buckets); + getDummyPersistence().setModifiedBuckets(std::move(buckets)); } TEST_F(FileStorModifiedBucketsTest, modified_buckets_send_notify_bucket_change) { diff --git a/storage/src/tests/persistence/filestorage/modifiedbucketcheckertest.cpp b/storage/src/tests/persistence/filestorage/modifiedbucketcheckertest.cpp index 55fd2680832..ae1ac2d65b4 100644 --- a/storage/src/tests/persistence/filestorage/modifiedbucketcheckertest.cpp +++ b/storage/src/tests/persistence/filestorage/modifiedbucketcheckertest.cpp @@ -69,7 +69,7 @@ ModifiedBucketCheckerTest::modifyBuckets(uint32_t count, uint32_t firstBucket) for (uint32_t i = firstBucket; i < firstBucket + count; ++i) { buckets.push_back(document::BucketId(16, i)); } - getDummyPersistence().setModifiedBuckets(buckets); + getDummyPersistence().setModifiedBuckets(std::move(buckets)); } void |