kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Guozhang Wang" <guw...@linkedin.com>
Subject Re: Review Request 18299: Proposed In-place Compression on MemoryRecords
Date Tue, 04 Mar 2014 23:28:56 GMT


> On March 2, 2014, 2:15 a.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java,
lines 155-157
> > <https://reviews.apache.org/r/18299/diff/5/?file=505388#file505388line155>
> >
> >     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.

Got it. Agreed.


> On March 2, 2014, 2:15 a.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java, lines 62-63
> > <https://reviews.apache.org/r/18299/diff/5/?file=505393#file505393line62>
> >
> >     What is the TODO item?

What I am trying to declare is that this hand-written logic is dependent on the fact that
compressed shallow message has key==null. If it is changed in the future this logic will break.
Probably I should just leave it as normal comments?


> On March 2, 2014, 2:15 a.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/record/Record.java, lines 156-158
> > <https://reviews.apache.org/r/18299/diff/5/?file=505394#file505394line156>
> >
> >     ???

Sorry, placeholder function that should be removed..


> On March 2, 2014, 2:15 a.m., Jun Rao wrote:
> > clients/src/test/java/org/apache/kafka/common/record/MemoryRecordsTest.java, lines
84-85
> > <https://reviews.apache.org/r/18299/diff/5/?file=505396#file505396line84>
> >
> >     Line 81 not needed?

Should be removed..


- Guozhang


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


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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message