You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Manikumar Reddy O <ma...@gmail.com> on 2014/12/16 18:11:01 UTC
Re: Review Request 24704: Patch for KAFKA-1499
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24704/
-----------------------------------------------------------
(Updated Dec. 16, 2014, 5:10 p.m.)
Review request for kafka.
Bugs: KAFKA-1499
https://issues.apache.org/jira/browse/KAFKA-1499
Repository: kafka
Description (updated)
-------
Support given for Broker-side compression
Diffs (updated)
-----
core/src/main/scala/kafka/log/Log.scala 4fae2f0d339b256832baa62ca4995d10546716b4
core/src/main/scala/kafka/log/LogConfig.scala ca7a99e99f641b2694848b88bf4ae94657071d03
core/src/main/scala/kafka/message/BrokerCompression.scala PRE-CREATION
core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 788c7864bc881b935975ab4a4e877b690e65f1f1
core/src/main/scala/kafka/server/KafkaConfig.scala 6e26c5436feb4629d17f199011f3ebb674aa767f
core/src/main/scala/kafka/server/KafkaServer.scala 1bf7d10cef23a77e716666eb16bf6d0e68bc4ebe
core/src/test/scala/kafka/log/LogConfigTest.scala 99b0df7b69c5e0a1b6251c54592c6ef63b1800fe
core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala PRE-CREATION
core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala 4e45d965bc423192ac704883ee75e9727006f89b
core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2377abe4933e065d037828a214c3a87e1773a8ef
Diff: https://reviews.apache.org/r/24704/diff/
Testing
-------
Thanks,
Manikumar Reddy O
Re: Review Request 24704: Patch for KAFKA-1499
Posted by Manikumar Reddy O <ma...@gmail.com>.
> On Dec. 18, 2014, 4:30 a.m., Eric Olander wrote:
> > core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala, line 78
> > <https://reviews.apache.org/r/24704/diff/13/?file=793051#file793051line78>
> >
> > A more Scala-like implementation is:
> >
> > def parameters = {
> > for (brokerCompression <- BrokerCompression.brokerCompressionOptions;
> > messageCompression <- CompressionType.values
> > ) yield Array(messageCompression.name, brokerCompression)
> > }
> >
> > If the resulting collection needs to be a java Collection you could import the JavaConversions implicits, or just ignore me.
Thanks for the suggestion.
- Manikumar Reddy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24704/#review65450
-----------------------------------------------------------
On Dec. 26, 2014, 4:09 p.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24704/
> -----------------------------------------------------------
>
> (Updated Dec. 26, 2014, 4:09 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1499
> https://issues.apache.org/jira/browse/KAFKA-1499
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Support given for Broker-side compression, Addersing Joel's comments
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/log/Log.scala 024506cd00556a0037c0b3b6b603da32968b69ab
> core/src/main/scala/kafka/log/LogConfig.scala ca7a99e99f641b2694848b88bf4ae94657071d03
> core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 788c7864bc881b935975ab4a4e877b690e65f1f1
> core/src/main/scala/kafka/message/CompressionCodec.scala 9439d2bc29a0c2327085f08577c6ce1b01db1489
> core/src/main/scala/kafka/server/KafkaConfig.scala 6e26c5436feb4629d17f199011f3ebb674aa767f
> core/src/main/scala/kafka/server/KafkaServer.scala 1bf7d10cef23a77e716666eb16bf6d0e68bc4ebe
> core/src/test/scala/kafka/log/LogConfigTest.scala 99b0df7b69c5e0a1b6251c54592c6ef63b1800fe
> core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala PRE-CREATION
> core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala 4e45d965bc423192ac704883ee75e9727006f89b
> core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2377abe4933e065d037828a214c3a87e1773a8ef
>
> Diff: https://reviews.apache.org/r/24704/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 24704: Patch for KAFKA-1499
Posted by Eric Olander <ol...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24704/#review65450
-----------------------------------------------------------
core/src/main/scala/kafka/message/BrokerCompression.scala
<https://reviews.apache.org/r/24704/#comment108597>
This could be simplified to:
def isValid(compressionType: String): Boolean = brokerCompressionOptions.contains(compressionType.toLowerCase())
core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala
<https://reviews.apache.org/r/24704/#comment108598>
A more Scala-like implementation is:
def parameters = {
for (brokerCompression <- BrokerCompression.brokerCompressionOptions;
messageCompression <- CompressionType.values
) yield Array(messageCompression.name, brokerCompression)
}
If the resulting collection needs to be a java Collection you could import the JavaConversions implicits, or just ignore me.
- Eric Olander
On Dec. 16, 2014, 5:10 p.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24704/
> -----------------------------------------------------------
>
> (Updated Dec. 16, 2014, 5:10 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1499
> https://issues.apache.org/jira/browse/KAFKA-1499
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Support given for Broker-side compression
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/log/Log.scala 4fae2f0d339b256832baa62ca4995d10546716b4
> core/src/main/scala/kafka/log/LogConfig.scala ca7a99e99f641b2694848b88bf4ae94657071d03
> core/src/main/scala/kafka/message/BrokerCompression.scala PRE-CREATION
> core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 788c7864bc881b935975ab4a4e877b690e65f1f1
> core/src/main/scala/kafka/server/KafkaConfig.scala 6e26c5436feb4629d17f199011f3ebb674aa767f
> core/src/main/scala/kafka/server/KafkaServer.scala 1bf7d10cef23a77e716666eb16bf6d0e68bc4ebe
> core/src/test/scala/kafka/log/LogConfigTest.scala 99b0df7b69c5e0a1b6251c54592c6ef63b1800fe
> core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala PRE-CREATION
> core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala 4e45d965bc423192ac704883ee75e9727006f89b
> core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2377abe4933e065d037828a214c3a87e1773a8ef
>
> Diff: https://reviews.apache.org/r/24704/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 24704: Patch for KAFKA-1499
Posted by Jonathan Creasy <jo...@ghostlab.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24704/#review65221
-----------------------------------------------------------
core/src/main/scala/kafka/log/Log.scala
<https://reviews.apache.org/r/24704/#comment108254>
I wouldn't block on this since there are lots of other cases of this that aren't new but if you happen to do another patch remove this whitespace error.
- Jonathan Creasy
On Dec. 16, 2014, 5:10 p.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24704/
> -----------------------------------------------------------
>
> (Updated Dec. 16, 2014, 5:10 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1499
> https://issues.apache.org/jira/browse/KAFKA-1499
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Support given for Broker-side compression
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/log/Log.scala 4fae2f0d339b256832baa62ca4995d10546716b4
> core/src/main/scala/kafka/log/LogConfig.scala ca7a99e99f641b2694848b88bf4ae94657071d03
> core/src/main/scala/kafka/message/BrokerCompression.scala PRE-CREATION
> core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 788c7864bc881b935975ab4a4e877b690e65f1f1
> core/src/main/scala/kafka/server/KafkaConfig.scala 6e26c5436feb4629d17f199011f3ebb674aa767f
> core/src/main/scala/kafka/server/KafkaServer.scala 1bf7d10cef23a77e716666eb16bf6d0e68bc4ebe
> core/src/test/scala/kafka/log/LogConfigTest.scala 99b0df7b69c5e0a1b6251c54592c6ef63b1800fe
> core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala PRE-CREATION
> core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala 4e45d965bc423192ac704883ee75e9727006f89b
> core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2377abe4933e065d037828a214c3a87e1773a8ef
>
> Diff: https://reviews.apache.org/r/24704/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 24704: Patch for KAFKA-1499
Posted by Jonathan Creasy <jo...@ghostlab.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24704/#review65223
-----------------------------------------------------------
Ship it!
Ship It!
- Jonathan Creasy
On Dec. 16, 2014, 5:10 p.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24704/
> -----------------------------------------------------------
>
> (Updated Dec. 16, 2014, 5:10 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1499
> https://issues.apache.org/jira/browse/KAFKA-1499
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Support given for Broker-side compression
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/log/Log.scala 4fae2f0d339b256832baa62ca4995d10546716b4
> core/src/main/scala/kafka/log/LogConfig.scala ca7a99e99f641b2694848b88bf4ae94657071d03
> core/src/main/scala/kafka/message/BrokerCompression.scala PRE-CREATION
> core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 788c7864bc881b935975ab4a4e877b690e65f1f1
> core/src/main/scala/kafka/server/KafkaConfig.scala 6e26c5436feb4629d17f199011f3ebb674aa767f
> core/src/main/scala/kafka/server/KafkaServer.scala 1bf7d10cef23a77e716666eb16bf6d0e68bc4ebe
> core/src/test/scala/kafka/log/LogConfigTest.scala 99b0df7b69c5e0a1b6251c54592c6ef63b1800fe
> core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala PRE-CREATION
> core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala 4e45d965bc423192ac704883ee75e9727006f89b
> core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2377abe4933e065d037828a214c3a87e1773a8ef
>
> Diff: https://reviews.apache.org/r/24704/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 24704: Patch for KAFKA-1499
Posted by Joel Koshy <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24704/#review67954
-----------------------------------------------------------
Ship it!
- Joel Koshy
On Dec. 26, 2014, 4:09 p.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24704/
> -----------------------------------------------------------
>
> (Updated Dec. 26, 2014, 4:09 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1499
> https://issues.apache.org/jira/browse/KAFKA-1499
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Support given for Broker-side compression, Addersing Joel's comments
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/log/Log.scala 024506cd00556a0037c0b3b6b603da32968b69ab
> core/src/main/scala/kafka/log/LogConfig.scala ca7a99e99f641b2694848b88bf4ae94657071d03
> core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 788c7864bc881b935975ab4a4e877b690e65f1f1
> core/src/main/scala/kafka/message/CompressionCodec.scala 9439d2bc29a0c2327085f08577c6ce1b01db1489
> core/src/main/scala/kafka/server/KafkaConfig.scala 6e26c5436feb4629d17f199011f3ebb674aa767f
> core/src/main/scala/kafka/server/KafkaServer.scala 1bf7d10cef23a77e716666eb16bf6d0e68bc4ebe
> core/src/test/scala/kafka/log/LogConfigTest.scala 99b0df7b69c5e0a1b6251c54592c6ef63b1800fe
> core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala PRE-CREATION
> core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala 4e45d965bc423192ac704883ee75e9727006f89b
> core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2377abe4933e065d037828a214c3a87e1773a8ef
>
> Diff: https://reviews.apache.org/r/24704/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 24704: Patch for KAFKA-1499
Posted by Jun Rao <ju...@gmail.com>.
> On Feb. 27, 2015, 10:53 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/message/CompressionCodec.scala, line 46
> > <https://reviews.apache.org/r/24704/diff/14/?file=801514#file801514line46>
> >
> > Not sure what you mean - can you elaborate?
Instead of contains((compressionType.toLowerCase())), can't it just be contains(compressionType.toLowerCase())?
- Jun
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24704/#review74615
-----------------------------------------------------------
On Dec. 26, 2014, 4:09 p.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24704/
> -----------------------------------------------------------
>
> (Updated Dec. 26, 2014, 4:09 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1499
> https://issues.apache.org/jira/browse/KAFKA-1499
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Support given for Broker-side compression, Addersing Joel's comments
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/log/Log.scala 024506cd00556a0037c0b3b6b603da32968b69ab
> core/src/main/scala/kafka/log/LogConfig.scala ca7a99e99f641b2694848b88bf4ae94657071d03
> core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 788c7864bc881b935975ab4a4e877b690e65f1f1
> core/src/main/scala/kafka/message/CompressionCodec.scala 9439d2bc29a0c2327085f08577c6ce1b01db1489
> core/src/main/scala/kafka/server/KafkaConfig.scala 6e26c5436feb4629d17f199011f3ebb674aa767f
> core/src/main/scala/kafka/server/KafkaServer.scala 1bf7d10cef23a77e716666eb16bf6d0e68bc4ebe
> core/src/test/scala/kafka/log/LogConfigTest.scala 99b0df7b69c5e0a1b6251c54592c6ef63b1800fe
> core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala PRE-CREATION
> core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala 4e45d965bc423192ac704883ee75e9727006f89b
> core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2377abe4933e065d037828a214c3a87e1773a8ef
>
> Diff: https://reviews.apache.org/r/24704/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 24704: Patch for KAFKA-1499
Posted by Joel Koshy <jj...@gmail.com>.
> On Feb. 27, 2015, 10:53 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/message/CompressionCodec.scala, line 46
> > <https://reviews.apache.org/r/24704/diff/14/?file=801514#file801514line46>
> >
> > Not sure what you mean - can you elaborate?
>
> Jun Rao wrote:
> Instead of contains((compressionType.toLowerCase())), can't it just be contains(compressionType.toLowerCase())?
I still don't get it.. no I'm kidding. You're right. I clearly cannot count parentheses
- Joel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24704/#review74615
-----------------------------------------------------------
On Dec. 26, 2014, 4:09 p.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24704/
> -----------------------------------------------------------
>
> (Updated Dec. 26, 2014, 4:09 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1499
> https://issues.apache.org/jira/browse/KAFKA-1499
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Support given for Broker-side compression, Addersing Joel's comments
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/log/Log.scala 024506cd00556a0037c0b3b6b603da32968b69ab
> core/src/main/scala/kafka/log/LogConfig.scala ca7a99e99f641b2694848b88bf4ae94657071d03
> core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 788c7864bc881b935975ab4a4e877b690e65f1f1
> core/src/main/scala/kafka/message/CompressionCodec.scala 9439d2bc29a0c2327085f08577c6ce1b01db1489
> core/src/main/scala/kafka/server/KafkaConfig.scala 6e26c5436feb4629d17f199011f3ebb674aa767f
> core/src/main/scala/kafka/server/KafkaServer.scala 1bf7d10cef23a77e716666eb16bf6d0e68bc4ebe
> core/src/test/scala/kafka/log/LogConfigTest.scala 99b0df7b69c5e0a1b6251c54592c6ef63b1800fe
> core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala PRE-CREATION
> core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala 4e45d965bc423192ac704883ee75e9727006f89b
> core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2377abe4933e065d037828a214c3a87e1773a8ef
>
> Diff: https://reviews.apache.org/r/24704/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 24704: Patch for KAFKA-1499
Posted by Joel Koshy <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24704/#review74615
-----------------------------------------------------------
core/src/main/scala/kafka/message/CompressionCodec.scala
<https://reviews.apache.org/r/24704/#comment121241>
Not sure what you mean - can you elaborate?
- Joel Koshy
On Dec. 26, 2014, 4:09 p.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24704/
> -----------------------------------------------------------
>
> (Updated Dec. 26, 2014, 4:09 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1499
> https://issues.apache.org/jira/browse/KAFKA-1499
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Support given for Broker-side compression, Addersing Joel's comments
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/log/Log.scala 024506cd00556a0037c0b3b6b603da32968b69ab
> core/src/main/scala/kafka/log/LogConfig.scala ca7a99e99f641b2694848b88bf4ae94657071d03
> core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 788c7864bc881b935975ab4a4e877b690e65f1f1
> core/src/main/scala/kafka/message/CompressionCodec.scala 9439d2bc29a0c2327085f08577c6ce1b01db1489
> core/src/main/scala/kafka/server/KafkaConfig.scala 6e26c5436feb4629d17f199011f3ebb674aa767f
> core/src/main/scala/kafka/server/KafkaServer.scala 1bf7d10cef23a77e716666eb16bf6d0e68bc4ebe
> core/src/test/scala/kafka/log/LogConfigTest.scala 99b0df7b69c5e0a1b6251c54592c6ef63b1800fe
> core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala PRE-CREATION
> core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala 4e45d965bc423192ac704883ee75e9727006f89b
> core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2377abe4933e065d037828a214c3a87e1773a8ef
>
> Diff: https://reviews.apache.org/r/24704/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 24704: Patch for KAFKA-1499
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24704/#review74612
-----------------------------------------------------------
Sorry for the late review. Just a minor comment below.
core/src/main/scala/kafka/message/CompressionCodec.scala
<https://reviews.apache.org/r/24704/#comment121240>
Do we need the inner bracket on compressionType.toLowerCase()?
- Jun Rao
On Dec. 26, 2014, 4:09 p.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24704/
> -----------------------------------------------------------
>
> (Updated Dec. 26, 2014, 4:09 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1499
> https://issues.apache.org/jira/browse/KAFKA-1499
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Support given for Broker-side compression, Addersing Joel's comments
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/log/Log.scala 024506cd00556a0037c0b3b6b603da32968b69ab
> core/src/main/scala/kafka/log/LogConfig.scala ca7a99e99f641b2694848b88bf4ae94657071d03
> core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 788c7864bc881b935975ab4a4e877b690e65f1f1
> core/src/main/scala/kafka/message/CompressionCodec.scala 9439d2bc29a0c2327085f08577c6ce1b01db1489
> core/src/main/scala/kafka/server/KafkaConfig.scala 6e26c5436feb4629d17f199011f3ebb674aa767f
> core/src/main/scala/kafka/server/KafkaServer.scala 1bf7d10cef23a77e716666eb16bf6d0e68bc4ebe
> core/src/test/scala/kafka/log/LogConfigTest.scala 99b0df7b69c5e0a1b6251c54592c6ef63b1800fe
> core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala PRE-CREATION
> core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala 4e45d965bc423192ac704883ee75e9727006f89b
> core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2377abe4933e065d037828a214c3a87e1773a8ef
>
> Diff: https://reviews.apache.org/r/24704/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 24704: Patch for KAFKA-1499
Posted by Manikumar Reddy O <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24704/
-----------------------------------------------------------
(Updated Dec. 26, 2014, 4:09 p.m.)
Review request for kafka.
Bugs: KAFKA-1499
https://issues.apache.org/jira/browse/KAFKA-1499
Repository: kafka
Description (updated)
-------
Support given for Broker-side compression, Addersing Joel's comments
Diffs (updated)
-----
core/src/main/scala/kafka/log/Log.scala 024506cd00556a0037c0b3b6b603da32968b69ab
core/src/main/scala/kafka/log/LogConfig.scala ca7a99e99f641b2694848b88bf4ae94657071d03
core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 788c7864bc881b935975ab4a4e877b690e65f1f1
core/src/main/scala/kafka/message/CompressionCodec.scala 9439d2bc29a0c2327085f08577c6ce1b01db1489
core/src/main/scala/kafka/server/KafkaConfig.scala 6e26c5436feb4629d17f199011f3ebb674aa767f
core/src/main/scala/kafka/server/KafkaServer.scala 1bf7d10cef23a77e716666eb16bf6d0e68bc4ebe
core/src/test/scala/kafka/log/LogConfigTest.scala 99b0df7b69c5e0a1b6251c54592c6ef63b1800fe
core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala PRE-CREATION
core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala 4e45d965bc423192ac704883ee75e9727006f89b
core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2377abe4933e065d037828a214c3a87e1773a8ef
Diff: https://reviews.apache.org/r/24704/diff/
Testing
-------
Thanks,
Manikumar Reddy O
Re: Review Request 24704: Patch for KAFKA-1499
Posted by Manikumar Reddy O <ma...@gmail.com>.
> On Dec. 17, 2014, 1:57 p.m., Joel Koshy wrote:
> > core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala, line 67
> > <https://reviews.apache.org/r/24704/diff/13/?file=793051#file793051line67>
> >
> > `...should produce " + readMessage(0).compressionCodec`
i didnt find any issue here
> On Dec. 17, 2014, 1:57 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/message/BrokerCompression.scala, line 20
> > <https://reviews.apache.org/r/24704/diff/13/?file=793046#file793046line20>
> >
> > Can you think of a way to avoid having to maintain this distinctly from CompressionCodec.scala?
> >
> > E.g., one thought is to have a separate BrokerCompressionCodec sealed trait in that class; have all the existing codecs extend from both CompressionCodec and BrokerCompressionCodec and the new ProducerCompressionCodec only extend from BrokerCompressionCodec
> >
> > (or see if there is an altogether different better way)
Joel, i implemented your suggestion here. Merged the new BrokerCompression code to existing CompressionCodec.scala. Pl let me know if any changes are required.
- Manikumar Reddy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24704/#review65316
-----------------------------------------------------------
On Dec. 26, 2014, 4:09 p.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24704/
> -----------------------------------------------------------
>
> (Updated Dec. 26, 2014, 4:09 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1499
> https://issues.apache.org/jira/browse/KAFKA-1499
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Support given for Broker-side compression, Addersing Joel's comments
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/log/Log.scala 024506cd00556a0037c0b3b6b603da32968b69ab
> core/src/main/scala/kafka/log/LogConfig.scala ca7a99e99f641b2694848b88bf4ae94657071d03
> core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 788c7864bc881b935975ab4a4e877b690e65f1f1
> core/src/main/scala/kafka/message/CompressionCodec.scala 9439d2bc29a0c2327085f08577c6ce1b01db1489
> core/src/main/scala/kafka/server/KafkaConfig.scala 6e26c5436feb4629d17f199011f3ebb674aa767f
> core/src/main/scala/kafka/server/KafkaServer.scala 1bf7d10cef23a77e716666eb16bf6d0e68bc4ebe
> core/src/test/scala/kafka/log/LogConfigTest.scala 99b0df7b69c5e0a1b6251c54592c6ef63b1800fe
> core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala PRE-CREATION
> core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala 4e45d965bc423192ac704883ee75e9727006f89b
> core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2377abe4933e065d037828a214c3a87e1773a8ef
>
> Diff: https://reviews.apache.org/r/24704/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 24704: Patch for KAFKA-1499
Posted by Joel Koshy <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24704/#review65316
-----------------------------------------------------------
Overall, looks good. I do have a bit of a concern on having to maintain a separate set of broker compression codecs and I think that can be avoided. Looking forward to an updated patch. Please ping me as you did if you don't get a timely follow-up review.
core/src/main/scala/kafka/log/Log.scala
<https://reviews.apache.org/r/24704/#comment108432>
Can you update the scaladoc?
core/src/main/scala/kafka/log/LogConfig.scala
<https://reviews.apache.org/r/24704/#comment108437>
Can you make this doc a little more descriptive? E.g., how it relates to the producer compression type
core/src/main/scala/kafka/message/BrokerCompression.scala
<https://reviews.apache.org/r/24704/#comment108434>
Can you think of a way to avoid having to maintain this distinctly from CompressionCodec.scala?
E.g., one thought is to have a separate BrokerCompressionCodec sealed trait in that class; have all the existing codecs extend from both CompressionCodec and BrokerCompressionCodec and the new ProducerCompressionCodec only extend from BrokerCompressionCodec
(or see if there is an altogether different better way)
core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/24704/#comment108438>
Similar note as above
core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala
<https://reviews.apache.org/r/24704/#comment108439>
`...should produce " + readMessage(0).compressionCodec`
- Joel Koshy
On Dec. 16, 2014, 5:10 p.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24704/
> -----------------------------------------------------------
>
> (Updated Dec. 16, 2014, 5:10 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1499
> https://issues.apache.org/jira/browse/KAFKA-1499
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Support given for Broker-side compression
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/log/Log.scala 4fae2f0d339b256832baa62ca4995d10546716b4
> core/src/main/scala/kafka/log/LogConfig.scala ca7a99e99f641b2694848b88bf4ae94657071d03
> core/src/main/scala/kafka/message/BrokerCompression.scala PRE-CREATION
> core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 788c7864bc881b935975ab4a4e877b690e65f1f1
> core/src/main/scala/kafka/server/KafkaConfig.scala 6e26c5436feb4629d17f199011f3ebb674aa767f
> core/src/main/scala/kafka/server/KafkaServer.scala 1bf7d10cef23a77e716666eb16bf6d0e68bc4ebe
> core/src/test/scala/kafka/log/LogConfigTest.scala 99b0df7b69c5e0a1b6251c54592c6ef63b1800fe
> core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala PRE-CREATION
> core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala 4e45d965bc423192ac704883ee75e9727006f89b
> core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2377abe4933e065d037828a214c3a87e1773a8ef
>
> Diff: https://reviews.apache.org/r/24704/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>