----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/#review50893 -----------------------------------------------------------
Thanks for the patch. Looks good overall. Could you run the stress test in TestLogCleaning with compression turned on and see if there is any problem? core/src/main/scala/kafka/log/LogCleaner.scala <https://reviews.apache.org/r/24214/#comment88766> Hmm, I think the original approach of throwing an exception is probably better. When handling the produce requests, we can reject messages w/o a key, if the topic is configured with compaction. Once we do that, there should be no messages with null key during compaction. If that happens, we should just fail the broker. core/src/main/scala/kafka/log/LogCleaner.scala <https://reviews.apache.org/r/24214/#comment88765> Could we use MemoryRecords.RecordsIterator to iterate compressed messages? core/src/main/scala/kafka/log/LogCleaner.scala <https://reviews.apache.org/r/24214/#comment88763> Could this be named compressMessages()? - Jun Rao On Aug. 12, 2014, 4:57 p.m., Manikumar Reddy O wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24214/ > ----------------------------------------------------------- > > (Updated Aug. 12, 2014, 4:57 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1374 > https://issues.apache.org/jira/browse/KAFKA-1374 > > > Repository: kafka > > > Description > ------- > > Ignored messages with null Keys during compaction. This is for KAFKA-1581. It > is a simple fix. so combining with this patch. > > > Diffs > ----- > > core/src/main/scala/kafka/log/LogCleaner.scala > c20de4ad4734c0bd83c5954fdb29464a27b91dff > core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala > 5bfa764638e92f217d0ff7108ec8f53193c22978 > > Diff: https://reviews.apache.org/r/24214/diff/ > > > Testing > ------- > > > Thanks, > > Manikumar Reddy O > >
