From 4021c5d77ac7de8f0d1c9df3a7f44610ef77b57e Mon Sep 17 00:00:00 2001 From: Nikolay Sokolov Date: Wed, 11 Jan 2017 11:20:59 -0800 Subject: [PATCH] Avoid another extra getObjectMetadata request If client already knows last modified time for S3 object, there is no reason to pull object metadata from the S3 service inside TransferManager just to retrieve this information again. --- .../services/s3/model/GetObjectRequest.java | 83 ++++++++++++++++++- .../services/s3/transfer/TransferManager.java | 17 +++- .../s3/transfer/internal/DownloadImpl.java | 11 +-- 3 files changed, 99 insertions(+), 12 deletions(-) diff --git a/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/model/GetObjectRequest.java b/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/model/GetObjectRequest.java index 5511b9f3fa03..6524d712cba6 100644 --- a/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/model/GetObjectRequest.java +++ b/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/model/GetObjectRequest.java @@ -108,6 +108,8 @@ public class GetObjectRequest extends AmazonWebServiceRequest implements */ private Integer partNumber; + private Long lastModifiedTime; + /** * Constructs a new {@link GetObjectRequest} with all the required parameters. * @@ -121,7 +123,25 @@ public class GetObjectRequest extends AmazonWebServiceRequest implements * @see GetObjectRequest#GetObjectRequest(String, String, boolean) */ public GetObjectRequest(String bucketName, String key) { - this(bucketName, key, null); + this(bucketName, key, (String) null); + } + + /** + * Constructs a new {@link GetObjectRequest} with all the required parameters. + * + * @param bucketName + * The name of the bucket containing the desired object. + * @param key + * The key in the specified bucket under which the object is + * stored. + * @param lastModifiedTime + * Last modified time for the object known by the client + * + * @see GetObjectRequest#GetObjectRequest(String, String, String) + * @see GetObjectRequest#GetObjectRequest(String, String, boolean) + */ + public GetObjectRequest(String bucketName, String key, Long lastModifiedTime) { + this(bucketName, key, null, lastModifiedTime); } /** @@ -140,9 +160,31 @@ public GetObjectRequest(String bucketName, String key) { * @see GetObjectRequest#GetObjectRequest(String, String, boolean) */ public GetObjectRequest(String bucketName, String key, String versionId) { + this(bucketName, key, versionId, null); + } + + /** + * Constructs a new {@link GetObjectRequest} with all the required parameters. + * + * @param bucketName + * The name of the bucket containing the desired object. + * @param key + * The key in the specified bucket under which the object is + * stored. + * @param versionId + * The Amazon S3 version ID specifying a specific version of the + * object to download. + * @param lastModifiedTime + * Last modified time for the object known by the client + * + * @see GetObjectRequest#GetObjectRequest(String, String) + * @see GetObjectRequest#GetObjectRequest(String, String, boolean) + */ + public GetObjectRequest(String bucketName, String key, String versionId, Long lastModifiedTime) { setBucketName(bucketName); setKey(key); setVersionId(versionId); + setLastModifiedTime(lastModifiedTime); } public GetObjectRequest(S3ObjectId s3ObjectId) { @@ -1003,9 +1045,48 @@ public void setS3ObjectId(S3ObjectId s3ObjectId) { /** * Fluent API to set the S3 object id for this request. + * + * @return This {@link GetObjectRequest}, enabling additional method + * calls to be chained together. */ public GetObjectRequest withS3ObjectId(S3ObjectId s3ObjectId) { setS3ObjectId(s3ObjectId); return this; } + + /** + * Set last modified time known by the client. Useful to avoid roundtrip + * to S3 to fetch the metadata but not required + * + * @param lastModifiedTime + * Last modified time (in milliseconds) for this object + */ + public void setLastModifiedTime(Long lastModifiedTime) { + this.lastModifiedTime = lastModifiedTime; + } + + /** + * Set last modified time known by the client. Useful to avoid roundtrip + * to S3 to fetch the metadata but not required + * + * @param lastModifiedTime + * Last modified time (in milliseconds) for this object + * + * @return This {@link GetObjectRequest}, enabling additional method + * calls to be chained together. + */ + public GetObjectRequest withLastModifiedTime(Long lastModifiedTime) { + setLastModifiedTime(lastModifiedTime); + return this; + } + + /** + * Get last modified time for this object (in milliseconds). Only if + * set previously + */ + public Long getLastModifiedTime() { + return lastModifiedTime; + } + + } diff --git a/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/transfer/TransferManager.java b/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/transfer/TransferManager.java index 65851eaa8d5c..0ff967eb0c5b 100644 --- a/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/transfer/TransferManager.java +++ b/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/transfer/TransferManager.java @@ -1013,15 +1013,21 @@ private Download doDownload(final GetObjectRequest getObjectRequest, getObjectRequest .setGeneralProgressListener(new ProgressListenerChain(new TransferCompletionFilter(), listenerChain)); + // Defer actual request till later stage if it's required GetObjectMetadataRequest getObjectMetadataRequest = new GetObjectMetadataRequest( getObjectRequest.getBucketName(), getObjectRequest.getKey(), getObjectRequest.getVersionId()); if (getObjectRequest.getSSECustomerKey() != null) { getObjectMetadataRequest.setSSECustomerKey(getObjectRequest.getSSECustomerKey()); } - final ObjectMetadata objectMetadata = s3.getObjectMetadata(getObjectMetadataRequest); + + ObjectMetadata objectMetadata = null; // Used to check if the object is modified between pause and resume - long lastModifiedTime = objectMetadata.getLastModified().getTime(); + if (getObjectRequest.getLastModifiedTime() == null) { + objectMetadata = s3.getObjectMetadata(getObjectMetadataRequest); + long lastModifiedTime = lastModifiedTime = objectMetadata.getLastModified().getTime(); + getObjectRequest.setLastModifiedTime(lastModifiedTime); + } long startingByte = 0; long lastByte; @@ -1031,6 +1037,9 @@ private Download doDownload(final GetObjectRequest getObjectRequest, startingByte = range[0]; lastByte = range[1]; } else { + if (objectMetadata == null) { + objectMetadata = s3.getObjectMetadata(getObjectMetadataRequest); + } lastByte = objectMetadata.getContentLength() - 1; } @@ -1040,7 +1049,7 @@ private Download doDownload(final GetObjectRequest getObjectRequest, // We still pass the unfiltered listener chain into DownloadImpl final DownloadImpl download = new DownloadImpl(description, transferProgress, listenerChain, null, - stateListener, getObjectRequest, file, objectMetadata, isDownloadParallel); + stateListener, getObjectRequest, file, isDownloadParallel); long totalBytesToDownload = lastByte - startingByte + 1; transferProgress.setTotalBytesToTransfer(totalBytesToDownload); @@ -1062,7 +1071,7 @@ private Download doDownload(final GetObjectRequest getObjectRequest, long fileLength = -1; if (resumeExistingDownload) { - if (isS3ObjectModifiedSincePause(lastModifiedTime, lastModifiedTimeRecordedDuringPause)) { + if (isS3ObjectModifiedSincePause(getObjectRequest.getLastModifiedTime(), lastModifiedTimeRecordedDuringPause)) { throw new AmazonClientException("The requested object in bucket " + getObjectRequest.getBucketName() + " with key " + getObjectRequest.getKey() + " is modified on Amazon S3 since the last pause."); } diff --git a/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/transfer/internal/DownloadImpl.java b/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/transfer/internal/DownloadImpl.java index 818541d57529..2ee111042108 100644 --- a/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/transfer/internal/DownloadImpl.java +++ b/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/transfer/internal/DownloadImpl.java @@ -43,7 +43,6 @@ public class DownloadImpl extends AbstractTransfer implements Download { private final GetObjectRequest getObjectRequest; private final File file; - private final ObjectMetadata objectMetadata; private final ProgressListenerChain progressListenerChain; @Deprecated @@ -51,16 +50,14 @@ public DownloadImpl(String description, TransferProgress transferProgress, ProgressListenerChain progressListenerChain, S3Object s3Object, TransferStateChangeListener listener, GetObjectRequest getObjectRequest, File file) { this(description, transferProgress, progressListenerChain, s3Object, listener, - getObjectRequest, file, null, false); + getObjectRequest, file, false); } public DownloadImpl(String description, TransferProgress transferProgress, ProgressListenerChain progressListenerChain, S3Object s3Object, TransferStateChangeListener listener, - GetObjectRequest getObjectRequest, File file, - ObjectMetadata objectMetadata, boolean isDownloadParallel) { + GetObjectRequest getObjectRequest, File file, boolean isDownloadParallel) { super(description, transferProgress, progressListenerChain, listener); this.s3Object = s3Object; - this.objectMetadata = objectMetadata; this.getObjectRequest = getObjectRequest; this.file = file; this.progressListenerChain = progressListenerChain; @@ -77,7 +74,7 @@ public synchronized ObjectMetadata getObjectMetadata() { if (s3Object != null) { return s3Object.getObjectMetadata(); } - return objectMetadata; + return null; } /** @@ -190,7 +187,7 @@ private PersistableDownload captureDownloadState( getObjectRequest.getVersionId(), getObjectRequest.getRange(), getObjectRequest.getResponseHeaders(), getObjectRequest.isRequesterPays(), file.getAbsolutePath(), getLastFullyDownloadedPartNumber(), - getObjectMetadata().getLastModified().getTime()); + getObjectRequest.getLastModifiedTime()); } return null; }