Make the noise level used when deciding whether to report resource usage configurable.

4 files changed, 59 insertions, 25 deletions

diff --git a/storage/src/tests/persistence/filestorage/service_layer_host_info_reporter_test.cpp b/storage/src/tests/persistence/filestorage/service_layer_host_info_reporter_test.cpp
index a54c5c8ccf9..e5ba04f789b 100644
--- a/storage/src/tests/persistence/filestorage/service_layer_host_info_reporter_test.cpp
+++ b/storage/src/tests/persistence/filestorage/service_layer_host_info_reporter_test.cpp
@@ -50,6 +50,9 @@ struct ServiceLayerHostInfoReporterTest : ::testing::Test {
     void notify(double disk_usage, double memory_usage) {
         notify(disk_usage, memory_usage, {0.0, ""}, {0.0, ""});
     }
+    void set_noise_level(double level) {
+        _reporter.set_noise_level(level);
+    }
 
     size_t requested_almost_immediate_replies() { return _state_manager.requested_almost_immediate_node_state_replies(); }
     ResourceUsage get_old_usage() { return _reporter.get_old_resource_usage(); }
@@ -102,26 +105,45 @@ TEST_F(ServiceLayerHostInfoReporterTest, request_almost_immediate_node_state_as_
     EXPECT_EQ(ResourceUsage(0.8, 0.7, {0.1, attr_es_name}, {0.2, attr_mv_name}), get_usage());
 }
 
+TEST_F(ServiceLayerHostInfoReporterTest, can_set_noise_level)
+{
+    set_noise_level(0.02);
+    notify(0.5, 0.4);
+    EXPECT_EQ(1, requested_almost_immediate_replies());
+    EXPECT_EQ(ResourceUsage(0.5, 0.4), get_old_usage());
+    EXPECT_EQ(ResourceUsage(0.5, 0.4), get_usage());
+    // the difference in disk usage is below the noise level
+    notify(0.519, 0.4);
+    EXPECT_EQ(1, requested_almost_immediate_replies());
+    EXPECT_EQ(ResourceUsage(0.5, 0.4), get_old_usage());
+    EXPECT_EQ(ResourceUsage(0.519, 0.4), get_usage());
+    // the difference in disk usage is above the noise level
+    notify(0.521, 0.4);
+    EXPECT_EQ(2, requested_almost_immediate_replies());
+    EXPECT_EQ(ResourceUsage(0.521, 0.4), get_old_usage());
+    EXPECT_EQ(ResourceUsage(0.521, 0.4), get_usage());
+}
+
 TEST_F(ServiceLayerHostInfoReporterTest,
-       first_valid_attribute_enum_store_sample_triggers_immediate_node_state_when_below_slack_diff)
+       first_valid_attribute_enum_store_sample_triggers_immediate_node_state_when_below_noise_level)
 {
-    // TODO: Assert this is below slack diff when that becomes configurable.
-    constexpr double usage_below_slack_diff = 0.00001;
-    notify(0.0, 0.0, {usage_below_slack_diff, attr_es_name}, {});
+    set_noise_level(0.02);
+    constexpr double usage_below_noise_level = 0.019;
+    notify(0.0, 0.0, {usage_below_noise_level, attr_es_name}, {});
     EXPECT_EQ(1, requested_almost_immediate_replies());
-    EXPECT_EQ(ResourceUsage(0.0, 0.0, {usage_below_slack_diff, attr_es_name}, {}), get_old_usage());
-    EXPECT_EQ(ResourceUsage(0.0, 0.0, {usage_below_slack_diff, attr_es_name}, {}), get_usage());
+    EXPECT_EQ(ResourceUsage(0.0, 0.0, {usage_below_noise_level, attr_es_name}, {}), get_old_usage());
+    EXPECT_EQ(ResourceUsage(0.0, 0.0, {usage_below_noise_level, attr_es_name}, {}), get_usage());
 }
 
 TEST_F(ServiceLayerHostInfoReporterTest,
-       first_valid_attribute_multi_value_sample_triggers_immediate_node_state_when_below_slack_diff)
+       first_valid_attribute_multi_value_sample_triggers_immediate_node_state_when_below_noise_level)
 {
-    // TODO: Assert this is below slack diff when that becomes configurable.
-    constexpr double usage_below_slack_diff = 0.00001;
-    notify(0.0, 0.0, {}, {usage_below_slack_diff, attr_mv_name});
+    set_noise_level(0.02);
+    constexpr double usage_below_noise_level = 0.019;
+    notify(0.0, 0.0, {}, {usage_below_noise_level, attr_mv_name});
     EXPECT_EQ(1, requested_almost_immediate_replies());
-    EXPECT_EQ(ResourceUsage(0.0, 0.0, {}, {usage_below_slack_diff, attr_mv_name}), get_old_usage());
-    EXPECT_EQ(ResourceUsage(0.0, 0.0, {}, {usage_below_slack_diff, attr_mv_name}), get_usage());
+    EXPECT_EQ(ResourceUsage(0.0, 0.0, {}, {usage_below_noise_level, attr_mv_name}), get_old_usage());
+    EXPECT_EQ(ResourceUsage(0.0, 0.0, {}, {usage_below_noise_level, attr_mv_name}), get_usage());
 }
 
 TEST_F(ServiceLayerHostInfoReporterTest, json_report_generated)
diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp
index 7c175686359..eee509e79f8 100644
--- a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp
+++ b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp
@@ -190,6 +190,7 @@ FileStorManager::configure(std::unique_ptr<vespa::config::content::StorFilestorC
     bool liveUpdate = ! _threads.empty();
 
     _use_async_message_handling_on_schedule = config->useAsyncMessageHandlingOnSchedule;
+    _host_info_reporter.set_noise_level(config->resourceUsageReporterNoiseLevel);
 
     if (!liveUpdate) {
         _config = std::move(config);
diff --git a/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.cpp b/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.cpp
index 88cdcc2b42f..97244582f50 100644
--- a/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.cpp
+++ b/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.cpp
@@ -16,8 +16,6 @@ using End = vespalib::JsonStream::End;
 
 namespace {
 
-constexpr double diff_slack = 0.001;
-
 const vespalib::string memory_label("memory");
 const vespalib::string disk_label("disk");
 const vespalib::string attribute_enum_store_label("attribute-enum-store");
@@ -38,7 +36,7 @@ void write_attribute_usage(vespalib::JsonStream& output, const vespalib::string
     output << End();
 }
 
-bool want_immediate_report(const spi::ResourceUsage& old_usage, const spi::ResourceUsage& new_usage)
+bool want_immediate_report(const spi::ResourceUsage& old_usage, const spi::ResourceUsage& new_usage, double noise_level)
 {
     auto disk_usage_diff = fabs(new_usage.get_disk_usage() - old_usage.get_disk_usage());
     auto memory_usage_diff = fabs(new_usage.get_memory_usage() - old_usage.get_memory_usage());
@@ -48,22 +46,24 @@ bool want_immediate_report(const spi::ResourceUsage& old_usage, const spi::Resou
             new_usage.get_attribute_enum_store_usage().valid();
     bool multivalue_got_valid = !old_usage.get_attribute_multivalue_usage().valid() &&
             new_usage.get_attribute_multivalue_usage().valid();
-    return ((disk_usage_diff > diff_slack) ||
-            (memory_usage_diff > diff_slack) ||
-            (enum_store_diff > diff_slack) ||
-            (multivalue_diff > diff_slack) ||
+    return ((disk_usage_diff > noise_level) ||
+            (memory_usage_diff > noise_level) ||
+            (enum_store_diff > noise_level) ||
+            (multivalue_diff > noise_level) ||
             enum_store_got_valid ||
             multivalue_got_valid);
 }
 
 }
 
-ServiceLayerHostInfoReporter::ServiceLayerHostInfoReporter(NodeStateUpdater& node_state_updater)
+ServiceLayerHostInfoReporter::ServiceLayerHostInfoReporter(NodeStateUpdater& node_state_updater,
+                                                           double noise_level)
     : HostReporter(),
       spi::ResourceUsageListener(),
       _node_state_updater(node_state_updater),
       _lock(),
-      _old_resource_usage()
+      _old_resource_usage(),
+      _noise_level(noise_level)
 {
 }
 
@@ -72,6 +72,12 @@ ServiceLayerHostInfoReporter::~ServiceLayerHostInfoReporter()
     spi::ResourceUsageListener::reset(); // detach
 }
 
+void
+ServiceLayerHostInfoReporter::set_noise_level(double level)
+{
+    _noise_level.store(level, std::memory_order_relaxed);
+}
+
 namespace {
 
 vespalib::string
@@ -87,9 +93,10 @@ to_string(const spi::ResourceUsage& usage)
 void
 ServiceLayerHostInfoReporter::update_resource_usage(const spi::ResourceUsage& resource_usage)
 {
-    bool immediate_report = want_immediate_report(_old_resource_usage, resource_usage);
-    LOG(debug, "update_resource_usage(): immediate_report=%s, old_usage=%s, new_usage=%s",
-        (immediate_report ? "true" : "false"), to_string(_old_resource_usage).c_str(),
+    double noise_level = _noise_level.load(std::memory_order_relaxed);
+    bool immediate_report = want_immediate_report(_old_resource_usage, resource_usage, noise_level);
+    LOG(debug, "update_resource_usage(): immediate_report=%s, noise_level=%f, old_usage=%s, new_usage=%s",
+        (immediate_report ? "true" : "false"), noise_level, to_string(_old_resource_usage).c_str(),
         to_string(resource_usage).c_str());
     if (immediate_report) {
         _old_resource_usage = resource_usage;
diff --git a/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.h b/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.h
index b58e047d0af..cc3af71fc26 100644
--- a/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.h
+++ b/storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.h
@@ -3,6 +3,7 @@
 
 #include <vespa/persistence/spi/resource_usage_listener.h>
 #include <vespa/storage/common/hostreporter/hostreporter.h>
+#include <atomic>
 #include <mutex>
 
 namespace storage {
@@ -18,15 +19,18 @@ class ServiceLayerHostInfoReporter : public HostReporter,
     NodeStateUpdater&  _node_state_updater;
     std::mutex         _lock;
     spi::ResourceUsage _old_resource_usage;
+    std::atomic<double> _noise_level;
 
     void update_resource_usage(const spi::ResourceUsage& resource_usage) override;
 public:
-    ServiceLayerHostInfoReporter(NodeStateUpdater& node_state_updater);
+    ServiceLayerHostInfoReporter(NodeStateUpdater& node_state_updater,
+                                 double noise_level = 0.001);
 
     ServiceLayerHostInfoReporter(const ServiceLayerHostInfoReporter&) = delete;
     ServiceLayerHostInfoReporter& operator=(const ServiceLayerHostInfoReporter&) = delete;
     ~ServiceLayerHostInfoReporter() override;
 
+    void set_noise_level(double level);
     void report(vespalib::JsonStream& output) override;
     const spi::ResourceUsage &get_old_resource_usage() noexcept { return _old_resource_usage; }
 };