> On June 30, 2012, 11:06 p.m., Mubarak Seyed wrote: > > flume-ng-core/src/main/java/org/apache/flume/sink/AvroSink.java, line 99 > > <https://reviews.apache.org/r/5683/diff/1/?file=117624#file117624line99> > > > > I think we need to add connect-timeout and request-timeout javadoc > > here? values in milliseconds
Added these Javadocs > On June 30, 2012, 11:06 p.m., Mubarak Seyed wrote: > > flume-ng-core/src/main/java/org/apache/flume/sink/AvroSink.java, line 297 > > <https://reviews.apache.org/r/5683/diff/1/?file=117624#file117624line297> > > > > counter for failure here? will be used in metrics (for rollbacks and > > exceptions) Added a counter > On June 30, 2012, 11:06 p.m., Mubarak Seyed wrote: > > flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java, > > line 188 > > <https://reviews.apache.org/r/5683/diff/1/?file=117627#file117627line188> > > > > single space needed before "ms" This is a pretty nitpicky comment, but I added the space anyway for ease of using awk/perl to parse the logs ;) > On June 30, 2012, 11:06 p.m., Mubarak Seyed wrote: > > flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java, line > > 161 > > <https://reviews.apache.org/r/5683/diff/1/?file=117628#file117628line161> > > > > If batchSize is null, can't we set it to 1 or > > RpcClientConfigurationConstants.DEFAULT_BATCH_SIZE. I think hostName > > and port are mandatory for client connection. This is the 3-arg version of the factory method. If developers don't want to specify batch size, there is the 2-arg factory method as well as the Properties-driven factory method available for them to use. - Mike ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5683/#review8773 ----------------------------------------------------------- On July 3, 2012, 7:35 p.m., Mike Percy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5683/ > ----------------------------------------------------------- > > (Updated July 3, 2012, 7:35 p.m.) > > > Review request for Flume. > > > Description > ------- > > Patch to add support for configurable connect and request timeout to Avro > Sink. Also refactors some of the RpcClient libs to reduce the # of codepaths. > > > This addresses bug FLUME-1316. > https://issues.apache.org/jira/browse/FLUME-1316 > > > Diffs > ----- > > flume-ng-core/src/main/java/org/apache/flume/sink/AvroSink.java 5c6d0e3 > flume-ng-core/src/test/java/org/apache/flume/sink/TestAvroSink.java 3765924 > flume-ng-doc/sphinx/FlumeUserGuide.rst 94f951f > flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java > 606a4bd > > flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientConfigurationConstants.java > e304689 > flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java > e19b093 > flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java e4f23a6 > flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java > 77bf331 > > Diff: https://reviews.apache.org/r/5683/diff/ > > > Testing > ------- > > Added a unit test for the new params. Existing unit tests pass. > > > Thanks, > > Mike Percy > >
