Re: Combating nginx 499 HTTP responses during flash traffic scenario

From: Eric Wong <normalperson@yhbt.net>
To: unicorn list <mongrel-unicorn@rubyforge.org>
Subject: Re: Combating nginx 499 HTTP responses during flash traffic scenario
Date: Tue, 30 Oct 2012 14:37:19 -0700	[thread overview]
Message-ID: <20121030213719.GA6701@dcvr.yhbt.net> (raw)
In-Reply-To: <CAK4qKG0UPqkbM8HYccx0moNJ7WsCf-e4uu6Eh6DGNvcn58BLYg@mail.gmail.com>

Tom Burns <tom.burns@jadedpixel.com> wrote:
> On Mon, Oct 29, 2012 at 5:53 PM, Eric Wong <normalperson@yhbt.net> wrote:
> > Maybe this gross hack can work for you guys.  It writes the first
> > chunk of the HTTP response header out immediately after reading
> > the request headers, and sends the rest once it gets the status...
> 
> I tested the patch today and it does what we want, dropping
> connections before passing them to the rails app when the client has
> already disconnected.
> 
> I also benchmarked the patch to see if it had a negligible performance
> hit and it did not.  The cost was absorbed by the variation in speed
> of the other components in the stack (nginx & rails).

Good to know.  Thanks for reporting back.

> I noticed on my computer applying the patch breaks
> test_rack_lint_big_put in the unicorn test suite.  This may be just my
> issue as the test suite does not run cleanly anyways if I checkout
> origin/master.

The test suite in master should be passing cleanly, at least on a
GNU/Linux machine...

Yes, this hacky patch breaks some tests/internals and screws up
exception error/reporting badly.

> We'd prefer to not have to fork unicorn for this change.  How do you
> feel about merging this or a derivative thereof?  I can develop this
> further if you can send me what you'd want.

Sure thing!

I strongly prefer this to be optional behavior and off-by-default.

Also, I'm nearly certain two write()s is all that's needed and the
each_char is unnecessary syscall/packet overhead.

This will trigger bugs in badly-written HTTP clients/parsers (probably
some test suites :x) which assume a header comes in a single read().

For TCP users, I believe this requires both tcp_nodelay:false and
tcp_nopush:false to be completely reliable, so we need to enforce that
those if this option is in effect.  The current each_char usage is
probably masking the tcp_nodelay:false requirement.

Thanks again.
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying