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

Storages/Kafka: fixed deadlocks, moved to background pool #3215

Merged
merged 3 commits into from Oct 1, 2018

Conversation

vavrusa
Copy link
Contributor

@vavrusa vavrusa commented Sep 26, 2018

This fixes two deadlocks in Kafka engine found previously:

  • When exception is thrown before starting reading, consumer
    was never returned to the storage. Now it is claimed only
    when actually starting reading.
  • Fixed lockup on deinitialization when consumer only unsubscribed,
    but didn't close, and the endine then timeouted when waiting for
    consumer destruction.

This also moves the stream thread to background worker pool.
The reason for that is that it will compete with other tasks for
time, so it will form a backpressure on insertion when the system
is busy.

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

cc @bocharov @dqminh

This fixes two deadlocks in Kafka engine found previously:
* When exception is thrown before starting reading, consumer
  was never returned to the storage. Now it is claimed only
  when actually starting reading.
* Fixed lockup on deinitialization when consumer only unsubscribed,
  but didn't close, and the endine then timeouted when waiting for
  consumer destruction.

This also moves the stream thread to background worker pool.
The reason for that is that it will compete with other tasks for
time, so it will form a backpressure on insertion when the system
is busy.
@alexey-milovidov alexey-milovidov merged commit fd04b2b into ClickHouse:master Oct 1, 2018
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.

None yet

2 participants