Skip to content

Commit

Permalink
Cleanly Handle S3 SDK Exceptions in Request Counting (#61686) (#61699)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
original-brownbear committed Aug 31, 2020
1 parent 6eb4748 commit 7d2eca9
Showing 1 changed file with 8 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
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;
Expand All @@ -40,6 +42,8 @@

class S3BlobStore implements BlobStore {

private static final Logger logger = LogManager.getLogger(S3BlobStore.class);

private final S3Service service;

private final String bucket;
Expand Down Expand Up @@ -105,8 +109,10 @@ public void collectMetrics(Request<?> request, Response<?> response) {
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();
}

Expand Down

0 comments on commit 7d2eca9

Please sign in to comment.