Skip to content

Fix reading from text index on s3#87399

Merged
CurtizJ merged 2 commits intoClickHouse:masterfrom
CurtizJ:fix-read-text-index-s3
Sep 29, 2025
Merged

Fix reading from text index on s3#87399
CurtizJ merged 2 commits intoClickHouse:masterfrom
CurtizJ:fix-read-text-index-s3

Conversation

@CurtizJ
Copy link
Copy Markdown
Member

@CurtizJ CurtizJ commented Sep 22, 2025

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):

Fixed direct reading from the text index on object storage.

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Sep 22, 2025

Workflow [PR], commit [6e4e7f6]

Summary:

job_name test_name status info comment
Stateless tests (amd_binary, ParallelReplicas, s3 storage, parallel) failure
03117_analyzer_same_column_name_as_func FAIL
Exception in test runner FAIL
Killed by signal (in clickhouse-server.log or clickhouse-server.err.log) FAIL
Fatal messages (in clickhouse-server.log or clickhouse-server.err.log) FAIL
Integration tests (amd_binary, 4/5) failure
test_merge_tree_s3/test.py::test_merge_canceled_by_s3_errors[node-broken_s3_always_multi_part] FAIL
Stress test (amd_debug) failure
Server died FAIL
Hung check failed, possible deadlock found (see hung_check.log) FAIL
Killed by signal (in clickhouse-server.log) FAIL
Fatal message in clickhouse-server.log (see fatal_messages.txt) FAIL
Killed by signal (output files) FAIL
Found signal in gdb.log FAIL
Stress test (amd_ubsan) failure
Server died FAIL
Hung check failed, possible deadlock found (see hung_check.log) FAIL
Killed by signal (in clickhouse-server.log) FAIL
Fatal message in clickhouse-server.log (see fatal_messages.txt) FAIL
Killed by signal (output files) FAIL
Found signal in gdb.log FAIL
Finish Workflow failure
python3 ./ci/jobs/scripts/workflow_hooks/new_tests_check.py failure

@clickhouse-gh clickhouse-gh bot added the pr-bugfix Pull request with bugfix, not backported by default label Sep 22, 2025
@CurtizJ CurtizJ marked this pull request as ready for review September 23, 2025 23:24
@ahmadov ahmadov self-assigned this Sep 24, 2025
Copy link
Copy Markdown
Member

@ahmadov ahmadov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@CurtizJ
Copy link
Copy Markdown
Member Author

CurtizJ commented Sep 29, 2025

I've tried to create a minimal repro to add a test. The minimal repro I could create still requires ingestion for about 1 minute. The bug reproduces under certain conditions when stealing of tasks is used in MergeTreePrefetchedReadPool (see here). I tested manually on S3, and this PR fixes Attempt to set read until position before already read data error.

@CurtizJ CurtizJ added this pull request to the merge queue Sep 29, 2025
Merged via the queue into ClickHouse:master with commit 40cbc7b Sep 29, 2025
117 of 123 checks passed
@CurtizJ CurtizJ deleted the fix-read-text-index-s3 branch September 29, 2025 12:27
@robot-ch-test-poll robot-ch-test-poll added the pr-synced-to-cloud The PR is synced to the cloud repo label Sep 29, 2025
@robot-ch-test-poll1 robot-ch-test-poll1 added pr-backports-created-cloud deprecated label, NOOP pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR labels Sep 29, 2025
robot-ch-test-poll3 added a commit that referenced this pull request Sep 29, 2025
Cherry pick #87399 to 25.9: Fix reading from text index on s3
@robot-ch-test-poll3 robot-ch-test-poll3 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Sep 29, 2025
CurtizJ added a commit that referenced this pull request Sep 29, 2025
Backport #87399 to 25.9: Fix reading from text index on s3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-backports-created-cloud deprecated label, NOOP pr-bugfix Pull request with bugfix, not backported by default 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

Development

Successfully merging this pull request may close these issues.

5 participants