Address Wieger's feedback; use time.Duration for delayOpts

swim/join_delayer.go

@@ -26,13 +26,14 @@ import (
 	"time"
 
 	"github.com/uber-common/bark"
+	"github.com/uber/ringpop-go/util"
 )
 
 var errLoggerRequired = errors.New("joinDelayer requires a logger")
 
 const (
-	defaultInitial = float64(100 * time.Millisecond)
-	defaultMax     = float64(60 * time.Second)
+	defaultInitial = 100 * time.Millisecond
+	defaultMax     = 60 * time.Second
 )
 
 var delayerRand = rand.New(rand.NewSource(time.Now().UnixNano()))
@@ -56,8 +57,8 @@ type delaySleeper func(time.Duration)
 // delayOpts is a struct that houses substitutable parameters of a joinDelayer
 // in order to alter its behavior.
 type delayOpts struct {
-	initial    float64
-	max        float64
+	initial    time.Duration
+	max        time.Duration
 	randomizer delayRandomizer
 	sleeper    delaySleeper
 }
@@ -83,10 +84,10 @@ type exponentialDelayer struct {
 
 	// initialDelay is the fixed portion of the delay by which
 	// the exponential backoff is multiplied.
-	initialDelay float64
+	initialDelay time.Duration
 
 	// maxDelay is the maximum delay applied to a join attempt.
-	maxDelay float64
+	maxDelay time.Duration
 
 	// maxDelayReached is a flag that is toggled once the delay
 	// applied to a join attempt reaches or exceeds the max delay.
@@ -151,12 +152,16 @@ func newExponentialDelayer(joiner string, logger bark.Logger,
 // the number of attempts is 0-based. It returns a time.Duration equal to the
 // amount of delay applied.
 func (d *exponentialDelayer) delay() time.Duration {
+	// Convert durations to time in millis
+	initialDelayMs := float64(util.MS(d.initialDelay))
+	maxDelayMs := float64(util.MS(d.maxDelay))
+
 	// Compute uncapped exponential delay (exponent is the number of join
 	// attempts so far). Then, make sure the computed delay is capped at its
 	// max. Apply a random jitter to the actual sleep duration and finally,
 	// sleep.
-	uncappedDelay := d.initialDelay * math.Pow(2, float64(d.numDelays))
-	cappedDelay := math.Min(d.maxDelay, uncappedDelay)
+	uncappedDelay := initialDelayMs * math.Pow(2, float64(d.numDelays))
+	cappedDelay := math.Min(maxDelayMs, uncappedDelay)
 
 	// If cappedDelay and nextDelayMin are equal, we have reached the point
 	// at which the exponential backoff has reached its max; apply no more
@@ -170,7 +175,7 @@ func (d *exponentialDelayer) delay() time.Duration {
 
 	// If this is the first time an uncapped delay reached or exceeded the
 	// maximum allowable delay, log a message.
-	if uncappedDelay >= d.maxDelay && d.maxDelayReached == false {
+	if uncappedDelay >= initialDelayMs && d.maxDelayReached == false {
 		d.logger.WithFields(bark.Fields{
 			"local":         d.joiner,
 			"numDelays":     d.numDelays,
@@ -187,13 +192,13 @@ func (d *exponentialDelayer) delay() time.Duration {
 	// Set lower-bound for next attempt to maximum of current attempt.
 	d.nextDelayMin = cappedDelay
 
-	actualDelay := time.Duration(jitteredDelay)
-	d.sleeper(actualDelay)
+	sleepDuration := time.Duration(jitteredDelay) * time.Millisecond
+	d.sleeper(sleepDuration)
 
 	// Increment the exponent used for backoff calculation.
 	d.numDelays++
 
-	return actualDelay
+	return sleepDuration
 }
 
 // nullDelayer is an empty implementation of joinDelayer.

swim/join_delayer_test.go

@@ -35,23 +35,23 @@ var noSleep = func(d time.Duration) {}
 type joinDelayerTestSuite struct {
 	suite.Suite
 	delayer        *exponentialDelayer
-	expectedDelays [6]float64
+	expectedDelays [6]time.Duration
 }
 
 func (s *joinDelayerTestSuite) SetupTest() {
 	opts := &delayOpts{
-		initial: 100,
-		max:     1000,
+		initial: 100 * time.Millisecond,
+		max:     1 * time.Second,
 		sleeper: noSleep, // Sleeper used for tests applies no actual delay
 	}
 
 	// Represents the steps of a complete exponential backoff
 	// as implemented by the exponentialDelayer.
-	s.expectedDelays = [...]float64{
+	s.expectedDelays = [...]time.Duration{
 		opts.initial,
-		200,
-		400,
-		800,
+		200 * time.Millisecond,
+		400 * time.Millisecond,
+		800 * time.Millisecond,
 		opts.max, // Backoff delay is capped at this point.
 		opts.max,
 	}
@@ -69,11 +69,12 @@ func (s *joinDelayerTestSuite) SetupTest() {
 func (s *joinDelayerTestSuite) TestDelayWithRandomness() {
 	for i, expectedDelay := range s.expectedDelays {
 		delay := s.delayer.delay()
-		delayF := float64(delay)
 		if i == 0 {
-			s.True(delayF >= 0 && delayF < expectedDelay)
+			s.True(delay >= 0 && delay < expectedDelay,
+				"first delay should be between 0 and min")
 		} else {
-			s.True(delayF >= s.expectedDelays[i-1] && delayF <= expectedDelay)
+			s.True(delay >= s.expectedDelays[i-1] && delay <= expectedDelay,
+				"next delays should be within bounds")
 		}
 	}
 }
@@ -85,8 +86,7 @@ func (s *joinDelayerTestSuite) TestDelayWithoutRandomness() {
 
 	for _, expectedDelay := range s.expectedDelays {
 		delay := s.delayer.delay()
-		s.EqualValues(time.Duration(expectedDelay), delay,
-			"join attempt delay is correct")
+		s.EqualValues(expectedDelay, delay, "join attempt delay is correct")
 	}
 }