[feat](storage vault) Add object storage op check when creating resource #48073
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 31827 ms |
TPC-DS: Total hot run time: 184819 ms |
ClickBench: Total hot run time: 30.26 s |
swjtu-zhanglei
force-pushed
the
master-vault
branch
from
afb02c3 to
cd22372
Compare
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 31446 ms |
TPC-DS: Total hot run time: 191453 ms |
ClickBench: Total hot run time: 31.98 s |
Contributor
Author
|
run feut |
swjtu-zhanglei
force-pushed
the
master-vault
branch
from
cd22372 to
009fd31
Compare
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 31586 ms |
TPC-DS: Total hot run time: 190038 ms |
ClickBench: Total hot run time: 30.6 s |
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 31212 ms |
TPC-DS: Total hot run time: 184657 ms |
ClickBench: Total hot run time: 30.79 s |
swjtu-zhanglei
force-pushed
the
master-vault
branch
from
376a385 to
684f1f5
Compare
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 31537 ms |
TPC-DS: Total hot run time: 189343 ms |
ClickBench: Total hot run time: 30.74 s |
gavinchou
reviewed
Feb 25, 2025
| * @param <C> cloud SDK Client | ||
| */ | ||
| public interface ObjStorage<C> { | ||
| public static final int CHUNK_SIZE = 5 * 1024 * 1024; |
Contributor
There was a problem hiding this comment.
add comment what is chunk_size and what it is for
Contributor
Author
There was a problem hiding this comment.
add comment what is chunk_size and what it is for
done
gavinchou
requested changes
Feb 25, 2025
| } | ||
| } | ||
|
|
||
| public Status multiPartPutObject(String remotePath, @Nullable InputStream inputStream, long totalBytes) { |
Contributor
Author
There was a problem hiding this comment.
what about abort?
done
| Status status = azureObjStorage.putObject(testFile, new ByteArrayInputStream(contentData), contentData.length); | ||
| if (!Status.OK.equals(status)) { | ||
| throw new DdlException( | ||
| "ping azure failed(put), status: " + status + ", properties: " + new PrintableMap<>( |
Contributor
There was a problem hiding this comment.
we may need to convert the raw message to what user can understand.
e.g.
if status is 403, return "failed try to test to put object, lack of permission of PUT"
if status is connection refused, return "failed to connect to azure, please check your connection or endpoint"
Contributor
Author
There was a problem hiding this comment.
we may need to convert the raw message to what user can understand. e.g. if status is 403, return "failed try to test to put object, lack of permission of PUT" if status is connection refused, return "failed to connect to azure, please check your connection or endpoint"
it is not necessary, keep the same as before
Contributor
Author
There was a problem hiding this comment.
we may need to convert the raw message to what user can understand. e.g. if status is 403, return "failed try to test to put object, lack of permission of PUT" if status is connection refused, return "failed to connect to azure, please check your connection or endpoint"
it is not necessary, keep the same as before
Can you show me the code, what you expect to deal with
| newProperties, "=", true, false, true, false)); | ||
| } | ||
|
|
||
| status = azureObjStorage.multiPartPutObject(testFile, |
Contributor
There was a problem hiding this comment.
abort should be also considered
Contributor
Author
There was a problem hiding this comment.
abort should be also considered
done
| LOG.info("remoteObjects: {}", remoteObjects); | ||
| Preconditions.checkArgument(remoteObjects.getObjectList().size() == 1, "remoteObjects.size() must equal 1"); | ||
|
|
||
| status = s3ObjStorage.multiPartPutObject(testObj, new ByteArrayInputStream(contentData), contentData.length); |
Contributor
There was a problem hiding this comment.
abort should be also considered
Contributor
Author
There was a problem hiding this comment.
abort should be also considered
done
| return; | ||
|
|
||
| String testFile = "azure://" + bucketName + "/" + rootPath + "/" | ||
| + UUID.randomUUID().toString() + "/test-object-valid.txt"; |
Contributor
There was a problem hiding this comment.
use timestamp in milliseconds instead
e.g. doris-test-object-valid-${timstamp}.txt
Contributor
Author
There was a problem hiding this comment.
use timestamp in milliseconds instead e.g. doris-test-object-valid-${timstamp}.txt
done
| String content = "doris will be better"; | ||
| if (FeConstants.runningUnitTest) { | ||
| return; | ||
| protected static void pingS3(String bucketName, String rootPath, Map<String, String> newProperties) |
Contributor
There was a problem hiding this comment.
same comments to this section like AzureResource.java
swjtu-zhanglei
force-pushed
the
master-vault
branch
from
684f1f5 to
bbe0fe8
Compare
Contributor
Author
|
run buildall |
* When creating `S3Resource/AzureResource`, we will check if it can be accessed with `put/multiPartPut/list/head/delete` operator
swjtu-zhanglei
force-pushed
the
master-vault
branch
from
8cf74e7 to
e690cdf
Compare
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 32680 ms |
TPC-DS: Total hot run time: 192343 ms |
ClickBench: Total hot run time: 30.71 s |
Contributor
Author
|
run cloud_p0 |
gavinchou
approved these changes
Mar 10, 2025
github-actions
bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
Yukang-Lian
approved these changes
Mar 10, 2025
gavinchou
approved these changes
Mar 10, 2025
github-actions bot
pushed a commit
that referenced
this pull request
Mar 10, 2025
…rce (#48073) * When creating `S3Resource/AzureResource`, we will check if it can be accessed with `put/multiPartPut/list/head/delete` operator
Closed
koarz
pushed a commit
to koarz/doris
that referenced
this pull request
Jun 4, 2025
…rce (apache#48073) * When creating `S3Resource/AzureResource`, we will check if it can be accessed with `put/multiPartPut/list/head/delete` operator
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
S3Resource/AzureResource, we will check if it can be accessed withput/multiPartPut/list/head/deleteoperatorWhat problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)