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



clients/src/main/java/org/apache/kafka/clients/NetworkClient.java
<https://reviews.apache.org/r/23697/#comment84551>

    For clarity, could we calculate the second part to a local val and give it 
a meaningful name?



clients/src/main/java/org/apache/kafka/clients/NetworkClient.java
<https://reviews.apache.org/r/23697/#comment84550>

    Thinking a bit more about this. There are two possibility after adding the 
metadata request to sends: (1) The metadata fetch succeeds. In this case 
Metadata.lastRefreshMs will be updated and this line is not needed. (2) The 
metadata fetch fails due to a connection issue. In this case, we probably want 
to fetch the metadata request from another node immediately, instead of backing 
off.
    
    So, it seems that in NetworkClient, we only need to take care of the case 
when no node is available for metadata. In this case, we should backoff. So, 
perhaps we should rename metadataLastUpdateAttemptMs to sth like lastNoNodeMs.



core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/23697/#comment84552>

    Indentation.


- Jun Rao


On July 18, 2014, 10:45 p.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23697/
> -----------------------------------------------------------
> 
> (Updated July 18, 2014, 10:45 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1533
>     https://issues.apache.org/jira/browse/KAFKA-1533
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> 1. Add the metadataRefreshAttemptMs in NetworkClient for backing off; 2. 
> Refactor Producer API tests using KafkaTestHarness; 3. Change default backoff 
> time to 100ms for test utils
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/clients/NetworkClient.java 
> d8f9ce663ee24d2b0852c974136741280c39f8f8 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java
>  4aa5b01d611631db72df47d50bbe30edb8c478db 
>   core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 
> 15fd5bcbaf175a0f7d7cf0b142e63f705ca9b6ae 
>   core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 
> 34a7db4b4ea2b720476c2b1f22a623a997faffbc 
>   core/src/test/scala/unit/kafka/integration/KafkaServerTestHarness.scala 
> 194dd70919a5f301d3131c56594e40a0ebb27311 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> 3faa884f8eb83c7c00baab416d0acfb488dc39c1 
> 
> Diff: https://reviews.apache.org/r/23697/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>

Reply via email to