From f8c21c963838f979bf8234afb543aaae8b0755cd Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 9 Aug 2023 14:33:26 +0000 Subject: - Use hash_map instead of std::unordered_set dur to performance and memory fragmentation. - Move code from .h to .cpp that uses a lot of templated code. --- .../storage/distributor/distributor_stripe.cpp | 5 +- .../maintenance/node_maintenance_stats_tracker.cpp | 61 +++++++++++++++++++++- .../maintenance/node_maintenance_stats_tracker.h | 56 ++++++-------------- .../maintenance/simplemaintenancescanner.cpp | 17 ++++-- .../maintenance/simplemaintenancescanner.h | 12 +++-- 5 files changed, 100 insertions(+), 51 deletions(-) (limited to 'storage') diff --git a/storage/src/vespa/storage/distributor/distributor_stripe.cpp b/storage/src/vespa/storage/distributor/distributor_stripe.cpp index ede85c036b3..c93c23ce8ff 100644 --- a/storage/src/vespa/storage/distributor/distributor_stripe.cpp +++ b/storage/src/vespa/storage/distributor/distributor_stripe.cpp @@ -314,7 +314,7 @@ DistributorStripe::enterRecoveryMode() { LOG(debug, "Entering recovery mode"); _schedulingMode = MaintenanceScheduler::RECOVERY_SCHEDULING_MODE; - _scanner->reset(); + auto stats = _scanner->reset(); // We enter recovery mode due to cluster state or distribution config changes. // Until we have completed a new DB scan round, we don't know the state of our // newly owned buckets and must not report stats for these out to the cluster @@ -643,7 +643,7 @@ DistributorStripe::updateInternalMetricsForCompletedScan() _bucketDBMetricUpdater.completeRound(); _bucketDbStats = _bucketDBMetricUpdater.getLastCompleteStats(); - _maintenanceStats = _scanner->getPendingMaintenanceStats(); + _maintenanceStats = _scanner->reset(); auto new_space_stats = toBucketSpacesStats(_maintenanceStats.perNodeStats); if (merge_no_longer_pending_edge(_bucketSpacesStats, new_space_stats)) { _must_send_updated_host_info = true; @@ -684,7 +684,6 @@ DistributorStripe::scanNextBucket() updateInternalMetricsForCompletedScan(); leaveRecoveryMode(); send_updated_host_info_if_required(); - _scanner->reset(); } else { const auto &distribution(_bucketSpaceRepo->get(scanResult.getBucketSpace()).getDistribution()); _bucketDBMetricUpdater.visit(scanResult.getEntry(), distribution.getRedundancy()); diff --git a/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.cpp b/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.cpp index 7ac99f5712f..2d15c10fbff 100644 --- a/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.cpp +++ b/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.cpp @@ -1,6 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "node_maintenance_stats_tracker.h" +#include +#include #include namespace storage::distributor { @@ -21,6 +23,54 @@ merge_bucket_spaces_stats(NodeMaintenanceStatsTracker::BucketSpacesStats& dest, } +void +NodeMaintenanceStatsTracker::incMovingOut(uint16_t node, document::BucketSpace bucketSpace) { + ++_node_stats[node][bucketSpace].movingOut; + ++_total_stats.movingOut; +} + +void +NodeMaintenanceStatsTracker::incSyncing(uint16_t node, document::BucketSpace bucketSpace) { + ++_node_stats[node][bucketSpace].syncing; + ++_total_stats.syncing; +} + +void +NodeMaintenanceStatsTracker::incCopyingIn(uint16_t node, document::BucketSpace bucketSpace) { + ++_node_stats[node][bucketSpace].copyingIn; + ++_total_stats.copyingIn; +} + +void +NodeMaintenanceStatsTracker::incCopyingOut(uint16_t node, document::BucketSpace bucketSpace) { + ++_node_stats[node][bucketSpace].copyingOut; + ++_total_stats.copyingOut; +} + +void +NodeMaintenanceStatsTracker::NodeMaintenanceStatsTracker::incTotal(uint16_t node, document::BucketSpace bucketSpace) { + ++_node_stats[node][bucketSpace].total; + ++_total_stats.total; +} + +const NodeMaintenanceStats& +NodeMaintenanceStatsTracker::forNode(uint16_t node, document::BucketSpace bucketSpace) const { + auto nodeItr = _node_stats.find(node); + if (nodeItr != _node_stats.end()) { + auto bucketSpaceItr = nodeItr->second.find(bucketSpace); + if (bucketSpaceItr != nodeItr->second.end()) { + return bucketSpaceItr->second; + } + } + return _emptyNodeMaintenanceStats; +} + +bool +NodeMaintenanceStatsTracker::operator==(const NodeMaintenanceStatsTracker& rhs) const noexcept { + return ((_node_stats == rhs._node_stats) && + (_max_observed_time_since_last_gc == rhs._max_observed_time_since_last_gc)); +} + void NodeMaintenanceStatsTracker::merge(const NodeMaintenanceStatsTracker& rhs) { @@ -45,13 +95,22 @@ operator<<(std::ostream& os, const NodeMaintenanceStats& stats) return os; } -NodeMaintenanceStatsTracker::NodeMaintenanceStatsTracker() +NodeMaintenanceStatsTracker::NodeMaintenanceStatsTracker() noexcept : _node_stats(), _total_stats(), _max_observed_time_since_last_gc(0) {} +NodeMaintenanceStatsTracker::NodeMaintenanceStatsTracker(NodeMaintenanceStatsTracker &&) noexcept = default; +NodeMaintenanceStatsTracker & NodeMaintenanceStatsTracker::operator =(NodeMaintenanceStatsTracker &&) noexcept = default; +NodeMaintenanceStatsTracker::NodeMaintenanceStatsTracker(const NodeMaintenanceStatsTracker &) = default; NodeMaintenanceStatsTracker::~NodeMaintenanceStatsTracker() = default; +void +NodeMaintenanceStatsTracker::reset(size_t nodes) { + _node_stats.resize(nodes); + _total_stats = NodeMaintenanceStats(); +} + } diff --git a/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.h b/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.h index 3818dd4bacb..84705fbca9d 100644 --- a/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.h +++ b/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.h @@ -1,9 +1,9 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -#include #include #include +#include namespace storage::distributor { @@ -51,8 +51,8 @@ std::ostream& operator<<(std::ostream&, const NodeMaintenanceStats&); class NodeMaintenanceStatsTracker { public: - using BucketSpacesStats = std::unordered_map; - using PerNodeStats = std::unordered_map; + using BucketSpacesStats = vespalib::hash_map; + using PerNodeStats = vespalib::hash_map; private: PerNodeStats _node_stats; @@ -62,33 +62,23 @@ private: static const NodeMaintenanceStats _emptyNodeMaintenanceStats; public: - NodeMaintenanceStatsTracker(); + NodeMaintenanceStatsTracker() noexcept; + NodeMaintenanceStatsTracker(NodeMaintenanceStatsTracker &&) noexcept; + NodeMaintenanceStatsTracker & operator =(NodeMaintenanceStatsTracker &&) noexcept; + NodeMaintenanceStatsTracker(const NodeMaintenanceStatsTracker &); ~NodeMaintenanceStatsTracker(); + void reset(size_t nodes); + size_t numNodes() const { return _node_stats.size(); } - void incMovingOut(uint16_t node, document::BucketSpace bucketSpace) { - ++_node_stats[node][bucketSpace].movingOut; - ++_total_stats.movingOut; - } + void incMovingOut(uint16_t node, document::BucketSpace bucketSpace); - void incSyncing(uint16_t node, document::BucketSpace bucketSpace) { - ++_node_stats[node][bucketSpace].syncing; - ++_total_stats.syncing; - } + void incSyncing(uint16_t node, document::BucketSpace bucketSpace); - void incCopyingIn(uint16_t node, document::BucketSpace bucketSpace) { - ++_node_stats[node][bucketSpace].copyingIn; - ++_total_stats.copyingIn; - } + void incCopyingIn(uint16_t node, document::BucketSpace bucketSpace); - void incCopyingOut(uint16_t node, document::BucketSpace bucketSpace) { - ++_node_stats[node][bucketSpace].copyingOut; - ++_total_stats.copyingOut; - } + void incCopyingOut(uint16_t node, document::BucketSpace bucketSpace); - void incTotal(uint16_t node, document::BucketSpace bucketSpace) { - ++_node_stats[node][bucketSpace].total; - ++_total_stats.total; - } + void incTotal(uint16_t node, document::BucketSpace bucketSpace); void update_observed_time_since_last_gc(vespalib::duration time_since_gc) noexcept { _max_observed_time_since_last_gc = std::max(time_since_gc, _max_observed_time_since_last_gc); @@ -98,18 +88,9 @@ public: * Returned statistics for a given node index and bucket space, or all zero statistics * if none have been recorded yet */ - const NodeMaintenanceStats& forNode(uint16_t node, document::BucketSpace bucketSpace) const { - auto nodeItr = _node_stats.find(node); - if (nodeItr != _node_stats.end()) { - auto bucketSpaceItr = nodeItr->second.find(bucketSpace); - if (bucketSpaceItr != nodeItr->second.end()) { - return bucketSpaceItr->second; - } - } - return _emptyNodeMaintenanceStats; - } + const NodeMaintenanceStats& forNode(uint16_t node, document::BucketSpace bucketSpace) const; - const PerNodeStats& perNodeStats() const { + const PerNodeStats& perNodeStats() const noexcept { return _node_stats; } @@ -124,10 +105,7 @@ public: return _max_observed_time_since_last_gc; } - bool operator==(const NodeMaintenanceStatsTracker& rhs) const { - return ((_node_stats == rhs._node_stats) && - (_max_observed_time_since_last_gc == rhs._max_observed_time_since_last_gc)); - } + bool operator==(const NodeMaintenanceStatsTracker& rhs) const noexcept; void merge(const NodeMaintenanceStatsTracker& rhs); }; diff --git a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp index afcbef32584..e0c1abaaffa 100644 --- a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp +++ b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp @@ -41,11 +41,20 @@ SimpleMaintenanceScanner::PendingMaintenanceStats::merge(const PendingMaintenanc perNodeStats.merge(rhs.perNodeStats); } -SimpleMaintenanceScanner::PendingMaintenanceStats::PendingMaintenanceStats() = default; +SimpleMaintenanceScanner::PendingMaintenanceStats::PendingMaintenanceStats() noexcept = default; SimpleMaintenanceScanner::PendingMaintenanceStats::~PendingMaintenanceStats() = default; SimpleMaintenanceScanner::PendingMaintenanceStats::PendingMaintenanceStats(const PendingMaintenanceStats &) = default; +SimpleMaintenanceScanner::PendingMaintenanceStats::PendingMaintenanceStats(PendingMaintenanceStats &&) noexcept = default; SimpleMaintenanceScanner::PendingMaintenanceStats & -SimpleMaintenanceScanner::PendingMaintenanceStats::operator = (const PendingMaintenanceStats &) = default; +SimpleMaintenanceScanner::PendingMaintenanceStats::operator = (PendingMaintenanceStats &&) noexcept = default; + +SimpleMaintenanceScanner::PendingMaintenanceStats +SimpleMaintenanceScanner::PendingMaintenanceStats::reset() { + PendingMaintenanceStats prev = std::move(*this); + global = GlobalMaintenanceStats(); + perNodeStats.reset(prev.perNodeStats.numNodes()); + return prev; +} MaintenanceScanner::ScanResult SimpleMaintenanceScanner::scanNext() @@ -68,12 +77,12 @@ SimpleMaintenanceScanner::scanNext() } } -void +SimpleMaintenanceScanner::PendingMaintenanceStats SimpleMaintenanceScanner::reset() { _bucketCursor = document::BucketId(); _bucketSpaceItr = _bucketSpaceRepo.begin(); - _pendingMaintenance = PendingMaintenanceStats(); + return _pendingMaintenance.reset(); } void diff --git a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h index 7af61815c31..35b022c7af7 100644 --- a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h +++ b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h @@ -23,11 +23,14 @@ public: void merge(const GlobalMaintenanceStats& rhs) noexcept; }; struct PendingMaintenanceStats { - PendingMaintenanceStats(); + PendingMaintenanceStats() noexcept; PendingMaintenanceStats(const PendingMaintenanceStats &); - PendingMaintenanceStats &operator = (const PendingMaintenanceStats &); + PendingMaintenanceStats &operator = (const PendingMaintenanceStats &) = delete; + PendingMaintenanceStats(PendingMaintenanceStats &&) noexcept; + PendingMaintenanceStats &operator = (PendingMaintenanceStats &&) noexcept; ~PendingMaintenanceStats(); - GlobalMaintenanceStats global; + PendingMaintenanceStats reset(); + GlobalMaintenanceStats global; NodeMaintenanceStatsTracker perNodeStats; void merge(const PendingMaintenanceStats& rhs); @@ -50,11 +53,12 @@ public: ~SimpleMaintenanceScanner() override; ScanResult scanNext() override; - void reset(); + PendingMaintenanceStats reset(); // TODO: move out into own interface! void prioritizeBucket(const document::Bucket &id); + // TODO Only for testing const PendingMaintenanceStats& getPendingMaintenanceStats() const noexcept { return _pendingMaintenance; } -- cgit v1.2.3