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