From 9f5015737bcb9fdb3a0d0d8056ca05dfc0c1302a Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Wed, 3 Jan 2024 23:46:13 +0100 Subject: [PATCH 1/2] fix a stupid case of intersecting parts --- src/Storages/MergeTree/MergeTreeData.cpp | 14 +++++++++----- ...02486_truncate_and_unexpected_parts.reference | 2 ++ .../02486_truncate_and_unexpected_parts.sql | 16 ++++++++++++++++ 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index 1c80778f1ca6..a23d59055caa 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -3985,8 +3985,15 @@ MergeTreeData::PartsToRemoveFromZooKeeper MergeTreeData::removePartsInRangeFromW /// FIXME refactor removePartsFromWorkingSet(...), do not remove parts twice removePartsFromWorkingSet(txn, parts_to_remove, clear_without_timeout, lock); + /// We can only create a covering part for a blocks range that starts with 0 (otherwise we may get "intersecting parts" + /// if we remove a range from the middle when dropping a part). + /// Maybe we could do it by incrementing mutation version to get a name for the empty covering part, + /// but it's okay to simply avoid creating it for DROP PART (for a part in the middle). + /// NOTE: Block numbers in ReplicatedMergeTree start from 0. For MergeTree, is_new_syntax is always false. + assert(!create_empty_part || supportsReplication()); + bool range_in_the_middle = drop_range.min_block; bool is_new_syntax = format_version >= MERGE_TREE_DATA_MIN_FORMAT_VERSION_WITH_CUSTOM_PARTITIONING; - if (create_empty_part && !parts_to_remove.empty() && is_new_syntax) + if (create_empty_part && !parts_to_remove.empty() && is_new_syntax && !range_in_the_middle) { /// We are going to remove a lot of parts from zookeeper just after returning from this function. /// And we will remove parts from disk later (because some queries may use them). @@ -3995,12 +4002,9 @@ MergeTreeData::PartsToRemoveFromZooKeeper MergeTreeData::removePartsInRangeFromW /// We don't need to commit it to zk, and don't even need to activate it. MergeTreePartInfo empty_info = drop_range; - empty_info.level = empty_info.mutation = 0; - if (!empty_info.min_block) - empty_info.min_block = MergeTreePartInfo::MAX_BLOCK_NUMBER; + empty_info.min_block = empty_info.level = empty_info.mutation = 0; for (const auto & part : parts_to_remove) { - empty_info.min_block = std::min(empty_info.min_block, part->info.min_block); empty_info.level = std::max(empty_info.level, part->info.level); empty_info.mutation = std::max(empty_info.mutation, part->info.mutation); } diff --git a/tests/queries/0_stateless/02486_truncate_and_unexpected_parts.reference b/tests/queries/0_stateless/02486_truncate_and_unexpected_parts.reference index 2ece1147d789..824d4bbec986 100644 --- a/tests/queries/0_stateless/02486_truncate_and_unexpected_parts.reference +++ b/tests/queries/0_stateless/02486_truncate_and_unexpected_parts.reference @@ -13,3 +13,5 @@ 5 rmt2 7 rmt2 9 rmt2 +1 +3 diff --git a/tests/queries/0_stateless/02486_truncate_and_unexpected_parts.sql b/tests/queries/0_stateless/02486_truncate_and_unexpected_parts.sql index 52e8be236c8f..755cba2a1556 100644 --- a/tests/queries/0_stateless/02486_truncate_and_unexpected_parts.sql +++ b/tests/queries/0_stateless/02486_truncate_and_unexpected_parts.sql @@ -50,3 +50,19 @@ system sync replica rmt1; system sync replica rmt2; select *, _table from merge(currentDatabase(), '') order by _table, (*,); + + +create table rmt3 (n int) engine=ReplicatedMergeTree('/test/02468/{database}3', '1') order by tuple() settings replicated_max_ratio_of_wrong_parts=0, max_suspicious_broken_parts=0, max_suspicious_broken_parts_bytes=0; +set insert_keeper_fault_injection_probability=0; +insert into rmt3 values (1); +insert into rmt3 values (2); +insert into rmt3 values (3); + +system stop cleanup rmt3; +alter table rmt3 drop part 'all_1_1_0'; +optimize table rmt3 final; + +detach table rmt3 sync; +attach table rmt3; + +select * from rmt3 order by n; From bc1c05e4cd4492d6e8735345000d3a50809047d1 Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Thu, 4 Jan 2024 20:15:52 +0100 Subject: [PATCH 2/2] Update 02486_truncate_and_unexpected_parts.sql --- .../queries/0_stateless/02486_truncate_and_unexpected_parts.sql | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/queries/0_stateless/02486_truncate_and_unexpected_parts.sql b/tests/queries/0_stateless/02486_truncate_and_unexpected_parts.sql index 755cba2a1556..5c90313b6b87 100644 --- a/tests/queries/0_stateless/02486_truncate_and_unexpected_parts.sql +++ b/tests/queries/0_stateless/02486_truncate_and_unexpected_parts.sql @@ -59,6 +59,7 @@ insert into rmt3 values (2); insert into rmt3 values (3); system stop cleanup rmt3; +system sync replica rmt3 pull; alter table rmt3 drop part 'all_1_1_0'; optimize table rmt3 final;