wajam/nlb — pull request #22 by clementgarnier (2013-10-24 15:03:56+00:00)
Title: Refactored ClientActor with a new connection flow

The previous `ClientActor` connection flow could lead to a race when `becomeAvailable()` was called after the `ClientActor` had been placed back in the pool, took from the pool again and was being used for another transaction: at this point, the reference to the `ForwarderActor` was reset even though it had just been set again. Thus, when the response was coming back from the cluster, the `ClientActor` had no clue where to forward it. This may explain the timeouts we had in production.

In case there is no pooled connection available when the `ForwarderActor` needs it, the flow will now be like:
1. The `ForwarderActor` asks the `SprayConnectionPool` for a connection
2. The `SprayConnectionPool` asks the `PoolSupervisor` for a new `ClientActor`
3. The `PoolSupervisor` props a new `ClientActor`, but does not send it back as it was doing before: wait for step 8
4. The `ClientActor` sends a `Http.Connect` to the destination
5. The `ClientActor` receives a `Http.Connected` indicating that the connection is successful
6. The `ClientActor` sends a `ClientActor.Connected` to the `PoolSupervisor`
7. The `PoolSupervisor` replies to the `SprayConnectionPool` with the now-connected `ClientActor`
8. The `SprayConnectionPool` sends the `ClientActor` back to the `ForwarderActor` that requested it

Still following? :grimacing:

At this point, the `ForwarderActor` can send its request to the `ClientActor`. For a pooled connection, the flow starts at this step and the request is treated in the same way – which is also a significant improvement.

Because tests were no longer passing with this fix, and because they were messy and very hard to debug, I also refactored them using Akka's `TestProbe` instead of the homemade `ProxyActor` trait, with `FlatSpec` and using a fixture trait instead of vars. Will do the same for the remaining tests soon.