----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26658/#review57005 -----------------------------------------------------------
Thanks for the patch. Looks good overall. Some minor comments below. clients/src/main/java/org/apache/kafka/common/message/KafkaLZ4BlockOutputStream.java <https://reviews.apache.org/r/26658/#comment97485> Should this be private? clients/src/main/java/org/apache/kafka/common/message/KafkaLZ4BlockOutputStream.java <https://reviews.apache.org/r/26658/#comment97505> Should we flush after writing the end mark? clients/src/main/java/org/apache/kafka/common/record/Compressor.java <https://reviews.apache.org/r/26658/#comment97444> Unused import. core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala <https://reviews.apache.org/r/26658/#comment97526> How about we make this 2000 so that we can test compression on more than 64KB, which is the default block size of lz4? core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala <https://reviews.apache.org/r/26658/#comment97527> How about also adding the following properties to test out compressing more than 64KB? props.put(ProducerConfig.BATCH_SIZE_CONFIG, "66000") props.put(ProducerConfig.LINGER_MS_CONFIG, "200") - Jun Rao On Oct. 16, 2014, 8:50 p.m., James Oliver wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26658/ > ----------------------------------------------------------- > > (Updated Oct. 16, 2014, 8:50 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1493 > https://issues.apache.org/jira/browse/KAFKA-1493 > > > Repository: kafka > > > Description > ------- > > KAFKA-1493 Implement LZ4 Frame I/O Streams > > > KAFKA-1493 Add utils functions, tweak test cases and OutputStream construction > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java > bf4ed66791b9a502aae6cb2ec7681f42732d9a43 > > clients/src/main/java/org/apache/kafka/common/message/KafkaLZ4BlockInputStream.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/message/KafkaLZ4BlockOutputStream.java > PRE-CREATION > clients/src/main/java/org/apache/kafka/common/record/CompressionType.java > 5227b2d7ab803389d1794f48c8232350c05b14fd > clients/src/main/java/org/apache/kafka/common/record/Compressor.java > 0323f5f7032dceb49d820c17a41b78c56591ffc4 > clients/src/main/java/org/apache/kafka/common/utils/Utils.java > a0827f576e8c38b1bd828cf0d6aefff9fd5ecc22 > config/producer.properties 39d65d7c6c21f4fccd7af89be6ca12a088d5dd98 > core/src/main/scala/kafka/message/CompressionCodec.scala > de0a0fade5387db63299c6b112b3c9a5e41d82ec > core/src/main/scala/kafka/message/CompressionFactory.scala > 8420e13d0d8680648df78f22ada4a0d4e3ab8758 > core/src/main/scala/kafka/tools/ConsoleProducer.scala > b024a693c23cb21f1efe405ed414bf23f3974f31 > core/src/main/scala/kafka/tools/PerfConfig.scala > c72002976d90416559090a665f6494072a6c2dec > core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala > c95485170fd8b4f5faad740f049e5d09aca8829d > core/src/test/scala/unit/kafka/message/MessageCompressionTest.scala > 6f0addcea64f1e78a4de50ec8135f4d02cebd305 > core/src/test/scala/unit/kafka/message/MessageTest.scala > 958c1a60069ad85ae20f5c58e74679cd9fa6f70e > > Diff: https://reviews.apache.org/r/26658/diff/ > > > Testing > ------- > > ./gradlew test > All tests passed > > > Thanks, > > James Oliver > >
