From: Mark Holt <mark@distributed.vision>
Date: Mon, 29 Apr 2024 21:05:49 +0000 (+0100)
Subject: Added comments and variables instead of raw reason strings
X-Git-Tag: v1.56.0~8
X-Git-Url: http://www.git.stargrave.org/?a=commitdiff_plain;h=3a0970cd3b9a8a1c85e3235528ab93f03579fdd0;p=btrtrc.git

Added comments and variables instead of raw reason strings
---

diff --git a/peer.go b/peer.go
index 733aa018..41749710 100644
--- a/peer.go
+++ b/peer.go
@@ -468,6 +468,8 @@ func (cn *Peer) request(r RequestIndex) (more bool, err error) {
 	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 @@ func (me *Peer) cancel(r RequestIndex) {
 	}
 	me.decPeakRequests()
 	if me.isLowOnRequests() {
-		me.updateRequests("Peer.cancel")
+		me.updateRequests(peerUpdateRequestsPeerCancelReason)
 	}
 }
 
@@ -566,6 +568,8 @@ func runSafeExtraneous(f func()) {
 	}
 }
 
+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 @@ func (c *Peer) remoteRejectedRequest(r RequestIndex) bool {
 		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 7ecb4e22..c5c797e7 100644
--- a/requesting.go
+++ b/requesting.go
@@ -312,13 +312,20 @@ func (p *Peer) applyRequestState(next desiredRequestState) {
 			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
 			}