unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help
 help / color / mirror / code / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
thread overview below | download mbox.gz: |
* Re: [RFC/PATCH] bind listeners after loading for preload_app users
  2012-08-01 20:34 11% ` Eric Wong
  2012-08-01 20:42 11%   ` Nic Benders
  2012-08-01 20:49 11%   ` Ben Somers
@ 2012-08-02 20:54 11%   ` Eric Wong
  2 siblings, 0 replies; 5+ results
From: Eric Wong @ 2012-08-02 20:54 UTC (permalink / raw)
  To: mongrel-unicorn

Eric Wong <normalperson@yhbt.net> wrote:
> Eric Wong <normalperson@yhbt.net> wrote:
> > Hey all, this is probably a sensible change to make for unicorn 4.4.0
> > 
> > There's a small chance of introducing an incompatibility if an app
> > somehow internally depends on having a listen socket ready while it's
> > loading.  A test case to avoid one breakage with Raindrops::Middleware
> > is included.
> > 
> > (original bug reporter Bcc-ed)
> 
> Any comments?   I'm leaning towards applying this to master
> 
> ref: http://mid.gmane.org/20120630001708.GA23513@dcvr.yhbt.net

Thanks both, pushed to master.
(commit 53c375dc933b62b24df2c54d3938b03fa9da1f06)

Looking to release 4.4.0 in a few days (need some more FreeBSD testing)
_______________________________________________
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

^ permalink raw reply	[relevance 11%]

* Re: [RFC/PATCH] bind listeners after loading for preload_app users
  2012-08-01 20:34 11% ` Eric Wong
  2012-08-01 20:42 11%   ` Nic Benders
@ 2012-08-01 20:49 11%   ` Ben Somers
  2012-08-02 20:54 11%   ` Eric Wong
  2 siblings, 0 replies; 5+ results
From: Ben Somers @ 2012-08-01 20:49 UTC (permalink / raw)
  To: unicorn list

> Any comments?   I'm leaning towards applying this to master

Seems like a good change to me, and my team would definitely like it.
_______________________________________________
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

^ permalink raw reply	[relevance 11%]

* Re: [RFC/PATCH] bind listeners after loading for preload_app users
  2012-08-01 20:34 11% ` Eric Wong
@ 2012-08-01 20:42 11%   ` Nic Benders
  2012-08-01 20:49 11%   ` Ben Somers
  2012-08-02 20:54 11%   ` Eric Wong
  2 siblings, 0 replies; 5+ results
From: Nic Benders @ 2012-08-01 20:42 UTC (permalink / raw)
  To: unicorn list

On Wed, Aug 1, 2012 at 1:34 PM, Eric Wong <normalperson@yhbt.net> wrote:
> > Hey all, this is probably a sensible change to make for unicorn 4.4.0
> >
> > There's a small chance of introducing an incompatibility if an app
> > somehow internally depends on having a listen socket ready while it's
> > loading.  A test case to avoid one breakage with Raindrops::Middleware
> > is included.
> >
> > (original bug reporter Bcc-ed)
>
> Any comments?   I'm leaning towards applying this to master


I would like to see this on master.  It would definitely benefit us
here at New Relic.  In addition to the case of a listener being bound
that can't yet serve requests, we also have some trouble caused by
forked processes inheriting the listen sockets during the application
startup.  Not binding them until afterwards would allow us to remove
our special case code around that.

-Nic
_______________________________________________
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

^ permalink raw reply	[relevance 11%]

* Re: [RFC/PATCH] bind listeners after loading for preload_app users
  2012-06-30  0:17 14% [RFC/PATCH] bind listeners after loading for preload_app users Eric Wong
@ 2012-08-01 20:34 11% ` Eric Wong
  2012-08-01 20:42 11%   ` Nic Benders
                     ` (2 more replies)
  0 siblings, 3 replies; 5+ results
From: Eric Wong @ 2012-08-01 20:34 UTC (permalink / raw)
  To: mongrel-unicorn

Eric Wong <normalperson@yhbt.net> wrote:
> Hey all, this is probably a sensible change to make for unicorn 4.4.0
> 
> There's a small chance of introducing an incompatibility if an app
> somehow internally depends on having a listen socket ready while it's
> loading.  A test case to avoid one breakage with Raindrops::Middleware
> is included.
> 
> (original bug reporter Bcc-ed)

Any comments?   I'm leaning towards applying this to master

ref: http://mid.gmane.org/20120630001708.GA23513@dcvr.yhbt.net

Thanks.
_______________________________________________
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

^ permalink raw reply	[relevance 11%]

* [RFC/PATCH] bind listeners after loading for preload_app users
@ 2012-06-30  0:17 14% Eric Wong
  2012-08-01 20:34 11% ` Eric Wong
  0 siblings, 1 reply; 5+ results
From: Eric Wong @ 2012-06-30  0:17 UTC (permalink / raw)
  To: mongrel-unicorn

Hey all, this is probably a sensible change to make for unicorn 4.4.0

There's a small chance of introducing an incompatibility if an app
somehow internally depends on having a listen socket ready while it's
loading.  A test case to avoid one breakage with Raindrops::Middleware
is included.

(original bug reporter Bcc-ed)

>From 0146ae19c22361d5f383cc0f8b962bd9c709d200 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Fri, 29 Jun 2012 16:22:17 -0700
Subject: [PATCH] bind listeners after loading for preload_app users

In the case where preload_app is true, delay binding new
listeners until after loading the application.

Some applications have very long load times (especially Rails
apps with Ruby 1.9.2).  Binding listeners early may cause a load
balancer to incorrectly believe the unicorn workers are ready to
serve traffic even while the app is being loaded.

Once a listener is bound, connect() requests from the load
balancer succeed until the listen backlog is filled.  This
allows requests to pile up for a bit (depending on backlog size)
before getting rejected by the kernel.  By the time the
application is loaded and ready-to-run, requests in the
listen backlog are likely stale and not useful to process.

Processes inheriting listeners do not suffer this effect, as the
old process should still be capable of serving new requests.

This change does not improve the situation for the
preload_app=false (default) use case.  There may not be a
solution for preload_app=false users using large applications.

Fortunately Ruby 1.9.3+ improves load times of large
applications significantly over 1.9.2 so this should be less of
a problem in the future.

Reported via private email sent on 2012-06-29T22:59:10Z
---
 lib/unicorn.rb                        |  2 +-
 lib/unicorn/http_server.rb            | 13 ++++++++++++-
 t/listener_names.ru                   |  4 ++++
 t/t0022-listener_names-preload_app.sh | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 49 insertions(+), 2 deletions(-)
 create mode 100644 t/listener_names.ru
 create mode 100644 t/t0022-listener_names-preload_app.sh

diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index b882ce3..d96ff91 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -82,7 +82,7 @@ module Unicorn
   def self.listener_names
     Unicorn::HttpServer::LISTENERS.map do |io|
       Unicorn::SocketHelper.sock_name(io)
-    end
+    end + Unicorn::HttpServer::NEW_LISTENERS
   end
 
   def self.log_error(logger, prefix, exc)
diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index 14a6f9a..13df55a 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -28,6 +28,9 @@ class Unicorn::HttpServer
   # all bound listener sockets
   LISTENERS = []
 
+  # listeners we have yet to bind
+  NEW_LISTENERS = []
+
   # This hash maps PIDs to Workers
   WORKERS = {}
 
@@ -134,6 +137,7 @@ class Unicorn::HttpServer
 
     self.master_pid = $$
     build_app! if preload_app
+    bind_new_listeners!
     spawn_missing_workers
     self
   end
@@ -738,7 +742,14 @@ class Unicorn::HttpServer
       @init_listeners << Unicorn::Const::DEFAULT_LISTEN
       START_CTX[:argv] << "-l#{Unicorn::Const::DEFAULT_LISTEN}"
     end
-    config_listeners.each { |addr| listen(addr) }
+    NEW_LISTENERS.replace(config_listeners)
+  end
+
+  # call only after calling inherit_listeners!
+  # This binds any listeners we did NOT inherit from the parent
+  def bind_new_listeners!
+    NEW_LISTENERS.each { |addr| listen(addr) }
     raise ArgumentError, "no listeners" if LISTENERS.empty?
+    NEW_LISTENERS.clear
   end
 end
diff --git a/t/listener_names.ru b/t/listener_names.ru
new file mode 100644
index 0000000..edb4e6a
--- /dev/null
+++ b/t/listener_names.ru
@@ -0,0 +1,4 @@
+use Rack::ContentLength
+use Rack::ContentType, "text/plain"
+names = Unicorn.listener_names.inspect # rely on preload_app=true
+run(lambda { |_| [ 200, {}, [ names ] ] })
diff --git a/t/t0022-listener_names-preload_app.sh b/t/t0022-listener_names-preload_app.sh
new file mode 100644
index 0000000..8cb5df8
--- /dev/null
+++ b/t/t0022-listener_names-preload_app.sh
@@ -0,0 +1,32 @@
+#!/bin/sh
+. ./test-lib.sh
+
+# Raindrops::Middleware depends on Unicorn.listener_names,
+# ensure we don't break Raindrops::Middleware when preload_app is true
+
+t_plan 4 "Unicorn.listener_names available with preload_app=true"
+
+t_begin "setup and startup" && {
+	unicorn_setup
+        echo preload_app true >> $unicorn_config
+	unicorn -E none -D listener_names.ru -c $unicorn_config
+	unicorn_wait_start
+}
+
+t_begin "read listener names includes listener" && {
+	resp=$(curl -sSf http://$listen/)
+	ok=false
+	t_info "resp=$resp"
+	case $resp in
+	*\"$listen\"*) ok=true ;;
+	esac
+	$ok
+}
+
+t_begin "killing succeeds" && {
+	kill $unicorn_pid
+}
+
+t_begin "check stderr" && check_stderr
+
+t_done
-- 
Eric Wong
_______________________________________________
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

^ permalink raw reply related	[relevance 14%]

Results 1-5 of 5 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2012-06-30  0:17 14% [RFC/PATCH] bind listeners after loading for preload_app users Eric Wong
2012-08-01 20:34 11% ` Eric Wong
2012-08-01 20:42 11%   ` Nic Benders
2012-08-01 20:49 11%   ` Ben Somers
2012-08-02 20:54 11%   ` Eric Wong

Code repositories for project(s) associated with this public inbox

	https://yhbt.net/unicorn.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).