Merge pull request #206 from Icinga:fix/background_service_checks_not_working

LordHepipud · web-flow · commit ec65665c25ad · 2021-02-23T11:02:17.000+01:00
Fix: Background service check daemon data pool separation and memory leak

Improves the background daemon by separating each single configured check into an own data pool, preventing data of leaking from one thread to another which might cause a memory leak in long term with plenty of background checks defined.

This might also reduce CPU impact because lesser data has to be processed.
diff --git a/doc/31-Changelog.md b/doc/31-Changelog.md
@@ -19,6 +19,10 @@ Released closed milestones can be found on [GitHub](https://github.com/Icinga/ic
 * [#203](https://github.com/Icinga/icinga-powershell-framework/pull/203) Removes experimental state of the Icinga PowerShell Framework code caching and adds docs on how to use the feature
 * [#205](https://github.com/Icinga/icinga-powershell-framework/pull/205) Ensure Icinga for Windows configuration file is opened as read-only for every single task besides actually modifying configuration content
 
+### Bugfixes
+
+* [#206](https://github.com/Icinga/icinga-powershell-framework/pull/206) Fixes background service check daemon for collecting metrics over time which will no longer share data between configured checks which might cause higher CPU load and a possible memory leak
+
 ## 1.3.1 (2021-02-04)
 
 [Issue and PRs](https://github.com/Icinga/icinga-powershell-framework/milestone/12?closed=1)
diff --git a/lib/core/cache/Get-IcingaCacheData.psm1 b/lib/core/cache/Get-IcingaCacheData.psm1
@@ -34,13 +34,13 @@ function Get-IcingaCacheData()
 
     $CacheFile       = Join-Path -Path (Join-Path -Path (Join-Path -Path (Get-IcingaCacheDir) -ChildPath $Space) -ChildPath $CacheStore) -ChildPath ([string]::Format('{0}.json', $KeyName));
     [string]$Content = '';
-    $cacheData       = @{};
+    $cacheData       = @{ };
 
     if ((Test-Path $CacheFile) -eq $FALSE) {
         return $null;
     }
 
-    $Content = Get-Content -Path $CacheFile;
+    $Content = Read-IcingaFileContent -File $CacheFile;
 
     if ([string]::IsNullOrEmpty($Content)) {
         return $null;
diff --git a/lib/core/cache/Set-IcingaCacheData.psm1 b/lib/core/cache/Set-IcingaCacheData.psm1
@@ -27,15 +27,15 @@
 
 function Set-IcingaCacheData()
 {
-    param(
+    param (
         [string]$Space,
         [string]$CacheStore,
         [string]$KeyName,
         $Value
     );
 
     $CacheFile = Join-Path -Path (Join-Path -Path (Join-Path -Path (Get-IcingaCacheDir) -ChildPath $Space) -ChildPath $CacheStore) -ChildPath ([string]::Format('{0}.json', $KeyName));
-    $cacheData = @{};
+    $cacheData = @{ };
 
     if ((Test-Path $CacheFile)) {
         $cacheData = Get-IcingaCacheData -Space $Space -CacheStore $CacheStore;
diff --git a/lib/core/framework/Clear-IcingaCheckSchedulerCheckData.psm1 b/lib/core/framework/Clear-IcingaCheckSchedulerCheckData.psm1
@@ -0,0 +1,28 @@
+<#
+.SYNOPSIS
+   Clear all cached values for all check commands executed by this thread.
+   This is mandatory as we might run into a memory leak otherwise!
+.DESCRIPTION
+   Clear all cached values for all check commands executed by this thread.
+   This is mandatory as we might run into a memory leak otherwise!
+.FUNCTIONALITY
+   Clear all cached values for all check commands executed by this thread.
+   This is mandatory as we might run into a memory leak otherwise!
+.OUTPUTS
+   System.Object
+.LINK
+   https://github.com/Icinga/icinga-powershell-framework
+#>
+
+function Clear-IcingaCheckSchedulerCheckData()
+{
+    if ($null -eq $global:Icinga) {
+        return;
+    }
+
+    if ($global:Icinga.ContainsKey('CheckData') -eq $FALSE) {
+        return;
+    }
+
+    $global:Icinga.CheckData.Clear();
+}
diff --git a/lib/core/framework/Clear-IcingaCheckSchedulerEnvironment.psm1 b/lib/core/framework/Clear-IcingaCheckSchedulerEnvironment.psm1
@@ -0,0 +1,26 @@
+<#
+.SYNOPSIS
+   Clears the entire check scheduler cache environment and frees memory as
+   well as cleaning the stack
+.DESCRIPTION
+   Clears the entire check scheduler cache environment and frees memory as
+   well as cleaning the stack
+.FUNCTIONALITY
+   Clears the entire check scheduler cache environment and frees memory as
+   well as cleaning the stack
+.OUTPUTS
+   System.Object
+.LINK
+   https://github.com/Icinga/icinga-powershell-framework
+#>
+
+function Clear-IcingaCheckSchedulerEnvironment()
+{
+    if ($null -eq $global:Icinga) {
+        return;
+    }
+
+    Get-IcingaCheckSchedulerPluginOutput | Out-Null;
+    Get-IcingaCheckSchedulerPerfData | Out-Null;
+    Clear-IcingaCheckSchedulerCheckData;
+}
diff --git a/lib/core/framework/Get-IcingaCheckSchedulerCheckData.psm1 b/lib/core/framework/Get-IcingaCheckSchedulerCheckData.psm1
@@ -0,0 +1,28 @@
+<#
+.SYNOPSIS
+   Fetch the raw output values for a check command for each single object
+   processed by New-IcingaCheck
+.DESCRIPTION
+   Fetch the raw output values for a check command for each single object
+   processed by New-IcingaCheck
+.FUNCTIONALITY
+   Fetch the raw output values for a check command for each single object
+   processed by New-IcingaCheck
+.OUTPUTS
+   System.Object
+.LINK
+   https://github.com/Icinga/icinga-powershell-framework
+#>
+
+function Get-IcingaCheckSchedulerCheckData()
+{
+    if ($null -eq $global:Icinga) {
+        return $null;
+    }
+
+    if ($global:Icinga.ContainsKey('CheckData') -eq $FALSE) {
+        return @{ };
+    }
+
+    return $global:Icinga.CheckData;
+}
diff --git a/lib/core/framework/New-IcingaCheckSchedulerEnvironment.psm1 b/lib/core/framework/New-IcingaCheckSchedulerEnvironment.psm1
@@ -1,12 +1,60 @@
+<#
+.SYNOPSIS
+   Create a new environment in which we can store check results, performance data
+   and values over time or executed plugins.
+
+   Usage:
+
+   Access the string plugin output by calling `Get-IcingaCheckSchedulerPluginOutput`
+   Access possible performance data with `Get-IcingaCheckSchedulerPerfData`
+
+   If you execute check plugins, ensure you read both of these functions to fetch the
+   result of the plugin call and to clear the stack and memory of the check data.
+
+   If you do not require the output, you can write them to Null
+
+   Get-IcingaCheckSchedulerPluginOutput | Out-Null;
+   Get-IcingaCheckSchedulerPerfData | Out-Null;
+
+   IMPORTANT:
+   In addition each value for each object created with `New-IcingaCheck` is stored
+   with a timestamp for the check command inside a hashtable. If you do not require
+   these data, you MUST call `Clear-IcingaCheckSchedulerCheckData` to free memory
+   and clear data from the stack!
+
+   If you are finished with all data processing and do not require anything within
+   memory anyway, you can safely call `Clear-IcingaCheckSchedulerEnvironment` to
+   do the same thing in one call.
+.DESCRIPTION
+   Fetch the raw output values for a check command for each single object
+   processed by New-IcingaCheck
+.FUNCTIONALITY
+   Fetch the raw output values for a check command for each single object
+   processed by New-IcingaCheck
+.OUTPUTS
+   System.Object
+.LINK
+   https://github.com/Icinga/icinga-powershell-framework
+#>
+
 function New-IcingaCheckSchedulerEnvironment()
 {
     # Legacy code
-    $IcingaDaemonData.IcingaThreadContent.Add('Scheduler', @{ });
+    if ($IcingaDaemonData.IcingaThreadContent.ContainsKey('Scheduler') -eq $FALSE) {
+        $IcingaDaemonData.IcingaThreadContent.Add('Scheduler', @{ });
+    }
 
     if ($null -eq $global:Icinga) {
-        $global:Icinga = @{};
+        $global:Icinga = @{ };
     }
 
-    $global:Icinga.Add('CheckResults', @());
-    $global:Icinga.Add('PerfData', @());
+    if ($global:Icinga.ContainsKey('CheckResults') -eq $FALSE) {
+        $global:Icinga.Add('CheckResults', @());
+    }
+    if ($global:Icinga.ContainsKey('PerfData') -eq $FALSE) {
+        $global:Icinga.Add('PerfData', @());
+    }
+    if ($global:Icinga.ContainsKey('CheckData') -eq $FALSE) {
+        $global:Icinga.Add('CheckData', @{ });
+    }
 }
diff --git a/lib/daemons/ServiceCheckDaemon/Start-IcingaServiceCheckDaemon.psm1 b/lib/daemons/ServiceCheckDaemon/Start-IcingaServiceCheckDaemon.psm1
@@ -5,7 +5,6 @@ function Start-IcingaServiceCheckDaemon()
 
         Use-Icinga -LibOnly -Daemon;
 
-        $IcingaDaemonData.BackgroundDaemon.Add('ServiceCheckScheduler', [hashtable]::Synchronized(@{}));
         $IcingaDaemonData.IcingaThreadPool.Add('ServiceCheckPool', (New-IcingaThreadPool -MaxInstances (Get-IcingaConfigTreeCount -Path 'BackgroundDaemon.RegisteredServices')));
 
         while ($TRUE) {
@@ -45,11 +44,16 @@ function Start-IcingaServiceCheckTask()
         Use-Icinga -LibOnly -Daemon;
         $PassedTime   = 0;
         $SortedResult = $null;
-        $OldData      = @{};
-        $PerfCache    = @{};
-        $AverageCalc  = @{};
+        $OldData      = @{ };
+        $PerfCache    = @{ };
+        $AverageCalc  = @{ };
         [int]$MaxTime = 0;
 
+        # Initialise some global variables we use to actually store check result data from
+        # plugins properly. This is doable from each thread instance as this part isn't
+        # shared between daemons
+        New-IcingaCheckSchedulerEnvironment;
+
         foreach ($index in $TimeIndexes) {
             # Only allow numeric index values
             if ((Test-Numeric $index) -eq $FALSE) {
@@ -73,22 +77,22 @@ function Start-IcingaServiceCheckTask()
 
         [int]$MaxTimeInSeconds = $MaxTime * 60;
 
-        if (-Not ($IcingaDaemonData.BackgroundDaemon.ServiceCheckScheduler.ContainsKey($CheckCommand))) {
-            $IcingaDaemonData.BackgroundDaemon.ServiceCheckScheduler.Add($CheckCommand, [hashtable]::Synchronized(@{}));
-            $IcingaDaemonData.BackgroundDaemon.ServiceCheckScheduler[$CheckCommand].Add('results', [hashtable]::Synchronized(@{}));
-            $IcingaDaemonData.BackgroundDaemon.ServiceCheckScheduler[$CheckCommand].Add('average', [hashtable]::Synchronized(@{}));
+        if (-Not ($global:Icinga.CheckData.ContainsKey($CheckCommand))) {
+            $global:Icinga.CheckData.Add($CheckCommand, @{ });
+            $global:Icinga.CheckData[$CheckCommand].Add('results', @{ });
+            $global:Icinga.CheckData[$CheckCommand].Add('average', @{ });
         }
 
         $LoadedCacheData = Get-IcingaCacheData -Space 'sc_daemon' -CacheStore 'checkresult_store' -KeyName $CheckCommand;
 
         if ($null -ne $LoadedCacheData) {
             foreach ($entry in $LoadedCacheData.PSObject.Properties) {
-                $IcingaDaemonData.BackgroundDaemon.ServiceCheckScheduler[$CheckCommand]['results'].Add(
+                $global:Icinga.CheckData[$CheckCommand]['results'].Add(
                     $entry.name,
-                    [hashtable]::Synchronized(@{})
+                    @{ }
                 );
                 foreach ($item in $entry.Value.PSObject.Properties) {
-                    $IcingaDaemonData.BackgroundDaemon.ServiceCheckScheduler[$CheckCommand]['results'][$entry.name].Add(
+                    $global:Icinga.CheckData[$CheckCommand]['results'][$entry.name].Add(
                         $item.Name,
                         $item.Value
                     );
@@ -106,11 +110,11 @@ function Start-IcingaServiceCheckTask()
 
                     $UnixTime = Get-IcingaUnixTime;
 
-                    foreach ($result in $IcingaDaemonData.BackgroundDaemon.ServiceCheckScheduler[$CheckCommand]['results'].Keys) {
+                    foreach ($result in $global:Icinga.CheckData[$CheckCommand]['results'].Keys) {
                         [string]$HashIndex = $result;
-                        $SortedResult = $IcingaDaemonData.BackgroundDaemon.ServiceCheckScheduler[$CheckCommand]['results'][$HashIndex].GetEnumerator() | Sort-Object name -Descending;
-                        Add-IcingaHashtableItem -Hashtable $OldData -Key $HashIndex -Value @{} | Out-Null;
-                        Add-IcingaHashtableItem -Hashtable $PerfCache -Key $HashIndex -Value @{} | Out-Null;
+                        $SortedResult = $global:Icinga.CheckData[$CheckCommand]['results'][$HashIndex].GetEnumerator() | Sort-Object name -Descending;
+                        Add-IcingaHashtableItem -Hashtable $OldData -Key $HashIndex -Value @{ } | Out-Null;
+                        Add-IcingaHashtableItem -Hashtable $PerfCache -Key $HashIndex -Value @{ } | Out-Null;
 
                         foreach ($timeEntry in $SortedResult) {
                             foreach ($calc in $AverageCalc.Keys) {
@@ -133,7 +137,7 @@ function Start-IcingaServiceCheckTask()
                             );
 
                             Add-IcingaHashtableItem `
-                                -Hashtable $IcingaDaemonData.BackgroundDaemon.ServiceCheckScheduler[$CheckCommand]['average'] `
+                                -Hashtable $global:Icinga.CheckData[$CheckCommand]['average'] `
                                 -Key $MetricName -Value $AverageValue -Override | Out-Null;
 
                             $AverageCalc[$calc].Sum   = 0;
@@ -144,11 +148,11 @@ function Start-IcingaServiceCheckTask()
                     # Flush data we no longer require in our cache to free memory
                     foreach ($entry in $OldData.Keys) {
                         foreach ($key in $OldData[$entry].Keys) {
-                            Remove-IcingaHashtableItem -Hashtable $IcingaDaemonData.BackgroundDaemon.ServiceCheckScheduler[$CheckCommand]['results'][$entry] -Key $key.Name
+                            Remove-IcingaHashtableItem -Hashtable $global:Icinga.CheckData[$CheckCommand]['results'][$entry] -Key $key.Name;
                         }
                     }
 
-                    Set-IcingaCacheData -Space 'sc_daemon' -CacheStore 'checkresult' -KeyName $CheckCommand -Value $IcingaDaemonData.BackgroundDaemon.ServiceCheckScheduler[$CheckCommand]['average'];
+                    Set-IcingaCacheData -Space 'sc_daemon' -CacheStore 'checkresult' -KeyName $CheckCommand -Value $global:Icinga.CheckData[$CheckCommand]['average'];
                     # Write collected metrics to disk in case we reload the daemon. We will load them back into the module after reload then
                     Set-IcingaCacheData -Space 'sc_daemon' -CacheStore 'checkresult_store' -KeyName $CheckCommand -Value $PerfCache;
                 } catch {
diff --git a/lib/icinga/plugin/New-IcingaCheck.psm1 b/lib/icinga/plugin/New-IcingaCheck.psm1
@@ -49,22 +49,19 @@ function New-IcingaCheck()
             return;
         }
 
-        if ($global:IcingaDaemonData.ContainsKey('BackgroundDaemon') -eq $FALSE) {
+        if ($null -eq $global:Icinga -Or $global:Icinga.ContainsKey('CheckData') -eq $FALSE) {
             return;
         }
 
-        if ($global:IcingaDaemonData.BackgroundDaemon.ContainsKey('ServiceCheckScheduler') -eq $FALSE) {
-            return;
-        }
-
-        if ($global:IcingaDaemonData.BackgroundDaemon.ServiceCheckScheduler.ContainsKey($this.checkcommand)) {
-            if ($global:IcingaDaemonData.BackgroundDaemon.ServiceCheckScheduler[$this.checkcommand]['results'].ContainsKey($this.name) -eq $FALSE) {
-                $global:IcingaDaemonData.BackgroundDaemon.ServiceCheckScheduler[$this.checkcommand]['results'].Add(
+        if ($global:Icinga.CheckData.ContainsKey($this.checkcommand)) {
+            if ($global:Icinga.CheckData[$this.checkcommand]['results'].ContainsKey($this.name) -eq $FALSE) {
+                $global:Icinga.CheckData[$this.checkcommand]['results'].Add(
                     $this.name,
-                    [hashtable]::Synchronized(@{})
+                    @{ }
                 );
             }
-            $global:IcingaDaemonData.BackgroundDaemon.ServiceCheckScheduler[$this.checkcommand]['results'][$this.name].Add(
+
+            $global:Icinga.CheckData[$this.checkcommand]['results'][$this.name].Add(
                 (Get-IcingaUnixTime),
                 $this.value
             );
