You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Jun Rao <ju...@gmail.com> on 2014/04/01 18:50:36 UTC

Re: Review Request 19831: Patch for KAFKA-1337

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


Could metadata.fetch.backoff.ms and retry.backoff.ms be merged into one property?


clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java
<https://reviews.apache.org/r/19831/#comment71529>

    Is the java variable name part of the public API too?



clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java
<https://reviews.apache.org/r/19831/#comment71530>

    We probably can include sth like "make sure that at least one of the brokers in the bootstrap list is accessible".



clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java
<https://reviews.apache.org/r/19831/#comment71535>

    leadership changes => partition leadership changes



clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java
<https://reviews.apache.org/r/19831/#comment71531>

    It's probably clearer if we change "The number of acknowledgements" to "The number of acknowledgements from the replicas"



clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java
<https://reviews.apache.org/r/19831/#comment71533>

    ellipses => elapses 



clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java
<https://reviews.apache.org/r/19831/#comment71532>

    We probably should mention that higher linger time improves throughput, but increases latency.



clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java
<https://reviews.apache.org/r/19831/#comment71534>

    Could we specify what exception will be thrown if the setting is false?



clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java
<https://reviews.apache.org/r/19831/#comment71536>

    It seems that we always include the built-in JmxReporter. So, we should mention this is the additional list of reporters that one can specify.


- Jun Rao


On March 30, 2014, 6:48 p.m., Jay Kreps wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19831/
> -----------------------------------------------------------
> 
> (Updated March 30, 2014, 6:48 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1337
>     https://issues.apache.org/jira/browse/KAFKA-1337
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1337 Rationalize the producer configs.
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 8c1c5751a9186e6bc3196d152e4c3e948cc34b39 
>   clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java 259c14bd072a18c8e6a6b851db9fb8131da7426c 
>   clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java 3b3fb2c36fc45cc24da5008141310019befb05a9 
>   clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 84a327e8c1f06840492abc38e5cb0446cfbc7541 
>   clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 67b349dc8d840b0b7bfb4bedde819e092dc48d21 
>   clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 3950eb13d37e41276db9b4d2db24c2b1f4118fc2 
>   clients/src/main/java/org/apache/kafka/common/metrics/MetricsReporter.java 2c395b1f8f3434625959f70d2f4fec40857270e8 
>   clients/src/test/java/org/apache/kafka/common/config/ConfigDefTest.java 29543dfa34d5f26397d9327d4e8d0c87b28b1fa9 
>   core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala 1d73aca658a9ea31e76c3e12575703823ed0428f 
>   core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 525a06076bda51eaf001fb0a47091094c06c35f7 
>   core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 3c37330a366e84eb3bf0a96ed45de6e3f01b0a71 
> 
> Diff: https://reviews.apache.org/r/19831/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jay Kreps
> 
>


Re: Review Request 19831: Patch for KAFKA-1337

Posted by Jay Kreps <bo...@gmail.com>.

> On April 1, 2014, 4:50 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java, lines 32-33
> > <https://reviews.apache.org/r/19831/diff/1/?file=540717#file540717line32>
> >
> >     Is the java variable name part of the public API too?

Yes, as long as we allow them to be public. Alternatively we could just get rid of them. But people have asked for variables and I suppose it doesn't hurt.


- Jay


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


On March 30, 2014, 6:48 p.m., Jay Kreps wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19831/
> -----------------------------------------------------------
> 
> (Updated March 30, 2014, 6:48 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1337
>     https://issues.apache.org/jira/browse/KAFKA-1337
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1337 Rationalize the producer configs.
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 8c1c5751a9186e6bc3196d152e4c3e948cc34b39 
>   clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java 259c14bd072a18c8e6a6b851db9fb8131da7426c 
>   clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java 3b3fb2c36fc45cc24da5008141310019befb05a9 
>   clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 84a327e8c1f06840492abc38e5cb0446cfbc7541 
>   clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 67b349dc8d840b0b7bfb4bedde819e092dc48d21 
>   clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 3950eb13d37e41276db9b4d2db24c2b1f4118fc2 
>   clients/src/main/java/org/apache/kafka/common/metrics/MetricsReporter.java 2c395b1f8f3434625959f70d2f4fec40857270e8 
>   clients/src/test/java/org/apache/kafka/common/config/ConfigDefTest.java 29543dfa34d5f26397d9327d4e8d0c87b28b1fa9 
>   core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala 1d73aca658a9ea31e76c3e12575703823ed0428f 
>   core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 525a06076bda51eaf001fb0a47091094c06c35f7 
>   core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 3c37330a366e84eb3bf0a96ed45de6e3f01b0a71 
> 
> Diff: https://reviews.apache.org/r/19831/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jay Kreps
> 
>