You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Jay Kreps <bo...@gmail.com> on 2014/05/27 21:15:08 UTC
Review Request 21937: KAFKA-1316 WIP refactor of sender.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21937/
-----------------------------------------------------------
Review request for kafka.
Bugs: KAFKA-1316
https://issues.apache.org/jira/browse/KAFKA-1316
Repository: kafka
Description
-------
KAFKA-1316 WIP.
Diffs
-----
clients/src/main/java/org/apache/kafka/clients/ClientRequest.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/ClientResponse.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/ConnectionState.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/InFlightRequests.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/NetworkClient.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/NodeConnectionState.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java f1def508153b7361b8720dbcc939e06b0e7e45d3
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 3e83ae0fefb7578cccbd8dc0cd9b28f22e0951b2
Diff: https://reviews.apache.org/r/21937/diff/
Testing
-------
Thanks,
Jay Kreps
Re: Review Request 21937: Patch for KAFKA-1316
Posted by Guozhang Wang <gu...@linkedin.com>.
> On June 6, 2014, 3:18 a.m., Neha Narkhede wrote:
> > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java, line 58
> > <https://reviews.apache.org/r/21937/diff/3/?file=603197#file603197line58>
> >
> > Now we will end up with potentially two Senders - one for the producer's state machine and another for the consumer's state machine. Can we rename this one to sth like ProduerSender?
I think this is fine: this Sender is under kafka.clients.producer.internal, the consumer Sender will be under consumer.internal and there will not likely classes we need to import both.
- Guozhang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21937/#review44850
-----------------------------------------------------------
On June 3, 2014, 9:33 p.m., Jay Kreps wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21937/
> -----------------------------------------------------------
>
> (Updated June 3, 2014, 9:33 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1316
> https://issues.apache.org/jira/browse/KAFKA-1316
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1316 Refactor a reusable NetworkClient interface out of Sender.
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/ClientRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ClientResponse.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ConnectionState.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/InFlightRequests.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/KafkaClient.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/NetworkClient.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/NodeConnectionState.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java d15562a968d9e4b08f26b8d30986881adfe29e31
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 9b1f5653548ba90defdae43940a5554066770b0a
> clients/src/main/java/org/apache/kafka/common/network/Selector.java 3e358985ed72a894a71d683acc7460695d6f2056
> clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java 68b8827f3bdd64580e1b443fce5b8c63152dd94a
> clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 428968cd38a7b12991f87868bf759926ff7e594e
> clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java 6fa4a58f5f9792776a647e8f682d7faadc0d1556
> clients/src/test/java/org/apache/kafka/clients/MockClient.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/clients/NetworkClientTest.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java c4072ae90fb58101a67f83054fbe0b8349e71c2e
> clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java 3ef692ca3e9fb83868a9da6f30c0705bb3d0aed2
> clients/src/test/java/org/apache/kafka/common/utils/MockTime.java cda8e644587aad9a8c9c96c222edc1ba27de1fb0
> core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala cd4ca2fa77763b090c6ad4ba4a5d46a6a8b76698
>
> Diff: https://reviews.apache.org/r/21937/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jay Kreps
>
>
Re: Review Request 21937: Patch for KAFKA-1316
Posted by Neha Narkhede <ne...@gmail.com>.
> On June 6, 2014, 3:18 a.m., Neha Narkhede wrote:
> > clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java, line 69
> > <https://reviews.apache.org/r/21937/diff/3/?file=603202#file603202line69>
> >
> > This API seems unused as well
>
> Jay Kreps wrote:
> Those two apis are meant to be used together
>
> val r = new ProduceResponse()
> r.addResponse(...)
Ah, I see. Gotcha!
> On June 6, 2014, 3:18 a.m., Neha Narkhede wrote:
> > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java, line 58
> > <https://reviews.apache.org/r/21937/diff/3/?file=603197#file603197line58>
> >
> > Now we will end up with potentially two Senders - one for the producer's state machine and another for the consumer's state machine. Can we rename this one to sth like ProduerSender?
>
> Guozhang Wang wrote:
> I think this is fine: this Sender is under kafka.clients.producer.internal, the consumer Sender will be under consumer.internal and there will not likely classes we need to import both.
>
> Jay Kreps wrote:
> I think the naming shouldn't conflict, though perhaps that name could be improved. The sender is a thread that sends (AKA produces data). The consumer won't have such a thread and it won't be sending (it will be receiving!).
I agree. My comment was a bit dumb, I think. Please ignore it.
- Neha
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21937/#review44850
-----------------------------------------------------------
On June 7, 2014, 6:20 p.m., Jay Kreps wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21937/
> -----------------------------------------------------------
>
> (Updated June 7, 2014, 6:20 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1316
> https://issues.apache.org/jira/browse/KAFKA-1316
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1316 Refactor a reusable NetworkClient interface out of Sender.
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/ClientRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ClientResponse.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ConnectionState.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/InFlightRequests.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/KafkaClient.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/NetworkClient.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/NodeConnectionState.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java d15562a968d9e4b08f26b8d30986881adfe29e31
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java f47a46159e9cf4161997470c6d4459cefcd0bd82
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java 5ededccef7cb8e161822e9e5fc25e70a51dbe581
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java 5ee5455a84446966e1e3c54d37d5f541cd30c8a3
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 9b1f5653548ba90defdae43940a5554066770b0a
> clients/src/main/java/org/apache/kafka/common/metrics/Measurable.java 7c2e33c3bf3b7acb6236553b9f6fd00881ffdf75
> clients/src/main/java/org/apache/kafka/common/metrics/stats/Avg.java c9963cb68ee227be7e6618a82b53714364f11531
> clients/src/main/java/org/apache/kafka/common/metrics/stats/Count.java efcd61bca983788c6132da5b23be847205f9421d
> clients/src/main/java/org/apache/kafka/common/metrics/stats/Max.java c492c3811cfab21bbd0dcb36904acdaf6a17b63f
> clients/src/main/java/org/apache/kafka/common/metrics/stats/Min.java bd0919cbec5bac9329e695480b57ad721eacfe51
> clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java 8300978eaef59d19be70453b84d8b9bfb589f4eb
> clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.java 4b481a54a4d532bc4ed50d30d986342eec6ef5e4
> clients/src/main/java/org/apache/kafka/common/metrics/stats/SampledStat.java 0d4056fbc6ec98e1ef7f467c90340cb7c8711090
> clients/src/main/java/org/apache/kafka/common/metrics/stats/Total.java 53dd3d5c2a0743111898d0483dc05f0cbb6306fc
> clients/src/main/java/org/apache/kafka/common/network/ByteBufferSend.java 63504246320473a2eb5c300ffe73c433f7946a25
> clients/src/main/java/org/apache/kafka/common/network/Selector.java 3e358985ed72a894a71d683acc7460695d6f2056
> clients/src/main/java/org/apache/kafka/common/network/Send.java d62dff9433f5098ae4662d98524efbaf9f57e62e
> clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java 68b8827f3bdd64580e1b443fce5b8c63152dd94a
> clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 428968cd38a7b12991f87868bf759926ff7e594e
> clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java 6fa4a58f5f9792776a647e8f682d7faadc0d1556
> clients/src/test/java/org/apache/kafka/clients/MockClient.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/clients/NetworkClientTest.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java c4072ae90fb58101a67f83054fbe0b8349e71c2e
> clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java 3ef692ca3e9fb83868a9da6f30c0705bb3d0aed2
> clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java e4e0a049740d317efa9a5a43e1006e4fc49d817e
> clients/src/test/java/org/apache/kafka/common/utils/MockTime.java cda8e644587aad9a8c9c96c222edc1ba27de1fb0
> core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala cd4ca2fa77763b090c6ad4ba4a5d46a6a8b76698
> core/src/test/scala/unit/kafka/utils/TestUtils.scala 4da0f2c245f75ff0dcab4ecf0af085ab9f8da1bb
>
> Diff: https://reviews.apache.org/r/21937/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jay Kreps
>
>
Re: Review Request 21937: Patch for KAFKA-1316
Posted by Jay Kreps <bo...@gmail.com>.
> On June 6, 2014, 3:18 a.m., Neha Narkhede wrote:
> > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java, line 58
> > <https://reviews.apache.org/r/21937/diff/3/?file=603197#file603197line58>
> >
> > Now we will end up with potentially two Senders - one for the producer's state machine and another for the consumer's state machine. Can we rename this one to sth like ProduerSender?
>
> Guozhang Wang wrote:
> I think this is fine: this Sender is under kafka.clients.producer.internal, the consumer Sender will be under consumer.internal and there will not likely classes we need to import both.
I think the naming shouldn't conflict, though perhaps that name could be improved. The sender is a thread that sends (AKA produces data). The consumer won't have such a thread and it won't be sending (it will be receiving!).
> On June 6, 2014, 3:18 a.m., Neha Narkhede wrote:
> > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java, line 179
> > <https://reviews.apache.org/r/21937/diff/3/?file=603197#file603197line179>
> >
> > Should we call client.close() here to close the selector?
Nice catch!
> On June 6, 2014, 3:18 a.m., Neha Narkhede wrote:
> > clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java, line 47
> > <https://reviews.apache.org/r/21937/diff/3/?file=603202#file603202line47>
> >
> > This API is unused. Can we remove it?
We can but eventually each of these requests needs to go from struct=>object and back to handle both client and server...
> On June 6, 2014, 3:18 a.m., Neha Narkhede wrote:
> > clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java, line 69
> > <https://reviews.apache.org/r/21937/diff/3/?file=603202#file603202line69>
> >
> > This API seems unused as well
Those two apis are meant to be used together
val r = new ProduceResponse()
r.addResponse(...)
- Jay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21937/#review44850
-----------------------------------------------------------
On June 3, 2014, 9:33 p.m., Jay Kreps wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21937/
> -----------------------------------------------------------
>
> (Updated June 3, 2014, 9:33 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1316
> https://issues.apache.org/jira/browse/KAFKA-1316
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1316 Refactor a reusable NetworkClient interface out of Sender.
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/ClientRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ClientResponse.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ConnectionState.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/InFlightRequests.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/KafkaClient.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/NetworkClient.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/NodeConnectionState.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java d15562a968d9e4b08f26b8d30986881adfe29e31
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 9b1f5653548ba90defdae43940a5554066770b0a
> clients/src/main/java/org/apache/kafka/common/network/Selector.java 3e358985ed72a894a71d683acc7460695d6f2056
> clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java 68b8827f3bdd64580e1b443fce5b8c63152dd94a
> clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 428968cd38a7b12991f87868bf759926ff7e594e
> clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java 6fa4a58f5f9792776a647e8f682d7faadc0d1556
> clients/src/test/java/org/apache/kafka/clients/MockClient.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/clients/NetworkClientTest.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java c4072ae90fb58101a67f83054fbe0b8349e71c2e
> clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java 3ef692ca3e9fb83868a9da6f30c0705bb3d0aed2
> clients/src/test/java/org/apache/kafka/common/utils/MockTime.java cda8e644587aad9a8c9c96c222edc1ba27de1fb0
> core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala cd4ca2fa77763b090c6ad4ba4a5d46a6a8b76698
>
> Diff: https://reviews.apache.org/r/21937/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jay Kreps
>
>
Re: Review Request 21937: Patch for KAFKA-1316
Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21937/#review44850
-----------------------------------------------------------
Good to see this patch. Looks great, I have few minor review comments. Also, at least one test seems to hang (kafka.api.ProducerFailureHandlingTest)
clients/src/main/java/org/apache/kafka/clients/ClientRequest.java
<https://reviews.apache.org/r/21937/#comment79474>
LICENSE header is missing here and in all the new files
clients/src/main/java/org/apache/kafka/clients/ClientRequest.java
<https://reviews.apache.org/r/21937/#comment79472>
typo: milliseonds
clients/src/main/java/org/apache/kafka/clients/ClientResponse.java
<https://reviews.apache.org/r/21937/#comment79473>
Could you add javadoc for the constructor and also for the class?
clients/src/main/java/org/apache/kafka/clients/KafkaClient.java
<https://reviews.apache.org/r/21937/#comment79439>
Will be good to specify what happens with requests that are sent on non ready connections
clients/src/main/java/org/apache/kafka/clients/NetworkClient.java
<https://reviews.apache.org/r/21937/#comment79440>
nodes->node's
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java
<https://reviews.apache.org/r/21937/#comment79478>
Now we will end up with potentially two Senders - one for the producer's state machine and another for the consumer's state machine. Can we rename this one to sth like ProduerSender?
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java
<https://reviews.apache.org/r/21937/#comment79436>
Should we call client.close() here to close the selector?
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java
<https://reviews.apache.org/r/21937/#comment79435>
nowMs->now
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java
<https://reviews.apache.org/r/21937/#comment79479>
nowMs->now
clients/src/main/java/org/apache/kafka/common/network/Selector.java
<https://reviews.apache.org/r/21937/#comment79433>
This import is unused. Probably we can just revert the changes to this file
clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java
<https://reviews.apache.org/r/21937/#comment79430>
This API is unused. Can we remove it?
clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java
<https://reviews.apache.org/r/21937/#comment79431>
This API seems unused as well
- Neha Narkhede
On June 3, 2014, 9:33 p.m., Jay Kreps wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21937/
> -----------------------------------------------------------
>
> (Updated June 3, 2014, 9:33 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1316
> https://issues.apache.org/jira/browse/KAFKA-1316
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1316 Refactor a reusable NetworkClient interface out of Sender.
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/ClientRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ClientResponse.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ConnectionState.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/InFlightRequests.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/KafkaClient.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/NetworkClient.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/NodeConnectionState.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java d15562a968d9e4b08f26b8d30986881adfe29e31
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 9b1f5653548ba90defdae43940a5554066770b0a
> clients/src/main/java/org/apache/kafka/common/network/Selector.java 3e358985ed72a894a71d683acc7460695d6f2056
> clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java 68b8827f3bdd64580e1b443fce5b8c63152dd94a
> clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 428968cd38a7b12991f87868bf759926ff7e594e
> clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java 6fa4a58f5f9792776a647e8f682d7faadc0d1556
> clients/src/test/java/org/apache/kafka/clients/MockClient.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/clients/NetworkClientTest.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java c4072ae90fb58101a67f83054fbe0b8349e71c2e
> clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java 3ef692ca3e9fb83868a9da6f30c0705bb3d0aed2
> clients/src/test/java/org/apache/kafka/common/utils/MockTime.java cda8e644587aad9a8c9c96c222edc1ba27de1fb0
> core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala cd4ca2fa77763b090c6ad4ba4a5d46a6a8b76698
>
> Diff: https://reviews.apache.org/r/21937/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jay Kreps
>
>
Re: Review Request 21937: Patch for KAFKA-1316
Posted by Jay Kreps <bo...@gmail.com>.
> On June 6, 2014, 4:06 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java, lines 1-16
> > <https://reviews.apache.org/r/21937/diff/2/?file=603027#file603027line1>
> >
> > Let's use the same license header in the HEADER file.
The header comment needs to be formatted with the same width as all other comments to use automatic comment formatting which is extremely useful.
- Jay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21937/#review44663
-----------------------------------------------------------
On June 7, 2014, 6:20 p.m., Jay Kreps wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21937/
> -----------------------------------------------------------
>
> (Updated June 7, 2014, 6:20 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1316
> https://issues.apache.org/jira/browse/KAFKA-1316
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1316 Refactor a reusable NetworkClient interface out of Sender.
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/ClientRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ClientResponse.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ConnectionState.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/InFlightRequests.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/KafkaClient.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/NetworkClient.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/NodeConnectionState.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java d15562a968d9e4b08f26b8d30986881adfe29e31
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java f47a46159e9cf4161997470c6d4459cefcd0bd82
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java 5ededccef7cb8e161822e9e5fc25e70a51dbe581
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java 5ee5455a84446966e1e3c54d37d5f541cd30c8a3
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 9b1f5653548ba90defdae43940a5554066770b0a
> clients/src/main/java/org/apache/kafka/common/metrics/Measurable.java 7c2e33c3bf3b7acb6236553b9f6fd00881ffdf75
> clients/src/main/java/org/apache/kafka/common/metrics/stats/Avg.java c9963cb68ee227be7e6618a82b53714364f11531
> clients/src/main/java/org/apache/kafka/common/metrics/stats/Count.java efcd61bca983788c6132da5b23be847205f9421d
> clients/src/main/java/org/apache/kafka/common/metrics/stats/Max.java c492c3811cfab21bbd0dcb36904acdaf6a17b63f
> clients/src/main/java/org/apache/kafka/common/metrics/stats/Min.java bd0919cbec5bac9329e695480b57ad721eacfe51
> clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java 8300978eaef59d19be70453b84d8b9bfb589f4eb
> clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.java 4b481a54a4d532bc4ed50d30d986342eec6ef5e4
> clients/src/main/java/org/apache/kafka/common/metrics/stats/SampledStat.java 0d4056fbc6ec98e1ef7f467c90340cb7c8711090
> clients/src/main/java/org/apache/kafka/common/metrics/stats/Total.java 53dd3d5c2a0743111898d0483dc05f0cbb6306fc
> clients/src/main/java/org/apache/kafka/common/network/ByteBufferSend.java 63504246320473a2eb5c300ffe73c433f7946a25
> clients/src/main/java/org/apache/kafka/common/network/Selector.java 3e358985ed72a894a71d683acc7460695d6f2056
> clients/src/main/java/org/apache/kafka/common/network/Send.java d62dff9433f5098ae4662d98524efbaf9f57e62e
> clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java 68b8827f3bdd64580e1b443fce5b8c63152dd94a
> clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 428968cd38a7b12991f87868bf759926ff7e594e
> clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java 6fa4a58f5f9792776a647e8f682d7faadc0d1556
> clients/src/test/java/org/apache/kafka/clients/MockClient.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/clients/NetworkClientTest.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java c4072ae90fb58101a67f83054fbe0b8349e71c2e
> clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java 3ef692ca3e9fb83868a9da6f30c0705bb3d0aed2
> clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java e4e0a049740d317efa9a5a43e1006e4fc49d817e
> clients/src/test/java/org/apache/kafka/common/utils/MockTime.java cda8e644587aad9a8c9c96c222edc1ba27de1fb0
> core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala cd4ca2fa77763b090c6ad4ba4a5d46a6a8b76698
> core/src/test/scala/unit/kafka/utils/TestUtils.scala 4da0f2c245f75ff0dcab4ecf0af085ab9f8da1bb
>
> Diff: https://reviews.apache.org/r/21937/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jay Kreps
>
>
Re: Review Request 21937: Patch for KAFKA-1316
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21937/#review44663
-----------------------------------------------------------
Thanks for the patch. Could we write short description in the comments on how the refactored producer works? For example, (1) what NetworkClient manages; (2) how metadata is refreshed, etc?
clients/src/main/java/org/apache/kafka/clients/ClientRequest.java
<https://reviews.apache.org/r/21937/#comment79112>
Apache header.
clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java
<https://reviews.apache.org/r/21937/#comment79470>
Let's use the same license header in the HEADER file.
clients/src/main/java/org/apache/kafka/clients/NetworkClient.java
<https://reviews.apache.org/r/21937/#comment79489>
Couldn't find where this is called in the producer or sender.
- Jun Rao
On June 3, 2014, 9:33 p.m., Jay Kreps wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21937/
> -----------------------------------------------------------
>
> (Updated June 3, 2014, 9:33 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1316
> https://issues.apache.org/jira/browse/KAFKA-1316
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1316 Refactor a reusable NetworkClient interface out of Sender.
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/ClientRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ClientResponse.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ConnectionState.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/InFlightRequests.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/KafkaClient.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/NetworkClient.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/NodeConnectionState.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java d15562a968d9e4b08f26b8d30986881adfe29e31
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 9b1f5653548ba90defdae43940a5554066770b0a
> clients/src/main/java/org/apache/kafka/common/network/Selector.java 3e358985ed72a894a71d683acc7460695d6f2056
> clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java 68b8827f3bdd64580e1b443fce5b8c63152dd94a
> clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 428968cd38a7b12991f87868bf759926ff7e594e
> clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java 6fa4a58f5f9792776a647e8f682d7faadc0d1556
> clients/src/test/java/org/apache/kafka/clients/MockClient.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/clients/NetworkClientTest.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java c4072ae90fb58101a67f83054fbe0b8349e71c2e
> clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java 3ef692ca3e9fb83868a9da6f30c0705bb3d0aed2
> clients/src/test/java/org/apache/kafka/common/utils/MockTime.java cda8e644587aad9a8c9c96c222edc1ba27de1fb0
> core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala cd4ca2fa77763b090c6ad4ba4a5d46a6a8b76698
>
> Diff: https://reviews.apache.org/r/21937/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jay Kreps
>
>
Re: Review Request 21937: Patch for KAFKA-1316
Posted by Jay Kreps <bo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21937/
-----------------------------------------------------------
(Updated June 7, 2014, 6:20 p.m.)
Review request for kafka.
Bugs: KAFKA-1316
https://issues.apache.org/jira/browse/KAFKA-1316
Repository: kafka
Description
-------
KAFKA-1316 Refactor a reusable NetworkClient interface out of Sender.
Diffs (updated)
-----
clients/src/main/java/org/apache/kafka/clients/ClientRequest.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/ClientResponse.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/ConnectionState.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/InFlightRequests.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/KafkaClient.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/NetworkClient.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/NodeConnectionState.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java d15562a968d9e4b08f26b8d30986881adfe29e31
clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java f47a46159e9cf4161997470c6d4459cefcd0bd82
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java 5ededccef7cb8e161822e9e5fc25e70a51dbe581
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java 5ee5455a84446966e1e3c54d37d5f541cd30c8a3
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 9b1f5653548ba90defdae43940a5554066770b0a
clients/src/main/java/org/apache/kafka/common/metrics/Measurable.java 7c2e33c3bf3b7acb6236553b9f6fd00881ffdf75
clients/src/main/java/org/apache/kafka/common/metrics/stats/Avg.java c9963cb68ee227be7e6618a82b53714364f11531
clients/src/main/java/org/apache/kafka/common/metrics/stats/Count.java efcd61bca983788c6132da5b23be847205f9421d
clients/src/main/java/org/apache/kafka/common/metrics/stats/Max.java c492c3811cfab21bbd0dcb36904acdaf6a17b63f
clients/src/main/java/org/apache/kafka/common/metrics/stats/Min.java bd0919cbec5bac9329e695480b57ad721eacfe51
clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java 8300978eaef59d19be70453b84d8b9bfb589f4eb
clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.java 4b481a54a4d532bc4ed50d30d986342eec6ef5e4
clients/src/main/java/org/apache/kafka/common/metrics/stats/SampledStat.java 0d4056fbc6ec98e1ef7f467c90340cb7c8711090
clients/src/main/java/org/apache/kafka/common/metrics/stats/Total.java 53dd3d5c2a0743111898d0483dc05f0cbb6306fc
clients/src/main/java/org/apache/kafka/common/network/ByteBufferSend.java 63504246320473a2eb5c300ffe73c433f7946a25
clients/src/main/java/org/apache/kafka/common/network/Selector.java 3e358985ed72a894a71d683acc7460695d6f2056
clients/src/main/java/org/apache/kafka/common/network/Send.java d62dff9433f5098ae4662d98524efbaf9f57e62e
clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java 68b8827f3bdd64580e1b443fce5b8c63152dd94a
clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 428968cd38a7b12991f87868bf759926ff7e594e
clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java PRE-CREATION
clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java 6fa4a58f5f9792776a647e8f682d7faadc0d1556
clients/src/test/java/org/apache/kafka/clients/MockClient.java PRE-CREATION
clients/src/test/java/org/apache/kafka/clients/NetworkClientTest.java PRE-CREATION
clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java c4072ae90fb58101a67f83054fbe0b8349e71c2e
clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java 3ef692ca3e9fb83868a9da6f30c0705bb3d0aed2
clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java e4e0a049740d317efa9a5a43e1006e4fc49d817e
clients/src/test/java/org/apache/kafka/common/utils/MockTime.java cda8e644587aad9a8c9c96c222edc1ba27de1fb0
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala cd4ca2fa77763b090c6ad4ba4a5d46a6a8b76698
core/src/test/scala/unit/kafka/utils/TestUtils.scala 4da0f2c245f75ff0dcab4ecf0af085ab9f8da1bb
Diff: https://reviews.apache.org/r/21937/diff/
Testing
-------
Thanks,
Jay Kreps
Re: Review Request 21937: Patch for KAFKA-1316
Posted by Jay Kreps <bo...@gmail.com>.
> On June 6, 2014, 5:14 p.m., Guozhang Wang wrote:
> > clients/src/main/java/org/apache/kafka/clients/NetworkClient.java, line 93
> > <https://reviews.apache.org/r/21937/diff/3/?file=603194#file603194line93>
> >
> > Maybe rename to getReady()?
It isn't a getter. It actually is something more like
boolean becomeReady(node)
I think of the usage in the same way you would say "Ready the lifeboats!".
Alternately we could break this up and have it be something like
if(client.isReady(node))
client.poll(...)
else
client.makeReady(node)
I guess the only argument for the current approach is that there isn't much point in checking readiness unless you also intend to make it ready...
> On June 6, 2014, 5:14 p.m., Guozhang Wang wrote:
> > clients/src/main/java/org/apache/kafka/clients/NetworkClient.java, line 158
> > <https://reviews.apache.org/r/21937/diff/3/?file=603194#file603194line158>
> >
> > numInFlightRequests()?
Changed to inFlightRequestCount(), good?
> On June 6, 2014, 5:14 p.m., Guozhang Wang wrote:
> > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java, line 490
> > <https://reviews.apache.org/r/21937/diff/3/?file=603197#file603197line490>
> >
> > I made now to nowMs since we have both MilliSec and NanoSec in the producer used in metrics, and I felt it is better to indicate that which values are for Milli and which are for Nano.
Yeah that makes sense. We have always had the convention on the server-side as well as other projects of using now to be the current time in milliseconds. So I had kept that and differentiated nowNs. I think it is kind of nice to have a short three letter variable for the current time. Either way is probably fine but it should probably be consistent...
- Jay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21937/#review44921
-----------------------------------------------------------
On June 3, 2014, 9:33 p.m., Jay Kreps wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21937/
> -----------------------------------------------------------
>
> (Updated June 3, 2014, 9:33 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1316
> https://issues.apache.org/jira/browse/KAFKA-1316
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1316 Refactor a reusable NetworkClient interface out of Sender.
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/ClientRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ClientResponse.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ConnectionState.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/InFlightRequests.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/KafkaClient.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/NetworkClient.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/NodeConnectionState.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java d15562a968d9e4b08f26b8d30986881adfe29e31
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 9b1f5653548ba90defdae43940a5554066770b0a
> clients/src/main/java/org/apache/kafka/common/network/Selector.java 3e358985ed72a894a71d683acc7460695d6f2056
> clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java 68b8827f3bdd64580e1b443fce5b8c63152dd94a
> clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 428968cd38a7b12991f87868bf759926ff7e594e
> clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java 6fa4a58f5f9792776a647e8f682d7faadc0d1556
> clients/src/test/java/org/apache/kafka/clients/MockClient.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/clients/NetworkClientTest.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java c4072ae90fb58101a67f83054fbe0b8349e71c2e
> clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java 3ef692ca3e9fb83868a9da6f30c0705bb3d0aed2
> clients/src/test/java/org/apache/kafka/common/utils/MockTime.java cda8e644587aad9a8c9c96c222edc1ba27de1fb0
> core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala cd4ca2fa77763b090c6ad4ba4a5d46a6a8b76698
>
> Diff: https://reviews.apache.org/r/21937/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jay Kreps
>
>
Re: Review Request 21937: Patch for KAFKA-1316
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21937/#review44921
-----------------------------------------------------------
clients/src/main/java/org/apache/kafka/clients/InFlightRequests.java
<https://reviews.apache.org/r/21937/#comment79509>
Rename ByteBufferSend.complete() to completed()?
clients/src/main/java/org/apache/kafka/clients/NetworkClient.java
<https://reviews.apache.org/r/21937/#comment79507>
Maybe rename to getReady()?
clients/src/main/java/org/apache/kafka/clients/NetworkClient.java
<https://reviews.apache.org/r/21937/#comment79508>
numInFlightRequests()?
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java
<https://reviews.apache.org/r/21937/#comment79516>
I made now to nowMs since we have both MilliSec and NanoSec in the producer used in metrics, and I felt it is better to indicate that which values are for Milli and which are for Nano.
- Guozhang Wang
On June 3, 2014, 9:33 p.m., Jay Kreps wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21937/
> -----------------------------------------------------------
>
> (Updated June 3, 2014, 9:33 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1316
> https://issues.apache.org/jira/browse/KAFKA-1316
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1316 Refactor a reusable NetworkClient interface out of Sender.
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/ClientRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ClientResponse.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/ConnectionState.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/InFlightRequests.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/KafkaClient.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/NetworkClient.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/NodeConnectionState.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java d15562a968d9e4b08f26b8d30986881adfe29e31
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 9b1f5653548ba90defdae43940a5554066770b0a
> clients/src/main/java/org/apache/kafka/common/network/Selector.java 3e358985ed72a894a71d683acc7460695d6f2056
> clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java 68b8827f3bdd64580e1b443fce5b8c63152dd94a
> clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 428968cd38a7b12991f87868bf759926ff7e594e
> clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java 6fa4a58f5f9792776a647e8f682d7faadc0d1556
> clients/src/test/java/org/apache/kafka/clients/MockClient.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/clients/NetworkClientTest.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java c4072ae90fb58101a67f83054fbe0b8349e71c2e
> clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java 3ef692ca3e9fb83868a9da6f30c0705bb3d0aed2
> clients/src/test/java/org/apache/kafka/common/utils/MockTime.java cda8e644587aad9a8c9c96c222edc1ba27de1fb0
> core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala cd4ca2fa77763b090c6ad4ba4a5d46a6a8b76698
>
> Diff: https://reviews.apache.org/r/21937/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jay Kreps
>
>
Re: Review Request 21937: Patch for KAFKA-1316
Posted by Jay Kreps <bo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21937/
-----------------------------------------------------------
(Updated June 3, 2014, 9:33 p.m.)
Review request for kafka.
Bugs: KAFKA-1316
https://issues.apache.org/jira/browse/KAFKA-1316
Repository: kafka
Description
-------
KAFKA-1316 Refactor a reusable NetworkClient interface out of Sender.
Diffs (updated)
-----
clients/src/main/java/org/apache/kafka/clients/ClientRequest.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/ClientResponse.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/ConnectionState.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/InFlightRequests.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/KafkaClient.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/NetworkClient.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/NodeConnectionState.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java d15562a968d9e4b08f26b8d30986881adfe29e31
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 9b1f5653548ba90defdae43940a5554066770b0a
clients/src/main/java/org/apache/kafka/common/network/Selector.java 3e358985ed72a894a71d683acc7460695d6f2056
clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java 68b8827f3bdd64580e1b443fce5b8c63152dd94a
clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 428968cd38a7b12991f87868bf759926ff7e594e
clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java PRE-CREATION
clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java 6fa4a58f5f9792776a647e8f682d7faadc0d1556
clients/src/test/java/org/apache/kafka/clients/MockClient.java PRE-CREATION
clients/src/test/java/org/apache/kafka/clients/NetworkClientTest.java PRE-CREATION
clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java c4072ae90fb58101a67f83054fbe0b8349e71c2e
clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java 3ef692ca3e9fb83868a9da6f30c0705bb3d0aed2
clients/src/test/java/org/apache/kafka/common/utils/MockTime.java cda8e644587aad9a8c9c96c222edc1ba27de1fb0
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala cd4ca2fa77763b090c6ad4ba4a5d46a6a8b76698
Diff: https://reviews.apache.org/r/21937/diff/
Testing
-------
Thanks,
Jay Kreps
Re: Review Request 21937: Patch for KAFKA-1316
Posted by Jay Kreps <bo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21937/
-----------------------------------------------------------
(Updated June 3, 2014, 6:15 p.m.)
Review request for kafka.
Summary (updated)
-----------------
Patch for KAFKA-1316
Bugs: KAFKA-1316
https://issues.apache.org/jira/browse/KAFKA-1316
Repository: kafka
Description (updated)
-------
KAFKA-1316 Refactor a reusable NetworkClient interface out of Sender.
Diffs (updated)
-----
clients/src/main/java/org/apache/kafka/clients/ClientRequest.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/ClientResponse.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/ConnectionState.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/InFlightRequests.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/KafkaClient.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/NetworkClient.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/NodeConnectionState.java PRE-CREATION
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java d15562a968d9e4b08f26b8d30986881adfe29e31
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 9b1f5653548ba90defdae43940a5554066770b0a
clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java 68b8827f3bdd64580e1b443fce5b8c63152dd94a
clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 428968cd38a7b12991f87868bf759926ff7e594e
clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java PRE-CREATION
clients/src/test/java/org/apache/kafka/clients/MockClient.java PRE-CREATION
clients/src/test/java/org/apache/kafka/clients/NetworkClientTest.java PRE-CREATION
clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java c4072ae90fb58101a67f83054fbe0b8349e71c2e
clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java 3ef692ca3e9fb83868a9da6f30c0705bb3d0aed2
clients/src/test/java/org/apache/kafka/common/utils/MockTime.java cda8e644587aad9a8c9c96c222edc1ba27de1fb0
Diff: https://reviews.apache.org/r/21937/diff/
Testing
-------
Thanks,
Jay Kreps