Merge pull request #7121 from Icinga/bugfix/concurrent-checks

Fix that MaxConcurrentChecks constant is overridden from 'checker' feature

doc/09-object-types.md

@@ -1156,7 +1156,7 @@ requires to use `cipher_list` compatible with the endpoint using the oldest vers
 other tools to connect to the API ensure also compatibility with them as this setting affects not only inter-cluster
 communcation but also the REST API.
 
-### CheckerComponent <a id="objecttype-checkcomponent"></a>
+### CheckerComponent <a id="objecttype-checkercomponent"></a>
 
 The checker component is responsible for scheduling active checks.
 This configuration object is available as [checker feature](11-cli-commands.md#cli-command-feature).
@@ -1167,11 +1167,10 @@ Example:
 object CheckerComponent "checker" { }
 ```
 
-Configuration Attributes:
-
-  Name                      | Type                  | Description
-  --------------------------|-----------------------|----------------------------------
-  concurrent\_checks        | Number                | **Optional and deprecated.** The maximum number of concurrent checks. Was replaced by global constant `MaxConcurrentChecks` which will be set if you still use `concurrent_checks`.
+In order to limit the concurrent checks on a master/satellite endpoint,
+use [MaxConcurrentChecks](17-language-reference.md#icinga-constants-global-config) constant.
+This also applies to an agent as command endpoint where the checker
+feature is disabled.
 
 ### CheckResultReader <a id="objecttype-checkresultreader"></a>
 

doc/12-icinga2-api.md

@@ -2215,11 +2215,11 @@ and developers have been working hard to add more REST API
 clients and integrations into DevOps tools.
 
 * [Libraries](12-icinga2-api.md#icinga2-api-clients-libraries)
-* [Status](#icinga2-api-clients-status)
-* [Management](#icinga2-api-clients-management)
-* [Event Streams](#icinga2-api-clients-event-streams)
-* [Actions](#icinga2-api-clients-actions)
-* [REST API Apps](#icinga2-api-clients-apps)
+* [Status](12-icinga2-api.md#icinga2-api-clients-status)
+* [Management](12-icinga2-api.md#icinga2-api-clients-management)
+* [Event Streams](12-icinga2-api.md#icinga2-api-clients-event-streams)
+* [Actions](12-icinga2-api.md#icinga2-api-clients-actions)
+* [REST API Apps](12-icinga2-api.md#icinga2-api-clients-apps)
 
 Additional [programmatic examples](12-icinga2-api.md#icinga2-api-clients-programmatic-examples)
 will help you getting started using the Icinga 2 API in your environment.

doc/16-upgrading-icinga-2.md

@@ -96,6 +96,12 @@ user has the capabilities to change to a different user.
 If you still encounter problems, run the aforementioned CLI commands as root,
 or with sudo.
 
+### Configuration <a id="upgrading-to-2-11-configuration"></a>
+
+The deprecated `concurrent_checks` attribute in the [checker feature](09-object-types.md#objecttype-checkercomponent)
+has no effect anymore if set. Please use the [MaxConcurrentChecks](17-language-reference.md#icinga-constants-global-config)
+constant in [constants.conf](04-configuring-icinga-2.md#constants-conf) instead.
+
 ## Upgrading to v2.10 <a id="upgrading-to-2-10"></a>
 
 ### Path Constant Changes <a id="upgrading-to-2-10-path-constant-changes"></a>

icinga-app/icinga.cpp

@@ -282,8 +282,7 @@ static int Main()
 #endif /* RLIMIT_STACK */
 	}
 
-	ScriptGlobal::Set("MaxConcurrentChecks", Application::GetDefaultMaxConcurrentChecks());
-
+	/* Calculate additional global constants. */
 	ScriptGlobal::Set("System.PlatformKernel", Utility::GetPlatformKernel(), true);
 	ScriptGlobal::Set("System.PlatformKernelVersion", Utility::GetPlatformKernelVersion(), true);
 	ScriptGlobal::Set("System.PlatformName", Utility::GetPlatformName(), true);

lib/base/application.cpp

@@ -60,14 +60,6 @@ void Application::OnConfigLoaded()
 
 	ASSERT(m_Instance == nullptr);
 	m_Instance = this;
-
-	String reloadTimeout;
-
-	if (ScriptGlobal::Exists("ReloadTimeout"))
-		reloadTimeout = ScriptGlobal::Get("ReloadTimeout");
-
-	if (!reloadTimeout.IsEmpty())
-		Configuration::ReloadTimeout = Convert::ToDouble(reloadTimeout);
 }
 
 /**
@@ -406,14 +398,16 @@ pid_t Application::StartReloadProcess()
 	args.push_back("--validate");
 #endif /* _WIN32 */
 
+	double reloadTimeout = Application::GetReloadTimeout();
+
 	Process::Ptr process = new Process(Process::PrepareCommand(new Array(std::move(args))));
-	process->SetTimeout(Configuration::ReloadTimeout);
+	process->SetTimeout(reloadTimeout);
 	process->Run(&ReloadProcessCallback);
 
 	Log(LogInformation, "Application")
 		<< "Got reload command: Started new instance with PID '"
 		<< (unsigned long)(process->GetPID()) << "' (timeout is "
-		<< Configuration::ReloadTimeout << "s).";
+		<< reloadTimeout << "s).";
 
 	return process->GetPID();
 }
@@ -1179,35 +1173,9 @@ int Application::GetDefaultRLimitStack()
 	return 256 * 1024;
 }
 
-/**
- * Sets the max concurrent checks.
- *
- * @param maxChecks The new limit.
- */
-void Application::SetMaxConcurrentChecks(int maxChecks)
+double Application::GetReloadTimeout()
 {
-	ScriptGlobal::Set("MaxConcurrentChecks", maxChecks, true);
-}
-
-/**
- * Retrieves the max concurrent checks.
- *
- * @returns The max number of concurrent checks.
- */
-int Application::GetMaxConcurrentChecks()
-{
-	Value defaultMaxConcurrentChecks = GetDefaultMaxConcurrentChecks();
-	return ScriptGlobal::Get("MaxConcurrentChecks", &defaultMaxConcurrentChecks);
-}
-
-/**
- * Retrieves the default value for max concurrent checks.
- *
- * @returns The default max number of concurrent checks.
- */
-int Application::GetDefaultMaxConcurrentChecks()
-{
-	return 512;
+	return ScriptGlobal::Get("ReloadTimeout");
 }
 
 /**

lib/base/application.hpp

@@ -77,9 +77,7 @@ public:
 	static int GetDefaultRLimitProcesses();
 	static int GetDefaultRLimitStack();
 
-	static int GetMaxConcurrentChecks();
-	static int GetDefaultMaxConcurrentChecks();
-	static void SetMaxConcurrentChecks(int maxChecks);
+	static double GetReloadTimeout();
 
 	static ThreadPool& GetTP();
 

lib/base/configuration.cpp

@@ -25,7 +25,6 @@ String Configuration::PidPath;
 String Configuration::PkgDataDir;
 String Configuration::PrefixDir;
 String Configuration::ProgramData;
-double Configuration::ReloadTimeout{300};
 int Configuration::RLimitFiles;
 int Configuration::RLimitProcesses;
 int Configuration::RLimitStack;
@@ -224,16 +223,6 @@ void Configuration::SetProgramData(const String& val, bool suppress_events, cons
 	HandleUserWrite("ProgramData", &Configuration::ProgramData, val, m_ReadOnly);
 }
 
-double Configuration::GetReloadTimeout() const
-{
-	return Configuration::ReloadTimeout;
-}
-
-void Configuration::SetReloadTimeout(double val, bool suppress_events, const Value& cookie)
-{
-	HandleUserWrite("ReloadTimeout", &Configuration::ReloadTimeout, val, m_ReadOnly);
-}
-
 int Configuration::GetRLimitFiles() const
 {
 	return Configuration::RLimitFiles;

lib/base/configuration.hpp

@@ -70,9 +70,6 @@ public:
 	String GetProgramData() const override;
 	void SetProgramData(const String& value, bool suppress_events = false, const Value& cookie = Empty) override;
 
-	double GetReloadTimeout() const override;
-	void SetReloadTimeout(double value, bool suppress_events = false, const Value& cookie = Empty) override;
-
 	int GetRLimitFiles() const override;
 	void SetRLimitFiles(int value, bool suppress_events = false, const Value& cookie = Empty) override;
 
@@ -133,7 +130,6 @@ public:
 	static String PkgDataDir;
 	static String PrefixDir;
 	static String ProgramData;
-	static double ReloadTimeout;
 	static int RLimitFiles;
 	static int RLimitProcesses;
 	static int RLimitStack;

lib/base/configuration.ti

@@ -94,11 +94,6 @@ abstract class Configuration
 		set;
 	};
 
-	[config, no_storage, virtual] double ReloadTimeout {
-		get;
-		set;
-	};
-
 	[config, no_storage, virtual] int RLimitFiles {
 		get;
 		set;

lib/checker/checkercomponent.cpp

@@ -5,6 +5,7 @@
 #include "icinga/icingaapplication.hpp"
 #include "icinga/cib.hpp"
 #include "remote/apilistener.hpp"
+#include "base/configuration.hpp"
 #include "base/configtype.hpp"
 #include "base/objectlock.hpp"
 #include "base/utility.hpp"
@@ -84,14 +85,15 @@ void CheckerComponent::Stop(bool runtimeRemoved)
 		wait += 0.1;
 
 		/* Pick a timeout slightly shorther than the process reload timeout. */
-		double waitMax = Configuration::ReloadTimeout - 30;
+		double reloadTimeout = Application::GetReloadTimeout();
+		double waitMax = reloadTimeout - 30;
 		if (waitMax <= 0)
 			waitMax = 1;
 
 		if (wait > waitMax) {
 			Log(LogWarning, "CheckerComponent")
 				<< "Checks running too long for " << wait
-				<< " seconds, hard shutdown before reload timeout: " << Configuration::ReloadTimeout << ".";
+				<< " seconds, hard shutdown before reload timeout: " << reloadTimeout << ".";
 			break;
 		}
 	}
@@ -108,6 +110,7 @@ void CheckerComponent::Stop(bool runtimeRemoved)
 void CheckerComponent::CheckThreadProc()
 {
 	Utility::SetThreadName("Check Scheduler");
+	IcingaApplication::Ptr icingaApp = IcingaApplication::GetInstance();
 
 	boost::mutex::scoped_lock lock(m_Mutex);
 
@@ -126,7 +129,13 @@ void CheckerComponent::CheckThreadProc()
 
 		double wait = csi.NextCheck - Utility::GetTime();
 
-		if (Checkable::GetPendingChecks() >= GetConcurrentChecks())
+//#ifdef I2_DEBUG
+//		Log(LogDebug, "CheckerComponent")
+//			<< "Pending checks " << Checkable::GetPendingChecks()
+//			<< " vs. max concurrent checks " << icingaApp->GetMaxConcurrentChecks() << ".";
+//#endif /* I2_DEBUG */
+
+		if (Checkable::GetPendingChecks() >= icingaApp->GetMaxConcurrentChecks())
 			wait = 0.5;
 
 		if (wait > 0) {
@@ -154,12 +163,12 @@ void CheckerComponent::CheckThreadProc()
 			Service::Ptr service;
 			tie(host, service) = GetHostService(checkable);
 
-			if (host && !service && (!checkable->GetEnableActiveChecks() || !IcingaApplication::GetInstance()->GetEnableHostChecks())) {
+			if (host && !service && (!checkable->GetEnableActiveChecks() || !icingaApp->GetEnableHostChecks())) {
 				Log(LogNotice, "CheckerComponent")
 					<< "Skipping check for host '" << host->GetName() << "': active host checks are disabled";
 				check = false;
 			}
-			if (host && service && (!checkable->GetEnableActiveChecks() || !IcingaApplication::GetInstance()->GetEnableServiceChecks())) {
+			if (host && service && (!checkable->GetEnableActiveChecks() || !icingaApp->GetEnableServiceChecks())) {
 				Log(LogNotice, "CheckerComponent")
 					<< "Skipping check for service '" << service->GetName() << "': active service checks are disabled";
 				check = false;

lib/checker/checkercomponent.ti

@@ -11,17 +11,8 @@ class CheckerComponent : ConfigObject
 {
 	activation_priority 300;
 
-	[config, no_storage] int concurrent_checks {
-		get {{{
-			return Application::GetMaxConcurrentChecks();
-		}}}
-		set {{{
-			Application::SetMaxConcurrentChecks(value);
-		}}}
-		default {{{
-			return Application::GetDefaultMaxConcurrentChecks();
-		}}}
-	};
+	/* Has no effect. Keep this here to avoid breaking config changes. */
+	[deprecated, config] int concurrent_checks;
 };
 
 }

lib/icinga/clusterevents-check.cpp

@@ -1,7 +1,9 @@
 /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */
 
 #include "icinga/clusterevents.hpp"
+#include "icinga/icingaapplication.hpp"
 #include "remote/apilistener.hpp"
+#include "base/configuration.hpp"
 #include "base/serializer.hpp"
 #include "base/exception.hpp"
 #include <boost/thread/once.hpp>
@@ -20,6 +22,8 @@ void ClusterEvents::RemoteCheckThreadProc()
 {
 	Utility::SetThreadName("Remote Check Scheduler");
 
+	int maxConcurrentChecks = IcingaApplication::GetInstance()->GetMaxConcurrentChecks();
+
 	boost::mutex::scoped_lock lock(m_Mutex);
 
 	for(;;) {
@@ -27,7 +31,7 @@ void ClusterEvents::RemoteCheckThreadProc()
 			break;
 
 		lock.unlock();
-		Checkable::AquirePendingCheckSlot(Application::GetMaxConcurrentChecks());
+		Checkable::AquirePendingCheckSlot(maxConcurrentChecks);
 		lock.lock();
 
 		auto callback = m_CheckRequestQueue.front();
@@ -205,4 +209,4 @@ void ClusterEvents::LogRemoteCheckQueueInformation() {
 		<< m_ChecksExecutedDuringInterval * 6 * 15 << "/15min" << ");";
 
 	m_ChecksExecutedDuringInterval = 0;
-}
+}

lib/icinga/icingaapplication.cpp

@@ -29,6 +29,7 @@ INITIALIZE_ONCE_WITH_PRIORITY(&IcingaApplication::StaticInitialize, 50);
 
 void IcingaApplication::StaticInitialize()
 {
+	/* Pre-fill global constants, can be overridden with user input later in icinga-app/icinga.cpp. */
 	String node_name = Utility::GetFQDN();
 
 	if (node_name.IsEmpty()) {
@@ -43,6 +44,9 @@ void IcingaApplication::StaticInitialize()
 
 	ScriptGlobal::Set("NodeName", node_name);
 
+	ScriptGlobal::Set("ReloadTimeout", 300);
+	ScriptGlobal::Set("MaxConcurrentChecks", 512);
+
 	Namespace::Ptr systemNS = ScriptGlobal::Get("System");
 	/* Ensure that the System namespace is already initialized. Otherwise this is a programming error. */
 	VERIFY(systemNS);
@@ -289,6 +293,12 @@ String IcingaApplication::GetNodeName() const
 	return ScriptGlobal::Get("NodeName");
 }
 
+/* Intentionally kept here, since an agent may not have the CheckerComponent loaded. */
+int IcingaApplication::GetMaxConcurrentChecks() const
+{
+	return ScriptGlobal::Get("MaxConcurrentChecks");
+}
+
 String IcingaApplication::GetEnvironment() const
 {
 	return Application::GetAppEnvironment();

lib/icinga/icingaapplication.hpp

@@ -33,6 +33,8 @@ public:
 
 	String GetNodeName() const;
 
+	int GetMaxConcurrentChecks() const;
+
 	String GetEnvironment() const override;
 	void SetEnvironment(const String& value, bool suppress_events = false, const Value& cookie = Empty) override;
 

lib/remote/configpackageutility.cpp

@@ -218,7 +218,7 @@ void ConfigPackageUtility::AsyncTryActivateStage(const String& packageName, cons
 	args->Add("ActiveStageOverride=" + packageName + ":" + stageName);
 
 	Process::Ptr process = new Process(Process::PrepareCommand(args));
-	process->SetTimeout(Configuration::ReloadTimeout);
+	process->SetTimeout(Application::GetReloadTimeout());
 	process->Run(std::bind(&TryActivateStageCallback, _1, packageName, stageName, reload));
 }