Fix flaky test_disallow_concurrency by pamarcos · Pull Request #93612 · ClickHouse/ClickHouse

pamarcos · 2026-01-07T17:00:10Z

Fix LOGICAL_ERROR when restoring ReplicatedMergeTree with deduplication race

When restoring a ReplicatedMergeTree table ON CLUSTER, multiple replicas
restore the same parts concurrently. If one replica commits a part to
ZooKeeper first, the other replica's part gets deduplicated. The code
assumed deduplicated parts always came from ATTACH PART (located in
detached/attaching_*) and threw LOGICAL_ERROR for other paths, causing
the server to abort.

During restore, parts are in tmp_restore_* directories instead. Those parts
are now properly deduplicated as well.

Log traces can be found in the logs of any of the failures:

Logical error: 'Unexpected relative path for a deduplicated part: 
store/813/81355abf-88f2-4516-9658-3396821e02f9/tmp_restore_all_36_41_1-781a6406-308b-4a6f-b253-aa6ea64efcb8/'.

In 89da254 I changed the flaky check to run 1000 times to ensure the tests are not flaky anymore. Considering from prior failures) a 2-8% failure rate, we should see at least some failures if the flakiness was still there. Fortunately, all tests succeeded after 5h of job running:

❯ grep "%] PASS" job.log | wc -l
2140

❯ grep "%] FAIL" job.log | wc -l
0

❯ grep "%] ERR" job.log | wc -l
0

Closes #93413
Closes #93235
Closes #93177
Closes #68012
Closes #93549
Closes #93550

Changelog category (leave one):

Critical Bug Fix (crash, data loss, RBAC) or LOGICAL_ERROR

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fix LOGICAL_ERROR when restoring ReplicatedMergeTree with deduplication race

Documentation entry for user-facing changes

Documentation is written (mandatory for new features)

…on race When restoring a ReplicatedMergeTree table ON CLUSTER, multiple replicas restore the same parts concurrently. If one replica commits a part to ZooKeeper first, the other replica's part gets deduplicated. The code assumed deduplicated parts always came from ATTACH PART (located in detached/attaching_*) and threw LOGICAL_ERROR for other paths, causing the server to abort. During restore, parts are in tmp_restore_* directories instead. Now deduplicated parts from restore are simply removed since the data already exists on another replica and will be fetched via normal replication.

clickhouse-gh · 2026-01-07T17:00:41Z

Workflow [PR], commit [d2b3915]

Summary: ❌

job_name	test_name	status	info	comment
Integration tests (amd_asan, db disk, old analyzer, 6/6)		failure
	test_max_temporary_data_size_on_disk/test.py::test	FAIL	cidb, issue	ISSUE CREATED
Integration tests (amd_asan, targeted)		error		IGNORED

Copilot

Pull request overview

This PR fixes a LOGICAL_ERROR crash that occurred when restoring ReplicatedMergeTree tables ON CLUSTER due to a deduplication race condition between replicas.

Key Changes:

Fixed handling of deduplicated parts during concurrent restore operations by removing them instead of throwing LOGICAL_ERROR
Removed unused import from test file
Increased flaky test repeat counts to better catch concurrency issues

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/Storages/MergeTree/ReplicatedMergeTreeSink.cpp	Fixed deduplication logic to handle parts from RESTORE operations by removing them instead of throwing LOGICAL_ERROR
tests/integration/test_backup_restore_on_cluster/test_disallow_concurrency.py	Removed unused `typing.List` import
ci/jobs/integration_test_job.py	Increased flaky check repeat counts for better test coverage

ci/jobs/integration_test_job.py

Copilot