You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Gwen Shapira <gs...@cloudera.com> on 2015/03/01 08:58:57 UTC
Review Request 31591: Patch for KAFKA-1992
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/
-----------------------------------------------------------
Review request for kafka.
Bugs: KAFKA-1992
https://issues.apache.org/jira/browse/KAFKA-1992
Repository: kafka
Description
-------
remove unnecessary requiredAcks parameter and clean up few comments
Diffs
-----
core/src/main/scala/kafka/cluster/Partition.scala c4bf48a801007ebe7497077d2018d6dffe1677d4
core/src/main/scala/kafka/server/DelayedProduce.scala 4d763bf05efb24a394662721292fc54d32467969
Diff: https://reviews.apache.org/r/31591/diff/
Testing
-------
Thanks,
Gwen Shapira
Re: Review Request 31591: Patch for KAFKA-1992
Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/
-----------------------------------------------------------
(Updated March 6, 2015, 9:37 p.m.)
Review request for kafka.
Bugs: KAFKA-1992
https://issues.apache.org/jira/browse/KAFKA-1992
Repository: kafka
Description (updated)
-------
add logging per Jiangjie Qin comment
revert unintentional changes to log4j
merge with trunk
few small fixes suggested by Jun
tiny typo
formatting
Diffs (updated)
-----
core/src/main/scala/kafka/cluster/Partition.scala c4bf48a801007ebe7497077d2018d6dffe1677d4
core/src/main/scala/kafka/server/DelayedProduce.scala 4d763bf05efb24a394662721292fc54d32467969
Diff: https://reviews.apache.org/r/31591/diff/
Testing
-------
Thanks,
Gwen Shapira
Re: Review Request 31591: Patch for KAFKA-1992
Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/
-----------------------------------------------------------
(Updated March 6, 2015, 9:36 p.m.)
Review request for kafka.
Bugs: KAFKA-1992
https://issues.apache.org/jira/browse/KAFKA-1992
Repository: kafka
Description (updated)
-------
add logging per Jiangjie Qin comment
revert unintentional changes to log4j
merge with trunk
few small fixes suggested by Jun
tiny typo
Diffs (updated)
-----
core/src/main/scala/kafka/cluster/Partition.scala c4bf48a801007ebe7497077d2018d6dffe1677d4
core/src/main/scala/kafka/server/DelayedProduce.scala 4d763bf05efb24a394662721292fc54d32467969
Diff: https://reviews.apache.org/r/31591/diff/
Testing
-------
Thanks,
Gwen Shapira
Re: Review Request 31591: Patch for KAFKA-1992
Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/
-----------------------------------------------------------
(Updated March 6, 2015, 9:34 p.m.)
Review request for kafka.
Bugs: KAFKA-1992
https://issues.apache.org/jira/browse/KAFKA-1992
Repository: kafka
Description (updated)
-------
add logging per Jiangjie Qin comment
revert unintentional changes to log4j
merge with trunk
few small fixes suggested by Jun
Diffs (updated)
-----
core/src/main/scala/kafka/cluster/Partition.scala c4bf48a801007ebe7497077d2018d6dffe1677d4
core/src/main/scala/kafka/server/DelayedProduce.scala 4d763bf05efb24a394662721292fc54d32467969
Diff: https://reviews.apache.org/r/31591/diff/
Testing
-------
Thanks,
Gwen Shapira
Re: Review Request 31591: Patch for KAFKA-1992
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/#review75120
-----------------------------------------------------------
Thanks for the patch. A few minor comments below.
core/src/main/scala/kafka/cluster/Partition.scala
<https://reviews.apache.org/r/31591/#comment122063>
The last %s should be %d.
core/src/main/scala/kafka/cluster/Partition.scala
<https://reviews.apache.org/r/31591/#comment122065>
Our current coding convention is not to wrap single line statement in {}.
core/src/main/scala/kafka/cluster/Partition.scala
<https://reviews.apache.org/r/31591/#comment122068>
Space should be added after comma.
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
<https://reviews.apache.org/r/31591/#comment122070>
This change is already committed.
- Jun Rao
On March 4, 2015, 1:17 a.m., Gwen Shapira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31591/
> -----------------------------------------------------------
>
> (Updated March 4, 2015, 1:17 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1992
> https://issues.apache.org/jira/browse/KAFKA-1992
>
>
> Repository: kafka
>
>
> Description
> -------
>
> add logging per Jiangjie Qin comment
>
>
> revert unintentional changes to log4j
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/cluster/Partition.scala c4bf48a801007ebe7497077d2018d6dffe1677d4
> core/src/main/scala/kafka/server/DelayedProduce.scala 4d763bf05efb24a394662721292fc54d32467969
> core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala ba48a636dd0b0ed06646d56bb36aa3d43228604f
>
> Diff: https://reviews.apache.org/r/31591/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gwen Shapira
>
>
Re: Review Request 31591: Patch for KAFKA-1992
Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/
-----------------------------------------------------------
(Updated March 4, 2015, 1:17 a.m.)
Review request for kafka.
Bugs: KAFKA-1992
https://issues.apache.org/jira/browse/KAFKA-1992
Repository: kafka
Description (updated)
-------
add logging per Jiangjie Qin comment
revert unintentional changes to log4j
Diffs (updated)
-----
core/src/main/scala/kafka/cluster/Partition.scala c4bf48a801007ebe7497077d2018d6dffe1677d4
core/src/main/scala/kafka/server/DelayedProduce.scala 4d763bf05efb24a394662721292fc54d32467969
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala ba48a636dd0b0ed06646d56bb36aa3d43228604f
Diff: https://reviews.apache.org/r/31591/diff/
Testing
-------
Thanks,
Gwen Shapira
Re: Review Request 31591: Patch for KAFKA-1992
Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/
-----------------------------------------------------------
(Updated March 3, 2015, 10:16 p.m.)
Review request for kafka.
Bugs: KAFKA-1992
https://issues.apache.org/jira/browse/KAFKA-1992
Repository: kafka
Description (updated)
-------
add logging per Jiangjie Qin comment
Diffs (updated)
-----
core/src/main/scala/kafka/cluster/Partition.scala c4bf48a801007ebe7497077d2018d6dffe1677d4
core/src/main/scala/kafka/server/DelayedProduce.scala 4d763bf05efb24a394662721292fc54d32467969
core/src/test/resources/log4j.properties 1b7d5d8f7d5fae7d272849715714781cad05d77b
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala ba48a636dd0b0ed06646d56bb36aa3d43228604f
Diff: https://reviews.apache.org/r/31591/diff/
Testing
-------
Thanks,
Gwen Shapira
Re: Review Request 31591: Patch for KAFKA-1992
Posted by Jiangjie Qin <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/#review74827
-----------------------------------------------------------
Ship it!
LGTM, just a minor comment.
core/src/main/scala/kafka/cluster/Partition.scala
<https://reviews.apache.org/r/31591/#comment121610>
This part seems now serving only logging purpose. If that is the case, can we make it even clearer. For example, print all the acked replicas instead of just a number.
- Jiangjie Qin
On March 1, 2015, 7:58 a.m., Gwen Shapira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31591/
> -----------------------------------------------------------
>
> (Updated March 1, 2015, 7:58 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1992
> https://issues.apache.org/jira/browse/KAFKA-1992
>
>
> Repository: kafka
>
>
> Description
> -------
>
> remove unnecessary requiredAcks parameter and clean up few comments
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/cluster/Partition.scala c4bf48a801007ebe7497077d2018d6dffe1677d4
> core/src/main/scala/kafka/server/DelayedProduce.scala 4d763bf05efb24a394662721292fc54d32467969
>
> Diff: https://reviews.apache.org/r/31591/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gwen Shapira
>
>
Re: Review Request 31591: Patch for KAFKA-1992
Posted by Jeff Holoman <je...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/#review74699
-----------------------------------------------------------
Looks good to me.
- Jeff Holoman
On March 1, 2015, 7:58 a.m., Gwen Shapira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31591/
> -----------------------------------------------------------
>
> (Updated March 1, 2015, 7:58 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1992
> https://issues.apache.org/jira/browse/KAFKA-1992
>
>
> Repository: kafka
>
>
> Description
> -------
>
> remove unnecessary requiredAcks parameter and clean up few comments
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/cluster/Partition.scala c4bf48a801007ebe7497077d2018d6dffe1677d4
> core/src/main/scala/kafka/server/DelayedProduce.scala 4d763bf05efb24a394662721292fc54d32467969
>
> Diff: https://reviews.apache.org/r/31591/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gwen Shapira
>
>