You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Jaikiran Pai <ja...@gmail.com> on 2015/05/19 07:14:46 UTC
Review Request 34394: Patch for KAFKA-1907
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34394/
-----------------------------------------------------------
Review request for kafka.
Bugs: KAFKA-1907
https://issues.apache.org/jira/browse/KAFKA-1907
Repository: kafka
Description
-------
KAFKA-1907 Set operation retry timeout on ZkClient. Also mark certain Kafka threads as daemon to allow proper JVM shutdown
Diffs
-----
clients/src/main/java/org/apache/kafka/common/utils/Utils.java f73eedb030987f018d8446bb1dcd98d19fa97331
core/src/main/scala/kafka/network/SocketServer.scala edf6214278935c031cf493d72d266e715d43dd06
core/src/main/scala/kafka/server/DelayedOperation.scala 2ed9b467c2865e5717d7f6fd933cd09a5c5b22c0
core/src/main/scala/kafka/server/KafkaServer.scala ea6d165d8e5c3146d2c65e8ad1a513308334bf6f
Diff: https://reviews.apache.org/r/34394/diff/
Testing
-------
Thanks,
Jaikiran Pai
Re: Review Request 34394: Patch for KAFKA-1907
Posted by Jaikiran Pai <ja...@gmail.com>.
> On May 29, 2015, 7:11 a.m., Joe Stein wrote:
> > core/src/main/scala/kafka/utils/ZkUtils.scala, line 39
> > <https://reviews.apache.org/r/34394/diff/2/?file=971310#file971310line39>
> >
> > if we are going to add this it should be exposed as a configuration and written up in a KIP. We can't hard code values that folks won't understand without some clear information about why it is 5000
Ok. I'll read through the KIP process and create a new one.
- Jaikiran
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34394/#review85691
-----------------------------------------------------------
On May 25, 2015, 3:49 a.m., Jaikiran Pai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34394/
> -----------------------------------------------------------
>
> (Updated May 25, 2015, 3:49 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1907
> https://issues.apache.org/jira/browse/KAFKA-1907
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1907 Set operation retry timeout on ZkClient. Also mark certain Kafka threads as daemon to allow proper JVM shutdown
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/common/utils/Utils.java f73eedb030987f018d8446bb1dcd98d19fa97331
> core/src/main/scala/kafka/network/SocketServer.scala edf6214278935c031cf493d72d266e715d43dd06
> core/src/main/scala/kafka/server/DelayedOperation.scala 123078d97a7bfe2121655c00f3b2c6af21c53015
> core/src/main/scala/kafka/server/KafkaServer.scala e66710d2368334ece66f70d55f57b3f888262620
> core/src/main/scala/kafka/utils/ZkUtils.scala 78475e3d5ec477cef00caeaa34ff2d196466be96
>
> Diff: https://reviews.apache.org/r/34394/diff/
>
>
> Testing
> -------
>
> ZkClient was recently upgraded to 0.5 version, as part of KAFKA-2169. The 0.5 version of ZkClient contains an enhancement which allows passing of operation retry timeout https://github.com/sgroschupf/zkclient/pull/29. This now allows us to fix the issue reported in https://issues.apache.org/jira/browse/KAFKA-1907.
>
> The commit here passes the operation retry timeout while creating the ZkClient instances. The commit was contains a change to mark certain threads as daemon to allow a clean shutdown of the Kafka server when the zookeeper instance has gone done first.
>
> I've locally tested that shutting down Kafka, after zookeeper has already shutdown, works fine now (it tries to reconnect to zoookeeper for a maximum of 5 seconds before cleanly shutting down). I've also checked that shutting down Kafka first, when zookeeper is still up, works fine too.
>
>
> Thanks,
>
> Jaikiran Pai
>
>
Re: Review Request 34394: Patch for KAFKA-1907
Posted by Joe Stein <cr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34394/#review85691
-----------------------------------------------------------
core/src/main/scala/kafka/utils/ZkUtils.scala
<https://reviews.apache.org/r/34394/#comment137408>
if we are going to add this it should be exposed as a configuration and written up in a KIP. We can't hard code values that folks won't understand without some clear information about why it is 5000
- Joe Stein
On May 25, 2015, 3:49 a.m., Jaikiran Pai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34394/
> -----------------------------------------------------------
>
> (Updated May 25, 2015, 3:49 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1907
> https://issues.apache.org/jira/browse/KAFKA-1907
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1907 Set operation retry timeout on ZkClient. Also mark certain Kafka threads as daemon to allow proper JVM shutdown
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/common/utils/Utils.java f73eedb030987f018d8446bb1dcd98d19fa97331
> core/src/main/scala/kafka/network/SocketServer.scala edf6214278935c031cf493d72d266e715d43dd06
> core/src/main/scala/kafka/server/DelayedOperation.scala 123078d97a7bfe2121655c00f3b2c6af21c53015
> core/src/main/scala/kafka/server/KafkaServer.scala e66710d2368334ece66f70d55f57b3f888262620
> core/src/main/scala/kafka/utils/ZkUtils.scala 78475e3d5ec477cef00caeaa34ff2d196466be96
>
> Diff: https://reviews.apache.org/r/34394/diff/
>
>
> Testing
> -------
>
> ZkClient was recently upgraded to 0.5 version, as part of KAFKA-2169. The 0.5 version of ZkClient contains an enhancement which allows passing of operation retry timeout https://github.com/sgroschupf/zkclient/pull/29. This now allows us to fix the issue reported in https://issues.apache.org/jira/browse/KAFKA-1907.
>
> The commit here passes the operation retry timeout while creating the ZkClient instances. The commit was contains a change to mark certain threads as daemon to allow a clean shutdown of the Kafka server when the zookeeper instance has gone done first.
>
> I've locally tested that shutting down Kafka, after zookeeper has already shutdown, works fine now (it tries to reconnect to zoookeeper for a maximum of 5 seconds before cleanly shutting down). I've also checked that shutting down Kafka first, when zookeeper is still up, works fine too.
>
>
> Thanks,
>
> Jaikiran Pai
>
>
Re: Review Request 34394: Patch for KAFKA-1907
Posted by Jaikiran Pai <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34394/
-----------------------------------------------------------
(Updated May 25, 2015, 3:49 a.m.)
Review request for kafka.
Bugs: KAFKA-1907
https://issues.apache.org/jira/browse/KAFKA-1907
Repository: kafka
Description
-------
KAFKA-1907 Set operation retry timeout on ZkClient. Also mark certain Kafka threads as daemon to allow proper JVM shutdown
Diffs (updated)
-----
clients/src/main/java/org/apache/kafka/common/utils/Utils.java f73eedb030987f018d8446bb1dcd98d19fa97331
core/src/main/scala/kafka/network/SocketServer.scala edf6214278935c031cf493d72d266e715d43dd06
core/src/main/scala/kafka/server/DelayedOperation.scala 123078d97a7bfe2121655c00f3b2c6af21c53015
core/src/main/scala/kafka/server/KafkaServer.scala e66710d2368334ece66f70d55f57b3f888262620
core/src/main/scala/kafka/utils/ZkUtils.scala 78475e3d5ec477cef00caeaa34ff2d196466be96
Diff: https://reviews.apache.org/r/34394/diff/
Testing
-------
ZkClient was recently upgraded to 0.5 version, as part of KAFKA-2169. The 0.5 version of ZkClient contains an enhancement which allows passing of operation retry timeout https://github.com/sgroschupf/zkclient/pull/29. This now allows us to fix the issue reported in https://issues.apache.org/jira/browse/KAFKA-1907.
The commit here passes the operation retry timeout while creating the ZkClient instances. The commit was contains a change to mark certain threads as daemon to allow a clean shutdown of the Kafka server when the zookeeper instance has gone done first.
I've locally tested that shutting down Kafka, after zookeeper has already shutdown, works fine now (it tries to reconnect to zoookeeper for a maximum of 5 seconds before cleanly shutting down). I've also checked that shutting down Kafka first, when zookeeper is still up, works fine too.
Thanks,
Jaikiran Pai
Re: Review Request 34394: Patch for KAFKA-1907
Posted by Jaikiran Pai <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34394/
-----------------------------------------------------------
(Updated May 19, 2015, 5:20 a.m.)
Review request for kafka.
Bugs: KAFKA-1907
https://issues.apache.org/jira/browse/KAFKA-1907
Repository: kafka
Description
-------
KAFKA-1907 Set operation retry timeout on ZkClient. Also mark certain Kafka threads as daemon to allow proper JVM shutdown
Diffs
-----
clients/src/main/java/org/apache/kafka/common/utils/Utils.java f73eedb030987f018d8446bb1dcd98d19fa97331
core/src/main/scala/kafka/network/SocketServer.scala edf6214278935c031cf493d72d266e715d43dd06
core/src/main/scala/kafka/server/DelayedOperation.scala 2ed9b467c2865e5717d7f6fd933cd09a5c5b22c0
core/src/main/scala/kafka/server/KafkaServer.scala ea6d165d8e5c3146d2c65e8ad1a513308334bf6f
Diff: https://reviews.apache.org/r/34394/diff/
Testing (updated)
-------
ZkClient was recently upgraded to 0.5 version, as part of KAFKA-2169. The 0.5 version of ZkClient contains an enhancement which allows passing of operation retry timeout https://github.com/sgroschupf/zkclient/pull/29. This now allows us to fix the issue reported in https://issues.apache.org/jira/browse/KAFKA-1907.
The commit here passes the operation retry timeout while creating the ZkClient instances. The commit was contains a change to mark certain threads as daemon to allow a clean shutdown of the Kafka server when the zookeeper instance has gone done first.
I've locally tested that shutting down Kafka, after zookeeper has already shutdown, works fine now (it tries to reconnect to zoookeeper for a maximum of 5 seconds before cleanly shutting down). I've also checked that shutting down Kafka first, when zookeeper is still up, works fine too.
Thanks,
Jaikiran Pai