> On Aug. 14, 2014, 9:52 p.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/server/KafkaConfig.scala, line 323
> > <https://reviews.apache.org/r/24704/diff/1/?file=660645#file660645line323>
> >
> >     Can we just call the config compression.type? Also, the default value 
> > of NoCompressionCodec is a little misleading. It gives the impression that 
> > if compressed data is sent, the broker would write it as uncompressed. 
> > Maybe we could specify that explicitly in the config doc?
> 
> Manikumar Reddy O wrote:
>     ok. I renamed the "default.compression.type" to "compression.type".  This 
> config property is applicable to all non-compressed messages received on the 
> broker. Yes, we should explicitly specify this in documentation.
> 
> Joel Koshy wrote:
>     Manikumar/Neha - would it be less confusing if we add another config as 
> well. Something like "broker.compression.enabled". If that is true, then 
> always honor "broker.compression.type" even if it is NoCompressionCodec
>     
>     The current patch behavior may be a bit confusing: Suppose the default 
> compression.type is GZIP. Although I cannot think of a very compelling 
> use-case for this, if someone intentionally overrides a topic's 
> compression.type to NoCompressioNCodec that override will be ignored.
> 
> Neha Narkhede wrote:
>     Something like "broker.compression.enabled". If that is true, then always 
> honor "broker.compression.type" even if it is NoCompressionCodec
>     
>     +1. That would certainly make it less confusing.

Joel -  We will not ignore any per-topic overrides. per-topic override always 
be used. 

If default compression.type is GZIP, if someone intentionally overrides a 
topic's compression.type to NoCompressionCodec then NoCompressionCodec will be 
used for that topic. 

Current patch supports the following behavior

 broker                 per-topic             final-compression  
compression.type     compression.type           for topic

none (default)             -                       none
gzip                       -                       gzip
gzip                      none                     none
gzip                     snappy                    snappy


And above is applicable to only non-compressed messages received on broker. 

Do you still think we need a new "broker.compression.enabled" property?


- Manikumar Reddy


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


On Aug. 15, 2014, 8:53 a.m., Manikumar Reddy O wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24704/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2014, 8:53 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1499
>     https://issues.apache.org/jira/browse/KAFKA-1499
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Addressing Neha's,Joel's comments
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/log/Log.scala 
> 0ddf97bd30311b6039e19abade41d2fbbad2f59b 
>   core/src/main/scala/kafka/log/LogConfig.scala 
> 5746ad4767589594f904aa085131dd95e56d72bb 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 
> 1a45f8716ccc0398cf9395d91d66199d16882aae 
>   core/src/main/scala/kafka/server/KafkaServer.scala 
> 28711182aaa70eaa623de858bc063cb2613b2a4d 
>   core/src/test/scala/unit/kafka/log/LogTest.scala 
> 577d102fc2eb6bb1a72326141ecd431db6d66f04 
>   core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 
> 2377abe4933e065d037828a214c3a87e1773a8ef 
> 
> Diff: https://reviews.apache.org/r/24704/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Manikumar Reddy O
> 
>

Reply via email to