ss77892 commented on code in PR #1538:
URL: https://github.com/apache/ratis/pull/1538#discussion_r3700675416
##########
ratis-netty/src/main/java/org/apache/ratis/netty/server/NettyRpcService.java:
##########
@@ -116,6 +136,11 @@ protected void initChannel(SocketChannel ch) {
}
};
+ this.requestExecutor = ConcurrentUtils.newThreadPoolWithMax(
Review Comment:
Backpressure control should be a part of these changes. You have removed the
one that was going through TCP flow control and don't provide any replacement.
Another thing: we have ThreadPoolExecutor with corePoolSize=0 and unbounded
queue. Let me quote "Java Concurrency in Practice":
**[3] Developers are sometimes tempted to set the core size to zero so that
the worker threads will eventually be torn down and therefore won't prevent the
JVM from exiting, but this can cause some strange‐seeming behavior in thread
pools that don't use a SynchronousQueue for their work queue (as
newCachedThreadPool does). If the pool is already at the core size,
ThreadPoolExecutor creates a new thread only if the work queue is full. So
tasks submitted to a thread pool with a work queue that has any capacity and a
core size of zero will not execute until the queue fills up, which is usually
not what is desired.**
So, it's still a single thread.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
