> On May 19, 2015, 9:41 p.m., Guozhang Wang wrote:
> > About duplicating the compression logic, one way would be:
> >
> > 1. Add the following functions in ByteBufferMessageSet as:
> >
> > fill(buffer: ByteBuffer, compressionCodec: CompressionCodec, messages:
> > MessageAndOffset*): Int
> >
> > 2. Change ByteBufferMessageSet.create() as:
> >
> > create(offsetCounter: AtomicLong, compressionCodec: CompressionCodec,
> > messages: Message*): ByteBuffer = {
> >
> > // 1. pair messages to MessageAndOffset with offsetCounter
> > // 2. create the buffer as
> > ByteBuffer.allocate(MessageSet.messageSetSize(messages))
> > // 3. call fill()
> > }
Yeah I thought of doing this, but thought it may be better to defer on it
because:
- The duplicate code isolates these immediate changes for log compaction. i.e.,
the existing producer request handling will be unaffected.
- It forces an additional iteration for the common case. i.e., create on
messages will do one iteration to assign offsets and create MessageAndOffset;
and then a subsequent iteration in fill
- We should evaluate using the client compressor due to the other issue I
mentioned (i.e., different compression code on broker vs client)
So just to be clear, I'm going to file a follow-up jira to investigate the
above and also fix the broker compression codec issue (i.e., recompress to
configured codec during log compaction).
> On May 19, 2015, 9:41 p.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/message/ByteBufferMessageSet.scala, lines 261-264
> > <https://reviews.apache.org/r/34397/diff/1/?file=963488#file963488line261>
> >
> > Do we need to modify this comment as well?
Yes these comments could use an edit.
> On May 19, 2015, 9:41 p.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/log/LogCleaner.scala, lines 432-433
> > <https://reviews.apache.org/r/34397/diff/1/?file=963487#file963487line432>
> >
> > I think the coding style convention is
> >
> > if {
> >
> > } else {
> >
> > }
Will do.
- Joel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34397/#review84390
-----------------------------------------------------------
On May 20, 2015, 12:20 a.m., Joel Koshy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34397/
> -----------------------------------------------------------
>
> (Updated May 20, 2015, 12:20 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1374
> https://issues.apache.org/jira/browse/KAFKA-1374
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Remove no-compression constraint for compacted topics
>
>
> Fix log cleaner integration test
>
>
> Incorporate edits from latest patch
>
>
> More minor edits
>
>
> Incorporate Guozhang's comments
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/log/LogCleaner.scala
> abea8b251895a5cc0788c6e25b112a2935a3f631
> core/src/main/scala/kafka/message/ByteBufferMessageSet.scala
> 9dfe914991aaf82162e5e300c587c794555d5fd0
> core/src/test/scala/kafka/tools/TestLogCleaning.scala
> 844589427cb9337acd89a5239a98b811ee58118e
> core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala
> 3b5aa9dc3b7ac5893c1d281ae1326be0e9ed8aad
> core/src/test/scala/unit/kafka/log/LogTest.scala
> 76d3bfd378f32fd2b216b3ebdec86e2070491924
>
> Diff: https://reviews.apache.org/r/34397/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joel Koshy
>
>
