From 2a9fda0c0f8c47c8fe513ac5b19c79aadba00c42 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 2 Feb 2024 09:49:22 +0000 Subject: Always report hostinfo --- .../distributor_host_info_reporter_test.cpp | 14 ------------- .../distributor/top_level_distributor_test.cpp | 21 ++++---------------- .../storage/config/distributorconfiguration.cpp | 2 -- .../storage/config/distributorconfiguration.h | 5 ----- .../storage/config/stor-distributormanager.def | 10 ---------- .../distributor/distributor_host_info_reporter.cpp | 9 +-------- .../distributor/distributor_host_info_reporter.h | 23 +--------------------- .../storage/distributor/top_level_distributor.cpp | 4 +--- 8 files changed, 7 insertions(+), 81 deletions(-) (limited to 'storage') 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 ecdb901a06a..e9c1cea38f3 100644 --- a/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp +++ b/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp @@ -196,20 +196,6 @@ TEST_F(DistributorHostInfoReporterTest, generate_example_json) { EXPECT_EQ(goldenSlime, jsonSlime); } -TEST_F(DistributorHostInfoReporterTest, no_report_generated_if_disabled) { - Fixture f; - f.reporter.enableReporting(false); - - MinReplicaStats minReplica; - minReplica[0] = 2; - minReplica[5] = 9; - f.minReplicaProvider.minReplica = minReplica; - - vespalib::Slime root; - util::reporterToSlime(f.reporter, root); - EXPECT_EQ(0, root.get().children()); -} - TEST_F(DistributorHostInfoReporterTest, bucket_spaces_stats_are_reported) { Fixture f; PerNodeBucketSpacesStats stats; diff --git a/storage/src/tests/distributor/top_level_distributor_test.cpp b/storage/src/tests/distributor/top_level_distributor_test.cpp index 9b859f59625..4e44abe71fb 100644 --- a/storage/src/tests/distributor/top_level_distributor_test.cpp +++ b/storage/src/tests/distributor/top_level_distributor_test.cpp @@ -57,7 +57,7 @@ struct TopLevelDistributorTest : Test, TopLevelDistributorTestUtil { using Redundancy = int; using ConfigBuilder = vespa::config::content::core::StorDistributormanagerConfigBuilder; - std::string resolve_stripe_operation_routing(std::shared_ptr msg) { + std::string resolve_stripe_operation_routing(const std::shared_ptr & msg) { handle_top_level_message(msg); vespalib::asciistream posted_msgs; @@ -106,7 +106,7 @@ struct TopLevelDistributorTest : Test, TopLevelDistributorTestUtil { return _node->getNodeStateUpdater().explicit_node_state_reply_send_invocations(); } - std::shared_ptr make_dummy_remove_command() { + static std::shared_ptr make_dummy_remove_command() { return std::make_shared( makeDocumentBucket(document::BucketId(0)), document::DocumentId("id:foo:testdoctype1:n=1:foo"), @@ -374,8 +374,8 @@ void TopLevelDistributorTest::reply_to_1_node_bucket_info_fetch_with_n_buckets(s if (bucket_req.getBucketSpace() == FixedBucketSpaces::default_space()) { auto& bucket_reply = dynamic_cast(*reply); for (size_t j = 1; j <= n; ++j) { - bucket_reply.getBucketInfo().push_back(api::RequestBucketInfoReply::Entry( - document::BucketId(16, j), api::BucketInfo(20, 10, 12, 50, 60, true, true))); + bucket_reply.getBucketInfo().emplace_back(document::BucketId(16, j), + api::BucketInfo(20, 10, 12, 50, 60, true, true)); } } handle_top_level_message(std::move(reply)); @@ -474,19 +474,6 @@ TEST_F(TopLevelDistributorTest, non_bootstrap_host_info_send_request_delays_send EXPECT_EQ(2, explicit_node_state_reply_send_invocations()); } -TEST_F(TopLevelDistributorTest, host_info_reporter_config_is_propagated_to_reporter) { - setup_distributor(Redundancy(2), NodeCount(2), "storage:2 distributor:1"); - - // Default is enabled=true. - EXPECT_TRUE(distributor_host_info_reporter().isReportingEnabled()); - - auto cfg = current_distributor_config(); - cfg.enableHostInfoReporting = false; - reconfigure(cfg); - - EXPECT_FALSE(distributor_host_info_reporter().isReportingEnabled()); -} - namespace { void assert_invalid_bucket_stats_for_all_spaces( diff --git a/storage/src/vespa/storage/config/distributorconfiguration.cpp b/storage/src/vespa/storage/config/distributorconfiguration.cpp index 7800eb625e3..8e5ca4dab2c 100644 --- a/storage/src/vespa/storage/config/distributorconfiguration.cpp +++ b/storage/src/vespa/storage/config/distributorconfiguration.cpp @@ -39,7 +39,6 @@ DistributorConfiguration::DistributorConfiguration(StorageComponent& component) _doInlineSplit(true), _enableJoinForSiblingLessBuckets(false), _enableInconsistentJoin(false), - _enableHostInfoReporting(true), _disableBucketActivation(false), _sequenceMutatingOperations(true), _allowStaleReadsDuringClusterStateTransitions(false), @@ -138,7 +137,6 @@ DistributorConfiguration::configure(const vespa::config::content::core::StorDist _enableJoinForSiblingLessBuckets = config.enableJoinForSiblingLessBuckets; _enableInconsistentJoin = config.enableInconsistentJoin; - _enableHostInfoReporting = config.enableHostInfoReporting; _disableBucketActivation = config.disableBucketActivation; _sequenceMutatingOperations = config.sequenceMutatingOperations; _allowStaleReadsDuringClusterStateTransitions = config.allowStaleReadsDuringClusterStateTransitions; diff --git a/storage/src/vespa/storage/config/distributorconfiguration.h b/storage/src/vespa/storage/config/distributorconfiguration.h index 330567953bd..c9ae51d89e4 100644 --- a/storage/src/vespa/storage/config/distributorconfiguration.h +++ b/storage/src/vespa/storage/config/distributorconfiguration.h @@ -168,10 +168,6 @@ public: return _enableInconsistentJoin; } - bool getEnableHostInfoReporting() const noexcept { - return _enableHostInfoReporting; - } - using ReplicaCountingMode = DistrConfig::MinimumReplicaCountingMode; ReplicaCountingMode getMinimumReplicaCountingMode() const noexcept { @@ -321,7 +317,6 @@ private: bool _doInlineSplit; bool _enableJoinForSiblingLessBuckets; bool _enableInconsistentJoin; - bool _enableHostInfoReporting; bool _disableBucketActivation; bool _sequenceMutatingOperations; bool _allowStaleReadsDuringClusterStateTransitions; diff --git a/storage/src/vespa/storage/config/stor-distributormanager.def b/storage/src/vespa/storage/config/stor-distributormanager.def index 3136a54d080..d586cca56b1 100644 --- a/storage/src/vespa/storage/config/stor-distributormanager.def +++ b/storage/src/vespa/storage/config/stor-distributormanager.def @@ -63,16 +63,6 @@ enable_join_for_sibling_less_buckets bool default=false ## being joined into. enable_inconsistent_join bool default=false -## The distributor host info reporter may be disabled entirely, in which case -## no per-node statistics for merges, latencies or bucket replication factors -## will be reported back to the cluster controller. Disabling this may make -## sense in large clusters that do not make use of these reports directly or -## indirectly, as it causes potentially significant processing overhead on the -## cluster controller. -## This host reporter must never be disabled on a Hosted Vespa system, or -## automatic upgrades will stall. -enable_host_info_reporting bool default=true - ## For each available node, the distributor will report back to the cluster ## controller a value which indicates the minimum replication factor for any ## bucket contained on said node. This config exposes a way to alter how this 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 918156c3048..330107e811c 100644 --- a/storage/src/vespa/storage/distributor/distributor_host_info_reporter.cpp +++ b/storage/src/vespa/storage/distributor/distributor_host_info_reporter.cpp @@ -17,8 +17,7 @@ DistributorHostInfoReporter::DistributorHostInfoReporter( MinReplicaProvider& minReplicaProvider, BucketSpacesStatsProvider& bucketSpacesStatsProvider) : _minReplicaProvider(minReplicaProvider), - _bucketSpacesStatsProvider(bucketSpacesStatsProvider), - _enabled(true) + _bucketSpacesStatsProvider(bucketSpacesStatsProvider) { } @@ -80,19 +79,13 @@ outputStorageNodes(vespalib::JsonStream& output, void DistributorHostInfoReporter::report(vespalib::JsonStream& output) { - if (!isReportingEnabled()) { - return; - } - auto minReplica = _minReplicaProvider.getMinReplica(); auto bucketSpacesStats = _bucketSpacesStatsProvider.getBucketSpacesStats(); output << "distributor" << Object(); { output << "storage-nodes" << Array(); - outputStorageNodes(output, minReplica, bucketSpacesStats); - output << End(); } 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 ab1fbe5feea..52921deb4b9 100644 --- a/storage/src/vespa/storage/distributor/distributor_host_info_reporter.h +++ b/storage/src/vespa/storage/distributor/distributor_host_info_reporter.h @@ -2,13 +2,11 @@ #pragma once #include -#include namespace storage::distributor { class BucketSpacesStatsProvider; class MinReplicaProvider; -struct OperationStats; class DistributorHostInfoReporter : public HostReporter { @@ -17,32 +15,13 @@ public: BucketSpacesStatsProvider& bucketSpacesStatsProvider); DistributorHostInfoReporter(const DistributorHostInfoReporter&) = delete; - DistributorHostInfoReporter& operator=( - const DistributorHostInfoReporter&) = delete; + DistributorHostInfoReporter& operator=(const DistributorHostInfoReporter&) = delete; void report(vespalib::JsonStream& output) override; - /** - * Set wether per-node latency, replication factors, merge stats etc are - * to be included in the generated JSON report. - * - * Thread safe. - */ - void enableReporting(bool enabled) noexcept { - _enabled.store(enabled, std::memory_order_relaxed); - } - - /** - * Thread safe. - */ - bool isReportingEnabled() const noexcept { - return _enabled.load(std::memory_order_relaxed); - } - private: MinReplicaProvider& _minReplicaProvider; BucketSpacesStatsProvider& _bucketSpacesStatsProvider; - std::atomic _enabled; }; } // storage::distributor diff --git a/storage/src/vespa/storage/distributor/top_level_distributor.cpp b/storage/src/vespa/storage/distributor/top_level_distributor.cpp index 710f554df4b..aa02ec9b7d3 100644 --- a/storage/src/vespa/storage/distributor/top_level_distributor.cpp +++ b/storage/src/vespa/storage/distributor/top_level_distributor.cpp @@ -109,10 +109,9 @@ TopLevelDistributor::TopLevelDistributor(DistributorComponentRegister& compReg, _bucket_db_status_delegate = std::make_unique(compReg, *this, *_bucket_db_updater); _bucket_db_status_delegate->registerStatusPage(); - _hostInfoReporter.enableReporting(config().getEnableHostInfoReporting()); hostInfoReporterRegistrar.registerReporter(&_hostInfoReporter); propagate_default_distribution_thread_unsafe(_component.getDistribution()); // Stripes not started yet -}; +} TopLevelDistributor::~TopLevelDistributor() { @@ -437,7 +436,6 @@ TopLevelDistributor::enable_next_config_if_changed() auto guard = _stripe_accessor->rendezvous_and_hold_all(); guard->update_total_distributor_config(_component.total_distributor_config_sp()); } - _hostInfoReporter.enableReporting(config().getEnableHostInfoReporting()); _maintenance_safe_time_delay = _total_config->getMaxClusterClockSkew(); _current_internal_config_generation = _component.internal_config_generation(); } -- cgit v1.2.3