You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Joel Koshy <jj...@gmail.com> on 2015/06/17 01:04:24 UTC

Re: Review Request 34805: Patch for KAFKA-2213

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


Thanks for the patch.


clients/src/main/java/org/apache/kafka/common/record/Compressor.java (line 94)
<https://reviews.apache.org/r/34805/#comment140554>

    I think a slightly better API here would be one that provides a relative start offset.



core/src/main/scala/kafka/log/Log.scala (line 279)
<https://reviews.apache.org/r/34805/#comment140556>

    Is this required? config is already exposed.



core/src/main/scala/kafka/log/LogCleaner.scala (line 359)
<https://reviews.apache.org/r/34805/#comment140558>

    Can we pass in just the compression type instead of the entire config?



core/src/main/scala/kafka/log/LogCleaner.scala (line 404)
<https://reviews.apache.org/r/34805/#comment140562>

    Can we now avoid this additional copy of retained messages by sending retained messages directly into the compressor?


- Joel Koshy


On May 29, 2015, 6:55 p.m., Manikumar Reddy O wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34805/
> -----------------------------------------------------------
> 
> (Updated May 29, 2015, 6:55 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2213
>     https://issues.apache.org/jira/browse/KAFKA-2213
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Write the compacted messages using the configured broker compression type, Used client Compressor/MemoryRecords classes
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/common/record/Compressor.java e570b29d5ffba5d3754c46670b708f7d511086f3 
>   clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java b2db2403868b1e7361b8514cfed2e76ef785edee 
>   core/src/main/scala/kafka/log/Log.scala 84e7b8fe9dd014884b60c4fbe13c835cf02a40e4 
>   core/src/main/scala/kafka/log/LogCleaner.scala c9ade7208798fbd92d4ff49e183fe5f8925c82a9 
>   core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala 471ddff9bff1bdfa277c071e59e5c6b749b9c74f 
> 
> Diff: https://reviews.apache.org/r/34805/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Manikumar Reddy O
> 
>


Re: Review Request 34805: Patch for KAFKA-2213

Posted by Manikumar Reddy O <ma...@gmail.com>.

> On June 16, 2015, 11:04 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/log/LogCleaner.scala, line 406
> > <https://reviews.apache.org/r/34805/diff/2/?file=974551#file974551line406>
> >
> >     Can we now avoid this additional copy of retained messages by sending retained messages directly into the compressor?

Extra iteration is required to find out latest message compression codec.


> On June 16, 2015, 11:04 p.m., Joel Koshy wrote:
> > clients/src/main/java/org/apache/kafka/common/record/Compressor.java, line 94
> > <https://reviews.apache.org/r/34805/diff/2/?file=974548#file974548line94>
> >
> >     I think a slightly better API here would be one that provides a relative start offset.

If we are compressing messages with offsets 10,13,15, we need to set the compressed message with offset 15. Can you explain how can we can achive this with relative start offset?


- Manikumar Reddy


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


On May 29, 2015, 6:55 p.m., Manikumar Reddy O wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34805/
> -----------------------------------------------------------
> 
> (Updated May 29, 2015, 6:55 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2213
>     https://issues.apache.org/jira/browse/KAFKA-2213
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Write the compacted messages using the configured broker compression type, Used client Compressor/MemoryRecords classes
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/common/record/Compressor.java e570b29d5ffba5d3754c46670b708f7d511086f3 
>   clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java b2db2403868b1e7361b8514cfed2e76ef785edee 
>   core/src/main/scala/kafka/log/Log.scala 84e7b8fe9dd014884b60c4fbe13c835cf02a40e4 
>   core/src/main/scala/kafka/log/LogCleaner.scala c9ade7208798fbd92d4ff49e183fe5f8925c82a9 
>   core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala 471ddff9bff1bdfa277c071e59e5c6b749b9c74f 
> 
> Diff: https://reviews.apache.org/r/34805/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Manikumar Reddy O
> 
>