-
Notifications
You must be signed in to change notification settings - Fork 814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[core] Fix pidfile bug for 5.2.2 #1435
Conversation
log.error(message % self.pidfile) | ||
sys.stderr.write(message % self.pidfile) | ||
sys.exit(1) | ||
# Check if the pid in the pidfile corresponds to a running process |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's use psutil.pid_exists, it's more concise, easier to read and works on Windows (even though this part of the code is not run on Windows) http://pythonhosted.org/psutil/#psutil.pid_exists
fb3c0af
to
d433efa
Compare
And thus add the ability to be restarted properly after a SIGKILL (since the pid in the pidfile is now tested)
d433efa
to
741de61
Compare
Now that datadog-agent runs under supervisord, is it necessary to write pid files like this? |
@remh is this for CentOS5 that we still carry pid files? |
This pull request is to fix a bug that was introduced with 5.2. But you are right a lot of this logic is not needed anymore, we have a card in our board to clean this but it won't be done before 5.4.0. |
Pidfiles for processes spawned by supervisor were introduced to fix #941 (so that old commands |
Gotcha. This looks like an adequate, simple improvement until the day comes when this code can be ripped out. Thanks! |
[core] Fix pidfile bug for 5.2.2
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
It's a legacy command which shouldn't be needed after #1435
And thus add the ability to be restarted properly after a SIGKILL (since
the pid in the pidfile is now tested)