You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Onur Karaman <ok...@linkedin.com> on 2015/07/09 11:37:02 UTC
Review Request 36346: fix heartbeat to allow offset commits during
PreparingRebalance
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36346/
-----------------------------------------------------------
Review request for kafka.
Bugs: KAFKA-1740
https://issues.apache.org/jira/browse/KAFKA-1740
Repository: kafka
Description
-------
fix heartbeat to allow offset commits during PreparingRebalance
Diffs
-----
core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala 476973b2c551db5be3f1c54f94990f0dd15ff65e
Diff: https://reviews.apache.org/r/36346/diff/
Testing
-------
Thanks,
Onur Karaman
Re: Review Request 36346: fix heartbeat to allow offset commits during
PreparingRebalance
Posted by Onur Karaman <ok...@linkedin.com>.
> On July 9, 2015, 4:58 p.m., Ewen Cheslack-Postava wrote:
> > Just FYI, this was also noticed in the patch for KAFKA-2123 (which is moving quite a bit of code around). The solution was a bit different (the condition is different and keeps the existing generation ID increment location).
Thanks for the heads up Ewen. I started looking at that patch yesterday but didn't reach the changes to core (it's at the end of the rb). It looks like Jason roughly followed option 1 from my comment in https://issues.apache.org/jira/browse/KAFKA-1740?focusedCommentId=14620038&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14620038 . I prefer the approach I provided because the generation id increment logic is easier to understand and the logged state transitions with their generation ids are more readable.
- Onur
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36346/#review91118
-----------------------------------------------------------
On July 9, 2015, 9:42 a.m., Onur Karaman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36346/
> -----------------------------------------------------------
>
> (Updated July 9, 2015, 9:42 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1740
> https://issues.apache.org/jira/browse/KAFKA-1740
>
>
> Repository: kafka
>
>
> Description
> -------
>
> fix heartbeat to allow offset commits during PreparingRebalance
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala 476973b2c551db5be3f1c54f94990f0dd15ff65e
>
> Diff: https://reviews.apache.org/r/36346/diff/
>
>
> Testing
> -------
>
> Manual testing only so far. The OffsetManager merge didn't add anything to ConsumerCoordinatorResponseTest.scala. I'll try to add all the missing OffsetCommitRequest and OffsetFetchRequest handling tests to this rb.
>
>
> Thanks,
>
> Onur Karaman
>
>
Re: Review Request 36346: fix heartbeat to allow offset commits during
PreparingRebalance
Posted by Ewen Cheslack-Postava <me...@ewencp.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36346/#review91118
-----------------------------------------------------------
Just FYI, this was also noticed in the patch for KAFKA-2123 (which is moving quite a bit of code around). The solution was a bit different (the condition is different and keeps the existing generation ID increment location).
- Ewen Cheslack-Postava
On July 9, 2015, 9:42 a.m., Onur Karaman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36346/
> -----------------------------------------------------------
>
> (Updated July 9, 2015, 9:42 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1740
> https://issues.apache.org/jira/browse/KAFKA-1740
>
>
> Repository: kafka
>
>
> Description
> -------
>
> fix heartbeat to allow offset commits during PreparingRebalance
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala 476973b2c551db5be3f1c54f94990f0dd15ff65e
>
> Diff: https://reviews.apache.org/r/36346/diff/
>
>
> Testing
> -------
>
> Manual testing only so far. The OffsetManager merge didn't add anything to ConsumerCoordinatorResponseTest.scala. I'll try to add all the missing OffsetCommitRequest and OffsetFetchRequest handling tests to this rb.
>
>
> Thanks,
>
> Onur Karaman
>
>
Re: Review Request 36346: fix heartbeat to allow offset commits during
PreparingRebalance
Posted by Onur Karaman <ok...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36346/
-----------------------------------------------------------
(Updated July 9, 2015, 9:42 a.m.)
Review request for kafka.
Bugs: KAFKA-1740
https://issues.apache.org/jira/browse/KAFKA-1740
Repository: kafka
Description
-------
fix heartbeat to allow offset commits during PreparingRebalance
Diffs
-----
core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala 476973b2c551db5be3f1c54f94990f0dd15ff65e
Diff: https://reviews.apache.org/r/36346/diff/
Testing (updated)
-------
Manual testing only so far. The OffsetManager merge didn't add anything to ConsumerCoordinatorResponseTest.scala. I'll try to add all the missing OffsetCommitRequest and OffsetFetchRequest handling tests to this rb.
Thanks,
Onur Karaman