Skip to content

Fix data race in DataPartStorageOnDiskBase::remove vs system.parts #94262

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
alexey-milovidov merged 3 commits into from
Jan 16, 2026
Merged

Fix data race in DataPartStorageOnDiskBase::remove vs system.parts #94262

alexey-milovidov merged 3 commits into from
Jan 16, 2026

Conversation

@alexey-milovidov
Copy link
Member

@alexey-milovidov alexey-milovidov commented Jan 14, 2026 •
edited
Loading

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 data race in DataPartStorageOnDiskBase::remove vs system.parts. Closes #49076.

The test reproduces the problem successfully. Prompts: https://pastila.nl/?0009496a/7630a430e8273325d3acd6bfc71045d3#Iv1vnXsu1NFcfdLKPyJkFw==GCM

alexey-milovidov and others added 2 commits January 14, 2026 22:32
@alexey-milovidov @claude
Fix data race in DataPartStorageOnDiskBase::remove()
6e4b80a 
The data race occurred between:
- Thread modifying part_dir in remove() after renaming directory
- Thread reading part_dir in getFullPath() from system.parts queries

The assignment to part_dir is unnecessary because:
- The 'to' variable is already computed and used for all subsequent operations
- The part is being removed, so the path doesn't need to be updated

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@alexey-milovidov @claude
Add test for data race in system.parts during part removal
54c92ad 
This test aims to trigger a race condition between:
- Reading system.parts table (calling getFullPath())
- Removing parts (calling remove() which modifies part_dir)

The test creates a table with minimal part lifetime, then runs
concurrent inserts, partition drops, and system.parts queries.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@clickhouse-gh
Copy link

clickhouse-gh bot commented Jan 14, 2026 •
edited
Loading

Workflow [PR], commit [3958d25]

Summary: ❌

job_name test_name status info comment
Bugfix validation (functional tests) failure
03791_system_parts_race_condition_drop_part FAIL cidb
Finish Workflow failure
python3 ./ci/jobs/scripts/workflow_hooks/pr_body_check.py failure

@clickhouse-gh clickhouse-gh bot added pr-critical-bugfix pr-must-backport Pull request should be backported intentionally. Use this label with great care! labels Jan 14, 2026
@alexey-milovidov
Copy link
Member Author

alexey-milovidov commented Jan 14, 2026

Bugfix validation (functional tests)

This is okay, because it fails only under TSan.

@nikitamikhaylov
Copy link
Member

nikitamikhaylov commented Jan 14, 2026

AssertionError: No tag found for image [clickhouse/dotnet-client]

