awslabs / dynamodb-data-mapper-js

A schema-based data mapper for Amazon DynamoDB.
https://awslabs.github.io/dynamodb-data-mapper-js/
Apache License 2.0
816 stars 106 forks source link

Retry & exponential backoff retry for batchPut doesn't work for ProvisionedThroughputExceededException #202

Open infigenie opened 3 years ago

infigenie commented 3 years ago

Retry & exponential backoff retry for batchPut doesn't work for ProvisionedThroughputExceededException. We have to handle manually if ProvisionedThroughputExceededException is thrown.

I am curious to know if there's any plan to include that in the code itself or we should continue handling it all by ourselves.

rationull commented 3 years ago

@infigenie I can't speak to the plan here (full disclosure: I am an Amazon employee but just another user of this library, not a maintainer), but just a note that what you bring up here sounds like the described/spec'd behavior from the readme to my reading:

Partial successes (i.e., BatchGetItem operations that return some responses and some unprocessed keys or BatchWriteItem operations that return some unprocessed items) will retry the unprocessed items automatically using exponential backoff.

I'm guessing you're referring to ProvisionedThroughputExceededException which fails a whole batch operation. This behavior is rooted in the AWS DynamoDB API and client (e.g. see the batchWriteItem docs) where partial batch failures are expressed via an UnprocessedItems field in the response and full batch failures are expressed via ProvisionedThroughputExceededException.

My guess is the idea here is to behave similarly to the underlying client and that users of the library need to handle retries in this case. I don't know where this is more intentional or incidental to the implementation, but it seems to be the requirement at this point at least.

This issue alludes to some potential retries within the AWS SDK itself but I don't know whether that applies to ProvisionedThroughputExceededException failures.

omrishaked commented 1 year ago

I'm also experiencing the same thing (and what @rationull says makes sense to me), and would expect DataMapper to handle retries for throughput exceptions as well. Having to handle retries/backoff myself for this exception misses the whole point of library retries/backoff.