From 7ad59e0c48e12febae2a2fe86b76116c05977c6f Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Mon, 20 Dec 2010 00:14:52 +0000 Subject: http: support keepalive_requests directive This limits the number of keepalive requests of a single connection to prevent a single client from monopolizing server resources. On multi-process servers (e.g. Rainbows!) with many keepalive clients per worker process, this can force a client to reconnect and increase its chances of being accepted on a less-busy worker process. This directive is named after the nginx directive which is identical in function. --- ext/unicorn_http/unicorn_http.rl | 38 +++++++++++++++++++-- test/unit/test_http_parser_ng.rb | 72 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 2 deletions(-) diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl index 42fcf1d..ba7ecb3 100644 --- a/ext/unicorn_http/unicorn_http.rl +++ b/ext/unicorn_http/unicorn_http.rl @@ -26,10 +26,34 @@ /* all of these flags need to be set for keepalive to be supported */ #define UH_FL_KEEPALIVE (UH_FL_KAVERSION | UH_FL_REQEOF | UH_FL_HASHEADER) +static unsigned long keepalive_requests = 100; /* same as nginx */ + +/* + * Returns the maximum number of keepalive requests a client may make + * before the parser refuses to continue. + */ +static VALUE ka_req(VALUE self) +{ + return ULONG2NUM(keepalive_requests); +} + +/* + * Sets the maximum number of keepalive requests a client may make. + * A special value of +nil+ causes this to be the maximum value + * possible (this is architecture-dependent). + */ +static VALUE set_ka_req(VALUE self, VALUE val) +{ + keepalive_requests = NIL_P(val) ? ULONG_MAX : NUM2ULONG(val); + + return ka_req(self); +} + /* keep this small for Rainbows! since every client has one */ struct http_parser { int cs; /* Ragel internal state */ unsigned int flags; + unsigned long nr_requests; size_t mark; size_t offset; union { /* these 2 fields don't nest */ @@ -465,6 +489,7 @@ static VALUE HttpParser_init(VALUE self) http_parser_init(hp); hp->buf = rb_str_new(NULL, 0); hp->env = rb_hash_new(); + hp->nr_requests = keepalive_requests; return self; } @@ -628,13 +653,15 @@ static VALUE HttpParser_keepalive(VALUE self) * parser.next? => true or false * * Exactly like HttpParser#keepalive?, except it will reset the internal - * parser state if it returns true. + * parser state on next parse if it returns true. It will also respect + * the maximum *keepalive_requests* value and return false if that is + * reached. */ static VALUE HttpParser_next(VALUE self) { struct http_parser *hp = data_get(self); - if (HP_FL_ALL(hp, KEEPALIVE)) { + if ((HP_FL_ALL(hp, KEEPALIVE)) && (hp->nr_requests-- != 0)) { http_parser_init(hp); hp->flags = UH_FL_TO_CLEAR; return Qtrue; @@ -781,6 +808,13 @@ void Init_unicorn_http(void) */ rb_define_const(cHttpParser, "LENGTH_MAX", OFFT2NUM(UH_OFF_T_MAX)); + /* default value for keepalive_requests */ + rb_define_const(cHttpParser, "KEEPALIVE_REQUESTS_DEFAULT", + ULONG2NUM(keepalive_requests)); + + rb_define_singleton_method(cHttpParser, "keepalive_requests", ka_req, 0); + rb_define_singleton_method(cHttpParser, "keepalive_requests=", set_ka_req, 1); + init_common_fields(); SET_GLOBAL(g_http_host, "HOST"); SET_GLOBAL(g_http_trailer, "TRAILER"); diff --git a/test/unit/test_http_parser_ng.rb b/test/unit/test_http_parser_ng.rb index ea46ea8..8bcb724 100644 --- a/test/unit/test_http_parser_ng.rb +++ b/test/unit/test_http_parser_ng.rb @@ -8,9 +8,81 @@ include Unicorn class HttpParserNgTest < Test::Unit::TestCase def setup + HttpParser.keepalive_requests = HttpParser::KEEPALIVE_REQUESTS_DEFAULT @parser = HttpParser.new end + def test_keepalive_requests_default_constant + assert_kind_of Integer, HttpParser::KEEPALIVE_REQUESTS_DEFAULT + assert HttpParser::KEEPALIVE_REQUESTS_DEFAULT >= 0 + end + + def test_keepalive_requests_setting + HttpParser.keepalive_requests = 0 + assert_equal 0, HttpParser.keepalive_requests + HttpParser.keepalive_requests = nil + assert HttpParser.keepalive_requests >= 0xffffffff + HttpParser.keepalive_requests = 1 + assert_equal 1, HttpParser.keepalive_requests + HttpParser.keepalive_requests = 666 + assert_equal 666, HttpParser.keepalive_requests + + assert_raises(TypeError) { HttpParser.keepalive_requests = "666" } + assert_raises(TypeError) { HttpParser.keepalive_requests = [] } + end + + def test_keepalive_requests_with_next? + req = "GET / HTTP/1.1\r\nHost: example.com\r\n\r\n".freeze + expect = { + "SERVER_NAME" => "example.com", + "HTTP_HOST" => "example.com", + "rack.url_scheme" => "http", + "REQUEST_PATH" => "/", + "SERVER_PROTOCOL" => "HTTP/1.1", + "PATH_INFO" => "/", + "HTTP_VERSION" => "HTTP/1.1", + "REQUEST_URI" => "/", + "SERVER_PORT" => "80", + "REQUEST_METHOD" => "GET", + "QUERY_STRING" => "" + }.freeze + HttpParser::KEEPALIVE_REQUESTS_DEFAULT.times do |nr| + @parser.buf << req + assert_equal expect, @parser.parse + assert @parser.next? + end + @parser.buf << req + assert_equal expect, @parser.parse + assert ! @parser.next? + end + + def test_fewer_keepalive_requests_with_next? + HttpParser.keepalive_requests = 5 + @parser = HttpParser.new + req = "GET / HTTP/1.1\r\nHost: example.com\r\n\r\n".freeze + expect = { + "SERVER_NAME" => "example.com", + "HTTP_HOST" => "example.com", + "rack.url_scheme" => "http", + "REQUEST_PATH" => "/", + "SERVER_PROTOCOL" => "HTTP/1.1", + "PATH_INFO" => "/", + "HTTP_VERSION" => "HTTP/1.1", + "REQUEST_URI" => "/", + "SERVER_PORT" => "80", + "REQUEST_METHOD" => "GET", + "QUERY_STRING" => "" + }.freeze + 5.times do |nr| + @parser.buf << req + assert_equal expect, @parser.parse + assert @parser.next? + end + @parser.buf << req + assert_equal expect, @parser.parse + assert ! @parser.next? + end + def test_default_keepalive_is_off assert ! @parser.keepalive? assert ! @parser.next? -- cgit v1.2.3-24-ge0c7