You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Guozhang Wang <gu...@linkedin.com> on 2014/02/03 21:02:35 UTC

Review Request 17671: Enable kafka server to keep retrying controlled shutdown with value -1

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17671/
-----------------------------------------------------------

Review request for kafka.


Bugs: KAFKA-1235
    https://issues.apache.org/jira/browse/KAFKA-1235


Repository: kafka


Description
-------

KAFKA-1235.v1


Diffs
-----

  core/src/main/scala/kafka/consumer/SimpleConsumer.scala 6dae149adfd4bfaa0d98aa641dfc41b00fcb6162 
  core/src/main/scala/kafka/controller/ControllerChannelManager.scala ea8485b479155b479c575ebc89a4f73086c872cb 
  core/src/main/scala/kafka/network/BlockingChannel.scala d22dabdf4fc2346c5487b9fd94cadfbcab70040d 
  core/src/main/scala/kafka/producer/SyncProducer.scala 041cfa59c18fa04706d28e98e58fda3937b6c992 
  core/src/main/scala/kafka/server/KafkaConfig.scala 3c3aafc2b3f06fc8f3168a8a9c1e0b08e944c1ef 
  core/src/main/scala/kafka/server/KafkaServer.scala 5e34f95e64eaf12ae7e904ffef32422a365eca86 

Diff: https://reviews.apache.org/r/17671/diff/


Testing
-------


Thanks,

Guozhang Wang


Re: Review Request 17671: Fix KAFKA-1235

Posted by Guozhang Wang <gu...@linkedin.com>.

> On Feb. 20, 2014, 5:28 p.m., Jun Rao wrote:
> > The patch doesn't apply to chunk. Could you rebase?

Wired.. I did a rebase already, would do that again.


> On Feb. 20, 2014, 5:28 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/consumer/SimpleConsumer.scala, lines 49-51
> > <https://reviews.apache.org/r/17671/diff/2/?file=498280#file498280line49>
> >
> >     Would it better to always call blockingChannel.disconnect() since it's already doing the same check there?

Yes.


> On Feb. 20, 2014, 5:28 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/producer/SyncProducer.scala, lines 129-131
> > <https://reviews.apache.org/r/17671/diff/2/?file=498282#file498282line129>
> >
> >     Would it better to always call blockingChannel.disconnect() since it's already doing the same check there?

Ack


> On Feb. 20, 2014, 5:28 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/server/KafkaServer.scala, lines 222-223
> > <https://reviews.apache.org/r/17671/diff/2/?file=498285#file498285line222>
> >
> >     Actual, not sure why we need this. Closing zkclient will automatically deregister the ephemeral node.

zkClient needs to be closed as the last step of shutting down the broker, since it is needed for other modules. The deadlock is caused by the step after shutting down socket server. That is why we want to de-register the ephemeral node before shutting down the socket server.


> On Feb. 20, 2014, 5:28 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/server/KafkaServer.scala, lines 136-137
> > <https://reviews.apache.org/r/17671/diff/2/?file=498285#file498285line136>
> >
> >     Wouldn't just setting maxRetries to maxInt achieve the same thing?

I am open either way, removing this logic then.


- Guozhang


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17671/#review35013
-----------------------------------------------------------


