Skip to content

Do not silently catch errors for writing to S3#22208

Merged
vdimir merged 4 commits intoClickHouse:masterfrom
azat:s3-writer
Apr 26, 2021
Merged

Do not silently catch errors for writing to S3#22208
vdimir merged 4 commits intoClickHouse:masterfrom
azat:s3-writer

Conversation

@azat
Copy link
Copy Markdown
Member

@azat azat commented Mar 27, 2021

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Do not silently catch errors for writing to S3

Cc: @vdimir
Cc: @excitoon

Refs: #22110

@robot-clickhouse robot-clickhouse added the pr-not-for-changelog This PR should not be mentioned in the changelog label Mar 27, 2021
@azat azat marked this pull request as draft March 27, 2021 04:23
@azat azat force-pushed the s3-writer branch 4 times, most recently from 3c1293f to 479fa89 Compare April 15, 2021 18:25
@azat azat marked this pull request as ready for review April 15, 2021 18:25
@azat
Copy link
Copy Markdown
Member Author

azat commented Apr 16, 2021

@vdimir can you take a look?

@azat azat marked this pull request as draft April 19, 2021 06:07
@azat azat force-pushed the s3-writer branch 2 times, most recently from 7d568cb to 4cefa4e Compare April 24, 2021 21:19
azat added 4 commits April 25, 2021 12:47
That may lead to uncaught exception [1].

  [1]: ClickHouse#21624 (comment)

v0: New method BlockOutputStream::finalize() -- bad (forgot some places
    and superfluous)
v2: Flush BlockOutputStream in QueryStatus::releaseQueryStreams()
    Safer query termination (report some uncaugh errors to user)
v3: Flush BlockOutputStream from TCPHandler only
    Other places uses different executeQuery() implementation
v4: Do not call writeSuffix() twice
@azat azat marked this pull request as ready for review April 25, 2021 09:50
@azat azat requested a review from vdimir April 26, 2021 04:38
@vdimir vdimir merged commit 3c48b88 into ClickHouse:master Apr 26, 2021
@azat azat deleted the s3-writer branch April 26, 2021 21:34
azat added a commit to azat/ClickHouse that referenced this pull request Apr 28, 2021
Before this patch (and after ClickHouse#22208) the INSERT may fail with "Cannot
schedule a task" because the pool in DistributedBlockOutputStream
already throws exception and simply fail in writeSuffix().
kitaisreal pushed a commit that referenced this pull request Apr 29, 2021
Before this patch (and after #22208) the INSERT may fail with "Cannot
schedule a task" because the pool in DistributedBlockOutputStream
already throws exception and simply fail in writeSuffix().
kitaisreal pushed a commit that referenced this pull request Apr 29, 2021
Before this patch (and after #22208) the INSERT may fail with "Cannot
schedule a task" because the pool in DistributedBlockOutputStream
already throws exception and simply fail in writeSuffix().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants