Skip to content

Stop query execution if exception happened in PipelineExecutor itself.#14402

Merged
KochetovNicolai merged 2 commits intomasterfrom
stop-threads-if-exception-in-pipeline-executor
Sep 4, 2020
Merged

Stop query execution if exception happened in PipelineExecutor itself.#14402
KochetovNicolai merged 2 commits intomasterfrom
stop-threads-if-exception-in-pipeline-executor

Conversation

@KochetovNicolai
Copy link
Copy Markdown
Member

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

Changelog category (leave one):

  • Bug Fix

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Stop query execution if exception happened in PipelineExecutor itself. This could prevent rare possible query hung. Continuation of #14334

@robot-clickhouse robot-clickhouse added the pr-bugfix Pull request with bugfix, not backported by default label Sep 2, 2020
for (auto & executor_context : executor_contexts)
if (executor_context->exception)
std::rethrow_exception(executor_context->exception);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems executeStepImpl() never set executor_context->exception, so this cycle doesn't look useful.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Right. Wring place.

@@ -735,7 +731,16 @@ void PipelineExecutor::executeImpl(size_t num_threads)
CurrentThread::detachQueryIfNotDetached();
Copy link
Copy Markdown
Member

@vitlibar vitlibar Sep 2, 2020

Choose a reason for hiding this comment

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

It's not easy to understand the difference between these execute* functions.
Probably it's better to rename executeImpl() to something like executeMultipleThreads()?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Agree.
Will fix it in other pr.

@KochetovNicolai
Copy link
Copy Markdown
Member Author

01412_cache_dictionary_race is flappy.

@KochetovNicolai KochetovNicolai merged commit 01d0f6e into master Sep 4, 2020
@KochetovNicolai KochetovNicolai deleted the stop-threads-if-exception-in-pipeline-executor branch September 4, 2020 20:40
robot-clickhouse pushed a commit that referenced this pull request Sep 4, 2020
robot-clickhouse pushed a commit that referenced this pull request Sep 4, 2020
KochetovNicolai added a commit that referenced this pull request Sep 7, 2020
…n-pipeline-executor

Stop query execution if exception happened in PipelineExecutor itself.

(cherry picked from commit 01d0f6e)
KochetovNicolai added a commit that referenced this pull request Sep 7, 2020
Backport #14402 to 20.8: Stop query execution if exception happened in PipelineExecutor itself.
KochetovNicolai added a commit that referenced this pull request Sep 7, 2020
Backport #14402 to 20.7: Stop query execution if exception happened in PipelineExecutor itself.
robot-clickhouse pushed a commit that referenced this pull request Sep 7, 2020
robot-clickhouse pushed a commit that referenced this pull request Sep 7, 2020
KochetovNicolai added a commit that referenced this pull request Sep 8, 2020
Backport #14402 to 20.6: Stop query execution if exception happened in PipelineExecutor itself.
KochetovNicolai added a commit that referenced this pull request Sep 8, 2020
Backport #14402 to 20.5: Stop query execution if exception happened in PipelineExecutor itself.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-docs-needed pr-bugfix Pull request with bugfix, not backported by default

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants