aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHenning Baldersheim <balder@yahoo-inc.com>2022-03-09 12:13:43 +0000
committerHenning Baldersheim <balder@yahoo-inc.com>2022-03-09 21:35:47 +0000
commit9e6251c1e2246da4f490ae1013fc3972504c42ab (patch)
tree201d9a0fc85e8ec1fbba9833fea58a8f81b041b7
parent1fa852eadd7b09b98ce6dd8bdd7b0446d07a37d6 (diff)
Move BucketIdListResult
-rw-r--r--document/src/vespa/document/bucket/bucketid.cpp1
-rw-r--r--document/src/vespa/document/bucket/bucketidlist.cpp3
-rw-r--r--document/src/vespa/document/bucket/bucketidlist.h11
-rw-r--r--persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp10
-rw-r--r--persistence/src/vespa/persistence/dummyimpl/dummypersistence.h4
-rw-r--r--persistence/src/vespa/persistence/spi/abstractpersistenceprovider.cpp3
-rw-r--r--persistence/src/vespa/persistence/spi/result.h28
-rw-r--r--searchcore/src/tests/proton/persistenceengine/persistence_handler_map/persistence_handler_map_test.cpp2
-rw-r--r--searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp13
-rw-r--r--searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp22
-rw-r--r--searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h4
-rw-r--r--searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp7
-rw-r--r--searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h2
-rw-r--r--searchcore/src/vespa/searchcore/proton/documentmetastore/i_bucket_handler.h2
-rw-r--r--searchcore/src/vespa/searchcore/proton/persistenceengine/ipersistencehandler.h2
-rw-r--r--searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp24
-rw-r--r--searchcore/src/vespa/searchcore/proton/persistenceengine/resulthandler.h6
-rw-r--r--searchcore/src/vespa/searchcore/proton/server/buckethandler.cpp18
-rw-r--r--searchcore/src/vespa/searchcore/proton/server/buckethandler.h2
-rw-r--r--searchcore/src/vespa/searchcore/proton/server/clusterstatehandler.cpp7
-rw-r--r--searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.cpp4
-rw-r--r--searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.h2
-rw-r--r--searchcore/src/vespa/searchcore/proton/test/CMakeLists.txt1
-rw-r--r--searchcore/src/vespa/searchcore/proton/test/bucketstatecalculator.h7
-rw-r--r--searchcore/src/vespa/searchcore/proton/test/document_meta_store_observer.h4
-rw-r--r--searchcore/src/vespa/searchcore/proton/test/resulthandler.cpp27
-rw-r--r--searchcore/src/vespa/searchcore/proton/test/resulthandler.h31
-rw-r--r--storage/src/tests/persistence/filestorage/filestormodifiedbucketstest.cpp2
-rw-r--r--storage/src/tests/persistence/filestorage/modifiedbucketcheckertest.cpp2
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