diff options
author | Tor Brede Vekterli <vekterli@yahooinc.com> | 2022-12-15 10:04:04 +0000 |
---|---|---|
committer | Tor Brede Vekterli <vekterli@yahooinc.com> | 2022-12-15 12:41:49 +0000 |
commit | c3a2b291ae397934d6b06ef027ce9cc80437d75a (patch) | |
tree | 0cc7f134e9e48bc88fdd4e5222ae2f55b1fd6587 | |
parent | 73e52213eb642957fa7a5ddc53dbebbb121761df (diff) |
Misc cleanup and formatting of distributor code
No functional changes
101 files changed, 455 insertions, 676 deletions
diff --git a/document/src/vespa/document/bucket/bucketid.h b/document/src/vespa/document/bucket/bucketid.h index 370948c1acc..e83418fb07e 100644 --- a/document/src/vespa/document/bucket/bucketid.h +++ b/document/src/vespa/document/bucket/bucketid.h @@ -49,7 +49,7 @@ public: /** Create an initially unset bucket id. */ constexpr BucketId() noexcept : _id(0) {} /** Create a bucket id with the given raw unchecked content. */ - explicit BucketId(Type id) noexcept : _id(id) {} + constexpr explicit BucketId(Type id) noexcept : _id(id) {} /** Create a bucket id using a set of bits from a raw unchecked value. */ BucketId(uint32_t useBits, Type id) noexcept : _id(createUsedBits(useBits, id)) { } diff --git a/storage/src/tests/distributor/bucketgctimecalculatortest.cpp b/storage/src/tests/distributor/bucketgctimecalculatortest.cpp index 33cd7bd637a..aaa228758b0 100644 --- a/storage/src/tests/distributor/bucketgctimecalculatortest.cpp +++ b/storage/src/tests/distributor/bucketgctimecalculatortest.cpp @@ -12,7 +12,7 @@ struct MockBucketIdHasher : public BucketGcTimeCalculator::BucketIdHasher { size_t nextGeneratedHash {0}; - size_t doHash(const document::BucketId&) const override { + size_t doHash(const document::BucketId&) const noexcept override { return nextGeneratedHash; } }; diff --git a/storage/src/tests/distributor/maintenancemocks.h b/storage/src/tests/distributor/maintenancemocks.h index 2809a81f79b..5bcd8f962a6 100644 --- a/storage/src/tests/distributor/maintenancemocks.h +++ b/storage/src/tests/distributor/maintenancemocks.h @@ -50,7 +50,7 @@ public: } void onClose(DistributorStripeMessageSender&) override {} - const char* getName() const override { return "MockOperation"; } + const char* getName() const noexcept override { return "MockOperation"; } const std::string& getDetailedReason() const override { return _reason; } diff --git a/storage/src/vespa/storage/bucketdb/bucketmanager.cpp b/storage/src/vespa/storage/bucketdb/bucketmanager.cpp index 9b3ad5c8e22..64c66401992 100644 --- a/storage/src/vespa/storage/bucketdb/bucketmanager.cpp +++ b/storage/src/vespa/storage/bucketdb/bucketmanager.cpp @@ -97,11 +97,12 @@ namespace { template<bool log> class DistributorInfoGatherer { - typedef api::RequestBucketInfoReply::EntryVector ResultArray; - DistributorStateCache _state; + using ResultArray = api::RequestBucketInfoReply::EntryVector; + + DistributorStateCache _state; std::unordered_map<uint16_t, ResultArray>& _result; - const document::BucketIdFactory& _factory; - std::shared_ptr<const lib::Distribution> _storageDistribution; + const document::BucketIdFactory& _factory; + std::shared_ptr<const lib::Distribution> _storageDistribution; public: DistributorInfoGatherer( @@ -123,9 +124,7 @@ namespace { try{ uint16_t i = _state.getOwner(b); auto it = _result.find(i); - // Template parameter. This block should not be included - // in version not logging. - if (log) { + if constexpr (log) { LOG(spam, "Bucket %s (reverse %" PRIu64 "), should be handled" " by distributor %u which we are %sgenerating " "state for.", @@ -164,13 +163,13 @@ namespace { uint64_t active; uint64_t ready; - Count() noexcept : docs(0), bytes(0), buckets(0), active(0), ready(0) {} + constexpr Count() noexcept : docs(0), bytes(0), buckets(0), active(0), ready(0) {} }; Count count; uint32_t lowestUsedBit; - MetricsUpdater() noexcept + constexpr MetricsUpdater() noexcept : count(), lowestUsedBit(58) {} void operator()(document::BucketId::Type bucketId, diff --git a/storage/src/vespa/storage/common/bucket_resolver.h b/storage/src/vespa/storage/common/bucket_resolver.h index 0b8e8b18430..ea261200b7b 100644 --- a/storage/src/vespa/storage/common/bucket_resolver.h +++ b/storage/src/vespa/storage/common/bucket_resolver.h @@ -12,7 +12,7 @@ namespace storage { * Interface for resolving which bucket a given a document id belongs to. */ struct BucketResolver { - virtual ~BucketResolver() {} + virtual ~BucketResolver() = default; virtual document::Bucket bucketFromId(const document::DocumentId &documentId) const = 0; virtual document::BucketSpace bucketSpaceFromName(const vespalib::string &bucketSpace) const = 0; virtual vespalib::string nameFromBucketSpace(const document::BucketSpace &bucketSpace) const = 0; diff --git a/storage/src/vespa/storage/common/bucketmessages.h b/storage/src/vespa/storage/common/bucketmessages.h index ca3b28188ec..ccee12938b2 100644 --- a/storage/src/vespa/storage/common/bucketmessages.h +++ b/storage/src/vespa/storage/common/bucketmessages.h @@ -17,8 +17,8 @@ class ReadBucketList : public api::InternalCommand { document::BucketSpace _bucketSpace; public: - typedef std::unique_ptr<ReadBucketList> UP; - static const uint32_t ID = 2003; + using UP = std::unique_ptr<ReadBucketList>; + static constexpr uint32_t ID = 2003; ReadBucketList(document::BucketSpace bucketSpace); ~ReadBucketList(); @@ -40,9 +40,9 @@ class ReadBucketListReply : public api::InternalReply { spi::BucketIdListResult::List _buckets; public: - typedef std::unique_ptr<ReadBucketListReply> UP; - typedef std::shared_ptr<ReadBucketListReply> SP; - static const uint32_t ID = 2004; + using UP = std::unique_ptr<ReadBucketListReply>; + using SP = std::shared_ptr<ReadBucketListReply>; + static constexpr uint32_t ID = 2004; ReadBucketListReply(const ReadBucketList& cmd); ~ReadBucketListReply(); @@ -72,7 +72,7 @@ class ReadBucketInfo : public api::InternalCommand { document::Bucket _bucket; public: - static const uint32_t ID = 2005; + static constexpr uint32_t ID = 2005; ReadBucketInfo(const document::Bucket &bucket); ~ReadBucketInfo(); @@ -95,7 +95,7 @@ class ReadBucketInfoReply : public api::InternalReply { document::Bucket _bucket; public: - static const uint32_t ID = 2006; + static constexpr uint32_t ID = 2006; ReadBucketInfoReply(const ReadBucketInfo& cmd); ~ReadBucketInfoReply(); diff --git a/storage/src/vespa/storage/common/doneinitializehandler.h b/storage/src/vespa/storage/common/doneinitializehandler.h index 41566f493b1..9aaa4e04dc7 100644 --- a/storage/src/vespa/storage/common/doneinitializehandler.h +++ b/storage/src/vespa/storage/common/doneinitializehandler.h @@ -13,7 +13,7 @@ namespace storage { struct DoneInitializeHandler { - virtual ~DoneInitializeHandler() {} + virtual ~DoneInitializeHandler() = default; virtual void notifyDoneInitializing() = 0; }; diff --git a/storage/src/vespa/storage/common/hostreporter/CMakeLists.txt b/storage/src/vespa/storage/common/hostreporter/CMakeLists.txt index 04006b3994b..60919a80bab 100644 --- a/storage/src/vespa/storage/common/hostreporter/CMakeLists.txt +++ b/storage/src/vespa/storage/common/hostreporter/CMakeLists.txt @@ -2,7 +2,6 @@ vespa_add_library(storage_hostreporter OBJECT SOURCES hostinfo.cpp - kernelmetrictool.cpp versionreporter.cpp DEPENDS ) diff --git a/storage/src/vespa/storage/common/hostreporter/kernelmetrictool.cpp b/storage/src/vespa/storage/common/hostreporter/kernelmetrictool.cpp deleted file mode 100644 index d831a404654..00000000000 --- a/storage/src/vespa/storage/common/hostreporter/kernelmetrictool.cpp +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "kernelmetrictool.h" -#include <vespa/vespalib/io/fileutil.h> -#include <vespa/vespalib/text/stringtokenizer.h> -#include <vespa/vespalib/stllike/asciistream.h> -#include <vespa/vespalib/util/exceptions.h> -#include <cctype> - -namespace storage { -namespace kernelmetrictool { - - -vespalib::string readFile(const char* fileName) { - return vespalib::asciistream::createFromDevice(fileName).str(); -} - -vespalib::string stripWhitespace(const vespalib::string& s) { - vespalib::string::size_type start(0); - vespalib::string::size_type stop(s.size() - 1); - while (true) { - if (start == s.size()) return vespalib::string(""); - if (!std::isspace(s[start])) break; - ++start; - } - while (true) { - if (!std::isspace(s[stop])) break; - --stop; - } - return s.substr(start, stop - start + 1); -} - -vespalib::string getLine(vespalib::stringref key, - vespalib::stringref content) -{ - vespalib::string::size_type start(0); - vespalib::string::size_type stop(content.find('\n')); - while (true) { - bool last = (stop == vespalib::string::npos); - vespalib::stringref line(content.substr(start, stop - start)); - for (uint32_t i=0, n=line.size(); i<n; ++i) { - if (std::isspace(line[i])) { - vespalib::stringref s(line.substr(0, i)); - if (s == key) return line; - } - } - if (last) break; - start = stop + 1; - stop = content.find('\n', start); - } - return ""; -} - -vespalib::string getToken(uint32_t index, const vespalib::string& line) { - vespalib::StringTokenizer st(line, " \t\n", ""); - st.removeEmptyTokens(); - return (index >= st.size() ? "" : st[index]); -} - -uint32_t getTokenCount(const vespalib::string& line) { - vespalib::StringTokenizer st(line, " \t\n", ""); - st.removeEmptyTokens(); - return st.size(); -} - -uint64_t toLong(vespalib::stringref s, int base) { - char* endptr; - // FIXME C++17 range-safe from_chars() instead of strtoull() - uint64_t result(strtoull(s.data(), &endptr, base)); - if ((s.data() + s.size()) != endptr) { - throw vespalib::IllegalArgumentException("Parsing '" + s + "' as a long."); - } - return result; -} -} -} /* namespace storage */ diff --git a/storage/src/vespa/storage/common/hostreporter/kernelmetrictool.h b/storage/src/vespa/storage/common/hostreporter/kernelmetrictool.h deleted file mode 100644 index df961c882e1..00000000000 --- a/storage/src/vespa/storage/common/hostreporter/kernelmetrictool.h +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -/* - * This file contains various tools for use by reporters when fetching os information. - */ - -#ifndef STORAGE_SRC_CPP_STORAGE_COMMON_HOSTREPORTER_KERNELMETRICTOOL_H_ -#define STORAGE_SRC_CPP_STORAGE_COMMON_HOSTREPORTER_KERNELMETRICTOOL_H_ - -#include <vespa/vespalib/stllike/string.h> - -namespace storage { -namespace kernelmetrictool { - -vespalib::string readFile(const char* fileName); - -vespalib::string stripWhitespace(const vespalib::string& s); - -vespalib::string getLine(vespalib::stringref key, - vespalib::stringref content); - -vespalib::string getToken(uint32_t index, const vespalib::string& line); - -uint32_t getTokenCount(const vespalib::string& line); - -uint64_t toLong(vespalib::stringref s, int base = 0) ; - -} /* namespace kernelmetrictool */ -} /* namespace storage */ - -#endif /* STORAGE_SRC_CPP_STORAGE_COMMON_HOSTREPORTER_KERNELMETRICTOOL_H_ */ diff --git a/storage/src/vespa/storage/common/messagebucket.h b/storage/src/vespa/storage/common/messagebucket.h index b84c36bf08c..c8805cad1b3 100644 --- a/storage/src/vespa/storage/common/messagebucket.h +++ b/storage/src/vespa/storage/common/messagebucket.h @@ -14,8 +14,7 @@ class StorageMessage; * @throws vespalib::IllegalArgumentException if msg does not * have a bucket id. */ -document::Bucket getStorageMessageBucket( - const api::StorageMessage& msg); +document::Bucket getStorageMessageBucket(const api::StorageMessage& msg); } diff --git a/storage/src/vespa/storage/common/nodestateupdater.h b/storage/src/vespa/storage/common/nodestateupdater.h index 8d378e1ecd5..842828a1b89 100644 --- a/storage/src/vespa/storage/common/nodestateupdater.h +++ b/storage/src/vespa/storage/common/nodestateupdater.h @@ -36,7 +36,7 @@ struct StateListener { }; struct NodeStateUpdater { - typedef std::unique_ptr<NodeStateUpdater> UP; + using UP = std::unique_ptr<NodeStateUpdater>; virtual ~NodeStateUpdater() = default; @@ -53,8 +53,8 @@ struct NodeStateUpdater { * before altering the state. */ struct Lock { - typedef std::shared_ptr<Lock> SP; - virtual ~Lock() {} + using SP = std::shared_ptr<Lock>; + virtual ~Lock() = default; }; virtual Lock::SP grabStateChangeLock() = 0; diff --git a/storage/src/vespa/storage/common/servicelayercomponent.h b/storage/src/vespa/storage/common/servicelayercomponent.h index 3eaec863aa5..2ab8d6a8dfb 100644 --- a/storage/src/vespa/storage/common/servicelayercomponent.h +++ b/storage/src/vespa/storage/common/servicelayercomponent.h @@ -59,7 +59,7 @@ class ServiceLayerComponent : public StorageComponent, _minUsedBitsTracker = &tracker; } public: - typedef std::unique_ptr<ServiceLayerComponent> UP; + using UP = std::unique_ptr<ServiceLayerComponent>; ServiceLayerComponent(ServiceLayerComponentRegister& compReg, vespalib::stringref name) diff --git a/storage/src/vespa/storage/common/statusmessages.h b/storage/src/vespa/storage/common/statusmessages.h index 336df137e41..12432bfe095 100644 --- a/storage/src/vespa/storage/common/statusmessages.h +++ b/storage/src/vespa/storage/common/statusmessages.h @@ -23,7 +23,7 @@ class RequestStatusPage : public api::InternalCommand { // in which results should be sorted on status page. // (Used by filestor threads) public: - static const uint32_t ID = 2100; + static constexpr uint32_t ID = 2100; RequestStatusPage(const framework::HttpUrlPath& path); ~RequestStatusPage(); @@ -46,7 +46,7 @@ class RequestStatusPageReply : public api::InternalReply { std::string _status; std::string _sortToken; public: - static const uint32_t ID = 2101; + static constexpr uint32_t ID = 2101; RequestStatusPageReply(const RequestStatusPage& cmd, const std::string& status); ~RequestStatusPageReply(); diff --git a/storage/src/vespa/storage/common/storagelink.h b/storage/src/vespa/storage/common/storagelink.h index 2111dbadbc4..4961bd96741 100644 --- a/storage/src/vespa/storage/common/storagelink.h +++ b/storage/src/vespa/storage/common/storagelink.h @@ -37,7 +37,7 @@ class StorageLink : public document::Printable, protected api::MessageHandler { public: - typedef std::unique_ptr<StorageLink> UP; + using UP = std::unique_ptr<StorageLink>; enum State { CREATED, OPENED, CLOSING, FLUSHINGDOWN, FLUSHINGUP, CLOSED }; diff --git a/storage/src/vespa/storage/common/vectorprinter.h b/storage/src/vespa/storage/common/vectorprinter.h deleted file mode 100644 index 62daba9bd70..00000000000 --- a/storage/src/vespa/storage/common/vectorprinter.h +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#pragma once - -#include <vector> -#include <ostream> - -namespace storage { - -template <typename T> -class VectorPrinter -{ -public: - VectorPrinter(const std::vector<T>& vec, - const char* separator) - : _vec(&vec), - _separator(separator) - {} - - void print(std::ostream& os) const { - for (uint32_t i = 0; i < _vec->size(); ++i) { - if (i != 0) { - os << _separator; - } - os << (*_vec)[i]; - } - } -private: - const std::vector<T>* _vec; - const char* _separator; -}; - -template <typename T> -inline std::ostream& -operator<<(std::ostream& os, const VectorPrinter<T>& printer) -{ - printer.print(os); - return os; -} - -template <typename T> -inline VectorPrinter<T> -commaSeparated(const std::vector<T>& vec) -{ - return VectorPrinter<T>(vec, ","); -} - -} - diff --git a/storage/src/vespa/storage/common/visitorfactory.h b/storage/src/vespa/storage/common/visitorfactory.h index 48b9dd6d15f..a60ffadbdb0 100644 --- a/storage/src/vespa/storage/common/visitorfactory.h +++ b/storage/src/vespa/storage/common/visitorfactory.h @@ -16,17 +16,17 @@ class Visitor; class VisitorEnvironment { public: - typedef std::unique_ptr<VisitorEnvironment> UP; - VisitorEnvironment() {} - virtual ~VisitorEnvironment() {} + using UP = std::unique_ptr<VisitorEnvironment>; + VisitorEnvironment() = default; + virtual ~VisitorEnvironment() = default; }; class VisitorFactory { public: - typedef std::shared_ptr<VisitorFactory> SP; - typedef std::map<std::string, std::shared_ptr<VisitorFactory> > Map; + using SP = std::shared_ptr<VisitorFactory>; + using Map = std::map<std::string, std::shared_ptr<VisitorFactory>>; - virtual ~VisitorFactory() {}; + virtual ~VisitorFactory() = default; virtual VisitorEnvironment::UP makeVisitorEnvironment(StorageComponent&) = 0; diff --git a/storage/src/vespa/storage/distributor/activecopy.cpp b/storage/src/vespa/storage/distributor/activecopy.cpp index 71fcc77bd0b..a4ee4a51135 100644 --- a/storage/src/vespa/storage/distributor/activecopy.cpp +++ b/storage/src/vespa/storage/distributor/activecopy.cpp @@ -15,7 +15,9 @@ namespace std { for (uint32_t i=0; i<v.size(); ++i) { out << "\n " << v[i]; } - if (!v.empty()) out << "\n"; + if (!v.empty()) { + out << "\n"; + } return out << "]"; } } @@ -113,7 +115,9 @@ namespace { result.reserve(e->getNodeCount()); for (uint32_t i=0, n=e->getNodeCount(); i < n; ++i) { const BucketCopy& cp = e->getNodeRef(i); - if (!cp.valid()) continue; + if (!cp.valid()) { + continue; + } result.push_back(cp.getNode()); } return result; @@ -185,9 +189,13 @@ ActiveList::print(std::ostream& out, bool verbose, out << "\n" << indent << " " << _v[i]._nodeIndex << " " << _v[i].getReason(); } - if (!_v.empty()) out << "\n" << indent; + if (!_v.empty()) { + out << "\n" << indent; + } } else { - if (!_v.empty()) out << _v[0]._nodeIndex; + if (!_v.empty()) { + out << _v[0]._nodeIndex; + } for (size_t i=1; i<_v.size(); ++i) { out << " " << _v[i]._nodeIndex; } @@ -196,10 +204,12 @@ ActiveList::print(std::ostream& out, bool verbose, } bool -ActiveList::contains(uint16_t node) const +ActiveList::contains(uint16_t node) const noexcept { - for (const auto & candadate : _v) { - if (node == candadate._nodeIndex) return true; + for (const auto& candidate : _v) { + if (node == candidate._nodeIndex) { + return true; + } } return false; } diff --git a/storage/src/vespa/storage/distributor/activecopy.h b/storage/src/vespa/storage/distributor/activecopy.h index ee679519a19..258fe3cdf16 100644 --- a/storage/src/vespa/storage/distributor/activecopy.h +++ b/storage/src/vespa/storage/distributor/activecopy.h @@ -10,7 +10,7 @@ namespace storage::distributor { class ActiveList; struct ActiveCopy { - ActiveCopy() : _nodeIndex(-1), _ideal(-1), _doc_count(0), _ready(false), _active(false) { } + constexpr ActiveCopy() noexcept : _nodeIndex(-1), _ideal(-1), _doc_count(0), _ready(false), _active(false) { } ActiveCopy(uint16_t node, const BucketDatabase::Entry& e, const std::vector<uint16_t>& idealState); vespalib::string getReason() const; @@ -35,11 +35,11 @@ public: ActiveList() {} ActiveList(std::vector<ActiveCopy>&& v) : _v(std::move(v)) { } - ActiveCopy& operator[](size_t i) { return _v[i]; } - const ActiveCopy& operator[](size_t i) const { return _v[i]; } - bool contains(uint16_t) const; - bool empty() const { return _v.empty(); } - size_t size() const { return _v.size(); } + ActiveCopy& operator[](size_t i) noexcept { return _v[i]; } + const ActiveCopy& operator[](size_t i) const noexcept { return _v[i]; } + [[nodiscard]] bool contains(uint16_t) const noexcept; + [[nodiscard]] bool empty() const noexcept { return _v.empty(); } + size_t size() const noexcept { return _v.size(); } void print(std::ostream&, bool verbose, const std::string& indent) const override; }; diff --git a/storage/src/vespa/storage/distributor/bucket_ownership_flags.h b/storage/src/vespa/storage/distributor/bucket_ownership_flags.h index a72aa33265e..e40af18d9df 100644 --- a/storage/src/vespa/storage/distributor/bucket_ownership_flags.h +++ b/storage/src/vespa/storage/distributor/bucket_ownership_flags.h @@ -17,14 +17,14 @@ class BucketOwnershipFlags { static constexpr uint8_t owned_in_pending_state_flag = 0x2; public: - BucketOwnershipFlags() noexcept + constexpr BucketOwnershipFlags() noexcept : _flags(0) { } - bool owned_in_current_state() const noexcept { return ((_flags & owned_in_current_state_flag) != 0); } - bool owned_in_pending_state() const noexcept { return ((_flags & owned_in_pending_state_flag) != 0); } - void set_owned_in_current_state() noexcept { _flags |= owned_in_current_state_flag; } - void set_owned_in_pending_state() noexcept { _flags |= owned_in_pending_state_flag; } + constexpr bool owned_in_current_state() const noexcept { return ((_flags & owned_in_current_state_flag) != 0); } + constexpr bool owned_in_pending_state() const noexcept { return ((_flags & owned_in_pending_state_flag) != 0); } + constexpr void set_owned_in_current_state() noexcept { _flags |= owned_in_current_state_flag; } + constexpr void set_owned_in_pending_state() noexcept { _flags |= owned_in_pending_state_flag; } }; } diff --git a/storage/src/vespa/storage/distributor/bucket_spaces_stats_provider.h b/storage/src/vespa/storage/distributor/bucket_spaces_stats_provider.h index eee56d92576..18dc3c93cd8 100644 --- a/storage/src/vespa/storage/distributor/bucket_spaces_stats_provider.h +++ b/storage/src/vespa/storage/distributor/bucket_spaces_stats_provider.h @@ -17,30 +17,30 @@ private: size_t _bucketsTotal; size_t _bucketsPending; public: - BucketSpaceStats(size_t bucketsTotal_, size_t bucketsPending_) + constexpr BucketSpaceStats(size_t bucketsTotal_, size_t bucketsPending_) noexcept : _valid(true), _bucketsTotal(bucketsTotal_), _bucketsPending(bucketsPending_) {} - BucketSpaceStats() + constexpr BucketSpaceStats() noexcept : _valid(false), _bucketsTotal(0), _bucketsPending(0) {} - static BucketSpaceStats make_invalid() noexcept { return {}; } + static constexpr BucketSpaceStats make_invalid() noexcept { return {}; } - bool valid() const noexcept { return _valid; } + [[nodiscard]] bool valid() const noexcept { return _valid; } size_t bucketsTotal() const noexcept { return _bucketsTotal; } size_t bucketsPending() const noexcept { return _bucketsPending; } - bool operator==(const BucketSpaceStats& rhs) const { + bool operator==(const BucketSpaceStats& rhs) const noexcept { return (_valid == rhs._valid) && (_bucketsTotal == rhs._bucketsTotal) && (_bucketsPending == rhs._bucketsPending); } - void merge(const BucketSpaceStats& rhs) { + void merge(const BucketSpaceStats& rhs) noexcept { _valid = _valid && rhs._valid; _bucketsTotal += rhs._bucketsTotal; _bucketsPending += rhs._bucketsPending; diff --git a/storage/src/vespa/storage/distributor/bucketgctimecalculator.h b/storage/src/vespa/storage/distributor/bucketgctimecalculator.h index 201173c661a..f8d30b6e526 100644 --- a/storage/src/vespa/storage/distributor/bucketgctimecalculator.h +++ b/storage/src/vespa/storage/distributor/bucketgctimecalculator.h @@ -22,14 +22,14 @@ class BucketGcTimeCalculator { public: class BucketIdHasher { - virtual size_t doHash(const document::BucketId&) const = 0; + virtual size_t doHash(const document::BucketId&) const noexcept = 0; public: - virtual ~BucketIdHasher() {} - size_t hash(const document::BucketId& b) const { return doHash(b); } + virtual ~BucketIdHasher() = default; + size_t hash(const document::BucketId& b) const noexcept { return doHash(b); } }; class BucketIdIdentityHasher : public BucketIdHasher { - size_t doHash(const document::BucketId& b) const override { + size_t doHash(const document::BucketId& b) const noexcept override { return b.getId(); } }; diff --git a/storage/src/vespa/storage/distributor/bucketownership.h b/storage/src/vespa/storage/distributor/bucketownership.h index 46b38b46bf7..89678e42375 100644 --- a/storage/src/vespa/storage/distributor/bucketownership.h +++ b/storage/src/vespa/storage/distributor/bucketownership.h @@ -12,15 +12,15 @@ class BucketOwnership const lib::ClusterState* _checkedState; bool _owned; - BucketOwnership(const lib::ClusterState& checkedState) + BucketOwnership(const lib::ClusterState& checkedState) noexcept : _checkedState(&checkedState), _owned(false) { } public: - BucketOwnership() : _checkedState(nullptr), _owned(true) {} + constexpr BucketOwnership() noexcept : _checkedState(nullptr), _owned(true) {} - bool isOwned() const { return _owned; } + [[nodiscard]] bool isOwned() const noexcept { return _owned; } /** * Cluster state in which the ownership check failed. Lifetime of returned * reference depends on when the active or pending cluster state of the @@ -30,16 +30,16 @@ public: * * Precondition: isOwned() == false */ - const lib::ClusterState& getNonOwnedState() { + const lib::ClusterState& getNonOwnedState() noexcept { assert(!isOwned()); return *_checkedState; } - static BucketOwnership createOwned() { + static constexpr BucketOwnership createOwned() noexcept { return BucketOwnership(); } - static BucketOwnership createNotOwnedInState(const lib::ClusterState& s) { + static BucketOwnership createNotOwnedInState(const lib::ClusterState& s) noexcept { return BucketOwnership(s); } }; diff --git a/storage/src/vespa/storage/distributor/common/.gitignore b/storage/src/vespa/storage/distributor/common/.gitignore deleted file mode 100644 index e69de29bb2d..00000000000 --- a/storage/src/vespa/storage/distributor/common/.gitignore +++ /dev/null diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space.h b/storage/src/vespa/storage/distributor/distributor_bucket_space.h index 3dfd1e1ce30..f38556a664c 100644 --- a/storage/src/vespa/storage/distributor/distributor_bucket_space.h +++ b/storage/src/vespa/storage/distributor/distributor_bucket_space.h @@ -32,7 +32,7 @@ namespace storage::distributor { * bucket spaces. */ class DistributorBucketSpace { - std::unique_ptr<BucketDatabase> _bucketDatabase; + std::unique_ptr<BucketDatabase> _bucketDatabase; std::shared_ptr<const lib::ClusterState> _clusterState; std::shared_ptr<const lib::Distribution> _distribution; uint16_t _node_index; diff --git a/storage/src/vespa/storage/distributor/distributor_host_info_reporter.h b/storage/src/vespa/storage/distributor/distributor_host_info_reporter.h index 6680059b3d6..412867b537f 100644 --- a/storage/src/vespa/storage/distributor/distributor_host_info_reporter.h +++ b/storage/src/vespa/storage/distributor/distributor_host_info_reporter.h @@ -4,8 +4,7 @@ #include <vespa/storage/common/hostreporter/hostreporter.h> #include <atomic> -namespace storage { -namespace distributor { +namespace storage::distributor { class BucketSpacesStatsProvider; class MinReplicaProvider; @@ -46,6 +45,4 @@ private: std::atomic<bool> _enabled; }; -} // distributor -} // storage - +} // storage::distributor diff --git a/storage/src/vespa/storage/distributor/distributor_interface.h b/storage/src/vespa/storage/distributor/distributor_interface.h index 083c49f4ae3..b66d3ea198f 100644 --- a/storage/src/vespa/storage/distributor/distributor_interface.h +++ b/storage/src/vespa/storage/distributor/distributor_interface.h @@ -14,7 +14,7 @@ class DistributorMetricSet; */ class DistributorInterface : public DistributorMessageSender { public: - virtual ~DistributorInterface() {} + virtual ~DistributorInterface() = default; virtual DistributorMetricSet& metrics() = 0; virtual const DistributorConfiguration& config() const = 0; }; diff --git a/storage/src/vespa/storage/distributor/distributor_node_context.h b/storage/src/vespa/storage/distributor/distributor_node_context.h index ad082a5a9e5..4e25a479456 100644 --- a/storage/src/vespa/storage/distributor/distributor_node_context.h +++ b/storage/src/vespa/storage/distributor/distributor_node_context.h @@ -17,7 +17,7 @@ namespace storage::distributor { */ class DistributorNodeContext : public ClusterContext { public: - virtual ~DistributorNodeContext() {} + virtual ~DistributorNodeContext() = default; virtual const framework::Clock& clock() const noexcept = 0; virtual const document::BucketIdFactory& bucket_id_factory() const noexcept = 0; virtual uint16_t node_index() const noexcept = 0; diff --git a/storage/src/vespa/storage/distributor/document_selection_parser.h b/storage/src/vespa/storage/distributor/document_selection_parser.h index 3e7e0209429..45ca2bf1cde 100644 --- a/storage/src/vespa/storage/distributor/document_selection_parser.h +++ b/storage/src/vespa/storage/distributor/document_selection_parser.h @@ -14,7 +14,7 @@ namespace storage::distributor { */ class DocumentSelectionParser { public: - virtual ~DocumentSelectionParser() {} + virtual ~DocumentSelectionParser() = default; virtual std::unique_ptr<document::select::Node> parse_selection(const vespalib::string& str) const = 0; }; diff --git a/storage/src/vespa/storage/distributor/externaloperationhandler.h b/storage/src/vespa/storage/distributor/externaloperationhandler.h index 076b182abdd..50a2019a2ae 100644 --- a/storage/src/vespa/storage/distributor/externaloperationhandler.h +++ b/storage/src/vespa/storage/distributor/externaloperationhandler.h @@ -11,17 +11,17 @@ namespace documentapi { class TestAndSetCondition; } namespace storage::lib { class ClusterState; } -namespace storage { class PersistenceOperationMetricSet; } namespace storage::distributor { class DistributorMetricSet; -class TopLevelDistributor; -class MaintenanceOperationGenerator; class DirectDispatchSender; -class SequencingHandle; +class MaintenanceOperationGenerator; class OperationSequencer; class OperationOwner; +class PersistenceOperationMetricSet; +class SequencingHandle; +class TopLevelDistributor; class UuidGenerator; class ExternalOperationHandler : public api::MessageHandler diff --git a/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.h b/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.h index 2d77cf0c996..f98c4699eef 100644 --- a/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.h +++ b/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.h @@ -17,12 +17,20 @@ public: IdealServiceLayerNodesBundle() noexcept; ~IdealServiceLayerNodesBundle(); - void set_available_nodes(std::vector<uint16_t> available_nodes) { _available_nodes = std::move(available_nodes); } - void set_available_nonretired_nodes(std::vector<uint16_t> available_nonretired_nodes) { _available_nonretired_nodes = std::move(available_nonretired_nodes); } - void set_available_nonretired_or_maintenance_nodes(std::vector<uint16_t> available_nonretired_or_maintenance_nodes) { _available_nonretired_or_maintenance_nodes = std::move(available_nonretired_or_maintenance_nodes); } + void set_available_nodes(std::vector<uint16_t> available_nodes) { + _available_nodes = std::move(available_nodes); + } + void set_available_nonretired_nodes(std::vector<uint16_t> available_nonretired_nodes) { + _available_nonretired_nodes = std::move(available_nonretired_nodes); + } + void set_available_nonretired_or_maintenance_nodes(std::vector<uint16_t> available_nonretired_or_maintenance_nodes) { + _available_nonretired_or_maintenance_nodes = std::move(available_nonretired_or_maintenance_nodes); + } std::vector<uint16_t> get_available_nodes() const { return _available_nodes; } std::vector<uint16_t> get_available_nonretired_nodes() const { return _available_nonretired_nodes; } - std::vector<uint16_t> get_available_nonretired_or_maintenance_nodes() const { return _available_nonretired_or_maintenance_nodes; } + std::vector<uint16_t> get_available_nonretired_or_maintenance_nodes() const { + return _available_nonretired_or_maintenance_nodes; + } }; } diff --git a/storage/src/vespa/storage/distributor/idealstatemanager.cpp b/storage/src/vespa/storage/distributor/idealstatemanager.cpp index 728040da50e..cf255b5ec18 100644 --- a/storage/src/vespa/storage/distributor/idealstatemanager.cpp +++ b/storage/src/vespa/storage/distributor/idealstatemanager.cpp @@ -27,16 +27,15 @@ IdealStateManager::IdealStateManager( DistributorStripeOperationContext& op_ctx, IdealStateMetricSet& metrics) : _metrics(metrics), + _stateCheckers(), + _splitBucketStateChecker(nullptr), _node_ctx(node_ctx), _op_ctx(op_ctx), _has_logged_phantom_replica_warning(false) { - LOG(debug, "Adding BucketStateStateChecker to state checkers"); _stateCheckers.emplace_back(std::make_shared<BucketStateStateChecker>()); - _stateCheckers.emplace_back(std::make_shared<SplitBucketStateChecker>()); _splitBucketStateChecker = dynamic_cast<SplitBucketStateChecker *>(_stateCheckers.back().get()); - _stateCheckers.emplace_back(std::make_shared<SplitInconsistentStateChecker>()); _stateCheckers.emplace_back(std::make_shared<SynchronizeAndMoveStateChecker>()); _stateCheckers.emplace_back(std::make_shared<JoinBucketsStateChecker>()); @@ -141,7 +140,7 @@ void IdealStateManager::verify_only_live_nodes_in_context(const StateChecker::Co StateChecker::Result IdealStateManager::generateHighestPriority( - const document::Bucket &bucket, + const document::Bucket& bucket, NodeMaintenanceStatsTracker& statsTracker) const { auto& distributorBucketSpace = _op_ctx.bucket_space_repo().get(bucket.getBucketSpace()); @@ -162,7 +161,7 @@ IdealStateManager::generateHighestPriority( MaintenancePriorityAndType IdealStateManager::prioritize( - const document::Bucket &bucket, + const document::Bucket& bucket, NodeMaintenanceStatsTracker& statsTracker) const { StateChecker::Result generated(generateHighestPriority(bucket, statsTracker)); @@ -198,7 +197,7 @@ IdealStateManager::generateInterceptingSplit(BucketSpace bucketSpace, } MaintenanceOperation::SP -IdealStateManager::generate(const document::Bucket &bucket) const +IdealStateManager::generate(const document::Bucket& bucket) const { NodeMaintenanceStatsTracker statsTracker; IdealStateOperation::SP op( diff --git a/storage/src/vespa/storage/distributor/idealstatemanager.h b/storage/src/vespa/storage/distributor/idealstatemanager.h index 32349541de2..0c9e3ffa1c6 100644 --- a/storage/src/vespa/storage/distributor/idealstatemanager.h +++ b/storage/src/vespa/storage/distributor/idealstatemanager.h @@ -42,15 +42,15 @@ public: // MaintenancePriorityGenerator interface MaintenancePriorityAndType prioritize( - const document::Bucket &bucket, + const document::Bucket& bucket, NodeMaintenanceStatsTracker& statsTracker) const override; // MaintenanceOperationGenerator - MaintenanceOperation::SP generate(const document::Bucket &bucket) const override; + MaintenanceOperation::SP generate(const document::Bucket& bucket) const override; // MaintenanceOperationGenerator std::vector<MaintenanceOperation::SP> generateAll( - const document::Bucket &bucket, + const document::Bucket& bucket, NodeMaintenanceStatsTracker& statsTracker) const override; /** @@ -62,49 +62,43 @@ public: const BucketDatabase::Entry& e, api::StorageMessage::Priority pri); - IdealStateMetricSet& getMetrics() { return _metrics; } - + IdealStateMetricSet& getMetrics() noexcept { return _metrics; } void dump_bucket_space_db_status(document::BucketSpace bucket_space, std::ostream& out) const; void getBucketStatus(std::ostream& out) const; - const DistributorNodeContext& node_context() const { return _node_ctx; } - DistributorStripeOperationContext& operation_context() { return _op_ctx; } - const DistributorStripeOperationContext& operation_context() const { return _op_ctx; } - DistributorBucketSpaceRepo &getBucketSpaceRepo() { return _op_ctx.bucket_space_repo(); } - const DistributorBucketSpaceRepo &getBucketSpaceRepo() const { return _op_ctx.bucket_space_repo(); } + const DistributorNodeContext& node_context() const noexcept { return _node_ctx; } + DistributorStripeOperationContext& operation_context() noexcept { return _op_ctx; } + const DistributorStripeOperationContext& operation_context() const noexcept { return _op_ctx; } + DistributorBucketSpaceRepo &getBucketSpaceRepo() noexcept { return _op_ctx.bucket_space_repo(); } + const DistributorBucketSpaceRepo &getBucketSpaceRepo() const noexcept { return _op_ctx.bucket_space_repo(); } private: void verify_only_live_nodes_in_context(const StateChecker::Context& c) const; static void fillParentAndChildBuckets(StateChecker::Context& c); static void fillSiblingBucket(StateChecker::Context& c); StateChecker::Result generateHighestPriority( - const document::Bucket &bucket, + const document::Bucket& bucket, NodeMaintenanceStatsTracker& statsTracker) const; StateChecker::Result runStateCheckers(StateChecker::Context& c) const; static BucketDatabase::Entry* getEntryForPrimaryBucket(StateChecker::Context& c); - IdealStateMetricSet& _metrics; - document::BucketId _lastPrioritizedBucket; - - // Prioritized of state checkers that generate operations - // for idealstatemanager. - std::vector<StateChecker::SP> _stateCheckers; - SplitBucketStateChecker* _splitBucketStateChecker; - - const DistributorNodeContext& _node_ctx; + IdealStateMetricSet& _metrics; + std::vector<StateChecker::SP> _stateCheckers; + SplitBucketStateChecker* _splitBucketStateChecker; + const DistributorNodeContext& _node_ctx; DistributorStripeOperationContext& _op_ctx; - mutable bool _has_logged_phantom_replica_warning; + mutable bool _has_logged_phantom_replica_warning; class StatusBucketVisitor : public BucketDatabase::EntryProcessor { // Stats tracker to use for all generateAll() calls to avoid having // to create a new hash map for each single bucket processed. NodeMaintenanceStatsTracker _statsTracker; - const IdealStateManager & _ism; + const IdealStateManager& _ism; document::BucketSpace _bucketSpace; - std::ostream & _out; + std::ostream& _out; public: StatusBucketVisitor(const IdealStateManager& ism, document::BucketSpace bucketSpace, std::ostream& out) : _statsTracker(), _ism(ism), _bucketSpace(bucketSpace), _out(out) {} diff --git a/storage/src/vespa/storage/distributor/idealstatemetricsset.h b/storage/src/vespa/storage/distributor/idealstatemetricsset.h index 8062a65a01a..6528ad7dc72 100644 --- a/storage/src/vespa/storage/distributor/idealstatemetricsset.h +++ b/storage/src/vespa/storage/distributor/idealstatemetricsset.h @@ -6,9 +6,7 @@ #include <vespa/metrics/countmetric.h> #include <vespa/storage/distributor/operations/idealstate/idealstateoperation.h> -namespace storage { - -namespace distributor { +namespace storage::distributor { class OperationMetricSet : public metrics::MetricSet { @@ -50,7 +48,7 @@ public: metrics::LongValueMetric buckets_toomanycopies; metrics::LongValueMetric buckets; metrics::LongValueMetric buckets_notrusted; - metrics::LongValueMetric buckets_rechecking; // TODO remove, not used (but exposed by VespaMetricSet) + metrics::LongValueMetric buckets_rechecking; // TODO Vespa 9: remove, not used (but exposed by VespaMetricSet) metrics::LongValueMetric buckets_replicas_moving_out; metrics::LongValueMetric buckets_replicas_copying_in; metrics::LongValueMetric buckets_replicas_copying_out; @@ -66,7 +64,4 @@ public: void setPendingOperations(const std::vector<uint64_t>& newMetrics); }; -} - -} - +} // storage::distributor diff --git a/storage/src/vespa/storage/distributor/maintenance/bucketprioritydatabase.h b/storage/src/vespa/storage/distributor/maintenance/bucketprioritydatabase.h index 4c193492f43..f2f4b7cc275 100644 --- a/storage/src/vespa/storage/distributor/maintenance/bucketprioritydatabase.h +++ b/storage/src/vespa/storage/distributor/maintenance/bucketprioritydatabase.h @@ -14,9 +14,9 @@ protected: { public: virtual ~ConstIteratorImpl() = default; - virtual void increment() = 0; - virtual bool equal(const ConstIteratorImpl& other) const = 0; - virtual PrioritizedBucket dereference() const = 0; + virtual void increment() noexcept = 0; + virtual bool equal(const ConstIteratorImpl& other) const noexcept = 0; + virtual PrioritizedBucket dereference() const noexcept = 0; }; using ConstIteratorImplPtr = std::unique_ptr<ConstIteratorImpl>; @@ -31,25 +31,25 @@ public: { ConstIteratorImplPtr _impl; public: - explicit ConstIterator(ConstIteratorImplPtr impl) + explicit ConstIterator(ConstIteratorImplPtr impl) noexcept : _impl(std::move(impl)) {} ConstIterator(const ConstIterator &) = delete; - ConstIterator(ConstIterator &&) = default; + ConstIterator(ConstIterator &&) noexcept = default; virtual ~ConstIterator() = default; private: friend class boost::iterator_core_access; - void increment() { + void increment() noexcept { _impl->increment(); } - bool equal(const ConstIterator& other) const { + [[nodiscard]] bool equal(const ConstIterator& other) const noexcept { return _impl->equal(*other._impl); } - PrioritizedBucket dereference() const { + PrioritizedBucket dereference() const noexcept { return _impl->dereference(); } }; @@ -59,7 +59,7 @@ public: virtual ~BucketPriorityDatabase() = default; virtual const_iterator begin() const = 0; - virtual const_iterator end() const = 0; + virtual const_iterator end() const = 0; virtual void setPriority(const PrioritizedBucket&) = 0; }; diff --git a/storage/src/vespa/storage/distributor/maintenance/maintenancepriority.h b/storage/src/vespa/storage/distributor/maintenance/maintenancepriority.h index aa4f7b715f9..bb7ec904c16 100644 --- a/storage/src/vespa/storage/distributor/maintenance/maintenancepriority.h +++ b/storage/src/vespa/storage/distributor/maintenance/maintenancepriority.h @@ -20,33 +20,33 @@ public: static constexpr const char* toString(Priority pri) noexcept { switch (pri) { case NO_MAINTENANCE_NEEDED: return "NO_MAINTENANCE_NEEDED"; - case VERY_LOW: return "VERY_LOW"; - case LOW: return "LOW"; - case MEDIUM: return "MEDIUM"; - case HIGH: return "HIGH"; - case VERY_HIGH: return "VERY_HIGH"; - case HIGHEST: return "HIGHEST"; - default: return "INVALID"; + case VERY_LOW: return "VERY_LOW"; + case LOW: return "LOW"; + case MEDIUM: return "MEDIUM"; + case HIGH: return "HIGH"; + case VERY_HIGH: return "VERY_HIGH"; + case HIGHEST: return "HIGHEST"; + default: return "INVALID"; } } - MaintenancePriority() + constexpr MaintenancePriority() noexcept : _priority(NO_MAINTENANCE_NEEDED) {} - explicit MaintenancePriority(Priority priority) + constexpr explicit MaintenancePriority(Priority priority) noexcept : _priority(priority) {} - Priority getPriority() const { + constexpr Priority getPriority() const noexcept { return _priority; } - bool requiresMaintenance() const { + constexpr bool requiresMaintenance() const noexcept { return _priority != NO_MAINTENANCE_NEEDED; } - static MaintenancePriority noMaintenanceNeeded() { + static constexpr MaintenancePriority noMaintenanceNeeded() noexcept { return MaintenancePriority(NO_MAINTENANCE_NEEDED); } diff --git a/storage/src/vespa/storage/distributor/maintenance/maintenancepriorityandtype.h b/storage/src/vespa/storage/distributor/maintenance/maintenancepriorityandtype.h index 68602958815..3be7a4a18a7 100644 --- a/storage/src/vespa/storage/distributor/maintenance/maintenancepriorityandtype.h +++ b/storage/src/vespa/storage/distributor/maintenance/maintenancepriorityandtype.h @@ -11,21 +11,21 @@ class MaintenancePriorityAndType MaintenancePriority _priority; MaintenanceOperation::Type _type; public: - MaintenancePriorityAndType(MaintenancePriority pri, - MaintenanceOperation::Type type) + constexpr MaintenancePriorityAndType(MaintenancePriority pri, + MaintenanceOperation::Type type) noexcept : _priority(pri), _type(type) {} - const MaintenancePriority& getPriority() const { + constexpr MaintenancePriority getPriority() const noexcept { return _priority; } - MaintenanceOperation::Type getType() const { + constexpr MaintenanceOperation::Type getType() const noexcept { return _type; } - bool requiresMaintenance() const { + constexpr bool requiresMaintenance() const noexcept { return (_priority.getPriority() != MaintenancePriority::NO_MAINTENANCE_NEEDED); } diff --git a/storage/src/vespa/storage/distributor/maintenance/maintenancescanner.h b/storage/src/vespa/storage/distributor/maintenance/maintenancescanner.h index 3d5280e3b5a..4fec2e57cbc 100644 --- a/storage/src/vespa/storage/distributor/maintenance/maintenancescanner.h +++ b/storage/src/vespa/storage/distributor/maintenance/maintenancescanner.h @@ -17,9 +17,9 @@ public: BucketDatabase::Entry _entry; public: - bool isDone() const { return _done; } - document::BucketSpace getBucketSpace() const { return _bucketSpace; } - const BucketDatabase::Entry& getEntry() const { return _entry; } + [[nodiscard]] bool isDone() const noexcept { return _done; } + document::BucketSpace getBucketSpace() const noexcept { return _bucketSpace; } + const BucketDatabase::Entry& getEntry() const noexcept { return _entry; } static ScanResult createDone() { return ScanResult(true); } static ScanResult createNotDone(document::BucketSpace bucketSpace, BucketDatabase::Entry entry) { @@ -27,7 +27,7 @@ public: } private: - ScanResult(bool done) : _done(done), _bucketSpace(document::BucketSpace::invalid()), _entry() {} + explicit ScanResult(bool done) : _done(done), _bucketSpace(document::BucketSpace::invalid()), _entry() {} ScanResult(document::BucketSpace bucketSpace, const BucketDatabase::Entry& e) : _done(false), _bucketSpace(bucketSpace), _entry(e) {} }; diff --git a/storage/src/vespa/storage/distributor/maintenance/prioritizedbucket.h b/storage/src/vespa/storage/distributor/maintenance/prioritizedbucket.h index 77f117803ec..e1660df7cdb 100644 --- a/storage/src/vespa/storage/distributor/maintenance/prioritizedbucket.h +++ b/storage/src/vespa/storage/distributor/maintenance/prioritizedbucket.h @@ -14,24 +14,24 @@ public: static const PrioritizedBucket INVALID; - PrioritizedBucket() + PrioritizedBucket() noexcept : _bucket(), _priority(MaintenancePriority::NO_MAINTENANCE_NEEDED) {} - PrioritizedBucket(const document::Bucket &bucket, Priority pri) + PrioritizedBucket(const document::Bucket &bucket, Priority pri) noexcept : _bucket(bucket), _priority(pri) { } - document::Bucket getBucket() const { return _bucket; } + document::Bucket getBucket() const noexcept { return _bucket; } - Priority getPriority() const { + Priority getPriority() const noexcept { return _priority; } - bool valid() const { + [[nodiscard]] bool valid() const noexcept { return _bucket.getBucketId().getRawId() != 0; } @@ -41,19 +41,19 @@ public: MaintenancePriority::toString(_priority)); } - bool operator==(const PrioritizedBucket& other) const { + bool operator==(const PrioritizedBucket& other) const noexcept { return _bucket == other._bucket && _priority == other._priority; } - bool requiresMaintenance() const { + [[nodiscard]] bool requiresMaintenance() const noexcept { return _priority != MaintenancePriority::NO_MAINTENANCE_NEEDED; } - bool moreImportantThan(const PrioritizedBucket& other) const { + [[nodiscard]] bool moreImportantThan(const PrioritizedBucket& other) const noexcept { return _priority > other._priority; } - bool moreImportantThan(Priority otherPri) const { + [[nodiscard]] bool moreImportantThan(Priority otherPri) const noexcept { return _priority > otherPri; } diff --git a/storage/src/vespa/storage/distributor/maintenance/simplebucketprioritydatabase.cpp b/storage/src/vespa/storage/distributor/maintenance/simplebucketprioritydatabase.cpp index e267626fd7f..d5104aefea7 100644 --- a/storage/src/vespa/storage/distributor/maintenance/simplebucketprioritydatabase.cpp +++ b/storage/src/vespa/storage/distributor/maintenance/simplebucketprioritydatabase.cpp @@ -42,7 +42,7 @@ SimpleBucketPriorityDatabase::setPriority(const PrioritizedBucket& bucket) } void -SimpleBucketPriorityDatabase::PriFifoMappingConstIteratorImpl::increment() +SimpleBucketPriorityDatabase::PriFifoMappingConstIteratorImpl::increment() noexcept { if (_pri_fifo_iter != _pri_fifo_end) { ++_pri_fifo_iter; @@ -50,14 +50,14 @@ SimpleBucketPriorityDatabase::PriFifoMappingConstIteratorImpl::increment() } bool -SimpleBucketPriorityDatabase::PriFifoMappingConstIteratorImpl::equal(const ConstIteratorImpl& other) const +SimpleBucketPriorityDatabase::PriFifoMappingConstIteratorImpl::equal(const ConstIteratorImpl& other) const noexcept { auto& typed_other = dynamic_cast<const PriFifoMappingConstIteratorImpl&>(other); return (_pri_fifo_iter == typed_other._pri_fifo_iter); } PrioritizedBucket -SimpleBucketPriorityDatabase::PriFifoMappingConstIteratorImpl::dereference() const +SimpleBucketPriorityDatabase::PriFifoMappingConstIteratorImpl::dereference() const noexcept { assert(_pri_fifo_iter != _pri_fifo_end); return {_pri_fifo_iter->second, _pri_fifo_iter->first._pri}; diff --git a/storage/src/vespa/storage/distributor/maintenance/simplebucketprioritydatabase.h b/storage/src/vespa/storage/distributor/maintenance/simplebucketprioritydatabase.h index aa2605e94ca..50bcadf4e18 100644 --- a/storage/src/vespa/storage/distributor/maintenance/simplebucketprioritydatabase.h +++ b/storage/src/vespa/storage/distributor/maintenance/simplebucketprioritydatabase.h @@ -50,15 +50,15 @@ private: PriFifoBucketMap::const_iterator _pri_fifo_end; public: PriFifoMappingConstIteratorImpl(PriFifoBucketMap::const_iterator pri_fifo_iter, - PriFifoBucketMap::const_iterator pri_fifo_end) + PriFifoBucketMap::const_iterator pri_fifo_end) noexcept : _pri_fifo_iter(pri_fifo_iter), _pri_fifo_end(pri_fifo_end) {} ~PriFifoMappingConstIteratorImpl() override = default; - void increment() override; - bool equal(const ConstIteratorImpl& other) const override; - PrioritizedBucket dereference() const override; + void increment() noexcept override; + bool equal(const ConstIteratorImpl& other) const noexcept override; + PrioritizedBucket dereference() const noexcept override; }; void clearAllEntriesForBucket(const document::Bucket &bucket); diff --git a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h index a66cae11477..a867d4a5267 100644 --- a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h +++ b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h @@ -33,12 +33,12 @@ public: void merge(const PendingMaintenanceStats& rhs); }; private: - BucketPriorityDatabase& _bucketPriorityDb; - const MaintenancePriorityGenerator& _priorityGenerator; - const DistributorBucketSpaceRepo& _bucketSpaceRepo; + BucketPriorityDatabase& _bucketPriorityDb; + const MaintenancePriorityGenerator& _priorityGenerator; + const DistributorBucketSpaceRepo& _bucketSpaceRepo; DistributorBucketSpaceRepo::BucketSpaceMap::const_iterator _bucketSpaceItr; - document::BucketId _bucketCursor; - PendingMaintenanceStats _pendingMaintenance; + document::BucketId _bucketCursor; + PendingMaintenanceStats _pendingMaintenance; void countBucket(document::BucketSpace bucketSpace, const BucketInfo &info); public: @@ -55,7 +55,7 @@ public: // TODO: move out into own interface! void prioritizeBucket(const document::Bucket &id); - const PendingMaintenanceStats& getPendingMaintenanceStats() const { + const PendingMaintenanceStats& getPendingMaintenanceStats() const noexcept { return _pendingMaintenance; } }; diff --git a/storage/src/vespa/storage/distributor/messagetracker.h b/storage/src/vespa/storage/distributor/messagetracker.h index d3d6899a854..73e2461eb7a 100644 --- a/storage/src/vespa/storage/distributor/messagetracker.h +++ b/storage/src/vespa/storage/distributor/messagetracker.h @@ -25,11 +25,11 @@ public: uint16_t _target; }; - MessageTracker(const ClusterContext &cluster_context); - MessageTracker(MessageTracker &&) = default; - MessageTracker & operator = (MessageTracker &&) = delete; + MessageTracker(const ClusterContext& cluster_context); + MessageTracker(MessageTracker&&) = default; + MessageTracker& operator=(MessageTracker&&) = delete; MessageTracker(const MessageTracker &) = delete; - MessageTracker & operator = (const MessageTracker &) = delete; + MessageTracker& operator=(const MessageTracker&) = delete; ~MessageTracker(); void queueCommand(std::shared_ptr<api::BucketCommand> msg, uint16_t target) { @@ -49,11 +49,10 @@ public: bool finished(); protected: - std::vector<ToSend> _commandQueue; - + std::vector<ToSend> _commandQueue; // Keeps track of which node a message was sent to. std::map<uint64_t, uint16_t> _sentMessages; - const ClusterContext &_cluster_ctx; + const ClusterContext& _cluster_ctx; }; } diff --git a/storage/src/vespa/storage/distributor/operationowner.cpp b/storage/src/vespa/storage/distributor/operationowner.cpp index c118209353b..81512393c5b 100644 --- a/storage/src/vespa/storage/distributor/operationowner.cpp +++ b/storage/src/vespa/storage/distributor/operationowner.cpp @@ -11,9 +11,7 @@ LOG_SETUP(".operationowner"); namespace storage::distributor { -OperationOwner::~OperationOwner() -{ -} +OperationOwner::~OperationOwner() = default; void OperationOwner::Sender::sendCommand(const std::shared_ptr<api::StorageCommand> & msg) diff --git a/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp index da53d48d461..41260115297 100644 --- a/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp @@ -18,7 +18,7 @@ using document::BucketSpace; namespace storage::distributor { -GetOperation::GroupId::GroupId(const document::BucketId& id, uint32_t checksum, int node) +GetOperation::GroupId::GroupId(const document::BucketId& id, uint32_t checksum, int node) noexcept : _id(id), _checksum(checksum), _node(node) @@ -26,7 +26,7 @@ GetOperation::GroupId::GroupId(const document::BucketId& id, uint32_t checksum, } bool -GetOperation::GroupId::operator<(const GroupId& other) const +GetOperation::GroupId::operator<(const GroupId& other) const noexcept { if (_id.getRawId() != other._id.getRawId()) { return (_id.getRawId() < other._id.getRawId()); @@ -41,7 +41,7 @@ GetOperation::GroupId::operator<(const GroupId& other) const } bool -GetOperation::GroupId::operator==(const GroupId& other) const +GetOperation::GroupId::operator==(const GroupId& other) const noexcept { return (_id == other._id && _checksum == other._checksum @@ -279,7 +279,7 @@ GetOperation::assignTargetNodeGroups(const BucketDatabase::ReadGuard& read_guard } bool -GetOperation::all_bucket_metadata_initially_consistent() const +GetOperation::all_bucket_metadata_initially_consistent() const noexcept { // TODO rename, calling this "responses" is confusing as it's populated before sending anything. return _responses.size() == 1; diff --git a/storage/src/vespa/storage/distributor/operations/external/getoperation.h b/storage/src/vespa/storage/distributor/operations/external/getoperation.h index 61443100695..96b5d970904 100644 --- a/storage/src/vespa/storage/distributor/operations/external/getoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/getoperation.h @@ -12,16 +12,13 @@ namespace document { class Document; } -namespace storage { +namespace storage::api { class GetCommand; } -namespace api { class GetCommand; } - -class PersistenceOperationMetricSet; - -namespace distributor { +namespace storage::distributor { class DistributorNodeContext; class DistributorBucketSpace; +class PersistenceOperationMetricSet; class GetOperation : public Operation { @@ -36,11 +33,11 @@ public: void onClose(DistributorStripeMessageSender& sender) override; void onStart(DistributorStripeMessageSender& sender) override; void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> & msg) override; - const char* getName() const override { return "get"; } + const char* getName() const noexcept override { return "get"; } std::string getStatus() const override { return ""; } - bool all_bucket_metadata_initially_consistent() const; - bool any_replicas_failed() const noexcept { + [[nodiscard]] bool all_bucket_metadata_initially_consistent() const noexcept; + [[nodiscard]] bool any_replicas_failed() const noexcept { return _any_replicas_failed; } @@ -66,12 +63,12 @@ private: class GroupId { public: // Node should be set only if bucket is incomplete - GroupId(const document::BucketId& id, uint32_t checksum, int node); + GroupId(const document::BucketId& id, uint32_t checksum, int node) noexcept; - bool operator<(const GroupId& other) const; - bool operator==(const GroupId& other) const; - const document::BucketId& getBucketId() const { return _id; } - int getNode() const { return _node; } + bool operator<(const GroupId& other) const noexcept; + bool operator==(const GroupId& other) const noexcept; + const document::BucketId& getBucketId() const noexcept { return _id; } + int getNode() const noexcept { return _node; } private: document::BucketId _id; uint32_t _checksum; @@ -129,4 +126,3 @@ private: }; } -} diff --git a/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp index 74ff8371fbe..dea215f47dc 100644 --- a/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp @@ -24,7 +24,7 @@ using document::BucketSpace; PutOperation::PutOperation(const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, - DistributorBucketSpace &bucketSpace, + DistributorBucketSpace& bucketSpace, std::shared_ptr<api::PutCommand> msg, PersistenceOperationMetricSet& metric, SequencingHandle sequencingHandle) : SequencedOperation(std::move(sequencingHandle)), @@ -222,7 +222,7 @@ PutOperation::shouldImplicitlyActivateReplica(const OperationTargetList& targets } void -PutOperation::onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> & msg) +PutOperation::onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply>& msg) { LOG(debug, "Received %s", msg->toString(true).c_str()); _tracker.receiveReply(sender, static_cast<api::BucketInfoReply&>(*msg)); diff --git a/storage/src/vespa/storage/distributor/operations/external/putoperation.h b/storage/src/vespa/storage/distributor/operations/external/putoperation.h index 7ec81d6570d..9801fed0c99 100644 --- a/storage/src/vespa/storage/distributor/operations/external/putoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/putoperation.h @@ -5,16 +5,15 @@ #include <vespa/storage/distributor/operations/sequenced_operation.h> #include <vespa/storage/distributor/persistencemessagetracker.h> -namespace document { - class Document; -} -namespace storage::lib { - class Distribution; -} +namespace document { class Document; } + +namespace storage::lib { class Distribution; } + namespace storage::api { - class CreateBucketReply; - class PutCommand; +class CreateBucketReply; +class PutCommand; } + namespace storage::distributor { class DistributorBucketSpace; @@ -25,16 +24,16 @@ class PutOperation : public SequencedOperation public: PutOperation(const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, - DistributorBucketSpace &bucketSpace, + DistributorBucketSpace& bucketSpace, std::shared_ptr<api::PutCommand> msg, PersistenceOperationMetricSet& metric, SequencingHandle sequencingHandle = SequencingHandle()); ~PutOperation() override; void onStart(DistributorStripeMessageSender& sender) override; - const char* getName() const override { return "put"; }; + const char* getName() const noexcept override { return "put"; }; std::string getStatus() const override { return ""; }; - void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> &) override; + void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply>&) override; void onClose(DistributorStripeMessageSender& sender) override; private: diff --git a/storage/src/vespa/storage/distributor/operations/external/read_for_write_visitor_operation.h b/storage/src/vespa/storage/distributor/operations/external/read_for_write_visitor_operation.h index dac9ed49bd3..f53342a9286 100644 --- a/storage/src/vespa/storage/distributor/operations/external/read_for_write_visitor_operation.h +++ b/storage/src/vespa/storage/distributor/operations/external/read_for_write_visitor_operation.h @@ -41,7 +41,7 @@ public: UuidGenerator& uuid_generator); ~ReadForWriteVisitorOperationStarter() override; - const char* getName() const override { return "ReadForWriteVisitorOperationStarter"; } + const char* getName() const noexcept override { return "ReadForWriteVisitorOperationStarter"; } void onClose(DistributorStripeMessageSender& sender) override; void onStart(DistributorStripeMessageSender& sender) override; void onReceive(DistributorStripeMessageSender& sender, diff --git a/storage/src/vespa/storage/distributor/operations/external/removelocationoperation.h b/storage/src/vespa/storage/distributor/operations/external/removelocationoperation.h index 1f951904ea1..d177676ff03 100644 --- a/storage/src/vespa/storage/distributor/operations/external/removelocationoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/removelocationoperation.h @@ -4,11 +4,9 @@ #include <vespa/storage/distributor/operations/operation.h> #include <vespa/storage/distributor/persistencemessagetracker.h> -namespace storage { +namespace storage::api { class RemoveLocationCommand; } -namespace api { class RemoveLocationCommand; } - -namespace distributor { +namespace storage::distributor { class DistributorBucketSpace; @@ -18,7 +16,7 @@ public: RemoveLocationOperation(const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, const DocumentSelectionParser& parser, - DistributorBucketSpace &bucketSpace, + DistributorBucketSpace& bucketSpace, std::shared_ptr<api::RemoveLocationCommand> msg, PersistenceOperationMetricSet& metric); ~RemoveLocationOperation() override; @@ -29,9 +27,9 @@ public: const api::RemoveLocationCommand& cmd, document::BucketId& id); void onStart(DistributorStripeMessageSender& sender) override; - const char* getName() const override { return "removelocation"; }; + const char* getName() const noexcept override { return "removelocation"; }; std::string getStatus() const override { return ""; }; - void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> &) override; + void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply>&) override; void onClose(DistributorStripeMessageSender& sender) override; private: PersistenceMessageTrackerImpl _trackerInstance; @@ -45,5 +43,3 @@ private: }; } - -} diff --git a/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp index 7beb95451e9..584ad9de2ce 100644 --- a/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp @@ -14,7 +14,7 @@ using document::BucketSpace; RemoveOperation::RemoveOperation(const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, - DistributorBucketSpace &bucketSpace, + DistributorBucketSpace& bucketSpace, std::shared_ptr<api::RemoveCommand> msg, PersistenceOperationMetricSet& metric, SequencingHandle sequencingHandle) @@ -81,7 +81,7 @@ RemoveOperation::onStart(DistributorStripeMessageSender& sender) void RemoveOperation::onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> & msg) { - api::RemoveReply& reply(static_cast<api::RemoveReply&>(*msg)); + auto& reply = static_cast<api::RemoveReply&>(*msg); if (_tracker.getReply().get()) { api::RemoveReply& replyToSend = diff --git a/storage/src/vespa/storage/distributor/operations/external/removeoperation.h b/storage/src/vespa/storage/distributor/operations/external/removeoperation.h index 53de79ac9b3..360ab332f96 100644 --- a/storage/src/vespa/storage/distributor/operations/external/removeoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/removeoperation.h @@ -4,42 +4,36 @@ #include <vespa/storage/distributor/operations/sequenced_operation.h> #include <vespa/storage/distributor/persistencemessagetracker.h> -namespace storage { +namespace storage::api { class RemoveCommand; } -namespace api { class RemoveCommand; } - -namespace distributor { +namespace storage::distributor { class DistributorBucketSpace; -class RemoveOperation : public SequencedOperation +class RemoveOperation : public SequencedOperation { public: RemoveOperation(const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, - DistributorBucketSpace &bucketSpace, + DistributorBucketSpace& bucketSpace, std::shared_ptr<api::RemoveCommand> msg, PersistenceOperationMetricSet& metric, SequencingHandle sequencingHandle = SequencingHandle()); ~RemoveOperation() override; void onStart(DistributorStripeMessageSender& sender) override; - const char* getName() const override { return "remove"; }; + const char* getName() const noexcept override { return "remove"; }; std::string getStatus() const override { return ""; }; void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> &) override; void onClose(DistributorStripeMessageSender& sender) override; private: - PersistenceMessageTrackerImpl _trackerInstance; - PersistenceMessageTracker& _tracker; - + PersistenceMessageTrackerImpl _trackerInstance; + PersistenceMessageTracker& _tracker; std::shared_ptr<api::RemoveCommand> _msg; - - const DistributorNodeContext& _node_ctx; - DistributorBucketSpace &_bucketSpace; + const DistributorNodeContext& _node_ctx; + DistributorBucketSpace& _bucketSpace; }; } - -} diff --git a/storage/src/vespa/storage/distributor/operations/external/statbucketlistoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/statbucketlistoperation.cpp index 8dd291c19bb..6b2550ab547 100644 --- a/storage/src/vespa/storage/distributor/operations/external/statbucketlistoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/statbucketlistoperation.cpp @@ -19,6 +19,8 @@ StatBucketListOperation::StatBucketListOperation( { } +StatBucketListOperation::~StatBucketListOperation() = default; + void StatBucketListOperation::getBucketStatus(const BucketDatabase::Entry& entry, std::ostream& ost) const @@ -42,20 +44,18 @@ StatBucketListOperation::getBucketStatus(const BucketDatabase::Entry& entry, void StatBucketListOperation::onStart(DistributorStripeMessageSender& sender) { - api::GetBucketListReply::SP reply(new api::GetBucketListReply(*_command)); + auto reply = std::make_shared<api::GetBucketListReply>(*_command); std::vector<BucketDatabase::Entry> entries; _bucketDb.getAll(_command->getBucketId(), entries); - for (uint32_t i = 0; i < entries.size(); i++) { + for (const auto& entry : entries) { std::ostringstream ost; ost << "[distributor:" << _distributorIndex << "] "; - getBucketStatus(entries[i], ost); + getBucketStatus(entry, ost); - reply->getBuckets().push_back(api::GetBucketListReply::BucketInfo( - entries[i].getBucketId(), - ost.str())); + reply->getBuckets().emplace_back(entry.getBucketId(), ost.str()); } sender.sendReply(reply); } diff --git a/storage/src/vespa/storage/distributor/operations/external/statbucketlistoperation.h b/storage/src/vespa/storage/distributor/operations/external/statbucketlistoperation.h index 9dcab1ee0f6..fe88ec50749 100644 --- a/storage/src/vespa/storage/distributor/operations/external/statbucketlistoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/statbucketlistoperation.h @@ -5,11 +5,9 @@ #include <vespa/storage/distributor/operations/operation.h> #include <vespa/storage/bucketdb/bucketdatabase.h> -namespace storage { +namespace storage::api { class GetBucketListCommand; } -namespace api { class GetBucketListCommand; } - -namespace distributor { +namespace storage::distributor { class MaintenanceOperationGenerator; @@ -21,9 +19,9 @@ public: const MaintenanceOperationGenerator& generator, uint16_t distributorIndex, const std::shared_ptr<api::GetBucketListCommand>& cmd); - ~StatBucketListOperation() {} + ~StatBucketListOperation() override; - const char* getName() const override { return "statBucketList"; } + const char* getName() const noexcept override { return "statBucketList"; } std::string getStatus() const override { return ""; } void onStart(DistributorStripeMessageSender& sender) override; @@ -43,5 +41,4 @@ private: std::shared_ptr<api::GetBucketListCommand> _command; }; -} // distributor -} // storage +} // storage::distributor diff --git a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp index 20cd8c67c29..85f36e7a963 100644 --- a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp @@ -42,27 +42,19 @@ StatBucketOperation::onStart(DistributorStripeMessageSender& sender) // If no entries exist, give empty reply if (nodes.size() == 0) { - api::StatBucketReply::SP reply(new api::StatBucketReply(*_command, "Bucket was not stored on any nodes.")); + auto reply = std::make_shared<api::StatBucketReply>(*_command, "Bucket was not stored on any nodes."); reply->setResult(api::ReturnCode(api::ReturnCode::OK)); sender.sendReply(reply); } else { - std::vector<std::shared_ptr<api::StorageCommand> > messages; - for (uint32_t i = 0; i < nodes.size(); i++) { - std::shared_ptr<api::StatBucketCommand> cmd( - new api::StatBucketCommand( - _command->getBucket(), - _command->getDocumentSelection())); - - messages.push_back(cmd); - _sent[cmd->getMsgId()] = nodes[i]; + std::vector<std::shared_ptr<api::StorageCommand>> messages; + for (uint16_t node : nodes) { + auto cmd = std::make_shared<api::StatBucketCommand>(_command->getBucket(), _command->getDocumentSelection()); + _sent[cmd->getMsgId()] = node; + messages.emplace_back(std::move(cmd)); } for (uint32_t i = 0; i < nodes.size(); i++) { - sender.sendToNode( - lib::NodeType::STORAGE, - nodes[i], - messages[i], - true); + sender.sendToNode(lib::NodeType::STORAGE, nodes[i], messages[i], true); } } }; @@ -71,9 +63,8 @@ void StatBucketOperation::onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> & msg) { assert(msg->getType() == api::MessageType::STATBUCKET_REPLY); - api::StatBucketReply& myreply(dynamic_cast<api::StatBucketReply&>(*msg)); - - std::map<uint64_t, uint16_t>::iterator found = _sent.find(msg->getMsgId()); + auto& myreply = dynamic_cast<api::StatBucketReply&>(*msg); + auto found = _sent.find(msg->getMsgId()); if (found != _sent.end()) { std::ostringstream ost; @@ -83,19 +74,15 @@ StatBucketOperation::onReceive(DistributorStripeMessageSender& sender, const std ost << "\tBucket information retrieval failed on node " << found->second << ": " << myreply.getResult() << "\n\n"; } _results[found->second] = ost.str(); - _sent.erase(found); } if (_sent.empty()) { std::ostringstream ost; - for (std::map<uint16_t, std::string>::iterator iter = _results.begin(); - iter != _results.end(); - iter++) { - ost << iter->second; + for (const auto& result : _results) { + ost << result.second; } - - api::StatBucketReply::SP reply(new api::StatBucketReply(*_command, ost.str())); + auto reply = std::make_shared<api::StatBucketReply>(*_command, ost.str()); sender.sendReply(reply); } } diff --git a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.h b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.h index 44215d2c68f..13850e642ad 100644 --- a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.h @@ -21,20 +21,18 @@ class StatBucketOperation : public Operation public: StatBucketOperation(DistributorBucketSpace &bucketSpace, const std::shared_ptr<api::StatBucketCommand> & cmd); - ~StatBucketOperation(); + ~StatBucketOperation() override; - const char* getName() const override { return "statBucket"; } + const char* getName() const noexcept override { return "statBucket"; } std::string getStatus() const override { return ""; } void onClose(DistributorStripeMessageSender& sender) override; void onStart(DistributorStripeMessageSender& sender) override; void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> & msg) override; private: - DistributorBucketSpace &_bucketSpace; - + DistributorBucketSpace& _bucketSpace; std::shared_ptr<api::StatBucketCommand> _command; - - std::map<uint64_t, uint16_t> _sent; - std::map<uint16_t, std::string> _results; + std::map<uint64_t, uint16_t> _sent; + std::map<uint16_t, std::string> _results; }; } diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp index 59f52465b93..2acd6068e1a 100644 --- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp @@ -22,13 +22,11 @@ using document::BucketSpace; namespace storage::distributor { - - TwoPhaseUpdateOperation::TwoPhaseUpdateOperation( const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, const DocumentSelectionParser& parser, - DistributorBucketSpace &bucketSpace, + DistributorBucketSpace& bucketSpace, std::shared_ptr<api::UpdateCommand> msg, DistributorMetricSet& metrics, SequencingHandle sequencingHandle) @@ -110,12 +108,13 @@ IntermediateMessageSender::IntermediateMessageSender(SentMessageMap& mm, callback(std::move(cb)), forward(fwd) { } + IntermediateMessageSender::~IntermediateMessageSender() = default; } const char* -TwoPhaseUpdateOperation::stateToString(SendState state) +TwoPhaseUpdateOperation::stateToString(SendState state) noexcept { switch (state) { case SendState::NONE_SENT: return "NONE_SENT"; diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h index 45dec86268f..7c1b6b3eed6 100644 --- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h @@ -8,25 +8,20 @@ #include <set> #include <optional> -namespace document { -class Document; -} - -namespace storage { +namespace document { class Document; } -namespace api { +namespace storage::api { class UpdateCommand; class UpdateReply; class CreateBucketReply; class ReturnCode; } -class UpdateMetricSet; - -namespace distributor { +namespace storage::distributor { class DistributorBucketSpace; class GetOperation; +class UpdateMetricSet; /* * General functional outline: @@ -59,7 +54,7 @@ public: TwoPhaseUpdateOperation(const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, const DocumentSelectionParser& parser, - DistributorBucketSpace &bucketSpace, + DistributorBucketSpace& bucketSpace, std::shared_ptr<api::UpdateCommand> msg, DistributorMetricSet& metrics, SequencingHandle sequencingHandle = SequencingHandle()); @@ -67,7 +62,7 @@ public: void onStart(DistributorStripeMessageSender& sender) override; - const char* getName() const override { return "twophaseupdate"; } + const char* getName() const noexcept override { return "twophaseupdate"; } std::string getStatus() const override { return ""; } @@ -92,7 +87,7 @@ private: }; void transitionTo(SendState newState); - static const char* stateToString(SendState); + static const char* stateToString(SendState) noexcept; void sendReply(DistributorStripeMessageSender&, std::shared_ptr<api::UpdateReply>); @@ -163,7 +158,3 @@ private: }; } - -} - - diff --git a/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp index 5e3fe161f92..f3ce71b08d6 100644 --- a/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp @@ -41,6 +41,8 @@ UpdateOperation::UpdateOperation(const DistributorNodeContext& node_ctx, { } +UpdateOperation::~UpdateOperation() = default; + bool UpdateOperation::anyStorageNodesAvailable() const { diff --git a/storage/src/vespa/storage/distributor/operations/external/updateoperation.h b/storage/src/vespa/storage/distributor/operations/external/updateoperation.h index 7f3fef1260a..96fd878a324 100644 --- a/storage/src/vespa/storage/distributor/operations/external/updateoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/updateoperation.h @@ -3,22 +3,17 @@ #include <vespa/storage/distributor/persistencemessagetracker.h> -namespace document { -class Document; -} - -namespace storage { +namespace document { class Document; } -namespace api { +namespace storage::api { class UpdateCommand; class CreateBucketReply; } -class UpdateMetricSet; - -namespace distributor { +namespace storage::distributor { class DistributorBucketSpace; +class UpdateMetricSet; class UpdateOperation : public Operation { @@ -29,9 +24,10 @@ public: const std::shared_ptr<api::UpdateCommand>& msg, std::vector<BucketDatabase::Entry> entries, UpdateMetricSet& metric); + ~UpdateOperation() override; void onStart(DistributorStripeMessageSender& sender) override; - const char* getName() const override { return "update"; }; + const char* getName() const noexcept override { return "update"; }; std::string getStatus() const override { return ""; }; void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> & msg) override; void onClose(DistributorStripeMessageSender& sender) override; @@ -77,5 +73,3 @@ private: }; } - -} diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp index 5abaad6ef9f..4d40e93477f 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp @@ -35,6 +35,8 @@ VisitorOperation::BucketInfo::print(vespalib::asciistream & out) const out << ")"; } +VisitorOperation::BucketInfo::~BucketInfo() = default; + vespalib::string VisitorOperation::BucketInfo::toString() const { diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h index ce10ea87c11..7f278c0383f 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h @@ -13,7 +13,6 @@ namespace document { class Document; } -namespace storage { struct VisitorMetricSet; } namespace storage::lib { class ClusterState; } namespace storage::distributor { @@ -21,6 +20,7 @@ namespace storage::distributor { class DistributorBucketSpace; class DistributorNodeContext; class DistributorStripeOperationContext; +struct VisitorMetricSet; class VisitorOperation : public Operation { @@ -37,7 +37,7 @@ public: VisitorOperation(const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, - DistributorBucketSpace &bucketSpace, + DistributorBucketSpace& bucketSpace, const std::shared_ptr<api::CreateVisitorCommand>& msg, const Config& config, VisitorMetricSet& metrics); @@ -47,7 +47,7 @@ public: void onClose(DistributorStripeMessageSender& sender) override; void onStart(DistributorStripeMessageSender& sender) override; void onReceive(DistributorStripeMessageSender& sender, - const std::shared_ptr<api::StorageReply> & msg) override; + const std::shared_ptr<api::StorageReply>& msg) override; // Only valid to call if is_read_for_write() == true void fail_with_bucket_already_locked(DistributorStripeMessageSender& sender); @@ -55,7 +55,7 @@ public: [[nodiscard]] bool verify_command_and_expand_buckets(DistributorStripeMessageSender& sender); - const char* getName() const override { return "visit"; } + const char* getName() const noexcept override { return "visit"; } std::string getStatus() const override { return ""; } [[nodiscard]] bool has_sent_reply() const noexcept { return _sentReply; } @@ -72,6 +72,7 @@ private: std::vector<uint16_t> triedNodes; BucketInfo() : done(false), activeNode(-1), failedCount(0), triedNodes() { } + ~BucketInfo(); void print(vespalib::asciistream & out) const; vespalib::string toString() const; diff --git a/storage/src/vespa/storage/distributor/operations/external/visitororder.h b/storage/src/vespa/storage/distributor/operations/external/visitororder.h index 21dc8b1c6c6..90d6b2eaf07 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitororder.h +++ b/storage/src/vespa/storage/distributor/operations/external/visitororder.h @@ -7,9 +7,9 @@ namespace storage::distributor { struct VisitorOrder { - VisitorOrder() { } + VisitorOrder() = default; - bool operator()(const document::BucketId& a, const document::BucketId& b) { + bool operator()(const document::BucketId& a, const document::BucketId& b) noexcept { if (a == document::BucketId(INT_MAX) || b == document::BucketId(0, 0)) { return false; // All before max, non before null diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.h b/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.h index 25308b0fb4b..27dc519dcc2 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.h @@ -22,8 +22,8 @@ public: void onStart(DistributorStripeMessageSender& sender) override; void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> &) override; - const char* getName() const override { return "garbagecollection"; }; - Type getType() const override { return GARBAGE_COLLECTION; } + const char* getName() const noexcept override { return "garbagecollection"; }; + Type getType() const noexcept override { return GARBAGE_COLLECTION; } bool shouldBlockThisOperation(uint32_t, uint16_t, uint8_t) const override; bool is_two_phase() const noexcept { return ((_phase == Phase::ReadMetadataPhase) || (_phase == Phase::WriteRemovesPhase)); diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.cpp index 744b24b593e..a69b7739e07 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.cpp @@ -28,8 +28,8 @@ IdealStateOperation::IdealStateOperation(const BucketAndNodes& bucketAndNodes) : _manager(nullptr), _bucketSpace(nullptr), _bucketAndNodes(bucketAndNodes), - _ok(true), - _priority(255) + _priority(255), + _ok(true) { } @@ -111,7 +111,7 @@ IdealStateOperation::on_throttled() } uint32_t -IdealStateOperation::memorySize() const +IdealStateOperation::memorySize() const noexcept { return sizeof(*this) + _detailedReason.size(); } diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.h b/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.h index 7e62506b77f..6c52bdb738d 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.h @@ -55,23 +55,23 @@ public: @return Returns the target bucket. */ - document::BucketId getBucketId() const { return _bucket.getBucketId(); } + document::BucketId getBucketId() const noexcept { return _bucket.getBucketId(); } - document::Bucket getBucket() const { return _bucket; } + document::Bucket getBucket() const noexcept { return _bucket; } /** Returns the target nodes @return the target nodes */ - std::vector<uint16_t>& getNodes() { return _nodes; } + std::vector<uint16_t>& getNodes() noexcept { return _nodes; } /** Returns the target nodes @return the target nodes */ - const std::vector<uint16_t>& getNodes() const { return _nodes; } + const std::vector<uint16_t>& getNodes() const noexcept { return _nodes; } /** Returns a string representation of this object. @@ -105,14 +105,14 @@ class IdealStateOperation : public MaintenanceOperation public: static const uint32_t MAINTENANCE_MESSAGE_TYPES[]; - typedef std::shared_ptr<IdealStateOperation> SP; - typedef std::unique_ptr<IdealStateOperation> UP; - typedef std::vector<SP> Vector; - typedef std::map<document::BucketId, SP> Map; + using SP = std::shared_ptr<IdealStateOperation>; + using UP = std::unique_ptr<IdealStateOperation>; + using Vector = std::vector<SP>; + using Map = std::map<document::BucketId, SP>; IdealStateOperation(const BucketAndNodes& bucketAndNodes); - virtual ~IdealStateOperation(); + ~IdealStateOperation() override; void onClose(DistributorStripeMessageSender&) override {} @@ -121,37 +121,37 @@ public: @return Returns the status of the operation. */ - virtual bool ok() { return _ok; } + [[nodiscard]] bool ok() const noexcept { return _ok; } /** Returns the target nodes of the operation. @return The target nodes */ - std::vector<uint16_t>& getNodes() { return _bucketAndNodes.getNodes(); } + std::vector<uint16_t>& getNodes() noexcept { return _bucketAndNodes.getNodes(); } /** Returns the target nodes of the operation. @return The target nodes */ - const std::vector<uint16_t>& getNodes() const { return _bucketAndNodes.getNodes(); } + const std::vector<uint16_t>& getNodes() const noexcept { return _bucketAndNodes.getNodes(); } /** Returns the target bucket of the operation. @return The target bucket. */ - document::BucketId getBucketId() const { return _bucketAndNodes.getBucketId(); } + document::BucketId getBucketId() const noexcept { return _bucketAndNodes.getBucketId(); } - document::Bucket getBucket() const { return _bucketAndNodes.getBucket(); } + document::Bucket getBucket() const noexcept { return _bucketAndNodes.getBucket(); } /** Returns the target of the operation. @return The target bucket and nodes */ - const BucketAndNodes& getBucketAndNodes() const { return _bucketAndNodes; } + const BucketAndNodes& getBucketAndNodes() const noexcept { return _bucketAndNodes; } /** Called by the operation when it is finished. Must be called, otherwise the active @@ -174,7 +174,7 @@ public: /** Returns the type of operation this is. */ - virtual Type getType() const = 0; + virtual Type getType() const noexcept = 0; /** Set the priority we should send messages from this operation with. @@ -192,7 +192,7 @@ public: /** Returns the priority we should send messages with. */ - api::StorageMessage::Priority getPriority() { return _priority; } + api::StorageMessage::Priority getPriority() const noexcept { return _priority; } void setDetailedReason(const std::string& detailedReason) { _detailedReason = detailedReason; @@ -205,7 +205,7 @@ public: return _detailedReason; } - uint32_t memorySize() const; + uint32_t memorySize() const noexcept; /** * Sets the various metadata for the given command that @@ -224,13 +224,12 @@ protected: friend struct IdealStateManagerTest; friend class IdealStateManager; - IdealStateManager* _manager; - DistributorBucketSpace *_bucketSpace; - BucketAndNodes _bucketAndNodes; - std::string _detailedReason; - - bool _ok; + IdealStateManager* _manager; + DistributorBucketSpace* _bucketSpace; + BucketAndNodes _bucketAndNodes; + std::string _detailedReason; api::StorageMessage::Priority _priority; + bool _ok; /** * Checks if the given bucket is blocked by any pending messages to any diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/joinoperation.h b/storage/src/vespa/storage/distributor/operations/idealstate/joinoperation.h index d8f77d89b8a..88fb3010654 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/joinoperation.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/joinoperation.h @@ -27,11 +27,11 @@ public: void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply>&) override; - const char* getName() const override { + const char* getName() const noexcept override { return "join"; } - Type getType() const override { + Type getType() const noexcept override { return JOIN_BUCKET; } diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/mergelimiter.h b/storage/src/vespa/storage/distributor/operations/idealstate/mergelimiter.h index 6558aaf7f04..c139109ff0c 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/mergelimiter.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/mergelimiter.h @@ -11,9 +11,9 @@ class MergeLimiter { uint16_t _maxNodes; public: - typedef std::vector<MergeMetaData> NodeArray; + using NodeArray = std::vector<MergeMetaData>; - MergeLimiter(uint16_t maxNodes); + explicit MergeLimiter(uint16_t maxNodes); void limitMergeToMaxNodes(NodeArray&); }; diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.cpp index c5487511296..09fec1a88fd 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.cpp @@ -3,14 +3,11 @@ #include "mergemetadata.h" #include <vespa/vespalib/stllike/asciistream.h> -namespace storage { -namespace distributor { +namespace storage::distributor { vespalib::asciistream& operator<<(vespalib::asciistream& out, const MergeMetaData& e) { return out << "MergeMetaData(" << e._nodeIndex << ")"; } -} // distributor -} // storage - +} // storage::distributor diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.h b/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.h index 446ab0407ca..07d1df4e81c 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.h @@ -17,13 +17,13 @@ struct MergeMetaData { MergeMetaData(uint16_t nodeIndex, const BucketCopy& copy) noexcept : _nodeIndex(nodeIndex), _sourceOnly(false), _copy(©) {} - bool trusted() const { + [[nodiscard]] bool trusted() const noexcept { return _copy->trusted(); } - uint32_t checksum() const { + [[nodiscard]] uint32_t checksum() const noexcept { return _copy->getChecksum(); } - bool source_only() const noexcept { return _sourceOnly; } + [[nodiscard]] bool source_only() const noexcept { return _sourceOnly; } }; vespalib::asciistream& operator<<(vespalib::asciistream& out, const MergeMetaData& e); diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.h b/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.h index 014bae842fa..5416df3a43d 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.h @@ -10,6 +10,7 @@ #include <vespa/storageapi/message/bucket.h> namespace storage::lib { class Distribution; } + namespace storage::distributor { class MergeBucketMetricSet; @@ -26,7 +27,6 @@ protected: MergeLimiter _limiter; public: - static const int LOAD = 10; MergeOperation(const BucketAndNodes& nodes, uint16_t maxNodes = 16) : IdealStateOperation(nodes), @@ -34,13 +34,13 @@ public: _limiter(maxNodes) {} - ~MergeOperation(); + ~MergeOperation() override; void onStart(DistributorStripeMessageSender& sender) override; void onReceive(DistributorStripeMessageSender& sender, const api::StorageReply::SP&) override; - const char* getName() const override { return "merge"; }; + const char* getName() const noexcept override { return "merge"; }; std::string getStatus() const override; - Type getType() const override { return MERGE_BUCKET; } + Type getType() const noexcept override { return MERGE_BUCKET; } /** Generates ordered list of nodes that should be included in the merge */ static void generateSortedNodeList( diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.cpp index 25cae5b9979..e46ccebffba 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.cpp @@ -9,7 +9,9 @@ LOG_SETUP(".distributor.operation.idealstate.remove"); -using namespace storage::distributor; +namespace storage::distributor { + +RemoveBucketOperation::~RemoveBucketOperation() = default; bool RemoveBucketOperation::onStartInternal(DistributorStripeMessageSender& sender) @@ -123,3 +125,4 @@ RemoveBucketOperation::shouldBlockThisOperation(uint32_t, uint16_t target_node, return false; } +} // storage::distributor diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.h b/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.h index 5e0922d5685..db6f5b997cc 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/removebucketoperation.h @@ -14,6 +14,7 @@ public: RemoveBucketOperation(const ClusterContext &cluster_context, const BucketAndNodes& nodes) : IdealStateOperation(nodes), _tracker(cluster_context) {} + ~RemoveBucketOperation() override; /** Sends messages, returns true if we are done (sent nothing). @@ -28,8 +29,8 @@ public: bool onReceiveInternal(const std::shared_ptr<api::StorageReply> &); void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> &) override; - const char* getName() const override { return "remove"; }; - Type getType() const override { return DELETE_BUCKET; } + const char* getName() const noexcept override { return "remove"; }; + Type getType() const noexcept override { return DELETE_BUCKET; } bool shouldBlockThisOperation(uint32_t, uint16_t, uint8_t) const override; protected: MessageTracker _tracker; diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/setbucketstateoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/setbucketstateoperation.cpp index 76fb6829609..00906d22ea4 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/setbucketstateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/setbucketstateoperation.cpp @@ -10,7 +10,7 @@ LOG_SETUP(".distributor.operation.idealstate.setactive"); namespace storage::distributor { -SetBucketStateOperation::SetBucketStateOperation(const ClusterContext &cluster_ctx, +SetBucketStateOperation::SetBucketStateOperation(const ClusterContext& cluster_ctx, const BucketAndNodes& nodes, const std::vector<uint16_t>& wantedActiveNodes) : IdealStateOperation(nodes), @@ -37,7 +37,9 @@ bool SetBucketStateOperation::shouldBeActive(uint16_t node) const { for (uint32_t i=0, n=_wantedActiveNodes.size(); i<n; ++i) { - if (_wantedActiveNodes[i] == node) return true; + if (_wantedActiveNodes[i] == node) { + return true; + } } return false; } @@ -73,8 +75,7 @@ void SetBucketStateOperation::onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply>& reply) { - api::SetBucketStateReply& rep( - static_cast<api::SetBucketStateReply&>(*reply)); + auto& rep = static_cast<api::SetBucketStateReply&>(*reply); const uint16_t node = _tracker.handleReply(rep); LOG(debug, "Got %s from node %u", reply->toString(true).c_str(), node); diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/setbucketstateoperation.h b/storage/src/vespa/storage/distributor/operations/idealstate/setbucketstateoperation.h index 18160ba4d84..76c9c704653 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/setbucketstateoperation.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/setbucketstateoperation.h @@ -16,8 +16,8 @@ public: void onStart(DistributorStripeMessageSender&) override; void onReceive(DistributorStripeMessageSender&, const std::shared_ptr<api::StorageReply>&) override; - const char* getName() const override { return "setbucketstate"; } - Type getType() const override { return SET_BUCKET_STATE; } + const char* getName() const noexcept override { return "setbucketstate"; } + Type getType() const noexcept override { return SET_BUCKET_STATE; } protected: MessageTracker _tracker; std::vector<uint16_t> _wantedActiveNodes; diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.cpp index 6f3924535ef..97d86528ea0 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.cpp @@ -11,7 +11,7 @@ LOG_SETUP(".distributor.operation.idealstate.split"); using namespace storage::distributor; -SplitOperation::SplitOperation(const ClusterContext &cluster_ctx, const BucketAndNodes& nodes, +SplitOperation::SplitOperation(const ClusterContext& cluster_ctx, const BucketAndNodes& nodes, uint32_t maxBits, uint32_t splitCount, uint32_t splitSize) : IdealStateOperation(nodes), _tracker(cluster_ctx), @@ -19,6 +19,7 @@ SplitOperation::SplitOperation(const ClusterContext &cluster_ctx, const BucketAn _splitCount(splitCount), _splitSize(splitSize) {} + SplitOperation::~SplitOperation() = default; void diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.h b/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.h index 6a268155fc8..604b29e296c 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/splitoperation.h @@ -12,14 +12,14 @@ public: SplitOperation(const ClusterContext& cluster_ctx, const BucketAndNodes& nodes, uint32_t maxBits, uint32_t splitCount, uint32_t splitSize); - SplitOperation(const SplitOperation &) = delete; - SplitOperation & operator = (const SplitOperation &) = delete; + SplitOperation(const SplitOperation&) = delete; + SplitOperation& operator=(const SplitOperation&) = delete; ~SplitOperation(); void onStart(DistributorStripeMessageSender& sender) override; - void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply> &) override; - const char* getName() const override { return "split"; }; - Type getType() const override { return SPLIT_BUCKET; } + void onReceive(DistributorStripeMessageSender& sender, const std::shared_ptr<api::StorageReply>&) override; + const char* getName() const noexcept override { return "split"; }; + Type getType() const noexcept override { return SPLIT_BUCKET; } bool isBlocked(const DistributorStripeOperationContext&, const OperationSequencer&) const override; bool shouldBlockThisOperation(uint32_t, uint16_t, uint8_t) const override; protected: diff --git a/storage/src/vespa/storage/distributor/operations/operation.h b/storage/src/vespa/storage/distributor/operations/operation.h index fff0450cc04..e24aa976221 100644 --- a/storage/src/vespa/storage/distributor/operations/operation.h +++ b/storage/src/vespa/storage/distributor/operations/operation.h @@ -23,7 +23,7 @@ class OperationSequencer; class Operation { public: - typedef std::shared_ptr<Operation> SP; + using SP = std::shared_ptr<Operation>; Operation(); @@ -45,7 +45,7 @@ public: onReceive(sender, msg); } - virtual const char* getName() const = 0; + virtual const char* getName() const noexcept = 0; virtual std::string getStatus() const; diff --git a/storage/src/vespa/storage/distributor/operationtargetresolver.cpp b/storage/src/vespa/storage/distributor/operationtargetresolver.cpp index 6362e6fa2ed..62be9e47125 100644 --- a/storage/src/vespa/storage/distributor/operationtargetresolver.cpp +++ b/storage/src/vespa/storage/distributor/operationtargetresolver.cpp @@ -3,8 +3,7 @@ #include "operationtargetresolver.h" #include <vespa/vespalib/stllike/asciistream.h> -namespace storage { -namespace distributor { +namespace storage::distributor { void OperationTarget::print(vespalib::asciistream& out, const PrintProperties&) const { @@ -13,5 +12,3 @@ OperationTarget::print(vespalib::asciistream& out, const PrintProperties&) const } } -} - diff --git a/storage/src/vespa/storage/distributor/operationtargetresolver.h b/storage/src/vespa/storage/distributor/operationtargetresolver.h index ea8c901fbeb..5e3c4a73f66 100644 --- a/storage/src/vespa/storage/distributor/operationtargetresolver.h +++ b/storage/src/vespa/storage/distributor/operationtargetresolver.h @@ -56,7 +56,7 @@ public: class OperationTargetResolver { public: - virtual ~OperationTargetResolver() {} + virtual ~OperationTargetResolver() = default; // Sadly all operations but put currently implement this by themselves. enum OperationType { diff --git a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp index 86b469fe471..6a9d7e0e6da 100644 --- a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp +++ b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp @@ -82,11 +82,10 @@ BucketInstanceList::populate(const document::BucketId& specificId, const Distrib void BucketInstanceList::removeNodeDuplicates() { - // Normally few entries in list. Probably heaper to just go through entries - // to detect whether it pre exist rather than creating a set or similar. + // Normally few entries in list. Probably cheaper to just go through entries + // to detect whether it preexists rather than creating a set or similar. BucketInstanceList other; - for (uint32_t i=0; i<_instances.size(); ++i) { - BucketInstance& instance(_instances[i]); + for (const auto& instance : _instances) { if (!other.contains(instance._node)) { other.add(instance); } @@ -97,7 +96,9 @@ BucketInstanceList::removeNodeDuplicates() void BucketInstanceList::limitToRedundancyCopies(uint16_t redundancy) { - while (_instances.size() > redundancy) _instances.pop_back(); + while (_instances.size() > redundancy) { + _instances.pop_back(); + } } document::BucketId @@ -143,8 +144,7 @@ OperationTargetList BucketInstanceList::createTargets(document::BucketSpace bucketSpace) { OperationTargetList result; - for (uint32_t i=0; i<_instances.size(); ++i) { - BucketInstance& bi(_instances[i]); + for (const auto& bi : _instances) { result.push_back(OperationTarget(document::Bucket(bucketSpace, bi._bucket), bi._node, !bi._exist)); } return result; diff --git a/storage/src/vespa/storage/distributor/ownership_transfer_safe_time_point_calculator.cpp b/storage/src/vespa/storage/distributor/ownership_transfer_safe_time_point_calculator.cpp index 6a03b2aa9bd..818c812ab4f 100644 --- a/storage/src/vespa/storage/distributor/ownership_transfer_safe_time_point_calculator.cpp +++ b/storage/src/vespa/storage/distributor/ownership_transfer_safe_time_point_calculator.cpp @@ -3,8 +3,7 @@ #include "ownership_transfer_safe_time_point_calculator.h" #include <thread> -namespace storage { -namespace distributor { +namespace storage::distributor { OwnershipTransferSafeTimePointCalculator::TimePoint OwnershipTransferSafeTimePointCalculator::safeTimePoint(TimePoint now) const @@ -31,4 +30,3 @@ OwnershipTransferSafeTimePointCalculator::safeTimePoint(TimePoint now) const } } -} diff --git a/storage/src/vespa/storage/distributor/pendingclusterstate.cpp b/storage/src/vespa/storage/distributor/pendingclusterstate.cpp index cf32d21eb82..c03b211d1aa 100644 --- a/storage/src/vespa/storage/distributor/pendingclusterstate.cpp +++ b/storage/src/vespa/storage/distributor/pendingclusterstate.cpp @@ -29,7 +29,7 @@ PendingClusterState::PendingClusterState( DistributorMessageSender& sender, const BucketSpaceStateMap& bucket_space_states, const std::shared_ptr<api::SetSystemStateCommand>& newStateCmd, - const OutdatedNodesMap &outdatedNodesMap, + const OutdatedNodesMap& outdatedNodesMap, api::Timestamp creationTimestamp) : _cmd(newStateCmd), _sentMessages(), @@ -79,7 +79,7 @@ PendingClusterState::PendingClusterState( PendingClusterState::~PendingClusterState() = default; void -PendingClusterState::initializeBucketSpaceTransitions(bool distributionChanged, const OutdatedNodesMap &outdatedNodesMap) +PendingClusterState::initializeBucketSpaceTransitions(bool distributionChanged, const OutdatedNodesMap& outdatedNodesMap) { OutdatedNodes emptyOutdatedNodes; for (const auto &elem : _bucket_space_states) { @@ -129,12 +129,6 @@ PendingClusterState::getOutdatedNodesMap() const return outdatedNodesMap; } -uint16_t -PendingClusterState::newStateStorageNodeCount() const -{ - return _newClusterStateBundle.getBaselineClusterState()->getNodeCount(lib::NodeType::STORAGE); -} - bool PendingClusterState::shouldRequestBucketInfo() const { @@ -318,7 +312,7 @@ PendingClusterState::getSummary() const (_clock.getTimeInMicros().getTime() - _creationTimestamp)); } -PendingBucketSpaceDbTransition & +PendingBucketSpaceDbTransition& PendingClusterState::getPendingBucketSpaceDbTransition(document::BucketSpace bucketSpace) { auto transitionIter = _pendingTransitions.find(bucketSpace); diff --git a/storage/src/vespa/storage/distributor/pendingclusterstate.h b/storage/src/vespa/storage/distributor/pendingclusterstate.h index 1a2f8901b47..cbc526d75d3 100644 --- a/storage/src/vespa/storage/distributor/pendingclusterstate.h +++ b/storage/src/vespa/storage/distributor/pendingclusterstate.h @@ -49,7 +49,7 @@ public: DistributorMessageSender& sender, const BucketSpaceStateMap& bucket_space_states, const std::shared_ptr<api::SetSystemStateCommand>& newStateCmd, - const OutdatedNodesMap &outdatedNodesMap, + const OutdatedNodesMap& outdatedNodesMap, api::Timestamp creationTimestamp) { // Naked new due to private constructor @@ -99,7 +99,7 @@ public: * Returns true if all the nodes we requested have replied to * the request bucket info commands. */ - bool done() { + [[nodiscard]] bool done() const noexcept { return _sentMessages.empty() && _delayedRequests.empty(); } @@ -111,7 +111,7 @@ public: return (_cmd.get() != nullptr); } - std::shared_ptr<api::SetSystemStateCommand> getCommand() { + std::shared_ptr<api::SetSystemStateCommand> getCommand() const noexcept { return _cmd; } @@ -128,7 +128,7 @@ public: && _newClusterStateBundle.deferredActivation()); } - void clearCommand() { + void clearCommand() noexcept { _cmd.reset(); } @@ -151,7 +151,7 @@ public: void merge_into_bucket_databases(StripeAccessGuard& guard); // Get pending transition for a specific bucket space. Only used by unit test. - PendingBucketSpaceDbTransition &getPendingBucketSpaceDbTransition(document::BucketSpace bucketSpace); + PendingBucketSpaceDbTransition& getPendingBucketSpaceDbTransition(document::BucketSpace bucketSpace); // May be a subset of the nodes in the cluster, depending on how many nodes were consulted // as part of the pending cluster state. Caller must take care to aggregate features. @@ -202,16 +202,12 @@ private: } }; - void initializeBucketSpaceTransitions(bool distributionChanged, const OutdatedNodesMap &outdatedNodesMap); + void initializeBucketSpaceTransitions(bool distributionChanged, const OutdatedNodesMap& outdatedNodesMap); void logConstructionInformation() const; void requestNode(BucketSpaceAndNode bucketSpaceAndNode); void requestNodes(); void requestBucketInfoFromStorageNodesWithChangedState(); - /** - * Number of nodes with node type 'storage' in _newClusterState. - */ - uint16_t newStateStorageNodeCount() const; bool shouldRequestBucketInfo() const; bool clusterIsDown() const; bool iAmDown() const; @@ -222,26 +218,30 @@ private: void update_reply_failure_statistics(const api::ReturnCode& result, const BucketSpaceAndNode& source); void update_node_supported_features_from_reply(uint16_t node, const api::RequestBucketInfoReply& reply); - std::shared_ptr<api::SetSystemStateCommand> _cmd; + using SentMessages = std::map<uint64_t, BucketSpaceAndNode>; + using DelayedRequests = std::deque<std::pair<framework::MilliSecTime, BucketSpaceAndNode>>; + using PendingTransitions = std::unordered_map<document::BucketSpace, std::unique_ptr<PendingBucketSpaceDbTransition>, document::BucketSpace::hash>; + using NodeFeatures = vespalib::hash_map<uint16_t, NodeSupportedFeatures>; - std::map<uint64_t, BucketSpaceAndNode> _sentMessages; - std::vector<bool> _requestedNodes; - std::deque<std::pair<framework::MilliSecTime, BucketSpaceAndNode> > _delayedRequests; + std::shared_ptr<api::SetSystemStateCommand> _cmd; - lib::ClusterStateBundle _prevClusterStateBundle; - lib::ClusterStateBundle _newClusterStateBundle; + SentMessages _sentMessages; + std::vector<bool> _requestedNodes; + DelayedRequests _delayedRequests; - const framework::Clock& _clock; - ClusterInformation::CSP _clusterInfo; - api::Timestamp _creationTimestamp; + lib::ClusterStateBundle _prevClusterStateBundle; + lib::ClusterStateBundle _newClusterStateBundle; - DistributorMessageSender& _sender; + const framework::Clock& _clock; + ClusterInformation::CSP _clusterInfo; + api::Timestamp _creationTimestamp; + DistributorMessageSender& _sender; const BucketSpaceStateMap& _bucket_space_states; - uint32_t _clusterStateVersion; - bool _isVersionedTransition; - bool _bucketOwnershipTransfer; - std::unordered_map<document::BucketSpace, std::unique_ptr<PendingBucketSpaceDbTransition>, document::BucketSpace::hash> _pendingTransitions; - vespalib::hash_map<uint16_t, NodeSupportedFeatures> _node_features; + uint32_t _clusterStateVersion; + bool _isVersionedTransition; + bool _bucketOwnershipTransfer; + PendingTransitions _pendingTransitions; + NodeFeatures _node_features; }; } diff --git a/storage/src/vespa/storage/distributor/pendingmessagetracker.h b/storage/src/vespa/storage/distributor/pendingmessagetracker.h index 8f5f05d8a1a..b7fa666fe42 100644 --- a/storage/src/vespa/storage/distributor/pendingmessagetracker.h +++ b/storage/src/vespa/storage/distributor/pendingmessagetracker.h @@ -77,7 +77,7 @@ public: */ using TimePoint = std::chrono::milliseconds; - explicit PendingMessageTracker(framework::ComponentRegister&, uint32_t stripe_index); + PendingMessageTracker(framework::ComponentRegister&, uint32_t stripe_index); ~PendingMessageTracker() override; void insert(const std::shared_ptr<api::StorageMessage>&); @@ -111,8 +111,8 @@ public: * The vector might be smaller than a given node index. In that case, that storage * node has never had any pending messages. */ - const NodeInfo& getNodeInfo() const { return _nodeInfo; } - NodeInfo& getNodeInfo() { return _nodeInfo; } + const NodeInfo& getNodeInfo() const noexcept { return _nodeInfo; } + NodeInfo& getNodeInfo() noexcept { return _nodeInfo; } /** * Clears all pending messages for the given node, and returns diff --git a/storage/src/vespa/storage/distributor/persistence_operation_metric_set.cpp b/storage/src/vespa/storage/distributor/persistence_operation_metric_set.cpp index 0e42a505567..944b4bafa0a 100644 --- a/storage/src/vespa/storage/distributor/persistence_operation_metric_set.cpp +++ b/storage/src/vespa/storage/distributor/persistence_operation_metric_set.cpp @@ -4,7 +4,7 @@ #include <vespa/storageapi/messageapi/returncode.h> #include <vespa/metrics/summetric.hpp> -namespace storage { +namespace storage::distributor { using metrics::MetricSet; @@ -107,5 +107,4 @@ PersistenceOperationMetricSet::updateFromResult(const api::ReturnCode& result) } } -} // storage - +} // storage::distributor diff --git a/storage/src/vespa/storage/distributor/persistence_operation_metric_set.h b/storage/src/vespa/storage/distributor/persistence_operation_metric_set.h index d42e6fa308c..b818d1bdd9f 100644 --- a/storage/src/vespa/storage/distributor/persistence_operation_metric_set.h +++ b/storage/src/vespa/storage/distributor/persistence_operation_metric_set.h @@ -8,11 +8,9 @@ #include <vespa/metrics/summetric.h> #include <mutex> -namespace storage { +namespace storage::api { class ReturnCode; } -namespace api { -class ReturnCode; -} +namespace storage::distributor { class PersistenceFailuresMetricSet : public metrics::MetricSet { diff --git a/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp b/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp index 45129f7be04..08b99cf89a8 100644 --- a/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp +++ b/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp @@ -4,7 +4,6 @@ #include "distributor_bucket_space_repo.h" #include "distributor_bucket_space.h" #include <vespa/vdslib/distribution/distribution.h> -#include <vespa/storage/common/vectorprinter.h> #include <vespa/storageapi/message/persistence.h> #include <vespa/log/log.h> @@ -41,7 +40,7 @@ PersistenceMessageTrackerImpl::updateDB() _op_ctx.update_bucket_database(entry.first, entry.second); } - for (const auto & entry : _remapBucketInfo){ + for (const auto & entry : _remapBucketInfo){ _op_ctx.update_bucket_database(entry.first, entry.second, DatabaseUpdate::CREATE_IF_NONEXISTING); } } diff --git a/storage/src/vespa/storage/distributor/persistencemessagetracker.h b/storage/src/vespa/storage/distributor/persistencemessagetracker.h index e65325cff20..ca330859259 100644 --- a/storage/src/vespa/storage/distributor/persistencemessagetracker.h +++ b/storage/src/vespa/storage/distributor/persistencemessagetracker.h @@ -43,7 +43,7 @@ public: void updateDB(); void updateMetrics(); - bool success() const { return _success; } + [[nodiscard]] bool success() const noexcept { return _success; } void fail(MessageSender& sender, const api::ReturnCode& result) override; /** @@ -67,19 +67,19 @@ public: private: using MessageBatch = std::vector<uint64_t>; - std::vector<MessageBatch> _messageBatches; - PersistenceOperationMetricSet& _metric; + std::vector<MessageBatch> _messageBatches; + PersistenceOperationMetricSet& _metric; std::shared_ptr<api::BucketInfoReply> _reply; - DistributorStripeOperationContext& _op_ctx; - api::Timestamp _revertTimestamp; - std::vector<BucketNodePair> _revertNodes; - mbus::Trace _trace; - framework::MilliSecTimer _requestTimer; - uint32_t _n_persistence_replies_total; - uint32_t _n_successful_persistence_replies; - uint8_t _priority; - bool _success; + DistributorStripeOperationContext& _op_ctx; + api::Timestamp _revertTimestamp; + std::vector<BucketNodePair> _revertNodes; + mbus::Trace _trace; + framework::MilliSecTimer _requestTimer; + uint32_t _n_persistence_replies_total; + uint32_t _n_successful_persistence_replies; + uint8_t _priority; + bool _success; bool canSendReplyEarly() const; void addBucketInfoFromReply(uint16_t node, const api::BucketInfoReply& reply); diff --git a/storage/src/vespa/storage/distributor/statechecker.h b/storage/src/vespa/storage/distributor/statechecker.h index 11c83fdde07..348d90bc712 100644 --- a/storage/src/vespa/storage/distributor/statechecker.h +++ b/storage/src/vespa/storage/distributor/statechecker.h @@ -37,7 +37,7 @@ class NodeMaintenanceStatsTracker; */ class StateChecker { public: - typedef std::shared_ptr<StateChecker> SP; + using SP = std::shared_ptr<StateChecker>; /** * Context object used when generating operations and metrics for a @@ -49,48 +49,46 @@ public: const DistributorStripeOperationContext& op_ctx_in, const DistributorBucketSpace &distributorBucketSpace, NodeMaintenanceStatsTracker&, - const document::Bucket &bucket_); + const document::Bucket& bucket_); ~Context(); - Context(const Context &) = delete; - Context & operator =(const Context &) = delete; + Context(const Context&) = delete; + Context& operator=(const Context&) = delete; // Per bucket - document::Bucket bucket; - document::BucketId siblingBucket; - - BucketDatabase::Entry entry; - BucketDatabase::Entry siblingEntry; + document::Bucket bucket; + document::BucketId siblingBucket; + BucketDatabase::Entry entry; + BucketDatabase::Entry siblingEntry; std::vector<BucketDatabase::Entry> entries; // Common - const lib::ClusterState& systemState; - const lib::ClusterState* pending_cluster_state; // nullptr if no state is pending. + const lib::ClusterState& systemState; + const lib::ClusterState* pending_cluster_state; // nullptr if no state is pending. const DistributorConfiguration& distributorConfig; - const lib::Distribution& distribution; - - BucketGcTimeCalculator gcTimeCalculator; + const lib::Distribution& distribution; + BucketGcTimeCalculator gcTimeCalculator; // Separate ideal state into ordered sequence and unordered set, as we // need to both know the actual order (activation prioritization etc) as // well as have the ability to quickly check if a node is in an ideal // location. - std::vector<uint16_t> idealState; + std::vector<uint16_t> idealState; std::unordered_set<uint16_t> unorderedIdealState; - const DistributorNodeContext& node_ctx; + const DistributorNodeContext& node_ctx; const DistributorStripeOperationContext& op_ctx; - const BucketDatabase& db; - NodeMaintenanceStatsTracker& stats; - const bool merges_inhibited_in_bucket_space; + const BucketDatabase& db; + NodeMaintenanceStatsTracker& stats; + const bool merges_inhibited_in_bucket_space; - const BucketDatabase::Entry& getSiblingEntry() const { + const BucketDatabase::Entry& getSiblingEntry() const noexcept { return siblingEntry; } - document::Bucket getBucket() const { return bucket; } - document::BucketId getBucketId() const { return bucket.getBucketId(); } - document::BucketSpace getBucketSpace() const { return bucket.getBucketSpace(); } + document::Bucket getBucket() const noexcept { return bucket; } + document::BucketId getBucketId() const noexcept { return bucket.getBucketId(); } + document::BucketSpace getBucketSpace() const noexcept { return bucket.getBucketSpace(); } std::string toString() const; }; @@ -151,7 +149,7 @@ public: /** * Returns the name of this state checker. */ - virtual const char* getName() const = 0; + virtual const char* getName() const noexcept = 0; }; } diff --git a/storage/src/vespa/storage/distributor/statecheckers.h b/storage/src/vespa/storage/distributor/statecheckers.h index f998dbeacfd..c1579cb5eda 100644 --- a/storage/src/vespa/storage/distributor/statecheckers.h +++ b/storage/src/vespa/storage/distributor/statecheckers.h @@ -9,14 +9,14 @@ class SynchronizeAndMoveStateChecker : public StateChecker { public: Result check(Context& c) const override; - const char* getName() const override { return "SynchronizeAndMove"; } + const char* getName() const noexcept override { return "SynchronizeAndMove"; } }; class DeleteExtraCopiesStateChecker : public StateChecker { public: Result check(Context& c) const override; - const char* getName() const override { return "DeleteExtraCopies"; } + const char* getName() const noexcept override { return "DeleteExtraCopies"; } private: static bool bucketHasNoData(const Context& c); @@ -33,7 +33,7 @@ class JoinBucketsStateChecker : public StateChecker { public: Result check(Context& c) const override; - const char* getName() const override { return "JoinBuckets"; } + const char* getName() const noexcept override { return "JoinBuckets"; } private: static uint64_t getTotalUsedFileSize(const Context& c); static uint64_t getTotalMetaCount(const Context& c); @@ -50,7 +50,7 @@ class SplitBucketStateChecker : public StateChecker { public: Result check(Context& c) const override; - const char* getName() const override { return "SplitBucket"; } + const char* getName() const noexcept override { return "SplitBucket"; } private: static Result generateMinimumBucketSplitOperation(Context& c); static Result generateMaxSizeExceededSplitOperation(Context& c); @@ -63,14 +63,12 @@ class SplitInconsistentStateChecker : public StateChecker { public: Result check(Context& c) const override; - const char* getName() const override { return "SplitInconsistentBuckets"; } + const char* getName() const noexcept override { return "SplitInconsistentBuckets"; } private: - typedef std::pair<document::BucketId, uint16_t> BucketAndNode; static bool isLeastSplitBucket(const document::BucketId& bucket,const std::vector<BucketDatabase::Entry>& entries); static uint32_t getHighestUsedBits(const std::vector<BucketDatabase::Entry>& entries); static vespalib::string getReason(const document::BucketId& bucketId, const std::vector<BucketDatabase::Entry>& entries); - static bool isLeastSplit(Context& c, std::vector<BucketAndNode>& others); }; class ActiveList; @@ -80,14 +78,14 @@ class BucketStateStateChecker : public StateChecker static bool shouldSkipActivationDueToMaintenance(const ActiveList& activeList, const Context& c); public: Result check(Context& c) const override; - const char* getName() const override { return "SetBucketState"; } + const char* getName() const noexcept override { return "SetBucketState"; } }; class GarbageCollectionStateChecker : public StateChecker { public: Result check(Context& c) const override; - const char* getName() const override { return "GarbageCollection"; } + const char* getName() const noexcept override { return "GarbageCollection"; } private: static bool garbage_collection_disabled(const Context& c) noexcept; static bool needs_garbage_collection(const Context& c, std::chrono::seconds time_since_epoch); diff --git a/storage/src/vespa/storage/distributor/statusdelegator.h b/storage/src/vespa/storage/distributor/statusdelegator.h index 0a4f3e63472..3001f135964 100644 --- a/storage/src/vespa/storage/distributor/statusdelegator.h +++ b/storage/src/vespa/storage/distributor/statusdelegator.h @@ -1,18 +1,16 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -namespace storage { -namespace distributor { +namespace storage::distributor { struct DelegatedStatusRequest; class StatusDelegator { public: - virtual ~StatusDelegator() {} + virtual ~StatusDelegator() = default; virtual bool handleStatusRequest(const DelegatedStatusRequest& request) const = 0; }; -} // distributor -} // storage +} // storage::distributor diff --git a/storage/src/vespa/storage/distributor/statusreporterdelegate.cpp b/storage/src/vespa/storage/distributor/statusreporterdelegate.cpp index 41fc85baca5..a441ed9504c 100644 --- a/storage/src/vespa/storage/distributor/statusreporterdelegate.cpp +++ b/storage/src/vespa/storage/distributor/statusreporterdelegate.cpp @@ -15,6 +15,8 @@ StatusReporterDelegate::StatusReporterDelegate( { } +StatusReporterDelegate::~StatusReporterDelegate() = default; + vespalib::string StatusReporterDelegate::getReportContentType(const framework::HttpUrlPath& path) const { diff --git a/storage/src/vespa/storage/distributor/statusreporterdelegate.h b/storage/src/vespa/storage/distributor/statusreporterdelegate.h index 0acf9b16edb..fb13812beb0 100644 --- a/storage/src/vespa/storage/distributor/statusreporterdelegate.h +++ b/storage/src/vespa/storage/distributor/statusreporterdelegate.h @@ -5,9 +5,7 @@ #include "statusdelegator.h" #include <vespa/storageframework/generic/component/component.h> - -namespace storage { -namespace distributor { +namespace storage::distributor { class StatusReporterDelegate : public framework::StatusReporter @@ -19,11 +17,11 @@ public: StatusReporterDelegate(framework::ComponentRegister& compReg, const StatusDelegator& delegator, const framework::StatusReporter& target); + ~StatusReporterDelegate() override; void registerStatusPage(); vespalib::string getReportContentType(const framework::HttpUrlPath&) const override; bool reportStatus(std::ostream&, const framework::HttpUrlPath&) const override; }; -} // distributor -} // storage +} // storage::distributor diff --git a/storage/src/vespa/storage/distributor/throttlingoperationstarter.h b/storage/src/vespa/storage/distributor/throttlingoperationstarter.h index 00d1237b2f7..a0613c60fa4 100644 --- a/storage/src/vespa/storage/distributor/throttlingoperationstarter.h +++ b/storage/src/vespa/storage/distributor/throttlingoperationstarter.h @@ -30,7 +30,7 @@ class ThrottlingOperationStarter : public OperationStarter, public PendingWindow void onClose(DistributorStripeMessageSender& sender) override { _operation->onClose(sender); } - const char* getName() const override { + const char* getName() const noexcept override { return _operation->getName(); } std::string getStatus() const override { diff --git a/storage/src/vespa/storage/distributor/update_metric_set.cpp b/storage/src/vespa/storage/distributor/update_metric_set.cpp index 82f55d3e819..fafce3dae5a 100644 --- a/storage/src/vespa/storage/distributor/update_metric_set.cpp +++ b/storage/src/vespa/storage/distributor/update_metric_set.cpp @@ -2,7 +2,7 @@ #include "update_metric_set.h" -namespace storage { +namespace storage::distributor { using metrics::MetricSet; diff --git a/storage/src/vespa/storage/distributor/update_metric_set.h b/storage/src/vespa/storage/distributor/update_metric_set.h index 4171a8c32da..ad39f8e93cc 100644 --- a/storage/src/vespa/storage/distributor/update_metric_set.h +++ b/storage/src/vespa/storage/distributor/update_metric_set.h @@ -3,7 +3,7 @@ #include "persistence_operation_metric_set.h" -namespace storage { +namespace storage::distributor { class UpdateMetricSet : public PersistenceOperationMetricSet { public: diff --git a/storage/src/vespa/storage/distributor/visitormetricsset.cpp b/storage/src/vespa/storage/distributor/visitormetricsset.cpp index c94dc025fa1..cbc2f0e25d3 100644 --- a/storage/src/vespa/storage/distributor/visitormetricsset.cpp +++ b/storage/src/vespa/storage/distributor/visitormetricsset.cpp @@ -2,7 +2,7 @@ #include "visitormetricsset.h" -namespace storage { +namespace storage::distributor { using metrics::MetricSet; diff --git a/storage/src/vespa/storage/distributor/visitormetricsset.h b/storage/src/vespa/storage/distributor/visitormetricsset.h index 35e03faf279..7751e0805f2 100644 --- a/storage/src/vespa/storage/distributor/visitormetricsset.h +++ b/storage/src/vespa/storage/distributor/visitormetricsset.h @@ -3,7 +3,7 @@ #include "persistence_operation_metric_set.h" -namespace storage { +namespace storage::distributor { struct VisitorMetricSet : public PersistenceOperationMetricSet { metrics::LongAverageMetric buckets_per_visitor; @@ -11,11 +11,10 @@ struct VisitorMetricSet : public PersistenceOperationMetricSet { metrics::LongAverageMetric bytes_per_visitor; VisitorMetricSet(MetricSet* owner = nullptr); - ~VisitorMetricSet(); + ~VisitorMetricSet() override; MetricSet * clone(std::vector<Metric::UP>& ownerList, CopyType copyType, MetricSet* owner, bool includeUnused) const override; }; -} // storage - +} // storage::distributor diff --git a/storage/src/vespa/storageapi/defs.h b/storage/src/vespa/storageapi/defs.h index 9ee9fdf2218..bf802a20a5c 100644 --- a/storage/src/vespa/storageapi/defs.h +++ b/storage/src/vespa/storageapi/defs.h @@ -10,9 +10,9 @@ namespace storage:: api { -typedef uint64_t Timestamp; -typedef uint32_t VisitorId; +using Timestamp = uint64_t; +using VisitorId = uint32_t; -const Timestamp MAX_TIMESTAMP = (Timestamp)-1ll; +constexpr Timestamp MAX_TIMESTAMP = (Timestamp)-1LL; } diff --git a/storage/src/vespa/storageapi/message/stat.h b/storage/src/vespa/storageapi/message/stat.h index 0797ae43799..875580ca064 100644 --- a/storage/src/vespa/storageapi/message/stat.h +++ b/storage/src/vespa/storageapi/message/stat.h @@ -58,15 +58,15 @@ class GetBucketListReply : public BucketReply { public: struct BucketInfo { document::BucketId _bucket; - vespalib::string _bucketInformation; + vespalib::string _bucketInformation; BucketInfo(const document::BucketId& id, - vespalib::stringref bucketInformation) + vespalib::stringref bucketInformation) noexcept : _bucket(id), _bucketInformation(bucketInformation) {} - bool operator==(const BucketInfo& other) const { + bool operator==(const BucketInfo& other) const noexcept { return (_bucket == other._bucket && _bucketInformation == other._bucketInformation); } |