From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS33070 50.56.128.0/17 X-Spam-Status: No, score=-0.2 required=5.0 tests=AWL shortcircuit=no autolearn=unavailable version=3.3.2 X-Original-To: archivist@yhbt.net Delivered-To: archivist@dcvr.yhbt.net Received: from rubyforge.org (50-56-192-79.static.cloud-ips.com [50.56.192.79]) by dcvr.yhbt.net (Postfix) with ESMTP id 2C8661F4D4 for ; Sat, 30 Jun 2012 00:40:13 +0000 (UTC) Received: from localhost.localdomain (localhost [127.0.0.1]) by rubyforge.org (Postfix) with ESMTP id 0B8C42E065; Sat, 30 Jun 2012 00:40:12 +0000 (UTC) X-Original-To: mongrel-unicorn@rubyforge.org Delivered-To: mongrel-unicorn@rubyforge.org Received: from dcvr.yhbt.net (dcvr.yhbt.net [64.71.152.64]) by rubyforge.org (Postfix) with ESMTP id D28052E064 for ; Sat, 30 Jun 2012 00:17:09 +0000 (UTC) Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id DE9871F42D; Sat, 30 Jun 2012 00:17:08 +0000 (UTC) Date: Sat, 30 Jun 2012 00:17:08 +0000 From: Eric Wong To: mongrel-unicorn@rubyforge.org Subject: [RFC/PATCH] bind listeners after loading for preload_app users Message-ID: <20120630001708.GA23513@dcvr.yhbt.net> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: mongrel-unicorn@rubyforge.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: mongrel-unicorn-bounces@rubyforge.org Errors-To: mongrel-unicorn-bounces@rubyforge.org 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 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