Merge pull request #28864 from ClickHouse/fix_replace_range_drop_part_reordering

Fix reordering of REPLACE_RANGE and DROP PART
This commit is contained in:
alesapin 2021-09-13 15:11:29 +03:00 committed by GitHub
commit 1d05fab295
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 35 additions and 0 deletions

View File

@ -1254,6 +1254,37 @@ bool ReplicatedMergeTreeQueue::shouldExecuteLogEntry(
out_postpone_reason = fmt::format(format_str, entry.znode_name, entry.typeToString(), entry.new_part_name);
return false;
}
if (entry.isDropPart(format_version))
{
/// We should avoid reordering of REPLACE_RANGE and DROP PART (DROP_RANGE),
/// because if replace_range_entry->new_part_names contains drop_range_entry->new_part_name
/// and we execute DROP PART before REPLACE_RANGE, then DROP PART will be no-op
/// (because part is not created yet, so there is nothing to drop;
/// DROP_RANGE does not cover all parts of REPLACE_RANGE, so removePartProducingOpsInRange(...) will not remove anything too)
/// and part will never be removed. Replicas may diverge due to such reordering.
/// We don't need to do anything for other entry types, because removePartProducingOpsInRange(...) will remove them as expected.
auto drop_part_info = MergeTreePartInfo::fromPartName(entry.new_part_name, format_version);
for (const auto & replace_entry : queue)
{
if (replace_entry->type != LogEntry::REPLACE_RANGE)
continue;
for (const auto & new_part_name : replace_entry->replace_range_entry->new_part_names)
{
auto new_part_info = MergeTreePartInfo::fromPartName(new_part_name, format_version);
if (!new_part_info.isDisjoint(drop_part_info))
{
const char * format_str = "Not executing log entry {} of type {} for part {} "
"because it probably depends on {} (REPLACE_RANGE).";
LOG_TRACE(log, format_str, entry.znode_name, entry.typeToString(), entry.new_part_name, replace_entry->znode_name);
out_postpone_reason = fmt::format(format_str, entry.znode_name, entry.typeToString(), entry.new_part_name, replace_entry->znode_name);
return false;
}
}
}
}
}
return true;

View File

@ -2199,7 +2199,11 @@ void StorageReplicatedMergeTree::executeDropRange(const LogEntry & entry)
auto data_parts_lock = lockParts();
parts_to_remove = removePartsInRangeFromWorkingSet(drop_range_info, true, data_parts_lock);
if (parts_to_remove.empty())
{
if (!drop_range_info.isFakeDropRangePart())
LOG_INFO(log, "Log entry {} tried to drop single part {}, but part does not exist", entry.znode_name, entry.new_part_name);
return;
}
}
if (entry.detach)