about summary refs log tree commit homepage
diff options
context:
space:
mode:
authorEric Wong <normalperson@yhbt.net>2010-12-20 00:14:52 +0000
committerEric Wong <normalperson@yhbt.net>2010-12-20 00:14:52 +0000
commit7ad59e0c48e12febae2a2fe86b76116c05977c6f (patch)
tree9fe5ba22397adc11adb1b62aa4ae221b1bead596
parent82ea9b442a9edaae6dc3b06a5c61035b2c2924c9 (diff)
downloadunicorn-7ad59e0c48e12febae2a2fe86b76116c05977c6f.tar.gz
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.
-rw-r--r--ext/unicorn_http/unicorn_http.rl38
-rw-r--r--test/unit/test_http_parser_ng.rb72
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?