Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: exception on shutdown of ksqlDB server (MINOR) #4483

Merged

Conversation

big-andy-coates
Copy link
Contributor

Description

On shutdown we're trying to close the Kafka Consumer from a different thread to the one that is using the consumer. Kafka Consumers are single threaded.
Hence we must close the consumer on the same thread as the one polling.

Fixes the following exception on shutdown:

[2020-02-07 15:10:44,646] ERROR Exception while waiting for CommandRunner thread to complete (io.confluent.ksql.rest.server.KsqlRestApplication:350)
    java.util.ConcurrentModificationException: KafkaConsumer is not safe for multi-threaded access
    at org.apache.kafka.clients.consumer.KafkaConsumer.acquire(KafkaConsumer.java:2422)
    at org.apache.kafka.clients.consumer.KafkaConsumer.close(KafkaConsumer.java:2305)
    at org.apache.kafka.clients.consumer.KafkaConsumer.close(KafkaConsumer.java:2260)
    at io.confluent.ksql.rest.server.CommandTopic.close(CommandTopic.java:121)
    at io.confluent.ksql.rest.server.computation.CommandStore.close(CommandStore.java:143)
    at io.confluent.ksql.rest.server.computation.CommandRunner.close(CommandRunner.java:148)
    at io.confluent.ksql.rest.server.KsqlRestApplication.triggerShutdown(KsqlRestApplication.java:348)
    at io.confluent.ksql.rest.server.ExecutableServer.triggerShutdown(ExecutableServer.java:54)
    at io.confluent.ksql.rest.server.KsqlServerMain.tryStartApp(KsqlServerMain.java:80)
    at io.confluent.ksql.rest.server.KsqlServerMain.main(KsqlServerMain.java:61)

Testing done

Usual

Reviewer checklist

  • Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
  • Ensure relevant issues are linked (description should include text like "Fixes #")

On shutdown we're trying to close the Kafka Consumer from a different thread to the one that is using the consumer. Kafka Consumers are single threaded.
Hence we must close the consumer on the same thread as the one polling.

Fixes the following exception on shutdown:

```
[2020-02-07 15:10:44,646] ERROR Exception while waiting for CommandRunner thread to complete (io.confluent.ksql.rest.server.KsqlRestApplication:350)
    java.util.ConcurrentModificationException: KafkaConsumer is not safe for multi-threaded access
    at org.apache.kafka.clients.consumer.KafkaConsumer.acquire(KafkaConsumer.java:2422)
    at org.apache.kafka.clients.consumer.KafkaConsumer.close(KafkaConsumer.java:2305)
    at org.apache.kafka.clients.consumer.KafkaConsumer.close(KafkaConsumer.java:2260)
    at io.confluent.ksql.rest.server.CommandTopic.close(CommandTopic.java:121)
    at io.confluent.ksql.rest.server.computation.CommandStore.close(CommandStore.java:143)
    at io.confluent.ksql.rest.server.computation.CommandRunner.close(CommandRunner.java:148)
    at io.confluent.ksql.rest.server.KsqlRestApplication.triggerShutdown(KsqlRestApplication.java:348)
    at io.confluent.ksql.rest.server.ExecutableServer.triggerShutdown(ExecutableServer.java:54)
    at io.confluent.ksql.rest.server.KsqlServerMain.tryStartApp(KsqlServerMain.java:80)
    at io.confluent.ksql.rest.server.KsqlServerMain.main(KsqlServerMain.java:61)
 ```
@big-andy-coates big-andy-coates requested a review from a team as a code owner February 7, 2020 17:46
Copy link
Contributor

@agavra agavra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch

@big-andy-coates big-andy-coates merged commit 126e2cf into confluentinc:master Feb 10, 2020
@big-andy-coates big-andy-coates deleted the shutdown_exception branch February 10, 2020 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants