From 3d088ac0ee4e2019eb8d3626fa4d3c19e67f105a Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 16 Dec 2022 09:31:47 +0000 Subject: No need to restart sampling or reconfigure anything unless the config has changed. --- .../disk_mem_usage_filter_test.cpp | 14 +++++++++++--- .../proton/server/disk_mem_usage_filter.cpp | 4 +++- .../searchcore/proton/server/disk_mem_usage_filter.h | 9 +++++++-- .../proton/server/disk_mem_usage_sampler.cpp | 19 ++++++++++++------- .../searchcore/proton/server/disk_mem_usage_sampler.h | 1 + 5 files changed, 34 insertions(+), 13 deletions(-) diff --git a/searchcore/src/tests/proton/server/disk_mem_usage_filter/disk_mem_usage_filter_test.cpp b/searchcore/src/tests/proton/server/disk_mem_usage_filter/disk_mem_usage_filter_test.cpp index dd38765c4f0..ccee7caa917 100644 --- a/searchcore/src/tests/proton/server/disk_mem_usage_filter/disk_mem_usage_filter_test.cpp +++ b/searchcore/src/tests/proton/server/disk_mem_usage_filter/disk_mem_usage_filter_test.cpp @@ -65,9 +65,17 @@ assertResourceUsage(double usage, double limit, double utilization, const Resour EXPECT_DOUBLE_EQ(utilization, state.utilization()); } +TEST_F(DiskMemUsageFilterTest, reconfig_with_identical_config_is_noop) +{ + EXPECT_TRUE(_filter.setConfig(Config(1.0, 0.8))); + assertResourceUsage(0.2, 0.8, 0.25, _filter.usageState().diskState()); + EXPECT_FALSE(_filter.setConfig(Config(1.0, 0.8))); + assertResourceUsage(0.2, 0.8, 0.25, _filter.usageState().diskState()); +} + TEST_F(DiskMemUsageFilterTest, disk_limit_can_be_reached) { - _filter.setConfig(Config(1.0, 0.8)); + EXPECT_TRUE(_filter.setConfig(Config(1.0, 0.8))); assertResourceUsage(0.2, 0.8, 0.25, _filter.usageState().diskState()); triggerDiskLimit(); testWrite("diskLimitReached: { " @@ -80,7 +88,7 @@ TEST_F(DiskMemUsageFilterTest, disk_limit_can_be_reached) TEST_F(DiskMemUsageFilterTest, memory_limit_can_be_reached) { - _filter.setConfig(Config(0.8, 1.0)); + EXPECT_TRUE(_filter.setConfig(Config(0.8, 1.0))); assertResourceUsage(0.3, 0.8, 0.375, _filter.usageState().memoryState()); triggerMemoryLimit(); testWrite("memoryLimitReached: { " @@ -95,7 +103,7 @@ TEST_F(DiskMemUsageFilterTest, memory_limit_can_be_reached) TEST_F(DiskMemUsageFilterTest, both_disk_limit_and_memory_limit_can_be_reached) { - _filter.setConfig(Config(0.8, 0.8)); + EXPECT_TRUE(_filter.setConfig(Config(0.8, 0.8))); triggerMemoryLimit(); triggerDiskLimit(); testWrite("memoryLimitReached: { " diff --git a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.cpp b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.cpp index 54eea6565cb..fc1d23741c2 100644 --- a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.cpp @@ -188,12 +188,14 @@ DiskMemUsageFilter::set_resource_usage(const TransientResourceUsage& transient_u recalcState(guard); } -void +bool DiskMemUsageFilter::setConfig(Config config_in) { Guard guard(_lock); + if (_config == config_in) return false; _config = config_in; recalcState(guard); + return true; } vespalib::ProcessMemoryStats diff --git a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.h b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.h index 16e4d1d6869..cc901fa72cf 100644 --- a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.h +++ b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.h @@ -24,7 +24,6 @@ namespace proton { class DiskMemUsageFilter : public IResourceWriteFilter, public IDiskMemUsageNotifier { public: - using space_info = std::filesystem::space_info; using Mutex = std::mutex; using Guard = std::lock_guard; @@ -39,6 +38,12 @@ public: : _memoryLimit(memoryLimit_in), _diskLimit(diskLimit_in) { } + bool operator == (const Config & rhs) const noexcept { + return (_memoryLimit == rhs._memoryLimit) && (_diskLimit == rhs._diskLimit); + } + bool operator != (const Config & rhs) const noexcept { + return ! (*this == rhs); + } }; private: @@ -66,7 +71,7 @@ public: DiskMemUsageFilter(const HwInfo &hwInfo); ~DiskMemUsageFilter() override; void set_resource_usage(const TransientResourceUsage& transient_usage, vespalib::ProcessMemoryStats memoryStats, uint64_t diskUsedSizeBytes); - void setConfig(Config config); + [[nodiscard]] bool setConfig(Config config); vespalib::ProcessMemoryStats getMemoryStats() const; uint64_t getDiskUsedSize() const; TransientResourceUsage get_transient_resource_usage() const; diff --git a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.cpp b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.cpp index 102d947e812..3e35e42d25a 100644 --- a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.cpp @@ -29,21 +29,25 @@ DiskMemUsageSampler::close() { _periodicHandle.reset(); } +bool +DiskMemUsageSampler::timeToSampleAgain() const noexcept { + return vespalib::steady_clock::now() >= (_lastSampleTime + _sampleInterval); +} + void DiskMemUsageSampler::setConfig(const Config &config, IScheduledExecutor & executor) { - _periodicHandle.reset(); - _filter.setConfig(config.filterConfig); + bool wasChanged = _filter.setConfig(config.filterConfig); + if (_periodicHandle && (_sampleInterval == config.sampleInterval) && !wasChanged) + return; _sampleInterval = config.sampleInterval; + _periodicHandle.reset(); sampleAndReportUsage(); - _lastSampleTime = vespalib::steady_clock::now(); vespalib::duration maxInterval = std::min(vespalib::duration(1s), _sampleInterval); _periodicHandle = executor.scheduleAtFixedRate(makeLambdaTask([this]() { - if (_filter.acceptWriteOperation() && (vespalib::steady_clock::now() < (_lastSampleTime + _sampleInterval))) { - return; + if (!_filter.acceptWriteOperation() || timeToSampleAgain()) { + sampleAndReportUsage(); } - sampleAndReportUsage(); - _lastSampleTime = vespalib::steady_clock::now(); }), maxInterval, maxInterval); } @@ -60,6 +64,7 @@ DiskMemUsageSampler::sampleAndReportUsage() vespalib::ProcessMemoryStats memoryStats = sampleMemoryUsage(); uint64_t diskUsage = sampleDiskUsage(); _filter.set_resource_usage(transientUsage, memoryStats, diskUsage); + _lastSampleTime = vespalib::steady_clock::now(); } namespace { diff --git a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.h b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.h index 16c89a253fd..d434c529836 100644 --- a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.h +++ b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.h @@ -27,6 +27,7 @@ class DiskMemUsageSampler { uint64_t sampleDiskUsage(); vespalib::ProcessMemoryStats sampleMemoryUsage(); TransientResourceUsage sample_transient_resource_usage(); + [[nodiscard]] bool timeToSampleAgain() const noexcept; public: struct Config { DiskMemUsageFilter::Config filterConfig; -- cgit v1.2.3