partial fix to idle best peer issue

- best peer cannot be idle for more than idleBestPeerTimeout
- introduce ErrIdleTooLong fatal error
- modify default values

blockpool/blockpool.go

@@ -17,7 +17,7 @@ var plog = ethlogger.NewLogger("Blockpool")
 
 var (
 	// max number of block hashes sent in one request
-	blockHashesBatchSize = 512
+	blockHashesBatchSize = 256
 	// max number of blocks sent in one request
 	blockBatchSize = 64
 	// interval between two consecutive block checks (and requests)
@@ -27,11 +27,13 @@ var (
 	// interval between two consecutive block hash checks (and requests)
 	blockHashesRequestInterval = 3 * time.Second
 	// max number of idle iterations, ie., check through a section without new blocks coming in
-	blocksRequestMaxIdleRounds = 100
+	blocksRequestMaxIdleRounds = 20
 	// timeout interval: max time allowed for peer without sending a block hash
 	blockHashesTimeout = 60 * time.Second
 	// timeout interval: max time allowed for peer without sending a block
-	blocksTimeout = 120 * time.Second
+	blocksTimeout = 60 * time.Second
+	//
+	idleBestPeerTimeout = 60 * time.Second
 )
 
 // config embedded in components, by default fall back to constants
@@ -45,6 +47,7 @@ type Config struct {
 	BlocksRequestInterval      time.Duration
 	BlockHashesTimeout         time.Duration
 	BlocksTimeout              time.Duration
+	IdleBestPeerTimeout        time.Duration
 }
 
 // blockpool errors
@@ -53,6 +56,7 @@ const (
 	ErrInvalidPoW
 	ErrUnrequestedBlock
 	ErrInsufficientChainInfo
+	ErrIdleTooLong
 )
 
 var errorToString = map[int]string{
@@ -60,6 +64,7 @@ var errorToString = map[int]string{
 	ErrInvalidPoW:            "Invalid PoW",
 	ErrUnrequestedBlock:      "Unrequested block",
 	ErrInsufficientChainInfo: "Insufficient chain info",
+	ErrIdleTooLong:           "Idle too long",
 }
 
 // init initialises all your laundry
@@ -88,6 +93,9 @@ func (self *Config) init() {
 	if self.BlocksTimeout == 0 {
 		self.BlocksTimeout = blocksTimeout
 	}
+	if self.IdleBestPeerTimeout == 0 {
+		self.IdleBestPeerTimeout = idleBestPeerTimeout
+	}
 }
 
 // node is the basic unit of the internal model of block chain/tree in the blockpool

blockpool/config_test.go

@@ -20,12 +20,13 @@ func TestBlockPoolConfig(t *testing.T) {
 	test.CheckDuration("BlocksRequestInterval", c.BlocksRequestInterval, blocksRequestInterval, t)
 	test.CheckDuration("BlockHashesTimeout", c.BlockHashesTimeout, blockHashesTimeout, t)
 	test.CheckDuration("BlocksTimeout", c.BlocksTimeout, blocksTimeout, t)
+	test.CheckDuration("IdleBestPeerTimeout", c.IdleBestPeerTimeout, idleBestPeerTimeout, t)
 }
 
 func TestBlockPoolOverrideConfig(t *testing.T) {
 	test.LogInit()
 	blockPool := &BlockPool{Config: &Config{}}
-	c := &Config{128, 32, 1, 0, 300 * time.Millisecond, 100 * time.Millisecond, 90 * time.Second, 0}
+	c := &Config{128, 32, 1, 0, 300 * time.Millisecond, 100 * time.Millisecond, 90 * time.Second, 0, 30 * time.Second}
 
 	blockPool.Config = c
 	blockPool.Start()
@@ -37,4 +38,5 @@ func TestBlockPoolOverrideConfig(t *testing.T) {
 	test.CheckDuration("BlocksRequestInterval", c.BlocksRequestInterval, 100*time.Millisecond, t)
 	test.CheckDuration("BlockHashesTimeout", c.BlockHashesTimeout, 90*time.Second, t)
 	test.CheckDuration("BlocksTimeout", c.BlocksTimeout, blocksTimeout, t)
+	test.CheckDuration("IdleBestPeerTimeout", c.IdleBestPeerTimeout, 30*time.Second, t)
 }

blockpool/peers.go

@@ -37,7 +37,7 @@ type peer struct {
 	currentBlockC chan *types.Block
 	headSectionC  chan *section
 
-	// channels to signal peers witch and peer quit
+	// channels to signal peer switch and peer quit to section processes
 	idleC   chan bool
 	switchC chan bool
 
@@ -47,7 +47,7 @@ type peer struct {
 	// timers for head section process
 	blockHashesRequestTimer <-chan time.Time
 	blocksRequestTimer      <-chan time.Time
-	suicide                 <-chan time.Time
+	suicideC                <-chan time.Time
 
 	idle bool
 }
@@ -286,8 +286,7 @@ func (self *peers) removePeer(id string) {
 	}
 }
 
-// switchPeer launches section processes based on information about
-// shared interest and legacy of peers
+// switchPeer launches section processes
 func (self *BlockPool) switchPeer(oldp, newp *peer) {
 
 	// first quit AddBlockHashes, requestHeadSection and activateChain
@@ -372,16 +371,16 @@ func (self *peer) handleSection(sec *section) {
 			self.bp.syncing()
 		}
 
-		self.suicide = time.After(self.bp.Config.BlockHashesTimeout)
+		self.suicideC = time.After(self.bp.Config.BlockHashesTimeout)
 
 		plog.DebugDetailf("HeadSection: <%s> head block hash changed (mined block received). New head %s", self.id, hex(self.currentBlockHash))
 	} else {
 		if !self.idle {
 			self.idle = true
-			self.suicide = nil
 			self.bp.wg.Done()
 		}
 		plog.DebugDetailf("HeadSection: <%s> head section [%s] created", self.id, sectionhex(sec))
+		self.suicideC = time.After(self.bp.Config.IdleBestPeerTimeout)
 	}
 }
 
@@ -451,7 +450,7 @@ func (self *peer) getBlockHashes() {
 	self.blockHashesRequestTimer = nil
 	if !self.idle {
 		self.idle = true
-		self.suicide = nil
+		self.suicideC = time.After(self.bp.Config.IdleBestPeerTimeout)
 		self.bp.wg.Done()
 	}
 }
@@ -467,7 +466,7 @@ func (self *peer) run() {
 	self.blockHashesRequestTimer = nil
 
 	self.blocksRequestTimer = time.After(0)
-	self.suicide = time.After(self.bp.Config.BlockHashesTimeout)
+	self.suicideC = time.After(self.bp.Config.BlockHashesTimeout)
 
 	var quit chan bool
 
@@ -476,9 +475,20 @@ func (self *peer) run() {
 LOOP:
 	for {
 		select {
+		// to minitor section process behaviou
 		case <-ping.C:
 			plog.Debugf("HeadSection: <%s> section with head %s, idle: %v", self.id, hex(self.currentBlockHash), self.idle)
 
+		// idle timer started when process goes idle
+		case <-self.idleC:
+			if self.idle {
+				self.peerError(self.bp.peers.errors.New(ErrIdleTooLong, "timed out without providing new blocks...quitting", currentBlockHash))
+
+				self.bp.status.lock.Lock()
+				self.bp.status.badPeers[self.id]++
+				self.bp.status.lock.Unlock()
+			}
+
 		// signal from AddBlockHashes that head section for current best peer is created
 		// if sec == nil, it signals that chain info has updated (new block message)
 		case sec := <-self.headSectionC:
@@ -503,7 +513,7 @@ LOOP:
 			self.getCurrentBlock(nil)
 
 		// quitting on timeout
-		case <-self.suicide:
+		case <-self.suicideC:
 			self.peerError(self.bp.peers.errors.New(ErrInsufficientChainInfo, "timed out without providing block hashes or head block %x", currentBlockHash))
 
 			self.bp.status.lock.Lock()