DynamoDBMapper batchSave would not retry retryable exceptions

[skborissov]

Describe the issue

DynamoDBMapper::batchSave uses AmazonDynamoDB::batchWriteItem, but does not retry when the latter throws an exception that is documented as retryable. E.g. from AmazonDynamoDB docs:

@throws ProvisionedThroughputExceededException - Your request rate is too high. The AWS SDKs for DynamoDB automatically retry requests that receive this exception.

DynamoDBMapper doesn't retry on any exception. The code:

            try {
                result = db.batchWriteItem(applyBatchOperationUserAgent(
                        new BatchWriteItemRequest().withRequestItems(pendingItems)));
            } catch (Exception e) {
                failedBatch = new FailedBatch();
                failedBatch.setUnprocessedItems(pendingItems);
                failedBatch.setException(e);
                return failedBatch;
            }
            /* retry logic below */

Steps to Reproduce

You'd need to induce throttling somehow.

Current Behavior

DynamoDBMapper::batchSave only covers AmazonDynamoDB::batchWriteItem retry contract partially. If this is by design, it should be documented that DynamoDBMapper clients should implement retry/backoff logic additional to what DynamoDBMapper already does.

Your Environment

• AWS Java SDK version used: 1.11.908
• JDK version used: openjdk 11.0.6 2020-01-14
• Operating System and version: macOS Mojave 10.14.6

[debora-ito]

@skborissov The low-level db.batchWriteItem() will not return an exception if one single operation of the batch failed to be processed, it will return a successful 200 OK response code in this case (see the example in the API Reference) and will not trigger the catch block.

Was this your concern? Are you experiencing the lack of retries by running the code?

[skborissov]

@skborissov The low-level db.batchWriteItem() will not return an exception if one single operation of the batch failed to be processed, it will return a successful 200 OK response code in this case (see the example in the API Reference) and will not trigger the catch block.

Was this your concern? Are you experiencing the lack of retries by running the code?

No, that behavior is expected. My problem is the following case:

1. I call DynamoDBMapper::batchSave with my batch.
2. It calls the lower-evel AmazonDynamoDB::batchWriteItem.
3. All items in the batch fail due to throttling.
4. AmazonDynamoDB::batchWriteItem throws a ProvisionedThroughputExceededException. (batch write docs specifically state this, it's also in the javadoc)
5. DynamoDBMapper catches the exception and does not retry (i.e. we enter the catch block in my first snippet), even though the AmazonDynamoDB::batchWriteItem contract states that its clients should retry in this case - "The AWS SDKs for DynamoDB automatically retry requests that receive this exception.".

My problem is that to cover this retryable case I'd have to write my own retry and exponential backoff logic around DynamoDBMapper::batchSave. But it seems like DynamoDBMapper::batchSave should already be handling this case internally - in no part of its doc does it say that you should retry it in any situation.

[debora-ito]

Got it, thank you for clarifying. Yes, it looks like batchSave does not retry if the whole batchWriteItem operation gets an exception. I'll mark this as a documentation issue, we should make it more clear. It's related to https://github.com/aws/aws-sdk-java/issues/2394.

[skborissov]

@debora-ito thanks, I agree that the documentation needs updating.

What do you think about changing the DynamoDBMapper::batchSave to retry on ProvisionedThroughputExceededException, and potentially InternalServerErrorException? From an API design perspective, It doesn't make sense to me for DynamoDBMapper::batchSave to retry in some cases and expect it's caller to retry in others.

I would be happy to make a PR for this, it doesn't seem like it'd be a big change.

[github-actions[bot]]

This is a very old issue that is probably not getting as much attention as it deserves. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to provide a comment or open a new issue.