Skip to content

Fix file_offset_of_buffer_end <= getFileSize() assertion in Log/StripeLog on S3#100763

Merged
alexey-milovidov merged 1 commit intomasterfrom
fix-file-offset-assertion-stripe-log
Mar 28, 2026
Merged

Fix file_offset_of_buffer_end <= getFileSize() assertion in Log/StripeLog on S3#100763
alexey-milovidov merged 1 commit intomasterfrom
fix-file-offset-assertion-stripe-log

Conversation

@alexey-milovidov
Copy link
Copy Markdown
Member

@alexey-milovidov alexey-milovidov commented Mar 25, 2026

The StripeLogSource and LogSource release the read lock before lazily opening the data file. On S3 object storage, the AsynchronousBoundedReadBuffer caches file size from metadata at open time, which may differ from the snapshotted size the index was built against if concurrent writes modified the file metadata between lock release and file open.

Fix by calling setReadUntilPosition(file_size) after opening the file, bounding reads to the snapshotted file size. This is a no-op on local disk and only takes effect on object storage where AsynchronousBoundedReadBuffer is used.

Found by BuzzHouse (amd_msan): https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=99537&sha=7338429ff8d3349ba147f9ba05558bbd6b84e4b2&name_0=PR&name_1=BuzzHouse%20%28amd_msan%29
#99537

Fixes #82555

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

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

Fix file_offset_of_buffer_end <= getFileSize() assertion failure (exception in debug builds) when reading from Log or StripeLog tables on S3 object storage with concurrent writes.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

…StripeLog` on S3

The `StripeLogSource` and `LogSource` release the read lock before lazily
opening the data file. On S3 object storage, the `AsynchronousBoundedReadBuffer`
caches file size from metadata at open time, which may differ from the
snapshotted size the index was built against if concurrent writes modified
the file metadata between lock release and file open.

Fix by calling `setReadUntilPosition(file_size)` after opening the file,
bounding reads to the snapshotted file size. This is a no-op on local disk
and only takes effect on object storage where `AsynchronousBoundedReadBuffer`
is used.

Fixes #82555
https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=99537&sha=7338429ff8d3349ba147f9ba05558bbd6b84e4b2&name_0=PR&name_1=BuzzHouse%20%28amd_msan%29

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Mar 25, 2026

Workflow [PR], commit [8add632]

Summary:

job_name test_name status info comment
Performance Comparison (arm_release, master_head, 4/6) failure
Check Results failure

AI Review

Summary

This PR addresses an S3/object-storage read bound mismatch in Log/StripeLog by explicitly calling setReadUntilPosition(file_size) after opening the data file, so reads stay within the snapshotted size captured under lock even if metadata changes after lock release. The fix is targeted, consistent across both affected readers, and aligns with the reported failure mode; I did not find new correctness, safety, or performance regressions introduced by this change.

ClickHouse Rules
Item Status Notes
Deletion logging
Serialization versioning
Core-area scrutiny
No test removal
Experimental gate
No magic constants
Backward compatibility
SettingsChangesHistory.cpp
PR metadata quality
Safe rollout
Compilation time
Final Verdict
  • Status: ✅ Approve

@clickhouse-gh clickhouse-gh Bot added the pr-bugfix Pull request with bugfix, not backported by default label Mar 25, 2026
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Mar 26, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 84.10% 84.20% +0.10%
Functions 24.40% 24.50% +0.10%
Branches 76.70% 76.70% +0.00%

Changed lines: 100.00% (11/11) · Uncovered code

Full report · Diff report

@kssenii kssenii self-assigned this Mar 26, 2026
@alexey-milovidov alexey-milovidov merged commit 9ab953d into master Mar 28, 2026
150 of 153 checks passed
@alexey-milovidov alexey-milovidov deleted the fix-file-offset-assertion-stripe-log branch March 28, 2026 01:38
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-synced-to-cloud The PR is synced to the cloud repo label Mar 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix Pull request with bugfix, not backported by default pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Logical error: 'file_offset_of_buffer_end <= getFileSize()'

3 participants