On Feb. 19, 2014, 11:57 p.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17671/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2014, 11:57 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1235
>     https://issues.apache.org/jira/browse/KAFKA-1235
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Also including several fixes in this patch:
> 
> 1. Check if channel is null or not for disconnecting instead of checking the isConnected variable, since otherwise a socket can be opened but not connected due to an exception, and hence lead to socket leak on channel.disconnect().
> 
> 2. Make sure all connect() is guarded by a disconnect in the catch clause. This is also for socket leak prevention.
> 
> 3. When closing the channel, check both channel and readChannel to see if they are null separately to avoid NPE.
> 
> 4. On shutdown, let the KafkaHealthCheck to progressively delete the registration path in ZK to resolve the controller-shutdown-itself deadlock.
> 
> 5. Some unit tests setup/teardown ordering.
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/consumer/SimpleConsumer.scala 6dae149adfd4bfaa0d98aa641dfc41b00fcb6162 
>   core/src/main/scala/kafka/network/BlockingChannel.scala ab04b3fe0dc674455a74659ad9975458bfbbac36 
>   core/src/main/scala/kafka/producer/SyncProducer.scala 041cfa59c18fa04706d28e98e58fda3937b6c992 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 3c3aafc2b3f06fc8f3168a8a9c1e0b08e944c1ef 
>   core/src/main/scala/kafka/server/KafkaHealthcheck.scala 8c69d095bfa9f79049bf9e6ec86adb7f956c2aeb 
>   core/src/main/scala/kafka/server/KafkaServer.scala 5e34f95e64eaf12ae7e904ffef32422a365eca86 
>   core/src/main/scala/kafka/utils/ZkUtils.scala fa86bb94475dec41d5ea1a94f4eebcd5500756e6 
>   core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala 17a99f182f64c0608a604f0d338b1ac272399e49 
>   core/src/test/scala/unit/kafka/zk/ZooKeeperTestHarness.scala 4e25b926d32e4885fa141ceaa9156c38629dd3d5 
> 
> Diff: https://reviews.apache.org/r/17671/diff/
> 
> 
> Testing
> -------
> 
> unit-tests
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>


Re: Review Request 17671: Fix KAFKA-1235

Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17671/#review35013
-----------------------------------------------------------


The patch doesn't apply to chunk. Could you rebase?


core/src/main/scala/kafka/consumer/SimpleConsumer.scala
<https://reviews.apache.org/r/17671/#comment65386>

    Would it better to always call blockingChannel.disconnect() since it's already doing the same check there?



core/src/main/scala/kafka/producer/SyncProducer.scala
<https://reviews.apache.org/r/17671/#comment65387>

    Would it better to always call blockingChannel.disconnect() since it's already doing the same check there?



core/src/main/scala/kafka/server/KafkaServer.scala
<https://reviews.apache.org/r/17671/#comment65388>

    Wouldn't just setting maxRetries to maxInt achieve the same thing?



core/src/main/scala/kafka/server/KafkaServer.scala
<https://reviews.apache.org/r/17671/#comment65389>

    Actual, not sure why we need this. Closing zkclient will automatically deregister the ephemeral node.


- Jun Rao


On Feb. 19, 2014, 11:57 p.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17671/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2014, 11:57 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1235
>     https://issues.apache.org/jira/browse/KAFKA-1235
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Also including several fixes in this patch:
> 
> 1. Check if channel is null or not for disconnecting instead of checking the isConnected variable, since otherwise a socket can be opened but not connected due to an exception, and hence lead to socket leak on channel.disconnect().
> 
> 2. Make sure all connect() is guarded by a disconnect in the catch clause. This is also for socket leak prevention.
> 
> 3. When closing the channel, check both channel and readChannel to see if they are null separately to avoid NPE.
> 
> 4. On shutdown, let the KafkaHealthCheck to progressively delete the registration path in ZK to resolve the controller-shutdown-itself deadlock.
> 
> 5. Some unit tests setup/teardown ordering.
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/consumer/SimpleConsumer.scala 6dae149adfd4bfaa0d98aa641dfc41b00fcb6162 
>   core/src/main/scala/kafka/network/BlockingChannel.scala ab04b3fe0dc674455a74659ad9975458bfbbac36 
>   core/src/main/scala/kafka/producer/SyncProducer.scala 041cfa59c18fa04706d28e98e58fda3937b6c992 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 3c3aafc2b3f06fc8f3168a8a9c1e0b08e944c1ef 
>   core/src/main/scala/kafka/server/KafkaHealthcheck.scala 8c69d095bfa9f79049bf9e6ec86adb7f956c2aeb 
>   core/src/main/scala/kafka/server/KafkaServer.scala 5e34f95e64eaf12ae7e904ffef32422a365eca86 
>   core/src/main/scala/kafka/utils/ZkUtils.scala fa86bb94475dec41d5ea1a94f4eebcd5500756e6 
>   core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala 17a99f182f64c0608a604f0d338b1ac272399e49 
>   core/src/test/scala/unit/kafka/zk/ZooKeeperTestHarness.scala 4e25b926d32e4885fa141ceaa9156c38629dd3d5 
> 
> Diff: https://reviews.apache.org/r/17671/diff/
> 
> 
> Testing
> -------
> 
> unit-tests
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>


