Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Meter accept(2) while taking into account races around thread pool mutex #1563

Merged
merged 1 commit into from
May 1, 2018

Commits on Apr 17, 2018

  1. Meter accept(2) while taking into account races around thread pool mutex

    As part of the work in puma#1278 we refactored the conditional in
    `ThreadPool#wait_until_not_full` because we found it difficult to reason about
    at the RailsConf '17 roundtable. We broke it up a bit and everyone went away
    happier. However it turns out that you need to account for both `@waiting` and
    `@todo.size` together to make a determination on whether to accept more work
    from the socket and the refactor was not equivalent to the old code.
    
    There are no guarantees that request threads that have been signalled due to
    work being added to the thread pool's queue will become active, take the mutex
    and decrement `@waiting` before the main worker's thread runs
    `ThreadPool#wait_until_not_full`. If we run that before the request thread
    decrements @waiting we end up hitting the early exit of `return @waiting > 0`,
    taking more work off the socket than the pool's capacity.
    
    The correct way to determine the number of requests we have queued in the pool
    that are outstanding is `@todo.size - @waiting`, so I've reverted to the old
    behaviour and augmented the comment above it.
    dannyfallon committed Apr 17, 2018
    Configuration menu
    Copy the full SHA
    9690d8f View commit details
    Browse the repository at this point in the history