Skip to content

Error while restarting using SIGHUP#662

Merged
ask merged 1 commit intocelery:masterfrom
ivirabyan:master
May 14, 2012
Merged

Error while restarting using SIGHUP#662
ask merged 1 commit intocelery:masterfrom
ivirabyan:master

Conversation

@ivirabyan
Copy link
Copy Markdown
Contributor

There was an error, saying "Pidfile is already exists" while restarting
using SIGHUP. The problem arises only when "--pidfile" option is used.

Related to: https://github.com/ask/celery/issues/26

@ask
Copy link
Copy Markdown
Contributor

ask commented Apr 16, 2012

Thank you!

This may have been fixed (or partly fixed) by f4fe925 which was released as part of version 2.5.2.

Please review and report :)

Pidfile is removed only when interpreter exists. While restarting using
SIGHUP signal, obviously, interpreter doesn't exit, so pidfile is not
removed. We're fixing this by deleting pidfile in
WorkController._shutdown().
@ivirabyan
Copy link
Copy Markdown
Contributor Author

Thanks for the answer. I've tried 2.5.2, the problem still arises. As I understand ask@f4fe925 is about corrupted pidfiles.
My problem is that pidfile does not get removed when the process is restarted. This is because pidfile is relased on interpreter exit, which is not the case.

Also, I have rewritten my patch, making PIDFile class unchanged. I moved creation/deletion of a pidfile into the WorkController class.

@ask
Copy link
Copy Markdown
Contributor

ask commented May 14, 2012

Aha, so I guess the process id is still alive, so that the stale pidfile detection doesn't work,
thanks I'll merge!

@ask
Copy link
Copy Markdown
Contributor

ask commented May 14, 2012

I have to move it to the end of the stop() method though, because not it could create a race
if the process has not been properly shutdown (imagine a blocking shutdown process and an automatic restarter)

@ask ask merged commit e4400d4 into celery:master May 14, 2012
@ivirabyan
Copy link
Copy Markdown
Contributor Author

The process id is still alive because it is the same process :)

@TylerSheffels
Copy link
Copy Markdown

I think

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants