From fe5102580d47355883a06e1eb383337b8fdc4286 Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Wed, 3 Jun 2015 07:54:55 +0100
Subject: [PATCH] fix connection retry delay calculation logic

The delay d(n) for the nth retry was
d(1) = rand(initial_interval)
d(n+1) = d(n) + rand(d(n))

There are two problem with this:

1) The entire sequence is dominated by the random value for d(1). If
this is a small value, lots of retries at short interval ensue. In
extremis, if d(1) is 0, indefinite immediate retries would ensue.

2) The delay is not a true exponential backoff; the upper bound only
increases exponentially *probabilistically*.

The new algorithm calculates

d(n) = random_value in range [i-i/2,i+i/2],
       where i = initial_interval * 1.5^(n-1).

Thus d(1) is at least initial_interval/2, and successive values are
independent of previous values, forming a true exponential sequence of
intervals bounds between which the retry delay is chosen randomly.

Due to the change in algorithm we reduce...

- the initial interval from 5s to 2s. Recall that previously d(1) was
  in the range [0,initial_interval=5s]. It is now [2-2/2=1s,2+2/2=3s].

- the max interval from 10 minutes to 6. Previously, once that was
  reached, it would stay exactly there. It is now randomly chosen in
  the range [6-6/2=3m,6+6/2=9min].

Thus previously delays would range from 0 to 10 minutes, now they
range from 1s to 9 minutes.

Fixes #412.
---
 docs/architecture.txt      |  2 +-
 router/connection_maker.go | 20 +++++++++++---------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/docs/architecture.txt b/docs/architecture.txt
index daa56730..4b0de429 100644
--- a/docs/architecture.txt
+++ b/docs/architecture.txt
@@ -34,7 +34,7 @@ Router:
   the known locations of peers as they're discovered and is informed
   when connections we have made die. It proactively tries to create
   connections to remote peers we're not connected to using a random
-  binary exponential backoff to regulate the period between connection
+  exponential backoff to regulate the period between connection
   attempts.
 
 - Local Peer
diff --git a/router/connection_maker.go b/router/connection_maker.go
index 4f5b542d..a7c4d409 100644
--- a/router/connection_maker.go
+++ b/router/connection_maker.go
@@ -10,8 +10,8 @@ import (
 )
 
 const (
-	InitialInterval = 5 * time.Second
-	MaxInterval     = 10 * time.Minute
+	InitialInterval = 2 * time.Second
+	MaxInterval     = 6 * time.Minute
 )
 
 type peerAddrs map[string]*net.TCPAddr
@@ -30,7 +30,7 @@ type Target struct {
 	attempting  bool          // are we currently attempting to connect there?
 	lastError   error         // reason for disconnection last time
 	tryAfter    time.Time     // next time to try this address
-	tryInterval time.Duration // backoff time on next failure
+	tryInterval time.Duration // retry delay on next failure
 }
 
 type ConnectionMakerAction func() bool
@@ -304,14 +304,16 @@ func (cm *ConnectionMaker) attemptConnection(address string, acceptNewPeer bool)
 }
 
 func tryImmediately() (time.Time, time.Duration) {
-	interval := time.Duration(rand.Int63n(int64(InitialInterval)))
-	return time.Now(), interval
+	return time.Now(), InitialInterval
 }
 
+// The delay at the nth retry is a random value in the range
+// [i-i/2,i+i/2], where i = InitialInterval * 1.5^(n-1).
 func tryAfter(interval time.Duration) (time.Time, time.Duration) {
-	interval += time.Duration(rand.Int63n(int64(interval)))
-	if interval > MaxInterval {
-		interval = MaxInterval
+	delay := interval/2 + time.Duration(rand.Int63n(int64(interval)))
+	nextInterval := interval * 3 / 2
+	if nextInterval > MaxInterval {
+		nextInterval = MaxInterval
 	}
-	return time.Now().Add(interval), interval
+	return time.Now().Add(delay), nextInterval
 }