From 5c700fc2cf398848ddcf71a2aa3f0f2a6563e87b Mon Sep 17 00:00:00 2001 From: Tom Burns Date: Tue, 30 Oct 2012 16:22:21 -0400 Subject: Begin writing HTTP request headers early to detect disconnected clients This patch checks incoming connections and avoids calling the application if the connection has been closed. It works by sending the beginning of the HTTP response before calling the application to see if the socket can successfully be written to. By enabling this feature users can avoid wasting application rendering time only to find the connection is closed when attempting to write, and throwing out the result. When a client disconnects while being queued or processed, Nginx will log HTTP response 499 but the application will log a 200. Enabling this feature will minimize the time window during which the problem can arise. The feature is disabled by default and can be enabled by adding 'check_client_connection true' to the unicorn config. [ew: After testing this change, Tom Burns wrote: So we just finished the US Black Friday / Cyber Monday weekend running unicorn forked with the last version of the patch I had sent you. It worked splendidly and helped us handle huge flash sales without increased response time over the weekend. Whereas in previous flash traffic scenarios we would see the number of HTTP 499 responses grow past the number of real HTTP 200 responses, over the weekend we saw no growth in 499s during flash sales. Unexpectedly the patch also helped us ward off a DoS attack where the attackers were disconnecting immediately after making a request. ref: ] Signed-off-by: Eric Wong --- test/exec/test_exec.rb | 7 ++++++- test/unit/test_configurator.rb | 24 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) (limited to 'test') diff --git a/test/exec/test_exec.rb b/test/exec/test_exec.rb index b30a3d6..1cee2b7 100644 --- a/test/exec/test_exec.rb +++ b/test/exec/test_exec.rb @@ -871,13 +871,14 @@ EOF wait_for_death(pid) end - def hup_test_common(preload) + def hup_test_common(preload, check_client=false) File.open("config.ru", "wb") { |fp| fp.syswrite(HI.gsub("HI", '#$$')) } pid_file = Tempfile.new('pid') ucfg = Tempfile.new('unicorn_test_config') ucfg.syswrite("listen '#@addr:#@port'\n") ucfg.syswrite("pid '#{pid_file.path}'\n") ucfg.syswrite("preload_app true\n") if preload + ucfg.syswrite("check_client_connection true\n") if check_client ucfg.syswrite("stderr_path 'test_stderr.#$$.log'\n") ucfg.syswrite("stdout_path 'test_stdout.#$$.log'\n") pid = xfork { @@ -942,6 +943,10 @@ EOF hup_test_common(false) end + def test_check_client_hup + hup_test_common(false, true) + end + def test_default_listen_hup_holds_listener default_listen_lock do res, pid_path = default_listen_setup diff --git a/test/unit/test_configurator.rb b/test/unit/test_configurator.rb index 267eea3..1298f0e 100644 --- a/test/unit/test_configurator.rb +++ b/test/unit/test_configurator.rb @@ -132,6 +132,30 @@ class TestConfigurator < Test::Unit::TestCase Unicorn::Configurator.new(:config_file => tmp.path) end + def test_check_client_connection + tmp = Tempfile.new('unicorn_config') + test_struct = TestStruct.new + tmp.syswrite("check_client_connection true\n") + + assert_nothing_raised do + Unicorn::Configurator.new(:config_file => tmp.path).commit!(test_struct) + end + + assert test_struct.check_client_connection + end + + def test_check_client_connection_with_tcp_bad + tmp = Tempfile.new('unicorn_config') + test_struct = TestStruct.new + listener = "127.0.0.1:12345" + tmp.syswrite("check_client_connection true\n") + tmp.syswrite("listen '#{listener}', :tcp_nopush => true\n") + + assert_raises(ArgumentError) do + Unicorn::Configurator.new(:config_file => tmp.path).commit!(test_struct) + end + end + def test_after_fork_proc test_struct = TestStruct.new [ proc { |a,b| }, Proc.new { |a,b| }, lambda { |a,b| } ].each do |my_proc| -- cgit v1.2.3-24-ge0c7