From 9fe3359ab1a5d88bd721436da854181b6843ca7c Mon Sep 17 00:00:00 2001 From: Jayden Teoh Date: Sun, 1 Oct 2023 17:53:01 +0800 Subject: [PATCH 1/7] disable client side overrides of OS default TCP keepalive and document implemention of server side overrides --- dialoptions.go | 3 +++ examples/helloworld/greeter_server/main.go | 7 ++++++- internal/transport/http2_client.go | 2 +- server.go | 6 ++++++ 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/dialoptions.go b/dialoptions.go index cfc9fd85e8dd..eeaae43eb3a1 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -413,6 +413,9 @@ func WithTimeout(d time.Duration) DialOption { // connections. If FailOnNonTempDialError() is set to true, and an error is // returned by f, gRPC checks the error's Temporary() method to decide if it // should try to reconnect to the network address. +// +// Note: Go overrides the OS defaults for TCP keepalive interval to 15s. +// To retain the OS defaults, pass in a custom dialer with KeepAlive set to a negative duration. func WithContextDialer(f func(context.Context, string) (net.Conn, error)) DialOption { return newFuncDialOption(func(o *dialOptions) { o.copts.Dialer = f diff --git a/examples/helloworld/greeter_server/main.go b/examples/helloworld/greeter_server/main.go index 7a62a9b9ff25..08d8e058acf6 100644 --- a/examples/helloworld/greeter_server/main.go +++ b/examples/helloworld/greeter_server/main.go @@ -25,6 +25,7 @@ import ( "fmt" "log" "net" + "time" "google.golang.org/grpc" pb "google.golang.org/grpc/examples/helloworld/helloworld" @@ -47,7 +48,11 @@ func (s *server) SayHello(ctx context.Context, in *pb.HelloRequest) (*pb.HelloRe func main() { flag.Parse() - lis, err := net.Listen("tcp", fmt.Sprintf(":%d", *port)) + // ListenConfig allows options to be passed to the listener. + var lc net.ListenConfig + // Setting KeepAlive to -1 allows the listener to retain OS default TCP keep-alive interval. + lc.KeepAlive = time.Duration(-1) + lis, err := lc.Listen(context.Background(), "tcp", fmt.Sprintf(":%d", *port)) if err != nil { log.Fatalf("failed to listen: %v", err) } diff --git a/internal/transport/http2_client.go b/internal/transport/http2_client.go index badab8acf3b1..6d142d7f3362 100644 --- a/internal/transport/http2_client.go +++ b/internal/transport/http2_client.go @@ -176,7 +176,7 @@ func dial(ctx context.Context, fn func(context.Context, string) (net.Conn, error if networkType == "tcp" && useProxy { return proxyDial(ctx, address, grpcUA) } - return (&net.Dialer{}).DialContext(ctx, networkType, address) + return (&net.Dialer{KeepAlive: time.Duration(-1)}).DialContext(ctx, networkType, address) } func isTemporary(err error) bool { diff --git a/server.go b/server.go index a7b5532dac2d..6e1b3557a59d 100644 --- a/server.go +++ b/server.go @@ -813,6 +813,12 @@ func (l *listenSocket) Close() error { // Serve returns when lis.Accept fails with fatal errors. lis will be closed when // this method returns. // Serve will return a non-nil error unless Stop or GracefulStop is called. +// +// Note: Go overrides the OS defaults for TCP keepalive interval to 15s. +// To retain the OS defaults, you will need to create a net.ListenConfig with the +// KeepAlive parameter set to a negative value and use the Listen method on it to create +// the net.Listener to pass to this function. +// See https://github.com/grpc/grpc-go/blob/master/examples/helloworld/greeter_client/main.go#L51 func (s *Server) Serve(lis net.Listener) error { s.mu.Lock() s.printf("serving") From 3e5bf86b1891598c7299cffe208682702f69b2a9 Mon Sep 17 00:00:00 2001 From: Jayden Teoh Date: Wed, 11 Oct 2023 23:55:52 +0800 Subject: [PATCH 2/7] minor changes to documentation --- dialoptions.go | 4 ++-- examples/helloworld/greeter_server/main.go | 6 ++---- internal/transport/http2_client.go | 1 + server.go | 9 ++++----- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/dialoptions.go b/dialoptions.go index eeaae43eb3a1..4e3f728fb7c2 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -414,8 +414,8 @@ func WithTimeout(d time.Duration) DialOption { // returned by f, gRPC checks the error's Temporary() method to decide if it // should try to reconnect to the network address. // -// Note: Go overrides the OS defaults for TCP keepalive interval to 15s. -// To retain the OS defaults, pass in a custom dialer with KeepAlive set to a negative duration. +// Note: Go overrides the OS defaults for TCP keepalive time and interval to 15s. +// To retain OS defaults, use a net.Dialer with the KeepAlive field set to a negative value. func WithContextDialer(f func(context.Context, string) (net.Conn, error)) DialOption { return newFuncDialOption(func(o *dialOptions) { o.copts.Dialer = f diff --git a/examples/helloworld/greeter_server/main.go b/examples/helloworld/greeter_server/main.go index 08d8e058acf6..08b623d93300 100644 --- a/examples/helloworld/greeter_server/main.go +++ b/examples/helloworld/greeter_server/main.go @@ -48,10 +48,8 @@ func (s *server) SayHello(ctx context.Context, in *pb.HelloRequest) (*pb.HelloRe func main() { flag.Parse() - // ListenConfig allows options to be passed to the listener. - var lc net.ListenConfig - // Setting KeepAlive to -1 allows the listener to retain OS default TCP keep-alive interval. - lc.KeepAlive = time.Duration(-1) + // Setting KeepAlive to -1 allows the listener to retain OS default TCP keepalive time and interval. + lc := net.ListenConfig{KeepAlive: time.Duration(-1)} lis, err := lc.Listen(context.Background(), "tcp", fmt.Sprintf(":%d", *port)) if err != nil { log.Fatalf("failed to listen: %v", err) diff --git a/internal/transport/http2_client.go b/internal/transport/http2_client.go index 6d142d7f3362..ec72b2d797fb 100644 --- a/internal/transport/http2_client.go +++ b/internal/transport/http2_client.go @@ -176,6 +176,7 @@ func dial(ctx context.Context, fn func(context.Context, string) (net.Conn, error if networkType == "tcp" && useProxy { return proxyDial(ctx, address, grpcUA) } + // KeepAlive is set to a negative value to prevent Go's override of the TCP keepalive time and interval; retain the OS default. return (&net.Dialer{KeepAlive: time.Duration(-1)}).DialContext(ctx, networkType, address) } diff --git a/server.go b/server.go index 6e1b3557a59d..de0feeb7b99d 100644 --- a/server.go +++ b/server.go @@ -814,11 +814,10 @@ func (l *listenSocket) Close() error { // this method returns. // Serve will return a non-nil error unless Stop or GracefulStop is called. // -// Note: Go overrides the OS defaults for TCP keepalive interval to 15s. -// To retain the OS defaults, you will need to create a net.ListenConfig with the -// KeepAlive parameter set to a negative value and use the Listen method on it to create -// the net.Listener to pass to this function. -// See https://github.com/grpc/grpc-go/blob/master/examples/helloworld/greeter_client/main.go#L51 +// Note: Go overrides OS defaults for TCP keepalive time and interval to 15s. +// To retain OS defaults, pass a net.Listener created by calling the Listen method +// on a net.ListenConfig with the `KeepAlive` field set to a negative value. See +// helloworld/greeter_server/main.go for an example. func (s *Server) Serve(lis net.Listener) error { s.mu.Lock() s.printf("serving") From 0b94503152d15f2a0729f70404d9b6a80f3a2ddf Mon Sep 17 00:00:00 2001 From: Jayden Teoh <90945854+JaydenTeoh@users.noreply.github.com> Date: Fri, 20 Oct 2023 01:55:39 +0800 Subject: [PATCH 3/7] Update internal/transport/http2_client.go Co-authored-by: Arvind Bright --- internal/transport/http2_client.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/transport/http2_client.go b/internal/transport/http2_client.go index ec72b2d797fb..19e419473d1a 100644 --- a/internal/transport/http2_client.go +++ b/internal/transport/http2_client.go @@ -176,7 +176,8 @@ func dial(ctx context.Context, fn func(context.Context, string) (net.Conn, error if networkType == "tcp" && useProxy { return proxyDial(ctx, address, grpcUA) } - // KeepAlive is set to a negative value to prevent Go's override of the TCP keepalive time and interval; retain the OS default. + // KeepAlive is set to a negative value to prevent Go's override of the TCP + // keepalive time and interval; retain the OS default. return (&net.Dialer{KeepAlive: time.Duration(-1)}).DialContext(ctx, networkType, address) } From 31381423cda6c322f35d78e9a656c8ebe078b0b1 Mon Sep 17 00:00:00 2001 From: Jayden Teoh Date: Fri, 20 Oct 2023 02:13:26 +0800 Subject: [PATCH 4/7] revert changes to greeter server example --- dialoptions.go | 3 ++- examples/helloworld/greeter_server/main.go | 5 +---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/dialoptions.go b/dialoptions.go index 4e3f728fb7c2..249c67a6ee94 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -415,7 +415,8 @@ func WithTimeout(d time.Duration) DialOption { // should try to reconnect to the network address. // // Note: Go overrides the OS defaults for TCP keepalive time and interval to 15s. -// To retain OS defaults, use a net.Dialer with the KeepAlive field set to a negative value. +// To retain OS defaults, use a net.Dialer with the KeepAlive field set to a +// negative value. func WithContextDialer(f func(context.Context, string) (net.Conn, error)) DialOption { return newFuncDialOption(func(o *dialOptions) { o.copts.Dialer = f diff --git a/examples/helloworld/greeter_server/main.go b/examples/helloworld/greeter_server/main.go index 08b623d93300..7a62a9b9ff25 100644 --- a/examples/helloworld/greeter_server/main.go +++ b/examples/helloworld/greeter_server/main.go @@ -25,7 +25,6 @@ import ( "fmt" "log" "net" - "time" "google.golang.org/grpc" pb "google.golang.org/grpc/examples/helloworld/helloworld" @@ -48,9 +47,7 @@ func (s *server) SayHello(ctx context.Context, in *pb.HelloRequest) (*pb.HelloRe func main() { flag.Parse() - // Setting KeepAlive to -1 allows the listener to retain OS default TCP keepalive time and interval. - lc := net.ListenConfig{KeepAlive: time.Duration(-1)} - lis, err := lc.Listen(context.Background(), "tcp", fmt.Sprintf(":%d", *port)) + lis, err := net.Listen("tcp", fmt.Sprintf(":%d", *port)) if err != nil { log.Fatalf("failed to listen: %v", err) } From 5993a5f5badb4046eb0df9ca26f7db46b28f7be1 Mon Sep 17 00:00:00 2001 From: Jayden Teoh Date: Fri, 20 Oct 2023 02:42:05 +0800 Subject: [PATCH 5/7] document Go version where TCP keepalive defaults was implemented --- dialoptions.go | 7 ++++++- server.go | 10 +++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/dialoptions.go b/dialoptions.go index 249c67a6ee94..1174ea3eee01 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -414,9 +414,14 @@ func WithTimeout(d time.Duration) DialOption { // returned by f, gRPC checks the error's Temporary() method to decide if it // should try to reconnect to the network address. // -// Note: Go overrides the OS defaults for TCP keepalive time and interval to 15s. +// Note: As of Go 1.13, the standard library overrides the OS defaults for +// TCP keepalive time and interval to 15s. // To retain OS defaults, use a net.Dialer with the KeepAlive field set to a // negative value. +// +// For more information, please see [issue 23459] in the Go github repo +// +// [issue 23459]: https://github.com/golang/go/issues/23459 func WithContextDialer(f func(context.Context, string) (net.Conn, error)) DialOption { return newFuncDialOption(func(o *dialOptions) { o.copts.Dialer = f diff --git a/server.go b/server.go index de0feeb7b99d..d981e35752dd 100644 --- a/server.go +++ b/server.go @@ -814,10 +814,14 @@ func (l *listenSocket) Close() error { // this method returns. // Serve will return a non-nil error unless Stop or GracefulStop is called. // -// Note: Go overrides OS defaults for TCP keepalive time and interval to 15s. +// Note: As of Go 1.13, the standard library overrides the OS defaults for +// TCP keepalive time and interval to 15s. // To retain OS defaults, pass a net.Listener created by calling the Listen method -// on a net.ListenConfig with the `KeepAlive` field set to a negative value. See -// helloworld/greeter_server/main.go for an example. +// on a net.ListenConfig with the `KeepAlive` field set to a negative value. +// +// For more information, please see [issue 23459] in the Go github repo +// +// [issue 23459]: https://github.com/golang/go/issues/23459 func (s *Server) Serve(lis net.Listener) error { s.mu.Lock() s.printf("serving") From 06962e38f5eb6d017fbd563b0d86ac523d320cbe Mon Sep 17 00:00:00 2001 From: Jayden Teoh Date: Sat, 28 Oct 2023 01:00:44 +0800 Subject: [PATCH 6/7] add negative duration fix for tcp proxydial and also fix documentation --- dialoptions.go | 2 +- internal/transport/proxy.go | 3 ++- server.go | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/dialoptions.go b/dialoptions.go index 1174ea3eee01..2d4d0528e986 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -414,7 +414,7 @@ func WithTimeout(d time.Duration) DialOption { // returned by f, gRPC checks the error's Temporary() method to decide if it // should try to reconnect to the network address. // -// Note: As of Go 1.13, the standard library overrides the OS defaults for +// Note: As of Go 1.21, the standard library overrides the OS defaults for // TCP keepalive time and interval to 15s. // To retain OS defaults, use a net.Dialer with the KeepAlive field set to a // negative value. diff --git a/internal/transport/proxy.go b/internal/transport/proxy.go index 415961987870..599d8a1d6c64 100644 --- a/internal/transport/proxy.go +++ b/internal/transport/proxy.go @@ -28,6 +28,7 @@ import ( "net/http" "net/http/httputil" "net/url" + "time" ) const proxyAuthHeaderKey = "Proxy-Authorization" @@ -122,7 +123,7 @@ func proxyDial(ctx context.Context, addr string, grpcUA string) (conn net.Conn, newAddr = proxyURL.Host } - conn, err = (&net.Dialer{}).DialContext(ctx, "tcp", newAddr) + conn, err = (&net.Dialer{KeepAlive: time.Duration(-1)}).DialContext(ctx, "tcp", newAddr) if err != nil { return } diff --git a/server.go b/server.go index d981e35752dd..7b98d4962676 100644 --- a/server.go +++ b/server.go @@ -814,7 +814,7 @@ func (l *listenSocket) Close() error { // this method returns. // Serve will return a non-nil error unless Stop or GracefulStop is called. // -// Note: As of Go 1.13, the standard library overrides the OS defaults for +// Note: As of Go 1.21, the standard library overrides the OS defaults for // TCP keepalive time and interval to 15s. // To retain OS defaults, pass a net.Listener created by calling the Listen method // on a net.ListenConfig with the `KeepAlive` field set to a negative value. From 50615a58f3cd493c3a28e9d759390cf586e494b6 Mon Sep 17 00:00:00 2001 From: Jayden Teoh Date: Tue, 7 Nov 2023 13:09:43 +0800 Subject: [PATCH 7/7] add punctuation and default OS config to benchmark --- benchmark/benchmain/main.go | 2 +- dialoptions.go | 2 +- server.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/benchmark/benchmain/main.go b/benchmark/benchmain/main.go index ec26909ba79d..5711540cf87c 100644 --- a/benchmark/benchmain/main.go +++ b/benchmark/benchmain/main.go @@ -373,7 +373,7 @@ func makeClients(bf stats.Features) ([]testgrpc.BenchmarkServiceClient, func()) logger.Fatalf("Failed to listen: %v", err) } opts = append(opts, grpc.WithContextDialer(func(ctx context.Context, address string) (net.Conn, error) { - return nw.ContextDialer((&net.Dialer{}).DialContext)(ctx, "tcp", lis.Addr().String()) + return nw.ContextDialer((&net.Dialer{KeepAlive: time.Duration(-1)}).DialContext)(ctx, "tcp", lis.Addr().String()) })) } lis = nw.Listener(lis) diff --git a/dialoptions.go b/dialoptions.go index 2d4d0528e986..d46ed37db1ec 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -419,7 +419,7 @@ func WithTimeout(d time.Duration) DialOption { // To retain OS defaults, use a net.Dialer with the KeepAlive field set to a // negative value. // -// For more information, please see [issue 23459] in the Go github repo +// For more information, please see [issue 23459] in the Go github repo. // // [issue 23459]: https://github.com/golang/go/issues/23459 func WithContextDialer(f func(context.Context, string) (net.Conn, error)) DialOption { diff --git a/server.go b/server.go index 7b98d4962676..dd2390f49313 100644 --- a/server.go +++ b/server.go @@ -819,7 +819,7 @@ func (l *listenSocket) Close() error { // To retain OS defaults, pass a net.Listener created by calling the Listen method // on a net.ListenConfig with the `KeepAlive` field set to a negative value. // -// For more information, please see [issue 23459] in the Go github repo +// For more information, please see [issue 23459] in the Go github repo. // // [issue 23459]: https://github.com/golang/go/issues/23459 func (s *Server) Serve(lis net.Listener) error {