Update guidance on underlying TCP connections

[na-ka-na]

Is your feature request related to a problem? Please describe.

We have a python based grpc server and a node js server which acts as the grpc client. Currently on each node js request, we create a new grpc.Client object providing to it the user credentials from the node js request.

  const credentials = grpc.credentials.combineChannelCredentials(SSL_CREDENTIALS, userCredentials);

  const grpcClient = new protoPackage[proto][service](
    API_URL,
    credentials,
    GRPC_OPTIONS
  );

We had a bug where we were not closing the client objects after the rpc call, leading to an explosion in tcp connections and eventually our grpc server would crash due to file handle exhaustion. This was evidenced by lsof output. After closing the clients on node js, the tcp connection leak disappeared.

Describe the solution you'd like

Does the node.js client reuse TCP connections underneath? It was clearly not doing so when we didn't close() the client.

If it does, can you please update the API documentation to say that closing() is an absolutely must. If it does not, please provide an API to reuse TCP connections.

Describe alternatives you've considered

None

Additional context

None

[murgatroid99]

The clients will reuse connections in many situations, but they can't do so in the situation you described. Credentials apply to the whole connection, so clients created with different credentials can't use the same connections. So, in this situation, you should close clients you have finished with so that their connections don't accumulate.

[na-ka-na]

@murgatroid99 Ok that makes sense and explains everything.

Follow up question to

Credentials apply to the whole connection, so clients created with different credentials can't use the same connections

Is this how client SDKs work in other languages too?

[murgatroid99]

Actually, I forgot about something that will help here. Credentials apply to the whole connection the way you are using them, but it is possible to apply just the call credentials (the userCredentials) to individual requests instead of the whole connection. You can do that by setting it in the credentials field of the options method parameter (after the argument object and metadata, before the callback). Then you should construct the client using just the SSL_CREDENTIALS, and you can use the same client object for every request. Even if you don't, if you construct your clients like that, with identical options, they will use the same underlying connections.

[na-ka-na]

This seems to work!

You should really consider updating the documentation. I couldn't even find the signature of generated rpc method.

Thanks

[glenjamin]

I've just run into this exact case.

Is there any way to apply CallCredentials to a client while sharing an underlying Channel or Subchannel? Conceptually this feels like a desirable thing to do, especially when dealing with a variety of incoming user requests from a single process.

In our case we have some middleware which creates Clients with credentials for the incoming request, and then the handlers are free to call whichever gRPC methods they need to after that.

From poking around in the code I was able to discover that the callInvocationTransformer executes at the correct point in the flow to be able to apply per-call metadata changes while sharing the channel - but this wasn't especially clear from the docs.

It is also unclear that using the same address leads to connection re-use, and that adding CallCredentials would stop this re-use - from my reading of the various _equals methods in on ChannelCredentials, the CallCredentials are not considered for equality - so should result in the sharing of a subchannel.

[murgatroid99]

To your first question, no. You can't have multiple different connection-level CallCredentials on the same connection. If you want to use different call credentials on the same connection, you can only do it by passing it in call options as I mentioned in my previous comment. Otherwise, accept that they will use different connections.

The callInvocationTransformer exists primarily because it is needed by grpc-gcp. We don't recommend that others use it.

You were looking at the wrong _equals method. This one in ComposedChannelCredentialsImpl is the one for a channel credential with call credentials. As you can see, it does check for equality of the call credentials.

In general, implicit connection sharing between channels is essentially just an implementation detail. It doesn't really impact how any API behaves.

[glenjamin]

You were looking at the wrong _equals method. This one in ComposedChannelCredentialsImpl is the one for a channel credential with call credentials. As you can see, it does check for equality of the call credentials.

aha, thanks!

In general, implicit connection sharing between channels is essentially just an implementation detail. It doesn't really impact how any API behaves.

While this is functionally true, its quite important from an operational perspective to understand which operations are expensive and which ones are not - so I do think it would be valuable to add some information to the docs.

Would it be reasonable to add these notes to the Client constructor's API docs? If so I can send in a PR for that.