You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/12/17 07:37:37 UTC
[GitHub] [pulsar] yufan022 opened a new pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
yufan022 opened a new pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985
<!--
### Contribution Checklist
- Name the pull request in the form "[Issue XYZ][component] Title of the pull request", where *XYZ* should be replaced by the actual issue number.
Skip *Issue XYZ* if there is no associated github issue for this pull request.
Skip *component* if you are unsure about which is the best component. E.g. `[docs] Fix typo in produce method`.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- Each pull request should address only one issue, not mix up code from multiple issues.
- Each commit in the pull request has a meaningful commit message
- Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below.
**(The sections below can be removed for hotfixes of typos)**
-->
*(If this PR fixes a github issue, please add `Fixes #<xyz>`.)*
Fixes #<8887>
*(or if this PR is one task of a github issue, please add `Master Issue: #<xyz>` to link to the master issue.)*
Master Issue: #<8887>
### Motivation
support tiered storage by aliyun oss.
### Modifications
Aliyun OSS is compatible with the S3 API.
For security reasons, OSS supports only virtual hosted style access.
So I use `S3ApiMetadata` to connect `OSS`, and set `jclouds.s3.virtual-host-buckets=true`.
### Verifying this change
test with local
### Documentation
- Does this pull request introduce a new feature? yes
- If yes, how is the feature documented? more tiered-storage provider
- If a feature is not documented yet in this PR, please create a followup issue for adding the documentation
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-749692367
@Renkai okay. make sense.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-747854187
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on a change in pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on a change in pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#discussion_r546372775
##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
}
},
+
+ /**
+ * Aliyun OSS is compatible with the S3 API
+ * https://www.alibabacloud.com/help/doc-detail/64919.htm
+ */
+ ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+ @Override
+ public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+ VALIDATION.validate(config);
+ }
+
+ @Override
+ public BlobStore getBlobStore(TieredStorageConfiguration config) {
+ return OSS_BLOB_STORE_BUILDER.getBlobStore(config);
Review comment:
if use `OSS_BLOB_STORE_BUILDER`,`OSS_VALIDATION` , it will only belongs to OSS.
But using `S3_API_BLOB_STORE_BUILDER`,`S3_API_VALIDATION` make it universal, both make sense.
What's your suggestion
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-747401731
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-785749329
@codelipenghui Hi, Will the code be merged into 2.7.1 or 2.8.0? Do I need to add documents for 2.7.1?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-749437989
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Renkai commented on a change in pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
Renkai commented on a change in pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#discussion_r545636617
##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,48 @@ public void buildCredentials(TieredStorageConfiguration config) {
}
},
+
+ /**
+ * Aliyun OSS is compatible with the S3 API
+ * https://www.alibabacloud.com/help/doc-detail/64919.htm
+ */
+ ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+ @Override
+ public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+ VALIDATION.validate(config);
+ }
+
+ @Override
+ public BlobStore getBlobStore(TieredStorageConfiguration config) {
+ ContextBuilder contextBuilder = ContextBuilder.newBuilder(config.getProviderMetadata());
Review comment:
I think it's better to define a new constant like here https://github.com/apache/pulsar/blob/bf9f61969c3fd7d82b85502266a12f85a793c63a/tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java#L258
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 removed a comment on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 removed a comment on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-747854187
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] jiazhai commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-752003548
@yufan022 Would you please also add the docs for this aliyun offloader in another PR? Or we add this code, but most of the user will not know how to use this.
Here is some reference: https://github.com/apache/pulsar/blob/master/site2/website/versioned_docs/version-2.7.0/tiered-storage-aws.md
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on a change in pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on a change in pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#discussion_r546373909
##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
}
},
+
+ /**
+ * Aliyun OSS is compatible with the S3 API
+ * https://www.alibabacloud.com/help/doc-detail/64919.htm
+ */
+ ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+ @Override
+ public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+ VALIDATION.validate(config);
+ }
+
+ @Override
+ public BlobStore getBlobStore(TieredStorageConfiguration config) {
+ return OSS_BLOB_STORE_BUILDER.getBlobStore(config);
+ }
+
+ @Override
+ public void buildCredentials(TieredStorageConfiguration config) {
+ AWS_CREDENTIAL_BUILDER.buildCredentials(config);
Review comment:
Environment variables are reuse for `AWS_ACCESS_KEY_ID `, `AWS_SECRET_ACCESS_KEY `.
You mean we should add new environment variables like `ALIYUN_ACCESS_KEY_ID`, `ALIYUN_SECRET_ACCESS_KEY`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on a change in pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on a change in pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#discussion_r546372775
##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
}
},
+
+ /**
+ * Aliyun OSS is compatible with the S3 API
+ * https://www.alibabacloud.com/help/doc-detail/64919.htm
+ */
+ ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+ @Override
+ public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+ VALIDATION.validate(config);
+ }
+
+ @Override
+ public BlobStore getBlobStore(TieredStorageConfiguration config) {
+ return OSS_BLOB_STORE_BUILDER.getBlobStore(config);
Review comment:
`AWS_S3` enum is already compatible with `S3 API`.
if use `OSS_BLOB_STORE_BUILDER`,`OSS_VALIDATION` , it will only belongs to Aliyun OSS.
But using `S3_API_BLOB_STORE_BUILDER`,`S3_API_VALIDATION` make it universal, both make sense.
What's your suggestion?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 removed a comment on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 removed a comment on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-747528783
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on a change in pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on a change in pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#discussion_r545789742
##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
}
},
+
+ /**
+ * Aliyun OSS is compatible with the S3 API
+ * https://www.alibabacloud.com/help/doc-detail/64919.htm
+ */
+ ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+ @Override
+ public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+ VALIDATION.validate(config);
+ }
+
+ @Override
+ public BlobStore getBlobStore(TieredStorageConfiguration config) {
+ return OSS_BLOB_STORE_BUILDER.getBlobStore(config);
Review comment:
`S3_STANDARD_BLOB_STORE_BUILDER`, `S3_COMMENT_BLOB_STORE_BUILDER` or other names? It seems that `oss` is a universal name.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui merged pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on a change in pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on a change in pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#discussion_r546372775
##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
}
},
+
+ /**
+ * Aliyun OSS is compatible with the S3 API
+ * https://www.alibabacloud.com/help/doc-detail/64919.htm
+ */
+ ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+ @Override
+ public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+ VALIDATION.validate(config);
+ }
+
+ @Override
+ public BlobStore getBlobStore(TieredStorageConfiguration config) {
+ return OSS_BLOB_STORE_BUILDER.getBlobStore(config);
Review comment:
if use `OSS_BLOB_STORE_BUILDER`,`OSS_VALIDATION` , it will only belongs to Aliyun OSS.
But using `S3_API_BLOB_STORE_BUILDER`,`S3_API_VALIDATION` make it universal, both make sense.
What's your suggestion
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on a change in pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on a change in pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#discussion_r545664486
##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,48 @@ public void buildCredentials(TieredStorageConfiguration config) {
}
},
+
+ /**
+ * Aliyun OSS is compatible with the S3 API
+ * https://www.alibabacloud.com/help/doc-detail/64919.htm
+ */
+ ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+ @Override
+ public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+ VALIDATION.validate(config);
+ }
+
+ @Override
+ public BlobStore getBlobStore(TieredStorageConfiguration config) {
+ ContextBuilder contextBuilder = ContextBuilder.newBuilder(config.getProviderMetadata());
Review comment:
fixed PTAL
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on a change in pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on a change in pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#discussion_r546377993
##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
}
},
+
+ /**
+ * Aliyun OSS is compatible with the S3 API
+ * https://www.alibabacloud.com/help/doc-detail/64919.htm
+ */
+ ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+ @Override
+ public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+ VALIDATION.validate(config);
+ }
+
+ @Override
+ public BlobStore getBlobStore(TieredStorageConfiguration config) {
+ return OSS_BLOB_STORE_BUILDER.getBlobStore(config);
Review comment:
`AWS_S3` enum is already compatible with S3 API.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 removed a comment on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 removed a comment on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-747402565
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 removed a comment on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 removed a comment on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-747528399
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 edited a comment on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 edited a comment on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-748604987
> @yufan022 @gaoran10 Could we add some tests for this part of code change?
Sure, Is there any example I can refer to? I couldn't find test case about AWS or AZURE
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Renkai commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
Renkai commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-748717274
> @yufan022 @gaoran10 Could we add some tests for this part of code change?
@jiazhai
Seems we don't have actually run any test on S3/Azureblob/Google cloud storage, only some mocks, so add a storage vendor will not actually change the mocks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-747917506
@yufan022 great contributions!
@gaoran10 @Renkai can you help review this change?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-752001074
Sorry for the late response, LGTM.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-747528783
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on a change in pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on a change in pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#discussion_r546372775
##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
}
},
+
+ /**
+ * Aliyun OSS is compatible with the S3 API
+ * https://www.alibabacloud.com/help/doc-detail/64919.htm
+ */
+ ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+ @Override
+ public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+ VALIDATION.validate(config);
+ }
+
+ @Override
+ public BlobStore getBlobStore(TieredStorageConfiguration config) {
+ return OSS_BLOB_STORE_BUILDER.getBlobStore(config);
Review comment:
if use `OSS_BLOB_STORE_BUILDER`,`OSS_VALIDATION` , it will only belongs to Aliyun OSS.
But using `S3_API_BLOB_STORE_BUILDER`,`S3_API_VALIDATION` make it universal, both make sense.
What's your suggestion?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on a change in pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on a change in pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#discussion_r546377993
##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
}
},
+
+ /**
+ * Aliyun OSS is compatible with the S3 API
+ * https://www.alibabacloud.com/help/doc-detail/64919.htm
+ */
+ ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+ @Override
+ public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+ VALIDATION.validate(config);
+ }
+
+ @Override
+ public BlobStore getBlobStore(TieredStorageConfiguration config) {
+ return OSS_BLOB_STORE_BUILDER.getBlobStore(config);
Review comment:
`AWS_S3` enum is already compatible with S3 API.
If this code is exclusive to Aliyun oss, maybe we don't need to extract a constant?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-752010068
@jiazhai No problem. Will the code be merged into the latest version(2.8.0)? It seems that the 2.8.0 document has not been created in the master branch.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-747402565
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Renkai edited a comment on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
Renkai edited a comment on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-748717274
> @yufan022 @gaoran10 Could we add some tests for this part of code change?
@jiazhai
Seems we don't have actually run any test on S3/Azureblob/Google cloud storage, only some mocks, so add a storage vendor will not actually change the mocks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-749692932
@jiazhai @gaoran10 can you review the change again?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on a change in pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on a change in pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#discussion_r546373042
##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
}
},
+
+ /**
+ * Aliyun OSS is compatible with the S3 API
+ * https://www.alibabacloud.com/help/doc-detail/64919.htm
+ */
+ ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+ @Override
+ public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+ VALIDATION.validate(config);
Review comment:
Yep, `serviceEndpoint`is a necessary configuration, I will add a new validator.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on a change in pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on a change in pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#discussion_r545789742
##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
}
},
+
+ /**
+ * Aliyun OSS is compatible with the S3 API
+ * https://www.alibabacloud.com/help/doc-detail/64919.htm
+ */
+ ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+ @Override
+ public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+ VALIDATION.validate(config);
+ }
+
+ @Override
+ public BlobStore getBlobStore(TieredStorageConfiguration config) {
+ return OSS_BLOB_STORE_BUILDER.getBlobStore(config);
Review comment:
`S3_STANDARD_BLOB_STORE_BUILDER`, `S3_COMMON_BLOB_STORE_BUILDER` or other names? It seems that `oss` is a universal name.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-750782120
Ping @gaoran10 @jiazhai
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] jiazhai commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-752019195
> @jiazhai No problem. Will the code be merged into the latest version(2.8.0)? It seems that the 2.8.0 document has not been created in the master branch.
Thanks @yufan022, For the latest master, the doc is put at : https://github.com/apache/pulsar/tree/master/site2/docs
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-747528399
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-749434971
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-747895396
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on a change in pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on a change in pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#discussion_r545789742
##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
}
},
+
+ /**
+ * Aliyun OSS is compatible with the S3 API
+ * https://www.alibabacloud.com/help/doc-detail/64919.htm
+ */
+ ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+ @Override
+ public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+ VALIDATION.validate(config);
+ }
+
+ @Override
+ public BlobStore getBlobStore(TieredStorageConfiguration config) {
+ return OSS_BLOB_STORE_BUILDER.getBlobStore(config);
Review comment:
`S3_STANDARD_BLOB_STORE_BUILDER` or other names? It seems that `oss` is a universal name.
##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
}
},
+
+ /**
+ * Aliyun OSS is compatible with the S3 API
+ * https://www.alibabacloud.com/help/doc-detail/64919.htm
+ */
+ ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+ @Override
+ public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+ VALIDATION.validate(config);
Review comment:
Does the `serviceEndpoint` is a necessary configuration? If necessary it's better to add a configuration validationcheck.
##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
}
},
+
+ /**
+ * Aliyun OSS is compatible with the S3 API
+ * https://www.alibabacloud.com/help/doc-detail/64919.htm
+ */
+ ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+ @Override
+ public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+ VALIDATION.validate(config);
+ }
+
+ @Override
+ public BlobStore getBlobStore(TieredStorageConfiguration config) {
+ return OSS_BLOB_STORE_BUILDER.getBlobStore(config);
+ }
+
+ @Override
+ public void buildCredentials(TieredStorageConfiguration config) {
+ AWS_CREDENTIAL_BUILDER.buildCredentials(config);
Review comment:
Does the Aliyun security validate is the same as AWS? Do we need to add environment configurations for the Aliyun service? Currently, the AWS security configurations could refer to [doc](https://pulsar.apache.org/docs/en/tiered-storage-aws/#authentication-required).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-785037057
Sorry for late.
docs: https://github.com/apache/pulsar/pull/9696
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-751982664
Hi, have we made any progress?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 removed a comment on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 removed a comment on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-747401731
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-748604987
> @yufan022 @gaoran10 Could we add some tests for this part of code change?
Sure, Is there any example I can refer to?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on a change in pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on a change in pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#discussion_r546373909
##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
}
},
+
+ /**
+ * Aliyun OSS is compatible with the S3 API
+ * https://www.alibabacloud.com/help/doc-detail/64919.htm
+ */
+ ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+ @Override
+ public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+ VALIDATION.validate(config);
+ }
+
+ @Override
+ public BlobStore getBlobStore(TieredStorageConfiguration config) {
+ return OSS_BLOB_STORE_BUILDER.getBlobStore(config);
+ }
+
+ @Override
+ public void buildCredentials(TieredStorageConfiguration config) {
+ AWS_CREDENTIAL_BUILDER.buildCredentials(config);
Review comment:
Environment variables are reuse for `AWS_ACCESS_KEY_ID `, `AWS_SECRET_ACCESS_KEY `.
You mean we should add new environment variables like `ALIYUN_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-747991984
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] jiazhai commented on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-748604360
@yufan022 @gaoran10 Could we add some tests for this part of code change?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on a change in pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on a change in pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#discussion_r545789742
##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
}
},
+
+ /**
+ * Aliyun OSS is compatible with the S3 API
+ * https://www.alibabacloud.com/help/doc-detail/64919.htm
+ */
+ ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+ @Override
+ public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+ VALIDATION.validate(config);
+ }
+
+ @Override
+ public BlobStore getBlobStore(TieredStorageConfiguration config) {
+ return OSS_BLOB_STORE_BUILDER.getBlobStore(config);
Review comment:
`S3_STANDARD_BLOB_STORE_BUILDER`, `S3_UNIVERSAL_BLOB_STORE_BUILDER` or other names? It seems that `oss` is a universal name.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yufan022 removed a comment on pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS
Posted by GitBox <gi...@apache.org>.
yufan022 removed a comment on pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#issuecomment-749434971
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org