@nikitamikhaylov nikitamikhaylov self-assigned this Jan 14, 2026
@alexey-milovidov alexey-milovidov merged commit 181c465 into master Jan 16, 2026
127 of 132 checks passed
@alexey-milovidov alexey-milovidov deleted the fix-data-race-part-storage-remove branch January 16, 2026 20:21
@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jan 16, 2026
@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Jan 16, 2026
@robot-ch-test-poll4 robot-ch-test-poll4 mentioned this pull request Jan 16, 2026
Merged
robot-ch-test-poll4 added a commit that referenced this pull request Jan 16, 2026
@robot-ch-test-poll4
Merge pull request #94437 from ClickHouse/cherrypick/25.3/94262
1dd9d2c 
Cherry pick #94262 to 25.3: Fix data race in `DataPartStorageOnDiskBase::remove` vs `system.parts`
robot-clickhouse added a commit that referenced this pull request Jan 16, 2026
@robot-clickhouse
Backport #94262 to 25.3: Fix data race in `DataPartStorageOnDiskBase:…
6c0367d 
…:remove` vs `system.parts`
This was referenced Jan 16, 2026
Closed
Merged
robot-ch-test-poll4 added a commit that referenced this pull request Jan 16, 2026
@robot-ch-test-poll4
Merge pull request #94439 from ClickHouse/cherrypick/25.8/94262
3de9b45 
Cherry pick #94262 to 25.8: Fix data race in `DataPartStorageOnDiskBase::remove` vs `system.parts`
robot-clickhouse added a commit that referenced this pull request Jan 16, 2026
@robot-clickhouse
Backport #94262 to 25.8: Fix data race in `DataPartStorageOnDiskBase:…
e78e97d 
…:remove` vs `system.parts`
This was referenced Jan 16, 2026
Merged
Merged
robot-ch-test-poll4 added a commit that referenced this pull request Jan 16, 2026
@robot-ch-test-poll4
Merge pull request #94441 from ClickHouse/cherrypick/25.10/94262
552ab31 
Cherry pick #94262 to 25.10: Fix data race in `DataPartStorageOnDiskBase::remove` vs `system.parts`
robot-clickhouse added a commit that referenced this pull request Jan 16, 2026
@robot-clickhouse
Backport #94262 to 25.10: Fix data race in `DataPartStorageOnDiskBase…
d13001e 
…::remove` vs `system.parts`
This was referenced Jan 16, 2026
Merged
Merged
robot-ch-test-poll4 added a commit that referenced this pull request Jan 16, 2026
@robot-ch-test-poll4
Merge pull request #94443 from ClickHouse/cherrypick/25.11/94262
884a0d3 
Cherry pick #94262 to 25.11: Fix data race in `DataPartStorageOnDiskBase::remove` vs `system.parts`
robot-clickhouse added a commit that referenced this pull request Jan 16, 2026
@robot-clickhouse
Backport #94262 to 25.11: Fix data race in `DataPartStorageOnDiskBase…
ac39b85 
…::remove` vs `system.parts`
@robot-ch-test-poll4 robot-ch-test-poll4 mentioned this pull request Jan 16, 2026
Merged
@robot-ch-test-poll4 robot-ch-test-poll4 mentioned this pull request Jan 16, 2026
Merged
robot-ch-test-poll4 added a commit that referenced this pull request Jan 16, 2026
@robot-ch-test-poll4
Merge pull request #94445 from ClickHouse/cherrypick/25.12/94262
96646b5 
Cherry pick #94262 to 25.12: Fix data race in `DataPartStorageOnDiskBase::remove` vs `system.parts`
@robot-ch-test-poll4 robot-ch-test-poll4 mentioned this pull request Jan 16, 2026
Merged
robot-clickhouse added a commit that referenced this pull request Jan 16, 2026
@robot-clickhouse
Backport #94262 to 25.12: Fix data race in `DataPartStorageOnDiskBase…
e5ab003 
…::remove` vs `system.parts`
@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Jan 16, 2026
clickhouse-gh bot added a commit that referenced this pull request Jan 16, 2026
@clickhouse-gh
Merge pull request #94446 from ClickHouse/backport/25.12/94262
42b5f6f 
Backport #94262 to 25.12: Fix data race in `DataPartStorageOnDiskBase::remove` vs `system.parts`
clickhouse-gh bot added a commit that referenced this pull request Jan 16, 2026
@clickhouse-gh
Merge pull request #94444 from ClickHouse/backport/25.11/94262
733d12c 
Backport #94262 to 25.11: Fix data race in `DataPartStorageOnDiskBase::remove` vs `system.parts`
alexey-milovidov added a commit that referenced this pull request Jan 17, 2026
@alexey-milovidov
Merge pull request #94440 from ClickHouse/backport/25.8/94262
aef5644 
Backport #94262 to 25.8: Fix data race in `DataPartStorageOnDiskBase::remove` vs `system.parts`
alexey-milovidov added a commit that referenced this pull request Jan 17, 2026
@alexey-milovidov
Merge pull request #94442 from ClickHouse/backport/25.10/94262
938b4ea 
Backport #94262 to 25.10: Fix data race in `DataPartStorageOnDiskBase::remove` vs `system.parts`
@filimonov filimonov mentioned this pull request Jan 28, 2026
Open
25 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nikitamikhaylov nikitamikhaylov nikitamikhaylov approved these changes

Assignees

@nikitamikhaylov nikitamikhaylov

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-critical-bugfix pr-must-backport Pull request should be backported intentionally. Use this label with great care! pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

data race on part_dir in DataPartStorageOnDiskBase

5 participants

@alexey-milovidov @nikitamikhaylov @robot-ch-test-poll1 @robot-ch-test-poll2