unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help
 help / color / Atom feed
From: Simon Eskildsen <simon.eskildsen@shopify.com>
To: Eric Wong <e@80x24.org>
Cc: unicorn-public@bogomips.org, raindrops-public@bogomips.org
Subject: Re: check_client_connection using getsockopt(2)
Date: Wed, 22 Feb 2017 15:09:38 -0500
Message-ID: <CAO3HKM7m7_9Fr6N8Wobqb3nqKCwJt4rxog=1ZLv_LraYeR9FUw@mail.gmail.com> (raw)
In-Reply-To: <20170222183352.GA28771@starla>

On Wed, Feb 22, 2017 at 1:33 PM, Eric Wong <e@80x24.org> wrote:
> Simon Eskildsen <simon.eskildsen@shopify.com> wrote:
>
> <snip> great to know it's still working after all these years :>
>
>> This confirms Eric's comment that the existing
>> `check_client_connection` works perfectly on loopback, but as soon as
>> you put an actual network between the Unicorn and client—it's only
>> effective 20% of the time, even with `TCP_NODELAY`. I'm assuming, due
>> to buffering, even when disabling Nagle's. As we're changing our
>> architecture, we move from ngx (lb) -> ngx (host) -> unicorn to ngx
>> (lb) -> unicorn. That means this patch will no longer work for us.
>
> Side comment: I'm a bit curious how you guys arrived at removing
> nginx at the host level (if you're allowed to share that info)
>
> I've mainly kept nginx on the same host as unicorn, but used
> haproxy or similar (with minimal buffering) at the LB level.
> That allows better filesystem load distribution for large uploads.


Absolutely. We're starting to experiment with Kubernetes, and a result
we're interested in simplifying ingress as much as possible. We could
still run them, but if I can avoid explaining why they're there for
the next 5 years—I'll be happy :) As I see, the current use-cases we
have for the host nginx are (with why we can get rid of it):

* Keepalive between edge nginx LBs and host LBs to avoid excessive
network traffic. This is not a deal-breaker, so we'll just ignore this
problem. It's a _massive_ amount of traffic normally though.
* Out of rotation. We take nodes gracefully out of rotation by
touching a file that'll return 404 on a health-checking endpoint from
the edge LBs. Kubernetes implements this by just removing all the
containers on that node.
* Graceful deploys. When we deploy with containers, we take each
container out of rotation with the local host Nginx, wait for it to
come up, and put it back in rotation. We don't utilize Unicorns
graceful restarts because we want a Ruby upgrade deploy (replacing the
container) to be the same as an updated code rollout.
* File uploads. As you mention, currently we load-balance this between
them. I haven't finished the investigation on whether this is feasible
on the front LBs. Otherwise we may go for a 2-tier Nginx solution or
expand the edge. However, with Kubernetes I'd really like to avoid
having host nginxes. It's not very native to the Kubernetes paradigm.
Does it balance across the network, or only to the pods on that node?
* check_client_connection working. :-) This thread.

Slow clients and other advantages we find from the edge Nginxes.

>> I propose instead of the early `write(2)` hack, that we use
>> `getsockopt(2)` with the `TCP_INFO` flag and read the state of the
>> socket. If it's in `CLOSE_WAIT` or `CLOSE`, we kill the connection and
>> move on to the next.
>>
>> https://github.com/torvalds/linux/blob/8fa3b6f9392bf6d90cb7b908e07bd90166639f0a/include/uapi/linux/tcp.h#L163
>>
>> This is not going to be portable, but we can do this on only Linux
>> which I suspect is where most production deployments of Unicorn that
>> would benefit from this feature run. It's not useful in development
>> (which is likely more common to not be Linux). We could also introduce
>> it under a new configuration option if desired. In my testing, this
>> works to reject 100% of requests early when not on loopback.
>
> Good to know about it's effectiveness!  I don't mind adding
> Linux-only features as long as existing functionality still
> works on the server-oriented *BSDs.
>
>> The code is essentially:
>>)?
>> def client_closed?
>>   tcp_info = socket.getsockopt(Socket::SOL_TCP, Socket::TCP_INFO)
>>   state = tcp_info.unpack("c")[0]
>>   state == TCP_CLOSE || state == TCP_CLOSE_WAIT
>> end
>
> +Cc: raindrops-public@bogomips.org -> https://bogomips.org/raindrops-public/
>
> Fwiw, raindrops (already a dependency of unicorn) has TCP_INFO
> support under Linux:
>
> https://bogomips.org/raindrops.git/tree/ext/raindrops/linux_tcp_info.c
>
> I haven't used it, much, but FreeBSD also implements a subset of
> this feature, nowadays, and ought to be supportable, too.  We
> can look into adding it for raindrops.

