----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34397/#review84390 -----------------------------------------------------------
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() } core/src/main/scala/kafka/log/LogCleaner.scala <https://reviews.apache.org/r/34397/#comment135612> I think the coding style convention is if { } else { } core/src/main/scala/kafka/message/ByteBufferMessageSet.scala <https://reviews.apache.org/r/34397/#comment135613> Do we need to modify this comment as well? - Guozhang Wang On May 19, 2015, 7:54 a.m., Joel Koshy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34397/ > ----------------------------------------------------------- > > (Updated May 19, 2015, 7:54 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1374 > https://issues.apache.org/jira/browse/KAFKA-1374 > > > Repository: kafka > > > Description > ------- > > Rebased patch; incorporate edits and further changes from Manikumar > > > 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 > >