Skip to content

Commit

Permalink
Reduce the default TCP timeout to fix #3700. (#3794) (#3814)
Browse files Browse the repository at this point in the history
* Reduce the default TCP timeout to fix #3700.

* Fix lint comments.

* Don't modify http.DefaultTransport but make a copy.
  • Loading branch information
tcnghia authored and knative-prow-robot committed Apr 18, 2019
1 parent de51b2c commit 98e0f5a
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 6 deletions.
19 changes: 16 additions & 3 deletions pkg/activator/util/transports.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@ limitations under the License.
package util

import (
"net"
"net/http"
"time"

h2cutil "github.com/knative/serving/pkg/http/h2c"
"github.com/knative/serving/pkg/network"
)

// RoundTripperFunc implementation roundtrips a request.
Expand All @@ -27,8 +30,8 @@ func (rt RoundTripperFunc) RoundTrip(r *http.Request) (*http.Response, error) {
return rt(r)
}

// NewHTTPTransport will use the appropriate transport for the request's HTTP protocol version
func NewHTTPTransport(v1 http.RoundTripper, v2 http.RoundTripper) http.RoundTripper {
// NewAutoTransport will use the appropriate transport for the request's HTTP protocol version
func NewAutoTransport(v1 http.RoundTripper, v2 http.RoundTripper) http.RoundTripper {
return RoundTripperFunc(func(r *http.Request) (*http.Response, error) {
t := v1
if r.ProtoMajor == 2 {
Expand All @@ -39,5 +42,15 @@ func NewHTTPTransport(v1 http.RoundTripper, v2 http.RoundTripper) http.RoundTrip
})
}

func newHTTPTransport(connTimeout time.Duration) http.RoundTripper {
transport := *http.DefaultTransport.(*http.Transport)
transport.DialContext = (&net.Dialer{
Timeout: connTimeout,
KeepAlive: 30 * time.Second,
DualStack: true,
}).DialContext
return &transport
}

// AutoTransport uses h2c for HTTP2 requests and falls back to `http.DefaultTransport` for all others
var AutoTransport = NewHTTPTransport(http.DefaultTransport, h2cutil.DefaultTransport)
var AutoTransport = NewAutoTransport(newHTTPTransport(network.DefaultConnTimeout), h2cutil.DefaultTransport)
2 changes: 1 addition & 1 deletion pkg/activator/util/transports_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestHTTPRoundTripper(t *testing.T) {
})
}

rt := NewHTTPTransport(frt("v1"), frt("v2"))
rt := NewAutoTransport(frt("v1"), frt("v2"))

examples := []struct {
label string
Expand Down
4 changes: 2 additions & 2 deletions pkg/http/h2c/h2c.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"net/http"
"time"

"github.com/knative/serving/pkg/network"
"golang.org/x/net/http2"
"golang.org/x/net/http2/h2c"
)
Expand Down Expand Up @@ -46,11 +47,10 @@ var DefaultTransport http.RoundTripper = &http2.Transport{
AllowHTTP: true,
DialTLS: func(netw, addr string, cfg *tls.Config) (net.Conn, error) {
d := &net.Dialer{
Timeout: 30 * time.Second,
Timeout: network.DefaultConnTimeout,
KeepAlive: 30 * time.Second,
DualStack: true,
}

return d.Dial(netw, addr)
},
}
14 changes: 14 additions & 0 deletions pkg/network/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package network
import (
"net"
"strings"
"time"

corev1 "k8s.io/api/core/v1"
)
Expand Down Expand Up @@ -55,6 +56,19 @@ const (
// Knative service's DNS name.
DomainTemplateKey = "domainTemplate"

// DefaultConnTimeout specifies a short default connection timeout
// to avoid hitting the issue fixed in
// https://github.com/kubernetes/kubernetes/pull/72534 but only
// avalailable after Kubernetes 1.14.
//
// Our connections are usually between pods in the same cluster
// like activator <-> queue-proxy, or even between containers
// within the same pod queue-proxy <-> user-container, so a
// smaller connect timeout would be justifiable.
//
// We should consider exposing this as a configuration.
DefaultConnTimeout = 200 * time.Millisecond

// DefaultDomainTemplate is the default golang template to use when
// constructing the Knative Route's Domain(host)
DefaultDomainTemplate = "{{.Name}}.{{.Namespace}}.{{.Domain}}"
Expand Down

0 comments on commit 98e0f5a

Please sign in to comment.