You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Tong Li <li...@us.ibm.com> on 2015/02/28 00:16:09 UTC
Review Request 31566: Patch for KAFKA-1988
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31566/
-----------------------------------------------------------
Review request for kafka.
Bugs: KAFKA-1988
https://issues.apache.org/jira/browse/KAFKA-1988
Repository: kafka
Description
-------
KAFKA-1988 org.apache.kafka.common.utils.Utils.abs method returns wrong value for negative numbers
Diffs
-----
clients/src/main/java/org/apache/kafka/clients/producer/internals/Partitioner.java dfb936d8f0d5842ee5c7a7f1584c5ed7463c4cf8
clients/src/main/java/org/apache/kafka/common/utils/Utils.java 69530c187cd1c41b8173b61de6f982aafe65c9fe
clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java 4c2ea34815b63174732d58b699e1a0a9e6ec3b6f
Diff: https://reviews.apache.org/r/31566/diff/
Testing
-------
Thanks,
Tong Li
Re: Review Request 31566: Patch for KAFKA-1988
Posted by Jay Kreps <bo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31566/#review74889
-----------------------------------------------------------
Ship it!
This looks good assuming the other patch, centralizes the scala code to all use this single abs function.
- Jay Kreps
On Feb. 27, 2015, 11:16 p.m., Tong Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31566/
> -----------------------------------------------------------
>
> (Updated Feb. 27, 2015, 11:16 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1988
> https://issues.apache.org/jira/browse/KAFKA-1988
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1988 org.apache.kafka.common.utils.Utils.abs method returns wrong value for negative numbers
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Partitioner.java dfb936d8f0d5842ee5c7a7f1584c5ed7463c4cf8
> clients/src/main/java/org/apache/kafka/common/utils/Utils.java 69530c187cd1c41b8173b61de6f982aafe65c9fe
> clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java 4c2ea34815b63174732d58b699e1a0a9e6ec3b6f
>
> Diff: https://reviews.apache.org/r/31566/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Tong Li
>
>
Re: Review Request 31566: Patch for KAFKA-1988
Posted by Guozhang Wang <wa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31566/#review74688
-----------------------------------------------------------
Ship it!
Ship It!
- Guozhang Wang
On Feb. 27, 2015, 11:16 p.m., Tong Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31566/
> -----------------------------------------------------------
>
> (Updated Feb. 27, 2015, 11:16 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1988
> https://issues.apache.org/jira/browse/KAFKA-1988
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1988 org.apache.kafka.common.utils.Utils.abs method returns wrong value for negative numbers
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Partitioner.java dfb936d8f0d5842ee5c7a7f1584c5ed7463c4cf8
> clients/src/main/java/org/apache/kafka/common/utils/Utils.java 69530c187cd1c41b8173b61de6f982aafe65c9fe
> clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java 4c2ea34815b63174732d58b699e1a0a9e6ec3b6f
>
> Diff: https://reviews.apache.org/r/31566/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Tong Li
>
>
Re: Review Request 31566: Patch for KAFKA-1988
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31566/#review74893
-----------------------------------------------------------
Thanks for the patch. A couple of minor comments below.
clients/src/main/java/org/apache/kafka/clients/producer/internals/Partitioner.java
<https://reviews.apache.org/r/31566/#comment121720>
Perhaps we can change the comment to the following.
A cheap way to deterministically convert a number to a positive value. When the input number is negative, the returned positive value is not the absolute value of the input though.
clients/src/main/java/org/apache/kafka/clients/producer/internals/Partitioner.java
<https://reviews.apache.org/r/31566/#comment121721>
We can just say it returns a positive number.
- Jun Rao
On Feb. 27, 2015, 11:16 p.m., Tong Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31566/
> -----------------------------------------------------------
>
> (Updated Feb. 27, 2015, 11:16 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1988
> https://issues.apache.org/jira/browse/KAFKA-1988
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1988 org.apache.kafka.common.utils.Utils.abs method returns wrong value for negative numbers
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Partitioner.java dfb936d8f0d5842ee5c7a7f1584c5ed7463c4cf8
> clients/src/main/java/org/apache/kafka/common/utils/Utils.java 69530c187cd1c41b8173b61de6f982aafe65c9fe
> clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java 4c2ea34815b63174732d58b699e1a0a9e6ec3b6f
>
> Diff: https://reviews.apache.org/r/31566/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Tong Li
>
>
Re: Review Request 31566: Patch for KAFKA-1988
Posted by Tong Li <li...@us.ibm.com>.
> On March 3, 2015, 4:48 p.m., Guozhang Wang wrote:
> > Tong, could you address Jun's last comments before committing?
Yes. absolutely, doing it now. Thanks.
- Tong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31566/#review74973
-----------------------------------------------------------
On Feb. 27, 2015, 11:16 p.m., Tong Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31566/
> -----------------------------------------------------------
>
> (Updated Feb. 27, 2015, 11:16 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1988
> https://issues.apache.org/jira/browse/KAFKA-1988
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1988 org.apache.kafka.common.utils.Utils.abs method returns wrong value for negative numbers
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Partitioner.java dfb936d8f0d5842ee5c7a7f1584c5ed7463c4cf8
> clients/src/main/java/org/apache/kafka/common/utils/Utils.java 69530c187cd1c41b8173b61de6f982aafe65c9fe
> clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java 4c2ea34815b63174732d58b699e1a0a9e6ec3b6f
>
> Diff: https://reviews.apache.org/r/31566/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Tong Li
>
>
Re: Review Request 31566: Patch for KAFKA-1988
Posted by Tong Li <li...@us.ibm.com>.
Guozhang,
Yes, I have rebased and updated the patch set to resolve the couple
comments for code comments. Also removed the trailing spaces (which always
annoying. :-) ), Please see the new patch set here. Sorry for the delay.
https://reviews.apache.org/r/31566/
Thanks.
Tong Li
OpenStack & Kafka Community Development
Building 501/B205
litong01@us.ibm.com
"Guozhang Wang" <no...@reviews.apache.org> wrote on 03/03/2015 11:48:36
AM:
> From: "Guozhang Wang" <wa...@gmail.com>
> To: "kafka" <de...@kafka.apache.org>, "Guozhang Wang"
> <wa...@gmail.com>, Tong Li/Raleigh/IBM@IBMUS
> Date: 03/03/2015 11:49 AM
> Subject: Re: Review Request 31566: Patch for KAFKA-1988
> Sent by: "Guozhang Wang" <no...@reviews.apache.org>
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31566/#review74973
> -----------------------------------------------------------
>
>
> Tong, could you address Jun's last comments before committing?
>
> - Guozhang Wang
>
>
> On Feb. 27, 2015, 11:16 p.m., Tong Li wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/31566/
> > -----------------------------------------------------------
> >
> > (Updated Feb. 27, 2015, 11:16 p.m.)
> >
> >
> > Review request for kafka.
> >
> >
> > Bugs: KAFKA-1988
> > https://issues.apache.org/jira/browse/KAFKA-1988
> >
> >
> > Repository: kafka
> >
> >
> > Description
> > -------
> >
> > KAFKA-1988 org.apache.kafka.common.utils.Utils.abs method returns
> wrong value for negative numbers
> >
> >
> > Diffs
> > -----
> >
> > clients/src/main/java/org/apache/kafka/clients/producer/
> internals/Partitioner.java dfb936d8f0d5842ee5c7a7f1584c5ed7463c4cf8
> > clients/src/main/java/org/apache/kafka/common/utils/Utils.java
> 69530c187cd1c41b8173b61de6f982aafe65c9fe
> > clients/src/test/java/org/apache/kafka/common/utils/
> UtilsTest.java 4c2ea34815b63174732d58b699e1a0a9e6ec3b6f
> >
> > Diff: https://reviews.apache.org/r/31566/diff/
> >
> >
> > Testing
> > -------
> >
> >
> > Thanks,
> >
> > Tong Li
> >
> >
>
Re: Review Request 31566: Patch for KAFKA-1988
Posted by Tong Li <li...@us.ibm.com>.
Yes. It was addressed in the latest patch set. A private method called toPositive was added so that the partition selection is preserved cross this and previous versions.
Thanks
Sent from my iPhone
> On Mar 4, 2015, at 12:49 AM, Guozhang Wang <wa...@gmail.com> wrote:
>
>
> This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31566/
>
> Tong, could you address Jun's last comments before committing?
>
> - Guozhang Wang
>
>
> On February 27th, 2015, 11:16 p.m. UTC, Tong Li wrote:
>
> Review request for kafka.
> By Tong Li.
> Updated Feb. 27, 2015, 11:16 p.m.
>
> Bugs: KAFKA-1988
> Repository: kafka
> Description
>
> KAFKA-1988 org.apache.kafka.common.utils.Utils.abs method returns wrong value for negative numbers
> Diffs
>
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Partitioner.java (dfb936d8f0d5842ee5c7a7f1584c5ed7463c4cf8)
> clients/src/main/java/org/apache/kafka/common/utils/Utils.java (69530c187cd1c41b8173b61de6f982aafe65c9fe)
> clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java (4c2ea34815b63174732d58b699e1a0a9e6ec3b6f)
> View Diff
Re: Review Request 31566: Patch for KAFKA-1988
Posted by Guozhang Wang <wa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31566/#review74973
-----------------------------------------------------------
Tong, could you address Jun's last comments before committing?
- Guozhang Wang
On Feb. 27, 2015, 11:16 p.m., Tong Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31566/
> -----------------------------------------------------------
>
> (Updated Feb. 27, 2015, 11:16 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1988
> https://issues.apache.org/jira/browse/KAFKA-1988
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1988 org.apache.kafka.common.utils.Utils.abs method returns wrong value for negative numbers
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Partitioner.java dfb936d8f0d5842ee5c7a7f1584c5ed7463c4cf8
> clients/src/main/java/org/apache/kafka/common/utils/Utils.java 69530c187cd1c41b8173b61de6f982aafe65c9fe
> clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java 4c2ea34815b63174732d58b699e1a0a9e6ec3b6f
>
> Diff: https://reviews.apache.org/r/31566/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Tong Li
>
>
Re: Review Request 31566: Patch for KAFKA-1988
Posted by Tong Li <li...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31566/
-----------------------------------------------------------
(Updated March 4, 2015, 12:03 a.m.)
Review request for kafka.
Bugs: KAFKA-1988
https://issues.apache.org/jira/browse/KAFKA-1988
Repository: kafka
Description
-------
KAFKA-1988 org.apache.kafka.common.utils.Utils.abs method returns wrong value for negative numbers
Diffs (updated)
-----
clients/src/main/java/org/apache/kafka/clients/producer/internals/Partitioner.java dfb936d8f0d5842ee5c7a7f1584c5ed7463c4cf8
clients/src/main/java/org/apache/kafka/common/utils/Utils.java 69530c187cd1c41b8173b61de6f982aafe65c9fe
clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java 4c2ea34815b63174732d58b699e1a0a9e6ec3b6f
Diff: https://reviews.apache.org/r/31566/diff/
Testing
-------
Thanks,
Tong Li
Re: Review Request 31566: Patch for KAFKA-1988
Posted by Tong Li <li...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31566/
-----------------------------------------------------------
(Updated March 4, 2015, midnight)
Review request for kafka.
Bugs: KAFKA-1988
https://issues.apache.org/jira/browse/KAFKA-1988
Repository: kafka
Description
-------
KAFKA-1988 org.apache.kafka.common.utils.Utils.abs method returns wrong value for negative numbers
Diffs (updated)
-----
clients/src/main/java/org/apache/kafka/clients/producer/internals/Partitioner.java dfb936d8f0d5842ee5c7a7f1584c5ed7463c4cf8
clients/src/main/java/org/apache/kafka/common/utils/Utils.java 69530c187cd1c41b8173b61de6f982aafe65c9fe
clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java 4c2ea34815b63174732d58b699e1a0a9e6ec3b6f
Diff: https://reviews.apache.org/r/31566/diff/
Testing
-------
Thanks,
Tong Li