Looks good. Two questions (both could be deferred to future work):

1. Would it be a good idea to try and shutdown all the clients in parallel, rather than waiting for each one?

2. Couldn't each channel have its own timeout? It looks like the model you made allows it, but then ignores them. If it's too complicated we'd either need a new abstraction in core, or we could pull the global timeout up out of the NamedChannel.

Looks good. Two questions (both could be deferred to future work):

1. Would it be a good idea to try and shutdown all the clients in parallel, rather than waiting for each one?

2. Couldn't each channel have its own timeout? It looks like the model you made allows it, but then ignores them. If it's too complicated we'd either need a new abstraction in core, or we could pull the global timeout up out of the NamedChannel.

I think it's a good idea to develop in the future. I guess the grace period for the client implies a grace period for each channel

However, making a grace period for all channels simultaneously and for individual channels is a good idea that can be developed in the future

Perhaps this can be put into two separate parameters as well

I suppose that as you said, in the channel factory it will be possible to delete all channels with 1 timeout in parallel in the future.
We can think about deleting each channel with timeout, where it is worth to implement this

I'm a bit worried that leaving the timeout as a NamedChannel property makes it obligatory to implement individual timeouts at some point. So I'd like to know if it's awkward or impossible at least, in case we don't want to do it. I can have a think but if you have any insight please share it.

We can now rename shutdownGracePeriod to shutdownGracePeriodPerRequest, thus ensuring backward compatibility and then we won't have to worry if (and I guess when) we want to do a parallel shutdown grace period

I don't understand what "per request" means. Isn't it "per channel" (in which case the fact that it is a property of a NamedChannel makes it redundant)?

Yes, I apologize, I mean “perChannel”.

So the current shutdownGracePeriod that I created we will rename directly now to shutdownGracePeriodPerChannel. This will solve any problems that may arise if we decide to create a parallel shutdownGracePeriod

OK, then the name doesn't need to change (because it's a property of a channel already). I don't think that solves any problems. Still thinking. If you have any ideas about how to implement the "per channel" behaviour then feel free to try them out.

I assume that by “per channel” we mean setting a specific shutdownGracePeriod separately for each channel? Is it possible to theriotically create some kind of settings class for channels to solve this issue

We could overload the createChannel method in GrpcChannelFactory and add another argument to the method with the settings class for the channel. Then directly in the DefaultGrpcChannelFactory we could monitor if the setting for that channel is present with the shutdownGracePeriod field, and if so, we use it for that channel in the destroy method. If not, we use the default one set in properties (or if not in properties - which we set systematically)

That is, my main point is to introduce a class for the settings for the channel to be set when creating the channel in createChannel

This seems kind of stale now. It might be better to close it and start again?

I guess it's worth updating the changes made in this PR and doing a rebase? Pending a decision though, how do we deal with this problem in the best way possible

@onobc has some changes planned to the channel factory. I think it might be best to regroup when they come through.

Hi @panic08 ,
I am closing this in favor of #95.

Thank you for that initial contribution. Things have changed quite a bit since the initial PR and we wanted to get this in for the 0.3.0 release tomorrow. I have marked you as co-author on the new PR.