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 02:43:03 UTC

Re: Review Request 17918: Patch for KAFKA-1232

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

(Updated Feb. 11, 2014, 1:42 a.m.)


Review request for kafka.


Summary (updated)
-----------------

Patch for KAFKA-1232


Bugs: KAFKA-1232 and KAFKA-1233
    https://issues.apache.org/jira/browse/KAFKA-1232
    https://issues.apache.org/jira/browse/KAFKA-1233


Repository: kafka


Description (updated)
-------

KAFKA-1233.v3


KAFKA-1235.v2


Dummy 3


Dummy


KAFKA-1233.v1


Diffs (updated)
-----

  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>.

> On Feb. 11, 2014, 7:18 p.m., Neha Narkhede wrote:
> > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, line 109
> > <https://reviews.apache.org/r/17918/diff/2/?file=482288#file482288line109>
> >
> >     These 3 statements are repeated in many tests. Can we create a helper method to create topic successfully?
> 
> Guozhang Wang wrote:
>     Have thought about that, the reason I did not do so is that if I do this may goes to testUtils. But I think this is actually fine now, will fix.


> On Feb. 11, 2014, 7:18 p.m., Neha Narkhede wrote:
> > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, line 211
> > <https://reviews.apache.org/r/17918/diff/2/?file=482288#file482288line211>
> >
> >     Shouldn't the rest of the tests have acks set to -1 as well?
> 
> Guozhang Wang wrote:
>     ack = -1 is only required when I check the produced message content using a consumer.

Makes sense.


> On Feb. 11, 2014, 7:18 p.m., Neha Narkhede wrote:
> > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, line 219
> > <https://reviews.apache.org/r/17918/diff/2/?file=482288#file482288line219>
> >
> >     why only wait until leader for partition 0 is elected? We should also probably wait until leader for partition 1 is elected so the test is not time dependent.
> 
> Guozhang Wang wrote:
>     For this test I only checks one partition, 0, which I think is sufficient. Not quite sure what you mean by "time dependent"?

I think I misread the test and thought that it used partition 1 without waiting for leader to be election on partition 1. But in your latest patch, the helper method take care of this properly.


- Neha


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


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, 7:18 p.m., Neha Narkhede wrote:
> > core/src/test/resources/log4j.properties, line 15
> > <https://reviews.apache.org/r/17918/diff/2/?file=482287#file482287line15>
> >
> >     this is probably included in the patch by accident right?

Yeah, will fix.


> On Feb. 11, 2014, 7:18 p.m., Neha Narkhede wrote:
> > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, line 58
> > <https://reviews.apache.org/r/17918/diff/2/?file=482288#file482288line58>
> >
> >     Naming it topic1 implies there will be topic2 as well. Can we stick to just topic?

ack


> On Feb. 11, 2014, 7:18 p.m., Neha Narkhede wrote:
> > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, line 86
> > <https://reviews.apache.org/r/17918/diff/2/?file=482288#file482288line86>
> >
> >     we probably don't want to print anything from unit tests.

These prints will only show up if the unit test runs with -i, otherwise they will not. I keep them here in case some exceptions are caught for running the callbacks.


> On Feb. 11, 2014, 7:18 p.m., Neha Narkhede wrote:
> > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, line 180
> > <https://reviews.apache.org/r/17918/diff/2/?file=482288#file482288line180>
> >
> >     What is the advantage of sending 10 times numRecords instead of just numRecords?

This is just a magic number, I think numRecords should be sufficient, will fix


> On Feb. 11, 2014, 7:18 p.m., Neha Narkhede wrote:
> > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, line 109
> > <https://reviews.apache.org/r/17918/diff/2/?file=482288#file482288line109>
> >
> >     These 3 statements are repeated in many tests. Can we create a helper method to create topic successfully?

Have thought about that, the reason I did not do so is that if I do this may goes to testUtils. But I think this is actually fine now, will fix.


> On Feb. 11, 2014, 7:18 p.m., Neha Narkhede wrote:
> > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, line 211
> > <https://reviews.apache.org/r/17918/diff/2/?file=482288#file482288line211>
> >
> >     Shouldn't the rest of the tests have acks set to -1 as well?

