liip / LiipMonitorBundle

Integrates the LiipMonitor library into Symfony
http://liip.ch
467 stars 103 forks source link

Race conditions when running multiple checks in a monitoring system #261

Closed lostfocus closed 2 years ago

lostfocus commented 3 years ago

I'm not sure if this is a bug or something that needs to be put into the documentation or maybe even a problem with the embedded library, but here's the problem:

We have a couple of checks triggered by Sensu and it is quite possible that multiple different checks are running at the same time. Ever since the new security checker was added (https://github.com/liip/LiipMonitorBundle/commit/d108cfadf1422b7a0a7c0118f10c7eda42136896) we're getting error messages because the different checks always trigger the security check which includes files being written to and deleted from the temporary directory - which is obviously a recipe for errors: one process deletes the temporary files of another process.

We're solving this now by changing the way Sensu calls the monitoring, but maybe it is something that should be caught/documented somewhere in this bundle?

kbond commented 3 years ago

Interesting, I'm not familiar with Sensu but gather that it can trigger the "full health check" before a previous one has completed? Is this correct?

lostfocus commented 3 years ago

Yeah, it runs multiple checks that are basically just CLI commands and it can run a whole bunch of them at the same time.

We have those three checks in there at the moment:

php -c /path/to/php.ini bin/console monitor:health doctrine_dbal_default_connection
php -c /path/to/php.ini bin/console monitor:health writable_directory
php -c /path/to/php.ini bin/console monitor:health readable_directory

and as soon as two of them run at the same time, we have the problem with the Enlightn Security Checker reading from and writing to the same files in /tmp/

lostfocus commented 3 years ago

Okay, some more digging into the code - as long as there is the information for the security_advisory check in the monitor.yaml the Enlightn Security Checker will download the advisories file from Github and unzip it into the temporary directory even if the CLI parameter is for any other check - like doctrine_dbal_default_connection

kbond commented 3 years ago

You're right, I created an upstream issue (https://github.com/laminas/laminas-diagnostics/issues/28).

lostfocus commented 3 years ago

Thanks!

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 2 years ago

This issue has been automatically closed. Feel free to re-open if it is still relevant.