Re: Review Request 17671: Fix KAKFKA-1235

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17671/
-----------------------------------------------------------

(Updated Feb. 20, 2014, 9:08 p.m.)


Review request for kafka.


Bugs: KAFKA-1235
    https://issues.apache.org/jira/browse/KAFKA-1235


Repository: kafka


Description (updated)
-------

Also including several fixes in this patch:

1. Check if channel is null or not for disconnecting instead of checking the isConnected variable, since otherwise a socket can be opened but not connected due to an exception, and hence lead to socket leak on channel.disconnect().

2. Make sure all connect() is guarded by a disconnect in the catch clause. This is also for socket leak prevention.

3. When closing the channel, check both channel and readChannel to see if they are null separately to avoid NPE.

4. On shutdown, let the KafkaHealthCheck to progressively delete the registration path in ZK to resolve the controller-shutdown-itself deadlock.

5. Some unit tests setup/teardown ordering.


Diffs
-----

  core/src/main/scala/kafka/consumer/SimpleConsumer.scala 6dae149adfd4bfaa0d98aa641dfc41b00fcb6162 
  core/src/main/scala/kafka/network/BlockingChannel.scala ab04b3fe0dc674455a74659ad9975458bfbbac36 
  core/src/main/scala/kafka/producer/SyncProducer.scala 041cfa59c18fa04706d28e98e58fda3937b6c992 
  core/src/main/scala/kafka/server/KafkaConfig.scala 3c3aafc2b3f06fc8f3168a8a9c1e0b08e944c1ef 
  core/src/main/scala/kafka/server/KafkaHealthcheck.scala 8c69d095bfa9f79049bf9e6ec86adb7f956c2aeb 
  core/src/main/scala/kafka/server/KafkaServer.scala 5e34f95e64eaf12ae7e904ffef32422a365eca86 
  core/src/main/scala/kafka/utils/ZkUtils.scala f8e798b231fbbd2c663744fb56a733918cfb6983 
  core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala 17a99f182f64c0608a604f0d338b1ac272399e49 
  core/src/test/scala/unit/kafka/zk/ZooKeeperTestHarness.scala 4e25b926d32e4885fa141ceaa9156c38629dd3d5 

Diff: https://reviews.apache.org/r/17671/diff/


Testing
-------

unit-tests


Thanks,

Guozhang Wang


Re: Review Request 17671: Fix KAKFKA-1235

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17671/
-----------------------------------------------------------

(Updated Feb. 20, 2014, 9:08 p.m.)


Review request for kafka.


Bugs: KAFKA-1235
    https://issues.apache.org/jira/browse/KAFKA-1235


Repository: kafka


Description (updated)
-------

Dummy


Dummy


Rebased


Dummy 2


Dummy


KAFKA-1235.v1


Diffs (updated)
-----

  core/src/main/scala/kafka/consumer/SimpleConsumer.scala 6dae149adfd4bfaa0d98aa641dfc41b00fcb6162 
  core/src/main/scala/kafka/network/BlockingChannel.scala ab04b3fe0dc674455a74659ad9975458bfbbac36 
  core/src/main/scala/kafka/producer/SyncProducer.scala 041cfa59c18fa04706d28e98e58fda3937b6c992 
  core/src/main/scala/kafka/server/KafkaConfig.scala 3c3aafc2b3f06fc8f3168a8a9c1e0b08e944c1ef 
  core/src/main/scala/kafka/server/KafkaHealthcheck.scala 8c69d095bfa9f79049bf9e6ec86adb7f956c2aeb 
  core/src/main/scala/kafka/server/KafkaServer.scala 5e34f95e64eaf12ae7e904ffef32422a365eca86 
  core/src/main/scala/kafka/utils/ZkUtils.scala f8e798b231fbbd2c663744fb56a733918cfb6983 
  core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala 17a99f182f64c0608a604f0d338b1ac272399e49 
  core/src/test/scala/unit/kafka/zk/ZooKeeperTestHarness.scala 4e25b926d32e4885fa141ceaa9156c38629dd3d5 

Diff: https://reviews.apache.org/r/17671/diff/


Testing
-------

unit-tests


Thanks,

Guozhang Wang


Re: Review Request 17671: Fix KAKFKA-1235

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17671/
-----------------------------------------------------------

