KushalP/serfclient-py — pull request #5 by derpston (2014-06-01 19:53:51+00:00)
Title: Not using a fixed size to read from the RPC socket 

...because one can't know how large an RPC response is going to be.

Instead of reading a fixed size and deserialising all msgpack messages inside it, RPC calls have knowledge of how many msgpack responses they expect and they keep reading until satisfied or hitting a timeout.

This is intended to fix a problem I was having with a serf cluster of 27/28 hosts. The members call worked fine until I added the 28th, at which point the fixed socket read size of 4k wasn't enough and I started getting responses like this:

```
SerfResult
SerfResult
SerfResult
SerfResult
SerfResult
SerfResult
SerfResult
SerfResult
SerfResult
SerfResult
```

All I was doing here was calling SerfClient.members() repeatedly:

```python
import serfclient
sc = serfclient.SerfClient()
for index in xrange(10):
print sc.members()
```

Reading a fixed amount from the RPC socket means that we don't get a full msgpack message when we return a response to the user, but the rest of the msgpack content is still in the socket buffer the next time we make an RPC call, leading to all kinds of unpredictable deserialisation results.

So, this change reads a variable amount from the RPC socket. The trouble with this is that there's no delimiter between serf RPC responses, so we don't know how much to read or how long to wait. The cleanest thing to do appears to be keep reading from the socket and feeding into the msgpack deserialiser until we get the number of messages we expect, and then we know we're done and can give the user a response.

The problem with this is that now the RPC caller needs to know how many msgpack messages it should expect in reply. For example, `handshake` has only a header and not a body, while `members` has a head and a body.

My biggest concern with this change is where the knowledge of the structure of the RPC response should live. I see two options:

1) SerfConnection exposes the need to know if it should wait for a body response as well as a head. This complicates SerfClient somewhat in that each RPC function needs to know this information and give it to SerfConnection.

2) Hide this mapping inside SerfConnection, so SerfClient never needs to know how many responses to expect.

I initially started implementing (2) but realised that it'd be annoying for someone else implementing a new RPC call to have it mysteriously break when they don't remember to update the mapping inside SerfConnection. For that reason, I went with option (1).

I don't like option (1) much because I don't think someone extending the functionality of SerfClient should need to know an implementation detail like whether a `force-leave` RPC call will generate a head, or a head and a body. I think the best way to avoid this problem would be for serf to include a flag in the head message meaning 'a body message follows'. If that were the case, SerfConnection could always do the right thing and SerfClient could remain ignorant of this protocol implementation detail.

I'm undecided about whether it's worth trying to get the Serf developers (or the developer of the go RPC library Serf uses...) to implement this small extension, but for now I propose this solution.