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

Fix finished_mutations_to_keep=0 for MergeTree (as docs says 0 is to keep everything) #60031

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 4 additions & 5 deletions src/Storages/StorageMergeTree.cpp
Expand Up @@ -1479,8 +1479,11 @@ UInt64 StorageMergeTree::getCurrentMutationVersion(

size_t StorageMergeTree::clearOldMutations(bool truncate)
{
size_t finished_mutations_to_keep = truncate ? 0 : getSettings()->finished_mutations_to_keep;
size_t finished_mutations_to_keep = getSettings()->finished_mutations_to_keep;
if (!truncate && !finished_mutations_to_keep)
return 0;

finished_mutations_to_keep = truncate ? 0 : finished_mutations_to_keep;
std::vector<MergeTreeMutationEntry> mutations_to_delete;
{
std::lock_guard lock(currently_processing_in_background_mutex);
Expand Down Expand Up @@ -1899,8 +1902,6 @@ void StorageMergeTree::dropPart(const String & part_name, bool detach, ContextPt
}
}

/// Old part objects is needed to be destroyed before clearing them from filesystem.
clearOldMutations(true);
clearOldPartsFromFilesystem();
clearEmptyParts();
}
Expand Down Expand Up @@ -1985,8 +1986,6 @@ void StorageMergeTree::dropPartition(const ASTPtr & partition, bool detach, Cont
}
}

/// Old parts are needed to be destroyed before clearing them from filesystem.
azat marked this conversation as resolved.
Show resolved Hide resolved
clearOldMutations(true);
clearOldPartsFromFilesystem();
clearEmptyParts();
}
Expand Down
Expand Up @@ -131,14 +131,13 @@ def test_all_projection_files_are_dropped_when_part_is_dropped(
"""
)

objects_empty_table = list_objects(cluster)

node.query(
"ALTER TABLE test_all_projection_files_are_dropped ADD projection b_order (SELECT a, b ORDER BY b)"
)
node.query(
"ALTER TABLE test_all_projection_files_are_dropped MATERIALIZE projection b_order"
)
objects_empty_table = list_objects(cluster)

node.query(
"""
Expand Down
@@ -0,0 +1,4 @@
mutations after ALTER for data_rmt 1
mutations after cleanup for data_rmt 1
mutations after ALTER for data_mt 1
mutations after cleanup for data_mt 1
@@ -0,0 +1,14 @@
drop table if exists data_rmt;
drop table if exists data_mt;

create table data_rmt (key Int) engine=ReplicatedMergeTree('/tables/{database}/data', '{table}') order by tuple() settings finished_mutations_to_keep=0, merge_tree_clear_old_parts_interval_seconds=1;
create table data_mt (key Int) engine=MergeTree() order by tuple() settings finished_mutations_to_keep=0, merge_tree_clear_old_parts_interval_seconds=1;

{% for table in ['data_rmt', 'data_mt'] %}
alter table {{table}} delete where 1 settings mutations_sync = 1;
select 'mutations after ALTER for {{table}}', count() from system.mutations where database = currentDatabase() and table = '{{table}}';
-- merge_tree_clear_old_parts_interval_seconds=1, but wait few seconds more
select sleep(5) settings function_sleep_max_microseconds_per_block=10e6 format Null;
select 'mutations after cleanup for {{table}}', count() from system.mutations where database = currentDatabase() and table = '{{table}}';
drop table {{table}};
{% endfor %}