InstantiatingGrpcChannelProvider requires ScheduledExecutorService for no good reason

[ChuntaoLu]

The InstantiatingGrpcChannelProvider.withExecutor method requires ScheduledExecutorService, and similarly the InstantiatingGrpcChannelProvider$Builder. setExecutorProvider requires a ExecutorProvider(which provides ScheduledExecutorService), but this requirement seems overly strict, because the executor is only used for ManagedChannelBuilder.executor, which takes Executor rather than ScheduledExecutorService.

This unnecessary restriction prevents users from providing a more performant executor (e.g. ForkJoinPool) than ScheduledThreadPoolExecutor when building a gRPC channel.

[hkdevandla]

@miraleung , can you please take a look? Thanks!

[igorbernstein2]

I think this is a subset of the work I started here (but never finished):
#869

Feel free to pull out the relevant parts from it to address this issue

[hkdevandla]

Thanks for the info. I'm wondering if this should be considered as a feature request (as it seems more like optimization). @miraleung , please let us know your thoughts.

CC'ed @vam-google

[miraleung]

FYI, BatcherFactory and ScheduledRetryingExecutor both take ScheduledExecutorService in their constructors, via Callables.java. Relying on the creation site and casting this isn't sufficient, a more involved refactoring is probably needed.

[igorbernstein2]

@miraleung I think that gax tangles 2 unrelated executors together. I think that the roles need to untangled. The transports need an executor for immediate actions, while retries & batching need to schedule non-latency sensitive work.

There are 2 places to set the executor: stub settings and the transport provider. StubSettings will propagate its executor to both places (retries & transport). This means that you can make the transport provider take a regular executor w/o breaking anything. In the general case people will still set a ScheduleExecutor on StubSettings which will be propagated to and shared by retries & transport. However users like @ChuntaoLu can set a more optimized executor directly on the transport

[miraleung]

Gotcha, thanks Igor.