Optimize-IcingaForWindowsMemory causes high load

[haxtibal]

#405 fixed a memory leak (thanks!) by calling Optimize-IcingaForWindowsMemory for each accepted REST API connection. This effectively means for each HTTP request, because in most scenarios a client sends only one request per connection.

Sadly Optimize-IcingaForWindowsMemory seems to be expensive. Wrapping it in Measure-Command

$ts = Measure-Command { Optimize-IcingaForWindowsMemory -ClearErrorStack; }
Write-IcingaConsolePlain -Message '{0}: Optimize-IcingaForWindowsMemory took {1}s' -Objects (Get-Date -Format HH:mm:ss.fff), $ts.TotalSeconds;

reveals execution times of 0.5s on our 2 core virtual machine (varying, but at same magnitude).

16:49:04.928: Optimize-IcingaForWindowsMemory took 0.4907853s

And so Optimize-IcingaForWindowsMemory may well take more time than the actual measurement. For a reference, Get-WmiObject takes 0.3s on the same host.

@LordHepipud Can you confirm this? Our affected machines had undergone hardening measures where it's difficult to estimate impact. However, if confirmed, I'd offer to help in finding an improvement.

[LordHepipud]

Thank you for the issue. I would have to run some tests, but in general the assumption would be correct that it takes some time to cleanup the memory. We did a lot of improvements, to ensure the memory cleanup is done after each executed task.

So here is what happens:

• Before the listener for a new connection starts, the garbage collection is forced
• Once a new connection attempt is made, Icinga for Windows will proceed with the task
• Once the data is send back to the client as result, we call the garbage collection for the thread in which the request was handled
• Before we accept a new connection, the garbage collection is called again, ensuring the memory on the listener thread is cleaned up

But I agree, if you are heavily utilizing the API, each connection attempt will require a memory cleanup call for the moment. The rest threads itself are fine, because they will cleanup the memory once the execution is done and will not impact the actual responsiveness of the API, as you could spawn more worker threads.

I think a valid and best approach would be, to not call the garbage connection for every connection attempt, but after a certain amount of connections was processed. This would mean one connection attempt will take longer to process, while all others will just work fine.

[LordHepipud]

Can you please test the linked PR for this? I made a change that every 50 connection attempts the memory will receive a GC instead for every single one

[haxtibal]

if you are heavily utilizing the API, each connection attempt will require a memory cleanup call for the moment

Heavily using the API is the new default, is it?😉 Because of #131.

will not impact the actual responsiveness of the API, as you could spawn more worker threads.

Yes, they will have impact. From .net Fundamentals of garbage collection: "Before a garbage collection starts, all managed threads are suspended except for the thread that triggered the garbage collection". Thus, [System.GC]::Collect called from a worker thread will implicitly suspend all other worker threads and the listener thread.

I think a valid and best approach would be, to not call the garbage connection for every connection attempt, but after a certain amount of connections was processed. This would mean one connection attempt will take longer to process, while all others will just work fine.

That improves the situation.

Otoh, I'm curious why we have to call [System.GC]::Collect at all. I know there's a user report about a bug in PowerShell, is that the sole reason? That inofficial bug aside, the garbage collector should magically do it's work and keep the managed heap below some threshold, unless we've unintentionally created long living references to objects that are reachable from [static fields, local variables on a thread's stack, CPU registers, GC handles, finalize queue]. But in that case, the garbage collector can't help, even if manually triggered.

[LordHepipud]

The problem is, that on older Windows and PowerShell versions, the memory consumption will go through the roof if we do not call [System.GC]::Collect ourselves. Even though this bug is not "official", removing it from the code will cause memory to explode.

The worker threads itself will not be suspended as far as I am concerned regarding my testing. I could run many different queues in parallel, accessing the API, not having to worry about memory or CPU utilization. The performance improvement to the CPU are of course way better now, as GC is only done once in a while and not for every connection attempt like before.

Meanwhile the threads handling the request will still cleanup the memory after each approach. Not doing so will cause a memory leak again.

[haxtibal]

on older Windows and PowerShell versions

Do you know these versions by your own trials? If it's a workaround for some specific PS version, can we make it conditional on that PS version? Or just bump PowerShellVersion in the manifest? I mean, having a non working GC in a GC collected language is such a terrible bug that it renders the runtime unusable, IMO.

The worker threads itself will not be suspended as far as I am concerned regarding my testing.

When the GC collects, it compacts memory (defragmentation) an corrects remaining pointers. This is done for the whole managed heap of the whole process, not for a single thread. How do they achieve to not cause races if threads aren't be suspended?

[LordHepipud]

I did some personal tests but also had lots of customer environments, suffering from this problem.
I'm right now testing certain other ways, to improve this method entirely.

[LordHepipud]

I updated the linked PR with a newer version, which removes all Optimize-IcingaForWindowsMemory calls and moves one call into the background daemon, which is executed every 5 minutes.

However, in my opinion and test environment, the memory usage can become quite excessively high between GC collections.

I would appreciate some tests in this matter. CPU usage is of course way better now.

[haxtibal]

I updated the linked PR with a newer version, which removes all Optimize-IcingaForWindowsMemory calls and moves one call into the background daemon, which is executed every 5 minutes.

Thanks! Nice approach, imo.

However, in my opinion and test environment, the memory usage can become quite excessively high between GC collections.

According to Conditions for a garbage collection, the GC shall only collect in one of these three conditions

• system has low physical memory
• managed heap surpasses acceptable threshold
• GC.Collect is called

So the main question is, what "acceptable threshold" happens to be on your test system and CLR. It's complicated to understand how defaults are derived, and values are automatically adjusted during runtime. My guess is that this can be large (some 100MBs, or even GBs). Watching your heap growing within that range doesn't necessarily indicate a leak, but may just be a matter of GC configuration.

Have you already ruled out such a "works as designed" scenario?

I would appreciate some tests in this matter. CPU usage is of course way better now.

Will try to test and do some profiling, but can't promise when I get to it.

[haxtibal]

Just stumbled upon some reads worth sharing

• Low-memory setup
• High performance setup

They deal with how to tune the garbage collector by setting $Env:DOTNET_* variables when starting a .net application like the powershell.exe host.

This seems promising. I could first reproduce something that looks like a leak, and then by changing some of the variables the behavior was gone. Will update here once I understand this better.

[LordHepipud]

I'm still evaluating everything, but for the moment, it doesn't look very good.

[haxtibal]

I'm still evaluating everything, but for the moment, it doesn't look very good.

I couldn't reproduce such an error. Memory consumption doesn't look leaky so far, but I have nor long term results neither multiple environments.

Would you consider #497 (in combination with your PR)? While not its primary purpose, it might have an effect on memory consumption too.