cloudflare / backoff

Backoff timer shared between several projects.
BSD 2-Clause "Simplified" License
46 stars 8 forks source link

Use local random source. #2

Closed kisom closed 8 years ago

kisom commented 8 years ago

Right now, backoff uses the global math/rand state for jitter. This change moves the random source into the Backoff structure.

Additionally, the package currently tries to use a simple approach where initialisation happens on the fly. This doesn't provide a satisfactory method of handling the case where there is an error in setting up the RNG. This change requires initialisation via a New function, which returns an error if the RNG cannot be setup.

lmb commented 8 years ago

I think this PR is the right direction: fixes RNG, make Jitter the default.

Some thoughts:

terinjokes commented 8 years ago
  • Seed a package wide RNG on init, with /dev/urandom. I would prefer it crashing if reading randomness fails. My daemons will all be under process supervision, so transient errors should go away. Just relying on local time might bite us in case we do concurrent updates or similar.

:+1:

lmb commented 8 years ago

Kyle, Any comments? Which direction do you want to move with this?

kisom commented 8 years ago

@lmb:

The code was updated to address your concerns:

In general, the latest code prefers the approach of creating a new Backoff if parameters need to be changed.

lmb commented 8 years ago

LGTM once we figure out what to do about Tries()!

lmb commented 8 years ago

LGTM, squash?

kisom commented 8 years ago

Squashed.