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

common.Backoff now implements jitter instead of sleeping for a fixed amount of time #10177

Merged
merged 2 commits into from
Jan 21, 2019

Conversation

ph
Copy link
Contributor

@ph ph commented Jan 18, 2019

This PR add a new interface called backoff.Backoff, this can be used to
generalize any backoff interaction. It move the current Backoff strategy
under an ExpBackoff type.

ExpBackoff is the same as before on every wait we just
exponentially increase the duration of the wait and sleep for that
amount.

EqualJitterBackoff uses an exponential increment of the duration but
will take half of that value as fixed sleep time and the other half
as a jitter. This will help distribute the new request when a cluster is
done instead of having all the beats trying to reconnect at once.

The Redis implementations and any clients wrapped with a backoff will
now use the EqualJitterBackoff, any other code will keep using the same
exponential strategy.

Fixes: #10172

@ph ph added review libbeat needs_backport PR is waiting to be backported to other branches. labels Jan 18, 2019
@ph ph requested a review from urso January 18, 2019 17:03
@ph ph requested a review from a team as a code owner January 18, 2019 17:03
@ph ph changed the title common.Backoff now implements jitter instead of sleeping for a fixed common.Backoff now implements jitter instead of sleeping for a fixed amount of time Jan 18, 2019
libbeat/common/backoff.go Outdated Show resolved Hide resolved
@ph ph added in progress Pull request is currently in progress. and removed review labels Jan 21, 2019
@ph ph force-pushed the feature/backoff-jitter branch 2 times, most recently from da6225f to 3f8f06e Compare January 21, 2019 17:11
@ph
Copy link
Contributor Author

ph commented Jan 21, 2019

@urso I've added the new interface and move any reference of *common.Backoff to use that interface, I haven't made it configurable by the user for this we probably want a bigger discussion so for now I left it to the implementer. Currently, Redis and output/backoff will now use the EqualJitterBackoff strategy as the default.

This mean that the default pipeline, the monitoring pipeline or any user of Redis will now have some random in it.

@kvch Journalbeat will now use the interface instead of the type for the backoff field but will still use the ExpBackoff strategy.

@ph
Copy link
Contributor Author

ph commented Jan 21, 2019

Note: I have added a minimal test suite for both the implementation that tests:

  • can be interrupted by the close channel
  • can wait for some time

This PR add a new interface called backoff.Backoff, this can be used to
generalize any backoff interaction. It move the current Backoff strategy
under an ExpBackoff type.

ExpBackoff times is the same as before on every wait we just
exponentially increase the duration of the wait and sleep for that
amount.

EqualJitterBackoff uses an exponential increment of the duration but
will take half of that value as a fixed sleep time and the other half
as a jitter. This will help distribute the new request when a cluster is
done instead of having all the beats trying to reconnect at once.

The Redis implementations and any clients wrapped with a backoff will
now use the EqualJitterBackoff, any other code will keep using the same
exponential strategy.

Fixes: elastic#10172
@ph ph force-pushed the feature/backoff-jitter branch from 3f8f06e to 5d7864b Compare January 21, 2019 17:16
@ph
Copy link
Contributor Author

ph commented Jan 21, 2019

@urso Also this is not a full jitter, more like an EqualJitter in that case, this ensures that the connection will at least sleep a period of time before retrying again, looking at the numbers from the article they look pretty similar with the rates.

b.duration *= 2
if b.duration > b.max {
b.duration = b.max
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: By moving this block before the computation of temp you don't need the init*2 on reset. In the end it doesn't matter :)

@ph
Copy link
Contributor Author

ph commented Jan 21, 2019

Let's merge into 6.6 and 6.x

