From 4e83adc634e00b0bbf4897bbef5b8ef0bec2de6f Mon Sep 17 00:00:00 2001
From: Dmitry Shulyak <dshulyak@mirantis.com>
Date: Sat, 10 Feb 2018 19:29:21 +0200
Subject: [PATCH] p2p: remove static peers from dial history

This change removes a peer information from dialing history when peer
is removed from static list.
It allows to force a server to re-dial concrete peer if it is needed.

In our case we are running geth node on mobile devices, and it is common
for a network connection to flap on mobile. Almost every time it flaps
or network connection is changed from cellular to wifi peers are disconnected
with read timeout. And usually it takes 30 seconds (default expiration timeout)
to recover connection with static peers after connectivity is restored.

This change allows us to reconnect with peers almost immediatly and it seems
harmless enough.
---
 p2p/dial.go      | 13 +++++++++++++
 p2p/dial_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/p2p/dial.go b/p2p/dial.go
index f5ff2c2117..4c8d39acdc 100644
--- a/p2p/dial.go
+++ b/p2p/dial.go
@@ -154,6 +154,9 @@ func (s *dialstate) addStatic(n *discover.Node) {
 func (s *dialstate) removeStatic(n *discover.Node) {
 	// This removes a task so future attempts to connect will not be made.
 	delete(s.static, n.ID)
+	// This removes a previous dial timestamp so that application
+	// can force a server to reconnect with chosen peer immediatly.
+	s.hist.remove(n.ID)
 }
 
 func (s *dialstate) newTasks(nRunning int, peers map[discover.NodeID]*Peer, now time.Time) []task {
@@ -390,6 +393,16 @@ func (h dialHistory) min() pastDial {
 }
 func (h *dialHistory) add(id discover.NodeID, exp time.Time) {
 	heap.Push(h, pastDial{id, exp})
+
+}
+func (h *dialHistory) remove(id discover.NodeID) bool {
+	for i, v := range *h {
+		if v.id == id {
+			heap.Remove(h, i)
+			return true
+		}
+	}
+	return false
 }
 func (h dialHistory) contains(id discover.NodeID) bool {
 	for _, v := range h {
diff --git a/p2p/dial_test.go b/p2p/dial_test.go
index ad18ef9abe..754463079a 100644
--- a/p2p/dial_test.go
+++ b/p2p/dial_test.go
@@ -515,6 +515,50 @@ func TestDialStateStaticDial(t *testing.T) {
 	})
 }
 
+// This test checks that static peers will be redialed immediatly if they were re-added to a static list.
+func TestDialStaticAfterReset(t *testing.T) {
+	wantStatic := []*discover.Node{
+		{ID: uintID(1)},
+		{ID: uintID(2)},
+	}
+
+	rounds := []round{
+		// Static dials are launched for the nodes that aren't yet connected.
+		{
+			peers: nil,
+			new: []task{
+				&dialTask{flags: staticDialedConn, dest: &discover.Node{ID: uintID(1)}},
+				&dialTask{flags: staticDialedConn, dest: &discover.Node{ID: uintID(2)}},
+			},
+		},
+		// No new dial tasks, all peers are connected.
+		{
+			peers: []*Peer{
+				{rw: &conn{flags: staticDialedConn, id: uintID(1)}},
+				{rw: &conn{flags: staticDialedConn, id: uintID(2)}},
+			},
+			done: []task{
+				&dialTask{flags: staticDialedConn, dest: &discover.Node{ID: uintID(1)}},
+				&dialTask{flags: staticDialedConn, dest: &discover.Node{ID: uintID(2)}},
+			},
+			new: []task{
+				&waitExpireTask{Duration: 30 * time.Second},
+			},
+		},
+	}
+	dTest := dialtest{
+		init:   newDialState(wantStatic, nil, fakeTable{}, 0, nil),
+		rounds: rounds,
+	}
+	runDialTest(t, dTest)
+	for _, n := range wantStatic {
+		dTest.init.removeStatic(n)
+		dTest.init.addStatic(n)
+	}
+	// without removing peers they will be considered recently dialed
+	runDialTest(t, dTest)
+}
+
 // This test checks that past dials are not retried for some time.
 func TestDialStateCache(t *testing.T) {
 	wantStatic := []*discover.Node{