From 3a0970cd3b9a8a1c85e3235528ab93f03579fdd0 Mon Sep 17 00:00:00 2001
From: Mark Holt <mark@distributed.vision>
Date: Mon, 29 Apr 2024 22:05:49 +0100
Subject: [PATCH] Added comments and variables instead of raw reason strings
---
peer.go | 8 ++++++--
requesting.go | 11 +++++++++--
2 files changed, 15 insertions(+), 4 deletions(-)
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
}
--
2.51.0