----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18299/#review35908 -----------------------------------------------------------
Could you rebase? clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java <https://reviews.apache.org/r/18299/#comment66722> Could you add the logging? clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java <https://reviews.apache.org/r/18299/#comment66723> My earlier comment about whether we should close the RecordBatch immediately after this record is appended, is not due to synchronization. My concern is that if we don't close this RecordBatch, the next message could be added as an uncompressed one when it can be added as a compressed one. Not sure if it's a big concern though. clients/src/main/java/org/apache/kafka/common/record/CompressionType.java <https://reviews.apache.org/r/18299/#comment66724> Could we just use Class.forName()? Could we also add a comment on why we want to load the class dynamically? clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java <https://reviews.apache.org/r/18299/#comment66726> What is the TODO item? clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java <https://reviews.apache.org/r/18299/#comment66728> BufferOverflowException is a RuntimeException, not an IOException. clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java <https://reviews.apache.org/r/18299/#comment66727> Could you add the logging? clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java <https://reviews.apache.org/r/18299/#comment66730> Should we call maybeInitForAppend() or assert that appendStream is not null? clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java <https://reviews.apache.org/r/18299/#comment66732> Shouldn't we set the limit for buffer to pos at the end so that RecordAccumulator.ready() can pick up this MemoryRecords after closed? clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java <https://reviews.apache.org/r/18299/#comment66731> What's the TODO item? The key for the shallow message is always null. clients/src/main/java/org/apache/kafka/common/record/Record.java <https://reviews.apache.org/r/18299/#comment66725> ??? clients/src/test/java/org/apache/kafka/common/record/MemoryRecordsTest.java <https://reviews.apache.org/r/18299/#comment66733> Line 81 not needed? core/src/test/scala/integration/kafka/api/ProducerSendTest.scala <https://reviews.apache.org/r/18299/#comment66734> Could we verify the content of the fetched messages too? - Jun Rao On Feb. 27, 2014, 1:33 a.m., Guozhang Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18299/ > ----------------------------------------------------------- > > (Updated Feb. 27, 2014, 1:33 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1253 > https://issues.apache.org/jira/browse/KAFKA-1253 > > > Repository: kafka > > > Description > ------- > > Incorporated Jun's comments. > > TODOs: > > Class loader for Snappy. > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java > e4bc97279585818860487a39a93b6481742b91db > clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java > d8e35e7d0e4cd27aad9a8d4bf14bc97458da9417 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java > ce5cf27efa08b79e501439cf79bc8666054a5429 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java > eb16f6d236e07b16654623606294a051531b5f58 > > clients/src/main/java/org/apache/kafka/common/record/ByteBufferInputStream.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/record/ByteBufferOutputStream.java > PRE-CREATION > clients/src/main/java/org/apache/kafka/common/record/CompressionType.java > 906da02d02c03aadd8ab73ed2fc9a1898acb8d72 > clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java > 9d8935fa3beeb2a78b109a41ed76fd4374239560 > clients/src/main/java/org/apache/kafka/common/record/Record.java > f1dc9778502cbdfe982254fb6e25947842622239 > clients/src/main/java/org/apache/kafka/common/utils/Utils.java > 9c34e7dc82f33df7406cad0e64eb6a896d068dc6 > clients/src/test/java/org/apache/kafka/common/record/MemoryRecordsTest.java > b0745b528cef929c4273f7e2ac4de1476cfc25ad > clients/src/test/java/org/apache/kafka/common/record/RecordTest.java > ae54d67da9907b0a043180c7395a1370b3d0528d > clients/src/test/java/org/apache/kafka/common/utils/CrcTest.java > PRE-CREATION > clients/src/test/java/org/apache/kafka/test/TestUtils.java > 36cfc0fda742eb501af2c2c0330e3f461cf1f40c > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala > 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e > > Diff: https://reviews.apache.org/r/18299/diff/ > > > Testing > ------- > > integration tests > > > Thanks, > > Guozhang Wang > >
