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

Parse partition key value from partition_id when need to create part in empty partition #31887

Merged
merged 4 commits into from
Dec 1, 2021

Conversation

tavplubix
Copy link
Member

@tavplubix tavplubix commented Nov 26, 2021

Changelog category (leave one):

  • Bug Fix (user-visible misbehaviour in official stable or prestable release)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Some GET_PART entry might hang in replication queue if part is lost on all replicas and there are no other parts in the same partition. It's fixed in cases when partition key contains only columns of integer types or Date[Time]. Fixes #31485.

@robot-clickhouse robot-clickhouse added the pr-bugfix Pull request with bugfix, not backported by default label Nov 26, 2021
@novikd novikd self-assigned this Nov 26, 2021
@tavplubix
Copy link
Member Author

Integration tests - test_lost_part/test.py::test_lost_last_part - need to update the test
Performance - a query from flat_dictionary.xml has unexpected duration, cc: @kitaisreal
Stateless tests flaky check (address, actions) - OK
Stress test (thread, actions) - #31958
Stress test (undefined, actions) - Memory limit (total) exceeded
Unit tests (tsan, actions) - Executor.RemoveTasksStress is flaky, cc: @nikitamikhaylov

Copy link
Member

@novikd novikd left a comment

Choose a reason for hiding this comment

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

LGTM, but I didn't understand why do we need changes in MergeTreeData::getPartitionIDFromQuery.

src/Storages/MergeTree/MergeTreePartition.cpp Outdated Show resolved Hide resolved
@tavplubix
Copy link
Member Author

LGTM, but I didn't understand why do we need changes in MergeTreeData::getPartitionIDFromQuery.

Changes in getPartitionIDFromQuery are not related to parsing of partition_id and are not really necessary. I just don't like the way it was implemented before, because saving substring of a query into fields_str in ParserPartition is a hack and using Values format to parse partition expression again also looks strange.

@tavplubix
Copy link
Member Author

Integration tests flaky check (asan) — Failed to read test_results.tsv, cc: @alesapin
Functional stateless tests (release, DatabaseReplicated) - 00993_system_parts_race_condition_drop_zookeeper - #29616, I'm trying to fix it
Stateless tests (release, actions) - 00925_zookeeper_empty_replicated_merge_tree_optimize_final_long is flaky - OPTIMIZE cannot be done on this replica because it is not a leader - #32050
Stateless tests flaky check (address, actions) - Test runs too long - OK
Stress test (thread, actions) - Memory limit (total) exceeded
Unit tests (tsan, actions) - Executor.RemoveTasksStress is flaky, already reported

@tavplubix tavplubix merged commit b623a38 into master Dec 1, 2021
@tavplubix tavplubix deleted the fix_cannot_create_empty_part branch December 1, 2021 12:38
robot-clickhouse pushed a commit that referenced this pull request Dec 1, 2021
…d` when need to create part in empty partition
robot-clickhouse pushed a commit that referenced this pull request Dec 1, 2021
…` when need to create part in empty partition
robot-clickhouse pushed a commit that referenced this pull request Dec 1, 2021
…d` when need to create part in empty partition
tavplubix added a commit that referenced this pull request Dec 2, 2021
Backport #31887 to 21.11: Parse partition key value from `partition_id` when need to create part in empty partition
tavplubix added a commit that referenced this pull request Dec 2, 2021
Backport #31887 to 21.9: Parse partition key value from `partition_id` when need to create part in empty partition
robot-clickhouse pushed a commit that referenced this pull request Dec 2, 2021
…` when need to create part in empty partition
tavplubix added a commit that referenced this pull request Dec 3, 2021
Backport #31887 to 21.8: Parse partition key value from `partition_id` when need to create part in empty partition
tavplubix added a commit that referenced this pull request Dec 5, 2021
Backport #31887 to 21.10: Parse partition key value from `partition_id` when need to create part in empty partition
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-bugfix Pull request with bugfix, not backported by default
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve lost parts replacement with empty parts
4 participants