thevar1able/ClickHouse
1
0
Fork 0
mirror of https://github.com/ClickHouse/ClickHouse.git synced 2024-11-23 16:12:01 +00:00
Code Issues Packages Projects Releases Wiki Activity

slightly beter fix

Browse Source
This commit is contained in:
Alexander Tokmakov 2022-04-11 16:25:59 +02:00
parent 2f11c5323e
commit 38c80940fd
2 changed files with 13 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
src/Interpreters/MergeTreeTransaction.cpp
View File

@ -250,21 +250,6 @@ bool MergeTreeTransaction::rollback() noexcept
/// Discard changes in active parts set
/// Remove parts that were created, restore parts that were removed (except parts that were created by this transaction too)
for (const auto & part : parts_to_remove)
{
if (part->version.isRemovalTIDLocked())
{
/// Don't need to remove part from working set if it was created and removed by this transaction
assert(part->version.removal_tid_lock == tid.getHash());
continue;
}
/// FIXME do not lock removal_tid when rolling back part creation, it's ugly
const_cast<MergeTreeData &>(part->storage).removePartsFromWorkingSet(NO_TRANSACTION_RAW, {part}, true);
}
for (const auto & part : parts_to_activate)
if (part->version.getCreationTID() != tid)
const_cast<MergeTreeData &>(part->storage).restoreAndActivatePart(part);
/// Kind of optimization: cleanup thread can remove these parts immediately
for (const auto & part : parts_to_remove)
@ -274,6 +259,18 @@ bool MergeTreeTransaction::rollback() noexcept
part->appendCSNToVersionMetadata(VersionMetadata::CREATION);
}
for (const auto & part : parts_to_remove)
{
/// NOTE It's possible that part is already removed from working set in the same transaction
/// (or, even worse, in a separate non-transactional query with PrehistoricTID),
/// but it's not a problem: removePartsFromWorkingSet(...) will do nothing in this case.
const_cast<MergeTreeData &>(part->storage).removePartsFromWorkingSet(NO_TRANSACTION_RAW, {part}, true);
}
for (const auto & part : parts_to_activate)
if (part->version.getCreationTID() != tid)
const_cast<MergeTreeData &>(part->storage).restoreAndActivatePart(part);
for (const auto & part : parts_to_activate)
{
/// Clear removal_tid from version metadata file, so we will not need to distinguish TIDs that were not committed

3
tests/queries/0_stateless/01172_transaction_counters.reference
View File

@ -6,7 +6,7 @@
3 all_1_1_0 0
3 all_3_3_0 1
4 all_1_1_0 1 (0,0,'00000000-0000-0000-0000-000000000000') 0
4 all_2_2_0 18446744073709551615 (1,1,'00000000-0000-0000-0000-000000000000') 0
4 all_2_2_0 18446744073709551615 (0,0,'00000000-0000-0000-0000-000000000000') 0
4 all_3_3_0 0 (0,0,'00000000-0000-0000-0000-000000000000') 0
5 1
6 all_1_1_0 0
@ -19,7 +19,6 @@
1 1 AddPart 1 1 1 1 all_1_1_0
2 1 Begin 1 1 1 1
2 1 AddPart 1 1 1 1 all_2_2_0
1 1 LockPart 1 1 1 1 all_2_2_0
2 1 Rollback 1 1 1 1
3 1 Begin 1 1 1 1
3 1 AddPart 1 1 1 1 all_3_3_0