-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[tiered-storage] set s3 offloader use Virtual Hosted-Style as default #15893
base: master
Are you sure you want to change the base?
[tiered-storage] set s3 offloader use Virtual Hosted-Style as default #15893
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could change all the jclouds properties by the environment variable or the properties. Adding by this https://github.com/apache/pulsar/pull/15710/files#diff-5fa8cee1e48758e8c2ace441539e0fd696aef158af2da84341532c3e492efc61R333. So the S3 should be a more general way for all the S3-compatible storage service.
pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/OffloadPoliciesImpl.java
Outdated
Show resolved
Hide resolved
0f31efb
to
e7b2f23
Compare
e7b2f23
to
4730c94
Compare
i think set this value true as default is more user-friendly and recommanded from the guide of latest AWS S3 userguide says. |
@hangc0276 PTAL |
The pr had no activity for 30 days, mark with Stale label. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about that -1, I think add the doc for use virtual Hosted-Style
would be great.
Motivation
This PR #15710 provides a common way to offload data to storages which compatible with S3 APIs.
I have a test on Tencent Cloud COS (a cloud storage which compatible with S3 APIs),but offload failed.
error messages as below:
because tencent cloud cos is use Virtual Hosted-Style as default for security reason and network performance, this is probably a reasonably good use way to set
PROPERTY_S3_VIRTUAL_HOST_BUCKETS=true
such as in #15860In latest AWS S3 userguide,Virtual Hosted-Style is more recommended.
otherwise,aliyun oss & tencent cos are compatible with S3 APIs. so we could simply this implementation. add a new Offload-Driver name "s3-compatible-storage" to offload data into all these s3-compatible-storages.
Modifications
Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
If
yes
was chosen, please highlight the changesDocumentation
Check the box below or label this PR directly.
Need to update docs?
doc-required
(Your PR needs to update docs and you will update later)
doc-not-needed
(Please explain why)
doc
(Your PR contains doc changes)
doc-complete
(Docs have been already added)