Re: Review Request 31742: Patch for KAFKA-527

2015-03-25 Thread Yasuhiro Matsuda
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31742/ --- (Updated March 25, 2015, 7:08 p.m.) Review request for kafka. Bugs: KAFKA-527

Re: Review Request 31742: Patch for KAFKA-527

2015-03-19 Thread Yasuhiro Matsuda
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31742/ --- (Updated March 20, 2015, 4:33 a.m.) Review request for kafka. Bugs: KAFKA-527

Re: Review Request 31742: Patch for KAFKA-527

2015-03-19 Thread Yasuhiro Matsuda
> On March 19, 2015, 3:58 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/message/MessageWriter.scala, line 144 > > > > > > Shouldn't we test off < b.length and off + len < b.length? This is actually fine. OutputSt

Re: Review Request 31742: Patch for KAFKA-527

2015-03-19 Thread Yasuhiro Matsuda
> On March 19, 2015, 3:58 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/message/ByteBufferMessageSet.scala, lines 43-55 > > > > > > Instead of passing in a function, I think it will be easier to > > understand the

Re: Review Request 31742: Patch for KAFKA-527

2015-03-19 Thread Yasuhiro Matsuda
> On March 19, 2015, 3:58 a.m., Jun Rao wrote: > > core/src/test/scala/unit/kafka/message/MessageWriterTest.scala, lines 31-33 > > > > > > Could we just use Random.nextBytes()? My intention was to have some control of

Re: Review Request 31742: Patch for KAFKA-527

2015-03-18 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31742/#review76985 --- Thanks for the patch. Could we test out the patch with StressTestLo

Re: Review Request 31742: Patch for KAFKA-527

2015-03-16 Thread Yasuhiro Matsuda
On March 13, 2015, 11:43 p.m., Yasuhiro Matsuda wrote: > > The inheritance of MessageWriter from BufferingOutputStream is a bit > > confusing, since it will always use itself in the writePayload function > > parameter. > > > > I feel it is more clear to read the code if we just let MessageWri

Re: Review Request 31742: Patch for KAFKA-527

2015-03-16 Thread Yasuhiro Matsuda
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31742/ --- (Updated March 16, 2015, 10:19 p.m.) Review request for kafka. Bugs: KAFKA-52

Re: Review Request 31742: Patch for KAFKA-527

2015-03-16 Thread Guozhang Wang
> On March 13, 2015, 11:43 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/message/MessageWriter.scala, line 29 > > > > > > Add a check that codec should not be NoCompression. > > Yasuhiro Matsuda wrote: >

Re: Review Request 31742: Patch for KAFKA-527

2015-03-16 Thread Yasuhiro Matsuda
> On March 13, 2015, 11:43 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/message/MessageWriter.scala, line 29 > > > > > > Add a check that codec should not be NoCompression. Why the codec should not be NoCom

Re: Review Request 31742: Patch for KAFKA-527

2015-03-13 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31742/#review76454 --- LGTM, with a few minor comments and a coding suggestion in the end.

Review Request 31742: Patch for KAFKA-527

2015-03-04 Thread Yasuhiro Matsuda
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31742/ --- Review request for kafka. Bugs: KAFKA-527 https://issues.apache.org/jira/br