From 49fb84ab643fd1a676c526482b6e3960046262b9 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 3 Jun 2016 16:34:48 -0700 Subject: [PATCH 1/4] rename ChannelState.FatalFailure to ChannelState.Shutdown --- src/csharp/Grpc.Core.Tests/ChannelTest.cs | 6 +++--- src/csharp/Grpc.Core/Channel.cs | 14 +++++++------- src/csharp/Grpc.Core/ChannelState.cs | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/ChannelTest.cs b/src/csharp/Grpc.Core.Tests/ChannelTest.cs index 850d70ce926..db0ef3a4cd3 100644 --- a/src/csharp/Grpc.Core.Tests/ChannelTest.cs +++ b/src/csharp/Grpc.Core.Tests/ChannelTest.cs @@ -71,7 +71,7 @@ namespace Grpc.Core.Tests public void WaitForStateChangedAsync_InvalidArgument() { var channel = new Channel("localhost", ChannelCredentials.Insecure); - Assert.ThrowsAsync(typeof(ArgumentException), async () => await channel.WaitForStateChangedAsync(ChannelState.FatalFailure)); + Assert.ThrowsAsync(typeof(ArgumentException), async () => await channel.WaitForStateChangedAsync(ChannelState.Shutdown)); channel.ShutdownAsync().Wait(); } @@ -102,11 +102,11 @@ namespace Grpc.Core.Tests } [Test] - public async Task StateIsFatalFailureAfterShutdown() + public async Task StateIsShutdownAfterShutdown() { var channel = new Channel("localhost", ChannelCredentials.Insecure); await channel.ShutdownAsync(); - Assert.AreEqual(ChannelState.FatalFailure, channel.State); + Assert.AreEqual(ChannelState.Shutdown, channel.State); } [Test] diff --git a/src/csharp/Grpc.Core/Channel.cs b/src/csharp/Grpc.Core/Channel.cs index 886adfec334..2ec763bec15 100644 --- a/src/csharp/Grpc.Core/Channel.cs +++ b/src/csharp/Grpc.Core/Channel.cs @@ -104,7 +104,7 @@ namespace Grpc.Core /// /// Gets current connectivity state of this channel. - /// After channel is has been shutdown, ChannelState.FatalFailure will be returned. + /// After channel is has been shutdown, ChannelState.Shutdown will be returned. /// public ChannelState State { @@ -121,8 +121,8 @@ namespace Grpc.Core /// public Task WaitForStateChangedAsync(ChannelState lastObservedState, DateTime? deadline = null) { - GrpcPreconditions.CheckArgument(lastObservedState != ChannelState.FatalFailure, - "FatalFailure is a terminal state. No further state changes can occur."); + GrpcPreconditions.CheckArgument(lastObservedState != ChannelState.Shutdown, + "Shutdown is a terminal state. No further state changes can occur."); var tcs = new TaskCompletionSource(); var deadlineTimespec = deadline.HasValue ? Timespec.FromDateTime(deadline.Value) : Timespec.InfFuture; var handler = new BatchCompletionDelegate((success, ctx) => @@ -172,7 +172,7 @@ namespace Grpc.Core /// /// Allows explicitly requesting channel to connect without starting an RPC. /// Returned task completes once state Ready was seen. If the deadline is reached, - /// or channel enters the FatalFailure state, the task is cancelled. + /// or channel enters the Shutdown state, the task is cancelled. /// There is no need to call this explicitly unless your use case requires that. /// Starting an RPC on a new channel will request connection implicitly. /// @@ -182,9 +182,9 @@ namespace Grpc.Core var currentState = GetConnectivityState(true); while (currentState != ChannelState.Ready) { - if (currentState == ChannelState.FatalFailure) + if (currentState == ChannelState.Shutdown) { - throw new OperationCanceledException("Channel has reached FatalFailure state."); + throw new OperationCanceledException("Channel has reached Shutdown state."); } await WaitForStateChangedAsync(currentState, deadline).ConfigureAwait(false); currentState = GetConnectivityState(false); @@ -264,7 +264,7 @@ namespace Grpc.Core } catch (ObjectDisposedException) { - return ChannelState.FatalFailure; + return ChannelState.Shutdown; } } diff --git a/src/csharp/Grpc.Core/ChannelState.cs b/src/csharp/Grpc.Core/ChannelState.cs index d293b98f753..a6c3b2a4889 100644 --- a/src/csharp/Grpc.Core/ChannelState.cs +++ b/src/csharp/Grpc.Core/ChannelState.cs @@ -64,6 +64,6 @@ namespace Grpc.Core /// /// Channel has seen a failure that it cannot recover from /// - FatalFailure + Shutdown } } From e1bee5bd599396aa3c1171ef2b5af20deff58ee5 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 3 Jun 2016 16:41:50 -0700 Subject: [PATCH 2/4] improve comments in math.proto --- src/csharp/Grpc.Examples/MathGrpc.cs | 26 +++++++++++++------------- src/proto/math/math.proto | 6 +++--- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/csharp/Grpc.Examples/MathGrpc.cs b/src/csharp/Grpc.Examples/MathGrpc.cs index 9319e311ee6..4bbefcbe01a 100644 --- a/src/csharp/Grpc.Examples/MathGrpc.cs +++ b/src/csharp/Grpc.Examples/MathGrpc.cs @@ -85,8 +85,8 @@ namespace Math { public abstract class MathBase { /// - /// Div divides args.dividend by args.divisor and returns the quotient and - /// remainder. + /// Div divides DivArgs.dividend by DivArgs.divisor and returns the quotient + /// and remainder. /// public virtual global::System.Threading.Tasks.Task Div(global::Math.DivArgs request, ServerCallContext context) { @@ -105,7 +105,7 @@ namespace Math { } /// - /// Fib generates numbers in the Fibonacci sequence. If args.limit > 0, Fib + /// Fib generates numbers in the Fibonacci sequence. If FibArgs.limit > 0, Fib /// generates up to limit numbers; otherwise it continues until the call is /// canceled. Unlike Fib above, Fib has no final FibReply. /// @@ -144,32 +144,32 @@ namespace Math { } /// - /// Div divides args.dividend by args.divisor and returns the quotient and - /// remainder. + /// Div divides DivArgs.dividend by DivArgs.divisor and returns the quotient + /// and remainder. /// public virtual global::Math.DivReply Div(global::Math.DivArgs request, Metadata headers = null, DateTime? deadline = null, CancellationToken cancellationToken = default(CancellationToken)) { return Div(request, new CallOptions(headers, deadline, cancellationToken)); } /// - /// Div divides args.dividend by args.divisor and returns the quotient and - /// remainder. + /// Div divides DivArgs.dividend by DivArgs.divisor and returns the quotient + /// and remainder. /// public virtual global::Math.DivReply Div(global::Math.DivArgs request, CallOptions options) { return CallInvoker.BlockingUnaryCall(__Method_Div, null, options, request); } /// - /// Div divides args.dividend by args.divisor and returns the quotient and - /// remainder. + /// Div divides DivArgs.dividend by DivArgs.divisor and returns the quotient + /// and remainder. /// public virtual AsyncUnaryCall DivAsync(global::Math.DivArgs request, Metadata headers = null, DateTime? deadline = null, CancellationToken cancellationToken = default(CancellationToken)) { return DivAsync(request, new CallOptions(headers, deadline, cancellationToken)); } /// - /// Div divides args.dividend by args.divisor and returns the quotient and - /// remainder. + /// Div divides DivArgs.dividend by DivArgs.divisor and returns the quotient + /// and remainder. /// public virtual AsyncUnaryCall DivAsync(global::Math.DivArgs request, CallOptions options) { @@ -196,7 +196,7 @@ namespace Math { return CallInvoker.AsyncDuplexStreamingCall(__Method_DivMany, null, options); } /// - /// Fib generates numbers in the Fibonacci sequence. If args.limit > 0, Fib + /// Fib generates numbers in the Fibonacci sequence. If FibArgs.limit > 0, Fib /// generates up to limit numbers; otherwise it continues until the call is /// canceled. Unlike Fib above, Fib has no final FibReply. /// @@ -205,7 +205,7 @@ namespace Math { return Fib(request, new CallOptions(headers, deadline, cancellationToken)); } /// - /// Fib generates numbers in the Fibonacci sequence. If args.limit > 0, Fib + /// Fib generates numbers in the Fibonacci sequence. If FibArgs.limit > 0, Fib /// generates up to limit numbers; otherwise it continues until the call is /// canceled. Unlike Fib above, Fib has no final FibReply. /// diff --git a/src/proto/math/math.proto b/src/proto/math/math.proto index 311e148c021..269c60bde83 100644 --- a/src/proto/math/math.proto +++ b/src/proto/math/math.proto @@ -55,8 +55,8 @@ message FibReply { } service Math { - // Div divides args.dividend by args.divisor and returns the quotient and - // remainder. + // Div divides DivArgs.dividend by DivArgs.divisor and returns the quotient + // and remainder. rpc Div (DivArgs) returns (DivReply) { } @@ -67,7 +67,7 @@ service Math { rpc DivMany (stream DivArgs) returns (stream DivReply) { } - // Fib generates numbers in the Fibonacci sequence. If args.limit > 0, Fib + // Fib generates numbers in the Fibonacci sequence. If FibArgs.limit > 0, Fib // generates up to limit numbers; otherwise it continues until the call is // canceled. Unlike Fib above, Fib has no final FibReply. rpc Fib (FibArgs) returns (stream Num) { From 6acc07d50dd33a63f492084ae035b268324bcff3 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 3 Jun 2016 17:03:30 -0700 Subject: [PATCH 3/4] improve the xmldocs on call cancellation and disposal --- src/csharp/Grpc.Core/AsyncDuplexStreamingCall.cs | 4 ++++ src/csharp/Grpc.Core/AsyncServerStreamingCall.cs | 4 ++++ src/csharp/Grpc.Core/AsyncUnaryCall.cs | 4 ++++ src/csharp/Grpc.Core/CallOptions.cs | 8 +++++++- 4 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/csharp/Grpc.Core/AsyncDuplexStreamingCall.cs b/src/csharp/Grpc.Core/AsyncDuplexStreamingCall.cs index e75108c7e57..68fd6d0b05e 100644 --- a/src/csharp/Grpc.Core/AsyncDuplexStreamingCall.cs +++ b/src/csharp/Grpc.Core/AsyncDuplexStreamingCall.cs @@ -117,6 +117,10 @@ namespace Grpc.Core /// Otherwise, requests cancellation of the call which should terminate all pending async operations associated with the call. /// As a result, all resources being used by the call should be released eventually. /// + /// + /// Normally, there is no need for you to dispose the call unless you want to utilize the + /// "Cancel" semantics of invoking Dispose. + /// public void Dispose() { disposeAction.Invoke(); diff --git a/src/csharp/Grpc.Core/AsyncServerStreamingCall.cs b/src/csharp/Grpc.Core/AsyncServerStreamingCall.cs index f9530919848..5777c726152 100644 --- a/src/csharp/Grpc.Core/AsyncServerStreamingCall.cs +++ b/src/csharp/Grpc.Core/AsyncServerStreamingCall.cs @@ -103,6 +103,10 @@ namespace Grpc.Core /// Otherwise, requests cancellation of the call which should terminate all pending async operations associated with the call. /// As a result, all resources being used by the call should be released eventually. /// + /// + /// Normally, there is no need for you to dispose the call unless you want to utilize the + /// "Cancel" semantics of invoking Dispose. + /// public void Dispose() { disposeAction.Invoke(); diff --git a/src/csharp/Grpc.Core/AsyncUnaryCall.cs b/src/csharp/Grpc.Core/AsyncUnaryCall.cs index 97df8f5e91b..d180c27922d 100644 --- a/src/csharp/Grpc.Core/AsyncUnaryCall.cs +++ b/src/csharp/Grpc.Core/AsyncUnaryCall.cs @@ -112,6 +112,10 @@ namespace Grpc.Core /// Otherwise, requests cancellation of the call which should terminate all pending async operations associated with the call. /// As a result, all resources being used by the call should be released eventually. /// + /// + /// Normally, there is no need for you to dispose the call unless you want to utilize the + /// "Cancel" semantics of invoking Dispose. + /// public void Dispose() { disposeAction.Invoke(); diff --git a/src/csharp/Grpc.Core/CallOptions.cs b/src/csharp/Grpc.Core/CallOptions.cs index 9ca88849ee5..35548cfc96b 100644 --- a/src/csharp/Grpc.Core/CallOptions.cs +++ b/src/csharp/Grpc.Core/CallOptions.cs @@ -88,7 +88,13 @@ namespace Grpc.Core } /// - /// Token that can be used for cancelling the call. + /// Token that can be used for cancelling the call on the client side. + /// Cancelling the token will request cancellation + /// of the remote call. Best effort will be made to deliver the cancellation + /// notification to the server and interaction of the call with the server side + /// will be terminated. Unless the call finishes before the cancellation could + /// happen (there is an inherent race), + /// the call will finish with StatusCode.Cancelled status. /// public CancellationToken CancellationToken { From a134fa7036d51e539714fa41b0ec780e084bab46 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 3 Jun 2016 17:24:50 -0700 Subject: [PATCH 4/4] improve docs on ShutdownAsync() for servers and channels --- src/csharp/Grpc.Core/Channel.cs | 11 +++++++++-- src/csharp/Grpc.Core/Server.cs | 6 ++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/csharp/Grpc.Core/Channel.cs b/src/csharp/Grpc.Core/Channel.cs index 2ec763bec15..9cee7526630 100644 --- a/src/csharp/Grpc.Core/Channel.cs +++ b/src/csharp/Grpc.Core/Channel.cs @@ -192,9 +192,16 @@ namespace Grpc.Core } /// - /// Waits until there are no more active calls for this channel and then cleans up - /// resources used by this channel. + /// Shuts down the channel cleanly. It is strongly recommended to shutdown + /// all previously created channels before exiting from the process. /// + /// + /// This method doesn't wait for all calls on this channel to finish (nor does + /// it explicitly cancel all outstanding calls). It is user's responsibility to make sure + /// all the calls on this channel have finished (successfully or with an error) + /// before shutting down the channel to ensure channel shutdown won't impact + /// the outcome of those remote calls. + /// public async Task ShutdownAsync() { lock (myLock) diff --git a/src/csharp/Grpc.Core/Server.cs b/src/csharp/Grpc.Core/Server.cs index 069185e13af..6bd79005617 100644 --- a/src/csharp/Grpc.Core/Server.cs +++ b/src/csharp/Grpc.Core/Server.cs @@ -152,6 +152,9 @@ namespace Grpc.Core /// cleans up used resources. The returned task finishes when shutdown procedure /// is complete. /// + /// + /// It is strongly recommended to shutdown all previously created servers before exiting from the process. + /// public async Task ShutdownAsync() { lock (myLock) @@ -173,6 +176,9 @@ namespace Grpc.Core /// Requests server shutdown while cancelling all the in-progress calls. /// The returned task finishes when shutdown procedure is complete. /// + /// + /// It is strongly recommended to shutdown all previously created servers before exiting from the process. + /// public async Task KillAsync() { lock (myLock)