(Updated Feb. 20, 2014, 7:29 p.m.)


Review request for kafka.


Bugs: KAFKA-1235
    https://issues.apache.org/jira/browse/KAFKA-1235


Repository: kafka


Description (updated)
-------

Also including several fixes in this patch:

1. Check if channel is null or not for disconnecting instead of checking the isConnected variable, since otherwise a socket can be opened but not connected due to an exception, and hence lead to socket leak on channel.disconnect().

2. Make sure all connect() is guarded by a disconnect in the catch clause. This is also for socket leak prevention.

3. When closing the channel, check both channel and readChannel to see if they are null separately to avoid NPE.

4. On shutdown, let the KafkaHealthCheck to progressively delete the registration path in ZK to resolve the controller-shutdown-itself deadlock.

5. Some unit tests setup/teardown ordering.


Diffs
-----

  core/src/main/scala/kafka/consumer/SimpleConsumer.scala 6dae149adfd4bfaa0d98aa641dfc41b00fcb6162 
  core/src/main/scala/kafka/network/BlockingChannel.scala ab04b3fe0dc674455a74659ad9975458bfbbac36 
  core/src/main/scala/kafka/producer/SyncProducer.scala 041cfa59c18fa04706d28e98e58fda3937b6c992 
  core/src/main/scala/kafka/server/KafkaConfig.scala 3c3aafc2b3f06fc8f3168a8a9c1e0b08e944c1ef 
  core/src/main/scala/kafka/server/KafkaHealthcheck.scala 8c69d095bfa9f79049bf9e6ec86adb7f956c2aeb 
  core/src/main/scala/kafka/server/KafkaServer.scala 5e34f95e64eaf12ae7e904ffef32422a365eca86 
  core/src/main/scala/kafka/utils/ZkUtils.scala f8e798b231fbbd2c663744fb56a733918cfb6983 
  core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala 17a99f182f64c0608a604f0d338b1ac272399e49 
  core/src/test/scala/unit/kafka/zk/ZooKeeperTestHarness.scala 4e25b926d32e4885fa141ceaa9156c38629dd3d5 

Diff: https://reviews.apache.org/r/17671/diff/


Testing
-------

unit-tests


Thanks,

Guozhang Wang


Re: Review Request 17671: Fix KAKFKA-1235

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17671/
-----------------------------------------------------------

(Updated Feb. 20, 2014, 7:28 p.m.)


Review request for kafka.


Summary (updated)
-----------------

Fix KAKFKA-1235


Bugs: KAFKA-1235
    https://issues.apache.org/jira/browse/KAFKA-1235


Repository: kafka


Description (updated)
-------

Dummy


Rebased


Dummy 2


Dummy


KAFKA-1235.v1


Diffs (updated)
-----

  core/src/main/scala/kafka/consumer/SimpleConsumer.scala 6dae149adfd4bfaa0d98aa641dfc41b00fcb6162 
  core/src/main/scala/kafka/network/BlockingChannel.scala ab04b3fe0dc674455a74659ad9975458bfbbac36 
  core/src/main/scala/kafka/producer/SyncProducer.scala 041cfa59c18fa04706d28e98e58fda3937b6c992 
  core/src/main/scala/kafka/server/KafkaConfig.scala 3c3aafc2b3f06fc8f3168a8a9c1e0b08e944c1ef 
  core/src/main/scala/kafka/server/KafkaHealthcheck.scala 8c69d095bfa9f79049bf9e6ec86adb7f956c2aeb 
  core/src/main/scala/kafka/server/KafkaServer.scala 5e34f95e64eaf12ae7e904ffef32422a365eca86 
  core/src/main/scala/kafka/utils/ZkUtils.scala f8e798b231fbbd2c663744fb56a733918cfb6983 
  core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala 17a99f182f64c0608a604f0d338b1ac272399e49 
  core/src/test/scala/unit/kafka/zk/ZooKeeperTestHarness.scala 4e25b926d32e4885fa141ceaa9156c38629dd3d5 

Diff: https://reviews.apache.org/r/17671/diff/


Testing
-------

unit-tests


Thanks,

Guozhang Wang


Re: Review Request 17671: Fix KAFKA-1235

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17671/
-----------------------------------------------------------

(Updated Feb. 19, 2014, 11:57 p.m.)


