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

exponentialBackoff() behavior question #204

Closed rationull closed 2 years ago

rationull commented 3 years ago

In BatchOperation, the current implementation of exponentialBackoff()'s combined use of Math.floor and Math.random means that you can get delays as low as 0 milliseconds even with a large attempt number. For instance, running the following generally shows a large number of 0 results:

for (let i = 0; i < 1000000; i++) {
   if (exponentialBackoff(15) === 0) {
      console.log('0 backoff')
   }
}

Two questions related to this:

  1. Math.random() here effectively allows jittering down to very small delays even after many attempts (since very small random fractions will take even a large power of 2 down to a small number of milliseconds). Is this an intentional part of the backoff + jitter strategy or is it actually a bug where the delay should be more straightforwardly exponential and the random jitter a much smaller component?
  2. If (1) is intended, then is the use of floor() intentional to allow 0 or would ceil() be a slightly better fit to at least ensure some delay (1 ms + timer and event loop latency rather than just event loop latency)? This is probably irrelevant since I doubt there's much difference between requesting a 0 vs a 1 ms delay when it comes to the JS event loop, but I was curious about the intent.
jeskew commented 2 years ago

The intent with the exponentialBackoff function was to implement the strategy referred to as "full jitter" in Marc Booker's blog post, Exponential Backoff and Jitter. It is expected that some backoffs will be 0ms in length.

rationull commented 2 years ago

Thanks @jeskew -- I appreciate the very interesting and insightful reference. This makes sense now, obviously not an issue and something to apply to my own projects in the future :)

I'll go ahead and close this issue.