etcd: add a read/write timeout to server #880
Conversation
|
lgtm |
|
@xiangli-cmu I am going to add flags to control this. I will also set a longish timeout on the peer address (5 min) and leave it hard coded unless we have a need to make it shorter. |
| @@ -255,6 +259,9 @@ func (c *Config) LoadFlags(arguments []string) error { | |||
| f.StringVar(&c.Peer.CertFile, "peer-cert-file", c.Peer.CertFile, "") | |||
| f.StringVar(&c.Peer.KeyFile, "peer-key-file", c.Peer.KeyFile, "") | |||
|
|
|||
| f.Float64Var(&c.HTTPReadTimeout, "read-timeout", c.HTTPReadTimeout, "") | |||
There was a problem hiding this comment.
using "http-read-timeout" here?
The name for flags is different from toml's and env's.
The default is for connections to last forever[1]. This leads to fds leaking. I set the timeout so high by default so that watches don't have to keep retrying but perhaps we should set it slower. Tested on a cluster with lots of clients and it seems to have relieved the problem. [1] https://groups.google.com/forum/#!msg/golang-nuts/JFhGwh1q9xU/heh4J8pul3QJ
|
ping @xiangli-cmu @unihorn. Can I merge and release or did you have more thoughts on the write timeout? |
|
lgtm. |
etcd: add a read/write timeout to server
|
@philips this change has uncovered a bad side-effect in go-etcd, specifically in Watch(). When the timeout happens, the http response has an empty body but no check is being done before Unmarshall() is run on it, so you get: What I'm not sure about is whether the empty http body is expected behaviour...if that's the case, then go-etcd should be modified to work around it. |
|
@porjo Thank you for the report. We should fix go-etcd in this case. |
|
Ah, it looks like this has already been reported in coreos/go-etcd#143. I should've checked there first! |
Since etcd-io/etcd#880 watches will timeout after five minutes, this patch reconnects when this happens. Signed-off-by: Jonathan Rudenberg <jonathan@titanous.com>
The default is for connections to last forever[1]. This leads to fds leaking.
I set the timeout so high by default so that watches don't have to keep
retrying but perhaps we should set it slower.
Tested on a cluster with lots of clients and it seems to have relieved the
problem.
[1] https://groups.google.com/forum/#!msg/golang-nuts/JFhGwh1q9xU/heh4J8pul3QJ