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

Set is_all_data_sent on exceptions too #36816

Merged

Conversation

azat
Copy link
Collaborator

@azat azat commented Apr 30, 2022

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

This will avoid clickhouse-test complains about left queries after the
test, like in 1.

Follow-up for: #36649 (cc @alexey-milovidov )

@robot-ch-test-poll robot-ch-test-poll added the pr-not-for-changelog This PR should not be mentioned in the changelog label Apr 30, 2022
@azat azat force-pushed the is_all_data_sent-on-exception branch from 39bcd0b to d82f2ff Compare April 30, 2022 10:01
@azat
Copy link
Collaborator Author

azat commented Apr 30, 2022

https://s3.amazonaws.com/clickhouse-test-reports/36816/39bcd0b9e7b22da150a6dc35497458a919b8ce74/stress_test__memory__actions_.html

Stress hung due to DROP DATABASE test hung, which hunged because of S3:

2022.04.30 05:06:37.793788 [ 1072 ] {} <Error> static void DB::WriteBufferFromS3::finalizeCacheIfNeeded(std::optional<FileSegmentsHolder> &): Code: 49. DB::Exception: Operation not allowed, file segment is detached. (LOGICAL_ERROR), Stack trace (when copying this message, always include the lines below):

@kssenii it seems that #36660 is not enough?

@alexey-milovidov
Copy link
Member

test_storage_postgresql_replica

Failed, CC @kssenii

@alexey-milovidov alexey-milovidov merged commit 321514a into ClickHouse:master May 1, 2022
@azat azat deleted the is_all_data_sent-on-exception branch May 1, 2022 11:31
azat added a commit to azat/ClickHouse that referenced this pull request May 4, 2022
Before tests can fail if there was implicit reconnect, with queries
left, and without referenced PR, it requires manual debugging to know
that the reason was reconnect.

But the problem is, that the server does send EndOfStream but hanged
after, but before removing this query from the system.processes.
But after adding is_all_data_sent (ClickHouse#36816, ClickHouse#36767, ClickHouse#36649),
clickhouse-test can check queries left only for which server did not
sent EndOfStream/Exception.

In other words after adding is_all_data_sent, it should not be possible
to have queries left in such cases.

Reverts: 53be9c5
Reverts: ClickHouse#36587
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.

None yet

3 participants