You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Sriharsha Chintalapani <ha...@hortonworks.com> on 2015/02/21 09:58:05 UTC
Review Request 31256: Patch for KAFKA-1887
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31256/
-----------------------------------------------------------
Review request for kafka.
Bugs: KAFKA-1887
https://issues.apache.org/jira/browse/KAFKA-1887
Repository: kafka
Description
-------
KAFKA-1887. controller error message on shutting the last broker.
Diffs
-----
core/src/main/scala/kafka/server/KafkaServer.scala 7e5ddcb9be8fcef3df6ebc82a13ef44ef95f73ae
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala ba48a636dd0b0ed06646d56bb36aa3d43228604f
Diff: https://reviews.apache.org/r/31256/diff/
Testing
-------
Thanks,
Sriharsha Chintalapani
Re: Review Request 31256: Patch for KAFKA-1887
Posted by Sriharsha Chintalapani <ha...@hortonworks.com>.
> On Feb. 21, 2015, 7:03 p.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/server/KafkaServer.scala, line 317
> > <https://reviews.apache.org/r/31256/diff/2/?file=871520#file871520line317>
> >
> > Overall, this change makes sense to me. However, now the startup and shutdown sequence is complicated and tricky to understand. It will be very helpful to document your findings in a comment on both startup and shutdown on why the sequence has to be this way. Also let's leave a note on being very careful before changing the sequence.
Neha, Sorry didn't see the comments. Will udpate the patch with comments and send new PR. Thanks for the reminder Gwen :).
- Sriharsha
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31256/#review73413
-----------------------------------------------------------
On Feb. 21, 2015, 9:12 a.m., Sriharsha Chintalapani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31256/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2015, 9:12 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1887
> https://issues.apache.org/jira/browse/KAFKA-1887
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1887. controller error message on shutting the last broker.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/server/KafkaServer.scala 7e5ddcb9be8fcef3df6ebc82a13ef44ef95f73ae
> core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala ba48a636dd0b0ed06646d56bb36aa3d43228604f
>
> Diff: https://reviews.apache.org/r/31256/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sriharsha Chintalapani
>
>
Re: Review Request 31256: Patch for KAFKA-1887
Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31256/#review73413
-----------------------------------------------------------
Ship it!
core/src/main/scala/kafka/server/KafkaServer.scala
<https://reviews.apache.org/r/31256/#comment119750>
Overall, this change makes sense to me. However, now the startup and shutdown sequence is complicated and tricky to understand. It will be very helpful to document your findings in a comment on both startup and shutdown on why the sequence has to be this way. Also let's leave a note on being very careful before changing the sequence.
- Neha Narkhede
On Feb. 21, 2015, 9:12 a.m., Sriharsha Chintalapani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31256/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2015, 9:12 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1887
> https://issues.apache.org/jira/browse/KAFKA-1887
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1887. controller error message on shutting the last broker.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/server/KafkaServer.scala 7e5ddcb9be8fcef3df6ebc82a13ef44ef95f73ae
> core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala ba48a636dd0b0ed06646d56bb36aa3d43228604f
>
> Diff: https://reviews.apache.org/r/31256/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sriharsha Chintalapani
>
>
Re: Review Request 31256: Patch for KAFKA-1887
Posted by Sriharsha Chintalapani <ha...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31256/
-----------------------------------------------------------
(Updated Feb. 21, 2015, 9:12 a.m.)
Review request for kafka.
Bugs: KAFKA-1887
https://issues.apache.org/jira/browse/KAFKA-1887
Repository: kafka
Description
-------
KAFKA-1887. controller error message on shutting the last broker.
Diffs (updated)
-----
core/src/main/scala/kafka/server/KafkaServer.scala 7e5ddcb9be8fcef3df6ebc82a13ef44ef95f73ae
core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala ba48a636dd0b0ed06646d56bb36aa3d43228604f
Diff: https://reviews.apache.org/r/31256/diff/
Testing
-------
Thanks,
Sriharsha Chintalapani