avro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Shaun Williams <shaun_willi...@apple.com>
Subject Re: [jira] [Commented] (AVRO-1001) Adding thread pool to NettyServerAvroHandler
Date Mon, 23 Jan 2012 16:29:08 GMT
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
> 
> 


Mime
View raw message