From: Matt Joiner <anacrolix@gmail.com>
Date: Tue, 5 Aug 2025 02:32:30 +0000 (+1000)
Subject: Convert GetRequestablePieces to an iterator
X-Git-Tag: v1.59.0~2^2~66
X-Git-Url: http://www.git.stargrave.org/?a=commitdiff_plain;h=395b9aad7901e230bdfe31e9ec097ddd297675b8;p=btrtrc.git

Convert GetRequestablePieces to an iterator
---

diff --git a/internal/request-strategy/order.go b/internal/request-strategy/order.go
index 5aee49f4..4384ea2c 100644
--- a/internal/request-strategy/order.go
+++ b/internal/request-strategy/order.go
@@ -25,7 +25,7 @@ type (
 func pieceOrderLess(i, j *PieceRequestOrderItem) multiless.Computation {
 	return multiless.New().Int(
 		int(j.State.Priority), int(i.State.Priority),
-		// TODO: Should we match on complete here to prevent churn when availability changes?
+		// TODO: Should we match on complete here to prevent churn when availability changes? (Answer: Yes).
 	).Bool(
 		j.State.Partial, i.State.Partial,
 	).Int(
@@ -42,17 +42,16 @@ func pieceOrderLess(i, j *PieceRequestOrderItem) multiless.Computation {
 	})
 }
 
-// Returns true if the piece should be considered against the unverified bytes limit. This is
-// based on whether the callee intends to request from the piece. Pieces submitted to this
-// callback passed Piece.Request and so are ready for immediate download.
+// This did return true if the piece should be considered against the unverified bytes limit. But
+// that would cause thrashing on completion: The order should be stable. This is now a 3-tuple
+// iterator.
 type RequestPieceFunc func(ih metainfo.Hash, pieceIndex int, orderState PieceRequestOrderState) bool
 
 // Calls f with requestable pieces in order.
 func GetRequestablePieces(
 	input Input, pro *PieceRequestOrder,
-	// Returns true if the piece should be considered against the unverified bytes limit. This is
-	// based on whether the callee intends to request from the piece. Pieces submitted to this
-	// callback passed Piece.Request and so are ready for immediate download.
+	// Pieces submitted to this callback passed Piece.Request and so are ready for immediate
+	// download.
 	requestPiece RequestPieceFunc,
 ) {
 	// Storage capacity left for this run, keyed by the storage capacity pointer on the storage
@@ -82,7 +81,7 @@ func GetRequestablePieces(
 			if !requestPiece(ih, item.Key.Index, item.State) {
 				// No blocks are being considered from this piece, so it won't result in unverified
 				// bytes.
-				return true
+				return false
 			}
 		} else if !piece.CountUnverified() {
 			// The piece is pristine, and we're not considering it for requests.
diff --git a/requesting.go b/requesting.go
index 63becb0a..3dd40c52 100644
--- a/requesting.go
+++ b/requesting.go
@@ -223,10 +223,10 @@ func (p *PeerConn) getDesiredRequestState() (desired desiredRequestState) {
 	t.getRequestablePieces(
 		func(ih metainfo.Hash, pieceIndex int, pieceExtra requestStrategy.PieceRequestOrderState) bool {
 			if ih != *t.canonicalShortInfohash() {
-				return false
+				return true
 			}
 			if !p.peerHasPiece(pieceIndex) {
-				return false
+				return true
 			}
 			requestHeap.pieceStates[pieceIndex].Set(pieceExtra)
 			allowedFast := p.peerAllowedFast.Contains(pieceIndex)
diff --git a/webseed-requesting.go b/webseed-requesting.go
index 15de0438..a4cc17d5 100644
--- a/webseed-requesting.go
+++ b/webseed-requesting.go
@@ -340,13 +340,12 @@ func (cl *Client) iterPossibleWebseedRequests() iter.Seq2[webseedUniqueRequestKe
 				func(ih metainfo.Hash, pieceIndex int, orderState requestStrategy.PieceRequestOrderState) bool {
 					t := cl.torrentsByShortHash[ih]
 					if len(t.webSeeds) == 0 {
-						return false
+						return true
 					}
 					p := t.piece(pieceIndex)
 					cleanOpt := p.firstCleanChunk()
 					if !cleanOpt.Ok {
-						// Could almost return true here, as clearly something is going on with the piece.
-						return false
+						return true
 					}
 					// Pretty sure we want this and not the order state priority. That one is for
 					// client piece request order and ignores other states like hashing, marking
@@ -357,7 +356,7 @@ func (cl *Client) iterPossibleWebseedRequests() iter.Seq2[webseedUniqueRequestKe
 					for url, ws := range t.webSeeds {
 						// Return value from this function (RequestPieceFunc) doesn't terminate
 						// iteration, so propagate that to not handling the yield return value.
-						yield(
+						if !yield(
 							webseedUniqueRequestKey{
 								aprioriWebseedRequestKey{
 									t:          t,
@@ -370,7 +369,9 @@ func (cl *Client) iterPossibleWebseedRequests() iter.Seq2[webseedUniqueRequestKe
 								priority: priority,
 								costKey:  ws.hostKey,
 							},
-						)
+						) {
+							return false
+						}
 					}
 					// Pieces iterated here are only to select webseed requests. There's no guarantee they're chosen.
 					return false