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

WriteBufferFromS3 potential deadlock fix #40070

Conversation

kitaisreal
Copy link
Contributor

@kitaisreal kitaisreal commented Aug 10, 2022

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in official stable or prestable release)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Fix potential deadlock in WriteBufferFromS3 during task scheduling failure.

@@ -272,7 +285,7 @@ void WriteBufferFromS3::writePart()
file_segments_holder.reset();
}

schedule([this, task]()
schedule([this, task, task_notify]()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Schedule can throw exception, in that case we could miss notify event.

@robot-clickhouse robot-clickhouse added the pr-not-for-changelog This PR should not be mentioned in the changelog label Aug 10, 2022
@kitaisreal kitaisreal force-pushed the write-buffer-from-s3-potential-deadlock-fix branch from e1b56c1 to fa65769 Compare August 10, 2022 17:08
@kitaisreal kitaisreal force-pushed the write-buffer-from-s3-potential-deadlock-fix branch from fa65769 to 4b1c93d Compare August 10, 2022 17:23
@alexey-milovidov
Copy link
Member

@KochetovNicolai Changelog should be edited.

@kitaisreal kitaisreal added the can be tested Allows running workflows for external contributors label Aug 10, 2022
@alexey-milovidov
Copy link
Member

@kitaisreal please check if it fixes #38158.

@kitaisreal kitaisreal force-pushed the write-buffer-from-s3-potential-deadlock-fix branch from 6c7576c to 60e4010 Compare August 12, 2022 12:06
@kitaisreal kitaisreal force-pushed the write-buffer-from-s3-potential-deadlock-fix branch from 60e4010 to ee5e61c Compare August 15, 2022 09:09
@kitaisreal kitaisreal added bug Confirmed user-visible misbehaviour in official release and removed pr-not-for-changelog This PR should not be mentioned in the changelog labels Aug 15, 2022
@robot-clickhouse robot-clickhouse added the pr-bugfix Pull request with bugfix, not backported by default label Aug 15, 2022
@tavplubix tavplubix removed the bug Confirmed user-visible misbehaviour in official release label Aug 17, 2022
@KochetovNicolai KochetovNicolai merged commit 335a5e3 into ClickHouse:master Aug 17, 2022
@kitaisreal kitaisreal added the pr-must-backport Pull request should be backported intentionally. Use this label with great care! label Aug 23, 2022
kitaisreal added a commit that referenced this pull request Aug 24, 2022
Backport #40070 to 22.7: WriteBufferFromS3 potential deadlock fix
@robot-ch-test-poll robot-ch-test-poll added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Sep 1, 2022
kitaisreal added a commit that referenced this pull request Sep 8, 2022
Backport #40070 to 22.6: WriteBufferFromS3 potential deadlock fix
kitaisreal added a commit that referenced this pull request Sep 8, 2022
Backport #40070 to 22.3: WriteBufferFromS3 potential deadlock fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
can be tested Allows running workflows for external contributors pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-bugfix Pull request with bugfix, not backported by default pr-must-backport Pull request should be backported intentionally. Use this label with great care!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants