>From Murtadha Al Hubail <[email protected]>: Murtadha Al Hubail has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17614 )
Change subject: [ASTERIXDB-3196][*DB] Skip upload on empty writes ...................................................................... [ASTERIXDB-3196][*DB] Skip upload on empty writes - user model changes: no - storage format changes: no - interface changes: no Details: - When no data was uploaded, skip completing the multipart upload. - Workaround S3Mock encoding issues by encoding/decoding keys on list objects. Change-Id: I31f9955e8a9c4897eba4f819746b2dd47fa7c50f Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17614 Reviewed-by: Ali Alsuliman <[email protected]> Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> --- M asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3ClientConfig.java M asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3CloudClient.java M asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3BufferedWriter.java M asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3Utils.java 4 files changed, 44 insertions(+), 2 deletions(-) Approvals: Ali Alsuliman: Looks good to me, approved Jenkins: Verified; Verified Objections: Anon. E. Moose #1000171: Violations found diff --git a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3BufferedWriter.java b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3BufferedWriter.java index 9104d9a..6c02bbc 100644 --- a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3BufferedWriter.java +++ b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3BufferedWriter.java @@ -69,6 +69,9 @@ @Override public void finish() throws HyracksDataException { + if (uploadId == null) { + return; + } CompletedMultipartUpload completedMultipartUpload = CompletedMultipartUpload.builder().parts(partQueue).build(); CompleteMultipartUploadRequest completeMultipartUploadRequest = CompleteMultipartUploadRequest.builder() .bucket(bucket).key(path).uploadId(uploadId).multipartUpload(completedMultipartUpload).build(); @@ -97,6 +100,9 @@ @Override public void abort() throws HyracksDataException { + if (uploadId == null) { + return; + } s3Client.abortMultipartUpload( AbortMultipartUploadRequest.builder().bucket(bucket).key(path).uploadId(uploadId).build()); } diff --git a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3ClientConfig.java b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3ClientConfig.java index 281fe77..bbe8586 100644 --- a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3ClientConfig.java +++ b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3ClientConfig.java @@ -55,7 +55,16 @@ return prefix; } + public boolean isEncodeKeys() { + // to workaround https://github.com/findify/s3mock/issues/187 in our S3Mock, we encode/decode keys + return isS3Mock(); + } + public AwsCredentialsProvider createCredentialsProvider() { return anonymousAuth ? AnonymousCredentialsProvider.create() : DefaultCredentialsProvider.create(); } + + private boolean isS3Mock() { + return endpoint != null && !endpoint.isEmpty(); + } } diff --git a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3CloudClient.java b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3CloudClient.java index 6d65f69..0fb9c08 100644 --- a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3CloudClient.java +++ b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3CloudClient.java @@ -18,6 +18,7 @@ */ package org.apache.asterix.cloud.clients.aws.s3; +import static org.apache.asterix.cloud.clients.aws.s3.S3Utils.encodeURI; import static org.apache.asterix.cloud.clients.aws.s3.S3Utils.listS3Objects; import java.io.File; @@ -108,6 +109,7 @@ @Override public Set<String> listObjects(String bucket, String path, FilenameFilter filter) { + path = config.isEncodeKeys() ? encodeURI(path) : path; return filterAndGet(listS3Objects(s3Client, bucket, path), filter); } @@ -171,6 +173,7 @@ @Override public void copy(String bucket, String srcPath, FileReference destPath) { + srcPath = config.isEncodeKeys() ? encodeURI(srcPath) : srcPath; List<S3Object> objects = listS3Objects(s3Client, bucket, srcPath); for (S3Object object : objects) { String srcKey = object.key(); @@ -223,7 +226,7 @@ private Set<String> filterAndGet(List<S3Object> contents, FilenameFilter filter) { Set<String> files = new HashSet<>(); for (S3Object s3Object : contents) { - String path = S3Utils.decodeURI(s3Object.key()); + String path = config.isEncodeKeys() ? S3Utils.decodeURI(s3Object.key()) : s3Object.key(); if (filter.accept(null, IoUtil.getFileNameFromPath(path))) { files.add(path); } diff --git a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3Utils.java b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3Utils.java index 4ea11b2..2faba79 100644 --- a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3Utils.java +++ b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/aws/s3/S3Utils.java @@ -40,7 +40,7 @@ String newMarker = null; ListObjectsV2Response listObjectsResponse; ListObjectsV2Request.Builder listObjectsBuilder = ListObjectsV2Request.builder().bucket(bucket); - listObjectsBuilder.prefix(encodeURI(toCloudPrefix(path))); + listObjectsBuilder.prefix(toCloudPrefix(path)); while (true) { // List the objects from the start, or from the last marker in case of truncated result if (newMarker == null) { -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17614 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: I31f9955e8a9c4897eba4f819746b2dd47fa7c50f Gerrit-Change-Number: 17614 Gerrit-PatchSet: 2 Gerrit-Owner: Murtadha Al Hubail <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Murtadha Al Hubail <[email protected]> Gerrit-MessageType: merged
