Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retry.timeouts() are descending when using a factor < 1 #3

Closed
felixge opened this issue Nov 3, 2011 · 1 comment
Closed

Retry.timeouts() are descending when using a factor < 1 #3

felixge opened this issue Nov 3, 2011 · 1 comment

Comments

@felixge
Copy link
Contributor

felixge commented Nov 3, 2011

When using a factor < 1, the longest timeout will come first:

> r = require('retry')
{ operation: [Function], timeouts: [Function], _createTimeout: [Function] }
> r.timeouts({factor: 0.5, retries: 3})
[ 1000, 500, 250 ]

To fix this, the resulting timeout array should be sorted in ascending order.

--fg

@tim-kos tim-kos closed this as completed in 3431ef4 Nov 3, 2011
@rahpuser
Copy link

rahpuser commented Oct 4, 2016

Inverting the array would break the maxTimeout, wouldn't ?. now the minTimeout would also be the maxTimeout ?

> r.timeouts({factor: 0.5, retries: 3, minTimeout: 1000})
[ 1000, 500, 250 ] // original result

A good output to be consistent with others timeouts would be
[1000, 1250, 1500]

Let me know what are yours thoughts about it.

UPDATE.
I just realized about another option, a factor < 1, implicitly says it wants to grow each time less, so consider the normal output:

> r.timeouts({factor: 0.5, retries: 3, minTimeout: 1000})
[ 1000, 500, 250 ] // 125, 87, etc

A timeouts array that grow each time less:

original output:          [ 1000, 500, 250 ] // 125, 87, etc
slow grow 1:        [1000, 2000, 2500, 2750] 
slow grow 2:              [1000, 1500, 1750]

This last example for me is more natural for what factor < 1 should be doing.

In the other hand, the output [ 250, 500, 1000 ] is easy to get if you specify the correct timeouts parameters in the following way:

> retry.timeouts({factor: 2, retries: 3, minTimeout: 250})
[ 250, 500, 1000 ]

So, please let me know what do you guys think about this.
@tim-kos @felixge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants