Fix: Background service check daemon data pool separation and memory leak

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)

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;

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;

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();
+}

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;
+}

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;
+}

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', @{ });
+    }
 }

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 {

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
             );