diff --git a/src/Interpreters/MergeTreeTransaction.cpp b/src/Interpreters/MergeTreeTransaction.cpp index b2f1d8ae150..91e4b51ea07 100644 --- a/src/Interpreters/MergeTreeTransaction.cpp +++ b/src/Interpreters/MergeTreeTransaction.cpp @@ -85,7 +85,7 @@ void MergeTreeTransaction::removeOldPart(const StoragePtr & storage, const DataP { CSN c = csn.load(); if (c == Tx::RolledBackCSN) - throw Exception(ErrorCodes::INVALID_TRANSACTION, "Transaction was cancelled"); + throw Exception(ErrorCodes::INVALID_TRANSACTION, "Transaction was cancelled");//FIXME else if (c != Tx::UnknownCSN) throw Exception(ErrorCodes::LOGICAL_ERROR, "Unexpected CSN state: {}", c); diff --git a/src/Interpreters/TransactionVersionMetadata.cpp b/src/Interpreters/TransactionVersionMetadata.cpp index c8e6b9dcfba..59d3b1ef4b9 100644 --- a/src/Interpreters/TransactionVersionMetadata.cpp +++ b/src/Interpreters/TransactionVersionMetadata.cpp @@ -38,6 +38,7 @@ TransactionID VersionMetadata::getMaxTID() const void VersionMetadata::lockMaxTID(const TransactionID & tid, const String & error_context) { + //LOG_TRACE(&Poco::Logger::get("WTF"), "Trying to lock maxtid by {}: {}\n{}", tid, error_context, StackTrace().toString()); TIDHash locked_by = 0; if (tryLockMaxTID(tid, &locked_by)) return; @@ -75,6 +76,7 @@ bool VersionMetadata::tryLockMaxTID(const TransactionID & tid, TIDHash * locked_ void VersionMetadata::unlockMaxTID(const TransactionID & tid) { + //LOG_TRACE(&Poco::Logger::get("WTF"), "Unlocking maxtid by {}", tid); assert(!tid.isEmpty()); TIDHash max_lock_value = tid.getHash(); TIDHash locked_by = maxtid_lock.load(); diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index c31f399522f..31d4d5a8098 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -1663,11 +1663,15 @@ size_t MergeTreeData::clearEmptyParts() auto parts = getDataPartsVector(); for (const auto & part : parts) { - if (part->rows_count == 0) - { - dropPartNoWaitNoThrow(part->name); - ++cleared_count; - } + if (part->rows_count != 0) + continue; + + /// Do not drop empty part if it may be visible for some transaction (otherwise it may cause conflicts) + if (!part->versions.canBeRemoved(TransactionLog::instance().getOldestSnapshot())) + continue; + + dropPartNoWaitNoThrow(part->name); + ++cleared_count; } return cleared_count; } @@ -2734,7 +2738,8 @@ MergeTreeData::DataPartsVector MergeTreeData::removePartsInRangeFromWorkingSet( void MergeTreeData::restoreAndActivatePart(const DataPartPtr & part, DataPartsLock * acquired_lock) { auto lock = (acquired_lock) ? DataPartsLock() : lockParts(); //-V1018 - assert(part->getState() != DataPartState::Committed); + if (part->getState() == DataPartState::Committed) + return; addPartContributionToColumnAndSecondaryIndexSizes(part); addPartContributionToDataVolume(part); modifyPartState(part, DataPartState::Committed); diff --git a/tests/queries/0_stateless/01168_mutations_isolation.reference b/tests/queries/0_stateless/01168_mutations_isolation.reference index 582a42418d6..56e7264b174 100644 --- a/tests/queries/0_stateless/01168_mutations_isolation.reference +++ b/tests/queries/0_stateless/01168_mutations_isolation.reference @@ -16,5 +16,19 @@ SERIALIZATION_ERROR 6 2 all_1_1_0_12 6 6 all_7_7_0_12 7 20 all_1_1_0_14 +7 40 all_15_15_0 7 60 all_7_7_0_14 7 80 all_13_13_0_14 +8 20 all_1_15_1_14 +8 40 all_1_15_1_14 +8 60 all_1_15_1_14 +8 80 all_1_15_1_14 +INVALID_TRANSACTION +9 21 all_1_15_1_18 +9 41 all_1_15_1_18 +9 61 all_1_15_1_18 +9 81 all_1_15_1_18 +10 22 all_1_15_1_19 +10 42 all_1_15_1_19 +10 62 all_1_15_1_19 +10 82 all_1_15_1_19 diff --git a/tests/queries/0_stateless/01168_mutations_isolation.sh b/tests/queries/0_stateless/01168_mutations_isolation.sh index 9791e2aa079..d4d76e91ee0 100755 --- a/tests/queries/0_stateless/01168_mutations_isolation.sh +++ b/tests/queries/0_stateless/01168_mutations_isolation.sh @@ -49,9 +49,36 @@ tx 6 "select 6, n, _part from mt orde tx 5 "rollback" tx 6 "insert into mt values (8)" tx 6 "alter table mt update n=n*10 where 1" +tx 6 "insert into mt values (40)" tx 6 "commit" -tx 8 "begin transaction" -tx 8 "select 7, n, _part from mt order by n" + +tx 7 "begin transaction" +tx 7 "select 7, n, _part from mt order by n" +tx 8 "begin transaction" +tx 8 "alter table mt update n = 0 where 1" +$CLICKHOUSE_CLIENT -q "kill mutation where database=currentDatabase() and mutation_id='mutation_16.txt' format Null" +tx 7 "optimize table mt final" +tx 7 "select 8, n, _part from mt order by n" +tx 8 "commit" | grep -Eo "INVALID_TRANSACTION" | uniq +tx 8 "rollback" +tx 10 "begin transaction" +tx 10 "alter table mt update n = 0 where 1" +tx 7 "alter table mt update n=n+1 where 1" +tx 10 "commit" | grep -Eo "INVALID_TRANSACTION" | uniq +tx 10 "rollback" +tx 7 "commit" + + +tx 11 "begin transaction" +tx 11 "select 9, n, _part from mt order by n" +tx 12 "begin transaction" +tx 11 "alter table mt update n=n+1 where 1" +tx 12 "alter table mt update n=n+1 where 1" +tx 11 "commit" >/dev/null +tx 12 "commit" >/dev/null + +tx 11 "begin transaction" +tx 11 "select 10, n, _part from mt order by n" $CLICKHOUSE_CLIENT --database_atomic_wait_for_drop_and_detach_synchronously=0 -q "drop table mt"