Bugzilla – Bug 5073
slimserver_safe logs to /var/log/slimserver.log (while server logs to /var/log/slimserver/slimserver.log)
Last modified: 2008-12-18 11:12:12 UTC
slimserver_safe is logging to /var/log/slimserver.log. This file is not rotated by default in the debian package, and is a bit confusing wrt the real slimserver logs in /var/log/slimserver/slimserver.log. Ideally, slimserver_safe should log to /var/log/slimserver/slimserver_safe.log, and that file should be rotated as well.
Here's a patch: --- /usr/sbin/slimserver_safe.orig 2007-05-17 15:28:27.000000000 -0400 +++ /usr/sbin/slimserver_safe 2007-05-17 15:30:08.000000000 -0400 @@ -2,6 +2,8 @@ # $Id$ +LOGFILE=/var/log/slimserver/slimserver_safe.log + if [ $# = 0 ] then echo "Script to restart slimserver over and over again." @@ -19,13 +21,13 @@ function clean_up { # Kill the slimserver daemon if it is running kill $SLIMPID - echo `date "+%F %H:%M:%S"` "slimserver_safe stopped." >> /var/log/slimserver.log + echo `date "+%F %H:%M:%S"` "slimserver_safe stopped." >> $LOGFILE exit } trap clean_up SIGINT SIGHUP SIGTERM -echo `date "+%F %H:%M:%S"` "slimserver_safe started." >> /var/log/slimserver.log +echo `date "+%F %H:%M:%S"` "slimserver_safe started." >> $LOGFILE while true do @@ -41,7 +43,7 @@ "$@" > /dev/null 2>&1 & SLIMPID=$! wait $SLIMPID - echo `date "+%F %H:%M:%S"` "Slimserver died. Restarting." >> /var/log/slimserver.log + echo `date "+%F %H:%M:%S"` "Slimserver died. Restarting." >> $LOGFILE # Normally, when slimserver realizes that the mysql-connection is gone, # the mysql server has already been started again. So no need to sleep --- /etc/logrotate.d/slimserver.orig 2007-05-17 15:30:47.000000000 -0400 +++ /etc/logrotate.d/slimserver 2007-05-17 15:32:35.000000000 -0400 @@ -13,3 +13,10 @@ invoke-rc.d slimserver start || true endscript } + +/var/log/slimserver/slimserver_safe.log { + rotate 5 + size 200k + compress + missingok +}
I would disagree with this. I find that server.log (7.0) is a good place for the slimserver_safe entries, as they are clear indications of stop and start points for the rest of the log info. both server.log and scanner.log should already be getting rotated weekly by default. As of 9 days ago, Fletch set the rotation to weekly for 6.5.2 as well as 7. See change 11935
Sure, that'd work, too. The only reason I didn't do that was because I was worried about two processes trying to write to the same log file simultaneously. The main point of the bug report was that having one process logging to "/var/log/slimserver.log" and the other logging to "/var/log/slimserver/slimserver.log" is very confusing.
Here's a modified patch: --- /usr/sbin/slimserver_safe.orig 2007-05-17 15:28:27.000000000 -0400 +++ /usr/sbin/slimserver_safe 2007-05-17 15:30:08.000000000 -0400 @@ -2,6 +2,8 @@ # $Id$ +LOGFILE=/var/log/slimserver/slimserver.log + if [ $# = 0 ] then echo "Script to restart slimserver over and over again." @@ -19,13 +21,13 @@ function clean_up { # Kill the slimserver daemon if it is running kill $SLIMPID - echo `date "+%F %H:%M:%S"` "slimserver_safe stopped." >> /var/log/slimserver.log + echo `date "+%F %H:%M:%S"` "slimserver_safe stopped." >> $LOGFILE exit } trap clean_up SIGINT SIGHUP SIGTERM -echo `date "+%F %H:%M:%S"` "slimserver_safe started." >> /var/log/slimserver.log +echo `date "+%F %H:%M:%S"` "slimserver_safe started." >> $LOGFILE while true do @@ -41,7 +43,7 @@ "$@" > /dev/null 2>&1 & SLIMPID=$! wait $SLIMPID - echo `date "+%F %H:%M:%S"` "Slimserver died. Restarting." >> /var/log/slimserver.log + echo `date "+%F %H:%M:%S"` "Slimserver died. Restarting." >> $LOGFILE # Normally, when slimserver realizes that the mysql-connection is gone, # the mysql server has already been started again. So no need to sleep
(In reply to comment #4) Makes sense to me, with the following additional comments: > +LOGFILE=/var/log/slimserver/slimserver.log 1) This patch is obviously only for the 6.5 branch. It would need to be /var/log/slimserver/server.log for trunk. 2) Do we really want to hardcode $LOGFILE in two different files? That's probably what led to this oversight in the first place. I suggest we add $LOGFILE= or $LOGDIR= to slimserver.default and source that file from both slimserver_safe and slimserver.init.
Hey, I was just re-reading KDFs comments, and do I understand correctly that in 7.0, slimserver_safe is already logging to the server.log file? If so, then I think you don't need to worry about the duplication, because this code has a very short lifespan anyway. If I misread that, and you need a solution applicable to both branches, then your suggestion of abstracting it out is a good one. Another alternative (which might be cleaner) is to add a --log param to the _safe script, and have the init script pass it in, since the init script is the only caller of the _safe script anyway.
(In reply to comment #6) > Hey, I was just re-reading KDFs comments, and do I understand correctly that in > 7.0, slimserver_safe is already logging to the server.log file? If so, then I > think you don't need to worry about the duplication, because this code has a > very short lifespan anyway. > Ah, I misread that. slimserver_safe in trunk does indeed log to the correct place. Your patch for the 6.5 branch seems sensible to me. My second point is probably best considered as a seperate issue.
Since the work is already done, I'll set this for a target of 6.5.3.
(In reply to comment #8) > Since the work is already done, I'll set this for a target of 6.5.3. Has the patch been applied in svn? I last updated 6.5.3 a few days ago, and had to reapply it manually. Or do you just mean that there's a reasonable patch when you say "the work is already done"?
merged at change 12303, please reopen if there are any problems.