Fix redundant file segment contention introduced in 26.1+ - #109577
Merged
Conversation
A CACHED-type reader is never the downloader of its file segment, but nextImplStep checked FileSegment::isDownloader unconditionally - both in the SCOPE_EXIT cleanup and at the end of the step. Each check takes the per-segment mutex. On cache-hit-heavy workloads with many concurrent readers of the same hot segments (e.g. compact parts, where all columns share the segments), these acquisitions collapse into a lock convoy: FileSegmentLockMicroseconds grew 21.7x per query after the read buffer was rewritten into a stateless read-state machine (which dropped the download_current_segment guard that 25.12 had). Gate both checks on state->read_type != ReadType::CACHED. The SCOPE_EXIT stays conservative when an exception is in flight, because prepareReadFromFileSegmentState could have thrown after becoming a downloader but before the new state was assigned. A download demoted to REMOTE_FS_READ_BYPASS_CACHE still passes the read_type gate. Analysis: https://pastila.clickhouse.com/?004d62dc/180549db80ba6c9a5781c3b74a02fc2f.html#dK09hbPCCbRg+qMl49WWeg==GCM Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
kssenii
added
the
pr-must-backport
Contributor
|
Workflow [PR], commit [ed0e0c7] Summary: ❌
AI ReviewSummaryThis PR removes redundant Final Verdict
|
clickhouse-gh
Bot
added
the
pr-bugfix
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
The check took the file segment lock before every buffer refill, keeping one contended acquisition per read on the cache-hit path. In release builds FileSegment::write and FileSegment::reserve still throw if the caller is not the downloader, and the reverse direction (a CACHED-type reader being the downloader) is enforced by the chassert in prepareReadFromFileSegmentState. Addresses #109577 (comment) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
antonio2368
approved these changes
Jul 7, 2026
Contributor
LLVM Coverage Report
Changed lines: Changed C/C++ lines covered: 16/16 (100.00%) · Uncovered code |
This was referenced Jul 7, 2026
Closed
robot-ch-test-poll2
added
the
pr-synced-to-cloud
robot-ch-test-poll3
added
the
pr-must-backport-synced
tavplubix
added
v25.12-must-backport
v26.2-must-backport
and removed
pr-must-backport-synced
robot-ch-test-poll4
added
the
pr-must-backport-synced
kssenii
added a commit
that referenced
this pull request
Jul 8, 2026
Backport #109577 to 26.5: Fix redundant file segment contention introduced in 26.1+
kssenii
added a commit
that referenced
this pull request
Jul 8, 2026
Backport #109577 to 26.4: Fix redundant file segment contention introduced in 26.1+
kssenii
added a commit
that referenced
this pull request
Jul 8, 2026
Backport #109577 to 26.3: Fix redundant file segment contention introduced in 26.1+
kssenii
added a commit
that referenced
this pull request
Jul 8, 2026
Backport #109577 to 26.6: Fix redundant file segment contention introduced in 26.1+
robot-ch-test-poll4
added
the
pr-backports-created
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Fix redundant file segment contention introduced in 26.1+.
Version info
26.7.1.597(included in26.7and later)26.6.2.48,26.5.6.26,26.4.5.113,26.3.17.32