> On March 7, 2014, 5:58 p.m., Jun Rao wrote:
> > Does the unit test pass? It seems to hang on ProducerFailureHandlingTest.

It does pass on my machine. Is the handing issue persistent or it is transient?


> On March 7, 2014, 5:58 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java,
> >  lines 158-159
> > <https://reviews.apache.org/r/18299/diff/6/?file=513284#file513284line158>
> >
> >     Could this be part of the constructor of MemoryRecords?

I use init() here for lazy construction of the stream since it may be only used 
for read (on the consumer side).


> On March 7, 2014, 5:58 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/record/Compressor.java, lines 
> > 98-99
> > <https://reviews.apache.org/r/18299/diff/6/?file=513289#file513289line98>
> >
> >     BufferOverflowException is not an IOException. It's a RuntimeException. 
> > Is that handled properly?

In my tests I observed that when writes out of the boundary IOException can be 
thrown, but I did not observe BufferOverflowException thrown.


> On March 7, 2014, 5:58 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java, 
> > lines 73-75
> > <https://reviews.apache.org/r/18299/diff/6/?file=513290#file513290line73>
> >
> >     Throwing exception is a bit expensive. We probably should avoid writing 
> > to the compressor after the first failure.

If the first part of && is false, the second part will not be executed I 
thought?


- Guozhang


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


On March 7, 2014, 1:49 a.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18299/
> -----------------------------------------------------------
> 
> (Updated March 7, 2014, 1:49 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1253
>     https://issues.apache.org/jira/browse/KAFKA-1253
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Refactored Compression logic into Compressor
> 
> GZIP/Snappy Integration Tests
> 
> 
> Diffs
> -----
> 
>   build.gradle 84fa0d6b5f7405af755c5d7ff7bdd7592bb8668f 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 
> 1ac69436f117800815b8d50f042e9e2a29364b43 
>   clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java 
> f8748960fe9a02d52f7caae675abec503df91a39 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java
>  616e1006f21c54af8260e84a96928cb8893ceb7c 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java
>  038a05a94b795ec0a95b2d40a89222394b5a74c4 
>   
> clients/src/main/java/org/apache/kafka/common/record/ByteBufferInputStream.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/record/ByteBufferOutputStream.java
>  PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/record/CompressionType.java 
> 906da02d02c03aadd8ab73ed2fc9a1898acb8d72 
>   clients/src/main/java/org/apache/kafka/common/record/Compressor.java 
> PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 
> 9d8935fa3beeb2a78b109a41ed76fd4374239560 
>   clients/src/main/java/org/apache/kafka/common/record/Record.java 
> f1dc9778502cbdfe982254fb6e25947842622239 
>   clients/src/main/java/org/apache/kafka/common/utils/Utils.java 
> 0c6b3656375721a718fb4de10118170aacce0ea9 
>   clients/src/test/java/org/apache/kafka/common/record/MemoryRecordsTest.java 
> b0745b528cef929c4273f7e2ac4de1476cfc25ad 
>   clients/src/test/java/org/apache/kafka/common/record/RecordTest.java 
> ae54d67da9907b0a043180c7395a1370b3d0528d 
>   clients/src/test/java/org/apache/kafka/common/utils/CrcTest.java 
> PRE-CREATION 
>   clients/src/test/java/org/apache/kafka/test/TestUtils.java 
> 36cfc0fda742eb501af2c2c0330e3f461cf1f40c 
>   core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 
> c002f5ea38ece66ad559fadb18ffaf40ac2026aa 
>   core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 
> 66ea76b9b6c0f8839f715c845fb9b9671b8f35c1 
> 
> Diff: https://reviews.apache.org/r/18299/diff/
> 
> 
> Testing
> -------
> 
> integration tests
> 
> snappy dynamic load test
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>

Reply via email to