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

backoff.expo base #42

Open
gregroberts opened this issue Aug 16, 2017 · 4 comments
Open

backoff.expo base #42

gregroberts opened this issue Aug 16, 2017 · 4 comments

Comments

@gregroberts
Copy link

First off, crackin' little library. Really like the api.

However, in backoff.expo:

https://github.com/litl/backoff/blob/master/backoff/_wait_gen.py#L14

Having n=0 means the second attempt will always happen only 1 second after the first attempt, regardles of base, factor.

My assumption was that base would set the minimum backoff time.

Is this intended behaviour? To me it seems counter-intuitive to the meaning of the 'base' parameter!

This could be addressed in one of two simple ways.

Change
https://github.com/litl/backoff/blob/master/backoff/_wait_gen.py#L14

to

n = 1

OR

change

https://github.com/litl/backoff/blob/master/backoff/_wait_gen.py#L16

to

a = base * factor ** n

Either of these changes would mean the minimum retry interval was equal to base, and not 1.

@bgreen-litl
Copy link
Member

Sorry for the slow response - I was away last week.

You may be right that it should be

a = base * factor ** n

but I need to refresh my thinking about base and factor - I believe those were included when adding support for the jitter algorithm from https://www.awsarchitectureblog.com/2015/03/backoff.html

I will get back to you when I have a few minutes to investigate.

@gregroberts
Copy link
Author

Happy to submit a ludicrously small PR if you like ; )

@tasercake
Copy link

tasercake commented Aug 31, 2018

I think the equation itself makes sense as it is.

In

a = factor * (base ** n)

base is the component that's exponentiated, and the result is scaled by factor.
There might be some confusion due to the lack of parentheses in the source.

If I'm not mistaken, it looks like the only change required here is to set n = 1, although you could simply adjust factor to get whatever starting value you'd like.

@flaurencin
Copy link

I think the equation itself makes sense as it is.

In

a = factor * (base ** n)

base is the component that's exponentiated, and the result is scaled by factor.
There might be some confusion due to the lack of parentheses in the source.

If I'm not mistaken, it looks like the only change required here is to set n = 1, although you could simply adjust factor to get whatever starting value you'd like.

I prefer this approach.

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

4 participants