Re: Review Request 26346: Patch for KAFKA-1670

2014-10-07 Thread Sriharsha Chintalapani
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26346/ --- (Updated Oct. 8, 2014, 1:39 a.m.) Review request for kafka. Bugs: KAFKA-1670

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-07 Thread Jun Rao
> On Oct. 7, 2014, 11:03 p.m., Jun Rao wrote: > > core/src/test/scala/unit/kafka/log/LogTest.scala, line 242 > > > > > > By increasing the segment size to 100, does the log still roll on every > > message as indicated

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-07 Thread Sriharsha Chintalapani
> On Oct. 7, 2014, 11:03 p.m., Jun Rao wrote: > > core/src/test/scala/unit/kafka/log/LogTest.scala, line 242 > > > > > > By increasing the segment size to 100, does the log still roll on every > > message as indicated

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-07 Thread Jun Rao
> On Oct. 7, 2014, 11:03 p.m., Jun Rao wrote: > > core/src/test/scala/unit/kafka/log/LogTest.scala, line 242 > > > > > > By increasing the segment size to 100, does the log still roll on every > > message as indicated

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-07 Thread Sriharsha Chintalapani
> On Oct. 7, 2014, 11:03 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/common/MessageSetSizeTooLargeException.scala, > > line 20 > > > > > > We need to add this exception to ErrorMapping and Errors. We also need

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-07 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26346/#review55720 --- core/src/main/scala/kafka/common/MessageSetSizeTooLargeException.sc

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-07 Thread Sriharsha Chintalapani
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26346/ --- (Updated Oct. 7, 2014, 8:49 p.m.) Review request for kafka. Bugs: KAFKA-1670

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-07 Thread Sriharsha Chintalapani
> On Oct. 7, 2014, 12:42 a.m., Jay Kreps wrote: > > core/src/main/scala/kafka/log/Log.scala, line 502 > > > > > > It is a bit subtle that you are checking for overflow this way. What we > > mean to check is just that

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-07 Thread Sriharsha Chintalapani
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26346/ --- (Updated Oct. 7, 2014, 8:39 p.m.) Review request for kafka. Bugs: KAFKA-1670

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-06 Thread Jay Kreps
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26346/#review55619 --- core/src/main/scala/kafka/log/Log.scala

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-06 Thread Jun Rao
> On Oct. 6, 2014, 5:24 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/log/Log.scala, line 502 > > > > > > We probably should enforce that a segment is never larger than > > config.segmentSize. So, if messageSize

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-06 Thread Sriharsha Chintalapani
> On Oct. 6, 2014, 5:24 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/log/Log.scala, line 502 > > > > > > We probably should enforce that a segment is never larger than > > config.segmentSize. So, if messageSize

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-06 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26346/#review55526 --- core/src/main/scala/kafka/log/Log.scala

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-06 Thread Neha Narkhede
> On Oct. 5, 2014, 11:35 p.m., Jun Rao wrote: > > core/src/test/scala/unit/kafka/log/LogTest.scala, lines 113-129 > > > > > > Appending 2GB of data in a unit test is probably too long. We can > > probably just manuall

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-06 Thread Sriharsha Chintalapani
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26346/ --- (Updated Oct. 6, 2014, 4:48 p.m.) Review request for kafka. Bugs: KAFKA-1670

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-06 Thread Sriharsha Chintalapani
> On Oct. 5, 2014, 11:35 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/log/Log.scala, line 499 > > > > > > Would it be simpler to just do the following? > > > > if (segment.size + messageSize < (long) con

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-05 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26346/#review55479 --- Thanks for the patch. Some comments below. core/src/main/scala/kaf

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-04 Thread Sriharsha Chintalapani
> On Oct. 4, 2014, 11:28 p.m., Neha Narkhede wrote: > > Can you try to add a unit test for this? I added unit test under LogTest. This test very basic its just adds messages to file until the file reaches Int.Max. This will result in couple of issues first being atleast having 2G+ tmp space a

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-04 Thread Sriharsha Chintalapani
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26346/ --- (Updated Oct. 5, 2014, 3:17 a.m.) Review request for kafka. Bugs: KAFKA-1670

Re: Review Request 26346: Patch for KAFKA-1670

2014-10-04 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26346/#review55455 --- Can you try to add a unit test for this? core/src/main/scala/kafka

Review Request 26346: Patch for KAFKA-1670

2014-10-04 Thread Sriharsha Chintalapani
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26346/ --- Review request for kafka. Bugs: KAFKA-1670 https://issues.apache.org/jira/b