And add a special case for only 1 server. Minor win, in any case. benchmark old ns/op new ns/op delta BenchmarkPickServer 262 234 -10.69% BenchmarkPickServer_Single 253 139 -45.06% benchmark old allocs new allocs delta BenchmarkPickServer 1 0 -100.00% BenchmarkPickServer_Single 1 0 -100.00% benchmark old bytes new bytes delta BenchmarkPickServer 8 0 -100.00% BenchmarkPickServer_Single 8 0 -100.00%
…the function SetServers is called the ServerList is replaced by a new set of net.Addr interfaces which no longer match those used as keys for the freeconn map. The underlying data structure is usually a TCPaddr or UnixAddr pointer. Go's equality operation on the map keys looks at the underlying pointer and thinks they are not the same so continues to create new connections assuming it does not have any free.
…on being reused If an error occurs inside the fn() that onItem calls it would have not been spotted by condRelease() and that could result in a bad connection being placed back in the pool of free connections. Under load the package was experiencing this problem manifested in the following fashion: 1. A connection gets an i/o timeout during a call to onItem() 2. The error returned by the function called by onItem() is placed in a new err value (because of the :=) but the condRelease() has been set up with &err from a previous declaration of err. Therefore err is nil. 3. condRelease() places the connection back in the free list. 4. The connection is reused at some point and there's data waiting to be read from the previous memcache command that got the i/o timeout. This results in (typically) an error from the Sscanf line at line 405 because that line ends up trying to parse part of the response that was not previously read because of the i/o timeout. This fix prevents this from happening. If an error occurs the connection will not be reused.