Review request for kafka.


Bugs: KAFKA-1235
    https://issues.apache.org/jira/browse/KAFKA-1235


Repository: kafka


Description
-------

Also including several fixes in this patch:

1. Check if channel is null or not for disconnecting instead of checking the isConnected variable, since otherwise a socket can be opened but not connected due to an exception, and hence lead to socket leak on channel.disconnect().

2. Make sure all connect() is guarded by a disconnect in the catch clause. This is also for socket leak prevention.

3. When closing the channel, check both channel and readChannel to see if they are null separately to avoid NPE.

4. On shutdown, let the KafkaHealthCheck to progressively delete the registration path in ZK to resolve the controller-shutdown-itself deadlock.

5. Some unit tests setup/teardown ordering.


Diffs
-----

  core/src/main/scala/kafka/consumer/SimpleConsumer.scala 6dae149adfd4bfaa0d98aa641dfc41b00fcb6162 
  core/src/main/scala/kafka/network/BlockingChannel.scala ab04b3fe0dc674455a74659ad9975458bfbbac36 
  core/src/main/scala/kafka/producer/SyncProducer.scala 041cfa59c18fa04706d28e98e58fda3937b6c992 
  core/src/main/scala/kafka/server/KafkaConfig.scala 3c3aafc2b3f06fc8f3168a8a9c1e0b08e944c1ef 
  core/src/main/scala/kafka/server/KafkaHealthcheck.scala 8c69d095bfa9f79049bf9e6ec86adb7f956c2aeb 
  core/src/main/scala/kafka/server/KafkaServer.scala 5e34f95e64eaf12ae7e904ffef32422a365eca86 
  core/src/main/scala/kafka/utils/ZkUtils.scala fa86bb94475dec41d5ea1a94f4eebcd5500756e6 
  core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala 17a99f182f64c0608a604f0d338b1ac272399e49 
  core/src/test/scala/unit/kafka/zk/ZooKeeperTestHarness.scala 4e25b926d32e4885fa141ceaa9156c38629dd3d5 

Diff: https://reviews.apache.org/r/17671/diff/


Testing
-------

unit-tests


Thanks,

Guozhang Wang


Re: Review Request 17671: Fix KAFKA-1235

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17671/
-----------------------------------------------------------

(Updated Feb. 19, 2014, 11:57 p.m.)


Review request for kafka.


Bugs: KAFKA-1235 and KAFKA-1260
    https://issues.apache.org/jira/browse/KAFKA-1235
    https://issues.apache.org/jira/browse/KAFKA-1260


Repository: kafka


Description (updated)
-------

Also including several fixes in this patch:

1. Check if channel is null or not for disconnecting instead of checking the isConnected variable, since otherwise a socket can be opened but not connected due to an exception, and hence lead to socket leak on channel.disconnect().

2. Make sure all connect() is guarded by a disconnect in the catch clause. This is also for socket leak prevention.

3. When closing the channel, check both channel and readChannel to see if they are null separately to avoid NPE.

4. On shutdown, let the KafkaHealthCheck to progressively delete the registration path in ZK to resolve the controller-shutdown-itself deadlock.

5. Some unit tests setup/teardown ordering.


Diffs
-----

  core/src/main/scala/kafka/consumer/SimpleConsumer.scala 6dae149adfd4bfaa0d98aa641dfc41b00fcb6162 
  core/src/main/scala/kafka/network/BlockingChannel.scala ab04b3fe0dc674455a74659ad9975458bfbbac36 
  core/src/main/scala/kafka/producer/SyncProducer.scala 041cfa59c18fa04706d28e98e58fda3937b6c992 
  core/src/main/scala/kafka/server/KafkaConfig.scala 3c3aafc2b3f06fc8f3168a8a9c1e0b08e944c1ef 
  core/src/main/scala/kafka/server/KafkaHealthcheck.scala 8c69d095bfa9f79049bf9e6ec86adb7f956c2aeb 
  core/src/main/scala/kafka/server/KafkaServer.scala 5e34f95e64eaf12ae7e904ffef32422a365eca86 
  core/src/main/scala/kafka/utils/ZkUtils.scala fa86bb94475dec41d5ea1a94f4eebcd5500756e6 
  core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala 17a99f182f64c0608a604f0d338b1ac272399e49 
  core/src/test/scala/unit/kafka/zk/ZooKeeperTestHarness.scala 4e25b926d32e4885fa141ceaa9156c38629dd3d5 

