gRPC java: Provide ScheduledExecutorService setter in NettyChannelBuilder

[dan...@gmail.com]

Hello,

I have observed that the ScheduledExecutorService utilized for setting up deadlines and exiting idle mode is obtained by calling NettyClientTransport.getScheduledExecutorService, which returns an EventLoopGroup instance that was previously set in NettyChannelBuilder.eventLoopGroup().

Is there a specific rationale behind utilizing the same EventLoopGroup instance for executing IO tasks and as a ScheduledExecutorService?

I have identified two disadvantages associated with this approach:

1. Developers relinquish control over EventLoop to subchannel assignment, potentially leading to multiple subchannels sharing the same EventLoop.
2. The deadline task may end up in the same slow EventLoop, which subsequently requires its cancellation.

To tackle these issues, I propose the addition of a setter method for a custom ScheduledExecutorService within the NettyChannelBuilder. This enhancement would enable the separate scheduling of tasks while exclusively utilizing the EventLoopGroup for IO operations.

Best regards,
Dan

[sanjay...@google.com]

Comments inline below:

On Wednesday, May 31, 2023 at 6:24:56 PM UTC-7 dan...@gmail.com wrote:

Hello,

I have observed that the ScheduledExecutorService utilized for setting up deadlines and exiting idle mode is obtained by calling NettyClientTransport.getScheduledExecutorService,

I think you mean NettyTransportFactory.getScheduledExecutorService which is inside NettyChannelBuilder.

 

which returns an EventLoopGroup instance that was previously set in NettyChannelBuilder.eventLoopGroup(). 

Is there a specific rationale behind utilizing the same EventLoopGroup instance for executing IO tasks and as a ScheduledExecutorService?

NettyChannelBuilder also has offloadExecutor which lets a user to set a custom executor that will be used for (IO) operations that block or are expensive.

When it is not set the builder will use a static cached thread pool (which presumably is the same as the one set from eventLoopGroup or the system default). 

 

Do you think that addresses your concern?

[dan...@gmail.com]

Yes, I mean  NettyTransportFactory.getScheduledExecutorService.

offloadExecutor does not solve the problem with eventLoopGroup used as as ScheduledExecutorService.
More specifically - eventLoopGroup.schedule() is calling to eventLoopGroup.next().schedule() which is moving a current eventLoop index and making impossible to control 
1 to 1 eventLoop to netty transport assignment.

Thanks,

   Dan

[sanjay...@google.com]

One solution I can think of is for you to pass your own implementation of EventLoopGroup where eventLoopGroup.schedule() does not call eventLoopGroup.next() but submits it to a subset of EventLoop's that are specifically reserved for submitted/scheduled tasks.

NettyClientTransport calls eventLoopGroup.next()  when the transport is started so you get the 1 to 1 mapping between EventLoop and netty client transport.

This way you don't have to depend on any method/API from gRPC. Hope it works?

[dan...@gmail.com]

Thanks for the suggestion! That's exactly what I already implemented. However, since I still want to use `EpollEventLoopGroup` and this class is final, I ended up using dynamic proxy. The dynamic proxy redirects calls to `schedule*` to an additional `DefaultEventLoopGroup`, while the rest is handled by `EpollEventLoopGroup`. But I would prefer to use a simple setter added to `NettyChannelBuilder` instead.

[sanjay...@google.com]

> But I would prefer to use a simple setter added to `NettyChannelBuilder` instead.

If I understand correctly the change is non-trivial: the 2 eventLoopGroups will have to be used for their respective roles (with appropriate default behavior for backward compatibility etc).

I suggest opening a feature request at https://github.com/grpc/grpc-java/issues . And also consider creating a pull request.