Prevent logfile from growing unlimited

[kevinpapst]

A first hack, that makes sure the users logfile will never exceed the size of 500kB.

At least partially fixes #1562

[Frenzie]

No objections here, but why not just checkLogSize or something? What's the "for" about? :-P

[kevinpapst]

Hahaha, lets see if I can manage to add at least one PR in the future without a wording issue from your side ;-)

[Frenzie]

I don't think the PR where I noticed that I somehow typod my own name really counts. :-P

[Frenzie]

Fine by me, but maybe not quite fine as it is right now to those who want to do their own logfile rotation?

[kevinpapst]

Someone probably setup logfile rotation for user specific logfiles ... but the vast majority of users will likely be the ones who pay the price in form of ever growing logfiles.
We could still raise the limit up to 10 MB, which should be enough for 99% of the logfile rotation users, using an interval of 1 day up to 1 week.

[Alkarex]

Please put a define in constants.php and here, something like

if (!defined('MAX_LOG_SIZE')) {
	define('MAX_LOG_SIZE', 512000);
}

[kevinpapst]

Done

[Alkarex]

I think file_exists() is superfluous, especially since the file can vanish before the next instruction filesize().
Something like @filesize() should be enough.

[kevinpapst]

I can use the shutup operator as well, thought you wouldn't like it.

[Alkarex]

We need to truncate the file, not delete it :-)

[kevinpapst]

Why? If we can delete it, we can create it with file_put_contents later on again. Yes, I can use file_put_contents with an empty string as well if you think it makes a difference.

[Alkarex]

I would like to keep the newest messages.
I think the algorithm could be to divide the logfile by 2 when it increases above a threshold (delete first half of the file).
It is probably not needed to care about keeping entire lines.

[Alkarex]

I would like to keep the newest messages.
I think the algorithm could be to divide the logfile by 2 when it increases above a threshold (delete first half of the file).
It is probably not needed to care about keeping entire lines.

[kevinpapst]

Sure, go ahead and adapt the PR. I don't see a use-case that justifies the effort. After using FreshRSS for more than a year, I did not take even one look into the log - its running to smooth ;-) I did not even know that it existed until I read #1562 and found out that my log file was impressive large as well.

[Alkarex]

I will :-)

[Alkarex]

Done in 3a4f23b

[Alkarex]

Ups, a few more fixes are on their way... Done.

[Alkarex]

I have increased the default limit to 1MB, after which the log is divided by 2.
With such a limit, a custom daily logrotate should still be fine, and if not, the value can be increased in constants.php.
Comments?

[kevinpapst]

To be honest, I would still solve that in a different way, all these low level calls make me nervous... if you really want to keep the old logs, why not just rename the "current.log" to "backup.log". The task would be done in one call and less magic.

[Alkarex]

Then the logic would be more complicated to show the logs in the UI, needing to read more than one log. Similarly, the logic for logrotate would also be more complicated. Furthermore, it would only push the problem further, by not deleting old logs if we keep them under a new name.

[Alkarex]

This seems to work fine on my server, with several logs rotated as expected.
Let's give it a try :-) Please shout if you spot any problem.

[kevinpapst]

Ok, I will copy a larger log file from /var/log/ over and see what happens - as a simulation for users who are running FreshRSS for a long time without noticing that the log file kept on growing. There might be quite a bigger group of users with such an issue.

[kevinpapst]

Ok, I ran actualize_script.php for only one missing feed and with a 16MB log file.
On my non-virtualized root server that took:
Feed actualizations took 0 day(s), 0 hour(s), 0 minute(s) and 0 seconds for 1 users
Afterwards the log file was as expected 513kb

Same test setup but a 420MB log file took about 3 seconds to process, great work!

[Alkarex]

Thanks for the test!