Adds jitter to DynamoDB retries and allows custom retry logic (#1418)

* Adds jitter to DynamoDB retries and allows custom retry logic

* Updates retry tests

Author: chrisradek

.changes/next-release/feature-DynamoDB-01315737.json

@@ -0,0 +1,5 @@
+{
+  "type": "feature",
+  "category": "DynamoDB",
+  "description": "Adds ability to customize retry delays for DynamoDB. This previously worked for all services except DynamoDB. Also adds jitter to DynamoDB retries. See `AWS.Config.retryDelayOptions` for more information."
+}

lib/config.js

@@ -96,12 +96,12 @@ var PromisesDependency;
  *     AWS.config.update({retryDelayOptions: {customBackoff: function(retryCount) {
  *       // returns delay in ms
  *     }}});
- *   @note This works with all services except DynamoDB.
  *   @return [map] A set of options to configure the retry delay on retryable errors.
  *     Currently supported options are:
  *
  *     * **base** [Integer] — The base number of milliseconds to use in the
- *       exponential backoff for operation retries. Defaults to 100 ms.
+ *       exponential backoff for operation retries. Defaults to 100 ms for all services accept
+ *       DynamoDB, where it defaults to 50ms.
  *     * **customBackoff ** [function] — A custom function that accepts a retry count
  *       and returns the amount of time to delay in milliseconds. The `base` option will be
  *       ignored if this option is supplied.
@@ -216,7 +216,8 @@ AWS.Config = AWS.util.inherit({
    *   the retry delay on retryable errors. Currently supported options are:
    *
    *   * **base** [Integer] — The base number of milliseconds to use in the
-   *     exponential backoff for operation retries. Defaults to 100 ms.
+   *     exponential backoff for operation retries. Defaults to 100 ms for all
+   *     services accept DynamoDB, where it defaults to 50ms.
    *   * **customBackoff ** [function] — A custom function that accepts a retry count
    *     and returns the amount of time to delay in milliseconds. The `base` option will be
    *     ignored if this option is supplied.
@@ -477,9 +478,7 @@ AWS.Config = AWS.util.inherit({
     systemClockOffset: 0,
     signatureVersion: null,
     signatureCache: true,
-    retryDelayOptions: {
-      base: 100
-    },
+    retryDelayOptions: {},
     useAccelerateEndpoint: false
   },
 

lib/services/dynamodb.js

@@ -47,7 +47,12 @@ AWS.util.update(AWS.DynamoDB.prototype, {
    * @api private
    */
   retryDelays: function retryDelays(retryCount) {
-    var delay = retryCount > 0 ? (50 * Math.pow(2, retryCount - 1)) : 0;
+    var retryDelayOptions = AWS.util.copy(this.config.retryDelayOptions);
+
+    if (typeof retryDelayOptions.base !== 'number') {
+        retryDelayOptions.base = 50; // default for dynamodb
+    }
+    var delay = AWS.util.calculateRetryDelay(retryCount, retryDelayOptions);
     return delay;
   }
 });

lib/util.js

@@ -824,7 +824,7 @@ var util = {
     if (typeof customBackoff === 'function') {
       return customBackoff(retryCount);
     }
-    var base = retryDelayOptions.base || 100;
+    var base = typeof retryDelayOptions.base === 'number' ? retryDelayOptions.base : 100;
     var delay = Math.random() * (Math.pow(2, retryCount) * base);
     return delay;
   },

test/config.spec.coffee

@@ -60,8 +60,6 @@ describe 'AWS.Config', ->
       expect(configure(maxRetries: 2).maxRetries).to.equal(2)
 
   describe 'retryDelayOptions', ->
-    it 'defaults to "base: 100"', ->
-      expect(configure().retryDelayOptions).to.eql({base: 100})
     it 'can set "base" to an integer', ->
       expect(configure(retryDelayOptions: {base: 30}).retryDelayOptions).to.eql({base: 30})
 

test/service.spec.coffee

@@ -337,6 +337,30 @@ describe 'AWS.Service', ->
       service.config.maxRetries = undefined
       expect(service.numRetries()).to.equal(13)
 
+  describe 'retryDelays', ->
+    beforeEach ->
+      helpers.spyOn(Math, 'random').andReturn 1
+
+    it 'has a default delay base of 100 ms', ->
+      client = new AWS.Service({})
+      expectedDelays = [100, 200, 400 ]
+      actualDelays = (client.retryDelays(i) for i in [0..client.numRetries()-1])
+      expect(actualDelays).to.eql(expectedDelays)
+
+    it 'can accept a user-defined delay base', ->
+      client = new AWS.Service({retryDelayOptions: {base: 200}})
+      expectedDelays = [ 200, 400, 800 ]
+      actualDelays = (client.retryDelays(i) for i in [0..client.numRetries()-1])
+      expect(actualDelays).to.eql(expectedDelays)
+    
+    it 'can accept a user-defined custom backoff', ->
+      customBackoff = (retryCount) ->
+        return 100 * retryCount
+      client = new AWS.Service({retryDelayOptions: {customBackoff: customBackoff}})
+      expectedDelays = [ 0, 100, 200 ]
+      actualDelays = (client.retryDelays(i) for i in [0..client.numRetries()-1])
+      expect(actualDelays).to.eql(expectedDelays)
+
   describe 'defineMethods', ->
     operations = null
     serviceConstructor = null

test/services/dynamodb.spec.coffee

@@ -27,12 +27,28 @@ describe 'AWS.DynamoDB', ->
       expect(ddb({ maxRetries: 2 }).numRetries()).to.equal(2)
 
   describe 'retryDelays', ->
+    beforeEach ->
+      helpers.spyOn(Math, 'random').andReturn 1
 
     it 'has a custom backoff function', ->
-      expectedDelays = [ 0, 50, 100, 200, 400, 800, 1600, 3200, 6400, 12800 ]
+      expectedDelays = [ 50, 100, 200, 400, 800, 1600, 3200, 6400, 12800, 25600 ]
       actualDelays = (client.retryDelays(i) for i in [0..client.numRetries()-1])
       expect(actualDelays).to.eql(expectedDelays)
 
+    it 'can accept a user-defined delay base', ->
+      service = ddb({retryDelayOptions: {base: 100}})
+      expectedDelays = [ 100, 200, 400, 800, 1600, 3200, 6400, 12800, 25600, 51200 ]
+      actualDelays = (service.retryDelays(i) for i in [0..service.numRetries()-1])
+      expect(actualDelays).to.eql(expectedDelays)
+    
+    it 'can accept a user-defined custom backoff', ->
+      customBackoff = (retryCount) ->
+        return 100 * retryCount
+      service = ddb({retryDelayOptions: {customBackoff: customBackoff}})
+      expectedDelays = [ 0, 100, 200, 300, 400, 500, 600, 700, 800, 900 ]
+      actualDelays = (service.retryDelays(i) for i in [0..service.numRetries()-1])
+      expect(actualDelays).to.eql(expectedDelays)
+
   describe 'CRC32 check', ->
     dynamo = null