itsgoingd / clockwork

Clockwork - php dev tools in your browser - server-side component
https://underground.works/clockwork
MIT License
5.7k stars 320 forks source link

Clockwork\Request\Log::log will grow its log indefinitely resulting in a memory leak #518

Open pvandommelen opened 3 years ago

pvandommelen commented 3 years ago

When handling a lot of events during a longer process, php will run out of memory because all log messages are kept in memory. This is an issue for us as we expect the process to be alive for potentially millions of events.

Our current solution uses a wrapper around clockwork's logger, accesses the public property $messages that's available on clockworks's class and only keep the last 100:

$log = $this->clockwork->log();
if ($log instanceof Log === false) {
    throw new \LogicException("Expected log to be of type Clockwork\Request\Log");
}
$log->log($level, $message, $data);
if (count($log->messages) > self::MAX_LOG_SIZE) {
    array_shift($log->messages);
}

This works, but maybe it would be worth to consider including this (optional) behaviour out of the box.

itsgoingd commented 3 years ago

This is a tough one. Obviously running out of memory is not an ideal scenario, but even with a limit on the number of logged messages there are so many ways how that could happen. Eg. nothing is stopping you logging a 128M large string, or more realistically you can execute too many database queries, or cache queries, or something else we collect.

In Clockwork 6.0 I want to tackle this problem more systematically and redesign the data-format to allow us flushing the collected data to disk during the request processing, instead of keeping everything in the memory.

As for the log messages limit itself as a feature, it might be part of the solution, but I'm not yet sure how things will pan out.