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
Introduce exponentially increasing delay after each join attempt #68
Conversation
@@ -74,21 +74,20 @@ func (s *BootstrapTestSuite) TestBootstrapJoinsTimeOut() { | |||
} | |||
|
|||
func (s *BootstrapTestSuite) TestBootstrapDestroy() { | |||
var lerr lError | |||
errChan := make(chan error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NOTES TO REVIEWER: Don't mind these changes. I cherry-picked a commit from #67 so that these tests pass. I'll delete this commit from this PR when that PR is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that #67 is merged can we undo this from this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
of course. i will take care of it.
5f85d33
to
8bc79ab
Compare
defaultMax = float64(60 * time.Second) | ||
) | ||
|
||
var defaultRandomizer = rand.Intn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this randomizer uses the default source that has deterministic behavior. Lets make our own Rand
with a newly seeded Source
. To prevent interference with the Source
of the application we are embedded in -that application might rely on the fact that random numbers are deterministic- and at the same time prevent all ringpops to use the same 'random' timeouts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a little bit more complicated then that. join_sender.go calls rand.Seed()
and I was relying on that as the defaultRandomizer's seed. But that's non-obvious and error prone. I'll move the call to Seed() to the delayer itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought, I'll leave the call to Seed() in join_sender.go and create a new instance of type Rand
with its own source.
lgtm |
} | ||
|
||
// nullDelayer is an empty implementation of joinDelayer. | ||
type nullDelayer struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the nullDelayer crammed in between the exponentialDelayer initializer and method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know. I'll move it and make it a bit more organized.
Tests are failing :( |
c72c272
to
f57dbf9
Compare
Looks good! |
Introduce exponentially increasing delay after each join attempt
This PR adds a delay in between each join attempt. The delay is a calculation of an exponential backoff with added jitter/fuzz. The maximum potential delay will increase until it is capped at its max. The actual delay will be a random number between the last used max and the new max. Therefore, after the first join attempt, the delay will be between 0 and 100. On the second attempt, 100 and 200. On the third attempt, between 200 and 400 and so on.
cc @uber/ringpop