Try to fix test_storage_s3: crash in WriteBufferFromS3#21624
Try to fix test_storage_s3: crash in WriteBufferFromS3#21624alexey-milovidov merged 1 commit intoClickHouse:masterfrom
Conversation
robot-clickhouse
added
the
pr-bugfix
| } | ||
| catch (...) | ||
| { | ||
| tryLogCurrentException(__PRETTY_FUNCTION__); |
There was a problem hiding this comment.
But this will hide write error, does not looks right...
There was a problem hiding this comment.
Let's merge this PR first and another PR later.
|
BTW There is another PR (#21325) that is trying to address this |
|
@azat FYI another idea is to make std::terminate in debug and logging in release. |
Backport #21624 to 21.3: Try to fix test_storage_s3: crash in WriteBufferFromS3
@alexey-milovidov I have similar thoughts, but decided not to, since write error may hide data loss. |
|
@vdimir can you provide a stack trace for uncaught exception? |
@azat https://pastebin.com/raw/1N0na6X4 |
@vdimir thanks! I will try to remove that try/catch to see if test 100% fails, and later push some fixes - #22208 |
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
4 participants
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
std::terminatewas called if there is an error writing data into s3.