@ph ph added review and removed in progress Pull request is currently in progress. labels Jan 21, 2019
@ph ph merged commit 4cb9bd7 into elastic:master Jan 21, 2019
ph added a commit to ph/beats that referenced this pull request Jan 21, 2019
…amount of time (elastic#10177)

This PR add a new interface called backoff.Backoff, this can be used to
generalize any backoff interaction. It move the current Backoff strategy
under an ExpBackoff type.

ExpBackoff is the same as before on every wait we just
exponentially increase the duration of the wait and sleep for that
amount.

EqualJitterBackoff uses an exponential increment of the duration but
will take half of that value as fixed sleep time and the other half
as a jitter. This will help distribute the new request when a cluster is
done instead of having all the beats trying to reconnect at once.

The Redis implementations and any clients wrapped with a backoff will
now use the EqualJitterBackoff, any other code will keep using the same
exponential strategy.

Fixes: elastic#10172
(cherry picked from commit 4cb9bd7)
@ph ph added v6.7.0 and removed needs_backport PR is waiting to be backported to other branches. labels Jan 21, 2019
ph added a commit to ph/beats that referenced this pull request Jan 21, 2019
…amount of time (elastic#10177)

This PR add a new interface called backoff.Backoff, this can be used to
generalize any backoff interaction. It move the current Backoff strategy
under an ExpBackoff type.

ExpBackoff is the same as before on every wait we just
exponentially increase the duration of the wait and sleep for that
amount.

EqualJitterBackoff uses an exponential increment of the duration but
will take half of that value as fixed sleep time and the other half
as a jitter. This will help distribute the new request when a cluster is
done instead of having all the beats trying to reconnect at once.

The Redis implementations and any clients wrapped with a backoff will
now use the EqualJitterBackoff, any other code will keep using the same
exponential strategy.

Fixes: elastic#10172
(cherry picked from commit 4cb9bd7)
@ph ph added the v6.6.0 label Jan 21, 2019
ph added a commit that referenced this pull request Jan 23, 2019
…ad of sleeping for a fixed amount of time (#10229)

Cherry-pick of PR #10177 to 6.x branch. Original message: 

This PR add a new interface called backoff.Backoff, this can be used to
generalize any backoff interaction. It move the current Backoff strategy
under an ExpBackoff type.

ExpBackoff is the same as before on every wait we just
exponentially increase the duration of the wait and sleep for that
amount.

EqualJitterBackoff uses an exponential increment of the duration but
will take half of that value as fixed sleep time and the other half
as a jitter. This will help distribute the new request when a cluster is
done instead of having all the beats trying to reconnect at once.

The Redis implementations and any clients wrapped with a backoff will
now use the EqualJitterBackoff, any other code will keep using the same
exponential strategy.

Fixes: #10172
ph added a commit that referenced this pull request Jan 23, 2019
…amount of time (#10177) (#10230)

This PR add a new interface called backoff.Backoff, this can be used to
generalize any backoff interaction. It move the current Backoff strategy
under an ExpBackoff type.

ExpBackoff is the same as before on every wait we just
exponentially increase the duration of the wait and sleep for that
amount.

EqualJitterBackoff uses an exponential increment of the duration but
will take half of that value as fixed sleep time and the other half
as a jitter. This will help distribute the new request when a cluster is
done instead of having all the beats trying to reconnect at once.

The Redis implementations and any clients wrapped with a backoff will
now use the EqualJitterBackoff, any other code will keep using the same
exponential strategy.

Fixes: #10172
(cherry picked from commit 4cb9bd7)
DStape pushed a commit to DStape/beats that referenced this pull request Aug 20, 2019
…amount of time (elastic#10177)

This PR add a new interface called backoff.Backoff, this can be used to
generalize any backoff interaction. It move the current Backoff strategy
under an ExpBackoff type.

ExpBackoff is the same as before on every wait we just
exponentially increase the duration of the wait and sleep for that
amount.

EqualJitterBackoff uses an exponential increment of the duration but
will take half of that value as fixed sleep time and the other half
as a jitter. This will help distribute the new request when a cluster is
done instead of having all the beats trying to reconnect at once.

The Redis implementations and any clients wrapped with a backoff will
now use the EqualJitterBackoff, any other code will keep using the same
exponential strategy.

Fixes: elastic#10172
DStape pushed a commit to DStape/beats that referenced this pull request Aug 20, 2019
…r instead of sleeping for a fixed amount of time (elastic#10229)

Cherry-pick of PR elastic#10177 to 6.x branch. Original message: 

This PR add a new interface called backoff.Backoff, this can be used to
generalize any backoff interaction. It move the current Backoff strategy
under an ExpBackoff type.

ExpBackoff is the same as before on every wait we just
exponentially increase the duration of the wait and sleep for that
amount.

EqualJitterBackoff uses an exponential increment of the duration but
will take half of that value as fixed sleep time and the other half
as a jitter. This will help distribute the new request when a cluster is
done instead of having all the beats trying to reconnect at once.

The Redis implementations and any clients wrapped with a backoff will
now use the EqualJitterBackoff, any other code will keep using the same
exponential strategy.

Fixes: elastic#10172
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…amount of time (elastic#10177) (elastic#10230)

This PR add a new interface called backoff.Backoff, this can be used to
generalize any backoff interaction. It move the current Backoff strategy
under an ExpBackoff type.

ExpBackoff is the same as before on every wait we just
exponentially increase the duration of the wait and sleep for that
amount.

EqualJitterBackoff uses an exponential increment of the duration but
will take half of that value as fixed sleep time and the other half
as a jitter. This will help distribute the new request when a cluster is
done instead of having all the beats trying to reconnect at once.

The Redis implementations and any clients wrapped with a backoff will
now use the EqualJitterBackoff, any other code will keep using the same
exponential strategy.

Fixes: elastic#10172
(cherry picked from commit 7e68306)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

common.Backoff doesn't have any form of jitter.
3 participants