From d215cfd5689bb90e83066ce992dba88eda174f65 Mon Sep 17 00:00:00 2001 From: Tobias Deiminger Date: Fri, 18 Mar 2022 17:19:07 +0100 Subject: [PATCH] Use a BlockingCollection to avoid busy loop in REST API threads The former implementation had 5 threads permanently spinning fast (10ms sleep) while waiting for a REST connection to process. This causes higher load in general and it breaks systems where "Turn on PowerShell Script Block Logging policy" is enabled, because then each PS statement including Start-Sleep is logged - resulting in 500 event log entries per second. It's a suggested setting in some hardening guidelines. We can easily replace the Queue with a BlockingCollection backed by a ConcurrentQueue, which has the built-in feature to sleep until there are new items. Now the REST API threads consumes zero CPU time while waiting. --- doc/100-General/10-Changelog.md | 1 + .../daemon/New-IcingaForWindowsRESTApi.psm1 | 2 +- .../daemon/Start-IcingaWindowsRESTApi.psm1 | 5 +++-- .../threads/New-IcingaForWindowsRESTThread.psm1 | 17 +++-------------- 4 files changed, 8 insertions(+), 17 deletions(-) diff --git a/doc/100-General/10-Changelog.md b/doc/100-General/10-Changelog.md index 0222eef..03b1358 100644 --- a/doc/100-General/10-Changelog.md +++ b/doc/100-General/10-Changelog.md @@ -19,6 +19,7 @@ Released closed milestones can be found on [GitHub](https://github.com/Icinga/ic * [#480](https://github.com/Icinga/icinga-powershell-framework/pull/480) Fixes service locking during Icinga Agent upgrade and ensures errors on service management are caught and printed with internal error handling * [#483](https://github.com/Icinga/icinga-powershell-framework/issues/483) Fixes REST-Api SSL certificate lookup from the Icinga Agent, in case a custom hostname was used or in certain domain environments were domain is not matching DNS domain * [#490](https://github.com/Icinga/icinga-powershell-framework/pull/490) Fixes the command `Uninstall-IcingaComponent` for the `service` component which is not doing anything +* [#497](https://github.com/Icinga/icinga-powershell-framework/pull/497) Fixes loop sleep for idle REST-Api threads by replacing them with [BlockingCollection](https://docs.microsoft.com/en-us/dotnet/api/system.collections.concurrent.blockingcollection-1?view=net-6.0) [ConcurrentQueue](https://docs.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentqueue-1?view=net-6.0) ### Enhancements diff --git a/lib/daemons/RestAPI/daemon/New-IcingaForWindowsRESTApi.psm1 b/lib/daemons/RestAPI/daemon/New-IcingaForWindowsRESTApi.psm1 index 5825d5a..c1f6750 100644 --- a/lib/daemons/RestAPI/daemon/New-IcingaForWindowsRESTApi.psm1 +++ b/lib/daemons/RestAPI/daemon/New-IcingaForWindowsRESTApi.psm1 @@ -108,7 +108,7 @@ function New-IcingaForWindowsRESTApi() continue; } - $Global:Icinga.Public.Daemons.RESTApi.ApiRequests.$NextRESTApiThreadId.Enqueue($Connection); + $Global:Icinga.Public.Daemons.RESTApi.ApiRequests.$NextRESTApiThreadId.Add($Connection); } catch { Write-IcingaEventMessage -Namespace 'RESTApi' -EvenId 2050 -ExceptionObject $_; } diff --git a/lib/daemons/RestAPI/daemon/Start-IcingaWindowsRESTApi.psm1 b/lib/daemons/RestAPI/daemon/Start-IcingaWindowsRESTApi.psm1 index 65f8dfa..394b9c2 100644 --- a/lib/daemons/RestAPI/daemon/Start-IcingaWindowsRESTApi.psm1 +++ b/lib/daemons/RestAPI/daemon/Start-IcingaWindowsRESTApi.psm1 @@ -75,8 +75,9 @@ function Start-IcingaWindowsRESTApi() while ($ConcurrentThreads -gt 0) { $ConcurrentThreads = $ConcurrentThreads - 1; - [System.Collections.Queue]$RESTThreadQueue = @(); - $Global:Icinga.Public.Daemons.RESTApi.ApiRequests.Add($ThreadId, [System.Collections.Queue]::Synchronized($RESTThreadQueue)); + $RESTThreadQueue = New-Object System.Collections.Concurrent.BlockingCollection[PSObject] ` + -ArgumentList (New-Object System.Collections.Concurrent.ConcurrentQueue[PSObject]); + $Global:Icinga.Public.Daemons.RESTApi.ApiRequests.Add($ThreadId, $RESTThreadQueue); Start-IcingaForWindowsRESTThread -ThreadId $ThreadId -RequireAuth:$RequireAuth; $ThreadId += 1; diff --git a/lib/daemons/RestAPI/threads/New-IcingaForWindowsRESTThread.psm1 b/lib/daemons/RestAPI/threads/New-IcingaForWindowsRESTThread.psm1 index 146a0e7..f141fd6 100644 --- a/lib/daemons/RestAPI/threads/New-IcingaForWindowsRESTThread.psm1 +++ b/lib/daemons/RestAPI/threads/New-IcingaForWindowsRESTThread.psm1 @@ -16,17 +16,8 @@ function New-IcingaForWindowsRESTThread() continue; } - if ($Global:Icinga.Public.Daemons.RESTApi.ApiRequests.$ThreadId.Count -eq 0) { - Start-Sleep -Milliseconds 10; - continue; - } - - $Connection = $Global:Icinga.Public.Daemons.RESTApi.ApiRequests.$ThreadId.Dequeue(); - - if ($null -eq $Connection) { - Start-Sleep -Milliseconds 10; - continue; - } + # block sleeping until content available + $Connection = $Global:Icinga.Public.Daemons.RESTApi.ApiRequests.$ThreadId.Take(); # Read the received message from the stream by using our smart functions [string]$RestMessage = Read-IcingaTCPStream -Client $Connection.Client -Stream $Connection.Stream; @@ -107,9 +98,7 @@ function New-IcingaForWindowsRESTThread() # Finally close the clients connection as we are done here and # ensure this thread will close by simply leaving the function - if ($null -ne $Connection) { - Close-IcingaTCPConnection -Client $Connection.Client; - } + Close-IcingaTCPConnection -Client $Connection.Client; # Force Icinga for Windows Garbage Collection Optimize-IcingaForWindowsMemory -ClearErrorStack;