Fix possible assertion when size of precommitted_parts <= precommitted_parts_need_rename

CI founds [1]:

    Logical error: 'precommitted_parts.size() >= precommitted_parts_need_rename.size()'

  [1]: https://s3.amazonaws.com/clickhouse-test-reports/61973/5c1e6a3e956917bdbb7eaa467934e5b75f17a923/stateless_tests__tsan__s3_storage__[5_5].html

The problem is that after precommitted_parts cleaned from detached parts
it may be less then precommitted_parts_need_rename, so to avoid this,
let's just copy it to a new container.

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
This commit is contained in:
Azat Khuzhin 2024-04-11 14:10:13 +02:00
parent 6f522c1d61
commit 6cfd5b2165

View File

@ -6638,19 +6638,21 @@ void MergeTreeData::Transaction::rollback(DataPartsLock * lock)
for (const auto & part : precommitted_parts)
part->version.creation_csn.store(Tx::RolledBackCSN);
auto non_detached_precommitted_parts = precommitted_parts;
/// Remove detached parts from working set.
///
/// It is possible to have detached parts here, only when rename (in
/// commit()) of detached parts had been broken (i.e. during ATTACH),
/// i.e. the part itself is broken.
DataPartsVector detached_precommitted_parts;
for (auto it = precommitted_parts.begin(); it != precommitted_parts.end();)
for (auto it = non_detached_precommitted_parts.begin(); it != non_detached_precommitted_parts.end();)
{
const auto & part = *it;
if (part->getDataPartStorage().getParentDirectory() == DETACHED_DIR_NAME)
{
detached_precommitted_parts.push_back(part);
it = precommitted_parts.erase(it);
it = non_detached_precommitted_parts.erase(it);
}
else
++it;
@ -6658,7 +6660,7 @@ void MergeTreeData::Transaction::rollback(DataPartsLock * lock)
WriteBufferFromOwnString buf;
buf << "Removing parts:";
for (const auto & part : precommitted_parts)
for (const auto & part : non_detached_precommitted_parts)
buf << " " << part->getDataPartStorage().getPartDirectory();
buf << ".";
if (!detached_precommitted_parts.empty())
@ -6679,7 +6681,7 @@ void MergeTreeData::Transaction::rollback(DataPartsLock * lock)
if (!data.all_data_dropped)
{
Strings part_names;
for (const auto & part : precommitted_parts)
for (const auto & part : non_detached_precommitted_parts)
part_names.emplace_back(part->name);
throw Exception(ErrorCodes::LOGICAL_ERROR, "There are some PreActive parts ({}) to rollback, "
"but data parts set is empty and table {} was not dropped. It's a bug",
@ -6693,7 +6695,7 @@ void MergeTreeData::Transaction::rollback(DataPartsLock * lock)
&our_lock);
data.removePartsFromWorkingSet(txn,
DataPartsVector(precommitted_parts.begin(), precommitted_parts.end()),
DataPartsVector(non_detached_precommitted_parts.begin(), non_detached_precommitted_parts.end()),
/* clear_without_timeout = */ true, &our_lock);
}
}