Add some retry to the service proxy. #2281
Conversation
| glog.V(3).Infof("Mapped service %q to endpoint %s", service, endpoint) | ||
| // TODO: This could spin up a new goroutine to make the outbound connection, | ||
| // and keep accepting inbound traffic. | ||
| outConn, err := net.DialTimeout("tcp", endpoint, endpointDialTimeout) |
There was a problem hiding this comment.
If we're going to retry, we should crank down the dial timeout, no? or maybe start it small and grow it on each failed backend (1sec on first backend, 2 on 2nd, 4 on 3rd)?
|
no love for UDP? |
brendandburns
force-pushed
the
proxy
branch
2 times, most recently
from
830ad7f
to
56a985d
Compare
|
comments addressed. thanks |
brendandburns
force-pushed
the
proxy
branch
2 times, most recently
from
f66346c
to
add8b76
Compare
|
Since @thockin is out, can I have mergez from someone else? Thanks! |
| @@ -76,6 +76,26 @@ type tcpProxySocket struct { | |||
| net.Listener | |||
| } | |||
|
|
|||
| func tryConnect(service string, srcAddr net.Addr, protocol string, proxier *Proxier) (out net.Conn, err error) { | |||
| for retries := 0; retries < 3; retries++ { | |||
There was a problem hiding this comment.
for _, timeout := range endpointDialTimeout?
|
Comments addressed. --brendan |
| // How long we wait for a connection to a backend. | ||
| const endpointDialTimeout = 5 * time.Second | ||
| // How long we wait for a connection to a backend in seconds | ||
| var endpointDialTimeout = []time.Duration{1, 2, 4, 8} |
There was a problem hiding this comment.
I'm really sorry, I should have noticed this before-- can you either make this an []int, or put the x*time.Second up here? It sort of breaks the Duration concept to multiply a duration by a duration, like what happens down on line 89.
There was a problem hiding this comment.
It won't let you do that with an int type variable without a cast, Go must
be doing something magical for 1*time.Second.
So the options are:
Duration[] { 1_time.Second, 2_time.Second, ...}
Or
time.Duration(timeout)*time.Second
Both of which are worse in my opinion.
Brendan
In pkg/proxy/proxier.go:
@@ -56,8 +56,8 @@ func (si *serviceInfo) setActive(val bool) bool {
return tmp
}-// How long we wait for a connection to a backend.
-const endpointDialTimeout = 5 * time.Second
+// How long we wait for a connection to a backend in seconds
+var endpointDialTimeout = []time.Duration{1, 2, 4, 8}
I'm really sorry, I should have noticed this before-- can you either make
this an []int, or put the x*time.Second up here? It sort of breaks the
Duration concept to multiply a duration by a duration, like what happens
down on line 89.
—
Reply to this email directly or view it on GitHub
https://github.com/GoogleCloudPlatform/kubernetes/pull/2281/files#r20266395
.
There was a problem hiding this comment.
ah, right, it works because constants are untyped.
sigh I'd prefer Duration[] { 1_time.Second, 2_time.Second, ...} but I guess I won't make you change it.
|
LGTM |
Add some retry to the service proxy.
No description provided.