> 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.
>
> Manikumar Reddy O wrote:
> 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?
Sorry if I'm misreading (let me know). From what I can see, the current
condition (line 371) in the latest diff is:
if (config.compressionType != NoCompressionCodec && codec ==
NoCompressionCodec)
codec = config.compressionType
So suppose a specific topic's override is NoCompressionCodec and an incoming
message-set has a gzip'd message in it. So codec would be set to gzip (line
367). So the condition (line 371) fails and codec will not be changed to
NoCompressionCodec.
> above is applicable to only non-compressed messages received on broker.
This also leaves some room for confusion. Ideally, if there is an explicit
per-topic override, then that should always be used for that topic.
So for example, if a topic's override is snappy; and an incoming message is
gzip. Then the final codec will be gzip - i..e, codec set to gzip (line 367)
and then the update is not applied because codec is not NoCompressionCodec
(line 371). When a user specifies an override for a config named
compression.type it is unintuitive if it is possible for the messages to be of
mixed compression types. This is not a contrived use case - say, if an
organization wants to immediately move its stored data from gzip to snappy but
cannot immediately move all its producers over from gzip to snappy.
So does that mean the above condition should be removed altogether and we
should always set codec to config.compressionType? Not really, because
presumably the default compression.type will be NoCompressionCodec and that is
also not ideal - i.e., everything sent to the broker will be stored
uncompressed. That's perhaps where a broker.compression.enabled will be useful.
- Joel
-----------------------------------------------------------
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
>
>
