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

More correct alters for non replicated MergeTree #8361

Merged
merged 5 commits into from Dec 25, 2019
Merged

Conversation

alesapin
Copy link
Member

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Non-significant (changelog entry is not needed)

@alesapin alesapin marked this pull request as ready for review December 24, 2019 18:09
@alesapin alesapin changed the title [WIP] More correct alters for non replicated MergeTree More correct alters for non replicated MergeTree Dec 24, 2019
* Taking this mutex means that we want to lock columns_lock on read with intention then, not
* unblocking, block it for writing.
*/
mutable std::mutex alter_mutex;
Copy link
Member Author

@alesapin alesapin Dec 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this clumsy mutex and corresponding lock, because they are not required (anymore?). Data part cannot be altered from different threads simultaneously because for non replicated MergeTree we execute them sequentially with alter_intention_lock and for ReplicatedMergeTree we use ReplicatedMergeTreeAltreThread executed in schedule_pool which also cannot run concurrently.

UPD: Actually data can be altered concurrently (ALTER PARTITION and ALTER MODIFY COLUMN), but I don't find any cases where alter thread can take columns_lock for read, and then upgrade this lock for write. BTW columns lock will be completely removed with alter on top of mutations.

@alesapin
Copy link
Member Author

alesapin commented Dec 25, 2019

test_parts_delete_zookeeper/test.py::test_merge_doesnt_work_without_zookeeper -- flap.

@alesapin alesapin merged commit fe6a63a into master Dec 25, 2019
@vitlibar vitlibar added the pr-bugfix Pull request with bugfix, not backported by default label Dec 26, 2019
@alesapin alesapin added pr-non-significant and removed pr-bugfix Pull request with bugfix, not backported by default labels Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants