You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by James Oliver <jd...@gmail.com> on 2014/05/20 08:13:08 UTC
Re: Review Request 21663: Patch for KAFKA-1432
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21663/
-----------------------------------------------------------
(Updated May 20, 2014, 6:13 a.m.)
Review request for kafka.
Summary (updated)
-----------------
Patch for KAFKA-1432
Bugs: KAFKA-1432 and KAFKA-1456
https://issues.apache.org/jira/browse/KAFKA-1432
https://issues.apache.org/jira/browse/KAFKA-1456
Repository: kafka
Description (updated)
-------
Fix LZ4HC case object typo
Revert unnecessary config changes, change server log folder to existing directory
Add test cases for LZ4 and LZ4HC message compression
Add LZ4 and LZ4HC codec wrappers to client
Diffs (updated)
-----
build.gradle b3bbd775366e29abb960e11b0b613b3a85ac4747
clients/src/main/java/org/apache/kafka/common/record/CompressionType.java c557e44b68bc201c7c7e8a31e9eb2f74c359762b
clients/src/main/java/org/apache/kafka/common/record/Compressor.java 6ae3d06ac6d27e4ecee3f5b4a18e2615d0581442
clients/src/main/java/org/apache/kafka/common/record/Record.java ce1177ef3c851572a3e3268a7a075e7db56fc3f7
config/producer.properties 52a76114f5d0968ca0ff01b3ab1a45450c0a1e33
core/src/main/scala/kafka/message/CompressionCodec.scala 8762a790af2ab7f60a5e4e19955c7eb7ad7bc749
core/src/main/scala/kafka/message/CompressionFactory.scala ca833ee317975548b3e0e91f010af36bce2af09b
core/src/main/scala/kafka/message/Message.scala 52c082f5213ba48a39bf0d17e4ced2714276f0e0
core/src/test/scala/unit/kafka/message/MessageCompressionTest.scala ed22931f24fed3530d374b56495a5e4bc89c72f2
system_test/producer_perf/bin/run-compression-test-all.sh PRE-CREATION
system_test/producer_perf/config/server.properties 9f8a633ab2f92862e7e7bcc40d699daf00c69e23
Diff: https://reviews.apache.org/r/21663/diff/
Testing
-------
Thanks,
James Oliver
Re: Review Request 21663: Patch for KAFKA-1456
Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21663/#review43783
-----------------------------------------------------------
Can we add unit tests for the new compression codecs? We heavily depend on unit tests to catch obvious issues on every checkin.
- Neha Narkhede
On May 20, 2014, 6:24 a.m., James Oliver wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21663/
> -----------------------------------------------------------
>
> (Updated May 20, 2014, 6:24 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1432 and KAFKA-1456
> https://issues.apache.org/jira/browse/KAFKA-1432
> https://issues.apache.org/jira/browse/KAFKA-1456
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Fix LZ4HC case object typo
>
>
> Revert unnecessary config changes, change server log folder to existing directory
>
>
> Add test cases for LZ4 and LZ4HC message compression
>
>
> Add LZ4 and LZ4HC codec wrappers to client
>
>
> Diffs
> -----
>
> build.gradle b3bbd775366e29abb960e11b0b613b3a85ac4747
> clients/src/main/java/org/apache/kafka/common/record/CompressionType.java c557e44b68bc201c7c7e8a31e9eb2f74c359762b
> clients/src/main/java/org/apache/kafka/common/record/Compressor.java 6ae3d06ac6d27e4ecee3f5b4a18e2615d0581442
> clients/src/main/java/org/apache/kafka/common/record/Record.java ce1177ef3c851572a3e3268a7a075e7db56fc3f7
> config/producer.properties 52a76114f5d0968ca0ff01b3ab1a45450c0a1e33
> core/src/main/scala/kafka/message/CompressionCodec.scala 8762a790af2ab7f60a5e4e19955c7eb7ad7bc749
> core/src/main/scala/kafka/message/CompressionFactory.scala ca833ee317975548b3e0e91f010af36bce2af09b
> core/src/main/scala/kafka/message/Message.scala 52c082f5213ba48a39bf0d17e4ced2714276f0e0
> core/src/test/scala/unit/kafka/message/MessageCompressionTest.scala ed22931f24fed3530d374b56495a5e4bc89c72f2
> system_test/producer_perf/bin/run-compression-test-all.sh PRE-CREATION
> system_test/producer_perf/config/server.properties 9f8a633ab2f92862e7e7bcc40d699daf00c69e23
>
> Diff: https://reviews.apache.org/r/21663/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> James Oliver
>
>
Re: Review Request 21663: Patch for KAFKA-1456
Posted by Joe Stein <cr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21663/#review43694
-----------------------------------------------------------
Reviewd code, applied patched and ran test cases. Looks good so far. I want to-do some more testing with clients tomorrow and Friday before committing.
- Joe Stein
On May 20, 2014, 6:24 a.m., James Oliver wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21663/
> -----------------------------------------------------------
>
> (Updated May 20, 2014, 6:24 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1432 and KAFKA-1456
> https://issues.apache.org/jira/browse/KAFKA-1432
> https://issues.apache.org/jira/browse/KAFKA-1456
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Fix LZ4HC case object typo
>
>
> Revert unnecessary config changes, change server log folder to existing directory
>
>
> Add test cases for LZ4 and LZ4HC message compression
>
>
> Add LZ4 and LZ4HC codec wrappers to client
>
>
> Diffs
> -----
>
> build.gradle b3bbd775366e29abb960e11b0b613b3a85ac4747
> clients/src/main/java/org/apache/kafka/common/record/CompressionType.java c557e44b68bc201c7c7e8a31e9eb2f74c359762b
> clients/src/main/java/org/apache/kafka/common/record/Compressor.java 6ae3d06ac6d27e4ecee3f5b4a18e2615d0581442
> clients/src/main/java/org/apache/kafka/common/record/Record.java ce1177ef3c851572a3e3268a7a075e7db56fc3f7
> config/producer.properties 52a76114f5d0968ca0ff01b3ab1a45450c0a1e33
> core/src/main/scala/kafka/message/CompressionCodec.scala 8762a790af2ab7f60a5e4e19955c7eb7ad7bc749
> core/src/main/scala/kafka/message/CompressionFactory.scala ca833ee317975548b3e0e91f010af36bce2af09b
> core/src/main/scala/kafka/message/Message.scala 52c082f5213ba48a39bf0d17e4ced2714276f0e0
> core/src/test/scala/unit/kafka/message/MessageCompressionTest.scala ed22931f24fed3530d374b56495a5e4bc89c72f2
> system_test/producer_perf/bin/run-compression-test-all.sh PRE-CREATION
> system_test/producer_perf/config/server.properties 9f8a633ab2f92862e7e7bcc40d699daf00c69e23
>
> Diff: https://reviews.apache.org/r/21663/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> James Oliver
>
>
Re: Review Request 21663: Patch for KAFKA-1456
Posted by Joe Stein <cr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21663/#review43937
-----------------------------------------------------------
Ship it!
Ship It!
- Joe Stein
On May 20, 2014, 6:24 a.m., James Oliver wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21663/
> -----------------------------------------------------------
>
> (Updated May 20, 2014, 6:24 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1432 and KAFKA-1456
> https://issues.apache.org/jira/browse/KAFKA-1432
> https://issues.apache.org/jira/browse/KAFKA-1456
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Fix LZ4HC case object typo
>
>
> Revert unnecessary config changes, change server log folder to existing directory
>
>
> Add test cases for LZ4 and LZ4HC message compression
>
>
> Add LZ4 and LZ4HC codec wrappers to client
>
>
> Diffs
> -----
>
> build.gradle b3bbd775366e29abb960e11b0b613b3a85ac4747
> clients/src/main/java/org/apache/kafka/common/record/CompressionType.java c557e44b68bc201c7c7e8a31e9eb2f74c359762b
> clients/src/main/java/org/apache/kafka/common/record/Compressor.java 6ae3d06ac6d27e4ecee3f5b4a18e2615d0581442
> clients/src/main/java/org/apache/kafka/common/record/Record.java ce1177ef3c851572a3e3268a7a075e7db56fc3f7
> config/producer.properties 52a76114f5d0968ca0ff01b3ab1a45450c0a1e33
> core/src/main/scala/kafka/message/CompressionCodec.scala 8762a790af2ab7f60a5e4e19955c7eb7ad7bc749
> core/src/main/scala/kafka/message/CompressionFactory.scala ca833ee317975548b3e0e91f010af36bce2af09b
> core/src/main/scala/kafka/message/Message.scala 52c082f5213ba48a39bf0d17e4ced2714276f0e0
> core/src/test/scala/unit/kafka/message/MessageCompressionTest.scala ed22931f24fed3530d374b56495a5e4bc89c72f2
> system_test/producer_perf/bin/run-compression-test-all.sh PRE-CREATION
> system_test/producer_perf/config/server.properties 9f8a633ab2f92862e7e7bcc40d699daf00c69e23
>
> Diff: https://reviews.apache.org/r/21663/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> James Oliver
>
>
Re: Review Request 21663: Patch for KAFKA-1456
Posted by James Oliver <jd...@gmail.com>.
Yeah I meant to get to that Friday...
Created subtask KAFKA-1471 to address this
https://reviews.apache.org/r/21938/
On Mon, May 26, 2014 at 8:40 AM, Joe Stein <cr...@gmail.com> wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21663/
>
> On May 23rd, 2014, 5:28 p.m. UTC, *Jun Rao* wrote:
>
> Thanks for the patch. Could we add the new codec in ProducerCompressionTest and MessageTest too?
>
> Sorry I missed this comment about ProducerCompressionTest the code is in MessageTest MessageCompressionTest and yes should be in ProducerCompressionTest too
>
> Jimi can you add code for the ProducerCompressionTest please? just do a pull on trunk and a new patch on a new ticket for that I think would be ok
>
>
> - Joe
>
> On May 20th, 2014, 6:24 a.m. UTC, James Oliver wrote:
> Review request for kafka.
> By James Oliver.
>
> *Updated May 20, 2014, 6:24 a.m.*
> *Bugs: * KAFKA-1432 <https://issues.apache.org/jira/browse/KAFKA-1432>,
> KAFKA-1456 <https://issues.apache.org/jira/browse/KAFKA-1456>
> *Repository: * kafka
> Description
>
> Fix LZ4HC case object typo
>
>
> Revert unnecessary config changes, change server log folder to existing directory
>
>
> Add test cases for LZ4 and LZ4HC message compression
>
>
> Add LZ4 and LZ4HC codec wrappers to client
>
> Diffs
>
> - build.gradle (b3bbd775366e29abb960e11b0b613b3a85ac4747)
> - clients/src/main/java/org/apache/kafka/common/record/CompressionType.java
> (c557e44b68bc201c7c7e8a31e9eb2f74c359762b)
> - clients/src/main/java/org/apache/kafka/common/record/Compressor.java
> (6ae3d06ac6d27e4ecee3f5b4a18e2615d0581442)
> - clients/src/main/java/org/apache/kafka/common/record/Record.java
> (ce1177ef3c851572a3e3268a7a075e7db56fc3f7)
> - config/producer.properties (52a76114f5d0968ca0ff01b3ab1a45450c0a1e33)
> - core/src/main/scala/kafka/message/CompressionCodec.scala
> (8762a790af2ab7f60a5e4e19955c7eb7ad7bc749)
> - core/src/main/scala/kafka/message/CompressionFactory.scala
> (ca833ee317975548b3e0e91f010af36bce2af09b)
> - core/src/main/scala/kafka/message/Message.scala
> (52c082f5213ba48a39bf0d17e4ced2714276f0e0)
> - core/src/test/scala/unit/kafka/message/MessageCompressionTest.scala
> (ed22931f24fed3530d374b56495a5e4bc89c72f2)
> - system_test/producer_perf/bin/run-compression-test-all.sh
> (PRE-CREATION)
> - system_test/producer_perf/config/server.properties
> (9f8a633ab2f92862e7e7bcc40d699daf00c69e23)
>
> View Diff <https://reviews.apache.org/r/21663/diff/>
>
Re: Review Request 21663: Patch for KAFKA-1456
Posted by Joe Stein <cr...@gmail.com>.
> On May 23, 2014, 5:28 p.m., Jun Rao wrote:
> > Thanks for the patch. Could we add the new codec in ProducerCompressionTest and MessageTest too?
Sorry I missed this comment about ProducerCompressionTest the code is in MessageTest MessageCompressionTest and yes should be in ProducerCompressionTest too
Jimi can you add code for the ProducerCompressionTest please? just do a pull on trunk and a new patch on a new ticket for that I think would be ok
- Joe
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21663/#review43842
-----------------------------------------------------------
On May 20, 2014, 6:24 a.m., James Oliver wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21663/
> -----------------------------------------------------------
>
> (Updated May 20, 2014, 6:24 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1432 and KAFKA-1456
> https://issues.apache.org/jira/browse/KAFKA-1432
> https://issues.apache.org/jira/browse/KAFKA-1456
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Fix LZ4HC case object typo
>
>
> Revert unnecessary config changes, change server log folder to existing directory
>
>
> Add test cases for LZ4 and LZ4HC message compression
>
>
> Add LZ4 and LZ4HC codec wrappers to client
>
>
> Diffs
> -----
>
> build.gradle b3bbd775366e29abb960e11b0b613b3a85ac4747
> clients/src/main/java/org/apache/kafka/common/record/CompressionType.java c557e44b68bc201c7c7e8a31e9eb2f74c359762b
> clients/src/main/java/org/apache/kafka/common/record/Compressor.java 6ae3d06ac6d27e4ecee3f5b4a18e2615d0581442
> clients/src/main/java/org/apache/kafka/common/record/Record.java ce1177ef3c851572a3e3268a7a075e7db56fc3f7
> config/producer.properties 52a76114f5d0968ca0ff01b3ab1a45450c0a1e33
> core/src/main/scala/kafka/message/CompressionCodec.scala 8762a790af2ab7f60a5e4e19955c7eb7ad7bc749
> core/src/main/scala/kafka/message/CompressionFactory.scala ca833ee317975548b3e0e91f010af36bce2af09b
> core/src/main/scala/kafka/message/Message.scala 52c082f5213ba48a39bf0d17e4ced2714276f0e0
> core/src/test/scala/unit/kafka/message/MessageCompressionTest.scala ed22931f24fed3530d374b56495a5e4bc89c72f2
> system_test/producer_perf/bin/run-compression-test-all.sh PRE-CREATION
> system_test/producer_perf/config/server.properties 9f8a633ab2f92862e7e7bcc40d699daf00c69e23
>
> Diff: https://reviews.apache.org/r/21663/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> James Oliver
>
>
Re: Review Request 21663: Patch for KAFKA-1456
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21663/#review43842
-----------------------------------------------------------
Thanks for the patch. Could we add the new codec in ProducerCompressionTest and MessageTest too?
- Jun Rao
On May 20, 2014, 6:24 a.m., James Oliver wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21663/
> -----------------------------------------------------------
>
> (Updated May 20, 2014, 6:24 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1432 and KAFKA-1456
> https://issues.apache.org/jira/browse/KAFKA-1432
> https://issues.apache.org/jira/browse/KAFKA-1456
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Fix LZ4HC case object typo
>
>
> Revert unnecessary config changes, change server log folder to existing directory
>
>
> Add test cases for LZ4 and LZ4HC message compression
>
>
> Add LZ4 and LZ4HC codec wrappers to client
>
>
> Diffs
> -----
>
> build.gradle b3bbd775366e29abb960e11b0b613b3a85ac4747
> clients/src/main/java/org/apache/kafka/common/record/CompressionType.java c557e44b68bc201c7c7e8a31e9eb2f74c359762b
> clients/src/main/java/org/apache/kafka/common/record/Compressor.java 6ae3d06ac6d27e4ecee3f5b4a18e2615d0581442
> clients/src/main/java/org/apache/kafka/common/record/Record.java ce1177ef3c851572a3e3268a7a075e7db56fc3f7
> config/producer.properties 52a76114f5d0968ca0ff01b3ab1a45450c0a1e33
> core/src/main/scala/kafka/message/CompressionCodec.scala 8762a790af2ab7f60a5e4e19955c7eb7ad7bc749
> core/src/main/scala/kafka/message/CompressionFactory.scala ca833ee317975548b3e0e91f010af36bce2af09b
> core/src/main/scala/kafka/message/Message.scala 52c082f5213ba48a39bf0d17e4ced2714276f0e0
> core/src/test/scala/unit/kafka/message/MessageCompressionTest.scala ed22931f24fed3530d374b56495a5e4bc89c72f2
> system_test/producer_perf/bin/run-compression-test-all.sh PRE-CREATION
> system_test/producer_perf/config/server.properties 9f8a633ab2f92862e7e7bcc40d699daf00c69e23
>
> Diff: https://reviews.apache.org/r/21663/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> James Oliver
>
>
Re: Review Request 21663: Patch for KAFKA-1456
Posted by James Oliver <jd...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21663/
-----------------------------------------------------------
(Updated May 20, 2014, 6:24 a.m.)
Review request for kafka.
Summary (updated)
-----------------
Patch for KAFKA-1456
Bugs: KAFKA-1432 and KAFKA-1456
https://issues.apache.org/jira/browse/KAFKA-1432
https://issues.apache.org/jira/browse/KAFKA-1456
Repository: kafka
Description
-------
Fix LZ4HC case object typo
Revert unnecessary config changes, change server log folder to existing directory
Add test cases for LZ4 and LZ4HC message compression
Add LZ4 and LZ4HC codec wrappers to client
Diffs (updated)
-----
build.gradle b3bbd775366e29abb960e11b0b613b3a85ac4747
clients/src/main/java/org/apache/kafka/common/record/CompressionType.java c557e44b68bc201c7c7e8a31e9eb2f74c359762b
clients/src/main/java/org/apache/kafka/common/record/Compressor.java 6ae3d06ac6d27e4ecee3f5b4a18e2615d0581442
clients/src/main/java/org/apache/kafka/common/record/Record.java ce1177ef3c851572a3e3268a7a075e7db56fc3f7
config/producer.properties 52a76114f5d0968ca0ff01b3ab1a45450c0a1e33
core/src/main/scala/kafka/message/CompressionCodec.scala 8762a790af2ab7f60a5e4e19955c7eb7ad7bc749
core/src/main/scala/kafka/message/CompressionFactory.scala ca833ee317975548b3e0e91f010af36bce2af09b
core/src/main/scala/kafka/message/Message.scala 52c082f5213ba48a39bf0d17e4ced2714276f0e0
core/src/test/scala/unit/kafka/message/MessageCompressionTest.scala ed22931f24fed3530d374b56495a5e4bc89c72f2
system_test/producer_perf/bin/run-compression-test-all.sh PRE-CREATION
system_test/producer_perf/config/server.properties 9f8a633ab2f92862e7e7bcc40d699daf00c69e23
Diff: https://reviews.apache.org/r/21663/diff/
Testing
-------
Thanks,
James Oliver