From 503e23e723df498c9cd9202618fc4964583f8009 Mon Sep 17 00:00:00 2001 From: Johannes Schmidt Date: Tue, 19 May 2026 12:11:04 +0200 Subject: [PATCH] Freeze perfdata arrays and remove locks in code using them Since perfdata is set once when a check result is created and never changed again, locking this is unnecessary. This avoids components unnecessarily waiting on each other when processing perfdata. This fixes the locking cascade observed sometimes when the perfdata writer work queue blocks, where it extends to a lock on the entire check result eventually, affecting even more components. --- lib/icinga/checkresult.cpp | 8 ++++++++ lib/icinga/checkresult.hpp | 1 + lib/icinga/checkresult.ti | 2 +- lib/icinga/pluginutility.cpp | 2 -- lib/perfdata/elasticsearchwriter.cpp | 1 - lib/perfdata/gelfwriter.cpp | 1 - lib/perfdata/graphitewriter.cpp | 1 - lib/perfdata/influxdbcommonwriter.cpp | 1 - lib/perfdata/opentsdbwriter.cpp | 1 - lib/perfdata/otlpmetricswriter.cpp | 1 - test/icinga-perfdata.cpp | 5 +++++ test/perfdata-perfdatawriterfixture.hpp | 2 +- 12 files changed, 16 insertions(+), 10 deletions(-) diff --git a/lib/icinga/checkresult.cpp b/lib/icinga/checkresult.cpp index f55b9025b2f..49ab45def53 100644 --- a/lib/icinga/checkresult.cpp +++ b/lib/icinga/checkresult.cpp @@ -33,3 +33,11 @@ double CheckResult::CalculateLatency() const return latency; } + +void CheckResult::SetPerformanceData(const Array::Ptr& value, bool suppress_events, const Value& cookie) +{ + if (value) { + value->Freeze(); + } + ObjectImpl::SetPerformanceData(value, suppress_events, cookie); +} diff --git a/lib/icinga/checkresult.hpp b/lib/icinga/checkresult.hpp index b89f98d2a20..ac5cec5fc4b 100644 --- a/lib/icinga/checkresult.hpp +++ b/lib/icinga/checkresult.hpp @@ -22,6 +22,7 @@ class CheckResult final : public ObjectImpl double CalculateExecutionTime() const; double CalculateLatency() const; + void SetPerformanceData(const Array::Ptr& value, bool suppress_events = false, const Value& cookie = Empty) override; }; } diff --git a/lib/icinga/checkresult.ti b/lib/icinga/checkresult.ti index 75b54f0fd03..0feb96c706b 100644 --- a/lib/icinga/checkresult.ti +++ b/lib/icinga/checkresult.ti @@ -60,7 +60,7 @@ class CheckResult [state, enum] ServiceState "state"; [state, enum] ServiceState previous_hard_state; [state] String output; - [state] Array::Ptr performance_data; + [state, set_virtual] Array::Ptr performance_data; [state] bool active { default {{{ return true; }}} diff --git a/lib/icinga/pluginutility.cpp b/lib/icinga/pluginutility.cpp index 9ebf901e1d3..cf8d320d040 100644 --- a/lib/icinga/pluginutility.cpp +++ b/lib/icinga/pluginutility.cpp @@ -185,8 +185,6 @@ String PluginUtility::FormatPerfdata(const Array::Ptr& perfdata, bool normalize) std::ostringstream result; - ObjectLock olock(perfdata); - bool first = true; for (const Value& pdv : perfdata) { if (!first) diff --git a/lib/perfdata/elasticsearchwriter.cpp b/lib/perfdata/elasticsearchwriter.cpp index 9f5108ddd8d..ce79b6939cd 100644 --- a/lib/perfdata/elasticsearchwriter.cpp +++ b/lib/perfdata/elasticsearchwriter.cpp @@ -202,7 +202,6 @@ void ElasticsearchWriter::AddCheckResult(const Dictionary::Ptr& fields, const Ch CheckCommand::Ptr checkCommand = checkable->GetCheckCommand(); if (perfdata) { - ObjectLock olock(perfdata); for (const Value& val : perfdata) { PerfdataValue::Ptr pdv; diff --git a/lib/perfdata/gelfwriter.cpp b/lib/perfdata/gelfwriter.cpp index 6f8567f7073..70ba70a55aa 100644 --- a/lib/perfdata/gelfwriter.cpp +++ b/lib/perfdata/gelfwriter.cpp @@ -196,7 +196,6 @@ void GelfWriter::CheckResultHandler(const Checkable::Ptr& checkable, const Check Array::Ptr perfdata = cr->GetPerformanceData(); if (perfdata) { - ObjectLock olock(perfdata); for (const Value& val : perfdata) { PerfdataValue::Ptr pdv; diff --git a/lib/perfdata/graphitewriter.cpp b/lib/perfdata/graphitewriter.cpp index e00cd927589..8264291131e 100644 --- a/lib/perfdata/graphitewriter.cpp +++ b/lib/perfdata/graphitewriter.cpp @@ -224,7 +224,6 @@ void GraphiteWriter::SendPerfdata(const Checkable::Ptr& checkable, const String& CheckCommand::Ptr checkCommand = checkable->GetCheckCommand(); - ObjectLock olock(perfdata); for (const Value& val : perfdata) { PerfdataValue::Ptr pdv; diff --git a/lib/perfdata/influxdbcommonwriter.cpp b/lib/perfdata/influxdbcommonwriter.cpp index 5b6f376de9b..1472c24e31b 100644 --- a/lib/perfdata/influxdbcommonwriter.cpp +++ b/lib/perfdata/influxdbcommonwriter.cpp @@ -213,7 +213,6 @@ void InfluxdbCommonWriter::CheckResultHandler(const Checkable::Ptr& checkable, c double ts = cr->GetExecutionEnd(); if (Array::Ptr perfdata = cr->GetPerformanceData()) { - ObjectLock olock(perfdata); for (const Value& val : perfdata) { PerfdataValue::Ptr pdv; diff --git a/lib/perfdata/opentsdbwriter.cpp b/lib/perfdata/opentsdbwriter.cpp index 1b2f82a7d9c..6bc3001e81f 100644 --- a/lib/perfdata/opentsdbwriter.cpp +++ b/lib/perfdata/opentsdbwriter.cpp @@ -308,7 +308,6 @@ void OpenTsdbWriter::AddPerfdata(const Checkable::Ptr& checkable, const String& CheckCommand::Ptr checkCommand = checkable->GetCheckCommand(); - ObjectLock olock(perfdata); for (const Value& val : perfdata) { PerfdataValue::Ptr pdv; diff --git a/lib/perfdata/otlpmetricswriter.cpp b/lib/perfdata/otlpmetricswriter.cpp index 5c60904be29..136cdc4f03a 100644 --- a/lib/perfdata/otlpmetricswriter.cpp +++ b/lib/perfdata/otlpmetricswriter.cpp @@ -181,7 +181,6 @@ void OTLPMetricsWriter::CheckResultHandler(const Checkable::Ptr& checkable, cons auto endTime = cr->GetExecutionEnd(); Array::Ptr perfdata = cr->GetPerformanceData(); - ObjectLock olock(perfdata); for (const Value& val : perfdata) { PerfdataValue::Ptr pdv; if (val.IsObjectType()) { diff --git a/test/icinga-perfdata.cpp b/test/icinga-perfdata.cpp index d1e7e75db2c..25516d88672 100644 --- a/test/icinga-perfdata.cpp +++ b/test/icinga-perfdata.cpp @@ -38,6 +38,7 @@ BOOST_AUTO_TEST_CASE(quotes) BOOST_AUTO_TEST_CASE(multiple) { Array::Ptr pd = PluginUtility::SplitPerfdata("testA=123456 testB=123456"); + pd->Freeze(); BOOST_CHECK_EQUAL(pd->GetLength(), 2); String str = PluginUtility::FormatPerfdata(pd); @@ -47,12 +48,14 @@ BOOST_AUTO_TEST_CASE(multiple) BOOST_AUTO_TEST_CASE(multiline) { Array::Ptr pd = PluginUtility::SplitPerfdata(" 'testA'=123456 'testB'=123456"); + pd->Freeze(); BOOST_CHECK_EQUAL(pd->GetLength(), 2); String str = PluginUtility::FormatPerfdata(pd); BOOST_CHECK_EQUAL(str, "testA=123456 testB=123456"); pd = PluginUtility::SplitPerfdata(" 'testA'=123456 \n'testB'=123456"); + pd->Freeze(); BOOST_CHECK_EQUAL(pd->GetLength(), 2); str = PluginUtility::FormatPerfdata(pd); @@ -62,6 +65,7 @@ BOOST_AUTO_TEST_CASE(multiline) BOOST_AUTO_TEST_CASE(normalize) { Array::Ptr pd = PluginUtility::SplitPerfdata("testA=2m;3;4;1;5 testB=2foobar"); + pd->Freeze(); BOOST_CHECK_EQUAL(pd->GetLength(), 2); String str = PluginUtility::FormatPerfdata(pd, true); @@ -333,6 +337,7 @@ BOOST_AUTO_TEST_CASE(ignore_warn_crit_ranges) BOOST_AUTO_TEST_CASE(empty_warn_crit_min_max) { Array::Ptr pd = PluginUtility::SplitPerfdata("testA=5;;7;1;9 testB=5;7;;1;9 testC=5;;;1;9 testD=2m;;;1 testE=5;;7;;"); + pd->Freeze(); BOOST_CHECK_EQUAL(pd->GetLength(), 5); String str = PluginUtility::FormatPerfdata(pd, true); diff --git a/test/perfdata-perfdatawriterfixture.hpp b/test/perfdata-perfdatawriterfixture.hpp index e70b2123c84..ddbdabdbb55 100644 --- a/test/perfdata-perfdatawriterfixture.hpp +++ b/test/perfdata-perfdatawriterfixture.hpp @@ -96,7 +96,7 @@ object Host "h1" { std::size_t unchangedCount = 0; while(true){ ReceiveCheckResults(10, ServiceCritical, [&](const CheckResult::Ptr& cr) { - cr->GetPerformanceData()->Add(new PerfdataValue{GetRandomString("", 4096), 1}); + cr->SetPerformanceData(new Array{new PerfdataValue{GetRandomString("", 4096), 1}}); }); if (std::chrono::steady_clock::now() - start >= timeout) {