Skip to content

Revert "Merge pull request #44922 from azat/dist/async-INSERT-metrics"#45492

Merged
tavplubix merged 1 commit intoClickHouse:masterfrom
azat:revert/dist/async-INSERT-metrics
Jan 21, 2023
Merged

Revert "Merge pull request #44922 from azat/dist/async-INSERT-metrics"#45492
tavplubix merged 1 commit intoClickHouse:masterfrom
azat:revert/dist/async-INSERT-metrics

Conversation

@azat
Copy link
Member

@azat azat commented Jan 21, 2023

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):

Revert "Merge pull request #44922 from azat/dist/async-INSERT-metrics"

There are the following problems with this patch:

  • Looses files on exception
  • Existing current_batch.txt on startup leads to ENOENT error and hung of distributed sends without ATTACH/DETACH
  • Race between creating the queue for sending at table startup and INSERT, if it had been created from INSERT, then it will not be initialized from disk

They were addressed in #45491, but it makes code more cmoplex and plus since, likely, the release is comming, it is better to revert the change.

This reverts commit 94604f7, reversing changes made to 80f6a45.

Note, marked as Not for changelog since the buggy patch had not been included into any release.

Cc: @tavplubix

…RT-metrics"

There are the following problems with this patch:
- Looses files on exception
- Existing current_batch.txt on startup leads to ENOENT error and hung
  of distributed sends without ATTACH/DETACH
- Race between creating the queue for sending at table startup and
  INSERT, if it had been created from INSERT, then it will not be
  initialized from disk

They were addressed in ClickHouse#45491, but it makes code more cmoplex and plus
since, likely, the release is comming, it is better to revert the
change.

This reverts commit 94604f7, reversing
changes made to 80f6a45.
@robot-ch-test-poll4 robot-ch-test-poll4 added the pr-not-for-changelog This PR should not be mentioned in the changelog label Jan 21, 2023
@tavplubix tavplubix merged commit 1174eaa into ClickHouse:master Jan 21, 2023
@azat
Copy link
Member Author

azat commented Jan 21, 2023

@tavplubix Merged too fast, there was some conflicts although they were trivial and it does complies, but I hope that nothing will pops up.

@azat azat deleted the revert/dist/async-INSERT-metrics branch January 22, 2023 19:34
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