Skip to content

Conversation

@antonio2368
Copy link
Member

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 using native copy on GCS for backups. Because of incorrect client cloning, GCS native copy always failed and used less optimal approach of manual reading and writing the data.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@clickhouse-gh
Copy link

clickhouse-gh bot commented Nov 12, 2025

Workflow [PR], commit [0dcb494]

Summary:

job_name test_name status info comment
PR formatter failure
check output failure cidb
Stateless tests (amd_binary, old analyzer, s3 storage, DatabaseReplicated, parallel) failure
03712_json_advanced_shared_data_bug FAIL cidb
Integration tests (amd_binary, 2/5) failure
test_kafka_bad_messages/test.py::test_bad_messages_parsing_stream FAIL cidb
test_kafka_bad_messages/test.py::test_bad_messages_parsing_dead_letter_queue FAIL cidb
test_kafka_bad_messages/test.py::test_bad_messages_parsing_exception FAIL cidb
test_kafka_bad_messages/test.py::test_bad_messages_to_mv FAIL cidb
BuzzHouse (amd_debug) failure
Buzzing result failure cidb
BuzzHouse (arm_asan) failure
Buzzing result failure cidb
BuzzHouse (amd_ubsan) failure
Buzzing result failure cidb
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 Nov 12, 2025
@nikitamikhaylov
Copy link
Member

@antonio2368 Time to think how to write good tests for it:
image
We repeatedly fix the same problem in the same place...

@nikitamikhaylov nikitamikhaylov added the pr-must-backport Pull request should be backported intentionally. Use this label with great care! label Nov 12, 2025
@antonio2368
Copy link
Member Author

Time to think how to write good tests for it

The issue is we are working around the specific behavior of GCS so I don't see a way to test it correctly unless we connect to it directly.

Also, the source of all those issues is using AWS client for GCS interaction which we reverse engineer based on the responses we get (+ it changes constantly). Using native client would definitely help.

@nikitamikhaylov
Copy link
Member

@antonio2368 I was thinking about checking it from the C++ standpoint. For example to write a private operator== and having the unit test that checks that a cloned client equals to an original one.

However, this test won't fail in case if you add a new field to the class and don't update the operator==/copy constructor.

@nikitamikhaylov
Copy link
Member

CI experiences some instability. Not related to the PR, because we have no real tests on top of GCP.

@nikitamikhaylov nikitamikhaylov added this pull request to the merge queue Nov 12, 2025
Merged via the queue into master with commit 290d461 Nov 12, 2025
122 of 130 checks passed
@nikitamikhaylov nikitamikhaylov deleted the fix-s3-client-clone branch November 12, 2025 22:02
@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Nov 12, 2025
robot-ch-test-poll4 added a commit that referenced this pull request Nov 12, 2025
Cherry pick #89923 to 25.3: Fix S3 client clone
robot-ch-test-poll4 added a commit that referenced this pull request Nov 12, 2025
Cherry pick #89923 to 25.8: Fix S3 client clone
robot-ch-test-poll4 added a commit that referenced this pull request Nov 12, 2025
Cherry pick #89923 to 25.9: Fix S3 client clone
robot-ch-test-poll4 added a commit that referenced this pull request Nov 12, 2025
Cherry pick #89923 to 25.10: Fix S3 client clone
@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Nov 12, 2025
@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Nov 12, 2025
clickhouse-gh bot added a commit that referenced this pull request Nov 13, 2025
antonio2368 added a commit that referenced this pull request Nov 13, 2025
antonio2368 added a commit that referenced this pull request Nov 13, 2025
antonio2368 added a commit that referenced this pull request Nov 13, 2025
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-bugfix Pull request with bugfix, not backported by default 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

Development

Successfully merging this pull request may close these issues.

6 participants