You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Guozhang Wang <gu...@linkedin.com> on 2014/02/21 00:34:59 UTC
Re: Review Request 18102: New Producer Integration Test: Phase II
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/
-----------------------------------------------------------
(Updated Feb. 20, 2014, 11:34 p.m.)
Review request for kafka.
Summary (updated)
-----------------
New Producer Integration Test: Phase II
Bugs: KAFKA-1260
https://issues.apache.org/jira/browse/KAFKA-1260
Repository: kafka
Description
-------
Rebased on producer re-try.
Incorporated Jay's fix for KAFKA-1266
Fixed some minor issues in producer:
1. Close accumulator on shutdown
2. Return offset for ack=-1
3. Categorize error handling in send call
Fixed the offset issue for ack=-1 on the server side, details in KAFKA-1255.
Four test cases included in Part II:
1. testErrorInResponse.
2. testNoResponse.
3. testSendException.
4. testDeadBroker. Currently blocks..
Diffs
-----
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
clients/src/main/java/org/apache/kafka/common/errors/FatalException.java PRE-CREATION
clients/src/main/java/org/apache/kafka/common/errors/RecordTooLargeException.java 737b7f07b16a0d6549e835c33c9dfad58008a590
clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
clients/src/test/java/org/apache/kafka/test/TestUtils.java 36cfc0fda742eb501af2c2c0330e3f461cf1f40c
core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala PRE-CREATION
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
Diff: https://reviews.apache.org/r/18102/diff/
Testing
-------
Thanks,
Guozhang Wang
Re: Review Request 18102: Fix KAFKA-1260
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/
-----------------------------------------------------------
(Updated Feb. 27, 2014, 5:37 p.m.)
Review request for kafka.
Summary (updated)
-----------------
Fix KAFKA-1260
Bugs: KAFKA-1260
https://issues.apache.org/jira/browse/KAFKA-1260
Repository: kafka
Description (updated)
-------
Add ProducerFailureHandlingTest
Dummy
Dummy
Dummy
Dummy
Diffs (updated)
-----
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java f43da80580f5a347f2e0435943ee2f0f829abc77
clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala PRE-CREATION
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
Diff: https://reviews.apache.org/r/18102/diff/
Testing
-------
Thanks,
Guozhang Wang
Re: Review Request 18102: Integration Test for New Producer: Part II
Posted by Jay Kreps <bo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/#review35593
-----------------------------------------------------------
Ship it!
Ship It!
- Jay Kreps
On Feb. 26, 2014, 9:51 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18102/
> -----------------------------------------------------------
>
> (Updated Feb. 26, 2014, 9:51 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1260
> https://issues.apache.org/jira/browse/KAFKA-1260
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Incorporated Jun's comments.
>
> Incorporated Jay's fix for KAFKA-1266
>
> Fixed some minor issues in producer:
>
> 1. Close accumulator on shutdown
>
> 2. Return offset for ack=-1
>
> 3. Categorize error handling in send call
>
> Fixed the offset issue for ack=-1 on the server side, details in KAFKA-1255.
>
> Four test cases included in Part II:
>
> 1. testErrorInResponse.
>
> 2. testNoResponse.
>
> 3. testSendException.
>
> 4. testDeadBroker. Currently blocks..
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
> clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java f43da80580f5a347f2e0435943ee2f0f829abc77
> clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
> clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
> clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
> clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
> core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
> core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
> core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
> core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
>
> Diff: https://reviews.apache.org/r/18102/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 18102: Integration Test for New Producer: Part II
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/
-----------------------------------------------------------
(Updated Feb. 26, 2014, 9:51 p.m.)
Review request for kafka.
Bugs: KAFKA-1260
https://issues.apache.org/jira/browse/KAFKA-1260
Repository: kafka
Description (updated)
-------
Incorporated Jun's comments.
Incorporated Jay's fix for KAFKA-1266
Fixed some minor issues in producer:
1. Close accumulator on shutdown
2. Return offset for ack=-1
3. Categorize error handling in send call
Fixed the offset issue for ack=-1 on the server side, details in KAFKA-1255.
Four test cases included in Part II:
1. testErrorInResponse.
2. testNoResponse.
3. testSendException.
4. testDeadBroker. Currently blocks..
Diffs
-----
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java f43da80580f5a347f2e0435943ee2f0f829abc77
clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
Diff: https://reviews.apache.org/r/18102/diff/
Testing
-------
Thanks,
Guozhang Wang
Re: Review Request 18102: Integration Test for New Producer: Part II
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/
-----------------------------------------------------------
(Updated Feb. 26, 2014, 9:49 p.m.)
Review request for kafka.
Bugs: KAFKA-1260
https://issues.apache.org/jira/browse/KAFKA-1260
Repository: kafka
Description (updated)
-------
Dummy
Dummy
Dummy
Dummy
Diffs (updated)
-----
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java f43da80580f5a347f2e0435943ee2f0f829abc77
clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
Diff: https://reviews.apache.org/r/18102/diff/
Testing
-------
Thanks,
Guozhang Wang
Re: Review Request 18102: Integration Test for New Producer: Part II
Posted by Guozhang Wang <gu...@linkedin.com>.
> On Feb. 26, 2014, 7:56 p.m., Jun Rao wrote:
> > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, lines 161-162
> > <https://reviews.apache.org/r/18102/diff/9/?file=504699#file504699line161>
> >
> > Shouldn't we add the code to call send after producer.close()?
The second part has been moved to the FailureHandling testsuite, I should have changed the comments.
> On Feb. 26, 2014, 7:56 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/server/KafkaApis.scala, lines 896-897
> > <https://reviews.apache.org/r/18102/diff/9/?file=504698#file504698line896>
> >
> > Could we change this to the following? Then we can use the name instead _1.
> >
> > for ( (topicPartition, responseStatus) <- ...
Done.
- Guozhang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/#review35559
-----------------------------------------------------------
On Feb. 26, 2014, 9:49 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18102/
> -----------------------------------------------------------
>
> (Updated Feb. 26, 2014, 9:49 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1260
> https://issues.apache.org/jira/browse/KAFKA-1260
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Dummy
>
>
> Dummy
>
>
> Dummy
>
>
> Dummy
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
> clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java f43da80580f5a347f2e0435943ee2f0f829abc77
> clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
> clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
> clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
> clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
> core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
> core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
> core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
> core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
>
> Diff: https://reviews.apache.org/r/18102/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 18102: Integration Test for New Producer: Part II
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/#review35559
-----------------------------------------------------------
Ship it!
+1 Just the following minor comments.
core/src/main/scala/kafka/server/KafkaApis.scala
<https://reviews.apache.org/r/18102/#comment66164>
Could we change this to the following? Then we can use the name instead _1.
for ( (topicPartition, responseStatus) <- ...
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
<https://reviews.apache.org/r/18102/#comment66162>
Shouldn't we add the code to call send after producer.close()?
- Jun Rao
On Feb. 26, 2014, 5:48 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18102/
> -----------------------------------------------------------
>
> (Updated Feb. 26, 2014, 5:48 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1260
> https://issues.apache.org/jira/browse/KAFKA-1260
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Incorporated Jun's comments.
>
> Incorporated Jay's fix for KAFKA-1266
>
> Fixed some minor issues in producer:
>
> 1. Close accumulator on shutdown
>
> 2. Return offset for ack=-1
>
> 3. Categorize error handling in send call
>
> Fixed the offset issue for ack=-1 on the server side, details in KAFKA-1255.
>
> Four test cases included in Part II:
>
> 1. testErrorInResponse.
>
> 2. testNoResponse.
>
> 3. testSendException.
>
> 4. testDeadBroker. Currently blocks..
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
> clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java f43da80580f5a347f2e0435943ee2f0f829abc77
> clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
> clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
> clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
> clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
> core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
> core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
> core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
> core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
>
> Diff: https://reviews.apache.org/r/18102/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 18102: Integration Test for New Producer: Part II
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/
-----------------------------------------------------------
(Updated Feb. 26, 2014, 5:48 p.m.)
Review request for kafka.
Bugs: KAFKA-1260
https://issues.apache.org/jira/browse/KAFKA-1260
Repository: kafka
Description (updated)
-------
Incorporated Jun's comments.
Incorporated Jay's fix for KAFKA-1266
Fixed some minor issues in producer:
1. Close accumulator on shutdown
2. Return offset for ack=-1
3. Categorize error handling in send call
Fixed the offset issue for ack=-1 on the server side, details in KAFKA-1255.
Four test cases included in Part II:
1. testErrorInResponse.
2. testNoResponse.
3. testSendException.
4. testDeadBroker. Currently blocks..
Diffs
-----
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java f43da80580f5a347f2e0435943ee2f0f829abc77
clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
Diff: https://reviews.apache.org/r/18102/diff/
Testing
-------
Thanks,
Guozhang Wang
Re: Review Request 18102: Integration Test for New Producer: Part II
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/
-----------------------------------------------------------
(Updated Feb. 26, 2014, 5:47 p.m.)
Review request for kafka.
Bugs: KAFKA-1260
https://issues.apache.org/jira/browse/KAFKA-1260
Repository: kafka
Description (updated)
-------
Dummy
Dummy
Dummy
Diffs (updated)
-----
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java f43da80580f5a347f2e0435943ee2f0f829abc77
clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
Diff: https://reviews.apache.org/r/18102/diff/
Testing
-------
Thanks,
Guozhang Wang
Re: Review Request 18102: Integration Test for New Producer: Part II
Posted by Guozhang Wang <gu...@linkedin.com>.
> On Feb. 26, 2014, 2:47 a.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java, lines 230-231
> > <https://reviews.apache.org/r/18102/diff/8/?file=504071#file504071line230>
> >
> > Not sure we need to wrap InterruptedException.
We need, since the accumulator.append can throw the runtime InterruptedException, which we do not want to throw outside KafkaProducer but instead wrap it with a checked KafkaException.
- Guozhang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/#review35508
-----------------------------------------------------------
On Feb. 26, 2014, 1:46 a.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18102/
> -----------------------------------------------------------
>
> (Updated Feb. 26, 2014, 1:46 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1260
> https://issues.apache.org/jira/browse/KAFKA-1260
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Incorporated Jay's fix for KAFKA-1266
>
> Fixed some minor issues in producer:
>
> 1. Close accumulator on shutdown
>
> 2. Return offset for ack=-1
>
> 3. Categorize error handling in send call
>
> Fixed the offset issue for ack=-1 on the server side, details in KAFKA-1255.
>
> Four test cases included in Part II:
>
> 1. testErrorInResponse.
>
> 2. testNoResponse.
>
> 3. testSendException.
>
> 4. testDeadBroker. Currently blocks..
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
> clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java f43da80580f5a347f2e0435943ee2f0f829abc77
> clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
> clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
> clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
> clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
> core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
> core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
> core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
> core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
>
> Diff: https://reviews.apache.org/r/18102/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 18102: Integration Test for New Producer: Part II
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/#review35508
-----------------------------------------------------------
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
<https://reviews.apache.org/r/18102/#comment66079>
remove println.
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
<https://reviews.apache.org/r/18102/#comment66081>
Not sure we need to wrap InterruptedException.
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
<https://reviews.apache.org/r/18102/#comment66082>
Can we just let it pass through instead of rethrowing?
clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java
<https://reviews.apache.org/r/18102/#comment66083>
Unused import.
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java
<https://reviews.apache.org/r/18102/#comment66084>
Do we need this comment? If so, let's add { } to the if clause.
core/src/main/scala/kafka/server/KafkaApis.scala
<https://reviews.apache.org/r/18102/#comment66085>
We don't need acksPending to be in the constructor.
- Jun Rao
On Feb. 26, 2014, 1:46 a.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18102/
> -----------------------------------------------------------
>
> (Updated Feb. 26, 2014, 1:46 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1260
> https://issues.apache.org/jira/browse/KAFKA-1260
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Incorporated Jay's fix for KAFKA-1266
>
> Fixed some minor issues in producer:
>
> 1. Close accumulator on shutdown
>
> 2. Return offset for ack=-1
>
> 3. Categorize error handling in send call
>
> Fixed the offset issue for ack=-1 on the server side, details in KAFKA-1255.
>
> Four test cases included in Part II:
>
> 1. testErrorInResponse.
>
> 2. testNoResponse.
>
> 3. testSendException.
>
> 4. testDeadBroker. Currently blocks..
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
> clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java f43da80580f5a347f2e0435943ee2f0f829abc77
> clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
> clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
> clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
> clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
> core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
> core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
> core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
> core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
>
> Diff: https://reviews.apache.org/r/18102/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 18102: Integration Test for New Producer: Part II
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/
-----------------------------------------------------------
(Updated Feb. 26, 2014, 1:46 a.m.)
Review request for kafka.
Bugs: KAFKA-1260
https://issues.apache.org/jira/browse/KAFKA-1260
Repository: kafka
Description (updated)
-------
Incorporated Jay's fix for KAFKA-1266
Fixed some minor issues in producer:
1. Close accumulator on shutdown
2. Return offset for ack=-1
3. Categorize error handling in send call
Fixed the offset issue for ack=-1 on the server side, details in KAFKA-1255.
Four test cases included in Part II:
1. testErrorInResponse.
2. testNoResponse.
3. testSendException.
4. testDeadBroker. Currently blocks..
Diffs
-----
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java f43da80580f5a347f2e0435943ee2f0f829abc77
clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
Diff: https://reviews.apache.org/r/18102/diff/
Testing
-------
Thanks,
Guozhang Wang
Re: Review Request 18102: Integration Test for New Producer: Part II
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/
-----------------------------------------------------------
(Updated Feb. 26, 2014, 1:46 a.m.)
Review request for kafka.
Bugs: KAFKA-1260
https://issues.apache.org/jira/browse/KAFKA-1260
Repository: kafka
Description (updated)
-------
Dummy
Dummy
Diffs (updated)
-----
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java f43da80580f5a347f2e0435943ee2f0f829abc77
clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
Diff: https://reviews.apache.org/r/18102/diff/
Testing
-------
Thanks,
Guozhang Wang
Re: Review Request 18102: Integration Test for New Producer: Part II
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/
-----------------------------------------------------------
(Updated Feb. 26, 2014, 12:59 a.m.)
Review request for kafka.
Bugs: KAFKA-1260
https://issues.apache.org/jira/browse/KAFKA-1260
Repository: kafka
Description (updated)
-------
Incorporated Jay's fix for KAFKA-1266
Fixed some minor issues in producer:
1. Close accumulator on shutdown
2. Return offset for ack=-1
3. Categorize error handling in send call
Fixed the offset issue for ack=-1 on the server side, details in KAFKA-1255.
Four test cases included in Part II:
1. testErrorInResponse.
2. testNoResponse.
3. testSendException.
4. testDeadBroker. Currently blocks..
Diffs
-----
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
Diff: https://reviews.apache.org/r/18102/diff/
Testing
-------
Thanks,
Guozhang Wang
Re: Review Request 18102: Integration Test for New Producer: Part II
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/
-----------------------------------------------------------
(Updated Feb. 26, 2014, 12:58 a.m.)
Review request for kafka.
Summary (updated)
-----------------
Integration Test for New Producer: Part II
Bugs: KAFKA-1260
https://issues.apache.org/jira/browse/KAFKA-1260
Repository: kafka
Description (updated)
-------
Dummy
Diffs (updated)
-----
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
Diff: https://reviews.apache.org/r/18102/diff/
Testing
-------
Thanks,
Guozhang Wang
Re: Review Request 18102: New Producer Integration Test Phase II
Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/#review35468
-----------------------------------------------------------
clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java
<https://reviews.apache.org/r/18102/#comment66026>
it seems this constructor now is only used by the MockProducer. It is slightly awkward to have both these constructors. With the new one, can we make this one private and change all usages to point to the new constructor instead?
core/src/main/scala/kafka/api/ProducerResponse.scala
<https://reviews.apache.org/r/18102/#comment65999>
Can this be DelayedProducerResponseStatus along the lines of ProducerResponseStatus. Also I'm not quite sure I understand the changes related to DelayedProduceResponseStatus. Let's chat about this.
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment66030>
I'm trying to understand the scope of this test. It says failure handling and I'm not sure where you want to put the tests for testing the various ack modes in the presence of broker failures. One test I can think of is to have the producer send data using ack = -1 and validate the offsets returned and actual data to verify that there is no data loss.
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment66005>
not-exist-topic -> non existing topics?
cased -> caused
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment66006>
cased -> caused
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment66007>
that can be thrown
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment66012>
cased -> caused
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment66014>
Instead of too large, can we say sending a message >= bufferSize will cause the producer to throw a BufferExhaustedException?
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment66013>
ditto
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment66021>
is this TODO still required?
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment66023>
can we remove this?
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment66024>
ditto here
- Neha Narkhede
On Feb. 20, 2014, 11:45 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18102/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 11:45 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1260
> https://issues.apache.org/jira/browse/KAFKA-1260
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Rebased on producer re-try.
>
> Incorporated Jay's fix for KAFKA-1266
>
> Fixed some minor issues in producer:
>
> 1. Close accumulator on shutdown
>
> 2. Return offset for ack=-1
>
> 3. Categorize error handling in send call
>
> Fixed the offset issue for ack=-1 on the server side, details in KAFKA-1255.
>
> Four test cases included in Part II:
>
> 1. testErrorInResponse.
>
> 2. testNoResponse.
>
> 3. testSendException.
>
> 4. testDeadBroker. Currently blocks..
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
> clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
> clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
> clients/src/main/java/org/apache/kafka/common/errors/FatalException.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/errors/RecordTooLargeException.java 737b7f07b16a0d6549e835c33c9dfad58008a590
> clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
> clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
> core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
> core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
> core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala PRE-CREATION
> core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
> core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
>
> Diff: https://reviews.apache.org/r/18102/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 18102: New Producer Integration Test Phase II
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/#review35418
-----------------------------------------------------------
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java
<https://reviews.apache.org/r/18102/#comment65901>
Not removing it :) Just make them in one line.
- Guozhang Wang
On Feb. 20, 2014, 11:45 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18102/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 11:45 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1260
> https://issues.apache.org/jira/browse/KAFKA-1260
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Rebased on producer re-try.
>
> Incorporated Jay's fix for KAFKA-1266
>
> Fixed some minor issues in producer:
>
> 1. Close accumulator on shutdown
>
> 2. Return offset for ack=-1
>
> 3. Categorize error handling in send call
>
> Fixed the offset issue for ack=-1 on the server side, details in KAFKA-1255.
>
> Four test cases included in Part II:
>
> 1. testErrorInResponse.
>
> 2. testNoResponse.
>
> 3. testSendException.
>
> 4. testDeadBroker. Currently blocks..
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
> clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
> clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
> clients/src/main/java/org/apache/kafka/common/errors/FatalException.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/errors/RecordTooLargeException.java 737b7f07b16a0d6549e835c33c9dfad58008a590
> clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
> clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
> core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
> core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
> core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala PRE-CREATION
> core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
> core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
>
> Diff: https://reviews.apache.org/r/18102/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 18102: New Producer Integration Test Phase II
Posted by Guozhang Wang <gu...@linkedin.com>.
> On Feb. 25, 2014, 4:39 a.m., Jay Kreps wrote:
> > core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala, line 163
> > <https://reviews.apache.org/r/18102/diff/6/?file=499586#file499586line163>
> >
> > It might be simpler to do something like
> > if(producer1 != null) producer1.close()
> > in the tearDown() method of the test to avoid the nested try/catch.
Originally I did this, but later realized that for some tests close() will block. I think probably I will just made it to null for now.
- Guozhang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/#review35370
-----------------------------------------------------------
On Feb. 20, 2014, 11:45 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18102/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 11:45 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1260
> https://issues.apache.org/jira/browse/KAFKA-1260
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Rebased on producer re-try.
>
> Incorporated Jay's fix for KAFKA-1266
>
> Fixed some minor issues in producer:
>
> 1. Close accumulator on shutdown
>
> 2. Return offset for ack=-1
>
> 3. Categorize error handling in send call
>
> Fixed the offset issue for ack=-1 on the server side, details in KAFKA-1255.
>
> Four test cases included in Part II:
>
> 1. testErrorInResponse.
>
> 2. testNoResponse.
>
> 3. testSendException.
>
> 4. testDeadBroker. Currently blocks..
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
> clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
> clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
> clients/src/main/java/org/apache/kafka/common/errors/FatalException.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/errors/RecordTooLargeException.java 737b7f07b16a0d6549e835c33c9dfad58008a590
> clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
> clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
> core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
> core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
> core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala PRE-CREATION
> core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
> core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
>
> Diff: https://reviews.apache.org/r/18102/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 18102: New Producer Integration Test Phase II
Posted by Guozhang Wang <gu...@linkedin.com>.
> On Feb. 25, 2014, 4:39 a.m., Jay Kreps wrote:
> > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java, line 222
> > <https://reviews.apache.org/r/18102/diff/6/?file=499574#file499574line222>
> >
> > Why are we using instanceof instead of catching the individual exception types?
> >
> > Also why are we wrapping RuntimeExceptions in KafkaException?
> >
> > Basically I don't think I follow the rationale here.
Originally the send call does not throw any exceptions, all errors are wrapped in the future returned. We decided to only wrap ApiExceptions but throw others in send call, so I think we either 1) declare the runtime throwable exceptions in the function signature, or 2) wrap them in checked KafkaException.
> On Feb. 25, 2014, 4:39 a.m., Jay Kreps wrote:
> > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java, line 267
> > <https://reviews.apache.org/r/18102/diff/6/?file=499574#file499574line267>
> >
> > You seem to have removed the close on the accumulator is this an accident?
Not an accident :) I realized that accumulator.close() is called in initiateClose already.
> On Feb. 25, 2014, 4:39 a.m., Jay Kreps wrote:
> > clients/src/main/java/org/apache/kafka/common/errors/FatalException.java, line 1
> > <https://reviews.apache.org/r/18102/diff/6/?file=499580#file499580line1>
> >
> > The current logic is that all exceptions that don't implement RetriableException are fatal so I don't think we need this. Originally I had thought to use RetriableException to mark an idempotent exception but the current usage is actually for any non-fatal exception. The rationale was (1) We shouldn't end up accidentally retrying if we through an unexpected error (out of memory or null pointer or something) and (2) when we have the idempotent producer retry will always be idempotent so modeling that in the exception hiearchy is premature.
Ack
- Guozhang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/#review35370
-----------------------------------------------------------
On Feb. 20, 2014, 11:45 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18102/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 11:45 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1260
> https://issues.apache.org/jira/browse/KAFKA-1260
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Rebased on producer re-try.
>
> Incorporated Jay's fix for KAFKA-1266
>
> Fixed some minor issues in producer:
>
> 1. Close accumulator on shutdown
>
> 2. Return offset for ack=-1
>
> 3. Categorize error handling in send call
>
> Fixed the offset issue for ack=-1 on the server side, details in KAFKA-1255.
>
> Four test cases included in Part II:
>
> 1. testErrorInResponse.
>
> 2. testNoResponse.
>
> 3. testSendException.
>
> 4. testDeadBroker. Currently blocks..
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
> clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
> clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
> clients/src/main/java/org/apache/kafka/common/errors/FatalException.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/errors/RecordTooLargeException.java 737b7f07b16a0d6549e835c33c9dfad58008a590
> clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
> clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
> core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
> core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
> core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala PRE-CREATION
> core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
> core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
>
> Diff: https://reviews.apache.org/r/18102/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 18102: New Producer Integration Test Phase II
Posted by Jay Kreps <bo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/#review35370
-----------------------------------------------------------
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
<https://reviews.apache.org/r/18102/#comment65847>
Why are we using instanceof instead of catching the individual exception types?
Also why are we wrapping RuntimeExceptions in KafkaException?
Basically I don't think I follow the rationale here.
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
<https://reviews.apache.org/r/18102/#comment65848>
You seem to have removed the close on the accumulator is this an accident?
clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java
<https://reviews.apache.org/r/18102/#comment65849>
Hmm so we need to make a new atomic reference now? This seems much much worse then just recreating the RecordMetadata instance, no?
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java
<https://reviews.apache.org/r/18102/#comment65850>
I'm not sold that this refactor actually made things better. Now everything goes through future.get which is needlessly synchronized, right?
clients/src/main/java/org/apache/kafka/common/errors/FatalException.java
<https://reviews.apache.org/r/18102/#comment65851>
The current logic is that all exceptions that don't implement RetriableException are fatal so I don't think we need this. Originally I had thought to use RetriableException to mark an idempotent exception but the current usage is actually for any non-fatal exception. The rationale was (1) We shouldn't end up accidentally retrying if we through an unexpected error (out of memory or null pointer or something) and (2) when we have the idempotent producer retry will always be idempotent so modeling that in the exception hiearchy is premature.
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java
<https://reviews.apache.org/r/18102/#comment65852>
How come you are removing this? Is it not an exception that can occur on the server?
core/src/main/scala/kafka/server/KafkaApis.scala
<https://reviews.apache.org/r/18102/#comment65854>
Not sure I grok the changes here, may need to chat about it...
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment65855>
This package needs to match the directory. Same with the other integration test. You have it in kafka.api.* but declare kafka.test.*
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment65862>
Does it make sense to combine these all? wouldn't it be easier to just have 4 test cases?
That is usually easier to reason about and understand when they fail.
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment65856>
Can you make a method for this. It can be a local method for now.
def makeProducer(brokers, acks, fetchTimeout) or whatever...
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment65858>
There is no need to catch the exception and fail, an uncaught exception will always fail the test.
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment65859>
If you expect a particular exception use the scalatest utility intercept. It looks like this:
intercept[ExecutionException]{
// do something that should throw that exception
}
Much easier to read.
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment65860>
Ditto
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment65861>
Ditto
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment65857>
It might be simpler to do something like
if(producer1 != null) producer1.close()
in the tearDown() method of the test to avoid the nested try/catch.
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/18102/#comment65863>
Would it make sense to break these into separate test methods?
- Jay Kreps
On Feb. 20, 2014, 11:45 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18102/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 11:45 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1260
> https://issues.apache.org/jira/browse/KAFKA-1260
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Rebased on producer re-try.
>
> Incorporated Jay's fix for KAFKA-1266
>
> Fixed some minor issues in producer:
>
> 1. Close accumulator on shutdown
>
> 2. Return offset for ack=-1
>
> 3. Categorize error handling in send call
>
> Fixed the offset issue for ack=-1 on the server side, details in KAFKA-1255.
>
> Four test cases included in Part II:
>
> 1. testErrorInResponse.
>
> 2. testNoResponse.
>
> 3. testSendException.
>
> 4. testDeadBroker. Currently blocks..
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
> clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
> clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
> clients/src/main/java/org/apache/kafka/common/errors/FatalException.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/errors/RecordTooLargeException.java 737b7f07b16a0d6549e835c33c9dfad58008a590
> clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
> clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
> core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
> core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
> core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala PRE-CREATION
> core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
> core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
>
> Diff: https://reviews.apache.org/r/18102/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 18102: New Producer Integration Test Phase II
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/
-----------------------------------------------------------
(Updated Feb. 20, 2014, 11:45 p.m.)
Review request for kafka.
Summary (updated)
-----------------
New Producer Integration Test Phase II
Bugs: KAFKA-1260
https://issues.apache.org/jira/browse/KAFKA-1260
Repository: kafka
Description (updated)
-------
Rebased on producer re-try.
Incorporated Jay's fix for KAFKA-1266
Fixed some minor issues in producer:
1. Close accumulator on shutdown
2. Return offset for ack=-1
3. Categorize error handling in send call
Fixed the offset issue for ack=-1 on the server side, details in KAFKA-1255.
Four test cases included in Part II:
1. testErrorInResponse.
2. testNoResponse.
3. testSendException.
4. testDeadBroker. Currently blocks..
Diffs
-----
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
clients/src/main/java/org/apache/kafka/common/errors/FatalException.java PRE-CREATION
clients/src/main/java/org/apache/kafka/common/errors/RecordTooLargeException.java 737b7f07b16a0d6549e835c33c9dfad58008a590
clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala PRE-CREATION
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
Diff: https://reviews.apache.org/r/18102/diff/
Testing
-------
Thanks,
Guozhang Wang
Re: Review Request 18102: Fix KAFKA-1235
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18102/
-----------------------------------------------------------
(Updated Feb. 20, 2014, 11:45 p.m.)
Review request for kafka.
Summary (updated)
-----------------
Fix KAFKA-1235
Bugs: KAFKA-1260
https://issues.apache.org/jira/browse/KAFKA-1260
Repository: kafka
Description (updated)
-------
Dummy
Dummy
Dummy 3
FIXED!
Dummy 2
Dummy
KAFKA-1260.v1
KAFKA-1260.Dummy
Diffs (updated)
-----
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java e4bc97279585818860487a39a93b6481742b91db
clients/src/main/java/org/apache/kafka/clients/producer/RecordMetadata.java 8c776980ef1f5167fb02dda36f6ad2385bd62bbd
clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java 22d4c79bc06fb77af30ab930855632c402c5a72e
clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 62613a3e29a7e5ffb1cc56d267793fef72857fc6
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java eb16f6d236e07b16654623606294a051531b5f58
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd
clients/src/main/java/org/apache/kafka/common/errors/FatalException.java PRE-CREATION
clients/src/main/java/org/apache/kafka/common/errors/RecordTooLargeException.java 737b7f07b16a0d6549e835c33c9dfad58008a590
clients/src/main/java/org/apache/kafka/common/network/Selector.java f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java f88992a0cafd9639b4c5823aa80de1daf8b0eadd
core/src/main/scala/kafka/api/ProducerResponse.scala 06261b9136399e48a8cb75f1c88a4cbfd1217de4
core/src/main/scala/kafka/server/KafkaApis.scala ae2df2014a08aaa95b5eaa430684cfdb79d4f55e
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala PRE-CREATION
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 34baa8c6c7a15bb4aa93c286604f0eb7b19cd58e
core/src/test/scala/unit/kafka/utils/TestUtils.scala 1c7a450651978e121376c226987b0d835f395a2a
Diff: https://reviews.apache.org/r/18102/diff/
Testing
-------
Thanks,
Guozhang Wang