PID file not removed

[manuelmeurer]

Shouldn't the PID file be removed when I run kill -TERM?

$ cat /etc/remote_syslog.yml
files:
  - /var/log/nginx/access.log
  - /var/log/nginx/error.log

destination:
  host: logs3.papertrailapp.com
  port: 123
  protocol: tls

$ ls /var/run/remote_syslog.pid
ls: cannot access /var/run/remote_syslog.pid: No such file or directory

$ sudo /usr/local/bin/remote_syslog -c /etc/remote_syslog.yml --pid-file=/var/run/remote_syslog.pid

$ ps aux | grep remote_syslog
root     24305  3.0  0.1  11972  6172 ?        Sl   11:09   0:00 /usr/local/bin/remote_syslog -c /etc/remote_syslog.yml --pid-file=/var/run/remote_syslog.pid
admin    24916  0.0  0.0   9732  2044 pts/0    S+   11:09   0:00 grep --color=auto remote_syslog

$ ls /var/run/remote_syslog.pid
-rw------- 1 root root 6 Oct 12 11:03 /var/run/remote_syslog.pid

$ sudo kill -TERM 24305

$ ps aux | grep remote_syslog
admin    24879  0.0  0.0   9732  2172 pts/0    S+   11:09   0:00 grep --color=auto remote_syslog

$ ls /var/run/remote_syslog.pid
-rw------- 1 root root 6 Oct 12 11:03 /var/run/remote_syslog.pid

[troy]

That seems logical to me. I don't think remote_syslog2 is currently using godaemon to handle any signals, but the capability seems to exist.

[manuelmeurer]

I don't know what the best practice is, but I'm using the init script, maybe we could add rm $pid_file in the stop() function?

[leonsodhi]

I'm seeing the same behaviour here on a Linux box.

We could add something into each init script, but that means it would still be broken for anyone that wants to roll their own. It seems like the binary should take care of this.

If anyone digs up anything on the best way to do this in Golang, I'd love to take a look. If I get a chance, I'll see what I can find.

[manuelmeurer]

Here are some examples I found: https://github.com/mindreframer/golang-stuff/blob/master/github.com/youtube/vitess/third_party/go/launchpad.net/gozk/zookeeper/runserver.go#L155 https://github.com/docker/docker/blob/master/pkg/pidfile/pidfile.go https://github.com/etix/mirrorbits/blob/master/process.go#L130 Does that help? Sorry, I'm not experienced in Go, otherwise I would implement it myself in a PR...

[leonsodhi]

Thanks, @manuelmeurer, and that Docker implementation is particularly interesting. Looks like they're handling signals here, which relies on their signal lib and the removal function you linked to.

The mirrorbits implementation was also quite a good read. Seems less sophisticated but might be enough for our needs.

If/when I get a chance, I'll throw something together based on these examples.

[manuelmeurer]

:+1:

[QuinnyPig]

Just tripped over this almost two months later. Awesome.

service remote_syslog stop; service remote_syslog start results in the process not running but reporting back that it is to the service handler.

[troy]

@QuinnyPig: totally agree and on our radar to change. If you happen to get to it first, pull requests are gratefully accepted (and this one would be reviewed and merged quickly).

[Bowbaq]

Just ran into this (@QuinnyPig fancy seeing you here :P). I'm happy to take a stab at fixing, the question is how to go about it.

I'm pretty sure the root cause is in https://github.com/leonsodhi/lockfile/blob/master/lockfile.go#L56-L59. On line 46 in https://golang.org/src/os/exec_unix.go#L39, errFinished is returned when the process is already dead (which is the case after stop). The type conversion fails, and the error from the os package is returned, when really we'd want to return ErrInvalidPid, which would then trigger the cleanup of the PID file.

With that said, it's fairly straightforward to implement signal handling in the daemon & remove it ourselves. Any preference?

[QuinnyPig]

@Bowbaq Small world! I've replaced the init script with something saner (but still an init script); it works really well. Should I submit a pull request, or is this something we're going to stuff into the daemon?

Either way, this init script needs fixing.

[Bowbaq]

@QuinnyPig I'm definitely interested in a good init script, if you don't mind submitting a PR

[huygn]

rm -f $pid_file if the service stopped successfully, however, imo removing the pid file when it start cover more cases and make sure it have a clean start.