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.
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
Revert "Fix GCS native copy (#48981)" #49194
Revert "Fix GCS native copy (#48981)" #49194
Conversation
Related ticket: #49195 |
We need to wait for Antonio's decision whether to revert or not. For now we can try to understand what exactly is wrong, probably these lines? https://github.com/ClickHouse/ClickHouse/pull/48981/files#diff-aaad9300ade1d534bdc2dd18a47e277eefab575cc0ecbe2b7954c1e5d7133d4dR263-R272 |
@jrdi Can you give me a hand checking what's broken so we can introduce the reintroduce feature without breaking GCS storage? |
Yep. Removing those 3 lines make it work again. I'm not sure if there are GCS alternatives as done in https://github.com/ClickHouse/ClickHouse/pull/48981/files#diff-64668bf89e92ea1f2b02bb24546b18197b20b9c730965ce7df22074c9f077bbcR28-R30 or they just need to be present |
22f212c
to
1107744
Compare
I've #ifdef'd that block for now in case we want to revert just that part of the PR and fix GCS disks for now. |
It seems that the header changes in #48981 also broke some (but not all) accesses to GCS to read files:
With current master:
With this revert on top: OK |
At this point, since 23.4 has been released, this should to be considered "bugfix" and backported |
@Algunenano thanks for reverting only part of the PR which caused the issue! |
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Documentation entry for user-facing changes
Partially reverts #48981
This PR has broken s3 disks over GCS (with s3 compat API). Reverting the header changes make them work again.
cc @antonio2368
More details about the issue coming in a ticket