----------------------------------------------------------- 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 > >