Diff: https://reviews.apache.org/r/17671/diff/


Testing
-------

unit-tests


Thanks,

Guozhang Wang


Re: Review Request 17671: Fix KAFKA-1235

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17671/
-----------------------------------------------------------

(Updated Feb. 19, 2014, 11:55 p.m.)


Review request for kafka.


Summary (updated)
-----------------

Fix KAFKA-1235


Bugs: KAFKA-1235 and KAFKA-1260
    https://issues.apache.org/jira/browse/KAFKA-1235
    https://issues.apache.org/jira/browse/KAFKA-1260


Repository: kafka


Description (updated)
-------

Dummy


Rebased


Dummy 2


Dummy


KAFKA-1235.v1


Diffs (updated)
-----

  core/src/main/scala/kafka/consumer/SimpleConsumer.scala 6dae149adfd4bfaa0d98aa641dfc41b00fcb6162 
  core/src/main/scala/kafka/network/BlockingChannel.scala ab04b3fe0dc674455a74659ad9975458bfbbac36 
  core/src/main/scala/kafka/producer/SyncProducer.scala 041cfa59c18fa04706d28e98e58fda3937b6c992 
  core/src/main/scala/kafka/server/KafkaConfig.scala 3c3aafc2b3f06fc8f3168a8a9c1e0b08e944c1ef 
  core/src/main/scala/kafka/server/KafkaHealthcheck.scala 8c69d095bfa9f79049bf9e6ec86adb7f956c2aeb 
  core/src/main/scala/kafka/server/KafkaServer.scala 5e34f95e64eaf12ae7e904ffef32422a365eca86 
  core/src/main/scala/kafka/utils/ZkUtils.scala fa86bb94475dec41d5ea1a94f4eebcd5500756e6 
  core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala 17a99f182f64c0608a604f0d338b1ac272399e49 
  core/src/test/scala/unit/kafka/zk/ZooKeeperTestHarness.scala 4e25b926d32e4885fa141ceaa9156c38629dd3d5 

Diff: https://reviews.apache.org/r/17671/diff/


Testing
-------

unit-tests


Thanks,

Guozhang Wang


Re: Review Request 17671: Enable kafka server to keep retrying controlled shutdown with value -1

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17671/
-----------------------------------------------------------

(Updated Feb. 3, 2014, 8:07 p.m.)


Review request for kafka.


Bugs: KAFKA-1235
    https://issues.apache.org/jira/browse/KAFKA-1235


Repository: kafka


Description (updated)
-------

Also including several fixes in this patch:

1. Check if channel is null or not for disconnecting instead of checking the isConnected variable, since otherwise a socket can be opened but not connected due to an exception, and hence lead to socket leak on channel.disconnect().

2. Make sure all connect() is guarded by a disconnect in the catch clause. This is also for socket leak prevention.

3. When closing the channel, check both channel and readChannel to see if they are null separately to avoid NPE.

4. Let the RequestSendThread to give up retrying sending to a broker if it is being shutdown, since its socket server may already be closed but itself is blocked on fully shutdown, hence leading to deadlock.


Diffs
-----

  core/src/main/scala/kafka/consumer/SimpleConsumer.scala 6dae149adfd4bfaa0d98aa641dfc41b00fcb6162 
  core/src/main/scala/kafka/controller/ControllerChannelManager.scala ea8485b479155b479c575ebc89a4f73086c872cb 
  core/src/main/scala/kafka/network/BlockingChannel.scala d22dabdf4fc2346c5487b9fd94cadfbcab70040d 
  core/src/main/scala/kafka/producer/SyncProducer.scala 041cfa59c18fa04706d28e98e58fda3937b6c992 
  core/src/main/scala/kafka/server/KafkaConfig.scala 3c3aafc2b3f06fc8f3168a8a9c1e0b08e944c1ef 
  core/src/main/scala/kafka/server/KafkaServer.scala 5e34f95e64eaf12ae7e904ffef32422a365eca86 

Diff: https://reviews.apache.org/r/17671/diff/


Testing (updated)
-------

unit-tests


Thanks,

Guozhang Wang