----------------------------------------------------------- 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. core/src/main/scala/kafka/message/MessageWriter.scala <https://reviews.apache.org/r/31742/#comment123993> Add a check that codec should not be NoCompression. core/src/main/scala/kafka/message/MessageWriter.scala <https://reviews.apache.org/r/31742/#comment123986> Could we use comments in /** * */ format? core/src/main/scala/kafka/message/MessageWriter.scala <https://reviews.apache.org/r/31742/#comment123989> We can just pass in the Byte here. core/src/main/scala/kafka/message/MessageWriter.scala <https://reviews.apache.org/r/31742/#comment123991> Better group the private functions together after the public functions. 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 MessageWriter contains a var of BufferingOutputStream; and instead of pass in the function logic of writing the message, we can just pass in messages and offsetCounter in the write() call which will then write the messages itself. - Guozhang Wang On March 4, 2015, 7:43 p.m., Yasuhiro Matsuda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31742/ > ----------------------------------------------------------- > > (Updated March 4, 2015, 7:43 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-527 > https://issues.apache.org/jira/browse/KAFKA-527 > > > Repository: kafka > > > Description > ------- > > less byte copies > > > Diffs > ----- > > core/src/main/scala/kafka/message/ByteBufferMessageSet.scala > 9c694719dc9b515fb3c3ae96435a87b334044272 > core/src/main/scala/kafka/message/MessageWriter.scala PRE-CREATION > core/src/test/scala/unit/kafka/message/MessageWriterTest.scala PRE-CREATION > > Diff: https://reviews.apache.org/r/31742/diff/ > > > Testing > ------- > > > Thanks, > > Yasuhiro Matsuda > >
