Great, I'll give it a try. Thanks! -Shaun
On Jan 22, 2012, at 2:04 PM, James Baldassari (Commented) (JIRA) wrote: > > [ > https://issues.apache.org/jira/browse/AVRO-1001?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13190801#comment-13190801 > ] > > James Baldassari commented on AVRO-1001: > ---------------------------------------- > > Thanks for the patch. I was surprised to see this issue because when the > NettyServer is initialized it actually passes two different thread pools to > the NioServerSocketChannelFactory: > > {code} > public NettyServer(Responder responder, InetSocketAddress addr) { > this(responder, addr, new NioServerSocketChannelFactory > (Executors .newCachedThreadPool(), Executors.newCachedThreadPool())); > } > {code} > > The first thread pool is for the Netty "boss" threads, and the second is for > the Netty "worker" threads. When I dug into the Netty code I found that > Netty only starts a single pair of boss and worker threads for each Netty > channel. The worker thread sits in a for (;; ) loop selecting on the channel > and then processing each message it receives in order. So you're correct > that only one RPC can be executing concurrently on a per-channel basis. I > was also able to reproduce this issue in a unit test. > > Given how Netty works, one work-around is to create a new client for each > concurrent RPC that you want to execute. This is not an ideal long-term > solution, and I think your idea to add a new thread pool in NettyServer is a > good fix. However, the cached thread pool might not be the best > implementation for all use cases. For example, cached thread pools can be > problematic in high-throughput environments because they are unbounded by > definition. So let's allow the ExecutorService to be passed into the > NettyServer constructor if the user wants to specify a different/custom > implementation. As the default implementation, the cached thread pool should > be fine, as long as it can be changed if necessary. > > I'll add my unit test to your patch to verify that this change fixes the > issue. > >> Adding thread pool to NettyServerAvroHandler >> -------------------------------------------- >> >> Key: AVRO-1001 >> URL: https://issues.apache.org/jira/browse/AVRO-1001 >> Project: Avro >> Issue Type: Improvement >> Components: java >> Affects Versions: 1.6.1 >> Reporter: Shaun Williams >> Labels: patch >> Attachments: AVRO-1001.patch >> >> >> Request code review. >> The current NettyServer implementation processes each request/response >> sequentially on a single channel. Thus in the case where a second request >> is received from the same client while a request is still being processed, >> the behavior is undefined. >> This patch updates NettyServerAvroHandler.messageReceived() to process each >> request in a separate thread using an ExecutorService. > > -- > This message is automatically generated by JIRA. > If you think it was sent incorrectly, please contact your JIRA > administrators: > https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa > For more information on JIRA, see: http://www.atlassian.com/software/jira > >