Cool, I think it makes sense to use Raindrops here, advantage being
it'd use the actual struct instead of a sketchy `#unpack`.

I meant to ask, in Raindrops why do you use the netlink API to get the
socket backlog instead of `getsockopt(2)` with `TCP_INFO` to get
`tcpi_unacked`? (as described in
http://www.ryanfrantz.com/posts/apache-tcp-backlog/) We use this to
monitor socket backlogs with a sidekick Ruby daemon. Although we're
looking to replace it with a middleware to simplify for Kubernetes.
It's one of our main metrics for monitoring performance, especially
around deploys.

>
> I don't know about other BSDs...
>
>> This could be called at the top of `#process_client` in `http_server.rb`.
>>
>> Would there be interest in this upstream? Any comments on this
>> proposed implementation? Currently, we're using a middleware with the
>> Rack hijack API, but this seems like it belongs at the webserver
>> level.
>
> I guess hijack means you have to discard other middlewares and
> the normal rack response handling in unicorn?  If so, ouch!
>
> Unfortunately, without hijack, there's no portable way in Rack
> to get at the underlying IO object; so I guess this needs to
> be done at the server level...
>
> So yeah, I guess it belongs in the webserver.

I was going to use `env["unicorn.socket"]`/`env["puma.socket"]`, but
you could also do `env.delete("hijack_io")` after hijacking to allow
Unicorn to still render the response. Unfortunately the
`<webserver>.socket` key is not part of the Rack standard, so I'm
hesitant to use it. When this gets into Unicorn I'm planning to
propose it upstream to Puma as well.

>
> I think we can automatically use TCP_INFO if available for
> check_client_connection; then fall back to the old early write
> hack for Unix sockets and systems w/o TCP_INFO (which would set
> the response_start_sent flag).
>
> No need to introduce yet another configuration option.

Cool. How would you suggest I check for TCP_INFO compatible platforms
in Unicorn? Is `RUBY_PLATFORM.ends_with?("linux".freeze)` sufficient
or do you prefer another mechanism? I agree that we should fall back
to the write hack on other platforms.

  reply index

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-22 12:02 Simon Eskildsen
2017-02-22 18:33 ` Eric Wong
2017-02-22 20:09   ` Simon Eskildsen [this message]
2017-02-23  1:42     ` Eric Wong
2017-02-23  2:42       ` Simon Eskildsen
2017-02-23  3:52         ` Eric Wong

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://bogomips.org/unicorn/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAO3HKM7m7_9Fr6N8Wobqb3nqKCwJt4rxog=1ZLv_LraYeR9FUw@mail.gmail.com' \
    --to=simon.eskildsen@shopify.com \
    --cc=e@80x24.org \
    --cc=raindrops-public@bogomips.org \
    --cc=unicorn-public@bogomips.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help

Archives are clonable:
	git clone --mirror https://bogomips.org/unicorn-public
	git clone --mirror http://ou63pmih66umazou.onion/unicorn-public

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.lang.ruby.unicorn
	nntp://ou63pmih66umazou.onion/inbox.comp.lang.ruby.unicorn

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox