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/11 20:24:12 UTC
Re: Review Request 17918: Patch for KAFKA-1233
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17918/
-----------------------------------------------------------
(Updated Feb. 11, 2014, 7:24 p.m.)
Review request for kafka.
Summary (updated)
-----------------
Patch for KAFKA-1233
Bugs: KAFKA-1233
https://issues.apache.org/jira/browse/KAFKA-1233
Repository: kafka
Description
-------
Three test cases included in Part I:
Incorporated Jay's comments.
1. Send API, with null key/value/partitionId/topic tested, check returned offset. Currently passed.
2. All messages drained before close, also check that messages with the same key go to the same partition. Currently passed.
3. The default round robin partitioner, and the specified partition id is respected. Currently broken with ack = -1. Filed KAFKA-1255.
4. Send with auto-topic-creation. Currently passed.
Diffs
-----
build.gradle 858d297b9e8bf8a2bca54c4817f9ca2affd0d3f2
core/src/test/resources/log4j.properties d7d03ea1b13299d4ee41b9907720d014c81faf66
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala PRE-CREATION
project/Build.scala ddcfc4176e68933377590e095c07283083249f4a
Diff: https://reviews.apache.org/r/17918/diff/
Testing
-------
Thanks,
Guozhang Wang
Re: Review Request 17918: Patch for KAFKA-1233
Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17918/#review34230
-----------------------------------------------------------
Ship it!
Ship It!
- Neha Narkhede
On Feb. 11, 2014, 8:28 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17918/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2014, 8:28 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1233
> https://issues.apache.org/jira/browse/KAFKA-1233
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Four test cases included in Part I:
>
> Incorporated Jay's comments.
>
> Rebased on new commits.
>
> 1. Send API, with null key/value/partitionId/topic tested, check returned offset. Currently passed.
>
> 2. All messages drained before close, also check that messages with the same key go to the same partition. Currently broken after some new commits ...
>
> 3. The default round robin partitioner, and the specified partition id is respected. Currently broken with ack = -1. Filed KAFKA-1255.
>
> 4. Send with auto-topic-creation. Currently passed.
>
>
> Diffs
> -----
>
> build.gradle d9d6e6a91e6f37c0247107b0655f58a2db9688cd
> core/src/test/scala/integration/kafka/api/ProducerSendTest.scala PRE-CREATION
> core/src/test/scala/unit/kafka/utils/TestUtils.scala 500eeca2f95d901536b1363b8c4b485c4893179f
>
> Diff: https://reviews.apache.org/r/17918/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 17918: Patch for KAFKA-1233
Posted by Guozhang Wang <gu...@linkedin.com>.
> On Feb. 11, 2014, 11:27 p.m., Jay Kreps wrote:
> > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, line 283
> > <https://reviews.apache.org/r/17918/diff/3/?file=482894#file482894line283>
> >
> > It would be good to add a test where we send a message, stop one of the servers, send another message (should fail) then send another message (should succeed going to the new leader).
This is happening in Part II, which is another JIRA :)
> On Feb. 11, 2014, 11:27 p.m., Jay Kreps wrote:
> > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, line 202
> > <https://reviews.apache.org/r/17918/diff/3/?file=482894#file482894line202>
> >
> > This test is very complicated. I think it validates two things:
> > 1. Partitioning works as expected
> > 2. The contents of the log is what we expect
> >
> > I recommend we remove (1). It is better tested in the direct test of the Partitioner.
Actually it does not intend to check 2), it checks the content of the messages just to validate the behavior of the default partitioner (round-robin).
- Guozhang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17918/#review34241
-----------------------------------------------------------
On Feb. 11, 2014, 8:28 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17918/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2014, 8:28 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1233
> https://issues.apache.org/jira/browse/KAFKA-1233
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Four test cases included in Part I:
>
> Incorporated Jay's comments.
>
> Rebased on new commits.
>
> 1. Send API, with null key/value/partitionId/topic tested, check returned offset. Currently passed.
>
> 2. All messages drained before close, also check that messages with the same key go to the same partition. Currently broken after some new commits ...
>
> 3. The default round robin partitioner, and the specified partition id is respected. Currently broken with ack = -1. Filed KAFKA-1255.
>
> 4. Send with auto-topic-creation. Currently passed.
>
>
> Diffs
> -----
>
> build.gradle d9d6e6a91e6f37c0247107b0655f58a2db9688cd
> core/src/test/scala/integration/kafka/api/ProducerSendTest.scala PRE-CREATION
> core/src/test/scala/unit/kafka/utils/TestUtils.scala 500eeca2f95d901536b1363b8c4b485c4893179f
>
> Diff: https://reviews.apache.org/r/17918/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 17918: Patch for KAFKA-1233
Posted by Jay Kreps <bo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17918/#review34241
-----------------------------------------------------------
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
<https://reviews.apache.org/r/17918/#comment64267>
This test is very complicated. I think it validates two things:
1. Partitioning works as expected
2. The contents of the log is what we expect
I recommend we remove (1). It is better tested in the direct test of the Partitioner.
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
<https://reviews.apache.org/r/17918/#comment64270>
It would be good to add a test where we send a message, stop one of the servers, send another message (should fail) then send another message (should succeed going to the new leader).
- Jay Kreps
On Feb. 11, 2014, 8:28 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17918/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2014, 8:28 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1233
> https://issues.apache.org/jira/browse/KAFKA-1233
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Four test cases included in Part I:
>
> Incorporated Jay's comments.
>
> Rebased on new commits.
>
> 1. Send API, with null key/value/partitionId/topic tested, check returned offset. Currently passed.
>
> 2. All messages drained before close, also check that messages with the same key go to the same partition. Currently broken after some new commits ...
>
> 3. The default round robin partitioner, and the specified partition id is respected. Currently broken with ack = -1. Filed KAFKA-1255.
>
> 4. Send with auto-topic-creation. Currently passed.
>
>
> Diffs
> -----
>
> build.gradle d9d6e6a91e6f37c0247107b0655f58a2db9688cd
> core/src/test/scala/integration/kafka/api/ProducerSendTest.scala PRE-CREATION
> core/src/test/scala/unit/kafka/utils/TestUtils.scala 500eeca2f95d901536b1363b8c4b485c4893179f
>
> Diff: https://reviews.apache.org/r/17918/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 17918: Patch for KAFKA-1233
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17918/
-----------------------------------------------------------
(Updated Feb. 11, 2014, 8:28 p.m.)
Review request for kafka.
Bugs: KAFKA-1233
https://issues.apache.org/jira/browse/KAFKA-1233
Repository: kafka
Description (updated)
-------
Four test cases included in Part I:
Incorporated Jay's comments.
Rebased on new commits.
1. Send API, with null key/value/partitionId/topic tested, check returned offset. Currently passed.
2. All messages drained before close, also check that messages with the same key go to the same partition. Currently broken after some new commits ...
3. The default round robin partitioner, and the specified partition id is respected. Currently broken with ack = -1. Filed KAFKA-1255.
4. Send with auto-topic-creation. Currently passed.
Diffs
-----
build.gradle d9d6e6a91e6f37c0247107b0655f58a2db9688cd
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala PRE-CREATION
core/src/test/scala/unit/kafka/utils/TestUtils.scala 500eeca2f95d901536b1363b8c4b485c4893179f
Diff: https://reviews.apache.org/r/17918/diff/
Testing
-------
Thanks,
Guozhang Wang
Re: Review Request 17918: Patch for KAFKA-1233
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17918/
-----------------------------------------------------------
(Updated Feb. 11, 2014, 8:27 p.m.)
Review request for kafka.
Bugs: KAFKA-1233
https://issues.apache.org/jira/browse/KAFKA-1233
Repository: kafka
Description (updated)
-------
KAFKA-1233.v4
KAFKA-1233.v3
KAFKA-1235.v2
Dummy 3
Dummy
KAFKA-1233.v1
Diffs (updated)
-----
build.gradle d9d6e6a91e6f37c0247107b0655f58a2db9688cd
core/src/test/scala/integration/kafka/api/ProducerSendTest.scala PRE-CREATION
core/src/test/scala/unit/kafka/utils/TestUtils.scala 500eeca2f95d901536b1363b8c4b485c4893179f
Diff: https://reviews.apache.org/r/17918/diff/
Testing
-------
Thanks,
Guozhang Wang