Skip to content

Disable S3 requests processing during context shutdown to speed up termination process#14496

Merged
alesapin merged 3 commits intoClickHouse:masterfrom
Jokser:disk-s3-shutdown
Sep 8, 2020
Merged

Disable S3 requests processing during context shutdown to speed up termination process#14496
alesapin merged 3 commits intoClickHouse:masterfrom
Jokser:disk-s3-shutdown

Conversation

@Jokser
Copy link
Copy Markdown
Contributor

@Jokser Jokser commented Sep 4, 2020

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

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Speed up server shutdown process if there are ongoing S3 requests

Detailed description / Documentation draft:
The shutdown process can be delayed if there are ongoing S3 requests, especially when S3 is unavailable (request is repeated again and again due to retry strategy).
When shutdown is called we can forcibly break all S3 connections and speed up the shutdown process.

@robot-clickhouse robot-clickhouse added the pr-improvement Pull request with some product improvements label Sep 4, 2020
for (auto & [disk_name, disk] : getDisksMap())
{
LOG_INFO(shared->log, "Shutdown disk {}", disk_name);
disk->shutdown();
Copy link
Copy Markdown
Member

@alesapin alesapin Sep 7, 2020

Choose a reason for hiding this comment

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

But tables shutdown will be called only after this line in shared->shutdown(). Can it lead to unexpected errors in log on server shutdown when tables still try to use their s3 disks?

Copy link
Copy Markdown
Contributor Author

@Jokser Jokser Sep 7, 2020

Choose a reason for hiding this comment

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

client->DisableRequestProcessing stops any next retry attempt for ongoing S3 request.
If an S3 request is failed and the code above is executed S3 client immediately returns the last failed S3 request outcome.
If S3 is healthy nothing wrong will be happened and S3 requests are processed in a regular way without errors.
When table is shutdown it waits for currently running merges or moves. If a table uses S3 and S3 is unhealthy we will just fail merge or move the process faster with the same result as we fail request after all unsuccessful retry attempts. If S3 is a healthy merge or move process will finish in a regular way without errors.

Copy link
Copy Markdown
Member

@alesapin alesapin Sep 8, 2020

Choose a reason for hiding this comment

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

Awesome, can you add the same explanation to the shutdown method of DiskS3.

@alesapin alesapin self-assigned this Sep 7, 2020
@alesapin
Copy link
Copy Markdown
Member

alesapin commented Sep 8, 2020

Only comment updated, merge without tests

@alesapin alesapin merged commit bea1517 into ClickHouse:master Sep 8, 2020
alesapin added a commit that referenced this pull request Sep 16, 2020
Backport to 20.8 - Disable S3 requests processing during context shutdown to speed up termination process #14496
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-improvement Pull request with some product improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants