mirror of
https://github.com/ClickHouse/ClickHouse.git
synced 2024-11-24 08:32:02 +00:00
Eliminate possible race between ALTER_METADATA and MERGE_PARTS
v2: move metadata version check after checking that the part is not covering part Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
This commit is contained in:
parent
7efe413575
commit
a12df35be4
@ -43,6 +43,8 @@ ReplicatedMergeMutateTaskBase::PrepareResult MergeFromLogEntryTask::prepare()
|
||||
LOG_TRACE(log, "Executing log entry to merge parts {} to {}",
|
||||
fmt::join(entry.source_parts, ", "), entry.new_part_name);
|
||||
|
||||
StorageMetadataPtr metadata_snapshot = storage.getInMemoryMetadataPtr();
|
||||
int32_t metadata_version = metadata_snapshot->getMetadataVersion();
|
||||
const auto storage_settings_ptr = storage.getSettings();
|
||||
|
||||
if (storage_settings_ptr->always_fetch_merged_part)
|
||||
@ -129,6 +131,18 @@ ReplicatedMergeMutateTaskBase::PrepareResult MergeFromLogEntryTask::prepare()
|
||||
};
|
||||
}
|
||||
|
||||
int32_t part_metadata_version = source_part_or_covering->getMetadataVersion();
|
||||
if (part_metadata_version > metadata_version)
|
||||
{
|
||||
LOG_DEBUG(log, "Source part metadata version {} is newer then the table metadata version {}. ALTER_METADATA is still in progress.",
|
||||
part_metadata_version, metadata_version);
|
||||
return PrepareResult{
|
||||
.prepared_successfully = false,
|
||||
.need_to_check_missing_part_in_fetch = false,
|
||||
.part_log_writer = {}
|
||||
};
|
||||
}
|
||||
|
||||
parts.push_back(source_part_or_covering);
|
||||
}
|
||||
|
||||
@ -176,8 +190,6 @@ ReplicatedMergeMutateTaskBase::PrepareResult MergeFromLogEntryTask::prepare()
|
||||
/// It will live until the whole task is being destroyed
|
||||
table_lock_holder = storage.lockForShare(RWLockImpl::NO_QUERY, storage_settings_ptr->lock_acquire_timeout_for_background_operations);
|
||||
|
||||
StorageMetadataPtr metadata_snapshot = storage.getInMemoryMetadataPtr();
|
||||
|
||||
auto future_merged_part = std::make_shared<FutureMergedMutatedPart>(parts, entry.new_part_format);
|
||||
if (future_merged_part->name != entry.new_part_name)
|
||||
{
|
||||
|
@ -1745,14 +1745,12 @@ bool StorageReplicatedMergeTree::checkPartChecksumsAndAddCommitOps(
|
||||
|
||||
if (replica_part_header.getColumnsHash() != local_part_header.getColumnsHash())
|
||||
{
|
||||
/// Currently there are two (known) cases when it may happen:
|
||||
/// Currently there are only one (known) cases when it may happen:
|
||||
/// - KILL MUTATION query had removed mutation before all replicas have executed assigned MUTATE_PART entries.
|
||||
/// Some replicas may skip this mutation and update part version without actually applying any changes.
|
||||
/// It leads to mismatching checksum if changes were applied on other replicas.
|
||||
/// - ALTER_METADATA and MERGE_PARTS were reordered on some replicas.
|
||||
/// It may lead to different number of columns in merged parts on these replicas.
|
||||
throw Exception(ErrorCodes::CHECKSUM_DOESNT_MATCH, "Part {} from {} has different columns hash "
|
||||
"(it may rarely happen on race condition with KILL MUTATION or ALTER COLUMN).", part_name, replica);
|
||||
"(it may rarely happen on race condition with KILL MUTATION).", part_name, replica);
|
||||
}
|
||||
|
||||
replica_part_header.getChecksums().checkEqual(local_part_header.getChecksums(), true);
|
||||
|
@ -1 +0,0 @@
|
||||
all_0_2_2_1 RegularMerge MergeParts CHECKSUM_DOESNT_MATCH
|
Loading…
Reference in New Issue
Block a user