Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qfix: Add net.Dialer with dialTimeout for nsmgr #395

Conversation

denis-tingaikin
Copy link
Member

@denis-tingaikin denis-tingaikin commented Oct 31, 2021

Signed-off-by: Denis Tingaikin denis.tingajkin@xored.com

Motivation

Since we are not using a grpc.WithBlock option in nsmgr (see at networkservicemesh/sdk#1083)

The grpc is not using our dialTimeout(https://github.com/networkservicemesh/sdk/blob/main/pkg/networkservice/common/dial/client.go#L52) see at

  1. https://github.com/grpc/grpc-go/blob/master/internal/transport/http2_client.go#L173
  2. https://github.com/grpc/grpc-go/blob/master/rpc_util.go#L261-L263

So to avoid problems with the long dial we can simply add for nsmgr dialer with a timeout.

Note: This is required for heal rework networkservicemesh/sdk#1113

To reproduce the problem with long dial just run this:

func TestDial2(t *testing.T) {
   ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond)
   defer cancel()
   cc, err := grpc.DialContext(ctx, "142.251.1.106:53", grpc.WithInsecure())
   if err != nil {
      panic(err.Error())
   }
   if cc == nil {
      panic(cc)
   }
   client := networkservice.NewNetworkServiceClient(cc)
   conn, err := client.Request(context.Background(), &networkservice.NetworkServiceRequest{})
   if err != nil {
      panic(err.Error())
   }
   if conn == nil {
      panic(cc)
   }
}

@denis-tingaikin denis-tingaikin force-pushed the use-dialer-for-dial-timeout branch from d1ea21a to 93f77d4 Compare October 31, 2021 14:53
Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>
@denis-tingaikin denis-tingaikin force-pushed the use-dialer-for-dial-timeout branch from f18ef31 to eede06b Compare October 31, 2021 14:56
@denis-tingaikin denis-tingaikin marked this pull request as draft October 31, 2021 18:23
@denis-tingaikin denis-tingaikin marked this pull request as ready for review November 1, 2021 23:16
Signed-off-by: denis-tingaikin <denis.tingajkin@xored.com>
@denis-tingaikin denis-tingaikin force-pushed the use-dialer-for-dial-timeout branch from 8793d3b to b3d22f6 Compare November 2, 2021 00:20
@edwarnicke edwarnicke merged commit 609ee76 into networkservicemesh:main Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants