Review Request 17263: New producer for Kafka.

2014-01-23 Thread Jay Kreps
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17263/ --- Review request for kafka. Bugs: KAFKA-1227 https://issues.apache.org/jira/b

Re: Review Request 17263: New producer for Kafka.

2014-01-26 Thread Sriram Subramanian
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17263/#review32800 --- clients/src/main/java/kafka/clients/producer/Callback.java

Re: Review Request 17263: New producer for Kafka.

2014-01-26 Thread Jay Kreps
> On Jan. 26, 2014, 11:30 p.m., Sriram Subramanian wrote: > > clients/src/main/java/kafka/clients/producer/Callback.java, line 14 > > > > > > If the caller implements onCompletion to block for a while, the io > > threa

Re: Review Request 17263: New producer for Kafka.

2014-01-26 Thread Sriram Subramanian
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17263/#review32802 --- clients/src/main/java/kafka/common/PartitionInfo.java

Re: Review Request 17263: New producer for Kafka.

2014-01-26 Thread Sriram Subramanian
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17263/#review32806 --- clients/src/main/java/kafka/clients/producer/internals/BufferPool.j

Re: Review Request 17263: New producer for Kafka.

2014-01-26 Thread Sriram Subramanian
> On Jan. 26, 2014, 11:30 p.m., Sriram Subramanian wrote: > > clients/src/main/java/kafka/clients/producer/KafkaProducer.java, line 150 > > > > > > I think what you meant here when you said "the callback will execute i

Re: Review Request 17263: New producer for Kafka.

2014-01-26 Thread Sriram Subramanian
> On Jan. 26, 2014, 11:30 p.m., Sriram Subramanian wrote: > > clients/src/main/java/kafka/clients/producer/internals/BufferPool.java, > > line 80 > > > > > > What happens if freeUp actually got memory from the free lis

Re: Review Request 17263: New producer for Kafka.

2014-01-27 Thread Sriram Subramanian
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17263/#review32808 --- clients/src/main/java/kafka/common/protocol/types/Struct.java

Re: Review Request 17263: New producer for Kafka.

2014-01-27 Thread Sriram Subramanian
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17263/#review32863 --- clients/src/main/java/kafka/clients/producer/internals/BufferPool.j

Re: Review Request 17263: New producer for Kafka.

2014-01-27 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17263/#review32850 --- Made a first pass over some parts of the code. Will follow up with m

Re: Review Request 17263: New producer for Kafka.

2014-01-27 Thread Jay Kreps
> On Jan. 28, 2014, 2:07 a.m., Neha Narkhede wrote: > > clients/src/main/java/kafka/clients/producer/ProducerConfig.java, line 19 > > > > > > I think someone on the mailing list suggested this as well and I agree. > >

Re: Review Request 17263: New producer for Kafka.

2014-01-28 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17263/#review33033 --- Overall, this looks great. Here are a few comments - I have some mor

Re: Review Request 17263: New producer for Kafka.

2014-01-29 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17263/#review33004 --- Some more comments regarding the blocking behavior clients/src/mai

Re: Review Request 17263: New producer for Kafka.

2014-01-29 Thread Neha Narkhede
> On Jan. 26, 2014, 11:30 p.m., Sriram Subramanian wrote: > > clients/src/main/java/kafka/clients/producer/internals/RecordAccumulator.java, > > line 87 > > > > > > We can allocate a batch larger than the batchSize? >

Re: Review Request 17263: New producer for Kafka.

2014-01-30 Thread Jay Kreps
Yeah I will definitely do this. One todo is to work out the final config names and document them all. -Jay On Wed, Jan 29, 2014 at 11:29 AM, Neha Narkhede wrote: > > > > On Jan. 26, 2014, 11:30 p.m., Sriram Subramanian wrote: > > > > clients/src/main/java/kafka/clients/producer/internals/Record

Re: Review Request 17263: New producer for Kafka.

2014-01-30 Thread Jay Kreps
> On Jan. 28, 2014, 8:48 p.m., Joel Koshy wrote: > > Overall, this looks great. Here are a few comments - I have some more > > minor/stylistic comments (typos in comments and broken javadoc, code > > conventions, minor javadoc edits and such) which I would rather defer for a > > later more final

Re: Review Request 17263: New producer for Kafka.

2014-01-30 Thread Joel Koshy
> On Jan. 28, 2014, 8:48 p.m., Joel Koshy wrote: > > clients/src/main/java/kafka/clients/producer/KafkaProducer.java, line 182 > > > > > > This could add a couple minutes startup for producers that send to > > several

Re: Review Request 17263: New producer for Kafka.

2014-03-20 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17263/#review37833 --- One general comment: can we refactor the structure of metrics/senso

Re: Review Request 17263: New producer for Kafka.

2014-03-20 Thread Jay Kreps
> On March 20, 2014, 8:20 p.m., Guozhang Wang wrote: > > clients/src/main/java/kafka/common/metrics/stats/SampledStat.java, lines > > 29-40 > > > > > > This is not clear to me how the logic works for resetting old samp