From 449a3c14cf404c3c10ff86ee568a8a5f7acbbf3d Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Mon, 17 Oct 2022 17:24:11 +0200 Subject: [PATCH 1/9] Allow intrusive pointers to ApplyRule --- lib/config/applyrule.hpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/config/applyrule.hpp b/lib/config/applyrule.hpp index 10520bfbd6c..5001d26cd3b 100644 --- a/lib/config/applyrule.hpp +++ b/lib/config/applyrule.hpp @@ -6,6 +6,7 @@ #include "config/i2-config.hpp" #include "config/expression.hpp" #include "base/debuginfo.hpp" +#include "base/shared-object.hpp" #include "base/type.hpp" #include @@ -15,9 +16,11 @@ namespace icinga /** * @ingroup config */ -class ApplyRule +class ApplyRule : public SharedObject { public: + DECLARE_PTR_TYPEDEFS(ApplyRule); + typedef std::map > TypeMap; typedef std::unordered_map>> RuleMap; From fd7ac4e5cae10501b510187687d3cdb92d717c7f Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Tue, 18 Oct 2022 10:26:13 +0200 Subject: [PATCH 2/9] Allow hashmaps of String --- lib/base/string.cpp | 5 +++++ lib/base/string.hpp | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/lib/base/string.cpp b/lib/base/string.cpp index eec5b8372d6..e4e5c273c24 100644 --- a/lib/base/string.cpp +++ b/lib/base/string.cpp @@ -461,3 +461,8 @@ String::ConstIterator icinga::range_end(const String& x) { return x.End(); } + +std::size_t std::hash::operator()(const String& s) const noexcept +{ + return std::hash{}(s.GetData()); +} diff --git a/lib/base/string.hpp b/lib/base/string.hpp index b9290eeee7c..10ddaf9779e 100644 --- a/lib/base/string.hpp +++ b/lib/base/string.hpp @@ -7,6 +7,7 @@ #include "base/object.hpp" #include #include +#include #include #include @@ -178,6 +179,12 @@ String::ConstIterator range_end(const String& x); } +template<> +struct std::hash +{ + std::size_t operator()(const icinga::String& s) const noexcept; +}; + extern template class std::vector; namespace boost From a56ad38ad3501fcd66d2d9f925b76d59d6210fdb Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Tue, 18 Oct 2022 16:48:58 +0200 Subject: [PATCH 3/9] Separately handle apply rules targetting only specific parent objects not to unnecessarily run e.g. the filter assign where host.name=="example.com" for all hosts being not example.com. --- lib/config/CMakeLists.txt | 2 +- lib/config/applyrule-targeted.cpp | 266 +++++++++++++++++++++++++ lib/config/applyrule.cpp | 45 ++++- lib/config/applyrule.hpp | 40 +++- lib/config/expression.hpp | 10 + lib/icinga/dependency-apply.cpp | 10 + lib/icinga/notification-apply.cpp | 10 + lib/icinga/scheduleddowntime-apply.cpp | 10 + lib/icinga/service-apply.cpp | 5 + 9 files changed, 387 insertions(+), 11 deletions(-) create mode 100644 lib/config/applyrule-targeted.cpp diff --git a/lib/config/CMakeLists.txt b/lib/config/CMakeLists.txt index 042668dc30b..80b8c2c4420 100644 --- a/lib/config/CMakeLists.txt +++ b/lib/config/CMakeLists.txt @@ -21,7 +21,7 @@ include_directories(${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR}) set(config_SOURCES i2-config.hpp activationcontext.cpp activationcontext.hpp - applyrule.cpp applyrule.hpp + applyrule.cpp applyrule-targeted.cpp applyrule.hpp configcompiler.cpp configcompiler.hpp configcompilercontext.cpp configcompilercontext.hpp configfragment.hpp diff --git a/lib/config/applyrule-targeted.cpp b/lib/config/applyrule-targeted.cpp new file mode 100644 index 00000000000..1a2ce564e42 --- /dev/null +++ b/lib/config/applyrule-targeted.cpp @@ -0,0 +1,266 @@ +/* Icinga 2 | (c) 2022 Icinga GmbH | GPLv2+ */ + +#include "base/string.hpp" +#include "config/applyrule.hpp" +#include "config/expression.hpp" +#include +#include + +using namespace icinga; + +/** + * @returns All ApplyRules targeting only specific parent objects including the given host. (See AddTargetedRule().) + */ +const std::vector& ApplyRule::GetTargetedHostRules(const Type::Ptr& sourceType, const Type::Ptr& targetType, const String& host) +{ + auto perSourceType (m_Rules.find(sourceType.get())); + + if (perSourceType != m_Rules.end()) { + auto perTargetType (perSourceType->second.find(targetType.get())); + + if (perTargetType != perSourceType->second.end()) { + auto perHost (perTargetType->second.Targeted.find(host)); + + if (perHost != perTargetType->second.Targeted.end()) { + return perHost->second.ForHost; + } + } + } + + static const std::vector noRules; + return noRules; +} + +/** + * @returns All ApplyRules targeting only specific parent objects including the given service. (See AddTargetedRule().) + */ +const std::vector& ApplyRule::GetTargetedServiceRules(const Type::Ptr& sourceType, const Type::Ptr& targetType, const String& host, const String& service) +{ + auto perSourceType (m_Rules.find(sourceType.get())); + + if (perSourceType != m_Rules.end()) { + auto perTargetType (perSourceType->second.find(targetType.get())); + + if (perTargetType != perSourceType->second.end()) { + auto perHost (perTargetType->second.Targeted.find(host)); + + if (perHost != perTargetType->second.Targeted.end()) { + auto perService (perHost->second.ForServices.find(service)); + + if (perService != perHost->second.ForServices.end()) { + return perService->second; + } + } + } + } + + static const std::vector noRules; + return noRules; +} + +/** + * If the given ApplyRule targets only specific parent objects, add it to the respective "index". + * + * - The above means for apply T "N" to Host: assign where host.name == "H" [ || host.name == "h" ... ] + * - For apply T "N" to Service it means: assign where host.name == "H" && service.name == "S" [ || host.name == "h" && service.name == "s" ... ] + * + * The order of operands of || && == doesn't matter. + * + * @returns Whether the rule has been added to the "index". + */ +bool ApplyRule::AddTargetedRule(ApplyRule& rule, const String& sourceType, const String& targetType, ApplyRule::PerTypes& rules) +{ + if (targetType == "Host") { + std::vector hosts; + + if (GetTargetHosts(rule.m_Filter.get(), hosts)) { + ApplyRule::Ptr sharedRule = new ApplyRule(std::move(rule)); + + for (auto host : hosts) { + rules.Targeted[*host].ForHost.emplace_back(sharedRule); + } + + return true; + } + } else if (targetType == "Service") { + std::vector> services; + + if (GetTargetServices(rule.m_Filter.get(), services)) { + ApplyRule::Ptr sharedRule = new ApplyRule(std::move(rule)); + + for (auto service : services) { + rules.Targeted[*service.first].ForServices[*service.second].emplace_back(sharedRule); + } + + return true; + } + } + + return false; +} + +/** + * If the given assign filter is like the following, extract the host names ("H", "h", ...) into the vector: + * + * host.name == "H" [ || host.name == "h" ... ] + * + * The order of operands of || == doesn't matter. + * + * @returns Whether the given assign filter is like above. + */ +bool ApplyRule::GetTargetHosts(Expression* assignFilter, std::vector& hosts) +{ + auto lor (dynamic_cast(assignFilter)); + + if (lor) { + return GetTargetHosts(lor->GetOperand1().get(), hosts) + && GetTargetHosts(lor->GetOperand2().get(), hosts); + } + + auto name (GetComparedName(assignFilter, "host")); + + if (name) { + hosts.emplace_back(name); + return true; + } + + return false; +} + +/** + * If the given assign filter is like the following, extract the host+service names ("H"+"S", "h"+"s", ...) into the vector: + * + * host.name == "H" && service.name == "S" [ || host.name == "h" && service.name == "s" ... ] + * + * The order of operands of || && == doesn't matter. + * + * @returns Whether the given assign filter is like above. + */ +bool ApplyRule::GetTargetServices(Expression* assignFilter, std::vector>& services) +{ + auto lor (dynamic_cast(assignFilter)); + + if (lor) { + return GetTargetServices(lor->GetOperand1().get(), services) + && GetTargetServices(lor->GetOperand2().get(), services); + } + + auto service (GetTargetService(assignFilter)); + + if (service.first) { + services.emplace_back(service); + return true; + } + + return false; +} + +/** + * If the given filter is like the following, extract the host+service names ("H"+"S"): + * + * host.name == "H" && service.name == "S" + * + * The order of operands of && == doesn't matter. + * + * @returns {host, service} on success and {nullptr, nullptr} on failure. + */ +std::pair ApplyRule::GetTargetService(Expression* assignFilter) +{ + auto land (dynamic_cast(assignFilter)); + + if (!land) { + return {nullptr, nullptr}; + } + + auto op1 (land->GetOperand1().get()); + auto op2 (land->GetOperand2().get()); + auto host (GetComparedName(op1, "host")); + + if (!host) { + std::swap(op1, op2); + host = GetComparedName(op1, "host"); + } + + if (host) { + auto service (GetComparedName(op2, "service")); + + if (service) { + return {host, service}; + } + } + + return {nullptr, nullptr}; +} + +/** + * If the given filter is like the following, extract the object name ("N"): + * + * $lcType$.name == "N" + * + * The order of operands of == doesn't matter. + * + * @returns The object name on success and nullptr on failure. + */ +const String * ApplyRule::GetComparedName(Expression* assignFilter, const char * lcType) +{ + auto eq (dynamic_cast(assignFilter)); + + if (!eq) { + return nullptr; + } + + auto op1 (eq->GetOperand1().get()); + auto op2 (eq->GetOperand2().get()); + + if (IsNameIndexer(op1, lcType)) { + return GetLiteralStringValue(op2); + } + + if (IsNameIndexer(op2, lcType)) { + return GetLiteralStringValue(op1); + } + + return nullptr; +} + +/** + * @returns Whether the given expression is like $lcType$.name. + */ +bool ApplyRule::IsNameIndexer(Expression* exp, const char * lcType) +{ + auto ixr (dynamic_cast(exp)); + + if (!ixr) { + return false; + } + + auto var (dynamic_cast(ixr->GetOperand1().get())); + + if (!var || var->GetVariable() != lcType) { + return false; + } + + auto val (GetLiteralStringValue(ixr->GetOperand2().get())); + + return val && *val == "name"; +} + +/** + * @returns If the given expression is a string literal, the string. nullptr on failure. + */ +const String * ApplyRule::GetLiteralStringValue(Expression* exp) +{ + auto lit (dynamic_cast(exp)); + + if (!lit) { + return nullptr; + } + + auto& val (lit->GetValue()); + + if (!val.IsString()) { + return nullptr; + } + + return &val.Get(); +} diff --git a/lib/config/applyrule.cpp b/lib/config/applyrule.cpp index 363dce90b82..cd268269e9b 100644 --- a/lib/config/applyrule.cpp +++ b/lib/config/applyrule.cpp @@ -3,6 +3,7 @@ #include "config/applyrule.hpp" #include "base/logger.hpp" #include +#include using namespace icinga; @@ -80,9 +81,12 @@ void ApplyRule::AddRule(const String& sourceType, const String& targetType, cons } } - m_Rules[Type::GetByName(sourceType).get()][Type::GetByName(*actualTargetType).get()].emplace_back(ApplyRule( - name, expression, filter, package, fkvar, fvvar, fterm, ignoreOnError, di, scope - )); + ApplyRule rule (name, expression, filter, package, fkvar, fvvar, fterm, ignoreOnError, di, scope); + auto& rules (m_Rules[Type::GetByName(sourceType).get()][Type::GetByName(*actualTargetType).get()]); + + if (!AddTargetedRule(rule, sourceType, *actualTargetType, rules)) { + rules.Regular.emplace_back(std::move(rule)); + } } bool ApplyRule::EvaluateFilter(ScriptFrame& frame) const @@ -148,7 +152,7 @@ std::vector& ApplyRule::GetRules(const Type::Ptr& sourceType, const T auto perTargetType (perSourceType->second.find(targetType.get())); if (perTargetType != perSourceType->second.end()) { - return perTargetType->second; + return perTargetType->second.Regular; } } @@ -160,13 +164,36 @@ void ApplyRule::CheckMatches(bool silent) { for (auto& perSourceType : m_Rules) { for (auto& perTargetType : perSourceType.second) { - for (auto& rule : perTargetType.second) { - if (!rule.HasMatches() && !silent) { - Log(LogWarning, "ApplyRule") - << "Apply rule '" << rule.GetName() << "' (" << rule.GetDebugInfo() << ") for type '" - << perSourceType.first->GetName() << "' does not match anywhere!"; + for (auto& rule : perTargetType.second.Regular) { + CheckMatches(rule, perSourceType.first, silent); + } + + std::unordered_set targeted; + + for (auto& perHost : perTargetType.second.Targeted) { + for (auto& rule : perHost.second.ForHost) { + targeted.emplace(rule.get()); + } + + for (auto& perService : perHost.second.ForServices) { + for (auto& rule : perService.second) { + targeted.emplace(rule.get()); + } } } + + for (auto rule : targeted) { + CheckMatches(*rule, perSourceType.first, silent); + } } } } + +void ApplyRule::CheckMatches(const ApplyRule& rule, Type* sourceType, bool silent) +{ + if (!rule.HasMatches() && !silent) { + Log(LogWarning, "ApplyRule") + << "Apply rule '" << rule.GetName() << "' (" << rule.GetDebugInfo() << ") for type '" + << sourceType->GetName() << "' does not match anywhere!"; + } +} diff --git a/lib/config/applyrule.hpp b/lib/config/applyrule.hpp index 5001d26cd3b..37ec04cfd88 100644 --- a/lib/config/applyrule.hpp +++ b/lib/config/applyrule.hpp @@ -21,8 +21,35 @@ class ApplyRule : public SharedObject public: DECLARE_PTR_TYPEDEFS(ApplyRule); + struct PerHost + { + std::vector ForHost; + std::unordered_map> ForServices; + }; + + struct PerTypes + { + std::vector Regular; + std::unordered_map Targeted; + }; + + /* + * m_Rules[T::TypeInstance.get()][Host::TypeInstance.get()].Targeted["H"].ForHost + * contains all apply rules like apply T "x" to Host { ... } + * which target only specific hosts incl. "H", e. g. via + * assign where host.name == "H" || host.name == "h". + * + * m_Rules[T::TypeInstance.get()][Service::TypeInstance.get()].Targeted["H"].ForServices["S"] + * contains all apply rules like apply T "x" to Service { ... } + * which target only specific services on specific hosts incl. "H!S", + * e. g. via assign where host.name == "H" && service.name == "S". + * + * m_Rules[T::TypeInstance.get()][C::TypeInstance.get()].Regular + * contains all other apply rules like apply T "x" to C { ... }. + */ + typedef std::unordered_map> RuleMap; + typedef std::map > TypeMap; - typedef std::unordered_map>> RuleMap; String GetName() const; Expression::Ptr GetExpression() const; @@ -43,6 +70,8 @@ class ApplyRule : public SharedObject const Expression::Ptr& filter, const String& package, const String& fkvar, const String& fvvar, const Expression::Ptr& fterm, bool ignoreOnError, const DebugInfo& di, const Dictionary::Ptr& scope); static std::vector& GetRules(const Type::Ptr& sourceType, const Type::Ptr& targetType); + static const std::vector& GetTargetedHostRules(const Type::Ptr& sourceType, const Type::Ptr& targetType, const String& host); + static const std::vector& GetTargetedServiceRules(const Type::Ptr& sourceType, const Type::Ptr& targetType, const String& host, const String& service); static void RegisterType(const String& sourceType, const std::vector& targetTypes); static bool IsValidSourceType(const String& sourceType); @@ -50,6 +79,7 @@ class ApplyRule : public SharedObject static const std::vector& GetTargetTypes(const String& sourceType); static void CheckMatches(bool silent); + static void CheckMatches(const ApplyRule& rule, Type* sourceType, bool silent); private: String m_Name; @@ -67,6 +97,14 @@ class ApplyRule : public SharedObject static TypeMap m_Types; static RuleMap m_Rules; + static bool AddTargetedRule(ApplyRule& rule, const String& sourceType, const String& targetType, ApplyRule::PerTypes& rules); + static bool GetTargetHosts(Expression* assignFilter, std::vector& hosts); + static bool GetTargetServices(Expression* assignFilter, std::vector>& services); + static std::pair GetTargetService(Expression* assignFilter); + static const String * GetComparedName(Expression* assignFilter, const char * lcType); + static bool IsNameIndexer(Expression* exp, const char * lcType); + static const String * GetLiteralStringValue(Expression* exp); + ApplyRule(String name, Expression::Ptr expression, Expression::Ptr filter, String package, String fkvar, String fvvar, Expression::Ptr fterm, bool ignoreOnError, DebugInfo di, Dictionary::Ptr scope); diff --git a/lib/config/expression.hpp b/lib/config/expression.hpp index 21ab7a2e61a..26ffb341bba 100644 --- a/lib/config/expression.hpp +++ b/lib/config/expression.hpp @@ -283,6 +283,16 @@ class BinaryExpression : public DebuggableExpression : DebuggableExpression(debugInfo), m_Operand1(std::move(operand1)), m_Operand2(std::move(operand2)) { } + inline const std::unique_ptr& GetOperand1() const noexcept + { + return m_Operand1; + } + + inline const std::unique_ptr& GetOperand2() const noexcept + { + return m_Operand2; + } + protected: std::unique_ptr m_Operand1; std::unique_ptr m_Operand2; diff --git a/lib/icinga/dependency-apply.cpp b/lib/icinga/dependency-apply.cpp index 07ab312541d..7b9503fb983 100644 --- a/lib/icinga/dependency-apply.cpp +++ b/lib/icinga/dependency-apply.cpp @@ -140,6 +140,11 @@ void Dependency::EvaluateApplyRules(const Host::Ptr& host) if (EvaluateApplyRule(host, rule)) rule.AddMatch(); } + + for (auto& rule : ApplyRule::GetTargetedHostRules(Dependency::TypeInstance, Host::TypeInstance, host->GetName())) { + if (EvaluateApplyRule(host, *rule)) + rule->AddMatch(); + } } void Dependency::EvaluateApplyRules(const Service::Ptr& service) @@ -150,4 +155,9 @@ void Dependency::EvaluateApplyRules(const Service::Ptr& service) if (EvaluateApplyRule(service, rule)) rule.AddMatch(); } + + for (auto& rule : ApplyRule::GetTargetedServiceRules(Dependency::TypeInstance, Service::TypeInstance, service->GetHost()->GetName(), service->GetName())) { + if (EvaluateApplyRule(service, *rule)) + rule->AddMatch(); + } } diff --git a/lib/icinga/notification-apply.cpp b/lib/icinga/notification-apply.cpp index 61d21921515..d5d3d48708c 100644 --- a/lib/icinga/notification-apply.cpp +++ b/lib/icinga/notification-apply.cpp @@ -140,6 +140,11 @@ void Notification::EvaluateApplyRules(const Host::Ptr& host) if (EvaluateApplyRule(host, rule)) rule.AddMatch(); } + + for (auto& rule : ApplyRule::GetTargetedHostRules(Notification::TypeInstance, Host::TypeInstance, host->GetName())) { + if (EvaluateApplyRule(host, *rule)) + rule->AddMatch(); + } } void Notification::EvaluateApplyRules(const Service::Ptr& service) @@ -150,4 +155,9 @@ void Notification::EvaluateApplyRules(const Service::Ptr& service) if (EvaluateApplyRule(service, rule)) rule.AddMatch(); } + + for (auto& rule : ApplyRule::GetTargetedServiceRules(Notification::TypeInstance, Service::TypeInstance, service->GetHost()->GetName(), service->GetName())) { + if (EvaluateApplyRule(service, *rule)) + rule->AddMatch(); + } } diff --git a/lib/icinga/scheduleddowntime-apply.cpp b/lib/icinga/scheduleddowntime-apply.cpp index 0dfaf1b09b9..893c42aca21 100644 --- a/lib/icinga/scheduleddowntime-apply.cpp +++ b/lib/icinga/scheduleddowntime-apply.cpp @@ -138,6 +138,11 @@ void ScheduledDowntime::EvaluateApplyRules(const Host::Ptr& host) if (EvaluateApplyRule(host, rule)) rule.AddMatch(); } + + for (auto& rule : ApplyRule::GetTargetedHostRules(ScheduledDowntime::TypeInstance, Host::TypeInstance, host->GetName())) { + if (EvaluateApplyRule(host, *rule)) + rule->AddMatch(); + } } void ScheduledDowntime::EvaluateApplyRules(const Service::Ptr& service) @@ -148,4 +153,9 @@ void ScheduledDowntime::EvaluateApplyRules(const Service::Ptr& service) if (EvaluateApplyRule(service, rule)) rule.AddMatch(); } + + for (auto& rule : ApplyRule::GetTargetedServiceRules(ScheduledDowntime::TypeInstance, Service::TypeInstance, service->GetHost()->GetName(), service->GetName())) { + if (EvaluateApplyRule(service, *rule)) + rule->AddMatch(); + } } diff --git a/lib/icinga/service-apply.cpp b/lib/icinga/service-apply.cpp index 24a19c95674..e14342948c5 100644 --- a/lib/icinga/service-apply.cpp +++ b/lib/icinga/service-apply.cpp @@ -127,4 +127,9 @@ void Service::EvaluateApplyRules(const Host::Ptr& host) if (EvaluateApplyRule(host, rule)) rule.AddMatch(); } + + for (auto& rule : ApplyRule::GetTargetedHostRules(Service::TypeInstance, Host::TypeInstance, host->GetName())) { + if (EvaluateApplyRule(host, *rule)) + rule->AddMatch(); + } } From 038a5e8ef6134c0e9483f0cf2975d6f6212f67cd Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Thu, 27 Oct 2022 16:54:09 +0200 Subject: [PATCH 4/9] Unify storages of regular/targeted apply rules: std::vector --- lib/config/applyrule-targeted.cpp | 14 +++++--------- lib/config/applyrule.cpp | 14 +++++++------- lib/config/applyrule.hpp | 8 ++++---- lib/icinga/dependency-apply.cpp | 8 ++++---- lib/icinga/notification-apply.cpp | 8 ++++---- lib/icinga/scheduleddowntime-apply.cpp | 8 ++++---- lib/icinga/service-apply.cpp | 4 ++-- 7 files changed, 30 insertions(+), 34 deletions(-) diff --git a/lib/config/applyrule-targeted.cpp b/lib/config/applyrule-targeted.cpp index 1a2ce564e42..f38f81f2185 100644 --- a/lib/config/applyrule-targeted.cpp +++ b/lib/config/applyrule-targeted.cpp @@ -68,16 +68,14 @@ const std::vector& ApplyRule::GetTargetedServiceRules(const Type * * @returns Whether the rule has been added to the "index". */ -bool ApplyRule::AddTargetedRule(ApplyRule& rule, const String& sourceType, const String& targetType, ApplyRule::PerTypes& rules) +bool ApplyRule::AddTargetedRule(const ApplyRule::Ptr& rule, const String& sourceType, const String& targetType, ApplyRule::PerTypes& rules) { if (targetType == "Host") { std::vector hosts; - if (GetTargetHosts(rule.m_Filter.get(), hosts)) { - ApplyRule::Ptr sharedRule = new ApplyRule(std::move(rule)); - + if (GetTargetHosts(rule->m_Filter.get(), hosts)) { for (auto host : hosts) { - rules.Targeted[*host].ForHost.emplace_back(sharedRule); + rules.Targeted[*host].ForHost.emplace_back(rule); } return true; @@ -85,11 +83,9 @@ bool ApplyRule::AddTargetedRule(ApplyRule& rule, const String& sourceType, const } else if (targetType == "Service") { std::vector> services; - if (GetTargetServices(rule.m_Filter.get(), services)) { - ApplyRule::Ptr sharedRule = new ApplyRule(std::move(rule)); - + if (GetTargetServices(rule->m_Filter.get(), services)) { for (auto service : services) { - rules.Targeted[*service.first].ForServices[*service.second].emplace_back(sharedRule); + rules.Targeted[*service.first].ForServices[*service.second].emplace_back(rule); } return true; diff --git a/lib/config/applyrule.cpp b/lib/config/applyrule.cpp index cd268269e9b..f56d7eb02b5 100644 --- a/lib/config/applyrule.cpp +++ b/lib/config/applyrule.cpp @@ -81,7 +81,7 @@ void ApplyRule::AddRule(const String& sourceType, const String& targetType, cons } } - ApplyRule rule (name, expression, filter, package, fkvar, fvvar, fterm, ignoreOnError, di, scope); + ApplyRule::Ptr rule = new ApplyRule(name, expression, filter, package, fkvar, fvvar, fterm, ignoreOnError, di, scope); auto& rules (m_Rules[Type::GetByName(sourceType).get()][Type::GetByName(*actualTargetType).get()]); if (!AddTargetedRule(rule, sourceType, *actualTargetType, rules)) { @@ -144,7 +144,7 @@ bool ApplyRule::HasMatches() const return m_HasMatches; } -std::vector& ApplyRule::GetRules(const Type::Ptr& sourceType, const Type::Ptr& targetType) +const std::vector& ApplyRule::GetRules(const Type::Ptr& sourceType, const Type::Ptr& targetType) { auto perSourceType (m_Rules.find(sourceType.get())); @@ -156,7 +156,7 @@ std::vector& ApplyRule::GetRules(const Type::Ptr& sourceType, const T } } - static std::vector noRules; + static const std::vector noRules; return noRules; } @@ -183,17 +183,17 @@ void ApplyRule::CheckMatches(bool silent) } for (auto rule : targeted) { - CheckMatches(*rule, perSourceType.first, silent); + CheckMatches(rule, perSourceType.first, silent); } } } } -void ApplyRule::CheckMatches(const ApplyRule& rule, Type* sourceType, bool silent) +void ApplyRule::CheckMatches(const ApplyRule::Ptr& rule, Type* sourceType, bool silent) { - if (!rule.HasMatches() && !silent) { + if (!rule->HasMatches() && !silent) { Log(LogWarning, "ApplyRule") - << "Apply rule '" << rule.GetName() << "' (" << rule.GetDebugInfo() << ") for type '" + << "Apply rule '" << rule->GetName() << "' (" << rule->GetDebugInfo() << ") for type '" << sourceType->GetName() << "' does not match anywhere!"; } } diff --git a/lib/config/applyrule.hpp b/lib/config/applyrule.hpp index 37ec04cfd88..49f964b50f3 100644 --- a/lib/config/applyrule.hpp +++ b/lib/config/applyrule.hpp @@ -29,7 +29,7 @@ class ApplyRule : public SharedObject struct PerTypes { - std::vector Regular; + std::vector Regular; std::unordered_map Targeted; }; @@ -69,7 +69,7 @@ class ApplyRule : public SharedObject static void AddRule(const String& sourceType, const String& targetType, const String& name, const Expression::Ptr& expression, const Expression::Ptr& filter, const String& package, const String& fkvar, const String& fvvar, const Expression::Ptr& fterm, bool ignoreOnError, const DebugInfo& di, const Dictionary::Ptr& scope); - static std::vector& GetRules(const Type::Ptr& sourceType, const Type::Ptr& targetType); + static const std::vector& GetRules(const Type::Ptr& sourceType, const Type::Ptr& targetType); static const std::vector& GetTargetedHostRules(const Type::Ptr& sourceType, const Type::Ptr& targetType, const String& host); static const std::vector& GetTargetedServiceRules(const Type::Ptr& sourceType, const Type::Ptr& targetType, const String& host, const String& service); @@ -79,7 +79,7 @@ class ApplyRule : public SharedObject static const std::vector& GetTargetTypes(const String& sourceType); static void CheckMatches(bool silent); - static void CheckMatches(const ApplyRule& rule, Type* sourceType, bool silent); + static void CheckMatches(const ApplyRule::Ptr& rule, Type* sourceType, bool silent); private: String m_Name; @@ -97,7 +97,7 @@ class ApplyRule : public SharedObject static TypeMap m_Types; static RuleMap m_Rules; - static bool AddTargetedRule(ApplyRule& rule, const String& sourceType, const String& targetType, ApplyRule::PerTypes& rules); + static bool AddTargetedRule(const ApplyRule::Ptr& rule, const String& sourceType, const String& targetType, ApplyRule::PerTypes& rules); static bool GetTargetHosts(Expression* assignFilter, std::vector& hosts); static bool GetTargetServices(Expression* assignFilter, std::vector>& services); static std::pair GetTargetService(Expression* assignFilter); diff --git a/lib/icinga/dependency-apply.cpp b/lib/icinga/dependency-apply.cpp index 7b9503fb983..2ad18d83e0e 100644 --- a/lib/icinga/dependency-apply.cpp +++ b/lib/icinga/dependency-apply.cpp @@ -137,8 +137,8 @@ void Dependency::EvaluateApplyRules(const Host::Ptr& host) CONTEXT("Evaluating 'apply' rules for host '" + host->GetName() + "'"); for (auto& rule : ApplyRule::GetRules(Dependency::TypeInstance, Host::TypeInstance)) { - if (EvaluateApplyRule(host, rule)) - rule.AddMatch(); + if (EvaluateApplyRule(host, *rule)) + rule->AddMatch(); } for (auto& rule : ApplyRule::GetTargetedHostRules(Dependency::TypeInstance, Host::TypeInstance, host->GetName())) { @@ -152,8 +152,8 @@ void Dependency::EvaluateApplyRules(const Service::Ptr& service) CONTEXT("Evaluating 'apply' rules for service '" + service->GetName() + "'"); for (auto& rule : ApplyRule::GetRules(Dependency::TypeInstance, Service::TypeInstance)) { - if (EvaluateApplyRule(service, rule)) - rule.AddMatch(); + if (EvaluateApplyRule(service, *rule)) + rule->AddMatch(); } for (auto& rule : ApplyRule::GetTargetedServiceRules(Dependency::TypeInstance, Service::TypeInstance, service->GetHost()->GetName(), service->GetName())) { diff --git a/lib/icinga/notification-apply.cpp b/lib/icinga/notification-apply.cpp index d5d3d48708c..177e447001b 100644 --- a/lib/icinga/notification-apply.cpp +++ b/lib/icinga/notification-apply.cpp @@ -137,8 +137,8 @@ void Notification::EvaluateApplyRules(const Host::Ptr& host) for (auto& rule : ApplyRule::GetRules(Notification::TypeInstance, Host::TypeInstance)) { - if (EvaluateApplyRule(host, rule)) - rule.AddMatch(); + if (EvaluateApplyRule(host, *rule)) + rule->AddMatch(); } for (auto& rule : ApplyRule::GetTargetedHostRules(Notification::TypeInstance, Host::TypeInstance, host->GetName())) { @@ -152,8 +152,8 @@ void Notification::EvaluateApplyRules(const Service::Ptr& service) CONTEXT("Evaluating 'apply' rules for service '" + service->GetName() + "'"); for (auto& rule : ApplyRule::GetRules(Notification::TypeInstance, Service::TypeInstance)) { - if (EvaluateApplyRule(service, rule)) - rule.AddMatch(); + if (EvaluateApplyRule(service, *rule)) + rule->AddMatch(); } for (auto& rule : ApplyRule::GetTargetedServiceRules(Notification::TypeInstance, Service::TypeInstance, service->GetHost()->GetName(), service->GetName())) { diff --git a/lib/icinga/scheduleddowntime-apply.cpp b/lib/icinga/scheduleddowntime-apply.cpp index 893c42aca21..ad11a4b4e1c 100644 --- a/lib/icinga/scheduleddowntime-apply.cpp +++ b/lib/icinga/scheduleddowntime-apply.cpp @@ -135,8 +135,8 @@ void ScheduledDowntime::EvaluateApplyRules(const Host::Ptr& host) CONTEXT("Evaluating 'apply' rules for host '" + host->GetName() + "'"); for (auto& rule : ApplyRule::GetRules(ScheduledDowntime::TypeInstance, Host::TypeInstance)) { - if (EvaluateApplyRule(host, rule)) - rule.AddMatch(); + if (EvaluateApplyRule(host, *rule)) + rule->AddMatch(); } for (auto& rule : ApplyRule::GetTargetedHostRules(ScheduledDowntime::TypeInstance, Host::TypeInstance, host->GetName())) { @@ -150,8 +150,8 @@ void ScheduledDowntime::EvaluateApplyRules(const Service::Ptr& service) CONTEXT("Evaluating 'apply' rules for service '" + service->GetName() + "'"); for (auto& rule : ApplyRule::GetRules(ScheduledDowntime::TypeInstance, Service::TypeInstance)) { - if (EvaluateApplyRule(service, rule)) - rule.AddMatch(); + if (EvaluateApplyRule(service, *rule)) + rule->AddMatch(); } for (auto& rule : ApplyRule::GetTargetedServiceRules(ScheduledDowntime::TypeInstance, Service::TypeInstance, service->GetHost()->GetName(), service->GetName())) { diff --git a/lib/icinga/service-apply.cpp b/lib/icinga/service-apply.cpp index e14342948c5..4fa8787438f 100644 --- a/lib/icinga/service-apply.cpp +++ b/lib/icinga/service-apply.cpp @@ -124,8 +124,8 @@ void Service::EvaluateApplyRules(const Host::Ptr& host) CONTEXT("Evaluating 'apply' rules for host '" + host->GetName() + "'"); for (auto& rule : ApplyRule::GetRules(Service::TypeInstance, Host::TypeInstance)) { - if (EvaluateApplyRule(host, rule)) - rule.AddMatch(); + if (EvaluateApplyRule(host, *rule)) + rule->AddMatch(); } for (auto& rule : ApplyRule::GetTargetedHostRules(Service::TypeInstance, Host::TypeInstance, host->GetName())) { From dacd6a206de2c54f496c86279dad7c8a4b822e4f Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Tue, 18 Oct 2022 16:51:58 +0200 Subject: [PATCH 5/9] VariableExpression#GetVariable(): return by const ref not to unnecessarily malloc() --- lib/config/expression.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/config/expression.hpp b/lib/config/expression.hpp index 26ffb341bba..7be667a2b99 100644 --- a/lib/config/expression.hpp +++ b/lib/config/expression.hpp @@ -303,7 +303,7 @@ class VariableExpression final : public DebuggableExpression public: VariableExpression(String variable, std::vector imports, const DebugInfo& debugInfo = DebugInfo()); - String GetVariable() const + inline const String& GetVariable() const { return m_Variable; } From a907c2ac9ae9496d8ea11d79c706b267e93a754a Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Thu, 27 Oct 2022 16:15:35 +0200 Subject: [PATCH 6/9] Targeted apply rules: don't unnecessarily eval filter --- lib/icinga/dependency-apply.cpp | 14 +++++++------- lib/icinga/dependency.hpp | 4 ++-- lib/icinga/notification-apply.cpp | 14 +++++++------- lib/icinga/notification.hpp | 4 ++-- lib/icinga/scheduleddowntime-apply.cpp | 14 +++++++------- lib/icinga/scheduleddowntime.hpp | 4 ++-- lib/icinga/service-apply.cpp | 12 ++++++------ lib/icinga/service.hpp | 4 ++-- 8 files changed, 35 insertions(+), 35 deletions(-) diff --git a/lib/icinga/dependency-apply.cpp b/lib/icinga/dependency-apply.cpp index 2ad18d83e0e..13b53f04c0c 100644 --- a/lib/icinga/dependency-apply.cpp +++ b/lib/icinga/dependency-apply.cpp @@ -17,9 +17,9 @@ INITIALIZE_ONCE([]() { ApplyRule::RegisterType("Dependency", { "Host", "Service" }); }); -bool Dependency::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule) +bool Dependency::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule, bool skipFilter) { - if (!rule.EvaluateFilter(frame)) + if (!skipFilter && !rule.EvaluateFilter(frame)) return false; DebugInfo di = rule.GetDebugInfo(); @@ -62,7 +62,7 @@ bool Dependency::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, cons return true; } -bool Dependency::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyRule& rule) +bool Dependency::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyRule& rule, bool skipFilter) { DebugInfo di = rule.GetDebugInfo(); @@ -111,7 +111,7 @@ bool Dependency::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyR name += instance; } - if (EvaluateApplyRuleInstance(checkable, name, frame, rule)) + if (EvaluateApplyRuleInstance(checkable, name, frame, rule, skipFilter)) match = true; } } else if (vinstances.IsObjectType()) { @@ -124,7 +124,7 @@ bool Dependency::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyR frame.Locals->Set(rule.GetFKVar(), key); frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); - if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule)) + if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule, skipFilter)) match = true; } } @@ -142,7 +142,7 @@ void Dependency::EvaluateApplyRules(const Host::Ptr& host) } for (auto& rule : ApplyRule::GetTargetedHostRules(Dependency::TypeInstance, Host::TypeInstance, host->GetName())) { - if (EvaluateApplyRule(host, *rule)) + if (EvaluateApplyRule(host, *rule, true)) rule->AddMatch(); } } @@ -157,7 +157,7 @@ void Dependency::EvaluateApplyRules(const Service::Ptr& service) } for (auto& rule : ApplyRule::GetTargetedServiceRules(Dependency::TypeInstance, Service::TypeInstance, service->GetHost()->GetName(), service->GetName())) { - if (EvaluateApplyRule(service, *rule)) + if (EvaluateApplyRule(service, *rule, true)) rule->AddMatch(); } } diff --git a/lib/icinga/dependency.hpp b/lib/icinga/dependency.hpp index bc3ae5388a5..75424cb8994 100644 --- a/lib/icinga/dependency.hpp +++ b/lib/icinga/dependency.hpp @@ -50,8 +50,8 @@ class Dependency final : public ObjectImpl Checkable::Ptr m_Parent; Checkable::Ptr m_Child; - static bool EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule); - static bool EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyRule& rule); + static bool EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule, bool skipFilter); + static bool EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyRule& rule, bool skipFilter = false); }; } diff --git a/lib/icinga/notification-apply.cpp b/lib/icinga/notification-apply.cpp index 177e447001b..ec9338c57ca 100644 --- a/lib/icinga/notification-apply.cpp +++ b/lib/icinga/notification-apply.cpp @@ -17,9 +17,9 @@ INITIALIZE_ONCE([]() { ApplyRule::RegisterType("Notification", { "Host", "Service" }); }); -bool Notification::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule) +bool Notification::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule, bool skipFilter) { - if (!rule.EvaluateFilter(frame)) + if (!skipFilter && !rule.EvaluateFilter(frame)) return false; DebugInfo di = rule.GetDebugInfo(); @@ -61,7 +61,7 @@ bool Notification::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, co return true; } -bool Notification::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyRule& rule) +bool Notification::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyRule& rule, bool skipFilter) { DebugInfo di = rule.GetDebugInfo(); @@ -110,7 +110,7 @@ bool Notification::EvaluateApplyRule(const Checkable::Ptr& checkable, const Appl name += instance; } - if (EvaluateApplyRuleInstance(checkable, name, frame, rule)) + if (EvaluateApplyRuleInstance(checkable, name, frame, rule, skipFilter)) match = true; } } else if (vinstances.IsObjectType()) { @@ -123,7 +123,7 @@ bool Notification::EvaluateApplyRule(const Checkable::Ptr& checkable, const Appl frame.Locals->Set(rule.GetFKVar(), key); frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); - if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule)) + if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule, skipFilter)) match = true; } } @@ -142,7 +142,7 @@ void Notification::EvaluateApplyRules(const Host::Ptr& host) } for (auto& rule : ApplyRule::GetTargetedHostRules(Notification::TypeInstance, Host::TypeInstance, host->GetName())) { - if (EvaluateApplyRule(host, *rule)) + if (EvaluateApplyRule(host, *rule, true)) rule->AddMatch(); } } @@ -157,7 +157,7 @@ void Notification::EvaluateApplyRules(const Service::Ptr& service) } for (auto& rule : ApplyRule::GetTargetedServiceRules(Notification::TypeInstance, Service::TypeInstance, service->GetHost()->GetName(), service->GetName())) { - if (EvaluateApplyRule(service, *rule)) + if (EvaluateApplyRule(service, *rule, true)) rule->AddMatch(); } } diff --git a/lib/icinga/notification.hpp b/lib/icinga/notification.hpp index 2922308c8e7..ec9164f199a 100644 --- a/lib/icinga/notification.hpp +++ b/lib/icinga/notification.hpp @@ -118,8 +118,8 @@ class Notification final : public ObjectImpl void ExecuteNotificationHelper(NotificationType type, const User::Ptr& user, const CheckResult::Ptr& cr, bool force, const String& author = "", const String& text = ""); - static bool EvaluateApplyRuleInstance(const intrusive_ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule); - static bool EvaluateApplyRule(const intrusive_ptr& checkable, const ApplyRule& rule); + static bool EvaluateApplyRuleInstance(const intrusive_ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule, bool skipFilter); + static bool EvaluateApplyRule(const intrusive_ptr& checkable, const ApplyRule& rule, bool skipFilter = false); static std::map m_StateFilterMap; static std::map m_TypeFilterMap; diff --git a/lib/icinga/scheduleddowntime-apply.cpp b/lib/icinga/scheduleddowntime-apply.cpp index ad11a4b4e1c..8bfadf975d9 100644 --- a/lib/icinga/scheduleddowntime-apply.cpp +++ b/lib/icinga/scheduleddowntime-apply.cpp @@ -16,9 +16,9 @@ INITIALIZE_ONCE([]() { ApplyRule::RegisterType("ScheduledDowntime", { "Host", "Service" }); }); -bool ScheduledDowntime::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule) +bool ScheduledDowntime::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule, bool skipFilter) { - if (!rule.EvaluateFilter(frame)) + if (!skipFilter && !rule.EvaluateFilter(frame)) return false; DebugInfo di = rule.GetDebugInfo(); @@ -60,7 +60,7 @@ bool ScheduledDowntime::EvaluateApplyRuleInstance(const Checkable::Ptr& checkabl return true; } -bool ScheduledDowntime::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyRule& rule) +bool ScheduledDowntime::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyRule& rule, bool skipFilter) { DebugInfo di = rule.GetDebugInfo(); @@ -109,7 +109,7 @@ bool ScheduledDowntime::EvaluateApplyRule(const Checkable::Ptr& checkable, const name += instance; } - if (EvaluateApplyRuleInstance(checkable, name, frame, rule)) + if (EvaluateApplyRuleInstance(checkable, name, frame, rule, skipFilter)) match = true; } } else if (vinstances.IsObjectType()) { @@ -122,7 +122,7 @@ bool ScheduledDowntime::EvaluateApplyRule(const Checkable::Ptr& checkable, const frame.Locals->Set(rule.GetFKVar(), key); frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); - if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule)) + if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule, skipFilter)) match = true; } } @@ -140,7 +140,7 @@ void ScheduledDowntime::EvaluateApplyRules(const Host::Ptr& host) } for (auto& rule : ApplyRule::GetTargetedHostRules(ScheduledDowntime::TypeInstance, Host::TypeInstance, host->GetName())) { - if (EvaluateApplyRule(host, *rule)) + if (EvaluateApplyRule(host, *rule, true)) rule->AddMatch(); } } @@ -155,7 +155,7 @@ void ScheduledDowntime::EvaluateApplyRules(const Service::Ptr& service) } for (auto& rule : ApplyRule::GetTargetedServiceRules(ScheduledDowntime::TypeInstance, Service::TypeInstance, service->GetHost()->GetName(), service->GetName())) { - if (EvaluateApplyRule(service, *rule)) + if (EvaluateApplyRule(service, *rule, true)) rule->AddMatch(); } } diff --git a/lib/icinga/scheduleddowntime.hpp b/lib/icinga/scheduleddowntime.hpp index f8d8e084083..e70123616b1 100644 --- a/lib/icinga/scheduleddowntime.hpp +++ b/lib/icinga/scheduleddowntime.hpp @@ -51,8 +51,8 @@ class ScheduledDowntime final : public ObjectImpl static std::atomic m_AllConfigLoaded; - static bool EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule); - static bool EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyRule& rule); + static bool EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule, bool skipFilter); + static bool EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyRule& rule, bool skipFilter = false); }; } diff --git a/lib/icinga/service-apply.cpp b/lib/icinga/service-apply.cpp index 4fa8787438f..63fce3648f9 100644 --- a/lib/icinga/service-apply.cpp +++ b/lib/icinga/service-apply.cpp @@ -16,9 +16,9 @@ INITIALIZE_ONCE([]() { ApplyRule::RegisterType("Service", { "Host" }); }); -bool Service::EvaluateApplyRuleInstance(const Host::Ptr& host, const String& name, ScriptFrame& frame, const ApplyRule& rule) +bool Service::EvaluateApplyRuleInstance(const Host::Ptr& host, const String& name, ScriptFrame& frame, const ApplyRule& rule, bool skipFilter) { - if (!rule.EvaluateFilter(frame)) + if (!skipFilter && !rule.EvaluateFilter(frame)) return false; DebugInfo di = rule.GetDebugInfo(); @@ -55,7 +55,7 @@ bool Service::EvaluateApplyRuleInstance(const Host::Ptr& host, const String& nam return true; } -bool Service::EvaluateApplyRule(const Host::Ptr& host, const ApplyRule& rule) +bool Service::EvaluateApplyRule(const Host::Ptr& host, const ApplyRule& rule, bool skipFilter) { DebugInfo di = rule.GetDebugInfo(); @@ -98,7 +98,7 @@ bool Service::EvaluateApplyRule(const Host::Ptr& host, const ApplyRule& rule) name += instance; } - if (EvaluateApplyRuleInstance(host, name, frame, rule)) + if (EvaluateApplyRuleInstance(host, name, frame, rule, skipFilter)) match = true; } } else if (vinstances.IsObjectType()) { @@ -111,7 +111,7 @@ bool Service::EvaluateApplyRule(const Host::Ptr& host, const ApplyRule& rule) frame.Locals->Set(rule.GetFKVar(), key); frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); - if (EvaluateApplyRuleInstance(host, rule.GetName() + key, frame, rule)) + if (EvaluateApplyRuleInstance(host, rule.GetName() + key, frame, rule, skipFilter)) match = true; } } @@ -129,7 +129,7 @@ void Service::EvaluateApplyRules(const Host::Ptr& host) } for (auto& rule : ApplyRule::GetTargetedHostRules(Service::TypeInstance, Host::TypeInstance, host->GetName())) { - if (EvaluateApplyRule(host, *rule)) + if (EvaluateApplyRule(host, *rule, true)) rule->AddMatch(); } } diff --git a/lib/icinga/service.hpp b/lib/icinga/service.hpp index 74237018bff..ac27c3d9373 100644 --- a/lib/icinga/service.hpp +++ b/lib/icinga/service.hpp @@ -54,8 +54,8 @@ class Service final : public ObjectImpl, public MacroResolver private: Host::Ptr m_Host; - static bool EvaluateApplyRuleInstance(const Host::Ptr& host, const String& name, ScriptFrame& frame, const ApplyRule& rule); - static bool EvaluateApplyRule(const Host::Ptr& host, const ApplyRule& rule); + static bool EvaluateApplyRuleInstance(const Host::Ptr& host, const String& name, ScriptFrame& frame, const ApplyRule& rule, bool skipFilter); + static bool EvaluateApplyRule(const Host::Ptr& host, const ApplyRule& rule, bool skipFilter = false); }; std::pair GetHostService(const Checkable::Ptr& checkable); From a698b9c3daebc2d7b1b16b7f92bdfec5e764e658 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Fri, 28 Oct 2022 12:41:21 +0200 Subject: [PATCH 7/9] ApplyRule::RuleMap: reduce complexity, save unnecessary lookups --- lib/config/applyrule-targeted.cpp | 30 +++++++------------ lib/config/applyrule.cpp | 40 +++++++++++++------------- lib/config/applyrule.hpp | 18 ++++++------ lib/icinga/dependency-apply.cpp | 4 +-- lib/icinga/notification-apply.cpp | 4 +-- lib/icinga/scheduleddowntime-apply.cpp | 4 +-- lib/icinga/service-apply.cpp | 2 +- 7 files changed, 47 insertions(+), 55 deletions(-) diff --git a/lib/config/applyrule-targeted.cpp b/lib/config/applyrule-targeted.cpp index f38f81f2185..8a4b2fd4526 100644 --- a/lib/config/applyrule-targeted.cpp +++ b/lib/config/applyrule-targeted.cpp @@ -11,19 +11,15 @@ using namespace icinga; /** * @returns All ApplyRules targeting only specific parent objects including the given host. (See AddTargetedRule().) */ -const std::vector& ApplyRule::GetTargetedHostRules(const Type::Ptr& sourceType, const Type::Ptr& targetType, const String& host) +const std::vector& ApplyRule::GetTargetedHostRules(const Type::Ptr& sourceType, const String& host) { auto perSourceType (m_Rules.find(sourceType.get())); if (perSourceType != m_Rules.end()) { - auto perTargetType (perSourceType->second.find(targetType.get())); + auto perHost (perSourceType->second.Targeted.find(host)); - if (perTargetType != perSourceType->second.end()) { - auto perHost (perTargetType->second.Targeted.find(host)); - - if (perHost != perTargetType->second.Targeted.end()) { - return perHost->second.ForHost; - } + if (perHost != perSourceType->second.Targeted.end()) { + return perHost->second.ForHost; } } @@ -34,22 +30,18 @@ const std::vector& ApplyRule::GetTargetedHostRules(const Type::P /** * @returns All ApplyRules targeting only specific parent objects including the given service. (See AddTargetedRule().) */ -const std::vector& ApplyRule::GetTargetedServiceRules(const Type::Ptr& sourceType, const Type::Ptr& targetType, const String& host, const String& service) +const std::vector& ApplyRule::GetTargetedServiceRules(const Type::Ptr& sourceType, const String& host, const String& service) { auto perSourceType (m_Rules.find(sourceType.get())); if (perSourceType != m_Rules.end()) { - auto perTargetType (perSourceType->second.find(targetType.get())); - - if (perTargetType != perSourceType->second.end()) { - auto perHost (perTargetType->second.Targeted.find(host)); + auto perHost (perSourceType->second.Targeted.find(host)); - if (perHost != perTargetType->second.Targeted.end()) { - auto perService (perHost->second.ForServices.find(service)); + if (perHost != perSourceType->second.Targeted.end()) { + auto perService (perHost->second.ForServices.find(service)); - if (perService != perHost->second.ForServices.end()) { - return perService->second; - } + if (perService != perHost->second.ForServices.end()) { + return perService->second; } } } @@ -68,7 +60,7 @@ const std::vector& ApplyRule::GetTargetedServiceRules(const Type * * @returns Whether the rule has been added to the "index". */ -bool ApplyRule::AddTargetedRule(const ApplyRule::Ptr& rule, const String& sourceType, const String& targetType, ApplyRule::PerTypes& rules) +bool ApplyRule::AddTargetedRule(const ApplyRule::Ptr& rule, const String& targetType, ApplyRule::PerSourceType& rules) { if (targetType == "Host") { std::vector hosts; diff --git a/lib/config/applyrule.cpp b/lib/config/applyrule.cpp index f56d7eb02b5..5cfbf9e35fd 100644 --- a/lib/config/applyrule.cpp +++ b/lib/config/applyrule.cpp @@ -82,10 +82,10 @@ void ApplyRule::AddRule(const String& sourceType, const String& targetType, cons } ApplyRule::Ptr rule = new ApplyRule(name, expression, filter, package, fkvar, fvvar, fterm, ignoreOnError, di, scope); - auto& rules (m_Rules[Type::GetByName(sourceType).get()][Type::GetByName(*actualTargetType).get()]); + auto& rules (m_Rules[Type::GetByName(sourceType).get()]); - if (!AddTargetedRule(rule, sourceType, *actualTargetType, rules)) { - rules.Regular.emplace_back(std::move(rule)); + if (!AddTargetedRule(rule, *actualTargetType, rules)) { + rules.Regular[Type::GetByName(*actualTargetType).get()].emplace_back(std::move(rule)); } } @@ -149,10 +149,10 @@ const std::vector& ApplyRule::GetRules(const Type::Ptr& sourceTy auto perSourceType (m_Rules.find(sourceType.get())); if (perSourceType != m_Rules.end()) { - auto perTargetType (perSourceType->second.find(targetType.get())); + auto perTargetType (perSourceType->second.Regular.find(targetType.get())); - if (perTargetType != perSourceType->second.end()) { - return perTargetType->second.Regular; + if (perTargetType != perSourceType->second.Regular.end()) { + return perTargetType->second; } } @@ -163,28 +163,28 @@ const std::vector& ApplyRule::GetRules(const Type::Ptr& sourceTy void ApplyRule::CheckMatches(bool silent) { for (auto& perSourceType : m_Rules) { - for (auto& perTargetType : perSourceType.second) { - for (auto& rule : perTargetType.second.Regular) { + for (auto& perTargetType : perSourceType.second.Regular) { + for (auto& rule : perTargetType.second) { CheckMatches(rule, perSourceType.first, silent); } + } - std::unordered_set targeted; + std::unordered_set targeted; - for (auto& perHost : perTargetType.second.Targeted) { - for (auto& rule : perHost.second.ForHost) { - targeted.emplace(rule.get()); - } + for (auto& perHost : perSourceType.second.Targeted) { + for (auto& rule : perHost.second.ForHost) { + targeted.emplace(rule.get()); + } - for (auto& perService : perHost.second.ForServices) { - for (auto& rule : perService.second) { - targeted.emplace(rule.get()); - } + for (auto& perService : perHost.second.ForServices) { + for (auto& rule : perService.second) { + targeted.emplace(rule.get()); } } + } - for (auto rule : targeted) { - CheckMatches(rule, perSourceType.first, silent); - } + for (auto rule : targeted) { + CheckMatches(rule, perSourceType.first, silent); } } } diff --git a/lib/config/applyrule.hpp b/lib/config/applyrule.hpp index 49f964b50f3..3756ede524a 100644 --- a/lib/config/applyrule.hpp +++ b/lib/config/applyrule.hpp @@ -27,27 +27,27 @@ class ApplyRule : public SharedObject std::unordered_map> ForServices; }; - struct PerTypes + struct PerSourceType { - std::vector Regular; + std::unordered_map> Regular; std::unordered_map Targeted; }; /* - * m_Rules[T::TypeInstance.get()][Host::TypeInstance.get()].Targeted["H"].ForHost + * m_Rules[T::TypeInstance.get()].Targeted["H"].ForHost * contains all apply rules like apply T "x" to Host { ... } * which target only specific hosts incl. "H", e. g. via * assign where host.name == "H" || host.name == "h". * - * m_Rules[T::TypeInstance.get()][Service::TypeInstance.get()].Targeted["H"].ForServices["S"] + * m_Rules[T::TypeInstance.get()].Targeted["H"].ForServices["S"] * contains all apply rules like apply T "x" to Service { ... } * which target only specific services on specific hosts incl. "H!S", * e. g. via assign where host.name == "H" && service.name == "S". * - * m_Rules[T::TypeInstance.get()][C::TypeInstance.get()].Regular + * m_Rules[T::TypeInstance.get()].Regular[C::TypeInstance.get()] * contains all other apply rules like apply T "x" to C { ... }. */ - typedef std::unordered_map> RuleMap; + typedef std::unordered_map RuleMap; typedef std::map > TypeMap; @@ -70,8 +70,8 @@ class ApplyRule : public SharedObject const Expression::Ptr& filter, const String& package, const String& fkvar, const String& fvvar, const Expression::Ptr& fterm, bool ignoreOnError, const DebugInfo& di, const Dictionary::Ptr& scope); static const std::vector& GetRules(const Type::Ptr& sourceType, const Type::Ptr& targetType); - static const std::vector& GetTargetedHostRules(const Type::Ptr& sourceType, const Type::Ptr& targetType, const String& host); - static const std::vector& GetTargetedServiceRules(const Type::Ptr& sourceType, const Type::Ptr& targetType, const String& host, const String& service); + static const std::vector& GetTargetedHostRules(const Type::Ptr& sourceType, const String& host); + static const std::vector& GetTargetedServiceRules(const Type::Ptr& sourceType, const String& host, const String& service); static void RegisterType(const String& sourceType, const std::vector& targetTypes); static bool IsValidSourceType(const String& sourceType); @@ -97,7 +97,7 @@ class ApplyRule : public SharedObject static TypeMap m_Types; static RuleMap m_Rules; - static bool AddTargetedRule(const ApplyRule::Ptr& rule, const String& sourceType, const String& targetType, ApplyRule::PerTypes& rules); + static bool AddTargetedRule(const ApplyRule::Ptr& rule, const String& targetType, PerSourceType& rules); static bool GetTargetHosts(Expression* assignFilter, std::vector& hosts); static bool GetTargetServices(Expression* assignFilter, std::vector>& services); static std::pair GetTargetService(Expression* assignFilter); diff --git a/lib/icinga/dependency-apply.cpp b/lib/icinga/dependency-apply.cpp index 13b53f04c0c..015254b9d96 100644 --- a/lib/icinga/dependency-apply.cpp +++ b/lib/icinga/dependency-apply.cpp @@ -141,7 +141,7 @@ void Dependency::EvaluateApplyRules(const Host::Ptr& host) rule->AddMatch(); } - for (auto& rule : ApplyRule::GetTargetedHostRules(Dependency::TypeInstance, Host::TypeInstance, host->GetName())) { + for (auto& rule : ApplyRule::GetTargetedHostRules(Dependency::TypeInstance, host->GetName())) { if (EvaluateApplyRule(host, *rule, true)) rule->AddMatch(); } @@ -156,7 +156,7 @@ void Dependency::EvaluateApplyRules(const Service::Ptr& service) rule->AddMatch(); } - for (auto& rule : ApplyRule::GetTargetedServiceRules(Dependency::TypeInstance, Service::TypeInstance, service->GetHost()->GetName(), service->GetName())) { + for (auto& rule : ApplyRule::GetTargetedServiceRules(Dependency::TypeInstance, service->GetHost()->GetName(), service->GetName())) { if (EvaluateApplyRule(service, *rule, true)) rule->AddMatch(); } diff --git a/lib/icinga/notification-apply.cpp b/lib/icinga/notification-apply.cpp index ec9338c57ca..072998dc552 100644 --- a/lib/icinga/notification-apply.cpp +++ b/lib/icinga/notification-apply.cpp @@ -141,7 +141,7 @@ void Notification::EvaluateApplyRules(const Host::Ptr& host) rule->AddMatch(); } - for (auto& rule : ApplyRule::GetTargetedHostRules(Notification::TypeInstance, Host::TypeInstance, host->GetName())) { + for (auto& rule : ApplyRule::GetTargetedHostRules(Notification::TypeInstance, host->GetName())) { if (EvaluateApplyRule(host, *rule, true)) rule->AddMatch(); } @@ -156,7 +156,7 @@ void Notification::EvaluateApplyRules(const Service::Ptr& service) rule->AddMatch(); } - for (auto& rule : ApplyRule::GetTargetedServiceRules(Notification::TypeInstance, Service::TypeInstance, service->GetHost()->GetName(), service->GetName())) { + for (auto& rule : ApplyRule::GetTargetedServiceRules(Notification::TypeInstance, service->GetHost()->GetName(), service->GetName())) { if (EvaluateApplyRule(service, *rule, true)) rule->AddMatch(); } diff --git a/lib/icinga/scheduleddowntime-apply.cpp b/lib/icinga/scheduleddowntime-apply.cpp index 8bfadf975d9..7c0166c20e1 100644 --- a/lib/icinga/scheduleddowntime-apply.cpp +++ b/lib/icinga/scheduleddowntime-apply.cpp @@ -139,7 +139,7 @@ void ScheduledDowntime::EvaluateApplyRules(const Host::Ptr& host) rule->AddMatch(); } - for (auto& rule : ApplyRule::GetTargetedHostRules(ScheduledDowntime::TypeInstance, Host::TypeInstance, host->GetName())) { + for (auto& rule : ApplyRule::GetTargetedHostRules(ScheduledDowntime::TypeInstance, host->GetName())) { if (EvaluateApplyRule(host, *rule, true)) rule->AddMatch(); } @@ -154,7 +154,7 @@ void ScheduledDowntime::EvaluateApplyRules(const Service::Ptr& service) rule->AddMatch(); } - for (auto& rule : ApplyRule::GetTargetedServiceRules(ScheduledDowntime::TypeInstance, Service::TypeInstance, service->GetHost()->GetName(), service->GetName())) { + for (auto& rule : ApplyRule::GetTargetedServiceRules(ScheduledDowntime::TypeInstance, service->GetHost()->GetName(), service->GetName())) { if (EvaluateApplyRule(service, *rule, true)) rule->AddMatch(); } diff --git a/lib/icinga/service-apply.cpp b/lib/icinga/service-apply.cpp index 63fce3648f9..666847cabb0 100644 --- a/lib/icinga/service-apply.cpp +++ b/lib/icinga/service-apply.cpp @@ -128,7 +128,7 @@ void Service::EvaluateApplyRules(const Host::Ptr& host) rule->AddMatch(); } - for (auto& rule : ApplyRule::GetTargetedHostRules(Service::TypeInstance, Host::TypeInstance, host->GetName())) { + for (auto& rule : ApplyRule::GetTargetedHostRules(Service::TypeInstance, host->GetName())) { if (EvaluateApplyRule(host, *rule, true)) rule->AddMatch(); } From 2610fb1285255c5f6d077e6bb7e9974c40bafd2b Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Fri, 4 Nov 2022 10:15:22 +0100 Subject: [PATCH 8/9] Avoid evaluating the same filter twice for the same target --- lib/config/applyrule-targeted.cpp | 12 ++++++------ lib/config/applyrule.hpp | 14 +++++++------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/config/applyrule-targeted.cpp b/lib/config/applyrule-targeted.cpp index 8a4b2fd4526..210c6776436 100644 --- a/lib/config/applyrule-targeted.cpp +++ b/lib/config/applyrule-targeted.cpp @@ -11,7 +11,7 @@ using namespace icinga; /** * @returns All ApplyRules targeting only specific parent objects including the given host. (See AddTargetedRule().) */ -const std::vector& ApplyRule::GetTargetedHostRules(const Type::Ptr& sourceType, const String& host) +const std::set& ApplyRule::GetTargetedHostRules(const Type::Ptr& sourceType, const String& host) { auto perSourceType (m_Rules.find(sourceType.get())); @@ -23,14 +23,14 @@ const std::vector& ApplyRule::GetTargetedHostRules(const Type::P } } - static const std::vector noRules; + static const std::set noRules; return noRules; } /** * @returns All ApplyRules targeting only specific parent objects including the given service. (See AddTargetedRule().) */ -const std::vector& ApplyRule::GetTargetedServiceRules(const Type::Ptr& sourceType, const String& host, const String& service) +const std::set& ApplyRule::GetTargetedServiceRules(const Type::Ptr& sourceType, const String& host, const String& service) { auto perSourceType (m_Rules.find(sourceType.get())); @@ -46,7 +46,7 @@ const std::vector& ApplyRule::GetTargetedServiceRules(const Type } } - static const std::vector noRules; + static const std::set noRules; return noRules; } @@ -67,7 +67,7 @@ bool ApplyRule::AddTargetedRule(const ApplyRule::Ptr& rule, const String& target if (GetTargetHosts(rule->m_Filter.get(), hosts)) { for (auto host : hosts) { - rules.Targeted[*host].ForHost.emplace_back(rule); + rules.Targeted[*host].ForHost.emplace(rule); } return true; @@ -77,7 +77,7 @@ bool ApplyRule::AddTargetedRule(const ApplyRule::Ptr& rule, const String& target if (GetTargetServices(rule->m_Filter.get(), services)) { for (auto service : services) { - rules.Targeted[*service.first].ForServices[*service.second].emplace_back(rule); + rules.Targeted[*service.first].ForServices[*service.second].emplace(rule); } return true; diff --git a/lib/config/applyrule.hpp b/lib/config/applyrule.hpp index 3756ede524a..6b6fca8f8fe 100644 --- a/lib/config/applyrule.hpp +++ b/lib/config/applyrule.hpp @@ -23,8 +23,8 @@ class ApplyRule : public SharedObject struct PerHost { - std::vector ForHost; - std::unordered_map> ForServices; + std::set ForHost; + std::unordered_map> ForServices; }; struct PerSourceType @@ -36,13 +36,13 @@ class ApplyRule : public SharedObject /* * m_Rules[T::TypeInstance.get()].Targeted["H"].ForHost * contains all apply rules like apply T "x" to Host { ... } - * which target only specific hosts incl. "H", e. g. via + * which target only specific hosts incl. "H", e.g. via * assign where host.name == "H" || host.name == "h". * * m_Rules[T::TypeInstance.get()].Targeted["H"].ForServices["S"] * contains all apply rules like apply T "x" to Service { ... } - * which target only specific services on specific hosts incl. "H!S", - * e. g. via assign where host.name == "H" && service.name == "S". + * which target only specific services on specific hosts, + * e.g. via assign where host.name == "H" && service.name == "S". * * m_Rules[T::TypeInstance.get()].Regular[C::TypeInstance.get()] * contains all other apply rules like apply T "x" to C { ... }. @@ -70,8 +70,8 @@ class ApplyRule : public SharedObject const Expression::Ptr& filter, const String& package, const String& fkvar, const String& fvvar, const Expression::Ptr& fterm, bool ignoreOnError, const DebugInfo& di, const Dictionary::Ptr& scope); static const std::vector& GetRules(const Type::Ptr& sourceType, const Type::Ptr& targetType); - static const std::vector& GetTargetedHostRules(const Type::Ptr& sourceType, const String& host); - static const std::vector& GetTargetedServiceRules(const Type::Ptr& sourceType, const String& host, const String& service); + static const std::set& GetTargetedHostRules(const Type::Ptr& sourceType, const String& host); + static const std::set& GetTargetedServiceRules(const Type::Ptr& sourceType, const String& host, const String& service); static void RegisterType(const String& sourceType, const std::vector& targetTypes); static bool IsValidSourceType(const String& sourceType); From a8d46e6d476d0baa312ff1352950d28333157478 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Fri, 4 Nov 2022 10:19:26 +0100 Subject: [PATCH 9/9] Use service short name for evaluating targeted service rules --- lib/icinga/dependency-apply.cpp | 2 +- lib/icinga/notification-apply.cpp | 2 +- lib/icinga/scheduleddowntime-apply.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/icinga/dependency-apply.cpp b/lib/icinga/dependency-apply.cpp index 015254b9d96..8f8840c1971 100644 --- a/lib/icinga/dependency-apply.cpp +++ b/lib/icinga/dependency-apply.cpp @@ -156,7 +156,7 @@ void Dependency::EvaluateApplyRules(const Service::Ptr& service) rule->AddMatch(); } - for (auto& rule : ApplyRule::GetTargetedServiceRules(Dependency::TypeInstance, service->GetHost()->GetName(), service->GetName())) { + for (auto& rule : ApplyRule::GetTargetedServiceRules(Dependency::TypeInstance, service->GetHost()->GetName(), service->GetShortName())) { if (EvaluateApplyRule(service, *rule, true)) rule->AddMatch(); } diff --git a/lib/icinga/notification-apply.cpp b/lib/icinga/notification-apply.cpp index 072998dc552..0983077f0b7 100644 --- a/lib/icinga/notification-apply.cpp +++ b/lib/icinga/notification-apply.cpp @@ -156,7 +156,7 @@ void Notification::EvaluateApplyRules(const Service::Ptr& service) rule->AddMatch(); } - for (auto& rule : ApplyRule::GetTargetedServiceRules(Notification::TypeInstance, service->GetHost()->GetName(), service->GetName())) { + for (auto& rule : ApplyRule::GetTargetedServiceRules(Notification::TypeInstance, service->GetHost()->GetName(), service->GetShortName())) { if (EvaluateApplyRule(service, *rule, true)) rule->AddMatch(); } diff --git a/lib/icinga/scheduleddowntime-apply.cpp b/lib/icinga/scheduleddowntime-apply.cpp index 7c0166c20e1..9571706a22d 100644 --- a/lib/icinga/scheduleddowntime-apply.cpp +++ b/lib/icinga/scheduleddowntime-apply.cpp @@ -154,7 +154,7 @@ void ScheduledDowntime::EvaluateApplyRules(const Service::Ptr& service) rule->AddMatch(); } - for (auto& rule : ApplyRule::GetTargetedServiceRules(ScheduledDowntime::TypeInstance, service->GetHost()->GetName(), service->GetName())) { + for (auto& rule : ApplyRule::GetTargetedServiceRules(ScheduledDowntime::TypeInstance, service->GetHost()->GetName(), service->GetShortName())) { if (EvaluateApplyRule(service, *rule, true)) rule->AddMatch(); }