-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31816/#review77328
-----------------------------------------------------------


Thanks for the patch. A few comments below.


core/src/main/scala/kafka/message/ByteBufferMessageSet.scala
<https://reviews.apache.org/r/31816/#comment125239>

    Can we just use wrapperMessage directly?



core/src/main/scala/kafka/message/ByteBufferMessageSet.scala
<https://reviews.apache.org/r/31816/#comment125242>

    This is inconsistent as in line 160 and 161. We should do the same check on 
size and throw the same type of exception.



core/src/main/scala/kafka/message/ByteBufferMessageSet.scala
<https://reviews.apache.org/r/31816/#comment125241>

    In the normal path, this should only happen when reading the offset. So we 
should probably handle the EOFException there. If we get EOFException in other 
places, it should be treated as an error.


- Jun Rao


On March 6, 2015, 11:48 p.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31816/
> -----------------------------------------------------------
> 
> (Updated March 6, 2015, 11:48 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-527
>     https://issues.apache.org/jira/browse/KAFKA-527
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Avoid double copying on decompress
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/consumer/ConsumerIterator.scala 
> ac491b4da2583ef7227c67f5b8bc0fd731d705c3 
>   core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 
> 788c7864bc881b935975ab4a4e877b690e65f1f1 
>   core/src/test/scala/unit/kafka/message/MessageCompressionTest.scala 
> 6f0addcea64f1e78a4de50ec8135f4d02cebd305 
>   core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala 
> 24deea06753e5358aa341c589ca7a7704317e29c 
> 
> Diff: https://reviews.apache.org/r/31816/diff/
> 
> 
> Testing
> -------
> 
> Unit tests
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>

Reply via email to