unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help
 help / color / Atom feed
From: Eric Wong <e@yhbt.net>
To: Stan Hu <stanhu@gmail.com>
Cc: unicorn-public@yhbt.net
Subject: Re: Sustained queuing on one listener can block requests from other listeners
Date: Thu, 16 Apr 2020 06:59:17 +0000
Message-ID: <20200416065917.GA7256@dcvr> (raw)
In-Reply-To: <CAMBWrQmzXYjDobExF9GL1BkHAc=ibvMv1ciaSv_fLQ36MyZW0g@mail.gmail.com>

Stan Hu <stanhu@gmail.com> wrote:
> Thanks, Eric. That patch didn't work; it spun the CPU. I think this worked?

Oops, sorry.  I was too eager to drop `nr += 1' :x

Btw, please don't top post.  Fwiw, I wouldn't mind if we stopped
quoting at all on publically-archived lists (saves space and
bandwidth).

> +++ b/lib/unicorn/http_server.rb
> @@ -708,7 +708,7 @@ def worker_loop(worker)
>        # we're probably reasonably busy, so avoid calling select()
>        # and do a speculative non-blocking accept() on ready listeners
>        # before we sleep again in select().
> -      unless nr == 0
> +      if nr == readers.size
>          tmp = ready.dup
>          redo
>        end

Your patch looks close.  However the `readers' array gets
dropped on SIGQUIT with `nuke_listeners!', so `readers.size'
is unstable.

How about this?

diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index a52931a..45a2e97 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -686,6 +686,7 @@ def worker_loop(worker)
     trap(:USR1) { nr = -65536 }
 
     ready = readers.dup
+    nr_listeners = readers.size
     @after_worker_ready.call(self, worker)
 
     begin
@@ -708,7 +709,7 @@ def worker_loop(worker)
       # we're probably reasonably busy, so avoid calling select()
       # and do a speculative non-blocking accept() on ready listeners
       # before we sleep again in select().
-      unless nr == 0
+      if nr == nr_listeners
         tmp = ready.dup
         redo
       end

Thanks

  reply index

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-15  5:06 Stan Hu
2020-04-15  5:26 ` Eric Wong
2020-04-16  5:46   ` Stan Hu
2020-04-16  6:59     ` Eric Wong [this message]
2020-04-16  7:24       ` Stan Hu
2020-04-16  9:24         ` [PATCH] prevent single listener from monopolizing a worker Eric Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://yhbt.net/unicorn/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200416065917.GA7256@dcvr \
    --to=e@yhbt.net \
    --cc=stanhu@gmail.com \
    --cc=unicorn-public@yhbt.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help

Archives are clonable:
	git clone --mirror https://yhbt.net/unicorn-public
	git clone --mirror http://ou63pmih66umazou.onion/unicorn-public

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.lang.ruby.unicorn
	nntp://ou63pmih66umazou.onion/inbox.comp.lang.ruby.unicorn

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git