From 63532725cd0d114fa6ec2719c9283e80ea65943d Mon Sep 17 00:00:00 2001 From: Geir Storli Date: Wed, 21 Feb 2018 10:19:43 +0000 Subject: Make NodeMaintenanceStatsTracker bucket space aware. --- storage/src/tests/distributor/distributortest.cpp | 10 +-- storage/src/tests/distributor/maintenancemocks.h | 7 +- .../nodemaintenancestatstrackertest.cpp | 90 ++++++++++++++++++---- .../distributor/simplemaintenancescannertest.cpp | 16 ++-- .../src/tests/distributor/statecheckerstest.cpp | 51 +++++------- .../maintenance/node_maintenance_stats_tracker.cpp | 2 +- .../maintenance/node_maintenance_stats_tracker.h | 57 +++++++++----- .../src/vespa/storage/distributor/statechecker.h | 1 + .../vespa/storage/distributor/statecheckers.cpp | 8 +- 9 files changed, 155 insertions(+), 87 deletions(-) diff --git a/storage/src/tests/distributor/distributortest.cpp b/storage/src/tests/distributor/distributortest.cpp index d585c4d0d32..4d08e4adb71 100644 --- a/storage/src/tests/distributor/distributortest.cpp +++ b/storage/src/tests/distributor/distributortest.cpp @@ -630,18 +630,18 @@ Distributor_Test::mergeStatsAreAccumulatedDuringDatabaseIteration() NodeMaintenanceStats wanted; wanted.syncing = 1; wanted.copyingOut = 2; - CPPUNIT_ASSERT_EQUAL(wanted, stats.perNodeStats.forNode(0)); + CPPUNIT_ASSERT_EQUAL(wanted, stats.perNodeStats.forNode(0, makeBucketSpace())); } { NodeMaintenanceStats wanted; wanted.movingOut = 1; - CPPUNIT_ASSERT_EQUAL(wanted, stats.perNodeStats.forNode(1)); + CPPUNIT_ASSERT_EQUAL(wanted, stats.perNodeStats.forNode(1, makeBucketSpace())); } { NodeMaintenanceStats wanted; wanted.syncing = 1; wanted.copyingIn = 2; - CPPUNIT_ASSERT_EQUAL(wanted, stats.perNodeStats.forNode(2)); + CPPUNIT_ASSERT_EQUAL(wanted, stats.perNodeStats.forNode(2, makeBucketSpace())); } } @@ -666,12 +666,12 @@ Distributor_Test::statsGeneratedForPreemptedOperations() { NodeMaintenanceStats wanted; wanted.syncing = 1; - CPPUNIT_ASSERT_EQUAL(wanted, stats.perNodeStats.forNode(0)); + CPPUNIT_ASSERT_EQUAL(wanted, stats.perNodeStats.forNode(0, makeBucketSpace())); } { NodeMaintenanceStats wanted; wanted.syncing = 1; - CPPUNIT_ASSERT_EQUAL(wanted, stats.perNodeStats.forNode(1)); + CPPUNIT_ASSERT_EQUAL(wanted, stats.perNodeStats.forNode(1, makeBucketSpace())); } } diff --git a/storage/src/tests/distributor/maintenancemocks.h b/storage/src/tests/distributor/maintenancemocks.h index 505a7cece9e..2be74ca1a8b 100644 --- a/storage/src/tests/distributor/maintenancemocks.h +++ b/storage/src/tests/distributor/maintenancemocks.h @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once +#include #include #include #include @@ -8,6 +9,8 @@ #include #include +using document::test::makeBucketSpace; + namespace storage { namespace distributor { @@ -18,8 +21,8 @@ class MockMaintenancePriorityGenerator const document::Bucket&, NodeMaintenanceStatsTracker& stats) const override { - stats.incMovingOut(1); - stats.incCopyingIn(2); + stats.incMovingOut(1, makeBucketSpace()); + stats.incCopyingIn(2, makeBucketSpace()); return MaintenancePriorityAndType( MaintenancePriority(MaintenancePriority::VERY_HIGH), MaintenanceOperation::MERGE_BUCKET); diff --git a/storage/src/tests/distributor/nodemaintenancestatstrackertest.cpp b/storage/src/tests/distributor/nodemaintenancestatstrackertest.cpp index 6da23103e0f..eee52f4d679 100644 --- a/storage/src/tests/distributor/nodemaintenancestatstrackertest.cpp +++ b/storage/src/tests/distributor/nodemaintenancestatstrackertest.cpp @@ -1,11 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include +#include #include +#include -#include +namespace storage::distributor { -namespace storage { -namespace distributor { +using document::test::makeBucketSpace; +using document::BucketSpace; class NodeMaintenanceStatsTrackerTest : public CppUnit::TestFixture { @@ -14,12 +17,17 @@ class NodeMaintenanceStatsTrackerTest : public CppUnit::TestFixture CPPUNIT_TEST(statsFieldsAffectEqualityComparison); CPPUNIT_TEST(requestingNonExistingNodeGivesEmptyStats); CPPUNIT_TEST(statsAreTrackedPerNode); + CPPUNIT_TEST(statsAreTrackedPerBucketSpace); CPPUNIT_TEST_SUITE_END(); void emptyStatsInstancesAreEqual(); void statsFieldsAffectEqualityComparison(); void requestingNonExistingNodeGivesEmptyStats(); void statsAreTrackedPerNode(); + void statsAreTrackedPerBucketSpace(); + void assertBucketStats(BucketSpace bucketSpace, const NodeMaintenanceStatsTracker& tracker); + void assertBucketStats(uint64_t expMovingOut, uint64_t expSyncing, uint64_t expCopyingIn, uint64_t expCopyingOut, + BucketSpace bucketSpace, const NodeMaintenanceStatsTracker& tracker); }; CPPUNIT_TEST_SUITE_REGISTRATION(NodeMaintenanceStatsTrackerTest); @@ -64,7 +72,7 @@ NodeMaintenanceStatsTrackerTest::requestingNonExistingNodeGivesEmptyStats() { NodeMaintenanceStatsTracker tracker; NodeMaintenanceStats wanted; - CPPUNIT_ASSERT_EQUAL(wanted, tracker.forNode(0)); + CPPUNIT_ASSERT_EQUAL(wanted, tracker.forNode(0, makeBucketSpace())); } void @@ -72,30 +80,78 @@ NodeMaintenanceStatsTrackerTest::statsAreTrackedPerNode() { NodeMaintenanceStatsTracker tracker; NodeMaintenanceStats wanted; + BucketSpace space(1); - tracker.incMovingOut(0); + tracker.incMovingOut(0, space); wanted.movingOut = 1; - CPPUNIT_ASSERT_EQUAL(wanted, tracker.forNode(0)); + CPPUNIT_ASSERT_EQUAL(wanted, tracker.forNode(0, space)); wanted.movingOut = 0; - CPPUNIT_ASSERT_EQUAL(wanted, tracker.forNode(1)); + CPPUNIT_ASSERT_EQUAL(wanted, tracker.forNode(1, space)); - tracker.incMovingOut(0); + tracker.incMovingOut(0, space); wanted.movingOut = 2; - CPPUNIT_ASSERT_EQUAL(wanted, tracker.forNode(0)); + CPPUNIT_ASSERT_EQUAL(wanted, tracker.forNode(0, space)); - tracker.incMovingOut(1); + tracker.incMovingOut(1, space); wanted.movingOut = 1; - CPPUNIT_ASSERT_EQUAL(wanted, tracker.forNode(1)); + CPPUNIT_ASSERT_EQUAL(wanted, tracker.forNode(1, space)); - tracker.incSyncing(1); - tracker.incCopyingIn(1); - tracker.incCopyingOut(1); + tracker.incSyncing(1, space); + tracker.incCopyingIn(1, space); + tracker.incCopyingOut(1, space); wanted.syncing = 1; wanted.copyingIn = 1; wanted.copyingOut = 1; - CPPUNIT_ASSERT_EQUAL(wanted, tracker.forNode(1)); + CPPUNIT_ASSERT_EQUAL(wanted, tracker.forNode(1, space)); } -} // distributor -} // storage +void +NodeMaintenanceStatsTrackerTest::statsAreTrackedPerBucketSpace() +{ + NodeMaintenanceStatsTracker tracker; + BucketSpace fooSpace(3); + BucketSpace barSpace(5); + + tracker.incMovingOut(0, fooSpace); + assertBucketStats(1, 0, 0, 0, fooSpace, tracker); + assertBucketStats(barSpace, tracker); + + tracker.incMovingOut(0, barSpace); + assertBucketStats(1, 0, 0, 0, fooSpace, tracker); + assertBucketStats(1, 0, 0, 0, barSpace, tracker); + + tracker.incSyncing(0, fooSpace); + assertBucketStats(1, 1, 0, 0, fooSpace, tracker); + assertBucketStats(1, 0, 0, 0, barSpace, tracker); + + tracker.incCopyingIn(0, fooSpace); + assertBucketStats(1, 1, 1, 0, fooSpace, tracker); + assertBucketStats(1, 0, 0, 0, barSpace, tracker); + + tracker.incCopyingOut(0, fooSpace); + assertBucketStats(1, 1, 1, 1, fooSpace, tracker); + assertBucketStats(1, 0, 0, 0, barSpace, tracker); +} + +void +NodeMaintenanceStatsTrackerTest::assertBucketStats(BucketSpace bucketSpace, + const NodeMaintenanceStatsTracker& tracker) +{ + NodeMaintenanceStats expStats; + CPPUNIT_ASSERT_EQUAL(expStats, tracker.forNode(0, bucketSpace)); +} + +void +NodeMaintenanceStatsTrackerTest::assertBucketStats(uint64_t expMovingOut, + uint64_t expSyncing, + uint64_t expCopyingIn, + uint64_t expCopyingOut, + BucketSpace bucketSpace, + const NodeMaintenanceStatsTracker& tracker) +{ + NodeMaintenanceStats expStats(expMovingOut, expSyncing, expCopyingIn, expCopyingOut); + CPPUNIT_ASSERT_EQUAL(expStats, tracker.forNode(0, bucketSpace)); +} + +} diff --git a/storage/src/tests/distributor/simplemaintenancescannertest.cpp b/storage/src/tests/distributor/simplemaintenancescannertest.cpp index 394df6024fd..ac4a5bbfb91 100644 --- a/storage/src/tests/distributor/simplemaintenancescannertest.cpp +++ b/storage/src/tests/distributor/simplemaintenancescannertest.cpp @@ -1,15 +1,13 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include #include -#include -#include #include -#include +#include #include -#include -#include +#include +#include #include -#include namespace storage::distributor { @@ -225,7 +223,7 @@ SimpleMaintenanceScannerTest::perNodeMaintenanceStatsAreTracked() { auto stats(_scanner->getPendingMaintenanceStats()); NodeMaintenanceStats emptyStats; - CPPUNIT_ASSERT_EQUAL(emptyStats, stats.perNodeStats.forNode(0)); + CPPUNIT_ASSERT_EQUAL(emptyStats, stats.perNodeStats.forNode(0, makeBucketSpace())); } CPPUNIT_ASSERT(scanEntireDatabase(2)); // Mock is currently hardwired to increment movingOut for node 1 and @@ -234,12 +232,12 @@ SimpleMaintenanceScannerTest::perNodeMaintenanceStatsAreTracked() { NodeMaintenanceStats wantedNode1Stats; wantedNode1Stats.movingOut = 2; - CPPUNIT_ASSERT_EQUAL(wantedNode1Stats, stats.perNodeStats.forNode(1)); + CPPUNIT_ASSERT_EQUAL(wantedNode1Stats, stats.perNodeStats.forNode(1, makeBucketSpace())); } { NodeMaintenanceStats wantedNode2Stats; wantedNode2Stats.copyingIn = 2; - CPPUNIT_ASSERT_EQUAL(wantedNode2Stats, stats.perNodeStats.forNode(2)); + CPPUNIT_ASSERT_EQUAL(wantedNode2Stats, stats.perNodeStats.forNode(2, makeBucketSpace())); } } diff --git a/storage/src/tests/distributor/statecheckerstest.cpp b/storage/src/tests/distributor/statecheckerstest.cpp index 306f92cdd6a..1130a8ae2d2 100644 --- a/storage/src/tests/distributor/statecheckerstest.cpp +++ b/storage/src/tests/distributor/statecheckerstest.cpp @@ -1,36 +1,26 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include #include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include #include -#include -#include #include -#include #include #include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include using namespace std::literals::string_literals; using document::test::makeBucketSpace; using document::test::makeDocumentBucket; -namespace storage { -namespace distributor { +namespace storage::distributor { struct StateCheckersTest : public CppUnit::TestFixture, public DistributorTestUtil @@ -1812,7 +1802,7 @@ StateCheckersTest::statsUpdatedWhenMergingDueToMove() { NodeMaintenanceStats wanted; wanted.copyingOut = 1; - CPPUNIT_ASSERT_EQUAL(wanted, runner.stats().forNode(1)); + CPPUNIT_ASSERT_EQUAL(wanted, runner.stats().forNode(1, makeBucketSpace())); } // Moving 1 bucket from nodes {0, 2} into 3. // Note that we do not at this point in time distinguish _which_ of these @@ -1820,13 +1810,13 @@ StateCheckersTest::statsUpdatedWhenMergingDueToMove() { NodeMaintenanceStats wanted; wanted.copyingIn = 1; - CPPUNIT_ASSERT_EQUAL(wanted, runner.stats().forNode(3)); + CPPUNIT_ASSERT_EQUAL(wanted, runner.stats().forNode(3, makeBucketSpace())); } { NodeMaintenanceStats wanted; wanted.movingOut = 1; - CPPUNIT_ASSERT_EQUAL(wanted, runner.stats().forNode(0)); - CPPUNIT_ASSERT_EQUAL(wanted, runner.stats().forNode(2)); + CPPUNIT_ASSERT_EQUAL(wanted, runner.stats().forNode(0, makeBucketSpace())); + CPPUNIT_ASSERT_EQUAL(wanted, runner.stats().forNode(2, makeBucketSpace())); } } @@ -1842,12 +1832,12 @@ StateCheckersTest::statsUpdatedWhenMergingDueToMissingCopy() { NodeMaintenanceStats wanted; wanted.copyingIn = 1; - CPPUNIT_ASSERT_EQUAL(wanted, runner.stats().forNode(3)); + CPPUNIT_ASSERT_EQUAL(wanted, runner.stats().forNode(3, makeBucketSpace())); } { NodeMaintenanceStats wanted; wanted.copyingOut = 1; - CPPUNIT_ASSERT_EQUAL(wanted, runner.stats().forNode(1)); + CPPUNIT_ASSERT_EQUAL(wanted, runner.stats().forNode(1, makeBucketSpace())); } } @@ -1861,10 +1851,9 @@ StateCheckersTest::statsUpdatedWhenMergingDueToOutOfSyncCopies() { NodeMaintenanceStats wanted; wanted.syncing = 1; - CPPUNIT_ASSERT_EQUAL(wanted, runner.stats().forNode(1)); - CPPUNIT_ASSERT_EQUAL(wanted, runner.stats().forNode(3)); + CPPUNIT_ASSERT_EQUAL(wanted, runner.stats().forNode(1, makeBucketSpace())); + CPPUNIT_ASSERT_EQUAL(wanted, runner.stats().forNode(3, makeBucketSpace())); } } -} // distributor -} // storage} +} 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 b8fd294d7dc..3a65d2d5336 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 @@ -5,7 +5,7 @@ namespace storage::distributor { -const NodeMaintenanceStats NodeMaintenanceStatsTracker::_emptyStats; +const NodeMaintenanceStats NodeMaintenanceStatsTracker::_emptyNodeMaintenanceStats; std::ostream& operator<<(std::ostream& os, const NodeMaintenanceStats& stats) 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 b18cdcd37d9..7cf65e4a7d8 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 @@ -4,16 +4,25 @@ #include #include #include +#include namespace storage { namespace distributor { struct NodeMaintenanceStats { - uint64_t movingOut {0}; - uint64_t syncing {0}; - uint64_t copyingIn {0}; - uint64_t copyingOut {0}; + uint64_t movingOut; + uint64_t syncing; + uint64_t copyingIn; + uint64_t copyingOut; + + NodeMaintenanceStats() + : movingOut(0), syncing(0), copyingIn(0), copyingOut(0) + {} + + NodeMaintenanceStats(uint64_t movingOut_, uint64_t syncing_, uint64_t copyingIn_, uint64_t copyingOut_) + : movingOut(movingOut_), syncing(syncing_), copyingIn(copyingIn_), copyingOut(copyingOut_) + {} bool operator==(const NodeMaintenanceStats& other) const noexcept { return (movingOut == other.movingOut @@ -27,35 +36,47 @@ std::ostream& operator<<(std::ostream&, const NodeMaintenanceStats&); class NodeMaintenanceStatsTracker { - std::unordered_map _stats; - static const NodeMaintenanceStats _emptyStats; +public: + using BucketSpacesStats = std::unordered_map; + +private: + std::unordered_map _stats; + static const NodeMaintenanceStats _emptyNodeMaintenanceStats; + public: NodeMaintenanceStatsTracker(); ~NodeMaintenanceStatsTracker(); - void incMovingOut(uint16_t node) { - ++_stats[node].movingOut; + void incMovingOut(uint16_t node, document::BucketSpace bucketSpace) { + ++_stats[node][bucketSpace].movingOut; } - void incSyncing(uint16_t node) { - ++_stats[node].syncing; + void incSyncing(uint16_t node, document::BucketSpace bucketSpace) { + ++_stats[node][bucketSpace].syncing; } - void incCopyingIn(uint16_t node) { - ++_stats[node].copyingIn; + void incCopyingIn(uint16_t node, document::BucketSpace bucketSpace) { + ++_stats[node][bucketSpace].copyingIn; } - void incCopyingOut(uint16_t node) { - ++_stats[node].copyingOut; + void incCopyingOut(uint16_t node, document::BucketSpace bucketSpace) { + ++_stats[node][bucketSpace].copyingOut; } /** - * Returned statistics for a given node index, or all zero statistics + * 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) const { - auto iter = _stats.find(node); - return (iter != _stats.end() ? iter->second : _emptyStats); + const NodeMaintenanceStats& forNode(uint16_t node, document::BucketSpace bucketSpace) const { + auto nodeItr = _stats.find(node); + if (nodeItr != _stats.end()) { + auto bucketSpaceItr = nodeItr->second.find(bucketSpace); + if (bucketSpaceItr != nodeItr->second.end()) { + return bucketSpaceItr->second; + } + } + return _emptyNodeMaintenanceStats; } + }; } // distributor diff --git a/storage/src/vespa/storage/distributor/statechecker.h b/storage/src/vespa/storage/distributor/statechecker.h index e204cf5325a..9c0fe2ef53c 100644 --- a/storage/src/vespa/storage/distributor/statechecker.h +++ b/storage/src/vespa/storage/distributor/statechecker.h @@ -86,6 +86,7 @@ public: document::Bucket getBucket() const { return bucket; } document::BucketId getBucketId() const { return bucket.getBucketId(); } + document::BucketSpace getBucketSpace() const { return bucket.getBucketSpace(); } std::string toString() const; }; diff --git a/storage/src/vespa/storage/distributor/statecheckers.cpp b/storage/src/vespa/storage/distributor/statecheckers.cpp index 1f0cb19ef93..4d6e9d8164f 100644 --- a/storage/src/vespa/storage/distributor/statecheckers.cpp +++ b/storage/src/vespa/storage/distributor/statecheckers.cpp @@ -737,11 +737,11 @@ addStatisticsForNonIdealNodes(const StateChecker::Context& c, for (uint32_t j = 0; j < c.entry->getNodeCount(); ++j) { const uint16_t node(c.entry->getNodeRef(j).getNode()); if (!presentInIdealState(c, node)) { - c.stats.incMovingOut(node); + c.stats.incMovingOut(node, c.getBucketSpace()); } else if (missingReplica) { // Copy is in ideal location and we're missing a replica. Thus // we treat all ideal copies as sources to copy from. - c.stats.incCopyingOut(node); + c.stats.incCopyingOut(node, c.getBucketSpace()); } } } @@ -773,7 +773,7 @@ checkForNodesMissingFromIdealState(StateChecker::Context& c) ret.markMoveToIdealLocation(c.idealState[i], mp.mergeMoveToIdealNode); } - c.stats.incCopyingIn(c.idealState[i]); + c.stats.incCopyingIn(c.idealState[i], c.getBucketSpace()); hasMissingReplica = true; } } @@ -788,7 +788,7 @@ addStatisticsForOutOfSyncCopies(StateChecker::Context& c) const uint32_t n = c.entry->getNodeCount(); for (uint32_t i = 0; i < n; ++i) { const BucketCopy& cp(c.entry->getNodeRef(i)); - c.stats.incSyncing(cp.getNode()); + c.stats.incSyncing(cp.getNode(), c.getBucketSpace()); } } -- cgit v1.2.3 From dffd9223346b4b9ffe424f1b58a3c637d1aaf271 Mon Sep 17 00:00:00 2001 From: Geir Storli Date: Wed, 21 Feb 2018 16:15:35 +0000 Subject: Produce per node bucket spaces stats from the distributor based on node maintenance stats. --- storage/src/tests/distributor/distributortest.cpp | 21 ++++++++++++++ .../src/vespa/storage/distributor/distributor.cpp | 33 +++++++++++++++++++++- .../src/vespa/storage/distributor/distributor.h | 6 ++-- .../maintenance/node_maintenance_stats_tracker.h | 6 +++- 4 files changed, 60 insertions(+), 6 deletions(-) diff --git a/storage/src/tests/distributor/distributortest.cpp b/storage/src/tests/distributor/distributortest.cpp index 4d08e4adb71..776f4e200e8 100644 --- a/storage/src/tests/distributor/distributortest.cpp +++ b/storage/src/tests/distributor/distributortest.cpp @@ -94,6 +94,8 @@ protected: void internal_messages_are_started_in_fifo_order_batch(); void closing_aborts_priority_queued_client_requests(); + void assertBucketSpaceStats(size_t expBucketPending, uint16_t node, const vespalib::string &bucketSpace, + const BucketSpacesStatsProvider::PerNodeBucketSpacesStats &stats); std::vector _bucketSpaces; public: @@ -643,6 +645,25 @@ Distributor_Test::mergeStatsAreAccumulatedDuringDatabaseIteration() wanted.copyingIn = 2; CPPUNIT_ASSERT_EQUAL(wanted, stats.perNodeStats.forNode(2, makeBucketSpace())); } + auto bucketStats = _distributor->getBucketSpacesStats(); + CPPUNIT_ASSERT_EQUAL(static_cast(3), bucketStats.size()); + assertBucketSpaceStats(1, 0, "default", bucketStats); + assertBucketSpaceStats(0, 1, "default", bucketStats); + assertBucketSpaceStats(3, 2, "default", bucketStats); +} + +void +Distributor_Test::assertBucketSpaceStats(size_t expBucketPending, uint16_t node, const vespalib::string &bucketSpace, + const BucketSpacesStatsProvider::PerNodeBucketSpacesStats &stats) +{ + auto nodeItr = stats.find(node); + CPPUNIT_ASSERT(nodeItr != stats.end()); + CPPUNIT_ASSERT_EQUAL(static_cast(1), nodeItr->second.size()); + auto bucketSpaceItr = nodeItr->second.find(bucketSpace); + CPPUNIT_ASSERT(bucketSpaceItr != nodeItr->second.end()); + CPPUNIT_ASSERT(bucketSpaceItr->second.valid()); + CPPUNIT_ASSERT_EQUAL(static_cast(0), bucketSpaceItr->second.bucketsTotal()); + CPPUNIT_ASSERT_EQUAL(expBucketPending, bucketSpaceItr->second.bucketsPending()); } /** diff --git a/storage/src/vespa/storage/distributor/distributor.cpp b/storage/src/vespa/storage/distributor/distributor.cpp index 32554d02397..39658912a2e 100644 --- a/storage/src/vespa/storage/distributor/distributor.cpp +++ b/storage/src/vespa/storage/distributor/distributor.cpp @@ -93,6 +93,7 @@ Distributor::Distributor(DistributorComponentRegister& compReg, _metricUpdateHook(*this), _metricLock(), _maintenanceStats(), + _bucketSpacesStats(), _bucketDbStats(), _hostInfoReporter(_pendingMessageTracker.getLatencyStatisticsProvider(), *this, *this), _ownershipSafeTimeCalc( @@ -610,6 +611,13 @@ Distributor::getMinReplica() const return _bucketDbStats._minBucketReplica; } +BucketSpacesStatsProvider::PerNodeBucketSpacesStats +Distributor::getBucketSpacesStats() const +{ + vespalib::LockGuard guard(_metricLock); + return _bucketSpacesStats; +} + void Distributor::propagateInternalScanMetricsToExternal() { @@ -624,6 +632,29 @@ Distributor::propagateInternalScanMetricsToExternal() } } +namespace { + +BucketSpaceStats +toBucketSpaceStats(const NodeMaintenanceStats &stats) +{ + return BucketSpaceStats(0, stats.syncing + stats.copyingIn); +} + +BucketSpacesStatsProvider::PerNodeBucketSpacesStats +toBucketSpacesStats(const NodeMaintenanceStatsTracker &maintenanceStats) +{ + BucketSpacesStatsProvider::PerNodeBucketSpacesStats result; + for (const auto &nodeEntry : maintenanceStats.perNodeStats()) { + for (const auto &bucketSpaceEntry : nodeEntry.second) { + auto bucketSpace = document::FixedBucketSpaces::to_string(bucketSpaceEntry.first); + result[nodeEntry.first][bucketSpace] = toBucketSpaceStats(bucketSpaceEntry.second); + } + } + return result; +} + +} + void Distributor::updateInternalMetricsForCompletedScan() { @@ -632,7 +663,7 @@ Distributor::updateInternalMetricsForCompletedScan() _bucketDBMetricUpdater.completeRound(); _bucketDbStats = _bucketDBMetricUpdater.getLastCompleteStats(); _maintenanceStats = _scanner->getPendingMaintenanceStats(); - + _bucketSpacesStats = toBucketSpacesStats(_maintenanceStats.perNodeStats); } void diff --git a/storage/src/vespa/storage/distributor/distributor.h b/storage/src/vespa/storage/distributor/distributor.h index 1cf0a4f1866..b4b235838c5 100644 --- a/storage/src/vespa/storage/distributor/distributor.h +++ b/storage/src/vespa/storage/distributor/distributor.h @@ -198,10 +198,7 @@ private: */ std::unordered_map getMinReplica() const override; - PerNodeBucketSpacesStats getBucketSpacesStats() const override { - // TODO: implement - return BucketSpacesStatsProvider::PerNodeBucketSpacesStats(); - } + PerNodeBucketSpacesStats getBucketSpacesStats() const override; /** * Atomically publish internal metrics to external ideal state metrics. @@ -305,6 +302,7 @@ private: * manager thread but written by distributor thread. */ SimpleMaintenanceScanner::PendingMaintenanceStats _maintenanceStats; + BucketSpacesStatsProvider::PerNodeBucketSpacesStats _bucketSpacesStats; BucketDBMetricUpdater::Stats _bucketDbStats; DistributorHostInfoReporter _hostInfoReporter; std::unique_ptr _ownershipSafeTimeCalc; 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 7cf65e4a7d8..891e6fd7fa7 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 @@ -38,9 +38,10 @@ class NodeMaintenanceStatsTracker { public: using BucketSpacesStats = std::unordered_map; + using PerNodeStats = std::unordered_map; private: - std::unordered_map _stats; + PerNodeStats _stats; static const NodeMaintenanceStats _emptyNodeMaintenanceStats; public: @@ -77,6 +78,9 @@ public: return _emptyNodeMaintenanceStats; } + const PerNodeStats& perNodeStats() const { + return _stats; + } }; } // distributor -- cgit v1.2.3 From ec029adea55af2681d0293cb9b19165cf58ae389 Mon Sep 17 00:00:00 2001 From: Geir Storli Date: Thu, 22 Feb 2018 09:37:32 +0000 Subject: Rename function for clarity. --- storage/src/tests/distributor/nodemaintenancestatstrackertest.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/storage/src/tests/distributor/nodemaintenancestatstrackertest.cpp b/storage/src/tests/distributor/nodemaintenancestatstrackertest.cpp index eee52f4d679..5663db78e12 100644 --- a/storage/src/tests/distributor/nodemaintenancestatstrackertest.cpp +++ b/storage/src/tests/distributor/nodemaintenancestatstrackertest.cpp @@ -25,7 +25,7 @@ class NodeMaintenanceStatsTrackerTest : public CppUnit::TestFixture void requestingNonExistingNodeGivesEmptyStats(); void statsAreTrackedPerNode(); void statsAreTrackedPerBucketSpace(); - void assertBucketStats(BucketSpace bucketSpace, const NodeMaintenanceStatsTracker& tracker); + void assertEmptyBucketStats(BucketSpace bucketSpace, const NodeMaintenanceStatsTracker& tracker); void assertBucketStats(uint64_t expMovingOut, uint64_t expSyncing, uint64_t expCopyingIn, uint64_t expCopyingOut, BucketSpace bucketSpace, const NodeMaintenanceStatsTracker& tracker); }; @@ -114,7 +114,7 @@ NodeMaintenanceStatsTrackerTest::statsAreTrackedPerBucketSpace() tracker.incMovingOut(0, fooSpace); assertBucketStats(1, 0, 0, 0, fooSpace, tracker); - assertBucketStats(barSpace, tracker); + assertEmptyBucketStats(barSpace, tracker); tracker.incMovingOut(0, barSpace); assertBucketStats(1, 0, 0, 0, fooSpace, tracker); @@ -134,8 +134,8 @@ NodeMaintenanceStatsTrackerTest::statsAreTrackedPerBucketSpace() } void -NodeMaintenanceStatsTrackerTest::assertBucketStats(BucketSpace bucketSpace, - const NodeMaintenanceStatsTracker& tracker) +NodeMaintenanceStatsTrackerTest::assertEmptyBucketStats(BucketSpace bucketSpace, + const NodeMaintenanceStatsTracker& tracker) { NodeMaintenanceStats expStats; CPPUNIT_ASSERT_EQUAL(expStats, tracker.forNode(0, bucketSpace)); -- cgit v1.2.3