Use a threadpool to handle incoming requests

[blinsay]

Adds a pool of worker threads to handle incoming requests. Uses a bounded channel to pass incoming requests to each worker. Closes #24.

Alternatives

This is an alternative to #25 which uses a semaphore to limit the number of requests in flight but still spawns a thread per request. This version uses existing dependencies instead of rolling our own semaphore.

There's another (easy) alternative to this approach that doesn't use a channel to pass the incoming UnixStream to worker threads:

    let pool = ThreadPool::new(N_WORKERS);

    for stream in listener.incoming() {
        match stream {
            Ok(stream) => {
                let logger = logger.clone();
                pool.execute(move || handle_stream(logger, stream));
            }
            Err(err) => {
                error!(logger, "error accepting connection"; "err" => %err);
                break;
            }
        }
    }

Calling pool.execute like this doesn't apply any backpressure. ThreadPool is build on std::sync::mpsc unbuffered channels so pool.execute never blocks. Using this means we have no signal we can use to do something (log? crash the process?) if things get slow.

On the plus side, this approach is less code and handles panics gracefully - ThreadPool makes sure there are always N_WORKERS threads in the pool. With the approach in the PR, on panic we'll kill a worker thread but nothing starts a new worker task.

I don't have a strong preference between the threadpool approaches, but they both seem better than #25. The pool.execute version requires the least code but doesn't let us detect a hang and crash.

[blinsay]

This is the version of crossbeam-channel we pull in through slog-async.

[leifwalsh]

I definitely prefer something with backpressure to the pool.submit(move || handle_stream(...)) one where some collection of unaccepted tasks grows unbounded out of our view.

I do agree, with the version in this PR, we need some communication back up from threads which errored or panicked to the main thread, possibly crashing the process.

[leifwalsh]

Can we make the last clause just Ok? I'm worried about send_timeout starting to return new error types and us silently ignoring them. I'd rather have an exhaustive match block.

[leifwalsh]

Why do we need another clone here?

[leifwalsh]

Can we log something useful on error?

[blinsay]

Nope. It's just a sentinel telling us the channel is closed.

https://docs.rs/crossbeam-channel/0.5.1/crossbeam_channel/struct.RecvError.html

[leifwalsh]

Can we call handle channel or tx or something instead? "handle" is so generic

[blinsay]

Closed in favor of #28