peer.go | 8 ++++++-- requesting.go | 11 +++++++++-- diff --git a/peer.go b/peer.go index 733aa018ef29727d10df21a44e5f3aa54e033cfb..41749710772c6f751868c242e730c8a85be50b10 100644 --- a/peer.go +++ b/peer.go @@ -468,6 +468,8 @@ } return cn.peerImpl._request(ppReq), nil } +var peerUpdateRequestsPeerCancelReason = "Peer.cancel" + func (me *Peer) cancel(r RequestIndex) { if !me.deleteRequest(r) { panic("request not existing should have been guarded") @@ -480,7 +482,7 @@ } } me.decPeakRequests() if me.isLowOnRequests() { - me.updateRequests("Peer.cancel") + me.updateRequests(peerUpdateRequestsPeerCancelReason) } } @@ -566,6 +568,8 @@ f() } } +var peerUpdateRequestsRemoteRejectReason = "Peer.remoteRejectedRequest" + // Returns true if it was valid to reject the request. func (c *Peer) remoteRejectedRequest(r RequestIndex) bool { if c.deleteRequest(r) { @@ -574,7 +578,7 @@ } else if !c.requestState.Cancelled.CheckedRemove(r) { return false } if c.isLowOnRequests() { - c.updateRequests("Peer.remoteRejectedRequest") + c.updateRequests(peerUpdateRequestsRemoteRejectReason) } c.decExpectedChunkReceive(r) return true diff --git a/requesting.go b/requesting.go index 7ecb4e222169bc0c8129e731ca8d3df393db3433..c5c797e7ebf52529d05e4d426f4226273fa52ee7 100644 --- a/requesting.go +++ b/requesting.go @@ -312,13 +312,20 @@ if cap(next.Requests.requestIndexes) != cap(orig) { panic("changed") } - if p.needRequestUpdate == "Peer.remoteRejectedRequest" { + // don't add requests on reciept of a reject - because this causes request back + // to potentially permanently unresponive peers - which just adds network noise. If + // the peer can handle more requests it will send an "unchoked" message - which + // will cause it to get added back to the request queue + if p.needRequestUpdate == peerUpdateRequestsRemoteRejectReason { continue } existing := t.requestingPeer(req) if existing != nil && existing != p { - if p.needRequestUpdate == "Peer.cancel" { + // don't steal on cancel - because this is triggered by t.cancelRequest below + // which means that the cancelled can immediately try to steal back a request + // it has lost which can lead to circular cancel/add processing + if p.needRequestUpdate == peerUpdateRequestsPeerCancelReason { continue }