Even accept #1920
Even accept #1920
Conversation
Perform non-blocking socket poll to determine if work is immediately available, to evenly balance work across multiple processes before balancing across multiple threads within a single process, which is less efficient due to Ruby's Global VM Lock.
This makes the loaded accept wait time dynamic, such that less loaded servers will wait less. That should even out the delay experienced by a new client connecting to a highly loaded cluster. This commit also makes the waiting dependent on worker mode with more than one worker.
|
So, next steps here will be to find a benchmark and a benchmarking app that reproduces the original case described in #1646. I tried (#1646 (comment)) but couldn't find anything yet. |
|
I think that the way a typical load generating tool (such as wrk) generates load means that this benchmarking this PR is a little more difficult than usual. I think the benchmarking app I laid out previously will work, but I need to work on the load generation to make it work in a more production-like way. |
|
I’ve been trying this with Siege as well (which emits requests after a set delay rather than after the last request has been responded to) and still not seeing a result. Next step will be for me to reproduce using TechEmpower, which is the only benchmark we can claim has gotten better so far. Sent with GitHawk |
nateberkopec
removed
the
waiting-for-changes
|
I opened this issue #2078, that seems to talk almost about the same problem as this one. However, this cares about a busy workers, but not rather idle ones. @nateberkopec @dentarg What do you think about the other issue and the proposed solution (not a sleep in that form, but something close that looks at thread pool), but by injecting latency earlier. I would think about making this change as a non-changing behavior (disabled by default), and this could be then enabled to improve the performance. I tested the changes introduced by this branch: https://docs.google.com/spreadsheets/d/1y2YqrPPgZ-RtjKiCGplJ7YkwjMXkZx6vEPq7prFbUn4/edit#gid=0. |
There was a problem hiding this comment.
This PR seems to have different behavior from #1646, due to a conflicting change introduced by #1648 (see comment). The result is that this PR basically has no effect which seems consistent with the benchmark tests above.
Compare to master...wjordan:out_of_band (the branch we have been running in production for the past year) that combines the behavior of the two PRs properly by moving the out_of_band logic into wait_until_not_full.
| (busy_threads.to_f / @max) * WORK_AVAILABLE_TIMEOUT_FACTOR | ||
| end | ||
| end | ||
|
|
||
| return busy_threads if @max > busy_threads |
There was a problem hiding this comment.
This line was added by #1648, and makes the behavior of this PR quite different from the one in #1646 (which is based on a commit prior to #1648). Here, the call to @not_full.wait will never be reached unless busy_threads >= @max (ie, the thread pool is maxxed out with requests), which would be an extremely rare case.
I believe this is the reason this PR has no observable effect in benchmarks.
|
Re-ran the TechEmpower benchmark to confirm this PR is currently not working, see #2079 (comment). |
Attempts to spread out work more appropriately. More loaded workers will listen less.