Re: Issues with PID file renaming

[Eric Wong <normalperson@yhbt.net>]

Michael Fischer <mfischer@zendesk.com> wrote:
> On Mon, Nov 25, 2013 at 5:20 PM, Eric Wong <normalperson@yhbt.net> wrote:
> 
> >> What I notice is the following: when upgrading with 4.6.2 the file
> >> unicorn.pid is copied to unicorn.pid.oldbin and the unicorn.pid file
> >> is updated (the (new) master process). After a while the worker pid
> >> files are updated and the unicorn.pid.oldbin file disappears, and all
> >> is fine.
> >
> > Ugh.  This is what I feared...  Slow startup time of most Ruby web apps
> > doesn't help.
> 
> Why doesn't the new master write its pid file immediately upon
> forking?  It shouldn't have to wait for everything else to load,
> should it?

It used to do this in 4.6, but I wanted to support dropping the PID
later in case of config failures.  I'll revert that part of the change.

> > How about having the old process create a hard link to .oldbin,
> > and having the new one override the pid if Process.ppid == pid file?
> > The check is still racy, but that's what pid files are :<
> 
> If you decide to implement this, please make it optional.  The current
> behavior is correct IMO, and it will work much better if the above
> issue is addressed.

I've decided against that :)

Just pushed this out to git://bogomips.org/unicorn
as 795c3527337ff4f03ae6db08c5df01141565ed96

---------------------------- 8< -----------------------------
Subject: [PATCH] always write PID file early for compatibility

This reduces the window for a non-existent PID for folks who monitor
PIDs (not a great idea anyways).  Unfortunately, this change also brings
us back to the case where having a PID later (for other process monitors)
is beneficial but more unicorn releases exist where we write the PID
early.

Thanks to Jimmy Soho for reporting this issue.
ref: <CAHStS5gFYcPBDxkVizAHrOeDKAkjT69kruFdgaY0CbB+vLbK8Q@mail.gmail.com>

This partially reverts 7d6ac0c17eb29a00a5b74099dbb3d4d015999f27

Folks: please monitor your app with HTTP requests rather than checking
processes, a stuck/wedged Ruby VM is still a running one.
---
 lib/unicorn/http_server.rb | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index 402f133..f15c8a7 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -136,20 +136,15 @@ class Unicorn::HttpServer
     trap(:CHLD) { awaken_master }
 
     # write pid early for Mongrel compatibility if we're not inheriting sockets
-    # This was needed for compatibility with some health checker a long time
-    # ago.  This unfortunately has the side effect of clobbering valid PID
-    # files.
-    self.pid = config[:pid] unless ENV["UNICORN_FD"]
+    # This is needed for compatibility some Monit setups at least.
+    # This unfortunately has the side effect of clobbering valid PID if
+    # we upgrade and the upgrade breaks during preload_app==true && build_app!
+    self.pid = config[:pid]
 
     self.master_pid = $$
     build_app! if preload_app
     bind_new_listeners!
 
-    # Assuming preload_app==false, we drop the pid file after the app is ready
-    # to process requests.  If binding or build_app! fails with
-    # preload_app==true, we'll never get here and the parent will recover
-    self.pid = config[:pid] if ENV["UNICORN_FD"]
-
     spawn_missing_workers
     self
   end
-- 
EW

_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying