Skip to content

Don't use a ThreadGroup for tracking worker threads #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 15 additions & 13 deletions lib/mongrel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ class HttpParams < Hash
# hold your breath until Ruby 1.9 is actually finally useful.
class HttpServer
attr_reader :acceptor
attr_reader :workers
attr_reader :classifier
attr_reader :host
attr_reader :port
Expand Down Expand Up @@ -101,7 +100,7 @@ def initialize(host, port, num_processors=950, throttle=0, timeout=60)
@classifier = URIClassifier.new
@host = host
@port = port
@workers = ThreadGroup.new
@workers = []
@throttle = throttle / 100.0
@num_processors = num_processors
@timeout = timeout
Expand Down Expand Up @@ -215,11 +214,11 @@ def process_client(client)
# currently servicing. It returns the count of workers still active
# after the reap is done. It only runs if there are workers to reap.
def reap_dead_workers(reason='unknown')
if @workers.list.length > 0
STDERR.puts "#{Time.now}: Reaping #{@workers.list.length} threads for slow workers because of '#{reason}'"
if workers.length > 0
STDERR.puts "#{Time.now}: Reaping #{workers.length} threads for slow workers because of '#{reason}'"
error_msg = "Mongrel timed out this thread: #{reason}"
mark = Time.now
@workers.list.each do |worker|
workers.each do |worker|
worker[:started_on] = Time.now if not worker[:started_on]

if mark - worker[:started_on] > @timeout + @throttle
Expand All @@ -229,7 +228,7 @@ def reap_dead_workers(reason='unknown')
end
end

return @workers.list.length
return workers.length
end

# Performs a wait on all the currently running threads and kills any that take
Expand All @@ -238,7 +237,7 @@ def reap_dead_workers(reason='unknown')
# that much longer.
def graceful_shutdown
while reap_dead_workers("shutdown") > 0
STDERR.puts "Waiting for #{@workers.list.length} requests to finish, could take #{@timeout + @throttle} seconds."
STDERR.puts "Waiting for #{workers.length} requests to finish, could take #{@timeout + @throttle} seconds."
sleep @timeout / 10
end
end
Expand Down Expand Up @@ -280,17 +279,15 @@ def run
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."

if workers.length >= @num_processors
STDERR.puts "Server overloaded with #{workers.length} processors (#@num_processors max). Dropping connection."
client.close rescue nil
reap_dead_workers("max processors")
else
thread = Thread.new(client) {|c| process_client(c) }
thread[:started_on] = Time.now
@workers.add(thread)
@workers << thread

sleep @throttle if @throttle > 0
end
Expand Down Expand Up @@ -356,6 +353,11 @@ def stop(synchronous=false)
end
end

def workers
@workers.delete_if { |th| !th.alive? }
@workers
end

end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/mongrel/handlers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ def initialize(ops={})

def process(request, response)
if rand(@sample_rate)+1 == @sample_rate
@processors.sample(listener.workers.list.length)
@processors.sample(listener.workers.length)
@headcount.sample(request.params.length)
@reqsize.sample(request.body.length / 1024.0)
@respsize.sample((response.body.length + response.header.out.length) / 1024.0)
Expand Down
22 changes: 22 additions & 0 deletions test/test_ws.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@ def process(request, response)
end
end

class ThreadSpawnHandler < Mongrel::HttpHandler
attr_reader :ran_test

def process(request, response)
20.times { Thread.new{ sleep 5 } }
@ran_test = true
response.socket.write("HTTP/1.1 200 OK\r\nContent-Type: text/plain\r\n\r\nhello!\n")
end
end


class WebServerTest < Test::Unit::TestCase

Expand All @@ -29,7 +39,9 @@ def setup
end

@tester = TestHandler.new
@threads = ThreadSpawnHandler.new
@server.register("/test", @tester)
@server.register("/threads", @threads)
redirect_test_io do
@server.run
end
Expand Down Expand Up @@ -105,6 +117,16 @@ def test_num_processors_overload
end
end

def test_thread_spawn
thread_spawn_request = "GET /threads HTTP/1.1\r\nHost: www.zedshaw.com\r\nContent-Type: text/plain\r\n\r\n"
redirect_test_io do
assert_nothing_raised do
do_test(thread_spawn_request, 1)
do_test(thread_spawn_request, 1)
end
end
end

def test_file_streamed_request
body = "a" * (Mongrel::Const::MAX_BODY * 2)
long = "GET /test HTTP/1.1\r\nContent-length: #{body.length}\r\n\r\n" + body
Expand Down