ack = -1 is only required when I check the produced message content using a consumer.


> On Feb. 11, 2014, 7:18 p.m., Neha Narkhede wrote:
> > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, line 219
> > <https://reviews.apache.org/r/17918/diff/2/?file=482288#file482288line219>
> >
> >     why only wait until leader for partition 0 is elected? We should also probably wait until leader for partition 1 is elected so the test is not time dependent.

For this test I only checks one partition, 0, which I think is sufficient. Not quite sure what you mean by "time dependent"?


> On Feb. 11, 2014, 7:18 p.m., Neha Narkhede wrote:
> > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, line 242
> > <https://reviews.apache.org/r/17918/diff/2/?file=482288#file482288line242>
> >
> >     The variable records for partition 0 is named record0. This is true for response as well. To be consistent, should this be fetchResponse0 for partition 0?

ack


> On Feb. 11, 2014, 7:18 p.m., Neha Narkhede wrote:
> > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, line 248
> > <https://reviews.apache.org/r/17918/diff/2/?file=482288#file482288line248>
> >
> >     can we pull this logic in a method and do the same check for partition 1 as well?

See above :)


- Guozhang


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


On Feb. 11, 2014, 7:24 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, 7:24 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> 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-1232

Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17918/#review34158
-----------------------------------------------------------



core/src/test/resources/log4j.properties
<https://reviews.apache.org/r/17918/#comment64169>

    this is probably included in the patch by accident right?



core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
<https://reviews.apache.org/r/17918/#comment64231>

    Naming it topic1 implies there will be topic2 as well. Can we stick to just topic?



core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
<https://reviews.apache.org/r/17918/#comment64225>

    we probably don't want to print anything from unit tests. 



core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
<https://reviews.apache.org/r/17918/#comment64227>

    These 3 statements are repeated in many tests. Can we create a helper method to create topic successfully?



core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
<https://reviews.apache.org/r/17918/#comment64233>

    What is the advantage of sending 10 times numRecords instead of just numRecords?



core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
<https://reviews.apache.org/r/17918/#comment64234>

    testClose->testSendToPartition



core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
<https://reviews.apache.org/r/17918/#comment64235>

    Shouldn't the rest of the tests have acks set to -1 as well?



core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
<https://reviews.apache.org/r/17918/#comment64236>

    why only wait until leader for partition 0 is elected? We should also probably wait until leader for partition 1 is elected so the test is not time dependent.



core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
<https://reviews.apache.org/r/17918/#comment64238>

    and check they has numRecords+1 messages with -> and check they have numRecords+1 messages



core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
<https://reviews.apache.org/r/17918/#comment64240>

    The variable records for partition 0 is named record0. This is true for response as well. To be consistent, should this be fetchResponse0 for partition 0?



core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
<https://reviews.apache.org/r/17918/#comment64242>

    can we pull this logic in a method and do the same check for partition 1 as well?



core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
<https://reviews.apache.org/r/17918/#comment64241>

    messageSet0 doesn't exist in this patch.



core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
<https://reviews.apache.org/r/17918/#comment64244>

    testCreateTopic -> testAutoCreateTopic



project/Build.scala
<https://reviews.apache.org/r/17918/#comment64170>

    since we intend to check this in on trunk, we don't need to touch the sbt files. Just depend on gradle to build/test/run tools etc. and it seems to work for each of these actions already.


- Neha Narkhede


On Feb. 11, 2014, 1:51 a.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17918/
> -----------------------------------------------------------
> 
> (Updated Feb. 11, 2014, 1:51 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> 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


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, 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-1232

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, 1:51 a.m.)


Review request for kafka.


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-1232

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, 1:44 a.m.)


Review request for kafka.


Bugs: KAFKA-1232 and KAFKA-1233
    https://issues.apache.org/jira/browse/KAFKA-1232
    https://issues.apache.org/jira/browse/KAFKA-1233


Repository: kafka


Description (updated)
-------

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