diff options
9 files changed, 23 insertions, 211 deletions
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/StorageNode.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/StorageNode.java index bf7c1fad6ca..f53c3eed28a 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/StorageNode.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/StorageNode.java @@ -14,31 +14,6 @@ import java.util.List; */ public class StorageNode { - static public class Put { - private final Long latencyMsSum; - private final Long count; - - @JsonCreator - public Put(@JsonProperty("latency-ms-sum") Long latencyMsSum, @JsonProperty("count") Long count) { - this.latencyMsSum = latencyMsSum; - this.count = count; - } - - public Long getLatencyMsSum() { return latencyMsSum; } - public Long getCount() { return count; } - } - - static public class OpsLatency { - private final Put put; - - @JsonCreator - public OpsLatency(@JsonProperty("put") Put put) { - this.put = put; - } - - public Put getPut() { return put; } - } - static public class BucketStats { private final long total; private final long pending; @@ -80,9 +55,6 @@ public class StorageNode { private final Integer index; - @JsonProperty("ops-latency") - private OpsLatency opsLatencies; - // If a Distributor does not manage any bucket copies for a particular storage node, // then the distributor will not return any min-current-replication-factor for that // storage node. @@ -101,10 +73,6 @@ public class StorageNode { return index; } - public OpsLatency getOpsLatenciesOrNull() { - return opsLatencies; - } - // See documentation on minCurrentReplicationFactor. public Integer getMinCurrentReplicationFactorOrNull() { return minCurrentReplicationFactor; diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/HostInfoTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/HostInfoTest.java index 9a69b998976..b96f5282971 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/HostInfoTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/HostInfoTest.java @@ -48,9 +48,6 @@ public class HostInfoTest { List<StorageNode> storageNodeList = hostInfo.getDistributor().getStorageNodes(); assertThat(storageNodeList.size(), is(2)); assertThat(storageNodeList.get(0).getIndex(), is(0)); - assertThat(storageNodeList.get(0).getOpsLatenciesOrNull().getPut().getCount(), is(16L)); - assertThat(storageNodeList.get(1).getOpsLatenciesOrNull().getPut().getCount(), is(18L)); - assertThat(storageNodeList.get(0).getOpsLatenciesOrNull().getPut().getLatencyMsSum(), is(15L)); List<Metrics.Metric> metrics = hostInfo.getMetrics().getMetrics(); assertThat(metrics.size(), is(2)); Metrics.Value value = metrics.get(0).getValue(); @@ -93,15 +90,9 @@ public class HostInfoTest { assertTrue(storageNodeByIndex.containsKey(0)); assertThat(storageNodeByIndex.get(0).getIndex(), is(0)); assertThat(storageNodeByIndex.get(0).getMinCurrentReplicationFactorOrNull(), is(2)); - assertNotNull(storageNodeByIndex.get(0).getOpsLatenciesOrNull()); - assertThat(storageNodeByIndex.get(0).getOpsLatenciesOrNull().getPut().getLatencyMsSum(), is(10000L)); - assertThat(storageNodeByIndex.get(0).getOpsLatenciesOrNull().getPut().getCount(), is(3L)); assertTrue(storageNodeByIndex.containsKey(5)); assertThat(storageNodeByIndex.get(5).getIndex(), is(5)); assertThat(storageNodeByIndex.get(5).getMinCurrentReplicationFactorOrNull(), is(9)); - assertNotNull(storageNodeByIndex.get(5).getOpsLatenciesOrNull()); - assertThat(storageNodeByIndex.get(5).getOpsLatenciesOrNull().getPut().getLatencyMsSum(), is(25000L)); - assertThat(storageNodeByIndex.get(5).getOpsLatenciesOrNull().getPut().getCount(), is(7L)); } } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/StateRestApiTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/StateRestApiTest.java index 5079f91a2b9..46a50800c53 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/StateRestApiTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/StateRestApiTest.java @@ -125,43 +125,19 @@ public abstract class StateRestApiTest { " \"storage-nodes\": [\n" + " {\n" + " \"node-index\": 1,\n" + - " \"min-current-replication-factor\": 2,\n" + - " \"ops-latency\": {\n" + - " \"put\": {\n" + - " \"latency-ms-sum\": 6,\n" + - " \"count\": 7\n" + - " }\n" + - " }\n" + + " \"min-current-replication-factor\": 2\n" + " },\n" + " {\n" + " \"node-index\": 3,\n" + - " \"min-current-replication-factor\": 2,\n" + - " \"ops-latency\": {\n" + - " \"put\": {\n" + - " \"latency-ms-sum\": 5,\n" + - " \"count\": 4\n" + - " }\n" + - " }\n" + + " \"min-current-replication-factor\": 2\n" + " },\n" + " {\n" + " \"node-index\": 5,\n" + - " \"min-current-replication-factor\": 2,\n" + - " \"ops-latency\": {\n" + - " \"put\": {\n" + - " \"latency-ms-sum\": 4,\n" + - " \"count\": 5\n" + - " }\n" + - " }\n" + + " \"min-current-replication-factor\": 2\n" + " },\n" + " {\n" + " \"node-index\": 7,\n" + - " \"min-current-replication-factor\": 2,\n" + - " \"ops-latency\": {\n" + - " \"put\": {\n" + - " \"latency-ms-sum\": 6,\n" + - " \"count\": 7\n" + - " }\n" + - " }\n" + + " \"min-current-replication-factor\": 2\n" + " }\n" + " ]\n" + " }\n" + diff --git a/protocols/getnodestate/distributor.json b/protocols/getnodestate/distributor.json index 970fd09f253..1db812bc78c 100644 --- a/protocols/getnodestate/distributor.json +++ b/protocols/getnodestate/distributor.json @@ -4,12 +4,6 @@ { "node-index": 0, "min-current-replication-factor": 2, - "ops-latency" : { - "put": { - "latency-ms-sum": 10000, - "count": 3 - } - }, "bucket-spaces" : [ { "name": "default", @@ -30,12 +24,6 @@ { "node-index": 5, "min-current-replication-factor": 9, - "ops-latency" : { - "put": { - "latency-ms-sum": 25000, - "count": 7 - } - }, "bucket-spaces" : [ { "name": "default" diff --git a/protocols/getnodestate/host_info.json b/protocols/getnodestate/host_info.json index 305f279c847..5bfa4f3171c 100644 --- a/protocols/getnodestate/host_info.json +++ b/protocols/getnodestate/host_info.json @@ -42,12 +42,6 @@ { "node-index": 0, "min-current-replication-factor": 2, - "ops-latency" : { - "put": { - "latency-ms-sum": 15, - "count": 16 - } - }, "bucket-spaces": [ { "name": "default", @@ -68,12 +62,6 @@ { "node-index": 1, "min-current-replication-factor": 2, - "ops-latency": { - "put": { - "latency-ms-sum": 17, - "count": 18 - } - }, "bucket-spaces": [ { "name": "default" diff --git a/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp b/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp index 64df05acb8f..df01b3e30a7 100644 --- a/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp +++ b/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp @@ -21,19 +21,13 @@ using Object = vespalib::JsonStream::Object; class DistributorHostInfoReporterTest : public CppUnit::TestFixture { CPPUNIT_TEST_SUITE(DistributorHostInfoReporterTest); - CPPUNIT_TEST(hostInfoWithPutLatenciesOnly); - CPPUNIT_TEST(hostInfoAllInfo); - CPPUNIT_TEST(generateExampleJson); - CPPUNIT_TEST(noReportGeneratedIfDisabled); + CPPUNIT_TEST(min_replica_stats_are_reported); + CPPUNIT_TEST(generate_example_json); + CPPUNIT_TEST(no_report_generated_if_disabled); CPPUNIT_TEST(bucket_spaces_stats_are_reported); CPPUNIT_TEST_SUITE_END(); - void hostInfoWithPutLatenciesOnly(); - void hostInfoAllInfo(); - void verifyReportedNodeLatencies(const vespalib::Slime& root, - uint16_t nodeIndex, - int64_t latencySum, - int64_t count); + void min_replica_stats_are_reported(); void verifyBucketSpaceStats(const vespalib::Slime& root, uint16_t nodeIndex, const vespalib::string& bucketSpaceName, @@ -42,8 +36,8 @@ class DistributorHostInfoReporterTest : public CppUnit::TestFixture void verifyBucketSpaceStats(const vespalib::Slime& root, uint16_t nodeIndex, const vespalib::string& bucketSpaceName); - void generateExampleJson(); - void noReportGeneratedIfDisabled(); + void generate_example_json(); + void no_report_generated_if_disabled(); void bucket_spaces_stats_are_reported(); }; @@ -53,25 +47,7 @@ using ms = std::chrono::milliseconds; namespace { -OperationStats -makeOpStats(std::chrono::milliseconds totalLatency, uint64_t numRequests) -{ - OperationStats stats; - stats.totalLatency = totalLatency; - stats.numRequests = numRequests; - return stats; -} - // My kingdom for GoogleMock! -struct MockedLatencyStatisticsProvider : LatencyStatisticsProvider -{ - NodeStatsSnapshot returnedSnapshot; - - NodeStatsSnapshot doGetLatencyStatistics() const override { - return returnedSnapshot; - } -}; - struct MockedMinReplicaProvider : MinReplicaProvider { std::unordered_map<uint16_t, uint32_t> minReplica; @@ -110,12 +86,6 @@ getMinReplica(const vespalib::Slime& root, uint16_t nodeIndex) } const vespalib::slime::Inspector& -getLatenciesForNode(const vespalib::Slime& root, uint16_t nodeIndex) -{ - return getNode(root, nodeIndex)["ops-latency"]; -} - -const vespalib::slime::Inspector& getBucketSpaceStats(const vespalib::Slime& root, uint16_t nodeIndex, const vespalib::string& bucketSpaceName) { const auto& bucketSpaces = getNode(root, nodeIndex)["bucket-spaces"]; @@ -130,18 +100,6 @@ getBucketSpaceStats(const vespalib::Slime& root, uint16_t nodeIndex, const vespa } void -DistributorHostInfoReporterTest::verifyReportedNodeLatencies(const vespalib::Slime& root, - uint16_t nodeIndex, - int64_t latencySum, - int64_t count) -{ - auto& latencies = getLatenciesForNode(root, nodeIndex); - CPPUNIT_ASSERT_EQUAL(latencySum, - latencies["put"]["latency-ms-sum"].asLong()); - CPPUNIT_ASSERT_EQUAL(count, latencies["put"]["count"].asLong()); -} - -void DistributorHostInfoReporterTest::verifyBucketSpaceStats(const vespalib::Slime& root, uint16_t nodeIndex, const vespalib::string& bucketSpaceName, @@ -164,43 +122,21 @@ DistributorHostInfoReporterTest::verifyBucketSpaceStats(const vespalib::Slime& r } struct Fixture { - MockedLatencyStatisticsProvider latencyStatsProvider; MockedMinReplicaProvider minReplicaProvider; MockedBucketSpacesStatsProvider bucketSpacesStatsProvider; DistributorHostInfoReporter reporter; Fixture() - : latencyStatsProvider(), - minReplicaProvider(), + : minReplicaProvider(), bucketSpacesStatsProvider(), - reporter(latencyStatsProvider, minReplicaProvider, bucketSpacesStatsProvider) + reporter(minReplicaProvider, bucketSpacesStatsProvider) {} ~Fixture() {} }; void -DistributorHostInfoReporterTest::hostInfoWithPutLatenciesOnly() +DistributorHostInfoReporterTest::min_replica_stats_are_reported() { Fixture f; - NodeStatsSnapshot snapshot; - snapshot.nodeToStats[0] = { makeOpStats(ms(10000), 3) }; - snapshot.nodeToStats[5] = { makeOpStats(ms(25000), 7) }; - - f.latencyStatsProvider.returnedSnapshot = snapshot; - - vespalib::Slime root; - util::reporterToSlime(f.reporter, root); - verifyReportedNodeLatencies(root, 0, 10000, 3); - verifyReportedNodeLatencies(root, 5, 25000, 7); -} - -void -DistributorHostInfoReporterTest::hostInfoAllInfo() -{ - Fixture f; - NodeStatsSnapshot latencySnapshot; - latencySnapshot.nodeToStats[0] = { makeOpStats(ms(10000), 3) }; - latencySnapshot.nodeToStats[5] = { makeOpStats(ms(25000), 7) }; - f.latencyStatsProvider.returnedSnapshot = latencySnapshot; std::unordered_map<uint16_t, uint32_t> minReplica; minReplica[0] = 2; @@ -209,21 +145,15 @@ DistributorHostInfoReporterTest::hostInfoAllInfo() vespalib::Slime root; util::reporterToSlime(f.reporter, root); - verifyReportedNodeLatencies(root, 0, 10000, 3); - verifyReportedNodeLatencies(root, 5, 25000, 7); CPPUNIT_ASSERT_EQUAL(2, getMinReplica(root, 0)); CPPUNIT_ASSERT_EQUAL(9, getMinReplica(root, 5)); } void -DistributorHostInfoReporterTest::generateExampleJson() +DistributorHostInfoReporterTest::generate_example_json() { Fixture f; - NodeStatsSnapshot snapshot; - snapshot.nodeToStats[0] = { makeOpStats(ms(10000), 3) }; - snapshot.nodeToStats[5] = { makeOpStats(ms(25000), 7) }; - f.latencyStatsProvider.returnedSnapshot = snapshot; std::unordered_map<uint16_t, uint32_t> minReplica; minReplica[0] = 2; @@ -261,16 +191,15 @@ DistributorHostInfoReporterTest::generateExampleJson() } void -DistributorHostInfoReporterTest::noReportGeneratedIfDisabled() +DistributorHostInfoReporterTest::no_report_generated_if_disabled() { Fixture f; f.reporter.enableReporting(false); - NodeStatsSnapshot snapshot; - snapshot.nodeToStats[0] = { makeOpStats(ms(10000), 3) }; - snapshot.nodeToStats[5] = { makeOpStats(ms(25000), 7) }; - - f.latencyStatsProvider.returnedSnapshot = snapshot; + std::unordered_map<uint16_t, uint32_t> minReplica; + minReplica[0] = 2; + minReplica[5] = 9; + f.minReplicaProvider.minReplica = minReplica; vespalib::Slime root; util::reporterToSlime(f.reporter, root); diff --git a/storage/src/vespa/storage/distributor/distributor.cpp b/storage/src/vespa/storage/distributor/distributor.cpp index ca0ca80bb4e..d698d07fa34 100644 --- a/storage/src/vespa/storage/distributor/distributor.cpp +++ b/storage/src/vespa/storage/distributor/distributor.cpp @@ -96,7 +96,7 @@ Distributor::Distributor(DistributorComponentRegister& compReg, _maintenanceStats(), _bucketSpacesStats(), _bucketDbStats(), - _hostInfoReporter(_pendingMessageTracker.getLatencyStatisticsProvider(), *this, *this), + _hostInfoReporter(*this, *this), _ownershipSafeTimeCalc( std::make_unique<OwnershipTransferSafeTimePointCalculator>( std::chrono::seconds(0))) // Set by config later diff --git a/storage/src/vespa/storage/distributor/distributor_host_info_reporter.cpp b/storage/src/vespa/storage/distributor/distributor_host_info_reporter.cpp index 8d2e45655d2..3a588179eb9 100644 --- a/storage/src/vespa/storage/distributor/distributor_host_info_reporter.cpp +++ b/storage/src/vespa/storage/distributor/distributor_host_info_reporter.cpp @@ -20,11 +20,9 @@ using Array = vespalib::JsonStream::Array; using End = vespalib::JsonStream::End; DistributorHostInfoReporter::DistributorHostInfoReporter( - LatencyStatisticsProvider& latencyProvider, MinReplicaProvider& minReplicaProvider, BucketSpacesStatsProvider& bucketSpacesStatsProvider) - : _latencyProvider(latencyProvider), - _minReplicaProvider(minReplicaProvider), + : _minReplicaProvider(minReplicaProvider), _bucketSpacesStatsProvider(bucketSpacesStatsProvider), _enabled(true) { @@ -33,16 +31,6 @@ DistributorHostInfoReporter::DistributorHostInfoReporter( namespace { void -writeOperationStats(vespalib::JsonStream& stream, - const OperationStats& stats) -{ - stream << "put" << Object() - << "latency-ms-sum" << stats.totalLatency.count() - << "count" << stats.numRequests - << End(); -} - -void writeBucketSpacesStats(vespalib::JsonStream& stream, const BucketSpacesStats& stats) { @@ -60,14 +48,10 @@ writeBucketSpacesStats(vespalib::JsonStream& stream, void outputStorageNodes(vespalib::JsonStream& output, - const unordered_map<uint16_t, NodeStats>& nodeStats, const unordered_map<uint16_t, uint32_t>& minReplica, const PerNodeBucketSpacesStats& bucketSpacesStats) { set<uint16_t> nodes; - for (const auto& element : nodeStats) { - nodes.insert(element.first); - } for (const auto& element : minReplica) { nodes.insert(element.first); } @@ -80,15 +64,6 @@ outputStorageNodes(vespalib::JsonStream& output, { output << "node-index" << node; - auto nodeStatsIt = nodeStats.find(node); - if (nodeStatsIt != nodeStats.end()) { - output << "ops-latency" << Object(); - { - writeOperationStats(output, nodeStatsIt->second.puts); - } - output << End(); - } - auto minReplicaIt = minReplica.find(node); if (minReplicaIt != minReplica.end()) { output << "min-current-replication-factor" @@ -115,7 +90,6 @@ DistributorHostInfoReporter::report(vespalib::JsonStream& output) return; } - auto nodeStats = _latencyProvider.getLatencyStatistics(); auto minReplica = _minReplicaProvider.getMinReplica(); auto bucketSpacesStats = _bucketSpacesStatsProvider.getBucketSpacesStats(); @@ -123,7 +97,7 @@ DistributorHostInfoReporter::report(vespalib::JsonStream& output) { output << "storage-nodes" << Array(); - outputStorageNodes(output, nodeStats.nodeToStats, minReplica, bucketSpacesStats); + outputStorageNodes(output, minReplica, bucketSpacesStats); output << End(); } 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 3e6a02120c2..f664b44135d 100644 --- a/storage/src/vespa/storage/distributor/distributor_host_info_reporter.h +++ b/storage/src/vespa/storage/distributor/distributor_host_info_reporter.h @@ -15,8 +15,7 @@ struct OperationStats; class DistributorHostInfoReporter : public HostReporter { public: - DistributorHostInfoReporter(LatencyStatisticsProvider& latencyProvider, - MinReplicaProvider& minReplicaProvider, + DistributorHostInfoReporter(MinReplicaProvider& minReplicaProvider, BucketSpacesStatsProvider& bucketSpacesStatsProvider); DistributorHostInfoReporter(const DistributorHostInfoReporter&) = delete; @@ -43,7 +42,6 @@ public: } private: - LatencyStatisticsProvider& _latencyProvider; MinReplicaProvider& _minReplicaProvider; BucketSpacesStatsProvider& _bucketSpacesStatsProvider; std::atomic<bool> _enabled; |