* [PATCH 5/6] worker_loop: get rid of select() avoidance hack
@ 2021-10-01 3:09 4% ` Eric Wong
0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2021-10-01 3:09 UTC (permalink / raw)
To: unicorn-public
It doesn't seem to do anything since commit 221340c4ebc15666
(prevent single listener from monopolizing a worker, 2020-04-16).
---
lib/unicorn/http_server.rb | 25 +++++++------------------
1 file changed, 7 insertions(+), 18 deletions(-)
diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index 09c5ec2..7f33f98 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -699,6 +699,7 @@ def reopen_worker_logs(worker_nr)
logger.info "worker=#{worker_nr} reopening logs..."
Unicorn::Util.reopen_logs
logger.info "worker=#{worker_nr} done reopening logs"
+ false
rescue => e
logger.error(e) rescue nil
exit!(77) # EX_NOPERM in sysexits.h
@@ -709,19 +710,17 @@ def reopen_worker_logs(worker_nr)
# given a INT, QUIT, or TERM signal)
def worker_loop(worker)
readers = init_worker_process(worker)
- nr = 0 # this becomes negative if we need to reopen logs
+ reopen = false
# this only works immediately if the master sent us the signal
# (which is the normal case)
- trap(:USR1) { nr = -65536 }
+ trap(:USR1) { reopen = true }
ready = readers.dup
- nr_listeners = readers.size
@after_worker_ready.call(self, worker)
begin
- nr < 0 and reopen_worker_logs(worker.nr)
- nr = 0
+ reopen = reopen_worker_logs(worker.nr) if reopen
worker.tick = time_now.to_i
tmp = ready.dup
while sock = tmp.shift
@@ -729,26 +728,16 @@ def worker_loop(worker)
# but that will return false
if client = sock.kgio_tryaccept
process_client(client)
- nr += 1
worker.tick = time_now.to_i
end
- break if nr < 0
+ break if reopen
end
- # make the following bet: if we accepted clients this round,
- # 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().
- if nr == nr_listeners
- tmp = ready.dup
- redo
- end
-
- # timeout used so we can detect parent death:
+ # timeout so we can .tick and keep parent from SIGKILL-ing us
worker.tick = time_now.to_i
ret = IO.select(readers, nil, nil, @timeout) and ready = ret[0]
rescue => e
- redo if nr < 0 && readers[0]
+ redo if reopen && readers[0]
Unicorn.log_error(@logger, "listen loop error", e) if readers[0]
end while readers[0]
end
^ permalink raw reply related [relevance 4%]
* [PATCH] prevent single listener from monopolizing a worker
@ 2020-04-16 9:24 14% ` Eric Wong
0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2020-04-16 9:24 UTC (permalink / raw)
To: Stan Hu; +Cc: unicorn-public
Stan Hu <stanhu@gmail.com> wrote:
> That seems to work, thanks!
Thanks for confirming. I'll push the patch below out.
(ugh, dealing with crazy packet loss all around)
Expect a v5.6.0 release within a few days or week at most.
(hopefully no regressions).
And... I wonder, are most deployments nowadays single listener?
I don't think I've used multiple listeners for this aside from
experiments in the early days.
---------8<----------
Subject: [PATCH] prevent single listener from monopolizing a worker
In setups with multiple listeners, it's possible for our greedy
select(2)-avoidance optimization to get pinned on a single, busy
listener and starve the other listener(s).
Prevent starvation by retrying the select(2)-avoidance
optimization if and only if all listeners were active. This
should have no effect on the majority of deployments with only a
single listener.
Thanks for Stan Hu for reporting and testing.
Reported-by: Stan Hu <stanhu@gmail.com>
Tested-by: Stan Hu <stanhu@gmail.com>
Link: https://yhbt.net/unicorn-public/CAMBWrQ=Yh42MPtzJCEO7XryVknDNetRMuA87irWfqVuLdJmiBQ@mail.gmail.com/
---
lib/unicorn/http_server.rb | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
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
^ permalink raw reply related [relevance 14%]
Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2020-04-15 5:06 Sustained queuing on one listener can block requests from other listeners Stan Hu
2020-04-15 5:26 ` Eric Wong
2020-04-16 5:46 ` Stan Hu
2020-04-16 6:59 ` Eric Wong
2020-04-16 7:24 ` Stan Hu
2020-04-16 9:24 14% ` [PATCH] prevent single listener from monopolizing a worker Eric Wong
2021-10-01 3:09 [PATCH 0/6] reduce thundering herds on Linux 4.5+ Eric Wong
2021-10-01 3:09 4% ` [PATCH 5/6] worker_loop: get rid of select() avoidance hack 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).