thevar1able/ClickHouse
1
0
Fork 0
mirror of https://github.com/ClickHouse/ClickHouse.git synced 2024-09-20 08:40:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix possible deadlocks with MergeTreeData::Transaction

Browse Source
This commit is contained in:
alesapin 2022-07-01 15:16:32 +02:00
parent 016176dc02
commit 57284e6a9d
3 changed files with 17 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/Storages/MergeTree/MergeTreeSink.cpp
View File

@ -139,6 +139,9 @@ void MergeTreeSink::finishDelayedChunk()
bool added = false; bool added = false;
/// It's important to create it outside of lock scope because
/// otherwise it can lock parts in desctructor and deadlock is possible.
MergeTreeData::Transaction transaction(storage, context->getCurrentTransaction().get());
{ {
auto lock = storage.lockParts(); auto lock = storage.lockParts();
storage.fillNewPartName(part, lock); storage.fillNewPartName(part, lock);
@ -155,19 +158,15 @@ void MergeTreeSink::finishDelayedChunk()
} }
else else
{ {
MergeTreeData::Transaction transaction(storage, context->getCurrentTransaction().get());
added = storage.renameTempPartAndAdd(part, transaction, lock); added = storage.renameTempPartAndAdd(part, transaction, lock);
transaction.commit(&lock); transaction.commit(&lock);
} }
} }
else else
{ {
MergeTreeData::Transaction transaction(storage, context->getCurrentTransaction().get());
added = storage.renameTempPartAndAdd(part, transaction, lock); added = storage.renameTempPartAndAdd(part, transaction, lock);
transaction.commit(&lock); transaction.commit(&lock);
} }
} }
/// Part can be deduplicated, so increment counters and add to part log only if it's really added /// Part can be deduplicated, so increment counters and add to part log only if it's really added

2
src/Storages/MergeTree/ReplicatedMergeTreeSink.cpp
View File

@ -471,6 +471,8 @@ void ReplicatedMergeTreeSink::commitPart(
/// Information about the part. /// Information about the part.
storage.getCommitPartOps(ops, part, block_id_path); storage.getCommitPartOps(ops, part, block_id_path);
/// It's important to create it outside of lock scope because
/// otherwise it can lock parts in desctructor and deadlock is possible.
MergeTreeData::Transaction transaction(storage, NO_TRANSACTION_RAW); /// If you can not add a part to ZK, we'll remove it back from the working set. MergeTreeData::Transaction transaction(storage, NO_TRANSACTION_RAW); /// If you can not add a part to ZK, we'll remove it back from the working set.
bool renamed = false; bool renamed = false;

17
src/Storages/StorageMergeTree.cpp
View File

@ -1540,9 +1540,11 @@ PartitionCommandsResultInfo StorageMergeTree::attachPartition(
loaded_parts[i]->storeVersionMetadata(); loaded_parts[i]->storeVersionMetadata();
String old_name = renamed_parts.old_and_new_names[i].old_name; String old_name = renamed_parts.old_and_new_names[i].old_name;
/// It's important to create it outside of lock scope because
/// otherwise it can lock parts in desctructor and deadlock is possible.
MergeTreeData::Transaction transaction(*this, local_context->getCurrentTransaction().get());
{ {
auto lock = lockParts(); auto lock = lockParts();
MergeTreeData::Transaction transaction(*this, local_context->getCurrentTransaction().get());
fillNewPartName(loaded_parts[i], lock); fillNewPartName(loaded_parts[i], lock);
renameTempPartAndAdd(loaded_parts[i], transaction, lock); renameTempPartAndAdd(loaded_parts[i], transaction, lock);
transaction.commit(&lock); transaction.commit(&lock);
@ -1797,13 +1799,18 @@ CheckResults StorageMergeTree::checkData(const ASTPtr & query, ContextPtr local_
void StorageMergeTree::attachRestoredParts(MutableDataPartsVector && parts) void StorageMergeTree::attachRestoredParts(MutableDataPartsVector && parts)
{ {
for (auto part : parts) for (auto part : parts)
{ {
auto lock = lockParts(); /// It's important to create it outside of lock scope because
/// otherwise it can lock parts in desctructor and deadlock is possible.
MergeTreeData::Transaction transaction(*this, NO_TRANSACTION_RAW); MergeTreeData::Transaction transaction(*this, NO_TRANSACTION_RAW);
fillNewPartName(part, lock); {
renameTempPartAndAdd(part, transaction, lock); auto lock = lockParts();
transaction.commit(&lock); fillNewPartName(part, lock);
renameTempPartAndAdd(part, transaction, lock);
transaction.commit(&lock);
}
} }
} }