Fix 00975_move_partition_merge_tree #30717
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…eTree" This reverts commit e9ce859.
robot-clickhouse
added
the
pr-bugfix
azat
reviewed
Oct 26, 2021
| auto lock1 = lockForShare(local_context->getCurrentQueryId(), local_context->getSettingsRef().lock_acquire_timeout); | ||
| auto lock2 = dest_table->lockForShare(local_context->getCurrentQueryId(), local_context->getSettingsRef().lock_acquire_timeout); | ||
| auto merges_blocker = stopMergesAndWait(); |
There was a problem hiding this comment.
Thank you!
P.S. I though that this will be too much... And by some reason I though that holding currently_processing_in_background_mutex was enough, which is obviously is incorrect, since new part will be commited only when it will be written/renamed.
src/Storages/StorageMergeTree.cpp
Outdated
| @@ -1470,6 +1468,7 @@ void StorageMergeTree::movePartitionToTable(const StoragePtr & dest_table, const | |||
| transaction.commit(&lock); | |||
| } | |||
|
|
|||
| clearOldMutations(true); | |||
There was a problem hiding this comment.
BTW it seems that this is redundant? (I see that it comes from reverting my patch initially, but maybe it should be removed?)
There was a problem hiding this comment.
They will be cleared in the background.
|
Test fixed here #30723 |
alesapin
approved these changes
Oct 27, 2021
src/Storages/StorageMergeTree.cpp
Outdated
| @@ -1470,6 +1468,7 @@ void StorageMergeTree::movePartitionToTable(const StoragePtr & dest_table, const | |||
| transaction.commit(&lock); | |||
| } | |||
|
|
|||
| clearOldMutations(true); | |||
There was a problem hiding this comment.
They will be cleared in the background.
|
AST fuzzer (MSan) - #30756 |
This was referenced Oct 27, 2021
robot-clickhouse
pushed a commit
that referenced
this pull request
Oct 27, 2021
robot-clickhouse
pushed a commit
that referenced
this pull request
Oct 27, 2021
robot-clickhouse
pushed a commit
that referenced
this pull request
Oct 27, 2021
tavplubix
added a commit
that referenced
this pull request
Oct 28, 2021
Backport #30717 to 21.10: Fix 00975_move_partition_merge_tree
tavplubix
added a commit
that referenced
this pull request
Oct 28, 2021
Backport #30717 to 21.8: Fix 00975_move_partition_merge_tree
tavplubix
added a commit
that referenced
this pull request
Oct 28, 2021
Backport #30717 to 21.9: Fix 00975_move_partition_merge_tree
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fixed a race condition between
REPLACE/MOVE PARTITIONand background merge in non-replicatedMergeTreethat might cause a part of moved/replaced data to remain in partition. Fixes #29327.Detailed description / Documentation draft:
Previous fix was incorrect, because locking
currently_processing_in_background_mutexdoes not cancel already started merges (and does not fix similar issue with replace partition).