[jira] [Commented] (KAFKA-4366) KafkaStreams.close() blocks indefinitely

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/KAFKA-4366?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15633814#comment-15633814 ] 

ASF GitHub Bot commented on KAFKA-4366:
---------------------------------------

GitHub user dguy opened a pull request:

    https://github.com/apache/kafka/pull/2097

    KAFKA-4366: KafkaStreams.close() blocks indefinitely

    Added `timeout` and `timeUnit` to `KafkaStreams.close(..)`. Now do close on a thread and `join` that thread with the provided `timeout`.
    Changed `state` in `KafkaStreams` to use an enum.
    Added system test to ensure we don't deadlock on close when an uncaught exception handler that calls `System.exit(..)` is used and there is also a shutdown hook that calls `KafkaStreams.close(...)`

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/dguy/kafka kafka-4366

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/kafka/pull/2097.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2097
    
----
commit 6dffa532ca0c065d2e67b812d82b3bdabe97b19d
Author: Damian Guy <da...@gmail.com>
Date:   2016-11-03T18:46:33Z

    add timeout to KafkaStreams.close(..)

----


> KafkaStreams.close() blocks indefinitely
> ----------------------------------------
>
>                 Key: KAFKA-4366
>                 URL: https://issues.apache.org/jira/browse/KAFKA-4366
>             Project: Kafka
>          Issue Type: Bug
>          Components: streams
>    Affects Versions: 0.10.1.0, 0.10.0.1
>            Reporter: Michal Borowiecki
>            Assignee: Damian Guy
>
> KafkaStreams.close() method calls join on all its threads without a timeout, meaning indefinitely, which makes it prone to deadlocks and unfit to be used in shutdown hooks.
> (KafkaStreams::close is used in numerous examples by confluent: https://github.com/confluentinc/examples/tree/kafka-0.10.0.1-cp-3.0.1/kafka-streams/src/main/java/io/confluent/examples/streams and https://www.confluent.io/blog/introducing-kafka-streams-stream-processing-made-simple/ so we assumed it to be recommended practice)
> A deadlock happens, for instance, if System.exit() is called from within the uncaughtExceptionHandler. (We need to call System.exit() from the uncaughtExceptionHandler because KAFKA-4355 issue shuts down the StreamThread and to recover we want the process to exit, as our infrastructure will then start it up again.)
> The System.exit call (from the uncaughtExceptionHandler, which runs in the StreamThread) will execute the shutdown hook in a new thread and wait for that thread to join. If the shutdown hook calls KafkaStreams.close, it will in turn block waiting for the StreamThread to join, hence the deadlock.
> Runtime.addShutdownHook javadocs state:
> {quote}
> Shutdown hooks run at a delicate time in the life cycle of a virtual machine and should therefore be coded defensively. They should, in particular, be written to be thread-safe and to avoid deadlocks insofar as possible
> {quote}
> and
> {quote}
> Shutdown hooks should also finish their work quickly.
> {quote}
> Therefore the current implementation of KafkaStreams.close() which waits forever for threads to join is completely unsuitable for use in a shutdown hook. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)