> On 2012-06-02 21:11:53, Hari Shreedharan wrote:
> > trunk/flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java,
> >  lines 215-216
> > <https://reviews.apache.org/r/5333/diff/1/?file=111427#file111427line215>
> >
> >     You should be setting the batchSize of the localClient also. If this is 
> > not set the batchSize is never passed to the actual client that does the 
> > Rpc writes.
> 
> Hari Shreedharan wrote:
>     The underlying Rpc client will send multiple batches of size=batchSize, 
> if the list has more events than batchSize. So the client need not check the 
> batchSize during append. It can pass list of arbitrary size, and the client 
> will send multiple batches till the list is empty.

Thanks Hari. I can remove the check from the app client code but if user 
specifies the custom batch-size then it always defaults to DEFAULT_BATCH_SIZE 
by NettyAvroRpcClient.

This patch will not fix if client app specifies the batch-size in Properties 
(using RpcClientFactory.getInstance(Properties))

We need to send the batch-size all the way to 
RpcClientFactory.getDefaultInstance(hostName, port, batchSize) from 
getNextClient

Today, batchSize is default to 0 which sets as DEFAULT_BATCH_SIZE (100) 

Will update the patch. Thanks.


- Mubarak


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5333/#review8300
-----------------------------------------------------------


On 2012-06-02 18:12:37, Mubarak Seyed wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5333/
> -----------------------------------------------------------
> 
> (Updated 2012-06-02 18:12:37)
> 
> 
> Review request for Flume.
> 
> 
> Summary
> -------
> 
> FailoverRpcClient uses batchSize as local variable and it breaks the contract 
> for getBatchSize(). Consumer of FailoverRpcClient checks for getBatchSize() 
> to send events using appendBatch(List<Event>), since batchSize is initialized 
> with null, clients gets NPE.
> 
> The fix is to remove the batchSize local variable in configureHosts() 
> 
> 
> This addresses bug FLUME-1226.
>     https://issues.apache.org/jira/browse/FLUME-1226
> 
> 
> Diffs
> -----
> 
>   
> trunk/flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java 
> 1342144 
> 
> Diff: https://reviews.apache.org/r/5333/diff
> 
> 
> Testing
> -------
> 
> Tested in lab environment.
> 
> 
> Thanks,
> 
> Mubarak
> 
>

Reply via email to