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 2014/08/05 18:39:05 UTC
Re: Review Request 23906: Patch for KAFKA-1527

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



core/src/main/scala/kafka/consumer/ConsumerConfig.scala
<https://reviews.apache.org/r/23906/#comment86374>

    May be clearer to use read.uncommitted:
    - false means read everything
    - true means to read only committed or non-transactional messages.



core/src/main/scala/kafka/consumer/SimpleConsumer.scala
<https://reviews.apache.org/r/23906/#comment86391>

    Would be clearer to use case classes instead of tuples - applies here and in the next three declarations. Otherwise it is cumbersome to guess what the String, Int, Int, etc. correspond to.



core/src/main/scala/kafka/consumer/SimpleConsumer.scala
<https://reviews.apache.org/r/23906/#comment86839>

    hard to read - let us use case classes for these (and other tuples). Tuples are okay only when it is used in a purely local context and are assigned. e.g.,
    (firstInt, secondInt) = (1, 2)



core/src/main/scala/kafka/consumer/SimpleConsumer.scala
<https://reviews.apache.org/r/23906/#comment86673>

    Would it be possible to only do shallow iteration here in order to avoid decompression overhead? For that to work the txid needs to be present at the message-set level which should be in the producer implementation. Also, the transaction coordinator should either only send individual transaction control messages (not batched) or if batching turn off compression.
    
    It's an optimization though since the consumer eventually does need to decompress - the only thing is that avoiding decompression allows longer buffering.



core/src/main/scala/kafka/consumer/SimpleConsumer.scala
<https://reviews.apache.org/r/23906/#comment86684>

    Why do this after the above statement?


- Joel Koshy


On July 24, 2014, 11:23 p.m., Raul Castro Fernandez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23906/
> -----------------------------------------------------------
> 
> (Updated July 24, 2014, 11:23 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1527
>     https://issues.apache.org/jira/browse/KAFKA-1527
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1527; SimpleConsumer should be transaction aware
> 
> 
> Diffs
> -----
> 
>   bin/kafka-simple-consumer-perf-test.sh PRE-CREATION 
>   core/src/main/scala/kafka/consumer/ConsumerConfig.scala 1cf2f62ba02e4aa66bfa7575865e5d57baf82212 
>   core/src/main/scala/kafka/consumer/SimpleConsumer.scala 0e64632210385ef63c2ad3445b55ac4f37a63df2 
>   core/src/main/scala/kafka/tools/SimpleConsumerPerformance.scala 7602b8d705970a5dab49ed36d117346a960701ac 
> 
> Diff: https://reviews.apache.org/r/23906/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Raul Castro Fernandez
> 
>