Accepting more connections than pool availability

[eprothro]

#1278 Was a fantastic improvement, allowing back pressure to the client via the socket backlog. Failure modes are much more graceful now with Puma. Thank you!

However, recently I was surprised to learn that accepted connections are placed in the thread pool work queue prior to getting placed "in" the Reactor (which would then put them back in the work queue when ready).

I assume this is because in the common case and/or with a fast client (e.g. certain reverse proxy configurations), connections will be ready for read immediately and not need the trip to the Reactor and back into the work queue. This makes sense.

However, with the current logic this means that the Server can still suck up more requests than its pool has actual availability for, especially for large requests or slow-clients.

Imagine the scenario below with 1 worker and 1 thread for simplicity of example (it applies just as well to multi-process and/or multi-thread).

1. thread is waiting (waiting == 1)
2. A connection is accepted (since pool is not full) and placed in work queue
3. The thread pops the connection, it's not ready yet, so it is added to Reactor and the thread finishes
4. The thread is now back in the pool, waiting for more work (waiting is 1 again)

This will continue accepting connections without limit, until the thread(s) are all busy with "real" work.

I verified this experimentally, with a slow client (JMeter with low httpclient.socket.http.cps), and watched requests get sucked up continuously until enough reads completed and all workers were busy generating/writing responses.

I don't think this is an enormous problem, just wanted to mention it since the goal as of #1278 is to not suck up more requests than intended.

One potential solution would be:

• Add a Reactor method returning the count of connections being monitored
• Change how inputs are added to sockets (use of += currently makes these changes opaque to @sockets, here)
• Take the Reactor count into account in addition to @waiting when determining when to accept connections

However, after prototyping with something like that for a bit, I'm realizing that keep-alive functionality is coupled to this "overcommiting" the worker pool, so "fixing" this is likely a significant refactor.

[stereobooster]

Any chance that this issue is behind #1405 ?

[eprothro]

Update: I played with this a bit with nginx and jmeter (fast and slow clients), tweaking when puma decides to accept new connections. After being out of the weeds for a bit, the thoughts that remain:

1. Due to how the thread pool, server, and reactor are coupled, an implementation fully addressing this would probably not be very clean without a non-trivial refactor.

2. Two different kinds of connections build up in the Reactor queue

• Slow clients that we are waiting to read request from (definitely producing more work soon)
• Keep-alive connections that we are checking (may or may not produce more work soon)

A Reactor refactor that threats these differently comes to mind (slight 👀 regarding #1403 (comment)):

• Allows tuning of "overcommit", that is, how many requests we allow to be "waiting for read" that don't have associated, available worker threads.
  • Meter calling accept(2) with available pool capacity #1278 attempted to make this "overcommit" zero, but fails in certain scenarios, as mentioned.
• Delegates handling of keep-alive connections elsewhere so we don't have to consider this "pool" in the context of work that will definitely need to get done.

3. In theory, a reverse proxy bound to by Puma over a unix socket mitigates both of these concerns. In practice, issues like High queue latency over TCP when under heavy load #1405 and throughput with extremely-slow-clients-behind-nginx scenarios pique my curiosity as to whether we're configured as optimally as we think we are.

[jf]

3. The thread pops the connection, it's not ready yet, so it is added to Reactor and the thread finishes

I dont think I can be of much help here, but if I may ask - what do you mean by connection is "not ready yet"? I'm reading https://github.com/puma/puma/blob/master/docs/architecture.md and I dont see how any connection picked up by a worker thread can be "not ready".

[eprothro]

@jf great question (and was mine too before digging into this).

High level explanation of "not ready" yet is that the HTTP request has not come over the wire completely yet. In other words, the connection is established but we can't start work on the request yet (hand off to web application) because we haven't read all the bytes of the request yet.

There are many scenarios, some where this probably wont happen (simple GET) and some scenarios where this might happen (POST with a substantial body and a very slow client).

This is over-simplified since there are a lot of variables here (socket type, nginx config, etc), but that's a high-level explanation of my understanding.

[HoneyryderChuck]

@eprothro last time I checked (2/3 minor versions ago), puma performs both reads and writes from the socket in worker threads, so this is also work for them. Slow clients emitting the request bytes slowly will one way or another fill the waiting sockets buffer and increase pool contention as much as ready requests.

In most deployment scenarios in the wild this won't impact much, as most reverse proxy standard configs just buffer the whole request and dump it to the back-end in 1/2 packets, but in the exceptional cases, I would not see the benefit of the accept then queue approach.

[eprothro]

@HoneyryderChuck I remember thinking that too and was surprised when I contrived a local scenario that proved me wrong.

From the first comment:

However, recently I was surprised to learn that accepted connections are placed in the thread pool work queue prior to getting placed "in" the Reactor (which would then put them back in the work queue when ready).

I assume this is because in the common case and/or with a fast client (e.g. certain reverse proxy configurations), connections will be ready for read immediately and not need the trip to the Reactor and back into the work queue. This makes sense.

So if this is unchanged, and I was right to begin with (neither guaranteed!), slow clients may be able to fill the reactor with more requests than the pool size.

My instinct was/is the same as yours, this is probably not a significant problem in the real world. On the other hand, I and others have seen some failure profiles that make me scratch my head...

In theory, a reverse proxy bound to by Puma over a unix socket mitigates both of these concerns. In practice, issues like #1405 and throughput with extremely-slow-clients-behind-nginx scenarios pique my curiosity as to whether we're configured as optimally as we think we are.

[nateberkopec]

Fixed in #2079.