From 0da20579caf3217a8977274f8b98e35ea1b27761 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 31 Aug 2020 11:05:59 +0200 Subject: [PATCH] Cleanly Handle S3 SDK Exceptions in Request Counting (#61686) (#61698) It looks like it is possible for a request to throw an exception early before any API interaciton has happened. This can lead to the request count map containing a `null` for the request count key. The assertion is not correct and we should not NPE here (as that might also hide the original exception since we are running this code in a `finally` block from within the S3 SDK). Closes #61670 --- .../org/elasticsearch/repositories/s3/S3BlobStore.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java index 10c0c0635e4..e32a7dd6fd2 100644 --- a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java +++ b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java @@ -25,6 +25,8 @@ import com.amazonaws.metrics.RequestMetricCollector; import com.amazonaws.services.s3.model.CannedAccessControlList; import com.amazonaws.services.s3.model.StorageClass; import com.amazonaws.util.AWSRequestMetrics; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.elasticsearch.cluster.metadata.RepositoryMetadata; import org.elasticsearch.common.blobstore.BlobContainer; import org.elasticsearch.common.blobstore.BlobPath; @@ -40,6 +42,8 @@ import java.util.concurrent.atomic.AtomicLong; class S3BlobStore implements BlobStore { + private static final Logger logger = LogManager.getLogger(S3BlobStore.class); + private final S3Service service; private final String bucket; @@ -105,8 +109,10 @@ class S3BlobStore implements BlobStore { private long getRequestCount(Request request) { Number requestCount = request.getAWSRequestMetrics().getTimingInfo() .getCounter(AWSRequestMetrics.Field.RequestCount.name()); - assert requestCount != null; - + if (requestCount == null) { + logger.warn("Expected request count to be tracked for request [{}] but found not count.", request); + return 0L; + } return requestCount.longValue(); }