grpc-dotnet icon indicating copy to clipboard operation
grpc-dotnet copied to clipboard

Should GrpcChannel provide ChannelBase.ShutdownAsync() functionality?

Open jtattermusch opened this issue 2 years ago • 5 comments

Currently there's a discrepancy in GrpcChannel (grpc-dotnet) and Channel (Grpc.Core) behavior when it comes to shutdown.

  • Both classes inherit from ChannelBase, but only Channel provides a meaningful implementation of channel.ShutdownAsync() (through overriding ChannelBase.ShutdownAsyncCore() method). For GrpcChannel, invoking ShutdownAsync() is a no-op.
  • on the other hand GrpcChannel provides a Dispose() method (not provided by Channel).

https://github.com/grpc/grpc/blob/1cd6e69347cbf62a012477fe184ee6fa8f25d32c/src/csharp/Grpc.Core.Api/ChannelBase.cs#L72

https://github.com/grpc/grpc/blob/1cd6e69347cbf62a012477fe184ee6fa8f25d32c/src/csharp/Grpc.Core/Channel.cs#L214

The issue is that when using channels gRPC in .NET, one must specialize the channel shutdown based on knowing the concrete implementation of a given channel (and use Channel.ShutdownAsync() or GrpcChannel.Dispose() based on that).

Could this be solved by e.g. making GrpcChannel.ShutdownAsync() basically behave the same way as Dispose()?

jtattermusch avatar Apr 27 '22 14:04 jtattermusch

CC @jskeet

jtattermusch avatar Apr 27 '22 14:04 jtattermusch

Does Channel.ShutdownAsync gracefully shutdown? i.e. waits for active calls in progress to finish

It looks like it doesn't.

JamesNK avatar Apr 27 '22 23:04 JamesNK

Does Channel.ShutdownAsync gracefully shutdown? i.e. waits for active calls in progress to finish

It looks like it doesn't.

Nope, it doesn't. The "graceful shutdown" behavior is actually part of Server.cs (which does wait for in-flight calls to finish upon server.ShutdownAsync() and it cancels in flight calls upon server.KillAsync()).

the channel.ShutdownAsync() also doesn't necessarily cancel all the inflight calls (under normal circumstances, you should only shutdown a channel once all the RPCs have finished. If you want RPCs to get automatically cancelled upon channel shutdown in Grpc.Core, you can use Channel.ShutdownToken and explicitly register it with the calls you start.

See the documentation for channelBase.ShutdownAsync() here: https://github.com/grpc/grpc/blob/2badafbc4d246d5165546a61eeb36aa017987d30/src/csharp/Grpc.Core.Api/ChannelBase.cs#L53-L64

jtattermusch avatar Apr 28 '22 09:04 jtattermusch

I would definitely appreciate this, as I was refactoring my solution recently to use "ChannelBase" in the Channel Pool, due to that being the common denominator for the legacy Grpc.Core package and the new implementation. Due to other reasons I don't want to repeat here I am stuck client side to Grpc.Core, so I would appreciate if I now don't have to differentiate between the implementation in my shared lib.... In my opinion, that's what abstraction and polymorphism is for, isn't it :)

Soon5 avatar Apr 30 '22 08:04 Soon5

It looks like this is affecting the Google Cloud Pub/Sub .NET client, see https://github.com/googleapis/google-cloud-dotnet/issues/10304

jeffijoe avatar May 04 '23 21:05 jeffijoe