Mongrel development - Feb 2008 - [PATCH] avoid needless syscall when num_processors limit is reached

Since we''re going to close the socket immediately after the
num_processors limit is reached, there''s no point in calling
setsockopt(2) to enable TCP_CORK on it.  Instead, only enable
TCP_CORK for connections we are able to handle.

While we''re at it, avoid calling worker_list.length twice in the
connection rejected case and instead just set num_workers to
@workers.list.length once.  I''m assuming the original caching of
worker_list = @workers.list to avoid having the log message
display a different number due to a race condition; and this
preserves that functionality while avoiding an extra method call.

---
 This patch is against
 svn://rubyforge.org/var/svn/mongrel/branches/stable_1-1 r981

 lib/mongrel.rb |   16 +++++++---------
 1 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/lib/mongrel.rb b/lib/mongrel.rb
index d99c56d..cfdfc49 100644
--- a/lib/mongrel.rb
+++ b/lib/mongrel.rb
@@ -270,18 +270,16 @@ module Mongrel
           while true
             begin
               client = @socket.accept
-  
-              if defined?($tcp_cork_opts) and $tcp_cork_opts
-                client.setsockopt(*$tcp_cork_opts) rescue nil
-              end
-  
-              worker_list = @workers.list
-  
-              if worker_list.length >= @num_processors
-                STDERR.puts "Server overloaded with #{worker_list.length}
processors (#@num_processors max). Dropping connection."
+
+              num_workers = @workers.list.length
+              if num_workers >= @num_processors
+                STDERR.puts "Server overloaded with #{num_workers}
processors (#@num_processors max). Dropping connection."
                 client.close rescue nil
                 reap_dead_workers("max processors")
               else
+                if defined?($tcp_cork_opts) and $tcp_cork_opts
+                  client.setsockopt(*$tcp_cork_opts) rescue nil
+                end
                 thread = Thread.new(client) {|c| process_client(c) }
                 thread[:started_on] = Time.now
                 @workers.add(thread)
-- 
Eric Wong

Is there already a ticket for this? I''m not seeing it.

Evan

On Mon, Feb 25, 2008 at 3:01 AM, Eric Wong <normalperson at yhbt.net>
wrote:
> Since we''re going to close the socket immediately after the
>  num_processors limit is reached, there''s no point in calling
>  setsockopt(2) to enable TCP_CORK on it.  Instead, only enable
>  TCP_CORK for connections we are able to handle.
>
>  While we''re at it, avoid calling worker_list.length twice in the
>  connection rejected case and instead just set num_workers to
>  @workers.list.length once.  I''m assuming the original caching of
>  worker_list = @workers.list to avoid having the log message
>  display a different number due to a race condition; and this
>  preserves that functionality while avoiding an extra method call.
>
>  ---
>   This patch is against
>   svn://rubyforge.org/var/svn/mongrel/branches/stable_1-1 r981
>
>   lib/mongrel.rb |   16 +++++++---------
>   1 files changed, 7 insertions(+), 9 deletions(-)
>
>  diff --git a/lib/mongrel.rb b/lib/mongrel.rb
>  index d99c56d..cfdfc49 100644
>  --- a/lib/mongrel.rb
>  +++ b/lib/mongrel.rb
>  @@ -270,18 +270,16 @@ module Mongrel
>            while true
>              begin
>                client = @socket.accept
>  -
>  -              if defined?($tcp_cork_opts) and $tcp_cork_opts
>  -                client.setsockopt(*$tcp_cork_opts) rescue nil
>  -              end
>  -
>  -              worker_list = @workers.list
>  -
>  -              if worker_list.length >= @num_processors
>  -                STDERR.puts "Server overloaded with
#{worker_list.length} processors (#@num_processors max). Dropping
connection."
>  +
>  +              num_workers = @workers.list.length
>  +              if num_workers >= @num_processors
>  +                STDERR.puts "Server overloaded with #{num_workers}
processors (#@num_processors max). Dropping connection."
>                  client.close rescue nil
>                  reap_dead_workers("max processors")
>                else
>  +                if defined?($tcp_cork_opts) and $tcp_cork_opts
>  +                  client.setsockopt(*$tcp_cork_opts) rescue nil
>  +                end
>                  thread = Thread.new(client) {|c| process_client(c) }
>                  thread[:started_on] = Time.now
>                  @workers.add(thread)
>  --
>  Eric Wong
>  _______________________________________________
>  Mongrel-development mailing list
>  Mongrel-development at rubyforge.org
>  http://rubyforge.org/mailman/listinfo/mongrel-development
>


-- 
Evan Weaver
Cloudburst, LLC

Evan Weaver <evan at cloudbur.st> wrote:
> Is there already a ticket for this? I''m not seeing it.
No, not for this patch in particular.  I was just browsing the mongrel
source and this patch just seems like the right thing to do when the
num_processors limit is reached.

I was referring to another patch in that email (ticket/14, --num-procs
not being accepted by mongrel_rails).

They''re related, but this is just a nitpicky patch and the other one
(ticket/14) is a real bug).
> On Mon, Feb 25, 2008 at 3:01 AM, Eric Wong <normalperson at yhbt.net>
wrote:
> > Since we''re going to close the socket immediately after the
> >  num_processors limit is reached, there''s no point in calling
> >  setsockopt(2) to enable TCP_CORK on it.  Instead, only enable
> >  TCP_CORK for connections we are able to handle.
> >
> >  While we''re at it, avoid calling worker_list.length twice in
the
> >  connection rejected case and instead just set num_workers to
> >  @workers.list.length once.  I''m assuming the original
caching of
> >  worker_list = @workers.list to avoid having the log message
> >  display a different number due to a race condition; and this
> >  preserves that functionality while avoiding an extra method call.
> >
> >  ---
> >   This patch is against
> >   svn://rubyforge.org/var/svn/mongrel/branches/stable_1-1 r981
> >
> >   lib/mongrel.rb |   16 +++++++---------
> >   1 files changed, 7 insertions(+), 9 deletions(-)
> >
> >  diff --git a/lib/mongrel.rb b/lib/mongrel.rb
> >  index d99c56d..cfdfc49 100644
> >  --- a/lib/mongrel.rb
> >  +++ b/lib/mongrel.rb
> >  @@ -270,18 +270,16 @@ module Mongrel
> >            while true
> >              begin
> >                client = @socket.accept
> >  -
> >  -              if defined?($tcp_cork_opts) and $tcp_cork_opts
> >  -                client.setsockopt(*$tcp_cork_opts) rescue nil
> >  -              end
> >  -
> >  -              worker_list = @workers.list
> >  -
> >  -              if worker_list.length >= @num_processors
> >  -                STDERR.puts "Server overloaded with
#{worker_list.length} processors (#@num_processors max). Dropping
connection."
> >  +
> >  +              num_workers = @workers.list.length
> >  +              if num_workers >= @num_processors
> >  +                STDERR.puts "Server overloaded with
#{num_workers} processors (#@num_processors max). Dropping connection."
> >                  client.close rescue nil
> >                  reap_dead_workers("max processors")
> >                else
> >  +                if defined?($tcp_cork_opts) and $tcp_cork_opts
> >  +                  client.setsockopt(*$tcp_cork_opts) rescue nil
> >  +                end
> >                  thread = Thread.new(client) {|c| process_client(c) }
> >                  thread[:started_on] = Time.now
> >                  @workers.add(thread)
> >  --
> >  Eric Wong