feat: implement support for logrotate#27
Conversation
onitake
left a comment
There was a problem hiding this comment.
Thanks for the PR!
Aside from my comments, I'd like to point out that daliserver also prints fatal and error messages to STDERR and all other messages to STDOUT.
If you're running daliserver in something that captures STDOUT/STDERR (such as systemd+journald), there's no need to enable the log file or syslog at all. I'd strongly recommend to use this instead.
Nevertheless, the default behavior of the I do however also agree that systemd+journald would be the better way forward, but that's a change I'd like to see in the daliserver Debian package itself, rather than manually installing a custom systemd unit alongside the package. |
It's high time I make that switch, then. 🙂 Sorry, dear Devuan users... |
If you would want to make Devuan users happy, you could ship both |
Please take a look: #28 |
onitake
left a comment
There was a problem hiding this comment.
With the changed logic, part of https://github.com/onitake/daliserver/pull/27/files#diff-f7dfb3ca741707d1a361d8cdd6f75f05bc99811daac01afaa258f43d4b71e4f6R165-R174 is now redundant and can be simplified. Please remove the extra fclose(), and fold the assignment to fp_logfile into the fopen() line. There's no need for an extra temp var.
Done. |
When daliserver writes a logfile, the logfile is never rotated and keeps on growing. On our system it has now reached ~6GB. This ended up filling the disk of our backup server, as we do daily backups with file-level deduplication: As the file was constantly appended to, it was never the same file, and every incremental backup contained a full copy of this file.
While trying to set up a logrotate config, I realized that daliserver did not yet contain any logic for re-opening the logfile, which is requried for logrotate to work. So this PR contains two things:
daliserverprocess withSIGHUPafter log rotationSIGHUPthat re-opens the logfile by name.