gavinchou commented on code in PR #48073:
URL: https://github.com/apache/doris/pull/48073#discussion_r1969979074


##########
fe/fe-core/src/main/java/org/apache/doris/fs/obj/S3ObjStorage.java:
##########
@@ -309,4 +318,63 @@ public RemoteObjects listObjects(String absolutePath, 
String continuationToken)
             throw new DdlException("Failed to list objects for S3, Error 
message: " + e.getMessage(), e);
         }
     }
+
+    public Status multiPartPutObject(String remotePath, @Nullable InputStream 
inputStream, long totalBytes) {

Review Comment:
   what about abort?



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/AzureResource.java:
##########
@@ -78,21 +81,56 @@ protected void setProperties(Map<String, String> 
newProperties) throws DdlExcept
         this.properties = newProperties;
     }
 
-    private static void pingAzure(String bucketName, String rootPath,
+    protected static void pingAzure(String bucketName, String rootPath,
             Map<String, String> newProperties) throws DdlException {
-        if (FeConstants.runningUnitTest) {
-            return;
+
+        String testFile = "azure://" + bucketName + "/" + rootPath + "/"
+                + UUID.randomUUID().toString() + "/test-object-valid.txt";

Review Comment:
   use timestamp in milliseconds instead
   e.g. doris-test-object-valid-${timstamp}.txt



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/S3Resource.java:
##########
@@ -102,45 +105,57 @@ protected void setProperties(Map<String, String> 
properties) throws DdlException
         }
         String region = S3Properties.getRegionOfEndpoint(pingEndpoint);
         properties.putIfAbsent(S3Properties.REGION, region);
-        String ak = properties.get(S3Properties.ACCESS_KEY);
-        String sk = properties.get(S3Properties.SECRET_KEY);
-        String token = properties.get(S3Properties.SESSION_TOKEN);
-        CloudCredentialWithEndpoint credential = new 
CloudCredentialWithEndpoint(pingEndpoint, region, ak, sk, token);
 
         if (needCheck) {
             String bucketName = properties.get(S3Properties.BUCKET);
             String rootPath = properties.get(S3Properties.ROOT_PATH);
-            pingS3(credential, bucketName, rootPath, properties);
+            pingS3(bucketName, rootPath, properties);
         }
         // optional
         S3Properties.optionalS3Property(properties);
         this.properties = properties;
     }
 
-    private static void pingS3(CloudCredentialWithEndpoint credential, String 
bucketName, String rootPath,
-            Map<String, String> properties) throws DdlException {
-        S3FileSystem fileSystem = new S3FileSystem(properties);
-        String testFile = "s3://" + bucketName + "/" + rootPath + 
"/test-object-valid.txt";
-        String content = "doris will be better";
-        if (FeConstants.runningUnitTest) {
-            return;
+    protected static void pingS3(String bucketName, String rootPath, 
Map<String, String> newProperties)

Review Comment:
   same comments to this section like AzureResource.java



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/AzureResource.java:
##########
@@ -78,21 +81,56 @@ protected void setProperties(Map<String, String> 
newProperties) throws DdlExcept
         this.properties = newProperties;
     }
 
-    private static void pingAzure(String bucketName, String rootPath,
+    protected static void pingAzure(String bucketName, String rootPath,
             Map<String, String> newProperties) throws DdlException {
-        if (FeConstants.runningUnitTest) {
-            return;
+
+        String testFile = "azure://" + bucketName + "/" + rootPath + "/"
+                + UUID.randomUUID().toString() + "/test-object-valid.txt";
+
+        byte[] contentData = new byte[2 * ObjStorage.CHUNK_SIZE];
+        Arrays.fill(contentData, (byte) 'A');
+        AzureObjStorage azureObjStorage = new AzureObjStorage(newProperties);
+
+        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<>(
+                            newProperties, "=", true, false, true, false));
         }
 
-        String testFile = "azure://" + bucketName + "/" + rootPath + 
"/test-object-valid.txt";
-        AzureFileSystem fileSystem = new AzureFileSystem(newProperties);
-        Status status = fileSystem.exists(testFile);
-        if (status != Status.OK && status.getErrCode() != 
Status.ErrCode.NOT_FOUND) {
+        status = azureObjStorage.headObject(testFile);
+        if (!Status.OK.equals(status)) {
             throw new DdlException(
                     "ping azure failed(head), status: " + status + ", 
properties: " + new PrintableMap<>(
                             newProperties, "=", true, false, true, false));
         }
-        LOG.info("success to ping azure");
+
+        RemoteObjects remoteObjects = azureObjStorage.listObjects(testFile, 
null);
+        LOG.info("remoteObjects: {}", remoteObjects);
+        Preconditions.checkArgument(remoteObjects.getObjectList().size() == 1, 
"remoteObjects.size() must equal 1");
+
+        status = azureObjStorage.deleteObject(testFile);
+        if (!Status.OK.equals(status)) {
+            throw new DdlException(
+                    "ping azure failed(delete), status: " + status + ", 
properties: " + new PrintableMap<>(
+                            newProperties, "=", true, false, true, false));
+        }
+
+        status = azureObjStorage.multiPartPutObject(testFile,

Review Comment:
   abort should be also considered



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/AzureResource.java:
##########
@@ -78,21 +81,56 @@ protected void setProperties(Map<String, String> 
newProperties) throws DdlExcept
         this.properties = newProperties;
     }
 
-    private static void pingAzure(String bucketName, String rootPath,
+    protected static void pingAzure(String bucketName, String rootPath,
             Map<String, String> newProperties) throws DdlException {
-        if (FeConstants.runningUnitTest) {
-            return;
+
+        String testFile = "azure://" + bucketName + "/" + rootPath + "/"
+                + UUID.randomUUID().toString() + "/test-object-valid.txt";
+
+        byte[] contentData = new byte[2 * ObjStorage.CHUNK_SIZE];
+        Arrays.fill(contentData, (byte) 'A');
+        AzureObjStorage azureObjStorage = new AzureObjStorage(newProperties);
+
+        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<>(

Review 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"



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/S3Resource.java:
##########
@@ -102,45 +105,57 @@ protected void setProperties(Map<String, String> 
properties) throws DdlException
         }
         String region = S3Properties.getRegionOfEndpoint(pingEndpoint);
         properties.putIfAbsent(S3Properties.REGION, region);
-        String ak = properties.get(S3Properties.ACCESS_KEY);
-        String sk = properties.get(S3Properties.SECRET_KEY);
-        String token = properties.get(S3Properties.SESSION_TOKEN);
-        CloudCredentialWithEndpoint credential = new 
CloudCredentialWithEndpoint(pingEndpoint, region, ak, sk, token);
 
         if (needCheck) {
             String bucketName = properties.get(S3Properties.BUCKET);
             String rootPath = properties.get(S3Properties.ROOT_PATH);
-            pingS3(credential, bucketName, rootPath, properties);
+            pingS3(bucketName, rootPath, properties);
         }
         // optional
         S3Properties.optionalS3Property(properties);
         this.properties = properties;
     }
 
-    private static void pingS3(CloudCredentialWithEndpoint credential, String 
bucketName, String rootPath,
-            Map<String, String> properties) throws DdlException {
-        S3FileSystem fileSystem = new S3FileSystem(properties);
-        String testFile = "s3://" + bucketName + "/" + rootPath + 
"/test-object-valid.txt";
-        String content = "doris will be better";
-        if (FeConstants.runningUnitTest) {
-            return;
+    protected static void pingS3(String bucketName, String rootPath, 
Map<String, String> newProperties)
+            throws DdlException {
+
+        String prefix = "s3://" + bucketName + "/" + rootPath + "/" + 
UUID.randomUUID().toString();
+        String testObj = prefix + "/test-object-valid.txt";
+
+        byte[] contentData = new byte[2 * ObjStorage.CHUNK_SIZE];
+        Arrays.fill(contentData, (byte) 'A');
+        S3ObjStorage s3ObjStorage = new S3ObjStorage(newProperties);
+
+        Status status = s3ObjStorage.putObject(testObj, new 
ByteArrayInputStream(contentData), contentData.length);
+        if (!Status.OK.equals(status)) {
+            throw new DdlException(
+                    "pingS3 failed(put), status: " + status + ", properties: " 
+ new PrintableMap<>(
+                            newProperties, "=", true, false, true, false));
         }
-        Status status = Status.OK;
-        try {
-            status = fileSystem.directUpload(content, testFile);
-            if (status != Status.OK) {
-                throw new DdlException(
-                        "ping s3 failed(upload), status: " + status + ", 
properties: " + new PrintableMap<>(
-                                properties, "=", true, false, true, false));
-            }
-        } finally {
-            if (status.ok()) {
-                Status delete = fileSystem.delete(testFile);
-                if (delete != Status.OK) {
-                    LOG.warn("delete test file failed, status: {}, properties: 
{}", delete, new PrintableMap<>(
-                            properties, "=", true, false, true, false));
-                }
-            }
+
+        status = s3ObjStorage.headObject(testObj);
+        if (!Status.OK.equals(status)) {
+            throw new DdlException(
+                    "pingS3 failed(head), status: " + status + ", properties: 
" + new PrintableMap<>(
+                            newProperties, "=", true, false, true, false));
+        }
+
+        RemoteObjects remoteObjects = s3ObjStorage.listObjects(prefix, null);
+        LOG.info("remoteObjects: {}", remoteObjects);
+        Preconditions.checkArgument(remoteObjects.getObjectList().size() == 1, 
"remoteObjects.size() must equal 1");
+
+        status = s3ObjStorage.multiPartPutObject(testObj, new 
ByteArrayInputStream(contentData), contentData.length);

Review Comment:
   abort should be also considered



-- 
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.

To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to